| Message ID | 20210624070516.21893-11-matthew.brost@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | GuC submission support | expand |
On 24.06.2021 09:04, Matthew Brost wrote: > Add lrc descriptor context lookup array which can resolve the > intel_context from the lrc descriptor index. In addition to lookup, it > can determine in the lrc descriptor context is currently registered with > the GuC by checking if an entry for a descriptor index is present. > Future patches in the series will make use of this array. s/lrc/LRC > > Cc: John Harrison <john.c.harrison@intel.com> > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > --- > drivers/gpu/drm/i915/gt/uc/intel_guc.h | 5 +++ > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 32 +++++++++++++++++-- > 2 files changed, 35 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > index b28fa54214f2..2313d9fc087b 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > @@ -6,6 +6,8 @@ > #ifndef _INTEL_GUC_H_ > #define _INTEL_GUC_H_ > > +#include "linux/xarray.h" #include <linux/xarray.h> > + > #include "intel_uncore.h" > #include "intel_guc_fw.h" > #include "intel_guc_fwif.h" > @@ -46,6 +48,9 @@ struct intel_guc { > struct i915_vma *lrc_desc_pool; > void *lrc_desc_pool_vaddr; > > + /* guc_id to intel_context lookup */ > + struct xarray context_lookup; > + > /* Control params for fw initialization */ > u32 params[GUC_CTL_MAX_DWORDS]; btw, IIRC there was idea to move most struct definitions to intel_guc_types.h, is this still a plan ? > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > index a366890fb840..23a94a896a0b 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -65,8 +65,6 @@ static inline struct i915_priolist *to_priolist(struct rb_node *rb) > return rb_entry(rb, struct i915_priolist, node); > } > > -/* Future patches will use this function */ > -__attribute__ ((unused)) > static struct guc_lrc_desc *__get_lrc_desc(struct intel_guc *guc, u32 index) > { > struct guc_lrc_desc *base = guc->lrc_desc_pool_vaddr; > @@ -76,6 +74,15 @@ static struct guc_lrc_desc *__get_lrc_desc(struct intel_guc *guc, u32 index) > return &base[index]; > } > > +static inline struct intel_context *__get_context(struct intel_guc *guc, u32 id) > +{ > + struct intel_context *ce = xa_load(&guc->context_lookup, id); > + > + GEM_BUG_ON(id >= GUC_MAX_LRC_DESCRIPTORS); > + > + return ce; > +} > + > static int guc_lrc_desc_pool_create(struct intel_guc *guc) > { > u32 size; > @@ -96,6 +103,25 @@ static void guc_lrc_desc_pool_destroy(struct intel_guc *guc) > i915_vma_unpin_and_release(&guc->lrc_desc_pool, I915_VMA_RELEASE_MAP); > } > > +static inline void reset_lrc_desc(struct intel_guc *guc, u32 id) > +{ > + struct guc_lrc_desc *desc = __get_lrc_desc(guc, id); > + > + memset(desc, 0, sizeof(*desc)); > + xa_erase_irq(&guc->context_lookup, id); > +} > + > +static inline bool lrc_desc_registered(struct intel_guc *guc, u32 id) > +{ > + return __get_context(guc, id); > +} > + > +static inline void set_lrc_desc_registered(struct intel_guc *guc, u32 id, > + struct intel_context *ce) > +{ > + xa_store_irq(&guc->context_lookup, id, ce, GFP_ATOMIC); > +} > + > static void guc_add_request(struct intel_guc *guc, struct i915_request *rq) > { > /* Leaving stub as this function will be used in future patches */ > @@ -400,6 +426,8 @@ int intel_guc_submission_init(struct intel_guc *guc) > */ > GEM_BUG_ON(!guc->lrc_desc_pool); > > + xa_init_flags(&guc->context_lookup, XA_FLAGS_LOCK_IRQ); > + > return 0; > } > >
On Fri, Jun 25, 2021 at 03:17:51PM +0200, Michal Wajdeczko wrote: > > > On 24.06.2021 09:04, Matthew Brost wrote: > > Add lrc descriptor context lookup array which can resolve the > > intel_context from the lrc descriptor index. In addition to lookup, it > > can determine in the lrc descriptor context is currently registered with > > the GuC by checking if an entry for a descriptor index is present. > > Future patches in the series will make use of this array. > > s/lrc/LRC > I guess? lrc and LRC are used interchangeably throughout the current code base. > > > > Cc: John Harrison <john.c.harrison@intel.com> > > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > > --- > > drivers/gpu/drm/i915/gt/uc/intel_guc.h | 5 +++ > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 32 +++++++++++++++++-- > > 2 files changed, 35 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > index b28fa54214f2..2313d9fc087b 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > @@ -6,6 +6,8 @@ > > #ifndef _INTEL_GUC_H_ > > #define _INTEL_GUC_H_ > > > > +#include "linux/xarray.h" > > #include <linux/xarray.h> > Yep. > > + > > #include "intel_uncore.h" > > #include "intel_guc_fw.h" > > #include "intel_guc_fwif.h" > > @@ -46,6 +48,9 @@ struct intel_guc { > > struct i915_vma *lrc_desc_pool; > > void *lrc_desc_pool_vaddr; > > > > + /* guc_id to intel_context lookup */ > > + struct xarray context_lookup; > > + > > /* Control params for fw initialization */ > > u32 params[GUC_CTL_MAX_DWORDS]; > > btw, IIRC there was idea to move most struct definitions to > intel_guc_types.h, is this still a plan ? > I don't ever recall discussing this but we can certainly do this. For what it is worth we do introduce intel_guc_submission_types.h a bit later. I'll make a note about intel_guc_types.h though. Matt > > > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > index a366890fb840..23a94a896a0b 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > @@ -65,8 +65,6 @@ static inline struct i915_priolist *to_priolist(struct rb_node *rb) > > return rb_entry(rb, struct i915_priolist, node); > > } > > > > -/* Future patches will use this function */ > > -__attribute__ ((unused)) > > static struct guc_lrc_desc *__get_lrc_desc(struct intel_guc *guc, u32 index) > > { > > struct guc_lrc_desc *base = guc->lrc_desc_pool_vaddr; > > @@ -76,6 +74,15 @@ static struct guc_lrc_desc *__get_lrc_desc(struct intel_guc *guc, u32 index) > > return &base[index]; > > } > > > > +static inline struct intel_context *__get_context(struct intel_guc *guc, u32 id) > > +{ > > + struct intel_context *ce = xa_load(&guc->context_lookup, id); > > + > > + GEM_BUG_ON(id >= GUC_MAX_LRC_DESCRIPTORS); > > + > > + return ce; > > +} > > + > > static int guc_lrc_desc_pool_create(struct intel_guc *guc) > > { > > u32 size; > > @@ -96,6 +103,25 @@ static void guc_lrc_desc_pool_destroy(struct intel_guc *guc) > > i915_vma_unpin_and_release(&guc->lrc_desc_pool, I915_VMA_RELEASE_MAP); > > } > > > > +static inline void reset_lrc_desc(struct intel_guc *guc, u32 id) > > +{ > > + struct guc_lrc_desc *desc = __get_lrc_desc(guc, id); > > + > > + memset(desc, 0, sizeof(*desc)); > > + xa_erase_irq(&guc->context_lookup, id); > > +} > > + > > +static inline bool lrc_desc_registered(struct intel_guc *guc, u32 id) > > +{ > > + return __get_context(guc, id); > > +} > > + > > +static inline void set_lrc_desc_registered(struct intel_guc *guc, u32 id, > > + struct intel_context *ce) > > +{ > > + xa_store_irq(&guc->context_lookup, id, ce, GFP_ATOMIC); > > +} > > + > > static void guc_add_request(struct intel_guc *guc, struct i915_request *rq) > > { > > /* Leaving stub as this function will be used in future patches */ > > @@ -400,6 +426,8 @@ int intel_guc_submission_init(struct intel_guc *guc) > > */ > > GEM_BUG_ON(!guc->lrc_desc_pool); > > > > + xa_init_flags(&guc->context_lookup, XA_FLAGS_LOCK_IRQ); > > + > > return 0; > > } > > > >
On 6/25/2021 10:26, Matthew Brost wrote: > On Fri, Jun 25, 2021 at 03:17:51PM +0200, Michal Wajdeczko wrote: >> On 24.06.2021 09:04, Matthew Brost wrote: >>> Add lrc descriptor context lookup array which can resolve the >>> intel_context from the lrc descriptor index. In addition to lookup, it >>> can determine in the lrc descriptor context is currently registered with >>> the GuC by checking if an entry for a descriptor index is present. >>> Future patches in the series will make use of this array. >> s/lrc/LRC >> > I guess? lrc and LRC are used interchangeably throughout the current > code base. It is an abbreviation so LRC is technically the correct version for a comment. The fact that other existing comments are incorrect is not a valid reason to perpetuate a mistake :). Might as well fix it if you are going to repost the patch anyway for any other reason, but I would not call it a blocking issue. Also, 'can determine in the' should be 'can determine if the'. Again, not exactly a blocking issue but should be fixed. >>> Cc: John Harrison <john.c.harrison@intel.com> >>> Signed-off-by: Matthew Brost <matthew.brost@intel.com> >>> --- >>> drivers/gpu/drm/i915/gt/uc/intel_guc.h | 5 +++ >>> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 32 +++++++++++++++++-- >>> 2 files changed, 35 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h >>> index b28fa54214f2..2313d9fc087b 100644 >>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h >>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h >>> @@ -6,6 +6,8 @@ >>> #ifndef _INTEL_GUC_H_ >>> #define _INTEL_GUC_H_ >>> >>> +#include "linux/xarray.h" >> #include <linux/xarray.h> >> > Yep. > >>> + >>> #include "intel_uncore.h" >>> #include "intel_guc_fw.h" >>> #include "intel_guc_fwif.h" >>> @@ -46,6 +48,9 @@ struct intel_guc { >>> struct i915_vma *lrc_desc_pool; >>> void *lrc_desc_pool_vaddr; >>> >>> + /* guc_id to intel_context lookup */ >>> + struct xarray context_lookup; >>> + >>> /* Control params for fw initialization */ >>> u32 params[GUC_CTL_MAX_DWORDS]; >> btw, IIRC there was idea to move most struct definitions to >> intel_guc_types.h, is this still a plan ? >> > I don't ever recall discussing this but we can certainly do this. For > what it is worth we do introduce intel_guc_submission_types.h a bit > later. I'll make a note about intel_guc_types.h though. > > Matt Yeah, my only recollection was about the submission types header. Are there sufficient non-submission fields in the GuC structure to warrant a general GuC types header? With the commit message tweaks and #include fix mentioned above, it looks good to me. Reviewed-by: John Harrison <John.C.Harrison@Intel.com> >>> >>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >>> index a366890fb840..23a94a896a0b 100644 >>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >>> @@ -65,8 +65,6 @@ static inline struct i915_priolist *to_priolist(struct rb_node *rb) >>> return rb_entry(rb, struct i915_priolist, node); >>> } >>> >>> -/* Future patches will use this function */ >>> -__attribute__ ((unused)) >>> static struct guc_lrc_desc *__get_lrc_desc(struct intel_guc *guc, u32 index) >>> { >>> struct guc_lrc_desc *base = guc->lrc_desc_pool_vaddr; >>> @@ -76,6 +74,15 @@ static struct guc_lrc_desc *__get_lrc_desc(struct intel_guc *guc, u32 index) >>> return &base[index]; >>> } >>> >>> +static inline struct intel_context *__get_context(struct intel_guc *guc, u32 id) >>> +{ >>> + struct intel_context *ce = xa_load(&guc->context_lookup, id); >>> + >>> + GEM_BUG_ON(id >= GUC_MAX_LRC_DESCRIPTORS); >>> + >>> + return ce; >>> +} >>> + >>> static int guc_lrc_desc_pool_create(struct intel_guc *guc) >>> { >>> u32 size; >>> @@ -96,6 +103,25 @@ static void guc_lrc_desc_pool_destroy(struct intel_guc *guc) >>> i915_vma_unpin_and_release(&guc->lrc_desc_pool, I915_VMA_RELEASE_MAP); >>> } >>> >>> +static inline void reset_lrc_desc(struct intel_guc *guc, u32 id) >>> +{ >>> + struct guc_lrc_desc *desc = __get_lrc_desc(guc, id); >>> + >>> + memset(desc, 0, sizeof(*desc)); >>> + xa_erase_irq(&guc->context_lookup, id); >>> +} >>> + >>> +static inline bool lrc_desc_registered(struct intel_guc *guc, u32 id) >>> +{ >>> + return __get_context(guc, id); >>> +} >>> + >>> +static inline void set_lrc_desc_registered(struct intel_guc *guc, u32 id, >>> + struct intel_context *ce) >>> +{ >>> + xa_store_irq(&guc->context_lookup, id, ce, GFP_ATOMIC); >>> +} >>> + >>> static void guc_add_request(struct intel_guc *guc, struct i915_request *rq) >>> { >>> /* Leaving stub as this function will be used in future patches */ >>> @@ -400,6 +426,8 @@ int intel_guc_submission_init(struct intel_guc *guc) >>> */ >>> GEM_BUG_ON(!guc->lrc_desc_pool); >>> >>> + xa_init_flags(&guc->context_lookup, XA_FLAGS_LOCK_IRQ); >>> + >>> return 0; >>> } >>> >>>
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index b28fa54214f2..2313d9fc087b 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -6,6 +6,8 @@ #ifndef _INTEL_GUC_H_ #define _INTEL_GUC_H_ +#include "linux/xarray.h" + #include "intel_uncore.h" #include "intel_guc_fw.h" #include "intel_guc_fwif.h" @@ -46,6 +48,9 @@ struct intel_guc { struct i915_vma *lrc_desc_pool; void *lrc_desc_pool_vaddr; + /* guc_id to intel_context lookup */ + struct xarray context_lookup; + /* Control params for fw initialization */ u32 params[GUC_CTL_MAX_DWORDS]; diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index a366890fb840..23a94a896a0b 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -65,8 +65,6 @@ static inline struct i915_priolist *to_priolist(struct rb_node *rb) return rb_entry(rb, struct i915_priolist, node); } -/* Future patches will use this function */ -__attribute__ ((unused)) static struct guc_lrc_desc *__get_lrc_desc(struct intel_guc *guc, u32 index) { struct guc_lrc_desc *base = guc->lrc_desc_pool_vaddr; @@ -76,6 +74,15 @@ static struct guc_lrc_desc *__get_lrc_desc(struct intel_guc *guc, u32 index) return &base[index]; } +static inline struct intel_context *__get_context(struct intel_guc *guc, u32 id) +{ + struct intel_context *ce = xa_load(&guc->context_lookup, id); + + GEM_BUG_ON(id >= GUC_MAX_LRC_DESCRIPTORS); + + return ce; +} + static int guc_lrc_desc_pool_create(struct intel_guc *guc) { u32 size; @@ -96,6 +103,25 @@ static void guc_lrc_desc_pool_destroy(struct intel_guc *guc) i915_vma_unpin_and_release(&guc->lrc_desc_pool, I915_VMA_RELEASE_MAP); } +static inline void reset_lrc_desc(struct intel_guc *guc, u32 id) +{ + struct guc_lrc_desc *desc = __get_lrc_desc(guc, id); + + memset(desc, 0, sizeof(*desc)); + xa_erase_irq(&guc->context_lookup, id); +} + +static inline bool lrc_desc_registered(struct intel_guc *guc, u32 id) +{ + return __get_context(guc, id); +} + +static inline void set_lrc_desc_registered(struct intel_guc *guc, u32 id, + struct intel_context *ce) +{ + xa_store_irq(&guc->context_lookup, id, ce, GFP_ATOMIC); +} + static void guc_add_request(struct intel_guc *guc, struct i915_request *rq) { /* Leaving stub as this function will be used in future patches */ @@ -400,6 +426,8 @@ int intel_guc_submission_init(struct intel_guc *guc) */ GEM_BUG_ON(!guc->lrc_desc_pool); + xa_init_flags(&guc->context_lookup, XA_FLAGS_LOCK_IRQ); + return 0; }
Add lrc descriptor context lookup array which can resolve the intel_context from the lrc descriptor index. In addition to lookup, it can determine in the lrc descriptor context is currently registered with the GuC by checking if an entry for a descriptor index is present. Future patches in the series will make use of this array. Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by: Matthew Brost <matthew.brost@intel.com> --- drivers/gpu/drm/i915/gt/uc/intel_guc.h | 5 +++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 32 +++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-)