| Message ID | 20180109232336.11029-2-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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? > /* > * 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 > }; >
Quoting Paulo Zanoni (2018-01-09 23:23:10) > From: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Icelake is a Intel® Processor containing Intel® HD Graphics. One thing to check, is the marketing term now UHD Graphics? -Chris
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 > > }; > > > >
On 01/10/2018 09:57 AM, Paulo Zanoni wrote: > 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. As a minimum there is the __initconst keyword and the fact that the .gen value is not assigned inside the GENX_FEATURES for any other platform. As for the PLATFORM, we can do that now or wait until we need it, I guess it doesn't matter. > >>> /* >>> * 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 >>> }; >>> >>
Em Qua, 2018-01-10 às 10:15 +0000, Chris Wilson escreveu: > Quoting Paulo Zanoni (2018-01-09 23:23:10) > > From: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > Icelake is a Intel® Processor containing Intel® HD Graphics. > > One thing to check, is the marketing term now UHD Graphics? Marketing names are not available yet for ICL, and Kernel commit messages should definitely not be considered reliable for that matter. From what I can see on ark.intel.com, in the 8th generation everything is UHD. But, for example, the Iris Plus products on the 7th generation don't even have "HD" in their names, so I guess we've been misleading in the previous commit messages too. I'll change the wording so it doesn't look like we're announcing marketing names. (also, I'm not a native English speaker, but shouldn't we "s/ a / an /" there too?) > -Chris
On Wed, Jan 10, 2018 at 06:08:21PM +0000, Oscar Mateo wrote: > > > On 01/10/2018 09:57 AM, Paulo Zanoni wrote: > > 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. > > As a minimum there is the __initconst keyword and the fact that the .gen > value is not assigned inside the GENX_FEATURES for any other platform. As > for the PLATFORM, we can do that now or wait until we need it, I guess it > doesn't matter. On the current code only gen9_lp add the .gen inside the feature blocks, core platform adds on the platform definition. I can't remember if this was intentional to make inheritance clear on platforms that reuse display from one gen and core from another or it was just a mistake. Anyways, for this patch here with __initconst I believe it follows the current standard so: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > > > > /* > > > > * 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 > > > > }; > > > >
Em Qua, 2018-01-10 às 10:22 -0800, Rodrigo Vivi escreveu: > On Wed, Jan 10, 2018 at 06:08:21PM +0000, Oscar Mateo wrote: > > > > > > On 01/10/2018 09:57 AM, Paulo Zanoni wrote: > > > 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_pri > > > > > v, > > > > > INTEL_GEMINILAKE) > > > > > #define IS_COFFEELAKE(dev_priv) IS_PLATFORM(dev_pri > > > > > v, > > > > > INTEL_COFFEELAKE) > > > > > #define IS_CANNONLAKE(dev_priv) IS_PLATFORM(dev_pri > > > > > v, > > > > > 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. > > > > As a minimum there is the __initconst keyword and the fact that the > > .gen > > value is not assigned inside the GENX_FEATURES for any other > > platform. As > > for the PLATFORM, we can do that now or wait until we need it, I > > guess it > > doesn't matter. > > On the current code only gen9_lp add the .gen inside the feature > blocks, > core platform adds on the platform definition. > I can't remember if this was intentional to make inheritance clear on > platforms that reuse display from one gen and core from another or it > was > just a mistake. Yeah, there's some confusion on gen 8/9. > > Anyways, for this patch here with __initconst I believe it follows > the > current standard so: > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> So, since you're the maintainer and I assume you're leading by example, can I start giving R-B tags to my own patches too? :) Anyway, I also think the next version I'm going to submit will be fine, so I suppose a r-b from me would count. If anybody disagrees the current plan is mergeable please say so. > > > > > > > > > > > > > /* > > > > > * 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 > > > > > }; > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Jan 10, 2018 at 06:38:13PM +0000, Paulo Zanoni wrote: > Em Qua, 2018-01-10 às 10:22 -0800, Rodrigo Vivi escreveu: > > On Wed, Jan 10, 2018 at 06:08:21PM +0000, Oscar Mateo wrote: > > > > > > > > > On 01/10/2018 09:57 AM, Paulo Zanoni wrote: > > > > 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_pri > > > > > > v, > > > > > > INTEL_GEMINILAKE) > > > > > > #define IS_COFFEELAKE(dev_priv) IS_PLATFORM(dev_pri > > > > > > v, > > > > > > INTEL_COFFEELAKE) > > > > > > #define IS_CANNONLAKE(dev_priv) IS_PLATFORM(dev_pri > > > > > > v, > > > > > > 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. > > > > > > As a minimum there is the __initconst keyword and the fact that the > > > .gen > > > value is not assigned inside the GENX_FEATURES for any other > > > platform. As > > > for the PLATFORM, we can do that now or wait until we need it, I > > > guess it > > > doesn't matter. > > > > On the current code only gen9_lp add the .gen inside the feature > > blocks, > > core platform adds on the platform definition. > > I can't remember if this was intentional to make inheritance clear on > > platforms that reuse display from one gen and core from another or it > > was > > just a mistake. > > Yeah, there's some confusion on gen 8/9. > > > > > Anyways, for this patch here with __initconst I believe it follows > > the > > current standard so: > > > > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > So, since you're the maintainer and I assume you're leading by example, > can I start giving R-B tags to my own patches too? :) hahaha I didn't remember and I didn't noticed it! :) the important part is that we have now 2 people agreeing with this patch > > Anyway, I also think the next version I'm going to submit will be fine, > so I suppose a r-b from me would count. > > If anybody disagrees the current plan is mergeable please say so. > > > > > > > > > > > > > > > > > > > > /* > > > > > > * 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 > > > > > > }; > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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, +}; + /* * 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 };