This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Patch to scan /dev/dri based on names rather than file type (kmsdrm.diff, text/plain, 2019-05-07 15:32:33 +0000, 696 bytes)

Reported in version: 2.0.9
Reported for operating system, platform: FreeBSD, All

Comments on the original bug report:

On 2019-05-07 15:32:33 +0000, Jan Martin Mikkelsen wrote:

Created attachment 3776
Patch to scan /dev/dri based on names rather than file type

Loading KMS/DRM on FreeBSD fails because the "available" code in the driver checks for character device nodes under /dev/dri and the /dev/dri/card* files are symlinks rather than device nodes nodes on FreeBSD. The symlink points to /dev/drm/0.

The attached patch counts /dev/dri/card* entries rather than directory entries which are character devices.

On 2019-06-18 14:16:31 +0000, Jan Martin Mikkelsen wrote:

Is any additional information necessary for this patch?

On 2019-06-18 15:00:02 +0000, Sam Lantinga wrote:

This is fixed for SDL 2.0.10, thanks!

*** This bug has been marked as a duplicate of bug 4241 ***

On 2019-06-18 15:17:54 +0000, Jan Martin Mikkelsen wrote:

Thanks.

The patch in that commit fails on FreeBSD. Specifically:

1.60 +    folder = opendir(KMSDRM_DRI_PATH);
1.61 +    if (folder) {
1.62 +        while ((res = readdir(folder))) {
1.63 +            if (res->d_type == DT_CHR) {
1.64 +                devcount++;
1.65 +            }
1.66 +        }
1.67 +        closedir(folder);
1.68 +    }
1.69 +

Only examines entries under /dev/dri which are character devices. On FreeBSD these are character devices which point to an entry under /dev/drm. For example:

janm@test-tipro: ~ $ ls -l /dev/dri
total 0
lrwxr-xr-x 1 root wheel 8 18 Jun 17:10 card0 -> ../drm/0
lrwxr-xr-x 1 root wheel 10 18 Jun 17:10 renderD128 -> ../drm/128

The patch in this bug report updates this to look for entries that start with "card" instead. It changes the "res->d_type == DT_CHR" test to "res->d_namlen > 4 && strncmp(res->d_name, "card", 4)".

I just looked at the patch and realised it was the wrong way around, sorry.

On 2019-06-18 21:16:29 +0000, Sam Lantinga wrote:

Got it, thanks!
https://hg.libsdl.org/SDL/rev/e490856ac51c

On 2019-06-19 07:18:32 +0000, Sylvain wrote:

This failed to compile on Linux,

error: no member named 'd_namlen' in 'struct dirent'; did you mean 'd_name'?
if (res->d_namlen > 4 && strncmp(res->d_name, "card", 4)) {
^~~~~~~~
d_name

Patch added https://hg.libsdl.org/SDL/rev/dd47d4ea431e

On 2019-06-19 07:30:49 +0000, Jan Martin Mikkelsen wrote:

OK, looking closer at struct dirent portability, I see that not all systems have all fields, and I shouldn't rely in d_namelen.

I'm not sure that "if (res->d_name)" is meaningful; it should always be true. Embarrassingly, I also see an inverted test here.

Something simple like:

     if (strncmp(res->d_name, "card", 4) == 0)

Looks more appropriate.

On 2019-06-19 07:45:40 +0000, Sylvain wrote:

You're right, "if (res->d_name)" is not meaningful.

Yes also for: strncmp(res->d_name, "card", 4) == 0

But it still requires "len > 4" right ?
(otherwise "c" "ca" "car" would match ?)

On 2019-06-19 08:12:31 +0000, Sylvain wrote:

This is committed https://hg.libsdl.org/SDL/rev/df4fda4ec707
Please try it!

On 2019-06-19 08:15:10 +0000, Jan Martin Mikkelsen wrote:

(In reply to Sylvain from comment # 7)

You're right, "if (res->d_name)" is not meaningful.

Yes also for: strncmp(res->d_name, "card", 4) == 0

But it still requires "len > 4" right ?
(otherwise "c" "ca" "car" would match ?)

Shorter than four characters (eg. "c", "ca" and "car") will not match. The issue with this approach is that "card" will match, where we really want "card0". This is why I originally had a "> 4" test.

It could also be:

    if (strlen(res->d_name) > 4 && strncmp(res->d_name, "card", 4) == 0)

I don't think it is worth looking too hard at this case. The code uses the count to later go through the "card%d" entries starting from zero up to the number of matches on this pattern. A more correct approach is to enumerate all of the /dev/dri/card* entries, and use the actual filename in check_modesetting, and when it matches, extract the card unit number.

I plan to have a closer look at kmsdrm to look at things like this, and to see how hard it would be to add multiple display support. I will do that when I have a little more time; this patch was intended to make what was there work on FreeBSD.

On 2019-06-19 08:20:32 +0000, Jan Martin Mikkelsen wrote:

(In reply to Sylvain from comment # 8)

This is committed https://hg.libsdl.org/SDL/rev/df4fda4ec707
Please try it!

This patch looks good, if I see problems (unlikely) I will let you know.

On 2019-06-19 08:21:49 +0000, Jan Martin Mikkelsen wrote:

(In reply to Sylvain from comment # 8)

This is committed https://hg.libsdl.org/SDL/rev/df4fda4ec707
Please try it!

Thanks for testing!