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

Jani Nikula
2015-08-27T10:42:54Z

On Thu, 27 Aug 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> 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.

In truth our inconsistent use of wake vs. non-wake can be mostly
attributed to the fact that we're clueless about sink sleep states, and
we've just added more wakes here and there to paper over it.

BR,
Jani.

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

--
Jani Nikula, Intel Open Source Technology Center