Re: [PATCH v2 2/3] drm/msm: move suspend/resume devfreq to their own functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, May 31, 2018 at 12:52:03PM +0530, Sharat Masetty wrote:
> This is needed for hardware revisions which do not rely on the generic
> suspend, resume handlers for power management.
> 
> Signed-off-by: Sharat Masetty <smasetty@xxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/msm/msm_gpu.c | 23 +++++++++++++++--------
>  drivers/gpu/drm/msm/msm_gpu.h |  2 ++
>  2 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index d7586f2..c5d4627 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -203,6 +203,19 @@ static int disable_axi(struct msm_gpu *gpu)
>  	return 0;
>  }
>  
> +void msm_gpu_resume_devfreq(struct msm_gpu *gpu)
> +{
> +	gpu->devfreq.busy_cycles = 0;
> +	gpu->devfreq.time = ktime_get();
> +
> +	devfreq_resume_device(gpu->devfreq.devfreq);
> +}

I'm on the fence about this - it isn't strictly needed because it would be fine
for the 6xx code to duplicate three lines of code but it does make it easier to
update the statistics in one place. If others feel more strongly, chime in.

> +void msm_gpu_suspend_devfreq(struct msm_gpu *gpu)
> +{
> +	devfreq_suspend_device(gpu->devfreq.devfreq);
> +}

This on the other hand is entirely not needed. When 6xx does its thing it can
just call devfreq_suspend_device directly. It is doubtful we will ever need
anything more than this one line to stop devfreq.

>  int msm_gpu_pm_resume(struct msm_gpu *gpu)
>  {
>  	int ret;
> @@ -221,12 +234,7 @@ int msm_gpu_pm_resume(struct msm_gpu *gpu)
>  	if (ret)
>  		return ret;
>  
> -	if (gpu->devfreq.devfreq) {
> -		gpu->devfreq.busy_cycles = 0;
> -		gpu->devfreq.time = ktime_get();
> -
> -		devfreq_resume_device(gpu->devfreq.devfreq);
> -	}
> +	msm_gpu_resume_devfreq(gpu);
>  
>  	gpu->needs_hw_init = true;
>  
> @@ -239,8 +247,7 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu)
>  
>  	DBG("%s", gpu->name);
>  
> -	if (gpu->devfreq.devfreq)
> -		devfreq_suspend_device(gpu->devfreq.devfreq);
> +	msm_gpu_suspend_devfreq(gpu);
>  
>  	ret = disable_axi(gpu);
>  	if (ret)
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index b824117..1876b81 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -226,6 +226,8 @@ static inline void gpu_write64(struct msm_gpu *gpu, u32 lo, u32 hi, u64 val)
>  
>  int msm_gpu_pm_suspend(struct msm_gpu *gpu);
>  int msm_gpu_pm_resume(struct msm_gpu *gpu);
> +void msm_gpu_resume_devfreq(struct msm_gpu *gpu);
> +void msm_gpu_suspend_devfreq(struct msm_gpu *gpu);
>  
>  int msm_gpu_hw_init(struct msm_gpu *gpu);
>  
> -- 
> 1.9.1

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux