[Mesa-dev] [Bug 90264] [Regression, bisected] Tooltip corruption in Chrome

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=90264

--- Comment #80 from Paviluf  ---
It seem that will be "fixed" soon in chromium. They will disable compositing
for tooltips on Linux. I still hope that the root of the bug will be fixed in
mesa.

https://bugs.chromium.org/p/chromium/issues/detail?id=442111

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] egl/dri2: Implement swapInterval fallback in a conformant way

[Tomasz Figa]

dri2_fallback_swap_interval() currently used to stub out swap interval
support in Android backend does nothing besides returning EGL_FALSE.
This causes at least one known application (Android Snapchat) to fail
due to an unexpected error and my loose interpretation of the EGL 1.5
specification justifies it. Relevant quote below:

The function

EGLBoolean eglSwapInterval(EGLDisplay dpy, EGLint interval);

specifies the minimum number of video frame periods per buffer swap
for the draw surface of the current context, for the current rendering
API. [...]

The parameter interval specifies the minimum number of video frames
that are displayed before a buffer swap will occur. The interval
specified by the function applies to the draw surface bound to the
context that is current on the calling thread. [...] interval is
silently clamped to minimum and maximum implementation dependent
values before being stored; these values are defined by EGLConfig
attributes EGL_MIN_SWAP_INTERVAL and EGL_MAX_SWAP_INTERVAL
respectively.

The default swap interval is 1.

Even though it does not specify the exact behavior if the platform does
not support changing the swap interval, the default assumed state is the
swap interval of 1, which I interpret as a value that eglSwapInterval()
should succeed if called with, even if there is no ability to change the
interval (but there is no change requested). Moreover, since the
behavior is defined to clamp the requested value to minimum and maximum
and at least the default value of 1 must be present in the range, the
implementation might be expected to have a valid range, which in case of
the feature being unsupported, would correspond to {1} and any request
might be expected to be clamped to this value.

This is further confirmed by the code in _eglSwapInterval() in
src/egl/main/eglsurface.c, which is the default fallback implementation
for EGL drivers not implementing its own. The problem with it is that
the DRI2 EGL driver provides its own implementation that calls into
platform backends, so we cannot just simply fall back to it.

Signed-off-by: Tomasz Figa 
---
 src/egl/drivers/dri2/egl_dri2.c   | 12 
 src/egl/drivers/dri2/egl_dri2_fallbacks.h |  9 -
 src/egl/drivers/dri2/platform_x11.c   |  1 +
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index f0d1ded408..686dd68d29 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -630,6 +630,18 @@ dri2_setup_screen(_EGLDisplay *disp)
struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
unsigned int api_mask;
 
+   /*
+* EGL 1.5 specification defines the default value to 1. Moreover,
+* eglSwapInterval() is required to clamp requested value to the supported
+* range. Since the default value is implicitly assumed to be supported,
+* use it as both minimum and maximum for the platforms that do not allow
+* changing the interval. Platforms, which allow it (e.g. x11, wayland)
+* override these values already.
+*/
+   dri2_dpy->min_swap_interval = 1;
+   dri2_dpy->max_swap_interval = 1;
+   dri2_dpy->default_swap_interval = 1;
+
if (dri2_dpy->image_driver) {
   api_mask = dri2_dpy->image_driver->getAPIMask(dri2_dpy->dri_screen);
} else if (dri2_dpy->dri2) {
diff --git a/src/egl/drivers/dri2/egl_dri2_fallbacks.h 
b/src/egl/drivers/dri2/egl_dri2_fallbacks.h
index 604db881a8..c70c686f8d 100644
--- a/src/egl/drivers/dri2/egl_dri2_fallbacks.h
+++ b/src/egl/drivers/dri2/egl_dri2_fallbacks.h
@@ -59,7 +59,14 @@ static inline EGLBoolean
 dri2_fallback_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
 _EGLSurface *surf, EGLint interval)
 {
-   return EGL_FALSE;
+   if (interval > surf->Config->MaxSwapInterval)
+  interval = surf->Config->MaxSwapInterval;
+   else if (interval < surf->Config->MinSwapInterval)
+  interval = surf->Config->MinSwapInterval;
+
+   surf->SwapInterval = interval;
+
+   return EGL_TRUE;
 }
 
 static inline EGLBoolean
diff --git a/src/egl/drivers/dri2/platform_x11.c 
b/src/egl/drivers/dri2/platform_x11.c
index 4610ec579f..97cdd09078 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -1283,6 +1283,7 @@ dri2_x11_setup_swap_interval(struct dri2_egl_display 
*dri2_dpy)
 */
dri2_dpy->min_swap_interval = 0;
dri2_dpy->max_swap_interval = 0;
+   dri2_dpy->default_swap_interval = 0;
 
if (!dri2_dpy->swap_available)
   return;
-- 
2.14.0.434.g98096fd7a8-goog

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/4] dri: Introduce SWAP_METHOD tokens

[Thomas Hellstrom]

On 08/10/2017 05:20 AM, Michel Dänzer wrote:

On 09/08/17 06:53 PM, Thomas Hellstrom wrote:

We shouldn't be using GLX tokens in the dri subsystem, so define dri
SWAP_METHOD tokens and translate when necessary. Unfortunately the X server
uses the dri swap method value untranslated as the GLX fbconfig swapMethod,
so we can't enumerate these tokens arbitrarily, but rather need to make them
have the same values as the corresponding GLX tokens.

Signed-off-by: Thomas Hellstrom 

[...]


diff --git a/src/gallium/state_trackers/dri/dri_screen.c 
b/src/gallium/state_trackers/dri/dri_screen.c
index 406e97d..1d9f441 100644
--- a/src/gallium/state_trackers/dri/dri_screen.c
+++ b/src/gallium/state_trackers/dri/dri_screen.c
@@ -156,7 +156,8 @@ dri_fill_in_modes(struct dri_screen *screen)
 boolean mixed_color_depth;
  
 static const GLenum back_buffer_modes[] = {

-  GLX_NONE, GLX_SWAP_UNDEFINED_OML, GLX_SWAP_COPY_OML
+  __DRI_ATTRIB_SWAP_NONE, __DRI_ATTRIB_SWAP_UNDEFINED,
+  __DRI_ATTRIB_SWAP_COPY
 };

BTW, does this array need to include __DRI_ATTRIB_SWAP_EXCHANGE as well,
or is it possible to use GLX_SWAP_EXCHANGE_OML anyway?


No it's not. I have a patch that adds __DRI_ATTRIB_SWAP_EXCHANGE for 
dri3 only (that's when the DRI driver / state tracker is initialized 
with the image loader extension) but still it doesn't get advertised 
since the advertised direct rendering fbconfigs are a subset only of the 
AIGLX exported fbconfigs, and AIGLX doesn't use the image loader 
extension / dri3 path. See the following email thread for more insight 
on this:


https://lists.freedesktop.org/archives/mesa-dev/2017-June/161417.html

As agreed in that thread, that should probably be remedied by rewriting 
the GLX fbconfig selection code so that the DRI startup decides what 
fbconfigs should be exported and the GLX code only makes sure there is a 
matching visual if needed. Since my primary motivation for doing these 
changes is to be able to run glretrace with GLX_SWAP_COPY_OML enabled, 
actually advertising GLX_SWAP_EXCHANGE_OML has lower priority ATM, but 
I'll try to do that at some point.


For the record, though, the code was tested with me unconditionally 
adding __DRI_ATTRIB_SWAP_EXCHANGE for both server and client and using a 
fixed version of  piglit glx-swap-exchange (posted on the piglit list).


Thanks,

Thomas




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/4] dri: Fix __DRIconfig reporting of __DRI_ATTRIB_SWAP_METHOD

[Thomas Hellstrom]

On 08/10/2017 05:20 AM, Michel Dänzer wrote:

On 09/08/17 06:53 PM, Thomas Hellstrom wrote:

The attribMap had two entries for this attribute, and
driGetConfigAttribIndex didn't return a proper value for this attribute.
Fix this, and also make sure we return SWAP_UNDEFINED for single-buffer
configs as required by the GLX_OML_swap_method spec.

Finally bump the dri core extension version to 2, indicating that we
correctly report __DRI_ATTRIB_SWAP_METHOD.

Signed-off-by: Thomas Hellstrom 

[...]


diff --git a/src/mesa/drivers/dri/common/utils.c 
b/src/mesa/drivers/dri/common/utils.c
index c37d446..f3ea61e 100644
--- a/src/mesa/drivers/dri/common/utils.c
+++ b/src/mesa/drivers/dri/common/utils.c
@@ -284,8 +284,9 @@ driCreateConfigs(mesa_format format,
modes->transparentIndex = GLX_DONT_CARE;
modes->rgbMode = GL_TRUE;
  
-		if ( db_modes[i] == GLX_NONE ) {

+   if ( db_modes[i] == GLX_NONE) {
modes->doubleBufferMode = GL_FALSE;
+modes->swapMethod = GLX_SWAP_UNDEFINED_OML;

The indentation of the added line doesn't match that of the previous
line. Patch 4 further changes this code (which is also what Brian's
comment was about AFAICT) to be even less consistently indented compared
to the surrounding code.


Hmm, Actually this looks awkward in the patches only due to me using 
spaces rather than tabs to indent in the new code (old code uses tabs).  
Not sure what the preferred strategy is here, use mesa coding style 
(spaces) or old dri coding style (tabs). If the latter, we should 
probably at some point add subsystem editor setup files that override 
the mesa default ones.




With that fixed in both patches, the series is





Reviewed-by: Michel Dänzer 



Thanks for reviewing!

Thomas



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 8/8] clover/llvm: Make __OPENCL_VERSION__ dynamic

[Aaron Watry]

On Fri, Aug 4, 2017 at 1:43 PM, Jan Vesely  wrote:
> On Sun, 2017-07-30 at 20:26 -0500, Aaron Watry wrote:
>> Signed-off-by: Aaron Watry 
>> CC: Jan Vesely 
>>
>> v2: base it on the device version
>> ---
>>  src/gallium/state_trackers/clover/llvm/invocation.cpp | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp 
>> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> index 63b2961752..443cd31e66 100644
>> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> @@ -224,7 +224,8 @@ namespace {
>>c.getPreprocessorOpts().Includes.push_back("clc/clc.h");
>>
>>// Add definition for the OpenCL version
>> -  c.getPreprocessorOpts().addMacroDef("__OPENCL_VERSION__=110");
>> +  c.getPreprocessorOpts().addMacroDef("__OPENCL_VERSION__=" +
>> +  
>> std::to_string(get_language_from_version_str(dev.device_version(;
>
> I don't think you can use the same parsing function here.
> __OPENCL_VERSION__ can go up to 2.2, while __OPENCL_C_VERSION__ is max
> 2.0

Is that an issue here? I thought that the device's highest supported
OpenCL version was what was required here?

__OPENCL_VERSION__ is defined as "substitutes an integer reflecting
the version number of the OpenCL
supported by the OpenCL device", which in this case should map
directly to dev.device_version().

__OPENCL_C_VERSION__ is already added by clang when the pre-processor
is initialized using the selected language version
(clang/FrontEnd/InitPreprocessor.cpp).

--Aaron

>
> Jan
>
>>
>>// clc.h requires that this macro be defined:
>>
>> c.getPreprocessorOpts().addMacroDef("cl_clang_storage_class_specifiers");
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] dri3: Swapbuffer update

[Michel Dänzer]

On 09/08/17 04:07 PM, Thomas Hellstrom wrote:
> On 08/09/2017 08:36 AM, Michel Dänzer wrote:
>> On 08/08/17 03:48 PM, Thomas Hellstrom wrote:
>>> Implement back-to-fake-front flips, Fix EGL_BUFFER_PRESERVED
>>> path. Implement dri3 support for GLX_SWAP_EXCHANGE_OML and
>>> GLX_SWAP_COPY_OML.
>>> 
>>> The back-to-fake-front flips will save a full buffer copy in the
>>> case of a fake front being enabled and GLX_SWAP_UNDEFINED_OML.
>>> 
>>> Support for EGL_BUFFER_PRESERVED and GLX_SWAP_X_OML are mostly
>>> useful for things like glretrace if traces are capured with
>>> applications relying on a specific swapbuffer behavior.
>>> 
>>> The EGL_BUFFER_PRESERVED path previously made sure the present
>>> was done as a copy, but there was nothing making sure that after
>>> the present, the same back buffer was chosen. This has now been
>>> changed so that if the previous back buffer is idle, we reuse it.
>>> Otherwise we grab a new and copy the contents and buffer age from
>>> the previous back buffer. Server side flips are allowed.
>>> 
>>> GLX_SWAP_COPY_OML will behave like EGL_BUFFER_PRESERVED.
>>> 
>>> GLX_SWAP_EXCHANGE_OML will behave similarly, except that we try
>>> to reuse the previous fake front as the new back buffer if it's
>>> idle. If not, we grab a new back buffer and copy the contents and
>>> buffer age from the old fake front.
>> I'm not sure it's worth copying the contents of the desired next
>> back buffer to a different one and using that instead. There might
>> be cases where doing so results in lower performance than simply
>> using the desired back buffer anyway. Have you made any
>> measurements WRT this?
> 
> Agreed. I haven't done any measurements but my reasoning was that 
> probably any performance loss would be mostly associated with the 
> allocating itself, hence a short-lived problem once enough buffers
> are allocated. The copying would, at least on modern tiling
> hardware, probably not be a big loss since we don't flush.
> 
> 
>> With EGL_BUFFER_PRESERVED/GLX_SWAP_COPY_OML, always re-using the
>> same back buffer means that the client only needs to allocate one
>> back buffer, resulting in lower graphics memory consumption.
>> 
>> 
> True. But there are some implications:
> 
> First, with GLX_SWAP_COPY_OML, reusing the back buffer means we need
> to disable server-side page-flipping, otherwise the buffer would
> never be idle. Second, if we have a fake front, there will be at
> least one copy anyway. So in essence we need to disable server-side
> page-flipping and might not gain anything in the end if using a fake
> front.
> 
> With GLX_SWAP_EXCHANGE_OML, we'd be reusing the old fake front which 
> will, after a delay, always be idle. Also we don't need an extra copy
> so the only loss will be a delay which might impact
> latency-sensitive applications. I don't expect SWAP_EXCHANGE will be
> used much anyway and even if we enable it in st/dri, it won't be
> advertised until the server side AIGLX starts to use the image loader
> extension or we rewrite the GLX fbconfig selection.

Okay, you've convinced me. Any chance this patch can be split up as well
though?


> What do you think of adding a driConf option to select "wait for 
> backbuffer idle in swapbuffers"

Seems like overkill. Let's wait for actual problem reports before
considering that.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/4] dri: Introduce SWAP_METHOD tokens

[Michel Dänzer]

On 09/08/17 06:53 PM, Thomas Hellstrom wrote:
> We shouldn't be using GLX tokens in the dri subsystem, so define dri
> SWAP_METHOD tokens and translate when necessary. Unfortunately the X server
> uses the dri swap method value untranslated as the GLX fbconfig swapMethod,
> so we can't enumerate these tokens arbitrarily, but rather need to make them
> have the same values as the corresponding GLX tokens.
> 
> Signed-off-by: Thomas Hellstrom 

[...]

> diff --git a/src/gallium/state_trackers/dri/dri_screen.c 
> b/src/gallium/state_trackers/dri/dri_screen.c
> index 406e97d..1d9f441 100644
> --- a/src/gallium/state_trackers/dri/dri_screen.c
> +++ b/src/gallium/state_trackers/dri/dri_screen.c
> @@ -156,7 +156,8 @@ dri_fill_in_modes(struct dri_screen *screen)
> boolean mixed_color_depth;
>  
> static const GLenum back_buffer_modes[] = {
> -  GLX_NONE, GLX_SWAP_UNDEFINED_OML, GLX_SWAP_COPY_OML
> +  __DRI_ATTRIB_SWAP_NONE, __DRI_ATTRIB_SWAP_UNDEFINED,
> +  __DRI_ATTRIB_SWAP_COPY
> };

BTW, does this array need to include __DRI_ATTRIB_SWAP_EXCHANGE as well,
or is it possible to use GLX_SWAP_EXCHANGE_OML anyway?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/4] dri: Fix __DRIconfig reporting of __DRI_ATTRIB_SWAP_METHOD

[Michel Dänzer]

On 09/08/17 06:53 PM, Thomas Hellstrom wrote:
> The attribMap had two entries for this attribute, and
> driGetConfigAttribIndex didn't return a proper value for this attribute.
> Fix this, and also make sure we return SWAP_UNDEFINED for single-buffer
> configs as required by the GLX_OML_swap_method spec.
> 
> Finally bump the dri core extension version to 2, indicating that we
> correctly report __DRI_ATTRIB_SWAP_METHOD.
> 
> Signed-off-by: Thomas Hellstrom 

[...]

> diff --git a/src/mesa/drivers/dri/common/utils.c 
> b/src/mesa/drivers/dri/common/utils.c
> index c37d446..f3ea61e 100644
> --- a/src/mesa/drivers/dri/common/utils.c
> +++ b/src/mesa/drivers/dri/common/utils.c
> @@ -284,8 +284,9 @@ driCreateConfigs(mesa_format format,
>   modes->transparentIndex = GLX_DONT_CARE;
>   modes->rgbMode = GL_TRUE;
>  
> - if ( db_modes[i] == GLX_NONE ) {
> + if ( db_modes[i] == GLX_NONE) {
>   modes->doubleBufferMode = GL_FALSE;
> +modes->swapMethod = GLX_SWAP_UNDEFINED_OML;

The indentation of the added line doesn't match that of the previous
line. Patch 4 further changes this code (which is also what Brian's
comment was about AFAICT) to be even less consistently indented compared
to the surrounding code.

With that fixed in both patches, the series is

Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] ac: fail shader compilation if libelf is replaced by an incompatible version

[Michel Dänzer]

On 10/08/17 05:34 AM, Marek Olšák wrote:
> From: Marek Olšák 
> 
> UE4Editor has this issue.
> 
> This commit prevents hangs (release build) or assertion failures (debug
> build).
> 
> Cc: 17.2 

Can't this go to 17.1, assuming there will be another 17.1 release? Anyway,

Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/8] glsl: clone builtin function constants

[Kenneth Graunke]

On Tuesday, August 8, 2017 8:34:06 PM PDT Timothy Arceri wrote:
> f81ede469910d fixed a problem with shaders including IR that was
> owned by builtins. However the approach of cloning the whole
> function each time we referenced it lead to a significant
> reduction in the GLSL IR compiler performance.
> 
> Everything was already cloned when inlining the function, as
> far as I can tell this is the only place where we are grabbing
> IR owned by the builtins without cloning it.
> 
> The double free can be easily reproduced by compiling the
> Deus Ex: Mankind Divided shaders with shader db, and then
> compiling them again after deleting mesa's shader cache
> index file. This will cause optimisations to never be performed
> on the IR and which presumably caused this issue to be hidden
> under normal circumstances.
> 
> Cc: Kenneth Graunke 
> Cc: Lionel Landwerlin 
> 
> Tested-by: Dieter Nützel 
> ---
>  src/compiler/glsl/ast_function.cpp | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compiler/glsl/ast_function.cpp 
> b/src/compiler/glsl/ast_function.cpp
> index f7e90fba5b..73c4c0df7b 100644
> --- a/src/compiler/glsl/ast_function.cpp
> +++ b/src/compiler/glsl/ast_function.cpp
> @@ -514,21 +514,29 @@ generate_call(exec_list *instructions, 
> ir_function_signature *sig,
>  *   return 0 when evaluated inside an initializer with an 
> argument
>  *   that is a constant expression."
>  *
>  * If the function call is a constant expression, don't generate any
>  * instructions; just generate an ir_constant.
>  */
> if (state->is_version(120, 100)) {
>ir_constant *value = sig->constant_expression_value(actual_parameters,
>NULL);

NAK on both 5 and 6.  Without cloning, "sig" is part of a global
"built-ins" memory context that's shared across multiple shaders.

Calling sig->constant_expression_value() will allocate new ir_constant
objects out of ralloc_parent(something in the same context as sig) - the
global context.  Ralloc is not thread-safe.  If you compile multiple
shaders concurrently, simply calling this will corrupt the linked list
in the ralloc context for built-ins, causing segmentation faults.

It's a really subtle bug, one that Lionel, Matt, and I spent a lot of
time tracking down.  Matt and I never figured it out - only Lionel
managed.  We were able to reproduce this by running shader-db on a
system with 36 cores / 72 threads.

I do dislike the cloning, though.  I think the best approach here would be
to make ::constant_expression_value() take a memory context, and allocate
all memory out of that, rather than ralloc_parent().  That way, we can make
generate_inline supply "ctx" here, so the new constant comes out of the
compile-local memory context.

With that changed, I think it would be safe to drop the cloning.

>if (value != NULL) {
> - return value;
> + if (sig->is_builtin()) {
> +/* This value belongs to a builtin so we must clone it to avoid
> + * race conditions when freeing shaders and destorying the
> + * context.
> + */
> +return value->clone(ctx, NULL);
> + } else {
> +return value;
> + }
>}
> }
>  
> ir_dereference_variable *deref = NULL;
> if (!sig->return_type->is_void()) {
>/* Create a new temporary to hold the return value. */
>char *const name = ir_variable::temporaries_allocate_names
>   ? ralloc_asprintf(ctx, "%s_retval", sig->function_name())
>   : NULL;
>  
> 



signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965/blorp: Correct type of src_format in call to intel_miptree_texture_aux_usage

[Jason Ekstrand]

oof...  Have you run this through Jenkins?  It should be ok, but it will be
a functional change.  It's a good change, but it is a change.  Also, this
should probably get CCd to stable.

--Jason

On Wed, Aug 9, 2017 at 3:52 PM, Scott D Phillips  wrote:

> intel_miptree_texture_aux_usage() takes an isl_format, but we are
> passing a mesa_format. clang warns:
>
>  brw_blorp.c:305:52: warning: implicit conversion from enumeration
> type 'mesa_format' to different enumeration type
> 'enum isl_format' [-Wenum-conversion]
>intel_miptree_texture_aux_usage(brw, src_mt, src_format);
>~~~  ^~
>
> Fixes: fc1639e46d ("i965/blorp: Use texture/render_aux_usage for blits")
> ---
>  src/mesa/drivers/dri/i965/brw_blorp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> index 446f507619..d8e48064e3 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -301,8 +301,9 @@ brw_blorp_blit_miptrees(struct brw_context *brw,
>src_format = dst_format = MESA_FORMAT_R_FLOAT32;
> }
>
> +   enum isl_format src_isl_format = brw_isl_format_for_mesa_
> format(src_format);
> enum isl_aux_usage src_aux_usage =
> -  intel_miptree_texture_aux_usage(brw, src_mt, src_format);
> +  intel_miptree_texture_aux_usage(brw, src_mt, src_isl_format);
> /* We do format workarounds for some depth formats so we can't reliably
>  * sample with HiZ.  One of these days, we should fix that.
>  */
> --
> 2.13.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] i965/blorp: Correct type of src_format in call to intel_miptree_texture_aux_usage

[Scott D Phillips]

intel_miptree_texture_aux_usage() takes an isl_format, but we are
passing a mesa_format. clang warns:

 brw_blorp.c:305:52: warning: implicit conversion from enumeration
type 'mesa_format' to different enumeration type
'enum isl_format' [-Wenum-conversion]
   intel_miptree_texture_aux_usage(brw, src_mt, src_format);
   ~~~  ^~

Fixes: fc1639e46d ("i965/blorp: Use texture/render_aux_usage for blits")
---
 src/mesa/drivers/dri/i965/brw_blorp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c 
b/src/mesa/drivers/dri/i965/brw_blorp.c
index 446f507619..d8e48064e3 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.c
+++ b/src/mesa/drivers/dri/i965/brw_blorp.c
@@ -301,8 +301,9 @@ brw_blorp_blit_miptrees(struct brw_context *brw,
   src_format = dst_format = MESA_FORMAT_R_FLOAT32;
}
 
+   enum isl_format src_isl_format = brw_isl_format_for_mesa_format(src_format);
enum isl_aux_usage src_aux_usage =
-  intel_miptree_texture_aux_usage(brw, src_mt, src_format);
+  intel_miptree_texture_aux_usage(brw, src_mt, src_isl_format);
/* We do format workarounds for some depth formats so we can't reliably
 * sample with HiZ.  One of these days, we should fix that.
 */
-- 
2.13.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] swr/rast: [rasterizer core] fix invalid casting for calls to Interlocked* functions

[Tim Rowley]

CID: 1416243, 1416244, 1416255
CC: mesa-sta...@lists.freedesktop.org
---
 src/gallium/drivers/swr/rasterizer/core/api.cpp | 2 +-
 src/gallium/drivers/swr/rasterizer/core/context.h   | 8 
 src/gallium/drivers/swr/rasterizer/core/threads.cpp | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/gallium/drivers/swr/rasterizer/core/api.cpp 
b/src/gallium/drivers/swr/rasterizer/core/api.cpp
index 8dc9ac2..ccb6dfb 100644
--- a/src/gallium/drivers/swr/rasterizer/core/api.cpp
+++ b/src/gallium/drivers/swr/rasterizer/core/api.cpp
@@ -189,7 +189,7 @@ void QueueWork(SWR_CONTEXT *pContext)
 
 if (IsDraw)
 {
-InterlockedIncrement((volatile long*)>drawsOutstandingFE);
+InterlockedIncrement(>drawsOutstandingFE);
 }
 
 _ReadWriteBarrier();
diff --git a/src/gallium/drivers/swr/rasterizer/core/context.h 
b/src/gallium/drivers/swr/rasterizer/core/context.h
index 131b3cb..bcd5801 100644
--- a/src/gallium/drivers/swr/rasterizer/core/context.h
+++ b/src/gallium/drivers/swr/rasterizer/core/context.h
@@ -409,12 +409,12 @@ struct DRAW_CONTEXT
 booldependent;  // Backend work is dependent on all 
previous BE
 boolisCompute;  // Is this DC a compute context?
 boolcleanupState;   // True if this is the last draw using an 
entry in the state ring.
-volatile bool   doneFE; // Is FE work done for this draw?
 
 FE_WORK FeWork;
 
+volatile OSALIGNLINE(bool)   doneFE; // Is FE work done for 
this draw?
 volatile OSALIGNLINE(uint32_t)   FeLock;
-volatile int32_tthreadsDone;
+volatile OSALIGNLINE(uint32_t)   threadsDone;
 
 SYNC_DESC   retireCallback; // Call this func when this DC is retired.
 };
@@ -503,9 +503,9 @@ struct SWR_CONTEXT
 // Scratch space for workers.
 uint8_t** ppScratch;
 
-volatile int32_t  drawsOutstandingFE;
+volatile OSALIGNLINE(uint32_t)  drawsOutstandingFE;
 
-CachingAllocator cachingArenaAllocator;
+OSALIGNLINE(CachingAllocator) cachingArenaAllocator;
 uint32_t frameCount;
 
 uint32_t lastFrameChecked;
diff --git a/src/gallium/drivers/swr/rasterizer/core/threads.cpp 
b/src/gallium/drivers/swr/rasterizer/core/threads.cpp
index 70bde02..b704d23 100644
--- a/src/gallium/drivers/swr/rasterizer/core/threads.cpp
+++ b/src/gallium/drivers/swr/rasterizer/core/threads.cpp
@@ -393,7 +393,7 @@ INLINE void ExecuteCallbacks(SWR_CONTEXT* pContext, 
uint32_t workerId, DRAW_CONT
 // inlined-only version
 INLINE int32_t CompleteDrawContextInl(SWR_CONTEXT* pContext, uint32_t 
workerId, DRAW_CONTEXT* pDC)
 {
-int32_t result = InterlockedDecrement((volatile long*)>threadsDone);
+int32_t result = 
static_cast(InterlockedDecrement(>threadsDone));
 SWR_ASSERT(result >= 0);
 
 AR_FLUSH(pDC->drawId);
@@ -639,7 +639,7 @@ INLINE void CompleteDrawFE(SWR_CONTEXT* pContext, uint32_t 
workerId, DRAW_CONTEX
 _mm_mfence();
 pDC->doneFE = true;
 
-InterlockedDecrement((volatile long*)>drawsOutstandingFE);
+InterlockedDecrement(>drawsOutstandingFE);
 }
 
 void WorkOnFifoFE(SWR_CONTEXT *pContext, uint32_t workerId, uint32_t 
)
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 09/12] radeon: use common pipe_screen ref counting

[Rob Herring]

On Wed, Aug 9, 2017 at 3:19 PM, Marek Olšák  wrote:
> You already know I can't accept the amdgpu part.

You don't agree with my explanation in the other patch?

> Anyway, I don't know if it's possible to keep the radeon part of the
> patch, because there is some shared code you would need to keep as
> well.

Yes, I think it should be possible.

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] ac: fail shader compilation if libelf is replaced by an incompatible version

[Marek Olšák]

From: Marek Olšák 

UE4Editor has this issue.

This commit prevents hangs (release build) or assertion failures (debug
build).

Cc: 17.2 
---
 src/amd/common/ac_binary.c  | 12 ++--
 src/amd/common/ac_binary.h  |  2 +-
 src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c |  5 -
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/amd/common/ac_binary.c b/src/amd/common/ac_binary.c
index 618b5cf..1bf52c7 100644
--- a/src/amd/common/ac_binary.c
+++ b/src/amd/common/ac_binary.c
@@ -102,59 +102,66 @@ static void parse_relocs(Elf *elf, Elf_Data *relocs, 
Elf_Data *symbols,
gelf_getrel(relocs, i, );
gelf_getsym(symbols, GELF_R_SYM(rel.r_info), );
symbol_name = elf_strptr(elf, symbol_sh_link, symbol.st_name);
 
reloc->offset = rel.r_offset;
strncpy(reloc->name, symbol_name, sizeof(reloc->name)-1);
reloc->name[sizeof(reloc->name)-1] = 0;
}
 }
 
-void ac_elf_read(const char *elf_data, unsigned elf_size,
+bool ac_elf_read(const char *elf_data, unsigned elf_size,
 struct ac_shader_binary *binary)
 {
char *elf_buffer;
Elf *elf;
Elf_Scn *section = NULL;
Elf_Data *symbols = NULL, *relocs = NULL;
size_t section_str_index;
unsigned symbol_sh_link = 0;
+   bool success = true;
 
/* One of the libelf implementations
 * (http://www.mr511.de/software/english.htm) requires calling
 * elf_version() before elf_memory().
 */
elf_version(EV_CURRENT);
elf_buffer = MALLOC(elf_size);
memcpy(elf_buffer, elf_data, elf_size);
 
elf = elf_memory(elf_buffer, elf_size);
 
elf_getshdrstrndx(elf, _str_index);
 
while ((section = elf_nextscn(elf, section))) {
const char *name;
Elf_Data *section_data = NULL;
GElf_Shdr section_header;
if (gelf_getshdr(section, _header) != _header) {
fprintf(stderr, "Failed to read ELF section header\n");
-   return;
+   success = false;
+   break;
}
name = elf_strptr(elf, section_str_index, 
section_header.sh_name);
if (!strcmp(name, ".text")) {
section_data = elf_getdata(section, section_data);
binary->code_size = section_data->d_size;
binary->code = MALLOC(binary->code_size * 
sizeof(unsigned char));
memcpy(binary->code, section_data->d_buf, 
binary->code_size);
} else if (!strcmp(name, ".AMDGPU.config")) {
section_data = elf_getdata(section, section_data);
binary->config_size = section_data->d_size;
+   if (!binary->config_size) {
+   fprintf(stderr, ".AMDGPU.config is empty!\n");
+   success = false;
+   break;
+   }
binary->config = MALLOC(binary->config_size * 
sizeof(unsigned char));
memcpy(binary->config, section_data->d_buf, 
binary->config_size);
} else if (!strcmp(name, ".AMDGPU.disasm")) {
/* Always read disassembly if it's available. */
section_data = elf_getdata(section, section_data);
binary->disasm_string = strndup(section_data->d_buf,
section_data->d_size);
} else if (!strncmp(name, ".rodata", 7)) {
section_data = elf_getdata(section, section_data);
binary->rodata_size = section_data->d_size;
@@ -179,20 +186,21 @@ void ac_elf_read(const char *elf_data, unsigned elf_size,
FREE(elf_buffer);
 
/* Cache the config size per symbol */
if (binary->global_symbol_count) {
binary->config_size_per_symbol =
binary->config_size / binary->global_symbol_count;
} else {
binary->global_symbol_count = 1;
binary->config_size_per_symbol = binary->config_size;
}
+   return success;
 }
 
 const unsigned char *ac_shader_binary_config_start(
const struct ac_shader_binary *binary,
uint64_t symbol_offset)
 {
unsigned i;
for (i = 0; i < binary->global_symbol_count; ++i) {
if (binary->global_symbol_offsets[i] == symbol_offset) {
unsigned offset = i * binary->config_size_per_symbol;
diff --git a/src/amd/common/ac_binary.h b/src/amd/common/ac_binary.h
index a784a72..45f554e 100644
--- a/src/amd/common/ac_binary.h
+++ 

Re: [Mesa-dev] [PATCH 4/7] configure.ac: Introduce HAVE_ARM_ASM/HAVE_AARCH64_ASM and the -D flags.

[Rob Herring]

On Tue, Aug 8, 2017 at 3:19 PM, Eric Anholt  wrote:
> I've been trying to get away without these conditionals in vc4, but it
> meant compiling extra unused code on x86, and build failing on ARMv6.
> ---
>  Android.common.mk |  8 
>  configure.ac  | 24 
>  2 files changed, 32 insertions(+)
>
> diff --git a/Android.common.mk b/Android.common.mk
> index 6bd30816bc41..435e3da40a5f 100644
> --- a/Android.common.mk
> +++ b/Android.common.mk
> @@ -87,6 +87,14 @@ LOCAL_CFLAGS += \
> -DUSE_X86_ASM
>
>  endif
> +ifeq ($(TARGET_ARCH),arm)
> +LOCAL_CFLAGS += \
> +   -DUSE_ARM_ASM
> +endif
> +ifeq ($(TARGET_ARCH),aarch64)
> +LOCAL_CFLAGS += \
> +   -DUSE_AARCH64_ASM
> +endif

All this can just be:

LOCAL_CFLAGS_arm +=  -DUSE_ARM_ASM
LOCAL_CFLAGS_arm64 += -DUSE_AARCH64_ASM

It also wouldn't have worked for dual 32/64 bit builds which arm64 builds are.

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/7] broadcom/vc4: Build the vc4_tiling_lt_neon.c with -mfpu=neon on ARM.

[Rob Herring]

On Tue, Aug 8, 2017 at 3:19 PM, Eric Anholt  wrote:
> If you don't pass this, the compiler refuses to compile the assembly for
> pre-v7 CPUs.  This also keeps us from building identical, non-NEON code on
> aarch64 and x86.
>
> Fixes: a373f77662c5 ("vc4: Use a wrapper file to set VC4_BUILD_NEON instead 
> of CFLAGS.")
> ---
>  src/gallium/drivers/vc4/Android.mk   | 10 ++
>  src/gallium/drivers/vc4/Makefile.am  |  7 +++
>  src/gallium/drivers/vc4/Makefile.sources |  3 ++-
>  src/gallium/drivers/vc4/vc4_tiling.h | 17 +++--
>  4 files changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/src/gallium/drivers/vc4/Android.mk 
> b/src/gallium/drivers/vc4/Android.mk
> index 84bfaa839f9e..eaf66ebe8cb9 100644
> --- a/src/gallium/drivers/vc4/Android.mk
> +++ b/src/gallium/drivers/vc4/Android.mk
> @@ -23,6 +23,15 @@ LOCAL_PATH := $(call my-dir)
>  # get C_SOURCES
>  include $(LOCAL_PATH)/Makefile.sources
>
> +# First, the architecture-specific library.
> +ifeq ($(TARGET_ARCH),arm)

This should probably be more specific. However, Android doesn't
actually support v6 builds, but it does support armv5te builds. IOW,
for the case you care about, you can't actually build.

> +include $(CLEAR_VARS)
> +LOCAL_SRC_FILES := $(NEON_C_SOURCES)
> +LOCAL_CFLAGS := -mfpu=neon
> +LOCAL_MODULE := libmesa_broadcom_vc4_neon
> +BROADCOM_VC4_NEON_LIB := $(LOCAL_MODULE)

You need an "include $(BUILD_STATIC_LIBRARY)" here to actually build the lib.

> +endif
> +
>  include $(CLEAR_VARS)
>
>  LOCAL_SRC_FILES := \
> @@ -37,6 +46,7 @@ LOCAL_STATIC_LIBRARIES := \
> libmesa_nir
>
>  LOCAL_WHOLE_STATIC_LIBRARIES := \
> +   $(BROADCOM_VC4_NEON_LIB) \
> libmesa_broadcom_cle \
> libmesa_broadcom_genxml
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] isl: Validate row pitch of stencil surfaces.

[Jason Ekstrand]

On Wed, Aug 9, 2017 at 1:09 PM, Kenneth Graunke 
wrote:

> Also, silence an obnoxious finishme that started occurring for all
> GL applications which use stencil after the i965 ISL conversion.
> ---
>  src/intel/isl/isl.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> index 6b4203d79d2..c35116214c8 100644
> --- a/src/intel/isl/isl.c
> +++ b/src/intel/isl/isl.c
> @@ -1367,8 +1367,10 @@ isl_calc_row_pitch(const struct isl_device *dev,
> !pitch_in_range(row_pitch, _3DSTATE_HIER_DEPTH_BUFFER_
> SurfacePitch_bits(dev->info)))
>return false;
>
> -   if (surf_info->usage & ISL_SURF_USAGE_STENCIL_BIT)
> -  isl_finishme("validate row pitch of stencil surfaces");
> +   if (dev->use_separate_stencil &&
> +   (surf_info->usage & ISL_SURF_USAGE_STENCIL_BIT) &&
> +   !pitch_in_range(row_pitch, _3DSTATE_STENCIL_BUFFER_
> SurfacePitch_bits(dev->info)))
>

Topi sent the same patch.  This doesn't work on gen4.


> +  return false;
>
>   done:
> *out_row_pitch = row_pitch;
> --
> 2.14.0
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 09/12] radeon: use common pipe_screen ref counting

[Marek Olšák]

You already know I can't accept the amdgpu part.

Anyway, I don't know if it's possible to keep the radeon part of the
patch, because there is some shared code you would need to keep as
well.

Marek

On Tue, Aug 8, 2017 at 12:58 AM, Rob Herring  wrote:
> Use the common pipe_screen ref counting.
>
> radeon uses the common pipe_screen_{un}reference functions, but amdgpu
> is unique in its hashing the dev pointer rather than the fd, so the
> common fd hashing cannot be used. However, the same reference counting
> can be used instead of the private one. The mutex can be dropped as the
> pipe loader serializes the create_screen() and destroy() calls.
>
> Signed-off-by: Rob Herring 
> Cc: "Marek Olšák" 
> Cc: Ilia Mirkin 
> ---
>  src/gallium/drivers/r300/r300_screen.c|  3 -
>  src/gallium/drivers/r600/r600_pipe.c  |  6 --
>  src/gallium/drivers/radeon/radeon_winsys.h|  8 ---
>  src/gallium/drivers/radeonsi/si_pipe.c|  3 -
>  src/gallium/include/pipe/p_screen.h   |  2 +
>  src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c | 44 ++---
>  src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h |  1 -
>  src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 79 
> ++-
>  src/gallium/winsys/radeon/drm/radeon_drm_winsys.h |  1 -
>  9 files changed, 13 insertions(+), 134 deletions(-)
>
> diff --git a/src/gallium/drivers/r300/r300_screen.c 
> b/src/gallium/drivers/r300/r300_screen.c
> index ff68ffd7bc65..6991e8396638 100644
> --- a/src/gallium/drivers/r300/r300_screen.c
> +++ b/src/gallium/drivers/r300/r300_screen.c
> @@ -696,9 +696,6 @@ static void r300_destroy_screen(struct pipe_screen* 
> pscreen)
>  struct r300_screen* r300screen = r300_screen(pscreen);
>  struct radeon_winsys *rws = radeon_winsys(pscreen);
>
> -if (rws && !rws->unref(rws))
> -  return;
> -
>  mtx_destroy(>cmask_mutex);
>  slab_destroy_parent(>pool_transfers);
>
> diff --git a/src/gallium/drivers/r600/r600_pipe.c 
> b/src/gallium/drivers/r600/r600_pipe.c
> index 023f1b4bd14f..c0f7312e956d 100644
> --- a/src/gallium/drivers/r600/r600_pipe.c
> +++ b/src/gallium/drivers/r600/r600_pipe.c
> @@ -612,12 +612,6 @@ static void r600_destroy_screen(struct pipe_screen* 
> pscreen)
>  {
> struct r600_screen *rscreen = (struct r600_screen *)pscreen;
>
> -   if (!rscreen)
> -   return;
> -
> -   if (!rscreen->b.ws->unref(rscreen->b.ws))
> -   return;
> -
> if (rscreen->global_pool) {
> compute_memory_pool_delete(rscreen->global_pool);
> }
> diff --git a/src/gallium/drivers/radeon/radeon_winsys.h 
> b/src/gallium/drivers/radeon/radeon_winsys.h
> index 351edcd76f98..6ccd2ddbb0b2 100644
> --- a/src/gallium/drivers/radeon/radeon_winsys.h
> +++ b/src/gallium/drivers/radeon/radeon_winsys.h
> @@ -230,14 +230,6 @@ struct radeon_winsys {
>  struct pipe_screen *screen;
>
>  /**
> - * Decrement the winsys reference count.
> - *
> - * \param ws  The winsys this function is called for.
> - * \returnTrue if the winsys and screen should be destroyed.
> - */
> -bool (*unref)(struct radeon_winsys *ws);
> -
> -/**
>   * Destroy this winsys.
>   *
>   * \param wsThe winsys this function is called from.
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
> b/src/gallium/drivers/radeonsi/si_pipe.c
> index 395853c7d9f3..881ed0c58339 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -843,9 +843,6 @@ static void si_destroy_screen(struct pipe_screen* pscreen)
> };
> unsigned i;
>
> -   if (!sscreen->b.ws->unref(sscreen->b.ws))
> -   return;
> -
> util_queue_destroy(>shader_compiler_queue);
> util_queue_destroy(>shader_compiler_queue_low_priority);
>
> diff --git a/src/gallium/include/pipe/p_screen.h 
> b/src/gallium/include/pipe/p_screen.h
> index dbb14ef882c0..8f866f83d763 100644
> --- a/src/gallium/include/pipe/p_screen.h
> +++ b/src/gallium/include/pipe/p_screen.h
> @@ -71,6 +71,8 @@ struct driOptionCache;
>  struct pipe_screen {
> struct pipe_reference reference;
> int fd;
> +   void *ws;
> +
> void (*destroy)( struct pipe_screen * );
>
> const char *(*get_name)( struct pipe_screen * );
> diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c 
> b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
> index 26c7dacb9df4..258872388ce3 100644
> --- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
> +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
> @@ -48,7 +48,6 @@
>  #endif
>
>  static struct util_hash_table *dev_tab = NULL;
> -static mtx_t dev_tab_mutex = _MTX_INITIALIZER_NP;
>
>  /* Helper function to do the ioctls needed for setup and init. */
>  static bool do_winsys_init(struct amdgpu_winsys *ws, int fd)
> @@ -97,6 +96,7 @@ static void 

Re: [Mesa-dev] [PATCH v5 03/12] gallium: add common pipe_screen reference counting functions

[Marek Olšák]

On Tue, Aug 8, 2017 at 12:58 AM, Rob Herring  wrote:
> In order to prevent multiple pipe_screens being created in the same
> process, lookup of the DRM FD and reference counting of the pipe_screen
> are needed. Several implementations of this exist in various gallium
> drivers/winsys already. This creates a common version which is opt-in
> for winsys implementations.
>
> The callers of pipe_screen_{un}reference() functions are responsible for
> serializing the calls. Currently, this is done by the pipe-loader mutex.
>
> Signed-off-by: Rob Herring 
> ---
>  src/gallium/auxiliary/Makefile.sources |   2 +
>  src/gallium/auxiliary/util/u_screen.c  | 112 
> +
>  src/gallium/auxiliary/util/u_screen.h  |  32 ++
>  src/gallium/include/pipe/p_screen.h|   3 +
>  4 files changed, 149 insertions(+)
>  create mode 100644 src/gallium/auxiliary/util/u_screen.c
>  create mode 100644 src/gallium/auxiliary/util/u_screen.h
>
> diff --git a/src/gallium/auxiliary/Makefile.sources 
> b/src/gallium/auxiliary/Makefile.sources
> index 9ae8e6c8ca53..f84bfec60fe7 100644
> --- a/src/gallium/auxiliary/Makefile.sources
> +++ b/src/gallium/auxiliary/Makefile.sources
> @@ -283,6 +283,8 @@ C_SOURCES := \
> util/u_ringbuffer.h \
> util/u_sampler.c \
> util/u_sampler.h \
> +   util/u_screen.c \
> +   util/u_screen.h \
> util/u_simple_shaders.c \
> util/u_simple_shaders.h \
> util/u_split_prim.h \
> diff --git a/src/gallium/auxiliary/util/u_screen.c 
> b/src/gallium/auxiliary/util/u_screen.c
> new file mode 100644
> index ..dec83b736883
> --- /dev/null
> +++ b/src/gallium/auxiliary/util/u_screen.c
> @@ -0,0 +1,112 @@
> +/*
> + * Copyright 2016-2017 Linaro, Ltd., Rob Herring 
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
> + * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
> + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/**
> + * Functions for managing pipe_screen's
> + */
> +
> +#include 
> +
> +#include "pipe/p_screen.h"
> +#include "util/u_hash_table.h"
> +#include "util/u_inlines.h"
> +#include "util/u_pointer.h"
> +#include "util/u_screen.h"
> +
> +static struct util_hash_table *fd_tab = NULL;
> +
> +static unsigned hash_fd(void *key)
> +{
> +   int fd = pointer_to_intptr(key);
> +   struct stat stat;
> +   fstat(fd, );
> +
> +   return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
> +}
> +
> +static int compare_fd(void *key1, void *key2)
> +{
> +   int fd1 = pointer_to_intptr(key1);
> +   int fd2 = pointer_to_intptr(key2);
> +   struct stat stat1, stat2;
> +   fstat(fd1, );
> +   fstat(fd2, );
> +
> +   return stat1.st_dev != stat2.st_dev ||
> + stat1.st_ino != stat2.st_ino ||
> + stat1.st_rdev != stat2.st_rdev;
> +}
> +
> +struct pipe_screen *
> +pipe_screen_reference(int fd)
> +{
> +   struct pipe_screen *pscreen;
> +
> +   if (!fd_tab) {
> +  fd_tab = util_hash_table_create(hash_fd, compare_fd);
> +  return NULL;
> +   }
> +
> +   pscreen = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
> +   if (pscreen)
> +  pipe_reference(NULL, >reference);
> +
> +   return pscreen;
> +}
> +
> +boolean
> +pipe_screen_unreference(struct pipe_screen *pscreen)
> +{
> +   boolean destroy;
> +
> +   if (!pscreen)
> +  return FALSE;
> +
> +   /* Work-around until all pipe_screens have ref counting */
> +   if (!pipe_is_referenced(>reference)) {
> +  pscreen->destroy(pscreen);
> +  return TRUE;
> +   }
> +
> +   destroy = pipe_reference(>reference, NULL);
> +   if (destroy) {
> +  int fd = pscreen->fd;
> +
> +  pscreen->destroy(pscreen);
> +
> +  if (fd >= 0) {
> + util_hash_table_remove(fd_tab, intptr_to_pointer(fd));
> + close(fd);
> +  }
> +   }
> +   return destroy;
> +}
> +
> +
> +void pipe_screen_reference_init(struct pipe_screen *pscreen, int fd)
> +{
> +   pscreen->fd = fd;
> +   

Re: [Mesa-dev] [PATCH v5 04/12] gallium: use pipe_screen_unreference to destroy screen in debug wrappers

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Tue, Aug 8, 2017 at 12:58 AM, Rob Herring  wrote:
> Use pipe_screen_unreference as it will call pipe_screen->destroy() when
> the pipe_screen is no longer referenced.
>
> Signed-off-by: Rob Herring 
> ---
>  src/gallium/drivers/ddebug/dd_screen.c | 3 ++-
>  src/gallium/drivers/noop/noop_pipe.c   | 3 ++-
>  src/gallium/drivers/rbug/rbug_screen.c | 3 ++-
>  src/gallium/drivers/trace/tr_screen.c  | 3 ++-
>  4 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/drivers/ddebug/dd_screen.c 
> b/src/gallium/drivers/ddebug/dd_screen.c
> index 14e6f6b011f7..806846573234 100644
> --- a/src/gallium/drivers/ddebug/dd_screen.c
> +++ b/src/gallium/drivers/ddebug/dd_screen.c
> @@ -28,6 +28,7 @@
>  #include "dd_pipe.h"
>  #include "dd_public.h"
>  #include "util/u_memory.h"
> +#include "util/u_screen.h"
>  #include 
>
>
> @@ -314,7 +315,7 @@ dd_screen_destroy(struct pipe_screen *_screen)
> struct dd_screen *dscreen = dd_screen(_screen);
> struct pipe_screen *screen = dscreen->screen;
>
> -   screen->destroy(screen);
> +   pipe_screen_unreference(screen);
> FREE(dscreen);
>  }
>
> diff --git a/src/gallium/drivers/noop/noop_pipe.c 
> b/src/gallium/drivers/noop/noop_pipe.c
> index d1e795dab163..28c095058a36 100644
> --- a/src/gallium/drivers/noop/noop_pipe.c
> +++ b/src/gallium/drivers/noop/noop_pipe.c
> @@ -29,6 +29,7 @@
>  #include "util/u_memory.h"
>  #include "util/u_inlines.h"
>  #include "util/u_format.h"
> +#include "util/u_screen.h"
>  #include "util/u_upload_mgr.h"
>  #include "noop_public.h"
>
> @@ -431,7 +432,7 @@ static void noop_destroy_screen(struct pipe_screen 
> *screen)
> struct noop_pipe_screen *noop_screen = (struct noop_pipe_screen*)screen;
> struct pipe_screen *oscreen = noop_screen->oscreen;
>
> -   oscreen->destroy(oscreen);
> +   pipe_screen_unreference(oscreen);
> FREE(screen);
>  }
>
> diff --git a/src/gallium/drivers/rbug/rbug_screen.c 
> b/src/gallium/drivers/rbug/rbug_screen.c
> index b12f029b3ea1..dc36425cc45f 100644
> --- a/src/gallium/drivers/rbug/rbug_screen.c
> +++ b/src/gallium/drivers/rbug/rbug_screen.c
> @@ -30,6 +30,7 @@
>  #include "pipe/p_state.h"
>  #include "util/u_memory.h"
>  #include "util/u_debug.h"
> +#include "util/u_screen.h"
>  #include "util/simple_list.h"
>
>  #include "rbug_public.h"
> @@ -45,7 +46,7 @@ rbug_screen_destroy(struct pipe_screen *_screen)
> struct rbug_screen *rb_screen = rbug_screen(_screen);
> struct pipe_screen *screen = rb_screen->screen;
>
> -   screen->destroy(screen);
> +   pipe_screen_unreference(screen);
>
> FREE(rb_screen);
>  }
> diff --git a/src/gallium/drivers/trace/tr_screen.c 
> b/src/gallium/drivers/trace/tr_screen.c
> index e56434c5bda6..697854185d54 100644
> --- a/src/gallium/drivers/trace/tr_screen.c
> +++ b/src/gallium/drivers/trace/tr_screen.c
> @@ -27,6 +27,7 @@
>
>  #include "util/u_format.h"
>  #include "util/u_memory.h"
> +#include "util/u_screen.h"
>  #include "util/simple_list.h"
>
>  #include "tr_dump.h"
> @@ -488,7 +489,7 @@ trace_screen_destroy(struct pipe_screen *_screen)
> trace_dump_arg(ptr, screen);
> trace_dump_call_end();
>
> -   screen->destroy(screen);
> +   pipe_screen_unreference(screen);
>
> FREE(tr_scr);
>  }
> --
> 2.11.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 03/12] gallium: add common pipe_screen reference counting functions

[Marek Olšák]

OK. This patch is:

Reviewed-by: Marek Olšák 

Marek

On Wed, Aug 9, 2017 at 9:53 PM, Rob Herring  wrote:
> On Wed, Aug 9, 2017 at 2:18 PM, Marek Olšák  wrote:
>> Hi Rob,
>>
>> This is not enough to do screen reference counting. There are primary
>> FDs and render node FDs, and we want to have only one pipe_screen for
>> all kinds of FDs pointing to the same device.
>>
>> Imagine someone creates a pipe_screen with a render node FD, and then
>> creates another pipe_screen with a primary FD. We want to return the
>> same pipe_screen instance in both cases.
>>
>> The pipe_screen should use the render node FD for everything if it was
>> created first with that FD. However, it also needs to keep the primary
>> FD in order to be able to do GEM_FLINK when required.
>>
>> libdrm_amdgpu handles all those cases correctly. It has "fd", which is
>> the main one, but also has "flink_fd" for GEM_FLINK. If "fd" is a
>> primary FD or there is no primary FD, "fd == flink_fd". If "fd" is a
>> render node and a primary FD has also been used to create the device
>> object, "fd" is the render node and "flink_fd" is the primary FD for
>> GEM_FLINK.
>>
>> The code:
>> https://cgit.freedesktop.org/mesa/drm/tree/amdgpu/amdgpu_device.c
>>
>> I can't accept this patch, because it effectively disables that
>> amdgpu_device code.
>
> This patch alone doesn't change any driver. I think you mean patch #8
> which converts amdgpu and which could be dropped (though I'd like to
> keep the radeon winsys parts). However, I believe I've maintained the
> behavior for amdgpu.
>
> The amdgpu winsys does not use the fd in these functions (the screen
> fd will be -1). The only change should be that the mutex is removed
> and the pipe_reference is moved from ws->reference to
> ws->base.screen->reference. It's still using the device in the hash
> table.
>
> Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] isl: Validate row pitch of stencil surfaces.

[Kenneth Graunke]

Also, silence an obnoxious finishme that started occurring for all
GL applications which use stencil after the i965 ISL conversion.
---
 src/intel/isl/isl.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
index 6b4203d79d2..c35116214c8 100644
--- a/src/intel/isl/isl.c
+++ b/src/intel/isl/isl.c
@@ -1367,8 +1367,10 @@ isl_calc_row_pitch(const struct isl_device *dev,
!pitch_in_range(row_pitch, 
_3DSTATE_HIER_DEPTH_BUFFER_SurfacePitch_bits(dev->info)))
   return false;
 
-   if (surf_info->usage & ISL_SURF_USAGE_STENCIL_BIT)
-  isl_finishme("validate row pitch of stencil surfaces");
+   if (dev->use_separate_stencil &&
+   (surf_info->usage & ISL_SURF_USAGE_STENCIL_BIT) &&
+   !pitch_in_range(row_pitch, 
_3DSTATE_STENCIL_BUFFER_SurfacePitch_bits(dev->info)))
+  return false;
 
  done:
*out_row_pitch = row_pitch;
-- 
2.14.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 03/12] gallium: add common pipe_screen reference counting functions

[Rob Herring]

On Wed, Aug 9, 2017 at 2:18 PM, Marek Olšák  wrote:
> Hi Rob,
>
> This is not enough to do screen reference counting. There are primary
> FDs and render node FDs, and we want to have only one pipe_screen for
> all kinds of FDs pointing to the same device.
>
> Imagine someone creates a pipe_screen with a render node FD, and then
> creates another pipe_screen with a primary FD. We want to return the
> same pipe_screen instance in both cases.
>
> The pipe_screen should use the render node FD for everything if it was
> created first with that FD. However, it also needs to keep the primary
> FD in order to be able to do GEM_FLINK when required.
>
> libdrm_amdgpu handles all those cases correctly. It has "fd", which is
> the main one, but also has "flink_fd" for GEM_FLINK. If "fd" is a
> primary FD or there is no primary FD, "fd == flink_fd". If "fd" is a
> render node and a primary FD has also been used to create the device
> object, "fd" is the render node and "flink_fd" is the primary FD for
> GEM_FLINK.
>
> The code:
> https://cgit.freedesktop.org/mesa/drm/tree/amdgpu/amdgpu_device.c
>
> I can't accept this patch, because it effectively disables that
> amdgpu_device code.

This patch alone doesn't change any driver. I think you mean patch #8
which converts amdgpu and which could be dropped (though I'd like to
keep the radeon winsys parts). However, I believe I've maintained the
behavior for amdgpu.

The amdgpu winsys does not use the fd in these functions (the screen
fd will be -1). The only change should be that the mutex is removed
and the pipe_reference is moved from ws->reference to
ws->base.screen->reference. It's still using the device in the hash
table.

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 03/12] gallium: add common pipe_screen reference counting functions

[Marek Olšák]

Hi Rob,

This is not enough to do screen reference counting. There are primary
FDs and render node FDs, and we want to have only one pipe_screen for
all kinds of FDs pointing to the same device.

Imagine someone creates a pipe_screen with a render node FD, and then
creates another pipe_screen with a primary FD. We want to return the
same pipe_screen instance in both cases.

The pipe_screen should use the render node FD for everything if it was
created first with that FD. However, it also needs to keep the primary
FD in order to be able to do GEM_FLINK when required.

libdrm_amdgpu handles all those cases correctly. It has "fd", which is
the main one, but also has "flink_fd" for GEM_FLINK. If "fd" is a
primary FD or there is no primary FD, "fd == flink_fd". If "fd" is a
render node and a primary FD has also been used to create the device
object, "fd" is the render node and "flink_fd" is the primary FD for
GEM_FLINK.

The code:
https://cgit.freedesktop.org/mesa/drm/tree/amdgpu/amdgpu_device.c

I can't accept this patch, because it effectively disables that
amdgpu_device code.

Marek


On Tue, Aug 8, 2017 at 12:58 AM, Rob Herring  wrote:
> In order to prevent multiple pipe_screens being created in the same
> process, lookup of the DRM FD and reference counting of the pipe_screen
> are needed. Several implementations of this exist in various gallium
> drivers/winsys already. This creates a common version which is opt-in
> for winsys implementations.
>
> The callers of pipe_screen_{un}reference() functions are responsible for
> serializing the calls. Currently, this is done by the pipe-loader mutex.
>
> Signed-off-by: Rob Herring 
> ---
>  src/gallium/auxiliary/Makefile.sources |   2 +
>  src/gallium/auxiliary/util/u_screen.c  | 112 
> +
>  src/gallium/auxiliary/util/u_screen.h  |  32 ++
>  src/gallium/include/pipe/p_screen.h|   3 +
>  4 files changed, 149 insertions(+)
>  create mode 100644 src/gallium/auxiliary/util/u_screen.c
>  create mode 100644 src/gallium/auxiliary/util/u_screen.h
>
> diff --git a/src/gallium/auxiliary/Makefile.sources 
> b/src/gallium/auxiliary/Makefile.sources
> index 9ae8e6c8ca53..f84bfec60fe7 100644
> --- a/src/gallium/auxiliary/Makefile.sources
> +++ b/src/gallium/auxiliary/Makefile.sources
> @@ -283,6 +283,8 @@ C_SOURCES := \
> util/u_ringbuffer.h \
> util/u_sampler.c \
> util/u_sampler.h \
> +   util/u_screen.c \
> +   util/u_screen.h \
> util/u_simple_shaders.c \
> util/u_simple_shaders.h \
> util/u_split_prim.h \
> diff --git a/src/gallium/auxiliary/util/u_screen.c 
> b/src/gallium/auxiliary/util/u_screen.c
> new file mode 100644
> index ..dec83b736883
> --- /dev/null
> +++ b/src/gallium/auxiliary/util/u_screen.c
> @@ -0,0 +1,112 @@
> +/*
> + * Copyright 2016-2017 Linaro, Ltd., Rob Herring 
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
> + * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
> + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/**
> + * Functions for managing pipe_screen's
> + */
> +
> +#include 
> +
> +#include "pipe/p_screen.h"
> +#include "util/u_hash_table.h"
> +#include "util/u_inlines.h"
> +#include "util/u_pointer.h"
> +#include "util/u_screen.h"
> +
> +static struct util_hash_table *fd_tab = NULL;
> +
> +static unsigned hash_fd(void *key)
> +{
> +   int fd = pointer_to_intptr(key);
> +   struct stat stat;
> +   fstat(fd, );
> +
> +   return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
> +}
> +
> +static int compare_fd(void *key1, void *key2)
> +{
> +   int fd1 = pointer_to_intptr(key1);
> +   int fd2 = pointer_to_intptr(key2);
> +   struct stat stat1, stat2;
> +   fstat(fd1, );
> +   fstat(fd2, );
> +
> +   return stat1.st_dev != stat2.st_dev ||
> + stat1.st_ino != stat2.st_ino ||
> + stat1.st_rdev != stat2.st_rdev;
> +}
> +
> +struct pipe_screen *
> 

Re: [Mesa-dev] [PATCH v5 01/12] gallium: move pipe_screen destroy into pipe-loader

[Marek Olšák]

With Nicolai's comment addressed, patches 1-2 are:

Reviewed-by: Marek Olšák 

Marek

On Wed, Aug 9, 2017 at 6:47 PM, Nicolai Hähnle  wrote:
> On 08.08.2017 00:58, Rob Herring wrote:
>>
>> In preparation to add reference counting of pipe_screen in the
>> pipe-loader,
>> pipe_loader_release needs to destroy the pipe_screen instead of state
>> trackers.
>
>
> Did you miss Nine?
>
> Cheers,
> Nicolai
>
>
>>
>> Signed-off-by: Rob Herring 
>> Cc: Emil Velikov 
>> ---
>>   src/gallium/auxiliary/pipe-loader/pipe_loader.c   | 14 --
>>   src/gallium/auxiliary/pipe-loader/pipe_loader.h   |  1 +
>>   src/gallium/auxiliary/vl/vl_winsys_dri.c  |  1 -
>>   src/gallium/auxiliary/vl/vl_winsys_dri3.c |  1 -
>>   src/gallium/auxiliary/vl/vl_winsys_drm.c  |  1 -
>>   src/gallium/state_trackers/clover/core/device.cpp |  4 +---
>>   src/gallium/state_trackers/dri/dri_screen.c   |  3 ---
>>   src/gallium/state_trackers/xa/xa_tracker.c|  2 --
>>   src/gallium/tests/trivial/compute.c   |  1 -
>>   src/gallium/tests/trivial/quad-tex.c  |  1 -
>>   src/gallium/tests/trivial/tri.c   |  1 -
>>   11 files changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.c
>> b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
>> index 926db49fd24b..61e5786a2528 100644
>> --- a/src/gallium/auxiliary/pipe-loader/pipe_loader.c
>> +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
>> @@ -67,13 +67,20 @@ pipe_loader_probe(struct pipe_loader_device **devs,
>> int ndev)
>>  return n;
>>   }
>>   +static void
>> +pipe_loader_release_dev(struct pipe_loader_device *dev)
>> +{
>> +   dev->pscreen->destroy(dev->pscreen);
>> +   dev->ops->release();
>> +}
>> +
>>   void
>>   pipe_loader_release(struct pipe_loader_device **devs, int ndev)
>>   {
>>  int i;
>>for (i = 0; i < ndev; i++)
>> -  devs[i]->ops->release([i]);
>> +  pipe_loader_release_dev(devs[i]);
>>   }
>> void
>> @@ -125,12 +132,15 @@ pipe_loader_get_driinfo_xml(const char *driver_name)
>>   struct pipe_screen *
>>   pipe_loader_create_screen(struct pipe_loader_device *dev)
>>   {
>> +   struct pipe_screen *pscreen;
>>  struct pipe_screen_config config;
>>pipe_loader_load_options(dev);
>>  config.options = >option_cache;
>>   -   return dev->ops->create_screen(dev, );
>> +   pscreen = dev->ops->create_screen(dev, );
>> +   dev->pscreen = pscreen;
>> +   return pscreen;
>>   }
>> struct util_dl_library *
>> diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
>> b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
>> index b50114310b4a..25cf5616f785 100644
>> --- a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
>> +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
>> @@ -66,6 +66,7 @@ struct pipe_loader_device {
>>char *driver_name;
>>  const struct pipe_loader_ops *ops;
>> +   struct pipe_screen *pscreen;
>>driOptionCache option_cache;
>>  driOptionCache option_info;
>> diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri.c
>> b/src/gallium/auxiliary/vl/vl_winsys_dri.c
>> index b4fb47ea8e46..444fff321eae 100644
>> --- a/src/gallium/auxiliary/vl/vl_winsys_dri.c
>> +++ b/src/gallium/auxiliary/vl/vl_winsys_dri.c
>> @@ -463,7 +463,6 @@ vl_dri2_screen_destroy(struct vl_screen *vscreen)
>>  }
>>vl_dri2_destroy_drawable(scrn);
>> -   scrn->base.pscreen->destroy(scrn->base.pscreen);
>>  pipe_loader_release(>base.dev, 1);
>>  FREE(scrn);
>>   }
>> diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
>> b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
>> index 8251087f3f90..4ed7ef0eacad 100644
>> --- a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
>> +++ b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
>> @@ -732,7 +732,6 @@ vl_dri3_screen_destroy(struct vl_screen *vscreen)
>> xcb_unregister_for_special_event(scrn->conn, scrn->special_event);
>>  }
>>  scrn->pipe->destroy(scrn->pipe);
>> -   scrn->base.pscreen->destroy(scrn->base.pscreen);
>>  pipe_loader_release(>base.dev, 1);
>>  FREE(scrn);
>>   diff --git a/src/gallium/auxiliary/vl/vl_winsys_drm.c
>> b/src/gallium/auxiliary/vl/vl_winsys_drm.c
>> index df8809c501cb..6bbc87635c78 100644
>> --- a/src/gallium/auxiliary/vl/vl_winsys_drm.c
>> +++ b/src/gallium/auxiliary/vl/vl_winsys_drm.c
>> @@ -81,7 +81,6 @@ vl_drm_screen_destroy(struct vl_screen *vscreen)
>>   {
>>  assert(vscreen);
>>   -   vscreen->pscreen->destroy(vscreen->pscreen);
>>  pipe_loader_release(>dev, 1);
>>  FREE(vscreen);
>>   }
>> diff --git a/src/gallium/state_trackers/clover/core/device.cpp
>> b/src/gallium/state_trackers/clover/core/device.cpp
>> index 8dfba1ad9fd9..bfdd32c794a1 100644
>> --- a/src/gallium/state_trackers/clover/core/device.cpp
>> +++ b/src/gallium/state_trackers/clover/core/device.cpp
>> @@ -45,14 +45,12 @@ 

Re: [Mesa-dev] [PATCH 10/25] i965: Use separate enums for register vs immediate types

[Scott D Phillips]

Matt Turner  writes:

> On Tue, Aug 8, 2017 at 4:25 PM, Scott D Phillips
>  wrote:
>>> + [BRW_HW_IMM_TYPE_UV]  = 2,
>>> + [BRW_HW_IMM_TYPE_VF]  = 4,
>>> + [BRW_HW_IMM_TYPE_V]   = 2,
>>
>> Is this right? I see it was there before, and perhaps I'm being dense,
>> but it seems like V and UV should be size 4 from the PRM.
>
> Yes. The encoded immediates themselves are 4 bytes, but this table
> captures the size of the individual components once expanded. That's
> admittedly a little weird.
>
> A V/UV immediate consists of 8x 4-bit integer values. A restriction
> documented in "Gen Graphics » BSpec » 3D and Compute Engine » 3D and
> GPGPU Programs » EU Overview » Registers and Register Regions »
> Immediate" states "When an immediate vector is used in an instruction,
> the destination must be 128-bit aligned with destination horizontal
> stride equivalent to a word for an immediate integer vector (v) and
> equivalent to a DWord for an immediate float vector (vf).".
>
> So we consider the individual components of the V/UV immediate to
> really be words, of size 2.
>
> Thanks for the good question!

Ah, I see. Thanks for the explanation.

Reviewed-by: Scott D Phillips 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] build: Fix up spirv_info.Plo

[Matt Turner]

spirv_info.c existed as a static file until commit 2dd4e2ece32f began
generating it as part of the build process. autotools is incapable of
coping, and so a build-tree from before this commit would then fail with
it:

[4]: *** No rule to make target 
'../../../mesa/src/compiler/spirv/spirv_info.c', needed by 
'spirv/spirv_info.lo'.  Stop.

Add a few lines to configure.ac to update the broken build files.
---
 configure.ac | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure.ac b/configure.ac
index 6302aa2b0c..9677d3fb28 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2920,6 +2920,8 @@ AC_OUTPUT
 # source file
 $SED -i -e 's/brw_blorp.cpp/brw_blorp.c/' 
src/mesa/drivers/dri/i965/.deps/brw_blorp.Plo
 
+rm src/compiler/spirv/spirv_info.lo
+echo "# dummy" > src/compiler/spirv/.deps/spirv_info.Plo
 
 dnl
 dnl Output some configuration info for the user
-- 
2.13.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/25] i965: Use separate enums for register vs immediate types

[Matt Turner]

On Tue, Aug 8, 2017 at 4:25 PM, Scott D Phillips
 wrote:
>> + [BRW_HW_IMM_TYPE_UV]  = 2,
>> + [BRW_HW_IMM_TYPE_VF]  = 4,
>> + [BRW_HW_IMM_TYPE_V]   = 2,
>
> Is this right? I see it was there before, and perhaps I'm being dense,
> but it seems like V and UV should be size 4 from the PRM.

Yes. The encoded immediates themselves are 4 bytes, but this table
captures the size of the individual components once expanded. That's
admittedly a little weird.

A V/UV immediate consists of 8x 4-bit integer values. A restriction
documented in "Gen Graphics » BSpec » 3D and Compute Engine » 3D and
GPGPU Programs » EU Overview » Registers and Register Regions »
Immediate" states "When an immediate vector is used in an instruction,
the destination must be 128-bit aligned with destination horizontal
stride equivalent to a word for an immediate integer vector (v) and
equivalent to a DWord for an immediate float vector (vf).".

So we consider the individual components of the V/UV immediate to
really be words, of size 2.

Thanks for the good question!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 3/7] util: Fix build on old glibc.

[Eric Anholt]

We need to link librt for u_thread.h's clock_gettime() call.

Fixes: b822d9dd67b5 ("gallium/util: move u_queue.{c,h} to src/util")
---
 src/util/Makefile.am | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/util/Makefile.am b/src/util/Makefile.am
index a8352a9053e3..4512dc99d5e1 100644
--- a/src/util/Makefile.am
+++ b/src/util/Makefile.am
@@ -46,7 +46,9 @@ libmesautil_la_SOURCES = \
$(MESA_UTIL_FILES) \
$(MESA_UTIL_GENERATED_FILES)
 
-libmesautil_la_LIBADD = $(ZLIB_LIBS)
+libmesautil_la_LIBADD = \
+   $(CLOCK_LIB) \
+   $(ZLIB_LIBS)
 
 libxmlconfig_la_SOURCES = $(XMLCONFIG_FILES)
 libxmlconfig_la_CFLAGS = \
-- 
2.13.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 4/7] configure.ac: Introduce HAVE_ARM_ASM/HAVE_AARCH64_ASM and the -D flags.

[Eric Anholt]

I've been trying to get away without these conditionals in vc4, but it
meant compiling extra unused code on x86, and build failing on ARMv6.
---
 Android.common.mk |  8 
 configure.ac  | 24 
 2 files changed, 32 insertions(+)

diff --git a/Android.common.mk b/Android.common.mk
index 6bd30816bc41..435e3da40a5f 100644
--- a/Android.common.mk
+++ b/Android.common.mk
@@ -87,6 +87,14 @@ LOCAL_CFLAGS += \
-DUSE_X86_ASM
 
 endif
+ifeq ($(TARGET_ARCH),arm)
+LOCAL_CFLAGS += \
+   -DUSE_ARM_ASM
+endif
+ifeq ($(TARGET_ARCH),aarch64)
+LOCAL_CFLAGS += \
+   -DUSE_AARCH64_ASM
+endif
 endif
 
 ifneq ($(LOCAL_IS_HOST_MODULE),true)
diff --git a/configure.ac b/configure.ac
index 5b12dd8506a5..d4f36898ba5b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -773,6 +773,20 @@ if test "x$enable_asm" = xyes; then
 ;;
 esac
 ;;
+aarch64)
+case "$host_os" in
+linux*)
+asm_arch=aarch64
+;;
+esac
+;;
+arm)
+case "$host_os" in
+linux*)
+asm_arch=arm
+;;
+esac
+;;
 esac
 
 case "$asm_arch" in
@@ -792,6 +806,14 @@ if test "x$enable_asm" = xyes; then
 DEFINES="$DEFINES -DUSE_PPC64LE_ASM"
 AC_MSG_RESULT([yes, ppc64le])
 ;;
+aarch64)
+DEFINES="$DEFINES -DUSE_AARCH64_ASM"
+AC_MSG_RESULT([yes, aarch64])
+;;
+arm)
+DEFINES="$DEFINES -DUSE_ARM_ASM"
+AC_MSG_RESULT([yes, arm])
+;;
 *)
 AC_MSG_RESULT([no, platform not supported])
 ;;
@@ -2729,6 +2751,8 @@ AM_CONDITIONAL(HAVE_X86_ASM, test "x$asm_arch" = xx86 -o 
"x$asm_arch" = xx86_64)
 AM_CONDITIONAL(HAVE_X86_64_ASM, test "x$asm_arch" = xx86_64)
 AM_CONDITIONAL(HAVE_SPARC_ASM, test "x$asm_arch" = xsparc)
 AM_CONDITIONAL(HAVE_PPC64LE_ASM, test "x$asm_arch" = xppc64le)
+AM_CONDITIONAL(HAVE_AARCH64_ASM, test "x$asm_arch" = xaarch64)
+AM_CONDITIONAL(HAVE_ARM_ASM, test "x$asm_arch" = xarm)
 
 AC_SUBST([NINE_MAJOR], 1)
 AC_SUBST([NINE_MINOR], 0)
-- 
2.13.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 0/7] old-glibc backport, vc4 NEON improvements

[Eric Anholt]

I was trying to build Mesa with NEON support for Raspbian wheezy
(gross), and found a couple of easy fixes.  I'm hoping for an ack here
on configure.ac/DEFINES changes and an Android test.

Branch available at:

https://github.com/anholt/mesa/commits/vc4-neon

Eric Anholt (5):
  broadcom: Add missing libexpat cflags for the decoder.
  broadcom: Add v3d_xml.h to gitignore.
  util: Fix build on old glibc.
  configure.ac: Introduce HAVE_ARM_ASM/HAVE_AARCH64_ASM and the -D
flags.
  broadcom/vc4: Build the vc4_tiling_lt_neon.c with -mfpu=neon on ARM.

Jonas Pfeil (1):
  broadcom/vc4: Port NEON-code to ARM64

Maxim Maslov (1):
  broadcom/vc4: Optimize vc4_load_utile/vc4_store_utile with sse for x86

 Android.common.mk|   8 ++
 configure.ac |  24 +
 src/broadcom/.gitignore  |   1 +
 src/broadcom/Makefile.am |   3 +
 src/gallium/drivers/vc4/Android.mk   |  10 ++
 src/gallium/drivers/vc4/Makefile.am  |  18 
 src/gallium/drivers/vc4/Makefile.sources |   3 +-
 src/gallium/drivers/vc4/vc4_tiling.h |  37 +--
 src/gallium/drivers/vc4/vc4_tiling_lt.c  | 180 ++-
 src/util/Makefile.am |   4 +-
 10 files changed, 279 insertions(+), 9 deletions(-)

-- 
2.13.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/7] broadcom: Add v3d_xml.h to gitignore.

[Eric Anholt]

---
 src/broadcom/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/broadcom/.gitignore b/src/broadcom/.gitignore
index fcc603f0cf01..5442872127e1 100644
--- a/src/broadcom/.gitignore
+++ b/src/broadcom/.gitignore
@@ -1 +1,2 @@
+cle/v3d_xml.h
 cle/*_pack.h
-- 
2.13.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 7/7] broadcom/vc4: Optimize vc4_load_utile/vc4_store_utile with sse for x86

[Eric Anholt]

From: Maxim Maslov 

Mesa vc4 drivers can be built for x86 in order to run hw accelerated
graphics inside virtual machines (QEMU, Exagear) on Raspberry Pi.

Improves playing intro videos on Diablo II inside Exagear to take 11% of
CPU instead of 20%.

v2: Runtime CPU detection by Maxim
v3: Fix up cross-compiling and make runtime CPU detection match NEON's, by
anholt.
---
 src/gallium/drivers/vc4/Makefile.am | 11 
 src/gallium/drivers/vc4/vc4_tiling.h| 20 +++
 src/gallium/drivers/vc4/vc4_tiling_lt.c | 96 -
 3 files changed, 126 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/vc4/Makefile.am 
b/src/gallium/drivers/vc4/Makefile.am
index 4c2b7486c522..1dd57c5bcd7e 100644
--- a/src/gallium/drivers/vc4/Makefile.am
+++ b/src/gallium/drivers/vc4/Makefile.am
@@ -51,6 +51,17 @@ libvc4_neon_la_SOURCES = $(NEON_C_SOURCES)
 libvc4_neon_la_CFLAGS = $(AM_CFLAGS) -mfpu=neon
 endif
 
+if HAVE_X86_64_ASM
+# Disable SSE build on x86-64, which also has HAVE_X86_ASM set.
+else
+if HAVE_X86_ASM
+noinst_LTLIBRARIES += libvc4_sse.la
+libvc4_la_LIBADD += libvc4_sse.la
+libvc4_sse_la_SOURCES = vc4_tiling_lt.c
+libvc4_sse_la_CFLAGS = $(AM_CFLAGS) -msse -DVC4_BUILD_SSE
+endif
+endif
+
 libvc4_la_LDFLAGS = $(SIM_LDFLAGS)
 
 EXTRA_DIST = kernel/README
diff --git a/src/gallium/drivers/vc4/vc4_tiling.h 
b/src/gallium/drivers/vc4/vc4_tiling.h
index 66767e7f1f83..7360ec1a9bca 100644
--- a/src/gallium/drivers/vc4/vc4_tiling.h
+++ b/src/gallium/drivers/vc4/vc4_tiling.h
@@ -75,6 +75,12 @@ void vc4_load_lt_image_neon(void *dst, uint32_t dst_stride,
 void vc4_store_lt_image_neon(void *dst, uint32_t dst_stride,
  void *src, uint32_t src_stride,
  int cpp, const struct pipe_box *box);
+void vc4_load_lt_image_sse(void *dst, uint32_t dst_stride,
+   void *src, uint32_t src_stride,
+   int cpp, const struct pipe_box *box);
+void vc4_store_lt_image_sse(void *dst, uint32_t dst_stride,
+void *src, uint32_t src_stride,
+int cpp, const struct pipe_box *box);
 void vc4_load_tiled_image(void *dst, uint32_t dst_stride,
   void *src, uint32_t src_stride,
   uint8_t tiling_format, int cpp,
@@ -96,6 +102,13 @@ vc4_load_lt_image(void *dst, uint32_t dst_stride,
 return;
 }
 #endif
+#ifdef USE_X86_ASM
+if (util_cpu_caps.has_sse2) {
+vc4_load_lt_image_sse(dst, dst_stride, src, src_stride,
+  cpp, box);
+return;
+}
+#endif
 vc4_load_lt_image_base(dst, dst_stride, src, src_stride,
cpp, box);
 }
@@ -112,6 +125,13 @@ vc4_store_lt_image(void *dst, uint32_t dst_stride,
 return;
 }
 #endif
+#ifdef USE_X86_ASM
+if (util_cpu_caps.has_sse) {
+vc4_store_lt_image_sse(dst, dst_stride, src, src_stride,
+   cpp, box);
+return;
+}
+#endif
 
 vc4_store_lt_image_base(dst, dst_stride, src, src_stride,
 cpp, box);
diff --git a/src/gallium/drivers/vc4/vc4_tiling_lt.c 
b/src/gallium/drivers/vc4/vc4_tiling_lt.c
index 29328bf0d332..865a4629f416 100644
--- a/src/gallium/drivers/vc4/vc4_tiling_lt.c
+++ b/src/gallium/drivers/vc4/vc4_tiling_lt.c
@@ -34,9 +34,12 @@
 #include 
 #include "pipe/p_state.h"
 #include "vc4_tiling.h"
+#include "util/u_cpu_detect.h"
 
 #ifdef VC4_BUILD_NEON
 #define NEON_TAG(x) x ## _neon
+#elif defined(VC4_BUILD_SSE)
+#define NEON_TAG(x) x ## _sse
 #else
 #define NEON_TAG(x) x ## _base
 #endif
@@ -149,7 +152,53 @@ vc4_load_utile(void *cpu, void *gpu, uint32_t cpu_stride, 
uint32_t cpp)
 : "r"(gpu), "r"(cpu), "r"(cpu + 8), "r"(cpu_stride)
 : "q0", "q1", "q2", "q3");
 }
+#elif defined(VC4_BUILD_SSE)
+if (gpu_stride == 8) {
+__asm__ volatile (
+"movdqu 0(%1), %%xmm0;"
+"movdqu 0x10(%1), %%xmm1;"
+"movdqu 0x20(%1), %%xmm2;"
+"movdqu 0x30(%1), %%xmm3;"
+"movlpd %%xmm0, 0(%0);"
+"mov %2, %%ecx;"
+"movhpd %%xmm0, 0(%0,%%ecx,1);"
+"add %2, %%ecx;"
+"movlpd %%xmm1, 0(%0,%%ecx,1);"
+"add %2, %%ecx;"
+"movhpd %%xmm1, 0(%0,%%ecx,1);"
+"add %2, %%ecx;"
+"movlpd %%xmm2, 0(%0,%%ecx,1);"
+"add %2, %%ecx;"
+"movhpd %%xmm2, 0(%0,%%ecx,1);"
+"add %2, %%ecx;"
+"movlpd %%xmm3, 0(%0,%%ecx,1);"
+"add %2, 

[Mesa-dev] [PATCH 5/7] broadcom/vc4: Build the vc4_tiling_lt_neon.c with -mfpu=neon on ARM.

[Eric Anholt]

If you don't pass this, the compiler refuses to compile the assembly for
pre-v7 CPUs.  This also keeps us from building identical, non-NEON code on
aarch64 and x86.

Fixes: a373f77662c5 ("vc4: Use a wrapper file to set VC4_BUILD_NEON instead of 
CFLAGS.")
---
 src/gallium/drivers/vc4/Android.mk   | 10 ++
 src/gallium/drivers/vc4/Makefile.am  |  7 +++
 src/gallium/drivers/vc4/Makefile.sources |  3 ++-
 src/gallium/drivers/vc4/vc4_tiling.h | 17 +++--
 4 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/src/gallium/drivers/vc4/Android.mk 
b/src/gallium/drivers/vc4/Android.mk
index 84bfaa839f9e..eaf66ebe8cb9 100644
--- a/src/gallium/drivers/vc4/Android.mk
+++ b/src/gallium/drivers/vc4/Android.mk
@@ -23,6 +23,15 @@ LOCAL_PATH := $(call my-dir)
 # get C_SOURCES
 include $(LOCAL_PATH)/Makefile.sources
 
+# First, the architecture-specific library.
+ifeq ($(TARGET_ARCH),arm)
+include $(CLEAR_VARS)
+LOCAL_SRC_FILES := $(NEON_C_SOURCES)
+LOCAL_CFLAGS := -mfpu=neon
+LOCAL_MODULE := libmesa_broadcom_vc4_neon
+BROADCOM_VC4_NEON_LIB := $(LOCAL_MODULE)
+endif
+
 include $(CLEAR_VARS)
 
 LOCAL_SRC_FILES := \
@@ -37,6 +46,7 @@ LOCAL_STATIC_LIBRARIES := \
libmesa_nir
 
 LOCAL_WHOLE_STATIC_LIBRARIES := \
+   $(BROADCOM_VC4_NEON_LIB) \
libmesa_broadcom_cle \
libmesa_broadcom_genxml
 
diff --git a/src/gallium/drivers/vc4/Makefile.am 
b/src/gallium/drivers/vc4/Makefile.am
index 9b0ef6ef790e..4c2b7486c522 100644
--- a/src/gallium/drivers/vc4/Makefile.am
+++ b/src/gallium/drivers/vc4/Makefile.am
@@ -44,6 +44,13 @@ libvc4_la_LIBADD = \
$(top_builddir)/src/broadcom/cle/libbroadcom_cle.la \
$()
 
+if HAVE_ARM_ASM
+noinst_LTLIBRARIES += libvc4_neon.la
+libvc4_la_LIBADD += libvc4_neon.la
+libvc4_neon_la_SOURCES = $(NEON_C_SOURCES)
+libvc4_neon_la_CFLAGS = $(AM_CFLAGS) -mfpu=neon
+endif
+
 libvc4_la_LDFLAGS = $(SIM_LDFLAGS)
 
 EXTRA_DIST = kernel/README
diff --git a/src/gallium/drivers/vc4/Makefile.sources 
b/src/gallium/drivers/vc4/Makefile.sources
index f67dffe00636..76dea7041b72 100644
--- a/src/gallium/drivers/vc4/Makefile.sources
+++ b/src/gallium/drivers/vc4/Makefile.sources
@@ -57,7 +57,8 @@ C_SOURCES := \
vc4_state.c \
vc4_tiling.c \
vc4_tiling_lt.c \
-   vc4_tiling_lt_neon.c \
vc4_tiling.h \
vc4_uniforms.c \
$()
+
+NEON_C_SOURCES := vc4_tiling_lt_neon.c
diff --git a/src/gallium/drivers/vc4/vc4_tiling.h 
b/src/gallium/drivers/vc4/vc4_tiling.h
index 3168ec20a606..66767e7f1f83 100644
--- a/src/gallium/drivers/vc4/vc4_tiling.h
+++ b/src/gallium/drivers/vc4/vc4_tiling.h
@@ -89,13 +89,15 @@ vc4_load_lt_image(void *dst, uint32_t dst_stride,
   void *src, uint32_t src_stride,
   int cpp, const struct pipe_box *box)
 {
+#ifdef USE_ARM_ASM
 if (util_cpu_caps.has_neon) {
 vc4_load_lt_image_neon(dst, dst_stride, src, src_stride,
cpp, box);
-} else {
-vc4_load_lt_image_base(dst, dst_stride, src, src_stride,
-   cpp, box);
+return;
 }
+#endif
+vc4_load_lt_image_base(dst, dst_stride, src, src_stride,
+   cpp, box);
 }
 
 static inline void
@@ -103,13 +105,16 @@ vc4_store_lt_image(void *dst, uint32_t dst_stride,
void *src, uint32_t src_stride,
int cpp, const struct pipe_box *box)
 {
+#ifdef USE_ARM_ASM
 if (util_cpu_caps.has_neon) {
 vc4_store_lt_image_neon(dst, dst_stride, src, src_stride,
 cpp, box);
-} else {
-vc4_store_lt_image_base(dst, dst_stride, src, src_stride,
-cpp, box);
+return;
 }
+#endif
+
+vc4_store_lt_image_base(dst, dst_stride, src, src_stride,
+cpp, box);
 }
 
 #endif /* VC4_TILING_H */
-- 
2.13.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 6/7] broadcom/vc4: Port NEON-code to ARM64

[Eric Anholt]

From: Jonas Pfeil 

Changed all register and instruction names, works the same.

v2: Rebase on build system changes (by anholt)

Signed-off-by: Jonas Pfeil 
---
 src/gallium/drivers/vc4/vc4_tiling_lt.c | 84 +
 1 file changed, 84 insertions(+)

diff --git a/src/gallium/drivers/vc4/vc4_tiling_lt.c 
b/src/gallium/drivers/vc4/vc4_tiling_lt.c
index f37a92e9390e..29328bf0d332 100644
--- a/src/gallium/drivers/vc4/vc4_tiling_lt.c
+++ b/src/gallium/drivers/vc4/vc4_tiling_lt.c
@@ -105,6 +105,50 @@ vc4_load_utile(void *cpu, void *gpu, uint32_t cpu_stride, 
uint32_t cpp)
 : "r"(gpu), "r"(cpu), "r"(cpu + 8), "r"(cpu_stride)
 : "q0", "q1", "q2", "q3");
 }
+#elif defined (PIPE_ARCH_AARCH64)
+   if (gpu_stride == 8) {
+__asm__ volatile (
+/* Load from the GPU in one shot, no interleave, to
+ * d0-d7.
+ */
+"ld1 {v0.2d, v1.2d, v2.2d, v3.2d}, [%0]\n"
+/* Store each 8-byte line to cpu-side destination,
+ * incrementing it by the stride each time.
+ */
+"st1 {v0.D}[0], [%1], %2\n"
+"st1 {v0.D}[1], [%1], %2\n"
+"st1 {v1.D}[0], [%1], %2\n"
+"st1 {v1.D}[1], [%1], %2\n"
+"st1 {v2.D}[0], [%1], %2\n"
+"st1 {v2.D}[1], [%1], %2\n"
+"st1 {v3.D}[0], [%1], %2\n"
+"st1 {v3.D}[1], [%1]\n"
+   :
+: "r"(gpu), "r"(cpu), "r"(cpu_stride)
+: "v0", "v1", "v2", "v3");
+} else {
+assert(gpu_stride == 16);
+__asm__ volatile (
+/* Load from the GPU in one shot, no interleave, to
+ * d0-d7.
+ */
+"ld1 {v0.2d, v1.2d, v2.2d, v3.2d}, [%0]\n"
+/* Store each 16-byte line in 2 parts to the cpu-side
+ * destination.  (vld1 can only store one d-register
+ * at a time).
+ */
+"st1 {v0.D}[0], [%1], %3\n"
+"st1 {v0.D}[1], [%2], %3\n"
+"st1 {v1.D}[0], [%1], %3\n"
+"st1 {v1.D}[1], [%2], %3\n"
+"st1 {v2.D}[0], [%1], %3\n"
+"st1 {v2.D}[1], [%2], %3\n"
+"st1 {v3.D}[0], [%1]\n"
+"st1 {v3.D}[1], [%2]\n"
+:
+: "r"(gpu), "r"(cpu), "r"(cpu + 8), "r"(cpu_stride)
+: "q0", "q1", "q2", "q3");
+}
 #else
 for (uint32_t gpu_offset = 0; gpu_offset < 64; gpu_offset += 
gpu_stride) {
 memcpy(cpu, gpu + gpu_offset, gpu_stride);
@@ -160,6 +204,46 @@ vc4_store_utile(void *gpu, void *cpu, uint32_t cpu_stride, 
uint32_t cpp)
 : "r"(gpu), "r"(cpu), "r"(cpu + 8), "r"(cpu_stride)
 : "q0", "q1", "q2", "q3");
 }
+#elif defined (PIPE_ARCH_AARCH64)
+   if (gpu_stride == 8) {
+__asm__ volatile (
+/* Load each 8-byte line from cpu-side source,
+ * incrementing it by the stride each time.
+ */
+"ld1 {v0.D}[0], [%1], %2\n"
+"ld1 {v0.D}[1], [%1], %2\n"
+"ld1 {v1.D}[0], [%1], %2\n"
+"ld1 {v1.D}[1], [%1], %2\n"
+"ld1 {v2.D}[0], [%1], %2\n"
+"ld1 {v2.D}[1], [%1], %2\n"
+"ld1 {v3.D}[0], [%1], %2\n"
+"ld1 {v3.D}[1], [%1]\n"
+/* Store to the GPU in one shot, no interleave. */
+"st1 {v0.2d, v1.2d, v2.2d, v3.2d}, [%0]\n"
+:
+: "r"(gpu), "r"(cpu), "r"(cpu_stride)
+: "v0", "v1", "v2", "v3");
+} else {
+assert(gpu_stride == 16);
+__asm__ volatile (
+/* Load each 16-byte line in 2 parts from the cpu-side
+ * destination.  (vld1 can only store one d-register
+ * at a time).
+ */
+"ld1 {v0.D}[0], [%1], %3\n"
+"ld1 {v0.D}[1], [%2], %3\n"
+"ld1 {v1.D}[0], [%1], %3\n"
+"ld1 {v1.D}[1], [%2], %3\n"
+"ld1 {v2.D}[0], [%1], %3\n"
+"ld1 {v2.D}[1], [%2], %3\n"
+"ld1 {v3.D}[0], [%1]\n"
+ 

[Mesa-dev] [PATCH 1/7] broadcom: Add missing libexpat cflags for the decoder.

[Eric Anholt]

The Raspbian ARMv6 cross compiler wasn't picking up my (amd64) system copy
of the header the way that the system gcc and armhf cross-compile did.
---
 src/broadcom/Makefile.am | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/broadcom/Makefile.am b/src/broadcom/Makefile.am
index cbcd970ecbba..9ebfe4584bf0 100644
--- a/src/broadcom/Makefile.am
+++ b/src/broadcom/Makefile.am
@@ -27,6 +27,9 @@ AM_CPPFLAGS = \
$(VALGRIND_CFLAGS) \
$(DEFINES)
 
+AM_CFLAGS = \
+   $(EXPAT_CFLAGS)
+
 include Makefile.sources
 
 lib_LTLIBRARIES =
-- 
2.13.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 101334] AMD SI cards: Some vulkan apps freeze the system

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=101334

--- Comment #48 from Marko  ---
It's working!

Today's pull + https://patchwork.freedesktop.org/series/28535/ + kernel
4.13-rc4.

Doom and Dota2 both work with no freezing. I'll test (read-play) some more
later but this finally seems to have fixed it.

Thanks!

Marko

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 01/12] gallium: move pipe_screen destroy into pipe-loader

[Nicolai Hähnle]

On 08.08.2017 00:58, Rob Herring wrote:

In preparation to add reference counting of pipe_screen in the pipe-loader,
pipe_loader_release needs to destroy the pipe_screen instead of state
trackers.


Did you miss Nine?

Cheers,
Nicolai



Signed-off-by: Rob Herring 
Cc: Emil Velikov 
---
  src/gallium/auxiliary/pipe-loader/pipe_loader.c   | 14 --
  src/gallium/auxiliary/pipe-loader/pipe_loader.h   |  1 +
  src/gallium/auxiliary/vl/vl_winsys_dri.c  |  1 -
  src/gallium/auxiliary/vl/vl_winsys_dri3.c |  1 -
  src/gallium/auxiliary/vl/vl_winsys_drm.c  |  1 -
  src/gallium/state_trackers/clover/core/device.cpp |  4 +---
  src/gallium/state_trackers/dri/dri_screen.c   |  3 ---
  src/gallium/state_trackers/xa/xa_tracker.c|  2 --
  src/gallium/tests/trivial/compute.c   |  1 -
  src/gallium/tests/trivial/quad-tex.c  |  1 -
  src/gallium/tests/trivial/tri.c   |  1 -
  11 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.c 
