[v7,2/4] drm/i915: Save the old CDCLK atomic state

Submitted by Imre Deak on March 19, 2019, 6:36 p.m.

Details

Message ID 20190319183618.22162-3-imre.deak@intel.com
State New
Headers show
Series "drm/i915: Ensure minimum CDCLK requirement for audio" ( rev: 2 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Imre Deak March 19, 2019, 6:36 p.m.
The old state will be needed by an upcoming patch to determine if the
commit increases or decreases CDCLK, so move the old state to the atomic
state (while keeping the new one in dev_priv). cdclk.logical and
cdclk.actual in the atomic state isn't used atm anywhere after the
atomic check phase, so this should be safe.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c   | 26 ++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  4 ++--
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 3 files changed, 29 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 7dcca84f31d1..97007d76f9a7 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2100,6 +2100,32 @@  bool intel_cdclk_changed(const struct intel_cdclk_state *a,
 		a->voltage_level != b->voltage_level;
 }
 
+/**
+ * intel_cdclk_swap_state - make atomic CDCLK configuration effective
+ * @state: atomic state
+ *
+ * This is the CDCLK version of drm_atomic_helper_swap_state() since the
+ * helper does not handle driver-specific global state.
+ *
+ * Similarly to the atomic helpers this function does a complete swap,
+ * i.e. it also puts the old state into @state. This is used by the commit
+ * code to determine how CDCLK has changed (for instance did it increase or
+ * decrease).
+ */
+void intel_cdclk_swap_state(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_cdclk_state tmp;
+
+	tmp = state->cdclk.logical;
+	state->cdclk.logical = dev_priv->cdclk.logical;
+	dev_priv->cdclk.logical = tmp;
+
+	tmp = state->cdclk.actual;
+	state->cdclk.actual = dev_priv->cdclk.actual;
+	dev_priv->cdclk.actual = tmp;
+}
+
 void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
 			    const char *context)
 {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b4199cd53349..9c4ad124302c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13663,10 +13663,10 @@  static int intel_atomic_commit(struct drm_device *dev,
 		       intel_state->min_voltage_level,
 		       sizeof(intel_state->min_voltage_level));
 		dev_priv->active_crtcs = intel_state->active_crtcs;
-		dev_priv->cdclk.logical = intel_state->cdclk.logical;
-		dev_priv->cdclk.actual = intel_state->cdclk.actual;
 		dev_priv->cdclk.force_min_cdclk =
 			intel_state->cdclk.force_min_cdclk;
+
+		intel_cdclk_swap_state(intel_state);
 	}
 
 	drm_atomic_state_get(state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0b84e557c267..85dd6a9d1e42 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1698,6 +1698,7 @@  bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
 			       const struct intel_cdclk_state *b);
 bool intel_cdclk_changed(const struct intel_cdclk_state *a,
 			 const struct intel_cdclk_state *b);
+void intel_cdclk_swap_state(struct intel_atomic_state *state);
 void intel_set_cdclk(struct drm_i915_private *dev_priv,
 		     const struct intel_cdclk_state *cdclk_state);
 void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,

Comments

