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

[Rodrigo Vivi]

we confirmed that with current supported CNL skus using IS_CANNONLAKE
is enogh for now.

So feel free to change that if and use

Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

on the v2....


On Tue, Aug 1, 2017 at 1:23 PM, Vivi, Rodrigo <rodrigo.vivi at intel.com> wrote:
> + 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;
>>               }
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br