Re: [Intel-xe] [PATCH 2/3] drm/xe: Fix platform order

[Matt Roper <matthew.d.roper@intel.com>]

On Fri, Mar 31, 2023 at 07:22:06AM -0600, Lucas De Marchi wrote:
> On Mon, Mar 27, 2023 at 10:02:38AM -0700, Matt Roper wrote:
> > On Thu, Mar 23, 2023 at 10:17:53PM -0700, Lucas De Marchi wrote:
> > > Platform order is important when looping through the list of guc
> > > firmware blobs since we use it to prevent loading a blob for a newer
> > > platform onto an older one. Move PVC after ADL.
> > 
> > Shouldn't we be moving the ADL platforms (graphics versions 12.0) higher
> > than DG1 (12.10) and DG2 (12.50) too?
> 
> question then would be:  would we be ordering them by gt
> version?  Or by when they were introduced?

Since all of the platforms here have the GuC inside the
graphics IP[*], then the graphics IP version seems natural to me.

"When they were introduced" would be identical for all of these
platforms for the Xe driver (since we just dumped a big megapatch that
contained all of these platforms at once).  But if you want to match
when they were introduced *in i915* that would be reasonable too,
although the ADLs would still need to come before DG2 in that case.


Matt

[*] MTL has a GuC in both the graphics IP and the media IP.  One of our
questions early on was whether the GuC IP itself would differ between
the two GTs (requiring different firmwares for each).  The response that
came back from the hardware team was that that's technically possible
with standalone media, but at least for MTL they'd keep them identical.
So for now, just basing 100% on the graphics IP version seems fine.  In
the future we may need to stop tying GuC to platform at all and instead
match on the appropriate IP version for whichever GT we're loading on.
But that's a problem for the future...


> 
> I think it makes more sense to be by when they were introduced as a
> platform in the driver.
> 
> 	1) what about media/display?
> 	2) allow us to always be appending in the enum and elsewhere in
> 	the driver.
> 
> Lucas De Marchi
> 
> > 
> > 
> > Matt
> > 
> > > 
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_platform_types.h | 3 +--
> > >  drivers/gpu/drm/xe/xe_uc_fw.c          | 2 +-
> > >  2 files changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_platform_types.h b/drivers/gpu/drm/xe/xe_platform_types.h
> > > index 72612c832e88..10367f6cc75a 100644
> > > --- a/drivers/gpu/drm/xe/xe_platform_types.h
> > > +++ b/drivers/gpu/drm/xe/xe_platform_types.h
> > > @@ -9,14 +9,13 @@
> > >  /* Keep in gen based order, and chronological order within a gen */
> > >  enum xe_platform {
> > >  	XE_PLATFORM_UNINITIALIZED = 0,
> > > -	/* gen12 */
> > >  	XE_TIGERLAKE,
> > >  	XE_ROCKETLAKE,
> > >  	XE_DG1,
> > >  	XE_DG2,
> > > -	XE_PVC,
> > >  	XE_ALDERLAKE_S,
> > >  	XE_ALDERLAKE_P,
> > > +	XE_PVC,
> > >  	XE_METEORLAKE,
> > >  };
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
> > > index e2c982b37e87..174c42873ebb 100644
> > > --- a/drivers/gpu/drm/xe/xe_uc_fw.c
> > > +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
> > > @@ -43,9 +43,9 @@ static struct xe_device *uc_fw_to_xe(struct xe_uc_fw *uc_fw)
> > >   */
> > >  #define XE_GUC_FIRMWARE_DEFS(fw_def, guc_def) \
> > >  	fw_def(METEORLAKE,   guc_def(mtl,  70, 5, 2)) \
> > > +	fw_def(PVC,          guc_def(pvc,  70, 5, 2)) \
> > >  	fw_def(ALDERLAKE_P,  guc_def(adlp,  70, 5, 2)) \
> > >  	fw_def(ALDERLAKE_S,  guc_def(tgl,  70, 5, 2)) \
> > > -	fw_def(PVC,          guc_def(pvc,  70, 5, 2)) \
> > >  	fw_def(DG2,          guc_def(dg2,  70, 5, 2)) \
> > >  	fw_def(DG1,          guc_def(dg1,  70, 5, 2)) \
> > >  	fw_def(TIGERLAKE,    guc_def(tgl,  70, 5, 2))
> > > --
> > > 2.39.0
> > > 
> > 
> > -- 
> > Matt Roper
> > Graphics Software Engineer
> > Linux GPU Platform Enablement
> > Intel Corporation

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