On 07/04/2017 14:32, Michal Wajdeczko wrote:
> In some cases we may want to spend more time in atomic wait than
> hardcoded 2us. Let's add additional hint parameter to allow extending
> internal atomic timeout to 10us before switching into heavy wait.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_i2c.c | 2 +-
> drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++--
> drivers/gpu/drm/i915/intel_uncore.c | 12 +++++++++---
> 5 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 41188d6..6f17517 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3098,6 +3098,7 @@ int intel_wait_for_register_fw(struct drm_i915_private *dev_priv,
> i915_reg_t reg,
> const u32 mask,
> const u32 value,
> + bool is_fast,
> const unsigned int timeout_ms);
>
> static inline bool intel_gvt_active(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index b6401e8..6753229 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -299,7 +299,7 @@ gmbus_wait_idle(struct drm_i915_private *dev_priv)
>
> ret = intel_wait_for_register_fw(dev_priv,
> GMBUS2, GMBUS_ACTIVE, 0,
> - 10);
> + true, 10);
>
> I915_WRITE_FW(GMBUS4, 0);
> remove_wait_queue(&dev_priv->gmbus_wait_queue, &wait);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 55e1e88..f7efaca 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -8137,7 +8137,7 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val
>
> if (intel_wait_for_register_fw(dev_priv,
> GEN6_PCODE_MAILBOX, GEN6_PCODE_READY, 0,
> - 500)) {
> + true, 500)) {
> DRM_ERROR("timeout waiting for pcode read (%d) to finish\n", mbox);
> return -ETIMEDOUT;
> }
> @@ -8182,7 +8182,7 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv,
>
> if (intel_wait_for_register_fw(dev_priv,
> GEN6_PCODE_MAILBOX, GEN6_PCODE_READY, 0,
> - 500)) {
> + true, 500)) {
> DRM_ERROR("timeout waiting for pcode write (%d) to finish\n", mbox);
> return -ETIMEDOUT;
> }
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index c98acc2..be649cf 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -540,7 +540,7 @@ static int init_ring_common(struct intel_engine_cs *engine)
> /* If the head is still not zero, the ring is dead */
> if (intel_wait_for_register_fw(dev_priv, RING_CTL(engine->mmio_base),
> RING_VALID, RING_VALID,
> - 50)) {
> + true, 50)) {
> DRM_ERROR("%s initialization failed "
> "ctl %08x (valid? %d) head %08x [%08x] tail %08x [%08x] start %08x [expected %08x]\n",
> engine->name,
> @@ -1733,7 +1733,7 @@ static void gen6_bsd_submit_request(struct drm_i915_gem_request *request)
> GEN6_BSD_SLEEP_PSMI_CONTROL,
> GEN6_BSD_SLEEP_INDICATOR,
> 0,
> - 50))
> + true, 50))
> DRM_ERROR("timed out waiting for the BSD ring to wake up\n");
>
> /* Now that the ring is fully powered up, update the tail */
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index bcabf54..324a758 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1537,7 +1537,7 @@ static int gen6_hw_domain_reset(struct drm_i915_private *dev_priv,
> /* Spin waiting for the device to ack the reset requests */
> return intel_wait_for_register_fw(dev_priv,
> GEN6_GDRST, hw_domain_mask, 0,
> - 500);
> + true, 500);
> }
>
> /**
> @@ -1590,6 +1590,7 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv,
> * @reg: the register to read
> * @mask: mask to apply to register value
> * @value: expected value
> + * @is_fast: hint that it is expected to be a fast match
> * @timeout_ms: timeout in millisecond
> *
> * This routine waits until the target register @reg contains the expected
> @@ -1610,10 +1611,15 @@ int intel_wait_for_register_fw(struct drm_i915_private *dev_priv,
> i915_reg_t reg,
> const u32 mask,
> const u32 value,
> + bool is_fast,
> const unsigned int timeout_ms)
> {
> #define done ((I915_READ_FW(reg) & mask) == value)
> - int ret = wait_for_us(done, 2);
> + int ret;
> + if (is_fast)
> + ret = wait_for_us(done, 2);
> + else
> + ret = wait_for_us(done, 10);
> if (ret)
> ret = wait_for(done, timeout_ms);
> return ret;
> @@ -1670,7 +1676,7 @@ static int gen8_request_engine_reset(struct intel_engine_cs *engine)
> RING_RESET_CTL(engine->mmio_base),
> RESET_CTL_READY_TO_RESET,
> RESET_CTL_READY_TO_RESET,
> - 700);
> + true, 700);
> if (ret)
> DRM_ERROR("%s: reset request timeout\n", engine->name);
>
>
I would recommend a different solution here.
Rename and change the prototype of intel_wait_for_register_fw to:
int __intel_wait_for_register_fw(dev_priv, reg, mask, vlaue, fast_timeout_us, timeout_ms)
{
..existing function body, just replace "2" with fast_timeout_us...
}
int intel_wait_for_register_fw(dev_priv, reg, mask, vlaue, timeout_ms
{
return __intel_wait_for_register_fw(dev_priv, reg, mask, value, 2, timeout_ms);
}
And use the __ version from the GuC code.
There is no churn elsewhere in the code like this and it is also
more flexible for potential other new users.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