b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
index 926db49fd24b..61e5786a2528 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
@@ -67,13 +67,20 @@ pipe_loader_probe(struct pipe_loader_device **devs, int 
ndev)
 return n;
  }
  
+static void

+pipe_loader_release_dev(struct pipe_loader_device *dev)
+{
+   dev->pscreen->destroy(dev->pscreen);
+   dev->ops->release();
+}
+
  void
  pipe_loader_release(struct pipe_loader_device **devs, int ndev)
  {
 int i;
  
 for (i = 0; i < ndev; i++)

-  devs[i]->ops->release([i]);
+  pipe_loader_release_dev(devs[i]);
  }
  
  void

@@ -125,12 +132,15 @@ pipe_loader_get_driinfo_xml(const char *driver_name)
  struct pipe_screen *
  pipe_loader_create_screen(struct pipe_loader_device *dev)
  {
+   struct pipe_screen *pscreen;
 struct pipe_screen_config config;
  
 pipe_loader_load_options(dev);

 config.options = >option_cache;
  
-   return dev->ops->create_screen(dev, );

+   pscreen = dev->ops->create_screen(dev, );
+   dev->pscreen = pscreen;
+   return pscreen;
  }
  
  struct util_dl_library *

diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.h 
b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
index b50114310b4a..25cf5616f785 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
@@ -66,6 +66,7 @@ struct pipe_loader_device {
  
 char *driver_name;

 const struct pipe_loader_ops *ops;
+   struct pipe_screen *pscreen;
  
 driOptionCache option_cache;

 driOptionCache option_info;
diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri.c 
b/src/gallium/auxiliary/vl/vl_winsys_dri.c
index b4fb47ea8e46..444fff321eae 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri.c
@@ -463,7 +463,6 @@ vl_dri2_screen_destroy(struct vl_screen *vscreen)
 }
  
 vl_dri2_destroy_drawable(scrn);

-   scrn->base.pscreen->destroy(scrn->base.pscreen);
 pipe_loader_release(>base.dev, 1);
 FREE(scrn);
  }
diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri3.c 
b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
index 8251087f3f90..4ed7ef0eacad 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
@@ -732,7 +732,6 @@ vl_dri3_screen_destroy(struct vl_screen *vscreen)
xcb_unregister_for_special_event(scrn->conn, scrn->special_event);
 }
 scrn->pipe->destroy(scrn->pipe);
-   scrn->base.pscreen->destroy(scrn->base.pscreen);
 pipe_loader_release(>base.dev, 1);
 FREE(scrn);
  
diff --git a/src/gallium/auxiliary/vl/vl_winsys_drm.c b/src/gallium/auxiliary/vl/vl_winsys_drm.c

index df8809c501cb..6bbc87635c78 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_drm.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_drm.c
@@ -81,7 +81,6 @@ vl_drm_screen_destroy(struct vl_screen *vscreen)
  {
 assert(vscreen);
  
-   vscreen->pscreen->destroy(vscreen->pscreen);

 pipe_loader_release(>dev, 1);
 FREE(vscreen);
  }
diff --git a/src/gallium/state_trackers/clover/core/device.cpp 
b/src/gallium/state_trackers/clover/core/device.cpp
index 8dfba1ad9fd9..bfdd32c794a1 100644
--- a/src/gallium/state_trackers/clover/core/device.cpp
+++ b/src/gallium/state_trackers/clover/core/device.cpp
@@ -45,14 +45,12 @@ device::device(clover::platform , 
pipe_loader_device *ldev) :
 pipe = pipe_loader_create_screen(ldev);
 if (!pipe || !pipe->get_param(pipe, PIPE_CAP_COMPUTE)) {
if (pipe)
- pipe->destroy(pipe);
+ pipe_loader_release(, 1);
throw error(CL_INVALID_DEVICE);
 }
  }
  
  device::~device() {

-   if (pipe)
-  pipe->destroy(pipe);
 if (ldev)
pipe_loader_release(, 1);
  }
diff --git a/src/gallium/state_trackers/dri/dri_screen.c 

Re: [Mesa-dev] [PATCH] gallium/util: add new module that allocate "numbers"

[Nicolai Hähnle]

On 08.08.2017 18:57, Samuel Pitoiset wrote:

Will be used for allocating bindless descriptor slots for
RadeonSI.

Signed-off-by: Samuel Pitoiset 
---
  src/gallium/auxiliary/Makefile.sources |   1 +
  src/gallium/auxiliary/util/u_idalloc.h | 103 +
  2 files changed, 104 insertions(+)
  create mode 100644 src/gallium/auxiliary/util/u_idalloc.h

diff --git a/src/gallium/auxiliary/Makefile.sources 
b/src/gallium/auxiliary/Makefile.sources
index 9ae8e6c8ca..10101d45a4 100644
--- a/src/gallium/auxiliary/Makefile.sources
+++ b/src/gallium/auxiliary/Makefile.sources
@@ -253,6 +253,7 @@ C_SOURCES := \
util/u_hash_table.h \
util/u_helpers.c \
util/u_helpers.h \
+   util/u_idalloc.h \
util/u_index_modify.c \
util/u_index_modify.h \
util/u_inlines.h \
diff --git a/src/gallium/auxiliary/util/u_idalloc.h 
b/src/gallium/auxiliary/util/u_idalloc.h
new file mode 100644
index 00..e98f47ccb1
--- /dev/null
+++ b/src/gallium/auxiliary/util/u_idalloc.h
@@ -0,0 +1,103 @@
+/**
+ *
+ * Copyright 2017 Valve Corporation
+ * 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 AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ **/
+
+#ifndef U_IDALLOC_H
+#define U_IDALLOC_H
+
+/* A simple allocator that allocates and release "numbers". */
+
+#include "util/u_memory.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct util_idalloc
+{
+   uint32_t *data;
+   unsigned num_elements;
+};
+
+static inline void
+util_idalloc_init(struct util_idalloc *buf)
+{
+   memset(buf, 0, sizeof(*buf));
+}
+
+static inline void
+util_idalloc_fini(struct util_idalloc *buf)
+{
+   if (buf->data)
+  free(buf->data);
+}
+
+static inline void
+util_idalloc_resize(struct util_idalloc *buf, unsigned new_num_elements)
+{
+   assert(!(new_num_elements % 32));


Just round up. That's cheap enough, and there's no reason to leak this 
implementation detail to the caller.




+
+   if (new_num_elements > buf->num_elements) {
+  unsigned i;
+
+  buf->data = realloc(buf->data,
+  (new_num_elements / 32) * sizeof(*buf->data));
+
+  for (i = buf->num_elements / 32; i < new_num_elements / 32; i++)
+ buf->data[i] = 0;
+  buf->num_elements = new_num_elements;
+   }
+}
+
+static inline int32_t
+util_idalloc_get_next_free(struct util_idalloc *buf)


Call it get_first_free and use bit scan per word. (I wouldn't be 
surprised if GCC actually recognized and optimized that pattern in 
optimized builds, but still...). But see below as well.




+{
+   for (unsigned i = 0; i < buf->num_elements; i++) {
+  if (!(buf->data[i / 32] & (1 << (i % 32
+ return i;
+   }
+   return -1;
+}
+
+static inline void
+util_idalloc_lock(struct util_idalloc *buf, unsigned id)
+{
+   assert(id < buf->num_elements);
+   buf->data[id / 32] |= 1 << (id % 32);
+}
+
+static inline void
+util_idalloc_unlock(struct util_idalloc *buf, unsigned id)
+{
+   assert(id < buf->num_elements);
+   buf->data[id / 32] &= ~(1 << (id % 32));
+}


lock/unlock sounds a lot like mutexes/locks in the threading sense.

Is the get_next_free/lock/unlock interface really needed? Since this 
module calls itself an ID allocator, how about an interface


unsigned util_idalloc_alloc(struct util_idalloc *buf);
void util_idalloc_free(struct util_idalloc *buf, unsigned id);

instead?

If that's the interface, I'd honestly prefer a non-inline implementation 
as well.


Cheers,
Nicolai



+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* U_IDALLOC_H */




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org

[Mesa-dev] [Bug 102123] [llvmpipe] piglit gl-3.2-layered-rendering-framebuffertexture regression

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=102123

--- Comment #1 from Roland Scheidegger  ---
If that's only softpipe/llvmpipe regressing, it is most likely due to only
having fake msaa support.
I'm a bit surprised the test result changed, but I suppose the test previously
actually used a msaa texture with just 1 sample, which now gets upgraded
automatically to 2 samples. Albeit I can't quite tell why this particular
result changed, since mostly the nr_samples in the resource should be ignored.
In any case, we don't handle msaa correctly, so one or two more tests failing
in that area (bug 102125 is really more of the same) isn't much of an issue
(albeit it's too bad we can't stop (most of) the tests trying even though we
report a maximum number of 0 samples).
If hw drivers fail too that would be definitely worrying.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/8] egl: rework input validation order in _eglCreateWindowSurfaceCommon

[Tapani Pälli]

On 08/09/2017 12:30 PM, Eric Engestrom wrote:

On Wednesday, 2017-08-09 08:35:04 +0300, Tapani Pälli wrote:

On 08/08/2017 08:07 PM, Emil Velikov wrote:

On 8 August 2017 at 16:10, Eric Engestrom  wrote:

On Saturday, 2017-08-05 00:25:49 +0100, Emil Velikov wrote:

From: Emil Velikov 

As mentioned in previous commit the negative tests in dEQP expect the
arguments to be evaluated in particular order.

The spec doesn't say that, so the test is wrong.
Changing it in Mesa doesn't hurt though, so I have nothing against it,
except for the fact it hide the dEQP bug.


I agree, the spec does not say anything on the topic.
I think it makes sense to have the patch regardless, since it provides
a bit more consistency.

Although fixing dEQP is also a good idea. I think Tapani/Chad have
some experience/pointers on the topic.

You can send patches to gerrit in same manner just like for rest of Android,
then assign reviewers from people that have committed to dEQP. I can help
trying to get those fixes forward. You should work on master branch though,
what I've experienced is that getting fixes to some specific release branch
is a lot more difficult matter.

This is to submit fixes though, which I don't always have the time or
knowledge to do myself. I was hoping there would be a way to report
"this is wrong because X" and let someone else figure out the fix.
Is there any bug/issue tracker for deqp?


I'm not aware of one :/

// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/3] intel/compiler: properly size attribute wa_flags array for Vulkan

[Lionel Landwerlin]

Acked-by: Lionel Landwerlin 

I can see that it fixes the tests and it makes sense, but I'm failing to 
see how gl_attrib_wa_flags ends up being set from anv :/


Thanks a lot!

On 21/07/17 09:26, Iago Toral Quiroga wrote:

Mesa will map user defined vertex input attributes to slots
starting at VERT_ATTRIB_GENERIC0 which gives us room for only 16
slots (up to GL_VERT_ATTRIB_MAX). This sufficient for GL, where
we expose exactly 16 vertex attributes for user defined inputs, but
in Vulkan we can expose up to 28 (which are also mapped from
VERT_ATTRIB_GENERIC0 onwards) so we need to account for this when
we scope the size of the array of attribute workaround flags
that is used during the brw_vertex_workarounds NIR pass. This
prevents out-of-bounds accesses in that array for NIR shaders
that use more than 16 vertex input attributes.

Fixes:
dEQP-VK.pipeline.vertex_input.max_attributes.*
---

I considered other options for this too:

*  Increase the value of GL_VERT_ATTRIB_MAX, but that would affect
other drivers, including GL drivers that do not need to increase
this value. If we do this we should at least check that increasing
the value doesn't have unexpected consequences for drivers that
rely in GL_VERT_ATTRIB_MAX not being larger than what it currently
is.

* We could remap vulkan vertex attrib slots to start at 0 at some
point in the process... but I think that could be a source of
confusion, since from that point forward we shouldn't be using
shader enums to identify particular slots and there would also be
a mismatch between input bits in vs_prog_data->inputs_read
and actual slot positions which looks like an undesirable situation.

* Since the brw_vertex_workarounds pass seems rather GL-specific at
the moment, we could let our compiler infrastructure know if we
are compiling a shader for Vulkan or GL APIs and use that info
to not run the pass for Vulkan shaders, however, there is a chance
that we need this pass in Vulkan in the future too (maybe with
Vulkan specific lowerings). There is actually a comment in
anv_pipipeline.c, function populate_vs_prog_key() suggesting this
possibility, so this solution would be invalid if that ever happened.

Since all solutions have some kind of con I decided to go with the
less intrusive one for now.

  src/intel/compiler/brw_compiler.h | 18 +-
  1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/intel/compiler/brw_compiler.h 
b/src/intel/compiler/brw_compiler.h
index bebd244736..41c0707003 100644
--- a/src/intel/compiler/brw_compiler.h
+++ b/src/intel/compiler/brw_compiler.h
@@ -188,6 +188,15 @@ struct brw_sampler_prog_key_data {
  #define BRW_ATTRIB_WA_SIGN  32  /* interpret as signed in shader */
  #define BRW_ATTRIB_WA_SCALE 64  /* interpret as scaled in shader */
  
+/**

+ * OpenGL attribute slots fall in [0, VERT_ATTRIB_MAX - 1] with the range
+ * [VERT_ATTRIB_GENERIC0, VERT_ATTRIB_MAX - 1] reserved for up to 16 user
+ * input vertex attributes. In Vulkan, we expose up to 28 user vertex input
+ * attributes that are mapped to slots also starting at VERT_ATTRIB_GENERIC0.
+ */
+#define MAX_GL_VERT_ATTRIB VERT_ATTRIB_MAX
+#define MAX_VK_VERT_ATTRIB (VERT_ATTRIB_GENERIC0 + 28)
+
  /** The program key for Vertex Shaders. */
  struct brw_vs_prog_key {
 unsigned program_string_id;
@@ -196,8 +205,15 @@ struct brw_vs_prog_key {
  * Per-attribute workaround flags
  *
  * For each attribute, a combination of BRW_ATTRIB_WA_*.
+*
+* For OpenGL, where we expose a maximum of 16 user input atttributes
+* we only need up to VERT_ATTRIB_MAX slots, however, in Vulkan
+* slots preceding VERT_ATTRIB_GENERIC0 are unused and we can
+* expose up to 28 user input vertex attributes that are mapped to slots
+* starting at VERT_ATTRIB_GENERIC0, so this array need to be large
+* enough to hold this many slots.
  */
-   uint8_t gl_attrib_wa_flags[VERT_ATTRIB_MAX];
+   uint8_t gl_attrib_wa_flags[MAX2(MAX_GL_VERT_ATTRIB, MAX_VK_VERT_ATTRIB)];
  
 bool copy_edgeflag:1;
  



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [0/4] glx/dri: Attempt to fix GLX_OML_swap_method v2

[Brian Paul]

Hi Thomas,

The series looks good to me.  One formatting nit: in one of the patches
there's unneeded space around the conditional in some the "if (" code.

Reviewed-by: Brian Paul 


On Wed, Aug 9, 2017 at 3:53 AM, Thomas Hellstrom 
wrote:

> The current implementation was suffering from the following problems:
> 1) The driGetConfigAttribIndex function was not returning any value for
> the driconfig swapMethod.
> 2) The X server GLX code is using the value obtained from
> the AIGLX dri driver driGetConfigAttribIndex to populate its GLX fbconfig.
> 3) The client side GLX code was assuming fbconfig and driconfig match
> regardless of whether there actually was a swapMethod match.
> 4) We were using GLX tokens in the dri code.
> 5) dri3 does currently not support GLX_SWAP_COPY_OML, even if that's
> advertized by the gallium dri2 state tracker.
>
> Attempt to fix 1-4. Some highlights:
>
> Because of 1) in combination with 2), if the X server uses an AIGLX dri
> driver
> that doesn't have this fix, the GLX transmitted fbconfig swapMethod value
> will
> be bogus. So if we, on the client side detect a bogus value, change it to
> GLX_SWAP_UNDEFINED_OML.
>
> v2: Split the single patch into four patches.
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 102030] private memory overflow in openCL

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=102030

Jan Vesely  changed:

   What|Removed |Added

 Resolution|WONTFIX |INVALID

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 102030] private memory overflow in openCL

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=102030

Janpieter Sollie  changed:

   What|Removed |Added

 Resolution|--- |WONTFIX
 Status|NEW |RESOLVED

--- Comment #6 from Janpieter Sollie  ---
seems like I am a very bad programmer...
after investigating the OpenCL reference manual, I found out my code was
unstable, and I need to fix it before I can post something useful

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] radeonsi: update non-resident bindless descriptors if needed

[Samuel Pitoiset]

Only resident bindless descriptors are currently updated and
re-uploaded, this makes sure that the non-resident ones are
also updated.

Signed-off-by: Samuel Pitoiset 
Cc: "17.2" 
---
 src/gallium/drivers/radeonsi/si_descriptors.c | 85 +--
 1 file changed, 55 insertions(+), 30 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
b/src/gallium/drivers/radeonsi/si_descriptors.c
index 1e0c422fb4..537dc7fa50 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -1997,45 +1997,55 @@ static void si_upload_bindless_descriptors(struct 
si_context *sctx)
 }
 
 /* Update mutable image descriptor fields of all resident textures. */
-static void si_update_all_resident_texture_descriptors(struct si_context *sctx)
+static void si_update_resident_texture_descriptor(struct si_context *sctx,
+ struct si_texture_handle 
*tex_handle)
 {
-   util_dynarray_foreach(>resident_tex_handles,
- struct si_texture_handle *, tex_handle) {
-   struct si_bindless_descriptor *desc = (*tex_handle)->desc;
-   struct si_sampler_view *sview =
-   (struct si_sampler_view *)(*tex_handle)->view;
-   uint32_t desc_list[16];
+   struct si_sampler_view *sview = (struct si_sampler_view 
*)tex_handle->view;
+   struct si_bindless_descriptor *desc = tex_handle->desc;
+   uint32_t desc_list[16];
 
-   if (sview->base.texture->target == PIPE_BUFFER)
-   continue;
+   if (sview->base.texture->target == PIPE_BUFFER)
+   return;
 
-   memcpy(desc_list, desc->desc_list, sizeof(desc_list));
-   si_set_sampler_view_desc(sctx, sview, &(*tex_handle)->sstate,
->desc_list[0]);
+   memcpy(desc_list, desc->desc_list, sizeof(desc_list));
+   si_set_sampler_view_desc(sctx, sview, _handle->sstate,
+>desc_list[0]);
 
-   if (memcmp(desc_list, desc->desc_list, sizeof(desc_list))) {
-   desc->dirty = true;
-   sctx->bindless_descriptors_dirty = true;
-   }
+   if (memcmp(desc_list, desc->desc_list, sizeof(desc_list))) {
+   desc->dirty = true;
+   sctx->bindless_descriptors_dirty = true;
}
+}
 
-   util_dynarray_foreach(>resident_img_handles,
- struct si_image_handle *, img_handle) {
-   struct si_bindless_descriptor *desc = (*img_handle)->desc;
-   struct pipe_image_view *view = &(*img_handle)->view;
-   uint32_t desc_list[16];
+static void si_update_resident_image_descriptor(struct si_context *sctx,
+   struct si_image_handle 
*img_handle)
+{
+   struct si_bindless_descriptor *desc = img_handle->desc;
+   struct pipe_image_view *view = _handle->view;
+   uint32_t desc_list[16];
 
-   if (view->resource->target == PIPE_BUFFER)
-   continue;
+   if (view->resource->target == PIPE_BUFFER)
+   return;
 
-   memcpy(desc_list, desc->desc_list, sizeof(desc_list));
-   si_set_shader_image_desc(sctx, view, true,
->desc_list[0]);
+   memcpy(desc_list, desc->desc_list, sizeof(desc_list));
+   si_set_shader_image_desc(sctx, view, true, >desc_list[0]);
 
-   if (memcmp(desc_list, desc->desc_list, sizeof(desc_list))) {
-   desc->dirty = true;
-   sctx->bindless_descriptors_dirty = true;
-   }
+   if (memcmp(desc_list, desc->desc_list, sizeof(desc_list))) {
+   desc->dirty = true;
+   sctx->bindless_descriptors_dirty = true;
+   }
+}
+
+static void si_update_all_resident_texture_descriptors(struct si_context *sctx)
+{
+   util_dynarray_foreach(>resident_tex_handles,
+ struct si_texture_handle *, tex_handle) {
+   si_update_resident_texture_descriptor(sctx, *tex_handle);
+   }
+
+   util_dynarray_foreach(>resident_img_handles,
+ struct si_image_handle *, img_handle) {
+   si_update_resident_image_descriptor(sctx, *img_handle);
}
 
si_upload_bindless_descriptors(sctx);
@@ -2513,6 +2523,13 @@ static void si_make_texture_handle_resident(struct 
pipe_context *ctx,
if (rtex->dcc_offset &&
p_atomic_read(>framebuffers_bound))
sctx->need_check_render_feedback = true;
+
+   /* Re-upload the descriptor if it has been updated
+* while it wasn't resident.
+   

Re: [Mesa-dev] [PATCH] st/dri2: fix kms_swrast driconf option handling

[Nicolai Hähnle]

On 08.08.2017 19:07, Rob Herring wrote:

On Tue, Aug 8, 2017 at 11:56 AM, Ilia Mirkin  wrote:

On Tue, Aug 8, 2017 at 12:50 PM, Rob Herring  wrote:

Commit e794f8bf8bdb ("gallium: move loading of drirc to pipe-loader")
moved the option cache to the pipe_loader_device. However, the
screen->dev pointer is not set when dri_init_options() is called. Move
the call to after the pipe_loader_sw_probe_kms() call so screen->dev is
set. This mirrors the code flow for dri2_init_screen().

Fixes: e794f8bf8bdb ("gallium: move loading of drirc to pipe-loader")
Cc: Nicolai Hähnle 
Cc: Marek Olšák 
Signed-off-by: Rob Herring 
---
  src/gallium/state_trackers/dri/dri2.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/gallium/state_trackers/dri/dri2.c 
b/src/gallium/state_trackers/dri/dri2.c
index 3555107856c8..680826f58144 100644
--- a/src/gallium/state_trackers/dri/dri2.c
+++ b/src/gallium/state_trackers/dri/dri2.c
@@ -2164,9 +2164,8 @@ dri_kms_init_screen(__DRIscreen * sPriv)
 if (screen->fd < 0 || (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, 3)) < 0)
goto free_screen;

-   dri_init_options(screen);
-
 if (pipe_loader_sw_probe_kms(>dev, fd))


{


+  dri_init_options(screen);
pscreen = pipe_loader_create_screen(screen->dev);


}


Good catch. By luck it worked for the non-error case.


With that fixed, the patch is

Reviewed-by: Nicolai Hähnle 




Rob



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 0/2] glsl: interpolateAt*() fixes

[Nicolai Hähnle]

Thanks, Timothy. I'm going to make a respin that relaxes the rules only 
in desktop GL for now, and I guess we should separately raise this with 
the GLES WG.


Cheers,
Nicolai


On 09.08.2017 02:30, Timothy Arceri wrote:

Hi Nicolai,

I put this series through Intels CI system and it hit a couple of 
issues. I haven't yet checked if these CTS test regress on radeonsi as 
well or if its just an i965 thing.


 Project: deqp-test
 Test: dEQP-

GLES31.functional.shaders.multisample_interpolation.interpolate_at_centroid.nega 


 tive.interpolate_struct_member
 Status: fail
 Platform/arch:
 skl/m64, ivb/m64, bdw/m64, hsw/m64
 Project: deqp-test
 Test: dEQP-

GLES31.functional.shaders.multisample_interpolation.interpolate_at_offset.negati 


 ve.interpolate_struct_member
 Status: fail
 Platform/arch:
 skl/m64, ivb/m64, bdw/m64, hsw/m64
 Project: deqp-test
 Test: dEQP-

GLES31.functional.shaders.multisample_interpolation.interpolate_at_sample.negati 


 ve.interpolate_struct_member
 Status: fail
 Platform/arch:
 skl/m64, ivb/m64, bdw/m64, hsw/m64


On 01/08/17 20:49, Nicolai Hähnle wrote:

Hi all,

I sent a v1 of this around ~6 weeks ago. Since then, I brought some 
related

issues up with the OpenGL WG, and we now have a rule clarification in
GLSL 4.60 (which is intended to apply to earlier versions as well). This
updated series implements that rule clarification.

It passes on radeonsi with some additional piglit tests that I'm going to
send around in a moment.

Please review!

Thanks,
Nicolai

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] mesa/st: add support for hw atomics (v2)

[Nicolai Hähnle]

Hi Dave,

Thanks for the update, I prefer this.

Have you considered Marek's query about pipeline-wide atomic buffers?

The issue I'm thinking about is what happens when multiple shaders 
access the same atomic counter. In a GDS/GWS-based implementation, those 
accesses must map to the same hardware counter slot, as far as I 
understand it. But if you follow a straight-forward mapping based on 
today's TGSI, they won't be.


There are two types of situations that can happen:

1. A GL program object has one counter that is accessed by multiple stages.

2. A GL program object (or even a single shader stage) has multiple 
counters with different bindings, and the same buffer range happens to 
be bound to both bindings.


I'm pretty sure the second case is intended to be undefined behavior 
(see Issue 19 of ARB_shader_atomic_counters), but I believe the first 
case is intended to be supported.


So *something* has to make sure that different stages end up accessing 
the same hardware counter. One way of doing this is to have a global (as 
opposed to per-shader-stage) list of atomic buffers at the Gallium 
pipe_context level.


Cheers,
Nicolai


On 08.08.2017 05:13, Dave Airlie wrote:

From: Dave Airlie 

This adds support for hw atomics to the state tracker,
it just sets the limits using the new CAPs, and sets
the maximums etc for it.

v2: rework for dropping CAP.

Signed-off-by: Dave Airlie 
---
  src/mesa/state_tracker/st_extensions.c | 52 +++---
  1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/src/mesa/state_tracker/st_extensions.c 
b/src/mesa/state_tracker/st_extensions.c
index 74193cc..2d93802 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -78,6 +78,8 @@ void st_init_limits(struct pipe_screen *screen,
 int supported_irs;
 unsigned sh;
 boolean can_ubo = TRUE;
+   uint32_t temp;
+   bool ssbo_atomic = true;
  
 c->MaxTextureLevels

= _min(screen->get_param(screen, PIPE_CAP_MAX_TEXTURE_2D_LEVELS),
@@ -242,11 +244,21 @@ void st_init_limits(struct pipe_screen *screen,
c->MaxUniformBlockSize / 4 *
pc->MaxUniformBlocks);
  
-  pc->MaxAtomicCounters = MAX_ATOMIC_COUNTERS;

-  pc->MaxAtomicBuffers = screen->get_shader_param(
-screen, sh, PIPE_SHADER_CAP_MAX_SHADER_BUFFERS) / 2;
-  pc->MaxShaderStorageBlocks = pc->MaxAtomicBuffers;
-
+  temp = screen->get_shader_param(screen, sh, 
PIPE_SHADER_CAP_MAX_HW_ATOMIC_COUNTERS);
+  if (temp) {
+ /*
+  * for separate atomic counters get the actual hw limits
+  * per stage on atomic counters and buffers
+  */
+ssbo_atomic = false;
+ pc->MaxAtomicCounters = temp;
+ pc->MaxAtomicBuffers = screen->get_shader_param(screen, sh, 
PIPE_SHADER_CAP_MAX_HW_ATOMIC_COUNTER_BUFFERS);
+ pc->MaxShaderStorageBlocks = screen->get_shader_param(screen, sh, 
PIPE_SHADER_CAP_MAX_SHADER_BUFFERS);
+  } else {
+ pc->MaxAtomicCounters = MAX_ATOMIC_COUNTERS;
+ pc->MaxAtomicBuffers = screen->get_shader_param(screen, sh, 
PIPE_SHADER_CAP_MAX_SHADER_BUFFERS) / 2;
+ pc->MaxShaderStorageBlocks = pc->MaxAtomicBuffers;
+  }
pc->MaxImageUniforms = screen->get_shader_param(
  screen, sh, PIPE_SHADER_CAP_MAX_SHADER_IMAGES);
  
@@ -406,14 +418,26 @@ void st_init_limits(struct pipe_screen *screen,

screen->get_param(screen, PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL);
  
 c->MaxAtomicBufferBindings =

- c->Program[MESA_SHADER_FRAGMENT].MaxAtomicBuffers;
-   c->MaxCombinedAtomicBuffers =
+  c->Program[MESA_SHADER_FRAGMENT].MaxAtomicBuffers;
+
+   if (!ssbo_atomic) {
+  /* for separate atomic buffers - there atomic buffer size will be
+ limitied */
+  c->MaxAtomicBufferSize = 
c->Program[MESA_SHADER_FRAGMENT].MaxAtomicCounters * ATOMIC_COUNTER_SIZE;
+  /* on all HW with separate atomic (evergreen) the following
+ lines are true. not sure it's worth adding CAPs for this at this
+ stage. */
+  c->MaxCombinedAtomicCounters = 
c->Program[MESA_SHADER_FRAGMENT].MaxAtomicCounters;
+  c->MaxCombinedAtomicBuffers = 
c->Program[MESA_SHADER_FRAGMENT].MaxAtomicBuffers;
+   } else {
+  c->MaxCombinedAtomicBuffers =
   c->Program[MESA_SHADER_VERTEX].MaxAtomicBuffers +
   c->Program[MESA_SHADER_TESS_CTRL].MaxAtomicBuffers +
   c->Program[MESA_SHADER_TESS_EVAL].MaxAtomicBuffers +
   c->Program[MESA_SHADER_GEOMETRY].MaxAtomicBuffers +
   c->Program[MESA_SHADER_FRAGMENT].MaxAtomicBuffers;
-   assert(c->MaxCombinedAtomicBuffers <= MAX_COMBINED_ATOMIC_BUFFERS);
+  assert(c->MaxCombinedAtomicBuffers <= MAX_COMBINED_ATOMIC_BUFFERS);
+   }
  
 if (c->MaxCombinedAtomicBuffers > 0) {

Re: [Mesa-dev] [PATCH 3/3] glsl: stop cloning builtin fuctions _mesa_glsl_find_builtin_function()

[Nicolai Hähnle]

On 04.08.2017 09:25, Timothy Arceri wrote:

The cloning was introduced in f81ede469910d to fixed a problem with


*to fix


shaders including IR that was owned by builtins.

However the approach of cloning the whole function each time we
reference a builtin lead to a significant reduction in the GLSL
IR compilers performance.

The previous patch fixes the ownership problem in a more precise
way. So we can now remove this cloning.

Testing on a Ryzan 7 1800X shows a ~15% decreases in compiling the


*Ryzen

Okay, generate_call immediately inlines builtin functions. So the normal 
case should not cross memory contexts, and the previous patch fixes the 
case of constant expression evaluation. Seems reasonable.


Assuming you fix the minor comments above and the comment in patch #2, 
the series is:


Reviewed-by: Nicolai Hähnle 



Deus Ex: Mankind Divided shaders on radeonsi (which take 5min+ on
some machines). Looking just at the GLSL IR compiler the speed up
is ~40%.

Cc: Kenneth Graunke 
Cc: Lionel Landwerlin 
---
  src/compiler/glsl/builtin_functions.cpp | 11 +--
  1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/compiler/glsl/builtin_functions.cpp 
b/src/compiler/glsl/builtin_functions.cpp
index 84833bdd7d..1393087cc6 100644
--- a/src/compiler/glsl/builtin_functions.cpp
+++ b/src/compiler/glsl/builtin_functions.cpp
@@ -6207,30 +6207,21 @@ _mesa_glsl_release_builtin_functions()
  
  ir_function_signature *

  _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state *state,
   const char *name, exec_list 
*actual_parameters)
  {
 ir_function_signature *s;
 mtx_lock(_lock);
 s = builtins.find(state, name, actual_parameters);
 mtx_unlock(_lock);
  
-   if (s == NULL)

-  return NULL;
-
-   struct hash_table *ht =
-  _mesa_hash_table_create(NULL, _mesa_hash_pointer, 
_mesa_key_pointer_equal);
-   void *mem_ctx = state;
-   ir_function *f = s->function()->clone(mem_ctx, ht);
-   _mesa_hash_table_destroy(ht, NULL);
-
-   return f->matching_signature(state, actual_parameters, true);
+   return s;
  }
  
  bool

  _mesa_glsl_has_builtin_function(_mesa_glsl_parse_state *state, const char 
*name)
  {
 ir_function *f;
 bool ret = false;
 mtx_lock(_lock);
 f = builtins.shader->symbols->get_function(name);
 if (f != NULL) {




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/3] glsl: clone builtin function constants

[Nicolai Hähnle]

On 04.08.2017 09:25, Timothy Arceri wrote:

f81ede469910d fixed a problem with shaders including IR that was
owned by builtins. However the approach of cloning the whole
function each time we referenced it lead to a significant
reduction in the GLSL IR compiler performance.

Everything was already cloned when inlining the function, as
far as I can tell this is the only place where we are grabbing
IR owned by the builtins without cloning it.

The double free can be easily reproduced by compiling the
Deus Ex: Mankind Divided shaders with shader db, and then
compiling them again after deleting mesa's shader cache
index file. This will cause optimisations to never be performed
on the IR and which presumably caused this issue to be hidden
under normal circumstances.

Cc: Kenneth Graunke 
Cc: Lionel Landwerlin 
---
  src/compiler/glsl/ast_function.cpp | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_function.cpp 
b/src/compiler/glsl/ast_function.cpp
index f7e90fba5b..73c4c0df7b 100644
--- a/src/compiler/glsl/ast_function.cpp
+++ b/src/compiler/glsl/ast_function.cpp
@@ -514,21 +514,29 @@ generate_call(exec_list *instructions, 
ir_function_signature *sig,
  *   return 0 when evaluated inside an initializer with an argument
  *   that is a constant expression."
  *
  * If the function call is a constant expression, don't generate any
  * instructions; just generate an ir_constant.
  */
 if (state->is_version(120, 100)) {
ir_constant *value = sig->constant_expression_value(actual_parameters,
NULL);
if (value != NULL) {
- return value;
+ if (sig->is_builtin()) {
+/* This value belongs to a builtin so we must clone it to avoid
+ * race conditions when freeing shaders and destorying the
+ * context.


*destroying

More importantly, this is not really a race condition, is it? More like 
use-after-free is possible if value is reparented.


Cheers,
Nicolai



+ */
+return value->clone(ctx, NULL);
+ } else {
+return value;
+ }
}
 }
  
 ir_dereference_variable *deref = NULL;

 if (!sig->return_type->is_void()) {
/* Create a new temporary to hold the return value. */
char *const name = ir_variable::temporaries_allocate_names
   ? ralloc_asprintf(ctx, "%s_retval", sig->function_name())
   : NULL;
  




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] egl/dri2: Allow modifiers to add FDs to imports

[Daniel Stone]

On 8 August 2017 at 09:48, Tapani Pälli  wrote:
> On 08/08/2017 10:37 AM, Philipp Zabel wrote:
>> On Tue, 2017-08-08 at 07:29 +0300, Tapani Pälli wrote:
 Since this increments plane_n, Should a check be added that the
 corresponding DMABufPlanFds[i] is present?
>>>
>>> Check for the fd is right above this check.
>>
>>
>> I see this right above:
>>
>>if (attrs->DMABufPlaneFds[i].IsPresent ||
>>attrs->DMABufPlaneOffsets[i].IsPresent ||
>>attrs->DMABufPlanePitches[i].IsPresent ||
>>attrs->DMABufPlaneModifiersLo[i].IsPresent ||
>>attrs->DMABufPlaneModifiersHi[i].IsPresent) {
>>
>> If modifiers are present, this is always true, regardless of whether the
>> fd is present.
>>
>> The loop that checks for fd presence even before that only loops up to
>> the number of planes determined by the non-modified fourcc.
>
> Ah that is correct, my bad. It would be possible to swim through with having
> modifiers and not fd present.

Thanks for the review both - v2 sent which makes it a bit more neat
and also fixes the bug.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2] egl/dri2: Allow modifiers to add FDs to imports

[Daniel Stone]

When using dmabuf import, make sure that the modifier is actually
allowed to add planes to the base format, as implied by the comment.

Signed-off-by: Daniel Stone 
---
 src/egl/drivers/dri2/egl_dri2.c | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index f0d1ded408..14decfed99 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -2120,6 +2120,24 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
   return 0;
}
 
+   for (unsigned i = plane_n; i < DMA_BUF_MAX_PLANES; i++) {
+  /**
+   * The modifiers extension spec says:
+   *
+   * "Modifiers may modify any attribute of a buffer import, including
+   *  but not limited to adding extra planes to a format which
+   *  otherwise does not have those planes. As an example, a modifier
+   *  may add a plane for an external compression buffer to a
+   *  single-plane format. The exact meaning and effect of any
+   *  modifier is canonically defined by drm_fourcc.h, not as part of
+   *  this extension."
+   */
+  if (attrs->DMABufPlaneModifiersLo[i].IsPresent &&
+  attrs->DMABufPlaneModifiersHi[i].IsPresent) {
+ plane_n = i + 1;
+  }
+   }
+
/**
  * The spec says:
  *
@@ -2146,25 +2164,7 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
for (unsigned i = plane_n; i < DMA_BUF_MAX_PLANES; ++i) {
   if (attrs->DMABufPlaneFds[i].IsPresent ||
   attrs->DMABufPlaneOffsets[i].IsPresent ||
-  attrs->DMABufPlanePitches[i].IsPresent ||
-  attrs->DMABufPlaneModifiersLo[i].IsPresent ||
-  attrs->DMABufPlaneModifiersHi[i].IsPresent) {
-
- /**
-  * The modifiers extension spec says:
-  *
-  * "Modifiers may modify any attribute of a buffer import, including
-  *  but not limited to adding extra planes to a format which
-  *  otherwise does not have those planes. As an example, a modifier
-  *  may add a plane for an external compression buffer to a
-  *  single-plane format. The exact meaning and effect of any
-  *  modifier is canonically defined by drm_fourcc.h, not as part of
-  *  this extension."
-  */
- if (attrs->DMABufPlaneModifiersLo[i].IsPresent &&
- attrs->DMABufPlaneModifiersHi[i].IsPresent)
-continue;
-
+  attrs->DMABufPlanePitches[i].IsPresent) {
  _eglError(EGL_BAD_ATTRIBUTE, "too many plane attributes");
  return 0;
   }
-- 
2.13.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 3/4] glx: Fix swap method config matching

[Thomas Hellstrom]

Due to bugs in dri swap method reporting, neither the fbconfigs received from
the server nor the value reported from driconfigs were correct. Now that's been
fixed and we can enable config swapmethod matching again.

Signed-off-by: Thomas Hellstrom 
---
 src/glx/dri_common.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
index 854733a..0f8f713 100644
--- a/src/glx/dri_common.c
+++ b/src/glx/dri_common.c
@@ -242,10 +242,8 @@ static const struct
   __ATTRIB(__DRI_ATTRIB_MAX_PBUFFER_PIXELS, maxPbufferPixels),
   __ATTRIB(__DRI_ATTRIB_OPTIMAL_PBUFFER_WIDTH, optimalPbufferWidth),
   __ATTRIB(__DRI_ATTRIB_OPTIMAL_PBUFFER_HEIGHT, optimalPbufferHeight),
-#if 0
   __ATTRIB(__DRI_ATTRIB_SWAP_METHOD, swapMethod),
-#endif
-__ATTRIB(__DRI_ATTRIB_BIND_TO_TEXTURE_RGB, bindToTextureRgb),
+  __ATTRIB(__DRI_ATTRIB_BIND_TO_TEXTURE_RGB, bindToTextureRgb),
   __ATTRIB(__DRI_ATTRIB_BIND_TO_TEXTURE_RGBA, bindToTextureRgba),
   __ATTRIB(__DRI_ATTRIB_BIND_TO_MIPMAP_TEXTURE,
  bindToMipmapTexture),
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 4/4] dri: Introduce SWAP_METHOD tokens

[Thomas Hellstrom]

We shouldn't be using GLX tokens in the dri subsystem, so define dri
SWAP_METHOD tokens and translate when necessary. Unfortunately the X server
uses the dri swap method value untranslated as the GLX fbconfig swapMethod,
so we can't enumerate these tokens arbitrarily, but rather need to make them
have the same values as the corresponding GLX tokens.

Signed-off-by: Thomas Hellstrom 
---
 include/GL/internal/dri_interface.h   | 12 
 src/gallium/state_trackers/dri/dri_screen.c   |  3 ++-
 src/glx/dri_common.c  | 13 +
 src/mesa/drivers/dri/common/utils.c   |  6 +++---
 src/mesa/drivers/dri/i915/intel_screen.c  |  2 +-
 src/mesa/drivers/dri/i965/intel_screen.c  |  2 +-
 src/mesa/drivers/dri/nouveau/nouveau_screen.c |  2 +-
 src/mesa/drivers/dri/radeon/radeon_screen.c   |  6 ++
 src/mesa/drivers/dri/swrast/swrast.c  |  2 +-
 9 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/include/GL/internal/dri_interface.h 
b/include/GL/internal/dri_interface.h
index 5e8fce7..2cbd738 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -724,6 +724,18 @@ struct __DRIuseInvalidateExtensionRec {
 #define __DRI_ATTRIB_TEXTURE_2D_BIT0x02
 #define __DRI_ATTRIB_TEXTURE_RECTANGLE_BIT 0x04
 
+/* __DRI_ATTRIB_SWAP_METHOD */
+/* Note that with the exception of __DRI_ATTRIB_SWAP_NONE, we need to define
+ * the same tokens as GLX. This is because old and current X servers will
+ * transmit the driconf value grabbed from the AIGLX driver untranslated as
+ * the GLX fbconfig value. __DRI_ATTRIB_SWAP_NONE is only used by dri drivers
+ * to signal to the dri core that the driconfig is single-buffer.
+ */
+#define __DRI_ATTRIB_SWAP_NONE  0x
+#define __DRI_ATTRIB_SWAP_EXCHANGE  0x8061
+#define __DRI_ATTRIB_SWAP_COPY  0x8062
+#define __DRI_ATTRIB_SWAP_UNDEFINED 0x8063
+
 /**
  * This extension defines the core DRI functionality.
  *
diff --git a/src/gallium/state_trackers/dri/dri_screen.c 
b/src/gallium/state_trackers/dri/dri_screen.c
index 406e97d..1d9f441 100644
--- a/src/gallium/state_trackers/dri/dri_screen.c
+++ b/src/gallium/state_trackers/dri/dri_screen.c
@@ -156,7 +156,8 @@ dri_fill_in_modes(struct dri_screen *screen)
boolean mixed_color_depth;
 
static const GLenum back_buffer_modes[] = {
-  GLX_NONE, GLX_SWAP_UNDEFINED_OML, GLX_SWAP_COPY_OML
+  __DRI_ATTRIB_SWAP_NONE, __DRI_ATTRIB_SWAP_UNDEFINED,
+  __DRI_ATTRIB_SWAP_COPY
};
 
if (driQueryOptionb(>dev->option_cache, 
"always_have_depth_buffer")) {
diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
index 0f8f713..e2bbd48 100644
--- a/src/glx/dri_common.c
+++ b/src/glx/dri_common.c
@@ -317,6 +317,19 @@ driConfigEqual(const __DRIcoreExtension *core,
 return GL_FALSE;
  break;
 
+  case __DRI_ATTRIB_SWAP_METHOD:
+ if (value == __DRI_ATTRIB_SWAP_EXCHANGE)
+glxValue = GLX_SWAP_EXCHANGE_OML;
+ else if (value == __DRI_ATTRIB_SWAP_COPY)
+glxValue = GLX_SWAP_COPY_OML;
+ else
+glxValue = GLX_SWAP_UNDEFINED_OML;
+
+ if (!scalarEqual(config, attrib, glxValue))
+return GL_FALSE;
+
+ break;
+
   default:
  if (!scalarEqual(config, attrib, value))
 return GL_FALSE;
diff --git a/src/mesa/drivers/dri/common/utils.c 
b/src/mesa/drivers/dri/common/utils.c
index f3ea61e..1ad4a62 100644
--- a/src/mesa/drivers/dri/common/utils.c
+++ b/src/mesa/drivers/dri/common/utils.c
@@ -284,9 +284,9 @@ driCreateConfigs(mesa_format format,
modes->transparentIndex = GLX_DONT_CARE;
modes->rgbMode = GL_TRUE;
 
-   if ( db_modes[i] == GLX_NONE) {
-   modes->doubleBufferMode = GL_FALSE;
-modes->swapMethod = GLX_SWAP_UNDEFINED_OML;
+if ( db_modes[i] == __DRI_ATTRIB_SWAP_NONE ) {
+modes->doubleBufferMode = GL_FALSE;
+modes->swapMethod = __DRI_ATTRIB_SWAP_UNDEFINED;
}
else {
modes->doubleBufferMode = GL_TRUE;
diff --git a/src/mesa/drivers/dri/i915/intel_screen.c 
b/src/mesa/drivers/dri/i915/intel_screen.c
index 367a734..6e32ac2 100644
--- a/src/mesa/drivers/dri/i915/intel_screen.c
+++ b/src/mesa/drivers/dri/i915/intel_screen.c
@@ -1060,7 +1060,7 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
 
/* GLX_SWAP_COPY_OML is not supported due to page flipping. */
static const GLenum back_buffer_modes[] = {
-   GLX_SWAP_UNDEFINED_OML, GLX_NONE,
+  __DRI_ATTRIB_SWAP_UNDEFINED, __DRI_ATTRIB_SWAP_NONE
};
 
static const uint8_t singlesample_samples[1] = {0};
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
b/src/mesa/drivers/dri/i965/intel_screen.c
index ec07cf0..452f0d1 

[Mesa-dev] [0/4] glx/dri: Attempt to fix GLX_OML_swap_method v2

[Thomas Hellstrom]

The current implementation was suffering from the following problems:
1) The driGetConfigAttribIndex function was not returning any value for
the driconfig swapMethod.
2) The X server GLX code is using the value obtained from
the AIGLX dri driver driGetConfigAttribIndex to populate its GLX fbconfig.
3) The client side GLX code was assuming fbconfig and driconfig match
regardless of whether there actually was a swapMethod match.
4) We were using GLX tokens in the dri code.
5) dri3 does currently not support GLX_SWAP_COPY_OML, even if that's
advertized by the gallium dri2 state tracker.

Attempt to fix 1-4. Some highlights:

Because of 1) in combination with 2), if the X server uses an AIGLX dri driver
that doesn't have this fix, the GLX transmitted fbconfig swapMethod value will
be bogus. So if we, on the client side detect a bogus value, change it to
GLX_SWAP_UNDEFINED_OML.

v2: Split the single patch into four patches.


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/4] glx: Work around X servers reporting bogus values of GLX_SWAP_METHOD_OML

[Thomas Hellstrom]

Due to the recently fixed bug where dri drivers didn't report a correct
__DRI_ATTRIB_SWAP_METHOD value, and the fact that X servers just forward this
incorrect value (from the AIGLX dri driver) untranslated as
GLX_SWAP_METHOD_OML, the latter value might be undefined when old dri AIGLX
values are used, which breaks client fbconfig matching with server fbconfigs.

So work around this by assuming GLX_SWAP_METHOD_UNDEFINED when a bogus value
is read.

Signed-off-by: Thomas Hellstrom 
---
 src/glx/glxext.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/glx/glxext.c b/src/glx/glxext.c
index 9ef7ff5..9cbe334 100644
--- a/src/glx/glxext.c
+++ b/src/glx/glxext.c
@@ -524,7 +524,17 @@ __glXInitializeVisualConfigFromTags(struct glx_config * 
config, int count,
  config->visualSelectGroup = *bp++;
  break;
   case GLX_SWAP_METHOD_OML:
- config->swapMethod = *bp++;
+ if (*bp == GLX_SWAP_UNDEFINED_OML ||
+ *bp == GLX_SWAP_COPY_OML ||
+ *bp == GLX_SWAP_EXCHANGE_OML) {
+config->swapMethod = *bp++;
+ } else {
+/* X servers with old HW drivers may return any value here, so
+ * assume GLX_SWAP_METHOD_UNDEFINED.
+ */
+config->swapMethod = GLX_SWAP_UNDEFINED_OML;
+bp++;
+ }
  break;
 #endif
   case GLX_SAMPLE_BUFFERS_SGIS:
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/4] dri: Fix __DRIconfig reporting of __DRI_ATTRIB_SWAP_METHOD

[Thomas Hellstrom]

The attribMap had two entries for this attribute, and
driGetConfigAttribIndex didn't return a proper value for this attribute.
Fix this, and also make sure we return SWAP_UNDEFINED for single-buffer
configs as required by the GLX_OML_swap_method spec.

Finally bump the dri core extension version to 2, indicating that we
correctly report __DRI_ATTRIB_SWAP_METHOD.

Signed-off-by: Thomas Hellstrom 
---
 include/GL/internal/dri_interface.h| 5 -
 src/mesa/drivers/dri/common/dri_util.c | 2 +-
 src/mesa/drivers/dri/common/utils.c| 8 ++--
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/include/GL/internal/dri_interface.h 
b/include/GL/internal/dri_interface.h
index f676ac5..5e8fce7 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -726,9 +726,12 @@ struct __DRIuseInvalidateExtensionRec {
 
 /**
  * This extension defines the core DRI functionality.
+ *
+ * Version >= 2 indicates that getConfigAttrib with __DRI_ATTRIB_SWAP_METHOD
+ * returns a reliable value.
  */
 #define __DRI_CORE "DRI_Core"
-#define __DRI_CORE_VERSION 1
+#define __DRI_CORE_VERSION 2
 
 struct __DRIcoreExtensionRec {
 __DRIextension base;
diff --git a/src/mesa/drivers/dri/common/dri_util.c 
b/src/mesa/drivers/dri/common/dri_util.c
index 39ecaf0..31a3040 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -767,7 +767,7 @@ driSwapBuffers(__DRIdrawable *pdp)
 
 /** Core interface */
 const __DRIcoreExtension driCoreExtension = {
-.base = { __DRI_CORE, 1 },
+.base = { __DRI_CORE, 2 },
 
 .createNewScreen= NULL,
 .destroyScreen  = driDestroyScreen,
diff --git a/src/mesa/drivers/dri/common/utils.c 
b/src/mesa/drivers/dri/common/utils.c
index c37d446..f3ea61e 100644
--- a/src/mesa/drivers/dri/common/utils.c
+++ b/src/mesa/drivers/dri/common/utils.c
@@ -284,8 +284,9 @@ driCreateConfigs(mesa_format format,
modes->transparentIndex = GLX_DONT_CARE;
modes->rgbMode = GL_TRUE;
 
-   if ( db_modes[i] == GLX_NONE ) {
+   if ( db_modes[i] == GLX_NONE) {
modes->doubleBufferMode = GL_FALSE;
+modes->swapMethod = GLX_SWAP_UNDEFINED_OML;
}
else {
modes->doubleBufferMode = GL_TRUE;
@@ -403,7 +404,6 @@ static const struct { unsigned int attrib, offset; } 
attribMap[] = {
  * so the iterator includes them though.*/
 __ATTRIB(__DRI_ATTRIB_RENDER_TYPE, level),
 __ATTRIB(__DRI_ATTRIB_CONFIG_CAVEAT,   level),
-__ATTRIB(__DRI_ATTRIB_SWAP_METHOD, level)
 };
 
 
@@ -428,10 +428,6 @@ driGetConfigAttribIndex(const __DRIconfig *config,
else
*value = 0;
break;
-case __DRI_ATTRIB_SWAP_METHOD:
-/* XXX no return value??? */
-   break;
-
 default:
 /* any other int-sized field */
*value = *(unsigned int *)
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 8/8] glsl: use ralloc_str_append() rather than ralloc_asprintf_rewrite_tail()

[Thomas Helland]

This is a nice stopgap until I get the time to finish the
stringbuffer tests and get that stuff merged. I think it should
get us most of the way there, which your numbers suggest.
Patch 7 and patch 8 are:

Reviewed-by: Thomas Helland

2017-08-09 5:34 GMT+02:00 Timothy Arceri :
> The Deus Ex: Mankind Divided shaders go from spending ~20 seconds
> in the GLSL IR compilers front-end down to ~18.5 seconds on a
> Ryzen 1800X.
>
> Tested by compiling once with shader-db then deleting the index file
> from the shader cache and compiling again.
>
> v2:
>  - fix rebasing issue in v1
> ---
>  src/compiler/glsl/glcpp/glcpp-parse.y | 144 
> ++
>  1 file changed, 113 insertions(+), 31 deletions(-)
>
> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y 
> b/src/compiler/glsl/glcpp/glcpp-parse.y
> index f1719f90b1..898a26044f 100644
> --- a/src/compiler/glsl/glcpp/glcpp-parse.y
> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y
> @@ -202,21 +202,26 @@ add_builtin_define(glcpp_parser_t *parser, const char 
> *name, int value);
>  input:
> /* empty */
>  |  input line
>  ;
>
>  line:
> control_line
>  |  SPACE control_line
>  |  text_line {
> _glcpp_parser_print_expanded_token_list (parser, $1);
> -   ralloc_asprintf_rewrite_tail (>output, 
> >output_length, "\n");
> +   const char *newline_str = "\n";
> +   size_t size = strlen(newline_str);
> +
> +   ralloc_str_append(>output, newline_str,
> + parser->output_length, size);
> +   parser->output_length += size;
> }
>  |  expanded_line
>  ;
>
>  expanded_line:
> IF_EXPANDED expression NEWLINE {
> if (parser->is_gles && $2.undefined_macro)
> glcpp_error(& @1, parser, "undefined macro %s in 
> expression (illegal in GLES)", $2.undefined_macro);
> _glcpp_parser_skip_stack_push_if (parser, & @1, $2.value);
> }
> @@ -252,21 +257,26 @@ define:
>  |  FUNC_IDENTIFIER '(' ')' replacement_list NEWLINE {
> _define_function_macro (parser, & @1, $1, NULL, $4);
> }
>  |  FUNC_IDENTIFIER '(' identifier_list ')' replacement_list NEWLINE {
> _define_function_macro (parser, & @1, $1, $3, $5);
> }
>  ;
>
>  control_line:
> control_line_success {
> -   ralloc_asprintf_rewrite_tail (>output, 
> >output_length, "\n");
> +   const char *newline_str = "\n";
> +   size_t size = strlen(newline_str);
> +
> +   ralloc_str_append(>output, newline_str,
> + parser->output_length, size);
> +   parser->output_length += size;
> }
>  |  control_line_error
>  |  HASH_TOKEN LINE pp_tokens NEWLINE {
>
> if (parser->skip_stack == NULL ||
> parser->skip_stack->type == SKIP_NO_SKIP)
> {
> _glcpp_parser_expand_and_lex_from (parser,
>LINE_EXPANDED, $3,
>
> EXPANSION_MODE_IGNORE_DEFINED);
> @@ -1123,72 +1133,144 @@ _token_list_equal_ignoring_space(token_list_t *a, 
> token_list_t *b)
>node_b = node_b->next;
> }
>
> return 1;
>  }
>
>  static void
>  _token_print(char **out, size_t *len, token_t *token)
>  {
> if (token->type < 256) {
> -  ralloc_asprintf_rewrite_tail (out, len, "%c", token->type);
> +  size_t size = sizeof(char);
> +
> +  ralloc_str_append(out, (char *) >type, *len, size);
> +  *len += size;
>return;
> }
>
> switch (token->type) {
> case INTEGER:
>ralloc_asprintf_rewrite_tail (out, len, "%" PRIiMAX, 
> token->value.ival);
>break;
> case IDENTIFIER:
> case INTEGER_STRING:
> -   case OTHER:
> -  ralloc_asprintf_rewrite_tail (out, len, "%s", token->value.str);
> +   case OTHER: {
> +  size_t size = strlen(token->value.str);
> +
> +  ralloc_str_append(out, token->value.str, *len, size);
> +  *len += size;
>break;
> -   case SPACE:
> -  ralloc_asprintf_rewrite_tail (out, len, " ");
> +   }
> +   case SPACE: {
> +  const char *token_str = " ";
> +  size_t size = strlen(token_str);
> +
> +  ralloc_str_append(out, token_str, *len, size);
> +  *len += size;
>break;
> -   case LEFT_SHIFT:
> -  ralloc_asprintf_rewrite_tail (out, len, "<<");
> +   }
> +   case LEFT_SHIFT: {
> +  const char *token_str = "<<";
> +  size_t size = strlen(token_str);
> +
> +  ralloc_str_append(out, token_str, *len, size);
> +  *len += size;
>break;
> -   case RIGHT_SHIFT:
> -  ralloc_asprintf_rewrite_tail (out, len, ">>");
> +   }
> +   case RIGHT_SHIFT: {
> +  const char *token_str = ">>";
> +  size_t size = 

Re: [Mesa-dev] [PATCH 4/8] egl: rework input validation order in _eglCreateWindowSurfaceCommon

[Eric Engestrom]

On Wednesday, 2017-08-09 08:35:04 +0300, Tapani Pälli wrote:
> On 08/08/2017 08:07 PM, Emil Velikov wrote:
> > On 8 August 2017 at 16:10, Eric Engestrom  wrote:
> > > On Saturday, 2017-08-05 00:25:49 +0100, Emil Velikov wrote:
> > > > From: Emil Velikov 
> > > > 
> > > > As mentioned in previous commit the negative tests in dEQP expect the
> > > > arguments to be evaluated in particular order.
> > > The spec doesn't say that, so the test is wrong.
> > > Changing it in Mesa doesn't hurt though, so I have nothing against it,
> > > except for the fact it hide the dEQP bug.
> > > 
> > I agree, the spec does not say anything on the topic.
> > I think it makes sense to have the patch regardless, since it provides
> > a bit more consistency.
> > 
> > Although fixing dEQP is also a good idea. I think Tapani/Chad have
> > some experience/pointers on the topic.
> 
> You can send patches to gerrit in same manner just like for rest of Android,
> then assign reviewers from people that have committed to dEQP. I can help
> trying to get those fixes forward. You should work on master branch though,
> what I've experienced is that getting fixes to some specific release branch
> is a lot more difficult matter.

This is to submit fixes though, which I don't always have the time or
knowledge to do myself. I was hoping there would be a way to report
"this is wrong because X" and let someone else figure out the fix.
Is there any bug/issue tracker for deqp?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] i965: Put freed but busy buffer to the head of the free list

[Chris Wilson]

We treat out free list as a rough LRU, at the tail we have active
buffers to be allocated for rendering, and at the head we have the
older, inactive buffers that we can allocate for use by the CPU.

At the time of freeing, we can inspect the busyness of the buffer to
decide if we should place it at the tail (for reuse of active buffers)
or head (for reuse of inactive buffers). Furthermore, we can reduce the
frequency of calling madvise by only using it for buffers that are
inactive (busy buffers will be pinned by they use by the GPU, so should
not significantly add to mempressure, but will still be reaped by our
timed cache.)
---
 src/mesa/drivers/dri/i965/brw_bufmgr.c | 48 --
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c 
b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index dc693f7d63..a869e7665e 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -112,7 +112,7 @@ struct brw_bufmgr {
/** Array of lists of cached gem objects of power-of-two sizes */
struct bo_cache_bucket cache_bucket[14 * 4];
int num_buckets;
-   time_t time;
+   unsigned time;
 
struct hash_table *name_table;
struct hash_table *handle_table;
@@ -336,7 +336,7 @@ retry:
   }
 
   if (alloc_from_cache) {
- if (!brw_bo_madvise(bo, I915_MADV_WILLNEED)) {
+ if (bo->free_time && !brw_bo_madvise(bo, I915_MADV_WILLNEED)) {
 bo_free(bo);
 brw_bo_cache_purge_bucket(bufmgr, bucket);
 goto retry;
@@ -408,7 +408,7 @@ retry:
 
bo->name = name;
p_atomic_set(>refcount, 1);
-   bo->reusable = true;
+   bo->reusable = bufmgr->bo_reuse;
bo->cache_coherent = bufmgr->has_llc;
bo->index = -1;
 
@@ -678,10 +678,17 @@ bo_free(struct brw_bo *bo)
 
 /** Frees all cached buffers significantly older than @time. */
 static void
-cleanup_bo_cache(struct brw_bufmgr *bufmgr, time_t time)
+cleanup_bo_cache(struct brw_bufmgr *bufmgr)
 {
+   struct timespec tv;
+   unsigned time;
int i;
 
+   clock_gettime(CLOCK_MONOTONIC, );
+   time = tv.tv_sec;
+   if (unlikely(!time)) /* 0 is reserved for unset */
+  time = 1;
+
if (bufmgr->time == time)
   return;
 
@@ -689,12 +696,16 @@ cleanup_bo_cache(struct brw_bufmgr *bufmgr, time_t time)
   struct bo_cache_bucket *bucket = >cache_bucket[i];
 
   list_for_each_entry_safe(struct brw_bo, bo, >head, head) {
- if (time - bo->free_time <= 1)
-break;
-
- list_del(>head);
+ if (!bo->free_time) {
+if (brw_bo_busy(bo))
+   break;
 
- bo_free(bo);
+brw_bo_madvise(bo, I915_MADV_DONTNEED);
+bo->free_time = time;
+ } else if (time - bo->free_time > 1) {
+list_del(>head);
+bo_free(bo);
+ }
   }
}
 
@@ -702,7 +713,7 @@ cleanup_bo_cache(struct brw_bufmgr *bufmgr, time_t time)
 }
 
 static void
-bo_unreference_final(struct brw_bo *bo, time_t time)
+bo_unreference_final(struct brw_bo *bo)
 {
struct brw_bufmgr *bufmgr = bo->bufmgr;
struct bo_cache_bucket *bucket;
@@ -711,14 +722,16 @@ bo_unreference_final(struct brw_bo *bo, time_t time)
 
bucket = bucket_for_size(bufmgr, bo->size);
/* Put the buffer into our internal cache for reuse if we can. */
-   if (bufmgr->bo_reuse && bo->reusable && bucket != NULL &&
-   brw_bo_madvise(bo, I915_MADV_DONTNEED)) {
-  bo->free_time = time;
+   if (bo->reusable && bucket != NULL) {
 
   bo->name = NULL;
   bo->kflags = 0;
 
-  list_addtail(>head, >head);
+  bo->free_time = 0;
+  if (brw_bo_busy(bo))
+ list_addtail(>head, >head);
+  else
+ list_add(>head, >head);
} else {
   bo_free(bo);
}
@@ -734,15 +747,12 @@ brw_bo_unreference(struct brw_bo *bo)
 
if (atomic_add_unless(>refcount, -1, 1)) {
   struct brw_bufmgr *bufmgr = bo->bufmgr;
-  struct timespec time;
-
-  clock_gettime(CLOCK_MONOTONIC, );
 
   pthread_mutex_lock(>lock);
 
   if (p_atomic_dec_zero(>refcount)) {
- bo_unreference_final(bo, time.tv_sec);
- cleanup_bo_cache(bufmgr, time.tv_sec);
+ cleanup_bo_cache(bufmgr);
+ bo_unreference_final(bo);
   }
 
   pthread_mutex_unlock(>lock);
-- 
2.13.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] egl: avoid eglCreatePlatform*Surface{EXT, } crash with invalid dpy

[Tapani Pälli]

Yeah, _eglLookupDisplay seems to return NULL indeed in this case;

Reviewed-by: Tapani Pälli 

On 08/08/2017 05:55 PM, Emil Velikov wrote:

From: Emil Velikov 

If we have an invalid display fed into the functions, the display lookup
will return NULL. Thus as we attempt to get the platform type, we'll
deref. it leading to a crash.

Keep in mind that this will not happen if Mesa is built without X11 or
when the legacy eglCreate*Surface codepaths are used.

An similar check was added with earlier commit 5e97b8f5ce9 ("egl: Fix
crashes in eglCreate*Surface), although it was only applicable when the
surfaceless platform is built.

Cc: mesa-sta...@lists.freedesktop.org
Cc: Tapani Pälli 
Cc: Chad Versace 
Signed-off-by: Emil Velikov 
---
  src/egl/main/eglapi.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index c5e3955c48c..8146009da4f 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -923,7 +923,7 @@ static void *
  _fixupNativeWindow(_EGLDisplay *disp, void *native_window)
  {
  #ifdef HAVE_X11_PLATFORM
-   if (disp->Platform == _EGL_PLATFORM_X11 && native_window != NULL) {
+   if (disp && disp->Platform == _EGL_PLATFORM_X11 && native_window != NULL) {
/* The `native_window` parameter for the X11 platform differs between
 * eglCreateWindowSurface() and eglCreatePlatformPixmapSurfaceEXT(). In
 * eglCreateWindowSurface(), the type of `native_window` is an Xlib
@@ -985,7 +985,7 @@ _fixupNativePixmap(_EGLDisplay *disp, void *native_pixmap)
 * `Pixmap*`.  Convert `Pixmap*` to `Pixmap` because that's what
 * dri2_x11_create_pixmap_surface() expects.
 */
-   if (disp->Platform == _EGL_PLATFORM_X11 && native_pixmap != NULL)
+   if (disp && disp->Platform == _EGL_PLATFORM_X11 && native_pixmap != NULL)
return (void *)(* (Pixmap*) native_pixmap);
  #endif
 return native_pixmap;



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] egl: avoid eglCreatePlatform*Surface{EXT, } crash with invalid dpy

[Eric Engestrom]

On Tuesday, 2017-08-08 15:55:36 +0100, Emil Velikov wrote:
> From: Emil Velikov 
> 
> If we have an invalid display fed into the functions, the display lookup
> will return NULL. Thus as we attempt to get the platform type, we'll
> deref. it leading to a crash.
> 
> Keep in mind that this will not happen if Mesa is built without X11 or
> when the legacy eglCreate*Surface codepaths are used.
> 
> An similar check was added with earlier commit 5e97b8f5ce9 ("egl: Fix
> crashes in eglCreate*Surface), although it was only applicable when the
> surfaceless platform is built.
> 
> Cc: mesa-sta...@lists.freedesktop.org
> Cc: Tapani Pälli 
> Cc: Chad Versace 
> Signed-off-by: Emil Velikov 

Reviewed-by: Eric Engestrom 

> ---
>  src/egl/main/eglapi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index c5e3955c48c..8146009da4f 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c
> @@ -923,7 +923,7 @@ static void *
>  _fixupNativeWindow(_EGLDisplay *disp, void *native_window)
>  {
>  #ifdef HAVE_X11_PLATFORM
> -   if (disp->Platform == _EGL_PLATFORM_X11 && native_window != NULL) {
> +   if (disp && disp->Platform == _EGL_PLATFORM_X11 && native_window != NULL) 
> {
>/* The `native_window` parameter for the X11 platform differs between
> * eglCreateWindowSurface() and eglCreatePlatformPixmapSurfaceEXT(). In
> * eglCreateWindowSurface(), the type of `native_window` is an Xlib
> @@ -985,7 +985,7 @@ _fixupNativePixmap(_EGLDisplay *disp, void *native_pixmap)
> * `Pixmap*`.  Convert `Pixmap*` to `Pixmap` because that's what
> * dri2_x11_create_pixmap_surface() expects.
> */
> -   if (disp->Platform == _EGL_PLATFORM_X11 && native_pixmap != NULL)
> +   if (disp && disp->Platform == _EGL_PLATFORM_X11 && native_pixmap != NULL)
>return (void *)(* (Pixmap*) native_pixmap);
>  #endif
> return native_pixmap;
> -- 
> 2.14.0
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 101334] AMD SI cards: Some vulkan apps freeze the system

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=101334

--- Comment #47 from Marko  ---
That's great news then!

I'm compiling as we speak on Suse OBS, but will able to try it out only this
afternoon after I get home from work.

Cheers,
Marko

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] Update TextureParameter* error for incompatible texture targets

[Iago Toral]

On Tue, 2017-08-08 at 12:12 -0700, Jordan Justen wrote:
> On 2017-08-06 21:18:23, Iago Toral Quiroga wrote:
> > The OpenGL 4.6 specs have been updated so that GetTextureParameter*
> > with a texture object with an incompatible TEXTURE_TARGET should
> > now
> > report INVALID_OPERATION instead of INVALID_ENUM.
> > 
> > Fixes:
> > KHR-GL45.direct_state_access.textures_parameter_errors
> 
> Assuming there is no regression with the GLES CTS, both patches:
> 
> Reviewed-by: Jordan Justen 

Thanks! Jenkins didn't report any regressions in GLES CTS, so I pushed
them.

Notice that this makes a couple of piglit tests that still expect the
previous error codes fail, I have sent a patch to get these tests
updated too:

https://lists.freedesktop.org/archives/piglit/2017-August/022756.html

Iago

> > ---
> >  src/mesa/main/texparam.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
> > index b6e91503ea..039b93349e 100644
> > --- a/src/mesa/main/texparam.c
> > +++ b/src/mesa/main/texparam.c
> > @@ -174,7 +174,7 @@ get_texobj_by_name(struct gl_context *ctx,
> > GLuint texture, const char *name)
> > case GL_TEXTURE_RECTANGLE:
> >    return texObj;
> > default:
> > -  _mesa_error(ctx, GL_INVALID_ENUM, "%s(target)", name);
> > +  _mesa_error(ctx, GL_INVALID_OPERATION, "%s(target)", name);
> >    return NULL;
> > }
> >  
> > -- 
> > 2.11.0
> > 
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 101334] AMD SI cards: Some vulkan apps freeze the system

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=101334

--- Comment #46 from John  ---
This patch only on master worked!

Congratulations Dave, this is the first time I've passed the whole benchmark!

I'll try a few other applications and wait on Marko's test before closing this,
but so far it looks good.


Feel free to add if you couldn't reproduce the original issue:
Tested-by: John Ettedgui 

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/8] glsl: stop copying struct and interface member names

[Thomas Helland]

I thought I sent out a reply to this yesterday. Apparently I was too
tired to remember... I was thinking a helper function to acquire the
name would be nice, but there is not that many uses, so we probably
should not bother. This patch is:

Reviewed-by: Thomas Helland

2017-08-09 5:34 GMT+02:00 Timothy Arceri :
> We are currently copying the name for each member dereference
> but we can just share a single instance of the string provided
> by the type.
>
> This change also stops us recalculating the field index
> repeatedly.
> ---
>  src/compiler/glsl/ast_array_index.cpp  | 14 -
>  src/compiler/glsl/glsl_to_nir.cpp  |  2 +-
>  src/compiler/glsl/ir.cpp   |  8 ++---
>  src/compiler/glsl/ir.h |  4 +--
>  src/compiler/glsl/ir_clone.cpp |  4 ++-
>  src/compiler/glsl/ir_constant_expression.cpp   |  4 +--
>  src/compiler/glsl/ir_print_visitor.cpp |  5 +++-
>  src/compiler/glsl/linker.cpp   |  9 +-
>  src/compiler/glsl/lower_buffer_access.cpp  |  6 ++--
>  src/compiler/glsl/lower_named_interface_blocks.cpp |  3 +-
>  src/compiler/glsl/opt_structure_splitting.cpp  | 10 ++-
>  src/mesa/program/ir_to_mesa.cpp|  6 ++--
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 34 
> ++
>  13 files changed, 50 insertions(+), 59 deletions(-)
>
> diff --git a/src/compiler/glsl/ast_array_index.cpp 
> b/src/compiler/glsl/ast_array_index.cpp
> index f6b7a64a28..efddbed6ea 100644
> --- a/src/compiler/glsl/ast_array_index.cpp
> +++ b/src/compiler/glsl/ast_array_index.cpp
> @@ -81,37 +81,37 @@ update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE 
> *loc,
>   while (deref_array != NULL) {
>  deref_array_prev = deref_array;
>  deref_array = deref_array->array->as_dereference_array();
>   }
>   if (deref_array_prev != NULL)
>  deref_var = deref_array_prev->array->as_dereference_variable();
>}
>
>if (deref_var != NULL) {
>   if (deref_var->var->is_interface_instance()) {
> -unsigned field_index =
> -   deref_record->record->type->field_index(deref_record->field);
> -assert(field_index < 
> deref_var->var->get_interface_type()->length);
> +unsigned field_idx = deref_record->field_idx;
> +assert(field_idx < deref_var->var->get_interface_type()->length);
>
>  int *const max_ifc_array_access =
> deref_var->var->get_max_ifc_array_access();
>
>  assert(max_ifc_array_access != NULL);
>
> -if (idx > max_ifc_array_access[field_index]) {
> -   max_ifc_array_access[field_index] = idx;
> +if (idx > max_ifc_array_access[field_idx]) {
> +   max_ifc_array_access[field_idx] = idx;
>
> /* Check whether this access will, as a side effect, 
> implicitly
>  * cause the size of a built-in array to be too large.
>  */
> -   check_builtin_array_max_size(deref_record->field, idx+1, *loc,
> -state);
> +   const char *field_name =
> +  
> deref_record->record->type->fields.structure[field_idx].name;
> +   check_builtin_array_max_size(field_name, idx+1, *loc, state);
>  }
>   }
>}
> }
>  }
>
>
>  static int
>  get_implicit_array_size(struct _mesa_glsl_parse_state *state,
>  ir_rvalue *array)
> diff --git a/src/compiler/glsl/glsl_to_nir.cpp 
> b/src/compiler/glsl/glsl_to_nir.cpp
> index e5166855e8..bb2ba17b22 100644
> --- a/src/compiler/glsl/glsl_to_nir.cpp
> +++ b/src/compiler/glsl/glsl_to_nir.cpp
> @@ -2198,21 +2198,21 @@ nir_visitor::visit(ir_dereference_variable *ir)
> nir_deref_var *deref = nir_deref_var_create(this->shader, var);
> this->deref_head = deref;
> this->deref_tail = >deref;
>  }
>
>  void
>  nir_visitor::visit(ir_dereference_record *ir)
>  {
> ir->record->accept(this);
>
> -   int field_index = this->deref_tail->type->field_index(ir->field);
> +   int field_index = ir->field_idx;
> assert(field_index >= 0);
>
> nir_deref_struct *deref = nir_deref_struct_create(this->deref_tail, 
> field_index);
> deref->deref.type = ir->type;
> this->deref_tail->child = >deref;
> this->deref_tail = >deref;
>  }
>
>  void
>  nir_visitor::visit(ir_dereference_array *ir)
> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
> index 98bbd91539..52ca83689e 100644
> --- a/src/compiler/glsl/ir.cpp
> +++ b/src/compiler/glsl/ir.cpp
> @@ -1097,24 +1097,22 @@ ir_constant::get_array_element(unsigned i) const
>  */
> if (int(i) < 0)
>i = 0;
> else if (i >= this->type->length)
>i = this->type->length - 1;
>
> return 

Re: [Mesa-dev] [PATCH 2/2] dri3: Swapbuffer update

[Thomas Hellstrom]

Hi, Michel,

On 08/09/2017 08:36 AM, Michel Dänzer wrote:

Hi Thomas,


first of all, would it be possible to split these patches up a bit
further? At least patch 1 seems to contain several logical changes,
which makes it a bit difficult to review.


OK. I'll try to do that.




On 08/08/17 03:48 PM, Thomas Hellstrom wrote:

Implement back-to-fake-front flips,
Fix EGL_BUFFER_PRESERVED path.
Implement dri3 support for GLX_SWAP_EXCHANGE_OML and GLX_SWAP_COPY_OML.

The back-to-fake-front flips will save a full buffer copy in the case of a
fake front being enabled and GLX_SWAP_UNDEFINED_OML.

Support for EGL_BUFFER_PRESERVED and GLX_SWAP_X_OML are mostly useful for
things like glretrace if traces are capured with applications relying on a
specific swapbuffer behavior.

The EGL_BUFFER_PRESERVED path previously made sure the present was done as
a copy, but there was nothing making sure that after the present,
the same back buffer was chosen.
This has now been changed so that if the previous back buffer is
idle, we reuse it. Otherwise we grab a new and copy the contents and
buffer age from the previous back buffer. Server side flips are allowed.

GLX_SWAP_COPY_OML will behave like EGL_BUFFER_PRESERVED.

GLX_SWAP_EXCHANGE_OML will behave similarly, except that we try to reuse the
previous fake front as the new back buffer if it's idle. If not, we grab
a new back buffer and copy the contents and buffer age from the old fake front.

I'm not sure it's worth copying the contents of the desired next back
buffer to a different one and using that instead. There might be cases
where doing so results in lower performance than simply using the
desired back buffer anyway. Have you made any measurements WRT this?


Agreed. I haven't done any measurements but my reasoning was that 
probably any performance loss would be mostly associated with the 
allocating itself, hence a short-lived problem once enough buffers are 
allocated. The copying would, at least on modern tiling hardware, 
probably not be a big loss since we don't flush.




With EGL_BUFFER_PRESERVED/GLX_SWAP_COPY_OML, always re-using the same
back buffer means that the client only needs to allocate one back
buffer, resulting in lower graphics memory consumption.



True. But there are some implications:

First, with GLX_SWAP_COPY_OML, reusing the back buffer means we need to 
disable server-side page-flipping, otherwise the buffer would never be 
idle. Second, if we have a fake front, there will be at least one copy 
anyway. So in essence we need to disable server-side page-flipping and 
might not gain anything in the end if using a fake front.


With GLX_SWAP_EXCHANGE_OML, we'd be reusing the old fake front which 
will, after a delay, always be idle. Also we don't need an extra copy so 
the only loss will be a delay which might impact latency-sensitive 
applications. I don't expect SWAP_EXCHANGE will be used much anyway and 
even if we enable it in st/dri, it won't be advertised until the server 
side AIGLX starts to use the image loader extension or we rewrite the 
GLX fbconfig selection.


What do you think of adding a driConf option to select "wait for 
backbuffer idle in swapbuffers"


/Thomas




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 101334] AMD SI cards: Some vulkan apps freeze the system

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=101334

--- Comment #45 from Dave Airlie  ---
https://patchwork.freedesktop.org/series/28535/

is a replacement for the cs flush, might be worth trying on master on its own.

and possibly with the hack patch on top (it might not apply cleanly though).

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] radv: force cs/ps/l2 flush at end of command stream. (v2)

[Dave Airlie]

From: Dave Airlie 

This seems like a workaround, but we don't see the bug on CIK/VI.

On SI with the dEQP-VK.memory.pipeline_barrier.host_read_transfer_dst.*
tests, when one tests complete, the first flush at the start of the next
test causes a VM fault as we've destroyed the VM, but we end up flushing
the compute shader then, and it must still be in the process of doing
something.

Could also be a kernel difference between SI and CIK.

v2: hit this with a bigger hammer. This fixes a bunch of hangs
in the vk cts with the robustness tests.

Signed-off-by: Dave Airlie 
---
 src/amd/vulkan/radv_cmd_buffer.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index f354fed..5d12c17 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -2232,8 +2232,11 @@ VkResult radv_EndCommandBuffer(
 {
RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
 
-   if (cmd_buffer->queue_family_index != RADV_QUEUE_TRANSFER)
+   if (cmd_buffer->queue_family_index != RADV_QUEUE_TRANSFER) {
+   if (cmd_buffer->device->physical_device->rad_info.chip_class == 
SI)
+   cmd_buffer->state.flush_bits |= 
RADV_CMD_FLAG_CS_PARTIAL_FLUSH | RADV_CMD_FLAG_PS_PARTIAL_FLUSH | 
RADV_CMD_FLAG_WRITEBACK_GLOBAL_L2;
si_emit_cache_flush(cmd_buffer);
+   }
 
if (!cmd_buffer->device->ws->cs_finalize(cmd_buffer->cs) ||
cmd_buffer->record_fail)
-- 
2.9.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] dri3: Swapbuffer update

[Michel Dänzer]

Hi Thomas,


first of all, would it be possible to split these patches up a bit
further? At least patch 1 seems to contain several logical changes,
which makes it a bit difficult to review.


On 08/08/17 03:48 PM, Thomas Hellstrom wrote:
> Implement back-to-fake-front flips,
> Fix EGL_BUFFER_PRESERVED path.
> Implement dri3 support for GLX_SWAP_EXCHANGE_OML and GLX_SWAP_COPY_OML.
> 
> The back-to-fake-front flips will save a full buffer copy in the case of a
> fake front being enabled and GLX_SWAP_UNDEFINED_OML.
> 
> Support for EGL_BUFFER_PRESERVED and GLX_SWAP_X_OML are mostly useful for
> things like glretrace if traces are capured with applications relying on a
> specific swapbuffer behavior.
> 
> The EGL_BUFFER_PRESERVED path previously made sure the present was done as
> a copy, but there was nothing making sure that after the present,
> the same back buffer was chosen.
> This has now been changed so that if the previous back buffer is
> idle, we reuse it. Otherwise we grab a new and copy the contents and
> buffer age from the previous back buffer. Server side flips are allowed.
> 
> GLX_SWAP_COPY_OML will behave like EGL_BUFFER_PRESERVED.
> 
> GLX_SWAP_EXCHANGE_OML will behave similarly, except that we try to reuse the
> previous fake front as the new back buffer if it's idle. If not, we grab
> a new back buffer and copy the contents and buffer age from the old fake 
> front.

I'm not sure it's worth copying the contents of the desired next back
buffer to a different one and using that instead. There might be cases
where doing so results in lower performance than simply using the
desired back buffer anyway. Have you made any measurements WRT this?

With EGL_BUFFER_PRESERVED/GLX_SWAP_COPY_OML, always re-using the same
back buffer means that the client only needs to allocate one back
buffer, resulting in lower graphics memory consumption.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v5 2/2] i965: Queue the buffer with a sync fence for Android OS

[Tomasz Figa]

On Wed, Aug 9, 2017 at 3:17 PM, Marathe, Yogesh
 wrote:
> Tomasz,
>
>> -Original Message-
>> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf
>> Of Tomasz Figa
>> Sent: Tuesday, August 8, 2017 7:43 AM
>> To: Marathe, Yogesh 
>> On Mon, Aug 7, 2017 at 2:19 PM, Marathe, Yogesh
>>  wrote:
>> > Tomasz,
>> >
>> >> -Original Message-
>> >> From: Tomasz Figa [mailto:tf...@chromium.org]
>> >> Sent: Saturday, August 5, 2017 8:47 AM
>> >>
>> >> Hi Yogesh,
>> >>
>> >> On Sat, Aug 5, 2017 at 1:22 AM, Marathe, Yogesh
>> >> 
>> >> wrote:
>> >> >> -Original Message-
>> >> >> From: Tomasz Figa [mailto:tf...@chromium.org]
>> >> >> Sent: Friday, August 4, 2017 9:39 PM On Sat, Aug 5, 2017 at 12:53
>> >> >> AM, Marathe, Yogesh  wrote:
>> >> >> > Tomasz, Emil,
>> >> >> >
>> >> >> >> -Original Message-
>> >> >> >> From: Tomasz Figa [mailto:tf...@chromium.org] On Fri, Aug 4,
>> >> >> >> 2017 at 9:55 PM, Emil Velikov 
>> >> >> wrote:
>> >> >> >> >>> >>  - version check (2+) the fence extension, calling
>> >> >> >> >>> >> .create_fence_fd() only when
>> >> >> >> >>> >> .get_capabilities() advertises __DRI_FENCE_CAP_NATIVE_FD
>> >> >> >> >>
>> >> >> >> >> The check looks like below now, this is in
>> >> >> >> >> dri2_surf_update_fence_fd() before
>> >> >> >> create_fence_fd is called.
>> >> >> >> >>
>> >> >> >> >> if (dri2_surf->enable_out_fence && dri2_dpy->fence) {
>> >> >> >> >>if(__DRI_FENCE_CAP_NATIVE_FD | dri2_dpy->fence-
>> >> >> >> >get_capabilities(dri2_dpy->dri_screen)) {
>> >> >> >> >>   //create_fence_fd call
>> >> >> >> >>}
>> >> >> >> >> }
>> >> >> >> >>
>> >> >> >> > Close but no cigar.
>> >> >> >> >
>> >> >> >> > if (dri2_surf->enable_out_fence && dri2_dpy->fence &&
>> >> >> >> > dri2_dpy->fence->base.version >= 2 &&
>> >> >> >> > dri2_dpy->fence->get_capabilities) {
>> >> >> >> >
>> >> >> >> >if
>> >> >> >> > (dri2_dpy->fence->get_capabilities(dri2_dpy->dri_screen) &
>> >> >> >> > __DRI_FENCE_CAP_NATIVE_FD) {
>> >> >> >> > //create_fence_fd call
>> >> >> >> >}
>> >> >> >> > }
>> >> >> >>
>> >> >> >> If this needs so complicated series of checks, maybe it would
>> >> >> >> make more sense to just set enable_out_fence based on
>> >> >> >> availability of the capability at initialization time?
>> >> >> >
>> >> >> > I liked this one compared to nested ifs in 
>> >> >> > dri2_surf_update_fence_fd().
>> >> >> >
>> >> >> >>
>> >> >> >> >
>> >> >> >> >> Overall, if I further go ahead and check, actually
>> >> >> >> >> get_capabilities() ultimately returns based on
>> >> >> >> >> has_exec_fence which depends on I915_PARAM_HAS_EXEC_FENCE.
>> >> >> >> >> This is always set to true for i915 in kernel drv unless
>> >> >> >> >> forced to false!! I'm not sure if that inner check of
>> >> >> >> get_capabilities still makes sense. Isn't the first one sufficient?
>> >> >> >> >>
>> >> >> >> > Not sure what you mean with "first one", but consider the
>> >> >> >> > following
>> >> >> example:
>> >> >> >> >  - old kernel which does not support (or has force disabled)
>> >> >> >> > I915_PARAM_HAS_EXEC_FENCE.
>> >> >> >> >  - new userspace which unconditionally advertises the fence
>> >> >> >> > v2 extension IIRC one may tweak that things to only
>> >> >> >> > conditionally advertise it, but IMHO it's not worth the hassle.
>> >> >> >> >
>> >> >> >> > Even then, Mesa can produce 20 DRI drivers (used by the EGL
>> >> >> >> > module) so focusing on one doesn't quite work.
>> >> >> >> >
>> >> >> >> >>> >>  - don't introduce unused variables (in make_current)
>> >> >> >> >>
>> >> >> >> >> Done.
>> >> >> >> >>
>> >> >> >> >>> >>  - the create fd for the old display surface (in
>> >> >> >> >>> >> make_current) seems bogus
>> >> >> >> >>
>> >> >> >> >> Done.
>> >> >> >> >>
>> >> >> >> > Did you drop it all together or changed to use some other surface?
>> >> >> >> > Would be nice to hear the reason why it was added - perhaps
>> >> >> >> > I'm missing something.
>> >> >> >>
>> >> >> >> We have to keep it, otherwise there would be no fence available
>> >> >> >> at the time of surface destruction, while, at least for
>> >> >> >> Android, a fence can be passed to window's cancelBuffer callback.
>> >> >> >>
>> >> >> >> >
>> >> >> >> > I think that we want a fence/fd for the new draw surface.
>> >> >> >> > Since otherwise one won't get created up until the first 
>> >> >> >> > SwapBuffers
>> call.
>> >> >> >>
>> >> >> >> I might be missing something, but wouldn't that insert a fence
>> >> >> >> at the beginning of command stream, before even doing anything?
>> >> >> >> At least in Android use cases, the only places we need the
>> >> >> >> fence is in SwapBuffers and DestroySurface and the fence should
>> >> >> >> be inserted after all the commands for rendering into given surface.
>> >> >> >>
>> >> 

Re: [Mesa-dev] [PATCH v5 2/2] i965: Queue the buffer with a sync fence for Android OS

[Marathe, Yogesh]

Tomasz,

> -Original Message-
> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf
> Of Tomasz Figa
> Sent: Tuesday, August 8, 2017 7:43 AM
> To: Marathe, Yogesh 
> On Mon, Aug 7, 2017 at 2:19 PM, Marathe, Yogesh
>  wrote:
> > Tomasz,
> >
> >> -Original Message-
> >> From: Tomasz Figa [mailto:tf...@chromium.org]
> >> Sent: Saturday, August 5, 2017 8:47 AM
> >>
> >> Hi Yogesh,
> >>
> >> On Sat, Aug 5, 2017 at 1:22 AM, Marathe, Yogesh
> >> 
> >> wrote:
> >> >> -Original Message-
> >> >> From: Tomasz Figa [mailto:tf...@chromium.org]
> >> >> Sent: Friday, August 4, 2017 9:39 PM On Sat, Aug 5, 2017 at 12:53
> >> >> AM, Marathe, Yogesh  wrote:
> >> >> > Tomasz, Emil,
> >> >> >
> >> >> >> -Original Message-
> >> >> >> From: Tomasz Figa [mailto:tf...@chromium.org] On Fri, Aug 4,
> >> >> >> 2017 at 9:55 PM, Emil Velikov 
> >> >> wrote:
> >> >> >> >>> >>  - version check (2+) the fence extension, calling
> >> >> >> >>> >> .create_fence_fd() only when
> >> >> >> >>> >> .get_capabilities() advertises __DRI_FENCE_CAP_NATIVE_FD
> >> >> >> >>
> >> >> >> >> The check looks like below now, this is in
> >> >> >> >> dri2_surf_update_fence_fd() before
> >> >> >> create_fence_fd is called.
> >> >> >> >>
> >> >> >> >> if (dri2_surf->enable_out_fence && dri2_dpy->fence) {
> >> >> >> >>if(__DRI_FENCE_CAP_NATIVE_FD | dri2_dpy->fence-
> >> >> >> >get_capabilities(dri2_dpy->dri_screen)) {
> >> >> >> >>   //create_fence_fd call
> >> >> >> >>}
> >> >> >> >> }
> >> >> >> >>
> >> >> >> > Close but no cigar.
> >> >> >> >
> >> >> >> > if (dri2_surf->enable_out_fence && dri2_dpy->fence &&
> >> >> >> > dri2_dpy->fence->base.version >= 2 &&
> >> >> >> > dri2_dpy->fence->get_capabilities) {
> >> >> >> >
> >> >> >> >if
> >> >> >> > (dri2_dpy->fence->get_capabilities(dri2_dpy->dri_screen) &
> >> >> >> > __DRI_FENCE_CAP_NATIVE_FD) {
> >> >> >> > //create_fence_fd call
> >> >> >> >}
> >> >> >> > }
> >> >> >>
> >> >> >> If this needs so complicated series of checks, maybe it would
> >> >> >> make more sense to just set enable_out_fence based on
> >> >> >> availability of the capability at initialization time?
> >> >> >
> >> >> > I liked this one compared to nested ifs in 
> >> >> > dri2_surf_update_fence_fd().
> >> >> >
> >> >> >>
> >> >> >> >
> >> >> >> >> Overall, if I further go ahead and check, actually
> >> >> >> >> get_capabilities() ultimately returns based on
> >> >> >> >> has_exec_fence which depends on I915_PARAM_HAS_EXEC_FENCE.
> >> >> >> >> This is always set to true for i915 in kernel drv unless
> >> >> >> >> forced to false!! I'm not sure if that inner check of
> >> >> >> get_capabilities still makes sense. Isn't the first one sufficient?
> >> >> >> >>
> >> >> >> > Not sure what you mean with "first one", but consider the
> >> >> >> > following
> >> >> example:
> >> >> >> >  - old kernel which does not support (or has force disabled)
> >> >> >> > I915_PARAM_HAS_EXEC_FENCE.
> >> >> >> >  - new userspace which unconditionally advertises the fence
> >> >> >> > v2 extension IIRC one may tweak that things to only
> >> >> >> > conditionally advertise it, but IMHO it's not worth the hassle.
> >> >> >> >
> >> >> >> > Even then, Mesa can produce 20 DRI drivers (used by the EGL
> >> >> >> > module) so focusing on one doesn't quite work.
> >> >> >> >
> >> >> >> >>> >>  - don't introduce unused variables (in make_current)
> >> >> >> >>
> >> >> >> >> Done.
> >> >> >> >>
> >> >> >> >>> >>  - the create fd for the old display surface (in
> >> >> >> >>> >> make_current) seems bogus
> >> >> >> >>
> >> >> >> >> Done.
> >> >> >> >>
> >> >> >> > Did you drop it all together or changed to use some other surface?
> >> >> >> > Would be nice to hear the reason why it was added - perhaps
> >> >> >> > I'm missing something.
> >> >> >>
> >> >> >> We have to keep it, otherwise there would be no fence available
> >> >> >> at the time of surface destruction, while, at least for
> >> >> >> Android, a fence can be passed to window's cancelBuffer callback.
> >> >> >>
> >> >> >> >
> >> >> >> > I think that we want a fence/fd for the new draw surface.
> >> >> >> > Since otherwise one won't get created up until the first 
> >> >> >> > SwapBuffers
> call.
> >> >> >>
> >> >> >> I might be missing something, but wouldn't that insert a fence
> >> >> >> at the beginning of command stream, before even doing anything?
> >> >> >> At least in Android use cases, the only places we need the
> >> >> >> fence is in SwapBuffers and DestroySurface and the fence should
> >> >> >> be inserted after all the commands for rendering into given surface.
> >> >> >>
> >> >> >
> >> >> > Emil,
> >> >> >
> >> >> > Tomasz sounds convincing to me here, I just went ahead with the
> >> >> > comment to try out and flatland worked even after removing that.
> >> >> > Zhongmin 

[Mesa-dev] [Bug 101334] AMD SI cards: Some vulkan apps freeze the system

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=101334

--- Comment #44 from John  ---
I've just tried on amd-staging 4.12, and without the hacky patch, and it still
froze the same (still heavy I/O when it did, but nothing in dmesg this time).

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 102030] private memory overflow in openCL

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=102030

--- Comment #5 from Janpieter Sollie  ---
Created attachment 133403
  --> https://bugs.freedesktop.org/attachment.cgi?id=133403=edit
working program source

the source contains the following modifications:
-use of local memory
-insertion of debug function
- removal of atom_add

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 102030] private memory overflow in openCL

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=102030

--- Comment #4 from Janpieter Sollie  ---
I got it running on both pocl and clover.  I will attach a working version with
a built_hints.txt file included, which also includes debug information to debug
memory contents.
what I concluded:
- atom_add does not work correctly
- this workaround works, where it should not:
i = ctx->l1;
i+= len << 3;
atom_xchg(&(ctx->l1), i);

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev