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

From: Daniel Vetter <daniel@ffwll.ch>
To: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
Cc: "daniel.vetter@ffwll.ch" <daniel.vetter@ffwll.ch>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: Use dpcd read wake for sink crc calls.
Date: Thu, 19 Nov 2015 10:12:30 +0100	[thread overview]
Message-ID: <20151119091230.GM20799@phenom.ffwll.local> (raw)
In-Reply-To: <1447871505.15751.92.camel@intel.com>

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
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx