Re: [PATCH v2 1/7] lib/igt_fb: Let others use igt_get_fb_tile_size

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu:
> This function can be used by igt_draw to get accurate
> tile dimensions for all tile formats.
> 
> v2: Added comments to function igt_get_fb_tile_size (Daniel)
> 
> Signed-off-by: Praveen Paneri <praveen.paneri@intel.com>
> ---
>  lib/igt_fb.c | 16 +++++++++++++---
>  lib/igt_fb.h |  3 ++-
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d2b7e9e..9ba1e3b 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -73,9 +73,19 @@ static struct format_desc_struct {
>  
>  #define for_each_format(f)	\
>  	for (f = format_desc; f - format_desc <
> ARRAY_SIZE(format_desc); f++)
> -

Please don't remove the blank line above.


> -static void igt_get_fb_tile_size(int fd, uint64_t tiling, int
> fb_bpp,
> -				 unsigned *width_ret, unsigned
> *height_ret)
> +/**
> + * igt_get_fb_tile_size:
> + * @fd: The DRM file descriptor
> + * @tiling: Tiling layout of the framebuffer (as framebuffer
> modifier)
> + * @fb_bpp: Bytes per pixel of the framebuffer

Bits, not bytes.


> + * @width_ret: Width of the tile in pixels

I think this is in bytes, not pixels.


> + * @height_ret: Height of the tile in pixels

I think saying "lines" instead of pixels would be more appropriate
here.


Oh, and since we're already going to have to change some other things,
here's an OCD bikeshed: the rest of the file seems to use:
 * @param: description
instead of:
 * @param: Description
so you may opt to use the current standard.


> + *
> + * This function returns width and height of a tile based on the
> given tiling
> + * format.
> + */
> +void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
> +			  unsigned *width_ret, unsigned *height_ret)
>  {
>  	switch (tiling) {
>  	case LOCAL_DRM_FORMAT_MOD_NONE:
> diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> index 4a680ce..414cb3d 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -94,7 +94,8 @@ enum igt_text_align {
>  	align_vcenter	= 0x04,
>  	align_hcenter	= 0x08,
>  };
> 
> -

Please don't remove the blank line above.


Thanks,
Paulo



> +void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
> +			  unsigned *width_ret, unsigned
> *height_ret);
>  void igt_calc_fb_size(int fd, int width, int height, int bpp,
> uint64_t tiling,
>  		      unsigned *size_ret, unsigned *stride_ret);
>  unsigned int
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx