[PATCH 0/3] omapdrm: Fix runtime PM issues at module load and unload time

[Tomi Valkeinen]

Hi Laurent,

On 01/11/18 12:25, Laurent Pinchart wrote:
> Hello,
> 
> This series fixes crashes in the omapdss driver at both load and unload
> time, due to runtime PM problems related to probe deferral. The bugs got
> introduced in v4.20-rc, this should thus be considered as v4.20 fixes.
> 
> At the core of the problem comes commit 27d624527d99 ("drm/omap: dss:
> Acquire next dssdev at probe time") which can cause probe deferral for
> the DSS when the encoder and panel modules are not loaded yet. As the
> omapdss module contains drivers for the DSS and all its children, this
> results in the internal encoders being probed before the DSS. Missing
> runtime PM handling around register access then caused imprecise
> external aborts.
> 
> Patch 1/3 moves population of the DSS children from arch code to the
> omapdss driver. This prevents the DSS children from being probed before
> the DSS. The change can be considered as a workaround in the sense that
> runtime PM of the DSS children should operate correctly even when the
> DSS probe fail. However, given that reducing the amount of arch code is
> an improvement in itself, I believe the solution to be acceptable.
> 
> Patches 2/3 and 3/3 then ensure that the HDMI4 and DSI devices are
> active at bind and probe time respectively, in order to access hardware
> registers there.
> 
> Tony, patch 1/3 touches both the mach-omap2 and omapdss. Would you be
> fine merging it through the DRM tree ? I don't think there's a risk of
> conflict during the v4.20-rc cycle.

Thanks for debugging this! I have to say I really don't like these
(well, 2 is fine), as they feel like hacks.

We do dispc_runtime_get/put in the HDMI driver's suspend/resume too, so
don't we need similar hack (as you add in dsi.c) there also?

So we have two problems (aside missing runtime_get/put calls in those
few places):

- All DSS submodules depend on DSS (dss core)

- All DSS encoders depend on DISPC. The dependency is there only when
we're starting the video stream.

Can we not defer the probe of the submodules until the dependencies have
been probed? Are there any other ways to manage such device
dependencies? Possibly we could handle the driver registration so that
we only register encoders when dss and dispc have been probed, but that
feels a bit hacky too.

And considering these issues, is the assumption that "There's no reason
to delay initialization of most of the driver (such as mapping memory
I/O or enabling runtime PM) to the component bind handler." still valid?
 edb715dffdee71bb8216ee4d71c0714d932e9acf doesn't really state any
benefit for this move either. Clearly there's a reason to do this in
bind, although it doesn't solve the dispc dependency.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki