Re: [PATCH v2 03/11] drm/i915/crt: drop a bunch of unnecessary register variables

[Andrzej Hajda <andrzej.hajda@xxxxxxxxx>]

On 07.12.2022 18:17, Jani Nikula wrote:
> There's no need to save the register offsets. Drop the variables.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>

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

Regards
Andrzej

> ---
>   drivers/gpu/drm/i915/display/intel_crt.c | 39 +++++++++---------------
>   1 file changed, 15 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> index 797ad9489f7e..ed94ba5c0302 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -689,23 +689,14 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   	u32 vsample;
>   	u32 vblank, vblank_start, vblank_end;
>   	u32 dsl;
> -	i915_reg_t bclrpat_reg, vtotal_reg,
> -		vblank_reg, vsync_reg, pipeconf_reg, pipe_dsl_reg;
>   	u8 st00;
>   	enum drm_connector_status status;
>   
>   	drm_dbg_kms(&dev_priv->drm, "starting load-detect on CRT\n");
>   
> -	bclrpat_reg = BCLRPAT(pipe);
> -	vtotal_reg = VTOTAL(pipe);
> -	vblank_reg = VBLANK(pipe);
> -	vsync_reg = VSYNC(pipe);
> -	pipeconf_reg = PIPECONF(pipe);
> -	pipe_dsl_reg = PIPEDSL(pipe);
> -
> -	save_bclrpat = intel_uncore_read(uncore, bclrpat_reg);
> -	save_vtotal = intel_uncore_read(uncore, vtotal_reg);
> -	vblank = intel_uncore_read(uncore, vblank_reg);
> +	save_bclrpat = intel_uncore_read(uncore, BCLRPAT(pipe));
> +	save_vtotal = intel_uncore_read(uncore, VTOTAL(pipe));
> +	vblank = intel_uncore_read(uncore, VBLANK(pipe));
>   
>   	vtotal = ((save_vtotal >> 16) & 0xfff) + 1;
>   	vactive = (save_vtotal & 0x7ff) + 1;
> @@ -714,14 +705,14 @@ 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_reg, 0x500050);
> +	intel_uncore_write(uncore, BCLRPAT(pipe), 0x500050);
>   
>   	if (DISPLAY_VER(dev_priv) != 2) {
> -		u32 pipeconf = intel_uncore_read(uncore, pipeconf_reg);
> +		u32 pipeconf = intel_uncore_read(uncore, PIPECONF(pipe));
>   		intel_uncore_write(uncore,
> -				   pipeconf_reg,
> +				   PIPECONF(pipe),
>   				   pipeconf | PIPECONF_FORCE_BORDER);
> -		intel_uncore_posting_read(uncore, pipeconf_reg);
> +		intel_uncore_posting_read(uncore, 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));
> @@ -730,7 +721,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   			connector_status_connected :
>   			connector_status_disconnected;
>   
> -		intel_uncore_write(uncore, pipeconf_reg, pipeconf);
> +		intel_uncore_write(uncore, PIPECONF(pipe), pipeconf);
>   	} else {
>   		bool restore_vblank = false;
>   		int count, detect;
> @@ -740,12 +731,12 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   		* Yes, this will flicker
>   		*/
>   		if (vblank_start <= vactive && vblank_end >= vtotal) {
> -			u32 vsync = intel_de_read(dev_priv, vsync_reg);
> +			u32 vsync = intel_de_read(dev_priv, VSYNC(pipe));
>   			u32 vsync_start = (vsync & 0xffff) + 1;
>   
>   			vblank_start = vsync_start;
>   			intel_uncore_write(uncore,
> -					   vblank_reg,
> +					   VBLANK(pipe),
>   					   (vblank_start - 1) |
>   					   ((vblank_end - 1) << 16));
>   			restore_vblank = true;
> @@ -759,9 +750,9 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   		/*
>   		 * Wait for the border to be displayed
>   		 */
> -		while (intel_uncore_read(uncore, pipe_dsl_reg) >= vactive)
> +		while (intel_uncore_read(uncore, PIPEDSL(pipe)) >= vactive)
>   			;
> -		while ((dsl = intel_uncore_read(uncore, pipe_dsl_reg)) <=
> +		while ((dsl = intel_uncore_read(uncore, PIPEDSL(pipe))) <=
>   		       vsample)
>   			;
>   		/*
> @@ -775,11 +766,11 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   			st00 = intel_uncore_read8(uncore, _VGA_MSR_WRITE);
>   			if (st00 & (1 << 4))
>   				detect++;
> -		} while ((intel_uncore_read(uncore, pipe_dsl_reg) == dsl));
> +		} while ((intel_uncore_read(uncore, PIPEDSL(pipe)) == dsl));
>   
>   		/* restore vblank if necessary */
>   		if (restore_vblank)
> -			intel_uncore_write(uncore, vblank_reg, vblank);
> +			intel_uncore_write(uncore, 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,
> @@ -792,7 +783,7 @@ intel_crt_load_detect(struct intel_crt *crt, u32 pipe)
>   	}
>   
>   	/* Restore previous settings */
> -	intel_uncore_write(uncore, bclrpat_reg, save_bclrpat);
> +	intel_uncore_write(uncore, BCLRPAT(pipe), save_bclrpat);
>   
>   	return status;
>   }