[Intel-gfx] [PATCH] drm/i915: fix comment on I915_{READ,WRITE}_FW

2016-10-27 Thread Arkadiusz Hiler
Comment mentioned use of intel_uncore_forcewake_irq{unlock, lock}
functions which are nonexistent (and never were).

The description was also incomplete and could cause confusion. Updated
comment is more elaborate on usage and caveats.

v2: mention __locked variant of intel_uncore_forcewake_{get,put} instead
of plain ones

Cc: Chris Wilson 
Cc: Matthew Auld 
Cc: Mika Kuoppala 
Signed-off-by: Arkadiusz Hiler 
---
 drivers/gpu/drm/i915/i915_drv.h | 28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b4cb1f0..e0f3fa4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3840,11 +3840,33 @@ __raw_write(64, q)
 #undef __raw_write
 
 /* These are untraced mmio-accessors that are only valid to be used inside
- * critical sections inside IRQ handlers where forcewake is explicitly
+ * critical sections, such as inside IRQ handlers, where forcewake is 
explicitly
  * controlled.
+ *
  * Think twice, and think again, before using these.
- * Note: Should only be used between intel_uncore_forcewake_irqlock() and
- * intel_uncore_forcewake_irqunlock().
+ *
+ * As an example, these accessors can possibly be used between:
+ *
+ * spin_lock_irq(_priv->uncore.lock);
+ * intel_uncore_forcewake_get__locked();
+ *
+ * and
+ *
+ * intel_uncore_forcewake_put__locked();
+ * spin_unlock_irq(_priv->uncore.lock);
+ *
+ *
+ * Note: some registers may not need forcewake held, so
+ * intel_uncore_forcewake_{get,put} can be omitted, see
+ * intel_uncore_forcewake_for_reg().
+ *
+ * Certain architectures will die if the same cacheline is concurrently 
accessed
+ * by different clients (e.g. Ivybridge). Access to registers should therefore
+ * generally be serialised, by either the dev_priv->uncore.lock or a more
+ * localised lock guarding all access to that bank of registers.
+ *
+ * Code may be serialised by different lock, so immediate
+ * spin_{lock,unlock}_irq() may not be necessary.
  */
 #define I915_READ_FW(reg__) __raw_i915_read32(dev_priv, (reg__))
 #define I915_WRITE_FW(reg__, val__) __raw_i915_write32(dev_priv, (reg__), 
(val__))
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: fix comment on I915_{READ, WRITE}_FW

2016-10-25 Thread Chris Wilson
On Tue, Oct 25, 2016 at 02:15:23PM +0200, Arkadiusz Hiler wrote:
> Comment mentioned use of intel_uncore_forcewake_irq{unlock, lock}
> functions which are nonexistent (and never were).
> 
> The description was also incomplete and could cause confusion. Updated
> comment is more elaborate on usage and caveats.
> 
> Cc: Chris Wilson 
> Cc: Matthew Auld 
> Cc: Mika Kuoppala 
> Signed-off-by: Arkadiusz Hiler 
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 28 +---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b4cb1f0..39238fc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3840,11 +3840,33 @@ __raw_write(64, q)
>  #undef __raw_write
>  
>  /* These are untraced mmio-accessors that are only valid to be used inside
> - * critical sections inside IRQ handlers where forcewake is explicitly
> + * critical sections, such as inside IRQ handlers, where forcewake is 
> explicitly
>   * controlled.
> + *
>   * Think twice, and think again, before using these.
> - * Note: Should only be used between intel_uncore_forcewake_irqlock() and
> - * intel_uncore_forcewake_irqunlock().
> + *
> + * As an example, these accessors can possibly be used between:
> + *
> + * spin_lock_irq(_priv->uncore.lock);
> + * intel_uncore_forcewake_get();

Ugh, intel_uncore_forcewake_get__locked();

> + *
> + * and
> + *
> + * intel_uncore_forcewake_put();

intel_uncore_forcewake_put__locked();

> + * spin_unlock_irq(_priv->uncore.lock);

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


[Intel-gfx] [PATCH] drm/i915: fix comment on I915_{READ,WRITE}_FW

2016-10-25 Thread Arkadiusz Hiler
Comment mentioned use of intel_uncore_forcewake_irq{unlock, lock}
functions which are nonexistent (and never were).

The description was also incomplete and could cause confusion. Updated
comment is more elaborate on usage and caveats.

Cc: Chris Wilson 
Cc: Matthew Auld 
Cc: Mika Kuoppala 
Signed-off-by: Arkadiusz Hiler 
---
 drivers/gpu/drm/i915/i915_drv.h | 28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b4cb1f0..39238fc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3840,11 +3840,33 @@ __raw_write(64, q)
 #undef __raw_write
 
 /* These are untraced mmio-accessors that are only valid to be used inside
- * critical sections inside IRQ handlers where forcewake is explicitly
+ * critical sections, such as inside IRQ handlers, where forcewake is 
explicitly
  * controlled.
+ *
  * Think twice, and think again, before using these.
- * Note: Should only be used between intel_uncore_forcewake_irqlock() and
- * intel_uncore_forcewake_irqunlock().
+ *
+ * As an example, these accessors can possibly be used between:
+ *
+ * spin_lock_irq(_priv->uncore.lock);
+ * intel_uncore_forcewake_get();
+ *
+ * and
+ *
+ * intel_uncore_forcewake_put();
+ * spin_unlock_irq(_priv->uncore.lock);
+ *
+ *
+ * Note: some registers may not need forcewake held, so
+ * intel_uncore_forcewake_{get,put} can be omitted, see
+ * intel_uncore_forcewake_for_reg().
+ *
+ * Certain architectures will die if the same cacheline is concurrently 
accessed
+ * by different clients (e.g. Ivybridge). Access to registers should therefore
+ * generally be serialised, by either the dev_priv->uncore.lock or a more
+ * localised lock guarding all access to that bank of registers.
+ *
+ * Code may be serialised by different lock, so immediate
+ * spin_{lock,unlock}_irq() may not be necessary.
  */
 #define I915_READ_FW(reg__) __raw_i915_read32(dev_priv, (reg__))
 #define I915_WRITE_FW(reg__, val__) __raw_i915_write32(dev_priv, (reg__), 
(val__))
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx