Re: [PATCH] drm/msm/dpu: add icc voting in dpu_mdss_init

2019-03-07 Thread Sean Paul
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

[PATCH] drm/msm/dpu: add icc voting in dpu_mdss_init

2019-02-11 Thread Abhinav Kumar
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 
---
 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;
+
+   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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel