Re: [PATCH 01/27] drm/i915/icl: Add initial Icelake definitions.

Paulo Zanoni
2018-01-10T17:57:44Z

Em Ter, 2018-01-09 às 15:59 -0800, Oscar Mateo escreveu:
>
> On 01/09/2018 03:23 PM, Paulo Zanoni wrote:
> > From: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >
> > Icelake is a Intel® Processor containing Intel® HD Graphics.
> >
> > This is just an initial Icelake definition. PCI IDs, Icelake
> > support
> > and new features coming in following patches.
> >
> > v2: Add .ddb_size and .has_guc (Michal Wajdeczko).
> > v3: Add the ICL_FEATURES macro (Kelvin Gardiner).
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 2 ++
> > drivers/gpu/drm/i915/i915_pci.c | 13 +++++++++++++
> > drivers/gpu/drm/i915/intel_device_info.c | 1 +
> > drivers/gpu/drm/i915/intel_device_info.h | 2 ++
> > 4 files changed, 18 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index a689396d0ff6..016920f58ae6 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2595,6 +2595,7 @@ intel_info(const struct drm_i915_private
> > *dev_priv)
> > #define IS_GEMINILAKE(dev_priv) IS_PLATFORM(dev_priv,
> > INTEL_GEMINILAKE)
> > #define IS_COFFEELAKE(dev_priv) IS_PLATFORM(dev_priv,
> > INTEL_COFFEELAKE)
> > #define IS_CANNONLAKE(dev_priv) IS_PLATFORM(dev_priv,
> > INTEL_CANNONLAKE)
> > +#define IS_ICELAKE(dev_priv) IS_PLATFORM(dev_priv,
> > INTEL_ICELAKE)
> > #define IS_MOBILE(dev_priv) ((dev_priv)->info.is_mobile)
> > #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \
> > (INTEL_DEVID(dev_priv) &
> > 0xFF00) == 0x0C00)
> > @@ -2706,6 +2707,7 @@ intel_info(const struct drm_i915_private
> > *dev_priv)
> > #define IS_GEN8(dev_priv) (!!((dev_priv)->info.gen_mask &
> > BIT(7)))
> > #define IS_GEN9(dev_priv) (!!((dev_priv)->info.gen_mask &
> > BIT(8)))
> > #define IS_GEN10(dev_priv) (!!((dev_priv)->info.gen_mask &
> > BIT(9)))
> > +#define IS_GEN11(dev_priv) (!!((dev_priv)->info.gen_mask &
> > BIT(10)))
> >
> > #define IS_LP(dev_priv) (INTEL_INFO(dev_priv)->is_lp)
> > #define IS_GEN9_LP(dev_priv) (IS_GEN9(dev_priv) &&
> > IS_LP(dev_priv))
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > b/drivers/gpu/drm/i915/i915_pci.c
> > index 36d48422b475..88cd4a3b12f5 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -579,6 +579,19 @@ static const struct intel_device_info
> > intel_cannonlake_gt2_info __initconst = {
> > .gt = 2,
> > };
> >
> > +#define GEN11_FEATURES \
> > + GEN10_FEATURES, \
> > + .gen = 11, \
> > + .ddb_size = 2048, \
> > + .has_csr = 0
> > +
> > +static const struct intel_device_info intel_icelake_11_info = {
> > + GEN11_FEATURES,
> > + .platform = INTEL_ICELAKE,
> > + .is_alpha_support = 1,
> > + .has_resource_streamer = 0,
> > +};
> > +
>
> This needs to change to fit the new GEN features inheritance
> organization recently introduced by Rodrigo (with a GEN11_FEATURES
> and I
> guess an ICL_11_PLATFORM). Should I go ahead?

My understanding is that this is already following the new inheritance
model. There's already a GEN11_FEATURES and it doesn't include an
ICL_11_PLATFORM because it doesn't need to (and CNL also doesn't have a
CNL_PLATFORM). What do you think needs to be changed?

The one thing I do see we lack is the __initconst keyword.

>
> > /*
> > * Make sure any device matches here are from most specific to
> > most
> > * general. For example, since the Quanta match is based on the
> > subsystem
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> > b/drivers/gpu/drm/i915/intel_device_info.c
> > index d28592e43512..a2c16140169f 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -56,6 +56,7 @@ static const char * const platform_names[] = {
> > PLATFORM_NAME(GEMINILAKE),
> > PLATFORM_NAME(COFFEELAKE),
> > PLATFORM_NAME(CANNONLAKE),
> > + PLATFORM_NAME(ICELAKE),
> > };
> > #undef PLATFORM_NAME
> >
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> > b/drivers/gpu/drm/i915/intel_device_info.h
> > index 49cb27bd04c1..9542018d11d0 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > @@ -69,6 +69,8 @@ enum intel_platform {
> > INTEL_COFFEELAKE,
> > /* gen10 */
> > INTEL_CANNONLAKE,
> > + /* gen11 */
> > + INTEL_ICELAKE,
> > INTEL_MAX_PLATFORMS
> > };
> >