diff mbox

[DPU] drm/msm: Add pm_runtime_get/put calls to dpu

Message ID 20180314150718.254814-1-seanpaul@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Sean Paul March 14, 2018, 3:07 p.m. UTC 
Ensure that pm_runtime is properly referenced/unreferenced when we need
it.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---

Didn't get a response to my suggestion, so wrote the patch anyways.
Thoughts?


 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c |  3 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  |  2 ++
 drivers/gpu/drm/msm/dpu_power_handle.c   | 12 ++++++++----
 3 files changed, 13 insertions(+), 4 deletions(-)

Comments

Sravanthi Kollukuduru March 15, 2018, 2:42 p.m. UTC | #1 
On 2018-03-14 20:37, Sean Paul wrote:
> Ensure that pm_runtime is properly referenced/unreferenced when we need
> it.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> 
> Didn't get a response to my suggestion, so wrote the patch anyways.
> Thoughts?
> 

This patch looks fine for now.
The plan is to deprecate the downstream power_handler APIs going forward 
and rely entirely on runtime pm APIs.

Currently, the pm_runtime_get/put API is used to control the MDSS main 
power supply (which is defined as a generic power domain). The idea is 
to extend the runtime_pm suspend/resume handlers to manage clock and bus 
resources so that the power_handler code can be trimmed off.

Thanks,
Sravanthi

> 
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c |  3 +++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  |  2 ++
>  drivers/gpu/drm/msm/dpu_power_handle.c   | 12 ++++++++----
>  3 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index f1642d72469e..df6cbeb15cf5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -3497,6 +3497,7 @@ static void dpu_crtc_disable(struct drm_crtc 
> *crtc)
>  	/* disable clk & bw control until clk & bw properties are set */
>  	cstate->bw_control = false;
>  	cstate->bw_split_vote = false;
> +	pm_runtime_put_sync(crtc->dev->dev);
> 
>  	mutex_unlock(&dpu_crtc->crtc_lock);
>  }
> @@ -3523,6 +3524,8 @@ static void dpu_crtc_enable(struct drm_crtc 
> *crtc,
>  	DPU_EVT32_VERBOSE(DRMID(crtc));
>  	dpu_crtc = to_dpu_crtc(crtc);
> 
> +	pm_runtime_get_sync(crtc->dev->dev);
> +
>  	drm_for_each_encoder(encoder, crtc->dev) {
>  		if (encoder->crtc != crtc)
>  			continue;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index fb4de59d8ed1..90608a303aec 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -346,12 +346,14 @@ static void _dpu_debugfs_destroy(struct dpu_kms 
> *dpu_kms)
> 
>  static int dpu_kms_enable_vblank(struct msm_kms *kms, struct drm_crtc 
> *crtc)
>  {
> +	pm_runtime_get_sync(crtc->dev->dev);
>  	return dpu_crtc_vblank(crtc, true);
>  }
> 
>  static void dpu_kms_disable_vblank(struct msm_kms *kms, struct 
> drm_crtc *crtc)
>  {
>  	dpu_crtc_vblank(crtc, false);
> +	pm_runtime_put_sync(crtc->dev->dev);
>  }
> 
>  static void dpu_kms_wait_for_frame_transfer_complete(struct msm_kms 
> *kms,
> diff --git a/drivers/gpu/drm/msm/dpu_power_handle.c
> b/drivers/gpu/drm/msm/dpu_power_handle.c
> index 477ea9f2778c..a52be861117f 100644
> --- a/drivers/gpu/drm/msm/dpu_power_handle.c
> +++ b/drivers/gpu/drm/msm/dpu_power_handle.c
> @@ -18,6 +18,7 @@
>  #include <linux/of.h>
>  #include <linux/string.h>
>  #include <linux/of_address.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/slab.h>
>  #include <linux/mutex.h>
>  #include <linux/of_platform.h>
> @@ -857,6 +858,9 @@ int dpu_power_resource_enable(struct
> dpu_power_handle *phandle,
>  		return -EINVAL;
>  	}
> 
> +	if (enable)
> +		pm_runtime_get_sync(phandle->dev);
> +
>  	mp = &phandle->mp;
> 
>  	mutex_lock(&phandle->phandle_lock);
> @@ -963,10 +967,6 @@ int dpu_power_resource_enable(struct
> dpu_power_handle *phandle,
>  				DPU_POWER_EVENT_POST_DISABLE);
>  	}
> 
> -end:
> -	mutex_unlock(&phandle->phandle_lock);
> -	return rc;
> -
>  clk_err:
>  	dpu_power_rsc_update(phandle, false);
>  rsc_err:
> @@ -979,7 +979,11 @@ int dpu_power_resource_enable(struct
> dpu_power_handle *phandle,
>  		dpu_power_data_bus_update(&phandle->data_bus_handle[i], 0);
>  data_bus_hdl_err:
>  	phandle->current_usecase_ndx = prev_usecase_ndx;
> +
> +end:
>  	mutex_unlock(&phandle->phandle_lock);
> +	if (!enable)
> +		pm_runtime_put_sync(phandle->dev);
>  	return rc;
>  }
Sravanthi Kollukuduru March 15, 2018, 2:55 p.m. UTC | #2 
On 2018-03-15 20:12, skolluku@codeaurora.org wrote:
> On 2018-03-14 20:37, Sean Paul wrote:
>> Ensure that pm_runtime is properly referenced/unreferenced when we 
>> need
>> it.
>> 
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
>> ---
>> 
>> Didn't get a response to my suggestion, so wrote the patch anyways.
>> Thoughts?
>> 
> 
> This patch looks fine for now.
> The plan is to deprecate the downstream power_handler APIs going
> forward and rely entirely on runtime pm APIs.
> 
> Currently, the pm_runtime_get/put API is used to control the MDSS main
> power supply (which is defined as a generic power domain). The idea is
> to extend the runtime_pm suspend/resume handlers to manage clock and
> bus resources so that the power_handler code can be trimmed off.
> 
> Thanks,
> Sravanthi
> 
>> 
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c |  3 +++
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  |  2 ++
>>  drivers/gpu/drm/msm/dpu_power_handle.c   | 12 ++++++++----
>>  3 files changed, 13 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> index f1642d72469e..df6cbeb15cf5 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -3497,6 +3497,7 @@ static void dpu_crtc_disable(struct drm_crtc 
>> *crtc)
>>  	/* disable clk & bw control until clk & bw properties are set */
>>  	cstate->bw_control = false;
>>  	cstate->bw_split_vote = false;
>> +	pm_runtime_put_sync(crtc->dev->dev);
>> 
>>  	mutex_unlock(&dpu_crtc->crtc_lock);
>>  }
>> @@ -3523,6 +3524,8 @@ static void dpu_crtc_enable(struct drm_crtc 
>> *crtc,
>>  	DPU_EVT32_VERBOSE(DRMID(crtc));
>>  	dpu_crtc = to_dpu_crtc(crtc);
>> 
>> +	pm_runtime_get_sync(crtc->dev->dev);
>> +
>>  	drm_for_each_encoder(encoder, crtc->dev) {
>>  		if (encoder->crtc != crtc)
>>  			continue;
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> index fb4de59d8ed1..90608a303aec 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> @@ -346,12 +346,14 @@ static void _dpu_debugfs_destroy(struct dpu_kms 
>> *dpu_kms)
>> 
>>  static int dpu_kms_enable_vblank(struct msm_kms *kms, struct drm_crtc 
>> *crtc)
>>  {
>> +	pm_runtime_get_sync(crtc->dev->dev);
>>  	return dpu_crtc_vblank(crtc, true);
>>  }
>> 
>>  static void dpu_kms_disable_vblank(struct msm_kms *kms, struct 
>> drm_crtc *crtc)
>>  {
>>  	dpu_crtc_vblank(crtc, false);
>> +	pm_runtime_put_sync(crtc->dev->dev);
>>  }
>> 
>>  static void dpu_kms_wait_for_frame_transfer_complete(struct msm_kms 
>> *kms,
>> diff --git a/drivers/gpu/drm/msm/dpu_power_handle.c
>> b/drivers/gpu/drm/msm/dpu_power_handle.c
>> index 477ea9f2778c..a52be861117f 100644
>> --- a/drivers/gpu/drm/msm/dpu_power_handle.c
>> +++ b/drivers/gpu/drm/msm/dpu_power_handle.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/of.h>
>>  #include <linux/string.h>
>>  #include <linux/of_address.h>
>> +#include <linux/pm_runtime.h>
>>  #include <linux/slab.h>
>>  #include <linux/mutex.h>
>>  #include <linux/of_platform.h>
>> @@ -857,6 +858,9 @@ int dpu_power_resource_enable(struct
>> dpu_power_handle *phandle,
>>  		return -EINVAL;
>>  	}
>> 
>> +	if (enable)
>> +		pm_runtime_get_sync(phandle->dev);
>> +
>>  	mp = &phandle->mp;
>> 
>>  	mutex_lock(&phandle->phandle_lock);
>> @@ -963,10 +967,6 @@ int dpu_power_resource_enable(struct
>> dpu_power_handle *phandle,
>>  				DPU_POWER_EVENT_POST_DISABLE);
>>  	}
>> 
>> -end:
>> -	mutex_unlock(&phandle->phandle_lock);
>> -	return rc;
>> -
>>  clk_err:
>>  	dpu_power_rsc_update(phandle, false);
>>  rsc_err:
>> @@ -979,7 +979,11 @@ int dpu_power_resource_enable(struct
>> dpu_power_handle *phandle,
>>  		dpu_power_data_bus_update(&phandle->data_bus_handle[i], 0);
>>  data_bus_hdl_err:
>>  	phandle->current_usecase_ndx = prev_usecase_ndx;
>> +
>> +end:
>>  	mutex_unlock(&phandle->phandle_lock);
>> +	if (!enable)
>> +		pm_runtime_put_sync(phandle->dev);
>>  	return rc;
>>  }
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index f1642d72469e..df6cbeb15cf5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -3497,6 +3497,7 @@  static void dpu_crtc_disable(struct drm_crtc *crtc)
 	/* disable clk & bw control until clk & bw properties are set */
 	cstate->bw_control = false;
 	cstate->bw_split_vote = false;
+	pm_runtime_put_sync(crtc->dev->dev);
 
 	mutex_unlock(&dpu_crtc->crtc_lock);
 }
@@ -3523,6 +3524,8 @@  static void dpu_crtc_enable(struct drm_crtc *crtc,
 	DPU_EVT32_VERBOSE(DRMID(crtc));
 	dpu_crtc = to_dpu_crtc(crtc);
 
+	pm_runtime_get_sync(crtc->dev->dev);
+
 	drm_for_each_encoder(encoder, crtc->dev) {
 		if (encoder->crtc != crtc)
 			continue;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index fb4de59d8ed1..90608a303aec 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -346,12 +346,14 @@  static void _dpu_debugfs_destroy(struct dpu_kms *dpu_kms)
 
 static int dpu_kms_enable_vblank(struct msm_kms *kms, struct drm_crtc *crtc)
 {
+	pm_runtime_get_sync(crtc->dev->dev);
 	return dpu_crtc_vblank(crtc, true);
 }
 
 static void dpu_kms_disable_vblank(struct msm_kms *kms, struct drm_crtc *crtc)
 {
 	dpu_crtc_vblank(crtc, false);
+	pm_runtime_put_sync(crtc->dev->dev);
 }
 
 static void dpu_kms_wait_for_frame_transfer_complete(struct msm_kms *kms,
diff --git a/drivers/gpu/drm/msm/dpu_power_handle.c b/drivers/gpu/drm/msm/dpu_power_handle.c
index 477ea9f2778c..a52be861117f 100644
--- a/drivers/gpu/drm/msm/dpu_power_handle.c
+++ b/drivers/gpu/drm/msm/dpu_power_handle.c
@@ -18,6 +18,7 @@ 
 #include <linux/of.h>
 #include <linux/string.h>
 #include <linux/of_address.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
 #include <linux/of_platform.h>
@@ -857,6 +858,9 @@  int dpu_power_resource_enable(struct dpu_power_handle *phandle,
 		return -EINVAL;
 	}
 
+	if (enable)
+		pm_runtime_get_sync(phandle->dev);
+
 	mp = &phandle->mp;
 
 	mutex_lock(&phandle->phandle_lock);
@@ -963,10 +967,6 @@  int dpu_power_resource_enable(struct dpu_power_handle *phandle,
 				DPU_POWER_EVENT_POST_DISABLE);
 	}
 
-end:
-	mutex_unlock(&phandle->phandle_lock);
-	return rc;
-
 clk_err:
 	dpu_power_rsc_update(phandle, false);
 rsc_err:
@@ -979,7 +979,11 @@  int dpu_power_resource_enable(struct dpu_power_handle *phandle,
 		dpu_power_data_bus_update(&phandle->data_bus_handle[i], 0);
 data_bus_hdl_err:
 	phandle->current_usecase_ndx = prev_usecase_ndx;
+
+end:
 	mutex_unlock(&phandle->phandle_lock);
+	if (!enable)
+		pm_runtime_put_sync(phandle->dev);
 	return rc;
 }