Re: [Intel-gfx] [PATCH] drm/i915/pmu: Check actual RC6 status

From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Intel-gfx@lists.freedesktop.org,
	Eero T Tamminen <eero.t.tamminen@intel.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/pmu: Check actual RC6 status
Date: Thu, 1 Apr 2021 05:19:03 -0400	[thread overview]
Message-ID: <YGWQB+8gWgmZ/6Mg@intel.com> (raw)
In-Reply-To: <20210331101850.2582027-1-tvrtko.ursulin@linux.intel.com>

On Wed, Mar 31, 2021 at 11:18:50AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> RC6 support cannot be simply established by looking at the static device
> HAS_RC6() flag. There are cases which disable RC6 at driver load time so
> use the status of those check when deciding whether to enumerate the rc6
> counter.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Reported-by: Eero T Tamminen <eero.t.tamminen@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pmu.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 41651ac255fa..a75cd1db320b 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -476,6 +476,8 @@ engine_event_status(struct intel_engine_cs *engine,
>  static int
>  config_status(struct drm_i915_private *i915, u64 config)
>  {
> +	struct intel_gt *gt = &i915->gt;
> +
>  	switch (config) {
>  	case I915_PMU_ACTUAL_FREQUENCY:
>  		if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> @@ -489,7 +491,7 @@ config_status(struct drm_i915_private *i915, u64 config)
>  	case I915_PMU_INTERRUPTS:
>  		break;
>  	case I915_PMU_RC6_RESIDENCY:
> -		if (!HAS_RC6(i915))
> +		if (!gt->rc6.supported)

Is this really going to remove any confusion?
Right now it is there but with residency 0, but after this change the event is
not there anymore so I wonder if we are not just changing to a different kind
of confusion on users.

>  			return -ENODEV;

would a different return help somehow?

>  		break;
>  	case I915_PMU_SOFTWARE_GT_AWAKE_TIME:
> -- 
> 2.27.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel