diff mbox series

[v2] drm/i915/selftests: Add delay to stabilize frequency in live_rps_power

Message ID 20241203061114.2790448-1-sk.anirban@intel.com (mailing list archive)
State New
Headers show
Series [v2] drm/i915/selftests: Add delay to stabilize frequency in live_rps_power | expand

Commit Message

Anirban, Sk Dec. 3, 2024, 6:11 a.m. UTC
Add delays to allow frequency stabilization before power measurement
to fix sporadic power conservation issues in live_rps_power test.

v2:
  - Move delay to respective function (Badal)

Signed-off-by: Sk Anirban <sk.anirban@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_rps.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Nilawar, Badal Dec. 3, 2024, 9:23 a.m. UTC | #1
On 03-12-2024 11:41, Sk Anirban wrote:
> Add delays to allow frequency stabilization before power measurement
> to fix sporadic power conservation issues in live_rps_power test.
> 
> v2:
>    - Move delay to respective function (Badal)
> 
> Signed-off-by: Sk Anirban <sk.anirban@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/selftest_rps.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
> index dcef8d498919..c207a4fb03bf 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> @@ -1125,6 +1125,7 @@ static u64 measure_power(struct intel_rps *rps, int *freq)
>   static u64 measure_power_at(struct intel_rps *rps, int *freq)
>   {
>   	*freq = rps_set_check(rps, *freq);
> +	msleep(100);

Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>

Regards,
Badal
>   	return measure_power(rps, freq);
>   }
>
Jani Nikula Dec. 3, 2024, 10:46 a.m. UTC | #2
On Tue, 03 Dec 2024, Sk Anirban <sk.anirban@intel.com> wrote:
> Add delays to allow frequency stabilization before power measurement
> to fix sporadic power conservation issues in live_rps_power test.

Looks like band-aid. What's the root cause?

BR,
Jani.

>
> v2:
>   - Move delay to respective function (Badal)
>
> Signed-off-by: Sk Anirban <sk.anirban@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/selftest_rps.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
> index dcef8d498919..c207a4fb03bf 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> @@ -1125,6 +1125,7 @@ static u64 measure_power(struct intel_rps *rps, int *freq)
>  static u64 measure_power_at(struct intel_rps *rps, int *freq)
>  {
>  	*freq = rps_set_check(rps, *freq);
> +	msleep(100);
>  	return measure_power(rps, freq);
>  }
Anirban, Sk Dec. 5, 2024, 6:37 a.m. UTC | #3
On 03-12-2024 16:16, Jani Nikula wrote:
> On Tue, 03 Dec 2024, Sk Anirban <sk.anirban@intel.com> wrote:
>> Add delays to allow frequency stabilization before power measurement
>> to fix sporadic power conservation issues in live_rps_power test.
> Looks like band-aid. What's the root cause?
>
> BR,
> Jani.

In some cases, we have observed that the power consumption at the 
minimum frequency is greater than at the maximum frequency. This is 
likely due to the actual frequency not settling. The issue is sporadic, 
and to address this, we are adding a delay after setting the freq and 
before measuring the power.

Thanks,
Anirban

>> v2:
>>    - Move delay to respective function (Badal)
>>
>> Signed-off-by: Sk Anirban <sk.anirban@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/selftest_rps.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
>> index dcef8d498919..c207a4fb03bf 100644
>> --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
>> +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
>> @@ -1125,6 +1125,7 @@ static u64 measure_power(struct intel_rps *rps, int *freq)
>>   static u64 measure_power_at(struct intel_rps *rps, int *freq)
>>   {
>>   	*freq = rps_set_check(rps, *freq);
>> +	msleep(100);
>>   	return measure_power(rps, freq);
>>   }
Raag Jadav Dec. 7, 2024, 6:22 a.m. UTC | #4
On Thu, Dec 05, 2024 at 12:07:53PM +0530, Anirban, Sk wrote:
> On 03-12-2024 16:16, Jani Nikula wrote:
> > On Tue, 03 Dec 2024, Sk Anirban <sk.anirban@intel.com> wrote:
> > > Add delays to allow frequency stabilization before power measurement
> > > to fix sporadic power conservation issues in live_rps_power test.
> > Looks like band-aid. What's the root cause?
> 
> In some cases, we have observed that the power consumption at the minimum
> frequency is greater than at the maximum frequency. This is likely due to
> the actual frequency not settling. The issue is sporadic, and to address
> this, we are adding a delay after setting the freq and before measuring the
> power.

Why not use wait_for_freq()?

Raag

> > > v2:
> > >    - Move delay to respective function (Badal)
> > > 
> > > Signed-off-by: Sk Anirban <sk.anirban@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/gt/selftest_rps.c | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
> > > index dcef8d498919..c207a4fb03bf 100644
> > > --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> > > +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> > > @@ -1125,6 +1125,7 @@ static u64 measure_power(struct intel_rps *rps, int *freq)
> > >   static u64 measure_power_at(struct intel_rps *rps, int *freq)
> > >   {
> > >   	*freq = rps_set_check(rps, *freq);
> > > +	msleep(100);
> > >   	return measure_power(rps, freq);
> > >   }
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
index dcef8d498919..c207a4fb03bf 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rps.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
@@ -1125,6 +1125,7 @@  static u64 measure_power(struct intel_rps *rps, int *freq)
 static u64 measure_power_at(struct intel_rps *rps, int *freq)
 {
 	*freq = rps_set_check(rps, *freq);
+	msleep(100);
 	return measure_power(rps, freq);
 }