diff mbox series

[5/5] drm/i915/mtl: C6 residency and C state type for MTL SAMedia

Message ID 20221019233721.3270601-6-ashutosh.dixit@intel.com (mailing list archive)
State New, archived
Headers show
Series i915: CAGF and RC6 changes for MTL | expand

Commit Message

Ashutosh Dixit Oct. 19, 2022, 11:37 p.m. UTC
From: Badal Nilawar <badal.nilawar@intel.com>

Add support for C6 residency and C state type for MTL SAMedia. Also add
mtl_drpc.

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@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
 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(-)

Comments

Rodrigo Vivi Oct. 21, 2022, 4:35 p.m. UTC | #1
On Wed, Oct 19, 2022 at 04:37:21PM -0700, Ashutosh Dixit wrote:
> From: Badal Nilawar <badal.nilawar@intel.com>
> 
> 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@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>  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
>
Ashutosh Dixit Oct. 24, 2022, 7:16 p.m. UTC | #2
On Fri, 21 Oct 2022 09:35:32 -0700, Rodrigo Vivi wrote:
>

Hi Rodrigo,

> On Wed, Oct 19, 2022 at 04:37:21PM -0700, Ashutosh Dixit wrote:
> > From: Badal Nilawar <badal.nilawar@intel.com>
> >
> > 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...

Sorry didn't understand what you mean by "slip", you mean the patch should
be split in two?

> 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@intel.com>
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > ---
> >  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?

This function is called for both render and media gt's. But let's think
about this. We can call easily call them e.g. RC6 for render and MC6 for
media too if that is more accurate and descriptive. On the other hand, do
we really need to introduce a new term like MC6? Maybe we just stick to
RC/RC6 terminology for anything on the GPU?

> But well, MC6 seems to be a totally different thing and CC6

MC6 is not the same as RC6 for the media tile?

> and CC6 is really strange because the C stands for Core and this can get
> very confusing with the SoC or CPU C states...  :(

Yes Bspec 66300 refers to these as core C states but refers to GT and
IA. So it's confusing.

> At least with the Render we know which level of the IP we
> are looking at when looking at media...

Yup that's why I've left this as RC/RC6 in Patch v6.

>
> > +	switch (REG_FIELD_GET(MTL_CC_MASK, gt_core_status)) {
> > +	case MTL_CC0:
> > +		seq_puts(m, "on\n");
>
> maybe "*C0" instead of "on"?

Done in v6. Though this string is "on" also in the previous function
gen6_drpc. Also, if we are calling this C0 we could call the C6 state as
just C6 (which would mean RC6 for render and MC6 for media). But I thought
RC6 is better for both render and media.

>
> > +		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?

Done in v6.

>
> > +		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?

Yes name is confusing but is the same as Bspec and also gen6_drpc. So the
prints "Media Power Well: Up" or "Media Power Well: Down" are correct (0 is
down, 1 is up). Similarly for Render below.

Thanks.
--
Ashutosh

>
> > +	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
> >
Rodrigo Vivi Oct. 24, 2022, 7:49 p.m. UTC | #3
On Mon, Oct 24, 2022 at 12:16:28PM -0700, Dixit, Ashutosh wrote:
> On Fri, 21 Oct 2022 09:35:32 -0700, Rodrigo Vivi wrote:
> >
> 
> Hi Rodrigo,
> 
> > On Wed, Oct 19, 2022 at 04:37:21PM -0700, Ashutosh Dixit wrote:
> > > From: Badal Nilawar <badal.nilawar@intel.com>
> > >
> > > 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...
> 
> Sorry didn't understand what you mean by "slip", you mean the patch should
> be split in two?

yeap, I meant split.. sorry for the typo...

> 
> > 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@intel.com>
> > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > > ---
> > >  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?
> 
> This function is called for both render and media gt's. But let's think
> about this. We can call easily call them e.g. RC6 for render and MC6 for
> media too if that is more accurate and descriptive. On the other hand, do
> we really need to introduce a new term like MC6? Maybe we just stick to
> RC/RC6 terminology for anything on the GPU?
> 
> > But well, MC6 seems to be a totally different thing and CC6
> 
> MC6 is not the same as RC6 for the media tile?

I saw people using this in some bug, but I thought this was for the package
since they were refering to the rc6 as well...

so you can see how  confusing this bad naming is by itself... not a single
document or bspec page that put a good name we can use... so let's not
invent it here and move with the RC6.

> 
> > and CC6 is really strange because the C stands for Core and this can get
> > very confusing with the SoC or CPU C states...  :(
> 
> Yes Bspec 66300 refers to these as core C states but refers to GT and
> IA. So it's confusing.
> 
> > At least with the Render we know which level of the IP we
> > are looking at when looking at media...
> 
> Yup that's why I've left this as RC/RC6 in Patch v6.

ack

> 
> >
> > > +	switch (REG_FIELD_GET(MTL_CC_MASK, gt_core_status)) {
> > > +	case MTL_CC0:
> > > +		seq_puts(m, "on\n");
> >
> > maybe "*C0" instead of "on"?
> 
> Done in v6. Though this string is "on" also in the previous function
> gen6_drpc. Also, if we are calling this C0 we could call the C6 state as
> just C6 (which would mean RC6 for render and MC6 for media). But I thought
> RC6 is better for both render and media.

oh, now that we agreed on staying with the RC I believe RC0 is better.


> 
> >
> > > +		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?
> 
> Done in v6.
> 
> >
> > > +		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?
> 
> Yes name is confusing but is the same as Bspec and also gen6_drpc. So the
> prints "Media Power Well: Up" or "Media Power Well: Down" are correct (0 is
> down, 1 is up). Similarly for Render below.

:( oh, indeed!

Let's stay with it.

> 
> Thanks.
> --
> Ashutosh
> 
> >
> > > +	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
> > >
diff mbox series

Patch

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: ");
+	switch (REG_FIELD_GET(MTL_CC_MASK, gt_core_status)) {
+	case MTL_CC0:
+		seq_puts(m, "on\n");
+		break;
+	case MTL_CC6:
+		seq_puts(m, "RC6\n");
+		break;
+	default:
+		seq_puts(m, "Unknown\n");
+		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");
+	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)