[PATCH v2 0/2] Fix ODEV_ATTRIB_DRIVER overlapping with ODEV_ATTRIB_FD

Keith Packard keithp at keithp.com
Tue Jul 15 02:33:05 PDT 2014

Hans de Goede <hdegoede at redhat.com> writes:

> 1) I was not around when the OdevAttributes stuff got added, but I
> think the idea behind it was to be able to add new atrributes without
> breaking ABI (as was done this cycle when adding the driver string,
> although that needed a bit of a fixup).

The server ABI changes at every single X server release, so ABI
compatibility of this interface isn't going to help driver authors at

> 2) It will require some ugly #ifdef-ery in almost all the drivers to work
> with both the old and the new way.

There are 18 references to ODEV_ in all of the open source drivers
(master as of this morning):

./xf86-video-freedreno/src/msm-driver.c:		fd = xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_FD, -1);
./xf86-video-opentegra/src/driver.c:                                               ODEV_ATTRIB_PATH);
./xf86-video-opentegra/src/driver.c:    char *path = xf86_get_platform_device_attrib(dev, ODEV_ATTRIB_PATH);
./xf86-video-modesetting/src/driver.c:        fd = xf86_get_platform_device_int_attrib(platform_dev, ODEV_ATTRIB_FD, -1);
./xf86-video-modesetting/src/driver.c:    const char *path = xf86_get_platform_device_attrib(dev, ODEV_ATTRIB_PATH);
./xf86-video-modesetting/src/driver.c:            ms->fd = xf86_get_platform_device_int_attrib(pEnt->location.id.plat, ODEV_ATTRIB_FD, -1);
./xf86-video-modesetting/src/driver.c:            char *path = xf86_get_platform_device_attrib(pEnt->location.id.plat, ODEV_ATTRIB_PATH);
./xf86-video-intel/src/intel_device.c:#if defined(ODEV_ATTRIB_PATH)
./xf86-video-intel/src/intel_device.c:	path = xf86_get_platform_device_attrib(dev, ODEV_ATTRIB_PATH);
./xf86-video-intel/src/intel_device.c:#if defined(ODEV_ATTRIB_FD)
./xf86-video-intel/src/intel_device.c:	return xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_FD, -1);
./xf86-video-omap/src/omap_driver.c:				ODEV_ATTRIB_BUSID);
./xf86-video-omap/src/omap_driver.c:	char *busid = xf86_get_platform_device_attrib(dev, ODEV_ATTRIB_BUSID);
./xf86-video-ati/src/radeon_kms.c:                                                 ODEV_ATTRIB_FD, -1);
./xf86-video-vmware/vmwgfx/vmwgfx_driver.c:#ifdef ODEV_ATTRIB_FD
./xf86-video-vmware/vmwgfx/vmwgfx_driver.c:	                                                 ODEV_ATTRIB_FD, -1);
./xf86-video-qxl/src/qxl_kms.c:#if defined(ODEV_ATTRIB_FD)
./xf86-video-qxl/src/qxl_kms.c:                                                          ODEV_ATTRIB_FD, -1);

We could either just have #ifdefs in these few places, or if we were
really lazy, we could provide inline functions for the two getter
functions used here and still net an overall savings in code.

keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 810 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20140715/57955a92/attachment.sig>

More information about the xorg-devel mailing list