Re: [Mesa-dev] Playing with display timing -- VK_MESA_present_period

2020-02-03 Thread Matias N. Goldberg 
  Hi!
I read your article.
What I feel are missing are just minor pesky details:
1. Written as is, the frame being submitted is rounded up to display timing, 
delaying *future* frames.But there is no way to delay the *currently displaying 
frame* (i.e. the 'previous' frame).
Right now if frame N was submitted without VkPresentTimeMESA but frame N+1 is, 
then frame N+1 will be presented to screen ASAP.
What I'm saying is that if frame N was submitted without VkPresentTimeMESA, 
then at frame N+1 I should be able to tell 'keep frame N displayed for at least 
P nanoseconds since it was displayed, and *then* present frame N+1, which is 
the frame I am now submitting'
> However, I think allowing the period to be specified in frames might be> a 
> mistake, because it won't work well with variable refresh rate
The API needs to be expanded further to explain Vulkan what a 'frame' is.Is it 
the monitor's refresh rate?
Or is it an arbitrary elapsed time defined in the form numerator and 
denominator? (e.g. 60hz is numerator = 1, denominator = 60; 59.94hz is 
numerator = 1001 denominator = 6000)By specifying arbitrary definitions of a 
frame, it is possible to be compatible with variable refresh rates e.g. for VRR 
monitors, applications may define denominator = 240 or denominator = 120
It should also be possible to dynamically change how long a frame lasts, in 
case the GPU simply can't catch up (e.g. specifying a denominator = 240 and 
using frames when the GPU can only render at 30hz is almost the same as 
presenting ASAP i.e. the same as not using VK_MESA_present_period at all)
Specifying denominator = 0 means using the monitor's native refresh rate. If 
such concept makes no sense (e.g. VRR?) then the behavior fallbacks to some 
unspecified low value (like denominator = 240) or some other vendor-defined 
behavior.The value is unspecified because vendors will likely want to adjust 
this to an optimal value (e.g. controlled via driver settings in the Control 
Panel, defined by the Monitor manufacturer, defined by the GPU vendor, etc).
CheersMatias___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Has anyone stressed radeonsi memory?

2017-11-15 Thread Matias N. Goldberg 
Hi!

> Are you using the amdgpu kernel driver from an amdgpu-pro release or
> from the upstream Linux kernel? (If you're not sure, provide the dmesg
> output and Xorg log file)

> If the latter, can you try a 4.13 or 4.14 kernel and see if that works
> better?


I'm using upstream Linux kernel (without my distro's patches), with amdgpu (not 
pro).

I could try newer kernels.

> Why, doesn't your distro have LLVM development packages?
They aren't as up to date. Keeping up-to-date with everything mesa needs is 
exhausting.
I started compiling LLVM from source when I needed to test an LLVM patch to fix 
a GLSL shader compiler bug.

I also compile mesa from source (rather thank using Oibaf or Padoka's PPA for 
Ubuntu) because as a graphics dev, being able to debug inside Mesa has proven 
to be an invaluable tool.


So I take it from your questions, that this behavior I saw isn't exactly 
"normal" or "expected" and I should dedicate some time to see if I can repro in 
newer versions, and if possible create a small repro.

Thanks,

Cheers
Matias

De: Michel Dänzer <mic...@daenzer.net>
Para: Matias N. Goldberg <dark_syl...@yahoo.com.ar> 
CC: ML Mesa-dev <mesa-dev@lists.freedesktop.org>
Enviado: Martes, 14 de noviembre, 2017 14:43:28
Asunto: Re: [Mesa-dev] Has anyone stressed radeonsi memory?



On 13/11/17 04:39 AM, Matias N. Goldberg wrote:
> 
> I am on a Radeon RX 560 2GB; using mesa git-57c8ead0cd (So... not too new not 
> too old), Kernel 4.12.10
> 
> I've been having complaints about our WIP branch of Ogre 2.2 about out of 
> memory crashes, and I fixed them.
> 
> I made a stress test where 495 textures with very different resolutions (most 
> of them not power-of-2), and total memory from those textures is around 700MB 
> (for some reason radentop reports all 2GB of my card are used during this 
> stress test).
> Additionally, 495 cubes (one cube for each texture) are rendered to screen to 
> ensure driver keeps them resident.
> 
> The problem is, we have different strategies:
> 1. In one extreme, we can load every texture to a staging region, one at a 
> time; and then from staging region copy to the final texture.
> 2. In the other extreme, we load all textures to RAM at once, and use one 
> giant staging region.
> 
> Loading everything at once causes a GL_OUT_OF_MEMORY while creating the 
> staging area of 700MB. Ok... sounds sorta reasonable.
> 
> But things get interesting when loading using a staging area of 512MB:
> 1. Loading goes fine.
> 2. For a time, everything works fine.
> 3. If I hide all cubes so that they aren't shown anymore:
> 1. Framerate usually goes way down (not always), like 8 fps or so (should 
> be at 1000 fps while empty, around 200 fps while showing the cubes).
> How slow it becomes is not consistent.2. radeontop shows consumption goes 
> down a lot (like half or more).
> 3. A few seconds later, I almost always get a crash (SIGBUS) while 
> writing to an UBO buffer that had been persistently mapped (non-coherent) 
> since the beginning of the application.
> 4. Running through valgrind, I don't get a crash.
> 5. There are no errors reported by OpenGL.
> 4. I don't get a crash if I never hide the cubes.
> 
> Using a smaller staging area (256MB or lower) everything is always fine.
> 
> So... is this behavior expected?
> Am I uncovering a weird bug in how radeonsi/amdgpu-pro handle memory pages?

Are you using the amdgpu kernel driver from an amdgpu-pro release or
from the upstream Linux kernel? (If you're not sure, provide the dmesg
output and Xorg log file)

If the latter, can you try a 4.13 or 4.14 kernel and see if that works
better?



> I'd normally update to latest git, then create a test if the problem 
> persists; but I've pulled latest git and saw that it required me to recompile 
> llvm as well...

Why, doesn't your distro have LLVM development packages?


-- 
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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] Has anyone stressed radeonsi memory?

2017-11-12 Thread Matias N. Goldberg 
Hi!

I am on a Radeon RX 560 2GB; using mesa git-57c8ead0cd (So... not too new not 
too old), Kernel 4.12.10

I've been having complaints about our WIP branch of Ogre 2.2 about out of 
memory crashes, and I fixed them.

I made a stress test where 495 textures with very different resolutions (most 
of them not power-of-2), and total memory from those textures is around 700MB 
(for some reason radentop reports all 2GB of my card are used during this 
stress test).
Additionally, 495 cubes (one cube for each texture) are rendered to screen to 
ensure driver keeps them resident.

The problem is, we have different strategies:
1. In one extreme, we can load every texture to a staging region, one at a 
time; and then from staging region copy to the final texture.
2. In the other extreme, we load all textures to RAM at once, and use one giant 
staging region.

Loading everything at once causes a GL_OUT_OF_MEMORY while creating the staging 
area of 700MB. Ok... sounds sorta reasonable.

But things get interesting when loading using a staging area of 512MB:
1. Loading goes fine.
2. For a time, everything works fine.
3. If I hide all cubes so that they aren't shown anymore:
1. Framerate usually goes way down (not always), like 8 fps or so (should 
be at 1000 fps while empty, around 200 fps while showing the cubes).
How slow it becomes is not consistent.2. radeontop shows consumption goes 
down a lot (like half or more).
3. A few seconds later, I almost always get a crash (SIGBUS) while writing 
to an UBO buffer that had been persistently mapped (non-coherent) since the 
beginning of the application.
4. Running through valgrind, I don't get a crash.
5. There are no errors reported by OpenGL.
4. I don't get a crash if I never hide the cubes.

Using a smaller staging area (256MB or lower) everything is always fine.

So... is this behavior expected?
Am I uncovering a weird bug in how radeonsi/amdgpu-pro handle memory pages?

I'd normally update to latest git, then create a test if the problem persists; 
but I've pulled latest git and saw that it required me to recompile llvm as 
well... so this is why I'm asking first, before losing any more time to this.

From my perspective, if a limit of 256MB works, then I'm happy.
If you tell me this isn't normal, then I'll try to find some time to update 
mesa to try again; and if problem persists create a small test.

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

Re: [Mesa-dev] [PATCH 5/5] radeonsi: allow out-of-order rasterization in commutative blending cases

2017-09-21 Thread Matias N. Goldberg 
I'm reviewing the patch and there's something that confuses me about 
radeonsi_assume_no_z_fights (which got implemented in a later patch). It 
appears to be that part of the logic is flawed.

Please correct me whenever you feel I misunderstood what is going on.

Assuming colour writes are enabled, si_out_of_order_rasterization will return 
true only if the following conditions are met (simplified):
* There is a zsbuf set (If I interpret it correctly, this means if there is a 
zs buffer attached)
* dsa_order_invariant.pass_set is true
* dsa_order_invariant.pass_last is true


_or_

* There is no zsbuf set
* dsa_order_invariant.pass_last is true


However, the logic is apparently contradictory, because:
* pass_set  will only be true when depth writes are disabled or depth func is 
set to always or depth func is set to never.
* pass_last will only be true when depth writes are enabled or depth func is 
not set to always nor not_equal.


!!This is impossible to satisfy unless depth function is set to never!!
Not only this is extremely rare, it appears this is not the intention behind 
the option "radeonsi_assume_no_z_fights" which I believe is an optimization for 
gamers to get a performance boost in most games where forcing this option 
doesn't matter (either because the artifacts are extremely rare or not present).

Additionally, there seems to be a bug because si_out_of_order_rasterization can 
return true if there is no zs buffer and user enabled 
radeonsi_assume_no_z_fights, which AFAIK is blatantly wrong (correct me if I'm 
wrong, but if there is no zs buffer, out of order rasterization can cause 
really wrong results).

Maybe I misunderstood what's going on, or I missed something key. But if I'm 
right then the logic needs to be revised.


It would appear to me that idea of radeonsi_assume_no_z_fights is that it 
should always enable OoO rasterization as long as depth writes are on and a 
valid zs is present (and other conditions are met such as shaders not 
requesting early depth stencil, blending operations, etc). But written as is 
right now, it will almost never be enabled even if the options is forced on.

Cheers
Matias

De: Marek Olšák 
Para: Nicolai Hähnle  
CC: "mesa-dev@lists.freedesktop.org" ; Nicolai 
Hähnle 
Enviado: Miércoles, 13 de septiembre, 2017 21:19:26
Asunto: Re: [Mesa-dev] [PATCH 5/5] radeonsi: allow out-of-order rasterization 
in commutative blending cases



For the series:

Reviewed-by: Marek Olšák 

Marek

On Sat, Sep 9, 2017 at 12:43 PM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> We do not enable this by default for additive blending, since it slightly
> breaks OpenGL invariance guarantees due to non-determinism.
>
> Still, there may be some applications can benefit from white-listing
> via the radeonsi_commutative_blend_add drirc setting without any real
> visible artifacts.
> ---
>  src/gallium/drivers/radeonsi/driinfo_radeonsi.h |  1 +
>  src/gallium/drivers/radeonsi/si_pipe.c  |  2 +
>  src/gallium/drivers/radeonsi/si_pipe.h  |  1 +
>  src/gallium/drivers/radeonsi/si_state.c | 67 
> +++--
>  src/gallium/drivers/radeonsi/si_state.h |  1 +
>  src/util/xmlpool/t_options.h|  5 ++
>  6 files changed, 73 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/driinfo_radeonsi.h 
> b/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
> index 8be85289a0c..989e5175cc0 100644
> --- a/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
> +++ b/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
> @@ -1,5 +1,6 @@
>  // DriConf options specific to radeonsi
>  DRI_CONF_SECTION_PERFORMANCE
>  DRI_CONF_RADEONSI_ENABLE_SISCHED("false")
>  DRI_CONF_RADEONSI_ASSUME_NO_Z_FIGHTS("false")
> +DRI_CONF_RADEONSI_COMMUTATIVE_BLEND_ADD("false")
>  DRI_CONF_SECTION_END
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
> b/src/gallium/drivers/radeonsi/si_pipe.c
> index b4972be739c..c44ea3be740 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -1043,20 +1043,22 @@ struct pipe_screen *radeonsi_screen_create(struct 
> radeon_winsys *ws,
> (sscreen->b.chip_class == SI &&
>  sscreen->b.info.pfp_fw_version >= 79 &&
>  sscreen->b.info.me_fw_version >= 142);
>
> sscreen->has_ds_bpermute = sscreen->b.chip_class >= VI;
> sscreen->has_out_of_order_rast = sscreen->b.chip_class >= VI &&
>  sscreen->b.info.max_se >= 2 &&
>  !(sscreen->b.debug_flags & 
> DBG_NO_OUT_OF_ORDER);
> sscreen->assume_no_z_fights =
> driQueryOptionb(config->options, 
> "radeonsi_assume_no_z_fights");
> +

[Mesa-dev] Review request: [PATCH] Fix grabbing the wrong variant if glDrawPixels is called

2017-07-07 Thread Matias N. Goldberg 
Hi!I just subscribed to this dev list.
I wrote this patch (copy at the end of this 
email)https://bugs.freedesktop.org/attachment.cgi?id=132462=edit
in order to fix bug Bug 101596 - Blender renders black UI elements 
(https://bugs.freedesktop.org/show_bug.cgi?id=101596)Note that this bug may not 
only affect Mesa.
I am asking for this patch to be reviewed for inclusion in Mesa.

Thanks
Matias

>From 3db888f8645acd5d41b689ee6522d465bcf71044 Mon Sep 17 00:00:00 2001
Message-Id: 
<3db888f8645acd5d41b689ee6522d465bcf71044.1499274200.git.dark_syl...@yahoo.com.ar>
From: "Matias N. Goldberg" <dark_syl...@yahoo.com.ar>
Date: Wed, 5 Jul 2017 14:02:50 -0300
Subject: [PATCH] Fix grabbing the wrong variant if glDrawPixels is called

By design pixel shaders can have up to 3 variants:
* The standard one.
* glDrawPixels variant.
* glBitmap variant.
However "shader_has_one_variant" ignores this fact, and therefore
st_update_fp would select the wrong variant if glDrawPixels or glBitmap
was ever called.

This patch fixes the problem. If the standard variant has been created,
calling glDrawPixels or glBitmap will append the variant to the second
entry of the linked list, so that st_update_fp still selects the right
one if shader_has_one_variant is set.

If the standard variant hasn't been created yet and glDrawPixel/Bitmap
has been called, st_update_fp will will see this and take the slow path
instead. The standard variant will then be added at the front of the
linked list, so that the next time the fast path is taken.

Blender in particular is hit by this bug.

Fixes https://bugs.freedesktop.org/show_bug.cgi?id=101596
---
 src/mesa/state_tracker/st_atom_shader.c |  4 +++-
 src/mesa/state_tracker/st_program.c | 23 ---
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/mesa/state_tracker/st_atom_shader.c 
b/src/mesa/state_tracker/st_atom_shader.c
index c1869d323b..07cf54f555 100644
--- a/src/mesa/state_tracker/st_atom_shader.c
+++ b/src/mesa/state_tracker/st_atom_shader.c
@@ -108,7 +108,9 @@ st_update_fp( struct st_context *st )
if (st->shader_has_one_variant[MESA_SHADER_FRAGMENT] &&
!stfp->ati_fs && /* ATI_fragment_shader always has multiple variants */
!stfp->Base.ExternalSamplersUsed && /* external samplers need variants 
*/
-   stfp->variants) {
+   stfp->variants &&
+   !stfp->variants->key.drawpixels &&
+   !stfp->variants->key.bitmap ) {
   shader = stfp->variants->driver_shader;
} else {
   memset(, 0, sizeof(key));
diff --git a/src/mesa/state_tracker/st_program.c 
b/src/mesa/state_tracker/st_program.c
index 6de61741dc..86faf5982d 100644
--- a/src/mesa/state_tracker/st_program.c
+++ b/src/mesa/state_tracker/st_program.c
@@ -1322,9 +1322,26 @@ st_get_fp_variant(struct st_context *st,
   /* create new */
   fpv = st_create_fp_variant(st, stfp, key);
   if (fpv) {
- /* insert into list */
- fpv->next = stfp->variants;
- stfp->variants = fpv;
+ if( key->bitmap || key->drawpixels ) {
+/* Regular variants should always come before the
+   bitmap & drawpixels variants, (unless there
+   are no regular variants) so that
+   st_update_fp can take a fast path when
+   shader_has_one_variant is set.
+*/
+/* insert into list */
+if( !stfp->variants ) {
+   fpv->next = stfp->variants;
+   stfp->variants = fpv;
+} else {
+   fpv->next = stfp->variants->next;
+   stfp->variants->next = fpv;
+}
+ } else {
+/* insert into list */
+fpv->next = stfp->variants;
+stfp->variants = fpv;
+ }
   }
}
 
-- 
2.11.0 IMPORTANT: The information contained in this email may be commercially 
sensitive and/or legally privileged. It is intended solely for the person(s) to 
whom it is addressed. If the reader of this message is not the intended 
recipient, you are on notice of its status and hereby notified that your access 
is unauthorized, and any review, dissemination, distribution, disclose or 
copying of this message including any attachments is strictly prohibited. 
Please notify the sender immediately by reply e-mail and then delete this 
message from your system.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] Review request: [PATCH] Fix grabbing the wrong variant if glDrawPixels is called

2017-07-07 Thread Matias N. Goldberg 
Hi!I just subscribed to this dev list.
I wrote this patch (copy at the end of this 
email)https://bugs.freedesktop.org/attachment.cgi?id=132462=edit
in order to fix bug Bug 101596 - Blender renders black UI elements 
(https://bugs.freedesktop.org/show_bug.cgi?id=101596)Note that this bug may not 
only affect Mesa.
I am asking for this patch to be reviewed for inclusion in Mesa.

Thanks
Matias

>From 3db888f8645acd5d41b689ee6522d465bcf71044 Mon Sep 17 00:00:00 2001
Message-Id: 
<3db888f8645acd5d41b689ee6522d465bcf71044.1499274200.git.dark_syl...@yahoo.com.ar>
From: "Matias N. Goldberg" <dark_syl...@yahoo.com.ar>
Date: Wed, 5 Jul 2017 14:02:50 -0300
Subject: [PATCH] Fix grabbing the wrong variant if glDrawPixels is called

By design pixel shaders can have up to 3 variants:
* The standard one.
* glDrawPixels variant.
* glBitmap variant.
However "shader_has_one_variant" ignores this fact, and therefore
st_update_fp would select the wrong variant if glDrawPixels or glBitmap
was ever called.

This patch fixes the problem. If the standard variant has been created,
calling glDrawPixels or glBitmap will append the variant to the second
entry of the linked list, so that st_update_fp still selects the right
one if shader_has_one_variant is set.

If the standard variant hasn't been created yet and glDrawPixel/Bitmap
has been called, st_update_fp will will see this and take the slow path
instead. The standard variant will then be added at the front of the
linked list, so that the next time the fast path is taken.

Blender in particular is hit by this bug.

Fixes https://bugs.freedesktop.org/show_bug.cgi?id=101596
---
 src/mesa/state_tracker/st_atom_shader.c |  4 +++-
 src/mesa/state_tracker/st_program.c | 23 ---
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/mesa/state_tracker/st_atom_shader.c 
b/src/mesa/state_tracker/st_atom_shader.c
index c1869d323b..07cf54f555 100644
--- a/src/mesa/state_tracker/st_atom_shader.c
+++ b/src/mesa/state_tracker/st_atom_shader.c
@@ -108,7 +108,9 @@ st_update_fp( struct st_context *st )
if (st->shader_has_one_variant[MESA_SHADER_FRAGMENT] &&
!stfp->ati_fs && /* ATI_fragment_shader always has multiple variants */
!stfp->Base.ExternalSamplersUsed && /* external samplers need variants 
*/
-   stfp->variants) {
+   stfp->variants &&
+   !stfp->variants->key.drawpixels &&
+   !stfp->variants->key.bitmap ) {
   shader = stfp->variants->driver_shader;
} else {
   memset(, 0, sizeof(key));
diff --git a/src/mesa/state_tracker/st_program.c 
b/src/mesa/state_tracker/st_program.c
index 6de61741dc..86faf5982d 100644
--- a/src/mesa/state_tracker/st_program.c
+++ b/src/mesa/state_tracker/st_program.c
@@ -1322,9 +1322,26 @@ st_get_fp_variant(struct st_context *st,
   /* create new */
   fpv = st_create_fp_variant(st, stfp, key);
   if (fpv) {
- /* insert into list */
- fpv->next = stfp->variants;
- stfp->variants = fpv;
+ if( key->bitmap || key->drawpixels ) {
+/* Regular variants should always come before the
+   bitmap & drawpixels variants, (unless there
+   are no regular variants) so that
+   st_update_fp can take a fast path when
+   shader_has_one_variant is set.
+*/
+/* insert into list */
+if( !stfp->variants ) {
+   fpv->next = stfp->variants;
+   stfp->variants = fpv;
+} else {
+   fpv->next = stfp->variants->next;
+   stfp->variants->next = fpv;
+}
+ } else {
+/* insert into list */
+fpv->next = stfp->variants;
+stfp->variants = fpv;
+ }
   }
}
 
-- 
2.11.0

 IMPORTANT: The information contained in this email may be commercially 
sensitive and/or legally privileged. It is intended solely for the person(s) to 
whom it is addressed. If the reader of this message is not the intended 
recipient, you are on notice of its status and hereby notified that your access 
is unauthorized, and any review, dissemination, distribution, disclose or 
copying of this message including any attachments is strictly prohibited. 
Please notify the sender immediately by reply e-mail and then delete this 
message from your system.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev