Re: [PATCH 2/2] drm: Handle ->disable_plane failures correctly

[Matt Roper <matthew.d.roper@intel.com>]

On Wed, Apr 23, 2014 at 10:30:05AM +0200, Daniel Vetter wrote:
> The ->disable_plane hook always had a return value, but only since the
> introduction of primary planes was there any implementation that
> actually failed.
> 
> So handle such failures correctly.
> 
> Note that drm_plane_force_disable is special: In the modeset cleanup
> case we first disable all crtc, so primary planes should all be freed
> already. And in the fb helper we only reset non-primary planes. Still
> better be paranoid and add an early return.
> 
> I don't see how this could happen, but it might fix the fb refcount
> underrun Thierry is seeing. Matt Roper spotted this issue.
> 
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  drivers/gpu/drm/drm_crtc.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f6633cb927bc..461d19bd14ee 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1152,8 +1152,10 @@ void drm_plane_force_disable(struct drm_plane *plane)
>  		return;
>  
>  	ret = plane->funcs->disable_plane(plane);
> -	if (ret)
> +	if (ret) {
>  		DRM_ERROR("failed to disable plane with busy fb\n");
> +		return;
> +	}
>  	/* disconnect the plane from the fb and crtc: */
>  	__drm_framebuffer_unreference(old_fb);
>  	plane->fb = NULL;
> @@ -2117,9 +2119,13 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
>  	if (!plane_req->fb_id) {
>  		drm_modeset_lock_all(dev);
>  		old_fb = plane->fb;
> -		plane->funcs->disable_plane(plane);
> -		plane->crtc = NULL;
> -		plane->fb = NULL;
> +		ret = plane->funcs->disable_plane(plane);
> +		if (!ret) {
> +			plane->crtc = NULL;
> +			plane->fb = NULL;
> +		} else {
> +			old_fb = NULL;
> +		}
>  		drm_modeset_unlock_all(dev);
>  		goto out;
>  	}
> -- 
> 1.9.2
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795