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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 10 May 2016, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux