Bug 47641 - [ilk cpu edp] Colour dithering not working for Ironlake on eDP.
Summary: [ilk cpu edp] Colour dithering not working for Ironlake on eDP.
Status: RESOLVED PATCH_ALREADY_AVAILABLE
Alias: None
Product: Drivers
Classification: Unclassified
Component: Video(DRI - Intel) (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_video-dri-intel@kernel-bugs.osdl.org
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-18 01:21 UTC by Jeremy Murphy
Modified: 2012-12-11 10:00 UTC (History)
2 users (show)

See Also:
Kernel Version: 3.4.10
Tree: Mainline
Regression: Yes


Attachments
3.4.10 dmesg with drm debugging (88.19 KB, text/plain)
2012-09-19 09:05 UTC, Jeremy Murphy
Details
dmesg, 3.7.0_rc2-drm-intel-fixes, w/debugging (94.95 KB, text/plain)
2012-10-25 01:06 UTC, Jeremy Murphy
Details
dmesg for 3.7.0-rc3-00624-g1ed5996: Works. (96.72 KB, text/plain)
2012-11-02 00:39 UTC, Jeremy Murphy
Details

Description Jeremy Murphy 2012-09-18 01:21:23 UTC
Changes to i915 driver introduced in 3.4.10 have broken dithering, at least for this particular configuration.  The display appears in what looks like 15/16-bit colour.  Usually the display is in 24-bit (presumably dithered) colour.
Comment 1 Daniel Vetter 2012-09-19 08:38:14 UTC
Please more details:
- dmesg with drm.debug=0xe added to your bootline
- make&model of your machine
- effects your seeing (screenshot with a camera would be awesome)

Also, if you can bisect the offending commit which broke things for you, that'd be really useful
Comment 2 Jeremy Murphy 2012-09-19 09:05:00 UTC
Created attachment 80511 [details]
3.4.10 dmesg with drm debugging

The machine is an HP EliteBook 2540p.  I don't have time to bisect at the moment, sorry.  Works fine with 3.4.9.  The effect is exactly the same as the last image on the following monitor test page under Reduced color depth.  So I guess I am making an assumption by saying that the dithering is broken, maybe something else is.

http://www.lagom.nl/lcd-test/gradient.php
Comment 3 Jeremy Murphy 2012-09-20 11:10:24 UTC
Do you still need more info?
Comment 4 Daniel Vetter 2012-09-20 11:46:52 UTC
The bisect result would help a lot, since right now I have no idea what's going wrong - we didn't change anything that obviously affects dithering on eDP.
Comment 5 Jeremy Murphy 2012-09-20 11:59:15 UTC
Oh, I saw there were a bunch of commits to 3.4.10 regarding drm/i915, such as:


commit c02a29650148f353b0fc86b7eba3f99ecafd23dd
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Fri Aug 10 11:10:20 2012 +0200

    drm/i915: ignore eDP bpc settings from vbt


I assumed that it would be caused by one of these.

I'll get the bisect to you asap, but that means in a few weeks.
Comment 6 Daniel Vetter 2012-09-20 12:18:30 UTC
(In reply to comment #5)
> commit c02a29650148f353b0fc86b7eba3f99ecafd23dd
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Fri Aug 10 11:10:20 2012 +0200
> 
>     drm/i915: ignore eDP bpc settings from vbt
> 
> 
> I assumed that it would be caused by one of these.

Yeah, but it would be really astonishing if it's this one here - that should only cause _more_ dithering, not less.
Comment 7 Jani Nikula 2012-10-02 07:03:28 UTC
I wonder if it might get fixed with

http://cgit.freedesktop.org/~danvet/drm-intel/commit/?id=0c96c65b48fba3ffe9822a554cbc0cd610765cd5

Unfortunately 3.4.10 didn't have enough logging to be sure from the dmesg.
Comment 8 Daniel Vetter 2012-10-02 07:30:26 UTC
I'll lean out the window here a bit and claim this fixed with

commit 0c96c65b48fba3ffe9822a554cbc0cd610765cd5                                                                                      
Author: Jani Nikula <jani.nikula@intel.com>                                                                                          
Date:   Wed Sep 26 18:43:10 2012 +0300                                                                                               
                                                                                                                                     
    drm/i915: use adjusted_mode instead of mode for checking the 6bpc force flag

Thanks a lot for your bug report, and if this commit does not fix your issue, please reopen.
Comment 9 Jeremy Murphy 2012-10-02 09:07:23 UTC
Great, thanks.  I'll endeavour to test it soon, cheers.
Comment 10 Jeremy Murphy 2012-10-02 11:36:48 UTC
Hmm, testing it is not as trivial as I had hoped.  Which drm-intel branch should I check out to test it?  And, without giving me a lesson in git, how?  I use git regularly for simple stuff, but I've just never played with the kernel git repository and branches before, so it's a little advanced for me.  I've checked out the kernel sources but I'm not sure how to get/integrate the drm-intel-* branches.  If it is too complicated to explain, don't worry, I'll figure it out somehow.
Comment 11 Jani Nikula 2012-10-02 11:52:40 UTC
If you have the upstream kernel repo, here's one way:

$ git remote add drm-intel git://people.freedesktop.org/~danvet/drm-intel
$ git fetch drm-intel
$ git checkout -b foo drm-intel/drm-intel-next-queued
$ make localmodconfig        # or use your distro config
$ make
Comment 12 Jeremy Murphy 2012-10-03 02:23:50 UTC
Thanks for the instructions!  Unfortunately, the bug persists.  Just to confirm, the kernel version that I get from 'uname -r' is: 3.6.0-rc7-aufs3-00286-gff1f525 (the "aufs3" is from me).

Now that I have the kernel sources checked out, though, I can start doing a bisect.

Is it possible/easy to test this bug with a virtualized kernel, or will I have to run it on the bare metal and reboot every time?
Comment 13 Daniel Vetter 2012-10-03 07:28:40 UTC
Oops, we've told you the wrong branch to test, -next-queued doesn't have the fix. You need drm-intel-fixes:

# git checkout drm-intel/drm-intel-fixes

(this will yell around a bit about detached head, but should work)

You can the check with gitk or git log whether the patch is really in your current checkout.
Comment 14 Jeremy Murphy 2012-10-05 09:40:56 UTC
I checked that the patch in question was in the log, but alas, the bug persists.
Comment 15 Jani Nikula 2012-10-05 14:18:30 UTC
(In reply to comment #14)
> I checked that the patch in question was in the log, but alas, the bug
> persists.

Hmm, back to square one then. Bisecting between 3.4.9 and 3.4.10 is only 4 steps, if those are the known good and bad versions for you. That would probably give us the best clue as to what's going on.

Also, now that you have the drm-intel-fixes kernel around, please run that with drm.debug=0xe kernel parameter, and attach the dmesg. It produces more relevant debug info than the 3.4 kernels.
Comment 16 Jani Nikula 2012-10-05 14:22:36 UTC
(In reply to comment #15)
> Hmm, back to square one then. Bisecting between 3.4.9 and 3.4.10 is only 4
> steps, if those are the known good and bad versions for you. That would
> probably give us the best clue as to what's going on.

Oh, and the stable kernel branches are in yet another repo:

$ git remote add stable git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
$ git fetch stable
$ git bisect start
$ git bisect good v3.4.9
$ git bisect bad v3.4.10
Comment 17 Jeremy Murphy 2012-10-06 13:20:15 UTC
I got started only to discover that my laptop has suddenly decided that compiling a kernel is too much work and overheats whenever I try.  <sigh>  So progress on this will be slow...
Comment 18 Jani Nikula 2012-10-08 11:39:20 UTC
(In reply to comment #17)
> I got started only to discover that my laptop has suddenly decided that
> compiling a kernel is too much work and overheats whenever I try.  <sigh>  So
> progress on this will be slow...

Did you try with a kernel .config from 'make localmodconfig'? It reduces the burden on your machine considerably.
Comment 19 Daniel Vetter 2012-10-23 11:51:41 UTC
I think this is fixed now, marking as closed. Thanks for reporting this bug, and if it's still a problem for you on latest kernels, please reopen.
Comment 20 Jeremy Murphy 2012-10-23 12:48:34 UTC
That's great.  Sorry, I got my CPU fan fixed (hooray), but I've been swamped with work.  Which branch should the fix be in, drm-intel-fixes?
Comment 21 Daniel Vetter 2012-10-23 13:10:47 UTC
Latest stable 3.6.2 should have the patch. But you can also try -fixes if you want - I'm always looking for guinea-pigs ;-)
Comment 22 Jeremy Murphy 2012-10-23 14:15:29 UTC
I just tried 3.6.2.  :(  I have to go to sleep now, will get on my hamster wheel tomorrow.
Comment 23 Jeremy Murphy 2012-10-24 04:23:34 UTC
Bisect done.  I forgot to fetch the drm-intel branches before I left home this morning and now I'm behind a proxy, so I can't actually check the latest fixes.

c02a29650148f353b0fc86b7eba3f99ecafd23dd is the first bad commit
commit c02a29650148f353b0fc86b7eba3f99ecafd23dd
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Fri Aug 10 11:10:20 2012 +0200

    drm/i915: ignore eDP bpc settings from vbt
    
    commit 4344b813f105a19f793f1fd93ad775b784648b95 upstream.
    
    This has originally been introduced to not oversubscribe the dp links
    in
    
    commit 885a5fb5b120a5c7e0b3baad7b0feb5a89f76c18
    Author: Zhenyu Wang <zhenyuw@linux.intel.com>
    Date:   Tue Jan 12 05:38:31 2010 +0800
    
        drm/i915: fix pixel color depth setting on eDP
    
    Since then we've fixed up the dp link bandwidth calculation code and
    should now automatically fall back to 6bpc dithering. So this is
    unnecessary.
    
    Furthermore it seems to break the new MacbookPro with retina display,
    hence let's just rip this out.
    
    Reported-by: Benoit Gschwind <gschwind@gnu-log.net>
    Cc: Benoit Gschwind <gschwind@gnu-log.net>
    Cc: Francois Rigaut <frigaut@gmail.com>
    Tested-by: Benoit Gschwind <gschwind@gnu-log.net>
    Tested-by: Bernhard Froemel <froemel at vmars tuwien.ac.at>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    
    --
    
    Testing feedback highgly welcome, and thanks for Benoit for finding
    out that the bpc computations are busted.
    -Daniel

:040000 040000 e0713270c16f4f65fc43172b0867640bf8d62ec8 8d700a8eb7baff264377e75e3ae0c76190e33c6e M      drivers
Comment 24 Jani Nikula 2012-10-24 07:05:30 UTC
(In reply to comment #23)
> Bisect done.  I forgot to fetch the drm-intel branches before I left home
> this
> morning and now I'm behind a proxy, so I can't actually check the latest
> fixes.
> 
> c02a29650148f353b0fc86b7eba3f99ecafd23dd is the first bad commit
> commit c02a29650148f353b0fc86b7eba3f99ecafd23dd
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Fri Aug 10 11:10:20 2012 +0200
> 
>     drm/i915: ignore eDP bpc settings from vbt
> 
>     commit 4344b813f105a19f793f1fd93ad775b784648b95 upstream.

Which (obviously stable) branch are you trying this on? I *still* believe this is fixed by

commit 0c96c65b48fba3ffe9822a554cbc0cd610765cd5
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Wed Sep 26 18:43:10 2012 +0300

    drm/i915: use adjusted_mode instead of mode for checking the 6bpc force flag

in upstream, backported to stable 3.6.3 as 6c34ed3be47036c173f7f43df112f93fbd89026f.

Please just verify you have that commit in whichever branch you're testing, and if it still doesn't work, attach dmesg with drm.debug=0xe. Thanks.
Comment 25 Jeremy Murphy 2012-10-25 01:06:24 UTC
Created attachment 84751 [details]
dmesg, 3.7.0_rc2-drm-intel-fixes, w/debugging

The bug is still present in this version.
Comment 26 Jeremy Murphy 2012-10-25 05:38:50 UTC
(In reply to comment #24)
> (In reply to comment #23)
> > Bisect done.  I forgot to fetch the drm-intel branches before I left home
> this
> > morning and now I'm behind a proxy, so I can't actually check the latest
> fixes.
> > 
> > c02a29650148f353b0fc86b7eba3f99ecafd23dd is the first bad commit
> > commit c02a29650148f353b0fc86b7eba3f99ecafd23dd
> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Date:   Fri Aug 10 11:10:20 2012 +0200
> > 
> >     drm/i915: ignore eDP bpc settings from vbt
> > 
> >     commit 4344b813f105a19f793f1fd93ad775b784648b95 upstream.
> 
> Which (obviously stable) branch are you trying this on?

Master.  Would it be different if I used stable/linux-3.4.y for example?
Comment 27 Jeremy Murphy 2012-11-01 12:06:36 UTC
Anything more that I can do to help?
Comment 28 Jani Nikula 2012-11-01 14:26:56 UTC
(In reply to comment #27)
> Anything more that I can do to help?

Please try the "for-jeremy" branch of git://gitorious.org/jani/drm.git repo, and attach the dmesg with drm.debug=0xe module parameter running that. Thanks.
Comment 29 Jani Nikula 2012-11-01 14:38:14 UTC
Please also attach 'hd < /sys/class/drm/*/edid', where * is the connector for eDP.
Comment 30 Jeremy Murphy 2012-11-02 00:39:55 UTC
Created attachment 85391 [details]
dmesg for 3.7.0-rc3-00624-g1ed5996: Works.

Dithering works here.

Sorry, what is that 'hd' command for processing the EDID?  I don't have such a command installed and the only reference I can find is to a command for hard-disc diagnostics, which I assume is not what you have in mind.  (Oh... hex dump?)

This is from X.log, is that equivalent?

[     6.348] (II) intel(0): EDID (in hex):
[     6.348] (II) intel(0):     00ffffffffffff004ca3414b00000000
[     6.348] (II) intel(0):     00130104801a10780a87f594574f8c27
[     6.348] (II) intel(0):     27505400000001010101010101010101
[     6.348] (II) intel(0):     010101010101121b0049502036301030
[     6.348] (II) intel(0):     130005a3100000190c12004950203630
[     6.348] (II) intel(0):     1030130005a310000019000000000000
[     6.348] (II) intel(0):     00000000000000000000000000000002
[     6.348] (II) intel(0):     000c3fd60a3c641a0e236400000000b6
[     6.348] (II) intel(0): EDID vendor "SEC", prod id 19265
Comment 31 Jani Nikula 2012-11-02 08:50:38 UTC
The facts:

* The eDP connection has enough bandwidth for the 8 BPC mode, so 6 BPC is not forced.

* The EDID for the panel has BPC undefined.

* The VBT has 6 BPC. (Concurs with comment #23)

* The display works with 6 BPC but not with 8 BPC.

The obvious conclusion is that we should still use VBT to clamp BPC. However that has been observed to break some Macbooks. We may need to look into using the VBT if it smells good, but not if it smells rotten...
Comment 32 Jani Nikula 2012-11-02 13:04:33 UTC
Additionally, the DPCD does not contain information on BPC (detailed CAP info is not present).
Comment 33 Daniel Vetter 2012-11-09 21:13:07 UTC
Jani, can you please submit the revert for that "ignore VBT for 6bpc edp dither" patch? https://bugs.freedesktop.org/show_bug.cgi?id=56401 is another case, so MacBook Pro's be damned.
Comment 34 Jeremy Murphy 2012-11-10 04:41:47 UTC
Woohoo!  :)

If it is not too much trouble (and assuming the revert goes ahead), could you let us know here which versions of the kernel it goes into?  Thanks!
Comment 35 Daniel Vetter 2012-11-13 12:14:48 UTC
Revert merged to -fixes:

commit 2f4f649a69a9eb51f6e98130e19dd90a260a4145
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Mon Nov 12 14:33:44 2012 +0200

    drm/i915: do not ignore eDP bpc settings from vbt

Please yell if this is still an issue for you.
Comment 36 Jeremy Murphy 2012-11-28 12:49:38 UTC
Judging by the git logs on kernel.org, it looks like this patch will make it into 3.7 but not 3.4 or 3.6 (as of yet).  Do you think it will get back-ported?
Comment 37 Jeremy Murphy 2012-12-10 23:58:14 UTC
Sorry to keep bugging you, but I see other patches for i915 going into the 3.4 and 3.6 branches; is there any reason why this one can't?
Comment 38 Jani Nikula 2012-12-11 07:32:21 UTC
(In reply to comment #37)
> Sorry to keep bugging you, but I see other patches for i915 going into the
> 3.4
> and 3.6 branches; is there any reason why this one can't?

It's missing CC: stable. Risk of breaking things, perhaps. Daniel?

For sure it can't go in without this one too, else the Retina Macbook Pros break:

commit 9a30a61f3516871c5c638fd7c025fbaa11ddf7fe
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Mon Nov 12 14:33:45 2012 +0200

    drm/i915: do not default to 18 bpp for eDP if missing from VBT
Comment 39 Daniel Vetter 2012-12-11 09:56:43 UTC
(In reply to comment #38)
> (In reply to comment #37)
> > Sorry to keep bugging you, but I see other patches for i915 going into the
> 3.4
> > and 3.6 branches; is there any reason why this one can't?
> 
> It's missing CC: stable. Risk of breaking things, perhaps. Daniel?

Yeah, just wanted to wait a bit to check whether this blows up again. I'm writing the mail to the stable team right now. So backports should show up rsn.
Comment 40 Jeremy Murphy 2012-12-11 10:00:31 UTC
Thaaaaanks!  :)

Note You need to log in before you can comment on or make changes to this bug.