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 795228 - kmssink: Update driver name for Xilinx DRM-KMS Driver
kmssink: Update driver name for Xilinx DRM-KMS Driver
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.14.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-04-13 09:00 UTC by Devarsh Thakkar
Modified: 2018-07-25 12:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Updates xilinx DRM-KMS driver name (980 bytes, patch)
2018-04-13 09:00 UTC, Devarsh Thakkar
none Details | Review
kmssink: Add new entry for Xilinx DRM Driver (1.19 KB, patch)
2018-04-18 11:15 UTC, Devarsh Thakkar
committed Details | Review

Description Devarsh Thakkar 2018-04-13 09:00:45 UTC
Created attachment 370884 [details] [review]
Updates xilinx DRM-KMS driver name

The older xilinx DRM-KMS driver "xilinx_drm" is deprecated and a new driver "xlnx" supporting atomic DRM API has been implemented, so the attached patch addresses the same.
Comment 1 Víctor Manuel Jáquez Leal 2018-04-13 09:38:30 UTC
Review of attachment 370884 [details] [review]:

::: sys/kms/gstkmssink.c
@@ +170,3 @@
   static const char *drivers[] = { "i915", "radeon", "nouveau", "vmwgfx",
     "exynos", "amdgpu", "imx-drm", "rockchip", "atmel-hlcdc", "msm",
+    "xlnx", "vc4",

I don't think it is a good idea to remove the old name, since we would be breaking the back compatibility. It is not expected that everybody will have the latest kernel.
Comment 2 Devarsh Thakkar 2018-04-13 11:03:34 UTC
Hi Victor,

Thanks for the quick review, I didn't keep older driver because of below reasons :

1) As I understand only the new DRM driver i.e "xlnx" is being upstreamed (https://patchwork.kernel.org/patch/10162095/) to community and it was also confirmed by xilinx DRM driver owners that older DRM driver entry is not needed in kmssink.

2) The older DRM driver "xilinx_drm" is also disabled in xilinx kernel https://github.com/Xilinx/linux-xlnx/ ) now. Most of the users for xilinx board use yocto build which internally use a specific commit for gstreamer and kernel. So I guess if someone is still using older DRM from previous yocto build of xilinx they might be also using an older gstreamer release.

Kindly let me know your opinion.

Thanks,
Devarsh
Comment 3 Víctor Manuel Jáquez Leal 2018-04-16 12:25:50 UTC
(In reply to Devarsh Thakkar from comment #2)
> Hi Victor,
> 
> Thanks for the quick review, I didn't keep older driver because of below
> reasons :
> 
> 1) As I understand only the new DRM driver i.e "xlnx" is being upstreamed
> (https://patchwork.kernel.org/patch/10162095/) to community and it was also
> confirmed by xilinx DRM driver owners that older DRM driver entry is not
> needed in kmssink.
> 
> 2) The older DRM driver "xilinx_drm" is also disabled in xilinx kernel
> https://github.com/Xilinx/linux-xlnx/ ) now. Most of the users for xilinx
> board use yocto build which internally use a specific commit for gstreamer
> and kernel. So I guess if someone is still using older DRM from previous
> yocto build of xilinx they might be also using an older gstreamer release.
> 
> Kindly let me know your opinion.
> 
> Thanks,
> Devarsh

I would keep both, adding a comment of the deprecation of the previous symbol.

Please rebase your patch with current master.
Comment 4 Devarsh Thakkar 2018-04-18 11:15:23 UTC
Created attachment 371084 [details] [review]
kmssink: Add new entry for Xilinx DRM Driver

Hi Victor,

As per review comment, rebased the patch on top of master branch, added a separate driver entry for xlnx and didn't delete old entry, also added a note describing the deprecation.

Thanks,
Devarsh
Comment 5 Víctor Manuel Jáquez Leal 2018-04-18 19:18:38 UTC
Attachment 371084 [details] pushed as d0575a0 - kmssink: Add new entry for Xilinx DRM Driver
Comment 6 Víctor Manuel Jáquez Leal 2018-04-18 19:20:18 UTC
I've ended changing a bit the patch. I believe the code is clearer. Thanks!
Comment 7 Nicolas Dufresne (ndufresne) 2018-07-25 12:01:57 UTC
Added to 1.14