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

[Tvrtko Ursulin]

On 05/05/16 11:33, Chris Wilson wrote:
> On Thu, May 05, 2016 at 11:18:41AM +0100, Tvrtko Ursulin wrote:
>>
>> Hi,
>>
>> On 05/05/16 10:15, Chris Wilson wrote:
>>> Move all of the constant assignments up front and into a common
>>> function. This is primarily to ensure the backpointers are set as early
>>> as possible for later use during initialisation.
>>>
>>> v2: Use a constant struct so that all the similar values are set
>>> together.
>>> v3: Sanitize the engine's IMR to disable any potential interrupt before
>>> we are ready (enabled in init_hw).
>>
>> Same as before - I don't like hardware access in this code path
>> since we otherwise have it split into a later init_hw phase. And I
>> don't like engine->dev being used for intel_engine_initialized.
>
> I think you raised a good point on the last round! It is an oversight
> that we have not explicitly sanitized the per-engine registers as is our
> mo. This gives us the symmetry with the init_hw phase where they are
> enabled.
>
>> On retrospect, interrupt vs engine->irq_queue race is already there
>> now, for the render ring at least. So maybe just drop the IMR bit
>> which would make the patch pure refactoring and can have my R-b
>> then.
>
> And this closes a race with a potential interrupt pending from takeover.

Okay but whether or not I have raised a good point I think it wouldn't 
harm to split the pure refactoring from functional changes. You get at 
least one R-b like that. ;)

Regards,

Tvrtko