Re: [Intel-gfx] [RFC 4/4] drm/i915: Expose RPS thresholds in sysfs

Tvrtko Ursulin
2023-05-22T12:03:59Z

On 19/05/2023 21:56, Rodrigo Vivi wrote:
> On Fri, Apr 28, 2023 at 09:44:53AM +0100, Tvrtko Ursulin wrote:
>>
>> On 28/04/2023 09:14, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> User feedback indicates significant performance gains are possible in
>>> specific games with non default RPS up/down thresholds.
>>>
>>> Expose these tunables via sysfs which will allow users to achieve best
>>> performance when running games and best power efficiency elsewhere.
>>>
>>> Note this patch supports non GuC based platforms only.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> References: https://gitlab.freedesktop.org/drm/intel/-/issues/8389
>>
>> [snip]
>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
>>> index a5a7315f5ace..f790e81546ff 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
>>> @@ -2588,7 +2588,12 @@ static int rps_set_threshold(struct intel_rps *rps, u8 *threshold, u8 val)
>>> ret = mutex_lock_interruptible(&rps->lock);
>>> if (ret)
>>> return ret;
>>> - *threshold = val;
>>> + if (*threshold != val) {
>>> + *threshold = val;
>>> + intel_rps_set(rps, clamp(rps->cur_freq,
>>> + rps->min_freq_softlimit,
>>> + rps->max_freq_softlimit));
>>> + }
>>> mutex_unlock(&rps->lock);
>>> return 0;
>>
>> This hunk belongs to a previous patch - moved locally.
>
> I probably missed something then becuase I didn't miss this in any
> of the previous patches. To the point that this looked like a new
> separated patch.

Depends how you look at it. 3/4 adds a helper which changes the
threshold, but it was only updating the sw state and so when those
values would actually get written into registers (derived) would depend
on the workload. So I thought it was important to move the call to
intel_rps_set, which actually will trigger hw reprogramming, into the
patch which claims to be adding the helper.

Note v2 of the helper (just sent) which actually fixes this further by
forcing this re-programming to happen.

Regards,

Tvrtko