| Message ID | 20190303173527.31055-2-jagan@amarulasolutions.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/sun4i: sun6i_mipi_dsi: Fixes/updates | expand |
On Sun, Mar 03, 2019 at 11:05:23PM +0530, Jagan Teki wrote: > Loop N1 instruction delay for burst mode devices are computed > based on horizontal sync and porch timing values. > > The current driver is using u16 type for computing this hsync_porch > value, which would failed to fit within the u16 type for large sync > and porch timings devices. This would result in hsync_porch overflow > and eventually computed wrong instruction delay value. > > Example, timings, where it produces the overflow > { > .hdisplay = 1080, > .hsync_start = 1080 + 408, > .hsync_end = 1080 + 408 + 4, > .htotal = 1080 + 408 + 4 + 38, > } > > It reproduces the desired delay value 65487 but the correct working > value should be 7. > > So, Fix it by computing hsync_porch value separately with u32 type. > > Fixes: 1c1a7aa3663c ("drm/sun4i: dsi: Add burst support") > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > Tested-by: Merlijn Wajer <merlijn@wizzup.org> > --- > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > index 6ff585055a07..465e7fc57899 100644 > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > @@ -457,8 +457,9 @@ static void sun6i_dsi_setup_inst_loop(struct sun6i_dsi *dsi, > u16 delay = 50 - 1; > > if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) { > - delay = (mode->htotal - mode->hdisplay) * 150; > - delay /= (mode->clock / 1000) * 8; > + u32 hsync_porch = (mode->htotal - mode->hdisplay); > + > + delay = ((hsync_porch * 150) / ((mode->clock / 1000) * 8)); shouldn't we keep the multiplication by 150 in the u32 assignation? Otherwise, we could keep a u16 for the delay Maxime
On Mon, Mar 4, 2019 at 9:24 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Sun, Mar 03, 2019 at 11:05:23PM +0530, Jagan Teki wrote: > > Loop N1 instruction delay for burst mode devices are computed > > based on horizontal sync and porch timing values. > > > > The current driver is using u16 type for computing this hsync_porch > > value, which would failed to fit within the u16 type for large sync > > and porch timings devices. This would result in hsync_porch overflow > > and eventually computed wrong instruction delay value. > > > > Example, timings, where it produces the overflow > > { > > .hdisplay = 1080, > > .hsync_start = 1080 + 408, > > .hsync_end = 1080 + 408 + 4, > > .htotal = 1080 + 408 + 4 + 38, > > } > > > > It reproduces the desired delay value 65487 but the correct working > > value should be 7. > > > > So, Fix it by computing hsync_porch value separately with u32 type. > > > > Fixes: 1c1a7aa3663c ("drm/sun4i: dsi: Add burst support") > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > Tested-by: Merlijn Wajer <merlijn@wizzup.org> > > --- > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > index 6ff585055a07..465e7fc57899 100644 > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > @@ -457,8 +457,9 @@ static void sun6i_dsi_setup_inst_loop(struct sun6i_dsi *dsi, > > u16 delay = 50 - 1; > > > > if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) { > > - delay = (mode->htotal - mode->hdisplay) * 150; > > - delay /= (mode->clock / 1000) * 8; > > + u32 hsync_porch = (mode->htotal - mode->hdisplay); > > + > > + delay = ((hsync_porch * 150) / ((mode->clock / 1000) * 8)); > > shouldn't we keep the multiplication by 150 in the u32 assignation? Yes, ie true. will move it.
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 6ff585055a07..465e7fc57899 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -457,8 +457,9 @@ static void sun6i_dsi_setup_inst_loop(struct sun6i_dsi *dsi, u16 delay = 50 - 1; if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) { - delay = (mode->htotal - mode->hdisplay) * 150; - delay /= (mode->clock / 1000) * 8; + u32 hsync_porch = (mode->htotal - mode->hdisplay); + + delay = ((hsync_porch * 150) / ((mode->clock / 1000) * 8)); delay -= 50; }
Loop N1 instruction delay for burst mode devices are computed based on horizontal sync and porch timing values. The current driver is using u16 type for computing this hsync_porch value, which would failed to fit within the u16 type for large sync and porch timings devices. This would result in hsync_porch overflow and eventually computed wrong instruction delay value. Example, timings, where it produces the overflow { .hdisplay = 1080, .hsync_start = 1080 + 408, .hsync_end = 1080 + 408 + 4, .htotal = 1080 + 408 + 4 + 38, } It reproduces the desired delay value 65487 but the correct working value should be 7. So, Fix it by computing hsync_porch value separately with u32 type. Fixes: 1c1a7aa3663c ("drm/sun4i: dsi: Add burst support") Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> Tested-by: Merlijn Wajer <merlijn@wizzup.org> --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)