Re: [PATCH 1/3] media: atmel-isc: Not support RBG format from sensor.
On 08/22/2017 03:18 AM, Yang, Wenyou wrote:
> 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.
OK, I think I see what is going on here. The isc_formats array really is two
arrays in one: up to RAW_FMT_IND_END it describes what it can receive from
the source, and after that it describes what it can convert it to.
But if you can't handle RGB formats from the sensor, then why not make
sure none of the mbus codes in isc_formats uses RGB? That makes much
more sense.
E.g.:
{ 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,
false, false },
Why use MEDIA_BUS_FMT_RGB565_2X8_LE if this apparently is not supported?
Regards,
Hans
>
>> Regards,
>>
>> Hans
>>
>>> +
>>> fmt = find_format_by_code(mbus_code.code, &i);
>>> if (!fmt)
>>> continue;
>>>
>
> Best Regards,
> Wenyou Yang
>
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 08:34:26 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-3736-g1df01c62 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 1/3] media: atmel-isc: Not support RBG format from sensor.
Hi Hans,
> -Original Message-
> From: Hans Verkuil [mailto:hverk...@xs4all.nl]
> Sent: 2017年8月22日 15:00
> To: Wenyou Yang - A41535 ; Mauro Carvalho
> Chehab
> Cc: Nicolas Ferre - M43238 ; linux-
> ker...@vger.kernel.org; Sakari Ailus ; Jonathan Corbet
> ; linux-arm-ker...@lists.infradead.org; Linux Media Mailing
> List
>
> Subject: Re: [PATCH 1/3] media: atmel-isc: Not support RBG format from sensor.
>
> On 08/22/2017 03:18 AM, Yang, Wenyou wrote:
> > 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.
>
> OK, I think I see what is going on here. The isc_formats array really is two
> arrays
> in one: up to RAW_FMT_IND_END it describes what it can receive from the
> source, and after that it describes what it can convert it to.
Not exactly.
Yes, up to RAW_FMT_IND_END, these formats must be got from the senor, they are
RAW formats.
From ISC_FMT_IND_START to ISC_FMT_IND_END, they can be generated by the ISC
controller.
It is possible they can be got from the sensor too, the driver will check it.
If it can be got from both the sensor and the ISC controller, the user can use
the "sensor_preferred" parameter to decide from which one to get.
The RBG formats are the exception.
>
> But if you can't handle RGB formats from the sensor, then why not make sure
> none of the mbus codes in isc_formats uses RGB? That makes much more sense.
>
> E.g.:
>
> { 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,
> false, false },
>
> Why use MEDIA_BUS_FMT_RGB565_2X8_LE if this apparently is not supported?
This array is also the lists of all formats supported by the ISC(including got
from the sensor).
The RGB formats are only generated by the ISC controller, not from the sensor.
>
> Regards,
>
> Hans
>
> >
> >> Regards,
> >>
> >>Hans
> >>
> >>> +
> >>> fmt = find_format_by_code(mbus_code.code, &i);
> >>> if (!fmt)
> >>> continue;
> >>>
> >
> > Best Regards,
> > Wenyou Yang
> >
Best Regards,
Wenyou Yang
Re: [PATCH 3/3] media: atmel-isc: Add more format configurations
Hi Hans,
On 2017/8/22 14:54, Hans Verkuil wrote:
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?
From ISC_FMT_IND_START to ISC_FMT_IND_END, it describes the formats
they can be got from both the sensor (possibly) and the ISC controller.
The last format (19) is in not this category. It maybe be got from the
sensor, can't be generated by the controller.
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 },
Best Regards,
Wenyou Yang
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
On Tue, 22 Aug 2017 08:43:35 +0200
Hans Verkuil wrote:
> 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.
OK. I will try to find a way to test it. If not, I will drop 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 s
[GIT PULL for 4.14] DW9714 DT support
Hi Mauro, This set adds Devicetree support for DW9714. Please pull. The following changes since commit ec0c3ec497cabbf3bfa03a9eb5edcc252190a4e0: media: ddbridge: split code into multiple files (2017-08-09 12:17:01 -0400) are available in the git repository at: ssh://linuxtv.org/git/sailus/media_tree.git dw for you to fetch changes up to cb94db0d0877db090fe342d47c8b689c4a1cec5d: dw9714: Remove ACPI match tables, convert to use probe_new (2017-08-22 09:33:09 +0300) Sakari Ailus (3): dt-bindings: Add bindings for Dongwoon DW9714 voice coil dw9714: Add Devicetree support dw9714: Remove ACPI match tables, convert to use probe_new .../bindings/media/i2c/dongwoon,dw9714.txt | 9 .../devicetree/bindings/vendor-prefixes.txt| 1 + drivers/media/i2c/dw9714.c | 26 +- 3 files changed, 21 insertions(+), 15 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt -- Sakari Ailus e-mail: sakari.ai...@iki.fi
[GIT PULL for 4.14] Fix fwnode lane-polarities property parsing
Hi Mauro, Fwnode property parings was recently broken by a smatch warning fix. Fix this. Please pull. 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: ssh://linuxtv.org/git/sailus/media_tree.git fwnode-fix for you to fetch changes up to 2a565a3828d0274ec1e911e7866d98c751370c0c: v4l: fwnode: Use a less clash-prone name for MAX_DATA_LANES macro (2017-08-22 10:57:52 +0300) Sakari Ailus (3): v4l: fwnode: Fix lane-polarities property parsing v4l: fwnode: The clock lane is the first lane in lane_polarities v4l: fwnode: Use a less clash-prone name for MAX_DATA_LANES macro drivers/media/v4l2-core/v4l2-fwnode.c | 15 ++- include/media/v4l2-fwnode.h | 6 +++--- 2 files changed, 13 insertions(+), 8 deletions(-) -- Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [PATCH v2 1/2] docs-rst: media: Document s_stream() video op usage for MC enabled devices
On Mon, 2017-08-21 at 11:01 -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
Re: [PATCH v2 1/2] dt-bindings: media: Add Cadence MIPI-CSI2 RX Device Tree bindings
Hi Laurent, Thanks a lot for reviewing those patches. On Mon, Aug 07, 2017 at 11:18:03PM +0300, Laurent Pinchart wrote: > Hi Maxime, > > Thank you for the patch. > > On Thursday 20 Jul 2017 11:23:01 Maxime Ripard wrote: > > The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up to > > 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending on > > the hardware implementation. > > > > It can operate with an external D-PHY, an internal one or no D-PHY at all > > in some configurations. > > Without any PHY ? I'm curious, how does that work ? We're currently working on an FPGA exactly with that configuration. So I guess the answer would be "it doesn't on an ASIC" :) > > Signed-off-by: Maxime Ripard > > --- > > .../devicetree/bindings/media/cdns-csi2rx.txt | 87 ++ > > 1 file changed, 87 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/cdns-csi2rx.txt > > > > diff --git a/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > > b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt new file mode > > 100644 > > index ..e08547abe885 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > > @@ -0,0 +1,87 @@ > > +Cadence MIPI-CSI2 RX controller > > +=== > > + > > +The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting up to 4 > > CSI > > +lanes in input, and 4 different pixel streams in output. > > + > > +Required properties: > > + - compatible: must be set to "cdns,csi2rx" and an SoC-specific compatible > > + - reg: base address and size of the memory mapped region > > + - clocks: phandles to the clocks driving the controller > > + - clock-names: must contain: > > +* sys_clk: main clock > > +* p_clk: register bank clock > > +* p_free_clk: free running register bank clock > > +* pixel_ifX_clk: pixel stream output clock, one for each stream > > + implemented in hardware, between 0 and 3 > > Nitpicking, I would write the name is pixel_if[0-3]_clk, it took me a few > seconds to see that the X was a placeholder. Ok. > > +* dphy_rx_clk: D-PHY byte clock, if implemented in hardware > > + - phys: phandle to the external D-PHY > > + - phy-names: must contain dphy, if the implementation uses an > > + external D-PHY > > I would move the last two properties in an optional category as they're > effectively optional. I think you should also explain a bit more clearly that > the phys property must not be present if the phy-names property is not > present. It's not really optional. The IP has a configuration register that allows you to see if it's been synthesized with or without a PHY. If the right bit is set, that property will be mandatory, if not, it's useless. Maybe it's just semantics, but to me, optional means that it can operate with or without it under any circumstances. It's not really the case here. > > + > > +Required subnodes: > > + - ports: A ports node with endpoint definitions as defined in > > + Documentation/devicetree/bindings/media/video-interfaces.txt. > > The > > + first port subnode should be the input endpoint, the second one > > the > > + outputs > > + > > + The output port should have as many endpoints as stream supported by > > + the hardware implementation, between 1 and 4, their ID being the > > + stream output number used in the implementation. > > I don't think that's correct. The IP has four independent outputs, it should > have 4 output ports for a total for 5 ports. Multiple endpoints per port > would > describe multiple connections from the same output to different sinks. Ok. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
Re: [PATCH v2 1/2] dt-bindings: media: Add Cadence MIPI-CSI2 RX Device Tree bindings
Hi Maxime, On Tuesday, 22 August 2017 11:53:20 EEST Maxime Ripard wrote: > On Mon, Aug 07, 2017 at 11:18:03PM +0300, Laurent Pinchart wrote: > > On Thursday 20 Jul 2017 11:23:01 Maxime Ripard wrote: > >> The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up > >> to 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending > >> on the hardware implementation. > >> > >> It can operate with an external D-PHY, an internal one or no D-PHY at > >> all in some configurations. > > > > Without any PHY ? I'm curious, how does that work ? > > We're currently working on an FPGA exactly with that configuration. So > I guess the answer would be "it doesn't on an ASIC" :) What's connected to the input of the CSI-2 receiver ? > >> Signed-off-by: Maxime Ripard > >> --- > >> > >> .../devicetree/bindings/media/cdns-csi2rx.txt | 87 > >> 1 file changed, 87 insertions(+) > >> create mode 100644 > >> Documentation/devicetree/bindings/media/cdns-csi2rx.txt > >> > >> diff --git a/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > >> b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt new file mode > >> 100644 > >> index ..e08547abe885 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > >> @@ -0,0 +1,87 @@ > >> +Cadence MIPI-CSI2 RX controller > >> +=== > >> + > >> +The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting up to > >> 4> CSI > >> +lanes in input, and 4 different pixel streams in output. > >> + > >> +Required properties: > >> + - compatible: must be set to "cdns,csi2rx" and an SoC-specific > >> compatible > >> + - reg: base address and size of the memory mapped region > >> + - clocks: phandles to the clocks driving the controller > >> + - clock-names: must contain: > >> +* sys_clk: main clock > >> +* p_clk: register bank clock > >> +* p_free_clk: free running register bank clock > >> +* pixel_ifX_clk: pixel stream output clock, one for each stream > >> + implemented in hardware, between 0 and 3 > > > > Nitpicking, I would write the name is pixel_if[0-3]_clk, it took me a few > > seconds to see that the X was a placeholder. > > Ok. > > >> +* dphy_rx_clk: D-PHY byte clock, if implemented in hardware > >> + - phys: phandle to the external D-PHY > >> + - phy-names: must contain dphy, if the implementation uses an > >> + external D-PHY > > > > I would move the last two properties in an optional category as they're > > effectively optional. I think you should also explain a bit more clearly > > that the phys property must not be present if the phy-names property is > > not present. > > It's not really optional. The IP has a configuration register that > allows you to see if it's been synthesized with or without a PHY. If > the right bit is set, that property will be mandatory, if not, it's > useless. Just to confirm, the PHY is a separate IP core, right ? Is the CSI-2 receiver input interface different when used with a PHY and when used without one ? Could a third-party PHY be used as well ? If so, would the PHY synthesis bit be set or not ? > Maybe it's just semantics, but to me, optional means that it can > operate with or without it under any circumstances. It's not really > the case here. It'sa semantic issue, but documenting a property as required when it can be ommitted under some circumstances seems even weirder to me :-) I understand the optional category as "can be ommitted in certain circumstances". > >> + > >> +Required subnodes: > >> + - ports: A ports node with endpoint definitions as defined in > >> + > >> Documentation/devicetree/bindings/media/video-interfaces.txt. > >> The > >> + first port subnode should be the input endpoint, the second > >> one the > >> + outputs > >> + > >> + The output port should have as many endpoints as stream supported by > >> + the hardware implementation, between 1 and 4, their ID being the > >> + stream output number used in the implementation. > > > > I don't think that's correct. The IP has four independent outputs, it > > should have 4 output ports for a total for 5 ports. Multiple endpoints > > per port would describe multiple connections from the same output to > > different sinks. > > Ok. -- Regards, Laurent Pinchart
Re: [PATCHv2 0/9] omapdrm: hdmi4: add CEC support
Hi Hans, >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 11/08/17 13:57, Tomi Valkeinen wrote: >> >>> I'm doing some testing with this series on my panda. One issue I see is >>> that when I unload the display modules, I get: >>> >>> [ 75.180206] platform 58006000.encoder: enabled after unload, idling >>> [ 75.187896] platform 58001000.dispc: enabled after unload, idling >>> [ 75.198242] platform 5800.dss: enabled after unload, idling >> >> This one is caused by hdmi_cec_adap_enable() never getting called with >> enable=false when unloading the modules. Should that be called >> explicitly in hdmi4_cec_uninit, or is the CEC framework supposed to call it? > > Nicely found! > > The cec_delete_adapter() function calls > __cec_s_phys_addr(CEC_PHYS_ADDR_INVALID) > which would normally call adap_enable(false), except when the device node was > already unregistered, in which case it just returns immediately. > > The patch below should fix this. Let me know if it works, and I'll post a > proper > patch and get that in for 4.14 (and possible backported as well, I'll have to > look at that). Thanks, this fixes the issue. I again saw "HDMICORE: omapdss HDMICORE error: operation stopped when reading edid" when I loaded the modules. My panda also froze just now when unloading the display modules, and it doesn't react to sysrq. After testing a bit without the CEC patches, I saw the above error, so I don't think it's related to your patches. I can't test with a TV, so no CEC for me... But otherwise I think the series works ok now, and looks ok. So I'll apply, but it's a bit late for the next merge window, so I'll aim for 4.15 with this. Tomi
Re: [PATCH v2 1/2] docs-rst: media: Document s_stream() video op usage for MC enabled devices
Hi Hans,
On 08/21/17 13:14, Hans Verkuil wrote:
...
> +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 at the MC
pipeline and call s_stream(), and not to the sub-drivers.
>>>
>>> I would agree with the above statement *if* we had no devices that
>>> require doing this in a different way.
>>>
>>> Consider the following case:
>>>
>>> sensor -> CSI-2 receiver -> ISP (DMA)
>>> subdev A -> subdev B -> video node
>>
>> Let me be clearer about the issue I see.
>>
>> In the above example, what subdevs are supposed to multicast the
>> s_stream() to their neighbors, and how they will know that they
>> need to multicast it.
>>
>> Let's say, that, in the first pipeline, it would be the sensor
>> and subdev A. How "sensor" and "subdev A" will know that they're
>> meant to broadcast s_stream(), and the other entities know they
>> won't?
>
> So my understanding is that the bridge driver (ISP) will call s_stream
> for the CSI-2 receiver, and that in turn calls s_stream of the sensor.
>
> This should only be done for mc-centric devices, so we need a clear
> property telling a subdev whether it is part of an mc-centric pipeline
> or a devnode-centric pipeline. Since in the latter case it should not
> call s_stream in this way. For devnode-centric pipelines the bridge
> driver broadcasts s_stream to all subdevs.
>
> For the record, I am not aware of any subdevs that are used by both
> mc and devnode-centric scenarios AND that can sit in the middle of a
> pipeline. Sensors/video receiver subdevs can certainly be used in both
> scenarios, but they don't have to propagate a s_stream call.
>
> It would be very helpful if we have a good description of these two
> scenarios in our documentation, and a capability indicating mc-centric
> behavior for devnodes. And also for v4l2-subdevs internally (i.e.
> am I used in a mc-centric scenario or not?).
>
> Then this documentation will start to make more sense as well.
>
>> Also, the same sensor may be used on a device whose CSI-2 is
>> integrated at the ISP driver (the main driver). That's why
>> I think that such logic should be started by the main driver, as
>> it is the only part of the pipeline that it is aware about
>> what it is needed. Also, as the DMA engines are controlled by
>> the main driver (via its associated video devnodes), it is the only
>> part of the pipeline that knows when a stream starts.
>
> Yes, and this driver is the one that calls s_stream on the
> adjacent subdevs. But just those and not all.
>
>>
>>> Assume that the CSI-2 receiver requires hardware setup both *before and
>>> after* streaming has been enabled on the sensor.
>>
>> calling s_stream() before and after seems to be an abuse of it.
>
> I think you misunderstand what Sakari tries to say.
>
> In the scenario above the bridge driver calls s_stream for the
> CSI receiver. That in turn has code like this:
>
> s_stream(bool enable)
> {
> ... initialize CSI ...
> if error initializing CSI
> return error
> call s_stream for adjacent source subdev (i.e. sensor)
> if success
> return 0
> ... de-initialize CSI
> return error
This isn't really about error handling: error handling can and is being
done by calling s_stream(0) on subdevs on which streaming had already
been started.
There are two purposes:
1) The knowledge whether the the upstream sub-device should start
streaming before or after is ultimately device specific. A driver for
another device may not know that (or without this information being
explicitly conveyed for which we don't have an API).
2) There may be a need
Re: [PATCH v3 1/3] omap3isp: Drop redundant isp->subdevs field and ISP_MAX_SUBDEVS
Hi Sakari,
Thank you for the patch.
On Friday, 18 August 2017 14:23:15 EEST Sakari Ailus wrote:
> struct omap3isp.subdevs field and ISP_MAX_SUBDEVS macro are both unused.
> Remove them.
>
> Signed-off-by: Sakari Ailus
The field and macro are still used, you only remove them in patch 2/3. You can
squash 1/3 and 2/3 together.
> ---
> drivers/media/platform/omap3isp/isp.h | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/isp.h
> b/drivers/media/platform/omap3isp/isp.h index e528df6efc09..848cd96b67ca
> 100644
> --- a/drivers/media/platform/omap3isp/isp.h
> +++ b/drivers/media/platform/omap3isp/isp.h
> @@ -220,9 +220,6 @@ struct isp_device {
>
> unsigned int sbl_resources;
> unsigned int subclk_resources;
> -
> -#define ISP_MAX_SUBDEVS 8
> - struct v4l2_subdev *subdevs[ISP_MAX_SUBDEVS];
> };
>
> struct isp_async_subdev {
--
Regards,
Laurent Pinchart
[PATCH v4 0/3] Unified fwnode endpoint parser
Hi folks, We have a large influx of new, unmerged, drivers that are now parsing fwnode endpoints and each one of them is doing this a little bit differently. The needs are still exactly the same for the graph data structure is device independent. This is still a non-trivial task and the majority of the driver implementations are buggy, just buggy in different ways. Facilitate parsing endpoints by adding a convenience function for parsing the endpoints, and make the omap3isp driver use it as an example. I plan to include the first and second patch to a pull request soonish, the third could go in with the first user. since v3: - Rebase on current mediatree master. since v2: - Rebase on CCP2 support patches. - Prepend a patch cleaning up omap3isp driver a little. since v1: - The first patch has been merged (it was a bugfix). - In anticipation that the parsing can take place over several iterations, take the existing number of async sub-devices into account when re-allocating an array of async sub-devices. - Rework the first patch to better anticipate parsing single endpoint at a time by a driver. - Add a second patch that adds a function for parsing endpoints one at a time based on port and endpoint numbers. Sakari Ailus (3): omap3isp: Drop redundant isp->subdevs field and ISP_MAX_SUBDEVS v4l: fwnode: Support generic parsing of graph endpoints in a device v4l: fwnode: Support generic parsing of graph endpoints in a single port drivers/media/platform/omap3isp/isp.c | 116 +++--- drivers/media/platform/omap3isp/isp.h | 3 - drivers/media/v4l2-core/v4l2-fwnode.c | 176 ++ include/media/v4l2-async.h| 4 +- include/media/v4l2-fwnode.h | 16 5 files changed, 231 insertions(+), 84 deletions(-) -- 2.11.0 *** BLURB HERE *** Sakari Ailus (3): omap3isp: Drop redundant isp->subdevs field and ISP_MAX_SUBDEVS v4l: fwnode: Support generic parsing of graph endpoints in a device v4l: fwnode: Support generic parsing of graph endpoints in a single port drivers/media/platform/omap3isp/isp.c | 115 +++--- drivers/media/platform/omap3isp/isp.h | 3 - drivers/media/v4l2-core/v4l2-fwnode.c | 176 ++ include/media/v4l2-async.h| 4 +- include/media/v4l2-fwnode.h | 16 5 files changed, 230 insertions(+), 84 deletions(-) -- 2.11.0
[PATCH v4 2/3] v4l: fwnode: Support generic parsing of graph endpoints in a device
The current practice is that drivers iterate over their endpoints and
parse each endpoint separately. This is very similar in a number of
drivers, implement a generic function for the job. Driver specific matters
can be taken into account in the driver specific callback.
Convert the omap3isp as an example.
Signed-off-by: Sakari Ailus
---
drivers/media/platform/omap3isp/isp.c | 115 ++-
drivers/media/v4l2-core/v4l2-fwnode.c | 125 ++
include/media/v4l2-async.h| 4 +-
include/media/v4l2-fwnode.h | 9 +++
4 files changed, 172 insertions(+), 81 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index 83aea08b832d..93fc3d93e602 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2011,44 +2011,41 @@ enum isp_of_phy {
ISP_OF_PHY_CSIPHY2,
};
-static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
- struct isp_async_subdev *isd)
+static int isp_fwnode_parse(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd)
{
+ struct isp_async_subdev *isd =
+ container_of(asd, struct isp_async_subdev, asd);
struct isp_bus_cfg *buscfg = &isd->bus;
- struct v4l2_fwnode_endpoint vep;
- unsigned int i;
- int ret;
bool csi1 = false;
-
- ret = v4l2_fwnode_endpoint_parse(fwnode, &vep);
- if (ret)
- return ret;
+ unsigned int i;
dev_dbg(dev, "parsing endpoint %pOF, interface %u\n",
- to_of_node(fwnode), vep.base.port);
+ to_of_node(vep->base.local_fwnode), vep->base.port);
- switch (vep.base.port) {
+ switch (vep->base.port) {
case ISP_OF_PHY_PARALLEL:
buscfg->interface = ISP_INTERFACE_PARALLEL;
buscfg->bus.parallel.data_lane_shift =
- vep.bus.parallel.data_shift;
+ vep->bus.parallel.data_shift;
buscfg->bus.parallel.clk_pol =
- !!(vep.bus.parallel.flags
+ !!(vep->bus.parallel.flags
& V4L2_MBUS_PCLK_SAMPLE_FALLING);
buscfg->bus.parallel.hs_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
+ !!(vep->bus.parallel.flags &
V4L2_MBUS_VSYNC_ACTIVE_LOW);
buscfg->bus.parallel.vs_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
+ !!(vep->bus.parallel.flags &
V4L2_MBUS_HSYNC_ACTIVE_LOW);
buscfg->bus.parallel.fld_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
+ !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
buscfg->bus.parallel.data_pol =
- !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
- buscfg->bus.parallel.bt656 = vep.bus_type == V4L2_MBUS_BT656;
+ !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
+ buscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;
break;
case ISP_OF_PHY_CSIPHY1:
case ISP_OF_PHY_CSIPHY2:
- switch (vep.bus_type) {
+ switch (vep->bus_type) {
case V4L2_MBUS_CCP2:
case V4L2_MBUS_CSI1:
dev_dbg(dev, "CSI-1/CCP-2 configuration\n");
@@ -2060,11 +2057,11 @@ static int isp_fwnode_parse(struct device *dev, struct
fwnode_handle *fwnode,
break;
default:
dev_err(dev, "unsupported bus type %u\n",
- vep.bus_type);
+ vep->bus_type);
return -EINVAL;
}
- switch (vep.base.port) {
+ switch (vep->base.port) {
case ISP_OF_PHY_CSIPHY1:
if (csi1)
buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
@@ -2080,47 +2077,47 @@ static int isp_fwnode_parse(struct device *dev, struct
fwnode_handle *fwnode,
}
if (csi1) {
buscfg->bus.ccp2.lanecfg.clk.pos =
- vep.bus.mipi_csi1.clock_lane;
+ vep->bus.mipi_csi1.clock_lane;
buscfg->bus.ccp2.lanecfg.clk.pol =
- vep.bus.mipi_csi1.lane_polarity[0];
+ vep->bus.mipi_csi1.lane_polarity[0];
dev_dbg(dev, "clock lane polarity %u, pos %u\n",
buscfg->bus.ccp2.lanecfg.clk.pol,
buscfg->bus.
[PATCH v4 1/3] omap3isp: Drop redundant isp->subdevs field and ISP_MAX_SUBDEVS
struct omap3isp.subdevs field and ISP_MAX_SUBDEVS macro are both unused.
Remove them.
Signed-off-by: Sakari Ailus
---
drivers/media/platform/omap3isp/isp.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.h
b/drivers/media/platform/omap3isp/isp.h
index e528df6efc09..848cd96b67ca 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -220,9 +220,6 @@ struct isp_device {
unsigned int sbl_resources;
unsigned int subclk_resources;
-
-#define ISP_MAX_SUBDEVS8
- struct v4l2_subdev *subdevs[ISP_MAX_SUBDEVS];
};
struct isp_async_subdev {
--
2.11.0
[PATCH v4 3/3] v4l: fwnode: Support generic parsing of graph endpoints in a single port
This is the preferred way to parse the endpoints.
Signed-off-by: Sakari Ailus
---
drivers/media/v4l2-core/v4l2-fwnode.c | 51 +++
include/media/v4l2-fwnode.h | 7 +
2 files changed, 58 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
b/drivers/media/v4l2-core/v4l2-fwnode.c
index cb0fc4b4e3bf..961bcdf22d9a 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -508,6 +508,57 @@ int v4l2_fwnode_endpoints_parse(
}
EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoints_parse);
+/**
+ * v4l2_fwnode_endpoint_parse - Parse V4L2 fwnode endpoints in a port node
+ * @dev: local struct device
+ * @notifier: async notifier related to @dev
+ * @port: port number
+ * @endpoint: endpoint number
+ * @asd_struct_size: size of the driver's async sub-device struct, including
+ * sizeof(struct v4l2_async_subdev)
+ * @parse_single: driver's callback function called on each V4L2 fwnode
endpoint
+ *
+ * Parse all V4L2 fwnode endpoints related to a given port. This is
+ * the preferred interface over v4l2_fwnode_endpoints_parse() and
+ * should be used by new drivers.
+ */
+int v4l2_fwnode_endpoint_parse_port(
+ struct device *dev, struct v4l2_async_notifier *notifier,
+ unsigned int port, unsigned int endpoint, size_t asd_struct_size,
+ int (*parse_single)(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd))
+{
+ struct fwnode_handle *fwnode;
+ struct v4l2_async_subdev *asd;
+ int ret;
+
+ fwnode = fwnode_graph_get_remote_node(dev_fwnode(dev), port, endpoint);
+ if (!fwnode)
+ return -ENOENT;
+
+ asd = devm_kzalloc(dev, asd_struct_size, GFP_KERNEL);
+ if (!asd)
+ return -ENOMEM;
+
+ ret = notifier_realloc(dev, notifier, notifier->num_subdevs + 1);
+ if (ret)
+ goto out_free;
+
+ ret = __v4l2_fwnode_endpoint_parse(dev, notifier, fwnode, asd,
+ parse_single);
+ if (ret)
+ goto out_free;
+
+ return 0;
+
+out_free:
+ devm_kfree(dev, asd);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_parse_port);
+
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Sakari Ailus ");
MODULE_AUTHOR("Sylwester Nawrocki ");
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index c75a768d4ef7..5adf28e7b070 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -131,4 +131,11 @@ int v4l2_fwnode_endpoints_parse(
struct v4l2_fwnode_endpoint *vep,
struct v4l2_async_subdev *asd));
+int v4l2_fwnode_endpoint_parse_port(
+ struct device *dev, struct v4l2_async_notifier *notifier,
+ unsigned int port, unsigned int endpoint, size_t asd_struct_size,
+ int (*parse_single)(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd));
+
#endif /* _V4L2_FWNODE_H */
--
2.11.0
Re: [PATCH v3 1/3] omap3isp: Drop redundant isp->subdevs field and ISP_MAX_SUBDEVS
On Tue, Aug 22, 2017 at 03:30:22PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Friday, 18 August 2017 14:23:15 EEST Sakari Ailus wrote:
> > struct omap3isp.subdevs field and ISP_MAX_SUBDEVS macro are both unused.
> > Remove them.
> >
> > Signed-off-by: Sakari Ailus
>
> The field and macro are still used, you only remove them in patch 2/3. You
> can
> squash 1/3 and 2/3 together.
Oh, I missed this indeed. I'll squash the patches.
>
> > ---
> > drivers/media/platform/omap3isp/isp.h | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/media/platform/omap3isp/isp.h
> > b/drivers/media/platform/omap3isp/isp.h index e528df6efc09..848cd96b67ca
> > 100644
> > --- a/drivers/media/platform/omap3isp/isp.h
> > +++ b/drivers/media/platform/omap3isp/isp.h
> > @@ -220,9 +220,6 @@ struct isp_device {
> >
> > unsigned int sbl_resources;
> > unsigned int subclk_resources;
> > -
> > -#define ISP_MAX_SUBDEVS8
> > - struct v4l2_subdev *subdevs[ISP_MAX_SUBDEVS];
> > };
> >
> > struct isp_async_subdev {
>
>
> --
> Regards,
>
> Laurent Pinchart
>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Re: [PATCH v3 2/3] v4l: fwnode: Support generic parsing of graph endpoints in a device
Hi Sakari,
Thank you for the patch.
On Friday, 18 August 2017 14:23:16 EEST Sakari Ailus wrote:
> The current practice is that drivers iterate over their endpoints and
> parse each endpoint separately. This is very similar in a number of
> drivers, implement a generic function for the job. Driver specific matters
> can be taken into account in the driver specific callback.
>
> Convert the omap3isp as an example.
It would be nice to convert at least two drivers to show that the code can
indeed be shared between multiple drivers. Even better, you could convert all
drivers.
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/platform/omap3isp/isp.c | 116 ++-
> drivers/media/v4l2-core/v4l2-fwnode.c | 125
> include/media/v4l2-async.h| 4 +-
> include/media/v4l2-fwnode.h | 9 +++
> 4 files changed, 173 insertions(+), 81 deletions(-)
[snip]
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> b/drivers/media/v4l2-core/v4l2-fwnode.c index 5cd2687310fe..cb0fc4b4e3bf
> 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -26,6 +26,7 @@
> #include
> #include
>
> +#include
> #include
>
> enum v4l2_fwnode_bus_type {
> @@ -383,6 +384,130 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link
> *link) }
> EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
>
> +static int notifier_realloc(struct device *dev,
> + struct v4l2_async_notifier *notifier,
> + unsigned int max_subdevs)
It looks like you interpret the variable as an increment. You shouldn't call
it max_subdevs in that case. I would however keep the name and pass the total
number of subdevs instead of an increment, to mimic the realloc API.
> +{
> + struct v4l2_async_subdev **subdevs;
> + unsigned int i;
> +
> + if (max_subdevs + notifier->num_subdevs <= notifier->max_subdevs)
> + return 0;
> +
> + subdevs = devm_kcalloc(
> + dev, max_subdevs + notifier->num_subdevs,
> + sizeof(*notifier->subdevs), GFP_KERNEL);
We know that we'll have to move away from devm_* allocation to fix object
lifetime management, so we could as well start now.
> + if (!subdevs)
> + return -ENOMEM;
> +
> + if (notifier->subdevs) {
> + for (i = 0; i < notifier->num_subdevs; i++)
> + subdevs[i] = notifier->subdevs[i];
Is there a reason to use a loop here instead of a memcpy() covering the whole
array ?
> + devm_kfree(dev, notifier->subdevs);
> + }
> +
> + notifier->subdevs = subdevs;
> + notifier->max_subdevs = max_subdevs + notifier->num_subdevs;
> +
> + return 0;
> +}
> +
> +static int __v4l2_fwnode_endpoint_parse(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + struct fwnode_handle *endpoint, struct v4l2_async_subdev *asd,
> + int (*parse_single)(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd))
> +{
> + struct v4l2_fwnode_endpoint *vep;
> + int ret;
> +
> + /* Ignore endpoints the parsing of which failed. */
Silently ignoring invalid DT sounds bad, I'd rather catch errors and return
with an error code to make sure that DT gets fixed.
> + vep = v4l2_fwnode_endpoint_alloc_parse(endpoint);
> + if (IS_ERR(vep))
> + return 0;
> +
> + notifier->subdevs[notifier->num_subdevs] = asd;
> +
> + ret = parse_single(dev, vep, asd);
> + v4l2_fwnode_endpoint_free(vep);
> + if (ret)
> + return ret;
> +
> + asd->match.fwnode.fwnode =
> + fwnode_graph_get_remote_port_parent(endpoint);
> + if (!asd->match.fwnode.fwnode) {
> + dev_warn(dev, "bad remote port parent\n");
> + return -EINVAL;
> + }
> +
> + asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> + notifier->num_subdevs++;
> +
> + return 0;
> +}
> +
> +/**
> + * v4l2_fwnode_endpoint_parse - Parse V4L2 fwnode endpoints in a device
> node
This doesn't match the function name.
> + * @dev: local struct device
Based on the documentation only and without a priori knowledge of the API,
local struct device is very vague.
> + * @notifier: async notifier related to @dev
Ditto. You need more documentation, especially given that this is the first
function in the core that fills a notifier from DT. You might also want to
reflect that fact in the function name.
> + * @asd_struct_size: size of the driver's async sub-device struct,
> including
> + *sizeof(struct v4l2_async_subdev)
> + * @parse_single: driver's callback function called on each V4L2 fwnode
> endpoint
The parse_single return values should be documented.
> + * Parse all V4L2 fwnode endpoints related to the device.
> + *
> + * Note that this function is intended for drivers to replace the existing
> + * implementa
Re: [PATCH v3 3/3] v4l: fwnode: Support generic parsing of graph endpoints in a single port
Hi Sakari,
Thank you for the patch.
On Friday, 18 August 2017 14:23:17 EEST Sakari Ailus wrote:
> This is the preferred way to parse the endpoints.
>
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/v4l2-core/v4l2-fwnode.c | 51
> include/media/v4l2-fwnode.h | 7 +
> 2 files changed, 58 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
> b/drivers/media/v4l2-core/v4l2-fwnode.c index cb0fc4b4e3bf..961bcdf22d9a
> 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -508,6 +508,57 @@ int v4l2_fwnode_endpoints_parse(
> }
> EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoints_parse);
>
> +/**
> + * v4l2_fwnode_endpoint_parse - Parse V4L2 fwnode endpoints in a port node
This doesn't match the function name.
> + * @dev: local struct device
> + * @notifier: async notifier related to @dev
> + * @port: port number
> + * @endpoint: endpoint number
> + * @asd_struct_size: size of the driver's async sub-device struct,
> including
> + *sizeof(struct v4l2_async_subdev)
> + * @parse_single: driver's callback function called on each V4L2 fwnode
> endpoint
> + *
> + * Parse all V4L2 fwnode endpoints related to a given port.
It doesn't, it parses a single endpoint only.
As for patch 2/3, more detailed documentation is needed.
> This is
> + * the preferred interface over v4l2_fwnode_endpoints_parse() and
> + * should be used by new drivers.
I think converting one driver as an example would make it clearer how this
function is supposed to be used.
> + */
> +int v4l2_fwnode_endpoint_parse_port(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + unsigned int port, unsigned int endpoint, size_t asd_struct_size,
> + int (*parse_single)(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd))
> +{
> + struct fwnode_handle *fwnode;
> + struct v4l2_async_subdev *asd;
> + int ret;
> +
> + fwnode = fwnode_graph_get_remote_node(dev_fwnode(dev), port, endpoint);
> + if (!fwnode)
> + return -ENOENT;
> +
> + asd = devm_kzalloc(dev, asd_struct_size, GFP_KERNEL);
> + if (!asd)
> + return -ENOMEM;
> +
> + ret = notifier_realloc(dev, notifier, notifier->num_subdevs + 1);
> + if (ret)
> + goto out_free;
> +
> + ret = __v4l2_fwnode_endpoint_parse(dev, notifier, fwnode, asd,
> +parse_single);
> + if (ret)
> + goto out_free;
> +
> + return 0;
> +
> +out_free:
> + devm_kfree(dev, asd);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_parse_port);
> +
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Sakari Ailus ");
> MODULE_AUTHOR("Sylwester Nawrocki ");
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index c75a768d4ef7..5adf28e7b070 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -131,4 +131,11 @@ int v4l2_fwnode_endpoints_parse(
> struct v4l2_fwnode_endpoint *vep,
> struct v4l2_async_subdev *asd));
>
> +int v4l2_fwnode_endpoint_parse_port(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + unsigned int port, unsigned int endpoint, size_t asd_struct_size,
> + int (*parse_single)(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd));
> +
> #endif /* _V4L2_FWNODE_H */
--
Regards,
Laurent Pinchart
[PATCH 2/4] [media] pci: constify videobuf_queue_ops structures
videobuf_queue_ops are not supposed to change at runtime. All functions
working with videobuf_queue_ops provided by work
with const videobuf_queue_ops. So mark the non-const structs as const.
Signed-off-by: Arvind Yadav
---
drivers/media/pci/bt8xx/bttv-driver.c | 2 +-
drivers/media/pci/cx18/cx18-streams.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/pci/bt8xx/bttv-driver.c
b/drivers/media/pci/bt8xx/bttv-driver.c
index ed319f1..c2f1ba0 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -1702,7 +1702,7 @@ static void buffer_release(struct videobuf_queue *q,
struct videobuf_buffer *vb)
bttv_dma_free(q,fh->btv,buf);
}
-static struct videobuf_queue_ops bttv_video_qops = {
+static const struct videobuf_queue_ops bttv_video_qops = {
.buf_setup= buffer_setup,
.buf_prepare = buffer_prepare,
.buf_queue= buffer_queue,
diff --git a/drivers/media/pci/cx18/cx18-streams.c
b/drivers/media/pci/cx18/cx18-streams.c
index 3c45e007..81d06c1 100644
--- a/drivers/media/pci/cx18/cx18-streams.c
+++ b/drivers/media/pci/cx18/cx18-streams.c
@@ -240,7 +240,7 @@ static void buffer_queue(struct videobuf_queue *q, struct
videobuf_buffer *vb)
list_add_tail(&buf->vb.queue, &s->vb_capture);
}
-static struct videobuf_queue_ops cx18_videobuf_qops = {
+static const struct videobuf_queue_ops cx18_videobuf_qops = {
.buf_setup= buffer_setup,
.buf_prepare = buffer_prepare,
.buf_queue= buffer_queue,
--
1.9.1
[PATCH 3/4] [media] platform: constify videobuf_queue_ops structures
videobuf_queue_ops are not supposed to change at runtime. All functions
working with videobuf_queue_ops provided by work
with const videobuf_queue_ops. So mark the non-const structs as const.
Signed-off-by: Arvind Yadav
---
drivers/media/platform/davinci/vpfe_capture.c | 2 +-
drivers/media/platform/fsl-viu.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/davinci/vpfe_capture.c
b/drivers/media/platform/davinci/vpfe_capture.c
index b1bf4a7..6792da1 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1288,7 +1288,7 @@ static void vpfe_videobuf_release(struct videobuf_queue
*vq,
vb->state = VIDEOBUF_NEEDS_INIT;
}
-static struct videobuf_queue_ops vpfe_videobuf_qops = {
+static const struct videobuf_queue_ops vpfe_videobuf_qops = {
.buf_setup = vpfe_videobuf_setup,
.buf_prepare= vpfe_videobuf_prepare,
.buf_queue = vpfe_videobuf_queue,
diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c
index 97e164b..2aac8e8 100644
--- a/drivers/media/platform/fsl-viu.c
+++ b/drivers/media/platform/fsl-viu.c
@@ -549,7 +549,7 @@ static void buffer_release(struct videobuf_queue *vq,
free_buffer(vq, buf);
}
-static struct videobuf_queue_ops viu_video_qops = {
+static const struct videobuf_queue_ops viu_video_qops = {
.buf_setup = buffer_setup,
.buf_prepare= buffer_prepare,
.buf_queue = buffer_queue,
--
1.9.1
[PATCH 4/4] [media] usb: constify videobuf_queue_ops structures
videobuf_queue_ops are not supposed to change at runtime. All functions
working with videobuf_queue_ops provided by work
with const videobuf_queue_ops. So mark the non-const structs as const.
Signed-off-by: Arvind Yadav
---
drivers/media/usb/cx231xx/cx231xx-417.c | 2 +-
drivers/media/usb/cx231xx/cx231xx-video.c | 2 +-
drivers/media/usb/tm6000/tm6000-video.c | 2 +-
drivers/media/usb/zr364xx/zr364xx.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c
b/drivers/media/usb/cx231xx/cx231xx-417.c
index 509d971..2b16808 100644
--- a/drivers/media/usb/cx231xx/cx231xx-417.c
+++ b/drivers/media/usb/cx231xx/cx231xx-417.c
@@ -1490,7 +1490,7 @@ static void bb_buf_release(struct videobuf_queue *q,
free_buffer(q, buf);
}
-static struct videobuf_queue_ops cx231xx_qops = {
+static const struct videobuf_queue_ops cx231xx_qops = {
.buf_setup= bb_buf_setup,
.buf_prepare = bb_buf_prepare,
.buf_queue= bb_buf_queue,
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c
b/drivers/media/usb/cx231xx/cx231xx-video.c
index f67f868..179b848 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -859,7 +859,7 @@ static void buffer_release(struct videobuf_queue *vq,
free_buffer(vq, buf);
}
-static struct videobuf_queue_ops cx231xx_video_qops = {
+static const struct videobuf_queue_ops cx231xx_video_qops = {
.buf_setup = buffer_setup,
.buf_prepare = buffer_prepare,
.buf_queue = buffer_queue,
diff --git a/drivers/media/usb/tm6000/tm6000-video.c
b/drivers/media/usb/tm6000/tm6000-video.c
index 7e960d0..35925f1 100644
--- a/drivers/media/usb/tm6000/tm6000-video.c
+++ b/drivers/media/usb/tm6000/tm6000-video.c
@@ -801,7 +801,7 @@ static void buffer_release(struct videobuf_queue *vq,
struct videobuf_buffer *vb
free_buffer(vq, buf);
}
-static struct videobuf_queue_ops tm6000_video_qops = {
+static const struct videobuf_queue_ops tm6000_video_qops = {
.buf_setup = buffer_setup,
.buf_prepare= buffer_prepare,
.buf_queue = buffer_queue,
diff --git a/drivers/media/usb/zr364xx/zr364xx.c
b/drivers/media/usb/zr364xx/zr364xx.c
index efdcd5b..24d5860 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -439,7 +439,7 @@ static void buffer_release(struct videobuf_queue *vq,
free_buffer(vq, buf);
}
-static struct videobuf_queue_ops zr364xx_video_qops = {
+static const struct videobuf_queue_ops zr364xx_video_qops = {
.buf_setup = buffer_setup,
.buf_prepare = buffer_prepare,
.buf_queue = buffer_queue,
--
1.9.1
[PATCH 0/4] constify videobuf_queue_ops structures
videobuf_queue_ops are not supposed to change at runtime. All functions working with videobuf_queue_ops provided by work with const videobuf_queue_ops. So mark the non-const structs as const. Arvind Yadav (4): [PATCH 1/4] [media] saa7146: constify videobuf_queue_ops structures [PATCH 2/4] [media] pci: constify videobuf_queue_ops structures [PATCH 3/4] [media] platform: constify videobuf_queue_ops structures [PATCH 4/4] [media] usb: constify videobuf_queue_ops structures drivers/media/common/saa7146/saa7146_vbi.c| 2 +- drivers/media/common/saa7146/saa7146_video.c | 2 +- drivers/media/pci/bt8xx/bttv-driver.c | 2 +- drivers/media/pci/cx18/cx18-streams.c | 2 +- drivers/media/platform/davinci/vpfe_capture.c | 2 +- drivers/media/platform/fsl-viu.c | 2 +- drivers/media/usb/cx231xx/cx231xx-417.c | 2 +- drivers/media/usb/cx231xx/cx231xx-video.c | 2 +- drivers/media/usb/tm6000/tm6000-video.c | 2 +- drivers/media/usb/zr364xx/zr364xx.c | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) -- 1.9.1
[PATCH 1/4] [media] saa7146: constify videobuf_queue_ops structures
videobuf_queue_ops are not supposed to change at runtime. All functions
working with videobuf_queue_ops provided by work
with const videobuf_queue_ops. So mark the non-const structs as const.
Signed-off-by: Arvind Yadav
---
drivers/media/common/saa7146/saa7146_vbi.c | 2 +-
drivers/media/common/saa7146/saa7146_video.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/common/saa7146/saa7146_vbi.c
b/drivers/media/common/saa7146/saa7146_vbi.c
index 3553ac4..d79e4d7 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -308,7 +308,7 @@ static void buffer_release(struct videobuf_queue *q, struct
videobuf_buffer *vb)
saa7146_dma_free(dev,q,buf);
}
-static struct videobuf_queue_ops vbi_qops = {
+static const struct videobuf_queue_ops vbi_qops = {
.buf_setup= buffer_setup,
.buf_prepare = buffer_prepare,
.buf_queue= buffer_queue,
diff --git a/drivers/media/common/saa7146/saa7146_video.c
b/drivers/media/common/saa7146/saa7146_video.c
index b3b29d4..37b4654 100644
--- a/drivers/media/common/saa7146/saa7146_video.c
+++ b/drivers/media/common/saa7146/saa7146_video.c
@@ -1187,7 +1187,7 @@ static void buffer_release(struct videobuf_queue *q,
struct videobuf_buffer *vb)
release_all_pagetables(dev, buf);
}
-static struct videobuf_queue_ops video_qops = {
+static const struct videobuf_queue_ops video_qops = {
.buf_setup= buffer_setup,
.buf_prepare = buffer_prepare,
.buf_queue= buffer_queue,
--
1.9.1
Re: [PATCH 15/15] usb: make device_type const
On Sat, Aug 19, 2017 at 01:52:26PM +0530, Bhumika Goyal wrote:
> Make this const as it is only stored in the type field of a device
> structure, which is const.
> Done using Coccinelle.
>
> Signed-off-by: Bhumika Goyal
Acked-by: Heikki Krogerus
> ---
> drivers/usb/common/ulpi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> index 930e8f3..4aa5195 100644
> --- a/drivers/usb/common/ulpi.c
> +++ b/drivers/usb/common/ulpi.c
> @@ -135,7 +135,7 @@ static void ulpi_dev_release(struct device *dev)
> kfree(to_ulpi_dev(dev));
> }
>
> -static struct device_type ulpi_dev_type = {
> +static const struct device_type ulpi_dev_type = {
> .name = "ulpi_device",
> .groups = ulpi_dev_attr_groups,
> .release = ulpi_dev_release,
Thanks,
--
heikki
Re: [PATCH] [media] ddbridge: add IOCTLs
Daniel Scheller writes: > Am Sun, 20 Aug 2017 08:53:56 -0300 > schrieb Mauro Carvalho Chehab : > > > Em Sun, 20 Aug 2017 13:08:55 +0200 > > Daniel Scheller escreveu: > > > > > From: Daniel Scheller > > > > > > This patch adds back the IOCTL API/functionality which is present > > > in the upstream dddvb driver package. In comparison, the IOCTL > > > handler has been factored to a separate object (and with that, some > > > functionality from -core has been moved there aswell), the IOCTLs > > > are defined in an include in the uAPI, and ioctl-number.txt is > > > updated to document that there are IOCTLs present in this driver. > > > > > > Signed-off-by: Daniel Scheller > > > --- > > > This patch depends on the ddbridge-0.9.29 bump, see [1]. The > > > functionality was part of the driver before. > > > > > > [1] http://www.spinics.net/lists/linux-media/msg119911.html > > > > > > Documentation/ioctl/ioctl-number.txt| 1 + > > > MAINTAINERS | 1 + > > > drivers/media/pci/ddbridge/Makefile | 2 +- > > > drivers/media/pci/ddbridge/ddbridge-core.c | 111 + > > > drivers/media/pci/ddbridge/ddbridge-ioctl.c | 334 > > > > > > drivers/media/pci/ddbridge/ddbridge-ioctl.h | 32 +++ > > > include/uapi/linux/ddbridge-ioctl.h | 110 + 7 files > > > changed, 481 insertions(+), 110 deletions(-) create mode 100644 > > > drivers/media/pci/ddbridge/ddbridge-ioctl.c create mode 100644 > > > drivers/media/pci/ddbridge/ddbridge-ioctl.h create mode 100644 > > > include/uapi/linux/ddbridge-ioctl.h > > > > > > diff --git a/Documentation/ioctl/ioctl-number.txt > > > b/Documentation/ioctl/ioctl-number.txt index > > > 3e3fdae5f3ed..d78d1cd092d2 100644 --- > > > a/Documentation/ioctl/ioctl-number.txt +++ > > > b/Documentation/ioctl/ioctl-number.txt @@ -215,6 +215,7 @@ Code > > > Seq#(hex)Include FileComments 'c' > > > A0-AF arch/x86/include/asm/msr.h conflict! 'd' > > > 00-FFlinux/char/drm/drm.hconflict! 'd' > > > 02-40pcmcia/ds.h conflict! +'d' > > > 00-0Blinux/ddbridge-ioctl.h conflict! > > > > That's where the problem with this patch starts: we don't add > > conflicts here :-) > > > > We need more discussions with regards to the features added by this > > patchset. > > Understood. The "good" thing is that this isn't a requirement to drive > any tuner boards (at the moment), however we shouldn't lose track on > this. Since this is the only complaint for now: > > - We need to clear with Ralph if changing the MAGIC to something > different is an option. In the end, if we change the userspace apps > to include the uAPI header from mainline if available (else fallback > to what ie. dddvb carries), I don't see an issue with this. But if > userspace apps keep on using private stuff, this will break ofc. > - Other option: Fork dddvb and change userspace apps accordingly, and > keep them in sync with upstream. Since we already have to care about > the kernel part, this option is rather suboptimal. > > Ralph, Ping :-) Changing to something different from 'd' should be fine. Is there anything still free?
[PATCH] [media] em28xx: calculate left volume level correctly
From: Colin Ian King
The calculation of the left volume looks suspect, the value of
0x1f - ((val << 8) & 0x1f) is always 0x1f. The debug prior to the
assignemnt of value[1] prints the left volume setting using the
calculation 0x1f - (val >> 8) & 0x1f which looks correct to me.
Fix the left volume by using the correct expression as used in
the debug.
Detected by CoverityScan, CID#146140 ("Wrong operator used")
Fixes: 850d24a5a861 ("[media] em28xx-alsa: add mixer support for AC97 volume
controls")
Signed-off-by: Colin Ian King
---
drivers/media/usb/em28xx/em28xx-audio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/em28xx/em28xx-audio.c
b/drivers/media/usb/em28xx/em28xx-audio.c
index 261620a57420..4628d73f46f2 100644
--- a/drivers/media/usb/em28xx/em28xx-audio.c
+++ b/drivers/media/usb/em28xx/em28xx-audio.c
@@ -564,7 +564,7 @@ static int em28xx_vol_get(struct snd_kcontrol *kcontrol,
val, (int)kcontrol->private_value);
value->value.integer.value[0] = 0x1f - (val & 0x1f);
- value->value.integer.value[1] = 0x1f - ((val << 8) & 0x1f);
+ value->value.integer.value[1] = 0x1f - ((val >> 8) & 0x1f);
return 0;
}
--
2.14.1
[PATCH v2 0/4] STM32 DCMI camera interface crop support
This patchset implements crop feature of Digital Camera Memory Interface (DCMI) of STMicroelectronics STM32 SoC series, allowing user to crop at pixel level inside sensor captured frame. This patchset follows discussions initiated from a first submission of DCMI crop support, see [1]. First part of patches brings few fixes and cleanup in DCMI driver to prepare support of crop through g_/s_selection interface in latest commit. This has been tested on STM32F746G-DISCO + STM32F4DIS-CAM extension running OV9655 sensor, using a modified version of yavta [2] utility to support crop through S_SELECTION(V4L2_SEL_TGT_CROP) ioctl: yavta -s 480x272 -n 1 --capture=2 /dev/video0 --crop=0,0,480,272 v4l2-compliance cropping test is failed due to OV9655 sensor supporting several discrete frame sizes and crop support added by DCMI interface [3]: fail: v4l2-test-formats.cpp(1266): node->frmsizes_count[pixfmt] > 1 test Cropping: FAIL If sensor is restricted to only a single supported resolution, test is OK. Compliance test should be adapted to support this case. [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg114652.html [2] http://git.ideasonboard.org/?p=yavta.git;a=summary [3] see v4l2-compliance test report below === = history = === version 2: - Fix remarks from Hans Verkuil on "cleanup" commit: http://www.mail-archive.com/linux-media@vger.kernel.org/msg116612.html - change "nb_of" to "num_of" - Drop "default format at open" commit as per Hans' remark: http://www.mail-archive.com/linux-media@vger.kernel.org/msg116614.html - Fix Kbuild warning on uninitialized ret variable: http://www.mail-archive.com/linux-media@vger.kernel.org/msg116398.html - Fix remarks from Hans Verkuil on crop commit: http://www.mail-archive.com/linux-media@vger.kernel.org/msg116615.html - refactor g_selection() code - revisit dcmi->do_crop true/false critera version 1: - Initial version === = v4l2-compliance = === Below is the v4l2-compliance report for this current version of the DCMI camera interface. v4l2-compliance has been built from v4l-utils-1.12.5. ~ # v4l2-compliance -s -f -d /dev/video0 v4l2-compliance SHA : f5f45e17ee98a0ebad7836ade2b34ceec909d751 Driver Info: Driver name : stm32-dcmi Card type : STM32 Camera Memory Interface Bus info : platform:dcmi Driver version: 4.12.0 Capabilities : 0x8521 Video Capture Read/Write Streaming Extended Pix Format Device Capabilities Device Caps : 0x0521 Video Capture Read/Write Streaming Extended Pix Format Compliance test for device /dev/video0 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Test input 0: Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 2 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) fail: v4l2-test-formats.cpp(1266): node->frms
[PATCH v2 4/4] [media] stm32-dcmi: g_/s_selection crop support
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 | 374 +-
1 file changed, 370 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 7713c10..35ba6f2 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 intnum_of_sd_formats;
const struct dcmi_format*sd_format;
+ struct dcmi_framesize *sd_framesizes;
+ unsigned intnum_of_sd_framesizes;
+ 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->num_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 dcmi_framesize *sd_framesize)
{
const struct dcmi_format *sd_fmt;
+ struct dcmi_framesize sd_fsize;
struct v4l2_pix_format *pix = &f->fmt.pix;
struct v4l2_subdev_pad_config pad_cfg;
struct v4l2_subdev_format format = {
@@ -718,6 +783,17 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct
v4l2_format *f,
pix->width = clamp(pix->width, MIN_WIDTH, MAX_WIDTH);
[PATCH v2 3/4] [media] stm32-dcmi: cleanup variable/fields namings
Uniformize "pixfmt" variables to "pix".
Change "current_fmt" & "dcmi_fmt" variables to variables
with "sd_" prefix to explicitly refer to subdev format.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 103 --
1 file changed, 54 insertions(+), 49 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 1fe2d0f..7713c10 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -132,9 +132,9 @@ struct stm32_dcmi {
struct dcmi_graph_entityentity;
struct v4l2_format fmt;
- const struct dcmi_format**user_formats;
- unsigned intnum_user_formats;
- const struct dcmi_format*current_fmt;
+ const struct dcmi_format**sd_formats;
+ unsigned intnum_of_sd_formats;
+ const struct dcmi_format*sd_format;
/* Protect this data structure */
struct mutexlock;
@@ -684,12 +684,12 @@ static int dcmi_g_fmt_vid_cap(struct file *file, void
*priv,
static const struct dcmi_format *find_format_by_fourcc(struct stm32_dcmi *dcmi,
unsigned int fourcc)
{
- unsigned int num_formats = dcmi->num_user_formats;
+ unsigned int num_formats = dcmi->num_of_sd_formats;
const struct dcmi_format *fmt;
unsigned int i;
for (i = 0; i < num_formats; i++) {
- fmt = dcmi->user_formats[i];
+ fmt = dcmi->sd_formats[i];
if (fmt->fourcc == fourcc)
return fmt;
}
@@ -698,40 +698,42 @@ static const struct dcmi_format
*find_format_by_fourcc(struct stm32_dcmi *dcmi,
}
static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
- const struct dcmi_format **current_fmt)
+ const struct dcmi_format **sd_format)
{
- const struct dcmi_format *dcmi_fmt;
- struct v4l2_pix_format *pixfmt = &f->fmt.pix;
+ const struct dcmi_format *sd_fmt;
+ struct v4l2_pix_format *pix = &f->fmt.pix;
struct v4l2_subdev_pad_config pad_cfg;
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_TRY,
};
int ret;
- dcmi_fmt = find_format_by_fourcc(dcmi, pixfmt->pixelformat);
- if (!dcmi_fmt) {
- dcmi_fmt = dcmi->user_formats[dcmi->num_user_formats - 1];
- pixfmt->pixelformat = dcmi_fmt->fourcc;
+ sd_fmt = find_format_by_fourcc(dcmi, pix->pixelformat);
+ if (!sd_fmt) {
+ sd_fmt = dcmi->sd_formats[dcmi->num_of_sd_formats - 1];
+ pix->pixelformat = sd_fmt->fourcc;
}
/* Limit to hardware capabilities */
- pixfmt->width = clamp(pixfmt->width, MIN_WIDTH, MAX_WIDTH);
- pixfmt->height = clamp(pixfmt->height, MIN_HEIGHT, MAX_HEIGHT);
+ pix->width = clamp(pix->width, MIN_WIDTH, MAX_WIDTH);
+ pix->height = clamp(pix->height, MIN_HEIGHT, MAX_HEIGHT);
- v4l2_fill_mbus_format(&format.format, pixfmt, dcmi_fmt->mbus_code);
+ v4l2_fill_mbus_format(&format.format, pix, sd_fmt->mbus_code);
ret = v4l2_subdev_call(dcmi->entity.subdev, pad, set_fmt,
&pad_cfg, &format);
if (ret < 0)
return ret;
- v4l2_fill_pix_format(pixfmt, &format.format);
+ /* Update pix regarding to what sensor can do */
+ v4l2_fill_pix_format(pix, &format.format);
- pixfmt->field = V4L2_FIELD_NONE;
- pixfmt->bytesperline = pixfmt->width * dcmi_fmt->bpp;
- pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height;
- if (current_fmt)
- *current_fmt = dcmi_fmt;
+ pix->field = V4L2_FIELD_NONE;
+ pix->bytesperline = pix->width * sd_fmt->bpp;
+ pix->sizeimage = pix->bytesperline * pix->height;
+
+ if (sd_format)
+ *sd_format = sd_fmt;
return 0;
}
@@ -741,22 +743,25 @@ static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct
v4l2_format *f)
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
};
- const struct dcmi_format *current_fmt;
+ const struct dcmi_format *sd_format;
+ struct v4l2_mbus_framefmt *mf = &format.format;
+ struct v4l2_pix_format *pix = &f->fmt.pix;
int ret;
- ret = dcmi_try_fmt(dcmi, f, ¤t_fmt);
+ ret = dcmi_try_fmt(dcmi, f, &sd_format);
if (ret)
return ret;
- v4l2_fill_mbus_format(&format.format, &f->fmt.pix,
- current_fmt->mbus_code);
+ /* pix to mbus format */
+ v4l2_fill_mbus_format(mf, pix,
+ sd_format->mbus_code);
ret = v4l2_subdev_call(dcmi->entity.subdev, pad,
[PATCH v2 1/4] [media] stm32-dcmi: catch dma submission error
Test cookie return by dmaengine_submit() and return error if any.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index c2168b5..7ffb2d3 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -295,6 +295,10 @@ static int dcmi_start_dma(struct stm32_dcmi *dcmi,
/* Push current DMA transaction in the pending queue */
dcmi->dma_cookie = dmaengine_submit(desc);
+ if (dma_submit_error(dcmi->dma_cookie)) {
+ dev_err(dcmi->dev, "%s: DMA submission failed\n", __func__);
+ return -ENXIO;
+ }
dma_async_issue_pending(dcmi->dma_chan);
--
1.9.1
[PATCH v2 2/4] [media] stm32-dcmi: revisit control register handling
Simplify bits handling of DCMI_CR register.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 7ffb2d3..1fe2d0f 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -490,7 +490,7 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
unsigned int count)
{
struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
struct dcmi_buf *buf, *node;
- u32 val;
+ u32 val = 0;
int ret;
ret = clk_enable(dcmi->mclk);
@@ -510,22 +510,16 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
unsigned int count)
spin_lock_irq(&dcmi->irqlock);
- val = reg_read(dcmi->regs, DCMI_CR);
-
- val &= ~(CR_PCKPOL | CR_HSPOL | CR_VSPOL |
-CR_EDM_0 | CR_EDM_1 | CR_FCRC_0 |
-CR_FCRC_1 | CR_JPEG | CR_ESS);
-
/* Set bus width */
switch (dcmi->bus.bus_width) {
case 14:
- val &= CR_EDM_0 + CR_EDM_1;
+ val |= CR_EDM_0 | CR_EDM_1;
break;
case 12:
- val &= CR_EDM_1;
+ val |= CR_EDM_1;
break;
case 10:
- val &= CR_EDM_0;
+ val |= CR_EDM_0;
break;
default:
/* Set bus width to 8 bits by default */
--
1.9.1
Re: [PATCH v2] device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()
On Mon, Aug 21, 2017 at 7:19 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 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(-)
Isn't this already fixed with this fix:
commit c0a480d1acf7dc184f9f3e7cf724483b0d28dc2e
Author: Tony Lindgren
Date: Fri Jul 28 01:23:15 2017 -0700
device property: Fix usecount for of_graph_get_port_parent()
Re: [PATCH v2] device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()
Hi Rob,
On Tue, Aug 22, 2017 at 4:49 PM, Rob Herring wrote:
> On Mon, Aug 21, 2017 at 7:19 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 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(-)
>
> Isn't this already fixed with this fix:
>
> commit c0a480d1acf7dc184f9f3e7cf724483b0d28dc2e
> Author: Tony Lindgren
> Date: Fri Jul 28 01:23:15 2017 -0700
>
> device property: Fix usecount for of_graph_get_port_parent()
No, this one is for of_fwnode_graph_get_port_parent().
You're letting too many similarly-named new functions through ;-)
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 v2] device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()
Hi Rob,
On 2017-08-22 09:49:35 -0500, Rob Herring wrote:
> On Mon, Aug 21, 2017 at 7:19 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 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(-)
>
> Isn't this already fixed with this fix:
>
> commit c0a480d1acf7dc184f9f3e7cf724483b0d28dc2e
> Author: Tony Lindgren
> Date: Fri Jul 28 01:23:15 2017 -0700
>
> device property: Fix usecount for of_graph_get_port_parent()
No, that commit fixes it for of_graph_get_port_parent() while this
commit fixes it for of_fwnode_graph_get_port_parent(). But in essence it
is the same issue but needs two separate fixes.
--
Regards,
Niklas Söderlund
Re: [PATCH] [media] ddbridge: add IOCTLs
Am Tue, 22 Aug 2017 16:05:21 +0200 schrieb Ralph Metzler : > Daniel Scheller writes: > > Am Sun, 20 Aug 2017 08:53:56 -0300 > > schrieb Mauro Carvalho Chehab : > > > > > Em Sun, 20 Aug 2017 13:08:55 +0200 > > > Daniel Scheller escreveu: > > > > > > > From: Daniel Scheller > > > > > > > > This patch adds back the IOCTL API/functionality which is > > > > present in the upstream dddvb driver package. In comparison, > > > > the IOCTL handler has been factored to a separate object (and > > > > with that, some functionality from -core has been moved there > > > > aswell), the IOCTLs are defined in an include in the uAPI, and > > > > ioctl-number.txt is updated to document that there are IOCTLs > > > > present in this driver. > > > > > > > > Signed-off-by: Daniel Scheller > > > > --- > > > > This patch depends on the ddbridge-0.9.29 bump, see [1]. The > > > > functionality was part of the driver before. > > > > > > > > [1] http://www.spinics.net/lists/linux-media/msg119911.html > > > > > > > > Documentation/ioctl/ioctl-number.txt| 1 + > > > > MAINTAINERS | 1 + > > > > drivers/media/pci/ddbridge/Makefile | 2 +- > > > > drivers/media/pci/ddbridge/ddbridge-core.c | 111 + > > > > drivers/media/pci/ddbridge/ddbridge-ioctl.c | 334 > > > > > > > > drivers/media/pci/ddbridge/ddbridge-ioctl.h | 32 +++ > > > > include/uapi/linux/ddbridge-ioctl.h | 110 + 7 > > > > files changed, 481 insertions(+), 110 deletions(-) create mode > > > > 100644 drivers/media/pci/ddbridge/ddbridge-ioctl.c create mode > > > > 100644 drivers/media/pci/ddbridge/ddbridge-ioctl.h create mode > > > > 100644 include/uapi/linux/ddbridge-ioctl.h > > > > > > > > diff --git a/Documentation/ioctl/ioctl-number.txt > > > > b/Documentation/ioctl/ioctl-number.txt index > > > > 3e3fdae5f3ed..d78d1cd092d2 100644 --- > > > > a/Documentation/ioctl/ioctl-number.txt +++ > > > > b/Documentation/ioctl/ioctl-number.txt @@ -215,6 +215,7 @@ Code > > > > Seq#(hex) Include FileComments 'c' > > > > A0-AF arch/x86/include/asm/msr.h conflict! 'd' > > > > 00-FF linux/char/drm/drm.hconflict! 'd' > > > > 02-40 pcmcia/ds.h conflict! +'d' > > > > 00-0B linux/ddbridge-ioctl.h conflict! > > > > > > That's where the problem with this patch starts: we don't add > > > conflicts here :-) > > > > > > We need more discussions with regards to the features added by > > > this patchset. > > > > Understood. The "good" thing is that this isn't a requirement to > > drive any tuner boards (at the moment), however we shouldn't lose > > track on this. Since this is the only complaint for now: > > > > - We need to clear with Ralph if changing the MAGIC to something > > different is an option. In the end, if we change the userspace > > apps to include the uAPI header from mainline if available (else > > fallback to what ie. dddvb carries), I don't see an issue with > > this. But if userspace apps keep on using private stuff, this will > > break ofc. > > - Other option: Fork dddvb and change userspace apps accordingly, > > and keep them in sync with upstream. Since we already have to care > > about the kernel part, this option is rather suboptimal. > > > > Ralph, Ping :-) > > Changing to something different from 'd' should be fine. > Is there anything still free? We could use 0xDD (for *D*igital *D*evices :-) ), subrange 0xc0-0xff or so, that isn't declared "in use" in ioctl-number.txt (0xDD/0x00-0x3f is used by some ZFCP driver). Other options would be ie. 0xD0-0xDA, 0xDC, 0xDE-0xE4. There are also other single values and value-ranges free, all according to ioctl-number.txt ofcourse. Still yet not sure if this is everything that needs changing to be accepted though. Best regards, Daniel Scheller -- https://github.com/herrnst
Possible memory leak in cafe_ccic.ko
Hello. While searching for races in the Linux kernel I've come across "drivers/media/platform/marvell-ccic/cafe_ccic.ko" module. Here are questions that I came up with while analyzing results. Lines are given using the info from Linux v4.12. Consider the following case: Thread 1: Thread 2: mcam_v4l_release ->mcam_free_dma_bufs cam->dma_bufs[i] = NULL cam->nbufs = 0 cafe_pci_resume(mcam-core.c: line 413) ->mccic_resume ->mcam_read_setup ->mcam_alloc_dma_bufs cam->dma_bufs[i] = dma_alloc_coherent() (mcam-core.c: line 381) It looks like mcam_v4l_release() doesn't really shut the device down. In this case cafe_pci_resume() leaks memory for cam->dma_bufs[i] after mcam_v4l_release() freed and poisoned them. Is this feasible from your point of view? Thank you for your time. -- Anton Volkov Linux Verification Center, ISPRAS web: http://linuxtesting.org e-mail: avol...@ispras.ru
Re: [PATCH 1/4] i2c: busses: make i2c_adapter const
On 08/19/2017 03:34 AM, Bhumika Goyal wrote:
Make these const as they are only used in a copy operation.
Done using Coccinelle.
Signed-off-by: Bhumika Goyal
i2c-octeon-platdrv.c and i2c-thunderx-pcidrv.c changes:
Acked-by: David Daney
Thanks.
---
drivers/i2c/busses/i2c-kempld.c | 2 +-
drivers/i2c/busses/i2c-ocores.c | 2 +-
drivers/i2c/busses/i2c-octeon-platdrv.c | 2 +-
drivers/i2c/busses/i2c-thunderx-pcidrv.c | 2 +-
drivers/i2c/busses/i2c-xiic.c| 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-kempld.c b/drivers/i2c/busses/i2c-kempld.c
index 25993d2..e879190 100644
--- a/drivers/i2c/busses/i2c-kempld.c
+++ b/drivers/i2c/busses/i2c-kempld.c
@@ -289,7 +289,7 @@ static u32 kempld_i2c_func(struct i2c_adapter *adap)
.functionality = kempld_i2c_func,
};
-static struct i2c_adapter kempld_i2c_adapter = {
+static const struct i2c_adapter kempld_i2c_adapter = {
.owner = THIS_MODULE,
.name = "i2c-kempld",
.class = I2C_CLASS_HWMON | I2C_CLASS_SPD,
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 34f1889..8c42ca7 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -276,7 +276,7 @@ static u32 ocores_func(struct i2c_adapter *adap)
.functionality = ocores_func,
};
-static struct i2c_adapter ocores_adapter = {
+static const struct i2c_adapter ocores_adapter = {
.owner = THIS_MODULE,
.name = "i2c-ocores",
.class = I2C_CLASS_DEPRECATED,
diff --git a/drivers/i2c/busses/i2c-octeon-platdrv.c
b/drivers/i2c/busses/i2c-octeon-platdrv.c
index 917524c..64bda83 100644
--- a/drivers/i2c/busses/i2c-octeon-platdrv.c
+++ b/drivers/i2c/busses/i2c-octeon-platdrv.c
@@ -126,7 +126,7 @@ static u32 octeon_i2c_functionality(struct i2c_adapter
*adap)
.functionality = octeon_i2c_functionality,
};
-static struct i2c_adapter octeon_i2c_ops = {
+static const struct i2c_adapter octeon_i2c_ops = {
.owner = THIS_MODULE,
.name = "OCTEON adapter",
.algo = &octeon_i2c_algo,
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index ea35a895..df0976f 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -75,7 +75,7 @@ static u32 thunderx_i2c_functionality(struct i2c_adapter
*adap)
.functionality = thunderx_i2c_functionality,
};
-static struct i2c_adapter thunderx_i2c_ops = {
+static const struct i2c_adapter thunderx_i2c_ops = {
.owner = THIS_MODULE,
.name = "ThunderX adapter",
.algo = &thunderx_i2c_algo,
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 34b27bf..ae6ed25 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -721,7 +721,7 @@ static u32 xiic_func(struct i2c_adapter *adap)
.functionality = xiic_func,
};
-static struct i2c_adapter xiic_adapter = {
+static const struct i2c_adapter xiic_adapter = {
.owner = THIS_MODULE,
.name = DRIVER_NAME,
.class = I2C_CLASS_DEPRECATED,
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Hi Yong,
On Mon, Jul 31, 2017 at 11:16:40AM +0800, Yong wrote:
> > > @@ -143,6 +143,7 @@ source "drivers/media/platform/am437x/Kconfig"
> > > source "drivers/media/platform/xilinx/Kconfig"
> > > source "drivers/media/platform/rcar-vin/Kconfig"
> > > source "drivers/media/platform/atmel/Kconfig"
> > > +source "drivers/media/platform/sun6i-csi/Kconfig"
> >
> > We're going to have several different drivers in v4l eventually, so I
> > guess it would make sense to move to a directory of our own.
>
> Like this?
> drivers/media/platform/sunxi/sun6i-csi
Yep.
> > > +static int sun6i_graph_notify_complete(struct v4l2_async_notifier
> > > *notifier)
> > > +{
> > > + struct sun6i_csi *csi =
> > > + container_of(notifier, struct sun6i_csi, notifier);
> > > + struct sun6i_graph_entity *entity;
> > > + int ret;
> > > +
> > > + dev_dbg(csi->dev, "notify complete, all subdevs registered\n");
> > > +
> > > + /* Create links for every entity. */
> > > + list_for_each_entry(entity, &csi->entities, list) {
> > > + ret = sun6i_graph_build_one(csi, entity);
> > > + if (ret < 0)
> > > + return ret;
> > > + }
> > > +
> > > + /* Create links for video node. */
> > > + ret = sun6i_graph_build_video(csi);
> > > + if (ret < 0)
> > > + return ret;
> >
> > Can you elaborate a bit on the difference between a node parsed with
> > _graph_build_one and _graph_build_video? Can't you just store the
> > remote sensor when you build the notifier, and reuse it here?
>
> There maybe many usercases:
> 1. CSI->Sensor.
> 2. CSI->MIPI->Sensor.
> 3. CSI->FPGA->Sensor1
> ->Sensor2.
> FPGA maybe some other video processor. FPGA, MIPI, Sensor can be
> registered as v4l2 subdevs. We do not care about the driver code
> of them. But they should be linked together here.
>
> So, the _graph_build_one is used to link CSI port and subdevs.
> _graph_build_video is used to link CSI port and video node.
So the graph_build_one is for the two first cases, and the
_build_video for the latter case?
If so, you should take a look at the last iteration of the
subnotifiers rework by Nikas Söderlund (v4l2-async: add subnotifier
registration for subdevices).
It allows subdevs to register notifiers, and you don't have to build
the graph from the video device, each device and subdev can only care
about what's next in the pipeline, but not really what's behind it.
That would mean in your case that you can only deal with your single
CSI pad, and whatever subdev driver will use it care about its own.
> This part is also difficult to understand for me. The one CSI module
> have only one DMA channel(single port). But thay can be linked to
> different physical port (Parallel or MIPI)(multiple ep) by IF select
> register.
>
> For now, the binding can have several ep, the driver will just pick
> the first valid one.
Yeah, I'm not really sure how we could deal with that, but I guess we
can do it later on.
> >
> > > +struct sun6i_csi_ops {
> > > + int (*get_supported_pixformats)(struct sun6i_csi *csi,
> > > + const u32 **pixformats);
> > > + bool (*is_format_support)(struct sun6i_csi *csi, u32 pixformat,
> > > + u32 mbus_code);
> > > + int (*s_power)(struct sun6i_csi *csi, bool enable);
> > > + int (*update_config)(struct sun6i_csi *csi,
> > > + struct sun6i_csi_config *config);
> > > + int (*update_buf_addr)(struct sun6i_csi *csi, dma_addr_t addr);
> > > + int (*s_stream)(struct sun6i_csi *csi, bool enable);
> > > +};
> >
> > Didn't we agreed on removing those in the first iteration? It's not
> > really clear at this point whether they will be needed at all. Make
> > something simple first, without those ops. When we'll support other
> > SoCs we'll have a better chance at seeing what and how we should deal
> > with potential quirks.
>
> OK. But without ops, it is inappropriate to sun6i_csi and sun6i_csi.
> Maybe I should merge the two files.
I'm not sure what you meant here, but if you think that's appropriate,
please go ahead.
> > > + return IRQ_HANDLED;
> > > + }
> > > +
> > > + if (status & CSI_CH_INT_STA_FD_PD) {
> > > + sun6i_video_frame_done(&sdev->csi.video);
> > > + }
> > > +
> > > + regmap_write(regmap, CSI_CH_INT_STA_REG, status);
> >
> > Isn't it redundant with the one you did in the condition a bit above?
> >
> > You should also check that your device indeed generated an
> > interrupt. In the occurence of a spourious interrupt, your code will
> > return IRQ_HANDLED, which is the wrong thing to do.
> >
> > I think you should reverse your logic a bit here to make this
> > easier. You should just check that your status flags are indeed set,
> > and if not just bail out and return IRQ_NONE.
> >
> > And if they are, go on with treating your interrupt.
>
> OK. I will add check for status flags.
> BTW, how can a spurious interrupt occurred?
Usually it's either through some interference, o
Re: [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a
Hi Sakari,
Thanks for the update.
On 08/19/2017 11:24 PM, Sakari Ailus wrote:
> From: Sakari Ailus
>
> Signed-off-by: Sakari Ailus
> ---
> .../devicetree/bindings/leds/ams,as3645a.txt | 71
> ++
> 1 file changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> new file mode 100644
> index ..12c5ef26ec73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
> @@ -0,0 +1,71 @@
> +Analog devices AS3645A device tree bindings
> +
> +The AS3645A flash LED controller can drive two LEDs, one high current
> +flash LED and one indicator LED. The high current flash LED can be
> +used in torch mode as well.
> +
> +Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
> +and b are included in the range.
> +
> +Please also see common.txt in the same directory.
> +
> +
> +Required properties
> +===
> +
> +compatible : Must be "ams,as3645a".
> +reg : The I2C address of the device. Typically 0x30.
> +
> +
> +Required properties of the "flash" child node
> +=
> +
> +flash-timeout-us: Flash timeout in microseconds. The value must be in
> + the range [10, 85] and divisible by 5.
> +flash-max-microamp: Maximum flash current in microamperes. Has to be
> + in the range between [20, 50] and
> + divisible by 2.
> +led-max-microamp: Maximum torch (assist) current in microamperes. The
> + value must be in the range between [2, 16] and
> + divisible by 2.
> +ams,input-max-microamp: Maximum flash controller input current. The
> + value must be in the range [125, 200]
> + and divisible by 5.
> +
> +
> +Optional properties of the "flash" child node
> +=
> +
> +label: The label of the flash LED.
> +
> +
> +Required properties of the "indicator" child node
> +=
> +
> +led-max-microamp: Maximum indicator current. The allowed values are
> + 2500, 5000, 7500 and 1.
> +
> +Optional properties of the "indicator" child node
> +=
> +
> +label: The label of the indicator LED.
> +
> +
> +Example
> +===
> +
> + as3645a@30 {
> + reg = <0x30>;
> + compatible = "ams,as3645a";
> + flash {
> + flash-timeout-us = <15>;
> + flash-max-microamp = <32>;
> + led-max-microamp = <6>;
> + ams,input-max-microamp = <175>;
> + label = "as3645a:flash";
> + };
> + indicator {
> + led-max-microamp = <1>;
> + label = "as3645a:indicator";
> + };
> + };
>
For the patch going through media tree:
Acked-by: Jacek Anaszewski
--
Best regards,
Jacek Anaszewski
Re: adv7281m and rcar-vin problem
Hi Niklas,
adv7281m driver powers up the CSI transmitter in s_power(), which is
called before setting up of D-PHY layer of R-Car CSI-2 Receiver.
I shifted the part of code which enables CSI transmitter in adv7281m
(Low Power state to High Speed state) to s_stream() -
if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
if (on) {
adv7180_csi_write(state, 0xDE, 0x02);
adv7180_csi_write(state, 0xD2, 0xF7);
adv7180_csi_write(state, 0xD8, 0x65);
adv7180_csi_write(state, 0xE0, 0x09);
adv7180_csi_write(state, 0x2C, 0x00);
if (state->field == V4L2_FIELD_NONE)
adv7180_csi_write(state, 0x1D, 0x80);
adv7180_csi_write(state, 0x00, 0x00);
} else {
adv7180_csi_write(state, 0x00, 0x80);
}
}
After this change, i am not getting timeout of reading the phy clock
lane and capture starts but nothing is displayed on the screen.
On Wed, Jul 26, 2017 at 2:08 PM, Niklas Söderlund
wrote:
> Hi Naman,
>
> On 2017-07-24 22:43:06 +0530, Naman Jain wrote:
>> On Mon, Jul 24, 2017 at 3:11 PM, Niklas Söderlund
>> wrote:
>> > Hi Naman,
>> >
>> > On 2017-07-24 14:30:52 +0530, Naman Jain wrote:
>> >> i am using renesas soc with video decoder adv7281m
>> >> i have done thr device tree configuration by following dt bindings
>> >> i am getting timeout of reading the phy clock lane, after i start
>> >> streaming
>> >> and nothing is displayed on the screen
>> >> kindly help me in configuration
>> >
>> > To be able to try and help you I would need a lot more information. For
>> > starters:
>> >
>> > - Which kernel version are you using?
>> >
>> > - How dose the device tree nodes for VIN and ADV7281m look like?
>> >
>> > --
>> > Regards,
>> > Niklas Söderlund
>>
>> Hi Niklas,
>>
>> I am using kernel version - 4.9
>
> The VIN driver which supports CSI-2 and the R-Car CSI-2 driver is not a
> part of the upstream kernel yet, and the latest patches with contains
> the most fixes are based on newer kernels then v4.9. So I assume you are
> using a BSP of some sort, if possible could you tell me which one?
>
> If you want to try with later increments of the VIN and CSI-2 patches
> please see:
>
> http://elinux.org/R-Car/Tests:rcar-vin
>
>
Soc version is rcar-h3 (r8a7795).
Can tell me dependency patches required?
>>
>> following is the device tree configuration :
>>
>> &i2c6 {
>> status = "okay";
>> clock-frequency = <40>;
>> adv7281m@21{
>>compatible = "adi,adv7281-m";
>>reg = <0x20>;
>>interrupt-parent = <&gpio6>;
>>interrupts = <4 IRQ_TYPE_LEVEL_LOW>
>>adv7281m_out: endpoint {
>> clock-lanes = <0>;
>> data-lanes = <1>;
>> remote-endpoint = <&csi20_in>;
>> };
>>};
>>
>> }
>>
>> &csi20 {
>> status = "okay";
>> ports {
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> port@0 {
>> reg = <0>;
>> csi20_in: endpoint {
>>clock-lanes = <0>;
>>data-lanes = <1>;
>>
>> virtual-channel-number=<0>;
>
> This is interesting for me, I have not worked with any driver for the
> R-Car CSI-2 driver which understands the virtual-channel-number
> property.
>
>>remote-endpoint =
>> <&adv7281m_out>;
>> };
>>};
>> };
>> };
>>
>> &vin0 {
>> status = "okay";
>> };
>>
>> &vin1 {
>> status = "okay";
>> };
>>
>> &vin2 {
>> status = "okay";
>> };
>>
>> &vin3 {
>> status = "okay";
>> };
>>
>> &vin4 {
>> status = "okay";
>> };
>>
>> &vin5 {
>> status = "okay";
>> };
>>
>> &vin6 {
>> status = "okay";
>> };
>>
>> &vin7 {
>> status = "okay";
>> };
>
> --
> Regards,
> Niklas Söderlund
Re: [PATCH v2 1/2] dt-bindings: media: Add Cadence MIPI-CSI2 RX Device Tree bindings
Hi Laurent, On 22/08/2017, 02:01, "Laurent Pinchart" wrote: EXTERNAL MAIL Hi Maxime, On Tuesday, 22 August 2017 11:53:20 EEST Maxime Ripard wrote: > On Mon, Aug 07, 2017 at 11:18:03PM +0300, Laurent Pinchart wrote: > > On Thursday 20 Jul 2017 11:23:01 Maxime Ripard wrote: > >> The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up > >> to 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending > >> on the hardware implementation. > >> > >> It can operate with an external D-PHY, an internal one or no D-PHY at > >> all in some configurations. > > > > Without any PHY ? I'm curious, how does that work ? > > We're currently working on an FPGA exactly with that configuration. So > I guess the answer would be "it doesn't on an ASIC" :) What's connected to the input of the CSI-2 receiver ? It is connected to an instance of a CSI TX digital interface. > >> Signed-off-by: Maxime Ripard > >> --- > >> > >> .../devicetree/bindings/media/cdns-csi2rx.txt | 87 > >> 1 file changed, 87 insertions(+) > >> create mode 100644 > >> Documentation/devicetree/bindings/media/cdns-csi2rx.txt > >> > >> diff --git a/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > >> b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt new file mode > >> 100644 > >> index ..e08547abe885 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > >> @@ -0,0 +1,87 @@ > >> +Cadence MIPI-CSI2 RX controller > >> +=== > >> + > >> +The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting up to > >> 4> CSI > >> +lanes in input, and 4 different pixel streams in output. > >> + > >> +Required properties: > >> + - compatible: must be set to "cdns,csi2rx" and an SoC-specific > >> compatible > >> + - reg: base address and size of the memory mapped region > >> + - clocks: phandles to the clocks driving the controller > >> + - clock-names: must contain: > >> +* sys_clk: main clock > >> +* p_clk: register bank clock > >> +* p_free_clk: free running register bank clock > >> +* pixel_ifX_clk: pixel stream output clock, one for each stream > >> + implemented in hardware, between 0 and 3 > > > > Nitpicking, I would write the name is pixel_if[0-3]_clk, it took me a few > > seconds to see that the X was a placeholder. > > Ok. > > >> +* dphy_rx_clk: D-PHY byte clock, if implemented in hardware > >> + - phys: phandle to the external D-PHY > >> + - phy-names: must contain dphy, if the implementation uses an > >> + external D-PHY > > > > I would move the last two properties in an optional category as they're > > effectively optional. I think you should also explain a bit more clearly > > that the phys property must not be present if the phy-names property is > > not present. > > It's not really optional. The IP has a configuration register that > allows you to see if it's been synthesized with or without a PHY. If > the right bit is set, that property will be mandatory, if not, it's > useless. Just to confirm, the PHY is a separate IP core, right ? Is the CSI-2 receiver input interface different when used with a PHY and when used without one ? Could a third-party PHY be used as well ? If so, would the PHY synthesis bit be set or not ? The PHY (in our case a D-PHY) is a separate entity, it can be from a 3rd party as the IP interface is standard, the SoC integrator would set the bit accordingly based on whether any PHY is present or not. There is also an option of routing digital output from a CSI-TX to a CSI-RX and in such case a PHY would not need to be used (as in the case of our current platform). > Maybe it's just semantics, but to me, optional means that it can > operate with or without it under any circumstances. It's not really > the case here. It'sa semantic issue, but documenting a property as required when it can be ommitted under some circumstances seems even weirder to me :-) I understand the optional category as "can be ommitted in certain circumstances". > >> + > >> +Required subnodes: > >> + - ports: A ports node with endpoint definitions as defined in > >> + > >> Documentation/devicetree/bindings/media/video-interfaces.txt. > >> The > >> + first port subnode should be the input endpoint, the second > >> one the > >> + outputs > >> + > >> + The output port should have as many endpoints as stream supported by > >> + the hardware implementation, between 1 and 4, their ID being the > >> + st
Re: adv7281m and rcar-vin problem
Hi Naman,
On 2017-08-23 00:15:41 +0530, Naman Jain wrote:
> Hi Niklas,
>
> adv7281m driver powers up the CSI transmitter in s_power(), which is
> called before setting up of D-PHY layer of R-Car CSI-2 Receiver.
> I shifted the part of code which enables CSI transmitter in adv7281m
> (Low Power state to High Speed state) to s_stream() -
>
> if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
>if (on) {
> adv7180_csi_write(state, 0xDE, 0x02);
> adv7180_csi_write(state, 0xD2, 0xF7);
> adv7180_csi_write(state, 0xD8, 0x65);
> adv7180_csi_write(state, 0xE0, 0x09);
> adv7180_csi_write(state, 0x2C, 0x00);
>if (state->field == V4L2_FIELD_NONE)
>adv7180_csi_write(state, 0x1D, 0x80);
> adv7180_csi_write(state, 0x00, 0x00);
> } else {
> adv7180_csi_write(state, 0x00, 0x80);
> }
> }
>
> After this change, i am not getting timeout of reading the phy clock
> lane and capture starts but nothing is displayed on the screen.
I know nothing about the adv7281m driver, but if you define DEBUG in the
rcar-vin and rcar-csi2 drivers it will provide you with a lot more
information about how they behave and maybe it can help you in your
troubleshooting. If you enable this and send me a console log of what
happens when you try to start a stream I can try and help you.
>
> On Wed, Jul 26, 2017 at 2:08 PM, Niklas Söderlund
> wrote:
> > Hi Naman,
> >
> > On 2017-07-24 22:43:06 +0530, Naman Jain wrote:
> >> On Mon, Jul 24, 2017 at 3:11 PM, Niklas Söderlund
> >> wrote:
> >> > Hi Naman,
> >> >
> >> > On 2017-07-24 14:30:52 +0530, Naman Jain wrote:
> >> >> i am using renesas soc with video decoder adv7281m
> >> >> i have done thr device tree configuration by following dt bindings
> >> >> i am getting timeout of reading the phy clock lane, after i start
> >> >> streaming
> >> >> and nothing is displayed on the screen
> >> >> kindly help me in configuration
> >> >
> >> > To be able to try and help you I would need a lot more information. For
> >> > starters:
> >> >
> >> > - Which kernel version are you using?
> >> >
> >> > - How dose the device tree nodes for VIN and ADV7281m look like?
> >> >
> >> > --
> >> > Regards,
> >> > Niklas Söderlund
> >>
> >> Hi Niklas,
> >>
> >> I am using kernel version - 4.9
> >
> > The VIN driver which supports CSI-2 and the R-Car CSI-2 driver is not a
> > part of the upstream kernel yet, and the latest patches with contains
> > the most fixes are based on newer kernels then v4.9. So I assume you are
> > using a BSP of some sort, if possible could you tell me which one?
> >
> > If you want to try with later increments of the VIN and CSI-2 patches
> > please see:
> >
> > http://elinux.org/R-Car/Tests:rcar-vin
> >
> >
>
> Soc version is rcar-h3 (r8a7795).
> Can tell me dependency patches required?
The dependencies are documented in the wiki page mentioned above, you
can also use the latest renesas-drivers master branch
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
>
> >>
> >> following is the device tree configuration :
> >>
> >> &i2c6 {
> >> status = "okay";
> >> clock-frequency = <40>;
> >> adv7281m@21{
> >>compatible = "adi,adv7281-m";
> >>reg = <0x20>;
> >>interrupt-parent = <&gpio6>;
> >>interrupts = <4 IRQ_TYPE_LEVEL_LOW>
> >>adv7281m_out: endpoint {
> >> clock-lanes = <0>;
> >> data-lanes = <1>;
> >> remote-endpoint = <&csi20_in>;
> >> };
> >>};
> >>
> >> }
> >>
> >> &csi20 {
> >> status = "okay";
> >> ports {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >>
> >> port@0 {
> >> reg = <0>;
> >> csi20_in: endpoint {
> >>clock-lanes = <0>;
> >>data-lanes = <1>;
> >>
> >> virtual-channel-number=<0>;
> >
> > This is interesting for me, I have not worked with any driver for the
> > R-Car CSI-2 driver which understands the virtual-channel-number
> > property.
> >
> >>remote-endpoint =
> >> <&adv7281m_out>;
> >> };
> >>};
> >> };
> >> };
> >>
> >> &vin0 {
> >> status = "okay";
> >> };
> >>
> >> &vin1 {
> >> status = "okay";
> >> };
> >>
> >> &vin2 {
> >> status = "okay";
> >> };
> >>
> >> &vin3 {
> >> status = "okay";
> >> };
> >>
> >> &vin4 {
> >> status = "okay";
> >> };
Re: [PATCH v2] device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()
On Tue, Aug 22, 2017 at 10:00 AM, Niklas Söderlund
wrote:
> Hi Rob,
>
> On 2017-08-22 09:49:35 -0500, Rob Herring wrote:
>> On Mon, Aug 21, 2017 at 7:19 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 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(-)
>>
>> Isn't this already fixed with this fix:
>>
>> commit c0a480d1acf7dc184f9f3e7cf724483b0d28dc2e
>> Author: Tony Lindgren
>> Date: Fri Jul 28 01:23:15 2017 -0700
>>
>> device property: Fix usecount for of_graph_get_port_parent()
>
> No, that commit fixes it for of_graph_get_port_parent() while this
> commit fixes it for of_fwnode_graph_get_port_parent(). But in essence it
> is the same issue but needs two separate fixes.
Ah, because one takes the port node and one takes the endpoint node.
That won't confuse anyone.
Can we please align this mess. I've tried to make the graph parsing
not a free for all, open coded mess. There's no reason to have the
port node handle and then need the parent device. Either you started
with the parent device to parse local ports and endpoints or you got
the remote endpoint with .graph_get_remote_endpoint(). Most of the
time you don't even need the endpoint node handles. You really just
need to know what is the remote device connected to port X, endpoint Y
of my local device.
Rob
Re: [PATCH v2] venus: fix copy/paste error in return_buf_error
On 08/21/2017 04:14 AM, Stanimir Varbanov wrote: Thanks Gustavo! Glad to help. :) 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); } -- Gustavo A. R. Silva
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
On Tue, Aug 22, 2017 at 08:43:35AM +0200, Hans Verkuil wrote:
> >>> +static int sun6i_video_link_setup(struct media_entity *entity,
> >>> + const struct media_pad *local,
> >>> + const struct media_pad *remote, u32 flags)
> >>> +{
> >>> + struct video_device *vdev = media_entity_to_video_device(entity);
> >>> + struct sun6i_video *video = video_get_drvdata(vdev);
> >>> +
> >>> + if (WARN_ON(video == NULL))
> >>> + return 0;
> >>> +
> >>> + return sun6i_video_formats_init(video);
> >>
> >> Why is this called here? Why not in video_init()?
> >
> > sun6i_video_init is in the driver probe context. sun6i_video_formats_init
> > use media_entity_remote_pad and media_entity_to_v4l2_subdev to find the
> > subdevs.
> > The media_entity_remote_pad can't work before all the media pad linked.
>
> A video_init is typically called from the notify_complete callback.
> Actually, that's where the video_register_device should be created as well.
> When you create it in probe() there is possibly no sensor yet, so it would
> be a dead video node (or worse, crash when used).
>
> There are still a lot of platform drivers that create the video node in the
> probe, but it's not the right place if you rely on the async loading of
> subdevs.
That's not really related, but I'm not really sure it's a good way to
operate. This essentially means that you might wait forever for a
component in your pipeline to be probed, without any chance of it
happening (not compiled, compiled as a module and not loaded, hardware
defect preventing the driver from probing properly, etc), even though
that component might not be essential.
This is how DRM operates, and you sometimes end up in some very dumb
situations where you wait for say, the DSI controller to probe, while
you only care about HDMI in your system.
But this seems to be on of the hot topic these days, so we might
discuss it further in some other thread :)
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
signature.asc
Description: PGP signature
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Hello,
On Tuesday, 22 August 2017 23:17:31 EEST Maxime Ripard wrote:
> On Tue, Aug 22, 2017 at 08:43:35AM +0200, Hans Verkuil wrote:
> +static int sun6i_video_link_setup(struct media_entity *entity,
> + const struct media_pad *local,
> + const struct media_pad *remote, u32
> flags)
> +{
> +struct video_device *vdev =
> media_entity_to_video_device(entity);
> +struct sun6i_video *video = video_get_drvdata(vdev);
> +
> +if (WARN_ON(video == NULL))
> +return 0;
> +
> +return sun6i_video_formats_init(video);
> >>>
> >>> Why is this called here? Why not in video_init()?
> >>
> >> sun6i_video_init is in the driver probe context.
> >> sun6i_video_formats_init use media_entity_remote_pad and
> >> media_entity_to_v4l2_subdev to find the subdevs.
> >> The media_entity_remote_pad can't work before all the media pad linked.
> >
> > A video_init is typically called from the notify_complete callback.
> > Actually, that's where the video_register_device should be created as
> > well. When you create it in probe() there is possibly no sensor yet, so it
> > would be a dead video node (or worse, crash when used).
> >
> > There are still a lot of platform drivers that create the video node in
> > the probe, but it's not the right place if you rely on the async loading
> > of subdevs.
>
> That's not really related, but I'm not really sure it's a good way to
> operate. This essentially means that you might wait forever for a
> component in your pipeline to be probed, without any chance of it
> happening (not compiled, compiled as a module and not loaded, hardware
> defect preventing the driver from probing properly, etc), even though
> that component might not be essential.
I agree with Maxime here, we should build the media device incrementally, and
offer userspace access early on without waiting for all pieces to be
available. If properly implemented (there should definitely be so crash) I
don't see any drawback to that approach.
> This is how DRM operates, and you sometimes end up in some very dumb
> situations where you wait for say, the DSI controller to probe, while
> you only care about HDMI in your system.
>
> But this seems to be on of the hot topic these days, so we might
> discuss it further in some other thread :)
Or here :-)
--
Regards,
Laurent Pinchart
Re: [PATCH v2 1/2] dt-bindings: media: Add Cadence MIPI-CSI2 RX Device Tree bindings
Hi Cyprian, On Tuesday, 22 August 2017 22:25:49 EEST Cyprian Wronka wrote: > On 22/08/2017, 02:01, Laurent Pinchart wrote: >> On Tuesday, 22 August 2017 11:53:20 EEST Maxime Ripard wrote: >>> On Mon, Aug 07, 2017 at 11:18:03PM +0300, Laurent Pinchart wrote: On Thursday 20 Jul 2017 11:23:01 Maxime Ripard wrote: The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that > supports up to 4 CSI-2 lanes, and can route the frames to up to 4 > streams, depending on the hardware implementation. > > It can operate with an external D-PHY, an internal one or no D-PHY > at all in some configurations. Without any PHY ? I'm curious, how does that work ? >>> >>> We're currently working on an FPGA exactly with that configuration. >>> So I guess the answer would be "it doesn't on an ASIC" :) >>> >> What's connected to the input of the CSI-2 receiver ? >> > It is connected to an instance of a CSI TX digital interface. That makes sense. I suppose it's a test setup > Signed-off-by: Maxime Ripard > --- > > > .../devicetree/bindings/media/cdns-csi2rx.txt> | 87 > > 1 file changed, 87 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/cdns-csi2rx.txt > > > diff --git > a/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt new file > mode 100644 > index ..e08547abe885 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > @@ -0,0 +1,87 @@ > +Cadence MIPI-CSI2 RX controller > +=== > + > +The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting > up to 4 CSI > +lanes in input, and 4 different pixel streams in output. > + > +Required properties: > + - compatible: must be set to "cdns,csi2rx" and an SoC-specific > compatible > + - reg: base address and size of the memory mapped region > + - clocks: phandles to the clocks driving the controller > + - clock-names: must contain: > +* sys_clk: main clock > +* p_clk: register bank clock > +* p_free_clk: free running register bank clock > +* pixel_ifX_clk: pixel stream output clock, one for each > stream > + implemented in hardware, between 0 and 3 Nitpicking, I would write the name is pixel_if[0-3]_clk, it took me a few seconds to see that the X was a placeholder. >>> >>> Ok. >>> > +* dphy_rx_clk: D-PHY byte clock, if implemented in hardware > + - phys: phandle to the external D-PHY > + - phy-names: must contain dphy, if the implementation uses an > + external D-PHY I would move the last two properties in an optional category as they're effectively optional. I think you should also explain a bit more clearly that the phys property must not be present if the phy-names property is not present. >>> >>> It's not really optional. The IP has a configuration register that >>> allows you to see if it's been synthesized with or without a PHY. If >>> the right bit is set, that property will be mandatory, if not, it's >>> useless. >> >> Just to confirm, the PHY is a separate IP core, right ? Is the CSI-2 >> receiver input interface different when used with a PHY and when used >> without one ? Could a third-party PHY be used as well ? If so, would the >> PHY synthesis bit be set or not ? > > The PHY (in our case a D-PHY) is a separate entity, it can be from a 3rd > party as the IP interface is standard, the SoC integrator would set the bit > accordingly based on whether any PHY is present or not. There is also an > option of routing digital output from a CSI-TX to a CSI-RX and in such case > a PHY would not need to be used (as in the case of our current platform). OK, thank you for the clarification. Maxime mentioned that a bit can be read from a register to notify whether a PHY has been synthesized or not. Does it influence the CSI-2 RX input interface at all, or is the CSI-2 RX IP core synthesized the same way regardless of whether a PHY is present or not ? >>> Maybe it's just semantics, but to me, optional means that it can >>> operate with or without it under any circumstances. It's not really >>> the case here. >> >> It's a semantic issue, but documenting a property as required when it can >> be ommitted under some circumstances seems even weirder to me :-) I >> understand the optional category as "can be ommitted in certain >> circumstances". >> > + > +Required subnodes: > + - ports: A ports node with endpoint definitions as defined in > + Documentation/devicetree/bindings/media/video-interfaces.txt. The > + first port subnode should be the input endpoint, the > second one the > + outputs > + > + The output port should have as many endpoints as stream > supported
[PATCH v6 00/25] rcar-vin: Add Gen3 with media controller
Hi,
This series adds Gen3 VIN support to rcar-vin driver for Renesas r8a7795
and r8a7796. It is based on the media-tree.
The driver is tested on both Renesas H3 (r8a7795, ES1.x + ES2.0) and
M3-W (r8a7796) together with the rcar-csi2 driver (posted separately and
not yet upstream) and the Salvator-X onboard ADV7482.
It is possible to capture both CVBS and HDMI video streams,
v4l2-compliance passes with no errors and media-ctl can be used to
change the routing and formats for the different entities in the media
graph.
Gen2 compatibility is verified on Koelsch and no problems where found, video
can be captured just like before and v4l2-compliance passes without errors or
warnings just like before this series.
I have started on a very basic test suite for the VIN driver at:
https://git.ragnatech.se/vin-tests
And as before the state of the driver and information about how to test it can
be found on the elinux wiki:
http://elinux.org/R-Car/Tests:rcar-vin
* Changes since v5
- Extract and make use of common format checking for both Gen2 and Gen3.
- Assign pad at declaration time in rvin_get_sd_format()
- Always call pm_runtime_{get_sync,put}() and v4l2_pipeline_pm_use()
when opening/closing a video device, remove the check of
v4l2_fh_is_singular_file().
- Make rvin_set_chsel() return void instead of int since it always
return 0.
- Simplify the VIN group allocator functions.
- Make the group notifier callbacks and setup more robust.
- Moved the video device registration back to probe time.
- Add H3 ES2.0 support.
- Fix handling of single field formats (top, bottom, alternate) as this
was obviously wrong before but hidden by the Gen2 scaler support.
- Added review tags from Kieran.
* Changes since v4 (Not posted to ML)
- Updated to the new fwnode functions.
- Moved the registration of the video devices to the async notification
callback.
* Changes since v3
- Only add neighboring subdevices to the async notifier. Instead of
parsing the whole OF graph depend on incremental async subnotifier to
discover the whole pipeline. This is needed to support arbitrarily
long graphs and support the new ADV7482 prototype driver which Kieran
is working on.
- Fix warning from lockdep, reported by Kieran.
- Fix commit messages from feedback from Sergei, thanks.
- Fix chip info an OF device ids sorting order, thanks Geert.
- Use subdev->of_node instead of subdev->dev->of_node, thanks Kieran.
* Changes since v2
- Do not try to control the subdevices in the media graph from the rcar-vin
driver. Have user-space configure to format in the pipeline instead.
- Add link validation before starting the stream.
- Rework on how the subdevices are and the video node behave by defining
specific V4L2 operations for the MC mode of operation, this simplified the
driver quit a bit, thanks Laurent!
- Add a new 'renesas,id' DT property which is needed to to be able to keep the
VIN to CSI-2 routing table inside the driver. Previously this information was
taken from the CSI-2 DT node which is obviusly the wrong way to do things.
Thanks Laurent for pointing this out.
- Fixed a memory leek in the group allocator function.
- Return -EMLINK instead of -EBUSY if a MC link is not possible given the
current routing setup.
- Add comments to clarify that the 4 channels from the CSI-2 node is not
directly related to CSI-2 virtual channels, the CSI-2 node can output any VC
on any of its output channels.
* Changes since v1
- Remove unneeded casts as pointed out by Geert.
- Fix spelling and DT documentation as pointed out by Geert and Sergei, thanks!
- Refresh patch 2/32 with an updated version, thanks Sakari for pointing this
out.
- Add Sakaris Ack to patch 1/32.
- Rebase on top of v4.9-rc1 instead of v4.9-rc3 to ease integration testing
together with renesas-drivers tree.
Niklas Söderlund (25):
rcar-vin: add Gen3 devicetree bindings documentation
rcar-vin: register the video device at probe time
rcar-vin: move chip information to own struct
rcar-vin: move max width and height information to chip information
rcar-vin: change name of video device
rcar-vin: move functions regarding scaling
rcar-vin: all Gen2 boards can scale simplify logic
rcar-vin: do not reset crop and compose when setting format
rcar-vin: do not allow changing scaling and composing while streaming
rcar-vin: read subdevice format for crop only when needed
rcar-vin: fix handling of single field frames (top, bottom and
alternate fields)
rcar-vin: move media bus configuration to struct rvin_info
rcar-vin: enable Gen3 hardware configuration
rcar-vin: add functions to manipulate Gen3 CHSEL value
rcar-vin: add flag to switch to media controller mode
rcar-vin: break out format alignment and checking
rcar-vin: use different v4l2 operations in media controller mode
rcar-vin: prepare for media controller mode initialization
rcar-vin: add group allocator functions
rcar-vin: add chsel information to
[PATCH v6 01/25] rcar-vin: add Gen3 devicetree bindings documentation
Document the devicetree bindings for the CSI-2 inputs available on Gen3.
There is a need to add a custom property 'renesas,id' and to define
which CSI-2 input is described in which endpoint under the port@1 node.
This information is needed since there are a set of predefined routes
between each VIN and CSI-2 block. This routing table will be kept
inside the driver but in order for it to act on it it must know which
VIN and CSI-2 is which.
Signed-off-by: Niklas Söderlund
---
.../devicetree/bindings/media/rcar_vin.txt | 106 +++--
1 file changed, 96 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 6e4ef8caf759e5d3..be38ad89d71ad05d 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -2,8 +2,12 @@ Renesas R-Car Video Input driver (rcar_vin)
---
The rcar_vin device provides video input capabilities for the Renesas R-Car
-family of devices. The current blocks are always slaves and suppot one input
-channel which can be either RGB, YUYV or BT656.
+family of devices.
+
+On Gen2 the current blocks are always slaves and support one input channel
+which can be either RGB, YUYV or BT656. On Gen3 the current blocks are
+always slaves and support multiple input channels which can be either RGB,
+YUVU, BT656 or CSI-2.
- compatible: Must be one or more of the following
- "renesas,vin-r8a7795" for the R8A7795 device
@@ -28,7 +32,7 @@ channel which can be either RGB, YUYV or BT656.
Additionally, an alias named vinX will need to be created to specify
which video input device this is.
-The per-board settings:
+The per-board settings Gen2:
- port sub-node describing a single endpoint connected to the vin
as described in video-interfaces.txt[1]. Only the first one will
be considered as each vin interface has one input port.
@@ -36,13 +40,21 @@ The per-board settings:
These settings are used to work out video input format and widths
into the system.
+The per-board settings Gen3:
+- renesas,id - ID number of the VIN
+- Port 0 - Digital video source (same as port node on Gen2)
+- Port 1 - CSI-2 video sources
+- Endpoint 0 - sub-node describing the endpoint which is CSI20
+- Endpoint 1 - sub-node describing the endpoint which is CSI21
+- Endpoint 2 - sub-node describing the endpoint which is CSI40
+- Endpoint 3 - sub-node describing the endpoint which is CSI41
-Device node example
+Device node example Gen2
+
- aliases {
- vin0 = &vin0;
- };
+aliases {
+vin0 = &vin0;
+};
vin0: vin@0xe6ef {
compatible = "renesas,vin-r8a7790", "renesas,rcar-gen2-vin";
@@ -52,8 +64,8 @@ Device node example
status = "disabled";
};
-Board setup example (vin1 composite video input)
-
+Board setup example Gen2 (vin1 composite video input)
+-
&i2c2 {
status = "ok";
@@ -92,6 +104,80 @@ Board setup example (vin1 composite video input)
};
};
+Device node example Gen3
+
+
+vin0: video@e6ef {
+compatible = "renesas,vin-r8a7795";
+reg = <0 0xe6ef 0 0x1000>;
+interrupts = ;
+clocks = <&cpg CPG_MOD 811>;
+power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+status = "disabled";
+
+renesas,id = <0>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@1 {
+#address-cells = <1>;
+#size-cells = <0>;
+
+reg = <1>;
+
+vin0csi20: endpoint@0 {
+reg = <0>;
+remote-endpoint= <&csi20vin0>;
+};
+vin0csi21: endpoint@1 {
+reg = <1>;
+remote-endpoint= <&csi21vin0>;
+};
+vin0csi40: endpoint@2 {
+reg = <2>;
+remote-endpoint= <&csi40vin0>;
+};
+};
+};
+};
+
+csi20: csi2@fea8 {
+compatible = "renesas,r8a7795-csi2", "renesas,rcar-gen3-csi2";
+reg = <0 0xfea8 0 0x1>;
+interrupts = ;
+clocks = <&
[PATCH v6 02/25] rcar-vin: register the video device at probe time
The driver registers the video device from the async complete callback
and unregistered in the async unbind callback. This creates problems if
if the subdevice is bound, unbound and later rebound. The second time
video_register_device() is called it fails:
kobject (eb3be918): tried to init an initialized object, something is
seriously wrong.
To prevent this register the video device at prob time and don't allow
user-space to open the video device if the subdevice have not yet been
bound.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-core.c | 42 +++--
drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 +
drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
3 files changed, 47 insertions(+), 38 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index 77dff047c41c803e..aefbe8e3ccddb3e4 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -74,6 +74,7 @@ static bool rvin_mbus_supported(struct rvin_graph_entity
*entity)
static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier)
{
struct rvin_dev *vin = notifier_to_vin(notifier);
+ struct v4l2_subdev *sd = vin_to_source(vin);
int ret;
/* Verify subdevices mbus format */
@@ -92,7 +93,35 @@ static int rvin_digital_notify_complete(struct
v4l2_async_notifier *notifier)
return ret;
}
- return rvin_v4l2_probe(vin);
+ /* Add the controls */
+ /*
+* Currently the subdev with the largest number of controls (13) is
+* ov6550. So let's pick 16 as a hint for the control handler. Note
+* that this is a hint only: too large and you waste some memory, too
+* small and there is a (very) small performance hit when looking up
+* controls in the internal hash.
+*/
+ ret = v4l2_ctrl_handler_init(&vin->ctrl_handler, 16);
+ if (ret < 0)
+ return ret;
+
+ ret = v4l2_ctrl_add_handler(&vin->ctrl_handler, sd->ctrl_handler, NULL);
+ if (ret < 0)
+ return ret;
+
+ ret = v4l2_subdev_call(sd, video, g_tvnorms, &vin->vdev.tvnorms);
+ if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
+ return ret;
+
+ if (vin->vdev.tvnorms == 0) {
+ /* Disable the STD API if there are no tvnorms defined */
+ v4l2_disable_ioctl(&vin->vdev, VIDIOC_G_STD);
+ v4l2_disable_ioctl(&vin->vdev, VIDIOC_S_STD);
+ v4l2_disable_ioctl(&vin->vdev, VIDIOC_QUERYSTD);
+ v4l2_disable_ioctl(&vin->vdev, VIDIOC_ENUMSTD);
+ }
+
+ return rvin_reset_format(vin);
}
static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier,
@@ -102,7 +131,7 @@ static void rvin_digital_notify_unbind(struct
v4l2_async_notifier *notifier,
struct rvin_dev *vin = notifier_to_vin(notifier);
vin_dbg(vin, "unbind digital subdev %s\n", subdev->name);
- rvin_v4l2_remove(vin);
+ v4l2_ctrl_handler_free(&vin->ctrl_handler);
vin->digital.subdev = NULL;
}
@@ -231,6 +260,10 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
vin->notifier.unbind = rvin_digital_notify_unbind;
vin->notifier.complete = rvin_digital_notify_complete;
+ ret = rvin_v4l2_probe(vin);
+ if (ret)
+ return ret;
+
ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
if (ret < 0) {
vin_err(vin, "Notifier registration failed\n");
@@ -314,6 +347,11 @@ static int rcar_vin_remove(struct platform_device *pdev)
v4l2_async_notifier_unregister(&vin->notifier);
+ /* Checks internaly if handlers have been init or not */
+ v4l2_ctrl_handler_free(&vin->ctrl_handler);
+
+ rvin_v4l2_remove(vin);
+
rvin_dma_remove(vin);
return 0;
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index dd37ea8116804df3..81ff59c3b4744075 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -103,7 +103,7 @@ static void rvin_reset_crop_compose(struct rvin_dev *vin)
vin->compose.height = vin->format.height;
}
-static int rvin_reset_format(struct rvin_dev *vin)
+int rvin_reset_format(struct rvin_dev *vin)
{
struct v4l2_subdev_format fmt = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
@@ -781,6 +781,11 @@ static int rvin_open(struct file *file)
mutex_lock(&vin->lock);
+ if (!vin->digital.subdev) {
+ ret = -ENODEV;
+ goto unlock;
+ }
+
file->private_data = vin;
ret = v4l2_fh_open(file);
@@ -844,9 +849,6 @@ void rvin_v4l2_remove(struct rvin_dev *vin)
v4l2_info(&vin->v4l2_dev, "Removing %s\n",
[PATCH v6 03/25] rcar-vin: move chip information to own struct
When Gen3 support is added to the driver more then chip id will be
different for the different Soc. To avoid a lot of if statements in the
code create a struct chip_info to contain this information.
Signed-off-by: Niklas Söderlund
Reviewed-by: Kieran Bingham
---
drivers/media/platform/rcar-vin/rcar-core.c | 49 -
drivers/media/platform/rcar-vin/rcar-v4l2.c | 3 +-
drivers/media/platform/rcar-vin/rcar-vin.h | 12 +--
3 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index aefbe8e3ccddb3e4..dae38de706b66b64 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -277,14 +277,47 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
* Platform Device Driver
*/
+static const struct rvin_info rcar_info_h1 = {
+ .chip = RCAR_H1,
+};
+
+static const struct rvin_info rcar_info_m1 = {
+ .chip = RCAR_M1,
+};
+
+static const struct rvin_info rcar_info_gen2 = {
+ .chip = RCAR_GEN2,
+};
+
static const struct of_device_id rvin_of_id_table[] = {
- { .compatible = "renesas,vin-r8a7794", .data = (void *)RCAR_GEN2 },
- { .compatible = "renesas,vin-r8a7793", .data = (void *)RCAR_GEN2 },
- { .compatible = "renesas,vin-r8a7791", .data = (void *)RCAR_GEN2 },
- { .compatible = "renesas,vin-r8a7790", .data = (void *)RCAR_GEN2 },
- { .compatible = "renesas,vin-r8a7779", .data = (void *)RCAR_H1 },
- { .compatible = "renesas,vin-r8a7778", .data = (void *)RCAR_M1 },
- { .compatible = "renesas,rcar-gen2-vin", .data = (void *)RCAR_GEN2 },
+ {
+ .compatible = "renesas,vin-r8a7794",
+ .data = &rcar_info_gen2,
+ },
+ {
+ .compatible = "renesas,vin-r8a7793",
+ .data = &rcar_info_gen2,
+ },
+ {
+ .compatible = "renesas,vin-r8a7791",
+ .data = &rcar_info_gen2,
+ },
+ {
+ .compatible = "renesas,vin-r8a7790",
+ .data = &rcar_info_gen2,
+ },
+ {
+ .compatible = "renesas,vin-r8a7779",
+ .data = &rcar_info_h1,
+ },
+ {
+ .compatible = "renesas,vin-r8a7778",
+ .data = &rcar_info_m1,
+ },
+ {
+ .compatible = "renesas,rcar-gen2-vin",
+ .data = &rcar_info_gen2,
+ },
{ },
};
MODULE_DEVICE_TABLE(of, rvin_of_id_table);
@@ -305,7 +338,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
return -ENODEV;
vin->dev = &pdev->dev;
- vin->chip = (enum chip_id)match->data;
+ vin->info = match->data;
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (mem == NULL)
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 81ff59c3b4744075..02a08cf5acfce1ce 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -266,7 +266,8 @@ static int __rvin_try_format(struct rvin_dev *vin,
pix->sizeimage = max_t(u32, pix->sizeimage,
rvin_format_sizeimage(pix));
- if (vin->chip == RCAR_M1 && pix->pixelformat == V4L2_PIX_FMT_XBGR32) {
+ if (vin->info->chip == RCAR_M1 &&
+ pix->pixelformat == V4L2_PIX_FMT_XBGR32) {
vin_err(vin, "pixel format XBGR32 not supported on M1\n");
return -EINVAL;
}
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
b/drivers/media/platform/rcar-vin/rcar-vin.h
index 9d0d4a5001b6ccd8..13466dfd72292fc0 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -88,11 +88,19 @@ struct rvin_graph_entity {
unsigned int sink_pad;
};
+/**
+ * struct rvin_info - Information about the particular VIN implementation
+ * @chip: type of VIN chip
+ */
+struct rvin_info {
+ enum chip_id chip;
+};
+
/**
* struct rvin_dev - Renesas VIN device structure
* @dev: (OF) device
* @base: device I/O register space remapped to virtual memory
- * @chip: type of VIN chip
+ * @info: info about VIN instance
*
* @vdev: V4L2 video device associated with VIN
* @v4l2_dev: V4L2 device
@@ -120,7 +128,7 @@ struct rvin_graph_entity {
struct rvin_dev {
struct device *dev;
void __iomem *base;
- enum chip_id chip;
+ const struct rvin_info *info;
struct video_device vdev;
struct v4l2_device v4l2_dev;
--
2.14.0
[PATCH v6 07/25] rcar-vin: all Gen2 boards can scale simplify logic
The logic to preserve the requested format width and height are too
complex and come from a premature optimization for Gen3. All Gen2 SoC
can scale and the Gen3 implementation will not use these functions at
all so simply preserve the width and hight when interacting with the
subdevice much like the field is preserved simplifies the logic quiet a
bit.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-dma.c | 8
drivers/media/platform/rcar-vin/rcar-v4l2.c | 22 ++
drivers/media/platform/rcar-vin/rcar-vin.h | 2 --
3 files changed, 10 insertions(+), 22 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
b/drivers/media/platform/rcar-vin/rcar-dma.c
index 03a79de197d19e43..5f9674dc898305ba 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -585,14 +585,6 @@ void rvin_crop_scale_comp(struct rvin_dev *vin)
0, 0);
}
-void rvin_scale_try(struct rvin_dev *vin, struct v4l2_pix_format *pix,
- u32 width, u32 height)
-{
- /* All VIN channels on Gen2 have scalers */
- pix->width = width;
- pix->height = height;
-}
-
/*
-
* Hardware setup
*/
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index ba88774bd5379a98..affdc128a75e502e 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -166,6 +166,7 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
.which = which,
};
enum v4l2_field field;
+ u32 width, height;
int ret;
sd = vin_to_source(vin);
@@ -178,7 +179,10 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
format.pad = vin->digital.source_pad;
+ /* Allow the video device to override field and to scale */
field = pix->field;
+ width = pix->width;
+ height = pix->height;
ret = v4l2_subdev_call(sd, pad, set_fmt, pad_cfg, &format);
if (ret < 0 && ret != -ENOIOCTLCMD)
@@ -191,6 +195,9 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
source->width = pix->width;
source->height = pix->height;
+ pix->width = width;
+ pix->height = height;
+
vin_dbg(vin, "Source resolution: %ux%u\n", source->width,
source->height);
@@ -204,13 +211,9 @@ static int __rvin_try_format(struct rvin_dev *vin,
struct v4l2_pix_format *pix,
struct rvin_source_fmt *source)
{
- u32 rwidth, rheight, walign;
+ u32 walign;
int ret;
- /* Requested */
- rwidth = pix->width;
- rheight = pix->height;
-
/* Keep current field if no specific one is asked for */
if (pix->field == V4L2_FIELD_ANY)
pix->field = vin->format.field;
@@ -248,10 +251,6 @@ static int __rvin_try_format(struct rvin_dev *vin,
break;
}
- /* If source can't match format try if VIN can scale */
- if (source->width != rwidth || source->height != rheight)
- rvin_scale_try(vin, pix, rwidth, rheight);
-
/* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */
walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
@@ -270,9 +269,8 @@ static int __rvin_try_format(struct rvin_dev *vin,
return -EINVAL;
}
- vin_dbg(vin, "Requested %ux%u Got %ux%u bpl: %d size: %d\n",
- rwidth, rheight, pix->width, pix->height,
- pix->bytesperline, pix->sizeimage);
+ vin_dbg(vin, "Format %ux%u bpl: %d size: %d\n",
+ pix->width, pix->height, pix->bytesperline, pix->sizeimage);
return 0;
}
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
b/drivers/media/platform/rcar-vin/rcar-vin.h
index 2d8b362012ea46a3..b2bac06c0a3cfcb7 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -177,8 +177,6 @@ int rvin_reset_format(struct rvin_dev *vin);
const struct rvin_video_format *rvin_format_from_pixel(u32 pixelformat);
/* Cropping, composing and scaling */
-void rvin_scale_try(struct rvin_dev *vin, struct v4l2_pix_format *pix,
- u32 width, u32 height);
void rvin_crop_scale_comp(struct rvin_dev *vin);
#endif
--
2.14.0
[PATCH v6 13/25] rcar-vin: enable Gen3 hardware configuration
Add the register needed to work with Gen3 hardware. This patch adds
the logic for how to work with the Gen3 hardware. More work is required
to enable the subdevice structure needed to configure capturing.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-dma.c | 94 --
drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
2 files changed, 64 insertions(+), 31 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
b/drivers/media/platform/rcar-vin/rcar-dma.c
index 9362e7dba5e3ba95..c4f8e81e88c99e28 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -33,21 +33,23 @@
#define VNELPRC_REG0x10/* Video n End Line Pre-Clip Register */
#define VNSPPRC_REG0x14/* Video n Start Pixel Pre-Clip Register */
#define VNEPPRC_REG0x18/* Video n End Pixel Pre-Clip Register */
-#define VNSLPOC_REG0x1C/* Video n Start Line Post-Clip Register */
-#define VNELPOC_REG0x20/* Video n End Line Post-Clip Register */
-#define VNSPPOC_REG0x24/* Video n Start Pixel Post-Clip Register */
-#define VNEPPOC_REG0x28/* Video n End Pixel Post-Clip Register */
#define VNIS_REG 0x2C/* Video n Image Stride Register */
#define VNMB_REG(m)(0x30 + ((m) << 2)) /* Video n Memory Base m Register */
#define VNIE_REG 0x40/* Video n Interrupt Enable Register */
#define VNINTS_REG 0x44/* Video n Interrupt Status Register */
#define VNSI_REG 0x48/* Video n Scanline Interrupt Register */
#define VNMTC_REG 0x4C/* Video n Memory Transfer Control Register */
-#define VNYS_REG 0x50/* Video n Y Scale Register */
-#define VNXS_REG 0x54/* Video n X Scale Register */
#define VNDMR_REG 0x58/* Video n Data Mode Register */
#define VNDMR2_REG 0x5C/* Video n Data Mode Register 2 */
#define VNUVAOF_REG0x60/* Video n UV Address Offset Register */
+
+/* Register offsets specific for Gen2 */
+#define VNSLPOC_REG0x1C/* Video n Start Line Post-Clip Register */
+#define VNELPOC_REG0x20/* Video n End Line Post-Clip Register */
+#define VNSPPOC_REG0x24/* Video n Start Pixel Post-Clip Register */
+#define VNEPPOC_REG0x28/* Video n End Pixel Post-Clip Register */
+#define VNYS_REG 0x50/* Video n Y Scale Register */
+#define VNXS_REG 0x54/* Video n X Scale Register */
#define VNC1A_REG 0x80/* Video n Coefficient Set C1A Register */
#define VNC1B_REG 0x84/* Video n Coefficient Set C1B Register */
#define VNC1C_REG 0x88/* Video n Coefficient Set C1C Register */
@@ -73,9 +75,13 @@
#define VNC8B_REG 0xF4/* Video n Coefficient Set C8B Register */
#define VNC8C_REG 0xF8/* Video n Coefficient Set C8C Register */
+/* Register offsets specific for Gen3 */
+#define VNCSI_IFMD_REG 0x20 /* Video n CSI2 Interface Mode Register */
/* Register bit fields for R-Car VIN */
/* Video n Main Control Register bits */
+#define VNMC_DPINE (1 << 27) /* Gen3 specific */
+#define VNMC_SCLE (1 << 26) /* Gen3 specific */
#define VNMC_FOC (1 << 21)
#define VNMC_YCAL (1 << 19)
#define VNMC_INF_YUV8_BT656(0 << 16)
@@ -119,6 +125,13 @@
#define VNDMR2_FTEV(1 << 17)
#define VNDMR2_VLV(n) ((n & 0xf) << 12)
+/* Video n CSI2 Interface Mode Register (Gen3) */
+#define VNCSI_IFMD_DES2(1 << 27)
+#define VNCSI_IFMD_DES1(1 << 26)
+#define VNCSI_IFMD_DES0(1 << 25)
+#define VNCSI_IFMD_CSI_CHSEL(n) ((n & 0xf) << 0)
+#define VNCSI_IFMD_CSI_CHSEL_MASK 0xf
+
struct rvin_buffer {
struct vb2_v4l2_buffer vb;
struct list_head list;
@@ -514,28 +527,10 @@ static void rvin_set_coeff(struct rvin_dev *vin, unsigned
short xs)
rvin_write(vin, p_set->coeff_set[23], VNC8C_REG);
}
-static void rvin_crop_scale_comp(struct rvin_dev *vin)
+static void rvin_crop_scale_comp_gen2(struct rvin_dev *vin)
{
u32 xs, ys;
- /* Set Start/End Pixel/Line Pre-Clip */
- rvin_write(vin, vin->crop.left, VNSPPRC_REG);
- rvin_write(vin, vin->crop.left + vin->crop.width - 1, VNEPPRC_REG);
- switch (vin->format.field) {
- case V4L2_FIELD_INTERLACED:
- case V4L2_FIELD_INTERLACED_TB:
- case V4L2_FIELD_INTERLACED_BT:
- rvin_write(vin, vin->crop.top / 2, VNSLPRC_REG);
- rvin_write(vin, (vin->crop.top + vin->crop.height) / 2 - 1,
- VNELPRC_REG);
- break;
- default:
- rvin_write(vin, vin->crop.top, VNSLPRC_REG);
- rvin_write(vin, vin->crop.top + vin->crop.height - 1,
- VNELPRC_REG);
- break;
- }
-
/* Set scaling coefficient */
ys = 0;
if (vin->crop.height != vin->compose.height)
@@ -573,11 +568,6 @@
[PATCH v6 11/25] rcar-vin: fix handling of single field frames (top, bottom and alternate fields)
It was never proper support in the VIN driver to deliver ALTERNATING
field format to user-space, remove this field option. For sources using
this field format instead use the VIN hardware feature of combining the
fields to an interlaced format. This mode of operation was previously
the default behavior and ALTERNATING was only delivered to user-space if
explicitly requested. Allowing this to be explicitly requested was a
mistake and was never properly tested and never worked due to the
constrains put on the field format when it comes to sequence numbers and
timestamps etc.
The height should not be cut in half for the format for TOP or BOTTOM
fields settings. This was a mistake and it was made visible by the
scaling refactoring. Correct behavior is that the user should request a
frame size that fits the half height frame reflected in the field
setting. If not the VIN will do it's best to scale the top or bottom to
the requested format and cropping and scaling do not work as expected.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-dma.c | 15 +
drivers/media/platform/rcar-vin/rcar-v4l2.c | 48 +++--
2 files changed, 19 insertions(+), 44 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
b/drivers/media/platform/rcar-vin/rcar-dma.c
index 6cc880e5ef7e0718..f22bec062db31772 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -617,7 +617,6 @@ static int rvin_setup(struct rvin_dev *vin)
case V4L2_FIELD_INTERLACED_BT:
vnmc = VNMC_IM_FULL | VNMC_FOC;
break;
- case V4L2_FIELD_ALTERNATE:
case V4L2_FIELD_NONE:
if (vin->continuous) {
vnmc = VNMC_IM_ODD_EVEN;
@@ -757,18 +756,6 @@ static int rvin_get_active_slot(struct rvin_dev *vin, u32
vnms)
return 0;
}
-static enum v4l2_field rvin_get_active_field(struct rvin_dev *vin, u32 vnms)
-{
- if (vin->format.field == V4L2_FIELD_ALTERNATE) {
- /* If FS is set it's a Even field */
- if (vnms & VNMS_FS)
- return V4L2_FIELD_BOTTOM;
- return V4L2_FIELD_TOP;
- }
-
- return vin->format.field;
-}
-
static void rvin_set_slot_addr(struct rvin_dev *vin, int slot, dma_addr_t addr)
{
const struct rvin_video_format *fmt;
@@ -941,7 +928,7 @@ static irqreturn_t rvin_irq(int irq, void *data)
goto done;
/* Capture frame */
- vin->queue_buf[slot]->field = rvin_get_active_field(vin, vnms);
+ vin->queue_buf[slot]->field = vin->format.field;
vin->queue_buf[slot]->sequence = sequence;
vin->queue_buf[slot]->vb2_buf.timestamp = ktime_get_ns();
vb2_buffer_done(&vin->queue_buf[slot]->vb2_buf, VB2_BUF_STATE_DONE);
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index c8c764188b85a926..9f0aac9c3398d613 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -102,6 +102,24 @@ static int rvin_get_sd_format(struct rvin_dev *vin, struct
v4l2_pix_format *pix)
if (ret)
return ret;
+ switch (fmt.format.field) {
+ case V4L2_FIELD_TOP:
+ case V4L2_FIELD_BOTTOM:
+ case V4L2_FIELD_NONE:
+ case V4L2_FIELD_INTERLACED_TB:
+ case V4L2_FIELD_INTERLACED_BT:
+ case V4L2_FIELD_INTERLACED:
+ break;
+ case V4L2_FIELD_ALTERNATE:
+ /* Use VIN hardware to combine the two fields */
+ fmt.format.field = V4L2_FIELD_INTERLACED;
+ fmt.format.height *= 2;
+ break;
+ default:
+ vin->format.field = V4L2_FIELD_NONE;
+ break;
+ }
+
v4l2_fill_pix_format(pix, &fmt.format);
return 0;
@@ -115,33 +133,6 @@ int rvin_reset_format(struct rvin_dev *vin)
if (ret)
return ret;
- /*
-* If the subdevice uses ALTERNATE field mode and G_STD is
-* implemented use the VIN HW to combine the two fields to
-* one INTERLACED frame. The ALTERNATE field mode can still
-* be requested in S_FMT and be respected, this is just the
-* default which is applied at probing or when S_STD is called.
-*/
- if (vin->format.field == V4L2_FIELD_ALTERNATE &&
- v4l2_subdev_has_op(vin_to_source(vin), video, g_std))
- vin->format.field = V4L2_FIELD_INTERLACED;
-
- switch (vin->format.field) {
- case V4L2_FIELD_TOP:
- case V4L2_FIELD_BOTTOM:
- case V4L2_FIELD_ALTERNATE:
- vin->format.height /= 2;
- break;
- case V4L2_FIELD_NONE:
- case V4L2_FIELD_INTERLACED_TB:
- case V4L2_FIELD_INTERLACED_BT:
- case V4L2_FIELD_INTERLACED:
- break;
- default:
- vin->format.field = V4
[PATCH v6 16/25] rcar-vin: break out format alignment and checking
Part of the format aliment and checking can be shared with the Gen3
format handling. Break that part out to it's own function. While doing
this clean up the checking and add more checks.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-v4l2.c | 98 +++--
1 file changed, 51 insertions(+), 47 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index fb9f802e553e9b80..f71dea8d5d40323a 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -86,6 +86,56 @@ static u32 rvin_format_sizeimage(struct v4l2_pix_format *pix)
return pix->bytesperline * pix->height;
}
+static int rvin_format_align(struct rvin_dev *vin, struct v4l2_pix_format *pix)
+{
+ u32 walign;
+
+ /* If requested format is not supported fallback to the default */
+ if (!rvin_format_from_pixel(pix->pixelformat)) {
+ vin_dbg(vin, "Format 0x%x not found, using default 0x%x\n",
+ pix->pixelformat, RVIN_DEFAULT_FORMAT);
+ pix->pixelformat = RVIN_DEFAULT_FORMAT;
+ }
+
+ switch (pix->field) {
+ case V4L2_FIELD_TOP:
+ case V4L2_FIELD_BOTTOM:
+ case V4L2_FIELD_NONE:
+ case V4L2_FIELD_INTERLACED_TB:
+ case V4L2_FIELD_INTERLACED_BT:
+ case V4L2_FIELD_INTERLACED:
+ break;
+ default:
+ pix->field = V4L2_FIELD_NONE;
+ break;
+ }
+
+ /* Check that colorspace is resonable, if not keep current */
+ if (!pix->colorspace || pix->colorspace >= 0xff)
+ pix->colorspace = vin->format.colorspace;
+
+ /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */
+ walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
+
+ /* Limit to VIN capabilities */
+ v4l_bound_align_image(&pix->width, 2, vin->info->max_width, walign,
+ &pix->height, 4, vin->info->max_height, 2, 0);
+
+ pix->bytesperline = rvin_format_bytesperline(pix);
+ pix->sizeimage = rvin_format_sizeimage(pix);
+
+ if (vin->info->chip == RCAR_M1 &&
+ pix->pixelformat == V4L2_PIX_FMT_XBGR32) {
+ vin_err(vin, "pixel format XBGR32 not supported on M1\n");
+ return -EINVAL;
+ }
+
+ vin_dbg(vin, "Format %ux%u bpl: %d size: %d\n",
+ pix->width, pix->height, pix->bytesperline, pix->sizeimage);
+
+ return 0;
+}
+
/*
-
* V4L2
*/
@@ -191,64 +241,18 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
static int __rvin_try_format(struct rvin_dev *vin,
u32 which, struct v4l2_pix_format *pix)
{
- u32 walign;
int ret;
/* Keep current field if no specific one is asked for */
if (pix->field == V4L2_FIELD_ANY)
pix->field = vin->format.field;
- /* If requested format is not supported fallback to the default */
- if (!rvin_format_from_pixel(pix->pixelformat)) {
- vin_dbg(vin, "Format 0x%x not found, using default 0x%x\n",
- pix->pixelformat, RVIN_DEFAULT_FORMAT);
- pix->pixelformat = RVIN_DEFAULT_FORMAT;
- }
-
- /* Always recalculate */
- pix->bytesperline = 0;
- pix->sizeimage = 0;
-
/* Limit to source capabilities */
ret = __rvin_try_format_source(vin, which, pix);
if (ret)
return ret;
- switch (pix->field) {
- case V4L2_FIELD_TOP:
- case V4L2_FIELD_BOTTOM:
- case V4L2_FIELD_NONE:
- case V4L2_FIELD_INTERLACED_TB:
- case V4L2_FIELD_INTERLACED_BT:
- case V4L2_FIELD_INTERLACED:
- break;
- default:
- pix->field = V4L2_FIELD_NONE;
- break;
- }
-
- /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */
- walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
-
- /* Limit to VIN capabilities */
- v4l_bound_align_image(&pix->width, 2, vin->info->max_width, walign,
- &pix->height, 4, vin->info->max_height, 2, 0);
-
- pix->bytesperline = max_t(u32, pix->bytesperline,
- rvin_format_bytesperline(pix));
- pix->sizeimage = max_t(u32, pix->sizeimage,
- rvin_format_sizeimage(pix));
-
- if (vin->info->chip == RCAR_M1 &&
- pix->pixelformat == V4L2_PIX_FMT_XBGR32) {
- vin_err(vin, "pixel format XBGR32 not supported on M1\n");
- return -EINVAL;
- }
-
- vin_dbg(vin, "Format %ux%u bpl: %d size: %d\n",
- pix->width, pix->height, pix->bytesperline, pix->sizeimage);
-
- return 0;
+ return rvin_format_align(vin, pix);
}
[PATCH v6 14/25] rcar-vin: add functions to manipulate Gen3 CHSEL value
On Gen3 the CSI-2 routing is controlled by the VnCSI_IFMD register. One
feature of this register is that it's only present in the VIN0 and VIN4
instances. The register in VIN0 controls the routing for VIN0-3 and the
register in VIN4 controls routing for VIN4-7.
To be able to control routing from a media device these functions need
to control runtime PM for the subgroup master (VIN0 and VIN4). The
subgroup master must be switched on before the register is manipulated,
once the operation is complete it's safe to switch the master off and
the new routing will still be in effect.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-dma.c | 41 ++
drivers/media/platform/rcar-vin/rcar-vin.h | 3 +++
2 files changed, 44 insertions(+)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
b/drivers/media/platform/rcar-vin/rcar-dma.c
index c4f8e81e88c99e28..6206fab7b6cdc55a 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -16,6 +16,7 @@
#include
#include
+#include
#include
@@ -1228,3 +1229,43 @@ int rvin_dma_probe(struct rvin_dev *vin, int irq)
return ret;
}
+
+/*
-
+ * Gen3 CHSEL manipulation
+ */
+
+void rvin_set_chsel(struct rvin_dev *vin, u8 chsel)
+{
+ u32 ifmd;
+
+ pm_runtime_get_sync(vin->dev);
+
+ /*
+* Undocumented feature: Writing to VNCSI_IFMD_REG will go
+* through and on read back look correct but won't have
+* any effect if VNMC_REG is not first set to 0.
+*/
+ rvin_write(vin, 0, VNMC_REG);
+
+ ifmd = VNCSI_IFMD_DES2 | VNCSI_IFMD_DES1 | VNCSI_IFMD_DES0 |
+ VNCSI_IFMD_CSI_CHSEL(chsel);
+
+ rvin_write(vin, ifmd, VNCSI_IFMD_REG);
+
+ vin_dbg(vin, "Set IFMD 0x%x\n", ifmd);
+
+ pm_runtime_put(vin->dev);
+}
+
+int rvin_get_chsel(struct rvin_dev *vin)
+{
+ int chsel;
+
+ pm_runtime_get_sync(vin->dev);
+
+ chsel = rvin_read(vin, VNCSI_IFMD_REG) & VNCSI_IFMD_CSI_CHSEL_MASK;
+
+ pm_runtime_put(vin->dev);
+
+ return chsel;
+}
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
b/drivers/media/platform/rcar-vin/rcar-vin.h
index c460868d2d81..94c606f2b8f2f246 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -164,4 +164,7 @@ int rvin_reset_format(struct rvin_dev *vin);
const struct rvin_video_format *rvin_format_from_pixel(u32 pixelformat);
+void rvin_set_chsel(struct rvin_dev *vin, u8 chsel);
+int rvin_get_chsel(struct rvin_dev *vin);
+
#endif
--
2.14.0
[PATCH v6 21/25] rcar-vin: parse Gen3 OF and setup media graph
Parse the VIN Gen3 OF graph and register all CSI-2 devices in the VIN
group common media device. Once all CSI-2 subdevice is added to the
common media device create links between them.
The parsing and registering CSI-2 subdevices with the v4l2 async
framework is a collaborative effort shared between the VIN instances
which are part of the group. The fist rcar-vin instance parses OF and
finds all other VIN and CSI-2 nodes which are part of the graph. It
stores a bit mask of all VIN instances found and handles to all CSI-2
nodes.
The bit mask is used to figure out when all VIN instances have been
probed. Once the last VIN instance is probed this is detected and this
instance registers all CSI-2 subdevices in its private async notifier.
Once the .complete() callback of this notifier is called it creates the
media controller links between all entities in the graph.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-core.c | 296 +++-
drivers/media/platform/rcar-vin/rcar-vin.h | 7 +-
2 files changed, 301 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index 4218a73eb6885486..2aba442a0750e91a 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -440,10 +440,268 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
* Group async notifier
*/
-static int rvin_group_init(struct rvin_dev *vin)
+/* group lock should be held when calling this function */
+static int rvin_group_add_link(struct rvin_dev *vin,
+ struct media_entity *source,
+ unsigned int source_idx,
+ struct media_entity *sink,
+ unsigned int sink_idx,
+ u32 flags)
+{
+ struct media_pad *source_pad, *sink_pad;
+ int ret = 0;
+
+ source_pad = &source->pads[source_idx];
+ sink_pad = &sink->pads[sink_idx];
+
+ if (!media_entity_find_link(source_pad, sink_pad))
+ ret = media_create_pad_link(source, source_idx,
+ sink, sink_idx, flags);
+
+ if (ret)
+ vin_err(vin, "Error adding link from %s to %s\n",
+ source->name, sink->name);
+
+ return ret;
+}
+
+static int rvin_group_update_links(struct rvin_dev *vin)
{
+ struct media_entity *source, *sink;
+ struct rvin_dev *master;
+ unsigned int i, n, idx, chsel, csi;
+ u32 flags;
int ret;
+ mutex_lock(&vin->group->lock);
+
+ for (n = 0; n < RCAR_VIN_NUM; n++) {
+
+ /* Check that VIN is part of the group */
+ if (!vin->group->vin[n])
+ continue;
+
+ /* Check that subgroup master is part of the group */
+ master = vin->group->vin[n < 4 ? 0 : 4];
+ if (!master)
+ continue;
+
+ chsel = rvin_get_chsel(master);
+
+ for (i = 0; i < vin->info->num_chsels; i++) {
+ csi = vin->info->chsels[n][i].csi;
+
+ /* If the CSI-2 is out of bounds it's a noop, skip */
+ if (csi >= RVIN_CSI_MAX)
+ continue;
+
+ /* Check that CSI-2 are part of the group */
+ if (!vin->group->csi[csi].subdev)
+ continue;
+
+ source = &vin->group->csi[csi].subdev->entity;
+ sink = &vin->group->vin[n]->vdev.entity;
+ idx = vin->info->chsels[n][i].chan + 1;
+ flags = i == chsel ? MEDIA_LNK_FL_ENABLED : 0;
+
+ ret = rvin_group_add_link(vin, source, idx, sink, 0,
+ flags);
+ if (ret)
+ goto out;
+ }
+ }
+out:
+ mutex_unlock(&vin->group->lock);
+
+ return ret;
+}
+
+static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
+{
+ struct rvin_dev *vin = notifier_to_vin(notifier);
+ int ret;
+
+ ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
+ if (ret) {
+ vin_err(vin, "Failed to register subdev nodes\n");
+ return ret;
+ }
+
+ return rvin_group_update_links(vin);
+}
+
+static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
+struct v4l2_subdev *subdev,
+struct v4l2_async_subdev *asd)
+{
+ struct rvin_dev *vin = notifier_to_vin(notifier);
+ struct rvin_graph_entity *csi = to_rvin_graph_entity(asd);
+
+ mutex_lock(&vin->group->lock);
+ csi->subdev = NULL;
+ mutex_unlock(&vin->group->lock);
[PATCH v6 24/25] rcar-vin: enable support for r8a7795
Add the SoC specific information for Renesas r8a7795.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/Kconfig | 2 +-
drivers/media/platform/rcar-vin/rcar-core.c | 145
2 files changed, 146 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/rcar-vin/Kconfig
b/drivers/media/platform/rcar-vin/Kconfig
index af4c98b44d2e22cb..8fa7ee468c63afb9 100644
--- a/drivers/media/platform/rcar-vin/Kconfig
+++ b/drivers/media/platform/rcar-vin/Kconfig
@@ -6,7 +6,7 @@ config VIDEO_RCAR_VIN
select V4L2_FWNODE
---help---
Support for Renesas R-Car Video Input (VIN) driver.
- Supports R-Car Gen2 SoCs.
+ Supports R-Car Gen2 and Gen3 SoCs.
To compile this driver as a module, choose M here: the
module will be called rcar-vin.
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index dec91e2f3ccdbd93..58d903ab9fb83faf 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
@@ -987,7 +988,139 @@ static const struct rvin_info rcar_info_gen2 = {
.max_height = 2048,
};
+static const struct rvin_info rcar_info_r8a7795 = {
+ .chip = RCAR_GEN3,
+ .use_mc = true,
+ .max_width = 4096,
+ .max_height = 4096,
+
+ .num_chsels = 5,
+ .chsels = {
+ {
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 1 },
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ }, {
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 1 },
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 1 },
+ { .csi = RVIN_CSI20, .chan = 1 },
+ }, {
+ { .csi = RVIN_CSI20, .chan = 1 },
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 2 },
+ { .csi = RVIN_CSI20, .chan = 2 },
+ }, {
+ { .csi = RVIN_CSI40, .chan = 1 },
+ { .csi = RVIN_CSI20, .chan = 1 },
+ { .csi = RVIN_CSI20, .chan = 1 },
+ { .csi = RVIN_CSI40, .chan = 3 },
+ { .csi = RVIN_CSI20, .chan = 3 },
+ }, {
+ { .csi = RVIN_CSI41, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_CSI41, .chan = 1 },
+ { .csi = RVIN_CSI41, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ }, {
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_CSI41, .chan = 1 },
+ { .csi = RVIN_CSI41, .chan = 0 },
+ { .csi = RVIN_CSI41, .chan = 1 },
+ { .csi = RVIN_CSI20, .chan = 1 },
+ }, {
+ { .csi = RVIN_CSI20, .chan = 1 },
+ { .csi = RVIN_CSI41, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_CSI41, .chan = 2 },
+ { .csi = RVIN_CSI20, .chan = 2 },
+ }, {
+ { .csi = RVIN_CSI41, .chan = 1 },
+ { .csi = RVIN_CSI20, .chan = 1 },
+ { .csi = RVIN_CSI20, .chan = 1 },
+ { .csi = RVIN_CSI41, .chan = 3 },
+ { .csi = RVIN_CSI20, .chan = 3 },
+ },
+ },
+};
+
+static const struct rvin_info rcar_info_r8a7795es1 = {
+ .chip = RCAR_GEN3,
+ .use_mc = true,
+ .max_width = 4096,
+ .max_height = 4096,
+
+ .num_chsels = 6,
+ .chsels = {
+ {
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_CSI21, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_CSI21, .chan = 0 },
+ }, {
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_CSI21, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 1 },
+ { .csi = RVIN_CSI20, .chan = 1 },
+ { .csi = RVIN_CSI21, .chan = 1 },
+ }, {
+
[PATCH v6 12/25] rcar-vin: move media bus configuration to struct rvin_info
Bus configuration will once the driver is extended to to support Gen3
contain information not specific to only the directly connected parallel
subdevice. Move it to struct rvin_info to show it's not always coupled
to the parallel subdevice.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-core.c | 14 +++---
drivers/media/platform/rcar-vin/rcar-dma.c | 11 ++-
drivers/media/platform/rcar-vin/rcar-v4l2.c | 2 +-
drivers/media/platform/rcar-vin/rcar-vin.h | 9 -
4 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index 4dc148e7835439ab..65f01b6781c0aefd 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -45,15 +45,15 @@ static int rvin_find_pad(struct v4l2_subdev *sd, int
direction)
return -EINVAL;
}
-static bool rvin_mbus_supported(struct rvin_graph_entity *entity)
+static bool rvin_mbus_supported(struct rvin_dev *vin)
{
- struct v4l2_subdev *sd = entity->subdev;
+ struct v4l2_subdev *sd = vin->digital.subdev;
struct v4l2_subdev_mbus_code_enum code = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
};
code.index = 0;
- code.pad = entity->source_pad;
+ code.pad = vin->digital.source_pad;
while (!v4l2_subdev_call(sd, pad, enum_mbus_code, NULL, &code)) {
code.index++;
switch (code.code) {
@@ -61,7 +61,7 @@ static bool rvin_mbus_supported(struct rvin_graph_entity
*entity)
case MEDIA_BUS_FMT_UYVY8_2X8:
case MEDIA_BUS_FMT_UYVY10_2X10:
case MEDIA_BUS_FMT_RGB888_1X24:
- entity->code = code.code;
+ vin->code = code.code;
return true;
default:
break;
@@ -78,14 +78,14 @@ static int rvin_digital_notify_complete(struct
v4l2_async_notifier *notifier)
int ret;
/* Verify subdevices mbus format */
- if (!rvin_mbus_supported(&vin->digital)) {
+ if (!rvin_mbus_supported(vin)) {
vin_err(vin, "Unsupported media bus format for %s\n",
vin->digital.subdev->name);
return -EINVAL;
}
vin_dbg(vin, "Found media bus format for %s: %d\n",
- vin->digital.subdev->name, vin->digital.code);
+ vin->digital.subdev->name, vin->code);
ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
if (ret < 0) {
@@ -219,7 +219,7 @@ static int rvin_digital_graph_parse(struct rvin_dev *vin)
}
of_node_put(np);
- ret = rvin_digitial_parse_v4l2(vin, ep, &vin->digital.mbus_cfg);
+ ret = rvin_digitial_parse_v4l2(vin, ep, &vin->mbus_cfg);
of_node_put(ep);
if (ret)
return ret;
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
b/drivers/media/platform/rcar-vin/rcar-dma.c
index f22bec062db31772..9362e7dba5e3ba95 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -633,7 +633,7 @@ static int rvin_setup(struct rvin_dev *vin)
/*
* Input interface
*/
- switch (vin->digital.code) {
+ switch (vin->code) {
case MEDIA_BUS_FMT_YUYV8_1X16:
/* BT.601/BT.1358 16bit YCbCr422 */
vnmc |= VNMC_INF_YUV16;
@@ -641,7 +641,7 @@ static int rvin_setup(struct rvin_dev *vin)
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
- vnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ?
+ vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
input_is_yuv = true;
break;
@@ -650,7 +650,7 @@ static int rvin_setup(struct rvin_dev *vin)
break;
case MEDIA_BUS_FMT_UYVY10_2X10:
/* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
- vnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ?
+ vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
input_is_yuv = true;
break;
@@ -662,11 +662,11 @@ static int rvin_setup(struct rvin_dev *vin)
dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);
/* Hsync Signal Polarity Select */
- if (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
+ if (!(vin->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
dmr2 |= VNDMR2_HPS;
/* Vsync Signal Polarity Select */
- if (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
+ if (!(vin->mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
dmr2 |= VNDMR2_VPS;
/*
@@ -8
[PATCH v6 17/25] rcar-vin: use different v4l2 operations in media controller mode
When the driver runs in media controller mode it should not directly
control the subdevice instead userspace will be responsible for
configuring the pipeline. To be able to run in this mode a different set
of v4l2 operations needs to be used.
Add a new set of v4l2 operations to support the running without directly
interacting with the source subdevice.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-dma.c | 3 +-
drivers/media/platform/rcar-vin/rcar-v4l2.c | 155 +++-
drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
3 files changed, 155 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
b/drivers/media/platform/rcar-vin/rcar-dma.c
index 6206fab7b6cdc55a..de1486ee2786b499 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -628,7 +628,8 @@ static int rvin_setup(struct rvin_dev *vin)
/* Default to TB */
vnmc = VNMC_IM_FULL;
/* Use BT if video standard can be read and is 60 Hz format */
- if (!v4l2_subdev_call(vin_to_source(vin), video, g_std, &std)) {
+ if (!vin->info->use_mc &&
+ !v4l2_subdev_call(vin_to_source(vin), video, g_std, &std)) {
if (std & V4L2_STD_525_60)
vnmc = VNMC_IM_FULL | VNMC_FOC;
}
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index f71dea8d5d40323a..9d540644bbe446f6 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -23,6 +23,9 @@
#include "rcar-vin.h"
#define RVIN_DEFAULT_FORMATV4L2_PIX_FMT_YUYV
+#define RVIN_DEFAULT_WIDTH 800
+#define RVIN_DEFAULT_HEIGHT600
+#define RVIN_DEFAULT_COLORSPACEV4L2_COLORSPACE_SRGB
/*
-
* Format Conversions
@@ -671,6 +674,84 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
};
+/*
-
+ * V4L2 Media Controller
+ */
+
+static int __rvin_mc_try_format(struct rvin_dev *vin,
+ struct v4l2_pix_format *pix)
+{
+ /* Keep current field if no specific one is asked for */
+ if (pix->field == V4L2_FIELD_ANY)
+ pix->field = vin->format.field;
+
+ return rvin_format_align(vin, pix);
+}
+
+static int rvin_mc_try_fmt_vid_cap(struct file *file, void *priv,
+ struct v4l2_format *f)
+{
+ struct rvin_dev *vin = video_drvdata(file);
+
+ return __rvin_mc_try_format(vin, &f->fmt.pix);
+}
+
+static int rvin_mc_s_fmt_vid_cap(struct file *file, void *priv,
+struct v4l2_format *f)
+{
+ struct rvin_dev *vin = video_drvdata(file);
+ int ret;
+
+ if (vb2_is_busy(&vin->queue))
+ return -EBUSY;
+
+ ret = __rvin_mc_try_format(vin, &f->fmt.pix);
+ if (ret)
+ return ret;
+
+ vin->format = f->fmt.pix;
+
+ return 0;
+}
+
+static int rvin_mc_enum_input(struct file *file, void *priv,
+ struct v4l2_input *i)
+{
+ if (i->index != 0)
+ return -EINVAL;
+
+ i->type = V4L2_INPUT_TYPE_CAMERA;
+ strlcpy(i->name, "Camera", sizeof(i->name));
+
+ return 0;
+}
+
+static const struct v4l2_ioctl_ops rvin_mc_ioctl_ops = {
+ .vidioc_querycap= rvin_querycap,
+ .vidioc_try_fmt_vid_cap = rvin_mc_try_fmt_vid_cap,
+ .vidioc_g_fmt_vid_cap = rvin_g_fmt_vid_cap,
+ .vidioc_s_fmt_vid_cap = rvin_mc_s_fmt_vid_cap,
+ .vidioc_enum_fmt_vid_cap= rvin_enum_fmt_vid_cap,
+
+ .vidioc_enum_input = rvin_mc_enum_input,
+ .vidioc_g_input = rvin_g_input,
+ .vidioc_s_input = rvin_s_input,
+
+ .vidioc_reqbufs = vb2_ioctl_reqbufs,
+ .vidioc_create_bufs = vb2_ioctl_create_bufs,
+ .vidioc_querybuf= vb2_ioctl_querybuf,
+ .vidioc_qbuf= vb2_ioctl_qbuf,
+ .vidioc_dqbuf = vb2_ioctl_dqbuf,
+ .vidioc_expbuf = vb2_ioctl_expbuf,
+ .vidioc_prepare_buf = vb2_ioctl_prepare_buf,
+ .vidioc_streamon= vb2_ioctl_streamon,
+ .vidioc_streamoff = vb2_ioctl_streamoff,
+
+ .vidioc_log_status = v4l2_ctrl_log_status,
+ .vidioc_subscribe_event = rvin_subscribe_event,
+ .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
+};
+
/*
-
* File Operations
*/
@@ -819
[PATCH v6 19/25] rcar-vin: add group allocator functions
In media controller mode all VIN instances needs to be part of the same
media graph. There is also a need to each VIN instance to know and in
some cases be able to communicate with other VIN instances.
Add a allocator framework where the first VIN instance to be probed
creates a shared data structure and creates a media device.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-core.c | 179 +++-
drivers/media/platform/rcar-vin/rcar-vin.h | 38 ++
2 files changed, 216 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index dd0525f2ba336bc2..4218a73eb6885486 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -20,11 +20,170 @@
#include
#include
#include
+#include
#include
#include "rcar-vin.h"
+/*
-
+ * Gen3 CSI2 Group Allocator
+ */
+
+static int rvin_group_read_id(struct rvin_dev *vin, struct device_node *np)
+{
+ u32 val;
+ int ret;
+
+ ret = of_property_read_u32(np, "renesas,id", &val);
+ if (ret) {
+ vin_err(vin, "%s: No renesas,id property found\n",
+ of_node_full_name(np));
+ return -EINVAL;
+ }
+
+ if (val >= RCAR_VIN_NUM) {
+ vin_err(vin, "%s: Invalid renesas,id '%u'\n",
+ of_node_full_name(np), val);
+ return -EINVAL;
+ }
+
+ return val;
+}
+
+static DEFINE_MUTEX(rvin_group_lock);
+static struct rvin_group *rvin_group_data;
+
+static void rvin_group_release(struct kref *kref)
+{
+ struct rvin_group *group =
+ container_of(kref, struct rvin_group, refcount);
+
+ mutex_lock(&rvin_group_lock);
+
+ media_device_unregister(&group->mdev);
+ media_device_cleanup(&group->mdev);
+
+ rvin_group_data = NULL;
+
+ mutex_unlock(&rvin_group_lock);
+
+ kfree(group);
+}
+
+static struct rvin_group *__rvin_group_allocate(struct rvin_dev *vin)
+{
+ struct rvin_group *group;
+
+ if (rvin_group_data) {
+ group = rvin_group_data;
+ kref_get(&group->refcount);
+ vin_dbg(vin, "%s: get group=%p\n", __func__, group);
+ return group;
+ }
+
+ group = kzalloc(sizeof(*group), GFP_KERNEL);
+ if (!group)
+ return NULL;
+
+ kref_init(&group->refcount);
+ rvin_group_data = group;
+
+ vin_dbg(vin, "%s: alloc group=%p\n", __func__, group);
+ return group;
+}
+
+static int rvin_group_add_vin(struct rvin_dev *vin)
+{
+ int ret;
+
+ ret = rvin_group_read_id(vin, vin->dev->of_node);
+ if (ret < 0)
+ return ret;
+
+ mutex_lock(&vin->group->lock);
+
+ if (vin->group->vin[ret]) {
+ mutex_unlock(&vin->group->lock);
+ vin_err(vin, "VIN number %d already occupied\n", ret);
+ return -EINVAL;
+ }
+
+ vin->group->vin[ret] = vin;
+
+ mutex_unlock(&vin->group->lock);
+
+ vin_dbg(vin, "I'm VIN number %d", ret);
+
+ return 0;
+}
+
+static int rvin_group_allocate(struct rvin_dev *vin)
+{
+ struct rvin_group *group;
+ struct media_device *mdev;
+ int ret;
+
+ mutex_lock(&rvin_group_lock);
+
+ group = __rvin_group_allocate(vin);
+ if (!group) {
+ mutex_unlock(&rvin_group_lock);
+ return -ENOMEM;
+ }
+
+ /* Init group data if its not already initialized */
+ mdev = &group->mdev;
+ if (!mdev->dev) {
+ mutex_init(&group->lock);
+ mdev->dev = vin->dev;
+
+ strlcpy(mdev->driver_name, "Renesas VIN",
+ sizeof(mdev->driver_name));
+ strlcpy(mdev->model, vin->dev->of_node->name,
+ sizeof(mdev->model));
+ strlcpy(mdev->bus_info, of_node_full_name(vin->dev->of_node),
+ sizeof(mdev->bus_info));
+ media_device_init(mdev);
+
+ ret = media_device_register(mdev);
+ if (ret) {
+ vin_err(vin, "Failed to register media device\n");
+ kref_put(&group->refcount, rvin_group_release);
+ mutex_unlock(&rvin_group_lock);
+ return ret;
+ }
+ }
+
+ vin->group = group;
+ vin->v4l2_dev.mdev = mdev;
+
+ ret = rvin_group_add_vin(vin);
+ if (ret) {
+ kref_put(&group->refcount, rvin_group_release);
+ mutex_unlock(&rvin_group_lock);
+ return ret;
+ }
+
+ mutex_unlock(&rvin_group_lock);
+
+ return 0;
+}
+
+static void rvin_group_delete(struct rvin_dev *vin)
+{
+ unsigned int i;
+
+ mutex_lock(&vin->group->l
[PATCH v6 10/25] rcar-vin: read subdevice format for crop only when needed
Instead of caching the subdevice format each time the video device
format is set read it directly when its needed. As it turns out the
format is only needed when figuring out the max rectangle for cropping.
This simplify the code and makes it clearer what the source format is
used for.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-v4l2.c | 88 ++---
drivers/media/platform/rcar-vin/rcar-vin.h | 12
2 files changed, 42 insertions(+), 58 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 305a74d033b2d9c5..c8c764188b85a926 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -90,24 +90,30 @@ static u32 rvin_format_sizeimage(struct v4l2_pix_format
*pix)
* V4L2
*/
-int rvin_reset_format(struct rvin_dev *vin)
+static int rvin_get_sd_format(struct rvin_dev *vin, struct v4l2_pix_format
*pix)
{
struct v4l2_subdev_format fmt = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ .pad = vin->digital.source_pad,
};
- struct v4l2_mbus_framefmt *mf = &fmt.format;
int ret;
- fmt.pad = vin->digital.source_pad;
-
ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, &fmt);
if (ret)
return ret;
- vin->format.width = mf->width;
- vin->format.height = mf->height;
- vin->format.colorspace = mf->colorspace;
- vin->format.field = mf->field;
+ v4l2_fill_pix_format(pix, &fmt.format);
+
+ return 0;
+}
+
+int rvin_reset_format(struct rvin_dev *vin)
+{
+ int ret;
+
+ ret = rvin_get_sd_format(vin, &vin->format);
+ if (ret)
+ return ret;
/*
* If the subdevice uses ALTERNATE field mode and G_STD is
@@ -137,12 +143,12 @@ int rvin_reset_format(struct rvin_dev *vin)
}
vin->crop.top = vin->crop.left = 0;
- vin->crop.width = mf->width;
- vin->crop.height = mf->height;
+ vin->crop.width = vin->format.width;
+ vin->crop.height = vin->format.height;
vin->compose.top = vin->compose.left = 0;
- vin->compose.width = mf->width;
- vin->compose.height = mf->height;
+ vin->compose.width = vin->format.width;
+ vin->compose.height = vin->format.height;
vin->format.bytesperline = rvin_format_bytesperline(&vin->format);
vin->format.sizeimage = rvin_format_sizeimage(&vin->format);
@@ -151,9 +157,7 @@ int rvin_reset_format(struct rvin_dev *vin)
}
static int __rvin_try_format_source(struct rvin_dev *vin,
- u32 which,
- struct v4l2_pix_format *pix,
- struct rvin_source_fmt *source)
+ u32 which, struct v4l2_pix_format *pix)
{
struct v4l2_subdev *sd;
struct v4l2_subdev_pad_config *pad_cfg;
@@ -186,25 +190,15 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
v4l2_fill_pix_format(pix, &format.format);
pix->field = field;
-
- source->width = pix->width;
- source->height = pix->height;
-
pix->width = width;
pix->height = height;
-
- vin_dbg(vin, "Source resolution: %ux%u\n", source->width,
- source->height);
-
done:
v4l2_subdev_free_pad_config(pad_cfg);
return ret;
}
static int __rvin_try_format(struct rvin_dev *vin,
-u32 which,
-struct v4l2_pix_format *pix,
-struct rvin_source_fmt *source)
+u32 which, struct v4l2_pix_format *pix)
{
u32 walign;
int ret;
@@ -225,7 +219,7 @@ static int __rvin_try_format(struct rvin_dev *vin,
pix->sizeimage = 0;
/* Limit to source capabilities */
- ret = __rvin_try_format_source(vin, which, pix, source);
+ ret = __rvin_try_format_source(vin, which, pix);
if (ret)
return ret;
@@ -234,7 +228,6 @@ static int __rvin_try_format(struct rvin_dev *vin,
case V4L2_FIELD_BOTTOM:
case V4L2_FIELD_ALTERNATE:
pix->height /= 2;
- source->height /= 2;
break;
case V4L2_FIELD_NONE:
case V4L2_FIELD_INTERLACED_TB:
@@ -286,30 +279,23 @@ static int rvin_try_fmt_vid_cap(struct file *file, void
*priv,
struct v4l2_format *f)
{
struct rvin_dev *vin = video_drvdata(file);
- struct rvin_source_fmt source;
- return __rvin_try_format(vin, V4L2_SUBDEV_FORMAT_TRY, &f->fmt.pix,
-&source);
+ return __rvin_try_format(vin, V4L2_SUBDEV_FORMAT_TRY, &f->fmt.pix);
}
static int rvin_s_fmt_vid_cap(struct file *file, void *priv,
struct v4l2_format *f)
{
[PATCH v6 18/25] rcar-vin: prepare for media controller mode initialization
When running in media controller mode a media pad is needed, register
one. Also set the media bus format to CSI-2.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-core.c | 23 ++-
drivers/media/platform/rcar-vin/rcar-vin.h | 4
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index fbbb22924cf3a045..dd0525f2ba336bc2 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -45,6 +45,10 @@ static int rvin_find_pad(struct v4l2_subdev *sd, int
direction)
return -EINVAL;
}
+/*
-
+ * Digital async notifier
+ */
+
static bool rvin_mbus_supported(struct rvin_dev *vin)
{
struct v4l2_subdev *sd = vin->digital.subdev;
@@ -273,6 +277,20 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
return 0;
}
+/*
-
+ * Group async notifier
+ */
+
+static int rvin_group_init(struct rvin_dev *vin)
+{
+ /* All our sources are CSI-2 */
+ vin->mbus_cfg.type = V4L2_MBUS_CSI2;
+ vin->mbus_cfg.flags = 0;
+
+ vin->pad.flags = MEDIA_PAD_FL_SINK;
+ return media_entity_pads_init(&vin->vdev.entity, 1, &vin->pad);
+}
+
/*
-
* Platform Device Driver
*/
@@ -365,7 +383,10 @@ static int rcar_vin_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = rvin_digital_graph_init(vin);
+ if (vin->info->use_mc)
+ ret = rvin_group_init(vin);
+ else
+ ret = rvin_digital_graph_init(vin);
if (ret < 0)
goto error;
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
b/drivers/media/platform/rcar-vin/rcar-vin.h
index 12daff804bb6f77f..9c47669669c0469c 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -103,6 +103,8 @@ struct rvin_info {
* @notifier: V4L2 asynchronous subdevs notifier
* @digital: entity in the DT for local digital subdevice
*
+ * @pad: pad for media controller
+ *
* @lock: protects @queue
* @queue: vb2 buffers queue
*
@@ -132,6 +134,8 @@ struct rvin_dev {
struct v4l2_async_notifier notifier;
struct rvin_graph_entity digital;
+ struct media_pad pad;
+
struct mutex lock;
struct vb2_queue queue;
--
2.14.0
[PATCH v6 06/25] rcar-vin: move functions regarding scaling
In preparation of refactoring the scaling code move the code regarding
scaling to to the top of the file to avoid the need to add forward
declarations. No code is changed in this commit only whole functions
moved inside the same file.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-dma.c | 806 +++--
1 file changed, 405 insertions(+), 401 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
b/drivers/media/platform/rcar-vin/rcar-dma.c
index b136844499f677cf..03a79de197d19e43 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -138,305 +138,6 @@ static u32 rvin_read(struct rvin_dev *vin, u32 offset)
return ioread32(vin->base + offset);
}
-static int rvin_setup(struct rvin_dev *vin)
-{
- u32 vnmc, dmr, dmr2, interrupts;
- v4l2_std_id std;
- bool progressive = false, output_is_yuv = false, input_is_yuv = false;
-
- switch (vin->format.field) {
- case V4L2_FIELD_TOP:
- vnmc = VNMC_IM_ODD;
- break;
- case V4L2_FIELD_BOTTOM:
- vnmc = VNMC_IM_EVEN;
- break;
- case V4L2_FIELD_INTERLACED:
- /* Default to TB */
- vnmc = VNMC_IM_FULL;
- /* Use BT if video standard can be read and is 60 Hz format */
- if (!v4l2_subdev_call(vin_to_source(vin), video, g_std, &std)) {
- if (std & V4L2_STD_525_60)
- vnmc = VNMC_IM_FULL | VNMC_FOC;
- }
- break;
- case V4L2_FIELD_INTERLACED_TB:
- vnmc = VNMC_IM_FULL;
- break;
- case V4L2_FIELD_INTERLACED_BT:
- vnmc = VNMC_IM_FULL | VNMC_FOC;
- break;
- case V4L2_FIELD_ALTERNATE:
- case V4L2_FIELD_NONE:
- if (vin->continuous) {
- vnmc = VNMC_IM_ODD_EVEN;
- progressive = true;
- } else {
- vnmc = VNMC_IM_ODD;
- }
- break;
- default:
- vnmc = VNMC_IM_ODD;
- break;
- }
-
- /*
-* Input interface
-*/
- switch (vin->digital.code) {
- case MEDIA_BUS_FMT_YUYV8_1X16:
- /* BT.601/BT.1358 16bit YCbCr422 */
- vnmc |= VNMC_INF_YUV16;
- input_is_yuv = true;
- break;
- case MEDIA_BUS_FMT_UYVY8_2X8:
- /* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
- vnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ?
- VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
- input_is_yuv = true;
- break;
- case MEDIA_BUS_FMT_RGB888_1X24:
- vnmc |= VNMC_INF_RGB888;
- break;
- case MEDIA_BUS_FMT_UYVY10_2X10:
- /* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
- vnmc |= vin->digital.mbus_cfg.type == V4L2_MBUS_BT656 ?
- VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
- input_is_yuv = true;
- break;
- default:
- break;
- }
-
- /* Enable VSYNC Field Toogle mode after one VSYNC input */
- dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);
-
- /* Hsync Signal Polarity Select */
- if (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
- dmr2 |= VNDMR2_HPS;
-
- /* Vsync Signal Polarity Select */
- if (!(vin->digital.mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
- dmr2 |= VNDMR2_VPS;
-
- /*
-* Output format
-*/
- switch (vin->format.pixelformat) {
- case V4L2_PIX_FMT_NV16:
- rvin_write(vin,
- ALIGN(vin->format.width * vin->format.height, 0x80),
- VNUVAOF_REG);
- dmr = VNDMR_DTMD_YCSEP;
- output_is_yuv = true;
- break;
- case V4L2_PIX_FMT_YUYV:
- dmr = VNDMR_BPSM;
- output_is_yuv = true;
- break;
- case V4L2_PIX_FMT_UYVY:
- dmr = 0;
- output_is_yuv = true;
- break;
- case V4L2_PIX_FMT_XRGB555:
- dmr = VNDMR_DTMD_ARGB1555;
- break;
- case V4L2_PIX_FMT_RGB565:
- dmr = 0;
- break;
- case V4L2_PIX_FMT_XBGR32:
- /* Note: not supported on M1 */
- dmr = VNDMR_EXRGB;
- break;
- default:
- vin_err(vin, "Invalid pixelformat (0x%x)\n",
- vin->format.pixelformat);
- return -EINVAL;
- }
-
- /* Always update on field change */
- vnmc |= VNMC_VUP;
-
- /* If input and output use the same colorspace, use bypass mode */
- if (input_is_yuv == output_is_yuv)
[PATCH v6 23/25] rcar-vin: extend {start,stop}_streaming to work with media controller
The procedure to start or stop streaming using the none MC single
subdevice and the MC graph and multiple subdevices are quiet different.
Create a new function to abstract which method is used based on which
mode the driver is running in and add logic to start the MC graph.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-dma.c | 112 +++--
1 file changed, 105 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
b/drivers/media/platform/rcar-vin/rcar-dma.c
index de1486ee2786b499..499253f94390f43e 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -1087,15 +1087,115 @@ static void rvin_buffer_queue(struct vb2_buffer *vb)
spin_unlock_irqrestore(&vin->qlock, flags);
}
+static int rvin_set_stream(struct rvin_dev *vin, int on)
+{
+ struct v4l2_subdev_format fmt = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+ struct media_pipeline *pipe;
+ struct v4l2_subdev *sd;
+ struct media_pad *pad;
+ int ret;
+
+ /* Not media controller used, simply pass operation to subdevice */
+ if (!vin->info->use_mc) {
+ ret = v4l2_subdev_call(vin->digital.subdev, video, s_stream,
+ on);
+
+ return ret == -ENOIOCTLCMD ? 0 : ret;
+ }
+
+ pad = media_entity_remote_pad(&vin->pad);
+ if (!pad)
+ return -EPIPE;
+
+ sd = media_entity_to_v4l2_subdev(pad->entity);
+ if (!sd)
+ return -EPIPE;
+
+ if (on) {
+ fmt.pad = pad->index;
+ if (v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt))
+ return -EPIPE;
+
+ switch (fmt.format.code) {
+ case MEDIA_BUS_FMT_YUYV8_1X16:
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ case MEDIA_BUS_FMT_UYVY10_2X10:
+ case MEDIA_BUS_FMT_RGB888_1X24:
+ vin->code = fmt.format.code;
+ break;
+ default:
+ return -EPIPE;
+ }
+
+ switch (fmt.format.field) {
+ case V4L2_FIELD_TOP:
+ case V4L2_FIELD_BOTTOM:
+ case V4L2_FIELD_NONE:
+ case V4L2_FIELD_INTERLACED_TB:
+ case V4L2_FIELD_INTERLACED_BT:
+ case V4L2_FIELD_INTERLACED:
+ case V4L2_FIELD_SEQ_TB:
+ case V4L2_FIELD_SEQ_BT:
+ /* Supported nativly */
+ break;
+ case V4L2_FIELD_ALTERNATE:
+ switch (vin->format.field) {
+ case V4L2_FIELD_TOP:
+ case V4L2_FIELD_BOTTOM:
+ case V4L2_FIELD_NONE:
+ break;
+ case V4L2_FIELD_INTERLACED_TB:
+ case V4L2_FIELD_INTERLACED_BT:
+ case V4L2_FIELD_INTERLACED:
+ case V4L2_FIELD_SEQ_TB:
+ case V4L2_FIELD_SEQ_BT:
+ /* Use VIN hardware to combine the two fields */
+ fmt.format.height *= 2;
+ break;
+ default:
+ return -EPIPE;
+ }
+ break;
+ default:
+ return -EPIPE;
+ }
+
+ if (fmt.format.width != vin->format.width ||
+ fmt.format.height != vin->format.height)
+ return -EPIPE;
+
+ pipe = sd->entity.pipe ? sd->entity.pipe : &vin->vdev.pipe;
+ if (media_pipeline_start(&vin->vdev.entity, pipe))
+ return -EPIPE;
+
+ ret = v4l2_subdev_call(sd, video, s_stream, 1);
+ if (ret == -ENOIOCTLCMD)
+ ret = 0;
+ if (ret)
+ media_pipeline_stop(&vin->vdev.entity);
+ } else {
+ media_pipeline_stop(&vin->vdev.entity);
+ ret = v4l2_subdev_call(sd, video, s_stream, 0);
+ }
+
+ return ret;
+}
+
static int rvin_start_streaming(struct vb2_queue *vq, unsigned int count)
{
struct rvin_dev *vin = vb2_get_drv_priv(vq);
- struct v4l2_subdev *sd;
unsigned long flags;
int ret;
- sd = vin_to_source(vin);
- v4l2_subdev_call(sd, video, s_stream, 1);
+ ret = rvin_set_stream(vin, 1);
+ if (ret) {
+ spin_lock_irqsave(&vin->qlock, flags);
+ return_all_buffers(vin, VB2_BUF_STATE_QUEUED);
+ spin_unlock_irqrestore(&vin->qlock, flags);
+ return ret;
+ }
spin_lock_irqsave(&vin->qlock, flags);
@@ -1104,7 +1204,7 @@ static int rvin_start_streaming(struct vb
[PATCH v6 15/25] rcar-vin: add flag to switch to media controller mode
On Gen3 a media controller API needs to be used to allow userspace to
configure the subdevices in the pipeline instead of directly controlling
a single source subdevice, which is and will continue to be the mode of
operation on Gen2.
Prepare for these two modes of operation by adding a flag to struct
rvin_graph_entity which will control which mode to use.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-core.c | 6 +-
drivers/media/platform/rcar-vin/rcar-vin.h | 2 ++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index 65f01b6781c0aefd..fbbb22924cf3a045 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -279,18 +279,21 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
static const struct rvin_info rcar_info_h1 = {
.chip = RCAR_H1,
+ .use_mc = false,
.max_width = 2048,
.max_height = 2048,
};
static const struct rvin_info rcar_info_m1 = {
.chip = RCAR_M1,
+ .use_mc = false,
.max_width = 2048,
.max_height = 2048,
};
static const struct rvin_info rcar_info_gen2 = {
.chip = RCAR_GEN2,
+ .use_mc = false,
.max_width = 2048,
.max_height = 2048,
};
@@ -387,7 +390,8 @@ static int rcar_vin_remove(struct platform_device *pdev)
v4l2_async_notifier_unregister(&vin->notifier);
/* Checks internaly if handlers have been init or not */
- v4l2_ctrl_handler_free(&vin->ctrl_handler);
+ if (!vin->info->use_mc)
+ v4l2_ctrl_handler_free(&vin->ctrl_handler);
rvin_v4l2_remove(vin);
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
b/drivers/media/platform/rcar-vin/rcar-vin.h
index 94c606f2b8f2f246..819d9c04ed8ffb36 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -77,12 +77,14 @@ struct rvin_graph_entity {
/**
* struct rvin_info - Information about the particular VIN implementation
* @chip: type of VIN chip
+ * @use_mc:use media controller instead of controlling subdevice
*
* max_width: max input width the VIN supports
* max_height: max input height the VIN supports
*/
struct rvin_info {
enum chip_id chip;
+ bool use_mc;
unsigned int max_width;
unsigned int max_height;
--
2.14.0
[PATCH v6 20/25] rcar-vin: add chsel information to rvin_info
Each Gen3 SoC has a limited set of predefined routing possibilities for
which CSI-2 device and virtual channel can be routed to which VIN
instance. Prepare to store this information in the struct rvin_info.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-vin.h | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
b/drivers/media/platform/rcar-vin/rcar-vin.h
index 88683aaee3b6acd5..617f254b52fe106d 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -35,6 +35,9 @@
/* Max number on VIN instances that can be in a system */
#define RCAR_VIN_NUM 8
+/* Max number of CHSEL values for any Gen3 SoC */
+#define RCAR_CHSEL_MAX 6
+
enum chip_id {
RCAR_H1,
RCAR_M1,
@@ -91,6 +94,19 @@ struct rvin_graph_entity {
struct rvin_group;
+
+/** struct rvin_group_chsel - Map a CSI2 device and channel for a CHSEL value
+ * @csi: VIN internal number for CSI2 device
+ * @chan: CSI-2 channel number on remote. Note that channel
+ * is not the same as VC. The CSI-2 hardware have 4
+ * channels it can output on but which VC is outputted
+ * on which channel is configurable inside the CSI-2.
+ */
+struct rvin_group_chsel {
+ enum rvin_csi_id csi;
+ unsigned int chan;
+};
+
/**
* struct rvin_info - Information about the particular VIN implementation
* @chip: type of VIN chip
@@ -98,6 +114,9 @@ struct rvin_group;
*
* max_width: max input width the VIN supports
* max_height: max input height the VIN supports
+ *
+ * num_chsels: number of possible chsel values for this VIN
+ * chsels: routing table VIN <-> CSI-2 for the chsel values
*/
struct rvin_info {
enum chip_id chip;
@@ -105,6 +124,9 @@ struct rvin_info {
unsigned int max_width;
unsigned int max_height;
+
+ unsigned int num_chsels;
+ struct rvin_group_chsel chsels[RCAR_VIN_NUM][RCAR_CHSEL_MAX];
};
/**
--
2.14.0
[PATCH v6 25/25] rcar-vin: enable support for r8a7796
Add the SoC specific information for Renesas r8a7796.
Signed-off-by: Niklas Söderlund
---
.../devicetree/bindings/media/rcar_vin.txt | 1 +
drivers/media/platform/rcar-vin/rcar-core.c| 64 ++
2 files changed, 65 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
b/Documentation/devicetree/bindings/media/rcar_vin.txt
index be38ad89d71ad05d..767358f39512aa17 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -10,6 +10,7 @@ always slaves and support multiple input channels which can
be either RGB,
YUVU, BT656 or CSI-2.
- compatible: Must be one or more of the following
+ - "renesas,vin-r8a7796" for the R8A7796 device
- "renesas,vin-r8a7795" for the R8A7795 device
- "renesas,vin-r8a7794" for the R8A7794 device
- "renesas,vin-r8a7793" for the R8A7793 device
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index 58d903ab9fb83faf..e01edd5f5925d26c 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -1116,7 +1116,71 @@ static const struct rvin_info rcar_info_r8a7795es1 = {
},
};
+static const struct rvin_info rcar_info_r8a7796 = {
+ .chip = RCAR_GEN3,
+ .use_mc = true,
+ .max_width = 4096,
+ .max_height = 4096,
+
+ .num_chsels = 5,
+ .chsels = {
+ {
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_NC, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ }, {
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_NC, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 1 },
+ { .csi = RVIN_CSI20, .chan = 1 },
+ }, {
+ { .csi = RVIN_NC, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 2 },
+ { .csi = RVIN_CSI20, .chan = 2 },
+ }, {
+ { .csi = RVIN_CSI40, .chan = 1 },
+ { .csi = RVIN_CSI20, .chan = 1 },
+ { .csi = RVIN_NC, .chan = 1 },
+ { .csi = RVIN_CSI40, .chan = 3 },
+ { .csi = RVIN_CSI20, .chan = 3 },
+ }, {
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_NC, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ }, {
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_NC, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 1 },
+ { .csi = RVIN_CSI20, .chan = 1 },
+ }, {
+ { .csi = RVIN_NC, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 0 },
+ { .csi = RVIN_CSI20, .chan = 0 },
+ { .csi = RVIN_CSI40, .chan = 2 },
+ { .csi = RVIN_CSI20, .chan = 2 },
+ }, {
+ { .csi = RVIN_CSI40, .chan = 1 },
+ { .csi = RVIN_CSI20, .chan = 1 },
+ { .csi = RVIN_NC, .chan = 1 },
+ { .csi = RVIN_CSI40, .chan = 3 },
+ { .csi = RVIN_CSI20, .chan = 3 },
+ },
+ },
+};
+
static const struct of_device_id rvin_of_id_table[] = {
+ {
+ .compatible = "renesas,vin-r8a7796",
+ .data = &rcar_info_r8a7796,
+ },
{
.compatible = "renesas,vin-r8a7795",
.data = &rcar_info_r8a7795,
--
2.14.0
[PATCH v6 22/25] rcar-vin: add link notify for Gen3
Add the ability to process media device link change request. Link
enablement are a bit complicated on Gen3, if it's possible to enable a
link depends on what other links already are enabled. On Gen3 the 8 VIN
are split into two subgroups (VIN0-3 and VIN4-7) and from a routing
perspective these two groups are independent of each other. Each
subgroups routing is controlled by the subgroup VIN master instance
(VIN0 and VIN4).
There are a limited number of possible route setups available for each
subgroup and the configuration of each setup is dictated by the
hardware. On H3 for example there are 6 possible route setups for each
subgroup to choose from.
This leads to the media device link notification code being rather large
since it will find the best routing configuration to try and accommodate
as many links as possible. When it's not possible to enable a new link
due to hardware constrains the link_notifier callback will return
-EMLINK.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-core.c | 203
1 file changed, 203 insertions(+)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index 2aba442a0750e91a..dec91e2f3ccdbd93 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -26,6 +26,207 @@
#include "rcar-vin.h"
+/*
-
+ * Media Controller link notification
+ */
+
+static unsigned int rvin_group_csi_pad_to_chan(unsigned int pad)
+{
+ /*
+* The CSI2 driver is rcar-csi2 and we know it's pad layout are
+* 0: Source 1-4: Sinks so if we remove one from the pad we
+* get the rcar-vin internal CSI2 channel number
+*/
+ return pad - 1;
+}
+
+/* group lock should be held when calling this function */
+static int rvin_group_entity_to_vin_num(struct rvin_group *group,
+ struct media_entity *entity)
+{
+ struct video_device *vdev;
+ int i;
+
+ if (!is_media_entity_v4l2_video_device(entity))
+ return -ENODEV;
+
+ vdev = media_entity_to_video_device(entity);
+
+ for (i = 0; i < RCAR_VIN_NUM; i++) {
+ if (!group->vin[i])
+ continue;
+
+ if (&group->vin[i]->vdev == vdev)
+ return i;
+ }
+
+ return -ENODEV;
+}
+
+/* group lock should be held when calling this function */
+static int rvin_group_entity_to_csi_num(struct rvin_group *group,
+ struct media_entity *entity)
+{
+ struct v4l2_subdev *sd;
+ int i;
+
+ if (!is_media_entity_v4l2_subdev(entity))
+ return -ENODEV;
+
+ sd = media_entity_to_v4l2_subdev(entity);
+
+ for (i = 0; i < RVIN_CSI_MAX; i++)
+ if (group->csi[i].subdev == sd)
+ return i;
+
+ return -ENODEV;
+}
+
+/* group lock should be held when calling this function */
+static void __rvin_group_build_link_list(struct rvin_group *group,
+struct rvin_group_chsel *map,
+int start, int len)
+{
+ struct media_pad *vin_pad, *remote_pad;
+ unsigned int n;
+
+ for (n = 0; n < len; n++) {
+ map[n].csi = -1;
+ map[n].chan = -1;
+
+ if (!group->vin[start + n])
+ continue;
+
+ vin_pad = &group->vin[start + n]->vdev.entity.pads[0];
+
+ remote_pad = media_entity_remote_pad(vin_pad);
+ if (!remote_pad)
+ continue;
+
+ map[n].csi =
+ rvin_group_entity_to_csi_num(group, remote_pad->entity);
+ map[n].chan = rvin_group_csi_pad_to_chan(remote_pad->index);
+ }
+}
+
+/* group lock should be held when calling this function */
+static int __rvin_group_try_get_chsel(struct rvin_group *group,
+ struct rvin_group_chsel *map,
+ int start, int len)
+{
+ const struct rvin_group_chsel *sel;
+ unsigned int i, n;
+ int chsel;
+
+ for (i = 0; i < group->vin[start]->info->num_chsels; i++) {
+ chsel = i;
+ for (n = 0; n < len; n++) {
+
+ /* If the link is not active it's OK */
+ if (map[n].csi == -1)
+ continue;
+
+ /* Check if chsel match requested link */
+ sel = &group->vin[start]->info->chsels[start + n][i];
+ if (map[n].csi != sel->csi ||
+ map[n].chan != sel->chan) {
+ chsel = -1;
+ break;
+ }
+ }
+
+
[PATCH v6 05/25] rcar-vin: change name of video device
The rcar-vin driver needs to be part of a media controller to support Gen3. Give each VIN instance a unique name so it can be referenced from userspace. Signed-off-by: Niklas Söderlund Reviewed-by: Kieran Bingham --- drivers/media/platform/rcar-vin/rcar-v4l2.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 3c4dd08261a0d3f5..ba88774bd5379a98 100644 --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c @@ -878,7 +878,8 @@ int rvin_v4l2_probe(struct rvin_dev *vin) vdev->fops = &rvin_fops; vdev->v4l2_dev = &vin->v4l2_dev; vdev->queue = &vin->queue; - strlcpy(vdev->name, KBUILD_MODNAME, sizeof(vdev->name)); + snprintf(vdev->name, sizeof(vdev->name), "%s %s", KBUILD_MODNAME, +dev_name(vin->dev)); vdev->release = video_device_release_empty; vdev->ioctl_ops = &rvin_ioctl_ops; vdev->lock = &vin->lock; -- 2.14.0
[PATCH v6 09/25] rcar-vin: do not allow changing scaling and composing while streaming
It is possible on Gen2 to change the registers controlling composing and
scaling while the stream is running. Is however not a good idea to do so
and could result in trouble. There are also no good reason to allow
this, remove immediate reflection in hardware registers from
vidioc_s_selection and only configure scaling and composing when the
stream starts.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-dma.c | 2 +-
drivers/media/platform/rcar-vin/rcar-v4l2.c | 3 ---
drivers/media/platform/rcar-vin/rcar-vin.h | 3 ---
3 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
b/drivers/media/platform/rcar-vin/rcar-dma.c
index 5f9674dc898305ba..6cc880e5ef7e0718 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -514,7 +514,7 @@ static void rvin_set_coeff(struct rvin_dev *vin, unsigned
short xs)
rvin_write(vin, p_set->coeff_set[23], VNC8C_REG);
}
-void rvin_crop_scale_comp(struct rvin_dev *vin)
+static void rvin_crop_scale_comp(struct rvin_dev *vin)
{
u32 xs, ys;
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 421820caf275b066..305a74d033b2d9c5 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -436,9 +436,6 @@ static int rvin_s_selection(struct file *file, void *fh,
return -EINVAL;
}
- /* HW supports modifying configuration while running */
- rvin_crop_scale_comp(vin);
-
return 0;
}
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
b/drivers/media/platform/rcar-vin/rcar-vin.h
index b2bac06c0a3cfcb7..fc70ded462ed3244 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -176,7 +176,4 @@ int rvin_reset_format(struct rvin_dev *vin);
const struct rvin_video_format *rvin_format_from_pixel(u32 pixelformat);
-/* Cropping, composing and scaling */
-void rvin_crop_scale_comp(struct rvin_dev *vin);
-
#endif
--
2.14.0
[PATCH v6 04/25] rcar-vin: move max width and height information to chip information
On Gen3 the max supported width and height will be different from Gen2.
Move the limits to the struct rvin_info to prepare for Gen3 support.
Signed-off-by: Niklas Söderlund
Reviewed-by: Kieran Bingham
---
drivers/media/platform/rcar-vin/rcar-core.c | 6 ++
drivers/media/platform/rcar-vin/rcar-v4l2.c | 6 ++
drivers/media/platform/rcar-vin/rcar-vin.h | 6 ++
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index dae38de706b66b64..4dc148e7835439ab 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -279,14 +279,20 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
static const struct rvin_info rcar_info_h1 = {
.chip = RCAR_H1,
+ .max_width = 2048,
+ .max_height = 2048,
};
static const struct rvin_info rcar_info_m1 = {
.chip = RCAR_M1,
+ .max_width = 2048,
+ .max_height = 2048,
};
static const struct rvin_info rcar_info_gen2 = {
.chip = RCAR_GEN2,
+ .max_width = 2048,
+ .max_height = 2048,
};
static const struct of_device_id rvin_of_id_table[] = {
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 02a08cf5acfce1ce..3c4dd08261a0d3f5 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -23,8 +23,6 @@
#include "rcar-vin.h"
#define RVIN_DEFAULT_FORMATV4L2_PIX_FMT_YUYV
-#define RVIN_MAX_WIDTH 2048
-#define RVIN_MAX_HEIGHT2048
/*
-
* Format Conversions
@@ -258,8 +256,8 @@ static int __rvin_try_format(struct rvin_dev *vin,
walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
/* Limit to VIN capabilities */
- v4l_bound_align_image(&pix->width, 2, RVIN_MAX_WIDTH, walign,
- &pix->height, 4, RVIN_MAX_HEIGHT, 2, 0);
+ v4l_bound_align_image(&pix->width, 2, vin->info->max_width, walign,
+ &pix->height, 4, vin->info->max_height, 2, 0);
pix->bytesperline = max_t(u32, pix->bytesperline,
rvin_format_bytesperline(pix));
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
b/drivers/media/platform/rcar-vin/rcar-vin.h
index 13466dfd72292fc0..2d8b362012ea46a3 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -91,9 +91,15 @@ struct rvin_graph_entity {
/**
* struct rvin_info - Information about the particular VIN implementation
* @chip: type of VIN chip
+ *
+ * max_width: max input width the VIN supports
+ * max_height: max input height the VIN supports
*/
struct rvin_info {
enum chip_id chip;
+
+ unsigned int max_width;
+ unsigned int max_height;
};
/**
--
2.14.0
[PATCH v6 08/25] rcar-vin: do not reset crop and compose when setting format
It was a bad idea to reset the crop and compose settings when a new
format is set. This would overwrite any crop/compose set by s_select and
cause unexpected behaviors, remove it. Also fold the reset helper in to
the only remaining caller.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-v4l2.c | 21 +++--
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index affdc128a75e502e..421820caf275b066 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -90,17 +90,6 @@ static u32 rvin_format_sizeimage(struct v4l2_pix_format *pix)
* V4L2
*/
-static void rvin_reset_crop_compose(struct rvin_dev *vin)
-{
- vin->crop.top = vin->crop.left = 0;
- vin->crop.width = vin->source.width;
- vin->crop.height = vin->source.height;
-
- vin->compose.top = vin->compose.left = 0;
- vin->compose.width = vin->format.width;
- vin->compose.height = vin->format.height;
-}
-
int rvin_reset_format(struct rvin_dev *vin)
{
struct v4l2_subdev_format fmt = {
@@ -147,7 +136,13 @@ int rvin_reset_format(struct rvin_dev *vin)
break;
}
- rvin_reset_crop_compose(vin);
+ vin->crop.top = vin->crop.left = 0;
+ vin->crop.width = mf->width;
+ vin->crop.height = mf->height;
+
+ vin->compose.top = vin->compose.left = 0;
+ vin->compose.width = mf->width;
+ vin->compose.height = mf->height;
vin->format.bytesperline = rvin_format_bytesperline(&vin->format);
vin->format.sizeimage = rvin_format_sizeimage(&vin->format);
@@ -317,8 +312,6 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv,
vin->format = f->fmt.pix;
- rvin_reset_crop_compose(vin);
-
return 0;
}
--
2.14.0
[PATCH] docs-rst: fix pdf build with Spinx 1.6
Sphinx 1.6 generates some LaTeX code before each table,
starting its own environment before calling tabulary,
apparently to improve table layout.
The problem is that such environment is incompatible with
adjustbox. While, in thesis, it should be possible to override
it or to redefine tabulary, I was unable to produce such patch.
Also, that would likely break on some future Sphinx version.
So, instead, let's just change the font size on bigger tables,
in order for them to fit into the page size. That is not as
good as adjustbox, and require some manual work, but it should
be less sensitive to Sphinx changes.
While here, adjust a few other tables whose text is exceeding
the cell boundaries.
Signed-off-by: Mauro Carvalho Chehab
---
Not sure it such patch should be merged via linux-media or via linux-doc
tree. Merging it via linux-media would likely cause less conflicts, although
I guess it won't rise any merge conflicts if merged via linux-doc.
Documentation/conf.py | 3 -
Documentation/doc-guide/sphinx.rst | 4 +-
Documentation/media/uapi/v4l/dev-meta.rst | 2 +
Documentation/media/uapi/v4l/dev-raw-vbi.rst | 2 +-
Documentation/media/uapi/v4l/dev-sliced-vbi.rst| 21 ++-
Documentation/media/uapi/v4l/dev-subdev.rst| 6 +-
Documentation/media/uapi/v4l/extended-controls.rst | 6 +-
Documentation/media/uapi/v4l/pixfmt-inzi.rst | 7 +-
Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst | 30 ++--
Documentation/media/uapi/v4l/pixfmt-packed-rgb.rst | 186 +++--
Documentation/media/uapi/v4l/pixfmt-packed-yuv.rst | 50 +++---
Documentation/media/uapi/v4l/pixfmt-srggb10p.rst | 7 +-
Documentation/media/uapi/v4l/subdev-formats.rst| 13 +-
Documentation/media/uapi/v4l/vidioc-dqevent.rst| 2 +-
.../media/uapi/v4l/vidioc-dv-timings-cap.rst | 2 +-
.../media/uapi/v4l/vidioc-enum-frameintervals.rst | 2 +
Documentation/media/uapi/v4l/vidioc-enumstd.rst| 9 +-
.../media/uapi/v4l/vidioc-g-dv-timings.rst | 4 +-
.../media/uapi/v4l/vidioc-g-enc-index.rst | 2 +-
.../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 2 +-
.../media/uapi/v4l/vidioc-g-sliced-vbi-cap.rst | 6 +-
Documentation/media/uapi/v4l/vidioc-g-tuner.rst| 4 +-
Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 2 +-
scripts/sphinx-pre-install | 1 -
24 files changed, 200 insertions(+), 173 deletions(-)
diff --git a/Documentation/conf.py b/Documentation/conf.py
index 71b032bb44fd..25a58564362f 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -331,9 +331,6 @@ latex_elements = {
\\setromanfont{DejaVu Sans}
\\setmonofont{DejaVu Sans Mono}
- % To allow adjusting table sizes
- \\usepackage{adjustbox}
-
'''
}
diff --git a/Documentation/doc-guide/sphinx.rst
b/Documentation/doc-guide/sphinx.rst
index 8faafb9b2d86..a2417633fdd8 100644
--- a/Documentation/doc-guide/sphinx.rst
+++ b/Documentation/doc-guide/sphinx.rst
@@ -80,9 +80,7 @@ output.
PDF and LaTeX builds
-Such builds are currently supported only with Sphinx versions 1.4 and 1.5.
-
-Currently, it is not possible to do pdf builds with Sphinx version 1.6.
+Such builds are currently supported only with Sphinx versions 1.4 and upper.
For PDF and LaTeX output, you'll also need ``XeLaTeX`` version 3.14159265.
diff --git a/Documentation/media/uapi/v4l/dev-meta.rst
b/Documentation/media/uapi/v4l/dev-meta.rst
index 62518adfe37b..f7ac8d0d3af1 100644
--- a/Documentation/media/uapi/v4l/dev-meta.rst
+++ b/Documentation/media/uapi/v4l/dev-meta.rst
@@ -42,6 +42,8 @@ the :c:type:`v4l2_format` structure to 0.
.. _v4l2-meta-format:
+.. tabularcolumns:: |p{1.4cm}|p{2.2cm}|p{13.9cm}|
+
.. flat-table:: struct v4l2_meta_format
:header-rows: 0
:stub-columns: 0
diff --git a/Documentation/media/uapi/v4l/dev-raw-vbi.rst
b/Documentation/media/uapi/v4l/dev-raw-vbi.rst
index 2e6878b624f6..8d0d93a9a254 100644
--- a/Documentation/media/uapi/v4l/dev-raw-vbi.rst
+++ b/Documentation/media/uapi/v4l/dev-raw-vbi.rst
@@ -183,7 +183,7 @@ and always returns default parameters as :ref:`VIDIOC_G_FMT
` does
applications must set it to zero.
-.. tabularcolumns:: |p{4.0cm}|p{1.5cm}|p{12.0cm}|
+.. .. tabularcolumns:: |p{4.0cm}|p{1.5cm}|p{12.0cm}|
.. _vbifmt-flags:
diff --git a/Documentation/media/uapi/v4l/dev-sliced-vbi.rst
b/Documentation/media/uapi/v4l/dev-sliced-vbi.rst
index 5f6d534ea73b..5eec1f666dd0 100644
--- a/Documentation/media/uapi/v4l/dev-sliced-vbi.rst
+++ b/Documentation/media/uapi/v4l/dev-sliced-vbi.rst
@@ -105,7 +105,13 @@ which may return ``EBUSY`` can be the
struct v4l2_sliced_vbi_format
-
-.. tabularcolumns:: |p{1.0cm}|p{4.5cm}|p{4.0cm}|p{4.0cm}|p{4.0cm}|
+.. raw:: latex
+
+\begingroup
+\scriptsize
+\setlength{\tabcolsep}{2pt}
+
+.. tabularcolumns:: |p{.75cm}
Re: [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a
On Sun, Aug 20, 2017 at 12:24:08AM +0300, Sakari Ailus wrote: > From: Sakari Ailus Commit msg? > Signed-off-by: Sakari Ailus Shouldn't author and SoB be the same email? > --- > .../devicetree/bindings/leds/ams,as3645a.txt | 71 > ++ > 1 file changed, 71 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/ams,as3645a.txt Otherwise, Acked-by: Rob Herring
RE: [PATCH 0/3] DW9714 DT support
Hi Sakari, Thanks for the patches. I have verified that the dw9714 driver gets probed via DT bindings, with these patches on 4.13-rc6 kernel. Feel free to add Tested-by: Rajmohan Mani Raj > -Original Message- > From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com] > Sent: Thursday, August 17, 2017 6:43 AM > To: linux-media@vger.kernel.org > Cc: devicet...@vger.kernel.org; Mani, Rajmohan > > Subject: [PATCH 0/3] DW9714 DT support > > Hi all, > > This patchset adds DT bindings as well as DT support for DW9714. The unused > ACPI match table is removed. > > Sakari Ailus (3): > dt-bindings: Add bindings for Dongwoon DW9714 voice coil > dw9714: Add Devicetree support > dw9714: Remove ACPI match tables, convert to use probe_new > > .../bindings/media/i2c/dongwoon,dw9714.txt | 9 > .../devicetree/bindings/vendor-prefixes.txt| 1 + > drivers/media/i2c/dw9714.c | 26 > +- > 3 files changed, 21 insertions(+), 15 deletions(-) create mode 100644 > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt > > -- > 2.7.4
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
On Tue, 22 Aug 2017 19:43:39 +0200
Maxime Ripard wrote:
> Hi Yong,
>
> On Mon, Jul 31, 2017 at 11:16:40AM +0800, Yong wrote:
> > > > @@ -143,6 +143,7 @@ source "drivers/media/platform/am437x/Kconfig"
> > > > source "drivers/media/platform/xilinx/Kconfig"
> > > > source "drivers/media/platform/rcar-vin/Kconfig"
> > > > source "drivers/media/platform/atmel/Kconfig"
> > > > +source "drivers/media/platform/sun6i-csi/Kconfig"
> > >
> > > We're going to have several different drivers in v4l eventually, so I
> > > guess it would make sense to move to a directory of our own.
> >
> > Like this?
> > drivers/media/platform/sunxi/sun6i-csi
>
> Yep.
>
> > > > +static int sun6i_graph_notify_complete(struct v4l2_async_notifier
> > > > *notifier)
> > > > +{
> > > > + struct sun6i_csi *csi =
> > > > + container_of(notifier, struct sun6i_csi,
> > > > notifier);
> > > > + struct sun6i_graph_entity *entity;
> > > > + int ret;
> > > > +
> > > > + dev_dbg(csi->dev, "notify complete, all subdevs registered\n");
> > > > +
> > > > + /* Create links for every entity. */
> > > > + list_for_each_entry(entity, &csi->entities, list) {
> > > > + ret = sun6i_graph_build_one(csi, entity);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > + }
> > > > +
> > > > + /* Create links for video node. */
> > > > + ret = sun6i_graph_build_video(csi);
> > > > + if (ret < 0)
> > > > + return ret;
> > >
> > > Can you elaborate a bit on the difference between a node parsed with
> > > _graph_build_one and _graph_build_video? Can't you just store the
> > > remote sensor when you build the notifier, and reuse it here?
> >
> > There maybe many usercases:
> > 1. CSI->Sensor.
> > 2. CSI->MIPI->Sensor.
> > 3. CSI->FPGA->Sensor1
> > ->Sensor2.
> > FPGA maybe some other video processor. FPGA, MIPI, Sensor can be
> > registered as v4l2 subdevs. We do not care about the driver code
> > of them. But they should be linked together here.
> >
> > So, the _graph_build_one is used to link CSI port and subdevs.
> > _graph_build_video is used to link CSI port and video node.
>
> So the graph_build_one is for the two first cases, and the
> _build_video for the latter case?
No.
The _graph_build_one is used to link the subdevs found in the device
tree. _build_video is used to link the closest subdev to video node.
Video node is created in the driver, so the method to get it's pad is
diffrent to the subdevs.
>
> If so, you should take a look at the last iteration of the
> subnotifiers rework by Nikas Söderlund (v4l2-async: add subnotifier
> registration for subdevices).
>
> It allows subdevs to register notifiers, and you don't have to build
> the graph from the video device, each device and subdev can only care
> about what's next in the pipeline, but not really what's behind it.
>
> That would mean in your case that you can only deal with your single
> CSI pad, and whatever subdev driver will use it care about its own.
Do you mean the subdevs create pad link in the notifier registered by
themself ?
If so, _graph_build_one is needless. But how to make sure the pipeline
has linked correctly when operateing the pipeline.
I will lookt at this in more detail.
>
> > This part is also difficult to understand for me. The one CSI module
> > have only one DMA channel(single port). But thay can be linked to
> > different physical port (Parallel or MIPI)(multiple ep) by IF select
> > register.
> >
> > For now, the binding can have several ep, the driver will just pick
> > the first valid one.
>
> Yeah, I'm not really sure how we could deal with that, but I guess we
> can do it later on.
>
> > >
> > > > +struct sun6i_csi_ops {
> > > > + int (*get_supported_pixformats)(struct sun6i_csi *csi,
> > > > + const u32 **pixformats);
> > > > + bool (*is_format_support)(struct sun6i_csi *csi, u32 pixformat,
> > > > + u32 mbus_code);
> > > > + int (*s_power)(struct sun6i_csi *csi, bool enable);
> > > > + int (*update_config)(struct sun6i_csi *csi,
> > > > +struct sun6i_csi_config *config);
> > > > + int (*update_buf_addr)(struct sun6i_csi *csi, dma_addr_t addr);
> > > > + int (*s_stream)(struct sun6i_csi *csi, bool enable);
> > > > +};
> > >
> > > Didn't we agreed on removing those in the first iteration? It's not
> > > really clear at this point whether they will be needed at all. Make
> > > something simple first, without those ops. When we'll support other
> > > SoCs we'll have a better chance at seeing what and how we should deal
> > > with potential quirks.
> >
> > OK. But without ops, it is inappropriate to sun6i_csi and sun6i_csi.
> > Maybe I should merge the two files.
>
> I'm not sure what you meant here, but if you think that's appropriate,
> please go ahead.
>
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
On Mon, 21 Aug 2017 22:21:45 +0200
Maxime Ripard wrote:
> 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) ||
> > > > +
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: Wed Aug 23 05:25:33 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/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.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.
On 08/22/2017 10:17 PM, Maxime Ripard wrote:
> On Tue, Aug 22, 2017 at 08:43:35AM +0200, Hans Verkuil wrote:
> +static int sun6i_video_link_setup(struct media_entity *entity,
> + const struct media_pad *local,
> + const struct media_pad *remote, u32 flags)
> +{
> + struct video_device *vdev = media_entity_to_video_device(entity);
> + struct sun6i_video *video = video_get_drvdata(vdev);
> +
> + if (WARN_ON(video == NULL))
> + return 0;
> +
> + return sun6i_video_formats_init(video);
Why is this called here? Why not in video_init()?
>>>
>>> sun6i_video_init is in the driver probe context. sun6i_video_formats_init
>>> use media_entity_remote_pad and media_entity_to_v4l2_subdev to find the
>>> subdevs.
>>> The media_entity_remote_pad can't work before all the media pad linked.
>>
>> A video_init is typically called from the notify_complete callback.
>> Actually, that's where the video_register_device should be created as well.
>> When you create it in probe() there is possibly no sensor yet, so it would
>> be a dead video node (or worse, crash when used).
>>
>> There are still a lot of platform drivers that create the video node in the
>> probe, but it's not the right place if you rely on the async loading of
>> subdevs.
>
> That's not really related, but I'm not really sure it's a good way to
> operate. This essentially means that you might wait forever for a
> component in your pipeline to be probed, without any chance of it
> happening (not compiled, compiled as a module and not loaded, hardware
> defect preventing the driver from probing properly, etc), even though
> that component might not be essential.
We're talking straightforward video pipelines here. I.e. a source, some
processing units and a DMA engine at the end. There is no point in
creating a video node if the pipeline is not complete since you need
the full pipeline.
I've had bad experiences in the past where video nodes were created too
soon and part of the internal state was still incomplete, causing at best
weird behavior and at worst crashes.
More complex devices are a whole different ballgame.
Regards,
Hans
> This is how DRM operates, and you sometimes end up in some very dumb
> situations where you wait for say, the DSI controller to probe, while
> you only care about HDMI in your system.
>
> But this seems to be on of the hot topic these days, so we might
> discuss it further in some other thread :)
>
> Maxime
>
