Re: [PATCH 02/12] drm/i915/guc: Support logical engine mapping table in ADS

[John Harrison <John.C.Harrison@xxxxxxxxx>]

On 9/16/2020 16:27, Daniele Ceraolo Spurio wrote:
> On 9/16/2020 10:16 AM, John.C.Harrison@xxxxxxxxx wrote:
> > From: Matthew Brost <matthew.brost@xxxxxxxxx>
> >
> > The new GuC FW introduces a physical to logical engine mapping table in
> > the GuC additional data structures which needs to be configured in order
> > for the firmware to load. This patch initializes the table with a 1 to 1
> > mapping.
> >
> > Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
> > CC: John Harrison <John.C.Harrison@xxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c  | 23 +++++++++++++++++++++
> >   drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h |  4 +++-
> >   2 files changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c 
> > b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> > index d44061033f23..57954c6360e0 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> > @@ -48,6 +48,27 @@ static void guc_ct_pool_entries_init(struct 
> > guc_ct_pool_entry *pool, u32 num)
> >       memset(pool, 0, num * sizeof(*pool));
> >   }
> >   +static void guc_mapping_table_init(struct intel_gt *gt,
> > +                   struct guc_gt_system_info *system_info)
> > +{
> > +    unsigned int i, j;
> > +    struct intel_engine_cs *engine;
> > +    enum intel_engine_id id;
> > +
> > +    /* Table must be set to invalid values for entries not used */
> > +    for (i = 0; i < GUC_MAX_ENGINE_CLASSES; ++i)
> > +        for (j = 0; j < GUC_MAX_INSTANCES_PER_CLASS; ++j)
> > +            system_info->mapping_table[i][j] =
> > +                GUC_MAX_INSTANCES_PER_CLASS;
> > +
> > +    for_each_engine(engine, gt, id) {
> > +        u8 guc_class = engine->class;
> > +
> > + system_info->mapping_table[guc_class][engine->instance] =
> > +            engine->instance;
> > +    }
> > +}
> > +
> >   /*
> >    * The first 80 dwords of the register state context, containing the
> >    * execlists and ppgtt registers.
> > @@ -107,6 +128,8 @@ static void __guc_ads_init(struct intel_guc *guc)
> >       blob->system_info.vebox_enable_mask = VEBOX_MASK(gt);
> >       blob->system_info.vdbox_sfc_support_mask = 
> > gt->info.vdbox_sfc_access;
> >   +    guc_mapping_table_init(guc_to_gt(guc), &blob->system_info);
> > +
> >       base = intel_guc_ggtt_offset(guc, guc->ads_vma);
> >         /* Clients info  */
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h 
> > b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> > index 8b3684c6a9a1..e283156624b5 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> > @@ -388,7 +388,9 @@ struct guc_gt_system_info {
> >       u32 vdbox_enable_mask;
> >       u32 vdbox_sfc_support_mask;
> >       u32 vebox_enable_mask;
> > -    u32 reserved[9];
> > +    u32 reserved1;
> > +    u8 
> > mapping_table[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
> > +    u32 reserved2[8];
>
> I think this patch should be squashed with patch 8 to move directly to 
> the v49 version of guc_gt_system_info.
> Apart from this the patch LGTM.
>
> Daniele
The whole set is going to be squashed into a single patch for merging. 
For review purposes, the discussion was that it makes sense to keep each 
as a separate functional change. This patch adds logical engine 
mappings. Patch #8 adds extra engine classes. These are different 
functional changes.

John.



> >   } __packed;
> >     /* Clients info */

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx