Re: [PATCH 11/11] drm/msm/dpu: Clean up dpu_media_info.h static inline functions

2018-11-06 Thread Sam Ravnborg 
Hi Jordan.

>   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);

As you touch this line, could you please add spaces around '*'
Same goes for use of '/' in a few places.

I assume checkpatch would have told you to fix this.

With this fixed you can add my:
Acked-by: Sam Ravnborg 

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

Re: [Freedreno] [PATCH 11/11] drm/msm/dpu: Clean up dpu_media_info.h static inline functions

2018-11-06 Thread Sean Paul 
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 
> ---
>  .../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 


>   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;
> +

[PATCH 11/11] drm/msm/dpu: Clean up dpu_media_info.h static inline functions

2018-11-05 Thread Jordan Crouse 
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 
---
 .../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);
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;
+

Re: [PATCH 11/11] drm/msm/dpu: Clean up dpu_media_info.h static inline functions

2018-10-19 Thread Bruce Wang 
On Thu, Oct 18, 2018 at 3:59 PM 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 

Reviewed-by: Bruce Wang 

> ---
>  .../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);
> 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

[PATCH 11/11] drm/msm/dpu: Clean up dpu_media_info.h static inline functions

2018-10-18 Thread Jordan Crouse 
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 
---
 .../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);
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;
+