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

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

On Wed, 28 Jan 2026, Suraj Kandpal <suraj.kandpal@intel.com> wrote:
> Add a meaningful return to intel_dp_read_dsc_dpcd so tha we avoid
> unwanted DPCD reads which are not needed once we know DSC DPCD
> read fails. While we are at it remove the drm_err since we do not
> shout error during intel_dp_detect phase since it may take time
> to come up after pps_on is called for eDP scenario.

Please pay more attention to the commit message.

> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 79fd3b8d8b25..d2ed8ec145a2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4281,20 +4281,21 @@ 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])
> +static int 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;
> +		drm_dbg_kms(aux->drm_dev,
> +			    "Could not read DSC DPCD register 0x%x\n",
> +			    DP_DSC_SUPPORT);
> +		return -EINVAL;

If we want to do this properly, then let's propagate the actual error
code instead of inventing -EINVAL here. And switch to
drm_dp_dpcd_read_data(), which returns an error if not all bytes were
transferred (also a case the current implementation completely ignores).

Finlly, the debug logging could log the error code. A combo of %pe in
the format string and ERR_PTR(ret) in the corresponding parameter will
do it nicely.

>  	}
>  
>  	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 +4346,11 @@ 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) {
> +		drm_err(display->drm, "Failed to read DSC DPCD register\n");
> +		return;
> +	}
>  
>  	if (drm_dp_dpcd_readb(connector->dp.dsc_decompression_aux, DP_FEC_CAPABILITY,
>  			      &connector->dp.fec_capability) < 0) {
> @@ -4376,7 +4380,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);

-- 
Jani Nikula, Intel