Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1618094
| From | Philipp Zabel <p.zabel@pengutronix.de> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default |
| Date | 2017-04-06 17:00 +0200 |
| Message-ID | <tth3c-1Ia-11@gated-at.bofh.it> (permalink) |
| References | <tpNuG-7Yq-3@gated-at.bofh.it> <tpNuH-7Yq-53@gated-at.bofh.it> <ttg77-MK-13@gated-at.bofh.it> <ttgAa-1jL-15@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Thu, 2017-04-06 at 16:20 +0200, Hans Verkuil wrote:
> On 04/06/2017 03:55 PM, Philipp Zabel wrote:
> > If the the field order is set to ANY in set_fmt, choose the currently
> > set field order. If the colorspace is set to DEFAULT, choose the current
> > colorspace. If any of xfer_func, ycbcr_enc or quantization are set to
> > DEFAULT, either choose the current setting, or the default setting for the
> > new colorspace, if non-DEFAULT colorspace was given.
> >
> > This allows to let field order and colorimetry settings be propagated
> > from upstream by calling media-ctl on the upstream entity source pad,
> > and then call media-ctl on the sink pad to manually set the input frame
> > interval, without changing the already set field order and colorimetry
> > information.
> >
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> > This is based on imx-media-staging-md-v14, and it is supposed to allow
> > configuring the pipeline with media-ctl like this:
> >
> > 1) media-ctl --set-v4l2 "'tc358743 1-000f':0[fmt:UYVY8_1X16/1920x1080]"
> > 2) media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY8_1X16/1920x108]"
> > 3) media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY8_1X16/1920x1080]"
> > 4) media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY8_1X16/1920x1080@1/60]"
> > 5) media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/1920x1080@1/30]"
> >
> > Without having step 4) overwrite the colorspace and field order set on
> > 'ipu1_csi0':0 by the propagation in step 3).
> > ---
> > drivers/staging/media/imx/imx-media-csi.c | 34 +++++++++++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> > index 64dc454f6b371..d94ce1de2bf05 100644
> > --- a/drivers/staging/media/imx/imx-media-csi.c
> > +++ b/drivers/staging/media/imx/imx-media-csi.c
> > @@ -1325,6 +1325,40 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
> > csi_try_fmt(priv, sensor, cfg, sdformat, crop, compose, &cc);
> >
> > fmt = __csi_get_fmt(priv, cfg, sdformat->pad, sdformat->which);
> > +
> > + /* Retain current field setting as default */
> > + if (sdformat->format.field == V4L2_FIELD_ANY)
> > + sdformat->format.field = fmt->field;
>
> sdformat->format.field should never be FIELD_ANY. If it is, then that's a
> subdev bug and I'm pretty sure FIELD_NONE was intended.
This is the subdev. sdformat is passed in from userspace, so we have to
deal with it being set to ANY. I'm trying hard right now not to return
ANY though. The values in sdformat->format are applied to fmt down
below.
> > +
> > + /* Retain current colorspace setting as default */
> > + if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
> > + sdformat->format.colorspace = fmt->colorspace;
>
> No! Subdevs should never return COLORSPACE_DEFAULT. If they do, then fix
> them. If this happens a lot (I'm not sure how reliably subdevs fill this
> in) you could set it to COLORSPACE_RAW. Perhaps with a WARN_ON_ONCE.
Same here, if userspace calls VIDIOC_SUBDEV_S_FMT with DEFAULT
colorspace, I don't want to return that unchanged, but overwrite it with
the currently set value.
> > + if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT)
> > + sdformat->format.xfer_func = fmt->xfer_func;
> > + if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
> > + sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
> > + if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT)
> > + sdformat->format.quantization = fmt->quantization;
>
> Nack. This is meaningless.
It isn't. It may be wrong, but it is not without effect. If the
colorimetry info (in fmt) is currently set to something that is
non-standard, calling VIDIOC_SUBDEV_S_FMT with DEFAULT in xfer_func,
ycbcr_enc or quantization will cause those old values to be retained,
instead of overwriting them to DEFAULT.
> > + } else {
> > + if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT) {
> > + sdformat->format.xfer_func =
> > + V4L2_MAP_XFER_FUNC_DEFAULT(
> > + sdformat->format.colorspace);
> > + }
> > + if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT) {
> > + sdformat->format.ycbcr_enc =
> > + V4L2_MAP_YCBCR_ENC_DEFAULT(
> > + sdformat->format.colorspace);
> > + }
> > + if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT) {
> > + sdformat->format.quantization =
> > + V4L2_MAP_QUANTIZATION_DEFAULT(
> > + cc->cs != IPUV3_COLORSPACE_YUV,
> > + sdformat->format.colorspace,
> > + sdformat->format.ycbcr_enc);
> > + }
>
> This isn't wrong, but it is perfectly fine to keep the DEFAULT here and let
> the application call V4L2_MAP_.
>
> I get the feeling this patch is a workaround for subdev errors. Either that,
> or the commit log doesn't give me enough information to really understand the
> problem that's being addressed here.
It's the latter. I should have written VIDIOC_SUBDEV_S_FMT instead of
just set_fmt in the comment.
> Regards,
>
> Hans
>
> > + }
> > +
> > *fmt = sdformat->format;
Here sdformat is applied to the subdev pad.
regards
Philipp
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v6 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
[PATCH v6 28/39] media: imx: csi: fix crop rectangle changes in set_fmt Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
[PATCH v6 10/39] ARM: dts: imx6-sabreauto: create i2cmux for i2c3 Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
[PATCH v6 37/39] media: imx-csi: add frame size/interval enumeration Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
[PATCH v6 11/39] ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
[PATCH v6 21/39] media: imx: Add CSI subdev driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
[PATCH] [media] imx: csi: retain current field order and colorimetry setting as default Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 16:00 +0200
Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-04-06 16:10 +0200
Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 17:10 +0200
Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-04-06 17:20 +0200
Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 17:30 +0200
Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default Hans Verkuil <hverkuil@xs4all.nl> - 2017-04-06 16:30 +0200
Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-04-06 16:50 +0200
Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 17:00 +0200
Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default Hans Verkuil <hverkuil@xs4all.nl> - 2017-04-06 17:50 +0200
Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 18:10 +0200
Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 17:20 +0200
[PATCH v6 01/39] [media] dt-bindings: Add bindings for video-multiplexer device Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
Re: [PATCH v6 01/39] [media] dt-bindings: Add bindings for video-multiplexer device Rob Herring <robh@kernel.org> - 2017-04-03 16:20 +0200
[PATCH v6 06/39] ARM: dts: imx6qdl: add capture-subsystem device Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
[PATCH v6 25/39] ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
[PATCH v6 05/39] ARM: dts: imx6qdl: Add mipi_ipu1/2 multiplexers, mipi_csi, and their connections Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
[PATCH v6 04/39] ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
[PATCH v6 14/39] add mux and video interface bridge entity functions Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
[PATCH v6 15/39] [media] v4l2-mc: add a function to inherit controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
[PATCH v6 08/39] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
[PATCH v6 13/39] ARM: dts: imx6-sabreauto: add the ADV7180 video decoder Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
[PATCH v6 03/39] [media] dt/bindings: Add bindings for OV5640 Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
Re: [PATCH v6 03/39] [media] dt/bindings: Add bindings for OV5640 Rob Herring <robh@kernel.org> - 2017-04-03 16:20 +0200
[PATCH v6 12/39] ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
Re: [PATCH v6 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-30 13:10 +0200
Re: [PATCH v6 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-30 18:20 +0200
Re: [PATCH v6 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-30 18:30 +0200
[RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Philipp Zabel <p.zabel@pengutronix.de> - 2017-03-30 19:30 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Steve Longerbeam <slongerbeam@gmail.com> - 2017-04-05 00:20 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-05 11:50 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-04-05 01:20 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Steve Longerbeam <slongerbeam@gmail.com> - 2017-04-05 02:50 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-04-05 10:30 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-05 11:40 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Javier Martinez Canillas <javier@dowhile0.org> - 2017-04-05 16:00 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-04-05 17:00 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Devin Heitmueller <dheitmueller@kernellabs.com> - 2017-04-05 17:50 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-04-05 18:20 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Devin Heitmueller <dheitmueller@kernellabs.com> - 2017-04-05 19:10 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-04-05 19:20 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 12:10 +0200
Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1 Steve Longerbeam <slongerbeam@gmail.com> - 2017-04-05 02:50 +0200
Re: [PATCH v6 19/39] media: Add i.MX media core driver Pavel Machek <pavel@ucw.cz> - 2017-04-05 13:40 +0200
Re: [PATCH v6 19/39] media: Add i.MX media core driver Pavel Machek <pavel@ucw.cz> - 2017-04-05 13:40 +0200
Re: [PATCH v6 19/39] media: Add i.MX media core driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 11:50 +0200
Re: [PATCH v6 19/39] media: Add i.MX media core driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-04-07 02:00 +0200
csiph-web