Re: [PATCH v9 3/5] drm/sun4i: sun6i_mipi_dsi: Support vblk timing for 4-lane devices

[Jagan Teki]

On Mon, Mar 4, 2019 at 9:19 PM Maxime Ripard <maxime.ripard@xxxxxxxxxxx> wrote:
>
> On Sun, Mar 03, 2019 at 11:05:25PM +0530, Jagan Teki wrote:
> > Like other dsi setup timings, or hblk for that matter vblk would
> > also require compute the timings based payload equation along with
> > packet overhead.
> >
> > But, on the other hand vblk computation is also depends on device
> > lane number.
> > - for 4 lane devices, it is computed based on vtotal, packet overhead
> >   along with hblk value.
> > - for others devices, it is simply 0
> >
> > BSP code from BPI-M64-bsp is computing vblk as for 4-lane devices
> > (from linux-sunxi
> > drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
> >
> > tmp = (ht*dsi_pixel_bits[format]/8)*vt-(4+dsi_hblk+2);
> > dsi_vblk = (lane-tmp%lane);
> >
> > So, update the vblk timing calculation to support all type of
> > devices.
> >
> > Tested on 2-lane, 4-lane MIPI-DSI LCD panels.
>
> You should be explaining which issue you faced, in which setup, what
> were its symptoms and how that solution is fixing it.

No, it is not a fix, didn't specify either. it is vblk timings support
like others dsi timings.

>
> > Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> > Tested-by: Merlijn Wajer <merlijn@xxxxxxxxxx>
> > ---
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 27 +++++++++++++++++++-------
> >  1 file changed, 20 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index 140e55f5ed2e..b38358465d87 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -527,6 +527,24 @@ static void sun6i_dsi_setup_format(struct sun6i_dsi *dsi,
> >                    SUN6I_DSI_PIXEL_CTL0_FORMAT(fmt));
> >  }
> >
> > +static u16 sun6i_dsi_get_timings_vblk(struct sun6i_dsi *dsi,
> > +                                   struct drm_display_mode *mode, u16 hblk)
> > +{
> > +     struct mipi_dsi_device *device = dsi->device;
> > +     unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
> > +     int tmp;
> > +
> > +     /*
> > +      * The vertical blank is set using a blanking packet (4 bytes +
> > +      * payload + 2 bytes). Its minimal size is therefore 6 bytes
> > +      */
> > +#define VBLK_PACKET_OVERHEAD 6
> > +     tmp = (mode->htotal * Bpp) * mode->vtotal -
> > +           (hblk + VBLK_PACKET_OVERHEAD);
> > +
> > +     return (device->lanes - tmp % device->lanes);
>
> This should have a comment explaining why it's needed

Grabbed from BSP as I mentioned in the commit message, to satisfy the
proper dsi timing initialization.

>
> > +}
> > +
> >  static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> >                                   struct drm_display_mode *mode)
> >  {
> > @@ -586,13 +604,8 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> >                          (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp -
> >                          HBLK_PACKET_OVERHEAD);
> >
> > -             /*
> > -              * And I'm not entirely sure what vblk is about. The driver in
> > -              * Allwinner BSP is using a rather convoluted calculation
> > -              * there only for 4 lanes. However, using 0 (the !4 lanes
> > -              * case) even with a 4 lanes screen seems to work...
> > -              */
> > -             vblk = 0;
> > +             if (device->lanes == 4)
>
> And that can be done in the function itself.

Since vblk is initialized to 0 starting of the function, I'm calling
directly this for 4-lane, because for rest it's 0 on non-burst mode.