Re: [Mesa-dev] [PATCH] Integrate precise trig into configuration infrastructure
On 2016-04-27 14:52:34, Gurchetan Singh wrote:
> With this change, to enable precise SIN and COS instructions
> on Intel hardware, one can put
>
>
>
> in the proper drirc file.
> ---
> src/mesa/drivers/dri/common/xmlpool/t_options.h | 5 +
> src/mesa/drivers/dri/i965/brw_compiler.c| 2 --
> src/mesa/drivers/dri/i965/brw_context.c | 3 +++
> src/mesa/drivers/dri/i965/intel_screen.c| 2 ++
No apps in mind for src/mesa/drivers/dri/common/drirc?
> 4 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/common/xmlpool/t_options.h
> b/src/mesa/drivers/dri/common/xmlpool/t_options.h
> index e5cbc46..615491a 100644
> --- a/src/mesa/drivers/dri/common/xmlpool/t_options.h
> +++ b/src/mesa/drivers/dri/common/xmlpool/t_options.h
> @@ -158,6 +158,11 @@ DRI_CONF_OPT_BEGIN_B(force_s3tc_enable, def) \
> DRI_CONF_DESC(en,gettext("Enable S3TC texture compression even if
> software support is not available")) \
> DRI_CONF_OPT_END
>
> +#define DRI_CONF_INTEL_PRECISE_TRIG(def) \
> +DRI_CONF_OPT_BEGIN_B(intel_precise_trig, def) \
The other conf options are more generically named, even if they were
added for hardware specific reasons.
Example: b3340cd32acf5935891f19833de0cfc500a93e0b
So, maybe precise_trig instead?
> +DRI_CONF_DESC(en,gettext("Minimize maximum error for Intel's SIN and
> COS instructions")) \
Is the above is changed, then maybe "Prefer accuracy over performance
in trig functions" instead.
> +DRI_CONF_OPT_END
> +
> #define DRI_CONF_COLOR_REDUCTION_ROUND 0
> #define DRI_CONF_COLOR_REDUCTION_DITHER 1
> #define DRI_CONF_COLOR_REDUCTION(def) \
> diff --git a/src/mesa/drivers/dri/i965/brw_compiler.c
> b/src/mesa/drivers/dri/i965/brw_compiler.c
> index 4496699..8753197 100644
> --- a/src/mesa/drivers/dri/i965/brw_compiler.c
> +++ b/src/mesa/drivers/dri/i965/brw_compiler.c
> @@ -148,8 +148,6 @@ brw_compiler_create(void *mem_ctx, const struct
> brw_device_info *devinfo)
> brw_fs_alloc_reg_sets(compiler);
> brw_vec4_alloc_reg_set(compiler);
>
> - compiler->precise_trig = env_var_as_boolean("INTEL_PRECISE_TRIG", false);
> -
Ken, I take it the dri conf would work fine for you instead of the env
var?
-Jordan
> compiler->scalar_stage[MESA_SHADER_VERTEX] =
>devinfo->gen >= 8 && !(INTEL_DEBUG & DEBUG_VEC4VS);
> compiler->scalar_stage[MESA_SHADER_TESS_CTRL] = false;
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index b190549..6439547 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -768,6 +768,9 @@ brw_process_driconf_options(struct brw_context *brw)
>
> brw->precompile = driQueryOptionb(&brw->optionCache, "shader_precompile");
>
> + brw->intelScreen->compiler->precise_trig =
> + driQueryOptionb(&brw->optionCache, "intel_precise_trig");
> +
> ctx->Const.ForceGLSLExtensionsWarn =
>driQueryOptionb(options, "force_glsl_extensions_warn");
>
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index beebfce..e571c2b 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -65,6 +65,8 @@ DRI_CONF_BEGIN
> DRI_CONF_SECTION_QUALITY
>DRI_CONF_FORCE_S3TC_ENABLE("false")
>
> + DRI_CONF_INTEL_PRECISE_TRIG("false")
> +
>DRI_CONF_OPT_BEGIN(clamp_max_samples, int, -1)
>DRI_CONF_DESC(en, "Clamp the value of GL_MAX_SAMPLES to the "
> "given integer. If negative, then do not clamp.")
> --
> 2.1.2
>
> ___
> 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 3/3] i965: Reimplement ARB_transform_feedback2 on Haswell and later.
On Friday, May 6, 2016 11:42:49 PM PDT Kenneth Graunke wrote: > My old implementation accumulated pairs in a buffer, > and eventually processed that data on the CPU. This meant flushing > the batchbuffer and waiting for it to completely execute before we > could map it, resulting in really long stalls. We could also run out > of space in the buffer, and have to do this early. > > Instead, we can use Haswell's MI_MATH command to do the (end - start) > subtraction, as well as the multiplication by 2 or 3 to convert from > the number of primitives written to the number of vertices written. > We still need to CS stall to read the counters, but otherwise everything > is completely pipelined - there's no CPU<->GPU synchronization required. > It also uses only 80 bytes in the buffer, no matter what. > > Improves performance in Manhattan on Skylake GT3e at 800x600 by > 6.1086% +/- 0.954166% (n=9). At 1920x1080, improves performance > by 2.82103% +/- 0.148596% (n=84). > > Signed-off-by: Kenneth Graunke Sorry, I forgot to do the s/has_mi_math/has_mi_math_and_lrr/ before sending. Fixed locally. signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] i965: Only enable ARB_query_buffer_object for newer kernels on Haswell.
On Haswell, we need version 6 of the kernel command parser in order to
write the math registers. Our implementation of ARB_query_buffer_object
heavily relies on MI_MATH, so we should only advertise it when MI_MATH
is available. We also need MI_LOAD_REGISTER_REG, which requires version
7 of the command parser.
To make these checks easier, introduce a screen->has_mi_math_and_lrr
flag that will be set when both commands are supported.
Cc: Jordan Justen
Signed-off-by: Kenneth Graunke
---
src/mesa/drivers/dri/i965/intel_extensions.c | 2 +-
src/mesa/drivers/dri/i965/intel_screen.c | 8
src/mesa/drivers/dri/i965/intel_screen.h | 6 ++
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c
b/src/mesa/drivers/dri/i965/intel_extensions.c
index 588df1a..8d98788 100644
--- a/src/mesa/drivers/dri/i965/intel_extensions.c
+++ b/src/mesa/drivers/dri/i965/intel_extensions.c
@@ -366,7 +366,7 @@ intelInitExtensions(struct gl_context *ctx)
}
}
- if (brw->gen >= 8 || brw->is_haswell) {
+ if (brw->intelScreen->has_mi_math_and_lrr) {
ctx->Extensions.ARB_query_buffer_object = true;
}
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
b/src/mesa/drivers/dri/i965/intel_screen.c
index 878901a..b6670e9 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1541,6 +1541,14 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
if (ret == -1)
intelScreen->cmd_parser_version = 0;
+ /* Haswell requires command parser version 6 in order to write to the
+* MI_MATH GPR registers, and version 7 in order to use
+* MI_LOAD_REGISTER_REG (which all users of MI_MATH use).
+*/
+ intelScreen->has_mi_math_and_lrr = intelScreen->devinfo->gen >= 8 ||
+ (intelScreen->devinfo->is_haswell &&
+ intelScreen->cmd_parser_version >= 7);
+
psp->extensions = !intelScreen->has_context_reset_notification
? intelScreenExtensions : intelRobustScreenExtensions;
diff --git a/src/mesa/drivers/dri/i965/intel_screen.h
b/src/mesa/drivers/dri/i965/intel_screen.h
index 01d45d0..98f9d24 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.h
+++ b/src/mesa/drivers/dri/i965/intel_screen.h
@@ -56,6 +56,12 @@ struct intel_screen
bool has_resource_streamer;
/**
+* Does the current hardware and kernel support MI_MATH and
+* MI_LOAD_REGISTER_REG?
+*/
+ bool has_mi_math_and_lrr;
+
+ /**
* Does the kernel support context reset notifications?
*/
bool has_context_reset_notification;
--
2.8.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] i965: Reimplement ARB_transform_feedback2 on Haswell and later.
My old implementation accumulated pairs in a buffer,
and eventually processed that data on the CPU. This meant flushing
the batchbuffer and waiting for it to completely execute before we
could map it, resulting in really long stalls. We could also run out
of space in the buffer, and have to do this early.
Instead, we can use Haswell's MI_MATH command to do the (end - start)
subtraction, as well as the multiplication by 2 or 3 to convert from
the number of primitives written to the number of vertices written.
We still need to CS stall to read the counters, but otherwise everything
is completely pipelined - there's no CPU<->GPU synchronization required.
It also uses only 80 bytes in the buffer, no matter what.
Improves performance in Manhattan on Skylake GT3e at 800x600 by
6.1086% +/- 0.954166% (n=9). At 1920x1080, improves performance
by 2.82103% +/- 0.148596% (n=84).
Signed-off-by: Kenneth Graunke
---
src/mesa/drivers/dri/i965/Makefile.sources | 1 +
src/mesa/drivers/dri/i965/brw_context.c| 14 +-
src/mesa/drivers/dri/i965/brw_context.h| 14 ++
src/mesa/drivers/dri/i965/brw_draw.c | 38 -
src/mesa/drivers/dri/i965/hsw_sol.c| 253 +
5 files changed, 308 insertions(+), 12 deletions(-)
create mode 100644 src/mesa/drivers/dri/i965/hsw_sol.c
diff --git a/src/mesa/drivers/dri/i965/Makefile.sources
b/src/mesa/drivers/dri/i965/Makefile.sources
index 8c60954..d35775e 100644
--- a/src/mesa/drivers/dri/i965/Makefile.sources
+++ b/src/mesa/drivers/dri/i965/Makefile.sources
@@ -228,6 +228,7 @@ i965_FILES = \
gen8_vs_state.c \
gen8_wm_depth_stencil.c \
hsw_queryobj.c \
+ hsw_sol.c \
intel_batchbuffer.c \
intel_batchbuffer.h \
intel_blit.c \
diff --git a/src/mesa/drivers/dri/i965/brw_context.c
b/src/mesa/drivers/dri/i965/brw_context.c
index 1380d41..2338514 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -372,13 +372,18 @@ brw_init_driver_functions(struct brw_context *brw,
functions->NewTransformFeedback = brw_new_transform_feedback;
functions->DeleteTransformFeedback = brw_delete_transform_feedback;
- functions->GetTransformFeedbackVertexCount =
- brw_get_transform_feedback_vertex_count;
- if (brw->gen >= 7) {
+ if (brw->intelScreen->has_mi_math) {
+ functions->BeginTransformFeedback = hsw_begin_transform_feedback;
+ functions->EndTransformFeedback = hsw_end_transform_feedback;
+ functions->PauseTransformFeedback = hsw_pause_transform_feedback;
+ functions->ResumeTransformFeedback = hsw_resume_transform_feedback;
+ } else if (brw->gen >= 7) {
functions->BeginTransformFeedback = gen7_begin_transform_feedback;
functions->EndTransformFeedback = gen7_end_transform_feedback;
functions->PauseTransformFeedback = gen7_pause_transform_feedback;
functions->ResumeTransformFeedback = gen7_resume_transform_feedback;
+ functions->GetTransformFeedbackVertexCount =
+ brw_get_transform_feedback_vertex_count;
} else {
functions->BeginTransformFeedback = brw_begin_transform_feedback;
functions->EndTransformFeedback = brw_end_transform_feedback;
@@ -494,7 +499,8 @@ brw_initialize_context_constants(struct brw_context *brw)
ctx->Const.MaxTransformFeedbackSeparateComponents =
BRW_MAX_SOL_BINDINGS / BRW_MAX_SOL_BUFFERS;
- ctx->Const.AlwaysUseGetTransformFeedbackVertexCount = true;
+ ctx->Const.AlwaysUseGetTransformFeedbackVertexCount =
+ !brw->intelScreen->has_mi_math;
int max_samples;
const int *msaa_modes = intel_supported_msaa_modes(brw->intelScreen);
diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index b620f14..035cbe9 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1656,6 +1656,20 @@ void
gen7_resume_transform_feedback(struct gl_context *ctx,
struct gl_transform_feedback_object *obj);
+/* hsw_sol.c */
+void
+hsw_begin_transform_feedback(struct gl_context *ctx, GLenum mode,
+ struct gl_transform_feedback_object *obj);
+void
+hsw_end_transform_feedback(struct gl_context *ctx,
+ struct gl_transform_feedback_object *obj);
+void
+hsw_pause_transform_feedback(struct gl_context *ctx,
+ struct gl_transform_feedback_object *obj);
+void
+hsw_resume_transform_feedback(struct gl_context *ctx,
+ struct gl_transform_feedback_object *obj);
+
/* brw_blorp_blit.cpp */
GLbitfield
brw_blorp_framebuffer(struct brw_context *brw,
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
b/src/mesa/drivers/dri/i965/brw_draw.c
index afa8a4e..9d034cf 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -153,7 +153,9 @@ trim(GLenum prim, GLuint length)
static void
brw_em
[Mesa-dev] [PATCH 2/3] i965: Add a brw_load_register_reg64 helper.
It appears that we can't do this in a single command (like we do for
MI_LOAD_REGISTER_IMM) - the Skylake simulator gets rather grumpy about
the command length if I try to combine them. No matter.
Signed-off-by: Kenneth Graunke
---
src/mesa/drivers/dri/i965/brw_context.h | 2 ++
src/mesa/drivers/dri/i965/intel_batchbuffer.c | 18 ++
2 files changed, 20 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index c216904..b620f14 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1463,6 +1463,8 @@ void brw_load_register_imm64(struct brw_context *brw,
uint32_t reg, uint64_t imm);
void brw_load_register_reg(struct brw_context *brw, uint32_t src,
uint32_t dest);
+void brw_load_register_reg64(struct brw_context *brw, uint32_t src,
+ uint32_t dest);
void brw_store_data_imm32(struct brw_context *brw, drm_intel_bo *bo,
uint32_t offset, uint32_t imm);
void brw_store_data_imm64(struct brw_context *brw, drm_intel_bo *bo,
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index 98b9485..77cdc0a 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -649,6 +649,24 @@ brw_load_register_reg(struct brw_context *brw, uint32_t
src, uint32_t dest)
}
/*
+ * Copies a 64-bit register.
+ */
+void
+brw_load_register_reg64(struct brw_context *brw, uint32_t src, uint32_t dest)
+{
+ assert(brw->gen >= 8 || brw->is_haswell);
+
+ BEGIN_BATCH(6);
+ OUT_BATCH(MI_LOAD_REGISTER_REG | (3 - 2));
+ OUT_BATCH(src);
+ OUT_BATCH(dest);
+ OUT_BATCH(MI_LOAD_REGISTER_REG | (3 - 2));
+ OUT_BATCH(src + sizeof(uint32_t));
+ OUT_BATCH(dest + sizeof(uint32_t));
+ ADVANCE_BATCH();
+}
+
+/*
* Write 32-bits of immediate data to a GPU memory buffer.
*/
void
--
2.8.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Added pbuffer hooks for surfaceless platform
On 05/06/2016 03:39 PM, Stéphane Marchesin wrote:
> On Fri, May 6, 2016 at 3:32 PM, Gurchetan Singh
> wrote:
>> This change enables the creation of pbuffer
>> surfaces on the surfaceless platform.
>>
>> V2: Use double-buffered pbuffer configuration
>
> Reviewed-by: Stéphane Marchesin
>
> Chad, do you also want to take a look at it?
On a philosophical note, it's ironic that we're now creating a *surface* in the
*surfaceless* platform. Don't you agree? Anyway, let's ignore the irony and
focus on practical matters.
There are a few bugs and minor style issues. See the comments below.
There is also one major issue that needs discussion. I believe pbuffers
are single-buffered, and that the single buffer is the back buffer.
As precedent, platform_x11.c implements pbuffers as single-buffered.
The relevant language in the EGL 1.5 spec is phrased badly, and could be
interpreted either way: (a) pbuffers are double-buffered, or (b) pbuffers have
only a back buffer. If I recall correctly, an internal Khronos conversation in
2014 arrived at conclusion (b). Here are the relevant quotes from the spec:
- Some platforms may not allow rendering directly to the front buffer of
a window surface. When such windows are made current to a context, the
context will always have an EGL_RENDER_BUFFER attribute value of
EGL_BACK_BUFFER. From the client API point of view these surfaces have
only a back buffer and no front buffer, similar to pbuffer rendering (see
section 2.2.2).
- Querying EGL_RENDER_BUFFER [with eglQueryContext()] returns the buffer
which client API rendering is requested to use. [...] For a pbuffer
surface, it is always EGL_BACK_BUFFER.
- [When eglSwapBuffers() is called,] If surface is a back-buffered window
surface, then the color buffer is copied to the native window associated
with that surface. [Otherwise, if] surface is a single-buffered window,
pixmap, or pbuffer surface, eglSwapBuffers has no effect.
The single-buffered nature has an impact on the implementation of
eglSwapBuffers, according the last bullet. It's a no-op. As precedent,
platform_x11.c correctly does nothing when swapping a pbuffer.
If you think my interpretation of the spec is wrong, and that
platform_x11.c is incorrect, then I'm open to discussing it. I'm
especially interested to learn whether any non-Mesa EGL implementations
treat pbuffers as double-buffered.
(Also, this patch should probably set EGL_MIN/MAX_SWAP_INTERVAL to 0/0
for pbuffer configs. But let's overlook that for now, as I don't think
it's important for the initial implementation. Depending on how Google
uses this patch, perhaps the swap interval bounds are never relevant.)
>> ---
>> src/egl/drivers/dri2/egl_dri2.h | 8 +-
>> src/egl/drivers/dri2/platform_surfaceless.c | 219
>> +++-
>> 2 files changed, 222 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/egl_dri2.h
>> b/src/egl/drivers/dri2/egl_dri2.h
>> index ddb5f39..ecf9c76 100644
>> --- a/src/egl/drivers/dri2/egl_dri2.h
>> +++ b/src/egl/drivers/dri2/egl_dri2.h
>> @@ -291,8 +291,14 @@ struct dri2_egl_surface
>> /* EGL-owned buffers */
>> __DRIbuffer *local_buffers[__DRI_BUFFER_COUNT];
>> #endif
>> -};
>>
>> +#if defined(HAVE_SURFACELESS_PLATFORM)
>> + __DRIimage *front;
>> + __DRIimage *back;
>> + unsigned int format;
>> +#endif
>> +
>> +};
>>
>> struct dri2_egl_config
>> {
>> diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
>> b/src/egl/drivers/dri2/platform_surfaceless.c
>> index e0ddc12..08d5448 100644
>> --- a/src/egl/drivers/dri2/platform_surfaceless.c
>> +++ b/src/egl/drivers/dri2/platform_surfaceless.c
>> @@ -23,6 +23,7 @@
>> * DEALINGS IN THE SOFTWARE.
>> */
>>
>> +#include
>> #include
>> #include
>> #include
>> @@ -37,9 +38,209 @@
>> #include "egl_dri2_fallbacks.h"
>> #include "loader.h"
>>
>> +static __DRIimage*
>> +surfaceless_alloc_image(struct dri2_egl_display *dri2_dpy,
>> + struct dri2_egl_surface *dri2_surf)
>> +{
>> + __DRIimage *img = NULL;
>> +
>> + img = dri2_dpy->image->createImage(
>> +dri2_dpy->dri_screen,
>> +dri2_surf->base.Width,
>> +dri2_surf->base.Height,
>> +dri2_surf->format,
>> +0,
>> +NULL);
>> +
>> + return img;
>> +}
The img variable doesn't do much here. This function would be cleaner
if it had no local variable.
>> +static void
>> +surfaceless_free_images(struct dri2_egl_surface *dri2_surf)
>> +{
>> + struct dri2_egl_display *dri2_dpy =
>> + dri2_egl_display(dri2_surf->base.Resource.Display);
>> +
>> + if(dri2_surf->front) {
>> + dri2_dpy->image->destroyImage(dri2_surf->front);
>> + dri2_surf->front = NULL;
>> + }
>> + if(dri2_surf->back) {
>> + dri2_dpy->image->destroyImage(dri2_surf->back);
>> + dri2_surf->back = NULL;
>> +
Re: [Mesa-dev] [PATCH] Added pbuffer hooks for surfaceless platform
On Fri, May 6, 2016 at 3:32 PM, Gurchetan Singh
wrote:
> This change enables the creation of pbuffer
> surfaces on the surfaceless platform.
>
> V2: Use double-buffered pbuffer configuration
Reviewed-by: Stéphane Marchesin
Chad, do you also want to take a look at it?
> ---
> src/egl/drivers/dri2/egl_dri2.h | 8 +-
> src/egl/drivers/dri2/platform_surfaceless.c | 219
> +++-
> 2 files changed, 222 insertions(+), 5 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> index ddb5f39..ecf9c76 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -291,8 +291,14 @@ struct dri2_egl_surface
> /* EGL-owned buffers */
> __DRIbuffer *local_buffers[__DRI_BUFFER_COUNT];
> #endif
> -};
>
> +#if defined(HAVE_SURFACELESS_PLATFORM)
> + __DRIimage *front;
> + __DRIimage *back;
> + unsigned int format;
> +#endif
> +
> +};
>
> struct dri2_egl_config
> {
> diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
> b/src/egl/drivers/dri2/platform_surfaceless.c
> index e0ddc12..08d5448 100644
> --- a/src/egl/drivers/dri2/platform_surfaceless.c
> +++ b/src/egl/drivers/dri2/platform_surfaceless.c
> @@ -23,6 +23,7 @@
> * DEALINGS IN THE SOFTWARE.
> */
>
> +#include
> #include
> #include
> #include
> @@ -37,9 +38,209 @@
> #include "egl_dri2_fallbacks.h"
> #include "loader.h"
>
> +static __DRIimage*
> +surfaceless_alloc_image(struct dri2_egl_display *dri2_dpy,
> + struct dri2_egl_surface *dri2_surf)
> +{
> + __DRIimage *img = NULL;
> +
> + img = dri2_dpy->image->createImage(
> +dri2_dpy->dri_screen,
> +dri2_surf->base.Width,
> +dri2_surf->base.Height,
> +dri2_surf->format,
> +0,
> +NULL);
> +
> + return img;
> +}
> +
> +static void
> +surfaceless_free_images(struct dri2_egl_surface *dri2_surf)
> +{
> + struct dri2_egl_display *dri2_dpy =
> + dri2_egl_display(dri2_surf->base.Resource.Display);
> +
> + if(dri2_surf->front) {
> + dri2_dpy->image->destroyImage(dri2_surf->front);
> + dri2_surf->front = NULL;
> + }
> + if(dri2_surf->back) {
> + dri2_dpy->image->destroyImage(dri2_surf->back);
> + dri2_surf->back = NULL;
> + }
> +}
> +
> +static EGLBoolean
> +surfaceless_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface
> *draw)
> +{
> + struct dri2_egl_surface *dri2_surf = dri2_egl_surface(draw);
> +__DRIimage *temp = dri2_surf->back;
> +
> + dri2_surf->back = dri2_surf->front;
> + dri2_surf->front = temp;
> +}
> +
> +static int
> +surfaceless_image_get_buffers(__DRIdrawable *driDrawable,
> +unsigned int format,
> +uint32_t *stamp,
> +void *loaderPrivate,
> +uint32_t buffer_mask,
> +struct __DRIimageList *buffers)
> +{
> + struct dri2_egl_surface *dri2_surf = loaderPrivate;
> + struct dri2_egl_display *dri2_dpy =
> + dri2_egl_display(dri2_surf->base.Resource.Display);
> + buffers->image_mask = 0;
> + buffers->front = NULL;
> + buffers->back = NULL;
> +
> + if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
> + if (!dri2_surf->front)
> + dri2_surf->front =
> +surfaceless_alloc_image(dri2_dpy, dri2_surf);
> +
> + buffers->image_mask |= __DRI_IMAGE_BUFFER_FRONT;
> + buffers->front = dri2_surf->front;
> + }
> + if (buffer_mask & __DRI_IMAGE_BUFFER_BACK) {
> + if (!dri2_surf->back)
> + dri2_surf->back =
> +surfaceless_alloc_image(dri2_dpy, dri2_surf);
> +
> + buffers->image_mask |= __DRI_IMAGE_BUFFER_BACK;
> + buffers->back = dri2_surf->back;
> + }
> +
> + return 1;
> +}
> +
> +static _EGLSurface *
> +dri2_surfaceless_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint
> type,
> +_EGLConfig *conf, void *native_surface,
> +const EGLint *attrib_list)
> +{
> + struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> + struct dri2_egl_config *dri2_conf = dri2_egl_config(conf);
> + struct dri2_egl_surface *dri2_surf;
> + const __DRIconfig *config;
> + bool srgb;
> +
> + /* Make sure to calloc so all pointers
> +* are originally NULL.
> +*/
> + dri2_surf = calloc(1, sizeof *dri2_surf);
> +
> + if(!dri2_surf)
> + return NULL;
> +
> + if (!_eglInitSurface(&dri2_surf->base, disp, type, conf, attrib_list))
> + goto cleanup_surface;
> +
> + /* Only double buffered configurations exist at this point (single
> buffered
> +* configs were filtered out in surfaceless_add_configs_for_visuals).
> +*/
> + srgb = (dri2_surf->base.GLColorspace == EGL_GL_COLORSPACE_SRGB_KHR);
> + config = dri2_conf->dri_double_config[srgb];
> +
> + if (!config)
> + goto cleanup_surface;
> +
> + dri2_surf->d
[Mesa-dev] [PATCH] Added pbuffer hooks for surfaceless platform
This change enables the creation of pbuffer
surfaces on the surfaceless platform.
V2: Use double-buffered pbuffer configuration
---
src/egl/drivers/dri2/egl_dri2.h | 8 +-
src/egl/drivers/dri2/platform_surfaceless.c | 219 +++-
2 files changed, 222 insertions(+), 5 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index ddb5f39..ecf9c76 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -291,8 +291,14 @@ struct dri2_egl_surface
/* EGL-owned buffers */
__DRIbuffer *local_buffers[__DRI_BUFFER_COUNT];
#endif
-};
+#if defined(HAVE_SURFACELESS_PLATFORM)
+ __DRIimage *front;
+ __DRIimage *back;
+ unsigned int format;
+#endif
+
+};
struct dri2_egl_config
{
diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
b/src/egl/drivers/dri2/platform_surfaceless.c
index e0ddc12..08d5448 100644
--- a/src/egl/drivers/dri2/platform_surfaceless.c
+++ b/src/egl/drivers/dri2/platform_surfaceless.c
@@ -23,6 +23,7 @@
* DEALINGS IN THE SOFTWARE.
*/
+#include
#include
#include
#include
@@ -37,9 +38,209 @@
#include "egl_dri2_fallbacks.h"
#include "loader.h"
+static __DRIimage*
+surfaceless_alloc_image(struct dri2_egl_display *dri2_dpy,
+ struct dri2_egl_surface *dri2_surf)
+{
+ __DRIimage *img = NULL;
+
+ img = dri2_dpy->image->createImage(
+dri2_dpy->dri_screen,
+dri2_surf->base.Width,
+dri2_surf->base.Height,
+dri2_surf->format,
+0,
+NULL);
+
+ return img;
+}
+
+static void
+surfaceless_free_images(struct dri2_egl_surface *dri2_surf)
+{
+ struct dri2_egl_display *dri2_dpy =
+ dri2_egl_display(dri2_surf->base.Resource.Display);
+
+ if(dri2_surf->front) {
+ dri2_dpy->image->destroyImage(dri2_surf->front);
+ dri2_surf->front = NULL;
+ }
+ if(dri2_surf->back) {
+ dri2_dpy->image->destroyImage(dri2_surf->back);
+ dri2_surf->back = NULL;
+ }
+}
+
+static EGLBoolean
+surfaceless_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *draw)
+{
+ struct dri2_egl_surface *dri2_surf = dri2_egl_surface(draw);
+__DRIimage *temp = dri2_surf->back;
+
+ dri2_surf->back = dri2_surf->front;
+ dri2_surf->front = temp;
+}
+
+static int
+surfaceless_image_get_buffers(__DRIdrawable *driDrawable,
+unsigned int format,
+uint32_t *stamp,
+void *loaderPrivate,
+uint32_t buffer_mask,
+struct __DRIimageList *buffers)
+{
+ struct dri2_egl_surface *dri2_surf = loaderPrivate;
+ struct dri2_egl_display *dri2_dpy =
+ dri2_egl_display(dri2_surf->base.Resource.Display);
+ buffers->image_mask = 0;
+ buffers->front = NULL;
+ buffers->back = NULL;
+
+ if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
+ if (!dri2_surf->front)
+ dri2_surf->front =
+surfaceless_alloc_image(dri2_dpy, dri2_surf);
+
+ buffers->image_mask |= __DRI_IMAGE_BUFFER_FRONT;
+ buffers->front = dri2_surf->front;
+ }
+ if (buffer_mask & __DRI_IMAGE_BUFFER_BACK) {
+ if (!dri2_surf->back)
+ dri2_surf->back =
+surfaceless_alloc_image(dri2_dpy, dri2_surf);
+
+ buffers->image_mask |= __DRI_IMAGE_BUFFER_BACK;
+ buffers->back = dri2_surf->back;
+ }
+
+ return 1;
+}
+
+static _EGLSurface *
+dri2_surfaceless_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint
type,
+_EGLConfig *conf, void *native_surface,
+const EGLint *attrib_list)
+{
+ struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
+ struct dri2_egl_config *dri2_conf = dri2_egl_config(conf);
+ struct dri2_egl_surface *dri2_surf;
+ const __DRIconfig *config;
+ bool srgb;
+
+ /* Make sure to calloc so all pointers
+* are originally NULL.
+*/
+ dri2_surf = calloc(1, sizeof *dri2_surf);
+
+ if(!dri2_surf)
+ return NULL;
+
+ if (!_eglInitSurface(&dri2_surf->base, disp, type, conf, attrib_list))
+ goto cleanup_surface;
+
+ /* Only double buffered configurations exist at this point (single buffered
+* configs were filtered out in surfaceless_add_configs_for_visuals).
+*/
+ srgb = (dri2_surf->base.GLColorspace == EGL_GL_COLORSPACE_SRGB_KHR);
+ config = dri2_conf->dri_double_config[srgb];
+
+ if (!config)
+ goto cleanup_surface;
+
+ dri2_surf->dri_drawable =
+ (*dri2_dpy->dri2->createNewDrawable)(dri2_dpy->dri_screen, config,
+ dri2_surf);
+ if (dri2_surf->dri_drawable == NULL) {
+ _eglError(EGL_BAD_ALLOC, "dri2->createNewDrawable");
+ goto cleanup_surface;
+}
+
+ if (conf->RedSize == 5)
+ dri2_surf->format = __DRI_IMAGE_FORMAT_RGB565;
+ else if (conf->AlphaSize == 0)
+ dri2_surf->format = __DRI_IMAGE_FORMAT_XRG
Re: [Mesa-dev] [v4 02/11] i965: Relax assertion of halign == 16 for lossless compressed aux
On Fri, May 06, 2016 at 10:19:58AM +0300, Pohjolainen, Topi wrote:
> On Thu, May 05, 2016 at 10:59:16AM -0700, Ben Widawsky wrote:
> > On Thu, May 05, 2016 at 10:51:32AM -0700, Ben Widawsky wrote:
> > > On Thu, Apr 21, 2016 at 02:58:57PM +0300, Topi Pohjolainen wrote:
> > > > Signed-off-by: Topi Pohjolainen
> > > > ---
> > > > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 7 ++-
> > > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > > index ae08300..b68575f 100644
> > > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > > @@ -556,8 +556,13 @@ intel_miptree_create_layout(struct brw_context
> > > > *brw,
> > > > } else if (brw->gen >= 9 && num_samples > 1) {
> > > >layout_flags |= MIPTREE_LAYOUT_FORCE_HALIGN16;
> > > > } else {
> > > > + const bool is_lossless_compressed_aux =
> > > > + brw->gen >= 9 && num_samples == 1 &&
> > > > + mt->format == MESA_FORMAT_R_UINT32;
> > > > +
> > > >/* For now, nothing else has this requirement */
> > > > - assert((layout_flags & MIPTREE_LAYOUT_FORCE_HALIGN16) == 0);
> > > > + assert(is_lossless_compressed_aux ||
> > > > + (layout_flags & MIPTREE_LAYOUT_FORCE_HALIGN16) == 0);
> > > > }
> > > >
> > > > brw_miptree_layout(brw, mt, layout_flags);
> > >
> > > I'd just drop the else condition entirely instead. The else case
> > > assertion also
> > > becomes invalid when we want to fast clear multi LOD and arrayed
> > > surfaces. I'm
> > > slightly worried that the assertion is somewhat frail and not future
> > > proof.
> > >
> > > Also the format check makes me wonder if you want to add some assertion or
> > > condition to intel_miptree_is_lossless_compressed? ie.
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > index 17c87a2..467a60b 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > @@ -296,6 +296,9 @@ intel_miptree_is_lossless_compressed(const struct
> > > brw_context *brw,
> > > if (mt->msaa_layout != INTEL_MSAA_LAYOUT_CMS)
> > >return false;
> > >
> > > + if (mt->format == MESA_FORMAT_R_UINT32)
> > > + return false;
> > > +
> > > /* And finally distinguish between msaa and single sample case. */
> > > return mt->num_samples <= 1;
> > > }
> > >
> > > Either way:
> > > Reviewed-by: Ben Widawsky
> >
> > Hmm. On second thought, doesn't this still violate what the spec says?
> > "When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E, HALIGN 16
> > must be
> > used."
>
> I'm not sure if I understood this question correctly. The rational for the
> patch is that intel_miptree_alloc_non_msrt_mcs() calls miptree creation
> for the aux the buffer with MIPTREE_LAYOUT_FORCE_HALIGN16. This ends up into
> the branch addressed above. Format in question in MESA_FORMAT_R_UINT32 for
> which neither of the first two conditions apply:
>
>if (intel_miptree_supports_non_msrt_fast_clear(brw, mt)) {
> ...
>} else if (brw->gen >= 9 && num_samples > 1) {
> ...
>} else {
>
> and hence it drops to the default where the assertion hits unless it is
> relaxed.
>
Forgive me, I misread the assertion actually. You already have my r-b whether or
not you drop the else entirely...
--
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: enforce invariant conditions for built-in variables
The conditions for which certain built-in special variables
can be declared invariant were not being checked.
OpenGL ES 1.00 specification "Invariance and linkage":
For the built-in special variables, gl_FragCoord can
only be declared invariant if and only if gl_Position is
declared invariant. Similarly gl_PointCoord can only be
declared invariant if and only if gl_PointSize is declared
invariant. It is an error to declare gl_FrontFacing as invariant.
This fixes the following piglit tests in spec/glsl-es-1.00/linker:
glsl-fcoord-invariant
glsl-fface-invariant
glsl-pcoord-invariant
---
NOTE: Someone with access will need to commit this after the
review process
src/compiler/glsl/link_varyings.cpp | 46 +++--
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/src/compiler/glsl/link_varyings.cpp
b/src/compiler/glsl/link_varyings.cpp
index 34e82c7..8ef815c 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -352,13 +352,23 @@ cross_validate_outputs_to_inputs(struct gl_shader_program
*prog,
glsl_symbol_table parameters;
ir_variable *explicit_locations[MAX_VARYING][4] = { {NULL, NULL} };
+ bool is_gl_position_invariant = false;
+ bool is_gl_point_size_invariant = false;
+
/* Find all shader outputs in the "producer" stage.
*/
foreach_in_list(ir_instruction, node, producer->ir) {
ir_variable *const var = node->as_variable();
if ((var == NULL) || (var->data.mode != ir_var_shader_out))
-continue;
+ continue;
+
+ if (prog->IsES) {
+ if (!strcmp(var->name, "gl_Position"))
+is_gl_position_invariant = var->data.invariant;
+ if (!strcmp(var->name, "gl_PointSize"))
+is_gl_point_size_invariant = var->data.invariant;
+ }
if (!var->data.explicit_location
|| var->data.location < VARYING_SLOT_VAR0)
@@ -442,7 +452,39 @@ cross_validate_outputs_to_inputs(struct gl_shader_program
*prog,
ir_variable *const input = node->as_variable();
if ((input == NULL) || (input->data.mode != ir_var_shader_in))
-continue;
+ continue;
+
+ /*
+ * OpenGL ES 1.00 specification "Invariance and linkage":
+ *
+ * "For the built-in special variables, gl_FragCoord can
+ * only be declared invariant if and only if gl_Position is
+ * declared invariant. Similarly gl_PointCoord can only be
+ * declared invariant if and only if gl_PointSize is declared
+ * invariant. It is an error to declare gl_FrontFacing as invariant."
+ */
+ if (prog->IsES) {
+ if (!strcmp(input->name, "gl_FrontFacing") &&
+ input->data.invariant) {
+linker_error(prog,
+ "gl_FrontFacing cannot be declared invariant");
+return;
+ } else if (!strcmp(input->name, "gl_FragCoord") &&
+input->data.invariant &&
+!is_gl_position_invariant) {
+linker_error(prog,
+ "gl_FragCoord cannot be declared invariant "
+ "unless gl_Position is also invariant");
+return;
+ } else if (!strcmp(input->name, "gl_PointCoord") &&
+input->data.invariant &&
+!is_gl_point_size_invariant) {
+linker_error(prog,
+ "gl_PointCoord cannot be declared invariant "
+ "unless gl_PointSize is also invariant");
+return;
+ }
+ }
if (strcmp(input->name, "gl_Color") == 0 && input->data.used) {
const ir_variable *const front_color =
--
2.5.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium: enable intel jitevents profiling
Am 06.05.2016 um 21:51 schrieb Rowley, Timothy O:
> Would like to get this small change reviewed to help with performance
> measuring.
>
> Thanks.
>
> -Tim
>
>> On Mar 28, 2016, at 2:29 PM, Rowley, Timothy O
>> wrote:
>>
>> LLVM when configured with "intel jitevents" enabled can inform
>> VTune about dynamic code, so individual shaders are attributed
>> profiling data and the resulting assembly can be examined.
>> ---
>> src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 9 +
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> index 30ef37c..f9bb92d 100644
>> --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>> @@ -81,6 +81,11 @@
>> #include
>> #include
>>
>> +#include
>> +#if LLVM_USE_INTEL_JITEVENTS
>> +#include
>> +#endif
>> +
>> // Workaround http://llvm.org/PR23628
>> #if HAVE_LLVM >= 0x0307
>> # pragma pop_macro("DEBUG")
>> @@ -625,6 +630,10 @@
>> lp_build_create_jit_compiler_for_module(LLVMExecutionEngineRef *OutJIT,
>>ExecutionEngine *JIT;
>>
>>JIT = builder.create();
>> +#if LLVM_USE_INTEL_JITEVENTS
>> + JITEventListener *JEL = JITEventListener::createIntelJITEventListener();
>> + JIT->RegisterJITEventListener(JEL);
>> +#endif
>>if (JIT) {
>> *OutJIT = wrap(JIT);
>> return 0;
>> --
>> 1.9.1
>>
>
I know nothing about how that interface works, but looks reasonable
enough to me.
Acked-by: Roland Scheidegger
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium: enable intel jitevents profiling
Would like to get this small change reviewed to help with performance measuring.
Thanks.
-Tim
> On Mar 28, 2016, at 2:29 PM, Rowley, Timothy O
> wrote:
>
> LLVM when configured with "intel jitevents" enabled can inform
> VTune about dynamic code, so individual shaders are attributed
> profiling data and the resulting assembly can be examined.
> ---
> src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
> b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
> index 30ef37c..f9bb92d 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
> @@ -81,6 +81,11 @@
> #include
> #include
>
> +#include
> +#if LLVM_USE_INTEL_JITEVENTS
> +#include
> +#endif
> +
> // Workaround http://llvm.org/PR23628
> #if HAVE_LLVM >= 0x0307
> # pragma pop_macro("DEBUG")
> @@ -625,6 +630,10 @@
> lp_build_create_jit_compiler_for_module(LLVMExecutionEngineRef *OutJIT,
>ExecutionEngine *JIT;
>
>JIT = builder.create();
> +#if LLVM_USE_INTEL_JITEVENTS
> + JITEventListener *JEL = JITEventListener::createIntelJITEventListener();
> + JIT->RegisterJITEventListener(JEL);
> +#endif
>if (JIT) {
> *OutJIT = wrap(JIT);
> return 0;
> --
> 1.9.1
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 12/14] gallium/radeon: add a heuristic for better (S)DMA performance
On Thu, May 5, 2016 at 1:43 AM, Marek Olšák wrote:
> From: Marek Olšák
>
> ---
> src/gallium/drivers/radeon/r600_pipe_common.c | 16
> src/gallium/drivers/radeon/radeon_winsys.h| 2 ++
> src/gallium/winsys/amdgpu/drm/amdgpu_cs.c | 8
> src/gallium/winsys/radeon/drm/radeon_drm_cs.c | 8
> 4 files changed, 34 insertions(+)
>
> diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c
> b/src/gallium/drivers/radeon/r600_pipe_common.c
> index eac7812..c0579da 100644
> --- a/src/gallium/drivers/radeon/r600_pipe_common.c
> +++ b/src/gallium/drivers/radeon/r600_pipe_common.c
> @@ -176,6 +176,22 @@ void r600_dma_emit_wait_idle(struct r600_common_context
> *rctx)
> /* done at the end of DMA calls, so increment this. */
> rctx->num_dma_calls++;
>
> + /* IBs using too little memory are limited by the IB submission
> overhead.
> +* IBs using too much memory are limited by the kernel/TTM overhead.
> +* Too long IBs create CPU-GPU pipeline bubbles and add latency.
> +*
> +* This heuristic makes sure that DMA requests are executed
> +* very soon or immediately after the call is made and lowers memory
> +* usage. It improves texture upload performance by keeping the DMA
> +* engine busy while uploads are being submitted.
> +*
> +* 512 KB of copied data (1 MB / 2) is enough to submit an IB.
> +*/
> + if (rctx->ws->cs_query_memory_usage(rctx->dma.cs) > 1024 * 1024) {
> + rctx->dma.flush(rctx, RADEON_FLUSH_ASYNC, NULL);
FYI, I'm changing this from 1MB to 64MB, which yields slightly better
performance.
Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] winsys/amdgpu: add back multithreaded command submission
From: Marek Olšák
Ported from the initial amdgpu winsys from the private AMD branch.
The thread creates the buffer list, submits IBs, and cleans up
the submission context, which can also destroy buffers.
3-5% reduction in CPU overhead is expected for apps submitting a lot
of IBs per frame. This is most visible with DMA IBs.
---
src/gallium/winsys/amdgpu/drm/amdgpu_bo.c | 26 ++-
src/gallium/winsys/amdgpu/drm/amdgpu_bo.h | 4 +
src/gallium/winsys/amdgpu/drm/amdgpu_cs.c | 311 +-
src/gallium/winsys/amdgpu/drm/amdgpu_cs.h | 52 +++--
src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c | 61 +
src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h | 9 +
6 files changed, 333 insertions(+), 130 deletions(-)
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
index 37a41c0..ec5fa6a 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
@@ -43,8 +43,21 @@ static bool amdgpu_bo_wait(struct pb_buffer *_buf, uint64_t
timeout,
{
struct amdgpu_winsys_bo *bo = amdgpu_winsys_bo(_buf);
struct amdgpu_winsys *ws = bo->ws;
+ int64_t abs_timeout;
int i;
+ if (timeout == 0) {
+ if (p_atomic_read(&bo->num_active_ioctls))
+ return false;
+
+ } else {
+ abs_timeout = os_time_get_absolute_timeout(timeout);
+
+ /* Wait if any ioctl is being submitted with this buffer. */
+ if (!os_wait_until_zero_abs_timeout(&bo->num_active_ioctls, abs_timeout))
+ return false;
+ }
+
if (bo->is_shared) {
/* We can't use user fences for shared buffers, because user fences
* are local to this process only. If we want to wait for all buffer
@@ -61,7 +74,6 @@ static bool amdgpu_bo_wait(struct pb_buffer *_buf, uint64_t
timeout,
}
if (timeout == 0) {
- /* Timeout == 0 is quite simple. */
pipe_mutex_lock(ws->bo_fence_lock);
for (i = 0; i < RING_LAST; i++)
if (bo->fence[i]) {
@@ -80,7 +92,6 @@ static bool amdgpu_bo_wait(struct pb_buffer *_buf, uint64_t
timeout,
struct pipe_fence_handle *fence[RING_LAST] = {};
bool fence_idle[RING_LAST] = {};
bool buffer_idle = true;
- int64_t abs_timeout = os_time_get_absolute_timeout(timeout);
/* Take references to all fences, so that we can wait for them
* without the lock. */
@@ -214,8 +225,15 @@ static void *amdgpu_bo_map(struct pb_buffer *buf,
RADEON_USAGE_WRITE);
} else {
/* Mapping for write. */
-if (cs && amdgpu_bo_is_referenced_by_cs(cs, bo))
- cs->flush_cs(cs->flush_data, 0, NULL);
+if (cs) {
+ if (amdgpu_bo_is_referenced_by_cs(cs, bo)) {
+ cs->flush_cs(cs->flush_data, 0, NULL);
+ } else {
+ /* Try to avoid busy-waiting in radeon_bo_wait. */
+ if (p_atomic_read(&bo->num_active_ioctls))
+ amdgpu_cs_sync_flush(rcs);
+ }
+}
amdgpu_bo_wait((struct pb_buffer*)bo, PIPE_TIMEOUT_INFINITE,
RADEON_USAGE_READWRITE);
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.h
b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.h
index 69ada10..a768771 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.h
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.h
@@ -53,6 +53,10 @@ struct amdgpu_winsys_bo {
/* how many command streams is this bo referenced in? */
int num_cs_references;
+ /* how many command streams, which are being emitted in a separate
+* thread, is this bo referenced in? */
+ volatile int num_active_ioctls;
+
/* whether buffer_get_handle or buffer_from_handle was called,
* it can only transition from false to true
*/
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c
b/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c
index 03e45a9..419523b 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c
@@ -50,6 +50,7 @@ amdgpu_fence_create(struct amdgpu_ctx *ctx, unsigned ip_type,
fence->fence.ip_type = ip_type;
fence->fence.ip_instance = ip_instance;
fence->fence.ring = ring;
+ fence->submission_in_progress = true;
p_atomic_inc(&ctx->refcount);
return (struct pipe_fence_handle *)fence;
}
@@ -62,6 +63,7 @@ static void amdgpu_fence_submitted(struct pipe_fence_handle
*fence,
rfence->fence.fence = request->seq_no;
rfence->user_fence_cpu_address = user_fence_cpu_address;
+ rfence->submission_in_progress = false;
}
static void amdgpu_fence_signalled(struct pipe_fence_handle *fence)
@@ -69,6 +71,7 @@ static void amdgpu_fence_signalled(struct pipe_fence_handle
*fence)
struct amdgpu_fence *rfence = (struct amdgpu_fence*)fence;
rfence->signalled = true;
+ rfence->submission_in_progress = false;
}
bool amdgpu_fence_wait(struct pipe_fence_handle *fence, uint64_t timeout
Re: [Mesa-dev] [04.5/11] i965: Add flag telling if miptree is for client consumption
On Thu, May 05, 2016 at 07:08:15PM -0700, Ben Widawsky wrote:
> On Mon, Apr 25, 2016 at 08:10:01PM +0300, Topi Pohjolainen wrote:
> > Signed-off-by: Topi Pohjolainen
> > ---
> > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 7 ---
> > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 9 +
> > 2 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index b11fafd..cdf2fbd 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -394,6 +394,7 @@ intel_miptree_create_layout(struct brw_context *brw,
> > mt->logical_depth0 = depth0;
> > mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_NO_MCS;
> > mt->disable_aux_buffers = (layout_flags & MIPTREE_LAYOUT_DISABLE_AUX)
> > != 0;
> > + mt->is_scanout = (layout_flags & MIPTREE_LAYOUT_FOR_SCANOUT) != 0;
> > exec_list_make_empty(&mt->hiz_map);
> > mt->cpp = _mesa_get_format_bytes(format);
> > mt->num_samples = num_samples;
> > @@ -900,7 +901,7 @@ intel_update_winsys_renderbuffer_miptree(struct
> > brw_context *intel,
> > height,
> > 1,
> > pitch,
> > - 0);
> > +
> > MIPTREE_LAYOUT_FOR_SCANOUT);
> > if (!singlesample_mt)
> >goto fail;
> >
> > @@ -959,8 +960,8 @@ intel_miptree_create_for_renderbuffer(struct
> > brw_context *brw,
> > bool ok;
> > GLenum target = num_samples > 1 ? GL_TEXTURE_2D_MULTISAMPLE :
> > GL_TEXTURE_2D;
> > const uint32_t layout_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD |
> > - MIPTREE_LAYOUT_TILING_ANY;
> > -
> > + MIPTREE_LAYOUT_TILING_ANY |
> > + MIPTREE_LAYOUT_FOR_SCANOUT;
> >
> > mt = intel_miptree_create(brw, target, format, 0, 0,
> > width, height, depth, num_samples,
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > index d4f8563..bb06522 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > @@ -657,6 +657,13 @@ struct intel_mipmap_tree
> > */
> > bool disable_aux_buffers;
> >
> > + /**
> > +* Tells if the underlying buffer is to be also consumed by entities
> > other
> > +* than the driver. This allows logic to turn off features such as
> > lossless
> > +* compression which is not currently understood by client applications.
> > +*/
> > + bool is_scanout;
> > +
> > /* These are also refcounted:
> > */
> > GLuint refcount;
> > @@ -697,6 +704,8 @@ enum {
> > MIPTREE_LAYOUT_TILING_NONE = 1 << 6,
> > MIPTREE_LAYOUT_TILING_ANY = MIPTREE_LAYOUT_TILING_Y |
> > MIPTREE_LAYOUT_TILING_NONE,
> > +
> > + MIPTREE_LAYOUT_FOR_SCANOUT = 1 << 7,
> > };
> >
> > struct intel_mipmap_tree *intel_miptree_create(struct brw_context *brw,
>
> Since we hope that we one day can use these buffers for scanout, I'd go for
> something more generic, like (requires other changes as well):
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 21e4718..deea30d 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -687,11 +687,14 @@ enum {
> MIPTREE_LAYOUT_ACCELERATED_UPLOAD = 1 << 0,
> MIPTREE_LAYOUT_FORCE_ALL_SLICE_AT_LOD = 1 << 1,
> MIPTREE_LAYOUT_FOR_BO = 1 << 2,
> - MIPTREE_LAYOUT_DISABLE_AUX = 1 << 3,
> - MIPTREE_LAYOUT_FORCE_HALIGN16 = 1 << 4,
> -
> - MIPTREE_LAYOUT_TILING_Y = 1 << 5,
> - MIPTREE_LAYOUT_TILING_NONE = 1 << 6,
> + MIPTREE_LAYOUT_DISABLE_AUX_MCS /* CCS_D */ = 1 << 3,
> + MIPTREE_LAYOUT_DISABLE_AUX_CCS_E= 1 << 4,
> + MIPTREE_LAYOUT_DISABLE_AUX = MIPTREE_LAYOUT_DISABLE_AUX_MCS |
> +
> MIPTREE_LAYOUT_DISABLE_AUX_CCS_E,
> + MIPTREE_LAYOUT_FORCE_HALIGN16 = 1 << 5,
> +
> + MIPTREE_LAYOUT_TILING_Y = 1 << 6,
> + MIPTREE_LAYOUT_TILING_NONE = 1 << 7,
> MIPTREE_LAYOUT_TILING_ANY = MIPTREE_LAYOUT_TILING_Y |
> MIPTREE_LAYOUT_TILING_NONE,
> };
>
>
> I do like mt->is_scanout though. That seems useful. Either way:
> Reviewed-by: Ben Widawsky
I'm wondering what you had in mind in addition to the _LAYOUT_DISABLE_ flags?
These are not available anymore when inte
Re: [Mesa-dev] [PATCH v2 9/9] gbm: Add map/unmap functions
Rob Herring writes: > This adds map and unmap functions to GBM utilizing the DRIimage extension > mapImage/unmapImage functions or existing internal mapping for dumb > buffers. Unlike prior attempts, this version provides a region to map and > usage flags for the mapping. The operation follows the same semantics as > the gallium transfer_map() function. > > This was tested with GBM based gralloc on Android. > > This still creates a context, but I've moved it into gbm_create_device > rather than in the map function. This should remove any need for reference > counting and problems with memory leaks. This last bit of commit message is stale. With that dropped, and the other whitespace nit fixed, this series is: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 6/9] st/dri: Add support for DRIimage extension mapImage/unmapImage
Rob Herring writes:
> Implement support for mapImage/unmapImage functions in version 12 of the
> DRIimage extension.
>
> Signed-off-by: Rob Herring
> ---
> v2:
> - Make READ flag optional
>
> src/gallium/state_trackers/dri/dri2.c | 43
> ++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/state_trackers/dri/dri2.c
> b/src/gallium/state_trackers/dri/dri2.c
> index ab4208e..2b021313 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> @@ -1266,6 +1305,8 @@ static __DRIimageExtension dri2ImageExtension = {
> .createImageFromDmaBufs = NULL,
> .blitImage= dri2_blit_image,
> .getCapabilities = dri2_get_capabilities,
> +.mapImage= dri2_map_image,
> +.unmapImage = dri2_unmap_image,
Whitespace misalignment, it looks like.
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [AppVeyor] mesa master #1162 completed
Build mesa 1162 completed Commit a1f698881e by Eric Anholt on 5/6/2016 1:04 AM: vc4: Add support for loading immediate values in QIR.\n\nThis will be used for resetting the uniform stream in the presence of\nbranching, but may also be useful as an optimization to reduce how many\nuniforms we have to copy out per draw call (in exchange for increasing\nicache pressure). Configure your notification preferences ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] glsl: Copy propagate in array index function parameters
Could you add something to piglit which tries this with AoA to make
sure that your logic is right?
On Fri, May 6, 2016 at 11:54 AM, Juan A. Suarez Romero
wrote:
> Copy propagate is not applied in function parameters when they are out
> or inout.
>
> But if the parameter is an array, we can copy propagate the index array.
>
> This also fixes shaders@out-parameter-indexing piglit tests, that
> exposes a wrong handling of inout function parameters in Mesa.
>
> This commit does not produce any piglit regression, nor any change in
> shader-db results.
>
> v2:
> - Deal with swizzles (jasuarez)
> - Use a loop top peel off all array dereferences (Ilia)
> ---
> src/compiler/glsl/opt_copy_propagation.cpp | 18 ++
> 1 file changed, 18 insertions(+)
>
> diff --git a/src/compiler/glsl/opt_copy_propagation.cpp
> b/src/compiler/glsl/opt_copy_propagation.cpp
> index ae62921..e464de3 100644
> --- a/src/compiler/glsl/opt_copy_propagation.cpp
> +++ b/src/compiler/glsl/opt_copy_propagation.cpp
> @@ -193,6 +193,24 @@ ir_copy_propagation_visitor::visit_enter(ir_call *ir)
>if (sig_param->data.mode != ir_var_function_out
>&& sig_param->data.mode != ir_var_function_inout) {
> ir->accept(this);
> + } else {
> + /* Peeling of all array dereferences / swizzles */
> + ir_dereference_array *ir_array;
> + ir_swizzle *ir_swizzle;
> + do {
> +ir_array = ir->as_dereference_array();
> +if (ir_array) {
> + ir_array->array_index->accept(this);
> + ir = ir_array->array;
> + continue;
> +}
> +ir_swizzle = ir->as_swizzle();
> +if (ir_swizzle) {
> + ir = ir_swizzle->val;
> + continue;
> +}
> +ir = NULL;
> + } while (ir);
How about
while (true) {
if ((ir_array = ir->as_dereference_array()) {
} else if ((ir_swizzle = ir->as_swizzle()) {
} else {
break;
}
}
IMHO that better expresses what you're trying to do. Or something else
like that.
Also, are vector extracts still a thing at this point, or are they
already lowered? Like vec4[1] instead of vec4.y.
-ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] scons: Improve Python module dependency discovery.
On 05/06/2016 10:04 AM, Jose Fonseca wrote:
Several NIR scripts were using `from ... import ...` syntax, which wasn't
supported.
Using Python standard libary's modulefinder solves the problem with less
effort and hacks.
---
scons/custom.py | 27 +--
1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/scons/custom.py b/scons/custom.py
index 043793b..ff7a7a9 100644
--- a/scons/custom.py
+++ b/scons/custom.py
@@ -30,11 +30,10 @@ Custom builders and methods.
#
-import os
import os.path
-import re
import sys
import subprocess
+import modulefinder
import SCons.Action
import SCons.Builder
@@ -93,27 +92,19 @@ def createConvenienceLibBuilder(env):
return convenience_lib
-# TODO: handle import statements with multiple modules
-# TODO: handle from import statements
-import_re = re.compile(r'^\s*import\s+(\S+)\s*$', re.M)
-
def python_scan(node, env, path):
# http://www.scons.org/doc/0.98.5/HTML/scons-user/c2781.html#AEN2789
+# https://docs.python.org/2/library/modulefinder.html
contents = node.get_contents()
source_dir = node.get_dir()
-imports = import_re.findall(contents)
+finder = modulefinder.ModuleFinder()
+finder.run_script(node.abspath)
results = []
-for imp in imports:
-for dir in path:
-file = os.path.join(str(dir), imp.replace('.', os.sep) + '.py')
-if os.path.exists(file):
-results.append(env.File(file))
-break
-file = os.path.join(str(dir), imp.replace('.', os.sep),
'__init__.py')
-if os.path.exists(file):
-results.append(env.File(file))
-break
-#print node, map(str, results)
+for name, mod in finder.modules.iteritems():
+if mod.__file__ is None:
+continue
+assert os.path.exists(mod.__file__)
+results.append(env.File(mod.__file__))
return results
python_scanner = SCons.Scanner.Scanner(function = python_scan, skeys =
['.py'])
Looks OK to me.
Reviewed-by: Brian Paul
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 3/3] i965: Stop splitting fma() prior to optimization
According to the GLSL spec, if the user uses the fma() intrinsic to generate a precise-consumed value, and you have it in your hardware, you shouldn't split it. For a while now, we've been splitting all ffma's up-front and then planned to fuse them later which isn't valid. Correctly handling the GLSL behaviour fixes rendering corruptions in Tomb Raider. The only reason why doing this possibly helped before was for ARB programs which is handled by the previous commit. Shader-db results on Haswell: total instructions in shared programs: 7560300 -> 7561510 (0.02%) instructions in affected programs: 56265 -> 57475 (2.15%) helped: 86 HURT: 291 The only shaders in the database that are affected are from "Shadow of Mordor" which is the first app in our database to use fma(). We could, at some point in the future, split inexact ffma opcodes which would fix the shader-db regressions since Shadow of Mordor doesn't ues precise. However, this fixes a bug now and and the shader-db impact is fairly small. Reported-by: Kenneth Graunke Cc: "11.1 11.2" --- src/mesa/drivers/dri/i965/brw_compiler.c | 5 - 1 file changed, 5 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_compiler.c b/src/mesa/drivers/dri/i965/brw_compiler.c index 0ea5e8b..70dd5ad 100644 --- a/src/mesa/drivers/dri/i965/brw_compiler.c +++ b/src/mesa/drivers/dri/i965/brw_compiler.c @@ -68,11 +68,6 @@ shader_perf_log_mesa(void *data, const char *fmt, ...) } #define COMMON_OPTIONS\ - /* In order to help allow for better CSE at the NIR level we tell NIR to \ -* split all ffma instructions during opt_algebraic and we then re-combine \ -* them as a later step. \ -*/\ - .lower_ffma = true,\ .lower_sub = true, \ .lower_fdiv = true,\ .lower_scmp = true,\ -- 2.5.0.400.gff86faf ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] scons: Improve Python module dependency discovery.
Several NIR scripts were using `from ... import ...` syntax, which wasn't
supported.
Using Python standard libary's modulefinder solves the problem with less
effort and hacks.
---
scons/custom.py | 27 +--
1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/scons/custom.py b/scons/custom.py
index 043793b..ff7a7a9 100644
--- a/scons/custom.py
+++ b/scons/custom.py
@@ -30,11 +30,10 @@ Custom builders and methods.
#
-import os
import os.path
-import re
import sys
import subprocess
+import modulefinder
import SCons.Action
import SCons.Builder
@@ -93,27 +92,19 @@ def createConvenienceLibBuilder(env):
return convenience_lib
-# TODO: handle import statements with multiple modules
-# TODO: handle from import statements
-import_re = re.compile(r'^\s*import\s+(\S+)\s*$', re.M)
-
def python_scan(node, env, path):
# http://www.scons.org/doc/0.98.5/HTML/scons-user/c2781.html#AEN2789
+# https://docs.python.org/2/library/modulefinder.html
contents = node.get_contents()
source_dir = node.get_dir()
-imports = import_re.findall(contents)
+finder = modulefinder.ModuleFinder()
+finder.run_script(node.abspath)
results = []
-for imp in imports:
-for dir in path:
-file = os.path.join(str(dir), imp.replace('.', os.sep) + '.py')
-if os.path.exists(file):
-results.append(env.File(file))
-break
-file = os.path.join(str(dir), imp.replace('.', os.sep),
'__init__.py')
-if os.path.exists(file):
-results.append(env.File(file))
-break
-#print node, map(str, results)
+for name, mod in finder.modules.iteritems():
+if mod.__file__ is None:
+continue
+assert os.path.exists(mod.__file__)
+results.append(env.File(mod.__file__))
return results
python_scanner = SCons.Scanner.Scanner(function = python_scan, skeys = ['.py'])
--
2.5.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 95296] nir_lower_double_packing.c:79:4: error: void function 'lower_double_pack_impl' should not return a value [-Wreturn-type]
https://bugs.freedesktop.org/show_bug.cgi?id=95296 --- Comment #2 from Jason Ekstrand --- (In reply to Jose Fonseca from comment #1) > GCC (at least 5.2.1) also throws an warning... > > src/compiler/nir/nir_lower_double_packing.c: In function > âlower_double_pack_implâ: > src/compiler/nir/nir_lower_double_packing.c:79:11: warning: âreturnâ with > a > value, in function returning void > return true; >^ > > Don't older GCC versions warn? > > > I've took the liberty of pushing fix 8ae78f7d2828778a386ed1737f17add69eb035a9 Thanks! Patch looks good to me. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] glsl: Copy propagate in array index function parameters
Copy propagate is not applied in function parameters when they are out
or inout.
But if the parameter is an array, we can copy propagate the index array.
This also fixes shaders@out-parameter-indexing piglit tests, that
exposes a wrong handling of inout function parameters in Mesa.
This commit does not produce any piglit regression, nor any change in
shader-db results.
v2:
- Deal with swizzles (jasuarez)
- Use a loop top peel off all array dereferences (Ilia)
---
src/compiler/glsl/opt_copy_propagation.cpp | 18 ++
1 file changed, 18 insertions(+)
diff --git a/src/compiler/glsl/opt_copy_propagation.cpp
b/src/compiler/glsl/opt_copy_propagation.cpp
index ae62921..e464de3 100644
--- a/src/compiler/glsl/opt_copy_propagation.cpp
+++ b/src/compiler/glsl/opt_copy_propagation.cpp
@@ -193,6 +193,24 @@ ir_copy_propagation_visitor::visit_enter(ir_call *ir)
if (sig_param->data.mode != ir_var_function_out
&& sig_param->data.mode != ir_var_function_inout) {
ir->accept(this);
+ } else {
+ /* Peeling of all array dereferences / swizzles */
+ ir_dereference_array *ir_array;
+ ir_swizzle *ir_swizzle;
+ do {
+ir_array = ir->as_dereference_array();
+if (ir_array) {
+ ir_array->array_index->accept(this);
+ ir = ir_array->array;
+ continue;
+}
+ir_swizzle = ir->as_swizzle();
+if (ir_swizzle) {
+ ir = ir_swizzle->val;
+ continue;
+}
+ir = NULL;
+ } while (ir);
}
}
--
2.5.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: correctly handle inout parameters post function inlining
Sounds good to me, go ahead and push the new version! On Fri, May 6, 2016 at 10:44 AM, Juan A. Suarez Romero wrote: > On Fri, 2016-05-06 at 10:39 -0400, Lars Hamre wrote: >> Hi Juan, >> >> Sorry I missed that. >> >> It looks like your patch doesn't fix the out parameter indexing for: >> vs-inout-index-inout-mat2-col >> vs-inout-index-inout-vec4-array >> >> I was able to extend your patch to get these tests passing by: >> - if the ir_array->array was a dereferenced_array, copy propagate >> it's index >> - if the ir->ir_type == ir_type_swizzle, if it's val was a >> dereferenced_array, >> copying propagate it's index > > That's great! Precisely, I have here a version that fixes those two > test by doing exactly that :D > > > I can push a new version with that change, or if you prefer doing it > yourself, just let me know > > > J.A. > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: correctly handle inout parameters post function inlining
On Fri, 2016-05-06 at 10:39 -0400, Lars Hamre wrote: > Hi Juan, > > Sorry I missed that. > > It looks like your patch doesn't fix the out parameter indexing for: > vs-inout-index-inout-mat2-col > vs-inout-index-inout-vec4-array > > I was able to extend your patch to get these tests passing by: > - if the ir_array->array was a dereferenced_array, copy propagate > it's index > - if the ir->ir_type == ir_type_swizzle, if it's val was a > dereferenced_array, > copying propagate it's index That's great! Precisely, I have here a version that fixes those two test by doing exactly that :D I can push a new version with that change, or if you prefer doing it yourself, just let me know J.A. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: correctly handle inout parameters post function inlining
I think you want a loop there, peeling off all array dereferences, due to AoA (and maybe throw in some piglit tests to cover that). [Assuming AoA's can be function arguments... haven't checked the spec.] On Fri, May 6, 2016 at 10:39 AM, Lars Hamre wrote: > Hi Juan, > > Sorry I missed that. > > It looks like your patch doesn't fix the out parameter indexing for: > vs-inout-index-inout-mat2-col > vs-inout-index-inout-vec4-array > > I was able to extend your patch to get these tests passing by: > - if the ir_array->array was a dereferenced_array, copy propagate it's index > - if the ir->ir_type == ir_type_swizzle, if it's val was a > dereferenced_array, > copying propagate it's index > > I would rather move ahead with your patch as mine seems quite > "hacky" in comparison :) > > Regards, > Lars Hamre > > On Fri, May 6, 2016 at 9:53 AM, Juan A. Suarez Romero > wrote: >> On Fri, 2016-05-06 at 08:49 -0400, Lars Hamre wrote: >>> Inout parameters which depended on other inout parameters >>> where not assigned in the correct order. >>> >>> Fixes the following piglit tests in shaders/out-parameter-indexing: >>> vs-inout-index-inout-float-array >>> vs-inout-index-inout-mat2-col >>> vs-inout-index-inout-mat2-row >>> vs-inout-index-inout-vec4 >>> vs-inout-index-inout-vec4-array >>> vs-inout-index-inout-vec4-array-element >> >> >> Some days ago I sent a patch that also fixes some of these tests in a >> different way. >> >> https://lists.freedesktop.org/archives/mesa-dev/2016-May/115621.html >> >> >> >> J.A. >> >> ___ >> 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
Re: [Mesa-dev] [PATCH] glsl: correctly handle inout parameters post function inlining
Hi Juan, Sorry I missed that. It looks like your patch doesn't fix the out parameter indexing for: vs-inout-index-inout-mat2-col vs-inout-index-inout-vec4-array I was able to extend your patch to get these tests passing by: - if the ir_array->array was a dereferenced_array, copy propagate it's index - if the ir->ir_type == ir_type_swizzle, if it's val was a dereferenced_array, copying propagate it's index I would rather move ahead with your patch as mine seems quite "hacky" in comparison :) Regards, Lars Hamre On Fri, May 6, 2016 at 9:53 AM, Juan A. Suarez Romero wrote: > On Fri, 2016-05-06 at 08:49 -0400, Lars Hamre wrote: >> Inout parameters which depended on other inout parameters >> where not assigned in the correct order. >> >> Fixes the following piglit tests in shaders/out-parameter-indexing: >> vs-inout-index-inout-float-array >> vs-inout-index-inout-mat2-col >> vs-inout-index-inout-mat2-row >> vs-inout-index-inout-vec4 >> vs-inout-index-inout-vec4-array >> vs-inout-index-inout-vec4-array-element > > > Some days ago I sent a patch that also fixes some of these tests in a > different way. > > https://lists.freedesktop.org/archives/mesa-dev/2016-May/115621.html > > > > J.A. > > ___ > 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] glsl: correctly handle inout parameters post function inlining
On Fri, 2016-05-06 at 08:49 -0400, Lars Hamre wrote: > Inout parameters which depended on other inout parameters > where not assigned in the correct order. > > Fixes the following piglit tests in shaders/out-parameter-indexing: > vs-inout-index-inout-float-array > vs-inout-index-inout-mat2-col > vs-inout-index-inout-mat2-row > vs-inout-index-inout-vec4 > vs-inout-index-inout-vec4-array > vs-inout-index-inout-vec4-array-element Some days ago I sent a patch that also fixes some of these tests in a different way. https://lists.freedesktop.org/archives/mesa-dev/2016-May/115621.html J.A. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 95294] Support importing R8 and GR88 dma_bufs on ARM
https://bugs.freedesktop.org/show_bug.cgi?id=95294 --- Comment #1 from Nicolas Dufresne --- Hey, just a note that Stanimir Varbanov reported on both gstreamer and mesa devel that he added that already for Qualcomm driver. As this is a Gallium driver, I suppose he also added all the required bits to map the new formats. So ideally, contact him before re-writing this. http://gstreamer-devel.966125.n4.nabble.com/Re-gstreamer-v4l2videodec-plugin-td4676823.html -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [AppVeyor] mesa master #1161 failed
Build mesa 1161 failed Commit 44de03b0f8 by Samuel Pitoiset on 5/6/2016 2:25 PM: nvc0: unreference images when the context is destroyed\n\nLike other resources, we need to unreference all images.\n\nSigned-off-by: Samuel Pitoiset \nReviewed-by: Ilia Mirkin Configure your notification preferences ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0: unreference images when the context is destroyed
Reviewed-by: Ilia Mirkin On May 6, 2016 9:09 AM, "Samuel Pitoiset" wrote: > Like other resources, we need to unreference all images. > > Signed-off-by: Samuel Pitoiset > --- > src/gallium/drivers/nouveau/nvc0/nvc0_context.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.c > b/src/gallium/drivers/nouveau/nvc0/nvc0_context.c > index f6c58f2..428b33d 100644 > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.c > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.c > @@ -161,6 +161,10 @@ nvc0_context_unreference_resources(struct > nvc0_context *nvc0) >for (i = 0; i < NVC0_MAX_BUFFERS; ++i) > pipe_resource_reference(&nvc0->buffers[s][i].buffer, NULL); > > + for (s = 0; s < 6; ++s) > + for (i = 0; i < NVC0_MAX_IMAGES; ++i) > + pipe_resource_reference(&nvc0->images[s][i].resource, NULL); > + > for (i = 0; i < nvc0->num_tfbbufs; ++i) >pipe_so_target_reference(&nvc0->tfbbuf[i], NULL); > > -- > 2.8.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] nvc0: unreference images when the context is destroyed
Like other resources, we need to unreference all images. Signed-off-by: Samuel Pitoiset --- src/gallium/drivers/nouveau/nvc0/nvc0_context.c | 4 1 file changed, 4 insertions(+) diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.c b/src/gallium/drivers/nouveau/nvc0/nvc0_context.c index f6c58f2..428b33d 100644 --- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.c +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.c @@ -161,6 +161,10 @@ nvc0_context_unreference_resources(struct nvc0_context *nvc0) for (i = 0; i < NVC0_MAX_BUFFERS; ++i) pipe_resource_reference(&nvc0->buffers[s][i].buffer, NULL); + for (s = 0; s < 6; ++s) + for (i = 0; i < NVC0_MAX_IMAGES; ++i) + pipe_resource_reference(&nvc0->images[s][i].resource, NULL); + for (i = 0; i < nvc0->num_tfbbufs; ++i) pipe_so_target_reference(&nvc0->tfbbuf[i], NULL); -- 2.8.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: correctly handle inout parameters post function inlining
Inout parameters which depended on other inout parameters
where not assigned in the correct order.
Fixes the following piglit tests in shaders/out-parameter-indexing:
vs-inout-index-inout-float-array
vs-inout-index-inout-mat2-col
vs-inout-index-inout-mat2-row
vs-inout-index-inout-vec4
vs-inout-index-inout-vec4-array
vs-inout-index-inout-vec4-array-element
Signed-off-by: Lars Hamre
---
NOTES:
- Someone with access will need to commit this post
review process.
- Not sure if this is the "mesa" approach to this
problem, feedback is appreciated.
src/compiler/glsl/opt_function_inlining.cpp | 80 ++---
1 file changed, 73 insertions(+), 7 deletions(-)
diff --git a/src/compiler/glsl/opt_function_inlining.cpp
b/src/compiler/glsl/opt_function_inlining.cpp
index 19f5fae..a5e7ea6 100644
--- a/src/compiler/glsl/opt_function_inlining.cpp
+++ b/src/compiler/glsl/opt_function_inlining.cpp
@@ -95,6 +95,54 @@ replace_return_with_assignment(ir_instruction *ir, void
*data)
}
}
+static void
+gather_inout_vars_from_params(void *ctx,
+ exec_list *actual_params,
+ exec_list *callee_params,
+ exec_list *inout_vars)
+{
+ foreach_two_lists(formal_node, callee_params,
+ actual_node, actual_params) {
+ ir_rvalue *const param = (ir_rvalue *)actual_node;
+ const ir_variable *const sig_param = (ir_variable *)formal_node;
+
+ if (sig_param->data.mode == ir_var_function_inout &&
+param->ir_type == ir_type_dereference_variable) {
+ inout_vars->push_tail(sig_param->clone(ctx, NULL));
+ }
+ }
+}
+
+static bool
+rvalue_depends_on_inout_var_helper(ir_rvalue *rv, exec_list *inout_vars)
+{
+ if (rv->ir_type == ir_type_dereference_array) {
+ ir_dereference_array *deref_arr = (ir_dereference_array *)rv;
+ return (rvalue_depends_on_inout_var_helper(deref_arr->array, inout_vars)
||
+ rvalue_depends_on_inout_var_helper(deref_arr->array_index,
inout_vars));
+ } else if (rv->ir_type == ir_type_swizzle) {
+ return rvalue_depends_on_inout_var_helper(((ir_swizzle *)rv)->val,
inout_vars);
+ } else if (rv->ir_type == ir_type_dereference_variable) {
+ const char *var_name = ((ir_dereference_variable *)rv)->var->name;
+ foreach_in_list(ir_variable, inout_var, inout_vars) {
+ if (!strcmp(var_name, inout_var->name)) {
+return true;
+ }
+ }
+ }
+ return false;
+}
+
+static bool
+rvalue_depends_on_inout_var(ir_rvalue *rv, exec_list *inout_vars)
+{
+ if (rv->ir_type == ir_type_dereference_array ||
+ rv->ir_type == ir_type_swizzle) {
+ return rvalue_depends_on_inout_var_helper(rv, inout_vars);
+ }
+ return false;
+}
+
void
ir_call::generate_inline(ir_instruction *next_ir)
{
@@ -185,7 +233,14 @@ ir_call::generate_inline(ir_instruction *next_ir)
/* Copy back the value of any 'out' parameters from the function body
* variables to our own.
*/
+
+ exec_list inout_vars;
+ gather_inout_vars_from_params(ctx, &this->actual_parameters,
+ &this->callee->parameters,
+ &inout_vars);
+
i = 0;
+ ir_instruction *last_non_dependent_inout_assigned = next_ir;
foreach_two_lists(formal_node, &this->callee->parameters,
actual_node, &this->actual_parameters) {
ir_rvalue *const param = (ir_rvalue *) actual_node;
@@ -194,12 +249,24 @@ ir_call::generate_inline(ir_instruction *next_ir)
/* Move our param variable into the actual param if it's an 'out' type.
*/
if (parameters[i] && (sig_param->data.mode == ir_var_function_out ||
sig_param->data.mode == ir_var_function_inout)) {
-ir_assignment *assign;
-
-assign = new(ctx) ir_assignment(param->clone(ctx, NULL)->as_rvalue(),
-new(ctx)
ir_dereference_variable(parameters[i]),
-NULL);
-next_ir->insert_before(assign);
+ ir_assignment *assign;
+
+ assign = new(ctx) ir_assignment(param->clone(ctx, NULL)->as_rvalue(),
+ new(ctx)
ir_dereference_variable(parameters[i]),
+ NULL);
+
+ if (sig_param->data.mode == ir_var_function_inout &&
+ rvalue_depends_on_inout_var(param, &inout_vars)) {
+/*
+ * Insert assignment before the others if it depends on another
+ * inout parameter.
+ */
+last_non_dependent_inout_assigned->insert_before(assign);
+ } else {
+/* Otherwise insert assignment before the next_ir */
+next_ir->insert_before(assign);
+last_non_dependent_inout_assigned = assign;
+ }
}
++i;
@@ -210,7 +277,6 @@ ir_call::generate_inline(ir_instruction
Re: [Mesa-dev] [PATCH demos] egl: Add es2eglkms demo
Andreas, Adam, could you review?
Br, DS
On Fri, Apr 29, 2016 at 7:54 AM, Dongseong Hwang
wrote:
> This demo shows how ChromeOS renders OpenGL ES2 via kms, drm and gbm.
> This demo consists of
> 1. kms modesetting and vsync
> 2. EGL and GLES2 context creation
> 3. gbm bo creation and EGL Image binding
>
> drm connection code is copied from eglkms.
> ES2 rendering code is copied from es2tri.
>
> Signed-off-by: Dongseong Hwang
> ---
> src/egl/opengles2/Makefile.am | 9 +
> src/egl/opengles2/es2eglkms.c | 608
> ++
> 2 files changed, 617 insertions(+)
> create mode 100644 src/egl/opengles2/es2eglkms.c
>
> diff --git a/src/egl/opengles2/Makefile.am b/src/egl/opengles2/Makefile.am
> index b80ba50..2564a74 100644
> --- a/src/egl/opengles2/Makefile.am
> +++ b/src/egl/opengles2/Makefile.am
> @@ -45,6 +45,11 @@ endif
> if HAVE_WAYLAND
> bin_PROGRAMS += es2gears_wayland
> endif
> +if HAVE_DRM
> +if HAVE_GBM
> +bin_PROGRAMS += es2eglkms
> +endif
> +endif
> endif
> endif
>
> @@ -57,3 +62,7 @@ es2gears_x11_LDADD = ../eglut/libeglut_x11.la
>
> es2gears_wayland_SOURCES = es2gears.c
> es2gears_wayland_LDADD = ../eglut/libeglut_wayland.la
> +
> +es2eglkms_SOURCES = es2eglkms.c
> +es2eglkms_CFLAGS = $(AM_CFLAGS) $(DRM_CFLAGS) $(GBM_CFLAGS)
> +es2eglkms_LDADD = $(AM_LDFLAGS) $(DRM_LIBS) $(GBM_LIBS)
> \ No newline at end of file
> diff --git a/src/egl/opengles2/es2eglkms.c b/src/egl/opengles2/es2eglkms.c
> new file mode 100644
> index 000..95b3ade
> --- /dev/null
> +++ b/src/egl/opengles2/es2eglkms.c
> @@ -0,0 +1,608 @@
> +/*
> + * Copyright © 2016 Dongseong Hwang
> + *
> + * Permission to use, copy, modify, distribute, and sell this software
> and its
> + * documentation for any purpose is hereby granted without fee, provided
> that
> + * the above copyright notice appear in all copies and that both that
> copyright
> + * notice and this permission notice appear in supporting documentation,
> and
> + * that the name of the copyright holders not be used in advertising or
> + * publicity pertaining to distribution of the software without specific,
> + * written prior permission. The copyright holders make no
> representations
> + * about the suitability of this software for any purpose. It is
> provided "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT
> OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF
> USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
> PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#define EGL_EGLEXT_PROTOTYPES
> +#define GL_GLEXT_PROTOTYPES
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +// double buffering
> +#define NUM_BUFFERS 2
> +
> +struct framebuffer {
> + struct gbm_bo *bo;
> + int fd;
> + uint32_t fb_id;
> + EGLImageKHR image;
> + GLuint gl_tex;
> + GLuint gl_fb;
> +};
> +
> +struct kms {
> + drmModeConnector *connector;
> + drmModeEncoder *encoder;
> + drmModeModeInfo mode;
> +};
> +
> +struct gl {
> + GLuint program;
> + GLfloat view_rotz;
> + GLint u_matrix;
> + GLint attr_pos;
> + GLint attr_color;
> +
> + PFNEGLCREATESYNCKHRPROC eglCreateSyncKHR;
> + PFNEGLCLIENTWAITSYNCKHRPROC eglClientWaitSyncKHR;
> +};
> +
> +static void make_z_rot_matrix(GLfloat angle, GLfloat *m)
> +{
> + float c = cos(angle * M_PI / 180.0);
> + float s = sin(angle * M_PI / 180.0);
> + int i;
> + for (i = 0; i < 16; i++)
> + m[i] = 0.0;
> + m[0] = m[5] = m[10] = m[15] = 1.0;
> +
> + m[0] = c;
> + m[1] = s;
> + m[4] = -s;
> + m[5] = c;
> +}
> +
> +static void make_scale_matrix(GLfloat xs, GLfloat ys, GLfloat zs, GLfloat
> *m)
> +{
> + int i;
> + for (i = 0; i < 16; i++)
> + m[i] = 0.0;
> + m[0] = xs;
> + m[5] = ys;
> + m[10] = zs;
> + m[15] = 1.0;
> +}
> +
> +static void mul_matrix(GLfloat *prod, const GLfloat *a, const GLfloat *b)
> +{
> +#define A(row, col) a[(col << 2) + row]
> +#define B(row, col) b[(col << 2) + row]
> +#define P(row, col) p[(col << 2) + row]
> + GLfloat p[16];
> + GLint i;
> + for (i = 0; i < 4; i++) {
> + const GLfloat ai0 = A(i, 0), ai1 = A(i, 1), ai2 = A(i, 2), ai3 =
> A(i, 3);
> + P(i, 0) = ai0 * B(0, 0) + ai1 * B(1, 0) + ai2 * B(2, 0) + ai3 *
> B(3, 0);
> + P(i, 1) = ai0 * B(0, 1) + ai1 * B(1, 1) + ai2 * B(2, 1) + ai3 *
> B(3, 1);
> + P(i, 2) = ai0 * B(0, 2) + ai1 * B(1, 2) + ai2 * B(2, 2) + ai3 *
> B(3, 2);
> + P(i, 3) = ai0 * B(0, 3) + ai1 * B(1, 3) + ai2 * B(2, 3) + ai3 *
> B(3, 3);
> + }
> + me
[Mesa-dev] [Bug 95296] nir_lower_double_packing.c:79:4: error: void function 'lower_double_pack_impl' should not return a value [-Wreturn-type]
https://bugs.freedesktop.org/show_bug.cgi?id=95296 Jose Fonseca changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #1 from Jose Fonseca --- GCC (at least 5.2.1) also throws an warning... src/compiler/nir/nir_lower_double_packing.c: In function âlower_double_pack_implâ: src/compiler/nir/nir_lower_double_packing.c:79:11: warning: âreturnâ with a value, in function returning void return true; ^ Don't older GCC versions warn? I've took the liberty of pushing fix 8ae78f7d2828778a386ed1737f17add69eb035a9 -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/14] RadeonSI: SDMA texture copy rewrite
On 06.05.2016 19:09, Marek Olšák wrote: > On Fri, May 6, 2016 at 12:01 PM, Michel Dänzer wrote: >> On 06.05.2016 02:01, Marek Olšák wrote: There is one more hardware limitation that can cause VM faults with T2L copies and needs a workaround. The exact workaround is still under discussion, but I think this is good enough for review already. >>> >>> BTW, the T2L VM fault is completely harmless. It's caused by SDMA >>> reading more entries from the page directory than needed. The >>> workaround can be added in a follow-up patch. >> >> So you're proposing pushing these patches in a form which results in >> dmesg being flooded by GPUVM faults? If so, not sure that's a good idea, >> it'll probably result in bug reports. > > Unlikely, I haven't been able to reproduce it even with piglit and > that crazy DMA test. It came from a discussion with hw guys that such > a bug exists. I see, I thought this was still about the VM faults myself and Alex were seeing when playing with using the old SDMA code replaced by this series in more cases. > I can squash the fix with the right patch before pushing. The series can land without the workaround then as far as I'm concerned, once Nicolai or somebody else has reviewed it in detail. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/14] RadeonSI: SDMA texture copy rewrite
On Fri, May 6, 2016 at 12:01 PM, Michel Dänzer wrote: > On 06.05.2016 02:01, Marek Olšák wrote: >>> There is one more hardware limitation that can cause VM faults with T2L >>> copies and needs a workaround. The exact workaround is still under >>> discussion, but I think this is good enough for review already. >> >> BTW, the T2L VM fault is completely harmless. It's caused by SDMA >> reading more entries from the page directory than needed. The >> workaround can be added in a follow-up patch. > > So you're proposing pushing these patches in a form which results in > dmesg being flooded by GPUVM faults? If so, not sure that's a good idea, > it'll probably result in bug reports. Unlikely, I haven't been able to reproduce it even with piglit and that crazy DMA test. It came from a discussion with hw guys that such a bug exists. I can squash the fix with the right patch before pushing. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeonsi: Compute correct LDS size for fragment shaders.
Reviewed-by: Marek Olšák
Marek
On Thu, May 5, 2016 at 10:33 PM, Bas Nieuwenhuizen
wrote:
> No sure where the 36 came from, but we clearly need at least
> 48 bytes per attribute per primitive.
>
> Signed-off-by: Bas Nieuwenhuizen
> ---
> src/gallium/drivers/radeonsi/si_shader.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_shader.c
> b/src/gallium/drivers/radeonsi/si_shader.c
> index 49c498d..211db9f 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.c
> +++ b/src/gallium/drivers/radeonsi/si_shader.c
> @@ -5640,15 +5640,18 @@ static void si_shader_dump_stats(struct si_screen
> *sscreen,
>
> /* Compute LDS usage for PS. */
> if (processor == PIPE_SHADER_FRAGMENT) {
> - /* The minimum usage per wave is (num_inputs * 36). The
> maximum
> -* usage is (num_inputs * 36 * 16).
> + /* The minimum usage per wave is (num_inputs * 48). The
> maximum
> +* usage is (num_inputs * 48 * 16).
> * We can get anything in between and it varies between waves.
> *
> +* The 48 bytes per input for a single primitive is equal to
> +* 4 bytes/component * 4 components/input * 3 points.
> +*
> * Other stages don't know the size at compile time or don't
> * allocate LDS per wave, but instead they do it per thread
> group.
> */
> lds_per_wave = conf->lds_size * lds_increment +
> - align(num_inputs * 36, lds_increment);
> + align(num_inputs * 48, lds_increment);
> }
>
> /* Compute the per-SIMD wave counts. */
> --
> 2.8.2
>
> ___
> 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 00/14] RadeonSI: SDMA texture copy rewrite
On 06.05.2016 02:01, Marek Olšák wrote: >> There is one more hardware limitation that can cause VM faults with T2L >> copies and needs a workaround. The exact workaround is still under >> discussion, but I think this is good enough for review already. > > BTW, the T2L VM fault is completely harmless. It's caused by SDMA > reading more entries from the page directory than needed. The > workaround can be added in a follow-up patch. So you're proposing pushing these patches in a form which results in dmesg being flooded by GPUVM faults? If so, not sure that's a good idea, it'll probably result in bug reports. Apart from that, the series looks great to me. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: use var with initializer on global var validation
Currently, when cross validating global variables, all global variables
seen in the shaders that are part of a program are saved in a table.
When checking a variable this already exist in the table, we check both
are initialized to the same value. If the already saved variable does
not have an initializer, we copy it from the new variable.
Unfortunately this is wrong, as we are modifying something it is
constant. Also, if this modified variable is used in
another program, it will keep the initializer, when it should have none.
Instead of copying the initializer, this commit replaces the old
variable with the new one. So if we see again the same variable with an
initializer, we can compare if both are the same or not.
---
src/compiler/glsl/glsl_symbol_table.cpp | 10 ++
src/compiler/glsl/glsl_symbol_table.h | 5 +
src/compiler/glsl/linker.cpp| 27 +--
3 files changed, 20 insertions(+), 22 deletions(-)
diff --git a/src/compiler/glsl/glsl_symbol_table.cpp
b/src/compiler/glsl/glsl_symbol_table.cpp
index 6c682ac..6d7baad 100644
--- a/src/compiler/glsl/glsl_symbol_table.cpp
+++ b/src/compiler/glsl/glsl_symbol_table.cpp
@@ -278,3 +278,13 @@ glsl_symbol_table::disable_variable(const char *name)
entry->v = NULL;
}
}
+
+void
+glsl_symbol_table::replace_variable(const char *name,
+ir_variable *v)
+{
+ symbol_table_entry *entry = get_entry(name);
+ if (entry != NULL) {
+ entry->v = v;
+ }
+}
diff --git a/src/compiler/glsl/glsl_symbol_table.h
b/src/compiler/glsl/glsl_symbol_table.h
index 5d654e5..2f94d4c 100644
--- a/src/compiler/glsl/glsl_symbol_table.h
+++ b/src/compiler/glsl/glsl_symbol_table.h
@@ -100,6 +100,11 @@ struct glsl_symbol_table {
*/
void disable_variable(const char *name);
+ /**
+* Replaces the variable in the entry by the new variable.
+*/
+ void replace_variable(const char *name, ir_variable *v);
+
private:
symbol_table_entry *get_entry(const char *name);
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 9c72478..4522734 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1093,21 +1093,11 @@ cross_validate_globals(struct gl_shader_program *prog,
return;
}
} else {
- /* If the first-seen instance of a particular uniform did not
- * have an initializer but a later instance does, copy the
- * initializer to the version stored in the symbol table.
- */
- /* FINISHME: This is wrong. The constant_value field should
- * FINISHME: not be modified! Imagine a case where a shader
- * FINISHME: without an initializer is linked in two different
- * FINISHME: programs with shaders that have differing
- * FINISHME: initializers. Linking with the first will
- * FINISHME: modify the shader, and linking with the second
- * FINISHME: will fail.
- */
- existing->constant_initializer =
-var->constant_initializer->clone(ralloc_parent(existing),
- NULL);
+ /* If the first-seen instance of a particular uniform did
+ * not have an initializer but a later instance does,
+ * replace the former with the later.
+ */
+ variables.replace_variable(existing->name, var);
}
}
@@ -1121,13 +,6 @@ cross_validate_globals(struct gl_shader_program *prog,
var->name);
return;
}
-
- /* Some instance had an initializer, so keep track of that. In
- * this location, all sorts of initializers (constant or
- * otherwise) will propagate the existence to the variable
- * stored in the symbol table.
- */
- existing->data.has_initializer = true;
}
if (existing->data.invariant != var->data.invariant) {
--
2.5.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [v6 05/11] i965: Deferred allocation of mcs for lossless compressed
Until now mcs was associated to single sampled buffers only for
fast clear purposes and it was therefore the responsibility of the
clear logic to allocate the aux buffer when needed. Now that normal
3D render or blorp blit may render with mcs enabled also, they need
to prepare the mcs just as well.
v2: Do not enable for scanout buffers
v3 (Ben):
- Fix typo in commit message.
- Check for gen < 9 and return early in brw_predraw_set_aux_buffers()
- Check for gen < 9 and return early in intel_miptree_prepare_mcs()
Signed-off-by: Topi Pohjolainen
Reviewed-by: Ben Widawsky
---
src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 2 ++
src/mesa/drivers/dri/i965/brw_draw.c | 20 +++
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 35 +++
src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 4 +++
4 files changed, 61 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
index ab2ceec..0672213 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
@@ -1881,6 +1881,8 @@ brw_blorp_blit_miptrees(struct brw_context *brw,
intel_miptree_slice_resolve_depth(brw, src_mt, src_level, src_layer);
intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level, dst_layer);
+ intel_miptree_prepare_mcs(brw, dst_mt);
+
DBG("%s from %dx %s mt %p %d %d (%f,%f) (%f,%f)"
"to %dx %s mt %p %d %d (%f,%f) (%f,%f) (flip %d,%d)\n",
__func__,
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
b/src/mesa/drivers/dri/i965/brw_draw.c
index afa8a4e..db135d1 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -371,6 +371,25 @@ brw_postdraw_set_buffers_need_resolve(struct brw_context
*brw)
}
}
+static void
+brw_predraw_set_aux_buffers(struct brw_context *brw)
+{
+ if (brw->gen < 9)
+ return;
+
+ struct gl_context *ctx = &brw->ctx;
+ struct gl_framebuffer *fb = ctx->DrawBuffer;
+
+ for (unsigned i = 0; i < fb->_NumColorDrawBuffers; i++) {
+ struct intel_renderbuffer *irb =
+ intel_renderbuffer(fb->_ColorDrawBuffers[i]);
+
+ if (irb) {
+ intel_miptree_prepare_mcs(brw, irb->mt);
+ }
+ }
+}
+
/* May fail if out of video memory for texture or vbo upload, or on
* fallback conditions.
*/
@@ -416,6 +435,7 @@ brw_try_draw_prims(struct gl_context *ctx,
_mesa_fls(ctx->VertexProgram._Current->Base.SamplersUsed);
intel_prepare_render(brw);
+ brw_predraw_set_aux_buffers(brw);
/* This workaround has to happen outside of brw_upload_render_state()
* because it may flush the batchbuffer for a blit, affecting the state
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index f1f3def..2fb9d0e 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1628,6 +1628,41 @@ intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,
return mt->mcs_mt;
}
+void
+intel_miptree_prepare_mcs(struct brw_context *brw,
+ struct intel_mipmap_tree *mt)
+{
+ if (mt->mcs_mt)
+ return;
+
+ if (brw->gen < 9)
+ return;
+
+ /* Clients are not currently capable of consuming compressed
+* single-sampled buffers.
+*/
+ if (mt->is_scanout)
+ return;
+
+ /* Consider if lossless compression is supported but the needed
+* auxiliary buffer doesn't exist yet.
+*/
+ if (!intel_tiling_supports_non_msrt_mcs(brw, mt->tiling) ||
+ !intel_miptree_supports_lossless_compressed(brw, mt))
+ return;
+
+ /* Failing to allocate the auxiliary buffer means running out of
+* memory. The pointer to the aux miptree is left NULL which should
+* signal non-compressed behavior.
+*/
+ if (!intel_miptree_alloc_non_msrt_mcs(brw, mt)) {
+ _mesa_warning(NULL,
+"Failed to allocated aux buffer for lossless"
+" compressed %p %u:%u %s\n",
+mt, mt->logical_width0, mt->logical_height0,
+_mesa_get_format_name(mt->format));
+ }
+}
/**
* Helper for intel_miptree_alloc_hiz() that sets
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index 7955c90..669daf1 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -694,6 +694,10 @@ bool
intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,
struct intel_mipmap_tree *mt);
+void
+intel_miptree_prepare_mcs(struct brw_context *brw,
+ struct intel_mipmap_tree *mt);
+
enum {
MIPTREE_LAYOUT_ACCELERATED_UPLOAD = 1 << 0,
MIPTREE_LAYOUT_FORCE_ALL_SLICE_AT_LOD = 1 << 1,
--
2.5.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
htt
Re: [Mesa-dev] [v4 04/11] i965: Add helper for lossless compression support
On Thu, May 05, 2016 at 11:17:41AM -0700, Ben Widawsky wrote:
> On Thu, Apr 21, 2016 at 02:58:59PM +0300, Topi Pohjolainen wrote:
> > v2: Check explicitly against base type of GL_FLOAT instead of
> > using _mesa_is_format_integer_color(). Otherwise we miss
> > GL_UNSIGNED_NORMALIZED.
> >
> > Signed-off-by: Topi Pohjolainen
> > ---
> > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 22 ++
> > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 3 +++
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index b68575f..78717df 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -294,6 +294,28 @@ intel_miptree_is_lossless_compressed(const struct
> > brw_context *brw,
> > return mt->num_samples <= 1;
> > }
> >
> > +bool
> > +intel_miptree_supports_lossless_compressed(mesa_format format)
> > +{
> > + /* For now compression is only enabled for integer formats even though
> > +* there exist supported floating point formats also. This is a
> > heuristic
> > +* decision based on current public benchmarks. In none of the cases
> > these
> > +* formats provided any improvement but a few cases were seen to
> > regress.
> > +* Hence these are left to to be enabled in the future when they are
> > known
> > +* to improve things.
> > +*/
> > + if (_mesa_get_format_datatype(format) == GL_FLOAT)
> > + return false;
> > +
> > + /* In principle, fast clear mechanism and lossless compression go hand
> > in
> > +* hand. However, fast clear can be also used to clear srgb surfaces by
> > +* using equivalent linear format. This trick, however, can't be
> > extended
> > +* to be used with lossless compression and therefore a check is needed
> > to
> > +* see if the format really is linear.
> > +*/
> > + return _mesa_get_srgb_format_linear(format) == format;
> > +}
> > +
>
> Picking nits here, but I don't like this function because it doesn't do what
> it
> purports to do in it's name. It's really an ancillary function to MCS support.
> How about?
I like it, changed locally and rebased the rest of the series (two patches)
to use it.
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 20c8a7f..17a6f7e 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -301,7 +301,9 @@ intel_miptree_is_lossless_compressed(const struct
> brw_context *brw,
> }
>
> bool
> -intel_miptree_supports_lossless_compressed(mesa_format format)
> +intel_miptree_supports_lossless_compressed(struct brw_context *brw,
> + const struct intel_mipmap_tree
> *mt,
> + mesa_format format)
> {
> /* For now compression is only enabled for integer formats even though
> * there exist supported floating point formats also. This is a heuristic
> @@ -313,11 +315,14 @@ intel_miptree_supports_lossless_compressed(mesa_format
> format)
> if (_mesa_get_format_datatype(format) == GL_FLOAT)
>return false;
>
> - /* In principle, fast clear mechanism and lossless compression go hand in
> -* hand. However, fast clear can be also used to clear srgb surfaces by
> -* using equivalent linear format. This trick, however, can't be extended
> -* to be used with lossless compression and therefore a check is needed to
> -* see if the format really is linear.
> + /* Fast clear mechanism and lossless compression go hand in hand. */
> + if (!intel_miptree_supports_non_msrt_fast_clear(brw, mt))
> + return false;
> +
> + /* Fast clear can be also used to clear srgb surfaces by using equivalent
> +* linear format. This trick, however, can't be extended to be used with
> +* lossless compression and therefore a check is needed to see if the
> format
> +* really is linear.
> */
> return _mesa_get_srgb_format_linear(format) == format;
> }
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index d4f8563..c859f6e 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -680,7 +680,9 @@ intel_miptree_supports_non_msrt_fast_clear(struct
> brw_context *brw,
> const struct intel_mipmap_tree
> *mt);
>
> bool
> -intel_miptree_supports_lossless_compressed(mesa_format format);
> +intel_miptree_supports_lossless_compressed(struct brw_context *brw,
> + const struct intel_mipmap_tree
> *mt,
> + mesa_format format);
>
> bool
> intel_miptree_alloc_non_msrt_mcs(struct brw_context *b
[Mesa-dev] [Bug 95296] nir_lower_double_packing.c:79:4: error: void function 'lower_double_pack_impl' should not return a value [-Wreturn-type]
https://bugs.freedesktop.org/show_bug.cgi?id=95296 Bug ID: 95296 Summary: nir_lower_double_packing.c:79:4: error: void function 'lower_double_pack_impl' should not return a value [-Wreturn-type] Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: All Status: NEW Keywords: bisected, regression Severity: normal Priority: medium Component: Mesa core Assignee: mesa-dev@lists.freedesktop.org Reporter: v...@freedesktop.org QA Contact: mesa-dev@lists.freedesktop.org CC: cwabbo...@gmail.com, i...@freedesktop.org, ja...@jlekstrand.net mesa: bd326c229c528a214c9fda705e7a961cfa49ac9e (master 11.3.0-devel) CC nir/nir_libnir_la-nir_lower_double_packing.lo nir/nir_lower_double_packing.c:79:4: error: void function 'lower_double_pack_impl' should not return a value [-Wreturn-type] return true; ^ Build error introduced by this commit. commit 450c0613627d5a472fcf1122c15b66988abfb372 Author: Jason Ekstrand Date: Wed Apr 13 16:26:57 2016 -0700 nir/lower_double_pack: fixup for new nir_foreach_block() Signed-off-by: Jason Ekstrand Reviewed-by: Connor Abbott Reviewed-by: Ian Romanick -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] add MAINTAINERS and get_maintainer.pl script
On 02.05.2016 20:01, Daniel Stone wrote: > On 2 May 2016 at 11:44, Rob Clark wrote: >> On Mon, May 2, 2016 at 2:15 AM, Michel Dänzer wrote: >>> So, what is this based on? Maybe I'm not looking in the right place, but >>> out of hundreds of changes in Git touching those files, I see one change >>> from you about six months ago and five changes with a Reviewed-by: tag >>> from you over a year ago. You didn't push any changes other than your >>> own either AFAICT. >>> >>> Looking at all of Mesa yields a similar picture; that is why I >>> previously questioned your authority to NAK patches in Mesa. >>> >>> Don't get me wrong, I'm not questioning your authority on all things >>> Wayland. Your review of Wayland related patches obviously carries a lot >>> of weight. But I'd expect to see a very different footprint in the Git >>> history from somebody who calls himself maintainer. >> >> fwiw, I had debated about renaming the file 'REVIEWERS' or something >> like that, to better reflect it's purpose (ie. it is more about >> finding the right people to CC to get reviews, rather than absolute >> 'maintainers' (like it is in the linux kernel). I'd left the name >> since I thought that would be less confusing. But maybe I should >> change it.. > > That's exactly what I meant, rather than a land grab on authority. But > if you're unhappy having my name there, [...] I'm happy with it being there, now that the intent has been clarified. > And if this is some kind of proxy argument about scanout formats, [...] It's not; I'm not the one who keeps referring to that patch indirectly. > The only concerns came from, as you say, someone with no grounds to > comment. That's not what I said or meant. Questioning authority to NAK patches != denying right to comment. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 56/59] i965/fs: push first double-based uniforms in push constant buffer
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256
Kenneth gave his R-b to this patch on IRC:
"i965/fs: push first double-based uniforms in push constant
buffer" gets my R-b
On 06/05/16 08:56, Samuel Iglesias Gonsálvez wrote:
> When there is a mix of definitions of uniforms with 32-bit or
> 64-bit data type sizes, the driver ends up doing misaligned access
> to double based variables in the push constant buffer.
>
> To fix this, this patch pushes first all the 64-bit variables and
> then the rest. Then, all the variables would be aligned to its data
> type size.
>
> Signed-off-by: Samuel Iglesias Gonsálvez
> --- src/mesa/drivers/dri/i965/brw_fs.cpp | 113
> +-- 1 file changed, 83
> insertions(+), 30 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp index 65aa3c7..7eaf1b4
> 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++
> b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -39,6 +39,7 @@ #include
> "brw_program.h" #include "brw_dead_control_flow.h" #include
> "compiler/glsl_types.h" +#include "program/prog_parameter.h"
>
> using namespace brw;
>
> @@ -2004,6 +2005,45 @@ fs_visitor::compact_virtual_grfs() return
> progress; }
>
> +static void +set_push_pull_constant_loc(unsigned uniform, int
> &chunk_start, bool contiguous, + int
> *push_constant_loc, int *pull_constant_loc, +
> unsigned &num_push_constants, + unsigned
> &num_pull_constants, + const unsigned
> max_push_components, + const unsigned
> max_chunk_size, + struct
> brw_stage_prog_data *stage_prog_data) +{ + /* This is the first
> live uniform in the chunk */ + if (chunk_start < 0) +
> chunk_start = uniform; + + /* If this element does not need to be
> contiguous with the next, we +* split at this point and
> everthing between chunk_start and u forms a +* single chunk. +
> */ + if (!contiguous) { + unsigned chunk_size = uniform -
> chunk_start + 1; + + /* Decide whether we should push or pull
> this parameter. In the + * Vulkan driver, push constants are
> explicitly exposed via the API + * so we push everything. In
> GL, we only push small arrays. + */ + if
> (stage_prog_data->pull_param == NULL || +
> (num_push_constants + chunk_size <= max_push_components && +
> chunk_size <= max_chunk_size)) { +
> assert(num_push_constants + chunk_size <= max_push_components); +
> for (unsigned j = chunk_start; j <= uniform; j++) +
> push_constant_loc[j] = num_push_constants++; + } else { +
> for (unsigned j = chunk_start; j <= uniform; j++) +
> pull_constant_loc[j] = num_pull_constants++; + } + +
> chunk_start = -1; + } +} + /** * Assign UNIFORM file registers to
> either push constants or pull constants. * @@ -2022,6 +2062,8 @@
> fs_visitor::assign_constant_locations()
>
> bool is_live[uniforms]; memset(is_live, 0, sizeof(is_live)); +
> bool is_live_64bit[uniforms]; + memset(is_live_64bit, 0,
> sizeof(is_live_64bit));
>
> /* For each uniform slot, a value of true indicates that the given
> slot and * the next slot must remain contiguous. This is used to
> keep us from @@ -2054,14 +2096,21 @@
> fs_visitor::assign_constant_locations() for (unsigned j =
> constant_nr; j < last; j++) { is_live[j] = true; contiguous[j] =
> true; + if (type_sz(inst->src[i].type) == 8) { +
> is_live_64bit[j] = true; + } } is_live[last] = true;
> } else { if (constant_nr >= 0 && constant_nr < (int) uniforms) {
> int regs_read = inst->components_read(i) *
> type_sz(inst->src[i].type) / 4; - for (int j = 0; j <
> regs_read; j++) + for (int j = 0; j < regs_read; j++)
> { is_live[constant_nr + j] = true; + if
> (type_sz(inst->src[i].type) == 8) { +
> is_live_64bit[constant_nr + j] = true; + } +
> } } } } @@ -2090,43 +2139,46 @@
> fs_visitor::assign_constant_locations() pull_constant_loc =
> ralloc_array(mem_ctx, int, uniforms);
>
> int chunk_start = -1; + + + /* First push 64-bit uniforms */ for
> (unsigned u = 0; u < uniforms; u++) { - push_constant_loc[u] =
> -1; + if (!is_live[u] || !is_live_64bit[u]) +
> continue; + pull_constant_loc[u] = -1; + push_constant_loc[u]
> = -1;
>
> - if (!is_live[u]) - continue; +
> set_push_pull_constant_loc(u, chunk_start, contiguous[u], +
> push_constant_loc, pull_constant_loc, +
> num_push_constants, num_pull_constants, +
> max_push_components, max_chunk_size, +
> stage_prog_data);
>
> - /* This is the first live uniform in the chunk */ - if
> (chunk_start < 0) - chunk_start = u; + }
>
> - /* If this element does not need to be contiguous with the
> next, we - * split at this point and everthing between
> chunk_start and u forms a - * single chunk. - */ -
> if (!contiguous[u]) { - unsigne
[Mesa-dev] [PATCH] i965/gen8: Expose auxiliary mode resolver
Also use the opportunity to drop the unused surface type argument.
Signed-off-by: Topi Pohjolainen
Reviewed-by: Ben Widawsky
---
src/mesa/drivers/dri/i965/brw_state.h | 2 ++
src/mesa/drivers/dri/i965/gen8_surface_state.c | 9 -
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_state.h
b/src/mesa/drivers/dri/i965/brw_state.h
index e89b388..5c971d1 100644
--- a/src/mesa/drivers/dri/i965/brw_state.h
+++ b/src/mesa/drivers/dri/i965/brw_state.h
@@ -336,6 +336,8 @@ uint32_t *gen8_allocate_surface_state(struct brw_context
*brw,
void gen8_emit_fast_clear_color(const struct brw_context *brw,
const struct intel_mipmap_tree *mt,
uint32_t *surf);
+uint32_t gen8_get_aux_mode(const struct brw_context *brw,
+ const struct intel_mipmap_tree *mt);
/* brw_sampler_state.c */
void brw_emit_sampler_state(struct brw_context *brw,
diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c
b/src/mesa/drivers/dri/i965/gen8_surface_state.c
index 5161d2b..4f9a6e9 100644
--- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
@@ -198,10 +198,9 @@ gen8_emit_fast_clear_color(const struct brw_context *brw,
surf[7] |= mt->fast_clear_color_value;
}
-static uint32_t
+uint32_t
gen8_get_aux_mode(const struct brw_context *brw,
- const struct intel_mipmap_tree *mt,
- uint32_t surf_type)
+ const struct intel_mipmap_tree *mt)
{
if (mt->mcs_mt == NULL)
return GEN8_SURFACE_AUX_MODE_NONE;
@@ -237,7 +236,7 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
unsigned tiling_mode, pitch;
const unsigned tr_mode = surface_tiling_resource_mode(mt->tr_mode);
const uint32_t surf_type = translate_tex_target(target);
- uint32_t aux_mode = gen8_get_aux_mode(brw, mt, surf_type);
+ uint32_t aux_mode = gen8_get_aux_mode(brw, mt);
if (mt->format == MESA_FORMAT_S_UINT8) {
tiling_mode = GEN8_SURFACE_TILING_W;
@@ -484,7 +483,7 @@ gen8_update_renderbuffer_surface(struct brw_context *brw,
}
struct intel_mipmap_tree *aux_mt = mt->mcs_mt;
- const uint32_t aux_mode = gen8_get_aux_mode(brw, mt, surf_type);
+ const uint32_t aux_mode = gen8_get_aux_mode(brw, mt);
uint32_t *surf = gen8_allocate_surface_state(brw, &offset, surf_index);
--
2.5.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] add MAINTAINERS and get_maintainer.pl script
On 04.05.2016 00:11, Rob Clark wrote: > On Tue, May 3, 2016 at 9:56 AM, Daniel Vetter wrote: >> On Mon, May 02, 2016 at 06:44:34AM -0400, Rob Clark wrote: >>> On Mon, May 2, 2016 at 2:15 AM, Michel Dänzer wrote: On 25.04.2016 21:36, Daniel Stone wrote: > On 20 April 2016 at 00:32, Rob Clark wrote: >> On Tue, Apr 19, 2016 at 7:04 PM, Matt Turner wrote: >>> Let's let people add themselves to the file if they want. No point in >>> trying to populate it up front. >> >> yeah, I expect people to add themselves, and for the MAINTAINERS file >> to evolve over time.. if people like the idea I'll send a non-rfc >> version of the patch which whatever entries people ask me to add >> themselves for over the next week or so.. mostly just to avoid >> starting off with a completely empty file. But wasn't planning to >> wait for it to be completely populated to start with. > > If you want a bit more to add: > > WAYLAND EGL SUPPORT > R: Daniel Stone > F: src/egl/wayland/* > F: src/egl/drivers/dri2/platform_wayland.c So, what is this based on? Maybe I'm not looking in the right place, but out of hundreds of changes in Git touching those files, I see one change from you about six months ago and five changes with a Reviewed-by: tag from you over a year ago. You didn't push any changes other than your own either AFAICT. Looking at all of Mesa yields a similar picture; that is why I previously questioned your authority to NAK patches in Mesa. Don't get me wrong, I'm not questioning your authority on all things Wayland. Your review of Wayland related patches obviously carries a lot of weight. But I'd expect to see a very different footprint in the Git history from somebody who calls himself maintainer. >>> >>> fwiw, I had debated about renaming the file 'REVIEWERS' or something >>> like that, to better reflect it's purpose (ie. it is more about >>> finding the right people to CC to get reviews, rather than absolute >>> 'maintainers' (like it is in the linux kernel). I'd left the name >>> since I thought that would be less confusing. But maybe I should >>> change it.. >> >> I think a REVIEWERS would be really useful for mesa (we're getting to the >> point where no longer everyone knows everyone else), and would also be >> much clearer in conveying the intended usage. >> >> +1 on that from me, who's mostly an outside occasionally jumping in. And I >> think that'd be the audience for such a tool really. >> > > Ok, I've renamed to REVIEWERS and scripts/get_reviewer.pl and updated > the verbage appropriately. Thanks for clearing up the confusion. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 95294] Support importing R8 and GR88 dma_bufs on ARM
https://bugs.freedesktop.org/show_bug.cgi?id=95294 Bug ID: 95294 Summary: Support importing R8 and GR88 dma_bufs on ARM Product: Mesa Version: unspecified Hardware: ARM OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: EGL Assignee: mesa-dev@lists.freedesktop.org Reporter: dongseong.hw...@intel.com QA Contact: mesa-dev@lists.freedesktop.org R8 and GR88 dma_bufs are already supported for Intel, but not ARM https://lists.freedesktop.org/archives/mesa-commit/2015-July/057676.html ChromeOS will use it for video decoding. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v4 02/11] i965: Relax assertion of halign == 16 for lossless compressed aux
On Thu, May 05, 2016 at 10:59:16AM -0700, Ben Widawsky wrote:
> On Thu, May 05, 2016 at 10:51:32AM -0700, Ben Widawsky wrote:
> > On Thu, Apr 21, 2016 at 02:58:57PM +0300, Topi Pohjolainen wrote:
> > > Signed-off-by: Topi Pohjolainen
> > > ---
> > > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 7 ++-
> > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > index ae08300..b68575f 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > @@ -556,8 +556,13 @@ intel_miptree_create_layout(struct brw_context *brw,
> > > } else if (brw->gen >= 9 && num_samples > 1) {
> > >layout_flags |= MIPTREE_LAYOUT_FORCE_HALIGN16;
> > > } else {
> > > + const bool is_lossless_compressed_aux =
> > > + brw->gen >= 9 && num_samples == 1 &&
> > > + mt->format == MESA_FORMAT_R_UINT32;
> > > +
> > >/* For now, nothing else has this requirement */
> > > - assert((layout_flags & MIPTREE_LAYOUT_FORCE_HALIGN16) == 0);
> > > + assert(is_lossless_compressed_aux ||
> > > + (layout_flags & MIPTREE_LAYOUT_FORCE_HALIGN16) == 0);
> > > }
> > >
> > > brw_miptree_layout(brw, mt, layout_flags);
> >
> > I'd just drop the else condition entirely instead. The else case assertion
> > also
> > becomes invalid when we want to fast clear multi LOD and arrayed surfaces.
> > I'm
> > slightly worried that the assertion is somewhat frail and not future proof.
> >
> > Also the format check makes me wonder if you want to add some assertion or
> > condition to intel_miptree_is_lossless_compressed? ie.
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index 17c87a2..467a60b 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -296,6 +296,9 @@ intel_miptree_is_lossless_compressed(const struct
> > brw_context *brw,
> > if (mt->msaa_layout != INTEL_MSAA_LAYOUT_CMS)
> >return false;
> >
> > + if (mt->format == MESA_FORMAT_R_UINT32)
> > + return false;
> > +
> > /* And finally distinguish between msaa and single sample case. */
> > return mt->num_samples <= 1;
> > }
> >
> > Either way:
> > Reviewed-by: Ben Widawsky
>
> Hmm. On second thought, doesn't this still violate what the spec says?
> "When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E, HALIGN 16 must
> be
> used."
I'm not sure if I understood this question correctly. The rational for the
patch is that intel_miptree_alloc_non_msrt_mcs() calls miptree creation
for the aux the buffer with MIPTREE_LAYOUT_FORCE_HALIGN16. This ends up into
the branch addressed above. Format in question in MESA_FORMAT_R_UINT32 for
which neither of the first two conditions apply:
if (intel_miptree_supports_non_msrt_fast_clear(brw, mt)) {
...
} else if (brw->gen >= 9 && num_samples > 1) {
...
} else {
and hence it drops to the default where the assertion hits unless it is
relaxed.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [v4 01/11] i965/blorp: Set full resolve for lossless compressed
On Thu, May 05, 2016 at 10:36:07AM -0700, Ben Widawsky wrote:
> On Thu, Apr 21, 2016 at 02:58:56PM +0300, Topi Pohjolainen wrote:
> > Signed-off-by: Topi Pohjolainen
> > ---
> > src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> > b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> > index 41ff2a5..b1da935 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> > @@ -217,7 +217,10 @@
> > brw_blorp_rt_resolve_params::brw_blorp_rt_resolve_params(
> >
> > brw_get_resolve_rect(brw, mt, &x0, &y0, &x1, &y1);
> >
> > - fast_clear_op = GEN7_PS_RENDER_TARGET_RESOLVE_ENABLE;
> > + if (intel_miptree_is_lossless_compressed(brw, mt))
> > + fast_clear_op = GEN9_PS_RENDER_TARGET_RESOLVE_FULL;
> > + else
> > + fast_clear_op = GEN7_PS_RENDER_TARGET_RESOLVE_ENABLE;
> >
> > /* Note: there is no need to initialize push constants because it
> > doesn't
> > * matter what data gets dispatched to the render target. However, we
> > must
>
> Reviewed-by: Ben Widawsky
>
>
>
> After rebasing this it becomes params.fast_clear_op and I would be in favor
> of:
I like it, thanks. Just rebased and implemented it this way.
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h
> b/src/mesa/drivers/dri/i965/brw_blorp.h
> index c5c2c4e..d61e9d5 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.h
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
> @@ -225,7 +225,10 @@ struct brw_blorp_params
> struct brw_blorp_surface_info src;
> struct brw_blorp_surface_info dst;
> enum gen6_hiz_op hiz_op;
> - unsigned fast_clear_op;
> + union {
> + unsigned fast_clear_op;
> + unsigned resolve_type;
> + };
> bool color_write_disable[4];
> struct brw_blorp_wm_push_constants wm_push_consts;
> unsigned num_varyings;
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> index 6c48dfc..9b6c95b 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> @@ -411,9 +411,9 @@ brw_blorp_resolve_color(struct brw_context *brw, struct
> intel_mipmap_tree *mt)
> ¶ms.x1, ¶ms.y1);
>
> if (intel_miptree_is_lossless_compressed(brw, mt))
> - params.fast_clear_op = GEN9_PS_RENDER_TARGET_RESOLVE_FULL;
> + params.resolve_type = GEN9_PS_RENDER_TARGET_RESOLVE_FULL;
> else
> - params.fast_clear_op = GEN7_PS_RENDER_TARGET_RESOLVE_ENABLE;
> + params.resolve_type = GEN7_PS_RENDER_TARGET_RESOLVE_ENABLE;
>
> /* Note: there is no need to initialize push constants because it doesn't
> * matter what data gets dispatched to the render target. However, we
> must
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
