Bug 51463

Created attachment 63502 [details]
dmesg hot plug with an external displays

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.

(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

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?

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

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 ...).

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 #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?

Created attachment 63755 [details]
reg_dump log

Can you please also test this patch:

http://cgit.freedesktop.org/~danvet/drm/commit/?h=modeset-rework&id=5d28aa9159fc0c89a1ed272dd44dc73e82dc5398

Link to the actual patch: http://cgit.freedesktop.org/~danvet/drm/patch/?id=5d28aa9159fc0c89a1ed272dd44dc73e82dc5398

- 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.

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

Created attachment 64085 [details] [review]
fixup backlight #defin

Please test this patch, thanks.

Created attachment 64096 [details]
The dmesg when the backlight doesn't work.

The issue still exists with this patch.

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

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.

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

Created attachment 64251 [details]
cant light up backlight while booting

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

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

Thanks for confirming.

Closing old verified.