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

[Lucas De Marchi]

On Wed, Dec 06, 2023 at 04:37:36PM -0800, Matt Roper wrote:
>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().

I was thinking that PICA could be available even without display, but
it's not relevant for now. If it ever becomes, we can move it to xe.
Thanks.


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

Lucas De Marchi