drm/crtc-helper: Fixup error handling in drm_helper_crtc_mode_set
diff mbox

Message ID 1435821829-11107-1-git-send-email-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter July 2, 2015, 7:23 a.m. UTC
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.

Cc: Daniel Stone <daniels@collabora.com>
CC: Sean Paul <seanpaul@chromium.org>
Cc: John Hunter <zhaojunwang@pku.edu.cn>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_crtc_helper.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

Comments

Daniel Stone July 2, 2015, 9:01 a.m. UTC | #1
Hi,

On Thu, 2015-07-02 at 09:23 +0200, Daniel Vetter 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.
> 
> @@ -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(crt
> c, crtc_state);
> -		else
> -			crtc_state->crtc = crtc;

Isn't this line (the crtc_state->crtc) assignment now missing from the
kzalloc branch?

With that fixed:
Reviewed-by: Daniel Stone <daniels@collabora.com>

Cheers,
Daniel

Patch
diff mbox

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 393114df88a3..0035c3395b30 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;