| Message ID | 20211215175910.1744151-1-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm: Don't use autosuspend for display | expand |
Quoting Rob Clark (2021-12-15 09:59:02) > From: Rob Clark <robdclark@chromium.org> > > No functional change, as we only actually enable autosuspend for the GPU > device. But lets not encourage thinking that autosuspend is a good idea > for anything display related. I'd prefer to see a small blurb about why it's not a good idea to use autosuspend for display things. Then this commit can be dug out of the history and someone new can quickly understand the reasoning behind it. Just saying it's not a good idea doesn't really help. > > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
On Wed, 15 Dec 2021 at 22:10, Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Rob Clark (2021-12-15 09:59:02) > > From: Rob Clark <robdclark@chromium.org> > > > > No functional change, as we only actually enable autosuspend for the GPU > > device. But lets not encourage thinking that autosuspend is a good idea > > for anything display related. > > I'd prefer to see a small blurb about why it's not a good idea to use > autosuspend for display things. Then this commit can be dug out of the > history and someone new can quickly understand the reasoning behind it. > Just saying it's not a good idea doesn't really help. I'd second this request, Nevertheless Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > > Reviewed-by: Stephen Boyd <swboyd@chromium.org>
On Wed, Dec 15, 2021 at 11:10 AM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Rob Clark (2021-12-15 09:59:02) > > From: Rob Clark <robdclark@chromium.org> > > > > No functional change, as we only actually enable autosuspend for the GPU > > device. But lets not encourage thinking that autosuspend is a good idea > > for anything display related. > > I'd prefer to see a small blurb about why it's not a good idea to use > autosuspend for display things. Then this commit can be dug out of the > history and someone new can quickly understand the reasoning behind it. > Just saying it's not a good idea doesn't really help. The issue is that we have multiple different devices at play, and potentially specific requirements about power sequencing when lighting up or shutting down the display.. autosuspend would just turn that into a giant race condition. I'll squash something about this into the commit msg BR, -R > > > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > > Reviewed-by: Stephen Boyd <swboyd@chromium.org>
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 5b4bb722f750..6b3ced4aaaf5 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -2020,7 +2020,7 @@ void msm_dsi_host_xfer_restore(struct mipi_dsi_host *host, /* TODO: unvote for bus bandwidth */ cfg_hnd->ops->link_clk_disable(msm_host); - pm_runtime_put_autosuspend(&msm_host->pdev->dev); + pm_runtime_put(&msm_host->pdev->dev); } int msm_dsi_host_cmd_tx(struct mipi_dsi_host *host, @@ -2252,7 +2252,7 @@ int msm_dsi_host_enable(struct mipi_dsi_host *host) */ /* if (msm_panel->mode == MSM_DSI_CMD_MODE) { * dsi_link_clk_disable(msm_host); - * pm_runtime_put_autosuspend(&msm_host->pdev->dev); + * pm_runtime_put(&msm_host->pdev->dev); * } */ msm_host->enabled = true; @@ -2344,7 +2344,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host, fail_disable_clk: cfg_hnd->ops->link_clk_disable(msm_host); - pm_runtime_put_autosuspend(&msm_host->pdev->dev); + pm_runtime_put(&msm_host->pdev->dev); fail_disable_reg: dsi_host_regulator_disable(msm_host); unlock_ret: @@ -2371,7 +2371,7 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host) pinctrl_pm_select_sleep_state(&msm_host->pdev->dev); cfg_hnd->ops->link_clk_disable(msm_host); - pm_runtime_put_autosuspend(&msm_host->pdev->dev); + pm_runtime_put(&msm_host->pdev->dev); dsi_host_regulator_disable(msm_host); diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c index 0b2ae5c15240..c2ed177717c7 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c @@ -602,7 +602,7 @@ static int dsi_phy_enable_resource(struct msm_dsi_phy *phy) static void dsi_phy_disable_resource(struct msm_dsi_phy *phy) { clk_disable_unprepare(phy->ahb_clk); - pm_runtime_put_autosuspend(&phy->pdev->dev); + pm_runtime_put(&phy->pdev->dev); } static const struct of_device_id dsi_phy_dt_match[] = { diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c index 211b73dddf65..68fba4bf7212 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c @@ -69,7 +69,7 @@ static void power_off(struct drm_bridge *bridge) if (ret) DRM_DEV_ERROR(dev->dev, "failed to disable pwr regulator: %d\n", ret); - pm_runtime_put_autosuspend(&hdmi->pdev->dev); + pm_runtime_put(&hdmi->pdev->dev); } #define AVI_IFRAME_LINE_NUMBER 1 diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c index 1cda7bf23b3b..75605ddac7c4 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c @@ -205,7 +205,7 @@ void msm_hdmi_hpd_disable(struct hdmi_bridge *hdmi_bridge) msm_hdmi_set_mode(hdmi, false); enable_hpd_clocks(hdmi, false); - pm_runtime_put_autosuspend(dev); + pm_runtime_put(dev); ret = gpio_config(hdmi, false); if (ret) @@ -260,7 +260,7 @@ static enum drm_connector_status detect_reg(struct hdmi *hdmi) hpd_int_status = hdmi_read(hdmi, REG_HDMI_HPD_INT_STATUS); enable_hpd_clocks(hdmi, false); - pm_runtime_put_autosuspend(&hdmi->pdev->dev); + pm_runtime_put(&hdmi->pdev->dev); return (hpd_int_status & HDMI_HPD_INT_STATUS_CABLE_DETECTED) ? connector_status_connected : connector_status_disconnected;