[Mesa-dev] [PATCH] i965: Advertise 8 bits subpixel precision for viewport bounds on gen6+
We use floating-points for viewport bounds so VIEWPORT_SUBPIXEL_BITS
should reflect this.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105975
Signed-off-by: Danylo Piliaiev
---
src/mesa/drivers/dri/i965/brw_context.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.c
b/src/mesa/drivers/dri/i965/brw_context.c
index 9ced230..eacf326 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -688,7 +688,7 @@ brw_initialize_context_constants(struct brw_context *brw)
/* ARB_viewport_array, OES_viewport_array */
if (devinfo->gen >= 6) {
ctx->Const.MaxViewports = GEN6_NUM_VIEWPORTS;
- ctx->Const.ViewportSubpixelBits = 0;
+ ctx->Const.ViewportSubpixelBits = 8;
/* Cast to float before negating because MaxViewportWidth is unsigned.
*/
--
2.7.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Fix fractional viewport coordinates handling on gen8+
On gen8+ fractional translation of viewport moved it by a whole integer.
The behaviour in this case isn't defined anywhere but some apps like
Wine may depend on viewport's width and height being ceiled.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106526
Signed-off-by: Danylo Piliaiev
---
src/mesa/drivers/dri/i965/genX_state_upload.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
b/src/mesa/drivers/dri/i965/genX_state_upload.c
index 88fde9d..886290f 100644
--- a/src/mesa/drivers/dri/i965/genX_state_upload.c
+++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
@@ -2586,12 +2586,11 @@ genX(upload_sf_clip_viewport)(struct brw_context *brw)
* pipeline stall so we're better off just being a little more clever
* with our viewport so we can emit it once at context creation time.
*/
- const float viewport_Xmin = MAX2(ctx->ViewportArray[i].X, 0);
- const float viewport_Ymin = MAX2(ctx->ViewportArray[i].Y, 0);
- const float viewport_Xmax =
- MIN2(ctx->ViewportArray[i].X + ctx->ViewportArray[i].Width, fb_width);
- const float viewport_Ymax =
- MIN2(ctx->ViewportArray[i].Y + ctx->ViewportArray[i].Height,
fb_height);
+ const struct gl_viewport_attrib *vp = >ViewportArray[i];
+ const float viewport_Xmin = MAX2(vp->X, 0);
+ const float viewport_Ymin = MAX2(vp->Y, 0);
+ const float viewport_Xmax = MIN2(ceilf(vp->X + vp->Width), fb_width);
+ const float viewport_Ymax = MIN2(ceilf(vp->Y + vp->Height), fb_height);
if (render_to_fbo) {
sfv.XMinViewPort = viewport_Xmin;
--
2.7.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa 2/4] vulkan: add VK_EXT_display_control [v8]
Thanks, then should this dependency be expressed in autoconf and meson? - Danil On 25.06.18 17:32, Keith Packard wrote: Danylo Piliaiev writes: Hello Keith, I am not able to build Mesa after this patch: wsi/wsi_common_display.c:991:4: error: unknown field ‘sequence_handler’ specified in initializer .sequence_handler = wsi_display_sequence_handler, Sounds like you need a newer libdrm that includes DRM_EVENT_CONTEXT_VERSION 4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa 2/4] vulkan: add VK_EXT_display_control [v8]
Hello Keith,
I am not able to build Mesa after this patch:
wsi/wsi_common_display.c:991:4: error: unknown field ‘sequence_handler’
specified in initializer
.sequence_handler = wsi_display_sequence_handler,
^
wsi/wsi_common_display.c:991:24: warning: initialization from
incompatible pointer type [-Wincompatible-pointer-types]
.sequence_handler = wsi_display_sequence_handler,
^
wsi/wsi_common_display.c:991:24: note: (near initialization for
‘event_context.page_flip_handler’)
wsi/wsi_common_display.c: In function ‘wsi_register_vblank_event’:
wsi/wsi_common_display.c:1373:17: error: implicit declaration of
function ‘drmCrtcQueueSequence’ [-Werror=implicit-function-declaration]
int ret = drmCrtcQueueSequence(wsi->fd, connector->crtc_id,
^
wsi/wsi_common_display.c: In function ‘wsi_acquire_xlib_display’:
wsi/wsi_common_display.c:2164:4: error: unknown type name
‘xcb_randr_lease_t’
xcb_randr_lease_t lease = xcb_generate_id(connection);
^
wsi/wsi_common_display.c:2165:4: error: unknown type name
‘xcb_randr_create_lease_cookie_t’
xcb_randr_create_lease_cookie_t cl_c =
^
wsi/wsi_common_display.c:2166:7: error: implicit declaration of function
‘xcb_randr_create_lease’ [-Werror=implicit-function-declaration]
xcb_randr_create_lease(connection, root, lease, 1, 1,
^
wsi/wsi_common_display.c:2168:4: error: unknown type name
‘xcb_randr_create_lease_reply_t’
xcb_randr_create_lease_reply_t *cl_r =
^
wsi/wsi_common_display.c:2169:7: error: implicit declaration of function
‘xcb_randr_create_lease_reply’ [-Werror=implicit-function-declaration]
xcb_randr_create_lease_reply(connection, cl_c, NULL);
^
wsi/wsi_common_display.c:2169:7: warning: initialization makes pointer
from integer without a cast [-Wint-conversion]
wsi/wsi_common_display.c:2174:12: error: request for member ‘nfd’ in
something not a structure or union
if (cl_r->nfd > 0) {
^
wsi/wsi_common_display.c:2175:20: error: implicit declaration of
function ‘xcb_randr_create_lease_reply_fds’
[-Werror=implicit-function-declaration]
int *rcl_f = xcb_randr_create_lease_reply_fds(connection, cl_r);
^
wsi/wsi_common_display.c:2175:20: warning: initialization makes pointer
from integer without a cast [-Wint-conversion]
wsi/wsi_common_display.c: In function ‘wsi_get_swapchain_counter’:
wsi/wsi_common_display.c:2313:14: error: implicit declaration of
function ‘drmCrtcGetSequence’ [-Werror=implicit-function-declaration]
int ret = drmCrtcGetSequence(wsi->fd, connector->crtc_id, value, NULL);
And before the patch Mesa compilation was also broken due to the
xcb-randr dependency, please see bug 106976
.
Without vulkan support Mesa compiles fine at the moment. I'm using
Ubuntu 16.04 and my libs are seem older than expected. If it's ok that
vulkan shouldn't compile with old libs then it's probably should be
reflected in autoconf/meson.
- Danil
On 21.06.18 01:52, Keith Packard wrote:
This extension provides fences and frame count information to direct
display contexts. It uses new kernel ioctls to provide 64-bits of
vblank sequence and nanosecond resolution.
v2: Remove DRM_CRTC_SEQUENCE_FIRST_PIXEL_OUT flag. This has
been removed from the proposed kernel API.
Add NULL parameter to drmCrtcQueueSequence ioctl as we
don't care what sequence the event was actually queued to.
v3: Adapt to pthread clock switch to MONOTONIC
v4: Fix scope for wsi_display_mode andwsi_display_connector allocs
Suggested-by: Jason Ekstrand
v5: Adopt Jason Ekstrand's coding conventions
Declare variables at first use, eliminate extra whitespace between
types and names. Wrap lines to 80 columns.
Use wsi_rel_to_abs_time helper function to convert relative
timeouts to absolute timeouts without causing overflow.
Suggested-by: Jason Ekstrand
v6:
Change WSI fence wait function to return VkResult instead of
bool. This makes the meaning of the return value easier to
understand, and allows for the indication of failure.
Also change the WSI fence wait function to take only absolute
timeouts and not provide an option for a relative timeout. No
users wanted relative timeouts, and it's simpler if that option
isn't available.
Terminate the DPMS property loop once we've found the property.
Assert that the fence hasn't already been destroyed in
wsi_display_fence_destroy.
Rearrange the event handler function order in the file to place
routines in an easier to find order.
Suggested-by: Jason Ekstrand
v7:
Adapt to API changes for surface_get_capabilities
v8:
Use wsi->alloc in register_display_event so that callers
don't have to dig out an allocator for us.
Signed-off-by: Keith Packard
---
src/vulkan/wsi/wsi_common.h | 10 +
[Mesa-dev] [PATCH] i965: Fix allow_higher_compat_version workaround limited by OpenGL 3.0
The workaround was designed to allow setting of 3.1+ compatibility
profiles but checks if current api is not API_OPENGL_COMPAT were
introduced later in order to enable certain extensions that were enabled
unconditionally before. These extensions are required for profiles higher
than 3.0 which caused the workaround to become useless.
This commit allows compatibility profiles as high as 4.0 thus fixes
Worms W.M.D, Crookz - The Big Heist and Tropico 5.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107052
Signed-off-by: Danylo Piliaiev
---
src/mesa/drivers/dri/i965/intel_extensions.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c
b/src/mesa/drivers/dri/i965/intel_extensions.c
index f837356..c56e70d 100644
--- a/src/mesa/drivers/dri/i965/intel_extensions.c
+++ b/src/mesa/drivers/dri/i965/intel_extensions.c
@@ -185,7 +185,7 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.ARB_pipeline_statistics_query = true;
ctx->Extensions.ARB_sample_shading = true;
ctx->Extensions.ARB_shading_language_420pack = true;
- if (ctx->API != API_OPENGL_COMPAT) {
+ if (ctx->API != API_OPENGL_COMPAT ||
ctx->Const.AllowHigherCompatVersion) {
ctx->Extensions.ARB_texture_buffer_object = true;
ctx->Extensions.ARB_texture_buffer_object_rgb32 = true;
ctx->Extensions.ARB_texture_buffer_range = true;
@@ -224,7 +224,7 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.ARB_conservative_depth = true;
ctx->Extensions.ARB_derivative_control = true;
ctx->Extensions.ARB_framebuffer_no_attachments = true;
- if (ctx->API != API_OPENGL_COMPAT) {
+ if (ctx->API != API_OPENGL_COMPAT ||
ctx->Const.AllowHigherCompatVersion) {
ctx->Extensions.ARB_gpu_shader5 = true;
ctx->Extensions.ARB_gpu_shader_fp64 = devinfo->has_64bit_types;
}
@@ -235,7 +235,7 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.ARB_shader_image_size = true;
ctx->Extensions.ARB_shader_precision = true;
ctx->Extensions.ARB_shader_texture_image_samples = true;
- if (ctx->API != API_OPENGL_COMPAT)
+ if (ctx->API != API_OPENGL_COMPAT || ctx->Const.AllowHigherCompatVersion)
ctx->Extensions.ARB_tessellation_shader = true;
ctx->Extensions.ARB_texture_compression_bptc = true;
ctx->Extensions.ARB_texture_view = true;
--
2.7.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa 2/4] vulkan: add VK_EXT_display_control [v8]
Also to fix a part of the issue in autoconf (with xcb-randr) I sent the
patch <https://patchwork.freedesktop.org/patch/230590/> previously. I
believe it won't merge cleanly now, should I remake it?
And I noticed that in meson build xcb-randr dependency is set to >= 1.12 :
dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.12')
When I checked it appeared only version 1.13 has required functionality.
On 25.06.18 18:13, Keith Packard wrote:
Danylo Piliaiev writes:
Thanks, then should this dependency be expressed in autoconf and
meson?
Yup; looks like we missed a step.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Disable dual source blending when shader doesn't support it on gen8+
Dual source blending behaviour is undefined when shader doesn't have second color output, dismissing fragment in such situation leads to a hang on gen8+ if depth test in enabled. Since blending cannot be gracefully fixed in such case and the result is undefined - blending is simply disabled. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107088 Signed-off-by: Danylo Piliaiev --- src/mesa/drivers/dri/i965/genX_state_upload.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index b279f01e..3d717bb 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -4791,7 +4791,24 @@ genX(upload_ps_blend)(struct brw_context *brw) dstA = fix_dual_blend_alpha_to_one(dstA); } - pb.ColorBufferBlendEnable = true; + const struct brw_wm_prog_data *wm_prog_data = +brw_wm_prog_data(brw->wm.base.prog_data); + + /* The Dual Source Blending documentation says: + * + * "If SRC1 is included in a src/dst blend factor and + * a DualSource RT Write message is not used, results + * are UNDEFINED. (This reflects the same restriction in DX APIs, + * where undefined results are produced if “o1” is not written + * by a PS – there are no default values defined). + * If SRC1 is not included in a src/dst blend factor, + * dual source blending must be disabled." + * + * There is no way to gracefully fix this undefined situation + * so we just disable the blending to prevent possible issues. + */ + pb.ColorBufferBlendEnable = +!color->Blend[0]._UsesDualSrc || wm_prog_data->dual_src_blend; pb.SourceAlphaBlendFactor = brw_translate_blend_factor(srcA); pb.DestinationAlphaBlendFactor = brw_translate_blend_factor(dstA); pb.SourceBlendFactor = brw_translate_blend_factor(srcRGB); -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: Return number of result bits for GL_ANY_SAMPLES_PASSED_CONSERVATIVE
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106986 Signed-off-by: Danylo Piliaiev --- src/mesa/main/queryobj.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c index 7ee820a..7547fa1 100644 --- a/src/mesa/main/queryobj.c +++ b/src/mesa/main/queryobj.c @@ -698,6 +698,7 @@ _mesa_GetQueryIndexediv(GLenum target, GLuint index, GLenum pname, *params = ctx->Const.QueryCounterBits.SamplesPassed; break; case GL_ANY_SAMPLES_PASSED: + case GL_ANY_SAMPLES_PASSED_CONSERVATIVE: /* The minimum value of this is 1 if it's nonzero, and the value * is only ever GL_TRUE or GL_FALSE, so no sense in reporting more * bits. -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] configure.ac: Check for xcb-randr version and xrandr to enable leasing
VK_USE_PLATFORM_XLIB_XRANDR_EXT requires xlib leasing which requires xcb-randr >= 1.13. Also xrandr header is required for this extension. The extension should not be automatically enabled if these dependencies aren't satisfied. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106976 Signed-off-by: Danylo Piliaiev --- configure.ac | 23 ++- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/configure.ac b/configure.ac index 0652410..ab59e06 100644 --- a/configure.ac +++ b/configure.ac @@ -97,6 +97,7 @@ XCBDRI2_REQUIRED=1.8 XCBDRI3_MODIFIERS_REQUIRED=1.13 XCBGLX_REQUIRED=1.8.1 XCBPRESENT_MODIFIERS_REQUIRED=1.13 +XCBRANDR_XLEASE_REQUIRED=1.13 XDAMAGE_REQUIRED=1.1 XSHMFENCE_REQUIRED=1.1 XVMC_REQUIRED=1.0.6 @@ -1867,18 +1868,6 @@ if test x"$enable_dri3" = xyes; then fi fi - -if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then -have_xlease=yes -else -have_xlease=no -fi - -if test x"$have_xlease" = xyes; then -randr_modules="x11-xcb xcb-randr" -PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules]) -fi - AM_CONDITIONAL(HAVE_PLATFORM_X11, echo "$platforms" | grep -q 'x11') AM_CONDITIONAL(HAVE_PLATFORM_WAYLAND, echo "$platforms" | grep -q 'wayland') AM_CONDITIONAL(HAVE_PLATFORM_DRM, echo "$platforms" | grep -q 'drm') @@ -1896,12 +1885,20 @@ xno) ;; *) if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then -enable_xlib_lease=yes +xlease_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED xrandr" +PKG_CHECK_EXISTS([$xlease_modules], [enable_xlib_lease=yes], [enable_xlib_lease=no]) else enable_xlib_lease=no fi esac +if test x"$enable_xlib_lease" = xyes; then +randr_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED" +PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules]) +xlib_randr_modules="xrandr" +PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules]) +fi + AM_CONDITIONAL(HAVE_XLIB_LEASE, test "x$enable_xlib_lease" = xyes) dnl -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glx: Add checks for context being direct before calling GetGLXDRIDrawable
If indirect context is explicitly created by application and not
with LIBGL_ALWAYS_INDIRECT=1 then dri may be initialized in
__glXInitialize which allows Mesa to take invalid code paths
due to GetGLXDRIDrawable returning non-null value.
In most such places there are already checks for context being
direct before calling GetGLXDRIDrawable. This patch adds checks
where they were missed.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42699
Signed-off-by: Danylo Piliaiev
---
src/glx/glx_pbuffer.c | 12 ++--
src/glx/glxcmds.c | 3 +++
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/glx/glx_pbuffer.c b/src/glx/glx_pbuffer.c
index fd3327f120..6ee2ed58d2 100644
--- a/src/glx/glx_pbuffer.c
+++ b/src/glx/glx_pbuffer.c
@@ -133,6 +133,10 @@ ChangeDrawableAttribute(Display * dpy, GLXDrawable
drawable,
SyncHandle();
#ifdef GLX_DIRECT_RENDERING
+ struct glx_context *gc = __glXGetCurrentContext();
+ if (!gc->isDirect)
+ return;
+
pdraw = GetGLXDRIDrawable(dpy, drawable);
if (!pdraw)
@@ -286,7 +290,7 @@ __glXGetDrawableAttribute(Display * dpy, GLXDrawable
drawable,
GLboolean use_glx_1_3;
#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
- __GLXDRIdrawable *pdraw;
+ __GLXDRIdrawable *pdraw = NULL;
#endif
if (dpy == NULL)
@@ -316,7 +320,11 @@ __glXGetDrawableAttribute(Display * dpy, GLXDrawable
drawable,
return 0;
#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
- pdraw = GetGLXDRIDrawable(dpy, drawable);
+ struct glx_context *gc = __glXGetCurrentContext();
+
+ if (gc->isDirect) {
+ pdraw = GetGLXDRIDrawable(dpy, drawable);
+ }
if (attribute == GLX_BACK_BUFFER_AGE_EXT) {
struct glx_context *gc = __glXGetCurrentContext();
diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
index 4db0228eab..3eb86b02a9 100644
--- a/src/glx/glxcmds.c
+++ b/src/glx/glxcmds.c
@@ -799,6 +799,8 @@ glXDestroyGLXPixmap(Display * dpy, GLXPixmap glxpixmap)
DestroyGLXDrawable(dpy, glxpixmap);
#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
+ struct glx_context *gc = __glXGetCurrentContext();
+ if (gc->isDirect)
{
struct glx_display *const priv = __glXInitialize(dpy);
__GLXDRIdrawable *pdraw = GetGLXDRIDrawable(dpy, glxpixmap);
@@ -831,6 +833,7 @@ glXSwapBuffers(Display * dpy, GLXDrawable drawable)
gc = __glXGetCurrentContext();
#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
+ if (gc->isDirect)
{
__GLXDRIdrawable *pdraw = GetGLXDRIDrawable(dpy, drawable);
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv: Disable dual source blending when shader doesn't support it on gen8+
bump. I can add test to Crucible if it will help.
On 20.07.18 12:54, Danylo Piliaiev wrote:
Dual source blending behaviour is undefined when shader doesn't
have second color output.
"If SRC1 is included in a src/dst blend factor and
a DualSource RT Write message is not used, results
are UNDEFINED. (This reflects the same restriction in DX APIs,
where undefined results are produced if “o1” is not written
by a PS – there are no default values defined)."
Dismissing fragment in such situation leads to a hang on gen8+
if depth test in enabled.
Since blending cannot be gracefully fixed in such case and the result
is undefined - blending is simply disabled.
Signed-off-by: Danylo Piliaiev
---
Also there is a similar patch which fixes the same issue
for OpenGL https://patchwork.freedesktop.org/patch/235939
src/intel/vulkan/genX_pipeline.c | 43
1 file changed, 32 insertions(+), 11 deletions(-)
diff --git a/src/intel/vulkan/genX_pipeline.c b/src/intel/vulkan/genX_pipeline.c
index 0821d71c9f..d7c532a22d 100644
--- a/src/intel/vulkan/genX_pipeline.c
+++ b/src/intel/vulkan/genX_pipeline.c
@@ -854,6 +854,15 @@ emit_ds_state(struct anv_pipeline *pipeline,
#endif
}
+MAYBE_UNUSED static bool
+is_dual_src_blend_factor(VkBlendFactor factor)
+{
+ return factor == VK_BLEND_FACTOR_SRC1_COLOR ||
+ factor == VK_BLEND_FACTOR_ONE_MINUS_SRC1_COLOR ||
+ factor == VK_BLEND_FACTOR_SRC1_ALPHA ||
+ factor == VK_BLEND_FACTOR_ONE_MINUS_SRC1_ALPHA;
+}
+
static void
emit_cb_state(struct anv_pipeline *pipeline,
const VkPipelineColorBlendStateCreateInfo *info,
@@ -886,6 +895,7 @@ emit_cb_state(struct anv_pipeline *pipeline,
state_pos += GENX(BLEND_STATE_length);
#if GEN_GEN >= 8
struct GENX(BLEND_STATE_ENTRY) bs0 = { 0 };
+ bool force_disable_color_blend = false;
#endif
for (unsigned i = 0; i < surface_count; i++) {
struct anv_pipeline_binding *binding = >surface_to_descriptor[i];
@@ -969,8 +979,27 @@ emit_cb_state(struct anv_pipeline *pipeline,
GENX(BLEND_STATE_ENTRY_pack)(NULL, state_pos, );
state_pos += GENX(BLEND_STATE_ENTRY_length);
#if GEN_GEN >= 8
- if (i == 0)
+ if (i == 0) {
bs0 = entry;
+
+ /* The Dual Source Blending documentation says:
+ *
+ * "If SRC1 is included in a src/dst blend factor and
+ * a DualSource RT Write message is not used, results
+ * are UNDEFINED. (This reflects the same restriction in DX APIs,
+ * where undefined results are produced if “o1” is not written
+ * by a PS – there are no default values defined)."
+ *
+ * There is no way to gracefully fix this undefined situation
+ * so we just disable the blending to prevent possible issues.
+ */
+ const struct brw_wm_prog_data *wm_prog_data =
get_wm_prog_data(pipeline);
+ force_disable_color_blend = !wm_prog_data->dual_src_blend &&
+(is_dual_src_blend_factor(a->srcColorBlendFactor) ||
+is_dual_src_blend_factor(a->dstColorBlendFactor) ||
+is_dual_src_blend_factor(a->srcAlphaBlendFactor) ||
+is_dual_src_blend_factor(a->dstAlphaBlendFactor));
+ }
#endif
}
@@ -978,7 +1007,8 @@ emit_cb_state(struct anv_pipeline *pipeline,
anv_batch_emit(>batch, GENX(3DSTATE_PS_BLEND), blend) {
blend.AlphaToCoverageEnable = blend_state.AlphaToCoverageEnable;
blend.HasWriteableRT= has_writeable_rt;
- blend.ColorBufferBlendEnable= bs0.ColorBufferBlendEnable;
+ blend.ColorBufferBlendEnable=
+ bs0.ColorBufferBlendEnable && !force_disable_color_blend;
blend.SourceAlphaBlendFactor= bs0.SourceAlphaBlendFactor;
blend.DestinationAlphaBlendFactor = bs0.DestinationAlphaBlendFactor;
blend.SourceBlendFactor = bs0.SourceBlendFactor;
@@ -1430,15 +1460,6 @@ emit_3dstate_wm(struct anv_pipeline *pipeline, struct
anv_subpass *subpass,
}
}
-UNUSED static bool
-is_dual_src_blend_factor(VkBlendFactor factor)
-{
- return factor == VK_BLEND_FACTOR_SRC1_COLOR ||
- factor == VK_BLEND_FACTOR_ONE_MINUS_SRC1_COLOR ||
- factor == VK_BLEND_FACTOR_SRC1_ALPHA ||
- factor == VK_BLEND_FACTOR_ONE_MINUS_SRC1_ALPHA;
-}
-
static void
emit_3dstate_ps(struct anv_pipeline *pipeline,
const VkPipelineColorBlendStateCreateInfo *blend,
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glx: Add checks for context being direct before calling GetGLXDRIDrawable
On 01.08.18 19:42, Adam Jackson wrote:
On Wed, 2018-08-01 at 17:15 +0300, Danylo Piliaiev wrote:
If indirect context is explicitly created by application and not
with LIBGL_ALWAYS_INDIRECT=1 then dri may be initialized in
__glXInitialize which allows Mesa to take invalid code paths
due to GetGLXDRIDrawable returning non-null value.
This doesn't make any sense. Initializing DRI doesn't add any GLX
drawables to the hash, unless something deeply insane is going on.
Ok, I see, since my patch seems to be inherently wrong I'll better
explain what I have found and ask for advice:
The initial issue is the crash in glXSwapBuffers with indirect context,
there GetGLXDRIDrawable returns
a non-null drawable then driScreen->swapBuffers is called then deep down
draw->vtable->get_dri_context(draw)
is called which a pointer to glx_dri3_get_dri_context and does such cast:
struct dri3_context *pcp = (struct dri3_context *) gc;
But the context isn't dri3_context.
So why GetGLXDRIDrawable return something that is not null?
When we call glXCreateWindow:
#0 CreateDRIDrawable () at glx_pbuffer.c:209
#1 CreateDrawable () at glx_pbuffer.c:492
#2 glXCreateWindow () at glx_pbuffer.c:967
And since driScreen was created before - driScreen->createDrawable is
called and succeeds. Now we have dri drawable in hashmap.
So later GetGLXDRIDrawable would return valid drawable.
The creation of drawable happens before we know that context will be
indirect. Should it be deleted when indirect context is created?
Or something else? I don't know.
In most such places there are already checks for context being
direct before calling GetGLXDRIDrawable. This patch adds checks
where they were missed.
These checks are pretty much all wrong. Whether a drawable has direct
context state is orthogonal to whether the current context is direct,
you could have two contexts with different directness.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42699
Signed-off-by: Danylo Piliaiev
---
src/glx/glx_pbuffer.c | 12 ++--
src/glx/glxcmds.c | 3 +++
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/glx/glx_pbuffer.c b/src/glx/glx_pbuffer.c
index fd3327f120..6ee2ed58d2 100644
--- a/src/glx/glx_pbuffer.c
+++ b/src/glx/glx_pbuffer.c
@@ -133,6 +133,10 @@ ChangeDrawableAttribute(Display * dpy, GLXDrawable
drawable,
SyncHandle();
#ifdef GLX_DIRECT_RENDERING
+ struct glx_context *gc = __glXGetCurrentContext();
+ if (!gc->isDirect)
+ return;
+
This is saying "if the current context is direct, then refuse to tell
the server that we want to change this drawable", which can't be right.
@@ -316,7 +320,11 @@ __glXGetDrawableAttribute(Display * dpy, GLXDrawable
drawable,
return 0;
#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
- pdraw = GetGLXDRIDrawable(dpy, drawable);
+ struct glx_context *gc = __glXGetCurrentContext();
+
+ if (gc->isDirect) {
+ pdraw = GetGLXDRIDrawable(dpy, drawable);
+ }
Again, whether the drawable has DRI state attached to it is not a
property of the current context.
I don't see how the existing code would be wrong to begin with. Even if
__glXInitialize does set up DRI state on the screen, it's not going to
add any drawables to the hash table, so this should still hit the pdraw
== NULL case below it safely.
if (attribute == GLX_BACK_BUFFER_AGE_EXT) {
struct glx_context *gc = __glXGetCurrentContext();
diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
index 4db0228eab..3eb86b02a9 100644
--- a/src/glx/glxcmds.c
+++ b/src/glx/glxcmds.c
@@ -799,6 +799,8 @@ glXDestroyGLXPixmap(Display * dpy, GLXPixmap glxpixmap)
DestroyGLXDrawable(dpy, glxpixmap);
#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
+ struct glx_context *gc = __glXGetCurrentContext();
+ if (gc->isDirect)
{
struct glx_display *const priv = __glXInitialize(dpy);
__GLXDRIdrawable *pdraw = GetGLXDRIDrawable(dpy, glxpixmap);
I believe this would introduce a leak case. If your current context is
indirect, and you glXDestroyGLXPixmap() on a pixmap with DRI state,
you'll never free that DRI state.
@@ -831,6 +833,7 @@ glXSwapBuffers(Display * dpy, GLXDrawable drawable)
gc = __glXGetCurrentContext();
#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
+ if (gc->isDirect)
{
__GLXDRIdrawable *pdraw = GetGLXDRIDrawable(dpy, drawable);
The real problem here, I think, is that we need to store what kind of
DRI a given DRI drawable is, and look up its SwapBuffer method based on
that. Instead we just have one method for the whole screen, so if the
screen happens to support DRI3 and DRISW...
Maybe it's another issue. In this case pdraw is dri3 drawable so the
correct SwapBuffer method
is called.
- ajax
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Advertise 8 bits subpixel precision for viewport bounds on gen6+
Hi,
Since the exact value is not important for users, Roland offered
compelling explanation
and the value is also 8 on Windows - could the patch be reviewed/pushed?
- Danil
On 06/19/2018 03:44 PM, Roland Scheidegger wrote:
My guess would be 8 because that's what the rasterization subpixel
precision is, so precision beyond that doesn't really do much, even if
this actually is a float.
Plus, with maximum sized fb (16kx16k dimension) you don't actually
really get a lot more than 8 fixed points bits anyway (near those 16k).
So imho 8 makes most sense.
Roland
Am 19.06.2018 um 10:35 schrieb danylo:
Hi Lionel,
Indeed the value 8 here is questionable. I picked it because other
drivers advertise the same value e.g. in Mesa radeon returns 8 for gl
and vulkan or on Windows Intel's driver returns 8. But why 8? It's some
kind of mystery.
"If the implementation truely has floating point viewport bounds, it
may report a sufficiently high value to indicate this. "
8 seems to be a sufficiently high value (it seems if someone even checks
the value it's going like 'precision > 0' - it is used as a flag). But
still it's probably not good enough argument...
Floating point (IEEE 754) has 24 bits of significand precision, in other
way - 6 to 9 significant decimal digits. And drivers return 8, the only
8 in float-point is 8 exponent bits.
Unless someone knows why 8, there two paths:
* Left it to be 8 - be the same as other drivers
* Make 24 - to reflect 24 bits of significand precision of float
- Danil
On 18.06.18 17:27, Lionel Landwerlin wrote:
Hey Danylo,
Thanks for this patch.
I'm not really an expert here but my understanding is that it should
reflect the number of bits in fixed point precision.
We use 32bits floats in the packets sent to the hardware.
Quoting the spec :
"If the implementation truely has floating point viewport bounds, it
may report a sufficiently high value to indicate this. "
Maybe we should use something a bit bigger than 8?
Cheers,
-
Lionel
On 18/06/18 13:50, Danylo Piliaiev wrote:
We use floating-points for viewport bounds so VIEWPORT_SUBPIXEL_BITS
should reflect this.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105975
Signed-off-by: Danylo Piliaiev
---
src/mesa/drivers/dri/i965/brw_context.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.c
b/src/mesa/drivers/dri/i965/brw_context.c
index 9ced230..eacf326 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -688,7 +688,7 @@ brw_initialize_context_constants(struct brw_context *brw)
/* ARB_viewport_array, OES_viewport_array */
if (devinfo->gen >= 6) {
ctx->Const.MaxViewports = GEN6_NUM_VIEWPORTS;
- ctx->Const.ViewportSubpixelBits = 0;
+ ctx->Const.ViewportSubpixelBits = 8;
/* Cast to float before negating because MaxViewportWidth is unsigned.
*/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev=02%7C01%7Csroland%40vmware.com%7Ce1ac7ef5378f4227aff508d5d5bfa182%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636649941389721543=of%2FIoIzosaje7A3euJESjJMySE3eFjNifMg5SXzeEJU%3D=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] [PATCH 2/2] glsl: Add an assert when cloning ir_dereference_record with invalid field
Signed-off-by: Danylo Piliaiev
---
src/compiler/glsl/ir_clone.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/compiler/glsl/ir_clone.cpp b/src/compiler/glsl/ir_clone.cpp
index 69441fae7d..e1f4f3b290 100644
--- a/src/compiler/glsl/ir_clone.cpp
+++ b/src/compiler/glsl/ir_clone.cpp
@@ -194,6 +194,7 @@ ir_dereference_array::clone(void *mem_ctx, struct
hash_table *ht) const
ir_dereference_record *
ir_dereference_record::clone(void *mem_ctx, struct hash_table *ht) const
{
+ assert(this->field_idx >= 0);
const char *field_name =
this->record->type->fields.structure[this->field_idx].name;
return new(mem_ctx) ir_dereference_record(this->record->clone(mem_ctx, ht),
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] glsl: Avoid propagating incompatible type of initializer
do_assignment validated assigment but when rhs type was not compatible
it proceeded without issues and returned error_emitted = false.
On the other hand process_initializer expected do_assignment to always
return compatible type and never fail.
As a result when variable was initialized with incompatible type
the type of variable changed to the incompatible one.
This manifested in unnecessary error messages and in one case in crash.
Example GLSL:
vec4 tmp = vec2(0.0);
tmp.z -= 1.0;
Past error messages:
initializer of type vec2 cannot be assigned to variable of type vec4
invalid swizzle / mask `z'
type mismatch
operands to arithmetic operators must be numeric
After this patch:
initializer of type vec2 cannot be assigned to variable of type vec4
In the other case when we initialize variable with incompatible struct,
accessing variable's field leaded to a crash. Example:
uniform struct {float field;} data;
...
vec4 tmp = data;
tmp.x -= 1.0;
After the patch there is only error line without a crash:
initializer of type #anon_struct cannot be assigned to variable of
type vec4
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107547
Signed-off-by: Danylo Piliaiev
---
src/compiler/glsl/ast_to_hir.cpp | 62 +---
1 file changed, 33 insertions(+), 29 deletions(-)
diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 5d3f10b682..93e7c8ec33 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -1012,6 +1012,8 @@ do_assignment(exec_list *instructions, struct
_mesa_glsl_parse_state *state,
mark_whole_array_access(rhs);
mark_whole_array_access(lhs);
}
+ } else {
+ error_emitted = true;
}
/* Most callers of do_assignment (assign, add_assign, pre_inc/dec,
@@ -4562,41 +4564,43 @@ process_initializer(ir_variable *var, ast_declaration
*decl,
/* Never emit code to initialize a uniform.
*/
const glsl_type *initializer_type;
+ bool error_emitted = false;
if (!type->qualifier.flags.q.uniform) {
- do_assignment(initializer_instructions, state,
- NULL,
- lhs, rhs,
- , true,
- true,
- type->get_location());
+ error_emitted =
+do_assignment(initializer_instructions, state,
+ NULL, lhs, rhs,
+ , true, true,
+ type->get_location());
initializer_type = result->type;
} else
initializer_type = rhs->type;
- var->constant_initializer = rhs->constant_expression_value(mem_ctx);
- var->data.has_initializer = true;
+ if (!error_emitted) {
+ var->constant_initializer = rhs->constant_expression_value(mem_ctx);
+ var->data.has_initializer = true;
- /* If the declared variable is an unsized array, it must inherrit
- * its full type from the initializer. A declaration such as
- *
- * uniform float a[] = float[](1.0, 2.0, 3.0, 3.0);
- *
- * becomes
- *
- * uniform float a[4] = float[](1.0, 2.0, 3.0, 3.0);
- *
- * The assignment generated in the if-statement (below) will also
- * automatically handle this case for non-uniforms.
- *
- * If the declared variable is not an array, the types must
- * already match exactly. As a result, the type assignment
- * here can be done unconditionally. For non-uniforms the call
- * to do_assignment can change the type of the initializer (via
- * the implicit conversion rules). For uniforms the initializer
- * must be a constant expression, and the type of that expression
- * was validated above.
- */
- var->type = initializer_type;
+ /* If the declared variable is an unsized array, it must inherrit
+ * its full type from the initializer. A declaration such as
+ *
+ * uniform float a[] = float[](1.0, 2.0, 3.0, 3.0);
+ *
+ * becomes
+ *
+ * uniform float a[4] = float[](1.0, 2.0, 3.0, 3.0);
+ *
+ * The assignment generated in the if-statement (below) will also
+ * automatically handle this case for non-uniforms.
+ *
+ * If the declared variable is not an array, the types must
+ * already match exactly. As a result, the type assignment
+ * here can be done unconditionally. For non-uniforms the call
+ * to do_assignment can change the type of the initializer (via
+ * the implicit conversion rules). For uniforms the initializer
+ * must be a constant expression, and the type of that expression
+ * was validated above.
+ */
+ var->type = initializer_type;
+ }
var-&
Re: [Mesa-dev] [PATCH] glsl: Avoid calling get_array_element for scalar constants
On 08/16/2018 07:47 AM, Tapani Pälli wrote:
On 08/16/2018 02:21 AM, Andres Gomez wrote:
Danylo, should we also include this in the stable queues ?
Yep makes sense, it's simple and fixes a crasher.
I agree.
On Mon, 2018-08-13 at 18:57 +0300, Danylo Piliaiev wrote:
Accessing scalar constant as an array in function call or
initializer list triggered assert in get_array_element.
Examples:
func(0[0]);
vec2 t = { 0[0], 0 };
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107550
Signed-off-by: Danylo Piliaiev
---
src/compiler/glsl/ir_constant_expression.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/compiler/glsl/ir_constant_expression.cpp
b/src/compiler/glsl/ir_constant_expression.cpp
index 4a0aff72c6..c9788c7053 100644
--- a/src/compiler/glsl/ir_constant_expression.cpp
+++ b/src/compiler/glsl/ir_constant_expression.cpp
@@ -826,7 +826,7 @@
ir_dereference_array::constant_expression_value(void *mem_ctx,
const unsigned component = idx->value.u[0];
return new(mem_ctx) ir_constant(array, component);
- } else {
+ } else if (array->type->is_array()) {
const unsigned index = idx->value.u[0];
return array->get_array_element(index)->clone(mem_ctx,
NULL);
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Avoid calling get_array_element for scalar constants
Hi,
On 08/14/2018 10:53 AM, Tapani Pälli wrote:
Hi;
On 08/13/2018 06:57 PM, Danylo Piliaiev wrote:
Accessing scalar constant as an array in function call or
initializer list triggered assert in get_array_element.
Examples:
func(0[0]);
vec2 t = { 0[0], 0 };
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107550
Signed-off-by: Danylo Piliaiev
---
src/compiler/glsl/ir_constant_expression.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/compiler/glsl/ir_constant_expression.cpp
b/src/compiler/glsl/ir_constant_expression.cpp
index 4a0aff72c6..c9788c7053 100644
--- a/src/compiler/glsl/ir_constant_expression.cpp
+++ b/src/compiler/glsl/ir_constant_expression.cpp
@@ -826,7 +826,7 @@
ir_dereference_array::constant_expression_value(void *mem_ctx,
const unsigned component = idx->value.u[0];
return new(mem_ctx) ir_constant(array, component);
- } else {
+ } else if (array->type->is_array()) {
IMO this is correct but we could alternatively bail out already
earlier before the topmost if-statement or just add this condition as
part of the topmost if-statement?
We don't have "accessible_as_array" kind of check (correct me if I'm
wrong), vector and matrix are not is_array so to bail out earlier there
should be a check:
is_matrix || is_vector || is_array
or add new method to glsl_type. But from my perspective this will only
invert conditions order in constant_expression_value.
const unsigned index = idx->value.u[0];
return array->get_array_element(index)->clone(mem_ctx, NULL);
}
// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Avoid calling get_array_element for scalar constants
Hi,
On 08/14/2018 10:43 AM, Tapani Pälli wrote:
On 08/13/2018 08:54 PM, Ian Romanick wrote:
On 08/13/2018 08:57 AM, Danylo Piliaiev wrote:
Accessing scalar constant as an array in function call or
initializer list triggered assert in get_array_element.
Examples:
func(0[0]);
vec2 t = { 0[0], 0 };
Do we have tests like this in piglit? What happens if 0[0] syntax is
used other places? What do happens in these cases after this change?
Some sort of graceful compile-time error, I hope...
In other places we already fail with the same error ("cannot dereference
non-array / non-matrix / non-vector").
I've sent a glslparsertest for this. With the change we fail
compilation in this case like:
0:8(9): error: cannot dereference non-array / non-matrix / non-vector
0:8(2): error: no function with name 'func'
Thanks for the test.
- Danil
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107550
Signed-off-by: Danylo Piliaiev
---
src/compiler/glsl/ir_constant_expression.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/compiler/glsl/ir_constant_expression.cpp
b/src/compiler/glsl/ir_constant_expression.cpp
index 4a0aff72c6..c9788c7053 100644
--- a/src/compiler/glsl/ir_constant_expression.cpp
+++ b/src/compiler/glsl/ir_constant_expression.cpp
@@ -826,7 +826,7 @@
ir_dereference_array::constant_expression_value(void *mem_ctx,
const unsigned component = idx->value.u[0];
return new(mem_ctx) ir_constant(array, component);
- } else {
+ } else if (array->type->is_array()) {
const unsigned index = idx->value.u[0];
return array->get_array_element(index)->clone(mem_ctx,
NULL);
}
___
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] glsl: Avoid calling get_array_element for scalar constants
Accessing scalar constant as an array in function call or
initializer list triggered assert in get_array_element.
Examples:
func(0[0]);
vec2 t = { 0[0], 0 };
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107550
Signed-off-by: Danylo Piliaiev
---
src/compiler/glsl/ir_constant_expression.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/compiler/glsl/ir_constant_expression.cpp
b/src/compiler/glsl/ir_constant_expression.cpp
index 4a0aff72c6..c9788c7053 100644
--- a/src/compiler/glsl/ir_constant_expression.cpp
+++ b/src/compiler/glsl/ir_constant_expression.cpp
@@ -826,7 +826,7 @@ ir_dereference_array::constant_expression_value(void
*mem_ctx,
const unsigned component = idx->value.u[0];
return new(mem_ctx) ir_constant(array, component);
- } else {
+ } else if (array->type->is_array()) {
const unsigned index = idx->value.u[0];
return array->get_array_element(index)->clone(mem_ctx, NULL);
}
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Advertise 8 bits subpixel precision for viewport bounds on gen6+
The patch seems to be safe, so yes, thanks.
On 8/21/18 1:17 AM, Andres Gomez wrote:
Danylo, should we also include this in the stable queues ?
On Mon, 2018-06-18 at 15:50 +0300, Danylo Piliaiev wrote:
We use floating-points for viewport bounds so VIEWPORT_SUBPIXEL_BITS
should reflect this.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105975
Signed-off-by: Danylo Piliaiev
---
src/mesa/drivers/dri/i965/brw_context.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.c
b/src/mesa/drivers/dri/i965/brw_context.c
index 9ced230..eacf326 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -688,7 +688,7 @@ brw_initialize_context_constants(struct brw_context *brw)
/* ARB_viewport_array, OES_viewport_array */
if (devinfo->gen >= 6) {
ctx->Const.MaxViewports = GEN6_NUM_VIEWPORTS;
- ctx->Const.ViewportSubpixelBits = 0;
+ ctx->Const.ViewportSubpixelBits = 8;
/* Cast to float before negating because MaxViewportWidth is unsigned.
*/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Advertise 8 bits subpixel precision for viewport bounds on gen6+
On 8/20/18 4:28 PM, Lionel Landwerlin wrote:
Reviewed-by: Lionel Landwerlin
Thanks!
Do you need me to push it?
Yes
Thanks,
-
Lionel
On 08/08/18 16:03, Danylo Piliaiev wrote:
Hi,
Since the exact value is not important for users, Roland offered
compelling explanation
and the value is also 8 on Windows - could the patch be reviewed/pushed?
- Danil
On 06/19/2018 03:44 PM, Roland Scheidegger wrote:
My guess would be 8 because that's what the rasterization subpixel
precision is, so precision beyond that doesn't really do much, even if
this actually is a float.
Plus, with maximum sized fb (16kx16k dimension) you don't actually
really get a lot more than 8 fixed points bits anyway (near those 16k).
So imho 8 makes most sense.
Roland
Am 19.06.2018 um 10:35 schrieb danylo:
Hi Lionel,
Indeed the value 8 here is questionable. I picked it because other
drivers advertise the same value e.g. in Mesa radeon returns 8 for gl
and vulkan or on Windows Intel's driver returns 8. But why 8? It's
some
kind of mystery.
"If the implementation truely has floating point viewport bounds, it
may report a sufficiently high value to indicate this. "
8 seems to be a sufficiently high value (it seems if someone even
checks
the value it's going like 'precision > 0' - it is used as a flag). But
still it's probably not good enough argument...
Floating point (IEEE 754) has 24 bits of significand precision, in
other
way - 6 to 9 significant decimal digits. And drivers return 8, the
only
8 in float-point is 8 exponent bits.
Unless someone knows why 8, there two paths:
* Left it to be 8 - be the same as other drivers
* Make 24 - to reflect 24 bits of significand precision of float
- Danil
On 18.06.18 17:27, Lionel Landwerlin wrote:
Hey Danylo,
Thanks for this patch.
I'm not really an expert here but my understanding is that it should
reflect the number of bits in fixed point precision.
We use 32bits floats in the packets sent to the hardware.
Quoting the spec :
"If the implementation truely has floating point viewport bounds, it
may report a sufficiently high value to indicate this. "
Maybe we should use something a bit bigger than 8?
Cheers,
-
Lionel
On 18/06/18 13:50, Danylo Piliaiev wrote:
We use floating-points for viewport bounds so VIEWPORT_SUBPIXEL_BITS
should reflect this.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105975
Signed-off-by: Danylo Piliaiev
---
src/mesa/drivers/dri/i965/brw_context.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.c
b/src/mesa/drivers/dri/i965/brw_context.c
index 9ced230..eacf326 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -688,7 +688,7 @@ brw_initialize_context_constants(struct
brw_context *brw)
/* ARB_viewport_array, OES_viewport_array */
if (devinfo->gen >= 6) {
ctx->Const.MaxViewports = GEN6_NUM_VIEWPORTS;
- ctx->Const.ViewportSubpixelBits = 0;
+ ctx->Const.ViewportSubpixelBits = 8;
/* Cast to float before negating because
MaxViewportWidth is unsigned.
*/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev=02%7C01%7Csroland%40vmware.com%7Ce1ac7ef5378f4227aff508d5d5bfa182%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636649941389721543=of%2FIoIzosaje7A3euJESjJMySE3eFjNifMg5SXzeEJU%3D=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 mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: Do not propagate 'precise' and 'invariant' on read-only variables
Read-only variables could be considered inherently invariant and precise
since no expression in shader affects them.
Explicitly marking them as such is unnecessary and can cause issues,
e.g. uniform marked as invariant may require the same uniform in other
stage to be invariant too but uniforms cannot have "invariant" qualifier.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100316
Signed-off-by: Danylo Piliaiev
---
My assumption is that variable being read_only means that it either
loads a value from an external source or is initialized from constant value.
From what I saw in code it's true but it may be wrong to depend on it.
src/compiler/glsl/propagate_invariance.cpp | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/compiler/glsl/propagate_invariance.cpp
b/src/compiler/glsl/propagate_invariance.cpp
index b3f1d810cd..0e4233aa9f 100644
--- a/src/compiler/glsl/propagate_invariance.cpp
+++ b/src/compiler/glsl/propagate_invariance.cpp
@@ -96,6 +96,16 @@
ir_invariance_propagation_visitor::visit(ir_dereference_variable *ir)
if (this->dst_var == NULL)
return visit_continue;
+ /* Read-only variables could be considered inherently
+* invariant and precise since no expression in shader affects them.
+* Explicitly marking them as such is unnecessary and can cause
+* issues, e.g. uniform marked as invariant may require the same
+* uniform in other stage to be invariant too but uniforms cannot
+* have "invariant" qualifier.
+*/
+ if (ir->var->data.read_only)
+ return visit_continue;
+
if (this->dst_var->data.invariant) {
if (!ir->var->data.invariant)
this->progress = true;
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Do not propagate 'precise' and 'invariant' on read-only variables
Hi Jason,
On 8/22/18 7:07 PM, Jason Ekstrand wrote:
I think this would be correct to do and would fix the bug at hand but
there may be other bugs lurking deeper. In particular, if you have
multiple shaders for the same stage that share some global variables
and get linked together, I think you could run into the same issue
only with a read-write global. If this is indeed possible, then I'm
not sure what to do. We have to run this pass prior to doing any
optimization because we depend on it to get precise and invariant
correct. On the other hand, it has to be run after linking. :-/
Maybe we just have to assume precise and invariant for all pre-linking
optimizations and then run this post-linking.
I didn't check this case, thanks for pointing it out. Now I have looked
at relevant code and saw that invariant propagation (linker.cpp:5024 in
linker_optimisation_loop) is done after 'link_intrastage_shaders'
(linker.cpp:4868) which combines a group of shaders for a single stage.
Actually invariant propagation is also done once before linking. In
addition I've done some tests and invariance from variable in one vertex
shader is propagated on variables directly used only in other vertex
shader.
However I have found other issue - cross_validate_globals is called in
link_intrastage_shaders and throws error if 'invariant' qualifier
doesn't match between shaders in same stage which happens when variable
in one shader becomes invariant before the 'link_intrastage_shaders'
call. This could be fixed separately.
Or there is another way to solve these issues: divide 'invariant'
qualifier into explicit and implicit 'invariant', where explicit is for
'invariant' set by user in shader and implicit is for 'invariant'
propagated from other invariants. Thus in interface checking we will be
able to check only for explicit invariance which besides two previous
issues with solve another one: if out variable is marked as invariant
due to invariance propagation this variable will be required to have
invariant qualifier in the next stage even though user never marked it
as such anywhere. And when the invariance will actually matter we will
check both implicit and explicit one. What do you think about such solution?
- Danil.
On Wed, Aug 22, 2018 at 7:51 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>> wrote:
Read-only variables could be considered inherently invariant and
precise
since no expression in shader affects them.
Explicitly marking them as such is unnecessary and can cause issues,
e.g. uniform marked as invariant may require the same uniform in other
stage to be invariant too but uniforms cannot have "invariant"
qualifier.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100316
Signed-off-by: Danylo Piliaiev mailto:danylo.pilia...@globallogic.com>>
---
My assumption is that variable being read_only means that it either
loads a value from an external source or is initialized from
constant value.
From what I saw in code it's true but it may be wrong to depend on it.
src/compiler/glsl/propagate_invariance.cpp | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/compiler/glsl/propagate_invariance.cpp
b/src/compiler/glsl/propagate_invariance.cpp
index b3f1d810cd..0e4233aa9f 100644
--- a/src/compiler/glsl/propagate_invariance.cpp
+++ b/src/compiler/glsl/propagate_invariance.cpp
@@ -96,6 +96,16 @@
ir_invariance_propagation_visitor::visit(ir_dereference_variable *ir)
if (this->dst_var == NULL)
return visit_continue;
+ /* Read-only variables could be considered inherently
+ * invariant and precise since no expression in shader affects
them.
+ * Explicitly marking them as such is unnecessary and can cause
+ * issues, e.g. uniform marked as invariant may require the same
+ * uniform in other stage to be invariant too but uniforms cannot
+ * have "invariant" qualifier.
+ */
+ if (ir->var->data.read_only)
+ return visit_continue;
+
if (this->dst_var->data.invariant) {
if (!ir->var->data.invariant)
this->progress = true;
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl/ast: Fix loss of error_emitted value due to reassignment
On 19.07.18 21:41, Ian Romanick wrote: On 07/18/2018 01:53 AM, Danylo Piliaiev wrote: Signed-off-by: Danylo Piliaiev --- src/compiler/glsl/ast_to_hir.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp index dd60a2a87f..8a4cc56511 100644 --- a/src/compiler/glsl/ast_to_hir.cpp +++ b/src/compiler/glsl/ast_to_hir.cpp @@ -1938,7 +1938,7 @@ ast_expression::do_hir(exec_list *instructions, result = get_lvalue_copy(instructions, op[0]->clone(ctx, NULL)); ir_rvalue *junk_rvalue; - error_emitted = + error_emitted |= do_assignment(instructions, state, this->subexpressions[0]->non_lvalue_description, op[0]->clone(ctx, NULL), temp_rhs, Are there any tests that encounter this? It seems like do_assignment should always generate an error if either of the operands have errors. Looked at it once more, and indeed error will be emitted by do_assignment in this case, so first assignment to error_emitted in ast_post_inc/dec is just superfluous. I found the reassignment by running cppcheck, should have paid more attention fixing it... So I'll just remove the assignment. I notice that the ast_pre_inc / ast_pre_dec case just before this only sets error_emitted once. Whatever we decided, ast_pre_* and ast_post_* should do it the same way. Intuitively, I think the ast_pre_inc / ast_pre_dec method is correct, but you'll have to confirm or refute that with testing. :) Since this is a Boolean value, I have a mild preference for error_emitted = do_assignment(instructions, state, this->subexpressions[0]->non_lvalue_description, op[0]->clone(ctx, NULL), temp_rhs, _rvalue, false, false, this->subexpressions[0]->get_location()) || error_emitted; or (as is done in the ast_array_index case just below this): if (do_assignment(instructions, state, this->subexpressions[0]->non_lvalue_description, op[0]->clone(ctx, NULL), temp_rhs, _rvalue, false, false, this->subexpressions[0]->get_location())) error_emitted = true; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] anv: Disable dual source blending when shader doesn't support it on gen8+
Dual source blending behaviour is undefined when shader doesn't
have second color output.
"If SRC1 is included in a src/dst blend factor and
a DualSource RT Write message is not used, results
are UNDEFINED. (This reflects the same restriction in DX APIs,
where undefined results are produced if “o1” is not written
by a PS – there are no default values defined)."
Dismissing fragment in such situation leads to a hang on gen8+
if depth test in enabled.
Since blending cannot be gracefully fixed in such case and the result
is undefined - blending is simply disabled.
Signed-off-by: Danylo Piliaiev
---
Also there is a similar patch which fixes the same issue
for OpenGL https://patchwork.freedesktop.org/patch/235939
src/intel/vulkan/genX_pipeline.c | 43
1 file changed, 32 insertions(+), 11 deletions(-)
diff --git a/src/intel/vulkan/genX_pipeline.c b/src/intel/vulkan/genX_pipeline.c
index 0821d71c9f..d7c532a22d 100644
--- a/src/intel/vulkan/genX_pipeline.c
+++ b/src/intel/vulkan/genX_pipeline.c
@@ -854,6 +854,15 @@ emit_ds_state(struct anv_pipeline *pipeline,
#endif
}
+MAYBE_UNUSED static bool
+is_dual_src_blend_factor(VkBlendFactor factor)
+{
+ return factor == VK_BLEND_FACTOR_SRC1_COLOR ||
+ factor == VK_BLEND_FACTOR_ONE_MINUS_SRC1_COLOR ||
+ factor == VK_BLEND_FACTOR_SRC1_ALPHA ||
+ factor == VK_BLEND_FACTOR_ONE_MINUS_SRC1_ALPHA;
+}
+
static void
emit_cb_state(struct anv_pipeline *pipeline,
const VkPipelineColorBlendStateCreateInfo *info,
@@ -886,6 +895,7 @@ emit_cb_state(struct anv_pipeline *pipeline,
state_pos += GENX(BLEND_STATE_length);
#if GEN_GEN >= 8
struct GENX(BLEND_STATE_ENTRY) bs0 = { 0 };
+ bool force_disable_color_blend = false;
#endif
for (unsigned i = 0; i < surface_count; i++) {
struct anv_pipeline_binding *binding = >surface_to_descriptor[i];
@@ -969,8 +979,27 @@ emit_cb_state(struct anv_pipeline *pipeline,
GENX(BLEND_STATE_ENTRY_pack)(NULL, state_pos, );
state_pos += GENX(BLEND_STATE_ENTRY_length);
#if GEN_GEN >= 8
- if (i == 0)
+ if (i == 0) {
bs0 = entry;
+
+ /* The Dual Source Blending documentation says:
+ *
+ * "If SRC1 is included in a src/dst blend factor and
+ * a DualSource RT Write message is not used, results
+ * are UNDEFINED. (This reflects the same restriction in DX APIs,
+ * where undefined results are produced if “o1” is not written
+ * by a PS – there are no default values defined)."
+ *
+ * There is no way to gracefully fix this undefined situation
+ * so we just disable the blending to prevent possible issues.
+ */
+ const struct brw_wm_prog_data *wm_prog_data =
get_wm_prog_data(pipeline);
+ force_disable_color_blend = !wm_prog_data->dual_src_blend &&
+(is_dual_src_blend_factor(a->srcColorBlendFactor) ||
+is_dual_src_blend_factor(a->dstColorBlendFactor) ||
+is_dual_src_blend_factor(a->srcAlphaBlendFactor) ||
+is_dual_src_blend_factor(a->dstAlphaBlendFactor));
+ }
#endif
}
@@ -978,7 +1007,8 @@ emit_cb_state(struct anv_pipeline *pipeline,
anv_batch_emit(>batch, GENX(3DSTATE_PS_BLEND), blend) {
blend.AlphaToCoverageEnable = blend_state.AlphaToCoverageEnable;
blend.HasWriteableRT= has_writeable_rt;
- blend.ColorBufferBlendEnable= bs0.ColorBufferBlendEnable;
+ blend.ColorBufferBlendEnable=
+ bs0.ColorBufferBlendEnable && !force_disable_color_blend;
blend.SourceAlphaBlendFactor= bs0.SourceAlphaBlendFactor;
blend.DestinationAlphaBlendFactor = bs0.DestinationAlphaBlendFactor;
blend.SourceBlendFactor = bs0.SourceBlendFactor;
@@ -1430,15 +1460,6 @@ emit_3dstate_wm(struct anv_pipeline *pipeline, struct
anv_subpass *subpass,
}
}
-UNUSED static bool
-is_dual_src_blend_factor(VkBlendFactor factor)
-{
- return factor == VK_BLEND_FACTOR_SRC1_COLOR ||
- factor == VK_BLEND_FACTOR_ONE_MINUS_SRC1_COLOR ||
- factor == VK_BLEND_FACTOR_SRC1_ALPHA ||
- factor == VK_BLEND_FACTOR_ONE_MINUS_SRC1_ALPHA;
-}
-
static void
emit_3dstate_ps(struct anv_pipeline *pipeline,
const VkPipelineColorBlendStateCreateInfo *blend,
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 1/3] i965: Sweep NIR after linking phase to free held memory
Could it be pushed? The commit is independent from other in series. - Danil On 12.07.18 00:27, Jason Ekstrand wrote: Reviewed-by: Jason Ekstrand <mailto:ja...@jlekstrand.net>> On Wed, Jul 11, 2018 at 5:29 AM Danylo Piliaiev mailto:danylo.pilia...@gmail.com>> wrote: After optimization passes and many trasfromations most of memory NIR holds is a garbage which was being freed only after shader deletion. Freeing it at the end of linking will save memory which would be useful in case there are a lot of complex shaders being compiled. The common case for this issue is 32bit game running under Wine. The cost of the optimization is around ~3-5% of compilation speed with complex shaders. V2: by Jason Ekstrand - Move nir_sweep up, right after the last change of NIR Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103274 Signed-off-by: Danylo Piliaiev mailto:danylo.pilia...@globallogic.com>> --- src/mesa/drivers/dri/i965/brw_link.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp index 1071056f14..378426101b 100644 --- a/src/mesa/drivers/dri/i965/brw_link.cpp +++ b/src/mesa/drivers/dri/i965/brw_link.cpp @@ -317,6 +317,8 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg) NIR_PASS_V(prog->nir, nir_lower_atomics_to_ssbo, prog->nir->info.num_abos); + nir_sweep(prog->nir); + infos[stage] = >nir->info; update_xfb_info(prog->sh.LinkedTransformFeedback, infos[stage]); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl/ast: Fix loss of error_emitted value due to reassignment
Signed-off-by: Danylo Piliaiev --- src/compiler/glsl/ast_to_hir.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp index dd60a2a87f..8a4cc56511 100644 --- a/src/compiler/glsl/ast_to_hir.cpp +++ b/src/compiler/glsl/ast_to_hir.cpp @@ -1938,7 +1938,7 @@ ast_expression::do_hir(exec_list *instructions, result = get_lvalue_copy(instructions, op[0]->clone(ctx, NULL)); ir_rvalue *junk_rvalue; - error_emitted = + error_emitted |= do_assignment(instructions, state, this->subexpressions[0]->non_lvalue_description, op[0]->clone(ctx, NULL), temp_rhs, -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] radv: Fix incorrect assumption about ternary operator precedence
Signed-off-by: Danylo Piliaiev --- src/amd/vulkan/radv_shader.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c index 1399005998..aac5b8a21a 100644 --- a/src/amd/vulkan/radv_shader.c +++ b/src/amd/vulkan/radv_shader.c @@ -361,8 +361,8 @@ radv_alloc_shader_memory(struct radv_device *device, slab->bo = device->ws->buffer_create(device->ws, slab->size, 256, RADEON_DOMAIN_VRAM, RADEON_FLAG_NO_INTERPROCESS_SHARING | - device->physical_device->cpdma_prefetch_writes_memory ? -0 : RADEON_FLAG_READ_ONLY); + (device->physical_device->cpdma_prefetch_writes_memory ? +0 : RADEON_FLAG_READ_ONLY)); slab->ptr = (char*)device->ws->buffer_map(slab->bo); list_inithead(>shaders); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: Fix copy-paste error in ConservativeRasterDilateRange initialization
Signed-off-by: Danylo Piliaiev --- src/mesa/main/context.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c index 4243e03c5c..0ef8fe32a6 100644 --- a/src/mesa/main/context.c +++ b/src/mesa/main/context.c @@ -742,7 +742,7 @@ _mesa_init_constants(struct gl_constants *consts, gl_api api) /** GL_NV_conservative_raster_dilate */ consts->ConservativeRasterDilateRange[0] = 0.0; - consts->ConservativeRasterDilateRange[0] = 0.0; + consts->ConservativeRasterDilateRange[1] = 0.0; consts->ConservativeRasterDilateGranularity = 0.0; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 029/129] nir: Support deref instructions in lower_wpos_ytransform
Hi,
I was running cppcheck on Mesa and found the issue mentioned here:
This can't possibly work. The function immediately dereferences
fragcoord, so it can't legally be NULL. Which then begs the question,
how on earth did it work before? It would just read a non-existent
variable out of the intrinsic and use that (i.e. NULL anyway).
So, I did a bit of poking around, and noticed that everybody calls this
before nir_lower_system_values, so I think this case could simply be
deleted. Not sure whether to do that before or after your patch.
We should really delete it at some point.
i965 calls nir_lower_system_values before calling nir_lower_wpos_ytransform,
but lower_fragcoord(state, intr, NULL); seems still to be unreachable.
The question is, is it still ok to remove it?
- Danil
On 12.06.18 22:50, Kenneth Graunke wrote:
On Thursday, May 31, 2018 10:02:12 PM PDT Jason Ekstrand wrote:
Reviewed-by: Caio Marcelo de Oliveira Filho
---
src/compiler/nir/nir_lower_wpos_ytransform.c | 53 ++--
1 file changed, 42 insertions(+), 11 deletions(-)
diff --git a/src/compiler/nir/nir_lower_wpos_ytransform.c
b/src/compiler/nir/nir_lower_wpos_ytransform.c
index 9cb5c71..6212702 100644
--- a/src/compiler/nir/nir_lower_wpos_ytransform.c
+++ b/src/compiler/nir/nir_lower_wpos_ytransform.c
@@ -77,11 +77,10 @@ nir_cmp(nir_builder *b, nir_ssa_def *src0, nir_ssa_def
*src1, nir_ssa_def *src2)
/* see emit_wpos_adjustment() in st_mesa_to_tgsi.c */
static void
emit_wpos_adjustment(lower_wpos_ytransform_state *state,
- nir_intrinsic_instr *intr,
+ nir_intrinsic_instr *intr, nir_variable *fragcoord,
bool invert, float adjX, float adjY[2])
{
nir_builder *b = >b;
- nir_variable *fragcoord = intr->variables[0]->var;
nir_ssa_def *wpostrans, *wpos_temp, *wpos_temp_y, *wpos_input;
assert(intr->dest.is_ssa);
@@ -144,10 +143,10 @@ emit_wpos_adjustment(lower_wpos_ytransform_state *state,
}
static void
-lower_fragcoord(lower_wpos_ytransform_state *state, nir_intrinsic_instr *intr)
+lower_fragcoord(lower_wpos_ytransform_state *state,
+nir_intrinsic_instr *intr, nir_variable *fragcoord)
{
const nir_lower_wpos_ytransform_options *options = state->options;
- nir_variable *fragcoord = intr->variables[0]->var;
float adjX = 0.0f;
float adjY[2] = { 0.0f, 0.0f };
bool invert = false;
@@ -229,7 +228,7 @@ lower_fragcoord(lower_wpos_ytransform_state *state,
nir_intrinsic_instr *intr)
}
}
- emit_wpos_adjustment(state, intr, invert, adjX, adjY);
+ emit_wpos_adjustment(state, intr, fragcoord, invert, adjX, adjY);
}
/* turns 'fddy(p)' into 'fddy(fmul(p, transform.x))' */
@@ -253,7 +252,25 @@ lower_fddy(lower_wpos_ytransform_state *state,
nir_alu_instr *fddy)
fddy->src[0].swizzle[i] = MIN2(i, pt->num_components - 1);
}
-/* Multiply interp_var_at_offset's offset by transform.x to flip it. */
+/* Multiply interp_deref_at_offset's offset by transform.x to flip it. */
+static void
+lower_interp_deref_at_offset(lower_wpos_ytransform_state *state,
+ nir_intrinsic_instr *interp)
+{
+ nir_builder *b = >b;
+ nir_ssa_def *offset;
+ nir_ssa_def *flip_y;
+
+ b->cursor = nir_before_instr(>instr);
+
+ offset = nir_ssa_for_src(b, interp->src[1], 2);
+ flip_y = nir_fmul(b, nir_channel(b, offset, 1),
+nir_channel(b, get_transform(state), 0));
+ nir_instr_rewrite_src(>instr, >src[1],
+ nir_src_for_ssa(nir_vec2(b, nir_channel(b, offset, 0),
+ flip_y)));
+}
+
static void
lower_interp_var_at_offset(lower_wpos_ytransform_state *state,
nir_intrinsic_instr *interp)
@@ -298,7 +315,21 @@ lower_wpos_ytransform_block(lower_wpos_ytransform_state
*state, nir_block *block
nir_foreach_instr_safe(instr, block) {
if (instr->type == nir_instr_type_intrinsic) {
nir_intrinsic_instr *intr = nir_instr_as_intrinsic(instr);
- if (intr->intrinsic == nir_intrinsic_load_var) {
+ if (intr->intrinsic == nir_intrinsic_load_deref) {
+nir_deref_instr *deref = nir_src_as_deref(intr->src[0]);
+nir_variable *var = nir_deref_instr_get_variable(deref);
+
+if ((var->data.mode == nir_var_shader_in &&
+ var->data.location == VARYING_SLOT_POS) ||
+(var->data.mode == nir_var_system_value &&
+ var->data.location == SYSTEM_VALUE_FRAG_COORD)) {
+ /* gl_FragCoord should not have array/struct derefs: */
+ lower_fragcoord(state, intr, var);
+} else if (var->data.mode == nir_var_system_value &&
+ var->data.location == SYSTEM_VALUE_SAMPLE_POS) {
+ lower_load_sample_pos(state, intr);
+}
Lots of duplication here :( But I suppose
Re: [Mesa-dev] [PATCH 2/3] st: Sweep NIR after linking phase to free held memory
On 19.07.18 12:30, Timothy Arceri wrote:
On 19.07.18 11:47, Timothy Arceri wrote:
On 19/07/18 08:31, Eric Anholt wrote:
Danylo Piliaiev writes:
After optimization passes and many trasfromations most of memory
"transformations"
NIR holds is a garbage which was being freed only after shader
deletion.
"is garbage"
Freeing it at the end of linking will save memory which would be
useful
in case there are a lot of complex shaders being compiled.
The common case for this issue is 32bit game running under Wine.
The cost of the optimization is around ~3-5% of compilation speed
with complex shaders.
Signed-off-by: Danylo Piliaiev
This seems good, and I'm running it through the CTS now.
The problem is this does the sweep too early. We still do lots of
work on NIR after this, I've thought about this a few times and it
really seems we should be able to call a driver specific function
from the st and pass it the IR so it can do what ever it wants
(lowering opts etc) and spits it back out. Once this is done we
should then call sweep and cache the IR.
At the very least we should call sweep before we cache NIR rather
than where this patch places it.
I debugged mesa once more and cannot see where else memory is
allocated for NIR after this sweep. Could you point it to me? After
this sweep the memory NIR holds never grows. Later this NIR is cloned
from and these cloned NIRs are being sweeped in other place.
Ah yes you are right. We do clone it when creating variants I'd
forgotten about that. In that case please ignore my comment.
Good, I thought I really missed something...
Also during my investigation of Mesa memory usage I wrote gdb pretty
printer which shows how much memory variable holds in its ralloc context
(with all its children), it was crudely written and at this moment has
sever limitation: x64 only, depends on internal malloc implementation
and other hardcoded things, also I wasn't able to nicely display
children of a variable. The reason that pretty printer was done this way
is that calling c function (e.g. malloc_usable_size) corrupts backtrace
somehow.
Example usage:
(gdb) source ralloc_info_pretty_printer.py
(gdb) backtrace
#0 brw_link_shader (ctx=0x558ca0a0 ,
shProg=0x55d850c0 ) at brw_link.cpp:320
#1 0x72732b6f in _mesa_glsl_link_shader (ctx=0x558ca0a0
, prog=0x55d850c0 ) at
program/ir_to_mesa.cpp:3174
#2 0x725a1862 in link_program (no_error=false,
shProg=0x55d850c0 , ctx=0x558ca0a0 ) at
main/shaderapi.c:1206
#3 link_program_error (ctx=0x558ca0a0 ,
shProg=0x55d850c0 ) at main/shaderapi.c:1286
#4 0x725a2f00 in _mesa_LinkProgram (programObj=3) at
main/shaderapi.c:1778
#5 0x6de1 in main () at
/home/danylo/Projects/shader_compilation_memory_test/test.cpp:421
(gdb) p prog->nir
$1 = 0x55dc2b20
If there is any interest in having it in Mesa I can clean it up. You can
find its code in attachment.
import gdb
import gdb.types
import gdb.printing
have_python_2 = (sys.version_info[0] == 2)
have_python_3 = (sys.version_info[0] == 3)
if have_python_3:
intptr = int
elif have_python_2:
intptr = long
def get_ralloc_header(val):
try:
ralloc_header_type = gdb.lookup_type("ralloc_header")
ralloc_header_type_pointer = ralloc_header_type.pointer()
val_as_header_ptr = val.cast(ralloc_header_type_pointer)
if not val_as_header_ptr:
return None
header_ptr = val_as_header_ptr - 1
if not header_ptr:
return None
canary = header_ptr["canary"]
if canary and int(canary) == 0x5A1106:
return header_ptr
else:
return None
except:
return None
class RAllocPrinter:
"Pretty Printer for ralloc"
printer_name = 'ralloc'
def __init__(self, val, header):
self.val = val
self.header = header;
self.has_child = True
def calc_alloc_size(self, header_ptr):
headers = [header_ptr]
size = 0
current_inferior = gdb.selected_inferior()
size_bits_mask = ~(0x1 | 0x2 | 0x4)
while headers:
current_header_ptr = headers.pop()
child_ptr = current_header_ptr["child"]
while intptr(child_ptr) != 0:
canary = child_ptr["canary"]
if not canary or int(canary) != 0x5A1106:
return -1
headers.append(child_ptr)
child_ptr = child_ptr["next"]
#struct malloc_chunk {
#INTERNAL_SIZE_T prev_size; /* Size of previous chunk (if free). */
#INTERNAL_SIZE_T size; /* Size in bytes, including overhead. */ <--
# --USABLE MEMORY
mem = current_inferior.read_memory(int(current_header_ptr) - 8, 8)
Re: [Mesa-dev] [PATCH 2/3] st: Sweep NIR after linking phase to free held memory
On 19.07.18 11:47, Timothy Arceri wrote: On 19/07/18 08:31, Eric Anholt wrote: Danylo Piliaiev writes: After optimization passes and many trasfromations most of memory "transformations" NIR holds is a garbage which was being freed only after shader deletion. "is garbage" Freeing it at the end of linking will save memory which would be useful in case there are a lot of complex shaders being compiled. The common case for this issue is 32bit game running under Wine. The cost of the optimization is around ~3-5% of compilation speed with complex shaders. Signed-off-by: Danylo Piliaiev This seems good, and I'm running it through the CTS now. The problem is this does the sweep too early. We still do lots of work on NIR after this, I've thought about this a few times and it really seems we should be able to call a driver specific function from the st and pass it the IR so it can do what ever it wants (lowering opts etc) and spits it back out. Once this is done we should then call sweep and cache the IR. At the very least we should call sweep before we cache NIR rather than where this patch places it. I debugged mesa once more and cannot see where else memory is allocated for NIR after this sweep. Could you point it to me? After this sweep the memory NIR holds never grows. Later this NIR is cloned from and these cloned NIRs are being sweeped in other place. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3] vulkan: Fix compilation on older platforms
Hi Eric,
On 24.07.18 18:43, Eric Engestrom wrote:
On Friday, 2018-07-13 16:57:34 +0300, Danylo Piliaiev wrote:
Make xlease automatically enabled only if xcb-randr >= 1.13,
check its version if manually enabled.
Enable VK_EXT_display_control only when libdrm >= 2.4.89
Check for DRM_EVENT_CONTEXT_VERSION >= 4 to use sequence_handler.
You're changing 3 unrelated things here; please send 3 patches :)
Ok, I'll separate patches this time:
1) xcb-randr check
2) Bumping libdrm for vulkan
3) Removal checks of DRM_EVENT_CONTEXT_VERSION as suggested by Emil.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107170
https://bugs.freedesktop.org/show_bug.cgi?id=106972
https://bugs.freedesktop.org/show_bug.cgi?id=107176
v2: - Add 'protect="VK_USE_DISPLAY_CONTROL"' attribute to
VK_EXT_display_control in vk.xml
- Add support for 'protect' attribute to anv_entrypoints_gen
(copied from radv_entrypoints_gen)
- Turn #if into #ifdef
- Remove unnecessary pkg-config call from meson build (Dylan Baker)
v3: by Dylan Baker
- Remove previously added changes to vk.xml and entrypoints
generation because vk.xml is meant to be pulled from the external
source.
Signed-off-by: Danylo Piliaiev
---
configure.ac | 29 +-
meson.build| 10 -
src/amd/vulkan/radv_extensions.py | 9 +++-
src/amd/vulkan/radv_wsi_display.c | 5 ++---
src/intel/vulkan/anv_extensions.py | 2 +-
src/intel/vulkan/anv_extensions_gen.py | 7 +++
src/intel/vulkan/anv_wsi_display.c | 4 ++--
src/vulkan/wsi/wsi_common_display.c| 8 +--
src/vulkan/wsi/wsi_common_display.h| 3 ++-
9 files changed, 51 insertions(+), 26 deletions(-)
diff --git a/configure.ac b/configure.ac
index f135d05736..0b04525014 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,8 @@ LIBDRM_FREEDRENO_REQUIRED=2.4.92
LIBDRM_ETNAVIV_REQUIRED=2.4.89
LIBDRM_VC4_REQUIRED=2.4.89
+LIBDRM_CRT_SEQUENCE_REQUIRED=2.4.89
I agree with everyone else here: just bump the libdrm version required
for vulkan :)
Making sure I got it right:
Since it's vulkan only issue LIBDRM_REQUIRED=2.4.75 should be left
intact and
new variable like LIBDRM_VULKAN_REQUIRED=2.4.89 be introduced
which will be checked when vulkan is enabled.
libdrm is absolutely trivial to update; any system that can update its
mesa can also update its libdrm.
Didn't notice that it's easy to update, thanks.
+
dnl Versions for external dependencies
DRI2PROTO_REQUIRED=2.8
GLPROTO_REQUIRED=1.4.14
@@ -97,6 +99,7 @@ XCBDRI2_REQUIRED=1.8
XCBDRI3_MODIFIERS_REQUIRED=1.13
XCBGLX_REQUIRED=1.8.1
XCBPRESENT_MODIFIERS_REQUIRED=1.13
+XCBRANDR_XLEASE_REQUIRED=1.13
XDAMAGE_REQUIRED=1.1
XSHMFENCE_REQUIRED=1.1
XVMC_REQUIRED=1.0.6
@@ -1874,20 +1877,6 @@ if test x"$enable_dri3" = xyes; then
fi
fi
-
-if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then
-have_xlease=yes
-else
-have_xlease=no
-fi
-
-if test x"$have_xlease" = xyes; then
-randr_modules="x11-xcb xcb-randr"
-PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules])
-xlib_randr_modules="xrandr"
-PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules])
-fi
-
AM_CONDITIONAL(HAVE_PLATFORM_X11, echo "$platforms" | grep -q 'x11')
AM_CONDITIONAL(HAVE_PLATFORM_WAYLAND, echo "$platforms" | grep -q 'wayland')
AM_CONDITIONAL(HAVE_PLATFORM_DRM, echo "$platforms" | grep -q 'drm')
@@ -1905,14 +1894,24 @@ xno)
;;
*)
if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q
'drm'; then
-enable_xlib_lease=yes
+xlease_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED xrandr"
+PKG_CHECK_EXISTS([$xlease_modules], [enable_xlib_lease=yes],
[enable_xlib_lease=no])
else
enable_xlib_lease=no
fi
esac
+if test x"$enable_xlib_lease" = xyes; then
+randr_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED"
+PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules])
+xlib_randr_modules="xrandr"
+PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules])
+fi
+
AM_CONDITIONAL(HAVE_XLIB_LEASE, test "x$enable_xlib_lease" = xyes)
+PKG_CHECK_EXISTS([libdrm >= $LIBDRM_CRT_SEQUENCE_REQUIRED], [DEFINES="${DEFINES} -DVK_USE_DISPLAY_CONTROL"], [])
+
dnl
dnl More DRI setup
dnl
diff --git a/meson.build b/meson.build
index 7d12af3d51..902074819c 100644
--- a/meson.build
+++ b/meson.build
@@ -1088,6 +1088,8 @@ _drm_freedreno_ver = '2.4.92'
_drm_intel_ver = '2.4.75'
_drm_ver = '2.4.75'
+_drm_crt_sequence_ver = '2.4.89'
+
_libdrm_checks = [
['intel', with_dri_i915 or with_gallium_i915],
['amdgpu', with_amd_vk or
Re: [Mesa-dev] [PATCH v3] vulkan: Fix compilation on older platforms
Hi Emil, On 24.07.18 19:23, Emil Velikov wrote: Hi Danylo, Having a closer look inspired by Eric's comments On 24 July 2018 at 15:37, Danylo Piliaiev wrote: Check for DRM_EVENT_CONTEXT_VERSION >= 4 to use sequence_handler. As you bump the libdrm version: - drop the existing DRM_EVENT_CONTEXT_VERSION guard - swap the DRM_EVENT_CONTEXT_VERSION macro with the actual version implemented Makes sense, will be done. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107170 https://bugs.freedesktop.org/show_bug.cgi?id=106972 https://bugs.freedesktop.org/show_bug.cgi?id=107176 Separate bugs it seems - hence separate patches? Yes +LIBDRM_CRT_SEQUENCE_REQUIRED=2.4.89 + There is no need for yet another version - just bump LIBDRM_REQUIRED. Bumping LIBDRM_REQUIRED would mean that older platforms won't be supported. I don't know how the decisions about support of older platforms are made but from my point of view bumping required libdrm version due to one vulkan extension may be overkill. If we consider this as the only merit, then the number of checks/version would practically explore. Simply take each user libGL, libEGL, gbm, other multiply that with the number of features we use. Looking at Eero's bug (last one in the list) - he's using Ubuntu 16.04 which has 2.4.91 in xenial-updates I didn't notice that libdrm is easy to update thus my argument fails. Managing dependencies is hard... Why do we need the new define? From a quick look we should be above to reuse VK_USE_PLATFORM_DISPLAY_KHR, we simply need the ifdef guards. If we will not bump global libdrm version requirement new define is necessary because rest of the code guarded by VK_USE_PLATFORM_DISPLAY_KHR works fine with older libdrm versions. And by addressing that, this can go - one diverging codepath less ;-) For good. HTH Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3] vulkan: Fix compilation on older platforms
Hi Emil,
On 24.07.18 17:10, Emil Velikov wrote:
Hi Danylo,
On 13 July 2018 at 14:57, Danylo Piliaiev wrote:
Make xlease automatically enabled only if xcb-randr >= 1.13,
check its version if manually enabled.
Enable VK_EXT_display_control only when libdrm >= 2.4.89
Check for DRM_EVENT_CONTEXT_VERSION >= 4 to use sequence_handler.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107170
https://bugs.freedesktop.org/show_bug.cgi?id=106972
https://bugs.freedesktop.org/show_bug.cgi?id=107176
v2: - Add 'protect="VK_USE_DISPLAY_CONTROL"' attribute to
VK_EXT_display_control in vk.xml
- Add support for 'protect' attribute to anv_entrypoints_gen
(copied from radv_entrypoints_gen)
- Turn #if into #ifdef
- Remove unnecessary pkg-config call from meson build (Dylan Baker)
v3: by Dylan Baker
- Remove previously added changes to vk.xml and entrypoints
generation because vk.xml is meant to be pulled from the external
source.
Signed-off-by: Danylo Piliaiev
---
configure.ac | 29 +-
meson.build| 10 -
src/amd/vulkan/radv_extensions.py | 9 +++-
src/amd/vulkan/radv_wsi_display.c | 5 ++---
src/intel/vulkan/anv_extensions.py | 2 +-
src/intel/vulkan/anv_extensions_gen.py | 7 +++
src/intel/vulkan/anv_wsi_display.c | 4 ++--
src/vulkan/wsi/wsi_common_display.c| 8 +--
src/vulkan/wsi/wsi_common_display.h| 3 ++-
9 files changed, 51 insertions(+), 26 deletions(-)
diff --git a/configure.ac b/configure.ac
index f135d05736..0b04525014 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,8 @@ LIBDRM_FREEDRENO_REQUIRED=2.4.92
LIBDRM_ETNAVIV_REQUIRED=2.4.89
LIBDRM_VC4_REQUIRED=2.4.89
+LIBDRM_CRT_SEQUENCE_REQUIRED=2.4.89
+
There is no need for yet another version - just bump LIBDRM_REQUIRED.
Bumping LIBDRM_REQUIRED would mean that older platforms won't be supported.
I don't know how the decisions about support of older platforms are made
but
from my point of view bumping required libdrm version due to one vulkan
extension
may be overkill.
dnl Versions for external dependencies
DRI2PROTO_REQUIRED=2.8
GLPROTO_REQUIRED=1.4.14
@@ -97,6 +99,7 @@ XCBDRI2_REQUIRED=1.8
XCBDRI3_MODIFIERS_REQUIRED=1.13
XCBGLX_REQUIRED=1.8.1
XCBPRESENT_MODIFIERS_REQUIRED=1.13
+XCBRANDR_XLEASE_REQUIRED=1.13
Thinking out loud: I wonder if we cannot use a single variable for all
the xcb 1.13 bits.
XDAMAGE_REQUIRED=1.1
XSHMFENCE_REQUIRED=1.1
XVMC_REQUIRED=1.0.6
@@ -1874,20 +1877,6 @@ if test x"$enable_dri3" = xyes; then
fi
fi
-
-if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then
-have_xlease=yes
-else
-have_xlease=no
-fi
-
-if test x"$have_xlease" = xyes; then
-randr_modules="x11-xcb xcb-randr"
-PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules])
-xlib_randr_modules="xrandr"
-PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules])
-fi
-
AM_CONDITIONAL(HAVE_PLATFORM_X11, echo "$platforms" | grep -q 'x11')
AM_CONDITIONAL(HAVE_PLATFORM_WAYLAND, echo "$platforms" | grep -q 'wayland')
AM_CONDITIONAL(HAVE_PLATFORM_DRM, echo "$platforms" | grep -q 'drm')
@@ -1905,14 +1894,24 @@ xno)
;;
*)
if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q
'drm'; then
-enable_xlib_lease=yes
+xlease_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED xrandr"
+PKG_CHECK_EXISTS([$xlease_modules], [enable_xlib_lease=yes],
[enable_xlib_lease=no])
else
enable_xlib_lease=no
fi
esac
+if test x"$enable_xlib_lease" = xyes; then
+randr_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED"
+PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules])
+xlib_randr_modules="xrandr"
+PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules])
+fi
+
AM_CONDITIONAL(HAVE_XLIB_LEASE, test "x$enable_xlib_lease" = xyes)
+PKG_CHECK_EXISTS([libdrm >= $LIBDRM_CRT_SEQUENCE_REQUIRED], [DEFINES="${DEFINES}
-DVK_USE_DISPLAY_CONTROL"], [])
+
Why do we need the new define? From a quick look we should be above to
reuse VK_USE_PLATFORM_DISPLAY_KHR, we simply need the ifdef guards.
If we will not bump global libdrm version requirement new define is
necessary because
rest of the code guarded by VK_USE_PLATFORM_DISPLAY_KHR works fine with
older libdrm versions.
The comments are more or less also applicable for meson.
HTH
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: Make invariant outputs in ES fragment shader not to cause error
In all GLSL ES versions output variables in fragment shader are allowed
to be invariant.
From Section 4.6.1 ("The Invariant Qualifier") GLSL ES 1.00 spec:
"Only the following variables may be declared as invariant:
...
- Built-in special variables output from the fragment shader."
From Section 4.6.1 ("The Invariant Qualifier") GLSL ES 3.00 spec:
"Only variables output from a shader can be candidates for invariance."
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107842
Signed-off-by: Danylo Piliaiev
---
src/compiler/glsl/ast_to_hir.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 5d3f10b682..21cf04ba99 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -2713,7 +2713,7 @@ is_allowed_invariant(ir_variable *var, struct
_mesa_glsl_parse_state *state)
* "Only variables output from a vertex shader can be candidates
* for invariance".
*/
- if (!state->is_version(130, 0))
+ if (!state->is_version(130, 100))
return false;
/*
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] i965: Fix calculation of layers array length for isl_view
Comment for array_len field states:
"Indicates the number of array elements starting at
Base Array Layer."
And most usages of array_len expect it to be equal or less than
total layers - base layer
v2: Do not subtract base layer if image unit is not layered (Ilia Mirkin)
Fixes: 5a8c8903
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
Signed-off-by: Danylo Piliaiev
---
src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 42af41aca3..54f62f0396 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1539,14 +1539,16 @@ update_image_surface(struct brw_context *brw,
} else {
struct intel_texture_object *intel_obj = intel_texture_object(obj);
struct intel_mipmap_tree *mt = intel_obj->mt;
+
+ const unsigned base_layer = obj->MinLayer + u->_Layer;
const unsigned num_layers = u->Layered ?
-get_image_num_layers(mt, obj->Target, u->Level) : 1;
+(get_image_num_layers(mt, obj->Target, u->Level) - base_layer) : 1;
struct isl_view view = {
.format = format,
.base_level = obj->MinLevel + u->Level,
.levels = 1,
-.base_array_layer = obj->MinLayer + u->_Layer,
+.base_array_layer = base_layer,
.array_len = num_layers,
.swizzle = ISL_SWIZZLE_IDENTITY,
.usage = ISL_SURF_USAGE_STORAGE_BIT,
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Fix calculation of layers array length for isl_view
On 9/7/18 5:48 PM, Ilia Mirkin wrote:
On Fri, Sep 7, 2018 at 10:41 AM, Danylo Piliaiev
wrote:
Comment for array_len field states:
"Indicates the number of array elements starting at
Base Array Layer."
And most usages of array_len expect it to be equal or less than
total layers - base layer
Fixes: 5a8c8903
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
Signed-off-by: Danylo Piliaiev
---
src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 42af41aca3..6adf4a5836 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1539,6 +1539,8 @@ update_image_surface(struct brw_context *brw,
} else {
struct intel_texture_object *intel_obj = intel_texture_object(obj);
struct intel_mipmap_tree *mt = intel_obj->mt;
+
+ const unsigned base_layer = obj->MinLayer + u->_Layer;
const unsigned num_layers = u->Layered ?
get_image_num_layers(mt, obj->Target, u->Level) : 1;
@@ -1546,8 +1548,8 @@ update_image_surface(struct brw_context *brw,
.format = format,
.base_level = obj->MinLevel + u->Level,
.levels = 1,
-.base_array_layer = obj->MinLayer + u->_Layer,
-.array_len = num_layers,
+.base_array_layer = base_layer,
+.array_len = num_layers - base_layer,
See above - num_layers can be 1 if the image isn't bound as a layered
image. But base layer can be whatever -- so this will end up as
negative. I think the adjustment needs to be done only for the
u->Layered case.
Oh, I see it now, thanks! Unless Lionel's patch for this issue is better
I'll send v2 of my patch.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Fix calculation of layers array length for isl_view
Comment for array_len field states:
"Indicates the number of array elements starting at
Base Array Layer."
And most usages of array_len expect it to be equal or less than
total layers - base layer
Fixes: 5a8c8903
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
Signed-off-by: Danylo Piliaiev
---
src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 42af41aca3..6adf4a5836 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1539,6 +1539,8 @@ update_image_surface(struct brw_context *brw,
} else {
struct intel_texture_object *intel_obj = intel_texture_object(obj);
struct intel_mipmap_tree *mt = intel_obj->mt;
+
+ const unsigned base_layer = obj->MinLayer + u->_Layer;
const unsigned num_layers = u->Layered ?
get_image_num_layers(mt, obj->Target, u->Level) : 1;
@@ -1546,8 +1548,8 @@ update_image_surface(struct brw_context *brw,
.format = format,
.base_level = obj->MinLevel + u->Level,
.levels = 1,
-.base_array_layer = obj->MinLayer + u->_Layer,
-.array_len = num_layers,
+.base_array_layer = base_layer,
+.array_len = num_layers - base_layer,
.swizzle = ISL_SWIZZLE_IDENTITY,
.usage = ISL_SURF_USAGE_STORAGE_BIT,
};
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: add missing return statement for GL_RG_SNORM case
Signed-off-by: Danylo Piliaiev --- src/mesa/main/fbobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c index 51e137dce9..edb86438e3 100644 --- a/src/mesa/main/fbobject.c +++ b/src/mesa/main/fbobject.c @@ -2080,7 +2080,7 @@ _mesa_base_fbo_format(const struct gl_context *ctx, GLenum internalFormat) return _mesa_has_EXT_texture_snorm(ctx) || _mesa_has_EXT_render_snorm(ctx) ? GL_RG : 0; case GL_RG_SNORM: - _mesa_has_EXT_texture_snorm(ctx) ? GL_RG : 0; + return _mesa_has_EXT_texture_snorm(ctx) ? GL_RG : 0; case GL_RG16_SNORM: return _mesa_has_EXT_texture_snorm(ctx) || (_mesa_has_EXT_render_snorm(ctx) && -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Fix calculation of layers array length for isl_view
Thank you Józef and Ilia for pointing this out! I believe I have fully understood what's happening here and I'll send a final version of the fix soon. On 9/7/18 7:47 PM, Józef Kucia wrote: On Fri, Sep 7, 2018 at 6:44 PM Ilia Mirkin wrote: On Fri, Sep 7, 2018 at 12:35 PM, Józef Kucia wrote: On Fri, Sep 7, 2018 at 4:42 PM Danylo Piliaiev wrote: @@ -1546,8 +1548,8 @@ update_image_surface(struct brw_context *brw, .format = format, .base_level = obj->MinLevel + u->Level, .levels = 1, -.base_array_layer = obj->MinLayer + u->_Layer, -.array_len = num_layers, +.base_array_layer = base_layer, +.array_len = num_layers - base_layer, .swizzle = ISL_SWIZZLE_IDENTITY, .usage = ISL_SURF_USAGE_STORAGE_BIT, }; This sets the "array_len" to the number of layers remaining in the original texture. Shouldn't it take into account the number of layers in the GL texture view? Errr, right. Here is the logic in st/mesa, which I believe is correct. (But convoluted. Because there are so many bits to it.) https://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/state_tracker/st_atom_image.c#n101 There is also view_num_layers in brw_update_texture_surface(). It could be adapted. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3] i965: Fix calculation of layers array length for isl_view
Handle all cases in calculation of layers count for isl_view
taking into account texture view and image unit.
st_convert_image was taken as a reference.
When u->Layered is true the whole level is taken with respect to
image view. In other case only one layer is taken.
v3: (Józef Kucia and Ilia Mirkin)
- Rewrote patch by taking st_convert_image as a reference
- Removed now unused get_image_num_layers function
- Changed commit message
Fixes: 5a8c8903
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
Signed-off-by: Danylo Piliaiev
---
.../drivers/dri/i965/brw_wm_surface_state.c | 32 ++-
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 944762ec46..9bfe6e2037 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1499,18 +1499,6 @@ update_buffer_image_param(struct brw_context *brw,
param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
}
-static unsigned
-get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum target,
- unsigned level)
-{
- if (target == GL_TEXTURE_CUBE_MAP)
- return 6;
-
- return target == GL_TEXTURE_3D ?
- minify(mt->surf.logical_level0_px.depth, level) :
- mt->surf.logical_level0_px.array_len;
-}
-
static void
update_image_surface(struct brw_context *brw,
struct gl_image_unit *u,
@@ -1541,14 +1529,28 @@ update_image_surface(struct brw_context *brw,
} else {
struct intel_texture_object *intel_obj = intel_texture_object(obj);
struct intel_mipmap_tree *mt = intel_obj->mt;
- const unsigned num_layers = u->Layered ?
-get_image_num_layers(mt, obj->Target, u->Level) : 1;
+
+ unsigned base_layer, num_layers;
+ if (u->Layered) {
+if (obj->Target == GL_TEXTURE_3D) {
+ base_layer = 0;
+ num_layers = minify(mt->surf.logical_level0_px.depth, u->Level);
+} else {
+ base_layer = obj->MinLayer;
+ num_layers = obj->Immutable ?
+obj->NumLayers :
+mt->surf.logical_level0_px.array_len;
+}
+ } else {
+base_layer = obj->MinLayer + u->_Layer;
+num_layers = 1;
+ }
struct isl_view view = {
.format = format,
.base_level = obj->MinLevel + u->Level,
.levels = 1,
-.base_array_layer = obj->MinLayer + u->_Layer,
+.base_array_layer = base_layer,
.array_len = num_layers,
.swizzle = ISL_SWIZZLE_IDENTITY,
.usage = ISL_SURF_USAGE_STORAGE_BIT,
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: Cross validate variable's invariance by explicit invariance only
'invariant' qualifier is propagated on variables which are used
to calculate other invariant variables, however when we are matching
variable's declarations we should take into account only explicitly
declared invariance because invariance propagation is an implementation
specific detail.
Thus new flag is added to ir_variable_data which indicates 'invariant'
qualifier being explicitly set in the shader.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100316
Signed-off-by: Danylo Piliaiev
---
I'll send additional piglit test to test fixed case with invariance
propagation when there are several shaders in one stage.
At the moment there is a piglit test which tests other case fixed
by this patch:
"[PATCH] glsl-1.20: test that 'invariant' qualifier does not propagate on
uniforms"
src/compiler/glsl/ast_to_hir.cpp| 8 ++--
src/compiler/glsl/ir.cpp| 1 +
src/compiler/glsl/ir.h | 13 +
src/compiler/glsl/ir_reader.cpp | 3 ++-
src/compiler/glsl/link_varyings.cpp | 6 +++---
src/compiler/glsl/linker.cpp| 2 +-
6 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 5d3f10b682..a535707449 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3932,7 +3932,8 @@ apply_type_qualifier_to_variable(const struct
ast_type_qualifier *qual,
"`invariant' after being used",
var->name);
} else {
- var->data.invariant = 1;
+ var->data.explicit_invariant = true;
+ var->data.invariant = true;
}
}
@@ -4140,8 +4141,10 @@ apply_type_qualifier_to_variable(const struct
ast_type_qualifier *qual,
}
}
- if (state->all_invariant && var->data.mode == ir_var_shader_out)
+ if (state->all_invariant && var->data.mode == ir_var_shader_out) {
+ var->data.explicit_invariant = true;
var->data.invariant = true;
+ }
var->data.interpolation =
interpret_interpolation_qualifier(qual, var->type,
@@ -4852,6 +4855,7 @@ ast_declarator_list::hir(exec_list *instructions,
"`invariant' after being used",
earlier->name);
} else {
+earlier->data.explicit_invariant = true;
earlier->data.invariant = true;
}
}
diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
index 1d1a56ae9a..f5aa1be4e2 100644
--- a/src/compiler/glsl/ir.cpp
+++ b/src/compiler/glsl/ir.cpp
@@ -1734,6 +1734,7 @@ ir_variable::ir_variable(const struct glsl_type *type,
const char *name,
this->data.centroid = false;
this->data.sample = false;
this->data.patch = false;
+ this->data.explicit_invariant = false;
this->data.invariant = false;
this->data.how_declared = ir_var_declared_normally;
this->data.mode = mode;
diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
index f478b29a6b..7e4b500b66 100644
--- a/src/compiler/glsl/ir.h
+++ b/src/compiler/glsl/ir.h
@@ -657,6 +657,19 @@ public:
unsigned centroid:1;
unsigned sample:1;
unsigned patch:1;
+ /**
+ * Was an 'invariant' qualifier explicitly set in the shader?
+ *
+ * This is used to cross validate qualifiers.
+ */
+ unsigned explicit_invariant:1;
+ /**
+ * Is the variable invariant?
+ *
+ * It can happen either by having the 'invariant' qualifier
+ * explicitly set in the shader or by being used in calculations
+ * of other invariant variables.
+ */
unsigned invariant:1;
unsigned precise:1;
diff --git a/src/compiler/glsl/ir_reader.cpp b/src/compiler/glsl/ir_reader.cpp
index b87933ba51..70677c5215 100644
--- a/src/compiler/glsl/ir_reader.cpp
+++ b/src/compiler/glsl/ir_reader.cpp
@@ -420,7 +420,8 @@ ir_reader::read_declaration(s_expression *expr)
} else if (strcmp(qualifier->value(), "patch") == 0) {
var->data.patch = 1;
} else if (strcmp(qualifier->value(), "invariant") == 0) {
-var->data.invariant = 1;
+var->data.explicit_invariant = true;
+var->data.invariant = true;
} else if (strcmp(qualifier->value(), "uniform") == 0) {
var->data.mode = ir_var_uniform;
} else if (strcmp(qualifier->value(), "shader_storage") == 0) {
diff --git a/src/compiler/glsl/link_varyings.cpp
b/src/compiler/glsl/link_varyings.cpp
index 211633d9ee..28bb2ca0e3 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -308,16 +308,16 @@ cross_validate_types_and_qualifiers(struct gl_context
*ctx,
*"The invariance of varyings that are declared in both the vertex
* and fragment
Re: [Mesa-dev] [PATCH] glsl: Do not propagate 'precise' and 'invariant' on read-only variables
Hi,
I went ahead and sent the patch which implements my proposed solution:
"[PATCH] glsl: Cross validate variable's invariance by explicit
invariance only".
In relation to the new patch this one becomes cosmetic since while the
new patch solves linking errors, it doesn't remove invariant qualifier
from uniforms.
- Danil
On 8/23/18 2:49 PM, Danylo Piliaiev wrote:
Hi Jason,
On 8/22/18 7:07 PM, Jason Ekstrand wrote:
I think this would be correct to do and would fix the bug at hand but
there may be other bugs lurking deeper. In particular, if you have
multiple shaders for the same stage that share some global variables
and get linked together, I think you could run into the same issue
only with a read-write global. If this is indeed possible, then I'm
not sure what to do. We have to run this pass prior to doing any
optimization because we depend on it to get precise and invariant
correct. On the other hand, it has to be run after linking. :-/
Maybe we just have to assume precise and invariant for all
pre-linking optimizations and then run this post-linking.
I didn't check this case, thanks for pointing it out. Now I have
looked at relevant code and saw that invariant propagation
(linker.cpp:5024 in linker_optimisation_loop) is done after
'link_intrastage_shaders' (linker.cpp:4868) which combines a group of
shaders for a single stage. Actually invariant propagation is also
done once before linking. In addition I've done some tests and
invariance from variable in one vertex shader is propagated on
variables directly used only in other vertex shader.
However I have found other issue - cross_validate_globals is called in
link_intrastage_shaders and throws error if 'invariant' qualifier
doesn't match between shaders in same stage which happens when
variable in one shader becomes invariant before the
'link_intrastage_shaders' call. This could be fixed separately.
Or there is another way to solve these issues: divide 'invariant'
qualifier into explicit and implicit 'invariant', where explicit is
for 'invariant' set by user in shader and implicit is for 'invariant'
propagated from other invariants. Thus in interface checking we will
be able to check only for explicit invariance which besides two
previous issues with solve another one: if out variable is marked as
invariant due to invariance propagation this variable will be required
to have invariant qualifier in the next stage even though user never
marked it as such anywhere. And when the invariance will actually
matter we will check both implicit and explicit one. What do you think
about such solution?
- Danil.
On Wed, Aug 22, 2018 at 7:51 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>> wrote:
Read-only variables could be considered inherently invariant and
precise
since no expression in shader affects them.
Explicitly marking them as such is unnecessary and can cause issues,
e.g. uniform marked as invariant may require the same uniform in
other
stage to be invariant too but uniforms cannot have "invariant"
qualifier.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100316
Signed-off-by: Danylo Piliaiev mailto:danylo.pilia...@globallogic.com>>
---
My assumption is that variable being read_only means that it either
loads a value from an external source or is initialized from
constant value.
From what I saw in code it's true but it may be wrong to depend
on it.
src/compiler/glsl/propagate_invariance.cpp | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/compiler/glsl/propagate_invariance.cpp
b/src/compiler/glsl/propagate_invariance.cpp
index b3f1d810cd..0e4233aa9f 100644
--- a/src/compiler/glsl/propagate_invariance.cpp
+++ b/src/compiler/glsl/propagate_invariance.cpp
@@ -96,6 +96,16 @@
ir_invariance_propagation_visitor::visit(ir_dereference_variable *ir)
if (this->dst_var == NULL)
return visit_continue;
+ /* Read-only variables could be considered inherently
+ * invariant and precise since no expression in shader
affects them.
+ * Explicitly marking them as such is unnecessary and can cause
+ * issues, e.g. uniform marked as invariant may require the same
+ * uniform in other stage to be invariant too but uniforms cannot
+ * have "invariant" qualifier.
+ */
+ if (ir->var->data.read_only)
+ return visit_continue;
+
if (this->dst_var->data.invariant) {
if (!ir->var->data.invariant)
this->progress = true;
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/3] Sweep NIR after linking phase to free held memory
This patch series is a draft changes to reduce memory usage when compiling shaders and add utility function to calculate a size of memory contexts. The issue I'm attempting to fix described in the bug: https://bugs.freedesktop.org/show_bug.cgi?id=103274 In short some 32bit games under Wine use a lot of complex shaders compiling which leads to huge memory usage and sometimes to crashes due to it. The changes themselves are trivial but have some price: After the changes shader with average complexity may link ~3-5% slower. The savings are huge when compiling a lot of shaders: In my test case it was around 1mb per shader where the size of nir after sweep was 20-40kb. Since the changes slightly slow the compilation and are useful for small amount of cases I believe that it may be better to introduce new environment variable to control these sweeps. I would like to get advice whether this change is desirable and should it be hidden behind environment variable (if yes suggestions about its name are welcomed). Also I wasn't able to test the change in st_link_nir and will welcome some help with this. Danylo Piliaiev (3): i965: Sweep NIR after linking phase to free held memory st: Sweep NIR after linking phase to free held memory ralloc: Add ralloc_calc_size to measure memory held by memory contexts src/mesa/drivers/dri/i965/brw_link.cpp| 9 +++ src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 ++ src/util/ralloc.c | 30 +++ src/util/ralloc.h | 5 4 files changed, 46 insertions(+) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] i965: Sweep NIR after linking phase to free held memory
After optimization passes and many trasfromations most of memory
NIR holds is a garbage which was being freed only after shader deletion.
Freeing it at the end of linking will save memory which would be useful
in case there are a lot of complex shaders being compiled.
The common case for this issue is 32bit game running under Wine.
The cost of the optimization is around ~3-5% of compilation speed
with complex shaders.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103274
Signed-off-by: Danylo Piliaiev
---
src/mesa/drivers/dri/i965/brw_link.cpp | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp
b/src/mesa/drivers/dri/i965/brw_link.cpp
index 1071056f14..8f34a58ce7 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -359,6 +359,15 @@ brw_link_shader(struct gl_context *ctx, struct
gl_shader_program *shProg)
}
}
+ for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders); stage++) {
+ struct gl_linked_shader *shader = shProg->_LinkedShaders[stage];
+ if (!shader)
+ continue;
+
+ struct gl_program *prog = shader->Program;
+ nir_sweep(prog->nir);
+ }
+
if (brw->ctx.Cache) {
for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders); stage++) {
struct gl_linked_shader *shader = shProg->_LinkedShaders[stage];
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] st: Sweep NIR after linking phase to free held memory
After optimization passes and many trasfromations most of memory NIR holds is a garbage which was being freed only after shader deletion. Freeing it at the end of linking will save memory which would be useful in case there are a lot of complex shaders being compiled. The common case for this issue is 32bit game running under Wine. The cost of the optimization is around ~3-5% of compilation speed with complex shaders. Signed-off-by: Danylo Piliaiev --- src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp index 1985fe1d53..ae2c49960c 100644 --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp @@ -773,6 +773,8 @@ st_link_nir(struct gl_context *ctx, _mesa_reference_program(ctx, >Program, NULL); return false; } + + nir_sweep(shader->Program->nir); } return true; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] ralloc: Add ralloc_calc_size to measure memory held by memory contexts
The function recursively calculates memory held by context's children.
It is aimed to easier debugging of excessive memory usage with ralloc.
It will work only if malloc_usable_size is available.
Signed-off-by: Danylo Piliaiev
---
src/util/ralloc.c | 30 ++
src/util/ralloc.h | 5 +
2 files changed, 35 insertions(+)
diff --git a/src/util/ralloc.c b/src/util/ralloc.c
index 5d77f75ee8..c8d217f6a3 100644
--- a/src/util/ralloc.c
+++ b/src/util/ralloc.c
@@ -28,6 +28,10 @@
#include
#include
+#ifdef __GLIBC__
+#include
+#endif
+
/* Some versions of MinGW are missing _vscprintf's declaration, although they
* still provide the symbol in the import library. */
#ifdef __MINGW32__
@@ -239,6 +243,32 @@ ralloc_free(void *ptr)
unsafe_free(info);
}
+size_t
+ralloc_calc_size(void *ctx)
+{
+#ifdef __GLIBC__
+ ralloc_header *info;
+
+ if (ctx == NULL)
+ return 0;
+
+ info = get_header(ctx);
+
+ size_t total_size = malloc_usable_size(info);
+
+ /* Recursively calculate children size */
+ ralloc_header *temp = info->child;
+ while (temp != NULL) {
+ total_size += ralloc_calc_size(PTR_FROM_HEADER(temp));
+ temp = temp->next;
+ }
+
+ return total_size;
+#else
+ return 0;
+#endif
+}
+
static void
unlink_block(ralloc_header *info)
{
diff --git a/src/util/ralloc.h b/src/util/ralloc.h
index 05ae8f8407..fecd31bdca 100644
--- a/src/util/ralloc.h
+++ b/src/util/ralloc.h
@@ -219,6 +219,11 @@ void *reralloc_array_size(const void *ctx, void *ptr,
size_t size,
unsigned count);
/// @}
+/**
+ * Calculate total memory size held by the context
+ */
+size_t ralloc_calc_size(void *ctx);
+
/**
* Free a piece of ralloc-managed memory.
*
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] i965: Sweep NIR after linking phase to free held memory
Indeed it would be better to move it, will be done.
On 10.07.18 18:04, Jason Ekstrand wrote:
I think we can just move it up a couple of loops to the place where we
do the final lowering and call gather_info. Other than that, this
seems like a good idea to me. NIR passes are pretty sloppy about
memory and assume nir_sweep gets called. Hanging on to linked shaders
with all that garbage floating around is a bad plan.
On Tue, Jul 10, 2018 at 1:53 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>> wrote:
After optimization passes and many trasfromations most of memory
NIR holds is a garbage which was being freed only after shader
deletion.
Freeing it at the end of linking will save memory which would be
useful
in case there are a lot of complex shaders being compiled.
The common case for this issue is 32bit game running under Wine.
The cost of the optimization is around ~3-5% of compilation speed
with complex shaders.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103274
Signed-off-by: Danylo Piliaiev mailto:danylo.pilia...@globallogic.com>>
---
src/mesa/drivers/dri/i965/brw_link.cpp | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp
b/src/mesa/drivers/dri/i965/brw_link.cpp
index 1071056f14..8f34a58ce7 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -359,6 +359,15 @@ brw_link_shader(struct gl_context *ctx,
struct gl_shader_program *shProg)
}
}
+ for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders);
stage++) {
+ struct gl_linked_shader *shader =
shProg->_LinkedShaders[stage];
+ if (!shader)
+ continue;
+
+ struct gl_program *prog = shader->Program;
+ nir_sweep(prog->nir);
+ }
+
if (brw->ctx.Cache) {
for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders);
stage++) {
struct gl_linked_shader *shader =
shProg->_LinkedShaders[stage];
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 1/3] i965: Sweep NIR after linking phase to free held memory
After optimization passes and many trasfromations most of memory NIR holds is a garbage which was being freed only after shader deletion. Freeing it at the end of linking will save memory which would be useful in case there are a lot of complex shaders being compiled. The common case for this issue is 32bit game running under Wine. The cost of the optimization is around ~3-5% of compilation speed with complex shaders. V2: by Jason Ekstrand - Move nir_sweep up, right after the last change of NIR Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103274 Signed-off-by: Danylo Piliaiev --- src/mesa/drivers/dri/i965/brw_link.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp index 1071056f14..378426101b 100644 --- a/src/mesa/drivers/dri/i965/brw_link.cpp +++ b/src/mesa/drivers/dri/i965/brw_link.cpp @@ -317,6 +317,8 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg) NIR_PASS_V(prog->nir, nir_lower_atomics_to_ssbo, prog->nir->info.num_abos); + nir_sweep(prog->nir); + infos[stage] = >nir->info; update_xfb_info(prog->sh.LinkedTransformFeedback, infos[stage]); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] vulkan: Fix compilation on older platforms
Make xlease automatically enabled only if xcb-randr >= 1.13,
check its version if manually enabled.
Enable VK_EXT_display_control only when libdrm >= 2.4.89
Check for DRM_EVENT_CONTEXT_VERSION >= 4 to use sequence_handler.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107170
https://bugs.freedesktop.org/show_bug.cgi?id=106972
https://bugs.freedesktop.org/show_bug.cgi?id=107176
Signed-off-by: Danylo Piliaiev
---
configure.ac | 29 +-
meson.build| 11 +-
src/amd/vulkan/radv_extensions.py | 9 +++-
src/amd/vulkan/radv_wsi_display.c | 5 ++---
src/intel/vulkan/anv_extensions.py | 2 +-
src/intel/vulkan/anv_extensions_gen.py | 7 +++
src/intel/vulkan/anv_wsi_display.c | 4 ++--
src/vulkan/wsi/wsi_common_display.c| 8 +--
src/vulkan/wsi/wsi_common_display.h| 3 ++-
9 files changed, 52 insertions(+), 26 deletions(-)
diff --git a/configure.ac b/configure.ac
index f135d05736..0b04525014 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,8 @@ LIBDRM_FREEDRENO_REQUIRED=2.4.92
LIBDRM_ETNAVIV_REQUIRED=2.4.89
LIBDRM_VC4_REQUIRED=2.4.89
+LIBDRM_CRT_SEQUENCE_REQUIRED=2.4.89
+
dnl Versions for external dependencies
DRI2PROTO_REQUIRED=2.8
GLPROTO_REQUIRED=1.4.14
@@ -97,6 +99,7 @@ XCBDRI2_REQUIRED=1.8
XCBDRI3_MODIFIERS_REQUIRED=1.13
XCBGLX_REQUIRED=1.8.1
XCBPRESENT_MODIFIERS_REQUIRED=1.13
+XCBRANDR_XLEASE_REQUIRED=1.13
XDAMAGE_REQUIRED=1.1
XSHMFENCE_REQUIRED=1.1
XVMC_REQUIRED=1.0.6
@@ -1874,20 +1877,6 @@ if test x"$enable_dri3" = xyes; then
fi
fi
-
-if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then
-have_xlease=yes
-else
-have_xlease=no
-fi
-
-if test x"$have_xlease" = xyes; then
-randr_modules="x11-xcb xcb-randr"
-PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules])
-xlib_randr_modules="xrandr"
-PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules])
-fi
-
AM_CONDITIONAL(HAVE_PLATFORM_X11, echo "$platforms" | grep -q 'x11')
AM_CONDITIONAL(HAVE_PLATFORM_WAYLAND, echo "$platforms" | grep -q 'wayland')
AM_CONDITIONAL(HAVE_PLATFORM_DRM, echo "$platforms" | grep -q 'drm')
@@ -1905,14 +1894,24 @@ xno)
;;
*)
if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm';
then
-enable_xlib_lease=yes
+xlease_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED xrandr"
+PKG_CHECK_EXISTS([$xlease_modules], [enable_xlib_lease=yes],
[enable_xlib_lease=no])
else
enable_xlib_lease=no
fi
esac
+if test x"$enable_xlib_lease" = xyes; then
+randr_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED"
+PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules])
+xlib_randr_modules="xrandr"
+PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules])
+fi
+
AM_CONDITIONAL(HAVE_XLIB_LEASE, test "x$enable_xlib_lease" = xyes)
+PKG_CHECK_EXISTS([libdrm >= $LIBDRM_CRT_SEQUENCE_REQUIRED],
[DEFINES="${DEFINES} -DVK_USE_DISPLAY_CONTROL"], [])
+
dnl
dnl More DRI setup
dnl
diff --git a/meson.build b/meson.build
index 7d12af3d51..2683060827 100644
--- a/meson.build
+++ b/meson.build
@@ -1088,6 +1088,8 @@ _drm_freedreno_ver = '2.4.92'
_drm_intel_ver = '2.4.75'
_drm_ver = '2.4.75'
+_drm_crt_sequence_ver = '2.4.89'
+
_libdrm_checks = [
['intel', with_dri_i915 or with_gallium_i915],
['amdgpu', with_amd_vk or with_gallium_radeonsi],
@@ -1361,11 +1363,18 @@ if with_platform_x11
dep_xcb_xfixes = dependency('xcb-xfixes')
endif
if with_xlib_lease
-dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.12')
+dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.13')
dep_xlib_xrandr = dependency('xrandr', version : '>= 1.3')
endif
endif
+if with_any_vk
+ dep_drm_crt_sequence = dependency('libdrm', version : '>=' +
_drm_crt_sequence_ver, required : false)
+ if dep_drm_crt_sequence.found()
+pre_args += '-DVK_USE_DISPLAY_CONTROL'
+ endif
+endif
+
if get_option('gallium-extra-hud')
pre_args += '-DHAVE_GALLIUM_EXTRA_HUD=1'
endif
diff --git a/src/amd/vulkan/radv_extensions.py
b/src/amd/vulkan/radv_extensions.py
index c36559f48e..e60b0d4773 100644
--- a/src/amd/vulkan/radv_extensions.py
+++ b/src/amd/vulkan/radv_extensions.py
@@ -91,7 +91,7 @@ EXTENSIONS = [
Extension('VK_EXT_direct_mode_display', 1,
'VK_USE_PLATFORM_DISPLAY_KHR'),
Extension('VK_EXT_acquire_xlib_display', 1,
'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
Extension('VK_EXT_display_surface_counter', 1,
'VK_USE_PLATFORM_DISPLAY_KHR'),
-Extension('VK_EXT_display_control', 1,
'VK_USE_PLATFORM_DISPLAY_KHR'),
+Extension('VK_EXT_dis
Re: [Mesa-dev] [PATCH v2] vulkan: Fix compilation on older platforms
On 12.07.18 21:49, Dylan Baker wrote:
Quoting Danylo Piliaiev (2018-07-12 06:09:57)
Make xlease automatically enabled only if xcb-randr >= 1.13,
check its version if manually enabled.
Enable VK_EXT_display_control only when libdrm >= 2.4.89
Check for DRM_EVENT_CONTEXT_VERSION >= 4 to use sequence_handler.
Add support for 'protect' attribute to anv_entrypoints_gen.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107170
https://bugs.freedesktop.org/show_bug.cgi?id=106972
https://bugs.freedesktop.org/show_bug.cgi?id=107176
v2: - Add 'protect="VK_USE_DISPLAY_CONTROL"' attribute to
VK_EXT_display_control in vk.xml
- Add support for 'protect' attribute to anv_entrypoints_gen
(copied from radv_entrypoints_gen)
- Turn #if into #ifdef
- Remove unnecessary pkg-config call from meson build (Dylan Baker)
Signed-off-by: Danylo Piliaiev
---
Other vulkan extensions are gated behind platforms: wayland, xlib, ...
This one depends on library version and I hope I handled it right,
did I took the right approach?
Also what if extension requires both platform and libary version?
Should *_entrypoints_gen be able to support several defines per extension?
configure.ac| 29 -
meson.build | 10 -
src/amd/vulkan/radv_extensions.py | 9 +++-
src/amd/vulkan/radv_wsi_display.c | 5 ++---
src/intel/vulkan/anv_entrypoints_gen.py | 7 ++
src/intel/vulkan/anv_extensions.py | 2 +-
src/intel/vulkan/anv_extensions_gen.py | 7 ++
src/intel/vulkan/anv_wsi_display.c | 4 ++--
src/vulkan/registry/vk.xml | 2 +-
src/vulkan/wsi/wsi_common_display.c | 8 +--
src/vulkan/wsi/wsi_common_display.h | 3 ++-
11 files changed, 59 insertions(+), 27 deletions(-)
diff --git a/configure.ac b/configure.ac
index f135d05736..0b04525014 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,8 @@ LIBDRM_FREEDRENO_REQUIRED=2.4.92
LIBDRM_ETNAVIV_REQUIRED=2.4.89
LIBDRM_VC4_REQUIRED=2.4.89
+LIBDRM_CRT_SEQUENCE_REQUIRED=2.4.89
+
dnl Versions for external dependencies
DRI2PROTO_REQUIRED=2.8
GLPROTO_REQUIRED=1.4.14
@@ -97,6 +99,7 @@ XCBDRI2_REQUIRED=1.8
XCBDRI3_MODIFIERS_REQUIRED=1.13
XCBGLX_REQUIRED=1.8.1
XCBPRESENT_MODIFIERS_REQUIRED=1.13
+XCBRANDR_XLEASE_REQUIRED=1.13
XDAMAGE_REQUIRED=1.1
XSHMFENCE_REQUIRED=1.1
XVMC_REQUIRED=1.0.6
@@ -1874,20 +1877,6 @@ if test x"$enable_dri3" = xyes; then
fi
fi
-
-if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then
-have_xlease=yes
-else
-have_xlease=no
-fi
-
-if test x"$have_xlease" = xyes; then
-randr_modules="x11-xcb xcb-randr"
-PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules])
-xlib_randr_modules="xrandr"
-PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules])
-fi
-
AM_CONDITIONAL(HAVE_PLATFORM_X11, echo "$platforms" | grep -q 'x11')
AM_CONDITIONAL(HAVE_PLATFORM_WAYLAND, echo "$platforms" | grep -q 'wayland')
AM_CONDITIONAL(HAVE_PLATFORM_DRM, echo "$platforms" | grep -q 'drm')
@@ -1905,14 +1894,24 @@ xno)
;;
*)
if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q
'drm'; then
-enable_xlib_lease=yes
+xlease_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED xrandr"
+PKG_CHECK_EXISTS([$xlease_modules], [enable_xlib_lease=yes],
[enable_xlib_lease=no])
else
enable_xlib_lease=no
fi
esac
+if test x"$enable_xlib_lease" = xyes; then
+randr_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED"
+PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules])
+xlib_randr_modules="xrandr"
+PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules])
+fi
+
AM_CONDITIONAL(HAVE_XLIB_LEASE, test "x$enable_xlib_lease" = xyes)
+PKG_CHECK_EXISTS([libdrm >= $LIBDRM_CRT_SEQUENCE_REQUIRED], [DEFINES="${DEFINES} -DVK_USE_DISPLAY_CONTROL"], [])
+
dnl
dnl More DRI setup
dnl
diff --git a/meson.build b/meson.build
index 7d12af3d51..902074819c 100644
--- a/meson.build
+++ b/meson.build
@@ -1088,6 +1088,8 @@ _drm_freedreno_ver = '2.4.92'
_drm_intel_ver = '2.4.75'
_drm_ver = '2.4.75'
+_drm_crt_sequence_ver = '2.4.89'
+
_libdrm_checks = [
['intel', with_dri_i915 or with_gallium_i915],
['amdgpu', with_amd_vk or with_gallium_radeonsi],
@@ -1361,11 +1363,17 @@ if with_platform_x11
dep_xcb_xfixes = dependency('xcb-xfixes')
endif
if with_xlib_lease
-dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.12')
+dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.13')
dep_xlib_xrandr = dependency('xrandr', version : '>= 1.3')
endif
endif
+if with_any_vk
+
[Mesa-dev] [PATCH v3] vulkan: Fix compilation on older platforms
Make xlease automatically enabled only if xcb-randr >= 1.13,
check its version if manually enabled.
Enable VK_EXT_display_control only when libdrm >= 2.4.89
Check for DRM_EVENT_CONTEXT_VERSION >= 4 to use sequence_handler.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107170
https://bugs.freedesktop.org/show_bug.cgi?id=106972
https://bugs.freedesktop.org/show_bug.cgi?id=107176
v2: - Add 'protect="VK_USE_DISPLAY_CONTROL"' attribute to
VK_EXT_display_control in vk.xml
- Add support for 'protect' attribute to anv_entrypoints_gen
(copied from radv_entrypoints_gen)
- Turn #if into #ifdef
- Remove unnecessary pkg-config call from meson build (Dylan Baker)
v3: by Dylan Baker
- Remove previously added changes to vk.xml and entrypoints
generation because vk.xml is meant to be pulled from the external
source.
Signed-off-by: Danylo Piliaiev
---
configure.ac | 29 +-
meson.build| 10 -
src/amd/vulkan/radv_extensions.py | 9 +++-
src/amd/vulkan/radv_wsi_display.c | 5 ++---
src/intel/vulkan/anv_extensions.py | 2 +-
src/intel/vulkan/anv_extensions_gen.py | 7 +++
src/intel/vulkan/anv_wsi_display.c | 4 ++--
src/vulkan/wsi/wsi_common_display.c| 8 +--
src/vulkan/wsi/wsi_common_display.h| 3 ++-
9 files changed, 51 insertions(+), 26 deletions(-)
diff --git a/configure.ac b/configure.ac
index f135d05736..0b04525014 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,8 @@ LIBDRM_FREEDRENO_REQUIRED=2.4.92
LIBDRM_ETNAVIV_REQUIRED=2.4.89
LIBDRM_VC4_REQUIRED=2.4.89
+LIBDRM_CRT_SEQUENCE_REQUIRED=2.4.89
+
dnl Versions for external dependencies
DRI2PROTO_REQUIRED=2.8
GLPROTO_REQUIRED=1.4.14
@@ -97,6 +99,7 @@ XCBDRI2_REQUIRED=1.8
XCBDRI3_MODIFIERS_REQUIRED=1.13
XCBGLX_REQUIRED=1.8.1
XCBPRESENT_MODIFIERS_REQUIRED=1.13
+XCBRANDR_XLEASE_REQUIRED=1.13
XDAMAGE_REQUIRED=1.1
XSHMFENCE_REQUIRED=1.1
XVMC_REQUIRED=1.0.6
@@ -1874,20 +1877,6 @@ if test x"$enable_dri3" = xyes; then
fi
fi
-
-if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then
-have_xlease=yes
-else
-have_xlease=no
-fi
-
-if test x"$have_xlease" = xyes; then
-randr_modules="x11-xcb xcb-randr"
-PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules])
-xlib_randr_modules="xrandr"
-PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules])
-fi
-
AM_CONDITIONAL(HAVE_PLATFORM_X11, echo "$platforms" | grep -q 'x11')
AM_CONDITIONAL(HAVE_PLATFORM_WAYLAND, echo "$platforms" | grep -q 'wayland')
AM_CONDITIONAL(HAVE_PLATFORM_DRM, echo "$platforms" | grep -q 'drm')
@@ -1905,14 +1894,24 @@ xno)
;;
*)
if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm';
then
-enable_xlib_lease=yes
+xlease_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED xrandr"
+PKG_CHECK_EXISTS([$xlease_modules], [enable_xlib_lease=yes],
[enable_xlib_lease=no])
else
enable_xlib_lease=no
fi
esac
+if test x"$enable_xlib_lease" = xyes; then
+randr_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED"
+PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules])
+xlib_randr_modules="xrandr"
+PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules])
+fi
+
AM_CONDITIONAL(HAVE_XLIB_LEASE, test "x$enable_xlib_lease" = xyes)
+PKG_CHECK_EXISTS([libdrm >= $LIBDRM_CRT_SEQUENCE_REQUIRED],
[DEFINES="${DEFINES} -DVK_USE_DISPLAY_CONTROL"], [])
+
dnl
dnl More DRI setup
dnl
diff --git a/meson.build b/meson.build
index 7d12af3d51..902074819c 100644
--- a/meson.build
+++ b/meson.build
@@ -1088,6 +1088,8 @@ _drm_freedreno_ver = '2.4.92'
_drm_intel_ver = '2.4.75'
_drm_ver = '2.4.75'
+_drm_crt_sequence_ver = '2.4.89'
+
_libdrm_checks = [
['intel', with_dri_i915 or with_gallium_i915],
['amdgpu', with_amd_vk or with_gallium_radeonsi],
@@ -1361,11 +1363,17 @@ if with_platform_x11
dep_xcb_xfixes = dependency('xcb-xfixes')
endif
if with_xlib_lease
-dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.12')
+dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.13')
dep_xlib_xrandr = dependency('xrandr', version : '>= 1.3')
endif
endif
+if with_any_vk
+ if dep_libdrm.version().version_compare('>= ' + _drm_crt_sequence_ver)
+pre_args += '-DVK_USE_DISPLAY_CONTROL'
+ endif
+endif
+
if get_option('gallium-extra-hud')
pre_args += '-DHAVE_GALLIUM_EXTRA_HUD=1'
endif
diff --git a/src/amd/vulkan/radv_extensions.py
b/src/amd/vulkan/radv_extensions.py
index 094ed3bce3..35b49243a3 100644
--- a/src/amd/vulkan/radv_extensions.py
+++ b/src/amd/vulkan/radv_extensions.py
@@ -92,7 +92,7
Re: [Mesa-dev] [PATCH] vulkan: Fix compilation on older platforms
On 11.07.18 20:43, Dylan Baker wrote:
Quoting Danylo Piliaiev (2018-07-11 04:26:03)
diff --git a/meson.build b/meson.build
index 7d12af3d51..2683060827 100644
--- a/meson.build
+++ b/meson.build
@@ -1088,6 +1088,8 @@ _drm_freedreno_ver = '2.4.92'
_drm_intel_ver = '2.4.75'
_drm_ver = '2.4.75'
+_drm_crt_sequence_ver = '2.4.89'
+
_libdrm_checks = [
['intel', with_dri_i915 or with_gallium_i915],
['amdgpu', with_amd_vk or with_gallium_radeonsi],
@@ -1361,11 +1363,18 @@ if with_platform_x11
dep_xcb_xfixes = dependency('xcb-xfixes')
endif
if with_xlib_lease
-dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.12')
+dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.13')
dep_xlib_xrandr = dependency('xrandr', version : '>= 1.3')
endif
endif
+if with_any_vk
+ dep_drm_crt_sequence = dependency('libdrm', version : '>=' +
_drm_crt_sequence_ver, required : false)
+ if dep_drm_crt_sequence.found()
+pre_args += '-DVK_USE_DISPLAY_CONTROL'
+ endif
+endif
+
Instead of calling into pkg-config again, how about in the "if
dep_libdrm.found()" check around line 1131, we add:
if with_any_vk
if dep_libdrm.version().version_compare('>= ' + _drm_crt_sequence_ver):
pre_args += '-DVK_USE_DISPLAY_CONTROL'
endif
endif
You are right, this would be better.
Or (since radv always requires libdrm > 2.4.89), why don't we just set the set
the minimum to 2.4.89 if vulkan is enabled and be done with it?
It's also affects anv which requires only libdrm >= 2.4.75
Dylan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] vulkan: Fix compilation on older platforms
Make xlease automatically enabled only if xcb-randr >= 1.13,
check its version if manually enabled.
Enable VK_EXT_display_control only when libdrm >= 2.4.89
Check for DRM_EVENT_CONTEXT_VERSION >= 4 to use sequence_handler.
Add support for 'protect' attribute to anv_entrypoints_gen.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107170
https://bugs.freedesktop.org/show_bug.cgi?id=106972
https://bugs.freedesktop.org/show_bug.cgi?id=107176
v2: - Add 'protect="VK_USE_DISPLAY_CONTROL"' attribute to
VK_EXT_display_control in vk.xml
- Add support for 'protect' attribute to anv_entrypoints_gen
(copied from radv_entrypoints_gen)
- Turn #if into #ifdef
- Remove unnecessary pkg-config call from meson build (Dylan Baker)
Signed-off-by: Danylo Piliaiev
---
Other vulkan extensions are gated behind platforms: wayland, xlib, ...
This one depends on library version and I hope I handled it right,
did I took the right approach?
Also what if extension requires both platform and libary version?
Should *_entrypoints_gen be able to support several defines per extension?
configure.ac| 29 -
meson.build | 10 -
src/amd/vulkan/radv_extensions.py | 9 +++-
src/amd/vulkan/radv_wsi_display.c | 5 ++---
src/intel/vulkan/anv_entrypoints_gen.py | 7 ++
src/intel/vulkan/anv_extensions.py | 2 +-
src/intel/vulkan/anv_extensions_gen.py | 7 ++
src/intel/vulkan/anv_wsi_display.c | 4 ++--
src/vulkan/registry/vk.xml | 2 +-
src/vulkan/wsi/wsi_common_display.c | 8 +--
src/vulkan/wsi/wsi_common_display.h | 3 ++-
11 files changed, 59 insertions(+), 27 deletions(-)
diff --git a/configure.ac b/configure.ac
index f135d05736..0b04525014 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,8 @@ LIBDRM_FREEDRENO_REQUIRED=2.4.92
LIBDRM_ETNAVIV_REQUIRED=2.4.89
LIBDRM_VC4_REQUIRED=2.4.89
+LIBDRM_CRT_SEQUENCE_REQUIRED=2.4.89
+
dnl Versions for external dependencies
DRI2PROTO_REQUIRED=2.8
GLPROTO_REQUIRED=1.4.14
@@ -97,6 +99,7 @@ XCBDRI2_REQUIRED=1.8
XCBDRI3_MODIFIERS_REQUIRED=1.13
XCBGLX_REQUIRED=1.8.1
XCBPRESENT_MODIFIERS_REQUIRED=1.13
+XCBRANDR_XLEASE_REQUIRED=1.13
XDAMAGE_REQUIRED=1.1
XSHMFENCE_REQUIRED=1.1
XVMC_REQUIRED=1.0.6
@@ -1874,20 +1877,6 @@ if test x"$enable_dri3" = xyes; then
fi
fi
-
-if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then
-have_xlease=yes
-else
-have_xlease=no
-fi
-
-if test x"$have_xlease" = xyes; then
-randr_modules="x11-xcb xcb-randr"
-PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules])
-xlib_randr_modules="xrandr"
-PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules])
-fi
-
AM_CONDITIONAL(HAVE_PLATFORM_X11, echo "$platforms" | grep -q 'x11')
AM_CONDITIONAL(HAVE_PLATFORM_WAYLAND, echo "$platforms" | grep -q 'wayland')
AM_CONDITIONAL(HAVE_PLATFORM_DRM, echo "$platforms" | grep -q 'drm')
@@ -1905,14 +1894,24 @@ xno)
;;
*)
if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm';
then
-enable_xlib_lease=yes
+xlease_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED xrandr"
+PKG_CHECK_EXISTS([$xlease_modules], [enable_xlib_lease=yes],
[enable_xlib_lease=no])
else
enable_xlib_lease=no
fi
esac
+if test x"$enable_xlib_lease" = xyes; then
+randr_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED"
+PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules])
+xlib_randr_modules="xrandr"
+PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules])
+fi
+
AM_CONDITIONAL(HAVE_XLIB_LEASE, test "x$enable_xlib_lease" = xyes)
+PKG_CHECK_EXISTS([libdrm >= $LIBDRM_CRT_SEQUENCE_REQUIRED],
[DEFINES="${DEFINES} -DVK_USE_DISPLAY_CONTROL"], [])
+
dnl
dnl More DRI setup
dnl
diff --git a/meson.build b/meson.build
index 7d12af3d51..902074819c 100644
--- a/meson.build
+++ b/meson.build
@@ -1088,6 +1088,8 @@ _drm_freedreno_ver = '2.4.92'
_drm_intel_ver = '2.4.75'
_drm_ver = '2.4.75'
+_drm_crt_sequence_ver = '2.4.89'
+
_libdrm_checks = [
['intel', with_dri_i915 or with_gallium_i915],
['amdgpu', with_amd_vk or with_gallium_radeonsi],
@@ -1361,11 +1363,17 @@ if with_platform_x11
dep_xcb_xfixes = dependency('xcb-xfixes')
endif
if with_xlib_lease
-dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.12')
+dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.13')
dep_xlib_xrandr = dependency('xrandr', version : '>= 1.3')
endif
endif
+if with_any_vk
+ if dep_libdrm.version().version_compare('>= ' + _drm_crt_sequence_ver)
+pre_args += '-DVK_USE_DISPLAY_CONTROL'
+ endif
+endif
+
if get_option('gall
Re: [Mesa-dev] [PATCH] configure.ac: Check for xcb-randr version and xrandr to enable leasing
Beside just checking for version being >= 1.13 I made sure that configuring with --enable-xlib-lease will produce error if version isn't right. Also due to small changes in autoconf the patch probably won't apply cleanly so if necessary I'll make new patch which will also include bumping version in meson. - Danil On 09.07.18 12:42, Lionel Landwerlin wrote: Hey Danylo, I didn't really understand why your change didn't just include a >= 1.13 in the if test x"$have_xlease" = xyes; then I guess just adding the revision there (and configure.ac) and bumping the revision in meson.build should be enough. Thanks, - Lionel On 09/07/18 10:16, Danylo Piliaiev wrote: Hi Lionel, Yes meson probably also has the issue since it checks for xcb-randr 1.12 and not 1.13 . I didn't send further patches since vulkan build got broken once more, see my reply to "[PATCH mesa 2/4] vulkan: add VK_EXT_display_control [v8]". Also I have upgrade to Ubuntu 18.04 and currently not affected by this. I'm not sure how to proceed further with the issue. - Danil On 06.07.18 22:30, Lionel Landwerlin wrote: Hi Danylo, I just run into the same issue, thanks for fixing this with autotools. I think the same issue remains with meson though. Thanks, - Lionel On 20/06/18 14:25, Danylo Piliaiev wrote: VK_USE_PLATFORM_XLIB_XRANDR_EXT requires xlib leasing which requires xcb-randr >= 1.13. Also xrandr header is required for this extension. The extension should not be automatically enabled if these dependencies aren't satisfied. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106976 Signed-off-by: Danylo Piliaiev --- configure.ac | 23 ++- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/configure.ac b/configure.ac index 0652410..ab59e06 100644 --- a/configure.ac +++ b/configure.ac @@ -97,6 +97,7 @@ XCBDRI2_REQUIRED=1.8 XCBDRI3_MODIFIERS_REQUIRED=1.13 XCBGLX_REQUIRED=1.8.1 XCBPRESENT_MODIFIERS_REQUIRED=1.13 +XCBRANDR_XLEASE_REQUIRED=1.13 XDAMAGE_REQUIRED=1.1 XSHMFENCE_REQUIRED=1.1 XVMC_REQUIRED=1.0.6 @@ -1867,18 +1868,6 @@ if test x"$enable_dri3" = xyes; then fi fi - -if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then - have_xlease=yes -else - have_xlease=no -fi - -if test x"$have_xlease" = xyes; then - randr_modules="x11-xcb xcb-randr" - PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules]) -fi - AM_CONDITIONAL(HAVE_PLATFORM_X11, echo "$platforms" | grep -q 'x11') AM_CONDITIONAL(HAVE_PLATFORM_WAYLAND, echo "$platforms" | grep -q 'wayland') AM_CONDITIONAL(HAVE_PLATFORM_DRM, echo "$platforms" | grep -q 'drm') @@ -1896,12 +1885,20 @@ xno) ;; *) if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then - enable_xlib_lease=yes + xlease_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED xrandr" + PKG_CHECK_EXISTS([$xlease_modules], [enable_xlib_lease=yes], [enable_xlib_lease=no]) else enable_xlib_lease=no fi esac +if test x"$enable_xlib_lease" = xyes; then + randr_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED" + PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules]) + xlib_randr_modules="xrandr" + PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules]) +fi + AM_CONDITIONAL(HAVE_XLIB_LEASE, test "x$enable_xlib_lease" = xyes) dnl ___ 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] configure.ac: Check for xcb-randr version and xrandr to enable leasing
Hi Lionel, Yes meson probably also has the issue since it checks for xcb-randr 1.12 and not 1.13 . I didn't send further patches since vulkan build got broken once more, see my reply to "[PATCH mesa 2/4] vulkan: add VK_EXT_display_control [v8]". Also I have upgrade to Ubuntu 18.04 and currently not affected by this. I'm not sure how to proceed further with the issue. - Danil On 06.07.18 22:30, Lionel Landwerlin wrote: Hi Danylo, I just run into the same issue, thanks for fixing this with autotools. I think the same issue remains with meson though. Thanks, - Lionel On 20/06/18 14:25, Danylo Piliaiev wrote: VK_USE_PLATFORM_XLIB_XRANDR_EXT requires xlib leasing which requires xcb-randr >= 1.13. Also xrandr header is required for this extension. The extension should not be automatically enabled if these dependencies aren't satisfied. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106976 Signed-off-by: Danylo Piliaiev --- configure.ac | 23 ++- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/configure.ac b/configure.ac index 0652410..ab59e06 100644 --- a/configure.ac +++ b/configure.ac @@ -97,6 +97,7 @@ XCBDRI2_REQUIRED=1.8 XCBDRI3_MODIFIERS_REQUIRED=1.13 XCBGLX_REQUIRED=1.8.1 XCBPRESENT_MODIFIERS_REQUIRED=1.13 +XCBRANDR_XLEASE_REQUIRED=1.13 XDAMAGE_REQUIRED=1.1 XSHMFENCE_REQUIRED=1.1 XVMC_REQUIRED=1.0.6 @@ -1867,18 +1868,6 @@ if test x"$enable_dri3" = xyes; then fi fi - -if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then - have_xlease=yes -else - have_xlease=no -fi - -if test x"$have_xlease" = xyes; then - randr_modules="x11-xcb xcb-randr" - PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules]) -fi - AM_CONDITIONAL(HAVE_PLATFORM_X11, echo "$platforms" | grep -q 'x11') AM_CONDITIONAL(HAVE_PLATFORM_WAYLAND, echo "$platforms" | grep -q 'wayland') AM_CONDITIONAL(HAVE_PLATFORM_DRM, echo "$platforms" | grep -q 'drm') @@ -1896,12 +1885,20 @@ xno) ;; *) if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then - enable_xlib_lease=yes + xlease_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED xrandr" + PKG_CHECK_EXISTS([$xlease_modules], [enable_xlib_lease=yes], [enable_xlib_lease=no]) else enable_xlib_lease=no fi esac +if test x"$enable_xlib_lease" = xyes; then + randr_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED" + PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules]) + xlib_randr_modules="xrandr" + PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules]) +fi + AM_CONDITIONAL(HAVE_XLIB_LEASE, test "x$enable_xlib_lease" = xyes) dnl ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/tools: Remove hardcoded PADDING_SIZE from sanitizer
Signed-off-by: Danylo Piliaiev
---
src/intel/tools/intel_sanitize_gpu.c | 38 +++-
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/src/intel/tools/intel_sanitize_gpu.c
b/src/intel/tools/intel_sanitize_gpu.c
index 9b49b0bbf2..36c4725a2f 100644
--- a/src/intel/tools/intel_sanitize_gpu.c
+++ b/src/intel/tools/intel_sanitize_gpu.c
@@ -51,14 +51,6 @@ static int (*libc_fcntl)(int fd, int cmd, int param);
#define DRM_MAJOR 226
-/* TODO: we want to make sure that the padding forces
- * the BO to take another page on the (PP)GTT; 4KB
- * may or may not be the page size for the BO. Indeed,
- * depending on GPU, kernel version and GEM size, the
- * page size can be one of 4KB, 64KB or 2M.
- */
-#define PADDING_SIZE 4096
-
struct refcnt_hash_table {
struct hash_table *t;
int refcnt;
@@ -80,6 +72,8 @@ pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
static struct hash_table *fds_to_bo_sizes = NULL;
+static long padding_size = 0;
+
static inline struct hash_table*
bo_size_table(int fd)
{
@@ -166,7 +160,7 @@ padding_is_good(int fd, uint32_t handle)
struct drm_i915_gem_mmap mmap_arg = {
.handle = handle,
.offset = bo_size(fd, handle),
- .size = PADDING_SIZE,
+ .size = padding_size,
.flags = 0,
};
@@ -189,17 +183,17 @@ padding_is_good(int fd, uint32_t handle)
* if the bo is not cache coherent we likely need to
* invalidate the cache lines to get it.
*/
- gen_invalidate_range(mapped, PADDING_SIZE);
+ gen_invalidate_range(mapped, padding_size);
expected_value = handle & 0xFF;
- for (uint32_t i = 0; i < PADDING_SIZE; ++i) {
+ for (uint32_t i = 0; i < padding_size; ++i) {
if (expected_value != mapped[i]) {
- munmap(mapped, PADDING_SIZE);
+ munmap(mapped, padding_size);
return false;
}
expected_value = next_noise_value(expected_value);
}
- munmap(mapped, PADDING_SIZE);
+ munmap(mapped, padding_size);
return true;
}
@@ -207,9 +201,9 @@ padding_is_good(int fd, uint32_t handle)
static int
create_with_padding(int fd, struct drm_i915_gem_create *create)
{
- create->size += PADDING_SIZE;
+ create->size += padding_size;
int ret = libc_ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, create);
- create->size -= PADDING_SIZE;
+ create->size -= padding_size;
if (ret != 0)
return ret;
@@ -218,7 +212,7 @@ create_with_padding(int fd, struct drm_i915_gem_create
*create)
struct drm_i915_gem_mmap mmap_arg = {
.handle = create->handle,
.offset = create->size,
- .size = PADDING_SIZE,
+ .size = padding_size,
.flags = 0,
};
@@ -228,8 +222,8 @@ create_with_padding(int fd, struct drm_i915_gem_create
*create)
noise_values = (uint8_t*) (uintptr_t) mmap_arg.addr_ptr;
fill_noise_buffer(noise_values, create->handle & 0xFF,
- PADDING_SIZE);
- munmap(noise_values, PADDING_SIZE);
+ padding_size);
+ munmap(noise_values, padding_size);
_mesa_hash_table_insert(bo_size_table(fd), (void*)(uintptr_t)create->handle,
(void*)(uintptr_t)create->size);
@@ -427,4 +421,12 @@ init(void)
libc_close = dlsym(RTLD_NEXT, "close");
libc_fcntl = dlsym(RTLD_NEXT, "fcntl");
libc_ioctl = dlsym(RTLD_NEXT, "ioctl");
+
+ /* We want to make sure that the padding forces
+* the BO to take another page on the (PP)GTT.
+*/
+ padding_size = sysconf(_SC_PAGESIZE);
+ if (padding_size == -1) {
+ unreachable("Bad page size");
+ }
}
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/tools: Remove hardcoded PADDING_SIZE from sanitizer
On Wed, Oct 17, 2018 at 6:47 PM Rafael Antognolli <
rafael.antogno...@intel.com> wrote:
> On Wed, Oct 17, 2018 at 06:08:34PM +0300, Danylo Piliaiev wrote:
> > Signed-off-by: Danylo Piliaiev
> > ---
> > src/intel/tools/intel_sanitize_gpu.c | 38 +++-
> > 1 file changed, 20 insertions(+), 18 deletions(-)
> >
> > diff --git a/src/intel/tools/intel_sanitize_gpu.c
> b/src/intel/tools/intel_sanitize_gpu.c
> > index 9b49b0bbf2..36c4725a2f 100644
> > --- a/src/intel/tools/intel_sanitize_gpu.c
> > +++ b/src/intel/tools/intel_sanitize_gpu.c
> > @@ -51,14 +51,6 @@ static int (*libc_fcntl)(int fd, int cmd, int param);
> >
> > #define DRM_MAJOR 226
> >
> > -/* TODO: we want to make sure that the padding forces
> > - * the BO to take another page on the (PP)GTT; 4KB
> > - * may or may not be the page size for the BO. Indeed,
> > - * depending on GPU, kernel version and GEM size, the
> > - * page size can be one of 4KB, 64KB or 2M.
> > - */
> > -#define PADDING_SIZE 4096
> > -
> > struct refcnt_hash_table {
> > struct hash_table *t;
> > int refcnt;
> > @@ -80,6 +72,8 @@ pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
> >
> > static struct hash_table *fds_to_bo_sizes = NULL;
> >
> > +static long padding_size = 0;
> > +
> > static inline struct hash_table*
> > bo_size_table(int fd)
> > {
> > @@ -166,7 +160,7 @@ padding_is_good(int fd, uint32_t handle)
> > struct drm_i915_gem_mmap mmap_arg = {
> >.handle = handle,
> >.offset = bo_size(fd, handle),
> > - .size = PADDING_SIZE,
> > + .size = padding_size,
> >.flags = 0,
> > };
> >
> > @@ -189,17 +183,17 @@ padding_is_good(int fd, uint32_t handle)
> > * if the bo is not cache coherent we likely need to
> > * invalidate the cache lines to get it.
> > */
> > - gen_invalidate_range(mapped, PADDING_SIZE);
> > + gen_invalidate_range(mapped, padding_size);
> >
> > expected_value = handle & 0xFF;
> > - for (uint32_t i = 0; i < PADDING_SIZE; ++i) {
> > + for (uint32_t i = 0; i < padding_size; ++i) {
> >if (expected_value != mapped[i]) {
> > - munmap(mapped, PADDING_SIZE);
> > + munmap(mapped, padding_size);
> > return false;
> >}
> >expected_value = next_noise_value(expected_value);
> > }
> > - munmap(mapped, PADDING_SIZE);
> > + munmap(mapped, padding_size);
> >
> > return true;
> > }
> > @@ -207,9 +201,9 @@ padding_is_good(int fd, uint32_t handle)
> > static int
> > create_with_padding(int fd, struct drm_i915_gem_create *create)
> > {
> > - create->size += PADDING_SIZE;
> > + create->size += padding_size;
> > int ret = libc_ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, create);
> > - create->size -= PADDING_SIZE;
> > + create->size -= padding_size;
> >
> > if (ret != 0)
> >return ret;
> > @@ -218,7 +212,7 @@ create_with_padding(int fd, struct
> drm_i915_gem_create *create)
> > struct drm_i915_gem_mmap mmap_arg = {
> >.handle = create->handle,
> >.offset = create->size,
> > - .size = PADDING_SIZE,
> > + .size = padding_size,
> >.flags = 0,
> > };
> >
> > @@ -228,8 +222,8 @@ create_with_padding(int fd, struct
> drm_i915_gem_create *create)
> >
> > noise_values = (uint8_t*) (uintptr_t) mmap_arg.addr_ptr;
> > fill_noise_buffer(noise_values, create->handle & 0xFF,
> > - PADDING_SIZE);
> > - munmap(noise_values, PADDING_SIZE);
> > + padding_size);
> > + munmap(noise_values, padding_size);
> >
> > _mesa_hash_table_insert(bo_size_table(fd),
> (void*)(uintptr_t)create->handle,
> > (void*)(uintptr_t)create->size);
> > @@ -427,4 +421,12 @@ init(void)
> > libc_close = dlsym(RTLD_NEXT, "close");
> > libc_fcntl = dlsym(RTLD_NEXT, "fcntl");
> > libc_ioctl = dlsym(RTLD_NEXT, "ioctl");
> > +
> > + /* We want to make sure that the padding forces
> > +* the BO to take another page on the (PP)GTT.
> > +*/
> > + padding_size = sysconf(_SC_PAGESIZE);
>
> I don't think this is the page size we want. This is the page size of
> CPU/system memory, which might be different from what the GPU is using
> to map pages. For instance, even if we are using 6
[Mesa-dev] [PATCH 3/3] anv: Implement VK_EXT_conditional_rendering for gen 7.5+
Conditional rendering affects next functions:
- vkCmdDraw, vkCmdDrawIndexed, vkCmdDrawIndirect, vkCmdDrawIndexedIndirect
- vkCmdDrawIndirectCountKHR, vkCmdDrawIndexedIndirectCountKHR
- vkCmdDispatch, vkCmdDispatchIndirect, vkCmdDispatchBase
- vkCmdClearAttachments
To reduce readings from the memory a result of the condition is calculated
and stored into designated register MI_ALU_REG15.
In current implementation affected functions expect MI_PREDICATE_RESULT
being set before their call so any code which changes the predicate
should restore it with restore_conditional_render_predicate.
An alternative is to restore MI_PREDICATE_RESULT in all affected
functions at their beginning.
Signed-off-by: Danylo Piliaiev
---
src/intel/vulkan/anv_blorp.c | 7 +-
src/intel/vulkan/anv_device.c | 12 ++
src/intel/vulkan/anv_extensions.py | 1 +
src/intel/vulkan/anv_private.h | 2 +
src/intel/vulkan/genX_cmd_buffer.c | 192 -
5 files changed, 209 insertions(+), 5 deletions(-)
diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
index 478b8e7a3d..157875d16f 100644
--- a/src/intel/vulkan/anv_blorp.c
+++ b/src/intel/vulkan/anv_blorp.c
@@ -1144,8 +1144,11 @@ void anv_CmdClearAttachments(
* trash our depth and stencil buffers.
*/
struct blorp_batch batch;
- blorp_batch_init(_buffer->device->blorp, , cmd_buffer,
-BLORP_BATCH_NO_EMIT_DEPTH_STENCIL);
+ enum blorp_batch_flags flags = BLORP_BATCH_NO_EMIT_DEPTH_STENCIL;
+ if (cmd_buffer->state.conditional_render_enabled) {
+ flags |= BLORP_BATCH_PREDICATE_ENABLE;
+ }
+ blorp_batch_init(_buffer->device->blorp, , cmd_buffer, flags);
for (uint32_t a = 0; a < attachmentCount; ++a) {
if (pAttachments[a].aspectMask & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) {
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index a2551452eb..930a192c25 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -957,6 +957,18 @@ void anv_GetPhysicalDeviceFeatures2(
break;
}
+ case
VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CONDITIONAL_RENDERING_FEATURES_EXT: {
+ VkPhysicalDeviceConditionalRenderingFeaturesEXT *features =
+(VkPhysicalDeviceConditionalRenderingFeaturesEXT*)ext;
+ ANV_FROM_HANDLE(anv_physical_device, pdevice, physicalDevice);
+
+ features->conditionalRendering = pdevice->info.gen >= 8 ||
+ pdevice->info.is_haswell;
+ features->inheritedConditionalRendering = pdevice->info.gen >= 8 ||
+ pdevice->info.is_haswell;
+ break;
+ }
+
default:
anv_debug_ignored_stype(ext->sType);
break;
diff --git a/src/intel/vulkan/anv_extensions.py
b/src/intel/vulkan/anv_extensions.py
index c13ce531ee..2ef7a52d01 100644
--- a/src/intel/vulkan/anv_extensions.py
+++ b/src/intel/vulkan/anv_extensions.py
@@ -127,6 +127,7 @@ EXTENSIONS = [
Extension('VK_EXT_vertex_attribute_divisor', 3, True),
Extension('VK_EXT_post_depth_coverage', 1, 'device->info.gen
>= 9'),
Extension('VK_EXT_sampler_filter_minmax', 1, 'device->info.gen
>= 9'),
+Extension('VK_EXT_conditional_rendering', 1, 'device->info.gen
>= 8 || device->info.is_haswell'),
]
class VkVersion:
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index 599b903f25..108da51a59 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -2032,6 +2032,8 @@ struct anv_cmd_state {
*/
bool hiz_enabled;
+ bool conditional_render_enabled;
+
/**
* Array length is anv_cmd_state::pass::attachment_count. Array content is
* valid only when recording a render pass instance.
diff --git a/src/intel/vulkan/genX_cmd_buffer.c
b/src/intel/vulkan/genX_cmd_buffer.c
index f07a6aa7c9..87abc443b6 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -479,8 +479,9 @@ transition_depth_buffer(struct anv_cmd_buffer *cmd_buffer,
0, 0, 1, hiz_op);
}
-#define MI_PREDICATE_SRC0 0x2400
-#define MI_PREDICATE_SRC1 0x2408
+#define MI_PREDICATE_SRC00x2400
+#define MI_PREDICATE_SRC10x2408
+#define MI_PREDICATE_RESULT 0x2418
static void
set_image_compressed_bit(struct anv_cmd_buffer *cmd_buffer,
@@ -545,6 +546,14 @@ mi_alu(uint32_t opcode, uint32_t operand1, uint32_t
operand2)
#define CS_GPR(n) (0x2600 + (n) * 8)
+#if GEN_GEN >= 8 || GEN_IS_HASWELL
+static void
+restore_conditional_render_predicate(struct anv_cmd_buffer *cmd_buffer)
+{
+ emit_lrr(_buffer->batch, MI_PREDICATE_RESULT, CS_GPR(MI_ALU_REG15));
+}
+#endif
+
/* This is only really practical on
[Mesa-dev] [PATCH 0/3] anv: Implement VK_KHR_draw_indirect_count and VK_EXT_conditional_rendering
This series implement VK_KHR_draw_indirect_count and VK_EXT_conditional_rendering extensions. They are implemented together because they are highly interweaved. There are already tests in VK_CTS for VK_KHR_draw_indirect_count and I made a pull request with the tests for VK_EXT_conditional_rendering (https://github.com/KhronosGroup/VK-GL-CTS/pull/131). VK_KHR_draw_indirect_count is implemented for gen7+. VK_EXT_conditional_rendering is implemented for gen7.5+ because it requires MI_MATH to be implemented correctly. Since part of the tests aren't in VK-GL-CTS master I'm not sure how to test the implementation of VK_EXT_conditional_rendering with my tests on CI. Could anyone help me with this? Also the one thing I'm uncertain of is described in the last patch. Many thanks to Jason Ekstrand for the help with the extensions. Danylo Piliaiev (3): anv: Implement VK_KHR_draw_indirect_count for gen 7.5+ anv: Implement VK_KHR_draw_indirect_count for gen 7 anv: Implement VK_EXT_conditional_rendering for gen 7.5+ src/intel/vulkan/anv_blorp.c | 7 +- src/intel/vulkan/anv_device.c | 12 + src/intel/vulkan/anv_extensions.py | 2 + src/intel/vulkan/anv_private.h | 2 + src/intel/vulkan/genX_cmd_buffer.c | 355 - 5 files changed, 373 insertions(+), 5 deletions(-) -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] anv: Implement VK_KHR_draw_indirect_count for gen 7.5+
Signed-off-by: Danylo Piliaiev
---
src/intel/vulkan/anv_extensions.py | 1 +
src/intel/vulkan/genX_cmd_buffer.c | 155 +
2 files changed, 156 insertions(+)
diff --git a/src/intel/vulkan/anv_extensions.py
b/src/intel/vulkan/anv_extensions.py
index d4915c9501..7f44da6648 100644
--- a/src/intel/vulkan/anv_extensions.py
+++ b/src/intel/vulkan/anv_extensions.py
@@ -113,6 +113,7 @@ EXTENSIONS = [
Extension('VK_KHR_xlib_surface', 6,
'VK_USE_PLATFORM_XLIB_KHR'),
Extension('VK_KHR_multiview', 1, True),
Extension('VK_KHR_display', 23,
'VK_USE_PLATFORM_DISPLAY_KHR'),
+Extension('VK_KHR_draw_indirect_count', 1, 'device->info.gen
>= 8 || device->info.is_haswell'),
Extension('VK_EXT_acquire_xlib_display', 1,
'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
Extension('VK_EXT_debug_report', 8, True),
Extension('VK_EXT_direct_mode_display', 1,
'VK_USE_PLATFORM_DISPLAY_KHR'),
diff --git a/src/intel/vulkan/genX_cmd_buffer.c
b/src/intel/vulkan/genX_cmd_buffer.c
index 43a02f2256..d7b94efd19 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -2982,6 +2982,161 @@ void genX(CmdDrawIndexedIndirect)(
}
}
+#if GEN_IS_HASWELL || GEN_GEN >= 8
+static void
+emit_draw_count_predicate(struct anv_cmd_buffer *cmd_buffer,
+ struct anv_address count_address,
+ uint32_t draw_index)
+{
+ /* Upload the current draw count from the draw parameters buffer to
+* MI_PREDICATE_SRC0.
+*/
+ emit_lrr(_buffer->batch, MI_PREDICATE_SRC0, CS_GPR(MI_ALU_REG14));
+
+ /* Upload the index of the current primitive to MI_PREDICATE_SRC1. */
+ emit_lri(_buffer->batch, MI_PREDICATE_SRC1, draw_index);
+ emit_lri(_buffer->batch, MI_PREDICATE_SRC1 + 4, 0);
+
+ if (draw_index == 0) {
+ anv_batch_emit(_buffer->batch, GENX(MI_PREDICATE), mip) {
+ mip.LoadOperation= LOAD_LOADINV;
+ mip.CombineOperation = COMBINE_SET;
+ mip.CompareOperation = COMPARE_SRCS_EQUAL;
+ }
+ } else {
+ /* While draw_index < draw_count the predicate's result will be
+* (draw_index == draw_count) ^ TRUE = TRUE
+* When draw_index == draw_count the result is
+* (TRUE) ^ TRUE = FALSE
+* After this all results will be:
+* (FALSE) ^ FALSE = FALSE
+*/
+ anv_batch_emit(_buffer->batch, GENX(MI_PREDICATE), mip) {
+ mip.LoadOperation= LOAD_LOAD;
+ mip.CombineOperation = COMBINE_XOR;
+ mip.CompareOperation = COMPARE_SRCS_EQUAL;
+ }
+ }
+}
+
+void genX(CmdDrawIndirectCountKHR)(
+VkCommandBuffer commandBuffer,
+VkBuffer_buffer,
+VkDeviceSizeoffset,
+VkBuffer_countBuffer,
+VkDeviceSizecountBufferOffset,
+uint32_tmaxDrawCount,
+uint32_tstride)
+{
+ ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer);
+ ANV_FROM_HANDLE(anv_buffer, buffer, _buffer);
+ ANV_FROM_HANDLE(anv_buffer, count_buffer, _countBuffer);
+ struct anv_cmd_state *cmd_state = _buffer->state;
+ struct anv_pipeline *pipeline = cmd_state->gfx.base.pipeline;
+ const struct brw_vs_prog_data *vs_prog_data = get_vs_prog_data(pipeline);
+
+ if (anv_batch_has_error(_buffer->batch))
+ return;
+
+ genX(cmd_buffer_flush_state)(cmd_buffer);
+
+ struct anv_address count_address =
+ anv_address_add(count_buffer->address, countBufferOffset);
+
+ /* Needed to ensure the memory is coherent for the MI_LOAD_REGISTER_MEM
+* command when loading the values into the predicate source registers.
+*/
+ anv_batch_emit(_buffer->batch, GENX(PIPE_CONTROL), pc) {
+ pc.PipeControlFlushEnable = true;
+ }
+
+ emit_lrm(_buffer->batch, CS_GPR(MI_ALU_REG14), count_address);
+ emit_lri(_buffer->batch, CS_GPR(MI_ALU_REG14) + 4, 0);
+
+ for (uint32_t i = 0; i < maxDrawCount; i++) {
+ struct anv_address draw = anv_address_add(buffer->address, offset);
+
+ emit_draw_count_predicate(cmd_buffer, count_address, i);
+
+ if (vs_prog_data->uses_firstvertex ||
+ vs_prog_data->uses_baseinstance)
+ emit_base_vertex_instance_bo(cmd_buffer, anv_address_add(draw, 8));
+ if (vs_prog_data->uses_drawid)
+ emit_draw_index(cmd_buffer, i);
+
+ load_indirect_parameters(cmd_buffer, draw, false);
+
+ anv_batch_emit(_buffer->batch, GENX(3DPRIMITIVE), prim) {
+ prim.IndirectParameterEnable = true;
+ prim.PredicateEnable = true;
+ prim.VertexAccessType = SEQUENTIAL;
+ prim.Pr
[Mesa-dev] [PATCH 2/3] anv: Implement VK_KHR_draw_indirect_count for gen 7
Without MI_MATH we are forced to load MI_PREDICATE_SRC0
from memory on every predicate emission.
Signed-off-by: Danylo Piliaiev
---
src/intel/vulkan/anv_extensions.py | 2 +-
src/intel/vulkan/genX_cmd_buffer.c | 12 ++--
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/intel/vulkan/anv_extensions.py
b/src/intel/vulkan/anv_extensions.py
index 7f44da6648..c13ce531ee 100644
--- a/src/intel/vulkan/anv_extensions.py
+++ b/src/intel/vulkan/anv_extensions.py
@@ -113,7 +113,7 @@ EXTENSIONS = [
Extension('VK_KHR_xlib_surface', 6,
'VK_USE_PLATFORM_XLIB_KHR'),
Extension('VK_KHR_multiview', 1, True),
Extension('VK_KHR_display', 23,
'VK_USE_PLATFORM_DISPLAY_KHR'),
-Extension('VK_KHR_draw_indirect_count', 1, 'device->info.gen
>= 8 || device->info.is_haswell'),
+Extension('VK_KHR_draw_indirect_count', 1, True),
Extension('VK_EXT_acquire_xlib_display', 1,
'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
Extension('VK_EXT_debug_report', 8, True),
Extension('VK_EXT_direct_mode_display', 1,
'VK_USE_PLATFORM_DISPLAY_KHR'),
diff --git a/src/intel/vulkan/genX_cmd_buffer.c
b/src/intel/vulkan/genX_cmd_buffer.c
index d7b94efd19..f07a6aa7c9 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -2982,7 +2982,6 @@ void genX(CmdDrawIndexedIndirect)(
}
}
-#if GEN_IS_HASWELL || GEN_GEN >= 8
static void
emit_draw_count_predicate(struct anv_cmd_buffer *cmd_buffer,
struct anv_address count_address,
@@ -2991,7 +2990,13 @@ emit_draw_count_predicate(struct anv_cmd_buffer
*cmd_buffer,
/* Upload the current draw count from the draw parameters buffer to
* MI_PREDICATE_SRC0.
*/
+#if GEN_GEN >= 8 || GEN_IS_HASWELL
emit_lrr(_buffer->batch, MI_PREDICATE_SRC0, CS_GPR(MI_ALU_REG14));
+#else
+ emit_lrm(_buffer->batch, MI_PREDICATE_SRC0, count_address);
+ /* Zero the top 32-bits of MI_PREDICATE_SRC0 */
+ emit_lri(_buffer->batch, MI_PREDICATE_SRC0 + 4, 0);
+#endif
/* Upload the index of the current primitive to MI_PREDICATE_SRC1. */
emit_lri(_buffer->batch, MI_PREDICATE_SRC1, draw_index);
@@ -3050,8 +3055,10 @@ void genX(CmdDrawIndirectCountKHR)(
pc.PipeControlFlushEnable = true;
}
+#if GEN_GEN >= 8 || GEN_IS_HASWELL
emit_lrm(_buffer->batch, CS_GPR(MI_ALU_REG14), count_address);
emit_lri(_buffer->batch, CS_GPR(MI_ALU_REG14) + 4, 0);
+#endif
for (uint32_t i = 0; i < maxDrawCount; i++) {
struct anv_address draw = anv_address_add(buffer->address, offset);
@@ -3108,8 +3115,10 @@ void genX(CmdDrawIndexedIndirectCountKHR)(
pc.PipeControlFlushEnable = true;
}
+#if GEN_GEN >= 8 || GEN_IS_HASWELL
emit_lrm(_buffer->batch, CS_GPR(MI_ALU_REG14), count_address);
emit_lri(_buffer->batch, CS_GPR(MI_ALU_REG14) + 4, 0);
+#endif
for (uint32_t i = 0; i < maxDrawCount; i++) {
struct anv_address draw = anv_address_add(buffer->address, offset);
@@ -3135,7 +3144,6 @@ void genX(CmdDrawIndexedIndirectCountKHR)(
offset += stride;
}
}
-#endif
static VkResult
flush_compute_descriptor_set(struct anv_cmd_buffer *cmd_buffer)
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3] i965: Fix calculation of layers array length for isl_view
I have made a Piglit test that exercises the issue:
https://patchwork.freedesktop.org/patch/258180/
- Danil
On 9/10/18 6:21 PM, Danylo Piliaiev wrote:
Handle all cases in calculation of layers count for isl_view
taking into account texture view and image unit.
st_convert_image was taken as a reference.
When u->Layered is true the whole level is taken with respect to
image view. In other case only one layer is taken.
v3: (Józef Kucia and Ilia Mirkin)
- Rewrote patch by taking st_convert_image as a reference
- Removed now unused get_image_num_layers function
- Changed commit message
Fixes: 5a8c8903
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
Signed-off-by: Danylo Piliaiev
---
.../drivers/dri/i965/brw_wm_surface_state.c | 32 ++-
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 944762ec46..9bfe6e2037 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1499,18 +1499,6 @@ update_buffer_image_param(struct brw_context *brw,
param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
}
-static unsigned
-get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum target,
- unsigned level)
-{
- if (target == GL_TEXTURE_CUBE_MAP)
- return 6;
-
- return target == GL_TEXTURE_3D ?
- minify(mt->surf.logical_level0_px.depth, level) :
- mt->surf.logical_level0_px.array_len;
-}
-
static void
update_image_surface(struct brw_context *brw,
struct gl_image_unit *u,
@@ -1541,14 +1529,28 @@ update_image_surface(struct brw_context *brw,
} else {
struct intel_texture_object *intel_obj = intel_texture_object(obj);
struct intel_mipmap_tree *mt = intel_obj->mt;
- const unsigned num_layers = u->Layered ?
-get_image_num_layers(mt, obj->Target, u->Level) : 1;
+
+ unsigned base_layer, num_layers;
+ if (u->Layered) {
+if (obj->Target == GL_TEXTURE_3D) {
+ base_layer = 0;
+ num_layers = minify(mt->surf.logical_level0_px.depth, u->Level);
+} else {
+ base_layer = obj->MinLayer;
+ num_layers = obj->Immutable ?
+obj->NumLayers :
+mt->surf.logical_level0_px.array_len;
+}
+ } else {
+base_layer = obj->MinLayer + u->_Layer;
+num_layers = 1;
+ }
struct isl_view view = {
.format = format,
.base_level = obj->MinLevel + u->Level,
.levels = 1,
-.base_array_layer = obj->MinLayer + u->_Layer,
+.base_array_layer = base_layer,
.array_len = num_layers,
.swizzle = ISL_SWIZZLE_IDENTITY,
.usage = ISL_SURF_USAGE_STORAGE_BIT,
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3] i965: Fix calculation of layers array length for isl_view
Hello,
Could anyone look at the patch?
Thanks!
On 10/24/18 2:22 PM, Danylo Piliaiev wrote:
I have made a Piglit test that exercises the issue:
https://patchwork.freedesktop.org/patch/258180/
- Danil
On 9/10/18 6:21 PM, Danylo Piliaiev wrote:
Handle all cases in calculation of layers count for isl_view
taking into account texture view and image unit.
st_convert_image was taken as a reference.
When u->Layered is true the whole level is taken with respect to
image view. In other case only one layer is taken.
v3: (Józef Kucia and Ilia Mirkin)
- Rewrote patch by taking st_convert_image as a reference
- Removed now unused get_image_num_layers function
- Changed commit message
Fixes: 5a8c8903
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
Signed-off-by: Danylo Piliaiev
---
.../drivers/dri/i965/brw_wm_surface_state.c | 32 ++-
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 944762ec46..9bfe6e2037 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1499,18 +1499,6 @@ update_buffer_image_param(struct brw_context
*brw,
param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
}
-static unsigned
-get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum target,
- unsigned level)
-{
- if (target == GL_TEXTURE_CUBE_MAP)
- return 6;
-
- return target == GL_TEXTURE_3D ?
- minify(mt->surf.logical_level0_px.depth, level) :
- mt->surf.logical_level0_px.array_len;
-}
-
static void
update_image_surface(struct brw_context *brw,
struct gl_image_unit *u,
@@ -1541,14 +1529,28 @@ update_image_surface(struct brw_context *brw,
} else {
struct intel_texture_object *intel_obj =
intel_texture_object(obj);
struct intel_mipmap_tree *mt = intel_obj->mt;
- const unsigned num_layers = u->Layered ?
- get_image_num_layers(mt, obj->Target, u->Level) : 1;
+
+ unsigned base_layer, num_layers;
+ if (u->Layered) {
+ if (obj->Target == GL_TEXTURE_3D) {
+ base_layer = 0;
+ num_layers = minify(mt->surf.logical_level0_px.depth,
u->Level);
+ } else {
+ base_layer = obj->MinLayer;
+ num_layers = obj->Immutable ?
+ obj->NumLayers :
+ mt->surf.logical_level0_px.array_len;
+ }
+ } else {
+ base_layer = obj->MinLayer + u->_Layer;
+ num_layers = 1;
+ }
struct isl_view view = {
.format = format,
.base_level = obj->MinLevel + u->Level,
.levels = 1,
- .base_array_layer = obj->MinLayer + u->_Layer,
+ .base_array_layer = base_layer,
.array_len = num_layers,
.swizzle = ISL_SWIZZLE_IDENTITY,
.usage = ISL_SURF_USAGE_STORAGE_BIT,
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4] i965: Fix calculation of layers array length for isl_view
Thanks!
On 11/16/18 7:09 AM, Jason Ekstrand wrote:
Pushed.
On Thu, Nov 15, 2018 at 8:15 PM Jason Ekstrand wrote:
Thanks. I'm going to push it once it gets done running through CI.
On Thu, Nov 15, 2018 at 4:03 AM Danylo Piliaiev
wrote:
Handle all cases in calculation of layers count for isl_view
taking into account texture view and image unit.
st_convert_image was taken as a reference.
When u->Layered is true the whole level is taken with respect to
image view. In other case only one layer is taken.
v3: (Józef Kucia and Ilia Mirkin)
- Rewrote patch by taking st_convert_image as a reference
- Removed now unused get_image_num_layers function
- Changed commit message
v4: (Jason Ekstrand)
- Added assert
Fixes: 5a8c8903
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
Signed-off-by: Danylo Piliaiev
Reviewed-by: Jason Ekstrand
---
.../drivers/dri/i965/brw_wm_surface_state.c | 33 ++-
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 8d21cf5fa7..3286c222e5 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1499,18 +1499,6 @@ update_buffer_image_param(struct brw_context *brw,
param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
}
-static unsigned
-get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum target,
- unsigned level)
-{
- if (target == GL_TEXTURE_CUBE_MAP)
- return 6;
-
- return target == GL_TEXTURE_3D ?
- minify(mt->surf.logical_level0_px.depth, level) :
- mt->surf.logical_level0_px.array_len;
-}
-
static void
update_image_surface(struct brw_context *brw,
struct gl_image_unit *u,
@@ -1541,14 +1529,29 @@ update_image_surface(struct brw_context *brw,
} else {
struct intel_texture_object *intel_obj =
intel_texture_object(obj);
struct intel_mipmap_tree *mt = intel_obj->mt;
- const unsigned num_layers = u->Layered ?
-get_image_num_layers(mt, obj->Target, u->Level) : 1;
+
+ unsigned base_layer, num_layers;
+ if (u->Layered) {
+if (obj->Target == GL_TEXTURE_3D) {
+ base_layer = 0;
+ num_layers = minify(mt->surf.logical_level0_px.depth,
u->Level);
+} else {
+ assert(obj->Immutable || obj->MinLayer == 0);
+ base_layer = obj->MinLayer;
+ num_layers = obj->Immutable ?
+obj->NumLayers :
+mt->surf.logical_level0_px.array_len;
+}
+ } else {
+base_layer = obj->MinLayer + u->_Layer;
+num_layers = 1;
+ }
struct isl_view view = {
.format = format,
.base_level = obj->MinLevel + u->Level,
.levels = 1,
-.base_array_layer = obj->MinLayer + u->_Layer,
+.base_array_layer = base_layer,
.array_len = num_layers,
.swizzle = ISL_SWIZZLE_IDENTITY,
.usage = ISL_SURF_USAGE_STORAGE_BIT,
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] anv: Implement VK_EXT_conditional_rendering for gen 7.5+
On 11/6/18 1:05 AM, Jason Ekstrand wrote:
On Wed, Oct 17, 2018 at 6:59 AM Danylo Piliaiev
wrote:
Conditional rendering affects next functions:
- vkCmdDraw, vkCmdDrawIndexed, vkCmdDrawIndirect, vkCmdDrawIndexedIndirect
- vkCmdDrawIndirectCountKHR, vkCmdDrawIndexedIndirectCountKHR
- vkCmdDispatch, vkCmdDispatchIndirect, vkCmdDispatchBase
- vkCmdClearAttachments
To reduce readings from the memory a result of the condition is calculated
and stored into designated register MI_ALU_REG15.
In current implementation affected functions expect MI_PREDICATE_RESULT
being set before their call so any code which changes the predicate
should restore it with restore_conditional_render_predicate.
An alternative is to restore MI_PREDICATE_RESULT in all affected
functions at their beginning.
Signed-off-by: Danylo Piliaiev
---
src/intel/vulkan/anv_blorp.c | 7 +-
src/intel/vulkan/anv_device.c | 12 ++
src/intel/vulkan/anv_extensions.py | 1 +
src/intel/vulkan/anv_private.h | 2 +
src/intel/vulkan/genX_cmd_buffer.c | 192 -
5 files changed, 209 insertions(+), 5 deletions(-)
diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
index 478b8e7a3d..157875d16f 100644
--- a/src/intel/vulkan/anv_blorp.c
+++ b/src/intel/vulkan/anv_blorp.c
@@ -1144,8 +1144,11 @@ void anv_CmdClearAttachments(
* trash our depth and stencil buffers.
*/
struct blorp_batch batch;
- blorp_batch_init(_buffer->device->blorp, , cmd_buffer,
-BLORP_BATCH_NO_EMIT_DEPTH_STENCIL);
+ enum blorp_batch_flags flags = BLORP_BATCH_NO_EMIT_DEPTH_STENCIL;
+ if (cmd_buffer->state.conditional_render_enabled) {
+ flags |= BLORP_BATCH_PREDICATE_ENABLE;
+ }
+ blorp_batch_init(_buffer->device->blorp, , cmd_buffer,
flags);
for (uint32_t a = 0; a < attachmentCount; ++a) {
if (pAttachments[a].aspectMask & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV)
{
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index a2551452eb..930a192c25 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -957,6 +957,18 @@ void anv_GetPhysicalDeviceFeatures2(
break;
}
+ case
VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CONDITIONAL_RENDERING_FEATURES_EXT: {
+ VkPhysicalDeviceConditionalRenderingFeaturesEXT *features =
+(VkPhysicalDeviceConditionalRenderingFeaturesEXT*)ext;
+ ANV_FROM_HANDLE(anv_physical_device, pdevice, physicalDevice);
+
+ features->conditionalRendering = pdevice->info.gen >= 8 ||
+ pdevice->info.is_haswell;
+ features->inheritedConditionalRendering = pdevice->info.gen >= 8
||
+
pdevice->info.is_haswell;
+ break;
+ }
+
default:
anv_debug_ignored_stype(ext->sType);
break;
diff --git a/src/intel/vulkan/anv_extensions.py
b/src/intel/vulkan/anv_extensions.py
index c13ce531ee..2ef7a52d01 100644
--- a/src/intel/vulkan/anv_extensions.py
+++ b/src/intel/vulkan/anv_extensions.py
@@ -127,6 +127,7 @@ EXTENSIONS = [
Extension('VK_EXT_vertex_attribute_divisor', 3, True),
Extension('VK_EXT_post_depth_coverage', 1,
'device->info.gen >= 9'),
Extension('VK_EXT_sampler_filter_minmax', 1,
'device->info.gen >= 9'),
+Extension('VK_EXT_conditional_rendering', 1,
'device->info.gen >= 8 || device->info.is_haswell'),
]
class VkVersion:
diff --git a/src/intel/vulkan/anv_private.h
b/src/intel/vulkan/anv_private.h
index 599b903f25..108da51a59 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -2032,6 +2032,8 @@ struct anv_cmd_state {
*/
bool hiz_enabled;
+ bool
conditional_render_enabled;
+
/**
* Array length is anv_cmd_state::pass::attachment_count. Array
content is
* valid only when recording a render pass instance.
diff --git a/src/intel/vulkan/genX_cmd_buffer.c
b/src/intel/vulkan/genX_cmd_buffer.c
index f07a6aa7c9..87abc443b6 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -479,8 +479,9 @@ transition_depth_buffer(struct anv_cmd_buffer
*cmd_buffer,
0, 0, 1, hiz_op);
}
-#define MI_PREDICATE_SRC0 0x2400
-#define MI_PREDICATE_SRC1 0x2408
+#define MI_PREDICATE_SRC00x2400
+#define MI_PREDICATE_SRC10x2408
+#define MI_PREDICATE_RESULT 0x2418
static void
set_image_compressed_bit(struct anv_cmd_buffer *cmd_buffer,
@@ -545,6 +546,14 @@ mi_alu(uint32_t opcode, uint32_t operand1, uint32_t
operand2)
#define CS_GPR(n) (0x2600 + (n) * 8)
+#if GEN_GEN >= 8 || GEN_IS_HASWELL
+static void
+restore_conditional_render_predicate(struct anv_cmd_buffer *cmd_buffer)
+{
+ emit_lrr(_buffer->batch, MI_PREDICATE_RESULT,
CS_GPR(MI_ALU_REG15));
Does this wo
Re: [Mesa-dev] [PATCH 1/3] anv: Implement VK_KHR_draw_indirect_count for gen 7.5+
On 11/6/18 12:39 AM, Jason Ekstrand wrote:
On Wed, Oct 17, 2018 at 6:59 AM Danylo Piliaiev
wrote:
Signed-off-by: Danylo Piliaiev
---
src/intel/vulkan/anv_extensions.py | 1 +
src/intel/vulkan/genX_cmd_buffer.c | 155 +
2 files changed, 156 insertions(+)
diff --git a/src/intel/vulkan/anv_extensions.py
b/src/intel/vulkan/anv_extensions.py
index d4915c9501..7f44da6648 100644
--- a/src/intel/vulkan/anv_extensions.py
+++ b/src/intel/vulkan/anv_extensions.py
@@ -113,6 +113,7 @@ EXTENSIONS = [
Extension('VK_KHR_xlib_surface', 6,
'VK_USE_PLATFORM_XLIB_KHR'),
Extension('VK_KHR_multiview', 1, True),
Extension('VK_KHR_display', 23,
'VK_USE_PLATFORM_DISPLAY_KHR'),
+Extension('VK_KHR_draw_indirect_count', 1,
'device->info.gen >= 8 || device->info.is_haswell'),
Extension('VK_EXT_acquire_xlib_display', 1,
'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
Extension('VK_EXT_debug_report', 8, True),
Extension('VK_EXT_direct_mode_display', 1,
'VK_USE_PLATFORM_DISPLAY_KHR'),
diff --git a/src/intel/vulkan/genX_cmd_buffer.c
b/src/intel/vulkan/genX_cmd_buffer.c
index 43a02f2256..d7b94efd19 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -2982,6 +2982,161 @@ void genX(CmdDrawIndexedIndirect)(
}
}
+#if GEN_IS_HASWELL || GEN_GEN >= 8
+static void
+emit_draw_count_predicate(struct anv_cmd_buffer *cmd_buffer,
+ struct anv_address count_address,
+ uint32_t draw_index)
+{
+ /* Upload the current draw count from the draw parameters buffer to
+* MI_PREDICATE_SRC0.
+*/
+ emit_lrr(_buffer->batch, MI_PREDICATE_SRC0, CS_GPR(MI_ALU_REG14));
Do we also need to set MI_PREDICATE_SRC0 + 4 to 0? I suspect we do.
Yes.
I'll also recheck other places.
Also, we can likely save some batch space if we have a "prepare" function
which sets MI_PREDICATE_SRC0, SRC0 + 4, and SRC1 + 4 and only emit one
LOAD_REGISTER_IMM and the MI_PREDICATE per-draw. For lots of primitives,
those extra three MI_LOAD_REGISTER_* calls will add up.
Makes sense
+
+ /* Upload the index of the current primitive to MI_PREDICATE_SRC1. */
+ emit_lri(_buffer->batch, MI_PREDICATE_SRC1, draw_index);
+ emit_lri(_buffer->batch, MI_PREDICATE_SRC1 + 4, 0);
+
+ if (draw_index == 0) {
+ anv_batch_emit(_buffer->batch, GENX(MI_PREDICATE), mip) {
+ mip.LoadOperation= LOAD_LOADINV;
+ mip.CombineOperation = COMBINE_SET;
+ mip.CompareOperation = COMPARE_SRCS_EQUAL;
+ }
+ } else {
+ /* While draw_index < draw_count the predicate's result will be
+* (draw_index == draw_count) ^ TRUE = TRUE
+* When draw_index == draw_count the result is
+* (TRUE) ^ TRUE = FALSE
+* After this all results will be:
+* (FALSE) ^ FALSE = FALSE
+*/
+ anv_batch_emit(_buffer->batch, GENX(MI_PREDICATE), mip) {
+ mip.LoadOperation= LOAD_LOAD;
+ mip.CombineOperation = COMBINE_XOR;
+ mip.CompareOperation = COMPARE_SRCS_EQUAL;
+ }
+ }
+}
+
+void genX(CmdDrawIndirectCountKHR)(
+VkCommandBuffer commandBuffer,
+VkBuffer_buffer,
+VkDeviceSizeoffset,
+VkBuffer_countBuffer,
+VkDeviceSizecountBufferOffset,
+uint32_tmaxDrawCount,
+uint32_tstride)
+{
+ ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer);
+ ANV_FROM_HANDLE(anv_buffer, buffer, _buffer);
+ ANV_FROM_HANDLE(anv_buffer, count_buffer, _countBuffer);
+ struct anv_cmd_state *cmd_state = _buffer->state;
+ struct anv_pipeline *pipeline = cmd_state->gfx.base.pipeline;
+ const struct brw_vs_prog_data *vs_prog_data =
get_vs_prog_data(pipeline);
+
+ if (anv_batch_has_error(_buffer->batch))
+ return;
+
+ genX(cmd_buffer_flush_state)(cmd_buffer);
+
+ struct anv_address count_address =
+ anv_address_add(count_buffer->address, countBufferOffset);
+
+ /* Needed to ensure the memory is coherent for the MI_LOAD_REGISTER_MEM
+* command when loading the values into the predicate source registers.
+*/
+ anv_batch_emit(_buffer->batch, GENX(PIPE_CONTROL), pc) {
+ pc.PipeControlFlushEnable = true;
+ }
Have you seen this be an actual problem? If not, why? A documentation
citation would be nice.
You are right - citation is needed:
Volume 7: 3D-Media-GPGPU (Skylake), MI_PREDICATE:
MI_LOAD_REGISTER_MEM commands can be used to load the MItemp0, MItemp1,
and PredicateData registers prior to MI_PREDICATE.
To ensure the memory sources of the MI_LOAD
Re: [Mesa-dev] [PATCH v3] i965: Fix calculation of layers array length for isl_view
Hi Ilia, Jason,
All these cases are indeed could induce confusion,
I'll try to explain all of them:
Texture is not layered:
Layers: 1
Base Layer: %layer of image unit% + %layer of texture view%
Layered 3D texture:
Layers: Entire level is bound, texture view does not affect layers
of it
Base Layer: 0
Layered non 3D texture (Array):
Immutable (it's a texture view, see _mesa_set_texture_view_state):
Layers: correct values of MinLayer and NumLayers is baked into
texObject in texture_view and in _mesa_set_texture_view_state
Base Layer: MinLayer
Mutable (MinLayer is always 0 since it can be set only via texture
view which makes texture immutable):
Layers: array_len
Base Layer: 0
So while the code from my pov should work it could be presented better.
However I'm not sure what will be better:
} else if (obj->Immutable) {
base_layer = obj->MinLayer;
num_layers = obj->NumLayers;
} else {
base_layer = 0;
num_layers = mt->surf.logical_level0_px.array_len;
}
Or always calculate base_layer as "obj->MinLayer + u->_Layer" before the
conditions,
which will look slightly misleading and induce questions of why we
don't subtract them from array_len.
What do you think?
Thanks!
On 11/14/18 12:01 AM, Ilia Mirkin wrote:
On Tue, Nov 13, 2018 at 4:53 PM Jason Ekstrand wrote:
On Mon, Sep 10, 2018 at 10:21 AM Danylo Piliaiev
wrote:
Handle all cases in calculation of layers count for isl_view
taking into account texture view and image unit.
st_convert_image was taken as a reference.
When u->Layered is true the whole level is taken with respect to
image view. In other case only one layer is taken.
v3: (Józef Kucia and Ilia Mirkin)
- Rewrote patch by taking st_convert_image as a reference
- Removed now unused get_image_num_layers function
- Changed commit message
Fixes: 5a8c8903
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
Signed-off-by: Danylo Piliaiev
---
.../drivers/dri/i965/brw_wm_surface_state.c | 32 ++-
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 944762ec46..9bfe6e2037 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1499,18 +1499,6 @@ update_buffer_image_param(struct brw_context *brw,
param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
}
-static unsigned
-get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum target,
- unsigned level)
-{
- if (target == GL_TEXTURE_CUBE_MAP)
- return 6;
-
- return target == GL_TEXTURE_3D ?
- minify(mt->surf.logical_level0_px.depth, level) :
- mt->surf.logical_level0_px.array_len;
-}
-
static void
update_image_surface(struct brw_context *brw,
struct gl_image_unit *u,
@@ -1541,14 +1529,28 @@ update_image_surface(struct brw_context *brw,
} else {
struct intel_texture_object *intel_obj = intel_texture_object(obj);
struct intel_mipmap_tree *mt = intel_obj->mt;
- const unsigned num_layers = u->Layered ?
-get_image_num_layers(mt, obj->Target, u->Level) : 1;
+
+ unsigned base_layer, num_layers;
+ if (u->Layered) {
+if (obj->Target == GL_TEXTURE_3D) {
+ base_layer = 0;
+ num_layers = minify(mt->surf.logical_level0_px.depth, u->Level);
+} else {
+ base_layer = obj->MinLayer;
+ num_layers = obj->Immutable ?
+obj->NumLayers :
+mt->surf.logical_level0_px.array_len;
Doesn't this need to be array_len - base_layer? I'm not sure on the others
without digging.
Probably not intuitively obvious, but MinLayer/NumLayers are only set
for Immutable textures.
-ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v4] i965: Fix calculation of layers array length for isl_view
Handle all cases in calculation of layers count for isl_view
taking into account texture view and image unit.
st_convert_image was taken as a reference.
When u->Layered is true the whole level is taken with respect to
image view. In other case only one layer is taken.
v3: (Józef Kucia and Ilia Mirkin)
- Rewrote patch by taking st_convert_image as a reference
- Removed now unused get_image_num_layers function
- Changed commit message
v4: (Jason Ekstrand)
- Added assert
Fixes: 5a8c8903
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
Signed-off-by: Danylo Piliaiev
Reviewed-by: Jason Ekstrand
---
.../drivers/dri/i965/brw_wm_surface_state.c | 33 ++-
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 8d21cf5fa7..3286c222e5 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1499,18 +1499,6 @@ update_buffer_image_param(struct brw_context *brw,
param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
}
-static unsigned
-get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum target,
- unsigned level)
-{
- if (target == GL_TEXTURE_CUBE_MAP)
- return 6;
-
- return target == GL_TEXTURE_3D ?
- minify(mt->surf.logical_level0_px.depth, level) :
- mt->surf.logical_level0_px.array_len;
-}
-
static void
update_image_surface(struct brw_context *brw,
struct gl_image_unit *u,
@@ -1541,14 +1529,29 @@ update_image_surface(struct brw_context *brw,
} else {
struct intel_texture_object *intel_obj = intel_texture_object(obj);
struct intel_mipmap_tree *mt = intel_obj->mt;
- const unsigned num_layers = u->Layered ?
-get_image_num_layers(mt, obj->Target, u->Level) : 1;
+
+ unsigned base_layer, num_layers;
+ if (u->Layered) {
+if (obj->Target == GL_TEXTURE_3D) {
+ base_layer = 0;
+ num_layers = minify(mt->surf.logical_level0_px.depth, u->Level);
+} else {
+ assert(obj->Immutable || obj->MinLayer == 0);
+ base_layer = obj->MinLayer;
+ num_layers = obj->Immutable ?
+obj->NumLayers :
+mt->surf.logical_level0_px.array_len;
+}
+ } else {
+base_layer = obj->MinLayer + u->_Layer;
+num_layers = 1;
+ }
struct isl_view view = {
.format = format,
.base_level = obj->MinLevel + u->Level,
.levels = 1,
-.base_array_layer = obj->MinLayer + u->_Layer,
+.base_array_layer = base_layer,
.array_len = num_layers,
.swizzle = ISL_SWIZZLE_IDENTITY,
.usage = ISL_SURF_USAGE_STORAGE_BIT,
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 0/2] anv: Implement VK_KHR_draw_indirect_count and VK_EXT_conditional_rendering
This series implement VK_KHR_draw_indirect_count and VK_EXT_conditional_rendering extensions. They are implemented together because they are highly interweaved. There are already tests in VK_CTS for VK_KHR_draw_indirect_count. Tests for VK_EXT_conditional_rendering (https://github.com/KhronosGroup/VK-GL-CTS/pull/131) got merged today. VK_KHR_draw_indirect_count is implemented for gen7+. VK_EXT_conditional_rendering is implemented for gen7.5+ because it requires MI_MATH to be implemented correctly. Many thanks to Jason Ekstrand for the help with the extensions. v2: Since ALU registers are not required for VK_KHR_draw_indirect_count two patches collapsed into one with new title. Danylo Piliaiev (2): anv: Implement VK_KHR_draw_indirect_count for gen 7+ anv: Implement VK_EXT_conditional_rendering for gen 7.5+ src/intel/vulkan/anv_blorp.c | 7 +- src/intel/vulkan/anv_device.c | 12 + src/intel/vulkan/anv_extensions.py | 2 + src/intel/vulkan/anv_private.h | 2 + src/intel/vulkan/genX_cmd_buffer.c | 353 - 5 files changed, 371 insertions(+), 5 deletions(-) -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 1/2] anv: Implement VK_KHR_draw_indirect_count for gen 7+
v2: by Jason Ekstrand
- Move out of the draw loop population of registers
which aren't changed in it
- Remove dependency on ALU registers
- Clarify usage of PIPE_CONTROL
- Without usage of ALU registers patch works for gen7+
Signed-off-by: Danylo Piliaiev
---
I'm uncertain about usage of PIPE_CONTROL with PipeControlFlushEnable here
since before it cmd_buffer_flush_state is called which does a lot of related
things so PIPE_CONTROL may be unnecessary.
src/intel/vulkan/anv_extensions.py | 1 +
src/intel/vulkan/genX_cmd_buffer.c | 156 +
2 files changed, 157 insertions(+)
diff --git a/src/intel/vulkan/anv_extensions.py
b/src/intel/vulkan/anv_extensions.py
index e9afe06bb1..a6a381b1f3 100644
--- a/src/intel/vulkan/anv_extensions.py
+++ b/src/intel/vulkan/anv_extensions.py
@@ -113,6 +113,7 @@ EXTENSIONS = [
Extension('VK_KHR_xlib_surface', 6,
'VK_USE_PLATFORM_XLIB_KHR'),
Extension('VK_KHR_multiview', 1, True),
Extension('VK_KHR_display', 23,
'VK_USE_PLATFORM_DISPLAY_KHR'),
+Extension('VK_KHR_draw_indirect_count', 1, True),
Extension('VK_EXT_acquire_xlib_display', 1,
'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
Extension('VK_EXT_debug_report', 8, True),
Extension('VK_EXT_direct_mode_display', 1,
'VK_USE_PLATFORM_DISPLAY_KHR'),
diff --git a/src/intel/vulkan/genX_cmd_buffer.c
b/src/intel/vulkan/genX_cmd_buffer.c
index ed88157170..4c6bca48f3 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -2989,6 +2989,162 @@ void genX(CmdDrawIndexedIndirect)(
}
}
+static void
+prepare_for_draw_count_predicate(struct anv_cmd_buffer *cmd_buffer,
+ struct anv_address count_address)
+{
+ /* From the Sky Lake PRM Vol 7, MI_PREDICATE:
+*
+*"MI_LOAD_REGISTER_MEM commands can be used to load the
+*MItemp0, MItemp1, and PredicateData registers prior
+*to MI_PREDICATE. To ensure the memory sources of the
+*MI_LOAD_REGISTER_MEM commands are coherent with previous
+*3D_PIPECONTROL store-DWord operations, software can use the
+*new Pipe Control Flush Enable bit in the PIPE_CONTROL command."
+*/
+ anv_batch_emit(_buffer->batch, GENX(PIPE_CONTROL), pc) {
+ pc.PipeControlFlushEnable = true;
+ }
+
+ /* Upload the current draw count from the draw parameters buffer to
+* MI_PREDICATE_SRC0.
+*/
+ emit_lrm(_buffer->batch, MI_PREDICATE_SRC0, count_address);
+ emit_lri(_buffer->batch, MI_PREDICATE_SRC0 + 4, 0);
+
+ emit_lri(_buffer->batch, MI_PREDICATE_SRC1 + 4, 0);
+}
+
+static void
+emit_draw_count_predicate(struct anv_cmd_buffer *cmd_buffer,
+ uint32_t draw_index)
+{
+ /* Upload the index of the current primitive to MI_PREDICATE_SRC1. */
+ emit_lri(_buffer->batch, MI_PREDICATE_SRC1, draw_index);
+
+ if (draw_index == 0) {
+ anv_batch_emit(_buffer->batch, GENX(MI_PREDICATE), mip) {
+ mip.LoadOperation= LOAD_LOADINV;
+ mip.CombineOperation = COMBINE_SET;
+ mip.CompareOperation = COMPARE_SRCS_EQUAL;
+ }
+ } else {
+ /* While draw_index < draw_count the predicate's result will be
+* (draw_index == draw_count) ^ TRUE = TRUE
+* When draw_index == draw_count the result is
+* (TRUE) ^ TRUE = FALSE
+* After this all results will be:
+* (FALSE) ^ FALSE = FALSE
+*/
+ anv_batch_emit(_buffer->batch, GENX(MI_PREDICATE), mip) {
+ mip.LoadOperation= LOAD_LOAD;
+ mip.CombineOperation = COMBINE_XOR;
+ mip.CompareOperation = COMPARE_SRCS_EQUAL;
+ }
+ }
+}
+
+void genX(CmdDrawIndirectCountKHR)(
+VkCommandBuffer commandBuffer,
+VkBuffer_buffer,
+VkDeviceSizeoffset,
+VkBuffer_countBuffer,
+VkDeviceSizecountBufferOffset,
+uint32_tmaxDrawCount,
+uint32_tstride)
+{
+ ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer);
+ ANV_FROM_HANDLE(anv_buffer, buffer, _buffer);
+ ANV_FROM_HANDLE(anv_buffer, count_buffer, _countBuffer);
+ struct anv_cmd_state *cmd_state = _buffer->state;
+ struct anv_pipeline *pipeline = cmd_state->gfx.base.pipeline;
+ const struct brw_vs_prog_data *vs_prog_data = get_vs_prog_data(pipeline);
+
+ if (anv_batch_has_error(_buffer->batch))
+ return;
+
+ genX(cmd_buffer_flush_state)(cmd_buffer);
+
+ struct anv_address count_address =
+ anv_address_add(count_buffer->address, countBufferOffset);
+
+ prepare_for_draw_count_predicate(cmd_buffer, count_address);
[Mesa-dev] [PATCH v2 2/2] anv: Implement VK_EXT_conditional_rendering for gen 7.5+
Conditional rendering affects next functions:
- vkCmdDraw, vkCmdDrawIndexed, vkCmdDrawIndirect, vkCmdDrawIndexedIndirect
- vkCmdDrawIndirectCountKHR, vkCmdDrawIndexedIndirectCountKHR
- vkCmdDispatch, vkCmdDispatchIndirect, vkCmdDispatchBase
- vkCmdClearAttachments
To reduce readings from the memory a result of the condition is calculated
and stored into designated register MI_ALU_REG15.
In current implementation affected functions expect MI_PREDICATE_RESULT
being set before their call so any code which changes the predicate
should restore it with restore_conditional_render_predicate.
An alternative is to restore MI_PREDICATE_RESULT in all affected
functions at their beginning.
v2: by Jason Ekstrand
- Use vk_find_struct_const instead of manually looping
- Move draw count loading to prepare function
- Zero the top 32-bits of MI_ALU_REG15
Signed-off-by: Danylo Piliaiev
---
src/intel/vulkan/anv_blorp.c | 7 +-
src/intel/vulkan/anv_device.c | 12 ++
src/intel/vulkan/anv_extensions.py | 1 +
src/intel/vulkan/anv_private.h | 2 +
src/intel/vulkan/genX_cmd_buffer.c | 215 +++--
5 files changed, 223 insertions(+), 14 deletions(-)
diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
index 478b8e7a3d..157875d16f 100644
--- a/src/intel/vulkan/anv_blorp.c
+++ b/src/intel/vulkan/anv_blorp.c
@@ -1144,8 +1144,11 @@ void anv_CmdClearAttachments(
* trash our depth and stencil buffers.
*/
struct blorp_batch batch;
- blorp_batch_init(_buffer->device->blorp, , cmd_buffer,
-BLORP_BATCH_NO_EMIT_DEPTH_STENCIL);
+ enum blorp_batch_flags flags = BLORP_BATCH_NO_EMIT_DEPTH_STENCIL;
+ if (cmd_buffer->state.conditional_render_enabled) {
+ flags |= BLORP_BATCH_PREDICATE_ENABLE;
+ }
+ blorp_batch_init(_buffer->device->blorp, , cmd_buffer, flags);
for (uint32_t a = 0; a < attachmentCount; ++a) {
if (pAttachments[a].aspectMask & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) {
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index ee35e01332..41563b322a 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -959,6 +959,18 @@ void anv_GetPhysicalDeviceFeatures2(
break;
}
+ case
VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CONDITIONAL_RENDERING_FEATURES_EXT: {
+ VkPhysicalDeviceConditionalRenderingFeaturesEXT *features =
+(VkPhysicalDeviceConditionalRenderingFeaturesEXT*)ext;
+ ANV_FROM_HANDLE(anv_physical_device, pdevice, physicalDevice);
+
+ features->conditionalRendering = pdevice->info.gen >= 8 ||
+ pdevice->info.is_haswell;
+ features->inheritedConditionalRendering = pdevice->info.gen >= 8 ||
+ pdevice->info.is_haswell;
+ break;
+ }
+
default:
anv_debug_ignored_stype(ext->sType);
break;
diff --git a/src/intel/vulkan/anv_extensions.py
b/src/intel/vulkan/anv_extensions.py
index a6a381b1f3..7562c8ccce 100644
--- a/src/intel/vulkan/anv_extensions.py
+++ b/src/intel/vulkan/anv_extensions.py
@@ -129,6 +129,7 @@ EXTENSIONS = [
Extension('VK_EXT_post_depth_coverage', 1, 'device->info.gen
>= 9'),
Extension('VK_EXT_sampler_filter_minmax', 1, 'device->info.gen
>= 9'),
Extension('VK_EXT_calibrated_timestamps', 1, True),
+Extension('VK_EXT_conditional_rendering', 1, 'device->info.gen
>= 8 || device->info.is_haswell'),
Extension('VK_GOOGLE_decorate_string',1, True),
Extension('VK_GOOGLE_hlsl_functionality1',1, True),
]
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index d8a08d9d67..bcd86b8ef8 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -2061,6 +2061,8 @@ struct anv_cmd_state {
*/
bool hiz_enabled;
+ bool conditional_render_enabled;
+
/**
* Array length is anv_cmd_state::pass::attachment_count. Array content is
* valid only when recording a render pass instance.
diff --git a/src/intel/vulkan/genX_cmd_buffer.c
b/src/intel/vulkan/genX_cmd_buffer.c
index 4c6bca48f3..4c001edcd3 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -479,8 +479,9 @@ transition_depth_buffer(struct anv_cmd_buffer *cmd_buffer,
0, 0, 1, hiz_op);
}
-#define MI_PREDICATE_SRC0 0x2400
-#define MI_PREDICATE_SRC1 0x2408
+#define MI_PREDICATE_SRC00x2400
+#define MI_PREDICATE_SRC10x2408
+#define MI_PREDICATE_RESULT 0x2418
static void
set_image_compressed_bit(struct anv_cmd_buffer *cmd_buffer,
@@ -545,6 +546,14 @@ mi_alu(uint32_t opcode, uint32_t operand1, uint32_t
operand2)
#def
[Mesa-dev] [PATCH v3 2/2] anv: Implement VK_EXT_conditional_rendering for gen 7.5+
Conditional rendering affects next functions:
- vkCmdDraw, vkCmdDrawIndexed, vkCmdDrawIndirect, vkCmdDrawIndexedIndirect
- vkCmdDrawIndirectCountKHR, vkCmdDrawIndexedIndirectCountKHR
- vkCmdDispatch, vkCmdDispatchIndirect, vkCmdDispatchBase
- vkCmdClearAttachments
To reduce readings from the memory a result of the condition is calculated
and stored into designated register MI_ALU_REG15.
In current implementation affected functions expect MI_PREDICATE_RESULT
being set before their call so any code which changes the predicate
should restore it with restore_conditional_render_predicate.
An alternative is to restore MI_PREDICATE_RESULT in all affected
functions at their beginning.
v2: by Jason Ekstrand
- Use vk_find_struct_const instead of manually looping
- Move draw count loading to prepare function
- Zero the top 32-bits of MI_ALU_REG15
v3: Apply pipeline flush before accessing conditional buffer
(The issue was found by Samuel Iglesias)
Signed-off-by: Danylo Piliaiev
---
I'm not sure whether VK_ACCESS_CONDITIONAL_RENDERING_READ_BIT_EXT
should be handled in anv_pipe_invalidate_bits_for_access_flags or not.
It would be good if someone could clarify this matter for me.
src/intel/vulkan/anv_blorp.c | 7 +-
src/intel/vulkan/anv_device.c | 12 ++
src/intel/vulkan/anv_extensions.py | 1 +
src/intel/vulkan/anv_private.h | 2 +
src/intel/vulkan/genX_cmd_buffer.c | 217 +++--
5 files changed, 225 insertions(+), 14 deletions(-)
diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
index 478b8e7a3d..157875d16f 100644
--- a/src/intel/vulkan/anv_blorp.c
+++ b/src/intel/vulkan/anv_blorp.c
@@ -1144,8 +1144,11 @@ void anv_CmdClearAttachments(
* trash our depth and stencil buffers.
*/
struct blorp_batch batch;
- blorp_batch_init(_buffer->device->blorp, , cmd_buffer,
-BLORP_BATCH_NO_EMIT_DEPTH_STENCIL);
+ enum blorp_batch_flags flags = BLORP_BATCH_NO_EMIT_DEPTH_STENCIL;
+ if (cmd_buffer->state.conditional_render_enabled) {
+ flags |= BLORP_BATCH_PREDICATE_ENABLE;
+ }
+ blorp_batch_init(_buffer->device->blorp, , cmd_buffer, flags);
for (uint32_t a = 0; a < attachmentCount; ++a) {
if (pAttachments[a].aspectMask & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) {
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index 6b5ba25c6b..2d6420826b 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -966,6 +966,18 @@ void anv_GetPhysicalDeviceFeatures2(
break;
}
+ case
VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CONDITIONAL_RENDERING_FEATURES_EXT: {
+ VkPhysicalDeviceConditionalRenderingFeaturesEXT *features =
+(VkPhysicalDeviceConditionalRenderingFeaturesEXT*)ext;
+ ANV_FROM_HANDLE(anv_physical_device, pdevice, physicalDevice);
+
+ features->conditionalRendering = pdevice->info.gen >= 8 ||
+ pdevice->info.is_haswell;
+ features->inheritedConditionalRendering = pdevice->info.gen >= 8 ||
+ pdevice->info.is_haswell;
+ break;
+ }
+
default:
anv_debug_ignored_stype(ext->sType);
break;
diff --git a/src/intel/vulkan/anv_extensions.py
b/src/intel/vulkan/anv_extensions.py
index 5cb1d517a3..578a462abd 100644
--- a/src/intel/vulkan/anv_extensions.py
+++ b/src/intel/vulkan/anv_extensions.py
@@ -130,6 +130,7 @@ EXTENSIONS = [
Extension('VK_EXT_post_depth_coverage', 1, 'device->info.gen
>= 9'),
Extension('VK_EXT_sampler_filter_minmax', 1, 'device->info.gen
>= 9'),
Extension('VK_EXT_calibrated_timestamps', 1, True),
+Extension('VK_EXT_conditional_rendering', 1, 'device->info.gen
>= 8 || device->info.is_haswell'),
Extension('VK_GOOGLE_decorate_string',1, True),
Extension('VK_GOOGLE_hlsl_functionality1',1, True),
]
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index 62c563294f..cd0283e802 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -2061,6 +2061,8 @@ struct anv_cmd_state {
*/
bool hiz_enabled;
+ bool conditional_render_enabled;
+
/**
* Array length is anv_cmd_state::pass::attachment_count. Array content is
* valid only when recording a render pass instance.
diff --git a/src/intel/vulkan/genX_cmd_buffer.c
b/src/intel/vulkan/genX_cmd_buffer.c
index 3ec10766ea..3f4b44903f 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -479,8 +479,9 @@ transition_depth_buffer(struct anv_cmd_buffer *cmd_buffer,
0, 0, 1, hiz_op);
}
-#define MI_PREDICATE_SRC0 0x2400
-#define M
Re: [Mesa-dev] [PATCH 1/4] nir: add if opt opt_if_loop_last_continue()
Thanks! Looks much better than my attempt.
Series overall work as expected and eliminate the loop from the
bug: https://bugs.freedesktop.org/show_bug.cgi?id=32211
Also found two typos:
On 11/28/18 5:25 AM, Timothy Arceri wrote:
From: Danylo Piliaiev
Removing the last continue can allow more loops to unroll. Also
inserting code into the if branch can allow the various if opts
to progress further.
The insertion of some loops into the if branch also reduces VGPR
use in some shaders.
vkpipeline-db results (VEGA):
Totals from affected shaders:
SGPRS: 6552 -> 6576 (0.37 %)
VGPRS: 6544 -> 6532 (-0.18 %)
Spilled SGPRs: 0 -> 0 (0.00 %)
Spilled VGPRs: 0 -> 0 (0.00 %)
Private memory VGPRs: 0 -> 0 (0.00 %)
Scratch size: 0 -> 0 (0.00 %) dwords per thread
Code Size: 481952 -> 478032 (-0.81 %) bytes
LDS: 13 -> 13 (0.00 %) blocks
Max Waves: 241 -> 242 (0.41 %)
Wait states: 0 -> 0 (0.00 %)
Shader-db results radeonsi (VEGA):
Totals from affected shaders:
SGPRS: 168 -> 168 (0.00 %)
VGPRS: 144 -> 140 (-2.78 %)
Spilled SGPRs: 157 -> 157 (0.00 %)
Spilled VGPRs: 0 -> 0 (0.00 %)
Private memory VGPRs: 0 -> 0 (0.00 %)
Scratch size: 0 -> 0 (0.00 %) dwords per thread
Code Size: 8524 -> 8488 (-0.42 %) bytes
LDS: 0 -> 0 (0.00 %) blocks
Max Waves: 7 -> 7 (0.00 %)
Wait states: 0 -> 0 (0.00 %)
v2: (Timothy Arceri):
- allow for continues in either branch
- move any trailing loops inside the if as well as blocks.
- leave nir_opt_trivial_continues() to actually remove the
continue.
Signed-off-by: Timothy Arceri
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=32211
---
src/compiler/nir/nir_opt_if.c | 95 +++
1 file changed, 95 insertions(+)
diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index dd488b1787..4a9dffb782 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -263,6 +263,100 @@ rewrite_phi_predecessor_blocks(nir_if *nif,
}
}
+static bool
+nir_block_ends_in_continue(nir_block *block)
+{
+ if (exec_list_is_empty(>instr_list))
+ return false;
+
+ nir_instr *instr = nir_block_last_instr(block);
+ return instr->type == nir_instr_type_jump &&
+ nir_instr_as_jump(instr)->type == nir_jump_continue;
+}
+
+/**
+ * This optimization turns:
+ *
+ * loop {
+ *...
+ *if (cond) {
+ * do_work_1();
+ * continue;
+ *} else {
+ *}
+ *do_work_2();
+ * }
+ *
+ * into:
+ *
+ * loop {
+ *...
+ *if (cond) {
+ * do_work_1();
+ * continue;
+ *} else {
+ * do_work_2();
+ *}
+ * }
+ *
+ * The continue should then be removed by nir_opt_trivial_continues() and the
+ * loop can potentially be unrolled.
+ *
+ * Note: do_work_2() is only ever blocks and nested loops. We could also nest
+ * other if-statments in the branch which would allow further continues to
s/statments/statements
+ * be removed. However in practice this can result in increased register
+ * pressure.
+ */
+static bool
+opt_if_loop_last_continue(nir_loop *loop)
+{
+ /* Get the last if-stament in the loop */
s/stament/statement
+ nir_block *last_block = nir_loop_last_block(loop);
+ nir_cf_node *if_node = nir_cf_node_prev(_block->cf_node);
+ while (if_node) {
+ if (if_node->type == nir_cf_node_if)
+ break;
+
+ if_node = nir_cf_node_prev(if_node);
+ }
+
+ if (!if_node || if_node->type != nir_cf_node_if)
+ return false;
+
+ nir_if *nif = nir_cf_node_as_if(if_node);
+ nir_block *then_block = nir_if_last_then_block(nif);
+ nir_block *else_block = nir_if_last_else_block(nif);
+
+ bool then_ends_in_continue = nir_block_ends_in_continue(then_block);
+ bool else_ends_in_continue = nir_block_ends_in_continue(else_block);
+
+ /* If both branches end in a continue do nothing, this should be handled
+* by nir_opt_dead_cf().
+*/
+ if (then_ends_in_continue && else_ends_in_continue)
+ return false;
+
+ if (!then_ends_in_continue && !else_ends_in_continue)
+ return false;
+
+ /* Move the last block of the loop inside the last if-statement */
+ nir_cf_list tmp;
+ nir_cf_extract(, nir_after_cf_node(if_node),
+nir_after_block(last_block));
+ if (then_ends_in_continue) {
+ nir_cf_reinsert(, nir_after_cf_list(>else_list));
+ } else {
+ nir_cf_reinsert(, nir_after_cf_list(>then_list));
+ }
+
+ /* In order to avoid running nir_lower_regs_to_ssa_impl() every time an if
+* opt makes progress we leave nir_opt_trivial_continues() to remove the
+* continue now that the end of the loop has been simplified.
+*/
+
+ return true;
+}
+
/**
* This optimization turns:
*
@@ -700,6 +794,7 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
nir_loop *loop = nir_cf_node_as_loop(cf_
Re: [Mesa-dev] Thoughts after hitting 100 merge requests?
My small thoughts/questions: - First of all discussions are really much more convenient. - Several (mine) merge requests were "Closed" and merged (not just merged, they are under "Closed" category), am I missing something? - Is there a way to grant rights to creator of merge request to add/change tags if he doesn't have "Developer" role? - Maybe adding more tags/more granular tags would be a good idea. - Could Intel CI be integrated in some way with gitlab? Overall as someone who didn't interact with mailing lists workflow much Gitlab is definitely a win. On 1/11/19 6:57 PM, Jason Ekstrand wrote: All, The mesa project has now hit 100 merge requests (36 are still open). I (and I'm sure others) would be curious to hear people's initial thoughts on the process. What's working well? What's not working? Is it total fail and should we go back to mailing lists? --Jason ___ 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 0/3] anv: Implement VK_KHR_draw_indirect_count and VK_EXT_conditional_rendering
Hi, Thanks! I finally got CI results with ALL relevant tests and my implementation works on everything but Haswell. So I'll fix whatever is wrong there (hope that's just something small) and send the final version. There are still some tests for VK-GL-CTS which are not merged in master e.g. https://github.com/KhronosGroup/VK-GL-CTS/pull/136 and there are some unpublished tests for VK_EXT_conditional_rendering from other devs which I have no idea when will be merged. I don't think this will prevent merging my patches but still I would be more confident if CI have ALL tests. And yes, I also was quite slow with my patches. On 1/8/19 11:13 AM, Erik Faye-Lund wrote: On Wed, 2018-10-17 at 14:58 +0300, Danylo Piliaiev wrote: This series implement VK_KHR_draw_indirect_count and VK_EXT_conditional_rendering extensions. They are implemented together because they are highly interweaved. There are already tests in VK_CTS for VK_KHR_draw_indirect_count and I made a pull request with the tests for VK_EXT_conditional_rendering ( https://github.com/KhronosGroup/VK-GL-CTS/pull/131). VK_KHR_draw_indirect_count is implemented for gen7+. VK_EXT_conditional_rendering is implemented for gen7.5+ because it requires MI_MATH to be implemented correctly. Since part of the tests aren't in VK-GL-CTS master I'm not sure how to test the implementation of VK_EXT_conditional_rendering with my tests on CI. Could anyone help me with this? Also the one thing I'm uncertain of is described in the last patch. Many thanks to Jason Ekstrand for the help with the extensions. Hey, great work :) What's holding this series back? I'm very interested in this functionality due to my work on Zink... :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Implementation of VK_KHR_draw_indirect_count extension for anv
Hi, Thank you for the directions! On 9/12/18 6:13 PM, Jason Ekstrand wrote: Danylo, You're free to implement anything not already implemented. Here are some other (probably simpler) extensions that I think can be reasonably implemented on Intel HW: - VK_EXT_conservative_rasterization - VK_EXT_conditional_render Didn't see them, will take closer look later. As far as VK_KHR_draw_indirect_count go, I haven't implemented it yet because the "proper" implementation is actually kind-of painful though not impossible. In general, there are two ways it can be done: ## 1. The cheap and easy way The spec explicitly allows for the cheap and easy way by requiring the caller to pass in a maxDrawCount. The idea here would be to emit maxDrawCount draw calls only have each one of them predicated on draw_id < draw_count_from_buffer. This one probably wouldn't take much to wire up but it does mean doing maxDrawCount 3DPRIMITIVE commands no matter how many of them are actually needed. I saw such implementation for i965, looked straightforward and I thought it will easily translate into Vulkan implementation. Didn't know that it's possible to do it other way on Intel. ## 2. The hard but maybe more correct way The Intel command streamer does have the ability, if used carefully, to loop. The difficulty here isn't in looping; that can be done fairly easily on gen8+ by emitting a predicated MI_BATCH_BUFFER_START that's predicated off of the looping condition which jumps to the top of the loop. The real difficult bit is taking your loop counter and using it to indirectly access the array of draw information. In order to do this, you have to have a self-modifying batch buffer. In short, you would emit MI commands which read the draw information into registers and also emit MI commands (which would probably come before the first set) which write the actual address into the location in the batch where the first set of MI commands has their address to read from. This would be a painful to debug mess of GPU hangs but could actually be kind-of fun to implement. The correct way looks interesting, I'll need some time to understand details. I hope I haven't scarred you away from working on anv; I just wanted to make it clear what you're getting yourself into. Both ways are totally implementable and I think you'd pretty much have to do the first method on gen7 if we really care about supporting it there. The second is totally doable, it'll just involve some headaches when it's broken. If you want to continue with this project after reading my scarry e-mail, I recommend starting with method 1 to get your feet wet and then we can look into method 2 once you have that working. I'll follow your recommendation and will start from the first method. - Danil --Jason On Wed, Sep 12, 2018 at 6:36 AM Danylo Piliaiev mailto:danylo.pilia...@gmail.com>> wrote: Hello everyone, I would like to try to implement one of the Vulkan extensions - VK_KHR_draw_indirect_count for anv, unless someone is already working on it. It's a relatively minor extension and I saw that the same functionality is already implemented for ARB_indirect_parameters in i965. Also I would appreciate any tips if there are any known possible tricky parts. - Danil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org <mailto: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] Implementation of VK_KHR_draw_indirect_count extension for anv
Hello everyone, I would like to try to implement one of the Vulkan extensions - VK_KHR_draw_indirect_count for anv, unless someone is already working on it. It's a relatively minor extension and I saw that the same functionality is already implemented for ARB_indirect_parameters in i965. Also I would appreciate any tips if there are any known possible tricky parts. - Danil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Implementation of VK_KHR_draw_indirect_count extension for anv
On 9/17/18 5:34 PM, Jason Ekstrand wrote:
On Mon, Sep 17, 2018 at 8:34 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>> wrote:
Hi Jason,
I have implemented the extension and it works, however before
sending the patch I decided to see how it can interact with other
extension - VK_EXT_conditional_render
and got confused:
From the spec it is not disallowed to call functions of
VK_KHR_draw_indirect_count in conditional rendering block. So
let's say that predicate of conditional rendering
will result in FALSE, we call vkCmdDrawIndirectCountKHR which sees
that there is already a predicate emitted and it should be taken
into account, since it will be FALSE
all next predicates should result in FALSE. The issue is that I
don't see an easy way to do this.
My current implementation uses the next predicate (it is same as
in GL implementation):
/* While draw_index < maxDrawCount the predicate's result
will be
* (draw_index == maxDrawCount) ^ TRUE = TRUE
* When draw_index == maxDrawCount the result is
* (TRUE) ^ TRUE = FALSE
* After this all results will be:
* (FALSE) ^ FALSE = FALSE
*/
anv_batch_emit(_buffer->batch, GENX(MI_PREDICATE), mip) {
mip.LoadOperation = LOAD_LOAD;
mip.CombineOperation = COMBINE_XOR;
mip.CompareOperation = COMPARE_SRCS_EQUAL;
}
But if the initial predicate state is FALSE then when draw_index
equals maxDrawCount the result will be
(FALSE) ^ TRUE = TRUE
Which isn't something we want. But without "not equal" operation
or without MI_MATH I don't see how to fix this.
First off, thanks for looking into the combination of these two
features. Getting them to work together nicely is half of the
difficulty of these two extensions.
On platforms which support MI_MATH, I think we're probably better off
just using it. For Ivy Bridge, the only thing I could think to do
when both are in use would be to do two MI_PREDICATEs for every draw
call. The first would be what you describe above and the second would
be the MI_PREDICATE for the conditional render with COMBINE_AND. When
the condition is true, the AND would have no effect and you would get
the behavior above. If the condition is false, the above logic for
implementing draw_indirect_count wouldn't matter because it would get
ANDed with false. On Haswell and later, it's likely more efficient to
just use MI_MATH and avoid re-loading the draw count and condition on
every draw call. (We could just leave the draw count in CS_GPR0, for
instance.) Does that work?
Looks like a plan. I'll try to go this path.
Also there is another interaction which wasn't thought of before:
Several vkCmdDrawIndirectCountKHR in conditional render block but using
MI_MATH should solve it.
Since you're already looking at it, it may be best to implement the
two extensions together as one patch series so we can be sure we have
the interactions right. If we can't get them to play nicely together,
we may have to disable one of them on Ivy Bridge and I'd rather not
enable an extension and then take the functionality away later.
I agree, the extensions are too interweaved which I realized when
implemented the most basic version of EXT_conditional_render. I'll also
make sure to test all of these on Ivy Bridge.
I don't see anything related in Vulkan or GL specs neither I see
anything in Piglit and CTS tests.
Maybe I'm missing something obvious, could you help me here?
There's nothing preventing the two from being used together. If we
don't have piglit tests that exercise the GL versions together, that
would be bad. Have you found good Vulkan CTS tests for either of
those two extensions? VK_KHR_indirect_count should have tests since
it's a KHR extension but we may need to write the tests for
EXT_conditional_render.
There are no tests of how these features work together in Piglit or
Vulkan CTS. Also my previous observations are true for GL so it also
ought to be fixed (I'll write a test for Piglit first to confirm this).
There are tests for VK_KHR_indirect_count in Vulkan CTS which my current
implementation passes. There aren't any tests for EXT_conditional_render
however I used an example from https://github.com/SaschaWillems/Vulkan
to test my initial implementation of it.
Should tests for EXT_conditional_render go into Vulkan CTS?
Also since the scope of the work grew quite a lot and I'll be soon on
vacation the implementation won't be ready until at least second week of
October (just making sure no one will think I ran away scared =) )
--Jason
You can find current implementation in
https://gitlab.freedesktop.org/GL/mesa/commit/9d1c7ae0db618c6f7281d5f667c96612ff0bb2c2
- Danil
On 9/12/18 6:30 PM, Danylo Piliaiev wrote:
Hi,
Thank
Re: [Mesa-dev] Implementation of VK_KHR_draw_indirect_count extension for anv
On 9/17/18 7:03 PM, Jason Ekstrand wrote:
On Mon, Sep 17, 2018 at 10:08 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>> wrote:
On 9/17/18 5:34 PM, Jason Ekstrand wrote:
On Mon, Sep 17, 2018 at 8:34 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>> wrote:
Hi Jason,
I have implemented the extension and it works, however before
sending the patch I decided to see how it can interact with
other extension - VK_EXT_conditional_render
and got confused:
From the spec it is not disallowed to call functions of
VK_KHR_draw_indirect_count in conditional rendering block. So
let's say that predicate of conditional rendering
will result in FALSE, we call vkCmdDrawIndirectCountKHR which
sees that there is already a predicate emitted and it should
be taken into account, since it will be FALSE
all next predicates should result in FALSE. The issue is that
I don't see an easy way to do this.
My current implementation uses the next predicate (it is same
as in GL implementation):
/* While draw_index < maxDrawCount the predicate's
result will be
* (draw_index == maxDrawCount) ^ TRUE = TRUE
* When draw_index == maxDrawCount the result is
* (TRUE) ^ TRUE = FALSE
* After this all results will be:
* (FALSE) ^ FALSE = FALSE
*/
anv_batch_emit(_buffer->batch, GENX(MI_PREDICATE), mip) {
mip.LoadOperation = LOAD_LOAD;
mip.CombineOperation = COMBINE_XOR;
mip.CompareOperation = COMPARE_SRCS_EQUAL;
}
But if the initial predicate state is FALSE then when
draw_index equals maxDrawCount the result will be
(FALSE) ^ TRUE = TRUE
Which isn't something we want. But without "not equal"
operation or without MI_MATH I don't see how to fix this.
First off, thanks for looking into the combination of these two
features. Getting them to work together nicely is half of the
difficulty of these two extensions.
On platforms which support MI_MATH, I think we're probably better
off just using it. For Ivy Bridge, the only thing I could think
to do when both are in use would be to do two MI_PREDICATEs for
every draw call. The first would be what you describe above and
the second would be the MI_PREDICATE for the conditional render
with COMBINE_AND. When the condition is true, the AND would have
no effect and you would get the behavior above. If the condition
is false, the above logic for implementing draw_indirect_count
wouldn't matter because it would get ANDed with false. On
Haswell and later, it's likely more efficient to just use MI_MATH
and avoid re-loading the draw count and condition on every draw
call. (We could just leave the draw count in CS_GPR0, for
instance.) Does that work?
Looks like a plan. I'll try to go this path.
Also there is another interaction which wasn't thought of before:
Several vkCmdDrawIndirectCountKHR in conditional render block but
using MI_MATH should solve it.
In that case, we'll have to basically re-do the conditional bit for
every draw call. There may be some interesting interactions here with
secondary command buffers as well. I don't remember what we decided
about inheriting conditions in secondaries. Again, if we decide we
need MI_MATH, then we'll just drop support for one or both extensions
on Ivy Bridge.
About the secondary command buffers:
If inherited conditional rendering
<https://www.khronos.org/registry/vulkan/specs/1.1-extensions/html/vkspec.html#features-features-inheritedConditionalRendering>
is supported it means that secondary buffers can be executed inside
conditional rendering block and commands which can be affected by
conditional rendering are affected by it in secondary buffer and also in
primary, is it right?
However at this point the secondary buffer is already composed without
commands for conditions and since our implementation depends on commands
emitted to the buffer making its commands to depend on condition either
highly tricky to do (secondary buffer needs to have certain points where
to inject conditions?) or just impossible.
And this secondary buffer may have been formed inside conditional render
block so they could be affected by two conditions if I understand correctly.
Is is doable to implement?
Since you're already looking at it, it may be best to implement
the two extensions together as one patch series so we can be sure
we have the interactions right. If we can't get them to play
nicely together, we may have to disable one of them on Ivy Bridge
and I'd rather not enable an extension and then take the
functionality away l
Re: [Mesa-dev] [PATCH 1/2] glsl: Avoid propagating incompatible type of initializer
Hi,
Thank you for the review.
On 9/15/18 2:15 AM, Timothy Arceri wrote:
Series:
Reviewed-by: Timothy Arceri
Are there piglit tests to go with this?
Yes, there is a test "[PATCH] glsl-1.10: add a
'initialization-incompatible-type-propagation' test":
https://patchwork.freedesktop.org/patch/244676/
On 15/8/18 10:46 pm, Danylo Piliaiev wrote:
do_assignment validated assigment but when rhs type was not compatible
it proceeded without issues and returned error_emitted = false.
On the other hand process_initializer expected do_assignment to always
return compatible type and never fail.
As a result when variable was initialized with incompatible type
the type of variable changed to the incompatible one.
This manifested in unnecessary error messages and in one case in crash.
Example GLSL:
vec4 tmp = vec2(0.0);
tmp.z -= 1.0;
Past error messages:
initializer of type vec2 cannot be assigned to variable of type vec4
invalid swizzle / mask `z'
type mismatch
operands to arithmetic operators must be numeric
After this patch:
initializer of type vec2 cannot be assigned to variable of type vec4
In the other case when we initialize variable with incompatible struct,
accessing variable's field leaded to a crash. Example:
uniform struct {float field;} data;
...
vec4 tmp = data;
tmp.x -= 1.0;
After the patch there is only error line without a crash:
initializer of type #anon_struct cannot be assigned to variable of
type vec4
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107547
Signed-off-by: Danylo Piliaiev
---
src/compiler/glsl/ast_to_hir.cpp | 62 +---
1 file changed, 33 insertions(+), 29 deletions(-)
diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index 5d3f10b682..93e7c8ec33 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -1012,6 +1012,8 @@ do_assignment(exec_list *instructions, struct
_mesa_glsl_parse_state *state,
mark_whole_array_access(rhs);
mark_whole_array_access(lhs);
}
+ } else {
+ error_emitted = true;
}
/* Most callers of do_assignment (assign, add_assign,
pre_inc/dec,
@@ -4562,41 +4564,43 @@ process_initializer(ir_variable *var,
ast_declaration *decl,
/* Never emit code to initialize a uniform.
*/
const glsl_type *initializer_type;
+ bool error_emitted = false;
if (!type->qualifier.flags.q.uniform) {
- do_assignment(initializer_instructions, state,
- NULL,
- lhs, rhs,
- , true,
- true,
- type->get_location());
+ error_emitted =
+ do_assignment(initializer_instructions, state,
+ NULL, lhs, rhs,
+ , true, true,
+ type->get_location());
initializer_type = result->type;
} else
initializer_type = rhs->type;
- var->constant_initializer =
rhs->constant_expression_value(mem_ctx);
- var->data.has_initializer = true;
+ if (!error_emitted) {
+ var->constant_initializer =
rhs->constant_expression_value(mem_ctx);
+ var->data.has_initializer = true;
- /* If the declared variable is an unsized array, it must
inherrit
- * its full type from the initializer. A declaration such as
- *
- * uniform float a[] = float[](1.0, 2.0, 3.0, 3.0);
- *
- * becomes
- *
- * uniform float a[4] = float[](1.0, 2.0, 3.0, 3.0);
- *
- * The assignment generated in the if-statement (below) will also
- * automatically handle this case for non-uniforms.
- *
- * If the declared variable is not an array, the types must
- * already match exactly. As a result, the type assignment
- * here can be done unconditionally. For non-uniforms the call
- * to do_assignment can change the type of the initializer (via
- * the implicit conversion rules). For uniforms the initializer
- * must be a constant expression, and the type of that expression
- * was validated above.
- */
- var->type = initializer_type;
+ /* If the declared variable is an unsized array, it must
inherrit
+ * its full type from the initializer. A declaration such as
+ *
+ * uniform float a[] = float[](1.0, 2.0, 3.0, 3.0);
+ *
+ * becomes
+ *
+ * uniform float a[4] = float[](1.0, 2.0, 3.0, 3.0);
+ *
+ * The assignment generated in the if-statement (below) will
also
+ * automatically handle this case for non-uniforms.
+ *
+ * If the declared variable is not an array, the types must
+ * already match exactly. As a result, the type assignment
+ * here can be do
Re: [Mesa-dev] Implementation of VK_KHR_draw_indirect_count extension for anv
Hi Jason,
I have implemented the extension and it works, however before sending
the patch I decided to see how it can interact with other extension -
VK_EXT_conditional_render
and got confused:
From the spec it is not disallowed to call functions of
VK_KHR_draw_indirect_count in conditional rendering block. So let's say
that predicate of conditional rendering
will result in FALSE, we call vkCmdDrawIndirectCountKHR which sees that
there is already a predicate emitted and it should be taken into
account, since it will be FALSE
all next predicates should result in FALSE. The issue is that I don't
see an easy way to do this.
My current implementation uses the next predicate (it is same as in GL
implementation):
/* While draw_index < maxDrawCount the predicate's result will be
* (draw_index == maxDrawCount) ^ TRUE = TRUE
* When draw_index == maxDrawCount the result is
* (TRUE) ^ TRUE = FALSE
* After this all results will be:
* (FALSE) ^ FALSE = FALSE
*/
anv_batch_emit(_buffer->batch, GENX(MI_PREDICATE), mip) {
mip.LoadOperation = LOAD_LOAD;
mip.CombineOperation = COMBINE_XOR;
mip.CompareOperation = COMPARE_SRCS_EQUAL;
}
But if the initial predicate state is FALSE then when draw_index equals
maxDrawCount the result will be
(FALSE) ^ TRUE = TRUE
Which isn't something we want. But without "not equal" operation or
without MI_MATH I don't see how to fix this.
I don't see anything related in Vulkan or GL specs neither I see
anything in Piglit and CTS tests.
Maybe I'm missing something obvious, could you help me here?
You can find current implementation in
https://gitlab.freedesktop.org/GL/mesa/commit/9d1c7ae0db618c6f7281d5f667c96612ff0bb2c2
- Danil
On 9/12/18 6:30 PM, Danylo Piliaiev wrote:
Hi,
Thank you for the directions!
On 9/12/18 6:13 PM, Jason Ekstrand wrote:
Danylo,
You're free to implement anything not already implemented. Here are
some other (probably simpler) extensions that I think can be
reasonably implemented on Intel HW:
- VK_EXT_conservative_rasterization
- VK_EXT_conditional_render
Didn't see them, will take closer look later.
As far as VK_KHR_draw_indirect_count go, I haven't implemented it yet
because the "proper" implementation is actually kind-of painful
though not impossible. In general, there are two ways it can be done:
## 1. The cheap and easy way
The spec explicitly allows for the cheap and easy way by requiring
the caller to pass in a maxDrawCount. The idea here would be to emit
maxDrawCount draw calls only have each one of them predicated on
draw_id < draw_count_from_buffer. This one probably wouldn't take
much to wire up but it does mean doing maxDrawCount 3DPRIMITIVE
commands no matter how many of them are actually needed.
I saw such implementation for i965, looked straightforward and I
thought it will easily translate into Vulkan implementation. Didn't
know that it's possible to do it other way on Intel.
## 2. The hard but maybe more correct way
The Intel command streamer does have the ability, if used carefully,
to loop. The difficulty here isn't in looping; that can be done
fairly easily on gen8+ by emitting a predicated MI_BATCH_BUFFER_START
that's predicated off of the looping condition which jumps to the top
of the loop. The real difficult bit is taking your loop counter and
using it to indirectly access the array of draw information. In
order to do this, you have to have a self-modifying batch buffer. In
short, you would emit MI commands which read the draw information
into registers and also emit MI commands (which would probably come
before the first set) which write the actual address into the
location in the batch where the first set of MI commands has their
address to read from. This would be a painful to debug mess of GPU
hangs but could actually be kind-of fun to implement.
The correct way looks interesting, I'll need some time to understand
details.
I hope I haven't scarred you away from working on anv; I just wanted
to make it clear what you're getting yourself into. Both ways are
totally implementable and I think you'd pretty much have to do the
first method on gen7 if we really care about supporting it there.
The second is totally doable, it'll just involve some headaches when
it's broken. If you want to continue with this project after reading
my scarry e-mail, I recommend starting with method 1 to get your feet
wet and then we can look into method 2 once you have that working.
I'll follow your recommendation and will start from the first method.
- Danil
--Jason
On Wed, Sep 12, 2018 at 6:36 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>> wrote:
Hello everyone,
I would like to try to implement one of the Vulkan extensions -
VK_KHR_draw_indirect_count for anv,
unless someone is already working on it.
It's a r
Re: [Mesa-dev] Implementation of VK_KHR_draw_indirect_count extension for anv
On 9/18/18 1:08 PM, Jason Ekstrand wrote:
On Tue, Sep 18, 2018 at 4:08 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>> wrote:
On 9/17/18 7:03 PM, Jason Ekstrand wrote:
On Mon, Sep 17, 2018 at 10:08 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>> wrote:
On 9/17/18 5:34 PM, Jason Ekstrand wrote:
On Mon, Sep 17, 2018 at 8:34 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>> wrote:
Hi Jason,
I have implemented the extension and it works, however
before sending the patch I decided to see how it can
interact with other extension - VK_EXT_conditional_render
and got confused:
From the spec it is not disallowed to call functions of
VK_KHR_draw_indirect_count in conditional rendering
block. So let's say that predicate of conditional rendering
will result in FALSE, we call vkCmdDrawIndirectCountKHR
which sees that there is already a predicate emitted and
it should be taken into account, since it will be FALSE
all next predicates should result in FALSE. The issue is
that I don't see an easy way to do this.
My current implementation uses the next predicate (it is
same as in GL implementation):
/* While draw_index < maxDrawCount the
predicate's result will be
* (draw_index == maxDrawCount) ^ TRUE = TRUE
* When draw_index == maxDrawCount the result is
* (TRUE) ^ TRUE = FALSE
* After this all results will be:
* (FALSE) ^ FALSE = FALSE
*/
anv_batch_emit(_buffer->batch, GENX(MI_PREDICATE),
mip) {
mip.LoadOperation = LOAD_LOAD;
mip.CombineOperation = COMBINE_XOR;
mip.CompareOperation = COMPARE_SRCS_EQUAL;
}
But if the initial predicate state is FALSE then when
draw_index equals maxDrawCount the result will be
(FALSE) ^ TRUE = TRUE
Which isn't something we want. But without "not equal"
operation or without MI_MATH I don't see how to fix this.
First off, thanks for looking into the combination of these
two features. Getting them to work together nicely is half
of the difficulty of these two extensions.
On platforms which support MI_MATH, I think we're probably
better off just using it. For Ivy Bridge, the only thing I
could think to do when both are in use would be to do two
MI_PREDICATEs for every draw call. The first would be what
you describe above and the second would be the MI_PREDICATE
for the conditional render with COMBINE_AND. When the
condition is true, the AND would have no effect and you
would get the behavior above. If the condition is false,
the above logic for implementing draw_indirect_count
wouldn't matter because it would get ANDed with false. On
Haswell and later, it's likely more efficient to just use
MI_MATH and avoid re-loading the draw count and condition on
every draw call. (We could just leave the draw count in
CS_GPR0, for instance.) Does that work?
Looks like a plan. I'll try to go this path.
Also there is another interaction which wasn't thought of before:
Several vkCmdDrawIndirectCountKHR in conditional render block
but using MI_MATH should solve it.
In that case, we'll have to basically re-do the conditional bit
for every draw call. There may be some interesting interactions
here with secondary command buffers as well. I don't remember
what we decided about inheriting conditions in secondaries.
Again, if we decide we need MI_MATH, then we'll just drop support
for one or both extensions on Ivy Bridge.
About the secondary command buffers:
If inherited conditional rendering
<https://www.khronos.org/registry/vulkan/specs/1.1-extensions/html/vkspec.html#features-features-inheritedConditionalRendering>
is supported it means that secondary buffers can be executed
inside conditional rendering block and commands which can be
affected by conditional rendering are affected by it in secondary
buffer and also in primary, is it right?
However at this point the secondary buffer is already composed
without commands for conditions and since our implementation
depends on commands emitted to the buffer making its commands to
depend on condition either highly tricky to do (secondary buffer
needs to have certain points where to inject conditions?) or just
impossible.
And this secondary buffer may have been formed inside conditional
render block so they could be affect
Re: [Mesa-dev] Implementation of VK_KHR_draw_indirect_count extension for anv
On 9/18/18 1:46 PM, Danylo Piliaiev wrote:
On 9/18/18 1:08 PM, Jason Ekstrand wrote:
On Tue, Sep 18, 2018 at 4:08 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>> wrote:
On 9/17/18 7:03 PM, Jason Ekstrand wrote:
On Mon, Sep 17, 2018 at 10:08 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>>
wrote:
On 9/17/18 5:34 PM, Jason Ekstrand wrote:
On Mon, Sep 17, 2018 at 8:34 AM Danylo Piliaiev
mailto:danylo.pilia...@gmail.com>> wrote:
Hi Jason,
I have implemented the extension and it works, however
before sending the patch I decided to see how it can
interact with other extension - VK_EXT_conditional_render
and got confused:
From the spec it is not disallowed to call functions of
VK_KHR_draw_indirect_count in conditional rendering
block. So let's say that predicate of conditional rendering
will result in FALSE, we call vkCmdDrawIndirectCountKHR
which sees that there is already a predicate emitted
and it should be taken into account, since it will be FALSE
all next predicates should result in FALSE. The issue
is that I don't see an easy way to do this.
My current implementation uses the next predicate (it
is same as in GL implementation):
/* While draw_index < maxDrawCount the
predicate's result will be
* (draw_index == maxDrawCount) ^ TRUE = TRUE
* When draw_index == maxDrawCount the result is
* (TRUE) ^ TRUE = FALSE
* After this all results will be:
* (FALSE) ^ FALSE = FALSE
*/
anv_batch_emit(_buffer->batch, GENX(MI_PREDICATE),
mip) {
mip.LoadOperation = LOAD_LOAD;
mip.CombineOperation = COMBINE_XOR;
mip.CompareOperation = COMPARE_SRCS_EQUAL;
}
But if the initial predicate state is FALSE then when
draw_index equals maxDrawCount the result will be
(FALSE) ^ TRUE = TRUE
Which isn't something we want. But without "not equal"
operation or without MI_MATH I don't see how to fix this.
First off, thanks for looking into the combination of these
two features. Getting them to work together nicely is half
of the difficulty of these two extensions.
On platforms which support MI_MATH, I think we're probably
better off just using it. For Ivy Bridge, the only thing I
could think to do when both are in use would be to do two
MI_PREDICATEs for every draw call. The first would be what
you describe above and the second would be the MI_PREDICATE
for the conditional render with COMBINE_AND. When the
condition is true, the AND would have no effect and you
would get the behavior above. If the condition is false,
the above logic for implementing draw_indirect_count
wouldn't matter because it would get ANDed with false. On
Haswell and later, it's likely more efficient to just use
MI_MATH and avoid re-loading the draw count and condition
on every draw call. (We could just leave the draw count in
CS_GPR0, for instance.) Does that work?
Looks like a plan. I'll try to go this path.
Also there is another interaction which wasn't thought of
before:
Several vkCmdDrawIndirectCountKHR in conditional render
block but using MI_MATH should solve it.
In that case, we'll have to basically re-do the conditional bit
for every draw call. There may be some interesting interactions
here with secondary command buffers as well. I don't remember
what we decided about inheriting conditions in secondaries.
Again, if we decide we need MI_MATH, then we'll just drop
support for one or both extensions on Ivy Bridge.
About the secondary command buffers:
If inherited conditional rendering
<https://www.khronos.org/registry/vulkan/specs/1.1-extensions/html/vkspec.html#features-features-inheritedConditionalRendering>
is supported it means that secondary buffers can be executed
inside conditional rendering block and commands which can be
affected by conditional rendering are affected by it in secondary
buffer and also in primary, is it right?
However at this point the secondary buffer is already composed
without commands for conditions and since our implementation
depends on commands emitted to the buffer making its commands to
depend on condition either highly tricky to do (secondary buffer
needs to have certain points where to inject conditions?) or just
impossible.
And this secondary buffer may have been formed inside co
Re: [Mesa-dev] [ANNOUNCE] mesa 20.0.3
"spirv: Implement OpCopyObject and OpCopyLogical as blind copies" was reverted yesterday due to the failures in several dEQP-VK tests, see: https://gitlab.freedesktop.org/mesa/mesa/-/commit/68f325b256d96dca923f6c7d84bc6faf43911245 https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4375 I'm not sure if it's already known or how important it is, but I'd better say it than not. On 02.04.20 00:52, Eric Engestrom wrote: Hi all, I'd like to announce the release of Mesa 20.0.3. Quite a busy cycle again, with fixes all over the tree, but nothing extraordinary; mostly AMD (radv, aco), NIR and Intel (isl, anv), as expected. Cheers, Eric --- Git shortlog Caio Marcelo de Oliveira Filho (1): mesa/main: Fix overflow in validation of DispatchComputeGroupSizeARB Dylan Baker (6): docs/relnotes: Add sha256 sums for 20.0.2 .pick_status.json: Update to cf62c2b2ac69637785f55b790fdd601c17e7e9d5 .pick_status.json: Mark 672d10619980687acec329742f055f7f3796c1b8 as backported .pick_status.json: Mark c923de68dd0ab10a5a5fb3196f539707d046d897 as backported .pick_status.json: Mark 56de6f698e3f164d97f132203e8159ef0b8e9bb8 as denominated .pick_status.json: Update to aee004a7c8900938d1c17f0ac299d40001b383b0 Eric Engestrom (8): .pick_status.json: Update to 3252041a7872c49e53bb02ffe8b079b5fc43f15e .pick_status.json: Update to 12711939320e4fcd3a0d86af22da1042ad92035f .pick_status.json: Update to 05069e1f0794aadd40ce9269f858e50c64254388 .pick_status.json: Update to 8970b7839aebefa7207c9535ac34ab4e8cc0ae25 .pick_status.json: Update to 5f4d9b419a1c931ad468b8b22b8a95b1216891e4 .pick_status.json: Update to 70ac7f5b0c46370075a35067c9f7dfe78e84b16d docs: add release notes for 20.0.3 VERSION: bump to 20.0.3 Erik Faye-Lund (3): rbug: do not return void-value pipebuffer: clean up cast-warnings vtn/opencl: fully enable OpenCLstd_Clz Francisco Jerez (1): intel/fs/gen12: Fix interaction of SWSB dependency combination with EU fusion workaround. Greg V (1): amd/addrlib: fix build on non-x86 platforms Ian Romanick (2): soft-fp64/fsat: Correctly handle NaN soft-fp64: Split a block that was missing a cast on a comparison Jason Ekstrand (5): intel/blorp: Add support for swizzling fast-clear colors anv: Swizzle fast-clear values nir/lower_int64: Lower 8 and 16-bit downcasts with nir_lower_mov64 anv: Account for the header in anv_state_stream_alloc spirv: Implement OpCopyObject and OpCopyLogical as blind copies John Stultz (2): gallium: hud_context: Fix scalar initializer warning. vc4_bufmgr: Remove duplicative VC definition Jordan Justen (2): intel: Update TGL PCI strings intel: Add TGL PCI ID Lionel Landwerlin (5): isl: implement linear tiling row pitch requirement for display isl: properly filter supported display modifiers on Gen9+ isl: only apply main surface ccs pitch constraint with CCS isl: drop min row pitch alignment when set by the driver intel: add new TGL pci ids Marek Olšák (3): nir: fix clip/cull_distance_array_size in nir_lower_clip_cull_distance_arrays ac: fix fast division st/mesa: fix use of uninitialized memory due to st_nir_lower_builtin Marek Vasut (1): etnaviv: Emit PE.ALPHA_COLOR_EXT* on GPUs with half-float support Neil Armstrong (1): Revert "ci: Remove T820 from CI temporarily" Pierre-Eric Pelloux-Prayer (1): st/mesa: disallow deferred flush if there are multiple contexts Rhys Perry (11): nir/gather_info: handle emit_vertex_with_counter aco: set has_divergent_branch for discards in loops aco: handle missing second predecessors at merge block phis aco: skip NIR in unreachable merge blocks aco: improve check for unreachable loop continue blocks aco: emit IR in IF's merge block instead if the other side ends in a jump aco: fix boolean undef regclass nir/gather_info: fix per-vertex handling in try_mask_partial_io aco: implement 64-bit VGPR constant copies in handle_operands() glsl: fix race in instance getters util/u_queue: fix race in total_jobs_size access Rob Clark (2): freedreno/ir3/ra: fix array liveranges util: fix u_fifo_pop() Samuel Pitoiset (7): radv/gfx10: fix required subgroup size with VK_EXT_subgroup_size_control radv/gfx10: fix required ballot size with VK_EXT_subgroup_size_control radv: fix optional pSizes parameter when binding streamout buffers radv: enable VK_KHR_8bit_storage on GFX6-GFX7 ac/nir: use llvm.amdgcn.rcp for nir_op_frcp ac/nir: use llvm.amdgcn.rsq for nir_op_frsq ac/nir: use llvm.amdgcn.rcp in ac_build_fdiv() Tapani Pälli (1): glsl: set error_emitted true if type not ok for assignment Thomas Hellstrom (1):
Re: [Mesa-dev] [ANNOUNCE] mesa 20.0.5
My commit "st/mesa: Update shader info of ffvp/ARB_vp after translation to NIR" did introduce regression and its fix, which was merged (but isn't in this release even though there is "fixes" tag), didn't fully fix it. See https://gitlab.freedesktop.org/mesa/mesa/-/issues/2815 This commit may break usage of fixed function and ARB_vp shaders. I thought that I would fix it until the next release and I wasn't expected this one. I'm sorry for this trouble. However looking at it I think: maybe something could be changed about applying patches to stable to safeguard against such issues. On 23.04.20 00:54, Dylan Baker wrote: Hi list, I'd like to announce the availability of mesa 20.0.5. It's one week late due to a number of issues, including a regression in mesa, a regression in piglit, and some CI trouble. I'm still planning to make 20.0.6 next week at it's secheduled time. This is a pretty big release, as it contiains 3 weeks rather than just two weeks of changes. We have the normal spattering of changes, with the AMD and Intel drivers receiving the majority of the work. Dylan Shortlog Arcady Goldmints-Orlov (1): nir: Lower returns correctly inside nested loops Bas Nieuwenhuizen (3): radv: Store 64-bit availability bools if requested. radv: Consider maximum sample distances for entire grid. radv: Use correct buffer count with variable descriptor set sizes. D Scott Phillips (1): util/sparse_array: don't stomp head's counter on pop operations Daniel Stone (1): EGL: Add eglSetDamageRegionKHR to GLVND dispatch list Danylo Piliaiev (1): st/mesa: Update shader info of ffvp/ARB_vp after translation to NIR Dave Airlie (2): draw: free the NIR IR. llvmpipe/nir: free the nir shader Dylan Baker (8): .pick_status.json: Update to 089e1fb287eb9b70c191091128ed5ba7edd2960a .pick_status.json: Update to 65e2eaa4d3a7095ac438fafb09d1e36a4210966e .pick_status.json: Update to 28d36d26c2212276e1238fad8f0b12caab97fee8 .pick_status.json: Update to acf7e73be54c7f1cc52fcc9be38a9df26849200e .pick_status.json: Update to 13ce637f1b28381e72470763ff5e39dd3c562476 .pick_status.json: Update to c3c1f4d6bcc210408f8b180727d269838b38193b docs: Add relnotes for 20.0.5 VERSION: bump for 20.0.5 Emil Velikov (4): glx: set the loader_logger early and for everyone egl/drm: reinstate (kms_)swrast support Revert "egl/dri2: Don't dlclose() the driver on dri2_load_driver_common failure" glx: omit loader_loader() for macOS Eric Anholt (1): ci: Remove LLVM from ARM test drivers. Eric Engestrom (1): docs/relnotes: add sha256sum for 20.0.4 Hyunjun Ko (1): nir: fix wrong assignment to buffer in xfb_varyings_info Ilia Mirkin (1): nv50: don't try to upload MSAA settings for BUFFER textures Jason Ekstrand (5): anv/image: Use align_u64 for image offsets nir/load_store_vectorize: Fix shared atomic info spirv: Handle OOB vector extract operations intel: Add _const versions of prog_data cast helpers anv: Report correct SLM size Jose Maria Casanova Crespo (1): v3d: Primitive Counts Feedback needs an extra 32-bit padding. Juan A. Suarez Romero (2): intel/compiler: store the FS inputs in WM prog data anv/pipeline: allow more than 16 FS inputs Karol Herbst (2): clover: fix build with single library clang build Revert "nvc0: fix line width on GM20x+" Lionel Landwerlin (7): iris: properly free resources on BO allocation failure iris: share buffer managers accross screens iris: make resources take a ref on the screen object i965: store DRM fd on intel_screen i965: share buffer managers across screens iris: drop cache coherent cpu mapping for external BO util/sparse_free_list: manipulate node pointers using atomic primitives Marek Olšák (1): st/mesa: fix a crash due to passing a draw vertex shader into the driver Mathias Fröhlich (1): i965: Move down genX_upload_sbe in profiles. Matt Turner (1): meson: Specify the maximum required libdrm in dri.pc Neil Armstrong (3): gitlab-ci/lava: fix handling of lava tags gitlab-ci: add FILES_HOST_URL and move FILES_HOST_NAME into jobs gitlab-ci: re-enable mali400/450 and t820 jobs Rhys Perry (1): aco: fix 1D textureGrad() on GFX9 Rob Clark (1): nir: fix definition of imadsh_mix16 for vectors Rohan Garg (1): ci: Split out radv build-testing on arm64 Samuel Pitoiset (9): ac/nir: split 8-bit load/store to global memory on GFX6 ac/nir: split 8-bit SSBO stores on GFX6 radv/llvm: enable 8-bit storage features on GFX6-GFX7 ac/nir: split 16-bit load/store to global memory on GFX6 ac/nir: split 16-bit SSBO stores on GFX6 radv/llvm: enable 16-bit storage feature
Re: [Mesa-dev] [ANNOUNCE] mesa 20.0.5
I want to sum up what happened from my perspective, I think it could be useful to improve the process: 1) There was a regression in 20.* (https://gitlab.freedesktop.org/mesa/mesa/-/issues/2758) 2) I "fixed" the regression and broke non-iris drivers (https://gitlab.freedesktop.org/mesa/mesa/-/commit/d684fb37bfbc47d098158cb03c0672119a4469fe) 3) I "fixed" the new regression of fix 2) (https://gitlab.freedesktop.org/mesa/mesa/-/commit/829013d0cad0fa2513b32ae07cf8d745f6e5c62d) 4) After that, it appeared that due to a bug in piglit, Intel CI didn't run piglit tests which gave me a false sense of commit's correctness (https://gitlab.freedesktop.org/mesa/mesa/-/issues/2815) 5) I aimed to have a fix before the next minor release on 2020-04-29 by consulting the release calendar. 6) I accidentally saw 20.0.5 being released with one of the two of my commits. I see multiple failure points: 1) Me not carefully examining all code paths, because at least one that failed should have been obvious to test. 2) CI not communicating that piglit tests were not executed. Again, I could have seen this, examined CI results, but did not. 3) After restoration of CI capabilities test what added to "expected failure" and this may have contributed to regression being missed when testing the release. I'm not sure about this part so correct me if I'm wrong. 4) I didn't know about this release and that this release was help up for the fix of 2758. 5) There were now window between announcing the scope of the release and release itself. Since I knew about regression I could have notified about it. Also there is no milestone for minor releases so it's problematic to link issue and release. It's a second release in a row with clear regression crept in. I believe that we can use this to improve the process and safeguard against such regressions in the future. P.S. I'm preparing and will test a final fix which will be sent soon. Danylo On 23.04.20 07:40, Dylan Baker wrote: Quoting Ilia Mirkin (2020-04-22 15:47:59) On Wed, Apr 22, 2020 at 6:39 PM Danylo Piliaiev wrote: I'm sorry for this trouble. However looking at it I think: maybe something could be changed about applying patches to stable to safeguard against such issues. We used to get pre-announcements a few days prior to a release which would allow developers to look things over, which would allow us to notice things like that. Not sure when this got dropped. Cheers, -ilia That was dropped in favor of a live staging branch that is updated daily (at least on week days). Dylan ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [ANNOUNCE] mesa 20.0.5
While we talked about the general communication issues, I want to finish with this particular one. Is this release going out as normal and the fix will be in the next 20.0.6 on 2020-04-29? The current scope of the regression is non-iris drivers or iris compiled without llvm. The regression will result in crash of ARB_vp shaders which don't use at least one vertex attribute or one particular case of fixed-function shader. On the brighter side it seams there weren't many such cases in the wild, we saw only UT2004. Danylo On 23.04.20 07:45, Dylan Baker wrote: This is particularly problematic, as that solved a regression in 20.0.4, and we held up the release for that fix. The second patch is in the staging branch and I can do an emergency release, but if there's another patch that's needed then that doesn't help. I'm not sure that reverting the original offending patch is an option either. Dylan Quoting Danylo Piliaiev (2020-04-22 15:38:59) My commit "st/mesa: Update shader info of ffvp/ARB_vp after translation to NIR" did introduce regression and its fix, which was merged (but isn't in this release even though there is "fixes" tag), didn't fully fix it. See https://gitlab.freedesktop.org/mesa/mesa/-/issues/2815 This commit may break usage of fixed function and ARB_vp shaders. I thought that I would fix it until the next release and I wasn't expected this one. I'm sorry for this trouble. However looking at it I think: maybe something could be changed about applying patches to stable to safeguard against such issues. On 23.04.20 00:54, Dylan Baker wrote: Hi list, I'd like to announce the availability of mesa 20.0.5. It's one week late due to a number of issues, including a regression in mesa, a regression in piglit, and some CI trouble. I'm still planning to make 20.0.6 next week at it's secheduled time. This is a pretty big release, as it contiains 3 weeks rather than just two weeks of changes. We have the normal spattering of changes, with the AMD and Intel drivers receiving the majority of the work. Dylan Shortlog Arcady Goldmints-Orlov (1): nir: Lower returns correctly inside nested loops Bas Nieuwenhuizen (3): radv: Store 64-bit availability bools if requested. radv: Consider maximum sample distances for entire grid. radv: Use correct buffer count with variable descriptor set sizes. D Scott Phillips (1): util/sparse_array: don't stomp head's counter on pop operations Daniel Stone (1): EGL: Add eglSetDamageRegionKHR to GLVND dispatch list Danylo Piliaiev (1): st/mesa: Update shader info of ffvp/ARB_vp after translation to NIR Dave Airlie (2): draw: free the NIR IR. llvmpipe/nir: free the nir shader Dylan Baker (8): .pick_status.json: Update to 089e1fb287eb9b70c191091128ed5ba7edd2960a .pick_status.json: Update to 65e2eaa4d3a7095ac438fafb09d1e36a4210966e .pick_status.json: Update to 28d36d26c2212276e1238fad8f0b12caab97fee8 .pick_status.json: Update to acf7e73be54c7f1cc52fcc9be38a9df26849200e .pick_status.json: Update to 13ce637f1b28381e72470763ff5e39dd3c562476 .pick_status.json: Update to c3c1f4d6bcc210408f8b180727d269838b38193b docs: Add relnotes for 20.0.5 VERSION: bump for 20.0.5 Emil Velikov (4): glx: set the loader_logger early and for everyone egl/drm: reinstate (kms_)swrast support Revert "egl/dri2: Don't dlclose() the driver on dri2_load_driver_common failure" glx: omit loader_loader() for macOS Eric Anholt (1): ci: Remove LLVM from ARM test drivers. Eric Engestrom (1): docs/relnotes: add sha256sum for 20.0.4 Hyunjun Ko (1): nir: fix wrong assignment to buffer in xfb_varyings_info Ilia Mirkin (1): nv50: don't try to upload MSAA settings for BUFFER textures Jason Ekstrand (5): anv/image: Use align_u64 for image offsets nir/load_store_vectorize: Fix shared atomic info spirv: Handle OOB vector extract operations intel: Add _const versions of prog_data cast helpers anv: Report correct SLM size Jose Maria Casanova Crespo (1): v3d: Primitive Counts Feedback needs an extra 32-bit padding. Juan A. Suarez Romero (2): intel/compiler: store the FS inputs in WM prog data anv/pipeline: allow more than 16 FS inputs Karol Herbst (2): clover: fix build with single library clang build Revert "nvc0: fix line width on GM20x+" Lionel Landwerlin (7): iris: properly free resources on BO allocation failure iris: share buffer managers accross screens iris: make resources take a
Re: [Mesa-dev] [ANNOUNCE] mesa 20.0.5
On 23.04.20 19:56, Dylan Baker wrote: Quoting Michel Dänzer (2020-04-23 09:24:45) On 2020-04-23 6:19 p.m., Mark Janes wrote: Michel Dänzer writes: On 2020-04-23 5:14 p.m., Mark Janes wrote: Does anyone have recommendations for how to use Gitlab to verify that there are no identified-but-unfixed bugs in a pending release? I'd say GitLab milestones could be used to address the issues you raised above: Create a milestone for each release, and only cut the release once all issues and MRs assigned to it have been dealt with. Milestones have been used to track progress toward recent releases. It is non-trivial to audit all open bugs to determine which ones must be assigned to a milestone. Bugs are opened long before milestones are created. Of course there are more complicated cases, but the particular case which spawned this thread should have been pretty straightforward to handle with a 20.0.5 milestone. Indeed, I like the milestone approach and I've gone ahead and created the milestone. Yes, even if milestones don't alleviate all issues - at least it is better than nothing, thanks! ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
