Re: [PATCH v2 06/18] drm: Add drm_format_plane_width() and drm_format_plane_height()

Daniel Vetter
2016-01-25T17:08:23Z

On Wed, Jan 20, 2016 at 09:05:27PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add a few helpers to get the dimensions of the chroma plane(s).
>
> v2: Add kernel-doc (Daniel)
>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> include/drm/drm_crtc.h | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c65a212db77e..91195c403422 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -2482,6 +2482,36 @@ extern int drm_format_num_planes(uint32_t format);
> extern int drm_format_plane_cpp(uint32_t format, int plane);
> extern int drm_format_horz_chroma_subsampling(uint32_t format);
> extern int drm_format_vert_chroma_subsampling(uint32_t format);
> +/**
> + * drm_format_plane_width - width of the plane given the first plane
> + * @width: width of the first plane
> + * @format: pixel format
> + * @plane: plane index
> + *

kerneldoc style is

+ * Returns:
+ * blabla

> + * Returns the width of @plane, given that the width of the first plane
> + * is @width.
> + */
> +static inline int drm_format_plane_width(int width, uint32_t format, int plane)
> +{

For consistency with other helpers I think we should put an

if (plane >= drm_format_num_planes(format))
return 0;

here. Also I think static inline is overkill for these, and grouping them
together with the others in drm_crtc.c will make it easier to extract them
into a new file (since drm_crtc.c is kinda sprawling a bit).

With those nitpicks applied: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> + if (plane == 0)
> + return width;
> + return width / drm_format_horz_chroma_subsampling(format);
> +}
> +/**
> + * drm_format_plane_height - height of the plane given the first plane
> + * @height: height of the first plane
> + * @format: pixel format
> + * @plane: plane index
> + *
> + * Returns the height of @plane, given that the height of the first plane
> + * is @height.
> + */
> +static inline int drm_format_plane_height(int height, uint32_t format, int plane)
> +{
> + if (plane == 0)
> + return height;
> + return height / drm_format_vert_chroma_subsampling(format);
> +}
> extern const char *drm_get_format_name(uint32_t format);
> extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> unsigned int supported_rotations);
> --
> 2.4.10
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org