Quoting Lionel Landwerlin (2020-07-09 21:50:30) > On 09/07/2020 23:46, Chris Wilson wrote: > > Quoting Lionel Landwerlin (2020-07-09 21:42:39) > >> 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@xxxxxxxxx> > >>>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx> > >>>> --- > >>>> 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. > > It's not a STORE_INDEX, it's writing to an absolute address based on the > > address space selector which is a combination of the batch address space > > and the command bits. Certainly the batch itself is read from the GGTT, > > but I can't off hand remember the rules for the various MI (whether they > > could even access ppGTT when launched from GGTT). > > -Chris > > My understanding was that from a GGTT batch you could read/write into PPGTT. > > But not the other way around (obvisously). > > I thought the kernel mapped a page throughout the entire PPGTT space to > prevent pagefaults. Is that not the case? Yes, a readonly page, where supported. Ah, now I understand you meant *that* scratch page as opposed to a page we allocated for the purpose of saving per-context state like the gt->scratch page. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx