Re: [PATCH] drm/i915: Reset request handling for gen8+

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Reset request handling for gen8+
Date: Thu, 18 Jun 2015 09:50:40 +0100	[thread overview]
Message-ID: <20150618085040.GY24012@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <1434616560-32187-1-git-send-email-mika.kuoppala@intel.com>

On Thu, Jun 18, 2015 at 11:36:00AM +0300, Mika Kuoppala wrote:
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0b979ad..3684f92 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1461,6 +1461,9 @@ enum skl_disp_power_wells {
>  #define RING_MAX_IDLE(base)	((base)+0x54)
>  #define RING_HWS_PGA(base)	((base)+0x80)
>  #define RING_HWS_PGA_GEN6(base)	((base)+0x2080)
> +#define RING_RESET_CTL(base)	((base)+0xd0)
> +#define   RESET_CTL_REQUEST_RESET  (1 << 0)
> +#define   RESET_CTL_READY_TO_RESET (1 << 1)
>  
>  #define HSW_GTT_CACHE_EN	0x4024
>  #define   GTT_CACHE_EN_ALL	0xF0007FFF
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 4a86cf0..6a19b3e 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1455,9 +1455,52 @@ static int gen6_do_reset(struct drm_device *dev)
>  	return ret;
>  }
>  
> +static int wait_for_register(struct drm_i915_private *dev_priv,
> +			     const u32 reg,
> +			     const u32 mask,
> +			     const u32 value,
> +			     const unsigned long timeout)

To be overly fussy, timeout_ms.

I like having units for frequently confused variables like timeouts.

(I am being fussy, because I like this function and expect to convert
lots of callsites over to it, so clarity is important.)

> +{
> +	return wait_for((I915_READ(reg) & mask) == value, timeout);
> +}
> +
> +static int gen8_do_reset(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_engine_cs *engine;
> +	int ret, i;
> +
> +	for_each_ring(engine, dev_priv, i) {
> +		I915_WRITE(RING_RESET_CTL(engine->mmio_base),
> +			   _MASKED_BIT_ENABLE(RESET_CTL_REQUEST_RESET));
> +
> +		ret = wait_for_register(dev_priv,
> +					RING_RESET_CTL(engine->mmio_base),
> +					RESET_CTL_READY_TO_RESET,
> +					RESET_CTL_READY_TO_RESET,
> +					700);
> +		if (ret) {

In a similar vein, we ignore ret here so just if (wait_for_register()) {
and not_ready: ...; return -EIO;
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx