Re: [PATCH v2 6/7] drm/fb: small cleanup

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Rob,

Thank you for the patch.

On Wednesday 11 March 2015 10:23:13 Rob Clark wrote:
> Flip conditional to reduce indentation level of rest of fxn, and use
> min/max to make the code clearer.
> 
> v2: surface_width -> surface_height typo
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c index 1e6a0c7..dca98a4 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1035,22 +1035,24 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper, for (i = 0; i < fb_helper->crtc_count; i++) {
>  		struct drm_display_mode *desired_mode;
>  		int x, y;
> +
>  		desired_mode = fb_helper->crtc_info[i].desired_mode;
> +
> +		if (!desired_mode)
> +			continue;
> +
> +		crtc_count++;
> +
>  		x = fb_helper->crtc_info[i].x;
>  		y = fb_helper->crtc_info[i].y;
> -		if (desired_mode) {
> -			if (gamma_size == 0)
> -				gamma_size = fb_helper->crtc_info[i].mode_set.crtc-
>gamma_size;
> -			if (desired_mode->hdisplay + x < sizes.fb_width)
> -				sizes.fb_width = desired_mode->hdisplay + x;
> -			if (desired_mode->vdisplay + y < sizes.fb_height)
> -				sizes.fb_height = desired_mode->vdisplay + y;
> -			if (desired_mode->hdisplay + x > sizes.surface_width)
> -				sizes.surface_width = desired_mode->hdisplay + x;
> -			if (desired_mode->vdisplay + y > sizes.surface_height)
> -				sizes.surface_height = desired_mode->vdisplay + y;
> -			crtc_count++;
> -		}
> +
> +		if (gamma_size == 0)
> +			gamma_size = fb_helper->crtc_info[i].mode_set.crtc->gamma_size;
> +
> +		sizes.surface_width  = max_t(u32, desired_mode->hdisplay + x,
> sizes.surface_width);
> +		sizes.surface_height = max_t(u32, desired_mode->vdisplay + y,
> sizes.surface_height);
> +		sizes.fb_width  = min_t(u32, desired_mode->hdisplay + x,
> sizes.fb_width);
> +		sizes.fb_height = min_t(u32, desired_mode->vdisplay + y,
> sizes.fb_height); }

Nitpicking, reducing the indentation level is nice, but you're making lines 
longer. I would add line breaks here.

>  	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {

-- 
Regards,

Laurent Pinchart

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