[Intel-gfx] [PATCH v2] drm/i915/execlists: Refactor common engine setup

[Daniel Vetter]

On Mon, May 02, 2016 at 11:58:38AM +0100, Chris Wilson wrote:
> On Mon, May 02, 2016 at 10:51:31AM +0200, Daniel Vetter wrote:
> > Imo the low-level irq clearing should all be done in the relevant irq
> > setup code in i915_irq.c. Atm we just forgot to do that. I guess you can
> > have a bikeshed whether the enginer IMR enable/disable functions should be
> > together with the clearing or not, placing them in either file is fine.
> > But since we already clear the higher-level IMR registers in i915_irq.c we
> > might as well clear the low-level ones too in i915_irq.c.
> 
> That's tricky since that is done before the engines - so how do we get
> the various bases to i915_irq.c without duplication? Enabling the irq
> for the engines is part of init_hw, so correspondingly putting the
> early disable into init looks fine to me.

I just don't particularly like that we have hw access in a function that
thus far (and at a glance still after this patch) only sets up software
state. I'll probably lead to some ugly surprise. That's why I'd like to
move this either to engine->init_hw or to i915_irq.c.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch