[Intel-gfx] [PATCH] drm/i915/perf: Use GTT when saving/restoring engine GPR

[Lionel Landwerlin]

On 09/07/2020 23:37, Chris Wilson wrote:
> Quoting Umesh Nerlige Ramappa (2020-07-09 20:58:37)
>> MI_STORE_REGISTER_MEM and MI_LOAD_REGISTER_MEM need to know which
>> translation to use when saving restoring the engine general purpose
>> registers to and from the GT scratch. Since GT scratch is mapped to
>> ggtt, we need to set an additional bit in the command to use GTT.
>>
>> Fixes: daed3e44396d17 ("drm/i915/perf: implement active wait for noa configurations")
>> Suggested-by: Prathap Kumar Valsan <prathap.kumar.valsan at intel.com>
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_perf.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index de69d430b1ed..c6f6370283cf 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -1592,6 +1592,7 @@ static u32 *save_restore_register(struct i915_perf_stream *stream, u32 *cs,
>>          u32 d;
>>   
>>          cmd = save ? MI_STORE_REGISTER_MEM : MI_LOAD_REGISTER_MEM;
>> +       cmd |= MI_SRM_LRM_GLOBAL_GTT;
> Indeed.
>
> For bonus points, please write a selftest to verify that the user's GPR
> are restored. Something as simple as live_noa_delay, but instead of
> measuring the time; submit a request to write telltales into the GPR,
> submit a request to run noa_wait; then readback the telltales from a
> final request.
>
> Now since the noa_wait is being run from the GGTT, the address space
> selector should be reading from the GGTT. So I am actually curious as to
> whether we have a bug or not.
> -Chris

I'm not super competent on the PPGTT setup, but I assumed this worked 
because we wrote into the ppgtt scratch page.

Potentially trashing an app VM space if it executes a reconfiguration.


-Lionel