Re: [PATCH] drm/crtc-helper: Fixup error handling in drm_helper_crtc_mode_set

On Thu, Jul 2, 2015 at 9:16 PM, Daniel Vetter daniel.vetter@ffwll.ch wrote:

In

commit 9f658b7b62e7aefc1ee067136126eca3f58cabfd Author: Daniel Stone daniels@collabora.com Date: Fri May 22 13:34:45 2015 +0100

drm/crtc_helper: Replace open-coded CRTC state helpers

error handling code was broken, resulting in the first path not being checked correctly. Fix this by using the same pattern as in the transitional plane helper function drm_plane_helper_update.

v2: Simplify the cleanup code while at it too.

v3: After some debugging with John we realized that the above patch from Daniel also accidentally removed the if (crtc_state) check. This is legal when transitioning to atomic, when the initial state reset isn't all wired up yet properly. Reinstate that check to fix the bug John has hit.

Cc: Daniel Stone daniels@collabora.com CC: Sean Paul seanpaul@chromium.org Cc: John Hunter zhaojunwang@pku.edu.cn Reported-by: John Hunter zhaojunwang@pku.edu.cn Signed-off-by: Daniel Vetter daniel.vetter@intel.com

Tested-by: Zhao Junwang zhjwpku@gmail.com

---

drivers/gpu/drm/drm_crtc_helper.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 393114df88a3..93104f3555f5 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -927,15 +927,13 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mod

    if (crtc->funcs->atomic_duplicate_state)
            crtc_state = crtc->funcs->atomic_duplicate_state(crtc);

•   else {
  

•   else if (crtc->state)
  

•           crtc_state = drm_atomic_helper_crtc_duplicate_state(crtc);
  

•   else
            crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
  

•           if (!crtc_state)
  

•                   return -ENOMEM;
  

•           if (crtc->state)
  

•                   __drm_atomic_helper_crtc_duplicate_state(crtc,
  

crtc_state);

•           else
  

•                   crtc_state->crtc = crtc;
  

•   }
  

•   if (!crtc_state)
  

•           return -ENOMEM;
  
    crtc_state->planes_changed = true;
    crtc_state->mode_changed = true;
  

@@ -957,11 +955,11 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mod ret = drm_helper_crtc_mode_set_base(crtc, x, y, old_fb);

out:

•   if (crtc->funcs->atomic_destroy_state)
  

•           crtc->funcs->atomic_destroy_state(crtc, crtc_state);
  

•   else {
  

•           __drm_atomic_helper_crtc_destroy_state(crtc, crtc_state);
  

•           kfree(crtc_state);
  

•   if (crtc_state) {
  

•           if (crtc->funcs->atomic_destroy_state)
  

•                   crtc->funcs->atomic_destroy_state(crtc,
  

crtc_state);

•           else
  

•                   drm_atomic_helper_crtc_destroy_state(crtc,
  

crtc_state); }

    return ret;

-- 2.1.4

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel