[PATCH] drm/vmwgfx: Fix fbdev emulation using legacy functions

[Sean Paul]

On Fri, Apr 07, 2017 at 10:31:29AM +0200, Thomas Hellstrom wrote:
> Hi, Daniel.
> 
> It appears to work fine. Thanks!
> 
> Do you want to take it through drm-misc or want me to take it throuch
> vmwgfx-next?

Applied to drm-misc

Thanks,

Sean

> 
> Reviewed-by: Thomas Hellstrom <thellstrom at vmware.com>
> /Thomas
> 
> 
> 
> On 04/06/2017 10:02 PM, Daniel Vetter wrote:
> > I've broken this by removing the backoff handling from the
> > set_config2atomic helper in
> >
> > commit 38b6441e4e75c0b319cfe4d9364c1059fc1e3c2b
> > Author: Daniel Vetter <daniel.vetter at 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 at vmware.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > ---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 58 ++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 56 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> > index 09e120d50e65..6f4cb4678cbc 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> > @@ -418,6 +418,60 @@ static int vmw_fb_compute_depth(struct fb_var_screeninfo *var,
> >  	return 0;
> >  }
> >  
> > +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;
> > +
> > +	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;
> > +
> > +retry_locking:
> > +		drm_modeset_backoff(ctx);
> > +
> > +		ret = drm_modeset_lock_all_ctx(dev, ctx);
> > +		if (ret)
> > +			goto retry_locking;
> > +
> > +		dev->mode_config.acquire_ctx = ctx;
> > +
> > +		goto restart;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> >  static int vmw_fb_kms_detach(struct vmw_fb_par *par,
> >  			     bool detach_bo,
> >  			     bool unref_bo)
> > @@ -436,7 +490,7 @@ static int vmw_fb_kms_detach(struct vmw_fb_par *par,
> >  		set.fb = NULL;
> >  		set.num_connectors = 0;
> >  		set.connectors = &par->con;
> > -		ret = drm_mode_set_config_internal(&set);
> > +		ret = vmwgfx_set_config_internal(&set);
> >  		if (ret) {
> >  			DRM_ERROR("Could not unset a mode.\n");
> >  			return ret;
> > @@ -578,7 +632,7 @@ static int vmw_fb_set_par(struct fb_info *info)
> >  	set.num_connectors = 1;
> >  	set.connectors = &par->con;
> >  
> > -	ret = drm_mode_set_config_internal(&set);
> > +	ret = vmwgfx_set_config_internal(&set);
> >  	if (ret)
> >  		goto out_unlock;
> >  
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS