> -----Original Message----- > From: Nikula, Jani <jani.nikula@xxxxxxxxx> > Sent: Tuesday, December 20, 2022 9:33 PM > To: Murthy, Arun R <arun.r.murthy@xxxxxxxxx>; intel- > gfx@xxxxxxxxxxxxxxxxxxxxx; ville.syrjala@xxxxxxxxxxxxxxx; Deak, Imre > <imre.deak@xxxxxxxxx> > Subject: RE: [PATCHv6] drm/i915/dp: change aux_ctl reg read to polling read > > On Mon, 19 Dec 2022, "Murthy, Arun R" <arun.r.murthy@xxxxxxxxx> wrote: > > Any comments? > > From #intel-gfx: > > <vsyrjala> bashing the hw for 500 usec seems a bit harsh > > Which is true. The default for intel_de_wait_for_register() is 2 us. Should > probably stick to that. Recommendation as per windows code base is 50us interval for polling the register, so will change it to 50us. Thanks and Regards, Arun R Murthy -------------------- > > BR, > Jani. > > > > > > Thanks and Regards, > > Arun R Murthy > > -------------------- > > > >> -----Original Message----- > >> From: Murthy, Arun R <arun.r.murthy@xxxxxxxxx> > >> Sent: Thursday, December 15, 2022 4:44 PM > >> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; ville.syrjala@xxxxxxxxxxxxxxx; > >> Nikula, Jani <jani.nikula@xxxxxxxxx>; Deak, Imre > >> <imre.deak@xxxxxxxxx> > >> Cc: Murthy, Arun R <arun.r.murthy@xxxxxxxxx> > >> Subject: [PATCHv6] drm/i915/dp: change aux_ctl reg read to polling > >> read > >> > >> The busy timeout logic checks for the AUX BUSY, then waits for the > >> timeout period and then after timeout reads the register for BUSY or > Success. > >> Instead replace interrupt with polling so as to read the AUX CTL > >> register often before the timeout period. Looks like there might be > >> some issue with interrupt-on-read. Hence changing the logic to polling > read. > >> > >> v2: replace interrupt with polling read > >> v3: use usleep_rang instead of msleep, updated commit msg > >> v4: use intel_wait_for_regiter internal function > >> v5: use __intel_de_wait_for_register with 500us slow and 10ms fast > >> timeout > >> v6: check return value of __intel_de_wait_for_register > >> > >> Signed-off-by: Arun R Murthy <arun.r.murthy@xxxxxxxxx> > >> --- > >> drivers/gpu/drm/i915/display/intel_dp_aux.c | 14 +++++--------- > >> 1 file changed, 5 insertions(+), 9 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c > >> b/drivers/gpu/drm/i915/display/intel_dp_aux.c > >> index 91c93c93e5fc..973dadecf712 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c > >> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c > >> @@ -41,20 +41,16 @@ intel_dp_aux_wait_done(struct intel_dp > *intel_dp) > >> i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp); > >> const unsigned int timeout_ms = 10; > >> u32 status; > >> - bool done; > >> - > >> -#define C (((status = intel_de_read_notrace(i915, ch_ctl)) & > >> DP_AUX_CH_CTL_SEND_BUSY) == 0) > >> - done = wait_event_timeout(i915->display.gmbus.wait_queue, C, > >> - msecs_to_jiffies_timeout(timeout_ms)); > >> + int ret; > >> > >> - /* just trace the final value */ > >> - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); > >> + ret = __intel_de_wait_for_register(i915, ch_ctl, > >> + DP_AUX_CH_CTL_SEND_BUSY, 0, > >> + 500, timeout_ms, &status); > >> > >> - if (!done) > >> + if (ret == -ETIMEDOUT) > >> drm_err(&i915->drm, > >> "%s: did not complete or timeout within %ums > >> (status 0x%08x)\n", > >> intel_dp->aux.name, timeout_ms, status); > >> -#undef C > >> > >> return status; > >> } > >> -- > >> 2.25.1 > > > > -- > Jani Nikula, Intel Open Source Graphics Center
