From: Ville Syrjälä ville.syrjala@linux.intel.com
Here are again the last (?) bits of eliminating the plane->fb/crtc usage for atomic drivers. I've pushed everything else (thanks to everyone who reviewed them).
Deepak said he'd tested the vmwgfx stuff, so I think it should be safe to land. Just missing a bit of review...
Cc: Alex Deucher alexander.deucher@amd.com Cc: amd-gfx@lists.freedesktop.org Cc: "Christian König" christian.koenig@amd.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: David Airlie airlied@linux.ie Cc: "David (ChunMing) Zhou" David1.Zhou@amd.com Cc: Deepak Rawat drawat@vmware.com Cc: Eric Anholt eric@anholt.net Cc: freedreno@lists.freedesktop.org Cc: Gerd Hoffmann kraxel@redhat.com Cc: Harry Wentland harry.wentland@amd.com Cc: Inki Dae inki.dae@samsung.com Cc: Joonyoung Shim jy0922.shim@samsung.com Cc: Kyungmin Park kyungmin.park@samsung.com Cc: linux-arm-msm@vger.kernel.org Cc: Rob Clark robdclark@gmail.com Cc: Seung-Woo Kim sw0312.kim@samsung.com Cc: Sinclair Yeh syeh@vmware.com Cc: Thomas Hellstrom thellstrom@vmware.com Cc: virtualization@lists.linux-foundation.org Cc: VMware Graphics linux-graphics-maintainer@vmware.com
Ville Syrjälä (13): drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset() drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty() drm/vmwgfx: Stop using plane->fb in vmw_kms_update_implicit_fb() drm/vmwgfx: Stop updating plane->fb drm/vmwgfx: Stop using plane->fb in atomic_enable() drm/vmwgfx: Stop messing about with plane->fb/old_fb/crtc drm/amdgpu/dc: Stop updating plane->fb drm/i915: Stop updating plane->fb/crtc drm/exynos: Stop updating plane->crtc drm/msm: Stop updating plane->fb/crtc drm/virtio: Stop updating plane->crtc drm/vc4: Stop updating plane->fb/crtc drm: Stop updating plane->crtc/fb/old_fb on atomic drivers
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 - drivers/gpu/drm/drm_atomic.c | 55 +++-------------------- drivers/gpu/drm/drm_atomic_helper.c | 15 +------ drivers/gpu/drm/drm_crtc.c | 8 +++- drivers/gpu/drm/drm_fb_helper.c | 7 --- drivers/gpu/drm/drm_framebuffer.c | 5 --- drivers/gpu/drm/drm_plane.c | 14 +++--- drivers/gpu/drm/drm_plane_helper.c | 4 +- drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 - drivers/gpu/drm/i915/intel_atomic_plane.c | 12 ----- drivers/gpu/drm/i915/intel_display.c | 7 ++- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 - drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 2 - drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 - drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 2 - drivers/gpu/drm/vc4/vc4_crtc.c | 3 -- drivers/gpu/drm/virtio/virtgpu_display.c | 2 - drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 24 ---------- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 24 +++++++--- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 - drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 5 +-- include/drm/drm_atomic.h | 3 -- 22 files changed, 46 insertions(+), 154 deletions(-)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of looking at plane->fb let's look at the proper new plane state.
Not that the code makes a ton of sense. It's only going through the crtcs in the atomic state, so assuming not all of them are included we're not even calculating the total bandwidth here. Also we're not considering whether each crtc is actually enabled or not.
Cc: Deepak Rawat drawat@vmware.com Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Sinclair Yeh syeh@vmware.com Cc: VMware Graphics linux-graphics-maintainer@vmware.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 01f2dc9e6f52..2e4c38bb846d 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -1536,9 +1536,13 @@ vmw_kms_atomic_check_modeset(struct drm_device *dev, unsigned long requested_bb_mem = 0;
if (dev_priv->active_display_unit == vmw_du_screen_target) { - if (crtc->primary->fb) { - int cpp = crtc->primary->fb->pitches[0] / - crtc->primary->fb->width; + struct drm_plane *plane = crtc->primary; + struct drm_plane_state *plane_state; + + plane_state = drm_atomic_get_new_plane_state(state, plane); + + if (plane_state && plane_state->fb) { + int cpp = plane_state->fb->format->cpp[0];
requested_bb_mem += crtc->mode.hdisplay * cpp * crtc->mode.vdisplay;
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of plane->fb (which we're going to deprecate for atomic drivers) we need to look at plane->state->fb. The maze of code leading to vmw_kms_helper_dirty() wasn't particularly clear, but my analysis concluded that the calls originating from vmw_*_primary_plane_atomic_update() all pass in the crtc which means we'll never end up in this branch of the function. All other callers use drm_modeset_lock_all() somewhere higher up, which means accessing plane->state is safe. We'll toss in a lockdep assert to catch wrongdoers.
v2: Drop the comment and make the code do what it did before (Thomas)
Cc: Deepak Rawat drawat@vmware.com Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Sinclair Yeh syeh@vmware.com Cc: VMware Graphics linux-graphics-maintainer@vmware.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 2e4c38bb846d..5417eb1b486e 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -2326,9 +2326,12 @@ int vmw_kms_helper_dirty(struct vmw_private *dev_priv, } else { list_for_each_entry(crtc, &dev_priv->dev->mode_config.crtc_list, head) { - if (crtc->primary->fb != &framebuffer->base) - continue; - units[num_units++] = vmw_crtc_to_du(crtc); + struct drm_plane *plane = crtc->primary; + + lockdep_assert_held(&plane->mutex); + + if (plane->state->fb == &framebuffer->base) + units[num_units++] = vmw_crtc_to_du(crtc); } }
On Fri, May 25, 2018 at 09:50:34PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of plane->fb (which we're going to deprecate for atomic drivers) we need to look at plane->state->fb. The maze of code leading to vmw_kms_helper_dirty() wasn't particularly clear, but my analysis concluded that the calls originating from vmw_*_primary_plane_atomic_update() all pass in the crtc which means we'll never end up in this branch of the function. All other callers use drm_modeset_lock_all() somewhere higher up, which means accessing plane->state is safe. We'll toss in a lockdep assert to catch wrongdoers.
v2: Drop the comment and make the code do what it did before (Thomas)
Cc: Deepak Rawat drawat@vmware.com Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Sinclair Yeh syeh@vmware.com Cc: VMware Graphics linux-graphics-maintainer@vmware.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 2e4c38bb846d..5417eb1b486e 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -2326,9 +2326,12 @@ int vmw_kms_helper_dirty(struct vmw_private *dev_priv, } else { list_for_each_entry(crtc, &dev_priv->dev->mode_config.crtc_list, head) {
if (crtc->primary->fb != &framebuffer->base)continue;units[num_units++] = vmw_crtc_to_du(crtc);
struct drm_plane *plane = crtc->primary;lockdep_assert_held(&plane->mutex);
kbuild test robot told me
include/linux/lockdep.h:347:52: error: 'struct drm_modeset_lock' has no member named 'dep_map'
#define lockdep_is_held(lock) lock_is_held(&(lock)->dep_map)
Maybe I'll just drop the asserts? Or do people really want them (in which case I gues I need to dig out the underlying mutex)?
if (plane->state->fb == &framebuffer->base)units[num_units++] = vmw_crtc_to_du(crtc);-- 2.16.1
On Wed, May 30, 2018 at 11:08:57PM +0300, Ville Syrjälä wrote:
On Fri, May 25, 2018 at 09:50:34PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of plane->fb (which we're going to deprecate for atomic drivers) we need to look at plane->state->fb. The maze of code leading to vmw_kms_helper_dirty() wasn't particularly clear, but my analysis concluded that the calls originating from vmw_*_primary_plane_atomic_update() all pass in the crtc which means we'll never end up in this branch of the function. All other callers use drm_modeset_lock_all() somewhere higher up, which means accessing plane->state is safe. We'll toss in a lockdep assert to catch wrongdoers.
v2: Drop the comment and make the code do what it did before (Thomas)
Cc: Deepak Rawat drawat@vmware.com Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Sinclair Yeh syeh@vmware.com Cc: VMware Graphics linux-graphics-maintainer@vmware.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 2e4c38bb846d..5417eb1b486e 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -2326,9 +2326,12 @@ int vmw_kms_helper_dirty(struct vmw_private *dev_priv, } else { list_for_each_entry(crtc, &dev_priv->dev->mode_config.crtc_list, head) {
if (crtc->primary->fb != &framebuffer->base)continue;units[num_units++] = vmw_crtc_to_du(crtc);
struct drm_plane *plane = crtc->primary;lockdep_assert_held(&plane->mutex);kbuild test robot told me
include/linux/lockdep.h:347:52: error: 'struct drm_modeset_lock' has no member named 'dep_map'
#define lockdep_is_held(lock) lock_is_held(&(lock)->dep_map)Maybe I'll just drop the asserts? Or do people really want them (in which case I gues I need to dig out the underlying mutex)?
Eitherway is fine with me.
if (plane->state->fb == &framebuffer->base)units[num_units++] = vmw_crtc_to_du(crtc);-- 2.16.1
-- Ville Syrjälä Intel
On Mon, Jun 04, 2018 at 11:13:53AM -0700, Sinclair Yeh wrote:
On Wed, May 30, 2018 at 11:08:57PM +0300, Ville Syrjälä wrote:
On Fri, May 25, 2018 at 09:50:34PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of plane->fb (which we're going to deprecate for atomic drivers) we need to look at plane->state->fb. The maze of code leading to vmw_kms_helper_dirty() wasn't particularly clear, but my analysis concluded that the calls originating from vmw_*_primary_plane_atomic_update() all pass in the crtc which means we'll never end up in this branch of the function. All other callers use drm_modeset_lock_all() somewhere higher up, which means accessing plane->state is safe. We'll toss in a lockdep assert to catch wrongdoers.
v2: Drop the comment and make the code do what it did before (Thomas)
Cc: Deepak Rawat drawat@vmware.com Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Sinclair Yeh syeh@vmware.com Cc: VMware Graphics linux-graphics-maintainer@vmware.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 2e4c38bb846d..5417eb1b486e 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -2326,9 +2326,12 @@ int vmw_kms_helper_dirty(struct vmw_private *dev_priv, } else { list_for_each_entry(crtc, &dev_priv->dev->mode_config.crtc_list, head) {
if (crtc->primary->fb != &framebuffer->base)continue;units[num_units++] = vmw_crtc_to_du(crtc);
struct drm_plane *plane = crtc->primary;lockdep_assert_held(&plane->mutex);kbuild test robot told me
include/linux/lockdep.h:347:52: error: 'struct drm_modeset_lock' has no member named 'dep_map'
#define lockdep_is_held(lock) lock_is_held(&(lock)->dep_map)Maybe I'll just drop the asserts? Or do people really want them (in which case I gues I need to dig out the underlying mutex)?
Eitherway is fine with me.
I just dropped them, and pushe the series to drm-misc-next. Thanks for the reviews everyone.
Fingers crossed that no new uses of the legacy pointers have been added in the meantime. A quick grep didn't seem to catch anything at least, but selective blindness can occur sometimes. So just holler if anything broke...
From: Ville Syrjälä ville.syrjala@linux.intel.com
The only caller of vmw_kms_update_implicit_fb() is the page_flip hook which itself gets called with the plane mutex already held. Hence we can look at plane->state safely. Toss in a lockdep assert to make the situation more clear.
Cc: Deepak Rawat drawat@vmware.com Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Sinclair Yeh syeh@vmware.com Cc: VMware Graphics linux-graphics-maintainer@vmware.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Deepak Rawat drawat@vmware.com --- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 5417eb1b486e..04d51bb639b8 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -2813,14 +2813,17 @@ void vmw_kms_update_implicit_fb(struct vmw_private *dev_priv, struct drm_crtc *crtc) { struct vmw_display_unit *du = vmw_crtc_to_du(crtc); + struct drm_plane *plane = crtc->primary; struct vmw_framebuffer *vfb;
+ lockdep_assert_held(&plane->mutex); + mutex_lock(&dev_priv->global_kms_state_mutex);
if (!du->is_implicit) goto out_unlock;
- vfb = vmw_framebuffer_to_vfb(crtc->primary->fb); + vfb = vmw_framebuffer_to_vfb(plane->state->fb); WARN_ON_ONCE(dev_priv->num_implicit != 1 && dev_priv->implicit_fb != vfb);
From: Ville Syrjälä ville.syrjala@linux.intel.com
We want to get rid of plane->fb on atomic drivers. Stop setting it.
Cc: Deepak Rawat drawat@vmware.com Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Sinclair Yeh syeh@vmware.com Cc: VMware Graphics linux-graphics-maintainer@vmware.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Deepak Rawat drawat@vmware.com --- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 -- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 -- 2 files changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c index 3d667e903beb..9798640cbfcd 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c @@ -527,8 +527,6 @@ vmw_sou_primary_plane_atomic_update(struct drm_plane *plane, */ if (ret != 0) DRM_ERROR("Failed to update screen.\n"); - - crtc->primary->fb = plane->state->fb; } else { /* * When disabling a plane, CRTC and FB should always be NULL diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c index 67331f01ef32..90445bc590cb 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c @@ -1285,8 +1285,6 @@ vmw_stdu_primary_plane_atomic_update(struct drm_plane *plane, 1, 1, NULL, crtc); if (ret) DRM_ERROR("Failed to update STDU.\n"); - - crtc->primary->fb = plane->state->fb; } else { crtc = old_state->crtc; stdu = vmw_crtc_to_stdu(crtc);
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of looking at the (soon to be deprecated) plane->fb we'll examing plane->state->fb instead. We can do this because vmw_du_crtc_atomic_check() prevents us from enabling a crtc without the primary plane also being enabled.
Due to that same reason, I'm actually not sure what the checks here are for NULL fb. If we can't enable the crtc without an enabled plane we should always have an fb. But I'll leave that for someone else to figure out.
Cc: Deepak Rawat drawat@vmware.com Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Sinclair Yeh syeh@vmware.com Cc: VMware Graphics linux-graphics-maintainer@vmware.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Deepak Rawat drawat@vmware.com --- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c index 90445bc590cb..152e96cb1c01 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c @@ -414,6 +414,7 @@ static void vmw_stdu_crtc_helper_prepare(struct drm_crtc *crtc) static void vmw_stdu_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { + struct drm_plane_state *plane_state = crtc->primary->state; struct vmw_private *dev_priv; struct vmw_screen_target_display_unit *stdu; struct vmw_framebuffer *vfb; @@ -422,7 +423,7 @@ static void vmw_stdu_crtc_atomic_enable(struct drm_crtc *crtc,
stdu = vmw_crtc_to_stdu(crtc); dev_priv = vmw_priv(crtc->dev); - fb = crtc->primary->fb; + fb = plane_state->fb;
vfb = (fb) ? vmw_framebuffer_to_vfb(fb) : NULL;
From: Ville Syrjälä ville.syrjala@linux.intel.com
plane->fb/old_fb/crtc should no longer be used by atomic drivers. Stop messing about with them.
Cc: Deepak Rawat drawat@vmware.com Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Sinclair Yeh syeh@vmware.com Cc: VMware Graphics linux-graphics-maintainer@vmware.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 24 ------------------------ 1 file changed, 24 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c index 54e300365a5c..fcca0487dab6 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c @@ -439,8 +439,6 @@ static int vmw_fb_compute_depth(struct fb_var_screeninfo *var, static int vmwgfx_set_config_internal(struct drm_mode_set *set) { struct drm_crtc *crtc = set->crtc; - struct drm_framebuffer *fb; - struct drm_crtc *tmp; struct drm_device *dev = set->crtc->dev; struct drm_modeset_acquire_ctx ctx; int ret; @@ -448,29 +446,7 @@ static int vmwgfx_set_config_internal(struct drm_mode_set *set) drm_modeset_acquire_init(&ctx, 0);
restart: - /* - * NOTE: ->set_config can also disable other crtcs (if we steal all - * connectors from it), hence we need to refcount the fbs across all - * crtcs. Atomic modeset will have saner semantics ... - */ - drm_for_each_crtc(tmp, dev) - tmp->primary->old_fb = tmp->primary->fb; - - fb = set->fb; - ret = crtc->funcs->set_config(set, &ctx); - if (ret == 0) { - crtc->primary->crtc = crtc; - crtc->primary->fb = fb; - } - - drm_for_each_crtc(tmp, dev) { - if (tmp->primary->fb) - drm_framebuffer_get(tmp->primary->fb); - if (tmp->primary->old_fb) - drm_framebuffer_put(tmp->primary->old_fb); - tmp->primary->old_fb = NULL; - }
if (ret == -EDEADLK) { drm_modeset_backoff(&ctx);
From: Ville Syrjälä ville.syrjala@linux.intel.com
We want to get rid of plane->fb on atomic drivers. Stop setting it.
Cc: Alex Deucher alexander.deucher@amd.com Cc: "Christian König" christian.koenig@amd.com Cc: "David (ChunMing) Zhou" David1.Zhou@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: amd-gfx@lists.freedesktop.org Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Reviewed-by: Harry Wentland harry.wentland@amd.com Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 1ce10bc2d37b..82bac02fffd7 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3927,8 +3927,6 @@ static void amdgpu_dm_do_flip(struct drm_crtc *crtc,
/* Flip */ spin_lock_irqsave(&crtc->dev->event_lock, flags); - /* update crtc fb */ - crtc->primary->fb = fb;
WARN_ON(acrtc->pflip_status != AMDGPU_FLIP_NONE); WARN_ON(!acrtc_state->stream);
From: Ville Syrjälä ville.syrjala@linux.intel.com
We want to get rid of plane->fb/crtc on atomic drivers. Stop setting them.
v2: Fix up the comment in intel_crtc_active() and nuke the rest of the stale comments (Daniel)
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com #v1 Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/i915/intel_atomic_plane.c | 12 ------------ drivers/gpu/drm/i915/intel_display.c | 7 +++---- 2 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 6d068786eb41..e8bf4cc499e1 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -120,12 +120,6 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ &crtc_state->base.adjusted_mode; int ret;
- /* - * Both crtc and plane->crtc could be NULL if we're updating a - * property while the plane is disabled. We don't actually have - * anything driver-specific we need to test in that case, so - * just return success. - */ if (!intel_state->base.crtc && !old_plane_state->base.crtc) return 0;
@@ -209,12 +203,6 @@ static int intel_plane_atomic_check(struct drm_plane *plane, const struct drm_crtc_state *old_crtc_state; struct drm_crtc_state *new_crtc_state;
- /* - * Both crtc and plane->crtc could be NULL if we're updating a - * property while the plane is disabled. We don't actually have - * anything driver-specific we need to test in that case, so - * just return success. - */ if (!crtc) return 0;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 90a6ff00c79d..8e7e87e4e11f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1022,7 +1022,7 @@ bool intel_crtc_active(struct intel_crtc *crtc) * We can ditch the adjusted_mode.crtc_clock check as soon * as Haswell has gained clock readout/fastboot support. * - * We can ditch the crtc->primary->fb check as soon as we can + * We can ditch the crtc->primary->state->fb check as soon as we can * properly reconstruct framebuffers. * * FIXME: The intel_crtc->active here should be switched to @@ -2879,9 +2879,8 @@ 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_get(fb); - primary->fb = primary->state->fb = fb; - primary->crtc = primary->state->crtc = &intel_crtc->base; + plane_state->fb = fb; + plane_state->crtc = &intel_crtc->base;
intel_set_plane_visible(to_intel_crtc_state(crtc_state), to_intel_plane_state(plane_state),
From: Ville Syrjälä ville.syrjala@linux.intel.com
We want to get rid of plane->crtc on atomic drivers. Stop setting it.
Cc: Inki Dae inki.dae@samsung.com Cc: Joonyoung Shim jy0922.shim@samsung.com Cc: Seung-Woo Kim sw0312.kim@samsung.com Cc: Kyungmin Park kyungmin.park@samsung.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Acked-by: Inki Dae inki.dae@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c index d2a90dae5c71..1b1af359c303 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c @@ -263,8 +263,6 @@ static void exynos_plane_atomic_update(struct drm_plane *plane, if (!state->crtc) return;
- plane->crtc = state->crtc; - if (exynos_crtc->ops->update_plane) exynos_crtc->ops->update_plane(exynos_crtc, exynos_plane); }
From: Ville Syrjälä ville.syrjala@linux.intel.com
We want to get rid of plane->fb/crtc on atomic drivers. Stop setting them.
v2: Catch a few more cases
Cc: Rob Clark robdclark@gmail.com Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com #v1 Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 - drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 2 -- drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 - drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 2 -- 4 files changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c index 20f9e5de5f19..457c29dba4a1 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c @@ -665,7 +665,6 @@ struct drm_crtc *mdp4_crtc_init(struct drm_device *dev, drm_crtc_init_with_planes(dev, crtc, plane, NULL, &mdp4_crtc_funcs, NULL); drm_crtc_helper_add(crtc, &mdp4_crtc_helper_funcs); - plane->crtc = crtc;
return crtc; } diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c index 7a1ad3af08e3..782b1e27f040 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c @@ -182,8 +182,6 @@ static void mdp4_plane_set_scanout(struct drm_plane *plane, msm_framebuffer_iova(fb, kms->aspace, 2)); mdp4_write(mdp4_kms, REG_MDP4_PIPE_SRCP3_BASE(pipe), msm_framebuffer_iova(fb, kms->aspace, 3)); - - plane->fb = fb; }
static void mdp4_write_csc_config(struct mdp4_kms *mdp4_kms, diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c index 76b96081916f..efedcac6e641 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c @@ -1198,7 +1198,6 @@ struct drm_crtc *mdp5_crtc_init(struct drm_device *dev, "unref cursor", unref_cursor_worker);
drm_crtc_helper_add(crtc, &mdp5_crtc_helper_funcs); - plane->crtc = crtc;
return crtc; } diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c index f2361f79fdce..6826aa10f3ac 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c @@ -1043,8 +1043,6 @@ static int mdp5_plane_mode_set(struct drm_plane *plane, src_img_w, src_img_h, src_x + src_w, src_y, src_w, src_h);
- plane->fb = fb; - return ret; }
From: Ville Syrjälä ville.syrjala@linux.intel.com
We want to get rid of plane->crtc on atomic drivers. Stop setting it.
v2: s/fb/crtc/ in the commit message (Gerd)
Cc: David Airlie airlied@linux.ie Cc: Gerd Hoffmann kraxel@redhat.com Cc: virtualization@lists.linux-foundation.org Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/virtio/virtgpu_display.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c index d6dd769a7ad3..ff9933e79416 100644 --- a/drivers/gpu/drm/virtio/virtgpu_display.c +++ b/drivers/gpu/drm/virtio/virtgpu_display.c @@ -282,8 +282,6 @@ static int vgdev_output_init(struct virtio_gpu_device *vgdev, int index) drm_crtc_init_with_planes(dev, crtc, primary, cursor, &virtio_gpu_crtc_funcs, NULL); drm_crtc_helper_add(crtc, &virtio_gpu_crtc_helper_funcs); - primary->crtc = crtc; - cursor->crtc = crtc;
drm_connector_init(dev, connector, &virtio_gpu_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL);
From: Ville Syrjälä ville.syrjala@linux.intel.com
We want to get rid of plane->fb/crtc on atomic drivers. Stop setting them.
Cc: Eric Anholt eric@anholt.net Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/vc4/vc4_crtc.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index c8650bbcbcb3..dcadf793ee80 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -862,7 +862,6 @@ static int vc4_async_page_flip(struct drm_crtc *crtc, * is released. */ drm_atomic_set_fb_for_plane(plane->state, fb); - plane->fb = fb;
vc4_queue_seqno_cb(dev, &flip_state->cb, bo->seqno, vc4_async_page_flip_complete); @@ -1057,7 +1056,6 @@ static int vc4_crtc_bind(struct device *dev, struct device *master, void *data) drm_crtc_init_with_planes(drm, crtc, primary_plane, NULL, &vc4_crtc_funcs, NULL); drm_crtc_helper_add(crtc, &vc4_crtc_helper_funcs); - primary_plane->crtc = crtc; vc4_crtc->channel = vc4_crtc->data->hvs_channel; drm_mode_crtc_set_gamma_size(crtc, ARRAY_SIZE(vc4_crtc->lut_r)); drm_crtc_enable_color_mgmt(crtc, 0, false, crtc->gamma_size); @@ -1093,7 +1091,6 @@ static int vc4_crtc_bind(struct device *dev, struct device *master, void *data) cursor_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_CURSOR); if (!IS_ERR(cursor_plane)) { cursor_plane->possible_crtcs = 1 << drm_crtc_index(crtc); - cursor_plane->crtc = crtc; crtc->cursor = cursor_plane; }
Ville Syrjala ville.syrjala@linux.intel.com writes:
From: Ville Syrjälä ville.syrjala@linux.intel.com
We want to get rid of plane->fb/crtc on atomic drivers. Stop setting them.
Cc: Eric Anholt eric@anholt.net Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Reviewed-by: Eric Anholt eric@anholt.net
From: Ville Syrjälä ville.syrjala@linux.intel.com
Stop playing around with plane->crtc/fb/old_fb with atomic drivers. Make life a lot simpler when we don't have to do the magic old_fb vs. fb dance around plane updates. That way we can't risk plane->fb getting out of sync with plane->state->fb and we're less likely to leak any refcounts as well.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Acked-by: Harry Wentland harry.wentland@amd.com --- drivers/gpu/drm/drm_atomic.c | 55 ++++--------------------------------- drivers/gpu/drm/drm_atomic_helper.c | 15 +--------- drivers/gpu/drm/drm_crtc.c | 8 ++++-- drivers/gpu/drm/drm_fb_helper.c | 7 ----- drivers/gpu/drm/drm_framebuffer.c | 5 ---- drivers/gpu/drm/drm_plane.c | 14 ++++++---- drivers/gpu/drm/drm_plane_helper.c | 4 ++- include/drm/drm_atomic.h | 3 -- 8 files changed, 24 insertions(+), 87 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 41334d0973a2..ee4b43b9404e 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -700,6 +700,11 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,
WARN_ON(!state->acquire_ctx);
+ /* the legacy pointers should never be set */ + WARN_ON(plane->fb); + WARN_ON(plane->old_fb); + WARN_ON(plane->crtc); + plane_state = drm_atomic_get_existing_plane_state(state, plane); if (plane_state) return plane_state; @@ -2051,45 +2056,6 @@ int drm_atomic_set_property(struct drm_atomic_state *state, return ret; }
-/** - * 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 &drm_plane.old_fb is set to &drm_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_get(new_fb); - plane->fb = new_fb; - plane->crtc = plane->state->crtc; - - if (plane->old_fb) - drm_framebuffer_put(plane->old_fb); - } - plane->old_fb = NULL; - } -} -EXPORT_SYMBOL(drm_atomic_clean_old_fb); - /** * DOC: explicit fencing properties * @@ -2310,9 +2276,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, unsigned int copied_objs, copied_props; struct drm_atomic_state *state; struct drm_modeset_acquire_ctx ctx; - struct drm_plane *plane; struct drm_out_fence_state *fence_state; - unsigned plane_mask; int ret = 0; unsigned int i, j, num_fences;
@@ -2352,7 +2316,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
retry: - plane_mask = 0; copied_objs = 0; copied_props = 0; fence_state = NULL; @@ -2423,12 +2386,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, copied_props++; }
- if (obj->type == DRM_MODE_OBJECT_PLANE && count_props && - !(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_put(obj); }
@@ -2449,8 +2406,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 130da5195f3b..232fa11a5e31 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2914,7 +2914,6 @@ static int __drm_atomic_helper_disable_all(struct drm_device *dev, struct drm_plane *plane; struct drm_crtc_state *crtc_state; struct drm_crtc *crtc; - unsigned plane_mask = 0; int ret, i;
state = drm_atomic_state_alloc(dev); @@ -2957,17 +2956,10 @@ static int __drm_atomic_helper_disable_all(struct drm_device *dev, goto free;
drm_atomic_set_fb_for_plane(plane_state, NULL); - - if (clean_old_fbs) { - plane->old_fb = plane->fb; - plane_mask |= BIT(drm_plane_index(plane)); - } }
ret = drm_atomic_commit(state); free: - if (plane_mask) - drm_atomic_clean_old_fb(dev, plane_mask, ret); drm_atomic_state_put(state); return ret; } @@ -3129,13 +3121,8 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
state->acquire_ctx = ctx;
- for_each_new_plane_in_state(state, plane, new_plane_state, i) { - WARN_ON(plane->crtc != new_plane_state->crtc); - WARN_ON(plane->fb != new_plane_state->fb); - WARN_ON(plane->old_fb); - + for_each_new_plane_in_state(state, plane, new_plane_state, i) state->planes[i].old_state = plane->state; - }
for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) state->crtcs[i].old_state = crtc->state; diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 237bd34212db..53828fc8d911 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -479,8 +479,12 @@ static int __drm_mode_set_config_internal(struct drm_mode_set *set,
ret = crtc->funcs->set_config(set, ctx); if (ret == 0) { - crtc->primary->crtc = fb ? crtc : NULL; - crtc->primary->fb = fb; + struct drm_plane *plane = crtc->primary; + + if (!plane->state) { + plane->crtc = fb ? crtc : NULL; + plane->fb = fb; + } }
drm_for_each_crtc(tmp, crtc->dev) { diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 2ee1eaa66188..6964fe99f8b1 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -368,7 +368,6 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ struct drm_plane *plane; struct drm_atomic_state *state; int i, ret; - unsigned int plane_mask; struct drm_modeset_acquire_ctx ctx;
drm_modeset_acquire_init(&ctx, 0); @@ -381,7 +380,6 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ
state->acquire_ctx = &ctx; retry: - plane_mask = 0; drm_for_each_plane(plane, dev) { plane_state = drm_atomic_get_plane_state(state, plane); if (IS_ERR(plane_state)) { @@ -391,9 +389,6 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ
plane_state->rotation = DRM_MODE_ROTATE_0;
- plane->old_fb = plane->fb; - plane_mask |= 1 << drm_plane_index(plane); - /* disable non-primary: */ if (plane->type == DRM_PLANE_TYPE_PRIMARY) continue; @@ -430,8 +425,6 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ ret = drm_atomic_commit(state);
out_state: - drm_atomic_clean_old_fb(dev, plane_mask, ret); - if (ret == -EDEADLK) goto backoff;
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index bfedceff87bb..46b11e46edbd 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -836,8 +836,6 @@ static int atomic_remove_fb(struct drm_framebuffer *fb) goto unlock;
plane_mask |= BIT(drm_plane_index(plane)); - - plane->old_fb = plane->fb; }
/* This list is only filled when disable_crtcs is set. */ @@ -852,9 +850,6 @@ static int atomic_remove_fb(struct drm_framebuffer *fb) ret = drm_atomic_commit(state);
unlock: - if (plane_mask) - drm_atomic_clean_old_fb(dev, plane_mask, ret); - if (ret == -EDEADLK) { drm_atomic_state_clear(state); drm_modeset_backoff(&ctx); diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 035054455301..143041666096 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -650,9 +650,11 @@ static int __setplane_internal(struct drm_plane *plane, crtc_x, crtc_y, crtc_w, crtc_h, src_x, src_y, src_w, src_h, ctx); if (!ret) { - plane->crtc = crtc; - plane->fb = fb; - drm_framebuffer_get(plane->fb); + if (!plane->state) { + plane->crtc = crtc; + plane->fb = fb; + drm_framebuffer_get(plane->fb); + } } else { plane->old_fb = NULL; } @@ -1092,8 +1094,10 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, /* Keep the old fb, don't unref it. */ plane->old_fb = NULL; } else { - plane->fb = fb; - drm_framebuffer_get(fb); + if (!plane->state) { + plane->fb = fb; + drm_framebuffer_get(fb); + } }
out: diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index f88f68161519..2010794943bc 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -502,6 +502,7 @@ EXPORT_SYMBOL(drm_plane_helper_update); int drm_plane_helper_disable(struct drm_plane *plane) { struct drm_plane_state *plane_state; + struct drm_framebuffer *old_fb;
/* crtc helpers love to call disable functions for already disabled hw * functions. So cope with that. */ @@ -521,8 +522,9 @@ int drm_plane_helper_disable(struct drm_plane *plane) plane_state->plane = plane;
plane_state->crtc = NULL; + old_fb = plane_state->fb; drm_atomic_set_fb_for_plane(plane_state, NULL);
- return drm_plane_helper_commit(plane, plane_state, plane->fb); + return drm_plane_helper_commit(plane, plane_state, old_fb); } EXPORT_SYMBOL(drm_plane_helper_disable); diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index a57a8aa90ffb..ca461b6cf71f 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -601,9 +601,6 @@ int __must_check drm_atomic_add_affected_planes(struct drm_atomic_state *state, struct drm_crtc *crtc);
-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);
Thanks Ville.
This series: Reviewed-by: Sinclair Yeh syeh@vmware.com
On Fri, May 25, 2018 at 09:50:32PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Here are again the last (?) bits of eliminating the plane->fb/crtc usage for atomic drivers. I've pushed everything else (thanks to everyone who reviewed them).
Deepak said he'd tested the vmwgfx stuff, so I think it should be safe to land. Just missing a bit of review...
Cc: Alex Deucher alexander.deucher@amd.com Cc: amd-gfx@lists.freedesktop.org Cc: "Christian König" christian.koenig@amd.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: David Airlie airlied@linux.ie Cc: "David (ChunMing) Zhou" David1.Zhou@amd.com Cc: Deepak Rawat drawat@vmware.com Cc: Eric Anholt eric@anholt.net Cc: freedreno@lists.freedesktop.org Cc: Gerd Hoffmann kraxel@redhat.com Cc: Harry Wentland harry.wentland@amd.com Cc: Inki Dae inki.dae@samsung.com Cc: Joonyoung Shim jy0922.shim@samsung.com Cc: Kyungmin Park kyungmin.park@samsung.com Cc: linux-arm-msm@vger.kernel.org Cc: Rob Clark robdclark@gmail.com Cc: Seung-Woo Kim sw0312.kim@samsung.com Cc: Sinclair Yeh syeh@vmware.com Cc: Thomas Hellstrom thellstrom@vmware.com Cc: virtualization@lists.linux-foundation.org Cc: VMware Graphics linux-graphics-maintainer@vmware.com
Ville Syrjälä (13): drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset() drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty() drm/vmwgfx: Stop using plane->fb in vmw_kms_update_implicit_fb() drm/vmwgfx: Stop updating plane->fb drm/vmwgfx: Stop using plane->fb in atomic_enable() drm/vmwgfx: Stop messing about with plane->fb/old_fb/crtc drm/amdgpu/dc: Stop updating plane->fb drm/i915: Stop updating plane->fb/crtc drm/exynos: Stop updating plane->crtc drm/msm: Stop updating plane->fb/crtc drm/virtio: Stop updating plane->crtc drm/vc4: Stop updating plane->fb/crtc drm: Stop updating plane->crtc/fb/old_fb on atomic drivers
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 - drivers/gpu/drm/drm_atomic.c | 55 +++-------------------- drivers/gpu/drm/drm_atomic_helper.c | 15 +------ drivers/gpu/drm/drm_crtc.c | 8 +++- drivers/gpu/drm/drm_fb_helper.c | 7 --- drivers/gpu/drm/drm_framebuffer.c | 5 --- drivers/gpu/drm/drm_plane.c | 14 +++--- drivers/gpu/drm/drm_plane_helper.c | 4 +- drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 - drivers/gpu/drm/i915/intel_atomic_plane.c | 12 ----- drivers/gpu/drm/i915/intel_display.c | 7 ++- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 - drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 2 - drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 - drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 2 - drivers/gpu/drm/vc4/vc4_crtc.c | 3 -- drivers/gpu/drm/virtio/virtgpu_display.c | 2 - drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 24 ---------- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 24 +++++++--- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 - drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 5 +-- include/drm/drm_atomic.h | 3 -- 22 files changed, 46 insertions(+), 154 deletions(-)
-- 2.16.1
dri-devel@lists.freedesktop.org
-
Eric Anholt -
Sinclair Yeh -
Ville Syrjala -
Ville Syrjälä