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

Daniel Vetter
2015-08-27T09:21:12Z

On Wed, Aug 26, 2015 at 6:41 PM, Vivi, Rodrigo <rodrigo.vivi@intel.com> wrote:
> On Wed, 2015-08-26 at 11:06 +0200, Daniel Vetter wrote:
>> On Thu, Aug 20, 2015 at 04:12:00PM -0700, Rodrigo Vivi wrote:
>> > From: Rodrigo Vivi <vivijim@rdvivi-budapest.jf.intel.com>
>> >
>> > 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.
>> >
>> > Cc: Sonika Jindal <sonika.jindal@intel.com>
>> > Signed-off-by: Rodrigo Vivi <vivijim@rdvivi-budapest.jf.intel.com>
>>
>> Seems like we should just move the trickery we do in our own version
>> into
>> the dp helpers in the core if this is needed all over the place?
>
> I've wondered this, but I thought there was a good reason to let this
> trick separated.

I think in general you can assume that if i915 dp sink handling is
special it's because we have more testing on various broken hw out
there.

>> At least in i915 we use it everywhere and it doesn't seem actively
>> harmful
>> really ... Maybe the only exception would be the i2c-over-dp_aux
>> code.
>
> Why this would be the exception? Maybe this was the good reason?

I'd be fairly easy to keep an internal __drm_dp_aux_read (need it
anyway to implement this trick) and use that in i2c. At least that's
what I'd do without any evidence that we need to make this wake dance
also for i2c transactions. i2c uses a special dp-aux mode on the wire,
so makes some sense if it's different. See also the recent work from
Ville to tune the i2c dp-aux timeouts and retries, it really seems to
be a world of its own a bit.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch