[Intel-gfx] [PATCH 1/3] igt/gem_stolen: Verifying extended gem_create ioctl

[Chris Wilson]

On Fri, Jul 03, 2015 at 10:05:05AM +0100, Tvrtko Ursulin wrote:
> >+bool gem_create__has_stolen_support(int fd)
> >+{
> >+	static int has_stolen_support = -1;
> >+
> >+	if (has_stolen_support < 0) {
> >+	struct drm_i915_getparam gp;
> >+	int val = -1;

> >+		memset(&gp, 0, sizeof(gp));
> >+		gp.param = 36; /* CREATE_VERSION */
> >+		gp.value = &val;
> >+
> >+		/* Do we have the extended gem_create_ioctl? */
> >+		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> >+		has_stolen_support = val >= 1;
> 
> If ioctl fails it will declare stolen support. (val remains -1)
> 
> I would also suggest "has_stolen_support = val > 1" as clearer.

Tvrtko, -ENOCOFFEE? :)

> >+	struct local_i915_gem_create_v2 create;
> >+	int ret;
> >+
> >+	memset(&create, 0, sizeof(create));
> >+	create.handle = 0;
> >+	create.size = size;
> >+	create.flags = I915_CREATE_PLACEMENT_STOLEN;
> >+	ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_CREATE, &create);
> >+
> >+	if (ret < 0)
> >+		return 0;
> >+
> >+	errno = 0;
> >+	return create.handle;
> >+}
> >+
> >+
> 
> Interestingly gem_create_stolen copies implementation from
> __gem_create and differs from normal gem_create in handling of ioctl
> failure. But it removes the double underscore from the name. I think
> this creates unnecessary inconsistency. It should either have double
> underscore prefix or copy implementation from gem_create.

Right, I have been using gem_(*) for abort-on-failure, and __gem_*() for
propagate the error back to the caller. I would like that idiom become
more common.
-Chris


-- 
Chris Wilson, Intel Open Source Technology Centre