diff mbox series

[2/4] drm/msm: dpu: Simplify frame_done watchdog timeout calculation

Message ID 20190128204306.95076-2-sean@poorly.run (mailing list archive)
State New, archived
Headers show
Series [1/4] drm/msm: Use drm_mode_vrefresh instead of mode->vrefresh | expand

Commit Message

Sean Paul Jan. 28, 2019, 8:42 p.m. UTC 
From: Sean Paul <seanpaul@chromium.org>

Instead of setting the timeout and then immediately reading it back
(along with the hand-rolled msecs_to_jiffies calculation), just
calculate it once and set it in both places at the same time.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Abhinav Kumar Jan. 28, 2019, 9:06 p.m. UTC | #1 
On 2019-01-28 12:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Instead of setting the timeout and then immediately reading it back
> (along with the hand-rolled msecs_to_jiffies calculation), just
> calculate it once and set it in both places at the same time.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index dd71cb6ba4f5c..83a4c47dbed2d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1791,6 +1791,7 @@ void dpu_encoder_kickoff(struct drm_encoder
> *drm_enc, bool async)
>  {
>  	struct dpu_encoder_virt *dpu_enc;
>  	struct dpu_encoder_phys *phys;
> +	unsigned long timeout_ms;
>  	ktime_t wakeup_time;
>  	unsigned int i;
> 
> @@ -1803,11 +1804,12 @@ void dpu_encoder_kickoff(struct drm_encoder
> *drm_enc, bool async)
> 
>  	trace_dpu_enc_kickoff(DRMID(drm_enc));
> 
> -	atomic_set(&dpu_enc->frame_done_timeout,
> -			DPU_FRAME_DONE_TIMEOUT * 1000 /
> -			drm_mode_vrefresh(&drm_enc->crtc->state->adjusted_mode));
> -	mod_timer(&dpu_enc->frame_done_timer, jiffies +
> -		((atomic_read(&dpu_enc->frame_done_timeout) * HZ) / 1000));
> +	timeout_ms = DPU_FRAME_DONE_TIMEOUT * 1000 /
> +		drm_mode_vrefresh(&drm_enc->crtc->state->adjusted_mode);
> +
> +	atomic_set(&dpu_enc->frame_done_timeout, timeout_ms);
> +	mod_timer(&dpu_enc->frame_done_timer,
> +		  jiffies + msecs_to_jiffies(timeout_ms));
> 
>  	/* All phys encs are ready to go, trigger the kickoff */
>  	_dpu_encoder_kickoff_phys(dpu_enc, async);
Jeykumar Sankaran Feb. 6, 2019, 7:42 p.m. UTC | #2 
On 2019-01-28 12:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Instead of setting the timeout and then immediately reading it back
> (along with the hand-rolled msecs_to_jiffies calculation), just
> calculate it once and set it in both places at the same time.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index dd71cb6ba4f5c..83a4c47dbed2d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1791,6 +1791,7 @@ void dpu_encoder_kickoff(struct drm_encoder 
> *drm_enc,
> bool async)
>  {
>  	struct dpu_encoder_virt *dpu_enc;
>  	struct dpu_encoder_phys *phys;
> +	unsigned long timeout_ms;
>  	ktime_t wakeup_time;
>  	unsigned int i;
> 
> @@ -1803,11 +1804,12 @@ void dpu_encoder_kickoff(struct drm_encoder
> *drm_enc, bool async)
> 
>  	trace_dpu_enc_kickoff(DRMID(drm_enc));
> 
> -	atomic_set(&dpu_enc->frame_done_timeout,
> -			DPU_FRAME_DONE_TIMEOUT * 1000 /
> -			drm_mode_vrefresh(&drm_enc->crtc->state->adjusted_mode));
> -	mod_timer(&dpu_enc->frame_done_timer, jiffies +
> -		((atomic_read(&dpu_enc->frame_done_timeout) * HZ) / 1000));
> +	timeout_ms = DPU_FRAME_DONE_TIMEOUT * 1000 /
> +		drm_mode_vrefresh(&drm_enc->crtc->state->adjusted_mode);

For future cleanup: Is drm_enc->crtc a valid usage here?

> +
> +	atomic_set(&dpu_enc->frame_done_timeout, timeout_ms);
> +	mod_timer(&dpu_enc->frame_done_timer,
> +		  jiffies + msecs_to_jiffies(timeout_ms));
> 
>  	/* All phys encs are ready to go, trigger the kickoff */
>  	_dpu_encoder_kickoff_phys(dpu_enc, async);

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index dd71cb6ba4f5c..83a4c47dbed2d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1791,6 +1791,7 @@  void dpu_encoder_kickoff(struct drm_encoder *drm_enc, bool async)
 {
 	struct dpu_encoder_virt *dpu_enc;
 	struct dpu_encoder_phys *phys;
+	unsigned long timeout_ms;
 	ktime_t wakeup_time;
 	unsigned int i;
 
@@ -1803,11 +1804,12 @@  void dpu_encoder_kickoff(struct drm_encoder *drm_enc, bool async)
 
 	trace_dpu_enc_kickoff(DRMID(drm_enc));
 
-	atomic_set(&dpu_enc->frame_done_timeout,
-			DPU_FRAME_DONE_TIMEOUT * 1000 /
-			drm_mode_vrefresh(&drm_enc->crtc->state->adjusted_mode));
-	mod_timer(&dpu_enc->frame_done_timer, jiffies +
-		((atomic_read(&dpu_enc->frame_done_timeout) * HZ) / 1000));
+	timeout_ms = DPU_FRAME_DONE_TIMEOUT * 1000 /
+		drm_mode_vrefresh(&drm_enc->crtc->state->adjusted_mode);
+
+	atomic_set(&dpu_enc->frame_done_timeout, timeout_ms);
+	mod_timer(&dpu_enc->frame_done_timer,
+		  jiffies + msecs_to_jiffies(timeout_ms));
 
 	/* All phys encs are ready to go, trigger the kickoff */
 	_dpu_encoder_kickoff_phys(dpu_enc, async);