Re: [PATCH 1/3] drm/i915/de: Introduce intel_de.c and move intel_de_{read, write}8() there

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

On Fri, 13 Mar 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> intel_de_{read,write}() aren't performance critical so having them
> as static inline is pointless. Introduce intel_de.c and move the
> implementation there.

I was surprised only the 8-bit read/write functions were moved. It's
only the 8 in the subject line that conveys that, while the commit
message implies all of them are.

What gets moved when is neither here nor there, I get that this is the
simple change to create the file, and also drops a dependency on
drm_print.h from the header.

But the commit message could be slightly more elaborate here. Can be
fixed whole applying.

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

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile           |  1 +
>  drivers/gpu/drm/i915/display/intel_de.c | 23 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_de.h | 22 +++-------------------
>  drivers/gpu/drm/xe/Makefile             |  1 +
>  4 files changed, 28 insertions(+), 19 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_de.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 7e9d9b666511..099f7b68bb30 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -253,6 +253,7 @@ i915-y += \
>  	display/intel_crtc_state_dump.o \
>  	display/intel_cursor.o \
>  	display/intel_dbuf_bw.o \
> +	display/intel_de.o \
>  	display/intel_display.o \
>  	display/intel_display_conversion.o \
>  	display/intel_display_driver.o \
> diff --git a/drivers/gpu/drm/i915/display/intel_de.c b/drivers/gpu/drm/i915/display/intel_de.c
> new file mode 100644
> index 000000000000..5348c1d51eb8
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_de.c
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#include <drm/drm_print.h>
> +
> +#include "intel_de.h"
> +
> +u8 intel_de_read8(struct intel_display *display, i915_reg_t reg)
> +{
> +	/* this is only used on VGA registers (possible on pre-g4x) */
> +	drm_WARN_ON(display->drm, DISPLAY_VER(display) >= 5 || display->platform.g4x);
> +
> +	return intel_uncore_read8(__to_uncore(display), reg);
> +}
> +
> +void intel_de_write8(struct intel_display *display, i915_reg_t reg, u8 val)
> +{
> +	drm_WARN_ON(display->drm, DISPLAY_VER(display) >= 5 || display->platform.g4x);
> +
> +	intel_uncore_write8(__to_uncore(display), reg, val);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_de.h b/drivers/gpu/drm/i915/display/intel_de.h
> index f30f3f8ebee1..8ca5904ba84e 100644
> --- a/drivers/gpu/drm/i915/display/intel_de.h
> +++ b/drivers/gpu/drm/i915/display/intel_de.h
> @@ -6,8 +6,6 @@
>  #ifndef __INTEL_DE_H__
>  #define __INTEL_DE_H__
>  
> -#include <drm/drm_print.h>
> -
>  #include "intel_display_core.h"
>  #include "intel_dmc_wl.h"
>  #include "intel_dsb.h"
> @@ -19,6 +17,9 @@ static inline struct intel_uncore *__to_uncore(struct intel_display *display)
>  	return to_intel_uncore(display->drm);
>  }
>  
> +u8 intel_de_read8(struct intel_display *display, i915_reg_t reg);
> +void intel_de_write8(struct intel_display *display, i915_reg_t reg, u8 val);
> +
>  static inline u32
>  intel_de_read(struct intel_display *display, i915_reg_t reg)
>  {
> @@ -33,23 +34,6 @@ intel_de_read(struct intel_display *display, i915_reg_t reg)
>  	return val;
>  }
>  
> -static inline u8
> -intel_de_read8(struct intel_display *display, i915_reg_t reg)
> -{
> -	/* this is only used on VGA registers (possible on pre-g4x) */
> -	drm_WARN_ON(display->drm, DISPLAY_VER(display) >= 5 || display->platform.g4x);
> -
> -	return intel_uncore_read8(__to_uncore(display), reg);
> -}
> -
> -static inline void
> -intel_de_write8(struct intel_display *display, i915_reg_t reg, u8 val)
> -{
> -	drm_WARN_ON(display->drm, DISPLAY_VER(display) >= 5 || display->platform.g4x);
> -
> -	intel_uncore_write8(__to_uncore(display), reg, val);
> -}
> -
>  static inline u64
>  intel_de_read64_2x32(struct intel_display *display,
>  		     i915_reg_t lower_reg, i915_reg_t upper_reg)
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 50608312bc66..0399a5f9a107 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -251,6 +251,7 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \
>  	i915-display/intel_dbuf_bw.o \
>  	i915-display/intel_ddi.o \
>  	i915-display/intel_ddi_buf_trans.o \
> +	i915-display/intel_de.o \
>  	i915-display/intel_display.o \
>  	i915-display/intel_display_conversion.o \
>  	i915-display/intel_display_device.o \

-- 
Jani Nikula, Intel