[Intel-gfx] [PATCH] drm/i915/gt: Sanitize GT first

[Tvrtko Ursulin]

On 27/04/2020 10:55, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2020-04-27 10:50:01)
>>
>> On 27/04/2020 09:40, Chris Wilson wrote:
>>> We see that if the HW doesn't actually sleep, the HW may eat the poison
>>> we set in its write-only HWSP during sanitize:
>>>
>>>     intel_gt_resume.part.8: 0000:00:02.0
>>>     __gt_unpark: 0000:00:02.0
>>>     gt_sanitize: 0000:00:02.0 force:yes
>>>     process_csb: 0000:00:02.0 vcs0: cs-irq head=5, tail=90
>>>     process_csb: 0000:00:02.0 vcs0: csb[0]: status=0x5a5a5a5a:0x5a5a5a5a
>>>     assert_pending_valid: Nothing pending for promotion!
>>>
>>> The CS TAIL pointer should have been reset by reset_csb_pointers(), so
>>> in this case it is likely that we have read back from the CPU cache and
>>> so we must clflush our control over that page. In doing so, push the
>>> sanitisation to the start of the GT sequence so that our poisoning is
>>> assuredly before we start talking to the HW.
>>>
>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> ---
>>>    drivers/gpu/drm/i915/gt/intel_gt_pm.c | 3 ++-
>>>    drivers/gpu/drm/i915/gt/intel_lrc.c   | 3 +++
>>>    2 files changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
>>> index 4c4c74ef4f21..5097786f4375 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
>>> @@ -198,11 +198,12 @@ int intel_gt_resume(struct intel_gt *gt)
>>>         * Only the kernel contexts should remain pinned over suspend,
>>>         * allowing us to fixup the user contexts on their first pin.
>>>         */
>>> +     gt_sanitize(gt, true);
>>> +
>>>        intel_gt_pm_get(gt);
>>
>> gt_sanitize declares hw access so I thin it should still be after this
>> pm get.
> 
> We control the rpm/uncore access inside gt_sanitize. And the reset we do
> there avoid interfering with the gt_pm, so I think this is safe. Note
> that the gt_sanitize() we do later is after we release gt_pm. So it
> seems reasonable to say that we do it before/after gt_pm.

On the suspend path? Okay.. I guess that proves it works.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko