[Intel-gfx] [PATCHv6] drm/i915/dp: change aux_ctl reg read to polling read

[Murthy, Arun R]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula at intel.com>
> Sent: Tuesday, December 20, 2022 9:33 PM
> To: Murthy, Arun R <arun.r.murthy at intel.com>; intel-
> gfx at lists.freedesktop.org; ville.syrjala at linux.intel.com; Deak, Imre
> <imre.deak at intel.com>
> 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 at intel.com> 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 at intel.com>
> >> Sent: Thursday, December 15, 2022 4:44 PM
> >> To: intel-gfx at lists.freedesktop.org; ville.syrjala at linux.intel.com;
> >> Nikula, Jani <jani.nikula at intel.com>; Deak, Imre
> >> <imre.deak at intel.com>
> >> Cc: Murthy, Arun R <arun.r.murthy at intel.com>
> >> 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 at intel.com>
> >> ---
> >>  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