Re: [Intel-gfx] [PATCH 6/6] drm/i915/rps: Freq caps for MTL

2022-09-07 Thread Dixit, Ashutosh 
On Mon, 05 Sep 2022 02:40:08 -0700, Jani Nikula wrote:
> On Fri, 02 Sep 2022, Ashutosh Dixit  wrote:
> > For MTL, when reading from HW, RP0, RP1 (actuall RPe) and RPn freq use an
> > entirely different set of registers with different fields, bitwidths and
> > units.
> >
> > Cc: Badal Nilawar 
> > Signed-off-by: Ashutosh Dixit 
> > ---
> >  drivers/gpu/drm/i915/gt/intel_rps.c | 20 
> >  drivers/gpu/drm/i915/i915_reg.h |  9 +
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c 
> > b/drivers/gpu/drm/i915/gt/intel_rps.c
> > index 579ae9ac089c..e7ab172698e3 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> > @@ -1085,6 +1085,23 @@ static u32 intel_rps_read_state_cap(struct intel_rps 
> > *rps)
> > return intel_uncore_read(uncore, GEN6_RP_STATE_CAP);
> >  }
> >
> > +static void
> > +mtl_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps *caps)
> > +{
> > +   struct intel_uncore *uncore = rps_to_uncore(rps);
> > +   u32 rp_state_cap = rps_to_gt(rps)->type == GT_MEDIA ?
> > +   intel_uncore_read(uncore, MTL_MEDIAP_STATE_CAP) 
> > :
> > +   intel_uncore_read(uncore, MTL_RP_STATE_CAP);
> > +   u32 rpe = rps_to_gt(rps)->type == GT_MEDIA ?
> > +   intel_uncore_read(uncore, MTL_MPE_FREQUENCY) :
> > +   intel_uncore_read(uncore, MTL_GT_RPE_FREQUENCY);
> > +
> > +   /* MTL values are in units of 16.67 MHz */
> > +   caps->rp0_freq = REG_FIELD_GET(MTL_RP0_CAP_MASK, rp_state_cap);
> > +   caps->min_freq = REG_FIELD_GET(MTL_RPN_CAP_MASK, rp_state_cap);
> > +   caps->rp1_freq = REG_FIELD_GET(MTL_RPE_MASK, rpe);
> > +}
> > +
> >  /**
> >   * gen6_rps_get_freq_caps - Get freq caps exposed by HW
> >   * @rps: the intel_rps structure
> > @@ -1098,6 +1115,9 @@ void gen6_rps_get_freq_caps(struct intel_rps *rps, 
> > struct intel_rps_freq_caps *c
> > struct drm_i915_private *i915 = rps_to_i915(rps);
> > u32 rp_state_cap;
> >
> > +   if (IS_METEORLAKE(i915))
> > +   return mtl_get_freq_caps(rps, caps);
> > +
>
> Please make gen6_rps_get_freq_caps() static, and add
>
> intel_rps_get_freq_caps()
> {
>   if (IS_METEORLAKE(i915))
>   return mtl_get_freq_caps(rps, caps);
>   else
>   return gen6_rps_get_freq_caps(rps, caps);
> }
>
> Or something.

A general name like intel_rps_get_freq_caps name does not sit well with the
current code. intel_rps_get_freq_caps was actually used in earlier versions
of the patch:

https://patchwork.freedesktop.org/patch/479179/?series=101606=3

but was later changed to gen6_rps_get_freq_caps based on review
comments. Afaiu in i915 a name such as gen6_rps_get_freq_caps implies "Gen6
and later" and the gen6_rps_get_freq_caps name has actually proved quite
useful in reminding people that there are earlier/other generations not
covered by the function. See intel_rps_init.

Further the call stack is:

intel_rps_init -> gen6_rps_init -> gen6_rps_get_freq_caps

So it would look odd if we called intel_rps_get_freq_caps from
gen6_rps_init.

Therefore what I have done in v2 is:

s/gen6_rps_get_freq_caps/__gen6_rps_get_freq_caps/

and then

gen6_rps_get_freq_caps()
{
if (IS_METEORLAKE(i915))
return mtl_get_freq_caps(rps, caps);
else
return __gen6_rps_get_freq_caps(rps, caps);
}

Thanks.
--
Ashutosh

> > rp_state_cap = intel_rps_read_state_cap(rps);
> >
> > /* static values from HW: RP0 > RP1 > RPn (min_freq) */
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 06d555321651..d78f9675aa57 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1792,6 +1792,15 @@
> >  #define XEHPSDV_RP_STATE_CAP   _MMIO(0x250014)
> >  #define PVC_RP_STATE_CAP   _MMIO(0x281014)
> >
> > +#define MTL_RP_STATE_CAP   _MMIO(0x138000)
> > +#define MTL_MEDIAP_STATE_CAP   _MMIO(0x138020)
> > +#define   MTL_RP0_CAP_MASK REG_GENMASK(8, 0)
> > +#define   MTL_RPN_CAP_MASK REG_GENMASK(24, 16)
> > +
> > +#define MTL_GT_RPE_FREQUENCY   _MMIO(0x13800c)
> > +#define MTL_MPE_FREQUENCY  _MMIO(0x13802c)
> > +#define   MTL_RPE_MASK REG_GENMASK(8, 0)
> > +
> >  #define GT0_PERF_LIMIT_REASONS _MMIO(0x1381a8)
> >  #define   GT0_PERF_LIMIT_REASONS_MASK  0xde3
> >  #define   PROCHOT_MASK REG_BIT(1)
>
> --
> Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 6/6] drm/i915/rps: Freq caps for MTL

