All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Rob Clark <robdclark@gmail.com>
Cc: linux-fbdev@vger.kernel.org, Pawel Moll <Pawel.Moll@arm.com>,
	dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC 1/1] drm/pl111: Initial drm/kms driver for pl111
Date: Fri, 09 Aug 2013 16:57:06 +0000	[thread overview]
Message-ID: <20130809165706.GC31670@phenom.ffwll.local> (raw)
In-Reply-To: <CAF6AEGu68ntQDSueQJmAM1KSsSA86j98GDEf8wOPbZKjECw99Q@mail.gmail.com>

On Fri, Aug 09, 2013 at 12:34:55PM -0400, Rob Clark wrote:
> On Fri, Aug 9, 2013 at 12:15 PM, Tom Cooksey <tom.cooksey@arm.com> wrote:
> >> > fwiw, this is at least different from how other drivers do triple
> >> > (or > double) buffering.  In other drivers (intel, omap, and
> >> > msm/freedreno, that I know of, maybe others too) the xorg driver
> >> > dri2 bits implement the double buffering (ie. send flip event back
> >> > to client immediately and queue up the flip and call page-flip
> >> > after the pageflip event back from kernel.
> >> >
> >> > I'm not saying not to do it this way, I guess I'd like to hear
> >> > what other folks think.  I kinda prefer doing this in userspace
> >> > as it keeps the kernel bits simpler (plus it would then work
> >> > properly on exynosdrm or other kms drivers).
> >>
> >> Yeah, if this is just a sw queue then I don't think it makes sense
> >> to have it in the kernel. Afaik the current pageflip interface drm
> >> exposes allows one oustanding flip only, and you _must_ wait for
> >> the flip complete event before you can submit the second one.
> >
> > Right, I'll have a think about this. I think our idea was to issue
> > enough page-flips into the kernel to make sure that any process
> > scheduling latencies on a heavily loaded system don't cause us to
> > miss a v_sync deadline. At the moment we issue the page flip from DRI2
> > schedule_swap. If we were to move that to the page flip event handler
> > of the previous page-flip, we're potentially adding in extra latency.
> >
> > I.e. Currently we have:
> >
> > DRI2SwapBuffers
> >  - drm_mode_page_flip to buffer B
> > DRI2SwapBuffers
> >  - drm_mode_page_flip to buffer A (gets queued in kernel)
> > ...
> > v_sync! (at this point buffer B is scanned out)
> >  - release buffer A's KDS resource/signal buffer A's fence
> >     - queued GPU job to render next frame to buffer A scheduled on HW
> > ...
> > GPU interrupt! (at this point buffer A is ready to be scanned out)
> >  - release buffer A's KDS resource/signal buffer A's fence
> >     - second page flip executed, buffer A's address written to scanout
> >       register, takes effect on next v_sync.
> >
> >
> > So in the above, after X receives the second DRI2SwapBuffers, it
> > doesn't need to get scheduled again for the next frame to be both
> > rendered by the GPU and issued to the display for scanout.
> 
> well, this is really only an issue if you are so loaded that you don't
> get a chance to schedule for ~16ms.. which is pretty long time.  If
> you are triple buffering, it should not end up in the critical path
> (since the gpu already has the 3rd buffer to start on the next frame).
>  And, well, if you do it all in the kernel you probably need to toss
> things over to a workqueue anyways.

Just a quick comment on the kernel flip queue issue.

16 ms scheduling latency sounds awful but totally doable with a less than
stellar ddx driver going into limbo land and so preventing your single
threaded X from doing more useful stuff. Is this really the linux
scheduler being stupid?

At least my impression was that the hw/kernel flip queue is to save power
so that you can queue up a few frames and everything goes to sleep for
half a second or so (at 24fps or whatever movie your showing). Needing to
schedule 5 frames ahead with pageflips under load is just guaranteed to
result in really horrible interactivity and so awful user experience ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

WARNING: multiple messages have this Message-ID (diff)
From: daniel@ffwll.ch (Daniel Vetter)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 1/1] drm/pl111: Initial drm/kms driver for pl111
Date: Fri, 9 Aug 2013 18:57:06 +0200	[thread overview]
Message-ID: <20130809165706.GC31670@phenom.ffwll.local> (raw)
In-Reply-To: <CAF6AEGu68ntQDSueQJmAM1KSsSA86j98GDEf8wOPbZKjECw99Q@mail.gmail.com>

On Fri, Aug 09, 2013 at 12:34:55PM -0400, Rob Clark wrote:
> On Fri, Aug 9, 2013 at 12:15 PM, Tom Cooksey <tom.cooksey@arm.com> wrote:
> >> > fwiw, this is at least different from how other drivers do triple
> >> > (or > double) buffering.  In other drivers (intel, omap, and
> >> > msm/freedreno, that I know of, maybe others too) the xorg driver
> >> > dri2 bits implement the double buffering (ie. send flip event back
> >> > to client immediately and queue up the flip and call page-flip
> >> > after the pageflip event back from kernel.
> >> >
> >> > I'm not saying not to do it this way, I guess I'd like to hear
> >> > what other folks think.  I kinda prefer doing this in userspace
> >> > as it keeps the kernel bits simpler (plus it would then work
> >> > properly on exynosdrm or other kms drivers).
> >>
> >> Yeah, if this is just a sw queue then I don't think it makes sense
> >> to have it in the kernel. Afaik the current pageflip interface drm
> >> exposes allows one oustanding flip only, and you _must_ wait for
> >> the flip complete event before you can submit the second one.
> >
> > Right, I'll have a think about this. I think our idea was to issue
> > enough page-flips into the kernel to make sure that any process
> > scheduling latencies on a heavily loaded system don't cause us to
> > miss a v_sync deadline. At the moment we issue the page flip from DRI2
> > schedule_swap. If we were to move that to the page flip event handler
> > of the previous page-flip, we're potentially adding in extra latency.
> >
> > I.e. Currently we have:
> >
> > DRI2SwapBuffers
> >  - drm_mode_page_flip to buffer B
> > DRI2SwapBuffers
> >  - drm_mode_page_flip to buffer A (gets queued in kernel)
> > ...
> > v_sync! (at this point buffer B is scanned out)
> >  - release buffer A's KDS resource/signal buffer A's fence
> >     - queued GPU job to render next frame to buffer A scheduled on HW
> > ...
> > GPU interrupt! (at this point buffer A is ready to be scanned out)
> >  - release buffer A's KDS resource/signal buffer A's fence
> >     - second page flip executed, buffer A's address written to scanout
> >       register, takes effect on next v_sync.
> >
> >
> > So in the above, after X receives the second DRI2SwapBuffers, it
> > doesn't need to get scheduled again for the next frame to be both
> > rendered by the GPU and issued to the display for scanout.
> 
> well, this is really only an issue if you are so loaded that you don't
> get a chance to schedule for ~16ms.. which is pretty long time.  If
> you are triple buffering, it should not end up in the critical path
> (since the gpu already has the 3rd buffer to start on the next frame).
>  And, well, if you do it all in the kernel you probably need to toss
> things over to a workqueue anyways.

Just a quick comment on the kernel flip queue issue.

16 ms scheduling latency sounds awful but totally doable with a less than
stellar ddx driver going into limbo land and so preventing your single
threaded X from doing more useful stuff. Is this really the linux
scheduler being stupid?

At least my impression was that the hw/kernel flip queue is to save power
so that you can queue up a few frames and everything goes to sleep for
half a second or so (at 24fps or whatever movie your showing). Needing to
schedule 5 frames ahead with pageflips under load is just guaranteed to
result in really horrible interactivity and so awful user experience ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch>
To: Rob Clark <robdclark@gmail.com>
Cc: linux-fbdev@vger.kernel.org, Pawel Moll <Pawel.Moll@arm.com>,
	dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC 1/1] drm/pl111: Initial drm/kms driver for pl111
Date: Fri, 9 Aug 2013 18:57:06 +0200	[thread overview]
Message-ID: <20130809165706.GC31670@phenom.ffwll.local> (raw)
In-Reply-To: <CAF6AEGu68ntQDSueQJmAM1KSsSA86j98GDEf8wOPbZKjECw99Q@mail.gmail.com>

On Fri, Aug 09, 2013 at 12:34:55PM -0400, Rob Clark wrote:
> On Fri, Aug 9, 2013 at 12:15 PM, Tom Cooksey <tom.cooksey@arm.com> wrote:
> >> > fwiw, this is at least different from how other drivers do triple
> >> > (or > double) buffering.  In other drivers (intel, omap, and
> >> > msm/freedreno, that I know of, maybe others too) the xorg driver
> >> > dri2 bits implement the double buffering (ie. send flip event back
> >> > to client immediately and queue up the flip and call page-flip
> >> > after the pageflip event back from kernel.
> >> >
> >> > I'm not saying not to do it this way, I guess I'd like to hear
> >> > what other folks think.  I kinda prefer doing this in userspace
> >> > as it keeps the kernel bits simpler (plus it would then work
> >> > properly on exynosdrm or other kms drivers).
> >>
> >> Yeah, if this is just a sw queue then I don't think it makes sense
> >> to have it in the kernel. Afaik the current pageflip interface drm
> >> exposes allows one oustanding flip only, and you _must_ wait for
> >> the flip complete event before you can submit the second one.
> >
> > Right, I'll have a think about this. I think our idea was to issue
> > enough page-flips into the kernel to make sure that any process
> > scheduling latencies on a heavily loaded system don't cause us to
> > miss a v_sync deadline. At the moment we issue the page flip from DRI2
> > schedule_swap. If we were to move that to the page flip event handler
> > of the previous page-flip, we're potentially adding in extra latency.
> >
> > I.e. Currently we have:
> >
> > DRI2SwapBuffers
> >  - drm_mode_page_flip to buffer B
> > DRI2SwapBuffers
> >  - drm_mode_page_flip to buffer A (gets queued in kernel)
> > ...
> > v_sync! (at this point buffer B is scanned out)
> >  - release buffer A's KDS resource/signal buffer A's fence
> >     - queued GPU job to render next frame to buffer A scheduled on HW
> > ...
> > GPU interrupt! (at this point buffer A is ready to be scanned out)
> >  - release buffer A's KDS resource/signal buffer A's fence
> >     - second page flip executed, buffer A's address written to scanout
> >       register, takes effect on next v_sync.
> >
> >
> > So in the above, after X receives the second DRI2SwapBuffers, it
> > doesn't need to get scheduled again for the next frame to be both
> > rendered by the GPU and issued to the display for scanout.
> 
> well, this is really only an issue if you are so loaded that you don't
> get a chance to schedule for ~16ms.. which is pretty long time.  If
> you are triple buffering, it should not end up in the critical path
> (since the gpu already has the 3rd buffer to start on the next frame).
>  And, well, if you do it all in the kernel you probably need to toss
> things over to a workqueue anyways.

Just a quick comment on the kernel flip queue issue.

16 ms scheduling latency sounds awful but totally doable with a less than
stellar ddx driver going into limbo land and so preventing your single
threaded X from doing more useful stuff. Is this really the linux
scheduler being stupid?

At least my impression was that the hw/kernel flip queue is to save power
so that you can queue up a few frames and everything goes to sleep for
half a second or so (at 24fps or whatever movie your showing). Needing to
schedule 5 frames ahead with pageflips under load is just guaranteed to
result in really horrible interactivity and so awful user experience ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2013-08-09 16:57 UTC|newest]

Thread overview: 131+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-25 17:17 [RFC 0/1] drm/pl111: Initial drm/kms driver for pl111 tom.cooksey
2013-07-25 17:17 ` tom.cooksey
2013-07-25 17:17 ` tom.cooksey at arm.com
2013-07-25 17:17 ` [RFC 1/1] " tom.cooksey
2013-08-07 16:17   ` Rob Clark
2013-08-07 16:46     ` Daniel Vetter
2013-08-07 16:46       ` Daniel Vetter
2013-08-07 16:46       ` Daniel Vetter
2013-08-09 16:15       ` Tom Cooksey
2013-08-09 16:15       ` Tom Cooksey
2013-08-09 16:15       ` Tom Cooksey
     [not found]       ` <520515b0.88b70e0a.3ecd.1004SMTPIN_ADDED_BROKEN@mx.google.com>
2013-08-09 16:34         ` Rob Clark
2013-08-09 16:34           ` Rob Clark
2013-08-09 16:34           ` Rob Clark
2013-08-09 16:57           ` Daniel Vetter [this message]
2013-08-09 16:57             ` Daniel Vetter
2013-08-09 16:57             ` Daniel Vetter
2013-08-09 17:31             ` Tom Cooksey
2013-08-13 14:35               ` Tom Cooksey
2013-08-13 14:35               ` Tom Cooksey
2013-08-13 14:35               ` Tom Cooksey
2013-08-09 17:31             ` Tom Cooksey
2013-08-09 17:31             ` Tom Cooksey
     [not found]             ` <5205277e.84320f0a.1cdf.ffff8816SMTPIN_ADDED_BROKEN@mx.google.com>
2013-08-10 12:30               ` Rob Clark
2013-08-10 12:30                 ` Rob Clark
2013-08-10 12:30                 ` Rob Clark
     [not found]             ` <520a4435.070a0e0a.4fce.ffff9731SMTPIN_ADDED_BROKEN@mx.google.com>
2013-08-13 14:58               ` Rob Clark
2013-08-13 14:58                 ` Rob Clark
2013-08-13 14:58                 ` Rob Clark
2013-08-14 14:59                 ` Tom Cooksey
2013-08-14 14:59                 ` Tom Cooksey
2013-08-14 14:59                 ` Tom Cooksey
2013-08-10  4:56       ` Inki Dae
2013-07-25 18:21 ` [RFC 0/1] " Rob Clark
2013-07-25 18:21   ` Rob Clark
2013-07-25 18:21   ` Rob Clark
2013-07-26 14:06   ` Pawel Moll
2013-07-26 14:06     ` Pawel Moll
2013-07-26 14:06     ` Pawel Moll
2013-07-26 14:21     ` Rob Clark
2013-07-26 14:21       ` Rob Clark
2013-07-26 14:21       ` Rob Clark
2013-07-26 14:41       ` Pawel Moll
2013-07-26 14:41         ` Pawel Moll
2013-07-26 14:41         ` Pawel Moll
2013-07-26 14:14   ` Russell King - ARM Linux
2013-07-26 14:14     ` Russell King - ARM Linux
2013-07-26 14:14     ` Russell King - ARM Linux
2013-07-26 14:24     ` Rob Clark
2013-07-26 14:24       ` Rob Clark
2013-07-26 14:24       ` Rob Clark
2013-07-26 15:58   ` Tom Cooksey
2013-07-26 15:58   ` Tom Cooksey
2013-07-26 15:58   ` Tom Cooksey
     [not found]   ` <51f29cd1.e686440a.66b2.fffff9d5SMTPIN_ADDED_BROKEN@mx.google.com>
2013-07-26 18:56     ` Daniel Vetter
2013-07-26 18:56       ` Daniel Vetter
2013-07-26 18:56       ` Daniel Vetter
     [not found]   ` <51f29ccd.f014b40a.34cc.ffffca2aSMTPIN_ADDED_BROKEN@mx.google.com>
2013-07-29 14:58     ` Rob Clark
2013-07-29 14:58       ` Rob Clark
2013-07-29 14:58       ` Rob Clark
2013-08-05 17:10       ` Tom Cooksey
2013-08-05 17:10       ` Tom Cooksey
2013-08-05 17:10       ` Tom Cooksey
     [not found]       ` <51ffdc7e.06b8b40a.2cc8.0fe0SMTPIN_ADDED_BROKEN@mx.google.com>
2013-08-05 18:07         ` Rob Clark
2013-08-05 18:07           ` Rob Clark
2013-08-05 18:07           ` Rob Clark
2013-08-06 11:31           ` Tom Cooksey
2013-08-06 11:31           ` Tom Cooksey
2013-08-06 11:31           ` Tom Cooksey
2013-08-06 12:18             ` [Linaro-mm-sig] " Lucas Stach
2013-08-06 12:18               ` Lucas Stach
2013-08-06 12:18               ` Lucas Stach
2013-08-06 14:14               ` Rob Clark
2013-08-06 14:14                 ` Rob Clark
2013-08-06 14:14                 ` Rob Clark
2013-08-06 14:36                 ` Lucas Stach
2013-08-06 14:36                   ` Lucas Stach
2013-08-06 14:36                   ` Lucas Stach
2013-08-06 14:59                   ` Rob Clark
2013-08-06 14:59                     ` Rob Clark
2013-08-06 14:59                     ` Rob Clark
2013-08-06 15:28               ` Daniel Vetter
2013-08-06 15:28                 ` Daniel Vetter
2013-08-06 15:28                 ` Daniel Vetter
2013-08-06 15:28                 ` Daniel Vetter
2013-09-14 21:33             ` Daniel Stone
2013-09-14 21:33               ` Daniel Stone
2013-09-14 21:33               ` Daniel Stone
     [not found]           ` <5200deb3.0b24b40a.3b26.ffffbadeSMTPIN_ADDED_BROKEN@mx.google.com>
2013-08-06 12:15             ` Rob Clark
2013-08-06 12:15               ` Rob Clark
2013-08-06 12:15               ` Rob Clark
2013-08-06 14:03               ` Tom Cooksey
2013-08-06 14:03               ` Tom Cooksey
2013-08-06 14:03               ` Tom Cooksey
     [not found]               ` <52010257.245fc20a.6ff8.1cfdSMTPIN_ADDED_BROKEN@mx.google.com>
2013-08-06 14:40                 ` Rob Clark
2013-08-06 14:40                   ` Rob Clark
2013-08-06 14:40                   ` Rob Clark
2013-08-06 17:38                   ` Tom Cooksey
2013-08-06 17:38                   ` Tom Cooksey
2013-08-06 17:38                   ` Tom Cooksey
     [not found]                   ` <52013482.e107c20a.27f9.ffffa718SMTPIN_ADDED_BROKEN@mx.google.com>
2013-08-06 19:06                     ` Rob Clark
2013-08-06 19:06                       ` Rob Clark
2013-08-06 19:06                       ` Rob Clark
2013-08-07 17:33                       ` Tom Cooksey
2013-08-07 17:33                       ` Tom Cooksey
2013-08-07 17:33                       ` Tom Cooksey
     [not found]                       ` <520284fe.a16ec20a.2d3c.6e19SMTPIN_ADDED_BROKEN@mx.google.com>
2013-08-07 17:56                         ` [Linaro-mm-sig] " Alex Deucher
2013-08-07 17:56                           ` Alex Deucher
2013-08-07 17:56                           ` Alex Deucher
     [not found]                       ` <520284d6.07300f0a.72a4.1623SMTPIN_ADDED_BROKEN@mx.google.com>
2013-08-07 18:12                         ` Rob Clark
2013-08-07 18:12                           ` Rob Clark
2013-08-07 18:12                           ` Rob Clark
2013-08-09 16:15                           ` Tom Cooksey
2013-08-09 16:15                           ` Tom Cooksey
2013-08-09 16:15                           ` Tom Cooksey
     [not found]                           ` <520515b9.87370f0a.16e6.2380SMTPIN_ADDED_BROKEN@mx.google.com>
2013-08-09 17:12                             ` Rob Clark
2013-08-09 17:12                               ` Rob Clark
2013-08-09 17:12                               ` Rob Clark
2013-08-14 15:01                               ` Tom Cooksey
2013-08-14 15:01                               ` Tom Cooksey
2013-08-14 15:01                               ` Tom Cooksey
2013-08-07  4:23               ` John Stultz
2013-08-07  4:23                 ` John Stultz
2013-08-07  4:23                 ` John Stultz
2013-08-07  4:23                 ` John Stultz
2013-08-07 13:27                 ` Rob Clark
2013-08-07 13:27                   ` Rob Clark
2013-08-07 13:27                   ` Rob Clark
2013-08-07  3:57       ` John Stultz
2013-08-07  3:57         ` John Stultz
2013-08-07  3:57         ` John Stultz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130809165706.GC31670@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=Pawel.Moll@arm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=robdclark@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.