| Message ID | 1462542231-7321-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > + device_info->gen_mask = 1 << device_info->gen; device_info->gen_mask = BIT(device_info->gen) for consistency; > + > 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. -Chris
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
On Fri, May 06, 2016 at 03:36:26PM +0100, Tvrtko Ursulin wrote: > > 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? i740 would be gen0 i810 is gen1 (which is an on-chipset version of i740 really) One day we may achieve a goal a completely unified driver if Daniel hasn't lost his i810 yet. -Chris
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); + device_info->gen_mask = 1 << device_info->gen; + 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; u8 ring_mask; /* Rings supported by the HW */ DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON); /* Register offsets for the various display pipes and transcoders */ @@ -2620,14 +2621,14 @@ struct drm_i915_cmd_table { * have their own (e.g. HAS_PCH_SPLIT for ILK+ display, IS_foo for particular * chips, etc.). */ -#define IS_GEN2(dev) (INTEL_INFO(dev)->gen == 2) -#define IS_GEN3(dev) (INTEL_INFO(dev)->gen == 3) -#define IS_GEN4(dev) (INTEL_INFO(dev)->gen == 4) -#define IS_GEN5(dev) (INTEL_INFO(dev)->gen == 5) -#define IS_GEN6(dev) (INTEL_INFO(dev)->gen == 6) -#define IS_GEN7(dev) (INTEL_INFO(dev)->gen == 7) -#define IS_GEN8(dev) (INTEL_INFO(dev)->gen == 8) -#define IS_GEN9(dev) (INTEL_INFO(dev)->gen == 9) +#define IS_GEN2(dev) (INTEL_INFO(dev)->gen_mask & BIT(2)) +#define IS_GEN3(dev) (INTEL_INFO(dev)->gen_mask & BIT(3)) +#define IS_GEN4(dev) (INTEL_INFO(dev)->gen_mask & BIT(4)) +#define IS_GEN5(dev) (INTEL_INFO(dev)->gen_mask & BIT(5)) +#define IS_GEN6(dev) (INTEL_INFO(dev)->gen_mask & BIT(6)) +#define IS_GEN7(dev) (INTEL_INFO(dev)->gen_mask & BIT(7)) +#define IS_GEN8(dev) (INTEL_INFO(dev)->gen_mask & BIT(8)) +#define IS_GEN9(dev) (INTEL_INFO(dev)->gen_mask & BIT(9)) #define RENDER_RING (1<<RCS) #define BSD_RING (1<<VCS)