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

[Thomas Hellstrom]

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?

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;
>