Commit 70df9610 authored by Sean Paul's avatar Sean Paul Committed by Rob Clark

drm/msm: dpu: Untangle frame_done timeout units

There exists a bunch of confusion as to what the actual units of
frame_done is:

- The definition states it's in # of frames
- CRTC treats it like it's ms
- frame_done_timeout comment thinks it's Hz, but it stores ms
- frame_done timer is setup such that it _should_ be in frames, but the
  timeout is super long

So this patch tries to interpret what the driver really wants. I've
de-centralized the #define since the consumers are expecting different
units.

For crtc, we just use 60ms since that's what it was doing before.
Perhaps we could get fancy and scale with vrefresh, but that's for
another time.

For encoder, fix the comments and rename frame_done_timeout so it's
obvious what the units are. In practice, frame_done_timeout is really
just checked against 0 || !0, which I guess is why the units being wrong
didn't matter. I've also dropped the timeout from the previous 60 frames
to 5. That seems like more than enough time to give up on a frame, and
my guess is that no one intended for the timeout to _actually_ be 60
frames.
Reviewed-by: default avatarFritz Koenig <frkoenig@google.com>
Signed-off-by: default avatarSean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20190128204306.95076-3-sean@poorly.runSigned-off-by: default avatarRob Clark <robdclark@chromium.org>
parent 2e039186
...@@ -46,6 +46,9 @@ ...@@ -46,6 +46,9 @@
#define LEFT_MIXER 0 #define LEFT_MIXER 0
#define RIGHT_MIXER 1 #define RIGHT_MIXER 1
/* timeout in ms waiting for frame done */
#define DPU_CRTC_FRAME_DONE_TIMEOUT_MS 60
static struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc) static struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
{ {
struct msm_drm_private *priv = crtc->dev->dev_private; struct msm_drm_private *priv = crtc->dev->dev_private;
...@@ -683,7 +686,7 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc) ...@@ -683,7 +686,7 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc)
DPU_ATRACE_BEGIN("frame done completion wait"); DPU_ATRACE_BEGIN("frame done completion wait");
ret = wait_for_completion_timeout(&dpu_crtc->frame_done_comp, ret = wait_for_completion_timeout(&dpu_crtc->frame_done_comp,
msecs_to_jiffies(DPU_FRAME_DONE_TIMEOUT)); msecs_to_jiffies(DPU_CRTC_FRAME_DONE_TIMEOUT_MS));
if (!ret) { if (!ret) {
DRM_ERROR("frame done wait timed out, ret:%d\n", ret); DRM_ERROR("frame done wait timed out, ret:%d\n", ret);
rc = -ETIMEDOUT; rc = -ETIMEDOUT;
......
...@@ -69,6 +69,9 @@ ...@@ -69,6 +69,9 @@
#define MAX_VDISPLAY_SPLIT 1080 #define MAX_VDISPLAY_SPLIT 1080
/* timeout in frames waiting for frame done */
#define DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES 5
/** /**
* enum dpu_enc_rc_events - events for resource control state machine * enum dpu_enc_rc_events - events for resource control state machine
* @DPU_ENC_RC_EVENT_KICKOFF: * @DPU_ENC_RC_EVENT_KICKOFF:
...@@ -158,7 +161,7 @@ enum dpu_enc_rc_states { ...@@ -158,7 +161,7 @@ enum dpu_enc_rc_states {
* Bit0 = phys_encs[0] etc. * Bit0 = phys_encs[0] etc.
* @crtc_frame_event_cb: callback handler for frame event * @crtc_frame_event_cb: callback handler for frame event
* @crtc_frame_event_cb_data: callback handler private data * @crtc_frame_event_cb_data: callback handler private data
* @frame_done_timeout: frame done timeout in Hz * @frame_done_timeout_ms: frame done timeout in ms
* @frame_done_timer: watchdog timer for frame done event * @frame_done_timer: watchdog timer for frame done event
* @vsync_event_timer: vsync timer * @vsync_event_timer: vsync timer
* @disp_info: local copy of msm_display_info struct * @disp_info: local copy of msm_display_info struct
...@@ -196,7 +199,7 @@ struct dpu_encoder_virt { ...@@ -196,7 +199,7 @@ struct dpu_encoder_virt {
void (*crtc_frame_event_cb)(void *, u32 event); void (*crtc_frame_event_cb)(void *, u32 event);
void *crtc_frame_event_cb_data; void *crtc_frame_event_cb_data;
atomic_t frame_done_timeout; atomic_t frame_done_timeout_ms;
struct timer_list frame_done_timer; struct timer_list frame_done_timer;
struct timer_list vsync_event_timer; struct timer_list vsync_event_timer;
...@@ -1182,7 +1185,7 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc) ...@@ -1182,7 +1185,7 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
} }
/* after phys waits for frame-done, should be no more frames pending */ /* after phys waits for frame-done, should be no more frames pending */
if (atomic_xchg(&dpu_enc->frame_done_timeout, 0)) { if (atomic_xchg(&dpu_enc->frame_done_timeout_ms, 0)) {
DPU_ERROR("enc%d timeout pending\n", drm_enc->base.id); DPU_ERROR("enc%d timeout pending\n", drm_enc->base.id);
del_timer_sync(&dpu_enc->frame_done_timer); del_timer_sync(&dpu_enc->frame_done_timer);
} }
...@@ -1339,7 +1342,7 @@ static void dpu_encoder_frame_done_callback( ...@@ -1339,7 +1342,7 @@ static void dpu_encoder_frame_done_callback(
} }
if (!dpu_enc->frame_busy_mask[0]) { if (!dpu_enc->frame_busy_mask[0]) {
atomic_set(&dpu_enc->frame_done_timeout, 0); atomic_set(&dpu_enc->frame_done_timeout_ms, 0);
del_timer(&dpu_enc->frame_done_timer); del_timer(&dpu_enc->frame_done_timer);
dpu_encoder_resource_control(drm_enc, dpu_encoder_resource_control(drm_enc,
...@@ -1801,10 +1804,10 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc, bool async) ...@@ -1801,10 +1804,10 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc, bool async)
trace_dpu_enc_kickoff(DRMID(drm_enc)); trace_dpu_enc_kickoff(DRMID(drm_enc));
timeout_ms = DPU_FRAME_DONE_TIMEOUT * 1000 / timeout_ms = DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES * 1000 /
drm_mode_vrefresh(&drm_enc->crtc->state->adjusted_mode); drm_mode_vrefresh(&drm_enc->crtc->state->adjusted_mode);
atomic_set(&dpu_enc->frame_done_timeout, timeout_ms); atomic_set(&dpu_enc->frame_done_timeout_ms, timeout_ms);
mod_timer(&dpu_enc->frame_done_timer, mod_timer(&dpu_enc->frame_done_timer,
jiffies + msecs_to_jiffies(timeout_ms)); jiffies + msecs_to_jiffies(timeout_ms));
...@@ -2126,7 +2129,7 @@ static void dpu_encoder_frame_done_timeout(struct timer_list *t) ...@@ -2126,7 +2129,7 @@ static void dpu_encoder_frame_done_timeout(struct timer_list *t)
DRM_DEBUG_KMS("id:%u invalid timeout frame_busy_mask=%lu\n", DRM_DEBUG_KMS("id:%u invalid timeout frame_busy_mask=%lu\n",
DRMID(drm_enc), dpu_enc->frame_busy_mask[0]); DRMID(drm_enc), dpu_enc->frame_busy_mask[0]);
return; return;
} else if (!atomic_xchg(&dpu_enc->frame_done_timeout, 0)) { } else if (!atomic_xchg(&dpu_enc->frame_done_timeout_ms, 0)) {
DRM_DEBUG_KMS("id:%u invalid timeout\n", DRMID(drm_enc)); DRM_DEBUG_KMS("id:%u invalid timeout\n", DRMID(drm_enc));
return; return;
} }
...@@ -2172,7 +2175,7 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, ...@@ -2172,7 +2175,7 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
spin_lock_init(&dpu_enc->enc_spinlock); spin_lock_init(&dpu_enc->enc_spinlock);
atomic_set(&dpu_enc->frame_done_timeout, 0); atomic_set(&dpu_enc->frame_done_timeout_ms, 0);
timer_setup(&dpu_enc->frame_done_timer, timer_setup(&dpu_enc->frame_done_timer,
dpu_encoder_frame_done_timeout, 0); dpu_encoder_frame_done_timeout, 0);
......
...@@ -73,9 +73,6 @@ ...@@ -73,9 +73,6 @@
#define DPU_NAME_SIZE 12 #define DPU_NAME_SIZE 12
/* timeout in frames waiting for frame done */
#define DPU_FRAME_DONE_TIMEOUT 60
/* /*
* struct dpu_irq_callback - IRQ callback handlers * struct dpu_irq_callback - IRQ callback handlers
* @list: list to callback * @list: list to callback
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment