| Message ID | 20240304050631.774920-1-shekhar.chauhan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] drm/i915/dp: Increase idle pattern wait timeout to 2ms | expand |
On Mon, 04 Mar 2024, Shekhar Chauhan <shekhar.chauhan@intel.com> wrote: > Currently, the driver is only waiting for 1ms for > idle patterns. But starting from LNL and beyond, > the MST wants the driver to wait for 1640us before What does it mean that "the MST wants"? > timing out (which we round up to 2ms). > > v1: Introduced the 2ms wait timeout. > v2: Segregated the wait timeout for platforms before & after LNL. I did not ask for this. I would rather all platforms used 2 ms. I even said the original change looked fine. But I'd like it to be explained in the commit message. > v3: Fixed 2 cosmetic changes. > > BSpec: 68849 > Signed-off-by: Shekhar Chauhan <shekhar.chauhan@intel.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index bea441590204..b59adb7685b8 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -3677,12 +3677,19 @@ static void intel_ddi_set_idle_link_train(struct intel_dp *intel_dp, > */ > if (port == PORT_A && DISPLAY_VER(dev_priv) < 12) > return; > - > - if (intel_de_wait_for_set(dev_priv, > - dp_tp_status_reg(encoder, crtc_state), > - DP_TP_STATUS_IDLE_DONE, 1)) > + if (DISPLAY_VER(dev_priv) >= 20) { > + if (intel_de_wait_for_set(dev_priv, > + dp_tp_status_reg(encoder, crtc_state), > + DP_TP_STATUS_IDLE_DONE, 2)) > + drm_err(&dev_priv->drm, > + "Timed out waiting for DP idle patterns\n"); > + } else { > + if (intel_de_wait_for_set(dev_priv, > + dp_tp_status_reg(encoder, crtc_state), > + DP_TP_STATUS_IDLE_DONE, 1)) > drm_err(&dev_priv->drm, > "Timed out waiting for DP idle patterns\n"); > + } So I'd like you to go back to how it was originally. > } > > static bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
On 3/4/2024 14:16, Jani Nikula wrote: > On Mon, 04 Mar 2024, Shekhar Chauhan <shekhar.chauhan@intel.com> wrote: >> Currently, the driver is only waiting for 1ms for >> idle patterns. But starting from LNL and beyond, >> the MST wants the driver to wait for 1640us before > What does it mean that "the MST wants"? I wanted to convey that MST streams require the driver to wait for 2ms. I'll rephrase. > >> timing out (which we round up to 2ms). >> >> v1: Introduced the 2ms wait timeout. >> v2: Segregated the wait timeout for platforms before & after LNL. > I did not ask for this. I would rather all platforms used 2 ms. I even > said the original change looked fine. But I'd like it to be explained in > the commit message. I felt that when you said "why bump for non lnl platforms", I felt that I should be segregating them. But, as discussed offline, I'll revert back to the original design with a change in the commit message. > >> v3: Fixed 2 cosmetic changes. >> >> BSpec: 68849 >> Signed-off-by: Shekhar Chauhan <shekhar.chauhan@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_ddi.c | 15 +++++++++++---- >> 1 file changed, 11 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c >> index bea441590204..b59adb7685b8 100644 >> --- a/drivers/gpu/drm/i915/display/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c >> @@ -3677,12 +3677,19 @@ static void intel_ddi_set_idle_link_train(struct intel_dp *intel_dp, >> */ >> if (port == PORT_A && DISPLAY_VER(dev_priv) < 12) >> return; >> - >> - if (intel_de_wait_for_set(dev_priv, >> - dp_tp_status_reg(encoder, crtc_state), >> - DP_TP_STATUS_IDLE_DONE, 1)) >> + if (DISPLAY_VER(dev_priv) >= 20) { >> + if (intel_de_wait_for_set(dev_priv, >> + dp_tp_status_reg(encoder, crtc_state), >> + DP_TP_STATUS_IDLE_DONE, 2)) >> + drm_err(&dev_priv->drm, >> + "Timed out waiting for DP idle patterns\n"); >> + } else { >> + if (intel_de_wait_for_set(dev_priv, >> + dp_tp_status_reg(encoder, crtc_state), >> + DP_TP_STATUS_IDLE_DONE, 1)) >> drm_err(&dev_priv->drm, >> "Timed out waiting for DP idle patterns\n"); >> + } > So I'd like you to go back to how it was originally. > > >> } >> >> static bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
On Mon, 04 Mar 2024, "Chauhan, Shekhar" <shekhar.chauhan@intel.com> wrote: > On 3/4/2024 14:16, Jani Nikula wrote: >> I did not ask for this. I would rather all platforms used 2 ms. I even >> said the original change looked fine. But I'd like it to be explained in >> the commit message. > I felt that when you said "why bump for non lnl platforms", I felt that > I should be segregating them. > But, as discussed offline, I'll revert back to the original design with > a change in the commit message. Thanks. Communication is hard. :) BR, Jani.
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index bea441590204..b59adb7685b8 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -3677,12 +3677,19 @@ static void intel_ddi_set_idle_link_train(struct intel_dp *intel_dp, */ if (port == PORT_A && DISPLAY_VER(dev_priv) < 12) return; - - if (intel_de_wait_for_set(dev_priv, - dp_tp_status_reg(encoder, crtc_state), - DP_TP_STATUS_IDLE_DONE, 1)) + if (DISPLAY_VER(dev_priv) >= 20) { + if (intel_de_wait_for_set(dev_priv, + dp_tp_status_reg(encoder, crtc_state), + DP_TP_STATUS_IDLE_DONE, 2)) + drm_err(&dev_priv->drm, + "Timed out waiting for DP idle patterns\n"); + } else { + if (intel_de_wait_for_set(dev_priv, + dp_tp_status_reg(encoder, crtc_state), + DP_TP_STATUS_IDLE_DONE, 1)) drm_err(&dev_priv->drm, "Timed out waiting for DP idle patterns\n"); + } } static bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
Currently, the driver is only waiting for 1ms for idle patterns. But starting from LNL and beyond, the MST wants the driver to wait for 1640us before timing out (which we round up to 2ms). v1: Introduced the 2ms wait timeout. v2: Segregated the wait timeout for platforms before & after LNL. v3: Fixed 2 cosmetic changes. BSpec: 68849 Signed-off-by: Shekhar Chauhan <shekhar.chauhan@intel.com> --- drivers/gpu/drm/i915/display/intel_ddi.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)