Ver Fonte

disp: msm: sde: avoid multiple frame-done encoder events

Currently there is a race condition in checking the
pending_kickoff_cnt in wr_ptr_irq wait from display-thread
and pp_done_irq from interrupt context. In both places,
pending_kickoff_cnt is read first and modified later. In
partial update cases where the frame-transfer is short,
such a race condition might happen and would lead to both
triggering the frame-done/release fence for the same frame.
Fix it by combining read/modify to one statement in both places.

Change-Id: I9162e7dc3f12af3590514f1ebfd68023aa920181
Signed-off-by: Veera Sundaram Sankaran <[email protected]>
Veera Sundaram Sankaran há 6 anos atrás
pai
commit
b8b095b57d
1 ficheiros alterados com 11 adições e 17 exclusões
  1. 11 17
      msm/sde/sde_encoder_phys_cmd.c

+ 11 - 17
msm/sde/sde_encoder_phys_cmd.c

@@ -174,10 +174,7 @@ static void _sde_encoder_phys_cmd_update_intf_cfg(
 static void sde_encoder_phys_cmd_pp_tx_done_irq(void *arg, int irq_idx)
 {
 	struct sde_encoder_phys *phys_enc = arg;
-	unsigned long lock_flags;
-	int new_cnt;
-	u32 event = SDE_ENCODER_FRAME_EVENT_DONE |
-			SDE_ENCODER_FRAME_EVENT_SIGNAL_RELEASE_FENCE;
+	u32 event = 0;
 
 	if (!phys_enc || !phys_enc->hw_pp)
 		return;
@@ -186,16 +183,15 @@ static void sde_encoder_phys_cmd_pp_tx_done_irq(void *arg, int irq_idx)
 
 	/* notify all synchronous clients first, then asynchronous clients */
 	if (phys_enc->parent_ops.handle_frame_done &&
-	    atomic_read(&phys_enc->pending_kickoff_cnt))
+	    atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0)) {
+		event = SDE_ENCODER_FRAME_EVENT_DONE |
+			SDE_ENCODER_FRAME_EVENT_SIGNAL_RELEASE_FENCE;
 		phys_enc->parent_ops.handle_frame_done(phys_enc->parent,
 				phys_enc, event);
-
-	spin_lock_irqsave(phys_enc->enc_spinlock, lock_flags);
-	new_cnt = atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0);
-	spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
+	}
 
 	SDE_EVT32_IRQ(DRMID(phys_enc->parent),
-			phys_enc->hw_pp->idx - PINGPONG_0, new_cnt, event);
+			phys_enc->hw_pp->idx - PINGPONG_0, event);
 
 	/* Signal any waiting atomic commit thread */
 	wake_up_all(&phys_enc->pending_kickoff_wq);
@@ -1372,13 +1368,11 @@ static int _sde_encoder_phys_cmd_wait_for_wr_ptr(
 				SDE_ENCODER_FRAME_EVENT_SIGNAL_RETIRE_FENCE);
 
 	} else if ((ret == 0) &&
-	    (phys_enc->frame_trigger_mode == FRAME_DONE_WAIT_POSTED_START) &&
-	    atomic_read(&phys_enc->pending_kickoff_cnt) &&
-	    ctl->ops.get_scheduler_status &&
-	    (ctl->ops.get_scheduler_status(ctl) & BIT(0)) &&
-	    phys_enc->parent_ops.handle_frame_done) {
-		atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0);
-
+		(phys_enc->frame_trigger_mode == FRAME_DONE_WAIT_POSTED_START)
+		  && ctl->ops.get_scheduler_status
+		  && (ctl->ops.get_scheduler_status(ctl) & BIT(0))
+		  && atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0)
+		  && phys_enc->parent_ops.handle_frame_done) {
 		phys_enc->parent_ops.handle_frame_done(
 			phys_enc->parent, phys_enc,
 			SDE_ENCODER_FRAME_EVENT_DONE |