[PATCH v7] drm/i915: Allocate intel_engine_cs structure only for the enabled engines

Chris Wilson chris at chris-wilson.co.uk
Tue Aug 9 13:31:14 UTC 2016


On Tue, Aug 09, 2016 at 02:17:46PM +0100, Tvrtko Ursulin wrote:
> 
> On 05/08/16 13:40, akash.goel at intel.com wrote:
> >From: Akash Goel <akash.goel at intel.com>
> >@@ -1753,7 +1753,7 @@ struct drm_i915_private {
> >
> >  	struct pci_dev *bridge_dev;
> >  	struct i915_gem_context *kernel_context;
> >-	struct intel_engine_cs engine[I915_NUM_ENGINES];
> >+	struct intel_engine_cs *engine[I915_NUM_ENGINES];
> 
> It is a little bit of a pity since iterator removal was a nice code
> shrink IIRC. Nevermind..

Yes, I asked Akash to check size i915.ko and he reported that it shrinks
again. Akash, please do include that finding in the changelog!

> What I wanted to say is, would it be feasible to have, in addition
> to the above:
> 
> 	unsigned int num_engines;
> 	struct intel_engine_cs **engine_list;
> 
> dev_priv->engine[] would be a direct map of intel_engine_id to
> engine for lookups, while the iterators would use dynamically sized
> (no holes) dev_priv->engine_list.

I actually had in mind allocating the block as a single array, with the
auxiliary id-to-pointer lookup table.
 
> On the other hand there probably isn't a lot of for_each_engines on
> fast paths so it may not be that interesting optimisation.

My feelings likewise. If someone wants to try and demonstrate that it is
a win under some scenarios, be my guest.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


More information about the Intel-gfx-trybot mailing list