diff mbox

[2/3] drm/i915: Promote IS_BROADWELL to a simple macro

Message ID 1462542231-7321-2-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tvrtko Ursulin May 6, 2016, 1:43 p.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

If we allow it a dedicated flag in dev_priv we enable the
compiler to nicely optimize conditions like IS_HASSWELL ||
IS_BROADWELL.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 4 ++++
 drivers/gpu/drm/i915/i915_drv.h | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Ville Syrjälä May 6, 2016, 2 p.m. UTC | #1
On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> If we allow it a dedicated flag in dev_priv we enable the
> compiler to nicely optimize conditions like IS_HASSWELL ||
> IS_BROADWELL.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 4 ++++
>  drivers/gpu/drm/i915/i915_drv.h | 3 ++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9fd221c97275..da6532da44e3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
>  static const struct intel_device_info intel_broadwell_d_info = {
>  	BDW_FEATURES,
>  	.gen = 8,
> +	.is_broadwell = 1,
>  };
>  
>  static const struct intel_device_info intel_broadwell_m_info = {
>  	BDW_FEATURES,
>  	.gen = 8, .is_mobile = 1,
> +	.is_broadwell = 1,
>  };
>  
>  static const struct intel_device_info intel_broadwell_gt3d_info = {
>  	BDW_FEATURES,
>  	.gen = 8,
> +	.is_broadwell = 1,
>  	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>  };
>  
>  static const struct intel_device_info intel_broadwell_gt3m_info = {
>  	BDW_FEATURES,
>  	.gen = 8, .is_mobile = 1,
> +	.is_broadwell = 1,
>  	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>  };
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c6351016eaf0..459561991081 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -735,6 +735,7 @@ struct intel_csr {
>  	func(is_valleyview) sep \
>  	func(is_cherryview) sep \
>  	func(is_haswell) sep \
> +	func(is_broadwell) sep \
>  	func(is_skylake) sep \
>  	func(is_broxton) sep \
>  	func(is_kabylake) sep \

We could just replace all of these with a single enum and save a bunch
of space.

> @@ -2553,7 +2554,7 @@ struct drm_i915_cmd_table {
>  #define IS_VALLEYVIEW(dev)	(INTEL_INFO(dev)->is_valleyview)
>  #define IS_CHERRYVIEW(dev)	(INTEL_INFO(dev)->is_cherryview)
>  #define IS_HASWELL(dev)	(INTEL_INFO(dev)->is_haswell)
> -#define IS_BROADWELL(dev)	(!INTEL_INFO(dev)->is_cherryview && IS_GEN8(dev))
> +#define IS_BROADWELL(dev)	(INTEL_INFO(dev)->is_broadwell)
>  #define IS_SKYLAKE(dev)	(INTEL_INFO(dev)->is_skylake)
>  #define IS_BROXTON(dev)		(INTEL_INFO(dev)->is_broxton)
>  #define IS_KABYLAKE(dev)	(INTEL_INFO(dev)->is_kabylake)
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Tvrtko Ursulin May 6, 2016, 2:18 p.m. UTC | #2
On 06/05/16 15:00, Ville Syrjälä wrote:
> On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> If we allow it a dedicated flag in dev_priv we enable the
>> compiler to nicely optimize conditions like IS_HASSWELL ||
>> IS_BROADWELL.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.c | 4 ++++
>>   drivers/gpu/drm/i915/i915_drv.h | 3 ++-
>>   2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 9fd221c97275..da6532da44e3 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
>>   static const struct intel_device_info intel_broadwell_d_info = {
>>   	BDW_FEATURES,
>>   	.gen = 8,
>> +	.is_broadwell = 1,
>>   };
>>
>>   static const struct intel_device_info intel_broadwell_m_info = {
>>   	BDW_FEATURES,
>>   	.gen = 8, .is_mobile = 1,
>> +	.is_broadwell = 1,
>>   };
>>
>>   static const struct intel_device_info intel_broadwell_gt3d_info = {
>>   	BDW_FEATURES,
>>   	.gen = 8,
>> +	.is_broadwell = 1,
>>   	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>>   };
>>
>>   static const struct intel_device_info intel_broadwell_gt3m_info = {
>>   	BDW_FEATURES,
>>   	.gen = 8, .is_mobile = 1,
>> +	.is_broadwell = 1,
>>   	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>>   };
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index c6351016eaf0..459561991081 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -735,6 +735,7 @@ struct intel_csr {
>>   	func(is_valleyview) sep \
>>   	func(is_cherryview) sep \
>>   	func(is_haswell) sep \
>> +	func(is_broadwell) sep \
>>   	func(is_skylake) sep \
>>   	func(is_broxton) sep \
>>   	func(is_kabylake) sep \
>
> We could just replace all of these with a single enum and save a bunch
> of space.

Bitfield is really nice here because compiler can then merge tests like 
IS_HASWELL || IS_BROADWELL into a single test instruction.

Regards,

Tvrtko

>
>> @@ -2553,7 +2554,7 @@ struct drm_i915_cmd_table {
>>   #define IS_VALLEYVIEW(dev)	(INTEL_INFO(dev)->is_valleyview)
>>   #define IS_CHERRYVIEW(dev)	(INTEL_INFO(dev)->is_cherryview)
>>   #define IS_HASWELL(dev)	(INTEL_INFO(dev)->is_haswell)
>> -#define IS_BROADWELL(dev)	(!INTEL_INFO(dev)->is_cherryview && IS_GEN8(dev))
>> +#define IS_BROADWELL(dev)	(INTEL_INFO(dev)->is_broadwell)
>>   #define IS_SKYLAKE(dev)	(INTEL_INFO(dev)->is_skylake)
>>   #define IS_BROXTON(dev)		(INTEL_INFO(dev)->is_broxton)
>>   #define IS_KABYLAKE(dev)	(INTEL_INFO(dev)->is_kabylake)
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Ville Syrjälä May 6, 2016, 2:29 p.m. UTC | #3
On Fri, May 06, 2016 at 03:18:16PM +0100, Tvrtko Ursulin wrote:
> 
> On 06/05/16 15:00, Ville Syrjälä wrote:
> > On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> If we allow it a dedicated flag in dev_priv we enable the
> >> compiler to nicely optimize conditions like IS_HASSWELL ||
> >> IS_BROADWELL.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/i915_drv.c | 4 ++++
> >>   drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> >>   2 files changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> index 9fd221c97275..da6532da44e3 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
> >>   static const struct intel_device_info intel_broadwell_d_info = {
> >>   	BDW_FEATURES,
> >>   	.gen = 8,
> >> +	.is_broadwell = 1,
> >>   };
> >>
> >>   static const struct intel_device_info intel_broadwell_m_info = {
> >>   	BDW_FEATURES,
> >>   	.gen = 8, .is_mobile = 1,
> >> +	.is_broadwell = 1,
> >>   };
> >>
> >>   static const struct intel_device_info intel_broadwell_gt3d_info = {
> >>   	BDW_FEATURES,
> >>   	.gen = 8,
> >> +	.is_broadwell = 1,
> >>   	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >>   };
> >>
> >>   static const struct intel_device_info intel_broadwell_gt3m_info = {
> >>   	BDW_FEATURES,
> >>   	.gen = 8, .is_mobile = 1,
> >> +	.is_broadwell = 1,
> >>   	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >>   };
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index c6351016eaf0..459561991081 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -735,6 +735,7 @@ struct intel_csr {
> >>   	func(is_valleyview) sep \
> >>   	func(is_cherryview) sep \
> >>   	func(is_haswell) sep \
> >> +	func(is_broadwell) sep \
> >>   	func(is_skylake) sep \
> >>   	func(is_broxton) sep \
> >>   	func(is_kabylake) sep \
> >
> > We could just replace all of these with a single enum and save a bunch
> > of space.
> 
> Bitfield is really nice here because compiler can then merge tests like 
> IS_HASWELL || IS_BROADWELL into a single test instruction.

Most if not all of those are in modeset code so I'm not entirely
convinced micro-optimizing that is all that useful.

Bunch of times I've just wished I could write 'whatever >= G4X' instead
of 'IS_G4X || gen >= 5' simply for readability.

> 
> Regards,
> 
> Tvrtko
> 
> >
> >> @@ -2553,7 +2554,7 @@ struct drm_i915_cmd_table {
> >>   #define IS_VALLEYVIEW(dev)	(INTEL_INFO(dev)->is_valleyview)
> >>   #define IS_CHERRYVIEW(dev)	(INTEL_INFO(dev)->is_cherryview)
> >>   #define IS_HASWELL(dev)	(INTEL_INFO(dev)->is_haswell)
> >> -#define IS_BROADWELL(dev)	(!INTEL_INFO(dev)->is_cherryview && IS_GEN8(dev))
> >> +#define IS_BROADWELL(dev)	(INTEL_INFO(dev)->is_broadwell)
> >>   #define IS_SKYLAKE(dev)	(INTEL_INFO(dev)->is_skylake)
> >>   #define IS_BROXTON(dev)		(INTEL_INFO(dev)->is_broxton)
> >>   #define IS_KABYLAKE(dev)	(INTEL_INFO(dev)->is_kabylake)
> >> --
> >> 1.9.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
Tvrtko Ursulin May 6, 2016, 2:44 p.m. UTC | #4
On 06/05/16 15:29, Ville Syrjälä wrote:
> On Fri, May 06, 2016 at 03:18:16PM +0100, Tvrtko Ursulin wrote:
>>
>> On 06/05/16 15:00, Ville Syrjälä wrote:
>>> On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> If we allow it a dedicated flag in dev_priv we enable the
>>>> compiler to nicely optimize conditions like IS_HASSWELL ||
>>>> IS_BROADWELL.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/i915_drv.c | 4 ++++
>>>>    drivers/gpu/drm/i915/i915_drv.h | 3 ++-
>>>>    2 files changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>>>> index 9fd221c97275..da6532da44e3 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.c
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>>>> @@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
>>>>    static const struct intel_device_info intel_broadwell_d_info = {
>>>>    	BDW_FEATURES,
>>>>    	.gen = 8,
>>>> +	.is_broadwell = 1,
>>>>    };
>>>>
>>>>    static const struct intel_device_info intel_broadwell_m_info = {
>>>>    	BDW_FEATURES,
>>>>    	.gen = 8, .is_mobile = 1,
>>>> +	.is_broadwell = 1,
>>>>    };
>>>>
>>>>    static const struct intel_device_info intel_broadwell_gt3d_info = {
>>>>    	BDW_FEATURES,
>>>>    	.gen = 8,
>>>> +	.is_broadwell = 1,
>>>>    	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>>>>    };
>>>>
>>>>    static const struct intel_device_info intel_broadwell_gt3m_info = {
>>>>    	BDW_FEATURES,
>>>>    	.gen = 8, .is_mobile = 1,
>>>> +	.is_broadwell = 1,
>>>>    	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>>>>    };
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>> index c6351016eaf0..459561991081 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>> @@ -735,6 +735,7 @@ struct intel_csr {
>>>>    	func(is_valleyview) sep \
>>>>    	func(is_cherryview) sep \
>>>>    	func(is_haswell) sep \
>>>> +	func(is_broadwell) sep \
>>>>    	func(is_skylake) sep \
>>>>    	func(is_broxton) sep \
>>>>    	func(is_kabylake) sep \
>>>
>>> We could just replace all of these with a single enum and save a bunch
>>> of space.
>>
>> Bitfield is really nice here because compiler can then merge tests like
>> IS_HASWELL || IS_BROADWELL into a single test instruction.
>
> Most if not all of those are in modeset code so I'm not entirely
> convinced micro-optimizing that is all that useful.

Maybe not, but IS_BROADWELL was just tasteless and it is better to save 
instructions across everything than a bit in dev_priv.

> Bunch of times I've just wished I could write 'whatever >= G4X' instead
> of 'IS_G4X || gen >= 5' simply for readability.

I don't even know what G4X is so can't really follow what you mean. Is 
it like Gen4.5, or Gen5-- ? So I can't figure out what kind of scheme 
you propose.

Regards,

Tvrtko
Chris Wilson May 6, 2016, 2:51 p.m. UTC | #5
On Fri, May 06, 2016 at 03:44:05PM +0100, Tvrtko Ursulin wrote:
> 
> On 06/05/16 15:29, Ville Syrjälä wrote:
> >On Fri, May 06, 2016 at 03:18:16PM +0100, Tvrtko Ursulin wrote:
> >>
> >>On 06/05/16 15:00, Ville Syrjälä wrote:
> >>>On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
> >>>>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>>If we allow it a dedicated flag in dev_priv we enable the
> >>>>compiler to nicely optimize conditions like IS_HASSWELL ||
> >>>>IS_BROADWELL.
> >>>>
> >>>>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>---
> >>>>   drivers/gpu/drm/i915/i915_drv.c | 4 ++++
> >>>>   drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> >>>>   2 files changed, 6 insertions(+), 1 deletion(-)
> >>>>
> >>>>diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >>>>index 9fd221c97275..da6532da44e3 100644
> >>>>--- a/drivers/gpu/drm/i915/i915_drv.c
> >>>>+++ b/drivers/gpu/drm/i915/i915_drv.c
> >>>>@@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
> >>>>   static const struct intel_device_info intel_broadwell_d_info = {
> >>>>   	BDW_FEATURES,
> >>>>   	.gen = 8,
> >>>>+	.is_broadwell = 1,
> >>>>   };
> >>>>
> >>>>   static const struct intel_device_info intel_broadwell_m_info = {
> >>>>   	BDW_FEATURES,
> >>>>   	.gen = 8, .is_mobile = 1,
> >>>>+	.is_broadwell = 1,
> >>>>   };
> >>>>
> >>>>   static const struct intel_device_info intel_broadwell_gt3d_info = {
> >>>>   	BDW_FEATURES,
> >>>>   	.gen = 8,
> >>>>+	.is_broadwell = 1,
> >>>>   	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >>>>   };
> >>>>
> >>>>   static const struct intel_device_info intel_broadwell_gt3m_info = {
> >>>>   	BDW_FEATURES,
> >>>>   	.gen = 8, .is_mobile = 1,
> >>>>+	.is_broadwell = 1,
> >>>>   	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >>>>   };
> >>>>
> >>>>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>>>index c6351016eaf0..459561991081 100644
> >>>>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>>>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>>>@@ -735,6 +735,7 @@ struct intel_csr {
> >>>>   	func(is_valleyview) sep \
> >>>>   	func(is_cherryview) sep \
> >>>>   	func(is_haswell) sep \
> >>>>+	func(is_broadwell) sep \
> >>>>   	func(is_skylake) sep \
> >>>>   	func(is_broxton) sep \
> >>>>   	func(is_kabylake) sep \
> >>>
> >>>We could just replace all of these with a single enum and save a bunch
> >>>of space.
> >>
> >>Bitfield is really nice here because compiler can then merge tests like
> >>IS_HASWELL || IS_BROADWELL into a single test instruction.
> >
> >Most if not all of those are in modeset code so I'm not entirely
> >convinced micro-optimizing that is all that useful.
> 
> Maybe not, but IS_BROADWELL was just tasteless and it is better to
> save instructions across everything than a bit in dev_priv.
> 
> >Bunch of times I've just wished I could write 'whatever >= G4X' instead
> >of 'IS_G4X || gen >= 5' simply for readability.
> 
> I don't even know what G4X is so can't really follow what you mean.
> Is it like Gen4.5, or Gen5-- ? So I can't figure out what kind of
> scheme you propose.

It's gen4.5. A quick refresh of the "eccentric" gen4.

The problem we always run into with a single gen field is that the GPU,
display engine and various bits and pieces have had a divergenet
evolution, e.g. Valleyview has a gen7 gpu with a gen4 display block.

We keep planning to separate the two, and that is mostly done today
through the distinction of IS_GENx() and IS_BROADWELL() etc, and we are
often better served by adding a specific feature flag rather than depend
upon a version field.
-Chris
Ville Syrjälä May 6, 2016, 2:58 p.m. UTC | #6
On Fri, May 06, 2016 at 03:44:05PM +0100, Tvrtko Ursulin wrote:
> 
> On 06/05/16 15:29, Ville Syrjälä wrote:
> > On Fri, May 06, 2016 at 03:18:16PM +0100, Tvrtko Ursulin wrote:
> >>
> >> On 06/05/16 15:00, Ville Syrjälä wrote:
> >>> On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> If we allow it a dedicated flag in dev_priv we enable the
> >>>> compiler to nicely optimize conditions like IS_HASSWELL ||
> >>>> IS_BROADWELL.
> >>>>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>> ---
> >>>>    drivers/gpu/drm/i915/i915_drv.c | 4 ++++
> >>>>    drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> >>>>    2 files changed, 6 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >>>> index 9fd221c97275..da6532da44e3 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_drv.c
> >>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >>>> @@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
> >>>>    static const struct intel_device_info intel_broadwell_d_info = {
> >>>>    	BDW_FEATURES,
> >>>>    	.gen = 8,
> >>>> +	.is_broadwell = 1,
> >>>>    };
> >>>>
> >>>>    static const struct intel_device_info intel_broadwell_m_info = {
> >>>>    	BDW_FEATURES,
> >>>>    	.gen = 8, .is_mobile = 1,
> >>>> +	.is_broadwell = 1,
> >>>>    };
> >>>>
> >>>>    static const struct intel_device_info intel_broadwell_gt3d_info = {
> >>>>    	BDW_FEATURES,
> >>>>    	.gen = 8,
> >>>> +	.is_broadwell = 1,
> >>>>    	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >>>>    };
> >>>>
> >>>>    static const struct intel_device_info intel_broadwell_gt3m_info = {
> >>>>    	BDW_FEATURES,
> >>>>    	.gen = 8, .is_mobile = 1,
> >>>> +	.is_broadwell = 1,
> >>>>    	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >>>>    };
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>>> index c6351016eaf0..459561991081 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_drv.h
> >>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >>>> @@ -735,6 +735,7 @@ struct intel_csr {
> >>>>    	func(is_valleyview) sep \
> >>>>    	func(is_cherryview) sep \
> >>>>    	func(is_haswell) sep \
> >>>> +	func(is_broadwell) sep \
> >>>>    	func(is_skylake) sep \
> >>>>    	func(is_broxton) sep \
> >>>>    	func(is_kabylake) sep \
> >>>
> >>> We could just replace all of these with a single enum and save a bunch
> >>> of space.
> >>
> >> Bitfield is really nice here because compiler can then merge tests like
> >> IS_HASWELL || IS_BROADWELL into a single test instruction.
> >
> > Most if not all of those are in modeset code so I'm not entirely
> > convinced micro-optimizing that is all that useful.
> 
> Maybe not, but IS_BROADWELL was just tasteless and it is better to save 
> instructions across everything than a bit in dev_priv.
> 
> > Bunch of times I've just wished I could write 'whatever >= G4X' instead
> > of 'IS_G4X || gen >= 5' simply for readability.
> 
> I don't even know what G4X is so can't really follow what you mean. Is 
> it like Gen4.5, or Gen5-- ? So I can't figure out what kind of scheme 
> you propose.

It's still a GMCH platform, but the display engine north side bits are
very close to ILK already. Oh and I suppose a bunch of the south side
bits are also pretty close to ILK as well since G4X has native HDMI
and DP ports already. So the display engine in many ways is closer to
ILK than to gen4. The one major difference to ILK is the lack of FDI.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9fd221c97275..da6532da44e3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -300,22 +300,26 @@  static const struct intel_device_info intel_haswell_m_info = {
 static const struct intel_device_info intel_broadwell_d_info = {
 	BDW_FEATURES,
 	.gen = 8,
+	.is_broadwell = 1,
 };
 
 static const struct intel_device_info intel_broadwell_m_info = {
 	BDW_FEATURES,
 	.gen = 8, .is_mobile = 1,
+	.is_broadwell = 1,
 };
 
 static const struct intel_device_info intel_broadwell_gt3d_info = {
 	BDW_FEATURES,
 	.gen = 8,
+	.is_broadwell = 1,
 	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
 };
 
 static const struct intel_device_info intel_broadwell_gt3m_info = {
 	BDW_FEATURES,
 	.gen = 8, .is_mobile = 1,
+	.is_broadwell = 1,
 	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
 };
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c6351016eaf0..459561991081 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -735,6 +735,7 @@  struct intel_csr {
 	func(is_valleyview) sep \
 	func(is_cherryview) sep \
 	func(is_haswell) sep \
+	func(is_broadwell) sep \
 	func(is_skylake) sep \
 	func(is_broxton) sep \
 	func(is_kabylake) sep \
@@ -2553,7 +2554,7 @@  struct drm_i915_cmd_table {
 #define IS_VALLEYVIEW(dev)	(INTEL_INFO(dev)->is_valleyview)
 #define IS_CHERRYVIEW(dev)	(INTEL_INFO(dev)->is_cherryview)
 #define IS_HASWELL(dev)	(INTEL_INFO(dev)->is_haswell)
-#define IS_BROADWELL(dev)	(!INTEL_INFO(dev)->is_cherryview && IS_GEN8(dev))
+#define IS_BROADWELL(dev)	(INTEL_INFO(dev)->is_broadwell)
 #define IS_SKYLAKE(dev)	(INTEL_INFO(dev)->is_skylake)
 #define IS_BROXTON(dev)		(INTEL_INFO(dev)->is_broxton)
 #define IS_KABYLAKE(dev)	(INTEL_INFO(dev)->is_kabylake)