Re: Kworld 340U (1b80:a340) kernel 4.8.0 ERROR: i2c_transfer returned: -6

[Kumar Vivek]

Thank you Frank! I appreciate your time.
This is what I have done so far -

On Wed, Jul 19, 2017 at 2:44 PM, Frank Schäfer
 wrote:
>
> Hi Kumar,
>
> I don't have time for the em28xx driver at the moment (and I also do not
> have access to a device with tda18271 tuner).
> But...
>
> Am 08.07.2017 um 22:29 schrieb Kumar Vivek:
>> New subscriber and first time poster. I have tried to read most of the
>> instructions and etiquettes regarding the mailing list but there might
>> still be some noob mistakes on my part.
>>
>> I have had this tuner for a while and I used it successfully in 2009
>> (with help from Markus Rechberger - who provided me with the
>> appropriate patch). I saw that the patches were included in the kernel
>> drivers and this was fully supported. I tried to use it again recently
>> and ran into problems and hence this mail. I have spent days trying to
>> figure out the problem and have been unsuccessful.
>>
>> I am using kernel 4.8.0
>>
>> The variant of this USB ATSC device I have has vid:pid = 1b80:a340 ,
>> EM2870 USB bridge, lgdt3304 demodulator/Frontend, TDA18271HDC2 tuner.
>>
>> I loaded the em28xx module with debugging on - including i2c bus scan
>> and i2c transfer.
>>
>> [  320.139648] em2870 #0 at em28xx_i2c_xfer: read stop addr=1c len=0:
>> [  320.139652] (pipe 0x8280): IN:  c0 02 00 00 1c 00 01 00
>> [  320.140008] <<< cf
>> [  320.140038] (pipe 0x8280): IN:  c0 00 00 00 05 00 01 00 <<< 00
>>
>> [  320.140732] em2870 #0: found i2c device @ 0x1c on bus 0 [lgdt330x]
>>
>> .
>> [  320.177163] (pipe 0x8280): IN:  c0 02 00 00 a0 00 01 00
>> [  320.177541] <<< 1a
>> [  320.177547] (pipe 0x8280): IN:  c0 00 00 00 05 00 01 00 <<< 00
>>
>> [  320.177663] em2870 #0: found i2c device @ 0xa0 on bus 0 [eeprom]
>> .
>>
>> [  320.186289] (pipe 0x8280): IN:  c0 02 00 00 c4 00 01 00
>> [  320.186659] <<< 84
>> [  320.186665] (pipe 0x8280): IN:  c0 00 00 00 05 00 01 00 <<< 00
>>
>> [  320.186945] em2870 #0: found i2c device @ 0xc4 on bus 0 [tuner (analog)]
>>
>> This is a bit strange since the tuner TDA18271HDC2 is at 0x60 (7 bit)
>> or 0xC0 (8 bit) i2c address and usually - i2cbus scan doesn't reveal
>> the tuner's address.
>>
>> And then this happens :
>> [  320.203404] em2870 #0: Identified as KWorld PlusTV 340U or UB435-Q
>> (ATSC) (card=76)
>> [  320.203406] em28xx: Currently, V4L2 is not supported on this model
>> [  320.203407] em2870 #0: dvb set to isoc mode.
>> [  320.260270] em2870 #0: Binding DVB extension
>> [  320.260274] em2870 #0 em28xx_alloc_urbs :em28xx: called
>> em28xx_alloc_isoc in mode 2
>> [  320.260276] em2870 #0 em28xx_uninit_usb_xfer :em28xx: called
>> em28xx_uninit_usb_xfer in mode 2
>> [  320.260279] (pipe 0x8280): IN:  c0 00 00 00 0c 00 01 00 <<< 00
>> [  320.260536] (pipe 0x8200): OUT: 40 00 00 00 0c 00 01 00 >>> 00
>> [  320.260631] (pipe 0x8200): OUT: 40 00 00 00 12 00 01 00 >>> 27
>> [  320.260833] (pipe 0x8200): OUT: 40 00 00 00 48 00 01 00 >>> 00
>> [  320.260987] (pipe 0x8200): OUT: 40 00 00 00 12 00 01 00 >>> 37
>> [  320.275990] (pipe 0x8280): IN:  c0 00 00 00 08 00 01 00 <<< ff
>> [  320.278154] (pipe 0x8200): OUT: 40 00 00 00 08 00 01 00 >>> 7d
>> [  320.337050] em2870 #0 at em28xx_i2c_xfer: write nonstop addr=1c len=2: 00 
>> 01
>> [  320.337056] (pipe 0x8200): OUT: 40 03 00 00 1c 00 02 00 >>>
>> [  320.337057]  00 01
>> [  320.337502] (pipe 0x8280): IN:  c0 00 00 00 05 00 01 00 <<< 00
>>
>> [  320.337618] em2870 #0 at em28xx_i2c_xfer: read stop addr=1c len=1:
>> [  320.337620] (pipe 0x8280): IN:  c0 02 00 00 1c 00 01 00
>> [  320.337860] <<< 30
>> [  320.337867] (pipe 0x8280): IN:  c0 00 00 00 05 00 01 00 <<< 00
>> [  320.337979]  30
>> [  320.337984] em2870 #0 at em28xx_i2c_xfer: write stop addr=1c len=3: 08 08 
>> 80
>> [  320.337987] (pipe 0x8200): OUT: 40 02 00 00 1c 00 03 00 >>>
>> [  320.337988]  08 08 80
>> [  320.338518] (pipe 0x8280): IN:  c0 00 00 00 05 00 01 00 <<< 00
>>
>> [  320.338618] em2870 #0 at em28xx_i2c_xfer: write nonstop addr=1c len=2: 08 
>> 08
>> [  320.338622] (pipe 0x8200): OUT: 40 03 00 00 1c 00 02 00 >>>
>> [  320.338623]  08 08
>> [  320.339018] (pipe 0x8280): IN:  c0 00 00 00 05 00 01 00 <<< 00
>>
>> [  320.339110] em2870 #0 at em28xx_i2c_xfer: read stop addr=1c len=1:
>> [  320.339113] (pipe 0x8280): IN:  c0 02 00 00 1c 00 01 00
>> [  320.339391] <<< 80
>> [  320.339397] (pipe 0x8280): IN:  c0 00 00 00 05 00 01 00 <<< 00
>> [  320.339490]  80
>> [  320.339496] em2870 #0 at em28xx_i2c_xfer: write stop addr=1c len=3: 08 08 
>> 00
>> [  320.339499] (pipe 0x8200): OUT: 40 02 00 00 1c 00 03 00 >>>
>> [  320.339499]  08 08 00
>> [  320.340768] (pipe 0x8280): IN:  c0 00 00 00 05 00 01 00 <<< 00
>>
>> [  320.341276] tda18271 12-0060: creating new instance
>> [  320.341279] em2870 #0 at em28xx_i2c_xfer: write nonstop addr=c0 len=1: 00
>> [  320.341283] (pipe 0x8200): OUT: 40 03 00 00 c0 00 01 00 

cron job: media_tree daily build: ERRORS

[Hans Verkuil]

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:   Sat Jul 29 05:00:15 CEST 2017
media-tree git hash:da48c948c263c9d87dfc64566b3373a858cc8aa2
media_build git hash:   1abc6be7b313cb92ff9128cea3d69df7f63e725f
v4l-utils git hash: 6b5204abea527469012d3c40b1909b199b532614
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: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.2.37-i686: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: WARNINGS
linux-3.12.67-i686: WARNINGS
linux-3.13.11-i686: WARNINGS
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: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9.26-i686: OK
linux-4.10.14-i686: ERRORS
linux-4.11-i686: OK
linux-4.12.1-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12.67-x86_64: WARNINGS
linux-3.13.11-x86_64: WARNINGS
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: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: ERRORS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
apps: WARNINGS
spec-git: OK
sparse: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Saturday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

Re: [PATCH v1] media: ov13858: Correct link-frequency and pixel-rate

[Tomasz Figa]

Hi Chiranjeevi,

On Sat, Jul 29, 2017 at 3:48 AM, Chiranjeevi Rapolu
 wrote:
> Previously both link-frequency and pixel-rate reported by
> the sensor was incorrect, resulting in incorrect FPS.
> Report link-frequency in Hz rather than link data rate in bps.
> Calculate pixel-rate from link-frequency.
>
> Signed-off-by: Chiranjeevi Rapolu 
> ---
>  drivers/media/i2c/ov13858.c | 28 +++-
>  1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
> index 86550d8..2a5bb43 100644
> --- a/drivers/media/i2c/ov13858.c
> +++ b/drivers/media/i2c/ov13858.c
> @@ -60,8 +60,8 @@
>  #define OV13858_VBLANK_MIN 56
>
>  /* HBLANK control - read only */
> -#define OV13858_PPL_540MHZ 2244
> -#define OV13858_PPL_1080MHZ4488
> +#define OV13858_PPL_270MHZ 2244
> +#define OV13858_PLL_540MHZ 4488

typo? "PPL" seems correct, because it's supposed to be pixels per line.

>
>  /* Exposure control */
>  #define OV13858_REG_EXPOSURE   0x3500
> @@ -944,31 +944,33 @@ struct ov13858_mode {
>
>  /* Configurations for supported link frequencies */
>  #define OV13858_NUM_OF_LINK_FREQS  2
> -#define OV13858_LINK_FREQ_1080MBPS 108000
> -#define OV13858_LINK_FREQ_540MBPS  54000
> +#define OV13858_LINK_FREQ_540MHZ   54000
> +#define OV13858_LINK_FREQ_270MHZ   27000
>  #define OV13858_LINK_FREQ_INDEX_0  0
>  #define OV13858_LINK_FREQ_INDEX_1  1
>
>  /* Menu items for LINK_FREQ V4L2 control */
>  static const s64 link_freq_menu_items[OV13858_NUM_OF_LINK_FREQS] = {
> -   OV13858_LINK_FREQ_1080MBPS,
> -   OV13858_LINK_FREQ_540MBPS
> +   OV13858_LINK_FREQ_540MHZ,
> +   OV13858_LINK_FREQ_270MHZ
>  };
>
>  /* Link frequency configs */
>  static const struct ov13858_link_freq_config
> link_freq_configs[OV13858_NUM_OF_LINK_FREQS] = {
> {
> -   .pixel_rate = 86400,
> -   .pixels_per_line = OV13858_PPL_1080MHZ,
> +   /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample 
> */
> +   .pixel_rate = ((uint64_t)OV13858_LINK_FREQ_540MHZ * 2 * 4) / 
> 10,

Instead of this cast, you can just define OV13858_LINK_FREQ_540MHZ to
be 54000ULL.

> +   .pixels_per_line = OV13858_PLL_540MHZ,

s/PLL/PPL/?

> .reg_list = {
> .num_of_regs = ARRAY_SIZE(mipi_data_rate_1080mbps),
> .regs = mipi_data_rate_1080mbps,
> }
> },
> {
> -   .pixel_rate = 43200,
> -   .pixels_per_line = OV13858_PPL_540MHZ,
> +   /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample 
> */
> +   .pixel_rate = ((uint64_t)OV13858_LINK_FREQ_270MHZ * 2 * 4) / 
> 10,

Ditto.

> +   .pixels_per_line = OV13858_PPL_270MHZ,
> .reg_list = {
> .num_of_regs = ARRAY_SIZE(mipi_data_rate_540mbps),
> .regs = mipi_data_rate_540mbps,
> @@ -1634,10 +1636,10 @@ static int ov13858_init_controls(struct ov13858 
> *ov13858)
>
> ov13858->hblank = v4l2_ctrl_new_std(
> ctrl_hdlr, _ctrl_ops, V4L2_CID_HBLANK,
> -   OV13858_PPL_1080MHZ - 
> ov13858->cur_mode->width,
> -   OV13858_PPL_1080MHZ - 
> ov13858->cur_mode->width,
> +   OV13858_PLL_540MHZ - ov13858->cur_mode->width,
> +   OV13858_PLL_540MHZ - ov13858->cur_mode->width,
> 1,
> -   OV13858_PPL_1080MHZ - 
> ov13858->cur_mode->width);
> +   OV13858_PLL_540MHZ - 
> ov13858->cur_mode->width);

Ditto.

Best regards,
Tomasz

[PATCH v1] media: ov13858: Increase digital gain granularity, range

[Chiranjeevi Rapolu]

Previously, possible digital gains were just 1x, 2x and 4x. These
coarse gains were not sufficient in fine-tuning the image capture.

Now, digital gain range is [0, 16x] with each step 1/1024, default 1x.
This is achieved through OV13858 MWB R/G/B gain controls.

Signed-off-by: Chiranjeevi Rapolu 
---
 drivers/media/i2c/ov13858.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index 86550d8..b78e323b 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -78,13 +78,13 @@
 #define OV13858_ANA_GAIN_DEFAULT   0x80
 
 /* Digital gain control */
-#define OV13858_REG_DIGITAL_GAIN   0x350a
-#define OV13858_DGTL_GAIN_MASK 0xf3
-#define OV13858_DGTL_GAIN_SHIFT2
-#define OV13858_DGTL_GAIN_MIN  1
-#define OV13858_DGTL_GAIN_MAX  4
-#define OV13858_DGTL_GAIN_STEP 1
-#define OV13858_DGTL_GAIN_DEFAULT  1
+#define OV13858_REG_B_MWB_GAIN 0x5100
+#define OV13858_REG_G_MWB_GAIN 0x5102
+#define OV13858_REG_R_MWB_GAIN 0x5104
+#define OV13858_DGTL_GAIN_MIN  0
+#define OV13858_DGTL_GAIN_MAX  16384   /* Max = 16 X */
+#define OV13858_DGTL_GAIN_DEFAULT  1024/* Default gain = 1 X */
+#define OV13858_DGTL_GAIN_STEP 1   /* Each step = 1/1024 */
 
 /* Test Pattern Control */
 #define OV13858_REG_TEST_PATTERN   0x4503
@@ -1161,21 +1161,21 @@ static int ov13858_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
 static int ov13858_update_digital_gain(struct ov13858 *ov13858, u32 d_gain)
 {
int ret;
-   u32 val;
 
-   if (d_gain == 3)
-   return -EINVAL;
+   ret = ov13858_write_reg(ov13858, OV13858_REG_B_MWB_GAIN,
+   OV13858_REG_VALUE_16BIT, d_gain);
+   if (ret)
+   return ret;
 
-   ret = ov13858_read_reg(ov13858, OV13858_REG_DIGITAL_GAIN,
-  OV13858_REG_VALUE_08BIT, );
+   ret = ov13858_write_reg(ov13858, OV13858_REG_G_MWB_GAIN,
+   OV13858_REG_VALUE_16BIT, d_gain);
if (ret)
return ret;
 
-   val &= OV13858_DGTL_GAIN_MASK;
-   val |= (d_gain - 1) << OV13858_DGTL_GAIN_SHIFT;
+   ret = ov13858_write_reg(ov13858, OV13858_REG_R_MWB_GAIN,
+   OV13858_REG_VALUE_16BIT, d_gain);
 
-   return ov13858_write_reg(ov13858, OV13858_REG_DIGITAL_GAIN,
-OV13858_REG_VALUE_08BIT, val);
+   return ret;
 }
 
 static int ov13858_enable_test_pattern(struct ov13858 *ov13858, u32 pattern)
-- 
1.9.1

[PATCH v1] media: ov13858: Correct link-frequency and pixel-rate

[Chiranjeevi Rapolu]

Previously both link-frequency and pixel-rate reported by
the sensor was incorrect, resulting in incorrect FPS.
Report link-frequency in Hz rather than link data rate in bps.
Calculate pixel-rate from link-frequency.

Signed-off-by: Chiranjeevi Rapolu 
---
 drivers/media/i2c/ov13858.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index 86550d8..2a5bb43 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -60,8 +60,8 @@
 #define OV13858_VBLANK_MIN 56
 
 /* HBLANK control - read only */
-#define OV13858_PPL_540MHZ 2244
-#define OV13858_PPL_1080MHZ4488
+#define OV13858_PPL_270MHZ 2244
+#define OV13858_PLL_540MHZ 4488
 
 /* Exposure control */
 #define OV13858_REG_EXPOSURE   0x3500
@@ -944,31 +944,33 @@ struct ov13858_mode {
 
 /* Configurations for supported link frequencies */
 #define OV13858_NUM_OF_LINK_FREQS  2
-#define OV13858_LINK_FREQ_1080MBPS 108000
-#define OV13858_LINK_FREQ_540MBPS  54000
+#define OV13858_LINK_FREQ_540MHZ   54000
+#define OV13858_LINK_FREQ_270MHZ   27000
 #define OV13858_LINK_FREQ_INDEX_0  0
 #define OV13858_LINK_FREQ_INDEX_1  1
 
 /* Menu items for LINK_FREQ V4L2 control */
 static const s64 link_freq_menu_items[OV13858_NUM_OF_LINK_FREQS] = {
-   OV13858_LINK_FREQ_1080MBPS,
-   OV13858_LINK_FREQ_540MBPS
+   OV13858_LINK_FREQ_540MHZ,
+   OV13858_LINK_FREQ_270MHZ
 };
 
 /* Link frequency configs */
 static const struct ov13858_link_freq_config
link_freq_configs[OV13858_NUM_OF_LINK_FREQS] = {
{
-   .pixel_rate = 86400,
-   .pixels_per_line = OV13858_PPL_1080MHZ,
+   /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
+   .pixel_rate = ((uint64_t)OV13858_LINK_FREQ_540MHZ * 2 * 4) / 10,
+   .pixels_per_line = OV13858_PLL_540MHZ,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mipi_data_rate_1080mbps),
.regs = mipi_data_rate_1080mbps,
}
},
{
-   .pixel_rate = 43200,
-   .pixels_per_line = OV13858_PPL_540MHZ,
+   /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
+   .pixel_rate = ((uint64_t)OV13858_LINK_FREQ_270MHZ * 2 * 4) / 10,
+   .pixels_per_line = OV13858_PPL_270MHZ,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mipi_data_rate_540mbps),
.regs = mipi_data_rate_540mbps,
@@ -1634,10 +1636,10 @@ static int ov13858_init_controls(struct ov13858 
*ov13858)
 
ov13858->hblank = v4l2_ctrl_new_std(
ctrl_hdlr, _ctrl_ops, V4L2_CID_HBLANK,
-   OV13858_PPL_1080MHZ - ov13858->cur_mode->width,
-   OV13858_PPL_1080MHZ - ov13858->cur_mode->width,
+   OV13858_PLL_540MHZ - ov13858->cur_mode->width,
+   OV13858_PLL_540MHZ - ov13858->cur_mode->width,
1,
-   OV13858_PPL_1080MHZ - ov13858->cur_mode->width);
+   OV13858_PLL_540MHZ - ov13858->cur_mode->width);
ov13858->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 
ov13858->exposure = v4l2_ctrl_new_std(
-- 
1.9.1

Re: [PATCH v2 2/3] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI)

[Maxime Ripard]

Hi,

On Thu, Jul 27, 2017 at 01:01:36PM +0800, Yong Deng wrote:
> Add binding documentation for Allwinner V3s CSI.
> 
> Signed-off-by: Yong Deng 
> ---
>  .../devicetree/bindings/media/sun6i-csi.txt| 49 
> ++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt 
> b/Documentation/devicetree/bindings/media/sun6i-csi.txt
> new file mode 100644
> index 000..f8d83f6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
> @@ -0,0 +1,49 @@
> +Allwinner V3s Camera Sensor Interface
> +--
> +
> +Required properties:
> +  - compatible: value must be "allwinner,sun8i-v3s-csi"
> +  - reg: base address and size of the memory-mapped region.
> +  - interrupts: interrupt associated to this IP
> +  - clocks: phandles to the clocks feeding the CSI
> +* ahb: the CSI interface clock

We've been bad at this, but we're trying to have the same clock name
here across all devices, and to use "bus" instead of "ahb".

Thanks!
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.

[Maxime Ripard]

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 

There's a significant amount of checkpatch warnings (and quite
important checks) in your driver. You should fix everything checkpatch
--strict reports.

> ---
>  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/Kconfig b/drivers/media/platform/Kconfig
> index 0c741d1..8371a87 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -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.

> + dev_dbg(csi->dev, "creating links for entity %s\n", local->name);
> +
> + while (1) {
> + /* Get the next endpoint and parse its link. */
> + next = of_graph_get_next_endpoint(entity->node, ep);
> + if (next == NULL)
> + break;
> +
> + of_node_put(ep);
> + ep = next;
> +
> + dev_dbg(csi->dev, "processing endpoint %s\n", ep->full_name);
> +
> + ret = v4l2_fwnode_parse_link(of_fwnode_handle(ep), );
> + if (ret < 0) {
> + dev_err(csi->dev, "failed to parse link for %s\n",
> + ep->full_name);
> + continue;
> + }
> +
> + /* Skip sink ports, they will be processed from the other end of
> +  * the link.
> +  */
> + if (link.local_port >= local->num_pads) {
> + dev_err(csi->dev, "invalid port number %u on %s\n",
> + link.local_port,
> + to_of_node(link.local_node)->full_name);
> + v4l2_fwnode_put_link();
> + ret = -EINVAL;
> + break;
> + }
> +
> + local_pad = >pads[link.local_port];
> +
> + if (local_pad->flags & MEDIA_PAD_FL_SINK) {
> + dev_dbg(csi->dev, "skipping sink port %s:%u\n",
> + to_of_node(link.local_node)->full_name,
> + link.local_port);
> + v4l2_fwnode_put_link();
> + continue;
> + }
> +
> + /* Skip video node, they will be processed separately. */
> + if (link.remote_node == of_fwnode_handle(csi->dev->of_node)) {
> + dev_dbg(csi->dev, "skipping CSI port %s:%u\n",
> + to_of_node(link.local_node)->full_name,
> + link.local_port);
> + v4l2_fwnode_put_link();
> + continue;
> + }
> +
> + /* Find the remote entity. */
> + ent = sun6i_graph_find_entity(csi,
> +   to_of_node(link.remote_node));
> + if (ent == NULL) {
> + dev_err(csi->dev, "no entity found for %s\n",
> + to_of_node(link.remote_node)->full_name);
> +  

Re: [RFCv2 PATCH 0/2] add VIDIOC_SUBDEV_QUERYCAP ioctl

[Hans Verkuil]

On 07/28/2017 04:28 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Friday 28 Jul 2017 16:04:48 Hans Verkuil wrote:
>> On 07/28/2017 03:25 PM, Laurent Pinchart wrote:
>>> On Friday 28 Jul 2017 13:05:27 Hans Verkuil wrote:
 From: Hans Verkuil 

 I tried to get this in back in 2015, but that effort stalled.

 Trying again, since I really need this in order to add proper v4l-subdev
 support to v4l2-ctl and v4l2-compliance. There currently is no way of
 unique identifying that the device really is a v4l-subdev device other
 than the device name (which can be changed by udev).

 So this patch series adds a VIDIOC_SUBDEV_QUERYCAP ioctl that is in
 the core so it's guaranteed to be there.

 If the subdev is part of an MC then it also gives the corresponding
 entity ID of the subdev and the major/minor numbers of the MC device
 so v4l2-compliance can relate the subdev device directly to the right
 MC device. The reserved array has room enough for more strings should
 we need them later, although I think what we have here is sufficient.
>>>
>>> I still think this is not correct for two reasons.
>>>
>>> First of all, the new querycap ioctl uses the same ioctl number as
>>> VIDIOC_QUERYCAP. The full 32-bit number is different as the structures
>>> used for the two ioctls currently have different sizes, but that's not
>>> guaranteed going forward when we'll extend the structures used by the two
>>> ioctls with new fields.
>>
>> I think it is extraordinarily unlikely that these two will ever become
>> identical. And anyway, we control that ourselves.
>>
>>> To solve this, if you really want to identify the type of device node at
>>> runtime, we should have a single ioctl supported by the two device nodes.
>>> Given that we"re running out of capabilities bits for VIDIOC_QUERYCAP,
>>> this could be a good occasion to introduce a new ioctl to query
>>> capabilities.
>>
>> This makes more sense :-)
>>
>>> The second reason is that I don't think we should report the media device
>>> node associated with the subdev. Userspace really needs to become
>>> MC-centric, starting with the MC device, and going to the video nodes and
>>> subdev nodes. The other way around just doesn't make sense to me.
>>
>> It's not for 'regular' applications. It's to easily find out to which media
>> device a /dev/v4l-subdevX belongs. Primarily for applications like
>> v4l2-compliance.
>>
>> We have the information, and right now there is no way to take a v4l-subdevX
>> device and determine of which media device it is part other than scanning
>> the topologies of all media devices. That's nuts. This is cheap and makes
>> life for a certain class of applications much easier. Creating good
>> compliance tests is critical and this is a small but important contribution
>> to that.
> 
> I fully agree with the need for compliance tools, but I believe they should 
> go 
> from MC to subdev, not the other way around. Let's discuss this below.
> 
>>> For MC-enabled devices, specifying subdev or video nodes by /dev node name
>>> is painful. When you have multiple video devices on the system, or even
>>> when you're modifying initialization order in a driver, the devnode names
>>> will not be stable across boots. I used to type them out manually in the
>>> very beginning and very quickly switched to retrieving them from the
>>> subdev entity name in my test scripts.
>>>
>>> What I'd like the compliance tools to do is to test all video nodes and
>>> subdev nodes for a given MC device, with an option to restrict the tests
>>> to a subset of the video devices and subdevs specified by media entity
>>> name. We obviously need to keep support for addressing video nodes
>>> manually as not all devices are MC-enabled, but for subdev we don't have
>>> to care about such a backward compatibility issue as there's currently no
>>> compliance tool.
>>
>> I want two things:
>>
>> 1) v4l2-compliance to be able to test a v4l-subdevX, just in isolation. And
>> to be able to find the corresponding media device and make sure that what
>> the v4l-subdev does is compatible with the entity/link information from the
>> MC.
> 
> Why do you want to test /dev/v4l-subdev$(random) instead of /dev/mediaX + 
> "subdev entity name" ?
> 
> Furthermore, if you want to test a subdev in complete isolation, why do you 
> need to know which media device it belongs to ?
> 
> By the way, I'd like Sakari to join the discussion, but he won't be back 
> before the end of next week.
> 
>> 2) make a media-compliance to look at the media topology as a whole.
>>
>> Having the major/minor numbers are specifically for 1.
>>
>> Actually, I really want to have the major/minor numbers of the media device
>> for /dev/videoX as well, but entity ID +  major + minor number would use up
>> the available space in struct v4l2_capability, so your suggestion of making
>> a new VIDIOC_EXT_QUERYCAP has merit.
>>
>>> On a side 

