Re: [PATCH v3 2/8] drm/sun4i: dsi: Change the start delay calculation

[Paul Kocialkowski <paul.kocialkowski@bootlin.com>]

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,
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel