mbox series

[v11,0/7] drm/sun4i: dsi: Fixes/updates (A33 reworked!)

Message ID 20191003064527.15128-1-jagan@amarulasolutions.com
Headers show
Series drm/sun4i: dsi: Fixes/updates (A33 reworked!) | expand

Message

Jagan Teki Oct. 3, 2019, 6:45 a.m. UTC 
This is v11 version changes for previous series[1].

This series dropped the tcon->dclk_min_div and tcon->dclk_max_div
patches, since the discussion on the respective threads[2][3] 
not finalized or not agree on both the ends.

This series mostly the commit messages changes suggested by Chen-Yu.

Most of these issues are reproduced while supporting Allwinner A64
MIPI-DSI [4] but to confirm it with other SoC this series is reworked
on A33 since the controller tested it A33 as of now.

Since we don't have proper evidence and documentation for dsi controller
in Allwinner, these changes are more likely to rely on new working BSP
codes (even the initial driver written as per this rule).

All these fixes/updates are tested in "bananapi,s070wv20-ct16" panel
on Allwinner A33, the same panel timings are available in panel-simple
in mainline tree.

Changes for v11:
- Drop DCLK dividers patches
- Updated the commit message changes suggested by Chen-Yu
- rebased on linux-next
Changes for v10:
- reworked previous "Fixes/updates" patches on A33
- reworked previous A64 DSI fixes on A33
- added proper detailed commit messages with logs
- tested on BPI-M2M board.
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

Any inputs?
Jagan.

[1] https://patchwork.freedesktop.org/series/60847/
[2] https://patchwork.freedesktop.org/patch/305923/?series=60847&rev=1
[3] https://patchwork.kernel.org/patch/10779937/
[4] https://patchwork.freedesktop.org/series/57834/

Jagan Teki (7):
  drm/sun4i: dsi: Fix TCON DRQ set bits
  drm/sun4i: dsi: Update start value in video start delay
  drm/sun4i: dsi: Fix video start delay computation
  dt-bindings: sun6i-dsi: Add VCC-DSI supply property
  drm/sun4i: sun6i_mipi_dsi: Add VCC-DSI regulator support
  [DO NOT MERGE] drm/panel: Add Bananapi S070WV20-CT16 ICN6211 MIPI-DSI to RGB bridge
  [DO NOT MERGE] ARM: dts: sun8i: bananapi-m2m: Enable Bananapi S070WV20-CT16 DSI panel

 .../display/allwinner,sun6i-a31-mipi-dsi.yaml |   3 +
 arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts  |  40 +++
 drivers/gpu/drm/panel/Kconfig                 |   9 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../panel/panel-bananapi-s070wv20-icn6211.c   | 293 ++++++++++++++++++
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c        |  32 +-
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h        |   2 +
 7 files changed, 376 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/panel/panel-bananapi-s070wv20-icn6211.c

Comments

Jagan Teki Oct. 3, 2019, 6:50 a.m. UTC | #1 
On Thu, Oct 3, 2019 at 12:16 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Bananapi S070WV20-CT16 ICN6211 is 800x480, 4-lane MIPI-DSI to RGB bridge
> panel which can be used to connect via DSI port on BPI-M64 board,
> so add a driver for it.
>
> The same panel PCB comes with parallel RBG which is supported via
> panel-simple driver with "bananapi,s070wv20-ct16" compatible.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/gpu/drm/panel/Kconfig                 |   9 +
>  drivers/gpu/drm/panel/Makefile                |   1 +
>  .../panel/panel-bananapi-s070wv20-icn6211.c   | 293 ++++++++++++++++++

This would be an overlay patch, which doesn't need to mege. please
correct the same.
Jagan Teki Oct. 3, 2019, 6:50 a.m. UTC | #2 
On Thu, Oct 3, 2019 at 12:16 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> This patch add support for Bananapi S070WV20-CT16 DSI panel to
> BPI-M2M board.
>
> DSI panel connected via board DSI port with,
> - DCDC1 as VCC-DSI supply
> - DLDO1 as VDD supply
> - PL5 gpio for lcd reset gpio pin
> - PB7 gpio for lcd enable gpio pin
> - PL4 gpio for backlight enable pin
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---

This would be an overlay patch, which doesn't need to mege. please
correct the same.
Icenowy Zheng Oct. 3, 2019, 6:51 a.m. UTC | #3 
于 2019年10月3日 GMT+08:00 下午2:45:21, Jagan Teki <jagan@amarulasolutions.com> 写到:
>The LCD timing definitions between Linux DRM vs Allwinner are
>different,
>below diagram shows this clear differences.
>
>           Active                 Front           Sync           Back
>           Region                 Porch                          Porch
><-----------------------><----------------><--------------><-------------->
>  //////////////////////|
> ////////////////////// |
>//////////////////////  |..................               
>................
>                                           ________________
><----- [hv]display ----->
><------------- [hv]sync_start ------------>
><--------------------- [hv]sync_end ---------------------->
><-------------------------------- [hv]total
>------------------------------>
>
><----- lcd_[xy] -------->		  <- lcd_[hv]spw ->
>					  <---------- lcd_[hv]bp --------->
><-------------------------------- lcd_[hv]t
>------------------------------>
>
>The DSI driver misinterpreted the hbp term from the BSP code to refer
>only to the backporch, when in fact it was backporch + sync. Thus the
>driver incorrectly used the horizontal front porch plus sync in its
>calculation of the DRQ set bit value, when it should not have included
>the sync timing.
>
>Including additional sync timings leads to flip_done timed out as:

I don't think attaching this error infomation is useful at all.

It's just timing mismatch.