Re: [PATCH v3 02/12] intel-ipu3: mmu: implement driver

[Tomasz Figa]

On Fri, Jul 28, 2017 at 11:10 PM, Robin Murphy  wrote:
> On 26/07/17 11:38, Tomasz Figa wrote:
>> Hi Robin,
>>
>> On Wed, Jul 19, 2017 at 10:37 PM, Robin Murphy  wrote:
>>> On 19/07/17 04:12, Yong Zhi wrote:
 From: Tomasz Figa 

 This driver translates Intel IPU3 internal virtual
 address to physical address.

 Signed-off-by: Tomasz Figa 
 Signed-off-by: Yong Zhi 
 ---
  drivers/media/pci/intel/ipu3/Kconfig|   9 +
  drivers/media/pci/intel/ipu3/Makefile   |  15 +
  drivers/media/pci/intel/ipu3/ipu3-mmu.c | 639 
 
  drivers/media/pci/intel/ipu3/ipu3-mmu.h |  27 ++
  4 files changed, 690 insertions(+)
  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.c
  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.h

 diff --git a/drivers/media/pci/intel/ipu3/Kconfig 
 b/drivers/media/pci/intel/ipu3/Kconfig
 index 2a895d6..7bcdfa5 100644
 --- a/drivers/media/pci/intel/ipu3/Kconfig
 +++ b/drivers/media/pci/intel/ipu3/Kconfig
 @@ -15,3 +15,12 @@ config VIDEO_IPU3_CIO2
   Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
   connected camera.
   The module will be called ipu3-cio2.
 +
 +config INTEL_IPU3_MMU
 + tristate
>>>
>>> Shouldn't this be bool now?
>>
>> Well, depends on what we expect it to be. I still didn't see any good
>> reason not to make it a loadable module.
>
> Sure, conceptually there's no real reason it shouldn't be *allowed* to
> be built as a module, but without all the necessary symbols exported, a
> tristate here is only going to make allmodconfig builds fail.

Have we already completely dismissed the idea of exporting the IOMMU
symbols needed to make this driver a loadable module? I think the most
problematic ones were around the DMA mapping stuff, but IOMMU could be
still a module. Anyway, I think we might be removing the DMA mapping
implementation from the driver as the opinion of few other reviewers
seemed to be that this is reserved only to arch code.

 +static struct iommu_domain *ipu3_mmu_domain_alloc(unsigned int type)
 +{
 + struct ipu3_mmu_domain *mmu_dom;
 + u32 pteval;
 +
 + if (WARN(type != IOMMU_DOMAIN_DMA,
 +  "IPU3 MMU only supports DMA domains\n"))
 + return NULL;
 +
 + mmu_dom = kzalloc(sizeof(*mmu_dom), GFP_KERNEL);
 + if (!mmu_dom)
 + return NULL;
 +
 + if (iommu_get_dma_cookie(_dom->domain))
 + goto fail_domain;
 +
 + mmu_dom->domain.geometry.aperture_start = 0;
 + mmu_dom->domain.geometry.aperture_end =
 + DMA_BIT_MASK(IPU3_MMU_ADDRESS_BITS);
 + mmu_dom->domain.geometry.force_aperture = true;
 +
 + /*
 +  * The MMU does not have a "valid" bit, so we have to use a dummy
 +  * page for invalid entries.
 +  */
 + mmu_dom->dummy_page = kzalloc(IPU3_PAGE_SIZE, GFP_KERNEL);
 + if (!mmu_dom->dummy_page)
 + goto fail_cookie;
 + pteval = IPU3_ADDR2PTE(virt_to_phys(mmu_dom->dummy_page));
 + mmu_dom->dummy_page_pteval = pteval;
>>>
>>> Conceptually, would it make sense for the dummy page to be per-mmu,
>>> rather than per-domain? I realise it doesn't make much practical
>>> difference if you only expect to ever use a single DMA ops domain, but
>>> it would neatly mirror existing drivers which do a similar thing (e.g.
>>> the Mediatek IOMMUs).
>>
>> It makes it a bit complicated to achieve correctness against the IOMMU
>> API, because it would leave the page tables invalid if the domain is
>> detached from the MMU.
>
> In general, I'm not convinced it's sane for anyone to be calling
> iommu_map/unmap on a domain that isn't live. However, since this driver
> only supports DMA ops domains anyway, I don't see how that could happen
> at all - a device is always attached to its default domain well before
> its driver has a chance to probe and start making DMA API calls. For the
> default domain to be detached, all the devices would have to be removed,
> at which point there's nobody left to be making DMA API calls (plus it
> would have been torn down along with the group anyway).
>
> That said, another way to safely have no MMU dependency at all would be
> to just allocate it globally at driver init. Even if you ever did have
> multiple IPUs in a system, I don't see that any harm could come of them
> sharing the same scratch page either.
>

Yeah, allocating them globally at driver init could simplify the
things a bit indeed. Let me take a look.

 + /*
 +  * Allocate a dummy L2 page table with all entries pointing to
 +  * the dummy page.
 +  */
 + mmu_dom->dummy_l2pt = 

Re: [RFCv2 PATCH 0/2] add VIDIOC_SUBDEV_QUERYCAP ioctl

[Laurent Pinchart]

Hi Hans,

On Friday 28 Jul 2017 16:04:48 Hans Verkuil wrote:
> On 07/28/2017 03:25 PM, Laurent Pinchart wrote:
> > On Friday 28 Jul 2017 13:05:27 Hans Verkuil wrote:
> >> From: Hans Verkuil 
> >> 
> >> I tried to get this in back in 2015, but that effort stalled.
> >> 
> >> Trying again, since I really need this in order to add proper v4l-subdev
> >> support to v4l2-ctl and v4l2-compliance. There currently is no way of
> >> unique identifying that the device really is a v4l-subdev device other
> >> than the device name (which can be changed by udev).
> >> 
> >> So this patch series adds a VIDIOC_SUBDEV_QUERYCAP ioctl that is in
> >> the core so it's guaranteed to be there.
> >> 
> >> If the subdev is part of an MC then it also gives the corresponding
> >> entity ID of the subdev and the major/minor numbers of the MC device
> >> so v4l2-compliance can relate the subdev device directly to the right
> >> MC device. The reserved array has room enough for more strings should
> >> we need them later, although I think what we have here is sufficient.
> > 
> > I still think this is not correct for two reasons.
> > 
> > First of all, the new querycap ioctl uses the same ioctl number as
> > VIDIOC_QUERYCAP. The full 32-bit number is different as the structures
> > used for the two ioctls currently have different sizes, but that's not
> > guaranteed going forward when we'll extend the structures used by the two
> > ioctls with new fields.
> 
> I think it is extraordinarily unlikely that these two will ever become
> identical. And anyway, we control that ourselves.
> 
> > To solve this, if you really want to identify the type of device node at
> > runtime, we should have a single ioctl supported by the two device nodes.
> > Given that we"re running out of capabilities bits for VIDIOC_QUERYCAP,
> > this could be a good occasion to introduce a new ioctl to query
> > capabilities.
> 
> This makes more sense :-)
> 
> > The second reason is that I don't think we should report the media device
> > node associated with the subdev. Userspace really needs to become
> > MC-centric, starting with the MC device, and going to the video nodes and
> > subdev nodes. The other way around just doesn't make sense to me.
> 
> It's not for 'regular' applications. It's to easily find out to which media
> device a /dev/v4l-subdevX belongs. Primarily for applications like
> v4l2-compliance.
> 
> We have the information, and right now there is no way to take a v4l-subdevX
> device and determine of which media device it is part other than scanning
> the topologies of all media devices. That's nuts. This is cheap and makes
> life for a certain class of applications much easier. Creating good
> compliance tests is critical and this is a small but important contribution
> to that.

I fully agree with the need for compliance tools, but I believe they should go 
from MC to subdev, not the other way around. Let's discuss this below.

> > For MC-enabled devices, specifying subdev or video nodes by /dev node name
> > is painful. When you have multiple video devices on the system, or even
> > when you're modifying initialization order in a driver, the devnode names
> > will not be stable across boots. I used to type them out manually in the
> > very beginning and very quickly switched to retrieving them from the
> > subdev entity name in my test scripts.
> > 
> > What I'd like the compliance tools to do is to test all video nodes and
> > subdev nodes for a given MC device, with an option to restrict the tests
> > to a subset of the video devices and subdevs specified by media entity
> > name. We obviously need to keep support for addressing video nodes
> > manually as not all devices are MC-enabled, but for subdev we don't have
> > to care about such a backward compatibility issue as there's currently no
> > compliance tool.
> 
> I want two things:
> 
> 1) v4l2-compliance to be able to test a v4l-subdevX, just in isolation. And
> to be able to find the corresponding media device and make sure that what
> the v4l-subdev does is compatible with the entity/link information from the
> MC.

Why do you want to test /dev/v4l-subdev$(random) instead of /dev/mediaX + 
"subdev entity name" ?

Furthermore, if you want to test a subdev in complete isolation, why do you 
need to know which media device it belongs to ?

By the way, I'd like Sakari to join the discussion, but he won't be back 
before the end of next week.

> 2) make a media-compliance to look at the media topology as a whole.
> 
> Having the major/minor numbers are specifically for 1.
> 
> Actually, I really want to have the major/minor numbers of the media device
> for /dev/videoX as well, but entity ID +  major + minor number would use up
> the available space in struct v4l2_capability, so your suggestion of making
> a new VIDIOC_EXT_QUERYCAP has merit.
> 
> > On a side note, I believe subdev nodes should depend on MC. It has been a
> > historical mistake not to do so, and 

Re: [RFCv2 PATCH 0/2] add VIDIOC_SUBDEV_QUERYCAP ioctl

[Hans Verkuil]

On 07/28/2017 04:04 PM, Hans Verkuil wrote:
> On 07/28/2017 03:25 PM, Laurent Pinchart wrote:
>> To solve this, if you really want to identify the type of device node at 
>> runtime, we should have a single ioctl supported by the two device nodes. 
>> Given that we"re running out of capabilities bits for VIDIOC_QUERYCAP, this 
>> could be a good occasion to introduce a new ioctl to query capabilities.
> 
> This makes more sense :-)

Here is a quick proposal:

struct v4l2_ext_capability {
chardriver[16];
charname[32];
charbus_info[32];
__u64   device_caps;
__u32   version;
__u32   entity_id;
/* Corresponding media controller device node specifications */
__u32   media_node_major;
__u32   media_node_minor;
__u32   reserved[16];
};

#define V4L2_CAP_SUBDEV 0x0008  /* This is a v4l-subdev 
device */
#define V4L2_CAP_ENTITY 0x0800  /* MC entity */

#define VIDIOC_EXT_QUERYCAP _IOR('V', 104, struct 
v4l2_ext_capability)

We keep the existing caps, but double the size of the device_caps field.

Add a CAP_SUBDEV to indicate that it is a subdev, and a CAP_ENTITY to indicate
that it is part of the media controller.

I dropped the old 'capabilities' field. In V4L2 that is meant to give the sum
of all the capabilities of all the video/vbi/radio/swradio device nodes, but
it never worked and is inconsistently implemented.

It's really historical so I decided to drop it. I also replaced __u8 by char
for the string fields (__u8 was very, very annoying!).

No driver changes needed for this, it can all be handled in the core.

Regards,

Hans

Re: [PATCH v3 02/12] intel-ipu3: mmu: implement driver

[Robin Murphy]

On 26/07/17 11:38, Tomasz Figa wrote:
> Hi Robin,
> 
> On Wed, Jul 19, 2017 at 10:37 PM, Robin Murphy  wrote:
>> On 19/07/17 04:12, Yong Zhi wrote:
>>> From: Tomasz Figa 
>>>
>>> This driver translates Intel IPU3 internal virtual
>>> address to physical address.
>>>
>>> Signed-off-by: Tomasz Figa 
>>> Signed-off-by: Yong Zhi 
>>> ---
>>>  drivers/media/pci/intel/ipu3/Kconfig|   9 +
>>>  drivers/media/pci/intel/ipu3/Makefile   |  15 +
>>>  drivers/media/pci/intel/ipu3/ipu3-mmu.c | 639 
>>> 
>>>  drivers/media/pci/intel/ipu3/ipu3-mmu.h |  27 ++
>>>  4 files changed, 690 insertions(+)
>>>  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.c
>>>  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.h
>>>
>>> diff --git a/drivers/media/pci/intel/ipu3/Kconfig 
>>> b/drivers/media/pci/intel/ipu3/Kconfig
>>> index 2a895d6..7bcdfa5 100644
>>> --- a/drivers/media/pci/intel/ipu3/Kconfig
>>> +++ b/drivers/media/pci/intel/ipu3/Kconfig
>>> @@ -15,3 +15,12 @@ config VIDEO_IPU3_CIO2
>>>   Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
>>>   connected camera.
>>>   The module will be called ipu3-cio2.
>>> +
>>> +config INTEL_IPU3_MMU
>>> + tristate
>>
>> Shouldn't this be bool now?
> 
> Well, depends on what we expect it to be. I still didn't see any good
> reason not to make it a loadable module.

Sure, conceptually there's no real reason it shouldn't be *allowed* to
be built as a module, but without all the necessary symbols exported, a
tristate here is only going to make allmodconfig builds fail.

>>> + default n
>>> + select IOMMU_API
>>> + select IOMMU_IOVA
>>> + ---help---
>>> +   For IPU3, this option enables its MMU driver to translate its 
>>> internal
>>> +   virtual address to 39 bits wide physical address for 64GBytes space 
>>> access.
>>> diff --git a/drivers/media/pci/intel/ipu3/Makefile 
>>> b/drivers/media/pci/intel/ipu3/Makefile
>>> index 20186e3..91cac9c 100644
>>> --- a/drivers/media/pci/intel/ipu3/Makefile
>>> +++ b/drivers/media/pci/intel/ipu3/Makefile
>>> @@ -1 +1,16 @@
>>> +#
>>> +#  Copyright (c) 2017, Intel Corporation.
>>> +#
>>> +#  This program is free software; you can redistribute it and/or modify it
>>> +#  under the terms and conditions of the GNU General Public License,
>>> +#  version 2, as published by the Free Software Foundation.
>>> +#
>>> +#  This program is distributed in the hope 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.
>>> +#
>>> +
>>>  obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
>>> +obj-$(CONFIG_INTEL_IPU3_MMU) += ipu3-mmu.o
>>> +
>>> diff --git a/drivers/media/pci/intel/ipu3/ipu3-mmu.c 
>>> b/drivers/media/pci/intel/ipu3/ipu3-mmu.c
>>> new file mode 100644
>>> index 000..093b821
>>> --- /dev/null
>>> +++ b/drivers/media/pci/intel/ipu3/ipu3-mmu.c
>>> @@ -0,0 +1,639 @@
>>> +/*
>>> + * Copyright (c) 2017 Intel Corporation.
>>> + * Copyright (C) 2017 Google, Inc.
>>> + *
>>> + * 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 
>>> +#include 
>>> +
>>> +#include 
>>> +
>>> +#include "ipu3-mmu.h"
>>> +
>>> +#define IPU3_PAGE_SHIFT  12
>>> +#define IPU3_PAGE_SIZE   (1UL << IPU3_PAGE_SHIFT)
>>> +
>>> +#define IPU3_PT_BITS 10
>>> +#define IPU3_PT_PTES (1UL << IPU3_PT_BITS)
>>> +
>>> +#define IPU3_ADDR2PTE(addr)  ((addr) >> IPU3_PAGE_SHIFT)
>>> +#define IPU3_PTE2ADDR(pte)   ((phys_addr_t)(pte) << IPU3_PAGE_SHIFT)
>>> +
>>> +#define IPU3_L2PT_SHIFT  IPU3_PT_BITS
>>> +#define IPU3_L2PT_MASK   ((1UL << IPU3_L2PT_SHIFT) - 1)
>>> +
>>> +#define IPU3_L1PT_SHIFT  IPU3_PT_BITS
>>> +#define IPU3_L1PT_MASK   ((1UL << IPU3_L1PT_SHIFT) - 1)
>>> +
>>> +#define IPU3_MMU_ADDRESS_BITS(IPU3_PAGE_SHIFT + \
>>> +  IPU3_L2PT_SHIFT + \
>>> +  IPU3_L1PT_SHIFT)
>>> +
>>> +#define IMGU_REG_BASE0x4000
>>> +#define REG_TLB_INVALIDATE   (IMGU_REG_BASE + 0x300)
>>> +#define TLB_INVALIDATE   1
>>> +#define REG_L1_PHYS  (IMGU_REG_BASE + 0x304) /* 27-bit pfn */
>>> +#define REG_GP_HALT  (IMGU_REG_BASE + 0x5dc)
>>> +#define 

Re: [RFCv2 PATCH 0/2] add VIDIOC_SUBDEV_QUERYCAP ioctl

[Hans Verkuil]

On 07/28/2017 03:25 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patches.
> 
> On Friday 28 Jul 2017 13:05:27 Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> I tried to get this in back in 2015, but that effort stalled.
>>
>> Trying again, since I really need this in order to add proper v4l-subdev
>> support to v4l2-ctl and v4l2-compliance. There currently is no way of
>> unique identifying that the device really is a v4l-subdev device other
>> than the device name (which can be changed by udev).
>>
>> So this patch series adds a VIDIOC_SUBDEV_QUERYCAP ioctl that is in
>> the core so it's guaranteed to be there.
>>
>> If the subdev is part of an MC then it also gives the corresponding
>> entity ID of the subdev and the major/minor numbers of the MC device
>> so v4l2-compliance can relate the subdev device directly to the right
>> MC device. The reserved array has room enough for more strings should
>> we need them later, although I think what we have here is sufficient.
> 
> I still think this is not correct for two reasons.
> 
> First of all, the new querycap ioctl uses the same ioctl number as 
> VIDIOC_QUERYCAP. The full 32-bit number is different as the structures used 
> for the two ioctls currently have different sizes, but that's not guaranteed 
> going forward when we'll extend the structures used by the two ioctls with 
> new 
> fields.

I think it is extraordinarily unlikely that these two will ever become 
identical.
And anyway, we control that ourselves.

> To solve this, if you really want to identify the type of device node at 
> runtime, we should have a single ioctl supported by the two device nodes. 
> Given that we"re running out of capabilities bits for VIDIOC_QUERYCAP, this 
> could be a good occasion to introduce a new ioctl to query capabilities.

This makes more sense :-)

> The second reason is that I don't think we should report the media device 
> node 
> associated with the subdev. Userspace really needs to become MC-centric, 
> starting with the MC device, and going to the video nodes and subdev nodes. 
> The other way around just doesn't make sense to me.

It's not for 'regular' applications. It's to easily find out to which media
device a /dev/v4l-subdevX belongs. Primarily for applications like 
v4l2-compliance.

We have the information, and right now there is no way to take a v4l-subdevX 
device
and determine of which media device it is part other than scanning the 
topologies
of all media devices. That's nuts. This is cheap and makes life for a certain
class of applications much easier. Creating good compliance tests is critical
and this is a small but important contribution to that.

> For MC-enabled devices, specifying subdev or video nodes by /dev node name is 
> painful. When you have multiple video devices on the system, or even when 
> you're modifying initialization order in a driver, the devnode names will not 
> be stable across boots. I used to type them out manually in the very 
> beginning 
> and very quickly switched to retrieving them from the subdev entity name in 
> my 
> test scripts.
> 
> What I'd like the compliance tools to do is to test all video nodes and 
> subdev 
> nodes for a given MC device, with an option to restrict the tests to a subset 
> of the video devices and subdevs specified by media entity name. We obviously 
> need to keep support for addressing video nodes manually as not all devices 
> are MC-enabled, but for subdev we don't have to care about such a backward 
> compatibility issue as there's currently no compliance tool.

I want two things:

1) v4l2-compliance to be able to test a v4l-subdevX, just in isolation. And to
   be able to find the corresponding media device and make sure that what the
   v4l-subdev does is compatible with the entity/link information from the MC.

2) make a media-compliance to look at the media topology as a whole.

Having the major/minor numbers are specifically for 1.

Actually, I really want to have the major/minor numbers of the media device for
/dev/videoX as well, but entity ID +  major + minor number would use up the
available space in struct v4l2_capability, so your suggestion of making a new
VIDIOC_EXT_QUERYCAP has merit.

> On a side note, I believe subdev nodes should depend on MC. It has been a 
> historical mistake not to do so, and as far as I can see only three drivers 
> register subdev nodes without registering a media device. They should be 
> fixed 
> if they want to benefit from the compliance tool.

Which ones?

I'm not opposed to that. It would simplify matters quite a bit.

But I very, very strongly believe that a VIDIOC_EXT_QUERYCAP should return the
corresponding entity ID and /dev/mediaX major and minor numbers. It's very 
useful
information for a certain class of applications.

Heck, I want to do 'v4l2-ctl -d /dev/video0 -D' or 'v4l2-ctl -d 
/dev/v4l-subdev0'
and see not only the device capabilities, but also the corresponding entity

Re: [media] vimc: API proposal, configuring the topology from user space

[Hans Verkuil]

Hi Helen,

Finally after way too long I found some time to review this. See my comments
below.

On 04/11/2017 12:53 AM, Helen Koike wrote:
> 
> Hi,
> 
> Continuing the discussion about the API of the vimc driver, I made some 
> changes
> based on the previous comments, please see below and let me know your 
> opinion about it.
> 
> Helen
> 
> /***
> Configfs considerations:
> /
> Informal definitions:
>   subsystem: the root driver folder in user space (/configfs/vimc)
>   item: aka a folder in user space
>   attributes: aka files in the folder
>   group: aka a folder that can contain subfolders (parent and child 
> relation)
>   default group: aka a subfolder that is created automatically when the 
> "parent" folder is created
>   it is not considered a child in terms of rmdir
> 
> * Performing rmdir in a group will fail if it contain children that are 
> not default groups, i.e, if the
> folder contain subfolders that are default group, then it can be removed 
> with rmdir, if the
> subfolders were created with mkdir, then rmdir in the parent will fail.
> 
> * Configfs has the notion of committable item but it is not implemented 
> yet. A committable item is an item
> that can be in one of two parent folders called: live and pending. The 
> idea is to create and modify the item
> in the pending directory and then to move the item through a rename to 
> the live directory where
> it can't be modified. This seems to be a nice feature for vimc, but as 
> it is not available yet the
> proposal below won't be based on this.
> 
> * Groups can be dynamically created/destroyed by the driver whenever it 
> wants. Afaik attributes can only
> be created when the group or item is created and symlinks can only be 
> create from user space, i.e, the
> driver don't know how to create/destroy attributes or symlinks in anytime.
> 
> /***
> The API:
> /
> 
> In short, a topology like this one: http://goo.gl/Y7eUfu
> Would look like this filesystem tree: https://goo.gl/mEOmOf

This mentions 'Yellow' lines, but since you dropped symlinks these no
longer exist. You probably need to update the legend.

> 
> v3 core changes:
> - I removed the use of symlinks as I wans't able to see how to do it nicely.
> - I use the names of the folders created by user space to retrieve 
> information at mkdir time
> - hotplug file in each entity
> - hotplug file in each device
> - reset file in each device
> 
> * The /configfs/vimc subsystem
> empty when the driver is loaded

I'm not sure about that. I think it would make sense that vimc when loaded
would make one instance, unless otherwise told via a module option.

Something like this (taken from vivid):

parm:   n_devs: number of driver instances to create (uint)

By default this is 1, but can also be 0, 2, 3, etc.

> 
> * Create a device
> Userspace can create a new vimc device with:
> 
>   $ mkdir /configfs/vimc/any_name
>   Example:
>   $ mkdir /configfs/vimc/vimc0
>   $ ls -l /configfs/vimc/vimc0
>   hotplug
>   reset
>   entities/
>   links/
> 
> entities/ and links/ folder are default groups, thus they don't prevent 
> rmdir vimc0/, but
> rmdir will fail if it has any child inside entities/ or links/.
> hotplug is used to plug and unplug the device, it can read "plugged" or 
> "unplugged" and user can
> write "plug" or "unplug" to change its state.

I would also support writing "plugged" and "unplugged". I.e. support both 
variants.

> Changing hotplug state will never fail as the configfs tree will always 
> be in a valid state.
> reset is used to easily destroy all the topology without the need to 
> walk through all the children
> to perform rmdir, writing 1 to reset file will set hotplug to 
> "unplugged" and erase all folders
> under entities/ and links/.
> 
> * Create an entity
> Userspace can create a new entity with:
> 
>   $ mkdir /configfs/vimc/vimc0/entities/:
>   Example:
>   $ mkdir /configfs/vimc/vimc0/entities/sensor:SensorA
>   $ ls -l /configfs/vimc/vimc0/entities/sensor:SensorA
>   hotplug
>   pad:source:0/
> 
> The name of the folder needs to be in the format : or it 
> will be rejected, this allows the
> creation of the right pads according to its role at mkdir time, 
> eliminating the previously proposed role
> and name files.
> hotplug is used to plug and unplug the hw block, it can read "plugged" 
> or "unplugged" and user can
> write "plug" or "unplug" to change its state. As we don't support this 
> yet in the media core, changing it
> will only be allowed if /configfs/vimc/vimc0/hotplug is "unplugged".
> hotplug file is "unplugged" by default.
> Pads will be created as default groups with the name in the format 
> pad:: and it
> will be an empty folder.
> If the hw block supports different number of pads, we could expose two 
> files:
> sinks
> sources
> where the user space 

[PATCH] [media] coda: reduce iram size to leave space for suspend to ram

[Philipp Zabel]

From: Jan Luebbe 

The on-chip SRAM of i.MX6S is only 128 KiB. 4 KiB of that are allocated
for suspend to RAM since commit df595746fa69 ("ARM: imx: add suspend in
ocram support for i.mx6q"). Reduce the requested IRAM size to 124 KiB to
avoid an allocation failure that causes the coda driver to not use the
SRAM at all.

Signed-off-by: Jan Luebbe 
Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda/coda-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/coda/coda-common.c 
b/drivers/media/platform/coda/coda-common.c
index 40f31038249d9..27c613752fbf7 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -2443,7 +2443,7 @@ static const struct coda_devtype coda_devdata[] = {
.num_vdevs= ARRAY_SIZE(coda9_video_devices),
.workbuf_size = 80 * 1024,
.tempbuf_size = 204 * 1024,
-   .iram_size= 0x2,
+   .iram_size= 0x1f000, /* leave 4k for suspend code */
},
 };
 
-- 
2.11.0

[PATCH] [media] coda: fix decoder sequence init escape flag

[Philipp Zabel]

coda_command_sync calls coda_command_async, which writes the
bit_stream_param context variable into the BIT_STREAM_PARAM register,
overwriting the previously set value during coda_start_decoding. Instead
of writing to the register, set bit_stream_param to ensure that the
decoder sequence init command is executed with the escape flag set.

Signed-off-by: Philipp Zabel 
---
 drivers/media/platform/coda/coda-bit.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c 
b/drivers/media/platform/coda/coda-bit.c
index 95e4b74d5dd01..291c409339357 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1639,9 +1639,6 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
ctx->frm_dis_flg = 0;
coda_write(dev, 0, CODA_REG_BIT_FRM_DIS_FLG(ctx->reg_idx));
 
-   coda_write(dev, CODA_BIT_DEC_SEQ_INIT_ESCAPE,
-   CODA_REG_BIT_BIT_STREAM_PARAM);
-
coda_write(dev, bitstream_buf, CODA_CMD_DEC_SEQ_BB_START);
coda_write(dev, bitstream_size / 1024, CODA_CMD_DEC_SEQ_BB_SIZE);
val = 0;
@@ -1676,18 +1673,18 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
if (dev->devtype->product != CODA_960)
coda_write(dev, 0, CODA_CMD_DEC_SEQ_SRC_SIZE);
 
-   if (coda_command_sync(ctx, CODA_COMMAND_SEQ_INIT)) {
+   ctx->bit_stream_param = CODA_BIT_DEC_SEQ_INIT_ESCAPE;
+   ret = coda_command_sync(ctx, CODA_COMMAND_SEQ_INIT);
+   ctx->bit_stream_param = 0;
+   if (ret) {
v4l2_err(>v4l2_dev, "CODA_COMMAND_SEQ_INIT timeout\n");
-   coda_write(dev, 0, CODA_REG_BIT_BIT_STREAM_PARAM);
-   return -ETIMEDOUT;
+   return ret;
}
ctx->initialized = 1;
 
/* Update kfifo out pointer from coda bitstream read pointer */
coda_kfifo_sync_from_device(ctx);
 
-   coda_write(dev, 0, CODA_REG_BIT_BIT_STREAM_PARAM);
-
if (coda_read(dev, CODA_RET_DEC_SEQ_SUCCESS) == 0) {
v4l2_err(>v4l2_dev,
"CODA_COMMAND_SEQ_INIT failed, error code = %d\n",
-- 
2.11.0

Re: [RFCv2 PATCH 0/2] add VIDIOC_SUBDEV_QUERYCAP ioctl

[Laurent Pinchart]

Hi Hans,

Thank you for the patches.

On Friday 28 Jul 2017 13:05:27 Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> I tried to get this in back in 2015, but that effort stalled.
> 
> Trying again, since I really need this in order to add proper v4l-subdev
> support to v4l2-ctl and v4l2-compliance. There currently is no way of
> unique identifying that the device really is a v4l-subdev device other
> than the device name (which can be changed by udev).
> 
> So this patch series adds a VIDIOC_SUBDEV_QUERYCAP ioctl that is in
> the core so it's guaranteed to be there.
> 
> If the subdev is part of an MC then it also gives the corresponding
> entity ID of the subdev and the major/minor numbers of the MC device
> so v4l2-compliance can relate the subdev device directly to the right
> MC device. The reserved array has room enough for more strings should
> we need them later, although I think what we have here is sufficient.

I still think this is not correct for two reasons.

First of all, the new querycap ioctl uses the same ioctl number as 
VIDIOC_QUERYCAP. The full 32-bit number is different as the structures used 
for the two ioctls currently have different sizes, but that's not guaranteed 
going forward when we'll extend the structures used by the two ioctls with new 
fields.

To solve this, if you really want to identify the type of device node at 
runtime, we should have a single ioctl supported by the two device nodes. 
Given that we"re running out of capabilities bits for VIDIOC_QUERYCAP, this 
could be a good occasion to introduce a new ioctl to query capabilities.

The second reason is that I don't think we should report the media device node 
associated with the subdev. Userspace really needs to become MC-centric, 
starting with the MC device, and going to the video nodes and subdev nodes. 
The other way around just doesn't make sense to me.

For MC-enabled devices, specifying subdev or video nodes by /dev node name is 
painful. When you have multiple video devices on the system, or even when 
you're modifying initialization order in a driver, the devnode names will not 
be stable across boots. I used to type them out manually in the very beginning 
and very quickly switched to retrieving them from the subdev entity name in my 
test scripts.

What I'd like the compliance tools to do is to test all video nodes and subdev 
nodes for a given MC device, with an option to restrict the tests to a subset 
of the video devices and subdevs specified by media entity name. We obviously 
need to keep support for addressing video nodes manually as not all devices 
are MC-enabled, but for subdev we don't have to care about such a backward 
compatibility issue as there's currently no compliance tool.

On a side note, I believe subdev nodes should depend on MC. It has been a 
historical mistake not to do so, and as far as I can see only three drivers 
register subdev nodes without registering a media device. They should be fixed 
if they want to benefit from the compliance tool.

> Changes since v1:
> - Add name field. Without that it is hard to figure out which subdev
>   it is since the entity ID is not very human readable.
> 
> Hans Verkuil (2):
>   v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl
>   v4l: document VIDIOC_SUBDEV_QUERYCAP
> 
>  Documentation/media/uapi/v4l/user-func.rst |   1 +
>  .../media/uapi/v4l/vidioc-subdev-querycap.rst  | 121 +
>  drivers/media/v4l2-core/v4l2-subdev.c  |  27 +
>  include/uapi/linux/v4l2-subdev.h   |  31 ++
>  4 files changed, 180 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/vidioc-subdev-querycap.rst

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] [media] vb2: core: Lower the log level of debug outputs

[Hans Verkuil]

On 06/28/2017 12:57 AM, Hirokazu Honda wrote:
> Hi,
> 
> According to patch work, this patch are not approved yet and its
> status are "Changes Requested."
> What changes are necessary actually?
> If there is no necessary change, can you approve this patch?

I was considering to have more fine grained control by changing
the debug parameter to a bitmask. But after thinking about it a
bit more I decided that this patch is OK after all.

I'll pick it up the next time I prepare a pull request.

Regards,

Hans

> 
> Best,
> Hirokazu Honda
> 
> On Thu, Jun 8, 2017 at 2:33 PM, Joe Perches  wrote:
>> On Thu, 2017-06-08 at 14:24 +0900, Tomasz Figa wrote:
>>> On Thu, Jun 8, 2017 at 2:16 PM, Joe Perches  wrote:
>> []
 If there automated systems that rely on specific levels, then
 changing the levels of individual messages could also cause
 those automated systems to fail.
>>>
>>> Well, that might be true for some of them indeed. I was thinking about
>>> our use case, which relies on particular numbers to get expected
>>> verbosity levels not caring about particular messages. I guess the
>>> break all or none rule is going to apply here, so we should do the
>>> bitmap conversion indeed. :)
>>>
>>> On the other hand, I think it would be still preferable to do the
>>> conversion in a separate patch.
>>
>> Right.  No worries.
>>

Re: [PATCH 4/6 v5] uvcvideo: add a metadata device node

[Guennadi Liakhovetski]

Hi Hans,

On Fri, 28 Jul 2017, Hans Verkuil wrote:

> On 07/28/2017 02:33 PM, Guennadi Liakhovetski wrote:

[snip]

> > +/**
> > + * struct uvc_meta_buf - metadata buffer building block
> > + * @ns - system timestamp of the payload in nanoseconds
> > + * @sof- USB Frame Number
> > + * @length - length of the payload header
> > + * @flags  - payload header flags
> > + * @buf- optional device-specific header data
> > + *
> > + * UVC metadata nodes fill buffers with possibly multiple instances of this
> > + * struct. The first two fields are added by the driver, they can be used 
> > for
> > + * clock synchronisation. The rest is an exact copy of a UVC payload 
> > header.
> > + * Only complete objects with complete buffers are included. Therefore it's
> > + * always sizeof(meta->ts) + sizeof(meta->sof) + meta->length bytes large.
> > + */
> > +struct uvc_meta_buf {
> > +   __u64 ns;
> > +   __u16 sof;
> > +   __u8 length;
> > +   __u8 flags;
> 
> I think the struct layout is architecture dependent. I could be wrong, but I 
> think
> for x64 there is a 4-byte hole here, but not for arm32/arm64.
> 
> Please double-check the struct layout.

It worked for me so far on an x86-64 system, I was able to access buffer 
data correctly, but yes, it's probably safer to use __packed.

> BTW: __u8 for length? The payload can never be longer in UVC?

No, it cannot. That's exactly how UVC defines it.

Thanks
Guennadi

> 
> Regards,
> 
>   Hans
> 
> > +   __u8 buf[];
> > +};
> > +
> >  #endif
> > 
> 

[GIT PULL FOR v4.14] Add meson-ao-cec driver

[Hans Verkuil]

The following changes since commit da48c948c263c9d87dfc64566b3373a858cc8aa2:

  media: fix warning on v4l2_subdev_call() result interpreted as bool 
(2017-07-26 13:43:17 -0400)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git meson-cec

for you to fetch changes up to 098b47db51270f6e671773160411e3d285ea0d66:

  dt-bindings: media: Add Amlogic Meson AO-CEC bindings (2017-07-28 14:58:38 
+0200)


Neil Armstrong (2):
  platform: Add Amlogic Meson AO CEC Controller driver
  dt-bindings: media: Add Amlogic Meson AO-CEC bindings

 Documentation/devicetree/bindings/media/meson-ao-cec.txt |  28 ++
 drivers/media/platform/Kconfig   |  11 +
 drivers/media/platform/Makefile  |   2 +
 drivers/media/platform/meson/Makefile|   1 +
 drivers/media/platform/meson/ao-cec.c| 744 
+++
 5 files changed, 786 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/meson-ao-cec.txt
 create mode 100644 drivers/media/platform/meson/Makefile
 create mode 100644 drivers/media/platform/meson/ao-cec.c

Re: [PATCH v3 1/2] platform: Add Amlogic Meson AO CEC Controller driver

[Hans Verkuil]

On 07/27/2017 05:20 PM, Neil Armstrong wrote:
> The Amlogic SoC embeds a standalone CEC controller, this patch adds a driver
> for such controller.
> The controller does not need HPD to be active, and could support up to max
> 5 logical addresses, but only 1 is handled since the Suspend firmware can
> make use of this unique logical address to wake up the device.
> 
> The Suspend firmware configuration will be added in an other patchset.
> 
> Signed-off-by: Neil Armstrong 
> ---
>  drivers/media/platform/Kconfig|  11 +
>  drivers/media/platform/Makefile   |   2 +
>  drivers/media/platform/meson/Makefile |   1 +
>  drivers/media/platform/meson/ao-cec.c | 744 
> ++
>  4 files changed, 758 insertions(+)
>  create mode 100644 drivers/media/platform/meson/Makefile
>  create mode 100644 drivers/media/platform/meson/ao-cec.c
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 1313cd5..1e67381 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -536,6 +536,17 @@ menuconfig CEC_PLATFORM_DRIVERS
>  
>  if CEC_PLATFORM_DRIVERS
>  
> +config VIDEO_MESON_AO_CEC
> + tristate "Amlogic Meson AO CEC driver"
> + depends on ARCH_MESON || COMPILE_TEST
> + select CEC_CORE
> + select CEC_NOTIFIER
> + ---help---
> +   This is a driver for Amlogic Meson SoCs AO CEC interface. It uses the
> +   generic CEC framework interface.
> +   CEC bus is present in the HDMI connector and enables communication
> +   between compatible devices.
> +
>  config VIDEO_SAMSUNG_S5P_CEC
> tristate "Samsung S5P CEC driver"
> depends on PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 9beadc7..a52d7b6 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -86,3 +86,5 @@ obj-$(CONFIG_VIDEO_MEDIATEK_MDP)+= mtk-mdp/
>  obj-$(CONFIG_VIDEO_MEDIATEK_JPEG)+= mtk-jpeg/
>  
>  obj-$(CONFIG_VIDEO_QCOM_VENUS)   += qcom/venus/
> +
> +obj-y+= meson/
> diff --git a/drivers/media/platform/meson/Makefile 
> b/drivers/media/platform/meson/Makefile
> new file mode 100644
> index 000..597beb8
> --- /dev/null
> +++ b/drivers/media/platform/meson/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_VIDEO_MESON_AO_CEC) += ao-cec.o
> diff --git a/drivers/media/platform/meson/ao-cec.c 
> b/drivers/media/platform/meson/ao-cec.c
> new file mode 100644
> index 000..5c3607c
> --- /dev/null
> +++ b/drivers/media/platform/meson/ao-cec.c
> @@ -0,0 +1,744 @@
> +/*
> + * Driver for Amlogic Meson AO CEC Controller
> + *
> + * Copyright (C) 2015 Amlogic, Inc. All rights reserved
> + * Copyright (C) 2017 BayLibre, SAS
> + * Author: Neil Armstrong 
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* CEC Registers */
> +
> +/*
> + * [2:1] cntl_clk
> + *  - 0 = Disable clk (Power-off mode)
> + *  - 1 = Enable gated clock (Normal mode)
> + *  - 2 = Enable free-run clk (Debug mode)
> + */
> +#define CEC_GEN_CNTL_REG 0x00
> +
> +#define CEC_GEN_CNTL_RESET   BIT(0)
> +#define CEC_GEN_CNTL_CLK_DISABLE 0
> +#define CEC_GEN_CNTL_CLK_ENABLE  1
> +#define CEC_GEN_CNTL_CLK_ENABLE_DBG  2
> +#define CEC_GEN_CNTL_CLK_CTRL_MASK   GENMASK(2, 1)
> +
> +/*
> + * [7:0] cec_reg_addr
> + * [15:8] cec_reg_wrdata
> + * [16] cec_reg_wr
> + *  - 0 = Read
> + *  - 1 = Write
> + * [23] bus free
> + * [31:24] cec_reg_rddata
> + */
> +#define CEC_RW_REG   0x04
> +
> +#define CEC_RW_ADDR  GENMASK(7, 0)
> +#define CEC_RW_WR_DATA   GENMASK(15, 8)
> +#define CEC_RW_WRITE_EN  BIT(16)
> +#define CEC_RW_BUS_BUSY  BIT(23)
> +#define CEC_RW_RD_DATA   GENMASK(31, 24)
> +
> +/*
> + * [1] tx intr
> + * [2] rx intr
> + */
> +#define CEC_INTR_MASKN_REG   0x08
> +#define CEC_INTR_CLR_REG 0x0c
> +#define CEC_INTR_STAT_REG0x10
> +
> +#define CEC_INTR_TX  BIT(1)
> +#define CEC_INTR_RX  BIT(2)
> +
> +/* CEC Commands */
> +
> +#define CEC_TX_MSG_0_HEADER  0x00
> +#define CEC_TX_MSG_1_OPCODE  0x01
> +#define CEC_TX_MSG_2_OP1 0x02
> +#define CEC_TX_MSG_3_OP2 0x03
> +#define CEC_TX_MSG_4_OP3 0x04
> +#define CEC_TX_MSG_5_OP4 0x05
> +#define CEC_TX_MSG_6_OP5 0x06
> +#define CEC_TX_MSG_7_OP6 0x07
> +#define CEC_TX_MSG_8_OP7 0x08
> +#define CEC_TX_MSG_9_OP8 0x09
> +#define CEC_TX_MSG_A_OP9 0x0A
> +#define 

