Re: [krogoth][PATCH 1/3] gstreamer1.0-plugins-bad: kmssink: remove DCE dependencies

[Denys Dmytriyenko <denys@ti.com>]

On Thu, Mar 16, 2017 at 02:19:07PM -0400, Ruei, Eric wrote:
> On 3/16/2017 2:02 PM, Denys Dmytriyenko wrote:
> >Thanks!
> >Do you plan to support am3?
> 
> No, the kmssink will not work on AM3 because it requires overlay DRM
> plane. And there is no customer requirement for advanced features
> for AM3.
> 
> >Is it applicable to morty? Any patches planned?
> No, it is not applicable to Morty because new version of gstreamer
> (1.9) is for morty.
> 
> I do plan to submit those patches for review.

morty has 1.8.3 - I meant the change in general, not specific to 1.6.3...


> >Also, see inline below.
> >
> >
> >On Thu, Mar 16, 2017 at 01:29:35PM -0400, Eric Ruei wrote:
> >>- remove DCE dependencies from kmssink
> >>- AM4(ti43x) shares the same branch as of AM5(omap-a15)
> >>
> >>Signed-off-by: Eric Ruei <e-ruei1@ti.com>
> >>---
> >> .../0001-kmssink-remove-DCE-dependencies.patch     | 96 ++++++++++++++++++++++
> >> .../gstreamer1.0-plugins-bad_1.6.3.bbappend        |  8 +-
> >> 2 files changed, 102 insertions(+), 2 deletions(-)
> >> create mode 100644 meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad/0001-kmssink-remove-DCE-dependencies.patch
> >>
> >>diff --git a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad/0001-kmssink-remove-DCE-dependencies.patch b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad/0001-kmssink-remove-DCE-dependencies.patch
> >>new file mode 100644
> >>index 0000000..e050e08
> >>--- /dev/null
> >>+++ b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad/0001-kmssink-remove-DCE-dependencies.patch
> >>@@ -0,0 +1,96 @@
> >>+From 6b04eb39296caec717d310872c1f0acfa85c5ae7 Mon Sep 17 00:00:00 2001
> >>+From: Eric Ruei <e-ruei1@ti.com>
> >>+Date: Wed, 15 Mar 2017 09:08:13 -0400
> >>+Subject: [PATCH 1/2] kmssink: remove DCE dependencies
> >>+
> >>+Replace DCE related API calls to libdrm and omapdrm API calls
> >>+to decouple DCE from kmssink
> >>+
> >>+Signed-off-by: Eric Ruei <e-ruei1@ti.com>
> >>+---
> >>+ configure.ac         |  1 -
> >>+ sys/kms/Makefile.am  |  2 --
> >>+ sys/kms/gstkmssink.c | 17 +++++++++++++----
> >>+ 3 files changed, 13 insertions(+), 7 deletions(-)
> >>+
> >>+diff --git a/configure.ac b/configure.ac
> >>+index 2e0d1f3..2b1ecaa 100644
> >>+--- a/configure.ac
> >>++++ b/configure.ac
> >>+@@ -2119,7 +2119,6 @@ dnl *** kms ***
> >>+ translit(dnm, m, l) AM_CONDITIONAL(USE_KMS, true)
> >>+ AG_GST_CHECK_FEATURE(KMS, [kmssink], kms, [
> >>+   PKG_CHECK_MODULES([DRM], [libdrm libdrm_omap], HAVE_KMS=yes, HAVE_KMS=no)
> >>+-  PKG_CHECK_MODULES(LIBDCE, [libdce >= 1.0.0], HAVE_KMS=yes, HAVE_KMS=no)
> >>+   AC_SUBST(DRM_CFLAGS)
> >>+   AC_SUBST(DRM_LIBS)
> >>+ ])
> >>+diff --git a/sys/kms/Makefile.am b/sys/kms/Makefile.am
> >>+index 6d56073..cc7353e 100644
> >>+--- a/sys/kms/Makefile.am
> >>++++ b/sys/kms/Makefile.am
> >>+@@ -9,7 +9,6 @@ libgstkmssink_la_CFLAGS = \
> >>+ 	$(GST_PLUGINS_BAD_CFLAGS) \
> >>+ 	$(GST_PLUGINS_BASE_CFLAGS) \
> >>+ 	$(GST_BASE_CFLAGS) \
> >>+-	$(LIBDCE_CFLAGS) \
> >>+ 	$(GST_CFLAGS) \
> >>+ 	$(DRM_CFLAGS)
> >>+
> >>+@@ -17,7 +16,6 @@ libgstkmssink_la_LIBADD = \
> >>+ 	$(GST_PLUGINS_BASE_LIBS) \
> >>+ 	$(GST_BASE_LIBS) \
> >>+ 	$(GST_LIBS) \
> >>+-	$(LIBDCE_LIBS) \
> >>+ 	$(DRM_LIBS) \
> >>+ 	-lgstvideo-$(GST_API_VERSION) \
> >>+ 	$(top_builddir)/gst-libs/gst/drm/libgstdrm-$(GST_API_VERSION).la
> >>+diff --git a/sys/kms/gstkmssink.c b/sys/kms/gstkmssink.c
> >>+index 17e6407..9795bdf 100644
> >>+--- a/sys/kms/gstkmssink.c
> >>++++ b/sys/kms/gstkmssink.c
> >>+@@ -31,7 +31,6 @@
> >>+ #include "gstkmssink.h"
> >>+ #include "gstkmsbufferpriv.h"
> >>+
> >>+-#include <libdce.h>
> >>+ #include <omap_drm.h>
> >>+ #include <omap_drmif.h>
> >>+ #include <xf86drmMode.h>
> >>+@@ -516,13 +515,18 @@ gst_kms_sink_start (GstBaseSink * bsink)
> >>+
> >>+   sink = GST_KMS_SINK (bsink);
> >>+
> >>+-  drm_dev = dce_init ();
> >>++  drm_fd = open("/dev/dri/card0", O_RDWR | O_CLOEXEC);
> >>++  if(drm_fd == NULL)
> >>++    goto drm_failed;
> >>++  else {
> >>++    drm_dev = omap_device_new(drm_fd);
> >>++  }
> >>++
> >>+   if (drm_dev == NULL)
> >>+     goto device_failed;
> >>+   else {
> >>++    sink->fd = drm_fd;
> >>+     sink->dev = drm_dev;
> >>+-    sink->fd = dce_get_fd ();
> >>+-    drm_fd = dce_get_fd ();
> >>+   }
> >>+
> >>+   sink->resources = drmModeGetResources (sink->fd);
> >>+@@ -539,6 +543,11 @@ fail:
> >>+   gst_kms_sink_reset (sink);
> >>+   return FALSE;
> >>+
> >>++drm_failed:
> >>++  GST_ELEMENT_ERROR (sink, RESOURCE, FAILED,
> >>++      (NULL), ("drmOpen failed"));
> >>++  goto fail;
> >>++
> >>+ device_failed:
> >>+   GST_ELEMENT_ERROR (sink, RESOURCE, FAILED,
> >>+       (NULL), ("omap_device_new failed"));
> >>+--
> >>+1.9.1
> >>+
> >>diff --git a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_1.6.3.bbappend b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_1.6.3.bbappend
> >>index b5ccebf..2c903d4 100644
> >>--- a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_1.6.3.bbappend
> >>+++ b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_1.6.3.bbappend
> >>@@ -8,7 +8,6 @@ PACKAGECONFIG_append_ti43x = " ${@bb.utils.contains('DISTRO_FEATURES','wayland',
> >> PACKAGECONFIG_append_ti33x = " ${@bb.utils.contains('DISTRO_FEATURES','wayland','wayland','',d)}"
> >>
> >> DEPENDS_append_omap-a15 = " \
> >>-    libdce \
> >>     libdrm \
> >> "
> >>
> >>@@ -18,10 +17,14 @@ DEPENDS_append_ti43x = " \
> >>
> >> DEPENDS_append_ti33x = " \
> >>     libdrm \
> >>+"
> >>+
> >>+SRC_URI_append_omap-a15 = " \
> >>+    file://0001-kmssink-remove-DCE-dependencies.patch \
> >> "
> >>
> >> SRC_URI_append_ti43x = " \
> >>-    file://0001-gstwaylandsink-Add-mouse-drag-and-drop-support.patch \
> >
> >This patch got dropped ^
> >If it was intentional, should mention that in the commit message.
> >
> >
> 
> Yes, it was intentional, a similar patch was covered by the commit
> ID update. Do you want me to resubmit this patch?

Yes, please. Just a short one-liner should be good enough. Thanks.


> >>+    file://0001-kmssink-remove-DCE-dependencies.patch \
> >> "
> >>
> >> SRC_URI_append_ti33x = " \
> >>@@ -38,6 +41,7 @@ SRC_URI = "git://git.ti.com/glsdk/gstreamer1-0-plugins-bad.git;protocol=git;bran
> >> S = "${WORKDIR}/git"
> >>
> >> SRCREV_omap-a15 = "eb41f1c8fde05f87587c83d157835e3b2d19a298"
> >>+SRCREV_ti43x = "eb41f1c8fde05f87587c83d157835e3b2d19a298"
> >> SRCREV = "d0160ca810be30bf2b2e7681f5047933402efb52"
> >>
> >> PR = "r25"
> >>--
> >>1.9.1
> >>
> >>_______________________________________________
> >>meta-arago mailing list
> >>meta-arago@arago-project.org
> >>http://arago-project.org/cgi-bin/mailman/listinfo/meta-arago
>