All of lore.kernel.org
 help / color / mirror / Atom feed
* bitfield structures
@ 2014-10-16 14:14 Alex Deucher
  2014-10-16 15:33 ` Jerome Glisse
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Alex Deucher @ 2014-10-16 14:14 UTC (permalink / raw)
  To: Maling list - DRI developers

As part of the amdgpu transition, we are moving to using database
generated register and packet headers.  We have a number of options
for formatting, some of which involve bitfields (don't worry there
will also be shift/mask style headers as well which is mainly what we
use in the code).  I think these formats are cleaner for a number of
cases, however, as far as I know, C does not define the ordering of
bits within bitfields.  That said, every compiler I've used seems to
do what you would expect.  It makes coding a lot cleaner as less
error-prone in certain cases.  Here are a couple of example of what
I'm talking about:

A register example:

union GRPH_SWAP_CNTL {
        struct {
#if BIG_ENDIAN
                unsigned int                                 : 20;
                unsigned int             GRPH_ALPHA_CROSSBAR : 2;
                unsigned int              GRPH_BLUE_CROSSBAR : 2;
                unsigned int             GRPH_GREEN_CROSSBAR : 2;
                unsigned int               GRPH_RED_CROSSBAR : 2;
                unsigned int                                 : 2;
                unsigned int                GRPH_ENDIAN_SWAP : 2;
#else
                unsigned int                GRPH_ENDIAN_SWAP : 2;
                unsigned int                                 : 2;
                unsigned int               GRPH_RED_CROSSBAR : 2;
                unsigned int             GRPH_GREEN_CROSSBAR : 2;
                unsigned int              GRPH_BLUE_CROSSBAR : 2;
                unsigned int             GRPH_ALPHA_CROSSBAR : 2;
                unsigned int                                 : 20;
#endif
        } bitfields, bits;
        unsigned int u32All;
        signed int i32All;
        float f32All;
};

A packet example:

typedef union PM4_TYPE_3_HEADER
{
    struct
    {
#if BIG_ENDIAN
        unsigned int type      : 2; ///< packet identifier. It should
be 3 for type 3 packets
        unsigned int count     : 14;///< number of DWORDs - 1 in the
information body.
        unsigned int opcode    : 8; ///< IT opcode
        unsigned int reserved1 : 6; ///< reserved
        unsigned int shaderType: 1; ///< 0: Graphics, 1: Compute Shader
        unsigned int predicate : 1; ///< predicated version of packet when set
#else
        unsigned int predicate : 1; ///< predicated version of packet when set
        unsigned int shaderType: 1; ///< 0: Graphics, 1: Compute Shader
        unsigned int reserved1 : 6; ///< reserved
        unsigned int opcode    : 8; ///< IT opcode
        unsigned int count     : 14;///< number of DWORDs - 1 in the
information body.
        unsigned int type      : 2; ///< packet identifier. It should
be 3 for type 3 packets
#endif
    };
    unsigned int u32All;
} PM4_TYPE_3_HEADER;

//--------------------MEM_SEMAPHORE--------------------

enum MEM_SEMAPHORE_signal_type_enum {
signal_type_mem_semaphore_SIGNAL_TYPE_INCREMENT_0 = 0,
signal_type_mem_semaphore_SIGNAL_TYPE_WRITE_1 = 1 };
enum MEM_SEMAPHORE_client_code_enum { client_code_mem_semaphore_CP_0 =
0, client_code_mem_semaphore_CB_1 = 1, client_code_mem_semaphore_DB_2
= 2, client_code_mem_semaphore_RESERVED_3 = 3 };
enum MEM_SEMAPHORE_sem_sel_enum {
sem_sel_mem_semaphore_SIGNAL_SEMAPHORE_6 = 6,
sem_sel_mem_semaphore_WAIT_SEMAPHORE_7 = 7 };

typedef struct _PM4_MEM_SEMAPHORE
{
    union
    {
        PM4_TYPE_3_HEADER   header;            ///header
        unsigned int        ordinal1;
    };

    union
    {
        struct
    {
#if BIG_ENDIAN
            unsigned int address_lo:29;
            unsigned int reserved1:3;
#else
            unsigned int reserved1:3;
            unsigned int address_lo:29;
#endif
        } bitfields2;
        unsigned int ordinal2;
    };

    union
    {
        struct
    {
#if BIG_ENDIAN
            MEM_SEMAPHORE_sem_sel_enum sem_sel:3;
            unsigned int reserved4:3;
            MEM_SEMAPHORE_client_code_enum client_code:2;
            unsigned int reserved3:3;
            MEM_SEMAPHORE_signal_type_enum signal_type:1;
            unsigned int reserved2:3;
            unsigned int use_mailbox:1;
            unsigned int address_hi:16;
#else
            unsigned int address_hi:16;
            unsigned int use_mailbox:1;
            unsigned int reserved2:3;
            MEM_SEMAPHORE_signal_type_enum signal_type:1;
            unsigned int reserved3:3;
            MEM_SEMAPHORE_client_code_enum client_code:2;
            unsigned int reserved4:3;
            MEM_SEMAPHORE_sem_sel_enum sem_sel:3;
#endif
        } bitfields3;
        unsigned int ordinal3;
    };

}  PM4MEM_SEMAPHORE, *PPM4MEM_SEMAPHORE;

Are there any strong objections to these sorts of structures?

Thanks,

Alex

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: bitfield structures
  2014-10-16 14:14 bitfield structures Alex Deucher
@ 2014-10-16 15:33 ` Jerome Glisse
  2014-10-16 15:57 ` Peter Hurley
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Jerome Glisse @ 2014-10-16 15:33 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Maling list - DRI developers

On Thu, Oct 16, 2014 at 10:14:27AM -0400, Alex Deucher wrote:
> As part of the amdgpu transition, we are moving to using database
> generated register and packet headers.  We have a number of options
> for formatting, some of which involve bitfields (don't worry there
> will also be shift/mask style headers as well which is mainly what we
> use in the code).  I think these formats are cleaner for a number of
> cases, however, as far as I know, C does not define the ordering of
> bits within bitfields.  That said, every compiler I've used seems to
> do what you would expect.  It makes coding a lot cleaner as less
> error-prone in certain cases.  Here are a couple of example of what
> I'm talking about:
> 
> A register example:
> 
> union GRPH_SWAP_CNTL {
>         struct {
> #if BIG_ENDIAN
>                 unsigned int                                 : 20;
>                 unsigned int             GRPH_ALPHA_CROSSBAR : 2;
>                 unsigned int              GRPH_BLUE_CROSSBAR : 2;
>                 unsigned int             GRPH_GREEN_CROSSBAR : 2;
>                 unsigned int               GRPH_RED_CROSSBAR : 2;
>                 unsigned int                                 : 2;
>                 unsigned int                GRPH_ENDIAN_SWAP : 2;
> #else
>                 unsigned int                GRPH_ENDIAN_SWAP : 2;
>                 unsigned int                                 : 2;
>                 unsigned int               GRPH_RED_CROSSBAR : 2;
>                 unsigned int             GRPH_GREEN_CROSSBAR : 2;
>                 unsigned int              GRPH_BLUE_CROSSBAR : 2;
>                 unsigned int             GRPH_ALPHA_CROSSBAR : 2;
>                 unsigned int                                 : 20;
> #endif
>         } bitfields, bits;
>         unsigned int u32All;
>         signed int i32All;
>         float f32All;
> };
> 
> A packet example:
> 
> typedef union PM4_TYPE_3_HEADER
> {
>     struct
>     {
> #if BIG_ENDIAN
>         unsigned int type      : 2; ///< packet identifier. It should
> be 3 for type 3 packets
>         unsigned int count     : 14;///< number of DWORDs - 1 in the
> information body.
>         unsigned int opcode    : 8; ///< IT opcode
>         unsigned int reserved1 : 6; ///< reserved
>         unsigned int shaderType: 1; ///< 0: Graphics, 1: Compute Shader
>         unsigned int predicate : 1; ///< predicated version of packet when set
> #else
>         unsigned int predicate : 1; ///< predicated version of packet when set
>         unsigned int shaderType: 1; ///< 0: Graphics, 1: Compute Shader
>         unsigned int reserved1 : 6; ///< reserved
>         unsigned int opcode    : 8; ///< IT opcode
>         unsigned int count     : 14;///< number of DWORDs - 1 in the
> information body.
>         unsigned int type      : 2; ///< packet identifier. It should
> be 3 for type 3 packets
> #endif
>     };
>     unsigned int u32All;
> } PM4_TYPE_3_HEADER;
> 
> //--------------------MEM_SEMAPHORE--------------------
> 
> enum MEM_SEMAPHORE_signal_type_enum {
> signal_type_mem_semaphore_SIGNAL_TYPE_INCREMENT_0 = 0,
> signal_type_mem_semaphore_SIGNAL_TYPE_WRITE_1 = 1 };
> enum MEM_SEMAPHORE_client_code_enum { client_code_mem_semaphore_CP_0 =
> 0, client_code_mem_semaphore_CB_1 = 1, client_code_mem_semaphore_DB_2
> = 2, client_code_mem_semaphore_RESERVED_3 = 3 };
> enum MEM_SEMAPHORE_sem_sel_enum {
> sem_sel_mem_semaphore_SIGNAL_SEMAPHORE_6 = 6,
> sem_sel_mem_semaphore_WAIT_SEMAPHORE_7 = 7 };
> 
> typedef struct _PM4_MEM_SEMAPHORE
> {
>     union
>     {
>         PM4_TYPE_3_HEADER   header;            ///header
>         unsigned int        ordinal1;
>     };
> 
>     union
>     {
>         struct
>     {
> #if BIG_ENDIAN
>             unsigned int address_lo:29;
>             unsigned int reserved1:3;
> #else
>             unsigned int reserved1:3;
>             unsigned int address_lo:29;
> #endif
>         } bitfields2;
>         unsigned int ordinal2;
>     };
> 
>     union
>     {
>         struct
>     {
> #if BIG_ENDIAN
>             MEM_SEMAPHORE_sem_sel_enum sem_sel:3;
>             unsigned int reserved4:3;
>             MEM_SEMAPHORE_client_code_enum client_code:2;

Do the enum really works with bitfield ? I am just lazy to go check c spec.

>             unsigned int reserved3:3;
>             MEM_SEMAPHORE_signal_type_enum signal_type:1;
>             unsigned int reserved2:3;
>             unsigned int use_mailbox:1;
>             unsigned int address_hi:16;
> #else
>             unsigned int address_hi:16;
>             unsigned int use_mailbox:1;
>             unsigned int reserved2:3;
>             MEM_SEMAPHORE_signal_type_enum signal_type:1;
>             unsigned int reserved3:3;
>             MEM_SEMAPHORE_client_code_enum client_code:2;
>             unsigned int reserved4:3;
>             MEM_SEMAPHORE_sem_sel_enum sem_sel:3;
> #endif
>         } bitfields3;
>         unsigned int ordinal3;
>     };
> 
> }  PM4MEM_SEMAPHORE, *PPM4MEM_SEMAPHORE;
> 
> Are there any strong objections to these sorts of structures?

I personally do not have any objection to using that though i would
rather avoid things like u32All (forgot the name for this kind of
variable naming convention) so just use all ie GRPH_SWAP_CNTL.all

Also using uint32_t instead of unsigned int would be saffer in my
mind even thought all arch we care about define unsigned int as
32 bits.

Finaly if you want to use that then the define with shift and mask
is pointless and risky, i rather have one and only one representation
than two different one that might diverge.

Cheers,
Jérôme

> Thanks,
> 
> Alex
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: bitfield structures
  2014-10-16 14:14 bitfield structures Alex Deucher
  2014-10-16 15:33 ` Jerome Glisse
@ 2014-10-16 15:57 ` Peter Hurley
  2014-10-16 15:57 ` Eric Anholt
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Peter Hurley @ 2014-10-16 15:57 UTC (permalink / raw)
  To: Alex Deucher, Maling list - DRI developers

On 10/16/2014 10:14 AM, Alex Deucher wrote:
> Are there any strong objections to these sorts of structures?

You may want to blacklist certain compiler version/arch combinations,
or get the affected arches to do it.

gcc up to 4.7.1 on ia64 and ppc64 generates 64-bit wide RMW cycles
on bitfields, regardless of the specified type or actual field width.
The enlarged write overwrites adjacent fields.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080

Regards,
Peter Hurley

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: bitfield structures
  2014-10-16 14:14 bitfield structures Alex Deucher
  2014-10-16 15:33 ` Jerome Glisse
  2014-10-16 15:57 ` Peter Hurley
@ 2014-10-16 15:57 ` Eric Anholt
  2014-10-16 19:16 ` Christian König
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Eric Anholt @ 2014-10-16 15:57 UTC (permalink / raw)
  To: Alex Deucher, Maling list - DRI developers


[-- Attachment #1.1: Type: text/plain, Size: 2231 bytes --]

Alex Deucher <alexdeucher@gmail.com> writes:

> As part of the amdgpu transition, we are moving to using database
> generated register and packet headers.  We have a number of options
> for formatting, some of which involve bitfields (don't worry there
> will also be shift/mask style headers as well which is mainly what we
> use in the code).  I think these formats are cleaner for a number of
> cases, however, as far as I know, C does not define the ordering of
> bits within bitfields.  That said, every compiler I've used seems to
> do what you would expect.  It makes coding a lot cleaner as less
> error-prone in certain cases.  Here are a couple of example of what
> I'm talking about:
>
> A register example:
>
> union GRPH_SWAP_CNTL {
>         struct {
> #if BIG_ENDIAN
>                 unsigned int                                 : 20;
>                 unsigned int             GRPH_ALPHA_CROSSBAR : 2;
>                 unsigned int              GRPH_BLUE_CROSSBAR : 2;
>                 unsigned int             GRPH_GREEN_CROSSBAR : 2;
>                 unsigned int               GRPH_RED_CROSSBAR : 2;
>                 unsigned int                                 : 2;
>                 unsigned int                GRPH_ENDIAN_SWAP : 2;
> #else
>                 unsigned int                GRPH_ENDIAN_SWAP : 2;
>                 unsigned int                                 : 2;
>                 unsigned int               GRPH_RED_CROSSBAR : 2;
>                 unsigned int             GRPH_GREEN_CROSSBAR : 2;
>                 unsigned int              GRPH_BLUE_CROSSBAR : 2;
>                 unsigned int             GRPH_ALPHA_CROSSBAR : 2;
>                 unsigned int                                 : 20;
> #endif
>         } bitfields, bits;
>         unsigned int u32All;
>         signed int i32All;
>         float f32All;
> };

> Are there any strong objections to these sorts of structures?

My experience with bitfields was that the code generated by gcc was
atrocious, and I wouldn't use them for any performance sensitive 32-bit
word setup.  As a result, publishing headers in the form of
bitfield-structs seems like a way to encourage people to generate bad
code for your hardware.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 818 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: bitfield structures
  2014-10-16 14:14 bitfield structures Alex Deucher
                   ` (2 preceding siblings ...)
  2014-10-16 15:57 ` Eric Anholt
@ 2014-10-16 19:16 ` Christian König
  2014-10-16 21:14   ` Dave Airlie
  2014-10-17  8:55 ` Michel Dänzer
  2014-10-17 11:19 ` Rob Clark
  5 siblings, 1 reply; 9+ messages in thread
From: Christian König @ 2014-10-16 19:16 UTC (permalink / raw)
  To: Alex Deucher, Maling list - DRI developers

Well, making a long story short: 
http://yarchive.net/comp/linux/bitfields.html

Regards,
Christian.

Am 16.10.2014 um 16:14 schrieb Alex Deucher:
> As part of the amdgpu transition, we are moving to using database
> generated register and packet headers.  We have a number of options
> for formatting, some of which involve bitfields (don't worry there
> will also be shift/mask style headers as well which is mainly what we
> use in the code).  I think these formats are cleaner for a number of
> cases, however, as far as I know, C does not define the ordering of
> bits within bitfields.  That said, every compiler I've used seems to
> do what you would expect.  It makes coding a lot cleaner as less
> error-prone in certain cases.  Here are a couple of example of what
> I'm talking about:
>
> A register example:
>
> union GRPH_SWAP_CNTL {
>          struct {
> #if BIG_ENDIAN
>                  unsigned int                                 : 20;
>                  unsigned int             GRPH_ALPHA_CROSSBAR : 2;
>                  unsigned int              GRPH_BLUE_CROSSBAR : 2;
>                  unsigned int             GRPH_GREEN_CROSSBAR : 2;
>                  unsigned int               GRPH_RED_CROSSBAR : 2;
>                  unsigned int                                 : 2;
>                  unsigned int                GRPH_ENDIAN_SWAP : 2;
> #else
>                  unsigned int                GRPH_ENDIAN_SWAP : 2;
>                  unsigned int                                 : 2;
>                  unsigned int               GRPH_RED_CROSSBAR : 2;
>                  unsigned int             GRPH_GREEN_CROSSBAR : 2;
>                  unsigned int              GRPH_BLUE_CROSSBAR : 2;
>                  unsigned int             GRPH_ALPHA_CROSSBAR : 2;
>                  unsigned int                                 : 20;
> #endif
>          } bitfields, bits;
>          unsigned int u32All;
>          signed int i32All;
>          float f32All;
> };
>
> A packet example:
>
> typedef union PM4_TYPE_3_HEADER
> {
>      struct
>      {
> #if BIG_ENDIAN
>          unsigned int type      : 2; ///< packet identifier. It should
> be 3 for type 3 packets
>          unsigned int count     : 14;///< number of DWORDs - 1 in the
> information body.
>          unsigned int opcode    : 8; ///< IT opcode
>          unsigned int reserved1 : 6; ///< reserved
>          unsigned int shaderType: 1; ///< 0: Graphics, 1: Compute Shader
>          unsigned int predicate : 1; ///< predicated version of packet when set
> #else
>          unsigned int predicate : 1; ///< predicated version of packet when set
>          unsigned int shaderType: 1; ///< 0: Graphics, 1: Compute Shader
>          unsigned int reserved1 : 6; ///< reserved
>          unsigned int opcode    : 8; ///< IT opcode
>          unsigned int count     : 14;///< number of DWORDs - 1 in the
> information body.
>          unsigned int type      : 2; ///< packet identifier. It should
> be 3 for type 3 packets
> #endif
>      };
>      unsigned int u32All;
> } PM4_TYPE_3_HEADER;
>
> //--------------------MEM_SEMAPHORE--------------------
>
> enum MEM_SEMAPHORE_signal_type_enum {
> signal_type_mem_semaphore_SIGNAL_TYPE_INCREMENT_0 = 0,
> signal_type_mem_semaphore_SIGNAL_TYPE_WRITE_1 = 1 };
> enum MEM_SEMAPHORE_client_code_enum { client_code_mem_semaphore_CP_0 =
> 0, client_code_mem_semaphore_CB_1 = 1, client_code_mem_semaphore_DB_2
> = 2, client_code_mem_semaphore_RESERVED_3 = 3 };
> enum MEM_SEMAPHORE_sem_sel_enum {
> sem_sel_mem_semaphore_SIGNAL_SEMAPHORE_6 = 6,
> sem_sel_mem_semaphore_WAIT_SEMAPHORE_7 = 7 };
>
> typedef struct _PM4_MEM_SEMAPHORE
> {
>      union
>      {
>          PM4_TYPE_3_HEADER   header;            ///header
>          unsigned int        ordinal1;
>      };
>
>      union
>      {
>          struct
>      {
> #if BIG_ENDIAN
>              unsigned int address_lo:29;
>              unsigned int reserved1:3;
> #else
>              unsigned int reserved1:3;
>              unsigned int address_lo:29;
> #endif
>          } bitfields2;
>          unsigned int ordinal2;
>      };
>
>      union
>      {
>          struct
>      {
> #if BIG_ENDIAN
>              MEM_SEMAPHORE_sem_sel_enum sem_sel:3;
>              unsigned int reserved4:3;
>              MEM_SEMAPHORE_client_code_enum client_code:2;
>              unsigned int reserved3:3;
>              MEM_SEMAPHORE_signal_type_enum signal_type:1;
>              unsigned int reserved2:3;
>              unsigned int use_mailbox:1;
>              unsigned int address_hi:16;
> #else
>              unsigned int address_hi:16;
>              unsigned int use_mailbox:1;
>              unsigned int reserved2:3;
>              MEM_SEMAPHORE_signal_type_enum signal_type:1;
>              unsigned int reserved3:3;
>              MEM_SEMAPHORE_client_code_enum client_code:2;
>              unsigned int reserved4:3;
>              MEM_SEMAPHORE_sem_sel_enum sem_sel:3;
> #endif
>          } bitfields3;
>          unsigned int ordinal3;
>      };
>
> }  PM4MEM_SEMAPHORE, *PPM4MEM_SEMAPHORE;
>
> Are there any strong objections to these sorts of structures?
>
> Thanks,
>
> Alex
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: bitfield structures
  2014-10-16 19:16 ` Christian König
@ 2014-10-16 21:14   ` Dave Airlie
  0 siblings, 0 replies; 9+ messages in thread
From: Dave Airlie @ 2014-10-16 21:14 UTC (permalink / raw)
  To: Christian König; +Cc: Maling list - DRI developers

On 17 October 2014 05:16, Christian König <deathsimple@vodafone.de> wrote:
> Well, making a long story short:
> http://yarchive.net/comp/linux/bitfields.html

Thanks, that++,

Just for that alone we should try and avoid using them in the driver.

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: bitfield structures
  2014-10-16 14:14 bitfield structures Alex Deucher
                   ` (3 preceding siblings ...)
  2014-10-16 19:16 ` Christian König
@ 2014-10-17  8:55 ` Michel Dänzer
  2014-10-17 11:19 ` Rob Clark
  5 siblings, 0 replies; 9+ messages in thread
From: Michel Dänzer @ 2014-10-17  8:55 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Maling list - DRI developers

On 16.10.2014 23:14, Alex Deucher wrote:
> As part of the amdgpu transition, we are moving to using database
> generated register and packet headers.  We have a number of options
> for formatting, some of which involve bitfields (don't worry there
> will also be shift/mask style headers as well which is mainly what we
> use in the code).  I think these formats are cleaner for a number of
> cases, however, as far as I know, C does not define the ordering of
> bits within bitfields.

That's only the tip of the iceberg of issues with bit-fields.

No bit-fields, please.


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: bitfield structures
  2014-10-16 14:14 bitfield structures Alex Deucher
                   ` (4 preceding siblings ...)
  2014-10-17  8:55 ` Michel Dänzer
@ 2014-10-17 11:19 ` Rob Clark
  2014-10-17 15:00   ` Alex Deucher
  5 siblings, 1 reply; 9+ messages in thread
From: Rob Clark @ 2014-10-17 11:19 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Maling list - DRI developers

btw, random question..  have you looked at all at the possibility of
generating envytools xml somehow from your internal db?  Not sure how
hard/easy that would be.  But then you could use headergen.  And who
knows, the rnndec register parsing stuff might be useful for things
like cmdstream decoder tools for debugging, etc..

BR,
-R

On Thu, Oct 16, 2014 at 10:14 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> As part of the amdgpu transition, we are moving to using database
> generated register and packet headers.  We have a number of options
> for formatting, some of which involve bitfields (don't worry there
> will also be shift/mask style headers as well which is mainly what we
> use in the code).  I think these formats are cleaner for a number of
> cases, however, as far as I know, C does not define the ordering of
> bits within bitfields.  That said, every compiler I've used seems to
> do what you would expect.  It makes coding a lot cleaner as less
> error-prone in certain cases.  Here are a couple of example of what
> I'm talking about:
>
> A register example:
>
> union GRPH_SWAP_CNTL {
>         struct {
> #if BIG_ENDIAN
>                 unsigned int                                 : 20;
>                 unsigned int             GRPH_ALPHA_CROSSBAR : 2;
>                 unsigned int              GRPH_BLUE_CROSSBAR : 2;
>                 unsigned int             GRPH_GREEN_CROSSBAR : 2;
>                 unsigned int               GRPH_RED_CROSSBAR : 2;
>                 unsigned int                                 : 2;
>                 unsigned int                GRPH_ENDIAN_SWAP : 2;
> #else
>                 unsigned int                GRPH_ENDIAN_SWAP : 2;
>                 unsigned int                                 : 2;
>                 unsigned int               GRPH_RED_CROSSBAR : 2;
>                 unsigned int             GRPH_GREEN_CROSSBAR : 2;
>                 unsigned int              GRPH_BLUE_CROSSBAR : 2;
>                 unsigned int             GRPH_ALPHA_CROSSBAR : 2;
>                 unsigned int                                 : 20;
> #endif
>         } bitfields, bits;
>         unsigned int u32All;
>         signed int i32All;
>         float f32All;
> };
>
> A packet example:
>
> typedef union PM4_TYPE_3_HEADER
> {
>     struct
>     {
> #if BIG_ENDIAN
>         unsigned int type      : 2; ///< packet identifier. It should
> be 3 for type 3 packets
>         unsigned int count     : 14;///< number of DWORDs - 1 in the
> information body.
>         unsigned int opcode    : 8; ///< IT opcode
>         unsigned int reserved1 : 6; ///< reserved
>         unsigned int shaderType: 1; ///< 0: Graphics, 1: Compute Shader
>         unsigned int predicate : 1; ///< predicated version of packet when set
> #else
>         unsigned int predicate : 1; ///< predicated version of packet when set
>         unsigned int shaderType: 1; ///< 0: Graphics, 1: Compute Shader
>         unsigned int reserved1 : 6; ///< reserved
>         unsigned int opcode    : 8; ///< IT opcode
>         unsigned int count     : 14;///< number of DWORDs - 1 in the
> information body.
>         unsigned int type      : 2; ///< packet identifier. It should
> be 3 for type 3 packets
> #endif
>     };
>     unsigned int u32All;
> } PM4_TYPE_3_HEADER;
>
> //--------------------MEM_SEMAPHORE--------------------
>
> enum MEM_SEMAPHORE_signal_type_enum {
> signal_type_mem_semaphore_SIGNAL_TYPE_INCREMENT_0 = 0,
> signal_type_mem_semaphore_SIGNAL_TYPE_WRITE_1 = 1 };
> enum MEM_SEMAPHORE_client_code_enum { client_code_mem_semaphore_CP_0 =
> 0, client_code_mem_semaphore_CB_1 = 1, client_code_mem_semaphore_DB_2
> = 2, client_code_mem_semaphore_RESERVED_3 = 3 };
> enum MEM_SEMAPHORE_sem_sel_enum {
> sem_sel_mem_semaphore_SIGNAL_SEMAPHORE_6 = 6,
> sem_sel_mem_semaphore_WAIT_SEMAPHORE_7 = 7 };
>
> typedef struct _PM4_MEM_SEMAPHORE
> {
>     union
>     {
>         PM4_TYPE_3_HEADER   header;            ///header
>         unsigned int        ordinal1;
>     };
>
>     union
>     {
>         struct
>     {
> #if BIG_ENDIAN
>             unsigned int address_lo:29;
>             unsigned int reserved1:3;
> #else
>             unsigned int reserved1:3;
>             unsigned int address_lo:29;
> #endif
>         } bitfields2;
>         unsigned int ordinal2;
>     };
>
>     union
>     {
>         struct
>     {
> #if BIG_ENDIAN
>             MEM_SEMAPHORE_sem_sel_enum sem_sel:3;
>             unsigned int reserved4:3;
>             MEM_SEMAPHORE_client_code_enum client_code:2;
>             unsigned int reserved3:3;
>             MEM_SEMAPHORE_signal_type_enum signal_type:1;
>             unsigned int reserved2:3;
>             unsigned int use_mailbox:1;
>             unsigned int address_hi:16;
> #else
>             unsigned int address_hi:16;
>             unsigned int use_mailbox:1;
>             unsigned int reserved2:3;
>             MEM_SEMAPHORE_signal_type_enum signal_type:1;
>             unsigned int reserved3:3;
>             MEM_SEMAPHORE_client_code_enum client_code:2;
>             unsigned int reserved4:3;
>             MEM_SEMAPHORE_sem_sel_enum sem_sel:3;
> #endif
>         } bitfields3;
>         unsigned int ordinal3;
>     };
>
> }  PM4MEM_SEMAPHORE, *PPM4MEM_SEMAPHORE;
>
> Are there any strong objections to these sorts of structures?
>
> Thanks,
>
> Alex
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: bitfield structures
  2014-10-17 11:19 ` Rob Clark
@ 2014-10-17 15:00   ` Alex Deucher
  0 siblings, 0 replies; 9+ messages in thread
From: Alex Deucher @ 2014-10-17 15:00 UTC (permalink / raw)
  To: Rob Clark; +Cc: Maling list - DRI developers

On Fri, Oct 17, 2014 at 7:19 AM, Rob Clark <robdclark@gmail.com> wrote:
> btw, random question..  have you looked at all at the possibility of
> generating envytools xml somehow from your internal db?  Not sure how
> hard/easy that would be.  But then you could use headergen.  And who
> knows, the rnndec register parsing stuff might be useful for things
> like cmdstream decoder tools for debugging, etc..

I can get xml from the dbs, so we can format it however we like, but
I'd like to keep the formats the same as what we use internally for
easier interop with internal tools and to aid in open sourcing other
internal packages.

Alex

>
> BR,
> -R
>
> On Thu, Oct 16, 2014 at 10:14 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>> As part of the amdgpu transition, we are moving to using database
>> generated register and packet headers.  We have a number of options
>> for formatting, some of which involve bitfields (don't worry there
>> will also be shift/mask style headers as well which is mainly what we
>> use in the code).  I think these formats are cleaner for a number of
>> cases, however, as far as I know, C does not define the ordering of
>> bits within bitfields.  That said, every compiler I've used seems to
>> do what you would expect.  It makes coding a lot cleaner as less
>> error-prone in certain cases.  Here are a couple of example of what
>> I'm talking about:
>>
>> A register example:
>>
>> union GRPH_SWAP_CNTL {
>>         struct {
>> #if BIG_ENDIAN
>>                 unsigned int                                 : 20;
>>                 unsigned int             GRPH_ALPHA_CROSSBAR : 2;
>>                 unsigned int              GRPH_BLUE_CROSSBAR : 2;
>>                 unsigned int             GRPH_GREEN_CROSSBAR : 2;
>>                 unsigned int               GRPH_RED_CROSSBAR : 2;
>>                 unsigned int                                 : 2;
>>                 unsigned int                GRPH_ENDIAN_SWAP : 2;
>> #else
>>                 unsigned int                GRPH_ENDIAN_SWAP : 2;
>>                 unsigned int                                 : 2;
>>                 unsigned int               GRPH_RED_CROSSBAR : 2;
>>                 unsigned int             GRPH_GREEN_CROSSBAR : 2;
>>                 unsigned int              GRPH_BLUE_CROSSBAR : 2;
>>                 unsigned int             GRPH_ALPHA_CROSSBAR : 2;
>>                 unsigned int                                 : 20;
>> #endif
>>         } bitfields, bits;
>>         unsigned int u32All;
>>         signed int i32All;
>>         float f32All;
>> };
>>
>> A packet example:
>>
>> typedef union PM4_TYPE_3_HEADER
>> {
>>     struct
>>     {
>> #if BIG_ENDIAN
>>         unsigned int type      : 2; ///< packet identifier. It should
>> be 3 for type 3 packets
>>         unsigned int count     : 14;///< number of DWORDs - 1 in the
>> information body.
>>         unsigned int opcode    : 8; ///< IT opcode
>>         unsigned int reserved1 : 6; ///< reserved
>>         unsigned int shaderType: 1; ///< 0: Graphics, 1: Compute Shader
>>         unsigned int predicate : 1; ///< predicated version of packet when set
>> #else
>>         unsigned int predicate : 1; ///< predicated version of packet when set
>>         unsigned int shaderType: 1; ///< 0: Graphics, 1: Compute Shader
>>         unsigned int reserved1 : 6; ///< reserved
>>         unsigned int opcode    : 8; ///< IT opcode
>>         unsigned int count     : 14;///< number of DWORDs - 1 in the
>> information body.
>>         unsigned int type      : 2; ///< packet identifier. It should
>> be 3 for type 3 packets
>> #endif
>>     };
>>     unsigned int u32All;
>> } PM4_TYPE_3_HEADER;
>>
>> //--------------------MEM_SEMAPHORE--------------------
>>
>> enum MEM_SEMAPHORE_signal_type_enum {
>> signal_type_mem_semaphore_SIGNAL_TYPE_INCREMENT_0 = 0,
>> signal_type_mem_semaphore_SIGNAL_TYPE_WRITE_1 = 1 };
>> enum MEM_SEMAPHORE_client_code_enum { client_code_mem_semaphore_CP_0 =
>> 0, client_code_mem_semaphore_CB_1 = 1, client_code_mem_semaphore_DB_2
>> = 2, client_code_mem_semaphore_RESERVED_3 = 3 };
>> enum MEM_SEMAPHORE_sem_sel_enum {
>> sem_sel_mem_semaphore_SIGNAL_SEMAPHORE_6 = 6,
>> sem_sel_mem_semaphore_WAIT_SEMAPHORE_7 = 7 };
>>
>> typedef struct _PM4_MEM_SEMAPHORE
>> {
>>     union
>>     {
>>         PM4_TYPE_3_HEADER   header;            ///header
>>         unsigned int        ordinal1;
>>     };
>>
>>     union
>>     {
>>         struct
>>     {
>> #if BIG_ENDIAN
>>             unsigned int address_lo:29;
>>             unsigned int reserved1:3;
>> #else
>>             unsigned int reserved1:3;
>>             unsigned int address_lo:29;
>> #endif
>>         } bitfields2;
>>         unsigned int ordinal2;
>>     };
>>
>>     union
>>     {
>>         struct
>>     {
>> #if BIG_ENDIAN
>>             MEM_SEMAPHORE_sem_sel_enum sem_sel:3;
>>             unsigned int reserved4:3;
>>             MEM_SEMAPHORE_client_code_enum client_code:2;
>>             unsigned int reserved3:3;
>>             MEM_SEMAPHORE_signal_type_enum signal_type:1;
>>             unsigned int reserved2:3;
>>             unsigned int use_mailbox:1;
>>             unsigned int address_hi:16;
>> #else
>>             unsigned int address_hi:16;
>>             unsigned int use_mailbox:1;
>>             unsigned int reserved2:3;
>>             MEM_SEMAPHORE_signal_type_enum signal_type:1;
>>             unsigned int reserved3:3;
>>             MEM_SEMAPHORE_client_code_enum client_code:2;
>>             unsigned int reserved4:3;
>>             MEM_SEMAPHORE_sem_sel_enum sem_sel:3;
>> #endif
>>         } bitfields3;
>>         unsigned int ordinal3;
>>     };
>>
>> }  PM4MEM_SEMAPHORE, *PPM4MEM_SEMAPHORE;
>>
>> Are there any strong objections to these sorts of structures?
>>
>> Thanks,
>>
>> Alex
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-10-17 15:00 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-16 14:14 bitfield structures Alex Deucher
2014-10-16 15:33 ` Jerome Glisse
2014-10-16 15:57 ` Peter Hurley
2014-10-16 15:57 ` Eric Anholt
2014-10-16 19:16 ` Christian König
2014-10-16 21:14   ` Dave Airlie
2014-10-17  8:55 ` Michel Dänzer
2014-10-17 11:19 ` Rob Clark
2014-10-17 15:00   ` Alex Deucher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.