On 25-02-2026 09:03, Murthy, Arun R wrote:

On 24-02-2026 20:20, Imre Deak wrote:
On Tue, Feb 24, 2026 at 01:18:30PM +0530, Arun R Murthy wrote:
Its observed that on AUX_CH failure, even if the retry is increased to
1000, it does not succeed. Either the command might be wrong or sink in
an unknown/sleep state can cause this. So try waking the sink device.
Before reading the DPCD caps wake the sink for eDP and for DP after
reading the lttpr caps if present and before reading the dpcd caps wake
up the sink device.

v2: Use poll_timeout_us (Jani N)
     Add the reason, why this change is required (Ville)

Closes: https://issues.redhat.com/browse/RHEL-120913
I wonder what should be the rule with non-public links like the above.
For instance, we do not put VLK-xxx links exactly because those are
non-public. Should/could we demand that RedHat opens a public ticket?
Jani?
There is a JIRA as well
https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4391
Signed-off-by: Arun R Murthy <[email protected]>
---
  drivers/gpu/drm/i915/display/intel_dp.c       | 38 +++++++++++++++++++
  drivers/gpu/drm/i915/display/intel_dp.h       |  1 +
  .../drm/i915/display/intel_dp_link_training.c |  3 ++
  3 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 025e906b63a9..fa0730f7b92a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4705,6 +4705,42 @@ intel_edp_set_sink_rates(struct intel_dp *intel_dp)
      intel_edp_set_data_override_rates(intel_dp);
  }
  +/* Spec says to try for 3 times, its doubled to add the software overhead */
+#define AUX_CH_WAKE_RETRY    6
+
+void intel_dp_wake_sink(struct intel_dp *intel_dp)
+{
+    u8 value = 0;
+    int ret = 0;
+
+    intel_dp_dpcd_set_probe(intel_dp, false);
Is there any particular reason to turn off/on the probing? I don't see
any reason why the DP_SET_POWER polling would need that. In any case
turning it off/on this way is not ok: reading the DPRX caps, which would
call this function, could happen at any time after a sink gets
connected, so turning probing back on at the end of this function would
re-enable it incorrectly for sinks where it's been already established
that the probing workaround is not needed and should stay disabled.
This function intel_dp_wake_sink() is called from edp_init_dpcd and dp_init_lttpr_dprx_caps. dpcd_set_probe is set to true in dp_aux_init which is called before calling intel_edp_init_connector.

Probe is set to true, hence in this function I am setting it to false and then setting back to true. But there can be a possibility of reading lttpr_dprx_caps being called later as well.

Will correct this to check if probe is already being set to true, will then disable before waking the sink and restore the probe status at the end.

Will get this corrected in the next revision.

I will drop the fix for DP from this patch and take it up later.

In this patch will only target fix for eDP for which dpcd probe will not be required as per intel_dp_needs_dpcd_probe()

Thanks and Regards,
Arun R Murthy
-------------------

Thanks and Regards,
Arun R Murthy
-------------------

+
+    /*
+     * Wake the sink device
+     * Spec DP2.1 section 2.3.1.2 if AUX CH is powered down by writing 0x02 +     * to DP_SET_POWER dpcd reg, 1ms time would be required to wake it up
+     */
+    ret = poll_timeout_us(ret = drm_dp_dpcd_readb(&intel_dp->aux, DP_SET_POWER, &value),
+                  ret > 0,
+                  1000, AUX_CH_WAKE_RETRY * 1000, true);
+
+    /*
+     * If sink is in D3 then it may not respond to the AUX tx so
+     * wake it up to D3_AUX_ON state
+     * If the above poll_timeout_us fails, try waking the sink.
+     */
+    if (value == DP_SET_POWER_D3 || ret < 0) {
+        /* After setting to D0 need a min of 1ms to wake(Spec DP2.1 sec 2.3.1.2) */
+        drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+                   DP_SET_POWER_D0);
+        fsleep(1000);
+        drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+                   DP_SET_POWER_D3_AUX_ON);
+    }
+
+    intel_dp_dpcd_set_probe(intel_dp, true);
+}
+
  static bool
  intel_edp_init_dpcd(struct intel_dp *intel_dp, struct intel_connector *connector)
  {
@@ -4713,6 +4749,8 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp, struct intel_connector *connector
      /* this function is meant to be called only once */
      drm_WARN_ON(display->drm, intel_dp->dpcd[DP_DPCD_REV] != 0);
  +    intel_dp_wake_sink(intel_dp);
+
      if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd) != 0)
          return false;
  diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index b0bbd5981f57..3f16077c0cc7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -232,6 +232,7 @@ bool intel_dp_dotclk_valid(struct intel_display *display,   bool intel_dp_joiner_candidate_valid(struct intel_connector *connector,
                       int hdisplay,
                       int num_joined_pipes);
+void intel_dp_wake_sink(struct intel_dp *intel_dp);
    #define for_each_joiner_candidate(__connector, __mode, __num_joined_pipes) \       for ((__num_joined_pipes) = 1; (__num_joined_pipes) <= (I915_MAX_PIPES); (__num_joined_pipes)++) \ diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 54c585c59b90..cbb712ea9f60 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -270,6 +270,9 @@ int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp)
          lttpr_count = intel_dp_init_lttpr(intel_dp, dpcd);
      }
  +    /* After reading LTTPR wake up the sink before reading DPRX caps */
+    intel_dp_wake_sink(intel_dp);
+
      /*
       * The DPTX shall read the DPRX caps after LTTPR detection, so re-read
       * it here.
--
2.25.1

Reply via email to