Re: [PATCH v3 0/2] media: Add Amlogic Meson AO CEC Controller support

[Neil Armstrong]

On 07/28/2017 02:37 PM, Hans Verkuil wrote:
> On 07/28/2017 02:35 PM, Hans Verkuil wrote:
>> Hi Neil,
>>
>> On 07/27/2017 05:20 PM, Neil Armstrong wrote:
>>> The Amlogic SoC embeds a standalone CEC controller, this patch adds a driver
>>> for such controller.
>>> The controller does not need HPD to be active, and could support up to max
>>> 5 logical addresses, but only 1 is handled since the Suspend firmware can
>>> make use of this unique logical address to wake up the device.
>>>
>>> The Suspend firmware configuration will be added in an other patchset.
>>
>> Note that to get the right module dependencies you will also need to add
>> this line:
>>
>>  select CEC_CORE if CEC_NOTIFIER
>>
>> to DRM_MESON_DW_HDMI in drivers/gpu/drm/meson/Kconfig.
>>
>> This ensures that if DRM_MESON_DW_HDMI is 'y' but VIDEO_MESON_AO_CEC is 'm'
>> the CEC_CORE config is set to 'y'.
>>
>> Obviously this is a patch for dri-devel.
> 
> I was too quick sending this: I expect this line to appear in DRM_DW_HDMI,
> not DRM_MESON_DW_HDMI. Sorry about the noise.

Indeed, I will respin russell's notifier patch with this.

Thanks,
Neil

> 
>   Hans
> 
>>
>> Regards,
>>
>>  Hans
>>
>>>
>>> Changes since v2 at [2] :
>>>  - change meson_ao_cec_read/write prototype to simplify error handling
>>>
>>> Changes since v1 at [1] :
>>>  - add timeout to wait busy, with error return
>>>  - handle busy error in all read/write operations
>>>  - add CEC_CAP_PASSTHROUGH
>>>  - add bindings ack
>>>
>>> [1] 
>>> https://lkml.kernel.org/r/1499336870-24118-1-git-send-email-narmstr...@baylibre.com
>>> [2] 
>>> https://lkml.kernel.org/r/1499673696-21372-1-git-send-email-narmstr...@baylibre.com
>>>
>>> Neil Armstrong (2):
>>>   platform: Add Amlogic Meson AO CEC Controller driver
>>>   dt-bindings: media: Add Amlogic Meson AO-CEC bindings
>>>
>>>  .../devicetree/bindings/media/meson-ao-cec.txt |  28 +
>>>  drivers/media/platform/Kconfig |  11 +
>>>  drivers/media/platform/Makefile|   2 +
>>>  drivers/media/platform/meson/Makefile  |   1 +
>>>  drivers/media/platform/meson/ao-cec.c  | 744 
>>> +
>>>  5 files changed, 786 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/media/meson-ao-cec.txt
>>>  create mode 100644 drivers/media/platform/meson/Makefile
>>>  create mode 100644 drivers/media/platform/meson/ao-cec.c
>>>
>>
> 

Re: [PATCH 4/6 v5] uvcvideo: add a metadata device node

[Hans Verkuil]

On 07/28/2017 02:33 PM, Guennadi Liakhovetski wrote:
> Some UVC video cameras contain metadata in their payload headers. This
> patch extracts that data, adding more clock synchronisation information,
> on both bulk and isochronous endpoints and makes it available to the
> user space on a separate video node, using the V4L2_CAP_META_CAPTURE
> capability and the V4L2_BUF_TYPE_META_CAPTURE buffer queue type. Even
> though different cameras will have different metadata formats, we use
> the same V4L2_META_FMT_UVC pixel format for all of them. Users have to
> parse data, based on the specific camera model information. This
> version of the patch only creates such metadata nodes for cameras,
> specifying a UVC_QUIRK_METADATA_NODE quirk flag.
> 
> Signed-off-by: Guennadi Liakhovetski 
> ---
>  drivers/media/usb/uvc/Makefile   |   2 +-
>  drivers/media/usb/uvc/uvc_ctrl.c |   3 +
>  drivers/media/usb/uvc/uvc_driver.c   |  12 +++
>  drivers/media/usb/uvc/uvc_isight.c   |   2 +-
>  drivers/media/usb/uvc/uvc_metadata.c | 139 
> +++
>  drivers/media/usb/uvc/uvc_queue.c|  41 +--
>  drivers/media/usb/uvc/uvc_video.c| 119 +++---
>  drivers/media/usb/uvc/uvcvideo.h |  17 -
>  drivers/media/v4l2-core/v4l2-ioctl.c |   1 +
>  include/uapi/linux/uvcvideo.h|  26 +++
>  10 files changed, 341 insertions(+), 21 deletions(-)
>  create mode 100644 drivers/media/usb/uvc/uvc_metadata.c
> 
> diff --git a/drivers/media/usb/uvc/Makefile b/drivers/media/usb/uvc/Makefile
> index c26d12f..06c7cd3 100644
> --- a/drivers/media/usb/uvc/Makefile
> +++ b/drivers/media/usb/uvc/Makefile
> @@ -1,5 +1,5 @@
>  uvcvideo-objs  := uvc_driver.o uvc_queue.o uvc_v4l2.o uvc_video.o uvc_ctrl.o 
> \
> -   uvc_status.o uvc_isight.o uvc_debugfs.o
> +   uvc_status.o uvc_isight.o uvc_debugfs.o uvc_metadata.o
>  ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
>  uvcvideo-objs  += uvc_entity.o
>  endif
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c 
> b/drivers/media/usb/uvc/uvc_ctrl.c
> index c2ee6e3..91ff2c7 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2179,6 +2179,9 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
>   ctrl->entity = entity;
>   ctrl->index = i;
>  
> + if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT)
> + uvc_trace(UVC_TRACE_CONTROL, "XU %u: ctrl 
> %d\n", entity->id, i);
> +
>   uvc_ctrl_init_ctrl(dev, ctrl);
>   ctrl++;
>   }
> diff --git a/drivers/media/usb/uvc/uvc_driver.c 
> b/drivers/media/usb/uvc/uvc_driver.c
> index cfe33bf..3d61cec 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1884,6 +1884,7 @@ static void uvc_unregister_video(struct uvc_device *dev)
>   continue;
>  
>   video_unregister_device(>vdev);
> + video_unregister_device(>meta.vdev);
>  
>   uvc_debugfs_cleanup_stream(stream);
>   }
> @@ -1944,6 +1945,11 @@ static int uvc_register_video(struct uvc_device *dev,
>   return ret;
>   }
>  
> + /* Register a metadata node, but ignore a possible failure, complete
> +  * registration of video nodes anyway.
> +  */
> + uvc_meta_register(stream);
> +
>   if (stream->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
>   stream->chain->caps |= V4L2_CAP_VIDEO_CAPTURE;
>   else
> @@ -2041,6 +2047,12 @@ static int uvc_probe(struct usb_interface *intf,
>   dev->udev = usb_get_dev(udev);
>   dev->intf = usb_get_intf(intf);
>   dev->intfnum = intf->cur_altsetting->desc.bInterfaceNumber;
> + if (uvc_quirks_param != -1 &&
> + uvc_quirks_param & UVC_DEV_FLAG_METADATA_NODE) {
> + uvc_quirks_param &= ~UVC_DEV_FLAG_METADATA_NODE;
> + if (uvc_quirks_param == 0)
> + uvc_quirks_param = -1;
> + }
>   dev->quirks = (uvc_quirks_param == -1)
>   ? id->driver_info : uvc_quirks_param;
>  
> diff --git a/drivers/media/usb/uvc/uvc_isight.c 
> b/drivers/media/usb/uvc/uvc_isight.c
> index 8510e725..fb940cf 100644
> --- a/drivers/media/usb/uvc/uvc_isight.c
> +++ b/drivers/media/usb/uvc/uvc_isight.c
> @@ -100,7 +100,7 @@ static int isight_decode(struct uvc_video_queue *queue, 
> struct uvc_buffer *buf,
>  }
>  
>  void uvc_video_decode_isight(struct urb *urb, struct uvc_streaming *stream,
> - struct uvc_buffer *buf)
> + struct uvc_buffer *buf, struct uvc_buffer *meta_buf)
>  {
>   int ret, i;
>  
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c 
> b/drivers/media/usb/uvc/uvc_metadata.c
> new file mode 100644
> index 000..062e6ec
> --- /dev/null
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -0,0 +1,139 @@
> +/*
> + *  uvc_metadata.c  --  

[PATCH] v4l: use struct v4l2_buffer explicitly instead of void *

[Guennadi Liakhovetski]

A number of functions use void * for a struct v4l2_buffer parameter.
Avoid that to improve compile-time checking.

Signed-off-by: Guennadi Liakhovetski 
---

This probably was done on purpose, maybe to reuse the video buffers by 
other than V4L2 users, but I haven't found any, and the code doesn't seem 
very new...

 drivers/media/v4l2-core/videobuf2-core.c | 17 +
 drivers/media/v4l2-core/videobuf2-v4l2.c | 15 +++
 include/media/videobuf2-core.h   | 19 ---
 3 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 14f83cec..170a416 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -958,7 +958,7 @@ void vb2_discard_done(struct vb2_queue *q)
 /**
  * __prepare_mmap() - prepare an MMAP buffer
  */
-static int __prepare_mmap(struct vb2_buffer *vb, const void *pb)
+static int __prepare_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *pb)
 {
int ret = 0;
 
@@ -971,7 +971,7 @@ static int __prepare_mmap(struct vb2_buffer *vb, const void 
*pb)
 /**
  * __prepare_userptr() - prepare a USERPTR buffer
  */
-static int __prepare_userptr(struct vb2_buffer *vb, const void *pb)
+static int __prepare_userptr(struct vb2_buffer *vb, const struct v4l2_buffer 
*pb)
 {
struct vb2_plane planes[VB2_MAX_PLANES];
struct vb2_queue *q = vb->vb2_queue;
@@ -1089,7 +1089,7 @@ static int __prepare_userptr(struct vb2_buffer *vb, const 
void *pb)
 /**
  * __prepare_dmabuf() - prepare a DMABUF buffer
  */
-static int __prepare_dmabuf(struct vb2_buffer *vb, const void *pb)
+static int __prepare_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer 
*pb)
 {
struct vb2_plane planes[VB2_MAX_PLANES];
struct vb2_queue *q = vb->vb2_queue;
@@ -1236,7 +1236,7 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
call_void_vb_qop(vb, buf_queue, vb);
 }
 
-static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
+static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *pb)
 {
struct vb2_queue *q = vb->vb2_queue;
unsigned int plane;
@@ -1279,7 +1279,8 @@ static int __buf_prepare(struct vb2_buffer *vb, const 
void *pb)
return 0;
 }
 
-int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
+int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index,
+struct v4l2_buffer *pb)
 {
struct vb2_buffer *vb;
int ret;
@@ -1514,7 +1515,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, 
int nonblocking)
  * Will sleep if required for nonblocking == false.
  */
 static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb,
-void *pb, int nonblocking)
+struct v4l2_buffer *pb, int nonblocking)
 {
unsigned long flags;
int ret = 0;
@@ -1583,8 +1584,8 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)
}
 }
 
-int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
-  bool nonblocking)
+int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex,
+  struct v4l2_buffer *pb, bool nonblocking)
 {
struct vb2_buffer *vb = NULL;
int ret;
diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c 
b/drivers/media/v4l2-core/videobuf2-v4l2.c
index 0c06699..3c425ea 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -53,7 +53,8 @@
  * __verify_planes_array() - verify that the planes array passed in struct
  * v4l2_buffer from userspace can be safely used
  */
-static int __verify_planes_array(struct vb2_buffer *vb, const struct 
v4l2_buffer *b)
+static int __verify_planes_array(struct vb2_buffer *vb,
+const struct v4l2_buffer *b)
 {
if (!V4L2_TYPE_IS_MULTIPLANAR(b->type))
return 0;
@@ -73,7 +74,8 @@ static int __verify_planes_array(struct vb2_buffer *vb, const 
struct v4l2_buffer
return 0;
 }
 
-static int __verify_planes_array_core(struct vb2_buffer *vb, const void *pb)
+static int __verify_planes_array_core(struct vb2_buffer *vb,
+ const struct v4l2_buffer *pb)
 {
return __verify_planes_array(vb, pb);
 }
@@ -118,9 +120,8 @@ static int __verify_length(struct vb2_buffer *vb, const 
struct v4l2_buffer *b)
return 0;
 }
 
-static void __copy_timestamp(struct vb2_buffer *vb, const void *pb)
+static void __copy_timestamp(struct vb2_buffer *vb, const struct v4l2_buffer 
*b)
 {
-   const struct v4l2_buffer *b = pb;
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
struct vb2_queue *q = vb->vb2_queue;
 
@@ -185,9 +186,8 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, 
struct v4l2_buffer *b,
  * __fill_v4l2_buffer() - 

Re: [PATCH 2/6 v5] V4L: Add a UVC Metadata format

[Hans Verkuil]

On 07/28/2017 02:33 PM, Guennadi Liakhovetski wrote:
> Add a pixel format, used by the UVC driver to stream metadata.
> 
> Signed-off-by: Guennadi Liakhovetski 
> ---
>  Documentation/media/uapi/v4l/meta-formats.rst|  1 +
>  Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst | 39 
> 
>  include/uapi/linux/videodev2.h   |  1 +
>  3 files changed, 41 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst
> 
> diff --git a/Documentation/media/uapi/v4l/meta-formats.rst 
> b/Documentation/media/uapi/v4l/meta-formats.rst
> index 01e24e3..1bb45a3f 100644
> --- a/Documentation/media/uapi/v4l/meta-formats.rst
> +++ b/Documentation/media/uapi/v4l/meta-formats.rst
> @@ -14,3 +14,4 @@ These formats are used for the :ref:`metadata` interface 
> only.
>  
>  pixfmt-meta-vsp1-hgo
>  pixfmt-meta-vsp1-hgt
> +pixfmt-meta-uvc
> diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst 
> b/Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst
> new file mode 100644
> index 000..58f78cb
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst
> @@ -0,0 +1,39 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _v4l2-meta-fmt-uvc:
> +
> +***
> +V4L2_META_FMT_UVC ('UVCH')
> +***
> +
> +UVC Payload Header Data
> +
> +
> +Description
> +===
> +
> +This format describes data, supplied by the UVC driver from metadata video
> +nodes. That data includes UVC Payload Header contents and auxiliary timing
> +information, required for precise interpretation of timestamps, contained in
> +those headers. Buffers, streamed via UVC metadata nodes, are composed of 
> blocks
> +of variable length. Those blocks contain are described by struct uvc_meta_buf
> +and contain the following fields:
> +
> +.. flat-table:: UVC Metadata Block
> +:widths: 1 4
> +:header-rows:  1
> +:stub-columns: 0
> +
> +* - Field
> +  - Description
> +* - struct timespec ts;
> +  - system timestamp, measured by the driver upon reception of the 
> payload

Out of date: this is now a __u64 ns field.

> +* - __u16 sof;
> +  - USB Frame Number, also obtained by the driver
> +* - :cspan:`1` *The rest is an exact copy of the payload header:*
> +* - __u8 length;
> +  - length of the rest of the block, including this field
> +* - __u8 flags;
> +  - Flags, indicating presence of other standard UVC fields
> +* - __u8 buf[];
> +  - The rest of the header, possibly including UVC PTS and SCR fields
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 45cf735..0aad91c 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -682,6 +682,7 @@ struct v4l2_pix_format {
>  /* Meta-data formats */
>  #define V4L2_META_FMT_VSP1_HGOv4l2_fourcc('V', 'S', 'P', 'H') /* R-Car 
> VSP1 1-D Histogram */
>  #define V4L2_META_FMT_VSP1_HGTv4l2_fourcc('V', 'S', 'P', 'T') /* R-Car 
> VSP1 2-D Histogram */
> +#define V4L2_META_FMT_UVC v4l2_fourcc('U', 'V', 'C', 'H') /* UVC 
> Payload Header metadata */
>  
>  /* priv field value to indicates that subsequent fields are valid. */
>  #define V4L2_PIX_FMT_PRIV_MAGIC  0xfeedcafe
> 

Re: [RFC 04/19] dt: bindings: Add lens-focus binding for image sensors

[Pavel Machek]

Hi!

> Is the lens-focus phandle specific to voice-coil controllers? What about
> motor-controlled focus systems? What when there are multiple motors? E.g.
> one for the focus, one for the iris control (yes, we have that).

I'd say motor vs. coil is not important. Iris control should go to
lens-iris: or something.

> What if you have two sensors (stereoscopic) controlled by one motor?

Are there such systems? I'd say lets solve it if we see it...

> >  - flash: phandle referring to the flash driver chip. A flash driver may
> >have multiple flashes connected to it.
> >  
> > +- lens-focus: A phandle to the node of the focus lens controller.
> > +
> >  

Re: [PATCH v3 0/2] media: Add Amlogic Meson AO CEC Controller support

[Hans Verkuil]

On 07/28/2017 02:35 PM, Hans Verkuil wrote:
> Hi Neil,
> 
> On 07/27/2017 05:20 PM, Neil Armstrong wrote:
>> The Amlogic SoC embeds a standalone CEC controller, this patch adds a driver
>> for such controller.
>> The controller does not need HPD to be active, and could support up to max
>> 5 logical addresses, but only 1 is handled since the Suspend firmware can
>> make use of this unique logical address to wake up the device.
>>
>> The Suspend firmware configuration will be added in an other patchset.
> 
> Note that to get the right module dependencies you will also need to add
> this line:
> 
>   select CEC_CORE if CEC_NOTIFIER
> 
> to DRM_MESON_DW_HDMI in drivers/gpu/drm/meson/Kconfig.
> 
> This ensures that if DRM_MESON_DW_HDMI is 'y' but VIDEO_MESON_AO_CEC is 'm'
> the CEC_CORE config is set to 'y'.
> 
> Obviously this is a patch for dri-devel.

I was too quick sending this: I expect this line to appear in DRM_DW_HDMI,
not DRM_MESON_DW_HDMI. Sorry about the noise.

Hans

> 
> Regards,
> 
>   Hans
> 
>>
>> Changes since v2 at [2] :
>>  - change meson_ao_cec_read/write prototype to simplify error handling
>>
>> Changes since v1 at [1] :
>>  - add timeout to wait busy, with error return
>>  - handle busy error in all read/write operations
>>  - add CEC_CAP_PASSTHROUGH
>>  - add bindings ack
>>
>> [1] 
>> https://lkml.kernel.org/r/1499336870-24118-1-git-send-email-narmstr...@baylibre.com
>> [2] 
>> https://lkml.kernel.org/r/1499673696-21372-1-git-send-email-narmstr...@baylibre.com
>>
>> Neil Armstrong (2):
>>   platform: Add Amlogic Meson AO CEC Controller driver
>>   dt-bindings: media: Add Amlogic Meson AO-CEC bindings
>>
>>  .../devicetree/bindings/media/meson-ao-cec.txt |  28 +
>>  drivers/media/platform/Kconfig |  11 +
>>  drivers/media/platform/Makefile|   2 +
>>  drivers/media/platform/meson/Makefile  |   1 +
>>  drivers/media/platform/meson/ao-cec.c  | 744 
>> +
>>  5 files changed, 786 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/meson-ao-cec.txt
>>  create mode 100644 drivers/media/platform/meson/Makefile
>>  create mode 100644 drivers/media/platform/meson/ao-cec.c
>>
> 

Re: [PATCH v3 0/2] media: Add Amlogic Meson AO CEC Controller support

[Hans Verkuil]

Hi Neil,

On 07/27/2017 05:20 PM, Neil Armstrong wrote:
> The Amlogic SoC embeds a standalone CEC controller, this patch adds a driver
> for such controller.
> The controller does not need HPD to be active, and could support up to max
> 5 logical addresses, but only 1 is handled since the Suspend firmware can
> make use of this unique logical address to wake up the device.
> 
> The Suspend firmware configuration will be added in an other patchset.

Note that to get the right module dependencies you will also need to add
this line:

select CEC_CORE if CEC_NOTIFIER

to DRM_MESON_DW_HDMI in drivers/gpu/drm/meson/Kconfig.

This ensures that if DRM_MESON_DW_HDMI is 'y' but VIDEO_MESON_AO_CEC is 'm'
the CEC_CORE config is set to 'y'.

Obviously this is a patch for dri-devel.

Regards,

Hans

> 
> Changes since v2 at [2] :
>  - change meson_ao_cec_read/write prototype to simplify error handling
> 
> Changes since v1 at [1] :
>  - add timeout to wait busy, with error return
>  - handle busy error in all read/write operations
>  - add CEC_CAP_PASSTHROUGH
>  - add bindings ack
> 
> [1] 
> https://lkml.kernel.org/r/1499336870-24118-1-git-send-email-narmstr...@baylibre.com
> [2] 
> https://lkml.kernel.org/r/1499673696-21372-1-git-send-email-narmstr...@baylibre.com
> 
> Neil Armstrong (2):
>   platform: Add Amlogic Meson AO CEC Controller driver
>   dt-bindings: media: Add Amlogic Meson AO-CEC bindings
> 
>  .../devicetree/bindings/media/meson-ao-cec.txt |  28 +
>  drivers/media/platform/Kconfig |  11 +
>  drivers/media/platform/Makefile|   2 +
>  drivers/media/platform/meson/Makefile  |   1 +
>  drivers/media/platform/meson/ao-cec.c  | 744 
> +
>  5 files changed, 786 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/meson-ao-cec.txt
>  create mode 100644 drivers/media/platform/meson/Makefile
>  create mode 100644 drivers/media/platform/meson/ao-cec.c
> 

[PATCH 1/6 v5] UVC: fix .queue_setup() to check the number of planes

[Guennadi Liakhovetski]

According to documentation of struct vb2_ops the .queue_setup() callback
should return an error if the number of planes parameter contains an
invalid value on input. Fix this instead of ignoring the value.

Signed-off-by: Guennadi Liakhovetski 
---
 drivers/media/usb/uvc/uvc_queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_queue.c 
b/drivers/media/usb/uvc/uvc_queue.c
index aa21997..371a4ad 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -84,7 +84,7 @@ static int uvc_queue_setup(struct vb2_queue *vq,
 
/* Make sure the image size is large enough. */
if (*nplanes)
-   return sizes[0] < size ? -EINVAL : 0;
+   return sizes[0] < size || *nplanes != 1 ? -EINVAL : 0;
*nplanes = 1;
sizes[0] = size;
return 0;
-- 
1.9.3

[PATCH 4/6 v5] uvcvideo: add a metadata device node

[Guennadi Liakhovetski]

Some UVC video cameras contain metadata in their payload headers. This
patch extracts that data, adding more clock synchronisation information,
on both bulk and isochronous endpoints and makes it available to the
user space on a separate video node, using the V4L2_CAP_META_CAPTURE
capability and the V4L2_BUF_TYPE_META_CAPTURE buffer queue type. Even
though different cameras will have different metadata formats, we use
the same V4L2_META_FMT_UVC pixel format for all of them. Users have to
parse data, based on the specific camera model information. This
version of the patch only creates such metadata nodes for cameras,
specifying a UVC_QUIRK_METADATA_NODE quirk flag.

Signed-off-by: Guennadi Liakhovetski 
---
 drivers/media/usb/uvc/Makefile   |   2 +-
 drivers/media/usb/uvc/uvc_ctrl.c |   3 +
 drivers/media/usb/uvc/uvc_driver.c   |  12 +++
 drivers/media/usb/uvc/uvc_isight.c   |   2 +-
 drivers/media/usb/uvc/uvc_metadata.c | 139 +++
 drivers/media/usb/uvc/uvc_queue.c|  41 +--
 drivers/media/usb/uvc/uvc_video.c| 119 +++---
 drivers/media/usb/uvc/uvcvideo.h |  17 -
 drivers/media/v4l2-core/v4l2-ioctl.c |   1 +
 include/uapi/linux/uvcvideo.h|  26 +++
 10 files changed, 341 insertions(+), 21 deletions(-)
 create mode 100644 drivers/media/usb/uvc/uvc_metadata.c

diff --git a/drivers/media/usb/uvc/Makefile b/drivers/media/usb/uvc/Makefile
index c26d12f..06c7cd3 100644
--- a/drivers/media/usb/uvc/Makefile
+++ b/drivers/media/usb/uvc/Makefile
@@ -1,5 +1,5 @@
 uvcvideo-objs  := uvc_driver.o uvc_queue.o uvc_v4l2.o uvc_video.o uvc_ctrl.o \
- uvc_status.o uvc_isight.o uvc_debugfs.o
+ uvc_status.o uvc_isight.o uvc_debugfs.o uvc_metadata.o
 ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
 uvcvideo-objs  += uvc_entity.o
 endif
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index c2ee6e3..91ff2c7 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2179,6 +2179,9 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
ctrl->entity = entity;
ctrl->index = i;
 
+   if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT)
+   uvc_trace(UVC_TRACE_CONTROL, "XU %u: ctrl 
%d\n", entity->id, i);
+
uvc_ctrl_init_ctrl(dev, ctrl);
ctrl++;
}
diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index cfe33bf..3d61cec 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1884,6 +1884,7 @@ static void uvc_unregister_video(struct uvc_device *dev)
continue;
 
video_unregister_device(>vdev);
+   video_unregister_device(>meta.vdev);
 
uvc_debugfs_cleanup_stream(stream);
}
@@ -1944,6 +1945,11 @@ static int uvc_register_video(struct uvc_device *dev,
return ret;
}
 
+   /* Register a metadata node, but ignore a possible failure, complete
+* registration of video nodes anyway.
+*/
+   uvc_meta_register(stream);
+
if (stream->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
stream->chain->caps |= V4L2_CAP_VIDEO_CAPTURE;
else
@@ -2041,6 +2047,12 @@ static int uvc_probe(struct usb_interface *intf,
dev->udev = usb_get_dev(udev);
dev->intf = usb_get_intf(intf);
dev->intfnum = intf->cur_altsetting->desc.bInterfaceNumber;
+   if (uvc_quirks_param != -1 &&
+   uvc_quirks_param & UVC_DEV_FLAG_METADATA_NODE) {
+   uvc_quirks_param &= ~UVC_DEV_FLAG_METADATA_NODE;
+   if (uvc_quirks_param == 0)
+   uvc_quirks_param = -1;
+   }
dev->quirks = (uvc_quirks_param == -1)
? id->driver_info : uvc_quirks_param;
 
diff --git a/drivers/media/usb/uvc/uvc_isight.c 
b/drivers/media/usb/uvc/uvc_isight.c
index 8510e725..fb940cf 100644
--- a/drivers/media/usb/uvc/uvc_isight.c
+++ b/drivers/media/usb/uvc/uvc_isight.c
@@ -100,7 +100,7 @@ static int isight_decode(struct uvc_video_queue *queue, 
struct uvc_buffer *buf,
 }
 
 void uvc_video_decode_isight(struct urb *urb, struct uvc_streaming *stream,
-   struct uvc_buffer *buf)
+   struct uvc_buffer *buf, struct uvc_buffer *meta_buf)
 {
int ret, i;
 
diff --git a/drivers/media/usb/uvc/uvc_metadata.c 
b/drivers/media/usb/uvc/uvc_metadata.c
new file mode 100644
index 000..062e6ec
--- /dev/null
+++ b/drivers/media/usb/uvc/uvc_metadata.c
@@ -0,0 +1,139 @@
+/*
+ *  uvc_metadata.c  --  USB Video Class driver - Metadata handling
+ *
+ *  Copyright (C) 2016
+ *  Guennadi Liakhovetski (guennadi.liakhovet...@intel.com)
+ *
+ *  This program is free software; you can redistribute it 

[PATCH 0/6 v5] uvcvideo: metadata nodes and controls

[Guennadi Liakhovetski]

The first four patches are for UVC metadata nodes, the last two patches
are for asynchronous controls and control error reporting.

Thanks
Guennadi

Guennadi Liakhovetski (6):
  UVC: fix .queue_setup() to check the number of planes
  V4L: Add a UVC Metadata format
  uvcvideo: convert from using an atomic variable to a reference count
  uvcvideo: add a metadata device node
  uvcvideo: send a control event when a Control Change interrupt arrives
  uvcvideo: handle control pipe protocol STALLs

 Documentation/media/uapi/v4l/meta-formats.rst|   1 +
 Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst |  39 +
 drivers/media/usb/uvc/Makefile   |   2 +-
 drivers/media/usb/uvc/uvc_ctrl.c | 150 +--
 drivers/media/usb/uvc/uvc_driver.c   |  43 --
 drivers/media/usb/uvc/uvc_isight.c   |   2 +-
 drivers/media/usb/uvc/uvc_metadata.c | 139 ++
 drivers/media/usb/uvc/uvc_queue.c|  43 +-
 drivers/media/usb/uvc/uvc_status.c   | 112 --
 drivers/media/usb/uvc/uvc_v4l2.c |   4 +-
 drivers/media/usb/uvc/uvc_video.c| 178 +--
 drivers/media/usb/uvc/uvcvideo.h |  33 -
 drivers/media/v4l2-core/v4l2-ioctl.c |   1 +
 include/uapi/linux/uvcvideo.h|  28 
 include/uapi/linux/videodev2.h   |   1 +
 15 files changed, 705 insertions(+), 71 deletions(-)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst
 create mode 100644 drivers/media/usb/uvc/uvc_metadata.c

-- 
1.9.3

[PATCH 6/6 v5] uvcvideo: handle control pipe protocol STALLs

[Guennadi Liakhovetski]

When a command ends up in a STALL on the control pipe, use the Request
Error Code control to provide a more precise error information to the
user.

Signed-off-by: Guennadi Liakhovetski 
---
 drivers/media/usb/uvc/uvc_video.c | 59 +++
 1 file changed, 53 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c 
b/drivers/media/usb/uvc/uvc_video.c
index 006691e..887561b 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -34,15 +34,59 @@ static int __uvc_query_ctrl(struct uvc_device *dev, __u8 
query, __u8 unit,
__u8 intfnum, __u8 cs, void *data, __u16 size,
int timeout)
 {
-   __u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE;
+   __u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE, tmp, error;
unsigned int pipe;
+   int ret;
 
pipe = (query & 0x80) ? usb_rcvctrlpipe(dev->udev, 0)
  : usb_sndctrlpipe(dev->udev, 0);
type |= (query & 0x80) ? USB_DIR_IN : USB_DIR_OUT;
 
-   return usb_control_msg(dev->udev, pipe, query, type, cs << 8,
+   ret = usb_control_msg(dev->udev, pipe, query, type, cs << 8,
unit << 8 | intfnum, data, size, timeout);
+
+   if (ret != -EPIPE)
+   return ret;
+
+   tmp = *(u8 *)data;
+
+   pipe = usb_rcvctrlpipe(dev->udev, 0);
+   type = USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN;
+   ret = usb_control_msg(dev->udev, pipe, UVC_GET_CUR, type,
+ UVC_VC_REQUEST_ERROR_CODE_CONTROL << 8,
+ unit << 8 | intfnum, data, 1, timeout);
+   error = *(u8 *)data;
+   *(u8 *)data = tmp;
+
+   if (ret < 0)
+   return ret;
+
+   if (!ret)
+   return -EINVAL;
+
+   uvc_trace(UVC_TRACE_CONTROL, "Control error %u\n", error);
+
+   switch (error) {
+   case 0:
+   /* Cannot happen - we received a STALL */
+   return -EPIPE;
+   case 1: /* Not ready */
+   return -EAGAIN;
+   case 2: /* Wrong state */
+   return -EILSEQ;
+   case 3: /* Power */
+   return -EREMOTE;
+   case 4: /* Out of range */
+   return -ERANGE;
+   case 5: /* Invalid unit */
+   case 6: /* Invalid control */
+   case 7: /* Invalid Request */
+   case 8: /* Invalid value within range */
+   default: /* reserved or unknown */
+   break;
+   }
+
+   return -EINVAL;
 }
 
 static const char *uvc_query_name(__u8 query)
@@ -80,7 +124,7 @@ int uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 
unit,
uvc_printk(KERN_ERR, "Failed to query (%s) UVC control %u on "
"unit %u: %d (exp. %u).\n", uvc_query_name(query), cs,
unit, ret, size);
-   return -EIO;
+   return ret < 0 ? ret : -EIO;
}
 
return 0;
@@ -203,13 +247,15 @@ static int uvc_get_video_ctrl(struct uvc_streaming 
*stream,
uvc_warn_once(stream->dev, UVC_WARN_PROBE_DEF, "UVC non "
"compliance - GET_DEF(PROBE) not supported. "
"Enabling workaround.\n");
-   ret = -EIO;
+   if (ret >= 0)
+   ret = -EIO;
goto out;
} else if (ret != size) {
uvc_printk(KERN_ERR, "Failed to query (%u) UVC %s control : "
"%d (exp. %u).\n", query, probe ? "probe" : "commit",
ret, size);
-   ret = -EIO;
+   if (ret >= 0)
+   ret = -EIO;
goto out;
}
 
@@ -290,7 +336,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
uvc_printk(KERN_ERR, "Failed to set UVC %s control : "
"%d (exp. %u).\n", probe ? "probe" : "commit",
ret, size);
-   ret = -EIO;
+   if (ret >= 0)
+   ret = -EIO;
}
 
kfree(data);
-- 
1.9.3

[PATCH 2/6 v5] V4L: Add a UVC Metadata format

[Guennadi Liakhovetski]

Add a pixel format, used by the UVC driver to stream metadata.

Signed-off-by: Guennadi Liakhovetski 
---
 Documentation/media/uapi/v4l/meta-formats.rst|  1 +
 Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst | 39 
 include/uapi/linux/videodev2.h   |  1 +
 3 files changed, 41 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst

diff --git a/Documentation/media/uapi/v4l/meta-formats.rst 
b/Documentation/media/uapi/v4l/meta-formats.rst
index 01e24e3..1bb45a3f 100644
--- a/Documentation/media/uapi/v4l/meta-formats.rst
+++ b/Documentation/media/uapi/v4l/meta-formats.rst
@@ -14,3 +14,4 @@ These formats are used for the :ref:`metadata` interface only.
 
 pixfmt-meta-vsp1-hgo
 pixfmt-meta-vsp1-hgt
+pixfmt-meta-uvc
diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst 
b/Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst
new file mode 100644
index 000..58f78cb
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst
@@ -0,0 +1,39 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _v4l2-meta-fmt-uvc:
+
+***
+V4L2_META_FMT_UVC ('UVCH')
+***
+
+UVC Payload Header Data
+
+
+Description
+===
+
+This format describes data, supplied by the UVC driver from metadata video
+nodes. That data includes UVC Payload Header contents and auxiliary timing
+information, required for precise interpretation of timestamps, contained in
+those headers. Buffers, streamed via UVC metadata nodes, are composed of blocks
+of variable length. Those blocks contain are described by struct uvc_meta_buf
+and contain the following fields:
+
+.. flat-table:: UVC Metadata Block
+:widths: 1 4
+:header-rows:  1
+:stub-columns: 0
+
+* - Field
+  - Description
+* - struct timespec ts;
+  - system timestamp, measured by the driver upon reception of the payload
+* - __u16 sof;
+  - USB Frame Number, also obtained by the driver
+* - :cspan:`1` *The rest is an exact copy of the payload header:*
+* - __u8 length;
+  - length of the rest of the block, including this field
+* - __u8 flags;
+  - Flags, indicating presence of other standard UVC fields
+* - __u8 buf[];
+  - The rest of the header, possibly including UVC PTS and SCR fields
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 45cf735..0aad91c 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -682,6 +682,7 @@ struct v4l2_pix_format {
 /* Meta-data formats */
 #define V4L2_META_FMT_VSP1_HGOv4l2_fourcc('V', 'S', 'P', 'H') /* R-Car 
VSP1 1-D Histogram */
 #define V4L2_META_FMT_VSP1_HGTv4l2_fourcc('V', 'S', 'P', 'T') /* R-Car 
VSP1 2-D Histogram */
+#define V4L2_META_FMT_UVC v4l2_fourcc('U', 'V', 'C', 'H') /* UVC 
Payload Header metadata */
 
 /* priv field value to indicates that subsequent fields are valid. */
 #define V4L2_PIX_FMT_PRIV_MAGIC0xfeedcafe
-- 
1.9.3

[PATCH 3/6 v5] uvcvideo: convert from using an atomic variable to a reference count

[Guennadi Liakhovetski]

When adding support for metadata nodes, we'll have to keep video
devices registered until all metadata nodes are closed too. Since
this has nothing to do with stream counting, replace the nstreams
atomic variable with a reference counter.

Signed-off-by: Guennadi Liakhovetski 
---
 drivers/media/usb/uvc/uvc_driver.c | 31 +--
 drivers/media/usb/uvc/uvcvideo.h   |  2 +-
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index 70842c5..cfe33bf 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1849,16 +1849,20 @@ static void uvc_delete(struct uvc_device *dev)
kfree(dev);
 }
 
+static void uvc_kref_release(struct kref *kref)
+{
+   struct uvc_device *dev = container_of(kref, struct uvc_device, ref);
+
+   uvc_delete(dev);
+}
+
 static void uvc_release(struct video_device *vdev)
 {
struct uvc_streaming *stream = video_get_drvdata(vdev);
struct uvc_device *dev = stream->dev;
 
-   /* Decrement the registered streams count and delete the device when it
-* reaches zero.
-*/
-   if (atomic_dec_and_test(>nstreams))
-   uvc_delete(dev);
+   /* Decrement the refcount and delete the device when it reaches zero */
+   kref_put(>ref, uvc_kref_release);
 }
 
 /*
@@ -1870,10 +1874,10 @@ static void uvc_unregister_video(struct uvc_device *dev)
 
/* Unregistering all video devices might result in uvc_delete() being
 * called from inside the loop if there's no open file handle. To avoid
-* that, increment the stream count before iterating over the streams
-* and decrement it when done.
+* that, increment the refcount before iterating over the streams and
+* decrement it when done.
 */
-   atomic_inc(>nstreams);
+   kref_get(>ref);
 
list_for_each_entry(stream, >streams, list) {
if (!video_is_registered(>vdev))
@@ -1884,11 +1888,10 @@ static void uvc_unregister_video(struct uvc_device *dev)
uvc_debugfs_cleanup_stream(stream);
}
 
-   /* Decrement the stream count and call uvc_delete explicitly if there
-* are no stream left.
+   /* Decrement the refcount and call uvc_delete explicitly if there are no
+* stream left.
 */
-   if (atomic_dec_and_test(>nstreams))
-   uvc_delete(dev);
+   kref_put(>ref, uvc_kref_release);
 }
 
 static int uvc_register_video(struct uvc_device *dev,
@@ -1946,7 +1949,7 @@ static int uvc_register_video(struct uvc_device *dev,
else
stream->chain->caps |= V4L2_CAP_VIDEO_OUTPUT;
 
-   atomic_inc(>nstreams);
+   kref_get(>ref);
return 0;
 }
 
@@ -2031,7 +2034,7 @@ static int uvc_probe(struct usb_interface *intf,
INIT_LIST_HEAD(>entities);
INIT_LIST_HEAD(>chains);
INIT_LIST_HEAD(>streams);
-   atomic_set(>nstreams, 0);
+   kref_init(>ref);
atomic_set(>nmappings, 0);
mutex_init(>lock);
 
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 15e415e..f157cf7 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -575,7 +575,7 @@ struct uvc_device {
 
/* Video Streaming interfaces */
struct list_head streams;
-   atomic_t nstreams;
+   struct kref ref;
 
/* Status Interrupt Endpoint */
struct usb_host_endpoint *int_ep;
-- 
1.9.3

[PATCH 5/6 v5] uvcvideo: send a control event when a Control Change interrupt arrives

[Guennadi Liakhovetski]

UVC defines a method of handling asynchronous controls, which sends a
USB packet over the interrupt pipe. This patch implements support for
such packets by sending a control event to the user. Since this can
involve USB traffic and, therefore, scheduling, this has to be done
in a work queue.

Signed-off-by: Guennadi Liakhovetski 
---
 drivers/media/usb/uvc/uvc_ctrl.c   | 147 +
 drivers/media/usb/uvc/uvc_status.c | 112 +---
 drivers/media/usb/uvc/uvc_v4l2.c   |   4 +-
 drivers/media/usb/uvc/uvcvideo.h   |  14 +++-
 include/uapi/linux/uvcvideo.h  |   2 +
 5 files changed, 251 insertions(+), 28 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 91ff2c7..be18707 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -1236,16 +1237,110 @@ static void uvc_ctrl_send_event(struct uvc_fh *handle,
}
 }
 
-static void uvc_ctrl_send_slave_event(struct uvc_fh *handle,
-   struct uvc_control *master, u32 slave_id,
-   const struct v4l2_ext_control *xctrls, unsigned int xctrls_count)
+static void __uvc_ctrl_send_slave_event(struct uvc_fh *handle,
+   struct uvc_control *master, u32 slave_id)
 {
struct uvc_control_mapping *mapping = NULL;
struct uvc_control *ctrl = NULL;
u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
-   unsigned int i;
s32 val = 0;
 
+   __uvc_find_control(master->entity, slave_id, , , 0);
+   if (ctrl == NULL)
+   return;
+
+   if (__uvc_ctrl_get(handle->chain, ctrl, mapping, ) == 0)
+   changes |= V4L2_EVENT_CTRL_CH_VALUE;
+
+   uvc_ctrl_send_event(handle, ctrl, mapping, val, changes);
+}
+
+static void uvc_ctrl_status_event_work(struct work_struct *work)
+{
+   struct uvc_device *dev = container_of(work, struct uvc_device,
+ async_ctrl.work);
+   struct uvc_ctrl_work *w = >async_ctrl;
+   struct uvc_control_mapping *mapping;
+   struct uvc_control *ctrl;
+   struct uvc_fh *handle;
+   __u8 *data;
+   unsigned int i;
+
+   spin_lock_irq(>lock);
+   data = w->data;
+   w->data = NULL;
+   ctrl = w->ctrl;
+   handle = ctrl->handle;
+   ctrl->handle = NULL;
+   spin_unlock_irq(>lock);
+
+   if (mutex_lock_interruptible(>chain->ctrl_mutex))
+   goto free;
+
+   list_for_each_entry(mapping, >info.mappings, list) {
+   s32 value = mapping->get(mapping, UVC_GET_CUR, data);
+
+   for (i = 0; i < ARRAY_SIZE(mapping->slave_ids); ++i) {
+   if (!mapping->slave_ids[i])
+   break;
+
+   __uvc_ctrl_send_slave_event(handle, ctrl,
+   mapping->slave_ids[i]);
+   }
+
+   if (mapping->v4l2_type == V4L2_CTRL_TYPE_MENU) {
+   struct uvc_menu_info *menu = mapping->menu_info;
+   unsigned int i;
+
+   for (i = 0; i < mapping->menu_count; ++i, ++menu)
+   if (menu->value == value) {
+   value = i;
+   break;
+   }
+   }
+
+   uvc_ctrl_send_event(handle, ctrl, mapping, value,
+   V4L2_EVENT_CTRL_CH_VALUE);
+   }
+
+   mutex_unlock(>chain->ctrl_mutex);
+
+free:
+   kfree(data);
+}
+
+void uvc_ctrl_status_event(struct uvc_device *dev, struct uvc_control *ctrl,
+  __u8 *data, size_t len)
+{
+   struct uvc_ctrl_work *w = >async_ctrl;
+
+   if (list_empty(>info.mappings))
+   return;
+
+   if (!ctrl->handle)
+   /* This is an auto-update, they are unsupported */
+   return;
+
+   spin_lock(>lock);
+   if (w->data)
+   /* A previous event work hasn't run yet, we lose 1 event */
+   kfree(w->data);
+
+   w->data = kmalloc(len, GFP_ATOMIC);
+   if (w->data) {
+   memcpy(w->data, data, len);
+   w->ctrl = ctrl;
+   schedule_work(>work);
+   }
+   spin_unlock(>lock);
+}
+
+static void uvc_ctrl_send_slave_event(struct uvc_fh *handle,
+   struct uvc_control *master, u32 slave_id,
+   const struct v4l2_ext_control *xctrls, unsigned int xctrls_count)
+{
+   unsigned int i;
+
/*
 * We can skip sending an event for the slave if the slave
 * is being modified in the same transaction.
@@ -1255,14 +1350,7 @@ static void uvc_ctrl_send_slave_event(struct uvc_fh 
*handle,
return;
}
 
-   __uvc_find_control(master->entity, 

[GIT PULL FOR v4.14] Various fixes

[Hans Verkuil]

The following changes since commit da48c948c263c9d87dfc64566b3373a858cc8aa2:

  media: fix warning on v4l2_subdev_call() result interpreted as bool 
(2017-07-26 13:43:17 -0400)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git for-v4.14c

for you to fetch changes up to 82fb31aa8e0dc11da47bc774dddafc14ac801e2c:

  cec: documentation fixes (2017-07-28 13:25:27 +0200)


Arnd Bergmann (5):
  v4l: omap_vout: vrfb: include linux/slab.h
  imx: add VIDEO_V4L2_SUBDEV_API dependency
  media: i2c: add KConfig dependencies
  media: v4l: use WARN_ON(1) instead of __WARN()
  media: v4l: omap_vout: vrfb: initialize DMA flags

Fabio Estevam (2):
  ov7670: Return the real error code
  ov7670: Check the return value from clk_prepare_enable()

Hans Verkuil (1):
  cec: documentation fixes

Hugues Fruchet (2):
  ov9650: fix coding style
  ov9655: fix missing mutex_destroy()

JB Van Puyvelde (1):
  staging: imx: fix non-static declarations

Kuninori Morimoto (1):
  media: ti-vpe: cal: use of_graph_get_remote_endpoint()

Philipp Zabel (1):
  stm32-dcmi: explicitly request exclusive reset control

Steve Longerbeam (1):
  media: imx: prpencvf: enable double write reduction

Tiffany Lin (1):
  mtk-vcodec: fix vp9 decode error

 Documentation/media/uapi/cec/cec-func-close.rst  |  2 +-
 Documentation/media/uapi/cec/cec-func-ioctl.rst  |  2 +-
 Documentation/media/uapi/cec/cec-func-open.rst   |  4 +--
 Documentation/media/uapi/cec/cec-func-poll.rst   |  8 +++---
 Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst |  2 +-
 Documentation/media/uapi/cec/cec-ioc-dqevent.rst |  2 +-
 drivers/media/i2c/Kconfig|  3 ++-
 drivers/media/i2c/ov7670.c   |  6 +++--
 drivers/media/i2c/ov9650.c   | 67 

 drivers/media/platform/mtk-vcodec/vdec/vdec_vp9_if.c | 37 
--
 drivers/media/platform/omap/omap_vout_vrfb.c |  3 ++-
 drivers/media/platform/pxa_camera.c  |  2 +-
 drivers/media/platform/soc_camera/soc_mediabus.c |  2 +-
 drivers/media/platform/stm32/stm32-dcmi.c|  2 +-
 drivers/media/platform/ti-vpe/cal.c  |  2 +-
 drivers/staging/media/atomisp/i2c/imx/imx.c  | 24 -
 drivers/staging/media/imx/Kconfig|  1 +
 drivers/staging/media/imx/imx-ic-prpencvf.c  | 11 
 18 files changed, 120 insertions(+), 60 deletions(-)

Re: [PATCH] v4l: rcar-vin: Simplify rvin_group_notify_{bound,unbind}

[Hans Verkuil]

On 07/21/2017 11:55 AM, Kieran Bingham wrote:
> Use a container_of macro to obtain the graph entity object from the ASD
> This removes the error conditions, and reduces the lock contention.
> 
> (The locking may even be potentially removed)

I've set the status in patchwork to 'Not Applicable' since the patch isn't
against the mainline code. I leave it to Niklas to decide what to do with this.

Regards,

Hans

> 
> Signed-off-by: Kieran Bingham 
> ---
> 
> Hi Niklas,
> 
> While working through the Multi camera setup, we came across this improvement.
> 
> If this code isn't yet upstream, feel free to squash this change into your
> existing branch if you wish.
> 
> Regards
> 
> Kieran
> 
>  drivers/media/platform/rcar-vin/rcar-core.c | 28 ++--
>  drivers/media/platform/rcar-vin/rcar-vin.h  |  3 +++
>  2 files changed, 9 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index 8393a1598660..b4fc7b56c8a1 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -688,20 +688,11 @@ static void rvin_group_notify_unbind(struct 
> v4l2_async_notifier *notifier,
>struct v4l2_async_subdev *asd)
>  {
>   struct rvin_dev *vin = notifier_to_vin(notifier);
> - unsigned int i;
> + struct rvin_graph_entity *csi = to_rvin_graph_entity(asd);
>  
>   mutex_lock(>group->lock);
> - for (i = 0; i < RVIN_CSI_MAX; i++) {
> - if (>group->csi[i].asd == asd) {
> - vin_dbg(vin, "Unbind CSI-2 %s\n", subdev->name);
> - vin->group->csi[i].subdev = NULL;
> - mutex_unlock(>group->lock);
> - return;
> - }
> - }
> + csi->subdev = NULL;
>   mutex_unlock(>group->lock);
> -
> - vin_err(vin, "No entity for subdev %s to unbind\n", subdev->name);
>  }
>  
>  static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
> @@ -709,23 +700,16 @@ static int rvin_group_notify_bound(struct 
> v4l2_async_notifier *notifier,
>  struct v4l2_async_subdev *asd)
>  {
>   struct rvin_dev *vin = notifier_to_vin(notifier);
> - unsigned int i;
> + struct rvin_graph_entity *csi = to_rvin_graph_entity(asd);
>  
>   v4l2_set_subdev_hostdata(subdev, vin);
>  
>   mutex_lock(>group->lock);
> - for (i = 0; i < RVIN_CSI_MAX; i++) {
> - if (>group->csi[i].asd == asd) {
> - vin_dbg(vin, "Bound CSI-2 %s\n", subdev->name);
> - vin->group->csi[i].subdev = subdev;
> - mutex_unlock(>group->lock);
> - return 0;
> - }
> - }
> + vin_dbg(vin, "Bound CSI-2 %s\n", subdev->name);
> + csi->subdev = subdev;
>   mutex_unlock(>group->lock);
>  
> - vin_err(vin, "No entity for subdev %s to bind\n", subdev->name);
> - return -EINVAL;
> + return 0;
>  }
>  
>  static struct device_node *rvin_group_get_csi(struct rvin_dev *vin,
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h 
> b/drivers/media/platform/rcar-vin/rcar-vin.h
> index e7e600fdf566..900c473c3d15 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -92,6 +92,9 @@ struct rvin_graph_entity {
>   unsigned int sink_pad;
>  };
>  
> +#define to_rvin_graph_entity(asd) \
> + container_of(asd, struct rvin_graph_entity, asd)
> +
>  struct rvin_group;
>  
>  
> 

[RFCv2 PATCH 0/2] add VIDIOC_SUBDEV_QUERYCAP ioctl

[Hans Verkuil]

From: Hans Verkuil 

I tried to get this in back in 2015, but that effort stalled.

Trying again, since I really need this in order to add proper v4l-subdev
support to v4l2-ctl and v4l2-compliance. There currently is no way of
unique identifying that the device really is a v4l-subdev device other
than the device name (which can be changed by udev).

So this patch series adds a VIDIOC_SUBDEV_QUERYCAP ioctl that is in
the core so it's guaranteed to be there.

If the subdev is part of an MC then it also gives the corresponding
entity ID of the subdev and the major/minor numbers of the MC device
so v4l2-compliance can relate the subdev device directly to the right
MC device. The reserved array has room enough for more strings should
we need them later, although I think what we have here is sufficient.

Regards,

Hans

Changes since v1:
- Add name field. Without that it is hard to figure out which subdev
  it is since the entity ID is not very human readable.

Hans Verkuil (2):
  v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl
  v4l: document VIDIOC_SUBDEV_QUERYCAP

 Documentation/media/uapi/v4l/user-func.rst |   1 +
 .../media/uapi/v4l/vidioc-subdev-querycap.rst  | 121 +
 drivers/media/v4l2-core/v4l2-subdev.c  |  27 +
 include/uapi/linux/v4l2-subdev.h   |  31 ++
 4 files changed, 180 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/vidioc-subdev-querycap.rst

-- 
2.13.1

[RFCv2 PATCH 2/2] v4l: document VIDIOC_SUBDEV_QUERYCAP

[Hans Verkuil]

From: Hans Verkuil 

Add documentation for the new VIDIOC_SUBDEV_QUERYCAP ioctl.

Signed-off-by: Hans Verkuil 
---
 Documentation/media/uapi/v4l/user-func.rst |   1 +
 .../media/uapi/v4l/vidioc-subdev-querycap.rst  | 121 +
 2 files changed, 122 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/vidioc-subdev-querycap.rst

diff --git a/Documentation/media/uapi/v4l/user-func.rst 
b/Documentation/media/uapi/v4l/user-func.rst
index 3e0413b83a33..eda5a01b5228 100644
--- a/Documentation/media/uapi/v4l/user-func.rst
+++ b/Documentation/media/uapi/v4l/user-func.rst
@@ -71,6 +71,7 @@ Function Reference
 vidioc-subdev-g-fmt
 vidioc-subdev-g-frame-interval
 vidioc-subdev-g-selection
+vidioc-subdev-querycap
 vidioc-subscribe-event
 func-mmap
 func-munmap
diff --git a/Documentation/media/uapi/v4l/vidioc-subdev-querycap.rst 
b/Documentation/media/uapi/v4l/vidioc-subdev-querycap.rst
new file mode 100644
index ..6143d201b11e
--- /dev/null
+++ b/Documentation/media/uapi/v4l/vidioc-subdev-querycap.rst
@@ -0,0 +1,121 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _VIDIOC_SUBDEV_QUERYCAP:
+
+
+ioctl VIDIOC_SUBDEV_QUERYCAP
+
+
+Name
+
+
+VIDIOC_SUBDEV_QUERYCAP - Query sub-device capabilities
+
+
+Synopsis
+
+
+.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_QUERYCAP, struct 
v4l2_subdev_capability *argp )
+:name: VIDIOC_SUBDEV_QUERYCAP
+
+
+Arguments
+=
+
+``fd``
+File descriptor returned by :ref:`open() `.
+
+``argp``
+
+
+Description
+===
+
+All V4L2 sub-devices support the
+``VIDIOC_SUBDEV_QUERYCAP`` ioctl. It is used to identify
+kernel devices compatible with this specification and to obtain
+information about driver and hardware capabilities. The ioctl takes a
+pointer to a struct :c:type:`v4l2_subdev_capability` which is filled by the
+driver. When the driver is not compatible with this specification the ioctl
+returns ``ENOTTY`` error code.
+
+.. tabularcolumns:: |p{1.5cm}|p{2.5cm}|p{13cm}|
+
+.. c:type:: v4l2_subdev_capability
+
+.. flat-table:: struct v4l2_subdev_capability
+:header-rows:  0
+:stub-columns: 0
+:widths:   3 4 20
+
+* - __u32
+  - ``version``
+  - Version number of the driver.
+
+   The version reported is provided by the
+   V4L2 subsystem following the kernel numbering scheme. However, it
+   may not always return the same version as the kernel if, for
+   example, a stable or distribution-modified kernel uses the V4L2
+   stack from a newer kernel.
+
+   The version number is formatted using the ``KERNEL_VERSION()``
+   macro:
+* - :cspan:`2`
+
+   ``#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))``
+
+   ``__u32 version = KERNEL_VERSION(0, 8, 1);``
+
+   ``printf ("Version: %u.%u.%u\\n",``
+
+   ``(version >> 16) & 0xFF, (version >> 8) & 0xFF, version & 0xFF);``
+* - __u32
+  - ``device_caps``
+  - Sub-device capabilities of the opened device, see
+   :ref:`subdevice-capabilities`.
+* - char
+  - ``name``\ [32]
+  - NUL-terminated name of the sub-device.
+* - __u32
+  - ``entity_id``
+  - The media controller entity ID of the sub-device. This is only valid if
+the ``V4L2_SUBDEV_CAP_ENTITY`` capability is set, it is 0 otherwise.
+* - __u32
+  - ``media_node_major``
+  - The major number of the media controller device node corresponding 
sub-device.
+This is only valid if the ``V4L2_SUBDEV_CAP_ENTITY`` capability is 
set, it is
+   0 otherwise.
+* - __u32
+  - ``media_node_minor``
+  - The minor number of the media controller device node corresponding 
sub-device.
+This is only valid if the ``V4L2_SUBDEV_CAP_ENTITY`` capability is 
set, it is
+   0 otherwise.
+* - __u32
+  - ``reserved``\ [19]
+  - Reserved for future extensions. Applications and drivers must set
+   the array to zero.
+
+.. tabularcolumns:: |p{6cm}|p{2.2cm}|p{8.8cm}|
+
+.. _subdevice-capabilities:
+
+.. cssclass:: longtable
+
+.. flat-table:: Sub-Device Capabilities Flags
+:header-rows:  0
+:stub-columns: 0
+:widths:   3 1 4
+
+* - ``V4L2_SUBDEV_CAP_ENTITY``
+  - 0x0001
+  - The sub-device is a media controller entity and the ``entity_id``,
+``media_node_major`` and ``media_node_minor`` fields of
+struct :c:type:`v4l2_subdev_capability` are valid. These fields
+   are 0 if this capability is not set.
+
+Return Value
+
+
+On success 0 is returned, on error -1 and the ``errno`` variable is set
+appropriately. The generic error codes are described at the
+:ref:`Generic Error Codes ` chapter.
-- 
2.13.1

[RFCv2 PATCH 1/2] v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl

[Hans Verkuil]

From: Hans Verkuil 

While normal video/radio/vbi/swradio nodes have a proper QUERYCAP ioctl
that apps can call to determine that it is indeed a V4L2 device, there
is currently no equivalent for v4l-subdev nodes. Adding this ioctl will
solve that, and it will allow utilities like v4l2-compliance to be used
with these devices as well.

SUBDEV_QUERYCAP currently returns the version and device_caps of the
subdevice. If the subdev is used as part of a media controller, then
it also returns the entity ID and the major and minor numbers of the
media controller device node.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-subdev.c | 27 +++
 include/uapi/linux/v4l2-subdev.h  | 31 +++
 2 files changed, 58 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
b/drivers/media/v4l2-core/v4l2-subdev.c
index 43fefa73e0a3..56cc255171d7 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -20,8 +20,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -186,6 +188,31 @@ static long subdev_do_ioctl(struct file *file, unsigned 
int cmd, void *arg)
 #endif
 
switch (cmd) {
+   case VIDIOC_SUBDEV_QUERYCAP: {
+   struct v4l2_subdev_capability *cap = arg;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   struct media_device *mdev = sd->entity.graph_obj.mdev;
+   struct media_devnode *devnode = mdev ? mdev->devnode : NULL;
+#endif
+
+   cap->version = LINUX_VERSION_CODE;
+   cap->device_caps = 0;
+   strlcpy(cap->name, sd->name, sizeof(cap->name));
+   cap->entity_id = 0;
+   cap->media_node_major = 0;
+   cap->media_node_minor = 0;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   if (devnode) {
+   cap->device_caps = V4L2_SUBDEV_CAP_ENTITY;
+   cap->entity_id = sd->entity.graph_obj.id;
+   cap->media_node_major = MAJOR(devnode->cdev.dev);
+   cap->media_node_minor = MINOR(devnode->cdev.dev);
+   }
+#endif
+   memset(cap->reserved, 0, sizeof(cap->reserved));
+   break;
+   }
+
case VIDIOC_QUERYCTRL:
return v4l2_queryctrl(vfh->ctrl_handler, arg);
 
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index dbce2b554e02..3dd1c412bf0d 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -154,9 +154,40 @@ struct v4l2_subdev_selection {
__u32 reserved[8];
 };
 
+/**
+ * struct v4l2_subdev_capability - subdev capabilities
+ * @version: the kernel version
+ * @device_caps: the subdev capabilities
+ * @name: the subdev name
+ * @entity_id: the entity ID as assigned by the media controller. Only
+ * valid if V4L2_SUBDEV_CAP_ENTITY is set
+ * @media_node_major: the major number of the media controller device node.
+ * Only valid if V4L2_SUBDEV_CAP_ENTITY is set
+ * @media_node_minor: the minor number of the media controller device node.
+ * Only valid if V4L2_SUBDEV_CAP_ENTITY is set
+ * @reserved: for future use, set to zero for now
+ */
+struct v4l2_subdev_capability {
+   __u32 version;
+   __u32 device_caps;
+   char  name[32];
+   __u32 entity_id;
+   /* Corresponding media controller device node specifications */
+   __u32 media_node_major;
+   __u32 media_node_minor;
+   __u32 reserved[19];
+};
+
+/*
+ * This v4l2_subdev is also a media entity and the entity_id, media_node_major
+ * and media_node_minor fields are valid
+ */
+#define V4L2_SUBDEV_CAP_ENTITY (1 << 0)
+
 /* Backwards compatibility define --- to be removed */
 #define v4l2_subdev_edid v4l2_edid
 
+#define VIDIOC_SUBDEV_QUERYCAP  _IOR('V',  0, struct 
v4l2_subdev_capability)
 #define VIDIOC_SUBDEV_G_FMT_IOWR('V',  4, struct 
v4l2_subdev_format)
 #define VIDIOC_SUBDEV_S_FMT_IOWR('V',  5, struct 
v4l2_subdev_format)
 #define VIDIOC_SUBDEV_G_FRAME_INTERVAL _IOWR('V', 21, struct 
v4l2_subdev_frame_interval)
-- 
2.13.1

Re: [RFC PATCH 0/2] add VIDIOC_SUBDEV_QUERYCAP ioctl

[Hans Verkuil]

Grrr. Please ignore, I pressed send even though I wanted to make
some more changes.

I'll post a v2 soon.

Regards,

Hans


On 07/28/2017 12:52 PM, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> I tried to get this in back in 2015, but that effort stalled.
> 
> Trying again, since I really need this in order to add proper v4l-subdev
> support to v4l2-ctl and v4l2-compliance. There currently is no way of
> unique identifying that the device really is a v4l-subdev device other
> than the device name (which can be changed by udev).
> 
> So this patch series adds a VIDIOC_SUBDEV_QUERYCAP ioctl that is in
> the core so it's guaranteed to be there.
> 
> If the subdev is part of an MC then it also gives the corresponding
> entity ID of the subdev and the major/minor numbers of the MC device
> so v4l2-compliance can relate the subdev device directly to the right
> MC device. The reserved array has room enough for strings should we
> need them later. 
> 
> Hans Verkuil (2):
>   v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl
>   v4l: document VIDIOC_SUBDEV_QUERYCAP
> 
>  Documentation/media/uapi/v4l/user-func.rst |   1 +
>  .../media/uapi/v4l/vidioc-subdev-querycap.rst  | 118 
> +
>  drivers/media/v4l2-core/v4l2-subdev.c  |  26 +
>  include/uapi/linux/v4l2-subdev.h   |  29 +
>  4 files changed, 174 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/vidioc-subdev-querycap.rst
> 

[RFC PATCH 2/2] v4l: document VIDIOC_SUBDEV_QUERYCAP

[Hans Verkuil]

From: Hans Verkuil 

Add documentation for the new VIDIOC_SUBDEV_QUERYCAP ioctl.

Signed-off-by: Hans Verkuil 
---
 Documentation/media/uapi/v4l/user-func.rst |   1 +
 .../media/uapi/v4l/vidioc-subdev-querycap.rst  | 118 +
 2 files changed, 119 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/vidioc-subdev-querycap.rst

diff --git a/Documentation/media/uapi/v4l/user-func.rst 
b/Documentation/media/uapi/v4l/user-func.rst
index 3e0413b83a33..eda5a01b5228 100644
--- a/Documentation/media/uapi/v4l/user-func.rst
+++ b/Documentation/media/uapi/v4l/user-func.rst
@@ -71,6 +71,7 @@ Function Reference
 vidioc-subdev-g-fmt
 vidioc-subdev-g-frame-interval
 vidioc-subdev-g-selection
+vidioc-subdev-querycap
 vidioc-subscribe-event
 func-mmap
 func-munmap
diff --git a/Documentation/media/uapi/v4l/vidioc-subdev-querycap.rst 
b/Documentation/media/uapi/v4l/vidioc-subdev-querycap.rst
new file mode 100644
index ..e43cdf34bc55
--- /dev/null
+++ b/Documentation/media/uapi/v4l/vidioc-subdev-querycap.rst
@@ -0,0 +1,118 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _VIDIOC_SUBDEV_QUERYCAP:
+
+
+ioctl VIDIOC_SUBDEV_QUERYCAP
+
+
+Name
+
+
+VIDIOC_SUBDEV_QUERYCAP - Query sub-device capabilities
+
+
+Synopsis
+
+
+.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_QUERYCAP, struct 
v4l2_subdev_capability *argp )
+:name: VIDIOC_SUBDEV_QUERYCAP
+
+
+Arguments
+=
+
+``fd``
+File descriptor returned by :ref:`open() `.
+
+``argp``
+
+
+Description
+===
+
+All V4L2 sub-devices support the
+``VIDIOC_SUBDEV_QUERYCAP`` ioctl. It is used to identify
+kernel devices compatible with this specification and to obtain
+information about driver and hardware capabilities. The ioctl takes a
+pointer to a struct :c:type:`v4l2_subdev_capability` which is filled by the
+driver. When the driver is not compatible with this specification the ioctl
+returns ``ENOTTY`` error code.
+
+.. tabularcolumns:: |p{1.5cm}|p{2.5cm}|p{13cm}|
+
+.. c:type:: v4l2_subdev_capability
+
+.. flat-table:: struct v4l2_subdev_capability
+:header-rows:  0
+:stub-columns: 0
+:widths:   3 4 20
+
+* - __u32
+  - ``version``
+  - Version number of the driver.
+
+   The version reported is provided by the
+   V4L2 subsystem following the kernel numbering scheme. However, it
+   may not always return the same version as the kernel if, for
+   example, a stable or distribution-modified kernel uses the V4L2
+   stack from a newer kernel.
+
+   The version number is formatted using the ``KERNEL_VERSION()``
+   macro:
+* - :cspan:`2`
+
+   ``#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))``
+
+   ``__u32 version = KERNEL_VERSION(0, 8, 1);``
+
+   ``printf ("Version: %u.%u.%u\\n",``
+
+   ``(version >> 16) & 0xFF, (version >> 8) & 0xFF, version & 0xFF);``
+* - __u32
+  - ``device_caps``
+  - Sub-device capabilities of the opened device, see
+   :ref:`subdevice-capabilities`.
+* - __u32
+  - ``entity_id``
+  - The media controller entity ID of the sub-device. This is only valid if
+the ``V4L2_SUBDEV_CAP_ENTITY`` capability is set, it is 0 otherwise.
+* - __u32
+  - ``media_node_major``
+  - The major number of the media controller device node corresponding 
sub-device.
+This is only valid if the ``V4L2_SUBDEV_CAP_ENTITY`` capability is 
set, it is
+   0 otherwise.
+* - __u32
+  - ``media_node_minor``
+  - The minor number of the media controller device node corresponding 
sub-device.
+This is only valid if the ``V4L2_SUBDEV_CAP_ENTITY`` capability is 
set, it is
+   0 otherwise.
+* - __u32
+  - ``reserved``\ [27]
+  - Reserved for future extensions. Applications and drivers must set
+   the array to zero.
+
+.. tabularcolumns:: |p{6cm}|p{2.2cm}|p{8.8cm}|
+
+.. _subdevice-capabilities:
+
+.. cssclass:: longtable
+
+.. flat-table:: Sub-Device Capabilities Flags
+:header-rows:  0
+:stub-columns: 0
+:widths:   3 1 4
+
+* - ``V4L2_SUBDEV_CAP_ENTITY``
+  - 0x0001
+  - The sub-device is a media controller entity and the ``entity_id``,
+``media_node_major`` and ``media_node_minor`` fields of
+struct :c:type:`v4l2_subdev_capability` are valid. These fields
+   are 0 if this capability is not set.
+
+Return Value
+
+
+On success 0 is returned, on error -1 and the ``errno`` variable is set
+appropriately. The generic error codes are described at the
+:ref:`Generic Error Codes ` chapter.
-- 
2.13.1

[RFC PATCH 0/2] add VIDIOC_SUBDEV_QUERYCAP ioctl

[Hans Verkuil]

From: Hans Verkuil 

I tried to get this in back in 2015, but that effort stalled.

Trying again, since I really need this in order to add proper v4l-subdev
support to v4l2-ctl and v4l2-compliance. There currently is no way of
unique identifying that the device really is a v4l-subdev device other
than the device name (which can be changed by udev).

So this patch series adds a VIDIOC_SUBDEV_QUERYCAP ioctl that is in
the core so it's guaranteed to be there.

If the subdev is part of an MC then it also gives the corresponding
entity ID of the subdev and the major/minor numbers of the MC device
so v4l2-compliance can relate the subdev device directly to the right
MC device. The reserved array has room enough for strings should we
need them later. 

Hans Verkuil (2):
  v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl
  v4l: document VIDIOC_SUBDEV_QUERYCAP

 Documentation/media/uapi/v4l/user-func.rst |   1 +
 .../media/uapi/v4l/vidioc-subdev-querycap.rst  | 118 +
 drivers/media/v4l2-core/v4l2-subdev.c  |  26 +
 include/uapi/linux/v4l2-subdev.h   |  29 +
 4 files changed, 174 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/vidioc-subdev-querycap.rst

-- 
2.13.1

[RFC PATCH 1/2] v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl

[Hans Verkuil]

From: Hans Verkuil 

While normal video/radio/vbi/swradio nodes have a proper QUERYCAP ioctl
that apps can call to determine that it is indeed a V4L2 device, there
is currently no equivalent for v4l-subdev nodes. Adding this ioctl will
solve that, and it will allow utilities like v4l2-compliance to be used
with these devices as well.

SUBDEV_QUERYCAP currently returns the version and device_caps of the
subdevice. If the subdev is used as part of a media controller, then
it also returns the entity ID and the major and minor numbers of the
media controller device node.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-subdev.c | 26 ++
 include/uapi/linux/v4l2-subdev.h  | 29 +
 2 files changed, 55 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
b/drivers/media/v4l2-core/v4l2-subdev.c
index 43fefa73e0a3..1c7881d0ab08 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -20,8 +20,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -186,6 +188,30 @@ static long subdev_do_ioctl(struct file *file, unsigned 
int cmd, void *arg)
 #endif
 
switch (cmd) {
+   case VIDIOC_SUBDEV_QUERYCAP: {
+   struct v4l2_subdev_capability *cap = arg;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   struct media_device *mdev = sd->entity.graph_obj.mdev;
+   struct media_devnode *devnode = mdev ? mdev->devnode : NULL;
+#endif
+
+   cap->version = LINUX_VERSION_CODE;
+   cap->device_caps = 0;
+   cap->entity_id = 0;
+   cap->media_node_major = 0;
+   cap->media_node_minor = 0;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   if (devnode) {
+   cap->device_caps = V4L2_SUBDEV_CAP_ENTITY;
+   cap->entity_id = sd->entity.graph_obj.id;
+   cap->media_node_major = MAJOR(devnode->cdev.dev);
+   cap->media_node_minor = MINOR(devnode->cdev.dev);
+   }
+#endif
+   memset(cap->reserved, 0, sizeof(cap->reserved));
+   break;
+   }
+
case VIDIOC_QUERYCTRL:
return v4l2_queryctrl(vfh->ctrl_handler, arg);
 
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index dbce2b554e02..83e8e68c9dcd 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -154,9 +154,38 @@ struct v4l2_subdev_selection {
__u32 reserved[8];
 };
 
+/**
+ * struct v4l2_subdev_capability - subdev capabilities
+ * @version: the kernel version
+ * @device_caps: the subdev capabilities
+ * @entity_id: the entity ID as assigned by the media controller. Only
+ * valid if V4L2_SUBDEV_CAP_ENTITY is set
+ * @media_node_major: the major number of the media controller device node.
+ * Only valid if V4L2_SUBDEV_CAP_ENTITY is set
+ * @media_node_minor: the minor number of the media controller device node.
+ * Only valid if V4L2_SUBDEV_CAP_ENTITY is set
+ * @reserved: for future use, set to zero for now
+ */
+struct v4l2_subdev_capability {
+   __u32 version;
+   __u32 device_caps;
+   __u32 entity_id;
+   /* Corresponding media controller device node specifications */
+   __u32 media_node_major;
+   __u32 media_node_minor;
+   __u32 reserved[27];
+};
+
+/*
+ * This v4l2_subdev is also a media entity and the entity_id, media_node_major
+ * and media_node_minor fields are valid
+ */
+#define V4L2_SUBDEV_CAP_ENTITY (1 << 0)
+
 /* Backwards compatibility define --- to be removed */
 #define v4l2_subdev_edid v4l2_edid
 
+#define VIDIOC_SUBDEV_QUERYCAP  _IOR('V',  0, struct 
v4l2_subdev_capability)
 #define VIDIOC_SUBDEV_G_FMT_IOWR('V',  4, struct 
v4l2_subdev_format)
 #define VIDIOC_SUBDEV_S_FMT_IOWR('V',  5, struct 
v4l2_subdev_format)
 #define VIDIOC_SUBDEV_G_FRAME_INTERVAL _IOWR('V', 21, struct 
v4l2_subdev_frame_interval)
-- 
2.13.1

Re: [PATCH v4] uvcvideo: add a metadata device node

[Laurent Pinchart]

Hi Guennadi,

On Friday 28 Jul 2017 10:59:29 Guennadi Liakhovetski wrote:
> On Fri, 28 Jul 2017, Laurent Pinchart wrote:
> > On Tuesday 25 Jul 2017 15:27:24 Guennadi Liakhovetski wrote:
> >> On Fri, 21 Jul 2017, Laurent Pinchart wrote:
> >>> Hi Guennadi,
> >>> 
> >>> Thank you for the patch.
> >>> 
>  Some UVC video cameras contain metadata in their payload headers.
>  This patch extracts that data, adding more clock synchronisation
>  information, on both bulk and isochronous endpoints and makes it
>  available to the user space on a separate video node, using the
>  V4L2_CAP_META_CAPTURE capability and the V4L2_BUF_TYPE_META_CAPTURE
>  buffer queue type. Even though different cameras will have different
>  metadata formats, we use the same V4L2_META_FMT_UVC pixel format for
>  all of them. Users have to parse data, based on the specific camera
>  model information.
> >>> 
> >>> The issue we still haven't addressed is how to ensure that vendors
> >>> will document their metadata format :-S
> >> 
> >> Uhm, add a black list of offending vendors and drop 60% of their frames?
> >> ;-)
> > 
> > This was actually a serious question :-)
> > 
> > How about white-listing cameras instead, and enabling extended metadata
> > (after the standard header) support only for vendors who have documented
> > their format ?
> > 
> > Speaking of which, where's the documentation for the camera you're working
> > on ? :-)
> 
> The metadata definition is at the known to you page
> https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensi
> ons-1-5 :-) But yes, I'll check with managers and submit a dev-flag patch
> too.

What bothers me is

"The MetadataId field is filled by an identifier from the following enum 
definition which contains well-defined identifiers as well as custom 
identifiers (identifiers >= MetadataId_Custom_Start)."

I want vendors to document the custom fields.

>  This version of the patch only creates such metadata nodes for
>  cameras, specifying a UVC_QUIRK_METADATA_NODE quirk flag.
>  
>  Signed-off-by: Guennadi Liakhovetski
>  
>  ---
>  
>  v4:
>  - add support for isochronous cameras. Metadata is now collected from
>  as many payloads as they fit in the buffer
>  - add a USB Start Of Frame and a system timestamp to each metadata
>  block for user-space clock synchronisation
>  - use a default buffer size of 1024 bytes
>  
>  Thanks to Laurent for patient long discussions and to everybody, who
>  helped me conduct all the investigation into various past, present
>  and future UVC cameras :-)
>  
>   drivers/media/usb/uvc/Makefile   |   2 +-
>   drivers/media/usb/uvc/uvc_driver.c   |   4 +
>   drivers/media/usb/uvc/uvc_isight.c   |   2 +-
>   drivers/media/usb/uvc/uvc_metadata.c | 158 ++
>   drivers/media/usb/uvc/uvc_queue.c|  68 ++-
>   drivers/media/usb/uvc/uvc_video.c| 101 +++---
>   drivers/media/usb/uvc/uvcvideo.h |  23 -
>   drivers/media/v4l2-core/v4l2-ioctl.c |   1 +
>   include/uapi/linux/uvcvideo.h|  19 +
>   include/uapi/linux/videodev2.h   |   3 +
>   10 files changed, 347 insertions(+), 34 deletions(-)
>   create mode 100644 drivers/media/usb/uvc/uvc_metadata.c

[snip]

>  diff --git a/drivers/media/usb/uvc/uvc_queue.c
>  b/drivers/media/usb/uvc/uvc_queue.c
>  index aa21997..77dedbc 100644
>  --- a/drivers/media/usb/uvc/uvc_queue.c
>  +++ b/drivers/media/usb/uvc/uvc_queue.c
> > 
> > [snip]
> > 
>  -static int uvc_buffer_prepare(struct vb2_buffer *vb)
>  +int uvc_buffer_prepare(struct vb2_buffer *vb)
>   {
>   struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>   struct uvc_video_queue *queue = vb2_get_drv_priv(vb
>  ->vb2_queue);
>   struct uvc_buffer *buf = uvc_vbuf_to_buffer(vbuf);
>  
>  -if (vb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT &&
>  -vb2_get_plane_payload(vb, 0) > vb2_plane_size(vb, 0)) {
>  -uvc_trace(UVC_TRACE_CAPTURE, "[E] Bytes used out of
>  bounds.\n");
>  -return -EINVAL;
>  -}
>  -
>   if (unlikely(queue->flags & UVC_QUEUE_DISCONNECTED))
>   return -ENODEV;
>  
>  +switch (vb->type) {
>  +case V4L2_BUF_TYPE_VIDEO_OUTPUT:
>  +if (vb2_get_plane_payload(vb, 0) > vb2_plane_size(vb,
>  0)) {
>  +uvc_trace(UVC_TRACE_CAPTURE,
>  +  "[E] Bytes used out of bounds.\n");
>  +return -EINVAL;
>  +}
>  +
>  +buf->bytesused = vb2_get_plane_payload(vb, 0);
>  +
>  +break;
>  +case 

[PATCH] cec: documentation fixes

[Hans Verkuil]

Various references to open() et al were wrong. Fix this so following
the link will get you to the correct place.

Signed-off-by: Hans Verkuil 
---
 Documentation/media/uapi/cec/cec-func-close.rst  | 2 +-
 Documentation/media/uapi/cec/cec-func-ioctl.rst  | 2 +-
 Documentation/media/uapi/cec/cec-func-open.rst   | 4 ++--
 Documentation/media/uapi/cec/cec-func-poll.rst   | 8 
 Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst | 2 +-
 Documentation/media/uapi/cec/cec-ioc-dqevent.rst | 2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/media/uapi/cec/cec-func-close.rst 
b/Documentation/media/uapi/cec/cec-func-close.rst
index 895d9c2d1c04..334358dfa72e 100644
--- a/Documentation/media/uapi/cec/cec-func-close.rst
+++ b/Documentation/media/uapi/cec/cec-func-close.rst
@@ -40,7 +40,7 @@ freed. The device configuration remain unchanged.
 Return Value
 

-:c:func:`close()` returns 0 on success. On error, -1 is returned, and
+:c:func:`close() ` returns 0 on success. On error, -1 is returned, 
and
 ``errno`` is set appropriately. Possible error codes are:

 ``EBADF``
diff --git a/Documentation/media/uapi/cec/cec-func-ioctl.rst 
b/Documentation/media/uapi/cec/cec-func-ioctl.rst
index 22fb6304a2df..e2b6260b0086 100644
--- a/Documentation/media/uapi/cec/cec-func-ioctl.rst
+++ b/Documentation/media/uapi/cec/cec-func-ioctl.rst
@@ -39,7 +39,7 @@ Arguments
 Description
 ===

-The :c:func:`ioctl()` function manipulates cec device parameters. The
+The :c:func:`ioctl() ` function manipulates cec device parameters. 
The
 argument ``fd`` must be an open file descriptor.

 The ioctl ``request`` code specifies the cec function to be called. It
diff --git a/Documentation/media/uapi/cec/cec-func-open.rst 
b/Documentation/media/uapi/cec/cec-func-open.rst
index 18dfb62f2efe..5d6663a649bd 100644
--- a/Documentation/media/uapi/cec/cec-func-open.rst
+++ b/Documentation/media/uapi/cec/cec-func-open.rst
@@ -46,7 +46,7 @@ Arguments
 Description
 ===

-To open a cec device applications call :c:func:`open()` with the
+To open a cec device applications call :c:func:`open() ` with the
 desired device name. The function has no side effects; the device
 configuration remain unchanged.

@@ -58,7 +58,7 @@ EBADF.
 Return Value
 

-:c:func:`open()` returns the new file descriptor on success. On error,
+:c:func:`open() ` returns the new file descriptor on success. On 
error,
 -1 is returned, and ``errno`` is set appropriately. Possible error codes
 include:

diff --git a/Documentation/media/uapi/cec/cec-func-poll.rst 
b/Documentation/media/uapi/cec/cec-func-poll.rst
index fa0abd8fb160..d49f1ee0742d 100644
--- a/Documentation/media/uapi/cec/cec-func-poll.rst
+++ b/Documentation/media/uapi/cec/cec-func-poll.rst
@@ -39,10 +39,10 @@ Arguments
 Description
 ===

-With the :c:func:`poll()` function applications can wait for CEC
+With the :c:func:`poll() ` function applications can wait for CEC
 events.

-On success :c:func:`poll()` returns the number of file descriptors
+On success :c:func:`poll() ` returns the number of file descriptors
 that have been selected (that is, file descriptors for which the
 ``revents`` field of the respective struct :c:type:`pollfd`
 is non-zero). CEC devices set the ``POLLIN`` and ``POLLRDNORM`` flags in
@@ -53,13 +53,13 @@ then the ``POLLPRI`` flag is set. When the function times 
out it returns
 a value of zero, on failure it returns -1 and the ``errno`` variable is
 set appropriately.

-For more details see the :c:func:`poll()` manual page.
+For more details see the :c:func:`poll() ` manual page.


 Return Value
 

-On success, :c:func:`poll()` returns the number structures which have
+On success, :c:func:`poll() ` returns the number structures which 
have
 non-zero ``revents`` fields, or zero if the call timed out. On error -1
 is returned, and the ``errno`` variable is set appropriately:

diff --git a/Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst 
b/Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst
index 882d6e025747..0a7aa21f24f4 100644
--- a/Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst
+++ b/Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst
@@ -21,7 +21,7 @@ Arguments
 =

 ``fd``
-File descriptor returned by :ref:`open() `.
+File descriptor returned by :c:func:`open() `.

 ``argp``

diff --git a/Documentation/media/uapi/cec/cec-ioc-dqevent.rst 
b/Documentation/media/uapi/cec/cec-ioc-dqevent.rst
index 3e2cd5fefd38..766d8b0ce431 100644
--- a/Documentation/media/uapi/cec/cec-ioc-dqevent.rst
+++ b/Documentation/media/uapi/cec/cec-ioc-dqevent.rst
@@ -22,7 +22,7 @@ Arguments
 =

 ``fd``
-File descriptor returned by :ref:`open() `.
+File descriptor returned by :c:func:`open() `.

 ``argp``

-- 
2.13.1

[PATCH] vimc: add test_pattern and h/vflip controls to the sensor

[Hans Verkuil]

Add support for the test_pattern control and the h/vflip controls.

This makes it possible to switch to more interesting test patterns and to
test control handling in v4l-subdevs.

There are more tpg-related controls that can be added, but this is a good
start.

Signed-off-by: Hans Verkuil 
---
diff --git a/drivers/media/platform/vimc/vimc-common.h 
b/drivers/media/platform/vimc/vimc-common.h
index dca528a316e7..2e9981b18166 100644
--- a/drivers/media/platform/vimc/vimc-common.h
+++ b/drivers/media/platform/vimc/vimc-common.h
@@ -22,6 +22,11 @@
 #include 
 #include 

+/* VIMC-specific controls */
+#define VIMC_CID_VIMC_BASE (0x00f0 | 0xf000)
+#define VIMC_CID_VIMC_CLASS(0x00f0 | 1)
+#define VIMC_CID_TEST_PATTERN  (VIMC_CID_VIMC_BASE + 0)
+
 #define VIMC_FRAME_MAX_WIDTH 4096
 #define VIMC_FRAME_MAX_HEIGHT 2160
 #define VIMC_FRAME_MIN_WIDTH 16
diff --git a/drivers/media/platform/vimc/vimc-sensor.c 
b/drivers/media/platform/vimc/vimc-sensor.c
index 615c2b18dcfc..532097566b27 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 

@@ -38,6 +39,7 @@ struct vimc_sen_device {
u8 *frame;
/* The active format */
struct v4l2_mbus_framefmt mbus_format;
+   struct v4l2_ctrl_handler hdl;
 };

 static const struct v4l2_mbus_framefmt fmt_default = {
@@ -291,6 +293,31 @@ static const struct v4l2_subdev_ops vimc_sen_ops = {
.video = _sen_video_ops,
 };

+static int vimc_sen_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct vimc_sen_device *vsen =
+   container_of(ctrl->handler, struct vimc_sen_device, hdl);
+
+   switch (ctrl->id) {
+   case VIMC_CID_TEST_PATTERN:
+   tpg_s_pattern(>tpg, ctrl->val);
+   break;
+   case V4L2_CID_HFLIP:
+   tpg_s_hflip(>tpg, ctrl->val);
+   break;
+   case V4L2_CID_VFLIP:
+   tpg_s_vflip(>tpg, ctrl->val);
+   break;
+   default:
+   return -EINVAL;
+   }
+   return 0;
+}
+
+static const struct v4l2_ctrl_ops vimc_sen_ctrl_ops = {
+   .s_ctrl = vimc_sen_s_ctrl,
+};
+
 static void vimc_sen_comp_unbind(struct device *comp, struct device *master,
 void *master_data)
 {
@@ -299,10 +326,29 @@ static void vimc_sen_comp_unbind(struct device *comp, 
struct device *master,
container_of(ved, struct vimc_sen_device, ved);

vimc_ent_sd_unregister(ved, >sd);
+   v4l2_ctrl_handler_free(>hdl);
tpg_free(>tpg);
kfree(vsen);
 }

+/* Image Processing Controls */
+static const struct v4l2_ctrl_config vimc_sen_ctrl_class = {
+   .ops = _sen_ctrl_ops,
+   .flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY,
+   .id = VIMC_CID_VIMC_CLASS,
+   .name = "VIMC Controls",
+   .type = V4L2_CTRL_TYPE_CTRL_CLASS,
+};
+
+static const struct v4l2_ctrl_config vimc_sen_ctrl_test_pattern = {
+   .ops = _sen_ctrl_ops,
+   .id = VIMC_CID_TEST_PATTERN,
+   .name = "Test Pattern",
+   .type = V4L2_CTRL_TYPE_MENU,
+   .max = TPG_PAT_NOISE,
+   .qmenu = tpg_pattern_strings,
+};
+
 static int vimc_sen_comp_bind(struct device *comp, struct device *master,
  void *master_data)
 {
@@ -316,6 +362,20 @@ static int vimc_sen_comp_bind(struct device *comp, struct 
device *master,
if (!vsen)
return -ENOMEM;

+   v4l2_ctrl_handler_init(>hdl, 4);
+
+   v4l2_ctrl_new_custom(>hdl, _sen_ctrl_class, NULL);
+   v4l2_ctrl_new_custom(>hdl, _sen_ctrl_test_pattern, NULL);
+   v4l2_ctrl_new_std(>hdl, _sen_ctrl_ops,
+   V4L2_CID_VFLIP, 0, 1, 1, 0);
+   v4l2_ctrl_new_std(>hdl, _sen_ctrl_ops,
+   V4L2_CID_HFLIP, 0, 1, 1, 0);
+   vsen->sd.ctrl_handler = >hdl;
+   if (vsen->hdl.error) {
+   ret = vsen->hdl.error;
+   goto err_free_vsen;
+   }
+
/* Initialize ved and sd */
ret = vimc_ent_sd_register(>ved, >sd, v4l2_dev,
   pdata->entity_name,
@@ -323,7 +383,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct 
device *master,
   (const unsigned long[1]) 
{MEDIA_PAD_FL_SOURCE},
   _sen_ops);
if (ret)
-   goto err_free_vsen;
+   goto err_free_hdl;

dev_set_drvdata(comp, >ved);
vsen->dev = comp;
@@ -342,6 +402,8 @@ static int vimc_sen_comp_bind(struct device *comp, struct 
device *master,

 err_unregister_ent_sd:
vimc_ent_sd_unregister(>ved,  >sd);
+err_free_hdl:
+   v4l2_ctrl_handler_free(>hdl);
 err_free_vsen:
kfree(vsen);

[PATCH v1 2/5] [media] stm32-dcmi: revisit control register handling

[Hugues Fruchet]

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 716b3db..526e354 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(>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

[PATCH v1 0/5] STM32 DCMI camera interface crop support

[Hugues Fruchet]

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 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->frmsizes_count[pixfmt] 
> 1
test Cropping: FAIL
test Composing: OK (Not Supported)
fail: v4l2-test-formats.cpp(1633): node->can_scale && 
node->frmsizes_count[v4l_format_g_pixelformat()]
test Scaling: OK

Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK

Test input 0:

Streaming ioctls:

[PATCH v1 4/5] [media] stm32-dcmi: set default format at open()

[Hugues Fruchet]

Ensure that we start with default pixel format and resolution
when opening a new instance.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/platform/stm32/stm32-dcmi.c | 49 ++-
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 4733d1f..2be56b6 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -890,6 +890,28 @@ static int dcmi_enum_frameintervals(struct file *file, 
void *fh,
return 0;
 }
 
+static int dcmi_set_default_fmt(struct stm32_dcmi *dcmi)
+{
+   struct v4l2_format f = {
+   .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+   .fmt.pix = {
+   .width  = CIF_WIDTH,
+   .height = CIF_HEIGHT,
+   .field  = V4L2_FIELD_NONE,
+   .pixelformat= dcmi->sd_formats[0]->fourcc,
+   },
+   };
+   int ret;
+
+   ret = dcmi_try_fmt(dcmi, , NULL);
+   if (ret)
+   return ret;
+   dcmi->sd_format = dcmi->sd_formats[0];
+   dcmi->fmt = f;
+
+   return 0;
+}
+
 static const struct of_device_id stm32_dcmi_of_match[] = {
{ .compatible = "st,stm32-dcmi"},
{ /* end node */ },
@@ -916,7 +938,13 @@ static int dcmi_open(struct file *file)
if (ret < 0 && ret != -ENOIOCTLCMD)
goto fh_rel;
 
+   ret = dcmi_set_default_fmt(dcmi);
+   if (ret)
+   goto power_off;
+
ret = dcmi_set_fmt(dcmi, >fmt);
+
+power_off:
if (ret)
v4l2_subdev_call(sd, core, s_power, 0);
 fh_rel:
@@ -991,27 +1019,6 @@ static int dcmi_release(struct file *file)
.read   = vb2_fop_read,
 };
 
-static int dcmi_set_default_fmt(struct stm32_dcmi *dcmi)
-{
-   struct v4l2_format f = {
-   .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
-   .fmt.pix = {
-   .width  = CIF_WIDTH,
-   .height = CIF_HEIGHT,
-   .field  = V4L2_FIELD_NONE,
-   .pixelformat= dcmi->sd_formats[0]->fourcc,
-   },
-   };
-   int ret;
-
-   ret = dcmi_try_fmt(dcmi, , NULL);
-   if (ret)
-   return ret;
-   dcmi->sd_format = dcmi->sd_formats[0];
-   dcmi->fmt = f;
-   return 0;
-}
-
 static const struct dcmi_format dcmi_formats[] = {
{
.fourcc = V4L2_PIX_FMT_RGB565,
-- 
1.9.1

[PATCH v1 5/5] [media] stm32-dcmi: g_/s_selection crop support

[Hugues Fruchet]

Implements g_/s_selection crop support by using DCMI crop
hardware feature.
User can first get the maximum supported resolution of the sensor
by calling g_selection(V4L2_SEL_TGT_CROP_BOUNDS).
Then user call to s_selection(V4L2_SEL_TGT_CROP) will reset sensor
to its maximum resolution and crop request is saved for later usage
in s_fmt().
Next call to s_fmt() will check if sensor can do frame size request
with crop request. If sensor supports only discrete frame sizes,
the frame size which is larger than user request is selected in
order to be able to match the crop request. Then s_fmt() resolution
user request is adjusted to match crop request resolution.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/platform/stm32/stm32-dcmi.c | 367 +-
 1 file changed, 363 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 2be56b6..f1fb0b3 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define DRV_NAME "stm32-dcmi"
@@ -107,6 +108,11 @@ struct dcmi_format {
u8  bpp;
 };
 
+struct dcmi_framesize {
+   u32 width;
+   u32 height;
+};
+
 struct dcmi_buf {
struct vb2_v4l2_buffer  vb;
boolprepared;
@@ -131,10 +137,16 @@ struct stm32_dcmi {
struct v4l2_async_notifier  notifier;
struct dcmi_graph_entityentity;
struct v4l2_format  fmt;
+   struct v4l2_rectcrop;
+   booldo_crop;
 
const struct dcmi_format**sd_formats;
unsigned intnb_of_sd_formats;
const struct dcmi_format*sd_format;
+   struct dcmi_framesize   *sd_framesizes;
+   unsigned intnb_of_sd_framesizes;
+   struct dcmi_framesize   sd_framesize;
+   struct v4l2_rectsd_bounds;
 
/* Protect this data structure */
struct mutexlock;
@@ -325,6 +337,28 @@ static int dcmi_start_capture(struct stm32_dcmi *dcmi)
return 0;
 }
 
+static void dcmi_set_crop(struct stm32_dcmi *dcmi)
+{
+   u32 size, start;
+
+   /* Crop resolution */
+   size = ((dcmi->crop.height - 1) << 16) |
+   ((dcmi->crop.width << 1) - 1);
+   reg_write(dcmi->regs, DCMI_CWSIZE, size);
+
+   /* Crop start point */
+   start = ((dcmi->crop.top) << 16) |
+((dcmi->crop.left << 1));
+   reg_write(dcmi->regs, DCMI_CWSTRT, start);
+
+   dev_dbg(dcmi->dev, "Cropping to %ux%u@%u:%u\n",
+   dcmi->crop.width, dcmi->crop.height,
+   dcmi->crop.left, dcmi->crop.top);
+
+   /* Enable crop */
+   reg_set(dcmi->regs, DCMI_CR, CR_CROP);
+}
+
 static irqreturn_t dcmi_irq_thread(int irq, void *arg)
 {
struct stm32_dcmi *dcmi = arg;
@@ -540,6 +574,10 @@ static int dcmi_start_streaming(struct vb2_queue *vq, 
unsigned int count)
 
reg_write(dcmi->regs, DCMI_CR, val);
 
+   /* Set crop */
+   if (dcmi->do_crop)
+   dcmi_set_crop(dcmi);
+
/* Enable dcmi */
reg_set(dcmi->regs, DCMI_CR, CR_ENABLE);
 
@@ -697,10 +735,37 @@ static const struct dcmi_format 
*find_format_by_fourcc(struct stm32_dcmi *dcmi,
return NULL;
 }
 
+static void __find_outer_frame_size(struct stm32_dcmi *dcmi,
+   struct v4l2_pix_format *pix,
+   struct dcmi_framesize *framesize)
+{
+   struct dcmi_framesize *match = NULL;
+   unsigned int i;
+   unsigned int min_err = UINT_MAX;
+
+   for (i = 0; i < dcmi->nb_of_sd_framesizes; i++) {
+   struct dcmi_framesize *fsize = >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 = >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 = >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, 

[PATCH v1 1/5] [media] stm32-dcmi: catch dma submission error

[Hugues Fruchet]

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 24ef888..716b3db 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 v1 3/5] [media] stm32-dcmi: cleanup variable/fields namings

[Hugues Fruchet]

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 526e354..4733d1f 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 intnb_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->nb_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 = >fmt.pix;
+   const struct dcmi_format *sd_fmt;
+   struct v4l2_pix_format *pix = >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->nb_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(, pixfmt, dcmi_fmt->mbus_code);
+   v4l2_fill_mbus_format(, pix, sd_fmt->mbus_code);
ret = v4l2_subdev_call(dcmi->entity.subdev, pad, set_fmt,
   _cfg, );
if (ret < 0)
return ret;
 
-   v4l2_fill_pix_format(pixfmt, );
+   /* Update pix regarding to what sensor can do */
+   v4l2_fill_pix_format(pix, );
 
-   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 = 
+   struct v4l2_pix_format *pix = >fmt.pix;
int ret;
 
-   ret = dcmi_try_fmt(dcmi, f, _fmt);
+   ret = dcmi_try_fmt(dcmi, f, _format);
if (ret)
return ret;
 
-   v4l2_fill_mbus_format(, >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,
   set_fmt, NULL, );
if (ret < 0)
return 

Re: [PATCH v4] uvcvideo: add a metadata device node

[Guennadi Liakhovetski]

Hi Laurent,

On Fri, 28 Jul 2017, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Tuesday 25 Jul 2017 15:27:24 Guennadi Liakhovetski wrote:
> > On Fri, 21 Jul 2017, Laurent Pinchart wrote:
> > > Hi Guennadi,
> > > 
> > > Thank you for the patch.
> > > 
> > >> Some UVC video cameras contain metadata in their payload headers. This
> > >> patch extracts that data, adding more clock synchronisation information,
> > >> on both bulk and isochronous endpoints and makes it available to the
> > >> user space on a separate video node, using the V4L2_CAP_META_CAPTURE
> > >> capability and the V4L2_BUF_TYPE_META_CAPTURE buffer queue type. Even
> > >> though different cameras will have different metadata formats, we use
> > >> the same V4L2_META_FMT_UVC pixel format for all of them. Users have to
> > >> parse data, based on the specific camera model information.
> > > 
> > > The issue we still haven't addressed is how to ensure that vendors will
> > > document their metadata format :-S
> > 
> > Uhm, add a black list of offending vendors and drop 60% of their frames?
> > ;-)
> 
> This was actually a serious question :-)
> 
> How about white-listing cameras instead, and enabling extended metadata 
> (after 
> the standard header) support only for vendors who have documented their 
> format 
> ?
> 
> Speaking of which, where's the documentation for the camera you're working on 
> ? :-)

The metadata definition is at the known to you page 
https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
 
:-) But yes, I'll check with managers and submit a dev-flag patch too.

> > >> This version of the patch only creates such metadata nodes for cameras,
> > >> specifying a UVC_QUIRK_METADATA_NODE quirk flag.
> > >> 
> > >> Signed-off-by: Guennadi Liakhovetski 
> > >> ---
> > >> 
> > >> v4:
> > >> - add support for isochronous cameras. Metadata is now collected from as
> > >> many payloads as they fit in the buffer
> > >> - add a USB Start Of Frame and a system timestamp to each metadata block
> > >> for user-space clock synchronisation
> > >> - use a default buffer size of 1024 bytes
> > >> 
> > >> Thanks to Laurent for patient long discussions and to everybody, who
> > >> helped me conduct all the investigation into various past, present and
> > >> future UVC cameras :-)
> > >> 
> > >>  drivers/media/usb/uvc/Makefile   |   2 +-
> > >>  drivers/media/usb/uvc/uvc_driver.c   |   4 +
> > >>  drivers/media/usb/uvc/uvc_isight.c   |   2 +-
> > >>  drivers/media/usb/uvc/uvc_metadata.c | 158 
> > >>  drivers/media/usb/uvc/uvc_queue.c|  68 ++-
> > >>  drivers/media/usb/uvc/uvc_video.c| 101 +++---
> > >>  drivers/media/usb/uvc/uvcvideo.h |  23 -
> > >>  drivers/media/v4l2-core/v4l2-ioctl.c |   1 +
> > >>  include/uapi/linux/uvcvideo.h|  19 +
> > >>  include/uapi/linux/videodev2.h   |   3 +
> > >>  10 files changed, 347 insertions(+), 34 deletions(-)
> > >>  create mode 100644 drivers/media/usb/uvc/uvc_metadata.c
> > > 
> > > [snip]
> > > 
> > >> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> > >> b/drivers/media/usb/uvc/uvc_driver.c
> > >> index 70842c5..9f23dcf 100644
> > >> --- a/drivers/media/usb/uvc/uvc_driver.c
> > >> +++ b/drivers/media/usb/uvc/uvc_driver.c
> 
> [snip]
> 
> > >> @@ -1941,6 +1942,9 @@ static int uvc_register_video(struct uvc_device
> > >> *dev,
> > > > return ret;
> > > > }
> > > > 
> > > > +   /* Register a metadata node. */
> > > > +   uvc_meta_register(stream);
> > > 
> > > This can fail, you should handle errors.
> > 
> > Yes, this is in a way deliberate. If metadata node registration fails, the
> > driver continues without one. Would you prefer to fail and unregister the
> > main node in this case?
> 
> No, that's fine, but you should then add a comment to explain why the error 
> isn't handled.
> 
> Please also make sure that no damage can occur at device removal time if the 
> metadata node hasn't been registered.

Don't see any potential problems in that case.

> > > > if (stream->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> > > > stream->chain->caps |= V4L2_CAP_VIDEO_CAPTURE;
> > > > else
> > > 
> > > [snip]
> > > 
> > >> diff --git a/drivers/media/usb/uvc/uvc_metadata.c
> > >> b/drivers/media/usb/uvc/uvc_metadata.c
> > >> new file mode 100644
> > >> index 000..876badd
> > >> --- /dev/null
> > >> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> 
> [snip]
> 
> > >> +/* 
> > >> + * videobuf2 Queue Operations
> > >> + */
> > >> +
> > >> +static struct vb2_ops uvc_meta_queue_ops = {
> > >> +.queue_setup = uvc_queue_setup,
> > >> +.buf_prepare = uvc_buffer_prepare,
> > >> +.buf_queue = uvc_buffer_queue,
> > >> +.wait_prepare = vb2_ops_wait_prepare,
> > >> +.wait_finish = vb2_ops_wait_finish,
> > >> 

Re: [RFC 04/19] dt: bindings: Add lens-focus binding for image sensors

[Hans Verkuil]

Hi Sakari,

Is the lens-focus phandle specific to voice-coil controllers? What about
motor-controlled focus systems? What when there are multiple motors? E.g.
one for the focus, one for the iris control (yes, we have that).

What if you have two sensors (stereoscopic) controlled by one motor?

Just trying to understand this from a bigger perspective. Specifically
how this will scale when more of these supporting devices as added.

Regards,

Hans

On 07/18/2017 09:03 PM, Sakari Ailus wrote:
> The lens-focus property contains a phandle to the lens voice coil driver
> that is associated to the sensor; typically both are contained in the same
> camera module.
> 
> Signed-off-by: Sakari Ailus 
> Acked-by: Pavel Machek 
> Reviewed-by: Sebastian Reichel 
> Acked-by: Rob Herring 
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt 
> b/Documentation/devicetree/bindings/media/video-interfaces.txt
> index 9723f7e1c7db..a18d9b2d309f 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -74,6 +74,8 @@ Optional properties
>  - flash: phandle referring to the flash driver chip. A flash driver may
>have multiple flashes connected to it.
>  
> +- lens-focus: A phandle to the node of the focus lens controller.
> +
>  
>  Optional endpoint properties
>  
> 

[GIT PULL FOR v4.14] Doc and TGP fixes

[Hans Verkuil]

Hi Mauro,

Here are various documentation fixes/improvements.

The first patch renames the old pixfmt-0XX.rst files to something I can
understand since I could never find the right rst file for the colorspace
documentation...

Regards,

Hans

The following changes since commit da48c948c263c9d87dfc64566b3373a858cc8aa2:

  media: fix warning on v4l2_subdev_call() result interpreted as bool 
(2017-07-26 13:43:17 -0400)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git tpg-doc-fixes

for you to fetch changes up to 2ebc8a9b217c24a2e12f775b1b107ce7b8c28166:

  v4l2-tpg-core.c: fix typo in bt2020_full matrix (2017-07-28 09:33:58 +0200)


Hans Verkuil (6):
  media/doc: rename and reorder pixfmt files
  media/doc: improve bt.2020 documentation
  media/doc: improve the SMPTE 2084 documentation
  v4l2-tpg: fix the SMPTE-2084 transfer function
  media/extended-controls.rst: fix wrong enum names
  v4l2-tpg-core.c: fix typo in bt2020_full matrix

 .../media/uapi/v4l/{pixfmt-006.rst => colorspaces-defs.rst}  |   2 +-
 .../media/uapi/v4l/{pixfmt-007.rst => colorspaces-details.rst}   |  47 
++
 Documentation/media/uapi/v4l/extended-controls.rst   |  26 
+++---
 Documentation/media/uapi/v4l/pixfmt-008.rst  |  32 
---
 .../media/uapi/v4l/{pixfmt-013.rst => pixfmt-compressed.rst} |   0
 Documentation/media/uapi/v4l/{pixfmt-004.rst => pixfmt-intro.rst}|   0
 .../media/uapi/v4l/{pixfmt-003.rst => pixfmt-v4l2-mplane.rst}|   0
 Documentation/media/uapi/v4l/{pixfmt-002.rst => pixfmt-v4l2.rst} |   0
 Documentation/media/uapi/v4l/pixfmt.rst  |  15 ++-
 drivers/media/common/v4l2-tpg/v4l2-tpg-colors.c  | 154 
---
 drivers/media/common/v4l2-tpg/v4l2-tpg-core.c|   2 +-
 11 files changed, 151 insertions(+), 127 deletions(-)
 rename Documentation/media/uapi/v4l/{pixfmt-006.rst => colorspaces-defs.rst} 
(98%)
 rename Documentation/media/uapi/v4l/{pixfmt-007.rst => 
colorspaces-details.rst} (92%)
 delete mode 100644 Documentation/media/uapi/v4l/pixfmt-008.rst
 rename Documentation/media/uapi/v4l/{pixfmt-013.rst => pixfmt-compressed.rst} 
(100%)
 rename Documentation/media/uapi/v4l/{pixfmt-004.rst => pixfmt-intro.rst} (100%)
 rename Documentation/media/uapi/v4l/{pixfmt-003.rst => pixfmt-v4l2-mplane.rst} 
(100%)
 rename Documentation/media/uapi/v4l/{pixfmt-002.rst => pixfmt-v4l2.rst} (100%)

[GIT PULL FOR v4.14] v2: Use LINUX_VERSION_CODE for media versioning

[Hans Verkuil]

Just a little thing that always annoyed me: the driver_version shouldn't
be set in drivers.

The version number never, ever gets updated in drivers. We saw that in
the other media subsystems and now the core always sets it, not drivers.

This works much better, and also works well when backporting the media
code to an older kernel using the media_build system, where the driver
version is set to the kernel version you are backporting from.

So just set the driver_version in media_device_get_info() to
LINUX_VERSION_CODE and drop the driver_version field from struct
media_device.

In addition do the same with media_version, that too is never updated
when it should.

Regards,

Hans

Changes since v1:

- Dropped a change to Documentation/media/uapi/v4l/extended-controls.rst
  that accidentally ended up in the previous pull request.

The following changes since commit da48c948c263c9d87dfc64566b3373a858cc8aa2:

  media: fix warning on v4l2_subdev_call() result interpreted as bool 
(2017-07-26 13:43:17 -0400)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git mc-version

for you to fetch changes up to 590a9c978c0fd8a58580e58245f1187d676a6f54:

  media: drop use of MEDIA_API_VERSION (2017-07-28 09:16:51 +0200)


Hans Verkuil (6):
  media-device: set driver_version directly
  s3c-camif: don't set driver_version
  uvc: don't set driver_version
  atomisp2: don't set driver_version
  media-device: remove driver_version
  media: drop use of MEDIA_API_VERSION

 Documentation/media/uapi/v4l/extended-controls.rst| 26 
+-
 drivers/media/media-device.c  |  7 ++-
 drivers/media/platform/s3c-camif/camif-core.c |  1 -
 drivers/media/usb/uvc/uvc_driver.c|  1 -
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c |  6 +-
 include/media/media-device.h  |  7 ---
 include/uapi/linux/media.h|  5 +++--
 7 files changed, 19 insertions(+), 34 deletions(-)

[GIT PULL FOR v4.14] Use LINUX_VERSION_CODE for media versioning

[Hans Verkuil]

Just a little thing that always annoyed me: the driver_version shouldn't
be set in drivers.

The version number never, ever gets updated in drivers. We saw that in
the other media subsystems and now the core always sets it, not drivers.

This works much better, and also works well when backporting the media
code to an older kernel using the media_build system, where the driver
version is set to the kernel version you are backporting from.

So just set the driver_version in media_device_get_info() to
LINUX_VERSION_CODE and drop the driver_version field from struct
media_device.

In addition do the same with media_version, that too is never updated
when it should.

Regards,

Hans

The following changes since commit da48c948c263c9d87dfc64566b3373a858cc8aa2:

  media: fix warning on v4l2_subdev_call() result interpreted as bool 
(2017-07-26 13:43:17 -0400)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git mc-version

for you to fetch changes up to 590a9c978c0fd8a58580e58245f1187d676a6f54:

  media: drop use of MEDIA_API_VERSION (2017-07-28 09:16:51 +0200)


Hans Verkuil (6):
  media-device: set driver_version directly
  s3c-camif: don't set driver_version
  uvc: don't set driver_version
  atomisp2: don't set driver_version
  media-device: remove driver_version
  media: drop use of MEDIA_API_VERSION

 Documentation/media/uapi/v4l/extended-controls.rst| 26 
+-
 drivers/media/media-device.c  |  7 ++-
 drivers/media/platform/s3c-camif/camif-core.c |  1 -
 drivers/media/usb/uvc/uvc_driver.c|  1 -
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c |  6 +-
 include/media/media-device.h  |  7 ---
 include/uapi/linux/media.h|  5 +++--
 7 files changed, 19 insertions(+), 34 deletions(-)

Re: pull request: linux-firmware: Update Mediatek MT8173 VPU firmware

[andrew-ct chen]

Hi linux-firmware maintainers,

Sorry, problems with this release firmware.
We will release v1.0.6 in a few days. Thanks a lot.

Andrew

On Thu, 2017-07-20 at 15:59 +0800, Andrew-CT Chen wrote:
> Hi linux-firmware maintainers,
> 
> The following changes since commit 7d2c913dcd1be083350d97a8cb1eba24cfacbc8a:
> 
>   ath10k: update year in license (2017-06-22 12:06:02 -0700)
> 
> are available in the git repository at:
> 
>   https://github.com/andrewct-chen/linux_fw_vpu_v1.0.5.git v1.0.5
> 
> for you to fetch changes up to 0d4ab52ba9be6f619024d7a57b1af05a03daa099:
> 
>   mediatek: update MT8173 VPU firmware to 1.0.5 (2017-07-20 15:27:05 +0800)
> 
> 
> Andrew-CT Chen (1):
>   mediatek: update MT8173 VPU firmware to 1.0.5
> 
>  vpu_d.bin | Bin 4082928 -> 2977008 bytes
>  vpu_p.bin | Bin 131160 -> 131324 bytes
>  2 files changed, 0 insertions(+), 0 deletions(-)
> 
> 
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek