* [PATCH] drm/i915/guc/slpc: Apply min softlimit correctly
@ 2023-06-09 22:02 Vinay Belgaumkar
2023-06-14 2:25 ` Dixit, Ashutosh
2023-06-15 17:04 ` Dixit, Ashutosh
0 siblings, 2 replies; 4+ messages in thread
From: Vinay Belgaumkar @ 2023-06-09 22:02 UTC (permalink / raw)
To: intel-gfx, dri-devel
We were skipping when min_softlimit was equal to RPn. We need to apply
it rergardless as efficient frequency will push the SLPC min to RPe.
This will break scenarios where user sets a min softlimit < RPe before
reset and then performs a GT reset.
Fixes: 95ccf312a1e4 ("drm/i915/guc/slpc: Allow SLPC to use efficient frequency")
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index 01b75529311c..ee9f83af7cf6 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -606,7 +606,7 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc)
if (unlikely(ret))
return ret;
slpc_to_gt(slpc)->defaults.min_freq = slpc->min_freq_softlimit;
- } else if (slpc->min_freq_softlimit != slpc->min_freq) {
+ } else {
return intel_guc_slpc_set_min_freq(slpc,
slpc->min_freq_softlimit);
}
--
2.38.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/i915/guc/slpc: Apply min softlimit correctly
2023-06-09 22:02 [PATCH] drm/i915/guc/slpc: Apply min softlimit correctly Vinay Belgaumkar
@ 2023-06-14 2:25 ` Dixit, Ashutosh
2023-06-14 7:35 ` Belgaumkar, Vinay
2023-06-15 17:04 ` Dixit, Ashutosh
1 sibling, 1 reply; 4+ messages in thread
From: Dixit, Ashutosh @ 2023-06-14 2:25 UTC (permalink / raw)
To: Vinay Belgaumkar; +Cc: intel-gfx, dri-devel
On Fri, 09 Jun 2023 15:02:52 -0700, Vinay Belgaumkar wrote:
>
Hi Vinay,
> We were skipping when min_softlimit was equal to RPn. We need to apply
> it rergardless as efficient frequency will push the SLPC min to RPe.
regardless
> This will break scenarios where user sets a min softlimit < RPe before
> reset and then performs a GT reset.
Can you explain the reason for the patch clearly in terms of variables in
the code, what variable has what value and what is the bug. I am not
following from the above description.
Thanks.
--
Ashutosh
>
> Fixes: 95ccf312a1e4 ("drm/i915/guc/slpc: Allow SLPC to use efficient frequency")
>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index 01b75529311c..ee9f83af7cf6 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -606,7 +606,7 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc)
> if (unlikely(ret))
> return ret;
> slpc_to_gt(slpc)->defaults.min_freq = slpc->min_freq_softlimit;
> - } else if (slpc->min_freq_softlimit != slpc->min_freq) {
> + } else {
> return intel_guc_slpc_set_min_freq(slpc,
> slpc->min_freq_softlimit);
> }
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/i915/guc/slpc: Apply min softlimit correctly
2023-06-14 2:25 ` Dixit, Ashutosh
@ 2023-06-14 7:35 ` Belgaumkar, Vinay
0 siblings, 0 replies; 4+ messages in thread
From: Belgaumkar, Vinay @ 2023-06-14 7:35 UTC (permalink / raw)
To: Dixit, Ashutosh; +Cc: intel-gfx, dri-devel
On 6/13/2023 7:25 PM, Dixit, Ashutosh wrote:
> On Fri, 09 Jun 2023 15:02:52 -0700, Vinay Belgaumkar wrote:
> Hi Vinay,
>
>> We were skipping when min_softlimit was equal to RPn. We need to apply
>> it rergardless as efficient frequency will push the SLPC min to RPe.
> regardless
>
>> This will break scenarios where user sets a min softlimit < RPe before
>> reset and then performs a GT reset.
> Can you explain the reason for the patch clearly in terms of variables in
> the code, what variable has what value and what is the bug. I am not
> following from the above description.
Hi Ashutosh,
Scenario being fixed here is exactly the one in i915_pm_freq_api
reset/suspend subtests (currently in review). Test sets min freq to RPn
and then performs a reset. It then checks if cur_freq is RPn.
Here's the sequence that shows the problem-
RPLS:/home/gta# modprobe i915
RPLS:/home/gta# echo 1 > /sys/class/drm/card0/gt/gt0/slpc_ignore_eff_freq
RPLS:/home/gta# echo 300 > /sys/class/drm/card0/gt_min_freq_mhz (RPn)
RPLS:/home/gta# cat /sys/class/drm/card0/gt_cur_freq_mhz --> cur == RPn
as expected
300
RPLS:/home/gta# echo 1 > /sys/kernel/debug/dri/0/gt0/reset --> reset
RPLS:/home/gta# cat /sys/class/drm/card0/gt_min_freq_mhz --> shows the
internal cached variable correctly
300
RPLS:/home/gta# cat /sys/class/drm/card0/gt_cur_freq_mhz --> actual freq
being requested by SLPC (it's not RPn!!)
700
We need to sync up driver min freq value and SLPC min after a
reset/suspend. Currently, we skip if the user had manually set min to
RPn (this was an optimization we had before we enabled efficient freq
usage).
Thanks,
Vinay.
>
> Thanks.
> --
> Ashutosh
>
>
>> Fixes: 95ccf312a1e4 ("drm/i915/guc/slpc: Allow SLPC to use efficient frequency")
>>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> ---
>> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>> index 01b75529311c..ee9f83af7cf6 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>> @@ -606,7 +606,7 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc)
>> if (unlikely(ret))
>> return ret;
>> slpc_to_gt(slpc)->defaults.min_freq = slpc->min_freq_softlimit;
>> - } else if (slpc->min_freq_softlimit != slpc->min_freq) {
>> + } else {
>> return intel_guc_slpc_set_min_freq(slpc,
>> slpc->min_freq_softlimit);
>> }
>> --
>> 2.38.1
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915/guc/slpc: Apply min softlimit correctly
2023-06-09 22:02 [PATCH] drm/i915/guc/slpc: Apply min softlimit correctly Vinay Belgaumkar
2023-06-14 2:25 ` Dixit, Ashutosh
@ 2023-06-15 17:04 ` Dixit, Ashutosh
1 sibling, 0 replies; 4+ messages in thread
From: Dixit, Ashutosh @ 2023-06-15 17:04 UTC (permalink / raw)
To: Vinay Belgaumkar; +Cc: intel-gfx, dri-devel
On Fri, 09 Jun 2023 15:02:52 -0700, Vinay Belgaumkar wrote:
>
Hi Vinay,
> We were skipping when min_softlimit was equal to RPn. We need to apply
> it rergardless as efficient frequency will push the SLPC min to RPe.
> This will break scenarios where user sets a min softlimit < RPe before
> reset and then performs a GT reset.
>
> Fixes: 95ccf312a1e4 ("drm/i915/guc/slpc: Allow SLPC to use efficient frequency")
>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index 01b75529311c..ee9f83af7cf6 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -606,7 +606,7 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc)
> if (unlikely(ret))
> return ret;
> slpc_to_gt(slpc)->defaults.min_freq = slpc->min_freq_softlimit;
> - } else if (slpc->min_freq_softlimit != slpc->min_freq) {
> + } else {
> return intel_guc_slpc_set_min_freq(slpc,
> slpc->min_freq_softlimit);
IMO the current code is unnecessarily complicated and confusing and similar
changes (with a little tweaking) should be made for max_freq too. But at
least this is a step in the right direction so:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> }
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-15 17:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-09 22:02 [PATCH] drm/i915/guc/slpc: Apply min softlimit correctly Vinay Belgaumkar
2023-06-14 2:25 ` Dixit, Ashutosh
2023-06-14 7:35 ` Belgaumkar, Vinay
2023-06-15 17:04 ` Dixit, Ashutosh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).