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

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 09/01/18 15:28, Paulo Zanoni wrote:
> From: kgardine <kelvin.gardiner@intel.com>
> 
> 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.
> 
> v2 (from Paulo): fix indentation.
> v3 (from Paulo): rebase.
> 
> Signed-off-by: kgardine <kelvin.gardiner@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h  |  2 ++
>   drivers/gpu/drm/i915/intel_lrc.c | 10 ++++++++--
>   2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f383ee5cc592..a16a8a2b17b4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2597,6 +2597,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 dab988f20833..d435a9982d0b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1500,8 +1500,14 @@ 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));
> +
> +	if (IS_GEN11(dev_priv))

INTEL_GEN >= 11? I'd expect this to be valid going forward instead of 
flipping back to the old value settings. Also we could use a comment,
because the bit name is not very clear. Something like:

		/*
		 * 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.
		 */

Daniele

> +		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));
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx