Bug 77767 - [regression] intel framebuffer broken by d978ef14456a38034f6c0e94a794129501f89200 in kernel 3.15-rc
Summary: [regression] intel framebuffer broken by d978ef14456a38034f6c0e94a794129501f8...
Status: CLOSED FIXED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: unspecified
Hardware: x86 (IA32) Linux (All)
: high blocker
Assignee: Chris Wilson
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-22 14:26 UTC by Knut Petersen
Modified: 2017-07-24 22:54 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
dmesg of boot to framebuffer console, drm debug level 0xe (105.69 KB, text/plain)
2014-04-22 14:30 UTC, Knut Petersen
no flags Details

Description Knut Petersen 2014-04-22 14:26:19 UTC

    
Comment 1 Knut Petersen 2014-04-22 14:30:06 UTC
Created attachment 97752 [details]
dmesg of boot to framebuffer console, drm debug level 0xe
Comment 2 Chris Wilson 2014-04-22 15:02:54 UTC
What does xrandr say? I'm curious as to what configuration we end up thinking everything is setup at. Perhaps also compare what happens with xrandr for 3.14. (Presuming everything remains insane during X start, if not, use SNA.)
Comment 3 Knut Petersen 2014-04-22 15:12:41 UTC
(In reply to comment #2)
> What does xrandr say? I'm curious as to what configuration we end up
> thinking everything is setup at. Perhaps also compare what happens with
> xrandr for 3.14. (Presuming everything remains insane during X start, if
> not, use SNA.)

Hmm. X always gets things right, it's the pure linux framebuffer console that is broken ....
Comment 4 Chris Wilson 2014-04-22 15:27:28 UTC
Well this is nonsense:

[    3.236667] [drm:drm_setup_crtcs], desired mode 1280x1024 set on crtc 3
[    3.236676] [drm:intelfb_create], re-using BIOS fb
[    3.236892] [drm:intelfb_create], allocated 720x400 fb: 0x00000000, bo f6373e40
Comment 5 Knut Petersen 2014-04-22 15:30:20 UTC
There is one difference in the Xorg logs:

xserver with kernel 3.14.1  finds some more outputs:


[    32.462] (II) intel: Driver for Intel(R) Integrated Graphics Chipsets:
        i810, i810-dc100, i810e, i815, i830M, 845G, 854, 852GM/855GM, 865G,
        915G, E7221 (i915), 915GM, 945G, 945GM, 945GME, Pineview GM,
        Pineview G, 965G, G35, 965Q, 946GZ, 965GM, 965GME/GLE, G33, Q35, Q33,
        GM45, 4 Series, G45/G43, Q45/Q43, G41, B43
[    32.463] (II) intel: Driver for Intel(R) HD Graphics: 2000-5000
[    32.463] (II) intel: Driver for Intel(R) Iris(TM) Graphics: 5100
[    32.463] (II) intel: Driver for Intel(R) Iris(TM) Pro Graphics: 5200
[    32.464] (--) using VT number 2

[    32.502] (II) intel(0): SNA compiled from 2.99.911-80-gfd05790
[    32.508] (--) intel(0): Integrated Graphics Chipset: Intel(R) 915GM
[    32.508] (--) intel(0): CPU: x86, sse2
[    32.508] (II) intel(0): Creating default Display subsection in Screen section
        "Default Screen Section" for depth/fbbpp 24/32
[    32.508] (==) intel(0): Depth 24, (--) framebuffer bpp 32
[    32.508] (==) intel(0): RGB weight 888
[    32.508] (==) intel(0): Default visual is TrueColor
[    32.509] (**) intel(0): Framebuffer tiled
[    32.509] (**) intel(0): Pixmaps tiled
[    32.509] (**) intel(0): "Tear free" disabled
[    32.509] (**) intel(0): Forcing per-crtc-pixmaps? no
[    32.509] (--) intel(0): Using a maximum size of 64x64 for hardware cursors
[    32.509] (II) intel(0): Output VGA1 has no monitor section
[    32.509] (II) intel(0): Output DVI1 has no monitor section
[    32.509] (II) intel(0): Output TV1 has no monitor section
[    32.509] (II) intel(0): Output TV2 has no monitor section
[    32.510] (II) intel(0): Output TV3 has no monitor section
[    32.510] (II) intel(0): Output VGA2 has no monitor section
[    32.510] (II) intel(0): Output TV4 has no monitor section
[    32.510] (II) intel(0): Output VIRTUAL1 has no monitor section
[    32.510] (--) intel(0): Output DVI1 using initial mode 1280x1024 on pipe 0
[    32.510] (==) intel(0): DPI set to (96, 96)
[    32.510] (II) Loading sub module "dri2"
[    32.510] (II) LoadModule: "dri2"


This is relevant part of the first broken kernel version:

[    38.203] (II) intel(0): Output VGA1 has no monitor section
[    38.203] (II) intel(0): Output DVI1 has no monitor section
[    38.206] (II) intel(0): Output TV1 has no monitor section
[    38.206] (II) intel(0): Output VIRTUAL1 has no monitor section
[    38.206] (--) intel(0): Output DVI1 using initial mode 1280x1024 on pipe 0


xrandr with that kernel:

knut@golem:~> xrandr
Screen 0: minimum 8 x 8, current 1280 x 1024, maximum 32767 x 32767
VGA1 disconnected (normal left inverted right x axis y axis)
DVI1 connected primary 1280x1024+0+0 (normal left inverted right x axis y axis) 337mm x 270mm
   1280x1024     60.02*+
   1024x768      60.00  
   800x600       60.32  
   640x480       60.00  
   720x400       70.08  
TV1 disconnected (normal left inverted right x axis y axis)
VIRTUAL1 disconnected (normal left inverted right x axis y axis)
knut@golem:~> 


The video mode is always correctly set to 1280x1024, it's only used incorrectly...

Switching between X and fb console does not cure anything
Comment 6 Knut Petersen 2014-04-22 15:32:58 UTC
(In reply to comment #4)
> Well this is nonsense:
> 
> [    3.236667] [drm:drm_setup_crtcs], desired mode 1280x1024 set on crtc 3
> [    3.236676] [drm:intelfb_create], re-using BIOS fb
> [    3.236892] [drm:intelfb_create], allocated 720x400 fb: 0x00000000, bo
> f6373e40

Yep, I said it's broken ;-)
Comment 7 Chris Wilson 2014-04-22 15:33:25 UTC
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index b16116db6c37..24463b643fc8 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -133,6 +133,12 @@ static int intelfb_create(struct drm_fb_helper *helper,
 
        mutex_lock(&dev->struct_mutex);
 
+       if (intel_fb &&
+           sizes->fb_width > intel_fb->base.width ||
+           sizes->fb_height > intel_fb->base.height) {
+               drm_framebuffer_reference(&ifbdev->fb->base);
+               intel_fb = NULL;
+       }
        if (!intel_fb || WARN_ON(!intel_fb->obj)) {
                DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n");
                ret = intelfb_alloc(helper, sizes);
Comment 8 Knut Petersen 2014-04-22 15:55:24 UTC
Yes, that cures the problem. I'd suggest to put the expression after "&&" into parentheses, otherwise the compiler complains ...

Thanks!

cu,
 Knut


(In reply to comment #7)
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> b/drivers/gpu/drm/i915/intel_fbdev.c
> index b16116db6c37..24463b643fc8 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -133,6 +133,12 @@ static int intelfb_create(struct drm_fb_helper *helper,
>  
>         mutex_lock(&dev->struct_mutex);
>  
> +       if (intel_fb &&
> +           sizes->fb_width > intel_fb->base.width ||
> +           sizes->fb_height > intel_fb->base.height) {
> +               drm_framebuffer_reference(&ifbdev->fb->base);
> +               intel_fb = NULL;
> +       }
>         if (!intel_fb || WARN_ON(!intel_fb->obj)) {
>                 DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n");
>                 ret = intelfb_alloc(helper, sizes);

(In reply to comment #7)
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> b/drivers/gpu/drm/i915/intel_fbdev.c
> index b16116db6c37..24463b643fc8 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -133,6 +133,12 @@ static int intelfb_create(struct drm_fb_helper *helper,
>  
>         mutex_lock(&dev->struct_mutex);
>  
> +       if (intel_fb &&
> +           sizes->fb_width > intel_fb->base.width ||
> +           sizes->fb_height > intel_fb->base.height) {
> +               drm_framebuffer_reference(&ifbdev->fb->base);
> +               intel_fb = NULL;
> +       }
>         if (!intel_fb || WARN_ON(!intel_fb->obj)) {
>                 DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n");
>                 ret = intelfb_alloc(helper, sizes);
Comment 9 Daniel Vetter 2014-04-22 17:32:53 UTC
(In reply to comment #8)
> Yes, that cures the problem. I'd suggest to put the expression after "&&"
> into parentheses, otherwise the compiler complains ...

That's actually required since && binds with higher prio than ||. Chris, can you please submit this to intel-gfx?
Comment 10 Chris Wilson 2014-04-23 13:28:45 UTC
commit 58b87a391d67e35e3aa1fc6c80890b491d73ad46
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Apr 23 08:54:31 2014 +0100

    drm/i915: Discard BIOS framebuffers too small to accommodate chosen mode
    
    If the inherited BIOS framebuffer is smaller than the mode selected for
    fbdev, then if we continue to use it then we cause display corruption as
    we do not setup the panel fitter to upscale.
    
    Regression from commit d978ef14456a38034f6c0e94a794129501f89200
    Author: Jesse Barnes <jbarnes@virtuousgeek.org>
    Date:   Fri Mar 7 08:57:51 2014 -0800
    
        drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v12
    
    v2: Add a debug message to track the discard of the BIOS fb.
    v3: Ville pointed out the difference between ref/unref
    
    Reported-by: Knut Petersen <Knut_Petersen@t-online.de>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77767
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
    Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
    Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Comment 11 Daniel Vetter 2014-05-19 16:16:12 UTC
Mark as regression for stats.


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.