On Mon, Feb 11, 2019 at 07:04:54PM -0800, Abhinav Kumar wrote: > dpu_mdss_destroy() can get called not just from > msm_drm_uninit() but also from msm_drm_bind() in case > of any failures. > > dpu_mdss_destroy() removes the icc voting by calling > icc_put. This could accidentally remove the voting > done by pm_runtime_enable. > > To make the voting balanced add a minimum vote in > dpu_mdss_init() to avoid any unclocked access. > > This change depends on the following patch which > introduces interconnect binding to MDSS driver: > > https://patchwork.codeaurora.org/patch/708155/ > > Signed-off-by: Abhinav Kumar <abhinavk@xxxxxxxxxxxxxx> Your commit message is unnecessarily trimmed. You might consider changing your editor settings :) > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c > index 38daf8a..bfabb9e 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c > @@ -44,6 +44,16 @@ static int dpu_mdss_parse_data_bus_icc_path(struct drm_device *dev, > return 0; > } > > +static void dpu_mdss_icc_request_bw(struct msm_mdss *mdss) > +{ > + struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss); > + int i; > + u64 avg_bw = dpu_mdss->num_paths ? MAX_BW / dpu_mdss->num_paths : 0; JFYI, since I realize this code already existed: We usually try to avoid using inline conditionals in kernel. So this code would be: u64 avg_bw = 0; if (dpu_mdss->num_paths) avg_bw = MAX_BW / dpu_mdss->num_paths; No need to change it since it's a copy/paste job, but something to remember going forward. This patch is: Reviewed-by: Sean Paul <sean@xxxxxxxxxx> > + > + for (i = 0; i < dpu_mdss->num_paths; i++) > + icc_set_bw(dpu_mdss->path[i], avg_bw, kBps_to_icc(MAX_BW)); > +} > + > static irqreturn_t dpu_mdss_irq(int irq, void *arg) > { > struct dpu_mdss *dpu_mdss = arg; > @@ -153,11 +163,9 @@ static int dpu_mdss_enable(struct msm_mdss *mdss) > { > struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss); > struct dss_module_power *mp = &dpu_mdss->mp; > - int ret, i; > - u64 avg_bw = dpu_mdss->num_paths ? MAX_BW / dpu_mdss->num_paths : 0; > + int ret; > > - for (i = 0; i < dpu_mdss->num_paths; i++) > - icc_set_bw(dpu_mdss->path[i], avg_bw, kBps_to_icc(MAX_BW)); > + dpu_mdss_icc_request_bw(mdss); > > ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, true); > if (ret) > @@ -266,6 +274,8 @@ int dpu_mdss_init(struct drm_device *dev) > > pm_runtime_enable(dev->dev); > > + dpu_mdss_icc_request_bw(priv->mdss); > + > pm_runtime_get_sync(dev->dev); > dpu_mdss->hwversion = readl_relaxed(dpu_mdss->mmio); > pm_runtime_put_sync(dev->dev); > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > -- Sean Paul, Software Engineer, Google / Chromium OS
