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

[Luca Coelho]

On Thu, 2024-11-07 at 17:47 -0300, Gustavo Sousa wrote:
> Quoting Gustavo Sousa (2024-11-07 17:14:36-03:00)
> > Quoting Luca Coelho (2024-11-07 16:23:06-03:00)
> > 
> > > > 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?
> > 
> > The initialization is done only once, during driver load. The wakelock
> > will be enabled only at a later moment. So, we are good in that regard.
> > 
> > However, now that you mentioned, yeah, we should also consider that that
> > we do concurrent work during initialization (e.g. loading the DMC).
> > Based on that, we will need to protect "initialized", which means:
> > 
> > - initializing the lock early together with the other ones;
> > - always going for the lock, even for hardware that does not support the
> >  wakelock.
> 
> Well, a hacky way to mitigate this is by checking the DISPLAY_VER() >=
> 20 before taking the spin lock, since that info is queried in
> probe_gmdid_display(), which happens at the "no-mmio" phase of driver
> initialization.
> 
> By the way, that makes me think: is it too bad to do the same kind of
> early MMIO via pci_iomap_range() for ICL_DFSM_DMC_DISABLE? We could
> avoid this whole thing, since we would already have the correct value
> for HAS_DMC() when i915/xe MMIO functions are called.

I'm not sure it's worth it, but if you feel this would be better, go
ahead and show us the code. :)

--
Cheers,
Luca.