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

[Sean Paul]

On Fri, May 11, 2018 at 08:19:34PM +0530, Rajesh Yadav wrote:
> Mdss main power supply (mdss_gdsc) is implemented as a
> generic power domain and mdss top level wrapper device
> manage it via runtime_pm. Remove custom power management
> code from dpu_power_handle.
> 
> Changes in v2:
>   - resolved merge conflict in dpu_power_resource_init
>   - dropped (Reviewed-by: Sean Paul) due to above change
> 
> Signed-off-by: Rajesh Yadav 
> ---
>  drivers/gpu/drm/msm/dpu_power_handle.c | 194 
> +
>  drivers/gpu/drm/msm/dpu_power_handle.h |   2 -
>  2 files changed, 3 insertions(+), 193 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dpu_power_handle.c 
> b/drivers/gpu/drm/msm/dpu_power_handle.c
> index 12602ae..77be106 100644
> --- a/drivers/gpu/drm/msm/dpu_power_handle.c
> +++ b/drivers/gpu/drm/msm/dpu_power_handle.c

/snip

> @@ -614,33 +470,18 @@ int dpu_power_resource_init(struct platform_device 
> *pdev,
>   struct dpu_power_handle *phandle)
>  {
>   int rc = 0, i;
> - struct dss_module_power *mp;
>  
>   if (!phandle || !pdev) {

Can this ever happen? It seems like another case of unnecessary checking.

Aside from this,

Reviewed-by: Sean Paul 


>   pr_err("invalid input param\n");
> - rc = -EINVAL;
> - goto end;
> - }
> - mp = &phandle->mp;
> - phandle->dev = &pdev->dev;
> -
> - rc = dpu_power_parse_dt_supply(pdev, mp);
> - if (rc) {
> - pr_err("device vreg supply parsing failed\n");
> - return rc;
> + return -EINVAL;
>   }
>  
> - rc = msm_dss_config_vreg(&pdev->dev,
> - mp->vreg_config, mp->num_vreg, 1);
> - if (rc) {
> - pr_err("vreg config failed rc=%d\n", rc);
> - goto vreg_err;
> - }
> + phandle->dev = &pdev->dev;
>  
>   rc = dpu_power_reg_bus_parse(pdev, phandle);
>   if (rc) {
>   pr_err("register bus parse failed rc=%d\n", rc);
> - goto bus_err;
> + return rc;
>   }
>  
>   for (i = DPU_POWER_HANDLE_DBUS_ID_MNOC;
> @@ -666,19 +507,12 @@ int dpu_power_resource_init(struct platform_device 
> *pdev,
>   for (i--; i >= 0; i--)
>   dpu_power_data_bus_unregister(&phandle->data_bus_handle[i]);
>   dpu_power_reg_bus_unregister(phandle->reg_bus_hdl);
> -bus_err:
> - msm_dss_config_vreg(&pdev->dev, mp->vreg_config, mp->num_vreg, 0);
> -vreg_err:
> - if (mp->vreg_config)
> - devm_kfree(&pdev->dev, mp->vreg_config);
> - mp->num_vreg = 0;
>   return rc;
>  }
>  
>  void dpu_power_resource_deinit(struct platform_device *pdev,
>   struct dpu_power_handle *phandle)
>  {
> - struct dss_module_power *mp;
>   struct dpu_power_client *curr_client, *next_client;
>   struct dpu_power_event *curr_event, *next_event;
>   int i;
> @@ -687,7 +521,6 @@ void dpu_power_resource_deinit(struct platform_device 
> *pdev,
>   pr_err("invalid input param\n");
>   return;
>   }
> - mp = &phandle->mp;
>  
>   mutex_lock(&phandle->phandle_lock);
>   list_for_each_entry_safe(curr_client, next_client,
> @@ -713,13 +546,6 @@ void dpu_power_resource_deinit(struct platform_device 
> *pdev,
>   dpu_power_data_bus_unregister(&phandle->data_bus_handle[i]);
>  
>   dpu_power_reg_bus_unregister(phandle->reg_bus_hdl);
> -
> - msm_dss_config_vreg(&pdev->dev, mp->vreg_config, mp->num_vreg, 0);
> -
> - if (mp->vreg_config)
> - devm_kfree(&pdev->dev, mp->vreg_config);
> -
> - mp->num_vreg = 0;
>  }
>  
>  int dpu_power_resource_enable(struct dpu_power_handle *phandle,
> @@ -729,15 +555,12 @@ int dpu_power_resource_enable(struct dpu_power_handle 
> *phandle,
>   bool changed = false;
>   u32 max_usecase_ndx = VOTE_INDEX_DISABLE, prev_usecase_ndx;
>   struct dpu_power_client *client;
> - struct dss_module_power *mp;
>  
>   if (!phandle || !pclient) {
>   pr_err("invalid input argument\n");
>   return -EINVAL;
>   }
>  
> - mp = &phandle->mp;
> -
>   mutex_lock(&phandle->phandle_lock);
>   if (enable)
>   pclient->refcount++;
> @@ -782,13 +605,6 @@ int dpu_power_resource_enable(struct dpu_power_handle 
> *phandle,
>   }
>   }
>  
> - rc = msm_dss_enable_vreg(mp->vreg_config, mp->num_vreg,
> - enable);
> - if (rc) {
> - pr_err("failed to enable vregs rc=%d\n", rc);
> - goto vreg_err;
> - }
> -
>   rc = dpu_power_reg_bus_update(phandle->reg_bus_hdl,
>   max_usecase_ndx);
>   if (rc) {
> @@ -806,8 +622,6 @@ int dpu_power_resource_enable(struct dpu_power_handle 
> *phandle,
>   dpu_power_reg_bus_update(phandle->reg_bu

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

[Rajesh Yadav]

Mdss main power supply (mdss_gdsc) is implemented as a
generic power domain and mdss top level wrapper device
manage it via runtime_pm. Remove custom power management
code from dpu_power_handle.

Changes in v2:
- resolved merge conflict in dpu_power_resource_init
- dropped (Reviewed-by: Sean Paul) due to above change

Signed-off-by: Rajesh Yadav 
---
 drivers/gpu/drm/msm/dpu_power_handle.c | 194 +
 drivers/gpu/drm/msm/dpu_power_handle.h |   2 -
 2 files changed, 3 insertions(+), 193 deletions(-)

diff --git a/drivers/gpu/drm/msm/dpu_power_handle.c 
b/drivers/gpu/drm/msm/dpu_power_handle.c
index 12602ae..77be106 100644
--- a/drivers/gpu/drm/msm/dpu_power_handle.c
+++ b/drivers/gpu/drm/msm/dpu_power_handle.c
@@ -101,150 +101,6 @@ void dpu_power_client_destroy(struct dpu_power_handle 
*phandle,
}
 }
 
-static int dpu_power_parse_dt_supply(struct platform_device *pdev,
-   struct dss_module_power *mp)
-{
-   int i = 0, rc = 0;
-   u32 tmp = 0;
-   struct device_node *of_node = NULL, *supply_root_node = NULL;
-   struct device_node *supply_node = NULL;
-
-   if (!pdev || !mp) {
-   pr_err("invalid input param pdev:%pK mp:%pK\n", pdev, mp);
-   return -EINVAL;
-   }
-
-   of_node = pdev->dev.of_node;
-
-   mp->num_vreg = 0;
-   supply_root_node = of_get_child_by_name(of_node,
-   "qcom,platform-supply-entries");
-   if (!supply_root_node) {
-   pr_debug("no supply entry present\n");
-   return rc;
-   }
-
-   for_each_child_of_node(supply_root_node, supply_node)
-   mp->num_vreg++;
-
-   if (mp->num_vreg == 0) {
-   pr_debug("no vreg\n");
-   return rc;
-   }
-
-   pr_debug("vreg found. count=%d\n", mp->num_vreg);
-   mp->vreg_config = devm_kzalloc(&pdev->dev, sizeof(struct dss_vreg) *
-   mp->num_vreg, GFP_KERNEL);
-   if (!mp->vreg_config) {
-   rc = -ENOMEM;
-   return rc;
-   }
-
-   for_each_child_of_node(supply_root_node, supply_node) {
-
-   const char *st = NULL;
-
-   rc = of_property_read_string(supply_node,
-   "qcom,supply-name", &st);
-   if (rc) {
-   pr_err("error reading name. rc=%d\n", rc);
-   goto error;
-   }
-
-   strlcpy(mp->vreg_config[i].vreg_name, st,
-   sizeof(mp->vreg_config[i].vreg_name));
-
-   rc = of_property_read_u32(supply_node,
-   "qcom,supply-min-voltage", &tmp);
-   if (rc) {
-   pr_err("error reading min volt. rc=%d\n", rc);
-   goto error;
-   }
-   mp->vreg_config[i].min_voltage = tmp;
-
-   rc = of_property_read_u32(supply_node,
-   "qcom,supply-max-voltage", &tmp);
-   if (rc) {
-   pr_err("error reading max volt. rc=%d\n", rc);
-   goto error;
-   }
-   mp->vreg_config[i].max_voltage = tmp;
-
-   rc = of_property_read_u32(supply_node,
-   "qcom,supply-enable-load", &tmp);
-   if (rc) {
-   pr_err("error reading enable load. rc=%d\n", rc);
-   goto error;
-   }
-   mp->vreg_config[i].enable_load = tmp;
-
-   rc = of_property_read_u32(supply_node,
-   "qcom,supply-disable-load", &tmp);
-   if (rc) {
-   pr_err("error reading disable load. rc=%d\n", rc);
-   goto error;
-   }
-   mp->vreg_config[i].disable_load = tmp;
-
-   rc = of_property_read_u32(supply_node,
-   "qcom,supply-pre-on-sleep", &tmp);
-   if (rc)
-   pr_debug("error reading supply pre sleep value. 
rc=%d\n",
-   rc);
-
-   mp->vreg_config[i].pre_on_sleep = (!rc ? tmp : 0);
-
-   rc = of_property_read_u32(supply_node,
-   "qcom,supply-pre-off-sleep", &tmp);
-   if (rc)
-   pr_debug("error reading supply pre sleep value. 
rc=%d\n",
-   rc);
-
-   mp->vreg_config[i].pre_off_sleep = (!rc ? tmp : 0);
-
-   rc = of_property_read_u32(supply_node,
-   "qcom,supply-post-on-sleep", &tmp);
-   if (rc)
-   pr_debug("error reading supply post sle