Re: [PATCH 19/20] drm/i915/ddi: prefer poll_timeout_us() over readx_poll_timeout()

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Thu, 2025-08-28 at 15:21 +0300, Jani Nikula wrote:
> Unify on using poll_timeout_us() throughout instead of mixing with
> readx_poll_timeout(). While the latter can be ever so slightly
> simpler,
> they are both complicated enough that it's better to unify on one
> approach only.
> 
> While at it, better separate the handling of error returns from
> drm_dp_dpcd_readb() and the actual status byte. This is best achieved
> by
> inlining the read_fec_detected_status() function, and switching to
> drm_dp_dpcd_read_byte().
> 
> v2: Use drm_dp_dpcd_read_byte() (Imre)
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 32 ++++++++--------------
> --
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4e4ea3a0ff83..46017091bb0b 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2340,34 +2340,24 @@ static void
> intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
>  		drm_dbg_kms(display->drm, "Failed to clear FEC
> detected flags\n");
>  }
>  
> -static int read_fec_detected_status(struct drm_dp_aux *aux)
> -{
> -	int ret;
> -	u8 status;
> -
> -	ret = drm_dp_dpcd_readb(aux, DP_FEC_STATUS, &status);
> -	if (ret < 0)
> -		return ret;
> -
> -	return status;
> -}
> -
>  static int wait_for_fec_detected(struct drm_dp_aux *aux, bool
> enabled)
>  {
>  	struct intel_display *display = to_intel_display(aux-
> >drm_dev);
>  	int mask = enabled ? DP_FEC_DECODE_EN_DETECTED :
> DP_FEC_DECODE_DIS_DETECTED;
> -	int status;
> -	int err;
> +	u8 status = 0;
> +	int ret, err;
>  
> -	err = readx_poll_timeout(read_fec_detected_status, aux,
> status,
> -				 status & mask || status < 0,
> -				 10000, 200000);
> +	ret = poll_timeout_us(err = drm_dp_dpcd_read_byte(aux,
> DP_FEC_STATUS, &status),
> +			      err || (status & mask),
> +			      10 * 1000, 200 * 1000, false);
>  
> -	if (err || status < 0) {
> +	/* Either can be non-zero, but not both */
> +	ret = ret ?: err;
> +	if (ret) {
>  		drm_dbg_kms(display->drm,
> -			    "Failed waiting for FEC %s to get
> detected: %d (status %d)\n",
> -			    str_enabled_disabled(enabled), err,
> status);
> -		return err ? err : status;
> +			    "Failed waiting for FEC %s to get
> detected: %d (status 0x%02x)\n",
> +			    str_enabled_disabled(enabled), ret,
> status);
> +		return ret;
>  	}
>  
>  	return 0;