diff mbox

[7/7] drm/vmwgfx: Stop messing about with plane->fb/old_fb/crtc

Message ID 20180405195035.24722-7-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä April 5, 2018, 7:50 p.m. UTC
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.

TODO: Squash with the core/helper patch?

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(-)

Comments

Daniel Vetter April 5, 2018, 8:39 p.m. UTC | #1
On Thu, Apr 05, 2018 at 10:50:35PM +0300, Ville Syrjala wrote:
> 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.
> 
> TODO: Squash with the core/helper patch?

Not possible, because the core setconfig one does no longer work with
atomic drivers ever since I've thrown away the magic backoff. Since then
we have the rule that for any atomic driver, you need a real atomic commit
(including passing the acquire_ctx down to that very call to
drm_atomic_commit).

See the patch that introduced this copypasta for full details plus how to
fix it up properly:

commit 3bacf4361cd07f988a13de78d672928606df24ad
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Apr 6 22:02:56 2017 +0200

    drm/vmwgfx: Fix fbdev emulation using legacy functions
    
    I've broken this by removing the backoff handling from the
    set_config2atomic helper in
    
    commit 38b6441e4e75c0b319cfe4d9364c1059fc1e3c2b
    Author: Daniel Vetter <daniel.vetter@ffwll.ch>
    Date:   Wed Mar 22 22:50:58 2017 +0100
    
        drm/atomic-helper: Remove the backoff hack from set_config
    
    Fixing this properly would mean we get to wire the acquire_ctx all the
    way through vmwgfx fbdev code, and doing the same was tricky for the
    shared fbdev layer. Probably much better to look into refactoring the
    entire code to use the helpers, but since that's not a viable
    long-term solution fix the issue by open-coding a vmwgfx version of
    set_config, that does the legacy backoff dance internally.
    
    Note: Just compile-tested. The idea is to take
    drm_mode_set_config_internal(), remove the "is this a legacy driver"
    check, and whack the drm_atomic_legacy_backoff trickery at the end.
    Since drm_atomic_legacy_backoff is for atomic commits only we need to
    open-code it.
    
    Cc: Thomas Hellstrom <thellstrom@vmware.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
    Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
    Signed-off-by: Sean Paul <seanpaul@chromium.org>
    Link: http://patchwork.freedesktop.org/patch/msgid/20170406200256.26040-1-daniel.vetter@ffwll.ch

I scrolled through your vmwgfx patches, and will try to look at them maybe
next week. Too complicated for this late, and tomorrow I'm ooo.
-Daniel

> 
> 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 2582ffd36bb5..3c5935f3d49e 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_modeset_acquire_ctx *ctx;
>  	struct drm_device *dev = set->crtc->dev;
>  	int ret;
> @@ -448,29 +446,7 @@ static int vmwgfx_set_config_internal(struct drm_mode_set *set)
>  	ctx = dev->mode_config.acquire_ctx;
>  
>  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) {
>  		dev->mode_config.acquire_ctx = NULL;
> -- 
> 2.16.1
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index 2582ffd36bb5..3c5935f3d49e 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_modeset_acquire_ctx *ctx;
 	struct drm_device *dev = set->crtc->dev;
 	int ret;
@@ -448,29 +446,7 @@  static int vmwgfx_set_config_internal(struct drm_mode_set *set)
 	ctx = dev->mode_config.acquire_ctx;
 
 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) {
 		dev->mode_config.acquire_ctx = NULL;