[Mesa-dev] [PATCH 5/5] i965/gen9: Allocate YF/YS tiled buffer objects
In case of I915_TILING_{X,Y} we need to pass tiling format to libdrm
using drm_intel_bo_alloc_tiled(). But, In case of YF/YS tiled buffers
libdrm need not know about the tiling format because these buffers
don't have hardware support to be tiled or detiled through a fenced
region. libdrm still need to know buffer alignment value for its use
in kernel when resolving the relocation.
Using drm_intel_bo_alloc_for_render() for YF/YS tiled buffers
satisfy both the above conditions.
Signed-off-by: Anuj Phogat
Cc: Ben Widawsky
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 86 +--
1 file changed, 80 insertions(+), 6 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 615cbfb..d4d9e76 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -522,6 +522,65 @@ intel_lower_compressed_format(struct brw_context *brw,
mesa_format format)
}
}
+/* This function computes Yf/Ys tiled bo size and alignment. */
+static uint64_t
+intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment)
+{
+ const uint32_t bpp = mt->cpp * 8;
+ const uint32_t aspect_ratio = (bpp == 16 || bpp == 64) ? 2 : 1;
+ uint32_t tile_width, tile_height;
+ const uint64_t min_size = 512 * 1024;
+ const uint64_t max_size = 64 * 1024 * 1024;
+ uint64_t i, stride, size, aligned_y;
+
+ assert(mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE);
+
+ switch (bpp) {
+ case 8:
+ tile_height = 64;
+ break;
+ case 16:
+ case 32:
+ tile_height = 32;
+ break;
+ case 64:
+ case 128:
+ tile_height = 16;
+ break;
+ default:
+ tile_height = 0;
+ printf("Invalid bits per pixel in %s: bpp = %d\n",
+ __FUNCTION__, bpp);
+ }
+
+ if (mt->tr_mode == INTEL_MIPTREE_TRMODE_YS)
+ tile_height *= 4;
+
+ aligned_y = ALIGN(mt->total_height, tile_height);
+
+ stride = mt->total_width * mt->cpp;
+ tile_width = tile_height * mt->cpp * aspect_ratio;
+ stride = ALIGN(stride, tile_width);
+ size = stride * aligned_y;
+
+ if (mt->tr_mode == INTEL_MIPTREE_TRMODE_YF) {
+ *alignment = 4096;
+ size = ALIGN(size, 4096);
+ } else {
+ *alignment = 64 * 1024;
+ size = ALIGN(size, 64 * 1024);
+ }
+
+ if (size > max_size) {
+ mt->tr_mode = INTEL_MIPTREE_TRMODE_NONE;
+ return 0;
+ } else {
+ mt->pitch = stride;
+ for (i = min_size; i < size; i <<= 1)
+ ;
+ return i;
+ }
+}
struct intel_mipmap_tree *
intel_miptree_create(struct brw_context *brw,
@@ -575,12 +634,27 @@ intel_miptree_create(struct brw_context *brw,
unsigned long pitch;
mt->etc_format = etc_format;
- mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree",
- total_width, total_height, mt->cpp,
- &mt->tiling, &pitch,
- (expect_accelerated_upload ?
- BO_ALLOC_FOR_RENDER : 0));
- mt->pitch = pitch;
+
+ if (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE) {
+ unsigned alignment = 0;
+ unsigned long size;
+ size = intel_get_yf_ys_bo_size(mt, &alignment);
+
+ /* intel_get_yf_ys_bo_size() might change the tr_mode. */
+ if (size > 0 && mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE) {
+ mt->bo = drm_intel_bo_alloc_for_render(brw->bufmgr, "miptree",
+size, alignment);
+ }
+ }
+
+ if (mt->tr_mode == INTEL_MIPTREE_TRMODE_NONE) {
+ mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree",
+total_width, total_height, mt->cpp,
+&mt->tiling, &pitch,
+(expect_accelerated_upload ?
+ BO_ALLOC_FOR_RENDER : 0));
+ mt->pitch = pitch;
+ }
/* If the BO is too large to fit in the aperture, we need to use the
* BLT engine to support it. Prior to Sandybridge, the BLT paths can't
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/5] i965/gen9: Plugin the code for selecting YF/YS tiling on skl+
Buffers with Yf/Ys tiling end up using meta upload / download
paths or the blitter for cases where they used tiled_memcpy paths
in case of Y tiling. This has exposed some bugs in meta path. To
avoid any piglit regressions on SKL this patch keeps the Yf/Ys
tiling disabled at the moment.
V3: Make brw_miptree_choose_tr_mode() actually choose TRMODE. (Ben)
Few cosmetic changes.
V4: Get rid of brw_miptree_choose_tr_mode().
Take care of all tile resource modes {Yf, Ys, none} for all
generations at one place.
Signed-off-by: Anuj Phogat
Cc: Ben Widawsky
---
src/mesa/drivers/dri/i965/brw_tex_layout.c | 97 --
1 file changed, 79 insertions(+), 18 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
b/src/mesa/drivers/dri/i965/brw_tex_layout.c
index b9ac4cf..c0ef5cc 100644
--- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
@@ -807,27 +807,88 @@ brw_miptree_layout(struct brw_context *brw,
enum intel_miptree_tiling_mode requested,
struct intel_mipmap_tree *mt)
{
- mt->tr_mode = INTEL_MIPTREE_TRMODE_NONE;
+ const unsigned bpp = mt->cpp * 8;
+ const bool is_tr_mode_yf_ys_allowed =
+ brw->gen >= 9 &&
+ !for_bo &&
+ !mt->compressed &&
+ /* Enable YF/YS tiling only for color surfaces because depth and
+ * stencil surfaces are not supported in blitter using fast copy
+ * blit and meta PBO upload, download paths. No other paths
+ * currently support Yf/Ys tiled surfaces.
+ * FIXME: Remove this restriction once we have a tiled_memcpy()
+ * path to do depth/stencil data upload/download to Yf/Ys tiled
+ * surfaces.
+ */
+ _mesa_is_format_color_format(mt->format) &&
+ (requested == INTEL_MIPTREE_TILING_Y ||
+ requested == INTEL_MIPTREE_TILING_ANY) &&
+ (bpp && is_power_of_two(bpp)) &&
+ /* FIXME: To avoid piglit regressions keep the Yf/Ys tiling
+ * disabled at the moment.
+ */
+ false;
- intel_miptree_set_alignment(brw, mt);
- intel_miptree_set_total_width_height(brw, mt);
+ /* Lower index (Yf) is the higher priority mode */
+ const uint32_t tr_mode[3] = {INTEL_MIPTREE_TRMODE_YF,
+INTEL_MIPTREE_TRMODE_YS,
+INTEL_MIPTREE_TRMODE_NONE};
+ int i = is_tr_mode_yf_ys_allowed ? 0 : ARRAY_SIZE(tr_mode) - 1;
- if (!mt->total_width || !mt->total_height) {
- intel_miptree_release(&mt);
- return;
- }
+ while (i < ARRAY_SIZE(tr_mode)) {
+ if (brw->gen < 9)
+ assert(tr_mode[i] == INTEL_MIPTREE_TRMODE_NONE);
+ else
+ assert(tr_mode[i] == INTEL_MIPTREE_TRMODE_YF ||
+tr_mode[i] == INTEL_MIPTREE_TRMODE_YS ||
+tr_mode[i] == INTEL_MIPTREE_TRMODE_NONE);
- /* On Gen9+ the alignment values are expressed in multiples of the block
-* size
-*/
- if (brw->gen >= 9) {
- unsigned int i, j;
- _mesa_get_format_block_size(mt->format, &i, &j);
- mt->align_w /= i;
- mt->align_h /= j;
- }
+ mt->tr_mode = tr_mode[i];
+ intel_miptree_set_alignment(brw, mt);
+ intel_miptree_set_total_width_height(brw, mt);
- if (!for_bo)
- mt->tiling = brw_miptree_choose_tiling(brw, requested, mt);
+ if (!mt->total_width || !mt->total_height) {
+ intel_miptree_release(&mt);
+ return;
+ }
+
+ /* On Gen9+ the alignment values are expressed in multiples of the
+ * block size.
+ */
+ if (brw->gen >= 9) {
+ unsigned int i, j;
+ _mesa_get_format_block_size(mt->format, &i, &j);
+ mt->align_w /= i;
+ mt->align_h /= j;
+ }
+
+ if (!for_bo)
+ mt->tiling = brw_miptree_choose_tiling(brw, requested, mt);
+
+ if (is_tr_mode_yf_ys_allowed) {
+ unsigned int level = 0;
+ assert(brw->gen >= 9);
+
+ if (mt->tiling == I915_TILING_Y ||
+ mt->tiling == (I915_TILING_Y | I915_TILING_X) ||
+ mt->tr_mode == INTEL_MIPTREE_TRMODE_NONE) {
+/* FIXME: Don't allow YS tiling at the moment. Using 64KB tiling
+ * for small textures might result in to memory wastage. Revisit
+ * this condition when we have more information about the specific
+ * cases where using YS over YF will be useful.
+ */
+if (mt->tr_mode != INTEL_MIPTREE_TRMODE_YS)
+ return;
+ }
+ /* Failed to use selected tr_mode. Free up the memory allocated
+ * for miptree levels in intel_miptree_total_width_height().
+ */
+ for (level = mt->first_level; level <= mt->last_level; level++) {
+free(mt->level
[Mesa-dev] [PATCH 1/5] i965: Make a helper function intel_miptree_set_alignment()
Signed-off-by: Anuj Phogat
Cc: Ben Widawsky
---
Jenkins showed no piglit regressions with this series.
src/mesa/drivers/dri/i965/brw_tex_layout.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
b/src/mesa/drivers/dri/i965/brw_tex_layout.c
index 41a9964..b9ac4cf 100644
--- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
@@ -761,16 +761,12 @@ intel_miptree_set_total_width_height(struct brw_context
*brw,
mt->total_width, mt->total_height, mt->cpp);
}
-void
-brw_miptree_layout(struct brw_context *brw,
- bool for_bo,
- enum intel_miptree_tiling_mode requested,
- struct intel_mipmap_tree *mt)
+static void
+intel_miptree_set_alignment(struct brw_context *brw,
+struct intel_mipmap_tree *mt)
{
bool gen6_hiz_or_stencil = false;
- mt->tr_mode = INTEL_MIPTREE_TRMODE_NONE;
-
if (brw->gen == 6 && mt->array_layout == ALL_SLICES_AT_EACH_LOD) {
const GLenum base_format = _mesa_get_format_base_format(mt->format);
gen6_hiz_or_stencil = _mesa_is_depth_or_stencil_format(base_format);
@@ -803,7 +799,17 @@ brw_miptree_layout(struct brw_context *brw,
mt->align_w = intel_horizontal_texture_alignment_unit(brw, mt);
mt->align_h = intel_vertical_texture_alignment_unit(brw, mt);
}
+}
+
+void
+brw_miptree_layout(struct brw_context *brw,
+ bool for_bo,
+ enum intel_miptree_tiling_mode requested,
+ struct intel_mipmap_tree *mt)
+{
+ mt->tr_mode = INTEL_MIPTREE_TRMODE_NONE;
+ intel_miptree_set_alignment(brw, mt);
intel_miptree_set_total_width_height(brw, mt);
if (!mt->total_width || !mt->total_height) {
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/5] i965: Make a helper function intel_miptree_release_levels()
Signed-off-by: Anuj Phogat
Cc: Ben Widawsky
---
src/mesa/drivers/dri/i965/brw_tex_layout.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
b/src/mesa/drivers/dri/i965/brw_tex_layout.c
index c0ef5cc..c185e41 100644
--- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
@@ -801,6 +801,17 @@ intel_miptree_set_alignment(struct brw_context *brw,
}
}
+static void
+intel_miptree_release_levels(struct intel_mipmap_tree *mt)
+{
+ unsigned int level = 0;
+
+ for (level = mt->first_level; level <= mt->last_level; level++) {
+ free(mt->level[level].slice);
+ mt->level[level].slice = NULL;
+ }
+}
+
void
brw_miptree_layout(struct brw_context *brw,
bool for_bo,
@@ -866,7 +877,6 @@ brw_miptree_layout(struct brw_context *brw,
mt->tiling = brw_miptree_choose_tiling(brw, requested, mt);
if (is_tr_mode_yf_ys_allowed) {
- unsigned int level = 0;
assert(brw->gen >= 9);
if (mt->tiling == I915_TILING_Y ||
@@ -883,10 +893,7 @@ brw_miptree_layout(struct brw_context *brw,
/* Failed to use selected tr_mode. Free up the memory allocated
* for miptree levels in intel_miptree_total_width_height().
*/
- for (level = mt->first_level; level <= mt->last_level; level++) {
-free(mt->level[level].slice);
-mt->level[level].slice = NULL;
- }
+ intel_miptree_release_levels(mt);
}
i++;
}
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/5] i965: Make a helper function intel_miptree_can_use_tr_mode()
Signed-off-by: Anuj Phogat
Cc: Ben Widawsky
---
src/mesa/drivers/dri/i965/brw_tex_layout.c | 30 +++---
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
b/src/mesa/drivers/dri/i965/brw_tex_layout.c
index c185e41..39c6a39 100644
--- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
@@ -812,6 +812,23 @@ intel_miptree_release_levels(struct intel_mipmap_tree *mt)
}
}
+static bool
+intel_miptree_can_use_tr_mode(const struct intel_mipmap_tree *mt)
+{
+ if (mt->tiling == I915_TILING_Y ||
+ mt->tiling == (I915_TILING_Y | I915_TILING_X) ||
+ mt->tr_mode == INTEL_MIPTREE_TRMODE_NONE) {
+ /* FIXME: Don't allow YS tiling at the moment. Using 64KB tiling
+ * for small textures might result in to memory wastage. Revisit
+ * this condition when we have more information about the specific
+ * cases where using YS over YF will be useful.
+ */
+ if (mt->tr_mode != INTEL_MIPTREE_TRMODE_YS)
+ return true;
+ }
+ return false;
+}
+
void
brw_miptree_layout(struct brw_context *brw,
bool for_bo,
@@ -879,17 +896,8 @@ brw_miptree_layout(struct brw_context *brw,
if (is_tr_mode_yf_ys_allowed) {
assert(brw->gen >= 9);
- if (mt->tiling == I915_TILING_Y ||
- mt->tiling == (I915_TILING_Y | I915_TILING_X) ||
- mt->tr_mode == INTEL_MIPTREE_TRMODE_NONE) {
-/* FIXME: Don't allow YS tiling at the moment. Using 64KB tiling
- * for small textures might result in to memory wastage. Revisit
- * this condition when we have more information about the specific
- * cases where using YS over YF will be useful.
- */
-if (mt->tr_mode != INTEL_MIPTREE_TRMODE_YS)
- return;
- }
+ if (intel_miptree_can_use_tr_mode(mt))
+return;
/* Failed to use selected tr_mode. Free up the memory allocated
* for miptree levels in intel_miptree_total_width_height().
*/
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/5] i965/gen9: Add XY_FAST_COPY_BLT support to intelEmitCopyBlit()
This patch enables using XY_FAST_COPY_BLT only for Yf/Ys tiled buffers.
It can be later turned on for other tiling patterns (X,Y) too.
V3: Flush in between sequential fast copy blits.
Fix src/dst alignment requirements.
Make can_fast_copy_blit() helper.
Use ffs(), is_power_of_two()
Move overlap computation inside intel_miptree_blit().
V4: Use _mesa_regions_overlap() function.
Simplify horizontal and vertical alignment computations.
Signed-off-by: Anuj Phogat
Cc: Ben Widawsky
---
src/mesa/drivers/dri/i965/intel_blit.c | 295 ++-
src/mesa/drivers/dri/i965/intel_blit.h | 2 +
src/mesa/drivers/dri/i965/intel_copy_image.c | 2 +
src/mesa/drivers/dri/i965/intel_reg.h| 16 ++
4 files changed, 268 insertions(+), 47 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
b/src/mesa/drivers/dri/i965/intel_blit.c
index 5afc771..800ed7e 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.c
+++ b/src/mesa/drivers/dri/i965/intel_blit.c
@@ -27,6 +27,7 @@
#include "main/mtypes.h"
+#include "main/blit.h"
#include "main/context.h"
#include "main/enums.h"
#include "main/colormac.h"
@@ -43,6 +44,23 @@
#define FILE_DEBUG_FLAG DEBUG_BLIT
+#define SET_TILING_XY_FAST_COPY_BLT(tiling, tr_mode, type) \
+({ \
+ switch (tiling) { \
+ case I915_TILING_X: \
+ CMD |= type ## _TILED_X; \
+ break; \
+ case I915_TILING_Y: \
+ if (tr_mode == INTEL_MIPTREE_TRMODE_YS)\
+ CMD |= type ## _TILED_64K; \
+ else \
+ CMD |= type ## _TILED_Y;\
+ break; \
+ default: \
+ unreachable("not reached");\
+ } \
+})
+
static void
intel_miptree_set_alpha_to_one(struct brw_context *brw,
struct intel_mipmap_tree *mt,
@@ -75,6 +93,10 @@ static uint32_t
br13_for_cpp(int cpp)
{
switch (cpp) {
+ case 16:
+ return BR13_32323232;
+ case 8:
+ return BR13_16161616;
case 4:
return BR13_;
break;
@@ -89,6 +111,66 @@ br13_for_cpp(int cpp)
}
}
+static uint32_t
+get_tr_horizontal_align(uint32_t tr_mode, uint32_t cpp, bool is_src) {
+ /* Alignment tables for YF/YS tiled surfaces. */
+ const uint32_t align_2d_yf[] = {64, 64, 32, 32, 16};
+ const uint32_t align_2d_ys[] = {256, 256, 128, 128, 64};
+ const uint32_t bpp = cpp * 8;
+ const uint32_t shift = is_src ? 17 : 10;
+ uint32_t align;
+ int i = 0;
+
+ if (tr_mode == INTEL_MIPTREE_TRMODE_NONE)
+ return 0;
+
+ /* Compute array index. */
+ assert (bpp >= 8 && bpp <= 128 && is_power_of_two(bpp));
+ i = ffs(bpp / 8) - 1;
+
+ align = tr_mode == INTEL_MIPTREE_TRMODE_YF ?
+ align_2d_yf[i] :
+ align_2d_ys[i];
+
+ assert(is_power_of_two(align));
+
+ /* XY_FAST_COPY_BLT doesn't support horizontal alignment of 16. */
+ if (align == 16)
+ align = 32;
+
+ return (ffs(align) - 6) << shift;
+}
+
+static uint32_t
+get_tr_vertical_align(uint32_t tr_mode, uint32_t cpp, bool is_src) {
+ /* Vertical alignment tables for YF/YS tiled surfaces. */
+ const unsigned align_2d_yf[] = {64, 32, 32, 16, 16};
+ const unsigned align_2d_ys[] = {256, 128, 128, 64, 64};
+ const uint32_t bpp = cpp * 8;
+ const uint32_t shift = is_src ? 15 : 8;
+ uint32_t align;
+ int i = 0;
+
+ if (tr_mode == INTEL_MIPTREE_TRMODE_NONE)
+ return 0;
+
+ /* Compute array index. */
+ assert (bpp >= 8 && bpp <= 128 && is_power_of_two(bpp));
+ i = ffs(bpp / 8) - 1;
+
+ align = tr_mode == INTEL_MIPTREE_TRMODE_YF ?
+ align_2d_yf[i] :
+ align_2d_ys[i];
+
+ assert(is_power_of_two(align));
+
+ /* XY_FAST_COPY_BLT doesn't support vertical alignments of 16 and 32. */
+ if (align == 16 || align == 32)
+ align = 64;
+
+ return (ffs(align) - 7) << shift;
+}
+
/**
* Emits the packet for switching the blitter from X to Y tiled or back.
*
@@ -281,9 +363,11 @@ intel_miptree_blit(struct brw_context *brw,
src_pitch,
src_mt->bo, src_mt->offset,
src_mt->tiling,
+ src_mt->tr_mode,
dst_mt->pitch,
[Mesa-dev] [PATCH 5/5] i965/skl: Extract the blit command setup in to a helper
Signed-off-by: Anuj Phogat
---
src/mesa/drivers/dri/i965/intel_blit.c | 93 ++
1 file changed, 61 insertions(+), 32 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
b/src/mesa/drivers/dri/i965/intel_blit.c
index 800ed7e..987b5f5 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.c
+++ b/src/mesa/drivers/dri/i965/intel_blit.c
@@ -458,6 +458,51 @@ can_fast_copy_blit(struct brw_context *brw,
return true;
}
+static uint32_t
+xy_blit_cmd(uint32_t src_tiling, uint32_t src_tr_mode,
+uint32_t dst_tiling, uint32_t dst_tr_mode,
+uint32_t cpp, bool use_fast_copy_blit)
+{
+ uint32_t CMD = 0;
+
+ if (use_fast_copy_blit) {
+ CMD = XY_FAST_COPY_BLT_CMD;
+
+ if (dst_tiling != I915_TILING_NONE)
+ SET_TILING_XY_FAST_COPY_BLT(dst_tiling, dst_tr_mode, XY_FAST_DST);
+
+ if (src_tiling != I915_TILING_NONE)
+ SET_TILING_XY_FAST_COPY_BLT(src_tiling, src_tr_mode, XY_FAST_SRC);
+
+ CMD |= get_tr_horizontal_align(src_tr_mode, cpp, true /* is_src */);
+ CMD |= get_tr_vertical_align(src_tr_mode, cpp, true /* is_src */);
+
+ CMD |= get_tr_horizontal_align(dst_tr_mode, cpp, false /* is_src */);
+ CMD |= get_tr_vertical_align(dst_tr_mode, cpp, false /* is_src */);
+
+ } else {
+ assert(cpp <= 4);
+ switch (cpp) {
+ case 1:
+ case 2:
+ CMD = XY_SRC_COPY_BLT_CMD;
+ break;
+ case 4:
+ CMD = XY_SRC_COPY_BLT_CMD | XY_BLT_WRITE_ALPHA | XY_BLT_WRITE_RGB;
+ break;
+ default:
+ unreachable("not reached");
+ }
+
+ if (dst_tiling != I915_TILING_NONE)
+ CMD |= XY_DST_TILED;
+
+ if (src_tiling != I915_TILING_NONE)
+ CMD |= XY_SRC_TILED;
+ }
+ return CMD;
+}
+
/* Copy BitBlt
*/
bool
@@ -547,24 +592,18 @@ intelEmitCopyBlit(struct brw_context *brw,
if (dst_tr_mode == INTEL_MIPTREE_TRMODE_YF)
BR13 |= XY_FAST_DST_TRMODE_YF;
- CMD = XY_FAST_COPY_BLT_CMD;
+ CMD = xy_blit_cmd(src_tiling, src_tr_mode,
+dst_tiling, dst_tr_mode,
+cpp, use_fast_copy_blit);
- if (dst_tiling != I915_TILING_NONE) {
- SET_TILING_XY_FAST_COPY_BLT(dst_tiling, dst_tr_mode, XY_FAST_DST);
- /* Pitch value should be specified as a number of Dwords. */
+ /* For tiled source and destination, pitch value should be specified
+ * as a number of Dwords.
+ */
+ if (dst_tiling != I915_TILING_NONE)
dst_pitch /= 4;
- }
- if (src_tiling != I915_TILING_NONE) {
- SET_TILING_XY_FAST_COPY_BLT(src_tiling, src_tr_mode, XY_FAST_SRC);
- /* Pitch value should be specified as a number of Dwords. */
- src_pitch /= 4;
- }
-
- CMD |= get_tr_horizontal_align(src_tr_mode, cpp, true /* is_src */);
- CMD |= get_tr_vertical_align(src_tr_mode, cpp, true /* is_src */);
- CMD |= get_tr_horizontal_align(dst_tr_mode, cpp, false /* is_src */);
- CMD |= get_tr_vertical_align(dst_tr_mode, cpp, false /* is_src */);
+ if (src_tiling != I915_TILING_NONE)
+ src_pitch /= 4;
} else {
/* For big formats (such as floating point), do the copy using 16 or
@@ -599,26 +638,16 @@ intelEmitCopyBlit(struct brw_context *brw,
assert(cpp <= 4);
BR13 = br13_for_cpp(cpp) | translate_raster_op(logic_op) << 16;
- switch (cpp) {
- case 1:
- case 2:
- CMD = XY_SRC_COPY_BLT_CMD;
- break;
- case 4:
- CMD = XY_SRC_COPY_BLT_CMD | XY_BLT_WRITE_ALPHA | XY_BLT_WRITE_RGB;
- break;
- default:
- return false;
- }
- if (dst_tiling != I915_TILING_NONE) {
- CMD |= XY_DST_TILED;
+ CMD = xy_blit_cmd(src_tiling, src_tr_mode,
+dst_tiling, dst_tr_mode,
+cpp, use_fast_copy_blit);
+
+ if (dst_tiling != I915_TILING_NONE)
dst_pitch /= 4;
- }
- if (src_tiling != I915_TILING_NONE) {
- CMD |= XY_SRC_TILED;
+
+ if (src_tiling != I915_TILING_NONE)
src_pitch /= 4;
- }
}
if (dst_y2 <= dst_y || dst_x2 <= dst_x) {
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/5] mesa/st: Use global function _mesa_regions_overlap()
Signed-off-by: Anuj Phogat
Cc: Ben Widawsky
---
src/mesa/state_tracker/st_cb_drawpixels.c | 30 +++---
1 file changed, 3 insertions(+), 27 deletions(-)
diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c
b/src/mesa/state_tracker/st_cb_drawpixels.c
index a6a98c8..e736d4b 100644
--- a/src/mesa/state_tracker/st_cb_drawpixels.c
+++ b/src/mesa/state_tracker/st_cb_drawpixels.c
@@ -33,6 +33,7 @@
#include "main/imports.h"
#include "main/image.h"
#include "main/bufferobj.h"
+#include "main/blit.h"
#include "main/format_pack.h"
#include "main/macros.h"
#include "main/mtypes.h"
@@ -1313,31 +1314,6 @@ st_get_color_read_renderbuffer(struct gl_context *ctx)
/**
- * \return TRUE if two regions overlap, FALSE otherwise
- */
-static boolean
-regions_overlap(int srcX0, int srcY0,
-int srcX1, int srcY1,
-int dstX0, int dstY0,
-int dstX1, int dstY1)
-{
- if (MAX2(srcX0, srcX1) < MIN2(dstX0, dstX1))
- return FALSE; /* src completely left of dst */
-
- if (MAX2(dstX0, dstX1) < MIN2(srcX0, srcX1))
- return FALSE; /* dst completely left of src */
-
- if (MAX2(srcY0, srcY1) < MIN2(dstY0, dstY1))
- return FALSE; /* src completely above dst */
-
- if (MAX2(dstY0, dstY1) < MIN2(srcY0, srcY1))
- return FALSE; /* dst completely above src */
-
- return TRUE; /* some overlap */
-}
-
-
-/**
* Try to do a glCopyPixels for simple cases with a blit by calling
* pipe->blit().
*
@@ -1420,8 +1396,8 @@ blit_copy_pixels(struct gl_context *ctx, GLint srcx,
GLint srcy,
}
if (rbRead != rbDraw ||
- !regions_overlap(readX, readY, readX + readW, readY + readH,
- drawX, drawY, drawX + drawW, drawY + drawH)) {
+ !_mesa_regions_overlap(readX, readY, readX + readW, readY + readH,
+ drawX, drawY, drawX + drawW, drawY + drawH)) {
struct pipe_blit_info blit;
memset(&blit, 0, sizeof(blit));
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/5] mesa: Add a new helper function _mesa_regions_overlap()
Signed-off-by: Anuj Phogat
Cc: Ben Widawsky
---
Jenkins showed no piglit regressions with this series.
src/mesa/main/blit.c | 26 ++
src/mesa/main/blit.h | 6 ++
2 files changed, 32 insertions(+)
diff --git a/src/mesa/main/blit.c b/src/mesa/main/blit.c
index db8fee5..4765198 100644
--- a/src/mesa/main/blit.c
+++ b/src/mesa/main/blit.c
@@ -37,6 +37,7 @@
#include "framebuffer.h"
#include "glformats.h"
#include "mtypes.h"
+#include "macros.h"
#include "state.h"
@@ -59,6 +60,31 @@ find_attachment(const struct gl_framebuffer *fb,
/**
+ * \return true if two regions overlap, false otherwise
+ */
+bool
+_mesa_regions_overlap(int srcX0, int srcY0,
+ int srcX1, int srcY1,
+ int dstX0, int dstY0,
+ int dstX1, int dstY1)
+{
+ if (MAX2(srcX0, srcX1) < MIN2(dstX0, dstX1))
+ return false; /* dst completely right of src */
+
+ if (MAX2(dstX0, dstX1) < MIN2(srcX0, srcX1))
+ return false; /* dst completely left of src */
+
+ if (MAX2(srcY0, srcY1) < MIN2(dstY0, dstY1))
+ return false; /* dst completely above src */
+
+ if (MAX2(dstY0, dstY1) < MIN2(srcY0, srcY1))
+ return false; /* dst completely below src */
+
+ return true; /* some overlap */
+}
+
+
+/**
* Helper function for checking if the datatypes of color buffers are
* compatible for glBlitFramebuffer. From the 3.1 spec, page 198:
*
diff --git a/src/mesa/main/blit.h b/src/mesa/main/blit.h
index 54b946e..88dd4a9 100644
--- a/src/mesa/main/blit.h
+++ b/src/mesa/main/blit.h
@@ -28,6 +28,12 @@
#include "glheader.h"
+extern bool
+_mesa_regions_overlap(int srcX0, int srcY0,
+ int srcX1, int srcY1,
+ int dstX0, int dstY0,
+ int dstX1, int dstY1);
+
extern void
_mesa_blit_framebuffer(struct gl_context *ctx,
struct gl_framebuffer *readFb,
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/5] mesa/swrast: Use global function _mesa_regions_overlap()
Signed-off-by: Anuj Phogat
---
src/mesa/swrast/s_copypix.c | 16 +++-
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/src/mesa/swrast/s_copypix.c b/src/mesa/swrast/s_copypix.c
index 68c83e4..8fde0c2 100644
--- a/src/mesa/swrast/s_copypix.c
+++ b/src/mesa/swrast/s_copypix.c
@@ -27,6 +27,7 @@
#include "main/context.h"
#include "main/condrender.h"
#include "main/macros.h"
+#include "main/blit.h"
#include "main/pixeltransfer.h"
#include "main/imports.h"
@@ -52,19 +53,8 @@ regions_overlap(GLint srcx, GLint srcy,
GLfloat zoomX, GLfloat zoomY)
{
if (zoomX == 1.0 && zoomY == 1.0) {
- /* no zoom */
- if (srcx >= dstx + width || (srcx + width <= dstx)) {
- return GL_FALSE;
- }
- else if (srcy < dsty) { /* this is OK */
- return GL_FALSE;
- }
- else if (srcy > dsty + height) {
- return GL_FALSE;
- }
- else {
- return GL_TRUE;
- }
+ return _mesa_regions_overlap(srcx, srcy, srcx + width, srcy + height,
+ dstx, dsty, dstx + width, dsty + height);
}
else {
/* add one pixel of slop when zooming, just to be safe */
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Remove break after return
Signed-off-by: Anuj Phogat
---
src/mesa/drivers/dri/i965/intel_blit.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
b/src/mesa/drivers/dri/i965/intel_blit.c
index 987b5f5..068565c 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.c
+++ b/src/mesa/drivers/dri/i965/intel_blit.c
@@ -99,13 +99,10 @@ br13_for_cpp(int cpp)
return BR13_16161616;
case 4:
return BR13_;
- break;
case 2:
return BR13_565;
- break;
case 1:
return BR13_8;
- break;
default:
unreachable("not reached");
}
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Check for miptree pitch alignment before using intel_miptree_map_movntdqa()
I don't know where this alignment restriction came from. We have an
assert() in intel_miptree_map_movntdqa() which expects the pitch to
be 16 byte aligned.
Signed-off-by: Anuj Phogat
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index d4d9e76..b0b2697 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -2659,7 +2659,9 @@ intel_miptree_map(struct brw_context *brw,
} else if (use_intel_mipree_map_blit(brw, mt, mode, level, slice)) {
intel_miptree_map_blit(brw, mt, map, level, slice);
#if defined(USE_SSE41)
- } else if (!(mode & GL_MAP_WRITE_BIT) && !mt->compressed && cpu_has_sse4_1)
{
+ } else if (!(mode & GL_MAP_WRITE_BIT) &&
+ !mt->compressed && cpu_has_sse4_1 &&
+ (mt->pitch % 16 == 0)) {
intel_miptree_map_movntdqa(brw, mt, map, level, slice);
#endif
} else {
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] meta: Use is_power_of_two() helper function
Signed-off-by: Anuj Phogat
---
src/mesa/drivers/common/meta_blit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/mesa/drivers/common/meta_blit.c
b/src/mesa/drivers/common/meta_blit.c
index bb21642..9cace2b 100644
--- a/src/mesa/drivers/common/meta_blit.c
+++ b/src/mesa/drivers/common/meta_blit.c
@@ -82,7 +82,7 @@ setup_glsl_msaa_blit_scaled_shader(struct gl_context *ctx,
y_scale = samples * 0.5;
/* We expect only power of 2 samples in source multisample buffer. */
- assert(samples > 0 && (samples & (samples - 1)) == 0);
+ assert(samples > 0 && is_power_of_two(samples));
while (samples >> (shader_offset + 1)) {
shader_offset++;
}
@@ -263,7 +263,7 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,
}
/* We expect only power of 2 samples in source multisample buffer. */
- assert(samples > 0 && (samples & (samples - 1)) == 0);
+ assert(samples > 0 && is_power_of_two(samples));
while (samples >> (shader_offset + 1)) {
shader_offset++;
}
@@ -434,7 +434,7 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,
* (so the floating point exponent just gets increased), rather than
* doing a naive sum and dividing.
*/
- assert((samples & (samples - 1)) == 0);
+ assert(is_power_of_two(samples));
/* Fetch each individual sample. */
sample_resolve = rzalloc_size(mem_ctx, 1);
for (i = 0; i < samples; i++) {
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] mesa: Turn need_rgb_to_luminance_conversion() in to a global function
This will be used by _mesa_meta_pbo_GetTexSubImage() in a later patch.
Signed-off-by: Anuj Phogat
---
src/mesa/main/readpix.c | 11 ++-
src/mesa/main/readpix.h | 3 +++
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index cba9db8..a3357cd 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -46,8 +46,8 @@
/**
* Return true if the conversion L=R+G+B is needed.
*/
-static GLboolean
-need_rgb_to_luminance_conversion(mesa_format texFormat, GLenum format)
+GLboolean
+_mesa_need_rgb_to_luminance_conversion(mesa_format texFormat, GLenum format)
{
GLenum baseTexFormat = _mesa_get_format_base_format(texFormat);
@@ -105,7 +105,7 @@ get_readpixels_transfer_ops(const struct gl_context *ctx,
mesa_format texFormat,
* have any effect anyway.
*/
if (_mesa_get_format_datatype(texFormat) == GL_UNSIGNED_NORMALIZED &&
- !need_rgb_to_luminance_conversion(texFormat, format)) {
+ !_mesa_need_rgb_to_luminance_conversion(texFormat, format)) {
transferOps &= ~IMAGE_CLAMP_BIT;
}
@@ -149,7 +149,7 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
*ctx, GLenum format,
default:
/* Color formats. */
- if (need_rgb_to_luminance_conversion(rb->Format, format)) {
+ if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format)) {
return GL_TRUE;
}
@@ -442,7 +442,8 @@ read_rgba_pixels( struct gl_context *ctx,
dst_is_integer = _mesa_is_enum_format_integer(format);
dst_stride = _mesa_image_row_stride(packing, width, format, type);
dst_format = _mesa_format_from_format_and_type(format, type);
- convert_rgb_to_lum = need_rgb_to_luminance_conversion(rb->Format, format);
+ convert_rgb_to_lum =
+ _mesa_need_rgb_to_luminance_conversion(rb->Format, format);
dst = (GLubyte *) _mesa_image_address2d(packing, pixels, width, height,
format, type, 0, 0);
diff --git a/src/mesa/main/readpix.h b/src/mesa/main/readpix.h
index 4bb35e1..1636dd9 100644
--- a/src/mesa/main/readpix.h
+++ b/src/mesa/main/readpix.h
@@ -37,6 +37,9 @@ extern GLboolean
_mesa_readpixels_needs_slow_path(const struct gl_context *ctx, GLenum format,
GLenum type, GLboolean uses_blit);
+extern GLboolean
+_mesa_need_rgb_to_luminance_conversion(mesa_format texFormat, GLenum format);
+
extern void
_mesa_readpixels(struct gl_context *ctx,
GLint x, GLint y, GLsizei width, GLsizei height,
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] mesa: Use helper function need_rgb_to_luminance_conversion()
Signed-off-by: Anuj Phogat
---
src/mesa/main/readpix.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index 9166a50..cba9db8 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -421,7 +421,7 @@ read_rgba_pixels( struct gl_context *ctx,
const struct gl_pixelstore_attrib *packing )
{
GLbitfield transferOps;
- bool dst_is_integer, dst_is_luminance, needs_rebase;
+ bool dst_is_integer, convert_rgb_to_lum, needs_rebase;
int dst_stride, src_stride, rb_stride;
uint32_t dst_format, src_format;
GLubyte *dst, *map;
@@ -442,10 +442,7 @@ read_rgba_pixels( struct gl_context *ctx,
dst_is_integer = _mesa_is_enum_format_integer(format);
dst_stride = _mesa_image_row_stride(packing, width, format, type);
dst_format = _mesa_format_from_format_and_type(format, type);
- dst_is_luminance = format == GL_LUMINANCE ||
- format == GL_LUMINANCE_ALPHA ||
- format == GL_LUMINANCE_INTEGER_EXT ||
- format == GL_LUMINANCE_ALPHA_INTEGER_EXT;
+ convert_rgb_to_lum = need_rgb_to_luminance_conversion(rb->Format, format);
dst = (GLubyte *) _mesa_image_address2d(packing, pixels, width, height,
format, type, 0, 0);
@@ -493,7 +490,7 @@ read_rgba_pixels( struct gl_context *ctx,
*/
assert(!transferOps || (transferOps && !dst_is_integer));
- needs_rgba = transferOps || dst_is_luminance;
+ needs_rgba = transferOps || convert_rgb_to_lum;
rgba = NULL;
if (needs_rgba) {
uint32_t rgba_format;
@@ -566,7 +563,7 @@ read_rgba_pixels( struct gl_context *ctx,
* If the dst format is Luminance, we need to do the conversion by computing
* L=R+G+B values.
*/
- if (!dst_is_luminance) {
+ if (!convert_rgb_to_lum) {
_mesa_format_convert(dst, dst_format, dst_stride,
src, src_format, src_stride,
width, height,
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] meta: Abort meta path if ReadPixels need rgb to luminance conversion
After recent addition of pbo testing in piglit test getteximage-luminance,
it fails on i965. This patch makes a sub test pass.
Signed-off-by: Anuj Phogat
Cc:
---
src/mesa/drivers/common/meta_tex_subimage.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
b/src/mesa/drivers/common/meta_tex_subimage.c
index ad6e787..65753f7 100644
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@ -34,6 +34,7 @@
#include "macros.h"
#include "meta.h"
#include "pbo.h"
+#include "readpix.h"
#include "shaderapi.h"
#include "state.h"
#include "teximage.h"
@@ -257,6 +258,7 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
GLuint pbo = 0, pbo_tex = 0, fbos[2] = { 0, 0 };
int full_height, image_height;
struct gl_texture_image *pbo_tex_image;
+ struct gl_renderbuffer *rb = NULL;
GLenum status;
bool success = false;
int z;
@@ -273,6 +275,12 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
if (ctx->_ImageTransferState)
return false;
+
+ if (!tex_image) {
+ if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format))
+ return false;
+ }
+
/* For arrays, use a tall (height * depth) 2D texture but taking into
* account the inter-image padding specified with the image height packing
* property.
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0.5/3] mesa: Handle integer formats in need_rgb_to_luminance_conversion()
Signed-off-by: Anuj Phogat --- src/mesa/main/readpix.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c index df46f83..9166a50 100644 --- a/src/mesa/main/readpix.c +++ b/src/mesa/main/readpix.c @@ -54,7 +54,10 @@ need_rgb_to_luminance_conversion(mesa_format texFormat, GLenum format) return (baseTexFormat == GL_RG || baseTexFormat == GL_RGB || baseTexFormat == GL_RGBA) && - (format == GL_LUMINANCE || format == GL_LUMINANCE_ALPHA); + (format == GL_LUMINANCE || + format == GL_LUMINANCE_ALPHA || + format == GL_LUMINANCE_INTEGER_EXT || + format == GL_LUMINANCE_ALPHA_INTEGER_EXT); } -- 1.9.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Use helper function need_rgb_to_luminance_conversion()
On Thu, Jun 11, 2015 at 2:44 AM, Tapani Pälli wrote:
>
>
> On 06/11/2015 02:54 AM, Anuj Phogat wrote:
>>
>> Signed-off-by: Anuj Phogat
>> ---
>> src/mesa/main/readpix.c | 11 ---
>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
>> index 9166a50..cba9db8 100644
>> --- a/src/mesa/main/readpix.c
>> +++ b/src/mesa/main/readpix.c
>> @@ -421,7 +421,7 @@ read_rgba_pixels( struct gl_context *ctx,
>> const struct gl_pixelstore_attrib *packing )
>> {
>> GLbitfield transferOps;
>> - bool dst_is_integer, dst_is_luminance, needs_rebase;
>> + bool dst_is_integer, convert_rgb_to_lum, needs_rebase;
>> int dst_stride, src_stride, rb_stride;
>> uint32_t dst_format, src_format;
>> GLubyte *dst, *map;
>> @@ -442,10 +442,7 @@ read_rgba_pixels( struct gl_context *ctx,
>> dst_is_integer = _mesa_is_enum_format_integer(format);
>> dst_stride = _mesa_image_row_stride(packing, width, format, type);
>> dst_format = _mesa_format_from_format_and_type(format, type);
>> - dst_is_luminance = format == GL_LUMINANCE ||
>> - format == GL_LUMINANCE_ALPHA ||
>> - format == GL_LUMINANCE_INTEGER_EXT ||
>> - format == GL_LUMINANCE_ALPHA_INTEGER_EXT;
>
>
>
> Is it ok to drop checks for GL_LUMINANCE_INTEGER_EXT and
> GL_LUMINANCE_ALPHA_INTEGER_EXT, these are not checked against in
> need_rgb_to_luminance_conversion function?
>
No, It's not ok. I missed a patch in this series. Sent it out as 0.5/3 of
this series.
>
>
>> + convert_rgb_to_lum = need_rgb_to_luminance_conversion(rb->Format,
>> format);
>> dst = (GLubyte *) _mesa_image_address2d(packing, pixels, width,
>> height,
>> format, type, 0, 0);
>>
>> @@ -493,7 +490,7 @@ read_rgba_pixels( struct gl_context *ctx,
>> */
>> assert(!transferOps || (transferOps && !dst_is_integer));
>>
>> - needs_rgba = transferOps || dst_is_luminance;
>> + needs_rgba = transferOps || convert_rgb_to_lum;
>> rgba = NULL;
>> if (needs_rgba) {
>> uint32_t rgba_format;
>> @@ -566,7 +563,7 @@ read_rgba_pixels( struct gl_context *ctx,
>> * If the dst format is Luminance, we need to do the conversion by
>> computing
>> * L=R+G+B values.
>> */
>> - if (!dst_is_luminance) {
>> + if (!convert_rgb_to_lum) {
>> _mesa_format_convert(dst, dst_format, dst_stride,
>> src, src_format, src_stride,
>> width, height,
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 3/3] meta: Abort meta path if ReadPixels need rgb to luminance conversion
After recent addition of pbo testing in piglit test getteximage-luminance,
it fails on i965. This patch makes a sub test pass.
Signed-off-by: Anuj Phogat
Cc:
Cc: Tapani Palli
---
src/mesa/drivers/common/meta_tex_subimage.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
b/src/mesa/drivers/common/meta_tex_subimage.c
index ad6e787..6bd74e1 100644
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@ -34,6 +34,7 @@
#include "macros.h"
#include "meta.h"
#include "pbo.h"
+#include "readpix.h"
#include "shaderapi.h"
#include "state.h"
#include "teximage.h"
@@ -257,6 +258,7 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
GLuint pbo = 0, pbo_tex = 0, fbos[2] = { 0, 0 };
int full_height, image_height;
struct gl_texture_image *pbo_tex_image;
+ struct gl_renderbuffer *rb = NULL;
GLenum status;
bool success = false;
int z;
@@ -273,6 +275,13 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
if (ctx->_ImageTransferState)
return false;
+
+ if (!tex_image) {
+ rb = ctx->ReadBuffer->_ColorReadBuffer;
+ if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format))
+ return false;
+ }
+
/* For arrays, use a tall (height * depth) 2D texture but taking into
* account the inter-image padding specified with the image height packing
* property.
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] meta: Abort meta path if ReadPixels need rgb to luminance conversion
On Thu, Jun 11, 2015 at 1:41 AM, Tapani Pälli wrote:
>
>
> On 06/11/2015 02:54 AM, Anuj Phogat wrote:
>>
>> After recent addition of pbo testing in piglit test getteximage-luminance,
>> it fails on i965. This patch makes a sub test pass.
>>
>> Signed-off-by: Anuj Phogat
>> Cc:
>> ---
>> src/mesa/drivers/common/meta_tex_subimage.c | 8
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
>> b/src/mesa/drivers/common/meta_tex_subimage.c
>> index ad6e787..65753f7 100644
>> --- a/src/mesa/drivers/common/meta_tex_subimage.c
>> +++ b/src/mesa/drivers/common/meta_tex_subimage.c
>> @@ -34,6 +34,7 @@
>> #include "macros.h"
>> #include "meta.h"
>> #include "pbo.h"
>> +#include "readpix.h"
>> #include "shaderapi.h"
>> #include "state.h"
>> #include "teximage.h"
>> @@ -257,6 +258,7 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
>> GLuint dims,
>> GLuint pbo = 0, pbo_tex = 0, fbos[2] = { 0, 0 };
>> int full_height, image_height;
>> struct gl_texture_image *pbo_tex_image;
>> + struct gl_renderbuffer *rb = NULL;
>> GLenum status;
>> bool success = false;
>> int z;
>> @@ -273,6 +275,12 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
>> GLuint dims,
>> if (ctx->_ImageTransferState)
>> return false;
>>
>> +
>> + if (!tex_image) {
>> + if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format))
>
>
> this will cause segfault as rb is set to NULL
>
Right. I have more patches fixing stuff in this function. And the rb
initialization went in some other patch. I'll fix it.
>
>> + return false;
>> + }
>> +
>> /* For arrays, use a tall (height * depth) 2D texture but taking into
>> * account the inter-image padding specified with the image height
>> packing
>> * property.
>>
>
>
> // Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] meta: Abort texture upload if pixels == null and no pixel unpack buffer set
in case of glTex{Sub}Image{1,2,3}D(). Texture has already been
allocated at this point and we have no data to upload. With out
this patch, with create_pbo = true, we end up creating a temporary
pbo and then uploading uninitialzed texture data.
Signed-off-by: Anuj Phogat
Cc: Neil Roberts
---
src/mesa/drivers/common/meta_tex_subimage.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
b/src/mesa/drivers/common/meta_tex_subimage.c
index e159127..d984a85 100644
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@ -153,7 +153,8 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint
dims,
bool success = false;
int z;
- if (!_mesa_is_bufferobj(packing->BufferObj) && !create_pbo)
+ if (!_mesa_is_bufferobj(packing->BufferObj) &&
+ (!create_pbo || pixels == NULL))
return false;
if (format == GL_DEPTH_COMPONENT ||
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0.5/3] mesa: Handle integer formats in need_rgb_to_luminance_conversion()
On Thu, Jun 11, 2015 at 10:05 AM, Tapani Pälli wrote: > Reviewed-by: Tapani Pälli > > > On 06/11/2015 07:58 PM, Anuj Phogat wrote: >> >> Signed-off-by: Anuj Phogat >> --- >> src/mesa/main/readpix.c | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c >> index df46f83..9166a50 100644 >> --- a/src/mesa/main/readpix.c >> +++ b/src/mesa/main/readpix.c >> @@ -54,7 +54,10 @@ need_rgb_to_luminance_conversion(mesa_format texFormat, >> GLenum format) >> return (baseTexFormat == GL_RG || >> baseTexFormat == GL_RGB || >> baseTexFormat == GL_RGBA) && >> - (format == GL_LUMINANCE || format == GL_LUMINANCE_ALPHA); >> + (format == GL_LUMINANCE || >> + format == GL_LUMINANCE_ALPHA || >> + format == GL_LUMINANCE_INTEGER_EXT || >> + format == GL_LUMINANCE_ALPHA_INTEGER_EXT); >> } >> > > A note to myself: I would like to change the signature of this function and just pass the base formats of src, dst. I will do that later in a separate patch. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] meta: Abort texture upload if pixels == null and no pixel unpack buffer set
On Fri, Jun 12, 2015 at 5:12 AM, Neil Roberts wrote:
> Ah, good catch. Looks good to me.
>
> Reviewed-by: Neil Roberts
>
> It seems a bit weird to use create_pbo=true at all for
> glTexImage{1,2,3}D because in that case we are completely replacing the
> texture. If the texture's buffer is busy instead of allocating a PBO we
> might as well just directly allocate some new storage for the texture
> and abandon the old storage. That would be a separate patch though and
> either way I think this patch makes sense.
>
> Anuj Phogat writes:
>
>> in case of glTex{Sub}Image{1,2,3}D().
>
> I think in practice this patch only applies to glTexImage* (not
> glTexSubImage*) doesn't it? It wouldn't make any sense to call
> glTexSubImage without a PBO and with NULL pixels.
>
Right.
> Regards,
> - Neil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Don't create a temp PBO when uploading data from glTexImage*
On Fri, Jun 12, 2015 at 7:34 AM, Neil Roberts wrote:
> Previously when glTexImage* is called it would attempt to create a
> temporary PBO if the texture is busy in order to avoid blocking when
> mapping the texture. This doesn't make much sense for glTexImage
> because in that case we are completely replacing the texture anyway so
> instead of allocating a PBO we can just allocate new storage for the
> texture.
>
> The code was buggy anyway because it was checking whether the buffer
> was busy before calling Driver->AllocTextureImageBuffer. That function
> actually always frees the buffer and recreates a new one so it was
> checking whether the previous buffer was busy and this is irrelevant.
>
> In practice I think this wouldn't matter too much because the upper
> layers of Mesa always call Driver->FreeTextureImageBuffer before
> calling Driver->TexImage anyway so there would never be a buffer that
> could be busy.
> ---
> src/mesa/drivers/dri/i965/intel_tex_image.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c
> b/src/mesa/drivers/dri/i965/intel_tex_image.c
> index 85d3d04..2874e5b 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -95,8 +95,6 @@ intelTexImage(struct gl_context * ctx,
> struct intel_texture_image *intelImage = intel_texture_image(texImage);
> bool ok;
>
> - bool tex_busy = intelImage->mt && drm_intel_bo_busy(intelImage->mt->bo);
> -
> DBG("%s mesa_format %s target %s format %s type %s level %d %dx%dx%d\n",
> __func__, _mesa_get_format_name(texImage->TexFormat),
> _mesa_lookup_enum_by_nr(texImage->TexObject->Target),
> @@ -116,7 +114,8 @@ intelTexImage(struct gl_context * ctx,
> texImage->Depth,
> format, type, pixels,
> false /*allocate_storage*/,
> - tex_busy, unpack);
> + false /*create_pbo*/,
> + unpack);
> if (ok)
>return;
>
> --
> 1.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
It makes sense.
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: fix compile error message
On Fri, Jun 12, 2015 at 3:30 AM, Timothy Arceri wrote:
> ---
> src/glsl/ast_to_hir.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 350f6ed..578711a 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -3644,7 +3644,7 @@ ast_declarator_list::hir(exec_list *instructions,
> if (check_type->is_record() || check_type->is_matrix())
> _mesa_glsl_error(&loc, state,
> "fragment shader output "
> -"cannot have struct or array type");
> +"cannot have struct or matrix type");
> switch (check_type->base_type) {
> case GLSL_TYPE_UINT:
> case GLSL_TYPE_INT:
> --
> 2.1.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 07/14] mesa: Set green, blue channels to zero only for formats with these components
Signed-off-by: Anuj Phogat --- src/mesa/drivers/common/meta.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c index 214a68a..fceb25d 100644 --- a/src/mesa/drivers/common/meta.c +++ b/src/mesa/drivers/common/meta.c @@ -3132,9 +3132,16 @@ decompress_texture_image(struct gl_context *ctx, * returned as red and two-channel texture values are returned as * red/alpha. */ - if ((baseTexFormat == GL_LUMINANCE || - baseTexFormat == GL_LUMINANCE_ALPHA || - baseTexFormat == GL_INTENSITY) || + if (((baseTexFormat == GL_LUMINANCE || +baseTexFormat == GL_LUMINANCE_ALPHA || +baseTexFormat == GL_INTENSITY) && + (destBaseFormat == GL_RGBA || +destBaseFormat == GL_RGB || +destBaseFormat == GL_RG || +destBaseFormat == GL_GREEN || +destBaseFormat == GL_BLUE || +destBaseFormat == GL_BGRA || +destBaseFormat == GL_BGR)) || /* If we're reading back an RGB(A) texture (using glGetTexImage) as * luminance then we need to return L=tex(R). */ -- 1.9.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 05/14] meta: Abort meta pbo path if readpixels need signed-unsigned conversion
Without this patch, piglit test fbo_integer_readpixels_sint_uint fails, when forced to use the meta pbo path. Signed-off-by: Anuj Phogat Cc: --- src/mesa/drivers/common/meta_tex_subimage.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/mesa/drivers/common/meta_tex_subimage.c b/src/mesa/drivers/common/meta_tex_subimage.c index 00364f8..84cbc50 100644 --- a/src/mesa/drivers/common/meta_tex_subimage.c +++ b/src/mesa/drivers/common/meta_tex_subimage.c @@ -283,6 +283,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, GLuint dims, if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format)) return false; + + if (_mesa_need_signed_unsigned_int_conversion(rb->Format, format, type)) + return false; } /* For arrays, use a tall (height * depth) 2D texture but taking into -- 1.9.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 09/14] meta: Use _mesa_unpack_format_to_base_format() to handle integer formats
_mesa_base_tex_format() doesn't handle GL_*_INTEGER formats.
Signed-off-by: Anuj Phogat
---
src/mesa/drivers/common/meta.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
index fceb25d..c9e58d8 100644
--- a/src/mesa/drivers/common/meta.c
+++ b/src/mesa/drivers/common/meta.c
@@ -3123,7 +3123,7 @@ decompress_texture_image(struct gl_context *ctx,
/* read pixels from renderbuffer */
{
GLenum baseTexFormat = texImage->_BaseFormat;
- GLenum destBaseFormat = _mesa_base_tex_format(ctx, destFormat);
+ GLenum destBaseFormat = _mesa_unpack_format_to_base_format(destFormat);
/* The pixel transfer state will be set to default values at this point
* (see MESA_META_PIXEL_TRANSFER) so pixel transfer ops are effectively
@@ -3149,9 +3149,7 @@ decompress_texture_image(struct gl_context *ctx,
baseTexFormat == GL_RGB ||
baseTexFormat == GL_RG) &&
(destBaseFormat == GL_LUMINANCE ||
- destBaseFormat == GL_LUMINANCE_ALPHA ||
- destBaseFormat == GL_LUMINANCE_INTEGER_EXT ||
- destBaseFormat == GL_LUMINANCE_ALPHA_INTEGER_EXT))) {
+ destBaseFormat == GL_LUMINANCE_ALPHA))) {
/* Green and blue must be zero */
_mesa_PixelTransferf(GL_GREEN_SCALE, 0.0f);
_mesa_PixelTransferf(GL_BLUE_SCALE, 0.0f);
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 10/14] mesa: Add a helper function _mesa_need_luminance_to_rgb_conversion()
Signed-off-by: Anuj Phogat
Cc:
---
src/mesa/main/readpix.c | 18 ++
src/mesa/main/readpix.h | 4
2 files changed, 22 insertions(+)
diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index 1038983..c98975f 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -60,6 +60,24 @@ _mesa_need_rgb_to_luminance_conversion(mesa_format
texFormat, GLenum format)
format == GL_LUMINANCE_ALPHA_INTEGER_EXT);
}
+/**
+ * Return true if the conversion L,I to RGB conversion is needed.
+ */
+GLboolean
+_mesa_need_luminance_to_rgb_conversion(GLenum srcBaseFormat,
+ GLenum dstBaseFormat)
+{
+ return (srcBaseFormat == GL_LUMINANCE ||
+ srcBaseFormat == GL_LUMINANCE_ALPHA ||
+ srcBaseFormat == GL_INTENSITY) &&
+ (dstBaseFormat == GL_GREEN ||
+ dstBaseFormat == GL_BLUE ||
+ dstBaseFormat == GL_RG ||
+ dstBaseFormat == GL_RGB ||
+ dstBaseFormat == GL_BGR ||
+ dstBaseFormat == GL_RGBA ||
+ dstBaseFormat == GL_BGRA);
+}
/**
* Return transfer op flags for this ReadPixels operation.
diff --git a/src/mesa/main/readpix.h b/src/mesa/main/readpix.h
index a93e263..b59decd 100644
--- a/src/mesa/main/readpix.h
+++ b/src/mesa/main/readpix.h
@@ -40,6 +40,10 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
*ctx, GLenum format,
extern GLboolean
_mesa_need_rgb_to_luminance_conversion(mesa_format texFormat, GLenum format);
+extern GLboolean
+_mesa_need_luminance_to_rgb_conversion(GLenum srcBaseFormat,
+ GLenum dstBaseFormat);
+
extern GLbitfield
_mesa_get_readpixels_transfer_ops(const struct gl_context *ctx,
mesa_format texFormat,
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 12/14] meta: Fix reading luminance texture as rgba in _mesa_meta_pbo_GetTexSubImage()
After recent addition of pbo testing in piglit test getteximage-luminance,
it fails on i965. This patch makes a sub test pass.
This patch adds additional clear color operation which I think is better
than falling back to software path.
Signed-off-by: Anuj Phogat
Cc:
---
src/mesa/drivers/common/meta_tex_subimage.c | 35 +++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
b/src/mesa/drivers/common/meta_tex_subimage.c
index ccb7dfb..6d52014 100644
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@ -28,6 +28,7 @@
#include "blend.h"
#include "bufferobj.h"
#include "buffers.h"
+#include "clear.h"
#include "fbobject.h"
#include "glformats.h"
#include "glheader.h"
@@ -261,8 +262,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
int full_height, image_height;
struct gl_texture_image *pbo_tex_image;
struct gl_renderbuffer *rb = NULL;
- GLenum status;
- bool success = false;
+ GLenum status, src_base_format;
+ bool success = false, clear_channels_to_zero = false;
+ float save_clear_color[4];
int z;
if (!_mesa_is_bufferobj(packing->BufferObj))
@@ -355,6 +357,26 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
GL_COLOR_BUFFER_BIT, GL_NEAREST))
goto fail;
+ src_base_format = tex_image ?
+ tex_image->_BaseFormat :
+ ctx->ReadBuffer->_ColorReadBuffer->_BaseFormat;
+
+ /* Depending on the base formats involved we might need to rebase some
+* values. For example if we download from a Luminance format to RGBA
+* format, we want G=0 and B=0.
+*/
+ clear_channels_to_zero =
+ _mesa_need_luminance_to_rgb_conversion(src_base_format,
+ pbo_tex_image->_BaseFormat);
+
+ if (clear_channels_to_zero) {
+ memcpy(save_clear_color, ctx->Color.ClearColor.f, 4 * sizeof(float));
+ /* Clear the Green, Blue channels. */
+ _mesa_ColorMask(GL_FALSE, GL_TRUE, GL_TRUE, GL_FALSE);
+ _mesa_ClearColor(0.0, 0.0, 0.0, 1.0);
+ _mesa_Clear(GL_COLOR_BUFFER_BIT);
+ }
+
for (z = 1; z < depth; z++) {
_mesa_meta_bind_fbo_image(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
tex_image, zoffset + z);
@@ -367,6 +389,15 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
0, z * image_height,
width, z * image_height + height,
GL_COLOR_BUFFER_BIT, GL_NEAREST);
+ if (clear_channels_to_zero)
+ _mesa_Clear(GL_COLOR_BUFFER_BIT);
+ }
+
+ /* Unmask the color channels and restore the saved clear color values. */
+ if (clear_channels_to_zero) {
+ _mesa_ColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
+ _mesa_ClearColor(save_clear_color[0], save_clear_color[1],
+ save_clear_color[2], save_clear_color[3]);
}
success = true;
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 01/14] mesa: Turn get_readpixels_transfer_ops() in to a global function
This utility function is utilized in a later patch.
Signed-off-by: Anuj Phogat
Cc:
---
Jenkins showed no piglit regressions with this series.
src/mesa/main/readpix.c | 14 --
src/mesa/main/readpix.h | 6 ++
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index a3357cd..caa2648 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -64,9 +64,11 @@ _mesa_need_rgb_to_luminance_conversion(mesa_format
texFormat, GLenum format)
/**
* Return transfer op flags for this ReadPixels operation.
*/
-static GLbitfield
-get_readpixels_transfer_ops(const struct gl_context *ctx, mesa_format
texFormat,
-GLenum format, GLenum type, GLboolean uses_blit)
+GLbitfield
+_mesa_get_readpixels_transfer_ops(const struct gl_context *ctx,
+ mesa_format texFormat,
+ GLenum format, GLenum type,
+ GLboolean uses_blit)
{
GLbitfield transferOps = ctx->_ImageTransferState;
@@ -169,7 +171,7 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
*ctx, GLenum format,
}
/* And finally, see if there are any transfer ops. */
- return get_readpixels_transfer_ops(ctx, rb->Format, format, type,
+ return _mesa_get_readpixels_transfer_ops(ctx, rb->Format, format, type,
uses_blit) != 0;
}
return GL_FALSE;
@@ -436,8 +438,8 @@ read_rgba_pixels( struct gl_context *ctx,
if (!rb)
return;
- transferOps = get_readpixels_transfer_ops(ctx, rb->Format, format, type,
- GL_FALSE);
+ transferOps = _mesa_get_readpixels_transfer_ops(ctx, rb->Format, format,
+ type, GL_FALSE);
/* Describe the dst format */
dst_is_integer = _mesa_is_enum_format_integer(format);
dst_stride = _mesa_image_row_stride(packing, width, format, type);
diff --git a/src/mesa/main/readpix.h b/src/mesa/main/readpix.h
index 1636dd9..f894036 100644
--- a/src/mesa/main/readpix.h
+++ b/src/mesa/main/readpix.h
@@ -40,6 +40,12 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
*ctx, GLenum format,
extern GLboolean
_mesa_need_rgb_to_luminance_conversion(mesa_format texFormat, GLenum format);
+extern GLbitfield
+_mesa_get_readpixels_transfer_ops(const struct gl_context *ctx,
+ mesa_format texFormat,
+ GLenum format, GLenum type,
+ GLboolean uses_blit);
+
extern void
_mesa_readpixels(struct gl_context *ctx,
GLint x, GLint y, GLsizei width, GLsizei height,
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 14/14] meta: Use _mesa_need_rgb_to_luminance_conversion() in decompress_texture_image()
Signed-off-by: Anuj Phogat
---
src/mesa/drivers/common/meta.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
index 6108d98..e123500 100644
--- a/src/mesa/drivers/common/meta.c
+++ b/src/mesa/drivers/common/meta.c
@@ -3137,11 +3137,8 @@ decompress_texture_image(struct gl_context *ctx,
/* If we're reading back an RGB(A) texture (using glGetTexImage) as
* luminance then we need to return L=tex(R).
*/
- ((baseTexFormat == GL_RGBA ||
-baseTexFormat == GL_RGB ||
-baseTexFormat == GL_RG) &&
- (destBaseFormat == GL_LUMINANCE ||
- destBaseFormat == GL_LUMINANCE_ALPHA))) {
+ _mesa_need_rgb_to_luminance_conversion(baseTexFormat,
+ destBaseFormat)) {
/* Green and blue must be zero */
_mesa_PixelTransferf(GL_GREEN_SCALE, 0.0f);
_mesa_PixelTransferf(GL_BLUE_SCALE, 0.0f);
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/14] meta: Don't do fragment color clamping in case of ReadPixels
Without this patch, piglit test arb_color_buffer_float-readpixels
fails, when forced to use the meta pbo path.
Signed-off-by: Anuj Phogat
Cc:
---
src/mesa/drivers/common/meta_tex_subimage.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
b/src/mesa/drivers/common/meta_tex_subimage.c
index 84cbc50..ccb7dfb 100644
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@ -25,6 +25,7 @@
*Jason Ekstrand
*/
+#include "blend.h"
#include "bufferobj.h"
#include "buffers.h"
#include "fbobject.h"
@@ -305,6 +306,10 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
_mesa_meta_begin(ctx, ~(MESA_META_PIXEL_TRANSFER |
MESA_META_PIXEL_STORE));
+ /* GL_CLAMP_FRAGMENT_COLOR doesn't affect ReadPixels. */
+ if (!tex_image && ctx->Extensions.ARB_color_buffer_float)
+ _mesa_ClampColor(GL_CLAMP_FRAGMENT_COLOR, GL_FALSE);
+
_mesa_GenFramebuffers(2, fbos);
if (tex_image && tex_image->TexObject->Target == GL_TEXTURE_1D_ARRAY) {
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 08/14] mesa: Add a helper function _mesa_unpack_format_to_base_format()
Signed-off-by: Anuj Phogat
---
src/mesa/main/glformats.c | 44
src/mesa/main/glformats.h | 3 +++
2 files changed, 47 insertions(+)
diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
index ac69fab..cac243c 100644
--- a/src/mesa/main/glformats.c
+++ b/src/mesa/main/glformats.c
@@ -1278,6 +1278,50 @@ _mesa_is_compressed_format(const struct gl_context *ctx,
GLenum format)
}
}
+/**
+ * Convert various unpack formats to the corresponding base format.
+ */
+GLenum
+_mesa_unpack_format_to_base_format(GLenum format)
+{
+ switch(format) {
+ case GL_RED_INTEGER:
+ return GL_RED;
+ case GL_GREEN_INTEGER:
+ return GL_GREEN;
+ case GL_BLUE_INTEGER:
+ return GL_BLUE;
+ case GL_RG_INTEGER:
+ return GL_RG;
+ case GL_RGB_INTEGER:
+ return GL_RGB;
+ case GL_RGBA_INTEGER:
+ return GL_RGBA;
+ case GL_BGR_INTEGER:
+ return GL_BGR;
+ case GL_BGRA_INTEGER:
+ return GL_BGRA;
+ case GL_ALPHA_INTEGER:
+ return GL_ALPHA;
+ case GL_LUMINANCE_INTEGER_EXT:
+ return GL_LUMINANCE;
+ case GL_LUMINANCE_ALPHA_INTEGER_EXT:
+ return GL_LUMINANCE_ALPHA;
+ case GL_RED:
+ case GL_GREEN:
+ case GL_BLUE:
+ case GL_RG:
+ case GL_RGB:
+ case GL_RGBA:
+ case GL_BGR:
+ case GL_BGRA:
+ case GL_ALPHA:
+ case GL_LUMINANCE:
+ case GL_LUMINANCE_ALPHA:
+ default:
+ return format;
+ }
+}
/**
* Convert various base formats to the cooresponding integer format.
diff --git a/src/mesa/main/glformats.h b/src/mesa/main/glformats.h
index 8881cb7..419955a 100644
--- a/src/mesa/main/glformats.h
+++ b/src/mesa/main/glformats.h
@@ -101,6 +101,9 @@ _mesa_is_compressed_format(const struct gl_context *ctx,
GLenum format);
extern GLenum
_mesa_base_format_to_integer_format(GLenum format);
+extern GLenum
+_mesa_unpack_format_to_base_format(GLenum format);
+
extern GLboolean
_mesa_base_format_has_channel(GLenum base_format, GLenum pname);
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/14] mesa: Fix conditions to test signed, unsigned integer format
Signed-off-by: Anuj Phogat
Cc:
---
src/mesa/main/readpix.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index caa2648..a9416ef 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -160,10 +160,12 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
*ctx, GLenum format,
srcType = _mesa_get_format_datatype(rb->Format);
if ((srcType == GL_INT &&
+ _mesa_is_enum_format_integer(format) &&
(type == GL_UNSIGNED_INT ||
type == GL_UNSIGNED_SHORT ||
type == GL_UNSIGNED_BYTE)) ||
(srcType == GL_UNSIGNED_INT &&
+ _mesa_is_enum_format_integer(format) &&
(type == GL_INT ||
type == GL_SHORT ||
type == GL_BYTE))) {
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 13/14] mesa: Change the signature of _mesa_need_rgb_to_luminance_conversion()
This allows us to handle cases when texImage->_BaseFormat doesn't match
_mesa_format_get_base_format(texImage->Format). _BaseFormat is what we
care about in this function.
Signed-off-by: Anuj Phogat
---
src/mesa/drivers/common/meta_tex_subimage.c | 4 +++-
src/mesa/main/readpix.c | 28 +++-
src/mesa/main/readpix.h | 3 ++-
3 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
b/src/mesa/drivers/common/meta_tex_subimage.c
index 6d52014..43e1210 100644
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@ -262,6 +262,7 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
int full_height, image_height;
struct gl_texture_image *pbo_tex_image;
struct gl_renderbuffer *rb = NULL;
+ GLenum dstBaseFormat = _mesa_unpack_format_to_base_format(format);
GLenum status, base_format;
bool success = false, clear_channels_to_zero = false;
float save_clear_color[4];
@@ -284,7 +285,8 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
type, GL_FALSE))
return false;
- if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format))
+ if (_mesa_need_rgb_to_luminance_conversion(rb->_BaseFormat,
+ dstBaseFormat))
return false;
if (_mesa_need_signed_unsigned_int_conversion(rb->Format, format, type))
diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index c98975f..3a9b766 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -47,17 +47,14 @@
* Return true if the conversion L=R+G+B is needed.
*/
GLboolean
-_mesa_need_rgb_to_luminance_conversion(mesa_format texFormat, GLenum format)
+_mesa_need_rgb_to_luminance_conversion(GLenum srcBaseFormat,
+ GLenum dstBaseFormat)
{
- GLenum baseTexFormat = _mesa_get_format_base_format(texFormat);
-
- return (baseTexFormat == GL_RG ||
- baseTexFormat == GL_RGB ||
- baseTexFormat == GL_RGBA) &&
- (format == GL_LUMINANCE ||
- format == GL_LUMINANCE_ALPHA ||
- format == GL_LUMINANCE_INTEGER_EXT ||
- format == GL_LUMINANCE_ALPHA_INTEGER_EXT);
+ return (srcBaseFormat == GL_RG ||
+ srcBaseFormat == GL_RGB ||
+ srcBaseFormat == GL_RGBA) &&
+ (dstBaseFormat == GL_LUMINANCE ||
+ dstBaseFormat == GL_LUMINANCE_ALPHA);
}
/**
@@ -89,6 +86,8 @@ _mesa_get_readpixels_transfer_ops(const struct gl_context
*ctx,
GLboolean uses_blit)
{
GLbitfield transferOps = ctx->_ImageTransferState;
+ GLenum srcBaseFormat = _mesa_get_format_base_format(texFormat);
+ GLenum dstBaseFormat = _mesa_unpack_format_to_base_format(format);
if (format == GL_DEPTH_COMPONENT ||
format == GL_DEPTH_STENCIL ||
@@ -125,7 +124,7 @@ _mesa_get_readpixels_transfer_ops(const struct gl_context
*ctx,
* have any effect anyway.
*/
if (_mesa_get_format_datatype(texFormat) == GL_UNSIGNED_NORMALIZED &&
- !_mesa_need_rgb_to_luminance_conversion(texFormat, format)) {
+ !_mesa_need_rgb_to_luminance_conversion(srcBaseFormat, dstBaseFormat)) {
transferOps &= ~IMAGE_CLAMP_BIT;
}
@@ -164,6 +163,7 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
*ctx, GLenum format,
{
struct gl_renderbuffer *rb =
_mesa_get_read_renderbuffer_for_format(ctx, format);
+ GLenum dstBaseFormat = _mesa_unpack_format_to_base_format(format);
assert(rb);
@@ -184,7 +184,8 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
*ctx, GLenum format,
default:
/* Color formats. */
- if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format)) {
+ if (_mesa_need_rgb_to_luminance_conversion(rb->_BaseFormat,
+ dstBaseFormat)) {
return GL_TRUE;
}
@@ -458,6 +459,7 @@ read_rgba_pixels( struct gl_context *ctx,
uint8_t rebase_swizzle[4];
struct gl_framebuffer *fb = ctx->ReadBuffer;
struct gl_renderbuffer *rb = fb->_ColorReadBuffer;
+ GLenum dstBaseFormat = _mesa_unpack_format_to_base_format(format);
if (!rb)
return;
@@ -469,7 +471,7 @@ read_rgba_pixels( struct gl_context *ctx,
dst_stride = _mesa_image_row_stride(packing, width, format, type);
dst_format = _mesa_format_from_format_and_type(format, type);
convert_rgb_to_lum =
- _mesa_need_rgb_to_luminance_conversion(rb->Format, format);
+ _mesa_need_rgb_to_luminance_conversion(rb->_BaseFormat, dstBaseFormat);
dst = (GLubyte *) _mesa_image_address2d(packing, pixels, width, height,
format, type, 0, 0);
diff --git a/src
[Mesa-dev] [PATCH 11/14] meta: Use _mesa_need_luminance_to_rgb_conversion() in decompress_texture_image()
Signed-off-by: Anuj Phogat --- src/mesa/drivers/common/meta.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c index c9e58d8..6108d98 100644 --- a/src/mesa/drivers/common/meta.c +++ b/src/mesa/drivers/common/meta.c @@ -3132,16 +3132,8 @@ decompress_texture_image(struct gl_context *ctx, * returned as red and two-channel texture values are returned as * red/alpha. */ - if (((baseTexFormat == GL_LUMINANCE || -baseTexFormat == GL_LUMINANCE_ALPHA || -baseTexFormat == GL_INTENSITY) && - (destBaseFormat == GL_RGBA || -destBaseFormat == GL_RGB || -destBaseFormat == GL_RG || -destBaseFormat == GL_GREEN || -destBaseFormat == GL_BLUE || -destBaseFormat == GL_BGRA || -destBaseFormat == GL_BGR)) || + if (_mesa_need_luminance_to_rgb_conversion(baseTexFormat, + destBaseFormat) || /* If we're reading back an RGB(A) texture (using glGetTexImage) as * luminance then we need to return L=tex(R). */ -- 1.9.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 04/14] mesa: Add a mesa utility function _mesa_need_signed_unsigned_int_conversion()
This utility function is used in a later patch.
Signed-off-by: Anuj Phogat
Cc:
---
src/mesa/main/readpix.c | 32 ++--
src/mesa/main/readpix.h | 4
2 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index a9416ef..1038983 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -114,6 +114,22 @@ _mesa_get_readpixels_transfer_ops(const struct gl_context
*ctx,
return transferOps;
}
+bool
+_mesa_need_signed_unsigned_int_conversion(mesa_format rbFormat,
+ GLenum format, GLenum type)
+{
+ const GLenum srcType = _mesa_get_format_datatype(rbFormat);
+ return (srcType == GL_INT &&
+ _mesa_is_enum_format_integer(format) &&
+ (type == GL_UNSIGNED_INT ||
+ type == GL_UNSIGNED_SHORT ||
+ type == GL_UNSIGNED_BYTE)) ||
+ (srcType == GL_UNSIGNED_INT &&
+ _mesa_is_enum_format_integer(format) &&
+ (type == GL_INT ||
+ type == GL_SHORT ||
+ type == GL_BYTE));
+}
/**
* Return true if memcpy cannot be used for ReadPixels.
@@ -130,7 +146,6 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
*ctx, GLenum format,
{
struct gl_renderbuffer *rb =
_mesa_get_read_renderbuffer_for_format(ctx, format);
- GLenum srcType;
assert(rb);
@@ -157,20 +172,9 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
*ctx, GLenum format,
/* Conversion between signed and unsigned integers needs masking
* (it isn't just memcpy). */
- srcType = _mesa_get_format_datatype(rb->Format);
-
- if ((srcType == GL_INT &&
- _mesa_is_enum_format_integer(format) &&
- (type == GL_UNSIGNED_INT ||
-type == GL_UNSIGNED_SHORT ||
-type == GL_UNSIGNED_BYTE)) ||
- (srcType == GL_UNSIGNED_INT &&
- _mesa_is_enum_format_integer(format) &&
- (type == GL_INT ||
-type == GL_SHORT ||
-type == GL_BYTE))) {
+ if (_mesa_need_signed_unsigned_int_conversion(rb->Format, format,
+ type))
return GL_TRUE;
- }
/* And finally, see if there are any transfer ops. */
return _mesa_get_readpixels_transfer_ops(ctx, rb->Format, format, type,
diff --git a/src/mesa/main/readpix.h b/src/mesa/main/readpix.h
index f894036..a93e263 100644
--- a/src/mesa/main/readpix.h
+++ b/src/mesa/main/readpix.h
@@ -46,6 +46,10 @@ _mesa_get_readpixels_transfer_ops(const struct gl_context
*ctx,
GLenum format, GLenum type,
GLboolean uses_blit);
+extern bool
+_mesa_need_signed_unsigned_int_conversion(mesa_format rbFormat,
+ GLenum format, GLenum type);
+
extern void
_mesa_readpixels(struct gl_context *ctx,
GLint x, GLint y, GLsizei width, GLsizei height,
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 02/14] meta: Fix transfer operations check in meta pbo path for readpixels
Without this patch, arb_color_buffer_float-readpixels test fails, when
forced to use meta pbo path.
Signed-off-by: Anuj Phogat
Cc:
---
src/mesa/drivers/common/meta_tex_subimage.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
b/src/mesa/drivers/common/meta_tex_subimage.c
index d2474f5..00364f8 100644
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@ -273,12 +273,14 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
format == GL_COLOR_INDEX)
return false;
- if (ctx->_ImageTransferState)
- return false;
-
-
+ /* Don't use meta path for readpixels in below conditions. */
if (!tex_image) {
rb = ctx->ReadBuffer->_ColorReadBuffer;
+
+ if (_mesa_get_readpixels_transfer_ops(ctx, rb->Format, format,
+type, GL_FALSE))
+ return false;
+
if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format))
return false;
}
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] What branch to get patch 47790
Adding Topi to Cc. On Tue, Jun 16, 2015 at 3:08 PM, Meng, David wrote: > Hi: > I am new to this email list. I would like to get a help from you. > > I found a patch with number of 47790 which supports Intel Broadwell(BDW) > system gen8 GPU. The author is Topi Pohjolainen. The description is in > below. > I need this patch to launch a virtual machine on BDW system in which we are > using Mesa library in user space. But I could not find this patch in mesa > master or any branches. Would you please pint me where I can find a branch > including this patch? > > I highly appreciate any help. > > Regards, > > David > patch title and > description- > [Mesa-dev] i965: Don't use gl-context for fbo-blits > This series introduces new blorp parameter type for blit programs > compiled from glsl-sources. For most parts the launch logic just > calls core i965 batch emission logic. > Vertex batches are handcrafted containing full vertex header > information. This is needed because the pipeline is programmed to > skip vertex shader, clip and viewport transformation in strips&fans > (SF) but to provide the vertices directly from vertex fetcher (VF) > to the windower (WM). > > Topi Pohjolainen (14): > i965/blorp/gen7: Support for loading glsl-based fragment shaders > i965/blorp/gen6: Support for loading glsl-based fragment shaders > meta: Provide read access to blit shaders > i965/meta: Add helper for looking up blit programs > i965/blorp: Add plumbing for glsl-based color blits > i965/blorp: Add support for loading vertices for glsl-based blits > i965/blorp: Add support for setting up surfaces for glsl-based blits > i965/blorp: Add support for setting samplers for glsl-based blits > i965/gen6: Add support for setting minimum layer for tex surfaces > i965/blorp: Enable glsl-based fbo blits > i965/blorp/gen7: Prepare re-using for gen8 > i965/blorp/gen7: Expose state setup applicable to gen8 > i965/blorp/gen6: Prepare vertex buffer setup logic for gen8 > i965/blorp/gen8: Execution support > > src/mesa/drivers/common/meta.c | 17 +- > src/mesa/drivers/common/meta.h | 5 +- > src/mesa/drivers/dri/i965/Makefile.sources | 1 + > src/mesa/drivers/dri/i965/brw_blorp.cpp | 21 +- > src/mesa/drivers/dri/i965/brw_blorp.h| 127 ++ > src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 90 + > src/mesa/drivers/dri/i965/brw_context.h | 10 + > src/mesa/drivers/dri/i965/brw_meta_util.c| 148 +++ > src/mesa/drivers/dri/i965/brw_meta_util.h| 9 + > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 5 +- > src/mesa/drivers/dri/i965/gen6_blorp.cpp | 280 - > src/mesa/drivers/dri/i965/gen7_blorp.cpp | 87 +++- > src/mesa/drivers/dri/i965/gen8_blorp.cpp | 494 > +++ > src/mesa/drivers/dri/i965/intel_fbo.c| 11 + > 14 files changed, 1273 insertions(+), 32 deletions(-) > create mode 100644 src/mesa/drivers/dri/i965/gen8_blorp.cpp > > ___ > 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
Re: [Mesa-dev] [PATCH] i965/skl: Fix aligning mt->total_width to the block size
On Tue, Jun 16, 2015 at 5:53 AM, Neil Roberts wrote:
> brw_miptree_layout_2d tries to ensure that mt->total_width is a
> multiple of the compressed block size, presumably because it wouldn't
> be possible to make an image that has a fraction of a block. However
> it was doing this by aligning mt->total_width to align_w. Previously
> align_w has been used as a shortcut for getting the block width
> because before Gen9 the block width was always equal to the alignment.
> Commit 4ab8d59a2 tried to fix these cases to use the block width
> instead of the alignment but it missed this case.
>
> I think in practice this probably won't make any difference because
> the buffer for the texture will be allocated to be large enough to
> contain the entire pitch and libdrm aligns the pitch to the tile width
> anyway. However I think the patch is worth having to make the
> intention clearer.
> ---
> src/mesa/drivers/dri/i965/brw_tex_layout.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> index 1e7d8a1..dbb6cef 100644
> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> @@ -366,9 +366,8 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
>
> mt->total_width = mt->physical_width0;
>
> - if (mt->compressed) {
> - mt->total_width = ALIGN(mt->physical_width0, mt->align_w);
> - }
> + if (mt->compressed)
> + mt->total_width = ALIGN(mt->total_width, bw);
>
> /* May need to adjust width to accommodate the placement of
> * the 2nd mipmap. This occurs when the alignment
> --
> 1.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/14] mesa: Fix conditions to test signed, unsigned integer format
On Tue, Jun 16, 2015 at 9:16 PM, Jason Ekstrand wrote:
> Please note in the commit message exactly what is broken.
>
I didn't notice any piglit failure without this change. As requested I'll add
this in the commit message:
"Just checking the type in glReadPixels() is not sufficient to decide if the
format is one of the integer formats. GL_INT, GL_SHORT or GL_BYTE types can
be used with both normalized fixed point formats and integer formats. So, add a
check to ensure the integer format."
> On Jun 16, 2015 11:15, "Anuj Phogat" wrote:
>>
>> Signed-off-by: Anuj Phogat
>> Cc:
>> ---
>> src/mesa/main/readpix.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
>> index caa2648..a9416ef 100644
>> --- a/src/mesa/main/readpix.c
>> +++ b/src/mesa/main/readpix.c
>> @@ -160,10 +160,12 @@ _mesa_readpixels_needs_slow_path(const struct
>> gl_context *ctx, GLenum format,
>>srcType = _mesa_get_format_datatype(rb->Format);
>>
>>if ((srcType == GL_INT &&
>> + _mesa_is_enum_format_integer(format) &&
>> (type == GL_UNSIGNED_INT ||
>> type == GL_UNSIGNED_SHORT ||
>> type == GL_UNSIGNED_BYTE)) ||
>>(srcType == GL_UNSIGNED_INT &&
>> + _mesa_is_enum_format_integer(format) &&
>> (type == GL_INT ||
>> type == GL_SHORT ||
>> type == GL_BYTE))) {
>> --
>> 1.9.3
>>
>> ___
>> 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
Re: [Mesa-dev] [PATCH 03/14] mesa: Fix conditions to test signed, unsigned integer format
On Thu, Jun 18, 2015 at 7:09 AM, Iago Toral wrote:
> On Tue, 2015-06-16 at 11:15 -0700, Anuj Phogat wrote:
>> Signed-off-by: Anuj Phogat
>> Cc:
>> ---
>> src/mesa/main/readpix.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
>> index caa2648..a9416ef 100644
>> --- a/src/mesa/main/readpix.c
>> +++ b/src/mesa/main/readpix.c
>> @@ -160,10 +160,12 @@ _mesa_readpixels_needs_slow_path(const struct
>> gl_context *ctx, GLenum format,
>>srcType = _mesa_get_format_datatype(rb->Format);
>>
>>if ((srcType == GL_INT &&
>> + _mesa_is_enum_format_integer(format) &&
>> (type == GL_UNSIGNED_INT ||
>> type == GL_UNSIGNED_SHORT ||
>> type == GL_UNSIGNED_BYTE)) ||
>>(srcType == GL_UNSIGNED_INT &&
>> + _mesa_is_enum_format_integer(format) &&
>> (type == GL_INT ||
>> type == GL_SHORT ||
>> type == GL_BYTE))) {
>
> As far as I understand this code we are trying to see if we can use
> memcpy to directly copy the contents of the framebuffer to the
> destination buffer. In that case, as long as the src/dst types have
> different sign we can't just use memcpy, right? In fact it looks like we
> might need to expand the checks to include the cases where srcType is
> GL_(UNSIGNED_)SHORT and GL_(UNSIGNED_)BYTE as well.
>
srcType returned by _mesa_get_format_datatype() is one of:
GL_UNSIGNED_NORMALIZED
GL_SIGNED_NORMALIZED
GL_UNSIGNED_INT
GL_INT
GL_FLOAT
So, the suggested checks for srcType are not required.
> That said, I think this code is not necessary with the call to
> _mesa_format_matches_format_and_type that we do immediately after this,
> since that will check that the framebuffer format exactly matches the
> destination format anyway, which is a much tighter check than this. In
> fact, a quick piglit run without these checks does not seem to break any
> tests on i965. Gallium uses these two functions in a slightly different
> way in st_cb_readpixels.c though, so I wonder if their use case requires
> these checks to exist in this function anyway.
>
> Iago
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] glsl: guard gl_NumSamples enablement on ARB_sample_shading
On Thu, Jun 18, 2015 at 4:08 PM, Ilia Mirkin wrote:
> gl_NumSamples should only be enabled when ARB_sample_shading is enabled.
>
> Signed-off-by: Ilia Mirkin
> ---
> src/glsl/builtin_variables.cpp | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
> index c52b252..a765d35 100644
> --- a/src/glsl/builtin_variables.cpp
> +++ b/src/glsl/builtin_variables.cpp
> @@ -764,7 +764,8 @@ builtin_variable_generator::generate_constants()
> void
> builtin_variable_generator::generate_uniforms()
> {
> - add_uniform(int_t, "gl_NumSamples");
> + if (state->is_version(400, 0) || state->ARB_sample_shading_enable)
> + add_uniform(int_t, "gl_NumSamples");
> add_uniform(type("gl_DepthRangeParameters"), "gl_DepthRange");
> add_uniform(array(vec4_t, VERT_ATTRIB_MAX), "gl_CurrentAttribVertMESA");
> add_uniform(array(vec4_t, VARYING_SLOT_MAX), "gl_CurrentAttribFragMESA");
> --
> 2.3.6
>
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] i965/gen9: Implement Push Constant Buffer workaround
On Wed, Jun 3, 2015 at 9:35 PM, Ben Widawsky
wrote:
> This implements a workaround (exact excerpt as a comment in the code). The
> docs
> specify [clearly, after you struggle for a while] that the offset isn't
> relative
> to state base. This actually makes sense.
>
> Buffer #0 is meant to be used for normal uniforms.
> Buffer #1 is typically used for gather constants when using RS.
> Buffer #1-#3 could be used to push a bunch of UBO data which would just be
> somewhere in memory, and not relative to the dynamic state.
>
> NOTE: I've moved away from the ternary operator for the new gen9 conditions.
> Admittedly it's probably not great to do this, but I really want to fix this
> all
> up in the subsequent patch and doing it here makes that diff a lot nicer. I
> want
> to split out the gen8/9 code to make the function a bit more readable, but to
> keep this easily cherry-pickable I am doing this fix first. If we decide not
> to
> merge the cleanup patch then I can revisit this.
>
> Anuj ran this on his SKL and said there were no fixes on regressions. There is
> some hope it fixes BXT issues.
>
> Cc: Imre Deak
> Cc: Neil Roberts
> Cc: Anuj Phogat
> Signed-off-by: Ben Widawsky
> ---
> src/mesa/drivers/dri/i965/gen7_vs_state.c | 48
> ++-
> 1 file changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen7_vs_state.c
> b/src/mesa/drivers/dri/i965/gen7_vs_state.c
> index 278b3ec..4b17d06 100644
> --- a/src/mesa/drivers/dri/i965/gen7_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_vs_state.c
> @@ -43,18 +43,52 @@ gen7_upload_constant_state(struct brw_context *brw,
> int dwords = brw->gen >= 8 ? 11 : 7;
> BEGIN_BATCH(dwords);
> OUT_BATCH(opcode << 16 | (dwords - 2));
> - OUT_BATCH(active ? stage_state->push_const_size : 0);
> - OUT_BATCH(0);
> +
> + /* Workaround for SKL+ (we use option #2 until we have a need for more
> +* constant buffers). This comes from the documentation for
> 3DSTATE_CONSTANT_*
> +*
> +* The driver must ensure The following case does not occur without a
> flush
> +* to the 3D engine: 3DSTATE_CONSTANT_* with buffer 3 read length equal to
> +* zero committed followed by a 3DSTATE_CONSTANT_* with buffer 0 read
> length
> +* not equal to zero committed. Possible ways to avoid this condition
> +* include:
> +* 1. always force buffer 3 to have a non zero read length
> +* 2. always force buffer 0 to a zero read length
> +*/
> + if (brw->gen >= 9 && active) {
> + OUT_BATCH(0);
> + OUT_BATCH(stage_state->push_const_size);
> + } else {
> + OUT_BATCH(active ? stage_state->push_const_size : 0);
> + OUT_BATCH(0);
> + }
> /* Pointer to the constant buffer. Covered by the set of state flags
> * from gen6_prepare_wm_contants
> */
> - OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0);
> - OUT_BATCH(0);
> - OUT_BATCH(0);
> - OUT_BATCH(0);
> - if (brw->gen >= 8) {
> + if (brw->gen >= 9 && active) {
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + /* XXX: When using buffers other than 0, you need to specify the
> + * graphics virtual address regardless of INSPM/debug bits
INSTPM
> + */
> + OUT_RELOC64(brw->batch.bo, I915_GEM_DOMAIN_RENDER, 0,
> + stage_state->push_const_offset);
>OUT_BATCH(0);
>OUT_BATCH(0);
> + } else if (brw->gen>= 8) {
> + OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + } else {
> + OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0);
> + OUT_BATCH(0);
>OUT_BATCH(0);
>OUT_BATCH(0);
> }
> --
> 2.4.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Verified with the spec. LGTM.
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/14] mesa: Fix conditions to test signed, unsigned integer format
On Thu, Jun 18, 2015 at 11:41 PM, Iago Toral wrote:
> On Thu, 2015-06-18 at 09:19 -0700, Anuj Phogat wrote:
>> On Thu, Jun 18, 2015 at 7:09 AM, Iago Toral wrote:
>> > On Tue, 2015-06-16 at 11:15 -0700, Anuj Phogat wrote:
>> >> Signed-off-by: Anuj Phogat
>> >> Cc:
>> >> ---
>> >> src/mesa/main/readpix.c | 2 ++
>> >> 1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
>> >> index caa2648..a9416ef 100644
>> >> --- a/src/mesa/main/readpix.c
>> >> +++ b/src/mesa/main/readpix.c
>> >> @@ -160,10 +160,12 @@ _mesa_readpixels_needs_slow_path(const struct
>> >> gl_context *ctx, GLenum format,
>> >>srcType = _mesa_get_format_datatype(rb->Format);
>> >>
>> >>if ((srcType == GL_INT &&
>> >> + _mesa_is_enum_format_integer(format) &&
>> >> (type == GL_UNSIGNED_INT ||
>> >> type == GL_UNSIGNED_SHORT ||
>> >> type == GL_UNSIGNED_BYTE)) ||
>> >>(srcType == GL_UNSIGNED_INT &&
>> >> + _mesa_is_enum_format_integer(format) &&
>> >> (type == GL_INT ||
>> >> type == GL_SHORT ||
>> >> type == GL_BYTE))) {
>> >
>> > As far as I understand this code we are trying to see if we can use
>> > memcpy to directly copy the contents of the framebuffer to the
>> > destination buffer. In that case, as long as the src/dst types have
>> > different sign we can't just use memcpy, right? In fact it looks like we
>> > might need to expand the checks to include the cases where srcType is
>> > GL_(UNSIGNED_)SHORT and GL_(UNSIGNED_)BYTE as well.
>> >
>> srcType returned by _mesa_get_format_datatype() is one of:
>> GL_UNSIGNED_NORMALIZED
>> GL_SIGNED_NORMALIZED
>> GL_UNSIGNED_INT
>> GL_INT
>> GL_FLOAT
>> So, the suggested checks for srcType are not required.
>
> Oh, right, although I think that does not invalidate my point: can we
> memcpy from a GL_UNSIGNED_NORMALIZED to a format with type GL_FLOAT or
> GL_SIGNED_NORMALIZED? It does not look like these checks here are
> thorough.
>
Helper function _mesa_need_signed_unsigned_int_conversion() is
meant to do the checks only for integer formats. May be add another
function to do the missing checks for other formats?
> In any case, that's beyond the point of your patch. Talking specifically
> about your patch: can we memcpy, for example, from a _signed_ integer
> format like MESA_FORMAT_R_SINT8 to an _unsigned_ format (integer or
> not)? I don't think we can, in which case your patch would not look
> correct to me.
>
Reading integer format to a non integer format is not allowed in
glReadPixels. That's why those cases are not relevant here and
we just check for integer formats. From ext_texture_integer:
"INVALID_OPERATON is generated by ReadPixels if is
an integer format and the color buffer is not an integer format, or
if is not an integer format and the color buffer is an
integer format."
> Also, as I said in my previous e-mail, I feel like these checks here do
> not add anything, at least when this function is called from
> readpixels_can_use_memcpy, since even if we return true here, we will
> later call _mesa_format_matches_format_and_type and that would check
> that the formats match anyway before going through the memcpy path. Even
> the other use of this function in Gallium would call
> _mesa_format_matches_format_and_type before it calls this... That's why
> I think we probably want to remove these checks from this function and
> rely on _mesa_format_matches_format_and_type exclusively to check
> allowed formats and types.
>
It does seem like calling _mesa_need_signed_unsigned_int_conversion()
is redundant in case of readpixels_can_use_memcpy(). Thanks for noticing
it. Feel free to submit a patch to remove the redundant check.
But, I still need _mesa_need_signed_unsigned_int_conversion() and changes
in this patch for one of my later patches fixing meta PBO path for glReadPixels.
Do you still have any concerns about this patch?
>> > That said, I think this code is not necessary with the call to
>> > _mesa_format_matches_format_and_type that we do immediately after this,
>> > since that will check that the framebuffer format exactly matches the
>> > destination format anyway, which is a much tighter check than this. In
>> > fact, a quick piglit run without these checks does not seem to break any
>> > tests on i965. Gallium uses these two functions in a slightly different
>> > way in st_cb_readpixels.c though, so I wonder if their use case requires
>> > these checks to exist in this function anyway.
>> >
>> > Iago
>> >
>>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/14] meta: Abort meta pbo path if readpixels need signed-unsigned conversion
On Tue, Jun 16, 2015 at 9:21 PM, Jason Ekstrand wrote: > > On Jun 16, 2015 11:15, "Anuj Phogat" wrote: >> >> Without this patch, piglit test fbo_integer_readpixels_sint_uint fails, >> when >> forced to use the meta pbo path. >> >> Signed-off-by: Anuj Phogat >> Cc: >> --- >> src/mesa/drivers/common/meta_tex_subimage.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/src/mesa/drivers/common/meta_tex_subimage.c >> b/src/mesa/drivers/common/meta_tex_subimage.c >> index 00364f8..84cbc50 100644 >> --- a/src/mesa/drivers/common/meta_tex_subimage.c >> +++ b/src/mesa/drivers/common/meta_tex_subimage.c >> @@ -283,6 +283,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, >> GLuint dims, >> >>if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format)) >> return false; >> + >> + if (_mesa_need_signed_unsigned_int_conversion(rb->Format, format, >> type)) >> + return false; > > Hrm... This seems fishy. Isn't glBlitFramebuffers supposed to handle format > conversion with integers? If so we should probably fix it rather than just > skip it for the meta pbo path. > As discussed offline, here is relevant text for glBlitFrameBuffer() from OpenGL 4.5 spec, section 18.3.1: "An INVALID_OPERATION error is generated if format conversions are not supported, which occurs under any of the following conditions: -The read buffer contains fixed-point or floating-point values and any draw buffer contains neither fixed-point nor floating-point values. -The read buffer contains unsigned integer values and any draw buffer does not contain unsigned integer values. - The read buffer contains signed integer values and any draw buffer does not contain signed integer values." I'll add a comment here explaining the reason to avoid meta path. >> } >> >> /* For arrays, use a tall (height * depth) 2D texture but taking into >> -- >> 1.9.3 >> >> ___ >> 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
Re: [Mesa-dev] [PATCH] i965/gen8: Use HALIGN_16 for single sample mcs buffers
On Fri, Jun 19, 2015 at 10:05 AM, Mark Janes wrote:
> Tested-by: Mark Janes
>
> Ben Widawsky writes:
>
>> The original code meant to do this, but was only checking num_samples == 1 to
>> figure out if a surface was fast clear capable. However, we can allocate
>> single
>> sample miptrees with num_samples == 0 (when it's an internally created
>> buffer).
>>
>> This fixes a bunch of the piglit tests on gen8. Other gens should have been
>> fine.
>>
>> Here is the order of events that allowed this to slip through:
>> t0: I wrote halign patches and tested them. These alignment assertions are
>> for
>>gen8 fast clear surfaces, basically.
>> t1: I pushed bogus perf patch which made fast clears never happen
>> t2: Reworked halign patches based on Chad's feedback and introduced the bug
>> this
>>patch fixes.
>> t2.5: I tested reworked patches, but assertion wasn't hit because of t1.
>> t3. Matt fixed issue in t1 which made fast clears happen here:
>> commit 22af95af8316f2888a3935cdf774ff0997b3dd42
>> Author: Matt Turner
>> Date: Thu Jun 18 16:14:50 2015 -0700
>>
>> i965: Add missing braces around if-statement.
>>
>> This logic should match that of the v1 of my halign patch series.
>>
>> Cc: Kenneth Graunke
>> Cc: Matt Turner
>> Reported-by: Kenneth Graunke
>> Signed-off-by: Ben Widawsky
>> ---
>> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> index 80c52f2..6aa969a 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> @@ -501,7 +501,7 @@ intel_miptree_create_layout(struct brw_context *brw,
>> * 6 | ? |?
>> */
>> if (intel_miptree_is_fast_clear_capable(brw, mt)) {
>> - if (brw->gen >= 9 || (brw->gen == 8 && num_samples == 1))
>> + if (brw->gen >= 9 || (brw->gen == 8 && num_samples <= 1))
>> layout_flags |= MIPTREE_LAYOUT_FORCE_HALIGN16;
>> } else if (brw->gen >= 9 && num_samples > 1) {
>>layout_flags |= MIPTREE_LAYOUT_FORCE_HALIGN16;
>> --
>> 2.4.4
>>
>> ___
>> 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
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/5] i965/gen9: Add XY_FAST_COPY_BLT support to intelEmitCopyBlit()
On Wed, Jun 10, 2015 at 3:34 PM, Anuj Phogat wrote:
> This patch enables using XY_FAST_COPY_BLT only for Yf/Ys tiled buffers.
> It can be later turned on for other tiling patterns (X,Y) too.
>
> V3: Flush in between sequential fast copy blits.
> Fix src/dst alignment requirements.
> Make can_fast_copy_blit() helper.
> Use ffs(), is_power_of_two()
> Move overlap computation inside intel_miptree_blit().
>
> V4: Use _mesa_regions_overlap() function.
> Simplify horizontal and vertical alignment computations.
>
> Signed-off-by: Anuj Phogat
> Cc: Ben Widawsky
> ---
> src/mesa/drivers/dri/i965/intel_blit.c | 295
> ++-
> src/mesa/drivers/dri/i965/intel_blit.h | 2 +
> src/mesa/drivers/dri/i965/intel_copy_image.c | 2 +
> src/mesa/drivers/dri/i965/intel_reg.h| 16 ++
> 4 files changed, 268 insertions(+), 47 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
> b/src/mesa/drivers/dri/i965/intel_blit.c
> index 5afc771..800ed7e 100644
> --- a/src/mesa/drivers/dri/i965/intel_blit.c
> +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> @@ -27,6 +27,7 @@
>
>
> #include "main/mtypes.h"
> +#include "main/blit.h"
> #include "main/context.h"
> #include "main/enums.h"
> #include "main/colormac.h"
> @@ -43,6 +44,23 @@
>
> #define FILE_DEBUG_FLAG DEBUG_BLIT
>
> +#define SET_TILING_XY_FAST_COPY_BLT(tiling, tr_mode, type) \
> +({ \
> + switch (tiling) { \
> + case I915_TILING_X: \
> + CMD |= type ## _TILED_X; \
> + break; \
> + case I915_TILING_Y: \
> + if (tr_mode == INTEL_MIPTREE_TRMODE_YS)\
> + CMD |= type ## _TILED_64K; \
> + else \
> + CMD |= type ## _TILED_Y;\
> + break; \
> + default: \
> + unreachable("not reached");\
> + } \
> +})
> +
> static void
> intel_miptree_set_alpha_to_one(struct brw_context *brw,
> struct intel_mipmap_tree *mt,
> @@ -75,6 +93,10 @@ static uint32_t
> br13_for_cpp(int cpp)
> {
> switch (cpp) {
> + case 16:
> + return BR13_32323232;
> + case 8:
> + return BR13_16161616;
> case 4:
>return BR13_;
>break;
> @@ -89,6 +111,66 @@ br13_for_cpp(int cpp)
> }
> }
>
> +static uint32_t
> +get_tr_horizontal_align(uint32_t tr_mode, uint32_t cpp, bool is_src) {
> + /* Alignment tables for YF/YS tiled surfaces. */
> + const uint32_t align_2d_yf[] = {64, 64, 32, 32, 16};
> + const uint32_t align_2d_ys[] = {256, 256, 128, 128, 64};
> + const uint32_t bpp = cpp * 8;
> + const uint32_t shift = is_src ? 17 : 10;
> + uint32_t align;
> + int i = 0;
> +
> + if (tr_mode == INTEL_MIPTREE_TRMODE_NONE)
> + return 0;
> +
> + /* Compute array index. */
> + assert (bpp >= 8 && bpp <= 128 && is_power_of_two(bpp));
> + i = ffs(bpp / 8) - 1;
> +
> + align = tr_mode == INTEL_MIPTREE_TRMODE_YF ?
> + align_2d_yf[i] :
> + align_2d_ys[i];
> +
> + assert(is_power_of_two(align));
> +
> + /* XY_FAST_COPY_BLT doesn't support horizontal alignment of 16. */
> + if (align == 16)
> + align = 32;
> +
> + return (ffs(align) - 6) << shift;
> +}
> +
> +static uint32_t
> +get_tr_vertical_align(uint32_t tr_mode, uint32_t cpp, bool is_src) {
> + /* Vertical alignment tables for YF/YS tiled surfaces. */
> + const unsigned align_2d_yf[] = {64, 32, 32, 16, 16};
> + const unsigned align_2d_ys[] = {256, 128, 128, 64, 64};
> + const uint32_t bpp = cpp * 8;
> + const uint32_t shift = is_src ? 15 : 8;
> + uint32_t align;
> + int i = 0;
> +
> + if (tr_mode == INTEL_MIPTREE_TRMODE_NONE)
> + return 0;
> +
> + /* Compute array index. */
> + assert (bpp >= 8 && bpp <= 128 && is_power_of_two(bpp));
> + i = ffs(bpp / 8) - 1;
> +
> + align = tr_mode == INTEL_MIPTREE_TRMODE_YF ?
> +
Re: [Mesa-dev] [PATCH 02/14] meta: Fix transfer operations check in meta pbo path for readpixels
On Thu, Jun 18, 2015 at 5:26 AM, Iago Toral wrote:
> On Tue, 2015-06-16 at 11:15 -0700, Anuj Phogat wrote:
>> Without this patch, arb_color_buffer_float-readpixels test fails, when
>> forced to use meta pbo path.
>>
>> Signed-off-by: Anuj Phogat
>> Cc:
>> ---
>> src/mesa/drivers/common/meta_tex_subimage.c | 10 ++
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
>> b/src/mesa/drivers/common/meta_tex_subimage.c
>> index d2474f5..00364f8 100644
>> --- a/src/mesa/drivers/common/meta_tex_subimage.c
>> +++ b/src/mesa/drivers/common/meta_tex_subimage.c
>> @@ -273,12 +273,14 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
>> GLuint dims,
>> format == GL_COLOR_INDEX)
>>return false;
>>
>> - if (ctx->_ImageTransferState)
>> - return false;
>> -
>> -
>
> That test uses glReadPixels so it should call this with tex_image set to
> NULL and it should flow through the if you have below. The call to
> _mesa_get_readpixels_transfer_ops that you add below looks like it does
> what we want for a pixel read from a framebuffer instead of simply
> checking ctx->_ImageTransferState directly. I suppose this is what fixes
> the test, right?
>
Right. Using _mesa_get_readpixels_transfer_ops() in place of simple
ctx->_ImageTransferState check takes care of setting IMAGE_CLAMP_BIT
in ctx->_ImageTransferState when GL_CLAMP_READ_COLOR is set.
I'll add some explanation in commit message.
> The patch also removes the ctx->_ImageTransferState check for the case
> where we are reading from a real texture (tex_image != NULL), that seems
> unrelated to fixing arb_color_buffer_float-readpixels... Looking at the
> texture read code from getteximage.c it seems like this should be fine
> since that file does not seem to use that field for anything either, so
> I guess the check might not valid in this case.
>
After another look I realized that glGetTexImage() do need transfer
operations check to fall back to software path. It's the lack of piglit
tests that I couldn't catch it. Thanks for noticing it. I'll send out a v2.
> I think it would be nice if you updated the changelog to explain these
> things.
>
> Iago
>
>> + /* Don't use meta path for readpixels in below conditions. */
>> if (!tex_image) {
>>rb = ctx->ReadBuffer->_ColorReadBuffer;
>> +
>> + if (_mesa_get_readpixels_transfer_ops(ctx, rb->Format, format,
>> +type, GL_FALSE))
>> + return false;
>> +
>>if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format))
>> return false;
>> }
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/14] mesa: Fix conditions to test signed, unsigned integer format
On Sun, Jun 21, 2015 at 11:25 PM, Iago Toral wrote:
> On Fri, 2015-06-19 at 13:32 -0700, Anuj Phogat wrote:
>> On Thu, Jun 18, 2015 at 11:41 PM, Iago Toral wrote:
>> > On Thu, 2015-06-18 at 09:19 -0700, Anuj Phogat wrote:
>> >> On Thu, Jun 18, 2015 at 7:09 AM, Iago Toral wrote:
>> >> > On Tue, 2015-06-16 at 11:15 -0700, Anuj Phogat wrote:
>> >> >> Signed-off-by: Anuj Phogat
>> >> >> Cc:
>> >> >> ---
>> >> >> src/mesa/main/readpix.c | 2 ++
>> >> >> 1 file changed, 2 insertions(+)
>> >> >>
>> >> >> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
>> >> >> index caa2648..a9416ef 100644
>> >> >> --- a/src/mesa/main/readpix.c
>> >> >> +++ b/src/mesa/main/readpix.c
>> >> >> @@ -160,10 +160,12 @@ _mesa_readpixels_needs_slow_path(const struct
>> >> >> gl_context *ctx, GLenum format,
>> >> >>srcType = _mesa_get_format_datatype(rb->Format);
>> >> >>
>> >> >>if ((srcType == GL_INT &&
>> >> >> + _mesa_is_enum_format_integer(format) &&
>> >> >> (type == GL_UNSIGNED_INT ||
>> >> >> type == GL_UNSIGNED_SHORT ||
>> >> >> type == GL_UNSIGNED_BYTE)) ||
>> >> >>(srcType == GL_UNSIGNED_INT &&
>> >> >> + _mesa_is_enum_format_integer(format) &&
>> >> >> (type == GL_INT ||
>> >> >> type == GL_SHORT ||
>> >> >> type == GL_BYTE))) {
>> >> >
>> >> > As far as I understand this code we are trying to see if we can use
>> >> > memcpy to directly copy the contents of the framebuffer to the
>> >> > destination buffer. In that case, as long as the src/dst types have
>> >> > different sign we can't just use memcpy, right? In fact it looks like we
>> >> > might need to expand the checks to include the cases where srcType is
>> >> > GL_(UNSIGNED_)SHORT and GL_(UNSIGNED_)BYTE as well.
>> >> >
>> >> srcType returned by _mesa_get_format_datatype() is one of:
>> >> GL_UNSIGNED_NORMALIZED
>> >> GL_SIGNED_NORMALIZED
>> >> GL_UNSIGNED_INT
>> >> GL_INT
>> >> GL_FLOAT
>> >> So, the suggested checks for srcType are not required.
>> >
>> > Oh, right, although I think that does not invalidate my point: can we
>> > memcpy from a GL_UNSIGNED_NORMALIZED to a format with type GL_FLOAT or
>> > GL_SIGNED_NORMALIZED? It does not look like these checks here are
>> > thorough.
>> >
>> Helper function _mesa_need_signed_unsigned_int_conversion() is
>> meant to do the checks only for integer formats. May be add another
>> function to do the missing checks for other formats?
>
> I have no concerns about the _mesa_need_signed_unsigned_int_conversion
> function that you add in a later patch for your PBO work, my concern is
> related to the fact that you are assuming that the checks that you need
> in the PBO path are the same that we have in
> _mesa_readpixels_needs_slow_path, so you make both the same when I think
> they are trying to address different things.
>
> In your PBO code, you can't handle signed/unsigned integer conversions,
> so you need to detect that and fall back to another path. That should be
> fine I guess and the function _mesa_need_signed_unsigned_int_conversion
> does what you need, so no problems there.
>
> However, in _mesa_readpixels_needs_slow_path I think we don't want to
> just do integer checking. The purpose of the function is to tell whether
> we can use memcpy to copy pixels from the framebuffer to the dst, and if
> we have types with different signs, *whether they are integer or not*,
> we can't, so limiting the check only to integer types does not look
> right to me. The key aspect here is that what this function needs to
> check is not specific to integer types, even if the current code only
> seems to check things when the framebuffer has an integer format.
>
>> > In any case, that's beyond the point of your patch. Talking specifically
>> > about your patch: can we memcpy, for example, from a _signed_ integer
>> > format like MESA_FORMAT_R_SINT8 to an _unsigned_ format (integer or
>> > not)? I don't think we can, in which case your patch would not look
>> > correct to me.
>&
Re: [Mesa-dev] [PATCH 2/5] i965/gen9: Plugin the code for selecting YF/YS tiling on skl+
On Mon, Jun 22, 2015 at 2:53 PM, Ben Widawsky wrote:
> On Wed, Jun 10, 2015 at 03:30:47PM -0700, Anuj Phogat wrote:
>> Buffers with Yf/Ys tiling end up using meta upload / download
>> paths or the blitter for cases where they used tiled_memcpy paths
>> in case of Y tiling. This has exposed some bugs in meta path. To
>> avoid any piglit regressions on SKL this patch keeps the Yf/Ys
>> tiling disabled at the moment.
>>
>> V3: Make brw_miptree_choose_tr_mode() actually choose TRMODE. (Ben)
>> Few cosmetic changes.
>> V4: Get rid of brw_miptree_choose_tr_mode().
>> Take care of all tile resource modes {Yf, Ys, none} for all
>> generations at one place.
>>
>> Signed-off-by: Anuj Phogat
>> Cc: Ben Widawsky
>> ---
>> src/mesa/drivers/dri/i965/brw_tex_layout.c | 97
>> --
>> 1 file changed, 79 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
>> b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>> index b9ac4cf..c0ef5cc 100644
>> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
>> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>> @@ -807,27 +807,88 @@ brw_miptree_layout(struct brw_context *brw,
>> enum intel_miptree_tiling_mode requested,
>> struct intel_mipmap_tree *mt)
>> {
>> - mt->tr_mode = INTEL_MIPTREE_TRMODE_NONE;
>> + const unsigned bpp = mt->cpp * 8;
>> + const bool is_tr_mode_yf_ys_allowed =
>> + brw->gen >= 9 &&
>> + !for_bo &&
>> + !mt->compressed &&
>> + /* Enable YF/YS tiling only for color surfaces because depth and
>> + * stencil surfaces are not supported in blitter using fast copy
>> + * blit and meta PBO upload, download paths. No other paths
>> + * currently support Yf/Ys tiled surfaces.
>> + * FIXME: Remove this restriction once we have a tiled_memcpy()
>> + * path to do depth/stencil data upload/download to Yf/Ys tiled
>> + * surfaces.
>> + */
>
> I think it's more readable to move this comment above the variable
> declaration.
> Up to you though. Also I think "FINISHME" is the more appropriate
> classification
> for this type of thing.
>
Sure.
>> + _mesa_is_format_color_format(mt->format) &&
>> + (requested == INTEL_MIPTREE_TILING_Y ||
>> + requested == INTEL_MIPTREE_TILING_ANY) &&
>
> This is where my tiling flags would have helped a bit since you should be able
> to do flags & Y_TILED :P
>
Yes, I will do a follow up patch to make use of that.
>> + (bpp && is_power_of_two(bpp)) &&
>> + /* FIXME: To avoid piglit regressions keep the Yf/Ys tiling
>> + * disabled at the moment.
>> + */
>> + false;
>
> Also, "FINISHME"
>
>>
>> - intel_miptree_set_alignment(brw, mt);
>> - intel_miptree_set_total_width_height(brw, mt);
>> + /* Lower index (Yf) is the higher priority mode */
>> + const uint32_t tr_mode[3] = {INTEL_MIPTREE_TRMODE_YF,
>> +INTEL_MIPTREE_TRMODE_YS,
>> +INTEL_MIPTREE_TRMODE_NONE};
>> + int i = is_tr_mode_yf_ys_allowed ? 0 : ARRAY_SIZE(tr_mode) - 1;
>>
>> - if (!mt->total_width || !mt->total_height) {
>> - intel_miptree_release(&mt);
>> - return;
>> - }
>> + while (i < ARRAY_SIZE(tr_mode)) {
>> + if (brw->gen < 9)
>> + assert(tr_mode[i] == INTEL_MIPTREE_TRMODE_NONE);
>> + else
>> + assert(tr_mode[i] == INTEL_MIPTREE_TRMODE_YF ||
>> +tr_mode[i] == INTEL_MIPTREE_TRMODE_YS ||
>> +tr_mode[i] == INTEL_MIPTREE_TRMODE_NONE);
>>
>> - /* On Gen9+ the alignment values are expressed in multiples of the block
>> -* size
>> -*/
>> - if (brw->gen >= 9) {
>> - unsigned int i, j;
>> - _mesa_get_format_block_size(mt->format, &i, &j);
>> - mt->align_w /= i;
>> - mt->align_h /= j;
>> - }
>> + mt->tr_mode = tr_mode[i];
>> + intel_miptree_set_alignment(brw, mt);
>> + intel_miptree_set_total_width_height(brw, mt);
>>
>> - if (!for_bo)
>> - mt->tiling = brw_miptree_choose_tiling(brw, requested, mt);
>> + if (!mt->total_width || !mt->total_height) {
>> + intel_miptree_release(&mt);
>> + return;
>> + }
&g
Re: [Mesa-dev] [PATCH 5/5] i965/gen9: Allocate YF/YS tiled buffer objects
On Mon, Jun 22, 2015 at 5:13 PM, Ben Widawsky wrote:
> On Wed, Jun 10, 2015 at 03:30:50PM -0700, Anuj Phogat wrote:
>> In case of I915_TILING_{X,Y} we need to pass tiling format to libdrm
>> using drm_intel_bo_alloc_tiled(). But, In case of YF/YS tiled buffers
>> libdrm need not know about the tiling format because these buffers
>> don't have hardware support to be tiled or detiled through a fenced
>> region. libdrm still need to know buffer alignment value for its use
>> in kernel when resolving the relocation.
>>
>> Using drm_intel_bo_alloc_for_render() for YF/YS tiled buffers
>> satisfy both the above conditions.
>>
>> Signed-off-by: Anuj Phogat
>> Cc: Ben Widawsky
>> ---
>> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 86
>> +--
>> 1 file changed, 80 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> index 615cbfb..d4d9e76 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> @@ -522,6 +522,65 @@ intel_lower_compressed_format(struct brw_context *brw,
>> mesa_format format)
>> }
>> }
>>
>> +/* This function computes Yf/Ys tiled bo size and alignment. */
>
> It also computes pitch for the yf/ys case
>
>> +static uint64_t
>> +intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment)
>> +{
>> + const uint32_t bpp = mt->cpp * 8;
>> + const uint32_t aspect_ratio = (bpp == 16 || bpp == 64) ? 2 : 1;
>> + uint32_t tile_width, tile_height;
>> + const uint64_t min_size = 512 * 1024;
>> + const uint64_t max_size = 64 * 1024 * 1024;
>
> Where do min/max come from? Add a comment?
>
Your suspicion is right. These values are not applicable to i965+.
I copied this from libdrm and is applicable only to older chips.
>> + uint64_t i, stride, size, aligned_y;
>> +
>> + assert(mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE);
>> +
>> + switch (bpp) {
>> + case 8:
>> + tile_height = 64;
>> + break;
>> + case 16:
>> + case 32:
>> + tile_height = 32;
>> + break;
>> + case 64:
>> + case 128:
>> + tile_height = 16;
>> + break;
>> + default:
>> + tile_height = 0;
>
> make this unreachable()
>
sure
>> + printf("Invalid bits per pixel in %s: bpp = %d\n",
>> + __FUNCTION__, bpp);
>> + }
>
> I think ideally you should roll this logic into
> intel_miptree_get_tile_masks().
>
I wasn't aware of this function. If it's fine with you, I'll do it in
a follow up patch.
>> +
>> + if (mt->tr_mode == INTEL_MIPTREE_TRMODE_YS)
>> + tile_height *= 4;
>> +
>> + aligned_y = ALIGN(mt->total_height, tile_height);
>> +
>> + stride = mt->total_width * mt->cpp;
>> + tile_width = tile_height * mt->cpp * aspect_ratio;
>> + stride = ALIGN(stride, tile_width);
>> + size = stride * aligned_y;
>> +
>> + if (mt->tr_mode == INTEL_MIPTREE_TRMODE_YF) {
>> + *alignment = 4096;
>> + size = ALIGN(size, 4096);
>> + } else {
>> + *alignment = 64 * 1024;
>> + size = ALIGN(size, 64 * 1024);
>> + }
>
> Hmm. I think the above calculation for size is redundant since you already
> aligned to tile_width and height, above. Right? assert((size % 64K) == 0);
>
right. I'll put in assert.
>> +
>> + if (size > max_size) {
>> + mt->tr_mode = INTEL_MIPTREE_TRMODE_NONE;
>> + return 0;
>> + } else {
>> + mt->pitch = stride;
>> + for (i = min_size; i < size; i <<= 1)
>> + ;
>> + return i;
>
> I don't understand this. Why don't you just return size? It seems incredibly
> wasteful to both start a 512K, and to increment by powers of 2. Did I miss
> something?
>
> Also, I don't understand max_size. I must be missing something in the spec
> with
> the min/max values, can you point me to them?
>
I'll get rid of this bogus code.
>> + }
>> +}
>>
>> struct intel_mipmap_tree *
>> intel_miptree_create(struct brw_context *brw,
>> @@ -575,12 +634,27 @@ intel_miptree_create(struct brw_context *brw,
>>
>> unsigned long pitch;
>> mt->etc_format = etc_format;
>> - mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree",
>> - to
Re: [Mesa-dev] [PATCH 2/2] i965: Split out gen8 push constant state upload
T_BATCH(opcode << 16 | (11 - 2));
>
> /* Workaround for SKL+ (we use option #2 until we have a need for more
> * constant buffers). This comes from the documentation for
> 3DSTATE_CONSTANT_*
> @@ -57,42 +53,40 @@ gen7_upload_constant_state(struct brw_context *brw,
> */
> if (brw->gen >= 9 && active) {
>OUT_BATCH(0);
> - OUT_BATCH(stage_state->push_const_size);
> + OUT_BATCH(stage_state->push_const_size); /* buffer 3, 2 */
> } else {
> - OUT_BATCH(active ? stage_state->push_const_size : 0);
> + OUT_BATCH(active ? stage_state->push_const_size : 0); /* buffer 1, 0 */
>OUT_BATCH(0);
> }
> - /* Pointer to the constant buffer. Covered by the set of state flags
> -* from gen6_prepare_wm_contants
> -*/
> - if (brw->gen >= 9 && active) {
> - OUT_BATCH(0);
> +
> + /* Push constant buffer #0 */
> + if (brw->gen >= 9 || !active) {
>OUT_BATCH(0);
>OUT_BATCH(0);
> + } else {
> + OUT_BATCH(stage_state->push_const_offset);
>OUT_BATCH(0);
> + }
> +
> + /* Push constant buffer #1 */
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> +
> + /* Push constant buffer #2 */
> + if (brw->gen >= 9 && active) {
>/* XXX: When using buffers other than 0, you need to specify the
> * graphics virtual address regardless of INSPM/debug bits
> */
>OUT_RELOC64(brw->batch.bo, I915_GEM_DOMAIN_RENDER, 0,
>stage_state->push_const_offset);
> - OUT_BATCH(0);
> - OUT_BATCH(0);
> - } else if (brw->gen>= 8) {
> - OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0);
> - OUT_BATCH(0);
> - OUT_BATCH(0);
> - OUT_BATCH(0);
> - OUT_BATCH(0);
> - OUT_BATCH(0);
> - OUT_BATCH(0);
> - OUT_BATCH(0);
> } else {
> - OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0);
> - OUT_BATCH(0);
>OUT_BATCH(0);
>OUT_BATCH(0);
> }
>
> + /* Push constant buffer #3 */
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> ADVANCE_BATCH();
>
>/* On SKL+ the new constants don't take effect until the next corresponding
> @@ -103,6 +97,36 @@ gen7_upload_constant_state(struct brw_context *brw,
>brw->ctx.NewDriverState |= BRW_NEW_SURFACES;
> }
>
> +static void
> +gen7_upload_constant_state(struct brw_context *brw,
> + const struct brw_stage_state *stage_state,
> + bool active, unsigned opcode)
> +{
> + BEGIN_BATCH(7);
> + OUT_BATCH(opcode << 16 | (7 - 2));
> + OUT_BATCH(active ? stage_state->push_const_size : 0);
> + OUT_BATCH(0);
> + OUT_BATCH(active ? (stage_state->push_const_offset | GEN7_MOCS_L3) : 0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + ADVANCE_BATCH();
> +}
> +
> +void
> +brw_upload_constant_state(struct brw_context *brw,
> + const struct brw_stage_state *stage_state,
> + bool active, unsigned opcode)
> +{
> + /* Disable if the shader stage is inactive or there are no push
> constants. */
> + active = active && stage_state->push_const_size != 0;
> +
> + if (brw->gen >= 8)
> + gen8_upload_constant_state(brw, stage_state, active, opcode);
> + else
> + gen7_upload_constant_state(brw, stage_state, active, opcode);
> +}
> +
>
> static void
> upload_vs_state(struct brw_context *brw)
> --
> 2.4.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
LGTM.
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 5/5] i965/gen9: Allocate YF/YS tiled buffer objects
In case of I915_TILING_{X,Y} we need to pass tiling format to libdrm
using drm_intel_bo_alloc_tiled(). But, In case of YF/YS tiled buffers
libdrm need not know about the tiling format because these buffers
don't have hardware support to be tiled or detiled through a fenced
region. libdrm still need to know buffer alignment value for its use
in kernel when resolving the relocation.
Using drm_intel_bo_alloc_for_render() for YF/YS tiled buffers
satisfy both the above conditions.
V2: Delete min/max buffer size restrictions not valid for i965+.
Remove redundant align to tile size statements.
Remove some redundant code now when there are no min/max buffer size.
Signed-off-by: Anuj Phogat
Cc: Ben Widawsky
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 62 +--
1 file changed, 58 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 80c52f2..5bcb094 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -558,6 +558,48 @@ intel_lower_compressed_format(struct brw_context *brw,
mesa_format format)
}
}
+/* This function computes Yf/Ys tiled bo size, alignment and pitch. */
+static uint64_t
+intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment,
+uint64_t *pitch)
+{
+ const uint32_t bpp = mt->cpp * 8;
+ const uint32_t aspect_ratio = (bpp == 16 || bpp == 64) ? 2 : 1;
+ uint32_t tile_width, tile_height;
+ uint64_t stride, size, aligned_y;
+
+ assert(mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE);
+
+ *alignment = mt->tr_mode == INTEL_MIPTREE_TRMODE_YF ? 4096 : 64 * 1024;
+
+ switch (bpp) {
+ case 8:
+ tile_height = 64;
+ break;
+ case 16:
+ case 32:
+ tile_height = 32;
+ break;
+ case 64:
+ case 128:
+ tile_height = 16;
+ break;
+ default:
+ unreachable("not reached");
+ }
+
+ if (mt->tr_mode == INTEL_MIPTREE_TRMODE_YS)
+ tile_height *= 4;
+
+ aligned_y = ALIGN(mt->total_height, tile_height);
+ stride = mt->total_width * mt->cpp;
+ tile_width = tile_height * mt->cpp * aspect_ratio;
+ stride = ALIGN(stride, tile_width);
+ size = stride * aligned_y;
+
+ *pitch = stride;
+ return size;
+}
struct intel_mipmap_tree *
intel_miptree_create(struct brw_context *brw,
@@ -616,11 +658,23 @@ intel_miptree_create(struct brw_context *brw,
alloc_flags |= BO_ALLOC_FOR_RENDER;
unsigned long pitch;
- mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree", total_width,
- total_height, mt->cpp, &mt->tiling,
- &pitch, alloc_flags);
mt->etc_format = etc_format;
- mt->pitch = pitch;
+
+ if (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE) {
+ unsigned alignment = 0;
+ unsigned long size;
+ size = intel_get_yf_ys_bo_size(mt, &alignment, &pitch);
+ assert(size);
+ mt->bo = drm_intel_bo_alloc_for_render(brw->bufmgr, "miptree",
+ size, alignment);
+ mt->pitch = pitch;
+ } else {
+ mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree",
+total_width, total_height, mt->cpp,
+&mt->tiling, &pitch,
+alloc_flags);
+ mt->pitch = pitch;
+ }
/* If the BO is too large to fit in the aperture, we need to use the
* BLT engine to support it. Prior to Sandybridge, the BLT paths can't
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa : NULL check InfoLog
On Tue, Jun 23, 2015 at 4:03 AM, Marta Lofstedt wrote: > From: Marta Lofstedt > > When a program is compiled, but linking failed the > sh->InfoLog could be NULL. This is expoloited > by OpenGL ES 3.1 conformance tests. > > Signed-off-by: Marta Lofstedt > --- > src/mesa/main/shaderapi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c > index a4296ad..c783c69 100644 > --- a/src/mesa/main/shaderapi.c > +++ b/src/mesa/main/shaderapi.c > @@ -1920,8 +1920,8 @@ _mesa_create_shader_program(struct gl_context* ctx, > GLboolean separate, > } > #endif > } > - > -ralloc_strcat(&shProg->InfoLog, sh->InfoLog); > + if (sh->InfoLog) > + ralloc_strcat(&shProg->InfoLog, sh->InfoLog); >} > >delete_shader(ctx, shader); > -- > 1.9.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev Reviewed-by: Anuj Phogat ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Drop brw->depthstencil.stencil_offset from gen8_depth_state.c.
On Wed, Jun 24, 2015 at 12:04 AM, Kenneth Graunke wrote: > This is always 0 - only brw_workaround_depthstencil_alignment ever sets > it, and that doesn't run on Gen6+. My initial Broadwell depth state > commit had this mistake. > > Signed-off-by: Kenneth Graunke > --- > src/mesa/drivers/dri/i965/gen8_depth_state.c |7 ++- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c > b/src/mesa/drivers/dri/i965/gen8_depth_state.c > index 7c4ec06..76ba09c 100644 > --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c > +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c > @@ -41,7 +41,6 @@ emit_depth_packets(struct brw_context *brw, > bool depth_writable, > struct intel_mipmap_tree *stencil_mt, > bool stencil_writable, > - uint32_t stencil_offset, > bool hiz, > uint32_t width, > uint32_t height, > @@ -127,8 +126,7 @@ emit_depth_packets(struct brw_context *brw, >OUT_BATCH(HSW_STENCIL_ENABLED | mocs_wb << 22 | > (2 * stencil_mt->pitch - 1)); >OUT_RELOC64(stencil_mt->bo, > - I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, > - stencil_offset); > + I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0); >OUT_BATCH(stencil_mt ? stencil_mt->qpitch >> 2 : 0); >ADVANCE_BATCH(); > } > @@ -220,7 +218,6 @@ gen8_emit_depth_stencil_hiz(struct brw_context *brw, > emit_depth_packets(brw, depth_mt, brw_depthbuffer_format(brw), surftype, >ctx->Depth.Mask != 0, >stencil_mt, ctx->Stencil._WriteEnabled, > - brw->depthstencil.stencil_offset, >hiz, width, height, depth, lod, min_array_element); > } > > @@ -439,7 +436,7 @@ gen8_hiz_exec(struct brw_context *brw, struct > intel_mipmap_tree *mt, >brw_depth_format(brw, mt->format), >BRW_SURFACE_2D, >true, /* depth writes */ > - NULL, false, 0, /* no stencil for now */ > + NULL, false, /* no stencil for now */ >true, /* hiz */ >surface_width, >surface_height, > -- > 1.7.10.4 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev Reviewed-by: Anuj Phogat ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 04/18] mesa: return bool instead of GLboolean in compressedteximage_only_format()
On Mon, Jun 22, 2015 at 4:02 PM, Nanley Chery wrote:
> From: Nanley Chery
>
> In agreement with the coding style, functions that aren't directly visible
> to the GL API should prefer the use of bool over GLboolean.
>
> Suggested-by: Ian Romanick
> Signed-off-by: Nanley Chery
> ---
> src/mesa/main/teximage.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index 86ef407..0e0488a 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -1763,7 +1763,7 @@ _mesa_test_proxy_teximage(struct gl_context *ctx,
> GLenum target, GLint level,
> /**
> * Return true if the format is only valid for glCompressedTexImage.
> */
> -static GLboolean
> +static bool
> compressedteximage_only_format(const struct gl_context *ctx, GLenum format)
> {
> switch (format) {
> @@ -1806,9 +1806,9 @@ compressedteximage_only_format(const struct gl_context
> *ctx, GLenum format)
> case GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x10_KHR:
> case GL_COMPRESSED_SRGB8_ALPHA8_ASTC_12x10_KHR:
> case GL_COMPRESSED_SRGB8_ALPHA8_ASTC_12x12_KHR:
> - return GL_TRUE;
> + return true;
> default:
> - return GL_FALSE;
> + return false;
> }
> }
>
> --
> 2.4.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 11/18] mesa/macros: add power-of-two assertions for alignment macros
On Mon, Jun 22, 2015 at 4:02 PM, Nanley Chery wrote:
> From: Nanley Chery
>
> ALIGN and ROUND_DOWN_TO both require that the alignment value passed
> into the macro be a power of two in the comments. Using software assertions
> verifies this to be the case.
>
> v2: use static inline functions instead of gcc-specific statement expressions.
>
> Signed-off-by: Nanley Chery
> ---
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 2 +-
> src/mesa/main/macros.h | 16 +---
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index 59081ea..1a57784 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -134,7 +134,7 @@ fs_visitor::nir_setup_outputs(nir_shader *shader)
> : var->type->vector_elements;
>
>if (stage == MESA_SHADER_VERTEX) {
> - for (int i = 0; i < ALIGN(type_size(var->type), 4) / 4; i++) {
> + for (unsigned int i = 0; i < ALIGN(type_size(var->type), 4) / 4;
> i++) {
> int output = var->data.location + i;
> this->outputs[output] = offset(reg, 4 * i);
> this->output_components[output] = vector_elements;
> diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
> index 0608650..4a640ad 100644
> --- a/src/mesa/main/macros.h
> +++ b/src/mesa/main/macros.h
> @@ -684,7 +684,7 @@ minify(unsigned value, unsigned levels)
> * Note that this considers 0 a power of two.
> */
> static inline bool
> -is_power_of_two(unsigned value)
> +is_power_of_two(uintptr_t value)
> {
> return (value & (value - 1)) == 0;
> }
> @@ -700,7 +700,12 @@ is_power_of_two(unsigned value)
> *
> * \sa ROUND_DOWN_TO()
> */
> -#define ALIGN(value, alignment) (((value) + (alignment) - 1) &
> ~((alignment) - 1))
> +static inline uintptr_t
> +ALIGN(uintptr_t value, uintptr_t alignment)
> +{
> + assert(is_power_of_two(alignment));
Also handle the 0 alignment. is_power_of_two() returns true for 0.
Use assert(alignment > 0 && is_power_of_two(alignment))?
> + return (((value) + (alignment) - 1) & ~((alignment) - 1));
> +}
>
> /**
> * Align a value down to an alignment value
> @@ -713,7 +718,12 @@ is_power_of_two(unsigned value)
> *
> * \sa ALIGN()
> */
> -#define ROUND_DOWN_TO(value, alignment) ((value) & ~(alignment - 1))
> +static inline uintptr_t
> +ROUND_DOWN_TO(uintptr_t value, uintptr_t alignment)
> +{
> + assert(is_power_of_two(alignment));
Here as well.
> + return ((value) & ~(alignment - 1));
> +}
>
>
> /** Cross product of two 3-element vectors */
> --
> 2.4.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
With above changes and indentation fixes:
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 12/18] mesa/macros: move ALIGN_NPOT to macros.h
On Mon, Jun 22, 2015 at 4:02 PM, Nanley Chery wrote:
> From: Nanley Chery
>
> Aligning with a non-power-of-two number is a general task that can be used in
> various places. This commit is required for the next one.
>
> Signed-off-by: Nanley Chery
> ---
> src/mesa/drivers/dri/i965/intel_upload.c | 6 --
> src/mesa/main/macros.h | 6 ++
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_upload.c
> b/src/mesa/drivers/dri/i965/intel_upload.c
> index 870aabc..deaae6c 100644
> --- a/src/mesa/drivers/dri/i965/intel_upload.c
> +++ b/src/mesa/drivers/dri/i965/intel_upload.c
> @@ -44,12 +44,6 @@
>
> #define INTEL_UPLOAD_SIZE (64*1024)
>
> -/**
> - * Like ALIGN(), but works with a non-power-of-two alignment.
> - */
> -#define ALIGN_NPOT(value, alignment) \
> - (((value) + (alignment) - 1) / (alignment) * (alignment))
> -
> void
> intel_upload_finish(struct brw_context *brw)
> {
> diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
> index 4a640ad..4a08130 100644
> --- a/src/mesa/main/macros.h
> +++ b/src/mesa/main/macros.h
> @@ -708,6 +708,12 @@ ALIGN(uintptr_t value, uintptr_t alignment)
> }
>
> /**
> + * Like ALIGN(), but works with a non-power-of-two alignment.
> + */
> +#define ALIGN_NPOT(value, alignment) \
> + (((value) + (alignment) - 1) / (alignment) * (alignment))
> +
> +/**
Add an assert for the 0 alignment?
> * Align a value down to an alignment value
> *
> * If \c value is not already aligned to the requested alignment value, it
> --
> 2.4.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
R
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 13/18] i965: use ALIGN_NPOT for setting ASTC mipmap layouts
On Mon, Jun 22, 2015 at 4:02 PM, Nanley Chery wrote:
> From: Nanley Chery
>
> ALIGN is changed to ALIGN_NPOT because alignment values are sometimes not
> powers of two when working with ASTC.
>
> Signed-off-by: Nanley Chery
> ---
> src/mesa/drivers/dri/i965/brw_tex_layout.c| 12 ++--
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 4 ++--
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> index 998d8c4..4007697 100644
> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> @@ -367,7 +367,7 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
> mt->total_width = mt->physical_width0;
>
> if (mt->compressed) {
> - mt->total_width = ALIGN(mt->physical_width0, mt->align_w);
> + mt->total_width = ALIGN_NPOT(mt->physical_width0, mt->align_w);
> }
>
> /* May need to adjust width to accommodate the placement of
> @@ -379,10 +379,10 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
> unsigned mip1_width;
>
> if (mt->compressed) {
> - mip1_width = ALIGN(minify(mt->physical_width0, 1), mt->align_w) +
> - ALIGN(minify(mt->physical_width0, 2), bw);
> + mip1_width = ALIGN_NPOT(minify(mt->physical_width0, 1),
> mt->align_w) +
> + ALIGN_NPOT(minify(mt->physical_width0, 2), bw);
> } else {
> - mip1_width = ALIGN(minify(mt->physical_width0, 1), mt->align_w) +
> + mip1_width = ALIGN_NPOT(minify(mt->physical_width0, 1),
> mt->align_w) +
> minify(mt->physical_width0, 2);
> }
>
> @@ -398,7 +398,7 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
>
>intel_miptree_set_level_info(mt, level, x, y, depth);
>
> - img_height = ALIGN(height, mt->align_h);
> + img_height = ALIGN_NPOT(height, mt->align_h);
>if (mt->compressed)
> img_height /= bh;
>
> @@ -415,7 +415,7 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
>/* Layout_below: step right after second mipmap.
> */
>if (level == mt->first_level + 1) {
> -x += ALIGN(width, mt->align_w);
> +x += ALIGN_NPOT(width, mt->align_w);
>} else {
> y += img_height;
>}
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 6aa969a..b47f49d0 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1213,8 +1213,8 @@ intel_miptree_copy_slice(struct brw_context *brw,
> if (dst_mt->compressed) {
>unsigned int i, j;
>_mesa_get_format_block_size(dst_mt->format, &i, &j);
> - height = ALIGN(height, j) / j;
> - width = ALIGN(width, i);
> + height = ALIGN_NPOT(height, j) / j;
> + width = ALIGN_NPOT(width, i);
> }
>
> /* If it's a packed depth/stencil buffer with separate stencil, the blit
> --
> 2.4.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: remove unnecessary checks in _mesa_readpixels_needs_slow_path
On Tue, Jun 23, 2015 at 3:34 AM, Iago Toral Quiroga wrote:
> readpixels_can_use_memcpy will later call _mesa_format_matches_format_and_type
> which does much tighter checks than these to decide if we can use
> memcpy for readpixels.
>
> Also, the checks do not seem to be extensive enough anyway, since we are
> checking for signed/unsigned conversion only when the framebuffer has
> integers,
> but the same checks could be done for other types anyway, since as long as
> there is a signed/unsigned conversion we can't memcpy.
>
> No regressions observed on i965/llvmpipe.
> ---
> The way gallium uses _mesa_format_matches_format_and_type and
> _mesa_readpixels_needs_slow_path is a bit different, they call these
> functions to decide if they want to fallback to Mesa's implementation
> of ReadPixels, not exactly to check if we can memcpy... so it is unclear
> to me if some combinations may still need these checks to make the right
> decision. I did not see any regressions with llvmpipe at least, so I am
> assuming that they are not needed, but maybe someone wants to give this
> patch a test run on nouveau or radeon, just in case. If they are needed
> I guess the right thing would be to move the checks to st_cb_readpixels.c.
>
> src/mesa/main/readpix.c | 16
> 1 file changed, 16 deletions(-)
>
> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> index a3357cd..e256695 100644
> --- a/src/mesa/main/readpix.c
> +++ b/src/mesa/main/readpix.c
> @@ -128,7 +128,6 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
> *ctx, GLenum format,
> {
> struct gl_renderbuffer *rb =
> _mesa_get_read_renderbuffer_for_format(ctx, format);
> - GLenum srcType;
>
> assert(rb);
>
> @@ -153,21 +152,6 @@ _mesa_readpixels_needs_slow_path(const struct gl_context
> *ctx, GLenum format,
> return GL_TRUE;
>}
>
> - /* Conversion between signed and unsigned integers needs masking
> - * (it isn't just memcpy). */
> - srcType = _mesa_get_format_datatype(rb->Format);
> -
> - if ((srcType == GL_INT &&
> - (type == GL_UNSIGNED_INT ||
> -type == GL_UNSIGNED_SHORT ||
> -type == GL_UNSIGNED_BYTE)) ||
> - (srcType == GL_UNSIGNED_INT &&
> - (type == GL_INT ||
> -type == GL_SHORT ||
> -type == GL_BYTE))) {
> - return GL_TRUE;
> - }
> -
>/* And finally, see if there are any transfer ops. */
>return get_readpixels_transfer_ops(ctx, rb->Format, format, type,
> uses_blit) != 0;
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Thanks for the patch Iago.
Reviewed-by: Anuj Phogat
You might want to wait few days to hear any comments on the gallium
usage of the function. If no one objects you can push it upstream.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/gen9: use an unreserved surface alignment value
On Wed, Jun 24, 2015 at 3:51 PM, Nanley Chery wrote:
> From: Nanley Chery
>
> Although the horizontal and vertical alignment fields are ignored here,
> 0 is a reserved value for them and may cause undefined behavior. Change
> the default value to an abitrary valid one.
>
> Signed-off-by: Nanley Chery
> ---
> src/mesa/drivers/dri/i965/gen8_surface_state.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index b2d1a57..22ae960 100644
> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> @@ -93,7 +93,7 @@ vertical_alignment(const struct brw_context *brw,
> if (brw->gen > 8 &&
> (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE ||
> surf_type == BRW_SURFACE_1D))
> - return 0;
> + return GEN8_SURFACE_VALIGN_4;
>
> switch (mt->align_h) {
> case 4:
> @@ -118,7 +118,7 @@ horizontal_alignment(const struct brw_context *brw,
> if (brw->gen > 8 &&
> (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE ||
> gen9_use_linear_1d_layout(brw, mt)))
> - return 0;
> + return GEN8_SURFACE_HALIGN_4;
>
> switch (mt->align_w) {
> case 4:
> --
> 2.4.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Good find Nanley. We had no known issues with value 0 but it's
always nice to avoid undefined behavior :).
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 05/14] meta: Abort meta pbo path if readpixels need signed-unsigned conversion
Meta pbo path for ReadPixels rely on BlitFramebuffer which doesn't support
signed to unsigned integer conversions and vice versa.
Without this patch, piglit test fbo_integer_readpixels_sint_uint fails, when
forced to use the meta pbo path.
v2: Make need_rgb_to_luminance_conversion() a static function. (Iago)
Bump up the comment and the commit message. (Jason)
Signed-off-by: Anuj Phogat
Reviewed-by: Jason Ekstrand
Cc: Iago Toral
Cc:
---
src/mesa/drivers/common/meta_tex_subimage.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
b/src/mesa/drivers/common/meta_tex_subimage.c
index 00364f8..a617b77 100644
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@ -248,6 +248,23 @@ fail:
return success;
}
+static bool
+need_signed_unsigned_int_conversion(mesa_format rbFormat,
+GLenum format, GLenum type)
+{
+ const GLenum srcType = _mesa_get_format_datatype(rbFormat);
+ return (srcType == GL_INT &&
+ _mesa_is_enum_format_integer(format) &&
+ (type == GL_UNSIGNED_INT ||
+type == GL_UNSIGNED_SHORT ||
+type == GL_UNSIGNED_BYTE)) ||
+ (srcType == GL_UNSIGNED_INT &&
+ _mesa_is_enum_format_integer(format) &&
+ (type == GL_INT ||
+type == GL_SHORT ||
+type == GL_BYTE));
+}
+
bool
_mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, GLuint dims,
struct gl_texture_image *tex_image,
@@ -283,6 +300,14 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format))
return false;
+
+ /* This function rely on BlitFramebuffer to fill in the pixel data for
+ * ReadPixels. But, BlitFrameBuffer doesn't support signed to unsigned
+ * or unsigned to signed integer conversions. OpenGL spec expects an
+ * invalid operation in that case.
+ */
+ if (need_signed_unsigned_int_conversion(rb->Format, format, type))
+ return false;
}
/* For arrays, use a tall (height * depth) 2D texture but taking into
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/gen9: use an unreserved surface alignment value
On Thu, Jun 25, 2015 at 12:37 AM, Pohjolainen, Topi
wrote:
> On Wed, Jun 24, 2015 at 05:57:13PM -0700, Anuj Phogat wrote:
>> On Wed, Jun 24, 2015 at 3:51 PM, Nanley Chery wrote:
>> > From: Nanley Chery
>> >
>> > Although the horizontal and vertical alignment fields are ignored here,
>> > 0 is a reserved value for them and may cause undefined behavior. Change
>> > the default value to an abitrary valid one.
>> >
>> > Signed-off-by: Nanley Chery
>> > ---
>> > src/mesa/drivers/dri/i965/gen8_surface_state.c | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> > b/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> > index b2d1a57..22ae960 100644
>> > --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> > +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> > @@ -93,7 +93,7 @@ vertical_alignment(const struct brw_context *brw,
>> > if (brw->gen > 8 &&
>> > (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE ||
>> > surf_type == BRW_SURFACE_1D))
>> > - return 0;
>> > + return GEN8_SURFACE_VALIGN_4;
>> >
>> > switch (mt->align_h) {
>> > case 4:
>> > @@ -118,7 +118,7 @@ horizontal_alignment(const struct brw_context *brw,
>> > if (brw->gen > 8 &&
>> > (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE ||
>> > gen9_use_linear_1d_layout(brw, mt)))
>> > - return 0;
>> > + return GEN8_SURFACE_HALIGN_4;
>> >
>> > switch (mt->align_w) {
>> > case 4:
>> > --
>> > 2.4.4
>> >
>> > ___
>> > mesa-dev mailing list
>> > mesa-dev@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>> Good find Nanley. We had no known issues with value 0 but it's
>> always nice to avoid undefined behavior :).
>
> Right, I thought about this when I reviewed the original. The spec says
> it is ignored in these cases and hence the reserved value seemed fine. Now
> that we put something meaningful there, somebody is going to compare the
> spec and wonder why we set it to 4. If we added also a comment here that
> says this is just an arbitrary (non-reserved) value and really ignored
> by the hardware, it would prevent misunderstandings. What do you guys think?
That's a good suggestion Topi.
>
>>
>> Reviewed-by: Anuj Phogat
>> ___
>> 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
Re: [Mesa-dev] [PATCH] i965/skl: Use more compact hiz dimensions
On Wed, Jun 24, 2015 at 8:07 PM, Ben Widawsky
wrote:
> gen8 had some special restrictions which don't seem to carry over to gen9.
> Quoting the spec for SKL:
> "The Z_Height and Z_Width values must equal those present in
> 3DSTATE_DEPTH_BUFFER incremented by one."
>
> This fixes nothing in piglit (and regresses nothing).
>
> Cc: Jordan Justen
> Signed-off-by: Ben Widawsky
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 32
> ++-
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 6aa969a..432a47c 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1550,21 +1550,23 @@ intel_gen8_hiz_buf_create(struct brw_context *brw,
> /* Gen7 PRM Volume 2, Part 1, 11.5.3 "Hierarchical Depth Buffer" documents
> * adjustments required for Z_Height and Z_Width based on multisampling.
> */
> - switch (mt->num_samples) {
> - case 0:
> - case 1:
> - break;
> - case 2:
> - case 4:
> - z_width *= 2;
> - z_height *= 2;
> - break;
> - case 8:
> - z_width *= 4;
> - z_height *= 2;
> - break;
> - default:
> - unreachable("unsupported sample count");
> + if (brw->gen < 9) {
> + switch (mt->num_samples) {
> + case 0:
> + case 1:
> + break;
> + case 2:
> + case 4:
> + z_width *= 2;
> + z_height *= 2;
> + break;
> + case 8:
> + z_width *= 4;
> + z_height *= 2;
> + break;
> + default:
> + unreachable("unsupported sample count");
> + }
> }
>
> const unsigned vertical_align = 8; /* 'j' in the docs */
> --
> 2.4.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Remove special case for layered drawbuffer attachments.
On Thu, Jun 25, 2015 at 10:08 AM, Kenneth Graunke wrote: > When binding a layered texture, the layer is already 0. There's no need > to special case this. > > Signed-off-by: Kenneth Graunke > --- > src/mesa/drivers/dri/i965/gen6_surface_state.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/mesa/drivers/dri/i965/gen6_surface_state.c > b/src/mesa/drivers/dri/i965/gen6_surface_state.c > index 03e913a..39de62f 100644 > --- a/src/mesa/drivers/dri/i965/gen6_surface_state.c > +++ b/src/mesa/drivers/dri/i965/gen6_surface_state.c > @@ -88,7 +88,8 @@ gen6_update_renderbuffer_surface(struct brw_context *brw, >break; > } > > - const int min_array_element = layered ? 0 : irb->mt_layer; > + const int min_array_element = irb->mt_layer; > + assert(!layered || irb->mt_layer == 0); > > surf[0] = SET_FIELD(surftype, BRW_SURFACE_TYPE) | > SET_FIELD(format, BRW_SURFACE_FORMAT); > -- > 2.4.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev Reviewed-by: Anuj Phogat ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 11/18] mesa/macros: add power-of-two assertions for alignment macros
On Thu, Jun 25, 2015 at 3:22 PM, Nanley Chery wrote:
> How about if I create a patch which puts the greater than 0 check into
> is_power_of_two()?
>
> (value > 0) && (value & (value - 1)) == 0);
>
Not a bad idea except that you'll need to fix conditions at all other places
which expect the current behavior. I'll leave it up to you. FYI gallium also
have a helper function util_is_power_of_two() which returns true for 0.
> Thanks,
> Nanley
>
> On Wed, Jun 24, 2015 at 3:22 PM, Anuj Phogat wrote:
>> On Mon, Jun 22, 2015 at 4:02 PM, Nanley Chery wrote:
>>> From: Nanley Chery
>>>
>>> ALIGN and ROUND_DOWN_TO both require that the alignment value passed
>>> into the macro be a power of two in the comments. Using software assertions
>>> verifies this to be the case.
>>>
>>> v2: use static inline functions instead of gcc-specific statement
>>> expressions.
>>>
>>> Signed-off-by: Nanley Chery
>>> ---
>>> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 2 +-
>>> src/mesa/main/macros.h | 16 +---
>>> 2 files changed, 14 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> index 59081ea..1a57784 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> @@ -134,7 +134,7 @@ fs_visitor::nir_setup_outputs(nir_shader *shader)
>>> : var->type->vector_elements;
>>>
>>>if (stage == MESA_SHADER_VERTEX) {
>>> - for (int i = 0; i < ALIGN(type_size(var->type), 4) / 4; i++) {
>>> + for (unsigned int i = 0; i < ALIGN(type_size(var->type), 4) / 4;
>>> i++) {
>>> int output = var->data.location + i;
>>> this->outputs[output] = offset(reg, 4 * i);
>>> this->output_components[output] = vector_elements;
>>> diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
>>> index 0608650..4a640ad 100644
>>> --- a/src/mesa/main/macros.h
>>> +++ b/src/mesa/main/macros.h
>>> @@ -684,7 +684,7 @@ minify(unsigned value, unsigned levels)
>>> * Note that this considers 0 a power of two.
>>> */
>>> static inline bool
>>> -is_power_of_two(unsigned value)
>>> +is_power_of_two(uintptr_t value)
>>> {
>>> return (value & (value - 1)) == 0;
>>> }
>>> @@ -700,7 +700,12 @@ is_power_of_two(unsigned value)
>>> *
>>> * \sa ROUND_DOWN_TO()
>>> */
>>> -#define ALIGN(value, alignment) (((value) + (alignment) - 1) &
>>> ~((alignment) - 1))
>>> +static inline uintptr_t
>>> +ALIGN(uintptr_t value, uintptr_t alignment)
>>> +{
>>> + assert(is_power_of_two(alignment));
>> Also handle the 0 alignment. is_power_of_two() returns true for 0.
>> Use assert(alignment > 0 && is_power_of_two(alignment))?
>>> + return (((value) + (alignment) - 1) & ~((alignment) - 1));
>>> +}
>>>
>>> /**
>>> * Align a value down to an alignment value
>>> @@ -713,7 +718,12 @@ is_power_of_two(unsigned value)
>>> *
>>> * \sa ALIGN()
>>> */
>>> -#define ROUND_DOWN_TO(value, alignment) ((value) & ~(alignment - 1))
>>> +static inline uintptr_t
>>> +ROUND_DOWN_TO(uintptr_t value, uintptr_t alignment)
>>> +{
>>> + assert(is_power_of_two(alignment));
>> Here as well.
>>> + return ((value) & ~(alignment - 1));
>>> +}
>>>
>>>
>>> /** Cross product of two 3-element vectors */
>>> --
>>> 2.4.2
>>>
>>> ___
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>> With above changes and indentation fixes:
>> Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/macros: move ALIGN_NPOT to macros.h
On Fri, Jun 26, 2015 at 11:39 AM, Nanley Chery wrote:
> From: Nanley Chery
>
> Aligning with a non-power-of-two number is a general task that can be used in
> various places. This commit is required for the next one.
>
> v2: add greater than 0 assertion (Anuj).
> convert the macro to a static inline function.
>
> Signed-off-by: Nanley Chery
> ---
> src/mesa/drivers/dri/i965/intel_upload.c | 6 --
> src/mesa/main/macros.h | 10 ++
> 2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_upload.c
> b/src/mesa/drivers/dri/i965/intel_upload.c
> index 870aabc..deaae6c 100644
> --- a/src/mesa/drivers/dri/i965/intel_upload.c
> +++ b/src/mesa/drivers/dri/i965/intel_upload.c
> @@ -44,12 +44,6 @@
>
> #define INTEL_UPLOAD_SIZE (64*1024)
>
> -/**
> - * Like ALIGN(), but works with a non-power-of-two alignment.
> - */
> -#define ALIGN_NPOT(value, alignment) \
> - (((value) + (alignment) - 1) / (alignment) * (alignment))
> -
> void
> intel_upload_finish(struct brw_context *brw)
> {
> diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
> index 7f89447..1a587fb 100644
> --- a/src/mesa/main/macros.h
> +++ b/src/mesa/main/macros.h
> @@ -708,6 +708,16 @@ ALIGN(uintptr_t value, int32_t alignment)
> }
>
> /**
> + * Like ALIGN(), but works with a non-power-of-two alignment.
> + */
> +static inline uintptr_t
> +ALIGN_NPOT(uintptr_t value, int32_t alignment)
> +{
> + assert(alignment > 0);
> + return (value + alignment - 1) / alignment * alignment;
> +}
> +
> +/**
> * Align a value down to an alignment value
> *
> * If \c value is not already aligned to the requested alignment value, it
> --
> 2.4.4
>
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 02/14] meta: Fix transfer operations check in meta pbo path for readpixels
Currently used ctx->_ImageTransferState check is not sufficient
because it doesn't include the read color clamping enabled with
GL_CLAMP_READ_COLOR. So, use the helper function
_mesa_get_readpixels_transfer_ops().
Also, transfer operations don't affect glGetTexImage(). So, do
the check only for glReadPixles.
Without this patch, arb_color_buffer_float-readpixels test fails, when
forced to use meta pbo path.
V2: Add a comment and bump up the commit message.
Signed-off-by: Anuj Phogat
Cc:
Cc: Iago Toral
Cc: Jason Ekstrand
---
src/mesa/drivers/common/meta_tex_subimage.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
b/src/mesa/drivers/common/meta_tex_subimage.c
index d2474f5..90d78e5 100644
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@ -273,12 +273,17 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
format == GL_COLOR_INDEX)
return false;
- if (ctx->_ImageTransferState)
- return false;
-
-
+ /* Don't use meta path for readpixels in below conditions. */
if (!tex_image) {
rb = ctx->ReadBuffer->_ColorReadBuffer;
+
+ /* _mesa_get_readpixels_transfer_ops() includes the cases of read
+ * color clamping along with the ctx->_ImageTransferState.
+ */
+ if (_mesa_get_readpixels_transfer_ops(ctx, rb->Format, format,
+type, GL_FALSE))
+ return false;
+
if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format))
return false;
}
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 02/14] meta: Fix transfer operations check in meta pbo path for readpixels
On Fri, Jun 19, 2015 at 4:48 PM, Anuj Phogat wrote:
> On Thu, Jun 18, 2015 at 5:26 AM, Iago Toral wrote:
>> On Tue, 2015-06-16 at 11:15 -0700, Anuj Phogat wrote:
>>> Without this patch, arb_color_buffer_float-readpixels test fails, when
>>> forced to use meta pbo path.
>>>
>>> Signed-off-by: Anuj Phogat
>>> Cc:
>>> ---
>>> src/mesa/drivers/common/meta_tex_subimage.c | 10 ++
>>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
>>> b/src/mesa/drivers/common/meta_tex_subimage.c
>>> index d2474f5..00364f8 100644
>>> --- a/src/mesa/drivers/common/meta_tex_subimage.c
>>> +++ b/src/mesa/drivers/common/meta_tex_subimage.c
>>> @@ -273,12 +273,14 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
>>> GLuint dims,
>>> format == GL_COLOR_INDEX)
>>>return false;
>>>
>>> - if (ctx->_ImageTransferState)
>>> - return false;
>>> -
>>> -
>>
>> That test uses glReadPixels so it should call this with tex_image set to
>> NULL and it should flow through the if you have below. The call to
>> _mesa_get_readpixels_transfer_ops that you add below looks like it does
>> what we want for a pixel read from a framebuffer instead of simply
>> checking ctx->_ImageTransferState directly. I suppose this is what fixes
>> the test, right?
>>
> Right. Using _mesa_get_readpixels_transfer_ops() in place of simple
> ctx->_ImageTransferState check takes care of setting IMAGE_CLAMP_BIT
> in ctx->_ImageTransferState when GL_CLAMP_READ_COLOR is set.
> I'll add some explanation in commit message.
>
>> The patch also removes the ctx->_ImageTransferState check for the case
>> where we are reading from a real texture (tex_image != NULL), that seems
>> unrelated to fixing arb_color_buffer_float-readpixels... Looking at the
>> texture read code from getteximage.c it seems like this should be fine
>> since that file does not seem to use that field for anything either, so
>> I guess the check might not valid in this case.
>>
> After another look I realized that glGetTexImage() do need transfer
> operations check to fall back to software path. It's the lack of piglit
> tests that I couldn't catch it. Thanks for noticing it. I'll send out a v2.
>
Sent out v2 with a comment in the code and additional details as
commit message.
For glGetTexImage I verified that software path doesn't use
ctx->_ImageTransferState.
>> I think it would be nice if you updated the changelog to explain these
>> things.
>>
>> Iago
>>
>>> + /* Don't use meta path for readpixels in below conditions. */
>>> if (!tex_image) {
>>>rb = ctx->ReadBuffer->_ColorReadBuffer;
>>> +
>>> + if (_mesa_get_readpixels_transfer_ops(ctx, rb->Format, format,
>>> +type, GL_FALSE))
>>> + return false;
>>> +
>>>if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format))
>>> return false;
>>> }
>>
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] i965/gen9: Plugin the code for selecting YF/YS tiling on skl+
On Mon, Jun 22, 2015 at 5:23 PM, Anuj Phogat wrote:
> On Mon, Jun 22, 2015 at 2:53 PM, Ben Widawsky wrote:
>> On Wed, Jun 10, 2015 at 03:30:47PM -0700, Anuj Phogat wrote:
>>> Buffers with Yf/Ys tiling end up using meta upload / download
>>> paths or the blitter for cases where they used tiled_memcpy paths
>>> in case of Y tiling. This has exposed some bugs in meta path. To
>>> avoid any piglit regressions on SKL this patch keeps the Yf/Ys
>>> tiling disabled at the moment.
>>>
>>> V3: Make brw_miptree_choose_tr_mode() actually choose TRMODE. (Ben)
>>> Few cosmetic changes.
>>> V4: Get rid of brw_miptree_choose_tr_mode().
>>> Take care of all tile resource modes {Yf, Ys, none} for all
>>> generations at one place.
>>>
>>> Signed-off-by: Anuj Phogat
>>> Cc: Ben Widawsky
>>> ---
>>> src/mesa/drivers/dri/i965/brw_tex_layout.c | 97
>>> --
>>> 1 file changed, 79 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
>>> b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>>> index b9ac4cf..c0ef5cc 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>>> @@ -807,27 +807,88 @@ brw_miptree_layout(struct brw_context *brw,
>>> enum intel_miptree_tiling_mode requested,
>>> struct intel_mipmap_tree *mt)
>>> {
>>> - mt->tr_mode = INTEL_MIPTREE_TRMODE_NONE;
>>> + const unsigned bpp = mt->cpp * 8;
>>> + const bool is_tr_mode_yf_ys_allowed =
>>> + brw->gen >= 9 &&
>>> + !for_bo &&
>>> + !mt->compressed &&
>>> + /* Enable YF/YS tiling only for color surfaces because depth and
>>> + * stencil surfaces are not supported in blitter using fast copy
>>> + * blit and meta PBO upload, download paths. No other paths
>>> + * currently support Yf/Ys tiled surfaces.
>>> + * FIXME: Remove this restriction once we have a tiled_memcpy()
>>> + * path to do depth/stencil data upload/download to Yf/Ys tiled
>>> + * surfaces.
>>> + */
>>
>> I think it's more readable to move this comment above the variable
>> declaration.
>> Up to you though. Also I think "FINISHME" is the more appropriate
>> classification
>> for this type of thing.
>>
> Sure.
>>> + _mesa_is_format_color_format(mt->format) &&
>>> + (requested == INTEL_MIPTREE_TILING_Y ||
>>> + requested == INTEL_MIPTREE_TILING_ANY) &&
>>
>> This is where my tiling flags would have helped a bit since you should be
>> able
>> to do flags & Y_TILED :P
>>
> Yes, I will do a follow up patch to make use of that.
>>> + (bpp && is_power_of_two(bpp)) &&
>>> + /* FIXME: To avoid piglit regressions keep the Yf/Ys tiling
>>> + * disabled at the moment.
>>> + */
>>> + false;
>>
>> Also, "FINISHME"
>>
>>>
>>> - intel_miptree_set_alignment(brw, mt);
>>> - intel_miptree_set_total_width_height(brw, mt);
>>> + /* Lower index (Yf) is the higher priority mode */
>>> + const uint32_t tr_mode[3] = {INTEL_MIPTREE_TRMODE_YF,
>>> +INTEL_MIPTREE_TRMODE_YS,
>>> +INTEL_MIPTREE_TRMODE_NONE};
>>> + int i = is_tr_mode_yf_ys_allowed ? 0 : ARRAY_SIZE(tr_mode) - 1;
>>>
>>> - if (!mt->total_width || !mt->total_height) {
>>> - intel_miptree_release(&mt);
>>> - return;
>>> - }
>>> + while (i < ARRAY_SIZE(tr_mode)) {
>>> + if (brw->gen < 9)
>>> + assert(tr_mode[i] == INTEL_MIPTREE_TRMODE_NONE);
>>> + else
>>> + assert(tr_mode[i] == INTEL_MIPTREE_TRMODE_YF ||
>>> +tr_mode[i] == INTEL_MIPTREE_TRMODE_YS ||
>>> +tr_mode[i] == INTEL_MIPTREE_TRMODE_NONE);
>>>
>>> - /* On Gen9+ the alignment values are expressed in multiples of the block
>>> -* size
>>> -*/
>>> - if (brw->gen >= 9) {
>>> - unsigned int i, j;
>>> - _mesa_get_format_block_size(mt->format, &i, &j);
>>> - mt->align_w /= i;
>>> - mt->align_h /= j;
>>&g
Re: [Mesa-dev] [PATCH 1/3] i965: Rename brw_upload_item_data to brw_alloc_item_data
On Thu, Jun 25, 2015 at 5:45 AM, Topi Pohjolainen
wrote:
> and simplify the interface to take directly the size and to return
> the offset. The routine does nothing more than allocate, it doesn't
> upload anything.
>
> CC: Kenneth Graunke
> Signed-off-by: Topi Pohjolainen
> ---
> src/mesa/drivers/dri/i965/brw_state_cache.c | 19 ++-
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_state_cache.c
> b/src/mesa/drivers/dri/i965/brw_state_cache.c
> index 157b33d..97a41b9 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_cache.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_cache.c
> @@ -248,18 +248,17 @@ brw_try_upload_using_copy(struct brw_cache *cache,
> return false;
> }
>
> -static void
> -brw_upload_item_data(struct brw_cache *cache,
> -struct brw_cache_item *item,
> -const void *data)
> +static uint32_t
> +brw_alloc_item_data(struct brw_cache *cache, uint32_t size)
> {
> + uint32_t offset;
> struct brw_context *brw = cache->brw;
>
> /* Allocate space in the cache BO for our new program. */
> - if (cache->next_offset + item->size > cache->bo->size) {
> + if (cache->next_offset + size > cache->bo->size) {
>uint32_t new_size = cache->bo->size * 2;
>
> - while (cache->next_offset + item->size > new_size)
> + while (cache->next_offset + size > new_size)
> new_size *= 2;
>
>brw_cache_new_bo(cache, new_size);
> @@ -273,10 +272,12 @@ brw_upload_item_data(struct brw_cache *cache,
>brw_cache_new_bo(cache, cache->bo->size);
> }
>
> - item->offset = cache->next_offset;
> + offset = cache->next_offset;
>
> /* Programs are always 64-byte aligned, so set up the next one now */
> - cache->next_offset = ALIGN(item->offset + item->size, 64);
> + cache->next_offset = ALIGN(offset + size, 64);
> +
> + return offset;
> }
>
> void
> @@ -312,7 +313,7 @@ brw_upload_cache(struct brw_cache *cache,
> * compile to the thing in our backend.
> */
> if (!brw_try_upload_using_copy(cache, item, data, aux)) {
> - brw_upload_item_data(cache, item, data);
> + item->offset = brw_alloc_item_data(cache, data_size);
> }
>
> /* Set up the memory containing the key and aux_data */
> --
> 1.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] i965: Only write program to cache when it doesn't exist yet
On Thu, Jun 25, 2015 at 5:45 AM, Topi Pohjolainen
wrote:
> Current logic re-writes the same data when existing data is found.
> Not that this actually matters at the moment in practice, the
> contraint for finding matching data is too severe to ever allow
> data to be shared between two items in the cache.
>
> CC: Kenneth Graunke
> Signed-off-by: Topi Pohjolainen
> ---
> src/mesa/drivers/dri/i965/brw_state_cache.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_state_cache.c
> b/src/mesa/drivers/dri/i965/brw_state_cache.c
> index 97a41b9..d42b4b4 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_cache.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_cache.c
> @@ -314,6 +314,13 @@ brw_upload_cache(struct brw_cache *cache,
> */
> if (!brw_try_upload_using_copy(cache, item, data, aux)) {
>item->offset = brw_alloc_item_data(cache, data_size);
> +
> + /* Copy data to the buffer */
> + if (brw->has_llc) {
> + memcpy((char *)cache->bo->virtual + item->offset, data, data_size);
> + } else {
> + drm_intel_bo_subdata(cache->bo, item->offset, data_size, data);
> + }
> }
>
> /* Set up the memory containing the key and aux_data */
> @@ -332,13 +339,6 @@ brw_upload_cache(struct brw_cache *cache,
> cache->items[hash] = item;
> cache->n_items++;
>
> - /* Copy data to the buffer */
> - if (brw->has_llc) {
> - memcpy((char *) cache->bo->virtual + item->offset, data, data_size);
> - } else {
> - drm_intel_bo_subdata(cache->bo, item->offset, data_size, data);
> - }
> -
> *out_offset = item->offset;
> *(void **)out_aux = (void *)((char *)item->key + item->key_size);
> cache->brw->ctx.NewDriverState |= 1 << cache_id;
> --
> 1.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Sounds like a right thing to do. It's worth waiting to see comments
from other people before pushing it upstream.
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] i965: Stop aux data compare preventing program binary re-use
On Thu, Jun 25, 2015 at 5:45 AM, Topi Pohjolainen
wrote:
> Items in the program cache consist of three things: key, the data
> representing the instructions and auxiliary data representing
> uniform storage. The data consisting of instructions is stored into
> a drm buffer object while the key and the auxiliary data reside in
> malloced section. Now the cache uploading is equipped with a check
> that iterates over existing items and seeks to find a another item
> using identical instruction data than the one being just uploaded.
> If such is found there is no need to add another section into the
> drm buffer object holding identical copy of the existing one. The
> item just being uploaded should instead simply point to the same
> offset in the underlying drm buffer object.
>
> Unfortunately the check for the matching instruction data is
> coupled with a check for matching auxiliary data also. This
> effectively prevents the cache from ever containing two items
> that could share a section in the drm buffer object.
>
> The constraint for the instruction data and auxiliary data to
> match is, fortunately, unnecessary strong. When items are stored
> into the cache they will anyway contain their own copy of the
> auxiliary data (even if they matched - which they in real world
> never will). The only thing the items would be sharing is the
> instruction data and hence we should only check for that to match
> and nothing else.
>
> No piglit regression in jenkins.
>
> CC: Kenneth Graunke
> Signed-off-by: Topi Pohjolainen
> ---
> src/mesa/drivers/dri/i965/brw_state_cache.c | 52
> +++--
> 1 file changed, 20 insertions(+), 32 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_state_cache.c
> b/src/mesa/drivers/dri/i965/brw_state_cache.c
> index d42b4b4..a0ec280 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_cache.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_cache.c
> @@ -200,36 +200,23 @@ brw_cache_new_bo(struct brw_cache *cache, uint32_t
> new_size)
> }
>
> /**
> - * Attempts to find an item in the cache with identical data and aux
> - * data to use
> + * Attempts to find an item in the cache with identical data.
> */
> -static bool
> -brw_try_upload_using_copy(struct brw_cache *cache,
> - struct brw_cache_item *result_item,
> - const void *data,
> - const void *aux)
> +static const struct brw_cache_item *
> +brw_lookup_prog(const struct brw_cache *cache,
> +enum brw_cache_id cache_id,
> +const void *data, unsigned data_size)
> {
> - struct brw_context *brw = cache->brw;
> + const struct brw_context *brw = cache->brw;
> int i;
> - struct brw_cache_item *item;
> + const struct brw_cache_item *item;
>
> for (i = 0; i < cache->size; i++) {
>for (item = cache->items[i]; item; item = item->next) {
> -const void *item_aux = item->key + item->key_size;
> int ret;
>
> -if (item->cache_id != result_item->cache_id ||
> -item->size != result_item->size ||
> -item->aux_size != result_item->aux_size) {
> - continue;
> -}
> -
> - if (cache->aux_compare[result_item->cache_id]) {
> -if (!cache->aux_compare[result_item->cache_id](item_aux, aux))
> - continue;
> - } else if (memcmp(item_aux, aux, item->aux_size) != 0) {
> +if (item->cache_id != cache_id || item->size != data_size)
> continue;
> -}
>
> if (!brw->has_llc)
> drm_intel_bo_map(cache->bo, false);
> @@ -239,13 +226,11 @@ brw_try_upload_using_copy(struct brw_cache *cache,
> if (ret)
> continue;
>
> -result_item->offset = item->offset;
> -
> -return true;
> +return item;
>}
> }
>
> - return false;
> + return NULL;
> }
>
> static uint32_t
> @@ -294,6 +279,8 @@ brw_upload_cache(struct brw_cache *cache,
> {
> struct brw_context *brw = cache->brw;
> struct brw_cache_item *item = CALLOC_STRUCT(brw_cache_item);
> + const struct brw_cache_item *matching_data =
> + brw_lookup_prog(cache, cache_id, data, data_size);
> GLuint hash;
> void *tmp;
>
> @@ -305,14 +292,15 @@ brw_upload_cache(struct brw_cache *cache,
> hash = hash_key(item);
> item->hash = hash;
>
> - /* If we can find a matching prog/prog_data combo in the cache
> -* already, then reuse the existing stuff. This will mean not
> -* flagging CACHE_NEW_* when transitioning between the two
> -* equivalent hash keys. This is notably useful for programs
> -* generating shaders at runtime, where multiple shaders may
> -* compile to the thing in our backend.
> + /* If we can find a matching prog in the cache already, then reuse the
> +* existing stuff without creating new copy into the underlying buffer
> +* object. This is notably useful for programs generating shaders at
Re: [Mesa-dev] [PATCH v3 17/18] i965: refactor miptree alignment calculation code
64 */
> - return align < 64 ? 64 : align;
> - }
>
> /* Broadwell only supports VALIGN of 4, 8, and 16. The BSpec says 4
> * should always be used, except for stencil buffers, which should be 8.
> @@ -780,6 +725,13 @@ brw_miptree_layout(struct brw_context *brw,
>
> mt->tr_mode = INTEL_MIPTREE_TRMODE_NONE;
>
> + /**
> +* From the "Alignment Unit Size" section of various specs, namely:
> +* - Gen3 Spec: "Memory Data Formats" Volume, Section 1.20.1.4
> +* - i965 and G45 PRMs: Volume 1, Section 6.17.3.4.
> +* - Ironlake and Sandybridge PRMs: Volume 1, Part 1, Section 7.18.3.4
> +* - BSpec (for Ivybridge and slight variations in separate stencil)
> +*/
> if (brw->gen == 6 && mt->array_layout == ALL_SLICES_AT_EACH_LOD) {
>const GLenum base_format = _mesa_get_format_base_format(mt->format);
>gen6_hiz_or_stencil = _mesa_is_depth_or_stencil_format(base_format);
> @@ -809,6 +761,29 @@ brw_miptree_layout(struct brw_context *brw,
> mt->align_w = 128 / mt->cpp;
> mt->align_h = 32;
>}
> + } else if (mt->compressed) {
> + /* The hardware alignment requirements for compressed textures
> +* happen to match the block boundaries.
> +*/
> + _mesa_get_format_block_size(mt->format, &mt->align_w, &mt->align_h);
> +
> + /* On Gen9+ we can pick our own alignment for compressed textures but
> it
> + * has to be a multiple of the block size. The minimum alignment we can
> + * pick is 4 so we effectively have to align to 4 times the block
> + * size
> + */
> + if (brw->gen >= 9) {
> + mt->align_w *= 4;
> + mt->align_h *= 4;
> + }
> + } else if (mt->format == MESA_FORMAT_S_UINT8) {
> + mt->align_w = 8;
> + mt->align_h = brw->gen >= 7 ? 8 : 4;
> + } else if (brw->gen >= 9 && mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE) {
> + /* XY_FAST_COPY_BLT doesn't support horizontal alignment < 32 or
> + * vertical alignment < 64. */
> + mt->align_w = MAX2(tr_mode_horizontal_texture_alignment(brw, mt), 32);
> + mt->align_h = MAX2(tr_mode_vertical_texture_alignment(brw, mt), 64);
> } else {
>mt->align_w =
> intel_horizontal_texture_alignment_unit(brw, mt, layout_flags);
> --
> 2.4.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
I'm fine with the changes. This patch will need rebasing.
You can move all of your changes in brw_miptree_layout()
to the function intel_miptree_set_alignment(). Let me know
if you face issues while rebasing.
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 15/18] i965: change the meaning of cpp for compressed textures
8 +100,12 @@ copy_image_with_blitter(struct brw_context *brw, > >dst_x /= (int)bw; >dst_y /= (int)bh; > - > - /* Inside of the miptree, the x offsets are stored in pixels while > - * the y offsets are stored in blocks. We need to scale just the x > - * offset. > - */ > - dst_image_x /= bw; > } > dst_x += dst_image_x; > dst_y += dst_image_y; > > return intelEmitCopyBlit(brw, > -cpp, > +src_mt->cpp, > src_mt->pitch, > src_mt->bo, src_mt->offset, > src_mt->tiling, > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > index b47f49d0..4f92935 100644 > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > @@ -314,15 +314,7 @@ intel_miptree_create_layout(struct brw_context *brw, > mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_NO_MCS; > mt->disable_aux_buffers = (layout_flags & MIPTREE_LAYOUT_DISABLE_AUX) != > 0; > exec_list_make_empty(&mt->hiz_map); > - > - /* The cpp is bytes per (1, blockheight)-sized block for compressed > -* textures. This is why you'll see divides by blockheight all over > -*/ > - unsigned bw, bh; > - _mesa_get_format_block_size(format, &bw, &bh); > - assert(_mesa_get_format_bytes(mt->format) % bw == 0); > - mt->cpp = _mesa_get_format_bytes(mt->format) / bw; > - > + mt->cpp = _mesa_get_format_bytes(format); > mt->num_samples = num_samples; > mt->compressed = _mesa_is_format_compressed(format); > mt->msaa_layout = INTEL_MSAA_LAYOUT_NONE; > @@ -1214,7 +1206,7 @@ intel_miptree_copy_slice(struct brw_context *brw, >unsigned int i, j; >_mesa_get_format_block_size(dst_mt->format, &i, &j); >height = ALIGN_NPOT(height, j) / j; > - width = ALIGN_NPOT(width, i); > + width = ALIGN_NPOT(width, i) / i; > } > > /* If it's a packed depth/stencil buffer with separate stencil, the blit > @@ -2043,6 +2035,7 @@ intel_miptree_map_gtt(struct brw_context *brw, > _mesa_get_format_block_size(mt->format, &bw, &bh); > assert(y % bh == 0); > y /= bh; > + x /= bw; > > base = intel_miptree_map_raw(brw, mt) + mt->offset; > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > index bde6daa..7101afa 100644 > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > @@ -390,7 +390,7 @@ struct intel_mipmap_tree > */ > GLuint physical_width0, physical_height0, physical_depth0; > > - GLuint cpp; /**< bytes per pixel */ > + GLuint cpp; /**< bytes per pixel (or bytes per block if compressed) */ > GLuint num_samples; > bool compressed; > > -- > 2.4.2 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev With no piglit regressions: Reviewed-by: Anuj Phogat ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 16/18] i965: enable ASTC support for Skylake
On Mon, Jun 22, 2015 at 4:02 PM, Nanley Chery wrote:
> From: Nanley Chery
>
> v2: remove OES ASTC extension reference.
>
> Signed-off-by: Nanley Chery
> ---
> src/mesa/drivers/dri/i965/intel_extensions.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c
> b/src/mesa/drivers/dri/i965/intel_extensions.c
> index 365b4b8..cc793e5 100644
> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> @@ -354,6 +354,11 @@ intelInitExtensions(struct gl_context *ctx)
>ctx->Extensions.ARB_stencil_texturing = true;
> }
>
> + if (brw->gen >= 9) {
> + ctx->Extensions.KHR_texture_compression_astc_ldr = true;
> + ctx->Extensions.KHR_texture_compression_astc_hdr = true;
> + }
> +
> if (ctx->API == API_OPENGL_CORE)
>ctx->Extensions.ARB_base_instance = true;
> if (ctx->API != API_OPENGL_CORE)
> --
> 2.4.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 10/18] i965/surface_formats: add support for 2D ASTC surface formats
x, x, x, x, x, ASTC_LDR_2D_6x5_U8sRGB)
> + SF(90, 90, x, x, x, x, x, x, x, ASTC_LDR_2D_6x6_U8sRGB)
> + SF(90, 90, x, x, x, x, x, x, x, ASTC_LDR_2D_8x5_U8sRGB)
> + SF(90, 90, x, x, x, x, x, x, x, ASTC_LDR_2D_8x6_U8sRGB)
> + SF(90, 90, x, x, x, x, x, x, x, ASTC_LDR_2D_8x8_U8sRGB)
> + SF(90, 90, x, x, x, x, x, x, x, ASTC_LDR_2D_10x5_U8sRGB)
> + SF(90, 90, x, x, x, x, x, x, x, ASTC_LDR_2D_10x6_U8sRGB)
> + SF(90, 90, x, x, x, x, x, x, x, ASTC_LDR_2D_10x8_U8sRGB)
> + SF(90, 90, x, x, x, x, x, x, x, ASTC_LDR_2D_10x10_U8sRGB)
> + SF(90, 90, x, x, x, x, x, x, x, ASTC_LDR_2D_12x10_U8sRGB)
> + SF(90, 90, x, x, x, x, x, x, x, ASTC_LDR_2D_12x12_U8sRGB)
> };
As we discussed change 90 to 80.
> #undef x
> #undef Y
> @@ -503,6 +531,35 @@ brw_format_for_mesa_format(mesa_format mesa_format)
>[MESA_FORMAT_BPTC_RGB_SIGNED_FLOAT] = BRW_SURFACEFORMAT_BC6H_SF16,
>[MESA_FORMAT_BPTC_RGB_UNSIGNED_FLOAT] = BRW_SURFACEFORMAT_BC6H_UF16,
>
> + [MESA_FORMAT_ASTC_4x4_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_4x4_FLT16,
> + [MESA_FORMAT_ASTC_5x4_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_5x4_FLT16,
> + [MESA_FORMAT_ASTC_5x5_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_5x5_FLT16,
> + [MESA_FORMAT_ASTC_6x5_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_6x5_FLT16,
> + [MESA_FORMAT_ASTC_6x6_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_6x6_FLT16,
> + [MESA_FORMAT_ASTC_8x5_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_8x5_FLT16,
> + [MESA_FORMAT_ASTC_8x6_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_8x6_FLT16,
> + [MESA_FORMAT_ASTC_8x8_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_8x8_FLT16,
> + [MESA_FORMAT_ASTC_10x5_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_10x5_FLT16,
> + [MESA_FORMAT_ASTC_10x6_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_10x6_FLT16,
> + [MESA_FORMAT_ASTC_10x8_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_10x8_FLT16,
> + [MESA_FORMAT_ASTC_10x10_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_10x10_FLT16,
> + [MESA_FORMAT_ASTC_12x10_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_12x10_FLT16,
> + [MESA_FORMAT_ASTC_12x12_RGBA] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_12x12_FLT16,
> + [MESA_FORMAT_ASTC_4x4_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_4x4_U8sRGB,
> + [MESA_FORMAT_ASTC_5x4_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_5x4_U8sRGB,
> + [MESA_FORMAT_ASTC_5x5_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_5x5_U8sRGB,
> + [MESA_FORMAT_ASTC_6x5_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_6x5_U8sRGB,
> + [MESA_FORMAT_ASTC_6x6_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_6x6_U8sRGB,
> + [MESA_FORMAT_ASTC_8x5_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_8x5_U8sRGB,
> + [MESA_FORMAT_ASTC_8x6_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_8x6_U8sRGB,
> + [MESA_FORMAT_ASTC_8x8_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_8x8_U8sRGB,
> + [MESA_FORMAT_ASTC_10x5_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_10x5_U8sRGB,
> + [MESA_FORMAT_ASTC_10x6_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_10x6_U8sRGB,
> + [MESA_FORMAT_ASTC_10x8_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_10x8_U8sRGB,
> + [MESA_FORMAT_ASTC_10x10_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_10x10_U8sRGB,
> + [MESA_FORMAT_ASTC_12x10_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_12x10_U8sRGB,
> + [MESA_FORMAT_ASTC_12x12_SRGB8_ALPHA8] =
> BRW_SURFACEFORMAT_ASTC_LDR_2D_12x12_U8sRGB,
> +
>[MESA_FORMAT_A_SNORM8] = 0,
>[MESA_FORMAT_L_SNORM8] = 0,
>[MESA_FORMAT_L8A8_SNORM] = 0,
> @@ -768,6 +825,29 @@ translate_tex_format(struct brw_context *brw,
>}
>return brw_format_for_mesa_format(mesa_format);
>
> + case MESA_FORMAT_ASTC_4x4_RGBA:
> + case MESA_FORMAT_ASTC_5x4_RGBA:
> + case MESA_FORMAT_ASTC_5x5_RGBA:
> + case MESA_FORMAT_ASTC_6x5_RGBA:
> + case MESA_FORMAT_ASTC_6x6_RGBA:
> + case MESA_FORMAT_ASTC_8x5_RGBA:
> + case MESA_FORMAT_ASTC_8x6_RGBA:
> + case MESA_FORMAT_ASTC_8x8_RGBA:
> + case MESA_FORMAT_ASTC_10x5_RGBA:
> + case MESA_FORMAT_ASTC_10x6_RGBA:
> + case MESA_FORMAT_ASTC_10x8_RGBA:
> + case MESA_FORMAT_ASTC_10x10_RGBA:
> + case MESA_FORMAT_ASTC_12x10_RGBA:
> + case MESA_FORMAT_ASTC_12x12_RGBA: {
> + GLuint brw_fmt = brw_format_for_mesa_format(mesa_format);
> +
> + /* Process this texture using the Full Profile if possible. */
Just say enable HDR/Full profile bit on SKL+.
> + if (brw->gen >= 9)
> +
Re: [Mesa-dev] [PATCH] i965/surface_formats: add support for 2D ASTC surface formats
On Tue, Jun 30, 2015 at 4:15 PM, Nanley Chery wrote:
> From: Nanley Chery
>
> Define two-thirds of the 2D Intel ASTC surface formats (LDR-only). This allows
> a 1-to-1 mapping from the mesa format to the Intel format.
>
> ASTC textures will default to being processed in LDR mode. If there is
> hardware support for HDR/Full mode and the texture is not sRGB, add the
> format bit necessary to process it in HDR/Full mode.
>
> v2: remove extra newlines.
> v3: follow existing coding style in translate_tex_format().
> v4: expound on the GEN9_SURFACE_ASTC_HDR_FORMAT_BIT comment.
> update SF table - ASTC is actually supported in Gen8.
>
> Reviewed-by: Anuj Phogat
> Signed-off-by: Nanley Chery
> ---
> src/mesa/drivers/dri/i965/brw_defines.h | 32 +
> src/mesa/drivers/dri/i965/brw_surface_formats.c | 87
> +
> 2 files changed, 119 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> b/src/mesa/drivers/dri/i965/brw_defines.h
> index 66b9abc..61e9b71 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -504,6 +504,38 @@
> #define BRW_SURFACEFORMAT_R8G8B8_UINT0x1C8
> #define BRW_SURFACEFORMAT_R8G8B8_SINT0x1C9
> #define BRW_SURFACEFORMAT_RAW0x1FF
> +
> +#define GEN9_SURFACE_ASTC_HDR_FORMAT_BIT 0x100
> +
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_4x4_U8sRGB 0x200
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_5x4_U8sRGB 0x208
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_5x5_U8sRGB 0x209
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_6x5_U8sRGB 0x211
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_6x6_U8sRGB 0x212
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_8x5_U8sRGB 0x221
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_8x6_U8sRGB 0x222
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_8x8_U8sRGB 0x224
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_10x5_U8sRGB0x231
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_10x6_U8sRGB0x232
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_10x8_U8sRGB0x234
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_10x10_U8sRGB 0x236
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_12x10_U8sRGB 0x23E
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_12x12_U8sRGB 0x23F
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_4x4_FLT16 0x240
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_5x4_FLT16 0x248
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_5x5_FLT16 0x249
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_6x5_FLT16 0x251
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_6x6_FLT16 0x252
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_8x5_FLT16 0x261
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_8x6_FLT16 0x262
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_8x8_FLT16 0x264
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_10x5_FLT16 0x271
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_10x6_FLT16 0x272
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_10x8_FLT16 0x274
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_10x10_FLT160x276
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_12x10_FLT160x27E
> +#define BRW_SURFACEFORMAT_ASTC_LDR_2D_12x12_FLT160x27F
> +
> #define BRW_SURFACE_FORMAT_SHIFT 18
> #define BRW_SURFACE_FORMAT_MASKINTEL_MASK(26, 18)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> index 0501606..2546ff8 100644
> --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> @@ -307,6 +307,34 @@ const struct surface_format_info surface_formats[] = {
> SF( x, x, x, x, x, x, x, x, x, ETC2_EAC_SRGB8_A8)
> SF( x, x, x, x, x, x, x, x, x, R8G8B8_UINT)
> SF( x, x, x, x, x, x, x, x, x, R8G8B8_SINT)
> + SF(80, 80, x, x, x, x, x, x, x, ASTC_LDR_2D_4x4_FLT16)
> + SF(80, 80, x, x, x, x, x, x, x, ASTC_LDR_2D_5x4_FLT16)
> + SF(80, 80, x, x, x, x, x, x, x, ASTC_LDR_2D_5x5_FLT16)
> + SF(80, 80, x, x, x, x, x, x, x, ASTC_LDR_2D_6x5_FLT16)
> + SF(80, 80, x, x, x, x, x, x, x, ASTC_LDR_2D_6x6_FLT16)
> + SF(80, 80, x, x, x, x, x, x, x, ASTC_LDR_2D_8x5_FLT16)
> + SF(80, 80, x, x, x, x, x, x, x, ASTC_LDR_2D_8x6_FLT16)
> + SF(80, 80, x, x, x, x, x, x, x, ASTC_LDR_2D_8x8_FLT16)
> + SF(80, 80, x, x, x, x, x, x, x, ASTC_LDR_2D_10x5_FLT16)
> + SF(80, 80, x, x, x, x, x, x, x, ASTC_LDR_2D_10x6_FLT16)
> + SF(80, 80, x, x, x, x, x, x, x, ASTC_LDR_2D_10x8_FLT16)
> + SF(80, 80, x, x, x, x, x, x, x, ASTC_LDR_2D_10x1
Re: [Mesa-dev] [PATCH] mesa: reset the source packing when creating temp transfer image
On Wed, Jul 1, 2015 at 12:18 PM, Ilia Mirkin wrote: > Commit 4b249d2ee (mesa: Handle transferOps in texstore_rgba) introduced > proper transferops handling, but in updating the source to the newly > allocated temporary image neglected to reset the source packing. Set it > to the default which should be appropriate for the floats used. > > Fixes: 4b249d2ee (mesa: Handle transferOps in texstore_rgba) > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91173 > Signed-off-by: Ilia Mirkin > Cc: "10.5 10.6" > --- > src/mesa/main/texstore.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/mesa/main/texstore.c b/src/mesa/main/texstore.c > index 1525205..37c0569 100644 > --- a/src/mesa/main/texstore.c > +++ b/src/mesa/main/texstore.c > @@ -787,6 +787,7 @@ texstore_rgba(TEXSTORE_PARAMS) >srcType = GL_FLOAT; >srcRowStride = srcWidth * 4 * sizeof(float); >srcMesaFormat = RGBA32_FLOAT; > + srcPacking = &ctx->DefaultPacking; > } > > src = (GLubyte *) > -- > 2.3.6 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev Reviewed-by: Anuj Phogat ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/skl: Set the pulls bary bit in 3DSTATE_PS_EXTRA
On Fri, Jul 3, 2015 at 5:15 AM, Neil Roberts wrote:
> On Gen9+ there is a new bit in 3DSTATE_PS_EXTRA that must be set if
> the shader sends a message to the pixel interpolator. This fixes the
> interpolateAt* tests on SKL, apart from interpolateatsample-nonconst
> but that is not implemented anywhere so it's not a regression.
> ---
> src/mesa/drivers/dri/i965/brw_context.h | 1 +
> src/mesa/drivers/dri/i965/brw_defines.h | 1 +
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 4
> src/mesa/drivers/dri/i965/gen8_ps_state.c | 3 +++
> 4 files changed, 9 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 3553f6e..7596139 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -415,6 +415,7 @@ struct brw_wm_prog_data {
> bool uses_pos_offset;
> bool uses_omask;
> bool uses_kill;
> + bool pulls_bary;
> uint32_t prog_offset_16;
>
> /**
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> b/src/mesa/drivers/dri/i965/brw_defines.h
> index 66b9abc..19489ab 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -2145,6 +2145,7 @@ enum brw_pixel_shader_computed_depth_mode {
> # define GEN8_PSX_SHADER_DISABLES_ALPHA_TO_COVERAGE (1 << 7)
> # define GEN8_PSX_SHADER_IS_PER_SAMPLE (1 << 6)
> # define GEN8_PSX_SHADER_COMPUTES_STENCIL (1 << 5)
> +# define GEN9_PSX_SHADER_PULLS_BARY (1 << 3)
> # define GEN8_PSX_SHADER_HAS_UAV(1 << 2)
> # define GEN8_PSX_SHADER_USES_INPUT_COVERAGE_MASK (1 << 1)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index bd71404..3ebc3a2 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -1481,6 +1481,10 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld,
> nir_intrinsic_instr *instr
> case nir_intrinsic_interp_var_at_centroid:
> case nir_intrinsic_interp_var_at_sample:
> case nir_intrinsic_interp_var_at_offset: {
> + assert(stage == MESA_SHADER_FRAGMENT);
> +
> + ((struct brw_wm_prog_data *) prog_data)->pulls_bary = true;
> +
>fs_reg dst_xy = bld.vgrf(BRW_REGISTER_TYPE_F, 2);
>
>/* For most messages, we need one reg of ignored data; the hardware
> diff --git a/src/mesa/drivers/dri/i965/gen8_ps_state.c
> b/src/mesa/drivers/dri/i965/gen8_ps_state.c
> index a88f109..d544509 100644
> --- a/src/mesa/drivers/dri/i965/gen8_ps_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_ps_state.c
> @@ -58,6 +58,9 @@ gen8_upload_ps_extra(struct brw_context *brw,
> if (prog_data->uses_omask)
>dw1 |= GEN8_PSX_OMASK_TO_RENDER_TARGET;
>
> + if (brw->gen >= 9 && prog_data->pulls_bary)
> + dw1 |= GEN9_PSX_SHADER_PULLS_BARY;
> +
> if (_mesa_active_fragment_shader_has_atomic_ops(&brw->ctx))
>dw1 |= GEN8_PSX_SHADER_HAS_UAV;
>
> --
> 1.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 5/5] i965/gen9: Allocate YF/YS tiled buffer objects
On Tue, Jul 7, 2015 at 2:35 AM, Kenneth Graunke wrote:
> On Tuesday, June 23, 2015 01:23:05 PM Anuj Phogat wrote:
>> In case of I915_TILING_{X,Y} we need to pass tiling format to libdrm
>> using drm_intel_bo_alloc_tiled(). But, In case of YF/YS tiled buffers
>> libdrm need not know about the tiling format because these buffers
>> don't have hardware support to be tiled or detiled through a fenced
>> region. libdrm still need to know buffer alignment value for its use
>> in kernel when resolving the relocation.
>>
>> Using drm_intel_bo_alloc_for_render() for YF/YS tiled buffers
>> satisfy both the above conditions.
>>
>> V2: Delete min/max buffer size restrictions not valid for i965+.
>> Remove redundant align to tile size statements.
>> Remove some redundant code now when there are no min/max buffer size.
>>
>> Signed-off-by: Anuj Phogat
>> Cc: Ben Widawsky
>> ---
>> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 62
>> +--
>> 1 file changed, 58 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> index 80c52f2..5bcb094 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> @@ -558,6 +558,48 @@ intel_lower_compressed_format(struct brw_context *brw,
>> mesa_format format)
>> }
>> }
>>
>> +/* This function computes Yf/Ys tiled bo size, alignment and pitch. */
>> +static uint64_t
>> +intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment,
>> +uint64_t *pitch)
>
> Hi Anuj,
>
> This patch has a subtle bug: you've specified pitch and stride to be
> uint64_t here, but below when you call it
>
> [snip]
>> @@ -616,11 +658,23 @@ intel_miptree_create(struct brw_context *brw,
>>alloc_flags |= BO_ALLOC_FOR_RENDER;
>>
>> unsigned long pitch;
>> - mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree", total_width,
>> - total_height, mt->cpp, &mt->tiling,
>> - &pitch, alloc_flags);
>> mt->etc_format = etc_format;
>> - mt->pitch = pitch;
>> +
>> + if (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE) {
>> + unsigned alignment = 0;
>> + unsigned long size;
>> + size = intel_get_yf_ys_bo_size(mt, &alignment, &pitch);
>
> ...you're passing a pointer to an unsigned long. On 32-bit builds,
> unsigned long is a 4 byte value, while uint64_t is 8 bytes. This could
> lead to stack corruption. (GCC warns about this during a 32-bit build.)
>
Thanks for noticing this Ken. I think I never did 32 bit build with these
patches :(.
> I assumed the solution was to make everything uint32_t, but apparently
> drm_intel_bo_alloc_tiled actually expects an unsigned long. So we can't
> change that.
>
How about changing the parameter type of pitch to unsigned long*
and types of size and stride to unsigned long? This fixes the 32 bit
build warnings.
> Then I looked at your code, and realized that nothing even uses the
> pitch value. Is there some point to the parameter existing at all?
>
pitch value is later assigned to mt->pitch. I could have avoided
passing &pitch parameter and instead assign mt->pitch in
drm_intel_bo_alloc_for_render(). But, I used the current approach
to keep mt->pitch assignments at a single place.
I'm working on some refactoring to make this code look better.
> --Ken
>
>> + assert(size);
>> + mt->bo = drm_intel_bo_alloc_for_render(brw->bufmgr, "miptree",
>> + size, alignment);
>> + mt->pitch = pitch;
>> + } else {
>> + mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree",
>> +total_width, total_height, mt->cpp,
>> +&mt->tiling, &pitch,
>> +alloc_flags);
>> + mt->pitch = pitch;
>> + }
>>
>> /* If the BO is too large to fit in the aperture, we need to use the
>> * BLT engine to support it. Prior to Sandybridge, the BLT paths can't
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 5/5] i965/gen9: Allocate YF/YS tiled buffer objects
On Tue, Jul 7, 2015 at 12:11 PM, Anuj Phogat wrote:
> On Tue, Jul 7, 2015 at 2:35 AM, Kenneth Graunke wrote:
>> On Tuesday, June 23, 2015 01:23:05 PM Anuj Phogat wrote:
>>> In case of I915_TILING_{X,Y} we need to pass tiling format to libdrm
>>> using drm_intel_bo_alloc_tiled(). But, In case of YF/YS tiled buffers
>>> libdrm need not know about the tiling format because these buffers
>>> don't have hardware support to be tiled or detiled through a fenced
>>> region. libdrm still need to know buffer alignment value for its use
>>> in kernel when resolving the relocation.
>>>
>>> Using drm_intel_bo_alloc_for_render() for YF/YS tiled buffers
>>> satisfy both the above conditions.
>>>
>>> V2: Delete min/max buffer size restrictions not valid for i965+.
>>> Remove redundant align to tile size statements.
>>> Remove some redundant code now when there are no min/max buffer size.
>>>
>>> Signed-off-by: Anuj Phogat
>>> Cc: Ben Widawsky
>>> ---
>>> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 62
>>> +--
>>> 1 file changed, 58 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>>> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>>> index 80c52f2..5bcb094 100644
>>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>>> @@ -558,6 +558,48 @@ intel_lower_compressed_format(struct brw_context *brw,
>>> mesa_format format)
>>> }
>>> }
>>>
>>> +/* This function computes Yf/Ys tiled bo size, alignment and pitch. */
>>> +static uint64_t
>>> +intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment,
>>> +uint64_t *pitch)
>>
>> Hi Anuj,
>>
>> This patch has a subtle bug: you've specified pitch and stride to be
>> uint64_t here, but below when you call it
>>
>> [snip]
>>> @@ -616,11 +658,23 @@ intel_miptree_create(struct brw_context *brw,
>>>alloc_flags |= BO_ALLOC_FOR_RENDER;
>>>
>>> unsigned long pitch;
>>> - mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree", total_width,
>>> - total_height, mt->cpp, &mt->tiling,
>>> - &pitch, alloc_flags);
>>> mt->etc_format = etc_format;
>>> - mt->pitch = pitch;
>>> +
>>> + if (mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE) {
>>> + unsigned alignment = 0;
>>> + unsigned long size;
>>> + size = intel_get_yf_ys_bo_size(mt, &alignment, &pitch);
>>
>> ...you're passing a pointer to an unsigned long. On 32-bit builds,
>> unsigned long is a 4 byte value, while uint64_t is 8 bytes. This could
>> lead to stack corruption. (GCC warns about this during a 32-bit build.)
>>
> Thanks for noticing this Ken. I think I never did 32 bit build with these
> patches :(.
>
>> I assumed the solution was to make everything uint32_t, but apparently
>> drm_intel_bo_alloc_tiled actually expects an unsigned long. So we can't
>> change that.
>>
> How about changing the parameter type of pitch to unsigned long*
> and types of size and stride to unsigned long? This fixes the 32 bit
> build warnings.
>
>> Then I looked at your code, and realized that nothing even uses the
>> pitch value. Is there some point to the parameter existing at all?
>>
> pitch value is later assigned to mt->pitch. I could have avoided
> passing &pitch parameter and instead assign mt->pitch in
> drm_intel_bo_alloc_for_render(). But, I used the current approach
Correction: assign mt->pitch in intel_get_yf_ys_bo_size()
> to keep mt->pitch assignments at a single place.
>
> I'm working on some refactoring to make this code look better.
>
>> --Ken
>>
>>> + assert(size);
>>> + mt->bo = drm_intel_bo_alloc_for_render(brw->bufmgr, "miptree",
>>> + size, alignment);
>>> + mt->pitch = pitch;
>>> + } else {
>>> + mt->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "miptree",
>>> +total_width, total_height, mt->cpp,
>>> +&mt->tiling, &pitch,
>>> +alloc_flags);
>>> + mt->pitch = pitch;
>>> + }
>>>
>>> /* If the BO is too large to fit in the aperture, we need to use the
>>> * BLT engine to support it. Prior to Sandybridge, the BLT paths can't
>>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] i965/gen9: Plugin the code for selecting YF/YS tiling on skl+
On Tue, Jul 7, 2015 at 2:47 AM, Kenneth Graunke wrote:
> On Wednesday, June 10, 2015 03:30:47 PM Anuj Phogat wrote:
>> Buffers with Yf/Ys tiling end up using meta upload / download
>> paths or the blitter for cases where they used tiled_memcpy paths
>> in case of Y tiling. This has exposed some bugs in meta path. To
>> avoid any piglit regressions on SKL this patch keeps the Yf/Ys
>> tiling disabled at the moment.
>>
>> V3: Make brw_miptree_choose_tr_mode() actually choose TRMODE. (Ben)
>> Few cosmetic changes.
>> V4: Get rid of brw_miptree_choose_tr_mode().
>> Take care of all tile resource modes {Yf, Ys, none} for all
>> generations at one place.
>>
>> Signed-off-by: Anuj Phogat
>> Cc: Ben Widawsky
>> ---
>> src/mesa/drivers/dri/i965/brw_tex_layout.c | 97
>> --
>> 1 file changed, 79 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
>> b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>> index b9ac4cf..c0ef5cc 100644
>> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
>> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>> @@ -807,27 +807,88 @@ brw_miptree_layout(struct brw_context *brw,
>> enum intel_miptree_tiling_mode requested,
>> struct intel_mipmap_tree *mt)
>> {
>> - mt->tr_mode = INTEL_MIPTREE_TRMODE_NONE;
>> + const unsigned bpp = mt->cpp * 8;
>> + const bool is_tr_mode_yf_ys_allowed =
>> + brw->gen >= 9 &&
>> + !for_bo &&
>> + !mt->compressed &&
>> + /* Enable YF/YS tiling only for color surfaces because depth and
>> + * stencil surfaces are not supported in blitter using fast copy
>> + * blit and meta PBO upload, download paths. No other paths
>> + * currently support Yf/Ys tiled surfaces.
>> + * FIXME: Remove this restriction once we have a tiled_memcpy()
>> + * path to do depth/stencil data upload/download to Yf/Ys tiled
>> + * surfaces.
>> + */
>> + _mesa_is_format_color_format(mt->format) &&
>> + (requested == INTEL_MIPTREE_TILING_Y ||
>> + requested == INTEL_MIPTREE_TILING_ANY) &&
>> + (bpp && is_power_of_two(bpp)) &&
>> + /* FIXME: To avoid piglit regressions keep the Yf/Ys tiling
>> + * disabled at the moment.
>> + */
>> + false;
>
> I must say, I was a bit surprised to see this land as is. You've got a
> lot of conditions there, only to finish them up with && false - with a
> comment saying that your code isn't passing Piglit yet. That doesn't
> really meet our usual qualifications for merging.
>
> Coverity also pointed out that your if (is_tr_mode_yf_ys_allowed) block
> below is dead code, issuing new warnings.
>
I agree I should have waited more before landing this patch and
especially this part of code you're pointing to doesn't look nice
upstream :(.
I'm happy to revert the changes related to 'is_tr_mode_yf_ys_allowed'
to avoid coverity warning or the whole patch if you think that's better?
As Ben suggested, I can use an env variable to enable Yf/Ys while
it's in development. Any thoughts?
> Forgive my ignorance, but what's the purpose of Yf/Ys tiling? My
> understanding was that Ys is primarily in support of a new OpenGL
> feature - GL_ARB_spare_texture(*) - which isn't yet enabled:
>
> https://www.opengl.org/registry/specs/ARB/sparse_texture.txt
>
> Is Yf tiling supposed to be more efficient than legacy Y-tiling? If so,
> then switching to it is an optimization, isn't it? We usually require
> data indicating some kind of performance improvement (any kind!) before
> landing a bunch of code for optimizations. Obviously that's pretty
> tricky with pre-release hardware, so I'd settle for "it's complete
> and functions correctly."
>
Looking at the Yf tiling details in the hw specs, It looks like a better
memory layout with possibility of performance improvement.
Considering that we're already using 4KB tiling patterns in our driver,
it made sense to try using Yf (4KB). I don't have much information
about the 64KB tiling use cases except that they might be useful in
cases of high resolution textures.
> At any rate, it's merged, and hopefully you're able to get it working...
>
>>
>> - intel_miptree_set_alignment(brw, mt);
>> - intel_miptree_set_total_width_height(brw, mt);
>> + /* Lower index (Yf) is the higher priority mode */
>> + const uin
[Mesa-dev] [PATCH] i965: Fix 32 bit build warnings in intel_get_yf_ys_bo_size()
Along with fixing the type of pitch parameter, patch also changes
the types of few local variables and function return type.
Warnings fixed are:
intel_mipmap_tree.c:671:7: warning: passing argument 3 of
'intel_get_yf_ys_bo_size' from incompatible pointer type
intel_mipmap_tree.c:563:1: note: expected 'uint64_t *' but
argument is of type 'long unsigned int *'
Reported-by: Kenneth Graunke
Signed-off-by: Anuj Phogat
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index fb896a9..1529651 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -559,14 +559,14 @@ intel_lower_compressed_format(struct brw_context *brw,
mesa_format format)
}
/* This function computes Yf/Ys tiled bo size, alignment and pitch. */
-static uint64_t
+static unsigned long
intel_get_yf_ys_bo_size(struct intel_mipmap_tree *mt, unsigned *alignment,
-uint64_t *pitch)
+unsigned long *pitch)
{
const uint32_t bpp = mt->cpp * 8;
const uint32_t aspect_ratio = (bpp == 16 || bpp == 64) ? 2 : 1;
uint32_t tile_width, tile_height;
- uint64_t stride, size, aligned_y;
+ unsigned long stride, size, aligned_y;
assert(mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE);
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/5] i965: Push miptree tiling request into flags
On Tue, Jul 14, 2015 at 9:56 AM, Ben Widawsky
wrote:
> With the last few patches a way was provided to influence lower layer miptree
> layout and allocation decisions via flags (replacing bools). For simplicity, I
> chose not to touch the tiling requests because the change was slightly less
> mechanical than replacing the bools.
>
> The goal is to organize the code so we can continue to add new parameters and
> tiling types while minimizing risk to the existing code, and not having to
> constantly add new function parameters.
>
> v2: Rebased on Anuj's recent Yf/Ys changes
> Fix non-msrt MCS allocation (was only happening in gen8 case before)
>
> Cc: Anuj Phogat
> Cc: Chad Versace
> Signed-off-by: Ben Widawsky
> ---
> src/mesa/drivers/dri/i965/brw_tex_layout.c | 21 ++--
> src/mesa/drivers/dri/i965/intel_fbo.c | 6 ++--
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 45
> +-
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 15 -
> src/mesa/drivers/dri/i965/intel_tex.c | 2 +-
> src/mesa/drivers/dri/i965/intel_tex_image.c| 3 +-
> src/mesa/drivers/dri/i965/intel_tex_validate.c | 5 +--
> 7 files changed, 50 insertions(+), 47 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> index 389834f..a12b4af 100644
> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> @@ -614,8 +614,8 @@ brw_miptree_layout_texture_3d(struct brw_context *brw,
> */
> static uint32_t
> brw_miptree_choose_tiling(struct brw_context *brw,
> - enum intel_miptree_tiling_mode requested,
> - const struct intel_mipmap_tree *mt)
> + const struct intel_mipmap_tree *mt,
> + uint32_t layout_flags)
> {
> if (mt->format == MESA_FORMAT_S_UINT8) {
>/* The stencil buffer is W tiled. However, we request from the kernel a
> @@ -624,15 +624,18 @@ brw_miptree_choose_tiling(struct brw_context *brw,
>return I915_TILING_NONE;
> }
>
> + /* Do not support changing the tiling for miptrees with pre-allocated
> BOs. */
> + assert((layout_flags & MIPTREE_LAYOUT_FOR_BO) == 0);
> +
> /* Some usages may want only one type of tiling, like depth miptrees (Y
> * tiled), or temporary BOs for uploading data once (linear).
> */
> - switch (requested) {
> - case INTEL_MIPTREE_TILING_ANY:
> + switch (layout_flags & MIPTREE_LAYOUT_ALLOC_ANY_TILED) {
> + case MIPTREE_LAYOUT_ALLOC_ANY_TILED:
>break;
> - case INTEL_MIPTREE_TILING_Y:
> + case MIPTREE_LAYOUT_ALLOC_YTILED:
>return I915_TILING_Y;
> - case INTEL_MIPTREE_TILING_NONE:
> + case MIPTREE_LAYOUT_ALLOC_LINEAR:
>return I915_TILING_NONE;
> }
>
> @@ -835,7 +838,6 @@ intel_miptree_can_use_tr_mode(const struct
> intel_mipmap_tree *mt)
> void
> brw_miptree_layout(struct brw_context *brw,
> struct intel_mipmap_tree *mt,
> - enum intel_miptree_tiling_mode requested,
> uint32_t layout_flags)
> {
> const unsigned bpp = mt->cpp * 8;
> @@ -852,8 +854,7 @@ brw_miptree_layout(struct brw_context *brw,
>!(layout_flags & MIPTREE_LAYOUT_FOR_BO) &&
>!mt->compressed &&
>_mesa_is_format_color_format(mt->format) &&
> - (requested == INTEL_MIPTREE_TILING_Y ||
> - requested == INTEL_MIPTREE_TILING_ANY) &&
> + (layout_flags & MIPTREE_LAYOUT_ALLOC_YTILED) &&
>(bpp && is_power_of_two(bpp)) &&
>/* FIXME: To avoid piglit regressions keep the Yf/Ys tiling
> * disabled at the moment.
> @@ -897,7 +898,7 @@ brw_miptree_layout(struct brw_context *brw,
>if (layout_flags & MIPTREE_LAYOUT_FOR_BO)
> break;
>
> - mt->tiling = brw_miptree_choose_tiling(brw, requested, mt);
> + mt->tiling = brw_miptree_choose_tiling(brw, mt, layout_flags);
>if (is_tr_mode_yf_ys_allowed) {
> if (intel_miptree_can_use_tr_mode(mt))
> break;
> diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c
> b/src/mesa/drivers/dri/i965/intel_fbo.c
> index 05e3f8b..26f895b 100644
> --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> @@ -1022,6 +1022,9 @@ intel_renderbuffer_move_to_temp(struct brw_context *brw,
> struct intel_mipmap_tree *new_mt;
> int width, height, depth;
>
> + uint32_t layout_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD |
> +
Re: [Mesa-dev] [PATCH 2/5] i965/miptree: Cleanup some of the miptree map logic
On Tue, Jul 14, 2015 at 9:56 AM, Ben Widawsky
wrote:
> At the crux of this change is moving whether or not we can even use the
> hardware
> blitter into the can_blit_slice check. Fundamentally this makes sense as
> blitting a slice is a subset in functionality of being able to use the blitter
> at all.
>
> NOTE: I think it's bad practice to have the assert in a function that is
> determining whether or not we should use the blitter, but I tried the
> alternatives, and they look worse IMO.
>
> Signed-off-by: Ben Widawsky
> ---
> src/mesa/drivers/dri/i965/intel_blit.c| 13 +
> src/mesa/drivers/dri/i965/intel_blit.h| 3 +++
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 27
> +--
> 3 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
> b/src/mesa/drivers/dri/i965/intel_blit.c
> index bc39053..c4701e3 100644
> --- a/src/mesa/drivers/dri/i965/intel_blit.c
> +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> @@ -241,6 +241,19 @@ intel_miptree_blit_compatible_formats(mesa_format src,
> mesa_format dst)
> return false;
> }
>
> +bool
> +intel_miptree_can_hw_blit(struct brw_context *brw, struct intel_mipmap_tree
> *mt)
> +{
> + if (mt->compressed)
> + return false;
> +
> + /* Prior to Sandybridge, the blitter can't handle Y tiling */
> + if (brw->gen < 6 && mt->tiling == I915_TILING_Y)
> + return false;
> +
> + return true;
> +}
> +
> /**
> * Implements a rectangular block transfer (blit) of pixels between two
> * miptrees.
> diff --git a/src/mesa/drivers/dri/i965/intel_blit.h
> b/src/mesa/drivers/dri/i965/intel_blit.h
> index c3d19a5..e60dd9b 100644
> --- a/src/mesa/drivers/dri/i965/intel_blit.h
> +++ b/src/mesa/drivers/dri/i965/intel_blit.h
> @@ -50,6 +50,9 @@ intelEmitCopyBlit(struct brw_context *brw,
>
> bool intel_miptree_blit_compatible_formats(mesa_format src, mesa_format dst);
>
> +bool intel_miptree_can_hw_blit(struct brw_context *brw,
> + struct intel_mipmap_tree *mt);
> +
> bool intel_miptree_blit(struct brw_context *brw,
> struct intel_mipmap_tree *src_mt,
> int src_level, int src_slice,
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 72fba49..1330c2f 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -2600,9 +2600,14 @@ intel_miptree_release_map(struct intel_mipmap_tree *mt,
> }
>
> static bool
> -can_blit_slice(struct intel_mipmap_tree *mt,
> +can_blit_slice(struct brw_context *brw,
> + struct intel_mipmap_tree *mt,
> unsigned int level, unsigned int slice)
> {
> +
> + if (!intel_miptree_can_hw_blit(brw, mt))
> + return false;
> +
> uint32_t image_x;
> uint32_t image_y;
> intel_miptree_get_image_offset(mt, level, slice, &image_x, &image_y);
> @@ -2624,20 +2629,22 @@ use_intel_mipree_map_blit(struct brw_context *brw,
>unsigned int slice)
> {
> if (brw->has_llc &&
> - /* It's probably not worth swapping to the blit ring because of
> - * all the overhead involved.
> - */
> !(mode & GL_MAP_WRITE_BIT) &&
> - !mt->compressed &&
> - (mt->tiling == I915_TILING_X ||
> -/* Prior to Sandybridge, the blitter can't handle Y tiling */
> -(brw->gen >= 6 && mt->tiling == I915_TILING_Y)) &&
> - can_blit_slice(mt, level, slice))
> + can_blit_slice(brw, mt, level, slice))
>return true;
>
> if (mt->tiling != I915_TILING_NONE &&
> mt->bo->size >= brw->max_gtt_map_object_size) {
> - assert(can_blit_slice(mt, level, slice));
> + /* XXX: This assertion is actually the final condition for platforms
> + * without SSE4.1. Returning false is not the right thing to do with
> + * the current code. On those platforms, the goal of this function is
> to give
> + * preference to the GTT, and at this point we've determined we cannot
> use
> + * the GTT, and we cannot blit, so we are out of options.
> + *
> + * NOTE: It should be possible to actually handle the case, but AFAIK,
> we
> + * never get this assertion.
> + */
> + assert(can_blit_slice(brw, mt, level, slice));
>return true;
> }
>
> --
> 2.4.5
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
This patch now allows using hw blitter with I915_TILING_NONE
which is not allowed for unexplained reason at present. So, we
have a bug fix in this patch. May be you should split this in to
two? Changes in the patch look fine to me.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/list
[Mesa-dev] [PATCH V2 12/14] meta: Fix reading luminance texture as rgba in _mesa_meta_pbo_GetTexSubImage()
After recent addition of pbo testing in piglit test getteximage-luminance,
it fails on i965. This patch makes a sub test pass.
This patch adds a clear color operation to meta pbo path, which I think is
better than falling back to software path.
V2: Fix color mask for GL_LUMINANCE_ALPHA
Signed-off-by: Anuj Phogat
Cc:
---
src/mesa/drivers/common/meta_tex_subimage.c | 36 +++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
b/src/mesa/drivers/common/meta_tex_subimage.c
index 13f8292..f4d5ac3 100644
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@ -28,6 +28,7 @@
#include "blend.h"
#include "bufferobj.h"
#include "buffers.h"
+#include "clear.h"
#include "fbobject.h"
#include "glformats.h"
#include "glheader.h"
@@ -278,8 +279,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
int full_height, image_height;
struct gl_texture_image *pbo_tex_image;
struct gl_renderbuffer *rb = NULL;
- GLenum status;
- bool success = false;
+ GLenum status, src_base_format;
+ bool success = false, clear_channels_to_zero = false;
+ float save_clear_color[4];
int z;
if (!_mesa_is_bufferobj(packing->BufferObj))
@@ -380,6 +382,27 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
GL_COLOR_BUFFER_BIT, GL_NEAREST))
goto fail;
+ src_base_format = tex_image ?
+ tex_image->_BaseFormat :
+ ctx->ReadBuffer->_ColorReadBuffer->_BaseFormat;
+
+ /* Depending on the base formats involved we might need to rebase some
+* values. For example if we download from a Luminance format to RGBA
+* format, we want G=0 and B=0.
+*/
+ clear_channels_to_zero =
+ _mesa_need_luminance_to_rgb_conversion(src_base_format,
+ pbo_tex_image->_BaseFormat);
+
+ if (clear_channels_to_zero) {
+ memcpy(save_clear_color, ctx->Color.ClearColor.f, 4 * sizeof(float));
+ /* Clear the Green, Blue channels. */
+ _mesa_ColorMask(GL_FALSE, GL_TRUE, GL_TRUE,
+ src_base_format != GL_LUMINANCE_ALPHA);
+ _mesa_ClearColor(0.0, 0.0, 0.0, 1.0);
+ _mesa_Clear(GL_COLOR_BUFFER_BIT);
+ }
+
for (z = 1; z < depth; z++) {
_mesa_meta_bind_fbo_image(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
tex_image, zoffset + z);
@@ -392,6 +415,15 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
GLuint dims,
0, z * image_height,
width, z * image_height + height,
GL_COLOR_BUFFER_BIT, GL_NEAREST);
+ if (clear_channels_to_zero)
+ _mesa_Clear(GL_COLOR_BUFFER_BIT);
+ }
+
+ /* Unmask the color channels and restore the saved clear color values. */
+ if (clear_channels_to_zero) {
+ _mesa_ColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
+ _mesa_ClearColor(save_clear_color[0], save_clear_color[1],
+ save_clear_color[2], save_clear_color[3]);
}
success = true;
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 02/14] meta: Fix transfer operations check in meta pbo path for readpixels
Iago, Jason: Patches 2 and 5 in this series depend on patches 1 and 4
respectively, Since you guys reviewed 2 and 5, would you also like to
review 1, 4 and/or other patches in this series?
Thanks
-Anuj
On Sun, Jun 28, 2015 at 11:29 PM, Iago Toral wrote:
> Reviewed-by: Iago Toral Quiroga
>
> On Fri, 2015-06-26 at 13:15 -0700, Anuj Phogat wrote:
>> Currently used ctx->_ImageTransferState check is not sufficient
>> because it doesn't include the read color clamping enabled with
>> GL_CLAMP_READ_COLOR. So, use the helper function
>> _mesa_get_readpixels_transfer_ops().
>>
>> Also, transfer operations don't affect glGetTexImage(). So, do
>> the check only for glReadPixles.
>>
>> Without this patch, arb_color_buffer_float-readpixels test fails, when
>> forced to use meta pbo path.
>>
>> V2: Add a comment and bump up the commit message.
>>
>> Signed-off-by: Anuj Phogat
>> Cc:
>> Cc: Iago Toral
>> Cc: Jason Ekstrand
>> ---
>> src/mesa/drivers/common/meta_tex_subimage.c | 13 +
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
>> b/src/mesa/drivers/common/meta_tex_subimage.c
>> index d2474f5..90d78e5 100644
>> --- a/src/mesa/drivers/common/meta_tex_subimage.c
>> +++ b/src/mesa/drivers/common/meta_tex_subimage.c
>> @@ -273,12 +273,17 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
>> GLuint dims,
>> format == GL_COLOR_INDEX)
>>return false;
>>
>> - if (ctx->_ImageTransferState)
>> - return false;
>> -
>> -
>> + /* Don't use meta path for readpixels in below conditions. */
>> if (!tex_image) {
>>rb = ctx->ReadBuffer->_ColorReadBuffer;
>> +
>> + /* _mesa_get_readpixels_transfer_ops() includes the cases of read
>> + * color clamping along with the ctx->_ImageTransferState.
>> + */
>> + if (_mesa_get_readpixels_transfer_ops(ctx, rb->Format, format,
>> +type, GL_FALSE))
>> + return false;
>> +
>>if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format))
>> return false;
>> }
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] Revert "i965/gen9: Plugin the code for selecting YF/YS tiling on skl+"
Commit c9dbdc0 introduced some dead code which is supposed to be used
once we have Yf/Ys tiling working and performing better. Ken reported
the issue that static analysis tool now shows warnings due to the dead
code. To fix these warnings, this patch reverts the changes made in
commit c9dbdc0.
It'll be better to add the Yf/Ys tiling selection code later, when we
are ready to use it.
Cc: Kenneth Graunke
Cc: Ben Widawsky
Signed-off-by: Anuj Phogat
---
src/mesa/drivers/dri/i965/brw_tex_layout.c | 110 +
1 file changed, 17 insertions(+), 93 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
b/src/mesa/drivers/dri/i965/brw_tex_layout.c
index 389834f..e35cb64 100644
--- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
@@ -804,109 +804,33 @@ intel_miptree_set_alignment(struct brw_context *brw,
}
}
-static void
-intel_miptree_release_levels(struct intel_mipmap_tree *mt)
-{
- unsigned int level = 0;
-
- for (level = mt->first_level; level <= mt->last_level; level++) {
- free(mt->level[level].slice);
- mt->level[level].slice = NULL;
- }
-}
-
-static bool
-intel_miptree_can_use_tr_mode(const struct intel_mipmap_tree *mt)
-{
- if (mt->tiling == I915_TILING_Y ||
- mt->tiling == (I915_TILING_Y | I915_TILING_X) ||
- mt->tr_mode == INTEL_MIPTREE_TRMODE_NONE) {
- /* FIXME: Don't allow YS tiling at the moment. Using 64KB tiling
- * for small textures might result in to memory wastage. Revisit
- * this condition when we have more information about the specific
- * cases where using YS over YF will be useful.
- */
- if (mt->tr_mode != INTEL_MIPTREE_TRMODE_YS)
- return true;
- }
- return false;
-}
-
void
brw_miptree_layout(struct brw_context *brw,
struct intel_mipmap_tree *mt,
enum intel_miptree_tiling_mode requested,
uint32_t layout_flags)
{
- const unsigned bpp = mt->cpp * 8;
- /* Enable YF/YS tiling only for color surfaces because depth and
-* stencil surfaces are not supported in blitter using fast copy
-* blit and meta PBO upload, download paths. No other paths
-* currently support Yf/Ys tiled surfaces.
-* FINISHME: Remove this restriction once we have a tiled_memcpy()
-* path to do depth/stencil data upload/download to Yf/Ys tiled
-* surfaces.
-*/
- const bool is_tr_mode_yf_ys_allowed =
- brw->gen >= 9 &&
- !(layout_flags & MIPTREE_LAYOUT_FOR_BO) &&
- !mt->compressed &&
- _mesa_is_format_color_format(mt->format) &&
- (requested == INTEL_MIPTREE_TILING_Y ||
- requested == INTEL_MIPTREE_TILING_ANY) &&
- (bpp && is_power_of_two(bpp)) &&
- /* FIXME: To avoid piglit regressions keep the Yf/Ys tiling
- * disabled at the moment.
- */
- false;
+ mt->tr_mode = INTEL_MIPTREE_TRMODE_NONE;
- /* Lower index (Yf) is the higher priority mode */
- const uint32_t tr_mode[3] = {INTEL_MIPTREE_TRMODE_YF,
-INTEL_MIPTREE_TRMODE_YS,
-INTEL_MIPTREE_TRMODE_NONE};
- int i = is_tr_mode_yf_ys_allowed ? 0 : ARRAY_SIZE(tr_mode) - 1;
-
- while (i < ARRAY_SIZE(tr_mode)) {
- if (brw->gen < 9)
- assert(tr_mode[i] == INTEL_MIPTREE_TRMODE_NONE);
- else
- assert(tr_mode[i] == INTEL_MIPTREE_TRMODE_YF ||
-tr_mode[i] == INTEL_MIPTREE_TRMODE_YS ||
-tr_mode[i] == INTEL_MIPTREE_TRMODE_NONE);
+ intel_miptree_set_alignment(brw, mt, layout_flags);
+ intel_miptree_set_total_width_height(brw, mt);
- mt->tr_mode = tr_mode[i];
- intel_miptree_set_alignment(brw, mt, layout_flags);
- intel_miptree_set_total_width_height(brw, mt);
-
- if (!mt->total_width || !mt->total_height) {
- intel_miptree_release(&mt);
- break;
- }
-
- /* On Gen9+ the alignment values are expressed in multiples of the
- * block size.
- */
- if (brw->gen >= 9) {
- unsigned int i, j;
- _mesa_get_format_block_size(mt->format, &i, &j);
- mt->align_w /= i;
- mt->align_h /= j;
- }
+ if (!mt->total_width || !mt->total_height) {
+ intel_miptree_release(&mt);
+ return;
+ }
- /* If there is already a BO, we cannot effect tiling modes */
- if (layout_flags & MIPTREE_LAYOUT_FOR_BO)
- break;
+ /* On Gen9+ the alignment values are expressed in multiples of the block
+* size
+*/
+ if (brw->gen >= 9) {
+ unsigned int i, j;
+ _mesa_get_format_block_size(mt->format, &i, &j);
+ mt->align_w /= i;
+ mt->align_h /= j;
+ }
+ if ((layout_flags & MIPTREE_LAYOUT_FOR_BO) == 0)
mt
Re: [Mesa-dev] [PATCH 01/14] meta: handle subimages in _mesa_meta_setup_texture_coords()
fset + width); > + coords1[1] = (float) yoffset; >coords1[2] = 0.0F; > - coords2[0] = (float) width; > - coords2[1] = (float) height; > + coords2[0] = (float) (xoffset + width); > + coords2[1] = (float) (yoffset + height); >coords2[2] = 0.0F; > - coords3[0] = 0.0F; > - coords3[1] = (float) height; > + coords3[0] = (float) xoffset; > + coords3[1] = (float) (yoffset + height); >coords3[2] = 0.0F; >break; > case GL_TEXTURE_1D_ARRAY: > - coords0[0] = 0.0F; /* s */ > + coords0[0] = st[0][0]; /* s */ >coords0[1] = (float) slice; /* t */ >coords0[2] = 0.0F; /* r */ > - coords1[0] = 1.0f; > + coords1[0] = st[1][0]; >coords1[1] = (float) slice; >coords1[2] = 0.0F; > - coords2[0] = 1.0F; > + coords2[0] = st[2][0]; >coords2[1] = (float) slice; >coords2[2] = 0.0F; > - coords3[0] = 0.0F; > + coords3[0] = st[3][0]; >coords3[1] = (float) slice; >coords3[2] = 0.0F; >break; > @@ -3069,7 +3092,10 @@ decompress_texture_image(struct gl_context *ctx, > /* Silence valgrind warnings about reading uninitialized stack. */ > memset(verts, 0, sizeof(verts)); > > - _mesa_meta_setup_texture_coords(faceTarget, slice, width, height, depth, > + _mesa_meta_setup_texture_coords(faceTarget, slice, > + 0, 0, width, height, > + texImage->Width, texImage->Height, > + texImage->Depth, > verts[0].tex, > verts[1].tex, > verts[2].tex, > diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h > index e7d894d..f5b74c4 100644 > --- a/src/mesa/drivers/common/meta.h > +++ b/src/mesa/drivers/common/meta.h > @@ -594,9 +594,13 @@ _mesa_meta_alloc_texture(struct temp_texture *tex, > void > _mesa_meta_setup_texture_coords(GLenum faceTarget, > GLint slice, > +GLint xoffset, > +GLint yoffset, > GLint width, > GLint height, > -GLint depth, > +GLint total_width, > +GLint total_height, > +GLint total_depth, > GLfloat coords0[4], > GLfloat coords1[4], > GLfloat coords2[4], > diff --git a/src/mesa/drivers/common/meta_generate_mipmap.c > b/src/mesa/drivers/common/meta_generate_mipmap.c > index c1b6d3c..2877a2e 100644 > --- a/src/mesa/drivers/common/meta_generate_mipmap.c > +++ b/src/mesa/drivers/common/meta_generate_mipmap.c > @@ -317,7 +317,9 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, GLenum > target, > /* Setup texture coordinates */ > _mesa_meta_setup_texture_coords(faceTarget, > layer, > - 0, 0, 1, /* width, height never > used here */ > + 0, 0, /* xoffset, yoffset */ > + srcWidth, srcHeight, /* img size */ > + srcWidth, srcHeight, srcDepth, > verts[0].tex, > verts[1].tex, > verts[2].tex, > -- > 1.9.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev Reviewed-by: Anuj Phogat ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH 16/18] i965: Prevent coordinate overflow in intel_emit_linear_blit
On Mon, Jul 6, 2015 at 3:33 AM, Chris Wilson wrote:
> Fixes regression from
> commit 8c17d53823c77ac1c56b0548e4e54f69a33285f1
> Author: Kenneth Graunke
> Date: Wed Apr 15 03:04:33 2015 -0700
>
> i965: Make intel_emit_linear_blit handle Gen8+ alignment restrictions.
>
> which adjusted the coordinates to be relative to the nearest cacheline.
> However, this then offsets the coordinates by up to 63 and this may then
> cause them to overflow the BLT limits. For the well aligned large
> transfer case, we can use 32bpp pixels and so reduce the coordinates by
> 4 (versus the current 8bpp pixels). We also have to be more careful
> doing the last line just in case it may exceed the coordinate limit.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90734
> Signed-off-by: Chris Wilson
> Cc: Kenneth Graunke
> Cc: Ian Romanick
> Cc: Anuj Phogat
> Cc: mesa-sta...@lists.freedesktop.org
> ---
> src/mesa/drivers/dri/i965/intel_blit.c | 62
> --
> 1 file changed, 29 insertions(+), 33 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
> b/src/mesa/drivers/dri/i965/intel_blit.c
> index 7e81685..dbb53d8 100644
> --- a/src/mesa/drivers/dri/i965/intel_blit.c
> +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> @@ -762,35 +762,20 @@ intel_emit_linear_blit(struct brw_context *brw,
> int16_t src_x, dst_x;
> bool ok;
>
> - /* The pitch given to the GPU must be DWORD aligned, and
> -* we want width to match pitch. Max width is (1 << 15 - 1),
> -* rounding that down to the nearest DWORD is 1 << 15 - 4
> -*/
> - pitch = ROUND_DOWN_TO(MIN2(size, (1 << 15) - 1), 4);
> - height = (pitch == 0) ? 1 : size / pitch;
> - src_x = src_offset % 64;
> - dst_x = dst_offset % 64;
> - ok = intelEmitCopyBlit(brw, 1,
> - pitch, src_bo, src_offset - src_x, I915_TILING_NONE,
> - INTEL_MIPTREE_TRMODE_NONE,
> - pitch, dst_bo, dst_offset - dst_x, I915_TILING_NONE,
> - INTEL_MIPTREE_TRMODE_NONE,
> - src_x, 0, /* src x/y */
> - dst_x, 0, /* dst x/y */
> - pitch, height, /* w, h */
> - GL_COPY);
> - if (!ok)
> - _mesa_problem(ctx, "Failed to linear blit %dx%d\n", pitch, height);
> -
> - src_offset += pitch * height;
> - dst_offset += pitch * height;
> - src_x = src_offset % 64;
> - dst_x = dst_offset % 64;
> - size -= pitch * height;
> - assert (size < (1 << 15));
> - pitch = ALIGN(size, 4);
> -
> - if (size != 0) {
> + do {
> + /* The pitch given to the GPU must be DWORD aligned, and
> + * we want width to match pitch. Max width is (1 << 15 - 1),
> + * rounding that down to the nearest DWORD is 1 << 15 - 4
> + */
> + pitch = ROUND_DOWN_TO(MIN2(size, (1 << 15) - 64), 4);
I understand why you are subtracting 64 in above statement, it'll
be nice to update above comment explaining the reason.
> + height = (size < pitch || pitch == 0) ? 1 : size / pitch;
> +
> + src_x = src_offset % 64;
> + dst_x = dst_offset % 64;
> + pitch = ALIGN(MIN2(size, (1 << 15) - 64), 4);
> + assert(src_x + pitch < 1 << 15);
> + assert(dst_x + pitch < 1 << 15);
> +
>ok = intelEmitCopyBlit(brw, 1,
> pitch, src_bo, src_offset - src_x,
> I915_TILING_NONE,
> INTEL_MIPTREE_TRMODE_NONE,
> @@ -798,11 +783,22 @@ intel_emit_linear_blit(struct brw_context *brw,
> INTEL_MIPTREE_TRMODE_NONE,
> src_x, 0, /* src x/y */
> dst_x, 0, /* dst x/y */
> -size, 1, /* w, h */
> +MIN2(size, pitch), height, /* w, h */
> GL_COPY);
> - if (!ok)
> - _mesa_problem(ctx, "Failed to linear blit %dx%d\n", size, 1);
> - }
> + if (!ok) {
> +_mesa_problem(ctx, "Failed to linear blit %dx%d\n",
> + MIN2(size, pitch), height);
> +return;
> + }
> +
> + pitch *= height;
> + if (size <= pitch)
I think size < pitch will never be true. How about:
assert(size < pitch);
if (size == pitch)
> +return;
> +
> + src_offset += pitch;
> + dst_offset += pitch;
> + size -= pitch;
> + } while (1);
> }
>
> /**
> --
> 2.1.4
>
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH 16/18] i965: Prevent coordinate overflow in intel_emit_linear_blit
On Sat, Jul 18, 2015 at 1:24 AM, Chris Wilson wrote:
> On Fri, Jul 17, 2015 at 05:12:54PM -0700, Anuj Phogat wrote:
>> On Mon, Jul 6, 2015 at 3:33 AM, Chris Wilson
>> wrote:
>> > + do {
>> > + /* The pitch given to the GPU must be DWORD aligned, and
>> > + * we want width to match pitch. Max width is (1 << 15 - 1),
>> > + * rounding that down to the nearest DWORD is 1 << 15 - 4
>> > + */
>> > + pitch = ROUND_DOWN_TO(MIN2(size, (1 << 15) - 64), 4);
>> I understand why you are subtracting 64 in above statement, it'll
>> be nice to update above comment explaining the reason.
>
> We use the pitch to set the copy width, so the maximum x coordinate
> becomes src_x + pitch. Since src_x has a maximum value of 63, we want to
> make sure that pitch is less than 32627-63. Simplified above.
>
>> > + height = (size < pitch || pitch == 0) ? 1 : size / pitch;
> ...
>> > + pitch *= height;
>> > + if (size <= pitch)
>> I think size < pitch will never be true. How about:
>> assert(size < pitch);
>
> For a single row copy, size can be less than pitch.
right.
Reviewed-by: Anuj Phogat
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/14] meta: Abort meta pbo path if readpixels need signed-unsigned conversion
On Mon, Jul 20, 2015 at 5:10 AM, Iago Toral wrote: > On Fri, 2015-06-19 at 13:40 -0700, Anuj Phogat wrote: >> On Tue, Jun 16, 2015 at 9:21 PM, Jason Ekstrand wrote: >> > >> > On Jun 16, 2015 11:15, "Anuj Phogat" wrote: >> >> >> >> Without this patch, piglit test fbo_integer_readpixels_sint_uint fails, >> >> when >> >> forced to use the meta pbo path. >> >> >> >> Signed-off-by: Anuj Phogat >> >> Cc: >> >> --- >> >> src/mesa/drivers/common/meta_tex_subimage.c | 3 +++ >> >> 1 file changed, 3 insertions(+) >> >> >> >> diff --git a/src/mesa/drivers/common/meta_tex_subimage.c >> >> b/src/mesa/drivers/common/meta_tex_subimage.c >> >> index 00364f8..84cbc50 100644 >> >> --- a/src/mesa/drivers/common/meta_tex_subimage.c >> >> +++ b/src/mesa/drivers/common/meta_tex_subimage.c >> >> @@ -283,6 +283,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, >> >> GLuint dims, >> >> >> >>if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format)) >> >> return false; >> >> + >> >> + if (_mesa_need_signed_unsigned_int_conversion(rb->Format, format, >> >> type)) >> >> + return false; >> > >> > Hrm... This seems fishy. Isn't glBlitFramebuffers supposed to handle >> > format >> > conversion with integers? If so we should probably fix it rather than just >> > skip it for the meta pbo path. >> > >> As discussed offline, here is relevant text for glBlitFrameBuffer() from >> OpenGL 4.5 spec, section 18.3.1: >> "An INVALID_OPERATION error is generated if format conversions are not >> supported, which occurs under any of the following conditions: >> -The read buffer contains fixed-point or floating-point values and any draw >> buffer contains neither fixed-point nor floating-point values. >> -The read buffer contains unsigned integer values and any draw buffer does >> not contain unsigned integer values. >> - The read buffer contains signed integer values and any draw buffer does >> not contain signed integer values." >> >> I'll add a comment here explaining the reason to avoid meta path. > > Is this code going to run only for glBlitFramebuffer? I see this > function being called from code paths that implement glReadPixels and > glGetTexImage too. > _mesa_meta_pbo_GetTexSubImage() is used only for glReadPixels and glGetTexImage. I quoted the glBliFrameBuffer restriction above because the function is later using _mesa_meta_BlitFramebuffer(), which doesn't support some format conversions. > Iago > >> >> } >> >> >> >> /* For arrays, use a tall (height * depth) 2D texture but taking into >> >> -- >> >> 1.9.3 >> >> >> >> ___ >> >> 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 mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] mesa: check for identity matrix in _mesa_LoadMatrixf()
On Fri, Jul 17, 2015 at 5:48 PM, Brian Paul wrote:
> Google Earth often calls glLoadMatrixf() with an identity matrix instead
> of glLoadIdentity() to set the modelview and texture matrices. In many
> cases, the matrix is already the identity so the calls are redundant.
>
> By being a bit smarter in _mesa_LoadMatrixf() we can avoid quite a few
> matrix-related state validations.
> ---
> src/mesa/main/matrix.c | 23 ---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/main/matrix.c b/src/mesa/main/matrix.c
> index 5ac97f8..e6bdff8 100644
> --- a/src/mesa/main/matrix.c
> +++ b/src/mesa/main/matrix.c
> @@ -330,8 +330,16 @@ _mesa_LoadIdentity( void )
> void GLAPIENTRY
> _mesa_LoadMatrixf( const GLfloat *m )
> {
> + static const GLfloat identity[16] = {
> + 1.0f, 0.0f, 0.0f, 0.0f,
> + 0.0f, 1.0f, 0.0f, 0.0f,
> + 0.0f, 0.0f, 1.0f, 0.0f,
> + 0.0f, 0.0f, 0.0f, 1.0f };
> GET_CURRENT_CONTEXT(ctx);
> - if (!m) return;
> +
> + if (!m)
> + return;
> +
> if (MESA_VERBOSE & VERBOSE_API)
>_mesa_debug(ctx,
>"glLoadMatrix(%f %f %f %f, %f %f %f %f, %f %f %f %f, %f %f %f
> %f\n",
> @@ -341,8 +349,17 @@ _mesa_LoadMatrixf( const GLfloat *m )
>m[3], m[7], m[11], m[15]);
>
> FLUSH_VERTICES(ctx, 0);
> - _math_matrix_loadf( ctx->CurrentStack->Top, m );
> - ctx->NewState |= ctx->CurrentStack->DirtyFlag;
> +
> + if (memcmp(m, identity, sizeof(identity)) == 0) {
> + /* Setting the identity matrix */
> + if (_math_matrix_set_identity(ctx->CurrentStack->Top)) {
> + ctx->NewState |= ctx->CurrentStack->DirtyFlag;
> + }
> + }
> + else {
> + _math_matrix_loadf(ctx->CurrentStack->Top, m);
> + ctx->NewState |= ctx->CurrentStack->DirtyFlag;
> + }
> }
>
>
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Series is:
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] swrast: remove unneeded & operators in _swrast_choose_texture_sample_func()
gt; +return sample_linear_1d_array;
> }
> else {
> assert(sampler->MinFilter == GL_NEAREST);
> -return &sample_nearest_1d_array;
> +return sample_nearest_1d_array;
> }
>case GL_TEXTURE_2D_ARRAY_EXT:
> if (is_depth_texture(t)) {
> -return &sample_depth_texture;
> +return sample_depth_texture;
> }
> else if (needLambda) {
> -return &sample_lambda_2d_array;
> +return sample_lambda_2d_array;
> }
> else if (sampler->MinFilter == GL_LINEAR) {
> -return &sample_linear_2d_array;
> +return sample_linear_2d_array;
> }
> else {
> assert(sampler->MinFilter == GL_NEAREST);
> -return &sample_nearest_2d_array;
> +return sample_nearest_2d_array;
> }
>default:
> _mesa_problem(ctx,
> "invalid target in
> _swrast_choose_texture_sample_func");
> - return &null_sample_func;
> + return null_sample_func;
>}
> }
> }
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Anuj Phogat
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 07/14] mesa: Set green, blue channels to zero only for formats with these components
On Tue, Jul 21, 2015 at 12:50 AM, Iago Toral wrote: > On Tue, 2015-06-16 at 11:15 -0700, Anuj Phogat wrote: >> Signed-off-by: Anuj Phogat >> --- >> src/mesa/drivers/common/meta.c | 13 ++--- >> 1 file changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c >> index 214a68a..fceb25d 100644 >> --- a/src/mesa/drivers/common/meta.c >> +++ b/src/mesa/drivers/common/meta.c >> @@ -3132,9 +3132,16 @@ decompress_texture_image(struct gl_context *ctx, >> * returned as red and two-channel texture values are returned as >> * red/alpha. >> */ >> - if ((baseTexFormat == GL_LUMINANCE || >> - baseTexFormat == GL_LUMINANCE_ALPHA || >> - baseTexFormat == GL_INTENSITY) || >> + if (((baseTexFormat == GL_LUMINANCE || >> +baseTexFormat == GL_LUMINANCE_ALPHA || >> +baseTexFormat == GL_INTENSITY) && >> + (destBaseFormat == GL_RGBA || >> +destBaseFormat == GL_RGB || >> +destBaseFormat == GL_RG || >> +destBaseFormat == GL_GREEN || >> +destBaseFormat == GL_BLUE || >> +destBaseFormat == GL_BGRA || >> +destBaseFormat == GL_BGR)) || > > Is this needed to achieve correct behavior or just an optimization? I > would expect that if the dest format does not have G/B channels, setting > pixel transfer options for these channels would not have any functional > effect anyway. > This is just an optimization. We set pixel transfer operations based on these conditions and then call _mesa_ReadPixels, which falls back to slower path if transfer operations are set. I'll bump up the commit message of this patch. >>/* If we're reading back an RGB(A) texture (using glGetTexImage) >> as >> * luminance then we need to return L=tex(R). >> */ > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/14] meta: Abort meta pbo path if readpixels need signed-unsigned conversion
On Tue, Jul 21, 2015 at 1:36 AM, Iago Toral wrote: > On Tue, 2015-07-21 at 08:13 +0200, Iago Toral wrote: >> On Mon, 2015-07-20 at 10:56 -0700, Anuj Phogat wrote: >> > On Mon, Jul 20, 2015 at 5:10 AM, Iago Toral wrote: >> > > On Fri, 2015-06-19 at 13:40 -0700, Anuj Phogat wrote: >> > >> On Tue, Jun 16, 2015 at 9:21 PM, Jason Ekstrand >> > >> wrote: >> > >> > >> > >> > On Jun 16, 2015 11:15, "Anuj Phogat" wrote: >> > >> >> >> > >> >> Without this patch, piglit test fbo_integer_readpixels_sint_uint >> > >> >> fails, >> > >> >> when >> > >> >> forced to use the meta pbo path. >> > >> >> >> > >> >> Signed-off-by: Anuj Phogat >> > >> >> Cc: >> > >> >> --- >> > >> >> src/mesa/drivers/common/meta_tex_subimage.c | 3 +++ >> > >> >> 1 file changed, 3 insertions(+) >> > >> >> >> > >> >> diff --git a/src/mesa/drivers/common/meta_tex_subimage.c >> > >> >> b/src/mesa/drivers/common/meta_tex_subimage.c >> > >> >> index 00364f8..84cbc50 100644 >> > >> >> --- a/src/mesa/drivers/common/meta_tex_subimage.c >> > >> >> +++ b/src/mesa/drivers/common/meta_tex_subimage.c >> > >> >> @@ -283,6 +283,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context >> > >> >> *ctx, >> > >> >> GLuint dims, >> > >> >> >> > >> >>if (_mesa_need_rgb_to_luminance_conversion(rb->Format, >> > >> >> format)) >> > >> >> return false; >> > >> >> + >> > >> >> + if (_mesa_need_signed_unsigned_int_conversion(rb->Format, >> > >> >> format, >> > >> >> type)) >> > >> >> + return false; >> > >> > >> > >> > Hrm... This seems fishy. Isn't glBlitFramebuffers supposed to handle >> > >> > format >> > >> > conversion with integers? If so we should probably fix it rather >> > >> > than just >> > >> > skip it for the meta pbo path. >> > >> > >> > >> As discussed offline, here is relevant text for glBlitFrameBuffer() from >> > >> OpenGL 4.5 spec, section 18.3.1: >> > >> "An INVALID_OPERATION error is generated if format conversions are not >> > >> supported, which occurs under any of the following conditions: >> > >> -The read buffer contains fixed-point or floating-point values and any >> > >> draw >> > >> buffer contains neither fixed-point nor floating-point values. >> > >> -The read buffer contains unsigned integer values and any draw buffer >> > >> does >> > >> not contain unsigned integer values. >> > >> - The read buffer contains signed integer values and any draw buffer >> > >> does >> > >> not contain signed integer values." >> > >> >> > >> I'll add a comment here explaining the reason to avoid meta path. >> > > >> > > Is this code going to run only for glBlitFramebuffer? I see this >> > > function being called from code paths that implement glReadPixels and >> > > glGetTexImage too. >> > > >> > _mesa_meta_pbo_GetTexSubImage() is used only for glReadPixels and >> > glGetTexImage. I quoted the glBliFrameBuffer restriction above because >> > the function is later using _mesa_meta_BlitFramebuffer(), which doesn't >> > support some format conversions. >> >> If this function can be used to resolve ReadPixels and GetTexImage but >> the checks you add are *specific* to BlitFramebuffer, it does not look >> like this is the right place for them. Shouldn't you put them inside >> _mesa_meta_BlitFramebuffer instead? Otherwise they would affect to >> ReadPixels and GetTexImage too and I don't see the same restrictions >> applying to ReadPixels for example. We already have error checks in place for glBlitFrameBuffer(). Take a look at compatible_color_datatypes() in _mesa_blit_framebuffer(). > Specifically for ReadPixels I only see this in the spec: > > "An INVALID_OPERATION error is generated if format is an integer format > and the color buffer is not an integer
Re: [Mesa-dev] [PATCH 06/14] meta: Don't do fragment color clamping in case of ReadPixels
On Tue, Jul 21, 2015 at 1:18 AM, Iago Toral wrote:
> On Tue, 2015-07-21 at 09:24 +0200, Iago Toral wrote:
>> On Tue, 2015-06-16 at 11:15 -0700, Anuj Phogat wrote:
>> > Without this patch, piglit test arb_color_buffer_float-readpixels
>> > fails, when forced to use the meta pbo path.
>> >
>> > Signed-off-by: Anuj Phogat
>> > Cc:
>> > ---
>> > src/mesa/drivers/common/meta_tex_subimage.c | 5 +
>> > 1 file changed, 5 insertions(+)
>> >
>> > diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
>> > b/src/mesa/drivers/common/meta_tex_subimage.c
>> > index 84cbc50..ccb7dfb 100644
>> > --- a/src/mesa/drivers/common/meta_tex_subimage.c
>> > +++ b/src/mesa/drivers/common/meta_tex_subimage.c
>> > @@ -25,6 +25,7 @@
>> > *Jason Ekstrand
>> > */
>> >
>> > +#include "blend.h"
>> > #include "bufferobj.h"
>> > #include "buffers.h"
>> > #include "fbobject.h"
>> > @@ -305,6 +306,10 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
>> > GLuint dims,
>> > _mesa_meta_begin(ctx, ~(MESA_META_PIXEL_TRANSFER |
>> > MESA_META_PIXEL_STORE));
>> >
>> > + /* GL_CLAMP_FRAGMENT_COLOR doesn't affect ReadPixels. */
>> > + if (!tex_image && ctx->Extensions.ARB_color_buffer_float)
>> > + _mesa_ClampColor(GL_CLAMP_FRAGMENT_COLOR, GL_FALSE);
>> > +
>>
>> I think you need to restore the original value of
>> GL_CLAMP_FRAGMENT_COLOR after you are done with ReadPixels, otherwise
>> it looks like you would be changing the setting for future API calls
>> too.
>
> Oh, I just noticed that this setting is already being considered in the
> state flags passed to _mesa_meta_begin, so ignore that comment.
>
>> Also, does this only apply to ReadPixels and not to GetTexImage? That
>> looks odd to me.
You're right. This should apply to GetTexImage too. I'll send out v2 and
also add a piglit test for GetTexImage.
>>
>> Iago
>>
>> > _mesa_GenFramebuffers(2, fbos);
>> >
>> > if (tex_image && tex_image->TexObject->Target == GL_TEXTURE_1D_ARRAY) {
>>
>>
>> ___
>> 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
