[Mesa-dev] [PATCH 1/2] i965: Add comments about URB size units and limits.

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/09/2011 01:03 AM, Eric Anholt wrote:
> On Fri,  8 Apr 2011 13:16:28 -0700, Kenneth Graunke <kenneth at whitecape.org> wrote:
>> ---
>>  src/mesa/drivers/dri/i965/brw_context.h |    8 ++++----
>>  src/mesa/drivers/dri/i965/brw_vs_emit.c |    6 ++++++
>>  src/mesa/drivers/dri/i965/gen6_urb.c    |   14 +++++++++++++-
>>  3 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
>> index 7b0551a..d63e8a4 100644
>> --- a/src/mesa/drivers/dri/i965/brw_context.h
>> +++ b/src/mesa/drivers/dri/i965/brw_context.h
>> @@ -555,12 +555,12 @@ struct brw_context
>>        GLuint nr_sf_entries;
>>        GLuint nr_cs_entries;
>>  
>> -      /* gen6 */
>> +      /* gen6:
>> +       * The length of each URB entry owned by the VS (or GS), as
>> +       * a number of 1024-bit (128-byte) units.  Should be >= 1.
>> +       */
> 
> I'd say "rows" instead of "units", since that's what they're called all
> through the docs.
> 
>>        GLuint vs_size;
>>  /*       GLuint gs_size; */
>> -/*       GLuint clip_size; */
>> -/*       GLuint sf_size; */
>> -/*       GLuint cs_size; */
>>  
>>        GLuint vs_start;
>>        GLuint gs_start;
>> diff --git a/src/mesa/drivers/dri/i965/brw_vs_emit.c b/src/mesa/drivers/dri/i965/brw_vs_emit.c
>> index acacf37..96150ec 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vs_emit.c
>> +++ b/src/mesa/drivers/dri/i965/brw_vs_emit.c
>> @@ -437,8 +437,14 @@ static void brw_vs_alloc_regs( struct brw_vs_compile *c )
>>        if (c->key.nr_userclip)
>>  	 header_regs += 2;
>>  
>> +      /* Each attribute is 16 bytes (1 vec4), 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)
>> +      /* 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;
>> diff --git a/src/mesa/drivers/dri/i965/gen6_urb.c b/src/mesa/drivers/dri/i965/gen6_urb.c
>> index c3819f9..38382f1 100644
>> --- a/src/mesa/drivers/dri/i965/gen6_urb.c
>> +++ b/src/mesa/drivers/dri/i965/gen6_urb.c
>> @@ -34,9 +34,13 @@
>>  static void
>>  prepare_urb( struct brw_context *brw )
>>  {
>> -   int urb_size, max_urb_entry;
>> +   int urb_size; /* total size of the URB, in bytes */
>> +   int max_urb_entry;
>>     struct intel_context *intel = &brw->intel;
>>  
>> +   /* According to volume 5c.5, Sandybridge GT1 has 32kB of URB space, while GT2
>> +    * has 64kB. In addition, each has a maximum number of VS URB entries (128 or 256).
>> +    */
>>     if (IS_GT1(intel->intelScreen->deviceID)) {
>>  	urb_size = 32 * 1024;
>>  	max_urb_entry = 128;
>> @@ -45,12 +49,20 @@ prepare_urb( struct brw_context *brw )
>>  	max_urb_entry = 256;
>>     }
> 
> I don't like how we have IS_GTn checks in here.  I'd rather the values
> be all set up once in brw-> in brw_context.c like the other
> chipset-dependent values.

That's a good point.

Ken, can you spin another version?  The fix to ROUND_DOWN_TO is in
master, so you can safely use that now. :)

It should be safe the push the first commit in the series.  Just collect
up the correct R-b list.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk2jmE0ACgkQX1gOwKyEAw/lGQCdFK8W3J86Gg0frTltu94kIASF
WpkAnA5Ib48OMW1An9PXOphLbLwjH1DZ
=fNGz
-----END PGP SIGNATURE-----