Re: [Mesa-dev] [PATCH 1/2] loader_dri3: Wait for pending swaps to complete before drawable_fini.

2018-05-05 Thread Roman Gilg 
On Sat, May 5, 2018 at 8:50 PM, Mario Kleiner
 wrote:
> Thanks. Can you see if you get any freezes in kwin_x11 by "violent
> alt-tabbing" with patch 1? I've seen two such freezes within 8 hours
> of normal use yesterday, each occuring when i alt-tabbed (normally)
> and the switcher side panel moved out from the left. Desktop was
> mostly blocked then, i assume it was kwin_x11 that got stuck. When
> VT-switching to the console and attaching gdb to kwin_x11 the
> backtrace showed it blocked in the loader_dri3_swapbuffer_barrier()
> from patch 1. I don't know if that was the cause of the freeze, or if
> something unrelated was going wrong and kwin just happens to block
> there when one VT switches away from the X-Server while kwin does its
> thing. Freezes happened both with compositing enabled and disabled.

Yes, I can confirm it happened to me as well now with patch 1.
Happened when holding Alt + Tab, but I had to try a few times. I
logged in directly with sddm and got the following backtrace without
VT switch by ssh session. So should be a clean one:

#0  0x7f4517e1074d in poll () at ../sysdeps/unix/syscall-template.S:84
#1  0x7f4516d32172 in poll (__timeout=-1, __nfds=1,
__fds=0x7ffed4e8ead0) at /usr/include/x86_64-linux-gnu/bits/poll2.h:46
#2  _xcb_conn_wait (c=c@entry=0xcc4be0, cond=cond@entry=0x19ef968,
vector=vector@entry=0x0, count=count@entry=0x0) at
/home/roman/dev/gfx/xcb/src/libxcb/src/xcb_conn.c:479
#3  0x7f4516d33e69 in xcb_wait_for_special_event (c=0xcc4be0,
se=0x19ef940) at /home/roman/dev/gfx/xcb/src/libxcb/src/xcb_in.c:795
#4  0x7f450d764d36 in dri3_wait_for_event_locked (draw=0x1a28288)
at ../../src/mesa/src/loader/loader_dri3_helper.c:457
#5  0x7f450d765568 in loader_dri3_wait_for_sbc
(draw=draw@entry=0x1a28288, target_sbc=47, target_sbc@entry=0,
ust=ust@entry=0x7ffed4e8eca0, msc=msc@entry=0x7ffed4e8eca8,
sbc=sbc@entry=0x7ffed4e8ecb0) at
../../src/mesa/src/loader/loader_dri3_helper.c:534
#6  0x7f450d76560b in loader_dri3_swapbuffer_barrier
(draw=0x1a28288) at
../../src/mesa/src/loader/loader_dri3_helper.c:1930
#7  loader_dri3_drawable_fini (draw=0x1a28288) at
../../src/mesa/src/loader/loader_dri3_helper.c:238
#8  0x7f450d75aaad in dri3_destroy_drawable (base=0x1a28250) at
../../src/mesa/src/glx/dri3_glx.c:349
#9  0x7f450d7537d2 in driReleaseDrawables (gc=gc@entry=0x1542920)
at ../../src/mesa/src/glx/dri_common.c:481
#10 0x7f450d75af0c in dri3_bind_context (context=0x1542920,
old=, draw=41946959, read=41946959) at
../../src/mesa/src/glx/dri3_glx.c:198
#11 0x7f450d746537 in MakeContextCurrent (dpy=0xcc3850,
draw=41946959, read=41946959, gc_user=0x1542920) at
../../src/mesa/src/glx/glxcurrent.c:213
#12 0x7f44fd3056fd in ?? () from
/usr/lib/x86_64-linux-gnu/qt5/plugins/xcbglintegrations/libqxcb-glx-integration.so
#13 0x7f45157d5229 in QOpenGLContext::makeCurrent(QSurface*) ()
from /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5
#14 0x7f451024ea5e in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Quick.so.5
#15 0x7f451024faa5 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Quick.so.5
#16 0x7f45157a5625 in QWindow::event(QEvent*) () from
/usr/lib/x86_64-linux-gnu/libQt5Gui.so.5
#17 0x7f45102c78c5 in QQuickWindow::event(QEvent*) () from
/usr/lib/x86_64-linux-gnu/libQt5Quick.so.5
#18 0x7f44d974565b in PlasmaQuick::Dialog::event(QEvent*) () from
/usr/lib/x86_64-linux-gnu/libKF5PlasmaQuick.so.5
#19 0x7f4515f40acc in QApplicationPrivate::notify_helper(QObject*,
QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#20 0x7f4515f48417 in QApplication::notify(QObject*, QEvent*) ()
from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#21 0x7f45152003c8 in QCoreApplication::notifyInternal2(QObject*,
QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#22 0x7f451579a28d in
QGuiApplicationPrivate::processExposeEvent(QWindowSystemInterfacePrivate::ExposeEvent*)
() from /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5
#23 0x7f451579aebd in
QGuiApplicationPrivate::processWindowSystemEvent(QWindowSystemInterfacePrivate::WindowSystemEvent*)
() from /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5
#24 0x7f45157748fb in
QWindowSystemInterface::sendWindowSystemEvents(QFlags)
() from /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5
#25 0x7f44fec534b6 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5XcbQpa.so.5
#26 0x7f45151fe64a in
QEventLoop::exec(QFlags) () from
/usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#27 0x7f4515207854 in QCoreApplication::exec() () from
/usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#28 0x7f45180e81f9 in kdemain () from
/usr/lib/x86_64-linux-gnu/libkdeinit5_kwin_x11.so
#29 0x7f4517d35830 in __libc_start_main (main=0x4006b0, argc=1,
argv=0x7ffed4e8f868, init=, fini=,
rtld_fini=, stack_end=0x7ffed4e8f858) at
../csu/libc-start.c:291
#30 0x004006e9 in _start ()

> So far, only using patch 2/2, i haven't seen any new freezes, but
> looking at the debug output i get a lot of these

Re: [Mesa-dev] [PATCH] intel/genxml: Fix a few invalid field widths

2018-05-05 Thread Jason Ekstrand 
Reviewed-by: Jason Ekstrand 

On Sat, May 5, 2018 at 11:39 AM, Chris Wilson 
wrote:

