Re: [PATCH v3 01/19] drm: Add |struct drm_gem_vram_object| and helpers

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1.1: Type: text/plain, Size: 3551 bytes --]

cc: noralf@tronnes.org

Am 03.05.19 um 14:07 schrieb Koenig, Christian:
> Am 03.05.19 um 14:01 schrieb Daniel Vetter:
>> [CAUTION: External Email]
>>
>> On Fri, May 3, 2019 at 12:15 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>> Hi Christian,
>>>
>>> would you review the whole patch set? Daniel mentioned that he'd prefer
>>> to leave the review to memory-mgmt developers.
>> I think Noralf Tronnes or Gerd Hoffmann would also make good reviewers
>> for this, fairly close to what they've been working on in the past.
> 
> I will try to take another look next week. Busy as usual here.

Thanks, I'll post v4 of the patches early next week.

> Christian.
> 
>> -Daniel
>>
>>> Best regards
>>> Thomas
>>>
>>> Am 30.04.19 um 11:35 schrieb Koenig, Christian:
>>>> Am 30.04.19 um 11:23 schrieb Sam Ravnborg:
>>>>> [CAUTION: External Email]
>>>>>
>>>>> Hi Thomas.
>>>>>
>>>>>>>> +
>>>>>>>> +/**
>>>>>>>> + * Returns the container of type &struct drm_gem_vram_object
>>>>>>>> + * for field bo.
>>>>>>>> + * @bo:           the VRAM buffer object
>>>>>>>> + * Returns:       The containing GEM VRAM object
>>>>>>>> + */
>>>>>>>> +static inline struct drm_gem_vram_object* drm_gem_vram_of_bo(
>>>>>>>> +  struct ttm_buffer_object *bo)
>>>>>>>> +{
>>>>>>>> +  return container_of(bo, struct drm_gem_vram_object, bo);
>>>>>>>> +}
>>>>>>> Indent funny. USe same indent as used in other parts of file for
>>>>>>> function arguments.
>>>>>> If I put the argument next to the function's name, it will exceed the
>>>>>> 80-character limit. From the coding-style document, I could not see what
>>>>>> to do in this case. One solution would move the return type to a
>>>>>> separate line before the function name. I've not seen that anywhere in
>>>>>> the source code, so moving the argument onto a separate line and
>>>>>> indenting by one tab appears to be the next best solution. Please let me
>>>>>> know if there's if there's a preferred style for cases like this one.
>>>>> Readability has IMO higher priority than some limit of 80 chars.
>>>>> And it hurts readability (at least my OCD) when style changes
>>>>> as you do with indent here. So my personal preference is to fix
>>>>> indent and accect longer lines.
>>>> In this case the an often used convention (which is also kind of
>>>> readable) is to add a newline after the return values, but before the
>>>> function name. E.g. something like this:
>>>>
>>>> static inline struct drm_gem_vram_object*
>>>> drm_gem_vram_of_bo(struct ttm_buffer_object *bo)
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>> But you ask for a preferred style - which I do not think we have in this
>>>>> case. So it boils down to what you prefer.
>>>>>
>>>>> Enough bikeshedding, thanks for the quick response.
>>>>>
>>>>>           Sam
>>> --
>>> Thomas Zimmermann
>>> Graphics Driver Developer
>>> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
>>> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
>>> HRB 21284 (AG Nürnberg)
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

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