Bug 89554

Sonika?

For me with kernel version 4.0.0-rc3, eDP is working fine.

The EDP low vswing table gets selected only when edp_vswing_preemph is set in vbt.
For me, it works with both low vswing table as well as the dp translation table with an edp1.3 panel.

Only thing I can suspect is that we are trying to use low vswing table for some panel which is not able to handle those values.
Can you please see if this variable is set in vbt?
If it is set, please try to disable it and try.

Please try http://patchwork.freedesktop.org/patch/42306, should impact your skl revision.

(In reply to Jani Nikula from comment #3)
> Please try http://patchwork.freedesktop.org/patch/42306, should impact your
> skl revision.

Tested --next-queued_bbe2b865 with this patch, this bug still exists.

It can works good on the latest -fixes_5e4f51.

(In reply to Sonika from comment #2)
> For me with kernel version 4.0.0-rc3, eDP is working fine.
> 
> The EDP low vswing table gets selected only when edp_vswing_preemph is set
> in vbt.
> For me, it works with both low vswing table as well as the dp translation
> table with an edp1.3 panel.
> 
> Only thing I can suspect is that we are trying to use low vswing table for
> some panel which is not able to handle those values.
> Can you please see if this variable is set in vbt?
> If it is set, please try to disable it and try.

I have no idea, could you tell me how to set this variable? where?

Hm, maybe that means the patch wasn't applied correctly?  Since -fixes works, I'll close.

The latest nightly(f7def43)is still fails.

(In reply to ye.tian from comment #6)
> (In reply to Sonika from comment #2)
> > For me with kernel version 4.0.0-rc3, eDP is working fine.
> > 
> > The EDP low vswing table gets selected only when edp_vswing_preemph is set
> > in vbt.
> > For me, it works with both low vswing table as well as the dp translation
> > table with an edp1.3 panel.
> > 
> > Only thing I can suspect is that we are trying to use low vswing table for
> > some panel which is not able to handle those values.
> > Can you please see if this variable is set in vbt?
> > If it is set, please try to disable it and try.
> 
> I have no idea, could you tell me how to set this variable? where?

There should be one field in the eDP config in vbt VswingPreEmphasisValue. Where it should be set to 0 for low vswing and 1 for default for the panel in use:
    0 =  Low Power Swing setting(200 mV)
    1 =  Default Swing settings(400 mV)

If you have access to the code, you can try hardcoding dev_priv->vbt.edp_low_vswing to false in parse_edp inside intel_bios.c to use the default vswing values

This issue was happened on the configuration with older CPU + older build in panel(LTN133YL01-G01,panel picture attached.)

and this issue still can be produced on latest -nightly branch, seems the fix patch on -fixed branch does not merge into nightly, so reopen this bug.

Does drm-intel-fixes work?

(In reply to Jani Nikula from comment #11)
> Does drm-intel-fixes work?

drm-intel-fixes can work.

And to double check, did your latest drm-intel-nightly test include

commit bec007187636a331a33db324252e7204d8434a31
Author: Damien Lespiau <damien.lespiau@intel.com>
Date:   Wed Feb 11 17:43:24 2015 +0000

    drm/i915/skl: Implement WaDisableHBR2

Please include dmesg for the latest nightly.

Tested nightly(4f7ab66), eDP display still can not be lighted up. please see the dmesg info.

commit 4f7ab66fd78ed940a5fcfcaad7e9f1bf8b1abe76
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Mon Mar 16 18:51:08 2015 +0100

    drm-intel-nightly: 2015y-03m-16d-17h-50m-27s UTC integration manifest

Created attachment 114354 [details]
dmesg info

(In reply to Jani Nikula from comment #13)
> And to double check, did your latest drm-intel-nightly test include
> 
> commit bec007187636a331a33db324252e7204d8434a31
> Author: Damien Lespiau <damien.lespiau@intel.com>
> Date:   Wed Feb 11 17:43:24 2015 +0000
> 
>     drm/i915/skl: Implement WaDisableHBR2

Tested nighlty(bec0071, eDP dislay can not be lighted up.

our drm-intel-nightly branch did included this commit:
commit bec007187636a331a33db324252e7204d8434a31
Author: Damien Lespiau <damien.lespiau@intel.com>
Date:   Wed Feb 11 17:43:24 2015 +0000

    drm/i915/skl: Implement WaDisableHBR2

and also build the kernel based on this commit, SKL edp still not light up with this commit.

(In reply to Sonika from comment #9)
> (In reply to ye.tian from comment #6)
> > (In reply to Sonika from comment #2)
> > > For me with kernel version 4.0.0-rc3, eDP is working fine.
> > > 
> > > The EDP low vswing table gets selected only when edp_vswing_preemph is set
> > > in vbt.
> > > For me, it works with both low vswing table as well as the dp translation
> > > table with an edp1.3 panel.
> > > 
> > > Only thing I can suspect is that we are trying to use low vswing table for
> > > some panel which is not able to handle those values.
> > > Can you please see if this variable is set in vbt?
> > > If it is set, please try to disable it and try.
> > 
> > I have no idea, could you tell me how to set this variable? where?
> 
> There should be one field in the eDP config in vbt VswingPreEmphasisValue.
> Where it should be set to 0 for low vswing and 1 for default for the panel
> in use:
>     0 =  Low Power Swing setting(200 mV)
>     1 =  Default Swing settings(400 mV)
> 
> If you have access to the code, you can try hardcoding
> dev_priv->vbt.edp_low_vswing to false in parse_edp inside intel_bios.c to
> use the default vswing values

Hello, we read GOP driver's vbt info:   pre-emphasis was set to level-0 by default, Voltage Swing was set to swing-0 by default.
Both the machines which installed older and newer panels flashed the same GOP driver version.

Can you please also attach the dmesg with the working panel?
This panel reports HBR2 as the max link rate in DPCD:
[    1.804801] [drm:intel_dp_get_dpcd] DPCD: 12 14 c4 41 00 00 01 c0 02 00 00 00 1f 0b 00

Also, the edp low vswing is selected based on one parameter in vbt VswingPreEmphasis Value which is a separate field other than the link parameters where it specifies the vswing and pre-emphasis level.

Following is my bios info from dmesg logs:
Intel Corporation Skylake Client platform/Skylake AIO DDR3L
RVP10, BIOS SKLSE2P1.86C.B060.R01.1411140050

Please note that, the low_vswing_preemph value is set to 1 in my case.
I don't have the vbt for this one to really see the value. But I added some logs and from there I see that this value is 1 which means default swing settings (400mv)

I tried on another SKL system, which has panel whose DPCD is same as your panels with HBR2 enabled:
[drm:intel_dp_get_dpcd] DPCD: 12 14 c4 41 00 00 01 c0 02 00 00 00 1f 0b 00

System + bios details:
Intel Corporation Skylake Client platform/Skylake U DDR3L RVP7
BIOS SKLSE2R1.86C.B067.R00.1412310711 12/31/2014

It still works for me with default table as well as edp translation table.
Please note that, I try by hardcoding this value in code to make use of the low vswing table.

Created attachment 114370 [details]
dmesg info with the working panel

Applied Sonika's apply which hardcoding
dev_priv->vbt.edp_low_vswing to false in parse_edp inside intel_bios.c, the older edp panel is working.

(In reply to wendy.wang from comment #23)
> Applied Sonika's apply which hardcoding
> dev_priv->vbt.edp_low_vswing to false in parse_edp inside intel_bios.c, the
> older edp panel is working.

Correct the typo: applied Sonika's patch(attached.)

Created attachment 114375 [details]
0001-Test-commit-for-edp-low-vswing-issue

Created attachment 114376 [details]
dmesg_with_hardcode_patch

So, this panel doesn't support low vswing and pre-emphasis settings and should use the default DP translation table:
 0 =  Low Power Swing setting(200 mV)
 1 =  Default Swing settings(400 mV)

The VBT sets this value as per the panel requirement from panel vendor.
Looks like in the VBT it is set to Low Power Swing setting which is not correct always.
By default it should be set to the Default Vswing/Preemph table.

(In reply to ye.tian from comment #17)
> (In reply to Jani Nikula from comment #13)
> > And to double check, did your latest drm-intel-nightly test include
> > 
> > commit bec007187636a331a33db324252e7204d8434a31
> > Author: Damien Lespiau <damien.lespiau@intel.com>
> > Date:   Wed Feb 11 17:43:24 2015 +0000
> > 
> >     drm/i915/skl: Implement WaDisableHBR2
> 
> Tested nighlty(bec0071, eDP dislay can not be lighted up.

According to the dmesg you have rev C0, which means the WA is not applicable.

Sonika, why do you need to look at VBT to support edp1.4 low vswing? Is the panel support not available from DPCD?

(In reply to Jani Nikula from comment #29)
> Sonika, why do you need to look at VBT to support edp1.4 low vswing? Is the
> panel support not available from DPCD?

Ideally, these low vswing/preemph values are supported from edp1.4 onwards.
But there are panels (edp1.2, 1.3) which can support low vswing values.
Thats why the AEs get the right value for particular panel and appropriately set the field in vbt if it can support low values (200mV)

Sonika, please send a patch to ignore VBT and do this for edp1.4 only. It appears we can't trust VBT now.

(In reply to Jani Nikula from comment #31)
> Sonika, please send a patch to ignore VBT and do this for edp1.4 only. It
> appears we can't trust VBT now.

But this will not be correct. VBT should default it to the default settings and as per panels it should be enabled or disabled.
There are other platforms which don't even support edp1.4 but have low vswing table.
So, low vswing selection should be based on vbt and not based upon edp1.4 panel.

We can base it on VBT when we can trust the VBT.

(In reply to Jani Nikula from comment #33)
> We can base it on VBT when we can trust the VBT.

I really feel, that it should just be fixed in VBT to default value. This table only gives the values which can be used to save power which can be achieved with any panel which supports them and not necessarily be edp1.4.

(In reply to Sonika from comment #34)
> (In reply to Jani Nikula from comment #33)
> > We can base it on VBT when we can trust the VBT.
> 
> I really feel, that it should just be fixed in VBT to default value. This
> table only gives the values which can be used to save power which can be
> achieved with any panel which supports them and not necessarily be edp1.4.

If we have shipping broken VBT out there then we can't trust it. That's how this works, because waiting for VBT to get fixed isn't an option. For pre-production it's a bit different, but again if too many ppl are affected we have to just ignore vbt unfortunately. And if a vbt escaped into the wild on shipping hw we've lost entirely.

Maybe in some shiny bright future we can start to trust vbt by default, but right now that's just not reality.

As per the VBT VBIOS doc, default value in VBT should be 1 and not 0. If we are setting it to 0 in VBT, it should be based upon the knowledge of the edp panel
VswingPreEmphasisValue:
    0 =  Low Power Swing setting(200 mV)
    1 =  Default Swing settings(400 mV)

I checked different versions of the GOP driver, and till 9.0.19, the default setting was 1, while from 9.0.20 (which Wendy is using), it changed the value to 0, which is not right.
Then in the release notes of this gop driver I see, that it was added to fix one bug:
https://vthsd.fm.intel.com/hsd/pcgsw/default.aspx#bug/default.aspx?bug_id=5642130

From the release notes:
[DCN][4132577] : [BUG] 5642130 : Change default VBT Setting to use Low Vswing and Pre-emphasis for eDP display. : 9.0.1020

I strongly feel, we should not put a edp1.4 check there because its not related.

For now, I would suggest to default it back to 1 in vbt and stitch a bios or use one version older bios with gop 9.0.19.

Sonika, can you follow up with the GOP team and make sure this is fixed going forward and also see if this code made it into any OEM reference BIOSes?  If it did, we may have to add a workaround to the kernel driver, since we can't rely on users upgrading their BIOS.

(In reply to Jesse Barnes from comment #37)
> Sonika, can you follow up with the GOP team and make sure this is fixed
> going forward and also see if this code made it into any OEM reference
> BIOSes?  If it did, we may have to add a workaround to the kernel driver,
> since we can't rely on users upgrading their BIOS.

Again, it is very panel specific what a panel can support/not support. 
We can't really have a workaround in the code. At most, we can just disable this feature for now and simply use the DP translation table (and not the low power table even though some panel might support it). Mostly all panels should support the default values
GOP team is still in discussion for the optimal values. I will check on that.

I guess this isn't as simple as trying other vswing params, since it affects the DDI side too.  But ideally, if we failed to train like we see here, we'd try a higher set of vswing values on the DDI side and restart DP training.  How feasible is that?  If Windows does that, we'll probably see busted VBTs in the wild too, so making our re-training code more robust would be helpful.

(In reply to Jesse Barnes from comment #39)
> I guess this isn't as simple as trying other vswing params, since it affects
> the DDI side too.  But ideally, if we failed to train like we see here, we'd
> try a higher set of vswing values on the DDI side and restart DP training. 
> How feasible is that?  If Windows does that, we'll probably see busted VBTs
> in the wild too, so making our re-training code more robust would be helpful.

The windows too don't try to retrain with different set of values.
Since any set of values cant guarantee working of all the panels universally, the end system integrators are expected to try both the settings in VBT.
This suggestion of retraining with higher/lower values came up earlier too.
We can give it a try.

Ok, so Sonika provided a module parameter to workaround bad VBTs for those early SDP packages. We don't really want to over-complicate the code to deal with development platforms shipping with a default VBT configuration.


For this workaround, one needs:

Author: Sonika Jindal <sonika.jindal@intel.com>
Date:   Wed May 6 17:35:48 2015 +0530

    drm/i915/skl: Add module parameter to select edp vswing table

And give the following command line parameter to the kernel

edp_vswing=2

Of course, I meant i915.edp_vswing=2

Test it on the latest nightly kernel(87afe90f) with i915.edp_vswing=2, This issue has gone away.

(In reply to ye.tian from comment #43)
> Test it on the latest nightly kernel(87afe90f) with i915.edp_vswing=2, This
> issue has gone away.

Without i915.edp_vswing=2, This issue also has gone away.
Verified.

(In reply to ye.tian from comment #44)
> (In reply to ye.tian from comment #43)
> > Test it on the latest nightly kernel(87afe90f) with i915.edp_vswing=2, This
> > issue has gone away.
> 
> Without i915.edp_vswing=2, This issue also has gone away.
> Verified.

Without i915.edp_vswing=2, the issue is not reproducible? Strange..
By default it uses VBT's input.
If it works without this param, are you sure you are using the original setup with default vbt (and not the stitched one in which the default value was changed)?

(In reply to Sonika from comment #45)
> (In reply to ye.tian from comment #44)
> > (In reply to ye.tian from comment #43)
> > > Test it on the latest nightly kernel(87afe90f) with i915.edp_vswing=2, This
> > > issue has gone away.
> > 
> > Without i915.edp_vswing=2, This issue also has gone away.
> > Verified.
> 
> Without i915.edp_vswing=2, the issue is not reproducible? Strange..
> By default it uses VBT's input.
> If it works without this param, are you sure you are using the original
> setup with default vbt (and not the stitched one in which the default value
> was changed)?

I am sure.
BTW, I re-test it on the other machine, it can't work without this param.

Closing old verified.