> Subject: RE: [PATCHv4] drm/i915/dp: On DPCD init wake the DPRx for eDP > > > > 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. > > > > > > v2: Use poll_timeout_us (Jani N) > > > Add the reason, why this change is required (Ville) > > > v3: Wake sink only for eDP > > > Remove the dpcd probe set to true/false in wake_sink (Imre) > > > v4: make edp_wake_sinc() static and dpcd_readb -> dpcd_reab_byte > > > (Suraj) > > > > > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4391 > > > Closes: > > > https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16654 > > > Signed-off-by: Arun R Murthy <[email protected]> > > > --- > > > drivers/gpu/drm/i915/display/intel_dp.c | 34 > > > +++++++++++++++++++++++++ > > > 1 file changed, 34 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > index 0cd5e6b5034c..d4cc5f7e82a4 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -4779,6 +4779,38 @@ 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 > > > + > > > +static void intel_edp_wake_sink(struct intel_dp *intel_dp) { > > > + u8 value = 0; > > > + int ret = 0; > > > + > > > + /* > > > + * 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 > > > > Nit: DPCD* > > > up > > > + */ > > > + ret = poll_timeout_us(ret = drm_dp_dpcd_read_byte(&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) { > > > > Should we maybe check ret < 0 first, > > Because if ret < 0 value will be bogus and if value == DP_SET_POWER_D3 > > than ret was not < 0 > > > Good catch, will change. > > > > + /* After setting to D0 need a min of 1ms to wake (Spec DP2.1 > > > sec 2.3.1.2) */ > > > + drm_dp_dpcd_write_byte(&intel_dp->aux, DP_SET_POWER, > > > + DP_SET_POWER_D0); > > > + fsleep(1000); > > > > I have not seen any use of fsleep maybe we should be using msleep() > > instead > > > fsleep() autoselects the sleep best mechanism depending in the requested > sleep duration. > > Ref: include/linux/delay.h > > > > > > + drm_dp_dpcd_write_byte(&intel_dp->aux, DP_SET_POWER, > > > + DP_SET_POWER_D3_AUX_ON); > > > + } > > > +} > > > + > > > static bool > > > intel_edp_init_dpcd(struct intel_dp *intel_dp, struct > > > intel_connector > > > *connector) { @@ -4792,6 +4824,8 @@ intel_edp_init_dpcd(struct > > > intel_dp *intel_dp, struct intel_connector *connector
Here [1] > > > if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd) != 0) > > > return false; > > > > > > + intel_edp_wake_sink(intel_dp); > > > > We are reading dpcd caps just before waking sink should we first be > > waking up sink even before caps are read. > > > Yes, this has to be done on the very first dpcd read. Yes so what I meant was should this be moved here [1] Regards, Suraj Kandpal > > Ref: DP 2.1 Spec Section 3.5.2.16 page 966 says " Prior to link training, a > DPTX > should perform the following: > 1 Verify that the SET_POWER_STATE field in the > SET_POWER_AND_SET_DP_PWR_VOLTAGE register is programmed to D0 > normal operation (DPCD 00600h[2:0] = 001b)." > > Also Section 2.14.9.7 POWER_UP_PHY says ensure the immediate downstream > device is awake > > Thanks and Regards, > Arun R Murthy > -------------------- > > > Regards, > > Suraj Kandpal > > > > > + > > > drm_dp_read_desc(&intel_dp->aux, &intel_dp->desc, > > > drm_dp_is_branch(intel_dp->dpcd)); > > > intel_init_dpcd_quirks(intel_dp, &intel_dp->desc.ident); > > > -- > > > 2.25.1
