[Mesa-dev] [PATCH v2] nouveau: Silence GCC maybe-uninitialized warnings.
nouveau_compiler.c: In function ‘main’:
nouveau_compiler.c:216:27: warning: ‘code’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
printf("%08x ", code[i / 4]);
^
nouveau_compiler.c:215:4: warning: ‘size’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
for (i = 0; i < size; i += 4) {
^
Signed-off-by: Vinson Lee
---
src/gallium/drivers/nouveau/nouveau_compiler.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/gallium/drivers/nouveau/nouveau_compiler.c
b/src/gallium/drivers/nouveau/nouveau_compiler.c
index 8660498..c31885f 100644
--- a/src/gallium/drivers/nouveau/nouveau_compiler.c
+++ b/src/gallium/drivers/nouveau/nouveau_compiler.c
@@ -128,6 +128,8 @@ nouveau_codegen(int chipset, int type, struct tgsi_token
tokens[],
ret = nv50_ir_generate_code(&info);
if (ret) {
_debug_printf("Error compiling program: %d\n", ret);
+ *size = 0;
+ *code = NULL;
return ret;
}
--
2.3.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] nouveau: Silence GCC maybe-uninitialized warnings.
nouveau_compiler.c: In function ‘main’:
nouveau_compiler.c:216:27: warning: ‘code’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
printf("%08x ", code[i / 4]);
^
nouveau_compiler.c:215:4: warning: ‘size’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
for (i = 0; i < size; i += 4) {
^
Signed-off-by: Vinson Lee
---
src/gallium/drivers/nouveau/nouveau_compiler.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/gallium/drivers/nouveau/nouveau_compiler.c
b/src/gallium/drivers/nouveau/nouveau_compiler.c
index 8660498..c31885f 100644
--- a/src/gallium/drivers/nouveau/nouveau_compiler.c
+++ b/src/gallium/drivers/nouveau/nouveau_compiler.c
@@ -128,6 +128,8 @@ nouveau_codegen(int chipset, int type, struct tgsi_token
tokens[],
ret = nv50_ir_generate_code(&info);
if (ret) {
_debug_printf("Error compiling program: %d\n", ret);
+ *size = 0;
+ *code = NULL;
return ret;
}
--
2.3.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/13] i965: Fix the untyped surface opcodes to deal with indirect surface access.
On Friday, March 06, 2015 03:20:26 PM Pohjolainen, Topi wrote:
> On Fri, Mar 06, 2015 at 02:46:51PM +0200, Francisco Jerez wrote:
> > "Pohjolainen, Topi" writes:
> >
> > > On Fri, Mar 06, 2015 at 02:29:15PM +0200, Francisco Jerez wrote:
> > >> "Pohjolainen, Topi" writes:
> > >>
> > >> > On Fri, Feb 27, 2015 at 05:34:48PM +0200, Francisco Jerez wrote:
> > >> >> Change brw_untyped_atomic() and brw_untyped_surface_read() to take the
> > >> >> surface index as a register instead of a constant and to use
> > >> >> brw_send_indirect_message() to emit the indirect variant of send with
> > >> >> a dynamically calculated message descriptor. This will be required to
> > >> >> support variable indexing of image arrays for
> > >> >> ARB_shader_image_load_store.
> > >> >> ---
> > >> >> src/mesa/drivers/dri/i965/brw_eu.h | 10 +-
> > >> >> src/mesa/drivers/dri/i965/brw_eu_emit.c | 158
> > >> >> +--
> > >> >> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 4 +-
> > >> >> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 4 +-
> > >> >> 4 files changed, 96 insertions(+), 80 deletions(-)
> > >> >>
> > >> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> > >> >> b/src/mesa/drivers/dri/i965/brw_eu.h
> > >> >> index 87a9f3f..9cc9123 100644
> > >> >> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> > >> >> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> > >> >> @@ -398,18 +398,18 @@ void brw_CMP(struct brw_compile *p,
> > >> >>
> > >> >> void
> > >> >> brw_untyped_atomic(struct brw_compile *p,
> > >> >> - struct brw_reg dest,
> > >> >> + struct brw_reg dst,
> > >> >> struct brw_reg payload,
> > >> >> + struct brw_reg surface,
> > >> >> unsigned atomic_op,
> > >> >> - unsigned bind_table_index,
> > >> >> unsigned msg_length,
> > >> >> bool response_expected);
> > >> >>
> > >> >> void
> > >> >> brw_untyped_surface_read(struct brw_compile *p,
> > >> >> - struct brw_reg dest,
> > >> >> - struct brw_reg mrf,
> > >> >> - unsigned bind_table_index,
> > >> >> + struct brw_reg dst,
> > >> >> + struct brw_reg payload,
> > >> >> + struct brw_reg surface,
> > >> >> unsigned msg_length,
> > >> >> unsigned num_channels);
> > >> >>
> > >> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > >> >> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > >> >> index 0b655d4..34695bf 100644
> > >> >> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > >> >> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > >> >> @@ -2518,6 +2518,48 @@ brw_send_indirect_message(struct brw_compile
> > >> >> *p,
> > >> >> return setup;
> > >> >> }
> > >> >>
> > >> >> +static struct brw_inst *
> > >> >> +brw_send_indirect_surface_message(struct brw_compile *p,
> > >> >> + unsigned sfid,
> > >> >> + struct brw_reg dst,
> > >> >> + struct brw_reg payload,
> > >> >> + struct brw_reg surface,
> > >> >> + unsigned message_len,
> > >> >> + unsigned response_len,
> > >> >> + bool header_present)
> > >> >> +{
> > >> >> + const struct brw_context *brw = p->brw;
> > >> >> + struct brw_inst *insn;
> > >> >> +
> > >> >> + if (surface.file != BRW_IMMEDIATE_VALUE) {
> > >> >> + struct brw_reg addr = retype(brw_address_reg(0),
> > >> >> BRW_REGISTER_TYPE_UD);
> > >> >> +
> > >> >> + brw_push_insn_state(p);
> > >> >> + brw_set_default_access_mode(p, BRW_ALIGN_1);
> > >> >> + brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> > >> >> + brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
> > >> >> +
> > >> >> + /* Mask out invalid bits from the surface index to avoid hangs
> > >> >> e.g. when
> > >> >> + * some surface array is accessed out of bounds.
> > >> >> + */
> > >> >> + insn = brw_AND(p, addr,
> > >> >> + suboffset(vec1(retype(surface,
> > >> >> BRW_REGISTER_TYPE_UD)),
> > >> >> + BRW_GET_SWZ(surface.dw1.bits.swizzle,
> > >> >> 0)),
> > >> >> + brw_imm_ud(0xff));
> > >> >> +
> > >> >> + brw_pop_insn_state(p);
> > >> >> +
> > >> >> + surface = addr;
> > >> >> + }
> > >> >> +
> > >> >> + insn = brw_send_indirect_message(p, sfid, dst, payload, surface);
> > >> >> + brw_inst_set_mlen(brw, insn, message_len);
> > >> >> + brw_inst_set_rlen(brw, insn, response_len);
> > >> >> + brw_inst_set_header_present(brw, insn, header_present);
> > >> >
> > >> > I'll continue the discussion we started with
Re: [Mesa-dev] [PATCH 01/13] i965: Factor out logic to build a send message instruction with indirect descriptor.
On Friday, February 27, 2015 05:34:44 PM Francisco Jerez wrote: > --- > src/mesa/drivers/dri/i965/brw_eu.h | 19 ++-- > src/mesa/drivers/dri/i965/brw_eu_emit.c | 58 > ++-- > src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 55 +- > src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 37 --- > 4 files changed, 77 insertions(+), 92 deletions(-) FYI, I glanced over this series briefly, and it looks reasonable to me. It looks like Topi's done a pretty thorough review (thanks!) - if he's happy with it, that's good enough for me. (I suppose we could call that an Acked-by.) signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages
The original patch from Curro was based on something that is not
present in the master yet. This patch tries to mimick the logic on
top master.
I wanted to see if could separate the building of the descriptor
instruction from building of the send instruction. This logic now
allows the caller to construct any kind of sequence of instructions
filling in the descriptor before giving it to the send instruction
builder.
This is only compile tested. Curro, how would you feel about this
sort of approach? I apologise for muddying the waters again but I
wasn't entirely comfortable with the logic and wanted to try to
something else.
I believe patch number five should go nicely on top of this as
the descriptor instruction could be followed by (or preceeeded by)
any additional instructions modifying the descriptor register
before the actual send instruction.
Signed-off-by: Topi Pohjolainen
---
src/mesa/drivers/dri/i965/brw_eu.h | 17 +--
src/mesa/drivers/dri/i965/brw_eu_emit.c | 53 --
src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 58 ++--
src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 38
4 files changed, 75 insertions(+), 91 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
b/src/mesa/drivers/dri/i965/brw_eu.h
index 736c54b..82570f5 100644
--- a/src/mesa/drivers/dri/i965/brw_eu.h
+++ b/src/mesa/drivers/dri/i965/brw_eu.h
@@ -205,11 +205,6 @@ void brw_set_sampler_message(struct brw_compile *p,
unsigned simd_mode,
unsigned return_format);
-void brw_set_indirect_send_descriptor(struct brw_compile *p,
- brw_inst *insn,
- unsigned sfid,
- struct brw_reg descriptor);
-
void brw_set_dp_read_message(struct brw_compile *p,
brw_inst *insn,
unsigned binding_table_index,
@@ -242,6 +237,18 @@ void brw_urb_WRITE(struct brw_compile *p,
unsigned offset,
unsigned swizzle);
+struct brw_inst *
+brw_build_indirect_message_descr(struct brw_compile *p,
+ struct brw_reg dst,
+ struct brw_reg src);
+
+struct brw_inst *
+brw_send_indirect_message(struct brw_compile *p,
+ unsigned sfid,
+ struct brw_reg dst,
+ struct brw_reg payload,
+ struct brw_reg desc);
+
void brw_ff_sync(struct brw_compile *p,
struct brw_reg dest,
unsigned msg_reg_nr,
diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index 6f29468..cd0d199 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -772,21 +772,6 @@ brw_set_sampler_message(struct brw_compile *p,
}
}
-void brw_set_indirect_send_descriptor(struct brw_compile *p,
- brw_inst *insn,
- unsigned sfid,
- struct brw_reg descriptor)
-{
- /* Only a0.0 may be used as SEND's descriptor operand. */
- assert(descriptor.file == BRW_ARCHITECTURE_REGISTER_FILE);
- assert(descriptor.type == BRW_REGISTER_TYPE_UD);
- assert(descriptor.nr == BRW_ARF_ADDRESS);
- assert(descriptor.subnr == 0);
-
- brw_set_message_descriptor(p, insn, sfid, 0, 0, false, false);
- brw_set_src1(p, insn, descriptor);
-}
-
static void
gen7_set_dp_scratch_message(struct brw_compile *p,
brw_inst *inst,
@@ -2495,6 +2480,44 @@ void brw_urb_WRITE(struct brw_compile *p,
swizzle);
}
+struct brw_inst *
+brw_build_indirect_message_descr(struct brw_compile *p,
+ struct brw_reg dst,
+ struct brw_reg src)
+{
+ assert(dst.type == BRW_REGISTER_TYPE_UD);
+ assert(src.type == BRW_REGISTER_TYPE_UD);
+
+ brw_push_insn_state(p);
+ brw_set_default_access_mode(p, BRW_ALIGN_1);
+ brw_set_default_mask_control(p, BRW_MASK_DISABLE);
+ brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
+
+ struct brw_inst *setup = brw_OR(p, dst, src, brw_imm_ud(0));
+
+ brw_pop_insn_state(p);
+
+ return setup;
+}
+
+struct brw_inst *
+brw_send_indirect_message(struct brw_compile *p,
+ unsigned sfid,
+ struct brw_reg dst,
+ struct brw_reg payload,
+ struct brw_reg desc)
+{
+ const struct brw_context *brw = p->brw;
+ struct brw_inst *send = next_insn(p, BRW_OPCODE_SEND);
+
+ brw_set_src1(p, send, desc);
+ brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UD));
+ brw_set_src0(p, send, retype(payload, BRW_REGISTER_TYPE_UD));
+ brw_inst_set_sfid(brw, s
[Mesa-dev] [PATCH] mesa: fix ARB_copy_image internal format check
The memory layout of compatible internal formats may differ in bytes per
block, so TexFormat is not a reliable measure of compatibility. Additionally,
the current check allows compressed textures of the same block size to be used,
which is in violation of the spec.
Additionally, add a fixme for another check against TexFormat.
---
src/mesa/main/copyimage.c | 127 ++
1 file changed, 106 insertions(+), 21 deletions(-)
diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
index 455929d..dd0b8ea 100644
--- a/src/mesa/main/copyimage.c
+++ b/src/mesa/main/copyimage.c
@@ -33,6 +33,51 @@
#include "texobj.h"
#include "fbobject.h"
#include "textureview.h"
+#include "glformats.h"
+
+enum mesa_block_class {
+ BLOCK_CLASS_128_BITS,
+ BLOCK_CLASS_64_BITS,
+ BLOCK_CLASS_UNDEFINED
+};
+
+struct internal_format_block_class_info {
+ enum mesa_block_class block_class;
+ GLenum internal_format;
+};
+
+static const struct internal_format_block_class_info compressed_formats[] = {
+ {BLOCK_CLASS_128_BITS, GL_COMPRESSED_RGBA_S3TC_DXT3_EXT},
+ {BLOCK_CLASS_128_BITS, GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT},
+ {BLOCK_CLASS_128_BITS, GL_COMPRESSED_RGBA_S3TC_DXT5_EXT},
+ {BLOCK_CLASS_128_BITS, GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT},
+ {BLOCK_CLASS_128_BITS, GL_COMPRESSED_RG_RGTC2},
+ {BLOCK_CLASS_128_BITS, GL_COMPRESSED_SIGNED_RG_RGTC2},
+ {BLOCK_CLASS_128_BITS, GL_COMPRESSED_RGBA_BPTC_UNORM},
+ {BLOCK_CLASS_128_BITS, GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM},
+ {BLOCK_CLASS_128_BITS, GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT},
+ {BLOCK_CLASS_128_BITS, GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT},
+ {BLOCK_CLASS_64_BITS, GL_COMPRESSED_RGB_S3TC_DXT1_EXT},
+ {BLOCK_CLASS_64_BITS, GL_COMPRESSED_SRGB_S3TC_DXT1_EXT},
+ {BLOCK_CLASS_64_BITS, GL_COMPRESSED_RGBA_S3TC_DXT1_EXT},
+ {BLOCK_CLASS_64_BITS, GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT},
+ {BLOCK_CLASS_64_BITS, GL_COMPRESSED_RED_RGTC1},
+ {BLOCK_CLASS_64_BITS, GL_COMPRESSED_SIGNED_RED_RGTC1}
+};
+
+static const struct internal_format_block_class_info uncompressed_formats[] = {
+ {BLOCK_CLASS_128_BITS, GL_RGBA32UI},
+ {BLOCK_CLASS_128_BITS, GL_RGBA32I},
+ {BLOCK_CLASS_128_BITS, GL_RGBA32F},
+ {BLOCK_CLASS_64_BITS, GL_RGBA16F},
+ {BLOCK_CLASS_64_BITS, GL_RG32F},
+ {BLOCK_CLASS_64_BITS, GL_RGBA16UI},
+ {BLOCK_CLASS_64_BITS, GL_RG32UI},
+ {BLOCK_CLASS_64_BITS, GL_RGBA16I},
+ {BLOCK_CLASS_64_BITS, GL_RG32I},
+ {BLOCK_CLASS_64_BITS, GL_RGBA16},
+ {BLOCK_CLASS_64_BITS, GL_RGBA16_SNORM}
+};
static bool
prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int level,
@@ -253,6 +298,59 @@ check_region_bounds(struct gl_context *ctx,
struct gl_texture_image *tex_image,
return true;
}
+static bool
+compressed_format_compatible(struct gl_context *ctx,
+ GLenum compressedFormat, GLenum otherFormat)
+{
+ GLuint i;
+ enum mesa_block_class compressedClass = BLOCK_CLASS_UNDEFINED,
+ otherClass = BLOCK_CLASS_UNDEFINED;
+
+ if (_mesa_is_compressed_format(ctx, otherFormat)) {
+ return false;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(compressed_formats); i++) {
+ if (compressed_formats[i].internal_format == compressedFormat) {
+ compressedClass = compressed_formats[i].block_class;
+ break;
+ }
+ }
+
+ for (i = 0; i < ARRAY_SIZE(uncompressed_formats); i++) {
+ if (uncompressed_formats[i].internal_format == otherFormat) {
+ otherClass = uncompressed_formats[i].block_class;
+ break;
+ }
+ }
+
+ if (compressedClass == BLOCK_CLASS_UNDEFINED
+ || otherClass == BLOCK_CLASS_UNDEFINED)
+ return false;
+
+ return compressedClass == otherClass;
+}
+
+static bool
+copy_format_compatible(struct gl_context *ctx,
+GLenum srcFormat, GLenum dstFormat)
+{
+ if (srcFormat == dstFormat) {
+ return true;
+ }
+ else if (_mesa_is_compressed_format(ctx, srcFormat)) {
+ return compressed_format_compatible(ctx, srcFormat, dstFormat);
+ }
+ else if (_mesa_is_compressed_format(ctx, dstFormat)) {
+ return compressed_format_compatible(ctx, dstFormat, srcFormat);
+ }
+ else {
+ return _mesa_texture_view_compatible_format(ctx,
+ srcFormat,
+ dstFormat);
+ }
+}
+
void GLAPIENTRY
_mesa_CopyImageSubData(GLuint srcName, GLenum srcTarget, GLint srcLevel,
GLint srcX, GLint srcY, GLint srcZ,
@@ -265,7 +363,7 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum
srcTarget, GLint srcLevel,
struct gl_texture_object *srcTexObj, *dstTexObj;
struct gl_texture_image *srcTexImage, *dstTexImage;
GLuint src_bw, src_bh, dst_bw, dst_bh;
- int i, srcNewZ, dstNewZ, Bpt;
+ int i, srcNewZ, dstNewZ;
if (MESA_VERBOSE & VERBOSE_API)
_mesa_debug(ctx, "glCopyImageSubData(%u, %s, %d, %d, %d, %d, "
Re: [Mesa-dev] [PATCH 08/13] i965/vec4: Add support for untyped surface message sends from GRF.
On Fri, Mar 06, 2015 at 03:11:27PM +0200, Francisco Jerez wrote:
> "Pohjolainen, Topi" writes:
>
> > On Fri, Feb 27, 2015 at 05:34:51PM +0200, Francisco Jerez wrote:
> >> This doesn't actually enable untyped surface message sends from GRF
> >> yet, the upcoming atomic counter and image intrinsic lowering code
> >> will.
> >> ---
> >> src/mesa/drivers/dri/i965/brw_vec4.cpp | 7 ---
> >> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 16 +++-
> >> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 5 +++--
> >> 3 files changed, 14 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> >> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> >> index e19..0004b10 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> >> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> >> @@ -256,6 +256,8 @@ vec4_instruction::is_send_from_grf()
> >> switch (opcode) {
> >> case SHADER_OPCODE_SHADER_TIME_ADD:
> >> case VS_OPCODE_PULL_CONSTANT_LOAD_GEN7:
> >> + case SHADER_OPCODE_UNTYPED_ATOMIC:
> >> + case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> >>return true;
> >> default:
> >>return false;
> >> @@ -270,6 +272,8 @@ vec4_instruction::regs_read(unsigned arg) const
> >>
> >> switch (opcode) {
> >> case SHADER_OPCODE_SHADER_TIME_ADD:
> >> + case SHADER_OPCODE_UNTYPED_ATOMIC:
> >> + case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> >>return arg == 0 ? mlen : 1;
> >
> > Before the logic always falled back to returning one. Now we may return
> > one, two or three I think. I may be mistaken though, I'm just reading
> > vec4_visitor::emit_untyped_atomic() and it can produce message lengths up
> > to three.
> > Does this effect the instruction scheduling logic and if not, can you
> > explain why not?
> >
>
> Before my change that wouldn't ever happen because we were using fake
> MRFs to assemble the message payload and the MRF register index would be
> specified as inst->base_mrf, so the payload wouldn't be an actual source
> of the untyped surface instruction. This change adds an additional
> source for the payload, but a fake MRF is still passed in as explicit
> source temporarily. A future commit will change the vec4 visitor to
> build untyped and typed surface message payloads directly in normal GRFs
> instead of fake MRFs.
I checked the scheduler and confirmed this shouldn't change the current
behavior. If you like, you could add your explanation to the commit message
also. Either way:
Reviewed-by: Topi Pohjolainen
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Move need_throttle and first_post_swapbuffers_batch into the renderbuffer
On Thu, Mar 05, 2015 at 09:51:22PM -0500, Vivek Kasireddy wrote: > If there are multiple EGL surfaces associated with one EGL context, for > any given surface, we unnecessarily have to wait for swapbuffers to > complete on other EGL surfaces because we throttle to the context. > By moving first_post_swapbuffers_batch from the brw context into > intel_renderuffer, we would no longer be throttled by other surface's > swapbuffers thereby leading to better performance. Since you don't actually flush between drawables or track all drawables in a batch, nor is there any guarantee on the progression of render targets, this is not equivalent at all to the previous throttling scheme, and could trivially prevent throttling at all (which may be counter-intuitative but lead to lower throughput as well as hurt latency). Meanwhile it still succumbs to all the shortcomings of the previous scheme, since often it is the same render target being used over and over again.. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages
Topi Pohjolainen writes:
> The original patch from Curro was based on something that is not
> present in the master yet. This patch tries to mimick the logic on
> top master.
> I wanted to see if could separate the building of the descriptor
> instruction from building of the send instruction. This logic now
> allows the caller to construct any kind of sequence of instructions
> filling in the descriptor before giving it to the send instruction
> builder.
>
> This is only compile tested. Curro, how would you feel about this
> sort of approach? I apologise for muddying the waters again but I
> wasn't entirely comfortable with the logic and wanted to try to
> something else.
>
> I believe patch number five should go nicely on top of this as
> the descriptor instruction could be followed by (or preceeeded by)
> any additional instructions modifying the descriptor register
> before the actual send instruction.
>
Topi, the goal I had in mind with PATCH 01 was to refactor a commonly
recurring pattern. In terms of the functions defined in this patch my
example from yesterday [1] would now look like:
| if (index.file == BRW_IMMEDIATE_VALUE) {
|
| uint32_t surf_index = index.dw1.ud;
|
| brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND);
| brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW));
| brw_set_src0(p, send, offset);
| brw_set_sampler_message(p, send,
| surf_index,
| 0, /* LD message ignores sampler unit */
| GEN5_SAMPLER_MESSAGE_SAMPLE_LD,
| rlen,
| mlen,
| false, /* no header */
| simd_mode,
| 0);
|
| brw_mark_surface_used(prog_data, surf_index);
|
| } else {
|
| struct brw_reg addr = vec1(retype(brw_address_reg(0),
BRW_REGISTER_TYPE_UD));
|
| brw_push_insn_state(p);
| brw_set_default_mask_control(p, BRW_MASK_DISABLE);
| brw_set_default_access_mode(p, BRW_ALIGN_1);
|
| /* a0.0 = surf_index & 0xff */
| brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND);
| brw_inst_set_exec_size(p->brw, insn_and, BRW_EXECUTE_1);
| brw_set_dest(p, insn_and, addr);
| brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD)));
| brw_set_src1(p, insn_and, brw_imm_ud(0x0ff));
|
|
| /* a0.0 |= */
| brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr);
| brw_set_sampler_message(p, descr_inst,
| 0 /* surface */,
| 0 /* sampler */,
| GEN5_SAMPLER_MESSAGE_SAMPLE_LD,
| rlen /* rlen */,
| mlen /* mlen */,
| false /* header */,
| simd_mode,
| 0);
|
| /* dst = send(offset, a0.0) */
| brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr);
|
| brw_pop_insn_state(p);
|
| /* visitor knows more than we do about the surface limit required,
| * so has already done marking.
| */
| }
I agree that this would be a modest improvement, but it still requires
the caller to handle the immediate and indirect descriptor cases
explicitly, which is what I wanted to avoid because the exact same
mechanism would still have to be repeated in each of the 11 indirect
message sends (counting the new surface opcodes). If you look at my
PATCH 11-12, you won't find any explicit handling for indirect surface
indices, the code for emitting each of the typed or untyped surface
opcodes looks like:
| void
| brw_typed_surface_read(/* ... */)
| {
|const struct brw_context *brw = p->brw;
|const unsigned sfid = /* ... */;
|struct brw_inst *insn = brw_send_indirect_surface_message(
| p, sfid, dst, payload, surface, msg_length,
| brw_surface_payload_size(p, num_channels,
|brw->gen >= 8 || brw->is_haswell, false),
| true);
|
|brw_set_dp_typed_surface_read_message(
| p, insn, num_channels);
| }
Which works regardless of whether surface is an immediate or a register
thanks to brw_send_indirect_message().
[1] http://lists.freedesktop.org/archives/mesa-dev/2015-March/078746.html
> Signed-off-by: Topi Pohjolainen
> ---
> src/mesa/drivers/dri/i965/brw_eu.h | 17 +--
> src/mesa/drivers/dri/i965/brw_eu_emit.c | 53 --
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 58
> ++--
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 38
> 4 files changed, 75 insertions(+), 91 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> b/src/mesa/drivers/dri/i965/brw_eu.h
> index 736c54b..82570f5 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> +++ b/src/mesa/drivers/dri/i965/
Re: [Mesa-dev] New stable-branch 10.4 candidate pushed
On 5 March 2015 at 01:57, Matt Turner wrote: > On Wed, Mar 4, 2015 at 4:53 AM, Emil Velikov wrote: >> Trivial merge conflicts >> --- >> Here are the commits where I manually merged conflicts, so these might >> merit additional review: >> >> commit a598a9bdfe9f5d0ed35ca89a55cf74a2b678e8e1 >> Author: Matt Turner >> >> mesa: Correct backwards NULL check. >> >> (cherry picked from commit 491d42135ad0e5670756216154f2ba9fc79d4ba7) > > Again looks like I tagged something for 10.4 that doesn't apply. > > This one needs to be reverted (we've changed it so that if length is > NULL, dereference it!) > > If we want to apply fixes for GetProgramBinary, we need 201b9c18, > 4fd8b301, and f591712e first. > > I'd revert this commit, cherry-pick those just mentioned and then > cherry-pick 491d42135 on top of that. > I recall that the series was picked for 10.4, or perhaps I was meant to pick it up. That explains the unusual conflict. Thanks you Matt ! -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] Revert "common: Fix PBOs for 1D_ARRAY."
On 4 March 2015 at 23:15, Emil Velikov wrote:
> On 4 March 2015 at 17:22, Neil Roberts wrote:
>> This reverts commit 546aba143d13ba3f993ead4cc30b2404abfc0202.
>>
>> I think the changes to the calls to glBlitFramebuffer from this patch
>> are no different to what it was doing previously because it used to
>> set height to 1 before doing the blits. However it was introducing
>> some problems with the blit for layer 0 because this was no longer
>> special cased. It didn't fix problems with the yoffset which needs to
>> be interpreted as a slice offset. I think a better solution would be
>> to modify the original if statement to cope with the yoffset.
>>
> Neil, if others agree with this revert can you cc stable. Seems that
> the offending commit already has the tag, plus I've already picked it
> up :-\
>
Hi Neil,
There was a conflict while picking this for 10.5 due to commit
a44606eb816(meta: In pbo_{Get,}TexSubImage don't repeatedly rebind the
source tex). I've resolved it as follows, and I'm planning to commit
it around Tuesday lunchtime. Do let me know if you have any comments.
Thanks
Emil
diff --cc src/mesa/drivers/common/meta_tex_subimage.c
index 9f0c115,1fef79d..000
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@@ -213,12 -229,7 +219,9 @@@ _mesa_meta_pbo_TexSubImage(struct gl_co
GL_COLOR_BUFFER_BIT, GL_NEAREST))
goto fail;
-iters = tex_image->TexObject->Target == GL_TEXTURE_1D_ARRAY ?
-height : depth;
-
-for (z = 1; z < iters; z++) {
+for (z = 1; z < depth; z++) {
+ _mesa_meta_bind_fbo_image(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+pbo_tex_image, z);
_mesa_meta_bind_fbo_image(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
tex_image, zoffset + z);
@@@ -342,16 -352,9 +344,11 @@@ _mesa_meta_pbo_GetTexSubImage(struct gl
GL_COLOR_BUFFER_BIT, GL_NEAREST))
goto fail;
-if (tex_image && tex_image->TexObject->Target == GL_TEXTURE_1D_ARRAY)
- iters = height;
-else
- iters = depth;
-
-for (z = 1; z < iters; z++) {
+for (z = 1; z < depth; z++) {
_mesa_meta_bind_fbo_image(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
tex_image, zoffset + z);
+ _mesa_meta_bind_fbo_image(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+pbo_tex_image, z);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH 3/3] i965: Avoid applying negate to wrong MAD source.
On 27 February 2015 at 19:34, Matt Turner wrote: > For some given GLSL IR like (+ (neg x) (* 1.2 x)), the try_emit_mad > function would see that one of the +'s sources was a negate expression > and set mul_negate = true without confirming that it was actually a > multiply. > Hi Matt, This commit does not pick cleanly due to with commit 8cfd1e2ac6b(i965/fs: Emit MAD instructions when possible.) and 3654b6d43ce(i965/fs: Emit MADs from (x + abs(y * z)).) Can you please provide a backport for 10.5. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH 4/4] i965/fs: Don't issue FB writes for bound but unwritten color targets.
On 27 February 2015 at 08:06, Kenneth Graunke wrote: > We used to loop over all color attachments, and emit FB writes for each > one, even if the shader didn't write to a corresponding output variable. > Those color attachments would be filled with garbage (undefined values). > > Football Manager binds a framebuffer with 4 color attachments, but draws > to it using a shader that only writes to gl_FragData[0..2]. This meant > that color attachment 3 would be filled with garbage, resulting in > rendering artifacts. Now we skip writing to it, fixing rendering. > > Writes to gl_FragColor initialize outputs[0..nr_color_regions-1] to > GRFs, while writes to gl_FragData[i] initialize outputs[i]. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86747 > Signed-off-by: Kenneth Graunke > Cc: mesa-sta...@lists.freedesktop.org Hi Ken, This commit does not seems to have not landed in master. Did it fall through the cracks ? Cheers, Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH 3/3] i965: Avoid applying negate to wrong MAD source.
On Sat, Mar 7, 2015 at 9:54 AM, Emil Velikov wrote: > On 27 February 2015 at 19:34, Matt Turner wrote: >> For some given GLSL IR like (+ (neg x) (* 1.2 x)), the try_emit_mad >> function would see that one of the +'s sources was a negate expression >> and set mul_negate = true without confirming that it was actually a >> multiply. >> > Hi Matt, > > This commit does not pick cleanly due to with commit > 8cfd1e2ac6b(i965/fs: Emit MAD instructions when possible.) and > 3654b6d43ce(i965/fs: Emit MADs from (x + abs(y * z)).) Those commits are why this one is needed. I think the problem is that the i965/vec4 versions of those commits aren't on 10.5, so you can simply drop the changes to i965_vec4_visitor.cpp from this patch for 10.5. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: fix ARB_copy_image internal format check
[+jekstrand, who wrote the initial ARB_copy_image support and knows a
thing or two about texture formats]
On Sat, Mar 7, 2015 at 4:42 AM, Sean Burke wrote:
> The memory layout of compatible internal formats may differ in bytes per
> block, so TexFormat is not a reliable measure of compatibility. Additionally,
> the current check allows compressed textures of the same block size to be
> used,
> which is in violation of the spec.
>
> Additionally, add a fixme for another check against TexFormat.
> ---
> src/mesa/main/copyimage.c | 127
> ++
> 1 file changed, 106 insertions(+), 21 deletions(-)
>
> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
> index 455929d..dd0b8ea 100644
> --- a/src/mesa/main/copyimage.c
> +++ b/src/mesa/main/copyimage.c
> @@ -33,6 +33,51 @@
> #include "texobj.h"
> #include "fbobject.h"
> #include "textureview.h"
> +#include "glformats.h"
> +
> +enum mesa_block_class {
> + BLOCK_CLASS_128_BITS,
> + BLOCK_CLASS_64_BITS,
> + BLOCK_CLASS_UNDEFINED
> +};
> +
> +struct internal_format_block_class_info {
> + enum mesa_block_class block_class;
> + GLenum internal_format;
> +};
> +
> +static const struct internal_format_block_class_info compressed_formats[] = {
> + {BLOCK_CLASS_128_BITS, GL_COMPRESSED_RGBA_S3TC_DXT3_EXT},
> + {BLOCK_CLASS_128_BITS, GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT},
> + {BLOCK_CLASS_128_BITS, GL_COMPRESSED_RGBA_S3TC_DXT5_EXT},
> + {BLOCK_CLASS_128_BITS, GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT},
> + {BLOCK_CLASS_128_BITS, GL_COMPRESSED_RG_RGTC2},
> + {BLOCK_CLASS_128_BITS, GL_COMPRESSED_SIGNED_RG_RGTC2},
> + {BLOCK_CLASS_128_BITS, GL_COMPRESSED_RGBA_BPTC_UNORM},
> + {BLOCK_CLASS_128_BITS, GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM},
> + {BLOCK_CLASS_128_BITS, GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT},
> + {BLOCK_CLASS_128_BITS, GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT},
> + {BLOCK_CLASS_64_BITS, GL_COMPRESSED_RGB_S3TC_DXT1_EXT},
> + {BLOCK_CLASS_64_BITS, GL_COMPRESSED_SRGB_S3TC_DXT1_EXT},
> + {BLOCK_CLASS_64_BITS, GL_COMPRESSED_RGBA_S3TC_DXT1_EXT},
> + {BLOCK_CLASS_64_BITS, GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT},
> + {BLOCK_CLASS_64_BITS, GL_COMPRESSED_RED_RGTC1},
> + {BLOCK_CLASS_64_BITS, GL_COMPRESSED_SIGNED_RED_RGTC1}
> +};
> +
> +static const struct internal_format_block_class_info uncompressed_formats[]
> = {
> + {BLOCK_CLASS_128_BITS, GL_RGBA32UI},
> + {BLOCK_CLASS_128_BITS, GL_RGBA32I},
> + {BLOCK_CLASS_128_BITS, GL_RGBA32F},
> + {BLOCK_CLASS_64_BITS, GL_RGBA16F},
> + {BLOCK_CLASS_64_BITS, GL_RG32F},
> + {BLOCK_CLASS_64_BITS, GL_RGBA16UI},
> + {BLOCK_CLASS_64_BITS, GL_RG32UI},
> + {BLOCK_CLASS_64_BITS, GL_RGBA16I},
> + {BLOCK_CLASS_64_BITS, GL_RG32I},
> + {BLOCK_CLASS_64_BITS, GL_RGBA16},
> + {BLOCK_CLASS_64_BITS, GL_RGBA16_SNORM}
> +};
>
> static bool
> prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int
> level,
> @@ -253,6 +298,59 @@ check_region_bounds(struct gl_context *ctx,
> struct gl_texture_image *tex_image,
> return true;
> }
>
> +static bool
> +compressed_format_compatible(struct gl_context *ctx,
> + GLenum compressedFormat, GLenum otherFormat)
> +{
> + GLuint i;
> + enum mesa_block_class compressedClass = BLOCK_CLASS_UNDEFINED,
> + otherClass = BLOCK_CLASS_UNDEFINED;
> +
> + if (_mesa_is_compressed_format(ctx, otherFormat)) {
> + return false;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(compressed_formats); i++) {
> + if (compressed_formats[i].internal_format == compressedFormat) {
> + compressedClass = compressed_formats[i].block_class;
> + break;
> + }
> + }
Each one of these is just a handful of options, rather than going
through the list each time, it'd be a lot more performant to have each
one just be a
switch (glformat) {
case ...
case ...
return BLOCK_CLASS_64;
case ..
case ..
return 128
default:
return undefined;
}
> +
> + for (i = 0; i < ARRAY_SIZE(uncompressed_formats); i++) {
> + if (uncompressed_formats[i].internal_format == otherFormat) {
> + otherClass = uncompressed_formats[i].block_class;
> + break;
> + }
> + }
> +
> + if (compressedClass == BLOCK_CLASS_UNDEFINED
> + || otherClass == BLOCK_CLASS_UNDEFINED)
> + return false;
> +
> + return compressedClass == otherClass;
> +}
> +
> +static bool
> +copy_format_compatible(struct gl_context *ctx,
> +GLenum srcFormat, GLenum dstFormat)
> +{
> + if (srcFormat == dstFormat) {
> + return true;
> + }
> + else if (_mesa_is_compressed_format(ctx, srcFormat)) {
> + return compressed_format_compatible(ctx, srcFormat, dstFormat);
> + }
> + else if (_mesa_is_compressed_format(ctx, dstFormat)) {
> + return compressed_format_compatible(ctx, dstFormat, srcFormat);
> + }
> + else {
> + return _mesa_texture_view_compatible_format(ctx,
> +
Re: [Mesa-dev] [Mesa-stable] [PATCH 3/3] i965: Avoid applying negate to wrong MAD source.
On 7 March 2015 at 18:00, Matt Turner wrote: > On Sat, Mar 7, 2015 at 9:54 AM, Emil Velikov wrote: >> On 27 February 2015 at 19:34, Matt Turner wrote: >>> For some given GLSL IR like (+ (neg x) (* 1.2 x)), the try_emit_mad >>> function would see that one of the +'s sources was a negate expression >>> and set mul_negate = true without confirming that it was actually a >>> multiply. >>> >> Hi Matt, >> >> This commit does not pick cleanly due to with commit >> 8cfd1e2ac6b(i965/fs: Emit MAD instructions when possible.) and >> 3654b6d43ce(i965/fs: Emit MADs from (x + abs(y * z)).) > > Those commits are why this one is needed. I think the problem is that > the i965/vec4 versions of those commits aren't on 10.5, so you can > simply drop the changes to i965_vec4_visitor.cpp from this patch for > 10.5. Thank you Matt. The vec4 changes were during 10.6-devel, thus dropping leaves the following trivial conflict. I'll give the branch a quick piglit test and push it. -Emil --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp @@@ -452,15 -449,13 +447,18 @@@ fs_visitor::try_emit_mad(ir_expression mul_negate = true; } - if (!mul || mul->operation != ir_binop_mul) - return false; + if (mul && mul->operation == ir_binop_mul) + break; } +if (!mul || mul->operation != ir_binop_mul) + return false; + + if (nonmul->as_constant() || + mul->operands[0]->as_constant() || + mul->operands[1]->as_constant()) + return false; + nonmul->accept(this); fs_reg src0 = this->result; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/st: remove unused TexData
On 4 March 2015 at 22:26, Dave Airlie wrote: > this isn't hooked up to anything at all from what I can see. > > Signed-off-by: Dave Airlie I've noticed the same thing the other day, thanks to Ilia for bringing me here :-) Seems like a left over from commit 5d67d4fbebb(st/mesa: remove st_TexImage(), use core Mesa code instead). Fwiw Reviewed-by: Emil Velikov -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] nv30: Add unused attribute to function nv40_fp_bra.
On 07/03/15 07:43, Vinson Lee wrote: > Silences GCC unused-function warning. > > nv30/nvfx_fragprog.c:333:1: warning: ‘nv40_fp_bra’ defined but not used > [-Wunused-function] > nv40_fp_bra(struct nvfx_fpc *fpc, unsigned target) > ^ > > Signed-off-by: Vinson Lee > --- > src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c > b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c > index 6600997..abd51c8 100644 > --- a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c > +++ b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c > @@ -329,7 +329,7 @@ nv40_fp_rep(struct nvfx_fpc *fpc, unsigned count, > unsigned target) > } > > /* warning: this only works forward, and probably only if not inside any IF > */ > -static void > +static __attribute__((unused)) void Would be nice to define this as a macro in util/macros.h so that others can use it. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] mesa: fix ARB_copy_image internal format check
The memory layout of compatible internal formats may differ in bytes per
block, so TexFormat is not a reliable measure of compatibility. Additionally,
the current check allows compressed textures of the same block size to be used,
which is in violation of the spec.
v2: Use a switch instead of array iteration for block class and show the
correct GL error when internal formats are mismatched.
---
src/mesa/main/copyimage.c | 112 +-
1 file changed, 91 insertions(+), 21 deletions(-)
diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
index 455929d..eaedaa6 100644
--- a/src/mesa/main/copyimage.c
+++ b/src/mesa/main/copyimage.c
@@ -33,6 +33,12 @@
#include "texobj.h"
#include "fbobject.h"
#include "textureview.h"
+#include "glformats.h"
+
+enum mesa_block_class {
+ BLOCK_CLASS_128_BITS,
+ BLOCK_CLASS_64_BITS
+};
static bool
prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int level,
@@ -253,6 +259,85 @@ check_region_bounds(struct gl_context *ctx,
struct gl_texture_image *tex_image,
return true;
}
+static bool
+compressed_format_compatible(struct gl_context *ctx,
+ GLenum compressedFormat, GLenum otherFormat)
+{
+ enum mesa_block_class compressedClass, otherClass;
+
+ /* Two different compressed formats are never compatible. */
+ if (_mesa_is_compressed_format(ctx, otherFormat)) {
+ return false;
+ }
+
+ switch (compressedFormat) {
+ case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
+ case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT:
+ case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
+ case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT:
+ case GL_COMPRESSED_RG_RGTC2:
+ case GL_COMPRESSED_SIGNED_RG_RGTC2:
+ case GL_COMPRESSED_RGBA_BPTC_UNORM:
+ case GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM:
+ case GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT:
+ case GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT:
+ compressedClass = BLOCK_CLASS_128_BITS;
+ break;
+ case GL_COMPRESSED_RGB_S3TC_DXT1_EXT:
+ case GL_COMPRESSED_SRGB_S3TC_DXT1_EXT:
+ case GL_COMPRESSED_RGBA_S3TC_DXT1_EXT:
+ case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT:
+ case GL_COMPRESSED_RED_RGTC1:
+ case GL_COMPRESSED_SIGNED_RED_RGTC1:
+ compressedClass = BLOCK_CLASS_64_BITS;
+ break;
+ default:
+ return false;
+ }
+
+ switch (otherFormat) {
+ case GL_RGBA32UI:
+ case GL_RGBA32I:
+ case GL_RGBA32F:
+ otherClass = BLOCK_CLASS_128_BITS;
+ break;
+ case GL_RGBA16F:
+ case GL_RG32F:
+ case GL_RGBA16UI:
+ case GL_RG32UI:
+ case GL_RGBA16I:
+ case GL_RG32I:
+ case GL_RGBA16:
+ case GL_RGBA16_SNORM:
+ otherClass = BLOCK_CLASS_64_BITS;
+ break;
+ default:
+ return false;
+ }
+
+ return compressedClass == otherClass;
+}
+
+static bool
+copy_format_compatible(struct gl_context *ctx,
+GLenum srcFormat, GLenum dstFormat)
+{
+ if (srcFormat == dstFormat) {
+ return true;
+ }
+ else if (_mesa_is_compressed_format(ctx, srcFormat)) {
+ return compressed_format_compatible(ctx, srcFormat, dstFormat);
+ }
+ else if (_mesa_is_compressed_format(ctx, dstFormat)) {
+ return compressed_format_compatible(ctx, dstFormat, srcFormat);
+ }
+ else {
+ return _mesa_texture_view_compatible_format(ctx,
+ srcFormat,
+ dstFormat);
+ }
+}
+
void GLAPIENTRY
_mesa_CopyImageSubData(GLuint srcName, GLenum srcTarget, GLint srcLevel,
GLint srcX, GLint srcY, GLint srcZ,
@@ -265,7 +350,7 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum
srcTarget, GLint srcLevel,
struct gl_texture_object *srcTexObj, *dstTexObj;
struct gl_texture_image *srcTexImage, *dstTexImage;
GLuint src_bw, src_bh, dst_bw, dst_bh;
- int i, srcNewZ, dstNewZ, Bpt;
+ int i, srcNewZ, dstNewZ;
if (MESA_VERBOSE & VERBOSE_API)
_mesa_debug(ctx, "glCopyImageSubData(%u, %s, %d, %d, %d, %d, "
@@ -306,15 +391,6 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum
srcTarget, GLint srcLevel,
goto cleanup;
}
- /* Very simple sanity check. This is sufficient if one of the textures
-* is compressed. */
- Bpt = _mesa_get_format_bytes(srcTexImage->TexFormat);
- if (_mesa_get_format_bytes(dstTexImage->TexFormat) != Bpt) {
- _mesa_error(ctx, GL_INVALID_VALUE,
- "glCopyImageSubData(internalFormat mismatch)");
- goto cleanup;
- }
-
if (!check_region_bounds(ctx, srcTexImage, srcX, srcY, srcZ,
srcWidth, srcHeight, srcDepth, "src"))
goto cleanup;
@@ -324,17 +400,11 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum
srcTarget, GLint srcLevel,
(srcHeight / src_bh) * dst_bh, srcDepth, "dst"))
goto cleanup;
- if (_mesa_
Re: [Mesa-dev] [PATCH v2] mesa: fix ARB_copy_image internal format check
On Mar 7, 2015 10:57 AM, "Sean Burke" wrote:
>
> The memory layout of compatible internal formats may differ in bytes per
> block, so TexFormat is not a reliable measure of compatibility.
Additionally,
> the current check allows compressed textures of the same block size to be
used,
> which is in violation of the spec.
Could you please be more specific about exactly when the old code fails.
> v2: Use a switch instead of array iteration for block class and show the
> correct GL error when internal formats are mismatched.
> ---
> src/mesa/main/copyimage.c | 112
+-
> 1 file changed, 91 insertions(+), 21 deletions(-)
>
> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
> index 455929d..eaedaa6 100644
> --- a/src/mesa/main/copyimage.c
> +++ b/src/mesa/main/copyimage.c
> @@ -33,6 +33,12 @@
> #include "texobj.h"
> #include "fbobject.h"
> #include "textureview.h"
> +#include "glformats.h"
> +
> +enum mesa_block_class {
> + BLOCK_CLASS_128_BITS,
> + BLOCK_CLASS_64_BITS
> +};
>
> static bool
> prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int
level,
> @@ -253,6 +259,85 @@ check_region_bounds(struct gl_context *ctx,
> struct gl_texture_image *tex_image,
> return true;
> }
>
> +static bool
> +compressed_format_compatible(struct gl_context *ctx,
> + GLenum compressedFormat, GLenum otherFormat)
> +{
> + enum mesa_block_class compressedClass, otherClass;
> +
> + /* Two different compressed formats are never compatible. */
> + if (_mesa_is_compressed_format(ctx, otherFormat)) {
> + return false;
> + }
> +
> + switch (compressedFormat) {
> + case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
> + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT:
> + case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
> + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT:
> + case GL_COMPRESSED_RG_RGTC2:
> + case GL_COMPRESSED_SIGNED_RG_RGTC2:
> + case GL_COMPRESSED_RGBA_BPTC_UNORM:
> + case GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM:
> + case GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT:
> + case GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT:
> + compressedClass = BLOCK_CLASS_128_BITS;
> + break;
> + case GL_COMPRESSED_RGB_S3TC_DXT1_EXT:
> + case GL_COMPRESSED_SRGB_S3TC_DXT1_EXT:
> + case GL_COMPRESSED_RGBA_S3TC_DXT1_EXT:
> + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT:
> + case GL_COMPRESSED_RED_RGTC1:
> + case GL_COMPRESSED_SIGNED_RED_RGTC1:
> + compressedClass = BLOCK_CLASS_64_BITS;
> + break;
> + default:
> + return false;
> + }
> +
> + switch (otherFormat) {
> + case GL_RGBA32UI:
> + case GL_RGBA32I:
> + case GL_RGBA32F:
> + otherClass = BLOCK_CLASS_128_BITS;
> + break;
> + case GL_RGBA16F:
> + case GL_RG32F:
> + case GL_RGBA16UI:
> + case GL_RG32UI:
> + case GL_RGBA16I:
> + case GL_RG32I:
> + case GL_RGBA16:
> + case GL_RGBA16_SNORM:
> + otherClass = BLOCK_CLASS_64_BITS;
> + break;
> + default:
> + return false;
> + }
> +
> + return compressedClass == otherClass;
> +}
For compressed formats this should be the same as doing a bits-per-block
check. Why do we need all this?
> +static bool
> +copy_format_compatible(struct gl_context *ctx,
> +GLenum srcFormat, GLenum dstFormat)
> +{
> + if (srcFormat == dstFormat) {
> + return true;
> + }
> + else if (_mesa_is_compressed_format(ctx, srcFormat)) {
> + return compressed_format_compatible(ctx, srcFormat, dstFormat);
> + }
> + else if (_mesa_is_compressed_format(ctx, dstFormat)) {
> + return compressed_format_compatible(ctx, dstFormat, srcFormat);
> + }
> + else {
> + return _mesa_texture_view_compatible_format(ctx,
> + srcFormat,
> + dstFormat);
> + }
> +}
> +
> void GLAPIENTRY
> _mesa_CopyImageSubData(GLuint srcName, GLenum srcTarget, GLint srcLevel,
> GLint srcX, GLint srcY, GLint srcZ,
> @@ -265,7 +350,7 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum
> srcTarget, GLint srcLevel,
> struct gl_texture_object *srcTexObj, *dstTexObj;
> struct gl_texture_image *srcTexImage, *dstTexImage;
> GLuint src_bw, src_bh, dst_bw, dst_bh;
> - int i, srcNewZ, dstNewZ, Bpt;
> + int i, srcNewZ, dstNewZ;
>
> if (MESA_VERBOSE & VERBOSE_API)
>_mesa_debug(ctx, "glCopyImageSubData(%u, %s, %d, %d, %d, %d, "
> @@ -306,15 +391,6 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum
> srcTarget, GLint srcLevel,
>goto cleanup;
> }
>
> - /* Very simple sanity check. This is sufficient if one of the
textures
> -* is compressed. */
> - Bpt = _mesa_get_format_bytes(srcTexImage->TexFormat);
> - if (_mesa_get_format_bytes(dstTexImage->TexFormat) != Bpt) {
> - _mesa
Re: [Mesa-dev] [PATCH 1/6] c11: add c11 compatibility wrapper around stdlib.h
On 07/03/15 07:23, Jose Fonseca wrote: ... > we still > didn't eliminate the use of non-portable _MTX_INITIALIZER_NP from Mesa > tree gave me pause. > The only way I can think about resolving this, is to use call_once() to initialize the mutex, but I'm not 100% sure if T2 will sync until T1 call_once's func has returned. How does it sound ? ... > We can can consider move the c99_foo.h/c11_foo.h them somewhere else > (another subdirectory, or util) or renaming them (like u_foo.h). I have no objection on moving the file, but please keep the file name in some form that makes it obvious about the spec compat/wrapping it provides. FYI I'm contemplating on about adding a final wrapper - c99_string.h. It should nuke nearly all of the remaining compiler abstraction that we have around - mapi, egl, gallium, mesa, glsl... -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/6] c11: add c11 compatibility wrapper around stdlib.h
On Sat, Mar 7, 2015 at 12:38 PM, Emil Velikov wrote: > > FYI I'm contemplating on about adding a final wrapper - c99_string.h. It > should nuke nearly all of the remaining compiler abstraction that we > have around - mapi, egl, gallium, mesa, glsl... > Yeah, I was looking at doing something like that for stroll() and strcasecmp(). I already have a patch series which removes _mesa_strdup() and another that moves fpclassify() to c99_math.h I'm on the road ATM and can't test the Windows build but I guess I could post my patches now. I can test on Windows in a few days. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] mesa: fix ARB_copy_image internal format check
On Sat, Mar 7, 2015 at 12:06 PM, Jason Ekstrand
wrote:
>
> On Mar 7, 2015 10:57 AM, "Sean Burke" wrote:
> >
> > The memory layout of compatible internal formats may differ in bytes per
> > block, so TexFormat is not a reliable measure of compatibility.
> Additionally,
> > the current check allows compressed textures of the same block size to
> be used,
> > which is in violation of the spec.
>
> Could you please be more specific about exactly when the old code fails.
>
One example I was finding in running piglit tests was that GL_INVALID_VALUE
was being set when the formats to be converted were GL_RGB8 and GL_RGB8UI.
TexFormat refers specifically to the actual memory layout which mesa has
chosen for the texture, not to the internal format specified by the user,
and there's no guarantee that the bytes-per-block of the memory layout
matches the internal format. GL_RGB8 was being backed by B8G8R8X8, even
though its internal format is still 3 bytes.
Additionally, the previous code indicated that textures with two different
compressed formats were compatible if block size was identical. According
to the spec, this is incorrect. Different texture formats are only
compatible if they're compatible texture view-wise or if one is compressed
and the other is uncompressed and they are listed in the given table.
In writing this out, I've discovered that I do have a bug in
copy_format_compatible() (view compatibility should be checked second,
immediately after checking if the formats are equal, to account for
view-compatible compressed formats), which I'll correct with a v3.
> > v2: Use a switch instead of array iteration for block class and show the
> > correct GL error when internal formats are mismatched.
> > ---
> > src/mesa/main/copyimage.c | 112
> +-
> > 1 file changed, 91 insertions(+), 21 deletions(-)
> >
> > diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
> > index 455929d..eaedaa6 100644
> > --- a/src/mesa/main/copyimage.c
> > +++ b/src/mesa/main/copyimage.c
> > @@ -33,6 +33,12 @@
> > #include "texobj.h"
> > #include "fbobject.h"
> > #include "textureview.h"
> > +#include "glformats.h"
> > +
> > +enum mesa_block_class {
> > + BLOCK_CLASS_128_BITS,
> > + BLOCK_CLASS_64_BITS
> > +};
> >
> > static bool
> > prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int
> level,
> > @@ -253,6 +259,85 @@ check_region_bounds(struct gl_context *ctx,
> > struct gl_texture_image *tex_image,
> > return true;
> > }
> >
> > +static bool
> > +compressed_format_compatible(struct gl_context *ctx,
> > + GLenum compressedFormat, GLenum
> otherFormat)
> > +{
> > + enum mesa_block_class compressedClass, otherClass;
> > +
> > + /* Two different compressed formats are never compatible. */
> > + if (_mesa_is_compressed_format(ctx, otherFormat)) {
> > + return false;
> > + }
> > +
> > + switch (compressedFormat) {
> > + case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
> > + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT:
> > + case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
> > + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT:
> > + case GL_COMPRESSED_RG_RGTC2:
> > + case GL_COMPRESSED_SIGNED_RG_RGTC2:
> > + case GL_COMPRESSED_RGBA_BPTC_UNORM:
> > + case GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM:
> > + case GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT:
> > + case GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT:
> > + compressedClass = BLOCK_CLASS_128_BITS;
> > + break;
> > + case GL_COMPRESSED_RGB_S3TC_DXT1_EXT:
> > + case GL_COMPRESSED_SRGB_S3TC_DXT1_EXT:
> > + case GL_COMPRESSED_RGBA_S3TC_DXT1_EXT:
> > + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT:
> > + case GL_COMPRESSED_RED_RGTC1:
> > + case GL_COMPRESSED_SIGNED_RED_RGTC1:
> > + compressedClass = BLOCK_CLASS_64_BITS;
> > + break;
> > + default:
> > + return false;
> > + }
> > +
> > + switch (otherFormat) {
> > + case GL_RGBA32UI:
> > + case GL_RGBA32I:
> > + case GL_RGBA32F:
> > + otherClass = BLOCK_CLASS_128_BITS;
> > + break;
> > + case GL_RGBA16F:
> > + case GL_RG32F:
> > + case GL_RGBA16UI:
> > + case GL_RG32UI:
> > + case GL_RGBA16I:
> > + case GL_RG32I:
> > + case GL_RGBA16:
> > + case GL_RGBA16_SNORM:
> > + otherClass = BLOCK_CLASS_64_BITS;
> > + break;
> > + default:
> > + return false;
> > + }
> > +
> > + return compressedClass == otherClass;
> > +}
>
> For compressed formats this should be the same as doing a bits-per-block
> check. Why do we need all this?
>
Given that the spec gives an exhaustive list of all compatible
compressed/uncompressed formats, rather than indicating that it's valid for
any compressed format of a given block size, I feel it's best to be
specific. A similar approach is taken for texture view compatibility, so it
makes sense t
Re: [Mesa-dev] [PATCH 1/6] c11: add c11 compatibility wrapper around stdlib.h
On Sat, Mar 7, 2015 at 1:13 PM, Brian Paul wrote:
> On Sat, Mar 7, 2015 at 12:38 PM, Emil Velikov
> wrote:
>
>
>> FYI I'm contemplating on about adding a final wrapper - c99_string.h. It
>> should nuke nearly all of the remaining compiler abstraction that we
>> have around - mapi, egl, gallium, mesa, glsl...
>>
>
> Yeah, I was looking at doing something like that for stroll() and
> strcasecmp(). I already have a patch series which removes _mesa_strdup()
> and another that moves fpclassify() to c99_math.h
>
> I'm on the road ATM and can't test the Windows build but I guess I could
> post my patches now. I can test on Windows in a few days.
>
Hmm, git send-email isn't cooperating with my gmail account right now. I'm
just attaching the patches for now.
-Brian
From 90cc53e67e076fe7a38ae7ac3943868258c820ae Mon Sep 17 00:00:00 2001
From: Brian Paul
Date: Sat, 7 Mar 2015 13:15:22 -0700
Subject: [PATCH 1/7] mesa: move fpclassify work-arounds into c99_math.h
---
include/c99_math.h | 52 +
src/mesa/main/querymatrix.c | 51 +---
2 files changed, 53 insertions(+), 50 deletions(-)
diff --git a/include/c99_math.h b/include/c99_math.h
index 0a49950..f1a6685 100644
--- a/include/c99_math.h
+++ b/include/c99_math.h
@@ -161,4 +161,56 @@ llrintf(float f)
#endif
+#if defined(fpclassify)
+/* ISO C99 says that fpclassify is a macro. Assume that any implementation
+ * of fpclassify, whether it's in a C99 compiler or not, will be a macro.
+ */
+#elif defined(__cplusplus)
+/* For C++, fpclassify() should be defined in */
+#elif defined(_MSC_VER)
+/* Not required on VS2013 and above. Oddly, the fpclassify() function
+ * doesn't exist in such a form on MSVC. This is an implementation using
+ * slightly different lower-level Windows functions.
+ */
+#include
+
+static inline enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL, FP_NORMAL}
+fpclassify(double x)
+{
+ switch(_fpclass(x)) {
+ case _FPCLASS_SNAN: /* signaling NaN */
+ case _FPCLASS_QNAN: /* quiet NaN */
+ return FP_NAN;
+ case _FPCLASS_NINF: /* negative infinity */
+ case _FPCLASS_PINF: /* positive infinity */
+ return FP_INFINITE;
+ case _FPCLASS_NN: /* negative normal */
+ case _FPCLASS_PN: /* positive normal */
+ return FP_NORMAL;
+ case _FPCLASS_ND: /* negative denormalized */
+ case _FPCLASS_PD: /* positive denormalized */
+ return FP_SUBNORMAL;
+ case _FPCLASS_NZ: /* negative zero */
+ case _FPCLASS_PZ: /* positive zero */
+ return FP_ZERO;
+ default:
+ /* Should never get here; but if we do, this will guarantee
+ * that the pattern is not treated like a number.
+ */
+ return FP_NAN;
+ }
+}
+
+#else
+
+static inline enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL, FP_NORMAL}
+fpclassify(double x)
+{
+ /* XXX do something better someday */
+ return FP_NORMAL;
+}
+
+#endif
+
+
#endif /* #define _C99_MATH_H_ */
diff --git a/src/mesa/main/querymatrix.c b/src/mesa/main/querymatrix.c
index ef85175..095817c 100644
--- a/src/mesa/main/querymatrix.c
+++ b/src/mesa/main/querymatrix.c
@@ -13,7 +13,7 @@
#include
-#include
+#include "c99_math.h"
#include "glheader.h"
#include "querymatrix.h"
#include "main/get.h"
@@ -37,55 +37,6 @@
#define INT_TO_FIXED(x) ((GLfixed) ((x) << 16))
#define FLOAT_TO_FIXED(x) ((GLfixed) ((x) * 65536.0))
-#if defined(fpclassify)
-/* ISO C99 says that fpclassify is a macro. Assume that any implementation
- * of fpclassify, whether it's in a C99 compiler or not, will be a macro.
- */
-#elif defined(_MSC_VER)
-/* Not required on VS2013 and above. */
-/* Oddly, the fpclassify() function doesn't exist in such a form
- * on MSVC. This is an implementation using slightly different
- * lower-level Windows functions.
- */
-#include
-
-enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL, FP_NORMAL}
-fpclassify(double x)
-{
-switch(_fpclass(x)) {
-case _FPCLASS_SNAN: /* signaling NaN */
-case _FPCLASS_QNAN: /* quiet NaN */
-return FP_NAN;
-case _FPCLASS_NINF: /* negative infinity */
-case _FPCLASS_PINF: /* positive infinity */
-return FP_INFINITE;
-case _FPCLASS_NN: /* negative normal */
-case _FPCLASS_PN: /* positive normal */
-return FP_NORMAL;
-case _FPCLASS_ND: /* negative denormalized */
-case _FPCLASS_PD: /* positive denormalized */
-return FP_SUBNORMAL;
-case _FPCLASS_NZ: /* negative zero */
-case _FPCLASS_PZ: /* positive zero */
-return FP_ZERO;
-default:
-/* Should never get here; but if we do, this will guarantee
- * that the pattern is not treated like a number.
- */
-return FP_NAN;
-}
-}
-
-#else
-
-enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL, FP_NORMAL}
-fpclassify(double x)
-{
- /* XXX do something better someday */
-
Re: [Mesa-dev] [PATCH v2] nv30: Add unused attribute to function nv40_fp_bra.
On Fri, Mar 6, 2015 at 11:43 PM, Vinson Lee wrote: > Silences GCC unused-function warning. > > nv30/nvfx_fragprog.c:333:1: warning: ‘nv40_fp_bra’ defined but not used > [-Wunused-function] > nv40_fp_bra(struct nvfx_fpc *fpc, unsigned target) > ^ > > Signed-off-by: Vinson Lee > --- > src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c > b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c > index 6600997..abd51c8 100644 > --- a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c > +++ b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c > @@ -329,7 +329,7 @@ nv40_fp_rep(struct nvfx_fpc *fpc, unsigned count, > unsigned target) > } > > /* warning: this only works forward, and probably only if not inside any IF > */ > -static void > +static __attribute__((unused)) void Doing this means gcc can't actually eliminate the function. I think your only option, if we want to keep the code but still let gcc eliminate the function, is to #if 0 it/comment it out. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] nv30: Add unused attribute to function nv40_fp_bra.
Matt Turner writes: > On Fri, Mar 6, 2015 at 11:43 PM, Vinson Lee wrote: >> Silences GCC unused-function warning. >> >> nv30/nvfx_fragprog.c:333:1: warning: ‘nv40_fp_bra’ defined but not used >> [-Wunused-function] >> nv40_fp_bra(struct nvfx_fpc *fpc, unsigned target) >> ^ >> >> Signed-off-by: Vinson Lee >> --- >> src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c >> b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c >> index 6600997..abd51c8 100644 >> --- a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c >> +++ b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c >> @@ -329,7 +329,7 @@ nv40_fp_rep(struct nvfx_fpc *fpc, unsigned count, >> unsigned target) >> } >> >> /* warning: this only works forward, and probably only if not inside any IF >> */ >> -static void >> +static __attribute__((unused)) void > > Doing this means gcc can't actually eliminate the function. > > I think your only option, if we want to keep the code but still let > gcc eliminate the function, is to #if 0 it/comment it out. I don't think that's right, __attribute__((unused)) doesn't prevent gcc From eliminating a function, maybe you're confusing it with __attribute__((used))? > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] nv30: Add unused attribute to function nv40_fp_bra.
On Sat, Mar 7, 2015 at 12:54 PM, Francisco Jerez wrote: > Matt Turner writes: > >> On Fri, Mar 6, 2015 at 11:43 PM, Vinson Lee wrote: >>> Silences GCC unused-function warning. >>> >>> nv30/nvfx_fragprog.c:333:1: warning: ‘nv40_fp_bra’ defined but not used >>> [-Wunused-function] >>> nv40_fp_bra(struct nvfx_fpc *fpc, unsigned target) >>> ^ >>> >>> Signed-off-by: Vinson Lee >>> --- >>> src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c >>> b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c >>> index 6600997..abd51c8 100644 >>> --- a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c >>> +++ b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c >>> @@ -329,7 +329,7 @@ nv40_fp_rep(struct nvfx_fpc *fpc, unsigned count, >>> unsigned target) >>> } >>> >>> /* warning: this only works forward, and probably only if not inside any >>> IF */ >>> -static void >>> +static __attribute__((unused)) void >> >> Doing this means gcc can't actually eliminate the function. >> >> I think your only option, if we want to keep the code but still let >> gcc eliminate the function, is to #if 0 it/comment it out. > > I don't think that's right, __attribute__((unused)) doesn't prevent gcc > From eliminating a function, maybe you're confusing it with > __attribute__((used))? I don't know. This commit uses ((unused)) as well, with apparently the intention of keeping the function around. commit 27b6ef7ecaa60ce192ec74eef2245c25ed4e703b Author: Kenneth Graunke Date: Fri Feb 6 00:36:26 2015 -0800 i965: Add a function to disassemble an instruction from the 4 dwords. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] nv30: Add unused attribute to function nv40_fp_bra.
Matt Turner writes: > On Sat, Mar 7, 2015 at 12:54 PM, Francisco Jerez > wrote: >> Matt Turner writes: >> >>> On Fri, Mar 6, 2015 at 11:43 PM, Vinson Lee wrote: Silences GCC unused-function warning. nv30/nvfx_fragprog.c:333:1: warning: ‘nv40_fp_bra’ defined but not used [-Wunused-function] nv40_fp_bra(struct nvfx_fpc *fpc, unsigned target) ^ Signed-off-by: Vinson Lee --- src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c index 6600997..abd51c8 100644 --- a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c +++ b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c @@ -329,7 +329,7 @@ nv40_fp_rep(struct nvfx_fpc *fpc, unsigned count, unsigned target) } /* warning: this only works forward, and probably only if not inside any IF */ -static void +static __attribute__((unused)) void >>> >>> Doing this means gcc can't actually eliminate the function. >>> >>> I think your only option, if we want to keep the code but still let >>> gcc eliminate the function, is to #if 0 it/comment it out. >> >> I don't think that's right, __attribute__((unused)) doesn't prevent gcc >> From eliminating a function, maybe you're confusing it with >> __attribute__((used))? > > I don't know. > > This commit uses ((unused)) as well, with apparently the intention of > keeping the function around. > > commit 27b6ef7ecaa60ce192ec74eef2245c25ed4e703b > Author: Kenneth Graunke > Date: Fri Feb 6 00:36:26 2015 -0800 > > i965: Add a function to disassemble an instruction from the 4 dwords. I don't think that guarantees that gcc will keep the function around, From [1]: | unused |This attribute, attached to a function, means that the function is |meant to be possibly unused. GCC does not produce a warning for |this function. So AFAICT it's only meant to suppress the warning. OTOH: | used |This attribute, attached to a function, means that code must be |emitted for the function even if it appears that the function is |not referenced. This is useful, for example, when the function is |referenced only in inline assembly. [1] https://gcc.gnu.org/onlinedocs/gcc-4.9.2/gcc/Function-Attributes.html#Function-Attributes signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] mesa: fix ARB_copy_image internal format check
On Sat, Mar 7, 2015 at 12:16 PM, Sean Burke wrote:
>
>
> On Sat, Mar 7, 2015 at 12:06 PM, Jason Ekstrand
> wrote:
>
>>
>> On Mar 7, 2015 10:57 AM, "Sean Burke" wrote:
>> >
>> > The memory layout of compatible internal formats may differ in bytes per
>> > block, so TexFormat is not a reliable measure of compatibility.
>> Additionally,
>> > the current check allows compressed textures of the same block size to
>> be used,
>> > which is in violation of the spec.
>>
>> Could you please be more specific about exactly when the old code fails.
>>
> One example I was finding in running piglit tests was that
> GL_INVALID_VALUE was being set when the formats to be converted were
> GL_RGB8 and GL_RGB8UI. TexFormat refers specifically to the actual memory
> layout which mesa has chosen for the texture, not to the internal format
> specified by the user, and there's no guarantee that the bytes-per-block of
> the memory layout matches the internal format. GL_RGB8 was being backed by
> B8G8R8X8, even though its internal format is still 3 bytes.
>
> Additionally, the previous code indicated that textures with two different
> compressed formats were compatible if block size was identical. According
> to the spec, this is incorrect. Different texture formats are only
> compatible if they're compatible texture view-wise or if one is compressed
> and the other is uncompressed and they are listed in the given table.
>
That's what I figured. Please put the above two paragraphs (or their
equivalent) in the commit message.
> In writing this out, I've discovered that I do have a bug in
> copy_format_compatible() (view compatibility should be checked second,
> immediately after checking if the formats are equal, to account for
> view-compatible compressed formats), which I'll correct with a v3.
>
Yeah, that will follow the spec better. I don't know that it is actually
going to have a different result but it doesn't hurt to follow the spec
more closely. It would also be good to put a spec citation in the
copy_format_compatible() function.
> > v2: Use a switch instead of array iteration for block class and show the
>>
>> > correct GL error when internal formats are mismatched.
>> > ---
>> > src/mesa/main/copyimage.c | 112
>> +-
>> > 1 file changed, 91 insertions(+), 21 deletions(-)
>> >
>> > diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
>> > index 455929d..eaedaa6 100644
>> > --- a/src/mesa/main/copyimage.c
>> > +++ b/src/mesa/main/copyimage.c
>> > @@ -33,6 +33,12 @@
>> > #include "texobj.h"
>> > #include "fbobject.h"
>> > #include "textureview.h"
>> > +#include "glformats.h"
>> > +
>> > +enum mesa_block_class {
>> > + BLOCK_CLASS_128_BITS,
>> > + BLOCK_CLASS_64_BITS
>> > +};
>> >
>> > static bool
>> > prepare_target(struct gl_context *ctx, GLuint name, GLenum *target,
>> int level,
>> > @@ -253,6 +259,85 @@ check_region_bounds(struct gl_context *ctx,
>> > struct gl_texture_image *tex_image,
>> > return true;
>> > }
>> >
>> > +static bool
>> > +compressed_format_compatible(struct gl_context *ctx,
>> > + GLenum compressedFormat, GLenum
>> otherFormat)
>> > +{
>> > + enum mesa_block_class compressedClass, otherClass;
>> > +
>> > + /* Two different compressed formats are never compatible. */
>> > + if (_mesa_is_compressed_format(ctx, otherFormat)) {
>> > + return false;
>> > + }
>> > +
>> > + switch (compressedFormat) {
>> > + case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
>> > + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT:
>> > + case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
>> > + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT:
>> > + case GL_COMPRESSED_RG_RGTC2:
>> > + case GL_COMPRESSED_SIGNED_RG_RGTC2:
>> > + case GL_COMPRESSED_RGBA_BPTC_UNORM:
>> > + case GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM:
>> > + case GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT:
>> > + case GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT:
>> > + compressedClass = BLOCK_CLASS_128_BITS;
>> > + break;
>> > + case GL_COMPRESSED_RGB_S3TC_DXT1_EXT:
>> > + case GL_COMPRESSED_SRGB_S3TC_DXT1_EXT:
>> > + case GL_COMPRESSED_RGBA_S3TC_DXT1_EXT:
>> > + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT:
>> > + case GL_COMPRESSED_RED_RGTC1:
>> > + case GL_COMPRESSED_SIGNED_RED_RGTC1:
>> > + compressedClass = BLOCK_CLASS_64_BITS;
>> > + break;
>> > + default:
>> > + return false;
>> > + }
>> > +
>> > + switch (otherFormat) {
>> > + case GL_RGBA32UI:
>> > + case GL_RGBA32I:
>> > + case GL_RGBA32F:
>> > + otherClass = BLOCK_CLASS_128_BITS;
>> > + break;
>> > + case GL_RGBA16F:
>> > + case GL_RG32F:
>> > + case GL_RGBA16UI:
>> > + case GL_RG32UI:
>> > + case GL_RGBA16I:
>> > + case GL_RG32I:
>> > + case GL_RGBA16:
>> > + case GL_RGBA16_SNORM:
>> > + otherClass = BLOCK_CLASS_64_BIT
Re: [Mesa-dev] [PATCH] i965/fs: Implement SIMD16 dual source blending.
On Thu, Mar 5, 2015 at 9:39 PM, Jason Ekstrand wrote:
> This looks fine to me. I just kicked off a build on our test farm and,
> assuming that looks good (I'll send another e-mail in the morning if it
> does),
>
> Reviewed-by: Jason Ekstrand
>
Jenkins results look god so feel free to apply the R-B above and push it.
Don't worry about the shader-db number given that, as ken pointed out,
shader-db is kind of useless for this. I wish we knew how many SIMD16
programs this gave us in practice, but short of doing lots of shader-db
work, we can't know at the moment so don't worry about it.
--Jason
>
> I ran shader-db on the change and I was kind of surprised to see that it
> doesn't really do anything.
>
> GAINED: shaders/dolphin/smg.1.shader_test FS SIMD16
>
> total instructions in shared programs: 5769629 -> 5769629 (0.00%)
> instructions in affected programs: 0 -> 0
> helped:0
> HURT: 0
> GAINED:1
> LOST: 0
>
> Perhaps shader-db doesn't account for some other GL state required for
> dual-source because I doubt only one shader uses it. Ken?
>
> --Jason
>
> On Thu, Mar 5, 2015 at 3:21 AM, Iago Toral Quiroga
> wrote:
>
>> From the SNB PRM, volume 4, part 1, page 193:
>>
>> "The dual source render target messages only have SIMD8 forms due to
>> maximum message length limitations. SIMD16 pixel shaders must send two of
>> these messages to cover all of the pixels. Each message contains two
>> colors
>> (4 channels each) for each pixel in the message payload."
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82831
>> ---
>> I sent this patch for review some months ago, but it was bad timing
>> because
>> it was when Jason was doing a large rewrite of the visitor code handling
>> FB writes, so the patch became immediately obsolete. This is the
>> up-to-date
>> version.
>>
>> If anyone wants to test this, I sent this patch to piglit with a test that
>> can be used to check for correct SIMD16 implementation specifically:
>> http://lists.freedesktop.org/archives/piglit/2015-March/015015.html
>>
>> src/mesa/drivers/dri/i965/brw_eu.h | 1 +
>> src/mesa/drivers/dri/i965/brw_eu_emit.c| 3 +-
>> src/mesa/drivers/dri/i965/brw_fs.h | 6 +-
>> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 15 -
>> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 77
>> +-
>> 5 files changed, 83 insertions(+), 19 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
>> b/src/mesa/drivers/dri/i965/brw_eu.h
>> index 736c54b..d9ad5bd 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu.h
>> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
>> @@ -266,6 +266,7 @@ void brw_fb_WRITE(struct brw_compile *p,
>>unsigned msg_length,
>>unsigned response_length,
>>bool eot,
>> + bool last_render_target,
>>bool header_present);
>>
>> void brw_SAMPLE(struct brw_compile *p,
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> index 1d6fd67..74cf138 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> @@ -2292,6 +2292,7 @@ void brw_fb_WRITE(struct brw_compile *p,
>>unsigned msg_length,
>>unsigned response_length,
>>bool eot,
>> + bool last_render_target,
>>bool header_present)
>> {
>> struct brw_context *brw = p->brw;
>> @@ -2333,7 +2334,7 @@ void brw_fb_WRITE(struct brw_compile *p,
>> msg_type,
>> msg_length,
>> header_present,
>> - eot, /* last render target write */
>> + last_render_target,
>> response_length,
>> eot,
>> 0 /* send_commit_msg */);
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
>> b/src/mesa/drivers/dri/i965/brw_fs.h
>> index 70098d8..5a4f66c 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> @@ -369,10 +369,12 @@ public:
>> bool optimize_frontfacing_ternary(nir_alu_instr *instr,
>> const fs_reg &result);
>>
>> - int setup_color_payload(fs_reg *dst, fs_reg color, unsigned
>> components);
>> + int setup_color_payload(fs_reg *dst, fs_reg color, unsigned
>> components,
>> + bool use_2nd_half);
>> void emit_alpha_test();
>> fs_inst *emit_single_fb_write(fs_reg color1, fs_reg color2,
>> - fs_reg src0_alpha, unsigned components);
>> + fs_reg src0_alpha, unsigned components,
>> + bool use_2n
Re: [Mesa-dev] [Mesa-stable] [PATCH 4/4] i965/fs: Don't issue FB writes for bound but unwritten color targets.
On Sat, Mar 7, 2015 at 9:58 AM, Emil Velikov wrote: > On 27 February 2015 at 08:06, Kenneth Graunke > wrote: > > We used to loop over all color attachments, and emit FB writes for each > > one, even if the shader didn't write to a corresponding output variable. > > Those color attachments would be filled with garbage (undefined values). > > > > Football Manager binds a framebuffer with 4 color attachments, but draws > > to it using a shader that only writes to gl_FragData[0..2]. This meant > > that color attachment 3 would be filled with garbage, resulting in > > rendering artifacts. Now we skip writing to it, fixing rendering. > > > > Writes to gl_FragColor initialize outputs[0..nr_color_regions-1] to > > GRFs, while writes to gl_FragData[i] initialize outputs[i]. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86747 > > Signed-off-by: Kenneth Graunke > > Cc: mesa-sta...@lists.freedesktop.org > Hi Ken, > > This commit does not seems to have not landed in master. Did it fall > through the cracks ? > I think it did. This series is definitely somethiing we would like to be in 10.5 --Jason > > Cheers, > Emil > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] Add macro for unused function attribute.
Suggested-by: Emil Velikov
Signed-off-by: Vinson Lee
---
configure.ac | 1 +
scons/gallium.py | 1 +
src/util/macros.h | 6 ++
3 files changed, 8 insertions(+)
diff --git a/configure.ac b/configure.ac
index 90c7737..2954f80 100644
--- a/configure.ac
+++ b/configure.ac
@@ -195,6 +195,7 @@ AX_GCC_FUNC_ATTRIBUTE([flatten])
AX_GCC_FUNC_ATTRIBUTE([format])
AX_GCC_FUNC_ATTRIBUTE([malloc])
AX_GCC_FUNC_ATTRIBUTE([packed])
+AX_GCC_FUNC_ATTRIBUTE([unused])
AM_CONDITIONAL([GEN_ASM_OFFSETS], test "x$GEN_ASM_OFFSETS" = xyes)
diff --git a/scons/gallium.py b/scons/gallium.py
index 7533f06..b162089 100755
--- a/scons/gallium.py
+++ b/scons/gallium.py
@@ -369,6 +369,7 @@ def generate(env):
'HAVE___BUILTIN_FFS',
'HAVE___BUILTIN_FFSLL',
'HAVE_FUNC_ATTRIBUTE_FLATTEN',
+'HAVE_FUNC_ATTRIBUTE_UNUSED',
# GCC 3.0
'HAVE_FUNC_ATTRIBUTE_FORMAT',
'HAVE_FUNC_ATTRIBUTE_PACKED',
diff --git a/src/util/macros.h b/src/util/macros.h
index 63daba3..6c7bda7 100644
--- a/src/util/macros.h
+++ b/src/util/macros.h
@@ -176,5 +176,11 @@ do { \
# endif
#endif
+#ifdef HAVE_FUNC_ATTRIBUTE_UNUSED
+#define UNUSED __attribute__((unused))
+#else
+#define UNUSED
+#endif
+
#endif /* UTIL_MACROS_H */
--
2.3.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3] mesa: improve ARB_copy_image internal format compat check
The memory layout of compatible internal formats may differ in bytes per
block, so TexFormat is not a reliable measure of compatibility. For example,
GL_RGB8 and GL_RGB8UI are compatible formats, but GL_RGB8 may be laid out in
memory as B8G8R8X8. If GL_RGB8UI has a 3 byte-per-block memory layout, the
existing compatibility check will fail.
Additionally, the current check allows any two compressed textures which share
block size to be used, which is in violation of the spec.
v2: Use a switch instead of array iteration for block class and show the
correct GL error when internal formats are mismatched.
v3: Include spec citations for new compatibility checks, rearrange check
order to ensure that compressed, view-compatible formats return the
correct result, and make style fixes. Original commit message amended
for clarity.
---
src/mesa/main/copyimage.c | 148 +++---
1 file changed, 127 insertions(+), 21 deletions(-)
diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
index 455929d..9fc9c82 100644
--- a/src/mesa/main/copyimage.c
+++ b/src/mesa/main/copyimage.c
@@ -33,6 +33,12 @@
#include "texobj.h"
#include "fbobject.h"
#include "textureview.h"
+#include "glformats.h"
+
+enum mesa_block_class {
+ BLOCK_CLASS_128_BITS,
+ BLOCK_CLASS_64_BITS
+};
static bool
prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int level,
@@ -253,6 +259,121 @@ check_region_bounds(struct gl_context *ctx,
struct gl_texture_image *tex_image,
return true;
}
+static bool
+compressed_format_compatible(struct gl_context *ctx,
+ GLenum compressedFormat, GLenum otherFormat)
+{
+ enum mesa_block_class compressedClass, otherClass;
+
+ /* Two view-incompatible compressed formats are never compatible. */
+ if (_mesa_is_compressed_format(ctx, otherFormat)) {
+ return false;
+ }
+
+ /* Table 4.X.1 (Compatible internal formats for copying between compressed
+* and uncompressed internal formats)
+*
+* | Texel / | Uncompressed | |
+* | Block | internal format | Compressed internal format |
+* | size| | |
+*
+* | 128-bit | RGBA32UI,| COMPRESSED_RGBA_S3TC_DXT3_EXT, |
+* | | RGBA32I, | COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT,|
+* | | RGBA32F | COMPRESSED_RGBA_S3TC_DXT5_EXT, |
+* | | | COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT,|
+* | | | COMPRESSED_RG_RGTC2,|
+* | | | COMPRESSED_SIGNED_RG_RGTC2, |
+* | | | COMPRESSED_RGBA_BPTC_UNORM, |
+* | | | COMPRESSED_SRGB_ALPHA_BPTC_UNORM, |
+* | | | COMPRESSED_RGB_BPTC_SIGNED_FLOAT, |
+* | | | COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT |
+*
+* | 64-bit | RGBA16F, RG32F, | COMPRESSED_RGB_S3TC_DXT1_EXT, |
+* | | RGBA16UI, RG32UI,| COMPRESSED_SRGB_S3TC_DXT1_EXT, |
+* | | RGBA16I, RG32I, | COMPRESSED_RGBA_S3TC_DXT1_EXT, |
+* | | RGBA16, RGBA16_SNORM | COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT,|
+* | | | COMPRESSED_RED_RGTC1, |
+* | | | COMPRESSED_SIGNED_RED_RGTC1 |
+*
+*/
+
+ switch (compressedFormat) {
+ case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
+ case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT:
+ case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
+ case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT:
+ case GL_COMPRESSED_RG_RGTC2:
+ case GL_COMPRESSED_SIGNED_RG_RGTC2:
+ case GL_COMPRESSED_RGBA_BPTC_UNORM:
+ case GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM:
+ case GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT:
+ case GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT:
+ compressedClass = BLOCK_CLASS_128_BITS;
+ break;
+ case GL_COMPRESSED_RGB_S3TC_DXT1_EXT:
+ case GL_COMPRESSED_SRGB_S3TC_DXT1_EXT:
+ case GL_COMPRESSED_RGBA_S3TC_DXT1_EXT:
+ case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT:
+ case GL_COMPRESSED_RED_RGTC1:
+ case GL_COMPRESSED_SIGNED_RED_RGTC1:
+ compressedClass = BLOCK_CLASS_64_BITS;
+ break;
+ default:
+ return false;
+ }
+
+ switch (otherFormat) {
+ case GL_RGBA32UI:
+ case GL_RGBA32I:
+ case GL_RGBA32F:
+ otherClass = BLOCK_CLASS_128_BITS;
+
Re: [Mesa-dev] [PATCH v3] mesa: improve ARB_copy_image internal format compat check
On Sat, Mar 7, 2015 at 2:10 PM, Sean Burke wrote:
> The memory layout of compatible internal formats may differ in bytes per
> block, so TexFormat is not a reliable measure of compatibility. For
> example,
> GL_RGB8 and GL_RGB8UI are compatible formats, but GL_RGB8 may be laid out
> in
> memory as B8G8R8X8. If GL_RGB8UI has a 3 byte-per-block memory layout, the
> existing compatibility check will fail.
>
> Additionally, the current check allows any two compressed textures which
> share
> block size to be used, which is in violation of the spec.
>
Rather than simply saying "which is in violation of the spec", perhaps it
would be better to say "whereas the spec gives an explicit table of
compatible formats". I think the original check actually worked because
mesa doesn't support anything not in that table. However, doing it
explicitly is probably better.
>
> v2: Use a switch instead of array iteration for block class and show the
> correct GL error when internal formats are mismatched.
> v3: Include spec citations for new compatibility checks, rearrange check
> order to ensure that compressed, view-compatible formats return the
> correct result, and make style fixes. Original commit message amended
> for clarity.
> ---
> src/mesa/main/copyimage.c | 148
> +++---
> 1 file changed, 127 insertions(+), 21 deletions(-)
>
> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
> index 455929d..9fc9c82 100644
> --- a/src/mesa/main/copyimage.c
> +++ b/src/mesa/main/copyimage.c
> @@ -33,6 +33,12 @@
> #include "texobj.h"
> #include "fbobject.h"
> #include "textureview.h"
> +#include "glformats.h"
> +
> +enum mesa_block_class {
> + BLOCK_CLASS_128_BITS,
> + BLOCK_CLASS_64_BITS
> +};
>
> static bool
> prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int
> level,
> @@ -253,6 +259,121 @@ check_region_bounds(struct gl_context *ctx,
> struct gl_texture_image *tex_image,
> return true;
> }
>
> +static bool
> +compressed_format_compatible(struct gl_context *ctx,
> + GLenum compressedFormat, GLenum otherFormat)
> +{
> + enum mesa_block_class compressedClass, otherClass;
> +
> + /* Two view-incompatible compressed formats are never compatible. */
> + if (_mesa_is_compressed_format(ctx, otherFormat)) {
> + return false;
> + }
> +
> + /* Table 4.X.1 (Compatible internal formats for copying between
> compressed
> +* and uncompressed internal formats)
> +*
>
> +* | Texel / | Uncompressed |
>|
> +* | Block | internal format | Compressed internal format
> |
> +* | size| |
>|
> +*
>
> +* | 128-bit | RGBA32UI,| COMPRESSED_RGBA_S3TC_DXT3_EXT,
> |
> +* | | RGBA32I, |
> COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT,|
> +* | | RGBA32F | COMPRESSED_RGBA_S3TC_DXT5_EXT,
> |
> +* | | |
> COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT,|
> +* | | | COMPRESSED_RG_RGTC2,
> |
> +* | | | COMPRESSED_SIGNED_RG_RGTC2,
>|
> +* | | | COMPRESSED_RGBA_BPTC_UNORM,
>|
> +* | | |
> COMPRESSED_SRGB_ALPHA_BPTC_UNORM, |
> +* | | |
> COMPRESSED_RGB_BPTC_SIGNED_FLOAT, |
> +* | | |
> COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT |
> +*
>
> +* | 64-bit | RGBA16F, RG32F, | COMPRESSED_RGB_S3TC_DXT1_EXT,
>|
> +* | | RGBA16UI, RG32UI,| COMPRESSED_SRGB_S3TC_DXT1_EXT,
> |
> +* | | RGBA16I, RG32I, | COMPRESSED_RGBA_S3TC_DXT1_EXT,
> |
> +* | | RGBA16, RGBA16_SNORM |
> COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT,|
> +* | | | COMPRESSED_RED_RGTC1,
>|
> +* | | | COMPRESSED_SIGNED_RED_RGTC1
>|
> +*
>
> +*/
>
Spec references should cite a spec version and page number or, if pulled
form the extension spec say as much. For instance:
>From the OpenGL specification version 4.4 core, p. 125:
blah
That way the reader knows exactly what document to go look at. Also, spec
citations are usually done as a block quite where the quoted text is
indented another 3 spaces from the rest of the comment.
> +
> + switch (compressedFormat) {
> + case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
> + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT:
> + case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
> + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EX
[Mesa-dev] [PATCH] freedreno/ir3: get the # of miplevels from getinfo
This fixes ARB_texture_query_levels to actually return the desired
value.
Signed-off-by: Ilia Mirkin
Cc: "10.4 10.5"
---
src/gallium/drivers/freedreno/ir3/ir3_compiler.c | 20
1 file changed, 20 insertions(+)
diff --git a/src/gallium/drivers/freedreno/ir3/ir3_compiler.c
b/src/gallium/drivers/freedreno/ir3/ir3_compiler.c
index dc4fd98..7e6e3df 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3_compiler.c
+++ b/src/gallium/drivers/freedreno/ir3/ir3_compiler.c
@@ -1646,6 +1646,26 @@ trans_txq(const struct instr_translater *t,
add_dst_reg_wrmask(ctx, instr, dst, 0, dst->WriteMask);
add_src_reg_wrmask(ctx, instr, level, level->SwizzleX, 0x1);
}
+
+ if (dst->WriteMask & TGSI_WRITEMASK_W) {
+ /* The # of levels comes from getinfo.z. We need to add 1 to
it, since
+* the value in TEX_CONST_0 is zero-based.
+*/
+ struct tgsi_dst_register tmp_dst;
+ struct tgsi_src_register *tmp_src;
+
+ tmp_src = get_internal_temp(ctx, &tmp_dst);
+ instr = instr_create(ctx, 5, OPC_GETINFO);
+ instr->cat5.type = get_utype(ctx);
+ instr->cat5.samp = samp->Index;
+ instr->cat5.tex = samp->Index;
+ add_dst_reg_wrmask(ctx, instr, &tmp_dst, 0, TGSI_WRITEMASK_Z);
+
+ instr = instr_create(ctx, 2, OPC_ADD_U);
+ add_dst_reg(ctx, instr, dst, 3);
+ add_src_reg(ctx, instr, tmp_src, src_swiz(tmp_src, 2));
+ ir3_reg_create(instr, 0, IR3_REG_IMMED)->iim_val = 1;
+ }
}
/* DDX/DDY */
--
2.0.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v4] mesa: improve ARB_copy_image internal format compat check
The memory layout of compatible internal formats may differ in bytes per
block, so TexFormat is not a reliable measure of compatibility. For example,
GL_RGB8 and GL_RGB8UI are compatible formats, but GL_RGB8 may be laid out in
memory as B8G8R8X8. If GL_RGB8UI has a 3 byte-per-block memory layout, the
existing compatibility check will fail.
Additionally, the current check allows any two compressed textures which share
block size to be used, whereas the spec gives an explicit table of compatible
formats.
v2: Use a switch instead of array iteration for block class and show the
correct GL error when internal formats are mismatched.
v3: Include spec citations for new compatibility checks, rearrange check
order to ensure that compressed, view-compatible formats return the
correct result, and make style fixes. Original commit message amended
for clarity.
v4: Reformatted spec citations.
Reviewed-by: Jason Ekstrand
---
src/mesa/main/copyimage.c | 151 +++---
1 file changed, 130 insertions(+), 21 deletions(-)
diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
index 455929d..fd22f28 100644
--- a/src/mesa/main/copyimage.c
+++ b/src/mesa/main/copyimage.c
@@ -33,6 +33,12 @@
#include "texobj.h"
#include "fbobject.h"
#include "textureview.h"
+#include "glformats.h"
+
+enum mesa_block_class {
+ BLOCK_CLASS_128_BITS,
+ BLOCK_CLASS_64_BITS
+};
static bool
prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int level,
@@ -253,6 +259,124 @@ check_region_bounds(struct gl_context *ctx,
struct gl_texture_image *tex_image,
return true;
}
+static bool
+compressed_format_compatible(struct gl_context *ctx,
+ GLenum compressedFormat, GLenum otherFormat)
+{
+ enum mesa_block_class compressedClass, otherClass;
+
+ /* Two view-incompatible compressed formats are never compatible. */
+ if (_mesa_is_compressed_format(ctx, otherFormat)) {
+ return false;
+ }
+
+ /*
+* From ARB_copy_image spec:
+*Table 4.X.1 (Compatible internal formats for copying between
+* compressed and uncompressed internal formats)
+*-
+*| Texel / | Uncompressed | |
+*| Block | internal format | Compressed internal format |
+*| size| | |
+*-
+*| 128-bit | RGBA32UI, | COMPRESSED_RGBA_S3TC_DXT3_EXT, |
+*| | RGBA32I, | COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT,|
+*| | RGBA32F | COMPRESSED_RGBA_S3TC_DXT5_EXT, |
+*| | | COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT,|
+*| | | COMPRESSED_RG_RGTC2,|
+*| | | COMPRESSED_SIGNED_RG_RGTC2, |
+*| | | COMPRESSED_RGBA_BPTC_UNORM, |
+*| | | COMPRESSED_SRGB_ALPHA_BPTC_UNORM, |
+*| | | COMPRESSED_RGB_BPTC_SIGNED_FLOAT, |
+*| | | COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT |
+*-
+*| 64-bit | RGBA16F, RG32F, | COMPRESSED_RGB_S3TC_DXT1_EXT, |
+*| | RGBA16UI, RG32UI, | COMPRESSED_SRGB_S3TC_DXT1_EXT, |
+*| | RGBA16I, RG32I, | COMPRESSED_RGBA_S3TC_DXT1_EXT, |
+*| | RGBA16, | COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT,|
+*| | RGBA16_SNORM | COMPRESSED_RED_RGTC1, |
+*| | | COMPRESSED_SIGNED_RED_RGTC1 |
+*-
+*/
+
+ switch (compressedFormat) {
+ case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
+ case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT:
+ case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
+ case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT:
+ case GL_COMPRESSED_RG_RGTC2:
+ case GL_COMPRESSED_SIGNED_RG_RGTC2:
+ case GL_COMPRESSED_RGBA_BPTC_UNORM:
+ case GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM:
+ case GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT:
+ case GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT:
+ compressedClass = BLOCK_CLASS_128_BITS;
+ break;
+ case GL_COMPRESSED_RGB_S3TC_DXT1_EXT:
+ case GL_COMPRESSED_SRGB_S3TC_DXT1_EXT:
+ case GL_COMPRESSED_RGBA_S3TC_DXT1_EXT:
+ case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT:
+ case GL_COMPRESSED_RED_RGTC1:
+ case GL_COMPRESSED_SIGNED_RED_RGTC1:
+ compressedClass = BLOCK_CLASS_64_BITS;
+ break;
+ default:
+ return false;
+ }
+
+ switch
Re: [Mesa-dev] [PATCH v4] mesa: improve ARB_copy_image internal format compat check
LGTM.
On Mar 7, 2015 8:34 PM, "Sean Burke" wrote:
> The memory layout of compatible internal formats may differ in bytes per
> block, so TexFormat is not a reliable measure of compatibility. For
> example,
> GL_RGB8 and GL_RGB8UI are compatible formats, but GL_RGB8 may be laid out
> in
> memory as B8G8R8X8. If GL_RGB8UI has a 3 byte-per-block memory layout, the
> existing compatibility check will fail.
>
> Additionally, the current check allows any two compressed textures which
> share
> block size to be used, whereas the spec gives an explicit table of
> compatible
> formats.
>
> v2: Use a switch instead of array iteration for block class and show the
> correct GL error when internal formats are mismatched.
> v3: Include spec citations for new compatibility checks, rearrange check
> order to ensure that compressed, view-compatible formats return the
> correct result, and make style fixes. Original commit message amended
> for clarity.
> v4: Reformatted spec citations.
>
> Reviewed-by: Jason Ekstrand
> ---
> src/mesa/main/copyimage.c | 151
> +++---
> 1 file changed, 130 insertions(+), 21 deletions(-)
>
> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
> index 455929d..fd22f28 100644
> --- a/src/mesa/main/copyimage.c
> +++ b/src/mesa/main/copyimage.c
> @@ -33,6 +33,12 @@
> #include "texobj.h"
> #include "fbobject.h"
> #include "textureview.h"
> +#include "glformats.h"
> +
> +enum mesa_block_class {
> + BLOCK_CLASS_128_BITS,
> + BLOCK_CLASS_64_BITS
> +};
>
> static bool
> prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int
> level,
> @@ -253,6 +259,124 @@ check_region_bounds(struct gl_context *ctx,
> struct gl_texture_image *tex_image,
> return true;
> }
>
> +static bool
> +compressed_format_compatible(struct gl_context *ctx,
> + GLenum compressedFormat, GLenum otherFormat)
> +{
> + enum mesa_block_class compressedClass, otherClass;
> +
> + /* Two view-incompatible compressed formats are never compatible. */
> + if (_mesa_is_compressed_format(ctx, otherFormat)) {
> + return false;
> + }
> +
> + /*
> +* From ARB_copy_image spec:
> +*Table 4.X.1 (Compatible internal formats for copying between
> +* compressed and uncompressed internal formats)
> +*
> -
> +*| Texel / | Uncompressed |
>|
> +*| Block | internal format | Compressed internal format
> |
> +*| size| |
>|
> +*
> -
> +*| 128-bit | RGBA32UI, | COMPRESSED_RGBA_S3TC_DXT3_EXT,
> |
> +*| | RGBA32I, |
> COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT,|
> +*| | RGBA32F | COMPRESSED_RGBA_S3TC_DXT5_EXT,
> |
> +*| | |
> COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT,|
> +*| | | COMPRESSED_RG_RGTC2,
> |
> +*| | | COMPRESSED_SIGNED_RG_RGTC2,
>|
> +*| | | COMPRESSED_RGBA_BPTC_UNORM,
>|
> +*| | |
> COMPRESSED_SRGB_ALPHA_BPTC_UNORM, |
> +*| | |
> COMPRESSED_RGB_BPTC_SIGNED_FLOAT, |
> +*| | |
> COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT |
> +*
> -
> +*| 64-bit | RGBA16F, RG32F, | COMPRESSED_RGB_S3TC_DXT1_EXT,
>|
> +*| | RGBA16UI, RG32UI, | COMPRESSED_SRGB_S3TC_DXT1_EXT,
> |
> +*| | RGBA16I, RG32I, | COMPRESSED_RGBA_S3TC_DXT1_EXT,
> |
> +*| | RGBA16, |
> COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT,|
> +*| | RGBA16_SNORM | COMPRESSED_RED_RGTC1,
>|
> +*| | | COMPRESSED_SIGNED_RED_RGTC1
>|
> +*
> -
> +*/
> +
> + switch (compressedFormat) {
> + case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
> + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT:
> + case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
> + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT:
> + case GL_COMPRESSED_RG_RGTC2:
> + case GL_COMPRESSED_SIGNED_RG_RGTC2:
> + case GL_COMPRESSED_RGBA_BPTC_UNORM:
> + case GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM:
> + case GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT:
> + case GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT:
> + compressedClass = BLOCK_CLASS_128_BITS;
> + break;
> + case GL_COMPRESSED_RGB_S3TC_DXT1_EXT:
> + case GL_COMPRESSED_SRGB_S3TC_DXT1_EXT:
> + case GL_COMPRESSED_RGBA_S3TC_DXT1_EXT:
> + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT:
> + case GL_COMPRES
Re: [Mesa-dev] [PATCH v4] mesa: improve ARB_copy_image internal format compat check
On Saturday, March 07, 2015 10:09:15 PM Jason Ekstrand wrote: > LGTM. Jason, Sean doesn't have commit access (it's actually his first patch) - if this is good to go, would you mind pushing it for him? Nice work, Sean! --Ken signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
