After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 794839 - kmssink: Add Allwinner DRM driver to the drivers list
kmssink: Add Allwinner DRM driver to the drivers list
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-03-30 12:45 UTC by Paul Kocialkowski
Modified: 2018-03-30 15:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Initial patch to implement the feature (1.04 KB, patch)
2018-03-30 12:52 UTC, Paul Kocialkowski
accepted-commit_now Details | Review
modetest output for sun4i-drm with extra patches (6.41 KB, text/plain)
2018-03-30 13:20 UTC, Paul Kocialkowski
  Details

Description Paul Kocialkowski 2018-03-30 12:45:34 UTC
The Allwinner DRM driver is currently not in the drivers list, although this is required to get kmspipe working on these devices with the mainline Linux kernel. The DRM driver supports multiple planes and is ready for kmssink.
Comment 1 Paul Kocialkowski 2018-03-30 12:52:44 UTC
Created attachment 370338 [details] [review]
Initial patch to implement the feature
Comment 2 Nicolas Dufresne (ndufresne) 2018-03-30 13:13:54 UTC
Review of attachment 370338 [details] [review]:

Thanks ! I still have hope we'll figure-out how to enumerate them, but that's the way it is. Can you attach modetest -M sun4i-drm, for later reference. I'll start collecting that for later work on kmssink. Is that driver an atomic driver ?
Comment 3 Nicolas Dufresne (ndufresne) 2018-03-30 13:19:11 UTC
Merged, I didn't realized that the bug link was wrong in your patch. Please be careful next time, or use git-bz. Don't be offended that I dropped the Signed-of-by, these have no use in GStreamer project. The author is always the code owner in GStreamer.

Author: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Date:   Fri Mar 30 13:47:00 2018 +0200

    kmssink: Add support for the Allwinner DRM driver (sun4i-drm)
    
    This adds the sun4i DRM driver to the list of DRM drivers in kmssink.
    The driver allows displaying video in either the main plane or an
    overlay plane.
    
    https://bugzilla.gnome.org/attachment.cgi?bugid=794839
Comment 4 Paul Kocialkowski 2018-03-30 13:20:03 UTC
Created attachment 370344 [details]
modetest output for sun4i-drm with extra patches

This was taken from a kernel tree that includes patches that are still under review, but should eventually land (there are no show-stoppers).

Mainly, the untiled modifier (reporting as UNKNOWN MODIFIER) as well as some YUV formats have not yet landed.
Comment 5 Paul Kocialkowski 2018-03-30 13:23:46 UTC
As for the bug link, I followed https://gstreamer.freedesktop.org/documentation/contribute/index.html#how-to-submit-patches which mentions show_bug.cgi over attachment.cgi.

Note that I proceeded the same way for the other patch I just submitted #794840
Comment 6 Paul Kocialkowski 2018-03-30 13:27:29 UTC
I forgot to comment on atomicity: this is indeed an atomic DRM driver!
Comment 7 Nicolas Dufresne (ndufresne) 2018-03-30 13:56:06 UTC
Sorry, accidental change, thanks for the info. Fyi, atomic framework implements legacy APIs in a weird way, ModeSetPlane will block up to next vblank and pageflip becomes a no up. We notice that the framerate get halved on these drivers. I need to review and test the workaround:

https://bugzilla.gnome.org/show_bug.cgi?id=793339
Comment 8 Nicolas Dufresne (ndufresne) 2018-03-30 13:59:09 UTC
(In reply to Paul Kocialkowski from comment #5)
> As for the bug link, I followed
> https://gstreamer.freedesktop.org/documentation/contribute/index.html#how-to-
> submit-patches which mentions show_bug.cgi over attachment.cgi.

The documentation is correct, you called into attachment.cgi instead of show_bug.cgi.
  
  https://bugzilla.gnome.org/show_bug.cgi?id=987654

> 
> Note that I proceeded the same way for the other patch I just submitted
> #794840

Ok, please update.
Comment 9 Paul Kocialkowski 2018-03-30 14:01:02 UTC
Thanks for the information (and the incredibly fast review), this is definitely good to know!
Comment 10 Nicolas Dufresne (ndufresne) 2018-03-30 14:01:33 UTC
Comment on attachment 370344 [details]
modetest output for sun4i-drm with extra patches

Nice, that's seems pretty flexible display.
Comment 11 Paul Kocialkowski 2018-03-30 14:02:42 UTC
Oh you're right, my mistake. My other patch should be fine then.

Note that the pushed commit apparently still has attachement.cgi (which added to my confusion): https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=a53019068da2f86336decd73a96ad4a68ac1b01d
Comment 12 Nicolas Dufresne (ndufresne) 2018-03-30 15:01:04 UTC
(In reply to Paul Kocialkowski from comment #11)
> Oh you're right, my mistake. My other patch should be fine then.
> 
> Note that the pushed commit apparently still has attachement.cgi (which
> added to my confusion):
> https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/
> ?id=a53019068da2f86336decd73a96ad4a68ac1b01d

Yes, I noticed to late unfortunately. But a human can see the error and find back the bug.