Bug 270942

Summary: Direct Rendering is wrongfully disabled on Intel graphics since mesa 7.10.1
Product: [Plasma] kwin Reporter: Antonis K <antkoul>
Component: compositingAssignee: KWin default assignee <kwin-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: cfeck, fredrik, jpsinthemix, kevin.kofler, seastland, stevekej
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Arch Linux   
OS: Linux   
Latest Commit: Version Fixed In: 4.7.0
Attachments: kde-intel-dri-fix.patch

Description Antonis K 2011-04-14 13:31:02 UTC
Version:           unspecified (using KDE 4.6.2) 
OS:                Linux

It seems that kwin opengl test is looking for "GEM" on the OpenGL renderer string to enable direct rendering on Intel graphics hardware. (http://quickgit.kde.org/?p=kde-workspace.git&a=blob&f=kwin/opengltest/opengltest.cpp) (lines 89-90).

Since mesa 7.10.1 though, the "GEM" string was removed (http://cgit.freedesktop.org/mesa/mesa/commit/?h=7.10&id=b0a7492aebeb4517346f0da2362d6991a7385b59) making kwin unable to identify Intel graphics correctly.

I'm copying part of my .xsession-errors a) with pre-7.10.1 mesa and b) with mesa 7.10.1

a)
OpenGL vendor string:                   Tungsten Graphics, Inc
OpenGL renderer string:                 Mesa DRI Intel(R) G41 GEM 20100330 DEVELOPMENT 
OpenGL version string:                  2.1 Mesa 7.10.1-devel
OpenGL shading language version string: 1.20
Driver:                                 Intel
GPU class:                              i965
OpenGL version:                         2.1
GLSL version:                           1.20
Mesa version:                           7.10.1
X server version:                       1.9.4
Linux kernel version:                   2.6.37
Direct rendering:                       yes
Requires strict binding:                yes
GLSL shaders:                           yes
Texture NPOT support:                   yes

b)
OpenGL vendor string:                   Tungsten Graphics, Inc
OpenGL renderer string:                 Mesa DRI Intel(R) G41  
OpenGL version string:                  1.4 (2.1 Mesa 7.10.1)
Driver:                                 Intel
GPU class:                              i965
OpenGL version:                         1.4
Mesa version:                           7.10.1
X server version:                       1.9.4
Linux kernel version:                   2.6.37
Direct rendering:                       no
Requires strict binding:                yes
GLSL shaders:                           no
Texture NPOT support:                   yes

This is the case also with every version since 7.10.1 and with 7.11 git branch.

The obvious solution is to stop the tester from looking for "GEM" and replace this with "Intel" or something.

Related forum topic and bug report:
https://bbs.archlinux.org/viewtopic.php?pid=904085
https://bugs.launchpad.net/ubuntu/+source/xserver-xorg-video-intel/+bug/753370


Reproducible: Always
Comment 1 Martin Flöser 2011-04-15 06:41:44 UTC
If you want to have it fixed for natty, please contact Kubuntu devs asap. Please reference me and also this bugreport.

Any fix in Kwin is too late for Natty as KDEs next release is after the release of Natty. Furthermore the change is too dangerous in a minor release. I will rework the detection code for 4.7 but have no time at all before Natty.

The only way to fix it right now is to revert the change in the driver which is completly unacceptable for a minor release.
Comment 2 Antonis K 2011-04-15 10:41:04 UTC
Created attachment 58999 [details]
kde-intel-dri-fix.patch
Comment 3 Antonis K 2011-04-15 10:41:39 UTC
By reverting the change in the driver, do you mean an upstream change or by patching on distribution packages? I don't think that the first would be accepted.

By the way, I created a patch that applies on 7.10.* (tested on 7.10.2) that reverts the change and fixes the issue.
On 7.11-git a lot more have changed (http://cgit.freedesktop.org/mesa/mesa/commit/?id=0fe34b7bbc9a8e089bbb4d0fe401b09095a571eb) and must be reverted to fix this, as the patch alone causes the build to fail.
Comment 4 Kevin Kofler 2011-04-19 19:09:37 UTC
FYI, 7.11-git is what Fedora 15 is shipping.

I'd personally rather see KWin fixed to use the documented APIs (the DRI2QueryVersion X protocol request) to check for DRI2 instead of keeping patching that hack.
Comment 5 Sergei Andreev 2011-04-19 19:10:43 UTC
*** This bug has been confirmed by popular vote. ***
Comment 6 Kevin Kofler 2011-04-19 22:17:18 UTC
Proposed patch: https://git.reviewboard.kde.org/r/101157/
Comment 7 Christoph Feck 2011-04-19 23:38:20 UTC
Nice to see how it went from OpenGL 2.1 back to 1.4...
Comment 8 Thomas Lübking 2011-04-20 17:33:55 UTC
*** Bug 271140 has been marked as a duplicate of this bug. ***
Comment 9 Kevin Kofler 2011-04-20 19:24:47 UTC
What Adam Jackson, the main X11 maintainer in Fedora, has to say about this issue:

[BEGIN QUOTE]

On 4/19/11 7:23 PM, Kevin Kofler wrote:

> DRI1 drivers do not work properly with direct rendering in KWin and require
> indirect rendering (LIBGL_ALWAYS_INDIRECT). But needless use of indirect
> rendering disables some effects (notably the Blur effect), when it works at all.

Pretty sure gnome-shell handles this by checking to see whether 
GLX_EXT_texture_from_pixmap is actually available in direct contexts, 
ie, look in glXQueryExtensionsString().  I don't have any DRI1 hardware 
handy to test this - because, ew - but checking against Xvfb (which is 
sorta like DRI1) with LIBGL_ALWAYS_INDIRECT both on and off does the 
right thing: indirect contexts support EXT_tfp, direct contexts do not.

Of course, the even better answer is to just not support DRI1 drivers, 
but F16 will solve that problem for you.

- ajax

[END QUOTE]

and:

[BEGIN QUOTE]

On 4/20/11 11:59 AM, Kevin Kofler wrote:
> On Wednesday 20 April 2011, Adam Jackson wrote:
>> On 4/20/11 11:51 AM, Kevin Kofler wrote:
>>> Checking whether GLX_EXT_texture_from_pixmap is actually available
>>> through glXQueryExtensionsString() is one of the checks they do. But
>>> they have found that not to be sufficient.
>>
>> Tell me why.
>
> Hmmm, I think the problem is that they assume that everything which supports
> OpenGL>= 1.4 supports the texture from pixmap API because it's supposedly
> part of 1.4.

That's completely broken.  The GLX spec doesn't mention 
EXT_texture_from_pixmap at all:

http://www.opengl.org/documentation/specs/glx/glx1.4.pdf

---
6.4     Version 1.4
The command glXGetProcAddress was added in GLX Version 1.4, promoted 
from the GLX_ARB_get_proc_address extension.
The GLX_SAMPLE_BUFFERS and GLX_SAMPLES attributes were added, enabling 
implementations to export GLXFBConfigs and Visuals with multisample 
capability. These attributes were promoted from the GLX_ARB_multisample 
extension.
---

That's the entirety of the 1.4 additions to GLX.

This comment from the source you linked to

     // glXCreatePixmap() is a GLX 1.3+ function, but it's also provided 
by EXT_texture_from_pixmap

is also false.  The extension spec:

http://www.opengl.org/registry/specs/EXT/texture_from_pixmap.txt

mentions nothing of the sort; rather it says that GLX 1.3 or newer is 
required.  DRI1 drivers don't fully support GLX 1.3 - pbuffers in 
particular - but may advertise EXT_texture_from_pixmap anyway, because 
they do support fbconfigs, which is all that the tfp spec actually 
depends on.  A gross hack, to be sure.

So: check for GLX 1.3 and EXT_tfp in direct context, then in indirect. 
If you still get nothing, check for GLX_SGIX_fbconfig and EXT_tfp in 
direct context, then in indirect.  Technically in the latter case you 
should use glXCreateGLXPixmapWithConfigSGIX() rather than 
glXCreatePixmap(), but Mesa and Xorg simply treat them as two wire 
encodings of the same operation.  I suspect NVIDIA and ATI's drivers do 
as well, but I suspect you'd never get there because all versions of 
their drivers that support EXT_tfp also support GLX 1.3.

- ajax

[END QUOTE]

So, in short, Adam Jackson recommends removing all the specific driver checks (i.e. the whole "// Assume that direct rendering works with DRI2 drivers" block) and changing:
if ((major == 1 && minor < 3) && !strstr(glxExtensions, "GLX_EXT_texture_from_pixmap"))
to just:
if (!strstr(glxExtensions, "GLX_EXT_texture_from_pixmap"))
Comment 10 Kevin Kofler 2011-04-21 04:58:00 UTC
Actually, reading his comment again, I think the check should be changed to:
if ((major == 1 && minor < 3) || !strstr(glxExtensions,
"GLX_EXT_texture_from_pixmap"))
i.e. change the && to ||, so fail if EITHER OpenGL is older than 1.3 OR TFP is not supported, in other words, require BOTH OpenGL 1.3 AND TFP.

Checking for GLX_SGIX_fbconfig as an alternative to OpenGL 1.3 shouldn't be necessary in our context because we are only checking whether direct rendering is supported and we can't use direct rendering on DRI1 anyway.
Comment 11 Martin Flöser 2011-04-21 13:46:11 UTC
 when I am back home I will have a look at how this looks like with 
 fglrx blob. My current assumption is that changing the && to || will 
 break the Ati blob as it is just too slow with direct rendering enabled 
 (but works).

 Which brings us back to in an ideal world we don't need hacks, but the 
 reality looks different. I think you can understand more and more why I 
 am opposed to allow such a change in 4.6 ;-)
Comment 12 Fredrik Höglund 2011-04-21 13:53:58 UTC
(In reply to comment #10)
> Actually, reading his comment again, I think the check should be changed to:
> if ((major == 1 && minor < 3) || !strstr(glxExtensions,
> "GLX_EXT_texture_from_pixmap"))
> i.e. change the && to ||, so fail if EITHER OpenGL is older than 1.3 OR TFP is
> not supported, in other words, require BOTH OpenGL 1.3 AND TFP.

Kevin, I'm sure your version is what was intended, and what is there now is the result of a thinko. The EXT_tfp extension is obviously required regardless of the GLX version.

As for the driver specific tests, we used to only check if EXT_tfp was supported, but IIRC there were complaints from FGLRX users that direct rendering made compositing unusably slow for them.

This is why the renderer and vendor checks were added. The intent isn't so much to determine if the driver is using DRI2 as it is to make sure we only enable direct rendering with drivers where we know it works.

Checking for "DRI2" || "GEM" || "Gallium" covered all the Mesa drivers, and also provided an additional (but hopefully not needed) sanity check that the driver is in fact using DRI2.

I'd love to just say that DRI1 isn't supported anymore and be done with it. But my impression is that we still have many users who want to use KWin with FGLRX, so I don't think we can do that just yet.

The driver tests could probably be simplified to just check if the version string contains "Mesa" though.
Comment 13 Kevin Kofler 2011-04-21 14:31:43 UTC
Shouldn't we explicitly blacklist FGLRX rather than whitelisting everything else then?
Comment 14 Martin Flöser 2011-04-21 15:05:22 UTC
> --- Comment #13 from Kevin Kofler <kevin kofler chello at>
> 2011-04-21 14:31:43 ---
> Shouldn't we explicitly blacklist FGLRX rather than whitelisting 
> everything
> else then?
 I think whitelisting is the better approach as there are more drivers 
 which do not support compositing in an expected manner. E.g. the 
 "Chromium" driver of VirtualBox which will give you a black screen when 
 using direct rendering (yes it also has GLX 1.3 and the TFP extension) 
 or recently we had bugreports about VMWare drivers.

 Assuming that all drivers work except fglrx sounds too dangerous to me. 
 Also trying to find all drivers which do not work is more difficult than 
 finding the drivers which work. I rather add a fix to make something 
 work again than fixing complete breakage.
Comment 15 Kevin Kofler 2011-04-21 15:08:30 UTC
That assumes that indirect rendering is always safe. Unfortunately, that's not the case: on my i965 notebook with Fedora 14 (Mesa 7.9), KWin OpenGL effects just crash if the "use direct rendering" checkbox is not checked.
Comment 16 Kevin Kofler 2011-04-21 16:51:50 UTC
So, since it is still not clear what the best long-term solution is, I had another look at http://sarvatt.com/downloads/patches/kdebase-workspace.patch as a quick fix.

AFAICT, the DRI1 i810 driver does not have “Intel” in its rendering string, but “i810″ or “i815″ (with varying prefixes and suffixes, but nothing with “Intel” in it), see:
http://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/drivers/dri/i810/i810context.c

So any current Intel driver shipped in any current GNU/Linux distribution should be the DRI2 version. In other words, the patch should be perfectly safe to ship in a current GNU/Linux distribution.
Comment 17 Martin Flöser 2011-04-21 23:37:20 UTC
> So any current Intel driver shipped in any current GNU/Linux 
> distribution
> should be the DRI2 version. In other words, the patch should be
> perfectly safe
> to ship in a current GNU/Linux distribution.
 I will have this one checked with Debian testing which is AFAIK still 
 at Mesa 7.7 but provides packages for 4.6 (through an external 
 repository). I think there we cannot use direct rendering. But I will 
 let it be checked on Saturday.

 Nevertheless it should be safe for 4.7 and I think we should mention in 
 the release note that KDE Plasma requires at least Mesa 7.11 (in case it 
 is released at that time).
Comment 18 Martin Flöser 2011-04-23 12:50:46 UTC
>  I will have this one checked with Debian testing which is AFAIK still 
>  at Mesa 7.7 but provides packages for 4.6 (through an external 
>  repository). I think there we cannot use direct rendering. But I will 
>  let it be checked on Saturday.
Just doing my updates on testing and Mesa 7.10 just get's installed (which means I can finally 
give the free drivers a try again \o/).

