On 2022-03-24 at 01:24:35 +0530, Ashutosh Dixit wrote: > Freq caps (i.e. RP0, RP1 and RPn frequencies) are read from HW. However the > formats (bit positions, widths, registers and units) of these vary for > different generations with even more variations arriving in the future. In > order not to have to do identical computation for these caps in multiple > places, here we centralize the computation of these caps. This makes the > code cleaner and also more extensible for the future. > > v2: Clarify that caps are in "hw units" in comments (Lucas De Marchi) > v3: Minor checkpatch fix > v4: s/intel_rps_get_freq_caps/gen6_rps_get_freq_caps/ (Badal Nilawar) > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx> > --- > drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 24 +---- > drivers/gpu/drm/i915/gt/intel_rps.c | 101 ++++++++++-------- > drivers/gpu/drm/i915/gt/intel_rps.h | 2 +- > drivers/gpu/drm/i915/gt/intel_rps_types.h | 10 ++ > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 14 +-- > 5 files changed, 79 insertions(+), 72 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 31dbb2b96738..280a27cb9bdf 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c > @@ -342,17 +342,16 @@ void intel_gt_pm_frequency_dump(struct intel_gt *gt, struct drm_printer *p) > } else if (GRAPHICS_VER(i915) >= 6) { > u32 rp_state_limits; > u32 gt_perf_status; > - u32 rp_state_cap; > + struct intel_rps_freq_caps caps; > u32 rpmodectl, rpinclimit, rpdeclimit; > u32 rpstat, cagf, reqf; > u32 rpcurupei, rpcurup, rpprevup; > u32 rpcurdownei, rpcurdown, rpprevdown; > u32 rpupei, rpupt, rpdownei, rpdownt; > u32 pm_ier, pm_imr, pm_isr, pm_iir, pm_mask; > - int max_freq; > > rp_state_limits = intel_uncore_read(uncore, GEN6_RP_STATE_LIMITS); > - rp_state_cap = intel_rps_read_state_cap(rps); > + gen6_rps_get_freq_caps(rps, &caps); > if (IS_GEN9_LP(i915)) > gt_perf_status = intel_uncore_read(uncore, BXT_GT_PERF_STATUS); > else > @@ -474,25 +473,12 @@ void intel_gt_pm_frequency_dump(struct intel_gt *gt, struct drm_printer *p) > drm_printf(p, "RP DOWN THRESHOLD: %d (%lldns)\n", > rpdownt, intel_gt_pm_interval_to_ns(gt, rpdownt)); > > - max_freq = (IS_GEN9_LP(i915) ? rp_state_cap >> 0 : > - rp_state_cap >> 16) & 0xff; > - max_freq *= (IS_GEN9_BC(i915) || > - GRAPHICS_VER(i915) >= 11 ? GEN9_FREQ_SCALER : 1); > drm_printf(p, "Lowest (RPN) frequency: %dMHz\n", > - intel_gpu_freq(rps, max_freq)); > - > - max_freq = (rp_state_cap & 0xff00) >> 8; > - max_freq *= (IS_GEN9_BC(i915) || > - GRAPHICS_VER(i915) >= 11 ? GEN9_FREQ_SCALER : 1); > + intel_gpu_freq(rps, caps.min_freq)); > drm_printf(p, "Nominal (RP1) frequency: %dMHz\n", > - intel_gpu_freq(rps, max_freq)); > - > - max_freq = (IS_GEN9_LP(i915) ? rp_state_cap >> 16 : > - rp_state_cap >> 0) & 0xff; > - max_freq *= (IS_GEN9_BC(i915) || > - GRAPHICS_VER(i915) >= 11 ? GEN9_FREQ_SCALER : 1); > + intel_gpu_freq(rps, caps.rp1_freq)); > drm_printf(p, "Max non-overclocked (RP0) frequency: %dMHz\n", > - intel_gpu_freq(rps, max_freq)); > + intel_gpu_freq(rps, caps.rp0_freq)); > drm_printf(p, "Max overclocked frequency: %dMHz\n", > intel_gpu_freq(rps, rps->max_freq)); > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c > index 6c9fdf7906c5..f21f6e454998 100644 > --- a/drivers/gpu/drm/i915/gt/intel_rps.c > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c > @@ -1070,23 +1070,59 @@ int intel_rps_set(struct intel_rps *rps, u8 val) > return 0; > } > > -static void gen6_rps_init(struct intel_rps *rps) > +static u32 intel_rps_read_state_cap(struct intel_rps *rps) > { > struct drm_i915_private *i915 = rps_to_i915(rps); > - u32 rp_state_cap = intel_rps_read_state_cap(rps); > + struct intel_uncore *uncore = rps_to_uncore(rps); > > - /* All of these values are in units of 50MHz */ > + if (IS_XEHPSDV(i915)) > + return intel_uncore_read(uncore, XEHPSDV_RP_STATE_CAP); > + else if (IS_GEN9_LP(i915)) > + return intel_uncore_read(uncore, BXT_RP_STATE_CAP); > + else > + return intel_uncore_read(uncore, GEN6_RP_STATE_CAP); > +} > + > +/* "Caps" frequencies should be converted to MHz using intel_gpu_freq() */ IMHO, if this exported function deserves a comment, it should Kernel Doc comment. for an example see Doc comment of intel_runtime_pm_get_raw(). Thanks, Anshuman Gupta. > +void gen6_rps_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps *caps) > +{ > + struct drm_i915_private *i915 = rps_to_i915(rps); > + u32 rp_state_cap; > + > + rp_state_cap = intel_rps_read_state_cap(rps); > > /* static values from HW: RP0 > RP1 > RPn (min_freq) */ > if (IS_GEN9_LP(i915)) { > - rps->rp0_freq = (rp_state_cap >> 16) & 0xff; > - rps->rp1_freq = (rp_state_cap >> 8) & 0xff; > - rps->min_freq = (rp_state_cap >> 0) & 0xff; > + caps->rp0_freq = (rp_state_cap >> 16) & 0xff; > + caps->rp1_freq = (rp_state_cap >> 8) & 0xff; > + caps->min_freq = (rp_state_cap >> 0) & 0xff; > } else { > - rps->rp0_freq = (rp_state_cap >> 0) & 0xff; > - rps->rp1_freq = (rp_state_cap >> 8) & 0xff; > - rps->min_freq = (rp_state_cap >> 16) & 0xff; > + caps->rp0_freq = (rp_state_cap >> 0) & 0xff; > + caps->rp1_freq = (rp_state_cap >> 8) & 0xff; > + caps->min_freq = (rp_state_cap >> 16) & 0xff; > + } > + > + if (IS_GEN9_BC(i915) || GRAPHICS_VER(i915) >= 11) { > + /* > + * In this case rp_state_cap register reports frequencies in > + * units of 50 MHz. Convert these to the actual "hw unit", i.e. > + * units of 16.67 MHz > + */ > + caps->rp0_freq *= GEN9_FREQ_SCALER; > + caps->rp1_freq *= GEN9_FREQ_SCALER; > + caps->min_freq *= GEN9_FREQ_SCALER; > } > +} > + > +static void gen6_rps_init(struct intel_rps *rps) > +{ > + struct drm_i915_private *i915 = rps_to_i915(rps); > + struct intel_rps_freq_caps caps; > + > + gen6_rps_get_freq_caps(rps, &caps); > + rps->rp0_freq = caps.rp0_freq; > + rps->rp1_freq = caps.rp1_freq; > + rps->min_freq = caps.min_freq; > > /* hw_max = RP0 until we check for overclocking */ > rps->max_freq = rps->rp0_freq; > @@ -1095,26 +1131,18 @@ static void gen6_rps_init(struct intel_rps *rps) > if (IS_HASWELL(i915) || IS_BROADWELL(i915) || > IS_GEN9_BC(i915) || GRAPHICS_VER(i915) >= 11) { > u32 ddcc_status = 0; > + u32 mult = 1; > > + if (IS_GEN9_BC(i915) || GRAPHICS_VER(i915) >= 11) > + mult = GEN9_FREQ_SCALER; > if (snb_pcode_read(i915, HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL, > &ddcc_status, NULL) == 0) > rps->efficient_freq = > - clamp_t(u8, > - (ddcc_status >> 8) & 0xff, > + clamp_t(u32, > + ((ddcc_status >> 8) & 0xff) * mult, > rps->min_freq, > rps->max_freq); > } > - > - if (IS_GEN9_BC(i915) || GRAPHICS_VER(i915) >= 11) { > - /* Store the frequency values in 16.66 MHZ units, which is > - * the natural hardware unit for SKL > - */ > - rps->rp0_freq *= GEN9_FREQ_SCALER; > - rps->rp1_freq *= GEN9_FREQ_SCALER; > - rps->min_freq *= GEN9_FREQ_SCALER; > - rps->max_freq *= GEN9_FREQ_SCALER; > - rps->efficient_freq *= GEN9_FREQ_SCALER; > - } > } > > static bool rps_reset(struct intel_rps *rps) > @@ -2219,19 +2247,6 @@ int intel_rps_set_min_frequency(struct intel_rps *rps, u32 val) > return set_min_freq(rps, val); > } > > -u32 intel_rps_read_state_cap(struct intel_rps *rps) > -{ > - struct drm_i915_private *i915 = rps_to_i915(rps); > - struct intel_uncore *uncore = rps_to_uncore(rps); > - > - if (IS_XEHPSDV(i915)) > - return intel_uncore_read(uncore, XEHPSDV_RP_STATE_CAP); > - else if (IS_GEN9_LP(i915)) > - return intel_uncore_read(uncore, BXT_RP_STATE_CAP); > - else > - return intel_uncore_read(uncore, GEN6_RP_STATE_CAP); > -} > - > static void intel_rps_set_manual(struct intel_rps *rps, bool enable) > { > struct intel_uncore *uncore = rps_to_uncore(rps); > @@ -2244,18 +2259,18 @@ static void intel_rps_set_manual(struct intel_rps *rps, bool enable) > void intel_rps_raise_unslice(struct intel_rps *rps) > { > struct intel_uncore *uncore = rps_to_uncore(rps); > - u32 rp0_unslice_req; > > mutex_lock(&rps->lock); > > if (rps_uses_slpc(rps)) { > /* RP limits have not been initialized yet for SLPC path */ > - rp0_unslice_req = ((intel_rps_read_state_cap(rps) >> 0) > - & 0xff) * GEN9_FREQ_SCALER; > + struct intel_rps_freq_caps caps; > + > + gen6_rps_get_freq_caps(rps, &caps); > > intel_rps_set_manual(rps, true); > intel_uncore_write(uncore, GEN6_RPNSWREQ, > - ((rp0_unslice_req << > + ((caps.rp0_freq << > GEN9_SW_REQ_UNSLICE_RATIO_SHIFT) | > GEN9_IGNORE_SLICE_RATIO)); > intel_rps_set_manual(rps, false); > @@ -2269,18 +2284,18 @@ void intel_rps_raise_unslice(struct intel_rps *rps) > void intel_rps_lower_unslice(struct intel_rps *rps) > { > struct intel_uncore *uncore = rps_to_uncore(rps); > - u32 rpn_unslice_req; > > mutex_lock(&rps->lock); > > if (rps_uses_slpc(rps)) { > /* RP limits have not been initialized yet for SLPC path */ > - rpn_unslice_req = ((intel_rps_read_state_cap(rps) >> 16) > - & 0xff) * GEN9_FREQ_SCALER; > + struct intel_rps_freq_caps caps; > + > + gen6_rps_get_freq_caps(rps, &caps); > > intel_rps_set_manual(rps, true); > intel_uncore_write(uncore, GEN6_RPNSWREQ, > - ((rpn_unslice_req << > + ((caps.min_freq << > GEN9_SW_REQ_UNSLICE_RATIO_SHIFT) | > GEN9_IGNORE_SLICE_RATIO)); > intel_rps_set_manual(rps, false); > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h b/drivers/gpu/drm/i915/gt/intel_rps.h > index ba1ed9f7ecda..1e8d56491308 100644 > --- a/drivers/gpu/drm/i915/gt/intel_rps.h > +++ b/drivers/gpu/drm/i915/gt/intel_rps.h > @@ -45,7 +45,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_state_cap(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/gt/intel_rps_types.h b/drivers/gpu/drm/i915/gt/intel_rps_types.h > index 3941d8551f52..4f7a351e0997 100644 > --- a/drivers/gpu/drm/i915/gt/intel_rps_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_rps_types.h > @@ -37,6 +37,16 @@ enum { > INTEL_RPS_TIMER, > }; > > +/* > + * Freq caps exposed by HW, values are in "hw units" and intel_gpu_freq() > + * should be used to convert to MHz > + */ > +struct intel_rps_freq_caps { > + u8 rp0_freq; /* non-overclocked max frequency */ > + u8 rp1_freq; /* "less than" RP0 power/freqency */ > + u8 min_freq; /* aka RPn, minimum frequency */ > +}; > + > struct intel_rps { > struct mutex lock; /* protects enabling and the worker */ > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > index 9f032c65a488..9e02355e44f1 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > @@ -582,16 +582,12 @@ static int slpc_use_fused_rp0(struct intel_guc_slpc *slpc) > static void slpc_get_rp_values(struct intel_guc_slpc *slpc) > { > struct intel_rps *rps = &slpc_to_gt(slpc)->rps; > - u32 rp_state_cap; > + struct intel_rps_freq_caps caps; > > - rp_state_cap = intel_rps_read_state_cap(rps); > - > - slpc->rp0_freq = REG_FIELD_GET(RP0_CAP_MASK, rp_state_cap) * > - GT_FREQUENCY_MULTIPLIER; > - slpc->rp1_freq = REG_FIELD_GET(RP1_CAP_MASK, rp_state_cap) * > - GT_FREQUENCY_MULTIPLIER; > - slpc->min_freq = REG_FIELD_GET(RPN_CAP_MASK, rp_state_cap) * > - GT_FREQUENCY_MULTIPLIER; > + gen6_rps_get_freq_caps(rps, &caps); > + slpc->rp0_freq = intel_gpu_freq(rps, caps.rp0_freq); > + slpc->rp1_freq = intel_gpu_freq(rps, caps.rp1_freq); > + slpc->min_freq = intel_gpu_freq(rps, caps.min_freq); > > if (!slpc->boost_freq) > slpc->boost_freq = slpc->rp0_freq; > -- > 2.34.1 >