| Message ID | 1049e7188a76c421fab7797b5c4a6aa1b709f4c9.1641317930.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/mst: DP MST ESI handling improvements | expand |
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani Nikula > Sent: Tuesday, January 4, 2022 11:10 PM > To: intel-gfx@lists.freedesktop.org > Cc: Nikula, Jani <jani.nikula@intel.com> > Subject: [Intel-gfx] [PATCH 2/7] drm/i915/mst: abstract intel_dp_ack_sink_irq_esi() > > Smaller functions make the thing easier to read. Debug log failures to ack. > > Note: Looks like we have the retry loop simply because of hysterical raisins, dating Nit: Typo in reasons. Reviewed-by: Uma Shankar <uma.shankar@intel.com> > back to the original DP MST enabling. Keep it, though I have no idea why we have it. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 25 +++++++++++++++---------- > 1 file changed, 15 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 357c39e09bf6..ebf80a875a41 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -2820,6 +2820,19 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 > *sink_irq_vector) > DP_DPRX_ESI_LEN; > } > > +static bool intel_dp_ack_sink_irq_esi(struct intel_dp *intel_dp, u8 > +esi[4]) { > + int retry; > + > + for (retry = 0; retry < 3; retry++) { > + if (drm_dp_dpcd_write(&intel_dp->aux, DP_SINK_COUNT_ESI + 1, > + &esi[1], 3) == 3) > + return true; > + } > + > + return false; > +} > + > bool > intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state, > const struct drm_connector_state *conn_state) @@ -3660,7 > +3673,6 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp) > */ > u8 esi[DP_DPRX_ESI_LEN+2] = {}; > bool handled; > - int retry; > > if (!intel_dp_get_sink_irq_esi(intel_dp, esi)) { > drm_dbg_kms(&i915->drm, > @@ -3685,15 +3697,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp) > if (!handled) > break; > > - for (retry = 0; retry < 3; retry++) { > - int wret; > - > - wret = drm_dp_dpcd_write(&intel_dp->aux, > - DP_SINK_COUNT_ESI+1, > - &esi[1], 3); > - if (wret == 3) > - break; > - } > + if (!intel_dp_ack_sink_irq_esi(intel_dp, esi)) > + drm_dbg_kms(&i915->drm, "Failed to ack ESI\n"); > } > > return link_ok; > -- > 2.30.2
On Thu, 20 Jan 2022, "Shankar, Uma" <uma.shankar@intel.com> wrote: >> -----Original Message----- >> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani Nikula >> Sent: Tuesday, January 4, 2022 11:10 PM >> To: intel-gfx@lists.freedesktop.org >> Cc: Nikula, Jani <jani.nikula@intel.com> >> Subject: [Intel-gfx] [PATCH 2/7] drm/i915/mst: abstract intel_dp_ack_sink_irq_esi() >> >> Smaller functions make the thing easier to read. Debug log failures to ack. >> >> Note: Looks like we have the retry loop simply because of hysterical raisins, dating > > Nit: Typo in reasons. ;) http://www.catb.org/jargon/html/H/hysterical-reasons.html > > Reviewed-by: Uma Shankar <uma.shankar@intel.com> > >> back to the original DP MST enabling. Keep it, though I have no idea why we have it. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_dp.c | 25 +++++++++++++++---------- >> 1 file changed, 15 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c >> b/drivers/gpu/drm/i915/display/intel_dp.c >> index 357c39e09bf6..ebf80a875a41 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dp.c >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c >> @@ -2820,6 +2820,19 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 >> *sink_irq_vector) >> DP_DPRX_ESI_LEN; >> } >> >> +static bool intel_dp_ack_sink_irq_esi(struct intel_dp *intel_dp, u8 >> +esi[4]) { >> + int retry; >> + >> + for (retry = 0; retry < 3; retry++) { >> + if (drm_dp_dpcd_write(&intel_dp->aux, DP_SINK_COUNT_ESI + 1, >> + &esi[1], 3) == 3) >> + return true; >> + } >> + >> + return false; >> +} >> + >> bool >> intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state, >> const struct drm_connector_state *conn_state) @@ -3660,7 >> +3673,6 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp) >> */ >> u8 esi[DP_DPRX_ESI_LEN+2] = {}; >> bool handled; >> - int retry; >> >> if (!intel_dp_get_sink_irq_esi(intel_dp, esi)) { >> drm_dbg_kms(&i915->drm, >> @@ -3685,15 +3697,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp) >> if (!handled) >> break; >> >> - for (retry = 0; retry < 3; retry++) { >> - int wret; >> - >> - wret = drm_dp_dpcd_write(&intel_dp->aux, >> - DP_SINK_COUNT_ESI+1, >> - &esi[1], 3); >> - if (wret == 3) >> - break; >> - } >> + if (!intel_dp_ack_sink_irq_esi(intel_dp, esi)) >> + drm_dbg_kms(&i915->drm, "Failed to ack ESI\n"); >> } >> >> return link_ok; >> -- >> 2.30.2 >
> -----Original Message----- > From: Nikula, Jani <jani.nikula@intel.com> > Sent: Thursday, January 20, 2022 4:02 PM > To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org > Subject: RE: [Intel-gfx] [PATCH 2/7] drm/i915/mst: abstract > intel_dp_ack_sink_irq_esi() > > On Thu, 20 Jan 2022, "Shankar, Uma" <uma.shankar@intel.com> wrote: > >> -----Original Message----- > >> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani > Nikula > >> Sent: Tuesday, January 4, 2022 11:10 PM > >> To: intel-gfx@lists.freedesktop.org > >> Cc: Nikula, Jani <jani.nikula@intel.com> > >> Subject: [Intel-gfx] [PATCH 2/7] drm/i915/mst: abstract > intel_dp_ack_sink_irq_esi() > >> > >> Smaller functions make the thing easier to read. Debug log failures to ack. > >> > >> Note: Looks like we have the retry loop simply because of hysterical raisins, > dating > > > > Nit: Typo in reasons. > > ;) > > http://www.catb.org/jargon/html/H/hysterical-reasons.html Oh, didn't knew that
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 357c39e09bf6..ebf80a875a41 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2820,6 +2820,19 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) DP_DPRX_ESI_LEN; } +static bool intel_dp_ack_sink_irq_esi(struct intel_dp *intel_dp, u8 esi[4]) +{ + int retry; + + for (retry = 0; retry < 3; retry++) { + if (drm_dp_dpcd_write(&intel_dp->aux, DP_SINK_COUNT_ESI + 1, + &esi[1], 3) == 3) + return true; + } + + return false; +} + bool intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state, const struct drm_connector_state *conn_state) @@ -3660,7 +3673,6 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp) */ u8 esi[DP_DPRX_ESI_LEN+2] = {}; bool handled; - int retry; if (!intel_dp_get_sink_irq_esi(intel_dp, esi)) { drm_dbg_kms(&i915->drm, @@ -3685,15 +3697,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp) if (!handled) break; - for (retry = 0; retry < 3; retry++) { - int wret; - - wret = drm_dp_dpcd_write(&intel_dp->aux, - DP_SINK_COUNT_ESI+1, - &esi[1], 3); - if (wret == 3) - break; - } + if (!intel_dp_ack_sink_irq_esi(intel_dp, esi)) + drm_dbg_kms(&i915->drm, "Failed to ack ESI\n"); } return link_ok;
Smaller functions make the thing easier to read. Debug log failures to ack. Note: Looks like we have the retry loop simply because of hysterical raisins, dating back to the original DP MST enabling. Keep it, though I have no idea why we have it. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)