kcm_opengl: Fix retrieval of DRI information
ClosedPublic

Authored by roberts on Apr 23 2018, 7:08 PM.

Details

Summary

Currently DRI information retrieval is attempted via /proc/dri, which
was removed from the linux kernel in v3.12 (2013). This information
includes the kernel driver in use as well as card identification sourced
from pci.ids database.

This adds support for retrieving this information in the same vein via
/dev and /sys for modern kernels.

Test Plan

kcmshell5 opengl should correctly display kernel driver.

Diff Detail

Repository
R102 KInfoCenter
Branch
kcm_opengl_fix_dri_info
Lint
No Linters Available
Unit
No Unit Test Coverage
roberts created this revision.Apr 23 2018, 7:08 PM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 23 2018, 7:08 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
roberts requested review of this revision.Apr 23 2018, 7:08 PM
wbauer added a subscriber: wbauer.Apr 24 2018, 10:13 AM

I gave the patch a try (openSUSE Leap 42.3 with kernel 4.4).

The 3D accelerator info is displayed again now (as with kernel < 3.12), but it shows "drm" as kernel module instead of "radeon" as I would expect.

Modules/opengl/opengl.cpp
211

Using just "/driver" (instead of "/driver/module") makes it display "radeon" instead of "drm" here as I would expect.

OTOH, as it already runs lspci to get the other information, wouldn't it be better to get the kernel module from lspci as well?
Additionally specifying the '-k' parameter to lspci adds these two lines to lspci's output here:
Driver: radeon
Module: radeon
(the first one is the driver in use, the second one lists all available modules for that device, see also "man lspci")
That should be independent of the kernel version and also work the same in the get_dri_device_proc() case.

Just a thought though.

roberts updated this revision to Diff 32999.Apr 24 2018, 5:10 PM

Update to use /driver over /driver/module per review.

roberts marked an inline comment as done.Apr 24 2018, 5:15 PM
roberts added inline comments.
Modules/opengl/opengl.cpp
211

Have changed to use /driver instead of /driver/module.

Am preferring to err on the side of not adding more numbered line reads from lspci output.

I've tested this and it seems to work!

But in wayland I had to add some changes: set IsDirect = true; before calling print_screen_info() in get_gl_info_egl_qt() line ~ 929.
Otherwise, get_dri_device() is never called in this block in print_screen_info():

if (IsDirect) {
    if (get_dri_device())  {
        l2 = newItem(l1, i18n("3D Accelerator"));
        l2->setExpanded(true);
        l3 = newItem(l2, l3, i18n("Vendor"), dri_info.vendor);
        l3 = newItem(l2, l3, i18n("Device"), dri_info.device);
        l3 = newItem(l2, l3, i18n("Subvendor"), dri_info.subvendor);
        l3 = newItem(l2, l3, i18n("Revision"), dri_info.rev);
    } else {
        l2 = newItem(l1, l2, i18n("3D Accelerator"), i18n("unknown"));
    }
}

For X11, IsDirect is initialized in get_gl_info_glx() line 767 by:

IsDirect = glXIsDirect(dpy, ctx);

Fow wayland path, it stays false forever, I guess. Though direct rendering IS used.

Without this hack I only get this:

3D Accelerator section is gone, and Driver section lacks kernel module information. :(

roberts marked an inline comment as done.May 3 2018, 5:32 PM

Alexey: That sounds great, but unfortunately I can't get a wayland session to log in no matter how hard I try, so I can't confirm your changes. I'd suggest creating a separate review with your additions if you want to pursue this.

Can I ask for a reviewer for this under X11?

roberts updated this revision to Diff 33565.May 3 2018, 6:32 PM

Update to set IsDirect under Wayland.

Managed to run kwin_wayland embedded in an X11 session to confirm.

I'm not completely sure that IsDirect should be manually set to true in wayland code path (it looks like a hack just to test).
What happens if IsDirect is set to true, but software rendering is active? Will get_dri_device() fail gracefully or something horrible will happen? :)

It will fail gracefully, it checks for the presence of these /proc or /sys files and only acts if they exist. This code is only called if an EGL context is successfully created in any case, and in fact on Wayland rendering _is_ always direct.

alexeymin accepted this revision.May 5 2018, 11:39 AM

It will fail gracefully, it checks for the presence of these /proc or /sys files and only acts if they exist. This code is only called if an EGL context is successfully created in any case, and in fact on Wayland rendering _is_ always direct.

Yeah, after reading about wayland more, I think so, it always uses DRM to render.

Can I ask for a reviewer for this under X11?

Seems to work for me in X11, too

About code, I don't like all those single-line ifs whout braces around, and KDE coding style says:
Use curly braces even when the body of a conditional statement contains only one line.
But I guess the existing code already has this problem.
I'd say +1

This revision is now accepted and ready to land.May 5 2018, 11:39 AM
This revision was automatically updated to reflect the committed changes.