All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nilawar, Badal" <badal.nilawar@intel.com>
To: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 4/6] drm/i915: Use GEN12 RPSTAT register
Date: Mon, 12 Sep 2022 16:59:38 +0530	[thread overview]
Message-ID: <1ce34139-0b3f-6709-597f-e55437bccc0d@intel.com> (raw)
In-Reply-To: <87k06c577l.wl-ashutosh.dixit@intel.com>



On 10-09-2022 07:19, Dixit, Ashutosh wrote:
> On Thu, 08 Sep 2022 19:56:44 -0700, Badal Nilawar wrote:
>>
>> From: Don Hiatt <don.hiatt@intel.com>
>>
>> On GEN12, use the correct GEN12 RPSTAT register mask/shift.
>>
>> HSD: 1409538411
> 
> I think let's remove this.
Sure.
> 
>> Cc: Don Hiatt <donhiatt@gmail.com>
>> Cc: Andi Shyti <andi.shyti@intel.com>
>> Signed-off-by: Don Hiatt <don.hiatt@intel.com>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>>   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 intel_rps_read_rpstat(struct intel_rps *rps);
>>   void gen6_rps_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps *caps);
>>   void intel_rps_raise_unslice(struct intel_rps *rps);
>>   void intel_rps_lower_unslice(struct intel_rps *rps);
>> 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 = &i915->pmu;
>> 	struct intel_rps *rps = &gt->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. 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.

Regards,
Badal Nilawar
> 
>> 		if (val)
>> 			val = intel_rps_get_cagf(rps, val);
>> 		else
>> --
>> 2.25.1
>>

  reply	other threads:[~2022-09-12 11:29 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-09  2:56 [Intel-gfx] [PATCH 0/6] i915: CAGF and RC6 changes for MTL Badal Nilawar
2022-09-09  2:56 ` [Intel-gfx] [PATCH 1/6] drm/i915: Prepare more multi-GT initialization Badal Nilawar
2022-09-09  2:56 ` [Intel-gfx] [PATCH 2/6] drm/i915: Rename and expose common GT early init routine Badal Nilawar
2022-09-09  2:56 ` [Intel-gfx] [PATCH 3/6] drm/i915/xelpmp: Expose media as another GT Badal Nilawar
2022-09-09  2:56 ` [Intel-gfx] [PATCH 4/6] drm/i915: Use GEN12 RPSTAT register Badal Nilawar
2022-09-10  1:49   ` Dixit, Ashutosh
2022-09-12 11:29     ` Nilawar, Badal [this message]
2022-09-13  0:09       ` Dixit, Ashutosh
2022-09-13  7:47         ` Tvrtko Ursulin
2022-09-14  9:56           ` Nilawar, Badal
2022-09-14 16:11             ` Dixit, Ashutosh
2022-09-15  7:33               ` Tvrtko Ursulin
2022-09-09  2:56 ` [Intel-gfx] [PATCH 5/6] drm/i915/mtl: Modify CAGF functions for MTL Badal Nilawar
2022-09-09  2:56 ` [Intel-gfx] [PATCH 6/6] drm/i915/mtl: Add C6 residency support for MTL SAMedia Badal Nilawar
2022-09-10  3:38   ` Dixit, Ashutosh
2022-09-09  3:19 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for i915: CAGF and RC6 changes for MTL (rev3) Patchwork
2022-09-09  3:19 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-09-09  3:30 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-09-09  8:23 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2022-09-12 11:18 ` [Intel-gfx] [PATCH 0/6] i915: CAGF and RC6 changes for MTL Andi Shyti
2022-09-12 12:07   ` Jani Nikula
2022-09-12 12:12     ` Nilawar, Badal
2022-09-12 17:16       ` Andi Shyti
2022-09-12 17:09     ` Andi Shyti

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1ce34139-0b3f-6709-597f-e55437bccc0d@intel.com \
    --to=badal.nilawar@intel.com \
    --cc=ashutosh.dixit@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.