public inbox for linux-arm-msm@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Paul <sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
To: Jordan Crouse <jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: sam-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org,
	seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org,
	abhinavk-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 11/11] drm/msm/dpu: Clean up dpu_media_info.h static inline functions
Date: Tue, 6 Nov 2018 10:13:48 -0500	[thread overview]
Message-ID: <20181106151348.GV154160@art_vandelay> (raw)
In-Reply-To: <20181105233103.7657-12-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

On Mon, Nov 05, 2018 at 04:31:03PM -0700, Jordan Crouse wrote:
> Do some cleanup in the static inline functions defined in
> dpu_media_info.h by cleaning up gotos and unneeded local
> variables.
> 
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>  .../gpu/drm/msm/disp/dpu1/msm_media_info.h    | 164 ++++++------------
>  1 file changed, 57 insertions(+), 107 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/msm_media_info.h b/drivers/gpu/drm/msm/disp/dpu1/msm_media_info.h
> index 75470ee5b18f..8b8309f25c1a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/msm_media_info.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/msm_media_info.h
> @@ -822,36 +822,30 @@ enum color_fmts {
>   */
>  static unsigned int VENUS_Y_STRIDE(int color_fmt, int width)
>  {
> -	unsigned int alignment, stride = 0;
> +	unsigned int stride = 0;
>  
>  	if (!width)
> -		goto invalid_input;
> +		return 0;
>  
>  	switch (color_fmt) {
>  	case COLOR_FMT_NV21:
>  	case COLOR_FMT_NV12:
>  	case COLOR_FMT_NV12_MVTB:
>  	case COLOR_FMT_NV12_UBWC:
> -		alignment = 128;
> -		stride = MSM_MEDIA_ALIGN(width, alignment);
> +		stride = MSM_MEDIA_ALIGN(width, 128);
>  		break;
>  	case COLOR_FMT_NV12_BPP10_UBWC:
> -		alignment = 256;
>  		stride = MSM_MEDIA_ALIGN(width, 192);
> -		stride = MSM_MEDIA_ALIGN(stride * 4/3, alignment);
> +		stride = MSM_MEDIA_ALIGN(stride * 4/3, 256);

nit: Can you please surround the binary operators with spaces? This applies
everywhere in the patch.

With that,

Reviewed-by: Sean Paul <sean@poorly.run>


>  		break;
>  	case COLOR_FMT_P010_UBWC:
> -		alignment = 256;
> -		stride = MSM_MEDIA_ALIGN(width * 2, alignment);
> +		stride = MSM_MEDIA_ALIGN(width * 2, 256);
>  		break;
>  	case COLOR_FMT_P010:
> -		alignment = 128;
> -		stride = MSM_MEDIA_ALIGN(width*2, alignment);
> -		break;
> -	default:
> +		stride = MSM_MEDIA_ALIGN(width * 2, 128);
>  		break;
>  	}
> -invalid_input:
> +
>  	return stride;
>  }
>  
> @@ -864,36 +858,30 @@ static unsigned int VENUS_Y_STRIDE(int color_fmt, int width)
>   */
>  static unsigned int VENUS_UV_STRIDE(int color_fmt, int width)
>  {
> -	unsigned int alignment, stride = 0;
> +	unsigned int stride = 0;
>  
>  	if (!width)
> -		goto invalid_input;
> +		return 0;
>  
>  	switch (color_fmt) {
>  	case COLOR_FMT_NV21:
>  	case COLOR_FMT_NV12:
>  	case COLOR_FMT_NV12_MVTB:
>  	case COLOR_FMT_NV12_UBWC:
> -		alignment = 128;
> -		stride = MSM_MEDIA_ALIGN(width, alignment);
> +		stride = MSM_MEDIA_ALIGN(width, 128);
>  		break;
>  	case COLOR_FMT_NV12_BPP10_UBWC:
> -		alignment = 256;
>  		stride = MSM_MEDIA_ALIGN(width, 192);
> -		stride = MSM_MEDIA_ALIGN(stride * 4/3, alignment);
> +		stride = MSM_MEDIA_ALIGN(stride * 4/3, 256);
>  		break;
>  	case COLOR_FMT_P010_UBWC:
> -		alignment = 256;
> -		stride = MSM_MEDIA_ALIGN(width * 2, alignment);
> +		stride = MSM_MEDIA_ALIGN(width * 2, 256);
>  		break;
>  	case COLOR_FMT_P010:
> -		alignment = 128;
> -		stride = MSM_MEDIA_ALIGN(width*2, alignment);
> -		break;
> -	default:
> +		stride = MSM_MEDIA_ALIGN(width*2, 128);
>  		break;
>  	}
> -invalid_input:
> +
>  	return stride;
>  }
>  
> @@ -906,10 +894,10 @@ static unsigned int VENUS_UV_STRIDE(int color_fmt, int width)
>   */
>  static unsigned int VENUS_Y_SCANLINES(int color_fmt, int height)
>  {
> -	unsigned int alignment, sclines = 0;
> +	unsigned int sclines = 0;
>  
>  	if (!height)
> -		goto invalid_input;
> +		return 0;
>  
>  	switch (color_fmt) {
>  	case COLOR_FMT_NV21:
> @@ -917,17 +905,14 @@ static unsigned int VENUS_Y_SCANLINES(int color_fmt, int height)
>  	case COLOR_FMT_NV12_MVTB:
>  	case COLOR_FMT_NV12_UBWC:
>  	case COLOR_FMT_P010:
> -		alignment = 32;
> +		sclines = MSM_MEDIA_ALIGN(height, 32);
>  		break;
>  	case COLOR_FMT_NV12_BPP10_UBWC:
>  	case COLOR_FMT_P010_UBWC:
> -		alignment = 16;
> +		sclines = MSM_MEDIA_ALIGN(height, 16);
>  		break;
> -	default:
> -		return 0;
>  	}
> -	sclines = MSM_MEDIA_ALIGN(height, alignment);
> -invalid_input:
> +
>  	return sclines;
>  }
>  
> @@ -940,10 +925,10 @@ static unsigned int VENUS_Y_SCANLINES(int color_fmt, int height)
>   */
>  static unsigned int VENUS_UV_SCANLINES(int color_fmt, int height)
>  {
> -	unsigned int alignment, sclines = 0;
> +	unsigned int sclines = 0;
>  
>  	if (!height)
> -		goto invalid_input;
> +		return 0;
>  
>  	switch (color_fmt) {
>  	case COLOR_FMT_NV21:
> @@ -952,18 +937,13 @@ static unsigned int VENUS_UV_SCANLINES(int color_fmt, int height)
>  	case COLOR_FMT_NV12_BPP10_UBWC:
>  	case COLOR_FMT_P010_UBWC:
>  	case COLOR_FMT_P010:
> -		alignment = 16;
> +		sclines = MSM_MEDIA_ALIGN((height+1)>>1, 16);
>  		break;
>  	case COLOR_FMT_NV12_UBWC:
> -		alignment = 32;
> +		sclines = MSM_MEDIA_ALIGN((height+1)>>1, 32);
>  		break;
> -	default:
> -		goto invalid_input;
>  	}
>  
> -	sclines = MSM_MEDIA_ALIGN((height+1)>>1, alignment);
> -
> -invalid_input:
>  	return sclines;
>  }
>  
> @@ -976,10 +956,10 @@ static unsigned int VENUS_UV_SCANLINES(int color_fmt, int height)
>   */
>  static unsigned int VENUS_Y_META_STRIDE(int color_fmt, int width)
>  {
> -	int y_tile_width = 0, y_meta_stride = 0;
> +	int y_tile_width = 0, y_meta_stride;
>  
>  	if (!width)
> -		goto invalid_input;
> +		return 0;
>  
>  	switch (color_fmt) {
>  	case COLOR_FMT_NV12_UBWC:
> @@ -990,14 +970,11 @@ static unsigned int VENUS_Y_META_STRIDE(int color_fmt, int width)
>  		y_tile_width = 48;
>  		break;
>  	default:
> -		goto invalid_input;
> +		return 0;
>  	}
>  
>  	y_meta_stride = MSM_MEDIA_ROUNDUP(width, y_tile_width);
> -	y_meta_stride = MSM_MEDIA_ALIGN(y_meta_stride, 64);
> -
> -invalid_input:
> -	return y_meta_stride;
> +	return MSM_MEDIA_ALIGN(y_meta_stride, 64);
>  }
>  
>  /*
> @@ -1009,10 +986,10 @@ static unsigned int VENUS_Y_META_STRIDE(int color_fmt, int width)
>   */
>  static unsigned int VENUS_Y_META_SCANLINES(int color_fmt, int height)
>  {
> -	int y_tile_height = 0, y_meta_scanlines = 0;
> +	int y_tile_height = 0, y_meta_scanlines;
>  
>  	if (!height)
> -		goto invalid_input;
> +		return 0;
>  
>  	switch (color_fmt) {
>  	case COLOR_FMT_NV12_UBWC:
> @@ -1023,14 +1000,11 @@ static unsigned int VENUS_Y_META_SCANLINES(int color_fmt, int height)
>  		y_tile_height = 4;
>  		break;
>  	default:
> -		goto invalid_input;
> +		return 0;
>  	}
>  
>  	y_meta_scanlines = MSM_MEDIA_ROUNDUP(height, y_tile_height);
> -	y_meta_scanlines = MSM_MEDIA_ALIGN(y_meta_scanlines, 16);
> -
> -invalid_input:
> -	return y_meta_scanlines;
> +	return MSM_MEDIA_ALIGN(y_meta_scanlines, 16);
>  }
>  
>  /*
> @@ -1042,10 +1016,10 @@ static unsigned int VENUS_Y_META_SCANLINES(int color_fmt, int height)
>   */
>  static unsigned int VENUS_UV_META_STRIDE(int color_fmt, int width)
>  {
> -	int uv_tile_width = 0, uv_meta_stride = 0;
> +	int uv_tile_width = 0, uv_meta_stride;
>  
>  	if (!width)
> -		goto invalid_input;
> +		return 0;
>  
>  	switch (color_fmt) {
>  	case COLOR_FMT_NV12_UBWC:
> @@ -1056,14 +1030,11 @@ static unsigned int VENUS_UV_META_STRIDE(int color_fmt, int width)
>  		uv_tile_width = 24;
>  		break;
>  	default:
> -		goto invalid_input;
> +		return 0;
>  	}
>  
>  	uv_meta_stride = MSM_MEDIA_ROUNDUP((width+1)>>1, uv_tile_width);
> -	uv_meta_stride = MSM_MEDIA_ALIGN(uv_meta_stride, 64);
> -
> -invalid_input:
> -	return uv_meta_stride;
> +	return MSM_MEDIA_ALIGN(uv_meta_stride, 64);
>  }
>  
>  /*
> @@ -1075,10 +1046,10 @@ static unsigned int VENUS_UV_META_STRIDE(int color_fmt, int width)
>   */
>  static unsigned int VENUS_UV_META_SCANLINES(int color_fmt, int height)
>  {
> -	int uv_tile_height = 0, uv_meta_scanlines = 0;
> +	int uv_tile_height = 0, uv_meta_scanlines;
>  
>  	if (!height)
> -		goto invalid_input;
> +		return 0;
>  
>  	switch (color_fmt) {
>  	case COLOR_FMT_NV12_UBWC:
> @@ -1089,22 +1060,19 @@ static unsigned int VENUS_UV_META_SCANLINES(int color_fmt, int height)
>  		uv_tile_height = 4;
>  		break;
>  	default:
> -		goto invalid_input;
> +		return 0;
>  	}
>  
>  	uv_meta_scanlines = MSM_MEDIA_ROUNDUP((height+1)>>1, uv_tile_height);
> -	uv_meta_scanlines = MSM_MEDIA_ALIGN(uv_meta_scanlines, 16);
> -
> -invalid_input:
> -	return uv_meta_scanlines;
> +	return MSM_MEDIA_ALIGN(uv_meta_scanlines, 16);
>  }
>  
>  static unsigned int VENUS_RGB_STRIDE(int color_fmt, int width)
>  {
> -	unsigned int alignment = 0, stride = 0, bpp = 4;
> +	unsigned int alignment = 0, bpp = 4;
>  
>  	if (!width)
> -		goto invalid_input;
> +		return 0;
>  
>  	switch (color_fmt) {
>  	case COLOR_FMT_RGBA8888:
> @@ -1119,21 +1087,18 @@ static unsigned int VENUS_RGB_STRIDE(int color_fmt, int width)
>  		alignment = 256;
>  		break;
>  	default:
> -		goto invalid_input;
> +		return 0;
>  	}
>  
> -	stride = MSM_MEDIA_ALIGN(width * bpp, alignment);
> -
> -invalid_input:
> -	return stride;
> +	return MSM_MEDIA_ALIGN(width * bpp, alignment);
>  }
>  
>  static unsigned int VENUS_RGB_SCANLINES(int color_fmt, int height)
>  {
> -	unsigned int alignment = 0, scanlines = 0;
> +	unsigned int alignment = 0;
>  
>  	if (!height)
> -		goto invalid_input;
> +		return 0;
>  
>  	switch (color_fmt) {
>  	case COLOR_FMT_RGBA8888:
> @@ -1145,61 +1110,46 @@ static unsigned int VENUS_RGB_SCANLINES(int color_fmt, int height)
>  		alignment = 16;
>  		break;
>  	default:
> -		goto invalid_input;
> +		return 0;
>  	}
>  
> -	scanlines = MSM_MEDIA_ALIGN(height, alignment);
> -
> -invalid_input:
> -	return scanlines;
> +	return MSM_MEDIA_ALIGN(height, alignment);
>  }
>  
>  static unsigned int VENUS_RGB_META_STRIDE(int color_fmt, int width)
>  {
> -	int rgb_tile_width = 0, rgb_meta_stride = 0;
> +	int rgb_meta_stride;
>  
>  	if (!width)
> -		goto invalid_input;
> +		return 0;
>  
>  	switch (color_fmt) {
>  	case COLOR_FMT_RGBA8888_UBWC:
>  	case COLOR_FMT_RGBA1010102_UBWC:
>  	case COLOR_FMT_RGB565_UBWC:
> -		rgb_tile_width = 16;
> -		break;
> -	default:
> -		goto invalid_input;
> +		rgb_meta_stride = MSM_MEDIA_ROUNDUP(width, 16);
> +		return MSM_MEDIA_ALIGN(rgb_meta_stride, 64);
>  	}
>  
> -	rgb_meta_stride = MSM_MEDIA_ROUNDUP(width, rgb_tile_width);
> -	rgb_meta_stride = MSM_MEDIA_ALIGN(rgb_meta_stride, 64);
> -
> -invalid_input:
> -	return rgb_meta_stride;
> +	return 0;
>  }
>  
>  static unsigned int VENUS_RGB_META_SCANLINES(int color_fmt, int height)
>  {
> -	int rgb_tile_height = 0, rgb_meta_scanlines = 0;
> +	int rgb_meta_scanlines;
>  
>  	if (!height)
> -		goto invalid_input;
> +		return 0;
>  
>  	switch (color_fmt) {
>  	case COLOR_FMT_RGBA8888_UBWC:
>  	case COLOR_FMT_RGBA1010102_UBWC:
>  	case COLOR_FMT_RGB565_UBWC:
> -		rgb_tile_height = 4;
> -		break;
> -	default:
> -		goto invalid_input;
> +		rgb_meta_scanlines = MSM_MEDIA_ROUNDUP(height, 4);
> +		return MSM_MEDIA_ALIGN(rgb_meta_scanlines, 16);
>  	}
>  
> -	rgb_meta_scanlines = MSM_MEDIA_ROUNDUP(height, rgb_tile_height);
> -	rgb_meta_scanlines = MSM_MEDIA_ALIGN(rgb_meta_scanlines, 16);
> -
> -invalid_input:
> -	return rgb_meta_scanlines;
> +	return 0;
>  }
>  
>  #endif
> -- 
> 2.18.0
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

  parent reply	other threads:[~2018-11-06 15:13 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-05 23:30 [PATCH v2 00/11] DPU cleanups Jordan Crouse
2018-11-05 23:30 ` [PATCH 01/11] drm/msm/dpu: Remove dpu_dbg Jordan Crouse
     [not found]   ` <20181105233103.7657-2-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-11-06 14:22     ` Sean Paul
     [not found] ` <20181105233103.7657-1-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-11-05 23:30   ` [PATCH 02/11] drm/msm/dpu: Use DEFINE_SHOW_ATTRIBUTE Jordan Crouse
2018-11-06 14:28     ` Sean Paul
2018-11-05 23:30   ` [PATCH 03/11] drm/msm/dpu: Remove dpu_crtc_get_mixer_height Jordan Crouse
     [not found]     ` <20181105233103.7657-4-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-11-06 14:30       ` Sean Paul
2018-11-05 23:30   ` [PATCH 04/11] drm/msm/dpu: Remove dpu_crtc_is_enabled() Jordan Crouse
2018-11-06 14:31     ` Sean Paul
2018-11-05 23:30   ` [PATCH 05/11] drm/msm/dpu: Remove unused functions Jordan Crouse
2018-11-06 14:31     ` Sean Paul
2018-11-05 23:30   ` [PATCH 06/11] drm/msm/dpu: Cleanup callers of dpu_hw_blk_init Jordan Crouse
     [not found]     ` <20181105233103.7657-7-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-11-06 14:45       ` Sean Paul
2018-11-05 23:30   ` [PATCH 07/11] drm/msm: Make irq_postinstall optional Jordan Crouse
     [not found]     ` <20181105233103.7657-8-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-11-06 14:46       ` Sean Paul
2018-11-05 23:31   ` [PATCH 08/11] drm/msm/dpu: Remove dpu_irq and unused functions Jordan Crouse
     [not found]     ` <20181105233103.7657-9-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-11-06 14:46       ` Sean Paul
2018-11-05 23:31   ` [PATCH 09/11] drm/msm/dpu: Cleanup the debugfs functions Jordan Crouse
     [not found]     ` <20181105233103.7657-10-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-11-06 15:05       ` Sean Paul
2018-11-05 23:31   ` [PATCH 11/11] drm/msm/dpu: Clean up dpu_media_info.h static inline functions Jordan Crouse
     [not found]     ` <20181105233103.7657-12-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-11-06 15:13       ` Sean Paul [this message]
2018-11-06 16:16       ` Sam Ravnborg
2018-11-07 19:55   ` [PATCH v3 1/2] drm/msm/dpu: Further cleanups for " Jordan Crouse
     [not found]     ` <20181107195503.12204-1-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-11-07 19:55       ` [PATCH v3 2/2] drm/msm/dpu: Clean up dpu_media_info.h " Jordan Crouse
2018-11-05 23:31 ` [PATCH 10/11] drm/msm/dpu: Further cleanups for " Jordan Crouse
2018-11-06 15:12   ` Sean Paul
  -- strict thread matches above, loose matches on Subject: below --
2018-10-18 19:58 [PATCH 00/11] DPU cleanups Jordan Crouse
     [not found] ` <20181018195836.15885-1-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-10-18 19:58   ` [PATCH 11/11] drm/msm/dpu: Clean up dpu_media_info.h static inline functions Jordan Crouse
     [not found]     ` <20181018195836.15885-12-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-10-19 15:10       ` Bruce Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181106151348.GV154160@art_vandelay \
    --to=sean-p7ytbzm4h96eqtr555yldq@public.gmane.org \
    --cc=abhinavk-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=sam-uyr5N9Q2VtJg9hUCZPvPmw@public.gmane.org \
    --cc=seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox