Bug 51463 - [SNB regression] LVDS can't light up while KMS starting
Summary: [SNB regression] LVDS can't light up while KMS starting
Status: CLOSED FIXED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: XOrg git
Hardware: Other All
: medium major
Assignee: Daniel Vetter
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-26 22:40 UTC by Du Yan
Modified: 2017-10-06 14:49 UTC (History)
6 users (show)

See Also:
i915 platform:
i915 features:


Attachments
dmesg LVDS is black (59.31 KB, application/octet-stream)
2012-06-26 22:40 UTC, Du Yan
no flags Details
dmesg hot plug with an external displays (80.00 KB, application/octet-stream)
2012-06-26 22:42 UTC, Du Yan
no flags Details
reg_dump log (12.55 KB, application/octet-stream)
2012-07-03 02:22 UTC, Du Yan
no flags Details
dmesg file while booting up. (58.68 KB, application/octet-stream)
2012-07-03 19:21 UTC, Yi Sun
no flags Details
fixup backlight #defin (1.13 KB, patch)
2012-07-10 22:24 UTC, Daniel Vetter
no flags Details | Splinter Review
The dmesg when the backlight doesn't work. (59.17 KB, text/plain)
2012-07-11 07:58 UTC, Yi Sun
no flags Details
fix for the problem (1.90 KB, patch)
2012-07-14 14:17 UTC, Paulo Zanoni
no flags Details | Splinter Review
cant light up backlight while booting (60.19 KB, text/plain)
2012-07-16 02:37 UTC, Du Yan
no flags Details

Description Du Yan 2012-06-26 22:40:42 UTC
Created attachment 63501 [details]
dmesg LVDS is black

System Environment:
--------------------------
Platform: SandyBridge
Kernel: (drm-intel-next-queued)8ecd1a6615f0d9de6759aafe229bc1cc4ee99c7b
Some additional commit info:
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Jun 7 15:56:03 2012 +0200

drm/i915: call intel_enable_gtt


Bug detailed description:
-------------------------
LVDS can't light up when KMS starting. But so long as doing a hot plug with an external displays (such as VGA), the LVDS can light up.
Both LVDS is black and LVDS light up dmesg files are attached.


We found the LVDS works well on the following commit.
Kernel: (drm-intel-next-queued)172cf15d18889313bf2c3bfb81fcea08369274ef
Some additional commit info:
Author: Ben Widawsky <ben@bwidawsk.net>
Date:   Tue Jun 5 15:24:25 2012 -0700

drm/i915: Add wait render timeout get param
Comment 1 Du Yan 2012-06-26 22:42:21 UTC
Created attachment 63502 [details]
dmesg hot plug with an external displays
Comment 2 Daniel Vetter 2012-06-27 02:43:20 UTC
Can you please check whether reverting

commit 24ded204429fa0f5501d37c63ee35c555c0b75ee
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Tue Jun 5 12:14:54 2012 +0200

    drm/i915: properly enable the blc controller on the right pipe

helps? Otherwise please bisect this issue, thanks.
Comment 3 Du Yan 2012-06-28 00:51:29 UTC
(In reply to comment #2)
> Can you please check whether reverting
> 
> commit 24ded204429fa0f5501d37c63ee35c555c0b75ee
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Tue Jun 5 12:14:54 2012 +0200
> 
>     drm/i915: properly enable the blc controller on the right pipe
> 
> helps? Otherwise please bisect this issue, thanks.


I have finished the bisect.

afba018898ae54b498e82b3cd4d2b61c74032c90 is the first bad commit
commit afba018898ae54b498e82b3cd4d2b61c74032c90
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Tue Jun 12 16:36:45 2012 +0200

    drm/i915: ensure HDMI port is disabled inside set_infoframes
    
    This function is supposed to be used at mode set time, so prevent
    against future mistakes by adding a WARN().
    
    Based on a patch by Paulo Zanoni, with the check extracted into a
    little assert_hdmi_port_disabled helper added to make things self
    documenting and move the assert stuff out of line.
    
    [fixed up spelling goof-up while applying.]
    Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
    Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
    Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
    Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

:040000 040000 8e616fe5a0bb5fa9d7c9f8907b55e299e0995115 ee405fd556509f1e3622ce569041921362307104 M	drivers
Comment 4 Daniel Vetter 2012-06-28 01:58:14 UTC
Hm, this is a strange bisect result, this commit only adds a few asserts (that you shouldn't hit anyway). Can you please check the bisect result by reverting this commit on top of latest -queued?
Comment 5 Du Yan 2012-06-29 01:11:16 UTC
Sorry for my mistake, and I re-bisected, then got the below result: 

534b5a5341cb7e16a98d44623d8fce9464ebf22c is the first bad commit
commit 534b5a5341cb7e16a98d44623d8fce9464ebf22c
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Tue Jun 5 10:07:08 2012 +0200

    drm/i915: pnv has a backlight polarity control bit, too
    
    We already correctly ignore bit0 on gen < 4, now we also know why ;-)
    I've decided that losing that single bit of precision isn't worth the
    trouble to sprinkle IS_PINEVIEW checks all over the backlight control
    code - that code is way too fragile imo.
    
    Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
    Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

:040000 040000 ee405fd556509f1e3622ce569041921362307104 80280715c68c6e6918448fb50bf930ecb871d5f6 M	drivers
Comment 6 Daniel Vetter 2012-06-29 01:49:49 UTC
Hm, 534b5a5341cb7e16a98d44623d8fce9464ebf22c is even more unlikely - this patch only changes a few # defines and comments to improve documentation. This should have _zero_ effect on the code (the older bisect result could at least have resulted in gcc moving things around in relevant parts of the code ...).
Comment 7 Yi Sun 2012-07-01 20:48:22 UTC
I tried to bisect this issue. The culprit commit I think is:

commit 24ded204429fa0f5501d37c63ee35c555c0b75ee
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Tue Jun 5 12:14:54 2012 +0200

    drm/i915: properly enable the blc controller on the right pipe

    On gen4+ we have a bitfield to specify from which pipe the backlight
    controller should take it's clock. For PCH split platforms we've
    already set these up, but only at initialization time. And without
    taking into account the 3rd pipe added with ivb.

    For gen4, we've completely ignored these. Although we do restrict lvds
    to the 2nd pipe, so this is only a problem on machines where we boot
    up with the lvds on the first pipe.

    So restructure the code to enable the backlight on the right pipe at
    modeset time.

    v2: For odd reasons panel_enable_backlight gets called twice in a
    modeset, so we can't WARN_ON in there if the backlight controller is
    switched on already.

    v3: backlight enable can also be called through dpms on, so the check
    in there is legit. Update the comment to reflect that.


The real issue of this bug is that LVDS can't light up while KMS starting. But after DPMS Suspend/stand-by and resume, the LVDS display would work. Hence, that may cause Du Yan's wrong bisect result, i think.
Comment 8 Daniel Vetter 2012-07-02 01:28:04 UTC
> --- Comment #7 from sunyi <yi.sun@intel.com> 2012-07-01 20:48:22 PDT ---
> The real issue of this bug is that LVDS can't light up while KMS starting. But
> after DPMS Suspend/stand-by and resume, the LVDS display would work. Hence,
> that may cause Du Yan's wrong bisect result, i think.

Yeah, that could explain the crazy bisect results. But the strange
thing is really that it only blows up at module reload. Can you please
test a few things:
- What happens when you reload the module?
- What happens when you switch the crtc of the panel, e.g.

xrandr --output LVDS1 --auto --crtc 1

- Can you please grab the output of intel_reg_dumper with i915 disable
(boot with nomodeset) and once when kms has loaded and the screen is
black?
Comment 9 Du Yan 2012-07-03 02:22:34 UTC
Created attachment 63755 [details]
reg_dump log
Comment 11 Daniel Vetter 2012-07-03 02:32:54 UTC
Link to the actual patch: http://cgit.freedesktop.org/~danvet/drm/patch/?id=5d28aa9159fc0c89a1ed272dd44dc73e82dc5398
Comment 12 Du Yan 2012-07-03 17:53:16 UTC
- What happens when you reload the module?
Nothing happened, still show a black screen.

- What happens when you switch the crtc of the panel, e.g. xrandr --output LVDS1 --auto --crtc 1
Screen can light up after start x, and switch the panel (xrandr --output LVDS1 --auto --crtc 1), screen displayed normally, nothing seriously happened.


- Can you please grab the output of intel_reg_dumper with i915 disable(boot with nomodeset) and once when kms has loaded and the screen is black?
Intel _reg_dumper log attached, pls. check it in attachment.
Comment 13 Yi Sun 2012-07-03 19:21:33 UTC
Created attachment 63793 [details]
dmesg file while booting up.

The issue still exists with the patch:
http://cgit.freedesktop.org/~danvet/drm/patch/?id=5d28aa9159fc0c89a1ed272dd44dc73e82dc5398
Comment 14 Daniel Vetter 2012-07-10 22:24:17 UTC
Created attachment 64085 [details] [review]
fixup backlight #defin

Please test this patch, thanks.
Comment 15 Yi Sun 2012-07-11 07:58:53 UTC
Created attachment 64096 [details]
The dmesg when the backlight doesn't work.

The issue still exists with this patch.
Comment 16 Daniel Vetter 2012-07-12 11:36:44 UTC
Can you please test this with my modeset-rework git branch from my personal git repo at:

http://cgit.freedesktop.org/~danvet/drm

Specifically the following commit could be interesting:

http://cgit.freedesktop.org/~danvet/drm/commit/?h=modeset-rework&id=59eb757e2d121dc47c50e1e6fe9099f3f0ac4a31
Comment 17 Paulo Zanoni 2012-07-14 14:17:47 UTC
Created attachment 64206 [details] [review]
fix for the problem

I'm also seeing a backlight regression that seems to be from the commit you mentioned, and the attached patch solves my problem.

Although I did not bisect to check, the way I understand the problem, it was introduced in the mentioned commit.

Please test this patch and report the results.
Comment 18 Du Yan 2012-07-16 02:34:52 UTC
Test Failed with the commit, LVDS still can't light up, dmesg log has attached.

Kernel: (modeset-rework)59eb757e2d121dc47c50e1e6fe9099f3f0ac4a31
Some additional commit info:
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Sat Jun 30 14:40:10 2012 +0200

    drm/i915/lvds: ditch ->prepare special case
Comment 19 Du Yan 2012-07-16 02:37:25 UTC
Created attachment 64251 [details]
cant light up backlight while booting
Comment 20 Daniel Vetter 2012-08-13 07:58:01 UTC
Please retest with latest -fixes, specifically:

commit 770c12312ad617172b1a65b911d3e6564fc5aca8
Author: Takashi Iwai <tiwai@suse.de>
Date:   Sat Aug 11 08:56:42 2012 +0200

    drm/i915: Fix blank panel at reopening lid
Comment 21 Du Yan 2012-08-14 02:30:06 UTC
Test OK with the below commit:

Kernel: (drm-intel-fixes)770c12312ad617172b1a65b911d3e6564fc5aca8
Some additional commit info:
Author: Takashi Iwai <tiwai@suse.de>
Date:   Sat Aug 11 08:56:42 2012 +0200

    drm/i915: Fix blank panel at reopening lid
Comment 22 Daniel Vetter 2012-08-14 07:35:57 UTC
Thanks for confirming.
Comment 23 Elizabeth 2017-10-06 14:49:24 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.