[Intel-gfx] [PATCH 01/27] drm/i915/icl: Add initial Icelake definitions.

[Oscar Mateo]

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 at 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 at intel.com>
>>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at 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
>>>    };
>>>    
>>