From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matt Roper Subject: Re: [PATCH 2/2] drm: Handle ->disable_plane failures correctly Date: Wed, 23 Apr 2014 07:47:33 -0700 Message-ID: <20140423144733.GV1063@intel.com> References: <1398241805-24564-1-git-send-email-daniel.vetter@ffwll.ch> <1398241805-24564-2-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id BDC6D6EABC for ; Wed, 23 Apr 2014 07:44:38 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1398241805-24564-2-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: Thierry Reding , DRI Development List-Id: dri-devel@lists.freedesktop.org On Wed, Apr 23, 2014 at 10:30:05AM +0200, Daniel Vetter wrote: > The ->disable_plane hook always had a return value, but only since the > introduction of primary planes was there any implementation that > actually failed. > = > So handle such failures correctly. > = > Note that drm_plane_force_disable is special: In the modeset cleanup > case we first disable all crtc, so primary planes should all be freed > already. And in the fb helper we only reset non-primary planes. Still > better be paranoid and add an early return. > = > I don't see how this could happen, but it might fix the fb refcount > underrun Thierry is seeing. Matt Roper spotted this issue. > = > Cc: Thierry Reding > Cc: Ville Syrj=E4l=E4 > Cc: Matt Roper > Signed-off-by: Daniel Vetter Reviewed-by: Matt Roper > --- > drivers/gpu/drm/drm_crtc.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > = > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index f6633cb927bc..461d19bd14ee 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1152,8 +1152,10 @@ void drm_plane_force_disable(struct drm_plane *pla= ne) > return; > = > ret =3D plane->funcs->disable_plane(plane); > - if (ret) > + if (ret) { > DRM_ERROR("failed to disable plane with busy fb\n"); > + return; > + } > /* disconnect the plane from the fb and crtc: */ > __drm_framebuffer_unreference(old_fb); > plane->fb =3D NULL; > @@ -2117,9 +2119,13 @@ int drm_mode_setplane(struct drm_device *dev, void= *data, > if (!plane_req->fb_id) { > drm_modeset_lock_all(dev); > old_fb =3D plane->fb; > - plane->funcs->disable_plane(plane); > - plane->crtc =3D NULL; > - plane->fb =3D NULL; > + ret =3D plane->funcs->disable_plane(plane); > + if (!ret) { > + plane->crtc =3D NULL; > + plane->fb =3D NULL; > + } else { > + old_fb =3D NULL; > + } > drm_modeset_unlock_all(dev); > goto out; > } > -- = > 1.9.2 > = -- = Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795