Doing this within the fb->destroy callback leads to a locking nightmare. And all other drm drivers that restore the fbcon do it in lastclose, too.
With this adjustments all fb->destroy callbacks optionally drop references to any gem objects used as backing storage, call drm_framebuffer_cleanup and then kfree the struct. Which nicely simplifies the locking for framebuffer unreferencing and freeing, since this doesn't require that we hold the mode_config lock. A slight exception is the vmwgfx surface backed framebuffer, it also calls drm_master_put and removes the object from a device-private framebuffer list. Both seem to have solid locking in place already.
Conclusion is that now it is no longer required to hold the mode_config lock while freeing a framebuffer.
v2: Drop the corresponding mutex_lock WARN check from drm_framebuffer_unreference.
v3: Use just the mode_config lock not modeset_lock_all, due to patch reordering.
Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_crtc.c | 2 -- drivers/gpu/drm/gma500/framebuffer.c | 24 ------------------------ drivers/gpu/drm/gma500/psb_drv.c | 10 ++++++++++ 3 files changed, 10 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 8154554..8d665fa 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -317,9 +317,7 @@ static void drm_framebuffer_free(struct kref *kref) */ void drm_framebuffer_unreference(struct drm_framebuffer *fb) { - struct drm_device *dev = fb->dev; DRM_DEBUG("FB ID: %d\n", fb->base.id); - WARN_ON(!drm_modeset_is_locked(dev)); kref_put(&fb->refcount, drm_framebuffer_free); } EXPORT_SYMBOL(drm_framebuffer_unreference); diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c index 38e7e75..49800d2 100644 --- a/drivers/gpu/drm/gma500/framebuffer.c +++ b/drivers/gpu/drm/gma500/framebuffer.c @@ -668,30 +668,6 @@ static void psb_user_framebuffer_destroy(struct drm_framebuffer *fb) { struct psb_framebuffer *psbfb = to_psb_fb(fb); struct gtt_range *r = psbfb->gtt; - struct drm_device *dev = fb->dev; - struct drm_psb_private *dev_priv = dev->dev_private; - struct psb_fbdev *fbdev = dev_priv->fbdev; - struct drm_crtc *crtc; - int reset = 0; - - /* Should never get stolen memory for a user fb */ - WARN_ON(r->stolen); - - /* Check if we are erroneously live */ - list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) - if (crtc->fb == fb) - reset = 1; - - if (reset) - /* - * Now force a sane response before we permit the DRM CRTC - * layer to do stupid things like blank the display. Instead - * we reset this framebuffer as if the user had forced a reset. - * We must do this before the cleanup so that the DRM layer - * doesn't get a chance to stick its oar in where it isn't - * wanted. - */ - drm_fb_helper_restore_fbdev_mode(&fbdev->psb_fb_helper);
/* Let DRM do its clean up */ drm_framebuffer_cleanup(fb); diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index dd1fbfa..dbcefe9 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -149,6 +149,16 @@ static struct drm_ioctl_desc psb_ioctls[] = {
static void psb_lastclose(struct drm_device *dev) { + int ret; + struct drm_psb_private *dev_priv = dev->dev_private; + struct psb_fbdev *fbdev = dev_priv->fbdev; + + mutex_lock(&dev->mode_config.mutex); + ret = drm_fb_helper_restore_fbdev_mode(&fbdev->psb_fb_helper); + if (ret) + DRM_DEBUG("failed to restore crtc mode\n"); + mutex_unlock(&dev->mode_config.mutex); + return; }