Re: [DPU PATCH v2 07/12] drm/msm/dpu: remove clock management code from dpu_power_handle

2018-05-11 Thread Sean Paul 
On Fri, May 11, 2018 at 08:19:33PM +0530, Rajesh Yadav wrote:
> MDSS and dpu drivers manage their respective clocks via
> runtime_pm. Remove custom clock management code from
> dpu_power_handle.
> 
> Also dpu core clock management code is restricted to
> dpu_core_perf module.
> 
> Changes in v2:
>   - remove local variable to hold and return error code
> in _dpu_core_perf_set_core_clk_rate() instead return
> retcode directly from msm_dss_clk_set_rate() call (Sean Paul)
>   - dpu_core_perf_init() is called from dpu_kms_hw_init() and
> most of the params passed are already validated so remove
> redundant checks from dpu_core_perf_init() (Sean Paul)
>   - return >clk_config[i] directly to avoid local variable
> in _dpu_kms_get_clk() (Sean Paul)
>   - invert conditional check to eliminate local rate variable
> from dpu_kms_get_clk_rate() (Sean Paul)
>   - remove end label from dpu_power_resource_init() and return
> directly on dpu_power_parse_dt_supply() failure as no cleanup
> is needed (Sean Paul)
>   - remove checks for vtotal and vrefresh from
> dpu_encoder_phys_cmd_tearcheck_config() as they should be
> valid in mode_set call (Sean Paul)
> 
> Signed-off-by: Rajesh Yadav 

Reviewed-by: Sean Paul 

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c  |  41 ++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h  |   8 +-
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   |   9 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c|  28 ++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h|   9 +
>  drivers/gpu/drm/msm/dpu_power_handle.c | 196 
> +
>  drivers/gpu/drm/msm/dpu_power_handle.h |  40 -
>  7 files changed, 63 insertions(+), 268 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> index 981f77f..5b79077 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -365,6 +365,17 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)
>   }
>  }
>  
> +static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate)
> +{
> + struct dss_clk *core_clk = kms->perf.core_clk;
> +
> + if (core_clk->max_rate && (rate > core_clk->max_rate))
> + rate = core_clk->max_rate;
> +
> + core_clk->rate = rate;
> + return msm_dss_clk_set_rate(core_clk, 1);
> +}
> +
>  static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms)
>  {
>   u64 clk_rate = kms->perf.perf_tune.min_core_clk;
> @@ -376,7 +387,8 @@ static u64 _dpu_core_perf_get_core_clk_rate(struct 
> dpu_kms *kms)
>   dpu_cstate = to_dpu_crtc_state(crtc->state);
>   clk_rate = max(dpu_cstate->new_perf.core_clk_rate,
>   clk_rate);
> - clk_rate = clk_round_rate(kms->perf.core_clk, clk_rate);
> + clk_rate = clk_round_rate(kms->perf.core_clk->clk,
> + clk_rate);
>   }
>   }
>  
> @@ -484,15 +496,11 @@ void dpu_core_perf_crtc_update(struct drm_crtc *crtc,
>  
>   DPU_EVT32(kms->dev, stop_req, clk_rate);
>  
> - /* Temp change to avoid crash in clk_set_rate API. */
> -#ifdef QCOM_DPU_SET_CLK
> - if (dpu_power_clk_set_rate(>phandle,
> -kms->perf.clk_name, clk_rate)) {
> + if (_dpu_core_perf_set_core_clk_rate(kms, clk_rate)) {
>   DPU_ERROR("failed to set %s clock rate %llu\n",
> - kms->perf.clk_name, clk_rate);
> + kms->perf.core_clk->clk_name, clk_rate);
>   return;
>   }
> -#endif
>  
>   kms->perf.core_clk_rate = clk_rate;
>   DPU_DEBUG("update clk rate = %lld HZ\n", clk_rate);
> @@ -656,7 +664,6 @@ void dpu_core_perf_destroy(struct dpu_core_perf *perf)
>   dpu_core_perf_debugfs_destroy(perf);
>   perf->max_core_clk_rate = 0;
>   perf->core_clk = NULL;
> - perf->clk_name = NULL;
>   perf->phandle = NULL;
>   perf->catalog = NULL;
>   perf->dev = NULL;
> @@ -667,9 +674,9 @@ int dpu_core_perf_init(struct dpu_core_perf *perf,
>   struct dpu_mdss_cfg *catalog,
>   struct dpu_power_handle *phandle,
>   struct dpu_power_client *pclient,
> - char *clk_name)
> + struct dss_clk *core_clk)
>  {
> - if (!perf || !dev || !catalog || !phandle || !pclient || !clk_name) {
> + if (!pclient) {
>   DPU_ERROR("invalid parameters\n");
>   return -EINVAL;
>   }
> @@ -678,23 +685,13 @@ int dpu_core_perf_init(struct dpu_core_perf *perf,
>

[DPU PATCH v2 07/12] drm/msm/dpu: remove clock management code from dpu_power_handle

2018-05-11 Thread Rajesh Yadav 
MDSS and dpu drivers manage their respective clocks via
runtime_pm. Remove custom clock management code from
dpu_power_handle.

Also dpu core clock management code is restricted to
dpu_core_perf module.

Changes in v2:
- remove local variable to hold and return error code
  in _dpu_core_perf_set_core_clk_rate() instead return
  retcode directly from msm_dss_clk_set_rate() call (Sean Paul)
- dpu_core_perf_init() is called from dpu_kms_hw_init() and
  most of the params passed are already validated so remove
  redundant checks from dpu_core_perf_init() (Sean Paul)
- return >clk_config[i] directly to avoid local variable
  in _dpu_kms_get_clk() (Sean Paul)
- invert conditional check to eliminate local rate variable
  from dpu_kms_get_clk_rate() (Sean Paul)
- remove end label from dpu_power_resource_init() and return
  directly on dpu_power_parse_dt_supply() failure as no cleanup
  is needed (Sean Paul)
- remove checks for vtotal and vrefresh from
  dpu_encoder_phys_cmd_tearcheck_config() as they should be
  valid in mode_set call (Sean Paul)

Signed-off-by: Rajesh Yadav 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c  |  41 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h  |   8 +-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   |   9 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c|  28 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h|   9 +
 drivers/gpu/drm/msm/dpu_power_handle.c | 196 +
 drivers/gpu/drm/msm/dpu_power_handle.h |  40 -
 7 files changed, 63 insertions(+), 268 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 981f77f..5b79077 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -365,6 +365,17 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)
}
 }
 
+static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate)
+{
+   struct dss_clk *core_clk = kms->perf.core_clk;
+
+   if (core_clk->max_rate && (rate > core_clk->max_rate))
+   rate = core_clk->max_rate;
+
+   core_clk->rate = rate;
+   return msm_dss_clk_set_rate(core_clk, 1);
+}
+
 static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms)
 {
u64 clk_rate = kms->perf.perf_tune.min_core_clk;
@@ -376,7 +387,8 @@ static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms 
*kms)
dpu_cstate = to_dpu_crtc_state(crtc->state);
clk_rate = max(dpu_cstate->new_perf.core_clk_rate,
clk_rate);
-   clk_rate = clk_round_rate(kms->perf.core_clk, clk_rate);
+   clk_rate = clk_round_rate(kms->perf.core_clk->clk,
+   clk_rate);
}
}
 
@@ -484,15 +496,11 @@ void dpu_core_perf_crtc_update(struct drm_crtc *crtc,
 
DPU_EVT32(kms->dev, stop_req, clk_rate);
 
-   /* Temp change to avoid crash in clk_set_rate API. */
-#ifdef QCOM_DPU_SET_CLK
-   if (dpu_power_clk_set_rate(>phandle,
-  kms->perf.clk_name, clk_rate)) {
+   if (_dpu_core_perf_set_core_clk_rate(kms, clk_rate)) {
DPU_ERROR("failed to set %s clock rate %llu\n",
-   kms->perf.clk_name, clk_rate);
+   kms->perf.core_clk->clk_name, clk_rate);
return;
}
-#endif
 
kms->perf.core_clk_rate = clk_rate;
DPU_DEBUG("update clk rate = %lld HZ\n", clk_rate);
@@ -656,7 +664,6 @@ void dpu_core_perf_destroy(struct dpu_core_perf *perf)
dpu_core_perf_debugfs_destroy(perf);
perf->max_core_clk_rate = 0;
perf->core_clk = NULL;
-   perf->clk_name = NULL;
perf->phandle = NULL;
perf->catalog = NULL;
perf->dev = NULL;
@@ -667,9 +674,9 @@ int dpu_core_perf_init(struct dpu_core_perf *perf,
struct dpu_mdss_cfg *catalog,
struct dpu_power_handle *phandle,
struct dpu_power_client *pclient,
-   char *clk_name)
+   struct dss_clk *core_clk)
 {
-   if (!perf || !dev || !catalog || !phandle || !pclient || !clk_name) {
+   if (!pclient) {
DPU_ERROR("invalid parameters\n");
return -EINVAL;
}
@@ -678,23 +685,13 @@ int dpu_core_perf_init(struct dpu_core_perf *perf,
perf->catalog = catalog;
perf->phandle = phandle;
perf->pclient = pclient;
-   perf->clk_name = clk_name;
-
-   perf->core_clk = dpu_power_clk_get_clk(phandle, clk_name);
-   if