Re: [PATCH 06/17] drm/i915/ddi: Simplify the port disabling via DDI_BUF_CTL

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

On Wed, 29 Jan 2025, Imre Deak <imre.deak@intel.com> wrote:
> A port can be disabled only via a modeset (or during HW state
> sanitization) when the port is enabled. Thus it's not required to check
> the port's enabled state before disabling it. In any case if the port
> happened to be disabled, the following disabling would be just a nop and
> waiting for the buffer's idle state should succeed. Simplify the
> disabling sequence accordingly.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 23 +++++------------------
>  1 file changed, 5 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 3138dc4034797..24c56d2aa5f31 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3062,17 +3062,12 @@ static void mtl_disable_ddi_buf(struct intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	enum port port = encoder->port;
> -	u32 val;
>  
>  	/* 3.b Clear DDI_CTL_DE Enable to 0. */
> -	val = intel_de_read(dev_priv, DDI_BUF_CTL(port));
> -	if (val & DDI_BUF_CTL_ENABLE) {
> -		val &= ~DDI_BUF_CTL_ENABLE;
> -		intel_de_write(dev_priv, DDI_BUF_CTL(port), val);
> +	intel_de_rmw(dev_priv, DDI_BUF_CTL(port), DDI_BUF_CTL_ENABLE, 0);
>  
> -		/* 3.c Poll for PORT_BUF_CTL Idle Status == 1, timeout after 100us */
> -		mtl_wait_ddi_buf_idle(dev_priv, port);
> -	}
> +	/* 3.c Poll for PORT_BUF_CTL Idle Status == 1, timeout after 100us */
> +	mtl_wait_ddi_buf_idle(dev_priv, port);
>  
>  	/* 3.d Disable D2D Link */
>  	mtl_ddi_disable_d2d_link(encoder);
> @@ -3089,15 +3084,8 @@ static void disable_ddi_buf(struct intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	enum port port = encoder->port;
> -	bool wait = false;
> -	u32 val;
>  
> -	val = intel_de_read(dev_priv, DDI_BUF_CTL(port));
> -	if (val & DDI_BUF_CTL_ENABLE) {
> -		val &= ~DDI_BUF_CTL_ENABLE;
> -		intel_de_write(dev_priv, DDI_BUF_CTL(port), val);
> -		wait = true;
> -	}
> +	intel_de_rmw(dev_priv, DDI_BUF_CTL(port), DDI_BUF_CTL_ENABLE, 0);
>  
>  	if (intel_crtc_has_dp_encoder(crtc_state))
>  		intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state),
> @@ -3105,8 +3093,7 @@ static void disable_ddi_buf(struct intel_encoder *encoder,
>  
>  	intel_ddi_disable_fec(encoder, crtc_state);
>  
> -	if (wait)
> -		intel_wait_ddi_buf_idle(dev_priv, port);
> +	intel_wait_ddi_buf_idle(dev_priv, port);
>  }
>  
>  static void intel_disable_ddi_buf(struct intel_encoder *encoder,

-- 
Jani Nikula, Intel