Re: [PATCH v3] drm/i915/icl: Set graphics mode register for gen11

Daniele Ceraolo Spurio
2018-01-19T22:46:58Z

On 19/01/18 11:30, Kelvin Gardiner wrote:
> This patch clears a single bit. The bit is 0 by default but expected not to be
> set. Explicitly clearing the bit in this patch is intended to indicate some
> thinking has occurred, and that we want this bit cleared and we are not just
> excepting the default value.
>

We also stop setting GFX_RUN_LIST_ENABLE, which is correct since that
bit is gone. Code is self-explanatory but could still use a mention in
the commit message IMHO.

> v2 (from Paulo): fix indentation.
> v3: Changed GEN check to >= 11. Corrected author name.
>
> Signed-off-by: Kelvin Gardiner <kelvin.gardiner@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Thanks,
Daniele

> ---
> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> drivers/gpu/drm/i915/intel_lrc.c | 18 ++++++++++++++++--
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 73c9c36..057f90e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2604,6 +2604,8 @@ enum i915_power_well_id {
> #define GFX_FORWARD_VBLANK_ALWAYS (1<<5)
> #define GFX_FORWARD_VBLANK_COND (2<<5)
>
> +#define GEN11_GFX_DISABLE_LEGACY_MODE (1<<3)
> +
> #define VLV_DISPLAY_BASE 0x180000
> #define VLV_MIPI_BASE VLV_DISPLAY_BASE
> #define BXT_MIPI_BASE 0x60000
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index dab988f..d4cc5c9 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1500,8 +1500,22 @@ static void enable_execlists(struct intel_engine_cs *engine)
> struct drm_i915_private *dev_priv = engine->i915;
>
> I915_WRITE(RING_HWSTAM(engine->mmio_base), 0xffffffff);
> - I915_WRITE(RING_MODE_GEN7(engine),
> - _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE));
> +
> + /*
> + * Make sure we're not enabling the new 12-deep CSB
> + * FIFO as that requires a slightly updated handling
> + * in the ctx switch irq. Since we're currently only
> + * using only 2 elements of the enhanced execlists the
> + * deeper FIFO it's not needed and it's not worth adding
> + * more statements to the irq handler to support it.
> + */
> + if (INTEL_GEN(dev_priv) >= 11)
> + I915_WRITE(RING_MODE_GEN7(engine),
> + _MASKED_BIT_DISABLE(GEN11_GFX_DISABLE_LEGACY_MODE));
> + else
> + I915_WRITE(RING_MODE_GEN7(engine),
> + _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE));
> +
> I915_WRITE(RING_HWS_PGA(engine->mmio_base),
> engine->status_page.ggtt_offset);
> POSTING_READ(RING_HWS_PGA(engine->mmio_base));