From: Jani Nikula <jani.nikula@linux.intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
"Chris Wilson" <chris@chris-wilson.co.uk>,
"Manasi Navare" <manasi.d.navare@intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v7 1/6] drm/i915: Fallback to lower link rate and lane count during link training
Date: Thu, 29 Sep 2016 18:48:43 +0300 [thread overview]
Message-ID: <87eg42671g.fsf@intel.com> (raw)
In-Reply-To: <20160929151054.GP4329@intel.com>
On Thu, 29 Sep 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Sep 29, 2016 at 12:44:19PM +0100, Chris Wilson wrote:
>> On Thu, Sep 29, 2016 at 02:26:16PM +0300, Jani Nikula wrote:
>> > On Thu, 29 Sep 2016, Manasi Navare <manasi.d.navare@intel.com> wrote:
>> > > On Tue, Sep 27, 2016 at 08:07:01PM +0300, Jani Nikula wrote:
>> > >> On Tue, 27 Sep 2016, Manasi Navare <manasi.d.navare@intel.com> wrote:
>> > >> > On Mon, Sep 26, 2016 at 04:39:34PM +0300, Jani Nikula wrote:
>> > >> >> On Fri, 16 Sep 2016, Manasi Navare <manasi.d.navare@intel.com> wrote:
>> > >> >> > According to the DisplayPort Spec, in case of Clock Recovery failure
>> > >> >> > the link training sequence should fall back to the lower link rate
>> > >> >> > followed by lower lane count until CR succeeds.
>> > >> >> > On CR success, the sequence proceeds with Channel EQ.
>> > >> >> > In case of Channel EQ failures, it should fallback to
>> > >> >> > lower link rate and lane count and start the CR phase again.
>> > >> >>
>> > >> >> This change makes the link training start at the max lane count and max
>> > >> >> link rate. This is not ideal, as it wastes the link. And it is not a
>> > >> >> spec requirement. "The Link Policy Maker of the upstream device may
>> > >> >> choose any link count and link rate as long as they do not exceed the
>> > >> >> capabilities of the DP receiver."
>> > >> >>
>> > >> >> Our current code starts at the minimum required bandwidth for the mode,
>> > >> >> therefore we can't fall back to lower link rate and lane count without
>> > >> >> reducing the mode.
>> > >> >>
>> > >> >> AFAICT this patch here makes it possible for the link bandwidth to drop
>> > >> >> below what is required for the mode. This is unacceptable.
>> > >> >>
>> > >> >> BR,
>> > >> >> Jani.
>> > >> >>
>> > >> >>
>> > >> >
>> > >> > Thanks Jani for your review comments.
>> > >> > Yes in this change we start at the max link rate and lane count. This
>> > >> > change was made according to the design document discussions we had
>> > >> > before strating this DP Redesign project. The main reason for starting
>> > >> > at the maxlink rate and max lane count was for ensuring proper
>> > >> > behavior of DP MST. In case of DP MST, we want to train the link at
>> > >> > the maximum supported link rate/lane count based on an early/ upfront
>> > >> > link training result so that we dont fail when we try to connect a
>> > >> > higher resolution monitor as a second monitor. This a trade off
>> > >> > between wsting the link or higher power vs. needing to retrain for
>> > >> > every monitor that requests a higher BW in case of DP MST.
>> > >>
>> > >> We already train at max bandwidth for DP MST, which seems to be the
>> > >> sensible thing to do.
>> > >>
>> > >> > Actually this is also the reason for enabling upfront link training in
>> > >> > the following patch where we train the link much ahead in the modeset
>> > >> > sequence to understand the link rate and lane count values at which
>> > >> > the link can be successfully trained and then the link training
>> > >> > through modeset will always start at the upfront values (maximum
>> > >> > supported values of lane count and link rate based on upfront link
>> > >> > training).
>> > >>
>> > >> I don't see a need to do this for DP SST.
>> > >>
>> > >> > As per the CTS, all the test 4.3.1.4 requires that you fall back to
>> > >> > the lower link rate after trying to train at the maximum link rate
>> > >> > advertised through the DPCD registers.
>> > >>
>> > >> That test does not require the source DUT to default to maximum lane
>> > >> count or link rate of the sink. The source may freely choose the lane
>> > >> count and link rate as long as they don't exceed sink capabilities.
>> > >>
>> > >> For the purposes of the test, the test setup can request specific
>> > >> parameters to be used, but that does not mean using maximum by
>> > >> *default*.
>> > >>
>> > >> We currently lack the feature to reduce lane count and link rate. The
>> > >> key to understand here is that starting at max and reducing down to the
>> > >> sufficient parameters for the mode (which is where we start now) offers
>> > >> no real benefit for any use case. What we're lacking is a feature to
>> > >> reduce the link parameters *below* what's required by the mode the
>> > >> userspace wants. This can only be achieved through cooperation with
>> > >> userspace.
>> > >>
>> > >
>> > > We can train at the optimal link rate required for the requested mode as
>> > > done in the existing implementation and retrain whenever the link training
>> > > test request is sent.
>> > > For the test 4.3.1.4 in CTS, it does force a failure in CR and expects the
>> > > driver to fall back to even lower link rate. We do not implement this in the
>> > > current driver and so this test fails. Could you elaborate on how this can
>> > > be achieved with the the cooperation with userspace?
>> > > Should we send a uevent to the userspace asking to retry at a lower resolution
>> > > after retraining at the lower link rate?
>> > > This is pertty much the place where majority of the compliance tests are failing.
>> > > How can we pass compliance with regards to this feature?
>> >
>> > So here's an idea Ville and I came up with. It's not completely thought
>> > out yet, probably has some wrinkles still, but then there are wrinkles
>> > with the upfront link training too (I'll get back to those separately).
>> >
>> > If link training fails during modeset (either for real or because it's a
>> > test sink that wants to test failures), we 1) store the link parameters
>> > as failing, 2) send a uevent to userspace, hopefully getting the
>> > userspace to do another get modes and try again, 3) propage errors from
>> > modeset.
>>
>> userspace already tries to do a reprobe after a setcrtc fails, to try
>> and gracefully handle the race between hotplug being in its event queue
>> and performing setcrtc, i.e. I think the error is enough.
>
> I presume we want the modeset to be async, so by the time we notice the
> problem we're no longer in the ioctl.
IOW, we'll just need to send the hotplug uevent anyway.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-09-29 15:48 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-14 1:08 [PATCH 0/5] Remaining patches for upfront link training on DDI platforms Manasi Navare
2016-09-14 1:08 ` [PATCH v5 1/5] drm/i915: Fallback to lower link rate and lane count during link training Manasi Navare
2016-09-14 8:15 ` Mika Kahola
2016-09-15 19:56 ` Manasi Navare
2016-09-14 1:08 ` [PATCH v3 2/5] drm/i915: Remove the link rate and lane count loop in compute config Manasi Navare
2016-09-14 1:08 ` [PATCH v2 3/5] drm/i915: Change the placement of some static functions in intel_dp.c Manasi Navare
2016-09-15 7:41 ` Mika Kahola
2016-09-15 19:08 ` Manasi Navare
2016-09-14 1:08 ` [PATCH v17 4/5] drm/i915/dp: Enable Upfront link training on DDI platforms Manasi Navare
2016-09-14 1:08 ` [PATCH v3 5/5] drm/i915/dp/mst: Add support for upfront link training for DP MST Manasi Navare
2016-09-15 17:48 ` Pandiyan, Dhinakaran
2016-09-15 19:25 ` Manasi Navare
2016-09-19 17:03 ` Jim Bride
2016-09-19 17:22 ` Manasi Navare
2016-09-14 5:38 ` ✓ Fi.CI.BAT: success for Remaining patches for upfront link training on DDI platforms Patchwork
2016-09-16 0:03 ` [PATCH 0/6] " Manasi Navare
2016-09-16 0:03 ` [PATCH v6 1/6] drm/i915: Fallback to lower link rate and lane count during link training Manasi Navare
2016-09-16 9:29 ` Mika Kahola
2016-09-16 18:45 ` [PATCH v7 " Manasi Navare
2016-09-26 13:39 ` Jani Nikula
2016-09-27 15:25 ` Manasi Navare
2016-09-27 17:07 ` Jani Nikula
2016-09-29 6:41 ` Manasi Navare
2016-09-29 11:26 ` Jani Nikula
2016-09-29 11:44 ` Chris Wilson
2016-09-29 15:10 ` Ville Syrjälä
2016-09-29 15:48 ` Jani Nikula [this message]
2016-09-29 16:05 ` Manasi Navare
2016-09-29 23:17 ` Manasi Navare
2016-10-03 23:29 ` Manasi Navare
2016-09-16 0:04 ` [PATCH v3 2/6] drm/i915: Remove the link rate and lane count loop in compute config Manasi Navare
2016-09-26 13:41 ` Jani Nikula
2016-09-27 13:39 ` Jani Nikula
2016-09-27 22:13 ` Manasi Navare
2016-09-28 7:14 ` Jani Nikula
2016-09-28 22:30 ` Manasi Navare
2016-09-27 21:55 ` Manasi Navare
2016-09-28 7:38 ` Jani Nikula
2016-09-28 16:45 ` Manasi Navare
2016-09-29 14:52 ` Jani Nikula
2016-09-16 0:04 ` [PATCH v3 3/6] drm/i915: Change the placement of some static functions in intel_dp.c Manasi Navare
2016-09-16 8:12 ` Mika Kahola
2016-09-16 0:04 ` [PATCH 4/6] drm/i915: Code cleanup to use dev_priv and INTEL_GEN Manasi Navare
2016-09-16 7:40 ` Mika Kahola
2016-09-26 13:45 ` Jani Nikula
2016-09-28 0:03 ` Manasi Navare
2016-09-16 0:04 ` [PATCH v17 5/6] drm/i915/dp: Enable Upfront link training on DDI platforms Manasi Navare
2016-09-20 22:04 ` [PATCH v18 " Manasi Navare
2016-09-27 13:59 ` Jani Nikula
2016-09-29 12:15 ` Jani Nikula
2016-09-29 16:05 ` Jani Nikula
2016-09-16 0:04 ` [PATCH v3 6/6] drm/i915/dp/mst: Add support for upfront link training for DP MST Manasi Navare
2016-09-16 0:47 ` ✓ Fi.CI.BAT: success for series starting with [v6,1/6] drm/i915: Fallback to lower link rate and lane count during link training Patchwork
2016-09-16 19:25 ` ✓ Fi.CI.BAT: success for series starting with [v7,1/6] drm/i915: Fallback to lower link rate and lane count during link training (rev2) Patchwork
2016-09-20 8:45 ` [PATCH 0/6] Remaining patches for upfront link training on DDI platforms Jani Nikula
2016-09-20 22:49 ` ✓ Fi.CI.BAT: success for series starting with [v7,1/6] drm/i915: Fallback to lower link rate and lane count during link training (rev3) Patchwork
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=87eg42671g.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
--cc=manasi.d.navare@intel.com \
--cc=ville.syrjala@linux.intel.com \
/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).