From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: release cursor when crtc is destroyed Date: Tue, 23 Apr 2013 16:56:09 +0200 Message-ID: <20130423145609.GI6169@phenom.ffwll.local> References: <1366727228-6207-1-git-send-email-mika.kuoppala@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ea0-f170.google.com (mail-ea0-f170.google.com [209.85.215.170]) by gabe.freedesktop.org (Postfix) with ESMTP id 9E9C6E5C75 for ; Tue, 23 Apr 2013 07:53:07 -0700 (PDT) Received: by mail-ea0-f170.google.com with SMTP id z7so296250eaf.1 for ; Tue, 23 Apr 2013 07:53:06 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1366727228-6207-1-git-send-email-mika.kuoppala@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Mika Kuoppala Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org 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=E4l=E4) > = > Signed-off-by: Mika Kuoppala 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 *c= rtc, > intel_crtc->cursor_width =3D width; > intel_crtc->cursor_height =3D height; > = > - intel_crtc_update_cursor(crtc, true); > + intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo !=3D 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 =3D x; > intel_crtc->cursor_y =3D y; > = > - intel_crtc_update_cursor(crtc, true); > + intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo !=3D NULL); > = > return 0; > } > @@ -7087,6 +7087,8 @@ static void intel_crtc_destroy(struct drm_crtc *crt= c) > 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