Re: [PATCH v3 6/7] drm/i915/de: allow intel_display and drm_i915_private for de functions

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

On Tue, Apr 09, 2024 at 03:26:48PM +0300, Jani Nikula wrote:
> It would be too much noise to convert the intel_de_* functions from
> using struct drm_i915_private to struct intel_display all at once. Add
> generic wrappers using __to_intel_display() to accept both.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> ---
> 
> This was done using a cocci + shell script combo.

the conversion below seems sane.
would you mind sharing the scripts in the commit message,
so scripts could be used when porting this patch to other
trees?

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

> ---
>  drivers/gpu/drm/i915/display/intel_de.h | 102 +++++++++++++++---------
>  1 file changed, 64 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_de.h b/drivers/gpu/drm/i915/display/intel_de.h
> index ba7a1c6ebc2a..a08f8ef630f3 100644
> --- a/drivers/gpu/drm/i915/display/intel_de.h
> +++ b/drivers/gpu/drm/i915/display/intel_de.h
> @@ -10,80 +10,101 @@
>  #include "i915_trace.h"
>  #include "intel_uncore.h"
>  
> +static inline struct intel_uncore *__to_uncore(struct intel_display *display)
> +{
> +	return &to_i915(display->drm)->uncore;
> +}
> +
>  static inline u32
> -intel_de_read(struct drm_i915_private *i915, i915_reg_t reg)
> +__intel_de_read(struct intel_display *display, i915_reg_t reg)
>  {
> -	return intel_uncore_read(&i915->uncore, reg);
> +	return intel_uncore_read(__to_uncore(display), reg);
>  }
> +#define intel_de_read(p,...) __intel_de_read(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline u8
> -intel_de_read8(struct drm_i915_private *i915, i915_reg_t reg)
> +__intel_de_read8(struct intel_display *display, i915_reg_t reg)
>  {
> -	return intel_uncore_read8(&i915->uncore, reg);
> +	return intel_uncore_read8(__to_uncore(display), reg);
>  }
> +#define intel_de_read8(p,...) __intel_de_read8(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline u64
> -intel_de_read64_2x32(struct drm_i915_private *i915,
> -		     i915_reg_t lower_reg, i915_reg_t upper_reg)
> +__intel_de_read64_2x32(struct intel_display *display,
> +		       i915_reg_t lower_reg, i915_reg_t upper_reg)
>  {
> -	return intel_uncore_read64_2x32(&i915->uncore, lower_reg, upper_reg);
> +	return intel_uncore_read64_2x32(__to_uncore(display), lower_reg,
> +					upper_reg);
>  }
> +#define intel_de_read64_2x32(p,...) __intel_de_read64_2x32(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline void
> -intel_de_posting_read(struct drm_i915_private *i915, i915_reg_t reg)
> +__intel_de_posting_read(struct intel_display *display, i915_reg_t reg)
>  {
> -	intel_uncore_posting_read(&i915->uncore, reg);
> +	intel_uncore_posting_read(__to_uncore(display), reg);
>  }
> +#define intel_de_posting_read(p,...) __intel_de_posting_read(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline void
> -intel_de_write(struct drm_i915_private *i915, i915_reg_t reg, u32 val)
> +__intel_de_write(struct intel_display *display, i915_reg_t reg, u32 val)
>  {
> -	intel_uncore_write(&i915->uncore, reg, val);
> +	intel_uncore_write(__to_uncore(display), reg, val);
>  }
> +#define intel_de_write(p,...) __intel_de_write(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline u32
> -intel_de_rmw(struct drm_i915_private *i915, i915_reg_t reg, u32 clear, u32 set)
> +__intel_de_rmw(struct intel_display *display, i915_reg_t reg, u32 clear,
> +	       u32 set)
>  {
> -	return intel_uncore_rmw(&i915->uncore, reg, clear, set);
> +	return intel_uncore_rmw(__to_uncore(display), reg, clear, set);
>  }
> +#define intel_de_rmw(p,...) __intel_de_rmw(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline int
> -intel_de_wait(struct drm_i915_private *i915, i915_reg_t reg,
> -	      u32 mask, u32 value, unsigned int timeout)
> +__intel_de_wait(struct intel_display *display, i915_reg_t reg,
> +		u32 mask, u32 value, unsigned int timeout)
>  {
> -	return intel_wait_for_register(&i915->uncore, reg, mask, value, timeout);
> +	return intel_wait_for_register(__to_uncore(display), reg, mask, value,
> +				       timeout);
>  }
> +#define intel_de_wait(p,...) __intel_de_wait(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline int
> -intel_de_wait_fw(struct drm_i915_private *i915, i915_reg_t reg,
> -		 u32 mask, u32 value, unsigned int timeout)
> +__intel_de_wait_fw(struct intel_display *display, i915_reg_t reg,
> +		   u32 mask, u32 value, unsigned int timeout)
>  {
> -	return intel_wait_for_register_fw(&i915->uncore, reg, mask, value, timeout);
> +	return intel_wait_for_register_fw(__to_uncore(display), reg, mask,
> +					  value, timeout);
>  }
> +#define intel_de_wait_fw(p,...) __intel_de_wait_fw(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline int
> -intel_de_wait_custom(struct drm_i915_private *i915, i915_reg_t reg,
> -		     u32 mask, u32 value,
> -		     unsigned int fast_timeout_us,
> -		     unsigned int slow_timeout_ms, u32 *out_value)
> +__intel_de_wait_custom(struct intel_display *display, i915_reg_t reg,
> +		       u32 mask, u32 value,
> +		       unsigned int fast_timeout_us,
> +		       unsigned int slow_timeout_ms, u32 *out_value)
>  {
> -	return __intel_wait_for_register(&i915->uncore, reg, mask, value,
> +	return __intel_wait_for_register(__to_uncore(display), reg, mask,
> +					 value,
>  					 fast_timeout_us, slow_timeout_ms, out_value);
>  }
> +#define intel_de_wait_custom(p,...) __intel_de_wait_custom(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline int
> -intel_de_wait_for_set(struct drm_i915_private *i915, i915_reg_t reg,
> -		      u32 mask, unsigned int timeout)
> +__intel_de_wait_for_set(struct intel_display *display, i915_reg_t reg,
> +			u32 mask, unsigned int timeout)
>  {
> -	return intel_de_wait(i915, reg, mask, mask, timeout);
> +	return intel_de_wait(display, reg, mask, mask, timeout);
>  }
> +#define intel_de_wait_for_set(p,...) __intel_de_wait_for_set(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline int
> -intel_de_wait_for_clear(struct drm_i915_private *i915, i915_reg_t reg,
> -			u32 mask, unsigned int timeout)
> +__intel_de_wait_for_clear(struct intel_display *display, i915_reg_t reg,
> +			  u32 mask, unsigned int timeout)
>  {
> -	return intel_de_wait(i915, reg, mask, 0, timeout);
> +	return intel_de_wait(display, reg, mask, 0, timeout);
>  }
> +#define intel_de_wait_for_clear(p,...) __intel_de_wait_for_clear(__to_intel_display(p), __VA_ARGS__)
>  
>  /*
>   * Unlocked mmio-accessors, think carefully before using these.
> @@ -94,33 +115,38 @@ intel_de_wait_for_clear(struct drm_i915_private *i915, i915_reg_t reg,
>   * a more localised lock guarding all access to that bank of registers.
>   */
>  static inline u32
> -intel_de_read_fw(struct drm_i915_private *i915, i915_reg_t reg)
> +__intel_de_read_fw(struct intel_display *display, i915_reg_t reg)
>  {
>  	u32 val;
>  
> -	val = intel_uncore_read_fw(&i915->uncore, reg);
> +	val = intel_uncore_read_fw(__to_uncore(display), reg);
>  	trace_i915_reg_rw(false, reg, val, sizeof(val), true);
>  
>  	return val;
>  }
> +#define intel_de_read_fw(p,...) __intel_de_read_fw(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline void
> -intel_de_write_fw(struct drm_i915_private *i915, i915_reg_t reg, u32 val)
> +__intel_de_write_fw(struct intel_display *display, i915_reg_t reg, u32 val)
>  {
>  	trace_i915_reg_rw(true, reg, val, sizeof(val), true);
> -	intel_uncore_write_fw(&i915->uncore, reg, val);
> +	intel_uncore_write_fw(__to_uncore(display), reg, val);
>  }
> +#define intel_de_write_fw(p,...) __intel_de_write_fw(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline u32
> -intel_de_read_notrace(struct drm_i915_private *i915, i915_reg_t reg)
> +__intel_de_read_notrace(struct intel_display *display, i915_reg_t reg)
>  {
> -	return intel_uncore_read_notrace(&i915->uncore, reg);
> +	return intel_uncore_read_notrace(__to_uncore(display), reg);
>  }
> +#define intel_de_read_notrace(p,...) __intel_de_read_notrace(__to_intel_display(p), __VA_ARGS__)
>  
>  static inline void
> -intel_de_write_notrace(struct drm_i915_private *i915, i915_reg_t reg, u32 val)
> +__intel_de_write_notrace(struct intel_display *display, i915_reg_t reg,
> +			 u32 val)
>  {
> -	intel_uncore_write_notrace(&i915->uncore, reg, val);
> +	intel_uncore_write_notrace(__to_uncore(display), reg, val);
>  }
> +#define intel_de_write_notrace(p,...) __intel_de_write_notrace(__to_intel_display(p), __VA_ARGS__)
>  
>  #endif /* __INTEL_DE_H__ */
> -- 
> 2.39.2
>