[Intel-gfx] [PATCH 3/3] drm: Track framebuffer references at the point of assignment

Chris Wilson chris at chris-wilson.co.uk
Fri Nov 25 15:32:31 UTC 2016


We can simplify the code greatly if both legacy and atomic paths updated
the references as they assigned the framebuffer to the planes. Long
before framebuffer reference counting was added, the code had to keep
the old_fb around until after the operation was completed - but now we
can simply leave it to the reference handling to prevent invalid use.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  2 +-
 drivers/gpu/drm/armada/armada_crtc.c        |  9 +----
 drivers/gpu/drm/bochs/bochs_kms.c           |  2 +-
 drivers/gpu/drm/drm_atomic.c                | 44 +-------------------
 drivers/gpu/drm/drm_atomic_helper.c         | 35 ++--------------
 drivers/gpu/drm/drm_crtc.c                  | 18 +--------
 drivers/gpu/drm/drm_crtc_helper.c           | 18 +++++----
 drivers/gpu/drm/drm_fb_helper.c             | 23 +++++------
 drivers/gpu/drm/drm_plane.c                 | 62 ++++++++++-------------------
 drivers/gpu/drm/i915/intel_display.c        | 17 ++++----
 drivers/gpu/drm/mgag200/mgag200_mode.c      |  2 +-
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c   |  2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |  2 +-
 drivers/gpu/drm/nouveau/nouveau_display.c   |  2 +-
 drivers/gpu/drm/nouveau/nv50_display.c      |  7 ----
 drivers/gpu/drm/qxl/qxl_display.c           |  4 +-
 drivers/gpu/drm/radeon/radeon_display.c     |  3 +-
 drivers/gpu/drm/shmobile/shmob_drm_crtc.c   |  2 +-
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c        |  4 +-
 drivers/gpu/drm/udl/udl_modeset.c           |  2 +-
 drivers/gpu/drm/vc4/vc4_crtc.c              |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c         |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c        | 10 ++---
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c        |  4 +-
 include/drm/drm_atomic.h                    |  3 --
 include/drm/drm_plane.h                     | 27 ++++++++++---
 26 files changed, 100 insertions(+), 210 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 741144fcc7bc..2aa4e707be1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -225,7 +225,7 @@ int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc,
 	DRM_DEBUG_DRIVER("crtc:%d[%p], pflip_stat:AMDGPU_FLIP_PENDING, work: %p,\n",
 					 amdgpu_crtc->crtc_id, amdgpu_crtc, work);
 	/* update crtc fb */
-	crtc->primary->fb = fb;
+	drm_plane_set_fb(crtc->primary, fb);
 	spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
 	amdgpu_flip_work_func(&work->flip_work.work);
 	return 0;
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
index 95cb3966b2ca..1b8cc4dbe5a5 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -1065,13 +1065,7 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc,
 		return ret;
 	}
 
