date:20170403

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

2017-04-03 Thread Tomasz Figa

Hi Mauro,

On Mon, Apr 3, 2017 at 2:48 AM, Mauro Rossi  wrote:
>
>
> 2017-03-31 13:05 GMT+02:00 Tapani Pälli :
>>
>>
>>
>> On 03/31/2017 10:12 AM, Tapani Pälli wrote:
>>>
>>>
>>>
>>> On 03/31/2017 09:06 AM, Tapani Pälli wrote:



 On 03/31/2017 08:24 AM, Rob Clark wrote:
>
> On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli
>  wrote:
>>
>>
>>
>> On 03/30/2017 05:57 PM, Emil Velikov wrote:
>>>
>>>
>>> On 30 March 2017 at 15:30, Tomasz Figa  wrote:


 On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov
 
 wrote:
>
>
>
> On 30 March 2017 at 11:55, Tomasz Figa  wrote:
>>
>>
>> Android buffer queues can be abandoned, which results in failing
>> to
>> dequeue next buffer. Currently this would fail somewhere deep
>> within
>> the DRI stack calling loader's getBuffers*(), without any error
>> reporting to the client app. However Android framework code
>> relies on
>> proper signaling of this event, so we move buffer dequeue to
>> createWindowSurface() and swapBuffers() call, which can generate
>> proper
>> EGL errors. To keep the performance benefits of delayed buffer
>> handling,
>> if any, fence wait and DRI image creation is kept delayed until
>> getBuffers*() is called by the DRI driver.
>>
> Thank you Tomasz.
>
> I'm fairly confident that this should resolve the crash [in
> swap_buffers] that Mauro was seeing.
> Mauro can you give it a test ?



 Ah, I actually noticed a problem with existing code, supposedly
 fixed
 by [1], but I'm afraid it's still wrong.

 Current swap_buffers calls get_back_bo(), but doesn't call
 update_buffers(), which is the function that should be called before
 to actually dequeue a buffer from Android's buffer queue. Given
 that,
 get_back_bo() would simply fail with !dri2_surf->buffer, because no
 buffer was dequeued.

>>> Right - I was wondering why we don't hit that on EGL/GBM or
>>> EGL/Wayland.
>>> From a quick look - may be because EGL/Android drops the dpy mutex in
>>> droid_window_enqueue_buffer().
>>>
 My patch removes update_buffers() and changes the buffer
 management so
 that there is always a buffer dequeued, starting from surface
 creation, unless there was an error somewhere.

>>> Of the top of your head - is there something stopping us from using
>>> the same method on $other platforms?
>>>
 [1]

 https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597



>
>
> Not that huge of an expert on the Android specifics, so just a
> humble
> request:
> Can we seek the code resuffle (droid_{alloc,free}_local_buffer,
>>>
>>>
>>> Oops silly typo - s/seek/split/.
>>>
> other?) separate from the functionality changes ?



 Sure. Thanks for suggestion.

>>> Please give it a day or two for others to comment.
>>
>>
>>
>> I'm trying to debug why this causes our homescreen (wallpaper) to be
>> black.
>> Otherwise I haven't seen any issues with these changes.
>>
>
> wallpaper seems to be a special sorta hell..  I wonder if there is
> somehow some sort of interaction with what I fixed / worked-around in
> a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??
>
> Maybe at least try commenting out the temp-pbuffer thing to get max
> texture size, and see if that "fixes" things


 Can you give more details, I still live in la la land and don't know
 about 'temp-pbuffer thing'?

