[Mesa-dev] [PATCH v2 2/4] st/omx_tizonia: Add --enable-omx-tizonia flag and build files
Gurkirpal Singh
gurkirpal204 at gmail.com
Mon Aug 14 15:19:18 UTC 2017
On Mon, Aug 14, 2017 at 8:05 PM, Leo Liu <leo.liu at amd.com> wrote:
>
>
> On 08/14/2017 05:46 AM, Julien Isorce wrote:
>
> Hi Leo,
>
> >It would be better if you can extract the common code between bellagio
> and tizonia to avoid the duplication.
>
> This is something Gurkirpal and me discussed, like having
> state_trackers/omx/common, state_trackers/omx/bellagio,
> state_trackers/omx/tizonia. To anticipate that Gurkirpal sent an email
> https://www.mail-archive.com/mesa-dev@lists.freedesktop.org/msg153562.html
> during the Community Bonding period in May (a few weeks from the date
> (early May) students are officially accepted to the date (end May) where
> student actually starts working)
>
> The problem with the directory layout above is that it would look like it
> will be built together in the same shared library. This is not good because
> it is suppose to export OMX_ComponentInit for each target. Of course this
> would still be possible to have 2 shared libs but I believe in
> state_trackers dir, all sub dir are for one target. Also we were afraid
> that there would be other limitations so we decided to go for a separate
> directory.
> And since there were no objections in Gurkirpal's mail above we went this
> way.
>
> Now if I look into state_trackers/omx_tizonia, in fact the common code
> with state_trackers/omx_bellagio does not have anything to do with omx. For
> example "slice_header". Maybe it can be moved to gallium/auxiliary/vl/ like
> it is done already for vl_rbsp_init. Same for omx_get_screen.
>
> So I suggest this 'factorization' to be not a blocker for merging
> state_trackers/omx_tizonia into usptream. But later on we can move
> gradually bits from state_trackers/omx_bellagio to gallium/auxiliary. And
> then make state_trackers/omx_tizonia use it as well.
> This way you will only bother about maintaining
> state_trackers/omx_bellagio the time being. This also allows to not slowly
> get its work lost.
>
> Of course we would have done differently if we knew advance. But as today
> Gurkirpal won't have enough time to do this factorization/move as the
> project ends in 1 week. Having all of this merged in upstream is not
> mandatory to succeed the project but Gurkirpal will need some rest after
> these 3 months of hard work. And who knows what happens after, whether he
> will still be around after sometimes or not. And this is entirely up to him.
>
> So again I suggest this factorization/move not to be a blocker for the
> reasons above. What do you think?
>
>
> When the whole project will be completed, as least same status as
> bellagio? Is there anyone keep working on this project after this step 1?
>
Hi, I plan on continuing working on this after the GSoC project ends.
>
> The left stuff compared to bellagio are codecs of mpeg2 and hevc, and
> encoding that is more important, since the most usage for omx currently is
> to use transcoding from one codec to another.
>
> Other is the transcoding performance, at least meet the performance of
> bellagio's.
>
> If the above could be addressed at early time, we could remove Bellagio
> completely, then there is no need to do any refactor now.
>
The original plan was this but making gst-omx OMX IL 1.2 compatible has
been a bit time consuming.
>
>
> BTW what's the plan for new feature like EGL Images?
>
We've been able to make it work but with wrong colours in output. I've
written more about it and the performance comparison in my blog post here
https://singhcodes.wordpress.com/2017/08/04/gsoc-2017-third-phase-starts/
But recently the EGLImage hook stopped working and I've opened the issue
https://github.com/gpalsingh/mesa/issues/15 to keep track of it.
This happened after we fixed another issue which made changes to gst-omx
which might be the cause
https://github.com/gpalsingh/mesa/issues/2#issuecomment-321839195
I'm still looking into this.
Cheers
>
> Regards,
> Leo
>
>
>
> Thx
> Julien
>
> On 13 August 2017 at 16:52, Gurkirpal Singh <gurkirpal204 at gmail.com>
> wrote:
>
>>
>>
>> On Sun, Aug 13, 2017 at 8:47 AM, Leo Liu <leo.liu at amd.com> wrote:
>>
>>> Where is the patch 1?
>>
>> Sorry for the patches got messed up somehow while sending, I could only
>> see two patches on mail-archive but three on patchwork.
>> Tried two times and same result.
>> About the first one I got a mail saying that it was too large has been
>> put aside for mod approval.
>> The changes I made were to just rename the st/omx directory to
>> st/omx_bellagio (the reason it became large)
>> and renaming bits in the configure.ac and Makefiles.
>>
>>>
>>>
>>>
>>> On 08/12/2017 12:07 PM, Gurkirpal Singh wrote:
>>>
>>>> Coexist with --enable-omx so they can be built independently
>>>> Detect tizonia package config file
>>>> Generate libomxtiz_mesa.so and install it to libtizcore.pc::pluginsdir
>>>> Only compile empty source (target.c) for now.
>>>>
>>>> v2: Show error message when --enable-omx is used (Christian)
>>>> Use single PKG_CHECK_MODULES for omx_tizonia checks (Emil)
>>>> Use spaces instead of tabs
>>>> Add checks around omx-tizonia
>>>>
>>>> GSoC Project link: https://summerofcode.withgoogl
>>>> e.com/projects/#4737166321123328
>>>>
>>>> Signed-off-by: Gurkirpal Singh <gurkirpal204 at gmail.com>
>>>> Reviewed-and-Tested-by: Julien Isorce <julien.iso... at gmail.com>
>>>> ---
>>>> configure.ac | 40 ++++++++++++++-
>>>> src/gallium/Makefile.am | 4 ++
>>>> src/gallium/targets/omx-tizonia/Makefile.am | 77
>>>> +++++++++++++++++++++++++++++
>>>> src/gallium/targets/omx-tizonia/omx.sym | 11 +++++
>>>> src/gallium/targets/omx-tizonia/target.c | 2 +
>>>> 5 files changed, 132 insertions(+), 2 deletions(-)
>>>> create mode 100644 src/gallium/targets/omx-tizonia/Makefile.am
>>>> create mode 100644 src/gallium/targets/omx-tizonia/omx.sym
>>>> create mode 100644 src/gallium/targets/omx-tizonia/target.c
>>>>
>>>> diff --git a/configure.ac b/configure.ac
>>>> index 38af96a..5669695 100644
>>>> --- a/configure.ac
>>>> +++ b/configure.ac
>>>> @@ -85,6 +85,7 @@ dnl Versions for external dependencies
>>>> DRI2PROTO_REQUIRED=2.8
>>>> GLPROTO_REQUIRED=1.4.14
>>>> LIBOMXIL_BELLAGIO_REQUIRED=0.0
>>>> +LIBOMXIL_TIZONIA_REQUIRED=0.9.0
>>>> LIBVA_REQUIRED=0.38.0
>>>> VDPAU_REQUIRED=1.1
>>>> WAYLAND_REQUIRED=1.11
>>>> @@ -1216,14 +1217,19 @@ AC_ARG_ENABLE([vdpau],
>>>> [enable_vdpau=auto])
>>>> AC_ARG_ENABLE([omx],
>>>> [AS_HELP_STRING([--enable-omx],
>>>> - [DEPRECATED: Use --enable-omx-bellagio instead
>>>> @<:@default=auto@:>@])],
>>>> - [AC_MSG_ERROR([--enable-omx is deprecated. Use
>>>> --enable-omx-bellagio instead.])],
>>>>
>>>
>>> Is this in patch 1?
>>
>> Yes, it is so.
>>
>>>
>>>
>>> + [DEPRECATED: Use --enable-omx-bellagio or --enable-omx-tizonia
>>>> instead @<:@default=auto@:>@])],
>>>> + [AC_MSG_ERROR([--enable-omx is deprecated. Use
>>>> --enable-omx-bellagio or --enable-omx-tizonia instead.])],
>>>> [])
>>>> AC_ARG_ENABLE([omx-bellagio],
>>>> [AS_HELP_STRING([--enable-omx-bellagio],
>>>> [enable OpenMAX Bellagio library @<:@default=disabled@
>>>> :>@])],
>>>> [enable_omx_bellagio="$enableval"],
>>>> [enable_omx_bellagio=no])
>>>> +AC_ARG_ENABLE([omx-tizonia],
>>>> + [AS_HELP_STRING([--enable-omx-tizonia],
>>>> + [enable OpenMAX Tizonia library @<:@default=disabled@:>@])],
>>>> + [enable_omx_tizonia="$enableval"],
>>>> + [enable_omx_tizonia=no])
>>>> AC_ARG_ENABLE([va],
>>>> [AS_HELP_STRING([--enable-va],
>>>> [enable va library @<:@default=auto@:>@])],
>>>> @@ -1275,6 +1281,7 @@ if test "x$enable_opengl" = xno -a \
>>>> "x$enable_xvmc" = xno -a \
>>>> "x$enable_vdpau" = xno -a \
>>>> "x$enable_omx_bellagio" = xno -a \
>>>> + "x$enable_omx_tizonia" = xno -a \
>>>> "x$enable_va" = xno -a \
>>>> "x$enable_opencl" = xno; then
>>>> AC_MSG_ERROR([at least one API should be enabled])
>>>> @@ -2121,6 +2128,10 @@ if test -n "$with_gallium_drivers" -a
>>>> "x$with_gallium_drivers" != xswrast; then
>>>> PKG_CHECK_EXISTS([libomxil-bellagio >=
>>>> $LIBOMXIL_BELLAGIO_REQUIRED], [enable_omx_bellagio=yes],
>>>> [enable_omx_bellagio=no])
>>>> fi
>>>> + if test "x$enable_omx_tizonia" = xauto -a "x$have_omx_platform"
>>>> = xyes; then
>>>> + PKG_CHECK_EXISTS([libtizonia >= $LIBOMXIL_TIZONIA_REQUIRED],
>>>> [enable_omx_tizonia=yes], [enable_omx_tizonia=no])
>>>> + fi
>>>> +
>>>> if test "x$enable_va" = xauto -a "x$have_va_platform" = xyes; then
>>>> PKG_CHECK_EXISTS([libva >= $LIBVA_REQUIRED], [enable_va=yes],
>>>> [enable_va=no])
>>>> fi
>>>> @@ -2130,6 +2141,7 @@ if test "x$enable_dri" = xyes -o \
>>>> "x$enable_xvmc" = xyes -o \
>>>> "x$enable_vdpau" = xyes -o \
>>>> "x$enable_omx_bellagio" = xyes -o \
>>>> + "x$enable_omx_tizonia" = xyes -o \
>>>> "x$enable_va" = xyes; then
>>>> need_gallium_vl=yes
>>>> fi
>>>> @@ -2138,6 +2150,7 @@ AM_CONDITIONAL(NEED_GALLIUM_VL, test
>>>> "x$need_gallium_vl" = xyes)
>>>> if test "x$enable_xvmc" = xyes -o \
>>>> "x$enable_vdpau" = xyes -o \
>>>> "x$enable_omx_bellagio" = xyes -o \
>>>> + "x$enable_omx_tizonia" = xyes -o \
>>>> "x$enable_va" = xyes; then
>>>> PKG_CHECK_MODULES([VL], [x11-xcb xcb xcb-dri2 >=
>>>> $XCBDRI2_REQUIRED])
>>>> need_gallium_vl_winsys=yes
>>>> @@ -2172,6 +2185,18 @@ if test "x$enable_omx_bellagio" = xyes; then
>>>> fi
>>>> AM_CONDITIONAL(HAVE_ST_OMX_BELLAGIO, test "x$enable_omx_bellagio" =
>>>> xyes)
>>>> +if test "x$enable_omx_tizonia" = xyes; then
>>>> + if test "x$have_omx_platform" != xyes; then
>>>> + AC_MSG_ERROR([OMX requires at least one of the x11 or drm
>>>> platforms])
>>>> + fi
>>>> + PKG_CHECK_MODULES([OMX_TIZONIA],
>>>> + [libtizonia >= $LIBOMXIL_TIZONIA_REQUIRED
>>>> + tizilheaders >= $LIBOMXIL_TIZONIA_REQUIRED
>>>> + libtizplatform >= $LIBOMXIL_TIZONIA_REQUIRED])
>>>> + gallium_st="$gallium_st omx_tizonia"
>>>> +fi
>>>> +AM_CONDITIONAL(HAVE_ST_OMX_TIZONIA, test "x$enable_omx_tizonia" =
>>>> xyes)
>>>> +
>>>> if test "x$enable_va" = xyes; then
>>>> if test "x$have_va_platform" != xyes; then
>>>> AC_MSG_ERROR([VA requires at least one of the x11 drm or
>>>> wayland platforms])
>>>> @@ -2342,6 +2367,15 @@ AC_ARG_WITH([omx-bellagio-libdir],
>>>> $PKG_CONFIG
>>>> --define-variable=libdir=\$libdir --variable=pluginsdir
>>>> libomxil-bellagio`])
>>>> AC_SUBST([OMX_BELLAGIO_LIB_INSTALL_DIR])
>>>> +dnl Directory for OMX_TIZONIA libs
>>>> +
>>>> +AC_ARG_WITH([omx-tizonia-libdir],
>>>> + [AS_HELP_STRING([--with-omx-tizonia-libdir=DIR],
>>>> + [directory for the OMX_TIZONIA libraries])],
>>>> + [OMX_TIZONIA_LIB_INSTALL_DIR="$withval"],
>>>> + [OMX_TIZONIA_LIB_INSTALL_DIR=`$PKG_CONFIG
>>>> --define-variable=libdir=\$libdir --variable=pluginsdir libtizcore`])
>>>> +AC_SUBST([OMX_TIZONIA_LIB_INSTALL_DIR])
>>>> +
>>>> dnl Directory for VA libs
>>>> AC_ARG_WITH([va-libdir],
>>>> @@ -2840,6 +2874,7 @@ AC_CONFIG_FILES([Makefile
>>>> src/gallium/state_trackers/glx/xlib/Makefile
>>>> src/gallium/state_trackers/nine/Makefile
>>>> src/gallium/state_trackers/omx_bellagio/Makefile
>>>> + src/gallium/state_trackers/omx_tizonia/Makefile
>>>> src/gallium/state_trackers/osmesa/Makefile
>>>> src/gallium/state_trackers/va/Makefile
>>>> src/gallium/state_trackers/vdpau/Makefile
>>>> @@ -2850,6 +2885,7 @@ AC_CONFIG_FILES([Makefile
>>>> src/gallium/targets/dri/Makefile
>>>> src/gallium/targets/libgl-xlib/Makefile
>>>> src/gallium/targets/omx-bellagio/Makefile
>>>> + src/gallium/targets/omx-tizonia/Makefile
>>>> src/gallium/targets/opencl/Makefile
>>>> src/gallium/targets/opencl/mesa.icd
>>>> src/gallium/targets/osmesa/Makefile
>>>> diff --git a/src/gallium/Makefile.am b/src/gallium/Makefile.am
>>>> index 2b930ac..e17c679 100644
>>>> --- a/src/gallium/Makefile.am
>>>> +++ b/src/gallium/Makefile.am
>>>> @@ -153,6 +153,10 @@ if HAVE_ST_OMX_BELLAGIO
>>>> SUBDIRS += state_trackers/omx_bellagio targets/omx-bellagio
>>>> endif
>>>> +if HAVE_ST_OMX_TIZONIA
>>>> +SUBDIRS += state_trackers/omx_tizonia targets/omx-tizonia
>>>> +endif
>>>> +
>>>>
>>>
>>> So now we have 2 OMX ST under gallium state trackers
>>>
>>> Is possible that we could have both tizonia and bellagio as
>>> sub-directory under st/omx?, so that they could share the same Makefile.am,
>>> Makefile.source, and entrypoint.[ch] etc.
>>> even maybe same idea for gallium/target?
>>>
>>> Regards,
>>> Leo
>>>
>>> The original plan was to completely replace the existing OMX IL ST
>> however porting wasn't that straightforward since the new OMX IL version
>> breaks backward compatibility.
>> I think Julien will be able to expain better here.
>>
>> So the plan is still to replace the ST but for now we've postponed that
>> task until all the components have been ported. As it is now we'll later
>> just remove omx_bellagio and rename omx_tizonia to omx.
>> I think that merging them both will make the transition a little bit
>> harder.
>>
>> So maybe if you still insist can we mark it as good-to-have for later?
>> Actually Julien gave the same idea earlier when we started on the project
>> and decided to postpone it for later due to project time restraints.
>>
>> Cheers
>>
>>
>>>
>>> if HAVE_GALLIUM_OSMESA
>>>> SUBDIRS += state_trackers/osmesa targets/osmesa
>>>> endif
>>>> diff --git a/src/gallium/targets/omx-tizonia/Makefile.am
>>>> b/src/gallium/targets/omx-tizonia/Makefile.am
>>>> new file mode 100644
>>>> index 0000000..6baacaa
>>>> --- /dev/null
>>>> +++ b/src/gallium/targets/omx-tizonia/Makefile.am
>>>> @@ -0,0 +1,77 @@
>>>> +include $(top_srcdir)/src/gallium/Automake.inc
>>>> +
>>>> +AM_CFLAGS = \
>>>> + $(GALLIUM_TARGET_CFLAGS)
>>>> +
>>>> +omxdir = $(OMX_TIZONIA_LIB_INSTALL_DIR)
>>>> +omx_LTLIBRARIES = libomxtiz_mesa.la
>>>> +
>>>> +nodist_EXTRA_libomxtiz_mesa_la_SOURCES = dummy.cpp
>>>> +libomxtiz_mesa_la_SOURCES =
>>>> +
>>>> +libomxtiz_mesa_la_LDFLAGS = \
>>>> + -shared \
>>>> + -module \
>>>> + -no-undefined \
>>>> + -avoid-version \
>>>> + $(GC_SECTIONS) \
>>>> + $(LD_NO_UNDEFINED)
>>>> +
>>>> +if HAVE_LD_VERSION_SCRIPT
>>>> +libomxtiz_mesa_la_LDFLAGS += \
>>>> + -Wl,--version-script=$(top_srcdir)/src/gallium/targets/omx-
>>>> tizonia/omx.sym
>>>> +endif # HAVE_LD_VERSION_SCRIPT
>>>> +
>>>> +libomxtiz_mesa_la_LIBADD = \
>>>> + $(top_builddir)/src/gallium/state_trackers/omx_tizonia/libo
>>>> mxtiztracker.la \
>>>> + $(top_builddir)/src/gallium/auxiliary/libgalliumvlwinsys.la \
>>>> + $(top_builddir)/src/gallium/auxiliary/libgalliumvl.la \
>>>> + $(top_builddir)/src/gallium/auxiliary/libgallium.la \
>>>> + $(top_builddir)/src/util/libmesautil.la \
>>>> + $(OMX_TIZONIA_LIBS) \
>>>> + $(OMX_TIZILHEADERS_LIBS) \
>>>> + $(OMX_TIZPLATFORM_LIBS) \
>>>> + $(LIBDRM_LIBS) \
>>>> + $(GALLIUM_COMMON_LIB_DEPS)
>>>> +
>>>> +if HAVE_PLATFORM_X11
>>>> +libomxtiz_mesa_la_LIBADD += \
>>>> + $(VL_LIBS) \
>>>> + $(XCB_DRI3_LIBS)
>>>> +endif
>>>> +
>>>> +EXTRA_libomxtiz_mesa_la_DEPENDENCIES = omx.sym
>>>> +EXTRA_DIST = omx.sym
>>>> +
>>>> +if HAVE_GALLIUM_STATIC_TARGETS
>>>> +
>>>> +TARGET_DRIVERS =
>>>> +TARGET_CPPFLAGS =
>>>> +TARGET_LIB_DEPS =
>>>> +
>>>> +
>>>> +include $(top_srcdir)/src/gallium/drivers/nouveau/Automake.inc
>>>> +
>>>> +include $(top_srcdir)/src/gallium/drivers/r600/Automake.inc
>>>> +include $(top_srcdir)/src/gallium/drivers/radeonsi/Automake.inc
>>>> +
>>>> +libomxtiz_mesa_la_SOURCES += target.c
>>>> +libomxtiz_mesa_la_CPPFLAGS = $(TARGET_CPPFLAGS)
>>>> +libomxtiz_mesa_la_LIBADD += \
>>>> + $(top_builddir)/src/gallium/auxiliary/pipe-loader/libpipe_l
>>>> oader_static.la \
>>>> + $(GALLIUM_PIPE_LOADER_WINSYS_LIBS) \
>>>> + $(TARGET_LIB_DEPS) \
>>>> + $(TARGET_COMPILER_LIB_DEPS) \
>>>> + $(TARGET_RADEON_WINSYS) $(TARGET_RADEON_COMMON)
>>>> +
>>>> +else # HAVE_GALLIUM_STATIC_TARGETS
>>>> +
>>>> +libomxtiz_mesa_la_LIBADD += \
>>>> + $(top_builddir)/src/gallium/auxiliary/pipe-loader/libpipe_l
>>>> oader_dynamic.la
>>>> +
>>>> +endif # HAVE_GALLIUM_STATIC_TARGETS
>>>> +
>>>> +if HAVE_GALLIUM_LLVM
>>>> +libomxtiz_mesa_la_LIBADD += $(LLVM_LIBS)
>>>> +libomxtiz_mesa_la_LDFLAGS += $(LLVM_LDFLAGS)
>>>> +endif
>>>> diff --git a/src/gallium/targets/omx-tizonia/omx.sym
>>>> b/src/gallium/targets/omx-tizonia/omx.sym
>>>> new file mode 100644
>>>> index 0000000..2aafb29
>>>> --- /dev/null
>>>> +++ b/src/gallium/targets/omx-tizonia/omx.sym
>>>> @@ -0,0 +1,11 @@
>>>> +{
>>>> + global:
>>>> + OMX_ComponentInit;
>>>> +
>>>> + # Workaround for an LLVM warning with
>>>> -simplifycfg-sink-common
>>>> + # due to LLVM being initialized multiple times.
>>>> + radeon_drm_winsys_create;
>>>> + amdgpu_winsys_create;
>>>> + local:
>>>> + *;
>>>> +};
>>>> diff --git a/src/gallium/targets/omx-tizonia/target.c
>>>> b/src/gallium/targets/omx-tizonia/target.c
>>>> new file mode 100644
>>>> index 0000000..308e23b
>>>> --- /dev/null
>>>> +++ b/src/gallium/targets/omx-tizonia/target.c
>>>> @@ -0,0 +1,2 @@
>>>> +#include "target-helpers/drm_helper.h"
>>>> +#include "target-helpers/sw_helper.h"
>>>>
>>>
>>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170814/070130b0/attachment-0001.html>
More information about the mesa-dev
mailing list