GNOME Bugzilla – Bug 793339
kmssink: Renders at half my display refresh rate
Last modified: 2018-11-03 14:17:58 UTC
While doing some latency test, I notice that kmssink renders at half the speed of my display. My test pipeline was: gst-launch-1.0 -v videotestsrc ! video/x-raw,framerate=60/1 ! fpsdisplaysink text-overlay=0 video-sink=kmssink Commenting out the code that (according to comment) tries to wait for previous frame to finish fixes it. https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/sys/kms/gstkmssink.c#n1499 My impression is that we endup waiting for an extra vblank all the time as the previous blank is behind.
Created attachment 369527 [details] [review] Skip VSYNC wait for synchronous SetPlane call The new atomic DRM driver implements synchronous DRM_IOCTL_MODE_SETPLANE ioctl which unlike legacy DRM driver blocks until a VSYNC is done. As a workaround we skip the explicit wait for VSYNC done using drmWaitVBlank and drmModePageFlip whenever drmModeSetPlane is used by an atomic DRM driver, until kmssink starts supporting fully atomic DRM drivers.
Hi, Attached a proposed fix, tested on xilinx platform which uses atomic DRM API. Appreciate review for the same. Thanks, Devarsh
Hi Devarsh, Sorry for the delay. If I understand correctly, although related, your patch doesn't fix the case reported by Nicolas. I recommend you is to open a new bug report with your patch.
Hey Victor, I think this patche partially fixes the issue I have describe here. It fixes it for Atomic drivers, as the legacy implementation completely ignores async page flip calls, and make a synchronous commit internally. So doing pageflip or waiting for vlank with any atomic drivers means that you will likely wait for an extra blank, hence reducing the render rate by two. It's partial, because some driver ended up with that behaviour before their transition to atomic, to get rid of the tearing. There is the other issue I've showed you on IRC yesterday, and that one need another bug, I'll file one today.
Created attachment 370952 [details] [review] The drm patch in rockchip 4.4 I talked to ndufresne in the IRC, in the rockchip kernel, we adapt a new method to solve the performance problem with 4k video. This patch won't solve the performance unless modified the kernel. Here is the log without the kernel patch I uploaded, it looks root@firefly-rk3288:~# gst-launch-1.0 --gst-debug=kmssink:5 filesrc location=videos/tokyo_2013_4k.mp4 ! qtdemux ! h264parse ! mppv ideodec ! queue ! kmssink Setting pipeline to PAUSED ... 0:00:00.562194390 393 0x137460 INFO kmssink gstkmssink.c:358:log_drm_version:<kmssink0> DRM v1.0.0 [rockchip — RockChip Soc DRM — 20140818] 0:00:00.562868432 393 0x137460 INFO kmssink gstkmssink.c:405:get_drm_caps:<kmssink0> prime import (✓) / prime export (✓) / async page flip (✗) 0:00:00.563487057 393 0x137460 DEBUG kmssink gstkmssink.c:616:gst_kms_sink_start:<kmssink0> Atomic modesetting support enabled 0:00:00.573946517 393 0x137460 INFO kmssink gstkmssink.c:539:ensure_allowed_caps:<kmssink0> ignoring format RG 24 .... 0:00:00.641102898 393 0x119400 DEBUG kmssink gstkmssink.c:1034:gst_kms_sink_set_caps:<kmssink0> negotiated cap$ = video/x-raw, format=(string)NV12, width=(int)3840, height=(int)2160, interlace-mode=(string)progressive, multiview-mode=(strin$ )mono, multiview-flags=(GstVideoMultiviewFlagsSet)0:ffffffff:/right-view-first/left-flipped/left-flopped/right-flipped/right-flop$ ed/half-aspect/mixed-mono, pixel-aspect-ratio=(fraction)1/1, chroma-site=(string)mpeg2, colorimetry=(string)bt2020, framerate=(fr$ ction)24/1 0:00:00.641244648 393 0x119400 DEBUG kmssink gstkmssink.c:1557:gst_kms_sink_drain:<kmssink0> draining Pipeline is PREROLLED ... Setting pipeline to PLAYING ... New clock: GstSystemClock WARNING: from element /GstPipeline:pipeline0/GstKMSSink:kmssink0: A lot of buffers are being dropped. Additional debug info: ../../../../git/libs/gst/base/gstbasesink.c(2902): gst_base_sink_is_too_late (): /GstPipeline:pipeline0/GstKMSSink:kmssink0: There may be a timestamping problem, or this computer is too slow. WARNING: from element /GstPipeline:pipeline0/GstKMSSink:kmssink0: A lot of buffers are being dropped. Additional debug info: ../../../../git/libs/gst/base/gstbasesink.c(2902): gst_base_sink_is_too_late (): /GstPipeline:pipeline0/GstKMSSink:kmssink0: There may be a timestamping problem, or this computer is too slow. WARNING: from element /GstPipeline:pipeline0/GstKMSSink:kmssink0: A lot of buffers are being dropped. Additional debug info: ../../../../git/libs/gst/base/gstbasesink.c(2902): gst_base_sink_is_too_late (): /GstPipeline:pipeline0/GstKMSSink:kmssink0: There may be a timestamping problem, or this computer is too slow. WARNING: from element /GstPipeline:pipeline0/GstKMSSink:kmssink0: A lot of buffers are being dropped.
Kernel patches have no place here. Though thanks for sharing. What was the pipeline you are testing ? Did you add a queue right before kmssink ? Turning kmssink render into something async should be filed on seperate bug imho.
Review of attachment 370952 [details] [review]: .
Yes, that patch is just used as a reference, It is not a good patch either, I just want to tell you what the rockchip does. It the pipeline can be found the previous post.
After rethinking this, if this patches is needed, it means that KMS Atomic drivers are simply not backward compatible peformance whise. At the same time, I'm pretty sure Devarsh is getting full frame rate with this patch, and I also see full framerate on i915. I'm wondering what would triggers this on Rockchip ...
Created attachment 371344 [details] [review] WIP: kmssink: Skip sync when atomic driver is detected DON'T MERGE, this causes regressions on Intel Atomic driver emulates the mode setting in a way that it blocks until the frame buffer is rendered. As a side effect, waiting for a vblank, or page flips halves the frame rate.
Patch didn't apply anymore, and just ended up rewriting it as it was not really following the code. This patch is not acceptable, because turning on that capability apparently breaks scaling on Intel, something Devarsh cannot test on his driver, has he got no scaling support. This patch is not going upstream.
Review of attachment 371344 [details] [review]: .
(In reply to Nicolas Dufresne (ndufresne) from comment #9) > After rethinking this, if this patches is needed, it means that KMS Atomic > drivers are simply not backward compatible peformance whise. At the same > time, I'm pretty sure Devarsh is getting full frame rate with this patch, > and I also see full framerate on i915. I'm wondering what would triggers > this on Rockchip ... It's not a matter of atomic vs. non-atomic drivers, it's that there is absolutely no specification of what SetPlane does. Some drivers block until the next vblank to apply, and some do no blocking at all - even to the point of generating pagefaults and display glitches if you destroy a buffer which is still active on a plane. Most atomic drivers wait until vblank, but not all (e.g. amdgpu still does it async, although I believe only on older hardware, and newer hardware has the blocking behaviour). Most non-atomic drivers are non-blocking, but some are blocking. Many drivers (e.g. exynos and I believe also rockchip) changed their behaviour with different kernel versions. The only way to get predictable and reliable behaviour from KMS planes is to use the atomic API. This has always been the case, and is the reason why Weston never used planes unless the user explicitly enabled them with a magic key combination. On the other hand, using atomic allows us to enable use of overlay planes by default. As an aside, the scaling behaviour is also completely unspecified when using SetPlane. If the hardware cannot apply the requested scaling factor, drivers may clip or enlarge either source or destination bounds completely arbitrarily in order to get _some_ kind of image on screen. This becomes an error in atomic, so it's possible that flipping the client cap on just exposes this rather than silent clipping or rounding.
Just an update, it seems there is a move along making the emulation asynchronous (though likely racy) in order to better support this legacy usage of the API. I still think we could improve this legacy a little, by syncing before releasing a buffer, rather then after updating (late sync). This would also be less in the way when we add atomic support.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/654.