[PATCH] drm/i915: Use bitmask for IS_REVID checking

[PATCH] drm/i915: Use bitmask for IS_REVID checking

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@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.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@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
-- 
1.9.1

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

✗ Ro.CI.BAT: warning for drm/i915: Use bitmask for IS_REVID checking

[Patchwork]

== Series Details ==

Series: drm/i915: Use bitmask for IS_REVID checking
URL   : https://patchwork.freedesktop.org/series/6972/
State : warning

== Summary ==

Series 6972v1 drm/i915: Use bitmask for IS_REVID checking
http://patchwork.freedesktop.org/api/1.0/series/6972/revisions/1/mbox

Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (ro-ivb2-i7-3770)
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (ro-bdw-i7-5557U)
Test kms_sink_crc_basic:
                pass       -> SKIP       (ro-skl-i7-6700hq)

ro-bdw-i5-5250u  total:219  pass:202  dwarn:4   dfail:0   fail:0   skip:13 
ro-bdw-i7-5557U  total:219  pass:205  dwarn:0   dfail:0   fail:0   skip:14 
ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-bsw-n3050     total:219  pass:175  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:218  pass:174  dwarn:0   dfail:0   fail:3   skip:41 
ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
ro-hsw-i7-4770r  total:219  pass:194  dwarn:0   dfail:0   fail:0   skip:25 
ro-ilk-i7-620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
ro-ilk1-i5-650   total:214  pass:152  dwarn:0   dfail:0   fail:1   skip:61 
ro-ivb-i7-3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
ro-ivb2-i7-3770  total:219  pass:186  dwarn:1   dfail:0   fail:0   skip:32 
ro-skl-i7-6700hq total:214  pass:189  dwarn:0   dfail:0   fail:0   skip:25 
ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 

Results at /archive/results/CI_IGT_test/RO_Patchwork_847/

2d4abf3 drm-intel-nightly: 2016y-05m-10d-09h-36m-54s UTC integration manifest
0a5a626 drm/i915: Use bitmask for IS_REVID checking

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

Re: [PATCH] drm/i915: Use bitmask for IS_REVID checking

[Jani Nikula]

On Tue, 10 May 2016, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@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.

BR,
Jani.

>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@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

-- 
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] drm/i915: Use bitmask for IS_REVID checking

[Tvrtko Ursulin]

Hi,

On 10/05/16 14:33, Jani Nikula wrote:
> On Tue, 10 May 2016, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Jani Nikula <jani.nikula@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
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Use bitmask for IS_REVID checking

[Jani Nikula]

On Tue, 10 May 2016, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> Hi,
>
> On 10/05/16 14:33, Jani Nikula wrote:
>> On Tue, 10 May 2016, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@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?

None of the ones we list and check for i.e. we should replace all of the
IS_SKL_REVID() uses with either 0 or IS_SKYLAKE(), and simplify from
there.

Maybe we should check the revid once on driver load, and whine about
early hardware. We've had such checks in the past on earlier
platforms. This might save our precious time, as we shouldn't be
debugging issues reported against pre-production hardware.

Ville here says dropping the checks is a good time to verify the
workarounds we do have in place are correct going forward.


BR,
Jani.



>
> Regards,
>
> Tvrtko.
>
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Jani Nikula <jani.nikula@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
>>

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

✗ Ro.CI.BAT: warning for drm/i915: Use bitmask for IS_REVID checking

[Patchwork]

== Series Details ==

Series: drm/i915: Use bitmask for IS_REVID checking
URL   : https://patchwork.freedesktop.org/series/6972/
State : warning

== Summary ==

Series 6972v1 drm/i915: Use bitmask for IS_REVID checking
http://patchwork.freedesktop.org/api/1.0/series/6972/revisions/1/mbox

Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (ro-ivb2-i7-3770)
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (ro-bdw-i7-5557U)
Test kms_sink_crc_basic:
                pass       -> SKIP       (ro-skl-i7-6700hq)

ro-bdw-i5-5250u  total:219  pass:202  dwarn:4   dfail:0   fail:0   skip:13 
ro-bdw-i7-5557U  total:219  pass:205  dwarn:0   dfail:0   fail:0   skip:14 
ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-bsw-n3050     total:219  pass:175  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:218  pass:174  dwarn:0   dfail:0   fail:3   skip:41 
ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
ro-hsw-i7-4770r  total:219  pass:194  dwarn:0   dfail:0   fail:0   skip:25 
ro-ilk-i7-620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
ro-ilk1-i5-650   total:214  pass:152  dwarn:0   dfail:0   fail:1   skip:61 
ro-ivb-i7-3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
ro-ivb2-i7-3770  total:219  pass:186  dwarn:1   dfail:0   fail:0   skip:32 
ro-skl-i7-6700hq total:214  pass:189  dwarn:0   dfail:0   fail:0   skip:25 
ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 

Results at /archive/results/CI_IGT_test/RO_Patchwork_847/

2d4abf3 drm-intel-nightly: 2016y-05m-10d-09h-36m-54s UTC integration manifest
0a5a626 drm/i915: Use bitmask for IS_REVID checking

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

[PATCH v2] drm/i915: Use bitmask for IS_REVID checking

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@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.

v2: Simplified, now saves ~880 bytes of text.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c |  8 ++++++++
 drivers/gpu/drm/i915/i915_drv.h | 21 ++++++++++++++++-----
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 46ac1da64a09..f2ad78d8296f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1072,6 +1072,14 @@ 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 (IS_SKYLAKE(dev_priv) && dev->pdev->revision <
+	    sizeof(device_info->skl_rev_mask) * BITS_PER_BYTE)
+		device_info->skl_rev_mask = BIT(dev->pdev->revision);
+	else if (IS_BROXTON(dev_priv) && dev->pdev->revision <
+	         sizeof(device_info->bxt_rev_mask) * BITS_PER_BYTE)
+		device_info->bxt_rev_mask = BIT(dev->pdev->revision);
+
 	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..18eee575b9ae 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -757,6 +757,8 @@ struct intel_csr {
 struct intel_device_info {
 	u32 display_mmio_offset;
 	u16 device_id;
+	u8 skl_rev_mask;
+	u8 bxt_rev_mask;
 	u8 num_pipes:3;
 	u8 num_sprites[I915_MAX_PIPES];
 	u8 gen;
@@ -2519,14 +2521,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)->mask) * BITS_PER_BYTE); \
+	BUILD_BUG_ON(until >= sizeof(INTEL_INFO(p)->mask) * BITS_PER_BYTE); \
+	if ((__e) == REVID_FOREVER) \
+		__e = BITS_PER_LONG - 1; \
+	!!(INTEL_INFO(p)->mask & GENMASK((__e), (__s))); \
+})
 
 #define IS_I830(dev)		(INTEL_DEVID(dev) == 0x3577)
 #define IS_845G(dev)		(INTEL_DEVID(dev) == 0x2562)
