drm: Move ->old_fb from crtc to plane
authorDaniel Vetter <daniel.vetter@ffwll.ch>
Sun, 27 Jul 2014 11:42:42 +0000 (13:42 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Fri, 8 Aug 2014 15:46:52 +0000 (17:46 +0200)
Atomic implemenations for legacy ioctls must be able to drop locks.
Which doesn't cause havoc since we only do that while constructing
the new state, so no driver or hardware state change has happened.

The only troubling bit is the fb refcounting the core does - if
someone else has snuck in then it might potentially unref an
outdated framebuffer. To fix that move the old_fb temporary storage
into struct drm_plane for all ioctls, so that the atomic helpers can
update it.

v2: Fix up the error case handling as suggested by Matt Roper and just
grab locks uncoditionally - there's no point in optimizing the locking
for when userspace gets it wrong.

Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/drm_crtc.c
include/drm/drm_crtc.h

index ab121b6d980c84a6a92acb08518627ab2bf07559..cacb460a714570b4de225152e84c59ccab30ed0c 100644 (file)
@@ -1287,19 +1287,21 @@ EXPORT_SYMBOL(drm_plane_index);
  */
 void drm_plane_force_disable(struct drm_plane *plane)
 {
-       struct drm_framebuffer *old_fb = plane->fb;
        int ret;
 
-       if (!old_fb)
+       if (!plane->fb)
                return;
 
+       plane->old_fb = plane->fb;
        ret = plane->funcs->disable_plane(plane);
        if (ret) {
                DRM_ERROR("failed to disable plane with busy fb\n");
+               plane->old_fb = NULL;
                return;
        }
        /* disconnect the plane from the fb and crtc: */
-       __drm_framebuffer_unreference(old_fb);
+       __drm_framebuffer_unreference(plane->old_fb);
+       plane->old_fb = NULL;
        plane->fb = NULL;
        plane->crtc = NULL;
 }
@@ -2275,23 +2277,21 @@ static int setplane_internal(struct drm_plane *plane,
                             uint32_t src_w, uint32_t src_h)
 {
        struct drm_device *dev = plane->dev;
-       struct drm_framebuffer *old_fb = NULL;
        int ret = 0;
        unsigned int fb_width, fb_height;
        int i;
 
+       drm_modeset_lock_all(dev);
        /* No fb means shut it down */
        if (!fb) {
-               drm_modeset_lock_all(dev);
-               old_fb = plane->fb;
+               plane->old_fb = plane->fb;
                ret = plane->funcs->disable_plane(plane);
                if (!ret) {
                        plane->crtc = NULL;
                        plane->fb = NULL;
                } else {
-                       old_fb = NULL;
+                       plane->old_fb = NULL;
                }
-               drm_modeset_unlock_all(dev);
                goto out;
        }
 
@@ -2331,8 +2331,7 @@ static int setplane_internal(struct drm_plane *plane,
                goto out;
        }
 
-       drm_modeset_lock_all(dev);
-       old_fb = plane->fb;
+       plane->old_fb = plane->fb;
        ret = plane->funcs->update_plane(plane, crtc, fb,
                                         crtc_x, crtc_y, crtc_w, crtc_h,
                                         src_x, src_y, src_w, src_h);
@@ -2341,15 +2340,16 @@ static int setplane_internal(struct drm_plane *plane,
                plane->fb = fb;
                fb = NULL;
        } else {
-               old_fb = NULL;
+               plane->old_fb = NULL;
        }
-       drm_modeset_unlock_all(dev);
 
 out:
        if (fb)
                drm_framebuffer_unreference(fb);
-       if (old_fb)
-               drm_framebuffer_unreference(old_fb);
+       if (plane->old_fb)
+               drm_framebuffer_unreference(plane->old_fb);
+       plane->old_fb = NULL;
+       drm_modeset_unlock_all(dev);
 
        return ret;
 
@@ -2456,7 +2456,7 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
         * crtcs. Atomic modeset will have saner semantics ...
         */
        list_for_each_entry(tmp, &crtc->dev->mode_config.crtc_list, head)
-               tmp->old_fb = tmp->primary->fb;
+               tmp->primary->old_fb = tmp->primary->fb;
 
        fb = set->fb;
 
@@ -2469,8 +2469,9 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
        list_for_each_entry(tmp, &crtc->dev->mode_config.crtc_list, head) {
                if (tmp->primary->fb)
                        drm_framebuffer_reference(tmp->primary->fb);
-               if (tmp->old_fb)
-                       drm_framebuffer_unreference(tmp->old_fb);
+               if (tmp->primary->old_fb)
+                       drm_framebuffer_unreference(tmp->primary->old_fb);
+               tmp->primary->old_fb = NULL;
        }
 
        return ret;
@@ -4545,7 +4546,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 {
        struct drm_mode_crtc_page_flip *page_flip = data;
        struct drm_crtc *crtc;
-       struct drm_framebuffer *fb = NULL, *old_fb = NULL;
+       struct drm_framebuffer *fb = NULL;
        struct drm_pending_vblank_event *e = NULL;
        unsigned long flags;
        int ret = -EINVAL;
@@ -4617,7 +4618,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
                        (void (*) (struct drm_pending_event *)) kfree;
        }
 
-       old_fb = crtc->primary->fb;
+       crtc->primary->old_fb = crtc->primary->fb;
        ret = crtc->funcs->page_flip(crtc, fb, e, page_flip->flags);
        if (ret) {
                if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
@@ -4627,7 +4628,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
                        kfree(e);
                }
                /* Keep the old fb, don't unref it. */
-               old_fb = NULL;
+               crtc->primary->old_fb = NULL;
        } else {
                /*
                 * Warn if the driver hasn't properly updated the crtc->fb
@@ -4643,8 +4644,9 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 out:
        if (fb)
                drm_framebuffer_unreference(fb);
-       if (old_fb)
-               drm_framebuffer_unreference(old_fb);
+       if (crtc->primary->old_fb)
+               drm_framebuffer_unreference(crtc->primary->old_fb);
+       crtc->primary->old_fb = NULL;
        drm_modeset_unlock_crtc(crtc);
 
        return ret;
index 508817bae53881059ef478f641167e6c3fcab950..279565aa0c33e4fb589c94266baf25dd1b731acc 100644 (file)
@@ -341,10 +341,6 @@ struct drm_crtc {
        int cursor_x;
        int cursor_y;
 
-       /* Temporary tracking of the old fb while a modeset is ongoing. Used
-        * by drm_mode_set_config_internal to implement correct refcounting. */
-       struct drm_framebuffer *old_fb;
-
        bool enabled;
 
        /* Requested mode from modesetting. */
@@ -623,6 +619,10 @@ struct drm_plane {
        struct drm_crtc *crtc;
        struct drm_framebuffer *fb;
 
+       /* Temporary tracking of the old fb while a modeset is ongoing. Used
+        * by drm_mode_set_config_internal to implement correct refcounting. */
+       struct drm_framebuffer *old_fb;
+
        const struct drm_plane_funcs *funcs;
 
        struct drm_object_properties properties;