* [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask
@ 2016-05-06 13:43 Tvrtko Ursulin
2016-05-06 13:43 ` [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro Tvrtko Ursulin
` (3 more replies)
0 siblings, 4 replies; 27+ messages in thread
From: Tvrtko Ursulin @ 2016-05-06 13:43 UTC (permalink / raw)
To: Intel-gfx
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);
+ 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)
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro
2016-05-06 13:43 [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask Tvrtko Ursulin
@ 2016-05-06 13:43 ` Tvrtko Ursulin
2016-05-06 14:00 ` Ville Syrjälä
2016-05-06 13:43 ` [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx Tvrtko Ursulin
` (2 subsequent siblings)
3 siblings, 1 reply; 27+ messages in thread
From: Tvrtko Ursulin @ 2016-05-06 13:43 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
If we allow it a dedicated flag in dev_priv we enable the
compiler to nicely optimize conditions like IS_HASSWELL ||
IS_BROADWELL.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 4 ++++
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9fd221c97275..da6532da44e3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
static const struct intel_device_info intel_broadwell_d_info = {
BDW_FEATURES,
.gen = 8,
+ .is_broadwell = 1,
};
static const struct intel_device_info intel_broadwell_m_info = {
BDW_FEATURES,
.gen = 8, .is_mobile = 1,
+ .is_broadwell = 1,
};
static const struct intel_device_info intel_broadwell_gt3d_info = {
BDW_FEATURES,
.gen = 8,
+ .is_broadwell = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
};
static const struct intel_device_info intel_broadwell_gt3m_info = {
BDW_FEATURES,
.gen = 8, .is_mobile = 1,
+ .is_broadwell = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
};
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c6351016eaf0..459561991081 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -735,6 +735,7 @@ struct intel_csr {
func(is_valleyview) sep \
func(is_cherryview) sep \
func(is_haswell) sep \
+ func(is_broadwell) sep \
func(is_skylake) sep \
func(is_broxton) sep \
func(is_kabylake) sep \
@@ -2553,7 +2554,7 @@ struct drm_i915_cmd_table {
#define IS_VALLEYVIEW(dev) (INTEL_INFO(dev)->is_valleyview)
#define IS_CHERRYVIEW(dev) (INTEL_INFO(dev)->is_cherryview)
#define IS_HASWELL(dev) (INTEL_INFO(dev)->is_haswell)
-#define IS_BROADWELL(dev) (!INTEL_INFO(dev)->is_cherryview && IS_GEN8(dev))
+#define IS_BROADWELL(dev) (INTEL_INFO(dev)->is_broadwell)
#define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
#define IS_BROXTON(dev) (INTEL_INFO(dev)->is_broxton)
#define IS_KABYLAKE(dev) (INTEL_INFO(dev)->is_kabylake)
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx
2016-05-06 13:43 [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask Tvrtko Ursulin
2016-05-06 13:43 ` [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro Tvrtko Ursulin
@ 2016-05-06 13:43 ` Tvrtko Ursulin
2016-05-06 13:59 ` Chris Wilson
` (2 more replies)
2016-05-06 14:16 ` [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro Tvrtko Ursulin
2016-05-06 14:28 ` [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask Chris Wilson
3 siblings, 3 replies; 27+ messages in thread
From: Tvrtko Ursulin @ 2016-05-06 13:43 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
This way optimization from a previous patch works even better.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
drivers/gpu/drm/i915/i915_dma.c | 4 ++--
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
drivers/gpu/drm/i915/i915_gem_stolen.c | 2 +-
drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +-
drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
drivers/gpu/drm/i915/i915_irq.c | 4 ++--
drivers/gpu/drm/i915/intel_display.c | 2 +-
drivers/gpu/drm/i915/intel_lrc.c | 4 ++--
drivers/gpu/drm/i915/intel_lvds.c | 2 +-
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++---
15 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 6ad008c196b5..e264f168ef02 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2310,12 +2310,12 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
- if (INTEL_INFO(dev)->gen == 6)
+ if (IS_GEN6(dev_priv))
seq_printf(m, "GFX_MODE: 0x%08x\n", I915_READ(GFX_MODE));
for_each_engine(engine, dev_priv) {
seq_printf(m, "%s\n", engine->name);
- if (INTEL_INFO(dev)->gen == 7)
+ if (IS_GEN7(dev_priv))
seq_printf(m, "GFX_MODE: 0x%08x\n",
I915_READ(RING_MODE_GEN7(engine)));
seq_printf(m, "PP_DIR_BASE: 0x%08x\n",
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 01163da51b1d..a65c85c8317c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -888,7 +888,7 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
DRM_INFO("Display disabled (module parameter)\n");
info->num_pipes = 0;
} else if (info->num_pipes > 0 &&
- (INTEL_INFO(dev)->gen == 7 || INTEL_INFO(dev)->gen == 8) &&
+ (IS_GEN7(dev_priv) || IS_GEN8(dev_priv)) &&
HAS_PCH_SPLIT(dev)) {
u32 fuse_strap = I915_READ(FUSE_STRAP);
u32 sfuse_strap = I915_READ(SFUSE_STRAP);
@@ -912,7 +912,7 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
DRM_INFO("PipeC fused off\n");
info->num_pipes -= 1;
}
- } else if (info->num_pipes > 0 && INTEL_INFO(dev)->gen == 9) {
+ } else if (info->num_pipes > 0 && IS_GEN9(dev_priv)) {
u32 dfsm = I915_READ(SKL_DFSM);
u8 disabled_mask = 0;
bool invalid;
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index da6532da44e3..f27cc15b6f8e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -548,7 +548,7 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
#ifdef CONFIG_INTEL_IOMMU
/* Enable semaphores on SNB when IO remapping is off */
- if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
+ if (IS_GEN6(dev) && intel_iommu_gfx_mapped)
return false;
#endif
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 459561991081..15fcbcece13c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2702,7 +2702,7 @@ struct drm_i915_cmd_table {
IS_CHERRYVIEW(dev) || IS_SKYLAKE(dev) || \
IS_KABYLAKE(dev) || IS_BROXTON(dev))
#define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6)
-#define HAS_RC6p(dev) (INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev))
+#define HAS_RC6p(dev) (IS_GEN6(dev) || IS_IVYBRIDGE(dev))
#define HAS_CSR(dev) (IS_GEN9(dev))
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a88e6c9e9516..f2e42c0cc5be 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1982,7 +1982,7 @@ i915_gem_get_gtt_size(struct drm_device *dev, uint32_t size, int tiling_mode)
return size;
/* Previous chips need a power-of-two fence region when tiling */
- if (INTEL_INFO(dev)->gen == 3)
+ if (IS_GEN3(dev))
gtt_size = 1024*1024;
else
gtt_size = 512*1024;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d284b17af431..61e85c2b500d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -144,7 +144,7 @@ int intel_sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
#ifdef CONFIG_INTEL_IOMMU
/* Disable ppgtt on SNB if VT-d is on. */
- if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped) {
+ if (IS_GEN6(dev) && intel_iommu_gfx_mapped) {
DRM_INFO("Disabling PPGTT because VT-d is on\n");
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 68fde8fba803..f9253f2b7ba0 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -56,7 +56,7 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
/* See the comment at the drm_mm_init() call for more about this check.
* WaSkipStolenMemoryFirstPage:bdw,chv (incomplete) */
- if (INTEL_INFO(dev_priv)->gen == 8 && start < 4096)
+ if (IS_GEN8(dev_priv) && start < 4096)
start = 4096;
mutex_lock(&dev_priv->mm.stolen_lock);
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 829dab69895f..2fcb4afade19 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -125,7 +125,7 @@ i915_gem_object_fence_ok(struct drm_i915_gem_object *obj, int tiling_mode)
if (INTEL_INFO(obj->base.dev)->gen >= 4)
return true;
- if (INTEL_INFO(obj->base.dev)->gen == 3) {
+ if (IS_GEN3(obj->base.dev)) {
if (i915_gem_obj_ggtt_offset(obj) & ~I915_FENCE_START_MASK)
return false;
} else {
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 89725c9efc25..12a749c9707a 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -411,7 +411,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
err_printf(m, "DONE_REG: 0x%08x\n", error->done_reg);
}
- if (INTEL_INFO(dev)->gen == 7)
+ if (IS_GEN7(dev))
err_printf(m, "ERR_INT: 0x%08x\n", error->err_int);
for (i = 0; i < ARRAY_SIZE(error->ring); i++)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2f6fd33c07ba..ba84b9b17fc2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4674,12 +4674,12 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
dev->driver->disable_vblank = ironlake_disable_vblank;
dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
} else {
- if (INTEL_INFO(dev_priv)->gen == 2) {
+ if (IS_GEN2(dev_priv)) {
dev->driver->irq_preinstall = i8xx_irq_preinstall;
dev->driver->irq_postinstall = i8xx_irq_postinstall;
dev->driver->irq_handler = i8xx_irq_handler;
dev->driver->irq_uninstall = i8xx_irq_uninstall;
- } else if (INTEL_INFO(dev_priv)->gen == 3) {
+ } else if (IS_GEN3(dev_priv)) {
dev->driver->irq_preinstall = i915_irq_preinstall;
dev->driver->irq_postinstall = i915_irq_postinstall;
dev->driver->irq_uninstall = i915_irq_uninstall;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 45c218db86be..1385423ac960 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1204,7 +1204,7 @@ static void assert_fdi_tx_pll_enabled(struct drm_i915_private *dev_priv,
u32 val;
/* ILK FDI PLL is always enabled */
- if (INTEL_INFO(dev_priv)->gen == 5)
+ if (IS_GEN5(dev_priv))
return;
/* On Haswell, DDI ports are responsible for the FDI PLL setup */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d8763524319d..f614a07a07bb 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1387,7 +1387,7 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
batch = kmap_atomic(page);
offset = 0;
- if (INTEL_INFO(engine->dev)->gen == 8) {
+ if (IS_GEN8(engine->dev)) {
ret = gen8_init_indirectctx_bb(engine,
&wa_ctx->indirect_ctx,
batch,
@@ -1401,7 +1401,7 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
&offset);
if (ret)
goto out;
- } else if (INTEL_INFO(engine->dev)->gen == 9) {
+ } else if (IS_GEN9(engine->dev)) {
ret = gen9_init_indirectctx_bb(engine,
&wa_ctx->indirect_ctx,
batch,
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index bc53c0dd34d0..d65fd945607a 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -190,7 +190,7 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
/* Set the dithering flag on LVDS as needed, note that there is no
* special lvds dither control bit on pch-split platforms, dithering is
* only controlled through the PIPECONF reg. */
- if (INTEL_INFO(dev)->gen == 4) {
+ if (IS_GEN4(dev_priv)) {
/* Bspec wording suggests that LVDS port dithering only exists
* for 18bpp panels. */
if (crtc->config->dither && crtc->config->pipe_bpp == 18)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 017c431f9363..5f17f682d37b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2146,14 +2146,14 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[8])
static void intel_fixup_spr_wm_latency(struct drm_device *dev, uint16_t wm[5])
{
/* ILK sprite LP0 latency is 1300 ns */
- if (INTEL_INFO(dev)->gen == 5)
+ if (IS_GEN5(dev))
wm[0] = 13;
}
static void intel_fixup_cur_wm_latency(struct drm_device *dev, uint16_t wm[5])
{
/* ILK cursor LP0 latency is 1300 ns */
- if (INTEL_INFO(dev)->gen == 5)
+ if (IS_GEN5(dev))
wm[0] = 13;
/* WaDoubleCursorLP3Latency:ivb */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8f3eb3033da0..710cd598c701 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1236,7 +1236,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
/* Required for the hardware to program scanline values for waiting */
/* WaEnableFlushTlbInvalidationMode:snb */
- if (INTEL_INFO(dev)->gen == 6)
+ if (IS_GEN6(dev_priv))
I915_WRITE(GFX_MODE,
_MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
@@ -2536,7 +2536,7 @@ void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno)
* the semaphore value, then when the seqno moves backwards all
* future waits will complete instantly (causing rendering corruption).
*/
- if (INTEL_INFO(dev_priv)->gen == 6 || INTEL_INFO(dev_priv)->gen == 7) {
+ if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) {
I915_WRITE(RING_SYNC_0(engine->mmio_base), 0);
I915_WRITE(RING_SYNC_1(engine->mmio_base), 0);
if (HAS_VEBOX(dev_priv))
@@ -2808,7 +2808,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
engine->init_context = intel_rcs_ctx_init;
engine->add_request = gen6_add_request;
engine->flush = gen7_render_ring_flush;
- if (INTEL_INFO(dev)->gen == 6)
+ if (IS_GEN6(dev_priv))
engine->flush = gen6_render_ring_flush;
engine->irq_get = gen6_ring_get_irq;
engine->irq_put = gen6_ring_put_irq;
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx
2016-05-06 13:43 ` [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx Tvrtko Ursulin
@ 2016-05-06 13:59 ` Chris Wilson
2016-05-06 14:16 ` Tvrtko Ursulin
2016-05-09 14:18 ` Dave Gordon
2016-05-10 7:47 ` Jani Nikula
2 siblings, 1 reply; 27+ messages in thread
From: Chris Wilson @ 2016-05-06 13:59 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Fri, May 06, 2016 at 02:43:51PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> This way optimization from a previous patch works even better.
Impact on object size? The hope is that gcc is able to combine tests so
reduce instruction count.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro
2016-05-06 13:43 ` [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro Tvrtko Ursulin
@ 2016-05-06 14:00 ` Ville Syrjälä
2016-05-06 14:18 ` Tvrtko Ursulin
0 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjälä @ 2016-05-06 14:00 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> If we allow it a dedicated flag in dev_priv we enable the
> compiler to nicely optimize conditions like IS_HASSWELL ||
> IS_BROADWELL.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 4 ++++
> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9fd221c97275..da6532da44e3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
> static const struct intel_device_info intel_broadwell_d_info = {
> BDW_FEATURES,
> .gen = 8,
> + .is_broadwell = 1,
> };
>
> static const struct intel_device_info intel_broadwell_m_info = {
> BDW_FEATURES,
> .gen = 8, .is_mobile = 1,
> + .is_broadwell = 1,
> };
>
> static const struct intel_device_info intel_broadwell_gt3d_info = {
> BDW_FEATURES,
> .gen = 8,
> + .is_broadwell = 1,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> };
>
> static const struct intel_device_info intel_broadwell_gt3m_info = {
> BDW_FEATURES,
> .gen = 8, .is_mobile = 1,
> + .is_broadwell = 1,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> };
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c6351016eaf0..459561991081 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -735,6 +735,7 @@ struct intel_csr {
> func(is_valleyview) sep \
> func(is_cherryview) sep \
> func(is_haswell) sep \
> + func(is_broadwell) sep \
> func(is_skylake) sep \
> func(is_broxton) sep \
> func(is_kabylake) sep \
We could just replace all of these with a single enum and save a bunch
of space.
> @@ -2553,7 +2554,7 @@ struct drm_i915_cmd_table {
> #define IS_VALLEYVIEW(dev) (INTEL_INFO(dev)->is_valleyview)
> #define IS_CHERRYVIEW(dev) (INTEL_INFO(dev)->is_cherryview)
> #define IS_HASWELL(dev) (INTEL_INFO(dev)->is_haswell)
> -#define IS_BROADWELL(dev) (!INTEL_INFO(dev)->is_cherryview && IS_GEN8(dev))
> +#define IS_BROADWELL(dev) (INTEL_INFO(dev)->is_broadwell)
> #define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
> #define IS_BROXTON(dev) (INTEL_INFO(dev)->is_broxton)
> #define IS_KABYLAKE(dev) (INTEL_INFO(dev)->is_kabylake)
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx
2016-05-06 13:59 ` Chris Wilson
@ 2016-05-06 14:16 ` Tvrtko Ursulin
2016-05-06 14:20 ` Tvrtko Ursulin
0 siblings, 1 reply; 27+ messages in thread
From: Tvrtko Ursulin @ 2016-05-06 14:16 UTC (permalink / raw)
To: Chris Wilson, Intel-gfx
On 06/05/16 14:59, Chris Wilson wrote:
> On Fri, May 06, 2016 at 02:43:51PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> This way optimization from a previous patch works even better.
>
> Impact on object size? The hope is that gcc is able to combine tests so
> reduce instruction count.
Every patch in this series shrinks the object size.
With the bitmask approach it is able to combine tests, previously it wasn't.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro
2016-05-06 13:43 [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask Tvrtko Ursulin
2016-05-06 13:43 ` [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro Tvrtko Ursulin
2016-05-06 13:43 ` [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx Tvrtko Ursulin
@ 2016-05-06 14:16 ` Tvrtko Ursulin
2016-05-06 14:16 ` [PATCH 2/2] drm/i915: Do not use a bitfield for INTEL_INFO->num_pipes Tvrtko Ursulin
2016-05-06 14:33 ` [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro Chris Wilson
2016-05-06 14:28 ` [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask Chris Wilson
3 siblings, 2 replies; 27+ messages in thread
From: Tvrtko Ursulin @ 2016-05-06 14:16 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
To be used for more efficient Gen range checking.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_fbc.c | 2 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 15fcbcece13c..935e381407ba 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
})
#define INTEL_INFO(p) (&__I915__(p)->info)
#define INTEL_GEN(p) (INTEL_INFO(p)->gen)
+#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
#define INTEL_DEVID(p) (INTEL_INFO(p)->device_id)
#define INTEL_REVID(p) (__I915__(p)->dev->pdev->revision)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index d5a7cfec589b..2c3681757aba 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
/* FIXME: We lack the proper locking here, so only run this on the
* platforms that need. */
- if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
+ if (INTEL_GEN_RANGE(dev_priv, 5, 6))
cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
cache->fb.pixel_format = fb->pixel_format;
cache->fb.stride = fb->pitches[0];
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 710cd598c701..c3dbb74d048b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -508,7 +508,7 @@ static void intel_ring_setup_status_page(struct intel_engine_cs *engine)
* arises: do we still need this and if so how should we go about
* invalidating the TLB?
*/
- if (INTEL_INFO(dev)->gen >= 6 && INTEL_INFO(dev)->gen < 8) {
+ if (INTEL_GEN_RANGE(dev_priv, 6, 7)) {
i915_reg_t reg = RING_INSTPM(engine->mmio_base);
/* ring should be idle before issuing a sync flush*/
@@ -1222,7 +1222,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
return ret;
/* WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb */
- if (INTEL_INFO(dev)->gen >= 4 && INTEL_INFO(dev)->gen < 7)
+ if (INTEL_GEN_RANGE(dev_priv, 4, 6))
I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(VS_TIMER_DISPATCH));
/* We need to disable the AsyncFlip performance optimisations in order
@@ -1231,7 +1231,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
*
* WaDisableAsyncFlipPerfMode:snb,ivb,hsw,vlv
*/
- if (INTEL_INFO(dev)->gen >= 6 && INTEL_INFO(dev)->gen < 8)
+ if (INTEL_GEN_RANGE(dev_priv, 6, 7))
I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(ASYNC_FLIP_PERF_DISABLE));
/* Required for the hardware to program scanline values for waiting */
@@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
_MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
/* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
- if (IS_GEN7(dev))
+ if (IS_GEN7(dev_priv))
I915_WRITE(GFX_MODE_GEN7,
_MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
_MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
- if (IS_GEN6(dev)) {
+ if (IS_GEN6(dev_priv)) {
/* From the Sandybridge PRM, volume 1 part 3, page 24:
* "If this bit is set, STCunit will have LRA as replacement
* policy. [...] This bit must be reset. LRA replacement
@@ -1256,7 +1256,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
_MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
}
- if (INTEL_INFO(dev)->gen >= 6 && INTEL_INFO(dev)->gen < 8)
+ if (INTEL_GEN_RANGE(dev_priv, 6, 7))
I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING));
if (HAS_L3_DPF(dev))
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/2] drm/i915: Do not use a bitfield for INTEL_INFO->num_pipes
2016-05-06 14:16 ` [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro Tvrtko Ursulin
@ 2016-05-06 14:16 ` Tvrtko Ursulin
2016-05-06 19:21 ` Dave Gordon
2016-05-06 14:33 ` [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro Chris Wilson
1 sibling, 1 reply; 27+ messages in thread
From: Tvrtko Ursulin @ 2016-05-06 14:16 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
It just makes more work for the compiler and generates more code.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 935e381407ba..f7a156b8d5a1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -758,7 +758,7 @@ struct intel_csr {
struct intel_device_info {
u32 display_mmio_offset;
u16 device_id;
- u8 num_pipes:3;
+ u8 num_pipes;
u8 num_sprites[I915_MAX_PIPES];
u8 gen;
u16 gen_mask;
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro
2016-05-06 14:00 ` Ville Syrjälä
@ 2016-05-06 14:18 ` Tvrtko Ursulin
2016-05-06 14:29 ` Ville Syrjälä
0 siblings, 1 reply; 27+ messages in thread
From: Tvrtko Ursulin @ 2016-05-06 14:18 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel-gfx
On 06/05/16 15:00, Ville Syrjälä wrote:
> On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> If we allow it a dedicated flag in dev_priv we enable the
>> compiler to nicely optimize conditions like IS_HASSWELL ||
>> IS_BROADWELL.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 4 ++++
>> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 9fd221c97275..da6532da44e3 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
>> static const struct intel_device_info intel_broadwell_d_info = {
>> BDW_FEATURES,
>> .gen = 8,
>> + .is_broadwell = 1,
>> };
>>
>> static const struct intel_device_info intel_broadwell_m_info = {
>> BDW_FEATURES,
>> .gen = 8, .is_mobile = 1,
>> + .is_broadwell = 1,
>> };
>>
>> static const struct intel_device_info intel_broadwell_gt3d_info = {
>> BDW_FEATURES,
>> .gen = 8,
>> + .is_broadwell = 1,
>> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>> };
>>
>> static const struct intel_device_info intel_broadwell_gt3m_info = {
>> BDW_FEATURES,
>> .gen = 8, .is_mobile = 1,
>> + .is_broadwell = 1,
>> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>> };
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index c6351016eaf0..459561991081 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -735,6 +735,7 @@ struct intel_csr {
>> func(is_valleyview) sep \
>> func(is_cherryview) sep \
>> func(is_haswell) sep \
>> + func(is_broadwell) sep \
>> func(is_skylake) sep \
>> func(is_broxton) sep \
>> func(is_kabylake) sep \
>
> We could just replace all of these with a single enum and save a bunch
> of space.
Bitfield is really nice here because compiler can then merge tests like
IS_HASWELL || IS_BROADWELL into a single test instruction.
Regards,
Tvrtko
>
>> @@ -2553,7 +2554,7 @@ struct drm_i915_cmd_table {
>> #define IS_VALLEYVIEW(dev) (INTEL_INFO(dev)->is_valleyview)
>> #define IS_CHERRYVIEW(dev) (INTEL_INFO(dev)->is_cherryview)
>> #define IS_HASWELL(dev) (INTEL_INFO(dev)->is_haswell)
>> -#define IS_BROADWELL(dev) (!INTEL_INFO(dev)->is_cherryview && IS_GEN8(dev))
>> +#define IS_BROADWELL(dev) (INTEL_INFO(dev)->is_broadwell)
>> #define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
>> #define IS_BROXTON(dev) (INTEL_INFO(dev)->is_broxton)
>> #define IS_KABYLAKE(dev) (INTEL_INFO(dev)->is_kabylake)
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx
2016-05-06 14:16 ` Tvrtko Ursulin
@ 2016-05-06 14:20 ` Tvrtko Ursulin
0 siblings, 0 replies; 27+ messages in thread
From: Tvrtko Ursulin @ 2016-05-06 14:20 UTC (permalink / raw)
To: Chris Wilson, Intel-gfx
On 06/05/16 15:16, Tvrtko Ursulin wrote:
>
> On 06/05/16 14:59, Chris Wilson wrote:
>> On Fri, May 06, 2016 at 02:43:51PM +0100, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> This way optimization from a previous patch works even better.
>>
>> Impact on object size? The hope is that gcc is able to combine tests so
>> reduce instruction count.
>
> Every patch in this series shrinks the object size.
>
> With the bitmask approach it is able to combine tests, previously it
> wasn't.
To expand, ~1.5k savings after the five patches in this series.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask
2016-05-06 13:43 [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask Tvrtko Ursulin
` (2 preceding siblings ...)
2016-05-06 14:16 ` [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro Tvrtko Ursulin
@ 2016-05-06 14:28 ` Chris Wilson
2016-05-06 14:36 ` Tvrtko Ursulin
3 siblings, 1 reply; 27+ messages in thread
From: Chris Wilson @ 2016-05-06 14:28 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
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
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro
2016-05-06 14:18 ` Tvrtko Ursulin
@ 2016-05-06 14:29 ` Ville Syrjälä
2016-05-06 14:44 ` Tvrtko Ursulin
0 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjälä @ 2016-05-06 14:29 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Fri, May 06, 2016 at 03:18:16PM +0100, Tvrtko Ursulin wrote:
>
> On 06/05/16 15:00, Ville Syrjälä wrote:
> > On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> If we allow it a dedicated flag in dev_priv we enable the
> >> compiler to nicely optimize conditions like IS_HASSWELL ||
> >> IS_BROADWELL.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.c | 4 ++++
> >> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> >> 2 files changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> index 9fd221c97275..da6532da44e3 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
> >> static const struct intel_device_info intel_broadwell_d_info = {
> >> BDW_FEATURES,
> >> .gen = 8,
> >> + .is_broadwell = 1,
> >> };
> >>
> >> static const struct intel_device_info intel_broadwell_m_info = {
> >> BDW_FEATURES,
> >> .gen = 8, .is_mobile = 1,
> >> + .is_broadwell = 1,
> >> };
> >>
> >> static const struct intel_device_info intel_broadwell_gt3d_info = {
> >> BDW_FEATURES,
> >> .gen = 8,
> >> + .is_broadwell = 1,
> >> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >> };
> >>
> >> static const struct intel_device_info intel_broadwell_gt3m_info = {
> >> BDW_FEATURES,
> >> .gen = 8, .is_mobile = 1,
> >> + .is_broadwell = 1,
> >> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >> };
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index c6351016eaf0..459561991081 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -735,6 +735,7 @@ struct intel_csr {
> >> func(is_valleyview) sep \
> >> func(is_cherryview) sep \
> >> func(is_haswell) sep \
> >> + func(is_broadwell) sep \
> >> func(is_skylake) sep \
> >> func(is_broxton) sep \
> >> func(is_kabylake) sep \
> >
> > We could just replace all of these with a single enum and save a bunch
> > of space.
>
> Bitfield is really nice here because compiler can then merge tests like
> IS_HASWELL || IS_BROADWELL into a single test instruction.
Most if not all of those are in modeset code so I'm not entirely
convinced micro-optimizing that is all that useful.
Bunch of times I've just wished I could write 'whatever >= G4X' instead
of 'IS_G4X || gen >= 5' simply for readability.
>
> Regards,
>
> Tvrtko
>
> >
> >> @@ -2553,7 +2554,7 @@ struct drm_i915_cmd_table {
> >> #define IS_VALLEYVIEW(dev) (INTEL_INFO(dev)->is_valleyview)
> >> #define IS_CHERRYVIEW(dev) (INTEL_INFO(dev)->is_cherryview)
> >> #define IS_HASWELL(dev) (INTEL_INFO(dev)->is_haswell)
> >> -#define IS_BROADWELL(dev) (!INTEL_INFO(dev)->is_cherryview && IS_GEN8(dev))
> >> +#define IS_BROADWELL(dev) (INTEL_INFO(dev)->is_broadwell)
> >> #define IS_SKYLAKE(dev) (INTEL_INFO(dev)->is_skylake)
> >> #define IS_BROXTON(dev) (INTEL_INFO(dev)->is_broxton)
> >> #define IS_KABYLAKE(dev) (INTEL_INFO(dev)->is_kabylake)
> >> --
> >> 1.9.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro
2016-05-06 14:16 ` [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro Tvrtko Ursulin
2016-05-06 14:16 ` [PATCH 2/2] drm/i915: Do not use a bitfield for INTEL_INFO->num_pipes Tvrtko Ursulin
@ 2016-05-06 14:33 ` Chris Wilson
2016-05-06 19:11 ` Dave Gordon
1 sibling, 1 reply; 27+ messages in thread
From: Chris Wilson @ 2016-05-06 14:33 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Fri, May 06, 2016 at 03:16:25PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> To be used for more efficient Gen range checking.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_fbc.c | 2 +-
> drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 15fcbcece13c..935e381407ba 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
> })
> #define INTEL_INFO(p) (&__I915__(p)->info)
> #define INTEL_GEN(p) (INTEL_INFO(p)->gen)
> +#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
> #define INTEL_DEVID(p) (INTEL_INFO(p)->device_id)
> #define INTEL_REVID(p) (__I915__(p)->dev->pdev->revision)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index d5a7cfec589b..2c3681757aba 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
>
> /* FIXME: We lack the proper locking here, so only run this on the
> * platforms that need. */
> - if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
> + if (INTEL_GEN_RANGE(dev_priv, 5, 6))
> cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
> cache->fb.pixel_format = fb->pixel_format;
> cache->fb.stride = fb->pitches[0];
> @@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>
> /* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
> - if (IS_GEN7(dev))
> + if (IS_GEN7(dev_priv))
> I915_WRITE(GFX_MODE_GEN7,
> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
> _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
>
> - if (IS_GEN6(dev)) {
> + if (IS_GEN6(dev_priv)) {
This chunk shouldn't be in this patch.
Couldn't improve upon INTEL_GEN_RANGE.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask
2016-05-06 14:28 ` [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask Chris Wilson
@ 2016-05-06 14:36 ` Tvrtko Ursulin
2016-05-06 14:43 ` Chris Wilson
0 siblings, 1 reply; 27+ messages in thread
From: Tvrtko Ursulin @ 2016-05-06 14:36 UTC (permalink / raw)
To: Chris Wilson, Intel-gfx
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask
2016-05-06 14:36 ` Tvrtko Ursulin
@ 2016-05-06 14:43 ` Chris Wilson
0 siblings, 0 replies; 27+ messages in thread
From: Chris Wilson @ 2016-05-06 14:43 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
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
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro
2016-05-06 14:29 ` Ville Syrjälä
@ 2016-05-06 14:44 ` Tvrtko Ursulin
2016-05-06 14:51 ` Chris Wilson
2016-05-06 14:58 ` Ville Syrjälä
0 siblings, 2 replies; 27+ messages in thread
From: Tvrtko Ursulin @ 2016-05-06 14:44 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel-gfx
On 06/05/16 15:29, Ville Syrjälä wrote:
> On Fri, May 06, 2016 at 03:18:16PM +0100, Tvrtko Ursulin wrote:
>>
>> On 06/05/16 15:00, Ville Syrjälä wrote:
>>> On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> If we allow it a dedicated flag in dev_priv we enable the
>>>> compiler to nicely optimize conditions like IS_HASSWELL ||
>>>> IS_BROADWELL.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/i915_drv.c | 4 ++++
>>>> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
>>>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>>>> index 9fd221c97275..da6532da44e3 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.c
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>>>> @@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
>>>> static const struct intel_device_info intel_broadwell_d_info = {
>>>> BDW_FEATURES,
>>>> .gen = 8,
>>>> + .is_broadwell = 1,
>>>> };
>>>>
>>>> static const struct intel_device_info intel_broadwell_m_info = {
>>>> BDW_FEATURES,
>>>> .gen = 8, .is_mobile = 1,
>>>> + .is_broadwell = 1,
>>>> };
>>>>
>>>> static const struct intel_device_info intel_broadwell_gt3d_info = {
>>>> BDW_FEATURES,
>>>> .gen = 8,
>>>> + .is_broadwell = 1,
>>>> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>>>> };
>>>>
>>>> static const struct intel_device_info intel_broadwell_gt3m_info = {
>>>> BDW_FEATURES,
>>>> .gen = 8, .is_mobile = 1,
>>>> + .is_broadwell = 1,
>>>> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>>>> };
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>> index c6351016eaf0..459561991081 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>> @@ -735,6 +735,7 @@ struct intel_csr {
>>>> func(is_valleyview) sep \
>>>> func(is_cherryview) sep \
>>>> func(is_haswell) sep \
>>>> + func(is_broadwell) sep \
>>>> func(is_skylake) sep \
>>>> func(is_broxton) sep \
>>>> func(is_kabylake) sep \
>>>
>>> We could just replace all of these with a single enum and save a bunch
>>> of space.
>>
>> Bitfield is really nice here because compiler can then merge tests like
>> IS_HASWELL || IS_BROADWELL into a single test instruction.
>
> Most if not all of those are in modeset code so I'm not entirely
> convinced micro-optimizing that is all that useful.
Maybe not, but IS_BROADWELL was just tasteless and it is better to save
instructions across everything than a bit in dev_priv.
> Bunch of times I've just wished I could write 'whatever >= G4X' instead
> of 'IS_G4X || gen >= 5' simply for readability.
I don't even know what G4X is so can't really follow what you mean. Is
it like Gen4.5, or Gen5-- ? So I can't figure out what kind of scheme
you propose.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro
2016-05-06 14:44 ` Tvrtko Ursulin
@ 2016-05-06 14:51 ` Chris Wilson
2016-05-06 14:58 ` Ville Syrjälä
1 sibling, 0 replies; 27+ messages in thread
From: Chris Wilson @ 2016-05-06 14:51 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Fri, May 06, 2016 at 03:44:05PM +0100, Tvrtko Ursulin wrote:
>
> On 06/05/16 15:29, Ville Syrjälä wrote:
> >On Fri, May 06, 2016 at 03:18:16PM +0100, Tvrtko Ursulin wrote:
> >>
> >>On 06/05/16 15:00, Ville Syrjälä wrote:
> >>>On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
> >>>>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>>If we allow it a dedicated flag in dev_priv we enable the
> >>>>compiler to nicely optimize conditions like IS_HASSWELL ||
> >>>>IS_BROADWELL.
> >>>>
> >>>>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>---
> >>>> drivers/gpu/drm/i915/i915_drv.c | 4 ++++
> >>>> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> >>>> 2 files changed, 6 insertions(+), 1 deletion(-)
> >>>>
> >>>>diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >>>>index 9fd221c97275..da6532da44e3 100644
> >>>>--- a/drivers/gpu/drm/i915/i915_drv.c
> >>>>+++ b/drivers/gpu/drm/i915/i915_drv.c
> >>>>@@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
> >>>> static const struct intel_device_info intel_broadwell_d_info = {
> >>>> BDW_FEATURES,
> >>>> .gen = 8,
> >>>>+ .is_broadwell = 1,
> >>>> };
> >>>>
> >>>> static const struct intel_device_info intel_broadwell_m_info = {
> >>>> BDW_FEATURES,
> >>>> .gen = 8, .is_mobile = 1,
> >>>>+ .is_broadwell = 1,
> >>>> };
> >>>>
> >>>> static const struct intel_device_info intel_broadwell_gt3d_info = {
> >>>> BDW_FEATURES,
> >>>> .gen = 8,
> >>>>+ .is_broadwell = 1,
> >>>> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >>>> };
> >>>>
> >>>> static const struct intel_device_info intel_broadwell_gt3m_info = {
> >>>> BDW_FEATURES,
> >>>> .gen = 8, .is_mobile = 1,
> >>>>+ .is_broadwell = 1,
> >>>> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >>>> };
> >>>>
> >>>>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>>>index c6351016eaf0..459561991081 100644
> >>>>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>>>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>>>@@ -735,6 +735,7 @@ struct intel_csr {
> >>>> func(is_valleyview) sep \
> >>>> func(is_cherryview) sep \
> >>>> func(is_haswell) sep \
> >>>>+ func(is_broadwell) sep \
> >>>> func(is_skylake) sep \
> >>>> func(is_broxton) sep \
> >>>> func(is_kabylake) sep \
> >>>
> >>>We could just replace all of these with a single enum and save a bunch
> >>>of space.
> >>
> >>Bitfield is really nice here because compiler can then merge tests like
> >>IS_HASWELL || IS_BROADWELL into a single test instruction.
> >
> >Most if not all of those are in modeset code so I'm not entirely
> >convinced micro-optimizing that is all that useful.
>
> Maybe not, but IS_BROADWELL was just tasteless and it is better to
> save instructions across everything than a bit in dev_priv.
>
> >Bunch of times I've just wished I could write 'whatever >= G4X' instead
> >of 'IS_G4X || gen >= 5' simply for readability.
>
> I don't even know what G4X is so can't really follow what you mean.
> Is it like Gen4.5, or Gen5-- ? So I can't figure out what kind of
> scheme you propose.
It's gen4.5. A quick refresh of the "eccentric" gen4.
The problem we always run into with a single gen field is that the GPU,
display engine and various bits and pieces have had a divergenet
evolution, e.g. Valleyview has a gen7 gpu with a gen4 display block.
We keep planning to separate the two, and that is mostly done today
through the distinction of IS_GENx() and IS_BROADWELL() etc, and we are
often better served by adding a specific feature flag rather than depend
upon a version field.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro
2016-05-06 14:44 ` Tvrtko Ursulin
2016-05-06 14:51 ` Chris Wilson
@ 2016-05-06 14:58 ` Ville Syrjälä
1 sibling, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2016-05-06 14:58 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Fri, May 06, 2016 at 03:44:05PM +0100, Tvrtko Ursulin wrote:
>
> On 06/05/16 15:29, Ville Syrjälä wrote:
> > On Fri, May 06, 2016 at 03:18:16PM +0100, Tvrtko Ursulin wrote:
> >>
> >> On 06/05/16 15:00, Ville Syrjälä wrote:
> >>> On Fri, May 06, 2016 at 02:43:50PM +0100, Tvrtko Ursulin wrote:
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> If we allow it a dedicated flag in dev_priv we enable the
> >>>> compiler to nicely optimize conditions like IS_HASSWELL ||
> >>>> IS_BROADWELL.
> >>>>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>> ---
> >>>> drivers/gpu/drm/i915/i915_drv.c | 4 ++++
> >>>> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> >>>> 2 files changed, 6 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >>>> index 9fd221c97275..da6532da44e3 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_drv.c
> >>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >>>> @@ -300,22 +300,26 @@ static const struct intel_device_info intel_haswell_m_info = {
> >>>> static const struct intel_device_info intel_broadwell_d_info = {
> >>>> BDW_FEATURES,
> >>>> .gen = 8,
> >>>> + .is_broadwell = 1,
> >>>> };
> >>>>
> >>>> static const struct intel_device_info intel_broadwell_m_info = {
> >>>> BDW_FEATURES,
> >>>> .gen = 8, .is_mobile = 1,
> >>>> + .is_broadwell = 1,
> >>>> };
> >>>>
> >>>> static const struct intel_device_info intel_broadwell_gt3d_info = {
> >>>> BDW_FEATURES,
> >>>> .gen = 8,
> >>>> + .is_broadwell = 1,
> >>>> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >>>> };
> >>>>
> >>>> static const struct intel_device_info intel_broadwell_gt3m_info = {
> >>>> BDW_FEATURES,
> >>>> .gen = 8, .is_mobile = 1,
> >>>> + .is_broadwell = 1,
> >>>> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> >>>> };
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>>> index c6351016eaf0..459561991081 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_drv.h
> >>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >>>> @@ -735,6 +735,7 @@ struct intel_csr {
> >>>> func(is_valleyview) sep \
> >>>> func(is_cherryview) sep \
> >>>> func(is_haswell) sep \
> >>>> + func(is_broadwell) sep \
> >>>> func(is_skylake) sep \
> >>>> func(is_broxton) sep \
> >>>> func(is_kabylake) sep \
> >>>
> >>> We could just replace all of these with a single enum and save a bunch
> >>> of space.
> >>
> >> Bitfield is really nice here because compiler can then merge tests like
> >> IS_HASWELL || IS_BROADWELL into a single test instruction.
> >
> > Most if not all of those are in modeset code so I'm not entirely
> > convinced micro-optimizing that is all that useful.
>
> Maybe not, but IS_BROADWELL was just tasteless and it is better to save
> instructions across everything than a bit in dev_priv.
>
> > Bunch of times I've just wished I could write 'whatever >= G4X' instead
> > of 'IS_G4X || gen >= 5' simply for readability.
>
> I don't even know what G4X is so can't really follow what you mean. Is
> it like Gen4.5, or Gen5-- ? So I can't figure out what kind of scheme
> you propose.
It's still a GMCH platform, but the display engine north side bits are
very close to ILK already. Oh and I suppose a bunch of the south side
bits are also pretty close to ILK as well since G4X has native HDMI
and DP ports already. So the display engine in many ways is closer to
ILK than to gen4. The one major difference to ILK is the lack of FDI.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro
2016-05-06 14:33 ` [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro Chris Wilson
@ 2016-05-06 19:11 ` Dave Gordon
2016-05-09 12:32 ` Jani Nikula
0 siblings, 1 reply; 27+ messages in thread
From: Dave Gordon @ 2016-05-06 19:11 UTC (permalink / raw)
To: Chris Wilson, Tvrtko Ursulin, Intel-gfx
On 06/05/16 15:33, Chris Wilson wrote:
> On Fri, May 06, 2016 at 03:16:25PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> To be used for more efficient Gen range checking.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 1 +
>> drivers/gpu/drm/i915/intel_fbc.c | 2 +-
>> drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
>> 3 files changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 15fcbcece13c..935e381407ba 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
>> })
>> #define INTEL_INFO(p) (&__I915__(p)->info)
>> #define INTEL_GEN(p) (INTEL_INFO(p)->gen)
>> +#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
>> #define INTEL_DEVID(p) (INTEL_INFO(p)->device_id)
>> #define INTEL_REVID(p) (__I915__(p)->dev->pdev->revision)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
>> index d5a7cfec589b..2c3681757aba 100644
>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>> @@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
>>
>> /* FIXME: We lack the proper locking here, so only run this on the
>> * platforms that need. */
>> - if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
>> + if (INTEL_GEN_RANGE(dev_priv, 5, 6))
>> cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
>> cache->fb.pixel_format = fb->pixel_format;
>> cache->fb.stride = fb->pitches[0];
>> @@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
>> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>>
>> /* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
>> - if (IS_GEN7(dev))
>> + if (IS_GEN7(dev_priv))
>> I915_WRITE(GFX_MODE_GEN7,
>> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
>> _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
>>
>> - if (IS_GEN6(dev)) {
>> + if (IS_GEN6(dev_priv)) {
>
> This chunk shouldn't be in this patch.
>
> Couldn't improve upon INTEL_GEN_RANGE.
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
INTEL_GEN_IN_RANGE() ?
Perhaps emphasises that we're using INclusive ranges?
(whereas the open-coded version might use inclusive or exclusive or
semi-inclusive at either end, as in the example above.)
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] drm/i915: Do not use a bitfield for INTEL_INFO->num_pipes
2016-05-06 14:16 ` [PATCH 2/2] drm/i915: Do not use a bitfield for INTEL_INFO->num_pipes Tvrtko Ursulin
@ 2016-05-06 19:21 ` Dave Gordon
0 siblings, 0 replies; 27+ messages in thread
From: Dave Gordon @ 2016-05-06 19:21 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
On 06/05/16 15:16, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> It just makes more work for the compiler and generates more code.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 935e381407ba..f7a156b8d5a1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -758,7 +758,7 @@ struct intel_csr {
> struct intel_device_info {
> u32 display_mmio_offset;
> u16 device_id;
> - u8 num_pipes:3;
> + u8 num_pipes;
> u8 num_sprites[I915_MAX_PIPES];
> u8 gen;
> u16 gen_mask;
On my build, this saves 322 bytes, which (since it eliminates just ONE
three-byte instruction on each use), suggests it's used quite a lot!
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro
2016-05-06 19:11 ` Dave Gordon
@ 2016-05-09 12:32 ` Jani Nikula
2016-05-09 14:26 ` Dave Gordon
0 siblings, 1 reply; 27+ messages in thread
From: Jani Nikula @ 2016-05-09 12:32 UTC (permalink / raw)
To: Dave Gordon, Chris Wilson, Tvrtko Ursulin, Intel-gfx
On Fri, 06 May 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
> On 06/05/16 15:33, Chris Wilson wrote:
>> On Fri, May 06, 2016 at 03:16:25PM +0100, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> To be used for more efficient Gen range checking.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_drv.h | 1 +
>>> drivers/gpu/drm/i915/intel_fbc.c | 2 +-
>>> drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
>>> 3 files changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index 15fcbcece13c..935e381407ba 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
>>> })
>>> #define INTEL_INFO(p) (&__I915__(p)->info)
>>> #define INTEL_GEN(p) (INTEL_INFO(p)->gen)
>>> +#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
>>> #define INTEL_DEVID(p) (INTEL_INFO(p)->device_id)
>>> #define INTEL_REVID(p) (__I915__(p)->dev->pdev->revision)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
>>> index d5a7cfec589b..2c3681757aba 100644
>>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>>> @@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
>>>
>>> /* FIXME: We lack the proper locking here, so only run this on the
>>> * platforms that need. */
>>> - if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
>>> + if (INTEL_GEN_RANGE(dev_priv, 5, 6))
>>> cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
>>> cache->fb.pixel_format = fb->pixel_format;
>>> cache->fb.stride = fb->pitches[0];
>>> @@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
>>> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>>>
>>> /* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
>>> - if (IS_GEN7(dev))
>>> + if (IS_GEN7(dev_priv))
>>> I915_WRITE(GFX_MODE_GEN7,
>>> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
>>> _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
>>>
>>> - if (IS_GEN6(dev)) {
>>> + if (IS_GEN6(dev_priv)) {
>>
>> This chunk shouldn't be in this patch.
>>
>> Couldn't improve upon INTEL_GEN_RANGE.
>>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>> -Chris
>
> INTEL_GEN_IN_RANGE() ?
>
> Perhaps emphasises that we're using INclusive ranges?
Or just IS_GEN(p, since, until) similar to IS_SKL_REVID() and
IS_BXT_REVID(). Might as well make this shorter to write.
BR,
Jani.
>
> (whereas the open-coded version might use inclusive or exclusive or
> semi-inclusive at either end, as in the example above.)
>
> .Dave.
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx
2016-05-06 13:43 ` [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx Tvrtko Ursulin
2016-05-06 13:59 ` Chris Wilson
@ 2016-05-09 14:18 ` Dave Gordon
2016-05-10 7:47 ` Jani Nikula
2 siblings, 0 replies; 27+ messages in thread
From: Dave Gordon @ 2016-05-09 14:18 UTC (permalink / raw)
To: intel-gfx
On 06/05/16 14:43, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> This way optimization from a previous patch works even better.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
> drivers/gpu/drm/i915/i915_dma.c | 4 ++--
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_stolen.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +-
> drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
> drivers/gpu/drm/i915/i915_irq.c | 4 ++--
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> drivers/gpu/drm/i915/intel_lrc.c | 4 ++--
> drivers/gpu/drm/i915/intel_lvds.c | 2 +-
> drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++---
> 15 files changed, 22 insertions(+), 22 deletions(-)
This looked a very small list, I thought, and then realised you've only
changed tests-for-equality. There are a *far* greater number of tests
for inequality!
These are the numbers I got for converting *all* instances of
INTEL_INFO()->gen ...
i915/i915_debugfs.c | 68 ++++++-------
i915/i915_dma.c | 20 ++--
i915/i915_drv.c | 8 +-
i915/i915_drv.h | 42 ++++----
i915/i915_gem.c | 18 ++--
i915/i915_gem_context.c | 14 +--
i915/i915_gem_execbuffer.c | 18 ++--
i915/i915_gem_fence.c | 8 +-
i915/i915_gem_gtt.c | 36 +++----
i915/i915_gem_render_state.c | 2 +-
i915/i915_gem_stolen.c | 10 +-
i915/i915_gem_tiling.c | 10 +-
i915/i915_gpu_error.c | 38 ++++----
i915/i915_irq.c | 72 +++++++-------
i915/i915_suspend.c | 20 ++--
i915/i915_sysfs.c | 2 +-
i915/intel_audio.c | 2 +-
i915/intel_bios.c | 2 +-
i915/intel_color.c | 2 +-
i915/intel_crt.c | 6 +-
i915/intel_ddi.c | 10 +-
i915/intel_display.c | 182 +++++++++++++++++------------------
i915/intel_dp.c | 30 +++---
i915/intel_dpll_mgr.c | 2 +-
i915/intel_fbc.c | 30 +++---
i915/intel_guc_loader.c | 2 +-
i915/intel_hdmi.c | 4 +-
i915/intel_lrc.c | 22 ++---
i915/intel_lvds.c | 14 +--
i915/intel_mocs.c | 2 +-
i915/intel_overlay.c | 4 +-
i915/intel_panel.c | 10 +-
i915/intel_pm.c | 68 ++++++-------
i915/intel_psr.c | 10 +-
i915/intel_ringbuffer.c | 50 +++++-----
i915/intel_sdvo.c | 10 +-
i915/intel_sprite.c | 12 +--
i915/intel_tv.c | 2 +-
i915/intel_uncore.c | 20 ++--
So if you want to convert the GEN info to a new representation, it
should ideally be one that efficiently handles "up to N" and "from N
onwards" as well as exact matches.
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro
2016-05-09 12:32 ` Jani Nikula
@ 2016-05-09 14:26 ` Dave Gordon
2016-05-09 14:40 ` Jani Nikula
0 siblings, 1 reply; 27+ messages in thread
From: Dave Gordon @ 2016-05-09 14:26 UTC (permalink / raw)
To: Jani Nikula, Chris Wilson, Tvrtko Ursulin, Intel-gfx
On 09/05/16 13:32, Jani Nikula wrote:
> On Fri, 06 May 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
>> On 06/05/16 15:33, Chris Wilson wrote:
>>> On Fri, May 06, 2016 at 03:16:25PM +0100, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> To be used for more efficient Gen range checking.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/i915_drv.h | 1 +
>>>> drivers/gpu/drm/i915/intel_fbc.c | 2 +-
>>>> drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
>>>> 3 files changed, 8 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>> index 15fcbcece13c..935e381407ba 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>> @@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
>>>> })
>>>> #define INTEL_INFO(p) (&__I915__(p)->info)
>>>> #define INTEL_GEN(p) (INTEL_INFO(p)->gen)
>>>> +#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
>>>> #define INTEL_DEVID(p) (INTEL_INFO(p)->device_id)
>>>> #define INTEL_REVID(p) (__I915__(p)->dev->pdev->revision)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
>>>> index d5a7cfec589b..2c3681757aba 100644
>>>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>>>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>>>> @@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
>>>>
>>>> /* FIXME: We lack the proper locking here, so only run this on the
>>>> * platforms that need. */
>>>> - if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
>>>> + if (INTEL_GEN_RANGE(dev_priv, 5, 6))
>>>> cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
>>>> cache->fb.pixel_format = fb->pixel_format;
>>>> cache->fb.stride = fb->pitches[0];
>>>> @@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
>>>> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>>>>
>>>> /* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
>>>> - if (IS_GEN7(dev))
>>>> + if (IS_GEN7(dev_priv))
>>>> I915_WRITE(GFX_MODE_GEN7,
>>>> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
>>>> _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
>>>>
>>>> - if (IS_GEN6(dev)) {
>>>> + if (IS_GEN6(dev_priv)) {
>>>
>>> This chunk shouldn't be in this patch.
>>>
>>> Couldn't improve upon INTEL_GEN_RANGE.
>>>
>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> -Chris
>>
>> INTEL_GEN_IN_RANGE() ?
>>
>> Perhaps emphasises that we're using INclusive ranges?
>
> Or just IS_GEN(p, since, until) similar to IS_SKL_REVID() and
> IS_BXT_REVID(). Might as well make this shorter to write.
>
> BR,
> Jani.
We need a notation that will look good (and be efficient) for "before
GEN x" and "from GEN y on" (or "up to and including GEN p" and "after
GEN q") -- these make up the vast majority of tests on the GEN number.
Also, we need real clarity on inclusion/exclusion. since..until notation
is perhaps ambiguous - in particular, is "until" included? English usage
is not clear here, it's often but not always exclusive!
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro
2016-05-09 14:26 ` Dave Gordon
@ 2016-05-09 14:40 ` Jani Nikula
2016-05-09 15:23 ` Chris Wilson
0 siblings, 1 reply; 27+ messages in thread
From: Jani Nikula @ 2016-05-09 14:40 UTC (permalink / raw)
To: Dave Gordon, Chris Wilson, Tvrtko Ursulin, Intel-gfx
On Mon, 09 May 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
> On 09/05/16 13:32, Jani Nikula wrote:
>> On Fri, 06 May 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
>>> On 06/05/16 15:33, Chris Wilson wrote:
>>>> On Fri, May 06, 2016 at 03:16:25PM +0100, Tvrtko Ursulin wrote:
>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>
>>>>> To be used for more efficient Gen range checking.
>>>>>
>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/i915/i915_drv.h | 1 +
>>>>> drivers/gpu/drm/i915/intel_fbc.c | 2 +-
>>>>> drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
>>>>> 3 files changed, 8 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>>> index 15fcbcece13c..935e381407ba 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>> @@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
>>>>> })
>>>>> #define INTEL_INFO(p) (&__I915__(p)->info)
>>>>> #define INTEL_GEN(p) (INTEL_INFO(p)->gen)
>>>>> +#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
>>>>> #define INTEL_DEVID(p) (INTEL_INFO(p)->device_id)
>>>>> #define INTEL_REVID(p) (__I915__(p)->dev->pdev->revision)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
>>>>> index d5a7cfec589b..2c3681757aba 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>>>>> @@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
>>>>>
>>>>> /* FIXME: We lack the proper locking here, so only run this on the
>>>>> * platforms that need. */
>>>>> - if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
>>>>> + if (INTEL_GEN_RANGE(dev_priv, 5, 6))
>>>>> cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
>>>>> cache->fb.pixel_format = fb->pixel_format;
>>>>> cache->fb.stride = fb->pitches[0];
>>>>> @@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
>>>>> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>>>>>
>>>>> /* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
>>>>> - if (IS_GEN7(dev))
>>>>> + if (IS_GEN7(dev_priv))
>>>>> I915_WRITE(GFX_MODE_GEN7,
>>>>> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
>>>>> _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
>>>>>
>>>>> - if (IS_GEN6(dev)) {
>>>>> + if (IS_GEN6(dev_priv)) {
>>>>
>>>> This chunk shouldn't be in this patch.
>>>>
>>>> Couldn't improve upon INTEL_GEN_RANGE.
>>>>
>>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>> -Chris
>>>
>>> INTEL_GEN_IN_RANGE() ?
>>>
>>> Perhaps emphasises that we're using INclusive ranges?
>>
>> Or just IS_GEN(p, since, until) similar to IS_SKL_REVID() and
>> IS_BXT_REVID(). Might as well make this shorter to write.
>>
>> BR,
>> Jani.
>
> We need a notation that will look good (and be efficient) for "before
> GEN x" and "from GEN y on" (or "up to and including GEN p" and "after
> GEN q") -- these make up the vast majority of tests on the GEN number.
>
> Also, we need real clarity on inclusion/exclusion. since..until notation
> is perhaps ambiguous - in particular, is "until" included? English usage
> is not clear here, it's often but not always exclusive!
If you want real clarity, you'll ditch this patch and use
if (INTEL_GEN(p) >= 5 && INTEL_GEN(p) <= 7)
etc...
and be done with it. There's too much churn going on with all the
related macros anyway, so I'm not at all convinced about the patches
anyway.
J.
>
> .Dave.
>
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro
2016-05-09 14:40 ` Jani Nikula
@ 2016-05-09 15:23 ` Chris Wilson
2016-05-10 7:54 ` Jani Nikula
0 siblings, 1 reply; 27+ messages in thread
From: Chris Wilson @ 2016-05-09 15:23 UTC (permalink / raw)
To: Jani Nikula; +Cc: Intel-gfx
On Mon, May 09, 2016 at 05:40:39PM +0300, Jani Nikula wrote:
> On Mon, 09 May 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
> > On 09/05/16 13:32, Jani Nikula wrote:
> >> On Fri, 06 May 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
> >>> On 06/05/16 15:33, Chris Wilson wrote:
> >>>> On Fri, May 06, 2016 at 03:16:25PM +0100, Tvrtko Ursulin wrote:
> >>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>>
> >>>>> To be used for more efficient Gen range checking.
> >>>>>
> >>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>> ---
> >>>>> drivers/gpu/drm/i915/i915_drv.h | 1 +
> >>>>> drivers/gpu/drm/i915/intel_fbc.c | 2 +-
> >>>>> drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
> >>>>> 3 files changed, 8 insertions(+), 7 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>>>> index 15fcbcece13c..935e381407ba 100644
> >>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
> >>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >>>>> @@ -2518,6 +2518,7 @@ struct drm_i915_cmd_table {
> >>>>> })
> >>>>> #define INTEL_INFO(p) (&__I915__(p)->info)
> >>>>> #define INTEL_GEN(p) (INTEL_INFO(p)->gen)
> >>>>> +#define INTEL_GEN_RANGE(p, s, e) (INTEL_INFO(p)->gen_mask & GENMASK(e, s))
> >>>>> #define INTEL_DEVID(p) (INTEL_INFO(p)->device_id)
> >>>>> #define INTEL_REVID(p) (__I915__(p)->dev->pdev->revision)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> >>>>> index d5a7cfec589b..2c3681757aba 100644
> >>>>> --- a/drivers/gpu/drm/i915/intel_fbc.c
> >>>>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> >>>>> @@ -740,7 +740,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
> >>>>>
> >>>>> /* FIXME: We lack the proper locking here, so only run this on the
> >>>>> * platforms that need. */
> >>>>> - if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
> >>>>> + if (INTEL_GEN_RANGE(dev_priv, 5, 6))
> >>>>> cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
> >>>>> cache->fb.pixel_format = fb->pixel_format;
> >>>>> cache->fb.stride = fb->pitches[0];
> >>>>> @@ -1241,12 +1241,12 @@ static int init_render_ring(struct intel_engine_cs *engine)
> >>>>> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
> >>>>>
> >>>>> /* WaBCSVCSTlbInvalidationMode:ivb,vlv,hsw */
> >>>>> - if (IS_GEN7(dev))
> >>>>> + if (IS_GEN7(dev_priv))
> >>>>> I915_WRITE(GFX_MODE_GEN7,
> >>>>> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT) |
> >>>>> _MASKED_BIT_ENABLE(GFX_REPLAY_MODE));
> >>>>>
> >>>>> - if (IS_GEN6(dev)) {
> >>>>> + if (IS_GEN6(dev_priv)) {
> >>>>
> >>>> This chunk shouldn't be in this patch.
> >>>>
> >>>> Couldn't improve upon INTEL_GEN_RANGE.
> >>>>
> >>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>> -Chris
> >>>
> >>> INTEL_GEN_IN_RANGE() ?
> >>>
> >>> Perhaps emphasises that we're using INclusive ranges?
> >>
> >> Or just IS_GEN(p, since, until) similar to IS_SKL_REVID() and
> >> IS_BXT_REVID(). Might as well make this shorter to write.
> >>
> >> BR,
> >> Jani.
> >
> > We need a notation that will look good (and be efficient) for "before
> > GEN x" and "from GEN y on" (or "up to and including GEN p" and "after
> > GEN q") -- these make up the vast majority of tests on the GEN number.
> >
> > Also, we need real clarity on inclusion/exclusion. since..until notation
> > is perhaps ambiguous - in particular, is "until" included? English usage
> > is not clear here, it's often but not always exclusive!
>
> If you want real clarity, you'll ditch this patch and use
>
> if (INTEL_GEN(p) >= 5 && INTEL_GEN(p) <= 7)
> etc...
>
> and be done with it. There's too much churn going on with all the
> related macros anyway, so I'm not at all convinced about the patches
> anyway.
IS_GEN(p, start, end) using inclusive ranges is fine.
This is a small patch that generates a remarkable amount of object code
reduction.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx
2016-05-06 13:43 ` [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx Tvrtko Ursulin
2016-05-06 13:59 ` Chris Wilson
2016-05-09 14:18 ` Dave Gordon
@ 2016-05-10 7:47 ` Jani Nikula
2 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2016-05-10 7:47 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
On Fri, 06 May 2016, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> This way optimization from a previous patch works even better.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
> drivers/gpu/drm/i915/i915_dma.c | 4 ++--
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_stolen.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +-
> drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
> drivers/gpu/drm/i915/i915_irq.c | 4 ++--
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> drivers/gpu/drm/i915/intel_lrc.c | 4 ++--
> drivers/gpu/drm/i915/intel_lvds.c | 2 +-
> drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++---
> 15 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 6ad008c196b5..e264f168ef02 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2310,12 +2310,12 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_engine_cs *engine;
>
> - if (INTEL_INFO(dev)->gen == 6)
> + if (IS_GEN6(dev_priv))
> seq_printf(m, "GFX_MODE: 0x%08x\n", I915_READ(GFX_MODE));
>
> for_each_engine(engine, dev_priv) {
> seq_printf(m, "%s\n", engine->name);
> - if (INTEL_INFO(dev)->gen == 7)
> + if (IS_GEN7(dev_priv))
> seq_printf(m, "GFX_MODE: 0x%08x\n",
> I915_READ(RING_MODE_GEN7(engine)));
> seq_printf(m, "PP_DIR_BASE: 0x%08x\n",
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 01163da51b1d..a65c85c8317c 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -888,7 +888,7 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
> DRM_INFO("Display disabled (module parameter)\n");
> info->num_pipes = 0;
> } else if (info->num_pipes > 0 &&
> - (INTEL_INFO(dev)->gen == 7 || INTEL_INFO(dev)->gen == 8) &&
> + (IS_GEN7(dev_priv) || IS_GEN8(dev_priv)) &&
> HAS_PCH_SPLIT(dev)) {
> u32 fuse_strap = I915_READ(FUSE_STRAP);
> u32 sfuse_strap = I915_READ(SFUSE_STRAP);
> @@ -912,7 +912,7 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
> DRM_INFO("PipeC fused off\n");
> info->num_pipes -= 1;
> }
> - } else if (info->num_pipes > 0 && INTEL_INFO(dev)->gen == 9) {
> + } else if (info->num_pipes > 0 && IS_GEN9(dev_priv)) {
> u32 dfsm = I915_READ(SKL_DFSM);
> u8 disabled_mask = 0;
> bool invalid;
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index da6532da44e3..f27cc15b6f8e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -548,7 +548,7 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
>
> #ifdef CONFIG_INTEL_IOMMU
> /* Enable semaphores on SNB when IO remapping is off */
> - if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
> + if (IS_GEN6(dev) && intel_iommu_gfx_mapped)
> return false;
> #endif
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 459561991081..15fcbcece13c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2702,7 +2702,7 @@ struct drm_i915_cmd_table {
> IS_CHERRYVIEW(dev) || IS_SKYLAKE(dev) || \
> IS_KABYLAKE(dev) || IS_BROXTON(dev))
> #define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6)
> -#define HAS_RC6p(dev) (INTEL_INFO(dev)->gen == 6 || IS_IVYBRIDGE(dev))
> +#define HAS_RC6p(dev) (IS_GEN6(dev) || IS_IVYBRIDGE(dev))
>
> #define HAS_CSR(dev) (IS_GEN9(dev))
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index a88e6c9e9516..f2e42c0cc5be 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1982,7 +1982,7 @@ i915_gem_get_gtt_size(struct drm_device *dev, uint32_t size, int tiling_mode)
> return size;
>
> /* Previous chips need a power-of-two fence region when tiling */
> - if (INTEL_INFO(dev)->gen == 3)
> + if (IS_GEN3(dev))
> gtt_size = 1024*1024;
> else
> gtt_size = 512*1024;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index d284b17af431..61e85c2b500d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -144,7 +144,7 @@ int intel_sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
>
> #ifdef CONFIG_INTEL_IOMMU
> /* Disable ppgtt on SNB if VT-d is on. */
> - if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped) {
> + if (IS_GEN6(dev) && intel_iommu_gfx_mapped) {
> DRM_INFO("Disabling PPGTT because VT-d is on\n");
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 68fde8fba803..f9253f2b7ba0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -56,7 +56,7 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
>
> /* See the comment at the drm_mm_init() call for more about this check.
> * WaSkipStolenMemoryFirstPage:bdw,chv (incomplete) */
> - if (INTEL_INFO(dev_priv)->gen == 8 && start < 4096)
> + if (IS_GEN8(dev_priv) && start < 4096)
> start = 4096;
>
> mutex_lock(&dev_priv->mm.stolen_lock);
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index 829dab69895f..2fcb4afade19 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -125,7 +125,7 @@ i915_gem_object_fence_ok(struct drm_i915_gem_object *obj, int tiling_mode)
> if (INTEL_INFO(obj->base.dev)->gen >= 4)
> return true;
>
> - if (INTEL_INFO(obj->base.dev)->gen == 3) {
> + if (IS_GEN3(obj->base.dev)) {
> if (i915_gem_obj_ggtt_offset(obj) & ~I915_FENCE_START_MASK)
> return false;
> } else {
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 89725c9efc25..12a749c9707a 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -411,7 +411,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> err_printf(m, "DONE_REG: 0x%08x\n", error->done_reg);
> }
>
> - if (INTEL_INFO(dev)->gen == 7)
> + if (IS_GEN7(dev))
> err_printf(m, "ERR_INT: 0x%08x\n", error->err_int);
>
> for (i = 0; i < ARRAY_SIZE(error->ring); i++)
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 2f6fd33c07ba..ba84b9b17fc2 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4674,12 +4674,12 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> dev->driver->disable_vblank = ironlake_disable_vblank;
> dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
> } else {
> - if (INTEL_INFO(dev_priv)->gen == 2) {
> + if (IS_GEN2(dev_priv)) {
> dev->driver->irq_preinstall = i8xx_irq_preinstall;
> dev->driver->irq_postinstall = i8xx_irq_postinstall;
> dev->driver->irq_handler = i8xx_irq_handler;
> dev->driver->irq_uninstall = i8xx_irq_uninstall;
> - } else if (INTEL_INFO(dev_priv)->gen == 3) {
> + } else if (IS_GEN3(dev_priv)) {
> dev->driver->irq_preinstall = i915_irq_preinstall;
> dev->driver->irq_postinstall = i915_irq_postinstall;
> dev->driver->irq_uninstall = i915_irq_uninstall;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 45c218db86be..1385423ac960 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1204,7 +1204,7 @@ static void assert_fdi_tx_pll_enabled(struct drm_i915_private *dev_priv,
> u32 val;
>
> /* ILK FDI PLL is always enabled */
> - if (INTEL_INFO(dev_priv)->gen == 5)
> + if (IS_GEN5(dev_priv))
> return;
>
> /* On Haswell, DDI ports are responsible for the FDI PLL setup */
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index d8763524319d..f614a07a07bb 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1387,7 +1387,7 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
> batch = kmap_atomic(page);
> offset = 0;
>
> - if (INTEL_INFO(engine->dev)->gen == 8) {
> + if (IS_GEN8(engine->dev)) {
> ret = gen8_init_indirectctx_bb(engine,
> &wa_ctx->indirect_ctx,
> batch,
> @@ -1401,7 +1401,7 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
> &offset);
> if (ret)
> goto out;
> - } else if (INTEL_INFO(engine->dev)->gen == 9) {
> + } else if (IS_GEN9(engine->dev)) {
> ret = gen9_init_indirectctx_bb(engine,
> &wa_ctx->indirect_ctx,
> batch,
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index bc53c0dd34d0..d65fd945607a 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -190,7 +190,7 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
> /* Set the dithering flag on LVDS as needed, note that there is no
> * special lvds dither control bit on pch-split platforms, dithering is
> * only controlled through the PIPECONF reg. */
> - if (INTEL_INFO(dev)->gen == 4) {
> + if (IS_GEN4(dev_priv)) {
> /* Bspec wording suggests that LVDS port dithering only exists
> * for 18bpp panels. */
> if (crtc->config->dither && crtc->config->pipe_bpp == 18)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 017c431f9363..5f17f682d37b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2146,14 +2146,14 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[8])
> static void intel_fixup_spr_wm_latency(struct drm_device *dev, uint16_t wm[5])
> {
> /* ILK sprite LP0 latency is 1300 ns */
> - if (INTEL_INFO(dev)->gen == 5)
> + if (IS_GEN5(dev))
> wm[0] = 13;
> }
>
> static void intel_fixup_cur_wm_latency(struct drm_device *dev, uint16_t wm[5])
> {
> /* ILK cursor LP0 latency is 1300 ns */
> - if (INTEL_INFO(dev)->gen == 5)
> + if (IS_GEN5(dev))
> wm[0] = 13;
>
> /* WaDoubleCursorLP3Latency:ivb */
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 8f3eb3033da0..710cd598c701 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1236,7 +1236,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
>
> /* Required for the hardware to program scanline values for waiting */
> /* WaEnableFlushTlbInvalidationMode:snb */
> - if (INTEL_INFO(dev)->gen == 6)
> + if (IS_GEN6(dev_priv))
> I915_WRITE(GFX_MODE,
> _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_EXPLICIT));
>
> @@ -2536,7 +2536,7 @@ void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno)
> * the semaphore value, then when the seqno moves backwards all
> * future waits will complete instantly (causing rendering corruption).
> */
> - if (INTEL_INFO(dev_priv)->gen == 6 || INTEL_INFO(dev_priv)->gen == 7) {
> + if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) {
> I915_WRITE(RING_SYNC_0(engine->mmio_base), 0);
> I915_WRITE(RING_SYNC_1(engine->mmio_base), 0);
> if (HAS_VEBOX(dev_priv))
> @@ -2808,7 +2808,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
> engine->init_context = intel_rcs_ctx_init;
> engine->add_request = gen6_add_request;
> engine->flush = gen7_render_ring_flush;
> - if (INTEL_INFO(dev)->gen == 6)
> + if (IS_GEN6(dev_priv))
> engine->flush = gen6_render_ring_flush;
> engine->irq_get = gen6_ring_get_irq;
> engine->irq_put = gen6_ring_put_irq;
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro
2016-05-09 15:23 ` Chris Wilson
@ 2016-05-10 7:54 ` Jani Nikula
0 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2016-05-10 7:54 UTC (permalink / raw)
To: Chris Wilson; +Cc: Intel-gfx
On Mon, 09 May 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> IS_GEN(p, start, end) using inclusive ranges is fine.
>
> This is a small patch that generates a remarkable amount of object code
> reduction.
Ok.
I guess 0 is okay for start, but how about a "GEN_FOREVER" style macro
for replacing ->gen >= 5 with IS_GEN(dev_priv, 5, GEN_FOREVER)? This
would be in line with the IS_{SKL,BXT}_REVID checks.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2016-05-10 7:54 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-06 13:43 [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask Tvrtko Ursulin
2016-05-06 13:43 ` [PATCH 2/3] drm/i915: Promote IS_BROADWELL to a simple macro Tvrtko Ursulin
2016-05-06 14:00 ` Ville Syrjälä
2016-05-06 14:18 ` Tvrtko Ursulin
2016-05-06 14:29 ` Ville Syrjälä
2016-05-06 14:44 ` Tvrtko Ursulin
2016-05-06 14:51 ` Chris Wilson
2016-05-06 14:58 ` Ville Syrjälä
2016-05-06 13:43 ` [PATCH 3/3] drm/i915: Replace "INTEL_INFO->gen == x" checks with IS_GENx Tvrtko Ursulin
2016-05-06 13:59 ` Chris Wilson
2016-05-06 14:16 ` Tvrtko Ursulin
2016-05-06 14:20 ` Tvrtko Ursulin
2016-05-09 14:18 ` Dave Gordon
2016-05-10 7:47 ` Jani Nikula
2016-05-06 14:16 ` [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro Tvrtko Ursulin
2016-05-06 14:16 ` [PATCH 2/2] drm/i915: Do not use a bitfield for INTEL_INFO->num_pipes Tvrtko Ursulin
2016-05-06 19:21 ` Dave Gordon
2016-05-06 14:33 ` [PATCH 1/2] drm/i915: Introduce INTEL_GEN_RANGE macro Chris Wilson
2016-05-06 19:11 ` Dave Gordon
2016-05-09 12:32 ` Jani Nikula
2016-05-09 14:26 ` Dave Gordon
2016-05-09 14:40 ` Jani Nikula
2016-05-09 15:23 ` Chris Wilson
2016-05-10 7:54 ` Jani Nikula
2016-05-06 14:28 ` [PATCH 1/3] drm/i915: Make IS_GENx macros work on a mask Chris Wilson
2016-05-06 14:36 ` Tvrtko Ursulin
2016-05-06 14:43 ` Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.