Re: [Mesa-dev] [PATCH 3/5] meson: Re-add auto option for omx

2018-03-07 Thread Dylan Baker 
Quoting Eric Engestrom (2018-03-07 10:08:33)
> On Tuesday, 2018-03-06 11:57:14 -0800, Dylan Baker wrote:
> > This re-adds the auto option for omx, without it we default to tizonia
> > and the build fails almost immediately, this is especially obnoxious
> > those building a driver that doesn't support the OMX state tracker to
> > begin with.
> > 
> > CC: Gurkirpal Singh 
> > Fixes: bb5e27fab6087a5c1528a5faf507acce700e883c
> >("st/omx/bellagio: Rename st and target directories")
> > Signed-off-by: Dylan Baker 
> > ---
> >  meson.build| 67 
> > --
> >  meson_options.txt  |  4 +-
> >  src/gallium/meson.build|  2 +-
> >  src/gallium/state_trackers/omx/meson.build | 18 
> >  4 files changed, 56 insertions(+), 35 deletions(-)
> > 
> > diff --git a/meson.build b/meson.build
> > index dd2fa603829..c85afdecee9 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -464,42 +464,63 @@ endif
> >  
> >  _omx = get_option('gallium-omx')
> >  if not system_has_kms_drm
> > -  if _omx != 'disabled'
> > -error('OMX state tracker can only be built on unix-like OSes.')
> > -  else
> > +  if ['auto', 'disabled'].contains(_omx)
> >  _omx = 'disabled'
> > +  else
> > +error('OMX state tracker can only be built on unix-like OSes.')
> >endif
> >  elif not (with_platform_x11 or with_platform_drm)
> > -  if _omx != 'disabled'
> > -error('OMX state tracker requires X11 or drm platform support.')
> > -  else
> > +  if ['auto', 'disabled'].contains(_omx)
> >  _omx = 'disabled'
> > +  else
> > +error('OMX state tracker requires X11 or drm platform support.')
> >endif
> >  elif not (with_gallium_r600 or with_gallium_radeonsi or 
> > with_gallium_nouveau)
> > -  if _omx != 'disabled'
> > -error('OMX state tracker requires at least one of the following 
> > gallium drivers: r600, radeonsi, nouveau.')
> > -  else
> > +  if ['auto', 'disabled'].contains(_omx)
> >  _omx = 'disabled'
> > +  else
> > +error('OMX state tracker requires at least one of the following 
> > gallium drivers: r600, radeonsi, nouveau.')
> >endif
> >  endif
> > -with_gallium_omx = _omx != 'disabled'
> > -gallium_omx = _omx
> > +with_gallium_omx = _omx
> >  dep_omx = []
> >  dep_omx_other = []
> > -if gallium_omx == 'bellagio'
> > +if with_gallium_omx == 'bellagio' or with_gallium_omx == 'auto'
> >pre_args += '-DENABLE_ST_OMX_BELLAGIO'
> 
> I didn't quite get what Julien was saying at first, but he's right: the
> above line should be moved ...
> 
> > -  dep_omx = dependency('libomxil-bellagio')
> > -elif gallium_omx == 'tizonia'
> > -  pre_args += '-DENABLE_ST_OMX_TIZONIA'
> > -  dep_omx = dependency('libtizonia', version : '>= 0.10.0')
> > -  dep_omx_other = [
> > -dependency('libtizplatform'),
> > -dependency('tizilheaders')
> > -  ]
> > +  dep_omx = dependency(
> > +'libomxil-bellagio', required : with_gallium_omx == 'bellagio'
> > +  )
> > +  if dep_omx.found()
> > +with_gallium_omx = 'bellagio'
> 
> ... here
> 
> > +  endif
> > +endif
> > +if with_gallium_omx == 'tizonia' or with_gallium_omx == 'auto'
> > +  if not (with_dri and with_egl)
> > +if with_gallium_omx == 'tizonia'
> > +  error('OMX-Tizonia state tracker requires dri and egl')
> > +else
> > +  with_gallium_omx == 'disabled'
> > +endif
> > +  else
> > +pre_args += '-DENABLE_ST_OMX_TIZONIA'
> 
> same with this one, it should be moved ...
> 
> > +dep_omx = dependency(
> > +  'libtizonia', version : '>= 0.10.0',
> > +  required : with_gallium_omx == 'tizonia',
> > +)
> > +dep_omx_other = [
> > +  dependency('libtizplatform', required : with_gallium_omx == 
> > 'tizonia'),
> > +  dependency('tizilheaders', required : with_gallium_omx == 'tizonia'),
> > +]
> > +if dep_omx.found() and dep_omx_other[0].found() and 
> > dep_omx_other[1].found()
> > +  with_gallium_omx = 'tizonia'
> 
> ... here

I realized that between the time you sent your mail and my client noticed it.
I've fixed that and I'll push with the pre_args in the right place.

> 
> With these two fixes, r-b stands :)
> 
> (Note, kind of a nit, but I'd prefer for _omx to be used until it's
> fully defined, ie. `with_gallium_omx` should never be 'auto' imho)
> 

Okay, lets fix that in a follow up, since this fixes the build.

Dylan


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/5] meson: Re-add auto option for omx

