On Tue, 06 Nov 2018, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote:
> From: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>
> RANGE makes it longer, but clear. We are also going to add a check for
> the display part, so make rename to GT.
I also still have my doubts about this patch I'm afraid. I've expressed
the concern before, but here goes again.
How much is the split of gen to GT gen and display gen going to help us
in the long run? The only current platform that would benefit from this
is GLK. However, not all IS_GEMINILAKE() can be replaced with
IS_DISPLAY_GEN() >= 10 or similar. We also have VLV/CHV display that is
better represented by HAS_GMCH_DISPLAY() and AFAICT can't usefully be
replaced by a display gen check.
My main fear is that the split adds a lot of confusion. (Where should I
use GT gen, where should I use display gen, patches to change between
one and the other. It's not 100% clear cut.)
Here too I wonder if we're better off adding more has_feature flags that
describe what gt or display features a platform has.
BR,
Jani.
>
> Diff generated with:
>
> sed 's/IS_GEN(/GT_GEN_RANGE(/g' drivers/gpu/drm/i915/*.{c,h} -i
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_perf.c | 4 ++--
> drivers/gpu/drm/i915/intel_bios.c | 2 +-
> drivers/gpu/drm/i915/intel_engine_cs.c | 2 +-
> drivers/gpu/drm/i915/intel_fbc.c | 2 +-
> drivers/gpu/drm/i915/intel_hangcheck.c | 2 +-
> drivers/gpu/drm/i915/intel_ringbuffer.c | 8 ++++----
> drivers/gpu/drm/i915/intel_uncore.c | 12 ++++++------
> 8 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8839a34f7648..e4749e93a1f9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2374,7 +2374,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> *
> * Use GEN_FOREVER for unbound start and or end.
> */
> -#define IS_GEN(dev_priv, s, e) \
> +#define GT_GEN_RANGE(dev_priv, s, e) \
> (!!((dev_priv)->info.gen_mask & INTEL_GEN_MASK((s), (e))))
>
> /*
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 2c2b63be7a6c..acc3502403b3 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1796,7 +1796,7 @@ static int gen8_enable_metric_set(struct i915_perf_stream *stream)
> * be read back from automatically triggered reports, as part of the
> * RPT_ID field.
> */
> - if (IS_GEN(dev_priv, 9, 11)) {
> + if (GT_GEN_RANGE(dev_priv, 9, 11)) {
> I915_WRITE(GEN8_OA_DEBUG,
> _MASKED_BIT_ENABLE(GEN9_OA_DEBUG_DISABLE_CLK_RATIO_REPORTS |
> GEN9_OA_DEBUG_INCLUDE_CLK_RATIO));
> @@ -3476,7 +3476,7 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
>
> dev_priv->perf.oa.gen8_valid_ctx_bit = (1<<16);
> }
> - } else if (IS_GEN(dev_priv, 10, 11)) {
> + } else if (GT_GEN_RANGE(dev_priv, 10, 11)) {
> dev_priv->perf.oa.ops.is_valid_b_counter_reg =
> gen7_is_valid_b_counter_addr;
> dev_priv->perf.oa.ops.is_valid_mux_reg =
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 0ad2304457ab..fb918e942e9a 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -446,7 +446,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
> * Only parse SDVO mappings on gens that could have SDVO. This isn't
> * accurate and doesn't have to be, as long as it's not too strict.
> */
> - if (!IS_GEN(dev_priv, 3, 7)) {
> + if (!GT_GEN_RANGE(dev_priv, 3, 7)) {
> DRM_DEBUG_KMS("Skipping SDVO device mapping\n");
> return;
> }
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index bc147d9e6c92..4f7b654614e9 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1286,7 +1286,7 @@ static void intel_engine_print_registers(const struct intel_engine_cs *engine,
> &engine->execlists;
> u64 addr;
>
> - if (engine->id == RCS && IS_GEN(dev_priv, 4, 7))
> + if (engine->id == RCS && GT_GEN_RANGE(dev_priv, 4, 7))
> drm_printf(m, "\tCCID: 0x%08x\n", I915_READ(CCID));
> drm_printf(m, "\tRING_START: 0x%08x\n",
> I915_READ(RING_START(engine->mmio_base)));
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 14cbaf4a0e93..9f41779988e5 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -784,7 +784,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
> * having a Y offset that isn't divisible by 4 causes FIFO underrun
> * and screen flicker.
> */
> - if (IS_GEN(dev_priv, 9, 10) &&
> + if (GT_GEN_RANGE(dev_priv, 9, 10) &&
> (fbc->state_cache.plane.adjusted_y & 3)) {
> fbc->no_fbc_reason = "plane Y offset is misaligned";
> return false;
> diff --git a/drivers/gpu/drm/i915/intel_hangcheck.c b/drivers/gpu/drm/i915/intel_hangcheck.c
> index e26d05a46451..9b15ed1409ae 100644
> --- a/drivers/gpu/drm/i915/intel_hangcheck.c
> +++ b/drivers/gpu/drm/i915/intel_hangcheck.c
> @@ -252,7 +252,7 @@ engine_stuck(struct intel_engine_cs *engine, u64 acthd)
> return ENGINE_WAIT_KICK;
> }
>
> - if (IS_GEN(dev_priv, 6, 7) && tmp & RING_WAIT_SEMAPHORE) {
> + if (GT_GEN_RANGE(dev_priv, 6, 7) && tmp & RING_WAIT_SEMAPHORE) {
> switch (semaphore_passed(engine)) {
> default:
> return ENGINE_DEAD;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index b8a7a014d46d..0e5f49c45298 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -407,7 +407,7 @@ static void intel_ring_setup_status_page(struct intel_engine_cs *engine)
> POSTING_READ(mmio);
>
> /* Flush the TLB for this page */
> - if (IS_GEN(dev_priv, 6, 7)) {
> + if (GT_GEN_RANGE(dev_priv, 6, 7)) {
> i915_reg_t reg = RING_INSTPM(engine->mmio_base);
>
> /* ring should be idle before issuing a sync flush*/
> @@ -629,7 +629,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
> intel_whitelist_workarounds_apply(engine);
>
> /* WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb */
> - if (IS_GEN(dev_priv, 4, 6))
> + if (GT_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
> @@ -638,7 +638,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
> *
> * WaDisableAsyncFlipPerfMode:snb,ivb,hsw,vlv
> */
> - if (IS_GEN(dev_priv, 6, 7))
> + if (GT_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 */
> @@ -663,7 +663,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
> _MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
> }
>
> - if (IS_GEN(dev_priv, 6, 7))
> + if (GT_GEN_RANGE(dev_priv, 6, 7))
> I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING));
>
> if (INTEL_GEN(dev_priv) >= 6)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 9289515108c3..4ffe26cc3c3f 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1567,13 +1567,13 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
> dev_priv->uncore.pmic_bus_access_nb.notifier_call =
> i915_pmic_bus_access_notifier;
>
> - if (IS_GEN(dev_priv, 2, 4) || intel_vgpu_active(dev_priv)) {
> + if (GT_GEN_RANGE(dev_priv, 2, 4) || intel_vgpu_active(dev_priv)) {
> ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, gen2);
> ASSIGN_READ_MMIO_VFUNCS(dev_priv, gen2);
> } else if (IS_GEN5(dev_priv)) {
> ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, gen5);
> ASSIGN_READ_MMIO_VFUNCS(dev_priv, gen5);
> - } else if (IS_GEN(dev_priv, 6, 7)) {
> + } else if (GT_GEN_RANGE(dev_priv, 6, 7)) {
> ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, gen6);
>
> if (IS_VALLEYVIEW(dev_priv)) {
> @@ -1592,7 +1592,7 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
> ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, gen8);
> ASSIGN_READ_MMIO_VFUNCS(dev_priv, gen6);
> }
> - } else if (IS_GEN(dev_priv, 9, 10)) {
> + } else if (GT_GEN_RANGE(dev_priv, 9, 10)) {
> ASSIGN_FW_DOMAINS_TABLE(__gen9_fw_ranges);
> ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, fwtable);
> ASSIGN_READ_MMIO_VFUNCS(dev_priv, fwtable);
> @@ -2321,7 +2321,7 @@ intel_uncore_forcewake_for_read(struct drm_i915_private *dev_priv,
> } else if (INTEL_GEN(dev_priv) >= 6) {
> fw_domains = __gen6_reg_read_fw_domains(offset);
> } else {
> - WARN_ON(!IS_GEN(dev_priv, 2, 5));
> + WARN_ON(!GT_GEN_RANGE(dev_priv, 2, 5));
> fw_domains = 0;
> }
>
> @@ -2343,10 +2343,10 @@ intel_uncore_forcewake_for_write(struct drm_i915_private *dev_priv,
> fw_domains = __fwtable_reg_write_fw_domains(offset);
> } else if (IS_GEN8(dev_priv)) {
> fw_domains = __gen8_reg_write_fw_domains(offset);
> - } else if (IS_GEN(dev_priv, 6, 7)) {
> + } else if (GT_GEN_RANGE(dev_priv, 6, 7)) {
> fw_domains = FORCEWAKE_RENDER;
> } else {
> - WARN_ON(!IS_GEN(dev_priv, 2, 5));
> + WARN_ON(!GT_GEN_RANGE(dev_priv, 2, 5));
> fw_domains = 0;
> }
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
