[RFC] drm: add flip-work helper

[Rob Clark]

On Mon, Aug 5, 2013 at 4:00 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Sun, Aug 04, 2013 at 10:20:54PM -0400, Rob Clark wrote:
>> On Sun, Aug 4, 2013 at 1:34 PM, Daniel Vetter <daniel at ffwll.ch> wrote:
>> > On Thu, Aug 1, 2013 at 1:23 AM, Rob Clark <robdclark at gmail.com> wrote:
>> >> A small helper to queue up work to do, from workqueue context, after a
>> >> flip.  Typically useful to defer unreffing buffers that may be read by
>> >> the display controller until vblank.
>> >>
>> >> Signed-off-by: Rob Clark <robdclark at gmail.com>
>> >> ---
>> >> I've re-inventing the same wheel three times in as many drivers (omapdrm,
>> >> tilcdc, and in upcoming msm driver I need two of 'em).  I guess it is
>> >> time to helper-up.
>> >>
>> >> I'll update omapdrm and tilcdc to use this as well, but I figured I'd
>> >> send an RFC first in case anyone wants to get their bikeshed on.  If
>> >> there are other drivers that could use this, and are straightforward
>> >> to convert over, let me know and I can update them as well.
>> >
>> >
>> > One thing drm/i915 needs is to be able to flush the workqueue (to make
>> > sure we don't pile up giant amounts of buffers waiting to be unpinned
>> > and so temporarily leak a bit of memory). So some way to synchronously
>> > flush out flip functions would be required (and make sure all that
>> > have been queued up to that point are really completed). But at that
>> > point a separate workqueue sounds simpler, so I wonder a bit what this
>> > gains us? At roughly 50Hz flip work functions aren't really that
>> > performance critical imo ...
>>
>> you could of course do separate or same workqueue from your main
>> render-complete workqueue, or flush the workqueue at whatever point
>> you want..  I suppose if you are queuing from a context that is safe
>> to sleep you could have a blocking-queue function.  I haven't really
>> needed anything like that so far, but feel free to extend the
>> flip-work helper however you need to be useful for i915.  (Or point me
>> at the bits in i915 that do something similar and I'll have a think
>> about how to convert that)
>
> queue_work in do_intel_finish_page_flip and flush_work in
> intel_crtc_page_flip is the relevant code. The former is called from irq
> context.

ok, it looks like intel_crtc_page_flip() would call
drm_flip_work_queue() and then something like:

  if (kfifo_len(&flipwork->fifo) >= 2)
    flush_workqueue(wq)

maybe a static inline fxn to wrap the kfifo_len().. but that is the
basic idea.  Although seems like you have a few other things wrapped
up in intel_unpin_work.  I guess you could move the pending event to
the crtc because you can only have one pending flip at a time (and
maybe enable_stall_check  I didn't really look at the stall-check
stuff..)

BR,
-R




> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch