[Mesa-dev] [Bug 72889] wayland/weston EGL/GLESv2 client corruption/flicker on r600 mesa-10.x
https://bugs.freedesktop.org/show_bug.cgi?id=72889 --- Comment #4 from Martin Andersson --- I had the the same issue and I bisected it to this commit http://cgit.freedesktop.org/mesa/mesa/commit/?h=10.0&id=e64633e8c3a5498998a45ab721bf80edca101cf5 (r600g,radeonsi: share r600_texture.c) As far as I can tell that commit changed the default tiling mode from LINEAR_ALIGNED to 2D. That change seems to cause this issue, because when I change the last return in r600_choose_tiling from: return RADEON_SURF_MODE_2D; to return RADEON_SURF_MODE_LINEAR_ALIGNED; It works correctly. -- You are receiving this mail because: You are the QA Contact for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/2] code de-duplication and non-pci support
bump.. Any chance someone from the pci camp could have a look at this series? Also, I'm not quite sure about other build systems (android.. and is scons still a thing?).. something is probably needed for those, but I wouldn't know how to build with either of those. But something like this patchset is needed for those of us without PC (well, right now freedreno, but there are a couple more that hopefully end up upstream eventually). So would like to know how to move forward. BR, -R On Tue, Dec 17, 2013 at 1:01 PM, Erik Faye-Lund wrote: > On Sat, Dec 14, 2013 at 8:28 PM, Rob Clark wrote: >> From: Rob Clark >> >> It seems that over time, code related to finding driver name, dealing >> with pci-id table, etc, has been copy/pasted everywhere it was needed. >> Which is lame. And annoying if you have a device which is not pci. >> >> This patchset refactors it out into a simple loader util lib which is >> statically linked wherever it is needed. Perhaps there is some room >> for sharing other util bits (like _eglLog, perhaps) later. >> >> And once all this code is collected in one place, the 2nd patch only >> has to fix one place to add support for platform devices ;-) >> >> Rob Clark (2): >> loader: refactor duplicated code into loader util lib >> loader: fallback to drmGetVersion() for non-pci devices >> > > Nice. I've replaced Thierry's patch that does the latter, but only for > GBM, and the result still works fine. So: > > Tested-by: Erik Faye-Lund ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/7] i965: Add an assert for when SET_FIELD's value exceeds the field size.
This was one of the things we always wanted to do to this, to make it more
useful than just (value << FIELD_MASK).
---
src/mesa/drivers/dri/i965/brw_defines.h | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
b/src/mesa/drivers/dri/i965/brw_defines.h
index dc38ace..21a97a9 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -30,7 +30,13 @@
*/
#define INTEL_MASK(high, low) (((1<<((high)-(low)+1))-1)<<(low))
-#define SET_FIELD(value, field) (((value) << field ## _SHIFT) & field ## _MASK)
+#define SET_FIELD(value, field) \
+ ({ \
+ uint32_t fieldval = (value) << field ## _SHIFT; \
+ assert((fieldval & ~ field ## _MASK) == 0); \
+ fieldval & field ## _MASK;\
+ })
+
#define GET_FIELD(word, field) (((word) & field ## _MASK) >> field ## _SHIFT)
#ifndef BRW_DEFINES_H
--
1.8.5.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/7] i965: Add a safety check for emitting blits.
With all of the flipping and pitch twiddling and miptree layout involved in our blits, there are lots of ways for us to scribble outside of a buffer. Put in a check that we're not about to do so. This catches a bug that glamor was running into. --- src/mesa/drivers/dri/i965/intel_blit.c | 4 1 file changed, 4 insertions(+) diff --git a/src/mesa/drivers/dri/i965/intel_blit.c b/src/mesa/drivers/dri/i965/intel_blit.c index 13cc777..9162b1f 100644 --- a/src/mesa/drivers/dri/i965/intel_blit.c +++ b/src/mesa/drivers/dri/i965/intel_blit.c @@ -391,6 +391,10 @@ intelEmitCopyBlit(struct brw_context *brw, assert(dst_x < dst_x2); assert(dst_y < dst_y2); + assert(src_offset + (src_y + h - 1) * abs(src_pitch) + + (w * cpp) <= src_buffer->size); + assert(dst_offset + (dst_y + h - 1) * abs(dst_pitch) + + (w * cpp) <= dst_buffer->size); BEGIN_BATCH_BLT_TILED(8, dst_y_tiled, src_y_tiled); -- 1.8.5.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/7] i965: Warning fix
---
src/mesa/drivers/dri/i965/brw_eu_emit.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index c653828..c2ca10d 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -108,8 +108,6 @@ unsigned
brw_reg_type_to_hw_type(const struct brw_context *brw,
enum brw_reg_type type, unsigned file)
{
- bool imm = file == BRW_IMMEDIATE_VALUE;
-
if (file == BRW_IMMEDIATE_VALUE) {
const static int imm_hw_types[] = {
[BRW_REGISTER_TYPE_UD] = BRW_HW_REG_TYPE_UD,
--
1.8.5.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/7] i965: Don't call the blitter on addresses it can't handle.
Noticed by tex3d-maxsize on my next commit to check that our addresses
don't overflow.
---
src/mesa/drivers/dri/i965/intel_blit.c| 20
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 23 ---
2 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
b/src/mesa/drivers/dri/i965/intel_blit.c
index 7bc289f..13cc777 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.c
+++ b/src/mesa/drivers/dri/i965/intel_blit.c
@@ -229,12 +229,32 @@ intel_miptree_blit(struct brw_context *brw,
src_x += src_image_x;
src_y += src_image_y;
+ /* The blitter interprets the 16-bit src x/y as a signed 16-bit value,
+* where negative values are invalid. The values we're working with are
+* unsigned, so make sure we don't overflow.
+*/
+ if (src_x >= 32768 || src_y >= 32768) {
+ perf_debug("Falling back due to >=32k src offset (%d, %d)\n",
+ src_x, src_y);
+ return false;
+ }
+
uint32_t dst_image_x, dst_image_y;
intel_miptree_get_image_offset(dst_mt, dst_level, dst_slice,
&dst_image_x, &dst_image_y);
dst_x += dst_image_x;
dst_y += dst_image_y;
+ /* The blitter interprets the 16-bit destination x/y as a signed 16-bit
+* value. The values we're working with are unsigned, so make sure we
+* don't overflow.
+*/
+ if (dst_x >= 32768 || dst_y >= 32768) {
+ perf_debug("Falling back due to >=32k dst offset (%d, %d)\n",
+ dst_x, dst_y);
+ return false;
+ }
+
if (!intelEmitCopyBlit(brw,
src_mt->cpp,
src_pitch,
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index de47143..0818226 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -443,7 +443,8 @@ intel_miptree_choose_tiling(struct brw_context *brw,
if (minimum_pitch < 64)
return I915_TILING_NONE;
- if (ALIGN(minimum_pitch, 512) >= 32768) {
+ if (ALIGN(minimum_pitch, 512) >= 32768 ||
+ mt->total_width >= 32768 || mt->total_height >= 32768) {
perf_debug("%dx%d miptree too large to blit, falling back to untiled",
mt->total_width, mt->total_height);
return I915_TILING_NONE;
@@ -2233,6 +2234,22 @@ intel_miptree_release_map(struct intel_mipmap_tree *mt,
*map = NULL;
}
+static bool
+can_blit_slice(struct intel_mipmap_tree *mt,
+ unsigned int level, unsigned int slice)
+{
+ uint32_t image_x;
+ uint32_t image_y;
+ intel_miptree_get_image_offset(mt, level, slice, &image_x, &image_y);
+ if (image_x >= 32768 || image_y >= 32768)
+ return false;
+
+ if (mt->region->pitch >= 32768)
+ return false;
+
+ return true;
+}
+
static void
intel_miptree_map_singlesample(struct brw_context *brw,
struct intel_mipmap_tree *mt,
@@ -2276,11 +2293,11 @@ intel_miptree_map_singlesample(struct brw_context *brw,
!mt->compressed &&
(mt->region->tiling == I915_TILING_X ||
(brw->gen >= 6 && mt->region->tiling == I915_TILING_Y)) &&
-mt->region->pitch < 32768) {
+can_blit_slice(mt, level, slice)) {
intel_miptree_map_blit(brw, mt, map, level, slice);
} else if (mt->region->tiling != I915_TILING_NONE &&
mt->region->bo->size >= brw->max_gtt_map_object_size) {
- assert(mt->region->pitch < 32768);
+ assert(can_blit_slice(mt, level, slice));
intel_miptree_map_blit(brw, mt, map, level, slice);
#ifdef __SSE4_1__
} else if (!(mode & GL_MAP_WRITE_BIT) && !mt->compressed) {
--
1.8.5.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 6/7] i965: Fix incorrect bounds tracking for blit readpixels's GPU access.
While incorrect, it probably wouldn't affect anyone ever: You'd have to do an appropriately-formatted readpixels into a PBO, then overwrite the tail end of the updated area of the PBO with glBufferSubData(), and you wouldn't get appropriate synchronization. --- src/mesa/drivers/dri/i965/intel_pixel_read.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_pixel_read.c b/src/mesa/drivers/dri/i965/intel_pixel_read.c index 08cb762..0f6d2aa 100644 --- a/src/mesa/drivers/dri/i965/intel_pixel_read.c +++ b/src/mesa/drivers/dri/i965/intel_pixel_read.c @@ -127,8 +127,7 @@ do_blit_readpixels(struct gl_context * ctx, brw->front_buffer_dirty = dirty; dst_buffer = intel_bufferobj_buffer(brw, dst, - dst_offset, width * height * - irb->mt->cpp); + dst_offset, height * dst_stride); struct intel_mipmap_tree *pbo_mt = intel_miptree_create_for_bo(brw, -- 1.8.5.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 7/7] i965: Fix handling of MESA_pack_invert in blit (PBO) readpixels.
Fixes piglit GL_MESA_pack_invert/readpixels and GPU hangs with glamor and
cairo-gl.
Cc: 10.0 9.2
---
src/mesa/drivers/dri/i965/intel_pixel_read.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/intel_pixel_read.c
b/src/mesa/drivers/dri/i965/intel_pixel_read.c
index 0f6d2aa..2c85811 100644
--- a/src/mesa/drivers/dri/i965/intel_pixel_read.c
+++ b/src/mesa/drivers/dri/i965/intel_pixel_read.c
@@ -106,13 +106,15 @@ do_blit_readpixels(struct gl_context * ctx,
/* Mesa flips the dst_stride for pack->Invert, but we want our mt to have a
* normal dst_stride.
*/
+ struct gl_pixelstore_attrib uninverted_pack = *pack;
if (pack->Invert) {
dst_stride = -dst_stride;
dst_flip = true;
+ uninverted_pack.Invert = false;
}
dst_offset = (GLintptr)pixels;
- dst_offset += _mesa_image_offset(2, pack, width, height,
+ dst_offset += _mesa_image_offset(2, &uninverted_pack, width, height,
format, type, 0, 0, 0);
if (!_mesa_clip_copytexsubimage(ctx,
--
1.8.5.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 5/7] i965: Use SET_FIELD to safety check our x/y offsets in blits.
The earlier assert made sure that our math didn't exceed our bounds, but
this makes sure that we don't overflow from the high bits X into the low
bits of Y. We've already put checks in intel_miptree_blit(), but I've
wanted to expand the type in our protoype from short to uint32_t, and we
could get in trouble with intel_emit_linear_blit() if we did.
---
src/mesa/drivers/dri/i965/brw_defines.h | 5 +
src/mesa/drivers/dri/i965/intel_blit.c | 15 ---
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
b/src/mesa/drivers/dri/i965/brw_defines.h
index 21a97a9..76c5244 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1929,6 +1929,11 @@ enum brw_wm_barycentric_interp_mode {
#define CMD_MI_FLUSH 0x0200
+# define BLT_X_SHIFT 0
+# define BLT_X_MASKINTEL_MASK(15, 0)
+# define BLT_Y_SHIFT 16
+# define BLT_Y_MASKINTEL_MASK(31, 16)
+
#define GEN5_MI_REPORT_PERF_COUNT ((0x26 << 23) | (3 - 2))
/* DW0 */
# define GEN5_MI_COUNTER_SET_0 (0 << 6)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
b/src/mesa/drivers/dri/i965/intel_blit.c
index 9162b1f..330bac4 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.c
+++ b/src/mesa/drivers/dri/i965/intel_blit.c
@@ -33,6 +33,7 @@
#include "main/fbobject.h"
#include "brw_context.h"
+#include "brw_defines.h"
#include "intel_blit.h"
#include "intel_buffers.h"
#include "intel_fbo.h"
@@ -400,12 +401,12 @@ intelEmitCopyBlit(struct brw_context *brw,
OUT_BATCH(CMD | (8 - 2));
OUT_BATCH(BR13 | (uint16_t)dst_pitch);
- OUT_BATCH((dst_y << 16) | dst_x);
- OUT_BATCH((dst_y2 << 16) | dst_x2);
+ OUT_BATCH(SET_FIELD(dst_y, BLT_Y) | SET_FIELD(dst_x, BLT_X));
+ OUT_BATCH(SET_FIELD(dst_y2, BLT_Y) | SET_FIELD(dst_x2, BLT_X));
OUT_RELOC(dst_buffer,
I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
dst_offset);
- OUT_BATCH((src_y << 16) | src_x);
+ OUT_BATCH(SET_FIELD(src_y, BLT_Y) | SET_FIELD(src_x, BLT_X));
OUT_BATCH((uint16_t)src_pitch);
OUT_RELOC(src_buffer, I915_GEM_DOMAIN_RENDER, 0, src_offset);
@@ -479,8 +480,8 @@ intelEmitImmediateColorExpandBlit(struct brw_context *brw,
OUT_BATCH(0); /* pattern base addr */
OUT_BATCH(blit_cmd | ((3 - 2) + dwords));
- OUT_BATCH((y << 16) | x);
- OUT_BATCH(((y + h) << 16) | (x + w));
+ OUT_BATCH(SET_FIELD(y, BLT_Y) | SET_FIELD(x, BLT_X));
+ OUT_BATCH(SET_FIELD(y + h, BLT_Y) | SET_FIELD(x + w, BLT_X));
ADVANCE_BATCH();
intel_batchbuffer_data(brw, src_bits, dwords * 4, BLT_RING);
@@ -588,8 +589,8 @@ intel_miptree_set_alpha_to_one(struct brw_context *brw,
BEGIN_BATCH_BLT_TILED(6, dst_y_tiled, false);
OUT_BATCH(CMD | (6 - 2));
OUT_BATCH(BR13);
- OUT_BATCH((y << 16) | x);
- OUT_BATCH(((y + height) << 16) | (x + width));
+ OUT_BATCH(SET_FIELD(y, BLT_Y) | SET_FIELD(x, BLT_X));
+ OUT_BATCH(SET_FIELD(y + height, BLT_Y) | SET_FIELD(x + width, BLT_X));
OUT_RELOC(region->bo,
I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
0);
--
1.8.5.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 72895] Missing trees in flightgear 2.12.1 with r600 driver and mesa 10.0.1
https://bugs.freedesktop.org/show_bug.cgi?id=72895 Michel Dänzer changed: What|Removed |Added Assignee|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop. |.org|org CC||fred...@kde.org Component|Drivers/Gallium/r600|Mesa core -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: rename MESA format names to have the same names as PIPE formats
On Son, 2013-12-22 at 03:46 +0100, Marek Olšák wrote: > From: Marek Olšák > > The renaming was driven by the function st_mesa_format_to_pipe_format. > Only whole words are renamed to prevent regressions. > > For the MESA formats which don't have corresponding PIPE formats, I tried > to follow the PIPE_FORMAT_* conventions except for a few REV packed formats, > whose renaming is left for a future patch. This patch conflicts with Mark's MESA_FORMAT patches, right? Can you guys work out which way you want to take this? :) > /* msb <-- TEXEL BITS ---> lsb */ > /* */ [...] > + MESA_FORMAT_B8G8R8_UNORM, /* */ > + MESA_FORMAT_R8G8B8_UNORM, /* */ [...] > + MESA_FORMAT_R8G8B8_SRGB, /* */ I guess these are examples of formats where Mark called out the memory layout documentation comments being wrong. These formats cannot be usefully defined as packed values, so the format names and comments should be changed to reflect that, e.g. per this example: > - MESA_FORMAT_SIGNED_RGB_16, /* ushort[0]=R, ushort[1]=G, ushort[2]=B */ > - MESA_FORMAT_RGBA_FLOAT32, > - MESA_FORMAT_RGBA_FLOAT16, > - MESA_FORMAT_RGB_FLOAT32, > - MESA_FORMAT_RGB_FLOAT16, [...] > + MESA_FORMAT_R32G32B32A32_FLOAT, > + MESA_FORMAT_R16G16B16A16_FLOAT, > + MESA_FORMAT_R32G32B32_FLOAT, > + MESA_FORMAT_R16G16B16_FLOAT, [...] > - MESA_FORMAT_R_INT16, > - MESA_FORMAT_RG_INT16, > - MESA_FORMAT_RGB_INT16, > - MESA_FORMAT_RGBA_INT16, > - MESA_FORMAT_R_INT32, > - MESA_FORMAT_RG_INT32, > - MESA_FORMAT_RGB_INT32, > - MESA_FORMAT_RGBA_INT32, [...] > + MESA_FORMAT_R16_SINT, > + MESA_FORMAT_R16G16_SINT, > + MESA_FORMAT_R16G16B16_SINT, > + MESA_FORMAT_R16G16B16A16_SINT, > + MESA_FORMAT_R32_SINT, > + MESA_FORMAT_R32G32_SINT, > + MESA_FORMAT_R32G32B32_SINT, > + MESA_FORMAT_R32G32B32A32_SINT, These changes remove the naming distinction between formats which are defined as packed values and formats which are defined as arrays of values. I think that's a bad idea, there should be an explicit naming distinction between the two kinds of formats. -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/1] Adjust MESA_FORMAT color component ordering to match API docs
On Fre, 2013-12-20 at 12:28 -0800, Mark Mueller wrote: > Also, because these Mesa formats are defined as packed values, > you're > essentially changing the notation from big endian (aka human > readable) > to little endian. It's unfortunate that the packed > PIPE_FORMATs are > named in little endian order, that's a concession we had to > make when > adding them. > > > Are they all really big endian, currently it looks like a mix of who > knows what. The formats defined as packed values use big endian notation. Don't confuse them with formats which are not defined as packed values. > > Remove comments giving MESA_FORMAT color packings, some of > which are > > misleading. > > > Which ones are misleading, and how? > > > Bottom line, if the MESA_FORMATs follow the API's convention, then the > reader can go to the API for clarification and we don't have to > maintain that in the code. Then replace the comments with specific references to which API format describes each Mesa format, don't just remove them. -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
