[Intel-xe] [PATCH 03/26] drm/xe: Add backpointer from gt to tile
Matt Roper
matthew.d.roper at intel.com
Fri May 12 16:20:02 UTC 2023
On Thu, May 11, 2023 at 05:07:46PM -0700, Lucas De Marchi wrote:
> On Wed, May 10, 2023 at 08:46:59PM -0700, Matt Roper wrote:
> > Rather than a backpointer to the xe_device, a GT should have a
> > backpointer to its tile (which can then be used to lookup the device if
> > necessary).
> >
> > The gt_to_xe() helper macro (which moves from xe_gt.h to xe_gt_types.h)
> > can and should still be used to jump directly from an xe_gt to
> > xe_device.
> >
> > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_bb.c | 2 +-
> > drivers/gpu/drm/xe/xe_gt.h | 5 -----
> > drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c | 4 ++--
> > drivers/gpu/drm/xe/xe_gt_types.h | 14 ++++++++++++--
> > drivers/gpu/drm/xe/xe_mocs.c | 14 +++++++-------
> > drivers/gpu/drm/xe/xe_pci.c | 11 +++++++----
> > drivers/gpu/drm/xe/xe_pt.c | 2 +-
> > 7 files changed, 30 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_bb.c b/drivers/gpu/drm/xe/xe_bb.c
> > index 3deb2d55f421..bf7c94b769d7 100644
> > --- a/drivers/gpu/drm/xe/xe_bb.c
> > +++ b/drivers/gpu/drm/xe/xe_bb.c
> > @@ -16,7 +16,7 @@
> >
> > static int bb_prefetch(struct xe_gt *gt)
> > {
> > - struct xe_device *xe = gt->xe;
> > + struct xe_device *xe = gt_to_xe(gt);
> >
> > if (GRAPHICS_VERx100(xe) >= 1250 && !xe_gt_is_media_type(gt))
> > /*
> > diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
> > index 086369f7ee6d..f4e98f499b36 100644
> > --- a/drivers/gpu/drm/xe/xe_gt.h
> > +++ b/drivers/gpu/drm/xe/xe_gt.h
> > @@ -49,11 +49,6 @@ static inline bool xe_gt_is_media_type(struct xe_gt *gt)
> > return gt->info.type == XE_GT_TYPE_MEDIA;
> > }
> >
> > -#define gt_to_xe(gt__) \
> > - _Generic(gt__, \
> > - const struct xe_gt *: (const struct xe_device *)((gt__)->xe), \
> > - struct xe_gt *: (gt__)->xe)
> > -
> > static inline bool xe_gt_is_usm_hwe(struct xe_gt *gt, struct xe_hw_engine *hwe)
> > {
> > struct xe_device *xe = gt_to_xe(gt);
> > diff --git a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> > index c815a42e2cdb..c9e8825c02aa 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> > @@ -322,8 +322,8 @@ int xe_guc_tlb_invalidation_done_handler(struct xe_guc *guc, u32 *msg, u32 len)
> > TLB_INVALIDATION_SEQNO_MAX;
> > if (!expected_seqno)
> > expected_seqno = 1;
> > - if (drm_WARN_ON(>->xe->drm, expected_seqno != msg[0])) {
> > - drm_err(>->xe->drm, "TLB expected_seqno(%d) != msg(%u)\n",
> > + if (drm_WARN_ON(>_to_xe(gt)->drm, expected_seqno != msg[0])) {
> > + drm_err(>_to_xe(gt)->drm, "TLB expected_seqno(%d) != msg(%u)\n",
> > expected_seqno, msg[0]);
> > }
> >
> > diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
> > index e0ed4508269b..c4376d50786b 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_types.h
> > +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> > @@ -76,6 +76,16 @@ enum xe_steering_type {
> > NUM_STEERING_TYPES
> > };
> >
> > +#define gt_to_tile(gt__) \
> > + _Generic(gt__, \
> > + const struct xe_gt *: (const struct xe_tile *)((gt__)->tile), \
> > + struct xe_gt *: (gt__)->tile)
> > +
> > +#define gt_to_xe(gt__) \
> > + _Generic(gt__, \
> > + const struct xe_gt *: (const struct xe_device *)(gt_to_tile(gt__)->xe), \
> > + struct xe_gt *: gt_to_tile(gt__)->xe)
> > +
> > /**
> > * struct xe_gt - A "Graphics Technology" unit of the GPU
> > *
> > @@ -90,8 +100,8 @@ enum xe_steering_type {
> > * within a tile.
> > */
> > struct xe_gt {
> > - /** @xe: backpointer to XE device */
> > - struct xe_device *xe;
> > + /** @tile: Backpointer to GT's tile */
> > + struct xe_tile *tile;
> >
> > /** @info: GT info */
> > struct {
> > diff --git a/drivers/gpu/drm/xe/xe_mocs.c b/drivers/gpu/drm/xe/xe_mocs.c
> > index 817afd301d52..d57fbf16a3ef 100644
> > --- a/drivers/gpu/drm/xe/xe_mocs.c
> > +++ b/drivers/gpu/drm/xe/xe_mocs.c
> > @@ -471,7 +471,7 @@ static void __init_mocs_table(struct xe_gt *gt,
> > unsigned int i;
> > u32 mocs;
> >
> > - mocs_dbg(>->xe->drm, "entries:%d\n", info->n_entries);
> > + mocs_dbg(>_to_xe(gt)->drm, "entries:%d\n", info->n_entries);
> > drm_WARN_ONCE(&xe->drm, !info->unused_entries_index,
> > "Unused entries index should have been defined\n");
> > for (i = 0;
> > @@ -479,7 +479,7 @@ static void __init_mocs_table(struct xe_gt *gt,
> > i++) {
> > struct xe_reg reg = XE_REG(addr + i * 4);
> >
> > - mocs_dbg(>->xe->drm, "%d 0x%x 0x%x\n", i, reg.addr, mocs);
> > + mocs_dbg(>_to_xe(gt)->drm, "%d 0x%x 0x%x\n", i, reg.addr, mocs);
> > xe_mmio_write32(gt, reg, mocs);
> > }
> > }
> > @@ -508,13 +508,13 @@ static void init_l3cc_table(struct xe_gt *gt,
> > unsigned int i;
> > u32 l3cc;
> >
> > - mocs_dbg(>->xe->drm, "entries:%d\n", info->n_entries);
> > + mocs_dbg(>_to_xe(gt)->drm, "entries:%d\n", info->n_entries);
> > for (i = 0;
> > i < (info->n_entries + 1) / 2 ?
> > (l3cc = l3cc_combine(get_entry_l3cc(info, 2 * i),
> > get_entry_l3cc(info, 2 * i + 1))), 1 : 0;
> > i++) {
> > - mocs_dbg(>->xe->drm, "%d 0x%x 0x%x\n", i, LNCFCMOCS(i).addr,
> > + mocs_dbg(>_to_xe(gt)->drm, "%d 0x%x 0x%x\n", i, LNCFCMOCS(i).addr,
> > l3cc);
> > xe_mmio_write32(gt, LNCFCMOCS(i), l3cc);
> > }
> > @@ -524,7 +524,7 @@ void xe_mocs_init_early(struct xe_gt *gt)
> > {
> > struct xe_mocs_info table;
> >
> > - get_mocs_settings(gt->xe, &table);
> > + get_mocs_settings(gt_to_xe(gt), &table);
> > gt->mocs.uc_index = table.uc_index;
> > gt->mocs.wb_index = table.wb_index;
> > }
> > @@ -537,8 +537,8 @@ void xe_mocs_init(struct xe_gt *gt)
> > /*
> > * LLC and eDRAM control values are not applicable to dgfx
> > */
> > - flags = get_mocs_settings(gt->xe, &table);
> > - mocs_dbg(>->xe->drm, "flag:0x%x\n", flags);
> > + flags = get_mocs_settings(gt_to_xe(gt), &table);
> > + mocs_dbg(>_to_xe(gt)->drm, "flag:0x%x\n", flags);
> >
> > if (flags & HAS_GLOBAL_MOCS)
> > __init_mocs_table(gt, &table, GLOBAL_MOCS(0).addr);
> > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> > index e79b16d8bf7f..87c328106aca 100644
> > --- a/drivers/gpu/drm/xe/xe_pci.c
> > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > @@ -471,6 +471,7 @@ static int xe_info_init(struct xe_device *xe,
> > {
> > const struct xe_graphics_desc *graphics_desc = NULL;
> > const struct xe_media_desc *media_desc = NULL;
> > + struct xe_tile *tile;
> > struct xe_gt *gt;
> > u8 id;
> >
> > @@ -525,13 +526,15 @@ static int xe_info_init(struct xe_device *xe,
> > xe->info.step = xe_step_get(xe);
> >
> > for (id = 0; id < xe->info.tile_count; ++id) {
> > - xe->tiles[id].xe = xe;
> > - xe->tiles[id].id = id;
> > + tile = &xe->tiles[id];
> > + tile->xe = xe;
> > + tile->id = id;
> >
> > - gt = &xe->tiles[id].primary_gt;
> > + gt = &tile->primary_gt;
>
> these seem to have been squash in the wrong patch, changing the style
> from using xe->tiles[i].* to declaring a local var. Maybe squash it in
> the previous one to avoid the back and forth here?
>
> otherwise,
>
>
> Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>
>
> > gt->info.id = id;
> > - gt->xe = xe;
> > + gt->tile = tile;
> >
> > + gt->info.id = id;
>
> the gt than has the id of the tile? Because right now there can only be
> primary_gt? Looks odd, but it's a consequence of removing the only
> platform with 1 tile and multiple gts.
Yeah, deciding how we want to number GTs is a bit of an open question.
Right now at the end of this series, the numbering is
* PVC: 0 = root tile primary, 1 = remote tile primary
* MTL: 0 = root tile primary, 1 = root tile media
* everything else: only has GT0
Some day we may have a platform with multiple tiles and separate
graphics/media GTs within each tile. In that case would all graphics
GTs get even numbers and all media GTs get odd? Or would all primary
GTs get gt_id = tile_id and all media GTs get gt_id = tile_id +
tile_count?
The bigger open question is whether uapi should still expose GTs (like
it does today) or whether it should be converted to just expose tiles
and keep the graphics vs media separation an internal implementation
detail. If uapi changes to only expose tiles, then we probably don't
even need IDs for the GTs...
Matt
>
> Lucas De Marchi
>
> > if (id == 0) {
> > gt->info.type = XE_GT_TYPE_MAIN;
> > gt->info.vram_id = id;
> > diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> > index f15282996c3b..61126cefe0b5 100644
> > --- a/drivers/gpu/drm/xe/xe_pt.c
> > +++ b/drivers/gpu/drm/xe/xe_pt.c
> > @@ -695,7 +695,7 @@ xe_pt_stage_bind_entry(struct xe_ptw *parent, pgoff_t offset,
> > * TODO: Suballocate the pt bo to avoid wasting a lot of
> > * memory.
> > */
> > - if (GRAPHICS_VERx100(xe_walk->gt->xe) >= 1250 && level == 1 &&
> > + if (GRAPHICS_VERx100(gt_to_xe(xe_walk->gt)) >= 1250 && level == 1 &&
> > covers && xe_pt_scan_64K(addr, next, xe_walk)) {
> > walk->shifts = xe_compact_pt_shifts;
> > flags |= XE_PDE_64K;
> > --
> > 2.40.0
> >
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
More information about the Intel-xe
mailing list