Re: [PATCH 09/12] drm/i915/de: Add a simple intel_de_read64_2x32()

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 25 Mar 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> intel_de_read64_2x32_volatile() is a complex beast because
> it needs to deal with volatile register values. For simpler
> cases we can simply do a pair normal intel_de_read()s.
>
> My main reason for hating overuse of intel_de_read64_2x32_volatile()
> is that it makes register tracepoints confusing. It always
> does three accesses in the somewhat weird udw,ldw,udw order,
> confusing the reader of the trace. Much more clear if we just
> observe the two reads in the natural little endian order.
>
> We also have no non-volatile use case where the LDW and UDW
> are stored in non-consecutive registers, so we can just pass
> along a single register offset.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_de.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_de.h b/drivers/gpu/drm/i915/display/intel_de.h
> index 295e7176b732..624a0627b95c 100644
> --- a/drivers/gpu/drm/i915/display/intel_de.h
> +++ b/drivers/gpu/drm/i915/display/intel_de.h
> @@ -53,6 +53,19 @@ intel_de_read64_2x32_volatile(struct intel_display *display,
>  	return val;
>  }
>  
> +static inline u64
> +intel_de_read64_2x32(struct intel_display *display,
> +		     i915_reg_t reg)

Could fit on one line, matter of taste I guess.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> +{
> +	i915_reg_t upper_reg = _MMIO(i915_mmio_reg_offset(reg) + 4);
> +	u32 lower, upper;
> +
> +	lower = intel_de_read(display, reg);
> +	upper = intel_de_read(display, upper_reg);
> +
> +	return (u64)upper << 32 | lower;
> +}
> +
>  static inline void
>  intel_de_posting_read(struct intel_display *display, i915_reg_t reg)
>  {

-- 
Jani Nikula, Intel