Bug 89554

Summary: [SKL Bisected] eDP can not be lighted up after entering kernel
Product: DRI Reporter: ye.tian <yex.tian>
Component: DRM/IntelAssignee: Sonika <sonika.jindal>
Status: CLOSED FIXED QA Contact: Intel GFX Bugs mailing list <intel-gfx-bugs>
Severity: critical    
Priority: highest CC: intel-gfx-bugs
Version: unspecified   
Hardware: All   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
dmesg with drm.debug info
none
dmesg info
none
dmesg info with the working panel
none
0001-Test-commit-for-edp-low-vswing-issue
none
dmesg_with_hardcode_patch none

Description ye.tian 2015-03-12 08:33:53 UTC
Created attachment 114247 [details]
dmesg with drm.debug info

System Environment:       
-----------------------------------------------------
Platform: SKL with new version CPU
Regression: Yes

==Kernel==
--------------------------------------------------
commit 3c0401df292f3cdf60435342a05bee3efb099ca0
Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Date:   Wed Mar 11 13:35:43 2015 +0200

    drm/i915: Simplify the way BC bifurcation state consistency is kept


Bug detailed description:
--------------------------------------------------
Edp display show blank screen with backlight is ON after entering kernel, while SSH still working.
If connect mini-DP, mini-DP can display normally.

It’s kernel regression, Bisect shows the first bad commit is (next-queued)7ad14a29f005.

commit 7ad14a29f0051ba4b631fb8ab3013e4577ffee95
Author:     Sonika Jindal <sonika.jindal@intel.com>
AuthorDate: Wed Feb 25 10:29:12 2015 +0530
Commit:     Daniel Vetter <daniel.vetter@ffwll.ch>
CommitDate: Wed Feb 25 16:06:42 2015 +0100

    drm/i915/skl: Add support for edp1.4 low vswing

    Based upon vbt's vswing preemph settings value select the appropriate
    translations for edp.


Please see the dmesg info.

Reproduce steps:
----------------------------
1, Boot up system with edp display connected
Comment 1 Jani Nikula 2015-03-12 08:53:16 UTC
Sonika?
Comment 2 Sonika 2015-03-12 12:00:39 UTC
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.
Comment 3 Jani Nikula 2015-03-12 18:36:50 UTC
Please try http://patchwork.freedesktop.org/patch/42306, should impact your skl revision.
Comment 4 ye.tian 2015-03-13 04:09:09 UTC
(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.
Comment 5 ye.tian 2015-03-13 04:15:26 UTC
It can works good on the latest -fixes_5e4f51.
Comment 6 ye.tian 2015-03-13 12:19:11 UTC
(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?
Comment 7 Jesse Barnes 2015-03-13 15:08:50 UTC
Hm, maybe that means the patch wasn't applied correctly?  Since -fixes works, I'll close.
Comment 8 ye.tian 2015-03-16 02:28:09 UTC
The latest nightly(f7def43)is still fails.
Comment 9 Sonika 2015-03-16 05:19:33 UTC
(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
Comment 10 wendy.wang 2015-03-16 08:36:08 UTC
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.
Comment 11 Jani Nikula 2015-03-16 09:26:23 UTC
Does drm-intel-fixes work?
Comment 12 ye.tian 2015-03-16 09:36:12 UTC
(In reply to Jani Nikula from comment #11)
> Does drm-intel-fixes work?

drm-intel-fixes can work.
Comment 13 Jani Nikula 2015-03-16 09:45:55 UTC
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
Comment 14 Jani Nikula 2015-03-16 09:47:00 UTC
Please include dmesg for the latest nightly.
Comment 15 ye.tian 2015-03-17 00:59:50 UTC
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
Comment 16 ye.tian 2015-03-17 01:01:12 UTC
Created attachment 114354 [details]
dmesg info
Comment 17 ye.tian 2015-03-17 02:39:28 UTC
(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.
Comment 18 wendy.wang 2015-03-17 02:45:07 UTC
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.
Comment 19 wendy.wang 2015-03-17 03:51:41 UTC
(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.
Comment 20 Sonika 2015-03-17 05:06:23 UTC
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)
Comment 21 Sonika 2015-03-17 05:21:13 UTC
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.
Comment 22 ye.tian 2015-03-17 06:28:48 UTC
Created attachment 114370 [details]
dmesg info with the working panel
Comment 23 wendy.wang 2015-03-17 07:39:55 UTC
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.
Comment 24 wendy.wang 2015-03-17 07:40:39 UTC
(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.)
Comment 25 wendy.wang 2015-03-17 07:41:23 UTC
Created attachment 114375 [details]
0001-Test-commit-for-edp-low-vswing-issue
Comment 26 wendy.wang 2015-03-17 07:43:36 UTC
Created attachment 114376 [details]
dmesg_with_hardcode_patch
Comment 27 Sonika 2015-03-17 08:01:03 UTC
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.
Comment 28 Jani Nikula 2015-03-17 08:17:47 UTC
(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.
Comment 29 Jani Nikula 2015-03-17 08:21:39 UTC
Sonika, why do you need to look at VBT to support edp1.4 low vswing? Is the panel support not available from DPCD?
Comment 30 Sonika 2015-03-17 08:24:46 UTC
(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)
Comment 31 Jani Nikula 2015-03-17 08:29:56 UTC
Sonika, please send a patch to ignore VBT and do this for edp1.4 only. It appears we can't trust VBT now.
Comment 32 Sonika 2015-03-17 09:12:38 UTC
(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.
Comment 33 Jani Nikula 2015-03-17 09:23:53 UTC
We can base it on VBT when we can trust the VBT.
Comment 34 Sonika 2015-03-17 09:34:25 UTC
(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.
Comment 35 Daniel Vetter 2015-03-17 12:48:53 UTC
(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.
Comment 36 Sonika 2015-03-18 05:19:20 UTC
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.
Comment 37 Jesse Barnes 2015-03-26 01:08:44 UTC
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.
Comment 38 Sonika 2015-03-26 04:11:30 UTC
(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.
Comment 39 Jesse Barnes 2015-04-17 15:36:54 UTC
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.
Comment 40 Sonika 2015-04-18 00:25:12 UTC
(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.
Comment 41 Damien Lespiau 2015-05-06 15:06:06 UTC
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
Comment 42 Damien Lespiau 2015-05-06 15:06:28 UTC
Of course, I meant i915.edp_vswing=2
Comment 43 ye.tian 2015-05-07 01:12:11 UTC
Test it on the latest nightly kernel(87afe90f) with i915.edp_vswing=2, This issue has gone away.
Comment 44 ye.tian 2015-05-07 02:10:32 UTC
(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.
Comment 45 Sonika 2015-05-07 04:19:09 UTC
(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)?
Comment 46 ye.tian 2015-05-07 06:00:00 UTC
(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.
Comment 47 Elizabeth 2017-10-06 14:31:06 UTC
Closing old verified.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.