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

[R, Durgadoss]

Hi Ville,

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
>Sent: Friday, February 12, 2016 10:43 PM
>To: R, Durgadoss <durgadoss.r at intel.com>
>Cc: intel-gfx at lists.freedesktop.org; Conselvan De Oliveira, Ander
><ander.conselvan.de.oliveira at 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.

>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.

Thanks,
Durga

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