On Tue, 08 Feb 2022, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Tue, Feb 08, 2022 at 02:12:33PM +0200, Jani Nikula wrote:
>> On Tue, 08 Feb 2022, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
>> > On Tue, Feb 08, 2022 at 11:17:22AM +0200, Jani Nikula wrote:
>> >> On Fri, 04 Feb 2022, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
>> >> > On Thu, Feb 03, 2022 at 11:03:54AM +0200, Jani Nikula wrote:
>> >> >> +
>> >> >> + if (timeout) {
>> >> >> + intel_dp_dump_link_status(intel_dp, DP_PHY_DPRX, link_status);
>> >> >> + drm_err(&i915->drm,
>> >> >> + "[ENCODER:%d:%s] Lane channel eq timeout\n",
>> >> >> + encoder->base.base.id, encoder->base.name);
>> >> >> + return false;
>> >> >> + }
>> >> >> +
>> >> >> + if (time_after(jiffies, deadline))
>> >> >> + timeout = true; /* try one last time after deadline */
>> >> >
>> >> > Is there a reason we can't do this just before drm_dp_dpcd_read_link_status()
>> >> > so we don't have to pass the timeout status from one loop iteration to
>> >> > the next?
>> >>
>> >> The point is to check one last time after timeout has passed, like you
>> >> suggested in previous review, and I agreed.
>> >
>> > Sure but why can't it be something more like?
>> >
>> > timeout = time_after();
>> > read_status();
>> > if (bad)
>> > bail;
>> > if (timeout)
>> > bail;
>> >
>> > I think we have it more like that in wait_for()/etc.
>>
>> I was going to fix this, but then realized the "one more time" really
>> only makes sense if it includes updating the signal levels and training
>> set and then checking the status. I don't think there's point in "one
>> more time" only covering the status read.
>
> Hmm. Yeah, I suppose that is true. We can't really know when the sink
> updated the status so checking for the timeout just before that might
> have the same issue as checking entirely after the status check.
>
>>
>> I've got the loop set up such that the flow is natural when entering the
>> loop i.e. I'd rather not have the adjust in the beginning with some if
>> (try != 0) check.
>>
>> Or am I missing something?
>
> Nah. I guess it's best leave it the way you have it now.
Thanks. Sent v3, but realized I'm still missing the intra-hop stuff.
--
Jani Nikula, Intel Open Source Graphics Center
