Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x11: don't do DRI3 with Intel drivers #727

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

x11: don't do DRI3 with Intel drivers #727

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion va/x11/va_x11.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,37 @@ static VAStatus va_DisplayContextGetDriverNames(
{
VAStatus vaStatus = VA_STATUS_ERROR_UNKNOWN;

if (!getenv("LIBVA_DRI3_DISABLE"))
const char* dri3_disable = getenv("LIBVA_DRI3_DISABLE");
if (!dri3_disable || !atoi(dri3_disable)) {
unsigned old_num_drivers = *num_drivers;

vaStatus = va_DRI3_GetDriverNames(pDisplayContext, drivers, num_drivers);
/* As of 8 July 2023, i965 and iHD drivers lack DRI3 support.
*
* Requests by the community were raised as early as 29 July 2017,
* with DRI3 support landing in libva on the 27 September 2022. At of
* time of writing it's unknown if/when that would materialise.
*
* To handle this on libva level, we are explicitly disabling DRI3
* support on said drivers - it scales better than having every user
* to set the environment override listed above.
*
* Omit them by default, set LIBVA_DRI3_DISABLE=0 to bypass.
*/
if (vaStatus == VA_STATUS_SUCCESS && dri3_disable && !atoi(dri3_disable)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above condition skips the Intel check when LIBVA_DRI_DISABLE=0 is set, so the behavior is reverse from intended (intended: LIBVA_DRI_DISABLE=0 allows intel dri3, unset variable denies intel dri3).

The following would work as intended (tested), i.e. automatic intel skipping only done if the variable is unset:

if (vaStatus == VA_STATUS_SUCCESS && !dri3_disable) {

for (unsigned i = 0; i < *num_drivers; i++) {
if (drivers[i] && (!strcmp(drivers[i], "iHD") ||
!strcmp(drivers[i], "i965")))
vaStatus = VA_STATUS_ERROR_UNKNOWN;
}
if (vaStatus == VA_STATUS_ERROR_UNKNOWN) {
for (unsigned i = 0; i < *num_drivers; i++)
free(drivers[i]);
*num_drivers = old_num_drivers;
}
Comment on lines +103 to +107
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my testing the above does not seem to be enough.

va_DRI3_GetDriverNames() will initialize drm_state and set drm_state->auth_type to VA_DRM_AUTH_CUSTOM, causing va_isDRI2Connected() to then skip authentication, causing VA-API rendering to again not work.

As a hack, I added the following in the above if statement and then all seemed to work:

            struct drm_state *drm_state = pDisplayContext->pDriverContext->drm_state;

            /* also undo other initialization GetDriverNames() did: */
            if (drm_state->fd != -1)
                close(drm_state->fd);
            drm_state->fd = -1;
            drm_state->auth_type = VA_DRM_AUTH_NONE;

However, that feels like too much of a hack so some more refactoring is probably needed.

}
}

if (vaStatus != VA_STATUS_SUCCESS)
vaStatus = va_DRI2_GetDriverNames(pDisplayContext, drivers, num_drivers);
#ifdef HAVE_NVCTRL
Expand Down