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

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, Intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask
Date: Fri, 6 May 2016 15:36:26 +0100	[thread overview]
Message-ID: <572CABEA.2000300@linux.intel.com> (raw)
In-Reply-To: <20160506142844.GE30822@nuc-i3427.alporthouse.com>

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