[Intel-xe] [PATCH 1/2] drm/xe: Stash GMD_ID value in xe_gt

[Matt Roper]

On Wed, Dec 06, 2023 at 05:19:41PM -0600, Lucas De Marchi wrote:
> On Wed, Dec 06, 2023 at 03:09:53PM -0800, Matt Roper wrote:
> > On Wed, Dec 06, 2023 at 04:55:10PM -0600, Lucas De Marchi wrote:
> > > On Wed, Dec 06, 2023 at 12:50:36PM -0800, Matt Roper wrote:
> > > > Although we've stored the major and minor versions for graphics/media in
> > > > xe_device, it will be simpler to implement the uapi version query if we
> > > > also stash the raw register value in the GT itself.
> > > >
> > > > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> > > > ---
> > > > drivers/gpu/drm/xe/xe_gt.c       | 6 ++++++
> > > > drivers/gpu/drm/xe/xe_gt_types.h | 2 ++
> > > > 2 files changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > > > index 154d6c7072b9..c6f7885ffdc0 100644
> > > > --- a/drivers/gpu/drm/xe/xe_gt.c
> > > > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > > > @@ -381,6 +381,12 @@ static int gt_fw_domain_init(struct xe_gt *gt)
> > > > 	/* Initialize CCS mode sysfs after early initialization of HW engines */
> > > > 	xe_gt_ccs_mode_sysfs_init(gt);
> > > >
> > > > +	/*
> > > > +	 * Stash hardware-reported version.  Since this register does not exist
> > > > +	 * on pre-MTL platforms, reading it there will (correctly) return 0.
> > > > +	 */
> > > > +	gt->info.gmdid = xe_mmio_read32(gt, GMD_ID);
> > > 
> > > I was coincidentally reading the PICA_DEVICE_ID for a debug (that we
> > > currently don't read) and stumbled upon this.
> > > 
> > > I'm wondering what's the correct place to read and save the GMDIDs. We
> > > already read it on xe_pci.c:handle_gmdid(). Why don't we simply stash it
> > > there? With the current design, it doesn't seem we allow 2 GTs with
> > > different graphics / media version since it still on the xe side
> > > initialization.
> > > 
> > > So, why don't we just stash it on xe rather than GT?
> > 
> > I mainly did it this way because it makes the actual uapi change a bit
> > simpler to actually keep this in the GT itself.  Plus it's a bit more
> > future-proof in case we ever do start allowing multiple GTs of the same
> 
> I don't disagree with the uapi. We can keep the uapi gt-based.... just
> wondering if we need to keep the read on the gt initialization, because
> if we allow GTs of the same type with different versions, we'd already
> have quite a lot to change in xe_pci.c
> 
> > type with different versions, or if we ever add new types of GTs beyond
> > just primary/media.
> 
> on the other hand, we have 2 other GMDIDs that could be useful, but
> aren't tied to the GT:  Display and PICA. I'm mainly concerned about
> PICA for now since I'd like to print it for debug. Should I tie it to
> the device while graphics and media remain on the respective GT ?

I don't think PICA should be in Xe itself (and we should remove the
display GMD_ID as well).  Instead the display code in i915/ should
probably read and stash it in intel_display_runtime_info during
intel_display_device_probe() (assuming we're on a platform with display
version 14 or higher) and can print it in
intel_display_device_info_print().


Matt

> 
> Lucas De Marchi
> 
> > 
> > 
> > Matt
> > 
> > > 
> > > Lucas De Marchi
> > > 
> > > > +
> > > > 	err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> > > > 	XE_WARN_ON(err);
> > > > 	xe_device_mem_access_put(gt_to_xe(gt));
> > > > diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
> > > > index 4e48c4643163..4e7a4494a554 100644
> > > > --- a/drivers/gpu/drm/xe/xe_gt_types.h
> > > > +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> > > > @@ -117,6 +117,8 @@ struct xe_gt {
> > > > 		 * hwconfig blob.
> > > > 		 */
> > > > 		u64 __engine_mask;
> > > > +		/** @gmdid: raw GMD_ID value from hardware */
> > > > +		u32 gmdid;
> > > > 	} info;
> > > >
> > > > 	/**
> > > > --
> > > > 2.43.0
> > > >
> > 
> > -- 
> > Matt Roper
> > Graphics Software Engineer
> > Linux GPU Platform Enablement
> > Intel Corporation

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation