On Wed, Oct 19, 2022 at 04:37:21PM -0700, Ashutosh Dixit wrote:
> From: Badal Nilawar <badal.nilawar@xxxxxxxxx>
>
> Add support for C6 residency and C state type for MTL SAMedia. Also add
> mtl_drpc.
I believe this patch deserves a slip between the actual support and
the debugfs, but I'm late to the review, so feel free to ignore this
comment...
but I do have more dummy doubts below:
>
> v2: Fixed review comments (Ashutosh)
> v3: Sort registers and fix whitespace errors in intel_gt_regs.h (Matt R)
> Remove MTL_CC_SHIFT (Ashutosh)
> Adapt to RC6 residency register code refactor (Jani N)
> v4: Move MTL branch to top in drpc_show
> v5: Use FORCEWAKE_MT identical to gen6_drpc (Ashutosh)
>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
> Signed-off-by: Badal Nilawar <badal.nilawar@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 58 ++++++++++++++++++-
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 5 ++
> drivers/gpu/drm/i915/gt/intel_rc6.c | 17 ++++--
> 3 files changed, 75 insertions(+), 5 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 5d6b346831393..f15a7486a9866 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> @@ -256,6 +256,60 @@ static int ilk_drpc(struct seq_file *m)
> return 0;
> }
>
> +static int mtl_drpc(struct seq_file *m)
> +{
> + struct intel_gt *gt = m->private;
> + struct intel_uncore *uncore = gt->uncore;
> + u32 gt_core_status, rcctl1, mt_fwake_req;
> + u32 mtl_powergate_enable = 0, mtl_powergate_status = 0;
> +
> + mt_fwake_req = intel_uncore_read_fw(uncore, FORCEWAKE_MT);
> + gt_core_status = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1);
> +
> + rcctl1 = intel_uncore_read(uncore, GEN6_RC_CONTROL);
> + mtl_powergate_enable = intel_uncore_read(uncore, GEN9_PG_ENABLE);
> + mtl_powergate_status = intel_uncore_read(uncore,
> + GEN9_PWRGT_DOMAIN_STATUS);
> +
> + seq_printf(m, "RC6 Enabled: %s\n",
> + str_yes_no(rcctl1 & GEN6_RC_CTL_RC6_ENABLE));
> + if (gt->type == GT_MEDIA) {
> + seq_printf(m, "Media Well Gating Enabled: %s\n",
> + str_yes_no(mtl_powergate_enable & GEN9_MEDIA_PG_ENABLE));
> + } else {
> + seq_printf(m, "Render Well Gating Enabled: %s\n",
> + str_yes_no(mtl_powergate_enable & GEN9_RENDER_PG_ENABLE));
> + }
> +
> + seq_puts(m, "Current RC state: ");
(Just a "loud" thought here in this chunck, but no actual action requested)
should we really use "R" (Render) for this Media C state?
But well, MC6 seems to be a totally different thing and CC6 is
really strange because the C stands for Core and this can get
very confusing with the SoC or CPU C states... :(
At least with the Render we know which level of the IP we
are looking at when looking at media...
> + switch (REG_FIELD_GET(MTL_CC_MASK, gt_core_status)) {
> + case MTL_CC0:
> + seq_puts(m, "on\n");
maybe "*C0" instead of "on"?
> + break;
> + case MTL_CC6:
> + seq_puts(m, "RC6\n");
> + break;
> + default:
> + seq_puts(m, "Unknown\n");
maybe use a MISSING_CASE() here?
or raise a WARN?
> + break;
> + }
> +
> + seq_printf(m, "Multi-threaded Forcewake Request: 0x%x\n", mt_fwake_req);
> + if (gt->type == GT_MEDIA)
> + seq_printf(m, "Media Power Well: %s\n",
> + (mtl_powergate_status &
> + GEN9_PWRGT_MEDIA_STATUS_MASK) ? "Up" : "Down");
gate is up and power is down or gate is down and power is up?
> + else
> + seq_printf(m, "Render Power Well: %s\n",
> + (mtl_powergate_status &
> + GEN9_PWRGT_RENDER_STATUS_MASK) ? "Up" : "Down");
> +
> + /* Works for both render and media gt's */
> + intel_rc6_print_residency(m, "RC6 residency since boot:", INTEL_RC6_RES_RC6);
> +
> + return fw_domains_show(m, NULL);
> +}
> +
> static int drpc_show(struct seq_file *m, void *unused)
> {
> struct intel_gt *gt = m->private;
> @@ -264,7 +318,9 @@ static int drpc_show(struct seq_file *m, void *unused)
> int err = -ENODEV;
>
> with_intel_runtime_pm(gt->uncore->rpm, wakeref) {
> - if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> + if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> + err = mtl_drpc(m);
> + else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> err = vlv_drpc(m);
> else if (GRAPHICS_VER(i915) >= 6)
> err = gen6_drpc(m);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index d8dbd0ac3b064..a0ddaf243593c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -24,6 +24,9 @@
> /* MTL workpoint reg to get core C state and actual freq of 3D, SAMedia */
> #define MTL_MIRROR_TARGET_WP1 _MMIO(0xc60)
> #define MTL_CAGF_MASK REG_GENMASK(8, 0)
> +#define MTL_CC0 0x0
> +#define MTL_CC6 0x3
> +#define MTL_CC_MASK REG_GENMASK(12, 9)
>
> /* RPM unit config (Gen8+) */
> #define RPM_CONFIG0 _MMIO(0xd00)
> @@ -1512,6 +1515,8 @@
> #define FORCEWAKE_MEDIA_VLV _MMIO(0x1300b8)
> #define FORCEWAKE_ACK_MEDIA_VLV _MMIO(0x1300bc)
>
> +#define MTL_MEDIA_MC6 _MMIO(0x138048)
> +
> #define GEN6_GT_THREAD_STATUS_REG _MMIO(0x13805c)
> #define GEN6_GT_THREAD_STATUS_CORE_MASK 0x7
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> index 6db4e60d5fba5..2ee4051e4d961 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> @@ -553,10 +553,19 @@ static void __intel_rc6_disable(struct intel_rc6 *rc6)
>
> 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;
> + memset(rc6->res_reg, INVALID_MMIO_REG.reg, sizeof(rc6->res_reg));
> +
> + switch (rc6_to_gt(rc6)->type) {
> + case GT_MEDIA:
> + rc6->res_reg[INTEL_RC6_RES_RC6] = MTL_MEDIA_MC6;
> + break;
> + default:
> + 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;
> + break;
> + }
> }
>
> void intel_rc6_init(struct intel_rc6 *rc6)
> --
> 2.38.0
>
