The internal encoders (DSI, HDMI4, HDMI5 and VENC) runtime PM handlers attempt to manage the runtime PM state of the connected DISPC, based on the rationale that the DISPC providing data to the encoders requires ensuring that the display is active whenever the encoders are active.
While the DISPC provides data to the encoders, it doesn't as such constitute a resource that encoders require in order to be taken out of suspend, contrary to for instance a functional clock or a power supply. Encoders registers can be accessed without the DISPC being active, and while the encoders will not output any video stream without being fed by the DISPC, the DISPC PM state doesn't influence the encoders PM state.
For this reason the DISPC PM state is better managed from the omapdrm driver, in the CRTC enable and disable operations. This allows the encoders PM state to be handled separately from the DISPC, and in particular at times when the DISPC may not be available (for instance at probe or remove time).
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/omapdrm/dss/dsi.c | 21 ---------------- drivers/gpu/drm/omapdrm/dss/hdmi4.c | 38 ----------------------------- drivers/gpu/drm/omapdrm/dss/hdmi5.c | 38 ----------------------------- drivers/gpu/drm/omapdrm/dss/venc.c | 18 -------------- drivers/gpu/drm/omapdrm/omap_crtc.c | 6 +++++ 5 files changed, 6 insertions(+), 115 deletions(-)
This patch applies on top of the "[PATCH v2 0/4] omapdrm: Fix runtime PM issues at module load and unload time" series. It demonstrates what I think is the proper long term fix for the issue addressed by patch 4/4. Due to its nature, I don't think this patch should be applied for v4.20 as it qualifies for very careful testing, hence my proposal to fix the runtime PM problem with 4/4 and to queue this patch for v4.21.
diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c index a1b9e6659b1e..36123c086d97 100644 --- a/drivers/gpu/drm/omapdrm/dss/dsi.c +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c @@ -5473,33 +5473,12 @@ static int dsi_runtime_suspend(struct device *dev) /* wait for current handler to finish before turning the DSI off */ synchronize_irq(dsi->irq);
- /* - * FIXME: DISPC runtime PM handling should be controlled from omapdrm, - * see dsi_runtime_resume(). - */ - if (dsi->dss && dsi->dss->dispc) - dispc_runtime_put(dsi->dss->dispc); - return 0; }
static int dsi_runtime_resume(struct device *dev) { struct dsi_data *dsi = dev_get_drvdata(dev); - int r; - - /* - * FIXME: The device is resumed from the probe function before the dss - * is available, in order to read a hardware configuration register. - * This doesn't require resuming the DISPC, so make it conditional. The - * DISPC runtime PM handling should instead be controlled from omapdrm, - * which is already partly the case, but needs additional testing. - */ - if (dsi->dss && dsi->dss->dispc) { - r = dispc_runtime_get(dsi->dss->dispc); - if (r) - return r; - }
dsi->is_enabled = true; /* ensure the irq handler sees the is_enabled value */ diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c index 21d1147c0827..164501642ed1 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c @@ -841,43 +841,6 @@ static int hdmi4_remove(struct platform_device *pdev) return 0; }
-static int hdmi_runtime_suspend(struct device *dev) -{ - struct omap_hdmi *hdmi = dev_get_drvdata(dev); - - /* - * FIXME: DISPC runtime PM handling should be controlled from omapdrm, - * see dsi_runtime_resume(). - */ - if (hdmi->dss && hdmi->dss->dispc) - dispc_runtime_put(hdmi->dss->dispc); - - return 0; -} - -static int hdmi_runtime_resume(struct device *dev) -{ - struct omap_hdmi *hdmi = dev_get_drvdata(dev); - int r; - - /* - * FIXME: DISPC runtime PM handling should be controlled from omapdrm, - * see dsi_runtime_resume(). - */ - if (hdmi->dss && hdmi->dss->dispc) { - r = dispc_runtime_get(hdmi->dss->dispc); - if (r < 0) - return r; - } - - return 0; -} - -static const struct dev_pm_ops hdmi_pm_ops = { - .runtime_suspend = hdmi_runtime_suspend, - .runtime_resume = hdmi_runtime_resume, -}; - static const struct of_device_id hdmi_of_match[] = { { .compatible = "ti,omap4-hdmi", }, {}, @@ -888,7 +851,6 @@ struct platform_driver omapdss_hdmi4hw_driver = { .remove = hdmi4_remove, .driver = { .name = "omapdss_hdmi", - .pm = &hdmi_pm_ops, .of_match_table = hdmi_of_match, .suppress_bind_attrs = true, }, diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c index 75e2cb74a66c..9e8556f67a29 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c @@ -825,43 +825,6 @@ static int hdmi5_remove(struct platform_device *pdev) return 0; }
-static int hdmi_runtime_suspend(struct device *dev) -{ - struct omap_hdmi *hdmi = dev_get_drvdata(dev); - - /* - * FIXME: DISPC runtime PM handling should be controlled from omapdrm, - * see dsi_runtime_resume(). - */ - if (hdmi->dss && hdmi->dss->dispc) - dispc_runtime_put(hdmi->dss->dispc); - - return 0; -} - -static int hdmi_runtime_resume(struct device *dev) -{ - struct omap_hdmi *hdmi = dev_get_drvdata(dev); - int r; - - /* - * FIXME: DISPC runtime PM handling should be controlled from omapdrm, - * see dsi_runtime_resume(). - */ - if (hdmi->dss && hdmi->dss->dispc) { - r = dispc_runtime_get(hdmi->dss->dispc); - if (r < 0) - return r; - } - - return 0; -} - -static const struct dev_pm_ops hdmi_pm_ops = { - .runtime_suspend = hdmi_runtime_suspend, - .runtime_resume = hdmi_runtime_resume, -}; - static const struct of_device_id hdmi_of_match[] = { { .compatible = "ti,omap5-hdmi", }, { .compatible = "ti,dra7-hdmi", }, @@ -873,7 +836,6 @@ struct platform_driver omapdss_hdmi5hw_driver = { .remove = hdmi5_remove, .driver = { .name = "omapdss_hdmi5", - .pm = &hdmi_pm_ops, .of_match_table = hdmi_of_match, .suppress_bind_attrs = true, }, diff --git a/drivers/gpu/drm/omapdrm/dss/venc.c b/drivers/gpu/drm/omapdrm/dss/venc.c index c39ad81fbc54..b5f52727f8b1 100644 --- a/drivers/gpu/drm/omapdrm/dss/venc.c +++ b/drivers/gpu/drm/omapdrm/dss/venc.c @@ -946,30 +946,12 @@ static int venc_runtime_suspend(struct device *dev) if (venc->tv_dac_clk) clk_disable_unprepare(venc->tv_dac_clk);
- /* - * FIXME: DISPC runtime PM handling should be controlled from omapdrm, - * see dsi_runtime_resume(). - */ - if (venc->dss && venc->dss->dispc) - dispc_runtime_put(venc->dss->dispc); - return 0; }
static int venc_runtime_resume(struct device *dev) { struct venc_device *venc = dev_get_drvdata(dev); - int r; - - /* - * FIXME: DISPC runtime PM handling should be controlled from omapdrm, - * see dsi_runtime_resume(). - */ - if (venc->dss && venc->dss->dispc) { - r = dispc_runtime_get(venc->dss->dispc); - if (r < 0) - return r; - }
if (venc->tv_dac_clk) clk_prepare_enable(venc->tv_dac_clk); diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 62928ec0e7db..caffc547ef97 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -350,11 +350,14 @@ static void omap_crtc_arm_event(struct drm_crtc *crtc) static void omap_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { + struct omap_drm_private *priv = crtc->dev->dev_private; struct omap_crtc *omap_crtc = to_omap_crtc(crtc); int ret;
DBG("%s", omap_crtc->name);
+ priv->dispc_ops->runtime_get(priv->dispc); + spin_lock_irq(&crtc->dev->event_lock); drm_crtc_vblank_on(crtc); ret = drm_crtc_vblank_get(crtc); @@ -367,6 +370,7 @@ static void omap_crtc_atomic_enable(struct drm_crtc *crtc, static void omap_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { + struct omap_drm_private *priv = crtc->dev->dev_private; struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
DBG("%s", omap_crtc->name); @@ -379,6 +383,8 @@ static void omap_crtc_atomic_disable(struct drm_crtc *crtc, spin_unlock_irq(&crtc->dev->event_lock);
drm_crtc_vblank_off(crtc); + + priv->dispc_ops->runtime_put(priv->dispc); }
static enum drm_mode_status omap_crtc_mode_valid(struct drm_crtc *crtc,