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

[Tvrtko Ursulin]

On 05/05/16 12:17, Tvrtko Ursulin wrote:
>
> 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. ;)

On the other hand, what I called pure refactoring is also adding the 
race window to other engines. So don't know.. engine->initialized? :) It 
doesn't look like anything in setup depends on it being true.

Regards,

Tvrtko