Re: [PATCH] drm/i915/fbdev: Call intel_unpin_fb_obj() on release

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 22/04/16 08:47, Chris Wilson wrote:
> When releasing the intel_fbdev, we should unpin the framebuffer that we
> pinned during construction.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> I'm hoping this explains some of the flip-flip in DMESG-WARN for
> drv_module_reload_basic...
> -Chris
> ---
>   drivers/gpu/drm/i915/intel_display.c | 2 +-
>   drivers/gpu/drm/i915/intel_drv.h     | 1 +
>   drivers/gpu/drm/i915/intel_fbdev.c   | 7 ++++++-
>   3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ff60241b1f76..a59fe7b032b0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2309,7 +2309,7 @@ err_pm:
>   	return ret;
>   }
>
> -static void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
> +void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
>   {
>   	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>   	struct i915_ggtt_view view;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index beed9e81252b..e336b19adb37 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1161,6 +1161,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
>   				    struct drm_modeset_acquire_ctx *ctx);
>   int intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
>   			       unsigned int rotation);
> +void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
>   struct drm_framebuffer *
>   __intel_framebuffer_create(struct drm_device *dev,
>   			   struct drm_mode_fb_cmd2 *mode_cmd,
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 79ac202f3870..c3c265d6f521 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -287,7 +287,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
>   out_destroy_fbi:
>   	drm_fb_helper_release_fbi(helper);
>   out_unpin:
> -	i915_gem_object_ggtt_unpin(obj);
> +	intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0));
>   out_unlock:
>   	mutex_unlock(&dev->struct_mutex);
>   	return ret;
> @@ -551,6 +551,11 @@ static void intel_fbdev_destroy(struct drm_device *dev,
>
>   	if (ifbdev->fb) {
>   		drm_framebuffer_unregister_private(&ifbdev->fb->base);
> +
> +		mutex_lock(&dev->struct_mutex);
> +		intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0));
> +		mutex_unlock(&dev->struct_mutex);
> +
>   		drm_framebuffer_remove(&ifbdev->fb->base);
>   	}
>   }
>

What is new to need this? I looked at the related code, and maybe got 
the wrong conclusions, but it looks like that warning in drm_mm_takedown 
would fire all the time if this drm_framebuffer_remove was not the last 
reference and wouldn't do the unpinning?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx