Skip to content

Commit 230ca8f

Browse files
Laurent Pinchartgregkh
authored andcommitted
drm: rcar-du: Fix race condition when disabling planes at CRTC stop
commit 641307d upstream. When stopping the CRTC the driver must disable all planes and wait for the change to take effect at the next vblank. Merely calling drm_crtc_wait_one_vblank() is not enough, as the function doesn't include any mechanism to handle the race with vblank interrupts. Replace the drm_crtc_wait_one_vblank() call with a manual mechanism that handles the vblank interrupt race. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Signed-off-by: thongsyho <thong.ho.px@rvc.renesas.com> Signed-off-by: Nhan Nguyen <nhan.nguyen.yb@renesas.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 758e22a commit 230ca8f

2 files changed

Lines changed: 55 additions & 6 deletions

File tree

drivers/gpu/drm/rcar-du/rcar_du_crtc.c

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,31 @@ static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc)
392392
rcrtc->started = true;
393393
}
394394

395+
static void rcar_du_crtc_disable_planes(struct rcar_du_crtc *rcrtc)
396+
{
397+
struct rcar_du_device *rcdu = rcrtc->group->dev;
398+
struct drm_crtc *crtc = &rcrtc->crtc;
399+
u32 status;
400+
/* Make sure vblank interrupts are enabled. */
401+
drm_crtc_vblank_get(crtc);
402+
/*
403+
* Disable planes and calculate how many vertical blanking interrupts we
404+
* have to wait for. If a vertical blanking interrupt has been triggered
405+
* but not processed yet, we don't know whether it occurred before or
406+
* after the planes got disabled. We thus have to wait for two vblank
407+
* interrupts in that case.
408+
*/
409+
spin_lock_irq(&rcrtc->vblank_lock);
410+
rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
411+
status = rcar_du_crtc_read(rcrtc, DSSR);
412+
rcrtc->vblank_count = status & DSSR_VBK ? 2 : 1;
413+
spin_unlock_irq(&rcrtc->vblank_lock);
414+
if (!wait_event_timeout(rcrtc->vblank_wait, rcrtc->vblank_count == 0,
415+
msecs_to_jiffies(100)))
416+
dev_warn(rcdu->dev, "vertical blanking timeout\n");
417+
drm_crtc_vblank_put(crtc);
418+
}
419+
395420
static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
396421
{
397422
struct drm_crtc *crtc = &rcrtc->crtc;
@@ -400,17 +425,16 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
400425
return;
401426

402427
/* Disable all planes and wait for the change to take effect. This is
403-
* required as the DSnPR registers are updated on vblank, and no vblank
404-
* will occur once the CRTC is stopped. Disabling planes when starting
405-
* the CRTC thus wouldn't be enough as it would start scanning out
406-
* immediately from old frame buffers until the next vblank.
428+
* required as the plane enable registers are updated on vblank, and no
429+
* vblank will occur once the CRTC is stopped. Disabling planes when
430+
* starting the CRTC thus wouldn't be enough as it would start scanning
431+
* out immediately from old frame buffers until the next vblank.
407432
*
408433
* This increases the CRTC stop delay, especially when multiple CRTCs
409434
* are stopped in one operation as we now wait for one vblank per CRTC.
410435
* Whether this can be improved needs to be researched.
411436
*/
412-
rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
413-
drm_crtc_wait_one_vblank(crtc);
437+
rcar_du_crtc_disable_planes(rcrtc);
414438

415439
/* Disable vertical blanking interrupt reporting. We first need to wait
416440
* for page flip completion before stopping the CRTC as userspace
@@ -548,9 +572,24 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
548572
irqreturn_t ret = IRQ_NONE;
549573
u32 status;
550574

575+
spin_lock(&rcrtc->vblank_lock);
576+
551577
status = rcar_du_crtc_read(rcrtc, DSSR);
552578
rcar_du_crtc_write(rcrtc, DSRCR, status & DSRCR_MASK);
553579

580+
if (status & DSSR_VBK) {
581+
/*
582+
* Wake up the vblank wait if the counter reaches 0. This must
583+
* be protected by the vblank_lock to avoid races in
584+
* rcar_du_crtc_disable_planes().
585+
*/
586+
if (rcrtc->vblank_count) {
587+
if (--rcrtc->vblank_count == 0)
588+
wake_up(&rcrtc->vblank_wait);
589+
}
590+
}
591+
spin_unlock(&rcrtc->vblank_lock);
592+
554593
if (status & DSSR_VBK) {
555594
drm_crtc_handle_vblank(&rcrtc->crtc);
556595
rcar_du_crtc_finish_page_flip(rcrtc);
@@ -606,6 +645,8 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
606645
}
607646

608647
init_waitqueue_head(&rcrtc->flip_wait);
648+
init_waitqueue_head(&rcrtc->vblank_wait);
649+
spin_lock_init(&rcrtc->vblank_lock);
609650

610651
rcrtc->group = rgrp;
611652
rcrtc->mmio_offset = mmio_offsets[index];

drivers/gpu/drm/rcar-du/rcar_du_crtc.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#define __RCAR_DU_CRTC_H__
1616

1717
#include <linux/mutex.h>
18+
#include <linux/spinlock.h>
1819
#include <linux/wait.h>
1920

2021
#include <drm/drmP.h>
@@ -33,6 +34,9 @@ struct rcar_du_vsp;
3334
* @started: whether the CRTC has been started and is running
3435
* @event: event to post when the pending page flip completes
3536
* @flip_wait: wait queue used to signal page flip completion
37+
* @vblank_lock: protects vblank_wait and vblank_count
38+
* @vblank_wait: wait queue used to signal vertical blanking
39+
* @vblank_count: number of vertical blanking interrupts to wait for
3640
* @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC
3741
* @group: CRTC group this CRTC belongs to
3842
*/
@@ -48,6 +52,10 @@ struct rcar_du_crtc {
4852
struct drm_pending_vblank_event *event;
4953
wait_queue_head_t flip_wait;
5054

55+
spinlock_t vblank_lock;
56+
wait_queue_head_t vblank_wait;
57+
unsigned int vblank_count;
58+
5159
unsigned int outputs;
5260

5361
struct rcar_du_group *group;

0 commit comments

Comments
 (0)