Re: [PATCH v4 2/2] drm/atomic-helper: Replace drm_atomic_helper_check_crtc_state()

[Thomas Zimmermann <tzimmermann@suse.de>]

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

Hi

Am 07.10.22 um 09:29 schrieb Ville Syrjälä:
> On Fri, Oct 07, 2022 at 09:17:50AM +0200, Javier Martinez Canillas wrote:
>> On 10/7/22 09:07, Ville Syrjälä wrote:
>>> On Thu, Oct 06, 2022 at 10:28:12PM +0200, Javier Martinez Canillas wrote:
>>>> On 10/5/22 13:40, Thomas Zimmermann wrote:
>>>>> Rename the atomic helper function drm_atomic_helper_check_crtc_state()
>>>>> to drm_atomic_helper_check_crtc_primary_plane() and only check for an
>>>>> attached primary plane. Adapt callers.
>>>>>
>>>>> Instead of having one big function to check for various CRTC state
>>>>> conditions, we rather want smaller functions that drivers can pick
>>>>> individually.
>>>>>
>>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>>> ---
>>>>
>>>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>>>>
>>>> [...]
>>>>
>>>>> +	drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
>>>>> +		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
>>>>> +			return 0;
>>>>>   	}
>>>>
>>>> I believe the code convention is to drop the curly braces when you
>>>> have a single statement inside the a loop ?
>>>
>>> This has two.
>>>
>>
>> No, it has only one that is the if statement. So according to the Linux
>> kernel coding style AFAIU it should be written as:
>>
>> 	drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask)
>> 		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
>> 			return 0;
> 
> That is exactly what it says not to do.

Hey, no need to be so upfront about it. Without the outer braces, I'd 
find it hard to parse anyway.

Best regard
Thomas

> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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