>
>WARNING: CPU: 0 PID: 31 at drivers/gpu/drm/drm_atomic_helper.c:1429
>drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0
>[CRTC:46:crtc-0] vblank wait timed out
>Modules linked in:
>CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted
>5.1.0-next-20190514-00026-g01f0c75b902d-dirty #13
>Hardware name: Allwinner sun8i Family
>Workqueue: events deferred_probe_work_func
>[<c010ed54>] (unwind_backtrace) from [<c010b76c>]
>(show_stack+0x10/0x14)
>[<c010b76c>] (show_stack) from [<c0688c70>] (dump_stack+0x84/0x98)
>[<c0688c70>] (dump_stack) from [<c011d9e4>] (__warn+0xfc/0x114)
>[<c011d9e4>] (__warn) from [<c011da40>] (warn_slowpath_fmt+0x44/0x68)
>[<c011da40>] (warn_slowpath_fmt) from [<c040cd50>]
>(drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0)
>[<c040cd50>] (drm_atomic_helper_wait_for_vblanks.part.1) from
>[<c040e694>] (drm_atomic_helper_commit_tail_rpm+0x5c/0x6c)
>[<c040e694>] (drm_atomic_helper_commit_tail_rpm) from [<c040e4dc>]
>(commit_tail+0x40/0x6c)
>[<c040e4dc>] (commit_tail) from [<c040e5cc>]
>(drm_atomic_helper_commit+0xbc/0x128)
>[<c040e5cc>] (drm_atomic_helper_commit) from [<c0411b64>]
>(restore_fbdev_mode_atomic+0x1cc/0x1dc)
>[<c0411b64>] (restore_fbdev_mode_atomic) from [<c04156f8>]
>(drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0)
>[<c04156f8>] (drm_fb_helper_restore_fbdev_mode_unlocked) from
>[<c0415774>] (drm_fb_helper_set_par+0x30/0x54)
>[<c0415774>] (drm_fb_helper_set_par) from [<c03ad450>]
>(fbcon_init+0x560/0x5ac)
>[<c03ad450>] (fbcon_init) from [<c03eb8a0>] (visual_init+0xbc/0x104)
>[<c03eb8a0>] (visual_init) from [<c03ed1b8>]
>(do_bind_con_driver+0x1b0/0x390)
>[<c03ed1b8>] (do_bind_con_driver) from [<c03ed780>]
>(do_take_over_console+0x13c/0x1c4)
>[<c03ed780>] (do_take_over_console) from [<c03ad800>]
>(do_fbcon_takeover+0x74/0xcc)
>[<c03ad800>] (do_fbcon_takeover) from [<c013c9c8>]
>(notifier_call_chain+0x44/0x84)
>[<c013c9c8>] (notifier_call_chain) from [<c013cd20>]
>(__blocking_notifier_call_chain+0x48/0x60)
>[<c013cd20>] (__blocking_notifier_call_chain) from [<c013cd50>]
>(blocking_notifier_call_chain+0x18/0x20)
>[<c013cd50>] (blocking_notifier_call_chain) from [<c03a6e44>]
>(register_framebuffer+0x1e0/0x2f8)
>[<c03a6e44>] (register_framebuffer) from [<c04153c0>]
>(__drm_fb_helper_initial_config_and_unlock+0x2fc/0x50c)
>[<c04153c0>] (__drm_fb_helper_initial_config_and_unlock) from
>[<c04158c8>] (drm_fbdev_client_hotplug+0xe8/0x1b8)
>[<c04158c8>] (drm_fbdev_client_hotplug) from [<c0415a20>]
>(drm_fbdev_generic_setup+0x88/0x118)
>[<c0415a20>] (drm_fbdev_generic_setup) from [<c043f060>]
>(sun4i_drv_bind+0x128/0x160)
>[<c043f060>] (sun4i_drv_bind) from [<c044b598>]
>(try_to_bring_up_master+0x164/0x1a0)
>[<c044b598>] (try_to_bring_up_master) from [<c044b668>]
>(__component_add+0x94/0x140)
>[<c044b668>] (__component_add) from [<c0445e1c>]
>(sun6i_dsi_probe+0x144/0x234)
>[<c0445e1c>] (sun6i_dsi_probe) from [<c0452ef4>]
>(platform_drv_probe+0x48/0x9c)
>[<c0452ef4>] (platform_drv_probe) from [<c04512cc>]
>(really_probe+0x1dc/0x2c8)
>[<c04512cc>] (really_probe) from [<c0451518>]
>(driver_probe_device+0x60/0x160)
>[<c0451518>] (driver_probe_device) from [<c044f7a4>]
>(bus_for_each_drv+0x74/0xb8)
>[<c044f7a4>] (bus_for_each_drv) from [<c045107c>]
>(__device_attach+0xd0/0x13c)
>[<c045107c>] (__device_attach) from [<c0450474>]
>(bus_probe_device+0x84/0x8c)
>[<c0450474>] (bus_probe_device) from [<c0450900>]
>(deferred_probe_work_func+0x64/0x90)
>[<c0450900>] (deferred_probe_work_func) from [<c0135970>]
>(process_one_work+0x204/0x420)
>[<c0135970>] (process_one_work) from [<c013690c>]
>(worker_thread+0x274/0x5a0)
>[<c013690c>] (worker_thread) from [<c013b3d8>] (kthread+0x11c/0x14c)
>[<c013b3d8>] (kthread) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
>Exception stack(0xde539fb0 to 0xde539ff8)
>9fa0:                                     00000000 00000000 00000000
>00000000
>9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>00000000
>9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>---[ end trace b57eb1e5c64c6b8b ]---
>random: fast init done
>[drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CRTC:46:crtc-0]
>flip_done timed out
>[drm:drm_atomic_helper_wait_for_dependencies] *ERROR*
>[CONNECTOR:48:DSI-1] flip_done timed out
>[drm:drm_atomic_helper_wait_for_dependencies] *ERROR*
>[PLANE:30:plane-0] flip_done timed out
>
>With the terms(as described in above diagram) fixed, the panel
>displays correctly without any timeouts.
>
>Tested-by: Merlijn Wajer <merlijn@wizzup.org>
>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 1636344ba9ec..f83522717488 100644
>--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>@@ -437,9 +437,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;
Icenowy Zheng Oct. 3, 2019, 6:52 a.m. UTC | #4 
于 2019年10月3日 GMT+08:00 下午2:45:22, Jagan Teki <jagan@amarulasolutions.com> 写到:
>start value in video start delay was changed in
>commit da676c6aa641 ("drm/sun4i: dsi: Change the start delay
>calculation")
>to match the legacy BSP driver [1].
>
>So, using this existing start delay computation gives the wrong
>start delay value for the "bananapi,s070wv20-ct16" panel. Due to
>this the panel trigger below flip_done timed out as during kernel
>bootup:
>
>WARNING: CPU: 0 PID: 31 at drivers/gpu/drm/drm_atomic_helper.c:1429
>drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0
> [CRTC:46:crtc-0] vblank wait timed out
> Modules linked in:
>CPU: 0 PID: 31 Comm: kworker/0:1 Tainted: G        W        
>5.1.0-next-20190514-00025-gf928bc7cc146 #15
> Hardware name: Allwinner sun8i Family
> Workqueue: events deferred_probe_work_func
>[<c010ed54>] (unwind_backtrace) from [<c010b76c>]
>(show_stack+0x10/0x14)
> [<c010b76c>] (show_stack) from [<c0688c90>] (dump_stack+0x84/0x98)
> [<c0688c90>] (dump_stack) from [<c011d9e4>] (__warn+0xfc/0x114)
> [<c011d9e4>] (__warn) from [<c011da40>] (warn_slowpath_fmt+0x44/0x68)
>[<c011da40>] (warn_slowpath_fmt) from [<c040cd50>]
>(drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0)
>[<c040cd50>] (drm_atomic_helper_wait_for_vblanks.part.1) from
>[<c040e694>] (drm_atomic_helper_commit_tail_rpm+0x5c/0x6c)
>[<c040e694>] (drm_atomic_helper_commit_tail_rpm) from [<c040e4dc>]
>(commit_tail+0x40/0x6c)
>[<c040e4dc>] (commit_tail) from [<c040e5cc>]
>(drm_atomic_helper_commit+0xbc/0x128)
>[<c040e5cc>] (drm_atomic_helper_commit) from [<c0411b64>]
>(restore_fbdev_mode_atomic+0x1cc/0x1dc)
>[<c0411b64>] (restore_fbdev_mode_atomic) from [<c0411cb0>]
>(drm_fb_helper_pan_display+0xac/0x1d0)
>[<c0411cb0>] (drm_fb_helper_pan_display) from [<c03a4e84>]
>(fb_pan_display+0xcc/0x134)
>[<c03a4e84>] (fb_pan_display) from [<c03b1214>]
>(bit_update_start+0x14/0x30)
>[<c03b1214>] (bit_update_start) from [<c03afe94>]
>(fbcon_switch+0x3d8/0x4e0)
>[<c03afe94>] (fbcon_switch) from [<c03ec930>]
>(redraw_screen+0x174/0x238)
>[<c03ec930>] (redraw_screen) from [<c03aceb4>]
>(fbcon_prepare_logo+0x3c4/0x400)
>[<c03aceb4>] (fbcon_prepare_logo) from [<c03ad2b8>]
>(fbcon_init+0x3c8/0x5ac)
> [<c03ad2b8>] (fbcon_init) from [<c03eb8a0>] (visual_init+0xbc/0x104)
>[<c03eb8a0>] (visual_init) from [<c03ed1b8>]
>(do_bind_con_driver+0x1b0/0x390)
>[<c03ed1b8>] (do_bind_con_driver) from [<c03ed780>]
>(do_take_over_console+0x13c/0x1c4)
>[<c03ed780>] (do_take_over_console) from [<c03ad800>]
>(do_fbcon_takeover+0x74/0xcc)
>[<c03ad800>] (do_fbcon_takeover) from [<c013c9c8>]
>(notifier_call_chain+0x44/0x84)
>[<c013c9c8>] (notifier_call_chain) from [<c013cd20>]
>(__blocking_notifier_call_chain+0x48/0x60)
>[<c013cd20>] (__blocking_notifier_call_chain) from [<c013cd50>]
>(blocking_notifier_call_chain+0x18/0x20)
>[<c013cd50>] (blocking_notifier_call_chain) from [<c03a6e44>]
>(register_framebuffer+0x1e0/0x2f8)
>[<c03a6e44>] (register_framebuffer) from [<c04153c0>]
>(__drm_fb_helper_initial_config_and_unlock+0x2fc/0x50c)
>[<c04153c0>] (__drm_fb_helper_initial_config_and_unlock) from
>[<c04158c8>] (drm_fbdev_client_hotplug+0xe8/0x1b8)
>[<c04158c8>] (drm_fbdev_client_hotplug) from [<c0415a20>]
>(drm_fbdev_generic_setup+0x88/0x118)
>[<c0415a20>] (drm_fbdev_generic_setup) from [<c043f060>]
>(sun4i_drv_bind+0x128/0x160)
>[<c043f060>] (sun4i_drv_bind) from [<c044b5b0>]
>(try_to_bring_up_master+0x164/0x1a0)
>[<c044b5b0>] (try_to_bring_up_master) from [<c044b680>]
>(__component_add+0x94/0x140)
>[<c044b680>] (__component_add) from [<c0445e1c>]
>(sun6i_dsi_probe+0x144/0x234)
>[<c0445e1c>] (sun6i_dsi_probe) from [<c0452f0c>]
>(platform_drv_probe+0x48/0x9c)
>[<c0452f0c>] (platform_drv_probe) from [<c04512e4>]
>(really_probe+0x1dc/0x2c8)
>[<c04512e4>] (really_probe) from [<c0451530>]
>(driver_probe_device+0x60/0x160)
>[<c0451530>] (driver_probe_device) from [<c044f7bc>]
>(bus_for_each_drv+0x74/0xb8)
>[<c044f7bc>] (bus_for_each_drv) from [<c0451094>]
>(__device_attach+0xd0/0x13c)
>[<c0451094>] (__device_attach) from [<c045048c>]
>(bus_probe_device+0x84/0x8c)
>[<c045048c>] (bus_probe_device) from [<c0450918>]
>(deferred_probe_work_func+0x64/0x90)
>[<c0450918>] (deferred_probe_work_func) from [<c0135970>]
>(process_one_work+0x204/0x420)
>[<c0135970>] (process_one_work) from [<c013690c>]
>(worker_thread+0x274/0x5a0)
> [<c013690c>] (worker_thread) from [<c013b3d8>] (kthread+0x11c/0x14c)
> [<c013b3d8>] (kthread) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
> Exception stack(0xde539fb0 to 0xde539ff8)
>9fa0:                                     00000000 00000000 00000000
>00000000
>9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>00000000
> 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> ---[ end trace 755e10f62b83f396 ]---
> Console: switching to colour frame buffer device 100x30
>[drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CRTC:46:crtc-0]
>flip_done timed out
>[drm:drm_atomic_helper_wait_for_dependencies] *ERROR*
>[CONNECTOR:48:DSI-1] flip_done timed out
>[drm:drm_atomic_helper_wait_for_dependencies] *ERROR*
>[PLANE:30:plane-0] flip_done timed out
>
>To fix this, adjust the start delay computation according to the
>new BSP code [2].
>
>Unfortunately we don't have any evidence or documentation for this
>reassignment to 1 in new bsp, but it is working with below mainline
>supported panels in A33, A64.
>- bananapi,s070wv20-ct16
>- feiyang,fy07024di26a30d
>- techstar,ts8550b
>
>So, use the start as per new bsp code since it is working in all
>the supported panels.
>
>[1]
>https://github.com/BPI-SINOVOIP/BPI-M2M-bsp/blob/master/linux-sunxi/drivers/video/sunxi/legacy/disp/de_bsp/de/ebios/de_dsi.c#L682
>[2]
>https://github.com/BPI-SINOVOIP/BPI-M2M-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp/de/lowlevel_sun8iw5/de_dsi.c#L807
>
>Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>---
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>index f83522717488..c9c99c52bf1e 100644
>--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>@@ -365,7 +365,17 @@ 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)
> {
>-	u16 start = clamp(mode->vtotal - mode->vdisplay - 10, 8, 100);
>+	/**
>+	 * Allwinner legacy (drivers/video/sunxi/legacy),
>+	 * new (drivers/video/sunxi/disp/de/lowlevel_sun8iw5) bsp drivers
>+	 * are evaluating start as:
>+	 *
>+	 *	vtotal - vdisplay - 10
>+	 *
>+	 * but the new drivers are reassigning start to 1, which seems to be
>+	 * working in DSI panels available in mainline.

Please see my patchset for the reason that set 1 here.

>+	 */
>+	u8 start = 1;
>	u16 delay = mode->vtotal - (mode->vsync_end - mode->vdisplay) + start;
> 
> 	if (delay > mode->vtotal)
Jagan Teki Oct. 3, 2019, 6:55 a.m. UTC | #5 
On Thu, Oct 3, 2019 at 12:21 PM Icenowy Zheng <icenowy@aosc.io> wrote:
>
>
>
> 于 2019年10月3日 GMT+08:00 下午2:45:21, Jagan Teki <jagan@amarulasolutions.com> 写到:
> >The LCD timing definitions between Linux DRM vs Allwinner are
> >different,
> >below diagram shows this clear differences.
> >
> >           Active                 Front           Sync           Back
> >           Region                 Porch                          Porch
> ><-----------------------><----------------><--------------><-------------->
> >  //////////////////////|
> > ////////////////////// |
> >//////////////////////  |..................
> >................
> >                                           ________________
> ><----- [hv]display ----->
> ><------------- [hv]sync_start ------------>
> ><--------------------- [hv]sync_end ---------------------->
> ><-------------------------------- [hv]total
> >------------------------------>
> >
> ><----- lcd_[xy] -------->                <- lcd_[hv]spw ->
> >                                         <---------- lcd_[hv]bp --------->
> ><-------------------------------- lcd_[hv]t
> >------------------------------>
> >
> >The DSI driver misinterpreted the hbp term from the BSP code to refer
> >only to the backporch, when in fact it was backporch + sync. Thus the
> >driver incorrectly used the horizontal front porch plus sync in its
> >calculation of the DRQ set bit value, when it should not have included
> >the sync timing.
> >
> >Including additional sync timings leads to flip_done timed out as:
>
> I don't think attaching this error infomation is useful at all.

Since the error would be common irrespective of panels and it would
trigger from dsi controller, I thought this would be useful for
reference. I have had this conversation in previous version changes,
so I have added it. let me know if have any comments.
Chen-Yu Tsai Oct. 3, 2019, 6:55 a.m. UTC | #6 
On Thu, Oct 3, 2019 at 2:46 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Allwinner MIPI DSI controllers are supplied with SoC
> DSI power rails via VCC-DSI pin.
>
> Add support for this supply pin by adding voltage
> regulator handling code to MIPI DSI driver.
>
> Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 14 ++++++++++++++
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 ++
>  2 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 446dc56cc44b..fe9a3667f3a1 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -1110,6 +1110,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
>                 return PTR_ERR(base);
>         }
>
> +       dsi->regulator = devm_regulator_get(dev, "vcc-dsi");
> +       if (IS_ERR(dsi->regulator)) {
> +               dev_err(dev, "Couldn't get VCC-DSI supply\n");
> +               return PTR_ERR(dsi->regulator);
> +       }
> +
>         dsi->regs = devm_regmap_init_mmio_clk(dev, "bus", base,
>                                               &sun6i_dsi_regmap_config);
>         if (IS_ERR(dsi->regs)) {
> @@ -1183,6 +1189,13 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
>  static int __maybe_unused sun6i_dsi_runtime_resume(struct device *dev)
>  {
>         struct sun6i_dsi *dsi = dev_get_drvdata(dev);
> +       int err;
> +
> +       err = regulator_enable(dsi->regulator);
> +       if (err) {
> +               dev_err(dsi->dev, "failed to enable VCC-DSI supply: %d\n", err);
> +               return err;
> +       }
>
>         reset_control_deassert(dsi->reset);
>         clk_prepare_enable(dsi->mod_clk);
> @@ -1215,6 +1228,7 @@ static int __maybe_unused sun6i_dsi_runtime_suspend(struct device *dev)
>
>         clk_disable_unprepare(dsi->mod_clk);
>         reset_control_assert(dsi->reset);
> +       regulator_disable(dsi->regulator);
>
>         return 0;
>  }
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> index 5c3ad5be0690..a01d44e9e461 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> @@ -12,6 +12,7 @@
>  #include <drm/drm_connector.h>
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_mipi_dsi.h>
> +#include <linux/regulator/consumer.h>

You don't need to include the header file since you are only
including a pointer to the struct, and nothing else.

Otherwise,

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

>
>  #define SUN6I_DSI_TCON_DIV     4
>
> @@ -23,6 +24,7 @@ struct sun6i_dsi {
>         struct clk              *bus_clk;
>         struct clk              *mod_clk;
>         struct regmap           *regs;
> +       struct regulator        *regulator;
>         struct reset_control    *reset;
>         struct phy              *dphy;
>
> --
> 2.18.0.321.gffc6fa0e3
>
Jagan Teki Oct. 3, 2019, 7:01 a.m. UTC | #7 
On Thu, Oct 3, 2019 at 12:26 PM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Thu, Oct 3, 2019 at 2:46 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Allwinner MIPI DSI controllers are supplied with SoC
> > DSI power rails via VCC-DSI pin.
> >
> > Add support for this supply pin by adding voltage
> > regulator handling code to MIPI DSI driver.
> >
> > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 14 ++++++++++++++
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 ++
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index 446dc56cc44b..fe9a3667f3a1 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -1110,6 +1110,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
> >                 return PTR_ERR(base);
> >         }
> >
> > +       dsi->regulator = devm_regulator_get(dev, "vcc-dsi");
> > +       if (IS_ERR(dsi->regulator)) {
> > +               dev_err(dev, "Couldn't get VCC-DSI supply\n");
> > +               return PTR_ERR(dsi->regulator);
> > +       }
> > +
> >         dsi->regs = devm_regmap_init_mmio_clk(dev, "bus", base,
> >                                               &sun6i_dsi_regmap_config);
> >         if (IS_ERR(dsi->regs)) {
> > @@ -1183,6 +1189,13 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
> >  static int __maybe_unused sun6i_dsi_runtime_resume(struct device *dev)
> >  {
> >         struct sun6i_dsi *dsi = dev_get_drvdata(dev);
> > +       int err;
> > +
> > +       err = regulator_enable(dsi->regulator);
> > +       if (err) {
> > +               dev_err(dsi->dev, "failed to enable VCC-DSI supply: %d\n", err);
> > +               return err;
> > +       }
> >
> >         reset_control_deassert(dsi->reset);
> >         clk_prepare_enable(dsi->mod_clk);
> > @@ -1215,6 +1228,7 @@ static int __maybe_unused sun6i_dsi_runtime_suspend(struct device *dev)
> >
> >         clk_disable_unprepare(dsi->mod_clk);
> >         reset_control_assert(dsi->reset);
> > +       regulator_disable(dsi->regulator);
> >
> >         return 0;
> >  }
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > index 5c3ad5be0690..a01d44e9e461 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > @@ -12,6 +12,7 @@
> >  #include <drm/drm_connector.h>
> >  #include <drm/drm_encoder.h>
> >  #include <drm/drm_mipi_dsi.h>
> > +#include <linux/regulator/consumer.h>
>
> You don't need to include the header file since you are only
> including a pointer to the struct, and nothing else.

Yes, make sense. I will drop it.

>
> Otherwise,
>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>

thanks.
Maxime Ripard Oct. 3, 2019, 11:46 a.m. UTC | #8 
On Thu, Oct 03, 2019 at 12:31:31PM +0530, Jagan Teki wrote:
> On Thu, Oct 3, 2019 at 12:26 PM Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > On Thu, Oct 3, 2019 at 2:46 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > Allwinner MIPI DSI controllers are supplied with SoC
> > > DSI power rails via VCC-DSI pin.
> > >
> > > Add support for this supply pin by adding voltage
> > > regulator handling code to MIPI DSI driver.
> > >
> > > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 14 ++++++++++++++
> > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 ++
> > >  2 files changed, 16 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > index 446dc56cc44b..fe9a3667f3a1 100644
> > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > @@ -1110,6 +1110,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
> > >                 return PTR_ERR(base);
> > >         }
> > >
> > > +       dsi->regulator = devm_regulator_get(dev, "vcc-dsi");
> > > +       if (IS_ERR(dsi->regulator)) {
> > > +               dev_err(dev, "Couldn't get VCC-DSI supply\n");
> > > +               return PTR_ERR(dsi->regulator);
> > > +       }
> > > +
> > >         dsi->regs = devm_regmap_init_mmio_clk(dev, "bus", base,
> > >                                               &sun6i_dsi_regmap_config);
> > >         if (IS_ERR(dsi->regs)) {
> > > @@ -1183,6 +1189,13 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
> > >  static int __maybe_unused sun6i_dsi_runtime_resume(struct device *dev)
> > >  {
> > >         struct sun6i_dsi *dsi = dev_get_drvdata(dev);
> > > +       int err;
> > > +
> > > +       err = regulator_enable(dsi->regulator);
> > > +       if (err) {
> > > +               dev_err(dsi->dev, "failed to enable VCC-DSI supply: %d\n", err);
> > > +               return err;
> > > +       }
> > >
> > >         reset_control_deassert(dsi->reset);
> > >         clk_prepare_enable(dsi->mod_clk);
> > > @@ -1215,6 +1228,7 @@ static int __maybe_unused sun6i_dsi_runtime_suspend(struct device *dev)
> > >
> > >         clk_disable_unprepare(dsi->mod_clk);
> > >         reset_control_assert(dsi->reset);
> > > +       regulator_disable(dsi->regulator);
> > >
> > >         return 0;
> > >  }
> > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > index 5c3ad5be0690..a01d44e9e461 100644
> > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > @@ -12,6 +12,7 @@
> > >  #include <drm/drm_connector.h>
> > >  #include <drm/drm_encoder.h>
> > >  #include <drm/drm_mipi_dsi.h>
> > > +#include <linux/regulator/consumer.h>
> >
> > You don't need to include the header file since you are only
> > including a pointer to the struct, and nothing else.
>
> Yes, make sense. I will drop it.
>
> >
> > Otherwise,
> >
> > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>
> thanks.

I've moved the include to the driver that was lacking it while
applying, thanks!

Maxime
Maxime Ripard Oct. 3, 2019, 1:17 p.m. UTC | #9 
On Thu, Oct 03, 2019 at 12:15:21PM +0530, Jagan Teki wrote:
> The LCD timing definitions between Linux DRM vs Allwinner are different,
> below diagram shows this clear differences.
>
>            Active                 Front           Sync           Back
>            Region                 Porch                          Porch
> <-----------------------><----------------><--------------><-------------->
>   //////////////////////|
>  ////////////////////// |
> //////////////////////  |..................                ................
>                                            ________________
> <----- [hv]display ----->
> <------------- [hv]sync_start ------------>
> <--------------------- [hv]sync_end ---------------------->
> <-------------------------------- [hv]total ------------------------------>
>
> <----- lcd_[xy] -------->		  <- lcd_[hv]spw ->
> 					  <---------- lcd_[hv]bp --------->
> <-------------------------------- lcd_[hv]t ------------------------------>
>
> The DSI driver misinterpreted the hbp term from the BSP code to refer
> only to the backporch, when in fact it was backporch + sync. Thus the
> driver incorrectly used the horizontal front porch plus sync in its
> calculation of the DRQ set bit value, when it should not have included
> the sync timing.
>
> Including additional sync timings leads to flip_done timed out as:
>
> WARNING: CPU: 0 PID: 31 at drivers/gpu/drm/drm_atomic_helper.c:1429 drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0
> [CRTC:46:crtc-0] vblank wait timed out
> Modules linked in:
> CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted 5.1.0-next-20190514-00026-g01f0c75b902d-dirty #13
> Hardware name: Allwinner sun8i Family
> Workqueue: events deferred_probe_work_func
> [<c010ed54>] (unwind_backtrace) from [<c010b76c>] (show_stack+0x10/0x14)
> [<c010b76c>] (show_stack) from [<c0688c70>] (dump_stack+0x84/0x98)
> [<c0688c70>] (dump_stack) from [<c011d9e4>] (__warn+0xfc/0x114)
> [<c011d9e4>] (__warn) from [<c011da40>] (warn_slowpath_fmt+0x44/0x68)
> [<c011da40>] (warn_slowpath_fmt) from [<c040cd50>] (drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0)
> [<c040cd50>] (drm_atomic_helper_wait_for_vblanks.part.1) from [<c040e694>] (drm_atomic_helper_commit_tail_rpm+0x5c/0x6c)
> [<c040e694>] (drm_atomic_helper_commit_tail_rpm) from [<c040e4dc>] (commit_tail+0x40/0x6c)
> [<c040e4dc>] (commit_tail) from [<c040e5cc>] (drm_atomic_helper_commit+0xbc/0x128)
> [<c040e5cc>] (drm_atomic_helper_commit) from [<c0411b64>] (restore_fbdev_mode_atomic+0x1cc/0x1dc)
> [<c0411b64>] (restore_fbdev_mode_atomic) from [<c04156f8>] (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0)
> [<c04156f8>] (drm_fb_helper_restore_fbdev_mode_unlocked) from [<c0415774>] (drm_fb_helper_set_par+0x30/0x54)
> [<c0415774>] (drm_fb_helper_set_par) from [<c03ad450>] (fbcon_init+0x560/0x5ac)
> [<c03ad450>] (fbcon_init) from [<c03eb8a0>] (visual_init+0xbc/0x104)
> [<c03eb8a0>] (visual_init) from [<c03ed1b8>] (do_bind_con_driver+0x1b0/0x390)
> [<c03ed1b8>] (do_bind_con_driver) from [<c03ed780>] (do_take_over_console+0x13c/0x1c4)
> [<c03ed780>] (do_take_over_console) from [<c03ad800>] (do_fbcon_takeover+0x74/0xcc)
> [<c03ad800>] (do_fbcon_takeover) from [<c013c9c8>] (notifier_call_chain+0x44/0x84)
> [<c013c9c8>] (notifier_call_chain) from [<c013cd20>] (__blocking_notifier_call_chain+0x48/0x60)
> [<c013cd20>] (__blocking_notifier_call_chain) from [<c013cd50>] (blocking_notifier_call_chain+0x18/0x20)
> [<c013cd50>] (blocking_notifier_call_chain) from [<c03a6e44>] (register_framebuffer+0x1e0/0x2f8)
> [<c03a6e44>] (register_framebuffer) from [<c04153c0>] (__drm_fb_helper_initial_config_and_unlock+0x2fc/0x50c)
> [<c04153c0>] (__drm_fb_helper_initial_config_and_unlock) from [<c04158c8>] (drm_fbdev_client_hotplug+0xe8/0x1b8)
> [<c04158c8>] (drm_fbdev_client_hotplug) from [<c0415a20>] (drm_fbdev_generic_setup+0x88/0x118)
> [<c0415a20>] (drm_fbdev_generic_setup) from [<c043f060>] (sun4i_drv_bind+0x128/0x160)
> [<c043f060>] (sun4i_drv_bind) from [<c044b598>] (try_to_bring_up_master+0x164/0x1a0)
> [<c044b598>] (try_to_bring_up_master) from [<c044b668>] (__component_add+0x94/0x140)
> [<c044b668>] (__component_add) from [<c0445e1c>] (sun6i_dsi_probe+0x144/0x234)
> [<c0445e1c>] (sun6i_dsi_probe) from [<c0452ef4>] (platform_drv_probe+0x48/0x9c)
> [<c0452ef4>] (platform_drv_probe) from [<c04512cc>] (really_probe+0x1dc/0x2c8)
> [<c04512cc>] (really_probe) from [<c0451518>] (driver_probe_device+0x60/0x160)
> [<c0451518>] (driver_probe_device) from [<c044f7a4>] (bus_for_each_drv+0x74/0xb8)
> [<c044f7a4>] (bus_for_each_drv) from [<c045107c>] (__device_attach+0xd0/0x13c)
> [<c045107c>] (__device_attach) from [<c0450474>] (bus_probe_device+0x84/0x8c)
> [<c0450474>] (bus_probe_device) from [<c0450900>] (deferred_probe_work_func+0x64/0x90)
> [<c0450900>] (deferred_probe_work_func) from [<c0135970>] (process_one_work+0x204/0x420)
> [<c0135970>] (process_one_work) from [<c013690c>] (worker_thread+0x274/0x5a0)
> [<c013690c>] (worker_thread) from [<c013b3d8>] (kthread+0x11c/0x14c)
> [<c013b3d8>] (kthread) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
> Exception stack(0xde539fb0 to 0xde539ff8)
> 9fa0:                                     00000000 00000000 00000000 00000000
> 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> ---[ end trace b57eb1e5c64c6b8b ]---
> random: fast init done
> [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CRTC:46:crtc-0] flip_done timed out
> [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CONNECTOR:48:DSI-1] flip_done timed out
> [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:30:plane-0] flip_done timed out
>
> With the terms(as described in above diagram) fixed, the panel
> displays correctly without any timeouts.
>
> Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

Applied, thanks

Maxime
Jagan Teki Oct. 3, 2019, 3:16 p.m. UTC | #10 
On Thu, Oct 3, 2019 at 6:47 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Thu, Oct 03, 2019 at 12:15:21PM +0530, Jagan Teki wrote:
> > The LCD timing definitions between Linux DRM vs Allwinner are different,
> > below diagram shows this clear differences.
> >
> >            Active                 Front           Sync           Back
> >            Region                 Porch                          Porch
> > <-----------------------><----------------><--------------><-------------->
> >   //////////////////////|
> >  ////////////////////// |
> > //////////////////////  |..................                ................
> >                                            ________________
> > <----- [hv]display ----->
> > <------------- [hv]sync_start ------------>
> > <--------------------- [hv]sync_end ---------------------->
> > <-------------------------------- [hv]total ------------------------------>
> >
> > <----- lcd_[xy] -------->               <- lcd_[hv]spw ->
> >                                         <---------- lcd_[hv]bp --------->
> > <-------------------------------- lcd_[hv]t ------------------------------>
> >
> > The DSI driver misinterpreted the hbp term from the BSP code to refer
> > only to the backporch, when in fact it was backporch + sync. Thus the
> > driver incorrectly used the horizontal front porch plus sync in its
> > calculation of the DRQ set bit value, when it should not have included
> > the sync timing.
> >
> > Including additional sync timings leads to flip_done timed out as:
> >
> > WARNING: CPU: 0 PID: 31 at drivers/gpu/drm/drm_atomic_helper.c:1429 drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0
> > [CRTC:46:crtc-0] vblank wait timed out
> > Modules linked in:
> > CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted 5.1.0-next-20190514-00026-g01f0c75b902d-dirty #13
> > Hardware name: Allwinner sun8i Family
> > Workqueue: events deferred_probe_work_func
> > [<c010ed54>] (unwind_backtrace) from [<c010b76c>] (show_stack+0x10/0x14)
> > [<c010b76c>] (show_stack) from [<c0688c70>] (dump_stack+0x84/0x98)
> > [<c0688c70>] (dump_stack) from [<c011d9e4>] (__warn+0xfc/0x114)
> > [<c011d9e4>] (__warn) from [<c011da40>] (warn_slowpath_fmt+0x44/0x68)
> > [<c011da40>] (warn_slowpath_fmt) from [<c040cd50>] (drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0)
> > [<c040cd50>] (drm_atomic_helper_wait_for_vblanks.part.1) from [<c040e694>] (drm_atomic_helper_commit_tail_rpm+0x5c/0x6c)
> > [<c040e694>] (drm_atomic_helper_commit_tail_rpm) from [<c040e4dc>] (commit_tail+0x40/0x6c)
> > [<c040e4dc>] (commit_tail) from [<c040e5cc>] (drm_atomic_helper_commit+0xbc/0x128)
> > [<c040e5cc>] (drm_atomic_helper_commit) from [<c0411b64>] (restore_fbdev_mode_atomic+0x1cc/0x1dc)
> > [<c0411b64>] (restore_fbdev_mode_atomic) from [<c04156f8>] (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0)
> > [<c04156f8>] (drm_fb_helper_restore_fbdev_mode_unlocked) from [<c0415774>] (drm_fb_helper_set_par+0x30/0x54)
> > [<c0415774>] (drm_fb_helper_set_par) from [<c03ad450>] (fbcon_init+0x560/0x5ac)
> > [<c03ad450>] (fbcon_init) from [<c03eb8a0>] (visual_init+0xbc/0x104)
> > [<c03eb8a0>] (visual_init) from [<c03ed1b8>] (do_bind_con_driver+0x1b0/0x390)
> > [<c03ed1b8>] (do_bind_con_driver) from [<c03ed780>] (do_take_over_console+0x13c/0x1c4)
> > [<c03ed780>] (do_take_over_console) from [<c03ad800>] (do_fbcon_takeover+0x74/0xcc)
> > [<c03ad800>] (do_fbcon_takeover) from [<c013c9c8>] (notifier_call_chain+0x44/0x84)
> > [<c013c9c8>] (notifier_call_chain) from [<c013cd20>] (__blocking_notifier_call_chain+0x48/0x60)
> > [<c013cd20>] (__blocking_notifier_call_chain) from [<c013cd50>] (blocking_notifier_call_chain+0x18/0x20)
> > [<c013cd50>] (blocking_notifier_call_chain) from [<c03a6e44>] (register_framebuffer+0x1e0/0x2f8)
> > [<c03a6e44>] (register_framebuffer) from [<c04153c0>] (__drm_fb_helper_initial_config_and_unlock+0x2fc/0x50c)
> > [<c04153c0>] (__drm_fb_helper_initial_config_and_unlock) from [<c04158c8>] (drm_fbdev_client_hotplug+0xe8/0x1b8)
> > [<c04158c8>] (drm_fbdev_client_hotplug) from [<c0415a20>] (drm_fbdev_generic_setup+0x88/0x118)
> > [<c0415a20>] (drm_fbdev_generic_setup) from [<c043f060>] (sun4i_drv_bind+0x128/0x160)
> > [<c043f060>] (sun4i_drv_bind) from [<c044b598>] (try_to_bring_up_master+0x164/0x1a0)
> > [<c044b598>] (try_to_bring_up_master) from [<c044b668>] (__component_add+0x94/0x140)
> > [<c044b668>] (__component_add) from [<c0445e1c>] (sun6i_dsi_probe+0x144/0x234)
> > [<c0445e1c>] (sun6i_dsi_probe) from [<c0452ef4>] (platform_drv_probe+0x48/0x9c)
> > [<c0452ef4>] (platform_drv_probe) from [<c04512cc>] (really_probe+0x1dc/0x2c8)
> > [<c04512cc>] (really_probe) from [<c0451518>] (driver_probe_device+0x60/0x160)
> > [<c0451518>] (driver_probe_device) from [<c044f7a4>] (bus_for_each_drv+0x74/0xb8)
> > [<c044f7a4>] (bus_for_each_drv) from [<c045107c>] (__device_attach+0xd0/0x13c)
> > [<c045107c>] (__device_attach) from [<c0450474>] (bus_probe_device+0x84/0x8c)
> > [<c0450474>] (bus_probe_device) from [<c0450900>] (deferred_probe_work_func+0x64/0x90)
> > [<c0450900>] (deferred_probe_work_func) from [<c0135970>] (process_one_work+0x204/0x420)
> > [<c0135970>] (process_one_work) from [<c013690c>] (worker_thread+0x274/0x5a0)
> > [<c013690c>] (worker_thread) from [<c013b3d8>] (kthread+0x11c/0x14c)
> > [<c013b3d8>] (kthread) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
> > Exception stack(0xde539fb0 to 0xde539ff8)
> > 9fa0:                                     00000000 00000000 00000000 00000000
> > 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > ---[ end trace b57eb1e5c64c6b8b ]---
> > random: fast init done
> > [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CRTC:46:crtc-0] flip_done timed out
> > [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CONNECTOR:48:DSI-1] flip_done timed out
> > [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:30:plane-0] flip_done timed out
> >
> > With the terms(as described in above diagram) fixed, the panel
> > displays correctly without any timeouts.
> >
> > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>
> Applied, thanks

Thanks, would you apply the similar change in 3/7?
Maxime Ripard Oct. 7, 2019, 11:19 a.m. UTC | #11 
On Thu, Oct 03, 2019 at 08:46:31PM +0530, Jagan Teki wrote:
> On Thu, Oct 3, 2019 at 6:47 PM Maxime Ripard <mripard@kernel.org> wrote:
> >
> > On Thu, Oct 03, 2019 at 12:15:21PM +0530, Jagan Teki wrote:
> > > The LCD timing definitions between Linux DRM vs Allwinner are different,
> > > below diagram shows this clear differences.
> > >
> > >            Active                 Front           Sync           Back
> > >            Region                 Porch                          Porch
> > > <-----------------------><----------------><--------------><-------------->
> > >   //////////////////////|
> > >  ////////////////////// |
> > > //////////////////////  |..................                ................
> > >                                            ________________
> > > <----- [hv]display ----->
> > > <------------- [hv]sync_start ------------>
> > > <--------------------- [hv]sync_end ---------------------->
> > > <-------------------------------- [hv]total ------------------------------>
> > >
> > > <----- lcd_[xy] -------->               <- lcd_[hv]spw ->
> > >                                         <---------- lcd_[hv]bp --------->
> > > <-------------------------------- lcd_[hv]t ------------------------------>
> > >
> > > The DSI driver misinterpreted the hbp term from the BSP code to refer
> > > only to the backporch, when in fact it was backporch + sync. Thus the
> > > driver incorrectly used the horizontal front porch plus sync in its
> > > calculation of the DRQ set bit value, when it should not have included
> > > the sync timing.
> > >
> > > Including additional sync timings leads to flip_done timed out as:
> > >
> > > WARNING: CPU: 0 PID: 31 at drivers/gpu/drm/drm_atomic_helper.c:1429 drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0
> > > [CRTC:46:crtc-0] vblank wait timed out
> > > Modules linked in:
> > > CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted 5.1.0-next-20190514-00026-g01f0c75b902d-dirty #13
> > > Hardware name: Allwinner sun8i Family
> > > Workqueue: events deferred_probe_work_func
> > > [<c010ed54>] (unwind_backtrace) from [<c010b76c>] (show_stack+0x10/0x14)
> > > [<c010b76c>] (show_stack) from [<c0688c70>] (dump_stack+0x84/0x98)
> > > [<c0688c70>] (dump_stack) from [<c011d9e4>] (__warn+0xfc/0x114)
> > > [<c011d9e4>] (__warn) from [<c011da40>] (warn_slowpath_fmt+0x44/0x68)
> > > [<c011da40>] (warn_slowpath_fmt) from [<c040cd50>] (drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0)
> > > [<c040cd50>] (drm_atomic_helper_wait_for_vblanks.part.1) from [<c040e694>] (drm_atomic_helper_commit_tail_rpm+0x5c/0x6c)
> > > [<c040e694>] (drm_atomic_helper_commit_tail_rpm) from [<c040e4dc>] (commit_tail+0x40/0x6c)
> > > [<c040e4dc>] (commit_tail) from [<c040e5cc>] (drm_atomic_helper_commit+0xbc/0x128)
> > > [<c040e5cc>] (drm_atomic_helper_commit) from [<c0411b64>] (restore_fbdev_mode_atomic+0x1cc/0x1dc)
> > > [<c0411b64>] (restore_fbdev_mode_atomic) from [<c04156f8>] (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0)
> > > [<c04156f8>] (drm_fb_helper_restore_fbdev_mode_unlocked) from [<c0415774>] (drm_fb_helper_set_par+0x30/0x54)
> > > [<c0415774>] (drm_fb_helper_set_par) from [<c03ad450>] (fbcon_init+0x560/0x5ac)
> > > [<c03ad450>] (fbcon_init) from [<c03eb8a0>] (visual_init+0xbc/0x104)
> > > [<c03eb8a0>] (visual_init) from [<c03ed1b8>] (do_bind_con_driver+0x1b0/0x390)
> > > [<c03ed1b8>] (do_bind_con_driver) from [<c03ed780>] (do_take_over_console+0x13c/0x1c4)
> > > [<c03ed780>] (do_take_over_console) from [<c03ad800>] (do_fbcon_takeover+0x74/0xcc)
> > > [<c03ad800>] (do_fbcon_takeover) from [<c013c9c8>] (notifier_call_chain+0x44/0x84)
> > > [<c013c9c8>] (notifier_call_chain) from [<c013cd20>] (__blocking_notifier_call_chain+0x48/0x60)
> > > [<c013cd20>] (__blocking_notifier_call_chain) from [<c013cd50>] (blocking_notifier_call_chain+0x18/0x20)
> > > [<c013cd50>] (blocking_notifier_call_chain) from [<c03a6e44>] (register_framebuffer+0x1e0/0x2f8)
> > > [<c03a6e44>] (register_framebuffer) from [<c04153c0>] (__drm_fb_helper_initial_config_and_unlock+0x2fc/0x50c)
> > > [<c04153c0>] (__drm_fb_helper_initial_config_and_unlock) from [<c04158c8>] (drm_fbdev_client_hotplug+0xe8/0x1b8)
> > > [<c04158c8>] (drm_fbdev_client_hotplug) from [<c0415a20>] (drm_fbdev_generic_setup+0x88/0x118)
> > > [<c0415a20>] (drm_fbdev_generic_setup) from [<c043f060>] (sun4i_drv_bind+0x128/0x160)
> > > [<c043f060>] (sun4i_drv_bind) from [<c044b598>] (try_to_bring_up_master+0x164/0x1a0)
> > > [<c044b598>] (try_to_bring_up_master) from [<c044b668>] (__component_add+0x94/0x140)
> > > [<c044b668>] (__component_add) from [<c0445e1c>] (sun6i_dsi_probe+0x144/0x234)
> > > [<c0445e1c>] (sun6i_dsi_probe) from [<c0452ef4>] (platform_drv_probe+0x48/0x9c)
> > > [<c0452ef4>] (platform_drv_probe) from [<c04512cc>] (really_probe+0x1dc/0x2c8)
> > > [<c04512cc>] (really_probe) from [<c0451518>] (driver_probe_device+0x60/0x160)
> > > [<c0451518>] (driver_probe_device) from [<c044f7a4>] (bus_for_each_drv+0x74/0xb8)
> > > [<c044f7a4>] (bus_for_each_drv) from [<c045107c>] (__device_attach+0xd0/0x13c)
> > > [<c045107c>] (__device_attach) from [<c0450474>] (bus_probe_device+0x84/0x8c)
> > > [<c0450474>] (bus_probe_device) from [<c0450900>] (deferred_probe_work_func+0x64/0x90)
> > > [<c0450900>] (deferred_probe_work_func) from [<c0135970>] (process_one_work+0x204/0x420)
> > > [<c0135970>] (process_one_work) from [<c013690c>] (worker_thread+0x274/0x5a0)
> > > [<c013690c>] (worker_thread) from [<c013b3d8>] (kthread+0x11c/0x14c)
> > > [<c013b3d8>] (kthread) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
> > > Exception stack(0xde539fb0 to 0xde539ff8)
> > > 9fa0:                                     00000000 00000000 00000000 00000000
> > > 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > ---[ end trace b57eb1e5c64c6b8b ]---
> > > random: fast init done
> > > [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CRTC:46:crtc-0] flip_done timed out
> > > [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CONNECTOR:48:DSI-1] flip_done timed out
> > > [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:30:plane-0] flip_done timed out
> > >
> > > With the terms(as described in above diagram) fixed, the panel
> > > displays correctly without any timeouts.
> > >
> > > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >
> > Applied, thanks
>
> Thanks, would you apply the similar change in 3/7?

It doesn't apply anymore, please resend it

Maxime