Re: [PATCH v3] drm/i915/dp: Add a meaningful return to intel_dp_read_dsc_dpcd

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 2/2/2026 1:48 PM, Suraj Kandpal wrote:
> Add a meaningful return to intel_dp_read_dsc_dpcd. This is to avoid
> unwanted DPCD reads which are not needed once we know DSC DPCD
> read fails. To do this convert drm_dp_dpcd_read to drm_dp_dpcd_read_data
> which returns a meaningful error which can be propogated up when all
> bits are not read.
> While we are at it convert the drm_err in intel_dp_read_dsc_dpcd to
> drm_dbg_kms. This is because we do not want a hard ERROR when we
> call this function, during the intel_dp_detect phase since AUX may not
> be up, it is expected to fail but we do not expect a failure in read
> when we call intel_dp_dsc_get_sink_cap so we move the drm_err there.
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>

LGTM.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>


> ---
> V1 -> V2:
> -Commit message fixup (Jani)
> -Convert drm_dp_dpcd_read to drm_dp_dpcd_read_data (Jani)
> -Propogate the error sent by drm_dp_dpcd_read_data up (Jani)
> -Use %pe and ERR_PTR() to log the extact error (Jani)
>
> V2 -> V3:
> -No need for the extra drm_err (Ankit)
>
>   drivers/gpu/drm/i915/display/intel_dp.c | 31 +++++++++++++++----------
>   1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 79fd3b8d8b25..380d9801c377 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4281,20 +4281,24 @@ static bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
>   	return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
>   }
>   
> -static void intel_dp_read_dsc_dpcd(struct drm_dp_aux *aux,
> -				   u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> -{
> -	if (drm_dp_dpcd_read(aux, DP_DSC_SUPPORT, dsc_dpcd,
> -			     DP_DSC_RECEIVER_CAP_SIZE) < 0) {
> -		drm_err(aux->drm_dev,
> -			"Failed to read DPCD register 0x%x\n",
> -			DP_DSC_SUPPORT);
> -		return;
> +static int intel_dp_read_dsc_dpcd(struct drm_dp_aux *aux,
> +				  u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> +{
> +	int ret;
> +
> +	ret = drm_dp_dpcd_read_data(aux, DP_DSC_SUPPORT, dsc_dpcd,
> +				    DP_DSC_RECEIVER_CAP_SIZE);
> +	if (ret) {
> +		drm_dbg_kms(aux->drm_dev,
> +			    "Could not read DSC DPCD register 0x%x Error: %pe\n",
> +			    DP_DSC_SUPPORT, ERR_PTR(ret));
> +		return ret;
>   	}
>   
>   	drm_dbg_kms(aux->drm_dev, "DSC DPCD: %*ph\n",
>   		    DP_DSC_RECEIVER_CAP_SIZE,
>   		    dsc_dpcd);
> +	return 0;
>   }
>   
>   static void init_dsc_overall_throughput_limits(struct intel_connector *connector, bool is_branch)
> @@ -4345,8 +4349,9 @@ void intel_dp_get_dsc_sink_cap(u8 dpcd_rev,
>   	if (dpcd_rev < DP_DPCD_REV_14)
>   		return;
>   
> -	intel_dp_read_dsc_dpcd(connector->dp.dsc_decompression_aux,
> -			       connector->dp.dsc_dpcd);
> +	if (intel_dp_read_dsc_dpcd(connector->dp.dsc_decompression_aux,
> +				   connector->dp.dsc_dpcd) < 0)
> +		return;
>   
>   	if (drm_dp_dpcd_readb(connector->dp.dsc_decompression_aux, DP_FEC_CAPABILITY,
>   			      &connector->dp.fec_capability) < 0) {
> @@ -4376,7 +4381,9 @@ static void intel_edp_get_dsc_sink_cap(u8 edp_dpcd_rev, struct intel_connector *
>   	if (edp_dpcd_rev < DP_EDP_14)
>   		return;
>   
> -	intel_dp_read_dsc_dpcd(connector->dp.dsc_decompression_aux, connector->dp.dsc_dpcd);
> +	if (intel_dp_read_dsc_dpcd(connector->dp.dsc_decompression_aux,
> +				   connector->dp.dsc_dpcd) < 0)
> +		return;
>   
>   	if (connector->dp.dsc_dpcd[0] & DP_DSC_DECOMPRESSION_IS_SUPPORTED)
>   		init_dsc_overall_throughput_limits(connector, false);