Re: [PATCH 02/19] drm/i915/execlists: Refactor common engine setup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux