On 20/07/16 18:31, Chris Wilson wrote:
On Wed, Jul 20, 2016 at 06:16:07PM +0100, Dave Gordon wrote:
'ring' is an old deprecated term for a GPU engine, so we're trying to
phase out all such terminology. eb_select_ring() not only has 'ring'
(meaning engine) in its name, but it has an ugly calling convention
whereby it returns an errno and stores a pointer-to-engine indirectly
through an output parameter. As there is only one error it ever returns
(-EINVAL), we can make it return the pointer directly, and have the
caller pass back the error code -EINVAL if the pointer result is NULL.

Thus we can replace
-       ret = eb_select_ring(dev_priv, file, args, &engine);
-       if (ret)
-               return ret;
with
+       engine = eb_select_engine(dev_priv, file, args);
+       if (!engine)
+               return -EINVAL;
for increased clarity and maybe save a few cycles too.

Signed-off-by: Dave Gordon <david.s.gor...@intel.com>

Thanks. I feel foolish for missing that easy transform before.
Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk>

There's a bsd_ring buried beneath here as well...

The ugly calling convention was my doing in:

  commit de1add360522c876c25ef2bbbbab1c94bdb509ab
  Author: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
  Date:   Fri Jan 15 15:12:50 2016 +0000

      drm/i915: Decouple execbuf uAPI from internal implementation

And the reason for it was to avoid growing the text size of i915_gem_do_execbuffer while at the same time extracting all the engine selection logic into a separate function. For some reason GCC most liked it like that. Or maybe I was only trying the ERR_PTR route, not the NULL/ptr return. Don't remember now.

Anyway, I don't mind, just providing reasoning for the "ugly" calling convention.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to