>>>
>>> aa I did not recall the problem, you mean the 'dummy pbuffer' in
>>> SurfaceFlinger .. yes I will check if this is related.
>>>
>>
>> If I take away that dummy pbuffer usage (which is useless anyway), couple
>> of errors disappear from the log. They are:
>>
>> SurfaceFlinger: releasePendingBuffer failed: Unknown error -1 (1)
>> SurfaceFlinger: releasePendingBuffer failed: Unknown error -1 (1)
>>
>> but otherwise the desktop still stays black, live wallpapers seem to work
>> so there is something special about this default wallpaper. Will continue
>> digging ..
>>
>> // Tapani
>
>
>
> Hi,
>
> about the black wallpaper the only sign found in logcat is the following:
>
> - beginning of main
> 04-02 15:09:43.148
> ...
> 04-02 15:10:41.710  1414  1414 E Layer   :
> [com.android.systemui.ImageWallpaper] rejecting buffer: bufWidth=1024,
> bufHeight=768, front.active.{w=1, h=1}
> ...
> 04-02 15:10:41.726  1414  1414 E Layer   :
> [com.andr

Re: [Mesa-dev] [PATCH] st/omx/dec: Properly undefine DEBUG macro

2017-04-03 Thread Christian König


Am 03.04.2017 um 06:35 schrieb Shaleen Jain:

---
  src/gallium/state_trackers/omx/vid_dec.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/state_trackers/omx/vid_dec.c 
b/src/gallium/state_trackers/omx/vid_dec.c
index 9a6efb8e28..94664eba04 100644
--- a/src/gallium/state_trackers/omx/vid_dec.c
+++ b/src/gallium/state_trackers/omx/vid_dec.c
@@ -37,7 +37,7 @@
  #include 
  
  /* bellagio defines a DEBUG macro that we don't want */

-#ifndef DEBUG
+#ifdef DEBUG


NAK, the logic locked correct as it was before.

Why do you want to invert it?

Regards,
Christian.


  #include 
  #undef DEBUG
  #else



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

Re: [Mesa-dev] [PATCH] glsl: add null pointer check in print_without_declaration()

2017-04-03 Thread Nicolai Hähnle


On 31.03.2017 20:14, Brian Paul wrote:

On 03/31/2017 12:01 AM, Nicolai Hähnle wrote:

On 31.03.2017 05:21, Brian Paul wrote:

To avoid/fix a segmentation fault when running the stand-alone GLSL
compiler utility for cases such as the Mesa demos toyball test:

glsl_compiler --dump-builder --version 120 CH11-toyball.vert
CH11-toyball.frag
---
 src/compiler/glsl/ir_builder_print_visitor.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ir_builder_print_visitor.cpp
b/src/compiler/glsl/ir_builder_print_visitor.cpp
index 825dbe1..164a237 100644
--- a/src/compiler/glsl/ir_builder_print_visitor.cpp
+++ b/src/compiler/glsl/ir_builder_print_visitor.cpp
@@ -581,7 +581,9 @@
ir_builder_print_visitor::print_without_declaration(const
ir_expression *ir)
  const struct hash_entry *const he =
 _mesa_hash_table_search(index_map, ir->operands[i]);

- print_without_indent("r%04X", (unsigned)(uintptr_t) he->data);
+ if (he) {
+print_without_indent("r%04X", (unsigned)(uintptr_t)
he->data);
+ }


Is the output still usable in this case? I don't quite understand the
use case and how this case can happen.


I didn't dig either and would appreciate any feedback.  I'm merely
trying to avoid a segfault (found while debugging the MinGW optimization
issue).


I'm not really familiar with that code and couldn't figure out from 
looking at it when/why it happens, but at least looking at the 
surrounding code, I'd say that this cannot possibly produce a good 
output when 'he' is NULL. The point is to generate well-formed C++ to be 
compiled later, and skipping the print will just lead to writing out a 
function call with a malformed parameter list.


Maybe the answer is that the --dump-builder simply isn't made for this 
type of use, in which case the question becomes: why do you want to use 
it this way? And if there's a good reason for using it this way, the bug 
needs to be understood properly. Maybe Ian knows more? I believe he 
wrote most of that code.


Nicolai
--
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] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

2017-04-03 Thread Tapani Pälli

On 03/31/2017 04:04 PM, Rob Clark wrote:

On Fri, Mar 31, 2017 at 2:06 AM, Tapani Pälli wrote:

On 03/31/2017 08:24 AM, Rob Clark wrote:

On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli
wrote:

On 03/30/2017 05:57 PM, Emil Velikov wrote:

On 30 March 2017 at 15:30, Tomasz Figa wrote:

On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov

wrote:

On 30 March 2017 at 11:55, Tomasz Figa wrote:

Android buffer queues can be abandoned, which results in failing to
dequeue next buffer. Currently this would fail somewhere deep within
the DRI stack calling loader's getBuffers*(), without any error
reporting to the client app. However Android framework code relies on
proper signaling of this event, so we move buffer dequeue to
createWindowSurface() and swapBuffers() call, which can generate
proper
EGL errors. To keep the performance benefits of delayed buffer
handling,
if any, fence wait and DRI image creation is kept delayed until
getBuffers*() is called by the DRI driver.

Thank you Tomasz.

I'm fairly confident that this should resolve the crash [in
swap_buffers] that Mauro was seeing.
Mauro can you give it a test ?

Ah, I actually noticed a problem with existing code, supposedly fixed
by [1], but I'm afraid it's still wrong.

Current swap_buffers calls get_back_bo(), but doesn't call
update_buffers(), which is the function that should be called before
to actually dequeue a buffer from Android's buffer queue. Given that,
get_back_bo() would simply fail with !dri2_surf->buffer, because no
buffer was dequeued.

Right - I was wondering why we don't hit that on EGL/GBM or EGL/Wayland.
From a quick look - may be because EGL/Android drops the dpy mutex in
droid_window_enqueue_buffer().

My patch removes update_buffers() and changes the buffer management so
that there is always a buffer dequeued, starting from surface
creation, unless there was an error somewhere.

Of the top of your head - is there something stopping us from using
the same method on $other platforms?

[1]

https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597

Not that huge of an expert on the Android specifics, so just a humble
request:
Can we seek the code resuffle (droid_{alloc,free}_local_buffer,

Oops silly typo - s/seek/split/.

other?) separate from the functionality changes ?

Sure. Thanks for suggestion.

Please give it a day or two for others to comment.

I'm trying to debug why this causes our homescreen (wallpaper) to be
black.
Otherwise I haven't seen any issues with these changes.

wallpaper seems to be a special sorta hell.. I wonder if there is
somehow some sort of interaction with what I fixed / worked-around in
a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??

Maybe at least try commenting out the temp-pbuffer thing to get max
texture size, and see if that "fixes" things

Can you give more details, I still live in la la land and don't know about
'temp-pbuffer thing'?

Sorry, this is something in the wallpaper app java code.. I forget
exactly where it lives now, but there was some code matching what I
pasted in the commit msg which was creating and then destroying a
temporary pbuffer..

OK yeah found it, it's very similar to what surfaceflinger does also on
startup before initializing it's rendering engine. It creates temporary
pbuffer only to query different things before actual init.

1 2 >

1 - 100 of 118 matches

Mail list logo