Primary node clarification for VK_EXT_physical_device_drm

[enunes]

The extension already hinted that the primary node represents display capabilities as its intended purpose, but the current text let it ambiguous enough that there has been confusion and divergence in proposed implementations.

Add clarification notes to further define the scope of primary node in the context of this extension.

There has been a long discussion about this issue mainly at https://gitlab.freedesktop.org/mesa/mesa/-/issues/9920 .
That discussion has evolved into solving many problems of split display/render systems, but here I would like to avoid doing that again and focusing only on defining the scope of this extension.

I also discussed with the involved people again recently about proposing this note to the extension.

cc @emersion @cubanismo @fooishbar

Other references:
https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2023-07-25&show_html=true
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25431
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/31635

[CLAassistant]

All committers have signed the CLA.

[cubanismo]

This is not what our implementation is doing right now. Is this what other drivers are doing currently?

[enunes]

This is not what drivers are currently doing but what I meant to put explicitly from what was hinted in the next paragraph (Physical devices may have no primary node (e.g. if the device does not have a display subsystem)).

To my knowledge what drivers in Mesa are doing is:

• v3dv: uses the other display DRM device as the primary. This is what we would like to fix here and prevent other drivers from following and applications to rely on.
• panvk: uses the DRM primary (cardN) node of the same render device as the primary, because it technically exists and from the DRM terminology it is in fact the "DRM primary node" (but is not a "display subsystem").
• pvr: same as panvk (but still pending to upstream Mesa).

I guess what panvk and pvr do is also acceptable to standardize on while similarly achieving the same goal, if we want to settle on something based on existing implementations. If so we also need to change the text to explicitly say that and probably remove parts of the text which associate primary devices as being display subsystems.

[fooishbar]

Neither panvk nor pvr are (yet) conformant, and require scary environment variables to unmask them, so I'm comfortable with changing their behaviour to match new wording.

[cubanismo]

Is this saying you can return primary node and render node from separate DRM devices on the same Vulkan physical device or not? It's not clear to me.

[enunes]

In this snippet I just meant to clarify the terminology for the context of the extension. As in "VK_EXT_physical_device_drm primary node" != "DRM primary node". This is also due to the existing sentence which says Physical devices may have no primary node (e.g. if the device does not have a display subsystem).

For the overall extension I want to say that it should not mix and return primary node and render node from separate DRM devices on the same Vulkan physical device.

I'm open to suggestions for phrasing this the best way possible.

[cubanismo]

For the overall extension I want to say that it should not mix and return primary node and render node from separate DRM devices on the same Vulkan physical device.

This seems fine to me. I'm less clear whether it would be useful to hide the primary node for DRM devices without a display engine. I don't recall whether there was a leaning or consensus on what was preferred by compositor authors from the FDO issue, but I'd probably be fine with whatever others prefer with the caveat that if we alter the language to prohibit advertising a primary node on displayless GPUs, we should probably bump the extension version so apps can easily detect older drivers that might do something different.

[enunes]

So I'm thinking now that it might be simpler to just drop the "hide the primary node for DRM devices without a display engine" part from this proposal. That is not a strictly needed change, so this keeps some implementations compliant and is still enough to achieve the goal. I guess applications can still check with the DRM API when they get a primary node whether it is a "display subsystem" or not, so this information does not need to be handled by this extension.

So I would modify this MR to propose: 1. focus on clarifying the language and decoupling "primary node" from "display subsystem" so that non-display-subsystem primary nodes of the same DRM render device are unambiguously allowed to go on the primary node if they exist; and 2. still explicitly mention that it should not mix and return primary node and render node from separate DRM devices on the same Vulkan physical device.

I'm fine bumping the extension version in any case. Is that just adding an entry in the changelog for VK_EXT_physical_device_drm.adoc or is the version tracked somewhere else?

[cubanismo]

The version is tracked in the XML file, which generates content in vulkan_core.h.

[fooishbar]

Yeah, I'm pretty comfortable with the suggested language; users can use libdrm to map between render nodes and primary nodes of the same device.

[emersion]

To me, the important bit is to forbid advertising two separate DRM devices with this extension. I don't have a strong preference regarding the primary device being exposed only if the device is KMS-capable. Some notes:

• If the device is not KMS-capable, the primary node is fairly useless.
• In general, applications can't expect to have the permission to open the primary node.

The primary node, in the context of this extension, refers to the capability of that device node to be used as a display subsystem.

That's a bit confusing IMHO. Maybe instead of trying to override what having a primary node means, we could add "If the DRM device is KMS-capable" elsewhere in the extension?

[enunes]

I pushed a new version considering the points I mentioned in comment #2447 (comment)

Since there is no strong need to hide the primary node (assuming that it belongs to the same DRM device) I think now that it is more intuitive to just include it there, since the primary node does exist and likely is not going away soon. So we don't introduce an entirely new rule here and have to go change every other implementation.

This new revision is less intrusive and just removes the term "display subsystem" and its association with "primary node". It also now explicitly forbids other DRM devices being used, so the only possible option is to expose the primary node of the same device. If it is not useful applications can ignore it, but I guess it is less harm it being there than having to introduce more potentially breaking changes for this extension.

[emersion]

I don't think that case is possible, actually. A render node always has a primary node.

[enunes]

I agree, I think the only case is the same as a software implementation which does not use DRM. Is it better to switch this text to something like "it always has the primary node if it is associated with a DRM device"?