On 11/28/2013 11:44 PM, Kenneth Graunke wrote:
> On 11/27/2013 01:13 PM, Topi Pohjolainen wrote:
>> Documentation for R0 and R1 is taken from
>> fs_visitor::setup_payload_gen6().
>>
>> Signed-off-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
>> ---
>> src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 15 +++------------
>> 1 file changed, 3 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
>> b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
>> index 2fa0b50..a937edb 100644
>> --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
>> @@ -104,12 +104,6 @@ private:
>> const brw_blorp_const_color_prog_key *key;
>> struct brw_compile func;
>>
>> - /* Thread dispatch header */
>> - struct brw_reg R0;
>> -
>> - /* Pixel X/Y coordinates (always in R1). */
>> - struct brw_reg R1;
>> -
>
> I like the fact that you're getting rid of these. They're trivially
> reconstructable as brw_vec8_grf(0, 0) and brw_vec8_grf(1, 0), so storing
> them isn't really worthwhile.
>
>> /* Register with push constants (a single vec4) */
>> struct brw_reg clear_rgba;
>>
>> @@ -123,8 +117,6 @@
>> brw_blorp_const_color_program::brw_blorp_const_color_program(
>> : mem_ctx(ralloc_context(NULL)),
>> brw(brw),
>> key(key),
>> - R0(),
>> - R1(),
>> clear_rgba(),
>> base_mrf(0)
>> {
>> @@ -363,11 +355,8 @@ brw_blorp_const_color_params::get_wm_prog(struct
>> brw_context *brw,
>> void
>> brw_blorp_const_color_program::alloc_regs()
>> {
>> - int reg = 0;
>> - this->R0 = retype(brw_vec8_grf(reg++, 0), BRW_REGISTER_TYPE_UW);
>> - this->R1 = retype(brw_vec8_grf(reg++, 0), BRW_REGISTER_TYPE_UW);
>> + int reg = prog_data.first_curbe_grf;
>
> This line doesn't make sense to me. This function is what sets
> prog_data.first_curbe_grf. Presumably you want:
>
> /* Reserve space for g0 and g1, which contain the thread payload. */
> int reg = 2;
>
> Otherwise, I think the line below will change clear_rgba to g0 rather
> than g2, which is probably not what you want. Maybe I'm misreading.
Indeed, I can't read, sorry. You changed it so that compile() sets
first_curbe_grf, and alloc_regs() uses it. So your patch is correct.
Still, I think it would probably be nicer to keep it all contained in
alloc_regs(). It's such a tiny function though...
>
>> - prog_data.first_curbe_grf = reg;
>> clear_rgba = retype(brw_vec4_grf(reg++, 0), BRW_REGISTER_TYPE_F);
>> reg += BRW_BLORP_NUM_PUSH_CONST_REGS;
>>
>> @@ -384,6 +373,8 @@ brw_blorp_const_color_program::compile(struct
>> brw_context *brw,
>> /* Set up prog_data */
>> memset(&prog_data, 0, sizeof(prog_data));
>> prog_data.persample_msaa_dispatch = false;
>> + /* R0-1: masks, pixel X/Y coordinates. */
>> + prog_data.first_curbe_grf = 2;
>>
>> alloc_regs();
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
