Re: [Intel-gfx] [PATCH 02/27] drm/i915/guc: Allow flexible number of context ids

John Harrison
2021-09-09T22:13:38Z

On 8/20/2021 15:44, Matthew Brost wrote:
> Number of available GuC contexts ids might be limited.
> Stop referring in code to macro and use variable instead.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc.h | 4 ++++
> drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 +++++++++------
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index 112dd29a63fe..6fd2719d1b75 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -60,6 +60,10 @@ struct intel_guc {
> spinlock_t contexts_lock;
> /** @guc_ids: used to allocate new guc_ids */
> struct ida guc_ids;
> + /** @num_guc_ids: number of guc_ids that can be used */
> + u32 num_guc_ids;
> + /** @max_guc_ids: max number of guc_ids that can be used */
> + u32 max_guc_ids;
How do these differ? The description needs to say how or why 'num' might
be less than 'max'. And surely 'max' is not the count 'that can be
used'? Num is how many can be used, but max is how many are physically
possible or something?

John.

> /**
> * @guc_id_list: list of intel_context with valid guc_ids but no refs
> */
> 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 46158d996bf6..8235e49bb347 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -344,7 +344,7 @@ static struct guc_lrc_desc *__get_lrc_desc(struct intel_guc *guc, u32 index)
> {
> struct guc_lrc_desc *base = guc->lrc_desc_pool_vaddr;
>
> - GEM_BUG_ON(index >= GUC_MAX_LRC_DESCRIPTORS);
> + GEM_BUG_ON(index >= guc->max_guc_ids);
>
> return &base[index];
> }
> @@ -353,7 +353,7 @@ static 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);
> + GEM_BUG_ON(id >= guc->max_guc_ids);
>
> return ce;
> }
> @@ -363,8 +363,7 @@ static int guc_lrc_desc_pool_create(struct intel_guc *guc)
> u32 size;
> int ret;
>
> - size = PAGE_ALIGN(sizeof(struct guc_lrc_desc) *
> - GUC_MAX_LRC_DESCRIPTORS);
> + size = PAGE_ALIGN(sizeof(struct guc_lrc_desc) * guc->max_guc_ids);
> ret = intel_guc_allocate_and_map_vma(guc, size, &guc->lrc_desc_pool,
> (void **)&guc->lrc_desc_pool_vaddr);
> if (ret)
> @@ -1193,7 +1192,7 @@ static void guc_submit_request(struct i915_request *rq)
> static int new_guc_id(struct intel_guc *guc)
> {
> return ida_simple_get(&guc->guc_ids, 0,
> - GUC_MAX_LRC_DESCRIPTORS, GFP_KERNEL |
> + guc->num_guc_ids, GFP_KERNEL |
> __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> }
>
> @@ -2704,6 +2703,8 @@ static bool __guc_submission_selected(struct intel_guc *guc)
>
> void intel_guc_submission_init_early(struct intel_guc *guc)
> {
> + guc->max_guc_ids = GUC_MAX_LRC_DESCRIPTORS;
> + guc->num_guc_ids = GUC_MAX_LRC_DESCRIPTORS;
> guc->submission_supported = __guc_submission_supported(guc);
> guc->submission_selected = __guc_submission_selected(guc);
> }
> @@ -2713,7 +2714,7 @@ g2h_context_lookup(struct intel_guc *guc, u32 desc_idx)
> {
> struct intel_context *ce;
>
> - if (unlikely(desc_idx >= GUC_MAX_LRC_DESCRIPTORS)) {
> + if (unlikely(desc_idx >= guc->max_guc_ids)) {
> drm_err(&guc_to_gt(guc)->i915->drm,
> "Invalid desc_idx %u", desc_idx);
> return NULL;
> @@ -3063,6 +3064,8 @@ void intel_guc_submission_print_info(struct intel_guc *guc,
>
> drm_printf(p, "GuC Number Outstanding Submission G2H: %u\n",
> atomic_read(&guc->outstanding_submission_g2h));
> + drm_printf(p, "GuC Number GuC IDs: %u\n", guc->num_guc_ids);
> + drm_printf(p, "GuC Max GuC IDs: %u\n", guc->max_guc_ids);
> drm_printf(p, "GuC tasklet count: %u\n\n",
> atomic_read(&sched_engine->tasklet.count));
>