Skip to content

Add feature to search for KMS DRI card #16

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

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

Add feature to search for KMS DRI card #16

Show file tree
Hide file tree
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
75 changes: 70 additions & 5 deletions gralloc_gbm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
#include <sys/stat.h>
#include <fcntl.h>
#include <assert.h>
#include <xf86drmMode.h>
#include <glob.h>

#include <hardware/gralloc.h>
#include <system/graphics.h>
Expand Down Expand Up @@ -311,13 +313,25 @@ struct gbm_device *gbm_dev_create(void)
{
struct gbm_device *gbm;
char path[PROPERTY_VALUE_MAX];
int path_len;
int fd;

property_get("gralloc.gbm.device", path, "/dev/dri/renderD128");
fd = open(path, O_RDWR | O_CLOEXEC);
if (fd < 0) {
ALOGE("failed to open %s", path);
return NULL;
path_len = property_get("gralloc.gbm.device", path, "/dev/dri/card*");
/* Search for KMS device with the pattern or open it from property */
if (path[path_len - 1] == '*') {
fd = open_first_kms_dev(path);
} else {
fd = open(path, O_RDWR | O_CLOEXEC);
}
Comment on lines +321 to +325
Copy link

@MarijnS95 MarijnS95 Sep 30, 2021

Choose a reason for hiding this comment

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

Seems a bit cheeky to assume that every glob must end in * otherwise it will be treated as a regular path. It shouldn't be too bad to call open_first_kms_dev unconditionally?


/* Open default path when KMS device wasn't found */
if (fd <= 0) {
strcpy(path, "/dev/dri/renderD128");
fd = open(path, O_RDWR | O_CLOEXEC);
if (fd < 0) {
ALOGE("failed to open %s with error %s", path, strerror(errno));
return NULL;
}
Copy link
Contributor

@rsglobal rsglobal Jan 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Please add drmDropMaster(fd); after this if () {} before line 337

See https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/-/merge_requests/134 for the explanation

}

gbm = gbm_create_device(fd);
Expand Down Expand Up @@ -529,3 +543,54 @@ int gralloc_gbm_bo_lock_ycbcr(buffer_handle_t handle,

return 0;
}

/*
* Check if target device has KMS.
*/
bool is_kms_dev(int fd)
{
auto res = drmModeGetResources(fd);
if (!res)
return false;

bool is_kms = res->count_crtcs > 0 && res->count_connectors > 0 &&
res->count_encoders > 0;

drmModeFreeResources(res);

return is_kms;
}

/*
* Search for a KMS device. Return opened file descriptor on success.
*/
int open_first_kms_dev(const char *pattern)
{
glob_t glob_buf;
memset(&glob_buf, 0, sizeof(glob_buf));
int fd;
Copy link

@MarijnS95 MarijnS95 Sep 30, 2021

Choose a reason for hiding this comment

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

Will glob() return 0 (instead of an error-code) if it found zero matching paths? If so, this should be initialized as to not return garbage when the loop never runs.


int ret = glob(pattern, 0, NULL, &glob_buf);
if (ret) {
globfree(&glob_buf);
return -EINVAL;
}

for (size_t i = 0; i < glob_buf.gl_pathc; ++i) {
fd = open(glob_buf.gl_pathv[i], O_RDWR | O_CLOEXEC);
if (fd < 0) {
ALOGE("failed to open %s with error %s", glob_buf.gl_pathv[i], strerror(errno));
continue;
}

if (is_kms_dev(fd))
break;

close(fd);
fd = 0;
Copy link

@MarijnS95 MarijnS95 Sep 30, 2021

Choose a reason for hiding this comment

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

Theoretically 0 is a valid fd (if it weren't usually used for stdin), this should probably be -1 (and the <= 0 above should be < 0).

}

globfree(&glob_buf);

return fd;
}
3 changes: 3 additions & 0 deletions gralloc_gbm_priv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ int gralloc_gbm_bo_lock_ycbcr(buffer_handle_t handle, int usage,
struct gbm_device *gbm_dev_create(void);
void gbm_dev_destroy(struct gbm_device *gbm);

bool is_kms_dev(int fd);
int open_first_kms_dev(const char *pattern);

#ifdef __cplusplus
}
#endif
Expand Down