| Message ID | 1461653672-17335-2-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 26/04/16 07:54, 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> > --- > 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 b7cb632a2a31..87ce7852482b 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 b9f1304439e2..54f7000e2fe9 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 af561542a5a1..1c3ad121f1b9 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); I think I've asked before - why this isn't the last fb reference so the obj unref would do the unpin automatically? Regards, Tvrtko
On Wed, Apr 27, 2016 at 01:20:52PM +0100, Tvrtko Ursulin wrote: > > On 26/04/16 07:54, 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> > >--- > > 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 b7cb632a2a31..87ce7852482b 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 b9f1304439e2..54f7000e2fe9 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 af561542a5a1..1c3ad121f1b9 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); > > I think I've asked before - why this isn't the last fb reference so > the obj unref would do the unpin automatically? And I thought I answered that it was bad practice to leave it to the core as then we can't detect a leak, and that later code I have will complain about the pin_display leak here. With user pinning gone, every pin should now be accountable and anything remaining should be a WARN. -Chris
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b7cb632a2a31..87ce7852482b 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 b9f1304439e2..54f7000e2fe9 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 af561542a5a1..1c3ad121f1b9 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); } }
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> --- 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(-)