| Message ID | 20181026144344.27778-13-jagan@amarulasolutions.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/sun4i: Allwinner A64 MIPI-DSI support | expand |
On Fri, Oct 26, 2018 at 08:13:31PM +0530, Jagan Teki wrote: > Add 10 bytes packet overhead for hblk where blank is set using > a blanking packet like (4 bytes + 4 bytes + payload + 2 bytes) > > This is according to BSP code from BPI-M64-bsp > (in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) > dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2); > > So, add 10 bytes packet overhead for DSI hblk. > > Tested on 2-lane, 4-lane MIPI-DSI LCD panels. > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > Tested-by: Jagan Teki <jagan@amarulasolutions.com> > --- > Changes for v3: > - new patch > Changes for v2: > - none > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > index 596e560263bf..cf42be1f1ba1 100644 > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > @@ -461,7 +461,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, > { > struct mipi_dsi_device *device = dsi->device; > unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8; > - u16 hbp, hfp, hsa, hblk, vblk; > + u16 hbp, hfp, hsa, hblk_max, hblk, vblk; > size_t bytes; > u8 *buffer; > > @@ -494,8 +494,13 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, > > /* > * hblk seems to be the line + porches length. > + * The blank is set using a blanking packet (4 bytes + 4 bytes + > + * payload + 2 bytes). So minimal size is 10 bytes > */ > - hblk = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp; > +#define HBLK_PACKET_OVERHEAD 10 > + hblk_max = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp; > + hblk_max -= HBLK_PACKET_OVERHEAD; > + hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, hblk_max); I'd rather use the same convention than the other assignments done before in that function. Maxime
On Mon, Oct 29, 2018 at 2:52 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Fri, Oct 26, 2018 at 08:13:31PM +0530, Jagan Teki wrote: > > Add 10 bytes packet overhead for hblk where blank is set using > > a blanking packet like (4 bytes + 4 bytes + payload + 2 bytes) > > > > This is according to BSP code from BPI-M64-bsp > > (in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) > > dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2); > > > > So, add 10 bytes packet overhead for DSI hblk. > > > > Tested on 2-lane, 4-lane MIPI-DSI LCD panels. > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > Tested-by: Jagan Teki <jagan@amarulasolutions.com> > > --- > > Changes for v3: > > - new patch > > Changes for v2: > > - none > > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > index 596e560263bf..cf42be1f1ba1 100644 > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > @@ -461,7 +461,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, > > { > > struct mipi_dsi_device *device = dsi->device; > > unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8; > > - u16 hbp, hfp, hsa, hblk, vblk; > > + u16 hbp, hfp, hsa, hblk_max, hblk, vblk; > > size_t bytes; > > u8 *buffer; > > > > @@ -494,8 +494,13 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, > > > > /* > > * hblk seems to be the line + porches length. > > + * The blank is set using a blanking packet (4 bytes + 4 bytes + > > + * payload + 2 bytes). So minimal size is 10 bytes > > */ > > - hblk = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp; > > +#define HBLK_PACKET_OVERHEAD 10 > > + hblk_max = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp; > > + hblk_max -= HBLK_PACKET_OVERHEAD; > > + hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, hblk_max); > > I'd rather use the same convention than the other assignments done > before in that function. Yes, old code did use hsa. but it's added 10 bytes packet head instead of subtracting it. hblk = mode->htotal * Bpp - hsa; => mode->htotal * Bpp - (mode->hsync_end - mode->hsync_start) * Bpp - HSA_PACKET_OVERHEAD); => (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp + HSA_PACKET_OVERHEAD; And it should be (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp - HSA_PACKET_OVERHEAD; This patch is simply doing the same by explicitly adding packet over ahead macro, which again used in hfp.
On Mon, Oct 29, 2018 at 07:56:27PM +0530, Jagan Teki wrote: > On Mon, Oct 29, 2018 at 2:52 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > On Fri, Oct 26, 2018 at 08:13:31PM +0530, Jagan Teki wrote: > > > Add 10 bytes packet overhead for hblk where blank is set using > > > a blanking packet like (4 bytes + 4 bytes + payload + 2 bytes) > > > > > > This is according to BSP code from BPI-M64-bsp > > > (in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) > > > dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2); > > > > > > So, add 10 bytes packet overhead for DSI hblk. > > > > > > Tested on 2-lane, 4-lane MIPI-DSI LCD panels. > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > > Tested-by: Jagan Teki <jagan@amarulasolutions.com> > > > --- > > > Changes for v3: > > > - new patch > > > Changes for v2: > > > - none > > > > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 9 +++++++-- > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > index 596e560263bf..cf42be1f1ba1 100644 > > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > > > @@ -461,7 +461,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, > > > { > > > struct mipi_dsi_device *device = dsi->device; > > > unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8; > > > - u16 hbp, hfp, hsa, hblk, vblk; > > > + u16 hbp, hfp, hsa, hblk_max, hblk, vblk; > > > size_t bytes; > > > u8 *buffer; > > > > > > @@ -494,8 +494,13 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, > > > > > > /* > > > * hblk seems to be the line + porches length. > > > + * The blank is set using a blanking packet (4 bytes + 4 bytes + > > > + * payload + 2 bytes). So minimal size is 10 bytes > > > */ > > > - hblk = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp; > > > +#define HBLK_PACKET_OVERHEAD 10 > > > + hblk_max = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp; > > > + hblk_max -= HBLK_PACKET_OVERHEAD; > > > + hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, hblk_max); > > > > I'd rather use the same convention than the other assignments done > > before in that function. > > Yes, old code did use hsa. but it's added 10 bytes packet head instead > of subtracting it. > > hblk = mode->htotal * Bpp - hsa; > => mode->htotal * Bpp - (mode->hsync_end - mode->hsync_start) * Bpp - > HSA_PACKET_OVERHEAD); > => (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp + > HSA_PACKET_OVERHEAD; > > And it should be > (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp - > HSA_PACKET_OVERHEAD; > > This patch is simply doing the same by explicitly adding packet over > ahead macro, which again used in hfp. That's not my point. The rest of the driver uses a construct that would be: hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp - HBLK_PACKET_OVERHEAD); We want to remain consistent. Maxime
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 596e560263bf..cf42be1f1ba1 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -461,7 +461,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, { struct mipi_dsi_device *device = dsi->device; unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8; - u16 hbp, hfp, hsa, hblk, vblk; + u16 hbp, hfp, hsa, hblk_max, hblk, vblk; size_t bytes; u8 *buffer; @@ -494,8 +494,13 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, /* * hblk seems to be the line + porches length. + * The blank is set using a blanking packet (4 bytes + 4 bytes + + * payload + 2 bytes). So minimal size is 10 bytes */ - hblk = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp; +#define HBLK_PACKET_OVERHEAD 10 + hblk_max = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp; + hblk_max -= HBLK_PACKET_OVERHEAD; + hblk = max((unsigned int)HBLK_PACKET_OVERHEAD, hblk_max); /* * And I'm not entirely sure what vblk is about. The driver in