diff mbox

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

Message ID 1462888436-10475-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tvrtko Ursulin May 10, 2016, 1:53 p.m. UTC 
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(-)

Comments

Tvrtko Ursulin May 10, 2016, 2:06 p.m. UTC | #1 
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
>
diff mbox

Patch

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