| Message ID | 20210517130450.v7.1.I9e947183e95c9bd067c9c1d51208ac6a96385139@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm: Fix EDID reading on ti-sn65dsi86 by introducing the DP AUX bus | expand |
Hi Doug, Thank you for the patch. On Mon, May 17, 2021 at 01:08:58PM -0700, Douglas Anderson wrote: > The PM Runtime docs specifically call out the need to call > pm_runtime_dont_use_autosuspend() in the remove() callback if > pm_runtime_use_autosuspend() was called in probe(): > > > Drivers in ->remove() callback should undo the runtime PM changes done > > in ->probe(). Usually this means calling pm_runtime_disable(), > > pm_runtime_dont_use_autosuspend() etc. ~/src/kernel/linux $ git grep pm_runtime_use_autosuspend -- drivers | wc -l 209 ~/src/kernel/linux $ git grep pm_runtime_dont_use_autosuspend -- drivers | wc -l 80 Seems like a lost battle :-( The fix is right, but I wonder if this shouldn't be handled automatically by the runtime PM core. The runtime PM API is notoriously difficult to use correctly. > We should do this. This fixes a warning splat that I saw when I was > testing out the panel-simple's remove(). > > Fixes: 3235b0f20a0a ("drm/panel: panel-simple: Use runtime pm to avoid excessive unprepare / prepare") > Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > > Changes in v7: > - pm_runtime_dont_use_autosuspend() fix new for v7. > > drivers/gpu/drm/panel/panel-simple.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index 9be050ab372f..21939d4352cf 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -798,6 +798,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) > return 0; > > disable_pm_runtime: > + pm_runtime_dont_use_autosuspend(dev); > pm_runtime_disable(dev); > free_ddc: > if (panel->ddc) > @@ -814,6 +815,7 @@ static int panel_simple_remove(struct device *dev) > drm_panel_disable(&panel->base); > drm_panel_unprepare(&panel->base); > > + pm_runtime_dont_use_autosuspend(dev); > pm_runtime_disable(dev); > if (panel->ddc) > put_device(&panel->ddc->dev);
Hi, On Mon, May 24, 2021 at 1:22 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Doug, > > Thank you for the patch. > > On Mon, May 17, 2021 at 01:08:58PM -0700, Douglas Anderson wrote: > > The PM Runtime docs specifically call out the need to call > > pm_runtime_dont_use_autosuspend() in the remove() callback if > > pm_runtime_use_autosuspend() was called in probe(): > > > > > Drivers in ->remove() callback should undo the runtime PM changes done > > > in ->probe(). Usually this means calling pm_runtime_disable(), > > > pm_runtime_dont_use_autosuspend() etc. > > ~/src/kernel/linux $ git grep pm_runtime_use_autosuspend -- drivers | wc -l > 209 > ~/src/kernel/linux $ git grep pm_runtime_dont_use_autosuspend -- drivers | wc -l > 80 > > Seems like a lost battle :-( > > The fix is right, but I wonder if this shouldn't be handled > automatically by the runtime PM core. The runtime PM API is notoriously > difficult to use correctly. No kidding. > > We should do this. This fixes a warning splat that I saw when I was > > testing out the panel-simple's remove(). > > > > Fixes: 3235b0f20a0a ("drm/panel: panel-simple: Use runtime pm to avoid excessive unprepare / prepare") > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks! I have pushed just this patch for now. -Doug
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 9be050ab372f..21939d4352cf 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -798,6 +798,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc) return 0; disable_pm_runtime: + pm_runtime_dont_use_autosuspend(dev); pm_runtime_disable(dev); free_ddc: if (panel->ddc) @@ -814,6 +815,7 @@ static int panel_simple_remove(struct device *dev) drm_panel_disable(&panel->base); drm_panel_unprepare(&panel->base); + pm_runtime_dont_use_autosuspend(dev); pm_runtime_disable(dev); if (panel->ddc) put_device(&panel->ddc->dev);