date:20141111

Re: [Mesa-dev] Bogus bounds checking in api_validate.c?

2014-11-11 Thread Ian Romanick

On 11/10/2014 08:12 PM, Timothy Arceri wrote:
> On Mon, 2014-11-10 at 06:09 -0800, Ian Romanick wrote:
>> On 11/06/2014 11:40 PM, Kenneth Graunke wrote:
>>> On Thursday, November 06, 2014 08:09:18 PM Ian Romanick wrote:
 While working on some other things, I came across some bounds checking
 code in _mesa_validate_DrawElements (and related functions) in
 api_validate.c.

   /* use indices in the buffer object */
   /* make sure count doesn't go outside buffer bounds */
   if (index_bytes(type, count) > ctx->Array.VAO->IndexBufferObj->Size) 
 {
  _mesa_warning(ctx, "glDrawElements index out of buffer bounds");
  return GL_FALSE;
   }

 index_bytes calculates how many bytes of data "count" indices will
 occupy based on the type.  The problem is that this doesn't consider
 the base pointer.  As far as I can tell, if I had a 64 byte buffer
 object for my index data, and I did

 glDrawElements(GL_POINTS, 16, GL_UNSIGNED_INT, 60);

 _mesa_validate_DrawElements would say, "Ok!"

 Am I missing something, or is this just broken?
>>>
>>> It sure seems broken to me - but, thankfully, in a conservative fashion.  
>>> (It 
>>> will say some invalid things are OK, but won't say legal things are 
>>> invalid.)
>>>
>>> Software drivers may be relying on this working to avoid a crash.
>>>
>>> I checked the Ivybridge documentation, and found:
>>>
>>> "Software is responsible for ensuring that accesses outside the IB do not
>>>  occur. This is possible as software can compute the range of IB values
>>>  referenced by a 3DPRIMITIVE command (knowing the StartVertexLocation,
>>>  InstanceCount, and VerticesPerInstance values) and can then compare this
>>>  range to the IB extent."
>>>
>>> which makes it sound like an accurate computation is necessary.  But, right 
>>> below that, it says:
>>>
>>> "this field contains the address of the last valid byte in the index buffer.
>>>  Any index buffer reads past this address returns an index value of 0 (as if
>>>  the index buffer was zero-extended)."
>>>
>>> So the earlier statement is false; i965 will draw the in-bounds elements 
>>> correctly, and then repeat element 0 over and over for any out-of-bounds 
>>> data, 
>>> resulting in one strange primitive and a lot of degenerate ones.
>>>
>>> It's proabbly worth fixing, but I doubt it's critical either.
>>
>> Hmm... I came across this while looking at cachegrind traces of GL
>> applications.  Time spent in _mesa_validate_Draw* was non-trivial.
>> Since at least some hardware doesn't need this check, I think I want to
>> move it down into drivers that actually need the check... which is kind
>> of a bummer since I came up with a clever optimization for index_bytes.
> 
> I'm not sure what applications you looked at in cachegrind but OpenArena
> and Nexuiz both spend a lot of time in here. On my Ivybridge:
> 
> OpenArena 5.4% out of a total 27.94% in i965_dri.so
> Nexuiz 3.28% out of a total of 29.4% in i965_dri.so

I was looking at an apitrace of Tesseract.  It's not spending quite as
much time in these functions as OpenArena and Nexuiz, but it's still
quite a bit of time.

> For those not up to speed are you able to give a one line explanation of
> why some hardware can get away without this check?

DX10 requires that the hardware bounds-check accesses to buffer objects.
 In most cases DX10 requires that out-of-bounds reads return zero, and
out-of-bounds writes are always dropped.

>>> A more interesting thing to fix, I think, would be enforcing alignment 
>>> restrictions (i.e. your offset has to be a multiple of the IB element size).
>>
>> That would probably be useful in debug builds, but I'm pretty sure the
>> GL spec says the behavior is undefined specifically to avoid the check
>> in the hot path.
>>
>>> --Ken
>>
>> ___
>> 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] [Bug 84566] Unify the format conversion code

2014-11-11 Thread bugzilla-daemon

https://bugs.freedesktop.org/show_bug.cgi?id=84566

--- Comment #54 from Iago Toral  ---
Hi Jason,

I think we are close to completing this, so here is a summary of the current
state and how we intend to submit the patches for review, since we assume that
you will be reviewing most of these:

(In reply to Jason Ekstrand from comment #0)
(...)
> I would like to fix clean up this mess and unify a lot of the conversion
> code.  Here's what I've envisioned:
> 
>  1) Autogenerate all of the color packing/unpacking functions

Done.

>  2) Convert all of the packing/unpacking functions to pack/unpack a
> rectangle taking a width, height, and stride.  We can use 1x1 for
> single-pixel conversions.

We decided not to do this yet because in the end format conversion will mostly
happen through _mesa_format_convert now which already accepts a rect. Although
making the pack/unpack functions accept a rect would slightly simplify
_mesa_format_covert I think it won't really make a big difference anywhere
else, so we have kept postponing this until now. We can still do this change if
you think it is worth it.

>  3) Make swrast use the unpacking functions instead of its own texture
> sampling functions.
>  4) Add an array format enum that allows us to enumerate all possible array
> formats.  Between mesa_format and this array format, we should also be able
> to enumerate all of the GL datatypes.
>  5) Make a masater conversion function that takes a void*, format, width,
> height, and stride for both source and destination and just does the
> conversion.  If the above mentioned array format enum is distinct from the
> mesa_format enum, the function could be written to take a uint32_t type and
> accept either mesa_format or an array format in the same parameter.
>  6) Use the above master conversion function for TexSubimage, TexImage,
> GetTexImage, DrawPixels, and ReadPixels.  We still have to deal with pixel
> conversion, but it should vastly simplify all of them.

All this is done.

We have run piglit for i965, classic swrast, gallium swrast, llvmpipe, nouveau
and gallium radeon without regressions so I guess it should be quite okay.
Actually, there are now about 14 new piglit subtests that pass when compared to
master.

That said, I think there are probably many conversions that piglit won't cover,
so I would not be surprised if we have a bunch of regressions anyway, but
hopefully they won't be anything major.

We are now preparing the patch set for review now. It is a large collection of
patches (>50 patches), so we are thinking to split it in 3 series: one with
general bugfixes, another for the auto-generation of pack/unpack functions and
a final series with the master convert function.

Git says that the patches add ~5K LOC and remove ~15K LOC, so it a huge
cleanup.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Fix segfault in WebGL Conformance on Ivybridge

2014-11-11 Thread Ian Romanick

On 11/10/2014 04:02 PM, Chad Versace wrote:
> Fixes regression of WebGL Conformance test texture-size-limit [1] on
> Ivybridge Mobile GT2 0x0166 with Google Chrome R38.
> 
> Regression introduced by
> 
> commit 6c044231535b93c5d16404528946cad618d96bd9
> Author: Kenneth Graunke 
> Date:   Sun Feb 2 02:58:42 2014 -0800
> 
> i965: Bump GL_MAX_CUBE_MAP_TEXTURE_SIZE to 8192.
> 
> The test regressed because the pointer offset arithmetic in
> intel_miptree_map_gtt() overflows for large textures. The pointer
> arithmetic is not 64-bit safe.
> 
> This patch fixes the bugzilla ticket below on Ivybridge. This patch
> doesn't close the ticket, though, because the bug report is against
> Sandybridge, and QA cannot confirm the fix on that hardware.
> 
> [1] 
> https://github.com/KhronosGroup/WebGL/blob/52f0dc240f04dce31b1b8e2b8107fe2b8332dc90/sdk/tests/conformance/textures/texture-size-limit.html
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78770
> Fixes: Intel CHRMOS-1377
> Reported-by: Lu Hua 
> Signed-off-by: Chad Versace 
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 11 ++-
>  1 file changed, 10 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 8fda25d..24e217c 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1769,7 +1769,16 @@ intel_miptree_map_gtt(struct brw_context *brw,
>y += image_y;
>  
>map->stride = mt->pitch;
> -  map->ptr = base + y * map->stride + x * mt->cpp;
> +
> +  /* The variables in below pointer arithmetic are 32-bit. The arithmetic
> +   * overflows for large textures.  Therefore the cast to intptr_t is
> +   * needed.
> +   *
> +   * TODO(chadv): Fix this everywhere in i965 by fixing the signature of
> +   * intel_miptree_get_image_offset() to use intptr_t.
> +   */
> +  map->ptr = base + (intptr_t) y * (intptr_t) map->stride
> +  + (intptr_t) x * (intptr_t) mt->cpp;

I don't even want to know how long it took to track that down. :(

Is it sufficient to just use unsigned?  y and map->stride are the
problematic values here.  Isn't y bounded by (basically) 2*8192 and
map->stride bounded by 16*8192?  That sounds like 0x8000, which is
negative.

It seems like we could have similar problems in many places, and I'd
like to figure out what the correct coding practices are to avoid it.

> }
>  
> DBG("%s: %d,%d %dx%d from mt %p (%s) %d,%d = %p/%d\n", __FUNCTION__,
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965/cfg: Remove if_block/else_block.

2014-11-11 Thread Jason Ekstrand

Both patches are Reviewed-by: Jason Ekstrand 
On Nov 10, 2014 11:37 PM, "Matt Turner"  wrote:

> I used these in the SEL peephole, but they require extra tracking and
> fix ups. The SEL peephole can pretty easily find the blocks it needs
> without these.
> ---
>  src/mesa/drivers/dri/i965/brw_cfg.cpp   | 15 +--
>  src/mesa/drivers/dri/i965/brw_cfg.h |  8 
>  src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp |  8 
>  3 files changed, 1 insertion(+), 30 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_cfg.cpp
> b/src/mesa/drivers/dri/i965/brw_cfg.cpp
> index bb49a0a..02149e2 100644
> --- a/src/mesa/drivers/dri/i965/brw_cfg.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_cfg.cpp
> @@ -51,8 +51,7 @@ link(void *mem_ctx, bblock_t *block)
>  }
>
>  bblock_t::bblock_t(cfg_t *cfg) :
> -   cfg(cfg), start_ip(0), end_ip(0), num(0),
> -   if_block(NULL), else_block(NULL)
> +   cfg(cfg), start_ip(0), end_ip(0), num(0)
>  {
> instructions.make_empty();
> parents.make_empty();
> @@ -136,7 +135,6 @@ bblock_t::combine_with(bblock_t *that)
> }
>
> this->end_ip = that->end_ip;
> -   this->else_block = that->else_block;
> this->instructions.append_list(&that->instructions);
>
> this->cfg->remove_block(that);
> @@ -238,17 +236,6 @@ cfg_t::cfg_t(exec_list *instructions)
>   assert(cur_if->end()->opcode == BRW_OPCODE_IF);
>   assert(!cur_else || cur_else->end()->opcode == BRW_OPCODE_ELSE);
>
> - cur_if->if_block = cur_if;
> - cur_if->else_block = cur_else;
> -
> -if (cur_else) {
> -cur_else->if_block = cur_if;
> -cur_else->else_block = cur_else;
> - }
> -
> - cur->if_block = cur_if;
> - cur->else_block = cur_else;
> -
>  /* Pop the stack so we're in the previous if/else/endif */
>  cur_if = pop_stack(&if_stack);
>  cur_else = pop_stack(&else_stack);
> diff --git a/src/mesa/drivers/dri/i965/brw_cfg.h
> b/src/mesa/drivers/dri/i965/brw_cfg.h
> index 388d29e..48bca9f 100644
> --- a/src/mesa/drivers/dri/i965/brw_cfg.h
> +++ b/src/mesa/drivers/dri/i965/brw_cfg.h
> @@ -89,14 +89,6 @@ struct bblock_t {
> struct exec_list parents;
> struct exec_list children;
> int num;
> -
> -   /* If the current basic block ends in an IF or ELSE instruction, these
> will
> -* point to the basic blocks containing the other associated
> instruction.
> -*
> -* Otherwise they are NULL.
> -*/
> -   struct bblock_t *if_block;
> -   struct bblock_t *else_block;
>  };
>
>  static inline struct backend_instruction *
> diff --git a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
> b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
> index 4c9d7b9..03f838d 100644
> --- a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
> @@ -85,7 +85,6 @@ dead_control_flow_eliminate(backend_visitor *v)
>   }
>
>   if (else_inst) {
> -else_block->if_block->else_block = NULL;
>  else_inst->remove(else_block);
>   }
>
> @@ -102,13 +101,6 @@ dead_control_flow_eliminate(backend_visitor *v)
>   if (earlier_block &&
> earlier_block->can_combine_with(later_block)) {
>  earlier_block->combine_with(later_block);
>
> -foreach_block (block, v->cfg) {
> -   if (block->if_block == later_block)
> -  block->if_block = earlier_block;
> -   if (block->else_block == later_block)
> -  block->else_block = earlier_block;
> -}
> -
>  /* If ENDIF was in its own block, then we've now deleted it
> and
>   * merged the two surrounding blocks, the latter of which the
>   * __next block pointer was pointing to.
> --
> 2.0.4
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965/cfg: Remove if_block/else_block.

2014-11-11 Thread Jason Ekstrand

Two questions: Does it fix the bug? And did you ever figure out what was
going on there?
On Nov 11, 2014 6:58 AM, "Jason Ekstrand"  wrote:

> Both patches are Reviewed-by: Jason Ekstrand 
> On Nov 10, 2014 11:37 PM, "Matt Turner"  wrote:
>
>> I used these in the SEL peephole, but they require extra tracking and
>> fix ups. The SEL peephole can pretty easily find the blocks it needs
>> without these.
>> ---
>>  src/mesa/drivers/dri/i965/brw_cfg.cpp   | 15 +--
>>  src/mesa/drivers/dri/i965/brw_cfg.h |  8 
>>  src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp |  8 
>>  3 files changed, 1 insertion(+), 30 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_cfg.cpp
>> b/src/mesa/drivers/dri/i965/brw_cfg.cpp
>> index bb49a0a..02149e2 100644
>> --- a/src/mesa/drivers/dri/i965/brw_cfg.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_cfg.cpp
>> @@ -51,8 +51,7 @@ link(void *mem_ctx, bblock_t *block)
>>  }
>>
>>  bblock_t::bblock_t(cfg_t *cfg) :
>> -   cfg(cfg), start_ip(0), end_ip(0), num(0),
>> -   if_block(NULL), else_block(NULL)
>> +   cfg(cfg), start_ip(0), end_ip(0), num(0)
>>  {
>> instructions.make_empty();
>> parents.make_empty();
>> @@ -136,7 +135,6 @@ bblock_t::combine_with(bblock_t *that)
>> }
>>
>> this->end_ip = that->end_ip;
>> -   this->else_block = that->else_block;
>> this->instructions.append_list(&that->instructions);
>>
>> this->cfg->remove_block(that);
>> @@ -238,17 +236,6 @@ cfg_t::cfg_t(exec_list *instructions)
>>   assert(cur_if->end()->opcode == BRW_OPCODE_IF);
>>   assert(!cur_else || cur_else->end()->opcode == BRW_OPCODE_ELSE);
>>
>> - cur_if->if_block = cur_if;
>> - cur_if->else_block = cur_else;
>> -
>> -if (cur_else) {
>> -cur_else->if_block = cur_if;
>> -cur_else->else_block = cur_else;
>> - }
>> -
>> - cur->if_block = cur_if;
>> - cur->else_block = cur_else;
>> -
>>  /* Pop the stack so we're in the previous if/else/endif */
>>  cur_if = pop_stack(&if_stack);
>>  cur_else = pop_stack(&else_stack);
>> diff --git a/src/mesa/drivers/dri/i965/brw_cfg.h
>> b/src/mesa/drivers/dri/i965/brw_cfg.h
>> index 388d29e..48bca9f 100644
>> --- a/src/mesa/drivers/dri/i965/brw_cfg.h
>> +++ b/src/mesa/drivers/dri/i965/brw_cfg.h
>> @@ -89,14 +89,6 @@ struct bblock_t {
>> struct exec_list parents;
>> struct exec_list children;
>> int num;
>> -
>> -   /* If the current basic block ends in an IF or ELSE instruction,
>> these will
>> -* point to the basic blocks containing the other associated
>> instruction.
>> -*
>> -* Otherwise they are NULL.
>> -*/
>> -   struct bblock_t *if_block;
>> -   struct bblock_t *else_block;
>>  };
>>
>>  static inline struct backend_instruction *
>> diff --git a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>> b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>> index 4c9d7b9..03f838d 100644
>> --- a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>> @@ -85,7 +85,6 @@ dead_control_flow_eliminate(backend_visitor *v)
>>   }
>>
>>   if (else_inst) {
>> -else_block->if_block->else_block = NULL;
>>  else_inst->remove(else_block);
>>   }
>>
>> @@ -102,13 +101,6 @@ dead_control_flow_eliminate(backend_visitor *v)
>>   if (earlier_block &&
>> earlier_block->can_combine_with(later_block)) {
>>  earlier_block->combine_with(later_block);
>>
>> -foreach_block (block, v->cfg) {
>> -   if (block->if_block == later_block)
>> -  block->if_block = earlier_block;
>> -   if (block->else_block == later_block)
>> -  block->else_block = earlier_block;
>> -}
>> -
>>  /* If ENDIF was in its own block, then we've now deleted it
>> and
>>   * merged the two surrounding blocks, the latter of which the
>>   * __next block pointer was pointing to.
>> --
>> 2.0.4
>>
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 85419] [llvmpipe] Assertion fail with triangle strips

2014-11-11 Thread bugzilla-daemon

https://bugs.freedesktop.org/show_bug.cgi?id=85419

José Fonseca  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |INVALID

--- Comment #6 from José Fonseca  ---
The trace crashes on NVIDIA GL driver.

This is because you never call glEnable(PRIMITIVE_RESTART), so the driver
interprets the index 0x literally, causing a buffer overflow.


I also add to manually edit the trace and remove trailing \\ from one of the
shaders, as NVIDIA GLSL compiler fails otherwise.


AFAICS, you're getting undefined behavior because GL state is not being
properly set.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 0/2] clover: CL_MEM_HOST_* flags checks

2014-11-11 Thread EdB

This serie add CL_MEM_HOST_WRITE_ONLY, CL_MEM_HOST_READ_ONLY and
CL_MEM_HOST_NO_ACCESS checks

Those flags should be inherited when creting sub buffer.
So I change clGetMemObjectInfo to report them and the otherd that should also 
be reported.

EdB (2):
  clover: add CL_MEM_HOST_* flags checks
  clover: clGetMemObjectInfo report sub buffer inherited flags

 src/gallium/state_trackers/clover/api/memory.cpp   | 32 +++---
 src/gallium/state_trackers/clover/api/transfer.cpp | 32 --
 2 files changed, 57 insertions(+), 7 deletions(-)

-- 
1.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] clover: add CL_MEM_HOST_* flags checks

2014-11-11 Thread EdB

those flags have been introduced in OpenCL 1.2
---
 src/gallium/state_trackers/clover/api/memory.cpp   | 15 --
 src/gallium/state_trackers/clover/api/transfer.cpp | 32 --
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/src/gallium/state_trackers/clover/api/memory.cpp 
b/src/gallium/state_trackers/clover/api/memory.cpp
index a094e74..fe01d3f 100644
--- a/src/gallium/state_trackers/clover/api/memory.cpp
+++ b/src/gallium/state_trackers/clover/api/memory.cpp
@@ -44,13 +44,18 @@ clCreateBuffer(cl_context d_ctx, cl_mem_flags flags, size_t 
size,
 
if (flags & ~(CL_MEM_READ_WRITE | CL_MEM_WRITE_ONLY | CL_MEM_READ_ONLY |
  CL_MEM_USE_HOST_PTR | CL_MEM_ALLOC_HOST_PTR |
- CL_MEM_COPY_HOST_PTR))
+ CL_MEM_COPY_HOST_PTR | CL_MEM_HOST_WRITE_ONLY |
+ CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS ))
   throw error(CL_INVALID_VALUE);
 
if (util_bitcount(flags & (CL_MEM_READ_ONLY | CL_MEM_WRITE_ONLY |
   CL_MEM_READ_WRITE)) > 1)
   throw error(CL_INVALID_VALUE);
 
+   if (util_bitcount(flags & (CL_MEM_HOST_WRITE_ONLY | CL_MEM_HOST_READ_ONLY |
+  CL_MEM_HOST_NO_ACCESS)) > 1)
+  throw error(CL_INVALID_VALUE);
+
if ((flags & CL_MEM_USE_HOST_PTR) &&
(flags & (CL_MEM_COPY_HOST_PTR | CL_MEM_ALLOC_HOST_PTR)))
   throw error(CL_INVALID_VALUE);
@@ -76,6 +81,11 @@ clCreateSubBuffer(cl_mem d_mem, cl_mem_flags flags,
CL_MEM_WRITE_ONLY)))
   throw error(CL_INVALID_VALUE);
 
+   if (util_bitcount((flags | parent.flags()) &
+ (CL_MEM_HOST_WRITE_ONLY | CL_MEM_HOST_READ_ONLY |
+  CL_MEM_HOST_NO_ACCESS)) > 1)
+  throw error(CL_INVALID_VALUE);
+
if (op == CL_BUFFER_CREATE_TYPE_REGION) {
   auto reg = reinterpret_cast(op_info);
 
@@ -182,7 +192,8 @@ clGetSupportedImageFormats(cl_context d_ctx, cl_mem_flags 
flags,
 
if (flags & ~(CL_MEM_READ_WRITE | CL_MEM_WRITE_ONLY | CL_MEM_READ_ONLY |
  CL_MEM_USE_HOST_PTR | CL_MEM_ALLOC_HOST_PTR |
- CL_MEM_COPY_HOST_PTR))
+ CL_MEM_COPY_HOST_PTR | CL_MEM_HOST_WRITE_ONLY |
+ CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS ))
   throw error(CL_INVALID_VALUE);
 
if (r_buf && !r_count)
diff --git a/src/gallium/state_trackers/clover/api/transfer.cpp 
b/src/gallium/state_trackers/clover/api/transfer.cpp
index b8d7771..32bd47a 100644
--- a/src/gallium/state_trackers/clover/api/transfer.cpp
+++ b/src/gallium/state_trackers/clover/api/transfer.cpp
@@ -171,10 +171,30 @@ namespace {
/// Checks that the mapping flags are correct.
///
void
-   validate_flags(const cl_map_flags flags) {
+   validate_map_flags(const cl_map_flags flags, const cl_mem_flags mem_flags) {
   if ((flags & (CL_MAP_WRITE | CL_MAP_READ)) &&
   (flags & CL_MAP_WRITE_INVALIDATE_REGION))
  throw error(CL_INVALID_VALUE);
+
+  if ((flags & CL_MAP_READ) &&
+  (mem_flags & (CL_MEM_HOST_WRITE_ONLY | CL_MEM_HOST_NO_ACCESS)))
+ throw error(CL_MEM_HOST_WRITE_ONLY);
+
+  if ((flags & (CL_MAP_WRITE | CL_MAP_WRITE_INVALIDATE_REGION)) &&
+  (mem_flags & (CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS)))
+ throw error(CL_MEM_HOST_WRITE_ONLY);
+   }
+
+   void
+   validate_read_permission(const cl_mem_flags mem_flags) {
+  if (mem_flags & (CL_MEM_HOST_WRITE_ONLY | CL_MEM_HOST_NO_ACCESS))
+ throw error(CL_INVALID_OPERATION);
+   }
+
+   void
+   validate_write_permission(const cl_mem_flags mem_flags) {
+  if (mem_flags & (CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS))
+ throw error(CL_INVALID_OPERATION);
}
 
///
@@ -269,6 +289,7 @@ clEnqueueReadBuffer(cl_command_queue d_q, cl_mem d_mem, 
cl_bool blocking,
validate_common(q, deps);
validate_object(q, ptr, {}, obj_pitch, region);
validate_object(q, mem, obj_origin, obj_pitch, region);
+   validate_read_permission(mem.flags());
 
auto hev = create(
   q, CL_COMMAND_READ_BUFFER, deps,
@@ -298,6 +319,7 @@ clEnqueueWriteBuffer(cl_command_queue d_q, cl_mem d_mem, 
cl_bool blocking,
validate_common(q, deps);
validate_object(q, mem, obj_origin, obj_pitch, region);
validate_object(q, ptr, {}, obj_pitch, region);
+   validate_write_permission(mem.flags());
 
auto hev = create(
   q, CL_COMMAND_WRITE_BUFFER, deps,
@@ -334,6 +356,7 @@ clEnqueueReadBufferRect(cl_command_queue d_q, cl_mem d_mem, 
cl_bool blocking,
validate_common(q, deps);
validate_object(q, ptr, host_origin, host_pitch, region);
validate_object(q, mem, obj_origin, obj_pitch, region);
+   validate_read_permission(mem.flags());
 
auto hev = create(
   q, CL_COMMAND_READ_BUFFER_RECT, deps,
@@ -370,6 +393,7 @@ clEnqueueWriteBufferRect(cl_command_queue d_q, cl_mem 
d_mem, cl_bool blocking,
validate_common(q, deps);
validate_object(q, mem

[Mesa-dev] [PATCH 2/2] clover: clGetMemObjectInfo report sub buffer inherited flags

2014-11-11 Thread EdB

when call cbCreateSubBuffer when followings memory flags are not set
they should be inherited from parent buffer.
Report them when calling clGetMemObjectInfo

flags:
CL_MEM_READ_WRITE, CL_MEM_READ_ONLY, CL_MEM_WRITE_ONLY,
CL_MEM_USE_HOST_PTR, CL_MEM_ALLOC_HOST_PTR, CL_MEM_COPY_HOST_PTR,
CL_MEM_HOST_WRITE_ONLY, CL_MEM_HOST_READ_ONLY, CL_MEM_HOST_NO_ACCESS
---
 src/gallium/state_trackers/clover/api/memory.cpp | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/gallium/state_trackers/clover/api/memory.cpp 
b/src/gallium/state_trackers/clover/api/memory.cpp
index fe01d3f..4843cff 100644
--- a/src/gallium/state_trackers/clover/api/memory.cpp
+++ b/src/gallium/state_trackers/clover/api/memory.cpp
@@ -226,9 +226,22 @@ clGetMemObjectInfo(cl_mem d_mem, cl_mem_info param,
   buf.as_scalar() = mem.type();
   break;
 
-   case CL_MEM_FLAGS:
-  buf.as_scalar() = mem.flags();
+   case CL_MEM_FLAGS: {
+  cl_mem_flags flags = mem.flags();
+
+  //inherited flags
+  sub_buffer *sub = dynamic_cast(&mem);
+  if (sub) {
+ flags = flags | (sub->parent().flags() &
+ (CL_MEM_READ_WRITE | CL_MEM_READ_ONLY | CL_MEM_WRITE_ONLY |
+  CL_MEM_USE_HOST_PTR | CL_MEM_ALLOC_HOST_PTR |
+  CL_MEM_COPY_HOST_PTR |
+  CL_MEM_HOST_WRITE_ONLY | CL_MEM_HOST_READ_ONLY |
+  CL_MEM_HOST_NO_ACCESS));
+  }
+  buf.as_scalar() = flags;
   break;
+   }
 
case CL_MEM_SIZE:
   buf.as_scalar() = mem.size();
-- 
1.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 85419] [llvmpipe] Assertion fail with triangle strips

2014-11-11 Thread bugzilla-daemon

https://bugs.freedesktop.org/show_bug.cgi?id=85419

--- Comment #7 from James Evans  ---
I am so sorry for bugging you with this.  You are absolutely correct.

I completely missed the glEnable call.  Adding that fixed everything.

Thank you.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 84566] Unify the format conversion code

2014-11-11 Thread bugzilla-daemon

https://bugs.freedesktop.org/show_bug.cgi?id=84566

--- Comment #55 from Jason Ekstrand  ---
(In reply to Iago Toral from comment #54)
> Hi Jason,
> 
> I think we are close to completing this, so here is a summary of the current
> state and how we intend to submit the patches for review, since we assume
> that you will be reviewing most of these:
> 
> (In reply to Jason Ekstrand from comment #0)
> (...)
> > I would like to fix clean up this mess and unify a lot of the conversion
> > code.  Here's what I've envisioned:
> > 
> >  1) Autogenerate all of the color packing/unpacking functions
> 
> Done.
> 
> >  2) Convert all of the packing/unpacking functions to pack/unpack a
> > rectangle taking a width, height, and stride.  We can use 1x1 for
> > single-pixel conversions.
> 
> We decided not to do this yet because in the end format conversion will
> mostly happen through _mesa_format_convert now which already accepts a rect.
> Although making the pack/unpack functions accept a rect would slightly
> simplify _mesa_format_covert I think it won't really make a big difference
> anywhere else, so we have kept postponing this until now. We can still do
> this change if you think it is worth it.

That's fine.  It's not that big of a change.  Mostly, it's just a small
performance boost for really narrow conversions.

> >  3) Make swrast use the unpacking functions instead of its own texture
> > sampling functions.
> >  4) Add an array format enum that allows us to enumerate all possible array
> > formats.  Between mesa_format and this array format, we should also be able
> > to enumerate all of the GL datatypes.
> >  5) Make a masater conversion function that takes a void*, format, width,
> > height, and stride for both source and destination and just does the
> > conversion.  If the above mentioned array format enum is distinct from the
> > mesa_format enum, the function could be written to take a uint32_t type and
> > accept either mesa_format or an array format in the same parameter.
> >  6) Use the above master conversion function for TexSubimage, TexImage,
> > GetTexImage, DrawPixels, and ReadPixels.  We still have to deal with pixel
> > conversion, but it should vastly simplify all of them.
> 
> All this is done.
> 
> We have run piglit for i965, classic swrast, gallium swrast, llvmpipe,
> nouveau and gallium radeon without regressions so I guess it should be quite
> okay. Actually, there are now about 14 new piglit subtests that pass when
> compared to master.

Good work!  If it passes all of those, then I'm not too concerned about the
kinds of hidden bugs we hit on my first format conversion series.

> That said, I think there are probably many conversions that piglit won't
> cover, so I would not be surprised if we have a bunch of regressions anyway,
> but hopefully they won't be anything major.

I'm sure there's things piglit covers.  However, I'm going to give it a good
look over and between doing what "looks right" and passing piglit, I think
we've got it mostly covered.

> We are now preparing the patch set for review now. It is a large collection
> of patches (>50 patches), so we are thinking to split it in 3 series: one
> with general bugfixes, another for the auto-generation of pack/unpack
> functions and a final series with the master convert function.

That seems perfectly reasonable.

> Git says that the patches add ~5K LOC and remove ~15K LOC, so it a huge
> cleanup.

Nice!

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] clover: add CL_MEM_HOST_* flags checks

2014-11-11 Thread EdB

On Tuesday 11 November 2014 17:42:42 EdB wrote:
> those flags have been introduced in OpenCL 1.2
> ---
>  src/gallium/state_trackers/clover/api/memory.cpp   | 15 --
>  src/gallium/state_trackers/clover/api/transfer.cpp | 32
> -- 2 files changed, 42 insertions(+), 5 deletions(-)
> 
> diff --git a/src/gallium/state_trackers/clover/api/memory.cpp
> b/src/gallium/state_trackers/clover/api/memory.cpp index a094e74..fe01d3f
> 100644
> --- a/src/gallium/state_trackers/clover/api/memory.cpp
> +++ b/src/gallium/state_trackers/clover/api/memory.cpp
> @@ -44,13 +44,18 @@ clCreateBuffer(cl_context d_ctx, cl_mem_flags flags,
> size_t size,
> 
> if (flags & ~(CL_MEM_READ_WRITE | CL_MEM_WRITE_ONLY | CL_MEM_READ_ONLY |
> CL_MEM_USE_HOST_PTR | CL_MEM_ALLOC_HOST_PTR |
> - CL_MEM_COPY_HOST_PTR))
> + CL_MEM_COPY_HOST_PTR | CL_MEM_HOST_WRITE_ONLY |
> + CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS ))
>throw error(CL_INVALID_VALUE);
> 
> if (util_bitcount(flags & (CL_MEM_READ_ONLY | CL_MEM_WRITE_ONLY |
>CL_MEM_READ_WRITE)) > 1)
>throw error(CL_INVALID_VALUE);
> 
> +   if (util_bitcount(flags & (CL_MEM_HOST_WRITE_ONLY |
> CL_MEM_HOST_READ_ONLY | + 
> CL_MEM_HOST_NO_ACCESS)) > 1)
> +  throw error(CL_INVALID_VALUE);
> +
> if ((flags & CL_MEM_USE_HOST_PTR) &&
> (flags & (CL_MEM_COPY_HOST_PTR | CL_MEM_ALLOC_HOST_PTR)))
>throw error(CL_INVALID_VALUE);
> @@ -76,6 +81,11 @@ clCreateSubBuffer(cl_mem d_mem, cl_mem_flags flags,
> CL_MEM_WRITE_ONLY)))
>throw error(CL_INVALID_VALUE);
> 
> +   if (util_bitcount((flags | parent.flags()) &
> + (CL_MEM_HOST_WRITE_ONLY | CL_MEM_HOST_READ_ONLY |
> +  CL_MEM_HOST_NO_ACCESS)) > 1)
> +  throw error(CL_INVALID_VALUE);
> +
> if (op == CL_BUFFER_CREATE_TYPE_REGION) {
>auto reg = reinterpret_cast(op_info);
> 
> @@ -182,7 +192,8 @@ clGetSupportedImageFormats(cl_context d_ctx,
> cl_mem_flags flags,
> 
> if (flags & ~(CL_MEM_READ_WRITE | CL_MEM_WRITE_ONLY | CL_MEM_READ_ONLY |
> CL_MEM_USE_HOST_PTR | CL_MEM_ALLOC_HOST_PTR |
> - CL_MEM_COPY_HOST_PTR))
> + CL_MEM_COPY_HOST_PTR | CL_MEM_HOST_WRITE_ONLY |
> + CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS ))
>throw error(CL_INVALID_VALUE);
> 
> if (r_buf && !r_count)
> diff --git a/src/gallium/state_trackers/clover/api/transfer.cpp
> b/src/gallium/state_trackers/clover/api/transfer.cpp index b8d7771..32bd47a
> 100644
> --- a/src/gallium/state_trackers/clover/api/transfer.cpp
> +++ b/src/gallium/state_trackers/clover/api/transfer.cpp
> @@ -171,10 +171,30 @@ namespace {
> /// Checks that the mapping flags are correct.
> ///
> void
> -   validate_flags(const cl_map_flags flags) {
> +   validate_map_flags(const cl_map_flags flags, const cl_mem_flags
> mem_flags) { if ((flags & (CL_MAP_WRITE | CL_MAP_READ)) &&
>(flags & CL_MAP_WRITE_INVALIDATE_REGION))
>   throw error(CL_INVALID_VALUE);
> +
> +  if ((flags & CL_MAP_READ) &&
> +  (mem_flags & (CL_MEM_HOST_WRITE_ONLY | CL_MEM_HOST_NO_ACCESS)))
> + throw error(CL_MEM_HOST_WRITE_ONLY);
sorry, I recreate the patches series before sending it and forgot one change.
This should be : throw error(CL_INVALID_OPERATION);

> +
> +  if ((flags & (CL_MAP_WRITE | CL_MAP_WRITE_INVALIDATE_REGION)) &&
> +  (mem_flags & (CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS)))
> + throw error(CL_MEM_HOST_WRITE_ONLY);
same here : throw error(CL_INVALID_OPERATION);
> +   }
> +
> +   void
> +   validate_read_permission(const cl_mem_flags mem_flags) {
> +  if (mem_flags & (CL_MEM_HOST_WRITE_ONLY | CL_MEM_HOST_NO_ACCESS))
> + throw error(CL_INVALID_OPERATION);
> +   }
> +
> +   void
> +   validate_write_permission(const cl_mem_flags mem_flags) {
> +  if (mem_flags & (CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS))
> + throw error(CL_INVALID_OPERATION);
> }
> 
> ///
> @@ -269,6 +289,7 @@ clEnqueueReadBuffer(cl_command_queue d_q, cl_mem d_mem,
> cl_bool blocking, validate_common(q, deps);
> validate_object(q, ptr, {}, obj_pitch, region);
> validate_object(q, mem, obj_origin, obj_pitch, region);
> +   validate_read_permission(mem.flags());
> 
> auto hev = create(
>q, CL_COMMAND_READ_BUFFER, deps,
> @@ -298,6 +319,7 @@ clEnqueueWriteBuffer(cl_command_queue d_q, cl_mem d_mem,
> cl_bool blocking, validate_common(q, deps);
> validate_object(q, mem, obj_origin, obj_pitch, region);
> validate_object(q, ptr, {}, obj_pitch, region);
> +   validate_write_permission(mem.flags());
> 
> auto hev = create(
>q, CL_COMMAND_WRITE_BUFFER, deps,
> @@ -334,6 +356,7 @@ clEnqueueReadBufferRect(cl_command_queue d_q, cl_mem
> d_mem, cl_bool blocking, validate_common(q, deps);
> validate_o

[Mesa-dev] [Bug 72572] no error on draw call without vbo set

2014-11-11 Thread bugzilla-daemon

https://bugs.freedesktop.org/show_bug.cgi?id=72572

José Fonseca  changed:

   What|Removed |Added

   Priority|medium  |low
Summary|[llvmpipe] no error on draw |no error on draw call
   |call without vbo set|without vbo set
   Severity|normal  |enhancement

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 79706] [TRACKER] Mesa regression tracker

2014-11-11 Thread bugzilla-daemon

https://bugs.freedesktop.org/show_bug.cgi?id=79706
Bug 79706 depends on bug 80835, which changed state.

Bug 80835 Summary: [llvmpipe] piglit fs-struct-pad regression
https://bugs.freedesktop.org/show_bug.cgi?id=80835

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 80835] [llvmpipe] piglit fs-struct-pad regression

2014-11-11 Thread bugzilla-daemon

https://bugs.freedesktop.org/show_bug.cgi?id=80835

José Fonseca  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #1 from José Fonseca  ---
It passes now.  It must have been fixed since.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965/cfg: Remove if_block/else_block.

2014-11-11 Thread Jason Ekstrand

One more comment: These and the other two CFG fixes should probably be
promoted to stable.

On Tue, Nov 11, 2014 at 7:37 AM, Jason Ekstrand 
wrote:

> Two questions: Does it fix the bug? And did you ever figure out what was
> going on there?
> On Nov 11, 2014 6:58 AM, "Jason Ekstrand"  wrote:
>
>> Both patches are Reviewed-by: Jason Ekstrand 
>> On Nov 10, 2014 11:37 PM, "Matt Turner"  wrote:
>>
>>> I used these in the SEL peephole, but they require extra tracking and
>>> fix ups. The SEL peephole can pretty easily find the blocks it needs
>>> without these.
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_cfg.cpp   | 15 +--
>>>  src/mesa/drivers/dri/i965/brw_cfg.h |  8 
>>>  src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp |  8 
>>>  3 files changed, 1 insertion(+), 30 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_cfg.cpp
>>> b/src/mesa/drivers/dri/i965/brw_cfg.cpp
>>> index bb49a0a..02149e2 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_cfg.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_cfg.cpp
>>> @@ -51,8 +51,7 @@ link(void *mem_ctx, bblock_t *block)
>>>  }
>>>
>>>  bblock_t::bblock_t(cfg_t *cfg) :
>>> -   cfg(cfg), start_ip(0), end_ip(0), num(0),
>>> -   if_block(NULL), else_block(NULL)
>>> +   cfg(cfg), start_ip(0), end_ip(0), num(0)
>>>  {
>>> instructions.make_empty();
>>> parents.make_empty();
>>> @@ -136,7 +135,6 @@ bblock_t::combine_with(bblock_t *that)
>>> }
>>>
>>> this->end_ip = that->end_ip;
>>> -   this->else_block = that->else_block;
>>> this->instructions.append_list(&that->instructions);
>>>
>>> this->cfg->remove_block(that);
>>> @@ -238,17 +236,6 @@ cfg_t::cfg_t(exec_list *instructions)
>>>   assert(cur_if->end()->opcode == BRW_OPCODE_IF);
>>>   assert(!cur_else || cur_else->end()->opcode ==
>>> BRW_OPCODE_ELSE);
>>>
>>> - cur_if->if_block = cur_if;
>>> - cur_if->else_block = cur_else;
>>> -
>>> -if (cur_else) {
>>> -cur_else->if_block = cur_if;
>>> -cur_else->else_block = cur_else;
>>> - }
>>> -
>>> - cur->if_block = cur_if;
>>> - cur->else_block = cur_else;
>>> -
>>>  /* Pop the stack so we're in the previous if/else/endif */
>>>  cur_if = pop_stack(&if_stack);
>>>  cur_else = pop_stack(&else_stack);
>>> diff --git a/src/mesa/drivers/dri/i965/brw_cfg.h
>>> b/src/mesa/drivers/dri/i965/brw_cfg.h
>>> index 388d29e..48bca9f 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_cfg.h
>>> +++ b/src/mesa/drivers/dri/i965/brw_cfg.h
>>> @@ -89,14 +89,6 @@ struct bblock_t {
>>> struct exec_list parents;
>>> struct exec_list children;
>>> int num;
>>> -
>>> -   /* If the current basic block ends in an IF or ELSE instruction,
>>> these will
>>> -* point to the basic blocks containing the other associated
>>> instruction.
>>> -*
>>> -* Otherwise they are NULL.
>>> -*/
>>> -   struct bblock_t *if_block;
>>> -   struct bblock_t *else_block;
>>>  };
>>>
>>>  static inline struct backend_instruction *
>>> diff --git a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>>> b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>>> index 4c9d7b9..03f838d 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>>> @@ -85,7 +85,6 @@ dead_control_flow_eliminate(backend_visitor *v)
>>>   }
>>>
>>>   if (else_inst) {
>>> -else_block->if_block->else_block = NULL;
>>>  else_inst->remove(else_block);
>>>   }
>>>
>>> @@ -102,13 +101,6 @@ dead_control_flow_eliminate(backend_visitor *v)
>>>   if (earlier_block &&
>>> earlier_block->can_combine_with(later_block)) {
>>>  earlier_block->combine_with(later_block);
>>>
>>> -foreach_block (block, v->cfg) {
>>> -   if (block->if_block == later_block)
>>> -  block->if_block = earlier_block;
>>> -   if (block->else_block == later_block)
>>> -  block->else_block = earlier_block;
>>> -}
>>> -
>>>  /* If ENDIF was in its own block, then we've now deleted it
>>> and
>>>   * merged the two surrounding blocks, the latter of which
>>> the
>>>   * __next block pointer was pointing to.
>>> --
>>> 2.0.4
>>>
>>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 70359] [llvmpipe] piglit arb_shader_texture_lod-texgrad fails

2014-11-11 Thread bugzilla-daemon

https://bugs.freedesktop.org/show_bug.cgi?id=70359

José Fonseca  changed:

   What|Removed |Added

Summary|[llvmpipe] piglit   |[llvmpipe] piglit
   |arb_shader_texture_lod-texg |arb_shader_texture_lod-texg
   |rad regression  |rad fails

--- Comment #4 from José Fonseca  ---
With no_brilinear it goes a bit further. But it still fails:

$ GALLIVM_DEBUG=no_quad_lod,no_rho_approx,no_brilinear
./bin/arb_shader_texture_lod-texgrad -auto
Left: texture2D, Right: texture2DGradARB
Probe color at (107,91)
  Left: 0.00 0.258824 0.737255 0.00
  Right: 0.00 0.235294 0.760784 0.00
PIGLIT: {"result": "fail" }


Anyway, this is not a proper regression, as Roland said, but just one case
where two wrongs were making a right.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965/cfg: Remove if_block/else_block.

2014-11-11 Thread Matt Turner

On Tue, Nov 11, 2014 at 7:37 AM, Jason Ekstrand  wrote:
> Two questions: Does it fix the bug? And did you ever figure out what was
> going on there?

It does fix the bug. I checked out your nir-v0.7 branch and did the
work from there.

I spent probably an hour in gdb trying to figure out what was going
wrong and narrowed it down to the 13th time the
dead_control_flow_eliminate pass made progress, but I couldn't figure
out what was going wrong.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965/cfg: Remove if_block/else_block.

2014-11-11 Thread Matt Turner

On Tue, Nov 11, 2014 at 9:27 AM, Jason Ekstrand  wrote:
> One more comment: These and the other two CFG fixes should probably be
> promoted to stable.

Seems like a good plan.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] i965/fs: Remove try_replace_with_sel().

2014-11-11 Thread Matt Turner

The rest of our backend optimizations have replaced the need for this
since it was written.

instructions in affected programs: 30626 -> 30564 (-0.20%)

Hurts a small number of CSGO shaders by one instruction, but helps even
more. Hurts two by a larger number because of something I noticed when I
first wrote the SEL peephole: try_replace_with_sel() operates on
instructions before we've demoted uniforms to pull constants. So code
like

   var.x = ( -abs(r6.w) >= 0.0 ) ? pc[82].x : r9.x;
   var.y = ( -abs(r6.w) >= 0.0 ) ? pc[82].y : r9.y;
   var.z = ( -abs(r6.w) >= 0.0 ) ? pc[82].z : r9.z;
   var.w = ( -abs(r6.w) >= 0.0 ) ? pc[82].w : r9.w;

where pc[82] gets demoted to a pull constant, we end up emitting a
send(4) instruction to load pc[82] each time, and since they're in
different basic blocks because we mishandle the ternary operator in this
case we can't combine them. Once we handle this common ternary pattern
better the problem will go away.
---
 src/mesa/drivers/dri/i965/brw_fs.h   |  1 -
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 87 
 2 files changed, 88 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
b/src/mesa/drivers/dri/i965/brw_fs.h
index d9150c3..1c14d13 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -510,7 +510,6 @@ public:
 const fs_reg &src0, const fs_reg &src1);
bool try_emit_saturate(ir_expression *ir);
bool try_emit_mad(ir_expression *ir);
-   void try_replace_with_sel();
bool opt_peephole_sel();
bool opt_peephole_predicated_break();
bool opt_saturate_propagation();
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 4e1badd..d4f08aa 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -2565,91 +2565,6 @@ fs_visitor::emit_if_gen6(ir_if *ir)
emit(IF(this->result, fs_reg(0), BRW_CONDITIONAL_NZ));
 }
 
-/**
- * Try to replace IF/MOV/ELSE/MOV/ENDIF with SEL.
- *
- * Many GLSL shaders contain the following pattern:
- *
- *x = condition ? foo : bar
- *
- * The compiler emits an ir_if tree for this, since each subexpression might be
- * a complex tree that could have side-effects or short-circuit logic.
- *
- * However, the common case is to simply select one of two constants or
- * variable values---which is exactly what SEL is for.  In this case, the
- * assembly looks like:
- *
- *(+f0) IF
- *MOV dst src0
- *ELSE
- *MOV dst src1
- *ENDIF
- *
- * which can be easily translated into:
- *
- *(+f0) SEL dst src0 src1
- *
- * If src0 is an immediate value, we promote it to a temporary GRF.
- */
-void
-fs_visitor::try_replace_with_sel()
-{
-   fs_inst *endif_inst = (fs_inst *) instructions.get_tail();
-   assert(endif_inst->opcode == BRW_OPCODE_ENDIF);
-
-   /* Pattern match in reverse: IF, MOV, ELSE, MOV, ENDIF. */
-   int opcodes[] = {
-  BRW_OPCODE_IF, BRW_OPCODE_MOV, BRW_OPCODE_ELSE, BRW_OPCODE_MOV,
-   };
-
-   fs_inst *match = (fs_inst *) endif_inst->prev;
-   for (int i = 0; i < 4; i++) {
-  if (match->is_head_sentinel() || match->opcode != opcodes[4-i-1])
- return;
-  match = (fs_inst *) match->prev;
-   }
-
-   /* The opcodes match; it looks like the right sequence of instructions. */
-   fs_inst *else_mov = (fs_inst *) endif_inst->prev;
-   fs_inst *then_mov = (fs_inst *) else_mov->prev->prev;
-   fs_inst *if_inst = (fs_inst *) then_mov->prev;
-
-   /* Check that the MOVs are the right form. */
-   if (then_mov->dst.equals(else_mov->dst) &&
-   !then_mov->is_partial_write() &&
-   !else_mov->is_partial_write()) {
-
-  /* Remove the matched instructions; we'll emit a SEL to replace them. */
-  while (!if_inst->next->is_tail_sentinel())
- if_inst->next->exec_node::remove();
-  if_inst->exec_node::remove();
-
-  /* Only the last source register can be a constant, so if the MOV in
-   * the "then" clause uses a constant, we need to put it in a temporary.
-   */
-  fs_reg src0(then_mov->src[0]);
-  if (src0.file == IMM) {
- src0 = fs_reg(this, glsl_type::float_type);
- src0.type = then_mov->src[0].type;
- emit(MOV(src0, then_mov->src[0]));
-  }
-
-  fs_inst *sel;
-  if (if_inst->conditional_mod) {
- /* Sandybridge-specific IF with embedded comparison */
- emit(CMP(reg_null_d, if_inst->src[0], if_inst->src[1],
-  if_inst->conditional_mod));
- sel = emit(BRW_OPCODE_SEL, then_mov->dst, src0, else_mov->src[0]);
- sel->predicate = BRW_PREDICATE_NORMAL;
-  } else {
- /* Separate CMP and IF instructions */
- sel = emit(BRW_OPCODE_SEL, then_mov->dst, src0, else_mov->src[0]);
- sel->predicate = if_inst->predicate;
- sel->predicate_inverse = if_inst->predicate_inverse;
-  }
-   }
-}
-
 void
 fs_visitor::visit(ir_if *ir)
 {
@@

Re: [Mesa-dev] [PATCH] i965/vec4: Allow CSE on uniform-vec4 expansion MOVs.

2014-11-11 Thread Kenneth Graunke

On Friday, October 24, 2014 03:09:45 PM Matt Turner wrote:
> Three source instructions cannot directly source a packed vec4 (<0,4,1>
> regioning) like vec4 uniforms, so we emit a MOV that expands the vec4 to
> both halves of a register.
> 
> If these uniform values are used by multiple three-source instructions,
> we'll emit multiple expansion moves, which we cannot combine in CSE
> (because CSE emits moves itself).
> 
> So emit a virtual instruction that we can CSE.
> 
> Sometimes we demote a uniform to to a pull constant after emitting an
> expansion move for it. In that case, recognize in opt_algebraic that if
> the .file of the new instruction is GRF then it's just a real move that
> we can copy propagate and such.
> 
> total instructions in shared programs: 5509805 -> 5499940 (-0.18%)
> instructions in affected programs: 348923 -> 339058 (-2.83%)
> ---
>  src/mesa/drivers/dri/i965/brw_defines.h  | 1 +
>  src/mesa/drivers/dri/i965/brw_shader.cpp | 2 ++
>  src/mesa/drivers/dri/i965/brw_vec4.cpp   | 7 +++
>  src/mesa/drivers/dri/i965/brw_vec4_cse.cpp   | 1 +
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 1 +
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   | 2 +-
>  6 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
> index a68d607..a6807ae 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -904,6 +904,7 @@ enum opcode {
> SHADER_OPCODE_GEN7_SCRATCH_READ,
>  
> VEC4_OPCODE_PACK_BYTES,
> +   VEC4_OPCODE_UNPACK_UNIFORM,
>  
> FS_OPCODE_DDX,
> FS_OPCODE_DDY,
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp 
b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 87f2681..ea73dd7 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -457,6 +457,8 @@ brw_instruction_name(enum opcode op)
>  
> case VEC4_OPCODE_PACK_BYTES:
>return "pack_bytes";
> +   case VEC$_OPCODE_UNPACK_UNIFORM:

MONEY! ^^^

I'm really not crazy about this patch...it's just another MOV operation, which 
can be CSE'd, but isn't considered everywhere else we handle MOV.  Such as 
copy propagation, for example.

I suppose this is OK, because you're only using it for "the following 
instruction can't handle " MOVs, where you're not likely to be able 
to do things like copy propagation anyway.

Speaking of, you could probably use it for other workarounds too:
- resolve_ud_negate
- fix_math_operand
- emit_math1_gen6
- emit_math2_gen6

But I don't know if that would be helpful.

Although I'm not a fan, I don't have anything better to suggest, and your 
statistics clearly show a benefit.  It's probably fine.

Reviewed-by: Kenneth Graunke 

> +  return "unpack_uniform";
>  
> case FS_OPCODE_DDX:
>return "ddx";
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 5029495..cb8658d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -717,6 +717,13 @@ vec4_visitor::opt_algebraic()
>  
> foreach_block_and_inst(block, vec4_instruction, inst, cfg) {
>switch (inst->opcode) {
> +  case VEC4_OPCODE_UNPACK_UNIFORM:
> + if (inst->src[0].file != UNIFORM) {
> +inst->opcode = BRW_OPCODE_MOV;
> +progress = true;
> + }
> + break;
> +
>case BRW_OPCODE_ADD:
>if (inst->src[1].is_zero()) {
>   inst->opcode = BRW_OPCODE_MOV;
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
> index 28c69ca..6ff6902 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
> @@ -69,6 +69,7 @@ is_expression(const vec4_instruction *const inst)
> case BRW_OPCODE_PLN:
> case BRW_OPCODE_MAD:
> case BRW_OPCODE_LRP:
> +   case VEC4_OPCODE_UNPACK_UNIFORM:
>return true;
> case SHADER_OPCODE_RCP:
> case SHADER_OPCODE_RSQ:
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index 37c0806..6150d1d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -1183,6 +1183,7 @@ vec4_generator::generate_code(const cfg_t *cfg)
>}
>  
>switch (inst->opcode) {
> +  case VEC4_OPCODE_UNPACK_UNIFORM:
>case BRW_OPCODE_MOV:
>   brw_MOV(p, dst, src[0]);
>   break;
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 3b279dc..e10972b 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -302,7 +302,7 @@ vec4_visitor::fix_3src_operand(src_reg src)
>  
> dst_reg expanded = dst_reg(

Re: [Mesa-dev] [PATCH] i965/vec4: Allow CSE on uniform-vec4 expansion MOVs.

2014-11-11 Thread Matt Turner

On Tue, Nov 11, 2014 at 9:48 AM, Kenneth Graunke  wrote:
> On Friday, October 24, 2014 03:09:45 PM Matt Turner wrote:
>> Three source instructions cannot directly source a packed vec4 (<0,4,1>
>> regioning) like vec4 uniforms, so we emit a MOV that expands the vec4 to
>> both halves of a register.
>>
>> If these uniform values are used by multiple three-source instructions,
>> we'll emit multiple expansion moves, which we cannot combine in CSE
>> (because CSE emits moves itself).
>>
>> So emit a virtual instruction that we can CSE.
>>
>> Sometimes we demote a uniform to to a pull constant after emitting an
>> expansion move for it. In that case, recognize in opt_algebraic that if
>> the .file of the new instruction is GRF then it's just a real move that
>> we can copy propagate and such.
>>
>> total instructions in shared programs: 5509805 -> 5499940 (-0.18%)
>> instructions in affected programs: 348923 -> 339058 (-2.83%)
>> ---
>>  src/mesa/drivers/dri/i965/brw_defines.h  | 1 +
>>  src/mesa/drivers/dri/i965/brw_shader.cpp | 2 ++
>>  src/mesa/drivers/dri/i965/brw_vec4.cpp   | 7 +++
>>  src/mesa/drivers/dri/i965/brw_vec4_cse.cpp   | 1 +
>>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 1 +
>>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   | 2 +-
>>  6 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> b/src/mesa/drivers/dri/i965/brw_defines.h
>> index a68d607..a6807ae 100644
>> --- a/src/mesa/drivers/dri/i965/brw_defines.h
>> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
>> @@ -904,6 +904,7 @@ enum opcode {
>> SHADER_OPCODE_GEN7_SCRATCH_READ,
>>
>> VEC4_OPCODE_PACK_BYTES,
>> +   VEC4_OPCODE_UNPACK_UNIFORM,
>>
>> FS_OPCODE_DDX,
>> FS_OPCODE_DDY,
>> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp
> b/src/mesa/drivers/dri/i965/brw_shader.cpp
>> index 87f2681..ea73dd7 100644
>> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
>> @@ -457,6 +457,8 @@ brw_instruction_name(enum opcode op)
>>
>> case VEC4_OPCODE_PACK_BYTES:
>>return "pack_bytes";
>> +   case VEC$_OPCODE_UNPACK_UNIFORM:
>
> MONEY! ^^^
>
> I'm really not crazy about this patch...it's just another MOV operation, which
> can be CSE'd, but isn't considered everywhere else we handle MOV.  Such as
> copy propagation, for example.
>
> I suppose this is OK, because you're only using it for "the following
> instruction can't handle " MOVs, where you're not likely to be able
> to do things like copy propagation anyway.

That's exactly right. The only place it's emitted is from is
fix_3src_operand, which we know can't take these arguments directly so
copy propagation can't help us.

opt_algebraic changes the opcode to MOV if the source isn't a uniform,
like for an immediate. From there we can constant propagate it or
whatever.

> Speaking of, you could probably use it for other workarounds too:
> - resolve_ud_negate
> - fix_math_operand
> - emit_math1_gen6
> - emit_math2_gen6
>
> But I don't know if that would be helpful.
>
> Although I'm not a fan, I don't have anything better to suggest, and your
> statistics clearly show a benefit.  It's probably fine.

I really don't have any idea why. Short of adding a bunch of code to
CSE to rewrite operands rather than emitting another MOV (I don't know
how feasible that is, given the complexity of register coalescing) I
really don't know of a more elegant solution.

> Reviewed-by: Kenneth Graunke 

Thanks!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Fix segfault in WebGL Conformance on Ivybridge

2014-11-11 Thread Chad Versace


On Mon 10 Nov 2014, kalyan kondapally wrote:

The patch seems to fix the crash I used to see while running this
WebGL conformance test on my machine (Intel(R) Core(TM) i7-4600U CPU @
2.10GHz). and now I can see all the tests pass. I believe 78770 is a
different issue (Added my comments to 78770) and not actually dealing
with fixing the crash we saw on 64 bit machines.


Based on Lua's comments on bug 78770, I'm coming to the same conclusion.  
It's probably a different issue.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Fix segfault in WebGL Conformance on Ivybridge

2014-11-11 Thread Chad Versace


On Tue 11 Nov 2014, Ian Romanick wrote:

On 11/10/2014 04:02 PM, Chad Versace wrote:


   map->stride = mt->pitch;
-  map->ptr = base + y * map->stride + x * mt->cpp;
+
+  /* The variables in below pointer arithmetic are 32-bit. The arithmetic
+   * overflows for large textures.  Therefore the cast to intptr_t is
+   * needed.
+   *
+   * TODO(chadv): Fix this everywhere in i965 by fixing the signature of
+   * intel_miptree_get_image_offset() to use intptr_t.
+   */
+  map->ptr = base + (intptr_t) y * (intptr_t) map->stride
+  + (intptr_t) x * (intptr_t) mt->cpp;


I don't even want to know how long it took to track that down. :(


It took way too long... :(  I spent a lot of time on the wrong trail, 
trying to find errors in Mesa Core's pack/unpack routines. The bisect 
gave me the clue to where the problem was.



Is it sufficient to just use unsigned?


No. This expression is doing pointer arithmetic. Let's move into the 
21st century and just use intptr_t and uintptr_t for pointer arithmetic.


Just using a 32-bit unsigned here may fix this particular bug, but still
leaves the door open for future buckets-of-fun debugging sessions.

Let's do the right thing and use a C99 pointer-width type.


y and map->stride are the
problematic values here.  Isn't y bounded by (basically) 2*8192 and
map->stride bounded by 16*8192?  That sounds like 0x8000, which is
negative.



It seems like we could have similar problems in many places, and I'd
like to figure out what the correct coding practices are to avoid it.


I believe the correct coding practice is to always use C99 pointer-width 
types (inptr_t, uintptr_t, ptrdiff_t) when doing pointer arithmetic.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Use the predicate enable bit for conditional rendering without stalling

2014-11-11 Thread Neil Roberts

Kenneth Graunke  writes:

> drm-intel-next must have the new software checker turned on, which
> disallows non-whitelisted register writes (along with libva, so it
> can't really be enabled upstream yet).

For what it's worth, I get the EINVAL error even on the stock Fedora 20
kernel on Haswell (and presumably IvyBridge) so I can only assume the
software checker is already upstream, unless I'm misunderstanding
something.

$ uname -r
3.16.7-200.fc20.x86_64
$ modinfo i915 | grep cmd_parser
parm: enable_cmd_parser:Enable command parsing [...]
  (1=enabled [default], 0=disabled) (int)
$ sudo cat /sys/module/i915/parameters/enable_cmd_parser 
1

If I cat 0 to /sys/module/i915/parameters/enable_cmd_parser then I no
longer get the EINVAL error.

- Neil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: Use the predicate enable bit for conditional rendering without stalling

2014-11-11 Thread Kenneth Graunke

On Tuesday, November 11, 2014 06:59:51 PM Neil Roberts wrote:
> Kenneth Graunke  writes:
> 
> > drm-intel-next must have the new software checker turned on, which
> > disallows non-whitelisted register writes (along with libva, so it
> > can't really be enabled upstream yet).
> 
> For what it's worth, I get the EINVAL error even on the stock Fedora 20
> kernel on Haswell (and presumably IvyBridge) so I can only assume the
> software checker is already upstream, unless I'm misunderstanding
> something.
> 
> $ uname -r
> 3.16.7-200.fc20.x86_64
> $ modinfo i915 | grep cmd_parser
> parm: enable_cmd_parser:Enable command parsing [...]
>   (1=enabled [default], 0=disabled) (int)
> $ sudo cat /sys/module/i915/parameters/enable_cmd_parser 
> 1
> 
> If I cat 0 to /sys/module/i915/parameters/enable_cmd_parser then I no
> longer get the EINVAL error.
> 
> - Neil

Huh.  Yeah, I thought they turned it on by default in 3.16, which I don't 
understand at all.  AFAIK the libva issue isn't fixed (or wasn't by then), so 
it sure seems like it would've broken userspace.  Which would be a pretty 
clear kernel policy violation...

I must have the facts wrong, I guess...

--Ken

signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965/vec4: Rewrite dead code elimination to use live in/out.

2014-11-11 Thread Matt Turner

On Sat, Nov 8, 2014 at 10:18 PM, Kenneth Graunke  wrote:
> On Monday, November 03, 2014 01:34:49 PM Matt Turner wrote:
>> +static bool
>> +can_do_writemask(const struct brw_context *brw,
>> + const vec4_instruction *inst)
>> +{
>> +   switch (inst->opcode) {
>> +   case SHADER_OPCODE_GEN4_SCRATCH_READ:
>> +   case VS_OPCODE_PULL_CONSTANT_LOAD:
>> +   case VS_OPCODE_PULL_CONSTANT_LOAD_GEN7:
>> +  return false;
>> +   default:
>> +  /* The MATH instruction on Gen6 only executes in align1 mode, which 
>> does
>> +   * not support writemasking.
>> +   */
>> +  if (brw->gen == 6 && inst->is_math())
>> + return false;
>> +
>> +  if (inst->is_tex())
>> + return false;
>
> I'd feel a lot more confident in this function if it were:
>
> {
>/* The MATH instruction on Gen6 only executes in align1 mode, which does
> * not support writemasking.
> */
>if (brw->gen == 6 && inst->is_math())
>   return false;
>
>return inst->mlen == 0;
> }

I like that too, but I noticed that VS_OPCODE_PULL_CONSTANT_LOAD_GEN7
doesn't set mlen. I guess that's just a bug?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965/vec4: Rewrite dead code elimination to use live in/out.

2014-11-11 Thread Matt Turner

On Tue, Nov 11, 2014 at 2:17 PM, Matt Turner  wrote:
> On Sat, Nov 8, 2014 at 10:18 PM, Kenneth Graunke  
> wrote:
>> On Monday, November 03, 2014 01:34:49 PM Matt Turner wrote:
>>> +static bool
>>> +can_do_writemask(const struct brw_context *brw,
>>> + const vec4_instruction *inst)
>>> +{
>>> +   switch (inst->opcode) {
>>> +   case SHADER_OPCODE_GEN4_SCRATCH_READ:
>>> +   case VS_OPCODE_PULL_CONSTANT_LOAD:
>>> +   case VS_OPCODE_PULL_CONSTANT_LOAD_GEN7:
>>> +  return false;
>>> +   default:
>>> +  /* The MATH instruction on Gen6 only executes in align1 mode, which 
>>> does
>>> +   * not support writemasking.
>>> +   */
>>> +  if (brw->gen == 6 && inst->is_math())
>>> + return false;
>>> +
>>> +  if (inst->is_tex())
>>> + return false;
>>
>> I'd feel a lot more confident in this function if it were:
>>
>> {
>>/* The MATH instruction on Gen6 only executes in align1 mode, which does
>> * not support writemasking.
>> */
>>if (brw->gen == 6 && inst->is_math())
>>   return false;
>>
>>return inst->mlen == 0;
>> }
>
> I like that too, but I noticed that VS_OPCODE_PULL_CONSTANT_LOAD_GEN7
> doesn't set mlen. I guess that's just a bug?

Looks like it's not. mlen seems to imply some MRF writes, according to
::implied_mrf_writes().

I think I'll settle for adding

   if (inst->opcode == VS_OPCODE_PULL_CONSTANT_LOAD_GEN7)
  return false;

to the function.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 01/14] i965/hiz: Start to separate miptree out from hiz buffers

2014-11-11 Thread Jordan Justen

On 2014-11-10 10:42:16, Ben Widawsky wrote:
> On Mon, Jul 21, 2014 at 11:00:50PM -0700, Jordan Justen wrote:
> > Today we allocate a miptree's for the hiz buffer. We needed this in
> > the past because we would point the hardware at offsets of the hiz
> > buffer. Since the hiz format is not documented, this is not a good
> > idea.
> > 
> > Since moving to support layered rendering on Gen7+, we no longer point
> > at an offset into the buffer on Gen7+.
> > 
> > Therefore, to support hiz on Gen7+, we don't need a full miptree
> > structure allocated.
> > 
> > This patch starts to create a new auxiliary buffer structure
> > (intel_miptree_aux_buffer) that can be a more simplistic miptree
> > side-band buffer associated with a miptree. (For example, to serve the
> > needs of the hiz buffer.)
> > 
> > Signed-off-by: Jordan Justen 
> > Reviewed-by: Topi Pohjolainen 
> > ---
> >  src/mesa/drivers/dri/i965/brw_misc_state.c|  4 +-
> >  src/mesa/drivers/dri/i965/gen6_blorp.cpp  |  2 +-
> >  src/mesa/drivers/dri/i965/gen7_blorp.cpp  |  2 +-
> >  src/mesa/drivers/dri/i965/gen7_misc_state.c   |  2 +-
> >  src/mesa/drivers/dri/i965/gen8_depth_state.c  |  6 +--
> >  src/mesa/drivers/dri/i965/intel_fbo.c |  4 +-
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 55 
> > +++
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 29 --
> >  8 files changed, 76 insertions(+), 28 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c 
> > b/src/mesa/drivers/dri/i965/brw_misc_state.c
> > index 76e22bd..af900cd 100644
> > --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> > @@ -182,7 +182,7 @@ brw_get_depthstencil_tile_masks(struct 
> > intel_mipmap_tree *depth_mt,
> >  
> >if (intel_miptree_level_has_hiz(depth_mt, depth_level)) {
> >   uint32_t hiz_tile_mask_x, hiz_tile_mask_y;
> > - intel_miptree_get_tile_masks(depth_mt->hiz_mt,
> > + intel_miptree_get_tile_masks(depth_mt->hiz_buf->mt,
> >&hiz_tile_mask_x, &hiz_tile_mask_y,
> >false);
> >  
> > @@ -643,7 +643,7 @@ brw_emit_depth_stencil_hiz(struct brw_context *brw,
> >  
> >/* Emit hiz buffer. */
> >if (hiz) {
> > - struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_mt;
> > + struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
> >BEGIN_BATCH(3);
> >OUT_BATCH((_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
> >OUT_BATCH(hiz_mt->pitch - 1);
> > diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp 
> > b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> > index eb865b9..282c7b2 100644
> > --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> > @@ -855,7 +855,7 @@ gen6_blorp_emit_depth_stencil_config(struct brw_context 
> > *brw,
> >  
> > /* 3DSTATE_HIER_DEPTH_BUFFER */
> > {
> > -  struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_mt;
> > +  struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
> >uint32_t hiz_offset =
> >   intel_miptree_get_aligned_offset(hiz_mt,
> >draw_x & ~tile_mask_x,
> > diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp 
> > b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > index 0ad570b..d0d623d 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > @@ -752,7 +752,7 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context 
> > *brw,
> >  
> > /* 3DSTATE_HIER_DEPTH_BUFFER */
> > {
> > -  struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_mt;
> > +  struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
> >  
> >BEGIN_BATCH(3);
> >OUT_BATCH((GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
> > diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c 
> > b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > index 22911bf..6c6a79b 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > @@ -145,7 +145,7 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
> >OUT_BATCH(0);
> >ADVANCE_BATCH();
> > } else {
> > -  struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_mt;
> > +  struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
> >BEGIN_BATCH(3);
> >OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2));
> >OUT_BATCH((mocs << 25) |
> > diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c 
> > b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > index 7c3bfe0..b6f373d 100644
> > --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > @@ -89,10 +89,10 @@ emit_depth_packets(struct brw_context *brw,
> > } else {
> >BEGIN_BATCH(5);
> >

Re: [Mesa-dev] [PATCH v2 02/14] i965/gen7: Don't rely directly on the hiz miptree structure

2014-11-11 Thread Jordan Justen

On 2014-11-10 10:51:17, Ben Widawsky wrote:
> On Mon, Jul 21, 2014 at 11:00:51PM -0700, Jordan Justen wrote:
> > We are still allocating a miptree for hiz, but we only use fields from
> > intel_miptree_aux_buffer. This will allow us to switch over to not
> > allocating a miptree.
> > 
> > Signed-off-by: Jordan Justen 
> > Reviewed-by: Topi Pohjolainen 
> > ---
> >  src/mesa/drivers/dri/i965/gen7_blorp.cpp| 6 +++---
> >  src/mesa/drivers/dri/i965/gen7_misc_state.c | 7 ---
> >  2 files changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp 
> > b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > index d0d623d..ebfe6d1 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > @@ -752,13 +752,13 @@ gen7_blorp_emit_depth_stencil_config(struct 
> > brw_context *brw,
> >  
> > /* 3DSTATE_HIER_DEPTH_BUFFER */
> > {
> > -  struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
> > +  struct intel_miptree_aux_buffer *hiz_buf = params->depth.mt->hiz_buf;
> >  
> >BEGIN_BATCH(3);
> >OUT_BATCH((GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
> >OUT_BATCH((mocs << 25) |
> > -(hiz_mt->pitch - 1));
> > -  OUT_RELOC(hiz_mt->bo,
> > +(hiz_buf->pitch - 1));
> > +  OUT_RELOC(hiz_buf->bo,
> >  I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
> >  0);
> >ADVANCE_BATCH();
> > diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c 
> > b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > index 6c6a79b..8f8c33e 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > @@ -145,12 +145,13 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
> >OUT_BATCH(0);
> >ADVANCE_BATCH();
> > } else {
> > -  struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
> > +  struct intel_miptree_aux_buffer *hiz_buf = depth_mt->hiz_buf;
> > +
> >BEGIN_BATCH(3);
> >OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2));
> >OUT_BATCH((mocs << 25) |
> > -(hiz_mt->pitch - 1));
> > -  OUT_RELOC(hiz_mt->bo,
> > +(hiz_buf->pitch - 1));
> > +  OUT_RELOC(hiz_buf->bo,
> >  I915_GEM_DOMAIN_RENDER,
> >  I915_GEM_DOMAIN_RENDER,
> >  0);
> > -- 
> 
> Just a simple grep turns up a few places that weren't modified.  I am
> not sure if you intentionally left out the <= gen6 state, but just for
> the pedant:
> 
> brw_emit_depth_stencil_hiz()
> gen6_blorp_emit_depth_stencil_config()
> gen6_emit_depth_stencil_hiz()

Yeah, this is intentional. Gen6 will have to keep using the miptree
structure because of limitations of hiz on gen6. To support layered
rendering (and thus GL 3.2) we'll need to stick with this.

For gen < 6, I'm not sure. We might be able to rework some of how the
texturing works there because I don't think they will have to support
layered rendering. (Except, I may be veering off track from hiz
now...)

> If it was intentional, maybe adjust the commit message?

The subject does say gen7. :)

Any suggestions on what to add? Maybe just mention that gen <= 6 will
not be updated similarly?

-Jordan

> I see nothing that would break as a result here though.
> Reviewed-by: Ben Widawsky 
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] How difficult would it be to have debugging information for Jitted code show up?

2014-11-11 Thread Steven Stewart-Gallus

I'm hoping that it would be possible to have symbol names for Jitted code to
show up for profiling tools. For example, with software rendering and examining
my code using perf utils 70% of my code is just a mysterious callstack inside
the i915 shared library that has no symbols and no source information as the
JITted code has no debug information.

Thank you,
Steven Stewart-Gallus
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 02/14] i965/gen7: Don't rely directly on the hiz miptree structure

2014-11-11 Thread Ben Widawsky

On Tue, Nov 11, 2014 at 03:06:03PM -0800, Jordan Justen wrote:
> On 2014-11-10 10:51:17, Ben Widawsky wrote:
> > On Mon, Jul 21, 2014 at 11:00:51PM -0700, Jordan Justen wrote:
> > > We are still allocating a miptree for hiz, but we only use fields from
> > > intel_miptree_aux_buffer. This will allow us to switch over to not
> > > allocating a miptree.
> > > 
> > > Signed-off-by: Jordan Justen 
> > > Reviewed-by: Topi Pohjolainen 
> > > ---
> > >  src/mesa/drivers/dri/i965/gen7_blorp.cpp| 6 +++---
> > >  src/mesa/drivers/dri/i965/gen7_misc_state.c | 7 ---
> > >  2 files changed, 7 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp 
> > > b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > > index d0d623d..ebfe6d1 100644
> > > --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > > +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > > @@ -752,13 +752,13 @@ gen7_blorp_emit_depth_stencil_config(struct 
> > > brw_context *brw,
> > >  
> > > /* 3DSTATE_HIER_DEPTH_BUFFER */
> > > {
> > > -  struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
> > > +  struct intel_miptree_aux_buffer *hiz_buf = 
> > > params->depth.mt->hiz_buf;
> > >  
> > >BEGIN_BATCH(3);
> > >OUT_BATCH((GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
> > >OUT_BATCH((mocs << 25) |
> > > -(hiz_mt->pitch - 1));
> > > -  OUT_RELOC(hiz_mt->bo,
> > > +(hiz_buf->pitch - 1));
> > > +  OUT_RELOC(hiz_buf->bo,
> > >  I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
> > >  0);
> > >ADVANCE_BATCH();
> > > diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c 
> > > b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > > index 6c6a79b..8f8c33e 100644
> > > --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > > +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > > @@ -145,12 +145,13 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
> > >OUT_BATCH(0);
> > >ADVANCE_BATCH();
> > > } else {
> > > -  struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
> > > +  struct intel_miptree_aux_buffer *hiz_buf = depth_mt->hiz_buf;
> > > +
> > >BEGIN_BATCH(3);
> > >OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2));
> > >OUT_BATCH((mocs << 25) |
> > > -(hiz_mt->pitch - 1));
> > > -  OUT_RELOC(hiz_mt->bo,
> > > +(hiz_buf->pitch - 1));
> > > +  OUT_RELOC(hiz_buf->bo,
> > >  I915_GEM_DOMAIN_RENDER,
> > >  I915_GEM_DOMAIN_RENDER,
> > >  0);
> > > -- 
> > 
> > Just a simple grep turns up a few places that weren't modified.  I am
> > not sure if you intentionally left out the <= gen6 state, but just for
> > the pedant:
> > 
> > brw_emit_depth_stencil_hiz()
> > gen6_blorp_emit_depth_stencil_config()
> > gen6_emit_depth_stencil_hiz()
> 
> Yeah, this is intentional. Gen6 will have to keep using the miptree
> structure because of limitations of hiz on gen6. To support layered
> rendering (and thus GL 3.2) we'll need to stick with this.
> 

As we discussed in my cube yesterday, I was sort of hoping there would
be one data structure for both gen6 and > gen6, but I think we agreed it
doesn't really work out well. So it seems okay to me.

> For gen < 6, I'm not sure. We might be able to rework some of how the
> texturing works there because I don't think they will have to support
> layered rendering. (Except, I may be veering off track from hiz
> now...)
> 
> > If it was intentional, maybe adjust the commit message?
> 
> The subject does say gen7. :)
> 
> Any suggestions on what to add? Maybe just mention that gen <= 6 will
> not be updated similarly?
> 
> -Jordan

You're right. The subject says enough.

You did already have my r-b...

> 
> > I see nothing that would break as a result here though.
> > Reviewed-by: Ben Widawsky 
> > 
> > -- 
> > Ben Widawsky, Intel Open Source Technology Center

-- 
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965/vec4: Rewrite dead code elimination to use live in/out.

2014-11-11 Thread Kenneth Graunke

On Tuesday, November 11, 2014 02:39:13 PM Matt Turner wrote:
> On Tue, Nov 11, 2014 at 2:17 PM, Matt Turner  wrote:
> > On Sat, Nov 8, 2014 at 10:18 PM, Kenneth Graunke  
wrote:
> >> On Monday, November 03, 2014 01:34:49 PM Matt Turner wrote:
> >>> +static bool
> >>> +can_do_writemask(const struct brw_context *brw,
> >>> + const vec4_instruction *inst)
> >>> +{
> >>> +   switch (inst->opcode) {
> >>> +   case SHADER_OPCODE_GEN4_SCRATCH_READ:
> >>> +   case VS_OPCODE_PULL_CONSTANT_LOAD:
> >>> +   case VS_OPCODE_PULL_CONSTANT_LOAD_GEN7:
> >>> +  return false;
> >>> +   default:
> >>> +  /* The MATH instruction on Gen6 only executes in align1 mode, 
which does
> >>> +   * not support writemasking.
> >>> +   */
> >>> +  if (brw->gen == 6 && inst->is_math())
> >>> + return false;
> >>> +
> >>> +  if (inst->is_tex())
> >>> + return false;
> >>
> >> I'd feel a lot more confident in this function if it were:
> >>
> >> {
> >>/* The MATH instruction on Gen6 only executes in align1 mode, which 
does
> >> * not support writemasking.
> >> */
> >>if (brw->gen == 6 && inst->is_math())
> >>   return false;
> >>
> >>return inst->mlen == 0;
> >> }
> >
> > I like that too, but I noticed that VS_OPCODE_PULL_CONSTANT_LOAD_GEN7
> > doesn't set mlen. I guess that's just a bug?
> 
> Looks like it's not. mlen seems to imply some MRF writes, according to
> ::implied_mrf_writes().
> 
> I think I'll settle for adding
> 
>if (inst->opcode == VS_OPCODE_PULL_CONSTANT_LOAD_GEN7)
>   return false;
> 
> to the function.

This would do the trick:

return inst->mlen == 0 && !inst->is_send_from_grf();

and has the benefit of fewer places to update if we add more send-from-GRF 
opcodes.

signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] How difficult would it be to have debugging information for Jitted code show up?

2014-11-11 Thread Jose Fonseca

Is this the i915 gallium or classic driver? What does glxinfo show?

Jose 

From: mesa-dev  on behalf of Steven 
Stewart-Gallus 
Sent: 11 November 2014 23:45
To: Kenneth Graunke
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] How difficult would it be to have debugging information 
for Jitted code show up?

I'm hoping that it would be possible to have symbol names for Jitted code to
show up for profiling tools. For example, with software rendering and examining
my code using perf utils 70% of my code is just a mysterious callstack inside
the i915 shared library that has no symbols and no source information as the
JITted code has no debug information.

Thank you,
Steven Stewart-Gallus
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AAIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=5-qyNgNTfbll239LRSjQLKJuiUBT0u_c9rBpLMXBFgA&s=xDSlgkTlh6J3y4qx789edpfnRxZPgS7OSx3twMfza0o&e=
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 86070] Host application crash on vmware fusion 7 in vmw_swc_flush

2014-11-11 Thread bugzilla-daemon

https://bugs.freedesktop.org/show_bug.cgi?id=86070

Sinclair Yeh  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 86070] Host application crash on vmware fusion 7 in vmw_swc_flush

2014-11-11 Thread bugzilla-daemon

https://bugs.freedesktop.org/show_bug.cgi?id=86070

--- Comment #1 from Sinclair Yeh  ---
>From the description I assume the submitter is using mplayer.  I've done the
following steps and could not reproduce this issue:

1.  download and build mplayer from http://www.mplayerhq.hu
2.  download an mpeg2 clip from http://support.apple.com/en-us/HT1425
3.  ran:  mplayer sample_mpeg2.m2v

The clip seems to run fine.  My system configuration:
  Fedora 20 64-bit
  MESA 10.4

Please provide more details in terms of steps to reproduce and the system
configuration.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 86070] Host application crash on vmware fusion 7 in vmw_swc_flush

2014-11-11 Thread bugzilla-daemon

https://bugs.freedesktop.org/show_bug.cgi?id=86070

--- Comment #2 from Nicholas Yue  ---
Hi Sinclair,

  The application mplay (not mplayer) is from the Houdini suite of software by
Side Effects

 
http://www.sidefx.com/index.php?option=com_download&task=apprentice&Itemid=208

  You can use the Apprentice (free) version to verify the crash when launching
mplay, gplay or houdini itself

  Mesa version was already included in the original report

OpenGL version string: 2.1 Mesa 8.0.4

  My OS in question is Ubuntu 12.04.5 x64

  Kernel is : Linux ubuntu 3.13.0-32-generic #57~precise1-Ubuntu SMP Tue Jul 15
03:51:20 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

Cheers

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 86070] Host application crash on vmware fusion 7 in vmw_swc_flush

2014-11-11 Thread bugzilla-daemon

https://bugs.freedesktop.org/show_bug.cgi?id=86070

--- Comment #3 from Nicholas Yue  ---
I am using the most recent version of VMWare Fusion 7 on OS X Yosemite to host
Ubuntu 12.04.5 x64 and it only provides Mesa3D at version 8.0.4 not the 10.4
you are testing with.

Is there an update in VMWare Fusion 7 I should get and retest before debugging
further ?

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nvc0: remove unused nvc0_screen::mm_VRAM_fe0

2014-11-11 Thread Ilia Mirkin

LG. I had this same patch locally I think... I came up with it after I went
looking at the various VRAM usage after you were asking questions about it.

On Tue, Nov 11, 2014 at 11:59 PM, Alexandre Courbot 
wrote:

> Ping, how about this guy?
>
> On Mon, Oct 27, 2014 at 7:36 PM, Alexandre Courbot 
> wrote:
> > This member is declared, allocated and destroyed, but doesn't seem to be
> > used or referenced anywhere in the code.
> >
> > Signed-off-by: Alexandre Courbot 
> > ---
> > Resending after fixing typo in email address - apologies for the
> inconvenience.
> >
> >  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 3 ---
> >  src/gallium/drivers/nouveau/nvc0/nvc0_screen.h | 2 --
> >  2 files changed, 5 deletions(-)
> >
> > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> > index a7581f286cfc..61b381693224 100644
> > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> > @@ -407,8 +407,6 @@ nvc0_screen_destroy(struct pipe_screen *pscreen)
> >
> > FREE(screen->tic.entries);
> >
> > -   nouveau_mm_destroy(screen->mm_VRAM_fe0);
> > -
> > nouveau_object_del(&screen->eng3d);
> > nouveau_object_del(&screen->eng2d);
> > nouveau_object_del(&screen->m2mf);
> > @@ -1027,7 +1025,6 @@ nvc0_screen_create(struct nouveau_device *dev)
> >
> > mm_config.nvc0.tile_mode = 0;
> > mm_config.nvc0.memtype = 0xfe0;
> > -   screen->mm_VRAM_fe0 = nouveau_mm_create(dev, NOUVEAU_BO_VRAM,
> &mm_config);
> >
> > if (!nvc0_blitter_create(screen))
> >goto fail;
> > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.h
> b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.h
> > index 4802057f70ee..8a1991f52eb4 100644
> > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.h
> > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.h
> > @@ -73,8 +73,6 @@ struct nvc0_screen {
> >boolean mp_counters_enabled;
> > } pm;
> >
> > -   struct nouveau_mman *mm_VRAM_fe0;
> > -
> > struct nouveau_object *eng3d; /* sqrt(1/2)|kepler> +
> sqrt(1/2)|fermi> */
> > struct nouveau_object *eng2d;
> > struct nouveau_object *m2mf;
> > --
> > 2.1.2
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nvc0: remove unused nvc0_screen::mm_VRAM_fe0

2014-11-11 Thread Alexandre Courbot


On 11/12/2014 03:07 PM, Ilia Mirkin wrote:

LG. I had this same patch locally I think... I came up with it after I
went looking at the various VRAM usage after you were asking questions
about it.


Good, I'm dropping this version then, and assume yours will get merged soon.

Thanks!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

40 matches

Mail list logo