[Mesa-dev] [PATCH 1/7] gallium: Refactor out vl_put_screen and vl_get_screen
Leo Liu
leo.liu at amd.com
Tue Jan 16 13:14:12 UTC 2018
On 01/16/2018 04:48 AM, Christian König wrote:
> 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.
Yeah. rebase your patches and add "RB/AB", and make sure the meson build
is added.
After that send patch set to the lists or to me, I will push them.
Regards,
Leo
>
> 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/a2614cdc/attachment-0001.html>
More information about the mesa-dev
mailing list