Re: [igt-dev] [PATCH i-g-t] i915/i915_pm_rpm: Only check for suspend failures after each debugfs entry

[Martin Peres <martin.peres@linux.intel.com>]

On 2020-02-21 10:21, Chris Wilson wrote:
> Quoting Martin Peres (2020-02-21 07:33:59)
>> On 2020-02-20 19:41, Chris Wilson wrote:
>>> Since we check before and then after each debugfs entry, we do not need
>>> to check before each time as well. We will error out as soon as it does
>>> fail, at all other times we know the system to be idle.
>>>
>>> No impact on runtime for glk (which apparently is one of the better
>>> behaving systems).
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Martin Peres <martin.peres@linux.intel.com>
>>
>> I don't like this patch because the first read might not have the gpu
>> suspended, and there shouldn't be much overhead in checking twice rather
>> than once.
>>
>> What's your rationale here?
> 
> We always do a check before after each file. We start in a known state,
> and expect to be able to return to that suspended state, and the _real_
> guts of the test is that any device access is accounted for.
> 
> assert(suspended) would be a better check for non-interference.

I would feel better with assert(suspended) added, but would it really
speed anything up since I assume wait_for_suspended() should be
instantaneous if we are already suspended, right?

>  
>> To me, the issue is that some platforms suspend in milliseconds while
>> some take seconds, and that might be indicative a real bug in the driver.
> 
> Exactly.

Good to hear :)

> -Chris
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev