libva

libva copied to clipboard

This MR fixes some pre-existing bugs in the libva (backend) driver handling and adds DRI3 support.

In other words:

• the correct module is picked on radeon (drm) + radeonsi (mesa/va) drivers
• ~~the LIBVA_DRIVER_NAME override works correctly~~ pulled out for now
• the render-node is properly detected, in some corner case
• DRI3 Xserver and Xwayland works.

Note: Only DRI3 open/fd retrieval is added enough for GetDisplay. All the winsys part is deliberately omitted, since it is heavily driver specific and makes little sense to add here.

In practise the x11/wayland winsys code in libva-{x11,wayland} has never been appealing enough to be used in the Mesa VA drivers. If i965 (legacy intel-driver) or iHD (new intel-media-driver) are interested they can adapt it to their needs.

Didn't realise the autotools build was still used. Squashed a fix in the respective commit.

This closes #79 and #122, right?

This closes #79 and #122, right?

Indeed it does. Thank you for tracking these down o/

Pushed a minor fix (were printing null instead of the env/api override string).

For testing I've used mpv --hwdec=vaapi and vainfo, against the radeonsi and i965 vaapi drivers. To confirm the correct display is used with I've used https://github.com/intel/libva-utils/pull/278 and/or explicitly setting vainfo --display foo

• drm session - switch to VT, kick off mpv
• X11 session - plasma/x11 + local debug for the DRI3 codepath
• wayland - switch to VT, start weston
• wayland+xwayland - switch to VT, configure printf "[core]\nxwayland=true" >.~/config/weston.ini, start weston and unset WAYLAND_DISPLAY before using mpv
• nested weston (with and w/o Xwayland) in a X11 session

For all the various readers, please give this a test. Let me know if it doesn't work or thumbs up if it does. Thanks o/

@XinfengZhang @uartie humble ping?

Fixing the LIBVA_DRIVER_NAME high-lighted a pre-existing issue, so Ive pulled it out for now. Will follow-up at later stage addressing that.

For now the MR is good to land.

Acked-by: David Heidelberg <[email protected]>

v4 includes:

• rebase against master
• beefy comment about the lack of close/fd ownership issue
• correct node-type switch

v5: drop bonus fd ownership comment, close() as applicable

the vaPutSurface will certainly failed, of course, maybe it could be resolved later

I think we can do that step-by-step. Authentication goes first. But adding a note for vaPutSurface behavior with DRI3 would be nice.

@evelikov : can you, please, also rebase the code on tip of the master. This will take in some ci adjustments.

@evelikov : can you, please, also rebase the code on tip of the master. This will take in some ci adjustments.

I rebased it with web operation

but it expose another question about the support of vaPutSurface: from this logic, if driver still only support DRI2, the vaPutSurface will certainly failed, of course, maybe it could be resolved later (we are recommended to use DMA buffer sharing instead of vaPutSurface)

As mentioned in the summary - I've explicitly omitted vaPutSurface. There are many reasons for that:

• as you said - people should be using buffer sharing dmabuf or otherwise
• the exact *implementation details of are driver specific and makes little sense to add here - for example: mesa's VA drivers do not use any of the libva x11-dri2 code, plus already has libva x11-dri3 support
• I could not find any software that uses vaPutSurface apart from libva-utils
• vaPutSurface does not work with intel-vaapi-driver on my BDW GPU - oops, is that intentional?
• vaPurSurface is available only on va-x11 - inconsistent API across targets makes for limited adoption and testing

there are conflict with #615 , and also need run style_unify script. have no other concern.

there are conflict with #615 , and also need run style_unify script. have no other concern.

Should be all good now

Humble ping - what can I do to move this forward?

Would be great to have it merged before its 3 month anniversary :wink:

sorry to let you feel any inconvenience , actually, I just tested it, it could pass with vainfo --display=x11 under wayland.
will merge it soon, but it is an important change, prefer to keep it in master branch more time, dont want to cherrypick to 2.16 directly, because we already test 2.16 with media driver and oneVPL several rounds.

and it will be a part of 2.17 release, is it ok?

Above all - I fully agree, this is not a something to pick for 2.16 and it should be in master for at least a week before going into a release.

Also there is no inconvenience, although clarity is sorely missing.

In particular, it is fine if you prefer that both maintainers have to review every MR. It's also fine for reviews or merging to take time. But please document that in the CONTRIBUTING (as requested in https://github.com/intel/libva/issues/622).

For example: The current project maintainers are X, Y, Z ... If you haven't received any feedback on your PR, within X weeks please @ tag them directly. Currently X (one, two, all) maintainers must approve a PR before it gets merged.

In some cases they will approve PRs, while not merging them due to X reasons. In such cases reply to the PR if it has been left unmerged for X weeks.

Feel free to reuse ^^ substituting X/Y/Z as applicable.

thanks, will change the contributing guide, 1 approval from maintainer is mandatory, and no objection from maintainers ， merged it

Great thanks. Do you mind closing the following PRs and issues:

• https://github.com/intel/libva/pull/292
• https://github.com/intel/libva/pull/491
• https://github.com/intel/libva/issues/79
• https://github.com/intel/libva/issues/122

Do you mind closing the following PRs and issues

I closed them.