On Fri, Feb 20, 2026 at 10:31:10AM +0530, Murthy, Arun R wrote: > > On 20-02-2026 08:41, Ville Syrjälä wrote: > > On Thu, Feb 19, 2026 at 08:42:49PM +0530, Murthy, Arun R wrote: > >> On 19-02-2026 20:23, Ville Syrjälä wrote: > >>> On Thu, Feb 19, 2026 at 03:13:26PM +0530, Arun R Murthy wrote: > >>>> Before reading the DPCD caps for eDP wake the sink device and for DP > >>>> after reading the lttpr caps and before reading the dpcd caps wake up > >>>> the sink device. > >>> Why? > >> Just to ensure that sink is awake. > >> On eDP init, as part of reading the DPCD caps during the AUX transaction > >> its sometimes observed that the AUX tx fails with timeout. In those > >> scenarios even if the retry is increased to 1000 AUX tx will not > >> succeed. May be that sink is in sleep or unknown state. > >> Spec DP2.1 sec 2.11.7.1.5.8 says if there is a NO REPLY for AUX tx this > >> can be due to illegal cmd or sink in low power state. > > That section is specifically about i2c-over-aux. > > > > Generally we have retries and appropriate timeouts to deal with AUX > > having to wake up from low power state. > We have tried this for 1000 times and didnt work.
Tried it where/when/what? I still have no idea what you're trying to solve here. > This leaves a question as to why not replying even after 1000 trials. > Answer might be the command/request is wrong for which sink is not able > to understand. But its not the case. > So the other thinking is "is the sink in low power mode" ? > In link training sections of the spec it says before starting link > training ensure that sink is up and provides steps to wake the sink device. > Example: Section 3.5.2.16 We already do that (intel_dp_set_power()). Though even then we seem to doing the OUI DPCD stuff before the D3->D0 transition. So that still assumes that any random DPCD accesses work while in D3. Why we do it in that order I have no idea. > > > Although, I suppose we might consider switching to D0 for eg. duration > > of the detection to make sure none of the AUX transactions there take > > too long. That *might* make things a bit faster (but we'd need actual > > numbers to show that). And once we're done we should switch back to D3 > > to save power. > I am doing that in the patch. I switch to D0 and then immediately switch > to D3_AUX_ON state. D3_AUX_ON isn't even a thing in the older DP versions. > The reason for reading DP_SET_POWER in a loop is the spec says that DPTX > should try for atleast 3 times as DPRX is waking from power saving state > which takes 1ms. > Section 2.3.5 explains the DPRX AUX_CH state and says that DPRX shall > avoid not issuing a reply when waking from a power saving state. > This section also explains the DPRX AUX_CH state on RESET. Usually this > is the state on power on, and in the issue that we are referring to can > also be co-related to this state as its during the kernel boot/driver > probe. > > Perhaps we could then also use a larger timeout just for > > the DP_SET_POWER AUX accesses, > spec mention on this time as 1ms, to allow sink to exit low power mode > or AUX_CH Receiver is monitoring differential signals (sec 2.3.1.2) > > and all other AUX accesses could assume > > that the thing is awake and use a smaller timeout. Although the LTTPR > > mess probably means we can't actually reduce the timeouts :/ > > > > Another slight snag in the current way of doing things is that IIRC > > we never put a device into D3 after the initial detection, unless we > > actually turned the main link on and off again. That's also something > > that could get fixed by always putting the device into D3 after > > detection. But to do that stuff safely we'd need some way to make sure > > nothing else (eg. the main link) requires the D0 at that time. So some > > kind of D0 vs. D3 reference counting scheme might be needed. > > > > I did consider implementing something like that years ago, but dealing > > with the reference counting seemed too messy at the time. And since I > > never implemented it I never measured it either. Perhaps things are a > > bit cleaner these days to make that easier. Dunno. > > > >> So in this patch we are trying to wake the sink device. > > Still the same question remains: Why? What *exactly* is the problem > > you're trying to solve here? > The problem we are trying to solve is that the AUX_CH Requested is not > succeeding even after sending multiple AUX requests and fails to get a > response from the AUX_CH Replier. > Increasing the retry count to a large value (1000) is also not helping. > > In order to solve this went through the spec in detail, able to get some > information scattered over multiple sections leaving us to a thinking > that may be AUX_CH Replier in low power state. Also in link training > section(3.5.2.16) it says source should wake the sink before starting > link training. > > Thanks and Regards, > Arun R Murthy > ------------------- > > >> Thanks and Regards, > >> Arun R Murthy > >> ------------------- > >> > >>>> Closes: https://issues.redhat.com/browse/RHEL-120913 > >>>> Signed-off-by: Arun R Murthy <[email protected]> > >>>> --- > >>>> drivers/gpu/drm/i915/display/intel_dp.c | 41 +++++++++++++++++++ > >>>> drivers/gpu/drm/i915/display/intel_dp.h | 1 + > >>>> .../drm/i915/display/intel_dp_link_training.c | 3 ++ > >>>> 3 files changed, 45 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > >>>> b/drivers/gpu/drm/i915/display/intel_dp.c > >>>> index 454e6144ee4e..2fbb947e6cc8 100644 > >>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c > >>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c > >>>> @@ -4705,6 +4705,45 @@ intel_edp_set_sink_rates(struct intel_dp > >>>> *intel_dp) > >>>> intel_edp_set_data_override_rates(intel_dp); > >>>> } > >>>> > >>>> +void intel_dp_wake_sink(struct intel_dp *intel_dp) > >>>> +{ > >>>> + u8 value = 0; > >>>> + int ret = 0, try = 0; > >>>> + > >>>> + intel_dp_dpcd_set_probe(intel_dp, false); > >>>> + > >>>> + /* > >>>> + * 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 > >>>> + */ > >>>> + while (try < 10 && ret < 0) { > >>>> + ret = drm_dp_dpcd_readb(&intel_dp->aux, DP_SET_POWER, > >>>> &value); > >>>> + /* > >>>> + * If sink is in D3 then it may not respond to the AUX > >>>> tx so > >>>> + * wake it up to D3_AUX_ON state > >>>> + */ > >>>> + if (value == DP_SET_POWER_D3) { > >>>> + /* 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); > >>>> + break; > >>>> + } else if ((value == DP_SET_POWER_D0) || > >>>> + (value == DP_SET_POWER_D3_AUX_ON)) { > >>>> + /* if in D0 or D3_AUX_ON exit */ > >>>> + break; > >>>> + } > >>>> + /* Sink in D0 or even if read fails a minimum of 1ms is > >>>> required to wake and respond */ > >>>> + fsleep(1000); > >>>> + try++; > >>>> + } > >>>> + > >>>> + 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 +4752,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 -- Ville Syrjälä Intel
