Re: [Mesa-dev] [PATCH] xvmc: Replace frame_started by picture_structure

2011-09-03 Thread Christian König
Looks good, I'm going to push it, but one small thing:

Am Freitag, den 02.09.2011, 16:20 +0200 schrieb Maarten Lankhorst:
 The preferred solution to keeping track of the picture structure
 has been putting it in the state tracker, so use picture_structure
 instead of frame_started to check if a frame needs to begin.
 
 If picture_structure has been changed, end the frame and start again.
 
 Signed-off-by: Maarten Lankhorst m.b.lankho...@gmail.com
[snip]

 @@ -273,7 +277,8 @@ Status XvMCRenderSurface(Display *dpy, XvMCContext 
 *context, unsigned int pictur
 xvmc_mb = macroblocks-macro_blocks + first_macroblock;
  
 /* If the surface we're rendering hasn't changed the ref frames shouldn't 
 change. */
 -   if (target_surface_priv-frame_started  (
 +   if (target_surface_priv-picture_structure  0 
 +   target_surface_priv-picture_structure != picture_structure  (
 target_surface_priv-ref[0] != past_surface ||
 target_surface_priv-ref[1] != future_surface ||
 (xvmc_mb-x == 0  xvmc_mb-y == 0))) {

That looks a bit odd, since we would call end_frame only when the
picture structure change, and that never happens for progressive
contents. I think it should look something like:

   if (target_surface_priv-picture_structure  0  (
   target_surface_priv-picture_structure != picture_structure ||
   target_surface_priv-ref[0] != past_surface ||
   target_surface_priv-ref[1] != future_surface ||

So we call end_frame under the following conditions:
1. begin_frame was called (picture structure is set) AND (
2. picture structure changes OR
3. ref frames change OR
4. mb address is (0,0)
)

Christian.

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


Re: [Mesa-dev] State tracker pipe_surface woes...

2011-09-03 Thread Jose Fonseca


- Original Message -
 2011/9/2 Stéphane Marchesin stephane.marche...@gmail.com:
  2011/9/2 Jose Fonseca jfons...@vmware.com:
  - Original Message -
  Hi,
 
  While debugging some code I ran across the following situation:
 
  - pipe_context c1 is created
  - pipe_surface s1 is created
  - strb- surface is set to s1 (s1's refcount goes up)
  - pipe_context c1 is destroyed
  - strb is destroyed
  - strb-surface is destroyed (so s1's refcount is now 0 and we
  want
  to
  destroy it)
 
  At that point s1 references c1 which is not available any more,
  so
  when we try to call ctx-surface_destroy to destroy s1 we crash.
 
  We discussed this a bit on IRC, and we agreed that the proper
  solution, since surfaces outlive their context, is to make
  surfaces
  screen-bound instead. I'm going to  implement that unless someone
  objects.
 
  As a side note, the same issue will happen with sampler_views, so
  it'll get a similar fix.
 
  Sampler views and surfaces were previously objects bound to
  screen, and we changed that because of poor multithreading
  semantics.  Per-context sampler views / render targets actually
  matches the 3D APIs semantics better, so I don't think that
  reverting is the solution.
 
  It looks to me that the issue here is that pipe_context should not
  be destroyed before the surfaces. strb-surface should only be
  used by one context, and should be destroyed before that context
  is destroyed.
 
  IIUC, strb matches GL renderbuffer semantics and can be shared by
  multiple context. If so, strb is treating pipe_surfaces as a
  entity shareable by contexts when really shouldn't.
 
  The solution is:
  - strb can only have pipe_resources, plus the key for the surface
  (face, level, etc)
  - the pipe_surfaces that are derived should be stored/cached in
  the GLcontext.
  - when the GLcontext / pipe_context is being destroy, the pipe
  surfaces can be destroyed before
 
 
  I don't understand some of it. From what I see, it should be
  enough,
  whenever strb binds a surface, to add a pointer to this strb  to a
  list of strb's to the pipe_context. By doing that, we would be able
  to
  unbind the surfaces from the strb before we destroy the context.
  However, pipe_context structures can't reference gl structures, so
  how
  would you solve that?
 
 
 Alright, maybe I'm too tired, I just have to put it in strb... My
 other questions still stand though :)
 
 Stéphane
 
 
  Also, what difference does it make if strb's only have
  pipe_resources?

Pipe resources can be shared between contexts, therefore they should not refer 
context or context data.
So it is always safe to destroy pipe_resources pipe_contexts on any order.

Using your example above, if you replace surface s1 with resource r1, a 
reference from r1 to c1 would be violating the semantics.

  And why do I need a key?

key is a bad name perhaps.  Unless the resource is always a single level 2d 
texture, if we replace the strb-surface with a strb-resource, we will need to 
specify separately which face/level is sought.

  This all is definitely more complex than it should be.

May be I'm not understanding what you were proposing. When you said that 

  the proper solution, since surfaces outlive their context, is to make 
  surfaces screen-bound instead

I interpreted this statement as moving the pipe_context::create_surface method 
to pipe_screen::create_surface. Was this really your plan or did you meant 
something else?

Because, my understanding is that it should be the other way around: when we 
moved the create_surface method from pipe_screen to pipe_context, we forgot to 
fix the st_renderbuffer code to do this properly.



The fix I proposed may seem a bit complex, but keeping pipe_surfaces as context 
objects actually helps pipe drivers that put derived data in pipe_surfaces to 
be much simpler, as they no longer need complicated locking to be thread safe 
-- the state tracker guarantees that pipe_surfaces belong to that context, and 
that context alone.

That is, moving stuff all into the screen sounds simple at first, but then it 
becomes a serious nightmare to make it thread safe.



Note that if st_renderbuffer can't be shared between GL contexts, then the 
solution can be much simpler: all we need to do is ensure that the surfaces are 
destroyed before the context. I haven't looked at the Mesa state tracker code 
in a while, so I'm a bit rusty in this regard. 

ARB_framebuffer_object says that framebuffer objects are per-context, but 
renderbuffer objects like textures can be shared between contexts.  So when one 
looks at st_renderbuffer definition:

/**
 * Derived renderbuffer class.  Just need to add a pointer to the
 * pipe surface.
 */
struct st_renderbuffer
{
   struct gl_renderbuffer Base;
   struct pipe_resource *texture;
   struct pipe_surface *surface; /* temporary view into texture */
   struct 

[Mesa-dev] [Bug 40600] New: avogadro crashes on startup due to bad target in _mesa_select_tex_object()

2011-09-03 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=40600

   Summary: avogadro crashes on startup due to bad target in
_mesa_select_tex_object()
   Product: Mesa
   Version: 7.11
  Platform: x86-64 (AMD64)
OS/Version: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: GLX
AssignedTo: mesa-dev@lists.freedesktop.org
ReportedBy: pa...@pks.mpg.de


Upon starting avogadro, a molecular editor application for research in
chemistry, the program crashes with the following output:

Mesa 7.11 implementation error: bad target in _mesa_select_tex_object()
Please report at bugs.freedesktop.org
Segmentation fault

I am not familiar with the inner workings of Mesa, but I'm following the
suggestion of filing the bug here.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- 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] intel: fix build error

2011-09-03 Thread Liu Aleaxander
From f02de851ba728642e26cce6fe76e5482619813e7 Mon Sep 17 00:00:00 2001
From: Yuanhan Liu yuanhan@linux.intel.com
Date: Sun, 4 Sep 2011 06:02:14 +0800
Subject: [PATCH] intel: fix build error

Fix a build error introduced by commit 6862b54f:
i965_dri.so.tmp: undefined reference to `strerr'

Signed-off-by: Yuanhan Liu yuanhan@linux.intel.com
---
 src/mesa/drivers/dri/intel/intel_batchbuffer.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
index c4bb836..98ae4e9 100644
--- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
@@ -138,7 +138,7 @@ do_flush_locked(struct intel_context *intel)
}

if (ret != 0) {
-  fprintf(stderr, intel_do_flush_locked failed: %s\n, strerr(ret));
+  fprintf(stderr, intel_do_flush_locked failed: %s\n, strerror(ret));
   exit(1);
}
intel-vtbl.new_batch(intel);
-- 
1.7.3.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] intel: fix build error

2011-09-03 Thread Paul Berry
On 3 September 2011 07:07, Liu Aleaxander aleaxan...@gmail.com wrote:

 From f02de851ba728642e26cce6fe76e5482619813e7 Mon Sep 17 00:00:00 2001
 From: Yuanhan Liu yuanhan@linux.intel.com
 Date: Sun, 4 Sep 2011 06:02:14 +0800
 Subject: [PATCH] intel: fix build error

 Fix a build error introduced by commit 6862b54f:
 i965_dri.so.tmp: undefined reference to `strerr'

 Signed-off-by: Yuanhan Liu yuanhan@linux.intel.com
 ---
  src/mesa/drivers/dri/intel/intel_batchbuffer.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
 b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
 index c4bb836..98ae4e9 100644
 --- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
 +++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
 @@ -138,7 +138,7 @@ do_flush_locked(struct intel_context *intel)
}

if (ret != 0) {
 -  fprintf(stderr, intel_do_flush_locked failed: %s\n, strerr(ret));
 +  fprintf(stderr, intel_do_flush_locked failed: %s\n,
 strerror(ret));
   exit(1);
}
intel-vtbl.new_batch(intel);
 --
 1.7.3.1
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev


I was just about to start tracking down this failure.  Thanks.

Reviewed-by: Paul Berry stereotype...@gmail.com

I'm guessing you don't have commit access since I don't see your name in the
commit history, so I'll go ahead and push the patch.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] intel: fix build error

2011-09-03 Thread Liu Aleaxander
On Sat, Sep 3, 2011 at 10:24 PM, Paul Berry stereotype...@gmail.com wrote:
 On 3 September 2011 07:07, Liu Aleaxander aleaxan...@gmail.com wrote:

 From f02de851ba728642e26cce6fe76e5482619813e7 Mon Sep 17 00:00:00 2001
 From: Yuanhan Liu yuanhan@linux.intel.com
 Date: Sun, 4 Sep 2011 06:02:14 +0800
 Subject: [PATCH] intel: fix build error

 Fix a build error introduced by commit 6862b54f:
 i965_dri.so.tmp: undefined reference to `strerr'

 Signed-off-by: Yuanhan Liu yuanhan@linux.intel.com
 ---
  src/mesa/drivers/dri/intel/intel_batchbuffer.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
 b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
 index c4bb836..98ae4e9 100644
 --- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
 +++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
 @@ -138,7 +138,7 @@ do_flush_locked(struct intel_context *intel)
    }

    if (ret != 0) {
 -      fprintf(stderr, intel_do_flush_locked failed: %s\n, strerr(ret));
 +      fprintf(stderr, intel_do_flush_locked failed: %s\n,
 strerror(ret));
       exit(1);
    }
    intel-vtbl.new_batch(intel);
 --
 1.7.3.1
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev

 I was just about to start tracking down this failure.  Thanks.

 Reviewed-by: Paul Berry stereotype...@gmail.com

 I'm guessing you don't have commit access since I don't see your name in the
 commit history, so I'll go ahead and push the patch.

Yes, please.

Thanks.

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


Re: [Mesa-dev] [PATCH] intel: fix build error

2011-09-03 Thread Eugeni Dodonov
On Sat, Sep 3, 2011 at 11:29, Liu Aleaxander aleaxan...@gmail.com wrote:

 On Sat, Sep 3, 2011 at 10:24 PM, Paul Berry stereotype...@gmail.com
 wrote:
  On 3 September 2011 07:07, Liu Aleaxander aleaxan...@gmail.com wrote:
 
  From f02de851ba728642e26cce6fe76e5482619813e7 Mon Sep 17 00:00:00 2001
  From: Yuanhan Liu yuanhan@linux.intel.com
  Date: Sun, 4 Sep 2011 06:02:14 +0800
  Subject: [PATCH] intel: fix build error
 
  Fix a build error introduced by commit 6862b54f:
  i965_dri.so.tmp: undefined reference to `strerr'
 
  Signed-off-by: Yuanhan Liu yuanhan@linux.intel.com
  ---
   src/mesa/drivers/dri/intel/intel_batchbuffer.c |2 +-
   1 files changed, 1 insertions(+), 1 deletions(-)
 
  diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
  b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
  index c4bb836..98ae4e9 100644
  --- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
  +++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
  @@ -138,7 +138,7 @@ do_flush_locked(struct intel_context *intel)
 }
 
 if (ret != 0) {
  -  fprintf(stderr, intel_do_flush_locked failed: %s\n,
 strerr(ret));
  +  fprintf(stderr, intel_do_flush_locked failed: %s\n,
  strerror(ret));
exit(1);
 }
 intel-vtbl.new_batch(intel);
  --
  1.7.3.1
  ___
  mesa-dev mailing list
  mesa-dev@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/mesa-dev
 
  I was just about to start tracking down this failure.  Thanks.
 
  Reviewed-by: Paul Berry stereotype...@gmail.com
 
  I'm guessing you don't have commit access since I don't see your name in
 the
  commit history, so I'll go ahead and push the patch.

 Yes, please.


Thanks - I fixed this locally but forgot to push yesterday.

This serves as a mental note to myself that even one-line patches could go
wrong :(. Sorry.

-- 
Eugeni Dodonov
http://eugeni.dodonov.net/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] xvmc: Replace frame_started by picture_structure

2011-09-03 Thread Maarten Lankhorst
On 09/03/2011 10:44 AM, Christian König wrote:
 Looks good, I'm going to push it, but one small thing:

 Am Freitag, den 02.09.2011, 16:20 +0200 schrieb Maarten Lankhorst:
 The preferred solution to keeping track of the picture structure
 has been putting it in the state tracker, so use picture_structure
 instead of frame_started to check if a frame needs to begin.

 If picture_structure has been changed, end the frame and start again.

 Signed-off-by: Maarten Lankhorst m.b.lankho...@gmail.com
 [snip]

 @@ -273,7 +277,8 @@ Status XvMCRenderSurface(Display *dpy, XvMCContext 
 *context, unsigned int pictur
 xvmc_mb = macroblocks-macro_blocks + first_macroblock;
  
 /* If the surface we're rendering hasn't changed the ref frames 
 shouldn't change. */
 -   if (target_surface_priv-frame_started  (
 +   if (target_surface_priv-picture_structure  0 
 +   target_surface_priv-picture_structure != picture_structure  (
 target_surface_priv-ref[0] != past_surface ||
 target_surface_priv-ref[1] != future_surface ||
 (xvmc_mb-x == 0  xvmc_mb-y == 0))) {
 That looks a bit odd, since we would call end_frame only when the
 picture structure change, and that never happens for progressive
 contents. I think it should look something like:
   if (target_surface_priv-picture_structure  0  (
   target_surface_priv-picture_structure != picture_structure ||
   target_surface_priv-ref[0] != past_surface ||
   target_surface_priv-ref[1] != future_surface ||
 So we call end_frame under the following conditions:
 1. begin_frame was called (picture structure is set) AND (
   2. picture structure changes OR
   3. ref frames change OR
   4. mb address is (0,0)
 )
You're right, that check is messed up, the ( should be moved up a line. Thanks 
for catching that. :)

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


Re: [Mesa-dev] new-vs on gen4

2011-09-03 Thread Paul Berry
On 2 September 2011 21:21, Paul Berry stereotype...@gmail.com wrote:

 On 2 September 2011 18:37, Eric Anholt e...@anholt.net wrote:

 This series gets gen4 to be non-regressing for the new vertex shader.
 I'd be fine with not pushing the last patch and letting Paul's patches
 land, then fixing the bug as it remains there.  Once these two land, I
 think it's time to turn on the new backend by default.


 This plan sounds good to me.  I'd like to land my patches as soon as I can,
 since my clip distance work depends on them.  Based on your review, I think
 only two issues remain with my patch series:

 1. You had some comments on i965: old VS: use the VUE map to compute the
 URB entry size. about the possibility of reading past the end of URB
 space.  My suspicion is that it's probably benign, and even if it's not,
 it's at worst a transitory problem (fixed by later commits) so I would be
 content to leave it as is.

 2. I promised to write a follow up patch to address your comments about
 color swizzling in i965: Write code to compute a VUE map.  I haven't done
 that yet.

 I'll try to do #2 tomorrow morning and send it to the list.  Assuming you
 like the look of that, and you feel ok about leaving issue #1 as is, I'd be
 happy to land my patches ASAP.


Ok, I did #2 but I think to avoid confusion I don't want to send it to the
list just yet, since it depends on the minor fixups that I've made to my
patch series based on Eric's comments--if I just sent the patches to the
list, they would appear to apply cleanly, but they wouldn't work properly.
If you want to have a look at it, pull from git://
github.com/stereotype441/mesa.git and look at the top two commits on the
vue_map branch.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 40600] avogadro crashes on startup due to bad target in _mesa_select_tex_object()

2011-09-03 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=40600

Kenneth Graunke kenn...@whitecape.org changed:

   What|Removed |Added

   Keywords||NEEDINFO
  Component|GLX |Mesa core

--- Comment #1 from Kenneth Graunke kenn...@whitecape.org 2011-09-03 11:04:46 
PDT ---
I can't reproduce this.  I just installed Avogadro 1.0.3 and successfully
started it with Mesa 7.11, the latest 7.11 branch, and master...all on Intel
Sandybridge.

What driver are you using?  Also, can you provide a backtrace?  In particular,
that should hopefully tell us (1) what OpenGL API call triggered this, and (2)
what got passed as target.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- 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 05/36] i965: old VS: use the VUE map to compute the URB entry size.

2011-09-03 Thread Kenneth Graunke
On 09/02/2011 09:06 AM, Paul Berry wrote:
 Previously, the old VS backend computed the URB entry size by adding
 the number of vertex shader outputs to the size of the URB header.
 This often produced a larger result than necessary, because some
 vertex shader outputs are stored in the header, so they were being
 double counted.  This patch changes the old VS backend to compute the
 URB entry size directly from the number of slots in the VUE map.
 ---
  src/mesa/drivers/dri/i965/brw_vs.h  |1 -
  src/mesa/drivers/dri/i965/brw_vs_emit.c |   31 
 +++
  2 files changed, 7 insertions(+), 25 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i965/brw_vs.h 
 b/src/mesa/drivers/dri/i965/brw_vs.h
 index a02c06d..c31869c 100644
 --- a/src/mesa/drivers/dri/i965/brw_vs.h
 +++ b/src/mesa/drivers/dri/i965/brw_vs.h
 @@ -65,7 +65,6 @@ struct brw_vs_compile {
  
 struct brw_vue_map vue_map;
 GLuint first_output;
 -   GLuint nr_outputs;
 GLuint last_scratch;
  
 GLuint first_tmp;
 diff --git a/src/mesa/drivers/dri/i965/brw_vs_emit.c 
 b/src/mesa/drivers/dri/i965/brw_vs_emit.c
 index e7667c8..7c430ce 100644
 --- a/src/mesa/drivers/dri/i965/brw_vs_emit.c
 +++ b/src/mesa/drivers/dri/i965/brw_vs_emit.c
 @@ -402,36 +402,19 @@ static void brw_vs_alloc_regs( struct brw_vs_compile *c 
 )
 /* The VS VUEs are shared by VF (outputting our inputs) and VS, so size
  * them to fit the biggest thing they need to.
  */
 -   attributes_in_vue = MAX2(c-nr_outputs, c-nr_inputs);
 +   attributes_in_vue = MAX2(c-vue_map.num_slots, c-nr_inputs);
  
 -   /* See emit_vertex_write() for where the VUE's overhead on top of the
 -* attributes comes from.
 -*/
 -   if (intel-gen = 7) {
 -  int header_regs = 2;
 -  if (c-key.nr_userclip)
 -  header_regs += 2;
 -
 -  /* Each attribute is 16 bytes (1 vec4), so dividing by 4 gives us the
 -   * number of 64-byte (512-bit) units.
 -   */
 -  c-prog_data.urb_entry_size = (attributes_in_vue + header_regs + 3) / 
 4;
 -   } else if (intel-gen == 6) {
 -  int header_regs = 2;
 -  if (c-key.nr_userclip)
 -  header_regs += 2;
 -
 -  /* Each attribute is 16 bytes (1 vec4), so dividing by 8 gives us the
 +   if (intel-gen == 6) {
 +  /* Each attribute is 32 bytes (2 vec4s), so dividing by 8 gives us the
 * number of 128-byte (1024-bit) units.
 */
 -  c-prog_data.urb_entry_size = (attributes_in_vue + header_regs + 7) / 
 8;
 -   } else if (intel-gen == 5)
 +  c-prog_data.urb_entry_size = (attributes_in_vue + 7) / 8;
 +   } else {
/* Each attribute is 16 bytes (1 vec4), so dividing by 4 gives us the
 * number of 64-byte (512-bit) units.
 */
 -  c-prog_data.urb_entry_size = (attributes_in_vue + 6 + 3) / 4;
 -   else
 -  c-prog_data.urb_entry_size = (attributes_in_vue + 2 + 3) / 4;
 +  c-prog_data.urb_entry_size = (attributes_in_vue + 3) / 4;
 +   }
  
 c-prog_data.total_grf = reg;

Totally non-obvious, but seems correct and is _much_ cleaner.

While we're at it, it might be nicer to do:

   if (intel-gen == 6)
  c-prog_data.urb_entry_size = ALIGN(attributes_in_vue, 8) / 8;
   else
  c-prog_data.urb_entry_size = ALIGN(attributes_in_vue, 4) / 4;

Otherwise, I get confused, thinking there are +7 or +3 registers
dedicated to something or other.  Most likely because in the original
code, (attributes_in_vue + 6 + 3) / 4, the +6 _is_ adding some header
registers, while the +3 is purely a round up! factor.

We can always do that in a follow-up patch if you prefer (or just not,
because this is all going away soon enough.)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 40600] avogadro crashes on startup due to bad target in _mesa_select_tex_object()

2011-09-03 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=40600

--- Comment #2 from pa...@pks.mpg.de 2011-09-03 11:47:31 PDT ---
My video card is a Mobility Radeon HD 4650, I'm running the closed source ATI
drivers (11.8) and the classic drivers in Mesa. Backtrace follows:

#0  0x7fffc5c68208 in _mesa_LoadMatrixd () from
/usr/lib64/dri/libdricore.so
#1  0x75f8f7eb in Avogadro::GLWidget::initializeGL (this=0x167dcd0) at
/var/tmp/portage/sci-chemistry/avogadro-1.0.3-r1/work/avogadro-1.0.3/libavogadro/src/glwidget.cpp:487
#2  0x75f86e73 in Avogadro::GLWidget::resizeEvent (this=0x167dcd0,
event=0x7fffce30)
at
/var/tmp/portage/sci-chemistry/avogadro-1.0.3-r1/work/avogadro-1.0.3/libavogadro/src/glwidget.cpp:604
#3  0x768d38ff in QWidget::event(QEvent*) () from
/usr/lib64/qt4/libQtGui.so.4
#4  0x773a841a in QGLWidget::event(QEvent*) () from
/usr/lib64/qt4/libQtOpenGL.so.4
#5  0x7687dcec in QApplicationPrivate::notify_helper(QObject*, QEvent*)
() from /usr/lib64/qt4/libQtGui.so.4
#6  0x76882fbd in QApplication::notify(QObject*, QEvent*) () from
/usr/lib64/qt4/libQtGui.so.4
#7  0x76382d6b in QCoreApplication::notifyInternal(QObject*, QEvent*)
() from /usr/lib64/qt4/libQtCore.so.4
#8  0x768d2949 in QWidgetPrivate::sendPendingMoveAndResizeEvents(bool,
bool) () from /usr/lib64/qt4/libQtGui.so.4
#9  0x768d7aa3 in QWidgetPrivate::show_helper() () from
/usr/lib64/qt4/libQtGui.so.4
#10 0x768d930a in QWidget::setVisible(bool) () from
/usr/lib64/qt4/libQtGui.so.4
#11 0x768d7f36 in QWidgetPrivate::showChildren(bool) () from
/usr/lib64/qt4/libQtGui.so.4
#12 0x768d7abf in QWidgetPrivate::show_helper() () from
/usr/lib64/qt4/libQtGui.so.4
#13 0x768d7ea0 in QWidgetPrivate::showChildren(bool) () from
/usr/lib64/qt4/libQtGui.so.4
#14 0x768d7abf in QWidgetPrivate::show_helper() () from
/usr/lib64/qt4/libQtGui.so.4
#15 0x768d930a in QWidget::setVisible(bool) () from
/usr/lib64/qt4/libQtGui.so.4
#16 0x768d7f36 in QWidgetPrivate::showChildren(bool) () from
/usr/lib64/qt4/libQtGui.so.4
#17 0x768d7abf in QWidgetPrivate::show_helper() () from
/usr/lib64/qt4/libQtGui.so.4
#18 0x768d930a in QWidget::setVisible(bool) () from
/usr/lib64/qt4/libQtGui.so.4
#19 0x768d7f36 in QWidgetPrivate::showChildren(bool) () from
/usr/lib64/qt4/libQtGui.so.4
#20 0x768d7abf in QWidgetPrivate::show_helper() () from
/usr/lib64/qt4/libQtGui.so.4
#21 0x768d930a in QWidget::setVisible(bool) () from
/usr/lib64/qt4/libQtGui.so.4
#22 0x768d7f36 in QWidgetPrivate::showChildren(bool) () from
/usr/lib64/qt4/libQtGui.so.4
#23 0x768d7abf in QWidgetPrivate::show_helper() () from
/usr/lib64/qt4/libQtGui.so.4
#24 0x768d930a in QWidget::setVisible(bool) () from
/usr/lib64/qt4/libQtGui.so.4
#25 0x00422b30 in main (argc=1, argv=value optimized out) at
/var/tmp/portage/sci-chemistry/avogadro-1.0.3-r1/work/avogadro-1.0.3/avogadro/src/main.cpp:244

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- 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 05/36] i965: old VS: use the VUE map to compute the URB entry size.

2011-09-03 Thread Paul Berry
On 3 September 2011 11:38, Kenneth Graunke kenn...@whitecape.org wrote:

 On 09/02/2011 09:06 AM, Paul Berry wrote:
  Previously, the old VS backend computed the URB entry size by adding
  the number of vertex shader outputs to the size of the URB header.
  This often produced a larger result than necessary, because some
  vertex shader outputs are stored in the header, so they were being
  double counted.  This patch changes the old VS backend to compute the
  URB entry size directly from the number of slots in the VUE map.
  ---
   src/mesa/drivers/dri/i965/brw_vs.h  |1 -
   src/mesa/drivers/dri/i965/brw_vs_emit.c |   31
 +++
   2 files changed, 7 insertions(+), 25 deletions(-)
 
  diff --git a/src/mesa/drivers/dri/i965/brw_vs.h
 b/src/mesa/drivers/dri/i965/brw_vs.h
  index a02c06d..c31869c 100644
  --- a/src/mesa/drivers/dri/i965/brw_vs.h
  +++ b/src/mesa/drivers/dri/i965/brw_vs.h
  @@ -65,7 +65,6 @@ struct brw_vs_compile {
 
  struct brw_vue_map vue_map;
  GLuint first_output;
  -   GLuint nr_outputs;
  GLuint last_scratch;
 
  GLuint first_tmp;
  diff --git a/src/mesa/drivers/dri/i965/brw_vs_emit.c
 b/src/mesa/drivers/dri/i965/brw_vs_emit.c
  index e7667c8..7c430ce 100644
  --- a/src/mesa/drivers/dri/i965/brw_vs_emit.c
  +++ b/src/mesa/drivers/dri/i965/brw_vs_emit.c
  @@ -402,36 +402,19 @@ static void brw_vs_alloc_regs( struct
 brw_vs_compile *c )
  /* The VS VUEs are shared by VF (outputting our inputs) and VS, so
 size
   * them to fit the biggest thing they need to.
   */
  -   attributes_in_vue = MAX2(c-nr_outputs, c-nr_inputs);
  +   attributes_in_vue = MAX2(c-vue_map.num_slots, c-nr_inputs);
 
  -   /* See emit_vertex_write() for where the VUE's overhead on top of the
  -* attributes comes from.
  -*/
  -   if (intel-gen = 7) {
  -  int header_regs = 2;
  -  if (c-key.nr_userclip)
  -  header_regs += 2;
  -
  -  /* Each attribute is 16 bytes (1 vec4), so dividing by 4 gives us
 the
  -   * number of 64-byte (512-bit) units.
  -   */
  -  c-prog_data.urb_entry_size = (attributes_in_vue + header_regs +
 3) / 4;
  -   } else if (intel-gen == 6) {
  -  int header_regs = 2;
  -  if (c-key.nr_userclip)
  -  header_regs += 2;
  -
  -  /* Each attribute is 16 bytes (1 vec4), so dividing by 8 gives us
 the
  +   if (intel-gen == 6) {
  +  /* Each attribute is 32 bytes (2 vec4s), so dividing by 8 gives us
 the
  * number of 128-byte (1024-bit) units.
  */
  -  c-prog_data.urb_entry_size = (attributes_in_vue + header_regs +
 7) / 8;
  -   } else if (intel-gen == 5)
  +  c-prog_data.urb_entry_size = (attributes_in_vue + 7) / 8;
  +   } else {
 /* Each attribute is 16 bytes (1 vec4), so dividing by 4 gives us
 the
  * number of 64-byte (512-bit) units.
  */
  -  c-prog_data.urb_entry_size = (attributes_in_vue + 6 + 3) / 4;
  -   else
  -  c-prog_data.urb_entry_size = (attributes_in_vue + 2 + 3) / 4;
  +  c-prog_data.urb_entry_size = (attributes_in_vue + 3) / 4;
  +   }
 
  c-prog_data.total_grf = reg;

 Totally non-obvious, but seems correct and is _much_ cleaner.

 While we're at it, it might be nicer to do:

   if (intel-gen == 6)
  c-prog_data.urb_entry_size = ALIGN(attributes_in_vue, 8) / 8;
   else
  c-prog_data.urb_entry_size = ALIGN(attributes_in_vue, 4) / 4;

 Otherwise, I get confused, thinking there are +7 or +3 registers
 dedicated to something or other.  Most likely because in the original
 code, (attributes_in_vue + 6 + 3) / 4, the +6 _is_ adding some header
 registers, while the +3 is purely a round up! factor.

 We can always do that in a follow-up patch if you prefer (or just not,
 because this is all going away soon enough.)


Yeah, I like your rewrite, Ken--it's clearer, and it's a simple enough
change.  I'll go ahead and update the patch.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 40612] New: Mesa-demos: build error against current Mesa git master

2011-09-03 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=40612

   Summary: Mesa-demos: build error against current Mesa git
master
   Product: Mesa
   Version: git
  Platform: All
OS/Version: All
Status: NEW
  Severity: blocker
  Priority: medium
 Component: Demos
AssignedTo: mesa-dev@lists.freedesktop.org
ReportedBy: johannesoberm...@gmx.de


eglkms-eglkms.o: In function `main':
/usr/src/packages/BUILD/Mesa-demos/src/egl/opengl/eglkms.c:175: undefined
reference to `gbm_create_device'
/usr/src/packages/BUILD/Mesa-demos/src/egl/opengl/eglkms.c:237: undefined
reference to `gbm_bo_create'
/usr/src/packages/BUILD/Mesa-demos/src/egl/opengl/eglkms.c:245: undefined
reference to `gbm_bo_get_handle'
/usr/src/packages/BUILD/Mesa-demos/src/egl/opengl/eglkms.c:246: undefined
reference to `gbm_bo_get_pitch'
/usr/src/packages/BUILD/Mesa-demos/src/egl/opengl/eglkms.c:332: undefined
reference to `gbm_bo_destroy'
/usr/src/packages/BUILD/Mesa-demos/src/egl/opengl/eglkms.c:340: undefined
reference to `gbm_device_destroy'
collect2: ld returned 1 exit status
make[3]: *** [eglkms] Error 1
make[3]: *** Waiting for unfinished jobs

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- 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] gallivm: fix build with LLVM 3.0svn

2011-09-03 Thread Tobias Droste
LLVM 3.0svn moved TargetRegistry.h and TargetSelect.h.
See revision 138450 of LLVM.

Signed-off-by: Tobias Droste tdro...@gmx.de
---
 src/gallium/auxiliary/gallivm/lp_bld_debug.cpp |   12 +---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp 
b/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp
index e252607..4302301 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp
+++ b/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp
@@ -27,17 +27,23 @@
 
 #include llvm-c/Core.h
 #include llvm/Target/TargetMachine.h
-#include llvm/Target/TargetRegistry.h
-#include llvm/Target/TargetSelect.h
 #include llvm/Target/TargetInstrInfo.h
 #include llvm/Support/raw_ostream.h
 #include llvm/Support/MemoryObject.h
 
+#if HAVE_LLVM = 0x0300
+#include llvm/Support/TargetRegistry.h
+#include llvm/Support/TargetSelect.h
+#else
+#include llvm/Target/TargetRegistry.h
+#include llvm/Target/TargetSelect.h
+#endif /* HAVE_LLVM = 0x0300 */
+
 #if HAVE_LLVM = 0x0209
 #include llvm/Support/Host.h
 #else
 #include llvm/System/Host.h
-#endif
+#endif /* HAVE_LLVM = 0x0209 */
 
 #if HAVE_LLVM = 0x0207
 #include llvm/MC/MCDisassembler.h
-- 
1.7.3.4

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


Re: [Mesa-dev] [PATCH v2] glsl_to_tgsi, mesa: fixes for native integers and integer booleans

2011-09-03 Thread Bryan Cain
On 09/02/2011 06:13 PM, Eric Anholt wrote:
 On Wed, 31 Aug 2011 01:33:59 -0500, Bryan Cain bryanca...@gmail.com wrote:
 With this patch, there are no piglit regressions on softpipe with native
 integers enabled.  Unlike my previous patch, this uses integer values of
 ~0 and 0 for true and false, respectively, instead of the float values 1.0
 and 0.0.
 This will break b2* conversions on our driver, since we expect true to
 be 1, not ~0.  The minimal change would require emit(AND(temp, uniform,
 1)) when deferencing boolean components of uniform variables.

Your driver already emits an AND instruction on every compare, according
to this code from brw_fs_visitor.cpp:

   case ir_binop_less:
   case ir_binop_greater:
   case ir_binop_lequal:
   case ir_binop_gequal:
   case ir_binop_equal:
   case ir_binop_all_equal:
   case ir_binop_nequal:
   case ir_binop_any_nequal:
  temp = this-result;
  /* original gen4 does implicit conversion before comparison. */
  if (intel-gen  5)
 temp.type = op[0].type;

  inst = emit(BRW_OPCODE_CMP, temp, op[0], op[1]);
  inst-conditional_mod = brw_conditional_for_comparison(ir-operation);
  emit(BRW_OPCODE_AND, this-result, this-result, fs_reg(0x1));
  break;

If the hardware sets ~0 on compares anyway, why not use that in your driver?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev