drm/msm: Runtime PM for MDP5 based devices
Subsystems (4)
cover drm/msm: Runtime PM for MDP5 based devices
This series sets up runtime PM for MDP5 based SoCs.
We have a top level MDSS device, which parents devices like MDP5, DSI, HDMI etc. The parent child relation between them establishes a similar relationship for their power domains too. In the HW (and the DT bindings), only MDSS has control over the power domain (i.e, the MDSS GDSC). In software, we do the same by assigning the GDSC to MDSS, and the children end up voting for the GDSC by calling the pm_runtime_get/put() API.
For this to work, we need to convert all the child drivers to DT. The patchset converts MDP5, DSI and HDMI to use runtime PM. The first 2 patches and the last patch are minor fixes not directly related to runtime PM.
Tested on DB410c and DB820c.
Archit Taneja (10):
drm/msm/mdp5: Fix typo in encoder_enable path drm/msm/mdp5: Drop clock names with "_clk" suffix drm/msm/mdp5: Use runtime PM get/put API instead of toggling clocks drm/msm/hdmi: Set up runtime PM for HDMI
drm/msm/dsi: Set up runtime PM for DSI
drm/msm/dsi: Implement RPM suspend/resume callbacks drm/msm/mdp5: Don't use mode_set helper funcs for encoders and CRTCs drm/msm/mdp5: Write to SMP registers even if allocations don't change drm/msm/mdp5: Set up runtime PM for MDSS
drm/msm/adreno: Prevent unclocked access when retrieving timestamps
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 11 ++- drivers/gpu/drm/msm/dsi/dsi.c | 5 ++ drivers/gpu/drm/msm/dsi/dsi.h | 2 + drivers/gpu/drm/msm/dsi/dsi_host.c | 94 ++++++++++++++----------- drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 2 +- drivers/gpu/drm/msm/hdmi/hdmi.c | 2 + drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 4 ++ drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 63 ++++++++++++----- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c | 7 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 25 ++++--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 14 ++-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c | 27 ++++--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 63 ++++++++++++----- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 3 - drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c | 63 ++++++++++++++--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c | 59 +++++++++++++--- drivers/gpu/drm/msm/msm_drv.c | 29 ++++++++ drivers/gpu/drm/msm/msm_kms.h | 2 + 18 files changed, 350 insertions(+), 125 deletions(-)
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 1/10 drm/msm/mdp5: Fix typo in encoder_enable path
The mdp5_cmd_encoder_disable is accidentally called in the encoder enable path. We've not seen any problems since we haven't tested with command mode panels in a while. Fix the copy-paste error.
--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c +1 -1
@@ -299,7 +299,7 @@ static void mdp5_encoder_enable(struct drm_encoder *encoder)struct mdp5_interface *intf = mdp5_encoder->intf;if (intf->mode ==MDP5_INTF_DSI_MODE_COMMAND ) -mdp5_cmd_encoder_disable (encoder); +mdp5_cmd_encoder_enable (encoder);else mdp5_vid_encoder_enable (encoder); }
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH 2/10 drm/msm/mdp5: Drop clock names with "_clk" suffix
In reply to Archit TanejaWe have upstream bindings (msm8916) that have the "_clk" suffix in the clock names. The downstream bindings also require it.
We want to drop the "_clk" suffix and at the same time support existing bindings. Update the MDP5 code with the the msm_clk_get() helper to support both old and new clock names.
--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +7 -7
@@ -511,7 +511,7 @@ static int get_clk(struct platform_device *pdev, struct clk **clkp, const char *name, bool mandatory) {struct device *dev = &pdev->dev; -struct clk *clk =devm_clk_get (dev, name); +struct clk *clk =msm_clk_get (pdev, name);if (IS_ERR (clk) && mandatory) {dev_err (dev,"failed to get %s (%ld)\n" , name,PTR_ERR (clk));return PTR_ERR (clk);
@@ -896,22 +896,22 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev) }/* mandatory clocks: */ - ret =get_clk (pdev, &mdp5_kms->axi_clk,"bus_clk" , true); + ret =get_clk (pdev, &mdp5_kms->axi_clk,"bus" , true);if (ret) goto fail; - ret =get_clk (pdev, &mdp5_kms->ahb_clk,"iface_clk" , true); + ret =get_clk (pdev, &mdp5_kms->ahb_clk,"iface" , true);if (ret) goto fail; - ret =get_clk (pdev, &mdp5_kms->core_clk,"core_clk" , true); + ret =get_clk (pdev, &mdp5_kms->core_clk,"core" , true);if (ret) goto fail; - ret =get_clk (pdev, &mdp5_kms->vsync_clk,"vsync_clk" , true); + ret =get_clk (pdev, &mdp5_kms->vsync_clk,"vsync" , true);if (ret) goto fail;/* optional clocks: */ -get_clk (pdev, &mdp5_kms->lut_clk,"lut_clk" , false); -get_clk (pdev, &mdp5_kms->iommu_clk,"iommu_clk" , false); +get_clk (pdev, &mdp5_kms->lut_clk,"lut" , false); +get_clk (pdev, &mdp5_kms->iommu_clk,"iommu" , false);/* we need to set a default rate before enabling. Set a safe * rate first, then figure out hw revision, and then set a
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH 3/10 drm/msm/mdp5: Use runtime PM get/put API instead of toggling clocks
In reply to Archit Tanejamdp5_enable/disable calls are scattered all around in the MDP5 code. Use the pm_runtime_get/put calls here instead, and populate the runtime PM suspend/resume ops to manage the clocks.
About the overall design: MDP5 is a child of the top level MDSS device. MDSS is also the parent to DSI, HDMI and other interfaces. When we enable MDP5's power domain, we end up enabling MDSS's PD too. It is only MDSS's PD that actually controlls the GDSC HW. Therefore, calling runtime_get/put on the MDP5 device is like just requesting a vote to enable/disable the GDSC.
Functionally, replacing the clock enable/disable calls with the RPM API can result in the power domain (GDSC) state being toggled if no other child isn't powered on. This can result in the register context being lost. We make sure (in future commits) that code paths don't end up configuring registers and then later lose state, resulting in a bad HW state.
For now, we've replaced each mdp5_enable/disable with runtime_get/put API. We could optimize things later by removing runtime_get/put calls which don't really need to be there. This could prevent unnecessary toggling of the power domain and clocks.
--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c | 7 +++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 21 +++++++---- drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 7 +++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c | 27 +++++++++----- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 49 +++++++++++++++++++------ drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 3 -- 6 files changed, 78 insertions(+), 36 deletions(-)
drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c +5 -2
@@ -192,6 +192,7 @@ int mdp5_cmd_encoder_set_split_display(struct drm_encoder *encoder, {struct mdp5_encoder *mdp5_cmd_enc =to_mdp5_encoder (encoder);struct mdp5_kms *mdp5_kms; +struct device *dev;int intf_num;u32 data =0 ;
@@ -214,14 +215,16 @@ int mdp5_cmd_encoder_set_split_display(struct drm_encoder *encoder,/* Smart Panel, Sync mode */ data |=MDP5_SPLIT_DPL_UPPER_SMART_PANEL ; + dev = &mdp5_kms->pdev->dev; +/* Make sure clocks are on when connectors calling this function. */ -mdp5_enable (mdp5_kms); +pm_runtime_get_sync (dev);mdp5_write (mdp5_kms,REG_MDP5_SPLIT_DPL_UPPER , data);mdp5_write (mdp5_kms,REG_MDP5_SPLIT_DPL_LOWER ,MDP5_SPLIT_DPL_LOWER_SMART_PANEL );mdp5_write (mdp5_kms,REG_MDP5_SPLIT_DPL_EN ,1 ); -mdp5_disable (mdp5_kms); +pm_runtime_put_autosuspend (dev);return 0 ; }
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c +13 -8
@@ -414,6 +414,7 @@ static void mdp5_crtc_disable(struct drm_crtc *crtc)struct mdp5_crtc *mdp5_crtc =to_mdp5_crtc (crtc);struct mdp5_crtc_state *mdp5_cstate =to_mdp5_crtc_state (crtc->state);struct mdp5_kms *mdp5_kms =get_kms (crtc); +struct device *dev = &mdp5_kms->pdev->dev;DBG ("%s" , crtc->name);
@@ -424,7 +425,7 @@ static void mdp5_crtc_disable(struct drm_crtc *crtc)mdp_irq_unregister (&mdp5_kms->base, &mdp5_crtc->pp_done);mdp_irq_unregister (&mdp5_kms->base, &mdp5_crtc->err); -mdp5_disable (mdp5_kms); +pm_runtime_put_autosuspend (dev); mdp5_crtc->enabled = false; }
@@ -434,13 +435,14 @@ static void mdp5_crtc_enable(struct drm_crtc *crtc)struct mdp5_crtc *mdp5_crtc =to_mdp5_crtc (crtc);struct mdp5_crtc_state *mdp5_cstate =to_mdp5_crtc_state (crtc->state);struct mdp5_kms *mdp5_kms =get_kms (crtc); +struct device *dev = &mdp5_kms->pdev->dev;DBG ("%s" , crtc->name);if (WARN_ON (mdp5_crtc->enabled))return ; -mdp5_enable (mdp5_kms); +pm_runtime_get_sync (dev);mdp_irq_register (&mdp5_kms->base, &mdp5_crtc->err);if (mdp5_cstate->cmd_mode)
@@ -725,6 +727,7 @@ static int mdp5_crtc_cursor_set(struct drm_crtc *crtc,struct mdp5_pipeline *pipeline = &mdp5_cstate->pipeline;struct drm_device *dev = crtc->dev;struct mdp5_kms *mdp5_kms =get_kms (crtc); +struct platform_device *pdev = mdp5_kms->pdev;struct msm_kms *kms = &mdp5_kms->base.base;struct drm_gem_object *cursor_bo, *old_bo =NULL ;uint32_t blendcfg, stride;
@@ -753,7 +756,7 @@ static int mdp5_crtc_cursor_set(struct drm_crtc *crtc,if (!handle) {DBG ("Cursor off" ); cursor_enable = false; -mdp5_enable (mdp5_kms); +pm_runtime_get_sync (&pdev->dev); goto set_cursor; }
@@ -768,6 +771,8 @@ static int mdp5_crtc_cursor_set(struct drm_crtc *crtc, lm = mdp5_cstate->pipeline.mixer->lm; stride = width *drm_format_plane_cpp (DRM_FORMAT_ARGB8888 ,0 ); +pm_runtime_get_sync (&pdev->dev); +spin_lock_irqsave (&mdp5_crtc->cursor.lock, flags); old_bo = mdp5_crtc->cursor.scanout_bo;
@@ -777,8 +782,6 @@ static int mdp5_crtc_cursor_set(struct drm_crtc *crtc,get_roi (crtc, &roi_w, &roi_h); -mdp5_enable (mdp5_kms); -mdp5_write (mdp5_kms,REG_MDP5_LM_CURSOR_STRIDE (lm), stride);mdp5_write (mdp5_kms,REG_MDP5_LM_CURSOR_FORMAT (lm),MDP5_LM_CURSOR_FORMAT_FORMAT (CURSOR_FMT_ARGB8888 ));
@@ -796,6 +799,8 @@ static int mdp5_crtc_cursor_set(struct drm_crtc *crtc,spin_unlock_irqrestore (&mdp5_crtc->cursor.lock, flags); +pm_runtime_put_autosuspend (&pdev->dev); + set_cursor: ret =mdp5_ctl_set_cursor (ctl, pipeline,0 , cursor_enable);if (ret) {
@@ -807,7 +812,7 @@ static int mdp5_crtc_cursor_set(struct drm_crtc *crtc,crtc_flush (crtc, flush_mask); end: -mdp5_disable (mdp5_kms); +pm_runtime_put_autosuspend (&pdev->dev);if (old_bo) {drm_flip_work_queue (&mdp5_crtc->unref_cursor_work, old_bo);/* enable vblank to complete cursor work: */
@@ -840,7 +845,7 @@ static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)get_roi (crtc, &roi_w, &roi_h); -mdp5_enable (mdp5_kms); +pm_runtime_get_sync (&mdp5_kms->pdev->dev);spin_lock_irqsave (&mdp5_crtc->cursor.lock, flags); mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_SIZE(lm),
@@ -853,7 +858,7 @@ static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)crtc_flush (crtc, flush_mask); -mdp5_disable (mdp5_kms); +pm_runtime_put_autosuspend (&mdp5_kms->pdev->dev);return 0 ; }
drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c +5 -2
@@ -350,6 +350,7 @@ int mdp5_vid_encoder_set_split_display(struct drm_encoder *encoder,struct mdp5_encoder *mdp5_encoder =to_mdp5_encoder (encoder);struct mdp5_encoder *mdp5_slave_enc =to_mdp5_encoder (slave_encoder);struct mdp5_kms *mdp5_kms; +struct device *dev;int intf_num;u32 data =0 ;
@@ -369,8 +370,10 @@ int mdp5_vid_encoder_set_split_display(struct drm_encoder *encoder,else return -EINVAL ; + dev = &mdp5_kms->pdev->dev;/* Make sure clocks are on when connectors calling this function. */ -mdp5_enable (mdp5_kms); +pm_runtime_get_sync (dev); +/* Dumb Panel, Sync mode */ mdp5_write (mdp5_kms,REG_MDP5_SPLIT_DPL_UPPER ,0 );mdp5_write (mdp5_kms,REG_MDP5_SPLIT_DPL_LOWER , data);
@@ -378,7 +381,7 @@ int mdp5_vid_encoder_set_split_display(struct drm_encoder *encoder,mdp5_ctl_pair (mdp5_encoder->ctl, mdp5_slave_enc->ctl, true); -mdp5_disable (mdp5_kms); +pm_runtime_put_autosuspend (dev);return 0 ; }
drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c +17 -10
@@ -49,16 +49,19 @@ static void mdp5_irq_error_handler(struct mdp_irq *irq, uint32_t irqstatus)void mdp5_irq_preinstall (struct msm_kms *kms) {struct mdp5_kms *mdp5_kms =to_mdp5_kms (to_mdp_kms (kms)); -mdp5_enable (mdp5_kms); +struct device *dev = &mdp5_kms->pdev->dev; + +pm_runtime_get_sync (dev);mdp5_write (mdp5_kms,REG_MDP5_INTR_CLEAR ,0xffffffff );mdp5_write (mdp5_kms,REG_MDP5_INTR_EN ,0x00000000 ); -mdp5_disable (mdp5_kms); +pm_runtime_put_autosuspend (dev); }int mdp5_irq_postinstall (struct msm_kms *kms) {struct mdp_kms *mdp_kms =to_mdp_kms (kms);struct mdp5_kms *mdp5_kms =to_mdp5_kms (mdp_kms); +struct device *dev = &mdp5_kms->pdev->dev;struct mdp_irq *error_handler = &mdp5_kms->error_handler; error_handler->irq = mdp5_irq_error_handler;
@@ -67,9 +70,9 @@ int mdp5_irq_postinstall(struct msm_kms *kms)MDP5_IRQ_INTF2_UNDER_RUN |MDP5_IRQ_INTF3_UNDER_RUN ; -mdp5_enable (mdp5_kms); +pm_runtime_get_sync (dev);mdp_irq_register (mdp_kms, error_handler); -mdp5_disable (mdp5_kms); +pm_runtime_put_autosuspend (dev);return 0 ; }
@@ -77,9 +80,11 @@ int mdp5_irq_postinstall(struct msm_kms *kms)void mdp5_irq_uninstall (struct msm_kms *kms) {struct mdp5_kms *mdp5_kms =to_mdp5_kms (to_mdp_kms (kms)); -mdp5_enable (mdp5_kms); +struct device *dev = &mdp5_kms->pdev->dev; + +pm_runtime_get_sync (dev);mdp5_write (mdp5_kms,REG_MDP5_INTR_EN ,0x00000000 ); -mdp5_disable (mdp5_kms); +pm_runtime_put_autosuspend (dev); }irqreturn_t mdp5_irq (struct msm_kms *kms)
@@ -109,11 +114,12 @@ irqreturn_t mdp5_irq(struct msm_kms *kms)int mdp5_enable_vblank (struct msm_kms *kms,struct drm_crtc *crtc) {struct mdp5_kms *mdp5_kms =to_mdp5_kms (to_mdp_kms (kms)); +struct device *dev = &mdp5_kms->pdev->dev; -mdp5_enable (mdp5_kms); +pm_runtime_get_sync (dev);mdp_update_vblank_mask (to_mdp_kms (kms),mdp5_crtc_vblank (crtc), true); -mdp5_disable (mdp5_kms); +pm_runtime_put_autosuspend (dev);return 0 ; }
@@ -121,9 +127,10 @@ int mdp5_enable_vblank(struct msm_kms *kms, struct drm_crtc *crtc)void mdp5_disable_vblank (struct msm_kms *kms,struct drm_crtc *crtc) {struct mdp5_kms *mdp5_kms =to_mdp5_kms (to_mdp_kms (kms)); +struct device *dev = &mdp5_kms->pdev->dev; -mdp5_enable (mdp5_kms); +pm_runtime_get_sync (dev);mdp_update_vblank_mask (to_mdp_kms (kms),mdp5_crtc_vblank (crtc), false); -mdp5_disable (mdp5_kms); +pm_runtime_put_autosuspend (dev); }
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +38 -11
119 lines not on this page · raw message
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h +0 -3
@@ -260,9 +260,6 @@ static inline uint32_t lm2ppdone(struct mdp5_hw_mixer *mixer)return MDP5_IRQ_PING_PONG_0_DONE << mixer->pp; } -int mdp5_disable (struct mdp5_kms *mdp5_kms); -int mdp5_enable (struct mdp5_kms *mdp5_kms); -void mdp5_set_irqmask (struct mdp_kms *mdp_kms,uint32_t irqmask,uint32_t old_irqmask);void mdp5_irq_preinstall (struct msm_kms *kms);
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 4/10 drm/msm/hdmi: Set up runtime PM for HDMI
In reply to Archit TanejaEnable rudimentary runtime PM in the HDMI driver. We can't really do agressive PM toggling at the moment because we need to leave the hpd clocks enabled all the time. There isn't much benefit of creating suspend/resume ops to toggle clocks either.
We just make sure that we configure the power domain in the HDMI bridge's enable/disable paths, and the HDMI connector's detect() op.
--- drivers/gpu/drm/msm/hdmi/hdmi.c | 2 + drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 4 ++ drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 63 +++++++++++++++++++++---------- 3 files changed, 50 insertions(+), 19 deletions(-)
drivers/gpu/drm/msm/hdmi/hdmi.c +2 -0
@@ -239,6 +239,8 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev) hdmi->pwr_clks[i] = clk; } +pm_runtime_enable (&pdev->dev); + hdmi->workq =alloc_ordered_workqueue ("msm_hdmi" ,0 ); hdmi->i2c =msm_hdmi_i2c_init (hdmi);
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c +4 -0
@@ -35,6 +35,8 @@ static void msm_hdmi_power_on(struct drm_bridge *bridge)const struct hdmi_platform_config *config = hdmi->config;int i, ret; +pm_runtime_get_sync (&hdmi->pdev->dev); +for (i =0 ; i < config->pwr_reg_cnt; i++) { ret =regulator_enable (hdmi->pwr_regs[i]);if (ret) {
@@ -84,6 +86,8 @@ static void power_off(struct drm_bridge *bridge) config->pwr_reg_names[i], ret); } } + +pm_runtime_put_autosuspend (&hdmi->pdev->dev); }#define AVI_IFRAME_LINE_NUMBER 1
drivers/gpu/drm/msm/hdmi/hdmi_connector.c +44 -19
87 lines not on this page · raw message
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH 5/10 drm/msm/dsi: Set up runtime PM for DSI
In reply to Archit TanejaCall the pm_runtime_get/put API where we need the clocks enabled.
The main entry/exit points are 1) enabling/disabling the DSI bridge and 2) Sending commands from the DSI host to the device.
--- drivers/gpu/drm/msm/dsi/dsi_host.c | 11 +++++++++++ drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-)
drivers/gpu/drm/msm/dsi/dsi_host.c +11 -0
@@ -221,6 +221,8 @@ static const struct msm_dsi_cfg_handler *dsi_get_config( goto put_gdsc; } +pm_runtime_get_sync (dev); + ret =regulator_enable (gdsc_reg);if (ret) {pr_err ("%s: unable to enable gdsc\n" , __func__);
@@ -247,6 +249,7 @@ static const struct msm_dsi_cfg_handler *dsi_get_config(clk_disable_unprepare (ahb_clk); disable_gdsc:regulator_disable (gdsc_reg); +pm_runtime_put_autosuspend (dev); put_clk:clk_put (ahb_clk); put_gdsc:
@@ -1713,6 +1716,8 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) goto fail; } +pm_runtime_enable (&pdev->dev); + msm_host->cfg_hnd =dsi_get_config (msm_host);if (!msm_host->cfg_hnd) { ret = -EINVAL ;
@@ -1786,6 +1791,8 @@ void msm_dsi_host_destroy(struct mipi_dsi_host *host)mutex_destroy (&msm_host->clk_mutex);mutex_destroy (&msm_host->cmd_mutex);mutex_destroy (&msm_host->dev_mutex); + +pm_runtime_disable (&msm_host->pdev->dev); } int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
@@ -1881,6 +1888,7 @@ int msm_dsi_host_xfer_prepare(struct mipi_dsi_host *host,* mdss interrupt is generated in mdp core clock domain * mdp clock need to be enabled to receive dsi interrupt */ +pm_runtime_get_sync (&msm_host->pdev->dev);dsi_clk_ctrl (msm_host,1 );/* TODO: vote for bus bandwidth */
@@ -1912,6 +1920,7 @@ void msm_dsi_host_xfer_restore(struct mipi_dsi_host *host,/* TODO: unvote for bus bandwidth */ dsi_clk_ctrl (msm_host,0 ); +pm_runtime_put_autosuspend (&msm_host->pdev->dev); } int msm_dsi_host_cmd_tx(struct mipi_dsi_host *host,
@@ -2217,6 +2226,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host, goto unlock_ret; } +pm_runtime_get_sync (&msm_host->pdev->dev); ret =dsi_clk_ctrl (msm_host,1 );if (ret) {pr_err ("%s: failed to enable clocks. ret=%d\n" , __func__, ret);
@@ -2269,6 +2279,7 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host)pinctrl_pm_select_sleep_state (&msm_host->pdev->dev);dsi_clk_ctrl (msm_host,0 ); +pm_runtime_put_autosuspend (&msm_host->pdev->dev);dsi_host_regulator_disable (msm_host);
drivers/gpu/drm/msm/dsi/phy/dsi_phy.c +1 -1
@@ -373,7 +373,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_sync (&phy->pdev->dev); +pm_runtime_put_autosuspend (&phy->pdev->dev); }static const struct of_device_id dsi_phy_dt_match[] = {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH 6/10 drm/msm/dsi: Implement RPM suspend/resume callbacks
In reply to Archit TanejaThe bus clocks are always enabled/disabled along with the power domain, so move it to the runtime suspend/resume ops. This cleans up the clock code a bit. Get rid of the clk_mutex mutex since it isn't needed.
--- drivers/gpu/drm/msm/dsi/dsi.c | 5 +++ drivers/gpu/drm/msm/dsi/dsi.h | 2 + drivers/gpu/drm/msm/dsi/dsi_host.c | 83 +++++++++++++++++++------------------- 3 files changed, 49 insertions(+), 41 deletions(-)
drivers/gpu/drm/msm/dsi/dsi.c +5 -0
@@ -161,12 +161,17 @@ static const struct of_device_id dt_match[] = { {} }; +static const struct dev_pm_ops dsi_pm_ops = { +SET_RUNTIME_PM_OPS (msm_dsi_runtime_suspend, msm_dsi_runtime_resume,NULL ) +}; +static struct platform_driver dsi_driver = { .probe = dsi_dev_probe, .remove = dsi_dev_remove, .driver = { .name ="msm_dsi" , .of_match_table = dt_match, + .pm = &dsi_pm_ops, }, };
drivers/gpu/drm/msm/dsi/dsi.h +2 -0
@@ -179,6 +179,8 @@ void msm_dsi_host_destroy(struct mipi_dsi_host *host);int msm_dsi_host_modeset_init (struct mipi_dsi_host *host,struct drm_device *dev);int msm_dsi_host_init (struct msm_dsi *msm_dsi); +int msm_dsi_runtime_suspend (struct device *dev); +int msm_dsi_runtime_resume (struct device *dev);/* dsi phy */ struct msm_dsi_phy ;
drivers/gpu/drm/msm/dsi/dsi_host.c +42 -41
162 lines not on this page · raw message
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH 7/10 drm/msm/mdp5: Don't use mode_set helper funcs for encoders and CRTCs
In reply to Archit TanejaWe shouldn't use use mode_set/mode_set_nofb helpers when we use runtime PM. The registers configured in these funcs lose their state when we eventually enable the display pipeline.
Do not implement these vfuncs in the helpers, and call them in the crtc_enable/encoder_enable paths instead.
--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 4 +++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-)
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c +3 -1
@@ -443,6 +443,9 @@ static void mdp5_crtc_enable(struct drm_crtc *crtc)return ;pm_runtime_get_sync (dev); + +mdp5_crtc_mode_set_nofb (crtc); +mdp_irq_register (&mdp5_kms->base, &mdp5_crtc->err);if (mdp5_cstate->cmd_mode)
@@ -951,7 +954,6 @@ static const struct drm_crtc_funcs mdp5_crtc_no_lm_cursor_funcs = { };static const struct drm_crtc_helper_funcs mdp5_crtc_helper_funcs = { - .mode_set_nofb = mdp5_crtc_mode_set_nofb, .disable = mdp5_crtc_disable, .enable = mdp5_crtc_enable, .atomic_check = mdp5_crtc_atomic_check,
drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c +4 -1
@@ -297,6 +297,10 @@ static void mdp5_encoder_enable(struct drm_encoder *encoder) {struct mdp5_encoder *mdp5_encoder =to_mdp5_encoder (encoder);struct mdp5_interface *intf = mdp5_encoder->intf; +/* this isn't right I think */ +struct drm_crtc_state *cstate = encoder->crtc->state; + +mdp5_encoder_mode_set (encoder, &cstate->mode, &cstate->adjusted_mode);if (intf->mode ==MDP5_INTF_DSI_MODE_COMMAND )mdp5_cmd_encoder_enable (encoder);
@@ -320,7 +324,6 @@ static int mdp5_encoder_atomic_check(struct drm_encoder *encoder, }static const struct drm_encoder_helper_funcs mdp5_encoder_helper_funcs = { - .mode_set = mdp5_encoder_mode_set, .disable = mdp5_encoder_disable, .enable = mdp5_encoder_enable, .atomic_check = mdp5_encoder_atomic_check,
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH 8/10 drm/msm/mdp5: Write to SMP registers even if allocations don't change
In reply to Archit TanejaRequests for assigning/freeing SMP blocks by planes are collected during the atomic check phase, and represented by mdp5_smp_state's 'assigned' and 'released' members.
Once the atomic state is committed, these members are reset to 0, indicating that the existing configuration satisfies all the planes. Future atomic commits will copy the old mdp5_smp_state, and the 'assigned' and 'released' members would be updated only if there was a change in the plane configurations.
When we disable and re-enable display, we lose the values we wrote to the SMP registers, but the code doesn't program the registers because there isn't any change in mdp5_smp_state.
Fix this by writing to the registers irrespective of whether there was a change in SMP state or not. We do this by keeping a cache of the register values, and write them every time we commit a state.
--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c | 59 ++++++++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 8 deletions(-)
drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c +51 -8
@@ -28,6 +28,13 @@ struct mdp5_smp {int blk_cnt;int blk_size; + +/* register cache */ +u32 alloc_w[22 ]; +u32 alloc_r[22 ]; +u32 pipe_reqprio_fifo_wm0[SSPP_MAX ]; +u32 pipe_reqprio_fifo_wm1[SSPP_MAX ]; +u32 pipe_reqprio_fifo_wm2[SSPP_MAX ]; }; static inline
@@ -98,16 +105,15 @@ static int smp_request_block(struct mdp5_smp *smp,static void set_fifo_thresholds (struct mdp5_smp *smp,enum mdp5_pipe pipe,int nblks) { -struct mdp5_kms *mdp5_kms =get_kms (smp);u32 smp_entries_per_blk = smp->blk_size / (128 /BITS_PER_BYTE );u32 val;/* 1/4 of SMP pool that is being fetched */ val = (nblks * smp_entries_per_blk) /4 ; -mdp5_write (mdp5_kms,REG_MDP5_PIPE_REQPRIO_FIFO_WM_0 (pipe), val *1 ); -mdp5_write (mdp5_kms,REG_MDP5_PIPE_REQPRIO_FIFO_WM_1 (pipe), val *2 ); -mdp5_write (mdp5_kms,REG_MDP5_PIPE_REQPRIO_FIFO_WM_2 (pipe), val *3 ); + smp->pipe_reqprio_fifo_wm0[pipe] = val *1 ; + smp->pipe_reqprio_fifo_wm1[pipe] = val *2 ; + smp->pipe_reqprio_fifo_wm2[pipe] = val *3 ; }/*
@@ -222,7 +228,6 @@ void mdp5_smp_release(struct mdp5_smp *smp, struct mdp5_smp_state *state,static unsigned update_smp_state (struct mdp5_smp *smp,u32 cid,mdp5_smp_state_t *assigned) { -struct mdp5_kms *mdp5_kms =get_kms (smp);int cnt = smp->blk_cnt;unsigned nblks =0 ;u32 blk, val;
@@ -231,7 +236,7 @@ static unsigned update_smp_state(struct mdp5_smp *smp,int idx = blk /3 ;int fld = blk %3 ; - val =mdp5_read (mdp5_kms,REG_MDP5_SMP_ALLOC_W_REG (idx)); + val = smp->alloc_w[idx];switch (fld) {case 0 :
@@ -248,8 +253,8 @@ static unsigned update_smp_state(struct mdp5_smp *smp,break ; } -mdp5_write (mdp5_kms,REG_MDP5_SMP_ALLOC_W_REG (idx), val); -mdp5_write (mdp5_kms,REG_MDP5_SMP_ALLOC_R_REG (idx), val); + smp->alloc_w[idx] = val; + smp->alloc_r[idx] = val; nblks++; }
@@ -257,6 +262,39 @@ static unsigned update_smp_state(struct mdp5_smp *smp,return nblks; } +static void write_smp_alloc_regs (struct mdp5_smp *smp) +{ +struct mdp5_kms *mdp5_kms =get_kms (smp); +int i, num_regs; + + num_regs = smp->blk_cnt /3 +1 ; + +for (i =0 ; i < num_regs; i++) { +mdp5_write (mdp5_kms,REG_MDP5_SMP_ALLOC_W_REG (i), + smp->alloc_w[i]); +mdp5_write (mdp5_kms,REG_MDP5_SMP_ALLOC_R_REG (i), + smp->alloc_r[i]); + } +} + +static void write_smp_fifo_regs (struct mdp5_smp *smp) +{ +struct mdp5_kms *mdp5_kms =get_kms (smp); +int i; + +for (i =0 ; i < mdp5_kms->num_hwpipes; i++) { +struct mdp5_hw_pipe *hwpipe = mdp5_kms->hwpipes[i]; +enum mdp5_pipe pipe = hwpipe->pipe; + +mdp5_write (mdp5_kms,REG_MDP5_PIPE_REQPRIO_FIFO_WM_0 (pipe), + smp->pipe_reqprio_fifo_wm0[pipe]); +mdp5_write (mdp5_kms,REG_MDP5_PIPE_REQPRIO_FIFO_WM_1 (pipe), + smp->pipe_reqprio_fifo_wm1[pipe]); +mdp5_write (mdp5_kms,REG_MDP5_PIPE_REQPRIO_FIFO_WM_2 (pipe), + smp->pipe_reqprio_fifo_wm2[pipe]); + } +} +void mdp5_smp_prepare_commit (struct mdp5_smp *smp,struct mdp5_smp_state *state) {enum mdp5_pipe pipe;
@@ -277,6 +315,9 @@ void mdp5_smp_prepare_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state)set_fifo_thresholds (smp, pipe, nblks); } +write_smp_alloc_regs (smp); +write_smp_fifo_regs (smp); + state->assigned =0 ; }
@@ -289,6 +330,8 @@ void mdp5_smp_complete_commit(struct mdp5_smp *smp, struct mdp5_smp_state *stateset_fifo_thresholds (smp, pipe,0 ); } +write_smp_fifo_regs (smp); + state->released =0 ; }
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH 9/10 drm/msm/mdp5: Set up runtime PM for MDSS
In reply to Archit TanejaMDSS represents the top level wrapper that contains MDP5, DSI, HDMI and other sub-blocks. W.r.t device heirarchy, it's the parent of all these devices. The power domain of this device is actually tied to the GDSC hw. When any sub-device enables its PD, MDSS's PD is also enabled.
The suspend/resume ops enable the top level clocks that end at the MDSS boundary. For now, we're letting them all be optional, since the child devices anyway hold a ref to these clocks.
Until now, we'd called a runtime_get() during probe, which ensured that the GDSC was always on. Now that we've set up runtime PM for the children devices, we can get rid of this hack.
Note: that the MDSS device is the platform_device in msm_drv.c. The msm_runtime_suspend/resume ops call the funcs that enable/disable the top level MDSS clocks. This is different from MDP4, where the platform device created in msm_drv.c represents MDP4 itself. It would have been nicer to hide these differences by adding new kms funcs, but runtime PM needs to be enabled before kms is set up (i.e, msm_kms_init is called).
--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c | 63 ++++++++++++++++++++++++++++---- drivers/gpu/drm/msm/msm_drv.c | 29 +++++++++++++++ drivers/gpu/drm/msm/msm_kms.h | 2 + 3 files changed, 86 insertions(+), 8 deletions(-)
drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c +55 -8
@@ -31,6 +31,10 @@ struct msm_mdss {struct regulator *vdd; +struct clk *ahb_clk; +struct clk *axi_clk; +struct clk *vsync_clk; +struct {volatile unsigned long enabled_mask;struct irq_domain *domain;
@@ -140,6 +144,51 @@ static int mdss_irq_domain_init(struct msm_mdss *mdss)return 0 ; } +int msm_mdss_enable (struct msm_mdss *mdss) +{ +DBG ("" ); + +clk_prepare_enable (mdss->ahb_clk); +if (mdss->axi_clk) +clk_prepare_enable (mdss->axi_clk); +if (mdss->vsync_clk) +clk_prepare_enable (mdss->vsync_clk); + +return 0 ; +} + +int msm_mdss_disable (struct msm_mdss *mdss) +{ +DBG ("" ); + +if (mdss->vsync_clk) +clk_disable_unprepare (mdss->vsync_clk); +if (mdss->axi_clk) +clk_disable_unprepare (mdss->axi_clk); +clk_disable_unprepare (mdss->ahb_clk); + +return 0 ; +} + +static int msm_mdss_get_clocks (struct msm_mdss *mdss) +{ +struct platform_device *pdev =to_platform_device (mdss->dev->dev); + + mdss->ahb_clk =msm_clk_get (pdev,"iface" ); +if (IS_ERR (mdss->ahb_clk)) + mdss->ahb_clk =NULL ; + + mdss->axi_clk =msm_clk_get (pdev,"bus" ); +if (IS_ERR (mdss->axi_clk)) + mdss->axi_clk =NULL ; + + mdss->vsync_clk =msm_clk_get (pdev,"vsync" ); +if (IS_ERR (mdss->vsync_clk)) + mdss->vsync_clk =NULL ; + +return 0 ; +} +void msm_mdss_destroy (struct drm_device *dev) {struct msm_drm_private *priv = dev->dev_private;
@@ -153,8 +202,6 @@ void msm_mdss_destroy(struct drm_device *dev)regulator_disable (mdss->vdd); -pm_runtime_put_sync (dev->dev); -pm_runtime_disable (dev->dev); }
@@ -190,6 +237,12 @@ int msm_mdss_init(struct drm_device *dev) goto fail; } + ret =msm_mdss_get_clocks (mdss); +if (ret) { +dev_err (dev->dev,"failed to get clocks: %d\n" , ret); + goto fail; + } +/* Regulator to enable GDSCs in downstream kernels */ mdss->vdd =devm_regulator_get (dev->dev,"vdd" );if (IS_ERR (mdss->vdd)) {
@@ -221,12 +274,6 @@ int msm_mdss_init(struct drm_device *dev)pm_runtime_enable (dev->dev); -/* -* TODO: This is needed as the MDSS GDSC is only tied to MDSS's power -* domain. Remove this once runtime PM is adapted for all the devices. -*/ -pm_runtime_get_sync (dev->dev); -return 0 ; fail_irq:regulator_disable (mdss->vdd);
drivers/gpu/drm/msm/msm_drv.c +29 -0
@@ -879,8 +879,37 @@ static int msm_pm_resume(struct device *dev) }#endif +#ifdef CONFIG_PM +static int msm_runtime_suspend (struct device *dev) +{ +struct drm_device *ddev =dev_get_drvdata (dev); +struct msm_drm_private *priv = ddev->dev_private; + +DBG ("" ); + +if (priv->mdss) +return msm_mdss_disable (priv->mdss); + +return 0 ; +} + +static int msm_runtime_resume (struct device *dev) +{ +struct drm_device *ddev =dev_get_drvdata (dev); +struct msm_drm_private *priv = ddev->dev_private; + +DBG ("" ); + +if (priv->mdss) +return msm_mdss_enable (priv->mdss); + +return 0 ; +} +#endif +static const struct dev_pm_ops msm_pm_ops = { SET_SYSTEM_SLEEP_PM_OPS(msm_pm_suspend, msm_pm_resume) +SET_RUNTIME_PM_OPS (msm_runtime_suspend, msm_runtime_resume,NULL ) };/*
drivers/gpu/drm/msm/msm_kms.h +2 -0
@@ -99,5 +99,7 @@ struct msm_kms *mdp4_kms_init(struct drm_device *dev);struct msm_kms *mdp5_kms_init (struct drm_device *dev);int msm_mdss_init (struct drm_device *dev);void msm_mdss_destroy (struct drm_device *dev); +int msm_mdss_enable (struct msm_mdss *mdss); +int msm_mdss_disable (struct msm_mdss *mdss);#endif /* __MSM_KMS_H__ */
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH 10/10 drm/msm/adreno: Prevent unclocked access when retrieving timestamps
In reply to Archit Tanejamsm_gpu's get_timestamp() op (called by the MSM_GET_PARAM ioctl) can result in register accesses. We need our power domain and clocks to be active for that. Make sure they are enabled here.
--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/gpu/drm/msm/adreno/adreno_gpu.c +9 -2
@@ -48,8 +48,15 @@ int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value) *value = adreno_gpu->base.fast_rate;return 0 ;case MSM_PARAM_TIMESTAMP : -if (adreno_gpu->funcs->get_timestamp) -return adreno_gpu->funcs->get_timestamp (gpu, value); +if (adreno_gpu->funcs->get_timestamp) { +int ret; + +pm_runtime_get_sync (&gpu->pdev->dev); + ret = adreno_gpu->funcs->get_timestamp (gpu, value); +pm_runtime_put_autosuspend (&gpu->pdev->dev); + +return ret; + }return -EINVAL ;default :DBG ("%s: invalid param: %u" , gpu->name, param);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 10/10] drm/msm/adreno: Prevent unclocked access when retrieving timestamps
Quoted 21 lines · Archit Taneja
msm_gpu's
get_timestamp()op (called by theMSM_GET_PARAMioctl) can result in register accesses. We need our power domain and clocks to be active for that. Make sure they are enabled here.Signed-off-by Archit Taneja <[email protected]>--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)drivers/gpu/drm/msm/adreno/adreno_gpu.c +2
-2@@ -48,8 +48,15 @@ int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value) *value = adreno_gpu->base.fast_rate;return 0 ;case MSM_PARAM_TIMESTAMP : -if (adreno_gpu->funcs->get_timestamp) -return adreno_gpu->funcs->get_timestamp (gpu, value); +if (adreno_gpu->funcs->get_timestamp) { +int ret;
drivers/gpu/drm/msm/adreno/adreno_gpu.c int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value) · line 62
Quoted 10 lines
drivers/gpu/drm/msm/adreno/adreno_gpu.c +7
-0+ +pm_runtime_get_sync (&gpu->pdev->dev); + ret = adreno_gpu->funcs->get_timestamp (gpu, value); +pm_runtime_put_autosuspend (&gpu->pdev->dev); + +return ret; + }return -EINVAL ;default :DBG ("%s: invalid param: %u" , gpu->name, param);
This is clearly correct from a stability standpoint but it does raise a few side questions.
When the system is idle it is not interesting to go to all the work of bringing up the system to read a timestamp that will be 0 (or very close to it) and then take the system down again. Furthermore when the system is going up and down repeatedly between submits the counter will keep getting reset to zero and makes it less useful over the long term.
Downstream had these problems too and has an elaborate mechanism to save the value of the counters at power down and (on 4XX and newer) reload the counters with the previous value when power comes back. This has the advantage of consistent numbers with the downside of a reasonable amount of work to maintain the database of allocated counters and taking the to save them on power down and restore them on power up.
This patch should be obviously merged because system hangs are bad but I just wanted to toss out more food for thought as we get more power aggressive and more interested in performance monitoring.
Jordan
Signature · 6 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel