* [PATCH v9 1/5] drm/sun4i: sun6i_mipi_dsi: Fix hsync_porch overflow
2019-03-03 17:35 [PATCH v9 0/5] drm/sun4i: sun6i_mipi_dsi: Fixes/updates Jagan Teki
@ 2019-03-03 17:35 ` Jagan Teki
2019-03-04 15:54 ` Maxime Ripard
2019-03-03 17:35 ` [PATCH v9 2/5] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits Jagan Teki
` (4 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Jagan Teki @ 2019-03-03 17:35 UTC (permalink / raw)
To: Maxime Ripard, David Airlie, Daniel Vetter, Chen-Yu Tsai
Cc: dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
Michael Trimarchi, linux-sunxi, Jagan Teki
Loop N1 instruction delay for burst mode devices are computed
based on horizontal sync and porch timing values.
The current driver is using u16 type for computing this hsync_porch
value, which would failed to fit within the u16 type for large sync
and porch timings devices. This would result in hsync_porch overflow
and eventually computed wrong instruction delay value.
Example, timings, where it produces the overflow
{
.hdisplay = 1080,
.hsync_start = 1080 + 408,
.hsync_end = 1080 + 408 + 4,
.htotal = 1080 + 408 + 4 + 38,
}
It reproduces the desired delay value 65487 but the correct working
value should be 7.
So, Fix it by computing hsync_porch value separately with u32 type.
Fixes: 1c1a7aa3663c ("drm/sun4i: dsi: Add burst support")
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Merlijn Wajer <merlijn@wizzup.org>
---
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 6ff585055a07..465e7fc57899 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -457,8 +457,9 @@ static void sun6i_dsi_setup_inst_loop(struct sun6i_dsi *dsi,
u16 delay = 50 - 1;
if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
- delay = (mode->htotal - mode->hdisplay) * 150;
- delay /= (mode->clock / 1000) * 8;
+ u32 hsync_porch = (mode->htotal - mode->hdisplay);
+
+ delay = ((hsync_porch * 150) / ((mode->clock / 1000) * 8));
delay -= 50;
}
--
2.18.0.321.gffc6fa0e3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v9 1/5] drm/sun4i: sun6i_mipi_dsi: Fix hsync_porch overflow
2019-03-03 17:35 ` [PATCH v9 1/5] drm/sun4i: sun6i_mipi_dsi: Fix hsync_porch overflow Jagan Teki
@ 2019-03-04 15:54 ` Maxime Ripard
2019-03-06 19:02 ` Jagan Teki
0 siblings, 1 reply; 24+ messages in thread
From: Maxime Ripard @ 2019-03-04 15:54 UTC (permalink / raw)
To: Jagan Teki
Cc: David Airlie, Daniel Vetter, Chen-Yu Tsai, dri-devel,
linux-arm-kernel, linux-kernel, linux-amarula, Michael Trimarchi,
linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 1986 bytes --]
On Sun, Mar 03, 2019 at 11:05:23PM +0530, Jagan Teki wrote:
> Loop N1 instruction delay for burst mode devices are computed
> based on horizontal sync and porch timing values.
>
> The current driver is using u16 type for computing this hsync_porch
> value, which would failed to fit within the u16 type for large sync
> and porch timings devices. This would result in hsync_porch overflow
> and eventually computed wrong instruction delay value.
>
> Example, timings, where it produces the overflow
> {
> .hdisplay = 1080,
> .hsync_start = 1080 + 408,
> .hsync_end = 1080 + 408 + 4,
> .htotal = 1080 + 408 + 4 + 38,
> }
>
> It reproduces the desired delay value 65487 but the correct working
> value should be 7.
>
> So, Fix it by computing hsync_porch value separately with u32 type.
>
> Fixes: 1c1a7aa3663c ("drm/sun4i: dsi: Add burst support")
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> ---
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 6ff585055a07..465e7fc57899 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -457,8 +457,9 @@ static void sun6i_dsi_setup_inst_loop(struct sun6i_dsi *dsi,
> u16 delay = 50 - 1;
>
> if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> - delay = (mode->htotal - mode->hdisplay) * 150;
> - delay /= (mode->clock / 1000) * 8;
> + u32 hsync_porch = (mode->htotal - mode->hdisplay);
> +
> + delay = ((hsync_porch * 150) / ((mode->clock / 1000) * 8));
shouldn't we keep the multiplication by 150 in the u32 assignation?
Otherwise, we could keep a u16 for the delay
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v9 1/5] drm/sun4i: sun6i_mipi_dsi: Fix hsync_porch overflow
2019-03-04 15:54 ` Maxime Ripard
@ 2019-03-06 19:02 ` Jagan Teki
0 siblings, 0 replies; 24+ messages in thread
From: Jagan Teki @ 2019-03-06 19:02 UTC (permalink / raw)
To: Maxime Ripard
Cc: David Airlie, Daniel Vetter, Chen-Yu Tsai, dri-devel,
linux-arm-kernel, linux-kernel, linux-amarula, Michael Trimarchi,
linux-sunxi
On Mon, Mar 4, 2019 at 9:24 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Sun, Mar 03, 2019 at 11:05:23PM +0530, Jagan Teki wrote:
> > Loop N1 instruction delay for burst mode devices are computed
> > based on horizontal sync and porch timing values.
> >
> > The current driver is using u16 type for computing this hsync_porch
> > value, which would failed to fit within the u16 type for large sync
> > and porch timings devices. This would result in hsync_porch overflow
> > and eventually computed wrong instruction delay value.
> >
> > Example, timings, where it produces the overflow
> > {
> > .hdisplay = 1080,
> > .hsync_start = 1080 + 408,
> > .hsync_end = 1080 + 408 + 4,
> > .htotal = 1080 + 408 + 4 + 38,
> > }
> >
> > It reproduces the desired delay value 65487 but the correct working
> > value should be 7.
> >
> > So, Fix it by computing hsync_porch value separately with u32 type.
> >
> > Fixes: 1c1a7aa3663c ("drm/sun4i: dsi: Add burst support")
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > ---
> > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index 6ff585055a07..465e7fc57899 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -457,8 +457,9 @@ static void sun6i_dsi_setup_inst_loop(struct sun6i_dsi *dsi,
> > u16 delay = 50 - 1;
> >
> > if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> > - delay = (mode->htotal - mode->hdisplay) * 150;
> > - delay /= (mode->clock / 1000) * 8;
> > + u32 hsync_porch = (mode->htotal - mode->hdisplay);
> > +
> > + delay = ((hsync_porch * 150) / ((mode->clock / 1000) * 8));
>
> shouldn't we keep the multiplication by 150 in the u32 assignation?
Yes, ie true. will move it.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v9 2/5] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits
2019-03-03 17:35 [PATCH v9 0/5] drm/sun4i: sun6i_mipi_dsi: Fixes/updates Jagan Teki
2019-03-03 17:35 ` [PATCH v9 1/5] drm/sun4i: sun6i_mipi_dsi: Fix hsync_porch overflow Jagan Teki
@ 2019-03-03 17:35 ` Jagan Teki
2019-03-04 15:43 ` Maxime Ripard
2019-03-03 17:35 ` [PATCH v9 3/5] drm/sun4i: sun6i_mipi_dsi: Support vblk timing for 4-lane devices Jagan Teki
` (3 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Jagan Teki @ 2019-03-03 17:35 UTC (permalink / raw)
To: Maxime Ripard, David Airlie, Daniel Vetter, Chen-Yu Tsai
Cc: dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
Michael Trimarchi, linux-sunxi, Jagan Teki
TCON DRQ for non-burst DSI mode can computed based on horizontal
front porch value, but the current driver trying to include sync
timings along with front porch resulting wrong drq.
This patch is trying to update the drq by subtracting hsync_start
with hdisplay, which is horizontal front porch.
Current code:
------------
mode->hsync_end - mode->hdisplay => horizontal front porch + sync
With this patch:
----------------
mode->hsync_start - mode->hdisplay => horizontal front porch
BSP code form BPI-M64-bsp is computing TCON DRQ set bits
for non-burts as (from linux-sunxi/
drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
=> panel->lcd_ht - panel->lcd_x - panel->lcd_hbp
=> (timmings->hor_front_porch + panel->lcd_hbp + panel->lcd_x)
- panel->lcd_x - panel->hbp
=> timmings->hor_front_porch
=> mode->hsync_start - mode->hdisplay
So, update the DRQ set bits accordingly.
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Merlijn Wajer <merlijn@wizzup.org>
---
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 465e7fc57899..140e55f5ed2e 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -436,9 +436,9 @@ static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
SUN6I_DSI_BURST_LINE_SYNC_POINT(SUN6I_DSI_SYNC_POINT));
val = SUN6I_DSI_TCON_DRQ_ENABLE_MODE;
- } else if ((mode->hsync_end - mode->hdisplay) > 20) {
+ } else if ((mode->hsync_start - mode->hdisplay) > 20) {
/* Maaaaaagic */
- u16 drq = (mode->hsync_end - mode->hdisplay) - 20;
+ u16 drq = (mode->hsync_start - mode->hdisplay) - 20;
drq *= mipi_dsi_pixel_format_to_bpp(device->format);
drq /= 32;
--
2.18.0.321.gffc6fa0e3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v9 2/5] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits
2019-03-03 17:35 ` [PATCH v9 2/5] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits Jagan Teki
@ 2019-03-04 15:43 ` Maxime Ripard
[not found] ` <CAMty3ZDWkLWgWhGWBjhXOsmAXzuGKGADAEhzB6gcL+jd7FRazQ@mail.gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Maxime Ripard @ 2019-03-04 15:43 UTC (permalink / raw)
To: Jagan Teki
Cc: David Airlie, Daniel Vetter, Chen-Yu Tsai, dri-devel,
linux-arm-kernel, linux-kernel, linux-amarula, Michael Trimarchi,
linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]
On Sun, Mar 03, 2019 at 11:05:24PM +0530, Jagan Teki wrote:
> TCON DRQ for non-burst DSI mode can computed based on horizontal
> front porch value, but the current driver trying to include sync
> timings along with front porch resulting wrong drq.
>
> This patch is trying to update the drq by subtracting hsync_start
> with hdisplay, which is horizontal front porch.
>
> Current code:
> ------------
> mode->hsync_end - mode->hdisplay => horizontal front porch + sync
>
> With this patch:
> ----------------
> mode->hsync_start - mode->hdisplay => horizontal front porch
>
> BSP code form BPI-M64-bsp is computing TCON DRQ set bits
> for non-burts as (from linux-sunxi/
> drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
>
> => panel->lcd_ht - panel->lcd_x - panel->lcd_hbp
> => (timmings->hor_front_porch + panel->lcd_hbp + panel->lcd_x)
^ + sync length +
> - panel->lcd_x - panel->hbp
> => timmings->hor_front_porch
^ + sync
> => mode->hsync_start - mode->hdisplay
s/hsync_start/hsync_end/
Did you encounter any panel where this was fixing something? If so,
which one, and what is the matching timings and / or datasheet?
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v9 3/5] drm/sun4i: sun6i_mipi_dsi: Support vblk timing for 4-lane devices
2019-03-03 17:35 [PATCH v9 0/5] drm/sun4i: sun6i_mipi_dsi: Fixes/updates Jagan Teki
2019-03-03 17:35 ` [PATCH v9 1/5] drm/sun4i: sun6i_mipi_dsi: Fix hsync_porch overflow Jagan Teki
2019-03-03 17:35 ` [PATCH v9 2/5] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits Jagan Teki
@ 2019-03-03 17:35 ` Jagan Teki
2019-03-04 15:49 ` Maxime Ripard
2019-03-03 17:35 ` [PATCH v9 4/5] drm/sun4i: sun6i_mipi_dsi: Support DSI GENERIC_SHORT_WRITE_2 transfer Jagan Teki
` (2 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Jagan Teki @ 2019-03-03 17:35 UTC (permalink / raw)
To: Maxime Ripard, David Airlie, Daniel Vetter, Chen-Yu Tsai
Cc: dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
Michael Trimarchi, linux-sunxi, Jagan Teki
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(-)
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? */
--
2.18.0.321.gffc6fa0e3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v9 3/5] drm/sun4i: sun6i_mipi_dsi: Support vblk timing for 4-lane devices
2019-03-03 17:35 ` [PATCH v9 3/5] drm/sun4i: sun6i_mipi_dsi: Support vblk timing for 4-lane devices Jagan Teki
@ 2019-03-04 15:49 ` Maxime Ripard
2019-03-07 16:03 ` Jagan Teki
0 siblings, 1 reply; 24+ messages in thread
From: Maxime Ripard @ 2019-03-04 15:49 UTC (permalink / raw)
To: Jagan Teki
Cc: David Airlie, Daniel Vetter, Chen-Yu Tsai, dri-devel,
linux-arm-kernel, linux-kernel, linux-amarula, Michael Trimarchi,
linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 3034 bytes --]
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
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v9 3/5] drm/sun4i: sun6i_mipi_dsi: Support vblk timing for 4-lane devices
2019-03-04 15:49 ` Maxime Ripard
@ 2019-03-07 16:03 ` Jagan Teki
2019-03-11 14:04 ` Maxime Ripard
0 siblings, 1 reply; 24+ messages in thread
From: Jagan Teki @ 2019-03-07 16:03 UTC (permalink / raw)
To: Maxime Ripard
Cc: David Airlie, Daniel Vetter, Chen-Yu Tsai, dri-devel,
linux-arm-kernel, linux-kernel, linux-amarula, Michael Trimarchi,
linux-sunxi
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.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v9 3/5] drm/sun4i: sun6i_mipi_dsi: Support vblk timing for 4-lane devices
2019-03-07 16:03 ` Jagan Teki
@ 2019-03-11 14:04 ` Maxime Ripard
2019-03-11 14:33 ` Jagan Teki
0 siblings, 1 reply; 24+ messages in thread
From: Maxime Ripard @ 2019-03-11 14:04 UTC (permalink / raw)
To: Jagan Teki
Cc: David Airlie, Daniel Vetter, Chen-Yu Tsai, dri-devel,
linux-arm-kernel, linux-kernel, linux-amarula, Michael Trimarchi,
linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]
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
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v9 3/5] drm/sun4i: sun6i_mipi_dsi: Support vblk timing for 4-lane devices
2019-03-11 14:04 ` Maxime Ripard
@ 2019-03-11 14:33 ` Jagan Teki
0 siblings, 0 replies; 24+ messages in thread
From: Jagan Teki @ 2019-03-11 14:33 UTC (permalink / raw)
To: Maxime Ripard
Cc: David Airlie, Daniel Vetter, Chen-Yu Tsai, dri-devel,
linux-arm-kernel, linux-kernel, linux-amarula, Michael Trimarchi,
linux-sunxi
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/
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v9 4/5] drm/sun4i: sun6i_mipi_dsi: Support DSI GENERIC_SHORT_WRITE_2 transfer
2019-03-03 17:35 [PATCH v9 0/5] drm/sun4i: sun6i_mipi_dsi: Fixes/updates Jagan Teki
` (2 preceding siblings ...)
2019-03-03 17:35 ` [PATCH v9 3/5] drm/sun4i: sun6i_mipi_dsi: Support vblk timing for 4-lane devices Jagan Teki
@ 2019-03-03 17:35 ` Jagan Teki
2019-03-03 17:35 ` [PATCH v9 5/5] drm/sun4i: sun6i_mipi_dsi: Simplify dsi setup timings code Jagan Teki
2019-03-18 18:28 ` [PATCH v9 0/5] drm/sun4i: sun6i_mipi_dsi: Fixes/updates Jagan Teki
5 siblings, 0 replies; 24+ messages in thread
From: Jagan Teki @ 2019-03-03 17:35 UTC (permalink / raw)
To: Maxime Ripard, David Airlie, Daniel Vetter, Chen-Yu Tsai
Cc: dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
Michael Trimarchi, linux-sunxi, Jagan Teki
Some DSI panels do use GENERIC_SHORT_WRITE_2 transfer protocol to host
DSI driver and which is similar to GENERIC_SHORT_WRITE.
Add support for the same transfer, so-that so-that the panels which are
requesting similar transfer type will process properly.
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Merlijn Wajer <merlijn@wizzup.org>
---
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index b38358465d87..31ec4048804d 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -993,6 +993,7 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
switch (msg->type) {
case MIPI_DSI_DCS_SHORT_WRITE:
case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
+ case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM:
ret = sun6i_dsi_dcs_write_short(dsi, msg);
break;
--
2.18.0.321.gffc6fa0e3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v9 5/5] drm/sun4i: sun6i_mipi_dsi: Simplify dsi setup timings code
2019-03-03 17:35 [PATCH v9 0/5] drm/sun4i: sun6i_mipi_dsi: Fixes/updates Jagan Teki
` (3 preceding siblings ...)
2019-03-03 17:35 ` [PATCH v9 4/5] drm/sun4i: sun6i_mipi_dsi: Support DSI GENERIC_SHORT_WRITE_2 transfer Jagan Teki
@ 2019-03-03 17:35 ` Jagan Teki
2019-03-04 15:50 ` Maxime Ripard
2019-03-18 18:28 ` [PATCH v9 0/5] drm/sun4i: sun6i_mipi_dsi: Fixes/updates Jagan Teki
5 siblings, 1 reply; 24+ messages in thread
From: Jagan Teki @ 2019-03-03 17:35 UTC (permalink / raw)
To: Maxime Ripard, David Airlie, Daniel Vetter, Chen-Yu Tsai
Cc: dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
Michael Trimarchi, linux-sunxi, Jagan Teki
DSI timings are varies between burst/non-burst devices and
current driver is handling this support via if, else statements
which would difficult to read.
Simplify it by using goto to make code more readable.
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Merlijn Wajer <merlijn@wizzup.org>
---
Note: This change is created based on previous version burst
changes [1] by addressing comment from [2] by Maxime to make
code readable.
[1] https://patchwork.kernel.org/cover/10813623/
[2] https://patchwork.kernel.org/patch/10666607/
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 79 ++++++++++++++------------
1 file changed, 42 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 31ec4048804d..898f32319c2d 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -550,7 +550,8 @@ 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 = 0, hfp = 0, hsa = 0, hblk = 0, vblk = 0;
+ u16 hbp, hfp, hsa, hblk;
+ u16 vblk = 0;
u32 basic_ctl = 0;
size_t bytes;
u8 *buffer;
@@ -558,6 +559,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
/* Do all timing calculations up front to allocate buffer space */
if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
+ hbp = hfp = hsa = 0;
hblk = mode->hdisplay * Bpp;
basic_ctl = SUN6I_DSI_BASIC_CTL_VIDEO_BURST |
SUN6I_DSI_BASIC_CTL_HSA_HSE_DIS |
@@ -566,48 +568,51 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
if (device->lanes == 4)
basic_ctl |= SUN6I_DSI_BASIC_CTL_TRAIL_FILL |
SUN6I_DSI_BASIC_CTL_TRAIL_INV(0xc);
- } else {
- /*
- * A sync period is composed of a blanking packet (4
- * bytes + payload + 2 bytes) and a sync event packet
- * (4 bytes). Its minimal size is therefore 10 bytes
- */
+
+ goto alloc_buf;
+ }
+
+ /*
+ * A sync period is composed of a blanking packet (4
+ * bytes + payload + 2 bytes) and a sync event packet
+ * (4 bytes). Its minimal size is therefore 10 bytes
+ */
#define HSA_PACKET_OVERHEAD 10
- hsa = max((unsigned int)HSA_PACKET_OVERHEAD,
- (mode->hsync_end - mode->hsync_start) * Bpp - HSA_PACKET_OVERHEAD);
-
- /*
- * The backporch is set using a blanking packet (4
- * bytes + payload + 2 bytes). Its minimal size is
- * therefore 6 bytes
- */
+ hsa = max((unsigned int)HSA_PACKET_OVERHEAD,
+ (mode->hsync_end - mode->hsync_start) * Bpp - HSA_PACKET_OVERHEAD);
+
+ /*
+ * The backporch is set using a blanking packet (4
+ * bytes + payload + 2 bytes). Its minimal size is
+ * therefore 6 bytes
+ */
#define HBP_PACKET_OVERHEAD 6
- hbp = max((unsigned int)HBP_PACKET_OVERHEAD,
- (mode->htotal - mode->hsync_end) * Bpp - HBP_PACKET_OVERHEAD);
-
- /*
- * The frontporch is set using a blanking packet (4
- * bytes + payload + 2 bytes). Its minimal size is
- * therefore 6 bytes
- */
+ hbp = max((unsigned int)HBP_PACKET_OVERHEAD,
+ (mode->htotal - mode->hsync_end) * Bpp - HBP_PACKET_OVERHEAD);
+
+ /*
+ * The frontporch is set using a blanking packet (4
+ * bytes + payload + 2 bytes). Its minimal size is
+ * therefore 6 bytes
+ */
#define HFP_PACKET_OVERHEAD 6
- hfp = max((unsigned int)HFP_PACKET_OVERHEAD,
- (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD);
-
- /*
- * The blanking is set using a sync event (4 bytes)
- * and a blanking packet (4 bytes + payload + 2
- * bytes). Its minimal size is therefore 10 bytes.
- */
+ hfp = max((unsigned int)HFP_PACKET_OVERHEAD,
+ (mode->hsync_start - mode->hdisplay) * Bpp - HFP_PACKET_OVERHEAD);
+
+ /*
+ * The blanking is set using a sync event (4 bytes)
+ * and a blanking packet (4 bytes + payload + 2
+ * bytes). Its minimal size is therefore 10 bytes.
+ */
#define HBLK_PACKET_OVERHEAD 10
- hblk = max((unsigned int)HBLK_PACKET_OVERHEAD,
- (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp -
- HBLK_PACKET_OVERHEAD);
+ hblk = max((unsigned int)HBLK_PACKET_OVERHEAD,
+ (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp -
+ HBLK_PACKET_OVERHEAD);
- if (device->lanes == 4)
- vblk = sun6i_dsi_get_timings_vblk(dsi, mode, hblk);
- }
+ if (device->lanes == 4)
+ vblk = sun6i_dsi_get_timings_vblk(dsi, mode, hblk);
+alloc_buf:
/* How many bytes do we need to send all payloads? */
bytes = max_t(size_t, max(max(hfp, hblk), max(hsa, hbp)), vblk);
buffer = kmalloc(bytes, GFP_KERNEL);
--
2.18.0.321.gffc6fa0e3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v9 5/5] drm/sun4i: sun6i_mipi_dsi: Simplify dsi setup timings code
2019-03-03 17:35 ` [PATCH v9 5/5] drm/sun4i: sun6i_mipi_dsi: Simplify dsi setup timings code Jagan Teki
@ 2019-03-04 15:50 ` Maxime Ripard
2019-03-07 11:46 ` Jagan Teki
0 siblings, 1 reply; 24+ messages in thread
From: Maxime Ripard @ 2019-03-04 15:50 UTC (permalink / raw)
To: Jagan Teki
Cc: David Airlie, Daniel Vetter, Chen-Yu Tsai, dri-devel,
linux-arm-kernel, linux-kernel, linux-amarula, Michael Trimarchi,
linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 2485 bytes --]
On Sun, Mar 03, 2019 at 11:05:27PM +0530, Jagan Teki wrote:
> DSI timings are varies between burst/non-burst devices and
> current driver is handling this support via if, else statements
> which would difficult to read.
>
> Simplify it by using goto to make code more readable.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> ---
> Note: This change is created based on previous version burst
> changes [1] by addressing comment from [2] by Maxime to make
> code readable.
>
> [1] https://patchwork.kernel.org/cover/10813623/
> [2] https://patchwork.kernel.org/patch/10666607/
>
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 79 ++++++++++++++------------
> 1 file changed, 42 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 31ec4048804d..898f32319c2d 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -550,7 +550,8 @@ 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 = 0, hfp = 0, hsa = 0, hblk = 0, vblk = 0;
> + u16 hbp, hfp, hsa, hblk;
> + u16 vblk = 0;
> u32 basic_ctl = 0;
> size_t bytes;
> u8 *buffer;
> @@ -558,6 +559,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> /* Do all timing calculations up front to allocate buffer space */
>
> if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> + hbp = hfp = hsa = 0;
This raises a checkpatch warning and isn't necessary
> hblk = mode->hdisplay * Bpp;
> basic_ctl = SUN6I_DSI_BASIC_CTL_VIDEO_BURST |
> SUN6I_DSI_BASIC_CTL_HSA_HSE_DIS |
> @@ -566,48 +568,51 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> if (device->lanes == 4)
> basic_ctl |= SUN6I_DSI_BASIC_CTL_TRAIL_FILL |
> SUN6I_DSI_BASIC_CTL_TRAIL_INV(0xc);
> - } else {
> - /*
> - * A sync period is composed of a blanking packet (4
> - * bytes + payload + 2 bytes) and a sync event packet
> - * (4 bytes). Its minimal size is therefore 10 bytes
> - */
> +
> + goto alloc_buf;
And I'd rather not have a goto in the middle of the code for no
particular reason.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v9 5/5] drm/sun4i: sun6i_mipi_dsi: Simplify dsi setup timings code
2019-03-04 15:50 ` Maxime Ripard
@ 2019-03-07 11:46 ` Jagan Teki
0 siblings, 0 replies; 24+ messages in thread
From: Jagan Teki @ 2019-03-07 11:46 UTC (permalink / raw)
To: Maxime Ripard
Cc: David Airlie, Daniel Vetter, Chen-Yu Tsai, dri-devel,
linux-arm-kernel, linux-kernel, linux-amarula, Michael Trimarchi,
linux-sunxi
On Mon, Mar 4, 2019 at 9:20 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Sun, Mar 03, 2019 at 11:05:27PM +0530, Jagan Teki wrote:
> > DSI timings are varies between burst/non-burst devices and
> > current driver is handling this support via if, else statements
> > which would difficult to read.
> >
> > Simplify it by using goto to make code more readable.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > ---
> > Note: This change is created based on previous version burst
> > changes [1] by addressing comment from [2] by Maxime to make
> > code readable.
> >
> > [1] https://patchwork.kernel.org/cover/10813623/
> > [2] https://patchwork.kernel.org/patch/10666607/
> >
> > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 79 ++++++++++++++------------
> > 1 file changed, 42 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index 31ec4048804d..898f32319c2d 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -550,7 +550,8 @@ 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 = 0, hfp = 0, hsa = 0, hblk = 0, vblk = 0;
> > + u16 hbp, hfp, hsa, hblk;
> > + u16 vblk = 0;
> > u32 basic_ctl = 0;
> > size_t bytes;
> > u8 *buffer;
> > @@ -558,6 +559,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> > /* Do all timing calculations up front to allocate buffer space */
> >
> > if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> > + hbp = hfp = hsa = 0;
>
> This raises a checkpatch warning and isn't necessary
>
> > hblk = mode->hdisplay * Bpp;
> > basic_ctl = SUN6I_DSI_BASIC_CTL_VIDEO_BURST |
> > SUN6I_DSI_BASIC_CTL_HSA_HSE_DIS |
> > @@ -566,48 +568,51 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> > if (device->lanes == 4)
> > basic_ctl |= SUN6I_DSI_BASIC_CTL_TRAIL_FILL |
> > SUN6I_DSI_BASIC_CTL_TRAIL_INV(0xc);
> > - } else {
> > - /*
> > - * A sync period is composed of a blanking packet (4
> > - * bytes + payload + 2 bytes) and a sync event packet
> > - * (4 bytes). Its minimal size is therefore 10 bytes
> > - */
> > +
> > + goto alloc_buf;
>
> And I'd rather not have a goto in the middle of the code for no
> particular reason.
OK, will try to think for better simplification otherwise will drop it
in next version.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v9 0/5] drm/sun4i: sun6i_mipi_dsi: Fixes/updates
2019-03-03 17:35 [PATCH v9 0/5] drm/sun4i: sun6i_mipi_dsi: Fixes/updates Jagan Teki
` (4 preceding siblings ...)
2019-03-03 17:35 ` [PATCH v9 5/5] drm/sun4i: sun6i_mipi_dsi: Simplify dsi setup timings code Jagan Teki
@ 2019-03-18 18:28 ` Jagan Teki
2019-03-19 10:58 ` Maxime Ripard
5 siblings, 1 reply; 24+ messages in thread
From: Jagan Teki @ 2019-03-18 18:28 UTC (permalink / raw)
To: Maxime Ripard, David Airlie, Daniel Vetter, Chen-Yu Tsai
Cc: dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
Michael Trimarchi, linux-sunxi
Hi,
On Sun, Mar 3, 2019 at 11:06 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Unfortunately due to various reasons[3] the previous fixes[1] and burst[2]
> changes are failed to apply.
>
> So, this series is filtered version of previous [1] and [2] changes on top
> of drm-misc.
>
> patch-1: Fix for burst mode instruction delay computation
>
> patch-2: Fix for TCOn DRQ set bits
>
> patch-3: Support vblk timing for 4-lane devices
>
> patch-4: GENERIC_SHORT_WRITE_2 dsi transfer support
>
> patch-5: Code simplification for dsi timings
>
> Changes for v9:
> - rebase on drm-misc
> - update commit messages
> - add hsync_porch overflow patch
> Changes for v8:
> - rebase on master
> - rework on commit messages
> - rework video start delay
> - include drq changes from previous version
> Changes for v7:
> - rebase on master
> - collect Merlijn Wajer Tested-by credits.
> Changes for v6:
> - fixed all burst mode patches as per previous version comments
> - rebase on master
> - update proper commit message
> - dropped unneeded comments
> - order the patches that make review easy
> Changes for v5, v4, v3, v2:
> - use existing driver code construct for hblk computation
> - create separate function for vblk computation
> - cleanup commit messages
> - update proper commit messages
> - fixed checkpatch warnings/errors
> - use proper return value for tcon0 probe
> - add logic to get tcon0 divider values
> - simplify timings code to support burst mode
> - fix drq computation return values
> - rebase on master
>
> [1] https://patchwork.kernel.org/cover/10813573/
> [2] https://patchwork.kernel.org/cover/10813623/
> [3] https://patchwork.kernel.org/cover/10805995/
>
> Any inputs?
> Jagan.
Any further comments on this series? Can I send next version with some changes?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v9 0/5] drm/sun4i: sun6i_mipi_dsi: Fixes/updates
2019-03-18 18:28 ` [PATCH v9 0/5] drm/sun4i: sun6i_mipi_dsi: Fixes/updates Jagan Teki
@ 2019-03-19 10:58 ` Maxime Ripard
0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2019-03-19 10:58 UTC (permalink / raw)
To: Jagan Teki
Cc: David Airlie, Daniel Vetter, Chen-Yu Tsai, dri-devel,
linux-arm-kernel, linux-kernel, linux-amarula, Michael Trimarchi,
linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 2321 bytes --]
On Mon, Mar 18, 2019 at 11:58:39PM +0530, Jagan Teki wrote:
> Hi,
>
> On Sun, Mar 3, 2019 at 11:06 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Unfortunately due to various reasons[3] the previous fixes[1] and burst[2]
> > changes are failed to apply.
> >
> > So, this series is filtered version of previous [1] and [2] changes on top
> > of drm-misc.
> >
> > patch-1: Fix for burst mode instruction delay computation
> >
> > patch-2: Fix for TCOn DRQ set bits
> >
> > patch-3: Support vblk timing for 4-lane devices
> >
> > patch-4: GENERIC_SHORT_WRITE_2 dsi transfer support
> >
> > patch-5: Code simplification for dsi timings
> >
> > Changes for v9:
> > - rebase on drm-misc
> > - update commit messages
> > - add hsync_porch overflow patch
> > Changes for v8:
> > - rebase on master
> > - rework on commit messages
> > - rework video start delay
> > - include drq changes from previous version
> > Changes for v7:
> > - rebase on master
> > - collect Merlijn Wajer Tested-by credits.
> > Changes for v6:
> > - fixed all burst mode patches as per previous version comments
> > - rebase on master
> > - update proper commit message
> > - dropped unneeded comments
> > - order the patches that make review easy
> > Changes for v5, v4, v3, v2:
> > - use existing driver code construct for hblk computation
> > - create separate function for vblk computation
> > - cleanup commit messages
> > - update proper commit messages
> > - fixed checkpatch warnings/errors
> > - use proper return value for tcon0 probe
> > - add logic to get tcon0 divider values
> > - simplify timings code to support burst mode
> > - fix drq computation return values
> > - rebase on master
> >
> > [1] https://patchwork.kernel.org/cover/10813573/
> > [2] https://patchwork.kernel.org/cover/10813623/
> > [3] https://patchwork.kernel.org/cover/10805995/
> >
> > Any inputs?
> > Jagan.
>
> Any further comments on this series? Can I send next version with some changes?
Unless you address what we've asked for the very beginning and provide
some accurate description of your problem, and references to why you
think your changes are the right thing to do, it's not worth sending a
new version, the discussion cannot make further progress.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread