[PATCH v3 17/18] drm/i915/dmc_wl: Do nothing until initialized

[Luca Coelho]

On Thu, 2024-11-07 at 15:27 -0300, Gustavo Sousa wrote:
> There is a bit of a chicken and egg situation where we depend on runtime
> info to know that DMC and wakelock are supported by the hardware, and
> such information is grabbed via display MMIO functions, which in turns
> call intel_dmc_wl_get() and intel_dmc_wl_put() as part of their regular
> flow.

s/which in turns call/which in turn calls/


> Since we do not expect DC states (and consequently the wakelock
> mechanism) to be enabled until DMC and DMC wakelock software structures
> are initialized, a simple and safe solution to this is to turn
> intel_dmc_wl_get() and intel_dmc_wl_put() into no-op until we have
> properly initialized.


About "safe" here... Can we be sure this will be race-free?


> Let's implement that via a new field "initialized". Not that, since we
> expect __intel_dmc_wl_supported() to be used for most non-static DMC
> wakelock functions, let's add a drm_WARN_ONCE() there for when it is
> called prior to initialization.


s/not that/note that/


> The only exception of functions that can be called before initialization
> are intel_dmc_wl_get() and intel_dmc_wl_put(), so we bail before
> calling __intel_dmc_wl_supported() if not initialized.
> 
> An alternative solution would be to revise MMIO-related stuff in the
> whole driver initialization sequence, but that would possibly come with
> the cost of some added ordering dependencies and complexity to the
> source code.

I think this can be kept out of the commit message.  It's not very
clear what you mean and it sounds much more complex than the solution
you implemented.  Unless race can really be an issue here, but then the
whole commit message should be changed to an eventual more complex
solution.

--
Cheers,
Luca.