[Intel-gfx] [PATCH] drm/i915: Decouple execbuf uAPI from internal implementation

[Chris Wilson]

On Fri, Jan 15, 2016 at 10:29:05AM +0000, Tvrtko Ursulin wrote:
> >One question, how does this look if we move this section to its own
> >function:
> >
> >ring = eb_select_ring(dev_priv, file, args);
> >if (IS_ERR(ring))
> >	return PTR_ERR(ring);
> >
> >Do you keep your code size reduction?
> 
> No, perhaps because of all the ERR_PTR business. But decoupling the
> ring and return value keeps it:
> 
> static int
> eb_select_ring(struct drm_i915_private *dev_priv,
> 	       struct drm_file *file,
> 	       struct drm_i915_gem_execbuffer2 *args,
> 	       struct intel_engine_cs **ring)

Hmm, I wonder why. ERR_PTR(ret) should be a no-op, so the only thing
that I would have thought changed would be the test afterwards. I guess
a foray into the assembly would be a good learning experience for me then!

> Saves ~100 bytes out of ~4600 on my build.
> 
> >>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>index 7349d9258191..fdc220fc2b18 100644
> >>--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>@@ -148,14 +148,14 @@ struct  i915_ctx_workarounds {
> >>  struct  intel_engine_cs {
> >>  	const char	*name;
> >>  	enum intel_ring_id {
> >>-		RCS = 0x0,
> >>-		VCS,
> >>+		RCS = 0,
> >>  		BCS,
> >>-		VECS,
> >>-		VCS2
> >>+		VCS,
> >>+		VCS2,	/* Keep instances of the same type engine together. */
> >>+		VECS
> >
> >Technically, this breaks userspace ABI elsewhere. :| Though the only
> >user of the id mask is only looking for RCS==0 vs the reset.
> >
> >So I think we would cope, but to be extra safe we could just avoid
> >reshuffling the ids. Let me sleep on the implications. We may say just
> >break that ABI whilst we still can and do a reverse-map to EXEC bit.
> >Hmm.
> 
> What are the other places it breaks the ABI? I'd like to understand it.

The busy-ioctl is strong abi, i915_trace.h has a few weak abi uses of
ring->id, and the use in debugfs is no abi barrier at all.

ring->id also crops up in the guc_context_desc, which is intriguing.
-Cris

-- 
Chris Wilson, Intel Open Source Technology Centre