[PATCH 1/3] drm: Add infrastructure to allow seamless mode switches

[Srinivas, Vidya]

Hello Jose,

Thanks much for the patch. I tested it on chrome system and the patch works.
Adding my Tested-by.
Tested-by: Vidya Srinivas <vidya.srinivas at intel.com>

Regards
Vidya

> -----Original Message-----
> From: Souza, Jose <jose.souza at intel.com>
> Sent: Friday, April 22, 2022 12:52 AM
> To: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
> Cc: Srinivas, Vidya <vidya.srinivas at intel.com>; Sean Paul
> <seanpaul at chromium.org>; Ville Syrjälä <ville.syrjala at linux.intel.com>;
> Souza, Jose <jose.souza at intel.com>
> Subject: [PATCH 1/3] drm: Add infrastructure to allow seamless mode
> switches
> 
> Intel hardware supports change between modes with different refresh rates
> without any glitches or visual artifacts, that feature is called seamless DRRS.
> 
> So far i915 driver was automatically changing between preferred panel mode
> and lower refresh rate mode based on idleness but ChromeOS compositor
> team is requesting to be in control of the mode switch.
> So for a certain types of content it can switch to mode with a lower refresh
> rate without user noticing a thing and saving power.
> 
> This seamless mode switch will be triggered when user-space dispatch a
> atomic commit with the new mode and clears the
> DRM_MODE_ATOMIC_ALLOW_MODESET flag.
> 
> A driver that don't implement atomic_seamless_mode_switch_check
> function will continue to fail in the atomic check phase with "[CRTC:%d:%s]
> requires full modeset" debug message.
> While a driver that implements it and support the seamless change between
> old and new mode will return 0 otherwise it should return the appropried
> errno.
> 
> So here adding basic drm infrastructure to that be supported by i915 and
> other drivers.
> 
> Cc: Vidya Srinivas <vidya.srinivas at intel.com>
> Cc: Sean Paul <seanpaul at chromium.org>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c              |  1 +
>  drivers/gpu/drm/drm_atomic_helper.c       | 16 +++++++++++++++
>  drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
>  include/drm/drm_crtc.h                    | 25 +++++++++++++++++++++++
>  4 files changed, 43 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 58c0283fb6b0c..21525f9f4b4c1 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -437,6 +437,7 @@ static void drm_atomic_crtc_print_state(struct
> drm_printer *p,
>  	drm_printf(p, "\tself_refresh_active=%d\n", state-
> >self_refresh_active);
>  	drm_printf(p, "\tplanes_changed=%d\n", state->planes_changed);
>  	drm_printf(p, "\tmode_changed=%d\n", state->mode_changed);
> +	drm_printf(p, "\tseamless_mode_changed=%d\n",
> +state->seamless_mode_changed);
>  	drm_printf(p, "\tactive_changed=%d\n", state->active_changed);
>  	drm_printf(p, "\tconnectors_changed=%d\n", state-
> >connectors_changed);
>  	drm_printf(p, "\tcolor_mgmt_changed=%d\n", state-
> >color_mgmt_changed); diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 9603193d2fa13..e6f3a966f7b86 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -631,6 +631,22 @@ drm_atomic_helper_check_modeset(struct
> drm_device *dev,
>  			drm_dbg_atomic(dev, "[CRTC:%d:%s] mode
> changed\n",
>  				       crtc->base.id, crtc->name);
>  			new_crtc_state->mode_changed = true;
> +
> +			if (!state->allow_modeset &&
> +			    crtc->funcs-
> >atomic_seamless_mode_switch_check) {
> +				ret = crtc->funcs-
> >atomic_seamless_mode_switch_check(state, crtc);
> +				if (ret == -EOPNOTSUPP) {
> +					drm_dbg_atomic(dev, "[CRTC:%d:%s]
> Seamless mode switch not supported\n",
> +						       crtc->base.id, crtc-
> >name);
> +					return ret;
> +				}
> +
> +				if (ret < 0)
> +					return ret;
> +
> +				new_crtc_state->seamless_mode_changed =
> true;
> +				new_crtc_state->mode_changed = false;
> +			}
>  		}
> 
>  		if (old_crtc_state->enable != new_crtc_state->enable) { diff --
> git a/drivers/gpu/drm/drm_atomic_state_helper.c
> b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 3b6d3bdbd0996..c093073ea6e11 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -142,6 +142,7 @@ void
> __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  	if (state->gamma_lut)
>  		drm_property_blob_get(state->gamma_lut);
>  	state->mode_changed = false;
> +	state->seamless_mode_changed = false;
>  	state->active_changed = false;
>  	state->planes_changed = false;
>  	state->connectors_changed = false;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index
> a70baea0636ca..b7ce378d679d3 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -140,6 +140,16 @@ struct drm_crtc_state {
>  	 */
>  	bool mode_changed : 1;
> 
> +	/**
> +	 * @seamless_mode_changed: @mode has been changed but user-
> space
> +	 * is requesting to change to the new mode with a fastset and driver
> +	 * supports this request.
> +	 * To be used by drivers to steer the atomic commit control flow to
> +	 * appropriate paths to change mode without any visual corruption.
> +	 * Never set together with @mode_changed.
> +	 */
> +	bool seamless_mode_changed : 1;
> +
>  	/**
>  	 * @active_changed: @active has been toggled. Used by the atomic
>  	 * helpers and drivers to steer the atomic commit control flow. See
> also @@ -939,6 +949,21 @@ struct drm_crtc_funcs {
>  				     int *max_error,
>  				     ktime_t *vblank_time,
>  				     bool in_vblank_irq);
> +
> +	/**
> +	 * @atomic_seamless_mode_switch_check
> +	 *
> +	 * Called when user-space wants to change mode without do a
> modeset.
> +	 * Drivers can optionally support do a mode switch without any visual
> +	 * corruption when changing between certain modes.
> +	 *
> +	 * Returns:
> +	 * Zero if possible to seamless switch mode, -EOPNOTSUPP if not
> +	 * supported seamless mode change or appropriate errno if an error
> +	 * happened.
> +	 */
> +	int (*atomic_seamless_mode_switch_check)(struct
> drm_atomic_state *state,
> +						 struct drm_crtc *crtc);
>  };
> 
>  /**
> --
> 2.36.0