| Message ID | 20190208031846.14453-5-vivek.kasireddy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for 32-bit packed YUV formats | expand |
On Thu, 2019-02-07 at 19:18 -0800, Vivek Kasireddy wrote: > Buffers generated with YUV32 format seems to be incorrect, hence use > VUYA32 instead. > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> Thanks! Acked-by: Philipp Zabel <p.zabel@pengutronix.de> regards Philipp
On 2/8/19 4:18 AM, Vivek Kasireddy wrote: > Buffers generated with YUV32 format seems to be incorrect, hence use > VUYA32 instead. > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> Philipp, I wonder whether VUYA32 or VUYX32 should be used? I think the alpha channel is completely ignored on the source side and on the destination side it is probably set by some fixed value? Note that there exists a control V4L2_CID_ALPHA_COMPONENT that can be used to let userspace specify the generated alpha value. What if both source and destination formats have an alpha channel? Can the hardware preserved the alpha values? Regards, Hans > --- > drivers/media/platform/imx-pxp.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c > index f087dc4fc729..70adbe38f802 100644 > --- a/drivers/media/platform/imx-pxp.c > +++ b/drivers/media/platform/imx-pxp.c > @@ -90,7 +90,7 @@ static struct pxp_fmt formats[] = { > .depth = 16, > .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT, > }, { > - .fourcc = V4L2_PIX_FMT_YUV32, > + .fourcc = V4L2_PIX_FMT_VUYA32, > .depth = 32, > .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT, > }, { > @@ -236,7 +236,7 @@ static u32 pxp_v4l2_pix_fmt_to_ps_format(u32 v4l2_pix_fmt) > case V4L2_PIX_FMT_RGB555: return BV_PXP_PS_CTRL_FORMAT__RGB555; > case V4L2_PIX_FMT_RGB444: return BV_PXP_PS_CTRL_FORMAT__RGB444; > case V4L2_PIX_FMT_RGB565: return BV_PXP_PS_CTRL_FORMAT__RGB565; > - case V4L2_PIX_FMT_YUV32: return BV_PXP_PS_CTRL_FORMAT__YUV1P444; > + case V4L2_PIX_FMT_VUYA32: return BV_PXP_PS_CTRL_FORMAT__YUV1P444; > case V4L2_PIX_FMT_UYVY: return BV_PXP_PS_CTRL_FORMAT__UYVY1P422; > case V4L2_PIX_FMT_YUYV: return BM_PXP_PS_CTRL_WB_SWAP | > BV_PXP_PS_CTRL_FORMAT__UYVY1P422; > @@ -265,7 +265,7 @@ static u32 pxp_v4l2_pix_fmt_to_out_format(u32 v4l2_pix_fmt) > case V4L2_PIX_FMT_RGB555: return BV_PXP_OUT_CTRL_FORMAT__RGB555; > case V4L2_PIX_FMT_RGB444: return BV_PXP_OUT_CTRL_FORMAT__RGB444; > case V4L2_PIX_FMT_RGB565: return BV_PXP_OUT_CTRL_FORMAT__RGB565; > - case V4L2_PIX_FMT_YUV32: return BV_PXP_OUT_CTRL_FORMAT__YUV1P444; > + case V4L2_PIX_FMT_VUYA32: return BV_PXP_OUT_CTRL_FORMAT__YUV1P444; > case V4L2_PIX_FMT_UYVY: return BV_PXP_OUT_CTRL_FORMAT__UYVY1P422; > case V4L2_PIX_FMT_VYUY: return BV_PXP_OUT_CTRL_FORMAT__VYUY1P422; > case V4L2_PIX_FMT_GREY: return BV_PXP_OUT_CTRL_FORMAT__Y8; > @@ -281,7 +281,7 @@ static u32 pxp_v4l2_pix_fmt_to_out_format(u32 v4l2_pix_fmt) > static bool pxp_v4l2_pix_fmt_is_yuv(u32 v4l2_pix_fmt) > { > switch (v4l2_pix_fmt) { > - case V4L2_PIX_FMT_YUV32: > + case V4L2_PIX_FMT_VUYA32: > case V4L2_PIX_FMT_UYVY: > case V4L2_PIX_FMT_YUYV: > case V4L2_PIX_FMT_VYUY: >
On Fri, 2019-02-08 at 10:15 +0100, Hans Verkuil wrote: > On 2/8/19 4:18 AM, Vivek Kasireddy wrote: > > Buffers generated with YUV32 format seems to be incorrect, hence use > > VUYA32 instead. > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> > > Philipp, I wonder whether VUYA32 or VUYX32 should be used? I think the alpha > channel is completely ignored on the source side and on the destination side > it is probably set by some fixed value? > > Note that there exists a control V4L2_CID_ALPHA_COMPONENT that can be used > to let userspace specify the generated alpha value. Oh, that is correct. V4L2_CID_ALPHA_COMPONENT is wired up in the imx-pxp driver to the alpha output override, and the value correctly appears in the VUYA32 output. > What if both source and destination formats have an alpha channel? Can the > hardware preserved the alpha values? The hardware should support preserving the alpha channel, but the driver currently always enables the V4L2_CID_ALPHA_COMPONENT override. Right now it would be correct to allow only VUYX32 on the output queue and both VUYA32 and VUYX32 on the capture queue. For VUYA32 on the output queue we'd have to disable the alpha override and check whether the hardware properly preserves the alpha channel. regards Philipp
Hi Vivek, On Thu, 2019-02-07 at 19:18 -0800, Vivek Kasireddy wrote: > Buffers generated with YUV32 format seems to be incorrect, hence use > VUYA32 instead. > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> > --- > drivers/media/platform/imx-pxp.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c > index f087dc4fc729..70adbe38f802 100644 > --- a/drivers/media/platform/imx-pxp.c > +++ b/drivers/media/platform/imx-pxp.c > @@ -90,7 +90,7 @@ static struct pxp_fmt formats[] = { > .depth = 16, > .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT, > }, { > - .fourcc = V4L2_PIX_FMT_YUV32, > + .fourcc = V4L2_PIX_FMT_VUYA32, After Hans' comment I noticed that to support VUYA32 input correctly, we'll have to disable the alpha override. Currently the alpha channel of buffers on the OUTPUT queue is ignored. The driver always uses the value of V4L2_CID_ALPHA_COMPONENT when writing alpha values. Therefore, it would be more correct to replace VUYA32 with VUYX32 everywhere. Or alternatively add both formats, but only VUYX32 on the output queue: }, { - .fourcc = V4L2_PIX_FMT_YUV32, + .fourcc = V4L2_PIX_FMT_VUYA32, + .depth = 32, + .types = MEM2MEM_CAPTURE, + }, { + .fourcc = V4L2_PIX_FMT_VUYX32, .depth = 32, > .depth = 32, > .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT, > }, { > @@ -236,7 +236,7 @@ static u32 pxp_v4l2_pix_fmt_to_ps_format(u32 v4l2_pix_fmt) > case V4L2_PIX_FMT_RGB555: return BV_PXP_PS_CTRL_FORMAT__RGB555; > case V4L2_PIX_FMT_RGB444: return BV_PXP_PS_CTRL_FORMAT__RGB444; > case V4L2_PIX_FMT_RGB565: return BV_PXP_PS_CTRL_FORMAT__RGB565; > - case V4L2_PIX_FMT_YUV32: return BV_PXP_PS_CTRL_FORMAT__YUV1P444; > + case V4L2_PIX_FMT_VUYA32: return BV_PXP_PS_CTRL_FORMAT__YUV1P444; This should be replaced with VUYX32: - case V4L2_PIX_FMT_YUV32: return BV_PXP_PS_CTRL_FORMAT__YUV1P444; + case V4L2_PIX_FMT_VUYX32: return BV_PXP_PS_CTRL_FORMAT__YUV1P444; > case V4L2_PIX_FMT_UYVY: return BV_PXP_PS_CTRL_FORMAT__UYVY1P422; > case V4L2_PIX_FMT_YUYV: return BM_PXP_PS_CTRL_WB_SWAP | > BV_PXP_PS_CTRL_FORMAT__UYVY1P422; > @@ -265,7 +265,7 @@ static u32 pxp_v4l2_pix_fmt_to_out_format(u32 v4l2_pix_fmt) > case V4L2_PIX_FMT_RGB555: return BV_PXP_OUT_CTRL_FORMAT__RGB555; > case V4L2_PIX_FMT_RGB444: return BV_PXP_OUT_CTRL_FORMAT__RGB444; > case V4L2_PIX_FMT_RGB565: return BV_PXP_OUT_CTRL_FORMAT__RGB565; > - case V4L2_PIX_FMT_YUV32: return BV_PXP_OUT_CTRL_FORMAT__YUV1P444; > + case V4L2_PIX_FMT_VUYA32: return BV_PXP_OUT_CTRL_FORMAT__YUV1P444; Here you could add both: - case V4L2_PIX_FMT_YUV32: return BV_PXP_OUT_CTRL_FORMAT__YUV1P444; + case V4L2_PIX_FMT_VUYA32: + case V4L2_PIX_FMT_VUYX32: return BV_PXP_OUT_CTRL_FORMAT__YUV1P444; > case V4L2_PIX_FMT_UYVY: return BV_PXP_OUT_CTRL_FORMAT__UYVY1P422; > case V4L2_PIX_FMT_VYUY: return BV_PXP_OUT_CTRL_FORMAT__VYUY1P422; > case V4L2_PIX_FMT_GREY: return BV_PXP_OUT_CTRL_FORMAT__Y8; > @@ -281,7 +281,7 @@ static u32 pxp_v4l2_pix_fmt_to_out_format(u32 v4l2_pix_fmt) > static bool pxp_v4l2_pix_fmt_is_yuv(u32 v4l2_pix_fmt) > { > switch (v4l2_pix_fmt) { > - case V4L2_PIX_FMT_YUV32: > + case V4L2_PIX_FMT_VUYA32: And here as well: - case V4L2_PIX_FMT_YUV32: + case V4L2_PIX_FMT_VUYA32: + case V4L2_PIX_FMT_VUYX32: > case V4L2_PIX_FMT_UYVY: > case V4L2_PIX_FMT_YUYV: > case V4L2_PIX_FMT_VYUY: regards Philipp
diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c index f087dc4fc729..70adbe38f802 100644 --- a/drivers/media/platform/imx-pxp.c +++ b/drivers/media/platform/imx-pxp.c @@ -90,7 +90,7 @@ static struct pxp_fmt formats[] = { .depth = 16, .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT, }, { - .fourcc = V4L2_PIX_FMT_YUV32, + .fourcc = V4L2_PIX_FMT_VUYA32, .depth = 32, .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT, }, { @@ -236,7 +236,7 @@ static u32 pxp_v4l2_pix_fmt_to_ps_format(u32 v4l2_pix_fmt) case V4L2_PIX_FMT_RGB555: return BV_PXP_PS_CTRL_FORMAT__RGB555; case V4L2_PIX_FMT_RGB444: return BV_PXP_PS_CTRL_FORMAT__RGB444; case V4L2_PIX_FMT_RGB565: return BV_PXP_PS_CTRL_FORMAT__RGB565; - case V4L2_PIX_FMT_YUV32: return BV_PXP_PS_CTRL_FORMAT__YUV1P444; + case V4L2_PIX_FMT_VUYA32: return BV_PXP_PS_CTRL_FORMAT__YUV1P444; case V4L2_PIX_FMT_UYVY: return BV_PXP_PS_CTRL_FORMAT__UYVY1P422; case V4L2_PIX_FMT_YUYV: return BM_PXP_PS_CTRL_WB_SWAP | BV_PXP_PS_CTRL_FORMAT__UYVY1P422; @@ -265,7 +265,7 @@ static u32 pxp_v4l2_pix_fmt_to_out_format(u32 v4l2_pix_fmt) case V4L2_PIX_FMT_RGB555: return BV_PXP_OUT_CTRL_FORMAT__RGB555; case V4L2_PIX_FMT_RGB444: return BV_PXP_OUT_CTRL_FORMAT__RGB444; case V4L2_PIX_FMT_RGB565: return BV_PXP_OUT_CTRL_FORMAT__RGB565; - case V4L2_PIX_FMT_YUV32: return BV_PXP_OUT_CTRL_FORMAT__YUV1P444; + case V4L2_PIX_FMT_VUYA32: return BV_PXP_OUT_CTRL_FORMAT__YUV1P444; case V4L2_PIX_FMT_UYVY: return BV_PXP_OUT_CTRL_FORMAT__UYVY1P422; case V4L2_PIX_FMT_VYUY: return BV_PXP_OUT_CTRL_FORMAT__VYUY1P422; case V4L2_PIX_FMT_GREY: return BV_PXP_OUT_CTRL_FORMAT__Y8; @@ -281,7 +281,7 @@ static u32 pxp_v4l2_pix_fmt_to_out_format(u32 v4l2_pix_fmt) static bool pxp_v4l2_pix_fmt_is_yuv(u32 v4l2_pix_fmt) { switch (v4l2_pix_fmt) { - case V4L2_PIX_FMT_YUV32: + case V4L2_PIX_FMT_VUYA32: case V4L2_PIX_FMT_UYVY: case V4L2_PIX_FMT_YUYV: case V4L2_PIX_FMT_VYUY:
Buffers generated with YUV32 format seems to be incorrect, hence use VUYA32 instead. Cc: Philipp Zabel <p.zabel@pengutronix.de> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> --- drivers/media/platform/imx-pxp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)