D27980: make opengl module support multiple dri devices

This revision was automatically updated to reflect the committed changes. Closed by commit R102:72a09a3f2787: make opengl module support multiple dri devices (authored by sitter). REPOSITORY R102 KInfoCenter CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27980?vs=79208&id=79216 REVI

D27980: make opengl module support multiple dri devices

apol accepted this revision. apol added a comment. This revision is now accepted and ready to land. LGTM REPOSITORY R102 KInfoCenter BRANCH multidri REVISION DETAIL https://phabricator.kde.org/D27980 To: sitter, apol Cc: pino, zzag, apol, adridg, plasma-devel, Orage, LeGast00n, The-Fe

D27980: make opengl module support multiple dri devices

sitter updated this revision to Diff 79208. sitter added a comment. - do however init the stat struct, its members are not initialized otherwise - streamline get_dri_device return - do not init l3 to nullptr inside the info loop. l3 is the entry for the infos, so simply leave it as-is (nul

D27980: make opengl module support multiple dri devices

sitter added inline comments. INLINE COMMENTS > apol wrote in opengl.cpp:625 > setting twice in a row? I'd skip setting a nullptr. Mh. I think that's to ensure l3 (which is passed into newItem) is null on >1 iteration. Indeed garbage though, I'll just pass in nullptr instead. REPOSITORY R102

D27980: make opengl module support multiple dri devices

apol added inline comments. INLINE COMMENTS > opengl.cpp:281 > +if (ReadPipe("sysctl -n hw.dri.0.name", pci_info)) { > +DriInfo info{}; > +info.module = pci_info[0].mid(0, pci_info[0].indexOf(0x20)); the {} isn't necessary here. > opengl.cpp:290 > > -static bool get_dri_de

D27980: make opengl module support multiple dri devices

sitter added inline comments. INLINE COMMENTS > apol wrote in opengl.cpp:281 > Go for it. Already is in latest diff ;) REPOSITORY R102 KInfoCenter REVISION DETAIL https://phabricator.kde.org/D27980 To: sitter Cc: pino, zzag, apol, adridg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev,

D27980: make opengl module support multiple dri devices

apol added inline comments. INLINE COMMENTS > sitter wrote in opengl.cpp:281 > As it turns out the /proc/dri stuff was supposedly removed here > https://github.com/torvalds/linux/commit/cb6458f97b53d7f73043206c18014b3ca63ac345 > making this code entirely irrelevant since kernel 3.12. > Any obje

D27980: make opengl module support multiple dri devices

adridg added inline comments. INLINE COMMENTS > apol wrote in opengl.cpp:335 > Probably want to list them all on BSD too, right @adridg? Possibly. I don't have any machines with multiple cards, so I can't tell -- I'd have to ask Niclas about this. I don't know if it's possible for the numbering

D27980: make opengl module support multiple dri devices

sitter updated this revision to Diff 78003. sitter added a comment. - isValid is const now - use qdiriterator - use QFile::encodeName for the stat call REPOSITORY R102 KInfoCenter CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27980?vs=77716&id=78003 BRANCH multidri REVISI

D27980: make opengl module support multiple dri devices

pino added inline comments. INLINE COMMENTS > sitter wrote in opengl.cpp:210 > Is that also true for paths that are only ascii? Specifically path is a /dev > node. sure, i get this specific path is ascii the general recommendation is to use QFile::encodeName for QString -> C paths conversions

D27980: make opengl module support multiple dri devices

sitter added inline comments. INLINE COMMENTS > pino wrote in opengl.cpp:210 > qPrintable is wrong when passing paths to native C functions; use > QFile::encodeName instead Is that also true for paths that are only ascii? Specifically path is a /dev node. > pino wrote in opengl.cpp:230-233 >

D27980: make opengl module support multiple dri devices

pino added inline comments. INLINE COMMENTS > opengl.cpp:169 > + > +bool isValid() > +{ const > opengl.cpp:210 > struct stat fileInfo; > -if (::stat("/dev/dri/card0", &fileInfo) != 0) > -return false; > -if ((fileInfo.st_mode & S_IFCHR) != S_IFCHR) > -return

D27980: make opengl module support multiple dri devices

sitter updated this revision to Diff 77716. sitter added a comment. use qvector instead of list fix device detection by doing readlink resolution on the correct directory (subdir appending got lost in the code rejiggering) /proc/dri support is now gone entirely as it is only relevant

D27980: make opengl module support multiple dri devices

zzag added a comment. In D27980#626500 , @sitter wrote: > Had a quick look, using libdrm seems a bit excessive for what we want to do here. Well, it was worth a try. REPOSITORY R102 KInfoCenter REVISION DETAIL https://phabricator.kd

D27980: make opengl module support multiple dri devices

sitter added a comment. In D27980#625840 , @zzag wrote: > I'm not sure whether this is useful, but libdrm has an API to enumerate devices. Had a quick look, using libdrm seems a bit excessive for what we want to do here. The current /sys

D27980: make opengl module support multiple dri devices

sitter added inline comments. INLINE COMMENTS > apol wrote in opengl.cpp:281 > Probably only want to list files? Dirs actually, e.g. /proc/dri/0/name I honestly do not know what they are so I figured best to just list everything. They could be symlinks for all I know. /proc/dri/ isn't a thing o

D27980: make opengl module support multiple dri devices

zzag added a comment. I'm not sure whether this is useful, but libdrm has an API to enumerate devices. REPOSITORY R102 KInfoCenter REVISION DETAIL https://phabricator.kde.org/D27980 To: sitter Cc: zzag, apol, adridg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, z

D27980: make opengl module support multiple dri devices

apol added subscribers: adridg, apol. apol added a comment. +1 overall INLINE COMMENTS > opengl.cpp:278 > + > +static QList get_dri_devices_proc() > { I'd make it QVector, less to worry in the future. > opengl.cpp:281 > +QList list; > +const auto entries = QDir(QStringLiteral("/pro

D27980: make opengl module support multiple dri devices

sitter created this revision. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. sitter requested review of this revision. REVISION SUMMARY previously the code would only ever look at dri0 when that may not actually be the device that is used. instead list all possible de