diff mbox series

[v9,4/4] drm/i915/selftests: skip comparison of power for discrete graphics

Message ID 20230405065930.3576936-5-riana.tauro@intel.com (mailing list archive)
State New, archived
Headers show
Series Add hwmon support for dgfx selftests | expand

Commit Message

Riana Tauro April 5, 2023, 6:59 a.m. UTC
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@intel.com>
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_rc6.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Dixit, Ashutosh April 6, 2023, 1:02 a.m. UTC | #1
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@intel.com>
> Reviewed-by: Anshuman Gupta <anshuman.gupta@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)?


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

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

Thanks.
--
Ashutosh
Dixit, Ashutosh April 10, 2023, 3:50 p.m. UTC | #2
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@intel.com>
> > Reviewed-by: Anshuman Gupta <anshuman.gupta@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;
diff mbox series

Patch

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) {
+
+		/*
+		 * Condition valid for integrated graphics
+		 * On discrete graphics, hwwmon reads the energy/power from
+		 * discrete SOC that includes non-gfx components.
+		 * TODO : Measure power of GT for discrete graphics and
+		 * modify the condition
+		 */
+		if (!IS_DGFX(gt->i915) && (2 * rc6_power > rc0_power)) {
 			pr_err("GPU leaked energy while in RC6!\n");
 			err = -EINVAL;
 			goto out_unlock;