Re: [PATCH 2/3] media: xilinx-vip: Add 8-bit YCbCr 4:2:0 to formats table

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

Hi Rohit,

Thank you for the patch.

On Wednesday 09 Aug 2017 11:27:53 Rohit Athavale wrote:
> Add Xilinx YCbCr 4:2:0 to xvip formats table. This commit
> will allow driver to setup media pad codes to YUV 420
> via DT properties.
> 
> Signed-off-by: Rohit Athavale <rathaval@xilinx.com>
> ---
>  drivers/media/platform/xilinx/xilinx-vip.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/platform/xilinx/xilinx-vip.c
> b/drivers/media/platform/xilinx/xilinx-vip.c index 3112591..37b80bf 100644
> --- a/drivers/media/platform/xilinx/xilinx-vip.c
> +++ b/drivers/media/platform/xilinx/xilinx-vip.c
> @@ -15,6 +15,7 @@
>  #include <linux/clk.h>
>  #include <linux/export.h>
>  #include <linux/kernel.h>
> +#include <linux/media-bus-format.h>

I'm pretty sure the file is included indirectly already, so this isn't 
strictly needed, but it shouldn't hurt either.

>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> 
> @@ -27,6 +28,8 @@
>   */
> 
>  static const struct xvip_video_format xvip_video_formats[] = {
> +	{ XVIP_VF_YUV_420, 8, NULL, MEDIA_BUS_FMT_XLNX8_VUY420_1X24,
> +	  2, V4L2_PIX_FMT_NV12, "4:2:0, semi-planar, YUYV" },

You're mapping XVIP_VF_YUV_420 to V4L2_PIX_FMT_NV12 which has an average bpp 
of 1.5 bytes per pixel, but you're setting bpp to 2. How does that work ? You 
obviously can't express a 1.5 bpp currently in the driver, so we might need to 
extend the xvip_video_format structure with additional fields (for instance 
turning bytes per pixel into bits per pixel, but we might need per-plane 
information too).

On a side note, how does this work with VDMA ? The latest VDMA version I 
checked (v6.2, a while ago) didn't seem to support planar formats. Has it 
changed in more recent versions ? Doesn't it require changes in the xilinx-vip 
driver ?

>  	{ XVIP_VF_YUV_422, 8, NULL, MEDIA_BUS_FMT_UYVY8_1X16,
>  	  2, V4L2_PIX_FMT_YUYV, "4:2:2, packed, YUYV" },
>  	{ XVIP_VF_YUV_444, 8, NULL, MEDIA_BUS_FMT_VUY8_1X24,

-- 
Regards,

Laurent Pinchart