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

[Lucas De Marchi]

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 ?

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