[Intel-gfx] ?==?utf-8?q? [RFC i-g-t 7/9] lib/intel_drm_stubs: Add stubs for functionality from libdrm_intel.

[Emil Velikov]

On Friday, May 20, 2016 23:59 BST, robert.foss at collabora.com wrote: 

> --- /dev/null
> +++ b/lib/intel_drm_stubs.c
Since this and the header file are stubs around intel_bufmgr, it might be better to call them intel_bufmgr_stubs.[ch]. In case "_stubs" in the name brings any confusion one could use _implementation/_internal and/or other.

> --- /dev/null
> +++ b/lib/intel_drm_stubs.h
> @@ -0,0 +1,999 @@
> +#ifndef INTEL_DRM_STUBS_H
> +#define INTEL_DRM_STUBS_H
> +
> +#include <xf86drm.h>
> +
> +#include "igt_core.h"
> +
Please don't. There is nothing here that requires either of these includes. As-is this makes the already convoluted header inclusion worse.

> +#ifdef HAVE_INTEL
> +#include <i915_drm.h>
> +#include <intel_bufmgr.h>
> +#include <drm.h>
> +
Similarly, please drop the drm headers. They are provided by libdrm itself (as opposed to libdrm_intel) and thus they should be available everywhere.

> +
> +#else
> +#define i915_execbuffer2_set_context_id(eb2, context) igt_require(false);
> +#define i915_execbuffer2_get_context_id(eb2) igt_require(false);
> +
Do we have (m)any test that don't any intel_bufmgr functionality and/or don't explicitly require intel drm device ? Afaict those will take precedense over the above two execbuf macros, thus one can omit the above.

For the rest of the patch you seems to be inlining i915_drm.h and intel_bufmgr.h which is a very bad idea imho. As-is it's fragile and hard to maintain.
If we drop the i915_drm.h changes, as suggested above, and copy/import the latter header as local_intel_bufmgr.h things will be better imho. It might be worth adding a note in the releasing document to sync the file with the latest libdrm_intel release.

Regards,
Emil