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
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
> +
> + 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel