[RFC,1/4,v2] drm_hwcomposer: Make sure we set the active state when doing modesets

Message ID 1516749399-29504-2-git-send-email-john.stultz@linaro.org
State New
Headers show
Series
  • drm_hwcomposer: Changes to support HiKey/HiKey960
Related show

Commit Message

John Stultz Jan. 23, 2018, 11:16 p.m.
In trying to use drm_hwcomposer with HiKey/HiKey960 boards, I
found that the crtc wouldn't intitalize and the atomic commit
calls were failing.

I initially chased this down to following check in the kernel
drm_atomic_crtc_check() function failing:

 if (state->event && !state->active && !crtc->state->active) {
     DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requesting event but off\n",
                      crtc->base.id, crtc->name);
     return -EINVAL;
 }

Where because a fence was submitted state->event was set, but
the crtc state was not active. This results in the atomic commit
to fail and no mode to be set.

After hacking around this in the kernel, Sean Paul helped me
understand that it was the kernel complaining about the crtc
state being provided in the atomic commit which did not have the
active flag set.

Thus, the proper fix is to make sure when we do the modesetting
that we also set the crtc state active flag in property set.

With this change, the kernel no longer rejects the atomic commit
and the crtc initializes properly.

Cc: Marissa Wall <marissaw@google.com>
Cc: Sean Paul <seanpaul@google.com>
Cc: Dmitry Shmidt <dimitrysh@google.com>
Cc: Robert Foss <robert.foss@collabora.com>
Cc: Matt Szczesiak <matt.szczesiak@arm.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: David Hanna <david.hanna11@gmail.com>
Cc: Rob Herring <rob.herring@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>

---
 drmdisplaycompositor.cpp | 7 +++++++
 1 file changed, 7 insertions(+)

-- 
2.7.4

Comments

Sean Paul Jan. 24, 2018, 3:28 p.m. | #1
On Tue, Jan 23, 2018 at 03:16:36PM -0800, John Stultz wrote:
> In trying to use drm_hwcomposer with HiKey/HiKey960 boards, I
> found that the crtc wouldn't intitalize and the atomic commit
> calls were failing.
> 
> I initially chased this down to following check in the kernel
> drm_atomic_crtc_check() function failing:
> 
>  if (state->event && !state->active && !crtc->state->active) {
>      DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requesting event but off\n",
>                       crtc->base.id, crtc->name);
>      return -EINVAL;
>  }
> 
> Where because a fence was submitted state->event was set, but
> the crtc state was not active. This results in the atomic commit
> to fail and no mode to be set.
> 
> After hacking around this in the kernel, Sean Paul helped me
> understand that it was the kernel complaining about the crtc
> state being provided in the atomic commit which did not have the
> active flag set.
> 
> Thus, the proper fix is to make sure when we do the modesetting
> that we also set the crtc state active flag in property set.
> 
> With this change, the kernel no longer rejects the atomic commit
> and the crtc initializes properly.
> 
> Cc: Marissa Wall <marissaw@google.com>
> Cc: Sean Paul <seanpaul@google.com>
> Cc: Dmitry Shmidt <dimitrysh@google.com>
> Cc: Robert Foss <robert.foss@collabora.com>
> Cc: Matt Szczesiak <matt.szczesiak@arm.com>
> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> Cc: David Hanna <david.hanna11@gmail.com>
> Cc: Rob Herring <rob.herring@linaro.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Applied.

Thanks for the patch!

Sean

> ---
>  drmdisplaycompositor.cpp | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index acd13b8..3a20b31 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -520,6 +520,13 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
>    }
>  
>    if (mode_.needs_modeset) {
> +    ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->active_property().id(), 1);
> +    if (ret < 0) {
> +      ALOGE("Failed to add crtc active to pset\n");
> +      drmModeAtomicFree(pset);
> +      return ret;
> +    }
> +
>      ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->mode_property().id(),
>                                     mode_.blob_id) < 0 ||
>            drmModeAtomicAddProperty(pset, connector->id(),
> -- 
> 2.7.4
>

Patch

diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
index acd13b8..3a20b31 100644
--- a/drmdisplaycompositor.cpp
+++ b/drmdisplaycompositor.cpp
@@ -520,6 +520,13 @@  int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
   }
 
   if (mode_.needs_modeset) {
+    ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->active_property().id(), 1);
+    if (ret < 0) {
+      ALOGE("Failed to add crtc active to pset\n");
+      drmModeAtomicFree(pset);
+      return ret;
+    }
+
     ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->mode_property().id(),
                                    mode_.blob_id) < 0 ||
           drmModeAtomicAddProperty(pset, connector->id(),