Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Oct 8, 2011 at 1:32 PM, Thomas Hellstrom <thomas@xxxxxxxxxxxx> wrote:
> On 10/08/2011 01:27 PM, Ville Syrjälä wrote:
>>
>> On Sat, Oct 08, 2011 at 01:10:13PM +0200, Thomas Hellstrom wrote:
>>
>>>
>>> On 10/08/2011 12:26 PM, Ville Syrjälä wrote:
>>>
>>>>
>>>> On Fri, Oct 07, 2011 at 10:58:13AM +0200, Thomas Hellstrom wrote:
>>>>
>>>>
>>>>>
>>>>> Oh, and one more style comment below:
>>>>>
>>>>> On 08/07/2011 10:39 PM, Marek Olšák wrote:
>>>>>
>>>>>
>>>>>>
>>>>>> +enum ttm_buffer_usage {
>>>>>> +    TTM_USAGE_READ = 1,
>>>>>> +    TTM_USAGE_WRITE = 2,
>>>>>> +    TTM_USAGE_READWRITE = TTM_USAGE_READ | TTM_USAGE_WRITE
>>>>>> +};
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> Please don't use enums for bit operations.
>>>>>
>>>>>
>>>>
>>>> Now I'm curious. Why not?
>>>>
>>>>
>>>>
>>>
>>> Because it's inconsistent with how flags are defined in the rest of the
>>> TTM module.
>>>
>>
>> Ah OK. I was wondering if there's some subtle technical issue involved.
>> I've recently gotten to the habit of using enums for pretty much all
>> constants. Just easier on the eye IMHO, and avoids cpp output from
>> looking like number soup.
>>
>>
>
> Yes, there are a number of advantages, including symbolic debugger output.
> If we had flag enums that enumerated 1, 2, 4, 8 etc. I'd feel motivated to
> move
> all TTM definitions over.

I don't think that how it is enumerated matters in any way. What I
like about enums, besides what has already been mentioned, is that it
adds a self-documentation in the code. Compare:

void ttm_set_bo_flags(unsigned flags);

And:

void ttm_set_bo_flags(enum ttm_bo_flags flags);

The latter is way easier to understand for somebody who doesn't know
the code and wants to implement his first patch. With the latter, it's
clear at first glance what are the valid values for "flags", because
you can just search for "enum ttm_bo_flags".

I will change the enum to defines for the sake of following your code
style convention, but it's an unreasonable convention to say the
least.

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



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux