[PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask

[PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

If instead of numerical comparison me make these test a
bitmask, we enable the compiler to optimize all instances
of IS_GENx || IS_GENy.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c |  3 +++
 drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++--------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index ad7abe517700..01163da51b1d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1071,6 +1071,9 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 	memcpy(device_info, info, sizeof(dev_priv->info));
 	device_info->device_id = dev->pdev->device;
 
+	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * 8);
+	device_info->gen_mask = 1 << device_info->gen;
+
 	spin_lock_init(&dev_priv->irq_lock);
 	spin_lock_init(&dev_priv->gpu_error.lock);
 	mutex_init(&dev_priv->backlight_lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d5496aba1cd5..c6351016eaf0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -760,6 +760,7 @@ struct intel_device_info {
 	u8 num_pipes:3;
 	u8 num_sprites[I915_MAX_PIPES];
 	u8 gen;
+	u16 gen_mask;
 	u8 ring_mask; /* Rings supported by the HW */
 	DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON);
 	/* Register offsets for the various display pipes and transcoders */
@@ -2620,14 +2621,14 @@ struct drm_i915_cmd_table {
  * have their own (e.g. HAS_PCH_SPLIT for ILK+ display, IS_foo for particular
  * chips, etc.).
  */
-#define IS_GEN2(dev)	(INTEL_INFO(dev)->gen == 2)
-#define IS_GEN3(dev)	(INTEL_INFO(dev)->gen == 3)
-#define IS_GEN4(dev)	(INTEL_INFO(dev)->gen == 4)
-#define IS_GEN5(dev)	(INTEL_INFO(dev)->gen == 5)
-#define IS_GEN6(dev)	(INTEL_INFO(dev)->gen == 6)
-#define IS_GEN7(dev)	(INTEL_INFO(dev)->gen == 7)
-#define IS_GEN8(dev)	(INTEL_INFO(dev)->gen == 8)
-#define IS_GEN9(dev)	(INTEL_INFO(dev)->gen == 9)
+#define IS_GEN2(dev)	(INTEL_INFO(dev)->gen_mask & BIT(2))
+#define IS_GEN3(dev)	(INTEL_INFO(dev)->gen_mask & BIT(3))
+#define IS_GEN4(dev)	(INTEL_INFO(dev)->gen_mask & BIT(4))
+#define IS_GEN5(dev)	(INTEL_INFO(dev)->gen_mask & BIT(5))
+#define IS_GEN6(dev)	(INTEL_INFO(dev)->gen_mask & BIT(6))
+#define IS_GEN7(dev)	(INTEL_INFO(dev)->gen_mask & BIT(7))
+#define IS_GEN8(dev)	(INTEL_INFO(dev)->gen_mask & BIT(8))
+#define IS_GEN9(dev)	(INTEL_INFO(dev)->gen_mask & BIT(9))
 
 #define RENDER_RING		(1<<RCS)
 #define BSD_RING		(1<<VCS)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Tvrtko Ursulin]

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 \
@@ -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

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

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Tvrtko Ursulin]

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
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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
> >

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Tvrtko Ursulin]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This way optimization from a previous patch works even better.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c     | 4 ++--
 drivers/gpu/drm/i915/i915_dma.c         | 4 ++--
 drivers/gpu/drm/i915/i915_drv.c         | 2 +-
 drivers/gpu/drm/i915/i915_drv.h         | 2 +-
 drivers/gpu/drm/i915/i915_gem.c         | 2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c     | 2 +-
 drivers/gpu/drm/i915/i915_gem_stolen.c  | 2 +-
 drivers/gpu/drm/i915/i915_gem_tiling.c  | 2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c   | 2 +-
 drivers/gpu/drm/i915/i915_irq.c         | 4 ++--
 drivers/gpu/drm/i915/intel_display.c    | 2 +-
 drivers/gpu/drm/i915/intel_lrc.c        | 4 ++--
 drivers/gpu/drm/i915/intel_lvds.c       | 2 +-
 drivers/gpu/drm/i915/intel_pm.c         | 4 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++---
 15 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 6ad008c196b5..e264f168ef02 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2310,12 +2310,12 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_engine_cs *engine;
 
-	if (INTEL_INFO(dev)->gen == 6)
+	if (IS_GEN6(dev_priv))
 		seq_printf(m, "GFX_MODE: 0x%08x\n", I915_READ(GFX_MODE));
 
 	for_each_engine(engine, dev_priv) {
 		seq_printf(m, "%s\n", engine->name);
-		if (INTEL_INFO(dev)->gen == 7)
+		if (IS_GEN7(dev_priv))
 			seq_printf(m, "GFX_MODE: 0x%08x\n",
 				   I915_READ(RING_MODE_GEN7(engine)));
 		seq_printf(m, "PP_DIR_BASE: 0x%08x\n",
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 01163da51b1d..a65c85c8317c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -888,7 +888,7 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
 		DRM_INFO("Display disabled (module parameter)\n");
 		info->num_pipes = 0;
 	} else if (info->num_pipes > 0 &&
-		   (INTEL_INFO(dev)->gen == 7 || INTEL_INFO(dev)->gen == 8) &&
+		   (IS_GEN7(dev_priv) || IS_GEN8(dev_priv)) &&
 		   HAS_PCH_SPLIT(dev)) {
 		u32 fuse_strap = I915_READ(FUSE_STRAP);
 		u32 sfuse_strap = I915_READ(SFUSE_STRAP);
@@ -912,7 +912,7 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
 			DRM_INFO("PipeC fused off\n");
 			info->num_pipes -= 1;
 		}
-	} else if (info->num_pipes > 0 && INTEL_INFO(dev)->gen == 9) {
+	} else if (info->num_pipes > 0 && IS_GEN9(dev_priv)) {
 		u32 dfsm = I915_READ(SKL_DFSM);
 		u8 disabled_mask = 0;
 		bool invalid;
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index da6532da44e3..f27cc15b6f8e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -548,7 +548,7 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
 
 #ifdef CONFIG_INTEL_IOMMU
 	/* Enable semaphores on SNB when IO remapping is off */
-	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
+	if (IS_GEN6(dev) && intel_iommu_gfx_mapped)
 		return false;
 #endif
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 459561991081..15fcbcece13c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2702,7 +2702,7 @@ struct drm_i915_cmd_table {
 				 IS_CHERRYVIEW(dev) || IS_SKYLAKE(dev) || \
 				 IS_KABYLAKE(dev) || IS_BROXTON(dev))
 #define HAS_RC6(dev)		(INTEL_INFO(dev)->gen >= 6)
-#define HAS_RC6p(dev)		(INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev))
+#define HAS_RC6p(dev)		(IS_GEN6(dev) || IS_IVYBRIDGE(dev))
 
 #define HAS_CSR(dev)	(IS_GEN9(dev))
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a88e6c9e9516..f2e42c0cc5be 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1982,7 +1982,7 @@ i915_gem_get_gtt_size(struct drm_device *dev, uint32_t size, int tiling_mode)
 		return size;
 
 	/* Previous chips need a power-of-two fence region when tiling */
-	if (INTEL_INFO(dev)->gen == 3)
+	if (IS_GEN3(dev))
 		gtt_size = 1024*1024;
 	else
 		gtt_size = 512*1024;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d284b17af431..61e85c2b500d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -144,7 +144,7 @@ int intel_sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
 
 #ifdef CONFIG_INTEL_IOMMU
 	/* Disable ppgtt on SNB if VT-d is on. */
-	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped) {
+	if (IS_GEN6(dev) && intel_iommu_gfx_mapped) {
 		DRM_INFO("Disabling PPGTT because VT-d is on\n");
 		return 0;
 	}
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 68fde8fba803..f9253f2b7ba0 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -56,7 +56,7 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
 
 	/* See the comment at the drm_mm_init() call for more about this check.
 	 * WaSkipStolenMemoryFirstPage:bdw,chv (incomplete) */
-	if (INTEL_INFO(dev_priv)->gen == 8 && start < 4096)
+	if (IS_GEN8(dev_priv) && start < 4096)
 		start = 4096;
 
 	mutex_lock(&dev_priv->mm.stolen_lock);
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 829dab69895f..2fcb4afade19 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -125,7 +125,7 @@ i915_gem_object_fence_ok(struct drm_i915_gem_object *obj, int tiling_mode)
 	if (INTEL_INFO(obj->base.dev)->gen >= 4)
 		return true;
 
-	if (INTEL_INFO(obj->base.dev)->gen == 3) {
+	if (IS_GEN3(obj->base.dev)) {
 		if (i915_gem_obj_ggtt_offset(obj) & ~I915_FENCE_START_MASK)
 			return false;
 	} else {
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 89725c9efc25..12a749c9707a 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -411,7 +411,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 		err_printf(m, "DONE_REG: 0x%08x\n", error->done_reg);
 	}
 
-	if (INTEL_INFO(dev)->gen == 7)
+	if (IS_GEN7(dev))
 		err_printf(m, "ERR_INT: 0x%08x\n", error->err_int);
 
 	for (i = 0; i < ARRAY_SIZE(error->ring); i++)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2f6fd33c07ba..ba84b9b17fc2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4674,12 +4674,12 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 		dev->driver->disable_vblank = ironlake_disable_vblank;
 		dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
 	} else {
-		if (INTEL_INFO(dev_priv)->gen == 2) {
+		if (IS_GEN2(dev_priv)) {
 			dev->driver->irq_preinstall = i8xx_irq_preinstall;
 			dev->driver->irq_postinstall = i8xx_irq_postinstall;
 			dev->driver->irq_handler = i8xx_irq_handler;
 			dev->driver->irq_uninstall = i8xx_irq_uninstall;
-		} else if (INTEL_INFO(dev_priv)->gen == 3) {
+		} else if (IS_GEN3(dev_priv)) {
 			dev->driver->irq_preinstall = i915_irq_preinstall;
 			dev->driver->irq_postinstall = i915_irq_postinstall;
 			dev->driver->irq_uninstall = i915_irq_uninstall;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 45c218db86be..1385423ac960 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1204,7 +1204,7 @@ static void assert_fdi_tx_pll_enabled(struct drm_i915_private *dev_priv,
 	u32 val;
 
 	/* ILK FDI PLL is always enabled */
-	if (INTEL_INFO(dev_priv)->gen == 5)
+	if (IS_GEN5(dev_priv))
 		return;
 
 	/* On Haswell, DDI ports are responsible for the FDI PLL setup */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d8763524319d..f614a07a07bb 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1387,7 +1387,7 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
 	batch = kmap_atomic(page);
 	offset = 0;
 
-	if (INTEL_INFO(engine->dev)->gen == 8) {
+	if (IS_GEN8(engine->dev)) {
 		ret = gen8_init_indirectctx_bb(engine,
 					       &wa_ctx->indirect_ctx,
 					       batch,
@@ -1401,7 +1401,7 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
 					  &offset);
 		if (ret)
 			goto out;
-	} else if (INTEL_INFO(engine->dev)->gen == 9) {
+	} else if (IS_GEN9(engine->dev)) {
 		ret = gen9_init_indirectctx_bb(engine,
 					       &wa_ctx->indirect_ctx,
 					       batch,
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index bc53c0dd34d0..d65fd945607a 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -190,7 +190,7 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
 	/* Set the dithering flag on LVDS as needed, note that there is no
 	 * special lvds dither control bit on pch-split platforms, dithering is
 	 * only controlled through the PIPECONF reg. */
-	if (INTEL_INFO(dev)->gen == 4) {
+	if (IS_GEN4(dev_priv)) {
 		/* Bspec wording suggests that LVDS port dithering only exists
 		 * for 18bpp panels. */
 		if (crtc->config->dither && crtc->config->pipe_bpp == 18)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 017c431f9363..5f17f682d37b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2146,14 +2146,14 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[8])
 static void intel_fixup_spr_wm_latency(struct drm_device *dev, uint16_t wm[5])
 {
 	/* ILK sprite LP0 latency is 1300 ns */
-	if (INTEL_INFO(dev)->gen == 5)
+	if (IS_GEN5(dev))
 		wm[0] = 13;
 }
 
 static void intel_fixup_cur_wm_latency(struct drm_device *dev, uint16_t wm[5])
 {
 	/* ILK cursor LP0 latency is 1300 ns */
-	if (INTEL_INFO(dev)->gen == 5)
+	if (IS_GEN5(dev))
 		wm[0] = 13;
 
 	/* WaDoubleCursorLP3Latency:ivb */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8f3eb3033da0..710cd598c701 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1236,7 +1236,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
 
 	/* Required for the hardware to program scanline values for waiting */
 	/* WaEnableFlushTlbInvalidationMode:snb */
-	if (INTEL_INFO(dev)->gen == 6)
+	if (IS_GEN6(dev_priv))
 		I915_WRITE(GFX_MODE,
 			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
 
@@ -2536,7 +2536,7 @@ void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno)
 	 * the semaphore value, then when the seqno moves backwards all
 	 * future waits will complete instantly (causing rendering corruption).
 	 */
-	if (INTEL_INFO(dev_priv)->gen == 6 || INTEL_INFO(dev_priv)->gen == 7) {
+	if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) {
 		I915_WRITE(RING_SYNC_0(engine->mmio_base), 0);
 		I915_WRITE(RING_SYNC_1(engine->mmio_base), 0);
 		if (HAS_VEBOX(dev_priv))
@@ -2808,7 +2808,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		engine->init_context = intel_rcs_ctx_init;
 		engine->add_request = gen6_add_request;
 		engine->flush = gen7_render_ring_flush;
-		if (INTEL_INFO(dev)->gen == 6)
+		if (IS_GEN6(dev_priv))
 			engine->flush = gen6_render_ring_flush;
 		engine->irq_get = gen6_ring_get_irq;
 		engine->irq_put = gen6_ring_put_irq;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx

[Chris Wilson]

On Fri, May 06, 2016 at 02:43:51PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This way optimization from a previous patch works even better.

Impact on object size? The hope is that gcc is able to combine tests so
reduce instruction count.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx

[Tvrtko Ursulin]

On 06/05/16 14:59, Chris Wilson wrote:
> On Fri, May 06, 2016 at 02:43:51PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> This way optimization from a previous patch works even better.
>
> Impact on object size? The hope is that gcc is able to combine tests so
> reduce instruction count.

Every patch in this series shrinks the object size.

With the bitmask approach it is able to combine tests, previously it wasn't.

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx

[Tvrtko Ursulin]

On 06/05/16 15:16, Tvrtko Ursulin wrote:
>
> On 06/05/16 14:59, Chris Wilson wrote:
>> On Fri, May 06, 2016 at 02:43:51PM +0100, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> This way optimization from a previous patch works even better.
>>
>> Impact on object size? The hope is that gcc is able to combine tests so
>> reduce instruction count.
>
> Every patch in this series shrinks the object size.
>
> With the bitmask approach it is able to combine tests, previously it
> wasn't.

To expand, ~1.5k savings after the five patches in this series.

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx

[Dave Gordon]

On 06/05/16 14:43, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> This way optimization from a previous patch works even better.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c     | 4 ++--
>   drivers/gpu/drm/i915/i915_dma.c         | 4 ++--
>   drivers/gpu/drm/i915/i915_drv.c         | 2 +-
>   drivers/gpu/drm/i915/i915_drv.h         | 2 +-
>   drivers/gpu/drm/i915/i915_gem.c         | 2 +-
>   drivers/gpu/drm/i915/i915_gem_gtt.c     | 2 +-
>   drivers/gpu/drm/i915/i915_gem_stolen.c  | 2 +-
>   drivers/gpu/drm/i915/i915_gem_tiling.c  | 2 +-
>   drivers/gpu/drm/i915/i915_gpu_error.c   | 2 +-
>   drivers/gpu/drm/i915/i915_irq.c         | 4 ++--
>   drivers/gpu/drm/i915/intel_display.c    | 2 +-
>   drivers/gpu/drm/i915/intel_lrc.c        | 4 ++--
>   drivers/gpu/drm/i915/intel_lvds.c       | 2 +-
>   drivers/gpu/drm/i915/intel_pm.c         | 4 ++--
>   drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++---
>   15 files changed, 22 insertions(+), 22 deletions(-)

This looked a very small list, I thought, and then realised you've only 
changed tests-for-equality. There are a *far* greater number of tests 
for inequality!

These are the numbers I got for converting *all* instances of 
INTEL_INFO()->gen ...

  i915/i915_debugfs.c          |  68 ++++++-------
  i915/i915_dma.c              |  20 ++--
  i915/i915_drv.c              |   8 +-
  i915/i915_drv.h              |  42 ++++----
  i915/i915_gem.c              |  18 ++--
  i915/i915_gem_context.c      |  14 +--
  i915/i915_gem_execbuffer.c   |  18 ++--
  i915/i915_gem_fence.c        |   8 +-
  i915/i915_gem_gtt.c          |  36 +++----
  i915/i915_gem_render_state.c |   2 +-
  i915/i915_gem_stolen.c       |  10 +-
  i915/i915_gem_tiling.c       |  10 +-
  i915/i915_gpu_error.c        |  38 ++++----
  i915/i915_irq.c              |  72 +++++++-------
  i915/i915_suspend.c          |  20 ++--
  i915/i915_sysfs.c            |   2 +-
  i915/intel_audio.c           |   2 +-
  i915/intel_bios.c            |   2 +-
  i915/intel_color.c           |   2 +-
  i915/intel_crt.c             |   6 +-
  i915/intel_ddi.c             |  10 +-
  i915/intel_display.c         | 182 +++++++++++++++++------------------
  i915/intel_dp.c              |  30 +++---
  i915/intel_dpll_mgr.c        |   2 +-
  i915/intel_fbc.c             |  30 +++---
  i915/intel_guc_loader.c      |   2 +-
  i915/intel_hdmi.c            |   4 +-
  i915/intel_lrc.c             |  22 ++---
  i915/intel_lvds.c            |  14 +--
  i915/intel_mocs.c            |   2 +-
  i915/intel_overlay.c         |   4 +-
  i915/intel_panel.c           |  10 +-
  i915/intel_pm.c              |  68 ++++++-------
  i915/intel_psr.c             |  10 +-
  i915/intel_ringbuffer.c      |  50 +++++-----
  i915/intel_sdvo.c            |  10 +-
  i915/intel_sprite.c          |  12 +--
  i915/intel_tv.c              |   2 +-
  i915/intel_uncore.c          |  20 ++--

So if you want to convert the GEN info to a new representation, it 
should ideally be one that efficiently handles "up to N" and "from N 
onwards" as well as exact matches.

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx

[Jani Nikula]

On Fri, 06 May 2016, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> This way optimization from a previous patch works even better.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/i915_debugfs.c     | 4 ++--
>  drivers/gpu/drm/i915/i915_dma.c         | 4 ++--
>  drivers/gpu/drm/i915/i915_drv.c         | 2 +-
>  drivers/gpu/drm/i915/i915_drv.h         | 2 +-
>  drivers/gpu/drm/i915/i915_gem.c         | 2 +-
>  drivers/gpu/drm/i915/i915_gem_gtt.c     | 2 +-
>  drivers/gpu/drm/i915/i915_gem_stolen.c  | 2 +-
>  drivers/gpu/drm/i915/i915_gem_tiling.c  | 2 +-
>  drivers/gpu/drm/i915/i915_gpu_error.c   | 2 +-
>  drivers/gpu/drm/i915/i915_irq.c         | 4 ++--
>  drivers/gpu/drm/i915/intel_display.c    | 2 +-
>  drivers/gpu/drm/i915/intel_lrc.c        | 4 ++--
>  drivers/gpu/drm/i915/intel_lvds.c       | 2 +-
>  drivers/gpu/drm/i915/intel_pm.c         | 4 ++--
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++---
>  15 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 6ad008c196b5..e264f168ef02 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2310,12 +2310,12 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_engine_cs *engine;
>  
> -	if (INTEL_INFO(dev)->gen == 6)
> +	if (IS_GEN6(dev_priv))
>  		seq_printf(m, "GFX_MODE: 0x%08x\n", I915_READ(GFX_MODE));
>  
>  	for_each_engine(engine, dev_priv) {
>  		seq_printf(m, "%s\n", engine->name);
> -		if (INTEL_INFO(dev)->gen == 7)
> +		if (IS_GEN7(dev_priv))
>  			seq_printf(m, "GFX_MODE: 0x%08x\n",
>  				   I915_READ(RING_MODE_GEN7(engine)));
>  		seq_printf(m, "PP_DIR_BASE: 0x%08x\n",
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 01163da51b1d..a65c85c8317c 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -888,7 +888,7 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
>  		DRM_INFO("Display disabled (module parameter)\n");
>  		info->num_pipes = 0;
>  	} else if (info->num_pipes > 0 &&
> -		   (INTEL_INFO(dev)->gen == 7 || INTEL_INFO(dev)->gen == 8) &&
> +		   (IS_GEN7(dev_priv) || IS_GEN8(dev_priv)) &&
>  		   HAS_PCH_SPLIT(dev)) {
>  		u32 fuse_strap = I915_READ(FUSE_STRAP);
>  		u32 sfuse_strap = I915_READ(SFUSE_STRAP);
> @@ -912,7 +912,7 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
>  			DRM_INFO("PipeC fused off\n");
>  			info->num_pipes -= 1;
>  		}
> -	} else if (info->num_pipes > 0 && INTEL_INFO(dev)->gen == 9) {
> +	} else if (info->num_pipes > 0 && IS_GEN9(dev_priv)) {
>  		u32 dfsm = I915_READ(SKL_DFSM);
>  		u8 disabled_mask = 0;
>  		bool invalid;
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index da6532da44e3..f27cc15b6f8e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -548,7 +548,7 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
>  
>  #ifdef CONFIG_INTEL_IOMMU
>  	/* Enable semaphores on SNB when IO remapping is off */
> -	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
> +	if (IS_GEN6(dev) && intel_iommu_gfx_mapped)
>  		return false;
>  #endif
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 459561991081..15fcbcece13c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2702,7 +2702,7 @@ struct drm_i915_cmd_table {
>  				 IS_CHERRYVIEW(dev) || IS_SKYLAKE(dev) || \
>  				 IS_KABYLAKE(dev) || IS_BROXTON(dev))
>  #define HAS_RC6(dev)		(INTEL_INFO(dev)->gen >= 6)
> -#define HAS_RC6p(dev)		(INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev))
> +#define HAS_RC6p(dev)		(IS_GEN6(dev) || IS_IVYBRIDGE(dev))
>  
>  #define HAS_CSR(dev)	(IS_GEN9(dev))
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index a88e6c9e9516..f2e42c0cc5be 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1982,7 +1982,7 @@ i915_gem_get_gtt_size(struct drm_device *dev, uint32_t size, int tiling_mode)
>  		return size;
>  
>  	/* Previous chips need a power-of-two fence region when tiling */
> -	if (INTEL_INFO(dev)->gen == 3)
> +	if (IS_GEN3(dev))
>  		gtt_size = 1024*1024;
>  	else
>  		gtt_size = 512*1024;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index d284b17af431..61e85c2b500d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -144,7 +144,7 @@ int intel_sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
>  
>  #ifdef CONFIG_INTEL_IOMMU
>  	/* Disable ppgtt on SNB if VT-d is on. */
> -	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped) {
> +	if (IS_GEN6(dev) && intel_iommu_gfx_mapped) {
>  		DRM_INFO("Disabling PPGTT because VT-d is on\n");
>  		return 0;
>  	}
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 68fde8fba803..f9253f2b7ba0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -56,7 +56,7 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
>  
>  	/* See the comment at the drm_mm_init() call for more about this check.
>  	 * WaSkipStolenMemoryFirstPage:bdw,chv (incomplete) */
> -	if (INTEL_INFO(dev_priv)->gen == 8 && start < 4096)
> +	if (IS_GEN8(dev_priv) && start < 4096)
>  		start = 4096;
>  
>  	mutex_lock(&dev_priv->mm.stolen_lock);
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index 829dab69895f..2fcb4afade19 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -125,7 +125,7 @@ i915_gem_object_fence_ok(struct drm_i915_gem_object *obj, int tiling_mode)
>  	if (INTEL_INFO(obj->base.dev)->gen >= 4)
>  		return true;
>  
> -	if (INTEL_INFO(obj->base.dev)->gen == 3) {
> +	if (IS_GEN3(obj->base.dev)) {
>  		if (i915_gem_obj_ggtt_offset(obj) & ~I915_FENCE_START_MASK)
>  			return false;
>  	} else {
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 89725c9efc25..12a749c9707a 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -411,7 +411,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>  		err_printf(m, "DONE_REG: 0x%08x\n", error->done_reg);
>  	}
>  
> -	if (INTEL_INFO(dev)->gen == 7)
> +	if (IS_GEN7(dev))
>  		err_printf(m, "ERR_INT: 0x%08x\n", error->err_int);
>  
>  	for (i = 0; i < ARRAY_SIZE(error->ring); i++)
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 2f6fd33c07ba..ba84b9b17fc2 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4674,12 +4674,12 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>  		dev->driver->disable_vblank = ironlake_disable_vblank;
>  		dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
>  	} else {
> -		if (INTEL_INFO(dev_priv)->gen == 2) {
> +		if (IS_GEN2(dev_priv)) {
>  			dev->driver->irq_preinstall = i8xx_irq_preinstall;
>  			dev->driver->irq_postinstall = i8xx_irq_postinstall;
>  			dev->driver->irq_handler = i8xx_irq_handler;
>  			dev->driver->irq_uninstall = i8xx_irq_uninstall;
> -		} else if (INTEL_INFO(dev_priv)->gen == 3) {
> +		} else if (IS_GEN3(dev_priv)) {
>  			dev->driver->irq_preinstall = i915_irq_preinstall;
>  			dev->driver->irq_postinstall = i915_irq_postinstall;
>  			dev->driver->irq_uninstall = i915_irq_uninstall;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 45c218db86be..1385423ac960 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1204,7 +1204,7 @@ static void assert_fdi_tx_pll_enabled(struct drm_i915_private *dev_priv,
>  	u32 val;
>  
>  	/* ILK FDI PLL is always enabled */
> -	if (INTEL_INFO(dev_priv)->gen == 5)
> +	if (IS_GEN5(dev_priv))
>  		return;
>  
>  	/* On Haswell, DDI ports are responsible for the FDI PLL setup */
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index d8763524319d..f614a07a07bb 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1387,7 +1387,7 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
>  	batch = kmap_atomic(page);
>  	offset = 0;
>  
> -	if (INTEL_INFO(engine->dev)->gen == 8) {
> +	if (IS_GEN8(engine->dev)) {
>  		ret = gen8_init_indirectctx_bb(engine,
>  					       &wa_ctx->indirect_ctx,
>  					       batch,
> @@ -1401,7 +1401,7 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
>  					  &offset);
>  		if (ret)
>  			goto out;
> -	} else if (INTEL_INFO(engine->dev)->gen == 9) {
> +	} else if (IS_GEN9(engine->dev)) {
>  		ret = gen9_init_indirectctx_bb(engine,
>  					       &wa_ctx->indirect_ctx,
>  					       batch,
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index bc53c0dd34d0..d65fd945607a 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -190,7 +190,7 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
>  	/* Set the dithering flag on LVDS as needed, note that there is no
>  	 * special lvds dither control bit on pch-split platforms, dithering is
>  	 * only controlled through the PIPECONF reg. */
> -	if (INTEL_INFO(dev)->gen == 4) {
> +	if (IS_GEN4(dev_priv)) {
>  		/* Bspec wording suggests that LVDS port dithering only exists
>  		 * for 18bpp panels. */
>  		if (crtc->config->dither && crtc->config->pipe_bpp == 18)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 017c431f9363..5f17f682d37b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2146,14 +2146,14 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[8])
>  static void intel_fixup_spr_wm_latency(struct drm_device *dev, uint16_t wm[5])
>  {
>  	/* ILK sprite LP0 latency is 1300 ns */
> -	if (INTEL_INFO(dev)->gen == 5)
> +	if (IS_GEN5(dev))
>  		wm[0] = 13;
>  }
>  
>  static void intel_fixup_cur_wm_latency(struct drm_device *dev, uint16_t wm[5])
>  {
>  	/* ILK cursor LP0 latency is 1300 ns */
> -	if (INTEL_INFO(dev)->gen == 5)
> +	if (IS_GEN5(dev))
>  		wm[0] = 13;
>  
>  	/* WaDoubleCursorLP3Latency:ivb */
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 8f3eb3033da0..710cd598c701 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1236,7 +1236,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
>  
>  	/* Required for the hardware to program scanline values for waiting */
>  	/* WaEnableFlushTlbInvalidationMode:snb */
> -	if (INTEL_INFO(dev)->gen == 6)
> +	if (IS_GEN6(dev_priv))
>  		I915_WRITE(GFX_MODE,
>  			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>  
> @@ -2536,7 +2536,7 @@ void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno)
>  	 * the semaphore value, then when the seqno moves backwards all
>  	 * future waits will complete instantly (causing rendering corruption).
>  	 */
> -	if (INTEL_INFO(dev_priv)->gen == 6 || INTEL_INFO(dev_priv)->gen == 7) {
> +	if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) {
>  		I915_WRITE(RING_SYNC_0(engine->mmio_base), 0);
>  		I915_WRITE(RING_SYNC_1(engine->mmio_base), 0);
>  		if (HAS_VEBOX(dev_priv))
> @@ -2808,7 +2808,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>  		engine->init_context = intel_rcs_ctx_init;
>  		engine->add_request = gen6_add_request;
>  		engine->flush = gen7_render_ring_flush;
> -		if (INTEL_INFO(dev)->gen == 6)
> +		if (IS_GEN6(dev_priv))
>  			engine->flush = gen6_render_ring_flush;
>  		engine->irq_get = gen6_ring_get_irq;
>  		engine->irq_put = gen6_ring_put_irq;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

To be used for more efficient Gen range checking.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 +
 drivers/gpu/drm/i915/intel_fbc.c        |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 15fcbcece13c..935e381407ba 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
 })
 #define INTEL_INFO(p) 	(&__I915__(p)->info)
 #define INTEL_GEN(p)	(INTEL_INFO(p)->gen)
+#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
 #define INTEL_DEVID(p)	(INTEL_INFO(p)->device_id)
 #define INTEL_REVID(p)	(__I915__(p)->dev->pdev->revision)
 
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index d5a7cfec589b..2c3681757aba 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
 
 	/* FIXME: We lack the proper locking here, so only run this on the
 	 * platforms that need. */
-	if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
+	if (INTEL_GEN_RANGE(dev_priv, 5, 6))
 		cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
 	cache->fb.pixel_format = fb->pixel_format;
 	cache->fb.stride = fb->pitches[0];
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 710cd598c701..c3dbb74d048b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -508,7 +508,7 @@ static void intel_ring_setup_status_page(struct intel_engine_cs *engine)
 	 * arises: do we still need this and if so how should we go about
 	 * invalidating the TLB?
 	 */
-	if (INTEL_INFO(dev)->gen >= 6 && INTEL_INFO(dev)->gen < 8) {
+	if (INTEL_GEN_RANGE(dev_priv, 6, 7)) {
 		i915_reg_t reg = RING_INSTPM(engine->mmio_base);
 
 		/* ring should be idle before issuing a sync flush*/
@@ -1222,7 +1222,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
 		return ret;
 
 	/* WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb */
-	if (INTEL_INFO(dev)->gen >= 4 && INTEL_INFO(dev)->gen < 7)
+	if (INTEL_GEN_RANGE(dev_priv, 4, 6))
 		I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(VS_TIMER_DISPATCH));
 
 	/* We need to disable the AsyncFlip performance optimisations in order
@@ -1231,7 +1231,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
 	 *
 	 * WaDisableAsyncFlipPerfMode:snb,ivb,hsw,vlv
 	 */
-	if (INTEL_INFO(dev)->gen >= 6 && INTEL_INFO(dev)->gen < 8)
+	if (INTEL_GEN_RANGE(dev_priv, 6, 7))
 		I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(ASYNC_FLIP_PERF_DISABLE));
 
 	/* Required for the hardware to program scanline values for waiting */
@@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
 			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
 
 	/* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
-	if (IS_GEN7(dev))
+	if (IS_GEN7(dev_priv))
 		I915_WRITE(GFX_MODE_GEN7,
 			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
 			   _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
 
-	if (IS_GEN6(dev)) {
+	if (IS_GEN6(dev_priv)) {
 		/* From the Sandybridge PRM, volume 1 part 3, page 24:
 		 * "If this bit is set, STCunit will have LRA as replacement
 		 *  policy. [...] This bit must be reset.  LRA replacement
@@ -1256,7 +1256,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
 			   _MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
 	}
 
-	if (INTEL_INFO(dev)->gen >= 6 && INTEL_INFO(dev)->gen < 8)
+	if (INTEL_GEN_RANGE(dev_priv, 6, 7))
 		I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING));
 
 	if (HAS_L3_DPF(dev))
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: Do not use a bitfield for INTEL_INFO->num_pipes

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

It just makes more work for the compiler and generates more code.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 935e381407ba..f7a156b8d5a1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -758,7 +758,7 @@ struct intel_csr {
 struct intel_device_info {
 	u32 display_mmio_offset;
 	u16 device_id;
-	u8 num_pipes:3;
+	u8 num_pipes;
 	u8 num_sprites[I915_MAX_PIPES];
 	u8 gen;
 	u16 gen_mask;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Do not use a bitfield for INTEL_INFO->num_pipes

[Dave Gordon]

On 06/05/16 15:16, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> It just makes more work for the compiler and generates more code.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 935e381407ba..f7a156b8d5a1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -758,7 +758,7 @@ struct intel_csr {
>   struct intel_device_info {
>   	u32 display_mmio_offset;
>   	u16 device_id;
> -	u8 num_pipes:3;
> +	u8 num_pipes;
>   	u8 num_sprites[I915_MAX_PIPES];
>   	u8 gen;
>   	u16 gen_mask;

On my build, this saves 322 bytes, which (since it eliminates just ONE 
three-byte instruction on each use), suggests it's used quite a lot!

Reviewed-by: Dave Gordon <david.s.gordon@intel.com>

.Dave.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro

[Chris Wilson]

On Fri, May 06, 2016 at 03:16:25PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> To be used for more efficient Gen range checking.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  1 +
>  drivers/gpu/drm/i915/intel_fbc.c        |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
>  3 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 15fcbcece13c..935e381407ba 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
>  })
>  #define INTEL_INFO(p) 	(&__I915__(p)->info)
>  #define INTEL_GEN(p)	(INTEL_INFO(p)->gen)
> +#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
>  #define INTEL_DEVID(p)	(INTEL_INFO(p)->device_id)
>  #define INTEL_REVID(p)	(__I915__(p)->dev->pdev->revision)
>  
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index d5a7cfec589b..2c3681757aba 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
>  
>  	/* FIXME: We lack the proper locking here, so only run this on the
>  	 * platforms that need. */
> -	if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
> +	if (INTEL_GEN_RANGE(dev_priv, 5, 6))
>  		cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
>  	cache->fb.pixel_format = fb->pixel_format;
>  	cache->fb.stride = fb->pitches[0];
> @@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
>  			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>  
>  	/* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
> -	if (IS_GEN7(dev))
> +	if (IS_GEN7(dev_priv))
>  		I915_WRITE(GFX_MODE_GEN7,
>  			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
>  			   _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
>  
> -	if (IS_GEN6(dev)) {
> +	if (IS_GEN6(dev_priv)) {

This chunk shouldn't be in this patch.

Couldn't improve upon INTEL_GEN_RANGE.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro

[Dave Gordon]

On 06/05/16 15:33, Chris Wilson wrote:
> On Fri, May 06, 2016 at 03:16:25PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> To be used for more efficient Gen range checking.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h         |  1 +
>>   drivers/gpu/drm/i915/intel_fbc.c        |  2 +-
>>   drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
>>   3 files changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 15fcbcece13c..935e381407ba 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
>>   })
>>   #define INTEL_INFO(p) 	(&__I915__(p)->info)
>>   #define INTEL_GEN(p)	(INTEL_INFO(p)->gen)
>> +#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
>>   #define INTEL_DEVID(p)	(INTEL_INFO(p)->device_id)
>>   #define INTEL_REVID(p)	(__I915__(p)->dev->pdev->revision)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
>> index d5a7cfec589b..2c3681757aba 100644
>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>> @@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
>>
>>   	/* FIXME: We lack the proper locking here, so only run this on the
>>   	 * platforms that need. */
>> -	if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
>> +	if (INTEL_GEN_RANGE(dev_priv, 5, 6))
>>   		cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
>>   	cache->fb.pixel_format = fb->pixel_format;
>>   	cache->fb.stride = fb->pitches[0];
>> @@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
>>   			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>>
>>   	/* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
>> -	if (IS_GEN7(dev))
>> +	if (IS_GEN7(dev_priv))
>>   		I915_WRITE(GFX_MODE_GEN7,
>>   			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
>>   			   _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
>>
>> -	if (IS_GEN6(dev)) {
>> +	if (IS_GEN6(dev_priv)) {
>
> This chunk shouldn't be in this patch.
>
> Couldn't improve upon INTEL_GEN_RANGE.
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

INTEL_GEN_IN_RANGE() ?

Perhaps emphasises that we're using INclusive ranges?

(whereas the open-coded version might use inclusive or exclusive or 
semi-inclusive at either end, as in the example above.)

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro

[Jani Nikula]

On Fri, 06 May 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
> On 06/05/16 15:33, Chris Wilson wrote:
>> On Fri, May 06, 2016 at 03:16:25PM +0100, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> To be used for more efficient Gen range checking.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_drv.h         |  1 +
>>>   drivers/gpu/drm/i915/intel_fbc.c        |  2 +-
>>>   drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
>>>   3 files changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index 15fcbcece13c..935e381407ba 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
>>>   })
>>>   #define INTEL_INFO(p) 	(&__I915__(p)->info)
>>>   #define INTEL_GEN(p)	(INTEL_INFO(p)->gen)
>>> +#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
>>>   #define INTEL_DEVID(p)	(INTEL_INFO(p)->device_id)
>>>   #define INTEL_REVID(p)	(__I915__(p)->dev->pdev->revision)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
>>> index d5a7cfec589b..2c3681757aba 100644
>>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>>> @@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
>>>
>>>   	/* FIXME: We lack the proper locking here, so only run this on the
>>>   	 * platforms that need. */
>>> -	if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
>>> +	if (INTEL_GEN_RANGE(dev_priv, 5, 6))
>>>   		cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
>>>   	cache->fb.pixel_format = fb->pixel_format;
>>>   	cache->fb.stride = fb->pitches[0];
>>> @@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
>>>   			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>>>
>>>   	/* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
>>> -	if (IS_GEN7(dev))
>>> +	if (IS_GEN7(dev_priv))
>>>   		I915_WRITE(GFX_MODE_GEN7,
>>>   			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
>>>   			   _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
>>>
>>> -	if (IS_GEN6(dev)) {
>>> +	if (IS_GEN6(dev_priv)) {
>>
>> This chunk shouldn't be in this patch.
>>
>> Couldn't improve upon INTEL_GEN_RANGE.
>>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>> -Chris
>
> INTEL_GEN_IN_RANGE() ?
>
> Perhaps emphasises that we're using INclusive ranges?

Or just IS_GEN(p, since, until) similar to IS_SKL_REVID() and
IS_BXT_REVID(). Might as well make this shorter to write.

BR,
Jani.

>
> (whereas the open-coded version might use inclusive or exclusive or 
> semi-inclusive at either end, as in the example above.)
>
> .Dave.
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro

[Dave Gordon]

On 09/05/16 13:32, Jani Nikula wrote:
> On Fri, 06 May 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
>> On 06/05/16 15:33, Chris Wilson wrote:
>>> On Fri, May 06, 2016 at 03:16:25PM +0100, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> To be used for more efficient Gen range checking.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/i915_drv.h         |  1 +
>>>>    drivers/gpu/drm/i915/intel_fbc.c        |  2 +-
>>>>    drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
>>>>    3 files changed, 8 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>> index 15fcbcece13c..935e381407ba 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>> @@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
>>>>    })
>>>>    #define INTEL_INFO(p) 	(&__I915__(p)->info)
>>>>    #define INTEL_GEN(p)	(INTEL_INFO(p)->gen)
>>>> +#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
>>>>    #define INTEL_DEVID(p)	(INTEL_INFO(p)->device_id)
>>>>    #define INTEL_REVID(p)	(__I915__(p)->dev->pdev->revision)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
>>>> index d5a7cfec589b..2c3681757aba 100644
>>>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>>>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>>>> @@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
>>>>
>>>>    	/* FIXME: We lack the proper locking here, so only run this on the
>>>>    	 * platforms that need. */
>>>> -	if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
>>>> +	if (INTEL_GEN_RANGE(dev_priv, 5, 6))
>>>>    		cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
>>>>    	cache->fb.pixel_format = fb->pixel_format;
>>>>    	cache->fb.stride = fb->pitches[0];
>>>> @@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
>>>>    			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>>>>
>>>>    	/* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
>>>> -	if (IS_GEN7(dev))
>>>> +	if (IS_GEN7(dev_priv))
>>>>    		I915_WRITE(GFX_MODE_GEN7,
>>>>    			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
>>>>    			   _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
>>>>
>>>> -	if (IS_GEN6(dev)) {
>>>> +	if (IS_GEN6(dev_priv)) {
>>>
>>> This chunk shouldn't be in this patch.
>>>
>>> Couldn't improve upon INTEL_GEN_RANGE.
>>>
>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> -Chris
>>
>> INTEL_GEN_IN_RANGE() ?
>>
>> Perhaps emphasises that we're using INclusive ranges?
>
> Or just IS_GEN(p, since, until) similar to IS_SKL_REVID() and
> IS_BXT_REVID(). Might as well make this shorter to write.
>
> BR,
> Jani.

We need a notation that will look good (and be efficient) for "before 
GEN x" and "from GEN y on" (or "up to and including GEN p" and "after 
GEN q") -- these make up the vast majority of tests on the GEN number.

Also, we need real clarity on inclusion/exclusion. since..until notation 
is perhaps ambiguous - in particular, is "until" included? English usage 
is not clear here, it's often but not always exclusive!

.Dave.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro

[Jani Nikula]

On Mon, 09 May 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
> On 09/05/16 13:32, Jani Nikula wrote:
>> On Fri, 06 May 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
>>> On 06/05/16 15:33, Chris Wilson wrote:
>>>> On Fri, May 06, 2016 at 03:16:25PM +0100, Tvrtko Ursulin wrote:
>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>
>>>>> To be used for more efficient Gen range checking.
>>>>>
>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/i915_drv.h         |  1 +
>>>>>    drivers/gpu/drm/i915/intel_fbc.c        |  2 +-
>>>>>    drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
>>>>>    3 files changed, 8 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>>> index 15fcbcece13c..935e381407ba 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>> @@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
>>>>>    })
>>>>>    #define INTEL_INFO(p) 	(&__I915__(p)->info)
>>>>>    #define INTEL_GEN(p)	(INTEL_INFO(p)->gen)
>>>>> +#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
>>>>>    #define INTEL_DEVID(p)	(INTEL_INFO(p)->device_id)
>>>>>    #define INTEL_REVID(p)	(__I915__(p)->dev->pdev->revision)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
>>>>> index d5a7cfec589b..2c3681757aba 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>>>>> @@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
>>>>>
>>>>>    	/* FIXME: We lack the proper locking here, so only run this on the
>>>>>    	 * platforms that need. */
>>>>> -	if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
>>>>> +	if (INTEL_GEN_RANGE(dev_priv, 5, 6))
>>>>>    		cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
>>>>>    	cache->fb.pixel_format = fb->pixel_format;
>>>>>    	cache->fb.stride = fb->pitches[0];
>>>>> @@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
>>>>>    			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>>>>>
>>>>>    	/* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
>>>>> -	if (IS_GEN7(dev))
>>>>> +	if (IS_GEN7(dev_priv))
>>>>>    		I915_WRITE(GFX_MODE_GEN7,
>>>>>    			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
>>>>>    			   _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
>>>>>
>>>>> -	if (IS_GEN6(dev)) {
>>>>> +	if (IS_GEN6(dev_priv)) {
>>>>
>>>> This chunk shouldn't be in this patch.
>>>>
>>>> Couldn't improve upon INTEL_GEN_RANGE.
>>>>
>>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>> -Chris
>>>
>>> INTEL_GEN_IN_RANGE() ?
>>>
>>> Perhaps emphasises that we're using INclusive ranges?
>>
>> Or just IS_GEN(p, since, until) similar to IS_SKL_REVID() and
>> IS_BXT_REVID(). Might as well make this shorter to write.
>>
>> BR,
>> Jani.
>
> We need a notation that will look good (and be efficient) for "before 
> GEN x" and "from GEN y on" (or "up to and including GEN p" and "after 
> GEN q") -- these make up the vast majority of tests on the GEN number.
>
> Also, we need real clarity on inclusion/exclusion. since..until notation 
> is perhaps ambiguous - in particular, is "until" included? English usage 
> is not clear here, it's often but not always exclusive!

If you want real clarity, you'll ditch this patch and use

	if (INTEL_GEN(p) >= 5 && INTEL_GEN(p) <= 7)
        	etc...

and be done with it. There's too much churn going on with all the
related macros anyway, so I'm not at all convinced about the patches
anyway.

J.



>
> .Dave.
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro

[Chris Wilson]

On Mon, May 09, 2016 at 05:40:39PM +0300, Jani Nikula wrote:
> On Mon, 09 May 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
> > On 09/05/16 13:32, Jani Nikula wrote:
> >> On Fri, 06 May 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
> >>> On 06/05/16 15:33, Chris Wilson wrote:
> >>>> On Fri, May 06, 2016 at 03:16:25PM +0100, Tvrtko Ursulin wrote:
> >>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>>
> >>>>> To be used for more efficient Gen range checking.
> >>>>>
> >>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>> ---
> >>>>>    drivers/gpu/drm/i915/i915_drv.h         |  1 +
> >>>>>    drivers/gpu/drm/i915/intel_fbc.c        |  2 +-
> >>>>>    drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
> >>>>>    3 files changed, 8 insertions(+), 7 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>>>> index 15fcbcece13c..935e381407ba 100644
> >>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
> >>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >>>>> @@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
> >>>>>    })
> >>>>>    #define INTEL_INFO(p) 	(&__I915__(p)->info)
> >>>>>    #define INTEL_GEN(p)	(INTEL_INFO(p)->gen)
> >>>>> +#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
> >>>>>    #define INTEL_DEVID(p)	(INTEL_INFO(p)->device_id)
> >>>>>    #define INTEL_REVID(p)	(__I915__(p)->dev->pdev->revision)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> >>>>> index d5a7cfec589b..2c3681757aba 100644
> >>>>> --- a/drivers/gpu/drm/i915/intel_fbc.c
> >>>>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> >>>>> @@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
> >>>>>
> >>>>>    	/* FIXME: We lack the proper locking here, so only run this on the
> >>>>>    	 * platforms that need. */
> >>>>> -	if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
> >>>>> +	if (INTEL_GEN_RANGE(dev_priv, 5, 6))
> >>>>>    		cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
> >>>>>    	cache->fb.pixel_format = fb->pixel_format;
> >>>>>    	cache->fb.stride = fb->pitches[0];
> >>>>> @@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
> >>>>>    			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
> >>>>>
> >>>>>    	/* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
> >>>>> -	if (IS_GEN7(dev))
> >>>>> +	if (IS_GEN7(dev_priv))
> >>>>>    		I915_WRITE(GFX_MODE_GEN7,
> >>>>>    			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
> >>>>>    			   _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
> >>>>>
> >>>>> -	if (IS_GEN6(dev)) {
> >>>>> +	if (IS_GEN6(dev_priv)) {
> >>>>
> >>>> This chunk shouldn't be in this patch.
> >>>>
> >>>> Couldn't improve upon INTEL_GEN_RANGE.
> >>>>
> >>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>> -Chris
> >>>
> >>> INTEL_GEN_IN_RANGE() ?
> >>>
> >>> Perhaps emphasises that we're using INclusive ranges?
> >>
> >> Or just IS_GEN(p, since, until) similar to IS_SKL_REVID() and
> >> IS_BXT_REVID(). Might as well make this shorter to write.
> >>
> >> BR,
> >> Jani.
> >
> > We need a notation that will look good (and be efficient) for "before 
> > GEN x" and "from GEN y on" (or "up to and including GEN p" and "after 
> > GEN q") -- these make up the vast majority of tests on the GEN number.
> >
> > Also, we need real clarity on inclusion/exclusion. since..until notation 
> > is perhaps ambiguous - in particular, is "until" included? English usage 
> > is not clear here, it's often but not always exclusive!
> 
> If you want real clarity, you'll ditch this patch and use
> 
> 	if (INTEL_GEN(p) >= 5 && INTEL_GEN(p) <= 7)
>         	etc...
> 
> and be done with it. There's too much churn going on with all the
> related macros anyway, so I'm not at all convinced about the patches
> anyway.

IS_GEN(p, start, end) using inclusive ranges is fine.

This is a small patch that generates a remarkable amount of object code
reduction.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro

[Jani Nikula]

On Mon, 09 May 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> IS_GEN(p, start, end) using inclusive ranges is fine.
>
> This is a small patch that generates a remarkable amount of object code
> reduction.

Ok.

I guess 0 is okay for start, but how about a "GEN_FOREVER" style macro
for replacing ->gen >= 5 with IS_GEN(dev_priv, 5, GEN_FOREVER)? This
would be in line with the IS_{SKL,BXT}_REVID checks.


BR,
Jani.



-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask

[Chris Wilson]

On Fri, May 06, 2016 at 02:43:49PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> If instead of numerical comparison me make these test a
> bitmask, we enable the compiler to optimize all instances
> of IS_GENx || IS_GENy.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++--------
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index ad7abe517700..01163da51b1d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1071,6 +1071,9 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>  	memcpy(device_info, info, sizeof(dev_priv->info));
>  	device_info->device_id = dev->pdev->device;
>  
> +	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * 8);

Should be gen >= num_bits
BITS_PER_BYTE

> +	device_info->gen_mask = 1 << device_info->gen;

device_info->gen_mask = BIT(device_info->gen) for consistency;
> +
>  	spin_lock_init(&dev_priv->irq_lock);
>  	spin_lock_init(&dev_priv->gpu_error.lock);
>  	mutex_init(&dev_priv->backlight_lock);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d5496aba1cd5..c6351016eaf0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -760,6 +760,7 @@ struct intel_device_info {
>  	u8 num_pipes:3;
>  	u8 num_sprites[I915_MAX_PIPES];
>  	u8 gen;
> +	u16 gen_mask;

Holey? Move gen_mask next to device id, and those u8
num_pipes/num_sprites can be moved down to the array of u8s.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask

[Tvrtko Ursulin]

On 06/05/16 15:28, Chris Wilson wrote:
> On Fri, May 06, 2016 at 02:43:49PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> If instead of numerical comparison me make these test a
>> bitmask, we enable the compiler to optimize all instances
>> of IS_GENx || IS_GENy.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_dma.c |  3 +++
>>   drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++--------
>>   2 files changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index ad7abe517700..01163da51b1d 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -1071,6 +1071,9 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>>   	memcpy(device_info, info, sizeof(dev_priv->info));
>>   	device_info->device_id = dev->pdev->device;
>>
>> +	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * 8);
>
> Should be gen >= num_bits
> BITS_PER_BYTE

Hm yes, or..

>> +	device_info->gen_mask = 1 << device_info->gen;
>
> device_info->gen_mask = BIT(device_info->gen) for consistency;

.. maybe better BIT(device_info->gen - 1) ? There is no Gen0 presumably?

>> +
>>   	spin_lock_init(&dev_priv->irq_lock);
>>   	spin_lock_init(&dev_priv->gpu_error.lock);
>>   	mutex_init(&dev_priv->backlight_lock);
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index d5496aba1cd5..c6351016eaf0 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -760,6 +760,7 @@ struct intel_device_info {
>>   	u8 num_pipes:3;
>>   	u8 num_sprites[I915_MAX_PIPES];
>>   	u8 gen;
>> +	u16 gen_mask;
>
> Holey? Move gen_mask next to device id, and those u8
> num_pipes/num_sprites can be moved down to the array of u8s.

Any fiddling I tried there made no difference (apart from getting rid of 
the num_pipes bitfield). But can do.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask

[Chris Wilson]

On Fri, May 06, 2016 at 03:36:26PM +0100, Tvrtko Ursulin wrote:
> 
> On 06/05/16 15:28, Chris Wilson wrote:
> >On Fri, May 06, 2016 at 02:43:49PM +0100, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >>If instead of numerical comparison me make these test a
> >>bitmask, we enable the compiler to optimize all instances
> >>of IS_GENx || IS_GENy.
> >>
> >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/i915_dma.c |  3 +++
> >>  drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++--------
> >>  2 files changed, 12 insertions(+), 8 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> >>index ad7abe517700..01163da51b1d 100644
> >>--- a/drivers/gpu/drm/i915/i915_dma.c
> >>+++ b/drivers/gpu/drm/i915/i915_dma.c
> >>@@ -1071,6 +1071,9 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
> >>  	memcpy(device_info, info, sizeof(dev_priv->info));
> >>  	device_info->device_id = dev->pdev->device;
> >>
> >>+	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * 8);
> >
> >Should be gen >= num_bits
> >BITS_PER_BYTE
> 
> Hm yes, or..
> 
> >>+	device_info->gen_mask = 1 << device_info->gen;
> >
> >device_info->gen_mask = BIT(device_info->gen) for consistency;
> 
> .. maybe better BIT(device_info->gen - 1) ? There is no Gen0 presumably?

i740 would be gen0
i810 is gen1 (which is an on-chipset version of i740 really)

One day we may achieve a goal a completely unified driver if Daniel
hasn't lost his i810 yet.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx