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

Chen-Yu Tsai
2019-02-05T16:48:43Z

On Wed, Jan 30, 2019 at 11:23 AM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Wed, Jan 23, 2019 at 11:54 PM Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> >
> > The current calculation for the video start delay in the current DSI driver
> > is that it is the total vertical size, minus the backporch and sync length,
> > plus 1.
> >
> > However, the Allwinner code has it as the active vertical size, plus the
> > back porch and the sync length. This doesn't make any difference on the
> > only panel it has been tested with so far, since in that particular case
> > the front porch is equal to the sum of the back porch and sync length.
> >
> > This is not the case for all panels, obviously, so we need to fix it. Since
> > the Allwinner code has a bunch of extra code to deal with out of bounds
> > values, so let's add them as well.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 7 ++++++-
> > 1 file changed, 6 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..e3e4ba90c059 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -357,7 +357,12 @@ 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;
>
> According to the diagram in include/drm/drm_modes.h ,
> This is active region plus back porch plus 1, as
>
> sync_end - display = length of front porch and sync
>
> > + u16 delay = (mode->vsync_end + 1) % mode->vtotal;
>
> And this actually means
>
> (length of active region and front porch and sync pulse plus 1) %
> total length
>
> So I don't really understand what's happening here. And the modulus
> is unexplained.

Attempting to make sense of this. Allwinner's A64 BSP the following
which uses their definitions for y, vbp, vt:

s32 start_delay = panel->lcd_vt - panel->lcd_y -10;
u32 vfp = panel->lcd_vt - panel->lcd_y - panel->lcd_vbp;
u32 dsi_start_delay;

/* put start_delay to tcon. set ready sync early to dramfreq, so
set start_delay 1 */
start_delay = 1;

dsi_start_delay = panel->lcd_vt - vfp + start_delay;
if (dsi_start_delay > panel->lcd_vt)
dsi_start_delay -= panel->lcd_vt;
if (dsi_start_delay==0)
dsi_start_delay = 1;

This can be converted to

dsi_start_delay = max(1, (mode->vtotal - (mode->vtotal -
mode->vdisplay - (mode->vtotal - mode->vsync_start)) + 1) %
mode->vtotal)

and simplified to

dsi_start_delay = max(1, (mode->vtotal + mode->vdisplay -
mode->vsync_start) + 1) % mode->vtotal)

and reordered to the following

dsi_start_delay = max(1, (mode->vtotal - (mode->vsync_start -
mode->vdisplay) + 1) % mode->vtotal)

which means total length minus front porch, or length of active
portion plus back porch plus sync.
Based on your commit message, the last derivation is what you want.

ChenYu

> > +
> > + if (!delay)
> > + delay = 1;
>
> Same comment as Paul.
>
> ChenYu
>
> > +
> > + return delay;
> > }
> >
> > static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
> > --
> > git-series 0.9.1
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel