Hi, On Fri, Sep 1, 2023 at 1:15 AM Maxime Ripard <mripard@xxxxxxxxxx> wrote: > > On Thu, Aug 31, 2023 at 11:18:49AM -0700, Doug Anderson wrote: > > Hi, > > > > On Thu, Aug 31, 2023 at 12:38 AM Maxime Ripard <mripard@xxxxxxxxxx> wrote: > > > > > > If so, then I think we can implement everything by doing something like: > > > > > > - Implement enable and disable refcounting in panels. > > > drm_panel_prepare and drm_panel_enable would increase it, > > > drm_panel_unprepare and drm_panel_disable would decrease it. > > > > > > - Only actually call the disable and unprepare functions when that > > > refcount goes to 0 in the normal case. > > > > Just to be clear: by refcounting do you mean switching this to actual > > refcount? > > Yes > > > Today this is explicitly _not_ refcounting, right? It is simply > > treating double-enables as no-ops and double-disables as no-ops. With > > our current understanding, the only thing we actually need to guard > > against is double-disable but at the moment we do guard against both. > > Specifically we believe the cases that are issues: > > > > a) At shutdown/remove time we want to disable the panel, but only if > > it was enabled (we wouldn't want to call disable if the panel was > > already off because userspace turned it off). > > Yeah, and that's doable with refcounting too. I don't understand the benefit of switching to refcounting, though. We don't ever expect the "prepare" or "enable" function to be called more than once and all we're guarding against is a double-unprepare and a double-enable. Switching this to refcounting would make the reader think that there was a legitimate case for things to be prepared or enabled twice. As far as I know, there isn't. In any case, I don't think there's any need to switch this to refcounting as part of this effort. Someone could, in theory, do it as a separate patch series. > > The above solves the problem with panels wanting to power sequence > > themselves at remove() time, but not at shutdown() time. Thus we'd > > still have a dependency on having all drivers use > > drm_atomic_helper_shutdown() so that work becomes a dependency. > > Does it? I think it can be done in parallel? I don't think it can be in parallel. While it makes sense for panels to call drm_panel_remove() at remove time, it doesn't make sense for them to call it at shutdown time. That means that the trick of having the panel get powered off in drm_panel_remove() won't help for shutdown. For shutdown, which IMO is the more important case, we need to wait until all drm drivers call drm_atomic_helper_shutdown() properly. -Doug
