[Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915

Teres Alexis, Alan Previn alan.previn.teres.alexis at intel.com
Thu Dec 1 19:06:29 UTC 2022



> > > 
> > > Please let's avoid the ** here and everywhere.
> > > 
> > Alan: In order to to avoid causing the entire driver into a rebuild because of any change in the intel_pxp structure,
> > the only way to accomplish that is to use a ptr in i915. But using a ptr means we allocate the memory at init time and
> > free it at fini time and those 2 cases would require the ptr-to-ptr to ensure we get the correct store. The only way i
> > can avoid the ** is be passing i915 as the param and then populating the ptr via i915->pxp. Would this work?
> 
> In general, one of two approaches should be used:
> 
> 1) The caller takes care of storing/clearing the pointer:
> 
> struct intel_pxp *intel_pxp_init(void);
> void intel_pxp_fini(struct intel_pxp *pxp);
> 
> 2) The callee takes care of storing/clearing the pointer:
> 
> int intel_pxp_init(struct drm_i915_private *i915);
> void intel_pxp_fini(struct drm_i915_private *i915);
> 
> Passing pointers to pointers is not as clean.
> 
> In this case, I'd choose 2) just because it's being called at the high
> level, and it's pretty much in line with everything else.
> 
> 
> 
> BR,
> Jani.

I like option 2. Also, I just remembered I used a similiar approach for guc-error-capture  (where intel_guc_capture_init
received a ptr to 'struct intel_guc' which owns a 'struct intel_guc_state_capture *capture;' member and did the
allocation). I did that back then exactly to address Jani's concerns because i was tired of waiting on build times when
developing offline :D

Will respin accordingly.

....alan



More information about the Intel-gfx mailing list