[Intel-gfx] [PATCH] drm/i915/cnl: DDIA Lane capability bit not set in clone mode

[Vivi, Rodrigo]

+ Art, couple questions below

On Tue, 2017-08-01 at 09:56 -0700, clinton.a.taylor at intel.com wrote:
> From: Clint Taylor <clinton.a.taylor at intel.com>
> 
> DDIA Lane capability control 4 lane bit is not being set by firmware during
> clone mode boot. This occurs when multiple monitors are connected during
> boot. The driver will configure the port for 2 lane maximum width if this
> bit is not set.
> 
> Once DDIA/E lane split is supported in vbt and the i915 driver we will need
> to revisit this code.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: Clint Taylor <clinton.a.taylor at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 494fbe0..e7644b4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2713,9 +2713,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)

we would need to fix more comment lines:

/*
* Bspec says that DDI_A_4_LANES is the only supported
configuration                                                                       
         * for Broxton.  Yet some BIOS fail to set this bit on port A if
eDP                                                                       
         * wasn't lit up at boot.  Force this bit on in our internal 
>  	 * configuration so that we use the proper lane count for our
>  	 * calculations.
>  	 */

But I believe the approach we currently have with this might not be
optimal. If BIOS is not bringing the port A up why should we expect it
to set this bit ever? We probably need to be able to do it by ourselves,
without expecting others to do...

However, I've never seen a production BIOS that never brings port A when
it is present. And it is also true that spec tells: "This field must be
programmed at system boot based on board configuration and may not be
changed afterwards."

So I have kind of mixed feelings here on this bit.

For BXT it is so clear: "Not supported on Broxton."

Also few other future cases the 0 won't be valid, but not entirely sure
if this is always true. Art?

I just noticed on spec that DDI E for CNL-U and CNL-Y are marked as "not
supported", what means that for that SKU we should be able to add this
workaround here, but at same time means that we need to remove the power
wells support for DDI-E for these SKUs probably.

Art, could/should we set this bit blindly for all CNL? 

and if yes:

Art, will it always be a decision based by SKUs now on? Or should we
really rely on BIOS?

Is there another way to detect the port E presence? I don't believe VBT
has that info in a reliable way, does it? Otherwise we would be using it
already for the missed straps...


Thanks,
Rodrigo.

> -	if (IS_GEN9_LP(dev_priv) && port == PORT_A) {
> +	if ((IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10) &&
> +	    port == PORT_A) {
>  		if (!(intel_dig_port->saved_port_bits & DDI_A_4_LANES)) {
> -			DRM_DEBUG_KMS("BXT BIOS forgot to set DDI_A_4_LANES for port A; fixing\n");
> +			DRM_DEBUG_KMS("BIOS forgot to set DDI_A_4_LANES for port A\n");
>  			intel_dig_port->saved_port_bits |= DDI_A_4_LANES;
>  			max_lanes = 4;
>  		}