* [PATCH] drm: Don't allow legacy cursor updates to stall others or be stalled
@ 2016-08-23 10:48 Chris Wilson
2016-08-23 10:57 ` Chris Wilson
2016-08-23 11:26 ` Chris Wilson
0 siblings, 2 replies; 4+ messages in thread
From: Chris Wilson @ 2016-08-23 10:48 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, intel-gfx
Legacy cursor updates are entirely asynchronous with respect to all
other users of the atomic pipeline. They neither wait for any
outstanding flips, nor do they cause subsequent flips to be delayed. The
only ordering we do require is given by making the legacy cursor update
nonblocking (so the sequence of userspace calls from a process is
ordered from the pov of the client).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/drm_atomic_helper.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 9abe0a242f96..9441fb9d9637 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1422,9 +1422,11 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
state->crtcs[i].commit = commit;
- ret = stall_checks(crtc, nonblock);
- if (ret)
- return ret;
+ if (!state->legacy_cursor_update) {
+ ret = stall_checks(crtc, nonblock);
+ if (ret)
+ return ret;
+ }
/* Drivers only send out events when at least either current or
* new CRTC state is active. Complete right away if everything
@@ -1989,7 +1991,7 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
swap(state->crtcs[i].state, crtc->state);
crtc->state->state = NULL;
- if (state->crtcs[i].commit) {
+ if (state->crtcs[i].commit && !state->legacy_cursor_update) {
spin_lock(&crtc->commit_lock);
list_add(&state->crtcs[i].commit->commit_entry,
&crtc->commit_list);
--
2.9.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm: Don't allow legacy cursor updates to stall others or be stalled
2016-08-23 10:48 [PATCH] drm: Don't allow legacy cursor updates to stall others or be stalled Chris Wilson
@ 2016-08-23 10:57 ` Chris Wilson
2016-08-23 11:26 ` Chris Wilson
1 sibling, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2016-08-23 10:57 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, intel-gfx
On Tue, Aug 23, 2016 at 11:48:34AM +0100, Chris Wilson wrote:
> Legacy cursor updates are entirely asynchronous with respect to all
> other users of the atomic pipeline. They neither wait for any
> outstanding flips, nor do they cause subsequent flips to be delayed. The
> only ordering we do require is given by making the legacy cursor update
> nonblocking (so the sequence of userspace calls from a process is
s/nonblocking/blocking/!
> ordered from the pov of the client).
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm: Don't allow legacy cursor updates to stall others or be stalled
2016-08-23 10:48 [PATCH] drm: Don't allow legacy cursor updates to stall others or be stalled Chris Wilson
2016-08-23 10:57 ` Chris Wilson
@ 2016-08-23 11:26 ` Chris Wilson
2016-08-23 11:35 ` Daniel Vetter
1 sibling, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2016-08-23 11:26 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, intel-gfx
On Tue, Aug 23, 2016 at 11:48:34AM +0100, Chris Wilson wrote:
> Legacy cursor updates are entirely asynchronous with respect to all
> other users of the atomic pipeline. They neither wait for any
> outstanding flips, nor do they cause subsequent flips to be delayed. The
> only ordering we do require is given by making the legacy cursor update
> nonblocking (so the sequence of userspace calls from a process is
> ordered from the pov of the client).
Hmm, it is also not entirely true. A modeset (reconfiguration of the
pipe size) would require all cursor updates to be flushed, or else we
may make the cursor out-of-bounds which can cause GPU hangs. At the
moment, I think that is governed by the crtc lock and the blocking
nature of the modeset + cursor update. But in the future we want far
more fine grained control over (mostly) asynchronous updates.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm: Don't allow legacy cursor updates to stall others or be stalled
2016-08-23 11:26 ` Chris Wilson
@ 2016-08-23 11:35 ` Daniel Vetter
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2016-08-23 11:35 UTC (permalink / raw)
To: Chris Wilson, dri-devel, intel-gfx, Daniel Vetter
On Tue, Aug 23, 2016 at 12:26:44PM +0100, Chris Wilson wrote:
> On Tue, Aug 23, 2016 at 11:48:34AM +0100, Chris Wilson wrote:
> > Legacy cursor updates are entirely asynchronous with respect to all
> > other users of the atomic pipeline. They neither wait for any
> > outstanding flips, nor do they cause subsequent flips to be delayed. The
> > only ordering we do require is given by making the legacy cursor update
> > nonblocking (so the sequence of userspace calls from a process is
> > ordered from the pov of the client).
>
> Hmm, it is also not entirely true. A modeset (reconfiguration of the
> pipe size) would require all cursor updates to be flushed, or else we
> may make the cursor out-of-bounds which can cause GPU hangs. At the
> moment, I think that is governed by the crtc lock and the blocking
> nature of the modeset + cursor update. But in the future we want far
> more fine grained control over (mostly) asynchronous updates.
Yup, that's why I chickened out of fixing this properly ;-) What we could
do is set a flag that it's a modeset in drm_crtc_commit, and hand a flag
to stall_check to only look for modeset updates. Entirely elliding isn't a
good idea.
The other issue is how to order cursor updates through the legacy update
ioctl against atomic updates on the plane. Not that userspace should mix
those, but it could, and we need to make sure they're ordered properly.
This is especially painful when watermarks have changed, which is entirely
driver-dependent.
At that point it's probably time to ragequit.
Aside: i915 atomic commit is atm stalling way too much, it signals hw_done
too late. Iirc I've sprinkled a fixme into either the commit or code about
what needs to be done for that. Legacy cursor updates with hw_done
signalled early enough (i.e. when we wrote the register, before the hw has
committed the update) should fix this stalling issue too.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-08-23 11:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-23 10:48 [PATCH] drm: Don't allow legacy cursor updates to stall others or be stalled Chris Wilson
2016-08-23 10:57 ` Chris Wilson
2016-08-23 11:26 ` Chris Wilson
2016-08-23 11:35 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox