From: Sean Paul <seanp...@chromium.org>

As Daniel mentions in his email [1], non-blocking commits don't hold the
modeset locks, so we can safely access state as long as these functions
are in the commit path. So remove the WARN_ON in dpu_kms_encoder_enable.

In dpu_crtc_get_intf_mode, things are a bit more complicated. So keep
the WARN_ON, but add a comment explaining the situation and hope someone
comes along and fixes the issue.

[1]- https://lists.freedesktop.org/archives/dri-devel/2019-October/239441.html

Link to v1: 
https://patchwork.freedesktop.org/patch/msgid/20191010151351.126735-1-s...@poorly.run

Changes in v2:
- Restored the WARN_ON in get_intf_mode and added a clarifying comment (Daniel)

Fixes: 1dfdb0e107db ("drm/msm: dpu: Add modeset lock checks where applicable")
Cc: Jeykumar Sankaran <jsa...@codeaurora.org>
Cc: Rob Clark <robdcl...@chromium.org>
Suggested-by: Daniel Vetter <dan...@ffwll.ch>
Partially-Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Signed-off-by: Sean Paul <seanp...@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 +++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 1 -
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 0b9dc042d2e22..f197dce545761 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -271,6 +271,15 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct drm_crtc 
*crtc)
                return INTF_MODE_NONE;
        }
 
+       /*
+        * TODO: This function is called from dpu debugfs and as part of atomic
+        * check. When called from debugfs, the crtc->mutex must be held to
+        * read crtc->state. However reading crtc->state from atomic check isn't
+        * allowed (unless you have a good reason, a big comment, and a deep
+        * understanding of how the atomic/modeset locks work (<- and this is
+        * probably not possible)). So we'll keep the WARN_ON here for now, but
+        * really we need to figure out a better way to track our operating mode
+        */
        WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
 
        /* TODO: Returns the first INTF_MODE, could there be multiple values? */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index b1645ad83a1e1..6c92f0fbeac98 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -316,7 +316,6 @@ void dpu_kms_encoder_enable(struct drm_encoder *encoder)
        if (funcs && funcs->commit)
                funcs->commit(encoder);
 
-       WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
        drm_for_each_crtc(crtc, dev) {
                if (!(crtc->state->encoder_mask & drm_encoder_mask(encoder)))
                        continue;
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Reply via email to