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

[Chris Wilson]

On Thu, Jul 28, 2016 at 03:42:19PM +0530, Goel, Akash wrote:
> 
> 
> On 7/28/2016 12:34 PM, Chris Wilson wrote:
> >On Thu, Jul 28, 2016 at 12:21:26PM +0530, Goel, Akash wrote:
> >>
> >>
> >>On 7/28/2016 12:04 PM, Chris Wilson wrote:
> >>>>Can the RING_FAULT_REG macro be re-defined, so that we can avoid
> >>>>using the for_each_engine() in this case, as per the below snippet.
> >>>
> >>>because we may then iterate over undefined registers (presuming the bank
> >>>may not be wired up for say VECS2 on IVB).
> >>
> >>Probably for that HAS_ENGINE macro could have helped,
> >
> >HAS_ENGINE() is a tricky one. It's value may change across
> >intel_engines_init() - I would much rather hide it from the user and
> >keep it private to intel_engine_cs.c. Everyone else then just tests
> >whether the engine exists in the dev_priv.
> 
> Went through these 2 patches,
> drm/i915: Clear per-engine fault register as early as possible
> drm/i915: Fix use of engine->index for register offset
> 
> But even after these patches, won't the following change be still
> needed in this patch ?
> 
> -    POSTING_READ(RING_FAULT_REG(&dev_priv->engine[RCS]));
> +
> +    /* Engine specific init may not have been done till this point. */
> +    if (dev_priv->engine[RCS])
> +        POSTING_READ(RING_FAULT_REG(dev_priv->engine[RCS]));
> }

Yes, or drop the POSTING_READ.

I was just responding to that this highlighted some broken assumptions
in the code.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre