Re: [PATCH 3/3] media: atmel-isc: Add more format configurations
On 08/17/2017 09:16 AM, Wenyou Yang wrote:
> Add the configuration of formats: GREY, ARGB444, ARGB555 and ARGB32.
>
> Signed-off-by: Wenyou Yang
> ---
>
> drivers/media/platform/atmel/atmel-isc.c | 22 --
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/atmel/atmel-isc.c
> b/drivers/media/platform/atmel/atmel-isc.c
> index d91f4e5f8a8d..4e18fe1104c8 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -184,7 +184,7 @@ struct isc_device {
> #define RAW_FMT_IND_START0
> #define RAW_FMT_IND_END 11
> #define ISC_FMT_IND_START12
> -#define ISC_FMT_IND_END 14
> +#define ISC_FMT_IND_END 18
Shouldn't this be 19?
Regards,
Hans
>
> static struct isc_format isc_formats[] = {
> /* 0 */
> @@ -246,12 +246,30 @@ static struct isc_format isc_formats[] = {
> { V4L2_PIX_FMT_YUV422P, 0x0, 16,
> ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_YYCC,
> ISC_DCFG_IMODE_YC422P, ISC_DCTRL_DVIEW_PLANAR, 0x3fb },
> +
> /* 14 */
> + { V4L2_PIX_FMT_GREY, MEDIA_BUS_FMT_Y8_1X8, 8,
> + ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DATY8,
> + ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x1fb },
> +
> + /* 15 */
> + { V4L2_PIX_FMT_ARGB444, MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE, 16,
> + ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_ARGB444,
> + ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x7b },
> + /* 16 */
> + { V4L2_PIX_FMT_ARGB555, MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE, 16,
> + ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_ARGB555,
> + ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x7b },
> + /* 17 */
> { V4L2_PIX_FMT_RGB565, MEDIA_BUS_FMT_RGB565_2X8_LE, 16,
> ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_RGB565,
> ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x7b },
> + /* 18 */
> + { V4L2_PIX_FMT_ARGB32, MEDIA_BUS_FMT_ARGB_1X32, 32,
> + ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_ARGB32,
> + ISC_DCFG_IMODE_PACKED32, ISC_DCTRL_DVIEW_PACKED, 0x7b },
>
> - /* 15 */
> + /* 19 */
> { V4L2_PIX_FMT_YUYV, MEDIA_BUS_FMT_YUYV8_2X8, 16,
> ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT8,
> ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0 },
>
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
On 08/22/2017 05:01 AM, Yong wrote:
> Hi Hans,
>
> On Mon, 21 Aug 2017 16:37:41 +0200
> Hans Verkuil wrote:
>
>> Hi Yong,
>>
>> First two high-level comments before I start the review:
>>
>> 1) Can you provide the v4l2-compliance output? I can't merge this unless I
>>see that it passes the compliance tests. Make sure you compile from the
>> git
>>repo (https://git.linuxtv.org/v4l-utils.git/) so you are using the latest
>>version of the compliance test. Test with just 'v4l2-compliance' and also
>>with the -s option (to test streaming) and with the -f option (to test all
>>the various pixel formats).
>
> OK. I will post with the next version.
>
>>
>> 2) I see you support interlaced formats. Did you actually test that?
>> Interlaced
>>is tricky and you shouldn't add support for it unless you know it works
>> and
>>that it passes the 'v4l2-compliance -s' test.
>
> No. I do not have the condition to test the all formats for now. Maybe this
> can
> be done when ourself's device with V3s is ready in the future months.
Then just drop the interlaced support until you can test it.
>
>>
>> On 07/27/2017 07:01 AM, Yong Deng wrote:
>>> Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
>>> and CSI1 is used for parallel interface. This is not documented in
>>> datasheet but by testing and guess.
>>>
>>> This patch implement a v4l2 framework driver for it.
>>>
>>> Currently, the driver only support the parallel interface. MIPI-CSI2,
>>> ISP's support are not included in this patch.
>>>
>>> Signed-off-by: Yong Deng
>>> ---
>>> drivers/media/platform/Kconfig | 1 +
>>> drivers/media/platform/Makefile | 2 +
>>> drivers/media/platform/sun6i-csi/Kconfig | 9 +
>>> drivers/media/platform/sun6i-csi/Makefile| 3 +
>>> drivers/media/platform/sun6i-csi/sun6i_csi.c | 545 +++
>>> drivers/media/platform/sun6i-csi/sun6i_csi.h | 203 ++
>>> drivers/media/platform/sun6i-csi/sun6i_csi_v3s.c | 827
>>> +++
>>> drivers/media/platform/sun6i-csi/sun6i_csi_v3s.h | 206 ++
>>> drivers/media/platform/sun6i-csi/sun6i_video.c | 663 ++
>>> drivers/media/platform/sun6i-csi/sun6i_video.h | 61 ++
>>> 10 files changed, 2520 insertions(+)
>>> create mode 100644 drivers/media/platform/sun6i-csi/Kconfig
>>> create mode 100644 drivers/media/platform/sun6i-csi/Makefile
>>> create mode 100644 drivers/media/platform/sun6i-csi/sun6i_csi.c
>>> create mode 100644 drivers/media/platform/sun6i-csi/sun6i_csi.h
>>> create mode 100644 drivers/media/platform/sun6i-csi/sun6i_csi_v3s.c
>>> create mode 100644 drivers/media/platform/sun6i-csi/sun6i_csi_v3s.h
>>> create mode 100644 drivers/media/platform/sun6i-csi/sun6i_video.c
>>> create mode 100644 drivers/media/platform/sun6i-csi/sun6i_video.h
>>>
>>
>>
>>
>>> diff --git a/drivers/media/platform/sun6i-csi/sun6i_video.c
>>> b/drivers/media/platform/sun6i-csi/sun6i_video.c
>>> new file mode 100644
>>> index 000..0c5dbd2
>>> --- /dev/null
>>> +++ b/drivers/media/platform/sun6i-csi/sun6i_video.c
>>> @@ -0,0 +1,663 @@
>>> +/*
>>> + * Copyright (c) 2017 Magewell Electronics Co., Ltd. (Nanjing).
>>> + * All rights reserved.
>>> + * Author: Yong Deng
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>> +#include
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +#include "sun6i_csi.h"
>>> +#include "sun6i_video.h"
>>> +
>>> +struct sun6i_csi_buffer {
>>> + struct vb2_v4l2_buffer vb;
>>> + struct list_headlist;
>>> +
>>> + dma_addr_t dma_addr;
>>> +};
>>> +
>>> +static struct sun6i_csi_format *
>>> +find_format_by_fourcc(struct sun6i_video *video, unsigned int fourcc)
>>> +{
>>> + unsigned int num_formats = video->num_formats;
>>> + struct sun6i_csi_format *fmt;
>>> + unsigned int i;
>>> +
>>> + for (i = 0; i < num_formats; i++) {
>>> + fmt = &video->formats[i];
>>> + if (fmt->fourcc == fourcc)
>>> + return fmt;
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>>> +static struct v4l2_subdev *
>>> +sun6i_video_remote_subdev(struct sun6i_video *video, u32 *pad)
>>> +{
>>> + struct media_pad *remote;
>>> +
>>> + remote = media_entity_remote_pad(&video->pad);
>>> +
>>> + if (!remote || !is_media_entity_v4l2_subdev(remote->entity))
>>> + return NULL;
>>> +
>>> + if (pad)
>>> + *pad = remote->index;
>>> +
>>> + retur
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Aug 22 05:00:16 CEST 2017 media-tree git hash:0779b8855c746c90b85bfe6e16d5dfa2a6a46655 media_build git hash: 1d9cbbf82bfb79eb8c47747121903f762d9cc9fb v4l-utils git hash: 15df21b333e243827ac0f89d7f4f307bf0968baf gcc version:i686-linux-gcc (GCC) 7.1.0 sparse version: v0.5.0 smatch version: v0.5.0-3553-g78b2ea6 host hardware: x86_64 host os:4.11.0-164 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: WARNINGS linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-blackfin-bf561: OK linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: WARNINGS linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.1.10-i686: ERRORS linux-3.2.37-i686: ERRORS linux-3.3.8-i686: ERRORS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: ERRORS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: ERRORS linux-3.10.1-i686: ERRORS linux-3.11.1-i686: ERRORS linux-3.12.67-i686: ERRORS linux-3.13.11-i686: ERRORS linux-3.14.9-i686: ERRORS linux-3.15.2-i686: ERRORS linux-3.16.7-i686: ERRORS linux-3.17.8-i686: ERRORS linux-3.18.7-i686: ERRORS linux-3.19-i686: ERRORS linux-4.0.9-i686: ERRORS linux-4.1.33-i686: ERRORS linux-4.2.8-i686: ERRORS linux-4.3.6-i686: ERRORS linux-4.4.22-i686: ERRORS linux-4.5.7-i686: ERRORS linux-4.6.7-i686: ERRORS linux-4.7.5-i686: ERRORS linux-4.8-i686: ERRORS linux-4.9.26-i686: ERRORS linux-4.10.14-i686: OK linux-4.11-i686: OK linux-4.12.1-i686: OK linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.37-x86_64: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: ERRORS linux-3.10.1-x86_64: ERRORS linux-3.11.1-x86_64: ERRORS linux-3.12.67-x86_64: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.9-x86_64: ERRORS linux-3.15.2-x86_64: ERRORS linux-3.16.7-x86_64: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.7-x86_64: ERRORS linux-3.19-x86_64: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.33-x86_64: ERRORS linux-4.2.8-x86_64: ERRORS linux-4.3.6-x86_64: ERRORS linux-4.4.22-x86_64: ERRORS linux-4.5.7-x86_64: ERRORS linux-4.6.7-x86_64: ERRORS linux-4.7.5-x86_64: ERRORS linux-4.8-x86_64: ERRORS linux-4.9.26-x86_64: ERRORS linux-4.10.14-x86_64: WARNINGS linux-4.11-x86_64: WARNINGS linux-4.12.1-x86_64: WARNINGS apps: WARNINGS spec-git: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Hi Hans,
On Mon, 21 Aug 2017 16:37:41 +0200
Hans Verkuil wrote:
> Hi Yong,
>
> First two high-level comments before I start the review:
>
> 1) Can you provide the v4l2-compliance output? I can't merge this unless I
>see that it passes the compliance tests. Make sure you compile from the git
>repo (https://git.linuxtv.org/v4l-utils.git/) so you are using the latest
>version of the compliance test. Test with just 'v4l2-compliance' and also
>with the -s option (to test streaming) and with the -f option (to test all
>the various pixel formats).
OK. I will post with the next version.
>
> 2) I see you support interlaced formats. Did you actually test that?
> Interlaced
>is tricky and you shouldn't add support for it unless you know it works and
>that it passes the 'v4l2-compliance -s' test.
No. I do not have the condition to test the all formats for now. Maybe this can
be done when ourself's device with V3s is ready in the future months.
>
> On 07/27/2017 07:01 AM, Yong Deng wrote:
> > Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
> > and CSI1 is used for parallel interface. This is not documented in
> > datasheet but by testing and guess.
> >
> > This patch implement a v4l2 framework driver for it.
> >
> > Currently, the driver only support the parallel interface. MIPI-CSI2,
> > ISP's support are not included in this patch.
> >
> > Signed-off-by: Yong Deng
> > ---
> > drivers/media/platform/Kconfig | 1 +
> > drivers/media/platform/Makefile | 2 +
> > drivers/media/platform/sun6i-csi/Kconfig | 9 +
> > drivers/media/platform/sun6i-csi/Makefile| 3 +
> > drivers/media/platform/sun6i-csi/sun6i_csi.c | 545 +++
> > drivers/media/platform/sun6i-csi/sun6i_csi.h | 203 ++
> > drivers/media/platform/sun6i-csi/sun6i_csi_v3s.c | 827
> > +++
> > drivers/media/platform/sun6i-csi/sun6i_csi_v3s.h | 206 ++
> > drivers/media/platform/sun6i-csi/sun6i_video.c | 663 ++
> > drivers/media/platform/sun6i-csi/sun6i_video.h | 61 ++
> > 10 files changed, 2520 insertions(+)
> > create mode 100644 drivers/media/platform/sun6i-csi/Kconfig
> > create mode 100644 drivers/media/platform/sun6i-csi/Makefile
> > create mode 100644 drivers/media/platform/sun6i-csi/sun6i_csi.c
> > create mode 100644 drivers/media/platform/sun6i-csi/sun6i_csi.h
> > create mode 100644 drivers/media/platform/sun6i-csi/sun6i_csi_v3s.c
> > create mode 100644 drivers/media/platform/sun6i-csi/sun6i_csi_v3s.h
> > create mode 100644 drivers/media/platform/sun6i-csi/sun6i_video.c
> > create mode 100644 drivers/media/platform/sun6i-csi/sun6i_video.h
> >
>
>
>
> > diff --git a/drivers/media/platform/sun6i-csi/sun6i_video.c
> > b/drivers/media/platform/sun6i-csi/sun6i_video.c
> > new file mode 100644
> > index 000..0c5dbd2
> > --- /dev/null
> > +++ b/drivers/media/platform/sun6i-csi/sun6i_video.c
> > @@ -0,0 +1,663 @@
> > +/*
> > + * Copyright (c) 2017 Magewell Electronics Co., Ltd. (Nanjing).
> > + * All rights reserved.
> > + * Author: Yong Deng
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "sun6i_csi.h"
> > +#include "sun6i_video.h"
> > +
> > +struct sun6i_csi_buffer {
> > + struct vb2_v4l2_buffer vb;
> > + struct list_headlist;
> > +
> > + dma_addr_t dma_addr;
> > +};
> > +
> > +static struct sun6i_csi_format *
> > +find_format_by_fourcc(struct sun6i_video *video, unsigned int fourcc)
> > +{
> > + unsigned int num_formats = video->num_formats;
> > + struct sun6i_csi_format *fmt;
> > + unsigned int i;
> > +
> > + for (i = 0; i < num_formats; i++) {
> > + fmt = &video->formats[i];
> > + if (fmt->fourcc == fourcc)
> > + return fmt;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static struct v4l2_subdev *
> > +sun6i_video_remote_subdev(struct sun6i_video *video, u32 *pad)
> > +{
> > + struct media_pad *remote;
> > +
> > + remote = media_entity_remote_pad(&video->pad);
> > +
> > + if (!remote || !is_media_entity_v4l2_subdev(remote->entity))
> > + return NULL;
> > +
> > + if (pad)
> > + *pad = remote->index;
> > +
> > + return media_entity_to_v4l2_subdev(remote->entity);
> > +}
> > +
> > +static int sun6i_video_queue_setup(struct vb2_queue *vq,
> > +
Re: [PATCH 1/3] dt-bindings: Add bindings for Dongwoon DW9714 voice coil
On Thu, Aug 17, 2017 at 04:42:54PM +0300, Sakari Ailus wrote: > Dongwoon DW9714 is a voice coil lens driver. > > Also add a vendor prefix for Dongwoon for one did not exist previously. > > Signed-off-by: Sakari Ailus > --- > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt | 9 + > Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + > 2 files changed, 10 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt Acked-by: Rob Herring
Re: [PATCH 1/3] media: atmel-isc: Not support RBG format from sensor.
Hi Hans,
On 2017/8/21 22:07, Hans Verkuil wrote:
On 08/17/2017 09:16 AM, Wenyou Yang wrote:
The 12-bit parallel interface supports the Raw Bayer, YCbCr,
Monochrome and JPEG Compressed pixel formats from the external
sensor, not support RBG pixel format.
Signed-off-by: Wenyou Yang
---
drivers/media/platform/atmel/atmel-isc.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/media/platform/atmel/atmel-isc.c
b/drivers/media/platform/atmel/atmel-isc.c
index d4df3d4ccd85..535bb03783fe 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1478,6 +1478,11 @@ static int isc_formats_init(struct isc_device *isc)
while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
NULL, &mbus_code)) {
mbus_code.index++;
+
+ /* Not support the RGB pixel formats from sensor */
+ if ((mbus_code.code & 0xf000) == 0x1000)
+ continue;
Am I missing something? Here you skip any RGB mediabus formats, but in patch 3/3
you add RGB mediabus formats. But this patch prevents those new formats from
being
selected, right?
This patch prevents getting the RGB format from the sensor directly.
The RGB format can be produced by ISC controller by itself.
Regards,
Hans
+
fmt = find_format_by_code(mbus_code.code, &i);
if (!fmt)
continue;
Best Regards,
Wenyou Yang
Re: [PATCH v2 1/2] docs-rst: media: Document s_stream() video op usage for MC enabled devices
Em Tue, 22 Aug 2017 02:44:00 +0200 Niklas Söderlund escreveu: > On 2017-08-21 11:01:00 -0300, Mauro Carvalho Chehab wrote: > > Em Mon, 21 Aug 2017 15:52:17 +0200 > > Hans Verkuil escreveu: > > > > > On 08/21/2017 02:07 PM, Mauro Carvalho Chehab wrote: > > > > Em Mon, 21 Aug 2017 12:14:18 +0200 > > > > Hans Verkuil escreveu: > > > > > > > >> On 08/21/2017 11:08 AM, Mauro Carvalho Chehab wrote: > > > >>> Em Mon, 21 Aug 2017 09:36:49 +0300 > > > >>> Sakari Ailus escreveu: > > > >>> > > > Hi Mauro, > > > > > > Mauro Carvalho Chehab wrote: > > > > Hi Sakari, > > > > > > > > Em Wed, 16 Aug 2017 15:20:17 +0300 > > > > Sakari Ailus escreveu: > > > > > > > >> As we begin to add support for systems with Media controller > > > >> pipelines > > > >> controlled by more than one device driver, it is essential that we > > > >> precisely define the responsibilities of each component in the > > > >> stream > > > >> control and common practices. > > > >> > > > >> Specifically, streaming control is done per sub-device and > > > >> sub-device > > > >> drivers themselves are responsible for streaming setup in upstream > > > >> sub-devices. > > > > > > > > IMO, before this patch, we need something like this: > > > > https://patchwork.linuxtv.org/patch/43325/ > > > > > > Thanks. I'll reply separately to that thread. > > > > > > > > > > >> > > > >> Signed-off-by: Sakari Ailus > > > >> Acked-by: Niklas Söderlund > > > >> --- > > > >> Documentation/media/kapi/v4l2-subdev.rst | 29 > > > >> + > > > >> 1 file changed, 29 insertions(+) > > > >> > > > >> diff --git a/Documentation/media/kapi/v4l2-subdev.rst > > > >> b/Documentation/media/kapi/v4l2-subdev.rst > > > >> index e1f0b72..45088ad 100644 > > > >> --- a/Documentation/media/kapi/v4l2-subdev.rst > > > >> +++ b/Documentation/media/kapi/v4l2-subdev.rst > > > >> @@ -262,6 +262,35 @@ is called. After all subdevices have been > > > >> located the .complete() callback is > > > >> called. When a subdevice is removed from the system the .unbind() > > > >> method is > > > >> called. All three callbacks are optional. > > > >> > > > >> +Streaming control on Media controller enabled devices > > > >> +^ > > > >> + > > > >> +Starting and stopping the stream are somewhat complex operations > > > >> that > > > >> +often require walking the media graph to enable streaming on > > > >> +sub-devices which the pipeline consists of. This involves > > > >> interaction > > > >> +between multiple drivers, sometimes more than two. > > > > > > > > That's still not ok, as it creates a black hole for devnode-based > > > > devices. > > > > > > > > I would change it to something like: > > > > > > > > Streaming control > > > > ^ > > > > > > > > Starting and stopping the stream are somewhat complex > > > > operations that > > > > often require to enable streaming on sub-devices which the > > > > pipeline > > > > consists of. This involves interaction between multiple > > > > drivers, sometimes > > > > more than two. > > > > > > > > The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is > > > > responsible > > > > for starting and stopping the stream on the sub-device it is > > > > called > > > > on. > > > > > > > > Streaming control on devnode-centric devices > > > > > > > > > > > > On **devnode-centric** devices, the main driver is responsible > > > > enable > > > > stream all all sub-devices. On most cases, all the main driver > > > > need > > > > to do is to broadcast s_stream to all connected sub-devices by > > > > calling > > > > :c:func:`v4l2_device_call_all`, e. g.:: > > > > > > > > v4l2_device_call_all(&dev->v4l2_dev, 0, video, > > > > s_stream, 1); > > > > > > Looks good to me. > > > > > > > > > > > Streaming control on mc-centric devices > > > > ~~~ > > > > > > > > On **mc-centric** devices, it usually requires walking the > > > > media graph > > > > to enable streaming only at the sub-devices which the pipeline > > > > consists > > > > of. > > > > > > > > (place here the details for such scenario) > > > > > > This part requires a more detailed description of the problem area. > > > What > > > makes a difference here is that there's a pipeline this pipeline may > > > be > > > cont
Re: [PATCH v2 1/2] docs-rst: media: Document s_stream() video op usage for MC enabled devices
On 2017-08-21 11:01:00 -0300, Mauro Carvalho Chehab wrote: > Em Mon, 21 Aug 2017 15:52:17 +0200 > Hans Verkuil escreveu: > > > On 08/21/2017 02:07 PM, Mauro Carvalho Chehab wrote: > > > Em Mon, 21 Aug 2017 12:14:18 +0200 > > > Hans Verkuil escreveu: > > > > > >> On 08/21/2017 11:08 AM, Mauro Carvalho Chehab wrote: > > >>> Em Mon, 21 Aug 2017 09:36:49 +0300 > > >>> Sakari Ailus escreveu: > > >>> > > Hi Mauro, > > > > Mauro Carvalho Chehab wrote: > > > Hi Sakari, > > > > > > Em Wed, 16 Aug 2017 15:20:17 +0300 > > > Sakari Ailus escreveu: > > > > > >> As we begin to add support for systems with Media controller > > >> pipelines > > >> controlled by more than one device driver, it is essential that we > > >> precisely define the responsibilities of each component in the stream > > >> control and common practices. > > >> > > >> Specifically, streaming control is done per sub-device and sub-device > > >> drivers themselves are responsible for streaming setup in upstream > > >> sub-devices. > > > > > > IMO, before this patch, we need something like this: > > > https://patchwork.linuxtv.org/patch/43325/ > > > > Thanks. I'll reply separately to that thread. > > > > > > > >> > > >> Signed-off-by: Sakari Ailus > > >> Acked-by: Niklas Söderlund > > >> --- > > >> Documentation/media/kapi/v4l2-subdev.rst | 29 > > >> + > > >> 1 file changed, 29 insertions(+) > > >> > > >> diff --git a/Documentation/media/kapi/v4l2-subdev.rst > > >> b/Documentation/media/kapi/v4l2-subdev.rst > > >> index e1f0b72..45088ad 100644 > > >> --- a/Documentation/media/kapi/v4l2-subdev.rst > > >> +++ b/Documentation/media/kapi/v4l2-subdev.rst > > >> @@ -262,6 +262,35 @@ is called. After all subdevices have been > > >> located the .complete() callback is > > >> called. When a subdevice is removed from the system the .unbind() > > >> method is > > >> called. All three callbacks are optional. > > >> > > >> +Streaming control on Media controller enabled devices > > >> +^ > > >> + > > >> +Starting and stopping the stream are somewhat complex operations > > >> that > > >> +often require walking the media graph to enable streaming on > > >> +sub-devices which the pipeline consists of. This involves > > >> interaction > > >> +between multiple drivers, sometimes more than two. > > > > > > That's still not ok, as it creates a black hole for devnode-based > > > devices. > > > > > > I would change it to something like: > > > > > > Streaming control > > > ^ > > > > > > Starting and stopping the stream are somewhat complex > > > operations that > > > often require to enable streaming on sub-devices which the > > > pipeline > > > consists of. This involves interaction between multiple > > > drivers, sometimes > > > more than two. > > > > > > The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is > > > responsible > > > for starting and stopping the stream on the sub-device it is > > > called > > > on. > > > > > > Streaming control on devnode-centric devices > > > > > > > > > On **devnode-centric** devices, the main driver is responsible > > > enable > > > stream all all sub-devices. On most cases, all the main driver > > > need > > > to do is to broadcast s_stream to all connected sub-devices by > > > calling > > > :c:func:`v4l2_device_call_all`, e. g.:: > > > > > > v4l2_device_call_all(&dev->v4l2_dev, 0, video, > > > s_stream, 1); > > > > Looks good to me. > > > > > > > > Streaming control on mc-centric devices > > > ~~~ > > > > > > On **mc-centric** devices, it usually requires walking the > > > media graph > > > to enable streaming only at the sub-devices which the pipeline > > > consists > > > of. > > > > > > (place here the details for such scenario) > > > > This part requires a more detailed description of the problem area. > > What > > makes a difference here is that there's a pipeline this pipeline may > > be > > controlled more than one driver. (More elaborate discussion below.) > > > > > > > >> +The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is > > >> responsible > > >> +for starting and stopping the stream on the sub-device it is called > > >> +on.
[PATCH v2] device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()
Using CONFIG_OF_DYNAMIC=y uncovered an imbalance in the usecount of the
node being passed to of_fwnode_graph_get_port_parent(). Preserve the
usecount by using of_get_parent() instead of of_get_next_parent() which
don't decrement the usecount of the node passed to it.
Fixes: 3b27d00e7b6d7c88 ("device property: Move fwnode graph ops to firmware
specific locations")
Signed-off-by: Niklas Söderlund
Acked-by: Sakari Ailus
---
drivers/of/property.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 067f9fab7b77c794..59afeea9888e3b3d 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -923,7 +923,7 @@ of_fwnode_graph_get_port_parent(struct fwnode_handle
*fwnode)
struct device_node *np;
/* Get the parent of the port */
- np = of_get_next_parent(to_of_node(fwnode));
+ np = of_get_parent(to_of_node(fwnode));
if (!np)
return NULL;
--
2.14.0
[PATCH v4l-utils] configure.ac: drop --disable-libv4l, disable plugin support instead
In commit 2e604dfbcd09b93f0808cedb2a0b324c5569a599 ("configure.ac: add
--disable-libv4l option"), an option --disable-libv4l was added. As
part of this, libv4l is no longer built at all in static linking
configurations, just because libv4l uses dlopen() for plugin support.
However, plugin support is only a side feature of libv4l, and one may
need to use libv4l in static configurations, just without plugin
support.
Therefore, this commit:
- Essentially reverts 2e604dfbcd09b93f0808cedb2a0b324c5569a599, so
that libv4l can be built in static linking configurations again.
- Adjusts the compilation of libv4l2 so that the plugin support is
not compiled in when dlopen() in static linking configuration
(dlopen is not available).
Signed-off-by: Thomas Petazzoni
---
NOTE: this was only build-time tested, not runtime tested.
---
Makefile.am | 11 ++-
configure.ac | 15 +++
lib/libv4l2/Makefile.am | 6 +-
lib/libv4l2/libv4l2-priv.h| 14 ++
utils/Makefile.am | 6 +-
utils/v4l2-compliance/Makefile.am | 4
utils/v4l2-ctl/Makefile.am| 4
7 files changed, 25 insertions(+), 35 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 07c3ef8..e603472 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,17 +1,10 @@
AUTOMAKE_OPTIONS = foreign
ACLOCAL_AMFLAGS = -I m4
-SUBDIRS = v4l-utils-po libdvbv5-po
-
-if WITH_LIBV4L
-SUBDIRS += lib
-endif
+SUBDIRS = v4l-utils-po libdvbv5-po lib
if WITH_V4LUTILS
-SUBDIRS += utils
-if WITH_LIBV4L
-SUBDIRS += contrib
-endif
+SUBDIRS += utils contrib
endif
EXTRA_DIST = android-config.h bootstrap.sh doxygen_libdvbv5.cfg include
COPYING.libv4l \
diff --git a/configure.ac b/configure.ac
index 72c9421..af44663 100644
--- a/configure.ac
+++ b/configure.ac
@@ -375,14 +375,6 @@ AC_ARG_ENABLE(libdvbv5,
esac]
)
-AC_ARG_ENABLE(libv4l,
- AS_HELP_STRING([--disable-libv4l], [disable libv4l compilation]),
- [case "${enableval}" in
- yes | no ) ;;
- *) AC_MSG_ERROR(bad value ${enableval} for --disable-libv4l) ;;
- esac]
-)
-
AC_ARG_ENABLE(dyn-libv4l,
AS_HELP_STRING([--disable-dyn-libv4l], [disable dynamic libv4l support]),
[case "${enableval}" in
@@ -448,7 +440,6 @@ AM_CONDITIONAL([WITH_LIBDVBV5], [test x$enable_libdvbv5
!= xno -a x$have_li
AM_CONDITIONAL([WITH_DVBV5_REMOTE], [test x$enable_libdvbv5 != xno -a
x$have_libudev = xyes -a x$have_pthread = xyes])
AM_CONDITIONAL([WITH_DYN_LIBV4L], [test x$enable_dyn_libv4l != xno])
-AM_CONDITIONAL([WITH_LIBV4L], [test x$enable_libv4l!= xno -a
x$enable_shared != xno])
AM_CONDITIONAL([WITH_V4LUTILS],[test x$enable_v4l_utils != xno -a
x$linux_os = xyes])
AM_CONDITIONAL([WITH_QV4L2], [test x${qt_pkgconfig} = xtrue -a
x$enable_qv4l2 != xno])
AM_CONDITIONAL([WITH_V4L_PLUGINS], [test x$enable_dyn_libv4l != xno -a
x$enable_shared != xno])
@@ -477,11 +468,12 @@ AM_COND_IF([WITH_LIBDVBV5], [USE_LIBDVBV5="yes"],
[USE_LIBDVBV5="no"])
AM_COND_IF([WITH_DVBV5_REMOTE], [USE_DVBV5_REMOTE="yes"
AC_DEFINE([HAVE_DVBV5_REMOTE], [1], [Usage of
DVBv5 remote enabled])],
[USE_DVBV5_REMOTE="no"])
-AM_COND_IF([WITH_LIBV4L], [USE_LIBV4L="yes"], [USE_LIBV4L="no"])
AM_COND_IF([WITH_DYN_LIBV4L], [USE_DYN_LIBV4L="yes"], [USE_DYN_LIBV4L="no"])
AM_COND_IF([WITH_V4LUTILS], [USE_V4LUTILS="yes"], [USE_V4LUTILS="no"])
AM_COND_IF([WITH_QV4L2], [USE_QV4L2="yes"], [USE_QV4L2="no"])
-AM_COND_IF([WITH_V4L_PLUGINS], [USE_V4L_PLUGINS="yes"], [USE_V4L_PLUGINS="no"])
+AM_COND_IF([WITH_V4L_PLUGINS], [USE_V4L_PLUGINS="yes"
+ AC_DEFINE([HAVE_V4L_PLUGINS], [1], [V4L plugin
support enabled])],
+ [USE_V4L_PLUGINS="no"])
AM_COND_IF([WITH_V4L_WRAPPERS], [USE_V4L_WRAPPERS="yes"],
[USE_V4L_WRAPPERS="no"])
AM_COND_IF([WITH_GCONV], [USE_GCONV="yes"], [USE_GCONV="no"])
AM_COND_IF([WITH_V4L2_CTL_LIBV4L], [USE_V4L2_CTL_LIBV4L="yes"],
[USE_V4L2_CTL_LIBV4L="no"])
@@ -513,7 +505,6 @@ compile time options summary
gconv : $USE_GCONV
-libv4l : $USE_LIBV4L
dynamic libv4l : $USE_DYN_LIBV4L
v4l_plugins: $USE_V4L_PLUGINS
v4l_wrappers : $USE_V4L_WRAPPERS
diff --git a/lib/libv4l2/Makefile.am b/lib/libv4l2/Makefile.am
index 811c45c..3a1bb90 100644
--- a/lib/libv4l2/Makefile.am
+++ b/lib/libv4l2/Makefile.am
@@ -15,7 +15,11 @@ else
noinst_LTLIBRARIES = libv4l2.la
endif
-libv4l2_la_SOURCES = libv4l2.c v4l2-plugin.c log.c libv4l2-priv.h
+libv4l2_la_SOURCES = libv4l2.c log.c libv4l2-priv.h
+if WITH_V4L_PLUGINS
+libv4l2_la_SOURCES += v4l2-plugin.c
+endif
+
libv4l2_la_CPPFLAGS = $(CFLAG_VISIBILITY) $(ENFORCE_LIBV4L_STATIC)
libv4l2_la_LDFLAGS = $(LIBV4L2_VERSION) -lpthread $(DLOPEN_LIBS)
$(ENFORCE_LIBV4L_STATIC)
libv4l2_la_LIBADD = ../libv4lconvert/libv4lconv
Re: [PATCH v2.1 2/3] leds: as3645a: Add LED flash class driver
Hi Jacek,
Jacek Anaszewski wrote:
> Hi Sakari,
>
> On 08/21/2017 03:53 PM, Sakari Ailus wrote:
>> Hi Jacek,
>>
>> Jacek Anaszewski wrote:
>>> Hi Sakari,
>>>
>>> Thanks for the update.
>>> I've noticed that you added node labels to the child device nodes
>>> in [0]:
>>>
>>> "as3645a_flash : flash" and "as3645a_indicator : indicator"
>>
>> The phandle references (as3645a_flash and as3645a_indicator) should
>> actually be moved to the patch adding the flash property to the sensor
>> device node. It doesn't do anything here, yet.
>>
>>>
>>> I am still seeing problems with this approach:
>>>
>>> 1) AFAIK these labels are only used for referencing nodes inside dts
>>>files and they don't affect the name property of struct device_node
>>
>> That's right.
>>
>>> 2) Even if you changed the node name from flash to as3645a_flash, you
>>>would get weird LED class device name "as3645a_flash:flash" in case
>>>label property is absent. Do you have any objections against the
>>>approach I proposed in the previous review?:
>>>
>>>
>>> snprintf(names->flash, sizeof(names->flash),
>>> AS_NAME":%s", node->name);
>>
>> In the current patch, the device node of the flash controller is used,
>> postfixed with colon and the name of the LED ("flash" or "indicator") if
>> no label is defined. In other words, with that DT source you'll have
>> "as3645a:flash" and "as3645a:indicator". So if you change the name of
>> the device node of the I²C device, that will be reflected in the label.
>>
>> If a label exists, then the label is used as such.
>>
>> I don't really have objections to what you're proposing as such but my
>> question is: is it useful? With that, the flash and indicator labels
>> will not come from DT if label properties are undefined. They'll always
>> be "as3645a:flash" and "as3645a:indicator", independently of the names
>> of the device nodes.
>>
>
> Ah, indeed, the node->name is put in place of devicename segment and
> the node points to the LED controller node. Neat approach, likely to
> be adopted as a pattern from now on for all new LED class drivers.
>
>
> For the patch going through media tree:
>
> Acked-by: Jacek Anaszewski
Thanks!
I sent a new version of the DT source patch for N9 / N950; I'll proceed
to send a pull request tomorrow / the day after unless there are further
comments.
--
Regards,
Sakari Ailus
sakari.ai...@iki.fi
Re: [PATCH] device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()
Hejssan Niklas,
Niklas Söderlund wrote:
Hi Sakari,
On 2017-08-21 22:03:02 +0300, Sakari Ailus wrote:
Hi Niklas,
Niklas Söderlund wrote:
Hi Sakari,
On 2017-08-21 16:30:17 +0300, Sakari Ailus wrote:
Hi Niklas,
Niklas Söderlund wrote:
Using CONFIG_OF_DYNAMIC=y uncovered an imbalance in the usecount of the
node being passed to of_fwnode_graph_get_port_parent(). Preserve the
usecount just like it is done in of_graph_get_port_parent().
The of_fwnode_graph_get_port_parent() is called by
fwnode_graph_get_port_parent() which obtains the port node through
fwnode_get_parent(). If you take a reference here, calling
fwnode_graph_get_port_parent() will end up incrementing the port node's use
count. In other words, my understanding is that dropping the reference to
the port node isn't a problem but intended behaviour here.
I'm not sure but I don't think the usecount will be incremented, without
this patch I think it's decremented by one instead. Lets look at the
code starting with fwnode_graph_get_port_parent().
struct fwnode_handle *
fwnode_graph_get_port_parent(struct fwnode_handle *endpoint)
{
struct fwnode_handle *port, *parent;
Increment usecount by 1
port = fwnode_get_parent(endpoint);
parent = fwnode_call_ptr_op(port, graph_get_port_parent);
Decrement usecount by 1
fwnode_handle_put(port); << Usecount -1
Here it is; this is the one I missed.
I spotted something else, too. Look at of_graph_get_port_parent(); it
appears to decrement the use count of the node passed to it, too:
struct device_node *of_graph_get_port_parent(struct device_node *node)
{
unsigned int depth;
/* Walk 3 levels up only if there is 'ports' node. */
for (depth = 3; depth && node; depth--) {
node = of_get_next_parent(node);
if (depth == 2 && of_node_cmp(node->name, "ports"))
break;
}
return node;
}
EXPORT_SYMBOL(of_graph_get_port_parent);
I think you'd need to of_node_get(node) first. I think it'd be good to
address this at the same time.
Your tree is old :-)
I did check of_graph_get_port_parent() when looking for how this was
handled elsewhere in the kernel. But I did not realise that the fix was
accepted after 4.13-rc1 so I did not mention that this was just a copy
of that fix in the patch description. For reference see
c0a480d1acf7dc18 ("device property: Fix usecount for
of_graph_get_port_parent()")
Ack, good. I didn't check new developments there, I have to admit.
One could claim the original design principle has truly been adopted in the
fwnode variant of the function. X-)
Yes and I adopted the same fix for the original :-)
On your original patch --- could you replace of_get_next_parent() by
of_get_parent()? In that case it won't drop the reference to the parent,
i.e. does what's required.
I do however think this is a much nicer solution. So I would still be
inclined to send a v2 whit this change instead. Which solution would you
prefer?
of_get_parent() is my preference; you can add to v2:
Acked-by: Sakari Ailus
of_get_next_parent() is intended for cases where you expressly want to
drop the reference AFAIK.
Thanks!
return parent;
}
Here it looks like the counting is correct and balanced. But without
this patch it's in this function 'fwnode_handle_put(port)' which
triggers the error which this patch aims to fix. Lets look at
of_fwnode_graph_get_port_parent() which in my case is what is called by
the fwnode_call_ptr_op().
static struct fwnode_handle *
of_fwnode_graph_get_port_parent(struct fwnode_handle *fwnode)
{
struct device_node *np;
Here in of_get_next_parent() the usecount is decremented by 1 and the
parents usecount is incremented by 1. So for our node node which passed
in from fwnode_graph_get_port_parent() (where it's named 'port') will be
decremented by 1.
/* Get the parent of the port */
np = of_get_next_parent(to_of_node(fwnode));
if (!np)
return NULL;
/* Is this the "ports" node? If not, it's the port parent. */
if (of_node_cmp(np->name, "ports"))
return of_fwnode_handle(np);
return of_fwnode_handle(of_get_next_parent(np));
}
So unless I miss something I do think this patch is needed to restore
balance to the usecount of the node being passed to
of_fwnode_graph_get_port_parent(). Or maybe I have misunderstood
something?
I wonder if I miss something.
I also wonder what I missed :-)
Fixes: 3b27d00e7b6d7c88 ("device property: Move fwnode graph ops to firmware
specific locations")
Signed-off-by: Niklas Söderlund
---
drivers/of/property.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 067f9fab7b77c794..637dcb4833e2af60 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -922,6 +922,12 @@ of_fwnode_graph_get_port_parent(struct fwnode_handle
*fwnode)
{
Re: [PATCH] device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()
Hi Sakari,
On 2017-08-21 22:03:02 +0300, Sakari Ailus wrote:
> Hi Niklas,
>
> Niklas Söderlund wrote:
> > Hi Sakari,
> >
> > On 2017-08-21 16:30:17 +0300, Sakari Ailus wrote:
> > > Hi Niklas,
> > >
> > > Niklas Söderlund wrote:
> > > > Using CONFIG_OF_DYNAMIC=y uncovered an imbalance in the usecount of the
> > > > node being passed to of_fwnode_graph_get_port_parent(). Preserve the
> > > > usecount just like it is done in of_graph_get_port_parent().
> > >
> > > The of_fwnode_graph_get_port_parent() is called by
> > > fwnode_graph_get_port_parent() which obtains the port node through
> > > fwnode_get_parent(). If you take a reference here, calling
> > > fwnode_graph_get_port_parent() will end up incrementing the port node's
> > > use
> > > count. In other words, my understanding is that dropping the reference to
> > > the port node isn't a problem but intended behaviour here.
> >
> > I'm not sure but I don't think the usecount will be incremented, without
> > this patch I think it's decremented by one instead. Lets look at the
> > code starting with fwnode_graph_get_port_parent().
> >
> > struct fwnode_handle *
> > fwnode_graph_get_port_parent(struct fwnode_handle *endpoint)
> > {
> > struct fwnode_handle *port, *parent;
> >
> > Increment usecount by 1
> >
> > port = fwnode_get_parent(endpoint);
> > parent = fwnode_call_ptr_op(port, graph_get_port_parent);
> >
> > Decrement usecount by 1
> >
> > fwnode_handle_put(port); << Usecount -1
>
> Here it is; this is the one I missed.
>
> I spotted something else, too. Look at of_graph_get_port_parent(); it
> appears to decrement the use count of the node passed to it, too:
>
> struct device_node *of_graph_get_port_parent(struct device_node *node)
> {
> unsigned int depth;
>
> /* Walk 3 levels up only if there is 'ports' node. */
> for (depth = 3; depth && node; depth--) {
> node = of_get_next_parent(node);
> if (depth == 2 && of_node_cmp(node->name, "ports"))
> break;
> }
> return node;
> }
> EXPORT_SYMBOL(of_graph_get_port_parent);
>
> I think you'd need to of_node_get(node) first. I think it'd be good to
> address this at the same time.
Your tree is old :-)
I did check of_graph_get_port_parent() when looking for how this was
handled elsewhere in the kernel. But I did not realise that the fix was
accepted after 4.13-rc1 so I did not mention that this was just a copy
of that fix in the patch description. For reference see
c0a480d1acf7dc18 ("device property: Fix usecount for
of_graph_get_port_parent()")
>
> One could claim the original design principle has truly been adopted in the
> fwnode variant of the function. X-)
Yes and I adopted the same fix for the original :-)
>
> On your original patch --- could you replace of_get_next_parent() by
> of_get_parent()? In that case it won't drop the reference to the parent,
> i.e. does what's required.
I do however think this is a much nicer solution. So I would still be
inclined to send a v2 whit this change instead. Which solution would you
prefer?
>
> >
> > return parent;
> > }
> >
> > Here it looks like the counting is correct and balanced. But without
> > this patch it's in this function 'fwnode_handle_put(port)' which
> > triggers the error which this patch aims to fix. Lets look at
> > of_fwnode_graph_get_port_parent() which in my case is what is called by
> > the fwnode_call_ptr_op().
> >
> > static struct fwnode_handle *
> > of_fwnode_graph_get_port_parent(struct fwnode_handle *fwnode)
> > {
> > struct device_node *np;
> >
> > Here in of_get_next_parent() the usecount is decremented by 1 and the
> > parents usecount is incremented by 1. So for our node node which passed
> > in from fwnode_graph_get_port_parent() (where it's named 'port') will be
> > decremented by 1.
> >
> > /* Get the parent of the port */
> > np = of_get_next_parent(to_of_node(fwnode));
> > if (!np)
> > return NULL;
> >
> > /* Is this the "ports" node? If not, it's the port parent. */
> > if (of_node_cmp(np->name, "ports"))
> > return of_fwnode_handle(np);
> >
> > return of_fwnode_handle(of_get_next_parent(np));
> > }
> >
> > So unless I miss something I do think this patch is needed to restore
> > balance to the usecount of the node being passed to
> > of_fwnode_graph_get_port_parent(). Or maybe I have misunderstood
> > something?
> >
> > >
> > > I wonder if I miss something.
> >
> > I also wonder what I missed :-)
> >
> > >
> > > >
> > > > Fixes: 3b27d00e7b6d7c88 ("device property: Move fwnode graph ops to
> > > > firmware specific locations")
> > > > Signed-off-by: Niklas Söderlund
> > > > ---
> > > > drivers/of/property.c | 6 ++
> > > > 1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/drivers/of/property.c b/drivers/of/property.c
> > > > index 067f9fab7b
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Hi Baruch,
On Sun, Jul 30, 2017 at 09:08:01AM +0300, Baruch Siach wrote:
> On Fri, Jul 28, 2017 at 06:02:33PM +0200, Maxime Ripard wrote:
> > Hi,
> >
> > Thanks for the second iteration!
> >
> > On Thu, Jul 27, 2017 at 01:01:35PM +0800, Yong Deng wrote:
> > > Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
> > > and CSI1 is used for parallel interface. This is not documented in
> > > datasheet but by testing and guess.
> > >
> > > This patch implement a v4l2 framework driver for it.
> > >
> > > Currently, the driver only support the parallel interface. MIPI-CSI2,
> > > ISP's support are not included in this patch.
> > >
> > > Signed-off-by: Yong Deng
>
> [...]
>
> > > +#ifdef DEBUG
> > > +static void sun6i_csi_dump_regs(struct sun6i_csi_dev *sdev)
> > > +{
> > > + struct regmap *regmap = sdev->regmap;
> > > + u32 val;
> > > +
> > > + regmap_read(regmap, CSI_EN_REG, &val);
> > > + printk("CSI_EN_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_IF_CFG_REG, &val);
> > > + printk("CSI_IF_CFG_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CAP_REG, &val);
> > > + printk("CSI_CAP_REG=0x%x\n",val);
> > > + regmap_read(regmap, CSI_SYNC_CNT_REG, &val);
> > > + printk("CSI_SYNC_CNT_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_FIFO_THRS_REG, &val);
> > > + printk("CSI_FIFO_THRS_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_PTN_LEN_REG, &val);
> > > + printk("CSI_PTN_LEN_REG=0x%x\n",val);
> > > + regmap_read(regmap, CSI_PTN_ADDR_REG, &val);
> > > + printk("CSI_PTN_ADDR_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_VER_REG, &val);
> > > + printk("CSI_VER_REG=0x%x\n",val);
> > > + regmap_read(regmap, CSI_CH_CFG_REG, &val);
> > > + printk("CSI_CH_CFG_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_SCALE_REG, &val);
> > > + printk("CSI_CH_SCALE_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_F0_BUFA_REG, &val);
> > > + printk("CSI_CH_F0_BUFA_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_F1_BUFA_REG, &val);
> > > + printk("CSI_CH_F1_BUFA_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_F2_BUFA_REG, &val);
> > > + printk("CSI_CH_F2_BUFA_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_STA_REG, &val);
> > > + printk("CSI_CH_STA_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_INT_EN_REG, &val);
> > > + printk("CSI_CH_INT_EN_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_INT_STA_REG, &val);
> > > + printk("CSI_CH_INT_STA_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_FLD1_VSIZE_REG, &val);
> > > + printk("CSI_CH_FLD1_VSIZE_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_HSIZE_REG, &val);
> > > + printk("CSI_CH_HSIZE_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_VSIZE_REG, &val);
> > > + printk("CSI_CH_VSIZE_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_BUF_LEN_REG, &val);
> > > + printk("CSI_CH_BUF_LEN_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_FLIP_SIZE_REG, &val);
> > > + printk("CSI_CH_FLIP_SIZE_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_FRM_CLK_CNT_REG, &val);
> > > + printk("CSI_CH_FRM_CLK_CNT_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_ACC_ITNL_CLK_CNT_REG, &val);
> > > + printk("CSI_CH_ACC_ITNL_CLK_CNT_REG=0x%x\n",val);
> > > + regmap_read(regmap, CSI_CH_FIFO_STAT_REG, &val);
> > > + printk("CSI_CH_FIFO_STAT_REG=0x%x\n", val);
> > > + regmap_read(regmap, CSI_CH_PCLK_STAT_REG, &val);
> > > + printk("CSI_CH_PCLK_STAT_REG=0x%x\n", val);
> > > +}
> > > +#endif
> >
> > You can already dump a regmap through debugfs, that's redundant.
>
> The advantage of in-code registers dump routine is the ability to
> synchronize the snapshot with the driver code execution. This is
> particularly important for the capture statistics registers. I have
> found it useful here.
You also have the option to use the traces to do that, but if that's
useful, this should be added to regmap itself. It can benefit others
too.
> > > +static irqreturn_t sun6i_csi_isr(int irq, void *dev_id)
> > > +{
> > > + struct sun6i_csi_dev *sdev = (struct sun6i_csi_dev *)dev_id;
> > > + struct regmap *regmap = sdev->regmap;
> > > + u32 status;
> > > +
> > > + regmap_read(regmap, CSI_CH_INT_STA_REG, &status);
> > > +
> > > + if ((status & CSI_CH_INT_STA_FIFO0_OF_PD) ||
> > > + (status & CSI_CH_INT_STA_FIFO1_OF_PD) ||
> > > + (status & CSI_CH_INT_STA_FIFO2_OF_PD) ||
> > > + (status & CSI_CH_INT_STA_HB_OF_PD)) {
> > > + regmap_write(regmap, CSI_CH_INT_STA_REG, status);
> > > + regmap_update_bits(regmap, CSI_EN_REG, CSI_EN_CSI_EN, 0);
> > > + regmap_update_bits(regmap, CSI_EN_REG, CSI_EN_CSI_EN,
> > > +CSI_EN_CSI_EN);
> >
> > You need to enable / disable it at every frame? How do you deal with
> > double buffering? (or did you choose to ignore it for now?)
>
> These *_OF_PD status bits indicate an overflow err
Re: [PATCH 06/15] mtd: make device_type const
Le Sat, 19 Aug 2017 13:52:17 +0530,
Bhumika Goyal a écrit :
> Make this const as it is only stored in the type field of a device
> structure, which is const.
> Done using Coccinelle.
>
Applied to l2-mtd/master.
Thanks,
Boris
> Signed-off-by: Bhumika Goyal
> ---
> drivers/mtd/mtdcore.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index f872a99..e7ea842 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -340,7 +340,7 @@ static ssize_t mtd_bbtblocks_show(struct device *dev,
> };
> ATTRIBUTE_GROUPS(mtd);
>
> -static struct device_type mtd_devtype = {
> +static const struct device_type mtd_devtype = {
> .name = "mtd",
> .groups = mtd_groups,
> .release= mtd_release,
[PATCH v2.1 3/3] arm: dts: omap3: N9/N950: Add AS3645A camera flash
From: Sakari Ailus
Add the as3645a flash controller to the DT source.
Signed-off-by: Sakari Ailus
Reviewed-by: Sebastian Reichel
---
since v3:
- Drop reference names from flash and indicator nodes. They were unused.
arch/arm/boot/dts/omap3-n950-n9.dtsi | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi
b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index df3366fa5409..cb47ae79a5f9 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -265,6 +265,20 @@
&i2c2 {
clock-frequency = <40>;
+
+ as3645a@30 {
+ reg = <0x30>;
+ compatible = "ams,as3645a";
+ flash {
+ flash-timeout-us = <15>;
+ flash-max-microamp = <32>;
+ led-max-microamp = <6>;
+ peak-current-limit = <175>;
+ };
+ indicator {
+ led-max-microamp = <1>;
+ };
+ };
};
&i2c3 {
--
2.11.0
Re: [PATCH] keytable: ensure udev rule fires on rc input device
Hi Sean!
On Wed, Aug 16, 2017 at 05:56:25PM +0100, Sean Young wrote:
> I've found a shorter way of doing this.
That's a really clever trick of dealing with the RUN/$id issue,
I like it a lot!
>
> From: Sean Young
> Date: Wed, 16 Aug 2017 17:41:53 +0100
> Subject: [PATCH] keytable: ensure the udev rule fires on creation of the input
> device
>
> The rc device is created before the input device, so if ir-keytable runs
> too early the input device does not exist yet.
>
> Ensure that rule fires on creation of a rc device's input device.
>
> Note that this also prevents udev from starting ir-keytable on an
> transmit only device, which has no input device.
>
> Note that $id in RUN will not work, since that is expanded after all the
> rules are matched, at which point the the parent might have been changed
> by another match in another rule. The argument to $env{key} is expanded
> immediately.
>
> Signed-off-by: Sean Young
Tested-by: Matthias Reichl
so long & thanks for the fix,
Hias
> ---
> utils/keytable/70-infrared.rules | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/utils/keytable/70-infrared.rules
> b/utils/keytable/70-infrared.rules
> index afffd951..41ca2089 100644
> --- a/utils/keytable/70-infrared.rules
> +++ b/utils/keytable/70-infrared.rules
> @@ -1,4 +1,4 @@
> # Automatically load the proper keymaps after the Remote Controller device
> # creation. The keycode tables rules should be at /etc/rc_maps.cfg
>
> -ACTION=="add", SUBSYSTEM=="rc", RUN+="/usr/bin/ir-keytable -a
> /etc/rc_maps.cfg -s $name"
> +ACTION=="add", SUBSYSTEM=="input", SUBSYSTEMS=="rc", KERNEL=="event*",
> ENV{.rc_sysdev}="$id", RUN+="/usr/bin/ir-keytable -a /etc/rc_maps.cfg -s
> $env{.rc_sysdev}"
> --
> 2.13.5
>
Re: [PATCH v2.1 2/3] leds: as3645a: Add LED flash class driver
Hi Sakari,
On 08/21/2017 03:53 PM, Sakari Ailus wrote:
> Hi Jacek,
>
> Jacek Anaszewski wrote:
>> Hi Sakari,
>>
>> Thanks for the update.
>> I've noticed that you added node labels to the child device nodes
>> in [0]:
>>
>> "as3645a_flash : flash" and "as3645a_indicator : indicator"
>
> The phandle references (as3645a_flash and as3645a_indicator) should
> actually be moved to the patch adding the flash property to the sensor
> device node. It doesn't do anything here, yet.
>
>>
>> I am still seeing problems with this approach:
>>
>> 1) AFAIK these labels are only used for referencing nodes inside dts
>>files and they don't affect the name property of struct device_node
>
> That's right.
>
>> 2) Even if you changed the node name from flash to as3645a_flash, you
>>would get weird LED class device name "as3645a_flash:flash" in case
>>label property is absent. Do you have any objections against the
>>approach I proposed in the previous review?:
>>
>>
>> snprintf(names->flash, sizeof(names->flash),
>> AS_NAME":%s", node->name);
>
> In the current patch, the device node of the flash controller is used,
> postfixed with colon and the name of the LED ("flash" or "indicator") if
> no label is defined. In other words, with that DT source you'll have
> "as3645a:flash" and "as3645a:indicator". So if you change the name of
> the device node of the I²C device, that will be reflected in the label.
>
> If a label exists, then the label is used as such.
>
> I don't really have objections to what you're proposing as such but my
> question is: is it useful? With that, the flash and indicator labels
> will not come from DT if label properties are undefined. They'll always
> be "as3645a:flash" and "as3645a:indicator", independently of the names
> of the device nodes.
>
Ah, indeed, the node->name is put in place of devicename segment and
the node points to the LED controller node. Neat approach, likely to
be adopted as a pattern from now on for all new LED class drivers.
For the patch going through media tree:
Acked-by: Jacek Anaszewski
--
Best regards,
Jacek Anaszewski
Re: [PATCH] device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()
Hi Niklas,
Niklas Söderlund wrote:
Hi Sakari,
On 2017-08-21 16:30:17 +0300, Sakari Ailus wrote:
Hi Niklas,
Niklas Söderlund wrote:
Using CONFIG_OF_DYNAMIC=y uncovered an imbalance in the usecount of the
node being passed to of_fwnode_graph_get_port_parent(). Preserve the
usecount just like it is done in of_graph_get_port_parent().
The of_fwnode_graph_get_port_parent() is called by
fwnode_graph_get_port_parent() which obtains the port node through
fwnode_get_parent(). If you take a reference here, calling
fwnode_graph_get_port_parent() will end up incrementing the port node's use
count. In other words, my understanding is that dropping the reference to
the port node isn't a problem but intended behaviour here.
I'm not sure but I don't think the usecount will be incremented, without
this patch I think it's decremented by one instead. Lets look at the
code starting with fwnode_graph_get_port_parent().
struct fwnode_handle *
fwnode_graph_get_port_parent(struct fwnode_handle *endpoint)
{
struct fwnode_handle *port, *parent;
Increment usecount by 1
port = fwnode_get_parent(endpoint);
parent = fwnode_call_ptr_op(port, graph_get_port_parent);
Decrement usecount by 1
fwnode_handle_put(port); << Usecount -1
Here it is; this is the one I missed.
I spotted something else, too. Look at of_graph_get_port_parent(); it
appears to decrement the use count of the node passed to it, too:
struct device_node *of_graph_get_port_parent(struct device_node *node)
{
unsigned int depth;
/* Walk 3 levels up only if there is 'ports' node. */
for (depth = 3; depth && node; depth--) {
node = of_get_next_parent(node);
if (depth == 2 && of_node_cmp(node->name, "ports"))
break;
}
return node;
}
EXPORT_SYMBOL(of_graph_get_port_parent);
I think you'd need to of_node_get(node) first. I think it'd be good to
address this at the same time.
One could claim the original design principle has truly been adopted in
the fwnode variant of the function. X-)
On your original patch --- could you replace of_get_next_parent() by
of_get_parent()? In that case it won't drop the reference to the parent,
i.e. does what's required.
return parent;
}
Here it looks like the counting is correct and balanced. But without
this patch it's in this function 'fwnode_handle_put(port)' which
triggers the error which this patch aims to fix. Lets look at
of_fwnode_graph_get_port_parent() which in my case is what is called by
the fwnode_call_ptr_op().
static struct fwnode_handle *
of_fwnode_graph_get_port_parent(struct fwnode_handle *fwnode)
{
struct device_node *np;
Here in of_get_next_parent() the usecount is decremented by 1 and the
parents usecount is incremented by 1. So for our node node which passed
in from fwnode_graph_get_port_parent() (where it's named 'port') will be
decremented by 1.
/* Get the parent of the port */
np = of_get_next_parent(to_of_node(fwnode));
if (!np)
return NULL;
/* Is this the "ports" node? If not, it's the port parent. */
if (of_node_cmp(np->name, "ports"))
return of_fwnode_handle(np);
return of_fwnode_handle(of_get_next_parent(np));
}
So unless I miss something I do think this patch is needed to restore
balance to the usecount of the node being passed to
of_fwnode_graph_get_port_parent(). Or maybe I have misunderstood
something?
I wonder if I miss something.
I also wonder what I missed :-)
Fixes: 3b27d00e7b6d7c88 ("device property: Move fwnode graph ops to firmware
specific locations")
Signed-off-by: Niklas Söderlund
---
drivers/of/property.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 067f9fab7b77c794..637dcb4833e2af60 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -922,6 +922,12 @@ of_fwnode_graph_get_port_parent(struct fwnode_handle
*fwnode)
{
struct device_node *np;
+ /*
+* Preserve usecount for passed in node as of_get_next_parent()
+* will do of_node_put() on it.
+*/
+ of_node_get(to_of_node(fwnode));
+
/* Get the parent of the port */
np = of_get_next_parent(to_of_node(fwnode));
if (!np)
--
Sakari Ailus
sakari.ai...@linux.intel.com
--
Sakari Ailus
sakari.ai...@linux.intel.com
Re: DRM Format Modifiers in v4l2
On 08/21/2017 06:01 PM, Daniel Vetter wrote: > On Mon, Aug 21, 2017 at 5:52 PM, Brian Starkey wrote: >> Hi all, >> >> I couldn't find this topic talked about elsewhere, but apologies if >> it's a duplicate - I'll be glad to be steered in the direction of a >> thread. >> >> We'd like to support DRM format modifiers in v4l2 in order to share >> the description of different (mostly proprietary) buffer formats >> between e.g. a v4l2 device and a DRM device. >> >> DRM format modifiers are defined in include/uapi/drm/drm_fourcc.h and >> are a vendor-namespaced 64-bit value used to describe various >> vendor-specific buffer layouts. They are combined with a (DRM) FourCC >> code to give a complete description of the data contained in a buffer. >> >> The same modifier definition is used in the Khronos EGL extension >> EGL_EXT_image_dma_buf_import_modifiers, and is supported in the >> Wayland linux-dmabuf protocol. >> >> >> This buffer information could of course be described in the >> vendor-specific part of V4L2_PIX_FMT_*, but this would duplicate the >> information already defined in drm_fourcc.h. Additionally, there >> would be quite a format explosion where a device supports a dozen or >> more formats, all of which can use one or more different >> layouts/compression schemes. >> >> So, I'm wondering if anyone has views on how/whether this could be >> incorporated? >> >> I spoke briefly about this to Laurent at LPC last year, and he >> suggested v4l2_control as one approach. >> >> I also wondered if could be added in v4l2_pix_format_mplane - looks >> like there's 8 bytes left before it exceeds the 200 bytes, or could go >> in the reserved portion of v4l2_plane_pix_format. >> >> Thanks for any thoughts, > > One problem is that the modifers sometimes reference the DRM fourcc > codes. v4l has a different (and incompatible set) of fourcc codes, > whereas all the protocols and specs (you can add DRI3.1 for Xorg to > that list btw) use both drm fourcc and drm modifiers. > > This might or might not make this proposal unworkable, but it's > something I'd at least review carefully. > > Otherwise I think it'd be great if we could have one namespace for all > modifiers, that's pretty much why we have them. Please also note that > for drm_fourcc.h we don't require an in-kernel user for a new modifier > since a bunch of them might need to be allocated just for > userspace-to-userspace buffer sharing (e.g. in EGL/vk). One example > for this would be compressed surfaces with fast-clearing, which is > planned for i915 (but current hw can't scan it out). And we really > want to have one namespace for everything. Who sets these modifiers? Kernel or userspace? Or can it be set by both? I assume any userspace code that sets/reads this is code specific for that hardware? I think Laurent's suggestion of using a 64 bit V4L2 control for this makes the most sense. Especially if you can assume that whoever sets this knows the hardware. I think this only makes sense if you pass buffers from one HW device to another. Because you cannot expect generic video capture code to be able to interpret all the zillion different combinations of modifiers. Regards, Hans
Re: DRM Format Modifiers in v4l2
On Mon, Aug 21, 2017 at 06:01:24PM +0200, Daniel Vetter wrote: On Mon, Aug 21, 2017 at 5:52 PM, Brian Starkey wrote: Hi all, I couldn't find this topic talked about elsewhere, but apologies if it's a duplicate - I'll be glad to be steered in the direction of a thread. We'd like to support DRM format modifiers in v4l2 in order to share the description of different (mostly proprietary) buffer formats between e.g. a v4l2 device and a DRM device. DRM format modifiers are defined in include/uapi/drm/drm_fourcc.h and are a vendor-namespaced 64-bit value used to describe various vendor-specific buffer layouts. They are combined with a (DRM) FourCC code to give a complete description of the data contained in a buffer. The same modifier definition is used in the Khronos EGL extension EGL_EXT_image_dma_buf_import_modifiers, and is supported in the Wayland linux-dmabuf protocol. This buffer information could of course be described in the vendor-specific part of V4L2_PIX_FMT_*, but this would duplicate the information already defined in drm_fourcc.h. Additionally, there would be quite a format explosion where a device supports a dozen or more formats, all of which can use one or more different layouts/compression schemes. So, I'm wondering if anyone has views on how/whether this could be incorporated? I spoke briefly about this to Laurent at LPC last year, and he suggested v4l2_control as one approach. I also wondered if could be added in v4l2_pix_format_mplane - looks like there's 8 bytes left before it exceeds the 200 bytes, or could go in the reserved portion of v4l2_plane_pix_format. Thanks for any thoughts, One problem is that the modifers sometimes reference the DRM fourcc codes. v4l has a different (and incompatible set) of fourcc codes, whereas all the protocols and specs (you can add DRI3.1 for Xorg to that list btw) use both drm fourcc and drm modifiers. This problem already exists (ignoring modifiers) in the case of any v4l2 <-> DRM buffer sharing (direct video scanout, for instance). I was hoping it would be possible to draw enough equivalency between the different definitions to manage a useful subset through a 1:1 lookup table. If that's not possible then this gets a whole lot more tricky. Are you already aware of incompatibilities which would prevent it? -Brian This might or might not make this proposal unworkable, but it's something I'd at least review carefully. Otherwise I think it'd be great if we could have one namespace for all modifiers, that's pretty much why we have them. Please also note that for drm_fourcc.h we don't require an in-kernel user for a new modifier since a bunch of them might need to be allocated just for userspace-to-userspace buffer sharing (e.g. in EGL/vk). One example for this would be compressed surfaces with fast-clearing, which is planned for i915 (but current hw can't scan it out). And we really want to have one namespace for everything. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCHv2 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
On Sat, Aug 19, 2017 at 02:05:16PM +0200, Hans Verkuil wrote: > On 08/12/2017 11:01 AM, Hans Verkuil wrote: > > From: Hans Verkuil > > > > This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX > > feature. This patch series is based on 4.13-rc4 which has all the needed cec > > and drm 4.13 patches merged. > > > > This patch series has been tested with my NUC7i5BNK and a Samsung USB-C to > > HDMI adapter. > > > > Please note this comment at the start of drm_dp_cec.c: > > > > -- > > Unfortunately it turns out that we have a chicken-and-egg situation > > here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI adapters > > have a converter chip that supports CEC-Tunneling-over-AUX (usually the > > Parade PS176), but they do not wire up the CEC pin, thus making CEC > > useless. > > > > Sadly there is no way for this driver to know this. What happens is > > that a /dev/cecX device is created that is isolated and unable to see > > any of the other CEC devices. Quite literally the CEC wire is cut > > (or in this case, never connected in the first place). > > > > I suspect that the reason so few adapters support this is that this > > tunneling protocol was never supported by any OS. So there was no > > easy way of testing it, and no incentive to correctly wire up the > > CEC pin. > > > > Hopefully by creating this driver it will be easier for vendors to > > finally fix their adapters and test the CEC functionality. > > > > I keep a list of known working adapters here: > > > > https://hverkuil.home.xs4all.nl/cec-status.txt > > > > Please mail me (hverk...@xs4all.nl) if you find an adapter that works > > and is not yet listed there. > > -- > > > > I really hope that this work will provide an incentive for vendors to > > finally connect the CEC pin. It's a shame that there are so few adapters > > that work (I found only two USB-C to HDMI adapters that work, and no > > (mini-)DP to HDMI adapters at all). > > > > Note that a colleague who actually knows his way around a soldering iron > > modified an UpTab DisplayPort-to-HDMI adapter for me, hooking up the CEC > > pin. And after that change it worked. I also received confirmation that > > this really is a chicken-and-egg situation: it is because there is no CEC > > support for this feature in any OS that they do not hook up the CEC pin. > > > > So hopefully if this gets merged there will be an incentive for vendors > > to make adapters where this actually works. It is a very nice feature > > for HTPC boxes. > > > > Changes since v1: > > > > - Incorporated Sean's review comments in patch 1/3. > > Ping? > > Who is supposed to merge this? Is there anything I should do? I'd love to > get this in for 4.14... 1) you have commit rights, so only really need to find a reviewer. Not exactly sure who'd be a good reviewer, maybe Imre or Ville? 2) 4.14 is done, this will go into 4.15. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: DRM Format Modifiers in v4l2
On Mon, Aug 21, 2017 at 5:52 PM, Brian Starkey wrote: > Hi all, > > I couldn't find this topic talked about elsewhere, but apologies if > it's a duplicate - I'll be glad to be steered in the direction of a > thread. > > We'd like to support DRM format modifiers in v4l2 in order to share > the description of different (mostly proprietary) buffer formats > between e.g. a v4l2 device and a DRM device. > > DRM format modifiers are defined in include/uapi/drm/drm_fourcc.h and > are a vendor-namespaced 64-bit value used to describe various > vendor-specific buffer layouts. They are combined with a (DRM) FourCC > code to give a complete description of the data contained in a buffer. > > The same modifier definition is used in the Khronos EGL extension > EGL_EXT_image_dma_buf_import_modifiers, and is supported in the > Wayland linux-dmabuf protocol. > > > This buffer information could of course be described in the > vendor-specific part of V4L2_PIX_FMT_*, but this would duplicate the > information already defined in drm_fourcc.h. Additionally, there > would be quite a format explosion where a device supports a dozen or > more formats, all of which can use one or more different > layouts/compression schemes. > > So, I'm wondering if anyone has views on how/whether this could be > incorporated? > > I spoke briefly about this to Laurent at LPC last year, and he > suggested v4l2_control as one approach. > > I also wondered if could be added in v4l2_pix_format_mplane - looks > like there's 8 bytes left before it exceeds the 200 bytes, or could go > in the reserved portion of v4l2_plane_pix_format. > > Thanks for any thoughts, One problem is that the modifers sometimes reference the DRM fourcc codes. v4l has a different (and incompatible set) of fourcc codes, whereas all the protocols and specs (you can add DRI3.1 for Xorg to that list btw) use both drm fourcc and drm modifiers. This might or might not make this proposal unworkable, but it's something I'd at least review carefully. Otherwise I think it'd be great if we could have one namespace for all modifiers, that's pretty much why we have them. Please also note that for drm_fourcc.h we don't require an in-kernel user for a new modifier since a bunch of them might need to be allocated just for userspace-to-userspace buffer sharing (e.g. in EGL/vk). One example for this would be compressed surfaces with fast-clearing, which is planned for i915 (but current hw can't scan it out). And we really want to have one namespace for everything. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
DRM Format Modifiers in v4l2
Hi all, I couldn't find this topic talked about elsewhere, but apologies if it's a duplicate - I'll be glad to be steered in the direction of a thread. We'd like to support DRM format modifiers in v4l2 in order to share the description of different (mostly proprietary) buffer formats between e.g. a v4l2 device and a DRM device. DRM format modifiers are defined in include/uapi/drm/drm_fourcc.h and are a vendor-namespaced 64-bit value used to describe various vendor-specific buffer layouts. They are combined with a (DRM) FourCC code to give a complete description of the data contained in a buffer. The same modifier definition is used in the Khronos EGL extension EGL_EXT_image_dma_buf_import_modifiers, and is supported in the Wayland linux-dmabuf protocol. This buffer information could of course be described in the vendor-specific part of V4L2_PIX_FMT_*, but this would duplicate the information already defined in drm_fourcc.h. Additionally, there would be quite a format explosion where a device supports a dozen or more formats, all of which can use one or more different layouts/compression schemes. So, I'm wondering if anyone has views on how/whether this could be incorporated? I spoke briefly about this to Laurent at LPC last year, and he suggested v4l2_control as one approach. I also wondered if could be added in v4l2_pix_format_mplane - looks like there's 8 bytes left before it exceeds the 200 bytes, or could go in the reserved portion of v4l2_plane_pix_format. Thanks for any thoughts, -Brian
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Hi Yong,
First two high-level comments before I start the review:
1) Can you provide the v4l2-compliance output? I can't merge this unless I
see that it passes the compliance tests. Make sure you compile from the git
repo (https://git.linuxtv.org/v4l-utils.git/) so you are using the latest
version of the compliance test. Test with just 'v4l2-compliance' and also
with the -s option (to test streaming) and with the -f option (to test all
the various pixel formats).
2) I see you support interlaced formats. Did you actually test that? Interlaced
is tricky and you shouldn't add support for it unless you know it works and
that it passes the 'v4l2-compliance -s' test.
On 07/27/2017 07:01 AM, Yong Deng wrote:
> Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
> and CSI1 is used for parallel interface. This is not documented in
> datasheet but by testing and guess.
>
> This patch implement a v4l2 framework driver for it.
>
> Currently, the driver only support the parallel interface. MIPI-CSI2,
> ISP's support are not included in this patch.
>
> Signed-off-by: Yong Deng
> ---
> drivers/media/platform/Kconfig | 1 +
> drivers/media/platform/Makefile | 2 +
> drivers/media/platform/sun6i-csi/Kconfig | 9 +
> drivers/media/platform/sun6i-csi/Makefile| 3 +
> drivers/media/platform/sun6i-csi/sun6i_csi.c | 545 +++
> drivers/media/platform/sun6i-csi/sun6i_csi.h | 203 ++
> drivers/media/platform/sun6i-csi/sun6i_csi_v3s.c | 827
> +++
> drivers/media/platform/sun6i-csi/sun6i_csi_v3s.h | 206 ++
> drivers/media/platform/sun6i-csi/sun6i_video.c | 663 ++
> drivers/media/platform/sun6i-csi/sun6i_video.h | 61 ++
> 10 files changed, 2520 insertions(+)
> create mode 100644 drivers/media/platform/sun6i-csi/Kconfig
> create mode 100644 drivers/media/platform/sun6i-csi/Makefile
> create mode 100644 drivers/media/platform/sun6i-csi/sun6i_csi.c
> create mode 100644 drivers/media/platform/sun6i-csi/sun6i_csi.h
> create mode 100644 drivers/media/platform/sun6i-csi/sun6i_csi_v3s.c
> create mode 100644 drivers/media/platform/sun6i-csi/sun6i_csi_v3s.h
> create mode 100644 drivers/media/platform/sun6i-csi/sun6i_video.c
> create mode 100644 drivers/media/platform/sun6i-csi/sun6i_video.h
>
> diff --git a/drivers/media/platform/sun6i-csi/sun6i_video.c
> b/drivers/media/platform/sun6i-csi/sun6i_video.c
> new file mode 100644
> index 000..0c5dbd2
> --- /dev/null
> +++ b/drivers/media/platform/sun6i-csi/sun6i_video.c
> @@ -0,0 +1,663 @@
> +/*
> + * Copyright (c) 2017 Magewell Electronics Co., Ltd. (Nanjing).
> + * All rights reserved.
> + * Author: Yong Deng
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "sun6i_csi.h"
> +#include "sun6i_video.h"
> +
> +struct sun6i_csi_buffer {
> + struct vb2_v4l2_buffer vb;
> + struct list_headlist;
> +
> + dma_addr_t dma_addr;
> +};
> +
> +static struct sun6i_csi_format *
> +find_format_by_fourcc(struct sun6i_video *video, unsigned int fourcc)
> +{
> + unsigned int num_formats = video->num_formats;
> + struct sun6i_csi_format *fmt;
> + unsigned int i;
> +
> + for (i = 0; i < num_formats; i++) {
> + fmt = &video->formats[i];
> + if (fmt->fourcc == fourcc)
> + return fmt;
> + }
> +
> + return NULL;
> +}
> +
> +static struct v4l2_subdev *
> +sun6i_video_remote_subdev(struct sun6i_video *video, u32 *pad)
> +{
> + struct media_pad *remote;
> +
> + remote = media_entity_remote_pad(&video->pad);
> +
> + if (!remote || !is_media_entity_v4l2_subdev(remote->entity))
> + return NULL;
> +
> + if (pad)
> + *pad = remote->index;
> +
> + return media_entity_to_v4l2_subdev(remote->entity);
> +}
> +
> +static int sun6i_video_queue_setup(struct vb2_queue *vq,
> + unsigned int *nbuffers, unsigned int *nplanes,
> + unsigned int sizes[],
> + struct device *alloc_devs[])
> +{
> + struct sun6i_video *video = vb2_get_drv_priv(vq);
> + unsigned int size = video->fmt.fmt.pix.sizeimage;
> +
> + if (*nplanes)
> + return sizes[0] < size ? -EINVAL : 0;
> +
> + *nplanes = 1;
> + sizes[0] = size;
> +
> + return 0;
> +}
> +
> +static int su
Re: [PATCH] device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()
Hi Niklas,
On Mon, Aug 21, 2017 at 2:51 PM, Niklas Söderlund
wrote:
> Using CONFIG_OF_DYNAMIC=y uncovered an imbalance in the usecount of the
> node being passed to of_fwnode_graph_get_port_parent(). Preserve the
> usecount just like it is done in of_graph_get_port_parent().
>
> Fixes: 3b27d00e7b6d7c88 ("device property: Move fwnode graph ops to firmware
> specific locations")
> Signed-off-by: Niklas Söderlund
> ---
> drivers/of/property.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 067f9fab7b77c794..637dcb4833e2af60 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -922,6 +922,12 @@ of_fwnode_graph_get_port_parent(struct fwnode_handle
> *fwnode)
> {
> struct device_node *np;
>
> + /*
> +* Preserve usecount for passed in node as of_get_next_parent()
> +* will do of_node_put() on it.
> +*/
> + of_node_get(to_of_node(fwnode));
> +
> /* Get the parent of the port */
> np = of_get_next_parent(to_of_node(fwnode));
> if (!np)
FWIW, I'd use "np" to store the intermediate value:
struct device_node *np = to_of_node(fwnode);
/*
* Preserve usecount for passed in node as of_get_next_parent()
* will do of_node_put() on it.
*/
of_node_get(np);
/* Get the parent of the port */
np = of_get_next_parent(np);
Alternatively, perhaps to_of_node() should increment the refcount and
call of_node_get()? Oh, there's (static) of_fwnode_get(), too.
Is drivers/iommu/iommu.c:iommu_fwspec_init() really the only place outside
drivers/of/property.c that calls of_node_get() on a node obtained by
to_of_node()?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v7] rockchip/rga: v4l2 m2m support
Hi Jacob, On 08/03/2017 07:23 AM, Jacob Chen wrote: > Rockchip RGA is a separate 2D raster graphic acceleration unit. It > accelerates 2D graphics operations, such as point/line drawing, image > scaling, rotation, BitBLT, alpha blending and image blur/sharpness > > The drvier is mostly based on s5p-g2d v4l2 m2m driver > And supports various operations from the rendering pipeline. > - copy > - fast solid color fill > - rotation > - flip > - alpha blending > > The code in rga-hw.c is used to configure regs according to operations > The code in rga-buf.c is used to create private mmu table for RGA. > > changes in V7: > - fix some warning reported by "checkpatch --strict" > > Signed-off-by: Jacob Chen Can you post the v4l2-compliance output? I gather that there is at least one colorspace-related error that appears to be a v4l2-compliance bug, so I'd like to see the exact error it gives. I'll see if I can fix it so this driver has a clean compliance output. I apologize that this driver probably won't make 4.14. Too much to review... I hope to do the v7 review within a week. Regards, Hans
Re: [PATCH 1/7 v3] media: vb2: add bidirectional flag in vb2_queue
Hi Marek, On 08/21/2017 03:21 PM, Marek Szyprowski wrote: > Hi Stanimir, > > On 2017-08-21 13:34, Stanimir Varbanov wrote: >> This change is intended to give to the v4l2 drivers a choice to >> change the default behavior of the v4l2-core DMA mapping direction >> from DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or >> OUTPUT) to DMA_BIDIRECTIONAL during queue_init time. >> >> Initially the issue with DMA mapping direction has been found in >> Venus encoder driver where the hardware (firmware side) adds few >> lines padding on bottom of the image buffer, and the consequence >> is triggering of IOMMU protection faults. >> >> This will help supporting venus encoder (and probably other drivers >> in the future) which wants to map output type of buffers as >> read/write. >> >> Signed-off-by: Stanimir Varbanov > > Thanks for the patch. > > Acked-by: Marek Szyprowski Thanks! > > While touching this, I would love to unify set_page_dirty_lock() > related code in videobuf2-dc, videobuf2-sg and videobuf2-vmalloc. > > IMHO the pattern used in videobuf2-vmalloc should be copied to > videobuf2-sg (currently checks for dma_dir for every single page) > and videobuf2-dc (currently it lacks any checks and calls > set_page_dirty_lock() unconditionally). If you have a little bit > of spare time, please prepare a separate patch for the above > mentioned fix. Sure, I'll unify set_page_dirty_lock invocations in separate patch. -- regards, Stan
Re: [PATCH 1/3] media: atmel-isc: Not support RBG format from sensor.
On 08/17/2017 09:16 AM, Wenyou Yang wrote:
> The 12-bit parallel interface supports the Raw Bayer, YCbCr,
> Monochrome and JPEG Compressed pixel formats from the external
> sensor, not support RBG pixel format.
>
> Signed-off-by: Wenyou Yang
> ---
>
> drivers/media/platform/atmel/atmel-isc.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/media/platform/atmel/atmel-isc.c
> b/drivers/media/platform/atmel/atmel-isc.c
> index d4df3d4ccd85..535bb03783fe 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -1478,6 +1478,11 @@ static int isc_formats_init(struct isc_device *isc)
> while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
> NULL, &mbus_code)) {
> mbus_code.index++;
> +
> + /* Not support the RGB pixel formats from sensor */
> + if ((mbus_code.code & 0xf000) == 0x1000)
> + continue;
Am I missing something? Here you skip any RGB mediabus formats, but in patch 3/3
you add RGB mediabus formats. But this patch prevents those new formats from
being
selected, right?
Regards,
Hans
> +
> fmt = find_format_by_code(mbus_code.code, &i);
> if (!fmt)
> continue;
>
Re: [PATCH] device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()
Hi Sakari,
On 2017-08-21 16:30:17 +0300, Sakari Ailus wrote:
> Hi Niklas,
>
> Niklas Söderlund wrote:
> > Using CONFIG_OF_DYNAMIC=y uncovered an imbalance in the usecount of the
> > node being passed to of_fwnode_graph_get_port_parent(). Preserve the
> > usecount just like it is done in of_graph_get_port_parent().
>
> The of_fwnode_graph_get_port_parent() is called by
> fwnode_graph_get_port_parent() which obtains the port node through
> fwnode_get_parent(). If you take a reference here, calling
> fwnode_graph_get_port_parent() will end up incrementing the port node's use
> count. In other words, my understanding is that dropping the reference to
> the port node isn't a problem but intended behaviour here.
I'm not sure but I don't think the usecount will be incremented, without
this patch I think it's decremented by one instead. Lets look at the
code starting with fwnode_graph_get_port_parent().
struct fwnode_handle *
fwnode_graph_get_port_parent(struct fwnode_handle *endpoint)
{
struct fwnode_handle *port, *parent;
Increment usecount by 1
port = fwnode_get_parent(endpoint);
parent = fwnode_call_ptr_op(port, graph_get_port_parent);
Decrement usecount by 1
fwnode_handle_put(port); << Usecount -1
return parent;
}
Here it looks like the counting is correct and balanced. But without
this patch it's in this function 'fwnode_handle_put(port)' which
triggers the error which this patch aims to fix. Lets look at
of_fwnode_graph_get_port_parent() which in my case is what is called by
the fwnode_call_ptr_op().
static struct fwnode_handle *
of_fwnode_graph_get_port_parent(struct fwnode_handle *fwnode)
{
struct device_node *np;
Here in of_get_next_parent() the usecount is decremented by 1 and the
parents usecount is incremented by 1. So for our node node which passed
in from fwnode_graph_get_port_parent() (where it's named 'port') will be
decremented by 1.
/* Get the parent of the port */
np = of_get_next_parent(to_of_node(fwnode));
if (!np)
return NULL;
/* Is this the "ports" node? If not, it's the port parent. */
if (of_node_cmp(np->name, "ports"))
return of_fwnode_handle(np);
return of_fwnode_handle(of_get_next_parent(np));
}
So unless I miss something I do think this patch is needed to restore
balance to the usecount of the node being passed to
of_fwnode_graph_get_port_parent(). Or maybe I have misunderstood
something?
>
> I wonder if I miss something.
I also wonder what I missed :-)
>
> >
> > Fixes: 3b27d00e7b6d7c88 ("device property: Move fwnode graph ops to
> > firmware specific locations")
> > Signed-off-by: Niklas Söderlund
> > ---
> > drivers/of/property.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/of/property.c b/drivers/of/property.c
> > index 067f9fab7b77c794..637dcb4833e2af60 100644
> > --- a/drivers/of/property.c
> > +++ b/drivers/of/property.c
> > @@ -922,6 +922,12 @@ of_fwnode_graph_get_port_parent(struct fwnode_handle
> > *fwnode)
> > {
> > struct device_node *np;
> >
> > + /*
> > +* Preserve usecount for passed in node as of_get_next_parent()
> > +* will do of_node_put() on it.
> > +*/
> > + of_node_get(to_of_node(fwnode));
> > +
> > /* Get the parent of the port */
> > np = of_get_next_parent(to_of_node(fwnode));
> > if (!np)
> >
>
>
> --
> Sakari Ailus
> sakari.ai...@linux.intel.com
--
Regards,
Niklas Söderlund
Re: [PATCH v2 1/2] docs-rst: media: Document s_stream() video op usage for MC enabled devices
Em Mon, 21 Aug 2017 15:52:17 +0200 Hans Verkuil escreveu: > On 08/21/2017 02:07 PM, Mauro Carvalho Chehab wrote: > > Em Mon, 21 Aug 2017 12:14:18 +0200 > > Hans Verkuil escreveu: > > > >> On 08/21/2017 11:08 AM, Mauro Carvalho Chehab wrote: > >>> Em Mon, 21 Aug 2017 09:36:49 +0300 > >>> Sakari Ailus escreveu: > >>> > Hi Mauro, > > Mauro Carvalho Chehab wrote: > > Hi Sakari, > > > > Em Wed, 16 Aug 2017 15:20:17 +0300 > > Sakari Ailus escreveu: > > > >> As we begin to add support for systems with Media controller pipelines > >> controlled by more than one device driver, it is essential that we > >> precisely define the responsibilities of each component in the stream > >> control and common practices. > >> > >> Specifically, streaming control is done per sub-device and sub-device > >> drivers themselves are responsible for streaming setup in upstream > >> sub-devices. > > > > IMO, before this patch, we need something like this: > > https://patchwork.linuxtv.org/patch/43325/ > > Thanks. I'll reply separately to that thread. > > > > >> > >> Signed-off-by: Sakari Ailus > >> Acked-by: Niklas Söderlund > >> --- > >> Documentation/media/kapi/v4l2-subdev.rst | 29 > >> + > >> 1 file changed, 29 insertions(+) > >> > >> diff --git a/Documentation/media/kapi/v4l2-subdev.rst > >> b/Documentation/media/kapi/v4l2-subdev.rst > >> index e1f0b72..45088ad 100644 > >> --- a/Documentation/media/kapi/v4l2-subdev.rst > >> +++ b/Documentation/media/kapi/v4l2-subdev.rst > >> @@ -262,6 +262,35 @@ is called. After all subdevices have been located > >> the .complete() callback is > >> called. When a subdevice is removed from the system the .unbind() > >> method is > >> called. All three callbacks are optional. > >> > >> +Streaming control on Media controller enabled devices > >> +^ > >> + > >> +Starting and stopping the stream are somewhat complex operations that > >> +often require walking the media graph to enable streaming on > >> +sub-devices which the pipeline consists of. This involves interaction > >> +between multiple drivers, sometimes more than two. > > > > That's still not ok, as it creates a black hole for devnode-based > > devices. > > > > I would change it to something like: > > > > Streaming control > > ^ > > > > Starting and stopping the stream are somewhat complex > > operations that > > often require to enable streaming on sub-devices which the > > pipeline > > consists of. This involves interaction between multiple > > drivers, sometimes > > more than two. > > > > The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is > > responsible > > for starting and stopping the stream on the sub-device it is > > called > > on. > > > > Streaming control on devnode-centric devices > > > > > > On **devnode-centric** devices, the main driver is responsible > > enable > > stream all all sub-devices. On most cases, all the main driver > > need > > to do is to broadcast s_stream to all connected sub-devices by > > calling > > :c:func:`v4l2_device_call_all`, e. g.:: > > > > v4l2_device_call_all(&dev->v4l2_dev, 0, video, > > s_stream, 1); > > Looks good to me. > > > > > Streaming control on mc-centric devices > > ~~~ > > > > On **mc-centric** devices, it usually requires walking the > > media graph > > to enable streaming only at the sub-devices which the pipeline > > consists > > of. > > > > (place here the details for such scenario) > > This part requires a more detailed description of the problem area. What > makes a difference here is that there's a pipeline this pipeline may be > controlled more than one driver. (More elaborate discussion below.) > > > > >> +The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is > >> responsible > >> +for starting and stopping the stream on the sub-device it is called > >> +on. A device driver is only responsible for calling the > >> ``.s_stream()`` ops > >> +of the adjacent sub-devices that are connected to its sink pads > >> +through an enabled link. A driver may not call ``.s_stream()`` op > >> +of any other sub-device further up in the pipeline, for instance. >
Re: [PATCH v2.1 2/3] leds: as3645a: Add LED flash class driver
Hi Jacek,
Jacek Anaszewski wrote:
> Hi Sakari,
>
> Thanks for the update.
> I've noticed that you added node labels to the child device nodes
> in [0]:
>
> "as3645a_flash : flash" and "as3645a_indicator : indicator"
The phandle references (as3645a_flash and as3645a_indicator) should
actually be moved to the patch adding the flash property to the sensor
device node. It doesn't do anything here, yet.
>
> I am still seeing problems with this approach:
>
> 1) AFAIK these labels are only used for referencing nodes inside dts
>files and they don't affect the name property of struct device_node
That's right.
> 2) Even if you changed the node name from flash to as3645a_flash, you
>would get weird LED class device name "as3645a_flash:flash" in case
>label property is absent. Do you have any objections against the
>approach I proposed in the previous review?:
>
>
> snprintf(names->flash, sizeof(names->flash),
>AS_NAME":%s", node->name);
In the current patch, the device node of the flash controller is used,
postfixed with colon and the name of the LED ("flash" or "indicator") if
no label is defined. In other words, with that DT source you'll have
"as3645a:flash" and "as3645a:indicator". So if you change the name of
the device node of the I²C device, that will be reflected in the label.
If a label exists, then the label is used as such.
I don't really have objections to what you're proposing as such but my
question is: is it useful? With that, the flash and indicator labels
will not come from DT if label properties are undefined. They'll always
be "as3645a:flash" and "as3645a:indicator", independently of the names
of the device nodes.
--
Kind regards,
Sakari Ailus
sakari.ai...@iki.fi
Re: [PATCH v2 1/2] docs-rst: media: Document s_stream() video op usage for MC enabled devices
On 08/21/2017 02:07 PM, Mauro Carvalho Chehab wrote: > Em Mon, 21 Aug 2017 12:14:18 +0200 > Hans Verkuil escreveu: > >> On 08/21/2017 11:08 AM, Mauro Carvalho Chehab wrote: >>> Em Mon, 21 Aug 2017 09:36:49 +0300 >>> Sakari Ailus escreveu: >>> Hi Mauro, Mauro Carvalho Chehab wrote: > Hi Sakari, > > Em Wed, 16 Aug 2017 15:20:17 +0300 > Sakari Ailus escreveu: > >> As we begin to add support for systems with Media controller pipelines >> controlled by more than one device driver, it is essential that we >> precisely define the responsibilities of each component in the stream >> control and common practices. >> >> Specifically, streaming control is done per sub-device and sub-device >> drivers themselves are responsible for streaming setup in upstream >> sub-devices. > > IMO, before this patch, we need something like this: > https://patchwork.linuxtv.org/patch/43325/ Thanks. I'll reply separately to that thread. > >> >> Signed-off-by: Sakari Ailus >> Acked-by: Niklas Söderlund >> --- >> Documentation/media/kapi/v4l2-subdev.rst | 29 >> + >> 1 file changed, 29 insertions(+) >> >> diff --git a/Documentation/media/kapi/v4l2-subdev.rst >> b/Documentation/media/kapi/v4l2-subdev.rst >> index e1f0b72..45088ad 100644 >> --- a/Documentation/media/kapi/v4l2-subdev.rst >> +++ b/Documentation/media/kapi/v4l2-subdev.rst >> @@ -262,6 +262,35 @@ is called. After all subdevices have been located >> the .complete() callback is >> called. When a subdevice is removed from the system the .unbind() >> method is >> called. All three callbacks are optional. >> >> +Streaming control on Media controller enabled devices >> +^ >> + >> +Starting and stopping the stream are somewhat complex operations that >> +often require walking the media graph to enable streaming on >> +sub-devices which the pipeline consists of. This involves interaction >> +between multiple drivers, sometimes more than two. > > That's still not ok, as it creates a black hole for devnode-based > devices. > > I would change it to something like: > > Streaming control > ^ > > Starting and stopping the stream are somewhat complex operations that > often require to enable streaming on sub-devices which the pipeline > consists of. This involves interaction between multiple drivers, > sometimes > more than two. > > The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is responsible > for starting and stopping the stream on the sub-device it is called > on. > > Streaming control on devnode-centric devices > > > On **devnode-centric** devices, the main driver is responsible enable > stream all all sub-devices. On most cases, all the main driver need > to do is to broadcast s_stream to all connected sub-devices by calling > :c:func:`v4l2_device_call_all`, e. g.:: > > v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 1); Looks good to me. > > Streaming control on mc-centric devices > ~~~ > > On **mc-centric** devices, it usually requires walking the media graph > to enable streaming only at the sub-devices which the pipeline consists > of. > > (place here the details for such scenario) This part requires a more detailed description of the problem area. What makes a difference here is that there's a pipeline this pipeline may be controlled more than one driver. (More elaborate discussion below.) > >> +The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is responsible >> +for starting and stopping the stream on the sub-device it is called >> +on. A device driver is only responsible for calling the ``.s_stream()`` >> ops >> +of the adjacent sub-devices that are connected to its sink pads >> +through an enabled link. A driver may not call ``.s_stream()`` op >> +of any other sub-device further up in the pipeline, for instance. >> + >> +This means that a sub-device driver is thus in direct control of >> +whether the upstream sub-devices start (or stop) streaming before or >> +after the sub-device itself is set up for streaming. >> + >> +.. note:: >> + >> + As the ``.s_stream()`` callback is called recursively through the >> + sub-devices along the pipeline, it is important to keep the >> + recursion as short as possible. To this end, drivers are encouraged >> + to avoid recursively calling ``.s_stream()`` internally
[PATCH] [media] cx18: Make i2c_algo_bit_data const
Make this const as it is only used in a copy operation.
Signed-off-by: Bhumika Goyal
---
drivers/media/pci/cx18/cx18-i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/pci/cx18/cx18-i2c.c
b/drivers/media/pci/cx18/cx18-i2c.c
index eabdd4c..d94ba24 100644
--- a/drivers/media/pci/cx18/cx18-i2c.c
+++ b/drivers/media/pci/cx18/cx18-i2c.c
@@ -216,7 +216,7 @@ static int cx18_getsda(void *data)
#define CX18_SCL_PERIOD (10) /* usecs. 10 usec is period for a 100 KHz clock */
#define CX18_ALGO_BIT_TIMEOUT (2) /* seconds */
-static struct i2c_algo_bit_data cx18_i2c_algo_template = {
+static const struct i2c_algo_bit_data cx18_i2c_algo_template = {
.setsda = cx18_setsda,
.setscl = cx18_setscl,
.getsda = cx18_getsda,
--
1.9.1
[PATCH] [media] bt8xx: Make i2c_algo_bit_data const
Make this const as it is only used in a copy operation.
Signed-off-by: Bhumika Goyal
---
drivers/media/pci/bt8xx/bttv-i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/pci/bt8xx/bttv-i2c.c
b/drivers/media/pci/bt8xx/bttv-i2c.c
index 274fd03..eccd1e3 100644
--- a/drivers/media/pci/bt8xx/bttv-i2c.c
+++ b/drivers/media/pci/bt8xx/bttv-i2c.c
@@ -97,7 +97,7 @@ static int bttv_bit_getsda(void *data)
return state;
}
-static struct i2c_algo_bit_data bttv_i2c_algo_bit_template = {
+static const struct i2c_algo_bit_data bttv_i2c_algo_bit_template = {
.setsda = bttv_bit_setsda,
.setscl = bttv_bit_setscl,
.getsda = bttv_bit_getsda,
--
1.9.1
Re: [PATCH] device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()
Hi Niklas,
Niklas Söderlund wrote:
Using CONFIG_OF_DYNAMIC=y uncovered an imbalance in the usecount of the
node being passed to of_fwnode_graph_get_port_parent(). Preserve the
usecount just like it is done in of_graph_get_port_parent().
The of_fwnode_graph_get_port_parent() is called by
fwnode_graph_get_port_parent() which obtains the port node through
fwnode_get_parent(). If you take a reference here, calling
fwnode_graph_get_port_parent() will end up incrementing the port node's
use count. In other words, my understanding is that dropping the
reference to the port node isn't a problem but intended behaviour here.
I wonder if I miss something.
Fixes: 3b27d00e7b6d7c88 ("device property: Move fwnode graph ops to firmware
specific locations")
Signed-off-by: Niklas Söderlund
---
drivers/of/property.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 067f9fab7b77c794..637dcb4833e2af60 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -922,6 +922,12 @@ of_fwnode_graph_get_port_parent(struct fwnode_handle
*fwnode)
{
struct device_node *np;
+ /*
+* Preserve usecount for passed in node as of_get_next_parent()
+* will do of_node_put() on it.
+*/
+ of_node_get(to_of_node(fwnode));
+
/* Get the parent of the port */
np = of_get_next_parent(to_of_node(fwnode));
if (!np)
--
Sakari Ailus
sakari.ai...@linux.intel.com
Re: [PATCH 5/5] [media] cxusb: add analog mode support for Medion MD95700
Hi Maciej,
On 08/10/2017 11:53 PM, Maciej S. Szmigiero wrote:
> This patch adds support for analog part of Medion 95700 in the cxusb
> driver.
>
> What works:
> * Video capture at various sizes with sequential fields,
> * Input switching (TV Tuner, Composite, S-Video),
> * TV and radio tuning,
> * Video standard switching and auto detection,
> * Radio mode switching (stereo / mono),
> * Unplugging while capturing,
> * DVB / analog coexistence,
> * Raw BT.656 stream support.
Another scary patch :-)
A high-level question first: is any of the code in cxusb-analog medion
specific? There are a lot of cxusb_medion_ prefixes, but I wonder if that
shouldn't be cxusb_analog_.
There are some obvious code cleanups that need to take place first, such
as the huge functions with too many indentations. I would also split off
cxusb-analog.c as a separate patch.
Regards,
Hans
> What does not work yet:
> * Audio,
> * VBI,
> * Picture controls.
>
> Signed-off-by: Maciej S. Szmigiero
> ---
> drivers/media/usb/dvb-usb/Kconfig|8 +-
> drivers/media/usb/dvb-usb/Makefile |2 +-
> drivers/media/usb/dvb-usb/cxusb-analog.c | 1867
> ++
> drivers/media/usb/dvb-usb/cxusb.c| 453 +++-
> drivers/media/usb/dvb-usb/cxusb.h| 137 +++
> drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 20 +-
> drivers/media/usb/dvb-usb/dvb-usb-init.c | 13 +
> drivers/media/usb/dvb-usb/dvb-usb.h |8 +
> 8 files changed, 2449 insertions(+), 59 deletions(-)
> create mode 100644 drivers/media/usb/dvb-usb/cxusb-analog.c
>
> diff --git a/drivers/media/usb/dvb-usb/Kconfig
> b/drivers/media/usb/dvb-usb/Kconfig
> index 959fa09dfd92..ba941069ae3b 100644
> --- a/drivers/media/usb/dvb-usb/Kconfig
> +++ b/drivers/media/usb/dvb-usb/Kconfig
> @@ -118,7 +118,9 @@ config DVB_USB_UMT_010
>
> config DVB_USB_CXUSB
> tristate "Conexant USB2.0 hybrid reference design support"
> - depends on DVB_USB
> + depends on DVB_USB && VIDEO_V4L2
> + select VIDEO_CX25840
> + select VIDEOBUF2_VMALLOC
> select DVB_PLL if MEDIA_SUBDRV_AUTOSELECT
> select DVB_CX22702 if MEDIA_SUBDRV_AUTOSELECT
> select DVB_LGDT330X if MEDIA_SUBDRV_AUTOSELECT
> @@ -136,8 +138,8 @@ config DVB_USB_CXUSB
> select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
> help
> Say Y here to support the Conexant USB2.0 hybrid reference design.
> - Currently, only DVB and ATSC modes are supported, analog mode
> - shall be added in the future. Devices that require this module:
> + DVB and ATSC modes are supported, with basic analog mode support
> + for Medion MD95700. Devices that require this module:
>
> Medion MD95700 hybrid USB2.0 device.
> DViCO FusionHDTV (Bluebird) USB2.0 devices
> diff --git a/drivers/media/usb/dvb-usb/Makefile
> b/drivers/media/usb/dvb-usb/Makefile
> index 3b3f32b426d1..24d222e9acc7 100644
> --- a/drivers/media/usb/dvb-usb/Makefile
> +++ b/drivers/media/usb/dvb-usb/Makefile
> @@ -40,7 +40,7 @@ obj-$(CONFIG_DVB_USB_M920X) += dvb-usb-m920x.o
> dvb-usb-digitv-objs := digitv.o
> obj-$(CONFIG_DVB_USB_DIGITV) += dvb-usb-digitv.o
>
> -dvb-usb-cxusb-objs := cxusb.o
> +dvb-usb-cxusb-objs := cxusb.o cxusb-analog.o
> obj-$(CONFIG_DVB_USB_CXUSB) += dvb-usb-cxusb.o
>
> dvb-usb-ttusb2-objs := ttusb2.o
> diff --git a/drivers/media/usb/dvb-usb/cxusb-analog.c
> b/drivers/media/usb/dvb-usb/cxusb-analog.c
> new file mode 100644
> index ..473d3f06145f
> --- /dev/null
> +++ b/drivers/media/usb/dvb-usb/cxusb-analog.c
> @@ -0,0 +1,1867 @@
> +/* DVB USB compliant linux driver for Conexant USB reference design -
> + * (analog part).
> + *
> + * Copyright (C) 2011, 2017 Maciej S. Szmigiero (m...@maciej.szmigiero.name)
> + *
> + * TODO:
> + * * audio support,
> + * * finish radio support (requires audio of course),
> + * * VBI support,
> + * * controls support
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "cxusb.h"
> +
> +static int cxusb_medion_v_queue_setup(struct vb2_queue *q,
> + unsigned int *num_buffers,
> + unsigned int *num_planes,
> + unsigned int sizes[],
> + struct device *alloc_devs[])
> +{
> + struct dvb_usb_device *dvbdev = vb2_get_drv_priv(q);
> + struct cxusb_medion_dev *cxdev = dvbdev->priv;
> + unsigned int size = cxdev->raw_mode ?
> + CXUSB_VIDEO_MAX_FRAME_SIZE :
> + cxdev->width * cxdev->height * 2;
Re: [PATCH 1/5] [media] cx25840: add pin to pad mapping and output format configuration
Hi Maciej,
On 08/10/2017 11:50 PM, Maciej S. Szmigiero wrote:
> This commit adds pin to pad mapping and output format configuration support
> in CX2584x-series chips to cx25840 driver.
>
> This functionality is then used to allow disabling ivtv-specific hacks
> (called a "generic mode"), so cx25840 driver can be used for other devices
> not needing them without risking compatibility problems.
>
> Signed-off-by: Maciej S. Szmigiero
I'll be honest: this patch is scary! This driver is quite fragile and I don't
want to apply this as-is.
It would help if you would explain what exactly the 'ivtv-specific' hacks are
that prevent you from using this in cxusb.
You also seem to support many more io pins than cxusb needs. I would certainly
only add support for the minimum you need to make it work.
I much prefer the way cx23885_s_io_pin_config is created: it's easier to follow
than the maze of macros and mapping functions that this patch introduces.
Regards,
Hans
> ---
> drivers/media/i2c/cx25840/cx25840-core.c | 413
> ++-
> drivers/media/i2c/cx25840/cx25840-core.h | 11 +
> drivers/media/i2c/cx25840/cx25840-vbi.c | 3 +
> drivers/media/pci/ivtv/ivtv-i2c.c| 1 +
> include/media/drv-intf/cx25840.h | 74 +-
> 5 files changed, 500 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/cx25840/cx25840-core.c
> b/drivers/media/i2c/cx25840/cx25840-core.c
> index 39f51daa7558..078b94ae55ac 100644
> --- a/drivers/media/i2c/cx25840/cx25840-core.c
> +++ b/drivers/media/i2c/cx25840/cx25840-core.c
> @@ -21,6 +21,9 @@
> * CX23888 DIF support for the HVR1850
> * Copyright (C) 2011 Steven Toth
> *
> + * CX2584x pin to pad mapping and output format configuration support are
> + * Copyright (C) 2011 Maciej S. Szmigiero
> + *
> * This program is free software; you can redistribute it and/or
> * modify it under the terms of the GNU General Public License
> * as published by the Free Software Foundation; either version 2
> @@ -316,6 +319,279 @@ static int cx23885_s_io_pin_config(struct v4l2_subdev
> *sd, size_t n,
> return 0;
> }
>
> +static u8 cx25840_function_to_pad(struct i2c_client *client, u8 function)
> +{
> + switch (function) {
> + case CX25840_PAD_ACTIVE:
> + return 1;
> +
> + case CX25840_PAD_VACTIVE:
> + return 2;
> +
> + case CX25840_PAD_CBFLAG:
> + return 3;
> +
> + case CX25840_PAD_VID_DATA_EXT0:
> + return 4;
> +
> + case CX25840_PAD_VID_DATA_EXT1:
> + return 5;
> +
> + case CX25840_PAD_GPO0:
> + return 6;
> +
> + case CX25840_PAD_GPO1:
> + return 7;
> +
> + case CX25840_PAD_GPO2:
> + return 8;
> +
> + case CX25840_PAD_GPO3:
> + return 9;
> +
> + case CX25840_PAD_IRQ_N:
> + return 10;
> +
> + case CX25840_PAD_AC_SYNC:
> + return 11;
> +
> + case CX25840_PAD_AC_SDOUT:
> + return 12;
> +
> + case CX25840_PAD_PLL_CLK:
> + return 13;
> +
> + case CX25840_PAD_VRESET:
> + return 14;
> +
> + default:
> + if (function != CX25840_PAD_DEFAULT)
> + v4l_err(client,
> + "invalid function %u, assuming default\n",
> + (unsigned int)function);
> + return 0;
> + }
> +}
> +
> +static void cx25840_set_invert(u8 *pinctrl3, u8 *voutctrl4, u8 function,
> +u8 pin, bool invert)
> +{
> + if (function != CX25840_PAD_DEFAULT)
> + switch (function) {
> + case CX25840_PAD_IRQ_N:
> + if (invert)
> + *pinctrl3 &= ~2;
> + else
> + *pinctrl3 |= 2;
> + break;
> +
> + case CX25840_PAD_ACTIVE:
> + if (invert)
> + *voutctrl4 |= BIT(2);
> + else
> + *voutctrl4 &= ~BIT(2);
> + break;
> +
> + case CX25840_PAD_VACTIVE:
> + if (invert)
> + *voutctrl4 |= BIT(5);
> + else
> + *voutctrl4 &= ~BIT(5);
> + break;
> +
> + case CX25840_PAD_CBFLAG:
> + if (invert)
> + *voutctrl4 |= BIT(4);
> + else
> + *voutctrl4 &= ~BIT(4);
> + break;
> +
> + case CX25840_PAD_VRESET:
> + if (invert)
> + *voutctrl4 |= BIT(0);
> + else
> + *voutctrl4 &= ~BIT(0);
> + break;
> +
> + default:
> + break;
> + }
> + else
> + s
[PATCH] device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()
Using CONFIG_OF_DYNAMIC=y uncovered an imbalance in the usecount of the
node being passed to of_fwnode_graph_get_port_parent(). Preserve the
usecount just like it is done in of_graph_get_port_parent().
Fixes: 3b27d00e7b6d7c88 ("device property: Move fwnode graph ops to firmware
specific locations")
Signed-off-by: Niklas Söderlund
---
drivers/of/property.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 067f9fab7b77c794..637dcb4833e2af60 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -922,6 +922,12 @@ of_fwnode_graph_get_port_parent(struct fwnode_handle
*fwnode)
{
struct device_node *np;
+ /*
+* Preserve usecount for passed in node as of_get_next_parent()
+* will do of_node_put() on it.
+*/
+ of_node_get(to_of_node(fwnode));
+
/* Get the parent of the port */
np = of_get_next_parent(to_of_node(fwnode));
if (!np)
--
2.14.0
For posterity here is the console log:
OF: ERROR: Bad of_node_put() on
/soc/i2c@e66d8000/gmsl-deserializer@1/ports/port@4
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.13.0-rc4-00418-g32df6aeea9a6f626 #14
Hardware name: Renesas Salvator-X board based on r8a7795 ES1.x (DT)
Call trace:
[] dump_backtrace+0x0/0x238
[] show_stack+0x14/0x20
[] dump_stack+0x9c/0xbc
[] of_node_release+0xb8/0xc0
[] kobject_put+0x84/0xf0
[] of_node_put+0x14/0x28
[] of_fwnode_put+0x24/0x40
[] fwnode_graph_get_port_parent+0x60/0xb0
[] match_fwnode+0x2c/0x88
[] v4l2_async_belongs+0x78/0x120
[] v4l2_async_notifier_register+0x11c/0x1d8
[] v4l2_async_test_notify+0x58/0x160
[] v4l2_async_notifier_register+0xf0/0x1d8
[] rcar_vin_probe+0x65c/0x718
[] platform_drv_probe+0x58/0xb8
[] driver_probe_device+0x22c/0x2d8
[] __driver_attach+0xbc/0xc0
[] bus_for_each_dev+0x4c/0x98
[] driver_attach+0x20/0x28
[] bus_add_driver+0x1b8/0x228
[] driver_register+0x60/0xf8
[] __platform_driver_register+0x40/0x48
[] rcar_vin_driver_init+0x18/0x20
[] do_one_initcall+0x80/0x110
[] kernel_init_freeable+0x188/0x224
[] kernel_init+0x10/0x100
[] ret_from_fork+0x10/0x50
Re: [PATCH 1/7 v3] media: vb2: add bidirectional flag in vb2_queue
Hi Stanimir,
On 2017-08-21 13:34, Stanimir Varbanov wrote:
This change is intended to give to the v4l2 drivers a choice to
change the default behavior of the v4l2-core DMA mapping direction
from DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or
OUTPUT) to DMA_BIDIRECTIONAL during queue_init time.
Initially the issue with DMA mapping direction has been found in
Venus encoder driver where the hardware (firmware side) adds few
lines padding on bottom of the image buffer, and the consequence
is triggering of IOMMU protection faults.
This will help supporting venus encoder (and probably other drivers
in the future) which wants to map output type of buffers as
read/write.
Signed-off-by: Stanimir Varbanov
Thanks for the patch.
Acked-by: Marek Szyprowski
While touching this, I would love to unify set_page_dirty_lock()
related code in videobuf2-dc, videobuf2-sg and videobuf2-vmalloc.
IMHO the pattern used in videobuf2-vmalloc should be copied to
videobuf2-sg (currently checks for dma_dir for every single page)
and videobuf2-dc (currently it lacks any checks and calls
set_page_dirty_lock() unconditionally). If you have a little bit
of spare time, please prepare a separate patch for the above
mentioned fix.
---
v3: update V4L2 dma-sg/contig and vmalloc memory type ops with a
check for DMA_BIDIRECTIONAL.
v2: move dma_dir into private section.
drivers/media/v4l2-core/videobuf2-core.c | 17 -
drivers/media/v4l2-core/videobuf2-dma-contig.c | 3 ++-
drivers/media/v4l2-core/videobuf2-dma-sg.c | 6 --
drivers/media/v4l2-core/videobuf2-vmalloc.c| 6 --
include/media/videobuf2-core.h | 13 +
5 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 0924594989b4..cb115ba6a1d2 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -194,8 +194,6 @@ static void __enqueue_in_driver(struct vb2_buffer *vb);
static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
{
struct vb2_queue *q = vb->vb2_queue;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
void *mem_priv;
int plane;
int ret = -ENOMEM;
@@ -209,7 +207,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
mem_priv = call_ptr_memop(vb, alloc,
q->alloc_devs[plane] ? : q->dev,
- q->dma_attrs, size, dma_dir, q->gfp_flags);
+ q->dma_attrs, size, q->dma_dir, q->gfp_flags);
if (IS_ERR_OR_NULL(mem_priv)) {
if (mem_priv)
ret = PTR_ERR(mem_priv);
@@ -978,8 +976,6 @@ static int __prepare_userptr(struct vb2_buffer *vb, const
void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1030,7 +1026,7 @@ static int __prepare_userptr(struct vb2_buffer *vb, const
void *pb)
mem_priv = call_ptr_memop(vb, get_userptr,
q->alloc_devs[plane] ? : q->dev,
planes[plane].m.userptr,
- planes[plane].length, dma_dir);
+ planes[plane].length, q->dma_dir);
if (IS_ERR(mem_priv)) {
dprintk(1, "failed acquiring userspace memory for plane
%d\n",
plane);
@@ -1096,8 +1092,6 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, const
void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1156,7 +1150,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, const
void *pb)
/* Acquire each plane's memory */
mem_priv = call_ptr_memop(vb, attach_dmabuf,
q->alloc_devs[plane] ? : q->dev,
- dbuf, planes[plane].length, dma_dir);
+ dbuf, planes[plane].length, q->dma_dir);
if (IS_ERR(mem_priv)) {
dprintk(1, "failed to attach dmabuf\n");
ret = PTR_ERR(mem_priv);
@@ -2003,6 +1997,11 @@ int vb2_core_queue_init(struct vb2_queue *q)
if (q->buf_struct_size == 0)
q->buf_struct_size = sizeof(struct vb2_buffer);
+ if (q->bidirectional)
+ q->dma_dir = DMA_BIDIRECTIONAL;
+ else
+
Re: [PATCH 2/2] [media] solo6x10: make snd_kcontrol_new const
On 16/Aug/2017 14:47, Bhumika Goyal wrote:
> Make this const as it is only used during a copy operation.
> Done using Coccinelle.
>
> Signed-off-by: Bhumika Goyal
> ---
> drivers/media/pci/solo6x10/solo6x10-g723.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/pci/solo6x10/solo6x10-g723.c
> b/drivers/media/pci/solo6x10/solo6x10-g723.c
> index 3ca9470..81be1b8 100644
> --- a/drivers/media/pci/solo6x10/solo6x10-g723.c
> +++ b/drivers/media/pci/solo6x10/solo6x10-g723.c
> @@ -319,7 +319,7 @@ static int snd_solo_capture_volume_put(struct
> snd_kcontrol *kcontrol,
> return 1;
> }
>
> -static struct snd_kcontrol_new snd_solo_capture_volume = {
> +static const struct snd_kcontrol_new snd_solo_capture_volume = {
> .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> .name = "Capture Volume",
> .info = snd_solo_capture_volume_info,
> --
> 1.9.1
>
Signed-off-by: Ismael Luceno
signature.asc
Description: PGP signature
Re: [PATCH v2 1/2] docs-rst: media: Document s_stream() video op usage for MC enabled devices
Em Mon, 21 Aug 2017 12:14:18 +0200 Hans Verkuil escreveu: > On 08/21/2017 11:08 AM, Mauro Carvalho Chehab wrote: > > Em Mon, 21 Aug 2017 09:36:49 +0300 > > Sakari Ailus escreveu: > > > >> Hi Mauro, > >> > >> Mauro Carvalho Chehab wrote: > >>> Hi Sakari, > >>> > >>> Em Wed, 16 Aug 2017 15:20:17 +0300 > >>> Sakari Ailus escreveu: > >>> > As we begin to add support for systems with Media controller pipelines > controlled by more than one device driver, it is essential that we > precisely define the responsibilities of each component in the stream > control and common practices. > > Specifically, streaming control is done per sub-device and sub-device > drivers themselves are responsible for streaming setup in upstream > sub-devices. > >>> > >>> IMO, before this patch, we need something like this: > >>> https://patchwork.linuxtv.org/patch/43325/ > >> > >> Thanks. I'll reply separately to that thread. > >> > >>> > > Signed-off-by: Sakari Ailus > Acked-by: Niklas Söderlund > --- > Documentation/media/kapi/v4l2-subdev.rst | 29 > + > 1 file changed, 29 insertions(+) > > diff --git a/Documentation/media/kapi/v4l2-subdev.rst > b/Documentation/media/kapi/v4l2-subdev.rst > index e1f0b72..45088ad 100644 > --- a/Documentation/media/kapi/v4l2-subdev.rst > +++ b/Documentation/media/kapi/v4l2-subdev.rst > @@ -262,6 +262,35 @@ is called. After all subdevices have been located > the .complete() callback is > called. When a subdevice is removed from the system the .unbind() > method is > called. All three callbacks are optional. > > +Streaming control on Media controller enabled devices > +^ > + > +Starting and stopping the stream are somewhat complex operations that > +often require walking the media graph to enable streaming on > +sub-devices which the pipeline consists of. This involves interaction > +between multiple drivers, sometimes more than two. > >>> > >>> That's still not ok, as it creates a black hole for devnode-based > >>> devices. > >>> > >>> I would change it to something like: > >>> > >>> Streaming control > >>> ^ > >>> > >>> Starting and stopping the stream are somewhat complex operations that > >>> often require to enable streaming on sub-devices which the pipeline > >>> consists of. This involves interaction between multiple drivers, > >>> sometimes > >>> more than two. > >>> > >>> The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is responsible > >>> for starting and stopping the stream on the sub-device it is called > >>> on. > >>> > >>> Streaming control on devnode-centric devices > >>> > >>> > >>> On **devnode-centric** devices, the main driver is responsible enable > >>> stream all all sub-devices. On most cases, all the main driver need > >>> to do is to broadcast s_stream to all connected sub-devices by calling > >>> :c:func:`v4l2_device_call_all`, e. g.:: > >>> > >>> v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 1); > >> > >> Looks good to me. > >> > >>> > >>> Streaming control on mc-centric devices > >>> ~~~ > >>> > >>> On **mc-centric** devices, it usually requires walking the media graph > >>> to enable streaming only at the sub-devices which the pipeline consists > >>> of. > >>> > >>> (place here the details for such scenario) > >> > >> This part requires a more detailed description of the problem area. What > >> makes a difference here is that there's a pipeline this pipeline may be > >> controlled more than one driver. (More elaborate discussion below.) > >> > >>> > +The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is responsible > +for starting and stopping the stream on the sub-device it is called > +on. A device driver is only responsible for calling the ``.s_stream()`` > ops > +of the adjacent sub-devices that are connected to its sink pads > +through an enabled link. A driver may not call ``.s_stream()`` op > +of any other sub-device further up in the pipeline, for instance. > + > +This means that a sub-device driver is thus in direct control of > +whether the upstream sub-devices start (or stop) streaming before or > +after the sub-device itself is set up for streaming. > + > +.. note:: > + > + As the ``.s_stream()`` callback is called recursively through the > + sub-devices along the pipeline, it is important to keep the > + recursion as short as possible. To this end, drivers are encouraged > + to avoid recursively calling ``.s_stream()`` internally to reduce > + stack usage. Instead, the ``.s_stream(
[GIT PULL FOR v4.14] More constify & venus fixes
The following changes since commit 0779b8855c746c90b85bfe6e16d5dfa2a6a46655: media: ddbridge: fix semicolon.cocci warnings (2017-08-20 10:25:22 -0400) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.14j for you to fetch changes up to b7573e3c684cb552cfbc86712279c131a691acef: media: venus: venc: drop VP9 codec support (2017-08-21 13:43:07 +0200) Arvind Yadav (6): ad9389b: constify i2c_device_id adv7511: constify i2c_device_id adv7842: constify i2c_device_id saa7127: constify i2c_device_id tc358743: constify i2c_device_id ths8200: constify i2c_device_id Bhumika Goyal (5): usb: make i2c_algorithm const i2c: make device_type const media: pci: make i2c_adapter const radio-usb-si4713: make i2c_adapter const usb: make i2c_adapter const Stanimir Varbanov (5): media: venus: mark venc and vdec PM functions as __maybe_unused media: venus: fill missing video_device name media: venus: add helper to check supported codecs media: venus: use helper function to check supported codecs media: venus: venc: drop VP9 codec support drivers/media/i2c/ad9389b.c | 2 +- drivers/media/i2c/adv7511.c | 2 +- drivers/media/i2c/adv7842.c | 2 +- drivers/media/i2c/saa7127.c | 2 +- drivers/media/i2c/soc_camera/mt9t031.c| 2 +- drivers/media/i2c/tc358743.c | 2 +- drivers/media/i2c/ths8200.c | 2 +- drivers/media/pci/cobalt/cobalt-i2c.c | 2 +- drivers/media/pci/cx18/cx18-i2c.c | 2 +- drivers/media/pci/cx23885/cx23885-i2c.c | 2 +- drivers/media/pci/cx25821/cx25821-i2c.c | 2 +- drivers/media/pci/ivtv/ivtv-i2c.c | 4 ++-- drivers/media/pci/netup_unidvb/netup_unidvb_i2c.c | 2 +- drivers/media/pci/saa7134/saa7134-i2c.c | 2 +- drivers/media/pci/saa7164/saa7164-i2c.c | 2 +- drivers/media/platform/qcom/venus/helpers.c | 49 + drivers/media/platform/qcom/venus/helpers.h | 1 + drivers/media/platform/qcom/venus/vdec.c | 31 --- drivers/media/platform/qcom/venus/venc.c | 39 ++- drivers/media/radio/si4713/radio-usb-si4713.c | 2 +- drivers/media/usb/au0828/au0828-i2c.c | 4 ++-- drivers/media/usb/cx231xx/cx231xx-i2c.c | 2 +- drivers/media/usb/em28xx/em28xx-i2c.c | 2 +- drivers/media/usb/hdpvr/hdpvr-i2c.c | 2 +- drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c | 4 ++-- drivers/media/usb/stk1160/stk1160-i2c.c | 2 +- drivers/media/usb/usbvision/usbvision-i2c.c | 4 ++-- 27 files changed, 119 insertions(+), 55 deletions(-)
Re: [PATCH 1/7 v2] media: vb2: add bidirectional flag in vb2_queue
Hi, On 08/21/2017 01:21 PM, Hans Verkuil wrote: > On 08/21/2017 11:29 AM, Marek Szyprowski wrote: >> Hi Stanimir, >> >> On 2017-08-21 11:09, Stanimir Varbanov wrote: >>> This change is intended to give to the v4l2 drivers a choice to >>> change the default behavior of the v4l2-core DMA mapping direction >>> from DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or >>> OUTPUT) to DMA_BIDIRECTIONAL during queue_init time. >>> >>> Initially the issue with DMA mapping direction has been found in >>> Venus encoder driver where the hardware (firmware side) adds few >>> lines padding on bottom of the image buffer, and the consequence >>> is triggering of IOMMU protection faults. >>> >>> This will help supporting venus encoder (and probably other drivers >>> in the future) which wants to map output type of buffers as >>> read/write. >>> >>> Signed-off-by: Stanimir Varbanov >> >> This has been already discussed about a year ago, but it got lost in >> meantime, probably due to lack of users. I hope that this time it >> finally will get into vb2. >> >> For the reference, see https://patchwork.kernel.org/patch/9388163/ > > Interesting. > > Stanimir, I like your implementation better than the macro in the old > patch. But as it said there, videobuf2-dma-sg/contig/vmalloc.c have references > to DMA_FROM_DEVICE that won't work with BIDIRECTIONAL, so those need > to be adapted as well. I missed that when I reviewed your patch :-( Thanks for catching this, I didn't thought too much about usrptr. Sent v3. -- regards, Stan
[PATCH 1/7 v3] media: vb2: add bidirectional flag in vb2_queue
This change is intended to give to the v4l2 drivers a choice to
change the default behavior of the v4l2-core DMA mapping direction
from DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or
OUTPUT) to DMA_BIDIRECTIONAL during queue_init time.
Initially the issue with DMA mapping direction has been found in
Venus encoder driver where the hardware (firmware side) adds few
lines padding on bottom of the image buffer, and the consequence
is triggering of IOMMU protection faults.
This will help supporting venus encoder (and probably other drivers
in the future) which wants to map output type of buffers as
read/write.
Signed-off-by: Stanimir Varbanov
---
v3: update V4L2 dma-sg/contig and vmalloc memory type ops with a
check for DMA_BIDIRECTIONAL.
v2: move dma_dir into private section.
drivers/media/v4l2-core/videobuf2-core.c | 17 -
drivers/media/v4l2-core/videobuf2-dma-contig.c | 3 ++-
drivers/media/v4l2-core/videobuf2-dma-sg.c | 6 --
drivers/media/v4l2-core/videobuf2-vmalloc.c| 6 --
include/media/videobuf2-core.h | 13 +
5 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 0924594989b4..cb115ba6a1d2 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -194,8 +194,6 @@ static void __enqueue_in_driver(struct vb2_buffer *vb);
static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
{
struct vb2_queue *q = vb->vb2_queue;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
void *mem_priv;
int plane;
int ret = -ENOMEM;
@@ -209,7 +207,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
mem_priv = call_ptr_memop(vb, alloc,
q->alloc_devs[plane] ? : q->dev,
- q->dma_attrs, size, dma_dir, q->gfp_flags);
+ q->dma_attrs, size, q->dma_dir, q->gfp_flags);
if (IS_ERR_OR_NULL(mem_priv)) {
if (mem_priv)
ret = PTR_ERR(mem_priv);
@@ -978,8 +976,6 @@ static int __prepare_userptr(struct vb2_buffer *vb, const
void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1030,7 +1026,7 @@ static int __prepare_userptr(struct vb2_buffer *vb, const
void *pb)
mem_priv = call_ptr_memop(vb, get_userptr,
q->alloc_devs[plane] ? : q->dev,
planes[plane].m.userptr,
- planes[plane].length, dma_dir);
+ planes[plane].length, q->dma_dir);
if (IS_ERR(mem_priv)) {
dprintk(1, "failed acquiring userspace memory for plane
%d\n",
plane);
@@ -1096,8 +1092,6 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, const
void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1156,7 +1150,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, const
void *pb)
/* Acquire each plane's memory */
mem_priv = call_ptr_memop(vb, attach_dmabuf,
q->alloc_devs[plane] ? : q->dev,
- dbuf, planes[plane].length, dma_dir);
+ dbuf, planes[plane].length, q->dma_dir);
if (IS_ERR(mem_priv)) {
dprintk(1, "failed to attach dmabuf\n");
ret = PTR_ERR(mem_priv);
@@ -2003,6 +1997,11 @@ int vb2_core_queue_init(struct vb2_queue *q)
if (q->buf_struct_size == 0)
q->buf_struct_size = sizeof(struct vb2_buffer);
+ if (q->bidirectional)
+ q->dma_dir = DMA_BIDIRECTIONAL;
+ else
+ q->dma_dir = q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+
return 0;
}
EXPORT_SYMBOL_GPL(vb2_core_queue_init);
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 5b90a66b9e78..9f389f36566d 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -508,7 +508,8 @@ static void *vb2_dc_get_userptr(struct device *dev,
unsigned long vaddr,
buf->dma_dir = dma_dir;
offset = vaddr & ~PAGE_MASK;
-
Re: [PATCH] Staging: bcm2048: fix bare use of 'unsigned' in radio-bcm2048.c
On 08/04/2017 04:58 PM, Branislav Radocaj wrote: > This is a patch to the radio-bcm2048.c file that fixes up > a warning found by the checkpatch.pl tool. > > Signed-off-by: Branislav Radocaj > --- > drivers/staging/media/bcm2048/radio-bcm2048.c | 50 > +-- > 1 file changed, 25 insertions(+), 25 deletions(-) > > diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c > b/drivers/staging/media/bcm2048/radio-bcm2048.c > index 38f72d069e27..90b8f05201ba 100644 > --- a/drivers/staging/media/bcm2048/radio-bcm2048.c > +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c > @@ -2000,9 +2000,9 @@ static ssize_t bcm2048_##prop##_read(struct device > *dev,\ > return sprintf(buf, mask "\n", value); \ > } > > -#define DEFINE_SYSFS_PROPERTY(prop, signal, size, mask, check) > \ > -property_write(prop, signal size, mask, check) > \ > -property_read(prop, size, mask) > +#define DEFINE_SYSFS_PROPERTY(prop, signal_size, size, mask, check) \ > +property_write(prop, signal_size, mask, check) > \ > +property_read(prop, size, mask) > \ Jeez, the code in this header is awful. Let's improve this a bit more: The 'size' argument in property_read is unused AFAICT, so remove it from that macro and wherever it is used. The 'signal, size' arguments of property_write is just a type, so replace 'signal, size' by prop_type. Update DEFINE_SYSFS_PROPERTY accordingly and then all the DEFINE_SYSFS_PROPERTY lines become: DEFINE_SYSFS_PROPERTY(power_state, unsigned int, "%u", 0) which finally makes sense when you read it. Regards, Hans > > #define property_str_read(prop, size) > \ > static ssize_t bcm2048_##prop##_read(struct device *dev, \ > @@ -2028,27 +2028,27 @@ static ssize_t bcm2048_##prop##_read(struct device > *dev, \ > return count; \ > } > > -DEFINE_SYSFS_PROPERTY(power_state, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(mute, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(audio_route, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(dac_output, unsigned, int, "%u", 0) > - > -DEFINE_SYSFS_PROPERTY(fm_hi_lo_injection, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(fm_frequency, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(fm_af_frequency, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(fm_deemphasis, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(fm_rds_mask, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(fm_best_tune_mode, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(fm_search_rssi_threshold, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(fm_search_mode_direction, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(fm_search_tune_mode, unsigned, int, "%u", value > 3) > - > -DEFINE_SYSFS_PROPERTY(rds, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(rds_b_block_mask, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(rds_b_block_match, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(rds_pi_mask, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(rds_pi_match, unsigned, int, "%u", 0) > -DEFINE_SYSFS_PROPERTY(rds_wline, unsigned, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(power_state, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(mute, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(audio_route, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(dac_output, unsigned int, int, "%u", 0) > + > +DEFINE_SYSFS_PROPERTY(fm_hi_lo_injection, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(fm_frequency, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(fm_af_frequency, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(fm_deemphasis, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(fm_rds_mask, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(fm_best_tune_mode, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(fm_search_rssi_threshold, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(fm_search_mode_direction, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(fm_search_tune_mode, unsigned int, int, "%u", value > > 3) > + > +DEFINE_SYSFS_PROPERTY(rds, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(rds_b_block_mask, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(rds_b_block_match, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(rds_pi_mask, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(rds_pi_match, unsigned int, int, "%u", 0) > +DEFINE_SYSFS_PROPERTY(rds_wline, unsigned int, int, "%u", 0) > property_read(rds_pi, unsigned int, "%x") > property_str_read(rds_rt, (BCM2048_MAX_RDS_RT + 1)) > property_str_read(rds_ps, (BCM2048_MAX_RDS_PS + 1)) > @@ -2060,7 +2060,7 @@ property_read(region_bottom_frequency, unsigned int, > "%u") > property_read(region_top_frequency, unsigned int, "%u") > property_signed_read(fm_carrier_error, in
Re: [PATCH 1/7 v2] media: vb2: add bidirectional flag in vb2_queue
On 08/21/2017 11:29 AM, Marek Szyprowski wrote:
> Hi Stanimir,
>
> On 2017-08-21 11:09, Stanimir Varbanov wrote:
>> This change is intended to give to the v4l2 drivers a choice to
>> change the default behavior of the v4l2-core DMA mapping direction
>> from DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or
>> OUTPUT) to DMA_BIDIRECTIONAL during queue_init time.
>>
>> Initially the issue with DMA mapping direction has been found in
>> Venus encoder driver where the hardware (firmware side) adds few
>> lines padding on bottom of the image buffer, and the consequence
>> is triggering of IOMMU protection faults.
>>
>> This will help supporting venus encoder (and probably other drivers
>> in the future) which wants to map output type of buffers as
>> read/write.
>>
>> Signed-off-by: Stanimir Varbanov
>
> This has been already discussed about a year ago, but it got lost in
> meantime, probably due to lack of users. I hope that this time it
> finally will get into vb2.
>
> For the reference, see https://patchwork.kernel.org/patch/9388163/
Interesting.
Stanimir, I like your implementation better than the macro in the old
patch. But as it said there, videobuf2-dma-sg/contig/vmalloc.c have references
to DMA_FROM_DEVICE that won't work with BIDIRECTIONAL, so those need
to be adapted as well. I missed that when I reviewed your patch :-(
Regards,
Hans
>
>> ---
>> v2: move dma_dir into private section.
>>
>> drivers/media/v4l2-core/videobuf2-core.c | 17 -
>> include/media/videobuf2-core.h | 13 +
>> 2 files changed, 21 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index 0924594989b4..cb115ba6a1d2 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -194,8 +194,6 @@ static void __enqueue_in_driver(struct vb2_buffer *vb);
>> static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>> {
>> struct vb2_queue *q = vb->vb2_queue;
>> -enum dma_data_direction dma_dir =
>> -q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>> void *mem_priv;
>> int plane;
>> int ret = -ENOMEM;
>> @@ -209,7 +207,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>>
>> mem_priv = call_ptr_memop(vb, alloc,
>> q->alloc_devs[plane] ? : q->dev,
>> -q->dma_attrs, size, dma_dir, q->gfp_flags);
>> +q->dma_attrs, size, q->dma_dir, q->gfp_flags);
>> if (IS_ERR_OR_NULL(mem_priv)) {
>> if (mem_priv)
>> ret = PTR_ERR(mem_priv);
>> @@ -978,8 +976,6 @@ static int __prepare_userptr(struct vb2_buffer *vb,
>> const void *pb)
>> void *mem_priv;
>> unsigned int plane;
>> int ret = 0;
>> -enum dma_data_direction dma_dir =
>> -q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>> bool reacquired = vb->planes[0].mem_priv == NULL;
>>
>> memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
>> @@ -1030,7 +1026,7 @@ static int __prepare_userptr(struct vb2_buffer *vb,
>> const void *pb)
>> mem_priv = call_ptr_memop(vb, get_userptr,
>> q->alloc_devs[plane] ? : q->dev,
>> planes[plane].m.userptr,
>> -planes[plane].length, dma_dir);
>> +planes[plane].length, q->dma_dir);
>> if (IS_ERR(mem_priv)) {
>> dprintk(1, "failed acquiring userspace memory for plane
>> %d\n",
>> plane);
>> @@ -1096,8 +1092,6 @@ static int __prepare_dmabuf(struct vb2_buffer *vb,
>> const void *pb)
>> void *mem_priv;
>> unsigned int plane;
>> int ret = 0;
>> -enum dma_data_direction dma_dir =
>> -q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>> bool reacquired = vb->planes[0].mem_priv == NULL;
>>
>> memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
>> @@ -1156,7 +1150,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb,
>> const void *pb)
>> /* Acquire each plane's memory */
>> mem_priv = call_ptr_memop(vb, attach_dmabuf,
>> q->alloc_devs[plane] ? : q->dev,
>> -dbuf, planes[plane].length, dma_dir);
>> +dbuf, planes[plane].length, q->dma_dir);
>> if (IS_ERR(mem_priv)) {
>> dprintk(1, "failed to attach dmabuf\n");
>> ret = PTR_ERR(mem_priv);
>> @@ -2003,6 +1997,11 @@ int vb2_core_queue_init(struct vb2_queue *q)
>> if (q->buf_struct_size == 0)
>> q->buf_struct_size = sizeof(struct vb2_buffer);
>>
>> +if (q->bidirectional)
>> +q->dma_dir = DMA_BIDIRECTIONAL;
>> +else
>> +q->dma_dir = q-
Re: [PATCH v1 4/5] [media] stm32-dcmi: set default format at open()
On 08/04/2017 02:00 PM, Hans Verkuil wrote:
> On 28/07/17 12:05, Hugues Fruchet wrote:
>> Ensure that we start with default pixel format and resolution
>> when opening a new instance.
>
> Why? The format is persistent in V4L2 and (re)opening the video device
> shouldn't change the format.
>
> Suppose you use v4l2-ctl to set up a format. E.g. v4l2-ctl -v
> width=320,height-240.
> Now run v4l2-ctl -V to get the format and with this code it would suddenly be
> back to the default!
>
> You set up the default format in the dcmi_graph_notify_complete, but after
> that
> it is only changed if userspace explicitly requests it.
>
> Regards,
>
> Hans
>
Thanks Hans, I didn't catch this before, thanks for explanation.
>>
>> Signed-off-by: Hugues Fruchet
>> ---
>> drivers/media/platform/stm32/stm32-dcmi.c | 49
>> ++-
>> 1 file changed, 28 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
>> b/drivers/media/platform/stm32/stm32-dcmi.c
>> index 4733d1f..2be56b6 100644
>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>> @@ -890,6 +890,28 @@ static int dcmi_enum_frameintervals(struct file *file,
>> void *fh,
>> return 0;
>> }
>>
>> +static int dcmi_set_default_fmt(struct stm32_dcmi *dcmi)
>> +{
>> +struct v4l2_format f = {
>> +.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
>> +.fmt.pix = {
>> +.width = CIF_WIDTH,
>> +.height = CIF_HEIGHT,
>> +.field = V4L2_FIELD_NONE,
>> +.pixelformat= dcmi->sd_formats[0]->fourcc,
>> +},
>> +};
>> +int ret;
>> +
>> +ret = dcmi_try_fmt(dcmi, &f, NULL);
>> +if (ret)
>> +return ret;
>> +dcmi->sd_format = dcmi->sd_formats[0];
>> +dcmi->fmt = f;
>> +
>> +return 0;
>> +}
>> +
>> static const struct of_device_id stm32_dcmi_of_match[] = {
>> { .compatible = "st,stm32-dcmi"},
>> { /* end node */ },
>> @@ -916,7 +938,13 @@ static int dcmi_open(struct file *file)
>> if (ret < 0 && ret != -ENOIOCTLCMD)
>> goto fh_rel;
>>
>> +ret = dcmi_set_default_fmt(dcmi);
>> +if (ret)
>> +goto power_off;
>> +
>> ret = dcmi_set_fmt(dcmi, &dcmi->fmt);
>> +
>> +power_off:
>> if (ret)
>> v4l2_subdev_call(sd, core, s_power, 0);
>> fh_rel:
>> @@ -991,27 +1019,6 @@ static int dcmi_release(struct file *file)
>> .read = vb2_fop_read,
>> };
>>
>> -static int dcmi_set_default_fmt(struct stm32_dcmi *dcmi)
>> -{
>> -struct v4l2_format f = {
>> -.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
>> -.fmt.pix = {
>> -.width = CIF_WIDTH,
>> -.height = CIF_HEIGHT,
>> -.field = V4L2_FIELD_NONE,
>> -.pixelformat= dcmi->sd_formats[0]->fourcc,
>> -},
>> -};
>> -int ret;
>> -
>> -ret = dcmi_try_fmt(dcmi, &f, NULL);
>> -if (ret)
>> -return ret;
>> -dcmi->sd_format = dcmi->sd_formats[0];
>> -dcmi->fmt = f;
>> -return 0;
>> -}
>> -
>> static const struct dcmi_format dcmi_formats[] = {
>> {
>> .fourcc = V4L2_PIX_FMT_RGB565,
>>
>
Re: [PATCH v1 5/5] [media] stm32-dcmi: g_/s_selection crop support
Hi Hans, thanks for reviewing
On 08/04/2017 02:26 PM, Hans Verkuil wrote:
> On 28/07/17 12:05, Hugues Fruchet wrote:
>> Implements g_/s_selection crop support by using DCMI crop
>> hardware feature.
>> User can first get the maximum supported resolution of the sensor
>> by calling g_selection(V4L2_SEL_TGT_CROP_BOUNDS).
>> Then user call to s_selection(V4L2_SEL_TGT_CROP) will reset sensor
>> to its maximum resolution and crop request is saved for later usage
>> in s_fmt().
>> Next call to s_fmt() will check if sensor can do frame size request
>> with crop request. If sensor supports only discrete frame sizes,
>> the frame size which is larger than user request is selected in
>> order to be able to match the crop request. Then s_fmt() resolution
>> user request is adjusted to match crop request resolution.
>>
>> Signed-off-by: Hugues Fruchet
>> ---
>> drivers/media/platform/stm32/stm32-dcmi.c | 367
>> +-
>> 1 file changed, 363 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
>> b/drivers/media/platform/stm32/stm32-dcmi.c
>> index 2be56b6..f1fb0b3 100644
>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>> @@ -33,6 +33,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>
>> #define DRV_NAME "stm32-dcmi"
>> @@ -107,6 +108,11 @@ struct dcmi_format {
>> u8 bpp;
>> };
>>
>> +struct dcmi_framesize {
>> +u32 width;
>> +u32 height;
>> +};
>> +
>> struct dcmi_buf {
>> struct vb2_v4l2_buffer vb;
>> boolprepared;
>> @@ -131,10 +137,16 @@ struct stm32_dcmi {
>> struct v4l2_async_notifier notifier;
>> struct dcmi_graph_entityentity;
>> struct v4l2_format fmt;
>> +struct v4l2_rectcrop;
>> +booldo_crop;
>>
>> const struct dcmi_format**sd_formats;
>> unsigned intnb_of_sd_formats;
>> const struct dcmi_format*sd_format;
>> +struct dcmi_framesize *sd_framesizes;
>> +unsigned intnb_of_sd_framesizes;
>
> num_of_sd_framesizes is better.
>
OK, will rename.
>> +struct dcmi_framesize sd_framesize;
>> +struct v4l2_rectsd_bounds;
>>
>> /* Protect this data structure */
>> struct mutexlock;
>> @@ -325,6 +337,28 @@ static int dcmi_start_capture(struct stm32_dcmi *dcmi)
>> return 0;
>> }
>>
>> +static void dcmi_set_crop(struct stm32_dcmi *dcmi)
>> +{
>> +u32 size, start;
>> +
>> +/* Crop resolution */
>> +size = ((dcmi->crop.height - 1) << 16) |
>> +((dcmi->crop.width << 1) - 1);
>> +reg_write(dcmi->regs, DCMI_CWSIZE, size);
>> +
>> +/* Crop start point */
>> +start = ((dcmi->crop.top) << 16) |
>> + ((dcmi->crop.left << 1));
>> +reg_write(dcmi->regs, DCMI_CWSTRT, start);
>> +
>> +dev_dbg(dcmi->dev, "Cropping to %ux%u@%u:%u\n",
>> +dcmi->crop.width, dcmi->crop.height,
>> +dcmi->crop.left, dcmi->crop.top);
>> +
>> +/* Enable crop */
>> +reg_set(dcmi->regs, DCMI_CR, CR_CROP);
>> +}
>> +
>> static irqreturn_t dcmi_irq_thread(int irq, void *arg)
>> {
>> struct stm32_dcmi *dcmi = arg;
>> @@ -540,6 +574,10 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
>> unsigned int count)
>>
>> reg_write(dcmi->regs, DCMI_CR, val);
>>
>> +/* Set crop */
>> +if (dcmi->do_crop)
>> +dcmi_set_crop(dcmi);
>> +
>> /* Enable dcmi */
>> reg_set(dcmi->regs, DCMI_CR, CR_ENABLE);
>>
>> @@ -697,10 +735,37 @@ static const struct dcmi_format
>> *find_format_by_fourcc(struct stm32_dcmi *dcmi,
>> return NULL;
>> }
>>
>> +static void __find_outer_frame_size(struct stm32_dcmi *dcmi,
>> +struct v4l2_pix_format *pix,
>> +struct dcmi_framesize *framesize)
>> +{
>> +struct dcmi_framesize *match = NULL;
>> +unsigned int i;
>> +unsigned int min_err = UINT_MAX;
>> +
>> +for (i = 0; i < dcmi->nb_of_sd_framesizes; i++) {
>> +struct dcmi_framesize *fsize = &dcmi->sd_framesizes[i];
>> +int w_err = (fsize->width - pix->width);
>> +int h_err = (fsize->height - pix->height);
>> +int err = w_err + h_err;
>> +
>> +if ((w_err >= 0) && (h_err >= 0) && (err < min_err)) {
>> +min_err = err;
>> +match = fsize;
>> +}
>> +}
>> +if (!match)
>> +match = &dcmi->sd_framesizes[0];
>> +
>> +*framesize = *match;
>> +}
>> +
>> static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
>> -const struct dcmi_format **sd_format)
>> +const struct dcmi_format **sd_format,
>> +struct
Re: [PATCH v2 2/2] docs-rst: media: Document broken frame handling in stream stop for CSI-2
On 08/16/2017 02:20 PM, Sakari Ailus wrote: > Some CSI-2 transmitters will finish an ongoing frame whereas others will > not. Document that receiver drivers should not assume a particular > behaviour but to work in both cases. > > Signed-off-by: Sakari Ailus > --- > Documentation/media/kapi/csi2.rst | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/Documentation/media/kapi/csi2.rst > b/Documentation/media/kapi/csi2.rst > index e33fcb9..0560100 100644 > --- a/Documentation/media/kapi/csi2.rst > +++ b/Documentation/media/kapi/csi2.rst > @@ -51,6 +51,16 @@ not active. Some transmitters do this automatically but > some have to > be explicitly programmed to do so, and some are unable to do so > altogether due to hardware constraints. > > +Stopping the transmitter > + > + > +A transmitter stops sending the stream of images as a result of > +calling the ``.s_stream()`` callback. Some transmitters may stop the > +stream at a frame boundary whereas others stop immediately, > +effectively leaving the current frame unfinished. The receiver driver > +should not make assumptions either way, but function properly in both > +cases. > + > Receiver drivers > > > Reviewed-by: Hans Verkuil Thanks, Hans
Re: [PATCH v2 1/2] docs-rst: media: Document s_stream() video op usage for MC enabled devices
On 08/21/2017 11:08 AM, Mauro Carvalho Chehab wrote: > Em Mon, 21 Aug 2017 09:36:49 +0300 > Sakari Ailus escreveu: > >> Hi Mauro, >> >> Mauro Carvalho Chehab wrote: >>> Hi Sakari, >>> >>> Em Wed, 16 Aug 2017 15:20:17 +0300 >>> Sakari Ailus escreveu: >>> As we begin to add support for systems with Media controller pipelines controlled by more than one device driver, it is essential that we precisely define the responsibilities of each component in the stream control and common practices. Specifically, streaming control is done per sub-device and sub-device drivers themselves are responsible for streaming setup in upstream sub-devices. >>> >>> IMO, before this patch, we need something like this: >>> https://patchwork.linuxtv.org/patch/43325/ >> >> Thanks. I'll reply separately to that thread. >> >>> Signed-off-by: Sakari Ailus Acked-by: Niklas Söderlund --- Documentation/media/kapi/v4l2-subdev.rst | 29 + 1 file changed, 29 insertions(+) diff --git a/Documentation/media/kapi/v4l2-subdev.rst b/Documentation/media/kapi/v4l2-subdev.rst index e1f0b72..45088ad 100644 --- a/Documentation/media/kapi/v4l2-subdev.rst +++ b/Documentation/media/kapi/v4l2-subdev.rst @@ -262,6 +262,35 @@ is called. After all subdevices have been located the .complete() callback is called. When a subdevice is removed from the system the .unbind() method is called. All three callbacks are optional. +Streaming control on Media controller enabled devices +^ + +Starting and stopping the stream are somewhat complex operations that +often require walking the media graph to enable streaming on +sub-devices which the pipeline consists of. This involves interaction +between multiple drivers, sometimes more than two. >>> >>> That's still not ok, as it creates a black hole for devnode-based >>> devices. >>> >>> I would change it to something like: >>> >>> Streaming control >>> ^ >>> >>> Starting and stopping the stream are somewhat complex operations that >>> often require to enable streaming on sub-devices which the pipeline >>> consists of. This involves interaction between multiple drivers, >>> sometimes >>> more than two. >>> >>> The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is responsible >>> for starting and stopping the stream on the sub-device it is called >>> on. >>> >>> Streaming control on devnode-centric devices >>> >>> >>> On **devnode-centric** devices, the main driver is responsible enable >>> stream all all sub-devices. On most cases, all the main driver need >>> to do is to broadcast s_stream to all connected sub-devices by calling >>> :c:func:`v4l2_device_call_all`, e. g.:: >>> >>> v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 1); >> >> Looks good to me. >> >>> >>> Streaming control on mc-centric devices >>> ~~~ >>> >>> On **mc-centric** devices, it usually requires walking the media graph >>> to enable streaming only at the sub-devices which the pipeline consists >>> of. >>> >>> (place here the details for such scenario) >> >> This part requires a more detailed description of the problem area. What >> makes a difference here is that there's a pipeline this pipeline may be >> controlled more than one driver. (More elaborate discussion below.) >> >>> +The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is responsible +for starting and stopping the stream on the sub-device it is called +on. A device driver is only responsible for calling the ``.s_stream()`` ops +of the adjacent sub-devices that are connected to its sink pads +through an enabled link. A driver may not call ``.s_stream()`` op +of any other sub-device further up in the pipeline, for instance. + +This means that a sub-device driver is thus in direct control of +whether the upstream sub-devices start (or stop) streaming before or +after the sub-device itself is set up for streaming. + +.. note:: + + As the ``.s_stream()`` callback is called recursively through the + sub-devices along the pipeline, it is important to keep the + recursion as short as possible. To this end, drivers are encouraged + to avoid recursively calling ``.s_stream()`` internally to reduce + stack usage. Instead, the ``.s_stream()`` op of the directly + connected sub-devices should come from the callback through which + the driver was first called. + >>> >>> That sounds too complex, and can lead into troubles, if the same >>> sub-device driver is used on completely different devi
Re: [PATCH 3/3] [media] radio: constify usb_device_id
Hi Arvind, thanks for the patch! On Sun, Aug 13, 2017 at 9:54 AM, Arvind Yadav wrote: > usb_device_id are not supposed to change at runtime. All functions > working with usb_device_id provided by work with > const usb_device_id. So mark the non-const structs as const. > > Signed-off-by: Arvind Yadav For dsbr100, radio-mr800 and radio-ma901 please feel free to use: Acked-by: Alexey Klimov > --- > drivers/media/radio/dsbr100.c | 2 +- > drivers/media/radio/radio-keene.c | 2 +- > drivers/media/radio/radio-ma901.c | 2 +- > drivers/media/radio/radio-mr800.c | 2 +- > drivers/media/radio/radio-raremono.c | 2 +- > drivers/media/radio/radio-shark.c | 2 +- > drivers/media/radio/radio-shark2.c| 2 +- > drivers/media/radio/si470x/radio-si470x-usb.c | 2 +- > drivers/media/radio/si4713/radio-usb-si4713.c | 2 +- > 9 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/radio/dsbr100.c b/drivers/media/radio/dsbr100.c > index 53bc8c0..8521bb2 100644 [...] Best regards, Alexey
Re: [PATCH 1/7 v2] media: vb2: add bidirectional flag in vb2_queue
Hi Stanimir,
On 2017-08-21 11:09, Stanimir Varbanov wrote:
This change is intended to give to the v4l2 drivers a choice to
change the default behavior of the v4l2-core DMA mapping direction
from DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or
OUTPUT) to DMA_BIDIRECTIONAL during queue_init time.
Initially the issue with DMA mapping direction has been found in
Venus encoder driver where the hardware (firmware side) adds few
lines padding on bottom of the image buffer, and the consequence
is triggering of IOMMU protection faults.
This will help supporting venus encoder (and probably other drivers
in the future) which wants to map output type of buffers as
read/write.
Signed-off-by: Stanimir Varbanov
This has been already discussed about a year ago, but it got lost in
meantime, probably due to lack of users. I hope that this time it
finally will get into vb2.
For the reference, see https://patchwork.kernel.org/patch/9388163/
---
v2: move dma_dir into private section.
drivers/media/v4l2-core/videobuf2-core.c | 17 -
include/media/videobuf2-core.h | 13 +
2 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 0924594989b4..cb115ba6a1d2 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -194,8 +194,6 @@ static void __enqueue_in_driver(struct vb2_buffer *vb);
static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
{
struct vb2_queue *q = vb->vb2_queue;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
void *mem_priv;
int plane;
int ret = -ENOMEM;
@@ -209,7 +207,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
mem_priv = call_ptr_memop(vb, alloc,
q->alloc_devs[plane] ? : q->dev,
- q->dma_attrs, size, dma_dir, q->gfp_flags);
+ q->dma_attrs, size, q->dma_dir, q->gfp_flags);
if (IS_ERR_OR_NULL(mem_priv)) {
if (mem_priv)
ret = PTR_ERR(mem_priv);
@@ -978,8 +976,6 @@ static int __prepare_userptr(struct vb2_buffer *vb, const
void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1030,7 +1026,7 @@ static int __prepare_userptr(struct vb2_buffer *vb, const
void *pb)
mem_priv = call_ptr_memop(vb, get_userptr,
q->alloc_devs[plane] ? : q->dev,
planes[plane].m.userptr,
- planes[plane].length, dma_dir);
+ planes[plane].length, q->dma_dir);
if (IS_ERR(mem_priv)) {
dprintk(1, "failed acquiring userspace memory for plane
%d\n",
plane);
@@ -1096,8 +1092,6 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, const
void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1156,7 +1150,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, const
void *pb)
/* Acquire each plane's memory */
mem_priv = call_ptr_memop(vb, attach_dmabuf,
q->alloc_devs[plane] ? : q->dev,
- dbuf, planes[plane].length, dma_dir);
+ dbuf, planes[plane].length, q->dma_dir);
if (IS_ERR(mem_priv)) {
dprintk(1, "failed to attach dmabuf\n");
ret = PTR_ERR(mem_priv);
@@ -2003,6 +1997,11 @@ int vb2_core_queue_init(struct vb2_queue *q)
if (q->buf_struct_size == 0)
q->buf_struct_size = sizeof(struct vb2_buffer);
+ if (q->bidirectional)
+ q->dma_dir = DMA_BIDIRECTIONAL;
+ else
+ q->dma_dir = q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+
return 0;
}
EXPORT_SYMBOL_GPL(vb2_core_queue_init);
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index cb97c224be73..ef9b64398c8c 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -427,6 +427,16 @@ struct vb2_buf_ops {
* @dev: device to use for the default allocation context if the driver
*doesn't fill in the @alloc_devs array.
* @dma_attrs:DMA attributes to use for the DMA.
+ * @b
Re: [PATCH v2] venus: fix copy/paste error in return_buf_error
Thanks Gustavo! On 08/18/2017 07:07 PM, Gustavo A. R. Silva wrote: > Call function v4l2_m2m_dst_buf_remove_by_buf() instead of > v4l2_m2m_src_buf_remove_by_buf() > > Addresses-Coverity-ID: 1415317 > Cc: Stanimir Varbanov > Cc: Hans Verkuil > Signed-off-by: Gustavo A. R. Silva > --- > Changes in v2: > Stanimir Varbanov confirmed this is a bug. The correct fix is to call > function v4l2_m2m_dst_buf_remove_by_buf instead of function > v4l2_m2m_src_buf_remove_by_buf in the _else_ branch. > > drivers/media/platform/qcom/venus/helpers.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Stanimir Varbanov > > diff --git a/drivers/media/platform/qcom/venus/helpers.c > b/drivers/media/platform/qcom/venus/helpers.c > index 5f4434c..2d61879 100644 > --- a/drivers/media/platform/qcom/venus/helpers.c > +++ b/drivers/media/platform/qcom/venus/helpers.c > @@ -243,7 +243,7 @@ static void return_buf_error(struct venus_inst *inst, > if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) > v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf); > else > - v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf); > + v4l2_m2m_dst_buf_remove_by_buf(m2m_ctx, vbuf); > > v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR); > } > -- regards, Stan
[PATCH 1/7 v2] media: vb2: add bidirectional flag in vb2_queue
This change is intended to give to the v4l2 drivers a choice to
change the default behavior of the v4l2-core DMA mapping direction
from DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or
OUTPUT) to DMA_BIDIRECTIONAL during queue_init time.
Initially the issue with DMA mapping direction has been found in
Venus encoder driver where the hardware (firmware side) adds few
lines padding on bottom of the image buffer, and the consequence
is triggering of IOMMU protection faults.
This will help supporting venus encoder (and probably other drivers
in the future) which wants to map output type of buffers as
read/write.
Signed-off-by: Stanimir Varbanov
---
v2: move dma_dir into private section.
drivers/media/v4l2-core/videobuf2-core.c | 17 -
include/media/videobuf2-core.h | 13 +
2 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 0924594989b4..cb115ba6a1d2 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -194,8 +194,6 @@ static void __enqueue_in_driver(struct vb2_buffer *vb);
static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
{
struct vb2_queue *q = vb->vb2_queue;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
void *mem_priv;
int plane;
int ret = -ENOMEM;
@@ -209,7 +207,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
mem_priv = call_ptr_memop(vb, alloc,
q->alloc_devs[plane] ? : q->dev,
- q->dma_attrs, size, dma_dir, q->gfp_flags);
+ q->dma_attrs, size, q->dma_dir, q->gfp_flags);
if (IS_ERR_OR_NULL(mem_priv)) {
if (mem_priv)
ret = PTR_ERR(mem_priv);
@@ -978,8 +976,6 @@ static int __prepare_userptr(struct vb2_buffer *vb, const
void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1030,7 +1026,7 @@ static int __prepare_userptr(struct vb2_buffer *vb, const
void *pb)
mem_priv = call_ptr_memop(vb, get_userptr,
q->alloc_devs[plane] ? : q->dev,
planes[plane].m.userptr,
- planes[plane].length, dma_dir);
+ planes[plane].length, q->dma_dir);
if (IS_ERR(mem_priv)) {
dprintk(1, "failed acquiring userspace memory for plane
%d\n",
plane);
@@ -1096,8 +1092,6 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, const
void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1156,7 +1150,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, const
void *pb)
/* Acquire each plane's memory */
mem_priv = call_ptr_memop(vb, attach_dmabuf,
q->alloc_devs[plane] ? : q->dev,
- dbuf, planes[plane].length, dma_dir);
+ dbuf, planes[plane].length, q->dma_dir);
if (IS_ERR(mem_priv)) {
dprintk(1, "failed to attach dmabuf\n");
ret = PTR_ERR(mem_priv);
@@ -2003,6 +1997,11 @@ int vb2_core_queue_init(struct vb2_queue *q)
if (q->buf_struct_size == 0)
q->buf_struct_size = sizeof(struct vb2_buffer);
+ if (q->bidirectional)
+ q->dma_dir = DMA_BIDIRECTIONAL;
+ else
+ q->dma_dir = q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+
return 0;
}
EXPORT_SYMBOL_GPL(vb2_core_queue_init);
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index cb97c224be73..ef9b64398c8c 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -427,6 +427,16 @@ struct vb2_buf_ops {
* @dev: device to use for the default allocation context if the driver
* doesn't fill in the @alloc_devs array.
* @dma_attrs: DMA attributes to use for the DMA.
+ * @bidirectional: when this flag is set the DMA direction for the buffers of
+ * this queue will be overridden with DMA_BIDIRECTIONAL direction.
+ * This is useful in cases where the hardware (firmware) writes to
+ * a buffer which is mapped as read (DMA_TO_
Re: [PATCH v2 1/2] docs-rst: media: Document s_stream() video op usage for MC enabled devices
Em Mon, 21 Aug 2017 09:36:49 +0300 Sakari Ailus escreveu: > Hi Mauro, > > Mauro Carvalho Chehab wrote: > > Hi Sakari, > > > > Em Wed, 16 Aug 2017 15:20:17 +0300 > > Sakari Ailus escreveu: > > > >> As we begin to add support for systems with Media controller pipelines > >> controlled by more than one device driver, it is essential that we > >> precisely define the responsibilities of each component in the stream > >> control and common practices. > >> > >> Specifically, streaming control is done per sub-device and sub-device > >> drivers themselves are responsible for streaming setup in upstream > >> sub-devices. > > > > IMO, before this patch, we need something like this: > > https://patchwork.linuxtv.org/patch/43325/ > > Thanks. I'll reply separately to that thread. > > > > >> > >> Signed-off-by: Sakari Ailus > >> Acked-by: Niklas Söderlund > >> --- > >> Documentation/media/kapi/v4l2-subdev.rst | 29 > >> + > >> 1 file changed, 29 insertions(+) > >> > >> diff --git a/Documentation/media/kapi/v4l2-subdev.rst > >> b/Documentation/media/kapi/v4l2-subdev.rst > >> index e1f0b72..45088ad 100644 > >> --- a/Documentation/media/kapi/v4l2-subdev.rst > >> +++ b/Documentation/media/kapi/v4l2-subdev.rst > >> @@ -262,6 +262,35 @@ is called. After all subdevices have been located the > >> .complete() callback is > >> called. When a subdevice is removed from the system the .unbind() method > >> is > >> called. All three callbacks are optional. > >> > >> +Streaming control on Media controller enabled devices > >> +^ > >> + > >> +Starting and stopping the stream are somewhat complex operations that > >> +often require walking the media graph to enable streaming on > >> +sub-devices which the pipeline consists of. This involves interaction > >> +between multiple drivers, sometimes more than two. > > > > That's still not ok, as it creates a black hole for devnode-based > > devices. > > > > I would change it to something like: > > > > Streaming control > > ^ > > > > Starting and stopping the stream are somewhat complex operations that > > often require to enable streaming on sub-devices which the pipeline > > consists of. This involves interaction between multiple drivers, > > sometimes > > more than two. > > > > The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is responsible > > for starting and stopping the stream on the sub-device it is called > > on. > > > > Streaming control on devnode-centric devices > > > > > > On **devnode-centric** devices, the main driver is responsible enable > > stream all all sub-devices. On most cases, all the main driver need > > to do is to broadcast s_stream to all connected sub-devices by calling > > :c:func:`v4l2_device_call_all`, e. g.:: > > > > v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 1); > > Looks good to me. > > > > > Streaming control on mc-centric devices > > ~~~ > > > > On **mc-centric** devices, it usually requires walking the media graph > > to enable streaming only at the sub-devices which the pipeline consists > > of. > > > > (place here the details for such scenario) > > This part requires a more detailed description of the problem area. What > makes a difference here is that there's a pipeline this pipeline may be > controlled more than one driver. (More elaborate discussion below.) > > > > >> +The ``.s_stream()`` op in :c:type:`v4l2_subdev_video_ops` is responsible > >> +for starting and stopping the stream on the sub-device it is called > >> +on. A device driver is only responsible for calling the ``.s_stream()`` > >> ops > >> +of the adjacent sub-devices that are connected to its sink pads > >> +through an enabled link. A driver may not call ``.s_stream()`` op > >> +of any other sub-device further up in the pipeline, for instance. > >> + > >> +This means that a sub-device driver is thus in direct control of > >> +whether the upstream sub-devices start (or stop) streaming before or > >> +after the sub-device itself is set up for streaming. > >> + > >> +.. note:: > >> + > >> + As the ``.s_stream()`` callback is called recursively through the > >> + sub-devices along the pipeline, it is important to keep the > >> + recursion as short as possible. To this end, drivers are encouraged > >> + to avoid recursively calling ``.s_stream()`` internally to reduce > >> + stack usage. Instead, the ``.s_stream()`` op of the directly > >> + connected sub-devices should come from the callback through which > >> + the driver was first called. > >> + > > > > That sounds too complex, and can lead into troubles, if the same > > sub-device driver is used on completely different devices. > > > > IMHO, it should be up to the main driver to navigate
[v4l-utils PATCH v3 2/2] v4l2-compliance: Add support for metadata output
Add support for metadata output video nodes, in other words,
V4L2_CAP_META_OUTPUT and V4L2_BUF_TYPE_META_OUTPUT.
Signed-off-by: Sakari Ailus
---
utils/v4l2-compliance/v4l2-compliance.cpp | 11 ---
utils/v4l2-compliance/v4l2-compliance.h | 2 +-
utils/v4l2-compliance/v4l2-test-formats.cpp | 8 +++-
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp
b/utils/v4l2-compliance/v4l2-compliance.cpp
index c40e3bd78..539c8c34b 100644
--- a/utils/v4l2-compliance/v4l2-compliance.cpp
+++ b/utils/v4l2-compliance/v4l2-compliance.cpp
@@ -216,6 +216,8 @@ std::string cap2s(unsigned cap)
s += "\t\tSDR Output\n";
if (cap & V4L2_CAP_META_CAPTURE)
s += "\t\tMetadata Capture\n";
+ if (cap & V4L2_CAP_META_OUTPUT)
+ s += "\t\tMetadata Output\n";
if (cap & V4L2_CAP_TOUCH)
s += "\t\tTouch Device\n";
if (cap & V4L2_CAP_TUNER)
@@ -283,6 +285,8 @@ std::string buftype2s(int type)
return "SDR Output";
case V4L2_BUF_TYPE_META_CAPTURE:
return "Metadata Capture";
+ case V4L2_BUF_TYPE_META_OUTPUT:
+ return "Metadata Output";
case V4L2_BUF_TYPE_PRIVATE:
return "Private";
default:
@@ -525,7 +529,7 @@ static int testCap(struct node *node)
const __u32 output_caps = V4L2_CAP_VIDEO_OUTPUT |
V4L2_CAP_VIDEO_OUTPUT_MPLANE |
V4L2_CAP_VIDEO_OUTPUT_OVERLAY | V4L2_CAP_VBI_OUTPUT |
V4L2_CAP_SDR_OUTPUT | V4L2_CAP_SLICED_VBI_OUTPUT |
- V4L2_CAP_MODULATOR;
+ V4L2_CAP_MODULATOR | V4L2_CAP_META_OUTPUT;
const __u32 overlay_caps = V4L2_CAP_VIDEO_OVERLAY |
V4L2_CAP_VIDEO_OUTPUT_OVERLAY;
const __u32 m2m_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE;
const __u32 io_caps = V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
@@ -1005,12 +1009,13 @@ int main(int argc, char **argv)
if (node.g_caps() & (V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_VBI_OUTPUT |
V4L2_CAP_VIDEO_OUTPUT_MPLANE |
V4L2_CAP_VIDEO_M2M_MPLANE |
V4L2_CAP_VIDEO_M2M | V4L2_CAP_SLICED_VBI_OUTPUT |
-V4L2_CAP_RDS_OUTPUT | V4L2_CAP_SDR_OUTPUT))
+V4L2_CAP_RDS_OUTPUT | V4L2_CAP_SDR_OUTPUT |
+V4L2_CAP_META_OUTPUT))
node.can_output = true;
if (node.g_caps() & (V4L2_CAP_VIDEO_CAPTURE_MPLANE |
V4L2_CAP_VIDEO_OUTPUT_MPLANE |
V4L2_CAP_VIDEO_M2M_MPLANE))
node.is_planar = true;
- if (node.g_caps() & V4L2_CAP_META_CAPTURE) {
+ if (node.g_caps() & (V4L2_CAP_META_CAPTURE | V4L2_CAP_META_OUTPUT)) {
node.is_video = false;
node.is_meta = true;
}
diff --git a/utils/v4l2-compliance/v4l2-compliance.h
b/utils/v4l2-compliance/v4l2-compliance.h
index 53ea5d5e6..e00128b57 100644
--- a/utils/v4l2-compliance/v4l2-compliance.h
+++ b/utils/v4l2-compliance/v4l2-compliance.h
@@ -61,7 +61,7 @@ typedef std::map<__u32, unsigned> frmsizes_count_map;
struct base_node;
-#define V4L2_BUF_TYPE_LAST V4L2_BUF_TYPE_META_CAPTURE
+#define V4L2_BUF_TYPE_LAST V4L2_BUF_TYPE_META_OUTPUT
struct base_node {
bool is_video;
diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp
b/utils/v4l2-compliance/v4l2-test-formats.cpp
index b7a32fe38..9da7436e8 100644
--- a/utils/v4l2-compliance/v4l2-test-formats.cpp
+++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
@@ -46,7 +46,7 @@ static const __u32 buftype2cap[] = {
V4L2_CAP_VIDEO_OUTPUT_MPLANE | V4L2_CAP_VIDEO_M2M_MPLANE,
V4L2_CAP_SDR_CAPTURE,
V4L2_CAP_SDR_OUTPUT,
- V4L2_CAP_META_CAPTURE,
+ V4L2_CAP_META_CAPTURE | V4L2_CAP_META_OUTPUT,
};
static int testEnumFrameIntervals(struct node *node, __u32 pixfmt,
@@ -298,6 +298,7 @@ int testEnumFormats(struct node *node)
case V4L2_BUF_TYPE_SDR_CAPTURE:
case V4L2_BUF_TYPE_SDR_OUTPUT:
case V4L2_BUF_TYPE_META_CAPTURE:
+ case V4L2_BUF_TYPE_META_OUTPUT:
if (ret && (node->g_caps() & buftype2cap[type]))
return fail("%s cap set, but no %s formats
defined\n",
buftype2s(type).c_str(),
buftype2s(type).c_str());
@@ -546,6 +547,7 @@ static int testFormatsType(struct node *node, int ret,
unsigned type, struct v4
fail_on_test(check_0(sdr.reserved, sizeof(sdr.reserved)));
break;
case V4L2_BUF_TYPE_META_CAPTURE:
+ case V4L2_BUF_TYPE_META_OUTPUT:
if (map.find(meta.dataformat) == map.end())
return fail("dataformat %08x (%s) for buftype %d not
reported by ENUM_FMT\n",
meta.dataformat,
fcc2s(meta.dataformat).c_str(), type);
@@
[v4l-utils PATCH v3 1/2] Add metadata output definitions from metadata output patches
Signed-off-by: Sakari Ailus
---
include/linux/videodev2.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 49fe06c97..4db47dba4 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -142,6 +142,7 @@ enum v4l2_buf_type {
V4L2_BUF_TYPE_SDR_CAPTURE = 11,
V4L2_BUF_TYPE_SDR_OUTPUT = 12,
V4L2_BUF_TYPE_META_CAPTURE = 13,
+ V4L2_BUF_TYPE_META_OUTPUT = 14,
/* Deprecated, do not use */
V4L2_BUF_TYPE_PRIVATE = 0x80,
};
@@ -453,6 +454,7 @@ struct v4l2_capability {
#define V4L2_CAP_READWRITE 0x0100 /* read/write systemcalls
*/
#define V4L2_CAP_ASYNCIO0x0200 /* async I/O */
#define V4L2_CAP_STREAMING 0x0400 /* streaming I/O ioctls */
+#define V4L2_CAP_META_OUTPUT 0x0800 /* Is a metadata output
device */
#define V4L2_CAP_TOUCH 0x1000 /* Is a touch device */
--
2.11.0
[v4l-utils PATCH v3 0/2] v4l2-compliance: Support for metadata output buffers
Hi, This set adds support for metadata output buffers in v4l2-compliance. It depends on this kernel patch: https://patchwork.linuxtv.org/patch/43308/> since v2: - Update V4L2_BUF_TYPE_LAST in v4l2-compliance.h Sakari Ailus (2): Add metadata output definitions from metadata output patches v4l2-compliance: Add support for metadata output include/linux/videodev2.h | 2 ++ utils/v4l2-compliance/v4l2-compliance.cpp | 11 --- utils/v4l2-compliance/v4l2-compliance.h | 2 +- utils/v4l2-compliance/v4l2-test-formats.cpp | 8 +++- 4 files changed, 18 insertions(+), 5 deletions(-) -- 2.11.0
Re: [v4l-utils PATCH 1/1] v4l2-compliance: Add support for metadata output
Hans Verkuil wrote: Hi Sakari, Thanks for the patch, but it is not complete. Most importantly the V4L2_BUF_TYPE_LAST define in v4l2-compliance.h isn't updated to V4L2_BUF_TYPE_META_OUTPUT. It's best to just do a 'git grep META_CAPTURE' in v4l-utils and check each place it is used whether META_OUTPUT support should also be added. That's what I did but I somehow missed META_OUTPUT in v4l2-compliance.h. I'll update the set, but I don't expect this to be merged until the kernel support for metadata output goes in with Yong's ipu3 patchset. Note for v4l2-ctl-meta.cpp: interestingly the usage help for meta formats already includes support for meta output, but no where else in v4l2-ctl is there meta output support. It appears to be unintentional that this was committed. Seems so. This set is (so far) only adding support for v4l2-compliance. -- Sakari Ailus sakari.ai...@linux.intel.com
[v4l-utils PATCH v2 1/2] Add metadata output definitions from metadata output patches
Signed-off-by: Sakari Ailus
---
include/linux/videodev2.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 49fe06c97..4db47dba4 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -142,6 +142,7 @@ enum v4l2_buf_type {
V4L2_BUF_TYPE_SDR_CAPTURE = 11,
V4L2_BUF_TYPE_SDR_OUTPUT = 12,
V4L2_BUF_TYPE_META_CAPTURE = 13,
+ V4L2_BUF_TYPE_META_OUTPUT = 14,
/* Deprecated, do not use */
V4L2_BUF_TYPE_PRIVATE = 0x80,
};
@@ -453,6 +454,7 @@ struct v4l2_capability {
#define V4L2_CAP_READWRITE 0x0100 /* read/write systemcalls
*/
#define V4L2_CAP_ASYNCIO0x0200 /* async I/O */
#define V4L2_CAP_STREAMING 0x0400 /* streaming I/O ioctls */
+#define V4L2_CAP_META_OUTPUT 0x0800 /* Is a metadata output
device */
#define V4L2_CAP_TOUCH 0x1000 /* Is a touch device */
--
2.11.0
Re: [v4l-utils PATCH 1/1] v4l2-compliance: Add support for metadata output
Hi Sakari,
Thanks for the patch, but it is not complete. Most importantly the
V4L2_BUF_TYPE_LAST
define in v4l2-compliance.h isn't updated to V4L2_BUF_TYPE_META_OUTPUT.
It's best to just do a 'git grep META_CAPTURE' in v4l-utils and check each
place it
is used whether META_OUTPUT support should also be added.
Note for v4l2-ctl-meta.cpp: interestingly the usage help for meta formats
already
includes support for meta output, but no where else in v4l2-ctl is there meta
output
support.
It appears to be unintentional that this was committed.
Regards,
Hans
On 08/21/2017 09:38 AM, Sakari Ailus wrote:
> Add support for metadata output video nodes, in other words,
> V4L2_CAP_META_OUTPUT and V4L2_BUF_TYPE_META_OUTPUT.
>
> Signed-off-by: Sakari Ailus
> ---
> Hi all,
>
> This patch adds support for metadata output in v4l2-compliance.
>
> It depends on the metadata output patch:
>
> https://patchwork.linuxtv.org/patch/43308/>
>
> include/linux/videodev2.h | 3 ++-
> utils/v4l2-compliance/v4l2-compliance.cpp | 11 ---
> utils/v4l2-compliance/v4l2-test-formats.cpp | 8 +++-
> 3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 49fe06c97..101be86c0 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -142,6 +142,7 @@ enum v4l2_buf_type {
> V4L2_BUF_TYPE_SDR_CAPTURE = 11,
> V4L2_BUF_TYPE_SDR_OUTPUT = 12,
> V4L2_BUF_TYPE_META_CAPTURE = 13,
> + V4L2_BUF_TYPE_META_OUTPUT = 14,
> /* Deprecated, do not use */
> V4L2_BUF_TYPE_PRIVATE = 0x80,
> };
> @@ -453,7 +454,7 @@ struct v4l2_capability {
> #define V4L2_CAP_READWRITE 0x0100 /* read/write
> systemcalls */
> #define V4L2_CAP_ASYNCIO0x0200 /* async I/O */
> #define V4L2_CAP_STREAMING 0x0400 /* streaming I/O ioctls
> */
> -
> +#define V4L2_CAP_META_OUTPUT 0x0800
> #define V4L2_CAP_TOUCH 0x1000 /* Is a touch device */
>
> #define V4L2_CAP_DEVICE_CAPS0x8000 /* sets device
> capabilities field */
> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp
> b/utils/v4l2-compliance/v4l2-compliance.cpp
> index c40e3bd78..539c8c34b 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> @@ -216,6 +216,8 @@ std::string cap2s(unsigned cap)
> s += "\t\tSDR Output\n";
> if (cap & V4L2_CAP_META_CAPTURE)
> s += "\t\tMetadata Capture\n";
> + if (cap & V4L2_CAP_META_OUTPUT)
> + s += "\t\tMetadata Output\n";
> if (cap & V4L2_CAP_TOUCH)
> s += "\t\tTouch Device\n";
> if (cap & V4L2_CAP_TUNER)
> @@ -283,6 +285,8 @@ std::string buftype2s(int type)
> return "SDR Output";
> case V4L2_BUF_TYPE_META_CAPTURE:
> return "Metadata Capture";
> + case V4L2_BUF_TYPE_META_OUTPUT:
> + return "Metadata Output";
> case V4L2_BUF_TYPE_PRIVATE:
> return "Private";
> default:
> @@ -525,7 +529,7 @@ static int testCap(struct node *node)
> const __u32 output_caps = V4L2_CAP_VIDEO_OUTPUT |
> V4L2_CAP_VIDEO_OUTPUT_MPLANE |
> V4L2_CAP_VIDEO_OUTPUT_OVERLAY | V4L2_CAP_VBI_OUTPUT |
> V4L2_CAP_SDR_OUTPUT | V4L2_CAP_SLICED_VBI_OUTPUT |
> - V4L2_CAP_MODULATOR;
> + V4L2_CAP_MODULATOR | V4L2_CAP_META_OUTPUT;
> const __u32 overlay_caps = V4L2_CAP_VIDEO_OVERLAY |
> V4L2_CAP_VIDEO_OUTPUT_OVERLAY;
> const __u32 m2m_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE;
> const __u32 io_caps = V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
> @@ -1005,12 +1009,13 @@ int main(int argc, char **argv)
> if (node.g_caps() & (V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_VBI_OUTPUT |
>V4L2_CAP_VIDEO_OUTPUT_MPLANE |
> V4L2_CAP_VIDEO_M2M_MPLANE |
>V4L2_CAP_VIDEO_M2M | V4L2_CAP_SLICED_VBI_OUTPUT |
> - V4L2_CAP_RDS_OUTPUT | V4L2_CAP_SDR_OUTPUT))
> + V4L2_CAP_RDS_OUTPUT | V4L2_CAP_SDR_OUTPUT |
> + V4L2_CAP_META_OUTPUT))
> node.can_output = true;
> if (node.g_caps() & (V4L2_CAP_VIDEO_CAPTURE_MPLANE |
> V4L2_CAP_VIDEO_OUTPUT_MPLANE |
>V4L2_CAP_VIDEO_M2M_MPLANE))
> node.is_planar = true;
> - if (node.g_caps() & V4L2_CAP_META_CAPTURE) {
> + if (node.g_caps() & (V4L2_CAP_META_CAPTURE | V4L2_CAP_META_OUTPUT)) {
> node.is_video = false;
> node.is_meta = true;
> }
> diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp
> b/utils/v4l2-compliance/v4l2-test-formats.cpp
> index b7a32fe38..9da7436e8 100644
> --- a/utils/v4l2-compliance/v4l2-test-formats.cpp
> +++ b/utils/v4l2-compliance/v4
[v4l-utils PATCH v2 0/2] v4l2-compliance: Support for metadata output buffers
Hi, This set adds support for metadata output buffers in v4l2-compliance. It depends on this kernel patch: https://patchwork.linuxtv.org/patch/43308/> Sakari Ailus (2): Add metadata output definitions from metadata output patches v4l2-compliance: Add support for metadata output include/linux/videodev2.h | 2 ++ utils/v4l2-compliance/v4l2-compliance.cpp | 11 --- utils/v4l2-compliance/v4l2-test-formats.cpp | 8 +++- 3 files changed, 17 insertions(+), 4 deletions(-) -- 2.11.0
[v4l-utils PATCH v2 2/2] v4l2-compliance: Add support for metadata output
Add support for metadata output video nodes, in other words,
V4L2_CAP_META_OUTPUT and V4L2_BUF_TYPE_META_OUTPUT.
Signed-off-by: Sakari Ailus
---
utils/v4l2-compliance/v4l2-compliance.cpp | 11 ---
utils/v4l2-compliance/v4l2-test-formats.cpp | 8 +++-
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp
b/utils/v4l2-compliance/v4l2-compliance.cpp
index c40e3bd78..539c8c34b 100644
--- a/utils/v4l2-compliance/v4l2-compliance.cpp
+++ b/utils/v4l2-compliance/v4l2-compliance.cpp
@@ -216,6 +216,8 @@ std::string cap2s(unsigned cap)
s += "\t\tSDR Output\n";
if (cap & V4L2_CAP_META_CAPTURE)
s += "\t\tMetadata Capture\n";
+ if (cap & V4L2_CAP_META_OUTPUT)
+ s += "\t\tMetadata Output\n";
if (cap & V4L2_CAP_TOUCH)
s += "\t\tTouch Device\n";
if (cap & V4L2_CAP_TUNER)
@@ -283,6 +285,8 @@ std::string buftype2s(int type)
return "SDR Output";
case V4L2_BUF_TYPE_META_CAPTURE:
return "Metadata Capture";
+ case V4L2_BUF_TYPE_META_OUTPUT:
+ return "Metadata Output";
case V4L2_BUF_TYPE_PRIVATE:
return "Private";
default:
@@ -525,7 +529,7 @@ static int testCap(struct node *node)
const __u32 output_caps = V4L2_CAP_VIDEO_OUTPUT |
V4L2_CAP_VIDEO_OUTPUT_MPLANE |
V4L2_CAP_VIDEO_OUTPUT_OVERLAY | V4L2_CAP_VBI_OUTPUT |
V4L2_CAP_SDR_OUTPUT | V4L2_CAP_SLICED_VBI_OUTPUT |
- V4L2_CAP_MODULATOR;
+ V4L2_CAP_MODULATOR | V4L2_CAP_META_OUTPUT;
const __u32 overlay_caps = V4L2_CAP_VIDEO_OVERLAY |
V4L2_CAP_VIDEO_OUTPUT_OVERLAY;
const __u32 m2m_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE;
const __u32 io_caps = V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
@@ -1005,12 +1009,13 @@ int main(int argc, char **argv)
if (node.g_caps() & (V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_VBI_OUTPUT |
V4L2_CAP_VIDEO_OUTPUT_MPLANE |
V4L2_CAP_VIDEO_M2M_MPLANE |
V4L2_CAP_VIDEO_M2M | V4L2_CAP_SLICED_VBI_OUTPUT |
-V4L2_CAP_RDS_OUTPUT | V4L2_CAP_SDR_OUTPUT))
+V4L2_CAP_RDS_OUTPUT | V4L2_CAP_SDR_OUTPUT |
+V4L2_CAP_META_OUTPUT))
node.can_output = true;
if (node.g_caps() & (V4L2_CAP_VIDEO_CAPTURE_MPLANE |
V4L2_CAP_VIDEO_OUTPUT_MPLANE |
V4L2_CAP_VIDEO_M2M_MPLANE))
node.is_planar = true;
- if (node.g_caps() & V4L2_CAP_META_CAPTURE) {
+ if (node.g_caps() & (V4L2_CAP_META_CAPTURE | V4L2_CAP_META_OUTPUT)) {
node.is_video = false;
node.is_meta = true;
}
diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp
b/utils/v4l2-compliance/v4l2-test-formats.cpp
index b7a32fe38..9da7436e8 100644
--- a/utils/v4l2-compliance/v4l2-test-formats.cpp
+++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
@@ -46,7 +46,7 @@ static const __u32 buftype2cap[] = {
V4L2_CAP_VIDEO_OUTPUT_MPLANE | V4L2_CAP_VIDEO_M2M_MPLANE,
V4L2_CAP_SDR_CAPTURE,
V4L2_CAP_SDR_OUTPUT,
- V4L2_CAP_META_CAPTURE,
+ V4L2_CAP_META_CAPTURE | V4L2_CAP_META_OUTPUT,
};
static int testEnumFrameIntervals(struct node *node, __u32 pixfmt,
@@ -298,6 +298,7 @@ int testEnumFormats(struct node *node)
case V4L2_BUF_TYPE_SDR_CAPTURE:
case V4L2_BUF_TYPE_SDR_OUTPUT:
case V4L2_BUF_TYPE_META_CAPTURE:
+ case V4L2_BUF_TYPE_META_OUTPUT:
if (ret && (node->g_caps() & buftype2cap[type]))
return fail("%s cap set, but no %s formats
defined\n",
buftype2s(type).c_str(),
buftype2s(type).c_str());
@@ -546,6 +547,7 @@ static int testFormatsType(struct node *node, int ret,
unsigned type, struct v4
fail_on_test(check_0(sdr.reserved, sizeof(sdr.reserved)));
break;
case V4L2_BUF_TYPE_META_CAPTURE:
+ case V4L2_BUF_TYPE_META_OUTPUT:
if (map.find(meta.dataformat) == map.end())
return fail("dataformat %08x (%s) for buftype %d not
reported by ENUM_FMT\n",
meta.dataformat,
fcc2s(meta.dataformat).c_str(), type);
@@ -585,6 +587,7 @@ int testGetFormats(struct node *node)
case V4L2_BUF_TYPE_SDR_CAPTURE:
case V4L2_BUF_TYPE_SDR_OUTPUT:
case V4L2_BUF_TYPE_META_CAPTURE:
+ case V4L2_BUF_TYPE_META_OUTPUT:
if (ret && (node->g_caps() & buftype2cap[type]))
return fail("%s cap set, but no %s formats
defined\n",
buftype2s(type).c_str(),
buftype2s(type).c_str());
@@ -641,6 +644,7 @@ sta
Re: [v4l-utils PATCH 1/1] v4l2-compliance: Add support for metadata output
Sakari Ailus wrote: > Add support for metadata output video nodes, in other words, > V4L2_CAP_META_OUTPUT and V4L2_BUF_TYPE_META_OUTPUT. > > Signed-off-by: Sakari Ailus > --- > Hi all, > > This patch adds support for metadata output in v4l2-compliance. > > It depends on the metadata output patch: > > https://patchwork.linuxtv.org/patch/43308/> Please ignore this version for now. This needs to be prepended with a proper header update in a separate patch. -- Sakari Ailus sakari.ai...@retiisi.org.uk
[GIT PULL FOR v4.14] cec/vivid fixes and enhancements
The first patch is a bug fix: the CEC adapter was not explicitly disabled when cec_delete_adapter was called. Normally this does not cause any problems, but for the upcoming omap4 cec driver it does. The next two patches add support for CEC pin emulation in the vivid driver. The third fixes a kernel logging bug in vivid. The last two patches simplify cec_allocate_adapter in two drivers by using CEC_CAP_DEFAULTS. Regards, Hans The following changes since commit 0779b8855c746c90b85bfe6e16d5dfa2a6a46655: media: ddbridge: fix semicolon.cocci warnings (2017-08-20 10:25:22 -0400) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git vivid-cec-pin for you to fetch changes up to f2fa856a538a6d9ddd58eab558b286e0f65c8e27: stm32-cec: use CEC_CAP_DEFAULTS (2017-08-21 09:35:38 +0200) Hans Verkuil (6): cec: ensure that adap_enable(false) is called from cec_delete_adapter() cec: replace pin->cur_value by adap->cec_pin_is_high vivid: add CEC pin monitoring emulation vivid: fix incorrect HDMI input/output CEC logging stih-cec: use CEC_CAP_DEFAULTS stm32-cec: use CEC_CAP_DEFAULTS drivers/media/cec/cec-adap.c | 4 +++- drivers/media/cec/cec-api.c | 6 ++ drivers/media/cec/cec-core.c | 1 + drivers/media/cec/cec-pin.c | 5 ++--- drivers/media/platform/sti/cec/stih-cec.c | 4 +--- drivers/media/platform/stm32/stm32-cec.c | 4 +--- drivers/media/platform/vivid/vivid-cec.c | 65 ++- drivers/media/platform/vivid/vivid-core.c | 8 include/media/cec-pin.h | 1 - include/media/cec.h | 1 + 10 files changed, 79 insertions(+), 20 deletions(-)
[v4l-utils PATCH 1/1] v4l2-compliance: Add support for metadata output
Add support for metadata output video nodes, in other words,
V4L2_CAP_META_OUTPUT and V4L2_BUF_TYPE_META_OUTPUT.
Signed-off-by: Sakari Ailus
---
Hi all,
This patch adds support for metadata output in v4l2-compliance.
It depends on the metadata output patch:
https://patchwork.linuxtv.org/patch/43308/>
include/linux/videodev2.h | 3 ++-
utils/v4l2-compliance/v4l2-compliance.cpp | 11 ---
utils/v4l2-compliance/v4l2-test-formats.cpp | 8 +++-
3 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 49fe06c97..101be86c0 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -142,6 +142,7 @@ enum v4l2_buf_type {
V4L2_BUF_TYPE_SDR_CAPTURE = 11,
V4L2_BUF_TYPE_SDR_OUTPUT = 12,
V4L2_BUF_TYPE_META_CAPTURE = 13,
+ V4L2_BUF_TYPE_META_OUTPUT = 14,
/* Deprecated, do not use */
V4L2_BUF_TYPE_PRIVATE = 0x80,
};
@@ -453,7 +454,7 @@ struct v4l2_capability {
#define V4L2_CAP_READWRITE 0x0100 /* read/write systemcalls
*/
#define V4L2_CAP_ASYNCIO0x0200 /* async I/O */
#define V4L2_CAP_STREAMING 0x0400 /* streaming I/O ioctls */
-
+#define V4L2_CAP_META_OUTPUT 0x0800
#define V4L2_CAP_TOUCH 0x1000 /* Is a touch device */
#define V4L2_CAP_DEVICE_CAPS0x8000 /* sets device
capabilities field */
diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp
b/utils/v4l2-compliance/v4l2-compliance.cpp
index c40e3bd78..539c8c34b 100644
--- a/utils/v4l2-compliance/v4l2-compliance.cpp
+++ b/utils/v4l2-compliance/v4l2-compliance.cpp
@@ -216,6 +216,8 @@ std::string cap2s(unsigned cap)
s += "\t\tSDR Output\n";
if (cap & V4L2_CAP_META_CAPTURE)
s += "\t\tMetadata Capture\n";
+ if (cap & V4L2_CAP_META_OUTPUT)
+ s += "\t\tMetadata Output\n";
if (cap & V4L2_CAP_TOUCH)
s += "\t\tTouch Device\n";
if (cap & V4L2_CAP_TUNER)
@@ -283,6 +285,8 @@ std::string buftype2s(int type)
return "SDR Output";
case V4L2_BUF_TYPE_META_CAPTURE:
return "Metadata Capture";
+ case V4L2_BUF_TYPE_META_OUTPUT:
+ return "Metadata Output";
case V4L2_BUF_TYPE_PRIVATE:
return "Private";
default:
@@ -525,7 +529,7 @@ static int testCap(struct node *node)
const __u32 output_caps = V4L2_CAP_VIDEO_OUTPUT |
V4L2_CAP_VIDEO_OUTPUT_MPLANE |
V4L2_CAP_VIDEO_OUTPUT_OVERLAY | V4L2_CAP_VBI_OUTPUT |
V4L2_CAP_SDR_OUTPUT | V4L2_CAP_SLICED_VBI_OUTPUT |
- V4L2_CAP_MODULATOR;
+ V4L2_CAP_MODULATOR | V4L2_CAP_META_OUTPUT;
const __u32 overlay_caps = V4L2_CAP_VIDEO_OVERLAY |
V4L2_CAP_VIDEO_OUTPUT_OVERLAY;
const __u32 m2m_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE;
const __u32 io_caps = V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
@@ -1005,12 +1009,13 @@ int main(int argc, char **argv)
if (node.g_caps() & (V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_VBI_OUTPUT |
V4L2_CAP_VIDEO_OUTPUT_MPLANE |
V4L2_CAP_VIDEO_M2M_MPLANE |
V4L2_CAP_VIDEO_M2M | V4L2_CAP_SLICED_VBI_OUTPUT |
-V4L2_CAP_RDS_OUTPUT | V4L2_CAP_SDR_OUTPUT))
+V4L2_CAP_RDS_OUTPUT | V4L2_CAP_SDR_OUTPUT |
+V4L2_CAP_META_OUTPUT))
node.can_output = true;
if (node.g_caps() & (V4L2_CAP_VIDEO_CAPTURE_MPLANE |
V4L2_CAP_VIDEO_OUTPUT_MPLANE |
V4L2_CAP_VIDEO_M2M_MPLANE))
node.is_planar = true;
- if (node.g_caps() & V4L2_CAP_META_CAPTURE) {
+ if (node.g_caps() & (V4L2_CAP_META_CAPTURE | V4L2_CAP_META_OUTPUT)) {
node.is_video = false;
node.is_meta = true;
}
diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp
b/utils/v4l2-compliance/v4l2-test-formats.cpp
index b7a32fe38..9da7436e8 100644
--- a/utils/v4l2-compliance/v4l2-test-formats.cpp
+++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
@@ -46,7 +46,7 @@ static const __u32 buftype2cap[] = {
V4L2_CAP_VIDEO_OUTPUT_MPLANE | V4L2_CAP_VIDEO_M2M_MPLANE,
V4L2_CAP_SDR_CAPTURE,
V4L2_CAP_SDR_OUTPUT,
- V4L2_CAP_META_CAPTURE,
+ V4L2_CAP_META_CAPTURE | V4L2_CAP_META_OUTPUT,
};
static int testEnumFrameIntervals(struct node *node, __u32 pixfmt,
@@ -298,6 +298,7 @@ int testEnumFormats(struct node *node)
case V4L2_BUF_TYPE_SDR_CAPTURE:
case V4L2_BUF_TYPE_SDR_OUTPUT:
case V4L2_BUF_TYPE_META_CAPTURE:
+ case V4L2_BUF_TYPE_META_OUTPUT:
if (ret && (node->g_caps() & buftype2cap[type]))
return fail("%
