Re: [PATCH 1/2] drm/core: Preserve the framebuffer after removing it.

From: Daniel Vetter <daniel@ffwll.ch>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/core: Preserve the framebuffer after removing it.
Date: Wed, 9 Sep 2015 17:02:37 +0200	[thread overview]
Message-ID: <20150909150237.GN2767@phenom.ffwll.local> (raw)
In-Reply-To: <1441809657-11411-2-git-send-email-maarten.lankhorst@linux.intel.com>

On Wed, Sep 09, 2015 at 04:40:56PM +0200, Maarten Lankhorst wrote:
> Previously RMFB and fd close chose to disable any plane that had
> an active framebuffer from this file. If it was a primary plane the
> crtc was disabled. However the fbdev code or any system compositor
> should restore the planes anyway so there's no need to do it twice.
> 
> The old fb_id is zero'd, so there's no danger of being able to
> restore the fb from fb_id.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

I think the current behaviour was just a side-effect of the original
implementation and not too intentional - with no refcounting for fbs they
_had_ to be synchronously reaped. And when I've done the conversion to
refcounting I kept this to avoid gettting tangled up in an ABI-change
mess.

But I don't the current behaviour makes much sense and worth an attemp to
rectify it. And as long as we still clear the fb ids there's no real
information leak either.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

But I do want other people's opinion before I pull this in.
-Daniel

> ---
>  drivers/gpu/drm/drm_crtc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 9b9c4b41422a..626b0a57efbf 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3327,7 +3327,7 @@ int drm_mode_rmfb(struct drm_device *dev,
>  	mutex_unlock(&dev->mode_config.fb_lock);
>  	mutex_unlock(&file_priv->fbs_lock);
>  
> -	drm_framebuffer_remove(fb);
> +	drm_framebuffer_unreference(fb);
>  
>  	return 0;
>  
> @@ -3517,7 +3517,7 @@ void drm_fb_release(struct drm_file *priv)
>  		list_del_init(&fb->filp_head);
>  
>  		/* This will also drop the fpriv->fbs reference. */
> -		drm_framebuffer_remove(fb);
> +		drm_framebuffer_unreference(fb);
>  	}
>  }
>  
> -- 
> 2.1.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel