From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Cc: "Stéphane Marchesin" <marcheu@chromium.org>,
"Daniel Vetter" <daniel.vetter@intel.com>,
intel-gfx <intel-gfx@lists.freedesktop.org>,
"Todd Broch" <tbroch@chromium.org>,
linux-kernel@vger.kernel.org
Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP
Date: Wed, 26 Aug 2015 10:29:14 -0400 [thread overview]
Message-ID: <20150826142914.GA17672@mail.corp.redhat.com> (raw)
In-Reply-To: <55DDAC84.7060105@intel.com>
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
>
> --
> regards,
> Sivakumar
>
next prev parent reply other threads:[~2015-08-26 14:29 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 ` [Intel-gfx] " 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 [this message]
2015-08-27 5:24 ` Sivakumar Thulasimani
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=20150826142914.GA17672@mail.corp.redhat.com \
--to=benjamin.tissoires@redhat.com \
--cc=daniel.vetter@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marcheu@chromium.org \
--cc=sivakumar.thulasimani@intel.com \
--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