[PATCHv2 41/45] drm: omapdrm: remove omap_crtc_wait_page_flip
Tomi Valkeinen
tomi.valkeinen at ti.com
Mon Jun 8 09:02:28 PDT 2015
On 06/06/15 07:09, Laurent Pinchart wrote:
> Hi Tomi,
>
> Thank you for the patch.
>
> On Thursday 04 June 2015 12:02:58 Tomi Valkeinen wrote:
>> omap_crtc_disable() waits for pending page flips to finish. However,
>> omap_crtc_disable() is always called via omap_atomic_complete() and we
>> never have page flips going on at that time.
>
> Why is that ? Because our omap_atomic_complete() implementation waits for
> vblanks before allowing the next atomic commit to run, and that our vblank IRQ
> handler completes all pending page flips ? If so, I believe we have a few
> corner cases that won't work properly.
Yes, I think what omap_atomic_complete() is supposed to do is to wait until
everything is done for the affected crtcs. But you are right, it wasn't quite
correct.
> For instance, if the user performs a full mode set on a CRTC without change
> the framebuffer, an event can be queued but
> drm_atomic_helper_wait_for_vblanks() won't wait for vblank on that CRTC as
> framebuffer_changed() will return false. If the next commit then disables the
> CRTC the event might not have completed yet.
I reworked the event/flip_wait code, removing this patch and 42 and 43. I didn't
have time to clean it up properly, but I'm probably not able to work on it for
the following days, so I'll post it now. Quick testing went fine.
I also pushed it to omapdrm-atomic-v2 branch.
From 8d6429616783c335d20bf8ebaf8cc4dc447d0385 Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen <tomi.valkeinen at ti.com>
Date: Fri, 29 May 2015 16:01:18 +0300
Subject: [PATCH] drm: omapdrm: new vblank and event handling
Rework the crtc event/flip_wait system as follows:
- If we enable a crtc (full modeset), we set omap_crtc->pending and
register vblank irq.
- If we need to set GO bit (page flip), we do the same but also set the
GO bit.
- On vblank we unregister the irq, clear the 'pending' flag, send vblank
event to userspace if crtc->state->event != NULL, and wake up
'pending_wait' wq.
- In omap_atomic_complete() we wait for the 'pending' flag to get reset
for all enabled crtcs using 'pending_wait wq.
The above ensures that we send the events to userspace in vblank, and
that after the wait in omap_atomic_complete() everything for the
affected crtcs has been completed.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 8d2bf8565ddd..9b067d10e8d2 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -17,8 +17,6 @@
* this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <linux/completion.h>
-
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_crtc.h>
@@ -49,13 +47,10 @@ struct omap_crtc {
struct omap_drm_irq vblank_irq;
struct omap_drm_irq error_irq;
- /* pending event */
- struct drm_pending_vblank_event *event;
- wait_queue_head_t flip_wait;
-
- struct completion completion;
-
bool ignore_digit_sync_lost;
+
+ bool pending;
+ wait_queue_head_t pending_wait;
};
/* -----------------------------------------------------------------------------
@@ -81,6 +76,15 @@ enum omap_channel omap_crtc_channel(struct drm_crtc *crtc)
return omap_crtc->channel;
}
+int omap_crtc_wait_pending(struct drm_crtc *crtc)
+{
+ struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+
+ return wait_event_timeout(omap_crtc->pending_wait,
+ !omap_crtc->pending,
+ msecs_to_jiffies(50));
+}
+
/* -----------------------------------------------------------------------------
* DSS Manager Functions
*/
@@ -253,77 +257,45 @@ static const struct dss_mgr_ops mgr_ops = {
* Setup, Flush and Page Flip
*/
-static void omap_crtc_complete_page_flip(struct drm_crtc *crtc)
+static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
{
- struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
- struct drm_pending_vblank_event *event;
- struct drm_device *dev = crtc->dev;
- unsigned long flags;
-
- spin_lock_irqsave(&dev->event_lock, flags);
-
- event = omap_crtc->event;
- omap_crtc->event = NULL;
-
- if (event) {
- list_del(&event->base.link);
-
- /*
- * Queue the event for delivery if it's still linked to a file
- * handle, otherwise just destroy it.
- */
- if (event->base.file_priv)
- drm_crtc_send_vblank_event(crtc, event);
- else
- event->base.destroy(&event->base);
+ struct omap_crtc *omap_crtc =
+ container_of(irq, struct omap_crtc, error_irq);
- wake_up(&omap_crtc->flip_wait);
- drm_crtc_vblank_put(crtc);
+ if (omap_crtc->ignore_digit_sync_lost) {
+ irqstatus &= ~DISPC_IRQ_SYNC_LOST_DIGIT;
+ if (!irqstatus)
+ return;
}
- spin_unlock_irqrestore(&dev->event_lock, flags);
+ DRM_ERROR_RATELIMITED("%s: errors: %08x\n", omap_crtc->name, irqstatus);
}
-static bool omap_crtc_page_flip_pending(struct drm_crtc *crtc)
+static void omap_crtc_complete_page_flip(struct drm_crtc *crtc)
{
- struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+ struct drm_pending_vblank_event *event;
struct drm_device *dev = crtc->dev;
unsigned long flags;
- bool pending;
- spin_lock_irqsave(&dev->event_lock, flags);
- pending = omap_crtc->event != NULL;
- spin_unlock_irqrestore(&dev->event_lock, flags);
+ event = crtc->state->event;
- return pending;
-}
-
-static void omap_crtc_wait_page_flip(struct drm_crtc *crtc)
-{
- struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-
- if (wait_event_timeout(omap_crtc->flip_wait,
- !omap_crtc_page_flip_pending(crtc),
- msecs_to_jiffies(50)))
+ if (!event)
return;
- dev_warn(crtc->dev->dev, "page flip timeout!\n");
-
- omap_crtc_complete_page_flip(crtc);
-}
+ spin_lock_irqsave(&dev->event_lock, flags);
-static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
-{
- struct omap_crtc *omap_crtc =
- container_of(irq, struct omap_crtc, error_irq);
+ list_del(&event->base.link);
- if (omap_crtc->ignore_digit_sync_lost) {
- irqstatus &= ~DISPC_IRQ_SYNC_LOST_DIGIT;
- if (!irqstatus)
- return;
- }
+ /*
+ * Queue the event for delivery if it's still linked to a file
+ * handle, otherwise just destroy it.
+ */
+ if (event->base.file_priv)
+ drm_crtc_send_vblank_event(crtc, event);
+ else
+ event->base.destroy(&event->base);
- DRM_ERROR_RATELIMITED("%s: errors: %08x\n", omap_crtc->name, irqstatus);
+ spin_unlock_irqrestore(&dev->event_lock, flags);
}
static void omap_crtc_vblank_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
@@ -336,12 +308,19 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
return;
DBG("%s: apply done", omap_crtc->name);
+
__omap_irq_unregister(dev, &omap_crtc->vblank_irq);
- /* wakeup userspace */
+ mb();
+ WARN_ON(!omap_crtc->pending);
+ omap_crtc->pending = false;
+ mb();
+
+ /* wake up userspace */
omap_crtc_complete_page_flip(&omap_crtc->base);
- complete(&omap_crtc->completion);
+ /* wake up omap_atomic_complete */
+ wake_up(&omap_crtc->pending_wait);
}
/* -----------------------------------------------------------------------------
@@ -375,6 +354,13 @@ static void omap_crtc_enable(struct drm_crtc *crtc)
DBG("%s", omap_crtc->name);
+ WARN_ON(omap_crtc->pending);
+
+ omap_crtc->pending = true;
+ mb();
+
+ omap_irq_register(crtc->dev, &omap_crtc->vblank_irq);
+
drm_crtc_vblank_on(crtc);
}
@@ -384,7 +370,6 @@ static void omap_crtc_disable(struct drm_crtc *crtc)
DBG("%s", omap_crtc->name);
- omap_crtc_wait_page_flip(crtc);
drm_crtc_vblank_off(crtc);
}
@@ -405,19 +390,7 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
static void omap_crtc_atomic_begin(struct drm_crtc *crtc)
{
- struct drm_pending_vblank_event *event = crtc->state->event;
- struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
- struct drm_device *dev = crtc->dev;
- unsigned long flags;
-
- if (event) {
- WARN_ON(omap_crtc->event);
- WARN_ON(drm_crtc_vblank_get(crtc) != 0);
- spin_lock_irqsave(&dev->event_lock, flags);
- omap_crtc->event = event;
- spin_unlock_irqrestore(&dev->event_lock, flags);
- }
}
static void omap_crtc_atomic_flush(struct drm_crtc *crtc)
@@ -425,16 +398,16 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc)
struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
WARN_ON(omap_crtc->vblank_irq.registered);
+ WARN_ON(omap_crtc->pending);
if (dispc_mgr_is_enabled(omap_crtc->channel)) {
DBG("%s: GO", omap_crtc->name);
+ omap_crtc->pending = true;
+ mb();
+
dispc_mgr_go(omap_crtc->channel);
omap_irq_register(crtc->dev, &omap_crtc->vblank_irq);
-
- WARN_ON(!wait_for_completion_timeout(&omap_crtc->completion,
- msecs_to_jiffies(100)));
- reinit_completion(&omap_crtc->completion);
}
crtc->invert_dimensions = !!(crtc->primary->state->rotation &
@@ -534,8 +507,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
crtc = &omap_crtc->base;
- init_waitqueue_head(&omap_crtc->flip_wait);
- init_completion(&omap_crtc->completion);
+ init_waitqueue_head(&omap_crtc->pending_wait);
omap_crtc->channel = channel;
omap_crtc->name = channel_names[channel];
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 50f555530e55..e8f4001d9d31 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -66,6 +66,25 @@ struct omap_atomic_state_commit {
u32 crtcs;
};
+static void omap_atomic_wait_for_gos(struct drm_device *dev,
+ struct drm_atomic_state *old_state)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *old_crtc_state;
+ int i, ret;
+
+ for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
+ if (!crtc->state->enable)
+ continue;
+
+ ret = omap_crtc_wait_pending(crtc);
+
+ if (!ret)
+ dev_warn(dev->dev,
+ "atomic flush timeout (pipe %u)!\n", i);
+ }
+}
+
static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
{
struct drm_device *dev = commit->dev;
@@ -79,7 +98,7 @@ static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
drm_atomic_helper_commit_planes(dev, old_state);
drm_atomic_helper_commit_modeset_enables(dev, old_state);
- drm_atomic_helper_wait_for_vblanks(dev, old_state);
+ omap_atomic_wait_for_gos(dev, old_state);
drm_atomic_helper_cleanup_planes(dev, old_state);
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 0b7a055bf007..ae2df41f216f 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -147,6 +147,7 @@ void omap_crtc_pre_init(void);
void omap_crtc_pre_uninit(void);
struct drm_crtc *omap_crtc_init(struct drm_device *dev,
struct drm_plane *plane, enum omap_channel channel, int id);
+int omap_crtc_wait_pending(struct drm_crtc *crtc);
struct drm_plane *omap_plane_init(struct drm_device *dev,
int id, enum drm_plane_type type);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150608/5dd43368/attachment-0001.sig>
More information about the dri-devel
mailing list