[Mesa-dev] [PATCH 02/10] r600g: Add pkt_flag member to struct r600_command_buffer

[Alex Deucher]

On Wed, Jun 27, 2012 at 1:26 PM, Tom Stellard <thomas.stellard at amd.com> wrote:
> On Wed, Jun 27, 2012 at 07:17:37PM +0200, Marek Olšák wrote:
>> On Wed, Jun 27, 2012 at 5:37 PM, Tom Stellard <tstellar at gmail.com> wrote:
>> > Some packets require the shader type bit (bit 1) to be set when
>> > used for compute shaders.  The pkt_flag will be initialized to
>> > RADEON_CP_PACKET3_COMPUTE_MODE for any struct r600_command_buffer used
>> > for dispatching compute shaders and it will be or'd against the result of
>> > the PKT3 macro when adding a new packet to a strcut r600_command buffer.
>> > ---
>> >  src/gallium/drivers/r600/r600_pipe.h |   19 ++++++++++++++++---
>> >  1 files changed, 16 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/src/gallium/drivers/r600/r600_pipe.h b/src/gallium/drivers/r600/r600_pipe.h
>> > index 82c4a12..cd67ee1 100644
>> > --- a/src/gallium/drivers/r600/r600_pipe.h
>> > +++ b/src/gallium/drivers/r600/r600_pipe.h
>> > @@ -68,6 +68,7 @@ struct r600_command_buffer {
>> >        struct r600_atom atom;
>> >        uint32_t *buf;
>> >        unsigned max_num_dw;
>> > +       unsigned pkt_flags;
>> >  };
>> >
>> >  struct r600_surface_sync_cmd {
>> > @@ -584,19 +585,27 @@ static INLINE void r600_store_config_reg_seq(struct r600_command_buffer *cb, uns
>> >        cb->buf[cb->atom.num_dw++] = (reg - R600_CONFIG_REG_OFFSET) >> 2;
>> >  }
>> >
>> > +/**
>> > + * Needs cb->pkt_flags set to  RADEON_CP_PACKET3_COMPUTE_MODE for compute
>> > + * shaders.
>> > + */
>> >  static INLINE void r600_store_context_reg_seq(struct r600_command_buffer *cb, unsigned reg, unsigned num)
>> >  {
>> >        assert(reg >= R600_CONTEXT_REG_OFFSET && reg < R600_CTL_CONST_OFFSET);
>> >        assert(cb->atom.num_dw+2+num <= cb->max_num_dw);
>> > -       cb->buf[cb->atom.num_dw++] = PKT3(PKT3_SET_CONTEXT_REG, num, 0);
>> > +       cb->buf[cb->atom.num_dw++] = PKT3(PKT3_SET_CONTEXT_REG, num, 0) | cb->pkt_flags;
>> >        cb->buf[cb->atom.num_dw++] = (reg - R600_CONTEXT_REG_OFFSET) >> 2;
>> >  }
>> >
>> > +/**
>> > + * Needs cb->pkt_flags set to  RADEON_CP_PACKET3_COMPUTE_MODE for compute
>> > + * shaders.
>> > + */
>> >  static INLINE void r600_store_ctl_const_seq(struct r600_command_buffer *cb, unsigned reg, unsigned num)
>> >  {
>> >        assert(reg >= R600_CTL_CONST_OFFSET);
>> >        assert(cb->atom.num_dw+2+num <= cb->max_num_dw);
>> > -       cb->buf[cb->atom.num_dw++] = PKT3(PKT3_SET_CTL_CONST, num, 0);
>> > +       cb->buf[cb->atom.num_dw++] = PKT3(PKT3_SET_CTL_CONST, num, 0) | cb->pkt_flags;
>> >        cb->buf[cb->atom.num_dw++] = (reg - R600_CTL_CONST_OFFSET) >> 2;
>> >  }
>> >
>> > @@ -608,11 +617,15 @@ static INLINE void r600_store_loop_const_seq(struct r600_command_buffer *cb, uns
>> >        cb->buf[cb->atom.num_dw++] = (reg - R600_LOOP_CONST_OFFSET) >> 2;
>> >  }
>> >
>> > +/**
>> > + * Needs cb->pkt_flags set to  RADEON_CP_PACKET3_COMPUTE_MODE for compute
>> > + * shaders.
>> > + */
>> >  static INLINE void eg_store_loop_const_seq(struct r600_command_buffer *cb, unsigned reg, unsigned num)
>> >  {
>> >        assert(reg >= EG_LOOP_CONST_OFFSET);
>> >        assert(cb->atom.num_dw+2+num <= cb->max_num_dw);
>> > -       cb->buf[cb->atom.num_dw++] = PKT3(PKT3_SET_LOOP_CONST, num, 0);
>> > +       cb->buf[cb->atom.num_dw++] = PKT3(PKT3_SET_LOOP_CONST, num, 0) | cb->pkt_flags;
>> >        cb->buf[cb->atom.num_dw++] = (reg - EG_LOOP_CONST_OFFSET) >> 2;
>> >  }
>>
>> There are also other functions that emit PKT3. Are those not used by
>> compute or they just don't need the shader type bit set that you didn't
>> update them too?
>>
>> Other than that:
>> Reviewed-by: Marek Olšák <maraeo at gmail.com>
>>
>
> The compute bit is not needed for PKT3_SET_CONFIG_REG packets, and the
> r600_write_*_reg functions aren't currently used for compute.  I wanted
> to limit the impact on the 3D code, so I didn't update these functions.

I guess it's not technically required for all packets, but I think it
should be safe to set it for all compute packets.

Alex

>
> -Tom
>> Marek
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev