Thanks for the comments, Frank. I will send out the updated patch later. Regards, Jammy -----Original Message----- From: Frank Binns [mailto:frank.binns@xxxxxxxxxx] Sent: Wednesday, February 11, 2015 1:04 AM To: Zhou, Jammy; dri-devel@xxxxxxxxxxxxxxxxxxxxx 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@xxxxxxx> > --- > 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); _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
