Re: [Intel-gfx] [PATCH 11/37] drm/i915/dg1: add initial DG-1 definitions

From: "Souza, Jose" <jose.souza@intel.com>
To: "De Marchi, Lucas" <lucas.demarchi@intel.com>
Cc: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Auld, Matthew" <matthew.auld@intel.com>
Subject: Re: [Intel-gfx] [PATCH 11/37] drm/i915/dg1: add initial DG-1 definitions
Date: Tue, 26 May 2020 18:02:58 +0000	[thread overview]
Message-ID: <523954f1cd291b4b1ccb7fdaac43b1578ef10843.camel@intel.com> (raw)
In-Reply-To: <20200526175107.lonqbwk35dmijaqo@ldmartin-desk1>

On Tue, 2020-05-26 at 10:51 -0700, Lucas De Marchi wrote:
> On Tue, May 26, 2020 at 10:34:47AM -0700, Jose Souza wrote:
> > On Wed, 2020-05-20 at 17:37 -0700, Lucas De Marchi wrote:
> > > From: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > > 
> > > Bspec: 33617, 33617
> > > 
> > > Cc: José Roberto de Souza <jose.souza@intel.com>
> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > Cc: Stuart Summers <stuart.summers@intel.com>
> > > Cc: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
> > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h          |  7 +++++++
> > >  drivers/gpu/drm/i915/i915_pci.c          | 12 ++++++++++++
> > >  drivers/gpu/drm/i915/intel_device_info.c |  1 +
> > >  drivers/gpu/drm/i915/intel_device_info.h |  1 +
> > >  4 files changed, 21 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index a9846205a5e2..382703a6c17d 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1421,6 +1421,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> > >  #define IS_ELKHARTLAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE)
> > >  #define IS_TIGERLAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_TIGERLAKE)
> > >  #define IS_ROCKETLAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_ROCKETLAKE)
> > > +#define IS_DG1(dev_priv)        IS_PLATFORM(dev_priv, INTEL_DG1)
> > >  #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \
> > >  				    (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00)
> > >  #define IS_BDW_ULT(dev_priv) \
> > > @@ -1541,6 +1542,12 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> > >  #define IS_RKL_REVID(p, since, until) \
> > >  	(IS_ROCKETLAKE(p) && IS_REVID(p, since, until))
> > > 
> > > +#define DG1_REVID_A0		0x0
> > > +#define DG1_REVID_B0		0x1
> > > +
> > > +#define IS_DG1_REVID(p, since, until) \
> > > +	(IS_DG1(p) && IS_REVID(p, since, until))
> > > +
> > >  #define IS_LP(dev_priv)	(INTEL_INFO(dev_priv)->is_lp)
> > >  #define IS_GEN9_LP(dev_priv)	(IS_GEN(dev_priv, 9) && IS_LP(dev_priv))
> > >  #define IS_GEN9_BC(dev_priv)	(IS_GEN(dev_priv, 9) && !IS_LP(dev_priv))
> > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> > > index be52d1b76b2e..e5a851a2dfe7 100644
> > > --- a/drivers/gpu/drm/i915/i915_pci.c
> > > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > > @@ -874,9 +874,21 @@ static const struct intel_device_info rkl_info = {
> > > 
> > >  #define GEN12_DGFX_FEATURES \
> > >  	GEN12_FEATURES, \
> > > +	.memory_regions = REGION_SMEM | REGION_LMEM, \
> > 
> > REGION_LMEM should be left here for all GEN12 platforms?
> 
> no, this define is GEN12_DGFX_FEATURES. For dgfx, yes... we should leave
> lmem region there.

Ops.
Also ignore my has_master_unit_irq comment.
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Lucas De Marchi
> 
> > I guess it will be detected in runtime that TGL and RKL don't have it but would be better to not have it in all GEN12 but will let memory experts to
> > shine in, other than LGTM.
> > 
> > 
> > >  	.is_dgfx = 1, \
> > > +	.has_master_unit_irq = 1, \
> > >  	.has_snoop_pcie = 1
> > > 
> > > +static const struct intel_device_info intel_dg1_info = {
> > > +	GEN12_DGFX_FEATURES,
> > > +	PLATFORM(INTEL_DG1),
> > > +	.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C) | BIT(PIPE_D),
> > > +	.require_force_probe = 1,
> > > +	.engine_mask =
> > > +		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) |
> > > +		BIT(VCS0) | BIT(VCS2),
> > > +};
> > > +
> > >  #undef GEN
> > >  #undef PLATFORM
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> > > index c245c10c9bee..207244b9a852 100644
> > > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > > @@ -62,6 +62,7 @@ static const char * const platform_names[] = {
> > >  	PLATFORM_NAME(ELKHARTLAKE),
> > >  	PLATFORM_NAME(TIGERLAKE),
> > >  	PLATFORM_NAME(ROCKETLAKE),
> > > +	PLATFORM_NAME(DG1),
> > >  };
> > >  #undef PLATFORM_NAME
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> > > index 4bcaa0d6a9e6..64260faac006 100644
> > > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > > @@ -81,6 +81,7 @@ enum intel_platform {
> > >  	/* gen12 */
> > >  	INTEL_TIGERLAKE,
> > >  	INTEL_ROCKETLAKE,
> > > +	INTEL_DG1,
> > >  	INTEL_MAX_PLATFORMS
> > >  };
> > > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx