Patches for performance improvement

Patches for performance improvement

[Kannan, Vandana]

[-- Attachment #1.1: Type: text/plain, Size: 795 bytes --]

Hi,

            For sprite performance improvement, instead of the patch "drm/i915: Don't wait for vblank for sprite plane flips", we made use of Chris Wilson's patches from "RFC asynchronous vblank tasks" and verified that it gives the expected performance boost.
            Chris's patches have not been merged yet, specifically,
            drm/i915: Introduce vblank work function
            drm/i915: Asynchronously unpin the old   framebuffer after the next vblank
            drm/i915/sprite: Make plane switching     asynchronous

            Will the updated patches for these be coming any time soon ? We require these patches as it gives us a very good performance improvement.
            Please let me know, I can verify with the updated patches.

Thanks,
Vandana


[-- Attachment #1.2: Type: text/html, Size: 4796 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: Patches for performance improvement

[Ville Syrjälä]

On Sun, Aug 25, 2013 at 04:35:52AM +0000, Kannan, Vandana wrote:
> Hi,
> 
>             For sprite performance improvement, instead of the patch "drm/i915: Don't wait for vblank for sprite plane flips", we made use of Chris Wilson's patches from "RFC asynchronous vblank tasks" and verified that it gives the expected performance boost.
>             Chris's patches have not been merged yet, specifically,
>             drm/i915: Introduce vblank work function
>             drm/i915: Asynchronously unpin the old   framebuffer after the next vblank
>             drm/i915/sprite: Make plane switching     asynchronous
> 
>             Will the updated patches for these be coming any time soon ? We require these patches as it gives us a very good performance improvement.
>             Please let me know, I can verify with the updated patches.

I don't seem to have said patches in my mailbox, so I can't comment on
the details, but...

My main concern is again the contents of the old fb after the setplane
ioctl returns. Due to the current synchronous nature of the ioctl,
userspace knows that when the ioctl returns the flip has completed, and
so can write the old fb again.

If the ioctl is async, userspace has to guess whether the old fb can
be safely written. We could solve this with a new event that gets sent
from setplane. We do have plenty of flags left in setplane so it's
certainly doable.

Also with the async ioctl userspace can issue another setplane before
the previous flip has completed. I personally think that's a nice
feature that allows no-fps-limits triple buffering, but it does make it
a bit more difficult to know which fb is actually being used at a given
time. I have it solved in my atomic page flip code though, as the
drm_flip_helper contraption knows how to track this stuff.

Maybe someone should just start pulling in the drm_flip_helper and
associated code from the atomic code to make this feasible. The new
event type I added there would also be suitable for this purpose as
it carries the plane id and information on which fb is freed by the
operation. The other option is to just add a new event, and disallow
another setplane until the previous has completed (maybe try to
generalize the current page flip code a bit to handle sprites as
well as primary planes?)

-- 
Ville Syrjälä
Intel OTC

Re: Patches for performance improvement

[Daniel Vetter]

On Mon, Aug 26, 2013 at 10:16 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> Maybe someone should just start pulling in the drm_flip_helper and
> associated code from the atomic code to make this feasible. The new
> event type I added there would also be suitable for this purpose as
> it carries the plane id and information on which fb is freed by the
> operation. The other option is to just add a new event, and disallow
> another setplane until the previous has completed (maybe try to
> generalize the current page flip code a bit to handle sprites as
> well as primary planes?)

I think extending the current pageflip ioctl to planes would be a
suitable stop-gap measure until we have nuclear pageflips for real.
It's only a 90% solution though since we can't update anything else
than the base address with the current pageflip ioctl.

Actually I think doing this would be a neat way to start pulling in
parts of the nuclear pageflip code and getting all the flip
complete/event signalling straigthened out (and properly tested with
igt like the primary plane flips).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: Patches for performance improvement

[Chris Wilson]

On Mon, Aug 26, 2013 at 11:16:40AM +0300, Ville Syrjälä wrote:
> On Sun, Aug 25, 2013 at 04:35:52AM +0000, Kannan, Vandana wrote:
> > Hi,
> > 
> >             For sprite performance improvement, instead of the patch "drm/i915: Don't wait for vblank for sprite plane flips", we made use of Chris Wilson's patches from "RFC asynchronous vblank tasks" and verified that it gives the expected performance boost.
> >             Chris's patches have not been merged yet, specifically,
> >             drm/i915: Introduce vblank work function
> >             drm/i915: Asynchronously unpin the old   framebuffer after the next vblank
> >             drm/i915/sprite: Make plane switching     asynchronous
> > 
> >             Will the updated patches for these be coming any time soon ? We require these patches as it gives us a very good performance improvement.
> >             Please let me know, I can verify with the updated patches.
> 
> I don't seem to have said patches in my mailbox, so I can't comment on
> the details, but...
> 
> My main concern is again the contents of the old fb after the setplane
> ioctl returns. Due to the current synchronous nature of the ioctl,
> userspace knows that when the ioctl returns the flip has completed, and
> so can write the old fb again.
> 
> If the ioctl is async, userspace has to guess whether the old fb can
> be safely written. We could solve this with a new event that gets sent
> from setplane. We do have plenty of flags left in setplane so it's
> certainly doable.

The patches were originally motivated because the setplane ioctl was
doing the opposite of the userspace expectations. Its user is quite
happy to have the ioctl pretend it completed immediately without an
event. (Okay, not happy, but the whole setup just ignores the complexity
by have a small ringbuffer of frames and hoping for the best [mplayer].)

The issue and discussion is as old as the original patch for setplane.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre