The high-speed clock is hard-coded to the burst-clock
frequency specified in the device tree. However, when
using devices like certain bridge chips without burst mode
and varying resolutions and refresh rates, it may be
necessary to set the high-speed clock dynamically based
on the desired pixel clock for the connected device.
This also removes the need to set a clock speed from
the device tree for non-burst mode operation, since the
pixel clock rate is the rate requested from the attached
device like a bridge chip. This should have no impact
for people using burst-mode and setting the burst clock
rate is still required for those users. If the burst
clock is not present, change the error message to
dev_info indicating the clock use the pixel clock.
Signed-off-by: Adam Ford <[email protected]>
Tested-by: Chen-Yu Tsai <[email protected]>
Tested-by: Frieder Schrempf <[email protected]>
Reviewed-by: Frieder Schrempf <[email protected]>
---
drivers/gpu/drm/bridge/samsung-dsim.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 3944b7cfbbdf..03b21d13f067 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -655,16 +655,28 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,
dsi->hs_clock = fout;
+ dsi->hs_clock = fout;
+
return fout;
}
static int samsung_dsim_enable_clock(struct samsung_dsim *dsi)
{
- unsigned long hs_clk, byte_clk, esc_clk;
+ unsigned long hs_clk, byte_clk, esc_clk, pix_clk;
unsigned long esc_div;
u32 reg;
+ struct drm_display_mode *m = &dsi->mode;
+ int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
+
+ /* m->clock is in KHz */
+ pix_clk = m->clock * 1000;
+
+ /* Use burst_clk_rate if available, otherwise use the pix_clk */
+ if (dsi->burst_clk_rate)
+ hs_clk = samsung_dsim_set_pll(dsi, dsi->burst_clk_rate);
+ else
+ hs_clk = samsung_dsim_set_pll(dsi, DIV_ROUND_UP(pix_clk * bpp, dsi->lanes));
- hs_clk = samsung_dsim_set_pll(dsi, dsi->burst_clk_rate);
if (!hs_clk) {
dev_err(dsi->dev, "failed to configure DSI PLL\n");
return -EFAULT;
@@ -935,7 +947,7 @@ static void samsung_dsim_set_display_mode(struct samsung_dsim *dsi)
u32 reg;
if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
- int byte_clk_khz = dsi->burst_clk_rate / 1000 / 8;
+ int byte_clk_khz = dsi->hs_clock / 1000 / 8;
int hfp = (m->hsync_start - m->hdisplay) * byte_clk_khz / m->clock;
int hbp = (m->htotal - m->hsync_end) * byte_clk_khz / m->clock;
int hsa = (m->hsync_end - m->hsync_start) * byte_clk_khz / m->clock;
@@ -1785,10 +1797,13 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
return PTR_ERR(pll_clk);
}
+ /* If it doesn't exist, use pixel clock instead of failing */
ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
- &dsi->burst_clk_rate, 0);
- if (ret < 0)
- return ret;
+ &dsi->burst_clk_rate, 1);
+ if (ret < 0) {
+ dev_info(dev, "Using pixel clock for HS clock frequency\n");
+ dsi->burst_clk_rate = 0;
+ }
ret = samsung_dsim_of_read_u32(node, "samsung,esc-clock-frequency",
&dsi->esc_clk_rate, 0);
--
2.39.2
On Tue, May 16, 2023 at 7:57 AM Adam Ford <[email protected]> wrote:
>
> The high-speed clock is hard-coded to the burst-clock
> frequency specified in the device tree. However, when
> using devices like certain bridge chips without burst mode
> and varying resolutions and refresh rates, it may be
> necessary to set the high-speed clock dynamically based
> on the desired pixel clock for the connected device.
>
> This also removes the need to set a clock speed from
> the device tree for non-burst mode operation, since the
> pixel clock rate is the rate requested from the attached
> device like a bridge chip. This should have no impact
> for people using burst-mode and setting the burst clock
> rate is still required for those users. If the burst
> clock is not present, change the error message to
> dev_info indicating the clock use the pixel clock.
>
> Signed-off-by: Adam Ford <[email protected]>
> Tested-by: Chen-Yu Tsai <[email protected]>
> Tested-by: Frieder Schrempf <[email protected]>
> Reviewed-by: Frieder Schrempf <[email protected]>
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 3944b7cfbbdf..03b21d13f067 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -655,16 +655,28 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,
>
> dsi->hs_clock = fout;
>
> + dsi->hs_clock = fout;
> +
Not sure about the double assignment. Was this caused by a rebase?
ChenYu
> return fout;
> }
>
> static int samsung_dsim_enable_clock(struct samsung_dsim *dsi)
> {
> - unsigned long hs_clk, byte_clk, esc_clk;
> + unsigned long hs_clk, byte_clk, esc_clk, pix_clk;
> unsigned long esc_div;
> u32 reg;
> + struct drm_display_mode *m = &dsi->mode;
> + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> +
> + /* m->clock is in KHz */
> + pix_clk = m->clock * 1000;
> +
> + /* Use burst_clk_rate if available, otherwise use the pix_clk */
> + if (dsi->burst_clk_rate)
> + hs_clk = samsung_dsim_set_pll(dsi, dsi->burst_clk_rate);
> + else
> + hs_clk = samsung_dsim_set_pll(dsi, DIV_ROUND_UP(pix_clk * bpp, dsi->lanes));
>
> - hs_clk = samsung_dsim_set_pll(dsi, dsi->burst_clk_rate);
> if (!hs_clk) {
> dev_err(dsi->dev, "failed to configure DSI PLL\n");
> return -EFAULT;
> @@ -935,7 +947,7 @@ static void samsung_dsim_set_display_mode(struct samsung_dsim *dsi)
> u32 reg;
>
> if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
> - int byte_clk_khz = dsi->burst_clk_rate / 1000 / 8;
> + int byte_clk_khz = dsi->hs_clock / 1000 / 8;
> int hfp = (m->hsync_start - m->hdisplay) * byte_clk_khz / m->clock;
> int hbp = (m->htotal - m->hsync_end) * byte_clk_khz / m->clock;
> int hsa = (m->hsync_end - m->hsync_start) * byte_clk_khz / m->clock;
> @@ -1785,10 +1797,13 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
> return PTR_ERR(pll_clk);
> }
>
> + /* If it doesn't exist, use pixel clock instead of failing */
> ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
> - &dsi->burst_clk_rate, 0);
> - if (ret < 0)
> - return ret;
> + &dsi->burst_clk_rate, 1);
> + if (ret < 0) {
> + dev_info(dev, "Using pixel clock for HS clock frequency\n");
> + dsi->burst_clk_rate = 0;
> + }
>
> ret = samsung_dsim_of_read_u32(node, "samsung,esc-clock-frequency",
> &dsi->esc_clk_rate, 0);
> --
> 2.39.2
>
On Mon, May 15, 2023 at 10:26 PM Chen-Yu Tsai <[email protected]> wrote:
>
> On Tue, May 16, 2023 at 7:57 AM Adam Ford <[email protected]> wrote:
> >
> > The high-speed clock is hard-coded to the burst-clock
> > frequency specified in the device tree. However, when
> > using devices like certain bridge chips without burst mode
> > and varying resolutions and refresh rates, it may be
> > necessary to set the high-speed clock dynamically based
> > on the desired pixel clock for the connected device.
> >
> > This also removes the need to set a clock speed from
> > the device tree for non-burst mode operation, since the
> > pixel clock rate is the rate requested from the attached
> > device like a bridge chip. This should have no impact
> > for people using burst-mode and setting the burst clock
> > rate is still required for those users. If the burst
> > clock is not present, change the error message to
> > dev_info indicating the clock use the pixel clock.
> >
> > Signed-off-by: Adam Ford <[email protected]>
> > Tested-by: Chen-Yu Tsai <[email protected]>
> > Tested-by: Frieder Schrempf <[email protected]>
> > Reviewed-by: Frieder Schrempf <[email protected]>
> > ---
> > drivers/gpu/drm/bridge/samsung-dsim.c | 27 +++++++++++++++++++++------
> > 1 file changed, 21 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index 3944b7cfbbdf..03b21d13f067 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -655,16 +655,28 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,
> >
> > dsi->hs_clock = fout;
> >
> > + dsi->hs_clock = fout;
> > +
>
> Not sure about the double assignment. Was this caused by a rebase?
Oops,
I moved this to the previous patch since the updated dphy changes
needed to know the hs_clock. I must forgot to check this when I
applied the subsequent patch, so the double assignment appeared. I am
surprised the patch tool didn't complain. I guess the good news is
that nothing is broken, but the bad news is I have to spam everyone
with a V7. I'll wait a couple days to see if anything finds anything
else.
adam
>
> ChenYu
>
> > return fout;
> > }
> >
> > static int samsung_dsim_enable_clock(struct samsung_dsim *dsi)
> > {
> > - unsigned long hs_clk, byte_clk, esc_clk;
> > + unsigned long hs_clk, byte_clk, esc_clk, pix_clk;
> > unsigned long esc_div;
> > u32 reg;
> > + struct drm_display_mode *m = &dsi->mode;
> > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> > +
> > + /* m->clock is in KHz */
> > + pix_clk = m->clock * 1000;
> > +
> > + /* Use burst_clk_rate if available, otherwise use the pix_clk */
> > + if (dsi->burst_clk_rate)
> > + hs_clk = samsung_dsim_set_pll(dsi, dsi->burst_clk_rate);
> > + else
> > + hs_clk = samsung_dsim_set_pll(dsi, DIV_ROUND_UP(pix_clk * bpp, dsi->lanes));
> >
> > - hs_clk = samsung_dsim_set_pll(dsi, dsi->burst_clk_rate);
> > if (!hs_clk) {
> > dev_err(dsi->dev, "failed to configure DSI PLL\n");
> > return -EFAULT;
> > @@ -935,7 +947,7 @@ static void samsung_dsim_set_display_mode(struct samsung_dsim *dsi)
> > u32 reg;
> >
> > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
> > - int byte_clk_khz = dsi->burst_clk_rate / 1000 / 8;
> > + int byte_clk_khz = dsi->hs_clock / 1000 / 8;
> > int hfp = (m->hsync_start - m->hdisplay) * byte_clk_khz / m->clock;
> > int hbp = (m->htotal - m->hsync_end) * byte_clk_khz / m->clock;
> > int hsa = (m->hsync_end - m->hsync_start) * byte_clk_khz / m->clock;
> > @@ -1785,10 +1797,13 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
> > return PTR_ERR(pll_clk);
> > }
> >
> > + /* If it doesn't exist, use pixel clock instead of failing */
> > ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
> > - &dsi->burst_clk_rate, 0);
> > - if (ret < 0)
> > - return ret;
> > + &dsi->burst_clk_rate, 1);
> > + if (ret < 0) {
> > + dev_info(dev, "Using pixel clock for HS clock frequency\n");
> > + dsi->burst_clk_rate = 0;
> > + }
> >
> > ret = samsung_dsim_of_read_u32(node, "samsung,esc-clock-frequency",
> > &dsi->esc_clk_rate, 0);
> > --
> > 2.39.2
> >
Am Montag, dem 15.05.2023 um 18:57 -0500 schrieb Adam Ford:
> The high-speed clock is hard-coded to the burst-clock
> frequency specified in the device tree. However, when
> using devices like certain bridge chips without burst mode
> and varying resolutions and refresh rates, it may be
> necessary to set the high-speed clock dynamically based
> on the desired pixel clock for the connected device.
>
> This also removes the need to set a clock speed from
> the device tree for non-burst mode operation, since the
> pixel clock rate is the rate requested from the attached
> device like a bridge chip.
>
Same as with the earlier patch, this needs to be documented in the DT
binding by moving "samsung,burst-clock-frequency" to be a optional
property.
Regards,
Lucas
> This should have no impact
> for people using burst-mode and setting the burst clock
> rate is still required for those users. If the burst
> clock is not present, change the error message to
> dev_info indicating the clock use the pixel clock.
>
> Signed-off-by: Adam Ford <[email protected]>
> Tested-by: Chen-Yu Tsai <[email protected]>
> Tested-by: Frieder Schrempf <[email protected]>
> Reviewed-by: Frieder Schrempf <[email protected]>
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 3944b7cfbbdf..03b21d13f067 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -655,16 +655,28 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,
>
> dsi->hs_clock = fout;
>
> + dsi->hs_clock = fout;
> +
> return fout;
> }
>
> static int samsung_dsim_enable_clock(struct samsung_dsim *dsi)
> {
> - unsigned long hs_clk, byte_clk, esc_clk;
> + unsigned long hs_clk, byte_clk, esc_clk, pix_clk;
> unsigned long esc_div;
> u32 reg;
> + struct drm_display_mode *m = &dsi->mode;
> + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> +
> + /* m->clock is in KHz */
> + pix_clk = m->clock * 1000;
> +
> + /* Use burst_clk_rate if available, otherwise use the pix_clk */
> + if (dsi->burst_clk_rate)
> + hs_clk = samsung_dsim_set_pll(dsi, dsi->burst_clk_rate);
> + else
> + hs_clk = samsung_dsim_set_pll(dsi, DIV_ROUND_UP(pix_clk * bpp, dsi->lanes));
>
> - hs_clk = samsung_dsim_set_pll(dsi, dsi->burst_clk_rate);
> if (!hs_clk) {
> dev_err(dsi->dev, "failed to configure DSI PLL\n");
> return -EFAULT;
> @@ -935,7 +947,7 @@ static void samsung_dsim_set_display_mode(struct samsung_dsim *dsi)
> u32 reg;
>
> if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
> - int byte_clk_khz = dsi->burst_clk_rate / 1000 / 8;
> + int byte_clk_khz = dsi->hs_clock / 1000 / 8;
> int hfp = (m->hsync_start - m->hdisplay) * byte_clk_khz / m->clock;
> int hbp = (m->htotal - m->hsync_end) * byte_clk_khz / m->clock;
> int hsa = (m->hsync_end - m->hsync_start) * byte_clk_khz / m->clock;
> @@ -1785,10 +1797,13 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
> return PTR_ERR(pll_clk);
> }
>
> + /* If it doesn't exist, use pixel clock instead of failing */
> ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
> - &dsi->burst_clk_rate, 0);
> - if (ret < 0)
> - return ret;
> + &dsi->burst_clk_rate, 1);
> + if (ret < 0) {
> + dev_info(dev, "Using pixel clock for HS clock frequency\n");
> + dsi->burst_clk_rate = 0;
> + }
>
> ret = samsung_dsim_of_read_u32(node, "samsung,esc-clock-frequency",
> &dsi->esc_clk_rate, 0);
On Tue, May 16, 2023 at 5:27 AM Adam Ford <[email protected]> wrote:
>
> The high-speed clock is hard-coded to the burst-clock
> frequency specified in the device tree. However, when
> using devices like certain bridge chips without burst mode
> and varying resolutions and refresh rates, it may be
> necessary to set the high-speed clock dynamically based
> on the desired pixel clock for the connected device.
>
> This also removes the need to set a clock speed from
> the device tree for non-burst mode operation, since the
> pixel clock rate is the rate requested from the attached
> device like a bridge chip. This should have no impact
> for people using burst-mode and setting the burst clock
> rate is still required for those users. If the burst
> clock is not present, change the error message to
> dev_info indicating the clock use the pixel clock.
>
> Signed-off-by: Adam Ford <[email protected]>
> Tested-by: Chen-Yu Tsai <[email protected]>
> Tested-by: Frieder Schrempf <[email protected]>
> Reviewed-by: Frieder Schrempf <[email protected]>
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 3944b7cfbbdf..03b21d13f067 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -655,16 +655,28 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,
>
> dsi->hs_clock = fout;
>
> + dsi->hs_clock = fout;
I dropped this and tested it.
Reviewed-by: Jagan Teki <[email protected]>
Tested-by: Jagan Teki <[email protected]> # imx8mm-icore