Re: [PATCH 9/9] drm/xe/ptl: Add PTL platform definition

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

On Fri, Sep 06, 2024 at 04:56:40PM -0700, Matt Roper wrote:
> On Fri, Sep 06, 2024 at 02:51:53PM -0700, Matt Atwood wrote:
> > From: Haridhar Kalvala <haridhar.kalvala@intel.com>
> > 
> > PTL is an integrated GPU based on the Xe3 architecture.
> > 
> > Bspec: 72574
> > 
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Haridhar Kalvala <haridhar.kalvala@intel.com>
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_pci.c            |  7 +++++++
> >  drivers/gpu/drm/xe/xe_platform_types.h |  1 +
> >  include/drm/intel/xe_pciids.h          | 11 +++++++++++
> >  3 files changed, 19 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> > index b30d09508a38..e471832a7a7f 100644
> > --- a/drivers/gpu/drm/xe/xe_pci.c
> > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > @@ -370,6 +370,12 @@ static const struct xe_device_desc bmg_desc = {
> >  	.has_heci_cscfi = 1,
> >  };
> >  
> > +static const struct xe_device_desc ptl_desc = {
> > +	PLATFORM(PANTHERLAKE),
> > +	.has_display = true,
> 
> You haven't sent the display patches yet, so this shouldn't be true
> (yet).  We usually leave this off until we start enabling the display
> and then include it in that patch series.
> 
> 
> Matt
> 
> > +	.require_force_probe = true,
> > +};
> > +
> >  #undef PLATFORM
> >  __diag_pop();
> >  
> > @@ -418,6 +424,7 @@ static const struct pci_device_id pciidlist[] = {
> >  	XE_MTL_IDS(INTEL_VGA_DEVICE, &mtl_desc),
> >  	XE_LNL_IDS(INTEL_VGA_DEVICE, &lnl_desc),
> >  	XE_BMG_IDS(INTEL_VGA_DEVICE, &bmg_desc),
> > +	XE_PTL_IDS(INTEL_VGA_DEVICE, &ptl_desc),
> >  	{ }
> >  };
> >  MODULE_DEVICE_TABLE(pci, pciidlist);
> > diff --git a/drivers/gpu/drm/xe/xe_platform_types.h b/drivers/gpu/drm/xe/xe_platform_types.h
> > index 79b7042c4534..d08574c4cdb8 100644
> > --- a/drivers/gpu/drm/xe/xe_platform_types.h
> > +++ b/drivers/gpu/drm/xe/xe_platform_types.h
> > @@ -23,6 +23,7 @@ enum xe_platform {
> >  	XE_METEORLAKE,
> >  	XE_LUNARLAKE,
> >  	XE_BATTLEMAGE,
> > +	XE_PANTHERLAKE,
> >  };
> >  
> >  enum xe_subplatform {
> > diff --git a/include/drm/intel/xe_pciids.h b/include/drm/intel/xe_pciids.h
> > index 73d972a8aca1..0e0256072acd 100644
> > --- a/include/drm/intel/xe_pciids.h
> > +++ b/include/drm/intel/xe_pciids.h
> > @@ -215,4 +215,15 @@
> >  	MACRO__(0xE20D, ## __VA_ARGS__), \
> >  	MACRO__(0xE212, ## __VA_ARGS__)
> >  
> > +#define XE_PTL_P_IDS(MACRO__, ...) \
> > +	MACRO__(0xB080, ## __VA_ARGS__)
> > +
> > +#define XE_PTL_U_IDS(MACRO__, ...) \
> > +	MACRO__(0xB090, ## __VA_ARGS__), \
> > +	MACRO__(0xB0A0, ## __VA_ARGS__)
> > +
> > +#define XE_PTL_IDS(MACRO__, ...) \
> > +	XE_PTL_P_IDS(MACRO__, ## __VA_ARGS__), \
> > +	XE_PTL_U_IDS(MACRO__, ## __VA_ARGS__)

It looks like we're missing a number of PCI IDs that have recently been
added to the bspec.

If nothing in the driver is making decisions based on the sub-type of
PTL here, we shouldn't even bother breaking these out like this.  The
letters here don't even appear to be accurate (similar to the mistake we
made with MTL a while back) so all we're doing is confusing people.


Matt

> > +
> >  #endif
> > -- 
> > 2.44.0
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation

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