[Intel-gfx] [PATCH 2/4] drm/i915: Convert sandybridge_pcode_*() to use intel_wait_for_register()
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Thu Jun 30 13:04:43 UTC 2016
On 30/06/16 12:30, Chris Wilson wrote:
> We want to replace the inline wait_for() with an out-of-line hybrid
> busy/sleep wait_for() in the hopes of speeding up the communication wit
> the PCode unit.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 43 +++++++++++++++++++++++++++--------------
> 1 file changed, 28 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d7f8ba89d2a5..f84bf253d04a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -7623,46 +7623,59 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val
> {
> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>
> - if (I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) {
> + /* GEN6_PCODE_* are outside of the forcewake domain, we can
> + * use te fw I915_READ variants to reduce the amount of work
> + * required when reading/writing.
> + */
> +
> + if (I915_READ_FW(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) {
> DRM_DEBUG_DRIVER("warning: pcode (read) mailbox access failed\n");
> return -EAGAIN;
> }
>
> - I915_WRITE(GEN6_PCODE_DATA, *val);
> - I915_WRITE(GEN6_PCODE_DATA1, 0);
> - I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
> + I915_WRITE_FW(GEN6_PCODE_DATA, *val);
> + I915_WRITE_FW(GEN6_PCODE_DATA1, 0);
> + I915_WRITE_FW(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
>
> - if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
> - 500)) {
> + if (intel_wait_for_register(dev_priv,
> + GEN6_PCODE_MAILBOX, GEN6_PCODE_READY, 0,
> + 500)) {
Why not the _fw version?
Actually brings me to the point I glanced over in the previous patch -
why would intel_wait_for_register_fw be not suitable for long waits? It
looks like a concern for the calling code grabbing the fw outside it for
a long period and not for the function itself. This call site being a
case in point for that.
> DRM_ERROR("timeout waiting for pcode read (%d) to finish\n", mbox);
> return -ETIMEDOUT;
> }
>
> - *val = I915_READ(GEN6_PCODE_DATA);
> - I915_WRITE(GEN6_PCODE_DATA, 0);
> + *val = I915_READ_FW(GEN6_PCODE_DATA);
> + I915_WRITE_FW(GEN6_PCODE_DATA, 0);
>
> return 0;
> }
>
> -int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u32 mbox, u32 val)
> +int sandybridge_pcode_write(struct drm_i915_private *dev_priv,
> + u32 mbox, u32 val)
> {
> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>
> - if (I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) {
> + /* GEN6_PCODE_* are outside of the forcewake domain, we can
> + * use te fw I915_READ variants to reduce the amount of work
> + * required when reading/writing.
> + */
> +
> + if (I915_READ_FW(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) {
> DRM_DEBUG_DRIVER("warning: pcode (write) mailbox access failed\n");
> return -EAGAIN;
> }
>
> - I915_WRITE(GEN6_PCODE_DATA, val);
> - I915_WRITE(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
> + I915_WRITE_FW(GEN6_PCODE_DATA, val);
> + I915_WRITE_FW(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
>
> - if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY) == 0,
> - 500)) {
> + if (intel_wait_for_register_fw(dev_priv,
> + GEN6_PCODE_MAILBOX, GEN6_PCODE_READY, 0,
> + 500)) {
Here you use the _fw variant.
> DRM_ERROR("timeout waiting for pcode write (%d) to finish\n", mbox);
> return -ETIMEDOUT;
> }
>
> - I915_WRITE(GEN6_PCODE_DATA, 0);
> + I915_WRITE_FW(GEN6_PCODE_DATA, 0);
>
> return 0;
> }
>
Regards,
Tvrtko
More information about the Intel-gfx
mailing list