Re: [Mesa-dev] [PATCH 3/3] mesa: add missing break in switch statement
On 21/04/15 08:27, Tapani Pälli wrote: Signed-off-by: Tapani Pälli --- src/mesa/main/shader_query.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index 1428058..336598d 100644 --- a/src/mesa/main/shader_query.cpp +++ b/src/mesa/main/shader_query.cpp @@ -634,7 +634,7 @@ _mesa_program_resource_find_index(struct gl_shader_program *shProg, case GL_ATOMIC_COUNTER_BUFFER: if (_mesa_program_resource_index(shProg, res) == index) return res; - + break; case GL_TRANSFORM_FEEDBACK_VARYING: case GL_PROGRAM_INPUT: case GL_PROGRAM_OUTPUT: Reviewed-By: Martin Peres ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: fix UBO queries for active uniforms
On 21/04/15 08:27, Tapani Pälli wrote:
Commit 34df5eb introduced regression to GetActiveUniformBlockiv
when querying one of the following properties:
GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS
GL_UNIFORM_BLOCK_ACTIVE_UNIFORM_INDICES
Implementation counted all uniforms in ubo directly while query should
check first if the uniform in quersion is _active_.
quersion=> question
Otherwise, looks good to me:
Reviewed-By: Martin Peres
Signed-off-by: Tapani Pälli
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90109
---
src/mesa/main/shader_query.cpp | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index b5f1d08..1428058 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -860,13 +860,23 @@ get_buffer_property(struct gl_shader_program *shProg,
*val = RESOURCE_UBO(res)->UniformBufferSize;
return 1;
case GL_NUM_ACTIVE_VARIABLES:
- *val = RESOURCE_UBO(res)->NumUniforms;
+ *val = 0;
+ for (unsigned i = 0; i < RESOURCE_UBO(res)->NumUniforms; i++) {
+const char *iname = RESOURCE_UBO(res)->Uniforms[i].IndexName;
+struct gl_program_resource *uni =
+ _mesa_program_resource_find_name(shProg, GL_UNIFORM, iname);
+if (!uni)
+ continue;
+(*val)++;
+ }
return 1;
case GL_ACTIVE_VARIABLES:
for (unsigned i = 0; i < RESOURCE_UBO(res)->NumUniforms; i++) {
const char *iname = RESOURCE_UBO(res)->Uniforms[i].IndexName;
struct gl_program_resource *uni =
_mesa_program_resource_find_name(shProg, GL_UNIFORM, iname);
+if (!uni)
+ continue;
*val++ =
_mesa_program_resource_index(shProg, uni);
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] glsl: add fallthrough comment on switch
On 21/04/15 08:37, Matt Turner wrote: On Mon, Apr 20, 2015 at 10:27 PM, Tapani Pälli wrote: Signed-off-by: Tapani Pälli --- src/glsl/linker.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index b6baa5d..08b4137 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -2571,6 +2571,7 @@ add_interface_variables(struct gl_shader_program *shProg, var->data.location != SYSTEM_VALUE_VERTEX_ID_ZERO_BASE && var->data.location != SYSTEM_VALUE_INSTANCE_ID) continue; The continue should be indented. Indeed, with the indent, this is: Reviewed-By: Martin Peres + /* FALLTHROUGH */ case ir_var_shader_in: if (programInterface != GL_PROGRAM_INPUT) continue; -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Mesa 10.6 release plan
Hi all, Here is the preliminary release schedule for Mesa 10.6. Personally I would love if we can go up to Mesa 11.0 (Spinal Tap anyone ?) although it might happen with the September release. May 15th 2015 - Feature freeze/Release candidate 1 May 22st 2015 - Release candidate 2 May 29th 2015 - Release candidate 3 June 5th 2015 - Release candidate 4/Mesa 10.6.0 This means that we have roughly four weeks to get new features in, and another four weeks to get all the serious bugs sorted out. Does this sound reasonable with the different teams ? If anyone has something special in mind please speak up. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] clover: compile all sources with c++11
On 20/04/15 21:34, EdB wrote: > Later we can remove the compat code. > --- > src/gallium/state_trackers/clover/Makefile.am | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/src/gallium/state_trackers/clover/Makefile.am > b/src/gallium/state_trackers/clover/Makefile.am > index 62c13fa..54b6fff 100644 > --- a/src/gallium/state_trackers/clover/Makefile.am > +++ b/src/gallium/state_trackers/clover/Makefile.am > @@ -3,6 +3,7 @@ AUTOMAKE_OPTIONS = subdir-objects > include Makefile.sources > > AM_CPPFLAGS = \ > + -std=c++11 \ Strictly speaking this should is not part of the pre-processor flags and one should use the CXXFLAGS variable(s). Barring any bugs things should work as is, so I'll leave the decision up-to you :-) > $(GALLIUM_PIPE_LOADER_DEFINES) \ > -DPIPE_SEARCH_DIR=\"$(libdir)/gallium-pipe\" \ > -I$(top_srcdir)/include \ > @@ -35,7 +36,6 @@ endif > noinst_LTLIBRARIES = libclover.la libcltgsi.la libclllvm.la > Now that all the code is built with c++11 does not make sense the have three separate static libraries ? Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] main: silence missing return value warning in array_index_of_resource()
On 16/04/15 21:52, Brian Paul wrote: > --- > src/mesa/main/shader_query.cpp | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp > index b5f1d08..ad936e5 100644 > --- a/src/mesa/main/shader_query.cpp > +++ b/src/mesa/main/shader_query.cpp > @@ -537,6 +537,7 @@ array_index_of_resource(struct gl_program_resource *res, >return get_matching_index(RESOURCE_VAR(res), name); > default: >assert(!"support for resource type not implemented"); > + return 0; Both callers assume that 0 is a valid value. Perhaps return -1 so that the error paths can kick in ? -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] main: remove __FUNCTION__ defined because it is obsolete
On 16/04/15 13:04, Predut, Marius wrote: > >> -Original Message- >> From: Emil Velikov [mailto:emil.l.veli...@gmail.com] >> Sent: Thursday, April 16, 2015 1:53 PM >> To: Predut, Marius >> Cc: ML mesa-dev >> Subject: Re: [Mesa-dev] [PATCH] main: remove __FUNCTION__ defined because it >> is obsolete >> >> Hi Marius, >> >> On 16 April 2015 at 11:36, Marius Predut wrote: >>> Consistently just use C99's __func__ everywhere. >>> No functional changes. >>> Apply this patch after "radeon: replace __FUNCTION__ with __func__" patch. >>> >> Tip: If you send the patches as series (git format-patch -2) this will >> implicitly provide/preserve the dependency chain. > > Ok thanks, in this case if u plan commit it , may be is better to adjust the > comment by removing last line. > Indeed that's the plan. Sorry for the delay - was pulled aside with other things, so I've just pushed these to master. Thanks again. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
On 14/04/15 22:19, Marek Olšák wrote:
> From: Marek Olšák
>
> ---
> src/gallium/include/state_tracker/opencl_interop.h | 42 ++
> src/gallium/state_trackers/clover/Makefile.sources | 1 +
> src/gallium/state_trackers/clover/core/event.hpp | 4 ++
> src/gallium/state_trackers/clover/core/interop.cpp | 66
> ++
> src/gallium/targets/opencl/opencl.sym | 1 +
> 5 files changed, 114 insertions(+)
> create mode 100644 src/gallium/include/state_tracker/opencl_interop.h
> create mode 100644 src/gallium/state_trackers/clover/core/interop.cpp
>
> diff --git a/src/gallium/include/state_tracker/opencl_interop.h
> b/src/gallium/include/state_tracker/opencl_interop.h
> new file mode 100644
> index 000..31a3bd3
> --- /dev/null
> +++ b/src/gallium/include/state_tracker/opencl_interop.h
> @@ -0,0 +1,42 @@
> +/**
> + *
> + * Copyright 2015 Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * 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 THE COPYRIGHT HOLDER(S) OR AUTHOR(S) 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 OPENCL_INTEROP_H
> +#define OPENCL_INTEROP_H
> +
> +/* dlsym these without the "_t" suffix. You should get the correct symbols
> + * if the OpenCL driver is loaded.
> + *
> + * All functions return non-zero on success.
> + */
> +
> +typedef int (*opencl_dri_event_add_ref_t)(intptr_t cl_event);
> +typedef int (*opencl_dri_event_release_t)(intptr_t cl_event);
> +typedef int (*opencl_dri_event_wait_t)(intptr_t cl_event, uint64_t timeout);
> +typedef struct pipe_fence_handle *(*opencl_dri_event_get_fence_t)(intptr_t
> cl_event);
> +
> +#endif /* OPENCL_INTEROP_H */
> diff --git a/src/gallium/state_trackers/clover/Makefile.sources
> b/src/gallium/state_trackers/clover/Makefile.sources
> index 5b3344c..4c2d0f3 100644
> --- a/src/gallium/state_trackers/clover/Makefile.sources
> +++ b/src/gallium/state_trackers/clover/Makefile.sources
> @@ -22,6 +22,7 @@ CPP_SOURCES := \
> core/event.hpp \
> core/format.cpp \
> core/format.hpp \
> + core/interop.cpp \
> core/kernel.cpp \
> core/kernel.hpp \
> core/memory.cpp \
> diff --git a/src/gallium/state_trackers/clover/core/event.hpp
> b/src/gallium/state_trackers/clover/core/event.hpp
> index 0e1359a..28f510f 100644
> --- a/src/gallium/state_trackers/clover/core/event.hpp
> +++ b/src/gallium/state_trackers/clover/core/event.hpp
> @@ -116,6 +116,10 @@ namespace clover {
>
>friend class command_queue;
>
> + struct pipe_fence_handle *get_fence() const {
> + return _fence;
> + }
> +
> private:
>virtual void fence(pipe_fence_handle *fence);
>action profile(command_queue &q, const action &action) const;
> diff --git a/src/gallium/state_trackers/clover/core/interop.cpp
> b/src/gallium/state_trackers/clover/core/interop.cpp
> new file mode 100644
> index 000..bb80cf5
> --- /dev/null
> +++ b/src/gallium/state_trackers/clover/core/interop.cpp
> @@ -0,0 +1,66 @@
> +//
> +// Copyright 2015 Advanced Micro Devices, Inc.
> +// All Rights Reserved.
> +//
> +// 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, sublicense,
> +// 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 shall be included in
> +// all copies or substantial portions of the Software.
> +//
> +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPR
Re: [Mesa-dev] [PATCH 1/3] main: silence missing return value warning in array_index_of_resource()
On 04/21/2015 04:01 PM, Emil Velikov wrote: On 16/04/15 21:52, Brian Paul wrote: --- src/mesa/main/shader_query.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index b5f1d08..ad936e5 100644 --- a/src/mesa/main/shader_query.cpp +++ b/src/mesa/main/shader_query.cpp @@ -537,6 +537,7 @@ array_index_of_resource(struct gl_program_resource *res, return get_matching_index(RESOURCE_VAR(res), name); default: assert(!"support for resource type not implemented"); + return 0; Both callers assume that 0 is a valid value. Perhaps return -1 so that the error paths can kick in ? Yes, -1 would be appropriate, 0 is a valid index. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
Only C warns about that. I didn't see any warning with C++.
Marek
On Tue, Apr 21, 2015 at 3:44 PM, Emil Velikov wrote:
> On 14/04/15 22:19, Marek Olšák wrote:
>> From: Marek Olšák
>>
>> ---
>> src/gallium/include/state_tracker/opencl_interop.h | 42 ++
>> src/gallium/state_trackers/clover/Makefile.sources | 1 +
>> src/gallium/state_trackers/clover/core/event.hpp | 4 ++
>> src/gallium/state_trackers/clover/core/interop.cpp | 66
>> ++
>> src/gallium/targets/opencl/opencl.sym | 1 +
>> 5 files changed, 114 insertions(+)
>> create mode 100644 src/gallium/include/state_tracker/opencl_interop.h
>> create mode 100644 src/gallium/state_trackers/clover/core/interop.cpp
>>
>> diff --git a/src/gallium/include/state_tracker/opencl_interop.h
>> b/src/gallium/include/state_tracker/opencl_interop.h
>> new file mode 100644
>> index 000..31a3bd3
>> --- /dev/null
>> +++ b/src/gallium/include/state_tracker/opencl_interop.h
>> @@ -0,0 +1,42 @@
>> +/**
>> + *
>> + * Copyright 2015 Advanced Micro Devices, Inc.
>> + * All Rights Reserved.
>> + *
>> + * 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 THE COPYRIGHT HOLDER(S) OR AUTHOR(S) 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 OPENCL_INTEROP_H
>> +#define OPENCL_INTEROP_H
>> +
>> +/* dlsym these without the "_t" suffix. You should get the correct symbols
>> + * if the OpenCL driver is loaded.
>> + *
>> + * All functions return non-zero on success.
>> + */
>> +
>> +typedef int (*opencl_dri_event_add_ref_t)(intptr_t cl_event);
>> +typedef int (*opencl_dri_event_release_t)(intptr_t cl_event);
>> +typedef int (*opencl_dri_event_wait_t)(intptr_t cl_event, uint64_t timeout);
>> +typedef struct pipe_fence_handle *(*opencl_dri_event_get_fence_t)(intptr_t
>> cl_event);
>> +
>> +#endif /* OPENCL_INTEROP_H */
>> diff --git a/src/gallium/state_trackers/clover/Makefile.sources
>> b/src/gallium/state_trackers/clover/Makefile.sources
>> index 5b3344c..4c2d0f3 100644
>> --- a/src/gallium/state_trackers/clover/Makefile.sources
>> +++ b/src/gallium/state_trackers/clover/Makefile.sources
>> @@ -22,6 +22,7 @@ CPP_SOURCES := \
>> core/event.hpp \
>> core/format.cpp \
>> core/format.hpp \
>> + core/interop.cpp \
>> core/kernel.cpp \
>> core/kernel.hpp \
>> core/memory.cpp \
>> diff --git a/src/gallium/state_trackers/clover/core/event.hpp
>> b/src/gallium/state_trackers/clover/core/event.hpp
>> index 0e1359a..28f510f 100644
>> --- a/src/gallium/state_trackers/clover/core/event.hpp
>> +++ b/src/gallium/state_trackers/clover/core/event.hpp
>> @@ -116,6 +116,10 @@ namespace clover {
>>
>>friend class command_queue;
>>
>> + struct pipe_fence_handle *get_fence() const {
>> + return _fence;
>> + }
>> +
>> private:
>>virtual void fence(pipe_fence_handle *fence);
>>action profile(command_queue &q, const action &action) const;
>> diff --git a/src/gallium/state_trackers/clover/core/interop.cpp
>> b/src/gallium/state_trackers/clover/core/interop.cpp
>> new file mode 100644
>> index 000..bb80cf5
>> --- /dev/null
>> +++ b/src/gallium/state_trackers/clover/core/interop.cpp
>> @@ -0,0 +1,66 @@
>> +//
>> +// Copyright 2015 Advanced Micro Devices, Inc.
>> +// All Rights Reserved.
>> +//
>> +// 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, sublicense,
>> +// and/or sell copies of the Software, and to permit persons to whom the
>> +// Software is furnished to do so, subject to the following condit
Re: [Mesa-dev] [PATCH] clover: compile all sources with c++11
On Tuesday 21 April 2015 12:54:28 Emil Velikov wrote: > On 20/04/15 21:34, EdB wrote: > > Later we can remove the compat code. > > --- > > > > src/gallium/state_trackers/clover/Makefile.am | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/src/gallium/state_trackers/clover/Makefile.am > > b/src/gallium/state_trackers/clover/Makefile.am > > index 62c13fa..54b6fff 100644 > > --- a/src/gallium/state_trackers/clover/Makefile.am > > +++ b/src/gallium/state_trackers/clover/Makefile.am > > @@ -3,6 +3,7 @@ AUTOMAKE_OPTIONS = subdir-objects > > > > include Makefile.sources > > > > AM_CPPFLAGS = \ > > > > + -std=c++11 \ > > Strictly speaking this should is not part of the pre-processor flags and > one should use the CXXFLAGS variable(s). Barring any bugs things should > work as is, so I'll leave the decision up-to you :-) Thanks, I'll change that in a v2 > > > $(GALLIUM_PIPE_LOADER_DEFINES) \ > > -DPIPE_SEARCH_DIR=\"$(libdir)/gallium-pipe\" \ > > -I$(top_srcdir)/include \ > > > > @@ -35,7 +36,6 @@ endif > > > > noinst_LTLIBRARIES = libclover.la libcltgsi.la libclllvm.la > > Now that all the code is built with c++11 does not make sense the have > three separate static libraries ? According to Francisco Jerez, it happen that some time llvm requiring exotic compilation flags and it would be preferable not to to contaminate the rest of clover build > > Thanks > Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/18] winsys/radeon: move radeon_winsys.h up one directory
On 20/04/15 22:23, Marek Olšák wrote: > From: Marek Olšák > > --- > .../auxiliary/target-helpers/inline_drm_helper.h | 6 +- > src/gallium/drivers/r300/r300_chipset.c| 2 +- > src/gallium/drivers/r300/r300_context.h| 1 - > src/gallium/drivers/r300/r300_screen.h | 2 +- > src/gallium/drivers/radeon/r600_pipe_common.h | 2 +- > src/gallium/drivers/radeon/radeon_uvd.c| 1 - > src/gallium/drivers/radeon/radeon_uvd.h| 2 +- > src/gallium/drivers/radeon/radeon_vce.c| 1 - > src/gallium/drivers/radeon/radeon_vce_40_2_2.c | 1 - > src/gallium/drivers/radeon/radeon_video.c | 1 - > src/gallium/drivers/radeon/radeon_video.h | 2 +- > src/gallium/drivers/radeonsi/si_pm4.h | 2 +- > src/gallium/targets/pipe-loader/pipe_r300.c| 2 +- > src/gallium/targets/pipe-loader/pipe_r600.c| 2 +- > src/gallium/targets/pipe-loader/pipe_radeonsi.c| 2 +- > src/gallium/winsys/radeon/drm/Makefile.sources | 2 +- > src/gallium/winsys/radeon/drm/radeon_drm_winsys.h | 2 +- > src/gallium/winsys/radeon/drm/radeon_winsys.h | 604 > - > src/gallium/winsys/radeon/radeon_winsys.h | 604 > + > 19 files changed, 618 insertions(+), 623 deletions(-) > delete mode 100644 src/gallium/winsys/radeon/drm/radeon_winsys.h > create mode 100644 src/gallium/winsys/radeon/radeon_winsys.h > Another solution would be to move it in gallium/drivers/radeon. This one might be better (and more consistent with every other driver) as the file defines the interface that the driver(s) require, while there can be many different providers for it - radeon/drm, amdgpu/drm or even amdgpu/sw :-) You might need to change two/three lines in the makefiles though :-\ -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] clover: compile all sources with c++11
Later we can remove the compat code According to Francisco Jerez, it happen that some time llvm requiring exotic compilation flags and it would be preferable not to to contaminate the rest of the clover build and keep the 3 statics libs --- src/gallium/state_trackers/clover/Makefile.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/gallium/state_trackers/clover/Makefile.am b/src/gallium/state_trackers/clover/Makefile.am index 62c13fa..f46d9ef 100644 --- a/src/gallium/state_trackers/clover/Makefile.am +++ b/src/gallium/state_trackers/clover/Makefile.am @@ -35,12 +35,13 @@ endif noinst_LTLIBRARIES = libclover.la libcltgsi.la libclllvm.la libcltgsi_la_CXXFLAGS = \ - -std=c++0x \ + -std=c++11 \ $(VISIBILITY_CXXFLAGS) libcltgsi_la_SOURCES = $(TGSI_SOURCES) libclllvm_la_CXXFLAGS = \ + -std=c++11 \ $(VISIBILITY_CXXFLAGS) \ $(LLVM_CXXFLAGS) \ $(DEFINES) \ -- 2.4.0.rc2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
Marek Olšák writes:
> From: Marek Olšák
>
Hi Marek, looks mostly OK to me, a few nits inline,
> ---
> src/gallium/include/state_tracker/opencl_interop.h | 42 ++
> src/gallium/state_trackers/clover/Makefile.sources | 1 +
> src/gallium/state_trackers/clover/core/event.hpp | 4 ++
> src/gallium/state_trackers/clover/core/interop.cpp | 66
> ++
> src/gallium/targets/opencl/opencl.sym | 1 +
> 5 files changed, 114 insertions(+)
> create mode 100644 src/gallium/include/state_tracker/opencl_interop.h
> create mode 100644 src/gallium/state_trackers/clover/core/interop.cpp
>
> diff --git a/src/gallium/include/state_tracker/opencl_interop.h
> b/src/gallium/include/state_tracker/opencl_interop.h
> new file mode 100644
> index 000..31a3bd3
> --- /dev/null
> +++ b/src/gallium/include/state_tracker/opencl_interop.h
> @@ -0,0 +1,42 @@
> +/**
> + *
> + * Copyright 2015 Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * 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 THE COPYRIGHT HOLDER(S) OR AUTHOR(S) 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 OPENCL_INTEROP_H
> +#define OPENCL_INTEROP_H
> +
> +/* dlsym these without the "_t" suffix. You should get the correct symbols
> + * if the OpenCL driver is loaded.
> + *
> + * All functions return non-zero on success.
> + */
> +
> +typedef int (*opencl_dri_event_add_ref_t)(intptr_t cl_event);
> +typedef int (*opencl_dri_event_release_t)(intptr_t cl_event);
> +typedef int (*opencl_dri_event_wait_t)(intptr_t cl_event, uint64_t timeout);
> +typedef struct pipe_fence_handle *(*opencl_dri_event_get_fence_t)(intptr_t
> cl_event);
> +
Just curious, is there any reason you use intptr_t here and elsewhere
rather than cl_event or void *? The former is an typedef of an opaque
pointer anyway. Using CL types would likely avoid some "ugly" casts
later on. And maybe bool as return type?
> +#endif /* OPENCL_INTEROP_H */
> diff --git a/src/gallium/state_trackers/clover/Makefile.sources
> b/src/gallium/state_trackers/clover/Makefile.sources
> index 5b3344c..4c2d0f3 100644
> --- a/src/gallium/state_trackers/clover/Makefile.sources
> +++ b/src/gallium/state_trackers/clover/Makefile.sources
> @@ -22,6 +22,7 @@ CPP_SOURCES := \
> core/event.hpp \
> core/format.cpp \
> core/format.hpp \
> + core/interop.cpp \
> core/kernel.cpp \
> core/kernel.hpp \
> core/memory.cpp \
> diff --git a/src/gallium/state_trackers/clover/core/event.hpp
> b/src/gallium/state_trackers/clover/core/event.hpp
> index 0e1359a..28f510f 100644
> --- a/src/gallium/state_trackers/clover/core/event.hpp
> +++ b/src/gallium/state_trackers/clover/core/event.hpp
> @@ -116,6 +116,10 @@ namespace clover {
>
>friend class command_queue;
>
> + struct pipe_fence_handle *get_fence() const {
The convention in the surrounding code is to name such accessors as the
object they access, how about "pipe_fence_handle *fence() const"?
> + return _fence;
> + }
> +
> private:
>virtual void fence(pipe_fence_handle *fence);
>action profile(command_queue &q, const action &action) const;
> diff --git a/src/gallium/state_trackers/clover/core/interop.cpp
> b/src/gallium/state_trackers/clover/core/interop.cpp
> new file mode 100644
> index 000..bb80cf5
> --- /dev/null
> +++ b/src/gallium/state_trackers/clover/core/interop.cpp
This probably belongs in clover/api/, as it's technically implementing
an API, clover/core/ is all about core data structures and such.
> @@ -0,0 +1,66 @@
> +//
> +// Copyright 2015 Advanced Micro Devices, Inc.
> +// All Rights Reserved.
> +//
> +// Permission is hereby granted, free of charge, to any person obtaining a
> +// copy of this software and associated docume
Re: [Mesa-dev] [PATCH v2] clover: compile all sources with c++11
EdB writes: > Later we can remove the compat code > > According to Francisco Jerez, it happen that some time llvm requiring exotic > compilation flags and it would be preferable not to to contaminate the rest of > the clover build and keep the 3 statics libs Not terribly informative... Without this paragraph, Reviewed-by: Francisco Jerez > --- > src/gallium/state_trackers/clover/Makefile.am | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/gallium/state_trackers/clover/Makefile.am > b/src/gallium/state_trackers/clover/Makefile.am > index 62c13fa..f46d9ef 100644 > --- a/src/gallium/state_trackers/clover/Makefile.am > +++ b/src/gallium/state_trackers/clover/Makefile.am > @@ -35,12 +35,13 @@ endif > noinst_LTLIBRARIES = libclover.la libcltgsi.la libclllvm.la > > libcltgsi_la_CXXFLAGS = \ > - -std=c++0x \ > + -std=c++11 \ > $(VISIBILITY_CXXFLAGS) > > libcltgsi_la_SOURCES = $(TGSI_SOURCES) > > libclllvm_la_CXXFLAGS = \ > + -std=c++11 \ > $(VISIBILITY_CXXFLAGS) \ > $(LLVM_CXXFLAGS) \ > $(DEFINES) \ > -- > 2.4.0.rc2 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] main: silence missing return value warning in array_index_of_resource()
On 04/21/2015 07:01 AM, Emil Velikov wrote: On 16/04/15 21:52, Brian Paul wrote: --- src/mesa/main/shader_query.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index b5f1d08..ad936e5 100644 --- a/src/mesa/main/shader_query.cpp +++ b/src/mesa/main/shader_query.cpp @@ -537,6 +537,7 @@ array_index_of_resource(struct gl_program_resource *res, return get_matching_index(RESOURCE_VAR(res), name); default: assert(!"support for resource type not implemented"); + return 0; Both callers assume that 0 is a valid value. Perhaps return -1 so that the error paths can kick in ? Yes, good point. I'll fix that before pushing. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/18] winsys/amdgpu: add a new winsys for the new kernel driver
Hi Marek, Must admit that the current "split"/location of the new winsys looks a bit strange. I'm thinking that if one places the new winsys alongside the radeon one (i.e. winsys/amdgpu/drm) things should still work and thus we'll result in shorter and cleaner patch. See below for more details. On 20/04/15 22:23, Marek Olšák wrote: > From: Marek Olšák > > --- > configure.ac | 5 + > src/gallium/Makefile.am | 1 + > src/gallium/drivers/r300/Automake.inc | 6 +- > src/gallium/drivers/r600/Automake.inc | 6 +- > src/gallium/drivers/radeonsi/Automake.inc | 6 +- > src/gallium/targets/pipe-loader/Makefile.am | 12 +- > src/gallium/winsys/radeon/amdgpu/Android.mk | 40 ++ > src/gallium/winsys/radeon/amdgpu/Makefile.am | 12 + > src/gallium/winsys/radeon/amdgpu/Makefile.sources | 8 + > src/gallium/winsys/radeon/amdgpu/amdgpu_bo.c | 643 > ++ > src/gallium/winsys/radeon/amdgpu/amdgpu_bo.h | 75 +++ > src/gallium/winsys/radeon/amdgpu/amdgpu_cs.c | 578 +++ > src/gallium/winsys/radeon/amdgpu/amdgpu_cs.h | 149 + > src/gallium/winsys/radeon/amdgpu/amdgpu_public.h | 14 + > src/gallium/winsys/radeon/amdgpu/amdgpu_winsys.c | 491 + > src/gallium/winsys/radeon/amdgpu/amdgpu_winsys.h | 80 +++ > src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 8 + > src/gallium/winsys/radeon/radeon_winsys.h | 4 + > 18 files changed, 2129 insertions(+), 9 deletions(-) > create mode 100644 src/gallium/winsys/radeon/amdgpu/Android.mk > create mode 100644 src/gallium/winsys/radeon/amdgpu/Makefile.am > create mode 100644 src/gallium/winsys/radeon/amdgpu/Makefile.sources > create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_bo.c > create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_bo.h > create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_cs.c > create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_cs.h > create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_public.h > create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_winsys.c > create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_winsys.h > > diff --git a/configure.ac b/configure.ac > index 095e23e..f22975f 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -68,6 +68,7 @@ AC_SUBST([OSMESA_VERSION]) > dnl Versions for external dependencies > LIBDRM_REQUIRED=2.4.38 > LIBDRM_RADEON_REQUIRED=2.4.56 > +LIBDRM_AMDGPU_REQUIRED=2.4.60 I guess this will need to be changed once the libdrm patches land ? > LIBDRM_INTEL_REQUIRED=2.4.60 > LIBDRM_NVVIEUX_REQUIRED=2.4.33 > LIBDRM_NOUVEAU_REQUIRED="2.4.33 libdrm >= 2.4.41" > @@ -2091,6 +2092,7 @@ if test -n "$with_gallium_drivers"; then > xr300) > HAVE_GALLIUM_R300=yes > PKG_CHECK_MODULES([RADEON], [libdrm_radeon >= > $LIBDRM_RADEON_REQUIRED]) > +PKG_CHECK_MODULES([AMDGPU], [libdrm_amdgpu >= > $LIBDRM_AMDGPU_REQUIRED]) > gallium_require_drm "Gallium R300" > gallium_require_drm_loader > gallium_require_llvm "Gallium R300" > @@ -2098,6 +2100,7 @@ if test -n "$with_gallium_drivers"; then > xr600) > HAVE_GALLIUM_R600=yes > PKG_CHECK_MODULES([RADEON], [libdrm_radeon >= > $LIBDRM_RADEON_REQUIRED]) > +PKG_CHECK_MODULES([AMDGPU], [libdrm_amdgpu >= > $LIBDRM_AMDGPU_REQUIRED]) We can drop the above two hunks. > gallium_require_drm "Gallium R600" > gallium_require_drm_loader > if test "x$enable_r600_llvm" = xyes -o "x$enable_opencl" = xyes; > then > @@ -2114,6 +2117,7 @@ if test -n "$with_gallium_drivers"; then > xradeonsi) > HAVE_GALLIUM_RADEONSI=yes > PKG_CHECK_MODULES([RADEON], [libdrm_radeon >= > $LIBDRM_RADEON_REQUIRED]) > +PKG_CHECK_MODULES([AMDGPU], [libdrm_amdgpu >= > $LIBDRM_AMDGPU_REQUIRED]) > gallium_require_drm "radeonsi" > gallium_require_drm_loader > radeon_llvm_check "radeonsi" > @@ -2384,6 +2388,7 @@ AC_CONFIG_FILES([Makefile > src/gallium/winsys/intel/drm/Makefile > src/gallium/winsys/nouveau/drm/Makefile > src/gallium/winsys/radeon/drm/Makefile > + src/gallium/winsys/radeon/amdgpu/Makefile > src/gallium/winsys/svga/drm/Makefile > src/gallium/winsys/sw/dri/Makefile > src/gallium/winsys/sw/kms-dri/Makefile > diff --git a/src/gallium/Makefile.am b/src/gallium/Makefile.am > index ede6e21..fa526d4 100644 > --- a/src/gallium/Makefile.am > +++ b/src/gallium/Makefile.am > @@ -63,6 +63,7 @@ endif > ## the radeon winsys - linked in by r300, r600 and radeonsi > if NEED_RADEON_DRM_WINSYS > SUBDIRS += winsys/radeon/drm > +SUBDIRS += winsys/radeon/amdgpu Move it to the if HAVE_GALLIUM_RADEONSI block ? It is the
Re: [Mesa-dev] [PATCH v2] clover: compile all sources with c++11
On 21/04/15 14:04, Francisco Jerez wrote: > EdB writes: > >> Later we can remove the compat code >> >> According to Francisco Jerez, it happen that some time llvm requiring exotic >> compilation flags and it would be preferable not to to contaminate the rest >> of >> the clover build and keep the 3 statics libs > > Not terribly informative... Without this paragraph, > Reviewed-by: Francisco Jerez > I don't know the gory details of LLVM_CXXFLAGS, like a few others, so it is quite informative actually :-) libclllvm aside, what is stopping us from merging libclover and libcltgsi ? Thanks Emil >> --- >> src/gallium/state_trackers/clover/Makefile.am | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/src/gallium/state_trackers/clover/Makefile.am >> b/src/gallium/state_trackers/clover/Makefile.am >> index 62c13fa..f46d9ef 100644 >> --- a/src/gallium/state_trackers/clover/Makefile.am >> +++ b/src/gallium/state_trackers/clover/Makefile.am >> @@ -35,12 +35,13 @@ endif >> noinst_LTLIBRARIES = libclover.la libcltgsi.la libclllvm.la >> >> libcltgsi_la_CXXFLAGS = \ >> --std=c++0x \ >> +-std=c++11 \ >> $(VISIBILITY_CXXFLAGS) >> >> libcltgsi_la_SOURCES = $(TGSI_SOURCES) >> >> libclllvm_la_CXXFLAGS = \ >> +-std=c++11 \ >> $(VISIBILITY_CXXFLAGS) \ >> $(LLVM_CXXFLAGS) \ >> $(DEFINES) \ >> -- >> 2.4.0.rc2 >> >> ___ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> >> >> ___ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] clover: compile all sources with c++11
Emil Velikov writes: > On 21/04/15 14:04, Francisco Jerez wrote: >> EdB writes: >> >>> Later we can remove the compat code >>> >>> According to Francisco Jerez, it happen that some time llvm requiring exotic >>> compilation flags and it would be preferable not to to contaminate the rest >>> of >>> the clover build and keep the 3 statics libs >> >> Not terribly informative... Without this paragraph, >> Reviewed-by: Francisco Jerez >> > I don't know the gory details of LLVM_CXXFLAGS, like a few others, so it > is quite informative actually :-) > Sorry, but I feel it's inaccurate, irrelevant to this commit, and not even grammatical. IMHO it creates more confusion than no comment at all. > libclllvm aside, what is stopping us from merging libclover and libcltgsi ? > No important reason other than symmetry. > Thanks > Emil > >>> --- >>> src/gallium/state_trackers/clover/Makefile.am | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/gallium/state_trackers/clover/Makefile.am >>> b/src/gallium/state_trackers/clover/Makefile.am >>> index 62c13fa..f46d9ef 100644 >>> --- a/src/gallium/state_trackers/clover/Makefile.am >>> +++ b/src/gallium/state_trackers/clover/Makefile.am >>> @@ -35,12 +35,13 @@ endif >>> noinst_LTLIBRARIES = libclover.la libcltgsi.la libclllvm.la >>> >>> libcltgsi_la_CXXFLAGS = \ >>> - -std=c++0x \ >>> + -std=c++11 \ >>> $(VISIBILITY_CXXFLAGS) >>> >>> libcltgsi_la_SOURCES = $(TGSI_SOURCES) >>> >>> libclllvm_la_CXXFLAGS = \ >>> + -std=c++11 \ >>> $(VISIBILITY_CXXFLAGS) \ >>> $(LLVM_CXXFLAGS) \ >>> $(DEFINES) \ >>> -- >>> 2.4.0.rc2 >>> >>> ___ >>> mesa-dev mailing list >>> mesa-dev@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev >>> >>> >>> ___ >>> mesa-dev mailing list >>> mesa-dev@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
Hmm pretty sure I've read somewhere (old version of GCC's manual?) that
such warnings are enabled by default and cannot be controlled by the
compiler (for C++ of course). I'm not an C++ expect so it could be that
the standard does not require declarations/prototypes for non static
functions ?
Seems like we're building clover without even a single -W flag - perhaps
we could add some (not with this patch of course) ?
-Emil
On 21/04/15 13:13, Marek Olšák wrote:
> Only C warns about that. I didn't see any warning with C++.
>
> Marek
>
> On Tue, Apr 21, 2015 at 3:44 PM, Emil Velikov
> wrote:
>> On 14/04/15 22:19, Marek Olšák wrote:
>>> From: Marek Olšák
>>>
>>> ---
>>> src/gallium/include/state_tracker/opencl_interop.h | 42 ++
>>> src/gallium/state_trackers/clover/Makefile.sources | 1 +
>>> src/gallium/state_trackers/clover/core/event.hpp | 4 ++
>>> src/gallium/state_trackers/clover/core/interop.cpp | 66
>>> ++
>>> src/gallium/targets/opencl/opencl.sym | 1 +
>>> 5 files changed, 114 insertions(+)
>>> create mode 100644 src/gallium/include/state_tracker/opencl_interop.h
>>> create mode 100644 src/gallium/state_trackers/clover/core/interop.cpp
>>>
>>> diff --git a/src/gallium/include/state_tracker/opencl_interop.h
>>> b/src/gallium/include/state_tracker/opencl_interop.h
>>> new file mode 100644
>>> index 000..31a3bd3
>>> --- /dev/null
>>> +++ b/src/gallium/include/state_tracker/opencl_interop.h
>>> @@ -0,0 +1,42 @@
>>> +/**
>>> + *
>>> + * Copyright 2015 Advanced Micro Devices, Inc.
>>> + * All Rights Reserved.
>>> + *
>>> + * 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 THE COPYRIGHT HOLDER(S) OR AUTHOR(S) 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 OPENCL_INTEROP_H
>>> +#define OPENCL_INTEROP_H
>>> +
>>> +/* dlsym these without the "_t" suffix. You should get the correct symbols
>>> + * if the OpenCL driver is loaded.
>>> + *
>>> + * All functions return non-zero on success.
>>> + */
>>> +
>>> +typedef int (*opencl_dri_event_add_ref_t)(intptr_t cl_event);
>>> +typedef int (*opencl_dri_event_release_t)(intptr_t cl_event);
>>> +typedef int (*opencl_dri_event_wait_t)(intptr_t cl_event, uint64_t
>>> timeout);
>>> +typedef struct pipe_fence_handle *(*opencl_dri_event_get_fence_t)(intptr_t
>>> cl_event);
>>> +
>>> +#endif /* OPENCL_INTEROP_H */
>>> diff --git a/src/gallium/state_trackers/clover/Makefile.sources
>>> b/src/gallium/state_trackers/clover/Makefile.sources
>>> index 5b3344c..4c2d0f3 100644
>>> --- a/src/gallium/state_trackers/clover/Makefile.sources
>>> +++ b/src/gallium/state_trackers/clover/Makefile.sources
>>> @@ -22,6 +22,7 @@ CPP_SOURCES := \
>>> core/event.hpp \
>>> core/format.cpp \
>>> core/format.hpp \
>>> + core/interop.cpp \
>>> core/kernel.cpp \
>>> core/kernel.hpp \
>>> core/memory.cpp \
>>> diff --git a/src/gallium/state_trackers/clover/core/event.hpp
>>> b/src/gallium/state_trackers/clover/core/event.hpp
>>> index 0e1359a..28f510f 100644
>>> --- a/src/gallium/state_trackers/clover/core/event.hpp
>>> +++ b/src/gallium/state_trackers/clover/core/event.hpp
>>> @@ -116,6 +116,10 @@ namespace clover {
>>>
>>>friend class command_queue;
>>>
>>> + struct pipe_fence_handle *get_fence() const {
>>> + return _fence;
>>> + }
>>> +
>>> private:
>>>virtual void fence(pipe_fence_handle *fence);
>>>action profile(command_queue &q, const action &action) const;
>>> diff --git a/src/gallium/state_trackers/clover/core/interop.cpp
>>> b/src/gallium/state_trackers/clover/core/interop.cpp
>>> new file mode 100644
>>> index 000..bb80cf5
>>> --- /dev/null
>>> +++ b/src/gallium/state_trackers/clover/core/interop.cp
Re: [Mesa-dev] [PATCH 2/3] glsl: add fallthrough comment on switch
On Mon, Apr 20, 2015 at 10:37 PM, Matt Turner wrote: > On Mon, Apr 20, 2015 at 10:27 PM, Tapani Pälli wrote: >> Signed-off-by: Tapani Pälli >> --- >> src/glsl/linker.cpp | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp >> index b6baa5d..08b4137 100644 >> --- a/src/glsl/linker.cpp >> +++ b/src/glsl/linker.cpp >> @@ -2571,6 +2571,7 @@ add_interface_variables(struct gl_shader_program >> *shProg, >> var->data.location != SYSTEM_VALUE_VERTEX_ID_ZERO_BASE && >> var->data.location != SYSTEM_VALUE_INSTANCE_ID) >> continue; > > The continue should be indented. > >> + /* FALLTHROUGH */ In what was committed, you indented both the continue and the /* FALLTHROUGH */ ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Initial amdgpu driver release
Hi Alex, On 20 April 2015 at 23:33, Alex Deucher wrote: > I'm pleased to announce the initial release of the new amdgpu driver. > This is a partial replacement for the radeon driver for newer AMD > asics. A number of components are still shared. Here is a comparison > of the radeon and amdgpu stacks: > > 1. radeon stack > kernel driver: radeon.ko > libdrm: libdrm_radeon > mesa: radeon, r200, r300, r600, radeonsi > ddx: xf86-video-ati > > 2. amdgpu stack > kernel driver: amdgpu.ko > libdrm: libdrm_amdgpu > mesa: radeonsi > ddx: xf86-video-amdgpu > > Older asics will continue to be supported by the radeon stack; new > asics will be supported by the amdgpu stack. CI (Sea Islands) asics > have support in both driver stacks, but this is purely for testing > purposes. CI parts are officially supported in the radeon stack. > Support for CI on the amdgpu stack is determined by a config option in > the kernel. CI support is not enabled by default for amdgpu. > > Most of our focus has been on Carrizo support, so there are some gaps > in the dGPU support for Tonga and Iceland, notably power management. > Those gaps will be filled in eventually. > > Also included in this code base are full register headers for just > about every block on the asics. > > Barring the gaps mentioned above, the driver stack is functionally on > par with radeon including: > - OpenGL 3.3 support using the radeonsi mesa driver > - Video decode support using UVD > - Video encode support using VCE > > The code can be found in the amdgpu branches of the following git trees. > xf86-video-amdgpu: > http://cgit.freedesktop.org/~agd5f/xf86-video-amdgpu/log/?h=amdgpu > libdrm: > http://cgit.freedesktop.org/~agd5f/drm/log/?h=amdgpu > kernel: > http://cgit.freedesktop.org/~agd5f/linux/log/?h=amdgpu > mesa: > http://cgit.freedesktop.org/~mareko/mesa/log/?h=amdgpu > > To test the new driver stack you will need to specify a device section > in your xorg.conf with the driver set to amdgpu rather than radeon. > > Please review! > Do you plan to sending out the libdrm patches to the ML ? I have a few comments that you might be interested in :-) Cheers, Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] glsl: add fallthrough comment on switch
On 04/21/2015 06:19 PM, Matt Turner wrote: On Mon, Apr 20, 2015 at 10:37 PM, Matt Turner wrote: On Mon, Apr 20, 2015 at 10:27 PM, Tapani Pälli wrote: Signed-off-by: Tapani Pälli --- src/glsl/linker.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index b6baa5d..08b4137 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -2571,6 +2571,7 @@ add_interface_variables(struct gl_shader_program *shProg, var->data.location != SYSTEM_VALUE_VERTEX_ID_ZERO_BASE && var->data.location != SYSTEM_VALUE_INSTANCE_ID) continue; The continue should be indented. + /* FALLTHROUGH */ In what was committed, you indented both the continue and the /* FALLTHROUGH */ Yep that seems to be the case, sorry about that. // Tapani ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Initial amdgpu driver release
On Tue, Apr 21, 2015 at 11:56 AM, Emil Velikov wrote: > Hi Alex, > > On 20 April 2015 at 23:33, Alex Deucher wrote: >> I'm pleased to announce the initial release of the new amdgpu driver. >> This is a partial replacement for the radeon driver for newer AMD >> asics. A number of components are still shared. Here is a comparison >> of the radeon and amdgpu stacks: >> >> 1. radeon stack >> kernel driver: radeon.ko >> libdrm: libdrm_radeon >> mesa: radeon, r200, r300, r600, radeonsi >> ddx: xf86-video-ati >> >> 2. amdgpu stack >> kernel driver: amdgpu.ko >> libdrm: libdrm_amdgpu >> mesa: radeonsi >> ddx: xf86-video-amdgpu >> >> Older asics will continue to be supported by the radeon stack; new >> asics will be supported by the amdgpu stack. CI (Sea Islands) asics >> have support in both driver stacks, but this is purely for testing >> purposes. CI parts are officially supported in the radeon stack. >> Support for CI on the amdgpu stack is determined by a config option in >> the kernel. CI support is not enabled by default for amdgpu. >> >> Most of our focus has been on Carrizo support, so there are some gaps >> in the dGPU support for Tonga and Iceland, notably power management. >> Those gaps will be filled in eventually. >> >> Also included in this code base are full register headers for just >> about every block on the asics. >> >> Barring the gaps mentioned above, the driver stack is functionally on >> par with radeon including: >> - OpenGL 3.3 support using the radeonsi mesa driver >> - Video decode support using UVD >> - Video encode support using VCE >> >> The code can be found in the amdgpu branches of the following git trees. >> xf86-video-amdgpu: >> http://cgit.freedesktop.org/~agd5f/xf86-video-amdgpu/log/?h=amdgpu >> libdrm: >> http://cgit.freedesktop.org/~agd5f/drm/log/?h=amdgpu >> kernel: >> http://cgit.freedesktop.org/~agd5f/linux/log/?h=amdgpu >> mesa: >> http://cgit.freedesktop.org/~mareko/mesa/log/?h=amdgpu >> >> To test the new driver stack you will need to specify a device section >> in your xorg.conf with the driver set to amdgpu rather than radeon. >> >> Please review! >> > Do you plan to sending out the libdrm patches to the ML ? I have a few > comments that you might be interested in :-) Yeah, just sent them out now. Ran out of time last night. Some of the patches in the kernel and the ddx patch are really big so probably too big for the ML. Alex > > Cheers, > Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 04/23] i965: Make the brw_inst helpers take a device_info instead of a context
On Fri, Apr 17, 2015 at 7:11 PM, Jason Ekstrand wrote:
> ---
> src/mesa/drivers/dri/i965/brw_clip_line.c| 21 +-
> src/mesa/drivers/dri/i965/brw_clip_tri.c | 45 +-
> src/mesa/drivers/dri/i965/brw_clip_unfilled.c| 26 +-
> src/mesa/drivers/dri/i965/brw_clip_util.c|2 +-
> src/mesa/drivers/dri/i965/brw_disasm.c | 472 +-
> src/mesa/drivers/dri/i965/brw_eu.c | 26 +-
> src/mesa/drivers/dri/i965/brw_eu.h | 10 +-
> src/mesa/drivers/dri/i965/brw_eu_compact.c | 123 +--
> src/mesa/drivers/dri/i965/brw_eu_emit.c | 1001
> +++---
> src/mesa/drivers/dri/i965/brw_ff_gs_emit.c |7 +-
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 54 +-
> src/mesa/drivers/dri/i965/brw_inst.h | 140 +--
> src/mesa/drivers/dri/i965/brw_reg.h |4 +-
> src/mesa/drivers/dri/i965/brw_sf_emit.c |7 +-
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 28 +-
> src/mesa/drivers/dri/i965/test_eu_compact.c | 36 +-
> 16 files changed, 1006 insertions(+), 996 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_line.c
> b/src/mesa/drivers/dri/i965/brw_clip_line.c
> index 3b8af58..070adba 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_line.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_line.c
> @@ -158,9 +158,9 @@ static void clip_and_emit_line( struct brw_clip_compile
> *c )
> if (brw->has_negative_rhw_bug) {
>brw_AND(p, brw_null_reg(), get_element_ud(c->reg.R0, 2),
>brw_imm_ud(1<<20));
> - brw_inst_set_cond_modifier(brw, brw_last_inst, BRW_CONDITIONAL_NZ);
> + brw_inst_set_cond_modifier(p->devinfo, brw_last_inst,
> BRW_CONDITIONAL_NZ);
>brw_OR(p, c->reg.planemask, c->reg.planemask, brw_imm_ud(0x3f));
> - brw_inst_set_pred_control(brw, brw_last_inst, BRW_PREDICATE_NORMAL);
> + brw_inst_set_pred_control(p->devinfo, brw_last_inst,
> BRW_PREDICATE_NORMAL);
> }
>
> /* Set the initial vertex source mask: The first 6 planes are the bounds
I don't know how reasonable this suggestion is until I read further in
the series, but I'd rather pass 'p' to the brw_inst functions instead
of p->devinfo.
That would match better with brw_$INST functions and would shorten the
argument name instead of more than tripling it.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 07/23] i965: Remove the context field from brw_compiler
On Fri, Apr 17, 2015 at 7:11 PM, Jason Ekstrand wrote:
> ---
> src/mesa/drivers/dri/i965/brw_clip.c | 2 +-
> src/mesa/drivers/dri/i965/brw_clip_line.c| 13 ++---
> src/mesa/drivers/dri/i965/brw_clip_tri.c | 4 ++--
> src/mesa/drivers/dri/i965/brw_clip_util.c| 12
> src/mesa/drivers/dri/i965/brw_eu.c | 18 +++---
> src/mesa/drivers/dri/i965/brw_eu.h | 5 ++---
> src/mesa/drivers/dri/i965/brw_eu_compact.c | 4 ++--
> src/mesa/drivers/dri/i965/brw_eu_emit.c | 8
> src/mesa/drivers/dri/i965/brw_ff_gs.c| 2 +-
> src/mesa/drivers/dri/i965/brw_ff_gs_emit.c | 12 +++-
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 2 +-
> src/mesa/drivers/dri/i965/brw_sf.c | 2 +-
> src/mesa/drivers/dri/i965/brw_sf_emit.c | 6 ++
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 2 +-
> src/mesa/drivers/dri/i965/test_eu_compact.c | 13 +
> 15 files changed, 42 insertions(+), 63 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip.c
> b/src/mesa/drivers/dri/i965/brw_clip.c
> index 07b10a2..3aa79b5 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip.c
> @@ -62,7 +62,7 @@ static void compile_clip_prog( struct brw_context *brw,
>
> /* Begin the compilation:
> */
> - brw_init_compile(brw, &c.func, mem_ctx);
> + brw_init_compile(brw->intelScreen->devinfo, &c.func, mem_ctx);
>
> c.func.single_program_flow = 1;
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_line.c
> b/src/mesa/drivers/dri/i965/brw_clip_line.c
> index 070adba..395cd2f 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_line.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_line.c
> @@ -45,7 +45,7 @@
>
> static void brw_clip_line_alloc_regs( struct brw_clip_compile *c )
> {
> - struct brw_context *brw = c->func.brw;
> + const struct brw_device_info *devinfo = c->func.devinfo;
> GLuint i = 0,j;
>
> /* Register usage is static, precompute here:
> @@ -89,7 +89,7 @@ static void brw_clip_line_alloc_regs( struct
> brw_clip_compile *c )
> c->reg.clipdistance_offset = retype(brw_vec1_grf(i, 1),
> BRW_REGISTER_TYPE_W);
> i++;
>
> - if (brw->gen == 5) {
> + if (devinfo->gen == 5) {
>c->reg.ff_sync = retype(brw_vec1_grf(i, 0), BRW_REGISTER_TYPE_UD);
>i++;
> }
> @@ -129,7 +129,6 @@ static void brw_clip_line_alloc_regs( struct
> brw_clip_compile *c )
> static void clip_and_emit_line( struct brw_clip_compile *c )
> {
> struct brw_compile *p = &c->func;
> - struct brw_context *brw = p->brw;
> struct brw_indirect vtx0 = brw_indirect(0, 0);
> struct brw_indirect vtx1 = brw_indirect(1, 0);
> struct brw_indirect newvtx0 = brw_indirect(2, 0);
> @@ -155,7 +154,7 @@ static void clip_and_emit_line( struct brw_clip_compile
> *c )
> brw_clip_init_clipmask(c);
>
> /* -ve rhw workaround */
> - if (brw->has_negative_rhw_bug) {
> + if (p->devinfo->has_negative_rhw_bug) {
>brw_AND(p, brw_null_reg(), get_element_ud(c->reg.R0, 2),
>brw_imm_ud(1<<20));
>brw_inst_set_cond_modifier(p->devinfo, brw_last_inst,
> BRW_CONDITIONAL_NZ);
> @@ -213,7 +212,7 @@ static void clip_and_emit_line( struct brw_clip_compile
> *c )
>* Both can be negative on GM965/G965 due to RHW workaround
>* if so, this object should be rejected.
>*/
> - if (brw->has_negative_rhw_bug) {
> + if (p->devinfo->has_negative_rhw_bug) {
> brw_CMP(p, vec1(brw_null_reg()), BRW_CONDITIONAL_LE,
> c->reg.dp0, brw_imm_f(0.0));
> brw_IF(p, BRW_EXECUTE_1);
> {
> @@ -239,7 +238,7 @@ static void clip_and_emit_line( struct brw_clip_compile
> *c )
>
> /* If both are positive, do nothing */
> /* Only on GM965/G965 */
> - if (brw->has_negative_rhw_bug) {
> + if (p->devinfo->has_negative_rhw_bug) {
> brw_CMP(p, vec1(brw_null_reg()), BRW_CONDITIONAL_L,
> c->reg.dp0, brw_imm_f(0.0));
> brw_IF(p, BRW_EXECUTE_1);
> }
> @@ -255,7 +254,7 @@ static void clip_and_emit_line( struct brw_clip_compile
> *c )
> BRW_PREDICATE_NORMAL);
> }
>
> - if (brw->has_negative_rhw_bug) {
> + if (p->devinfo->has_negative_rhw_bug) {
> brw_ENDIF(p);
> }
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_tri.c
> b/src/mesa/drivers/dri/i965/brw_clip_tri.c
> index d4babc9..ad5e588 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_tri.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_tri.c
> @@ -50,7 +50,7 @@ static void release_tmps( struct brw_clip_compile *c )
> void brw_clip_tri_alloc_regs( struct brw_clip_compile *c,
>
Re: [Mesa-dev] [PATCH 08/23] i965/fs: Remove the GL context from the generator
On Fri, Apr 17, 2015 at 7:11 PM, Jason Ekstrand wrote:
> ---
> src/mesa/drivers/dri/i965/brw_fs.h | 1 -
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 11 +--
> 2 files changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index 32063f0..fa2a028 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -641,7 +641,6 @@ private:
> bool patch_discard_jumps_to_fb_writes();
>
> struct brw_context *brw;
> - struct gl_context *ctx;
>
> struct brw_compile *p;
> const void * const key;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 58fdc40..b9ec248 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -136,8 +136,6 @@ fs_generator::fs_generator(struct brw_context *brw,
> runtime_check_aads_emit(runtime_check_aads_emit), debug_flag(false),
> stage_abbrev(stage_abbrev), mem_ctx(mem_ctx)
> {
> - ctx = &brw->ctx;
> -
> p = rzalloc(mem_ctx, struct brw_compile);
> brw_init_compile(brw->intelScreen->devinfo, p, mem_ctx);
> }
> @@ -2084,14 +2082,7 @@ fs_generator::generate_code(const cfg_t *cfg, int
> dispatch_width)
> break;
>
>default:
> -if (inst->opcode < (int) ARRAY_SIZE(opcode_descs)) {
> - _mesa_problem(ctx, "Unsupported opcode `%s' in %s",
> - opcode_descs[inst->opcode].name, stage_abbrev);
> -} else {
> - _mesa_problem(ctx, "Unsupported opcode %d in %s", inst->opcode,
> - stage_abbrev);
> -}
> -abort();
> + assert("!Unsupported opcode");
unreachable()
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 08/23] i965/fs: Remove the GL context from the generator
On Tue, Apr 21, 2015 at 1:14 PM, Matt Turner wrote:
> On Fri, Apr 17, 2015 at 7:11 PM, Jason Ekstrand wrote:
>> ---
>> src/mesa/drivers/dri/i965/brw_fs.h | 1 -
>> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 11 +--
>> 2 files changed, 1 insertion(+), 11 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
>> b/src/mesa/drivers/dri/i965/brw_fs.h
>> index 32063f0..fa2a028 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> @@ -641,7 +641,6 @@ private:
>> bool patch_discard_jumps_to_fb_writes();
>>
>> struct brw_context *brw;
>> - struct gl_context *ctx;
>>
>> struct brw_compile *p;
>> const void * const key;
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> index 58fdc40..b9ec248 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> @@ -136,8 +136,6 @@ fs_generator::fs_generator(struct brw_context *brw,
>> runtime_check_aads_emit(runtime_check_aads_emit), debug_flag(false),
>> stage_abbrev(stage_abbrev), mem_ctx(mem_ctx)
>> {
>> - ctx = &brw->ctx;
>> -
>> p = rzalloc(mem_ctx, struct brw_compile);
>> brw_init_compile(brw->intelScreen->devinfo, p, mem_ctx);
>> }
>> @@ -2084,14 +2082,7 @@ fs_generator::generate_code(const cfg_t *cfg, int
>> dispatch_width)
>> break;
>>
>>default:
>> -if (inst->opcode < (int) ARRAY_SIZE(opcode_descs)) {
>> - _mesa_problem(ctx, "Unsupported opcode `%s' in %s",
>> - opcode_descs[inst->opcode].name, stage_abbrev);
>> -} else {
>> - _mesa_problem(ctx, "Unsupported opcode %d in %s", inst->opcode,
>> - stage_abbrev);
>> -}
>> -abort();
>> + assert("!Unsupported opcode");
>
> unreachable()
Or at the very least assert(!"...");
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 13/23] i965/vec4: Add a devinfo field to the generator and use it for gen checks
On Fri, Apr 17, 2015 at 7:11 PM, Jason Ekstrand wrote:
> ---
> src/mesa/drivers/dri/i965/brw_vec4.h | 1 +
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 87
> +++-
> 2 files changed, 42 insertions(+), 46 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h
> b/src/mesa/drivers/dri/i965/brw_vec4.h
> index a0ee2cc..cafbb64 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> @@ -514,6 +514,7 @@ private:
>struct brw_reg surf_index);
>
> struct brw_context *brw;
> + const struct brw_device_info *devinfo;
>
> struct brw_compile *p;
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index 6e3a6a5..84ed99e 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -142,7 +142,8 @@ vec4_generator::vec4_generator(struct brw_context *brw,
> bool debug_flag,
> const char *stage_name,
> const char *stage_abbrev)
> - : brw(brw), shader_prog(shader_prog), prog(prog), prog_data(prog_data),
> + : brw(brw), devinfo(brw->intelScreen->devinfo),
> + shader_prog(shader_prog), prog(prog), prog_data(prog_data),
> mem_ctx(mem_ctx), stage_name(stage_name), stage_abbrev(stage_abbrev),
> debug_flag(debug_flag)
> {
> @@ -235,7 +236,7 @@ vec4_generator::generate_tex(vec4_instruction *inst,
> {
> int msg_type = -1;
>
> - if (brw->gen >= 5) {
> + if (devinfo->gen >= 5) {
>switch (inst->opcode) {
>case SHADER_OPCODE_TEX:
>case SHADER_OPCODE_TXL:
> @@ -248,7 +249,7 @@ vec4_generator::generate_tex(vec4_instruction *inst,
>case SHADER_OPCODE_TXD:
> if (inst->shadow_compare) {
> /* Gen7.5+. Otherwise, lowered by
> brw_lower_texture_gradients(). */
> -assert(brw->gen >= 8 || brw->is_haswell);
> +assert(devinfo->gen >= 8 || devinfo->is_haswell);
> msg_type = HSW_SAMPLER_MESSAGE_SAMPLE_DERIV_COMPARE;
> } else {
> msg_type = GEN5_SAMPLER_MESSAGE_SAMPLE_DERIVS;
> @@ -258,13 +259,13 @@ vec4_generator::generate_tex(vec4_instruction *inst,
> msg_type = GEN5_SAMPLER_MESSAGE_SAMPLE_LD;
> break;
>case SHADER_OPCODE_TXF_CMS:
> - if (brw->gen >= 7)
> + if (devinfo->gen >= 7)
> msg_type = GEN7_SAMPLER_MESSAGE_SAMPLE_LD2DMS;
> else
> msg_type = GEN5_SAMPLER_MESSAGE_SAMPLE_LD;
> break;
>case SHADER_OPCODE_TXF_MCS:
> - assert(brw->gen >= 7);
> + assert(devinfo->gen >= 7);
> msg_type = GEN7_SAMPLER_MESSAGE_SAMPLE_LD_MCS;
> break;
>case SHADER_OPCODE_TXS:
> @@ -326,7 +327,7 @@ vec4_generator::generate_tex(vec4_instruction *inst,
> * use an implied move from g0 to the first message register.
> */
> if (inst->header_present) {
> - if (brw->gen < 6 && !inst->offset) {
> + if (devinfo->gen < 6 && !inst->offset) {
> /* Set up an implied move from g0 to the MRF. */
> src = brw_vec8_grf(0, 0);
>} else {
> @@ -345,7 +346,7 @@ vec4_generator::generate_tex(vec4_instruction *inst,
> /* Set the texel offset bits in DWord 2. */
> dw2 = inst->offset;
>
> - if (brw->gen >= 9)
> + if (devinfo->gen >= 9)
> /* SKL+ overloads BRW_SAMPLER_SIMD_MODE_SIMD4X2 to also do
> SIMD8D,
> * based on bit 22 in the header.
> */
> @@ -504,7 +505,7 @@ vec4_generator::generate_gs_thread_end(vec4_instruction
> *inst)
> inst->base_mrf, /* starting mrf reg nr */
> src,
> BRW_URB_WRITE_EOT | inst->urb_write_flags,
> - brw->gen >= 8 ? 2 : 1,/* message len */
> + devinfo->gen >= 8 ? 2 : 1,/* message len */
> 0, /* response len */
> 0, /* urb destination offset */
> BRW_URB_SWIZZLE_INTERLEAVE);
> @@ -536,7 +537,7 @@ vec4_generator::generate_gs_set_write_offset(struct
> brw_reg dst,
> brw_push_insn_state(p);
> brw_set_default_access_mode(p, BRW_ALIGN_1);
> brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> - assert(brw->gen >= 7 &&
> + assert(devinfo->gen >= 7 &&
>src1.file == BRW_IMMEDIATE_VALUE &&
>src1.type == BRW_REGISTER_TYPE_UD &&
>src1.dw1.ud <= USHRT_MAX);
> @@ -553,7 +554,7 @@ vec4_generator::generate_gs_set_vertex_count(struct
> brw_reg dst,
> brw_push_insn_state(p);
> brw_set_default_mask_control(p, BRW_MASK_DISABLE);
>
> - if (brw->gen >= 8) {
> + if (devinfo->gen >= 8) {
>/* Move the vertex count into the second MRF for the EOT write. */
>brw_MOV(p
Re: [Mesa-dev] [PATCH 14/23] i965: Remove remaining uses of ctx->Const.UniformBooleanTrue in visitors
On Fri, Apr 17, 2015 at 7:11 PM, Jason Ekstrand wrote:
> Since commit 2881b123, we have used 0/~0 for representing booleans on all
> gens. However, we still had a bunch of places in the visitor code where we
> were still referring to ctx->Const.UniformBooleanTrue. Since this is
> always ~0, we can just remove them.
>
> Reviewed-by: Kenneth Graunke
> ---
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 4 +---
> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 14 +-
> 2 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index a99b12ef..8a43e3a 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -2641,9 +2641,7 @@ fs_visitor::visit(ir_constant *ir)
> emit(MOV(dst_reg, fs_reg(ir->value.i[i])));
> break;
> case GLSL_TYPE_BOOL:
> -emit(MOV(dst_reg,
> - fs_reg(ir->value.b[i] != 0 ?
> (int)ctx->Const.UniformBooleanTrue
> -: 0)));
> +emit(MOV(dst_reg, fs_reg(ir->value.b[i] != 0 ? ~0 : 0)));
> break;
> default:
> unreachable("Non-float/uint/int/bool constant");
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 20e6109..32eb75b 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -1635,7 +1635,7 @@ vec4_visitor::visit(ir_expression *ir)
> ir->operands[1]->type->is_vector()) {
> emit(CMP(dst_null_d(), op[0], op[1], BRW_CONDITIONAL_Z));
> emit(MOV(result_dst, src_reg(0)));
> - inst = emit(MOV(result_dst,
> src_reg((int)ctx->Const.UniformBooleanTrue)));
> + inst = emit(MOV(result_dst, src_reg(~0 /* boolean true */)));
I don't think we need the comment.
> inst->predicate = BRW_PREDICATE_ALIGN16_ALL4H;
>} else {
> emit(CMP(result_dst, op[0], op[1], BRW_CONDITIONAL_Z));
> @@ -1653,7 +1653,7 @@ vec4_visitor::visit(ir_expression *ir)
> emit(CMP(dst_null_d(), op[0], op[1], BRW_CONDITIONAL_NZ));
>
> emit(MOV(result_dst, src_reg(0)));
> - inst = emit(MOV(result_dst,
> src_reg((int)ctx->Const.UniformBooleanTrue)));
> + inst = emit(MOV(result_dst, src_reg(~0 /* boolean true */)));
Here as well.
> inst->predicate = BRW_PREDICATE_ALIGN16_ANY4H;
>} else {
> emit(CMP(result_dst, op[0], op[1], BRW_CONDITIONAL_NZ));
> @@ -1667,7 +1667,7 @@ vec4_visitor::visit(ir_expression *ir)
>emit(CMP(dst_null_d(), op[0], src_reg(0), BRW_CONDITIONAL_NZ));
>emit(MOV(result_dst, src_reg(0)));
>
> - inst = emit(MOV(result_dst,
> src_reg((int)ctx->Const.UniformBooleanTrue)));
> + inst = emit(MOV(result_dst, src_reg(~0 /* boolean true */)));
Here as well.
>inst->predicate = BRW_PREDICATE_ALIGN16_ANY4H;
>break;
>
> @@ -1862,9 +1862,7 @@ vec4_visitor::visit(ir_expression *ir)
> const_offset % 16 / 4,
> const_offset % 16 / 4);
>
> - /* UBO bools are any nonzero int. We need to convert them to use the
> - * value of true stored in ctx->Const.UniformBooleanTrue.
> - */
> + /* UBO bools are any nonzero int. We need to convert them to 0/~0. */
>if (ir->type->base_type == GLSL_TYPE_BOOL) {
> emit(CMP(result_dst, packed_consts, src_reg(0u),
>BRW_CONDITIONAL_NZ));
> @@ -2370,9 +2368,7 @@ vec4_visitor::emit_constant_values(dst_reg *dst,
> ir_constant *ir)
> emit(MOV(*dst, src_reg(ir->value.u[i])));
> break;
>case GLSL_TYPE_BOOL:
> - emit(MOV(*dst,
> - src_reg(ir->value.b[i] != 0 ?
> (int)ctx->Const.UniformBooleanTrue
> - : 0)));
> + emit(MOV(*dst, src_reg(ir->value.b[i] != 0 ? ~0 : 0)));
> break;
>default:
> unreachable("Non-float/uint/int/bool constant");
> --
> 2.3.5
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 04/23] i965: Make the brw_inst helpers take a device_info instead of a context
On Tue, Apr 21, 2015 at 9:59 AM, Matt Turner wrote: > I don't know how reasonable this suggestion is until I read further in > the series, but I'd rather pass 'p' to the brw_inst functions instead > of p->devinfo. > > That would match better with brw_$INST functions and would shorten the > argument name instead of more than tripling it. Okay, yeah. Everything gets churned between here and the end of the series. Ultimately the change seems to be s/brw/devinfo/... I think that's probably fine. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/10] gallium, clover: add OpenCL interoperability support for CL events
On Tue, Apr 21, 2015 at 5:45 PM, Emil Velikov wrote: > Hmm pretty sure I've read somewhere (old version of GCC's manual?) that > such warnings are enabled by default and cannot be controlled by the > compiler (for C++ of course). I'm not an C++ expect so it could be that > the standard does not require declarations/prototypes for non static > functions ? A prototype is only required by C++ when a function is called. Functions don't need prototypes for themselves. Only their call sites need them. > > Seems like we're building clover without even a single -W flag - perhaps > we could add some (not with this patch of course) ? Yes, I agree. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3] glx/dri3: Use a separate xcb connection for Present events
Le 20/04/2015 07:08, Axel Davy a écrit : Le 20/04/2015 03:58, Michel Dänzer a écrit : On 19.04.2015 06:17, Axel Davy wrote: v3: put the xcb_connection in dri3_screen, instead of dri3_context What's the rationale for that? Couldn't that result in the same xcb_connection being used in several threads again (for several GLX contexts using the same screen)? One way is to have one xcb_connection to listen to present events, which can be in dri3_screen, while we could have a second xcb_connection in dri3_context, that would be used to send the present requests. ___ The GLX spec seems to allow having several threads rendering to the same drawable, and suggests only one thread does the glxSwapBuffers call. Thus I think I have to do similar to what is done in gallium nine that is: . A mutex to prevent the drawable structure fields related to Present . A system with mutexes to make only one thread dispatch present events at a time . One connection to dispatch the present events, the other one for sending them. (That way you can send while one thread is waiting to dispatch). Axel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] Fix life cycle issue of egl context.
From: Enpeng Xu
Currently, a simple egl test case will fail like this:
eglMakeCurrent(dpy, surf, surf, ctx);
eglDestroyContext(dpy, ctx);
ctx2 = eglGetCurrentContext(); // ctx2 will be EGL_NO_CONTEXT
assert(ctx == ctx2);
It's caused by function _eglGetContextHandle, the function will check the link
status of egl context and return EGL_NO_CONTEXT if it's unlinked.
We should remove link status checking(_eglIsResourceLinked) inside of function
_eglGetContextHandle, the function is used for checking current context(TLS)
only, so it should be safe enough to trust the context pointer. Since current
egl context could be removed from resource list by function eglDestroyContext,
according to egl spec, it should be valid if the ctx is still in use, so
_eglLookupContext should return it instead of EGL_NO_CONTEXT.
---
src/egl/main/eglcontext.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/egl/main/eglcontext.h b/src/egl/main/eglcontext.h
index 241917f..aeec84f 100644
--- a/src/egl/main/eglcontext.h
+++ b/src/egl/main/eglcontext.h
@@ -145,7 +145,7 @@ static inline EGLContext
_eglGetContextHandle(_EGLContext *ctx)
{
_EGLResource *res = (_EGLResource *) ctx;
- return (res && _eglIsResourceLinked(res)) ?
+ return (res) ?
(EGLContext) ctx : EGL_NO_CONTEXT;
}
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] EXT_multisampled_render_to_texture support in gallium
This extension allows the same thing you can do with MSAA visuals, but with actual textures. Additionally it allows a tiler's pipeline to stay MSAA until the end, at which point the resolve happens to slower memory (and it's thus advantageous for that to not be multisampled). I've had a few thoughts about how to implement it, but nothing too actionable thus far. The main issue is in conveying the desire to perform MS operations without actually supplying MS cbufs. My current thinking is to switch the "multisample" rasterizer bit to be wider and supply the # of samples to it. I'm not sure if GLES3.1 will interfere with this, since it seems to support MS textures, but something like this would be useful for freedreno and probably other tiling drivers. Thoughts appreciated. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 90130] gl_PrimitiveId seems to reset at 340
https://bugs.freedesktop.org/show_bug.cgi?id=90130 Bug ID: 90130 Summary: gl_PrimitiveId seems to reset at 340 Product: Mesa Version: 10.5 Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: critical Priority: medium Component: Mesa core Assignee: mesa-dev@lists.freedesktop.org Reporter: kennethmorrismar...@gmail.com QA Contact: mesa-dev@lists.freedesktop.org Using gl_PrimitiveId with mesa 10.5.3 on Linux (running in a Virtualbox VM on windows) I am seeing glPrimitiveId increment up to 339 and then it resets back to zero and starts incrementing again. Yeh, 0-339, that's funky. This is the source of many of the failures on the following dashboard https://open.cdash.org/viewTest.php?onlyfailed&buildid=3781713 as the mapping of cells to colors is based on gl_PrimitiveId and it seems to be restarting after 339. My code (a VTK branch) has a vertex shader and fragment shader with no geometry shader. I am requesting and getting a 3.2 core context. The same codebase on Windows using nvidia or intel drivers (non mesa) is working. Any help appreciated. If there is additional information etc that would be helpful just let me know and I can try to dig it up. -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2 01/22] meta: Enable _mesa_meta_pbo_GetTexSubImage() to read in to non-pbo buffers
Anuj Phogat writes:
> This will allow Skylake to use _mesa_meta_pbo_GetTexSubImage() for reading
> YF/YS
> tiled surfaces.
>
> V2: Make changes suggested by Neil.
>
> Signed-off-by: Anuj Phogat
> Cc: Neil Roberts
> ---
> src/mesa/drivers/common/meta.h | 1 +
> src/mesa/drivers/common/meta_tex_subimage.c | 43
>
> src/mesa/drivers/dri/i965/intel_pixel_read.c | 11 +++
> src/mesa/drivers/dri/i965/intel_tex_image.c | 3 +-
> 4 files changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h
> index e7d894d..fd1ca1f 100644
> --- a/src/mesa/drivers/common/meta.h
> +++ b/src/mesa/drivers/common/meta.h
> @@ -542,6 +542,7 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
> GLuint dims,
>int xoffset, int yoffset, int zoffset,
>int width, int height, int depth,
>GLenum format, GLenum type, const void *pixels,
> + bool create_pbo, bool for_read_pixels,
>const struct gl_pixelstore_attrib *packing);
>
> extern void
> diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
> b/src/mesa/drivers/common/meta_tex_subimage.c
> index ad6e787..fbd7e76 100644
> --- a/src/mesa/drivers/common/meta_tex_subimage.c
> +++ b/src/mesa/drivers/common/meta_tex_subimage.c
> @@ -34,6 +34,7 @@
> #include "macros.h"
> #include "meta.h"
> #include "pbo.h"
> +#include "readpix.h"
> #include "shaderapi.h"
> #include "state.h"
> #include "teximage.h"
> @@ -150,7 +151,8 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint
> dims,
> bool success = false;
> int z;
>
> - if (!_mesa_is_bufferobj(packing->BufferObj) && !create_pbo)
> + if (!_mesa_is_bufferobj(packing->BufferObj) &&
> + (!create_pbo || pixels == NULL))
>return false;
Is this hunk supposed to be here? It looks unrelated and I'm not sure
what it's trying to achieve. I guess it would catch situations where a
PBO is not used but pixels == NULL? It seems like this is a bit of a
strange place to try and catch that as it's an API usage error rather
than a failure for the driver function.
> if (format == GL_DEPTH_COMPONENT ||
> @@ -252,6 +254,7 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
> GLuint dims,
>int xoffset, int yoffset, int zoffset,
>int width, int height, int depth,
>GLenum format, GLenum type, const void *pixels,
> + bool create_pbo, bool for_read_pixels,
>const struct gl_pixelstore_attrib *packing)
> {
> GLuint pbo = 0, pbo_tex = 0, fbos[2] = { 0, 0 };
> @@ -259,9 +262,11 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
> GLuint dims,
> struct gl_texture_image *pbo_tex_image;
> GLenum status;
> bool success = false;
> + const bool is_bufferobj = _mesa_is_bufferobj(packing->BufferObj);
> + const bool create_temp_pbo = create_pbo && !is_bufferobj;
> int z;
>
> - if (!_mesa_is_bufferobj(packing->BufferObj))
> + if (!is_bufferobj && !create_pbo)
>return false;
>
> if (format == GL_DEPTH_COMPONENT ||
> @@ -280,13 +285,26 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
> GLuint dims,
> image_height = packing->ImageHeight == 0 ? height : packing->ImageHeight;
> full_height = image_height * (depth - 1) + height;
>
> - pbo_tex_image = create_texture_for_pbo(ctx, false, GL_PIXEL_PACK_BUFFER,
> - width, full_height * depth,
> - format, type, pixels, packing,
> + pbo_tex_image = create_texture_for_pbo(ctx, create_pbo,
> + GL_PIXEL_PACK_BUFFER,
> + width, full_height,
> + is_bufferobj ? format : GL_RGBA,
> + is_bufferobj ? type: GL_FLOAT,
> + pixels, packing,
>&pbo, &pbo_tex);
Why does this pass GL_RGBA/GL_FLOAT here? Won't that make
create_texture_for_pbo use the wrong rowstride and size for the buffer?
I guess it would probably work anyway because there's unlikely to be a
format that needs a bigger rowstride than this, but it might end up
making the buffer much larger than it needs to be.
> if (!pbo_tex_image)
>return false;
>
> + /* Copy the _BaseFormat of tex_image to pbo_tex_image. Depending on the
> +* base format involved we may need to apply a rebase transform later in
> +* _mesa_meta_GetTexImage() (See get_tex_rgba_compressed() and
> +* get_tex_rgba_uncompressed()). For example if we download to a Luminance
> +* format we want G=0 and B=0.
> +*
[Mesa-dev] [PATCH] i965: Remove end-of-thread SEND alignment code.
This was present in Eric's initial implementation of the compaction code
for Sandybridge (commit 077d01b6). There is no documentation saying this
is necessary, and removing it causes no regressions in piglit on any
platform.
In fact, the BSpec says
- "[Nop and Illegal] cannot be compressed."; and
- "Currently, there is no need for between-instruction padding."
---
src/mesa/drivers/dri/i965/brw_eu_compact.c | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_eu_compact.c
b/src/mesa/drivers/dri/i965/brw_eu_compact.c
index 26c41ea..72adbe0 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_compact.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_compact.c
@@ -1398,19 +1398,11 @@ brw_compact_instructions(struct brw_compile *p, int
start_offset,
offset += sizeof(brw_compact_inst);
} else {
- /* It appears that the end of thread SEND instruction needs to be
- * aligned, or the GPU hangs. All uncompacted instructions need to be
- * aligned on G45.
- */
- if ((offset & sizeof(brw_compact_inst)) != 0 &&
- (((brw_inst_opcode(brw, src) == BRW_OPCODE_SEND ||
-brw_inst_opcode(brw, src) == BRW_OPCODE_SENDC) &&
- brw_inst_eot(brw, src)) ||
- brw->is_g4x)) {
+ /* All uncompacted instructions need to be aligned on G45. */
+ if ((offset & sizeof(brw_compact_inst)) != 0 && brw->is_g4x){
brw_compact_inst *align = store + offset;
memset(align, 0, sizeof(*align));
-brw_compact_inst_set_opcode(align, brw->is_g4x ? BRW_OPCODE_NENOP :
- BRW_OPCODE_NOP);
+brw_compact_inst_set_opcode(align, BRW_OPCODE_NENOP);
brw_compact_inst_set_cmpt_control(align, true);
offset += sizeof(brw_compact_inst);
compacted_count--;
--
2.0.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 90130] gl_PrimitiveId seems to reset at 340
https://bugs.freedesktop.org/show_bug.cgi?id=90130 --- Comment #1 from Ilia Mirkin --- Any particular driver this happens on? -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 90130] gl_PrimitiveId seems to reset at 340
https://bugs.freedesktop.org/show_bug.cgi?id=90130 --- Comment #2 from Ken Martin --- I'm not sure if I am looking up the driver correctly but a version of glxinfo I installed says OpenGL vendor string: VMware, Inc. OpenGL renderer string: Gallium 0.4 on llvmpipe (LLVM 3.5, 128 bits) OpenGL version string: 3.0 Mesa 10.3.0 but an ldd on it shows it linking against /usr/lib/x86_64-linux-gnu/mesa/libGL.so.1 I did build mesa 10.5.3 and sudo make install it so I'm not sure why glxinfo is reporting 10.3.0. ldd on my app shows linking against /usr/local/lib/libGL.so.1.2.0 -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 90130] gl_PrimitiveId seems to reset at 340
https://bugs.freedesktop.org/show_bug.cgi?id=90130 --- Comment #3 from Ilia Mirkin --- (In reply to Ken Martin from comment #2) > I'm not sure if I am looking up the driver correctly but a version of > glxinfo I installed says > > OpenGL vendor string: VMware, Inc. > OpenGL renderer string: Gallium 0.4 on llvmpipe (LLVM 3.5, 128 bits) > OpenGL version string: 3.0 Mesa 10.3.0 > > but an ldd on it shows it linking against > > /usr/lib/x86_64-linux-gnu/mesa/libGL.so.1 > > I did build mesa 10.5.3 and sudo make install it so I'm not sure why glxinfo > is reporting 10.3.0. > > ldd on my app shows linking against /usr/local/lib/libGL.so.1.2.0 You can probably get the info by doing LD_LIBRARY_PATH=/usr/local/lib glxinfo -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 90130] gl_PrimitiveId seems to reset at 340
https://bugs.freedesktop.org/show_bug.cgi?id=90130 --- Comment #4 from Ilia Mirkin --- By the way, supplying an apitrace that demonstrates the issue would make debugging this quite useful. (https://github.com/apitrace/apitrace) -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/4] docs: mark off texture_stencil8 (v2.1)
copy drivers from the stencil_texturing list, softpipe is definitely broken for stencil texturing since it uses float, but I'll look at that later. v2.1: update relnotes Signed-off-by: Dave Airlie --- docs/GL3.txt | 2 +- docs/relnotes/10.6.0.html | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/GL3.txt b/docs/GL3.txt index 2dbd987..172fd3c 100644 --- a/docs/GL3.txt +++ b/docs/GL3.txt @@ -182,7 +182,7 @@ GL 4.4, GLSL 4.40: GL_ARB_multi_bindDONE (all drivers) GL_ARB_query_buffer_object not started GL_ARB_texture_mirror_clamp_to_edge DONE (i965, nv50, nvc0, r600, radeonsi, llvmpipe, softpipe) - GL_ARB_texture_stencil8 not started + GL_ARB_texture_stencil8 DONE (nv50, nvc0, r600, radeonsi, llvmpipe, softpipe) GL_ARB_vertex_type_10f_11f_11f_rev DONE (i965, nv50, nvc0, r600, radeonsi, llvmpipe, softpipe) GL 4.5, GLSL 4.50: diff --git a/docs/relnotes/10.6.0.html b/docs/relnotes/10.6.0.html index 82aea5c..48f76f9 100644 --- a/docs/relnotes/10.6.0.html +++ b/docs/relnotes/10.6.0.html @@ -54,6 +54,7 @@ Note: some of the new features are only available with certain drivers. GL_EXT_draw_buffers2 on freedreno GL_ARB_clip_control on i965 GL_ARB_program_interface_query (all drivers) +GL_ARB_texture_stencil8 on nv50, nvc0, r600, radeonsi, softpipe Bug fixes -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/4] st/mesa: add ARB_texture_stencil8 support (v4)
if we support stencil texturing, enable texture_stencil8
there is no requirement to support native S8 for this,
the texture can be converted to x24s8 fine.
v2: fold fixes from Marek in:
a) put S8 last in the list
b) fix renderable to always test for d/s renderable
fixup the texture case to use a stencil only format
for picking the format for the texture view.
v3: hit fallback for getteximage
v4: put s8 back in front, it shouldn't get picked now (Ilia)
Signed-off-by: Dave Airlie
---
src/mesa/state_tracker/st_atom_texture.c | 14 ++
src/mesa/state_tracker/st_cb_texture.c | 2 +-
src/mesa/state_tracker/st_extensions.c | 3 +++
src/mesa/state_tracker/st_format.c | 19 ---
4 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/src/mesa/state_tracker/st_atom_texture.c
b/src/mesa/state_tracker/st_atom_texture.c
index eff28fc..04ba864 100644
--- a/src/mesa/state_tracker/st_atom_texture.c
+++ b/src/mesa/state_tracker/st_atom_texture.c
@@ -287,16 +287,22 @@ st_get_texture_sampler_view_from_stobj(struct st_context
*st,
enum pipe_format format)
{
struct pipe_sampler_view **sv;
-
+ const struct st_texture_image *firstImage;
if (!stObj || !stObj->pt) {
return NULL;
}
sv = st_texture_get_sampler_view(st, stObj);
- if (stObj->base.StencilSampling &&
- util_format_is_depth_and_stencil(format))
- format = util_format_stencil_only(format);
+ if (util_format_is_depth_and_stencil(format)) {
+ if (stObj->base.StencilSampling)
+ format = util_format_stencil_only(format);
+ else {
+ firstImage =
st_texture_image_const(_mesa_base_tex_image(&stObj->base));
+ if (firstImage->base._BaseFormat == GL_STENCIL_INDEX)
+format = util_format_stencil_only(format);
+ }
+ }
/* if sampler view has changed dereference it */
if (*sv) {
diff --git a/src/mesa/state_tracker/st_cb_texture.c
b/src/mesa/state_tracker/st_cb_texture.c
index bdf236e..7ea3846 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -950,7 +950,7 @@ st_GetTexImage(struct gl_context * ctx,
/* XXX Fallback to _mesa_GetTexImage_sw for depth-stencil formats
* due to an incomplete stencil blit implementation in some drivers. */
- if (format == GL_DEPTH_STENCIL) {
+ if (format == GL_DEPTH_STENCIL || format == GL_STENCIL_INDEX) {
goto fallback;
}
diff --git a/src/mesa/state_tracker/st_extensions.c
b/src/mesa/state_tracker/st_extensions.c
index bc20f73..25932dd 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -650,6 +650,9 @@ void st_init_extensions(struct pipe_screen *screen,
ARRAY_SIZE(vertex_mapping), PIPE_BUFFER,
PIPE_BIND_VERTEX_BUFFER);
+ if (extensions->ARB_stencil_texturing)
+ extensions->ARB_texture_stencil8 = GL_TRUE;
+
/* Figure out GLSL support. */
glsl_feature_level = screen->get_param(screen, PIPE_CAP_GLSL_FEATURE_LEVEL);
diff --git a/src/mesa/state_tracker/st_format.c
b/src/mesa/state_tracker/st_format.c
index 72dbf3b..181465d 100644
--- a/src/mesa/state_tracker/st_format.c
+++ b/src/mesa/state_tracker/st_format.c
@@ -1942,11 +1942,6 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum
target,
GLint internalFormat,
GLenum format, GLenum type)
{
- const boolean want_renderable =
- internalFormat == 3 || internalFormat == 4 ||
- internalFormat == GL_RGB || internalFormat == GL_RGBA ||
- internalFormat == GL_RGB8 || internalFormat == GL_RGBA8 ||
- internalFormat == GL_BGRA;
struct st_context *st = st_context(ctx);
enum pipe_format pFormat;
unsigned bindings;
@@ -1962,15 +1957,17 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum
target,
}
/* GL textures may wind up being render targets, but we don't know
-* that in advance. Specify potential render target flags now.
+* that in advance. Specify potential render target flags now for formats
+* that we know should always be renderable.
*/
bindings = PIPE_BIND_SAMPLER_VIEW;
- if (want_renderable) {
- if (_mesa_is_depth_or_stencil_format(internalFormat))
-bindings |= PIPE_BIND_DEPTH_STENCIL;
- else
+ if (_mesa_is_depth_or_stencil_format(internalFormat))
+ bindings |= PIPE_BIND_DEPTH_STENCIL;
+ else if (internalFormat == 3 || internalFormat == 4 ||
+internalFormat == GL_RGB || internalFormat == GL_RGBA ||
+internalFormat == GL_RGB8 || internalFormat == GL_RGBA8 ||
+internalFormat == GL_BGRA)
bindings |= PIPE_BIND_RENDER_TARGET;
- }
/* GLES allows the driver to choose any format which matches
* the format+type combo, because GLES only supports unsized internal
--
2.1.0
_
[Mesa-dev] [PATCH 4/4] mesa/teximage: use correct extension for accept stencil texture.
This was using the wrong extension, ARB_stencil_texturing
doesn't mention any changes in this area.
(This should only be pushed once Intel supports stencil8
and fixes meta.)
Signed-off-by: Dave Airlie
---
src/mesa/main/teximage.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index d07263c..faca801 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -222,7 +222,7 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint
internalFormat )
}
}
- if (ctx->Extensions.ARB_stencil_texturing) {
+ if (ctx->Extensions.ARB_texture_stencil8) {
switch (internalFormat) {
case GL_STENCIL_INDEX:
case GL_STENCIL_INDEX1:
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] ARB_texture_stencil8 repost
This has some fixups since last version, I've moved the teximage patch to the end someone from Intel can fix that up once they add stencil8 to i965 and test it. Dave. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/4] mesa: finish implementing ARB_texture_stencil8 (v4)
Parts of this were implemented previously, so finish it off.
v2: fix getteximage falling into the integer check
add fixes for the FBO paths, (fbo-stencil8 test).
v3: fix getteximage path harder.
v4: remove swapbytes from getteximage path (Ilia)
Signed-off-by: Dave Airlie
---
src/mesa/main/extensions.c | 1 +
src/mesa/main/fbobject.c| 9 +---
src/mesa/main/texgetimage.c | 50 -
src/mesa/main/teximage.c| 3 ++-
4 files changed, 58 insertions(+), 5 deletions(-)
diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
index 861b150..3d4965c 100644
--- a/src/mesa/main/extensions.c
+++ b/src/mesa/main/extensions.c
@@ -186,6 +186,7 @@ static const struct extension extension_table[] = {
{ "GL_ARB_texture_rectangle", o(NV_texture_rectangle),
GL, 2004 },
{ "GL_ARB_texture_rgb10_a2ui", o(ARB_texture_rgb10_a2ui),
GL, 2009 },
{ "GL_ARB_texture_rg", o(ARB_texture_rg),
GL, 2008 },
+ { "GL_ARB_texture_stencil8",o(ARB_texture_stencil8),
GL, 2013 },
{ "GL_ARB_texture_storage", o(dummy_true),
GL, 2011 },
{ "GL_ARB_texture_storage_multisample", o(ARB_texture_multisample),
GL, 2012 },
{ "GL_ARB_texture_view",o(ARB_texture_view),
GL, 2012 },
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 8032585..27cf97f 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -813,8 +813,10 @@ test_attachment_completeness(const struct gl_context *ctx,
GLenum format,
if (ctx->Extensions.ARB_depth_texture &&
baseFormat == GL_DEPTH_STENCIL) {
/* OK */
- }
- else {
+ } else if (ctx->Extensions.ARB_texture_stencil8 &&
+baseFormat == GL_STENCIL_INDEX) {
+/* OK */
+ } else {
/* no such thing as stencil-only textures */
att_incomplete("illegal stencil texture");
att->Complete = GL_FALSE;
@@ -978,7 +980,8 @@ _mesa_test_framebuffer_completeness(struct gl_context *ctx,
if (!is_format_color_renderable(ctx, attFormat,
texImg->InternalFormat) &&
- !is_legal_depth_format(ctx, f)) {
+ !is_legal_depth_format(ctx, f) &&
+ f != GL_STENCIL_INDEX) {
fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
fbo_incomplete(ctx, "texture attachment incomplete", -1);
return;
diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
index 255d365..e28d972 100644
--- a/src/mesa/main/texgetimage.c
+++ b/src/mesa/main/texgetimage.c
@@ -161,6 +161,51 @@ get_tex_depth_stencil(struct gl_context *ctx, GLuint
dimensions,
width,
(const GLuint *) src,
type, dest);
+ }
+
+ ctx->Driver.UnmapTextureImage(ctx, texImage, img);
+ }
+ else {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glGetTexImage");
+ break;
+ }
+ }
+}
+
+/**
+ * glGetTexImage for stencil pixels.
+ */
+static void
+get_tex_stencil(struct gl_context *ctx, GLuint dimensions,
+GLenum format, GLenum type, GLvoid *pixels,
+struct gl_texture_image *texImage)
+{
+ const GLint width = texImage->Width;
+ const GLint height = texImage->Height;
+ const GLint depth = texImage->Depth;
+ GLint img, row;
+
+ assert(format == GL_STENCIL_INDEX);
+
+ for (img = 0; img < depth; img++) {
+ GLubyte *srcMap;
+ GLint rowstride;
+
+ /* map src texture buffer */
+ ctx->Driver.MapTextureImage(ctx, texImage, img,
+ 0, 0, width, height, GL_MAP_READ_BIT,
+ &srcMap, &rowstride);
+
+ if (srcMap) {
+ for (row = 0; row < height; row++) {
+const GLubyte *src = srcMap + row * rowstride;
+void *dest = _mesa_image_address(dimensions, &ctx->Pack, pixels,
+ width, height, format, type,
+ img, row, 0);
+_mesa_unpack_ubyte_stencil_row(texImage->TexFormat,
+ width,
+ (const GLuint *) src,
+ dest);
if (ctx->Pack.SwapBytes) {
_mesa_swap4((GLuint *) dest, width);
}
@@ -684,6 +729,9 @@ _mesa_GetTexImage_sw(struct gl_context *ctx,
else if (format == GL_DEPTH_STENCIL_EXT) {
[Mesa-dev] [PATCH] mesa: finish implementing ARB_texture_stencil8 (v5)
Parts of this were implemented previously, so finish it off.
v2: fix getteximage falling into the integer check
add fixes for the FBO paths, (fbo-stencil8 test).
v3: fix getteximage path harder.
v4: remove swapbytes from getteximage path (Ilia)
v5: brown paper bag the swapbytes removal. (Ilia)
Signed-off-by: Dave Airlie
---
src/mesa/main/extensions.c | 1 +
src/mesa/main/fbobject.c| 9 +---
src/mesa/main/texgetimage.c | 50 -
src/mesa/main/teximage.c| 3 ++-
4 files changed, 58 insertions(+), 5 deletions(-)
diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
index 861b150..3d4965c 100644
--- a/src/mesa/main/extensions.c
+++ b/src/mesa/main/extensions.c
@@ -186,6 +186,7 @@ static const struct extension extension_table[] = {
{ "GL_ARB_texture_rectangle", o(NV_texture_rectangle),
GL, 2004 },
{ "GL_ARB_texture_rgb10_a2ui", o(ARB_texture_rgb10_a2ui),
GL, 2009 },
{ "GL_ARB_texture_rg", o(ARB_texture_rg),
GL, 2008 },
+ { "GL_ARB_texture_stencil8",o(ARB_texture_stencil8),
GL, 2013 },
{ "GL_ARB_texture_storage", o(dummy_true),
GL, 2011 },
{ "GL_ARB_texture_storage_multisample", o(ARB_texture_multisample),
GL, 2012 },
{ "GL_ARB_texture_view",o(ARB_texture_view),
GL, 2012 },
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 8032585..27cf97f 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -813,8 +813,10 @@ test_attachment_completeness(const struct gl_context *ctx,
GLenum format,
if (ctx->Extensions.ARB_depth_texture &&
baseFormat == GL_DEPTH_STENCIL) {
/* OK */
- }
- else {
+ } else if (ctx->Extensions.ARB_texture_stencil8 &&
+baseFormat == GL_STENCIL_INDEX) {
+/* OK */
+ } else {
/* no such thing as stencil-only textures */
att_incomplete("illegal stencil texture");
att->Complete = GL_FALSE;
@@ -978,7 +980,8 @@ _mesa_test_framebuffer_completeness(struct gl_context *ctx,
if (!is_format_color_renderable(ctx, attFormat,
texImg->InternalFormat) &&
- !is_legal_depth_format(ctx, f)) {
+ !is_legal_depth_format(ctx, f) &&
+ f != GL_STENCIL_INDEX) {
fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
fbo_incomplete(ctx, "texture attachment incomplete", -1);
return;
diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
index 255d365..e6d6471 100644
--- a/src/mesa/main/texgetimage.c
+++ b/src/mesa/main/texgetimage.c
@@ -175,6 +175,51 @@ get_tex_depth_stencil(struct gl_context *ctx, GLuint
dimensions,
}
}
+/**
+ * glGetTexImage for stencil pixels.
+ */
+static void
+get_tex_stencil(struct gl_context *ctx, GLuint dimensions,
+GLenum format, GLenum type, GLvoid *pixels,
+struct gl_texture_image *texImage)
+{
+ const GLint width = texImage->Width;
+ const GLint height = texImage->Height;
+ const GLint depth = texImage->Depth;
+ GLint img, row;
+
+ assert(format == GL_STENCIL_INDEX);
+
+ for (img = 0; img < depth; img++) {
+ GLubyte *srcMap;
+ GLint rowstride;
+
+ /* map src texture buffer */
+ ctx->Driver.MapTextureImage(ctx, texImage, img,
+ 0, 0, width, height, GL_MAP_READ_BIT,
+ &srcMap, &rowstride);
+
+ if (srcMap) {
+ for (row = 0; row < height; row++) {
+const GLubyte *src = srcMap + row * rowstride;
+void *dest = _mesa_image_address(dimensions, &ctx->Pack, pixels,
+ width, height, format, type,
+ img, row, 0);
+_mesa_unpack_ubyte_stencil_row(texImage->TexFormat,
+ width,
+ (const GLuint *) src,
+ dest);
+ }
+
+ ctx->Driver.UnmapTextureImage(ctx, texImage, img);
+ }
+ else {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glGetTexImage");
+ break;
+ }
+ }
+}
+
/**
* glGetTexImage for YCbCr pixels.
@@ -684,6 +729,9 @@ _mesa_GetTexImage_sw(struct gl_context *ctx,
else if (format == GL_DEPTH_STENCIL_EXT) {
get_tex_depth_stencil(ctx, dimensions, format, type, pixels, texImage);
}
+ else if (format == GL_STENCIL_INDEX) {
+ get_tex_stencil(ctx, dimensions, format, type, pix
Re: [Mesa-dev] ARB_texture_stencil8 repost
FWIW, series is Reviewed-by: Ilia Mirkin But I'm not extremely familiar with all that texture logic. May be nice to recruit someone to review who knows this stuff... maybe Jason Ekstrand? Or just push and sort any issues out later. -ilia On Tue, Apr 21, 2015 at 9:43 PM, Dave Airlie wrote: > This has some fixups since last version, I've moved the teximage patch > to the end someone from Intel can fix that up once they add stencil8 > to i965 and test it. > > Dave. > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 90130] gl_PrimitiveId seems to reset at 340
https://bugs.freedesktop.org/show_bug.cgi?id=90130 --- Comment #5 from Roland Scheidegger --- Or even better, a piglit test :-). I suspect there's an issue with the prim assembler in draw. Since from a quick look it seems like the prim assembler would always reset the prim_id it's going to inject whenever it's run, and that's going to happen per chunk (as we don't process all vertices in a draw call at once if there's too many). Probably would need to fix that somehow so it's only reset per instance. We possibly (?) handle this correctly if there's a gs. -- 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 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] glsl: rewrite glsl_type::record_key_hash() to avoid buffer overflow
Reviewed-by: Ian Romanick
On 04/17/2015 06:52 AM, Brian Paul wrote:
> This should be more efficient than the previous snprintf() solution.
> But more importantly, it avoids a buffer overflow bug that could result
> in crashes or unpredictable results when processing very large interface
> blocks.
>
> For the app in question, key->length = 103 for some interfaces. The check
> if size >= sizeof(hash_key) was insufficient to prevent overflows of the
> hash_key[128] array because it didn't account for the terminating zero.
> In this case, this caused the call to hash_table_string_hash() to return
> different results for identical inputs, and then shader linking failed.
>
> This new solution also takes all structure fields into account instead
> of just the first 15 when sizeof(pointer)==8.
>
> Cc: mesa-sta...@lists.freedesktop.org
> ---
> src/glsl/glsl_types.cpp | 23 +--
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 4aa36a7..9c9b7ef 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -738,24 +738,27 @@ glsl_type::record_key_compare(const void *a, const void
> *b)
> }
>
>
> +/**
> + * Generate an integer hash value for a glsl_type structure type.
> + */
> unsigned
> glsl_type::record_key_hash(const void *a)
> {
> const glsl_type *const key = (glsl_type *) a;
> - char hash_key[128];
> - unsigned size = 0;
> -
> - size = snprintf(hash_key, sizeof(hash_key), "%08x", key->length);
> + uintptr_t hash = key->length;
> + unsigned retval;
>
> for (unsigned i = 0; i < key->length; i++) {
> - if (size >= sizeof(hash_key))
> - break;
> -
> - size += snprintf(& hash_key[size], sizeof(hash_key) - size,
> -"%p", (void *) key->fields.structure[i].type);
> + /* casting pointer to uintptr_t */
> + hash = (hash * 13 ) + (uintptr_t) key->fields.structure[i].type;
> }
>
> - return hash_table_string_hash(& hash_key);
> + if (sizeof(hash) == 8)
> + retval = (hash & 0x) ^ ((uint64_t) hash >> 32);
> + else
> + retval = hash;
> +
> + return retval;
> }
>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Remove end-of-thread SEND alignment code.
On Tuesday, April 21, 2015 03:25:07 PM Matt Turner wrote:
> This was present in Eric's initial implementation of the compaction code
> for Sandybridge (commit 077d01b6). There is no documentation saying this
> is necessary, and removing it causes no regressions in piglit on any
> platform.
>
> In fact, the BSpec says
>
>- "[Nop and Illegal] cannot be compressed."; and
That's "compression", not "compaction" - it doesn't seem related.
Compressed NOPs or ILLEGALs (nop(16) that decodes as two nop(8)s?) do
seem entirely pointless. Compacted NOPs make sense.
>- "Currently, there is no need for between-instruction padding."
This text appears in the original 965 PRM...and that platform didn't
support instruction compaction at all. So I expect the text has been
carried forward from generation to generation without much thought,
rather than intended to convey something about compacted NOPs.
The fact that this hasn't broken anything is compelling, though...I
wonder if we had a bug in the code to create padding between SIMD8 and
SIMD16 programs, and aligning the EOT on the SIMD8 prevented the entire
SIMD16 program from getting misaligned?
I can't find any documentation indicating it's necessary either.
> ---
> src/mesa/drivers/dri/i965/brw_eu_compact.c | 14 +++---
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_compact.c
> b/src/mesa/drivers/dri/i965/brw_eu_compact.c
> index 26c41ea..72adbe0 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_compact.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_compact.c
> @@ -1398,19 +1398,11 @@ brw_compact_instructions(struct brw_compile *p, int
> start_offset,
>
> offset += sizeof(brw_compact_inst);
>} else {
> - /* It appears that the end of thread SEND instruction needs to be
> - * aligned, or the GPU hangs. All uncompacted instructions need to
> be
> - * aligned on G45.
> - */
> - if ((offset & sizeof(brw_compact_inst)) != 0 &&
> - (((brw_inst_opcode(brw, src) == BRW_OPCODE_SEND ||
> -brw_inst_opcode(brw, src) == BRW_OPCODE_SENDC) &&
> - brw_inst_eot(brw, src)) ||
> - brw->is_g4x)) {
> + /* All uncompacted instructions need to be aligned on G45. */
> + if ((offset & sizeof(brw_compact_inst)) != 0 && brw->is_g4x){
> brw_compact_inst *align = store + offset;
> memset(align, 0, sizeof(*align));
> -brw_compact_inst_set_opcode(align, brw->is_g4x ?
> BRW_OPCODE_NENOP :
> - BRW_OPCODE_NOP);
> +brw_compact_inst_set_opcode(align, BRW_OPCODE_NENOP);
> brw_compact_inst_set_cmpt_control(align, true);
> offset += sizeof(brw_compact_inst);
> compacted_count--;
>
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] i965: Disallow linear blits that are not cacheline aligned.
The BLT engine on Gen8+ requires linear surfaces to be cacheline
aligned. This restriction was added as part of converting the BLT to
use 48-bit addressing.
The main user, intel_emit_linear_blit, now handles this properly.
But we might also have linear miptrees; just refuse to blit those.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88521
Signed-off-by: Kenneth Graunke
Cc: mesa-sta...@lists.freedesktop.org
---
src/mesa/drivers/dri/i965/intel_blit.c | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
b/src/mesa/drivers/dri/i965/intel_blit.c
index 98d414c..7680a40 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.c
+++ b/src/mesa/drivers/dri/i965/intel_blit.c
@@ -271,6 +271,20 @@ intel_miptree_blit(struct brw_context *brw,
return true;
}
+static bool
+alignment_valid(struct brw_context *brw, unsigned offset, uint32_t tiling)
+{
+ /* Tiled buffers must be page-aligned (4K). */
+ if (tiling != I915_TILING_NONE)
+ return (offset & 4095) == 0;
+
+ /* On Gen8+, linear buffers must be cacheline-aligned. */
+ if (brw->gen >= 8)
+ return (offset & 63) == 0;
+
+ return true;
+}
+
/* Copy BitBlt
*/
bool
@@ -296,14 +310,11 @@ intelEmitCopyBlit(struct brw_context *brw,
bool dst_y_tiled = dst_tiling == I915_TILING_Y;
bool src_y_tiled = src_tiling == I915_TILING_Y;
- if (dst_tiling != I915_TILING_NONE) {
- if (dst_offset & 4095)
-return false;
- }
- if (src_tiling != I915_TILING_NONE) {
- if (src_offset & 4095)
-return false;
- }
+ if (!alignment_valid(brw, dst_offset, dst_tiling))
+ return false;
+ if (!alignment_valid(brw, src_offset, src_tiling))
+ return false;
+
if ((dst_y_tiled || src_y_tiled) && brw->gen < 6)
return false;
--
2.3.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] i965: Make intel_emit_linear_blit handle Gen8+ alignment restrictions.
The BLT engine on Gen8+ requires linear surfaces to be cacheline
aligned. This restriction was added as part of converting the BLT to
use 48-bit addressing.
intel_emit_linear_blit needs to handle blits that are not cacheline
aligned, as we use it for arbitrary glBufferSubData calls and subrange
mappings.
Since intel_emit_linear_blit uses 1 byte per pixel, we can use the src/dst
pixel X offset field to represent the unaligned portion, and subtract
that from the address so it's cacheline aligned.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88521
Signed-off-by: Kenneth Graunke
Cc: mesa-sta...@lists.freedesktop.org
---
src/mesa/drivers/dri/i965/intel_blit.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
b/src/mesa/drivers/dri/i965/intel_blit.c
index 4993f60..98d414c 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.c
+++ b/src/mesa/drivers/dri/i965/intel_blit.c
@@ -524,6 +524,7 @@ intel_emit_linear_blit(struct brw_context *brw,
{
struct gl_context *ctx = &brw->ctx;
GLuint pitch, height;
+ int16_t src_x, dst_x;
bool ok;
/* The pitch given to the GPU must be DWORD aligned, and
@@ -532,11 +533,13 @@ intel_emit_linear_blit(struct brw_context *brw,
*/
pitch = ROUND_DOWN_TO(MIN2(size, (1 << 15) - 1), 4);
height = (pitch == 0) ? 1 : size / pitch;
+ src_x = src_offset % 64;
+ dst_x = dst_offset % 64;
ok = intelEmitCopyBlit(brw, 1,
- pitch, src_bo, src_offset, I915_TILING_NONE,
- pitch, dst_bo, dst_offset, I915_TILING_NONE,
- 0, 0, /* src x/y */
- 0, 0, /* dst x/y */
+ pitch, src_bo, src_offset - src_x, I915_TILING_NONE,
+ pitch, dst_bo, dst_offset - dst_x, I915_TILING_NONE,
+ src_x, 0, /* src x/y */
+ dst_x, 0, /* dst x/y */
pitch, height, /* w, h */
GL_COPY);
if (!ok)
@@ -544,15 +547,18 @@ intel_emit_linear_blit(struct brw_context *brw,
src_offset += pitch * height;
dst_offset += pitch * height;
+ src_x = src_offset % 64;
+ dst_x = dst_offset % 64;
size -= pitch * height;
assert (size < (1 << 15));
pitch = ALIGN(size, 4);
+
if (size != 0) {
ok = intelEmitCopyBlit(brw, 1,
-pitch, src_bo, src_offset, I915_TILING_NONE,
-pitch, dst_bo, dst_offset, I915_TILING_NONE,
-0, 0, /* src x/y */
-0, 0, /* dst x/y */
+pitch, src_bo, src_offset - src_x,
I915_TILING_NONE,
+pitch, dst_bo, dst_offset - dst_x,
I915_TILING_NONE,
+src_x, 0, /* src x/y */
+dst_x, 0, /* dst x/y */
size, 1, /* w, h */
GL_COPY);
if (!ok)
--
2.3.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH 1/2] i965: Make intel_emit_linear_blit handle Gen8+ alignment restrictions.
Assuming this solves the GPU hangs, the series is
Reviewed-by: Ian Romanick
On 04/22/2015 02:13 PM, Kenneth Graunke wrote:
> The BLT engine on Gen8+ requires linear surfaces to be cacheline
> aligned. This restriction was added as part of converting the BLT to
> use 48-bit addressing.
>
> intel_emit_linear_blit needs to handle blits that are not cacheline
> aligned, as we use it for arbitrary glBufferSubData calls and subrange
> mappings.
>
> Since intel_emit_linear_blit uses 1 byte per pixel, we can use the src/dst
> pixel X offset field to represent the unaligned portion, and subtract
> that from the address so it's cacheline aligned.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88521
> Signed-off-by: Kenneth Graunke
> Cc: mesa-sta...@lists.freedesktop.org
> ---
> src/mesa/drivers/dri/i965/intel_blit.c | 22 ++
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
> b/src/mesa/drivers/dri/i965/intel_blit.c
> index 4993f60..98d414c 100644
> --- a/src/mesa/drivers/dri/i965/intel_blit.c
> +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> @@ -524,6 +524,7 @@ intel_emit_linear_blit(struct brw_context *brw,
> {
> struct gl_context *ctx = &brw->ctx;
> GLuint pitch, height;
> + int16_t src_x, dst_x;
> bool ok;
>
> /* The pitch given to the GPU must be DWORD aligned, and
> @@ -532,11 +533,13 @@ intel_emit_linear_blit(struct brw_context *brw,
> */
> pitch = ROUND_DOWN_TO(MIN2(size, (1 << 15) - 1), 4);
> height = (pitch == 0) ? 1 : size / pitch;
> + src_x = src_offset % 64;
> + dst_x = dst_offset % 64;
> ok = intelEmitCopyBlit(brw, 1,
> - pitch, src_bo, src_offset, I915_TILING_NONE,
> - pitch, dst_bo, dst_offset, I915_TILING_NONE,
> - 0, 0, /* src x/y */
> - 0, 0, /* dst x/y */
> + pitch, src_bo, src_offset - src_x, I915_TILING_NONE,
> + pitch, dst_bo, dst_offset - dst_x, I915_TILING_NONE,
> + src_x, 0, /* src x/y */
> + dst_x, 0, /* dst x/y */
> pitch, height, /* w, h */
> GL_COPY);
> if (!ok)
> @@ -544,15 +547,18 @@ intel_emit_linear_blit(struct brw_context *brw,
>
> src_offset += pitch * height;
> dst_offset += pitch * height;
> + src_x = src_offset % 64;
> + dst_x = dst_offset % 64;
> size -= pitch * height;
> assert (size < (1 << 15));
> pitch = ALIGN(size, 4);
> +
> if (size != 0) {
>ok = intelEmitCopyBlit(brw, 1,
> - pitch, src_bo, src_offset, I915_TILING_NONE,
> - pitch, dst_bo, dst_offset, I915_TILING_NONE,
> - 0, 0, /* src x/y */
> - 0, 0, /* dst x/y */
> + pitch, src_bo, src_offset - src_x,
> I915_TILING_NONE,
> + pitch, dst_bo, dst_offset - dst_x,
> I915_TILING_NONE,
> + src_x, 0, /* src x/y */
> + dst_x, 0, /* dst x/y */
>size, 1, /* w, h */
>GL_COPY);
>if (!ok)
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH 1/2] i965: Make intel_emit_linear_blit handle Gen8+ alignment restrictions.
On Wednesday, April 22, 2015 02:19:35 PM Ian Romanick wrote: > Assuming this solves the GPU hangs, the series is > > Reviewed-by: Ian Romanick Hmm? What GPU hangs? These patches solve misrendering in GLbenchmark 2.7/TRex on Broadwell, where there were a bunch of stray triangles. When our glBufferSubData path hits a busy buffer, we queue a BLT upload. That blit might be unaligned. I suspect that the new data in the unaligned section never landed (or some data was trashed?), so some vertices end up with bogus data, and some triangles go awry. I'm not aware of any GPU hangs. signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
