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

["Vivi, Rodrigo" <rodrigo.vivi@intel.com>]

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?

> 
> Cheers, Daniel

Thanks,
Rodrigo.

> 
> > 
> > Please let me know witch way and if necessary I rebase the patch 
> > and
> > re-send.
> > 
> > Thanks,
> > Rodrigo.
> > 
> > 
> > 
> > 
> > On Wed, Oct 21, 2015 at 8:27 PM Thulasimani, Sivakumar <
> > sivakumar.thulasimani@intel.com> wrote:
> > 
> > > 
> > > 
> > > On 10/22/2015 1:44 AM, Damien Lespiau wrote:
> > > > On Thu, Oct 22, 2015 at 12:01:21AM +0530, Thulasimani, 
> > > > Sivakumar wrote:
> > > > > 
> > > > > On 8/25/2015 2:50 AM, Vivi, Rodrigo wrote:
> > > > > > On Mon, 2015-08-24 at 19:54 +0000, Zanoni, Paulo R wrote:
> > > > > > > Em Qui, 2015-08-20 às 16:23 -0700, Rodrigo Vivi escreveu:
> > > > > > > > Let's use a native read with retry as suggested per 
> > > > > > > > spec to
> > > > > > > > fix Sink CRC on SKL when PSR is enabled.
> > > > > > > > 
> > > > > > > > With PSR enabled panel is probably taking more time to 
> > > > > > > > wake
> > > > > > > > and dpcd read is faling.
> > > > > > > Does this commit actually fix any known problem with Sink 
> > > > > > > CRC? Or is
> > > > > > > it
> > > > > > > just a try? It would be nice to have this clarified in 
> > > > > > > the commit
> > > > > > > message.
> > > > > > It was just a try but that made sink crc working on my SKL 
> > > > > > when PSR is
> > > > > > enabled. nothing much to add...
> > > > > SKL has new register AUX_MUTEX which should be used when 
> > > > > accessing dpcd
> > > > > on edp. just searched the nightly code and could not find it. 
> > > > > it might
> > > be
> > > > > the reason
> > > > > for random dpcd failures reported in the other thread.
> > > > We had patches for that back in December 2013 :)
> > > > 
> > > > The feedback from Art was:
> > > > 
> > > >      The non-software aux users are PSR/SRD and GTC.
> > > >      Better leave out the mutex for now. Hardware is going to 
> > > > try do the
> > > >      arbitration itself. I expect you will then need to 
> > > > increase any
> > > software
> > > >      timeout you may have.
> > > > 
> > > > Do you know if anything has changed since then?
> > > > 
> > > Not sure, it is in the bspec sequence to use AUX hence forwarded. 
> > > Art
> > > might be the
> > > right person to contact :). it might be due to some minor DPCD 
> > > access
> > > issues we
> > > observed in BDW when PSR was enabled.
> > > 
> > > regards,
> > > Sivakumar
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx