Patchwork [v3,2/8] drm/sun4i: dsi: Change the start delay calculation

login
register
mail settings
Submitter Maxime Ripard
Date Feb. 11, 2019, 2:41 p.m.
Message ID <6e5f72e68f47ca0223877464bf12f0c3f3978de8.1549896081.git-series.maxime.ripard@bootlin.com>
Download mbox | patch
Permalink /patch/722999/
State New
Headers show

Comments

Maxime Ripard - Feb. 11, 2019, 2:41 p.m.
The current calculation for the video start delay in the current DSI driver
is that it is the total vertical size, minus the front porch and sync length,
plus 1. This equals to the active vertical size plus the back porch plus 1.

That 1 is coming in the Allwinner BSP from an variable that is set to 1.
However, if we look at the Allwinner BSP more closely, and especially in
the "legacy" code for the display (in drivers/video/sunxi/legacy/), we can
see that this variable is actually computed from the porches and the sync
minus 10, clamped between 8 and 100.

This fixes the start delay symptom we've seen on some panels (vblank
timeouts with vertical white stripes at the bottom of the panel).

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Paul Kocialkowski - Feb. 12, 2019, 3:28 p.m.
Hi,

On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard wrote:
> The current calculation for the video start delay in the current DSI driver
> is that it is the total vertical size, minus the front porch and sync length,
> plus 1. This equals to the active vertical size plus the back porch plus 1.
> 
> That 1 is coming in the Allwinner BSP from an variable that is set to 1.
> However, if we look at the Allwinner BSP more closely, and especially in
> the "legacy" code for the display (in drivers/video/sunxi/legacy/), we can
> see that this variable is actually computed from the porches and the sync
> minus 10, clamped between 8 and 100.

For future reference, it look like there is a version of the BSP where
this is clamped between 8 and 31 in the legacy ebios code:
https://github.com/Icenowy/linux-kernel-lichee-a33/blob/master/drivers/video/sunxi/legacy/disp/de_bsp/de/ebios/de_dsi.c#L687

As well as in the A23 code:
https://github.com/Icenowy/linux-kernel-lichee-a33/blob/master/drivers/video/sunxi/disp/de/lowlevel_sun8iw3/de_dsi.c#L686

> This fixes the start delay symptom we've seen on some panels (vblank
> timeouts with vertical white stripes at the bottom of the panel).
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 380fc527a707..9471fa695ec7 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -357,7 +357,9 @@ static void sun6i_dsi_inst_init(struct sun6i_dsi *dsi,
>  static u16 sun6i_dsi_get_video_start_delay(struct sun6i_dsi *dsi,
>  					   struct drm_display_mode *mode)
>  {
> -	return mode->vtotal - (mode->vsync_end - mode->vdisplay) + 1;
> +	u16 start = clamp(mode->vtotal - mode->vdisplay - 10, 8, 100);
> +
> +	return mode->vtotal - (mode->vsync_end - mode->vdisplay) + start;

Looking at the formula from the BSP, I get a different result from
yours.

From what I can see, the BSP formula is:
start_delay = vtotal - vfp + start
vfp = vtotal - y - vbp

With vbp counting the back porch and sync length.
So the final value can be simplified:
start_delay = vbp + y + start

Translated to DRM:
start_delay = vtotal - vsync_start + vdisplay + start

Which differs from your formula by the sync length.

Does this also work out for your hardware? I guess it's best to stick
close to the BSP if we can.

Cheers,

Paul

>  }
>  
>  static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
Maxime Ripard - Feb. 12, 2019, 3:45 p.m.
On Tue, Feb 12, 2019 at 04:28:13PM +0100, Paul Kocialkowski wrote:
> Hi,
> 
> On Mon, 2019-02-11 at 15:41 +0100, Maxime Ripard wrote:
> > The current calculation for the video start delay in the current DSI driver
> > is that it is the total vertical size, minus the front porch and sync length,
> > plus 1. This equals to the active vertical size plus the back porch plus 1.
> > 
> > That 1 is coming in the Allwinner BSP from an variable that is set to 1.
> > However, if we look at the Allwinner BSP more closely, and especially in
> > the "legacy" code for the display (in drivers/video/sunxi/legacy/), we can
> > see that this variable is actually computed from the porches and the sync
> > minus 10, clamped between 8 and 100.
> 
> For future reference, it look like there is a version of the BSP where
> this is clamped between 8 and 31 in the legacy ebios code:
> https://github.com/Icenowy/linux-kernel-lichee-a33/blob/master/drivers/video/sunxi/legacy/disp/de_bsp/de/ebios/de_dsi.c#L687
> 
> As well as in the A23 code:
> https://github.com/Icenowy/linux-kernel-lichee-a33/blob/master/drivers/video/sunxi/disp/de/lowlevel_sun8iw3/de_dsi.c#L686
> 
> > This fixes the start delay symptom we've seen on some panels (vblank
> > timeouts with vertical white stripes at the bottom of the panel).
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index 380fc527a707..9471fa695ec7 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -357,7 +357,9 @@ static void sun6i_dsi_inst_init(struct sun6i_dsi *dsi,
> >  static u16 sun6i_dsi_get_video_start_delay(struct sun6i_dsi *dsi,
> >  					   struct drm_display_mode *mode)
> >  {
> > -	return mode->vtotal - (mode->vsync_end - mode->vdisplay) + 1;
> > +	u16 start = clamp(mode->vtotal - mode->vdisplay - 10, 8, 100);
> > +
> > +	return mode->vtotal - (mode->vsync_end - mode->vdisplay) + start;
> 
> Looking at the formula from the BSP, I get a different result from
> yours.
> 
> From what I can see, the BSP formula is:
> start_delay = vtotal - vfp + start
> vfp = vtotal - y - vbp
> 
> With vbp counting the back porch and sync length.

vbp is only the back porch.

Maxime

Patch

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 380fc527a707..9471fa695ec7 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -357,7 +357,9 @@  static void sun6i_dsi_inst_init(struct sun6i_dsi *dsi,
 static u16 sun6i_dsi_get_video_start_delay(struct sun6i_dsi *dsi,
 					   struct drm_display_mode *mode)
 {
-	return mode->vtotal - (mode->vsync_end - mode->vdisplay) + 1;
+	u16 start = clamp(mode->vtotal - mode->vdisplay - 10, 8, 100);
+
+	return mode->vtotal - (mode->vsync_end - mode->vdisplay) + start;
 }
 
 static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,