Re: [PATCH] drm/i915: release cursor when crtc is destroyed

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Apr 23, 2013 at 05:27:08PM +0300, Mika Kuoppala wrote:
> crtc is holding a reference to a cursor bo and it needs
> to be released when crtc is destroyed so that we don't leak
> the cursor bo.
> 
> v2: Enhance set and move cursor so that disabled
> cursor is handled correctly (Ville Syrjälä)
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Oh, nice catch!

Could we somehow test this in an igt? I'm thinking of the following
sequence:
- Check how many objects there are in debugfs (maybe that needs a slightly
  saner interface than what we currently have in i915_gem_objects).
- Setup a mode and provoke the leak (we could augment the tests with
  sprites and similar stuff).
- Check whether the object count dropped back to the old value or not. If
  not, fail the test.

We need to check the object count both before&afterwards to account for
pinned kernel objects (which might chance depending upon kernel version
and similar things).

Cheers, Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 74156e2..f5cdd91 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6554,7 +6554,7 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
>  	intel_crtc->cursor_width = width;
>  	intel_crtc->cursor_height = height;
>  
> -	intel_crtc_update_cursor(crtc, true);
> +	intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
>  
>  	return 0;
>  fail_unpin:
> @@ -6573,7 +6573,7 @@ static int intel_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
>  	intel_crtc->cursor_x = x;
>  	intel_crtc->cursor_y = y;
>  
> -	intel_crtc_update_cursor(crtc, true);
> +	intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
>  
>  	return 0;
>  }
> @@ -7087,6 +7087,8 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
>  		kfree(work);
>  	}
>  
> +	intel_crtc_cursor_set(crtc, NULL, 0, 0, 0);
> +
>  	drm_crtc_cleanup(crtc);
>  
>  	kfree(intel_crtc);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch