[Intel-gfx] [PATCH 2/4] drm/fbdev-helper: don't force restores

[Noralf Trønnes]

Den 28.01.2020 11.45, skrev Daniel Vetter:
> Instead check for master status, in case we've raced.
> 
> This is the last exception to the general rule that we restore fbcon
> only when there's no master active. Compositors are supposed to drop
> their master status before they switch to a different console back to
> text mode (or just switch to text mode directly, without a vt switch).
> 
> This is known to break some subtests of kms_fbcon_fbt in igt, but they're
> just wrong - it does a graphics/text mode switch for the vt without
> updating the master status.
> 
> Also add a comment to the drm_client->restore hook that this is expected
> going forward from all clients (there's currently just one).
> 
> v2: Also drop the force in pan_display
> 
> Cc: Noralf Trønnes <noralf at tronnes.org>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 14 ++------------
>  include/drm/drm_client.h        |  5 +++++
>  2 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 4c7cbce7bae7..926187a82255 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -250,17 +250,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>  		return 0;
>  
>  	mutex_lock(&fb_helper->lock);
> -	/*
> -	 * TODO:
> -	 * We should bail out here if there is a master by dropping _force.
> -	 * Currently these igt tests fail if we do that:
> -	 * - kms_fbcon_fbt at psr
> -	 * - kms_fbcon_fbt at psr-suspend
> -	 *
> -	 * So first these tests need to be fixed so they drop master or don't
> -	 * have an fd open.
> -	 */
> -	ret = drm_client_modeset_commit_force(&fb_helper->client);
> +	ret = drm_client_modeset_commit(&fb_helper->client);
>  
>  	do_delayed = fb_helper->delayed_hotplug;
>  	if (do_delayed)
> @@ -1357,7 +1347,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
>  
>  	pan_set(fb_helper, var->xoffset, var->yoffset);
>  
> -	ret = drm_client_modeset_commit_force(&fb_helper->client);
> +	ret = drm_client_modeset_commit(&fb_helper->client);

This needs _force because drm_fb_helper_pan_display() already holds the
locks.

With that fixed:
Reviewed-by: Noralf Trønnes <noralf at tronnes.org>

Maybe a better and more descriptive name would have been
drm_client_modeset_commit_locked().

Noralf.

>  	if (!ret) {
>  		info->var.xoffset = var->xoffset;
>  		info->var.yoffset = var->yoffset;
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 5cf2c5dd8b1e..d01d311023ac 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -44,6 +44,11 @@ struct drm_client_funcs {
>  	 * returns zero gets the privilege to restore and no more clients are
>  	 * called. This callback is not called after @unregister has been called.
>  	 *
> +	 * Note that the core does not guarantee exclusion against concurrent
> +	 * drm_open(). Clients need to ensure this themselves, for example by
> +	 * using drm_master_internal_acquire() and
> +	 * drm_master_internal_release().
> +	 *
>  	 * This callback is optional.
>  	 */
>  	int (*restore)(struct drm_client_dev *client);
>