Re: [Intel-gfx] [PATCH v2 04/11] drm/i915/crt: switch to intel_de_* register accessors in display code

[Andrzej Hajda <andrzej.hajda@intel.com>]

On 07.12.2022 18:17, Jani Nikula wrote:
> Avoid direct uncore use in display code.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Just curious if compiler is able to keep the code size with such changes.

Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

Regards
Andrzej

> ---
>   drivers/gpu/drm/i915/display/intel_crt.c | 42 +++++++++++-------------
>   1 file changed, 19 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> index ed94ba5c0302..7267ffc7f539 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -682,7 +682,6 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   {
>   	struct drm_device *dev = crt->base.base.dev;
>   	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_uncore *uncore = &dev_priv->uncore;
>   	u32 save_bclrpat;
>   	u32 save_vtotal;
>   	u32 vtotal, vactive;
> @@ -694,9 +693,9 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   
>   	drm_dbg_kms(&dev_priv->drm, "starting load-detect on CRT\n");
>   
> -	save_bclrpat = intel_uncore_read(uncore, BCLRPAT(pipe));
> -	save_vtotal = intel_uncore_read(uncore, VTOTAL(pipe));
> -	vblank = intel_uncore_read(uncore, VBLANK(pipe));
> +	save_bclrpat = intel_de_read(dev_priv, BCLRPAT(pipe));
> +	save_vtotal = intel_de_read(dev_priv, VTOTAL(pipe));
> +	vblank = intel_de_read(dev_priv, VBLANK(pipe));
>   
>   	vtotal = ((save_vtotal >> 16) & 0xfff) + 1;
>   	vactive = (save_vtotal & 0x7ff) + 1;
> @@ -705,23 +704,23 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   	vblank_end = ((vblank >> 16) & 0xfff) + 1;
>   
>   	/* Set the border color to purple. */
> -	intel_uncore_write(uncore, BCLRPAT(pipe), 0x500050);
> +	intel_de_write(dev_priv, BCLRPAT(pipe), 0x500050);
>   
>   	if (DISPLAY_VER(dev_priv) != 2) {
> -		u32 pipeconf = intel_uncore_read(uncore, PIPECONF(pipe));
> -		intel_uncore_write(uncore,
> -				   PIPECONF(pipe),
> -				   pipeconf | PIPECONF_FORCE_BORDER);
> -		intel_uncore_posting_read(uncore, PIPECONF(pipe));
> +		u32 pipeconf = intel_de_read(dev_priv, PIPECONF(pipe));
> +
> +		intel_de_write(dev_priv, PIPECONF(pipe),
> +			       pipeconf | PIPECONF_FORCE_BORDER);
> +		intel_de_posting_read(dev_priv, PIPECONF(pipe));
>   		/* Wait for next Vblank to substitue
>   		 * border color for Color info */
>   		intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(dev_priv, pipe));
> -		st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
> +		st00 = intel_de_read8(dev_priv, _VGA_MSR_WRITE);
>   		status = ((st00 & (1 << 4)) != 0) ?
>   			connector_status_connected :
>   			connector_status_disconnected;
>   
> -		intel_uncore_write(uncore, PIPECONF(pipe), pipeconf);
> +		intel_de_write(dev_priv, PIPECONF(pipe), pipeconf);
>   	} else {
>   		bool restore_vblank = false;
>   		int count, detect;
> @@ -735,10 +734,8 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   			u32 vsync_start = (vsync & 0xffff) + 1;
>   
>   			vblank_start = vsync_start;
> -			intel_uncore_write(uncore,
> -					   VBLANK(pipe),
> -					   (vblank_start - 1) |
> -					   ((vblank_end - 1) << 16));
> +			intel_de_write(dev_priv, VBLANK(pipe),
> +				       (vblank_start - 1) | ((vblank_end - 1) << 16));
>   			restore_vblank = true;
>   		}
>   		/* sample in the vertical border, selecting the larger one */
> @@ -750,10 +747,9 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   		/*
>   		 * Wait for the border to be displayed
>   		 */
> -		while (intel_uncore_read(uncore, PIPEDSL(pipe)) >= vactive)
> +		while (intel_de_read(dev_priv, PIPEDSL(pipe)) >= vactive)
>   			;
> -		while ((dsl = intel_uncore_read(uncore, PIPEDSL(pipe))) <=
> -		       vsample)
> +		while ((dsl = intel_de_read(dev_priv, PIPEDSL(pipe))) <= vsample)
>   			;
>   		/*
>   		 * Watch ST00 for an entire scanline
> @@ -763,14 +759,14 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   		do {
>   			count++;
>   			/* Read the ST00 VGA status register */
> -			st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
> +			st00 = intel_de_read8(dev_priv, _VGA_MSR_WRITE);
>   			if (st00 & (1 << 4))
>   				detect++;
> -		} while ((intel_uncore_read(uncore, PIPEDSL(pipe)) == dsl));
> +		} while ((intel_de_read(dev_priv, PIPEDSL(pipe)) == dsl));
>   
>   		/* restore vblank if necessary */
>   		if (restore_vblank)
> -			intel_uncore_write(uncore, VBLANK(pipe), vblank);
> +			intel_de_write(dev_priv, VBLANK(pipe), vblank);
>   		/*
>   		 * If more than 3/4 of the scanline detected a monitor,
>   		 * then it is assumed to be present. This works even on i830,
> @@ -783,7 +779,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   	}
>   
>   	/* Restore previous settings */
> -	intel_uncore_write(uncore, BCLRPAT(pipe), save_bclrpat);
> +	intel_de_write(dev_priv, BCLRPAT(pipe), save_bclrpat);
>   
>   	return status;
>   }