Commit 641307df authored by Laurent Pinchart's avatar Laurent Pinchart

drm: rcar-du: Fix race condition when disabling planes at CRTC stop

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: default avatarLaurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: default avatarKieran Bingham <kieran.bingham+renesas@ideasonboard.com>
parent d6160246
...@@ -490,23 +490,51 @@ static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc) ...@@ -490,23 +490,51 @@ static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc)
rcar_du_group_start_stop(rcrtc->group, true); rcar_du_group_start_stop(rcrtc->group, true);
} }
static void rcar_du_crtc_disable_planes(struct rcar_du_crtc *rcrtc)
{
struct rcar_du_device *rcdu = rcrtc->group->dev;
struct drm_crtc *crtc = &rcrtc->crtc;
u32 status;
/* Make sure vblank interrupts are enabled. */
drm_crtc_vblank_get(crtc);
/*
* Disable planes and calculate how many vertical blanking interrupts we
* have to wait for. If a vertical blanking interrupt has been triggered
* but not processed yet, we don't know whether it occurred before or
* after the planes got disabled. We thus have to wait for two vblank
* interrupts in that case.
*/
spin_lock_irq(&rcrtc->vblank_lock);
rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
status = rcar_du_crtc_read(rcrtc, DSSR);
rcrtc->vblank_count = status & DSSR_VBK ? 2 : 1;
spin_unlock_irq(&rcrtc->vblank_lock);
if (!wait_event_timeout(rcrtc->vblank_wait, rcrtc->vblank_count == 0,
msecs_to_jiffies(100)))
dev_warn(rcdu->dev, "vertical blanking timeout\n");
drm_crtc_vblank_put(crtc);
}
static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc) static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
{ {
struct drm_crtc *crtc = &rcrtc->crtc; struct drm_crtc *crtc = &rcrtc->crtc;
/* /*
* Disable all planes and wait for the change to take effect. This is * Disable all planes and wait for the change to take effect. This is
* required as the DSnPR registers are updated on vblank, and no vblank * required as the plane enable registers are updated on vblank, and no
* will occur once the CRTC is stopped. Disabling planes when starting * vblank will occur once the CRTC is stopped. Disabling planes when
* the CRTC thus wouldn't be enough as it would start scanning out * starting the CRTC thus wouldn't be enough as it would start scanning
* immediately from old frame buffers until the next vblank. * out immediately from old frame buffers until the next vblank.
* *
* This increases the CRTC stop delay, especially when multiple CRTCs * This increases the CRTC stop delay, especially when multiple CRTCs
* are stopped in one operation as we now wait for one vblank per CRTC. * are stopped in one operation as we now wait for one vblank per CRTC.
* Whether this can be improved needs to be researched. * Whether this can be improved needs to be researched.
*/ */
rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0); rcar_du_crtc_disable_planes(rcrtc);
drm_crtc_wait_one_vblank(crtc);
/* /*
* Disable vertical blanking interrupt reporting. We first need to wait * Disable vertical blanking interrupt reporting. We first need to wait
...@@ -695,9 +723,25 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg) ...@@ -695,9 +723,25 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
irqreturn_t ret = IRQ_NONE; irqreturn_t ret = IRQ_NONE;
u32 status; u32 status;
spin_lock(&rcrtc->vblank_lock);
status = rcar_du_crtc_read(rcrtc, DSSR); status = rcar_du_crtc_read(rcrtc, DSSR);
rcar_du_crtc_write(rcrtc, DSRCR, status & DSRCR_MASK); rcar_du_crtc_write(rcrtc, DSRCR, status & DSRCR_MASK);
if (status & DSSR_VBK) {
/*
* Wake up the vblank wait if the counter reaches 0. This must
* be protected by the vblank_lock to avoid races in
* rcar_du_crtc_disable_planes().
*/
if (rcrtc->vblank_count) {
if (--rcrtc->vblank_count == 0)
wake_up(&rcrtc->vblank_wait);
}
}
spin_unlock(&rcrtc->vblank_lock);
if (status & DSSR_VBK) { if (status & DSSR_VBK) {
drm_crtc_handle_vblank(&rcrtc->crtc); drm_crtc_handle_vblank(&rcrtc->crtc);
...@@ -756,6 +800,8 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index) ...@@ -756,6 +800,8 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
} }
init_waitqueue_head(&rcrtc->flip_wait); init_waitqueue_head(&rcrtc->flip_wait);
init_waitqueue_head(&rcrtc->vblank_wait);
spin_lock_init(&rcrtc->vblank_lock);
rcrtc->group = rgrp; rcrtc->group = rgrp;
rcrtc->mmio_offset = mmio_offsets[index]; rcrtc->mmio_offset = mmio_offsets[index];
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#define __RCAR_DU_CRTC_H__ #define __RCAR_DU_CRTC_H__
#include <linux/mutex.h> #include <linux/mutex.h>
#include <linux/spinlock.h>
#include <linux/wait.h> #include <linux/wait.h>
#include <drm/drmP.h> #include <drm/drmP.h>
...@@ -33,6 +34,9 @@ struct rcar_du_vsp; ...@@ -33,6 +34,9 @@ struct rcar_du_vsp;
* @initialized: whether the CRTC has been initialized and clocks enabled * @initialized: whether the CRTC has been initialized and clocks enabled
* @event: event to post when the pending page flip completes * @event: event to post when the pending page flip completes
* @flip_wait: wait queue used to signal page flip completion * @flip_wait: wait queue used to signal page flip completion
* @vblank_lock: protects vblank_wait and vblank_count
* @vblank_wait: wait queue used to signal vertical blanking
* @vblank_count: number of vertical blanking interrupts to wait for
* @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC
* @group: CRTC group this CRTC belongs to * @group: CRTC group this CRTC belongs to
* @vsp: VSP feeding video to this CRTC * @vsp: VSP feeding video to this CRTC
...@@ -50,6 +54,10 @@ struct rcar_du_crtc { ...@@ -50,6 +54,10 @@ struct rcar_du_crtc {
struct drm_pending_vblank_event *event; struct drm_pending_vblank_event *event;
wait_queue_head_t flip_wait; wait_queue_head_t flip_wait;
spinlock_t vblank_lock;
wait_queue_head_t vblank_wait;
unsigned int vblank_count;
unsigned int outputs; unsigned int outputs;
struct rcar_du_group *group; struct rcar_du_group *group;
......
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