On Tue, May 11, 2021 at 05:26:34PM +0200, Daniel Vetter wrote:
On Thu, May 06, 2021 at 12:13:57PM -0700, 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.
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 d84f37afb9d8..2eb6c497e43c 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" @@ -47,6 +49,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;
The current code sets a disastrous example, but for stuff like this it's always good to explain the locking, and who's holding references and how you're handling cycles. Since I guess the intel_context also holds the guc_id alive somehow.
I think (?) I know what you mean by this comment. How about adding:
'If an entry in the the context_lookup is present, that means a context associated with the guc_id is registered with the GuC. We use this xarray as a lookup mechanism when the GuC communicate with the i915 about the context.'
Again holds for the entire series, where it makes sense (as in we don't expect to rewrite the entire code anyway).
Slightly out of order but one of the last patches in the series, 'Update GuC documentation' adds a big section of comments that attempts to clarify how all of this code works. I likely should add a section explaining the data structures as well.
Matt
-Daniel
- /* 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 6acc1ef34f92..c2b6d27404b7 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 */ @@ -404,6 +430,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;
}
-- 2.28.0
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch