[Intel-gfx] [PATCH v9 4/4] drm/i915/selftests: skip comparison of power for discrete graphics

[Dixit, Ashutosh]

On Wed, 05 Apr 2023 18:02:04 -0700, Dixit, Ashutosh wrote:
>
> On Tue, 04 Apr 2023 23:59:30 -0700, Riana Tauro wrote:
> >
>
> Hi Riana,
>
> > Hwmon reads the energy/power consumed by discrete soc,
> > i.e. energy/power includes the power drawn from non-gfx discrete
> > components
> >
> > This test uses the power consumed by GT to validate RC6
> > power consumption.
> > Skip comparison of power for discrete graphics
> >
> > TODO : measure power of GT in discrete graphics and modify the
> > condition
> >
> > v2: update commit message (Anshuman)
> >
> > Signed-off-by: Riana Tauro <riana.tauro at intel.com>
> > Reviewed-by: Anshuman Gupta <anshuman.gupta at intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/selftest_rc6.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> > index 682f2fe67b3a..47165f490449 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> > @@ -107,7 +107,15 @@ int live_rc6_manual(void *arg)
> >				      ktime_to_ns(dt));
> >		pr_info("GPU consumed %llduW in RC0 and %llduW in RC6\n",
> >			rc0_power, rc6_power);
> > -		if (2 * rc6_power > rc0_power) {
>
> So this condition is not being met for dGfx?
>
> > +
> > +		/*
> > +		 * Condition valid for integrated graphics
> > +		 * On discrete graphics, hwwmon reads the energy/power from
> > +		 * discrete SOC that includes non-gfx components.
>
> On dGfx, is this true even when we have per-gt level energy available? Or
> only when we have device level energy but not per-gt level energy (when
> total number of gt's is 1 and we only expose device level energy but not gt
> level energy)?

Seems we are going this for device level energy. In that case should we
change the condition below to say device level energy for dGfx (rather than
just dGfx)? So it will be clear. Something "analogous" to how we do this in
i915_hwmon_get_energy():

+       else if (!HAS_EXTRA_GT_LIST(gt->i915) && hwm_energy_is_visible(ddat, hwmon_energy_input))
+               hwm_energy(ddat, energy);

> > +		 * TODO : Measure power of GT for discrete graphics and
> > +		 * modify the condition
>
> If we are adding this TODO, how are we planning to do this?

So here we are going to come up with a different condition for this device
level energy? Is it possible to do this now itself rather than add the
TODO?

Thanks.
--
Ashutosh

>
> > +		 */
> > +		if (!IS_DGFX(gt->i915) && (2 * rc6_power > rc0_power)) {
> >			pr_err("GPU leaked energy while in RC6!\n");
> >			err = -EINVAL;
> >			goto out_unlock;