2022-09-05 Thread Jani Nikula 
On Fri, 02 Sep 2022, Ashutosh Dixit  wrote:
> For MTL, when reading from HW, RP0, RP1 (actuall RPe) and RPn freq use an
> entirely different set of registers with different fields, bitwidths and
> units.
>
> Cc: Badal Nilawar 
> Signed-off-by: Ashutosh Dixit 
> ---
>  drivers/gpu/drm/i915/gt/intel_rps.c | 20 
>  drivers/gpu/drm/i915/i915_reg.h |  9 +
>  2 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c 
> b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 579ae9ac089c..e7ab172698e3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -1085,6 +1085,23 @@ static u32 intel_rps_read_state_cap(struct intel_rps 
> *rps)
>   return intel_uncore_read(uncore, GEN6_RP_STATE_CAP);
>  }
>  
> +static void
> +mtl_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps *caps)
> +{
> + struct intel_uncore *uncore = rps_to_uncore(rps);
> + u32 rp_state_cap = rps_to_gt(rps)->type == GT_MEDIA ?
> + intel_uncore_read(uncore, MTL_MEDIAP_STATE_CAP) 
> :
> + intel_uncore_read(uncore, MTL_RP_STATE_CAP);
> + u32 rpe = rps_to_gt(rps)->type == GT_MEDIA ?
> + intel_uncore_read(uncore, MTL_MPE_FREQUENCY) :
> + intel_uncore_read(uncore, MTL_GT_RPE_FREQUENCY);
> +
> + /* MTL values are in units of 16.67 MHz */
> + caps->rp0_freq = REG_FIELD_GET(MTL_RP0_CAP_MASK, rp_state_cap);
> + caps->min_freq = REG_FIELD_GET(MTL_RPN_CAP_MASK, rp_state_cap);
> + caps->rp1_freq = REG_FIELD_GET(MTL_RPE_MASK, rpe);
> +}
> +
>  /**
>   * gen6_rps_get_freq_caps - Get freq caps exposed by HW
>   * @rps: the intel_rps structure
> @@ -1098,6 +1115,9 @@ void gen6_rps_get_freq_caps(struct intel_rps *rps, 
> struct intel_rps_freq_caps *c
>   struct drm_i915_private *i915 = rps_to_i915(rps);
>   u32 rp_state_cap;
>  
> + if (IS_METEORLAKE(i915))
> + return mtl_get_freq_caps(rps, caps);
> +

Please make gen6_rps_get_freq_caps() static, and add

intel_rps_get_freq_caps()
{
if (IS_METEORLAKE(i915))
return mtl_get_freq_caps(rps, caps);
else
return gen6_rps_get_freq_caps(rps, caps);
}

Or something.

BR,
Jani.


>   rp_state_cap = intel_rps_read_state_cap(rps);
>  
>   /* static values from HW: RP0 > RP1 > RPn (min_freq) */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 06d555321651..d78f9675aa57 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1792,6 +1792,15 @@
>  #define XEHPSDV_RP_STATE_CAP _MMIO(0x250014)
>  #define PVC_RP_STATE_CAP _MMIO(0x281014)
>  
> +#define MTL_RP_STATE_CAP _MMIO(0x138000)
> +#define MTL_MEDIAP_STATE_CAP _MMIO(0x138020)
> +#define   MTL_RP0_CAP_MASK   REG_GENMASK(8, 0)
> +#define   MTL_RPN_CAP_MASK   REG_GENMASK(24, 16)
> +
> +#define MTL_GT_RPE_FREQUENCY _MMIO(0x13800c)
> +#define MTL_MPE_FREQUENCY_MMIO(0x13802c)
> +#define   MTL_RPE_MASK   REG_GENMASK(8, 0)
> +
>  #define GT0_PERF_LIMIT_REASONS   _MMIO(0x1381a8)
>  #define   GT0_PERF_LIMIT_REASONS_MASK0xde3
>  #define   PROCHOT_MASK   REG_BIT(1)

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] [PATCH 6/6] drm/i915/rps: Freq caps for MTL