On Tue, Mar 19, 2019 at 08:36:16PM +0200, Imre Deak wrote:
> The old state will be needed by an upcoming patch to determine if the
> commit increases or decreases CDCLK, so move the old state to the atomic
> state (while keeping the new one in dev_priv). cdclk.logical and
> cdclk.actual in the atomic state isn't used atm anywhere after the
> atomic check phase, so this should be safe.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_cdclk.c   | 26 ++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  4 ++--
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  3 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 7dcca84f31d1..97007d76f9a7 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2100,6 +2100,32 @@ bool intel_cdclk_changed(const struct intel_cdclk_state *a,
>  		a->voltage_level != b->voltage_level;
>  }
>  
> +/**
> + * intel_cdclk_swap_state - make atomic CDCLK configuration effective
> + * @state: atomic state
> + *
> + * This is the CDCLK version of drm_atomic_helper_swap_state() since the
> + * helper does not handle driver-specific global state.
> + *
> + * Similarly to the atomic helpers this function does a complete swap,
> + * i.e. it also puts the old state into @state. This is used by the commit
> + * code to determine how CDCLK has changed (for instance did it increase or
> + * decrease).
> + */
> +void intel_cdclk_swap_state(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	struct intel_cdclk_state tmp;
> +
> +	tmp = state->cdclk.logical;
> +	state->cdclk.logical = dev_priv->cdclk.logical;
> +	dev_priv->cdclk.logical = tmp;
> +
> +	tmp = state->cdclk.actual;
> +	state->cdclk.actual = dev_priv->cdclk.actual;
> +	dev_priv->cdclk.actual = tmp;

No love for swap()?

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +}
> +
>  void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
>  			    const char *context)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b4199cd53349..9c4ad124302c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13663,10 +13663,10 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		       intel_state->min_voltage_level,
>  		       sizeof(intel_state->min_voltage_level));
>  		dev_priv->active_crtcs = intel_state->active_crtcs;
> -		dev_priv->cdclk.logical = intel_state->cdclk.logical;
> -		dev_priv->cdclk.actual = intel_state->cdclk.actual;
>  		dev_priv->cdclk.force_min_cdclk =
>  			intel_state->cdclk.force_min_cdclk;
> +
> +		intel_cdclk_swap_state(intel_state);
>  	}
>  
>  	drm_atomic_state_get(state);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0b84e557c267..85dd6a9d1e42 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1698,6 +1698,7 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
>  			       const struct intel_cdclk_state *b);
>  bool intel_cdclk_changed(const struct intel_cdclk_state *a,
>  			 const struct intel_cdclk_state *b);
> +void intel_cdclk_swap_state(struct intel_atomic_state *state);
>  void intel_set_cdclk(struct drm_i915_private *dev_priv,
>  		     const struct intel_cdclk_state *cdclk_state);
>  void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
> -- 
> 2.13.2
On Tue, Mar 19, 2019 at 08:51:25PM +0200, Ville Syrjälä wrote:
> On Tue, Mar 19, 2019 at 08:36:16PM +0200, Imre Deak wrote:
> > The old state will be needed by an upcoming patch to determine if the
> > commit increases or decreases CDCLK, so move the old state to the atomic
> > state (while keeping the new one in dev_priv). cdclk.logical and
> > cdclk.actual in the atomic state isn't used atm anywhere after the
> > atomic check phase, so this should be safe.
> > 
> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_cdclk.c   | 26 ++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_display.c |  4 ++--
> >  drivers/gpu/drm/i915/intel_drv.h     |  1 +
> >  3 files changed, 29 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 7dcca84f31d1..97007d76f9a7 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -2100,6 +2100,32 @@ bool intel_cdclk_changed(const struct intel_cdclk_state *a,
> >  		a->voltage_level != b->voltage_level;
> >  }
> >  
> > +/**
> > + * intel_cdclk_swap_state - make atomic CDCLK configuration effective
> > + * @state: atomic state
> > + *
> > + * This is the CDCLK version of drm_atomic_helper_swap_state() since the
> > + * helper does not handle driver-specific global state.
> > + *
> > + * Similarly to the atomic helpers this function does a complete swap,
> > + * i.e. it also puts the old state into @state. This is used by the commit
> > + * code to determine how CDCLK has changed (for instance did it increase or
> > + * decrease).
> > + */
> > +void intel_cdclk_swap_state(struct intel_atomic_state *state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > +	struct intel_cdclk_state tmp;
> > +
> > +	tmp = state->cdclk.logical;
> > +	state->cdclk.logical = dev_priv->cdclk.logical;
> > +	dev_priv->cdclk.logical = tmp;
> > +
> > +	tmp = state->cdclk.actual;
> > +	state->cdclk.actual = dev_priv->cdclk.actual;
> > +	dev_priv->cdclk.actual = tmp;
> 
> No love for swap()?

Didn't realize I can also use it for structs.. will fix that.

> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > +}
> > +
> >  void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
> >  			    const char *context)
> >  {
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index b4199cd53349..9c4ad124302c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13663,10 +13663,10 @@ static int intel_atomic_commit(struct drm_device *dev,
> >  		       intel_state->min_voltage_level,
> >  		       sizeof(intel_state->min_voltage_level));
> >  		dev_priv->active_crtcs = intel_state->active_crtcs;
> > -		dev_priv->cdclk.logical = intel_state->cdclk.logical;
> > -		dev_priv->cdclk.actual = intel_state->cdclk.actual;
> >  		dev_priv->cdclk.force_min_cdclk =
> >  			intel_state->cdclk.force_min_cdclk;
> > +
> > +		intel_cdclk_swap_state(intel_state);
> >  	}
> >  
> >  	drm_atomic_state_get(state);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 0b84e557c267..85dd6a9d1e42 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1698,6 +1698,7 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
> >  			       const struct intel_cdclk_state *b);
> >  bool intel_cdclk_changed(const struct intel_cdclk_state *a,
> >  			 const struct intel_cdclk_state *b);
> > +void intel_cdclk_swap_state(struct intel_atomic_state *state);
> >  void intel_set_cdclk(struct drm_i915_private *dev_priv,
> >  		     const struct intel_cdclk_state *cdclk_state);
> >  void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
> > -- 
> > 2.13.2
> 
> -- 
> Ville Syrjälä
> Intel