-	/*
-	 * Don't take a reference on the new framebuffer;
-	 * drm_mode_page_flip_ioctl() has already grabbed a reference and
-	 * will _not_ drop that reference on successful return from this
-	 * function.  Simply mark this new framebuffer as the current one.
-	 */
-	dcrtc->crtc.primary->fb = fb;
+	drm_plane_set_fb(dcrtc->crtc.primary, fb);
 
 	/*
 	 * Finally, if the display is blanked, we won't receive an
@@ -1080,6 +1074,7 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc,
 	if (dpms_blanked(dcrtc->dpms))
 		armada_drm_plane_work_run(dcrtc, dcrtc->crtc.primary);
 
+	drm_framebuffer_unreference(fb);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
index 0b4e5d117043..afe3bd2cd79e 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -103,7 +103,7 @@ static int bochs_crtc_page_flip(struct drm_crtc *crtc,
 	struct drm_framebuffer *old_fb = crtc->primary->fb;
 	unsigned long irqflags;
 
-	crtc->primary->fb = fb;
+	drm_plane_set_fb(crtc->primary, fb);
 	bochs_crtc_mode_set_base(crtc, 0, 0, old_fb);
 	if (event) {
 		spin_lock_irqsave(&bochs->dev->event_lock, irqflags);
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 19d7bcb88217..63dd5d5becdf 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1253,7 +1253,7 @@ drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state,
 		DRM_DEBUG_ATOMIC("Set [NOFB] for plane state %p\n",
 				 plane_state);
 
-	drm_framebuffer_assign(&plane_state->fb, fb);
+	drm_framebuffer_assign(__mkwrite_drm_framebuffer(&plane_state->fb), fb);
 }
 EXPORT_SYMBOL(drm_atomic_set_fb_for_plane);
 
@@ -1774,45 +1774,6 @@ static int atomic_set_prop(struct drm_atomic_state *state,
 }
 
 /**
- * drm_atomic_clean_old_fb -- Unset old_fb pointers and set plane->fb pointers.
- *
- * @dev: drm device to check.
- * @plane_mask: plane mask for planes that were updated.
- * @ret: return value, can be -EDEADLK for a retry.
- *
- * Before doing an update plane->old_fb is set to plane->fb,
- * but before dropping the locks old_fb needs to be set to NULL
- * and plane->fb updated. This is a common operation for each
- * atomic update, so this call is split off as a helper.
- */
-void drm_atomic_clean_old_fb(struct drm_device *dev,
-			     unsigned plane_mask,
-			     int ret)
-{
-	struct drm_plane *plane;
-
-	/* if succeeded, fixup legacy plane crtc/fb ptrs before dropping
-	 * locks (ie. while it is still safe to deref plane->state).  We
-	 * need to do this here because the driver entry points cannot
-	 * distinguish between legacy and atomic ioctls.
-	 */
-	drm_for_each_plane_mask(plane, dev, plane_mask) {
-		if (ret == 0) {
-			struct drm_framebuffer *new_fb = plane->state->fb;
-			if (new_fb)
-				drm_framebuffer_reference(new_fb);
-			plane->fb = new_fb;
-			plane->crtc = plane->state->crtc;
-
-			if (plane->old_fb)
-				drm_framebuffer_unreference(plane->old_fb);
-		}
-		plane->old_fb = NULL;
-	}
-}
-EXPORT_SYMBOL(drm_atomic_clean_old_fb);
-
-/**
  * DOC: explicit fencing properties
  *
  * Explicit fencing allows userspace to control the buffer synchronization
@@ -2148,7 +2109,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 		    !(arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)) {
 			plane = obj_to_plane(obj);
 			plane_mask |= (1 << drm_plane_index(plane));
-			plane->old_fb = plane->fb;
 		}
 		drm_mode_object_unreference(obj);
 	}
@@ -2174,8 +2134,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 	}
 
 out:
-	drm_atomic_clean_old_fb(dev, plane_mask, ret);
-
 	complete_crtc_signaling(dev, state, fence_state, num_fences, !ret);
 
 	if (ret == -EDEADLK) {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 494680c9056e..05c7bbf2745e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2052,6 +2052,9 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 	}
 
 	for_each_plane_in_state(state, plane, plane_state, i) {
+		drm_plane_set_fb(plane, plane_state->fb);
+		plane->crtc = plane_state->crtc;
+
 		plane->state->state = state;
 		swap(state->planes[i].state, plane->state);
 		plane->state->state = NULL;
@@ -2129,14 +2132,6 @@ int drm_atomic_helper_update_plane(struct drm_plane *plane,
 backoff:
 	drm_atomic_state_clear(state);
 	drm_atomic_legacy_backoff(state);
-
-	/*
-	 * Someone might have exchanged the framebuffer while we dropped locks
-	 * in the backoff code. We need to fix up the fb refcount tracking the
-	 * core does for us.
-	 */
-	plane->old_fb = plane->fb;
-
 	goto retry;
 }
 EXPORT_SYMBOL(drm_atomic_helper_update_plane);
@@ -2197,14 +2192,6 @@ int drm_atomic_helper_disable_plane(struct drm_plane *plane)
 backoff:
 	drm_atomic_state_clear(state);
 	drm_atomic_legacy_backoff(state);
-
-	/*
-	 * Someone might have exchanged the framebuffer while we dropped locks
-	 * in the backoff code. We need to fix up the fb refcount tracking the
-	 * core does for us.
-	 */
-	plane->old_fb = plane->fb;
-
 	goto retry;
 }
 EXPORT_SYMBOL(drm_atomic_helper_disable_plane);
@@ -2332,14 +2319,6 @@ int drm_atomic_helper_set_config(struct drm_mode_set *set)
 backoff:
 	drm_atomic_state_clear(state);
 	drm_atomic_legacy_backoff(state);
-
-	/*
-	 * Someone might have exchanged the framebuffer while we dropped locks
-	 * in the backoff code. We need to fix up the fb refcount tracking the
-	 * core does for us.
-	 */
-	crtc->primary->old_fb = crtc->primary->fb;
-
 	goto retry;
 }
 EXPORT_SYMBOL(drm_atomic_helper_set_config);
@@ -2810,14 +2789,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
 backoff:
 	drm_atomic_state_clear(state);
 	drm_atomic_legacy_backoff(state);
-
-	/*
-	 * Someone might have exchanged the framebuffer while we dropped locks
-	 * in the backoff code. We need to fix up the fb refcount tracking the
-	 * core does for us.
-	 */
-	plane->old_fb = plane->fb;
-
 	goto retry;
 }
 EXPORT_SYMBOL(drm_atomic_helper_page_flip);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 90931e039731..f94550a40ec9 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -385,8 +385,6 @@ int drm_mode_getcrtc(struct drm_device *dev,
 int drm_mode_set_config_internal(struct drm_mode_set *set)
 {
 	struct drm_crtc *crtc = set->crtc;
-	struct drm_framebuffer *fb;
-	struct drm_crtc *tmp;
 	int ret;
 
 	/*
@@ -394,24 +392,10 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
 	 * connectors from it), hence we need to refcount the fbs across all
 	 * crtcs. Atomic modeset will have saner semantics ...
 	 */
-	drm_for_each_crtc(tmp, crtc->dev)
-		tmp->primary->old_fb = tmp->primary->fb;
-
-	fb = set->fb;
 
 	ret = crtc->funcs->set_config(set);
-	if (ret == 0) {
+	if (ret == 0)
 		crtc->primary->crtc = crtc;
-		crtc->primary->fb = fb;
-	}
-
-	drm_for_each_crtc(tmp, crtc->dev) {
-		if (tmp->primary->fb)
-			drm_framebuffer_reference(tmp->primary->fb);
-		if (tmp->primary->old_fb)
-			drm_framebuffer_unreference(tmp->primary->old_fb);
-		tmp->primary->old_fb = NULL;
-	}
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 5d2cb138eba6..9818fd8c5988 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -177,7 +177,7 @@ static void __drm_helper_disable_unused_functions(struct drm_device *dev)
 				(*crtc_funcs->disable)(crtc);
 			else
 				(*crtc_funcs->dpms)(crtc, DRM_MODE_DPMS_OFF);
-			crtc->primary->fb = NULL;
+			drm_plane_set_fb(crtc->primary, NULL);
 		}
 	}
 }
@@ -579,7 +579,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
 	save_set.mode = &set->crtc->mode;
 	save_set.x = set->crtc->x;
 	save_set.y = set->crtc->y;
-	save_set.fb = set->crtc->primary->fb;
+	save_set.fb = drm_plane_get_fb(set->crtc->primary);
 
 	/* We should be able to check here if the fb has the same properties
 	 * and then just flip_or_move it */
@@ -700,13 +700,14 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
 			DRM_DEBUG_KMS("attempting to set mode from"
 					" userspace\n");
 			drm_mode_debug_printmodeline(set->mode);
-			set->crtc->primary->fb = set->fb;
+			drm_plane_set_fb(set->crtc->primary, set->fb);
 			if (!drm_crtc_helper_set_mode(set->crtc, set->mode,
 						      set->x, set->y,
 						      save_set.fb)) {
 				DRM_ERROR("failed to set mode on [CRTC:%d:%s]\n",
 					  set->crtc->base.id, set->crtc->name);
-				set->crtc->primary->fb = save_set.fb;
+				drm_plane_set_fb(set->crtc->primary,
+						 save_set.fb);
 				ret = -EINVAL;
 				goto fail;
 			}
@@ -721,17 +722,18 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
 	} else if (fb_changed) {
 		set->crtc->x = set->x;
 		set->crtc->y = set->y;
-		set->crtc->primary->fb = set->fb;
+		drm_plane_set_fb(set->crtc->primary, set->fb);
 		ret = crtc_funcs->mode_set_base(set->crtc,
 						set->x, set->y, save_set.fb);
 		if (ret != 0) {
 			set->crtc->x = save_set.x;
 			set->crtc->y = save_set.y;
-			set->crtc->primary->fb = save_set.fb;
+			drm_plane_set_fb(set->crtc->primary, save_set.fb);
 			goto fail;
 		}
 	}
 
