Re: [Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP

Sivakumar Thulasimani
2015-08-27T05:24:41Z

On 8/26/2015 7:59 PM, Benjamin Tissoires wrote:
> On Aug 26 2015 or thereabouts, Sivakumar Thulasimani wrote:
>>
>> On 8/18/2015 1:36 AM, Benjamin Tissoires wrote:
>>> On Aug 14 2015 or thereabouts, Stéphane Marchesin wrote:
>>>> On Wed, Aug 5, 2015 at 12:34 PM, Benjamin Tissoires
>>>> <benjamin.tissoires@redhat.com> wrote:
>>>>> On Jul 30 2015 or thereabouts, Sivakumar Thulasimani wrote:
>>>>>> On 7/29/2015 8:52 PM, Benjamin Tissoires wrote:
>>>>>>> On Jul 29 2015 or thereabouts, Sivakumar Thulasimani wrote:
>>>>>>>> why not detect reverse in intel_dp_detect/intel_hpd_pulse ? that way you can
>>>>>>>> identify both lane count and reversal state without touching anything in the
>>>>>>>> link training code. i am yet to upstream my changes for CHT that i can share
>>>>>>>> if required that does the same in intel_dp_detect without touching any line
>>>>>>>> in link training path.
>>>>>>> With my current limited knowledge of the dp hotplug (and i915 driver) I
>>>>>>> am not sure we could detect the reversed state without trying to train 1
>>>>>>> lane only. I'd be glad to look at your changes and test them on my
>>>>>>> system if you think that could help having a cleaner solution.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Benjamin
>>>>>> No, what i recommended was to do link training but in intel_dp_detect. Since
>>>>>> USB Type C cable
>>>>>> also has its own lane count restriction (it can have different lane count
>>>>>> than the one supported
>>>>>> by panel) you might have to figure that out as well. so both reversal and
>>>>>> lane count detection
>>>>>> can be done outside the modeset path and keep the code free of type C
>>>>>> changes outside
>>>>>> detection path.
>>>>>>
>>>>>> Please find below the code to do the same. Do not waste time trying to apply
>>>>>> this directly on
>>>>>> nightly since this is based on a local tree and because this is pre- atomic
>>>>>> changes code, so you
>>>>>> might have to modify chv_upfront_link_train to work on top of the latest
>>>>>> nightly code. we
>>>>>> are supposed to upstream this and is in my todo list.
>>>>>>
>>>>> [original patch snipped...]
>>>>>
>>>>> Hi Sivakumar,
>>>>>
>>>>> So I managed to manually re-apply your patch on top of
>>>>> drm-intel-nightly, and tried to port it to make Broadwell working too.
>>>>> It works OK if the system is already boot without any external DP used.
>>>>> In this case, the detection works and I can see my external monitor
>>>>> working properly.
>>>>>
>>>>> However, if the monitor is cold plugged, the cpu/GPU hangs and I can not
>>>>> understand why. I think I enabled all that is mentioned in the PRM to be
>>>>> able to train the DP link, but I am obviously missing something else.
>>>>> Can you have a look?
>>>>>
>>>> Hi Benjamin,
>>>>
>>>> I would recommend against this approach. Some adapters will claim that
>>>> they recovered a clock even when it isn't on the lanes you enabled,
>>>> which means that the reversal detection doesn't always work. The only
>>>> reliable way to do this is to go talk to the Chrome OS EC (you can
>>>> find these patches later in the Chrome OS tree). It's not as generic,
>>>> but we might be able to abstract that logic, maybe.
>>>>
>>> Hi Stéphane,
>>>
>>> This is a very good news. I was afraid we would not have access to the
>>> hardware controller because the Intel controller hub spec was not
>>> public.
>>>
>>> I will try to have a look at it, but the latest chromeos branch (3.18)
>>> seems to differ quite a lot from the upstream one. Anyway, fingers
>>> crossed.
>>>
>>> Cheers,
>>> Benjamin
>> Hi Benjamin/Stéphan,
> Hi Sivakumar,
>
>> All Intel platforms (at-least those i inquired about) handle lane
>> reversal in HW.
> That is the theory and what is written in the USB Type C spec IIRC.
> Problem is, the Chromebook Pixel 2 external display does not work when a
> USB Type-C adapter is in the reversed position (or believe me, I would
> definitively not have submitted a patch for the beauty of it).
> Everything else works (link training when 4 lanes are activated, or
> other communication channels). Only the order of the 4 data lanes
> matters in this situation and the USB controller does not reverse them
> for us on this laptop.
>> your statement that link training will pass even on reversed lane seems to
>> point
>> to the same fact. in such a scenario why should the encoder/connector care
>> if the lane is reversed or not ?
> Problem is that Stephane said some adapters are lying regarding the
> clock recovery. They claim everything is fine while in the end, the
> display doesn't show anything because the lanes are reversed. If this is
> just a chromebook Pixel 2 issue, that's better then. I won't have to try
> to put some generic interface to notify that the display port lanes have
> to be reversed.
>
> Cheers,
> Benjamin
Thanks for the info :). There is no way in code to confirm if after link
training display
is working properly (other than short pulse for link related issues)
so this has to be on a case by case basis. i would recommend
if you plan to upstream it to restrict the changes for chromebook Pixel
2 alone.
>> --
>> regards,
>> Sivakumar
>>

--
regards,
Sivakumar