2018-03-07 Thread Eric Engestrom 
On Tuesday, 2018-03-06 11:57:14 -0800, Dylan Baker wrote:
> This re-adds the auto option for omx, without it we default to tizonia
> and the build fails almost immediately, this is especially obnoxious
> those building a driver that doesn't support the OMX state tracker to
> begin with.
> 
> CC: Gurkirpal Singh 
> Fixes: bb5e27fab6087a5c1528a5faf507acce700e883c
>("st/omx/bellagio: Rename st and target directories")
> Signed-off-by: Dylan Baker 
> ---
>  meson.build| 67 
> --
>  meson_options.txt  |  4 +-
>  src/gallium/meson.build|  2 +-
>  src/gallium/state_trackers/omx/meson.build | 18 
>  4 files changed, 56 insertions(+), 35 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index dd2fa603829..c85afdecee9 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -464,42 +464,63 @@ endif
>  
>  _omx = get_option('gallium-omx')
>  if not system_has_kms_drm
> -  if _omx != 'disabled'
> -error('OMX state tracker can only be built on unix-like OSes.')
> -  else
> +  if ['auto', 'disabled'].contains(_omx)
>  _omx = 'disabled'
> +  else
> +error('OMX state tracker can only be built on unix-like OSes.')
>endif
>  elif not (with_platform_x11 or with_platform_drm)
> -  if _omx != 'disabled'
> -error('OMX state tracker requires X11 or drm platform support.')
> -  else
> +  if ['auto', 'disabled'].contains(_omx)
>  _omx = 'disabled'
> +  else
> +error('OMX state tracker requires X11 or drm platform support.')
>endif
>  elif not (with_gallium_r600 or with_gallium_radeonsi or with_gallium_nouveau)
> -  if _omx != 'disabled'
> -error('OMX state tracker requires at least one of the following gallium 
> drivers: r600, radeonsi, nouveau.')
> -  else
> +  if ['auto', 'disabled'].contains(_omx)
>  _omx = 'disabled'
> +  else
> +error('OMX state tracker requires at least one of the following gallium 
> drivers: r600, radeonsi, nouveau.')
>endif
>  endif
> -with_gallium_omx = _omx != 'disabled'
> -gallium_omx = _omx
> +with_gallium_omx = _omx
>  dep_omx = []
>  dep_omx_other = []
> -if gallium_omx == 'bellagio'
> +if with_gallium_omx == 'bellagio' or with_gallium_omx == 'auto'
>pre_args += '-DENABLE_ST_OMX_BELLAGIO'

I didn't quite get what Julien was saying at first, but he's right: the
above line should be moved ...

> -  dep_omx = dependency('libomxil-bellagio')
> -elif gallium_omx == 'tizonia'
> -  pre_args += '-DENABLE_ST_OMX_TIZONIA'
> -  dep_omx = dependency('libtizonia', version : '>= 0.10.0')
> -  dep_omx_other = [
> -dependency('libtizplatform'),
> -dependency('tizilheaders')
> -  ]
> +  dep_omx = dependency(
> +'libomxil-bellagio', required : with_gallium_omx == 'bellagio'
> +  )
> +  if dep_omx.found()
> +with_gallium_omx = 'bellagio'

... here

> +  endif
> +endif
> +if with_gallium_omx == 'tizonia' or with_gallium_omx == 'auto'
> +  if not (with_dri and with_egl)
> +if with_gallium_omx == 'tizonia'
> +  error('OMX-Tizonia state tracker requires dri and egl')
> +else
> +  with_gallium_omx == 'disabled'
> +endif
> +  else
> +pre_args += '-DENABLE_ST_OMX_TIZONIA'

same with this one, it should be moved ...

> +dep_omx = dependency(
> +  'libtizonia', version : '>= 0.10.0',
> +  required : with_gallium_omx == 'tizonia',
> +)
> +dep_omx_other = [
> +  dependency('libtizplatform', required : with_gallium_omx == 'tizonia'),
> +  dependency('tizilheaders', required : with_gallium_omx == 'tizonia'),
> +]
> +if dep_omx.found() and dep_omx_other[0].found() and 
> dep_omx_other[1].found()
> +  with_gallium_omx = 'tizonia'

... here

With these two fixes, r-b stands :)

(Note, kind of a nit, but I'd prefer for _omx to be used until it's
fully defined, ie. `with_gallium_omx` should never be 'auto' imho)

> +else
> +  with_gallium_omx = 'disabled'
> +endif
> +  endif
>  endif
>  
>  omx_drivers_path = get_option('omx-libs-path')
> -if with_gallium_omx
> +if with_gallium_omx != 'disabled'
># Figure out where to put the omx driver.
># FIXME: this could all be vastly simplified by adding a 'defined_variable'
># argument to meson's get_pkgconfig_variable method.
> @@ -1193,8 +1214,8 @@ if with_platform_x11
>  dep_xxf86vm = dependency('xxf86vm', required : false)
>endif
>if (with_any_vk or with_glx == 'dri' or
> -   (with_gallium_vdpau or with_gallium_xvmc or with_gallium_omx or
> -with_gallium_xa))
> +   (with_gallium_vdpau or with_gallium_xvmc or with_gallium_va or
> +with_gallium_omx != 'disabled'))
>  dep_xcb = dependency('xcb')
>  dep_x11_xcb = dependency('x11-xcb')
>endif
> diff --git a/meson_options.txt b/meson_options.txt
> index 50ae19685e7..a573290b774 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -90,8 +90,8 @@ option(
>  option(
>

[Mesa-dev] [PATCH 3/5] meson: Re-add auto option for omx

2018-03-06 Thread Dylan Baker 
This re-adds the auto option for omx, without it we default to tizonia
and the build fails almost immediately, this is especially obnoxious
those building a driver that doesn't support the OMX state tracker to
begin with.

CC: Gurkirpal Singh 
Fixes: bb5e27fab6087a5c1528a5faf507acce700e883c
   ("st/omx/bellagio: Rename st and target directories")
Signed-off-by: Dylan Baker 
---
 meson.build| 67 --
 meson_options.txt  |  4 +-
 src/gallium/meson.build|  2 +-
 src/gallium/state_trackers/omx/meson.build | 18 
 4 files changed, 56 insertions(+), 35 deletions(-)

diff --git a/meson.build b/meson.build
index dd2fa603829..c85afdecee9 100644
--- a/meson.build
+++ b/meson.build
@@ -464,42 +464,63 @@ endif
 
 _omx = get_option('gallium-omx')
 if not system_has_kms_drm
-  if _omx != 'disabled'
-error('OMX state tracker can only be built on unix-like OSes.')
-  else
+  if ['auto', 'disabled'].contains(_omx)
 _omx = 'disabled'
+  else
+error('OMX state tracker can only be built on unix-like OSes.')
   endif
 elif not (with_platform_x11 or with_platform_drm)
-  if _omx != 'disabled'
-error('OMX state tracker requires X11 or drm platform support.')
-  else
+  if ['auto', 'disabled'].contains(_omx)
 _omx = 'disabled'
+  else
+error('OMX state tracker requires X11 or drm platform support.')
   endif
 elif not (with_gallium_r600 or with_gallium_radeonsi or with_gallium_nouveau)
-  if _omx != 'disabled'
-error('OMX state tracker requires at least one of the following gallium 
drivers: r600, radeonsi, nouveau.')
-  else
+  if ['auto', 'disabled'].contains(_omx)
 _omx = 'disabled'
+  else
+error('OMX state tracker requires at least one of the following gallium 
drivers: r600, radeonsi, nouveau.')
   endif
 endif
-with_gallium_omx = _omx != 'disabled'
-gallium_omx = _omx
+with_gallium_omx = _omx
 dep_omx = []
 dep_omx_other = []
-if gallium_omx == 'bellagio'
+if with_gallium_omx == 'bellagio' or with_gallium_omx == 'auto'
   pre_args += '-DENABLE_ST_OMX_BELLAGIO'
-  dep_omx = dependency('libomxil-bellagio')
-elif gallium_omx == 'tizonia'
-  pre_args += '-DENABLE_ST_OMX_TIZONIA'
-  dep_omx = dependency('libtizonia', version : '>= 0.10.0')
-  dep_omx_other = [
-dependency('libtizplatform'),
-dependency('tizilheaders')
-  ]
+  dep_omx = dependency(
+'libomxil-bellagio', required : with_gallium_omx == 'bellagio'
+  )
+  if dep_omx.found()
+with_gallium_omx = 'bellagio'
+  endif
+endif
+if with_gallium_omx == 'tizonia' or with_gallium_omx == 'auto'
+  if not (with_dri and with_egl)
+if with_gallium_omx == 'tizonia'
+  error('OMX-Tizonia state tracker requires dri and egl')
+else
+  with_gallium_omx == 'disabled'
+endif
+  else
+pre_args += '-DENABLE_ST_OMX_TIZONIA'
+dep_omx = dependency(
+  'libtizonia', version : '>= 0.10.0',
+  required : with_gallium_omx == 'tizonia',
+)
+dep_omx_other = [
+  dependency('libtizplatform', required : with_gallium_omx == 'tizonia'),
+  dependency('tizilheaders', required : with_gallium_omx == 'tizonia'),
+]
+if dep_omx.found() and dep_omx_other[0].found() and 
dep_omx_other[1].found()
+  with_gallium_omx = 'tizonia'
+else
+  with_gallium_omx = 'disabled'
+endif
+  endif
 endif
 
 omx_drivers_path = get_option('omx-libs-path')
-if with_gallium_omx
+if with_gallium_omx != 'disabled'
   # Figure out where to put the omx driver.
   # FIXME: this could all be vastly simplified by adding a 'defined_variable'
   # argument to meson's get_pkgconfig_variable method.
@@ -1193,8 +1214,8 @@ if with_platform_x11
 dep_xxf86vm = dependency('xxf86vm', required : false)
   endif
   if (with_any_vk or with_glx == 'dri' or
-   (with_gallium_vdpau or with_gallium_xvmc or with_gallium_omx or
-with_gallium_xa))
+   (with_gallium_vdpau or with_gallium_xvmc or with_gallium_va or
+with_gallium_omx != 'disabled'))
 dep_xcb = dependency('xcb')
 dep_x11_xcb = dependency('x11-xcb')
   endif
diff --git a/meson_options.txt b/meson_options.txt
index 50ae19685e7..a573290b774 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -90,8 +90,8 @@ option(
 option(
   'gallium-omx',
   type : 'combo',
-  value : 'tizonia',
-  choices : ['disabled', 'bellagio', 'tizonia'],
+  value : 'auto',
+  choices : ['auto', 'disabled', 'bellagio', 'tizonia'],
   description : 'enable gallium omx state tracker.',
 )
 option(
diff --git a/src/gallium/meson.build b/src/gallium/meson.build
index d3cbb76d94d..108234ad322 100644
--- a/src/gallium/meson.build
+++ b/src/gallium/meson.build
@@ -170,7 +170,7 @@ if with_gallium_xvmc
   subdir('state_trackers/xvmc')
   subdir('targets/xvmc')
 endif
-if with_gallium_omx
+if with_gallium_omx != 'disabled'
   subdir('state_trackers/omx')
   subdir('targets/omx')
 endif
diff --git