+	drm_framebuffer_unreference(save_set.fb);
 	kfree(save_connector_encoders);
 	kfree(save_encoder_crtcs);
 	return 0;
@@ -763,6 +765,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
 				      save_set.y, save_set.fb))
 		DRM_ERROR("failed to restore config after modeset failure\n");
 
+	drm_framebuffer_unreference(save_set.fb);
 	kfree(save_connector_encoders);
 	kfree(save_encoder_crtcs);
 	return ret;
@@ -924,7 +927,8 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
 			continue;
 
 		ret = drm_crtc_helper_set_mode(crtc, &crtc->mode,
-					       crtc->x, crtc->y, crtc->primary->fb);
+					       crtc->x, crtc->y,
+					       crtc->primary->fb);
 
 		/* Restoring the old config should never fail! */
 		if (ret == false)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 62641d59a2d7..851a7e30444b 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -285,7 +285,7 @@ static struct drm_framebuffer *drm_mode_config_fb(struct drm_crtc *crtc)
 
 	drm_for_each_crtc(c, dev) {
 		if (crtc->base.id == c->base.id)
-			return c->primary->fb;
+			return drm_plane_get_fb(c->primary);
 	}
 
 	return NULL;
@@ -305,24 +305,27 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
 
 	for (i = 0; i < helper->crtc_count; i++) {
 		struct drm_mode_set *mode_set = &helper->crtc_info[i].mode_set;
-		crtc = mode_set->crtc;
-		funcs = crtc->helper_private;
-		fb = drm_mode_config_fb(crtc);
 
+		crtc = mode_set->crtc;
 		if (!crtc->enabled)
 			continue;
 
+		funcs = crtc->helper_private;
+		if (funcs->mode_set_base_atomic == NULL)
+			continue;
+
+		fb = drm_mode_config_fb(crtc);
 		if (!fb) {
 			DRM_ERROR("no fb to restore??\n");
 			continue;
 		}
 
-		if (funcs->mode_set_base_atomic == NULL)
-			continue;
-
 		drm_fb_helper_restore_lut_atomic(mode_set->crtc);
+
 		funcs->mode_set_base_atomic(mode_set->crtc, fb, crtc->x,
 					    crtc->y, LEAVE_ATOMIC_MODE_SET);
+
+		drm_framebuffer_unreference(fb);
 	}
 
 	return 0;
@@ -355,7 +358,6 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
 
 		plane_state->rotation = DRM_ROTATE_0;
 
-		plane->old_fb = plane->fb;
 		plane_mask |= 1 << drm_plane_index(plane);
 
 		/* disable non-primary: */
@@ -378,8 +380,6 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
 	ret = drm_atomic_commit(state);
 
 fail:
-	drm_atomic_clean_old_fb(dev, plane_mask, ret);
-
 	if (ret == -EDEADLK)
 		goto backoff;
 
@@ -1391,7 +1391,6 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
 
 		plane = mode_set->crtc->primary;
 		plane_mask |= (1 << drm_plane_index(plane));
-		plane->old_fb = plane->fb;
 	}
 
 	ret = drm_atomic_commit(state);
@@ -1402,8 +1401,6 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
 	info->var.yoffset = var->yoffset;
 
 fail:
-	drm_atomic_clean_old_fb(dev, plane_mask, ret);
-
 	if (ret == -EDEADLK)
 		goto backoff;
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 419ac313c36f..b424a5b40f5e 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -285,17 +285,13 @@ void drm_plane_force_disable(struct drm_plane *plane)
 	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(plane->old_fb);
-	plane->old_fb = NULL;
-	plane->fb = NULL;
+	drm_plane_set_fb(plane, NULL);
 	plane->crtc = NULL;
 }
 EXPORT_SYMBOL(drm_plane_force_disable);
@@ -459,13 +455,10 @@ static int __setplane_internal(struct drm_plane *plane,
 
 	/* No fb means shut it down */
 	if (!fb) {
-		plane->old_fb = plane->fb;
 		ret = plane->funcs->disable_plane(plane);
 		if (!ret) {
+			drm_plane_set_fb(plane, NULL);
 			plane->crtc = NULL;
-			plane->fb = NULL;
-		} else {
-			plane->old_fb = NULL;
 		}
 		goto out;
 	}
@@ -502,25 +495,15 @@ static int __setplane_internal(struct drm_plane *plane,
 	if (ret)
 		goto out;
 
-	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);
 	if (!ret) {
 		plane->crtc = crtc;
-		plane->fb = fb;
-		fb = NULL;
-	} else {
-		plane->old_fb = NULL;
+		drm_plane_set_fb(plane, fb);
 	}
 
 out:
-	if (fb)
-		drm_framebuffer_unreference(fb);
-	if (plane->old_fb)
-		drm_framebuffer_unreference(plane->old_fb);
-	plane->old_fb = NULL;
-
 	return ret;
 }
 
@@ -551,6 +534,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
 	struct drm_plane *plane;
 	struct drm_crtc *crtc = NULL;
 	struct drm_framebuffer *fb = NULL;
+	int err;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
@@ -586,11 +570,16 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
 	 * setplane_internal will take care of deref'ing either the old or new
 	 * framebuffer depending on success.
 	 */
-	return setplane_internal(plane, crtc, fb,
-				 plane_req->crtc_x, plane_req->crtc_y,
-				 plane_req->crtc_w, plane_req->crtc_h,
-				 plane_req->src_x, plane_req->src_y,
-				 plane_req->src_w, plane_req->src_h);
+	err = setplane_internal(plane, crtc, fb,
+				plane_req->crtc_x, plane_req->crtc_y,
+				plane_req->crtc_w, plane_req->crtc_h,
+				plane_req->src_x, plane_req->src_y,
+				plane_req->src_w, plane_req->src_h);
+
+	if (fb)
+		drm_framebuffer_unreference(fb);
+
+	return err;
 }
 
 static int drm_mode_cursor_universal(struct drm_crtc *crtc,
@@ -852,19 +841,19 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 		ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb);
 	}
 	if (ret)
-		goto out;
+		goto out_fb;
 
 	if (crtc->primary->fb->pixel_format != fb->pixel_format) {
 		DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer format.\n");
 		ret = -EINVAL;
-		goto out;
+		goto out_fb;
 	}
 
 	if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
 		e = kzalloc(sizeof *e, GFP_KERNEL);
 		if (!e) {
 			ret = -ENOMEM;
-			goto out;
+			goto out_fb;
 		}
 		e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
 		e->event.base.length = sizeof(e->event);
@@ -872,11 +861,10 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 		ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
 		if (ret) {
 			kfree(e);
-			goto out;
+			goto out_fb;
 		}
 	}
 
-	crtc->primary->old_fb = crtc->primary->fb;
 	if (crtc->funcs->page_flip_target)
 		ret = crtc->funcs->page_flip_target(crtc, fb, e,
 						    page_flip->flags,
@@ -886,23 +874,13 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 	if (ret) {
 		if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT)
 			drm_event_cancel_free(dev, &e->base);
-		/* Keep the old fb, don't unref it. */
-		crtc->primary->old_fb = NULL;
-	} else {
-		crtc->primary->fb = fb;
-		/* Unref only the old framebuffer. */
-		fb = NULL;
 	}
 
+out_fb:
+	drm_framebuffer_unreference(fb);
 out:
 	if (ret && crtc->funcs->page_flip_target)
 		drm_crtc_vblank_put(crtc);
-	if (fb)
-		drm_framebuffer_unreference(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;
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8630de472f9a..b887b7caf1d1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2818,12 +2818,15 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	if (i915_gem_object_is_tiled(obj))
 		dev_priv->preserve_bios_swizzle = true;
 
-	drm_framebuffer_reference(fb);
-	primary->fb = primary->state->fb = fb;
+	drm_plane_set_fb(primary, fb);
+	update_state_fb(primary);
+
 	primary->crtc = primary->state->crtc = &intel_crtc->base;
 	intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary));
 	atomic_or(to_intel_plane(primary)->frontbuffer_bit,
 		  &obj->frontbuffer_bits);
+
+	drm_framebuffer_unreference(fb);
 }
 
 static int skl_max_plane_width(const struct drm_framebuffer *fb, int plane,
@@ -12191,7 +12194,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	/* Reference the objects for the scheduled work. */
 	drm_framebuffer_reference(work->old_fb);
 
-	crtc->primary->fb = fb;
+	drm_plane_set_fb(crtc->primary, fb);
 	update_state_fb(crtc->primary);
 
 	work->pending_flip_obj = i915_gem_object_get(obj);
@@ -12293,7 +12296,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	atomic_dec(&intel_crtc->unpin_work_count);
 	mutex_unlock(&dev->struct_mutex);
 cleanup:
-	crtc->primary->fb = old_fb;
+	drm_plane_set_fb(crtc->primary, old_fb);
 	update_state_fb(crtc->primary);
 
 	i915_gem_object_put(obj);
@@ -13080,7 +13083,6 @@ intel_modeset_update_crtc_state(struct drm_atomic_state *state)
 		if (drm_atomic_get_existing_plane_state(state, crtc->primary)) {
 			struct drm_plane_state *plane_state = crtc->primary->state;
 
-			crtc->primary->fb = plane_state->fb;
 			crtc->x = plane_state->src_x >> 16;
 			crtc->y = plane_state->src_y >> 16;
 		}
@@ -17093,10 +17095,9 @@ void intel_modeset_gem_init(struct drm_device *dev)
 		if (IS_ERR(vma)) {
 			DRM_ERROR("failed to pin boot fb on pipe %d\n",
 				  to_intel_crtc(c)->pipe);
-			drm_framebuffer_unreference(c->primary->fb);
-			c->primary->fb = NULL;
-			c->primary->crtc = c->primary->state->crtc = NULL;
+			drm_plane_set_fb(c->primary, NULL);
 			update_state_fb(c->primary);
+			c->primary->crtc = c->primary->state->crtc = NULL;
 			c->state->plane_mask &= ~(1 << drm_plane_index(c->primary));
 		}
 	}
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 6b21cb27e1cc..a2d3ba237f45 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1392,7 +1392,7 @@ static void mga_crtc_disable(struct drm_crtc *crtc)
 		mgag200_bo_push_sysram(bo);
 		mgag200_bo_unreserve(bo);
 	}
-	crtc->primary->fb = NULL;
+	drm_plane_set_fb(crtc->primary, NULL);
 }
 
 /* These provide the minimum set of functions required to handle a CRTC */
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
index 911e4690d36a..5c902561c715 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
@@ -180,7 +180,7 @@ static void mdp4_plane_set_scanout(struct drm_plane *plane,
 	mdp4_write(mdp4_kms, REG_MDP4_PIPE_SRCP3_BASE(pipe),
 			msm_framebuffer_iova(fb, mdp4_kms->id, 3));
 
-	plane->fb = fb;
+	drm_plane_set_fb(plane, fb);
 }
 
 static void mdp4_write_csc_config(struct mdp4_kms *mdp4_kms,
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 81c0562ab489..0191ecbcae0b 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -425,7 +425,7 @@ static void set_scanout_locked(struct drm_plane *plane,
 	mdp5_write(mdp5_kms, REG_MDP5_PIPE_SRC3_ADDR(pipe),
 			msm_framebuffer_iova(fb, mdp5_kms->id, 3));
 
-	plane->fb = fb;
+	drm_plane_set_fb(plane, fb);
 }
 
 /* Note: mdp5_plane->pipe_lock must be locked */
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index bd37ae127582..7979617f5709 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -977,7 +977,7 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	mutex_unlock(&cli->mutex);
 
 	/* Update the crtc struct and cleanup */
-	crtc->primary->fb = fb;
+	drm_plane_set_fb(crtc->primary, fb);
 
 	nouveau_bo_fence(old_bo, fence, false);
 	ttm_bo_unreserve(&old_bo->bo);
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 22a8b70a4d1e..b098b6d04595 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -2277,13 +2277,6 @@ nv50_head_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	drm_atomic_state_clear(state);
 	drm_atomic_legacy_backoff(state);
 
-	/*
-	 * Someone might have exchanged the framebuffer while we dropped locks
-	 * in the backoff code. We need to fix up the fb refcount tracking the
-	 * core does for us.
-	 */
-	plane->old_fb = plane->fb;
-
 	goto retry;
 }
 
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index a61c0d460ec2..2e01c6e5d905 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -237,7 +237,6 @@ static int qxl_crtc_page_flip(struct drm_crtc *crtc,
 	int one_clip_rect = 1;
 	int ret = 0;
 
-	crtc->primary->fb = fb;
 	bo_old->is_primary = false;
 	bo->is_primary = true;
 
@@ -249,6 +248,7 @@ static int qxl_crtc_page_flip(struct drm_crtc *crtc,
 	if (ret)
 		return ret;
 
+	drm_plane_set_fb(crtc->primary, fb);
 	qxl_draw_dirty_fb(qdev, qfb_src, bo, 0, 0,
 			  &norect, one_clip_rect, inc);
 
@@ -755,7 +755,7 @@ static void qxl_crtc_disable(struct drm_crtc *crtc)
 		ret = qxl_bo_reserve(bo, false);
 		qxl_bo_unpin(bo);
 		qxl_bo_unreserve(bo);
-		crtc->primary->fb = NULL;
+		drm_plane_set_fb(crtc->primary, NULL);
 	}
 
 	qxl_monitors_config_set(qdev, qcrtc->index, 0, 0, 0, 0, 0);
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index e7409e8a9f87..1eb004dc4cd6 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -598,8 +598,7 @@ static int radeon_crtc_page_flip_target(struct drm_crtc *crtc,
 	radeon_crtc->flip_work = work;
 
 	/* update crtc fb */
-	crtc->primary->fb = fb;
-
+	drm_plane_set_fb(crtc->primary, fb);
 	spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
 
 	queue_work(radeon_crtc->flip_queue, &work->flip_work);
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
index 6547b1db460a..fc5df59976cd 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
@@ -462,7 +462,7 @@ static int shmob_drm_crtc_page_flip(struct drm_crtc *crtc,
 	}
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 
-	crtc->primary->fb = fb;
+	drm_plane_set_fb(crtc->primary, fb);
 	shmob_drm_crtc_update_base(scrtc);
 
 	if (event) {
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 822531ebd4b0..f57154b69e9d 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -260,9 +260,7 @@ int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
 		return -EBUSY;
 	}
 
-	drm_framebuffer_reference(fb);
-
-	crtc->primary->fb = fb;
+	drm_plane_set_fb(crtc->primary, fb);
 
 	spin_lock_irqsave(&tilcdc_crtc->irq_lock, flags);
 
diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
index f2b2481cad52..05133ef8942c 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -380,7 +380,7 @@ static int udl_crtc_page_flip(struct drm_crtc *crtc,
 	if (event)
 		drm_crtc_send_vblank_event(crtc, event);
 	spin_unlock_irqrestore(&dev->event_lock, flags);
-	crtc->primary->fb = fb;
+	drm_plane_set_fb(crtc->primary, fb);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 7f08d681a74b..b13597b3f699 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -785,7 +785,7 @@ static int vc4_async_page_flip(struct drm_crtc *crtc,
 	 * is released.
 	 */
 	drm_atomic_set_fb_for_plane(plane->state, fb);
-	plane->fb = fb;
+	drm_plane_set_fb(plane, fb);
 
 	vc4_queue_seqno_cb(dev, &flip_state->cb, bo->seqno,
 			   vc4_async_page_flip_complete);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
index 23ec673d5e16..61a3cce08dfe 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
@@ -260,7 +260,7 @@ static int vmw_ldu_crtc_set_config(struct drm_mode_set *set)
 
 		connector->encoder = NULL;
 		encoder->crtc = NULL;
-		crtc->primary->fb = NULL;
+		drm_plane_set_fb(crtc->primary, NULL);
 		crtc->enabled = false;
 
 		vmw_ldu_del_active(dev_priv, ldu);
@@ -281,7 +281,7 @@ static int vmw_ldu_crtc_set_config(struct drm_mode_set *set)
 
 	vmw_svga_enable(dev_priv);
 
-	crtc->primary->fb = fb;
+	drm_plane_set_fb(crtc->primary, fb);
 	encoder->crtc = crtc;
 	connector->encoder = encoder;
 	crtc->x = set->x;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index f42359084adc..573c7407c32c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -310,7 +310,7 @@ static int vmw_sou_crtc_set_config(struct drm_mode_set *set)
 
 		connector->encoder = NULL;
 		encoder->crtc = NULL;
-		crtc->primary->fb = NULL;
+		drm_plane_set_fb(crtc->primary, NULL);
 		crtc->x = 0;
 		crtc->y = 0;
 		crtc->enabled = false;
@@ -371,7 +371,7 @@ static int vmw_sou_crtc_set_config(struct drm_mode_set *set)
 
 		connector->encoder = NULL;
 		encoder->crtc = NULL;
-		crtc->primary->fb = NULL;
+		drm_plane_set_fb(crtc->primary, NULL);
 		crtc->x = 0;
 		crtc->y = 0;
 		crtc->enabled = false;
@@ -384,7 +384,7 @@ static int vmw_sou_crtc_set_config(struct drm_mode_set *set)
 	connector->encoder = encoder;
 	encoder->crtc = crtc;
 	crtc->mode = *mode;
-	crtc->primary->fb = fb;
+	drm_plane_set_fb(crtc->primary, fb);
 	crtc->x = set->x;
 	crtc->y = set->y;
 	crtc->enabled = true;
@@ -407,7 +407,7 @@ static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc,
 	if (!vmw_kms_crtc_flippable(dev_priv, crtc))
 		return -EINVAL;
 
-	crtc->primary->fb = fb;
+	drm_plane_set_fb(crtc->primary, fb);
 
 	/* do a full screen dirty update */
 	vclips.x = crtc->x;
@@ -454,7 +454,7 @@ static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc,
 	return ret;
 
 out_no_fence:
-	crtc->primary->fb = old_fb;
+	drm_plane_set_fb(crtc->primary, old_fb);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 94ad8d2acf9a..01c4d574fa78 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -486,7 +486,7 @@ static int vmw_stdu_bind_fb(struct vmw_private *dev_priv,
 		new_display_srf = NULL;
 	}
 
-	crtc->primary->fb = new_fb;
+	drm_plane_set_fb(crtc->primary, new_fb);
 	stdu->content_fb_type = new_content_type;
 	return 0;
 
@@ -580,7 +580,7 @@ static int vmw_stdu_crtc_set_config(struct drm_mode_set *set)
 		if (ret)
 			return ret;
 
-		crtc->primary->fb = NULL;
+		drm_plane_set_fb(crtc->primary, NULL);
 		crtc->enabled = false;
 		encoder->crtc = NULL;
 		connector->encoder = NULL;
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 5d5f85db43f0..fb616039a2bf 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -360,9 +360,6 @@ drm_atomic_add_affected_planes(struct drm_atomic_state *state,
 
 void drm_atomic_legacy_backoff(struct drm_atomic_state *state);
 
-void
-drm_atomic_clean_old_fb(struct drm_device *dev, unsigned plane_mask, int ret);
-
 int __must_check drm_atomic_check_only(struct drm_atomic_state *state);
 int __must_check drm_atomic_commit(struct drm_atomic_state *state);
 int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 5b38eb94783b..eacb1124616b 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -72,7 +72,7 @@ struct drm_plane_state {
 	 * Currently bound framebuffer. Do not write this directly, use
 	 * drm_atomic_set_fb_for_plane()
 	 */
-	struct drm_framebuffer *fb;
+	struct drm_framebuffer * const fb;
 
 	/**
 	 * @fence:
@@ -451,8 +451,6 @@ enum drm_plane_type {
  * @format_default: driver hasn't supplied supported formats for the plane
  * @crtc: currently bound CRTC
  * @fb: currently bound fb
- * @old_fb: Temporary tracking of the old fb while a modeset is ongoing. Used by
- * 	drm_mode_set_config_internal() to implement correct refcounting.
  * @funcs: helper functions
  * @properties: property tracking for this plane
  * @type: type of plane (overlay, primary, cursor)
@@ -484,9 +482,7 @@ struct drm_plane {
 	bool format_default;
 
 	struct drm_crtc *crtc;
-	struct drm_framebuffer *fb;
-
-	struct drm_framebuffer *old_fb;
+	struct drm_framebuffer * const fb;
 
 	const struct drm_plane_funcs *funcs;
 
@@ -596,5 +592,24 @@ static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
 #define drm_for_each_plane(plane, dev) \
 	list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)
 
+#define __mkwrite_drm_framebuffer(fb__) (struct drm_framebuffer **)(fb__)
+
+static inline void drm_plane_set_fb(struct drm_plane *plane,
+				    struct drm_framebuffer *fb)
+{
+	if (plane->fb == fb)
+		return;
+
+	drm_framebuffer_assign(__mkwrite_drm_framebuffer(&plane->fb), fb);
+}
+
+static inline struct drm_framebuffer *
+drm_plane_get_fb(struct drm_plane *plane)
+{
+	struct drm_framebuffer *fb = plane->fb;
+	if (fb)
+		drm_framebuffer_reference(fb);
+	return fb;
+}
 
 #endif
-- 
2.10.2



More information about the Intel-gfx mailing list