* [PATCH 1/3] drm/i915: Use helper for setting plane->state->fb
@ 2016-11-25 15:32 Chris Wilson
2016-11-25 15:32 ` [PATCH 2/3] drm: Introduce drm_framebuffer_assign() Chris Wilson
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Chris Wilson @ 2016-11-25 15:32 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
We are told not to set plane_state->fb directly, but use
drm_atomic_set_fb_for_plane() instead. Using the helper, means one less
piece of code that needs fixing should the interface change...
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_display.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 77c4ff9efbe3..8630de472f9a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2732,11 +2732,7 @@ update_state_fb(struct drm_plane *plane)
if (plane->fb == plane->state->fb)
return;
- if (plane->state->fb)
- drm_framebuffer_unreference(plane->state->fb);
- plane->state->fb = plane->fb;
- if (plane->state->fb)
- drm_framebuffer_reference(plane->state->fb);
+ drm_atomic_set_fb_for_plane(plane->state, plane->fb);
}
static void
--
2.10.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] drm: Introduce drm_framebuffer_assign()
2016-11-25 15:32 [PATCH 1/3] drm/i915: Use helper for setting plane->state->fb Chris Wilson
@ 2016-11-25 15:32 ` Chris Wilson
2016-11-25 15:32 ` [PATCH 3/3] drm: Track framebuffer references at the point of assignment Chris Wilson
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2016-11-25 15:32 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
In a couple of places currently, and with the intent to add more, we
update a pointer to a framebuffer to hold a new fb reference (evicting
the old).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/drm_atomic.c | 8 ++------
include/drm/drm_framebuffer.h | 18 ++++++++++++++++++
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 89737e42fa83..19d7bcb88217 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1246,18 +1246,14 @@ void
drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state,
struct drm_framebuffer *fb)
{
- if (plane_state->fb)
- drm_framebuffer_unreference(plane_state->fb);
- if (fb)
- drm_framebuffer_reference(fb);
- plane_state->fb = fb;
-
if (fb)
DRM_DEBUG_ATOMIC("Set [FB:%d] for plane state %p\n",
fb->base.id, plane_state);
else
DRM_DEBUG_ATOMIC("Set [NOFB] for plane state %p\n",
plane_state);
+
+ drm_framebuffer_assign(&plane_state->fb, fb);
}
EXPORT_SYMBOL(drm_atomic_set_fb_for_plane);
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index b3141a0e609b..1ddfa2928802 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -251,6 +251,24 @@ static inline uint32_t drm_framebuffer_read_refcount(struct drm_framebuffer *fb)
}
/**
+ * drm_framebuffer_assign - store a reference to the fb
+ * @p: location to store framebuffer
+ * @fb: new framebuffer (maybe NULL)
+ *
+ * This functions sets the location to store a reference to the framebuffer,
+ * unreferencing the framebuffer that was previously stored in that location.
+ */
+static inline void drm_framebuffer_assign(struct drm_framebuffer **p,
+ struct drm_framebuffer *fb)
+{
+ if (fb)
+ drm_framebuffer_reference(fb);
+ if (*p)
+ drm_framebuffer_unreference(*p);
+ *p = fb;
+}
+
+/*
* drm_for_each_fb - iterate over all framebuffers
* @fb: the loop cursor
* @dev: the DRM device
--
2.10.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] drm: Track framebuffer references at the point of assignment
2016-11-25 15:32 [PATCH 1/3] drm/i915: Use helper for setting plane->state->fb Chris Wilson
2016-11-25 15:32 ` [PATCH 2/3] drm: Introduce drm_framebuffer_assign() Chris Wilson
@ 2016-11-25 15:32 ` Chris Wilson
2016-11-28 7:48 ` Daniel Vetter
2016-11-25 16:23 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Use helper for setting plane->state->fb Patchwork
2016-11-28 7:20 ` [PATCH 1/3] " Daniel Vetter
3 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2016-11-25 15:32 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
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@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Use helper for setting plane->state->fb
2016-11-25 15:32 [PATCH 1/3] drm/i915: Use helper for setting plane->state->fb Chris Wilson
2016-11-25 15:32 ` [PATCH 2/3] drm: Introduce drm_framebuffer_assign() Chris Wilson
2016-11-25 15:32 ` [PATCH 3/3] drm: Track framebuffer references at the point of assignment Chris Wilson
@ 2016-11-25 16:23 ` Patchwork
2016-11-28 7:20 ` [PATCH 1/3] " Daniel Vetter
3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2016-11-25 16:23 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/3] drm/i915: Use helper for setting plane->state->fb
URL : https://patchwork.freedesktop.org/series/15972/
State : success
== Summary ==
Series 15972v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/15972/revisions/1/mbox/
Test kms_force_connector_basic:
Subgroup force-load-detect:
dmesg-warn -> PASS (fi-snb-2520m)
fi-bdw-5557u total:245 pass:230 dwarn:0 dfail:0 fail:0 skip:15
fi-bsw-n3050 total:245 pass:205 dwarn:0 dfail:0 fail:0 skip:40
fi-bxt-t5700 total:245 pass:217 dwarn:0 dfail:0 fail:0 skip:28
fi-byt-j1900 total:245 pass:217 dwarn:0 dfail:0 fail:0 skip:28
fi-byt-n2820 total:245 pass:213 dwarn:0 dfail:0 fail:0 skip:32
fi-hsw-4770 total:245 pass:225 dwarn:0 dfail:0 fail:0 skip:20
fi-hsw-4770r total:245 pass:225 dwarn:0 dfail:0 fail:0 skip:20
fi-ilk-650 total:245 pass:192 dwarn:0 dfail:0 fail:0 skip:53
fi-ivb-3520m total:245 pass:223 dwarn:0 dfail:0 fail:0 skip:22
fi-ivb-3770 total:245 pass:223 dwarn:0 dfail:0 fail:0 skip:22
fi-kbl-7500u total:245 pass:223 dwarn:0 dfail:0 fail:0 skip:22
fi-skl-6260u total:245 pass:231 dwarn:0 dfail:0 fail:0 skip:14
fi-skl-6700hq total:245 pass:224 dwarn:0 dfail:0 fail:0 skip:21
fi-skl-6700k total:245 pass:223 dwarn:1 dfail:0 fail:0 skip:21
fi-skl-6770hq total:245 pass:231 dwarn:0 dfail:0 fail:0 skip:14
fi-snb-2520m total:245 pass:213 dwarn:0 dfail:0 fail:0 skip:32
fi-snb-2600 total:245 pass:212 dwarn:0 dfail:0 fail:0 skip:33
c744533b1d54437a7986108711de03523d591b27 drm-tip: 2016y-11m-25d-15h-02m-01s UTC integration manifest
8bee8a1 drm: Track framebuffer references at the point of assignment
06e5620 drm: Introduce drm_framebuffer_assign()
63b83ad drm/i915: Use helper for setting plane->state->fb
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3116/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] drm/i915: Use helper for setting plane->state->fb
2016-11-25 15:32 [PATCH 1/3] drm/i915: Use helper for setting plane->state->fb Chris Wilson
` (2 preceding siblings ...)
2016-11-25 16:23 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Use helper for setting plane->state->fb Patchwork
@ 2016-11-28 7:20 ` Daniel Vetter
3 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2016-11-28 7:20 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, dri-devel
On Fri, Nov 25, 2016 at 03:32:29PM +0000, Chris Wilson wrote:
> We are told not to set plane_state->fb directly, but use
> drm_atomic_set_fb_for_plane() instead. Using the helper, means one less
> piece of code that needs fixing should the interface change...
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 77c4ff9efbe3..8630de472f9a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2732,11 +2732,7 @@ update_state_fb(struct drm_plane *plane)
> if (plane->fb == plane->state->fb)
> return;
>
> - if (plane->state->fb)
> - drm_framebuffer_unreference(plane->state->fb);
> - plane->state->fb = plane->fb;
> - if (plane->state->fb)
> - drm_framebuffer_reference(plane->state->fb);
> + drm_atomic_set_fb_for_plane(plane->state, plane->fb);
> }
>
> static void
> --
> 2.10.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] drm: Track framebuffer references at the point of assignment
2016-11-25 15:32 ` [PATCH 3/3] drm: Track framebuffer references at the point of assignment Chris Wilson
@ 2016-11-28 7:48 ` Daniel Vetter
2016-11-28 8:46 ` Chris Wilson
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2016-11-28 7:48 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, dri-devel
On Fri, Nov 25, 2016 at 03:32:31PM +0000, Chris Wilson wrote:
> 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@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);
Feels like const gone the wrong way round? Or I'm not entirely clear on
what this is supposed to achieve. Just dropping the const would still be a
nice improvement.
-Daniel
> }
> 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
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] drm: Track framebuffer references at the point of assignment
2016-11-28 7:48 ` Daniel Vetter
@ 2016-11-28 8:46 ` Chris Wilson
2016-11-28 14:15 ` Daniel Vetter
0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2016-11-28 8:46 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
On Mon, Nov 28, 2016 at 08:48:34AM +0100, Daniel Vetter wrote:
> On Fri, Nov 25, 2016 at 03:32:31PM +0000, Chris Wilson wrote:
> > --- 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);
>
> Feels like const gone the wrong way round? Or I'm not entirely clear on
> what this is supposed to achieve. Just dropping the const would still be a
> nice improvement.
No one is supposed to be writing to it. Adding the const generates a
compiler warning for incorrect code that doesn't do the refcounting -
but doesn't generate a warning for anything simply checking the value.
It is the moral equivalent to calling it _fb to catch all the users.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] drm: Track framebuffer references at the point of assignment
2016-11-28 8:46 ` Chris Wilson
@ 2016-11-28 14:15 ` Daniel Vetter
2016-11-28 14:44 ` Chris Wilson
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2016-11-28 14:15 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, dri-devel, intel-gfx
On Mon, Nov 28, 2016 at 08:46:11AM +0000, Chris Wilson wrote:
> On Mon, Nov 28, 2016 at 08:48:34AM +0100, Daniel Vetter wrote:
> > On Fri, Nov 25, 2016 at 03:32:31PM +0000, Chris Wilson wrote:
> > > --- 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);
> >
> > Feels like const gone the wrong way round? Or I'm not entirely clear on
> > what this is supposed to achieve. Just dropping the const would still be a
> > nice improvement.
>
> No one is supposed to be writing to it. Adding the const generates a
> compiler warning for incorrect code that doesn't do the refcounting -
> but doesn't generate a warning for anything simply checking the value.
> It is the moral equivalent to calling it _fb to catch all the users.
But then shouldn't all users use the drm_plane_set_fb helper (and that
maybe gain a comment)? The above escaped the conversion/consolidation,
which is why it looked funny to me ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] drm: Track framebuffer references at the point of assignment
2016-11-28 14:15 ` Daniel Vetter
@ 2016-11-28 14:44 ` Chris Wilson
2016-11-28 15:44 ` Daniel Vetter
0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2016-11-28 14:44 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
On Mon, Nov 28, 2016 at 03:15:12PM +0100, Daniel Vetter wrote:
> On Mon, Nov 28, 2016 at 08:46:11AM +0000, Chris Wilson wrote:
> > On Mon, Nov 28, 2016 at 08:48:34AM +0100, Daniel Vetter wrote:
> > > On Fri, Nov 25, 2016 at 03:32:31PM +0000, Chris Wilson wrote:
> > > > --- 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);
> > >
> > > Feels like const gone the wrong way round? Or I'm not entirely clear on
> > > what this is supposed to achieve. Just dropping the const would still be a
> > > nice improvement.
> >
> > No one is supposed to be writing to it. Adding the const generates a
> > compiler warning for incorrect code that doesn't do the refcounting -
> > but doesn't generate a warning for anything simply checking the value.
> > It is the moral equivalent to calling it _fb to catch all the users.
>
> But then shouldn't all users use the drm_plane_set_fb helper (and that
> maybe gain a comment)? The above escaped the conversion/consolidation,
> which is why it looked funny to me ...
Yes, all users (of plane->fb = fb) are. The above is for
plane_state->fb.
I wasn't sure about adding comments to struct drm_plane, I thought the
crtc/fb there was just a crutch for !atomic.
But adding
+
+ /**
+ * @fb:
+ *
+ * Currently active framebuffer. Do not write this directly, use
+ * drm_plane_set_fb()
+ */
struct drm_framebuffer * const fb;
is not a hardship.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] drm: Track framebuffer references at the point of assignment
2016-11-28 14:44 ` Chris Wilson
@ 2016-11-28 15:44 ` Daniel Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2016-11-28 15:44 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, dri-devel, intel-gfx
On Mon, Nov 28, 2016 at 02:44:15PM +0000, Chris Wilson wrote:
> On Mon, Nov 28, 2016 at 03:15:12PM +0100, Daniel Vetter wrote:
> > On Mon, Nov 28, 2016 at 08:46:11AM +0000, Chris Wilson wrote:
> > > On Mon, Nov 28, 2016 at 08:48:34AM +0100, Daniel Vetter wrote:
> > > > On Fri, Nov 25, 2016 at 03:32:31PM +0000, Chris Wilson wrote:
> > > > > --- 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);
> > > >
> > > > Feels like const gone the wrong way round? Or I'm not entirely clear on
> > > > what this is supposed to achieve. Just dropping the const would still be a
> > > > nice improvement.
> > >
> > > No one is supposed to be writing to it. Adding the const generates a
> > > compiler warning for incorrect code that doesn't do the refcounting -
> > > but doesn't generate a warning for anything simply checking the value.
> > > It is the moral equivalent to calling it _fb to catch all the users.
> >
> > But then shouldn't all users use the drm_plane_set_fb helper (and that
> > maybe gain a comment)? The above escaped the conversion/consolidation,
> > which is why it looked funny to me ...
>
> Yes, all users (of plane->fb = fb) are. The above is for
> plane_state->fb.
Ah, missed that ;-)
> I wasn't sure about adding comments to struct drm_plane, I thought the
> crtc/fb there was just a crutch for !atomic.
>
> But adding
> +
> + /**
> + * @fb:
> + *
> + * Currently active framebuffer. Do not write this directly, use
> + * drm_plane_set_fb()
> + */
> struct drm_framebuffer * const fb;
>
> is not a hardship.
Yeah, that'd be good. We already have a similar comment for
drm_plane_state.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-11-28 15:44 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-25 15:32 [PATCH 1/3] drm/i915: Use helper for setting plane->state->fb Chris Wilson
2016-11-25 15:32 ` [PATCH 2/3] drm: Introduce drm_framebuffer_assign() Chris Wilson
2016-11-25 15:32 ` [PATCH 3/3] drm: Track framebuffer references at the point of assignment Chris Wilson
2016-11-28 7:48 ` Daniel Vetter
2016-11-28 8:46 ` Chris Wilson
2016-11-28 14:15 ` Daniel Vetter
2016-11-28 14:44 ` Chris Wilson
2016-11-28 15:44 ` Daniel Vetter
2016-11-25 16:23 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Use helper for setting plane->state->fb Patchwork
2016-11-28 7:20 ` [PATCH 1/3] " Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.