Re: [Intel-gfx] [PATCH 2/4] drm/i915/squash: s/intel_cdclk_can_squash/intel_cdclk_squash

2022-09-14 Thread Navare, Manasi
On Fri, Aug 19, 2022 at 05:58:20PM -0700, Anusha Srivatsa wrote:
> Apart from checking if squashing can be performed,
> accommodate accessing in-flight cdclk state for any changes
> that are needed during commit phase.
> 
> v2: Move squashing bits to switch case.(Anusha)
> 
> Cc: Jani Nikula 
> Signed-off-by: Anusha Srivatsa 
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 62 ++
>  1 file changed, 40 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c 
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 86a22c3766e5..f98fd48fe905 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1693,12 +1693,18 @@ static void bxt_set_cdclk(struct drm_i915_private 
> *dev_priv,
> const struct intel_cdclk_config *cdclk_config,
> enum pipe pipe)
>  {
> + struct intel_cdclk_state *cdclk_state = 
> to_intel_cdclk_state(dev_priv->cdclk.obj.state);
> + struct intel_atomic_state *state = cdclk_state->base.state;
> + struct intel_cdclk_state *new_cdclk_state = 
> intel_atomic_get_new_cdclk_state(state);

This will not work, make cdclk steps as part of cdlck_config struct and
access that through the passed cdclk_config

This already gets passed from the calling function intel_set_cdclk()
which is getting called from intel_set_cdclk_post_plane_update() from
commit_tail()
This is the correct way of programming req HW registers from cdclk state



Manasi

> + struct cdclk_step *cdclk_steps = new_cdclk_state->steps;
>   int cdclk = cdclk_config->cdclk;
>   int vco = cdclk_config->vco;
> + u32 squash_ctl = 0;
>   u32 val;
>   u16 waveform;
>   int clock;
>   int ret;
> + int i;
>  
>   /* Inform power controller of upcoming frequency change. */
>   if (DISPLAY_VER(dev_priv) >= 11)
> @@ -1742,21 +1748,27 @@ static void bxt_set_cdclk(struct drm_i915_private 
> *dev_priv,
>  
>   waveform = cdclk_squash_waveform(dev_priv, cdclk);
>  
> - if (waveform)
> + if (waveform && has_cdclk_squasher(dev_priv)) {
>   clock = vco / 2;
> - else
> + for (i = 0; i < MAX_CDCLK_ACTIONS; i++) {
> + switch (cdclk_steps[i].action) {
> + case INTEL_CDCLK_SQUASH:
> + waveform =  cdclk_squash_waveform(dev_priv, 
> cdclk_steps[i].cdclk);
> + squash_ctl = CDCLK_SQUASH_ENABLE |
> +  CDCLK_SQUASH_WINDOW_SIZE(0xf) | 
> waveform;
> + intel_de_write(dev_priv, CDCLK_SQUASH_CTL, 
> squash_ctl);
> + break;
> + case INTEL_CDCLK_NOOP:
> + case INTEL_CDCLK_CRAWL:
> + case INTEL_CDCLK_MODESET:
> + break;
> + default:
> + break;
> + }
> + }
> + } else
>   clock = cdclk;
>  
> - if (has_cdclk_squasher(dev_priv)) {
> - u32 squash_ctl = 0;
> -
> - if (waveform)
> - squash_ctl = CDCLK_SQUASH_ENABLE |
> - CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform;
> -
> - intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl);
> - }
> -
>   val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) |
>   bxt_cdclk_cd2x_pipe(dev_priv, pipe) |
>   skl_cdclk_decimal(cdclk);
> @@ -1966,10 +1978,11 @@ static bool intel_cdclk_can_crawl(struct 
> drm_i915_private *dev_priv,
>   a->ref == b->ref;
>  }
>  
> -static bool intel_cdclk_can_squash(struct drm_i915_private *dev_priv,
> -const struct intel_cdclk_config *a,
> -const struct intel_cdclk_config *b)
> +static bool intel_cdclk_squash(struct drm_i915_private *dev_priv,
> +const struct intel_cdclk_state *a,
> +struct intel_cdclk_state *b)
>  {
> + struct cdclk_step *cdclk_transition = b->steps;
>   /*
>* FIXME should store a bit more state in intel_cdclk_config
>* to differentiate squasher vs. cd2x divider properly. For
> @@ -1978,11 +1991,16 @@ static bool intel_cdclk_can_squash(struct 
> drm_i915_private *dev_priv,
>*/
>   if (!has_cdclk_squasher(dev_priv))
>   return false;
> + 
> + cdclk_transition[0].action = INTEL_CDCLK_SQUASH;
> + cdclk_transition[0].cdclk = b->actual.cdclk;
> + cdclk_transition[1].action = INTEL_CDCLK_NOOP;
> + cdclk_transition[1].cdclk = b->actual.cdclk;
>  
> - return a->cdclk != b->cdclk &&
> - a->vco != 0 &&
> - a->vco == b->vco &&
> - a->ref == b->ref;
> + return a->actual.cdclk != b->actual.cdclk &&
> + a->actual.vco != 0 

[Intel-gfx] [PATCH 2/4] drm/i915/squash: s/intel_cdclk_can_squash/intel_cdclk_squash

2022-08-19 Thread Anusha Srivatsa
Apart from checking if squashing can be performed,
accommodate accessing in-flight cdclk state for any changes
that are needed during commit phase.

v2: Move squashing bits to switch case.(Anusha)

Cc: Jani Nikula 
Signed-off-by: Anusha Srivatsa 
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 62 ++
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c 
b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 86a22c3766e5..f98fd48fe905 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1693,12 +1693,18 @@ static void bxt_set_cdclk(struct drm_i915_private 
*dev_priv,
  const struct intel_cdclk_config *cdclk_config,
  enum pipe pipe)
 {
+   struct intel_cdclk_state *cdclk_state = 
to_intel_cdclk_state(dev_priv->cdclk.obj.state);
+   struct intel_atomic_state *state = cdclk_state->base.state;
+   struct intel_cdclk_state *new_cdclk_state = 
intel_atomic_get_new_cdclk_state(state);
+   struct cdclk_step *cdclk_steps = new_cdclk_state->steps;
int cdclk = cdclk_config->cdclk;
int vco = cdclk_config->vco;
+   u32 squash_ctl = 0;
u32 val;
u16 waveform;
int clock;
int ret;
+   int i;
 
/* Inform power controller of upcoming frequency change. */
if (DISPLAY_VER(dev_priv) >= 11)
@@ -1742,21 +1748,27 @@ static void bxt_set_cdclk(struct drm_i915_private 
*dev_priv,
 
waveform = cdclk_squash_waveform(dev_priv, cdclk);
 
-   if (waveform)
+   if (waveform && has_cdclk_squasher(dev_priv)) {
clock = vco / 2;
-   else
+   for (i = 0; i < MAX_CDCLK_ACTIONS; i++) {
+   switch (cdclk_steps[i].action) {
+   case INTEL_CDCLK_SQUASH:
+   waveform =  cdclk_squash_waveform(dev_priv, 
cdclk_steps[i].cdclk);
+   squash_ctl = CDCLK_SQUASH_ENABLE |
+CDCLK_SQUASH_WINDOW_SIZE(0xf) | 
waveform;
+   intel_de_write(dev_priv, CDCLK_SQUASH_CTL, 
squash_ctl);
+   break;
+   case INTEL_CDCLK_NOOP:
+   case INTEL_CDCLK_CRAWL:
+   case INTEL_CDCLK_MODESET:
+   break;
+   default:
+   break;
+   }
+   }
+   } else
clock = cdclk;
 
-   if (has_cdclk_squasher(dev_priv)) {
-   u32 squash_ctl = 0;
-
-   if (waveform)
-   squash_ctl = CDCLK_SQUASH_ENABLE |
-   CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform;
-
-   intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl);
-   }
-
val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) |
bxt_cdclk_cd2x_pipe(dev_priv, pipe) |
skl_cdclk_decimal(cdclk);
@@ -1966,10 +1978,11 @@ static bool intel_cdclk_can_crawl(struct 
drm_i915_private *dev_priv,
a->ref == b->ref;
 }
 
-static bool intel_cdclk_can_squash(struct drm_i915_private *dev_priv,
-  const struct intel_cdclk_config *a,
-  const struct intel_cdclk_config *b)
+static bool intel_cdclk_squash(struct drm_i915_private *dev_priv,
+  const struct intel_cdclk_state *a,
+  struct intel_cdclk_state *b)
 {
+   struct cdclk_step *cdclk_transition = b->steps;
/*
 * FIXME should store a bit more state in intel_cdclk_config
 * to differentiate squasher vs. cd2x divider properly. For
@@ -1978,11 +1991,16 @@ static bool intel_cdclk_can_squash(struct 
drm_i915_private *dev_priv,
 */
if (!has_cdclk_squasher(dev_priv))
return false;
+   
+   cdclk_transition[0].action = INTEL_CDCLK_SQUASH;
+   cdclk_transition[0].cdclk = b->actual.cdclk;
+   cdclk_transition[1].action = INTEL_CDCLK_NOOP;
+   cdclk_transition[1].cdclk = b->actual.cdclk;
 
-   return a->cdclk != b->cdclk &&
-   a->vco != 0 &&
-   a->vco == b->vco &&
-   a->ref == b->ref;
+   return a->actual.cdclk != b->actual.cdclk &&
+   a->actual.vco != 0 &&
+   a->actual.vco == b->actual.vco &&
+   a->actual.ref == b->actual.ref;
 }
 
 /**
@@ -2758,9 +2776,9 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state 
*state)
pipe = INVALID_PIPE;
}
 
-   if (intel_cdclk_can_squash(dev_priv,
-  _cdclk_state->actual,
-  _cdclk_state->actual)) {
+   if (intel_cdclk_squash(dev_priv,
+  old_cdclk_state,
+