Re: [Intel-gfx] [PATCH 2/9] drm/i915: remove last traces of I915_READ_FW() and I915_WRITE_FW()

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Nov 12, 2020 at 01:44:35PM +0200, Jani Nikula wrote:
> Good riddance! Remove the macros and their remaining references in
> comments.
> 
> intel_uncore_read_fw() and intel_uncore_write_fw() should be used
> instead.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h     | 29 -----------------------------
>  drivers/gpu/drm/i915/intel_uncore.c |  2 +-
>  drivers/gpu/drm/i915/intel_uncore.h |  2 +-
>  3 files changed, 2 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 15be8debae54..fecb5899cbac 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1978,35 +1978,6 @@ int i915_reg_read_ioctl(struct drm_device *dev, void *data,
>  
>  #define POSTING_READ(reg__)	__I915_REG_OP(posting_read, dev_priv, (reg__))
>  
> -/* These are untraced mmio-accessors that are only valid to be used inside
> - * critical sections, such as inside IRQ handlers, where forcewake is explicitly
> - * controlled.
> - *
> - * Think twice, and think again, before using these.
> - *
> - * As an example, these accessors can possibly be used between:
> - *
> - * spin_lock_irq(&dev_priv->uncore.lock);
> - * intel_uncore_forcewake_get__locked();
> - *
> - * and
> - *
> - * intel_uncore_forcewake_put__locked();
> - * spin_unlock_irq(&dev_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. on 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.
> - */
> -#define I915_READ_FW(reg__) __I915_REG_OP(read_fw, dev_priv, (reg__))
> -#define I915_WRITE_FW(reg__, val__) __I915_REG_OP(write_fw, dev_priv, (reg__), (val__))
> -
>  /* i915_mm.c */
>  int remap_io_mapping(struct vm_area_struct *vma,
>  		     unsigned long addr, unsigned long pfn, unsigned long size,
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 1c14a07eba7d..ef40edfff412 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -2070,7 +2070,7 @@ int i915_reg_read_ioctl(struct drm_device *dev,
>   * This routine waits until the target register @reg contains the expected
>   * @value after applying the @mask, i.e. it waits until ::
>   *
> - *     (I915_READ_FW(reg) & mask) == value
> + *     (intel_uncore_read_fw(uncore, reg) & mask) == value
>   *
>   * Otherwise, the wait will timeout after @slow_timeout_ms milliseconds.
>   * For atomic context @slow_timeout_ms must be zero and @fast_timeout_us
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index bd2467284295..5dcb7f4183b2 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -216,7 +216,7 @@ void intel_uncore_forcewake_flush(struct intel_uncore *uncore,
>  
>  /*
>   * Like above but the caller must manage the uncore.lock itself.
> - * Must be used with I915_READ_FW and friends.
> + * Must be used with intel_uncore_read_fw() and friends.
>   */
>  void intel_uncore_forcewake_get__locked(struct intel_uncore *uncore,
>  					enum forcewake_domains domains);
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx