Re: [PATCH] drm/i915: Use dpcd read wake for sink crc calls.

Daniel Vetter
2015-11-19T09:12:34Z

On Wed, Nov 18, 2015 at 06:31:05PM +0000, Vivi, Rodrigo wrote:
> On Tue, 2015-11-17 at 15:08 +0100, Daniel Vetter wrote:
> > On Mon, Nov 16, 2015 at 04:05:42PM +0000, Rodrigo Vivi wrote:
> > > Ok, so after trying it we saw that we really cannot trust on aux
> > > mutex.At
> > > least not on all SKL/KBL
> > > It worked in a KBL but failed on a SKL that I have here...
> > >
> > > So without aux mutex option we still need to get sink_crc more
> > > reliable and
> > > I see only 2 quick ways here:
> > > - This read wake
> > > - Return -EBUSY to force the drm retries on message size = 0.
> > >
> > > Daniel, what do you believe?
> >
> > It's still a mess. My opinion is still that we should move the hacks
> > from
> > read_wake into a more suitable place:
> > a) either into drm_dp_dpcd_read in drm_dp_helper.c
>
> Well, but drm_dp_helper already does many retries already (32 times)
> but only on EBUSY. My idea is that we should consider that and return
> EBUSY instead of adding another retry case at drm.
>
>
> > b) or into intel_dp_aux_transfer in intel_dp.c
>
> Oh, I thought you had nacked this option.
>
> >
> > Option a) is the right one if this is a generic sink issue (and it
> > seems
> > to be the case, at least for edp panels). Option b) if it's an issue
> > with
> > our hw. Either way I think intel_dp_dpcd_read_wake should die.
>
> Well, Jani and Ville kind of nacked this while we don't understand why
> this was ever introduced at first place.
> Although I believe with the proper EBUSY returns in place and 32 times
> retry I believe we could safely remove this as I tried on that series.
>
> >
> > On a personal gut level I'd go with option a).
>
> So, EBUSY is ok or should we create another case on drm level?

Well, what I'd really like is to get rid of our read_wake code, since
pretty obviously it's a hack we seem to need everywhere. If the EBUSY
trick will allow us to do that I'm all for it.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation