Hi,
On 07/15/2014 11:33 AM, Keith Packard wrote:
Hans de Goede <hdego...@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
all.
True.
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.
I think defining 2 inlines for the 2 getter functions is a good idea.
Can you resend the patch as a proper top-level mail with the inlines
added? Then I'll review it.
I guess we should probably wait a bit with applying this to give others a
chance to respond.
Regards,
Hans
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
