These fixes are encounter in Allwinner MIPI-DSI driver while adding new SoC support and ofcourse all these are generic with respect to controller.
All these changes are grabbed from previous versions[1] and grouped it in a proper order so-that it can help to review and merge easy.
Changes for v8: - rebase on master - rework on commit messages - rework video start delay 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 - rebase on master - fixed checkpatch warnings/errors
[1] https://patchwork.kernel.org/cover/10680247/ [2] https://patchwork.kernel.org/cover/10680247/
Any inputs? Jagan.
Jagan Teki (8): drm/sun4i: sun6i_mipi_dsi: Fix VBP size calculation drm/sun4i: sun6i_mipi_dsi: Fix DSI hbp timing value drm/sun4i: sun6i_mipi_dsi: Fix DSI hfp timing value drm/sun4i: sun6i_mipi_dsi: Fix DSI hblk timing calculation drm/sun4i: sun6i_mipi_dsi: Add DSI hblk packet overhead drm/sun4i: sun6i_mipi_dsi: Set proper vblk timing calculation drm/sun4i: sun6i_mipi_dsi: Refactor vertical video start delay drm/sun4i: sun6i_mipi_dsi: Add DSI Generic short write 2 param transfer
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 65 ++++++++++++++++++++------ 1 file changed, 52 insertions(+), 13 deletions(-)
The horizontal and vertical back porch calculation in BSP code is simply following the Linux drm comment diagram, in include/drm/drm_modes.h which is
[hv]back porch = [hv]total - [hv]sync_end
BSP code form BPI-M64-bsp is calculating vertical back porch as (from linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c)
timmings->ver_sync_time= panel_info->lcd_vspw; timmings->ver_back_porch= panel_info->lcd_vbp-panel_info->lcd_vspw;
vbp = panel->lcd_vbp; vspw = panel->lcd_vspw; dsi_dev[sel]->dsi_basic_size0.bits.vbp = vbp-vspw; dsi_dev[sel]->dsi_basic_size0.bits.vbp = panel->lcd_vbp - panel->lcd_vspw; => timmings->ver_back_porch + panel_info->lcd_vspw - panel_info->lcd_vspw => timmings->ver_back_porch => mode->vtotal - mode->end
Which evatually same as mode->vtotal - mode->vsync_end so fix the VBP value in SUN6I_DSI_BASIC_SIZE0_VBP
On the information note, existing SUN6I_DSI_BASIC_SIZE0_VSA is proper value.
Signed-off-by: Jagan Teki jagan@amarulasolutions.com --- 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 318994cd1b85..d70a8c43a4b8 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -526,8 +526,8 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, regmap_write(dsi->regs, SUN6I_DSI_BASIC_SIZE0_REG, SUN6I_DSI_BASIC_SIZE0_VSA(mode->vsync_end - mode->vsync_start) | - SUN6I_DSI_BASIC_SIZE0_VBP(mode->vsync_start - - mode->vdisplay)); + SUN6I_DSI_BASIC_SIZE0_VBP(mode->vtotal - + mode->vsync_end));
regmap_write(dsi->regs, SUN6I_DSI_BASIC_SIZE1_REG, SUN6I_DSI_BASIC_SIZE1_VACT(mode->vdisplay) |
Current driver is calculating hbp maximum value by subtracting hsync_start with hdisplay which is front porch value, but the hbp refers to back porch.
Back porch value is calculating by subtracting htotal with hsync_end as per drm_mode timings, and BSP code from BPI-M64-bsp is eventually following the same.
BPI-M64-bsp is computing hbp as (in drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c) dsi_hbp = (hbp-hspw)*dsi_pixel_bits[format]/8 - (4+2); => (panel->lcd_hbp - timmings->hor_sync_time) => (timmings->hor_back_porch + timmings->hor_sync_time - timmings->hor_sync_time) => timmings->hor_back_porch => mode->htotal - mode->hsync_end
So, update the hbp value accordingly in sun6i_dsi_setup_timings.
Tested on 2-lane, 4-lane 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 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index d70a8c43a4b8..982ae6b17654 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -472,7 +472,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, */ #define HBP_PACKET_OVERHEAD 6 hbp = max((unsigned int)HBP_PACKET_OVERHEAD, - (mode->hsync_start - mode->hdisplay) * Bpp - HBP_PACKET_OVERHEAD); + (mode->htotal - mode->hsync_end) * Bpp - HBP_PACKET_OVERHEAD);
/* * The frontporch is set using a blanking packet (4 bytes +
Current driver is calculating hfp maximum value by subtracting htotal with hsync_end which is back porch value, but the hfp refers to front porch.
Front porch value is calculating by subtracting hsync_start with hdisplay as per drm_mode timings, and BSP code from BPI-M64-bsp is eventually following the same.
BPI-M64-bsp is computing hfp as (from linux-sunxi/ drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
dsi_hbp = (hbp-hspw)*dsi_pixel_bits[format]/8 - (4+2); dsi_hact = x * dsi_pixel_bits[format]/8; dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2); dsi_hfp = dsi_hblk - (4+dsi_hact+2) - (4+dsi_hbp+2);
Example, u32 fmt = dsi_pixel_bits[format]/8; => ((ht-hspw)*fmt - 10) - (6 + x * fmt) - (6 + (hbp-hspw)*fmt - 6) => (ht - hspw - x - (hbp - hspw)) * fmt - 16 => (ht - x - hbp) * fmt - 16 => (ht - x - (timmings->hor_total_time - timmings->hor_front_porch - x) * fmt - 16 => (timmings->hor_total_time - x - timmings->hor_total_time + timmings->hor_front_porch + x) * fmt - 16 => timmings->hor_front_porch * fmt - 16
So, update the hfp value accordingly in sun6i_dsi_setup_timings.
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 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 982ae6b17654..21f39f11a8de 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -480,7 +480,8 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, */ #define HFP_PACKET_OVERHEAD 6 hfp = max((unsigned int)HFP_PACKET_OVERHEAD, - (mode->htotal - mode->hsync_end) * Bpp - HFP_PACKET_OVERHEAD); + (mode->hsync_start - mode->hdisplay) * Bpp - + HFP_PACKET_OVERHEAD);
/* * hblk seems to be the line + porches length.
horizontal blank is computed by adding all porch timing values, or htotal total timing value without sync time.
Based on "DRM kernel-internal display mode structure" from include/drm/drm_modes.h
hblk = htotal - (hsync value); hblk = htotal - (hsync_end - hsync_start);
Current driver is subtracting htotal with hsa, but the hsa is bounded with packet overhead. So subtract htotal with hsync value(hsync_end - hsync_start) proper hblk computation.
BPI-M64-bsp is computing hbp as (from linux-sunxi/ drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
dsi_hblk = (ht-hspw)*dsi_pixel_bits[format]/8-(4+4+2); => (timmings->hor_total_time - timmings->hor_sync_time) => (mode->htotal - (mode->hsync_end - mode->hsync_start))
So, update the hblk value accordingly in sun6i_dsi_setup_timings.
Tested on 2-lane, 4-lane MIPI-DSI LCD panels.
Signed-off-by: Jagan Teki jagan@amarulasolutions.com --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 21f39f11a8de..6b26dba883e8 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -486,7 +486,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi, /* * hblk seems to be the line + porches length. */ - hblk = mode->htotal * Bpp - hsa; + hblk = (mode->htotal - (mode->hsync_end - mode->hsync_start)) * Bpp;
/* * And I'm not entirely sure what vblk is about. The driver in
Like other dsi setup timings, hblk would also require to add packet overhead.
Add 10 bytes packet overhead for hblk, so the blank is set using a blanking packet like (4 bytes + 4 bytes + payload + 2 bytes)
The value 10 bytes are refereed from Allwinner BSP like how other dsi setup timings grabs in existing driver.
This is according to BSP code from BPI-M64-bsp (from linux-sunxi/ 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 --- 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 6b26dba883e8..a64e6ec00f22 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -485,8 +485,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((unsigned int)HBLK_PACKET_OVERHEAD, + (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
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 accordingly.
Tested on 2-lane, 4-lane MIPI-DSI LCD panels.
Signed-off-by: Jagan Teki jagan@amarulasolutions.com --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 29 +++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index a64e6ec00f22..a08dfdcbe9e8 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -446,6 +446,27 @@ 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; + + if (device->lanes != 4) + return 0; + + /* + * 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) { @@ -493,13 +514,7 @@ 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; + vblk = sun6i_dsi_get_timings_vblk(dsi, mode, hblk);
/* How many bytes do we need to send all payloads? */ bytes = max_t(size_t, max(max(hfp, hblk), max(hsa, hbp)), vblk);
Vertical video start delay is computed by excluding vertical porch value from total vertical timings, but the current driver excluding vertical porch along with vertical sync values from total vertical timings resulting wrong start delay.
This patch trying to update the video start delay by subtracting vertical porch from vertical total, on the other hand it added 1 extra start_delay line for TCON based on the Allwinner BSP reference.
BSP code form BPI-M64-bsp is computing video start delay as (from linux-sunxi/ drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
u32 vfp = panel->lcd_vt - panel->lcd_y - panel->lcd_vbp; => (panel->lcd_vt) - panel->lcd_y - (panel->lcd_vbp) => (timmings->ver_front_porch + panel->lcd_vbp + panel->lcd_y) - panel->lcd_y - (panel->lcd_vbp) => timmings->ver_front_porch + panel->lcd_vbp + panel->lcd_y - panel->lcd_y - panel->lcd_vbp => timmings->ver_front_porch
So, update the start delay computation accordingly.
Signed-off-by: Jagan Teki jagan@amarulasolutions.com --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index a08dfdcbe9e8..31cf9c58e98d 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -358,7 +358,24 @@ 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; + u32 vfp, start_delay; + + /* vertical front porch */ + vfp = mode->vsync_start - mode->vdisplay; + + /* start_delay = vertical total - vertical front porch */ + start_delay = mode->vtotal - vfp; + + /* add extra 1 delay line for TCON, as per Allwinner BSP */ + start_delay = 1; + + if (start_delay > mode->vtotal) + start_delay -= mode->vtotal; + + if (!start_delay) + start_delay = 1; + + return start_delay; }
static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
On Fri, Feb 15, 2019 at 12:56 AM Jagan Teki jagan@amarulasolutions.com wrote:
Vertical video start delay is computed by excluding vertical porch value from total vertical timings, but the current driver excluding vertical porch along with vertical sync values from total vertical timings resulting wrong start delay.
This patch trying to update the video start delay by subtracting vertical porch from vertical total, on the other hand it added 1 extra start_delay line for TCON based on the Allwinner BSP reference.
BSP code form BPI-M64-bsp is computing video start delay as (from linux-sunxi/ drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
u32 vfp = panel->lcd_vt - panel->lcd_y - panel->lcd_vbp; => (panel->lcd_vt) - panel->lcd_y - (panel->lcd_vbp) => (timmings->ver_front_porch + panel->lcd_vbp + panel->lcd_y)
- panel->lcd_y - (panel->lcd_vbp)
=> timmings->ver_front_porch + panel->lcd_vbp + panel->lcd_y - panel->lcd_y - panel->lcd_vbp => timmings->ver_front_porch
So, update the start delay computation accordingly.
Signed-off-by: Jagan Teki jagan@amarulasolutions.com
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index a08dfdcbe9e8..31cf9c58e98d 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -358,7 +358,24 @@ 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;
u32 vfp, start_delay;/* vertical front porch */vfp = mode->vsync_start - mode->vdisplay;/* start_delay = vertical total - vertical front porch */start_delay = mode->vtotal - vfp;/* add extra 1 delay line for TCON, as per Allwinner BSP */start_delay = 1;
This has to be += 1;
Typo change while re-basing, I will fix this in next version.
Short transfer write support for DCS and Generic transfer types share similar way to process command sequence in DSI block so add generic write 2 param transfer type macro so-that the panels which are requesting similar transfer type may process properly.
Signed-off-by: Jagan Teki jagan@amarulasolutions.com --- 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 31cf9c58e98d..3a28d70dbdc1 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -918,6 +918,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;
dri-devel@lists.freedesktop.org
-
Jagan Teki