On Wed, Oct 19, 2022 at 04:37:20PM -0700, Ashutosh Dixit wrote:
> Previously RC6 residency functions directly accepted RC6 residency register
> MMIO offsets (there are four RC6 residency registers). This worked but
> required an assumption on the residency register layout so was not future
> proof.
>
> Therefore change RC6 residency functions to accept RC6 residency types
> instead of register MMIO offsets. The knowledge of register offsets as well
> as ID to offset mapping is now maintained solely in intel_rc6 and can be
> tailored for different platforms and different register layouts as need
> arises.
>
> v2: Address review comments by Jani N
> - Change residency functions to accept RC6 residency types instead of
> register ID's
> - s/intel_rc6_print_rc5_res/intel_rc6_print_residency/
> - Remove "const enum" in function arguments
> - Naming: intel_rc6_* for enum
> - Use INTEL_RC6_RES_MAX and other minor changes
> v3: Don't include intel_rc6_types.h in intel_rc6.h (Jani)
>
> Suggested-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Suggested-by: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Reported-by: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 27 +++------
> drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 12 ++--
> drivers/gpu/drm/i915/gt/intel_rc6.c | 55 +++++++++++--------
> drivers/gpu/drm/i915/gt/intel_rc6.h | 11 ++--
> drivers/gpu/drm/i915/gt/intel_rc6_types.h | 15 ++++-
> drivers/gpu/drm/i915/gt/selftest_rc6.c | 6 +-
> drivers/gpu/drm/i915/i915_pmu.c | 6 +-
> 7 files changed, 72 insertions(+), 60 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 979e602946549..5d6b346831393 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> @@ -83,19 +83,6 @@ static int fw_domains_show(struct seq_file *m, void *data)
> }
> DEFINE_INTEL_GT_DEBUGFS_ATTRIBUTE(fw_domains);
>
> -static void print_rc6_res(struct seq_file *m,
> - const char *title,
> - const i915_reg_t reg)
> -{
> - struct intel_gt *gt = m->private;
> - intel_wakeref_t wakeref;
> -
> - with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> - seq_printf(m, "%s %u (%llu us)\n", title,
> - intel_uncore_read(gt->uncore, reg),
> - intel_rc6_residency_us(>->rc6, reg));
> -}
> -
> static int vlv_drpc(struct seq_file *m)
> {
> struct intel_gt *gt = m->private;
> @@ -115,8 +102,8 @@ static int vlv_drpc(struct seq_file *m)
> seq_printf(m, "Media Power Well: %s\n",
> (pw_status & VLV_GTLC_PW_MEDIA_STATUS_MASK) ? "Up" : "Down");
>
> - print_rc6_res(m, "Render RC6 residency since boot:", GEN6_GT_GFX_RC6);
> - print_rc6_res(m, "Media RC6 residency since boot:", VLV_GT_MEDIA_RC6);
> + intel_rc6_print_residency(m, "Render RC6 residency since boot:", INTEL_RC6_RES_RC6);
> + intel_rc6_print_residency(m, "Media RC6 residency since boot:", INTEL_RC6_RES_VLV_MEDIA);
>
> return fw_domains_show(m, NULL);
> }
> @@ -192,11 +179,11 @@ static int gen6_drpc(struct seq_file *m)
> }
>
> /* Not exactly sure what this is */
> - print_rc6_res(m, "RC6 \"Locked to RPn\" residency since boot:",
> - GEN6_GT_GFX_RC6_LOCKED);
> - print_rc6_res(m, "RC6 residency since boot:", GEN6_GT_GFX_RC6);
> - print_rc6_res(m, "RC6+ residency since boot:", GEN6_GT_GFX_RC6p);
> - print_rc6_res(m, "RC6++ residency since boot:", GEN6_GT_GFX_RC6pp);
> + intel_rc6_print_residency(m, "RC6 \"Locked to RPn\" residency since boot:",
> + INTEL_RC6_RES_RC6_LOCKED);
> + intel_rc6_print_residency(m, "RC6 residency since boot:", INTEL_RC6_RES_RC6);
> + intel_rc6_print_residency(m, "RC6+ residency since boot:", INTEL_RC6_RES_RC6p);
> + intel_rc6_print_residency(m, "RC6++ residency since boot:", INTEL_RC6_RES_RC6pp);
>
> if (GRAPHICS_VER(i915) <= 7) {
> seq_printf(m, "RC6 voltage: %dmV\n",
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> index 9041609523697..19a60000e052c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> @@ -93,13 +93,13 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> sysfs_gt_attribute_r_func(d, a, f, INTEL_GT_SYSFS_MAX)
>
> #ifdef CONFIG_PM
> -static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
> +static u32 get_residency(struct intel_gt *gt, enum intel_rc6_res_type id)
> {
> intel_wakeref_t wakeref;
> u64 res = 0;
>
> with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> - res = intel_rc6_residency_us(>->rc6, reg);
> + res = intel_rc6_residency_us(>->rc6, id);
>
> return DIV_ROUND_CLOSEST_ULL(res, 1000);
> }
> @@ -123,7 +123,7 @@ static ssize_t rc6_enable_show(struct device *dev,
>
> static u32 __rc6_residency_ms_show(struct intel_gt *gt)
> {
> - return get_residency(gt, GEN6_GT_GFX_RC6);
> + return get_residency(gt, INTEL_RC6_RES_RC6);
> }
>
> static ssize_t rc6_residency_ms_show(struct device *dev,
> @@ -138,7 +138,7 @@ static ssize_t rc6_residency_ms_show(struct device *dev,
>
> static u32 __rc6p_residency_ms_show(struct intel_gt *gt)
> {
> - return get_residency(gt, GEN6_GT_GFX_RC6p);
> + return get_residency(gt, INTEL_RC6_RES_RC6p);
> }
>
> static ssize_t rc6p_residency_ms_show(struct device *dev,
> @@ -153,7 +153,7 @@ static ssize_t rc6p_residency_ms_show(struct device *dev,
>
> static u32 __rc6pp_residency_ms_show(struct intel_gt *gt)
> {
> - return get_residency(gt, GEN6_GT_GFX_RC6pp);
> + return get_residency(gt, INTEL_RC6_RES_RC6pp);
> }
>
> static ssize_t rc6pp_residency_ms_show(struct device *dev,
> @@ -168,7 +168,7 @@ static ssize_t rc6pp_residency_ms_show(struct device *dev,
>
> static u32 __media_rc6_residency_ms_show(struct intel_gt *gt)
> {
> - return get_residency(gt, VLV_GT_MEDIA_RC6);
> + return get_residency(gt, INTEL_RC6_RES_VLV_MEDIA);
> }
>
> static ssize_t media_rc6_residency_ms_show(struct device *dev,
> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> index f8d0523f4c18e..6db4e60d5fba5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> @@ -551,6 +551,14 @@ static void __intel_rc6_disable(struct intel_rc6 *rc6)
> intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
> }
>
> +static void rc6_res_reg_init(struct intel_rc6 *rc6)
> +{
> + rc6->res_reg[INTEL_RC6_RES_RC6_LOCKED] = GEN6_GT_GFX_RC6_LOCKED;
> + rc6->res_reg[INTEL_RC6_RES_RC6] = GEN6_GT_GFX_RC6;
> + rc6->res_reg[INTEL_RC6_RES_RC6p] = GEN6_GT_GFX_RC6p;
> + rc6->res_reg[INTEL_RC6_RES_RC6pp] = GEN6_GT_GFX_RC6pp;
> +}
> +
> void intel_rc6_init(struct intel_rc6 *rc6)
> {
> struct drm_i915_private *i915 = rc6_to_i915(rc6);
> @@ -562,6 +570,8 @@ void intel_rc6_init(struct intel_rc6 *rc6)
> if (!rc6_supported(rc6))
> return;
>
> + rc6_res_reg_init(rc6);
> +
> if (IS_CHERRYVIEW(i915))
> err = chv_rc6_init(rc6);
> else if (IS_VALLEYVIEW(i915))
> @@ -736,31 +746,19 @@ static u64 vlv_residency_raw(struct intel_uncore *uncore, const i915_reg_t reg)
> return lower | (u64)upper << 8;
> }
>
> -u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
> +u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, enum intel_rc6_res_type id)
> {
> struct drm_i915_private *i915 = rc6_to_i915(rc6);
> struct intel_uncore *uncore = rc6_to_uncore(rc6);
> u64 time_hw, prev_hw, overflow_hw;
> + i915_reg_t reg = rc6->res_reg[id];
> unsigned int fw_domains;
> unsigned long flags;
> - unsigned int i;
> u32 mul, div;
>
> if (!rc6->supported)
> return 0;
>
> - /*
> - * Store previous hw counter values for counter wrap-around handling.
> - *
> - * There are only four interesting registers and they live next to each
> - * other so we can use the relative address, compared to the smallest
> - * one as the index into driver storage.
> - */
> - i = (i915_mmio_reg_offset(reg) -
> - i915_mmio_reg_offset(GEN6_GT_GFX_RC6_LOCKED)) / sizeof(u32);
> - if (drm_WARN_ON_ONCE(&i915->drm, i >= ARRAY_SIZE(rc6->cur_residency)))
> - return 0;
> -
> fw_domains = intel_uncore_forcewake_for_reg(uncore, reg, FW_REG_READ);
>
> spin_lock_irqsave(&uncore->lock, flags);
> @@ -789,11 +787,11 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
> /*
> * Counter wrap handling.
> *
> - * But relying on a sufficient frequency of queries otherwise counters
> - * can still wrap.
> + * Store previous hw counter values for counter wrap-around handling. But
> + * relying on a sufficient frequency of queries otherwise counters can still wrap.
> */
> - prev_hw = rc6->prev_hw_residency[i];
> - rc6->prev_hw_residency[i] = time_hw;
> + prev_hw = rc6->prev_hw_residency[id];
> + rc6->prev_hw_residency[id] = time_hw;
>
> /* RC6 delta from last sample. */
> if (time_hw >= prev_hw)
> @@ -802,8 +800,8 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
> time_hw += overflow_hw - prev_hw;
>
> /* Add delta to RC6 extended raw driver copy. */
> - time_hw += rc6->cur_residency[i];
> - rc6->cur_residency[i] = time_hw;
> + time_hw += rc6->cur_residency[id];
> + rc6->cur_residency[id] = time_hw;
>
> intel_uncore_forcewake_put__locked(uncore, fw_domains);
> spin_unlock_irqrestore(&uncore->lock, flags);
> @@ -811,9 +809,22 @@ u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, const i915_reg_t reg)
> return mul_u64_u32_div(time_hw, mul, div);
> }
>
> -u64 intel_rc6_residency_us(struct intel_rc6 *rc6, i915_reg_t reg)
> +u64 intel_rc6_residency_us(struct intel_rc6 *rc6, enum intel_rc6_res_type id)
> +{
> + return DIV_ROUND_UP_ULL(intel_rc6_residency_ns(rc6, id), 1000);
> +}
> +
> +void intel_rc6_print_residency(struct seq_file *m, const char *title,
> + enum intel_rc6_res_type id)
> {
> - return DIV_ROUND_UP_ULL(intel_rc6_residency_ns(rc6, reg), 1000);
> + struct intel_gt *gt = m->private;
> + i915_reg_t reg = gt->rc6.res_reg[id];
> + intel_wakeref_t wakeref;
> +
> + with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> + seq_printf(m, "%s %u (%llu us)\n", title,
> + intel_uncore_read(gt->uncore, reg),
> + intel_rc6_residency_us(>->rc6, id));
> }
>
> #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.h b/drivers/gpu/drm/i915/gt/intel_rc6.h
> index b6fea71afc223..456fa668a2769 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rc6.h
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.h
> @@ -6,10 +6,11 @@
> #ifndef INTEL_RC6_H
> #define INTEL_RC6_H
>
> -#include "i915_reg_defs.h"
> +#include <linux/types.h>
>
> -struct intel_engine_cs;
> +enum intel_rc6_res_type;
> struct intel_rc6;
> +struct seq_file;
>
> void intel_rc6_init(struct intel_rc6 *rc6);
> void intel_rc6_fini(struct intel_rc6 *rc6);
> @@ -21,7 +22,9 @@ void intel_rc6_sanitize(struct intel_rc6 *rc6);
> void intel_rc6_enable(struct intel_rc6 *rc6);
> void intel_rc6_disable(struct intel_rc6 *rc6);
>
> -u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, i915_reg_t reg);
> -u64 intel_rc6_residency_us(struct intel_rc6 *rc6, i915_reg_t reg);
> +u64 intel_rc6_residency_ns(struct intel_rc6 *rc6, enum intel_rc6_res_type id);
> +u64 intel_rc6_residency_us(struct intel_rc6 *rc6, enum intel_rc6_res_type id);
> +void intel_rc6_print_residency(struct seq_file *m, const char *title,
> + enum intel_rc6_res_type id);
>
> #endif /* INTEL_RC6_H */
> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6_types.h b/drivers/gpu/drm/i915/gt/intel_rc6_types.h
> index e747492b2f46e..fa23c4dce00b4 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rc6_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6_types.h
> @@ -13,9 +13,20 @@
>
> struct drm_i915_gem_object;
>
> +/* RC6 residency types */
> +enum intel_rc6_res_type {
> + INTEL_RC6_RES_RC6_LOCKED,
> + INTEL_RC6_RES_RC6,
> + INTEL_RC6_RES_RC6p,
> + INTEL_RC6_RES_RC6pp,
> + INTEL_RC6_RES_MAX,
> + INTEL_RC6_RES_VLV_MEDIA = INTEL_RC6_RES_RC6p,
> +};
> +
> struct intel_rc6 {
> - u64 prev_hw_residency[4];
> - u64 cur_residency[4];
> + i915_reg_t res_reg[INTEL_RC6_RES_MAX];
> + u64 prev_hw_residency[INTEL_RC6_RES_MAX];
> + u64 cur_residency[INTEL_RC6_RES_MAX];
>
> u32 ctl_enable;
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> index 8c70b7e120749..2ceeadecc639c 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> @@ -19,11 +19,11 @@ static u64 rc6_residency(struct intel_rc6 *rc6)
>
> /* XXX VLV_GT_MEDIA_RC6? */
>
> - result = intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6);
> + result = intel_rc6_residency_ns(rc6, INTEL_RC6_RES_RC6);
> if (HAS_RC6p(rc6_to_i915(rc6)))
> - result += intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6p);
> + result += intel_rc6_residency_ns(rc6, INTEL_RC6_RES_RC6p);
> if (HAS_RC6pp(rc6_to_i915(rc6)))
> - result += intel_rc6_residency_ns(rc6, GEN6_GT_GFX_RC6pp);
> + result += intel_rc6_residency_ns(rc6, INTEL_RC6_RES_RC6pp);
>
> return result;
> }
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 67140a87182f8..52531ab28c5f5 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -148,13 +148,13 @@ static u64 __get_rc6(struct intel_gt *gt)
> struct drm_i915_private *i915 = gt->i915;
> u64 val;
>
> - val = intel_rc6_residency_ns(>->rc6, GEN6_GT_GFX_RC6);
> + val = intel_rc6_residency_ns(>->rc6, INTEL_RC6_RES_RC6);
>
> if (HAS_RC6p(i915))
> - val += intel_rc6_residency_ns(>->rc6, GEN6_GT_GFX_RC6p);
> + val += intel_rc6_residency_ns(>->rc6, INTEL_RC6_RES_RC6p);
>
> if (HAS_RC6pp(i915))
> - val += intel_rc6_residency_ns(>->rc6, GEN6_GT_GFX_RC6pp);
> + val += intel_rc6_residency_ns(>->rc6, INTEL_RC6_RES_RC6pp);
>
> return val;
> }
> --
> 2.38.0
>
