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

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro
Date: Fri, 6 May 2016 15:18:16 +0100	[thread overview]
Message-ID: <572CA7A8.7010806@linux.intel.com> (raw)
In-Reply-To: <20160506140010.GB4329@intel.com>

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