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
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to