Re: [Intel-gfx] [PATCH 3/3] drm/i915/pmu: Use common freq functions with sysfs

["Belgaumkar, Vinay" <vinay.belgaumkar@intel.com>]

On 3/7/2023 9:33 PM, Ashutosh Dixit wrote:
> Using common freq functions with sysfs in PMU (but without taking
> forcewake) solves the following issues (a) missing support for MTL (b)

For the requested_freq, we read it only if actual_freq is zero below 
(meaning, GT is in C6). So then what is the point of reading it without 
a force wake? It will also be zero, correct?

Thanks,

Vinay.

> missing support for older generation (prior to Gen6) (c) missing support
> for slpc when freq sampling has to fall back to requested freq. It also
> makes the PMU code future proof where sometimes code has been updated for
> sysfs and PMU has been missed.
>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_rps.c | 10 ----------
>   drivers/gpu/drm/i915/gt/intel_rps.h |  1 -
>   drivers/gpu/drm/i915/i915_pmu.c     | 10 ++++------
>   3 files changed, 4 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 49df31927c0e..b03bfbe7ee23 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -2046,16 +2046,6 @@ void intel_rps_sanitize(struct intel_rps *rps)
>   		rps_disable_interrupts(rps);
>   }
>   
> -u32 intel_rps_read_rpstat_fw(struct intel_rps *rps)
> -{
> -	struct drm_i915_private *i915 = rps_to_i915(rps);
> -	i915_reg_t rpstat;
> -
> -	rpstat = (GRAPHICS_VER(i915) >= 12) ? GEN12_RPSTAT1 : GEN6_RPSTAT1;
> -
> -	return intel_uncore_read_fw(rps_to_gt(rps)->uncore, rpstat);
> -}
> -
>   u32 intel_rps_read_rpstat(struct intel_rps *rps)
>   {
>   	struct drm_i915_private *i915 = rps_to_i915(rps);
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h b/drivers/gpu/drm/i915/gt/intel_rps.h
> index a990f985ab23..60ae27679011 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.h
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.h
> @@ -53,7 +53,6 @@ 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_frequency(struct intel_rps *rps);
>   u32 intel_rps_read_rpstat(struct intel_rps *rps);
> -u32 intel_rps_read_rpstat_fw(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 a76c5ce9513d..1a4c9fed257c 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -392,14 +392,12 @@ 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_rps_read_rpstat_fw(rps);
> -		if (val)
> -			val = intel_rps_get_cagf(rps, val);
> -		else
> -			val = rps->cur_freq;
> +		val = intel_rps_read_actual_frequency_fw(rps);
> +		if (!val)
> +			val = intel_rps_get_requested_frequency_fw(rps),
>   
>   		add_sample_mult(&pmu->sample[__I915_SAMPLE_FREQ_ACT],
> -				intel_gpu_freq(rps, val), period_ns / 1000);
> +				val, period_ns / 1000);
>   	}
>   
>   	if (pmu->enable & config_mask(I915_PMU_REQUESTED_FREQUENCY)) {