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

[Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]

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 <ryadav@codeaurora.org>
> ---
>  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 <seanpaul@chromium.org>


>  		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_bus_hdl,
>  							max_usecase_ndx);
>  
> -		msm_dss_enable_vreg(mp->vreg_config, mp->num_vreg,
> -							enable);
>  		for (i = 0 ; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++)
>  			dpu_power_data_bus_update(&phandle->data_bus_handle[i],
>  					enable);
> @@ -821,8 +635,6 @@ int dpu_power_resource_enable(struct dpu_power_handle *phandle,
>  	return rc;
>  
>  reg_bus_hdl_err:
> -	msm_dss_enable_vreg(mp->vreg_config, mp->num_vreg, 0);
> -vreg_err:
>  	for (i = 0 ; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++)
>  		dpu_power_data_bus_update(&phandle->data_bus_handle[i], 0);
>  data_bus_hdl_err:
> diff --git a/drivers/gpu/drm/msm/dpu_power_handle.h b/drivers/gpu/drm/msm/dpu_power_handle.h
> index 83f048d..9a6d4b9 100644
> --- a/drivers/gpu/drm/msm/dpu_power_handle.h
> +++ b/drivers/gpu/drm/msm/dpu_power_handle.h
> @@ -147,7 +147,6 @@ struct dpu_power_event {
>  
>  /**
>   * struct dpu_power_handle: power handle main struct
> - * @mp:		module power for clock and regulator
>   * @client_clist: master list to store all clients
>   * @phandle_lock: lock to synchronize the enable/disable
>   * @dev: pointer to device structure
> @@ -157,7 +156,6 @@ struct dpu_power_event {
>   * @event_list: current power handle event list
>   */
>  struct dpu_power_handle {
> -	struct dss_module_power mp;
>  	struct list_head power_client_clist;
>  	struct mutex phandle_lock;
>  	struct device *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
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno