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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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.

-- 
Ville Syrjälä
Intel