2022-07-07 16:38:48

by Rob Clark

[permalink] [raw]
Subject: [PATCH] drm/msm/mdp5: Fix global state lock backoff

From: Rob Clark <[email protected]>

We need to grab the lock after the early return for !hwpipe case.
Otherwise, we could have hit contention yet still returned 0.

Fixes an issue that the new CONFIG_DRM_DEBUG_MODESET_LOCK stuff flagged
in CI:

WARNING: CPU: 0 PID: 282 at drivers/gpu/drm/drm_modeset_lock.c:296 drm_modeset_lock+0xf8/0x154
Modules linked in:
CPU: 0 PID: 282 Comm: kms_cursor_lega Tainted: G W 5.19.0-rc2-15930-g875cc8bc536a #1
Hardware name: Qualcomm Technologies, Inc. DB820c (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : drm_modeset_lock+0xf8/0x154
lr : drm_atomic_get_private_obj_state+0x84/0x170
sp : ffff80000cfab6a0
x29: ffff80000cfab6a0 x28: 0000000000000000 x27: ffff000083bc4d00
x26: 0000000000000038 x25: 0000000000000000 x24: ffff80000957ca58
x23: 0000000000000000 x22: ffff000081ace080 x21: 0000000000000001
x20: ffff000081acec18 x19: ffff80000cfabb80 x18: 0000000000000038
x17: 0000000000000000 x16: 0000000000000000 x15: fffffffffffea0d0
x14: 0000000000000000 x13: 284e4f5f4e524157 x12: 5f534b434f4c5f47
x11: ffff80000a386aa8 x10: 0000000000000029 x9 : ffff80000cfab610
x8 : 0000000000000029 x7 : 0000000000000014 x6 : 0000000000000000
x5 : 0000000000000001 x4 : ffff8000081ad904 x3 : 0000000000000029
x2 : ffff0000801db4c0 x1 : ffff80000cfabb80 x0 : ffff000081aceb58
Call trace:
drm_modeset_lock+0xf8/0x154
drm_atomic_get_private_obj_state+0x84/0x170
mdp5_get_global_state+0x54/0x6c
mdp5_pipe_release+0x2c/0xd4
mdp5_plane_atomic_check+0x2ec/0x414
drm_atomic_helper_check_planes+0xd8/0x210
drm_atomic_helper_check+0x54/0xb0
...
---[ end trace 0000000000000000 ]---
drm_modeset_lock attempting to lock a contended lock without backoff:
drm_modeset_lock+0x148/0x154
mdp5_get_global_state+0x30/0x6c
mdp5_pipe_release+0x2c/0xd4
mdp5_plane_atomic_check+0x290/0x414
drm_atomic_helper_check_planes+0xd8/0x210
drm_atomic_helper_check+0x54/0xb0
drm_atomic_check_only+0x4b0/0x8f4
drm_atomic_commit+0x68/0xe0

Fixes: d59be579fa93 ("drm/msm/mdp5: Return error code in mdp5_pipe_release when deadlock is detected")
Signed-off-by: Rob Clark <[email protected]>
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
index a4f5cb90f3e8..e4b8a789835a 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
@@ -123,12 +123,13 @@ int mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe)
{
struct msm_drm_private *priv = s->dev->dev_private;
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
- struct mdp5_global_state *state = mdp5_get_global_state(s);
+ struct mdp5_global_state *state;
struct mdp5_hw_pipe_state *new_state;

if (!hwpipe)
return 0;

+ state = mdp5_get_global_state(s);
if (IS_ERR(state))
return PTR_ERR(state);

--
2.36.1


2022-07-07 17:22:51

by Abhinav Kumar

[permalink] [raw]
Subject: Re: [PATCH] drm/msm/mdp5: Fix global state lock backoff



On 7/7/2022 9:20 AM, Rob Clark wrote:
> From: Rob Clark <[email protected]>
>
> We need to grab the lock after the early return for !hwpipe case.
> Otherwise, we could have hit contention yet still returned 0.
>
> Fixes an issue that the new CONFIG_DRM_DEBUG_MODESET_LOCK stuff flagged
> in CI:
>
> WARNING: CPU: 0 PID: 282 at drivers/gpu/drm/drm_modeset_lock.c:296 drm_modeset_lock+0xf8/0x154
> Modules linked in:
> CPU: 0 PID: 282 Comm: kms_cursor_lega Tainted: G W 5.19.0-rc2-15930-g875cc8bc536a #1
> Hardware name: Qualcomm Technologies, Inc. DB820c (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : drm_modeset_lock+0xf8/0x154
> lr : drm_atomic_get_private_obj_state+0x84/0x170
> sp : ffff80000cfab6a0
> x29: ffff80000cfab6a0 x28: 0000000000000000 x27: ffff000083bc4d00
> x26: 0000000000000038 x25: 0000000000000000 x24: ffff80000957ca58
> x23: 0000000000000000 x22: ffff000081ace080 x21: 0000000000000001
> x20: ffff000081acec18 x19: ffff80000cfabb80 x18: 0000000000000038
> x17: 0000000000000000 x16: 0000000000000000 x15: fffffffffffea0d0
> x14: 0000000000000000 x13: 284e4f5f4e524157 x12: 5f534b434f4c5f47
> x11: ffff80000a386aa8 x10: 0000000000000029 x9 : ffff80000cfab610
> x8 : 0000000000000029 x7 : 0000000000000014 x6 : 0000000000000000
> x5 : 0000000000000001 x4 : ffff8000081ad904 x3 : 0000000000000029
> x2 : ffff0000801db4c0 x1 : ffff80000cfabb80 x0 : ffff000081aceb58
> Call trace:
> drm_modeset_lock+0xf8/0x154
> drm_atomic_get_private_obj_state+0x84/0x170
> mdp5_get_global_state+0x54/0x6c
> mdp5_pipe_release+0x2c/0xd4
> mdp5_plane_atomic_check+0x2ec/0x414
> drm_atomic_helper_check_planes+0xd8/0x210
> drm_atomic_helper_check+0x54/0xb0
> ...
> ---[ end trace 0000000000000000 ]---
> drm_modeset_lock attempting to lock a contended lock without backoff:
> drm_modeset_lock+0x148/0x154
> mdp5_get_global_state+0x30/0x6c
> mdp5_pipe_release+0x2c/0xd4
> mdp5_plane_atomic_check+0x290/0x414
> drm_atomic_helper_check_planes+0xd8/0x210
> drm_atomic_helper_check+0x54/0xb0
> drm_atomic_check_only+0x4b0/0x8f4
> drm_atomic_commit+0x68/0xe0
>
> Fixes: d59be579fa93 ("drm/msm/mdp5: Return error code in mdp5_pipe_release when deadlock is detected")
> Signed-off-by: Rob Clark <[email protected]>
Reviewed-by: Abhinav Kumar <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
> index a4f5cb90f3e8..e4b8a789835a 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
> @@ -123,12 +123,13 @@ int mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe)
> {
> struct msm_drm_private *priv = s->dev->dev_private;
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> - struct mdp5_global_state *state = mdp5_get_global_state(s);
> + struct mdp5_global_state *state;
> struct mdp5_hw_pipe_state *new_state;
>
> if (!hwpipe)
> return 0;
>
> + state = mdp5_get_global_state(s);
> if (IS_ERR(state))
> return PTR_ERR(state);
>