2022-09-02 Thread Ashutosh Dixit 
For MTL, when reading from HW, RP0, RP1 (actuall RPe) and RPn freq use an
entirely different set of registers with different fields, bitwidths and
units.

Cc: Badal Nilawar 
Signed-off-by: Ashutosh Dixit 
---
 drivers/gpu/drm/i915/gt/intel_rps.c | 20 
 drivers/gpu/drm/i915/i915_reg.h |  9 +
 2 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c 
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 579ae9ac089c..e7ab172698e3 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -1085,6 +1085,23 @@ static u32 intel_rps_read_state_cap(struct intel_rps 
*rps)
return intel_uncore_read(uncore, GEN6_RP_STATE_CAP);
 }
 
+static void
+mtl_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps *caps)
+{
+   struct intel_uncore *uncore = rps_to_uncore(rps);
+   u32 rp_state_cap = rps_to_gt(rps)->type == GT_MEDIA ?
+   intel_uncore_read(uncore, MTL_MEDIAP_STATE_CAP) 
:
+   intel_uncore_read(uncore, MTL_RP_STATE_CAP);
+   u32 rpe = rps_to_gt(rps)->type == GT_MEDIA ?
+   intel_uncore_read(uncore, MTL_MPE_FREQUENCY) :
+   intel_uncore_read(uncore, MTL_GT_RPE_FREQUENCY);
+
+   /* MTL values are in units of 16.67 MHz */
+   caps->rp0_freq = REG_FIELD_GET(MTL_RP0_CAP_MASK, rp_state_cap);
+   caps->min_freq = REG_FIELD_GET(MTL_RPN_CAP_MASK, rp_state_cap);
+   caps->rp1_freq = REG_FIELD_GET(MTL_RPE_MASK, rpe);
+}
+
 /**
  * gen6_rps_get_freq_caps - Get freq caps exposed by HW
  * @rps: the intel_rps structure
@@ -1098,6 +1115,9 @@ void gen6_rps_get_freq_caps(struct intel_rps *rps, struct 
intel_rps_freq_caps *c
struct drm_i915_private *i915 = rps_to_i915(rps);
u32 rp_state_cap;
 
+   if (IS_METEORLAKE(i915))
+   return mtl_get_freq_caps(rps, caps);
+
rp_state_cap = intel_rps_read_state_cap(rps);
 
/* static values from HW: RP0 > RP1 > RPn (min_freq) */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 06d555321651..d78f9675aa57 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1792,6 +1792,15 @@
 #define XEHPSDV_RP_STATE_CAP   _MMIO(0x250014)
 #define PVC_RP_STATE_CAP   _MMIO(0x281014)
 
+#define MTL_RP_STATE_CAP   _MMIO(0x138000)
+#define MTL_MEDIAP_STATE_CAP   _MMIO(0x138020)
+#define   MTL_RP0_CAP_MASK REG_GENMASK(8, 0)
+#define   MTL_RPN_CAP_MASK REG_GENMASK(24, 16)
+
+#define MTL_GT_RPE_FREQUENCY   _MMIO(0x13800c)
+#define MTL_MPE_FREQUENCY  _MMIO(0x13802c)
+#define   MTL_RPE_MASK REG_GENMASK(8, 0)
+
 #define GT0_PERF_LIMIT_REASONS _MMIO(0x1381a8)
 #define   GT0_PERF_LIMIT_REASONS_MASK  0xde3
 #define   PROCHOT_MASK REG_BIT(1)
-- 
2.34.1