Re: [Intel-gfx] [PATCH v4 1/6] drm/i915/pxp: Make gt and pxp init/fini aware of PXP-owning-GT

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

After a more comprehensive offline discussion with Daniele and Rodrigo, design direction was made to go with Option2
where we elevate pxp to a global subsystem and within it it establish a pointer to the correct gt for pxp-controls. This
also reflects the current HW architectures where the PXP feature (when viewed as a service for contexts and buffers) is
global to all subsystems including any workload on any tile, despite its single control-knobs being only in the media
tile (because PXP controls needs to be global to the GPU to avoid any races).

This would mean we move 'struct intel_pxp' to be under i915 so that all subsystems that need to call into PXP would now
pass in i915 as its parameter. PXP internally would have a pointer to the correct GT (media-tile for MTL and gt0 for
prior).

It would also mean that certain code will still look a little kludgy or needs attention:
 - power-related operations like init/fini and suspend/resume would now need to called either before or after all-gt
equivalents to ensure proper flow.
 - KCR IRQ will although being a gt level IRQ will now require passing i915 into the pxp subsystem.

*NOTE: above list, even if i missed any, would still be nowhere near the amount of other external facing interfaces that
would be called by global subsystems that would now look clean with i915->pxp as its param.

...alan


On Tue, 2022-11-22 at 10:52 -0800, Alan Previn Teres Alexis wrote:
> 
> On Tue, 2022-11-22 at 12:57 -0500, Vivi, Rodrigo wrote:
> > 
> > 
> [Alan:snip]
> 
> > As I had told I don't have a strong preference, as long as it keep clean
> > and without these many helpers of something "on_gt"...
> > 
> > If this stays inside the gt, just make sure that you call for all the gts,
> > but then you inside the functions you can skip if pxp not enabled... without
> > asking if enabled on_gt... 
> > 
> I think we have here conflicting requests. The "consumers" of pxp feature are gem-execbuf, gem-context, gem-create (and
> even display, for checking). Are we okay with making these callers be aware of "if mtl, ensure i call intel_pxp_foo with
> the media-tile's pxp, agnostic to the request, context or buffer i am dealing with now". If you are okay with this, then
> we can make this stay inside gt without "enabled on_gt" functions. But if dont want to polute such low level backend
> awareness into those upper layers, we need them to call something more global like "intel_gpu_has_pxp_enabled" or
> "intel_gpu_has_pxp_started" at the least with an i915 param. So that these callers dont need to worry about it. Or
> intel_pxp_enabled has to internally check with gt we are being passed with and verify we are on the correct gt to - but
> you said you dont want to have an "enabled on_gt" inside the pxp folder since pxp is a subset of gt. The only thing i
> can think of is just a rename  - i.e. instead of "intel_pxp_enabled_on_gt", have a "intel_gpu_has_pxp_enabled(i915)" -
> but it would reside in the pxp folder. Would this work?
> 
> > > 
> > > ...alan
>