From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from fllnx209.ext.ti.com (fllnx209.ext.ti.com [198.47.19.16]) by arago-project.org (Postfix) with ESMTPS id 86069529F3 for ; Tue, 25 Apr 2017 23:28:28 +0000 (UTC) Received: from dflxv15.itg.ti.com ([128.247.5.124]) by fllnx209.ext.ti.com (8.15.1/8.15.1) with ESMTP id v3PNSHO8032132 for ; Tue, 25 Apr 2017 18:28:17 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com; s=ti-com-17Q1; t=1493162897; bh=LvnLiEmw5v8WGksVoUIZrES3UVGgJF4aNxCOk2xA8Ao=; h=Date:From:To:CC:Subject:References:In-Reply-To; b=kVQspp/5Pw7eaNfKNRbWAb1+hwua7TMjtKxjyTKBIoj23r3GYLrFE29E42Yfk5Ius zG5VyJGM+LzShbJOe+FitkuNM2Mp7t0NNW9H79/PIX5MvYzNQpp4ACIO5SuKakU+Lk XNXH4n2f4ZvyxNFU2jloOO2tX1EgfEx87J4QBORY= Received: from DFLE73.ent.ti.com (dfle73.ent.ti.com [128.247.5.110]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id v3PNSCKs004751 for ; Tue, 25 Apr 2017 18:28:12 -0500 Received: from dflp32.itg.ti.com (10.64.6.15) by DFLE73.ent.ti.com (128.247.5.110) with Microsoft SMTP Server id 14.3.294.0; Tue, 25 Apr 2017 18:28:11 -0500 Received: from localhost (ileax41-snat.itg.ti.com [10.172.224.153]) by dflp32.itg.ti.com (8.14.3/8.13.8) with ESMTP id v3PNSCfE018584; Tue, 25 Apr 2017 18:28:12 -0500 Date: Tue, 25 Apr 2017 19:28:11 -0400 From: Denys Dmytriyenko To: Karthik Ramanan Message-ID: <20170425232811.GU31608@edge> References: <1492704863-125562-1-git-send-email-a0393906@ti.com> <20170424170439.GA24846@edge> <58FF13CB.6060700@ti.com> MIME-Version: 1.0 In-Reply-To: <58FF13CB.6060700@ti.com> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: meta-arago@arago-project.org Subject: Re: [morty 1/2] wayland-ivi-extension: add recipe X-BeenThere: meta-arago@arago-project.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Arago metadata layer for TI SDKs - OE-Core/Yocto compatible List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Apr 2017 23:28:29 -0000 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline On Tue, Apr 25, 2017 at 02:45:55PM +0530, Karthik Ramanan wrote: > Denys, > > Comments inlined. > > Regards > Karthik > > On 24-Apr-17 10:34 PM, Denys Dmytriyenko wrote: > >On Thu, Apr 20, 2017 at 09:44:22PM +0530, Karthik Ramanan wrote: > >>This recipe is primarily adapted from the meta-agl. > >>URL reference: https://goo.gl/m0LLmT > >> > >>The recipe brings in the capability to use the ivi-shell > >>feature of the Weston and provides utilities to manage > >>the screens, layers and surfaces. > >> > >>force-type-conversion.patch overlayed in the recipe is required > >>to address a compilation issue seen only with TI graphics > >>stack. > >> > >>The other patches are required on top of the 1.11.0 version to > >>fully use the multiple screen features on TI platforms. > >> > >>Signed-off-by: Karthik Ramanan > >>--- > >> ...-add-surfaces-Add-screenId-as-an-argument.patch | 101 +++++++++++++++++++++ > >> ...yer-add-surfaces-surface-layer-management.patch | 69 ++++++++++++++ > >> .../force-type-conversion.patch | 13 +++ > >> .../wayland/wayland-ivi-extension_1.11.0.bb | 33 +++++++ > >> 4 files changed, 216 insertions(+) > >> create mode 100644 meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0001-layer-add-surfaces-Add-screenId-as-an-argument.patch > >> create mode 100644 meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0002-layer-add-surfaces-surface-layer-management.patch > >> create mode 100644 meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/force-type-conversion.patch > >> create mode 100644 meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension_1.11.0.bb > >> > >>diff --git a/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0001-layer-add-surfaces-Add-screenId-as-an-argument.patch b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0001-layer-add-surfaces-Add-screenId-as-an-argument.patch > >>new file mode 100644 > >>index 0000000..b8afc0d > >>--- /dev/null > >>+++ b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0001-layer-add-surfaces-Add-screenId-as-an-argument.patch > >>@@ -0,0 +1,101 @@ > >>+From 0a368e3d65f3f3cbe193e9e4c6b7b7ddf4ec7b54 Mon Sep 17 00:00:00 2001 > >>+From: Ramprasad N > >>+Date: Thu, 20 Apr 2017 13:24:20 +0530 > >>+Subject: [PATCH 1/2] layer-add-surfaces: Add screenId as an argument > >>+ > >>+Add Screen_ID as a argument to handle multiple screens. > >>+Removed choose_screen function as it is redundant > >>+after this change > >>+ > >>+Signed-off-by: Ramprasad N > >>+Reviewed-by: Karthik Ramanan > >>+--- > >>+ .../layer-add-surfaces/src/layer-add-surfaces.c | 47 +++++----------------- > >>+ 1 file changed, 11 insertions(+), 36 deletions(-) > >>+ > >>+diff --git a/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c b/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>+index aaff7de..46d7d15 100644 > >>+--- a/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>++++ b/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>+@@ -32,6 +32,7 @@ > >>+ > >>+ t_ilm_uint screenWidth; > >>+ t_ilm_uint screenHeight; > >>++t_ilm_uint screen_ID; > >>+ t_ilm_uint layer; > >>+ pthread_mutex_t mutex; > >>+ static pthread_cond_t waiterVariable = PTHREAD_COND_INITIALIZER; > >>+@@ -91,48 +92,20 @@ static void callbackFunction(ilmObjectType object, t_ilm_uint id, t_ilm_bool cre > >>+ } > >>+ } > >>+ > >>+-/* Choose the display with the largest resolution.*/ > >>+-static t_ilm_uint choose_screen(void) > >>++int main (int argc, const char * argv[]) > >>+ { > >>+ struct ilmScreenProperties screenProperties; > >>+- t_ilm_uint* screen_IDs = NULL; > >>+- t_ilm_uint screen_ID = 0; > >>+- t_ilm_uint screen_count = NULL; > >>+- t_ilm_uint choosen_width = 0; > >>+- t_ilm_uint choosen_height = 0; > >>+- int i; > >>+- > >>+- ilm_getScreenIDs(&screen_count, &screen_IDs); > >>+- > >>+- for (i = 0; i >>+- { > >>+- ilm_getPropertiesOfScreen(screen_IDs[i], &screenProperties); > >>+- if (screenProperties.screenWidth > choosen_width) { > >>+- choosen_width = screenProperties.screenWidth; > >>+- choosen_height = screenProperties.screenHeight; > >>+- screen_ID = screen_IDs[i]; > >>+- } > >>+- } > >>+ > >>+- screenWidth = choosen_width; > >>+- screenHeight = choosen_height; > >>+- > >>+- free(screen_IDs); > >>+- > >>+- return screen_ID; > >>+-} > >>+- > >>+-int main (int argc, const char * argv[]) > >>+-{ > >>+ // Get command-line options > >>+- if ( argc != 3) { > >>+- printf("Call layer-add-surface \n"); > >>++ if ( argc != 4) { > >>++ printf("Call layer-add-surface \n"); > >>+ return -1; > >>+ } > >>+ > >>+- layer = strtol(argv[1], NULL, 0); > >>++ screen_ID = strtol(argv[1], NULL, 0); > >>++ layer = strtol(argv[2], NULL, 0); > >>+ > >>+- number_of_surfaces = strtol(argv[2], NULL, 0); > >>++ number_of_surfaces = strtol(argv[3], NULL, 0); > >>+ > >>+ pthread_mutexattr_t a; > >>+ if (pthread_mutexattr_init(&a) != 0) > >>+@@ -156,11 +129,13 @@ int main (int argc, const char * argv[]) > >>+ pthread_mutexattr_destroy(&a); > >>+ > >>+ t_ilm_layer renderOrder[1]; > >>+- t_ilm_uint screen_ID; > >>+ renderOrder[0] = layer; > >>+ ilm_init(); > >>+ > >>+- screen_ID = choose_screen(); > >>++ ilm_getPropertiesOfScreen(screen_ID, &screenProperties); > >>++ screenWidth = screenProperties.screenWidth; > >>++ screenHeight = screenProperties.screenHeight; > >>++ printf("GetPropertiesOfScreen: screen ID (%d), Width (%u), Height (%u)\n", screen_ID, screenWidth, screenHeight); > >>+ ilm_layerCreateWithDimension(&layer, screenWidth, screenHeight); > >>+ printf("CreateWithDimension: layer ID (%d), Width (%u), Height (%u)\n", layer, screenWidth, screenHeight); > >>+ ilm_layerSetVisibility(layer,ILM_TRUE); > >>+-- > >>+1.9.1 > >>+ > >>diff --git a/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0002-layer-add-surfaces-surface-layer-management.patch b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0002-layer-add-surfaces-surface-layer-management.patch > >>new file mode 100644 > >>index 0000000..eff4a79 > >>--- /dev/null > >>+++ b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0002-layer-add-surfaces-surface-layer-management.patch > >>@@ -0,0 +1,69 @@ > >>+From 0d164d686147d2d3b08cd79ef9930f62ff8d528b Mon Sep 17 00:00:00 2001 > >>+From: Ramprasad N > >>+Date: Thu, 20 Apr 2017 13:30:31 +0530 > >>+Subject: [PATCH 2/2] layer-add-surfaces: surface-layer management > >>+ > >>+This patch prevents surfaces from getting added to a new layer > >>+if they are already registered to an existing layer. > >>+ > >>+Signed-off-by: Ramprasad N > >>+Reviewed-by: Karthik Ramanan > >>+--- > >>+ .../layer-add-surfaces/src/layer-add-surfaces.c | 35 ++++++++++++++++++++-- > >>+ 1 file changed, 33 insertions(+), 2 deletions(-) > >>+ > >>+diff --git a/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c b/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>+index 46d7d15..cc76117 100644 > >>+--- a/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>++++ b/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>+@@ -59,14 +59,45 @@ static void surfaceCallbackFunction(t_ilm_uint id, struct ilmSurfaceProperties* > >>+ configure_ilm_surface(id, sp->origSourceWidth, sp->origSourceHeight); > >>+ } > >>+ } > >>+- > >>+ static void callbackFunction(ilmObjectType object, t_ilm_uint id, t_ilm_bool created, void *user_data) > >>+ { > >>+ (void)user_data; > >>+ struct ilmSurfaceProperties sp; > >>++ t_ilm_int llength, slength, i, j; > >>++ t_ilm_uint* layerIDs = NULL; > >>++ t_ilm_uint* surfaceIDs = NULL; > >>++ t_ilm_bool addRequired = ILM_TRUE; > >>+ > >>+ if (object == ILM_SURFACE) { > >>+- if (created) { > >>++ ilm_getLayerIDs(&llength, &layerIDs); > >>++ > >>++ if(llength > 1) { > >>++ for(i=0; i < llength; i++) { > >>++ if(layerIDs[i] != layer) { > >>++ ilm_getSurfaceIDsOnLayer(layerIDs[i], &slength, &surfaceIDs); > >>++ for(j=0; j < slength; j++){ > >>++ if(surfaceIDs[j] == id){ > >>++ addRequired = ILM_FALSE; > >>++ printf("surface %d is already attached to layer %d\n", id, layerIDs[i]); > >>++ } > >>++ > >>++ } > >>++ > >>++ if(surfaceIDs) { > >>++ free(surfaceIDs); > >>++ } > >>++ > >>++ } > >>++ > >>++ } > >>++ > >>++ } > >>++ > >>++ if(layerIDs) { > >>++ free(layerIDs); > >>++ } > >>++ > >>++ if (created && addRequired) { > >>+ if (number_of_surfaces > 0) { > >>+ number_of_surfaces--; > >>+ printf("surface : %d created\n",id); > >>+-- > >>+1.9.1 > >>+ > >>diff --git a/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/force-type-conversion.patch b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/force-type-conversion.patch > >>new file mode 100644 > >>index 0000000..a72aeed > >>--- /dev/null > >>+++ b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/force-type-conversion.patch > >>@@ -0,0 +1,13 @@ > >>+diff --git a/ivi-layermanagement-examples/EGLWLMockNavigation/src/OpenGLES2App.cpp b/ivi-layermanagement-examples/EGLWLMockNavigation/src/OpenGLES2App.cpp > >>+index 2e65864..e14dbc0 100644 > >>+--- a/ivi-layermanagement-examples/EGLWLMockNavigation/src/OpenGLES2App.cpp > >>++++ b/ivi-layermanagement-examples/EGLWLMockNavigation/src/OpenGLES2App.cpp > >>+@@ -244,7 +247,7 @@ bool OpenGLES2App::createEGLContext() > >>+ m_eglContextStruct.eglSurface = NULL; > >>+ m_eglContextStruct.eglContext = NULL; > >>+ > >>+- m_eglContextStruct.eglDisplay = eglGetDisplay(m_wlContextStruct.wlDisplay); > >>++ m_eglContextStruct.eglDisplay = eglGetDisplay((EGLNativeDisplayType)m_wlContextStruct.wlDisplay); > >>+ eglstatus = eglGetError(); > >>+ if (!m_eglContextStruct.eglDisplay) > >>+ { > >>diff --git a/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension_1.11.0.bb b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension_1.11.0.bb > >>new file mode 100644 > >>index 0000000..157e1ee > >>--- /dev/null > >>+++ b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension_1.11.0.bb > >>@@ -0,0 +1,33 @@ > >>+SUMMARY = "Wayland IVI Extension" > >>+DESCRIPTION = "GENIVI Layer Management API based on Wayland IVI Extension" > >>+HOMEPAGE = "http://projects.genivi.org/wayland-ivi-extension" > >>+BUGTRACKER = "http://bugs.genivi.org/enter_bug.cgi?product=Wayland%20IVI%20Extension" > >>+LICENSE = "Apache-2.0" > >>+LIC_FILES_CHKSUM = "file://LICENSE;md5=1f1a56bb2dadf5f2be8eb342acf4ed79" > >>+ > >>+SRCREV = "c9001582b10ce209c37b42dd560947c5aa8928b3" > >>+ > >>+SRC_URI = "git://github.com/GENIVI/${BPN}.git;protocol=http \ > >>+ file://force-type-conversion.patch \ > >>+ file://0001-layer-add-surfaces-Add-screenId-as-an-argument.patch \ > >>+ file://0002-layer-add-surfaces-surface-layer-management.patch \ > >>+ " > >>+ > >>+S = "${WORKDIR}/git" > >>+ > >>+DEPENDS = "weston virtual/libgles2 pixman" > >>+ > >>+inherit cmake > >>+ > >>+EXTRA_OECMAKE := "-DWITH_ILM_INPUT=1" > >>+EXTRA_OECMAKE += "-DLIB_SUFFIX=${@d.getVar('baselib', True).replace('lib', '')}" > >What these 2 lines are doing? There's no need to use := in the first line. And > >in the second line, why so convoluted way - all it does is pass an empty string. > >Can these 2 lines be simplified and merged into one? > Few points: > a) This recipe section with is taken directly from the meta-agl > reference that I provided. > b) The immediate variable expansion ":=", can be avoided but its > fine to have right. Is there a recommended best practice? Well, it's not clear why it's needed. It would force any expansion early on, but there are none. > c) You are right that the expansion results in an empty string, but > I looked further into this and I see that such a usage is quite > prevalent. > I don't quite understand the intended purpose. Can you please let me > know what the intended purpose of this could be? > Again, I can change this but like I mentioned this is coming from > the upstream recipe. Let me know what you think? Ok, since the recipe is coming from another layer, it explains few things. The code there that extracts LIB_SUFFIX is made to be platform-agnostic - most of the architectures have baselib defined as "lib", but PowerPC64 defines it as "lib64", hence LIB_SUFFIX will be "64" in that case. It wouldn't make much sense for meta-arago, since we only care/support ARM, but other layers may want to be more inclusive and platform-agnostic... Now, it would be nice to mention that the recipe comes from meta-agl with minimal changes (only extra patches?) And Eric mentioned that it required some extra config steps to make it work - do you want to incorporate that into v2 patchset? > >>+ > >>+PR = "r3" > >>+ > >>+# Need these temporarily to prevent a non-fatal do_package_qa issue > >>+INSANE_SKIP_${PN} += "dev-deps" > >>+INSANE_SKIP_${PN}-dev += "dev-elf dev-so" > >>-- > >>1.9.1 > >> > >>_______________________________________________ > >>meta-arago mailing list > >>meta-arago@arago-project.org > >>http://arago-project.org/cgi-bin/mailman/listinfo/meta-arago >