2012/1/6 Christian K?nig <deathsimple at vodafone.de>:
> On 06.01.2012 15:12, Alex Deucher wrote:
>>
>> 2012/1/6 Christian K?nig<deathsimple at vodafone.de>:
>>>
>>> On -10.01.-28163 20:59, alexdeucher at gmail.com wrote:
>>> [SNIP]
>>>
>>>> ?#define RADEON_CHUNK_ID_RELOCS ? ? ? ?0x01
>>>> ?#define RADEON_CHUNK_ID_IB ? ?0x02
>>>> ?#define RADEON_CHUNK_ID_FLAGS 0x03
>>>>
>>>> ?/* The first dword of RADEON_CHUNK_ID_FLAGS is a uint32 of these flags:
>>>> */
>>>> ?#define RADEON_CS_KEEP_TILING_FLAGS 0x01
>>>> +#define RADEON_CS_USE_VM ? ? ? ? ? ?0x02
>>>> +/* The second dword of RADEON_CHUNK_ID_FLAGS is a uint32 that sets the
>>>> ring type */
>>>> +#define RADEON_CS_RING_GFX ? ? ? ? ?0
>>>> +#define RADEON_CS_RING_COMPUTE ? ? ?1
>>>> +/* The third dword of RADEON_CHUNK_ID_FLAGS is a sint32 that sets the
>>>> priority */
>>>> +/* 0 = normal, + = higher priority, - = lower priority */
>>>> +struct drm_radeon_cs_ring_priority {
>>>> + ? ? ? int32_t ? ? ? ? ? ? ? ? priority;
>>>> +};
>>>
>>> Sorry, missed that one before, the "struct drm_radeon_cs_ring_priority"
>>> is
>>> pretty much pointless.
>>
>> I didn't have a struct for priority before, but someone suggested it
>> would be clearer what the 3rd dword was used for if I added a struct.
>> Basically just treat the third flag dword as a signed int32.
>
> Yeah that suggestion came from me, but what I had in mind was a structure
> for the whole flags chunk, instead of just the priority field.
>
>
>>> My comment was going more into that direction:
>>> struct drm_radeon_cs_flags {
>>> ? ?uint32_t ? ? ?flags;
>>> ? ?uint32_t ? ? ?ring_type;
>>> ? ?int32_t ? ? ? ?priority;
>>> };
>>>
>>> Anyway, the patch is finally committed, but I think we should fix
>>> (remove?)
>>> that before it goes further upstream.
>>
>> Would there be any issues if we need to extend the stuct to add an
>> extra field later? ?Also, not all of the fields are necessary. ?You
>> can just allocate 1 dword if you just need the flags dword. ?I've fine
>> to just drop the struct.
>>
> Well, using structs as interface definitions has always suffered from those
> problems, e. g. for compatibility you can only extend it at the end, members
> might have ambiguous sizes, members might be packed differently..... ?I'm
> still just prefering them because they are machine readable, instead of
> human readable comments, and machines tend to produce less bugs than humans
> while calculations memory offsets...
>
> But I don't have enough experience with kernel interfaces to favor one style
> of coding over another, so if you think it's ok to drop it then just go
> ahead. I just wanted to note that there was a misunderstanding.I just went ahead and dropped it for now. I'm open too suggestions for better approaches though. Alex > > Christian.
