| Message ID | 1398241805-24564-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <treding@nvidia.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > --- > 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 *plane) > return; > > ret = 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 = 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 = plane->fb; > - plane->funcs->disable_plane(plane); > - plane->crtc = NULL; > - plane->fb = NULL; > + ret = plane->funcs->disable_plane(plane); > + if (!ret) { > + plane->crtc = NULL; > + plane->fb = NULL; > + } else { > + old_fb = NULL; > + } > drm_modeset_unlock_all(dev); > goto out; > } > -- > 1.9.2 >
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 *plane) return; ret = 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 = 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 = plane->fb; - plane->funcs->disable_plane(plane); - plane->crtc = NULL; - plane->fb = NULL; + ret = plane->funcs->disable_plane(plane); + if (!ret) { + plane->crtc = NULL; + plane->fb = NULL; + } else { + old_fb = NULL; + } drm_modeset_unlock_all(dev); goto out; }
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 <treding@nvidia.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_crtc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)