Re: [PATCH 1/2] drm/i915: Use normal fb deref for the fbcon framebuffer

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Mon, 10 Feb 2014 18:00:38 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Now that it's a normally kmalloce buffer we can use the usual cleanup
> paths. The upside here is that if we get the refcounting wrong will be
> able to catch it, since the drm core will complain about leftover
> framebuffers and kref about underflows.
> 
> v2: Kill intel_framebuffer_fini - no longer needed now that we
> refcount all fbs properly and only confusing.
> 
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++--------
>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>  drivers/gpu/drm/i915/intel_fbdev.c   |  4 +---
>  3 files changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1b2faa44764b..6600931f213c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10526,18 +10526,13 @@ static void intel_setup_outputs(struct drm_device *dev)
>  	drm_helper_move_panel_connectors_to_head(dev);
>  }
>  
> -void intel_framebuffer_fini(struct intel_framebuffer *fb)
> -{
> -	drm_framebuffer_cleanup(&fb->base);
> -	WARN_ON(!fb->obj->framebuffer_references--);
> -	drm_gem_object_unreference_unlocked(&fb->obj->base);
> -}
> -
>  static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
>  {
>  	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
>  
> -	intel_framebuffer_fini(intel_fb);
> +	drm_framebuffer_cleanup(fb);
> +	WARN_ON(!intel_fb->obj->framebuffer_references--);
> +	drm_gem_object_unreference_unlocked(&intel_fb->obj->base);
>  	kfree(intel_fb);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4386faf34a9b..59348a4d0238 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -685,7 +685,6 @@ int intel_framebuffer_init(struct drm_device *dev,
>  			   struct intel_framebuffer *ifb,
>  			   struct drm_mode_fb_cmd2 *mode_cmd,
>  			   struct drm_i915_gem_object *obj);
> -void intel_framebuffer_fini(struct intel_framebuffer *fb);
>  void intel_prepare_page_flip(struct drm_device *dev, int plane);
>  void intel_finish_page_flip(struct drm_device *dev, int pipe);
>  void intel_finish_page_flip_plane(struct drm_device *dev, int plane);
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index cd969c3c301e..e4f45293ccf5 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -268,9 +268,7 @@ static void intel_fbdev_destroy(struct drm_device *dev,
>  
>  	drm_fb_helper_fini(&ifbdev->helper);
>  
> -	drm_framebuffer_unregister_private(&ifbdev->fb->base);
> -	intel_framebuffer_fini(ifbdev->fb);
> -	kfree(ifbdev->fb);
> +	drm_framebuffer_unreference(&ifbdev->fb->base);
>  }
>  
>  int intel_fbdev_init(struct drm_device *dev)

Yeah, this looks correct, and gets rid of one of the functions in this
maze of fb handling...

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center