From: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
To: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>,
Todd Broch <tbroch@chromium.org>,
linux-kernel@vger.kernel.org,
Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [PATCH 3/3] drm/i915: Support DDI lane reversal for DP
Date: Thu, 27 Aug 2015 10:54:19 +0530 [thread overview]
Message-ID: <55DE9F03.7090603@intel.com> (raw)
In-Reply-To: <20150826142914.GA17672@mail.corp.redhat.com>
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
prev parent reply other threads:[~2015-08-27 5:24 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-28 16:03 [PATCH 0/3] drm/i915: fix USB Type-C reversed connector Benjamin Tissoires
2015-07-28 16:03 ` [PATCH 1/3] drm/i915: add parameters to dp_start_link_train and dp_complete_link_train Benjamin Tissoires
2015-07-28 16:03 ` [PATCH 2/3] drm/i915: hide errors when probing for a reverse display port Benjamin Tissoires
2015-07-28 16:18 ` [Intel-gfx] " Chris Wilson
2015-07-29 15:20 ` Benjamin Tissoires
2015-07-28 16:03 ` [PATCH 3/3] drm/i915: Support DDI lane reversal for DP Benjamin Tissoires
2015-07-29 8:26 ` Sivakumar Thulasimani
2015-07-29 15:22 ` Benjamin Tissoires
2015-07-30 4:13 ` Sivakumar Thulasimani
2015-07-30 14:44 ` Benjamin Tissoires
2015-08-05 19:34 ` Benjamin Tissoires
2015-08-06 9:41 ` Sivakumar Thulasimani
2015-08-14 23:27 ` Stéphane Marchesin
2015-08-17 20:06 ` Benjamin Tissoires
2015-08-26 12:09 ` Sivakumar Thulasimani
2015-08-26 14:29 ` Benjamin Tissoires
2015-08-27 5:24 ` Sivakumar Thulasimani [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55DE9F03.7090603@intel.com \
--to=sivakumar.thulasimani@intel.com \
--cc=benjamin.tissoires@redhat.com \
--cc=daniel.vetter@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tbroch@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).