Re: [PATCHv2] drm/i915/dp: On DPCD init/caps wake the DPRx

[Imre Deak <imre.deak@intel.com>]

On Mon, Mar 02, 2026 at 02:34:45PM +0530, Murthy, Arun R wrote:
> On 02-03-2026 13:27, Imre Deak wrote:
> > On Wed, Feb 25, 2026 at 09:03:06AM +0530, 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
> > The above ticket is about an
> > 
> > "AUX x did not complete or timeout within 10ms"
> > 
> > error, which means the DPTX didn't complete the transfer. A transfer is
> > completed either (a) in response to a DPRX reply (AUX ACK,NAK,DEFER
> > reply) or (b) in case the DPRX is not replying at all. The above error
> > indicates that DPTX observed/signaled neither a or b. That's a problem
> > in the DPTX not in the DPRX which this patch is trying to fix (by
> > setting the DPRX power state to D0).
> On a Native AUX_CH transaction DPRX can reply with AUX_ACK/NACK/DEFER as per
> Table2-177 in the Spec DP2.1.
> In the error from the above ticker, we are getting a timeout for a AUX_CH
> initiated by DPTX.

No, we are not getting a timeout from the DPTX AUX HW in the unrelated
ticket you refer to. The error message in the unrelated issues/4391
ticket you refer to merely indicates that the _SW polling_ for
completion times out, which is a completely different thing.

Please open a public ticket with the information I asked, from my side
any workaround like the one suggested in this patch is not acceptable
until that info is available.

> Section 2.3.4 of Spec DP2.1 says timeout can be due to No Reply and the
> reason for No Reply is either sending invalid command or DPRX in sleep state
> or waking up from sleep state.
> 
> As part of this state, this patch is trying to wake the DPRX by setting to
> D0.
> 
> Thanks and Regards,
> Arun R Murthy
> --------------------
> 
> > Please open a separate public ticket for the actually reported
> > RHEL-120913 issue - which based on the changes in this patch must have a
> > separate root cause than issues/4391 - and attach a dmesg log having the
> > AUX logging enabled (drm.debug=0x10e) and reporducing the problem. Also
> > please ask the reporter to provide the details of the connected eDP
> > panel model and wiring to the CPU (is there any retimer, mux etc. HW
> > component between the CPU and the panel?).
> > 
> > > > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> > > > > ---
> > > > >    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.
> > > 
> > > 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
> > > > >