@@ -2605,14 +2616,14 @@ 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, 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, bxt_rev_mask, since, until)
 
 /*
  * The genX designation typically refers to the render engine, so render
-- 
1.9.1

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

Re: [PATCH v2] drm/i915: Use bitmask for IS_REVID checking

[Tvrtko Ursulin]

On 10/05/16 14:53, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@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.
>
> v2: Simplified, now saves ~880 bytes of text.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_dma.c |  8 ++++++++
>   drivers/gpu/drm/i915/i915_drv.h | 21 ++++++++++++++++-----
>   2 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 46ac1da64a09..f2ad78d8296f 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1072,6 +1072,14 @@ 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 (IS_SKYLAKE(dev_priv) && dev->pdev->revision <
> +	    sizeof(device_info->skl_rev_mask) * BITS_PER_BYTE)
> +		device_info->skl_rev_mask = BIT(dev->pdev->revision);
> +	else if (IS_BROXTON(dev_priv) && dev->pdev->revision <
> +	         sizeof(device_info->bxt_rev_mask) * BITS_PER_BYTE)
> +		device_info->bxt_rev_mask = BIT(dev->pdev->revision);
> +

No this is wrong, > /dev/null please, sorry for the noise. :)

Tvrtko

>   	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..18eee575b9ae 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -757,6 +757,8 @@ struct intel_csr {
>   struct intel_device_info {
>   	u32 display_mmio_offset;
>   	u16 device_id;
> +	u8 skl_rev_mask;
> +	u8 bxt_rev_mask;
>   	u8 num_pipes:3;
>   	u8 num_sprites[I915_MAX_PIPES];
>   	u8 gen;
> @@ -2519,14 +2521,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)->mask) * BITS_PER_BYTE); \
> +	BUILD_BUG_ON(until >= sizeof(INTEL_INFO(p)->mask) * BITS_PER_BYTE); \
> +	if ((__e) == REVID_FOREVER) \
> +		__e = BITS_PER_LONG - 1; \
> +	!!(INTEL_INFO(p)->mask & GENMASK((__e), (__s))); \
> +})
>
>   #define IS_I830(dev)		(INTEL_DEVID(dev) == 0x3577)
>   #define IS_845G(dev)		(INTEL_DEVID(dev) == 0x2562)
> @@ -2605,14 +2616,14 @@ 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, 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, bxt_rev_mask, since, until)
>
>   /*
>    * The genX designation typically refers to the render engine, so render
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: failure for drm/i915: Use bitmask for IS_REVID checking (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Use bitmask for IS_REVID checking (rev2)
URL   : https://patchwork.freedesktop.org/series/6972/
State : failure

== Summary ==

Series 6972v2 drm/i915: Use bitmask for IS_REVID checking
http://patchwork.freedesktop.org/api/1.0/series/6972/revisions/2/mbox

Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> FAIL       (ro-ilk1-i5-650)
                incomplete -> PASS       (fi-snb-i7-2600)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (ro-bdw-i7-5557U)
Test kms_sink_crc_basic:
                pass       -> SKIP       (ro-skl-i7-6700hq)

fi-bdw-i7-5557u  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
fi-byt-n2820     total:219  pass:175  dwarn:0   dfail:0   fail:3   skip:41 
fi-hsw-i7-4770k  total:219  pass:197  dwarn:0   dfail:0   fail:1   skip:21 
fi-skl-i5-6260u  total:219  pass:207  dwarn:0   dfail:0   fail:0   skip:12 
fi-skl-i7-6700k  total:219  pass:191  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-i7-2600   total:219  pass:176  dwarn:0   dfail:0   fail:0   skip:43 
ro-bdw-i5-5250u  total:219  pass:202  dwarn:4   dfail:0   fail:0   skip:13 
ro-bdw-i7-5557U  total:219  pass:205  dwarn:0   dfail:0   fail:0   skip:14 
ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-bsw-n3050     total:219  pass:174  dwarn:0   dfail:0   fail:3   skip:42 
ro-byt-n2820     total:218  pass:174  dwarn:0   dfail:0   fail:3   skip:41 
ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
ro-hsw-i7-4770r  total:219  pass:194  dwarn:0   dfail:0   fail:0   skip:25 
ro-ilk1-i5-650   total:214  pass:151  dwarn:0   dfail:0   fail:2   skip:61 
ro-ivb-i7-3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
ro-ivb2-i7-3770  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-skl-i7-6700hq total:214  pass:189  dwarn:0   dfail:0   fail:0   skip:25 
ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 
fi-hsw-i7-4770r failed to connect after reboot
fi-kbl-y failed to connect after reboot
ro-ilk-i7-620lm failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_849/

2d4abf3 drm-intel-nightly: 2016y-05m-10d-09h-36m-54s UTC integration manifest
4e1a637 drm/i915: Use bitmask for IS_REVID checking

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