Re: [Intel-gfx] [PATCH 4/6] drm/i915: Use GEN12 RPSTAT register
On 14/09/2022 17:11, Dixit, Ashutosh wrote:
On Wed, 14 Sep 2022 02:56:26 -0700, Nilawar, Badal wrote:
On 13-09-2022 13:17, Tvrtko Ursulin wrote:
On 13/09/2022 01:09, Dixit, Ashutosh wrote:
On Mon, 12 Sep 2022 04:29:38 -0700, Nilawar, Badal wrote:
diff --git a/drivers/gpu/drm/i915/i915_pmu.c
b/drivers/gpu/drm/i915/i915_pmu.c
index 958b37123bf1..a24704ec2c18 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -371,7 +371,6 @@ static void
frequency_sample(struct intel_gt *gt, unsigned int period_ns)
{
struct drm_i915_private *i915 = gt->i915;
- struct intel_uncore *uncore = gt->uncore;
struct i915_pmu *pmu = >pmu;
struct intel_rps *rps = >rps;
@@ -394,7 +393,7 @@ frequency_sample(struct intel_gt *gt, unsigned
int period_ns)
* case we assume the system is running at the intended
* frequency. Fortunately, the read should rarely fail!
*/
- val = intel_uncore_read_fw(uncore, GEN6_RPSTAT1);
+ val = intel_rps_read_rpstat(rps);
Hmm, we got rid of _fw which the comment above refers to. Maybe we
need a
fw flag to intel_rps_read_rpstat?
Above function before reading rpstat it checks if gt is awake.
Ok, so you are referring to intel_gt_pm_get_if_awake check in
frequency_sample.
So when gt is awake shouldn't matter if we read GEN6_RPSTAT1 with
forcewake.In that case we can remove above comment. Let me know your
thoughts on this.
I am not entirely sure about this. For example in c1c82d267ae8
intel_uncore_read_fw was introduced with the same
intel_gt_pm_get_if_awake
check. So this would mean even if gt is awake not taking forcewake makes
a
difference. The same code pattern was retained in b66ecd0438bf. Maybe
it's
because there are no locks?
Its about power. As c1c82d267ae8 ("drm/i915/pmu: Cheat when reading the
actual frequency to avoid fw") explains the _fw variant is to avoid
preventing RC6, and so increased GPU power draw, just because someone has
PMU open. (Because of the 200Hz sampling timer that is needed for PMU
frequency reporting.)
Under the circumstances I think we could do one of two things:
1. If we want to drop _fw, we should do it as a separate patch with its
own
justification so it can be reviewed separately.
2. Otherwise as I mentioned we should retain the _fw and add a fw flag to
intel_rps_read_rpstat.
Agreed. Or instead of the flag, the usual pattern of having
intel_rps_read_rpstat_fw and make intel_rps_read_rpsstat get the
forcewake.
Also, may I ask, this patch is in the MTL enablement series but the
commit message and patch content seem like it is fixing a wider Gen12
issue? What is the extent of incorrect behaviour without it? Should it be
tagged for stable for first Tigerlake supporting kernel?
GEN6_RPSTAT1(0xa01c) and GEN12_RPSTAT1(0x1381b4) both are supported by
gen12 and above. The difference between two is GEN6_RPSTAT1 falls under
RENDER forcewake domain and GEN12_RPSTAT1 does not require forcewake to
access. GEN12_RPSTAT1 is punit register and when GT is in RC6 it will give
frequency as 0.
Correct, so no changes needed for stable kernels. But going forward Badal
is proposing (which I sort of agree with but may need some discussion) that
we change i915 behavior to return 0 freq (instead of cur_freq or RPn) when
GT is idle or in RC6 (so we don't take forcewake to read freq when GT is in
RC6).
We already report zero when GT is idle (as considered by software
tracking) so I guess the only part you'd like to change is drop the else
branch in:
val = intel_uncore_read_fw(uncore, GEN6_RPSTAT1);
if (val)
val = intel_rps_get_cagf(rps, val);
else
val = rps->cur_freq;
?
What would be pros and cons?
Regards,
Tvrtko
Re: [Intel-gfx] [PATCH 4/6] drm/i915: Use GEN12 RPSTAT register
On Wed, 14 Sep 2022 02:56:26 -0700, Nilawar, Badal wrote:
>
> On 13-09-2022 13:17, Tvrtko Ursulin wrote:
> >
> > On 13/09/2022 01:09, Dixit, Ashutosh wrote:
> >> On Mon, 12 Sep 2022 04:29:38 -0700, Nilawar, Badal wrote:
> >>>
> > diff --git a/drivers/gpu/drm/i915/i915_pmu.c
> > b/drivers/gpu/drm/i915/i915_pmu.c
> > index 958b37123bf1..a24704ec2c18 100644
> > --- a/drivers/gpu/drm/i915/i915_pmu.c
> > +++ b/drivers/gpu/drm/i915/i915_pmu.c
> > @@ -371,7 +371,6 @@ static void
> > frequency_sample(struct intel_gt *gt, unsigned int period_ns)
> > {
> > struct drm_i915_private *i915 = gt->i915;
> > - struct intel_uncore *uncore = gt->uncore;
> > struct i915_pmu *pmu = >pmu;
> > struct intel_rps *rps = >rps;
> >
> > @@ -394,7 +393,7 @@ frequency_sample(struct intel_gt *gt, unsigned
> > int period_ns)
> > * case we assume the system is running at the intended
> > * frequency. Fortunately, the read should rarely fail!
> > */
> > - val = intel_uncore_read_fw(uncore, GEN6_RPSTAT1);
> > + val = intel_rps_read_rpstat(rps);
>
> Hmm, we got rid of _fw which the comment above refers to. Maybe we
> need a
> fw flag to intel_rps_read_rpstat?
> >>>
> >>> Above function before reading rpstat it checks if gt is awake.
> >>
> >> Ok, so you are referring to intel_gt_pm_get_if_awake check in
> >> frequency_sample.
> >>
> >>> So when gt is awake shouldn't matter if we read GEN6_RPSTAT1 with
> >>> forcewake.In that case we can remove above comment. Let me know your
> >>> thoughts on this.
> >>
> >> I am not entirely sure about this. For example in c1c82d267ae8
> >> intel_uncore_read_fw was introduced with the same
> >> intel_gt_pm_get_if_awake
> >> check. So this would mean even if gt is awake not taking forcewake makes
> >> a
> >> difference. The same code pattern was retained in b66ecd0438bf. Maybe
> >> it's
> >> because there are no locks?
> >
> > Its about power. As c1c82d267ae8 ("drm/i915/pmu: Cheat when reading the
> > actual frequency to avoid fw") explains the _fw variant is to avoid
> > preventing RC6, and so increased GPU power draw, just because someone has
> > PMU open. (Because of the 200Hz sampling timer that is needed for PMU
> > frequency reporting.)
> >
> >> Under the circumstances I think we could do one of two things:
> >> 1. If we want to drop _fw, we should do it as a separate patch with its
> >> own
> >> justification so it can be reviewed separately.
> >> 2. Otherwise as I mentioned we should retain the _fw and add a fw flag to
> >> intel_rps_read_rpstat.
> >
> > Agreed. Or instead of the flag, the usual pattern of having
> > intel_rps_read_rpstat_fw and make intel_rps_read_rpsstat get the
> > forcewake.
> >
> > Also, may I ask, this patch is in the MTL enablement series but the
> > commit message and patch content seem like it is fixing a wider Gen12
> > issue? What is the extent of incorrect behaviour without it? Should it be
> > tagged for stable for first Tigerlake supporting kernel?
>
> GEN6_RPSTAT1(0xa01c) and GEN12_RPSTAT1(0x1381b4) both are supported by
> gen12 and above. The difference between two is GEN6_RPSTAT1 falls under
> RENDER forcewake domain and GEN12_RPSTAT1 does not require forcewake to
> access. GEN12_RPSTAT1 is punit register and when GT is in RC6 it will give
> frequency as 0.
Correct, so no changes needed for stable kernels. But going forward Badal
is proposing (which I sort of agree with but may need some discussion) that
we change i915 behavior to return 0 freq (instead of cur_freq or RPn) when
GT is idle or in RC6 (so we don't take forcewake to read freq when GT is in
RC6).
> Reason for clubbing this patch with MTL series is due to common function
> intel_rps_read_rpstat. I think I should send this patch in separate series.
Agree!
Thanks.
--
Ashutosh
Re: [Intel-gfx] [PATCH 4/6] drm/i915: Use GEN12 RPSTAT register
On 13-09-2022 13:17, Tvrtko Ursulin wrote:
On 13/09/2022 01:09, Dixit, Ashutosh wrote:
On Mon, 12 Sep 2022 04:29:38 -0700, Nilawar, Badal wrote:
diff --git a/drivers/gpu/drm/i915/i915_pmu.c
b/drivers/gpu/drm/i915/i915_pmu.c
index 958b37123bf1..a24704ec2c18 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -371,7 +371,6 @@ static void
frequency_sample(struct intel_gt *gt, unsigned int period_ns)
{
struct drm_i915_private *i915 = gt->i915;
- struct intel_uncore *uncore = gt->uncore;
struct i915_pmu *pmu = >pmu;
struct intel_rps *rps = >rps;
@@ -394,7 +393,7 @@ frequency_sample(struct intel_gt *gt, unsigned
int period_ns)
* case we assume the system is running at the intended
* frequency. Fortunately, the read should rarely fail!
*/
- val = intel_uncore_read_fw(uncore, GEN6_RPSTAT1);
+ val = intel_rps_read_rpstat(rps);
Hmm, we got rid of _fw which the comment above refers to. Maybe we
need a
fw flag to intel_rps_read_rpstat?
Above function before reading rpstat it checks if gt is awake.
Ok, so you are referring to intel_gt_pm_get_if_awake check in
frequency_sample.
So when gt is awake shouldn't matter if we read GEN6_RPSTAT1 with
forcewake.In that case we can remove above comment. Let me know your
thoughts on this.
I am not entirely sure about this. For example in c1c82d267ae8
intel_uncore_read_fw was introduced with the same
intel_gt_pm_get_if_awake
check. So this would mean even if gt is awake not taking forcewake
makes a
difference. The same code pattern was retained in b66ecd0438bf. Maybe
it's
because there are no locks?
Its about power. As c1c82d267ae8 ("drm/i915/pmu: Cheat when reading the
actual frequency to avoid fw") explains the _fw variant is to avoid
preventing RC6, and so increased GPU power draw, just because someone
has PMU open. (Because of the 200Hz sampling timer that is needed for
PMU frequency reporting.)
Under the circumstances I think we could do one of two things:
1. If we want to drop _fw, we should do it as a separate patch with
its own
justification so it can be reviewed separately.
2. Otherwise as I mentioned we should retain the _fw and add a fw flag to
intel_rps_read_rpstat.
Agreed. Or instead of the flag, the usual pattern of having
intel_rps_read_rpstat_fw and make intel_rps_read_rpsstat get the forcewake.
Also, may I ask, this patch is in the MTL enablement series but the
commit message and patch content seem like it is fixing a wider Gen12
issue? What is the extent of incorrect behaviour without it? Should it
be tagged for stable for first Tigerlake supporting kernel?
GEN6_RPSTAT1(0xa01c) and GEN12_RPSTAT1(0x1381b4) both are supported by
gen12 and above. The difference between two is GEN6_RPSTAT1 falls under
RENDER forcewake domain and GEN12_RPSTAT1 does not require forcewake to
access. GEN12_RPSTAT1 is punit register and when GT is in RC6 it will
give frequency as 0.
Reason for clubbing this patch with MTL series is due to common function
intel_rps_read_rpstat. I think I should send this patch in separate
series.
Regards,
Badal
Regards,
Tvrtko
Re: [Intel-gfx] [PATCH 4/6] drm/i915: Use GEN12 RPSTAT register
On 13/09/2022 01:09, Dixit, Ashutosh wrote:
On Mon, 12 Sep 2022 04:29:38 -0700, Nilawar, Badal wrote:
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 958b37123bf1..a24704ec2c18 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -371,7 +371,6 @@ static void
frequency_sample(struct intel_gt *gt, unsigned int period_ns)
{
struct drm_i915_private *i915 = gt->i915;
- struct intel_uncore *uncore = gt->uncore;
struct i915_pmu *pmu = >pmu;
struct intel_rps *rps = >rps;
@@ -394,7 +393,7 @@ frequency_sample(struct intel_gt *gt, unsigned int
period_ns)
* case we assume the system is running at the intended
* frequency. Fortunately, the read should rarely fail!
*/
- val = intel_uncore_read_fw(uncore, GEN6_RPSTAT1);
+ val = intel_rps_read_rpstat(rps);
Hmm, we got rid of _fw which the comment above refers to. Maybe we need a
fw flag to intel_rps_read_rpstat?
Above function before reading rpstat it checks if gt is awake.
Ok, so you are referring to intel_gt_pm_get_if_awake check in frequency_sample.
So when gt is awake shouldn't matter if we read GEN6_RPSTAT1 with
forcewake.In that case we can remove above comment. Let me know your
thoughts on this.
I am not entirely sure about this. For example in c1c82d267ae8
intel_uncore_read_fw was introduced with the same intel_gt_pm_get_if_awake
check. So this would mean even if gt is awake not taking forcewake makes a
difference. The same code pattern was retained in b66ecd0438bf. Maybe it's
because there are no locks?
Its about power. As c1c82d267ae8 ("drm/i915/pmu: Cheat when reading the
actual frequency to avoid fw") explains the _fw variant is to avoid
preventing RC6, and so increased GPU power draw, just because someone
has PMU open. (Because of the 200Hz sampling timer that is needed for
PMU frequency reporting.)
Under the circumstances I think we could do one of two things:
1. If we want to drop _fw, we should do it as a separate patch with its own
justification so it can be reviewed separately.
2. Otherwise as I mentioned we should retain the _fw and add a fw flag to
intel_rps_read_rpstat.
Agreed. Or instead of the flag, the usual pattern of having
intel_rps_read_rpstat_fw and make intel_rps_read_rpsstat get the forcewake.
Also, may I ask, this patch is in the MTL enablement series but the
commit message and patch content seem like it is fixing a wider Gen12
issue? What is the extent of incorrect behaviour without it? Should it
be tagged for stable for first Tigerlake supporting kernel?
Regards,
Tvrtko
Re: [Intel-gfx] [PATCH 4/6] drm/i915: Use GEN12 RPSTAT register
On Mon, 12 Sep 2022 04:29:38 -0700, Nilawar, Badal wrote:
>
> >> diff --git a/drivers/gpu/drm/i915/i915_pmu.c
> >> b/drivers/gpu/drm/i915/i915_pmu.c
> >> index 958b37123bf1..a24704ec2c18 100644
> >> --- a/drivers/gpu/drm/i915/i915_pmu.c
> >> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> >> @@ -371,7 +371,6 @@ static void
> >> frequency_sample(struct intel_gt *gt, unsigned int period_ns)
> >> {
> >>struct drm_i915_private *i915 = gt->i915;
> >> - struct intel_uncore *uncore = gt->uncore;
> >>struct i915_pmu *pmu = >pmu;
> >>struct intel_rps *rps = >rps;
> >>
> >> @@ -394,7 +393,7 @@ frequency_sample(struct intel_gt *gt, unsigned int
> >> period_ns)
> >> * case we assume the system is running at the intended
> >> * frequency. Fortunately, the read should rarely fail!
> >> */
> >> - val = intel_uncore_read_fw(uncore, GEN6_RPSTAT1);
> >> + val = intel_rps_read_rpstat(rps);
> >
> > Hmm, we got rid of _fw which the comment above refers to. Maybe we need a
> > fw flag to intel_rps_read_rpstat?
>
> Above function before reading rpstat it checks if gt is awake.
Ok, so you are referring to intel_gt_pm_get_if_awake check in frequency_sample.
> So when gt is awake shouldn't matter if we read GEN6_RPSTAT1 with
> forcewake.In that case we can remove above comment. Let me know your
> thoughts on this.
I am not entirely sure about this. For example in c1c82d267ae8
intel_uncore_read_fw was introduced with the same intel_gt_pm_get_if_awake
check. So this would mean even if gt is awake not taking forcewake makes a
difference. The same code pattern was retained in b66ecd0438bf. Maybe it's
because there are no locks?
Under the circumstances I think we could do one of two things:
1. If we want to drop _fw, we should do it as a separate patch with its own
justification so it can be reviewed separately.
2. Otherwise as I mentioned we should retain the _fw and add a fw flag to
intel_rps_read_rpstat.
Thanks.
--
Ashutosh
> >>if (val)
> >>val = intel_rps_get_cagf(rps, val);
> >>else
> >> --
> >> 2.25.1
> >>
Re: [Intel-gfx] [PATCH 4/6] drm/i915: Use GEN12 RPSTAT register
On 10-09-2022 07:19, Dixit, Ashutosh wrote:
On Thu, 08 Sep 2022 19:56:44 -0700, Badal Nilawar wrote:
From: Don Hiatt
On GEN12, use the correct GEN12 RPSTAT register mask/shift.
HSD: 1409538411
I think let's remove this.
Sure.
Cc: Don Hiatt
Cc: Andi Shyti
Signed-off-by: Don Hiatt
Signed-off-by: Badal Nilawar
---
drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 5 +
drivers/gpu/drm/i915/gt/intel_rps.c | 17 -
drivers/gpu/drm/i915/gt/intel_rps.h | 1 +
drivers/gpu/drm/i915/i915_pmu.c | 3 +--
5 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
index 108b9e76c32e..96c03a1258e1 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
@@ -380,7 +380,7 @@ void intel_gt_pm_frequency_dump(struct intel_gt *gt, struct
drm_printer *p)
rpinclimit = intel_uncore_read(uncore, GEN6_RP_UP_THRESHOLD);
rpdeclimit = intel_uncore_read(uncore, GEN6_RP_DOWN_THRESHOLD);
- rpstat = intel_uncore_read(uncore, GEN6_RPSTAT1);
+ rpstat = intel_rps_read_rpstat(rps);
rpcurupei = intel_uncore_read(uncore, GEN6_RP_CUR_UP_EI) &
GEN6_CURICONT_MASK;
rpcurup = intel_uncore_read(uncore, GEN6_RP_CUR_UP) &
GEN6_CURBSYTAVG_MASK;
rpprevup = intel_uncore_read(uncore, GEN6_RP_PREV_UP) &
GEN6_CURBSYTAVG_MASK;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index fb2c56777480..dac59c3e68db 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1510,6 +1510,11 @@
#define VLV_RENDER_C0_COUNT _MMIO(0x138118)
#define VLV_MEDIA_C0_COUNT_MMIO(0x13811c)
+#define GEN12_RPSTAT1 _MMIO(0x1381b4)
+#define GEN12_CAGF_SHIFT 11
+#define GEN12_CAGF_MASK REG_GENMASK(19, 11)
+#define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
Let's remove GEN12_VOLTAGE_MASK, looks like it's not being used.
Yes, not used. I will remove this.
+
#define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
#define GEN11_CSME (31)
#define GEN11_GUNIT (28)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 6fadde4ee7bf..341f96f536e8 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -2040,6 +2040,19 @@ void intel_rps_sanitize(struct intel_rps *rps)
rps_disable_interrupts(rps);
}
+u32 intel_rps_read_rpstat(struct intel_rps *rps)
+{
+ struct drm_i915_private *i915 = rps_to_i915(rps);
+ u32 rpstat;
+
+ if (GRAPHICS_VER(i915) >= 12)
+ rpstat = intel_uncore_read(rps_to_gt(rps)->uncore,
GEN12_RPSTAT1);
+ else
+ rpstat = intel_uncore_read(rps_to_gt(rps)->uncore,
GEN6_RPSTAT1);
Probably nit but how about something like this:
i915_reg_t rpstat;
if (GRAPHICS_VER(i915) >= 12)
rpstat = GEN12_RPSTAT1;
else
rpstat = GEN6_RPSTAT1;
return intel_uncore_read(rps_to_gt(rps)->uncore, rpstat);
Ok
+
+ return rpstat;
+}
+
u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
{
struct drm_i915_private *i915 = rps_to_i915(rps);
@@ -2047,6 +2060,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
cagf = (rpstat >> 8) & 0xff;
+ else if (GRAPHICS_VER(i915) >= 12)
+ cagf = (rpstat & GEN12_CAGF_MASK) >> GEN12_CAGF_SHIFT;
else if (GRAPHICS_VER(i915) >= 9)
cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
@@ -2071,7 +2086,7 @@ static u32 read_cagf(struct intel_rps *rps)
freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
vlv_punit_put(i915);
} else if (GRAPHICS_VER(i915) >= 6) {
- freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
+ freq = intel_rps_read_rpstat(rps);
} else {
freq = intel_uncore_read(uncore, MEMSTAT_ILK);
}
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h
b/drivers/gpu/drm/i915/gt/intel_rps.h
index 4509dfdc52e0..08bae6d97870 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.h
+++ b/drivers/gpu/drm/i915/gt/intel_rps.h
@@ -47,6 +47,7 @@ u32 intel_rps_get_rp1_frequency(struct intel_rps *rps);
u32 intel_rps_get_rpn_frequency(struct intel_rps *rps);
u32 intel_rps_read_punit_req(struct intel_rps *rps);
u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps);
+u32
Re: [Intel-gfx] [PATCH 4/6] drm/i915: Use GEN12 RPSTAT register
On Thu, 08 Sep 2022 19:56:44 -0700, Badal Nilawar wrote:
>
> From: Don Hiatt
>
> On GEN12, use the correct GEN12 RPSTAT register mask/shift.
>
> HSD: 1409538411
I think let's remove this.
> Cc: Don Hiatt
> Cc: Andi Shyti
> Signed-off-by: Don Hiatt
> Signed-off-by: Badal Nilawar
> ---
> drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 5 +
> drivers/gpu/drm/i915/gt/intel_rps.c | 17 -
> drivers/gpu/drm/i915/gt/intel_rps.h | 1 +
> drivers/gpu/drm/i915/i915_pmu.c | 3 +--
> 5 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> index 108b9e76c32e..96c03a1258e1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> @@ -380,7 +380,7 @@ void intel_gt_pm_frequency_dump(struct intel_gt *gt,
> struct drm_printer *p)
> rpinclimit = intel_uncore_read(uncore, GEN6_RP_UP_THRESHOLD);
> rpdeclimit = intel_uncore_read(uncore, GEN6_RP_DOWN_THRESHOLD);
>
> - rpstat = intel_uncore_read(uncore, GEN6_RPSTAT1);
> + rpstat = intel_rps_read_rpstat(rps);
> rpcurupei = intel_uncore_read(uncore, GEN6_RP_CUR_UP_EI) &
> GEN6_CURICONT_MASK;
> rpcurup = intel_uncore_read(uncore, GEN6_RP_CUR_UP) &
> GEN6_CURBSYTAVG_MASK;
> rpprevup = intel_uncore_read(uncore, GEN6_RP_PREV_UP) &
> GEN6_CURBSYTAVG_MASK;
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index fb2c56777480..dac59c3e68db 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1510,6 +1510,11 @@
> #define VLV_RENDER_C0_COUNT _MMIO(0x138118)
> #define VLV_MEDIA_C0_COUNT _MMIO(0x13811c)
>
> +#define GEN12_RPSTAT1_MMIO(0x1381b4)
> +#define GEN12_CAGF_SHIFT 11
> +#define GEN12_CAGF_MASKREG_GENMASK(19, 11)
> +#define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
Let's remove GEN12_VOLTAGE_MASK, looks like it's not being used.
> +
> #define GEN11_GT_INTR_DW(x) _MMIO(0x190018 + ((x) * 4))
> #define GEN11_CSME (31)
> #define GEN11_GUNIT(28)
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
> b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 6fadde4ee7bf..341f96f536e8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -2040,6 +2040,19 @@ void intel_rps_sanitize(struct intel_rps *rps)
> rps_disable_interrupts(rps);
> }
>
> +u32 intel_rps_read_rpstat(struct intel_rps *rps)
> +{
> + struct drm_i915_private *i915 = rps_to_i915(rps);
> + u32 rpstat;
> +
> + if (GRAPHICS_VER(i915) >= 12)
> + rpstat = intel_uncore_read(rps_to_gt(rps)->uncore,
> GEN12_RPSTAT1);
> + else
> + rpstat = intel_uncore_read(rps_to_gt(rps)->uncore,
> GEN6_RPSTAT1);
Probably nit but how about something like this:
i915_reg_t rpstat;
if (GRAPHICS_VER(i915) >= 12)
rpstat = GEN12_RPSTAT1;
else
rpstat = GEN6_RPSTAT1;
return intel_uncore_read(rps_to_gt(rps)->uncore, rpstat);
> +
> + return rpstat;
> +}
> +
> u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat)
> {
> struct drm_i915_private *i915 = rps_to_i915(rps);
> @@ -2047,6 +2060,8 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32
> rpstat)
>
> if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> cagf = (rpstat >> 8) & 0xff;
> + else if (GRAPHICS_VER(i915) >= 12)
> + cagf = (rpstat & GEN12_CAGF_MASK) >> GEN12_CAGF_SHIFT;
> else if (GRAPHICS_VER(i915) >= 9)
> cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
> else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> @@ -2071,7 +2086,7 @@ static u32 read_cagf(struct intel_rps *rps)
> freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
> vlv_punit_put(i915);
> } else if (GRAPHICS_VER(i915) >= 6) {
> - freq = intel_uncore_read(uncore, GEN6_RPSTAT1);
> + freq = intel_rps_read_rpstat(rps);
> } else {
> freq = intel_uncore_read(uncore, MEMSTAT_ILK);
> }
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h
> b/drivers/gpu/drm/i915/gt/intel_rps.h
> index 4509dfdc52e0..08bae6d97870 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.h
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.h
> @@ -47,6 +47,7 @@ u32 intel_rps_get_rp1_frequency(struct intel_rps *rps);
> u32 intel_rps_get_rpn_frequency(struct intel_rps *rps);
> u32 intel_rps_read_punit_req(struct intel_rps *rps);
> u32 intel_rps_read_punit_req_frequency(struct