Concerning fglrx though I am right: it supports glx 1.4, but we may not use it with direct 
rendering.
Comment 19 Thomas Lübking 2011-04-23 13:19:36 UTC
me tests https://git.reviewboard.kde.org/r/101157/#review2785 against prop nvidia now
Comment 20 Kevin Kofler 2011-04-23 13:42:26 UTC
I can't see your comment, looks like you forgot to publish it.
Comment 21 Thomas Lübking 2011-04-23 13:46:10 UTC
still compiling, you'll read when i've some insight ;-)
Comment 22 Martin Flöser 2011-05-01 11:41:31 UTC
Git commit afe966579d56f85444c432a3a6adf026d2941bd5 by Martin Gräßlin.
Committed on 01/05/2011 at 11:49.
Pushed by graesslin into branch 'master'.

Enable direct rendering for all Mesa drivers

As KWin requires at least Mesa 7.10 for OpenGL we can be sure
that all Mesa drivers support DRI2 and can enable direct rendering
for it.

Parsing the version string to identify if it is a mesa driver.
This can cause breakage again in future versions of Mesa.
Unfortunately version, vendor and renderer are the only information
queryable just with OpenGL API.
BUG: 270942
FIXED-IN: 4.7.0

M  +3    -10   kwin/opengltest/opengltest.cpp     

http://commits.kde.org/kde-workspace/afe966579d56f85444c432a3a6adf026d2941bd5