Hi, On Sat, Nov 10, 2018 at 01:16:54PM +0200, Laurent Pinchart wrote: > 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 due to the DSS probe being deferred, or at remove time du to the > DISPC being already removed). > > Fixes: edb715dffdee ("drm/omap: dss: dsi: Move initialization code from bind to probe") > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > --- +1 for writing fixes, that cleanup the code at the same time :) Decoupling DISPC from encoders also helps to fix DSI support. Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> -- Sebastian > drivers/gpu/drm/omapdrm/dss/dsi.c | 7 ------- > drivers/gpu/drm/omapdrm/dss/hdmi4.c | 27 --------------------------- > drivers/gpu/drm/omapdrm/dss/hdmi5.c | 27 --------------------------- > drivers/gpu/drm/omapdrm/dss/venc.c | 7 ------- > drivers/gpu/drm/omapdrm/omap_crtc.c | 6 ++++++ > 5 files changed, 6 insertions(+), 68 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index b9d5ad7e67d8..36123c086d97 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -5473,19 +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); > > - 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; > - > - 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 073fa462930a..aabdda394c9c 100644 > --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c > +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c > @@ -841,32 +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); > - > - 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; > - > - 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", }, > {}, > @@ -877,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 b0e4a7463f8c..9e8556f67a29 100644 > --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c > +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c > @@ -825,32 +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); > - > - 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; > - > - 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", }, > @@ -862,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 ff0b18c8e4ac..b5f52727f8b1 100644 > --- a/drivers/gpu/drm/omapdrm/dss/venc.c > +++ b/drivers/gpu/drm/omapdrm/dss/venc.c > @@ -946,19 +946,12 @@ static int venc_runtime_suspend(struct device *dev) > if (venc->tv_dac_clk) > clk_disable_unprepare(venc->tv_dac_clk); > > - 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; > - > - 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, > -- > Regards, > > Laurent Pinchart >
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel