linux-raw-sys icon indicating copy to clipboard operation
linux-raw-sys copied to clipboard

Add bindings to drm api

Open morr0ne opened this issue 1 year ago • 23 comments

Adds bindings to drm.h Contains userspace api for interacting with the direct rendering manager infrastructure

morr0ne avatar Jun 25 '24 15:06 morr0ne

How does this differentiate itself from drm-sys?

notgull avatar Jun 25 '24 15:06 notgull

How does this differentiate itself from drm-sys?

Unless they are generating against a different header it actually shouldn't differ.

This crate is meant to be a general purpose binding library against linux user space apis. Adding support for drm here avoids having to pull an extra dependency just for that one api.

morr0ne avatar Jun 25 '24 15:06 morr0ne

To add to my comment above. This reasoning can be applied to any linux api. We could have a a separate crate for each api but having one unified crate makes maintenance easier and avoids dependency hell to a certain extent.

This crate already gets pulled by almost everything either directly or transitively via rustix

morr0ne avatar Jun 25 '24 15:06 morr0ne

I've opened an issue in drm-rs for this.

notgull avatar Jun 26 '24 01:06 notgull

I wouldn't mind deprecating drm-sys in favor of this (though it wouldn't be my decision alone).

Is it the goal to eventually have the full DRM UAPI generated and included in this crate? I see drm.h is generated (which also includes drm_mode.h I think?) but I was looking to get the various vendor-specific headers included at some point. I'm not sure how difficult it would be to do that here.

Slabity avatar Jun 26 '24 01:06 Slabity

Another thing I'm curious about is whether these will work with non-Linux systems.

Unlike many other kernel subsystems, DRM is a fairly standard interface across many Unix-like kernels including FreeBSD and OpenBSD. I'm guessing the cross-platform interfaces might be better suited for something slightly higher-level like rustix?

Slabity avatar Jun 26 '24 18:06 Slabity

Another thing I'm curious about is whether these will work with non-Linux systems.

This bindings are generated specifically from the header provided by the linux kernel. They might work on other systems but it's not guaranteed. As far as I am aware the canonical definitions for drm are this header but in case they differ you could consider this bindings for the "linux flavor" of drm. After all this crate is meant to be a wrapper for linux apis.

Unlike many other kernel subsystems, DRM is a fairly standard interface across many Unix-like kernels including FreeBSD and OpenBSD. I'm guessing the cross-platform interfaces might be better suited for something slightly higher-level like rustix?

Even if the interface is standard there is no guarantee that the exact definitions are the same unless they all rely on the same source of truth. Ideally one would generate the bindings for each os/kernel and let an higher level library deal with the differences.

I am not too informed on other os support as my main use case for this api is diretto for which I have no plans of supporting anything but the linux kernel. Of course the beauty of foss is that anyone can benefit from this bindings :)

A quick search does seem to suggest that FreeBSD and OpenBSD drm drives are based on the existing linux ones so one could operate on the assumption that the interfaces are identical in which case this bindings should work without issues.

morr0ne avatar Jun 26 '24 19:06 morr0ne

I wouldn't mind deprecating drm-sys in favor of this (though it wouldn't be my decision alone).

Having one less dependency is always a plus in my book however before doing that it we need to ensure the bindings actually match. Some stuff is probably getting missed by this bindings because of macros bindgen is unable to expand.

Is it the goal to eventually have the full DRM UAPI generated and included in this crate? I see drm.h is generated (which also includes drm_mode.h I think?) but I was looking to get the various vendor-specific headers included at some point. I'm not sure how difficult it would be to do that here.

Eventually it might make sense to have all the vendor specific headers but I am not sure what the best approach is. The headers make heavy use of macros and I can't confidently say that something isn't missed until someone tries to use it. The current bindings do source drm_mode.h since some structs are referenced by drm.h. The header isn't actually included so just for correctness I added it to the module. The actual bindings haven't changed.

morr0ne avatar Jun 26 '24 19:06 morr0ne

@Slabity I think you made what seemed like an interesting point here, but I no longer see it in the thread. Did you wish to make a follow-up comment here?

sunfishcode avatar Jul 02 '24 14:07 sunfishcode

@Slabity I think you made what seemed like an interesting point here, but I no longer see it in the thread. Did you wish to make a follow-up comment here?

Yea, sorry about that. I was writing a draft reply and accidentally posted it before reviewing properly. Then I immediately deleted it because it was 3AM and I wanted to make the post while not sleep-deprived. I guess Github email notifications still go out with the full message even if you remove it?

I'll try to make a better write-up soon about how the DRM API is developed and why I recommend keeping those bindings separate from the rest of the UAPI bindings. Though please take my words with a grain of salt as I am not a contributor to this project.

Slabity avatar Jul 03 '24 21:07 Slabity

I'll try to make a better write-up soon about how the DRM API is developed and why I recommend keeping those bindings separate from the rest of the UAPI bindings. Though please take my words with a grain of salt as I am not a contributor to this project.

The bindings should definitely be in this crate since they’re part of the kernel uapi. They’re just like any other uapi headers, and as long as the kernel doesn’t break userspace, they’ll remain valid. There’s no reason they can’t coexist with other bindings such as drm-sys. However the drm api isn’t special, other unix-like systems can implement the same interfaces, but the Linux kernel headers are the canonical source for such interface, any divergence should be treated as a potential implementation issue.

This crate only provides raw definitions for linux uapi interfaces. It doesn’t handle linking or loading. These definitions can be used in any OS that supports them. Actual calls still need to happen through external crates using syscalls, ioctls, or similar mechanisms

morr0ne avatar Jul 03 '24 22:07 morr0ne

I had already read the entire message from my mailbox before noticing on GitHub that the comment had been deleted. I cannot say that I agree with the points brought up in the message, however, I will await a complete response on your part before adding more to the discussion. In the meantime, I would appreciate it if you could consider my previous comment on the matter

morr0ne avatar Jul 03 '24 22:07 morr0ne

I'm leaning towards saying that since drm-sys already exists and appears to be maintained by people familiar with the DRM APIs, it makes sense to leave DRM out of linux-raw-sys.

In particular:

Eventually it might make sense to have all the vendor specific headers but I am not sure what the best approach is.

As the maintainer of linux-raw-sys, I am not currently experienced with the DRM API, so I don't know how to evaluate questions like this. If linux-raw-sys were the only option, it might make sense to try to figure this out, but here, we seem to already have a better option available.

By my reading of the discussion above, the advantage of putting DRM APIs in linux-raw-sys is to allow users that only target linux to avoid pulling in an extra dependency. That seems to me to be a relatively minor cost.

sunfishcode avatar Aug 14 '24 19:08 sunfishcode

I am not too informed on other os support as my main use case for this api is diretto for which I have no plans of supporting anything but the linux kernel. Of course the beauty of foss is that anyone can benefit from this bindings :)

@morr0ne if I may ask, how does diretto plan to differentiate itself from the already existing and definitely more complete/mature drm-rs crate, that also provides high-level bindings to DRM APIs?

MarijnS95 avatar Aug 14 '24 20:08 MarijnS95

@morr0ne if I may ask, how does diretto plan to differentiate itself from the already existing and definitely more complete/mature drm-rs crate, that also provides high-level bindings to DRM APIs?

While this might be a more appropriate question for the diretto repo, I'm happy to answer it here. The primary differences between the two projects are as follows

Diretto's api is designed differently because it manages the device for you and includes helper functions to ensure safer interactions with the hardware. On the other hand, drm-rs provides a trait that allows users to call the drm ioctls on any file descriptor, placing the responsibility for safety checks on the user rather than the library itself.

Additionally, diretto offers (or at least will offer) both low-level apis and higher-level wrappers with utility functions, whereas drm-rs focuses mainly on low-level bindings, often requiring external crates like smithay-drm-extras for added functionality. Another difference is that diretto's low level api is intended to align more closely with the kernel api, while drm-rs appears to be more closely related to the libdrm library.

In the future diretto might also include apis for dealing with kms planes similar to libliftoff

There are also some broader considerations that might be worth mentioning. Diretto is part of a larger effort on my part to rethink the wayland ecosystem in rust, using an approach that differs significantly from smithay’s. In line with this vision, I’ve been working to rewrite various parts of the ecosystem under the verdi org, including diretto. Using an already existing library like drm-rs goes somewhat against the goals of the project.

morr0ne avatar Aug 14 '24 20:08 morr0ne

I'm leaning towards saying that since drm-sys already exists and appears to be maintained by people familiar with the DRM APIs, it makes sense to leave DRM out of linux-raw-sys.

In particular:

Eventually it might make sense to have all the vendor specific headers but I am not sure what the best approach is.

As the maintainer of linux-raw-sys, I am not currently experienced with the DRM API, so I don't know how to evaluate questions like this. If linux-raw-sys were the only option, it might make sense to try to figure this out, but here, we seem to already have a better option available.

By my reading of the discussion above, the advantage of putting DRM APIs in linux-raw-sys is to allow users that only target linux to avoid pulling in an extra dependency. That seems to me to be a relatively minor cost.

Given that the drm api is inherently linux api, accessible from user space, it makes sense to place it here for direct integration. While more sophisticated abstractions might be better suited for projects like drm-sys, the primary goal here is to support the bindings to the linux kernel in a relatively straightforward manner. There is minimal ambiguity in this approach, as the drm api functions similarly to any other kernel api, and should not introduce additional maintenance overhead. In the end this provides just a few structs definitions and some constants since the interactions with the api itself happen through ioctls

morr0ne avatar Aug 14 '24 20:08 morr0ne

Hey, I apologize for not following up earlier as I promised. I should have followed up a few weeks back without stagnating the conversation. That's my bad.

Anyway, to get straight to the point, my main concern essentially starts and stops at the fact that this API is not Linux-exclusive. Providing a crate that explicitly does not support the other platforms means that downstream projects will either a) need to support 2 different dependencies depending on the platform targeted, or b) they will simply not support the non-Linux platforms for arbitrary reasons.

The reason for my concern is that significant effort is made upstream to make the DRM API compatible with many other platforms without needing to change the code significantly. The vast majority of the code is MIT licensed (as opposed to GPL for almost the rest of the UAPI), and I think it's the only part of the kernel UAPI that actually has checks in the headers to determine if you're compiling on Linux or the other BSD systems. These UAPI headers are shared directly with other kernels and the Mesa project so the API is essentially 100% compatible with other platforms without any actual changes. There's a lot to be said on the kernel-driver side as well that demonstrates this further, but those details are probably not important for anyone in userspace.

So that's basically my concern. I hope that was a bit more coherent than the early-morning rambling I accidentally sent weeks ago :upside_down_face:

Slabity avatar Aug 15 '24 22:08 Slabity

The reason for my concern is that significant effort is made upstream to make the DRM API compatible with many other platforms without needing to change the code significantly. The vast majority of the code is MIT licensed (as opposed to GPL for almost the rest of the UAPI), and I think it's the only part of the kernel UAPI that actually has checks in the headers to determine if you're compiling on Linux or the other BSD systems. These UAPI headers are shared directly with other kernels and the Mesa project so the API is essentially 100% compatible with other platforms without any actual changes. There's a lot to be said on the kernel-driver side as well that demonstrates this further, but those details are probably not important for anyone in userspace.

I was not fully aware of the extensive efforts made in this regard. I recognize that I should have taken the initiative to better inform myself. That said, the canonical source for the api remains the linux uapi headers, correct? Given that this remains the case, I still think it makes sense to include them here. What would be the concern in having such bindings in this crate?

morr0ne avatar Aug 15 '24 22:08 morr0ne

On the other hand, drm-rs provides a trait that allows users to call the drm ioctls on any file descriptor, placing the responsibility for safety checks on the user rather than the library itself.

There's nothing inherently unsafe about this. Running an ioctl on a character device file-descriptor that doesn't support the underlying call will just result in an error. The reasoning for exposing it as a trait is because different applications acquire the card with different means (whether through /dev/dri/*, sysfs, udev, dbus, a UNIX socket, or even through an already open GL/Vulkan context/instance).

Additionally, diretto offers (or at least will offer) both low-level apis and higher-level wrappers with utility functions, whereas drm-rs focuses mainly on low-level bindings, often requiring external crates like smithay-drm-extras for added functionality. Another difference is that diretto's low level api is intended to align more closely with the kernel api, while drm-rs appears to be more closely related to the libdrm library.

These two statements are contradictory. The drm-rs crate does focus on low-level bindings, but it aligns very closely with the kernel API with very few exceptions. The drm-rs crate does not have anything to do with the higher-level libdrm and does not provide the same functionality (with is closer to what smithay-drm-extras does).

Diretto is part of a larger effort on my part to rethink the wayland ecosystem in rust, using an approach that differs significantly from smithay’s.

I'm definitely interested in different Wayland approaches and will probably follow the project based on that alone. However, the drm-* crates have nothing to do with Smithay besides the fact that the org helps maintains it and prep releases.

In line with this vision, I’ve been working to rewrite various parts of the ecosystem under the verdi org, including diretto. Using an already existing library like drm-rs goes somewhat against the goals of the project.

If that's the case, why not just use drm-sys or drm-ffi? The drm-sys crate should provide the exact bindings you generated here. It should actually be more complete as it also exposes the C macros that Bindgen does not include by default. The drm-ffi crate is a thin wrapper around those that makes the interfaces memory-safe.

Given that this remains the case, I still think it makes sense to include them here. What would be the concern in having such bindings in this crate?

Well one thing that could be done to both expose the API in this crate and keep it supported across all viable platforms is to export the bindings from drm-sys in this crate directly. That would ensure that this crate has the complete UAPI, downstream projects could still be compatible with all platforms, and it would reduce maintenance overall.

After all, the DRM API is the largest UAPI in the kernel by far (whether measured by lines of code, number or ioctls, or number of maintainers) and once we start including the vendor-specific bindings then things will start being a bit more difficult.

Slabity avatar Aug 15 '24 22:08 Slabity

To clarify, if there are concerns beyond just code duplication, I am more than willing to close this PR. However, I would appreciate a more comprehensive understanding of the situation before proceeding. At the moment, I am struggling to see how including the bindings here presents an issue for compatibility.

morr0ne avatar Aug 15 '24 22:08 morr0ne

There's nothing inherently unsafe about this. Running an ioctl on a character device file-descriptor that doesn't support the underlying call will just result in an error. The reasoning for exposing it as a trait is because different applications acquire the card with different means (whether through /dev/dri/*, sysfs, udev, dbus, a UNIX socket, or even through an already open GL/Vulkan context/instance).

I want to emphasize that this isn't a critique of how drm-rs handles things. Drm-rs is a solid project, but I have different requirements for my work that wouldn’t necessarily make sense to push onto drm-rs. My preference for an api that owns the device, rather than a trait-based one, reflects my specific needs. The difference in approach is just that, different, not inherently better or worse. I do however recognize that "unsafe" might not have been the best way to word things

These two statements are contradictory. The drm-rs crate does focus on low-level bindings, but it aligns very closely with the kernel API with very few exceptions. The drm-rs crate does not have anything to do with the higher-level libdrm and does not provide the same functionality (with is closer to what smithay-drm-extras does).

These two statements are contradictory. The drm-rs crate does focus on low-level bindings, but it aligns very closely with the kernel API with very few exceptions. The drm-rs crate does not have anything to do with the higher-level libdrm and does not provide the same functionality (with is closer to what smithay-drm-extras does).

I understand that the crate closely aligns with the kernel API, focusing on low-level bindings rather than higher-level functionality like libdrm. My own dissatisfaction with the drm-rs API stems from the added complexity I experienced. To me, it felt like it was layering too many abstractions, which made it harder to work with in some cases. That’s why I’ve pursued a more direct and streamlined approach with Diretto.

I'm definitely interested in different Wayland approaches and will probably follow the project based on that alone. However, the drm-* crates have nothing to do with Smithay besides the fact that the org helps maintains it and prep releases.

On the topic of Smithay's involvement with the drm-* crates, while it’s true that Smithay may not explicitly tie them to the overall project, the fact that the organization manages them inherently associates them with the project’s ecosystem. This connection, whether acknowledged officially or not, creates some level of association that I feel is worth considering.

If that's the case, why not just use drm-sys or drm-ffi? The drm-sys crate should provide the exact bindings you generated here. It should actually be more complete as it also exposes the C macros that Bindgen does not include by default. The drm-ffi crate is a thin wrapper around those that makes the interfaces memory-safe.

I am aware of that but like state above I prefer having less layers of indirection. It also is one less crate that needs vetting and auditing in the future. I have been considering dropping even the linux-raw-sys crate and use internal bindings directly which would make this pr unnecessary.

Well one thing that could be done to both expose the API in this crate and keep it supported across all viable platforms is to export the bindings from drm-sys in this crate directly. That would ensure that this crate has the complete UAPI, downstream projects could still be compatible with all platforms, and it would reduce maintenance overall.

After all, the DRM API is the largest UAPI in the kernel by far (whether measured by lines of code, number or ioctls, or number of maintainers) and once we start including the vendor-specific bindings then things will start being a bit more difficult.

That would worse an extra dependency which would intern make this pr redudant since one could depend directly on drm-sys.

Finally, I want to be clear that I have no issues with drm-rs. I simply favor a different approach to handling the drm api, and Diretto better addresses my use case. Additionally, I have broader plans for Diretto that haven’t been made public yet but will further differentiate it as its own project.

This work remains experimental, and my intention here is to contribute back to the community rather than keeping everything private. That said, I believe we’ve discussed this PR in depth, and any further conversation might be better suited under the verdi org if you’re interested in continuing the dialogue.

morr0ne avatar Aug 15 '24 22:08 morr0ne

To clarify, if there are concerns beyond just code duplication, I am more than willing to close this PR. However, I would appreciate a more comprehensive understanding of the situation before proceeding. At the moment, I am struggling to see how including the bindings here presents an issue for compatibility.

Ah, so let's take diretto for example. Let's say you create a high-quality interface to the DRM API using the bindings generated for this crate as the interface. And then let's say diretto has no other Linux-specific dependencies to speak of (I don't know if that's the case, just speaking as an example).

If someone decides to use diretto in their own project, then any crate or program that they create will not be able to compile on something like FreeBSD because linux-raw-sys does not support that platform. Even if their own code has zero dependencies on any other Linux-specific APIs, their project is essentially locked out of those platforms. The result would be one of the following:

  1. The project will not care to support other platforms than Linux, resulting either a) those other platforms miss out on software that has no actual technical issue on their system or b) those other platforms will need to maintain their own patches/forks.
  2. The project will refuse to use diretto because it doesn't support those platforms.
  3. The diretto crate would need to have 2 different paths for Linux and non-Linux platforms.

Slabity avatar Aug 15 '24 22:08 Slabity

If someone decides to use diretto in their own project, then any crate or program that they create will not be able to compile on something like FreeBSD because linux-raw-sys does not support that platform. Even if their own code has zero dependencies on any other Linux-specific APIs, their project is essentially locked out of those platforms. The result would be one of the following:

If my project depends on linux-raw-sys other platforms have been locked out regardless. If it only depends on the drm part of the api then the code should compile without issues on non linux platform. There is no linking or bindings. Just a bunch of structs, types and consts

  • The project will not care to support other platforms than Linux, resulting either a) those other platforms miss out on software that has no actual technical issue on their system or b) those other platforms will need to maintain their own patches/forks.

  • The project will refuse to use diretto because it doesn't support those platforms.

  • The diretto crate would need to have 2 different paths for Linux and non-Linux platforms.

I have to admit I have taken little to no consideration of other platforms since I do not have plans to support non-linux platforms, at least for now. Diretto is mostly meant to suit the needs of verdi but as I said before. I might forgo any external crate entirely and instead use internal bindings.

morr0ne avatar Aug 15 '24 22:08 morr0ne

While I see value in this, I decided it's better to avoid more fragmentation in the ecosystem

morr0ne avatar Dec 19 '24 19:12 morr0ne