[Intel-gfx] [PATCH] drm/i915: Use bitmask for IS_REVID checking
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Tue May 10 13:59:01 UTC 2016
Hi,
On 10/05/16 14:33, Jani Nikula wrote:
> On Tue, 10 May 2016, Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com> wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> With this scheme all call sites of IS_SKL_REVID and IS_BXT_REVID
>> result in a maximum of one conditional jump instruction (was
>> three before) and overall reduction in code size.
>
> IMO we really shouldn't bother with this. The end goal is that we remove
> all of these when we decide we no longer care about early
> (non-production) revisions for a platform. For Skylake I'd argue we are
> beyond this point.
Thought that simply removing everything guarded by IS_SKL_REVID might be
possible never crossed my mind. :) Which one is the first production
stepping then?
Regards,
Tvrtko.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_dma.c | 11 +++++++++++
>> drivers/gpu/drm/i915/i915_drv.h | 24 +++++++++++++++++++-----
>> 2 files changed, 30 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 46ac1da64a09..1ae812436827 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -1072,6 +1072,17 @@ 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;
>>
>> + /* For unknown revisions mask stays at zero which is correct. */
>> + if (dev->pdev->revision <
>> + sizeof(device_info->revision_mask) * BITS_PER_BYTE)
>> + device_info->revision_mask = BIT(dev->pdev->revision);
>> +
>> + if (IS_SKYLAKE(dev_priv))
>> + device_info->skl_rev_mask = ~0;
>> +
>> + if (IS_BROXTON(dev_priv))
>> + device_info->bxt_rev_mask = ~0;
>> +
>> 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 26e7de415966..fbeb68e9ec96 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -757,6 +757,9 @@ struct intel_csr {
>> struct intel_device_info {
>> u32 display_mmio_offset;
>> u16 device_id;
>> + u8 revision_mask;
>> + u8 skl_rev_mask;
>> + u8 bxt_rev_mask;
>> u8 num_pipes:3;
>> u8 num_sprites[I915_MAX_PIPES];
>> u8 gen;
>> @@ -2519,14 +2522,23 @@ struct drm_i915_cmd_table {
>> #define INTEL_DEVID(p) (INTEL_INFO(p)->device_id)
>> #define INTEL_REVID(p) (__I915__(p)->dev->pdev->revision)
>>
>> -#define REVID_FOREVER 0xff
>> +#define REVID_FOREVER 0
>> +
>> /*
>> * Return true if revision is in range [since,until] inclusive.
>> *
>> * Use 0 for open-ended since, and REVID_FOREVER for open-ended until.
>> */
>> -#define IS_REVID(p, since, until) \
>> - (INTEL_REVID(p) >= (since) && INTEL_REVID(p) <= (until))
>> +#define IS_REVID(p, mask, since, until) ({\
>> + unsigned int __s = (since), __e = (until); \
>> + BUILD_BUG_ON(!__builtin_constant_p(since)); \
>> + BUILD_BUG_ON(!__builtin_constant_p(until)); \
>> + BUILD_BUG_ON(since >= sizeof(INTEL_INFO(p)->revision_mask) * BITS_PER_BYTE); \
>> + BUILD_BUG_ON(until >= sizeof(INTEL_INFO(p)->revision_mask) * BITS_PER_BYTE); \
>> + if ((__e) == REVID_FOREVER) \
>> + __e = BITS_PER_LONG - 1; \
>> + !!(INTEL_INFO(p)->revision_mask & (mask) & GENMASK((__e), (__s))); \
>> +})
>>
>> #define IS_I830(dev) (INTEL_DEVID(dev) == 0x3577)
>> #define IS_845G(dev) (INTEL_DEVID(dev) == 0x2562)
>> @@ -2605,14 +2617,16 @@ struct drm_i915_cmd_table {
>> #define SKL_REVID_E0 0x4
>> #define SKL_REVID_F0 0x5
>>
>> -#define IS_SKL_REVID(p, since, until) (IS_SKYLAKE(p) && IS_REVID(p, since, until))
>> +#define IS_SKL_REVID(p, since, until) IS_REVID(p, INTEL_INFO(p)->skl_rev_mask, \
>> + since, until)
>>
>> #define BXT_REVID_A0 0x0
>> #define BXT_REVID_A1 0x1
>> #define BXT_REVID_B0 0x3
>> #define BXT_REVID_C0 0x9
>>
>> -#define IS_BXT_REVID(p, since, until) (IS_BROXTON(p) && IS_REVID(p, since, until))
>> +#define IS_BXT_REVID(p, since, until) IS_REVID(p, INTEL_INFO(p)->bxt_rev_mask, \
>> + since, until)
>>
>> /*
>> * The genX designation typically refers to the render engine, so render
>
More information about the Intel-gfx
mailing list