RE: [PATCH 02/11] drm/i915/lspcon: switch to drm_dp_dpcd_{read_byte, write_byte, write_data}

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

On Wed, 16 Sep 2026, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote:
>> Subject: [PATCH 02/11] drm/i915/lspcon: switch to drm_dp_dpcd_{read_byte,
>> write_byte, write_data}
>> 
>> Switch to the modern DPCD access functions that return negative error codes
>> on errors, -EIO for incomplete access, and 0 on success.
>> 
>> Use 1-byte accessors where sensible. Use local ret variables for the return value
>> instead of checking inline.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_lspcon.c | 34 +++++++++------------
>>  1 file changed, 15 insertions(+), 19 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c
>> b/drivers/gpu/drm/i915/display/intel_lspcon.c
>> index b306bf5f6130..07b5a9d010ff 100644
>> --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
>> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
>> @@ -137,9 +137,7 @@ bool intel_lspcon_detect_hdr_capability(struct
>> intel_digital_port *dig_port)
>>  	u8 hdr_caps;
>>  	int ret;
>> 
>> -	ret = drm_dp_dpcd_read(&intel_dp->aux, get_hdr_status_reg(lspcon),
>> -			       &hdr_caps, 1);
>> -
>> +	ret = drm_dp_dpcd_read_byte(&intel_dp->aux,
>> +get_hdr_status_reg(lspcon), &hdr_caps);
>>  	if (ret < 0) {
>>  		drm_dbg_kms(display->drm, "HDR capability detection
>> failed\n");
>>  		lspcon->hdr_supported = false;
>> @@ -244,10 +242,11 @@ static bool lspcon_wake_native_aux_ch(struct
>> intel_lspcon *lspcon)  {
>>  	struct intel_dp *intel_dp = lspcon_to_intel_dp(lspcon);
>>  	struct intel_display *display = to_intel_display(intel_dp);
>> +	int ret;
>>  	u8 rev;
>> 
>> -	if (drm_dp_dpcd_readb(&lspcon_to_intel_dp(lspcon)->aux,
>> DP_DPCD_REV,
>> -			      &rev) != 1) {
>> +	ret = drm_dp_dpcd_read_byte(&lspcon_to_intel_dp(lspcon)->aux,
>> DP_DPCD_REV, &rev);
>> +	if (ret < 0) {
>>  		drm_dbg_kms(display->drm, "Native AUX CH down\n");
>
> I was wondering now that we have ret should we add it in the debug message using ERR_PTR()
> Same for all the changes done below ?

Feels like the changes are pretty heavy already, maybe that should be
another series on top.

BR,
Jani.

>
> Regards,
> Suraj Kandpal
>
>>  		return false;
>>  	}
>> @@ -331,15 +330,14 @@ static bool lspcon_parade_fw_ready(struct
>> drm_dp_aux *aux)  {
>>  	u8 avi_if_ctrl;
>>  	u8 retry;
>> -	ssize_t ret;
>> +	int ret;
>> 
>>  	/* Check if LSPCON FW is ready for data */
>>  	for (retry = 0; retry < 5; retry++) {
>>  		if (retry)
>>  			usleep_range(200, 300);
>> 
>> -		ret = drm_dp_dpcd_read(aux, LSPCON_PARADE_AVI_IF_CTRL,
>> -				       &avi_if_ctrl, 1);
>> +		ret = drm_dp_dpcd_read_byte(aux,
>> LSPCON_PARADE_AVI_IF_CTRL,
>> +&avi_if_ctrl);
>>  		if (ret < 0) {
>>  			drm_err(aux->drm_dev, "Failed to read AVI IF
>> control\n");
>>  			return false;
>> @@ -371,7 +369,7 @@ static bool
>> _lspcon_parade_write_infoframe_blocks(struct drm_dp_aux *aux,
>> 
>>  		reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
>>  		data = avi_buf + block_count * 8;
>> -		ret = drm_dp_dpcd_write(aux, reg, data, 8);
>> +		ret = drm_dp_dpcd_write_data(aux, reg, data, 8);
>>  		if (ret < 0) {
>>  			drm_err(aux->drm_dev, "Failed to write AVI IF block
>> %d\n",
>>  				block_count);
>> @@ -386,7 +384,7 @@ static bool
>> _lspcon_parade_write_infoframe_blocks(struct drm_dp_aux *aux,
>>  		 */
>>  		reg = LSPCON_PARADE_AVI_IF_CTRL;
>>  		avi_if_ctrl = LSPCON_PARADE_AVI_IF_KICKOFF | block_count;
>> -		ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
>> +		ret = drm_dp_dpcd_write_byte(aux, reg, avi_if_ctrl);
>>  		if (ret < 0) {
>>  			drm_err(aux->drm_dev, "Failed to update (0x%x),
>> block %d\n",
>>  				reg, block_count);
>> @@ -445,8 +443,8 @@ static bool _lspcon_write_avi_infoframe_mca(struct
>> drm_dp_aux *aux,
>>  	while (written < len) {
>>  		/* DPCD write for AVI IF can fail on a slow FW day, so retry */
>>  		for (retry = 0; retry < 5; retry++) {
>> -			ret = drm_dp_dpcd_write(aux, reg, (void *)data, 1);
>> -			if (ret == 1) {
>> +			ret = drm_dp_dpcd_write_byte(aux, reg, *data);
>> +			if (!ret) {
>>  				break;
>>  			} else if (retry < 4) {
>>  				mdelay(50);
>> @@ -461,9 +459,8 @@ static bool _lspcon_write_avi_infoframe_mca(struct
>> drm_dp_aux *aux,
>>  		data++;
>>  	}
>> 
>> -	val = 0;
>>  	reg = LSPCON_MCA_AVI_IF_CTRL;
>> -	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
>> +	ret = drm_dp_dpcd_read_byte(aux, reg, &val);
>>  	if (ret < 0) {
>>  		drm_err(aux->drm_dev, "DPCD read failed, address 0x%x\n",
>> reg);
>>  		return false;
>> @@ -473,14 +470,13 @@ static bool _lspcon_write_avi_infoframe_mca(struct
>> drm_dp_aux *aux,
>>  	val &= ~LSPCON_MCA_AVI_IF_HANDLED;
>>  	val |= LSPCON_MCA_AVI_IF_KICKOFF;
>> 
>> -	ret = drm_dp_dpcd_write(aux, reg, &val, 1);
>> +	ret = drm_dp_dpcd_write_byte(aux, reg, val);
>>  	if (ret < 0) {
>>  		drm_err(aux->drm_dev, "DPCD read failed, address 0x%x\n",
>> reg);
>>  		return false;
>>  	}
>> 
>> -	val = 0;
>> -	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
>> +	ret = drm_dp_dpcd_read_byte(aux, reg, &val);
>>  	if (ret < 0) {
>>  		drm_err(aux->drm_dev, "DPCD read failed, address 0x%x\n",
>> reg);
>>  		return false;
>> @@ -617,7 +613,7 @@ static bool
>> _lspcon_read_avi_infoframe_enabled_mca(struct drm_dp_aux *aux)
>>  	u16 reg = LSPCON_MCA_AVI_IF_CTRL;
>>  	u8 val;
>> 
>> -	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
>> +	ret = drm_dp_dpcd_read_byte(aux, reg, &val);
>>  	if (ret < 0) {
>>  		drm_err(aux->drm_dev, "DPCD read failed, address 0x%x\n",
>> reg);
>>  		return false;
>> @@ -632,7 +628,7 @@ static bool
>> _lspcon_read_avi_infoframe_enabled_parade(struct drm_dp_aux *aux)
>>  	u16 reg = LSPCON_PARADE_AVI_IF_CTRL;
>>  	u8 val;
>> 
>> -	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
>> +	ret = drm_dp_dpcd_read_byte(aux, reg, &val);
>>  	if (ret < 0) {
>>  		drm_err(aux->drm_dev, "DPCD read failed, address 0x%x\n",
>> reg);
>>  		return false;
>> --
>> 2.47.3
>

-- 
Jani Nikula, Intel