From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: Possible fb ref count issue with drm_plane_force_disable() Date: Tue, 15 Apr 2014 16:30:21 +0300 Message-ID: <534D346D.4000001@ti.com> References: <534684E8.9000203@ti.com> <20140411115054.GC18465@intel.com> <534B9FC0.2020700@ti.com> <534CF904.1070105@ti.com> <534D09F9.2030608@samsung.com> <534D0D8D.3070100@ti.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0369675536==" Return-path: Received: from arroyo.ext.ti.com (arroyo.ext.ti.com [192.94.94.40]) by gabe.freedesktop.org (Postfix) with ESMTP id 7A1746E217 for ; Tue, 15 Apr 2014 06:30:25 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Rob Clark Cc: Andrzej Hajda , dri-devel List-Id: dri-devel@lists.freedesktop.org --===============0369675536== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="12EC5TqfPu3a0EICdkMDbmpg8Sic0pdmS" --12EC5TqfPu3a0EICdkMDbmpg8Sic0pdmS Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 15/04/14 15:24, Rob Clark wrote: > probably split out omap_plane_mode_set_internal(), call that directly > from update_plane() for plane operations. And then do the refcnt > dance in the new omap_plane_mode_set() which calls _internal().. We don't actually need that. This did the trick: diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index df1725247cca..3cf31ee59aac 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -225,6 +225,11 @@ int omap_plane_mode_set(struct drm_plane *plane, omap_plane->apply_done_cb.arg =3D arg; } + if (plane->fb) + drm_framebuffer_unreference(plane->fb); + + drm_framebuffer_reference(fb); + plane->fb =3D fb; plane->crtc =3D crtc; @@ -241,11 +246,6 @@ static int omap_plane_update(struct drm_plane *plane= , struct omap_plane *omap_plane =3D to_omap_plane(plane); omap_plane->enabled =3D true; - if (plane->fb) - drm_framebuffer_unreference(plane->fb); - - drm_framebuffer_reference(fb); - /* omap_plane_mode_set() takes adjusted src */ switch (omap_plane->win.rotation & 0xf) { case BIT(DRM_ROTATE_90): With quick tests, works fine so far. Still I have to say that the ref counting doesn't feel quite right (or I'm missing something). As far as I understand, the drm_mode_setplane() gets a ref to the fb, and stores it in plane->fb. Why do we take a new ref in omap_plane_update (or with the patch above, in omap_plane_mode_set), and also store it in plane->fb there? Feels like the same task is done in two places. It looks to me like drm_mode_setplane() doesn't really presume that the update_plane would set plane->fb or plane->crtc, as it does that itself (and only if update_plane has succeeded). Tomi --12EC5TqfPu3a0EICdkMDbmpg8Sic0pdmS Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) iQIcBAEBAgAGBQJTTTRtAAoJEPo9qoy8lh71tdcP/ieOs7yXFg+nts5hnADJHzqN cBErk7YYGYvMFTYdpdMVe9b06tWbh1wgzJUkpn4dZ04svaUNkdpfHSvwL9TQI0JH Aa2Xi697SBcorikAQ9y0FryZys27jFT6WgWm333PHSz/TBUMNOqBiGxwvU2BlURy M/I4dsiyhHoSwrFeI+1a4PsEFIo+Sdp/uZtK+PQOEviQ5tcTQipRgHUpgE6jTboo 97jC4F61YW6pSeu+q4uuxzaI/L7XS5DOsTybw0nDzfev7CpDphPc/qavEIArj1Ap AtuaYQiz5vjSzzq74h7zDNIl1ac/ZfwE8vS1QLn/HynwpcPpz16UxkGfo45D1Dtk s3X8dTrG9lHGeiyXeLyUWzJ9+vImmYZz5uR+jtxE/pabtBTRXJPZBmS0/Xd5uQaI IZFVrhBkAxr5ONOJoj89ZQZSaE2HvX7w37a3o1F3+Or1HBpxwaCTDESdn4h2+hMN AY/lvzbHw/dD2mrO7vHDLjltiFtWe9g+e+vEDt5xLz2M6StW9MZ7wLu143fQ7UlK Fgj/Pv9dZEljkHitr9bRPGlU1tDn6zWlf7Nyj0BRu3WycadqI1jIUAXLBVDDxVHL gXOob+rdPDOdBXt/XpywYXx6JfNjl2iSE4R4jkLiWyTQB0tQGTV39D8qAro0Otdx /x7sL1UYkUs1uw3s+amH =qJ7p -----END PGP SIGNATURE----- --12EC5TqfPu3a0EICdkMDbmpg8Sic0pdmS-- --===============0369675536== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============0369675536==--