Bug 38135

Summary: [G45] testdisplay can't show all modes with HDMI after removing too-early enable_plane (QA)
Product: DRI Reporter: bo.b.wang
Component: DRM/IntelAssignee: Chris Wilson <chris>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: medium CC: jbarnes, keithp
Version: XOrg git   
Hardware: All   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
dmesg info of "comment 6" none

Description bo.b.wang 2011-06-09 22:28:06 UTC
System Environment:
--------------------------
kernel: (drm-intel-next)commit:6a574b5b9b186e28abd3e571dfd1700c5220b510
port: HDMI port
platform: G45

Bug detailed description:
-------------------------
when do "testdisplay -a 10 -s 10", it can't show all listed mode. Exactly some
modes  below 800x600@75  can't be displayed , screen is black. At the same time
when "testdisplay -a 10 -s 10" is done,the screen can't be lighted too.

this issue exists in Linux 3.0-rc1. and it is same to 
Bug 37525 - [G45] testdisplay can't show all modes with DVI interface

Reproduce steps:
----------------
1. HDMI port
2. testdisplay -a 10 -s 10
Comment 1 bo.b.wang 2011-06-13 18:32:35 UTC
It's a regression  
kernel (drm-intel-fixes)dabc8ce34938f9fa2929f51d9fab730e78bd2184 is a good commit
Comment 2 Jesse Barnes 2011-06-15 09:59:18 UTC
I don't see that commit, can you include the full changelog when you bisect down to something as well?  That way if a rebase happens I can still find the commit, and often the changelog is enough to give me an idea of where to look.

What's the first bad commit?
Comment 3 bo.b.wang 2011-06-16 01:19:12 UTC
(In reply to comment #2)
> I don't see that commit, can you include the full changelog when you bisect
> down to something as well?  That way if a rebase happens I can still find the
> commit, and often the changelog is enough to give me an idea of where to look.
> 
> What's the first bad commit?

commit 91e8549bde9e5cc88c5a2e8c8114389279e240b5
Merge: 37fc67c 7eec77a
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu Apr 21 10:50:56 2011 -0700

    Merge branch 'for-linus' of git://git.kernel.dk/linux-2.6-block
    
    * 'for-linus' of git://git.kernel.dk/linux-2.6-block:
      ide: unexport DISK_EVENT_MEDIA_CHANGE for ide-gd and ide-cd
      block: don't propagate unlisted DISK_EVENTs to userland
      elevator: check for ELEVATOR_INSERT_SORT_MERGE in !elvpriv case too

this is a good commit
first bad commit is bisectiong
Comment 4 bo.b.wang 2011-06-17 00:29:45 UTC
49183b2818de6899383bb82bc032f9344d6791ff is the first bad commit

commit 49183b2818de6899383bb82bc032f9344d6791ff
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Apr 19 21:14:14 2011 +0100

    drm/i915: Only enable the plane after setting the fb base (pre-ILK)
    
    When enabling the plane, it is helpful to have already pointed that
    plane to valid memory or else we may incur the wrath of a PGTBL_ER.
    This code preserved the behaviour from the bad old days for unknown
    reasons...
    
    Found by assert_fb_bound_for_plane().
    
    References: https://bugs.freedesktop.org/show_bug.cgi?id=36246
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
    Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
    Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
    Signed-off-by: Keith Packard <keithp@keithp.com>
Comment 5 Chris Wilson 2011-07-08 03:33:40 UTC
That was reverted back to the old behaviour (and causing other people to complain about how their machine suddenly broke, fun).

Downgrading priority since the test machine should now work again, but keeping the bug around as a reminder that we do have machine that breaks due to fixing the enable plane before we attach any memory to that plane...
Comment 6 Chris Wilson 2011-07-29 02:18:47 UTC
Now this is an interesting one to retest with drm-intel-fixes and removing the "surplus" enable_plane().

Can you please checkout drm-intel-fixes, inc.

commit 2704cf5fbd248871a745d210733c6319959d2b0c
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Thu Jul 28 11:52:45 2011 -0700

    drm/i915: flush plane control changes on ILK+ as well
    
    After writing to the plane control reg we need to write to the surface
    reg to trigger the double buffered register latch.  On previous
    chipsets, writing to DSPADDR was enough, but on ILK+ DSPSURF is the reg
    that triggers the double buffer latch.
    
    v2: write DSPADDR too to cover pre-965 chipsets
    v3: use flush_display_plane instead, that's what it's for
    v4: send the right patch
    
    Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
    Tested-by: Keith Packard <keithp@keithp.com>
    Reviewed-by: Keith Packard <keithp@keithp.com>
    Signed-off-by: Keith Packard <keithp@keithp.com>

and then revert this patch:

commit efc2924e733631a64c0afed8dbba2741d186998c
Author: Keith Packard <keithp@keithp.com>
Date:   Mon Jun 6 17:12:49 2011 -0700

    drm/i915: Call intel_enable_plane from i9xx_crtc_mode_set (again)
    
    This change got placed in the ironlake path instead of the 9xx path
    during a recent code shuffle.
    
    Signed-off-by: Keith Packard <keithp@keithp.com>
Comment 7 bo.b.wang 2011-08-14 23:12:26 UTC
(In reply to comment #6)
Hello Chris, I have tried your method, but the issue still exists.
Comment 8 bo.b.wang 2011-08-14 23:16:00 UTC
Created attachment 50215 [details]
 dmesg info of "comment 6"
Comment 9 Chris Wilson 2012-09-05 12:46:07 UTC
Presuming fixed in the interim with all the modesetting/edid rework

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.