[Mesa-dev] [PATCH 1/7] gallium: Refactor out vl_put_screen and vl_get_screen
Leo Liu
leo.liu at amd.com
Mon Dec 4 13:58:01 UTC 2017
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>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171204/41518944/attachment-0001.html>
More information about the mesa-dev
mailing list