diff mbox series

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

Message ID 20220421192205.32649-1-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/3] drm: Add infrastructure to allow seamless mode switches | expand

Commit Message

Souza, Jose April 21, 2022, 7:22 p.m. UTC
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@intel.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@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(+)

Comments

Srinivas, Vidya April 26, 2022, 4:53 p.m. UTC | #1
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@intel.com>

Regards
Vidya

> -----Original Message-----
> From: Souza, Jose <jose.souza@intel.com>
> Sent: Friday, April 22, 2022 12:52 AM
> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Srinivas, Vidya <vidya.srinivas@intel.com>; Sean Paul
> <seanpaul@chromium.org>; Ville Syrjälä <ville.syrjala@linux.intel.com>;
> Souza, Jose <jose.souza@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@intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@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
Ville Syrjälä April 26, 2022, 6:08 p.m. UTC | #2
On Thu, Apr 21, 2022 at 12:22:03PM -0700, José Roberto de Souza wrote:
> 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.

I don't see the need for any extra infrastructure for this.

I think we just need:
- fix the fastset code to not suck
- reprogram M/N during fastset
- calculate eDP link params using panel's highest refresh rate mode
  to make sure we get the same link params for all refresh rates

> 
> Cc: Vidya Srinivas <vidya.srinivas@intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@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
Souza, Jose April 26, 2022, 6:32 p.m. UTC | #3
On Tue, 2022-04-26 at 21:08 +0300, Ville Syrjälä wrote:
> On Thu, Apr 21, 2022 at 12:22:03PM -0700, José Roberto de Souza wrote:
> > 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.
> 
> I don't see the need for any extra infrastructure for this.
> 
> I think we just need:
> - fix the fastset code to not suck

How would it know that only mode changed and not all other things that causes mode_changed to be set?
For example: intel_digital_connector_atomic_check()

> - reprogram M/N during fastset
> - calculate eDP link params using panel's highest refresh rate mode
>   to make sure we get the same link params for all refresh rates
> 
> > 
> > Cc: Vidya Srinivas <vidya.srinivas@intel.com>
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@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
>
Ville Syrjälä April 26, 2022, 6:44 p.m. UTC | #4
On Tue, Apr 26, 2022 at 06:32:01PM +0000, Souza, Jose wrote:
> On Tue, 2022-04-26 at 21:08 +0300, Ville Syrjälä wrote:
> > On Thu, Apr 21, 2022 at 12:22:03PM -0700, José Roberto de Souza wrote:
> > > 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.
> > 
> > I don't see the need for any extra infrastructure for this.
> > 
> > I think we just need:
> > - fix the fastset code to not suck
> 
> How would it know that only mode changed and not all other things that causes mode_changed to be set?
> For example: intel_digital_connector_atomic_check()

That's what the fastset stuff does. It checks if anything changes
that needs a full modeset or not.

> 
> > - reprogram M/N during fastset
> > - calculate eDP link params using panel's highest refresh rate mode
> >   to make sure we get the same link params for all refresh rates
> > 
> > > 
> > > Cc: Vidya Srinivas <vidya.srinivas@intel.com>
> > > Cc: Sean Paul <seanpaul@chromium.org>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@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
> > 
>
diff mbox series

Patch

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);
 };
 
 /**