On Wed, 28 Jan 2026, Suraj Kandpal <[email protected]> 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 <[email protected]>
> ---
>  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

Reply via email to