Re: [PATCH v2] drm/atomic_helper: add a flag for duplicating drm_private_obj state

2020-07-11 Thread Shawn Guo
On Sat, Jun 27, 2020 at 7:53 PM Shawn Guo  wrote:
>
> From: Shawn Guo 
>
> The msm/mdp5 driver uses state of drm_private_obj as its global atomic
> state, which keeps the assignment of hwpipe to plane.  With
> drm_private_obj missing from duplicate state call in context of atomic
> suspend/resume helpers, mdp5 suspend works with no problem only for the
> very first time.  Any subsequent suspend will hit the following warning,
> because hwpipe assignment doesn't get duplicated for suspend state.
>
> $ echo mem > /sys/power/state
> [   38.44] PM: suspend entry (deep)
> [   38.85] PM: Syncing filesystems ... done.
> [   38.114630] Freezing user space processes ... (elapsed 0.001 seconds) done.
> [   38.115912] OOM killer disabled.
> [   38.115914] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) 
> done.
> [   38.122170] [ cut here ]
> [   38.122212] WARNING: CPU: 0 PID: 1747 at 
> drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c:145 mdp5_pipe_release+0x90/0xc0
> [   38.122215] Modules linked in:
> [   38.12] CPU: 0 PID: 1747 Comm: sh Not tainted 
> 4.19.107-00515-g9d5e4d7a33ed-dirty #323
> [   38.14] Hardware name: Square, Inc. T2 Devkit (DT)
> [   38.18] pstate: 4005 (nZcv daif -PAN -UAO)
> [   38.122230] pc : mdp5_pipe_release+0x90/0xc0
> [   38.122233] lr : mdp5_pipe_release+0x90/0xc0
> [   38.122235] sp : 0d13b7f0
> [   38.122236] x29: 0d13b7f0 x28: 
> [   38.122240] x27: 0002 x26: 800079adce00
> [   38.122243] x25: 800079405200 x24: 
> [   38.122246] x23: 80007a78cc08 x22: 80007b1cc018
> [   38.122249] x21: 80007b1cc000 x20: 80007b317080
> [   38.122252] x19: 80007a78ce80 x18: 0002
> [   38.122255] x17:  x16: 
> [   38.122258] x15: fff0 x14: 08c3fb48
> [   38.122261] x13: 08cdac4a x12: 08c3f000
> [   38.122264] x11:  x10: 08cda000
> [   38.122267] x9 :  x8 : 08ce4a40
> [   38.122269] x7 :  x6 : 39ea41a9
> [   38.122272] x5 :  x4 : 
> [   38.122275] x3 :  x2 : c7580c109cae4500
> [   38.122278] x1 :  x0 : 0024
> [   38.122281] Call trace:
> [   38.122285]  mdp5_pipe_release+0x90/0xc0
> [   38.122288]  mdp5_plane_atomic_check+0x2c0/0x448
> [   38.122294]  drm_atomic_helper_check_planes+0xd0/0x208
> [   38.122298]  drm_atomic_helper_check+0x38/0xa8
> [   38.122302]  drm_atomic_check_only+0x3e8/0x630
> [   38.122305]  drm_atomic_commit+0x18/0x58
> [   38.122309]  __drm_atomic_helper_disable_all.isra.12+0x15c/0x1a8
> [   38.122312]  drm_atomic_helper_suspend+0x80/0xf0
> [   38.122316]  msm_pm_suspend+0x4c/0x70
> [   38.122320]  dpm_run_callback.isra.6+0x20/0x68
> [   38.122323]  __device_suspend+0x110/0x308
> [   38.122326]  dpm_suspend+0x100/0x1f0
> [   38.122329]  dpm_suspend_start+0x64/0x70
> [   38.122334]  suspend_devices_and_enter+0x110/0x500
> [   38.122336]  pm_suspend+0x268/0x2c0
> [   38.122339]  state_store+0x88/0x110
> [   38.122345]  kobj_attr_store+0x14/0x28
> [   38.122352]  sysfs_kf_write+0x3c/0x50
> [   38.122355]  kernfs_fop_write+0x118/0x1e0
> [   38.122360]  __vfs_write+0x30/0x168
> [   38.122363]  vfs_write+0xa4/0x1a8
> [   38.122366]  ksys_write+0x64/0xe8
> [   38.122368]  __arm64_sys_write+0x18/0x20
> [   38.122374]  el0_svc_common+0x6c/0x178
> [   38.122377]  el0_svc_compat_handler+0x1c/0x28
> [   38.122381]  el0_svc_compat+0x8/0x18
> [   38.122383] ---[ end trace 24145b7d8545345b ]---
> [   38.491552] Disabling non-boot CPUs ...
>
> Let's add a flag for duplicating the state of drm_private_obj and set
> the flag from msm/mdp5 driver, so that the problem can be fixed while
> other drivers using drm_private_obj stay unaffected.
>
> Signed-off-by: Shawn Guo 
> ---
> Changes for v2:
>  - Add a flag to duplicate the state of drm_private_obj conditionally,
>so that other drivers using drm_private_obj stay unaffected.

Hi Daniel,

Are you okay with this version?

Shawn
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2] drm/atomic_helper: add a flag for duplicating drm_private_obj state

2020-06-27 Thread Shawn Guo
From: Shawn Guo 

The msm/mdp5 driver uses state of drm_private_obj as its global atomic
state, which keeps the assignment of hwpipe to plane.  With
drm_private_obj missing from duplicate state call in context of atomic
suspend/resume helpers, mdp5 suspend works with no problem only for the
very first time.  Any subsequent suspend will hit the following warning,
because hwpipe assignment doesn't get duplicated for suspend state.

$ echo mem > /sys/power/state
[   38.44] PM: suspend entry (deep)
[   38.85] PM: Syncing filesystems ... done.
[   38.114630] Freezing user space processes ... (elapsed 0.001 seconds) done.
[   38.115912] OOM killer disabled.
[   38.115914] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) 
done.
[   38.122170] [ cut here ]
[   38.122212] WARNING: CPU: 0 PID: 1747 at 
drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c:145 mdp5_pipe_release+0x90/0xc0
[   38.122215] Modules linked in:
[   38.12] CPU: 0 PID: 1747 Comm: sh Not tainted 
4.19.107-00515-g9d5e4d7a33ed-dirty #323
[   38.14] Hardware name: Square, Inc. T2 Devkit (DT)
[   38.18] pstate: 4005 (nZcv daif -PAN -UAO)
[   38.122230] pc : mdp5_pipe_release+0x90/0xc0
[   38.122233] lr : mdp5_pipe_release+0x90/0xc0
[   38.122235] sp : 0d13b7f0
[   38.122236] x29: 0d13b7f0 x28: 
[   38.122240] x27: 0002 x26: 800079adce00
[   38.122243] x25: 800079405200 x24: 
[   38.122246] x23: 80007a78cc08 x22: 80007b1cc018
[   38.122249] x21: 80007b1cc000 x20: 80007b317080
[   38.122252] x19: 80007a78ce80 x18: 0002
[   38.122255] x17:  x16: 
[   38.122258] x15: fff0 x14: 08c3fb48
[   38.122261] x13: 08cdac4a x12: 08c3f000
[   38.122264] x11:  x10: 08cda000
[   38.122267] x9 :  x8 : 08ce4a40
[   38.122269] x7 :  x6 : 39ea41a9
[   38.122272] x5 :  x4 : 
[   38.122275] x3 :  x2 : c7580c109cae4500
[   38.122278] x1 :  x0 : 0024
[   38.122281] Call trace:
[   38.122285]  mdp5_pipe_release+0x90/0xc0
[   38.122288]  mdp5_plane_atomic_check+0x2c0/0x448
[   38.122294]  drm_atomic_helper_check_planes+0xd0/0x208
[   38.122298]  drm_atomic_helper_check+0x38/0xa8
[   38.122302]  drm_atomic_check_only+0x3e8/0x630
[   38.122305]  drm_atomic_commit+0x18/0x58
[   38.122309]  __drm_atomic_helper_disable_all.isra.12+0x15c/0x1a8
[   38.122312]  drm_atomic_helper_suspend+0x80/0xf0
[   38.122316]  msm_pm_suspend+0x4c/0x70
[   38.122320]  dpm_run_callback.isra.6+0x20/0x68
[   38.122323]  __device_suspend+0x110/0x308
[   38.122326]  dpm_suspend+0x100/0x1f0
[   38.122329]  dpm_suspend_start+0x64/0x70
[   38.122334]  suspend_devices_and_enter+0x110/0x500
[   38.122336]  pm_suspend+0x268/0x2c0
[   38.122339]  state_store+0x88/0x110
[   38.122345]  kobj_attr_store+0x14/0x28
[   38.122352]  sysfs_kf_write+0x3c/0x50
[   38.122355]  kernfs_fop_write+0x118/0x1e0
[   38.122360]  __vfs_write+0x30/0x168
[   38.122363]  vfs_write+0xa4/0x1a8
[   38.122366]  ksys_write+0x64/0xe8
[   38.122368]  __arm64_sys_write+0x18/0x20
[   38.122374]  el0_svc_common+0x6c/0x178
[   38.122377]  el0_svc_compat_handler+0x1c/0x28
[   38.122381]  el0_svc_compat+0x8/0x18
[   38.122383] ---[ end trace 24145b7d8545345b ]---
[   38.491552] Disabling non-boot CPUs ...

Let's add a flag for duplicating the state of drm_private_obj and set
the flag from msm/mdp5 driver, so that the problem can be fixed while
other drivers using drm_private_obj stay unaffected.

Signed-off-by: Shawn Guo 
---
Changes for v2:
 - Add a flag to duplicate the state of drm_private_obj conditionally,
   so that other drivers using drm_private_obj stay unaffected.

 drivers/gpu/drm/drm_atomic_helper.c  | 22 ++
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c |  7 +++
 include/drm/drm_atomic.h | 11 +++
 3 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 85d163f16801..4bf041730b9e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3140,6 +3140,7 @@ drm_atomic_helper_duplicate_state(struct drm_device *dev,
struct drm_atomic_state *state;
struct drm_connector *conn;
struct drm_connector_list_iter conn_iter;
+   struct drm_private_obj *priv_obj;
struct drm_plane *plane;
struct drm_crtc *crtc;
int err = 0;
@@ -3184,6 +3185,19 @@ drm_atomic_helper_duplicate_state(struct drm_device *dev,
}
drm_connector_list_iter_end(&conn_iter);
 
+   drm_for_each_privobj(priv_obj, dev) {
+   struct drm_private_state *priv_state;
+
+   if (!priv_obj->needs_duplicate_state)
+   continue;
+
+   priv_state = drm_