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

[Chris Wilson]

On Thu, Aug 04, 2016 at 08:55:18PM +0530, akash.goel at intel.com wrote:
> From: Akash Goel <akash.goel at intel.com>
> 
> With the possibility of addition of many more number of rings in future,
> the drm_i915_private structure could bloat as an array, of type
> intel_engine_cs, is embedded inside it.
> 	struct intel_engine_cs engine[I915_NUM_ENGINES];
> Though this is still fine as generally there is only a single instance of
> drm_i915_private structure used, but not all of the possible rings would be
> enabled or active on most of the platforms. Some memory can be saved by
> allocating intel_engine_cs structure only for the enabled/active engines.
> Currently the engine/ring ID is kept static and dev_priv->engine[] is simply
> indexed using the enums defined in intel_engine_id.
> To save memory and continue using the static engine/ring IDs, 'engine' is
> defined as an array of pointers.
> 	struct intel_engine_cs *engine[I915_NUM_ENGINES];
> dev_priv->engine[engine_ID] will be NULL for disabled engine instances.
> 
> v2:
> - Remove the engine iterator field added in drm_i915_private structure,
>   instead pass a local iterator variable to the for_each_engine**
>   macros. (Chris)
> - Do away with intel_engine_initialized() and instead directly use the
>   NULL pointer check on engine pointer. (Chris)
> 
> v3:
> - Remove for_each_engine_id() macro, as the updated macro for_each_engine()
>   can be used in place of it. (Chris)
> - Protect the access to Render engine Fault register with a NULL check, as
>   engine specific init is done later in Driver load sequence.
> 
> v4:
> - Use !!dev_priv->engine[VCS] style for the engine check in getparam. (Chris)
> - Kill the superfluous init_engine_lists().
> 
> v5:
> - Cleanup the intel_engines_init() & intel_engines_setup(), with respect to
>   allocation of intel_engine_cs structure. (Chris)
> 
> v6:
> - Rebase.

Already out of date, sorry.

> @@ -394,10 +394,11 @@ static void print_batch_pool_stats(struct seq_file *m,
>  	struct file_stats stats;
>  	struct intel_engine_cs *engine;
>  	int j;
> +	u32 iter;

Can you please use s/u32 iter/unsigned int iter/ ? I don't think there
is a good reason to be specific about the integer size. And in doing so,
keep the locals looking neat and not too much like a disjointed stair
case.

>  /* Iterator over subset of engines selected by mask */
> -#define for_each_engine_masked(engine__, dev_priv__, mask__) \
> -	for ((engine__) = &(dev_priv__)->engine[0]; \
> -	     (engine__) < &(dev_priv__)->engine[I915_NUM_ENGINES]; \
> -	     (engine__)++) \
> -		for_each_if (((mask__) & intel_engine_flag(engine__)) && \
> -			     intel_engine_initialized(engine__))
> +#define for_each_engine_masked(engine__, dev_priv__, mask__, iter__) \
> +	for ((iter__) = 0; \
> +	     ((iter__) < I915_NUM_ENGINES) && \
> +	     ((engine__) = (dev_priv__)->engine[(iter__)], true); \
> +	     (iter__)++) \
> +		for_each_if ((engine__) && ((mask__) & intel_engine_flag(engine__)))

Having restored the iterator, you can use mask__ & BIT(iter__) instead
of intel_engine_flag()
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre