Re: [Mesa-dev] Introduction to Community and Queries Regarding Task : Unit and performance tests for VA-API
On Sat, Sep 1, 2018 at 9:02 AM Iti Shree wrote: > > Good morning, > > I am Iti Shree currently pursuing B.Tech degree in Information and > Technology. I was browsing through some project and saw Xorg project ideas. > Many of them are indeed interesting, though they have precise hardware > requirement (I have AMD processor). > > I was reading ideas and I think it's good to start with " Unit and > performance tests for VA-API", however, the task doesn't mention potential > mentors, therefore, I thought I should send a mail. > > I'd like to know more about this project and to know who to contact exactly > for further queries regarding the same. Julien Isorce , Leo Liu , or Christian König would be possible mentors for this project. Alex ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] radv: Add VEGA20 support.
Just mirror the radeonsi bits. Since this is just adding the extra
switch entries for new HW I think this should be fine for stable.
CC:
---
src/amd/vulkan/radv_pipeline.c | 1 +
src/amd/vulkan/si_cmd_buffer.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index 13477231a4f..1741d5e9047 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -2539,6 +2539,7 @@ radv_pipeline_generate_binning_state(struct radeon_cmdbuf
*cs,
switch (pipeline->device->physical_device->rad_info.family) {
case CHIP_VEGA10:
case CHIP_VEGA12:
+ case CHIP_VEGA20:
context_states_per_bin = 1;
persistent_states_per_bin = 1;
fpovs_per_batch = 63;
diff --git a/src/amd/vulkan/si_cmd_buffer.c b/src/amd/vulkan/si_cmd_buffer.c
index 2cfa7f4c2c3..435878c3722 100644
--- a/src/amd/vulkan/si_cmd_buffer.c
+++ b/src/amd/vulkan/si_cmd_buffer.c
@@ -340,6 +340,7 @@ si_emit_config(struct radv_physical_device *physical_device,
switch (physical_device->rad_info.family) {
case CHIP_VEGA10:
case CHIP_VEGA12:
+ case CHIP_VEGA20:
pc_lines = 4096;
break;
case CHIP_RAVEN:
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] radv: Use a lower max offchip buffer count.
No clue what gets fixed by this but both radeonsi and amdvlk do it.
CC:
---
src/amd/vulkan/radv_device.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index c300c88468a..3d208ab053d 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -1901,10 +1901,30 @@ radv_get_hs_offchip_param(struct radv_device *device,
uint32_t *max_offchip_buff
device->physical_device->rad_info.family != CHIP_CARRIZO &&
device->physical_device->rad_info.family != CHIP_STONEY;
unsigned max_offchip_buffers_per_se = double_offchip_buffers ? 128 : 64;
- unsigned max_offchip_buffers = max_offchip_buffers_per_se *
- device->physical_device->rad_info.max_se;
+ unsigned max_offchip_buffers;
unsigned offchip_granularity;
unsigned hs_offchip_param;
+
+ /*
+* Per RadeonSI:
+* This must be one less than the maximum number due to a hw limitation.
+ * Various hardware bugs in SI, CIK, and GFX9 need this.
+*
+* Per AMDVLK:
+* Vega10 should limit max_offchip_buffers to 508 (4 * 127).
+* Gfx7 should limit max_offchip_buffers to 508
+* Gfx6 should limit max_offchip_buffers to 126 (2 * 63)
+*
+* Follow AMDVLK here.
+ */
+ if (device->physical_device->rad_info.family == CHIP_VEGA10 ||
+ device->physical_device->rad_info.chip_class == CIK ||
+ device->physical_device->rad_info.chip_class == SI)
+ --max_offchip_buffers_per_se;
+
+ max_offchip_buffers = max_offchip_buffers_per_se *
+ device->physical_device->rad_info.max_se;
+
switch (device->tess_offchip_block_dw_size) {
default:
assert(0);
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] radv: Fix CMASK dimensions.
Mirrors 1e40f694831 "ac/surface: fix CMASK fast clear for NPOT textures with mipmapping on SI/CI/VI" CC: --- src/amd/vulkan/radv_image.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c index f1c78e8115d..b316242dc5a 100644 --- a/src/amd/vulkan/radv_image.c +++ b/src/amd/vulkan/radv_image.c @@ -801,8 +801,8 @@ radv_image_get_cmask_info(struct radv_device *device, unsigned base_align = num_pipes * pipe_interleave_bytes; - unsigned width = align(image->info.width, cl_width*8); - unsigned height = align(image->info.height, cl_height*8); + unsigned width = align(image->surface.u.legacy.level[0].nblk_x, cl_width*8); + unsigned height = align(image->surface.u.legacy.level[0].nblk_y, cl_height*8); unsigned slice_elements = (width * height) / (8*8); /* Each element of CMASK is a nibble. */ -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] tnl: Fix green gun regression in xonotic.
On 08/21/2018 09:57 PM, mathias.froehl...@gmx.net wrote:
> From: Mathias Fröhlich
>
> Hi Ville, Brian,
>
> The below patch fixes the regression to tnl drivers that Ville reported
> a hand full weeks ago.
> Please review!
>
> best
> Mathias
>
>
> Fix an other regression of patch 64d2a2048054
> mesa: Make gl_vertex_array contain pointers to first order VAO members.
Rewrite the as
Fixes: 64d2a204805 ("mesa: Make gl_vertex_array contain pointers to first order
VAO members.")
and put it down with the *-by: lines below so that it can be applied to
the correct release branches.
> The regression showed up with drivers using the tnl module and
> was reproducible using xonotic-glx -benchmark demos/the-big-keybench.dem.
>
> This patch survives intels CI system without changes in the tests.
>
> Tested-by: Ville Syrjälä
> Signed-off-by: Mathias Fröhlich
> ---
> src/mesa/tnl/t_split_copy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/tnl/t_split_copy.c b/src/mesa/tnl/t_split_copy.c
> index cbb7eb409f..085ae9a28c 100644
> --- a/src/mesa/tnl/t_split_copy.c
> +++ b/src/mesa/tnl/t_split_copy.c
> @@ -531,7 +531,7 @@ replay_init(struct copy_context *copy)
> for (offset = 0, i = 0; i < copy->nr_varying; i++) {
>const struct tnl_vertex_array *src = copy->varying[i].array;
>const struct gl_array_attributes *srcattr = src->VertexAttrib;
> - struct tnl_vertex_array *dst = ©->dstarray[i];
> + struct tnl_vertex_array *dst = ©->dstarray[copy->varying[i].attr];
>struct gl_vertex_buffer_binding *dstbind =
> ©->varying[i].dstbinding;
>struct gl_array_attributes *dstattr = ©->varying[i].dstattribs;
>
I don't know this code very well, but this seems to better match the
original (before 64d2a204805). Since it works for Ville and passes the
Intel CI,
Reviewed-by: Ian Romanick
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] radv: Fix CMASK dimensions.
Reviewed-by: Dave Airlie for the series. On Mon, 3 Sep 2018 at 10:39, Bas Nieuwenhuizen wrote: > > Mirrors > > 1e40f694831 "ac/surface: fix CMASK fast clear for NPOT textures with > mipmapping on SI/CI/VI" > > CC: > --- > src/amd/vulkan/radv_image.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c > index f1c78e8115d..b316242dc5a 100644 > --- a/src/amd/vulkan/radv_image.c > +++ b/src/amd/vulkan/radv_image.c > @@ -801,8 +801,8 @@ radv_image_get_cmask_info(struct radv_device *device, > > unsigned base_align = num_pipes * pipe_interleave_bytes; > > - unsigned width = align(image->info.width, cl_width*8); > - unsigned height = align(image->info.height, cl_height*8); > + unsigned width = align(image->surface.u.legacy.level[0].nblk_x, > cl_width*8); > + unsigned height = align(image->surface.u.legacy.level[0].nblk_y, > cl_height*8); > unsigned slice_elements = (width * height) / (8*8); > > /* Each element of CMASK is a nibble. */ > -- > 2.18.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/7] gallium: add PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS
screen.rst is missing documentation for this cap.
Marek
On Thu, Aug 30, 2018 at 9:40 AM, Erik Faye-Lund
wrote:
> This gets rid of a r600 specific hack in the state-tracker, and prepares
> for other drivers to be able to use hw-atomics.
>
> While we're at it, clean up some indentation in the various drivers.
>
> Signed-off-by: Erik Faye-Lund
> ---
> src/gallium/drivers/etnaviv/etnaviv_screen.c | 3 +++
> src/gallium/drivers/freedreno/freedreno_screen.c | 3 +++
> src/gallium/drivers/nouveau/nv30/nv30_screen.c | 2 ++
> src/gallium/drivers/nouveau/nv50/nv50_screen.c | 2 ++
> src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 2 ++
> src/gallium/drivers/r300/r300_screen.c | 6 +-
> src/gallium/drivers/r600/r600_pipe.c | 4
> src/gallium/drivers/radeonsi/si_get.c| 2 ++
> src/gallium/drivers/svga/svga_screen.c | 2 ++
> src/gallium/drivers/v3d/v3d_screen.c | 8 ++--
> src/gallium/drivers/vc4/vc4_screen.c | 8 ++--
> src/gallium/drivers/virgl/virgl_screen.c | 2 ++
> src/gallium/include/pipe/p_defines.h | 1 +
> src/mesa/state_tracker/st_extensions.c | 9 +
> 14 files changed, 45 insertions(+), 9 deletions(-)
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c
> b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> index 9669bd2f60..108b97d35c 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> @@ -289,8 +289,11 @@ etna_screen_get_param(struct pipe_screen *pscreen, enum
> pipe_cap param)
> case PIPE_CAP_MAX_GS_INVOCATIONS:
>return 32;
>
> + /* shader buffer objects */
> case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
>return 1 << 27;
> + case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS:
> + return 0;
>
> /* Stream output. */
> case PIPE_CAP_MAX_STREAM_OUTPUT_BUFFERS:
> diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c
> b/src/gallium/drivers/freedreno/freedreno_screen.c
> index 489986703c..af44ab698e 100644
> --- a/src/gallium/drivers/freedreno/freedreno_screen.c
> +++ b/src/gallium/drivers/freedreno/freedreno_screen.c
> @@ -376,8 +376,11 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum
> pipe_cap param)
> case PIPE_CAP_MAX_GS_INVOCATIONS:
> return 32;
>
> + /* shader buffer objects */
> case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
> return 1 << 27;
> + case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS:
> + return 0;
>
> case PIPE_CAP_CONTEXT_PRIORITY_MASK:
> return screen->priority_mask;
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> index da7ebecd5d..d52d8f3988 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> @@ -245,6 +245,8 @@ nv30_screen_get_param(struct pipe_screen *pscreen, enum
> pipe_cap param)
>return 32;
> case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
>return 1 << 27;
> + case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS:
> + return 0;
> case PIPE_CAP_VENDOR_ID:
>return 0x10de;
> case PIPE_CAP_DEVICE_ID: {
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> index 0007a2b957..cd36795e56 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> @@ -299,6 +299,8 @@ nv50_screen_get_param(struct pipe_screen *pscreen, enum
> pipe_cap param)
>return 32;
> case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
>return 1 << 27;
> + case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS:
> + return 0;
> case PIPE_CAP_VENDOR_ID:
>return 0x10de;
> case PIPE_CAP_DEVICE_ID: {
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> index 4701b768be..446e30dcc8 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> @@ -272,6 +272,8 @@ nvc0_screen_get_param(struct pipe_screen *pscreen, enum
> pipe_cap param)
>return 32;
> case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
>return 1 << 27;
> + case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS:
> + return 0;
> case PIPE_CAP_CONSERVATIVE_RASTER_PRE_SNAP_TRIANGLES:
>return class_3d >= GP100_3D_CLASS;
> case PIPE_CAP_SEAMLESS_CUBE_MAP_PER_TEXTURE:
> diff --git a/src/gallium/drivers/r300/r300_screen.c
> b/src/gallium/drivers/r300/r300_screen.c
> index 01a95d98dc..d27c2b8f1d 100644
> --- a/src/gallium/drivers/r300/r300_screen.c
> +++ b/src/gallium/drivers/r300/r300_screen.c
> @@ -265,8 +265,12 @@ static int r300_get_param(struct pipe_screen* pscreen,
> enum pipe_cap param)
>
> case PIPE_CAP_MAX_GS_INVOCATIONS:
> return 32;
> - case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
> +
>
[Mesa-dev] [Bug 107786] [DXVK] MSAA reflections are broken in GTA V
https://bugs.freedesktop.org/show_bug.cgi?id=107786 Bug ID: 107786 Summary: [DXVK] MSAA reflections are broken in GTA V Product: Mesa Version: git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Vulkan/radeon Assignee: mesa-dev@lists.freedesktop.org Reporter: li...@protonmail.com QA Contact: mesa-dev@lists.freedesktop.org Created attachment 141419 --> https://bugs.freedesktop.org/attachment.cgi?id=141419&action=edit Screenshot In GTA V, setting "Reflection MSAA" setting to X2, X4 or X8 results in broken reflections, with the top of the car reflecting the ground (see attachment). Info: - DXVK 0.70 - wine-staging 3.14 - mesa-git c1ba33c34b - llvm-svn 341191 -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] egl/android: rework device probing
Hi Emil,
On Sat, Sep 1, 2018 at 2:03 AM Emil Velikov wrote:
>
> From: Emil Velikov
>
Thanks for the patch! Please see my comments below.
[snip]
> @@ -1214,10 +1215,13 @@ droid_open_device_drm_gralloc(struct dri2_egl_display
> *dri2_dpy)
>&fd);
> if (err || fd < 0) {
>_eglLog(_EGL_WARNING, "fail to get drm fd");
> - fd = -1;
> + return EGL_FALSE;
> }
>
> - return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1;
> + if (dri2_dpy->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3) < 0)
This will assign the boolean result of the comparison to dri2_dpy->fd.
To avoid parenthesis hell, I'd rewrite this to:
dri2_dpy->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
if (dri2_dpy->fd < 0)
return EGL_FALSE;
> + return EGL_FALSE;
> +
> + return droid_probe_device(disp);
> }
> #endif /* HAVE_DRM_GRALLOC */
>
> @@ -1365,7 +1369,7 @@ static const __DRIextension
> *droid_image_loader_extensions[] = {
> EGLBoolean
> droid_load_driver(_EGLDisplay *disp)
Not related to this patch, but I guess we could fix it up, while at
it. Fails to build with:
src/egl/drivers/dri2/platform_android
.c:1369:1: error: no previous prototype for function
'droid_load_driver' [-Werror,-Wmissing-prototypes]
droid_load_driver(_EGLDisplay *disp)
^
The function should be static.
> {
> - struct dri2_egl_display *dri2_dpy = disp->DriverData;
> + struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> const char *err;
>
> dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd);
> @@ -1404,6 +1408,17 @@ error:
> return false;
> }
[snip]
> +static EGLBoolean
> droid_open_device(_EGLDisplay *disp)
> {
> #define MAX_DRM_DEVICES 32
> + struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> drmDevicePtr device, devices[MAX_DRM_DEVICES] = { NULL };
> int prop_set, num_devices;
Not related to this patch, but prop_set is unused. We could add a
fixup in this patch, while reworking this already.
I'm going to test it on Chrome OS with the fixups above applied.
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 107786] [DXVK] MSAA reflections are broken in GTA V
https://bugs.freedesktop.org/show_bug.cgi?id=107786 --- Comment #1 from Clément Guérin --- Here's an renderdoc capture: https://mega.nz/#!WK5HlAKQ!g77NnAuH_XmeR20i2IXjTUSG2NDDiFy86b6cqVU8ACI The game creates an environment cubemap in Colour Pass #3. Each face of the cube is rendered to a single texture, then copied to a cubemap. When MSAA is disabled, vkCmdCopyImage is used and works fine. When MSAA is enabled, vkCmdResolveImage is used but it doesn't seem to take into account `dstSubresource->baseArrayLayer`, resulting in each texture being copied to the X+ face of the cubemap (layer 0), generating a corrupted cubemap. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 107786] [DXVK] MSAA reflections are broken in GTA V
https://bugs.freedesktop.org/show_bug.cgi?id=107786 --- Comment #2 from Clément Guérin --- Created attachment 141420 --> https://bugs.freedesktop.org/attachment.cgi?id=141420&action=edit Dual-paraboloid map, MSAA off -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 107786] [DXVK] MSAA reflections are broken in GTA V
https://bugs.freedesktop.org/show_bug.cgi?id=107786 --- Comment #3 from Clément Guérin --- Created attachment 141421 --> https://bugs.freedesktop.org/attachment.cgi?id=141421&action=edit Dual-paraboloid map, MSAA on Dual-paraboloid map generated in Colour Pass #4. See http://www.adriancourreges.com/blog/2015/11/02/gta-v-graphics-study/ -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2] Check if the window is non-NULL before setting swap interval.
On Thu, Aug 30, 2018 at 11:23 PM Emil Velikov wrote: > > On 30 August 2018 at 11:41, Eric Engestrom wrote: > > On Thursday, 2018-08-30 17:55:14 +0900, Tomasz Figa wrote: > >> Hi Erik, Emil, Eric, > >> > >> On Tue, Jul 10, 2018 at 12:41 AM Erik Faye-Lund > >> wrote: > >> > > >> > On Thu, Jul 5, 2018 at 7:02 PM Emil Velikov > >> > wrote: > >> > > > >> > > On 5 July 2018 at 17:17, Eric Engestrom > >> > > wrote: > >> > > > On Thursday, 2018-07-05 14:43:02 +0100, Emil Velikov wrote: > >> > > >> On 5 July 2018 at 10:53, Eric Engestrom > >> > > >> wrote: > >> > > >> > On Monday, 2018-07-02 14:12:44 +0530, samiuddi wrote: > >> > > >> >> This fixes crash due to NULL window when swap interval is set > >> > > >> >> for pbuffer surface. > >> > > >> >> > >> > > >> >> Test: CtsDisplayTestCases pass > >> > > >> >> > >> > > >> >> Signed-off-by: samiuddi > >> > > >> >> --- > >> > > >> >> > >> > > >> >> Kindly ignore this patch > >> > > >> >> https://lists.freedesktop.org/archives/mesa-dev/2018-July/199098.html > >> > > >> >> > >> > > >> >> src/egl/drivers/dri2/platform_android.c | 2 +- > >> > > >> >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> >> > >> > > >> >> diff --git a/src/egl/drivers/dri2/platform_android.c > >> > > >> >> b/src/egl/drivers/dri2/platform_android.c > >> > > >> >> index ca8708a..b5b960a 100644 > >> > > >> >> --- a/src/egl/drivers/dri2/platform_android.c > >> > > >> >> +++ b/src/egl/drivers/dri2/platform_android.c > >> > > >> >> @@ -485,7 +485,7 @@ droid_swap_interval(_EGLDriver *drv, > >> > > >> >> _EGLDisplay *dpy, > >> > > >> >> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); > >> > > >> >> struct ANativeWindow *window = dri2_surf->window; > >> > > >> >> > >> > > >> >> - if (window->setSwapInterval(window, interval)) > >> > > >> >> + if (window && window->setSwapInterval(window, interval)) > >> > > >> >>return EGL_FALSE; > >> > > >> > > >> > > >> > Shouldn't we return false if we couldn't set the swap interval? > >> > > >> > > >> > > >> > I think this should be: > >> > > >> >if (!window || window->setSwapInterval(window, interval)) > >> > > >> > return EGL_FALSE; > >> > > >> > > >> > > >> Changing the patch as above will lead to eglSwapInterval > >> > > >> consistently > >> > > >> failing for pbuffer surfaces. > >> > > > > >> > > > I'm not that familiar with pbuffers, but does SwapInterval really > >> > > > make > >> > > > sense for them? I thought you couldn't swap a pbuffer. > >> > > > > >> > > > If so, failing an invalid op seems like the right thing to do. > >> > > > Otherwise, I guess sure, no-op returning success is fine. > >> > > > > >> > > Looking at eglSwapInterval manpage [1] (with my annotation): > >> > > "eglSwapInterval — specifies the minimum number of video frame periods > >> > > per buffer swap for the _window_ associated with the current context." > >> > > While the spec does not mention window/pbuffer/pixmap at all - it > >> > > extensively refers to eglSwapBuffers. > >> > > > >> > > Wrt eglSwapBuffers manpage [2] and spec are consistent: > >> > > > >> > > "If surface is a pixel buffer or a pixmap, eglSwapBuffers has no > >> > > effect, and no error is generated." > >> > > > >> > > Perhaps it's wrong to set eglSwapInterval for !window surfaces, but > >> > > its sibling (eglSwapBuffers) does not error out. > >> > > Hence I doubt many users make a distinction between window and pbuffer > >> > > surfaces for eglSwap*. > >> > > > >> > > Worth bringing to the WG meeting - it' planned for 1st August. > >> > > > >> > > >> > As I pointed out when I proposed this variant here: > >> > https://patchwork.freedesktop.org/patch/219313/ > >> > > >> > "Also, I don't think EGL_FALSE is the right return-value, as it doesn't > >> > seem the EGL 1.5 spec defines any such error. Also, for instance > >> > dri2_swap_interval returns EGL_TRUE when there's no driver-function, > >> > which further backs the "silent failure" in this case IMO." > >> > > >> > So I think EGL_TRUE is the correct return-value for now. If the spec > >> > gets changed, we can of course update our implementation. > >> > >> What happens to this patch in the end? It looks like we're observing a > >> similar problem in Chrome OS. > >> > >> Emil, was there any follow-up on the WG meeting? > > > > Meeting was cancelled, but I raised the issue here: > > https://gitlab.khronos.org/egl/API/merge_requests/17 > > > > Right now we have ARM saying they return false + error and NVIDIA saying > > they return true + no error and that ARM has a bug. > > I think another party adding their opinion might nudge it forward :) > > > Fwiw I put forward a suggestion to add a workaround in the > Android/CrOS libEGL wrapper for the ARM drivers. > Although one could also consider the Nvidia/Mesa drivers as > non-compliant and add a workaround for those instead. > > I have to admit it's not pretty, but it seems consistent with all the > other workarounds in the wrapper. > > As Eric said -
Re: [Mesa-dev] [PATCH v2] egl/android: rework device probing
On Mon, Sep 3, 2018 at 2:45 PM Tomasz Figa wrote:
>
> Hi Emil,
>
> On Sat, Sep 1, 2018 at 2:03 AM Emil Velikov wrote:
> >
> > From: Emil Velikov
> >
>
> Thanks for the patch! Please see my comments below.
>
> [snip]
> > @@ -1214,10 +1215,13 @@ droid_open_device_drm_gralloc(struct
> > dri2_egl_display *dri2_dpy)
> >&fd);
> > if (err || fd < 0) {
> >_eglLog(_EGL_WARNING, "fail to get drm fd");
> > - fd = -1;
> > + return EGL_FALSE;
> > }
> >
> > - return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1;
> > + if (dri2_dpy->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3) < 0)
>
> This will assign the boolean result of the comparison to dri2_dpy->fd.
> To avoid parenthesis hell, I'd rewrite this to:
>
> dri2_dpy->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3);
> if (dri2_dpy->fd < 0)
>return EGL_FALSE;
>
> > + return EGL_FALSE;
> > +
> > + return droid_probe_device(disp);
> > }
> > #endif /* HAVE_DRM_GRALLOC */
> >
> > @@ -1365,7 +1369,7 @@ static const __DRIextension
> > *droid_image_loader_extensions[] = {
> > EGLBoolean
> > droid_load_driver(_EGLDisplay *disp)
>
> Not related to this patch, but I guess we could fix it up, while at
> it. Fails to build with:
>
> src/egl/drivers/dri2/platform_android
> .c:1369:1: error: no previous prototype for function
> 'droid_load_driver' [-Werror,-Wmissing-prototypes]
> droid_load_driver(_EGLDisplay *disp)
> ^
>
> The function should be static.
>
> > {
> > - struct dri2_egl_display *dri2_dpy = disp->DriverData;
> > + struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> > const char *err;
> >
> > dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd);
> > @@ -1404,6 +1408,17 @@ error:
> > return false;
> > }
> [snip]
> > +static EGLBoolean
> > droid_open_device(_EGLDisplay *disp)
> > {
> > #define MAX_DRM_DEVICES 32
> > + struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> > drmDevicePtr device, devices[MAX_DRM_DEVICES] = { NULL };
> > int prop_set, num_devices;
>
> Not related to this patch, but prop_set is unused. We could add a
> fixup in this patch, while reworking this already.
>
> I'm going to test it on Chrome OS with the fixups above applied.
The probing itself seems to be working fine, but it looks like there
is some EGL image regression in master, which I don't have time to
investigate.
Tested without the vendor property set, with both cases of the desired
device being probed in first or some later iteration. In both cases
the initialization succeeded and I could see the right GL renderer
string.
Tested-by: Tomasz Figa
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] egl/wayland: do not leak wl_buffer when it is locked
On Sat, 2018-09-01 at 02:14 +0300, Andres Gomez wrote:
> Juan, should we also include this in the stable queues ?
>
Unless Daniel has a different opinion, yes, it should be included. Forgot to CC
to stable.
Thanks!
J.A.
>
> On Thu, 2018-08-30 at 13:59 +0200, Juan A. Suarez Romero wrote:
> > If color buffer is locked, do not set its wayland buffer to NULL;
> > otherwise it can not be freed later.
> >
> > Rather, flag it in order to destroy it later on the release event.
> >
> > v2: instruct release event to unlock only or free wl_buffer too (Daniel)
> >
> > This also fixes dEQP-EGL.functional.swap_buffers_with_damage.* tests.
> >
> > CC: Daniel Stone
> > ---
> > src/egl/drivers/dri2/egl_dri2.h | 1 +
> > src/egl/drivers/dri2/platform_wayland.c | 22 +++---
> > 2 files changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/egl_dri2.h
> > b/src/egl/drivers/dri2/egl_dri2.h
> > index d1e4e8dcf22..349f66a3506 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.h
> > +++ b/src/egl/drivers/dri2/egl_dri2.h
> > @@ -297,6 +297,7 @@ struct dri2_egl_surface
> > struct {
> > #ifdef HAVE_WAYLAND_PLATFORM
> >struct wl_buffer *wl_buffer;
> > + boolwl_release;
> >__DRIimage *dri_image;
> >/* for is_different_gpu case. NULL else */
> >__DRIimage *linear_copy;
> > diff --git a/src/egl/drivers/dri2/platform_wayland.c
> > b/src/egl/drivers/dri2/platform_wayland.c
> > index 11c57de0f89..03a3e0993b0 100644
> > --- a/src/egl/drivers/dri2/platform_wayland.c
> > +++ b/src/egl/drivers/dri2/platform_wayland.c
> > @@ -182,9 +182,12 @@ wl_buffer_release(void *data, struct wl_buffer *buffer)
> >if (dri2_surf->color_buffers[i].wl_buffer == buffer)
> > break;
> >
> > - if (i == ARRAY_SIZE(dri2_surf->color_buffers)) {
> > + assert (i < ARRAY_SIZE(dri2_surf->color_buffers));
> > +
> > + if (dri2_surf->color_buffers[i].wl_release) {
> >wl_buffer_destroy(buffer);
> > - return;
> > + dri2_surf->color_buffers[i].wl_release = false;
> > + dri2_surf->color_buffers[i].wl_buffer = NULL;
> > }
> >
> > dri2_surf->color_buffers[i].locked = false;
> > @@ -425,9 +428,14 @@ dri2_wl_release_buffers(struct dri2_egl_surface
> > *dri2_surf)
> >dri2_egl_display(dri2_surf->base.Resource.Display);
> >
> > for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> > - if (dri2_surf->color_buffers[i].wl_buffer &&
> > - !dri2_surf->color_buffers[i].locked)
> > - wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
> > + if (dri2_surf->color_buffers[i].wl_buffer) {
> > + if (dri2_surf->color_buffers[i].locked) {
> > +dri2_surf->color_buffers[i].wl_release = true;
> > + } else {
> > +wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
> > +dri2_surf->color_buffers[i].wl_buffer = NULL;
> > + }
> > + }
> >if (dri2_surf->color_buffers[i].dri_image)
> >
> > dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].dri_image);
> >if (dri2_surf->color_buffers[i].linear_copy)
> > @@ -436,11 +444,9 @@ dri2_wl_release_buffers(struct dri2_egl_surface
> > *dri2_surf)
> > munmap(dri2_surf->color_buffers[i].data,
> > dri2_surf->color_buffers[i].data_size);
> >
> > - dri2_surf->color_buffers[i].wl_buffer = NULL;
> >dri2_surf->color_buffers[i].dri_image = NULL;
> >dri2_surf->color_buffers[i].linear_copy = NULL;
> >dri2_surf->color_buffers[i].data = NULL;
> > - dri2_surf->color_buffers[i].locked = false;
> > }
> >
> > if (dri2_dpy->dri2)
> > @@ -968,6 +974,8 @@ dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
> >dri2_surf->current->wl_buffer =
> > create_wl_buffer(dri2_dpy, dri2_surf, image);
> >
> > + dri2_surf->current->wl_release = false;
> > +
> >wl_buffer_add_listener(dri2_surf->current->wl_buffer,
> > &wl_buffer_listener, dri2_surf);
> > }
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
