On 1/29/2026 9:48 AM, 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 <[email protected]>
---
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)
drivers/gpu/drm/i915/display/intel_dp.c | 33 ++++++++++++++++---------
1 file changed, 21 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..ec277dcb24bc 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,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;
I think we can skip this. We have already mentioned the dbg msg in
intel_dp_read_dsc_dpcd().
Also that way DP/eDP will both have symmetrical behavior as far as
reading DSC DPCD is concerned.
Regards,
Ankit
+ }
if (drm_dp_dpcd_readb(connector->dp.dsc_decompression_aux, DP_FEC_CAPABILITY,
&connector->dp.fec_capability) < 0) {
@@ -4376,7 +4383,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);