> A couple of typos found by inspecting field.end - field.start, revealed
> a few wide integers declared as bool and some that ended before they
> started.
>
> Cc: Lionel Landwerlin  ---
>  src/intel/genxml/gen4.xml  | 12 ++--
>  src/intel/genxml/gen45.xml | 12 ++--
>  src/intel/genxml/gen5.xml  | 12 ++--
>  src/intel/genxml/gen6.xml  |  6 +++---
>  src/intel/genxml/gen7.xml  |  6 +++---
>  src/intel/genxml/gen75.xml |  8 
>  6 files changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/src/intel/genxml/gen4.xml b/src/intel/genxml/gen4.xml
> index 6f513c5833b..cd50a1012bc 100644
> --- a/src/intel/genxml/gen4.xml
> +++ b/src/intel/genxml/gen4.xml
> @@ -961,12 +961,12 @@
>   type="bool"/>
>   type="bool"/>
>   type="bool"/>
> -
> -
> -
> -
> -
> -
> +
> +
> +
> +
> +
> +
>
>
>
> diff --git a/src/intel/genxml/gen45.xml b/src/intel/genxml/gen45.xml
> index fbd57a00c50..4d2c1534d3f 100644
> --- a/src/intel/genxml/gen45.xml
> +++ b/src/intel/genxml/gen45.xml
> @@ -994,12 +994,12 @@
>   type="bool"/>
>   type="bool"/>
>   type="bool"/>
> -
> -
> -
> -
> -
> -
> +
> +
> +
> +
> +
> +
>
>
>
> diff --git a/src/intel/genxml/gen5.xml b/src/intel/genxml/gen5.xml
> index 5c93ecdda30..5bb5a2c3312 100644
> --- a/src/intel/genxml/gen5.xml
> +++ b/src/intel/genxml/gen5.xml
> @@ -1086,12 +1086,12 @@ i
>   type="bool"/>
>   type="bool"/>
>   type="bool"/>
> -
> -
> -
> -
> -
> -
> +
> +
> +
> +
> +
> +
>
>
>
> diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml
> index 0493221bd72..f258065ebae 100644
> --- a/src/intel/genxml/gen6.xml
> +++ b/src/intel/genxml/gen6.xml
> @@ -1888,7 +1888,7 @@
>
>  
>  
> -
> +
>
>
>  
> @@ -1904,7 +1904,7 @@
>
>  
>  
> -
> +
>
>
>  
> @@ -1920,7 +1920,7 @@
>
>  
>  
> -
> +
>
>
>  
> diff --git a/src/intel/genxml/gen7.xml b/src/intel/genxml/gen7.xml
> index baf42a7d32d..895f5d232b5 100644
> --- a/src/intel/genxml/gen7.xml
> +++ b/src/intel/genxml/gen7.xml
> @@ -2537,7 +2537,7 @@
>
>  
>  
> -
> +
>
>
>  
> @@ -2553,7 +2553,7 @@
>
>  
>  
> -
> +
>
>
>  
> @@ -2569,7 +2569,7 @@
>
>  
>  
> -
> +
>
>
>  
> diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
> index 7b635b22dac..fe59446d83f 100644
> --- a/src/intel/genxml/gen75.xml
> +++ b/src/intel/genxml/gen75.xml
> @@ -3021,7 +3021,7 @@
>
>  
>  
> -
> +
>
>
>  
> @@ -3037,7 +3037,7 @@
>
>  
>  
> -
> +
>
>
>  
> @@ -3053,7 +3053,7 @@
>
>  
>  
> -
> +
>
>
>  
> @@ -3069,7 +3069,7 @@
>
>  
>  
> -
> +
>
>
>  
> --
> 2.17.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 1/2] loader_dri3: Wait for pending swaps to complete before drawable_fini.

2018-05-05 Thread Mario Kleiner 
On Sat, May 5, 2018 at 4:44 PM, Roman Gilg  wrote:
> Without this patch plasmashell on Xserver/Mesa master freezes on me
> when opening the launcher menu (Kickoff). With the patch haven't
> experienced freezes yet.
>
> Haven't tested the Steam client yet. Might be a different problem though.
>
> Tested-by: Roman Gilg 
>

Thanks. Can you see if you get any freezes in kwin_x11 by "violent
alt-tabbing" with patch 1? I've seen two such freezes within 8 hours
of normal use yesterday, each occuring when i alt-tabbed (normally)
and the switcher side panel moved out from the left. Desktop was
mostly blocked then, i assume it was kwin_x11 that got stuck. When
VT-switching to the console and attaching gdb to kwin_x11 the
backtrace showed it blocked in the loader_dri3_swapbuffer_barrier()
from patch 1. I don't know if that was the cause of the freeze, or if
something unrelated was going wrong and kwin just happens to block
there when one VT switches away from the X-Server while kwin does its
thing. Freezes happened both with compositing enabled and disabled.

So far, only using patch 2/2, i haven't seen any new freezes, but
looking at the debug output i get a lot of these when alt-tabbing:

INIT: wxh = 264 x 1062, drawable 54662577 eid 54662586
QXcbConnection: XCB error: 3 (BadWindow), sequence: 13890, resource
id: 54662577, major code: 20 (GetProperty), minor code: 0
QXcbConnection: XCB error: 3 (BadWindow), sequence: 13902, resource
id: 54662577, major code: 20 (GetProperty), minor code: 0
FINI: wxh = 264 x 1062, drawable 54662577 eid 54662586 recv_sbc 3,
send_sbc 4 PENDING 1
INIT: wxh = 264 x 1062, drawable 54662633 eid 54662644
QXcbConnection: XCB error: 3 (BadWindow), sequence: 14502, resource
id: 54662633, major code: 20 (GetProperty), minor code: 0
QXcbConnection: XCB error: 3 (BadWindow), sequence: 14514, resource
id: 54662633, major code: 20 (GetProperty), minor code: 0
FINI: wxh = 264 x 1062, drawable 54662633 eid 54662644 recv_sbc 2,
send_sbc 3 PENDING 1

Curiously my display is only 1050 pixels high, that window looks higher.

Anyway, would be good to know if patch 1/1 replaces a high frequency
of lockups of plasmashell by a low frequency of lockups of kwin_x11 if
you notice something like that.

Ideally one of the more experienced Mesa developers would find a
better solution than one of these patches, at least long-term.

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

[Mesa-dev] [PATCH] intel/genxml: Fix a few invalid field widths

2018-05-05 Thread Chris Wilson 
A couple of typos found by inspecting field.end - field.start, revealed
a few wide integers declared as bool and some that ended before they
started.

Cc: Lionel Landwerlin 
---
 src/intel/genxml/gen4.xml  | 12 ++--
 src/intel/genxml/gen45.xml | 12 ++--
 src/intel/genxml/gen5.xml  | 12 ++--
 src/intel/genxml/gen6.xml  |  6 +++---
 src/intel/genxml/gen7.xml  |  6 +++---
 src/intel/genxml/gen75.xml |  8 
 6 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/src/intel/genxml/gen4.xml b/src/intel/genxml/gen4.xml
index 6f513c5833b..cd50a1012bc 100644
--- a/src/intel/genxml/gen4.xml
+++ b/src/intel/genxml/gen4.xml
@@ -961,12 +961,12 @@
 
 
 
-
-
-
-
-
-
+
+
+
+
+
+
   
 
   
diff --git a/src/intel/genxml/gen45.xml b/src/intel/genxml/gen45.xml
index fbd57a00c50..4d2c1534d3f 100644
--- a/src/intel/genxml/gen45.xml
+++ b/src/intel/genxml/gen45.xml
@@ -994,12 +994,12 @@
 
 
 
-
-
-
-
-
-
+
+
+
+
+
+
   
 
   
diff --git a/src/intel/genxml/gen5.xml b/src/intel/genxml/gen5.xml
index 5c93ecdda30..5bb5a2c3312 100644
--- a/src/intel/genxml/gen5.xml
+++ b/src/intel/genxml/gen5.xml
@@ -1086,12 +1086,12 @@ i
 
 
 
-
-
-
-
-
-
+
+
+
+
+
+
   
 
   
diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml
index 0493221bd72..f258065ebae 100644
--- a/src/intel/genxml/gen6.xml
+++ b/src/intel/genxml/gen6.xml
@@ -1888,7 +1888,7 @@
   
 
 
-
+
   
   
 
@@ -1904,7 +1904,7 @@
   
 
 
-
+
   
   
 
@@ -1920,7 +1920,7 @@
   
 
 
-
+
   
   
 
diff --git a/src/intel/genxml/gen7.xml b/src/intel/genxml/gen7.xml
index baf42a7d32d..895f5d232b5 100644
--- a/src/intel/genxml/gen7.xml
+++ b/src/intel/genxml/gen7.xml
@@ -2537,7 +2537,7 @@
   
 
 
-
+
   
   
 
@@ -2553,7 +2553,7 @@
   
 
 
-
+
   
   
 
@@ -2569,7 +2569,7 @@
   
 
 
-
+
   
   
 
diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
index 7b635b22dac..fe59446d83f 100644
--- a/src/intel/genxml/gen75.xml
+++ b/src/intel/genxml/gen75.xml
@@ -3021,7 +3021,7 @@
   
 
 
-
+
   
   
 
@@ -3037,7 +3037,7 @@
   
 
 
-
+
   
   
 
@@ -3053,7 +3053,7 @@
   
 
 
-
+
   
   
 
@@ -3069,7 +3069,7 @@
   
 
 
-
+
   
   
 
-- 
2.17.0

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

[Mesa-dev] [PATCH] intel/genxml: Fix a few invalid field widths

2018-05-05 Thread Chris Wilson 
A couple of typos found by inspecting field.end - field.start, revealed
a few wide integers declared as bool and some that ended before they
started.

Cc: Lionel Landwerlin

[Mesa-dev] [Bug 106411] Invalid gl_InstanceID value with combination of gl_DrawIDARB under OpenGL

2018-05-05 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=106411

Bug ID: 106411
   Summary: Invalid gl_InstanceID value with combination of
gl_DrawIDARB under OpenGL
   Product: Mesa
   Version: 18.0
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: glsl-compiler
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: frust...@gmail.com
QA Contact: intel-3d-b...@lists.freedesktop.org

Created attachment 139374
  --> https://bugs.freedesktop.org/attachment.cgi?id=139374=edit
app

The value of gl_InstanceID variable is incorrect when shader has gl_DrawIDARB.
Inserting additional dependency on gl_VertexID or gl_BaseInstanceARB variable
restores gl_InstanceID value.

Uncomment 17 line inside main_vertex.glsl file to fix gl_InstanceID:
// position.x += gl_VertexID * 1e-6f;

There is no such problem under Vulkan API.

Mesa DRI Intel(R) Iris Pro Graphics 580 (Skylake GT4e) 
4.5 (Core Profile) Mesa 18.0.0-rc4

-- 
You are receiving this mail because:
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] [PATCH] wsi/x11: Don't cal pixmap_from_buffers if modifiers is not supported

2018-05-05 Thread Jason Ekstrand 
In the prime case, our modifier is always DRM_FORMAT_MOD_LINEAR and we
would end up calling dri3_pixmap_from_buffers on an X server which does
not support it.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106180
Fixes: c80c08e226 "vulkan/wsi/x11: Add support for DRI3 v1.2"
---
 src/vulkan/wsi/wsi_common_x11.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c
index 3a00cad..29d0e67 100644
--- a/src/vulkan/wsi/wsi_common_x11.c
+++ b/src/vulkan/wsi/wsi_common_x11.c
@@ -1055,10 +1055,8 @@ x11_image_init(VkDevice device_h, struct x11_swapchain 
*chain,
image->pixmap = xcb_generate_id(chain->conn);
 
 #ifdef HAVE_DRI3_MODIFIERS
-   if (image->base.drm_modifier != DRM_FORMAT_MOD_INVALID) {
-  /* If the image has a modifier, we must have DRI3 v1.2. */
-  assert(chain->has_dri3_modifiers);
-
+   if (chain->has_dri3_modifiers &&
+   image->base.drm_modifier != DRM_FORMAT_MOD_INVALID) {
   cookie =
  xcb_dri3_pixmap_from_buffers_checked(chain->conn,
   image->pixmap,
@@ -1083,6 +1081,9 @@ x11_image_init(VkDevice device_h, struct x11_swapchain 
*chain,
   /* Without passing modifiers, we can't have multi-plane RGB images. */
   assert(image->base.num_planes == 1);
 
+  assert(image->base.drm_modifier == DRM_FORMAT_MOD_LINEAR ||
+ image->base.drm_modifier == DRM_FORMAT_MOD_INVALID);
+
   cookie =
  xcb_dri3_pixmap_from_buffer_checked(chain->conn,
  image->pixmap,
-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [PATCH] wsi/x11: Don't cal pixmap_from_buffers if modifiers is not supported

2018-05-05 Thread Jason Ekstrand 
In the prime case, our modifier is always DRM_FORMAT_MOD_LINEAR and we
would end up calling dri3_pixmap_from_buffers on an X server which does
not support it.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106180
Fixes: c80c08e226 "vulkan/wsi/x11: Add support for DRI3 v1.2"
---
 src/vulkan/wsi/wsi_common_x11.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c
index 3a00cad..1c40271 100644
--- a/src/vulkan/wsi/wsi_common_x11.c
+++ b/src/vulkan/wsi/wsi_common_x11.c
@@ -1055,7 +1055,8 @@ x11_image_init(VkDevice device_h, struct x11_swapchain 
*chain,
image->pixmap = xcb_generate_id(chain->conn);
 
 #ifdef HAVE_DRI3_MODIFIERS
-   if (image->base.drm_modifier != DRM_FORMAT_MOD_INVALID) {
+   if (wsi_conn->has_dri3_modifiers &&
+   image->base.drm_modifier != DRM_FORMAT_MOD_INVALID) {
   /* If the image has a modifier, we must have DRI3 v1.2. */
   assert(chain->has_dri3_modifiers);
 
@@ -1083,6 +1084,9 @@ x11_image_init(VkDevice device_h, struct x11_swapchain 
*chain,
   /* Without passing modifiers, we can't have multi-plane RGB images. */
   assert(image->base.num_planes == 1);
 
+  assert(image->base.drm_modifier == DRM_FORMAT_MOD_LINEAR ||
+ image->base.drm_modifier == DRM_FORMAT_MOD_INVALID);
+
   cookie =
  xcb_dri3_pixmap_from_buffer_checked(chain->conn,
  image->pixmap,
-- 
2.5.0.400.gff86faf

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

Re: [Mesa-dev] [PATCH] vulkan/wsi: Only use LINEAR modifier for prime if supported.

2018-05-05 Thread Jason Ekstrand 
On Sat, May 5, 2018 at 6:34 AM, Bas Nieuwenhuizen 
wrote:

> This was setting the LINEAR modifier if neither the
> X server nor the driver supported modifiers.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106180
> Fixes: c80c08e226 "vulkan/wsi/x11: Add support for DRI3 v1.2"
> CC: 18.1 
> CC: Abel Garcia Dorta 
> CC: Daniel Stone 
> ---
>  src/vulkan/wsi/wsi_common.c | 3 ++-
>  src/vulkan/wsi/wsi_common_private.h | 1 +
>  src/vulkan/wsi/wsi_common_x11.c | 3 ++-
>  3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
> index fe262b4968d..87e508ddf85 100644
> --- a/src/vulkan/wsi/wsi_common.c
> +++ b/src/vulkan/wsi/wsi_common.c
> @@ -442,6 +442,7 @@ fail:
>  VkResult
>  wsi_create_prime_image(const struct wsi_swapchain *chain,
> const VkSwapchainCreateInfoKHR *pCreateInfo,
> +   bool use_modifier,
> struct wsi_image *image)
>  {
> const struct wsi_device *wsi = chain->wsi;
> @@ -626,7 +627,7 @@ wsi_create_prime_image(const struct wsi_swapchain
> *chain,
> if (result != VK_SUCCESS)
>goto fail;
>
> -   image->drm_modifier = DRM_FORMAT_MOD_LINEAR;
> +   image->drm_modifier = use_modifier ? DRM_FORMAT_MOD_LINEAR :
> DRM_FORMAT_MOD_INVALID;
> image->num_planes = 1;
> image->sizes[0] = linear_size;
> image->row_pitches[0] = linear_stride;
> diff --git a/src/vulkan/wsi/wsi_common_private.h
> b/src/vulkan/wsi/wsi_common_private.h
> index b608119b969..90941c8201b 100644
> --- a/src/vulkan/wsi/wsi_common_private.h
> +++ b/src/vulkan/wsi/wsi_common_private.h
> @@ -89,6 +89,7 @@ wsi_create_native_image(const struct wsi_swapchain
> *chain,
>  VkResult
>  wsi_create_prime_image(const struct wsi_swapchain *chain,
> const VkSwapchainCreateInfoKHR *pCreateInfo,
> +   bool use_modifier,
> struct wsi_image *image);
>
>  void
> diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_
> x11.c
> index 3a00caddfb9..62739b99125 100644
> --- a/src/vulkan/wsi/wsi_common_x11.c
> +++ b/src/vulkan/wsi/wsi_common_x11.c
> @@ -1043,7 +1043,8 @@ x11_image_init(VkDevice device_h, struct
> x11_swapchain *chain,
> uint32_t bpp = 32;
>
> if (chain->base.use_prime_blit) {
> -  result = wsi_create_prime_image(>base, pCreateInfo,
> >base);
> +  bool use_modifier = num_tranches > 0;
> +  result = wsi_create_prime_image(>base, pCreateInfo,
> use_modifier, >base);
>

This confused me for a bit but I think I see what's going on.  You have an
X server which doesn't know about modifiers but mesa is built with modifier
support and prime is in use.  This results in the WSI code calling
dri3_pixmap_from_buffers (plural) on an X server which doesn't support it.

I'm not sure what I think about this way of solving the problem.  In my
twisted mind, DRM_FORMAT_MOD_LINEAR should always be valid even if the
modifier is ultimately ignored.  I'm going to send a counter-patch which
solves it another way but I'm not sure if my solution is better.  Daniel,
do you have any thoughts?


> } else {
>result = wsi_create_native_image(>base, pCreateInfo,
> num_tranches, num_modifiers,
> modifiers,
> --
> 2.17.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 1/1] clover: Add explicit virtual destructors to argument and scalar_argument class

2018-05-05 Thread Jan Vesely 
On Fri, 2018-05-04 at 10:56 -0700, Francisco Jerez wrote:
> Jan Vesely  writes:
> 
> > These are needed to destroy the v vector.
> > Fixes memory leaks on kernel launch.
> > Signed-off-by: Jan Vesely 
> > ---
> >  src/gallium/state_trackers/clover/core/kernel.hpp | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/src/gallium/state_trackers/clover/core/kernel.hpp 
> > b/src/gallium/state_trackers/clover/core/kernel.hpp
> > index 4ba6ff467b..218f92cb1d 100644
> > --- a/src/gallium/state_trackers/clover/core/kernel.hpp
> > +++ b/src/gallium/state_trackers/clover/core/kernel.hpp
> > @@ -93,6 +93,7 @@ namespace clover {
> >   /// Free any resources that were allocated in bind().
> >   virtual void unbind(exec_context ) = 0;
> >  
> > + virtual ~argument() {};
> >protected:
> >   argument();
> >  
> > @@ -143,6 +144,7 @@ namespace clover {
> >class scalar_argument : public argument {
> >public:
> >   scalar_argument(size_t size);
> > + virtual ~scalar_argument() {};
> 
> This line shouldn't be necessary.  With that fixed:
> 
> Reviewed-by: Francisco Jerez 

thanks, fixed locally (also adapted the commit message) and pushed.
just fyi: this allows math_bruteforce conformance test to finish on
carrizo (1 error) instead of running out of memory half way through.

JAn

> 
> Thanks!
> 
> >  
> >   virtual void set(size_t size, const void *value);
> >   virtual void bind(exec_context ,
> > -- 
> > 2.17.0

-- 
Jan Vesely 

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 2/2] loader_dri3: Variant 2: Wait for pending swaps to complete before drawable_fini.

2018-05-05 Thread Mike Lothian 
Thank you for the clarification.

On Sat, 5 May 2018 at 14:31 Daniel Stone  wrote:

> On 5 May 2018 at 10:15, Mike Lothian  wrote:
> > Out of interest can you try running the vulkan smoketest, I'm seeing
> this:
> >
> > smoketest
> > terminate called after throwing an instance of 'std::runtime_error'
> >  what():  VkResult -101004 returned
> > Aborted (core dumped)
>
> VK_ERROR_OUT_OF_DATE_KHR is returned from vkQueuePresentKHR when the
> application _must_ recreate the swapchain it uses for rendering.
> Usually this happens due to the window resizing (which the window
> manager can do), which Vulkan apps _must_ handle themselves. The
> smoketest chooses to handle this error by crashing.
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] loader_dri3: Wait for pending swaps to complete before drawable_fini.

2018-05-05 Thread Roman Gilg 
Without this patch plasmashell on Xserver/Mesa master freezes on me
when opening the launcher menu (Kickoff). With the patch haven't
experienced freezes yet.

Haven't tested the Steam client yet. Might be a different problem though.

Tested-by: Roman Gilg 

On Fri, May 4, 2018 at 3:45 PM, Mario Kleiner
 wrote:
> Before destroying the loader_dri3_drawable, make sure all pending
> swaps for it have completed. This guides against the following scenario,
> which happens, e.g., with KDE Plasma-5's plasmashell (which uses
> QT-5's QtGui/QtQuick for rendering), when it repaints multiple
> UI elements, each represented by its own Window/GLXDrawable, using
> one common GLXContext for all GLXDrawable's:
>
> 1. glXMakeCurrent(dpy, drawable1, context);
> 2. glXXX render to drawable1
> 3. glXSwapBuffers(dpy, drawable1); #1
> 4. glXMakeCurrent(dpy, drawable2, context);
> 5. glXXX render to drawable2
> 6. glXSwapBuffers(dpy, drawable2);
> // While the swap #1 is still pending for drawable1:
> 7. glXMakeCurrent(dpy, drawable1, context);
> 8. glXXX render to drawable1
> 9. glXSwapBuffers(dpy, drawable1);
>
> Binding a different drawable2 to the same context via glXMakeCurrent
> will cause its previous drawable1 to be released (cfe. dri3_bind_context
> -> driReleaseDrawables), which in turn calls loader_dri3_drawable_fini().
> This unselects for Present notify event delivery on the associated
> X-Window and loses all dri3 related state. If drawable1 is selected for
> the context again [7], a new incarnation of loader_dri3_drawable is
> created in dri3_bind_context->driFetchDrawable->dri3_create_drawable->
> loader_dri3_drawable_init(), which again selects for Present notify
> event delivery for the underlying X-Window, but the new incarnation lost
> all state wrt. to previous rendering and swaps. The server now delivers
> PresentPixmapIdle and PresentPixmapComplete events from the completed
> previous swapbuffers call #1 [3] to the new loader_dri3_drawable, which
> doesn't expect those. One problem is that the new incarnation has a
> draw->send_sbc == 0, but now receives PresentPixmapComplete events with
> sbc's > 0, therefore updating draw->recv_sbc to > 0 in
> dri3_handle_present_event(). The draw->recv_sbc > draw_send_sbc is
> misinterpreted as sbc wraparound, triggers recv_sbc wraparound handling
> and ends up with a very large draw->recv_sbc. During the next swapbuffers
> call [9], the totally wrong recv_sbc is used for calculating the target_msc
> for the PresentPixmap request, leading to a target_msc billions of vblanks
> in the future, leading to a swap that never completes and thereby frozen UI
> and hang of the client.
>
> Make sure that a loader_dri3_drawable can only be destroyed after all
> its pending swaps have completed, to prevent misdelivery of PresentNotify
> events to the right X-Window, but the wrong incarnation of the associated
> loader_dri3_drawable.
>
> Signed-off-by: Mario Kleiner 
> Cc: xorg-de...@lists.x.org
> Cc: dan...@fooishbar.org
> Cc: eero.t.tammi...@intel.com
> Cc: m...@fireburn.co.uk
> ---
>  src/loader/loader_dri3_helper.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
> index 6bb11c4..7bd79af 100644
> --- a/src/loader/loader_dri3_helper.c
> +++ b/src/loader/loader_dri3_helper.c
> @@ -234,6 +234,9 @@ loader_dri3_drawable_fini(struct loader_dri3_drawable 
> *draw)
>  {
> int i;
>
> +   if (draw->special_event)
> +  loader_dri3_swapbuffer_barrier(draw);
> +
> draw->ext->core->destroyDrawable(draw->dri_drawable);
>
> for (i = 0; i < ARRAY_SIZE(draw->buffers); i++) {
> --
> 2.7.4
>
> ___
> 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] vulkan/wsi: Only use LINEAR modifier for prime if supported.

2018-05-05 Thread Bas Nieuwenhuizen 
This was setting the LINEAR modifier if neither the
X server nor the driver supported modifiers.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106180
Fixes: c80c08e226 "vulkan/wsi/x11: Add support for DRI3 v1.2"
CC: 18.1 
CC: Abel Garcia Dorta 
CC: Daniel Stone 
---
 src/vulkan/wsi/wsi_common.c | 3 ++-
 src/vulkan/wsi/wsi_common_private.h | 1 +
 src/vulkan/wsi/wsi_common_x11.c | 3 ++-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
index fe262b4968d..87e508ddf85 100644
--- a/src/vulkan/wsi/wsi_common.c
+++ b/src/vulkan/wsi/wsi_common.c
@@ -442,6 +442,7 @@ fail:
 VkResult
 wsi_create_prime_image(const struct wsi_swapchain *chain,
const VkSwapchainCreateInfoKHR *pCreateInfo,
+   bool use_modifier,
struct wsi_image *image)
 {
const struct wsi_device *wsi = chain->wsi;
@@ -626,7 +627,7 @@ wsi_create_prime_image(const struct wsi_swapchain *chain,
if (result != VK_SUCCESS)
   goto fail;
 
-   image->drm_modifier = DRM_FORMAT_MOD_LINEAR;
+   image->drm_modifier = use_modifier ? DRM_FORMAT_MOD_LINEAR : 
DRM_FORMAT_MOD_INVALID;
image->num_planes = 1;
image->sizes[0] = linear_size;
image->row_pitches[0] = linear_stride;
diff --git a/src/vulkan/wsi/wsi_common_private.h 
b/src/vulkan/wsi/wsi_common_private.h
index b608119b969..90941c8201b 100644
--- a/src/vulkan/wsi/wsi_common_private.h
+++ b/src/vulkan/wsi/wsi_common_private.h
@@ -89,6 +89,7 @@ wsi_create_native_image(const struct wsi_swapchain *chain,
 VkResult
 wsi_create_prime_image(const struct wsi_swapchain *chain,
const VkSwapchainCreateInfoKHR *pCreateInfo,
+   bool use_modifier,
struct wsi_image *image);
 
 void
diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c
index 3a00caddfb9..62739b99125 100644
--- a/src/vulkan/wsi/wsi_common_x11.c
+++ b/src/vulkan/wsi/wsi_common_x11.c
@@ -1043,7 +1043,8 @@ x11_image_init(VkDevice device_h, struct x11_swapchain 
*chain,
uint32_t bpp = 32;
 
if (chain->base.use_prime_blit) {
-  result = wsi_create_prime_image(>base, pCreateInfo, >base);
+  bool use_modifier = num_tranches > 0;
+  result = wsi_create_prime_image(>base, pCreateInfo, use_modifier, 
>base);
} else {
   result = wsi_create_native_image(>base, pCreateInfo,
num_tranches, num_modifiers, modifiers,
-- 
2.17.0

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

Re: [Mesa-dev] [PATCH 2/2] loader_dri3: Variant 2: Wait for pending swaps to complete before drawable_fini.

2018-05-05 Thread Daniel Stone 
On 5 May 2018 at 10:15, Mike Lothian  wrote:
> Out of interest can you try running the vulkan smoketest, I'm seeing this:
>
> smoketest
> terminate called after throwing an instance of 'std::runtime_error'
>  what():  VkResult -101004 returned
> Aborted (core dumped)

VK_ERROR_OUT_OF_DATE_KHR is returned from vkQueuePresentKHR when the
application _must_ recreate the swapchain it uses for rendering.
Usually this happens due to the window resizing (which the window
manager can do), which Vulkan apps _must_ handle themselves. The
smoketest chooses to handle this error by crashing.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] dri3: Only update number of back buffers in loader_dri3_get_buffers

2018-05-05 Thread Jason Ekstrand 
On Wed, May 2, 2018 at 2:35 AM, Michel Dänzer  wrote:

> From: Michel Dänzer 
>
> And only free no longer needed back buffers there as well.
>
> We want to stick to the same back buffer throughout a frame, otherwise
> we can run into various issues.
>
> Bugzilla: https://bugs.freedesktop.org/105906
> Fixes: 3160cb86aa92 "egl/x11: Re-allocate buffers if format is suboptimal"
> Reported-by: Sergii Romantsov 
> Tested-by: Eero Tamminen 
> Acked-by: Daniel Stone 
> Signed-off-by: Michel Dänzer 
> ---
>  src/loader/loader_dri3_helper.c | 19 +++
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_
> helper.c
> index 23729f7ecb2..6db8303d26d 100644
> --- a/src/loader/loader_dri3_helper.c
> +++ b/src/loader/loader_dri3_helper.c
> @@ -420,13 +420,6 @@ dri3_handle_present_event(struct
> loader_dri3_drawable *draw,
>
>   if (buf && buf->pixmap == ie->pixmap)
>  buf->busy = 0;
> -
> - if (buf && draw->cur_blit_source != b && !buf->busy &&
> - (buf->reallocate ||
> - (draw->num_back <= b && b < LOADER_DRI3_MAX_BACK))) {
> -dri3_free_render_buffer(draw, buf);
> -draw->buffers[b] = NULL;
> - }
>}
>break;
> }
> @@ -559,7 +552,6 @@ dri3_find_back(struct loader_dri3_drawable *draw)
> /* Check whether we need to reuse the current back buffer as new back.
>  * In that case, wait until it's not busy anymore.
>  */
> -   dri3_update_num_back(draw);
> num_to_consider = draw->num_back;
> if (!loader_dri3_have_image_blit(draw) && draw->cur_blit_source !=
> -1) {
>num_to_consider = 1;
> @@ -1815,6 +1807,7 @@ loader_dri3_get_buffers(__DRIdrawable *driDrawable,
>  {
> struct loader_dri3_drawable *draw = loaderPrivate;
> struct loader_dri3_buffer   *front, *back;
> +   int buf_id;
>
> buffers->image_mask = 0;
> buffers->front = NULL;
> @@ -1826,6 +1819,16 @@ loader_dri3_get_buffers(__DRIdrawable *driDrawable,
> if (!dri3_update_drawable(driDrawable, draw))
>return false;
>
> +   dri3_update_num_back(draw);
> +
> +   /* Free no longer needed back buffers */
> +   for (buf_id = draw->num_back; buf_id < LOADER_DRI3_MAX_BACK; buf_id++)
> {
> +  if (draw->cur_blit_source != buf_id && draw->buffers[buf_id]) {
> + dri3_free_render_buffer(draw, draw->buffers[buf_id]);
> + draw->buffers[buf_id] = NULL;
> +  }
> +   }
>

Moving this hear means that dri3_update_num_back is no longer getting
called from dri3_find_back_alloc which is used by swapbuffers_msc and
copy_sub_buffer.  Is this intended?


> +
> /* pixmaps always have front buffers.
>  * Exchange swaps also mandate fake front buffers.
>  */
> --
> 2.17.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 106180] [bisected] radv vulkan smoke test black screen (Add support for DRI3 v1.2)

2018-05-05 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=106180

--- Comment #15 from mercuriete  ---
Created attachment 139368
  --> https://bugs.freedesktop.org/attachment.cgi?id=139368=edit
Xorg log

-- 
You are receiving this mail because:
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 106180] [bisected] radv vulkan smoke test black screen (Add support for DRI3 v1.2)

2018-05-05 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=106180

--- Comment #14 from mercuriete  ---
I think i am.

I am using a laptop with prime configuration.

So I think when using plasma kde (kwin_x11 compositor) i am using X11 over
intel.

What I not sure is what DDX drivers i am using. I think i am using modesetting
for both graphic cards but I not sure.

I will attach the Xorg.log

and xrandr --list providers

xrandr --listproviders 
Providers: number : 2
Provider 0: id: 0x7a cap: 0xf, Source Output, Sink Output, Source Offload, Sink
Offload crtcs: 3 outputs: 2 associated providers: 0 name:modesetting
Provider 1: id: 0x45 cap: 0x5, Source Output, Source Offload crtcs: 6 outputs:
0 associated providers: 0 name:modesetting

-- 
You are receiving this mail because:
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 2/2] loader_dri3: Variant 2: Wait for pending swaps to complete before drawable_fini.

2018-05-05 Thread Mike Lothian 
Which is:

VK_ERROR_OUT_OF_DATE_KHR = -101004,


On Sat, 5 May 2018 at 10:15 Mike Lothian  wrote:

> Out of interest can you try running the vulkan smoketest, I'm seeing this:
>
> smoketest
> terminate called after throwing an instance of 'std::runtime_error'
>  what():  VkResult -101004 returned
> Aborted (core dumped)
>
> On Sat, 5 May 2018 at 05:25 Mario Kleiner 
> wrote:
>
>> On Sat, May 5, 2018 at 4:08 AM, Mike Lothian  wrote:
>> > I definately saw the steam bug with patch 1 but not with plasmashell,
>> > I started seeing it with patch 2 but it seemed to fix itself
>> >
>>
>> I had two hangs of kwin_x11 within the last 6 hours when alt-tabbing
>> between windows, where it got stuck in the
>> loader_dri3_swapbuffer_barrier() from patch 1/2. Not sure how that is
>> possible, or if the stacktrace was misleading, because i had to VT
>> switch to a text console to attach the debugger and this might be just
>> a side effect of that. But if it is true, then patch 1/2 would not be
>> it. Also 1/2 has a potential performance impact, whereas 2/2 doesn't.
>> However 2/2 would also need more work, as i can think of more complex
>> scenarios where it would filter the wrong events, although not in the
>> case of plasmashell or steam. Probably we'd need to sacrifice a few
>> sbc bits in the Present events serial field to transport a unique tag
>> for each incarnation of the loader_dri3_drawable, like a mini-hash of
>> the draw->eid. Ugly ugly...
>>
>> -mario
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] loader_dri3: Variant 2: Wait for pending swaps to complete before drawable_fini.

2018-05-05 Thread Mike Lothian 
Out of interest can you try running the vulkan smoketest, I'm seeing this:

smoketest
terminate called after throwing an instance of 'std::runtime_error'
 what():  VkResult -101004 returned
Aborted (core dumped)

On Sat, 5 May 2018 at 05:25 Mario Kleiner 
wrote:

> On Sat, May 5, 2018 at 4:08 AM, Mike Lothian  wrote:
> > I definately saw the steam bug with patch 1 but not with plasmashell,
> > I started seeing it with patch 2 but it seemed to fix itself
> >
>
> I had two hangs of kwin_x11 within the last 6 hours when alt-tabbing
> between windows, where it got stuck in the
> loader_dri3_swapbuffer_barrier() from patch 1/2. Not sure how that is
> possible, or if the stacktrace was misleading, because i had to VT
> switch to a text console to attach the debugger and this might be just
> a side effect of that. But if it is true, then patch 1/2 would not be
> it. Also 1/2 has a potential performance impact, whereas 2/2 doesn't.
> However 2/2 would also need more work, as i can think of more complex
> scenarios where it would filter the wrong events, although not in the
> case of plasmashell or steam. Probably we'd need to sacrifice a few
> sbc bits in the Present events serial field to transport a unique tag
> for each incarnation of the loader_dri3_drawable, like a mini-hash of
> the draw->eid. Ugly ugly...
>
> -mario
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/4] improve buffer cache and reuse

2018-05-05 Thread Chris Wilson 
Quoting James Xiong (2018-05-05 01:56:01)
> This series align the buffer size up to page instead of a bucket size
> to improve memory allocation efficiency.

It doesn't though. It still retrieves up to the bucket size, so with a
little cache poisoning (or a series of unfortunate events) it will be no
better than before.

Perhaps open with the problem statement. What is it you are trying to
fix? Would adding metrics to the buffer cache be a good start to
demonstrating what needs improving?
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3] i965: Fix ETC2/EAC GetCompressed* functions on Gen7 GPUs

2018-05-05 Thread Alejandro Piñeiro 
On 04/05/18 21:06, Eleni Maria Stea wrote:
> Hi Eero,
>
> On Fri, 4 May 2018 18:29:55 +0300
> Eero Tamminen  wrote:
>
>> You mean returning CAVEAT_SUPPORT in params for compressed formats
>> which are transparently converted to uncompressed data?
> Well, that would be the best solution I think, if it's possible to
> modify an existing query in the extension, although I am not certain
> which is the best query to modify: TEXTURE_COMPRESSED, or
> INTERNALFORMAT_SUPPORTED (or maybe both?). 

Note: by spec, INTERNALFORMAT_SUPPORTED can only return TRUE/FALSE.
Probably that is because it is intended to return TRUE as far as it is
supported on any subset of possible operations. In that sense it doesn't
mention resource/target, so it is computed without it. I guess that they
reserved CAVEAT_SUPPORT for queries that ask for a given resource on a
specific context.

> There's also another solution that we already have, but we are not sure
> if it is correct:
>
> I noticed that both mesa and nvidia drivers return GL_FALSE when the
> pname is GL_TEXTURE_COMPRESSED and the format is emulated and GL_TRUE
> for the natively supported formats. (Specifically on mesa the code that
> performs the check is in src/mesa/main/formatquery.c and tests only
> for native support). 

FWIW2: as I mentioned to you in private, right now that is cause by a
bug, and not really intended. In any case, taking into account that
NVIDIA is doing that, perhaps the intention of that query was asking
what you point, if the format is really compressed or not.


> But if you take a look at this part of the extension specification:
>
> TEXTURE_COMPRESSED: If  is a compressed format
>   that is supported for this type of resource, TRUE is returned in 
>   . If the internal format is not compressed, or the type of
>   resource is not supported, FALSE is returned.
>
> it is not very clear if we should return true or false for an
> emulated format. Maybe returning false when we provide emulation is a
> bug in both drivers, just a convenient one in this case. :-)
>
> Is there any way to clarify what should be the correct behavior?

I think that it would be faster to just create a spec issue/bug instead
of debate here what that paragraph really means.
ARB_internalformat_query2 is a really long spec, so it is normal that
some parts need clarification (we already needed to ask). I will create
an issue in short.

FWIW, imho, there is another alternative. The thing is that I still find
strange to allow a format to be used on CompressedTexImage*D, and then
return FALSE when asking TEXTURE_COMPRESSED. I think that the compromise
would be allow this query to return CAVEAT_SUPPORT, because that is
exactly the problem here. Mesa supports that format (you can use on
CompressedTexImage*D, but with a CAVEAT.

>
> Do you think that even if the current behavior of the
> TEXTURE_COMPRESSED query is correct, in which case it should keep
> returning GL_FALSE for the emulated formats, we should nevertheless
> modify something else, e.g. the INTERNALFORMAT_SUPPORTED query, to
> return CAVEAT_SUPPORT? 
>
>> That API's not available for GLES v2, where I think ETC is most widely
>> used, so it would be more of a solution for GLES v3.x applications
>> only. Sounds OK to me.

Bummer here: query2 is not available on GLES v3.x either. Right now is a
desktop only extension. The spec have several references to GLES (some
of them are in fact implemented), but we found that they were leftovers,
as initially the plan was to use the extension on both GL and GLES, when
we tried to enable that extension for OpenGL ES:

https://lists.freedesktop.org/archives/piglit/2016-May/019765.html

>>
>> Hardest part will be propagating use of this query to engines &
>> toolkits that would benefit from using it. :-)
> +1 on that :)
>
> Thanks a lot for the suggestions and the feedback,
> Eleni
>
> PS: here is some code to clarify the current situation:
>
> [1]: https://github.com/hikiko/test-compression is a test program to
> quickly check the compressed formats supported (see
> the function print_compressed_formats at the end of main.c)
>
> [2]: https://pastebin.com/Qif74fFn is the output of [1] on HSW using
> the ETC patch and on nvidia where you can see that the natively
> supported compression formats return GL_TRUE in both cards whereas the
> emulated ones return GL_FALSE in both cards
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

Re: [Mesa-dev] [PATCH 4/4] i965/drm: Purge the bucket when its cached buffer is evicted

2018-05-05 Thread Chris Wilson 
Quoting James Xiong (2018-05-05 01:56:05)
> From: "Xiong, James" 
> 
> When one of cached buffers is found to be evicted by kernel,
> most likely the buffers freed earlier than this buffer are
> gone too, go through the cached list in the bucket and purge.

The order in this list bears little relevance to the MRU lists in the
kernel. (I wish it did.) So why? The list will be processed in due
course, so why pay the cost now? Even in the worst case, they will be
cleaned up as we are in the middle of processing the lists.
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 06/10] i965: Add virtual memory allocator infrastructure to brw_bufmgr.

2018-05-05 Thread Chris Wilson 
Quoting Chris Wilson (2018-05-04 22:27:27)
> Quoting Kenneth Graunke (2018-05-04 02:12:36)
> > +   if (brw_using_softpin(bufmgr) && bo->gtt_offset == 0ull) {
> > +  bo->gtt_offset = vma_alloc(bufmgr, memzone, bo->size, 1);
> > +
> > +  if (bo->gtt_offset == 0ull)
> > + goto err_free;
> > +   }
> > +
> > bo->name = name;
> > p_atomic_set(>refcount, 1);
> > bo->reusable = true;
> > @@ -545,6 +792,9 @@ brw_bo_gem_create_from_name(struct brw_bufmgr *bufmgr,
> > bo->external = true;
> > bo->kflags = bufmgr->initial_kflags;
> >  
> > +   if (brw_using_softpin(bufmgr))
> > +  bo->gtt_offset = vma_alloc(bufmgr, BRW_MEMZONE_OTHER, bo->size, 1);
> 
> At this point, I think you want bo_using_softpoin() and pull it from the
> kflags. Not any different today, but I think more defensive, especially
> on the free paths.

In particular, err_free: is suspect in that it may try to free an
unassigned address. (Fortunately, it should be non-existent, but it
should throw a few errors!)
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 06/10] i965: Add virtual memory allocator infrastructure to brw_bufmgr.

2018-05-05 Thread Chris Wilson 
Quoting Kenneth Graunke (2018-05-04 02:12:36)
> This introduces a new fast virtual memory allocator integrated with our
> BO cache bucketing.  For larger objects, it falls back to the simple
> free-list allocator (util_vma).
> 
> This puts the allocators in place but doesn't enable softpin yet.
> ---
>  src/mesa/drivers/dri/i965/brw_bufmgr.c | 291 -
>  src/mesa/drivers/dri/i965/brw_bufmgr.h |   2 +
>  2 files changed, 292 insertions(+), 1 deletion(-)
> 
> I'm happy to write more comments here.  It's a pretty simple system, but
> not necessarily the most intuitive.  Feel free to ask questions.
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c 
> b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> index 66828f319be..07c0d2f7633 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> @@ -60,6 +60,8 @@
>  #include "util/macros.h"
>  #include "util/hash_table.h"
>  #include "util/list.h"
> +#include "util/u_dynarray.h"
> +#include "util/vma.h"
>  #include "brw_bufmgr.h"
>  #include "brw_context.h"
>  #include "string.h"
> @@ -98,9 +100,40 @@ atomic_add_unless(int *v, int add, int unless)
> return c == unless;
>  }
>  
> +/**
> + * i965 fixed-size bucketing VMA allocator.
> + *
> + * The BO cache maintains "cache buckets" for buffers of various sizes.
> + * All buffers in a given bucket are identically sized - when allocating,
> + * we always round up to the bucket size.  This means that virtually all
> + * allocations are fixed-size; only buffers which are too large to fit in
> + * a bucket can be variably-sized.
> + *
> + * We create an allocator for each bucket.  Each contains a free-list, where
> + * each node contains a  pair.  Each bit
> + * represents a bucket-sized block of memory.  (At the first level, each
> + * bit corresponds to a page.  For the second bucket, bits correspond to
> + * two pages, and so on.)  1 means a block is free, and 0 means it's in-use.
> + *
> + * This makes allocations cheap - any bit of any node will do.  We can pick
> + * the head of the list and use ffs() to find a free block.  If there are
> + * none, we allocate 64 blocks from a larger allocator - either a bigger
> + * bucketing allocator, or a fallback top-level allocator for large objects.
> + */
> +struct vma_bucket_node {
> +   uint64_t start_address;
> +   uint64_t bitmap;
> +};
> +
>  struct bo_cache_bucket {
> +   /** List of cached BOs. */
> struct list_head head;
> +
> +   /** Size of this bucket, in bytes. */
> uint64_t size;
> +
> +   /** List of vma_bucket_nodes. */
> +   struct util_dynarray vma_list[BRW_MEMZONE_COUNT];
>  };
>  
>  struct brw_bufmgr {
> @@ -116,6 +149,8 @@ struct brw_bufmgr {
> struct hash_table *name_table;
> struct hash_table *handle_table;
>  
> +   struct util_vma_heap vma_allocator[BRW_MEMZONE_COUNT];
> +
> bool has_llc:1;
> bool has_mmap_wc:1;
> bool bo_reuse:1;
> @@ -128,6 +163,10 @@ static int bo_set_tiling_internal(struct brw_bo *bo, 
> uint32_t tiling_mode,
>  
>  static void bo_free(struct brw_bo *bo);
>  
> +static uint64_t __vma_alloc(struct brw_bufmgr *bufmgr,
> +enum brw_memory_zone memzone,
> +uint64_t size, uint64_t alignment);
> +
>  static uint32_t
>  key_hash_uint(const void *key)
>  {
> @@ -222,6 +261,198 @@ bucket_for_size(struct brw_bufmgr *bufmgr, uint64_t 
> size)
>>cache_bucket[index] : NULL;
>  }
>  
> +static enum brw_memory_zone
> +memzone_for_address(uint64_t address)
> +{
> +   const uint64_t _4GB = 1ull << 32;
> +
> +   if (address >= _4GB)
> +  return BRW_MEMZONE_OTHER;
> +
> +   return BRW_MEMZONE_LOW_4G;
> +}
> +
> +static uint64_t
> +bucket_vma_alloc(struct brw_bufmgr *bufmgr,
> + struct bo_cache_bucket *bucket,
> + enum brw_memory_zone memzone)
> +{
> +   struct util_dynarray *vma_list = >vma_list[memzone];
> +   struct vma_bucket_node *node;
> +
> +   if (vma_list->size == 0) {
> +  /* This bucket allocator is out of space - allocate a new block of
> +   * memory for 64 blocks from a larger allocator (either a larger
> +   * bucket or util_vma).
> +   *
> +   * We align the address to the node size (64 blocks) so that
> +   * bucket_vma_free can easily compute the starting address of this
> +   * block by rounding any address we return down to the node size.
> +   *
> +   * Set the first bit used, and return the start address.
> +   */
> +  uint64_t node_size = 64ull * bucket->size;
> +  node = util_dynarray_grow(vma_list, sizeof(struct vma_bucket_node));

if (!node)
return 0;

> +  node->start_address = __vma_alloc(bufmgr, memzone, node_size, 
> node_size);
> +  node->bitmap = ~1ull;
> +  return node->start_address;
> +   }
> +
> +   /* Pick any bit from any node - they're all the right size and free. */
> +   node = util_dynarray_top_ptr(vma_list, struct vma_bucket_node);
> +   int