[Mesa-dev] [PATCH 1/7] gallium: Refactor out vl_put_screen and vl_get_screen

Christian König ckoenig.leichtzumerken at gmail.com
Tue Jan 16 09:48:23 UTC 2018


Am 16.01.2018 um 10:40 schrieb Gurkirpal Singh:
>
>
> On Mon, Jan 15, 2018 at 9:24 PM, Leo Liu <leo.liu at amd.com 
> <mailto:leo.liu at amd.com>> wrote:
>
>     Hi lists,
>
>
>     If there is no more questions, and no objection, I would like to
>     commit this new OMX st. to upstream.
>
>     @Gurkipal, If you can send me or lists your rebased patches set
>     for committing, that would be appreciated.
>
> I wanted to know if it's required to test the patches before sending them.
> I've since then switched to a laptop with NVIDIA card and have limited 
> access to the desktop with the AMD card (that i used to work on during 
> the project).
> So I'm unable to test the encoder on this machine right now. It could 
> take a few  weeks until I could do so.

Leo is just asking for the final cleanup before pushing the code, no 
need for an extra test here.

Christian.

>
>     Leo
>
>
>     On 12/04/2017 08:58 AM, Leo Liu wrote:
>>
>>
>>
>>     On 12/03/2017 10:04 AM, Gurkirpal Singh wrote:
>>>     I sent the modified patches in another thread a while ago.
>>>     Please review in case got missed.
>>     Please be patient for a few days to see if any other comments.
>>     After then please rebase, add rb/ab to your patches, and send
>>     them to me,
>>     I will commit them for you.
>>
>>     Leo
>>
>>
>>>
>>>     On Thu, Nov 30, 2017 at 7:35 PM, Leo Liu <leo.liu at amd.com
>>>     <mailto:leo.liu at amd.com>> wrote:
>>>
>>>
>>>
>>>         On 11/30/2017 06:22 AM, Julien Isorce wrote:
>>>>         Hi Gurkirpal,
>>>>
>>>>         > Before refactoring process both the state trackers were in independent
>>>>         directories.
>>>>         > During earlier refactoring effort we decided to keep that directory
>>>>         structure so it made
>>>>         > sense to move them to auxiliary code. After that I moved them both under st/omx.
>>>>         > Since there could be a chance of it being useful out of st/omx, I left the
>>>>         decision to
>>>>         > keep it or move it back to st/omx to the mailing list.
>>>>
>>>>         Yes please move it back to st/omx for the reasons you said,
>>>>         i.e. there will now 2
>>>>         sub directories, st/omx/bellagio and st/omx/tizonia and
>>>>         common code in st/omx.
>>>         Yes. Please move them back to st/omx.
>>>
>>>         With that fixed, the series are:
>>>
>>>         Acked-by: Leo Liu <leo.liu at amd.com> <mailto:leo.liu at amd.com>
>>>
>>>         Thanks for the work!
>>>
>>>         Leo
>>>
>>>
>>>
>>>>
>>>>         Another reason is that the env var "OMX_RENDER_NODE"
>>>>         mentions OMX.
>>>>
>>>>         Thx!
>>>>         Julien
>>>>
>>>>
>>>>         On 29 November 2017 at 04:02, Gurkirpal Singh
>>>>         <gurkirpal204 at gmail.com <mailto:gurkirpal204 at gmail.com>> wrote:
>>>>
>>>>             ---
>>>>              src/gallium/auxiliary/Makefile.sources             |   2 +
>>>>              src/gallium/auxiliary/vl/vl_screen.c               |
>>>>             107 +++++++++++++++++++++
>>>>              src/gallium/auxiliary/vl/vl_screen.h               | 
>>>>             33 +++++++
>>>>              .../state_trackers/omx_bellagio/entrypoint.c       | 
>>>>             83 ----------------
>>>>              .../state_trackers/omx_bellagio/entrypoint.h       |   3 -
>>>>              src/gallium/state_trackers/omx_bellagio/vid_dec.c  | 
>>>>              5 +-
>>>>              src/gallium/state_trackers/omx_bellagio/vid_enc.c  | 
>>>>              5 +-
>>>>              7 files changed, 148 insertions(+), 90 deletions(-)
>>>>              create mode 100644 src/gallium/auxiliary/vl/vl_screen.c
>>>>              create mode 100644 src/gallium/auxiliary/vl/vl_screen.h
>>>>
>>>>             diff --git a/src/gallium/auxiliary/Makefile.sources
>>>>             b/src/gallium/auxiliary/Makefile.sources
>>>>             index f40c472..35e89f9 100644
>>>>             --- a/src/gallium/auxiliary/Makefile.sources
>>>>             +++ b/src/gallium/auxiliary/Makefile.sources
>>>>             @@ -343,6 +343,8 @@ VL_SOURCES := \
>>>>             vl/vl_mpeg12_decoder.c \
>>>>             vl/vl_mpeg12_decoder.h \
>>>>                     vl/vl_rbsp.h \
>>>>             +       vl/vl_screen.c \
>>>>             +       vl/vl_screen.h \
>>>>                     vl/vl_types.h \
>>>>             vl/vl_vertex_buffers.c \
>>>>             vl/vl_vertex_buffers.h \
>>>>             diff --git a/src/gallium/auxiliary/vl/vl_screen.c
>>>>             b/src/gallium/auxiliary/vl/vl_screen.c
>>>>             new file mode 100644
>>>>             index 0000000..7192802
>>>>             --- /dev/null
>>>>             +++ b/src/gallium/auxiliary/vl/vl_screen.c
>>>>             @@ -0,0 +1,107 @@
>>>>             +/**************************************************************************
>>>>             + *
>>>>             + * Permission is hereby granted, free of charge, to
>>>>             any person obtaining a
>>>>             + * copy of this software and associated documentation
>>>>             files (the
>>>>             + * "Software"), to deal in the Software without
>>>>             restriction, including
>>>>             + * without limitation the rights to use, copy, modify,
>>>>             merge, publish,
>>>>             + * distribute, sub license, and/or sell copies of the
>>>>             Software, and to
>>>>             + * permit persons to whom the Software is furnished to
>>>>             do so, subject to
>>>>             + * the following conditions:
>>>>             + *
>>>>             + * The above copyright notice and this permission
>>>>             notice (including the
>>>>             + * next paragraph) shall be included in all copies or
>>>>             substantial portions
>>>>             + * of the Software.
>>>>             + *
>>>>             + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY
>>>>             OF ANY KIND, EXPRESS
>>>>             + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
>>>>             WARRANTIES OF
>>>>             + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE
>>>>             AND NON-INFRINGEMENT.
>>>>             + * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE
>>>>             LIABLE FOR
>>>>             + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
>>>>             AN ACTION OF CONTRACT,
>>>>             + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>>>>             CONNECTION WITH THE
>>>>             + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>>>>             + *
>>>>             +
>>>>             **************************************************************************/
>>>>             +
>>>>             +#include <assert.h>
>>>>             +#include <string.h>
>>>>             +#include <stdbool.h>
>>>>             +
>>>>             +#include "os/os_thread.h"
>>>>             +#include "util/u_memory.h"
>>>>             +#include "loader/loader.h"
>>>>             +#include "vl_screen.h"
>>>>             +
>>>>             +#if defined(HAVE_X11_PLATFORM)
>>>>             +#include <X11/Xlib.h>
>>>>             +#else
>>>>             +#define XOpenDisplay(x) NULL
>>>>             +#define XCloseDisplay(x)
>>>>             +#define Display void
>>>>             +#endif
>>>>             +
>>>>             +static mtx_t st_lock = _MTX_INITIALIZER_NP;
>>>>             +static Display *st_display = NULL;
>>>>             +static struct vl_screen *st_screen = NULL;
>>>>             +static unsigned st_usecount = 0;
>>>>             +static const char *st_render_node = NULL;
>>>>             +static int drm_fd;
>>>>             +
>>>>             +struct vl_screen *vl_get_screen(const char* render_node)
>>>>             +{
>>>>             +   static bool first_time = true;
>>>>             +   mtx_lock(&st_lock);
>>>>             +
>>>>             +   if (!st_screen) {
>>>>             +      if (first_time) {
>>>>             +         st_render_node =
>>>>             debug_get_option(render_node, NULL);
>>>>             +         first_time = false;
>>>>             +      }
>>>>             +      if (st_render_node) {
>>>>             +         drm_fd = loader_open_device(st_render_node);
>>>>             +         if (drm_fd < 0)
>>>>             +            goto error;
>>>>             +
>>>>             +         st_screen = vl_drm_screen_create(drm_fd);
>>>>             +         if (!st_screen) {
>>>>             +            close(drm_fd);
>>>>             +            goto error;
>>>>             +         }
>>>>             +      } else {
>>>>             +         st_display = XOpenDisplay(NULL);
>>>>             +         if (!st_display)
>>>>             +            goto error;
>>>>             +
>>>>             +         st_screen = vl_dri3_screen_create(st_display, 0);
>>>>             +         if (!st_screen)
>>>>             +            st_screen =
>>>>             vl_dri2_screen_create(st_display, 0);
>>>>             +         if (!st_screen) {
>>>>             + XCloseDisplay(st_display);
>>>>             +            goto error;
>>>>             +         }
>>>>             +      }
>>>>             +   }
>>>>             +
>>>>             +   ++st_usecount;
>>>>             +
>>>>             +  mtx_unlock(&st_lock);
>>>>             +   return st_screen;
>>>>             +
>>>>             +error:
>>>>             +  mtx_unlock(&st_lock);
>>>>             +   return NULL;
>>>>             +}
>>>>             +
>>>>             +void vl_put_screen(void)
>>>>             +{
>>>>             +   mtx_lock(&st_lock);
>>>>             +   if ((--st_usecount) == 0) {
>>>>             + st_screen->destroy(st_screen);
>>>>             +      st_screen = NULL;
>>>>             +
>>>>             +      if (st_render_node)
>>>>             +         close(drm_fd);
>>>>             +      else
>>>>             +  XCloseDisplay(st_display);
>>>>             +   }
>>>>             +  mtx_unlock(&st_lock);
>>>>             +}
>>>>             diff --git a/src/gallium/auxiliary/vl/vl_screen.h
>>>>             b/src/gallium/auxiliary/vl/vl_screen.h
>>>>             new file mode 100644
>>>>             index 0000000..1e14775
>>>>             --- /dev/null
>>>>             +++ b/src/gallium/auxiliary/vl/vl_screen.h
>>>>             @@ -0,0 +1,33 @@
>>>>             +/**************************************************************************
>>>>             + *
>>>>             + * Permission is hereby granted, free of charge, to
>>>>             any person obtaining a
>>>>             + * copy of this software and associated documentation
>>>>             files (the
>>>>             + * "Software"), to deal in the Software without
>>>>             restriction, including
>>>>             + * without limitation the rights to use, copy, modify,
>>>>             merge, publish,
>>>>             + * distribute, sub license, and/or sell copies of the
>>>>             Software, and to
>>>>             + * permit persons to whom the Software is furnished to
>>>>             do so, subject to
>>>>             + * the following conditions:
>>>>             + *
>>>>             + * The above copyright notice and this permission
>>>>             notice (including the
>>>>             + * next paragraph) shall be included in all copies or
>>>>             substantial portions
>>>>             + * of the Software.
>>>>             + *
>>>>             + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY
>>>>             OF ANY KIND, EXPRESS
>>>>             + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
>>>>             WARRANTIES OF
>>>>             + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE
>>>>             AND NON-INFRINGEMENT.
>>>>             + * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE
>>>>             LIABLE FOR
>>>>             + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
>>>>             AN ACTION OF CONTRACT,
>>>>             + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>>>>             CONNECTION WITH THE
>>>>             + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>>>>             + *
>>>>             +
>>>>             **************************************************************************/
>>>>             +
>>>>             +#ifndef VL_VL_SCREEN_H
>>>>             +#define VL_VL_SCREEN_H
>>>>             +
>>>>             +#include "vl_winsys.h"
>>>>             +
>>>>             +struct vl_screen *vl_get_screen(const char* render_node);
>>>>             +void vl_put_screen(void);
>>>>             +
>>>>             +#endif
>>>>             diff --git
>>>>             a/src/gallium/state_trackers/omx_bellagio/entrypoint.c
>>>>             b/src/gallium/state_trackers/omx_bellagio/entrypoint.c
>>>>             index 251cc7d..5c75e8d 100644
>>>>             --- a/src/gallium/state_trackers/omx_bellagio/entrypoint.c
>>>>             +++ b/src/gallium/state_trackers/omx_bellagio/entrypoint.c
>>>>             @@ -31,33 +31,10 @@
>>>>               *
>>>>               */
>>>>
>>>>             -#include <assert.h>
>>>>             -#include <string.h>
>>>>             -#include <stdbool.h>
>>>>             -
>>>>             -#if defined(HAVE_X11_PLATFORM)
>>>>             -#include <X11/Xlib.h>
>>>>             -#else
>>>>             -#define XOpenDisplay(x) NULL
>>>>             -#define XCloseDisplay(x)
>>>>             -#define Display void
>>>>             -#endif
>>>>             -
>>>>             -#include "os/os_thread.h"
>>>>             -#include "util/u_memory.h"
>>>>             -#include "loader/loader.h"
>>>>             -
>>>>              #include "entrypoint.h"
>>>>              #include "vid_dec.h"
>>>>              #include "vid_enc.h"
>>>>
>>>>             -static mtx_t omx_lock = _MTX_INITIALIZER_NP;
>>>>             -static Display *omx_display = NULL;
>>>>             -static struct vl_screen *omx_screen = NULL;
>>>>             -static unsigned omx_usecount = 0;
>>>>             -static const char *omx_render_node = NULL;
>>>>             -static int drm_fd;
>>>>             -
>>>>              int omx_component_library_Setup(stLoaderComponentType
>>>>             **stComponents)
>>>>              {
>>>>                 OMX_ERRORTYPE r;
>>>>             @@ -78,66 +55,6 @@ int
>>>>             omx_component_library_Setup(stLoaderComponentType
>>>>             **stComponents)
>>>>                 return 2;
>>>>              }
>>>>
>>>>             -struct vl_screen *omx_get_screen(void)
>>>>             -{
>>>>             -   static bool first_time = true;
>>>>             -   mtx_lock(&omx_lock);
>>>>             -
>>>>             -   if (!omx_screen) {
>>>>             -      if (first_time) {
>>>>             -         omx_render_node =
>>>>             debug_get_option("OMX_RENDER_NODE", NULL);
>>>>             -         first_time = false;
>>>>             -      }
>>>>             -      if (omx_render_node) {
>>>>             -         drm_fd = loader_open_device(omx_render_node);
>>>>             -         if (drm_fd < 0)
>>>>             -            goto error;
>>>>             -
>>>>             -         omx_screen = vl_drm_screen_create(drm_fd);
>>>>             -         if (!omx_screen) {
>>>>             -            close(drm_fd);
>>>>             -            goto error;
>>>>             -         }
>>>>             -      } else {
>>>>             -         omx_display = XOpenDisplay(NULL);
>>>>             -         if (!omx_display)
>>>>             -            goto error;
>>>>             -
>>>>             -         omx_screen =
>>>>             vl_dri3_screen_create(omx_display, 0);
>>>>             -         if (!omx_screen)
>>>>             -            omx_screen =
>>>>             vl_dri2_screen_create(omx_display, 0);
>>>>             -         if (!omx_screen) {
>>>>             - XCloseDisplay(omx_display);
>>>>             -            goto error;
>>>>             -         }
>>>>             -      }
>>>>             -   }
>>>>             -
>>>>             -   ++omx_usecount;
>>>>             -
>>>>             -  mtx_unlock(&omx_lock);
>>>>             -   return omx_screen;
>>>>             -
>>>>             -error:
>>>>             -  mtx_unlock(&omx_lock);
>>>>             -   return NULL;
>>>>             -}
>>>>             -
>>>>             -void omx_put_screen(void)
>>>>             -{
>>>>             -   mtx_lock(&omx_lock);
>>>>             -   if ((--omx_usecount) == 0) {
>>>>             - omx_screen->destroy(omx_screen);
>>>>             -      omx_screen = NULL;
>>>>             -
>>>>             -      if (omx_render_node)
>>>>             -         close(drm_fd);
>>>>             -      else
>>>>             -  XCloseDisplay(omx_display);
>>>>             -   }
>>>>             -  mtx_unlock(&omx_lock);
>>>>             -}
>>>>             -
>>>>              OMX_ERRORTYPE
>>>>             omx_workaround_Destructor(OMX_COMPONENTTYPE *comp)
>>>>              {
>>>>             omx_base_component_PrivateType* priv =
>>>>             (omx_base_component_PrivateType*)comp->pComponentPrivate;
>>>>             diff --git
>>>>             a/src/gallium/state_trackers/omx_bellagio/entrypoint.h
>>>>             b/src/gallium/state_trackers/omx_bellagio/entrypoint.h
>>>>             index 7625d7a..d566d1e 100644
>>>>             --- a/src/gallium/state_trackers/omx_bellagio/entrypoint.h
>>>>             +++ b/src/gallium/state_trackers/omx_bellagio/entrypoint.h
>>>>             @@ -40,9 +40,6 @@
>>>>
>>>>              PUBLIC extern int
>>>>             omx_component_library_Setup(stLoaderComponentType
>>>>             **stComponents);
>>>>
>>>>             -struct vl_screen *omx_get_screen(void);
>>>>             -void omx_put_screen(void);
>>>>             -
>>>>              OMX_ERRORTYPE
>>>>             omx_workaround_Destructor(OMX_COMPONENTTYPE *comp);
>>>>
>>>>              #endif
>>>>             diff --git
>>>>             a/src/gallium/state_trackers/omx_bellagio/vid_dec.c
>>>>             b/src/gallium/state_trackers/omx_bellagio/vid_dec.c
>>>>             index f9fe19f..b62c705 100644
>>>>             --- a/src/gallium/state_trackers/omx_bellagio/vid_dec.c
>>>>             +++ b/src/gallium/state_trackers/omx_bellagio/vid_dec.c
>>>>             @@ -50,6 +50,7 @@
>>>>              #include "util/u_surface.h"
>>>>              #include "vl/vl_video_buffer.h"
>>>>              #include "vl/vl_vlc.h"
>>>>             +#include "vl/vl_screen.h"
>>>>
>>>>              #include "entrypoint.h"
>>>>              #include "vid_dec.h"
>>>>             @@ -173,7 +174,7 @@ static OMX_ERRORTYPE
>>>>             vid_dec_Constructor(OMX_COMPONENTTYPE *comp, OMX_STRING nam
>>>>                 comp->SetParameter = vid_dec_SetParameter;
>>>>                 comp->GetParameter = vid_dec_GetParameter;
>>>>
>>>>             -   priv->screen = omx_get_screen();
>>>>             +   priv->screen = vl_get_screen("OMX_RENDER_NODE");
>>>>                 if (!priv->screen)
>>>>                    return OMX_ErrorInsufficientResources;
>>>>
>>>>             @@ -253,7 +254,7 @@ static OMX_ERRORTYPE
>>>>             vid_dec_Destructor(OMX_COMPONENTTYPE *comp)
>>>>                 }
>>>>
>>>>                 if (priv->screen)
>>>>             -      omx_put_screen();
>>>>             +      vl_put_screen();
>>>>
>>>>                 return omx_workaround_Destructor(comp);
>>>>              }
>>>>             diff --git
>>>>             a/src/gallium/state_trackers/omx_bellagio/vid_enc.c
>>>>             b/src/gallium/state_trackers/omx_bellagio/vid_enc.c
>>>>             index 1a4fb62..40e7a0f 100644
>>>>             --- a/src/gallium/state_trackers/omx_bellagio/vid_enc.c
>>>>             +++ b/src/gallium/state_trackers/omx_bellagio/vid_enc.c
>>>>             @@ -50,6 +50,7 @@
>>>>              #include "pipe/p_video_codec.h"
>>>>              #include "util/u_memory.h"
>>>>              #include "vl/vl_video_buffer.h"
>>>>             +#include "vl/vl_screen.h"
>>>>
>>>>              #include "entrypoint.h"
>>>>              #include "vid_enc.h"
>>>>             @@ -170,7 +171,7 @@ static OMX_ERRORTYPE
>>>>             vid_enc_Constructor(OMX_COMPONENTTYPE *comp, OMX_STRING nam
>>>>                 comp->GetConfig = vid_enc_GetConfig;
>>>>                 comp->SetConfig = vid_enc_SetConfig;
>>>>
>>>>             -   priv->screen = omx_get_screen();
>>>>             +   priv->screen = vl_get_screen("OMX_RENDER_NODE");
>>>>                 if (!priv->screen)
>>>>                    return OMX_ErrorInsufficientResources;
>>>>
>>>>             @@ -296,7 +297,7 @@ static OMX_ERRORTYPE
>>>>             vid_enc_Destructor(OMX_COMPONENTTYPE *comp)
>>>>              priv->t_pipe->destroy(priv->t_pipe);
>>>>
>>>>                 if (priv->screen)
>>>>             -      omx_put_screen();
>>>>             +      vl_put_screen();
>>>>
>>>>                 return omx_workaround_Destructor(comp);
>>>>              }
>>>>             --
>>>>             2.7.4
>>>>
>>>>             _______________________________________________
>>>>             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>
>>>>
>>>>
>>>>
>>>>
>>>>         _______________________________________________
>>>>         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>
>>>
>>>
>>
>
>
>
>
> _______________________________________________
> 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/20180116/63669d74/attachment-0001.html>


More information about the mesa-dev mailing list