Re: [PATCH V2 1/6] drm: bridge: samsung-dsim: fix blanking packet size calculation
Am Mittwoch, dem 03.05.2023 um 22:05 +0530 schrieb Jagan Teki: > On Mon, Apr 24, 2023 at 3:17 PM Adam Ford wrote: > > > > On Mon, Apr 24, 2023 at 4:03 AM Jagan Teki > > wrote: > > > > > > On Sun, Apr 23, 2023 at 5:42 PM Adam Ford wrote: > > > > > > > > From: Lucas Stach > > > > > > > > Scale the blanking packet sizes to match the ratio between HS clock > > > > and DPI interface clock. The controller seems to do internal scaling > > > > to the number of active lanes, so we don't take those into account. > > > > > > > > Signed-off-by: Lucas Stach > > > > Signed-off-by: Adam Ford > > > > --- > > > > drivers/gpu/drm/bridge/samsung-dsim.c | 18 +++--- > > > > 1 file changed, 15 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c > > > > b/drivers/gpu/drm/bridge/samsung-dsim.c > > > > index e0a402a85787..2be3b58624c3 100644 > > > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > > > @@ -874,17 +874,29 @@ 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 hfp = (m->hsync_start - m->hdisplay) * byte_clk_khz > > > > / m->clock; > > > > > > I do not quite understand why it depends on burst_clk_rate, would you > > > please explain? does it depends on bpp something like this > > > > > > mipi_dsi_pixel_format_to_bpp(format) / 8 > > > > The pixel clock is currently set to the burst clock rate. Dividing > > the clock by 1000 gets the pixel clock in KHz, and dividing by 8 > > converts bits to bytes. > > Later in the series, I change the clock from the burst clock to the > > cached value returned from samsung_dsim_set_pll. > > Okay. > > > > > > > > > > + 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; > > > > + > > > > + /* remove packet overhead when possible */ > > > > + hfp = max(hfp - 6, 0); > > > > + hbp = max(hbp - 6, 0); > > > > + hsa = max(hsa - 6, 0); > > > > > > 6 blanking packet overhead here means, 4 bytes + payload + 2 bytes > > > format? does this packet overhead depends on the respective porch's > > > like hpf, hbp and hsa has different packet overheads? > > > > Lucas might be able to explain this better. However, it does match > > the values of the downstream NXP kernel, and I tried playing with > > these values manually, and 6 appeared to be the only number that > > seemed to work for me too. I abandoned my approach for Lucas' > > implementation, because it seemed more clear than mine. > > Maybe Lucas can chime in, since this is really his patch. > > Lucan, any inputs? > The blanking packets are are MIPI long packets, so 4 byte header, payload, 2 bytes footer. I experimented with different blanking sizes and haven't found that it would make any difference. I don't think any real-world horizontal blanking sizes would require multiple packets to be sent. Regards, Lucas
Re: [PATCH V2 1/6] drm: bridge: samsung-dsim: fix blanking packet size calculation
On Mon, Apr 24, 2023 at 3:17 PM Adam Ford wrote: > > On Mon, Apr 24, 2023 at 4:03 AM Jagan Teki wrote: > > > > On Sun, Apr 23, 2023 at 5:42 PM Adam Ford wrote: > > > > > > From: Lucas Stach > > > > > > Scale the blanking packet sizes to match the ratio between HS clock > > > and DPI interface clock. The controller seems to do internal scaling > > > to the number of active lanes, so we don't take those into account. > > > > > > Signed-off-by: Lucas Stach > > > Signed-off-by: Adam Ford > > > --- > > > drivers/gpu/drm/bridge/samsung-dsim.c | 18 +++--- > > > 1 file changed, 15 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c > > > b/drivers/gpu/drm/bridge/samsung-dsim.c > > > index e0a402a85787..2be3b58624c3 100644 > > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > > @@ -874,17 +874,29 @@ 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 hfp = (m->hsync_start - m->hdisplay) * byte_clk_khz / > > > m->clock; > > > > I do not quite understand why it depends on burst_clk_rate, would you > > please explain? does it depends on bpp something like this > > > > mipi_dsi_pixel_format_to_bpp(format) / 8 > > The pixel clock is currently set to the burst clock rate. Dividing > the clock by 1000 gets the pixel clock in KHz, and dividing by 8 > converts bits to bytes. > Later in the series, I change the clock from the burst clock to the > cached value returned from samsung_dsim_set_pll. Okay. > > > > > > + 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; > > > + > > > + /* remove packet overhead when possible */ > > > + hfp = max(hfp - 6, 0); > > > + hbp = max(hbp - 6, 0); > > > + hsa = max(hsa - 6, 0); > > > > 6 blanking packet overhead here means, 4 bytes + payload + 2 bytes > > format? does this packet overhead depends on the respective porch's > > like hpf, hbp and hsa has different packet overheads? > > Lucas might be able to explain this better. However, it does match > the values of the downstream NXP kernel, and I tried playing with > these values manually, and 6 appeared to be the only number that > seemed to work for me too. I abandoned my approach for Lucas' > implementation, because it seemed more clear than mine. > Maybe Lucas can chime in, since this is really his patch. Lucan, any inputs? Jagan.
Re: [PATCH V2 1/6] drm: bridge: samsung-dsim: fix blanking packet size calculation
On Mon, Apr 24, 2023 at 4:03 AM Jagan Teki wrote: > > On Sun, Apr 23, 2023 at 5:42 PM Adam Ford wrote: > > > > From: Lucas Stach > > > > Scale the blanking packet sizes to match the ratio between HS clock > > and DPI interface clock. The controller seems to do internal scaling > > to the number of active lanes, so we don't take those into account. > > > > Signed-off-by: Lucas Stach > > Signed-off-by: Adam Ford > > --- > > drivers/gpu/drm/bridge/samsung-dsim.c | 18 +++--- > > 1 file changed, 15 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c > > b/drivers/gpu/drm/bridge/samsung-dsim.c > > index e0a402a85787..2be3b58624c3 100644 > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > @@ -874,17 +874,29 @@ 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 hfp = (m->hsync_start - m->hdisplay) * byte_clk_khz / > > m->clock; > > I do not quite understand why it depends on burst_clk_rate, would you > please explain? does it depends on bpp something like this > > mipi_dsi_pixel_format_to_bpp(format) / 8 The pixel clock is currently set to the burst clock rate. Dividing the clock by 1000 gets the pixel clock in KHz, and dividing by 8 converts bits to bytes. Later in the series, I change the clock from the burst clock to the cached value returned from samsung_dsim_set_pll. > > > + 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; > > + > > + /* remove packet overhead when possible */ > > + hfp = max(hfp - 6, 0); > > + hbp = max(hbp - 6, 0); > > + hsa = max(hsa - 6, 0); > > 6 blanking packet overhead here means, 4 bytes + payload + 2 bytes > format? does this packet overhead depends on the respective porch's > like hpf, hbp and hsa has different packet overheads? Lucas might be able to explain this better. However, it does match the values of the downstream NXP kernel, and I tried playing with these values manually, and 6 appeared to be the only number that seemed to work for me too. I abandoned my approach for Lucas' implementation, because it seemed more clear than mine. Maybe Lucas can chime in, since this is really his patch. adam > > Jagan.
Re: [PATCH V2 1/6] drm: bridge: samsung-dsim: fix blanking packet size calculation
On Sun, Apr 23, 2023 at 5:42 PM Adam Ford wrote: > > From: Lucas Stach > > Scale the blanking packet sizes to match the ratio between HS clock > and DPI interface clock. The controller seems to do internal scaling > to the number of active lanes, so we don't take those into account. > > Signed-off-by: Lucas Stach > Signed-off-by: Adam Ford > --- > drivers/gpu/drm/bridge/samsung-dsim.c | 18 +++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c > b/drivers/gpu/drm/bridge/samsung-dsim.c > index e0a402a85787..2be3b58624c3 100644 > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > @@ -874,17 +874,29 @@ 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 hfp = (m->hsync_start - m->hdisplay) * byte_clk_khz / > m->clock; I do not quite understand why it depends on burst_clk_rate, would you please explain? does it depends on bpp something like this mipi_dsi_pixel_format_to_bpp(format) / 8 > + 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; > + > + /* remove packet overhead when possible */ > + hfp = max(hfp - 6, 0); > + hbp = max(hbp - 6, 0); > + hsa = max(hsa - 6, 0); 6 blanking packet overhead here means, 4 bytes + payload + 2 bytes format? does this packet overhead depends on the respective porch's like hpf, hbp and hsa has different packet overheads? Jagan.