[Intel-gfx] [PATCH v2 4/4] drm/i915:bxt: Enable Pooled EU support

[Chris Wilson]

On Fri, Jul 17, 2015 at 07:13:34PM +0100, Arun Siluvery wrote:
> This mode allows to assign EUs to pools which can process work collectively.
> The command to enable this mode should be issued as part of context initialization.
> 
> The pooled mode is global, once enabled it has to stay the same across all
> contexts until HW reset hence this is sent in auxiliary golden context batch.
> Thanks to Mika for the preliminary review and comments.
> 
> v2: explain why this is enabled in golden context, use feature flag while
> enabling the support (Chris)

You fell into the trap of telling userspace this was setup before we
actually do so.
 
> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Armin Reese <armin.c.reese at intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_render_state.c | 15 +++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h              |  2 ++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
> index 5026a62..e4ff342 100644
> --- a/drivers/gpu/drm/i915/i915_gem_render_state.c
> +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
> @@ -93,6 +93,7 @@ free_gem:
>  
>  static int render_state_setup(struct render_state *so)
>  {
> +	struct drm_device *dev = so->obj->base.dev;
>  	const struct intel_renderstate_rodata *rodata = so->rodata;
>  	unsigned int i = 0, reloc_index = 0;
>  	struct page *page;
> @@ -134,6 +135,20 @@ static int render_state_setup(struct render_state *so)
>  
>  	so->aux_batch_offset = i * sizeof(u32);
>  
> +	if (HAS_POOLED_EU(dev)) {
> +		u32 pool_config = 0;
> +		struct drm_i915_private *dev_priv = to_i915(dev);

Just a minor, as this would be neater as

		u32 pool_config =
			INTEL_INFO(dev)->subslice_total == 3 ? 0x00777000 : 0;

At the very least keep both paths to set pool_config next to each other,
e.g.
u32 pool_config;
...
pool_config = 0;
if (INTEL_INFO(dev)->subslice_total == 3)
	pool_config = 0x00777000;

Then we just have

> +		OUT_BATCH(d, i, GEN9_MEDIA_POOL_STATE);
> +		OUT_BATCH(d, i, GEN9_MEDIA_POOL_ENABLE);
> +		OUT_BATCH(d, i, pool_config);
> +		OUT_BATCH(d, i, 0);
> +		OUT_BATCH(d, i, 0);
> +		OUT_BATCH(d, i, 0);

Which is much easier to read.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre