[PATCH 1/2] Add new drmOpenWithType function (v3)
Zhou, Jammy
Jammy.Zhou at amd.com
Tue Feb 10 20:01:51 PST 2015
Thanks for the comments, Frank. I will send out the updated patch later.
Regards,
Jammy
-----Original Message-----
From: Frank Binns [mailto:frank.binns at imgtec.com]
Sent: Wednesday, February 11, 2015 1:04 AM
To: Zhou, Jammy; dri-devel at lists.freedesktop.org
Subject: Re: [PATCH 1/2] Add new drmOpenWithType function (v3)
On 02/02/15 10:06, Jammy Zhou wrote:
> v2: Add drmGetMinorBase, and call drmOpenWithType in drmOpen
> v3: Pass 'type' to drmOpenByBusid and drmOpenDevice in drmOpenByName
>
> Signed-off-by: Jammy Zhou <Jammy.Zhou at amd.com>
> ---
> xf86drm.c | 63
> ++++++++++++++++++++++++++++++++++++++++++++++++---------------
> xf86drm.h | 9 ++++++++-
> 2 files changed, 56 insertions(+), 16 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 345325a..810edfa 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -85,10 +85,6 @@
>
> #define DRM_MSG_VERBOSITY 3
>
> -#define DRM_NODE_CONTROL 0
> -#define DRM_NODE_PRIMARY 1
> -#define DRM_NODE_RENDER 2
> -
> static drmServerInfoPtr drm_server_info;
>
> void drmSetServerInfo(drmServerInfoPtr info) @@ -493,11 +489,24 @@
> int drmAvailable(void)
> return retval;
> }
>
> +static int drmGetMinorBase(int type)
> +{
> + switch (type) {
> + case DRM_NODE_PRIMARY:
> + default:
> + return 0;
> + case DRM_NODE_CONTROL:
> + return 64;
> + case DRM_NODE_RENDER:
> + return 128;
> + };
Wouldn't it be more sensible to return -1 in the default case given that there's no validation of the 'type' argument in drmOpenWithType? It feels wrong defaulting to the primary node in this case.
> +}
>
> /**
> * Open the device by bus ID.
> *
> * \param busid bus ID.
> + * \param type device node type.
> *
> * \return a file descriptor on success, or a negative value on error.
> *
> @@ -507,16 +516,17 @@ int drmAvailable(void)
> *
> * \sa drmOpenMinor() and drmGetBusid().
> */
> -static int drmOpenByBusid(const char *busid)
> +static int drmOpenByBusid(const char *busid, int type)
> {
> int i, pci_domain_ok = 1;
> int fd;
> const char *buf;
> drmSetVersion sv;
> + int base = drmGetMinorBase(type);
>
> drmMsg("drmOpenByBusid: Searching for BusID %s\n", busid);
> - for (i = 0; i < DRM_MAX_MINOR; i++) {
> - fd = drmOpenMinor(i, 1, DRM_NODE_PRIMARY);
> + for (i = base; i < base + DRM_MAX_MINOR; i++) {
> + fd = drmOpenMinor(i, 1, type);
> drmMsg("drmOpenByBusid: drmOpenMinor returns %d\n", fd);
> if (fd >= 0) {
> /* We need to try for 1.4 first for proper PCI domain support @@
> -556,6 +566,7 @@ static int drmOpenByBusid(const char *busid)
> * Open the device by name.
> *
> * \param name driver name.
> + * \param type the device node type.
> *
> * \return a file descriptor on success, or a negative value on error.
> *
> @@ -566,19 +577,20 @@ static int drmOpenByBusid(const char *busid)
> *
> * \sa drmOpenMinor(), drmGetVersion() and drmGetBusid().
> */
> -static int drmOpenByName(const char *name)
> +static int drmOpenByName(const char *name, int type)
> {
> int i;
> int fd;
> drmVersionPtr version;
> char * id;
> + int base = drmGetMinorBase(type);
>
> /*
> * Open the first minor number that matches the driver name and isn't
> * already in use. If it's in use it will have a busid assigned already.
> */
> - for (i = 0; i < DRM_MAX_MINOR; i++) {
> - if ((fd = drmOpenMinor(i, 1, DRM_NODE_PRIMARY)) >= 0) {
> + for (i = base; i < base + DRM_MAX_MINOR; i++) {
> + if ((fd = drmOpenMinor(i, 1, type)) >= 0) {
> if ((version = drmGetVersion(fd))) {
> if (!strcmp(version->name, name)) {
> drmFreeVersion(version);
> @@ -620,9 +632,9 @@ static int drmOpenByName(const char *name)
> for (devstring = ++pt; *pt && *pt != ' '; ++pt)
> ;
> if (*pt) { /* Found busid */
> - return drmOpenByBusid(++pt);
> + return drmOpenByBusid(++pt, type);
> } else { /* No busid */
> - return drmOpenDevice(strtol(devstring, NULL, 0),i, DRM_NODE_PRIMARY);
> + return drmOpenDevice(strtol(devstring, NULL, 0),i, type);
> }
> }
> }
> @@ -652,8 +664,29 @@ static int drmOpenByName(const char *name)
> */
> int drmOpen(const char *name, const char *busid) {
> + return drmOpenWithType(name, busid, DRM_NODE_PRIMARY); }
> +
> +/**
> + * Open the DRM device with specified type.
> + *
> + * Looks up the specified name and bus ID, and opens the device
> +found. The
> + * entry in /dev/dri is created if necessary and if called by root.
> + *
> + * \param name driver name. Not referenced if bus ID is supplied.
> + * \param busid bus ID. Zero if not known.
> + * \param type the device node type to open, PRIMARY, CONTROL or
> +RENDER
> + *
> + * \return a file descriptor on success, or a negative value on error.
> + *
> + * \internal
> + * It calls drmOpenByBusid() if \p busid is specified or
> +drmOpenByName()
> + * otherwise.
> + */
> +int drmOpenWithType(const char *name, const char *busid, int type) {
> if (!drmAvailable() && name != NULL && drm_server_info) {
> - /* try to load the kernel */
> + /* try to load the kernel module */
> if (!drm_server_info->load_module(name)) {
> drmMsg("[drm] failed to load kernel module \"%s\"\n", name);
> return -1;
> @@ -661,13 +694,13 @@ int drmOpen(const char *name, const char *busid)
> }
>
> if (busid) {
> - int fd = drmOpenByBusid(busid);
> + int fd = drmOpenByBusid(busid, type);
> if (fd >= 0)
> return fd;
> }
>
> if (name)
> - return drmOpenByName(name);
> + return drmOpenByName(name, type);
>
> return -1;
> }
> diff --git a/xf86drm.h b/xf86drm.h
> index bfd0670..f145d42 100644
> --- a/xf86drm.h
> +++ b/xf86drm.h
> @@ -552,7 +552,14 @@ do { register unsigned int __old __asm("o0"); \
> /* General user-level programmer's API: unprivileged */
> extern int drmAvailable(void);
> extern int drmOpen(const char *name, const char *busid);
> -extern int drmOpenControl(int minor);
> +
> +#define DRM_NODE_CONTROL 0
> +#define DRM_NODE_PRIMARY 1
> +#define DRM_NODE_RENDER 2
Nit, now that these are public it might be nice to have primary first,
.i.e.:
#define DRM_NODE_PRIMARY 0
#define DRM_NODE_CONTROL 1
#define DRM_NODE_RENDER 2
> +extern int drmOpenWithType(const char *name, const char *busid,
> + int type);
> +
> +extern int drmOpenControl(int minor);
> extern int drmOpenRender(int minor);
> extern int drmClose(int fd);
> extern drmVersionPtr drmGetVersion(int fd);
More information about the dri-devel
mailing list