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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Nov 22, 2022 at 06:50:14PM +0000, Teres Alexis, Alan Previn 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?

In the end I believe that the pxp needs to be the one with knowledge of the
serving_gt. Either gt0 on TGL or media_gt on MTL.

So, if we keep the intel_pxp inside the intel_gt to make the initialization,
irq, and pm flows cleaner, we need probably need to save the
struct intel_gt *gt_serving_pxp;
inside all the intel_pxp, or in drm_i915_private, and then
have a helper that returns the ...gt_serving_pxp(...)
but then skipping the init/fini functions if gt parent != gt_serving_pxp.

> 
> > > 
> > > ...alan
>