[Mesa-dev] [PATCH v2 2/4] st/omx_tizonia: Add --enable-omx-tizonia flag and build files
Leo Liu
leo.liu at amd.com
Mon Aug 14 15:25:14 UTC 2017
On 08/14/2017 11:19 AM, Gurkirpal Singh wrote:
>
>
> On Mon, Aug 14, 2017 at 8:05 PM, Leo Liu <leo.liu at amd.com
> <mailto: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
>> <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.
Is the GSoC project done with your current patches?
Regards,
Leo
>
> 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 <mailto:gurkirpal204 at gmail.com>> wrote:
>>
>>
>>
>> On Sun, Aug 13, 2017 at 8:47 AM, Leo Liu <leo.liu at amd.com
>> <mailto: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 <http://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.withgoogle.com/projects/#4737166321123328
>> <https://summerofcode.withgoogle.com/projects/#4737166321123328>
>>
>> Signed-off-by: Gurkirpal Singh
>> <gurkirpal204 at gmail.com <mailto:gurkirpal204 at gmail.com>>
>> Reviewed-and-Tested-by: Julien Isorce
>> <julien.iso... at gmail.com
>> <mailto:julien.iso... at gmail.com>>
>> ---
>> configure.ac <http://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 <http://configure.ac>
>> b/configure.ac <http://configure.ac>
>> index 38af96a..5669695 100644
>> --- a/configure.ac <http://configure.ac>
>> +++ b/configure.ac <http://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
>> <http://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/libomxtiztracker.la
>> <http://libomxtiztracker.la> \
>> +
>> $(top_builddir)/src/gallium/auxiliary/libgalliumvlwinsys.la
>> <http://libgalliumvlwinsys.la> \
>> +
>> $(top_builddir)/src/gallium/auxiliary/libgalliumvl.la
>> <http://libgalliumvl.la> \
>> +
>> $(top_builddir)/src/gallium/auxiliary/libgallium.la
>> <http://libgallium.la> \
>> + $(top_builddir)/src/util/libmesautil.la
>> <http://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_loader_static.la
>> <http://libpipe_loader_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_loader_dynamic.la
>> <http://libpipe_loader_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
>> <mailto:mesa-dev at lists.freedesktop.org>
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> <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/1a010b38/attachment-0001.html>
More information about the mesa-dev
mailing list