[PATCH 18/18] drm/i915/ddi: prefer read_poll_timeout() over readx_poll_timeout()

[Jani Nikula]

On Fri, 27 Jun 2025, Imre Deak <imre.deak at intel.com> wrote:
> On Fri, Jun 27, 2025 at 02:36:32PM +0300, Jani Nikula wrote:
>> Unify on using read_poll_timeout() 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.
>> 
>> Cc: Imre Deak <imre.deak at intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_ddi.c | 33 +++++++++---------------
>>  1 file changed, 12 insertions(+), 21 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index 0405396c7750..fc4587311607 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -2339,34 +2339,25 @@ 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 = read_poll_timeout(drm_dp_dpcd_readb, err,
>
> drm_dp_dpcd_read_byte()? With that it looks ok:

Oh, yes.

>
> Reviewed-by: Imre Deak <imre.deak at intel.com>
>
>> +				err || (status & mask),
>> +				10 * 1000, 200 * 1000, false,
>
> Nit: there's also USEC_PER_MSEC.

I considered that, and decided the straight 1000 is more obvious.

BR,
Jani.

>
>> +				aux, DP_FEC_STATUS, &status);
>>  
>> -	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;
>> -- 
>> 2.39.5
>> 

-- 
Jani Nikula, Intel