diff mbox series

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

Message ID 20190303173527.31055-4-jagan@amarulasolutions.com (mailing list archive)
State New, archived
Headers show
Series drm/sun4i: sun6i_mipi_dsi: Fixes/updates | expand

Commit Message

Jagan Teki March 3, 2019, 5:35 p.m. UTC 
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.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Merlijn Wajer <merlijn@wizzup.org>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 27 +++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

Comments

Maxime Ripard March 4, 2019, 3:49 p.m. UTC | #1 
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.

> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> ---
>  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

> +}
> +
>  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.

Maxime
Jagan Teki March 7, 2019, 4:03 p.m. UTC | #2 
On Mon, Mar 4, 2019 at 9:19 PM Maxime Ripard <maxime.ripard@bootlin.com> 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@amarulasolutions.com>
> > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > ---
> >  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.
Maxime Ripard March 11, 2019, 2:04 p.m. UTC | #3 
On Thu, Mar 07, 2019 at 09:33:38PM +0530, Jagan Teki wrote:
> On Mon, Mar 4, 2019 at 9:19 PM Maxime Ripard <maxime.ripard@bootlin.com> 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.

So it's not fixing anything, and this isn't a new feature
either. What's the point then?

Maxime
Jagan Teki March 11, 2019, 2:33 p.m. UTC | #4 
On Mon, Mar 11, 2019 at 7:34 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Thu, Mar 07, 2019 at 09:33:38PM +0530, Jagan Teki wrote:
> > On Mon, Mar 4, 2019 at 9:19 PM Maxime Ripard <maxime.ripard@bootlin.com> 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.
>
> So it's not fixing anything, and this isn't a new feature
> either. What's the point then?

You can consider a feature, as comments says it's vblk support for
4-lane devices which doesn't be 0. don't know why you are asking like
a first time patch since it's commented before[1]

[1] https://patchwork.kernel.org/patch/10657543/
diff mbox series

Patch

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);
+}
+
 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)
+			vblk = sun6i_dsi_get_timings_vblk(dsi, mode, hblk);
 	}
 
 	/* How many bytes do we need to send all payloads? */