Re: [PATCHv2 4/4] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "R, Durgadoss" <durgadoss.r@intel.com>
Cc: "Conselvan De Oliveira,
	Ander" <ander.conselvan.de.oliveira@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCHv2 4/4] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
Date: Mon, 15 Feb 2016 15:25:48 +0200	[thread overview]
Message-ID: <20160215132548.GU23290@intel.com> (raw)
In-Reply-To: <4D68720C2E767A4AA6A8796D42C8EB59093B0B84@BGSMSX101.gar.corp.intel.com>

On Mon, Feb 15, 2016 at 06:55:07AM +0000, R, Durgadoss wrote:
> Hi Ville,
> 
> >-----Original Message-----
> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >Sent: Friday, February 12, 2016 10:43 PM
> >To: R, Durgadoss <durgadoss.r@intel.com>
> >Cc: intel-gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander
> ><ander.conselvan.de.oliveira@intel.com>
> >Subject: Re: [Intel-gfx] [PATCHv2 4/4] drm/i915/dp: Enable Upfront link training for typeC DP support on
> >BXT
> >
> >On Mon, Feb 01, 2016 at 07:27:53PM +0530, Durgadoss R wrote:
> >> To support USB type C alternate DP mode, the display driver needs to
> >> know the number of lanes required by the DP panel as well as number
> >> of lanes that can be supported by the type-C cable. Sometimes, the
> >> type-C cable may limit the bandwidth even if Panel can support
> >> more lanes. To address these scenarios, the display driver will
> >> start link training with max lanes, and if that fails, the driver
> >> falls back to x2 lanes; and repeats this procedure for all
> >> bandwidth/lane configurations.
> >>
> >> * Since link training is done before modeset only the port
> >>   (and not pipe/planes) and its associated PLLs are enabled.
> >> * On DP hotplug: Directly start link training on the crtc
> >>   associated with the DP encoder.
> >> * On Connected boot scenarios: When booted with an LFP and a DP,
> >>   typically, BIOS brings up DP. In these cases, we disable the
> >>   crtc, do upfront link training and then re-enable it back.
> >> * All local changes made for upfront link training are reset
> >>   to their previous values once it is done; so that the
> >>   subsequent modeset is not aware of these changes.
> >
> >Glancing over this stuff, it doesn't look all that good on the locking
> >front. Mainly there doesn't seem to locking.
> >
> >In general I'm not really convinced upfront link training is a very good
> >idea if it needs a pipe. What happens if we fail to find a free pipe?
> >
> >I'd be much happier about this if we could make do without a pipe at
> 
> We actually don't enable/disable the pipe. We need the crtc structures
> because values like port clock/link bw/dpll hw state are stored in
> crtc->config structures. This is why we had to touch crtc related
> data structures.
> 
> Ander has pulled the dpll_hw_states out of crtc structures and made them
> independent. With this, things should improve.. I need to try this
> implementation to see if we can/can't completely get away
> with modifying crtc structures.

One option might involve adding a fake crtc if we can't easily detangle it
from the code. But obviously if we can remove the crtc dependency things
ought to be much cleaner.

I already had the thought that we might want to add fake crtcs for
dealing with the MST main link, and just use atomic to modeset both
the main link with its fake crtc and real encoder alongside any streams
with their real crtcs and fake encoders.

> 
> >least on some modern platforms and actually limit the feature to
> >those platforms. PLLs are another concern, but I think modern platforms
> >often have some kind of way to always get the standard DP frequencies
> >from eg. the LCPLL.
> >
> >If we do go with the "hope you find a free pipe" approach, then it
> >really needs to do that atomic locking stuff right. Otherwise I'd expect
> 
> Yes, we missed it. Ander pointed out the place where we need this
> locking. So, we will re-work this.

One concern is how whether it might block other threads from eg. page
flipping. I suppose if we have independent crtcs it should be fine since
page flip won't require the connetion_mutex.

> 
> Thanks,
> Durga
> 
> >fireworks when someone plugs in a display while there's modeset
> >happening.
> >
> >--
> >Ville Syrjälä
> >Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx