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:   Tue Sep  1 04:00:24 CEST 2015
git branch: test
git hash:   d071c833a0d30e7aae0ea565d92ef83c79106d6f
gcc version:i686-linux-gcc (GCC) 5.1.0
sparse version: v0.5.0-51-ga53cea2
smatch version: 0.4.1-3153-g7d56ab3
host hardware:  x86_64
host os:4.0.0-3.slh.1-amd64

linux-git-Module.symvers: ERRORS
linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: 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: WARNINGS
linux-2.6.32.27-i686: OK
linux-2.6.33.7-i686: OK
linux-2.6.34.7-i686: OK
linux-2.6.35.9-i686: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-i686: OK
linux-2.6.32.27-x86_64: OK
linux-2.6.33.7-x86_64: OK
linux-2.6.34.7-x86_64: OK
linux-2.6.35.9-x86_64: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: ERRORS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 16/55] [media] media: Don't accept early-created links

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 16:39:34 +0200
Javier Martinez Canillas  escreveu:

> Hello Hans,
> 
> On 08/31/2015 01:01 PM, Hans Verkuil wrote:
> > On 08/31/2015 12:54 PM, Mauro Carvalho Chehab wrote:
> >> Em Mon, 31 Aug 2015 12:30:16 +0200
> >> Hans Verkuil  escreveu:
> >>
> >>> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
>  Links are graph objects that represent the links of two already
>  existing objects in the graph.
> 
>  While with the current implementation, it is possible to create
>  the links earlier, It doesn't make any sense to allow linking
>  two objects when they are not both created.
> 
>  So, remove the code that would be handling those early-created
>  links and add a BUG_ON() to ensure that.
> 
>  Signed-off-by: Mauro Carvalho Chehab 
> >>>
> >>> The code is OK, so:
> >>>
> >>> Acked-by: Hans Verkuil 
> >>>
> >>> But shouldn't this go *after* the omap3isp fixes? After this patch the
> >>> omap3isp will call BUG_ON, and that's not what you want.
> >>
> >> Yes. I'll change the order on my git tree.
> >>
> >>> It is also not clear if the omap3isp driver is the only one that has this
> >>> 'create link before objects' problem. I would expect that the omap4 
> >>> staging
> >>> driver has the same issue and possibly others as well.
> >>>
> >>> Did someone look at that?
> >>
> >> I guess other drivers are doing the same.
> >>
> >> Javier's planning to review the other platform drivers in order to add the 
> >> needed fixes there too, and to do more tests with some other platform
> >> drivers that he has hardware for testing.
> > 
> > OK, good. Just wanted to know that.
> >
> 
> Yes, the omap4iss driver in staging has a similar logic and thus the
> same issues than the omap3isp driver. I'll write patches for this as
> well but I don't have an OMAP4 board here so testing is appreciated.

Well, I can try to test it here on my PandaBoard. Not sure if it will work,
as I think that the sensor connector was not properly soldered.

> 
> As Mauro mentioned, I was reviewing the others driver that are creating
> pad links and found more that are creating links before registering:
> 
> drivers/media/usb/uvc
> drivers/media/platform/vsp1
> drivers/media/i2c/smiapp
> 
> I'll try to take care of these too.
> 
> The other drivers that are creating pad links are doing the right thing
> AFAICT by registering the entity with the media device before.
> 
> > Perhaps it is a good idea to add a TODO list to the cover letter. This would
> > be one item on that list.
> >
> 
> A TODO list is a good idea indeed.
>  
> > Regards,
> > 
> > Hans
> > 
> 
> Best regards,
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios

[Rob Herring]

On Fri, Aug 28, 2015 at 12:52 PM, Peter Griffin
 wrote:
> gpio.txt documents that GPIO properties should be named
> "[-]gpios", with  being the purpose of this
> GPIO for the device.
>
> This change has been done as one atomic commit.

dtb and kernel updates are not necessarily atomic, so you are breaking
compatibility with older dtbs. You should certainly highlight that in
the commit message. I only point this out. I'll leave it to platform
maintainers whether or not this breakage is acceptable.

Rob

>
> Signed-off-by: Peter Griffin 
> Acked-by: Lee Jones 
> ---
>  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
>  arch/arm/boot/dts/stihxxx-b2120.dtsi  | 4 ++--
>  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt 
> b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> index d4def76..e70d840 100644
> --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
>
>  - tsin-num : tsin id of the InputBlock (must be between 0 to 6)
>  - i2c-bus  : phandle to the I2C bus DT node which the demodulators & 
> tuners on this tsin channel are connected.
> -- rst-gpio : reset gpio for this tsin channel.
> +- reset-gpios  : reset gpio for this tsin channel.
>
>  Optional properties (tsin (child) node):
>
> @@ -75,7 +75,7 @@ Example:
> tsin-num= <0>;
> serial-not-parallel;
> i2c-bus = <>;
> -   rst-gpio= < 4 0>;
> +   reset-gpios = < 4 GPIO_ACTIVE_HIGH>;
> dvb-card= ;
> };
>
> @@ -83,7 +83,7 @@ Example:
> tsin-num= <3>;
> serial-not-parallel;
> i2c-bus = <>;
> -   rst-gpio= < 7 0>;
> +   reset-gpios = < 7 GPIO_ACTIVE_HIGH>;
> dvb-card= ;
> };
> };
> diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi 
> b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> index f9fca10..0b7592e 100644
> --- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
> +++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> @@ -6,8 +6,8 @@
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
>   */
> -
>  #include 
> +#include 
>  #include 
>  / {
> soc {
> @@ -116,7 +116,7 @@
> tsin-num= <0>;
> serial-not-parallel;
> i2c-bus = <>;
> -   rst-gpio= < 4 GPIO_ACTIVE_HIGH>;
> +   reset-gpios = < 4 GPIO_ACTIVE_HIGH>;
> dvb-card= ;
> };
> };
> diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c 
> b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
> index 3a91093..c691e13 100644
> --- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
> +++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
> @@ -822,7 +822,7 @@ static int c8sectpfe_probe(struct platform_device *pdev)
> }
> of_node_put(i2c_bus);
>
> -   tsin->rst_gpio = of_get_named_gpio(child, "rst-gpio", 0);
> +   tsin->rst_gpio = of_get_named_gpio(child, "reset-gpios", 0);
>
> ret = gpio_is_valid(tsin->rst_gpio);
> if (!ret) {
> --
> 1.9.1
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: New Terratec Cinergy S2 Box usb-id

[Johann Klammer]

On 08/31/2015 01:19 PM, Maximilian Imgrund wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA256
> 
> Dear all,
> 
> I am currently figuring out how to get the Terratec Cinergy S2 USB Box
> up and running. I already modified a patch to  previous version (see
> attachment) to include the new ID in the device driver, module is also
> loading with the ds3000 firmware. However, when using w_scan, the
> reported frequency range is .95GHz ... 2.15Ghz which is roughly a
> factor of 10 lower than I expected (Astra is 12.515Ghz e.g.). Since
> the tuner seems to tune in correctly but in the wrong frequency range,
> I feel that is the reason for me not getting in any channels.
> 
> Can you help me with what to change in the driver to get this working
> ? I feel like an additional .frequency_div should do the job, however
> I am unable to find further informaion on that...
> 
> Best
> Maximilian Imgrund
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v2
> 
> iF4EAREIAAYFAlXkOFIACgkQR/X5cR0fI/6sfAD+OVauTyLw0oWSMr8ONzmrguF+
> Ci/vg4uO9mxZwzjgGXkA/ipgQ/IuX+8n2CSScHg6CFjt9tIBbFOAVzStuUrOpwx2
> =AAXS
> -END PGP SIGNATURE-
> 

The driver has to tune to the intermediate frequency 
that's output by the LNB. (There is a local oscillator+downmixer in there...)

This is what the one here has

info = {
  name = "ST STV0299 DVB-S", '\000' , type = FE_QPSK, 
  frequency_min = 95, frequency_max = 215, 
  frequency_stepsize = 125, frequency_tolerance = 0, 
  symbol_rate_min = 100, symbol_rate_max = 4500, 
  symbol_rate_tolerance = 500, notifier_delay = 0, 
  caps = (FE_CAN_INVERSION_AUTO | FE_CAN_FEC_1_2 | FE_CAN_FEC_2_3 | 
FE_CAN_FEC_3_4 | FE_CAN_FEC_5_6 | FE_CAN_FEC_7_8 | FE_CAN_FEC_AUTO | 
FE_CAN_QPSK)}, 

Frequency_min and max match yours. 

Try a different scan tool, and make sure you get a signal out of your LNB. 
Proper DISH ALIGNMENT is important, 
and don't forget a grounding bloc. 


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: dvb-core: Don't force CAN_INVERSION_AUTO in oneshot mode.

[Johann Klammer]

Why not just remove the line?
info->caps |= FE_CAN_INVERSION_AUTO;

The capabilities call interacting with the oneshot setting is rather weird and 
maybe unexpected.


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: dvb-core: Don't force CAN_INVERSION_AUTO in oneshot mode.

[Malcolm Priestley]

On 31/08/15 18:03, Johann Klammer wrote:


Why not just remove the line?
info->caps |= FE_CAN_INVERSION_AUTO;

The capabilities call interacting with the oneshot setting is rather weird and 
maybe unexpected.




No, because in normal mode it can do auto inversion.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH TRY 2] Support for EVOLVEO XtraTV stick

[CIJOML CIJOMLovic]

Hello guys,

please find out down this email patch to support EVOLVEO XtraTV stick.
This tuner is for android phones with microusb connecter, however with
reduction it works perfectly with linux kernel:
The device identify itself at USB bus as Bus 002 Device 004: ID
1f4d:a115 G-Tek Electronics Group
so I have created new vendor group but device named as its commercial name.

Thank you for merging this patch to upstream

Best regards

Michal


diff -urN media_build/linux/drivers/media/dvb-core/dvb-usb-ids.h
media_build.new/linux/drivers/media/dvb-core/dvb-usb-ids.h
--- media_build/linux/drivers/media/dvb-core/dvb-usb-ids.h
2015-05-11 13:20:08.0 +0200
+++ media_build.new/linux/drivers/media/dvb-core/dvb-usb-ids.h
2015-06-16 22:26:01.917990493 +0200
@@ -70,6 +70,8 @@
 #define USB_VID_EVOLUTEPC0x1e59
 #define USB_VID_AZUREWAVE0x13d3
 #define USB_VID_TECHNISAT0x14f7
+#define USB_VID_GTEK0x1f4d
+

 /* Product IDs */
 #define USB_PID_ADSTECH_USB2_COLD0xa333
@@ -388,4 +390,5 @@
 #define USB_PID_PCTV_2002E_SE   0x025d
 #define USB_PID_SVEON_STV27 0xd3af
 #define USB_PID_TURBOX_DTT_2000 0xd3a4
+#define USB_PID_EVOLVEO_XTRATV_STICK0xa115
 #endif
diff -urN media_build/linux/drivers/media/usb/dvb-usb-v2/af9035.c
media_build.new/linux/drivers/media/usb/dvb-usb-v2/af9035.c
--- media_build/linux/drivers/media/usb/dvb-usb-v2/af9035.c
2015-05-30 17:32:46.0 +0200
+++ media_build.new/linux/drivers/media/usb/dvb-usb-v2/af9035.c
2015-06-16 22:26:14.561990868 +0200
@@ -2075,6 +2075,8 @@
 _props, "PCTV AndroiDTV (78e)", RC_MAP_IT913X_V1) },
 { DVB_USB_DEVICE(USB_VID_PCTV, USB_PID_PCTV_79E,
 _props, "PCTV microStick (79e)", RC_MAP_IT913X_V2) },
+{ DVB_USB_DEVICE(USB_VID_GTEK, USB_PID_EVOLVEO_XTRATV_STICK,
+_props, "EVOLVEO XtraTV stick", RC_MAP_IT913X_V2) },

 /* IT930x devices */
 { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
r
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[linux-next:master 2531/11785] videobuf2-core.c:undefined reference to `__tracepoint_vb2_buf_queue'

[kbuild test robot]

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   4da7932545f1474564af3b2558b738c7a15ed853
commit: 2091f5181c66b3617a977e79843aba10e087be6c [2531/11785] [media] 
videobuf2: add trace events
config: i386-randconfig-sb0-08312345 (attached as .config)
reproduce:
  git checkout 2091f5181c66b3617a977e79843aba10e087be6c
  # save the attached .config to linux build tree
  make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   drivers/built-in.o: In function `sur40_disconnect':
   sur40.c:(.text+0x15e496): undefined reference to `video_unregister_device'
   sur40.c:(.text+0x15e49e): undefined reference to `v4l2_device_unregister'
   drivers/built-in.o: In function `sur40_process_video':
   sur40.c:(.text+0x15e805): undefined reference to `v4l2_get_timestamp'
   drivers/built-in.o: In function `sur40_probe':
   sur40.c:(.text+0x15f05f): undefined reference to `v4l2_device_register'
   sur40.c:(.text+0x15f0e7): undefined reference to `v4l2_device_unregister'
   sur40.c:(.text+0x15f1e7): undefined reference to `video_device_release_empty'
   sur40.c:(.text+0x15f21c): undefined reference to `__video_register_device'
   sur40.c:(.text+0x15f260): undefined reference to `video_unregister_device'
   drivers/built-in.o: In function `sur40_vidioc_querycap':
   sur40.c:(.text+0x15f280): undefined reference to `video_devdata'
   drivers/built-in.o: In function `vb2_ioctl_expbuf':
   (.text+0x164da0): undefined reference to `video_devdata'
   drivers/built-in.o: In function `vb2_fop_mmap':
   (.text+0x164dec): undefined reference to `video_devdata'
   drivers/built-in.o: In function `vb2_ioctl_querybuf':
   (.text+0x1657cc): undefined reference to `video_devdata'
   drivers/built-in.o: In function `vb2_ioctl_create_bufs':
   (.text+0x1666e0): undefined reference to `video_devdata'
   drivers/built-in.o:(.text+0x1676f0): more undefined references to 
`video_devdata' follow
   drivers/built-in.o: In function `__enqueue_in_driver':
>> videobuf2-core.c:(.text+0x16775b): undefined reference to 
>> `__tracepoint_vb2_buf_queue'
   videobuf2-core.c:(.text+0x167784): undefined reference to 
`__tracepoint_vb2_buf_queue'
   videobuf2-core.c:(.text+0x1677c2): undefined reference to 
`__tracepoint_vb2_buf_queue'
   drivers/built-in.o: In function `vb2_buffer_done':
>> (.text+0x167a68): undefined reference to `__tracepoint_vb2_buf_done'
   drivers/built-in.o: In function `vb2_buffer_done':
   (.text+0x167a91): undefined reference to `__tracepoint_vb2_buf_done'
   drivers/built-in.o: In function `vb2_buffer_done':
   (.text+0x167ad2): undefined reference to `__tracepoint_vb2_buf_done'
   drivers/built-in.o: In function `vb2_ioctl_reqbufs':
   (.text+0x168243): undefined reference to `video_devdata'
   drivers/built-in.o: In function `vb2_ioctl_streamoff':
   (.text+0x1683e0): undefined reference to `video_devdata'
   drivers/built-in.o: In function `_vb2_fop_release':
   (.text+0x168560): undefined reference to `video_devdata'
   drivers/built-in.o: In function `_vb2_fop_release':
   (.text+0x16858e): undefined reference to `v4l2_fh_release'
   drivers/built-in.o: In function `vb2_fop_release':
   (.text+0x1685ec): undefined reference to `video_devdata'
   drivers/built-in.o: In function `vb2_ioctl_streamon':
   (.text+0x1689c0): undefined reference to `video_devdata'
   drivers/built-in.o: In function `vb2_internal_qbuf':
>> videobuf2-core.c:(.text+0x168aac): undefined reference to 
>> `__tracepoint_vb2_qbuf'
   videobuf2-core.c:(.text+0x168ad9): undefined reference to 
`__tracepoint_vb2_qbuf'
   videobuf2-core.c:(.text+0x168b7d): undefined reference to 
`__tracepoint_vb2_qbuf'
   drivers/built-in.o: In function `vb2_ioctl_qbuf':
   (.text+0x168e30): undefined reference to `video_devdata'
   drivers/built-in.o: In function `vb2_poll':
   (.text+0x169298): undefined reference to `video_devdata'
   drivers/built-in.o: In function `vb2_poll':
   (.text+0x16948c): undefined reference to `v4l2_event_pending'
   drivers/built-in.o: In function `vb2_fop_poll':
   (.text+0x169605): undefined reference to `video_devdata'
   drivers/built-in.o: In function `vb2_internal_dqbuf':
>> videobuf2-core.c:(.text+0x169a72): undefined reference to 
>> `__tracepoint_vb2_dqbuf'
   videobuf2-core.c:(.text+0x169aa2): undefined reference to 
`__tracepoint_vb2_dqbuf'
   videobuf2-core.c:(.text+0x169af5): undefined reference to 
`__tracepoint_vb2_dqbuf'
   drivers/built-in.o: In function `vb2_ioctl_dqbuf':
   (.text+0x169fb0): undefined reference to `video_devdata'
   drivers/built-in.o: In function `__vb2_perform_fileio':
   videobuf2-core.c:(.text+0x16a47f): undefined reference to 
`v4l2_get_timestamp'
   drivers/built-in.o: In function `vb2_fop_write':
   (.text+0x16a617): undefined reference to `video_devdata'
   drivers/built-in.o: In function `vb2_fop_read':
   (.text+0x16a717): undefined reference to `video_devdata'
   drivers/built-in.o: In function `vb2_thread':
   videobuf2-core.c:(.text+0x16aa14): 

Re: [PATCH] media: dvb-core: Don't force CAN_INVERSION_AUTO in oneshot mode.

[Devin Heitmueller]

Hi Malcolm,

>> The capabilities call interacting with the oneshot setting is rather weird
>> and maybe unexpected.
>>
>>
>
> No, because in normal mode it can do auto inversion.

This isn't my area of expertise, but I suspect this is going to cause
some pretty confusing behavior.  Generally speaking device
capabilities are queried right after the frontend is opened, and the
frontend capabilities typically don't ever change.  In this case, an
application would have to know to check the capabilities a second time
after calling FE_SET_FRONTEND_TUNE_MODE in order to determine whether
auto inversion *really* is available.

If the goal was for the software-emulated auto inversion to be
transparent to userland, perhaps it makes more sense for the oneshot
mode to toggle the inversion if needed.  The oneshot mode would
continue to disable zigzag and the stats monitoring.  I realize that
this is a bit messy since it won't really be "oneshot", but I don't
know what else can be done without breaking the ABI.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH TRY 2] Support for EVOLVEO XtraTV stick

[Antti Palosaari]

On 08/31/2015 09:04 PM, CIJOML CIJOMLovic wrote:

Hello guys,

please find out down this email patch to support EVOLVEO XtraTV stick.
This tuner is for android phones with microusb connecter, however with
reduction it works perfectly with linux kernel:
The device identify itself at USB bus as Bus 002 Device 004: ID
1f4d:a115 G-Tek Electronics Group
so I have created new vendor group but device named as its commercial name.

Thank you for merging this patch to upstream


VID for GTEK is already defined there.

Could you remove also remote controller default keymap as I think there 
is no remote controller at all.


regards
Antti



Best regards

Michal


diff -urN media_build/linux/drivers/media/dvb-core/dvb-usb-ids.h
media_build.new/linux/drivers/media/dvb-core/dvb-usb-ids.h
--- media_build/linux/drivers/media/dvb-core/dvb-usb-ids.h
2015-05-11 13:20:08.0 +0200
+++ media_build.new/linux/drivers/media/dvb-core/dvb-usb-ids.h
2015-06-16 22:26:01.917990493 +0200
@@ -70,6 +70,8 @@
  #define USB_VID_EVOLUTEPC0x1e59
  #define USB_VID_AZUREWAVE0x13d3
  #define USB_VID_TECHNISAT0x14f7
+#define USB_VID_GTEK0x1f4d
+

  /* Product IDs */
  #define USB_PID_ADSTECH_USB2_COLD0xa333
@@ -388,4 +390,5 @@
  #define USB_PID_PCTV_2002E_SE   0x025d
  #define USB_PID_SVEON_STV27 0xd3af
  #define USB_PID_TURBOX_DTT_2000 0xd3a4
+#define USB_PID_EVOLVEO_XTRATV_STICK0xa115
  #endif
diff -urN media_build/linux/drivers/media/usb/dvb-usb-v2/af9035.c
media_build.new/linux/drivers/media/usb/dvb-usb-v2/af9035.c
--- media_build/linux/drivers/media/usb/dvb-usb-v2/af9035.c
2015-05-30 17:32:46.0 +0200
+++ media_build.new/linux/drivers/media/usb/dvb-usb-v2/af9035.c
2015-06-16 22:26:14.561990868 +0200
@@ -2075,6 +2075,8 @@
  _props, "PCTV AndroiDTV (78e)", RC_MAP_IT913X_V1) },
  { DVB_USB_DEVICE(USB_VID_PCTV, USB_PID_PCTV_79E,
  _props, "PCTV microStick (79e)", RC_MAP_IT913X_V2) },
+{ DVB_USB_DEVICE(USB_VID_GTEK, USB_PID_EVOLVEO_XTRATV_STICK,
+_props, "EVOLVEO XtraTV stick", RC_MAP_IT913X_V2) },

  /* IT930x devices */
  { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
r
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: dvb-core: Don't force CAN_INVERSION_AUTO in oneshot mode.

[Malcolm Priestley]

Hi Devin

On 31/08/15 19:07, Devin Heitmueller wrote:

Hi Malcolm,


The capabilities call interacting with the oneshot setting is rather weird
and maybe unexpected.




No, because in normal mode it can do auto inversion.

...


If the goal was for the software-emulated auto inversion to be
transparent to userland, perhaps it makes more sense for the oneshot
mode to toggle the inversion if needed.  The oneshot mode would
continue to disable zigzag and the stats monitoring.  I realize that
this is a bit messy since it won't really be "oneshot", but I don't
know what else can be done without breaking the ABI.


I did think flagging INVERSION_AUTO to INVERSION_OFF on frontends
not supporting inversion in oneshot mode.

But it's still messy, as INVERSION_AUTO is need for emulation to work.

Regards


Malcolm
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Do you need a loan?

We offer private, commercial and personal loans with very low annualinterest 
rates as low as 2% in one year to 50 years repayment period anywhere in the 
world. We offer loans ranging from $5000 to $100 million. Our loans are well 
insured for maximum security is our priority. Are you losing sleep at night 
worrying how to get a legitimate loan lender? You bite your nails that fast? 
Instead of beating you, contact JM Financial Services Ltd now, specialists who 
help stop loans bad credit history to find a solution that victory is our 
mission. Applicants must fill out a loan application form below:

FORM credit application

loan applications
Your full name *
Your e-mail *
Your phone *
Your address *
Your city *
State / Province *
Country *
Fax*
Date of birth *
Do you have an account? *
Have you applied before? *
The loan amount is needed *
The Loan Duration*
The life expectancy *
The purpose of the loan *
Send me a scanned copy of your passport: *

Creditor: Mr. Prakash Lass Dickson.
Company: JM Financial Services Ltd.
Copyright JM Financial Ltd. All rights reserved.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

tvtime cannot set PAL-DK for cx88 device

[Paweł Kiełkowski]

Hello,

Last week I made a big jump from Fedora 16 to Fedora 22 and tvtime is no
longer able to set the PAL-DK norm. What I am getting is: "videoinput:
Driver refuses to set norm: Device or resource busy". There is no such
message when running tvtime on Fedora 16.
I presume that following change introduced this check.
http://git.linuxtv.org/cgit.cgi/media_tree.git/commit/?id=078859a3230c123ed9cb798fb1cd7f89b4fde102

Fedora 16 kernel: 3.6.11-4.fc16.i686.PAE
Fedora 22 kernel: 4.1.6-200.fc22.x86_64
tvtime 1.0.7 from git on both systems
TV card: 01:09.0 Multimedia video controller [0400]: Conexant Systems,
Inc. CX23880/1/2/3 PCI Video and Audio Decoder [14f1:8800] (rev 05)
Subsystem: LeadTek Research Inc. Winfast TV 2000XP Expert
[107d:6611]
Flags: bus master, medium devsel, latency 32, IRQ 19
Memory at f900 (32-bit, non-prefetchable) [size=16M]
Capabilities: [44] Vital Product Data
Capabilities: [4c] Power Management version 2
Kernel driver in use: cx8800
Kernel modules: cx8800

Best regards
PK
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rcar_vin: propagate querystd() error upstream

[Sergei Shtylyov]

Hello.

On 08/25/2015 05:25 PM, Hans Verkuil wrote:


rcar_vin_set_fmt() defaults to  PAL when the subdevice's querystd() method call
fails (e.g. due to I2C error).  This doesn't work very well when a camera being
used  outputs NTSC which has different order of fields and resolution.  Let  us
stop  pretending and return the actual error (which would prevent video capture
on at least Renesas Henninger/Porter board where I2C seems particularly buggy).

Signed-off-by: Sergei Shtylyov 

---
The patch is against the 'media_tree.git' repo's 'fixes' branch.

  drivers/media/platform/soc_camera/rcar_vin.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
===
--- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
+++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
@@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
/* Query for standard if not explicitly mentioned _TB/_BT */
ret = v4l2_subdev_call(sd, video, querystd, );



Ouch, this should never be done like this.


   Too late. :-)


Instead the decision should be made using the last set std, never by querying.
So querystd should be replaced by g_std in the v4l2_subdev_call above.


   Hm, then this code will stop working, as adv7180.c and ml86v7667.c we use 
don't support the g_std() method yet...



The only place querystd can be called is in the QUERYSTD ioctl, all other
ioctls should use the last set standard.


   OK, I'll have to fix all the drivers involved...


Regards,
Hans


MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Linux 4.2 ALSA snd-usb-audio inconsistent lock state warn in PCM nonatomic mode

[Shuah Khan]

Hi Takashi,

I am seeing the following inconsistent lock state warning when PCM
is run in nonatomic mode. This is on 4.2.0 and with the following
change to force PCM on nonatomic mode:

diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index 310a382..16bbb71 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -370,6 +370,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
pcm->private_data = as;
pcm->private_free = snd_usb_audio_pcm_free;
pcm->info_flags = 0;
+   pcm->nonatomic = true;
if (chip->pcm_devs > 0)
sprintf(pcm->name, "USB Audio #%d", chip->pcm_devs);
else

The device Bus 003 Device 002: ID 2040:7200 Hauppauge
Please let me know if you need more information and any ideas on
how to fix this problem.

thanks,
-- Shuah

[  120.283960] =
[  120.283964] [ INFO: inconsistent lock state ]
[  120.283968] 4.2.0+ #29 Not tainted
[  120.283972] -
[  120.283975] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
[  120.283980] swapper/1/0 [HC1[1]:SC0[0]:HE0:SE1] takes:
[  120.283983]  (&(>lock)->rlock){?.+...}, at:
[] retire_capture_urb+0x140/0x2b0 [snd_usb_audio]
[  120.284005] {HARDIRQ-ON-W} state was registered at:
[  120.284008]   [] __lock_acquire+0xc50/0x2380
[  120.284016]   [] lock_acquire+0xb1/0x130
[  120.284022]   [] _raw_spin_lock+0x31/0x40
[  120.284028]   [] snd_usb_pcm_pointer+0x5d/0xc0
[snd_usb_audio]
[  120.284040]   [] snd_pcm_update_hw_ptr0+0x38/0x3a0
[snd_pcm]
[  120.284052]   [] snd_pcm_update_hw_ptr+0x10/0x20
[snd_pcm]
[  120.284063]   [] snd_pcm_hwsync+0x45/0xa0 [snd_pcm]
[  120.284071]   [] snd_pcm_common_ioctl1+0x277/0xce0
[snd_pcm]
[  120.284081]   [] snd_pcm_capture_ioctl1+0x1be/0x2d0
[snd_pcm]
[  120.284090]   [] snd_pcm_capture_ioctl+0x34/0x40
[snd_pcm]
[  120.284100]   [] do_vfs_ioctl+0x301/0x560
[  120.284107]   [] SyS_ioctl+0x79/0x90
[  120.284112]   [] entry_SYSCALL_64_fastpath+0x12/0x6f
[  120.284119] irq event stamp: 823304
[  120.284122] hardirqs last  enabled at (823301): []
cpuidle_enter_state+0xed/0x230
[  120.284129] hardirqs last disabled at (823302): []
common_interrupt+0x68/0x6d
[  120.284135] softirqs last  enabled at (823304): []
_local_bh_enable+0x21/0x50
[  120.284139] softirqs last disabled at (823303): []
irq_enter+0x4c/0x70
[  120.284143]
other info that might help us debug this:
[  120.284146]  Possible unsafe locking scenario:

[  120.284149]CPU0
[  120.284150]
[  120.284152]   lock(&(>lock)->rlock);
[  120.284155]   
[  120.284157] lock(&(>lock)->rlock);
[  120.284160]
 *** DEADLOCK ***
[  120.284163] no locks held by swapper/1/0.
[  120.284165]
stack backtrace:
[  120.284170] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.2.0+ #29
[  120.284173] Hardware name: Hewlett-Packard HP ProBook 6475b/180F,
BIOS 68TTU Ver. F.04 08/03/2012
[  120.284176]  828d5630 88023ec83a68 817f4ea0
0007
[  120.284181]  880235a6a500 88023ec83ac8 810ad97f

[  120.284186]   88020001 810134df
827c5390
[  120.284192] Call Trace:
[  120.284194][] dump_stack+0x45/0x57
[  120.284203]  [] print_usage_bug+0x1ff/0x210
[  120.284209]  [] ? save_stack_trace+0x2f/0x50
[  120.284214]  [] mark_lock+0x66e/0x6f0
[  120.284218]  [] ?
print_shortest_lock_dependencies+0x1d0/0x1d0
[  120.284222]  [] __lock_acquire+0xdcb/0x2380
[  120.284226]  [] ? __enqueue_entity+0x6c/0x70
[  120.284230]  [] ? __lock_is_held+0x4d/0x70
[  120.284234]  [] ? __lock_is_held+0x4d/0x70
[  120.284238]  [] ? __lock_is_held+0x4d/0x70
[  120.284242]  [] ? __lock_is_held+0x4d/0x70
[  120.284246]  [] lock_acquire+0xb1/0x130
[  120.284256]  [] ? retire_capture_urb+0x140/0x2b0
[snd_usb_audio]
[  120.284261]  [] _raw_spin_lock_irqsave+0x3c/0x50
[  120.284270]  [] ? retire_capture_urb+0x140/0x2b0
[snd_usb_audio]
[  120.284276]  [] ? usb_hcd_get_frame_number+0x25/0x30
[  120.284285]  [] retire_capture_urb+0x140/0x2b0
[snd_usb_audio]
[  120.284294]  [] snd_complete_urb+0x13c/0x250
[snd_usb_audio]
[  120.284298]  [] __usb_hcd_giveback_urb+0x72/0x110
[  120.284303]  [] usb_hcd_giveback_urb+0x43/0x140
[  120.284307]  [] xhci_irq+0xd42/0x1fc0
[  120.284312]  [] xhci_msi_irq+0x11/0x20
[  120.284317]  [] handle_irq_event_percpu+0x80/0x1a0
[  120.284322]  [] handle_irq_event+0x4a/0x70
[  120.284325]  [] ? handle_edge_irq+0x24/0x150
[  120.284329]  [] handle_edge_irq+0x81/0x150
[  120.284333]  [] handle_irq+0x25/0x40
[  120.284337]  [] do_IRQ+0x4f/0xe0
[  120.284341]  [] common_interrupt+0x6d/0x6d
[  120.284343][] ? cpuidle_enter_state+0xf2/0x230
[  120.284351]  [] ? cpuidle_enter_state+0xed/0x230
[  120.284355]  [] cpuidle_enter+0x17/0x20
[  120.284360]  [] call_cpuidle+0x32/0x60
[  120.284364]  [] ? cpuidle_select+0x13/0x20
[  120.284369]  [] cpu_startup_entry+0x216/0x2d0
[  120.284374]  [] start_secondary+0x12d/0x150
[  125.426447] device: 'ep_84': 

Re: [PATCH v8 25/55] [media] dvbdev: add support for interfaces

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Now that the infrastruct for that is set, add support for
> interfaces.
> 
> Please notice that we're missing two links:
>   DVB FE intf-> tuner
>   DVB demux intf -> dvr
> 
> Those should be added latter, after having the entire graph
> set. With the current infrastructure, those should be added
> at dvb_create_media_graph(), but it would also require some
> extra core changes, to allow the function to enumerate the
> interfaces.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 65f59f2124b4..6bf61d42c017 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -180,14 +180,36 @@ skip:
>   return -ENFILE;
>  }
>  
> -static void dvb_register_media_device(struct dvb_device *dvbdev,
> -   int type, int minor)
> +static void dvb_create_media_entity(struct dvb_device *dvbdev,
> +int type, int minor)
>  {
>  #if defined(CONFIG_MEDIA_CONTROLLER_DVB)
>   int ret = 0, npads;
>  
> - if (!dvbdev->adapter->mdev)
> + switch (type) {
> + case DVB_DEVICE_FRONTEND:
> + npads = 2;
> + break;
> + case DVB_DEVICE_DEMUX:
> + npads = 2;
> + break;
> + case DVB_DEVICE_CA:
> + npads = 2;
> + break;
> + case DVB_DEVICE_NET:
> + /*
> +  * We should be creating entities for the MPE/ULE
> +  * decapsulation hardware (or software implementation).
> +  *
> +  * However, the number of for the MPE/ULE decaps may not be
> +  * fixed. As we don't have yet dynamic support for PADs at
> +  * the Media Controller, let's not create the decap
> +  * entities yet.
> +  */
>   return;
> + default:
> + return;
> + }
>  
>   dvbdev->entity = kzalloc(sizeof(*dvbdev->entity), GFP_KERNEL);
>   if (!dvbdev->entity)
> @@ -197,19 +219,6 @@ static void dvb_register_media_device(struct dvb_device 
> *dvbdev,
>   dvbdev->entity->info.dev.minor = minor;
>   dvbdev->entity->name = dvbdev->name;
>  
> - switch (type) {
> - case DVB_DEVICE_CA:
> - case DVB_DEVICE_DEMUX:
> - case DVB_DEVICE_FRONTEND:
> - npads = 2;
> - break;
> - case DVB_DEVICE_NET:
> - npads = 0;
> - break;
> - default:
> - npads = 1;
> - }
> -
>   if (npads) {
>   dvbdev->pads = kcalloc(npads, sizeof(*dvbdev->pads),
>  GFP_KERNEL);
> @@ -230,18 +239,11 @@ static void dvb_register_media_device(struct dvb_device 
> *dvbdev,
>   dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
>   dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
>   break;
> - case DVB_DEVICE_DVR:
> - dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DVR;
> - dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> - break;
>   case DVB_DEVICE_CA:
>   dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
>   dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
>   dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
>   break;
> - case DVB_DEVICE_NET:
> - dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_NET;
> - break;
>   default:
>   kfree(dvbdev->entity);
>   dvbdev->entity = NULL;
> @@ -263,11 +265,63 @@ static void dvb_register_media_device(struct dvb_device 
> *dvbdev,
>   return;
>   }
>  
> - printk(KERN_DEBUG "%s: media device '%s' registered.\n",
> + printk(KERN_DEBUG "%s: media entity '%s' registered.\n",
>   __func__, dvbdev->entity->name);
>  #endif
>  }
>  
> +static void dvb_register_media_device(struct dvb_device *dvbdev,
> +   int type, int minor)
> +{
> +#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
> + u32 intf_type;
> +
> + if (!dvbdev->adapter->mdev)
> + return;
> +
> + dvb_create_media_entity(dvbdev, type, minor);
> +
> + switch (type) {
> + case DVB_DEVICE_FRONTEND:
> + intf_type = MEDIA_INTF_T_DVB_FE;
> + break;
> + case DVB_DEVICE_DEMUX:
> + intf_type = MEDIA_INTF_T_DVB_DEMUX;
> + break;
> + case DVB_DEVICE_DVR:
> + intf_type = MEDIA_INTF_T_DVB_DVR;
> + break;
> + case DVB_DEVICE_CA:
> + intf_type = MEDIA_INTF_T_DVB_CA;
> + break;
> + case DVB_DEVICE_NET:
> + intf_type = MEDIA_INTF_T_DVB_NET;
> + break;
> + default:
> + return;
> + }
> +
> + dvbdev->intf_devnode = 

Re: [PATCH v8 16/55] [media] media: Don't accept early-created links

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Links are graph objects that represent the links of two already
> existing objects in the graph.
> 
> While with the current implementation, it is possible to create
> the links earlier, It doesn't make any sense to allow linking
> two objects when they are not both created.
> 
> So, remove the code that would be handling those early-created
> links and add a BUG_ON() to ensure that.
> 
> Signed-off-by: Mauro Carvalho Chehab 

The code is OK, so:

Acked-by: Hans Verkuil 

But shouldn't this go *after* the omap3isp fixes? After this patch the
omap3isp will call BUG_ON, and that's not what you want.

It is also not clear if the omap3isp driver is the only one that has this
'create link before objects' problem. I would expect that the omap4 staging
driver has the same issue and possibly others as well.

Did someone look at that?

Regards,

Hans

> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 138b18416460..0d85c6c28004 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -443,13 +443,6 @@ int __must_check media_device_register_entity(struct 
> media_device *mdev,
>   media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, >graph_obj);
>   list_add_tail(>list, >entities);
>  
> - /*
> -  * Initialize objects at the links
> -  * in the case where links got created before entity register
> -  */
> - for (i = 0; i < entity->num_links; i++)
> - media_gobj_init(mdev, MEDIA_GRAPH_LINK,
> - >links[i].graph_obj);
>   /* Initialize objects at the pads */
>   for (i = 0; i < entity->num_pads; i++)
>   media_gobj_init(mdev, MEDIA_GRAPH_PAD,
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 01946baa32d5..9f8e0145db7a 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -161,6 +161,8 @@ void media_gobj_init(struct media_device *mdev,
>  enum media_gobj_type type,
>  struct media_gobj *gobj)
>  {
> + BUG_ON(!mdev);
> +
>   gobj->mdev = mdev;
>  
>   /* Create a per-type unique object ID */
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 42/55] [media] dvb: modify core to implement interfaces/entities at MC new gen

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> The Media Controller New Generation redefines the types for both
> interfaces and entities to be used on DVB. Make the needed
> changes at the DVB core for all interfaces, entities and
> data and interface links to appear in the graph.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
> index d0e3f9d85f34..baaed28ee975 100644
> --- a/drivers/media/dvb-core/dmxdev.c
> +++ b/drivers/media/dvb-core/dmxdev.c
> @@ -1242,9 +1242,9 @@ int dvb_dmxdev_init(struct dmxdev *dmxdev, struct 
> dvb_adapter *dvb_adapter)
>   }
>  
>   dvb_register_device(dvb_adapter, >dvbdev, _demux, dmxdev,
> - DVB_DEVICE_DEMUX);
> + DVB_DEVICE_DEMUX, dmxdev->filternum);
>   dvb_register_device(dvb_adapter, >dvr_dvbdev, _dvr,
> - dmxdev, DVB_DEVICE_DVR);
> + dmxdev, DVB_DEVICE_DVR, dmxdev->filternum);
>  
>   dvb_ringbuffer_init(>dvr_buffer, NULL, 8192);
>  
> diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
> b/drivers/media/dvb-core/dvb_ca_en50221.c
> index fb66184dc9b6..f82cd1ff4f3a 100644
> --- a/drivers/media/dvb-core/dvb_ca_en50221.c
> +++ b/drivers/media/dvb-core/dvb_ca_en50221.c
> @@ -1695,7 +1695,7 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
>   pubca->private = ca;
>  
>   /* register the DVB device */
> - ret = dvb_register_device(dvb_adapter, >dvbdev, _ca, ca, 
> DVB_DEVICE_CA);
> + ret = dvb_register_device(dvb_adapter, >dvbdev, _ca, ca, 
> DVB_DEVICE_CA, 0);
>   if (ret)
>   goto free_slot_info;
>  
> diff --git a/drivers/media/dvb-core/dvb_frontend.c 
> b/drivers/media/dvb-core/dvb_frontend.c
> index 2d06bcff0946..58601bfe0b8d 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -2754,7 +2754,7 @@ int dvb_register_frontend(struct dvb_adapter* dvb,
>   fe->dvb->num, fe->id, fe->ops.info.name);
>  
>   dvb_register_device (fe->dvb, >dvbdev, _template,
> -  fe, DVB_DEVICE_FRONTEND);
> +  fe, DVB_DEVICE_FRONTEND, 0);
>  
>   /*
>* Initialize the cache to the proper values according with the
> diff --git a/drivers/media/dvb-core/dvb_net.c 
> b/drivers/media/dvb-core/dvb_net.c
> index b81e026edab3..14f51b68f4fe 100644
> --- a/drivers/media/dvb-core/dvb_net.c
> +++ b/drivers/media/dvb-core/dvb_net.c
> @@ -1503,6 +1503,6 @@ int dvb_net_init (struct dvb_adapter *adap, struct 
> dvb_net *dvbnet,
>   dvbnet->state[i] = 0;
>  
>   return dvb_register_device(adap, >dvbdev, _net,
> -  dvbnet, DVB_DEVICE_NET);
> +  dvbnet, DVB_DEVICE_NET, 0);
>  }
>  EXPORT_SYMBOL(dvb_net_init);
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 88013d1a2c39..f638c67defbe 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -180,18 +180,86 @@ skip:
>   return -ENFILE;
>  }
>  
> +static void dvb_create_tsout_entity(struct dvb_device *dvbdev,
> + const char *name, int npads)
> +{
> +#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
> + int i, ret = 0;
> +
> + dvbdev->tsout_pads = kcalloc(npads, sizeof(*dvbdev->tsout_pads),
> +  GFP_KERNEL);
> + if (!dvbdev->tsout_pads)
> + return;
> + dvbdev->tsout_entity = kcalloc(npads, sizeof(*dvbdev->tsout_entity),
> +GFP_KERNEL);
> + if (!dvbdev->tsout_entity) {
> + kfree(dvbdev->tsout_pads);
> + dvbdev->tsout_pads = NULL;
> + return;
> + }
> + for (i = 0; i < npads; i++) {
> + struct media_pad *pads = >tsout_pads[i];
> + struct media_entity *entity = >tsout_entity[i];
> +
> + entity->name = kasprintf(GFP_KERNEL, "%s #%d", name, i);
> + if (!entity->name) {
> + ret = -ENOMEM;
> + break;
> + }
> +
> + entity->type = MEDIA_ENT_T_DVB_TSOUT;
> + pads->flags = MEDIA_PAD_FL_SINK;
> +
> + ret = media_entity_init(entity, 1, pads);
> + if (ret < 0)
> + break;
> +
> + ret = media_device_register_entity(dvbdev->adapter->mdev,
> +entity);
> + if (ret < 0)
> + break;
> + }
> +
> + if (!ret) {
> + dvbdev->tsout_num_entities = npads;
> + return;
> + }
> +
> + for (i--; i >= 0; i--) {
> + media_device_unregister_entity(>tsout_entity[i]);
> + kfree(dvbdev->tsout_entity[i].name);
> + }
> +
> + printk(KERN_ERR
> + "%s: 

Re: [RFC PATCH v3 2/5] media: videobuf2: Restructure vb2_buffer

[Hans Verkuil]

On 08/31/2015 09:56 AM, Junghak Sung wrote:
> 
> 
> On 08/31/2015 11:01 AM, Junghak Sung wrote:
>> Hello Hans,
>>
>> Thank you for your review.
>> I leave some comments in the body for reply.
>>
>> Regards,
>> Junghak
>>
>>
>>
>> On 08/28/2015 10:31 PM, Hans Verkuil wrote:
>>> Hi Junghak,
>>>
>>> Thanks for this patch, it looks much better. I do have a number of
>>> comments, though...
>>>
>>> On 08/26/2015 01:59 PM, Junghak Sung wrote:
 Remove v4l2-specific stuff from struct vb2_buffer and add member
 variables
 related with buffer management.

 struct vb2_plane {
  
  /* plane information */
  unsigned intbytesused;
  unsigned intlength;
  union {
  unsigned intoffset;
  unsigned long   userptr;
  int fd;
  } m;
  unsigned intdata_offset;
 }

 struct vb2_buffer {
  
  /* buffer information */
  unsigned intnum_planes;
  unsigned intindex;
  unsigned inttype;
  unsigned intmemory;

  struct vb2_planeplanes[VIDEO_MAX_PLANES];
  
 };

 And create struct vb2_v4l2_buffer as container buffer for v4l2 use.

 struct vb2_v4l2_buffer {
  struct vb2_buffer   vb2_buf;

  __u32   flags;
  __u32   field;
  struct timeval  timestamp;
  struct v4l2_timecodetimecode;
  __u32   sequence;
 };

 This patch includes only changes inside of videobuf2. So, it is
 required to
 modify all device drivers which use videobuf2.

 Signed-off-by: Junghak Sung 
 Signed-off-by: Geunyoung Kim 
 Acked-by: Seung-Woo Kim 
 Acked-by: Inki Dae 
 ---
   drivers/media/v4l2-core/videobuf2-core.c |  324
 +-
   include/media/videobuf2-core.h   |   50 ++---
   include/media/videobuf2-v4l2.h   |   26 +++
   3 files changed, 236 insertions(+), 164 deletions(-)

 diff --git a/drivers/media/v4l2-core/videobuf2-core.c
 b/drivers/media/v4l2-core/videobuf2-core.c
 index ab00ea0..9266d50 100644
 --- a/drivers/media/v4l2-core/videobuf2-core.c
 +++ b/drivers/media/v4l2-core/videobuf2-core.c
 @@ -35,10 +35,10 @@
   static int debug;
   module_param(debug, int, 0644);

 -#define dprintk(level, fmt, arg...)  \
 -do {  \
 -if (debug >= level)  \
 -pr_info("vb2: %s: " fmt, __func__, ## arg); \
 +#define dprintk(level, fmt, arg...)\
 +do {\
 +if (debug >= level)\
 +pr_info("vb2: %s: " fmt, __func__, ## arg);\
>>>
>>> These are just whitespace changes, and that is something it see *a
>>> lot* in this
>>> patch. And usually for no clear reason.
>>>
>>> Please remove those whitespace changes, it makes a difficult patch
>>> even harder
>>> to read than it already is.
>>>
>>
>> I just wanted to remove unnecessary white spaces or adjust alignment.
>> OK, I will revert those whitespace changes for better review.
>>
   } while (0)

   #ifdef CONFIG_VIDEO_ADV_DEBUG
>>>
>>> 
>>>
 @@ -656,12 +652,21 @@ static bool __buffers_in_use(struct vb2_queue *q)
*/
   static void __fill_v4l2_buffer(struct vb2_buffer *vb, struct
 v4l2_buffer *b)
   {
 +struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
   struct vb2_queue *q = vb->vb2_queue;
 +unsigned int plane;

   /* Copy back data such as timestamp, flags, etc. */
 -memcpy(b, >v4l2_buf, offsetof(struct v4l2_buffer, m));
 -b->reserved2 = vb->v4l2_buf.reserved2;
 -b->reserved = vb->v4l2_buf.reserved;
>>>
>>> Hmm, I'm not sure why these reserved fields were copied here. I think
>>> it was
>>> for compatibility reasons for some old drivers that abused the
>>> reserved field.
>>> However, in the new code these reserved fields should probably be
>>> explicitly
>>> initialized to 0.
>>>
 +b->index = vb->index;
 +b->type = vb->type;
 +b->memory = vb->memory;
 +b->bytesused = 0;
 +
 +b->flags = vbuf->flags;
 +b->field = vbuf->field;
 +b->timestamp = vbuf->timestamp;
 +b->timecode = vbuf->timecode;
 +b->sequence = vbuf->sequence;

   if (V4L2_TYPE_IS_MULTIPLANAR(q->type)) {
   /*
 @@ -669,21 +674,33 @@ static void 

[PATCH 2/2] [media] media-controller: enable all interface links at init

[Mauro Carvalho Chehab]

Interface links are normally enabled, meaning that the interfaces are
bound to the entities. So, any ioctl send to the interface are reflected
at the entities managed by the interface.

However, when a device is usage, other interfaces for the same hardware
could be decoupled from the entities linked to them, because the
hardware may have some parts busy.

That's for example, what happens when an hybrid TV device is in usage.
If it is streaming analog TV or capturing signals from S-Video/Composite
connectors, typically the digital part of the hardware can't be used and
vice-versa.

This is generally due to some internal hardware or firmware limitation,
that it is not easily mapped via data pipelines.

What the Kernel drivers do internally is that they decouple the hardware
from the interface. So, all changes, if allowed, are done only at some
interface cache, but not physically changed at the hardware.

The usage is similar to the usage of the MEDIA_LNK_FL_ENABLED on data
links. So, let's use the same flag to indicate if ether the interface
to entity link is bound/enabled or not.

Signed-off-by: Mauro Carvalho Chehab 

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 95b5b4b11230..e4234deac34d 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -583,17 +583,21 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
media_create_intf_link(ca, intf, 0);
 
if (intf->type == MEDIA_INTF_T_DVB_FE && tuner)
-   media_create_intf_link(tuner, intf, 0);
+   media_create_intf_link(tuner, intf,
+  MEDIA_LNK_FL_ENABLED);
 
if (intf->type == MEDIA_INTF_T_DVB_DVR && demux)
-   media_create_intf_link(demux, intf, 0);
+   media_create_intf_link(demux, intf,
+  MEDIA_LNK_FL_ENABLED);
 
media_device_for_each_entity(entity, mdev) {
if (entity->type == MEDIA_ENT_T_DVB_TSOUT) {
if (!strcmp(entity->name, DVR_TSOUT))
-   media_create_intf_link(entity, intf, 0);
+   media_create_intf_link(entity, intf,
+  
MEDIA_LNK_FL_ENABLED);
if (!strcmp(entity->name, DEMUX_TSOUT))
-   media_create_intf_link(entity, intf, 0);
+   media_create_intf_link(entity, intf,
+  
MEDIA_LNK_FL_ENABLED);
break;
}
}
diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
b/drivers/media/v4l2-core/v4l2-dev.c
index 427a5a32b3de..d7ee31a20c12 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -787,7 +787,8 @@ static int video_register_media_controller(struct 
video_device *vdev, int type)
 
if (create_entity)
media_create_intf_link(>entity,
-  >intf_devnode->intf, 0);
+  >intf_devnode->intf,
+  MEDIA_LNK_FL_ENABLED);
 
/* FIXME: how to create the other interface links? */
 
diff --git a/drivers/media/v4l2-core/v4l2-device.c 
b/drivers/media/v4l2-core/v4l2-device.c
index 17ec73b1796e..3e4d7cb9ed97 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -253,7 +253,8 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device 
*v4l2_dev)
/* Interface is created by __video_register_device() */
if (vdev->v4l2_dev->mdev)
media_create_intf_link(>entity,
-  >intf_devnode->intf, 0);
+  >intf_devnode->intf,
+  MEDIA_LNK_FL_ENABLED);
 #endif
sd->devnode = vdev;
}
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] [media] media-device: supress backlinks at G_TOPOLOGY ioctl

[Mauro Carvalho Chehab]

Due to the graph traversal algorithm currently in usage, we
need a copy of all data links. Those backlinks should not be
send to userspace, as otherwise, all links there will be
duplicated.

Signed-off-by: Mauro Carvalho Chehab 

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 2c16a46ea530..c181bdcfc72e 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -318,6 +318,9 @@ static long __media_device_get_topology(struct media_device 
*mdev,
/* Get links and number of links */
i = 0;
media_device_for_each_link(link, mdev) {
+   if (link->is_backlink)
+   continue;
+
i++;
 
if (ret || !topo->links)
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 1cdda9cb0512..f75b4c9ac330 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -648,6 +648,7 @@ media_create_pad_link(struct media_entity *source, u16 
source_pad,
backlink->source = >pads[source_pad];
backlink->sink = >pads[sink_pad];
backlink->flags = flags;
+   backlink->is_backlink = true;
 
/* Initialize graph object embedded at the new link */
media_gobj_init(sink->graph_obj.mdev, MEDIA_GRAPH_LINK,
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index c42d191fa5a8..a45eaa1bf801 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -96,6 +96,7 @@ struct media_pipeline {
  * @reverse:   Pointer to the link for the reverse direction of a pad to pad
  * link.
  * @flags: Link flags, as defined at uapi/media.h (MEDIA_LNK_FL_*)
+ * @is_backlink: Indicate if the link is a backlink.
  */
 struct media_link {
struct media_gobj graph_obj;
@@ -112,6 +113,7 @@ struct media_link {
};
struct media_link *reverse;
unsigned long flags;
+   bool is_backlink;
 };
 
 /**
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 03/55] [media] omap3isp: get entity ID using media_entity_id()

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> From: Javier Martinez Canillas 
> 
> Assessing media_entity ID should now use media_entity_id() macro to
> obtain the entity ID, as a next patch will remove the .id field from
> struct media_entity .
> 
> So, get rid of it, otherwise the omap3isp driver will fail to build.
> 
> Signed-off-by: Javier Martinez Canillas 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

Regards,

Hans

> 
> diff --git a/drivers/media/platform/omap3isp/isp.c 
> b/drivers/media/platform/omap3isp/isp.c
> index 56e683b19a73..e08183f9d0f7 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -975,6 +975,7 @@ static int isp_pipeline_disable(struct isp_pipeline *pipe)
>   struct v4l2_subdev *subdev;
>   int failure = 0;
>   int ret;
> + u32 id;
>  
>   /*
>* We need to stop all the modules after CCDC first or they'll
> @@ -1027,8 +1028,10 @@ static int isp_pipeline_disable(struct isp_pipeline 
> *pipe)
>   if (ret) {
>   dev_info(isp->dev, "Unable to stop %s\n", subdev->name);
>   isp->stop_failure = true;
> - if (subdev == >isp_prev.subdev)
> - isp->crashed |= 1U << subdev->entity.id;
> + if (subdev == >isp_prev.subdev) {
> + id = media_entity_id(>entity);
> + isp->crashed |= 1U << id;
> + }
>   failure = -ETIMEDOUT;
>   }
>   }
> diff --git a/drivers/media/platform/omap3isp/ispccdc.c 
> b/drivers/media/platform/omap3isp/ispccdc.c
> index 3b10304b580b..d96e3be5e252 100644
> --- a/drivers/media/platform/omap3isp/ispccdc.c
> +++ b/drivers/media/platform/omap3isp/ispccdc.c
> @@ -1608,7 +1608,7 @@ static int ccdc_isr_buffer(struct isp_ccdc_device *ccdc)
>   /* Wait for the CCDC to become idle. */
>   if (ccdc_sbl_wait_idle(ccdc, 1000)) {
>   dev_info(isp->dev, "CCDC won't become idle!\n");
> - isp->crashed |= 1U << ccdc->subdev.entity.id;
> + isp->crashed |= 1U << media_entity_id(>subdev.entity);
>   omap3isp_pipeline_cancel_stream(pipe);
>   return 0;
>   }
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c 
> b/drivers/media/platform/omap3isp/ispvideo.c
> index 3094572f8897..6c89dc40df85 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -235,7 +235,7 @@ static int isp_video_get_graph_data(struct isp_video 
> *video,
>   while ((entity = media_entity_graph_walk_next())) {
>   struct isp_video *__video;
>  
> - pipe->entities |= 1 << entity->id;
> + pipe->entities |= 1 << media_entity_id(entity);
>  
>   if (far_end != NULL)
>   continue;
> @@ -891,6 +891,7 @@ static int isp_video_check_external_subdevs(struct 
> isp_video *video,
>   struct v4l2_ext_control ctrl;
>   unsigned int i;
>   int ret;
> + u32 id;
>  
>   /* Memory-to-memory pipelines have no external subdev. */
>   if (pipe->input != NULL)
> @@ -898,7 +899,7 @@ static int isp_video_check_external_subdevs(struct 
> isp_video *video,
>  
>   for (i = 0; i < ARRAY_SIZE(ents); i++) {
>   /* Is the entity part of the pipeline? */
> - if (!(pipe->entities & (1 << ents[i]->id)))
> + if (!(pipe->entities & (1 << media_entity_id(ents[i]
>   continue;
>  
>   /* ISP entities have always sink pad == 0. Find source. */
> @@ -950,7 +951,8 @@ static int isp_video_check_external_subdevs(struct 
> isp_video *video,
>  
>   pipe->external_rate = ctrl.value64;
>  
> - if (pipe->entities & (1 << isp->isp_ccdc.subdev.entity.id)) {
> + id = media_entity_id(>isp_ccdc.subdev.entity);
> + if (pipe->entities & (1 << id)) {
>   unsigned int rate = UINT_MAX;
>   /*
>* Check that maximum allowed CCDC pixel rate isn't
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 14/55] [media] media: add functions to allow creating interfaces

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 12:20:43 +0200
Hans Verkuil  escreveu:

> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> > Interfaces are different than entities: they represent a
> > Kernel<->userspace interaction, while entities represent a
> > piece of hardware/firmware/software that executes a function.
> > 
> > Let's distinguish them by creating a separate structure to
> > store the interfaces.
> > 
> > Latter patches should change the existing drivers and logic
> 
> s/Latter/Later/
> 
> FYI: you almost never use 'latter' in English texts. The only common use
> of 'latter' is in the phrase "the latter of two options" where 'latter' means
> 'second'.

I did a global replace on the patches, but perhaps the regex
expression I used didn't got this one.

> 
> > to split the current interface embedded inside the entity
> > structure (device nodes) into a separate object of the graph.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > 
> > diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> > index a23c93369a04..583666e2cc25 100644
> > --- a/drivers/media/media-entity.c
> > +++ b/drivers/media/media-entity.c
> > @@ -44,11 +44,41 @@ static inline const char *gobj_type(enum 
> > media_gobj_type type)
> > return "pad";
> > case MEDIA_GRAPH_LINK:
> > return "link";
> > +   case MEDIA_GRAPH_INTF_DEVNODE:
> > +   return "intf_devnode";
> 
> You use '-' below for 'v4l-subdev' and 'unknown-intf', so I think this too 
> should
> use '-'. My opinion though.

Gah, forgot this one ;)

> 
> It's a nitpick, so I give my Ack anyway:
> 
> Acked-by: Hans Verkuil 
> 
> Regards,
> 
>   Hans
> 
> > default:
> > return "unknown";
> > }
> >  }
> >  
> > +static inline const char *intf_type(struct media_interface *intf)
> > +{
> > +   switch (intf->type) {
> > +   case MEDIA_INTF_T_DVB_FE:
> > +   return "frontend";
> > +   case MEDIA_INTF_T_DVB_DEMUX:
> > +   return "demux";
> > +   case MEDIA_INTF_T_DVB_DVR:
> > +   return "DVR";
> > +   case MEDIA_INTF_T_DVB_CA:
> > +   return  "CA";
> > +   case MEDIA_INTF_T_DVB_NET:
> > +   return "dvbnet";
> > +   case MEDIA_INTF_T_V4L_VIDEO:
> > +   return "video";
> > +   case MEDIA_INTF_T_V4L_VBI:
> > +   return "vbi";
> > +   case MEDIA_INTF_T_V4L_RADIO:
> > +   return "radio";
> > +   case MEDIA_INTF_T_V4L_SUBDEV:
> > +   return "v4l2-subdev";
> > +   case MEDIA_INTF_T_V4L_SWRADIO:
> > +   return "swradio";
> > +   default:
> > +   return "unknown-intf";
> > +   }
> > +};
> > +
> >  static void dev_dbg_obj(const char *event_name,  struct media_gobj *gobj)
> >  {
> >  #if defined(DEBUG) || defined (CONFIG_DYNAMIC_DEBUG)
> > @@ -84,6 +114,19 @@ static void dev_dbg_obj(const char *event_name,  struct 
> > media_gobj *gobj)
> > "%s: id 0x%08x pad#%d: '%s':%d\n",
> > event_name, gobj->id, media_localid(gobj),
> > pad->entity->name, pad->index);
> > +   break;
> > +   }
> > +   case MEDIA_GRAPH_INTF_DEVNODE:
> > +   {
> > +   struct media_interface *intf = gobj_to_intf(gobj);
> > +   struct media_intf_devnode *devnode = intf_to_devnode(intf);
> > +
> > +   dev_dbg(gobj->mdev->dev,
> > +   "%s: id 0x%08x intf_devnode#%d: %s - major: %d, minor: 
> > %d\n",
> > +   event_name, gobj->id, media_localid(gobj),
> > +   intf_type(intf),
> > +   devnode->major, devnode->minor);
> > +   break;
> > }
> > }
> >  #endif
> > @@ -119,6 +162,9 @@ void media_gobj_init(struct media_device *mdev,
> > case MEDIA_GRAPH_LINK:
> > gobj->id = media_gobj_gen_id(type, ++mdev->link_id);
> > break;
> > +   case MEDIA_GRAPH_INTF_DEVNODE:
> > +   gobj->id = media_gobj_gen_id(type, ++mdev->intf_devnode_id);
> > +   break;
> > }
> > dev_dbg_obj(__func__, gobj);
> >  }
> > @@ -793,3 +839,40 @@ struct media_pad *media_entity_remote_pad(struct 
> > media_pad *pad)
> >  
> >  }
> >  EXPORT_SYMBOL_GPL(media_entity_remote_pad);
> > +
> > +
> > +/* Functions related to the media interface via device nodes */
> > +
> > +struct media_intf_devnode *media_devnode_create(struct media_device *mdev,
> > +   u32 type, u32 flags,
> > +   u32 major, u32 minor,
> > +   gfp_t gfp_flags)
> > +{
> > +   struct media_intf_devnode *devnode;
> > +   struct media_interface *intf;
> > +
> > +   devnode = kzalloc(sizeof(*devnode), gfp_flags);
> > +   if (!devnode)
> > +   return NULL;
> > +
> > +   intf = >intf;
> > +
> > +   intf->type = type;
> > +   intf->flags = flags;
> > +
> > +   devnode->major = major;
> > +   devnode->minor = minor;
> > +
> > +   

Re: [PATCH v8 41/55] [media] DocBook: update descriptions for the media controller entities

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Cleanup the media controller entities description:
> - remove MEDIA_ENT_T_DEVNODE and MEDIA_ENT_T_V4L2_SUBDEV entity
>   types, as they don't mean anything;

Shouldn't this add MEDIA_ENT_T_V4L2_VBI and _SWRADIO descriptions?

Regards,

Hans

> - add MEDIA_ENT_T_UNKNOWN with a proper description;
> - remove ALSA and FB entity types. Those should not be used, as
>   the types are deprecated. We'll soon be adidng ALSA, but with
>   a different entity namespace;
> - improve the description of some entities.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml 
> b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> index 32a783635649..bd90dde54416 100644
> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> @@ -179,70 +179,59 @@
>  
>   
> 
> - MEDIA_ENT_T_DEVNODE
> - Unknown device node
> + MEDIA_ENT_T_UNKNOWN and 
> MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN
> + Unknown entity. That generally indicates that
> + a driver didn't initialize properly the entity, with is a Kernel 
> bug
> 
> 
>   MEDIA_ENT_T_V4L2_VIDEO
> - V4L video, radio or vbi device node
> -   
> -   
> - MEDIA_ENT_T_DEVNODE_FB
> - Frame buffer device node
> -   
> -   
> - MEDIA_ENT_T_DEVNODE_ALSA
> - ALSA card
> + V4L video streaming input or output entity
> 
> 
>   MEDIA_ENT_T_DVB_DEMOD
> - DVB frontend devnode
> + DVB demodulator entity
> 
> 
>   MEDIA_ENT_T_DVB_DEMUX
> - DVB demux devnode
> + DVB demux entity. Could be implemented on hardware or in 
> Kernelspace
> 
> 
>   MEDIA_ENT_T_DVB_TSOUT
> - DVB DVR devnode
> + DVB Transport Stream output entity
> 
> 
>   MEDIA_ENT_T_DVB_CA
> - DVB CAM devnode
> + DVB Conditional Access module (CAM) entity
> 
> 
>   MEDIA_ENT_T_DVB_DEMOD_NET_DECAP
> - DVB network devnode
> -   
> -   
> - MEDIA_ENT_T_V4L2_SUBDEV
> - Unknown V4L sub-device
> + DVB network ULE/MLE desencapsulation entity. Could be 
> implemented on hardware or in Kernelspace
> 
> 
>   MEDIA_ENT_T_V4L2_SUBDEV_SENSOR
> - Video sensor
> + Camera video sensor entity
> 
> 
>   MEDIA_ENT_T_V4L2_SUBDEV_FLASH
> - Flash controller
> + Flash controller entity
> 
> 
>   MEDIA_ENT_T_V4L2_SUBDEV_LENS
> - Lens controller
> + Lens controller entity
> 
> 
>   MEDIA_ENT_T_V4L2_SUBDEV_DECODER
> - Video decoder, the basic function of the video decoder is to
> - accept analogue video from a wide variety of sources such as
> + Analog video decoder, the basic function of the video decoder
> + is to accept analogue video from a wide variety of sources such as
>   broadcast, DVD players, cameras and video cassette recorders, in
> - either NTSC, PAL or HD format and still occasionally SECAM, separate
> - it into its component parts, luminance and chrominance, and output
> + either NTSC, PAL, SECAM or HD format, separating the stream
> + into its component parts, luminance and chrominance, and output
>   it in some digital video standard, with appropriate embedded timing
>   signals.
> 
> 
>   MEDIA_ENT_T_V4L2_SUBDEV_TUNER
> - TV and/or radio tuner
> + Digital TV, analog TV, radio and/or software radio 
> tuner
> 
>   
>
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

New Terratec Cinergy S2 Box usb-id

[Maximilian Imgrund]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Dear all,

I am currently figuring out how to get the Terratec Cinergy S2 USB Box
up and running. I already modified a patch to  previous version (see
attachment) to include the new ID in the device driver, module is also
loading with the ds3000 firmware. However, when using w_scan, the
reported frequency range is .95GHz ... 2.15Ghz which is roughly a
factor of 10 lower than I expected (Astra is 12.515Ghz e.g.). Since
the tuner seems to tune in correctly but in the wrong frequency range,
I feel that is the reason for me not getting in any channels.

Can you help me with what to change in the driver to get this working
? I feel like an additional .frequency_div should do the job, however
I am unable to find further informaion on that...

Best
Maximilian Imgrund
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iF4EAREIAAYFAlXkOFIACgkQR/X5cR0fI/6sfAD+OVauTyLw0oWSMr8ONzmrguF+
Ci/vg4uO9mxZwzjgGXkA/ipgQ/IuX+8n2CSScHg6CFjt9tIBbFOAVzStuUrOpwx2
=AAXS
-END PGP SIGNATURE-
--- a/drivers/media/usb/dvb-usb/dw2102.c
+++ b/drivers/media/usb/dvb-usb/dw2102.c
@@ -1657,6 +1657,7 @@
GOTVIEW_SAT_HD,
GENIATECH_T220,
TECHNOTREND_S2_4600,
+   TERRATEC_CINERGY_S2_R3,
 };
 
 static struct usb_device_id dw2102_table[] = {
@@ -1682,6 +1683,7 @@
[GENIATECH_T220] = {USB_DEVICE(0x1f4d, 0xD220)},
[TECHNOTREND_S2_4600] = {USB_DEVICE(USB_VID_TECHNOTREND,
USB_PID_TECHNOTREND_CONNECT_S2_4600)},
+   [TERRATEC_CINERGY_S2_R3] = {USB_DEVICE(USB_VID_TERRATEC, 0x0105)},
{ }
 };
 
@@ -2085,7 +2087,7 @@
}},
}
},
-   .num_device_descs = 5,
+   .num_device_descs = 6,
.devices = {
{ "SU3000HD DVB-S USB2.0",
{ _table[GENIATECH_SU3000], NULL },
@@ -2107,6 +2109,10 @@
{ _table[GOTVIEW_SAT_HD], NULL },
{ NULL },
},
+   { "Terratec Cinergy S2 USB HD Rev.3",
+   { _table[TERRATEC_CINERGY_S2_R3], NULL },
+   { NULL },
+   },
}
 };
 

Re: [PATCH v8 43/55] [media] media: report if a pad is sink or source at debug msg

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Sometimes, it is important to see if the created pad is
> sink or source. Add info to track that.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index f638c67defbe..610d2bab1368 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -528,8 +528,8 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
>   struct media_entity *entity, *tuner = NULL, *demod = NULL;
>   struct media_entity *demux = NULL, *ca = NULL;
>   struct media_interface *intf;
> - unsigned demux_pad = 1;
> - unsigned dvr_pad = 1;
> + unsigned demux_pad = 0;
> + unsigned dvr_pad = 0;
>  
>   if (!mdev)
>   return;
> @@ -561,15 +561,19 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
>  
>   /* Create demux links for each ringbuffer/pad */
>   if (demux) {
> - if (entity->type == MEDIA_ENT_T_DVB_TSOUT) {
> - if (!strncmp(entity->name, DVR_TSOUT,
> -  sizeof(DVR_TSOUT)))
> - media_create_pad_link(demux, ++dvr_pad,
> -   entity, 0, 0);
> - if (!strncmp(entity->name, DEMUX_TSOUT,
> -  sizeof(DEMUX_TSOUT)))
> - media_create_pad_link(demux, ++demux_pad,
> -   entity, 0, 0);
> + media_device_for_each_entity(entity, mdev) {
> + if (entity->type == MEDIA_ENT_T_DVB_TSOUT) {
> + if (!strncmp(entity->name, DVR_TSOUT,
> + strlen(DVR_TSOUT)))
> + media_create_pad_link(demux,
> +   ++dvr_pad,
> + entity, 0, 0);
> + if (!strncmp(entity->name, DEMUX_TSOUT,
> + strlen(DEMUX_TSOUT)))
> + media_create_pad_link(demux,
> +   ++demux_pad,
> + entity, 0, 0);
> + }
>   }
>   }
>  

Does this chunk belong here? I'd expect this in the previous patch or in a
patch on its own.

> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 15bc92d3a648..d62a6ffbc929 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -121,8 +121,11 @@ static void dev_dbg_obj(const char *event_name,  struct 
> media_gobj *gobj)
>   struct media_pad *pad = gobj_to_pad(gobj);
>  
>   dev_dbg(gobj->mdev->dev,
> - "%s: id 0x%08x pad#%d: '%s':%d\n",
> - event_name, gobj->id, media_localid(gobj),
> + "%s: id 0x%08x %s%spad#%d: '%s':%d\n",
> + event_name, gobj->id,
> + pad->flags & MEDIA_PAD_FL_SINK   ? "  sink " : "",
> + pad->flags & MEDIA_PAD_FL_SOURCE ? "source " : "",
> + media_localid(gobj),
>   pad->entity->name, pad->index);
>   break;
>   }
> 

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 26/55] [media] media: add a linked list to track interfaces by mdev

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> The media device should list the interface objects, so add a linked list
> for those interfaces in struct media_device.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 3e649cacfc07..659507bce63f 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -381,6 +381,7 @@ int __must_check __media_device_register(struct 
> media_device *mdev,
>   return -EINVAL;
>  
>   INIT_LIST_HEAD(>entities);
> + INIT_LIST_HEAD(>interfaces);
>   spin_lock_init(>lock);
>   mutex_init(>graph_mutex);
>  
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 417673a32c21..15bc92d3a648 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -861,6 +861,8 @@ static void media_interface_init(struct media_device 
> *mdev,
>   INIT_LIST_HEAD(>links);
>  
>   media_gobj_init(mdev, gobj_type, >graph_obj);
> +
> + list_add_tail(>list, >interfaces);
>  }
>  
>  /* Functions related to the media interface via device nodes */
> @@ -889,6 +891,7 @@ EXPORT_SYMBOL_GPL(media_devnode_create);
>  void media_devnode_remove(struct media_intf_devnode *devnode)
>  {
>   media_gobj_remove(>intf.graph_obj);
> + list_del(>intf.list);
>   kfree(devnode);
>  }
>  EXPORT_SYMBOL_GPL(media_devnode_remove);
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index 3b14394d5701..51807efa505b 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -46,6 +46,7 @@ struct device;
>   * @link_id: Unique ID used on the last link registered
>   * @intf_devnode_id: Unique ID used on the last interface devnode registered
>   * @entities:List of registered entities
> + * @interfaces:  List of registered interfaces
>   * @lock:Entities list lock
>   * @graph_mutex: Entities graph operation lock
>   * @link_notify: Link state change notification callback
> @@ -77,6 +78,7 @@ struct media_device {
>   u32 intf_devnode_id;
>  
>   struct list_head entities;
> + struct list_head interfaces;
>  
>   /* Protects the entities list */
>   spinlock_t lock;
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 423ff804e686..e7b20bdc735d 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -156,6 +156,8 @@ struct media_entity {
>   * struct media_intf_devnode - Define a Kernel API interface
>   *
>   * @graph_obj:   embedded graph object
> + * @list:Linked list used to find other interfaces that belong
> + *   to the same media controller
>   * @links:   List of links pointing to graph entities
>   * @type:Type of the interface as defined at the
>   *   uapi/media/media.h header, e. g.
> @@ -164,6 +166,7 @@ struct media_entity {
>   */
>  struct media_interface {
>   struct media_gobj   graph_obj;
> + struct list_headlist;
>   struct list_headlinks;
>   u32 type;
>   u32 flags;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] media: atmel-isi: setup the ISI_CFG2 register directly

[Josh Wu]

Hi, Guennadi

Thanks for the review.

On 8/30/2015 4:48 PM, Guennadi Liakhovetski wrote:

Hi Josh,

On Wed, 17 Jun 2015, Josh Wu wrote:


In the function configure_geometry(), we will setup the ISI CFG2
according to the sensor output format.

It make no sense to just read back the CFG2 register and just set part
of it.

So just set up this register directly makes things simpler.

Simpler doesn't necessarily mean better or more correct:) There are other
fields in that register and currently the driver preserves them, with this
patch you overwrite them with 0. 0 happens to be the reset value of that
register. So, you should at least mention that in this patch description,
just saying "simpler" doesn't convince me.


Correct, I should mention that the reset value (0) of cfg2 means the YUV 
mode in the commit message.
To use YUV mode we need to clear COL_SPACE (bit 15 of CFG2) and since 
the reset value is 0, so in the code, I didn't need do anything.



So, at least I'd modify that, I
can do that myself. But in general I'm not even sure why this patch is
needed. Yes, currently those fields of that register are unused, so, we
can assume they stay at their reset values. But firstly the hardware can
change and at some point the reset value can change, or those other fields
will get set indirectly by something. Or the driver will change at some
point to support more fields of that register and then this code will have
to be changed again.


I understand your concern.
maybe a better solution is explicitly set the COL_SPACE (bit 15) to 0 
for the YUV formats. like:


#define ISI_CFG2_COL_SPACE_YUV(0 << 15)

case MEDIA_BUS_FMT_YVYU8_2X8:
cfg2 = ISI_CFG2_COL_SPACE_YUV | ISI_CFG2_YCC_SWAP_MODE_1;
break;

And above modifications can be sent with RGB format support patch in future.


So, I'd ask you again - do you really want this
patch?


yes, this patch is needed. And in future i will add the RGB format settings.


If you insist - I'll take it, but I'd add the "reset value"
reasoning.


That would be great, thank you very much.

Best Regards,
Josh Wu


Otherwise maybe just drop it?

Thanks
Guennadi


Currently only support YUV format from camera sensor.

Signed-off-by: Josh Wu 
---

  drivers/media/platform/soc_camera/atmel-isi.c | 20 +++-
  1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/soc_camera/atmel-isi.c 
b/drivers/media/platform/soc_camera/atmel-isi.c
index 9070172..8bc40ca 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -105,24 +105,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
  static int configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
  {
-   u32 cfg2, cr;
+   u32 cfg2;
  
+	/* According to sensor's output format to set cfg2 */

switch (code) {
/* YUV, including grey */
case MEDIA_BUS_FMT_Y8_1X8:
-   cr = ISI_CFG2_GRAYSCALE;
+   cfg2 = ISI_CFG2_GRAYSCALE;
break;
case MEDIA_BUS_FMT_VYUY8_2X8:
-   cr = ISI_CFG2_YCC_SWAP_MODE_3;
+   cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
-   cr = ISI_CFG2_YCC_SWAP_MODE_2;
+   cfg2 = ISI_CFG2_YCC_SWAP_MODE_2;
break;
case MEDIA_BUS_FMT_YVYU8_2X8:
-   cr = ISI_CFG2_YCC_SWAP_MODE_1;
+   cfg2 = ISI_CFG2_YCC_SWAP_MODE_1;
break;
case MEDIA_BUS_FMT_YUYV8_2X8:
-   cr = ISI_CFG2_YCC_SWAP_DEFAULT;
+   cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
default:
@@ -130,17 +131,10 @@ static int configure_geometry(struct atmel_isi *isi, u32 
width,
}
  
  	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);

-
-   cfg2 = isi_readl(isi, ISI_CFG2);
-   /* Set YCC swap mode */
-   cfg2 &= ~ISI_CFG2_YCC_SWAP_MODE_MASK;
-   cfg2 |= cr;
/* Set width */
-   cfg2 &= ~(ISI_CFG2_IM_HSIZE_MASK);
cfg2 |= ((width - 1) << ISI_CFG2_IM_HSIZE_OFFSET) &
ISI_CFG2_IM_HSIZE_MASK;
/* Set height */
-   cfg2 &= ~(ISI_CFG2_IM_VSIZE_MASK);
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
--
1.9.1



--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 16/55] [media] media: Don't accept early-created links

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 12:30:16 +0200
Hans Verkuil  escreveu:

> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> > Links are graph objects that represent the links of two already
> > existing objects in the graph.
> > 
> > While with the current implementation, it is possible to create
> > the links earlier, It doesn't make any sense to allow linking
> > two objects when they are not both created.
> > 
> > So, remove the code that would be handling those early-created
> > links and add a BUG_ON() to ensure that.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> 
> The code is OK, so:
> 
> Acked-by: Hans Verkuil 
> 
> But shouldn't this go *after* the omap3isp fixes? After this patch the
> omap3isp will call BUG_ON, and that's not what you want.

Yes. I'll change the order on my git tree.

> It is also not clear if the omap3isp driver is the only one that has this
> 'create link before objects' problem. I would expect that the omap4 staging
> driver has the same issue and possibly others as well.
> 
> Did someone look at that?

I guess other drivers are doing the same.

Javier's planning to review the other platform drivers in order to add the 
needed fixes there too, and to do more tests with some other platform
drivers that he has hardware for testing.

> 
> Regards,
> 
>   Hans
> 
> > 
> > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> > index 138b18416460..0d85c6c28004 100644
> > --- a/drivers/media/media-device.c
> > +++ b/drivers/media/media-device.c
> > @@ -443,13 +443,6 @@ int __must_check media_device_register_entity(struct 
> > media_device *mdev,
> > media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, >graph_obj);
> > list_add_tail(>list, >entities);
> >  
> > -   /*
> > -* Initialize objects at the links
> > -* in the case where links got created before entity register
> > -*/
> > -   for (i = 0; i < entity->num_links; i++)
> > -   media_gobj_init(mdev, MEDIA_GRAPH_LINK,
> > -   >links[i].graph_obj);
> > /* Initialize objects at the pads */
> > for (i = 0; i < entity->num_pads; i++)
> > media_gobj_init(mdev, MEDIA_GRAPH_PAD,
> > diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> > index 01946baa32d5..9f8e0145db7a 100644
> > --- a/drivers/media/media-entity.c
> > +++ b/drivers/media/media-entity.c
> > @@ -161,6 +161,8 @@ void media_gobj_init(struct media_device *mdev,
> >enum media_gobj_type type,
> >struct media_gobj *gobj)
> >  {
> > +   BUG_ON(!mdev);
> > +
> > gobj->mdev = mdev;
> >  
> > /* Create a per-type unique object ID */
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 16/55] [media] media: Don't accept early-created links

[Hans Verkuil]

On 08/31/2015 12:54 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 31 Aug 2015 12:30:16 +0200
> Hans Verkuil  escreveu:
> 
>> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
>>> Links are graph objects that represent the links of two already
>>> existing objects in the graph.
>>>
>>> While with the current implementation, it is possible to create
>>> the links earlier, It doesn't make any sense to allow linking
>>> two objects when they are not both created.
>>>
>>> So, remove the code that would be handling those early-created
>>> links and add a BUG_ON() to ensure that.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab 
>>
>> The code is OK, so:
>>
>> Acked-by: Hans Verkuil 
>>
>> But shouldn't this go *after* the omap3isp fixes? After this patch the
>> omap3isp will call BUG_ON, and that's not what you want.
> 
> Yes. I'll change the order on my git tree.
> 
>> It is also not clear if the omap3isp driver is the only one that has this
>> 'create link before objects' problem. I would expect that the omap4 staging
>> driver has the same issue and possibly others as well.
>>
>> Did someone look at that?
> 
> I guess other drivers are doing the same.
> 
> Javier's planning to review the other platform drivers in order to add the 
> needed fixes there too, and to do more tests with some other platform
> drivers that he has hardware for testing.

OK, good. Just wanted to know that.

Perhaps it is a good idea to add a TODO list to the cover letter. This would
be one item on that list.

Regards,

Hans

> 
>>
>> Regards,
>>
>>  Hans
>>
>>>
>>> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
>>> index 138b18416460..0d85c6c28004 100644
>>> --- a/drivers/media/media-device.c
>>> +++ b/drivers/media/media-device.c
>>> @@ -443,13 +443,6 @@ int __must_check media_device_register_entity(struct 
>>> media_device *mdev,
>>> media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, >graph_obj);
>>> list_add_tail(>list, >entities);
>>>  
>>> -   /*
>>> -* Initialize objects at the links
>>> -* in the case where links got created before entity register
>>> -*/
>>> -   for (i = 0; i < entity->num_links; i++)
>>> -   media_gobj_init(mdev, MEDIA_GRAPH_LINK,
>>> -   >links[i].graph_obj);
>>> /* Initialize objects at the pads */
>>> for (i = 0; i < entity->num_pads; i++)
>>> media_gobj_init(mdev, MEDIA_GRAPH_PAD,
>>> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
>>> index 01946baa32d5..9f8e0145db7a 100644
>>> --- a/drivers/media/media-entity.c
>>> +++ b/drivers/media/media-entity.c
>>> @@ -161,6 +161,8 @@ void media_gobj_init(struct media_device *mdev,
>>>enum media_gobj_type type,
>>>struct media_gobj *gobj)
>>>  {
>>> +   BUG_ON(!mdev);
>>> +
>>> gobj->mdev = mdev;
>>>  
>>> /* Create a per-type unique object ID */
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 27/55] [media] dvbdev: add support for indirect interface links

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Some interfaces indirectly control multiple entities.
> Add support for those.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 6bf61d42c017..14d32cdcdd47 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -441,6 +441,7 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
>   struct media_device *mdev = adap->mdev;
>   struct media_entity *entity, *tuner = NULL, *fe = NULL;
>   struct media_entity *demux = NULL, *dvr = NULL, *ca = NULL;
> + struct media_interface *intf;
>  
>   if (!mdev)
>   return;
> @@ -476,6 +477,18 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
>  
>   if (demux && ca)
>   media_create_pad_link(demux, 1, ca, 0, MEDIA_LNK_FL_ENABLED);
> +
> + /* Create indirect interface links for FE->tuner, DVR->demux and CA->ca 
> */
> + list_for_each_entry(intf, >interfaces, list) {
> + if (intf->type == MEDIA_INTF_T_DVB_CA && ca)
> + media_create_intf_link(ca, intf, 0);
> + if (intf->type == MEDIA_INTF_T_DVB_FE && tuner)
> + media_create_intf_link(tuner, intf, 0);
> + if (intf->type == MEDIA_INTF_T_DVB_DVR && demux)
> + media_create_intf_link(demux, intf, 0);
> + }
> +
> +

Spurious newlines.

After removing you can add my:

Acked-by: Hans Verkuil 

>  }
>  EXPORT_SYMBOL_GPL(dvb_create_media_graph);
>  #endif
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 31/55] [media] media: add macros to check if subdev or V4L2 DMA

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> As we'll be removing entity subtypes from the Kernel, we need
> to provide a way for drivers and core to check if a given
> entity is represented by a V4L2 subdev or if it is an V4L2
> I/O entity (typically with DMA).

This needs more discussion. The plan (as I understand it) is to have properties
that describe the entity's functionalities.

The existing entity subtypes will exist only as backwards compat types, but in
the future properties should be used to describe the functionalities.

This raises the question if we shouldn't use MEDIA_ENT_T_V4L2_SUBDEV to tell
userspace that this is a subdev-controlled entity, and let userspace look at
the properties to figure out what it is exactly?

It could be that this is a transitional patch, and this will be fixed later.
If so, this should be mentioned in the commit message.

Regards,

Hans

> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index e7b20bdc735d..b0cfbc0dffc7 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -220,6 +220,39 @@ static inline u32 media_gobj_gen_id(enum media_gobj_type 
> type, u32 local_id)
>   return id;
>  }
>  
> +static inline bool is_media_entity_v4l2_io(struct media_entity *entity)
> +{
> + if (!entity)
> + return false;
> +
> + switch (entity->type) {
> + case MEDIA_ENT_T_V4L2_VIDEO:
> + case MEDIA_ENT_T_V4L2_VBI:
> + case MEDIA_ENT_T_V4L2_SWRADIO:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static inline bool is_media_entity_v4l2_subdev(struct media_entity *entity)
> +{
> + if (!entity)
> + return false;
> +
> + switch (entity->type) {
> + case MEDIA_ENT_T_V4L2_SUBDEV_SENSOR:
> + case MEDIA_ENT_T_V4L2_SUBDEV_FLASH:
> + case MEDIA_ENT_T_V4L2_SUBDEV_LENS:
> + case MEDIA_ENT_T_V4L2_SUBDEV_DECODER:
> + case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
> + return true;
> +
> + default:
> + return false;
> + }
> +}
> +
>  #define MEDIA_ENTITY_ENUM_MAX_DEPTH  16
>  #define MEDIA_ENTITY_ENUM_MAX_ID 64
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 38/55] [media] v4l2-subdev: use MEDIA_ENT_T_UNKNOWN for new subdevs

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Instead of abusing MEDIA_ENT_T_V4L2_SUBDEV, initialize
> new subdev entities as MEDIA_ENT_T_UNKNOWN.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 659507bce63f..134fe7510195 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -435,6 +435,12 @@ int __must_check media_device_register_entity(struct 
> media_device *mdev,
>  {
>   int i;
>  
> + if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN ||
> + entity->type == MEDIA_ENT_T_UNKNOWN)
> + dev_warn(mdev->dev,
> +  "Entity type for entity %s was not initialized!\n",
> +  entity->name);
> +
>   /* Warn if we apparently re-register an entity */
>   WARN_ON(entity->graph_obj.mdev != NULL);
>   entity->graph_obj.mdev = mdev;
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
> b/drivers/media/v4l2-core/v4l2-subdev.c
> index 60da43772de9..b3bcc8253182 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -584,7 +584,7 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const 
> struct v4l2_subdev_ops *ops)
>   sd->host_priv = NULL;
>  #if defined(CONFIG_MEDIA_CONTROLLER)
>   sd->entity.name = sd->name;
> - sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV;
> + sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN;
>  #endif
>  }
>  EXPORT_SYMBOL(v4l2_subdev_init);
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 3bbda409353f..44b84aae8b02 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -42,6 +42,14 @@ struct media_device_info {
>  
>  #define MEDIA_ENT_ID_FLAG_NEXT   (1 << 31)
>  
> +/* Used values for media_entity_desc::type */
> +
> +/*
> + * Initial value when an entity is created
> + * Drivers should change it to something useful
> + */
> +#define MEDIA_ENT_T_UNKNOWN  0x
> +
>  /*
>   * Base numbers for entity types
>   *
> @@ -77,6 +85,15 @@ struct media_device_info {
>  #define MEDIA_ENT_T_V4L2_SWRADIO (MEDIA_ENT_T_V4L2_BASE + 7)
>  
>  /* V4L2 Sub-device entities */
> +
> + /*
> +  * Subdevs are initialized with MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN,
> +  * in order to preserve backward compatibility.
> +  * Drivers should change to the proper subdev type before
> +  * registering the entity.
> +  */
> +#define MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN  MEDIA_ENT_T_V4L2_SUBDEV_BASE
> +
>  #define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR   (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 
> 1)
>  #define MEDIA_ENT_T_V4L2_SUBDEV_FLASH(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 
> 2)
>  #define MEDIA_ENT_T_V4L2_SUBDEV_LENS (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 3)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 39/55] [media] media controller: get rid of entity subtype on Kernel

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Don't use anymore the type/subtype entity data/macros
> inside the Kernel.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index b0cfbc0dffc7..756e1960fd7f 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -185,16 +185,6 @@ struct media_intf_devnode {
>   u32 minor;
>  };
>  
> -static inline u32 media_entity_type(struct media_entity *entity)
> -{
> - return entity->type & MEDIA_ENT_TYPE_MASK;
> -}
> -
> -static inline u32 media_entity_subtype(struct media_entity *entity)
> -{
> - return entity->type & MEDIA_ENT_SUBTYPE_MASK;
> -}
> -
>  static inline u32 media_entity_id(struct media_entity *entity)
>  {
>   return entity->graph_obj.id;
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 44b84aae8b02..cd486fc25f1e 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -42,10 +42,8 @@ struct media_device_info {
>  
>  #define MEDIA_ENT_ID_FLAG_NEXT   (1 << 31)
>  
> -/* Used values for media_entity_desc::type */
> -
>  /*
> - * Initial value when an entity is created
> + * Initial value to be used when a new entity is created

This change should be moved to patch 38.

>   * Drivers should change it to something useful
>   */
>  #define MEDIA_ENT_T_UNKNOWN  0x
> 

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 21/55] [media] media: make media_link more generic to handle interace links

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> By adding an union at media_link, we get for free a way to
> represent interface->entity links.
> 
> No need to change anything at the code, just at the internal
> header file.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index bb89cedb0c40..b4923a24efd5 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -75,14 +75,20 @@ struct media_pipeline {
>  struct media_link {
>   struct media_gobj graph_obj;
>   struct list_head list;
> - struct media_pad *source;   /* Source pad */
> - struct media_pad *sink; /* Sink pad  */
> + union {
> + struct media_gobj *gobj0;
> + struct media_pad *source;
> + };
> + union {
> + struct media_gobj *gobj1;
> + struct media_pad *sink;
> + };
>   struct media_link *reverse; /* Link in the reverse direction */
>   unsigned long flags;/* Link flags (MEDIA_LNK_FL_*) */
>  };
>  
>  struct media_pad {
> - struct media_gobj graph_obj;
> + struct media_gobj graph_obj;/* must be first field in struct */
>   struct media_entity *entity;/* Entity this pad belongs to */
>   u16 index;  /* Pad index in the entity pads array */
>   unsigned long flags;/* Pad flags (MEDIA_PAD_FL_*) */
> @@ -105,7 +111,7 @@ struct media_entity_operations {
>  };
>  
>  struct media_entity {
> - struct media_gobj graph_obj;
> + struct media_gobj graph_obj;/* must be first field in struct */
>   struct list_head list;
>   const char *name;   /* Entity name */
>   u32 type;   /* Entity type (MEDIA_ENT_T_*) */
> @@ -119,7 +125,7 @@ struct media_entity {
>   u16 num_backlinks;  /* Number of backlinks */
>  
>   struct media_pad *pads; /* Pads array (num_pads objects) */
> - struct list_head links; /* Links list */
> + struct list_head links; /* Pad-to-pad links list */
>  
>   const struct media_entity_operations *ops;  /* Entity operations */
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 20/55] [media] media: make add link more generic

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> The media_entity_add_link() function takes an entity
> as an argument just to get the list head.
> 
> Make it more generic by changing the function argument
> to list_head.
> 
> No functional changes.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index ff63201443d7..6d06be6c9ef3 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -580,7 +580,7 @@ EXPORT_SYMBOL_GPL(media_entity_put);
>   * Links management
>   */
>  
> -static struct media_link *media_entity_add_link(struct media_entity *entity)
> +static struct media_link *media_add_link(struct list_head *head)
>  {
>   struct media_link *link;
>  
> @@ -588,7 +588,7 @@ static struct media_link *media_entity_add_link(struct 
> media_entity *entity)
>   if (link == NULL)
>   return NULL;
>  
> - list_add_tail(>list, >links);
> + list_add_tail(>list, head);
>  
>   return link;
>  }
> @@ -607,7 +607,7 @@ media_create_pad_link(struct media_entity *source, u16 
> source_pad,
>   BUG_ON(source_pad >= source->num_pads);
>   BUG_ON(sink_pad >= sink->num_pads);
>  
> - link = media_entity_add_link(source);
> + link = media_add_link(>links);
>   if (link == NULL)
>   return -ENOMEM;
>  
> @@ -622,7 +622,7 @@ media_create_pad_link(struct media_entity *source, u16 
> source_pad,
>   /* Create the backlink. Backlinks are used to help graph traversal and
>* are not reported to userspace.
>*/
> - backlink = media_entity_add_link(sink);
> + backlink = media_add_link(>links);
>   if (backlink == NULL) {
>   __media_entity_remove_link(source, link);
>   return -ENOMEM;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v3 2/5] media: videobuf2: Restructure vb2_buffer

[Junghak Sung]

On 08/31/2015 11:01 AM, Junghak Sung wrote:

Hello Hans,

Thank you for your review.
I leave some comments in the body for reply.

Regards,
Junghak



On 08/28/2015 10:31 PM, Hans Verkuil wrote:

Hi Junghak,

Thanks for this patch, it looks much better. I do have a number of
comments, though...

On 08/26/2015 01:59 PM, Junghak Sung wrote:

Remove v4l2-specific stuff from struct vb2_buffer and add member
variables
related with buffer management.

struct vb2_plane {
 
 /* plane information */
 unsigned intbytesused;
 unsigned intlength;
 union {
 unsigned intoffset;
 unsigned long   userptr;
 int fd;
 } m;
 unsigned intdata_offset;
}

struct vb2_buffer {
 
 /* buffer information */
 unsigned intnum_planes;
 unsigned intindex;
 unsigned inttype;
 unsigned intmemory;

 struct vb2_planeplanes[VIDEO_MAX_PLANES];
 
};

And create struct vb2_v4l2_buffer as container buffer for v4l2 use.

struct vb2_v4l2_buffer {
 struct vb2_buffer   vb2_buf;

 __u32   flags;
 __u32   field;
 struct timeval  timestamp;
 struct v4l2_timecodetimecode;
 __u32   sequence;
};

This patch includes only changes inside of videobuf2. So, it is
required to
modify all device drivers which use videobuf2.

Signed-off-by: Junghak Sung 
Signed-off-by: Geunyoung Kim 
Acked-by: Seung-Woo Kim 
Acked-by: Inki Dae 
---
  drivers/media/v4l2-core/videobuf2-core.c |  324
+-
  include/media/videobuf2-core.h   |   50 ++---
  include/media/videobuf2-v4l2.h   |   26 +++
  3 files changed, 236 insertions(+), 164 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index ab00ea0..9266d50 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -35,10 +35,10 @@
  static int debug;
  module_param(debug, int, 0644);

-#define dprintk(level, fmt, arg...)  \
-do {  \
-if (debug >= level)  \
-pr_info("vb2: %s: " fmt, __func__, ## arg); \
+#define dprintk(level, fmt, arg...)\
+do {\
+if (debug >= level)\
+pr_info("vb2: %s: " fmt, __func__, ## arg);\


These are just whitespace changes, and that is something it see *a
lot* in this
patch. And usually for no clear reason.

Please remove those whitespace changes, it makes a difficult patch
even harder
to read than it already is.



I just wanted to remove unnecessary white spaces or adjust alignment.
OK, I will revert those whitespace changes for better review.


  } while (0)

  #ifdef CONFIG_VIDEO_ADV_DEBUG





@@ -656,12 +652,21 @@ static bool __buffers_in_use(struct vb2_queue *q)
   */
  static void __fill_v4l2_buffer(struct vb2_buffer *vb, struct
v4l2_buffer *b)
  {
+struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
  struct vb2_queue *q = vb->vb2_queue;
+unsigned int plane;

  /* Copy back data such as timestamp, flags, etc. */
-memcpy(b, >v4l2_buf, offsetof(struct v4l2_buffer, m));
-b->reserved2 = vb->v4l2_buf.reserved2;
-b->reserved = vb->v4l2_buf.reserved;


Hmm, I'm not sure why these reserved fields were copied here. I think
it was
for compatibility reasons for some old drivers that abused the
reserved field.
However, in the new code these reserved fields should probably be
explicitly
initialized to 0.


+b->index = vb->index;
+b->type = vb->type;
+b->memory = vb->memory;
+b->bytesused = 0;
+
+b->flags = vbuf->flags;
+b->field = vbuf->field;
+b->timestamp = vbuf->timestamp;
+b->timecode = vbuf->timecode;
+b->sequence = vbuf->sequence;

  if (V4L2_TYPE_IS_MULTIPLANAR(q->type)) {
  /*
@@ -669,21 +674,33 @@ static void __fill_v4l2_buffer(struct
vb2_buffer *vb, struct v4l2_buffer *b)
   * for it. The caller has already verified memory and size.
   */
  b->length = vb->num_planes;
-memcpy(b->m.planes, vb->v4l2_planes,
-b->length * sizeof(struct v4l2_plane));


A similar problem occurs here: the memcpy would have copied the reserved
field in v4l2_plane as well, but that no longer happens, so you need to
do an explicit memset for the reserved field in the new code.



It means that I'd better add reserved fields to struct vb2_buffer and
struct vb2_plane in order to keep the information in struct v4l2_buffer
and struct v4l2_plane???



Oh, I've mistaken your 

Re: [PATCH v8 40/55] [media] media.h: don't use legacy entity macros at Kernel

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Put the legacy MEDIA_ENT_* macros under a #ifndef __KERNEL__,
> in order to be sure that none of those old symbols are used
> inside the Kernel.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index cd486fc25f1e..4186891e5e81 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -107,6 +107,7 @@ struct media_device_info {
>  #define MEDIA_ENT_T_DVB_CA   (MEDIA_ENT_T_DVB_BASE + 3)
>  #define MEDIA_ENT_T_DVB_NET_DECAP(MEDIA_ENT_T_DVB_BASE + 4)
>  
> +#ifndef __KERNEL__
>  /* Legacy symbols used to avoid userspace compilation breakages */
>  #define MEDIA_ENT_TYPE_SHIFT 16
>  #define MEDIA_ENT_TYPE_MASK  0x00ff
> @@ -120,6 +121,7 @@ struct media_device_info {
>  #define MEDIA_ENT_T_DEVNODE_FB   (MEDIA_ENT_T_DEVNODE + 2)
>  #define MEDIA_ENT_T_DEVNODE_ALSA (MEDIA_ENT_T_DEVNODE + 3)
>  #define MEDIA_ENT_T_DEVNODE_DVB  (MEDIA_ENT_T_DEVNODE + 4)
> +#endif
>  
>  /* Entity types */
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] media: dvb-core: Don't force CAN_INVERSION_AUTO in oneshot mode.

[Malcolm Priestley]

When in FE_TUNE_MODE_ONESHOT the frontend must report
the actual capabilities so user can take appropriate
action.

With frontends that can't do auto inversion this is done
by dvb-core automatically so CAN_INVERSION_AUTO is valid.

However, when in FE_TUNE_MODE_ONESHOT this is not true.

So only set FE_CAN_INVERSION_AUTO in modes other than
FE_TUNE_MODE_ONESHOT

Signed-off-by: Malcolm Priestley 
Cc: 
---
 drivers/media/dvb-core/dvb_frontend.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index c38ef1a..e2a3833 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2313,9 +2313,9 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
dev_dbg(fe->dvb->device, "%s: current delivery system on cache: 
%d, V3 type: %d\n",
 __func__, c->delivery_system, 
fe->ops.info.type);
 
-   /* Force the CAN_INVERSION_AUTO bit on. If the frontend doesn't
-* do it, it is done for it. */
-   info->caps |= FE_CAN_INVERSION_AUTO;
+   /* Set CAN_INVERSION_AUTO bit on in other than oneshot mode */
+   if (!(fepriv->tune_mode_flags & FE_TUNE_MODE_ONESHOT))
+   info->caps |= FE_CAN_INVERSION_AUTO;
err = 0;
break;
}
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 23/55] [media] media: add support to link interfaces and entities

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Now that we have a new graph object called "interfaces", we
> need to be able to link them to the entities.
> 
> Add a linked list to the interfaces to allow them to be
> linked to the entities.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 973d1be427c5..08239128fbc4 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -869,6 +869,7 @@ struct media_intf_devnode *media_devnode_create(struct 
> media_device *mdev,
>  
>   intf->type = type;
>   intf->flags = flags;
> + INIT_LIST_HEAD(>links);
>  
>   devnode->major = major;
>   devnode->minor = minor;
> @@ -886,3 +887,40 @@ void media_devnode_remove(struct media_intf_devnode 
> *devnode)
>   kfree(devnode);
>  }
>  EXPORT_SYMBOL_GPL(media_devnode_remove);
> +
> +struct media_link *media_create_intf_link(struct media_entity *entity,
> + struct media_interface *intf,
> + u32 flags)
> +{
> + struct media_link *link;
> +
> + link = media_add_link(>links);
> + if (link == NULL)
> + return NULL;
> +
> + link->intf = intf;
> + link->entity = entity;
> + link->flags = flags;
> +
> + /* Initialize graph object embedded at the new link */
> + media_gobj_init(intf->graph_obj.mdev, MEDIA_GRAPH_LINK,
> + >graph_obj);
> +
> + return link;
> +}
> +EXPORT_SYMBOL_GPL(media_create_intf_link);
> +
> +
> +static void __media_remove_intf_link(struct media_link *link)
> +{
> + media_gobj_remove(>graph_obj);
> + kfree(link);
> +}
> +
> +void media_remove_intf_link(struct media_link *link)
> +{
> + mutex_lock(>graph_obj.mdev->graph_mutex);
> + __media_remove_intf_link(link);
> + mutex_unlock(>graph_obj.mdev->graph_mutex);

link was just freed, so you can't dereference link anymore.

Instead use a temp 'mdev' pointer to access the mutex.

Regards,

Hans

> +}
> +EXPORT_SYMBOL_GPL(media_remove_intf_link);
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index b4923a24efd5..423ff804e686 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -78,10 +78,12 @@ struct media_link {
>   union {
>   struct media_gobj *gobj0;
>   struct media_pad *source;
> + struct media_interface *intf;
>   };
>   union {
>   struct media_gobj *gobj1;
>   struct media_pad *sink;
> + struct media_entity *entity;
>   };
>   struct media_link *reverse; /* Link in the reverse direction */
>   unsigned long flags;/* Link flags (MEDIA_LNK_FL_*) */
> @@ -154,6 +156,7 @@ struct media_entity {
>   * struct media_intf_devnode - Define a Kernel API interface
>   *
>   * @graph_obj:   embedded graph object
> + * @links:   List of links pointing to graph entities
>   * @type:Type of the interface as defined at the
>   *   uapi/media/media.h header, e. g.
>   *   MEDIA_INTF_T_*
> @@ -161,6 +164,7 @@ struct media_entity {
>   */
>  struct media_interface {
>   struct media_gobj   graph_obj;
> + struct list_headlinks;
>   u32 type;
>   u32 flags;
>  };
> @@ -283,6 +287,11 @@ struct media_intf_devnode *media_devnode_create(struct 
> media_device *mdev,
>   u32 major, u32 minor,
>   gfp_t gfp_flags);
>  void media_devnode_remove(struct media_intf_devnode *devnode);
> +struct media_link *media_create_intf_link(struct media_entity *entity,
> + struct media_interface *intf,
> + u32 flags);
> +void media_remove_intf_link(struct media_link *link);
> +
>  #define media_entity_call(entity, operation, args...)
> \
>   (((entity)->ops && (entity)->ops->operation) ?  \
>(entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 24/55] [media] media-entity: add a helper function to create interface

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> As we'll be adding other interface types in the future, put the
> common interface create code on a separate function.
> 
> Suggested-by: Hans Verkuil 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 08239128fbc4..417673a32c21 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -851,6 +851,18 @@ struct media_pad *media_entity_remote_pad(struct 
> media_pad *pad)
>  EXPORT_SYMBOL_GPL(media_entity_remote_pad);
>  
>  
> +static void media_interface_init(struct media_device *mdev,
> +  struct media_interface *intf,
> +  u32 gobj_type,
> +  u32 intf_type, u32 flags)
> +{
> + intf->type = intf_type;
> + intf->flags = flags;
> + INIT_LIST_HEAD(>links);
> +
> + media_gobj_init(mdev, gobj_type, >graph_obj);
> +}
> +
>  /* Functions related to the media interface via device nodes */
>  
>  struct media_intf_devnode *media_devnode_create(struct media_device *mdev,
> @@ -859,23 +871,16 @@ struct media_intf_devnode *media_devnode_create(struct 
> media_device *mdev,
>   gfp_t gfp_flags)
>  {
>   struct media_intf_devnode *devnode;
> - struct media_interface *intf;
>  
>   devnode = kzalloc(sizeof(*devnode), gfp_flags);
>   if (!devnode)
>   return NULL;
>  
> - intf = >intf;
> -
> - intf->type = type;
> - intf->flags = flags;
> - INIT_LIST_HEAD(>links);
> -
>   devnode->major = major;
>   devnode->minor = minor;
>  
> - media_gobj_init(mdev, MEDIA_GRAPH_INTF_DEVNODE,
> ->intf.graph_obj);
> + media_interface_init(mdev, >intf, MEDIA_GRAPH_INTF_DEVNODE,
> +  type, flags);
>  
>   return devnode;
>  }
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 30/55] [media] replace all occurrences of MEDIA_ENT_T_DEVNODE_DVB

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Now that interfaces and entities are distinct, it makes no sense
> of keeping something named as MEDIA_ENT_T_DEVNODE_DVB_foo.
> 
> Made via this script:
>   for i in $(git grep -l MEDIA_ENT_T|grep -v uapi/linux/media.h); do sed 
> s,MEDIA_ENT_T_DEVNODE_DVB_,MEDIA_ENT_T_DVB_, <$i >a && mv a $i; done
>   for i in $(git grep -l MEDIA_ENT_T|grep -v uapi/linux/media.h); do sed 
> s,MEDIA_ENT_T_DVB_DVR,MEDIA_ENT_T_DVB_TSOUT, <$i >a && mv a $i; done
>   for i in $(git grep -l MEDIA_ENT_T|grep -v uapi/linux/media.h); do sed 
> s,MEDIA_ENT_T_DVB_FE,MEDIA_ENT_T_DVB_DEMOD, <$i >a && mv a $i; done
>   for i in $(git grep -l MEDIA_ENT_T|grep -v uapi/linux/media.h); do sed 
> s,MEDIA_ENT_T_DVB_NET,MEDIA_ENT_T_DVB_DEMOD_NET_DECAP, <$i >a && mv a $i; done
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml 
> b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> index 910243d4edb8..32a783635649 100644
> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> @@ -195,23 +195,23 @@
>   ALSA card
> 
> 
> - MEDIA_ENT_T_DEVNODE_DVB_FE
> + MEDIA_ENT_T_DVB_DEMOD
>   DVB frontend devnode
> 
> 
> - MEDIA_ENT_T_DEVNODE_DVB_DEMUX
> + MEDIA_ENT_T_DVB_DEMUX
>   DVB demux devnode
> 
> 
> - MEDIA_ENT_T_DEVNODE_DVB_DVR
> + MEDIA_ENT_T_DVB_TSOUT
>   DVB DVR devnode
> 
> 
> - MEDIA_ENT_T_DEVNODE_DVB_CA
> + MEDIA_ENT_T_DVB_CA
>   DVB CAM devnode
> 
> 
> - MEDIA_ENT_T_DEVNODE_DVB_NET
> + MEDIA_ENT_T_DVB_DEMOD_NET_DECAP
>   DVB network devnode
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 29/55] [media] replace all occurrences of MEDIA_ENT_T_DEVNODE_V4L

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Now that interfaces and entities are distinct, it makes no sense
> of keeping something named as MEDIA_ENT_T_DEVNODE.
> 
> This change was done with this script:
> 
>   for i in $(git grep -l MEDIA_ENT_T|grep -v uapi/linux/media.h); do sed 
> s,MEDIA_ENT_T_DEVNODE_V4L,MEDIA_ENT_T_V4L2_VIDEO, <$i >a && mv a $i; done
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml 
> b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> index 5872f8bbf774..910243d4edb8 100644
> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> @@ -183,7 +183,7 @@
>   Unknown device node
> 
> 
> - MEDIA_ENT_T_DEVNODE_V4L
> + MEDIA_ENT_T_V4L2_VIDEO
>   V4L video, radio or vbi device node
> 
> 
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c 
> b/drivers/media/platform/xilinx/xilinx-dma.c
> index 92e8116dc28f..88cd789cdaf7 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.c
> +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> @@ -193,7 +193,7 @@ static int xvip_pipeline_validate(struct xvip_pipeline 
> *pipe,
>   while ((entity = media_entity_graph_walk_next())) {
>   struct xvip_dma *dma;
>  
> - if (entity->type != MEDIA_ENT_T_DEVNODE_V4L)
> + if (entity->type != MEDIA_ENT_T_V4L2_VIDEO)
>   continue;
>  
>   dma = to_xvip_dma(media_entity_to_video_device(entity));
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
> b/drivers/media/v4l2-core/v4l2-dev.c
> index 71a1b93b0790..44b330589787 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -912,7 +912,7 @@ int __video_register_device(struct video_device *vdev, 
> int type, int nr,
>   /* Part 5: Register the entity. */
>   if (vdev->v4l2_dev->mdev &&
>   vdev->vfl_type != VFL_TYPE_SUBDEV) {
> - vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L;
> + vdev->entity.type = MEDIA_ENT_T_V4L2_VIDEO;
>   vdev->entity.name = vdev->name;
>   vdev->entity.info.dev.major = VIDEO_MAJOR;
>   vdev->entity.info.dev.minor = vdev->minor;
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
> b/drivers/media/v4l2-core/v4l2-subdev.c
> index 83615b8fb46a..e6e1115d8215 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -535,7 +535,7 @@ v4l2_subdev_link_validate_get_format(struct media_pad 
> *pad,
>   return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);
>   }
>  
> - WARN(pad->entity->type != MEDIA_ENT_T_DEVNODE_V4L,
> + WARN(pad->entity->type != MEDIA_ENT_T_V4L2_VIDEO,
>"Driver bug! Wrong media entity type 0x%08x, entity %s\n",
>pad->entity->type, pad->entity->name);
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 31/55] [media] media: add macros to check if subdev or V4L2 DMA

[Hans Verkuil]

On 08/31/2015 01:31 PM, Hans Verkuil wrote:
> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
>> As we'll be removing entity subtypes from the Kernel, we need
>> to provide a way for drivers and core to check if a given
>> entity is represented by a V4L2 subdev or if it is an V4L2
>> I/O entity (typically with DMA).
> 
> This needs more discussion. The plan (as I understand it) is to have 
> properties
> that describe the entity's functionalities.
> 
> The existing entity subtypes will exist only as backwards compat types, but in
> the future properties should be used to describe the functionalities.
> 
> This raises the question if we shouldn't use MEDIA_ENT_T_V4L2_SUBDEV to tell
> userspace that this is a subdev-controlled entity, and let userspace look at
> the properties to figure out what it is exactly?
> 
> It could be that this is a transitional patch, and this will be fixed later.
> If so, this should be mentioned in the commit message.

FYI, I'm skipping patches that change the MEDIA_ENT_T_V4L2_SUBDEV usage until
I understand the long-term plan for this define as discussed above.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 14/55] [media] media: add functions to allow creating interfaces

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Interfaces are different than entities: they represent a
> Kernel<->userspace interaction, while entities represent a
> piece of hardware/firmware/software that executes a function.
> 
> Let's distinguish them by creating a separate structure to
> store the interfaces.
> 
> Latter patches should change the existing drivers and logic

s/Latter/Later/

FYI: you almost never use 'latter' in English texts. The only common use
of 'latter' is in the phrase "the latter of two options" where 'latter' means
'second'.

> to split the current interface embedded inside the entity
> structure (device nodes) into a separate object of the graph.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index a23c93369a04..583666e2cc25 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -44,11 +44,41 @@ static inline const char *gobj_type(enum media_gobj_type 
> type)
>   return "pad";
>   case MEDIA_GRAPH_LINK:
>   return "link";
> + case MEDIA_GRAPH_INTF_DEVNODE:
> + return "intf_devnode";

You use '-' below for 'v4l-subdev' and 'unknown-intf', so I think this too 
should
use '-'. My opinion though.

It's a nitpick, so I give my Ack anyway:

Acked-by: Hans Verkuil 

Regards,

Hans

>   default:
>   return "unknown";
>   }
>  }
>  
> +static inline const char *intf_type(struct media_interface *intf)
> +{
> + switch (intf->type) {
> + case MEDIA_INTF_T_DVB_FE:
> + return "frontend";
> + case MEDIA_INTF_T_DVB_DEMUX:
> + return "demux";
> + case MEDIA_INTF_T_DVB_DVR:
> + return "DVR";
> + case MEDIA_INTF_T_DVB_CA:
> + return  "CA";
> + case MEDIA_INTF_T_DVB_NET:
> + return "dvbnet";
> + case MEDIA_INTF_T_V4L_VIDEO:
> + return "video";
> + case MEDIA_INTF_T_V4L_VBI:
> + return "vbi";
> + case MEDIA_INTF_T_V4L_RADIO:
> + return "radio";
> + case MEDIA_INTF_T_V4L_SUBDEV:
> + return "v4l2-subdev";
> + case MEDIA_INTF_T_V4L_SWRADIO:
> + return "swradio";
> + default:
> + return "unknown-intf";
> + }
> +};
> +
>  static void dev_dbg_obj(const char *event_name,  struct media_gobj *gobj)
>  {
>  #if defined(DEBUG) || defined (CONFIG_DYNAMIC_DEBUG)
> @@ -84,6 +114,19 @@ static void dev_dbg_obj(const char *event_name,  struct 
> media_gobj *gobj)
>   "%s: id 0x%08x pad#%d: '%s':%d\n",
>   event_name, gobj->id, media_localid(gobj),
>   pad->entity->name, pad->index);
> + break;
> + }
> + case MEDIA_GRAPH_INTF_DEVNODE:
> + {
> + struct media_interface *intf = gobj_to_intf(gobj);
> + struct media_intf_devnode *devnode = intf_to_devnode(intf);
> +
> + dev_dbg(gobj->mdev->dev,
> + "%s: id 0x%08x intf_devnode#%d: %s - major: %d, minor: 
> %d\n",
> + event_name, gobj->id, media_localid(gobj),
> + intf_type(intf),
> + devnode->major, devnode->minor);
> + break;
>   }
>   }
>  #endif
> @@ -119,6 +162,9 @@ void media_gobj_init(struct media_device *mdev,
>   case MEDIA_GRAPH_LINK:
>   gobj->id = media_gobj_gen_id(type, ++mdev->link_id);
>   break;
> + case MEDIA_GRAPH_INTF_DEVNODE:
> + gobj->id = media_gobj_gen_id(type, ++mdev->intf_devnode_id);
> + break;
>   }
>   dev_dbg_obj(__func__, gobj);
>  }
> @@ -793,3 +839,40 @@ struct media_pad *media_entity_remote_pad(struct 
> media_pad *pad)
>  
>  }
>  EXPORT_SYMBOL_GPL(media_entity_remote_pad);
> +
> +
> +/* Functions related to the media interface via device nodes */
> +
> +struct media_intf_devnode *media_devnode_create(struct media_device *mdev,
> + u32 type, u32 flags,
> + u32 major, u32 minor,
> + gfp_t gfp_flags)
> +{
> + struct media_intf_devnode *devnode;
> + struct media_interface *intf;
> +
> + devnode = kzalloc(sizeof(*devnode), gfp_flags);
> + if (!devnode)
> + return NULL;
> +
> + intf = >intf;
> +
> + intf->type = type;
> + intf->flags = flags;
> +
> + devnode->major = major;
> + devnode->minor = minor;
> +
> + media_gobj_init(mdev, MEDIA_GRAPH_INTF_DEVNODE,
> +>intf.graph_obj);
> +
> + return devnode;
> +}
> +EXPORT_SYMBOL_GPL(media_devnode_create);
> +
> +void media_devnode_remove(struct media_intf_devnode *devnode)
> +{
> + media_gobj_remove(>intf.graph_obj);
> + kfree(devnode);
> +}
> +EXPORT_SYMBOL_GPL(media_devnode_remove);
> diff 

Re: [PATCH v8 28/55] [media] uapi/media.h: Fix entity namespace

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Now that interfaces got created, we need to fix the entity
> namespace.
> 
> So, let's create a consistent new namespace and add backward
> compatibility macros to keep the old namespace preserved.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 14d32cdcdd47..88013d1a2c39 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -230,17 +230,17 @@ static void dvb_create_media_entity(struct dvb_device 
> *dvbdev,
>  
>   switch (type) {
>   case DVB_DEVICE_FRONTEND:
> - dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
> + dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMOD;
>   dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
>   dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
>   break;
>   case DVB_DEVICE_DEMUX:
> - dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
> + dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMUX;
>   dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
>   dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
>   break;
>   case DVB_DEVICE_CA:
> - dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
> + dvbdev->entity->type = MEDIA_ENT_T_DVB_CA;
>   dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
>   dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
>   break;
> @@ -439,7 +439,7 @@ EXPORT_SYMBOL(dvb_unregister_device);
>  void dvb_create_media_graph(struct dvb_adapter *adap)
>  {
>   struct media_device *mdev = adap->mdev;
> - struct media_entity *entity, *tuner = NULL, *fe = NULL;
> + struct media_entity *entity, *tuner = NULL, *demod = NULL;
>   struct media_entity *demux = NULL, *dvr = NULL, *ca = NULL;
>   struct media_interface *intf;
>  
> @@ -451,26 +451,26 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
>   case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
>   tuner = entity;
>   break;
> - case MEDIA_ENT_T_DEVNODE_DVB_FE:
> - fe = entity;
> + case MEDIA_ENT_T_DVB_DEMOD:
> + demod = entity;
>   break;
> - case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
> + case MEDIA_ENT_T_DVB_DEMUX:
>   demux = entity;
>   break;
> - case MEDIA_ENT_T_DEVNODE_DVB_DVR:
> + case MEDIA_ENT_T_DVB_TSOUT:
>   dvr = entity;
>   break;
> - case MEDIA_ENT_T_DEVNODE_DVB_CA:
> + case MEDIA_ENT_T_DVB_CA:
>   ca = entity;
>   break;
>   }
>   }
>  
> - if (tuner && fe)
> - media_create_pad_link(tuner, 0, fe, 0, 0);
> + if (tuner && demod)
> + media_create_pad_link(tuner, 0, demod, 0, 0);
>  
> - if (fe && demux)
> - media_create_pad_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
> + if (demod && demux)
> + media_create_pad_link(demod, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
>  
>   if (demux && dvr)
>   media_create_pad_link(demux, 1, dvr, 0, MEDIA_LNK_FL_ENABLED);
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index aca828709bad..3bbda409353f 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -42,31 +42,71 @@ struct media_device_info {
>  
>  #define MEDIA_ENT_ID_FLAG_NEXT   (1 << 31)
>  
> +/*
> + * Base numbers for entity types
> + *
> + * Please notice that the huge gap of 16 bits for each base is overkill!
> + * 8 bits is more than enough to avoid starving entity types for each
> + * subsystem.
> + *
> + * However, It is kept this way just to avoid binary breakages with the
> + * namespace provided on legacy versions of this header.
> + */
> +#define MEDIA_ENT_T_DVB_BASE 0x0001

I would change this to 0x, see follow-up comment later for why.

> +#define MEDIA_ENT_T_V4L2_BASE0x0001
> +#define MEDIA_ENT_T_V4L2_SUBDEV_BASE 0x0002
> +
> +/*
> + * V4L2 entities - Those are used for DMA (mmap/DMABUF) and
> + *   read()/write() data I/O associated with the V4L2 devnodes.
> + */
> +#define MEDIA_ENT_T_V4L2_VIDEO   (MEDIA_ENT_T_V4L2_BASE + 1)
> + /*
> +  * Please notice that numbers between MEDIA_ENT_T_V4L2_BASE + 2 and
> +  * MEDIA_ENT_T_V4L2_BASE + 4 can't be used, as those values used
> +  * to be declared for FB, ALSA and DVB entities.
> +  * As those values were never actually used in practice, we're just
> +  * adding them as backward compatibility macros and keeping the
> +  * numberspace clean here. This way, we avoid breaking compilation,
> +  * in the case of having some userspace 

Re: Linux 4.2 ALSA snd-usb-audio inconsistent lock state warn in PCM nonatomic mode

[Takashi Iwai]

On Tue, 01 Sep 2015 00:48:30 +0200,
Shuah Khan wrote:
> 
> Hi Takashi,
> 
> I am seeing the following inconsistent lock state warning when PCM
> is run in nonatomic mode. This is on 4.2.0 and with the following
> change to force PCM on nonatomic mode:
> 
> diff --git a/sound/usb/stream.c b/sound/usb/stream.c
> index 310a382..16bbb71 100644
> --- a/sound/usb/stream.c
> +++ b/sound/usb/stream.c
> @@ -370,6 +370,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
> pcm->private_data = as;
> pcm->private_free = snd_usb_audio_pcm_free;
> pcm->info_flags = 0;
> +   pcm->nonatomic = true;
> if (chip->pcm_devs > 0)
> sprintf(pcm->name, "USB Audio #%d", chip->pcm_devs);
> else
> 
> The device Bus 003 Device 002: ID 2040:7200 Hauppauge
> Please let me know if you need more information and any ideas on
> how to fix this problem.

The USB-audio can't be used in the non-atomic mode.
snd_pcm_period_elapsed() is handled in the complete callback that is
already atomic.


Takashi

> 
> thanks,
> -- Shuah
> 
> [  120.283960] =
> [  120.283964] [ INFO: inconsistent lock state ]
> [  120.283968] 4.2.0+ #29 Not tainted
> [  120.283972] -
> [  120.283975] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
> [  120.283980] swapper/1/0 [HC1[1]:SC0[0]:HE0:SE1] takes:
> [  120.283983]  (&(>lock)->rlock){?.+...}, at:
> [] retire_capture_urb+0x140/0x2b0 [snd_usb_audio]
> [  120.284005] {HARDIRQ-ON-W} state was registered at:
> [  120.284008]   [] __lock_acquire+0xc50/0x2380
> [  120.284016]   [] lock_acquire+0xb1/0x130
> [  120.284022]   [] _raw_spin_lock+0x31/0x40
> [  120.284028]   [] snd_usb_pcm_pointer+0x5d/0xc0
> [snd_usb_audio]
> [  120.284040]   [] snd_pcm_update_hw_ptr0+0x38/0x3a0
> [snd_pcm]
> [  120.284052]   [] snd_pcm_update_hw_ptr+0x10/0x20
> [snd_pcm]
> [  120.284063]   [] snd_pcm_hwsync+0x45/0xa0 [snd_pcm]
> [  120.284071]   [] snd_pcm_common_ioctl1+0x277/0xce0
> [snd_pcm]
> [  120.284081]   [] snd_pcm_capture_ioctl1+0x1be/0x2d0
> [snd_pcm]
> [  120.284090]   [] snd_pcm_capture_ioctl+0x34/0x40
> [snd_pcm]
> [  120.284100]   [] do_vfs_ioctl+0x301/0x560
> [  120.284107]   [] SyS_ioctl+0x79/0x90
> [  120.284112]   [] entry_SYSCALL_64_fastpath+0x12/0x6f
> [  120.284119] irq event stamp: 823304
> [  120.284122] hardirqs last  enabled at (823301): []
> cpuidle_enter_state+0xed/0x230
> [  120.284129] hardirqs last disabled at (823302): []
> common_interrupt+0x68/0x6d
> [  120.284135] softirqs last  enabled at (823304): []
> _local_bh_enable+0x21/0x50
> [  120.284139] softirqs last disabled at (823303): []
> irq_enter+0x4c/0x70
> [  120.284143]
> other info that might help us debug this:
> [  120.284146]  Possible unsafe locking scenario:
> 
> [  120.284149]CPU0
> [  120.284150]
> [  120.284152]   lock(&(>lock)->rlock);
> [  120.284155]   
> [  120.284157] lock(&(>lock)->rlock);
> [  120.284160]
>  *** DEADLOCK ***
> [  120.284163] no locks held by swapper/1/0.
> [  120.284165]
> stack backtrace:
> [  120.284170] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.2.0+ #29
> [  120.284173] Hardware name: Hewlett-Packard HP ProBook 6475b/180F,
> BIOS 68TTU Ver. F.04 08/03/2012
> [  120.284176]  828d5630 88023ec83a68 817f4ea0
> 0007
> [  120.284181]  880235a6a500 88023ec83ac8 810ad97f
> 
> [  120.284186]   88020001 810134df
> 827c5390
> [  120.284192] Call Trace:
> [  120.284194][] dump_stack+0x45/0x57
> [  120.284203]  [] print_usage_bug+0x1ff/0x210
> [  120.284209]  [] ? save_stack_trace+0x2f/0x50
> [  120.284214]  [] mark_lock+0x66e/0x6f0
> [  120.284218]  [] ?
> print_shortest_lock_dependencies+0x1d0/0x1d0
> [  120.284222]  [] __lock_acquire+0xdcb/0x2380
> [  120.284226]  [] ? __enqueue_entity+0x6c/0x70
> [  120.284230]  [] ? __lock_is_held+0x4d/0x70
> [  120.284234]  [] ? __lock_is_held+0x4d/0x70
> [  120.284238]  [] ? __lock_is_held+0x4d/0x70
> [  120.284242]  [] ? __lock_is_held+0x4d/0x70
> [  120.284246]  [] lock_acquire+0xb1/0x130
> [  120.284256]  [] ? retire_capture_urb+0x140/0x2b0
> [snd_usb_audio]
> [  120.284261]  [] _raw_spin_lock_irqsave+0x3c/0x50
> [  120.284270]  [] ? retire_capture_urb+0x140/0x2b0
> [snd_usb_audio]
> [  120.284276]  [] ? usb_hcd_get_frame_number+0x25/0x30
> [  120.284285]  [] retire_capture_urb+0x140/0x2b0
> [snd_usb_audio]
> [  120.284294]  [] snd_complete_urb+0x13c/0x250
> [snd_usb_audio]
> [  120.284298]  [] __usb_hcd_giveback_urb+0x72/0x110
> [  120.284303]  [] usb_hcd_giveback_urb+0x43/0x140
> [  120.284307]  [] xhci_irq+0xd42/0x1fc0
> [  120.284312]  [] xhci_msi_irq+0x11/0x20
> [  120.284317]  [] handle_irq_event_percpu+0x80/0x1a0
> [  120.284322]  [] handle_irq_event+0x4a/0x70
> [  120.284325]  [] ? handle_edge_irq+0x24/0x150
> [  120.284329]  [] handle_edge_irq+0x81/0x150
> [  120.284333]  [] handle_irq+0x25/0x40
> [  120.284337] 

Re: [PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Add a new ioctl that will report the entire topology on
> one go.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 756e1960fd7f..358a0c6b1f86 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -181,6 +181,8 @@ struct media_interface {
>   */
>  struct media_intf_devnode {
>   struct media_interface  intf;
> +
> + /* Should match the fields at media_v2_intf_devnode */
>   u32 major;
>   u32 minor;
>  };
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 4186891e5e81..fa0b68e670b0 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -251,11 +251,94 @@ struct media_links_enum {
>  #define MEDIA_INTF_T_ALSA_RAWMIDI   (MEDIA_INTF_T_ALSA_BASE + 4)
>  #define MEDIA_INTF_T_ALSA_HWDEP (MEDIA_INTF_T_ALSA_BASE + 5)
>  
> -/* TBD: declare the structs needed for the new G_TOPOLOGY ioctl */
> +/*
> + * MC next gen API definitions
> + *
> + * NOTE: The declarations below are close to the MC RFC for the Media
> + *Controller, the next generation. Yet, there are a few adjustments
> + *to do, as we want to be able to have a functional API before
> + *the MC properties change. Those will be properly marked below.
> + *Please also notice that I removed "num_pads", "num_links",
> + *from the proposal, as a proper userspace application will likely
> + *use lists for pads/links, just as we intend todo in Kernelspace.

s/todo/to do/

> + *The API definition should be freed from fields that are bound to
> + *some specific data structure.
> + *
> + * FIXME: Currently, I opted to name the new types as "media_v2", as this
> + * won't cause any conflict with the Kernelspace namespace, nor with
> + * the previous kAPI media_*_desc namespace. This can be changed
> + * latter, before the adding this API upstream.

s/latter/later/ :-)

I think this comment belongs to the commit log and not in this header.

> + */
> +
> +
> +#define MEDIA_NEW_LNK_FL_ENABLED MEDIA_LNK_FL_ENABLED
> +#define MEDIA_NEW_LNK_FL_IMMUTABLE   MEDIA_LNK_FL_IMMUTABLE
> +#define MEDIA_NEW_LNK_FL_DYNAMIC MEDIA_NEW_FL_DYNAMIC
> +#define MEDIA_NEW_LNK_FL_INTERFACE_LINK  (1 << 3)

Shouldn't this be MEDIA_V2_ instead of MEDIA_NEW_?

Do we need the INTERFACE_LINK flag? You can deduce it by checking the
ID type.

I don't have a clear preference one way or another, just wondering about the
reason for adding it.

> +
> +struct media_v2_entity {
> + __u32 id;
> + char name[64];  /* FIXME: move to a property? (RFC says so) */
> + __u16 reserved[14];
> +};
> +
> +/* Should match the specific fields at media_intf_devnode */
> +struct media_v2_intf_devnode {
> + __u32 major;
> + __u32 minor;
> +};
> +
> +struct media_v2_interface {
> + __u32 id;
> + __u32 intf_type;
> + __u32 flags;
> + __u32 reserved[9];
> +
> + union {
> + struct media_v2_intf_devnode devnode;
> + __u32 raw[16];
> + };
> +};
> +
> +struct media_v2_pad {
> + __u32 id;
> + __u32 entity_id;
> + __u32 flags;
> + __u16 reserved[9];
> +};
> +
> +struct media_v2_link {
> +__u32 id;
> +__u32 source_id;
> +__u32 sink_id;

Like in media_link I would use a union here as well to be able to refer to
source/sink_id and entity/interface_id.

> +__u32 flags;
> +__u32 reserved[5];
> +};
> +
> +struct media_v2_topology {
> + __u32 topology_version;
> +
> + __u32 num_entities;
> + struct media_v2_entity *entities;
> +
> + __u32 num_interfaces;
> + struct media_v2_interface *interfaces;
> +
> + __u32 num_pads;
> + struct media_v2_pad *pads;
> +
> + __u32 num_links;
> + struct media_v2_link *links;
> +
> + __u32 reserved[64];

As mentioned before: use this instead to prevent horrible 32/64 bit arch
compat code:

struct {
__u32 reserved_num;
void *reserved_ptr;
} reserved_types[16];
__u32 reserved[8];

Sizes for these arrays are TBD.

> +};
> +
> +/* ioctls */
>  
>  #define MEDIA_IOC_DEVICE_INFO_IOWR('|', 0x00, struct 
> media_device_info)
>  #define MEDIA_IOC_ENUM_ENTITIES  _IOWR('|', 0x01, struct 
> media_entity_desc)
>  #define MEDIA_IOC_ENUM_LINKS _IOWR('|', 0x02, struct 
> media_links_enum)
>  #define MEDIA_IOC_SETUP_LINK _IOWR('|', 0x03, struct media_link_desc)
> +#define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct 
> media_v2_topology)
>  
>  #endif /* __LINUX_MEDIA_H */
> 

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

[PATCH] v4l2-compat-ioctl32: fix alignment for ARM64

[Andrzej Hajda]

Alignment/padding rules on AMD64 and ARM64 differs. To allow properly match
compatible ioctls on ARM64 kernels without breaking AMD64 some fields
should be aligned using compat_s64 type and in one case struct should be
unpacked.

Signed-off-by: Andrzej Hajda 
---
Hi Hans,

I have tested following structures:
struct v4l2_format32
struct v4l2_buffer32
struct v4l2_framebuffer32
struct v4l2_standard32
struct v4l2_input32
struct v4l2_edid32
struct v4l2_ext_controls32
struct v4l2_ext_control32
struct v4l2_event32
struct v4l2_create_buffers32

Following structures should be fixed:
v4l2_standard32 - .id alignment
v4l2_input32 - whole struct padding
v4l2_event32 - .data alignment

I hope this patch does it correctly.

Regards
Andrzej
---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index af63543..d309d270 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -266,7 +266,7 @@ static int put_v4l2_create32(struct v4l2_create_buffers 
*kp, struct v4l2_create_
 
 struct v4l2_standard32 {
__u32index;
-   __u32id[2]; /* __u64 would get the alignment wrong */
+   compat_u64   id;
__u8 name[24];
struct v4l2_fractframeperiod; /* Frames, not fields */
__u32framelines;
@@ -286,7 +286,7 @@ static int put_v4l2_standard32(struct v4l2_standard *kp, 
struct v4l2_standard32
 {
if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_standard32)) ||
put_user(kp->index, >index) ||
-   copy_to_user(up->id, >id, sizeof(__u64)) ||
+   put_user(kp->id, >id) ||
copy_to_user(up->name, kp->name, 24) ||
copy_to_user(>frameperiod, >frameperiod, 
sizeof(kp->frameperiod)) ||
put_user(kp->framelines, >framelines) ||
@@ -587,10 +587,10 @@ struct v4l2_input32 {
__u32type;  /*  Type of input */
__u32audioset;  /*  Associated audios (bitfield) */
__u32tuner; /*  Associated tuner */
-   v4l2_std_id  std;
+   compat_s64   std;
__u32status;
__u32reserved[4];
-} __attribute__ ((packed));
+};
 
 /* The 64-bit v4l2_input struct has extra padding at the end of the struct.
Otherwise it is identical to the 32-bit version. */
@@ -738,6 +738,7 @@ static int put_v4l2_ext_controls32(struct v4l2_ext_controls 
*kp, struct v4l2_ext
 struct v4l2_event32 {
__u32   type;
union {
+   compat_s64  value64;
__u8data[64];
} u;
__u32   pending;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 47/55] [media] media-device: add pads and links to media_device

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> The MC next gen API sends objects to userspace grouped by
> their types.
> 
> In the case of pads and links, in order to improve performance
> and have a simpler code, the best is to store them also on
> separate linked lists at MC.
> 
> If we don't do that, we would need this kind of interaction
> to send data to userspace (code is in structured english):
> 
>   for each entity:
>   for each pad:
>   store pads
> 
>   for each entity:
>   for each link:
>   store link
> 
>   for each interface:
>   for each link:
>   store link
> 
> With would require one nexted loop for pads and two nested
> loops for links. By using  separate linked lists for them,
> just one loop would be enough.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index ec98595b8a7a..5b2c9f7fcd45 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -382,6 +382,8 @@ int __must_check __media_device_register(struct 
> media_device *mdev,
>  
>   INIT_LIST_HEAD(>entities);
>   INIT_LIST_HEAD(>interfaces);
> + INIT_LIST_HEAD(>pads);
> + INIT_LIST_HEAD(>links);
>   spin_lock_init(>lock);
>   mutex_init(>graph_mutex);
>  
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 192af193a394..c89f51bc688d 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -174,13 +174,15 @@ void media_gobj_init(struct media_device *mdev,
>   break;
>   case MEDIA_GRAPH_PAD:
>   gobj->id = media_gobj_gen_id(type, ++mdev->pad_id);
> + list_add_tail(>list, >pads);
>   break;
>   case MEDIA_GRAPH_LINK:
>   gobj->id = media_gobj_gen_id(type, ++mdev->link_id);
> + list_add_tail(>list, >links);
>   break;
>   case MEDIA_GRAPH_INTF_DEVNODE:
> - list_add_tail(>list, >interfaces);
>   gobj->id = media_gobj_gen_id(type, ++mdev->intf_devnode_id);
> + list_add_tail(>list, >interfaces);
>   break;
>   }
>   dev_dbg_obj(__func__, gobj);
> @@ -195,16 +197,10 @@ void media_gobj_init(struct media_device *mdev,
>   */
>  void media_gobj_remove(struct media_gobj *gobj)
>  {
> + dev_dbg_obj(__func__, gobj);
> +
>   /* Remove the object from mdev list */
> - switch (media_type(gobj)) {
> - case MEDIA_GRAPH_ENTITY:
> - case MEDIA_GRAPH_INTF_DEVNODE:
> - list_del(>list);
> - default:
> - break;
> - }
> -
> - dev_dbg_obj(__func__, gobj);
> + list_del(>list);
>  }
>  
>  /**
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index 85fa302047bd..0d1b9c687454 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -47,6 +47,8 @@ struct device;
>   * @intf_devnode_id: Unique ID used on the last interface devnode registered
>   * @entities:List of registered entities
>   * @interfaces:  List of registered interfaces
> + * @pads:List of registered pads
> + * @links:   List of registered links
>   * @lock:Entities list lock
>   * @graph_mutex: Entities graph operation lock
>   * @link_notify: Link state change notification callback
> @@ -79,6 +81,8 @@ struct media_device {
>  
>   struct list_head entities;
>   struct list_head interfaces;
> + struct list_head pads;
> + struct list_head links;
>  
>   /* Protects the entities list */
>   spinlock_t lock;
> @@ -117,6 +121,14 @@ struct media_device *media_device_find_devres(struct 
> device *dev);
>  #define media_device_for_each_intf(intf, mdev)   \
>   list_for_each_entry(intf, &(mdev)->interfaces, graph_obj.list)
>  
> +/* Iterate over all pads. */
> +#define media_device_for_each_pad(pad, mdev) \
> + list_for_each_entry(pad, &(mdev)->pads, graph_obj.list)
> +
> +/* Iterate over all links. */
> +#define media_device_for_each_link(link, mdev)   \
> + list_for_each_entry(link, &(mdev)->links, graph_obj.list)
> +
>  
>  #else
>  static inline int media_device_register(struct media_device *mdev)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

muxing ES to mpeg-ts

[Ran Shalit]

Hello,

I would please like to ask what is a good choice for muxing ES to mpeg
transport stream. It is required to do this in application (muxing the
encoder output into mpeg-ts which is transffered in ethernet udp).

I know that both ffmpeg and opencaster can support this.

What do you think will be a good choice for this ? (simplicity to
integrate in code, latency, debug, etc)

Regards,
Ran
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 31/55] [media] media: add macros to check if subdev or V4L2 DMA

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 13:31:32 +0200
Hans Verkuil  escreveu:

> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> > As we'll be removing entity subtypes from the Kernel, we need
> > to provide a way for drivers and core to check if a given
> > entity is represented by a V4L2 subdev or if it is an V4L2
> > I/O entity (typically with DMA).
> 
> This needs more discussion. The plan (as I understand it) is to have 
> properties
> that describe the entity's functionalities.
> 
> The existing entity subtypes will exist only as backwards compat types, but in
> the future properties should be used to describe the functionalities.
> 
> This raises the question if we shouldn't use MEDIA_ENT_T_V4L2_SUBDEV to tell
> userspace that this is a subdev-controlled entity, and let userspace look at
> the properties to figure out what it is exactly?
> 
> It could be that this is a transitional patch, and this will be fixed later.
> If so, this should be mentioned in the commit message.

There are several different issues here:

1) drivers should not rely on type/subtype at MEDIA_ENT_T_*, as we need
   to get rid of it, as this got deprecated;

2) Keep backward compatibility.

3) the addition of properties;

The next patch on this series addresses (1) and (2), and this change is
needed for them.

We can't tell if this is a transitional patch or not, as we don't have
yet any idea about how (3) will be added. Maybe it will preserve
entity->type. Maybe it will convert it into an array. Maybe it would
do something different.

In any case, whatever the property patches will be doing, after this
patch, it will need to touch only at the implementation of the two
macros, not needing to touch at the drivers again.

What I can do is to tell at the description that this patch is in
preparation for the removal of media type/subtype concept that will 
happen on the next patches, but this is what's described there already
at:
"As we'll be removing entity subtypes from the Kernel,"

Perhaps I should let it clearer there.

Regards,
Mauro

> 
> Regards,
> 
>   Hans
> 
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > 
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index e7b20bdc735d..b0cfbc0dffc7 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -220,6 +220,39 @@ static inline u32 media_gobj_gen_id(enum 
> > media_gobj_type type, u32 local_id)
> > return id;
> >  }
> >  
> > +static inline bool is_media_entity_v4l2_io(struct media_entity *entity)
> > +{
> > +   if (!entity)
> > +   return false;
> > +
> > +   switch (entity->type) {
> > +   case MEDIA_ENT_T_V4L2_VIDEO:
> > +   case MEDIA_ENT_T_V4L2_VBI:
> > +   case MEDIA_ENT_T_V4L2_SWRADIO:
> > +   return true;
> > +   default:
> > +   return false;
> > +   }
> > +}
> > +
> > +static inline bool is_media_entity_v4l2_subdev(struct media_entity *entity)
> > +{
> > +   if (!entity)
> > +   return false;
> > +
> > +   switch (entity->type) {
> > +   case MEDIA_ENT_T_V4L2_SUBDEV_SENSOR:
> > +   case MEDIA_ENT_T_V4L2_SUBDEV_FLASH:
> > +   case MEDIA_ENT_T_V4L2_SUBDEV_LENS:
> > +   case MEDIA_ENT_T_V4L2_SUBDEV_DECODER:
> > +   case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
> > +   return true;
> > +
> > +   default:
> > +   return false;
> > +   }
> > +}
> > +
> >  #define MEDIA_ENTITY_ENUM_MAX_DEPTH16
> >  #define MEDIA_ENTITY_ENUM_MAX_ID   64
> >  
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 53/55] [media] v4l2-core: create MC interfaces for devnodes

[Hans Verkuil]

On 08/30/2015 05:07 AM, Mauro Carvalho Chehab wrote:
> V4L2 device (and subdevice) nodes should create an
> interface, if the Media Controller support is enabled.
> 
> Please notice that radio devices should not create an
> entity, as radio input/output is either via wires or
> via ALSA.

A general note: I think this patch (and any prerequisite patches) should come 
before
the patches adding G_TOPOLOGY support.

What the G_TOPOLOGY ioctl returns only makes sense IMHO if this code is present 
as well,
so it is a more logical order for this patch series.

In addition, since G_TOPOLOGY is a userspace API it is likely that that will 
create
more discussions, whereas this is internal to the kernel and could be merged 
before
G_TOPOLOGY.

> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
> b/drivers/media/v4l2-core/v4l2-dev.c
> index 44b330589787..427a5a32b3de 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -194,9 +194,12 @@ static void v4l2_device_release(struct device *cd)
>   mutex_unlock(_lock);
>  
>  #if defined(CONFIG_MEDIA_CONTROLLER)
> - if (v4l2_dev->mdev &&
> - vdev->vfl_type != VFL_TYPE_SUBDEV)
> - media_device_unregister_entity(>entity);
> + if (v4l2_dev->mdev) {
> + /* Remove interfaces and interface links */
> + media_devnode_remove(vdev->intf_devnode);
> + if (vdev->vfl_type != VFL_TYPE_SUBDEV)
> + media_device_unregister_entity(>entity);

RADIO doesn't have an entity either, so this should probably check for both
SUBDEV and RADIO.

I think it is cleaner if video_register_media_controller() sets a new 
video_device
flag: V4L2_FL_CREATED_ENTITY, and if this release function would just check the
flag.

> + }
>  #endif
>  
>   /* Do not call v4l2_device_put if there is no release callback set.
> @@ -713,6 +716,85 @@ static void determine_valid_ioctls(struct video_device 
> *vdev)
>   BASE_VIDIOC_PRIVATE);
>  }
>  
> +
> +static int video_register_media_controller(struct video_device *vdev, int 
> type)
> +{
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + u32 entity_type = MEDIA_ENT_T_UNKNOWN;
> + u32 intf_type;
> + int ret;
> + bool create_entity = true;
> +
> + if (!vdev->v4l2_dev->mdev)
> + return 0;
> +
> + switch (type) {
> + case VFL_TYPE_GRABBER:
> + intf_type = MEDIA_INTF_T_V4L_VIDEO;
> + entity_type = MEDIA_ENT_T_V4L2_VIDEO;
> + break;
> + case VFL_TYPE_VBI:
> + intf_type = MEDIA_INTF_T_V4L_VBI;
> + entity_type = MEDIA_ENT_T_V4L2_VBI;
> + break;
> + case VFL_TYPE_SDR:
> + intf_type = MEDIA_INTF_T_V4L_SWRADIO;
> + entity_type = MEDIA_ENT_T_V4L2_SWRADIO;
> + break;
> + case VFL_TYPE_RADIO:
> + intf_type = MEDIA_INTF_T_V4L_RADIO;
> + /*
> +  * Radio doesn't have an entity at the V4L2 side to represent
> +  * radio input or output. Instead, the audio input/output goes
> +  * via either physical wires or ALSA.
> +  */
> + create_entity = false;
> + break;
> + case VFL_TYPE_SUBDEV:
> + intf_type = MEDIA_INTF_T_V4L_SUBDEV;
> + /* Entity will be created via v4l2_device_register_subdev() */
> + create_entity = false;
> + break;
> + default:
> + return 0;
> + }
> +
> + if (create_entity) {
> + vdev->entity.type = entity_type;
> + vdev->entity.name = vdev->name;
> +
> + /* Needed just for backward compatibility with legacy MC API */
> + vdev->entity.info.dev.major = VIDEO_MAJOR;
> + vdev->entity.info.dev.minor = vdev->minor;
> +
> + ret = media_device_register_entity(vdev->v4l2_dev->mdev,
> +>entity);
> + if (ret < 0) {
> + printk(KERN_WARNING
> + "%s: media_device_register_entity failed\n",
> + __func__);
> + return ret;
> + }
> + }
> +
> + vdev->intf_devnode = media_devnode_create(vdev->v4l2_dev->mdev,
> +   intf_type,
> +   0, VIDEO_MAJOR,
> +   vdev->minor,
> +   GFP_KERNEL);
> + if (!vdev->intf_devnode)
> + return -ENOMEM;
> +
> + if (create_entity)
> + media_create_intf_link(>entity,
> +>intf_devnode->intf, 0);

You need a NULL pointer check here as well. It might be a good idea to add
__must_check to the media_create_intf_link() prototype. I think there are

Re: [PATCH v8 31/55] [media] media: add macros to check if subdev or V4L2 DMA

[Hans Verkuil]

On 08/31/2015 03:08 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 31 Aug 2015 13:31:32 +0200
> Hans Verkuil  escreveu:
> 
>> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
>>> As we'll be removing entity subtypes from the Kernel, we need
>>> to provide a way for drivers and core to check if a given
>>> entity is represented by a V4L2 subdev or if it is an V4L2
>>> I/O entity (typically with DMA).
>>
>> This needs more discussion. The plan (as I understand it) is to have 
>> properties
>> that describe the entity's functionalities.
>>
>> The existing entity subtypes will exist only as backwards compat types, but 
>> in
>> the future properties should be used to describe the functionalities.
>>
>> This raises the question if we shouldn't use MEDIA_ENT_T_V4L2_SUBDEV to tell
>> userspace that this is a subdev-controlled entity, and let userspace look at
>> the properties to figure out what it is exactly?
>>
>> It could be that this is a transitional patch, and this will be fixed later.
>> If so, this should be mentioned in the commit message.
> 
> There are several different issues here:
> 
> 1) drivers should not rely on type/subtype at MEDIA_ENT_T_*, as we need
>to get rid of it, as this got deprecated;
> 
> 2) Keep backward compatibility.
> 
> 3) the addition of properties;
> 
> The next patch on this series addresses (1) and (2), and this change is
> needed for them.
> 
> We can't tell if this is a transitional patch or not, as we don't have
> yet any idea about how (3) will be added. Maybe it will preserve
> entity->type. Maybe it will convert it into an array. Maybe it would
> do something different.
> 
> In any case, whatever the property patches will be doing, after this
> patch, it will need to touch only at the implementation of the two
> macros, not needing to touch at the drivers again.
> 
> What I can do is to tell at the description that this patch is in
> preparation for the removal of media type/subtype concept that will 
> happen on the next patches, but this is what's described there already
> at:
>   "As we'll be removing entity subtypes from the Kernel,"
> 
> Perhaps I should let it clearer there.

Just mention this in the cover letter as part of the todo list.

As long as I know that this might change later depending on how properties
are going to affect this, then that's OK and I can keep it in mind when I
review.

Regards,

Hans

> 
> Regards,
> Mauro
> 
>>
>> Regards,
>>
>>  Hans
>>
>>>
>>> Signed-off-by: Mauro Carvalho Chehab 
>>>
>>> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
>>> index e7b20bdc735d..b0cfbc0dffc7 100644
>>> --- a/include/media/media-entity.h
>>> +++ b/include/media/media-entity.h
>>> @@ -220,6 +220,39 @@ static inline u32 media_gobj_gen_id(enum 
>>> media_gobj_type type, u32 local_id)
>>> return id;
>>>  }
>>>  
>>> +static inline bool is_media_entity_v4l2_io(struct media_entity *entity)
>>> +{
>>> +   if (!entity)
>>> +   return false;
>>> +
>>> +   switch (entity->type) {
>>> +   case MEDIA_ENT_T_V4L2_VIDEO:
>>> +   case MEDIA_ENT_T_V4L2_VBI:
>>> +   case MEDIA_ENT_T_V4L2_SWRADIO:
>>> +   return true;
>>> +   default:
>>> +   return false;
>>> +   }
>>> +}
>>> +
>>> +static inline bool is_media_entity_v4l2_subdev(struct media_entity *entity)
>>> +{
>>> +   if (!entity)
>>> +   return false;
>>> +
>>> +   switch (entity->type) {
>>> +   case MEDIA_ENT_T_V4L2_SUBDEV_SENSOR:
>>> +   case MEDIA_ENT_T_V4L2_SUBDEV_FLASH:
>>> +   case MEDIA_ENT_T_V4L2_SUBDEV_LENS:
>>> +   case MEDIA_ENT_T_V4L2_SUBDEV_DECODER:
>>> +   case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
>>> +   return true;
>>> +
>>> +   default:
>>> +   return false;
>>> +   }
>>> +}
>>> +
>>>  #define MEDIA_ENTITY_ENUM_MAX_DEPTH16
>>>  #define MEDIA_ENTITY_ENUM_MAX_ID   64
>>>  
>>>
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 16/55] [media] media: Don't accept early-created links

[Javier Martinez Canillas]

Hello Hans,

On 08/31/2015 01:01 PM, Hans Verkuil wrote:
> On 08/31/2015 12:54 PM, Mauro Carvalho Chehab wrote:
>> Em Mon, 31 Aug 2015 12:30:16 +0200
>> Hans Verkuil  escreveu:
>>
>>> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
 Links are graph objects that represent the links of two already
 existing objects in the graph.

 While with the current implementation, it is possible to create
 the links earlier, It doesn't make any sense to allow linking
 two objects when they are not both created.

 So, remove the code that would be handling those early-created
 links and add a BUG_ON() to ensure that.

 Signed-off-by: Mauro Carvalho Chehab 
>>>
>>> The code is OK, so:
>>>
>>> Acked-by: Hans Verkuil 
>>>
>>> But shouldn't this go *after* the omap3isp fixes? After this patch the
>>> omap3isp will call BUG_ON, and that's not what you want.
>>
>> Yes. I'll change the order on my git tree.
>>
>>> It is also not clear if the omap3isp driver is the only one that has this
>>> 'create link before objects' problem. I would expect that the omap4 staging
>>> driver has the same issue and possibly others as well.
>>>
>>> Did someone look at that?
>>
>> I guess other drivers are doing the same.
>>
>> Javier's planning to review the other platform drivers in order to add the 
>> needed fixes there too, and to do more tests with some other platform
>> drivers that he has hardware for testing.
> 
> OK, good. Just wanted to know that.
>

Yes, the omap4iss driver in staging has a similar logic and thus the
same issues than the omap3isp driver. I'll write patches for this as
well but I don't have an OMAP4 board here so testing is appreciated.

As Mauro mentioned, I was reviewing the others driver that are creating
pad links and found more that are creating links before registering:

drivers/media/usb/uvc
drivers/media/platform/vsp1
drivers/media/i2c/smiapp

I'll try to take care of these too.

The other drivers that are creating pad links are doing the right thing
AFAICT by registering the entity with the media device before.

> Perhaps it is a good idea to add a TODO list to the cover letter. This would
> be one item on that list.
>

A TODO list is a good idea indeed.
 
> Regards,
> 
>   Hans
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 52/55] [media] media-device: remove interfaces and interface links

[Hans Verkuil]

On 08/30/2015 05:07 AM, Mauro Carvalho Chehab wrote:
> Just like what's done with entities, when the media controller is
> unregistered, release and interface and interface links that

s/release and/release any/

> might still be there.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 638c682b79c4..2c16a46ea530 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -554,6 +554,22 @@ void media_device_unregister(struct media_device *mdev)
>  {
>   struct media_entity *entity;
>   struct media_entity *next;
> + struct media_link *link, *tmp_link;
> + struct media_interface *intf, *tmp_intf;
> +
> + /* Remove interface links from the media device */
> + list_for_each_entry_safe(link, tmp_link, >links,
> +  graph_obj.list) {
> + media_gobj_remove(>graph_obj);
> + kfree(link);
> + }
> +
> + /* Remove all interfaces from the media device */
> + list_for_each_entry_safe(intf, tmp_intf, >interfaces,
> +  graph_obj.list) {
> + media_gobj_remove(>graph_obj);
> + kfree(intf);
> + }
>  
>   list_for_each_entry_safe(entity, next, >entities, graph_obj.list)
>   media_device_unregister_entity(entity);
> @@ -631,7 +647,6 @@ void media_device_unregister_entity(struct media_entity 
> *entity)
>   /* Remove all data links that belong to this entity */
>   list_for_each_entry_safe(link, tmp, >links, list) {
>   media_gobj_remove(>graph_obj);
> - list_del(>list);
>   kfree(link);
>   }
>  
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 96303a0ade59..1cdda9cb0512 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -206,6 +206,10 @@ void media_gobj_remove(struct media_gobj *gobj)
>  
>   /* Remove the object from mdev list */
>   list_del(>list);
> +
> + /* Links have their own list - we need to drop them there too */
> + if (media_type(gobj) == MEDIA_GRAPH_LINK)
> + list_del(_to_link(gobj)->list);
>  }
>  
>  /**
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 0e7e193a6736..7fd6265f0bcb 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -153,7 +153,7 @@ struct media_entity {
>  };
>  
>  /**
> - * struct media_intf_devnode - Define a Kernel API interface
> + * struct media_interface - Define a Kernel API interface
>   *
>   * @graph_obj:   embedded graph object
>   * @list:Linked list used to find other interfaces that belong
> @@ -163,6 +163,11 @@ struct media_entity {
>   *   uapi/media/media.h header, e. g.
>   *   MEDIA_INTF_T_*
>   * @flags:   Interface flags as defined at uapi/media/media.h
> + *
> + * NOTE: As media_device_unregister() will free the address of the
> + *media_interface, this structure should be embedded as the first
> + *element of the derivated functions, in order for the address to be

s/derivated/derived/

> + *the same.
>   */
>  struct media_interface {
>   struct media_gobj   graph_obj;
> @@ -179,11 +184,11 @@ struct media_interface {
>   * @minor:   Minor number of a device node
>   */
>  struct media_intf_devnode {
> - struct media_interface  intf;
> + struct media_interface  intf; /* must be first field in struct */
>  
>   /* Should match the fields at media_v2_intf_devnode */
> - u32 major;
> - u32 minor;
> + u32 major;
> + u32 minor;
>  };
>  
>  static inline u32 media_entity_id(struct media_entity *entity)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 51/55] [media] remove interface links at media_entity_unregister()

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 14:53:45 +0200
Hans Verkuil  escreveu:

> On 08/30/2015 05:07 AM, Mauro Carvalho Chehab wrote:
> > Interface links connected to an entity should be removed
> > before being able of removing the entity.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > 
> > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> > index a91e1ec076a6..638c682b79c4 100644
> > --- a/drivers/media/media-device.c
> > +++ b/drivers/media/media-device.c
> > @@ -618,14 +618,30 @@ void media_device_unregister_entity(struct 
> > media_entity *entity)
> > return;
> >  
> > spin_lock(>lock);
> > +
> > +   /* Remove interface links with this entity on it */
> > +   list_for_each_entry_safe(link, tmp, >links, graph_obj.list) {
> > +   if (media_type(link->gobj1) == MEDIA_GRAPH_ENTITY
> > +   && link->entity == entity) {
> 
> I don't think you need the == MEDIA_GRAPH_ENTITY check here. That should 
> always be
> true if link->entity == entity.

Yes, I know. Actually, I coded it as just  if (link->entity == entity).
Latter, when reviewing my own patch, I decided to add the extra
check, as it sounded me a little better.

Not sure really what would be the better.

> 
> > +   media_gobj_remove(>graph_obj);
> > +   kfree(link);
> > +   }
> > +   }
> > +
> > +   /* Remove all data links that belong to this entity */
> > list_for_each_entry_safe(link, tmp, >links, list) {
> > media_gobj_remove(>graph_obj);
> > list_del(>list);
> > kfree(link);
> > }
> > +
> > +   /* Remove all pads that belong to this entity */
> > for (i = 0; i < entity->num_pads; i++)
> > media_gobj_remove(>pads[i].graph_obj);
> > +
> > +   /* Remove the entity */
> > media_gobj_remove(>graph_obj);
> > +
> > spin_unlock(>lock);
> > entity->graph_obj.mdev = NULL;
> >  }
> > 
> 
> Regards,
> 
>   Hans
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 48/55] [media] media_device: add a topology version field

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Every time a graph object is added or removed, the version
> of the topology changes. That's a requirement for the new
> MEDIA_IOC_G_TOPOLOGY, in order to allow userspace to know
> that the topology has changed after a previous call to it.
> 
> Signed-off-by: Mauro Carvalho Chehab 

I think this should be postponed until we actually have dynamic reconfigurable
graphs.

I would also like to reserve version 0: if 0 is returned, then the graph is
static.

In G_TOPOLOGY we'd return always 0 for now.

Regards,

Hans

> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index c89f51bc688d..c18f4af52771 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -185,6 +185,9 @@ void media_gobj_init(struct media_device *mdev,
>   list_add_tail(>list, >interfaces);
>   break;
>   }
> +
> + mdev->topology_version++;
> +
>   dev_dbg_obj(__func__, gobj);
>  }
>  
> @@ -199,6 +202,8 @@ void media_gobj_remove(struct media_gobj *gobj)
>  {
>   dev_dbg_obj(__func__, gobj);
>  
> + gobj->mdev->topology_version++;
> +
>   /* Remove the object from mdev list */
>   list_del(>list);
>  }
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index 0d1b9c687454..1b12774a9ab4 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -41,6 +41,8 @@ struct device;
>   * @bus_info:Unique and stable device location identifier
>   * @hw_revision: Hardware device revision
>   * @driver_version: Device driver version
> + * @topology_version: Monotonic counter for storing the version of the graph
> + *   topology. Should be incremented each time the topology changes.
>   * @entity_id:   Unique ID used on the last entity registered
>   * @pad_id:  Unique ID used on the last pad registered
>   * @link_id: Unique ID used on the last link registered
> @@ -74,6 +76,8 @@ struct media_device {
>   u32 hw_revision;
>   u32 driver_version;
>  
> + u32 topology_version;
> +
>   u32 entity_id;
>   u32 pad_id;
>   u32 link_id;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 54/55] [media] au0828: unregister MC at the end

[Hans Verkuil]

On 08/30/2015 05:07 AM, Mauro Carvalho Chehab wrote:
> au0828_analog_unregister() calls video_unregister_device(),
> with, in turn, calls media_devnode_remove() in order to drop
> the media interfaces.
> 
> We can't release the media controller before that, or an
> OOPS will occur:

So this patch should be moved to a place earlier in the patch series,
right? To prevent bisects that hit this bug.

Regards,

Hans

> 
> [  176.938752] usb 1-4.4: Media device released
> [  176.938753] usb 1-4.4: Media device unregistered
> [  177.091235] general protection fault:  [#1] SMP
> [  177.091253] Modules linked in: ir_lirc_codec ir_xmp_decoder lirc_dev 
> ir_mce_kbd_decoder ir_sharp_decoder ir_sanyo_decoder ir_sony_decoder 
> ir_jvc_decoder ir_rc6_decoder ir_nec_decoder ir_rc5_decoder au8522_dig xc5000 
> tuner au8522_decoder au8522_common au0828(-) videobuf2_vmalloc 
> videobuf2_memops tveeprom videobuf2_core dvb_core rc_core v4l2_common 
> videodev media cpufreq_powersave cpufreq_conservative cpufreq_userspace 
> cpufreq_stats parport_pc ppdev lp parport snd_hda_codec_hdmi i915 
> x86_pkg_temp_thermal intel_powerclamp intel_rapl iosf_mbi coretemp kvm_intel 
> kvm btusb crct10dif_pclmul snd_hda_codec_realtek crc32_pclmul btrtl 
> crc32c_intel btbcm snd_hda_codec_generic ghash_clmulni_intel btintel 
> i2c_algo_bit drm_kms_helper bluetooth iTCO_wdt snd_usb_audio snd_hda_intel 
> iTCO_vendor_support jitterentropy_rng
> [  177.091455]  snd_hda_codec evdev sha256_ssse3 drm sha256_generic hmac 
> snd_usbmidi_lib snd_hwdep snd_hda_core snd_rawmidi drbg snd_seq_device 
> snd_pcm aesni_intel aes_x86_64 lrw gf128mul mei_me glue_helper snd_timer 
> ablk_helper cryptd mei rfkill snd psmouse sg shpchp soundcore lpc_ich 
> serio_raw i2c_i801 pcspkr mfd_core tpm_tis tpm battery dw_dmac video 
> i2c_designware_platform dw_dmac_core i2c_designware_core acpi_pad processor 
> button ext4 crc16 mbcache jbd2 dm_mod sd_mod ahci libahci libata e1000e 
> scsi_mod ptp pps_core ehci_pci xhci_pci ehci_hcd xhci_hcd thermal fan 
> thermal_sys sdhci_acpi sdhci mmc_core i2c_hid hid [last unloaded: rc_core]
> [  177.091632] CPU: 1 PID: 18040 Comm: rmmod Tainted: GW   
> 4.2.0-rc2+ #9
> [  177.091648] Hardware name: 
> \x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x
>  
> \x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x\x/NUC5i7RYB,
>  BIOS RYBDWi35.86A.0246.2015.0309.1355 03/09/2015
> [  177.091677] task: 88040811b080 ti: 880036b88000 task.ti: 
> 880036b88000
> [  177.091693] RIP: 0010:[]  [] 
> native_queued_spin_lock_slowpath+0x103/0x180
> [  177.091718] RSP: 0018:880036b8bcd8  EFLAGS: 00010202
> [  177.091730] RAX: 3ffe RBX: 880407e11b70 RCX: 
> 88041ec962c0
> [  177.091745] RDX: 7463656a62506357 RSI: 0008 RDI: 
> 880407e11b74
> [  177.091760] RBP: 880407e11b74 R08: 0001 R09: 
> 812bf4e0
> [  177.091776] R10:  R11:  R12: 
> 88040811b080
> [  177.091791] R13: 88003601cec8 R14: 8804084b8890 R15: 
> 8804084b8800
> [  177.091807] FS:  7f2f9bab1700() GS:88041ec8() 
> knlGS:
> [  177.091824] CS:  0010 DS:  ES:  CR0: 80050033
> [  177.091837] CR2: 7fe8c0f900e0 CR3: 35c47000 CR4: 
> 003407e0
> [  177.091852] Stack:
> [  177.091857]  81562e3d 815613b3  
> 
> [  177.091876]  88040b6d3240 8804084b8890 880407e11b70 
> 88003601cd30
> [  177.091895]  88003601ce28 88003601cec8 8804084b8890 
> 8156148b
> [  177.091914] Call Trace:
> [  177.091923]  [] ? _raw_spin_lock+0x1d/0x20
> [  177.091936]  [] ? __mutex_lock_slowpath+0x43/0x100
> [  177.091951]  [] ? mutex_lock+0x1b/0x30
> [  177.091965]  [] ? media_remove_intf_links+0x1d/0x40 
> [media]
> [  177.091981]  [] ? media_devnode_remove+0xe/0x20 [media]
> [  177.091997]  [] ? v4l2_device_release+0x95/0x100 
> [videodev]
> [  177.092014]  [] ? device_release+0x2d/0x90
> [  177.092028]  [] ? kobject_release+0x79/0x1b0
> [  177.092042]  [] ? au0828_analog_unregister+0x2a/0x60 
> [au0828]
> [  177.092059]  [] ? au0828_usb_disconnect+0x9e/0xd0 
> [au0828]
> [  177.092075]  [] ? usb_unbind_interface+0x79/0x270
> [  177.092090]  [] ? __device_release_driver+0x95/0x130
> [  177.092105]  [] ? driver_detach+0xab/0xb0
> [  177.092120]  [] ? bus_remove_driver+0x55/0xd0
> 

Re: [PATCH v8 28/55] [media] uapi/media.h: Fix entity namespace

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 13:17:08 +0200
Hans Verkuil  escreveu:

> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> > Now that interfaces got created, we need to fix the entity
> > namespace.
> > 
> > So, let's create a consistent new namespace and add backward
> > compatibility macros to keep the old namespace preserved.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > 
> > diff --git a/drivers/media/dvb-core/dvbdev.c 
> > b/drivers/media/dvb-core/dvbdev.c
> > index 14d32cdcdd47..88013d1a2c39 100644
> > --- a/drivers/media/dvb-core/dvbdev.c
> > +++ b/drivers/media/dvb-core/dvbdev.c
> > @@ -230,17 +230,17 @@ static void dvb_create_media_entity(struct dvb_device 
> > *dvbdev,
> >  
> > switch (type) {
> > case DVB_DEVICE_FRONTEND:
> > -   dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
> > +   dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMOD;
> > dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> > dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> > break;
> > case DVB_DEVICE_DEMUX:
> > -   dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
> > +   dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMUX;
> > dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> > dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> > break;
> > case DVB_DEVICE_CA:
> > -   dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
> > +   dvbdev->entity->type = MEDIA_ENT_T_DVB_CA;
> > dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> > dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> > break;
> > @@ -439,7 +439,7 @@ EXPORT_SYMBOL(dvb_unregister_device);
> >  void dvb_create_media_graph(struct dvb_adapter *adap)
> >  {
> > struct media_device *mdev = adap->mdev;
> > -   struct media_entity *entity, *tuner = NULL, *fe = NULL;
> > +   struct media_entity *entity, *tuner = NULL, *demod = NULL;
> > struct media_entity *demux = NULL, *dvr = NULL, *ca = NULL;
> > struct media_interface *intf;
> >  
> > @@ -451,26 +451,26 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
> > case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
> > tuner = entity;
> > break;
> > -   case MEDIA_ENT_T_DEVNODE_DVB_FE:
> > -   fe = entity;
> > +   case MEDIA_ENT_T_DVB_DEMOD:
> > +   demod = entity;
> > break;
> > -   case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
> > +   case MEDIA_ENT_T_DVB_DEMUX:
> > demux = entity;
> > break;
> > -   case MEDIA_ENT_T_DEVNODE_DVB_DVR:
> > +   case MEDIA_ENT_T_DVB_TSOUT:
> > dvr = entity;
> > break;
> > -   case MEDIA_ENT_T_DEVNODE_DVB_CA:
> > +   case MEDIA_ENT_T_DVB_CA:
> > ca = entity;
> > break;
> > }
> > }
> >  
> > -   if (tuner && fe)
> > -   media_create_pad_link(tuner, 0, fe, 0, 0);
> > +   if (tuner && demod)
> > +   media_create_pad_link(tuner, 0, demod, 0, 0);
> >  
> > -   if (fe && demux)
> > -   media_create_pad_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
> > +   if (demod && demux)
> > +   media_create_pad_link(demod, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
> >  
> > if (demux && dvr)
> > media_create_pad_link(demux, 1, dvr, 0, MEDIA_LNK_FL_ENABLED);
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index aca828709bad..3bbda409353f 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -42,31 +42,71 @@ struct media_device_info {
> >  
> >  #define MEDIA_ENT_ID_FLAG_NEXT (1 << 31)
> >  
> > +/*
> > + * Base numbers for entity types
> > + *
> > + * Please notice that the huge gap of 16 bits for each base is overkill!
> > + * 8 bits is more than enough to avoid starving entity types for each
> > + * subsystem.
> > + *
> > + * However, It is kept this way just to avoid binary breakages with the
> > + * namespace provided on legacy versions of this header.
> > + */
> > +#define MEDIA_ENT_T_DVB_BASE   0x0001
> 
> I would change this to 0x, see follow-up comment later for why.
> 
> > +#define MEDIA_ENT_T_V4L2_BASE  0x0001
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_BASE   0x0002
> > +
> > +/*
> > + * V4L2 entities - Those are used for DMA (mmap/DMABUF) and
> > + * read()/write() data I/O associated with the V4L2 devnodes.
> > + */
> > +#define MEDIA_ENT_T_V4L2_VIDEO (MEDIA_ENT_T_V4L2_BASE + 1)
> > +   /*
> > +* Please notice that numbers between MEDIA_ENT_T_V4L2_BASE + 2 and
> > +* MEDIA_ENT_T_V4L2_BASE + 4 can't be used, as those values used
> > +* to be declared for FB, ALSA and DVB entities.
> > +* As those values were never actually used in practice, we're just
> > 

Re: [PATCH v8 48/55] [media] media_device: add a topology version field

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 14:29:28 +0200
Hans Verkuil  escreveu:

> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> > Every time a graph object is added or removed, the version
> > of the topology changes. That's a requirement for the new
> > MEDIA_IOC_G_TOPOLOGY, in order to allow userspace to know
> > that the topology has changed after a previous call to it.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> 
> I think this should be postponed until we actually have dynamic reconfigurable
> graphs.

So far, we're using the term "dynamic" to mean partial graph object
removal.

But even today, MC does support "dynamic" in the sense of graph
object additions.

You should notice that having a topology_version is something that
IMHO, it is needed since the beginning, even without dynamic
reconfigurable graphs, because the graph may grow in runtime.

That will happen, for example, if usb-snd-audio is blacklisted
at /etc/modprobe*, and someone connects an au0828.

New entities/links will be created (after Shuah patches) if one would
modprobe latter snd-usb-audio.

> 
> I would also like to reserve version 0: if 0 is returned, then the graph is
> static.

Why? Implementing this would be really hard, as that would mean that
G_TOPOLOGY would need to be blocked until all drivers and subdevices
get probed.

In order to implement that, some logic would be needed at the drivers
to identify if everything was set and unlock G_TOPOLOGY.

What would be the gain for that? I fail to see any.

On the other hand, the patch below offers a simple way to detect if topology
changes, as, no matter if an object was added or removed, the topology
version will be increased.

Btw, I added a logic at the mc_nextgen_test program to identify if the
topology changes between the two calls:

http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-utils.git/tree/contrib/test/mc_nextgen_test.c?h=mc-next-gen=fdc16ece9732c94cfa76eee86978158c5976c00a#n504

Regards,
Mauro

> 
> In G_TOPOLOGY we'd return always 0 for now.
> 
> Regards,
> 
>   Hans
> 
> > 
> > diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> > index c89f51bc688d..c18f4af52771 100644
> > --- a/drivers/media/media-entity.c
> > +++ b/drivers/media/media-entity.c
> > @@ -185,6 +185,9 @@ void media_gobj_init(struct media_device *mdev,
> > list_add_tail(>list, >interfaces);
> > break;
> > }
> > +
> > +   mdev->topology_version++;
> > +
> > dev_dbg_obj(__func__, gobj);
> >  }
> >  
> > @@ -199,6 +202,8 @@ void media_gobj_remove(struct media_gobj *gobj)
> >  {
> > dev_dbg_obj(__func__, gobj);
> >  
> > +   gobj->mdev->topology_version++;
> > +
> > /* Remove the object from mdev list */
> > list_del(>list);
> >  }
> > diff --git a/include/media/media-device.h b/include/media/media-device.h
> > index 0d1b9c687454..1b12774a9ab4 100644
> > --- a/include/media/media-device.h
> > +++ b/include/media/media-device.h
> > @@ -41,6 +41,8 @@ struct device;
> >   * @bus_info:  Unique and stable device location identifier
> >   * @hw_revision: Hardware device revision
> >   * @driver_version: Device driver version
> > + * @topology_version: Monotonic counter for storing the version of the 
> > graph
> > + * topology. Should be incremented each time the topology changes.
> >   * @entity_id: Unique ID used on the last entity registered
> >   * @pad_id:Unique ID used on the last pad registered
> >   * @link_id:   Unique ID used on the last link registered
> > @@ -74,6 +76,8 @@ struct media_device {
> > u32 hw_revision;
> > u32 driver_version;
> >  
> > +   u32 topology_version;
> > +
> > u32 entity_id;
> > u32 pad_id;
> > u32 link_id;
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 48/55] [media] media_device: add a topology version field

[Hans Verkuil]

On 08/31/2015 02:52 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 31 Aug 2015 14:29:28 +0200
> Hans Verkuil  escreveu:
> 
>> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
>>> Every time a graph object is added or removed, the version
>>> of the topology changes. That's a requirement for the new
>>> MEDIA_IOC_G_TOPOLOGY, in order to allow userspace to know
>>> that the topology has changed after a previous call to it.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab 
>>
>> I think this should be postponed until we actually have dynamic 
>> reconfigurable
>> graphs.
> 
> So far, we're using the term "dynamic" to mean partial graph object
> removal.
> 
> But even today, MC does support "dynamic" in the sense of graph
> object additions.
> 
> You should notice that having a topology_version is something that
> IMHO, it is needed since the beginning, even without dynamic
> reconfigurable graphs, because the graph may grow in runtime.
> 
> That will happen, for example, if usb-snd-audio is blacklisted
> at /etc/modprobe*, and someone connects an au0828.
> 
> New entities/links will be created (after Shuah patches) if one would
> modprobe latter snd-usb-audio.

latter -> later :-)

You are right, this would trigger a topology change. I hadn't thought about
that.

> 
>>
>> I would also like to reserve version 0: if 0 is returned, then the graph is
>> static.
> 
> Why? Implementing this would be really hard, as that would mean that
> G_TOPOLOGY would need to be blocked until all drivers and subdevices
> get probed.
> 
> In order to implement that, some logic would be needed at the drivers
> to identify if everything was set and unlock G_TOPOLOGY.

That wouldn't be needed if the media device node was created last. Which
I think is a good idea anyway.

> 
> What would be the gain for that? I fail to see any.

It would tell userspace that it doesn't have to cope with dynamically
changing graphs.

Even though with the au0828 example you can expect to see cases like that,
I can pretty much guarantee that no generic v4l2 applications will ever
support dynamic changes. Those that will support it will be custom-made.

Being able to see that graphs can change dynamically would allow such apps
to either refuse to use the device, or warn the user.

Regards,

Hans

> 
> On the other hand, the patch below offers a simple way to detect if topology
> changes, as, no matter if an object was added or removed, the topology
> version will be increased.
> 
> Btw, I added a logic at the mc_nextgen_test program to identify if the
> topology changes between the two calls:
>   
> http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-utils.git/tree/contrib/test/mc_nextgen_test.c?h=mc-next-gen=fdc16ece9732c94cfa76eee86978158c5976c00a#n504
> 
> Regards,
> Mauro
> 
>>
>> In G_TOPOLOGY we'd return always 0 for now.
>>
>> Regards,
>>
>>  Hans
>>
>>>
>>> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
>>> index c89f51bc688d..c18f4af52771 100644
>>> --- a/drivers/media/media-entity.c
>>> +++ b/drivers/media/media-entity.c
>>> @@ -185,6 +185,9 @@ void media_gobj_init(struct media_device *mdev,
>>> list_add_tail(>list, >interfaces);
>>> break;
>>> }
>>> +
>>> +   mdev->topology_version++;
>>> +
>>> dev_dbg_obj(__func__, gobj);
>>>  }
>>>  
>>> @@ -199,6 +202,8 @@ void media_gobj_remove(struct media_gobj *gobj)
>>>  {
>>> dev_dbg_obj(__func__, gobj);
>>>  
>>> +   gobj->mdev->topology_version++;
>>> +
>>> /* Remove the object from mdev list */
>>> list_del(>list);
>>>  }
>>> diff --git a/include/media/media-device.h b/include/media/media-device.h
>>> index 0d1b9c687454..1b12774a9ab4 100644
>>> --- a/include/media/media-device.h
>>> +++ b/include/media/media-device.h
>>> @@ -41,6 +41,8 @@ struct device;
>>>   * @bus_info:  Unique and stable device location identifier
>>>   * @hw_revision: Hardware device revision
>>>   * @driver_version: Device driver version
>>> + * @topology_version: Monotonic counter for storing the version of the 
>>> graph
>>> + * topology. Should be incremented each time the topology changes.
>>>   * @entity_id: Unique ID used on the last entity registered
>>>   * @pad_id:Unique ID used on the last pad registered
>>>   * @link_id:   Unique ID used on the last link registered
>>> @@ -74,6 +76,8 @@ struct media_device {
>>> u32 hw_revision;
>>> u32 driver_version;
>>>  
>>> +   u32 topology_version;
>>> +
>>> u32 entity_id;
>>> u32 pad_id;
>>> u32 link_id;
>>>
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 46/55] [media] media: move mdev list init to gobj

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Let's control the topology changes inside the graph_object.
> So, move the removal of interfaces/entities from the mdev
> lists to media_gobj_init() and media_gobj_remove().

s/removal/addition and removal/

> 
> The main reason is that mdev should have lists for all
> object types, as the new MC api will require to store
> objects in separate places.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 134fe7510195..ec98595b8a7a 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -415,7 +415,7 @@ void media_device_unregister(struct media_device *mdev)
>   struct media_entity *entity;
>   struct media_entity *next;
>  
> - list_for_each_entry_safe(entity, next, >entities, list)
> + list_for_each_entry_safe(entity, next, >entities, graph_obj.list)
>   media_device_unregister_entity(entity);
>  
>   device_remove_file(>devnode.dev, _attr_model);
> @@ -449,7 +449,6 @@ int __must_check media_device_register_entity(struct 
> media_device *mdev,
>   spin_lock(>lock);
>   /* Initialize media_gobj embedded at the entity */
>   media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, >graph_obj);
> - list_add_tail(>list, >entities);
>  
>   /* Initialize objects at the pads */
>   for (i = 0; i < entity->num_pads; i++)
> @@ -487,7 +486,6 @@ void media_device_unregister_entity(struct media_entity 
> *entity)
>   for (i = 0; i < entity->num_pads; i++)
>   media_gobj_remove(>pads[i].graph_obj);
>   media_gobj_remove(>graph_obj);
> - list_del(>list);
>   spin_unlock(>lock);
>   entity->graph_obj.mdev = NULL;
>  }
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index d62a6ffbc929..192af193a394 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -170,6 +170,7 @@ void media_gobj_init(struct media_device *mdev,
>   switch (type) {
>   case MEDIA_GRAPH_ENTITY:
>   gobj->id = media_gobj_gen_id(type, ++mdev->entity_id);
> + list_add_tail(>list, >entities);
>   break;
>   case MEDIA_GRAPH_PAD:
>   gobj->id = media_gobj_gen_id(type, ++mdev->pad_id);
> @@ -178,6 +179,7 @@ void media_gobj_init(struct media_device *mdev,
>   gobj->id = media_gobj_gen_id(type, ++mdev->link_id);
>   break;
>   case MEDIA_GRAPH_INTF_DEVNODE:
> + list_add_tail(>list, >interfaces);
>   gobj->id = media_gobj_gen_id(type, ++mdev->intf_devnode_id);
>   break;
>   }
> @@ -193,6 +195,15 @@ void media_gobj_init(struct media_device *mdev,
>   */
>  void media_gobj_remove(struct media_gobj *gobj)
>  {
> + /* Remove the object from mdev list */
> + switch (media_type(gobj)) {
> + case MEDIA_GRAPH_ENTITY:
> + case MEDIA_GRAPH_INTF_DEVNODE:
> + list_del(>list);

Missing break!

> + default:
> + break;
> + }
> +
>   dev_dbg_obj(__func__, gobj);
>  }
>  
> @@ -864,8 +875,6 @@ static void media_interface_init(struct media_device 
> *mdev,
>   INIT_LIST_HEAD(>links);
>  
>   media_gobj_init(mdev, gobj_type, >graph_obj);
> -
> - list_add_tail(>list, >interfaces);
>  }
>  
>  /* Functions related to the media interface via device nodes */
> @@ -894,7 +903,6 @@ EXPORT_SYMBOL_GPL(media_devnode_create);
>  void media_devnode_remove(struct media_intf_devnode *devnode)
>  {
>   media_gobj_remove(>intf.graph_obj);
> - list_del(>intf.list);
>   kfree(devnode);
>  }
>  EXPORT_SYMBOL_GPL(media_devnode_remove);
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index f23d686aaac6..85fa302047bd 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -111,11 +111,11 @@ struct media_device *media_device_find_devres(struct 
> device *dev);
>  
>  /* Iterate over all entities. */
>  #define media_device_for_each_entity(entity, mdev)   \
> - list_for_each_entry(entity, &(mdev)->entities, list)
> + list_for_each_entry(entity, &(mdev)->entities, graph_obj.list)
>  
>  /* Iterate over all interfaces. */
>  #define media_device_for_each_intf(intf, mdev)   \
> - list_for_each_entry(intf, &(mdev)->interfaces, list)
> + list_for_each_entry(intf, &(mdev)->interfaces, graph_obj.list)
>  
>  
>  #else
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 358a0c6b1f86..8c344a07636c 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -66,6 +66,7 @@ enum media_gobj_type {
>  struct media_gobj {
>   struct media_device *mdev;
>   u32 id;
> + struct list_headlist;
>  };
>  
>  
> @@ -114,7 +115,6 @@ struct media_entity_operations {
>  
>  struct media_entity {
>   

Re: [PATCH v8 50/55] [media] media-entity: unregister entity links

[Hans Verkuil]

On 08/30/2015 05:07 AM, Mauro Carvalho Chehab wrote:
> Add functions to explicitly unregister all entity links.
> This function is called automatically when an entity
> link is destroyed.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index c18f4af52771..96303a0ade59 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -903,6 +903,7 @@ EXPORT_SYMBOL_GPL(media_devnode_create);
>  
>  void media_devnode_remove(struct media_intf_devnode *devnode)
>  {
> + media_remove_intf_links(>intf);
>   media_gobj_remove(>intf.graph_obj);
>   kfree(devnode);
>  }
> @@ -944,3 +945,25 @@ void media_remove_intf_link(struct media_link *link)
>   mutex_unlock(>graph_obj.mdev->graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_remove_intf_link);
> +
> +void __media_remove_intf_links(struct media_interface *intf)
> +{
> + struct media_link *link, *tmp;
> +
> + list_for_each_entry_safe(link, tmp, >links, list)
> + __media_remove_intf_link(link);
> +
> +}
> +EXPORT_SYMBOL_GPL(__media_remove_intf_links);
> +
> +void media_remove_intf_links(struct media_interface *intf)
> +{
> + /* Do nothing if the intf is not registered. */
> + if (intf->graph_obj.mdev == NULL)
> + return;
> +
> + mutex_lock(>graph_obj.mdev->graph_mutex);
> + __media_remove_intf_links(intf);
> + mutex_unlock(>graph_obj.mdev->graph_mutex);
> +}
> +EXPORT_SYMBOL_GPL(media_remove_intf_links);
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 8c344a07636c..0e7e193a6736 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -318,6 +318,9 @@ struct media_link *media_create_intf_link(struct 
> media_entity *entity,
>   struct media_interface *intf,
>   u32 flags);
>  void media_remove_intf_link(struct media_link *link);
> +void __media_remove_intf_links(struct media_interface *intf);
> +void media_remove_intf_links(struct media_interface *intf);
> +
>  
>  #define media_entity_call(entity, operation, args...)
> \
>   (((entity)->ops && (entity)->ops->operation) ?  \
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 14:00:42 +0200
Hans Verkuil  escreveu:

> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> > Add a new ioctl that will report the entire topology on
> > one go.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > 
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index 756e1960fd7f..358a0c6b1f86 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -181,6 +181,8 @@ struct media_interface {
> >   */
> >  struct media_intf_devnode {
> > struct media_interface  intf;
> > +
> > +   /* Should match the fields at media_v2_intf_devnode */
> > u32 major;
> > u32 minor;
> >  };
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index 4186891e5e81..fa0b68e670b0 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -251,11 +251,94 @@ struct media_links_enum {
> >  #define MEDIA_INTF_T_ALSA_RAWMIDI   (MEDIA_INTF_T_ALSA_BASE + 4)
> >  #define MEDIA_INTF_T_ALSA_HWDEP (MEDIA_INTF_T_ALSA_BASE + 5)
> >  
> > -/* TBD: declare the structs needed for the new G_TOPOLOGY ioctl */
> > +/*
> > + * MC next gen API definitions
> > + *
> > + * NOTE: The declarations below are close to the MC RFC for the Media
> > + *  Controller, the next generation. Yet, there are a few adjustments
> > + *  to do, as we want to be able to have a functional API before
> > + *  the MC properties change. Those will be properly marked below.
> > + *  Please also notice that I removed "num_pads", "num_links",
> > + *  from the proposal, as a proper userspace application will likely
> > + *  use lists for pads/links, just as we intend todo in Kernelspace.
> 
> s/todo/to do/
> 
> > + *  The API definition should be freed from fields that are bound to
> > + *  some specific data structure.
> > + *
> > + * FIXME: Currently, I opted to name the new types as "media_v2", as this
> > + *   won't cause any conflict with the Kernelspace namespace, nor with
> > + *   the previous kAPI media_*_desc namespace. This can be changed
> > + *   latter, before the adding this API upstream.
> 
> s/latter/later/ :-)
> 
> I think this comment belongs to the commit log and not in this header.

True, but I opted to keep it here for now to produce some discussions ;)

I'm actually in doubt if we should rename the flags as proposed below,
and use the newer flags only at G_TOPOLOGY or if we should keep the same
namespace for them and accept newer flags with legacy ioctls.

> 
> > + */
> > +
> > +
> > +#define MEDIA_NEW_LNK_FL_ENABLED   MEDIA_LNK_FL_ENABLED
> > +#define MEDIA_NEW_LNK_FL_IMMUTABLE MEDIA_LNK_FL_IMMUTABLE
> > +#define MEDIA_NEW_LNK_FL_DYNAMIC   MEDIA_NEW_FL_DYNAMIC
> > +#define MEDIA_NEW_LNK_FL_INTERFACE_LINK(1 << 3)
> 
> Shouldn't this be MEDIA_V2_ instead of MEDIA_NEW_?
> 
> Do we need the INTERFACE_LINK flag? You can deduce it by checking the
> ID type.

Yes, this can be deduced from the type of the objects inside the link.

I guess I added it because of some comment from your media.h RFC
proposal.

Right now, I'm using it at the application to better represent the graph
elements:


http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-utils.git/tree/contrib/test/mc_nextgen_test.c?h=mc-next-gen=fdc16ece9732c94cfa76eee86978158c5976c00a#n438
 

http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-utils.git/tree/contrib/test/mc_nextgen_test.c?h=mc-next-gen=fdc16ece9732c94cfa76eee86978158c5976c00a#n444

But it could, instead, be doing something like:

if media_type(link->gobj1.id == MEDIA_GRAPH_PAD)
link_is_pad = true;
else
link_is_pad = false;


Btw, I'm using the same type for both data and interface links, as
I don't see any reason why to differentiate internally: they all share
the same linked list at mdev and the same object ID range.

> 
> I don't have a clear preference one way or another, just wondering about the
> reason for adding it.
> 
> > +
> > +struct media_v2_entity {
> > +   __u32 id;
> > +   char name[64];  /* FIXME: move to a property? (RFC says so) */
> > +   __u16 reserved[14];
> > +};
> > +
> > +/* Should match the specific fields at media_intf_devnode */
> > +struct media_v2_intf_devnode {
> > +   __u32 major;
> > +   __u32 minor;
> > +};
> > +
> > +struct media_v2_interface {
> > +   __u32 id;
> > +   __u32 intf_type;
> > +   __u32 flags;
> > +   __u32 reserved[9];
> > +
> > +   union {
> > +   struct media_v2_intf_devnode devnode;
> > +   __u32 raw[16];
> > +   };
> > +};
> > +
> > +struct media_v2_pad {
> > +   __u32 id;
> > +   __u32 entity_id;
> > +   __u32 flags;
> > +   __u16 reserved[9];
> > +};
> > +
> > +struct media_v2_link {
> > +__u32 id;
> > +__u32 source_id;
> > +__u32 sink_id;
> 
> Like in media_link I would 

Re: [PATCH v8 49/55] [media] media-device: add support for MEDIA_IOC_G_TOPOLOGY ioctl

[Mauro Carvalho Chehab]

Em Mon, 31 Aug 2015 14:47:03 +0200
Hans Verkuil  escreveu:

> On 08/30/2015 05:07 AM, Mauro Carvalho Chehab wrote:
> > Add support for the new MEDIA_IOC_G_TOPOLOGY ioctl, according
> > with the RFC for the MC next generation.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > 
> > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> > index 5b2c9f7fcd45..a91e1ec076a6 100644
> > --- a/drivers/media/media-device.c
> > +++ b/drivers/media/media-device.c
> > @@ -232,6 +232,136 @@ static long media_device_setup_link(struct 
> > media_device *mdev,
> > return ret;
> >  }
> >  
> > +static long __media_device_get_topology(struct media_device *mdev,
> > + struct media_v2_topology *topo)
> > +{
> > +   struct media_entity *entity;
> > +   struct media_interface *intf;
> > +   struct media_pad *pad;
> > +   struct media_link *link;
> > +   struct media_v2_entity uentity;
> > +   struct media_v2_interface uintf;
> > +   struct media_v2_pad upad;
> > +   struct media_v2_link ulink;
> > +   int ret = 0, i;
> > +
> > +   topo->topology_version = mdev->topology_version;
> > +
> > +   /* Get entities and number of entities */
> > +   i = 0;
> > +   media_device_for_each_entity(entity, mdev) {
> > +   i++;
> > +
> > +   if (ret || !topo->entities)
> > +   continue;
> 
> I would add:
> 
>   if (i > topo->num_entities)
>   continue;
> 
> The copy_to_user can succeed, even if i > num_entities depending on how the
> memory was allocated. So I would always check num_entities and refuse to go
> beyond it.

I think that the best is:

if (i > topo->num_entities) {
ret = -ENOSPC;
continue;
}

> 
> > +
> > +   /* Copy fields to userspace struct if not error */
> > +   memset(, 0, sizeof(uentity));
> > +   uentity.id = entity->graph_obj.id;
> > +   strncpy(uentity.name, entity->name,
> > +   sizeof(uentity.name));
> > +
> > +   if (copy_to_user(>entities[i - 1], , 
> > sizeof(uentity)))
> > +   ret = -EFAULT;
> 
> I would just return here. If the user gives bogus values for this, then just
> give up.

With the above change, yes, we can just return -EFAULT here.

> 
> > +   }
> > +   topo->num_entities = i;
> 
> What to do if topo->entities != NULL and i > topo->num_entities? Just return 0
> or return ENOSPC? I'm in favor of ENOSPC since a partial topology result is
> useless. But if ENOSPC is returned, then all num_foo values should be updated
> with the current number of elements.
> 
> This behavior would be consistent with e.g. VIDIOC_G_EXT_CTRLS.

Ok. See above.

> > +
> > +   /* Get interfaces and number of interfaces */
> > +   i = 0;
> > +   media_device_for_each_intf(intf, mdev) {
> > +   i++;
> > +
> > +   if (ret || !topo->interfaces)
> > +   continue;
> > +
> > +   memset(, 0, sizeof(uintf));
> > +
> > +   /* Copy intf fields to userspace struct */
> > +   uintf.id = intf->graph_obj.id;
> > +   uintf.intf_type = intf->type;
> > +   uintf.flags = intf->flags;
> > +
> > +   if (media_type(>graph_obj) == MEDIA_GRAPH_INTF_DEVNODE) {
> > +   struct media_intf_devnode *devnode;
> > +
> > +   devnode = intf_to_devnode(intf);
> > +
> > +   uintf.devnode.major = devnode->major;
> > +   uintf.devnode.minor = devnode->minor;
> > +   }
> > +
> > +   if (copy_to_user(>interfaces[i - 1], , 
> > sizeof(uintf)))
> > +   ret = -EFAULT;
> > +   }
> > +   topo->num_interfaces = i;
> > +
> > +   /* Get pads and number of pads */
> > +   i = 0;
> > +   media_device_for_each_pad(pad, mdev) {
> > +   i++;
> > +
> > +   if (ret || !topo->pads)
> > +   continue;
> > +
> > +   memset(, 0, sizeof(upad));
> > +
> > +   /* Copy pad fields to userspace struct */
> > +   upad.id = pad->graph_obj.id;
> > +   upad.entity_id = pad->entity->graph_obj.id;
> > +   upad.flags = pad->flags;
> > +
> > +   if (copy_to_user(>pads[i - 1], , sizeof(upad)))
> > +   ret = -EFAULT;
> > +   }
> > +   topo->num_pads = i;
> > +
> > +   /* Get links and number of links */
> > +   i = 0;
> > +   media_device_for_each_link(link, mdev) {
> > +   i++;
> > +
> > +   if (ret || !topo->links)
> > +   continue;
> > +
> > +   memset(, 0, sizeof(ulink));
> > +
> > +   /* Copy link fields to userspace struct */
> > +   ulink.id = link->graph_obj.id;
> > +   ulink.source_id = link->gobj0->id;
> > +   ulink.sink_id = link->gobj1->id;
> > +   ulink.flags = link->flags;
> > +
> > +   if (media_type(link->gobj0) != MEDIA_GRAPH_PAD)
> > +  

Re: [PATCH v8 45/55] [media] media: Use a macro to interate between all interfaces

[Hans Verkuil]

On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Just like we do with entities, use a similar macro for the
> interfaces loop.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

> 
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 610d2bab1368..95b5b4b11230 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -578,9 +578,10 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
>   }
>  
>   /* Create indirect interface links for FE->tuner, DVR->demux and CA->ca 
> */
> - list_for_each_entry(intf, >interfaces, list) {
> + media_device_for_each_intf(intf, mdev) {
>   if (intf->type == MEDIA_INTF_T_DVB_CA && ca)
>   media_create_intf_link(ca, intf, 0);
> +
>   if (intf->type == MEDIA_INTF_T_DVB_FE && tuner)
>   media_create_intf_link(tuner, intf, 0);
>  
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index 51807efa505b..f23d686aaac6 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -113,6 +113,11 @@ struct media_device *media_device_find_devres(struct 
> device *dev);
>  #define media_device_for_each_entity(entity, mdev)   \
>   list_for_each_entry(entity, &(mdev)->entities, list)
>  
> +/* Iterate over all interfaces. */
> +#define media_device_for_each_intf(intf, mdev)   \
> + list_for_each_entry(intf, &(mdev)->interfaces, list)
> +
> +
>  #else
>  static inline int media_device_register(struct media_device *mdev)
>  {
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 49/55] [media] media-device: add support for MEDIA_IOC_G_TOPOLOGY ioctl

[Hans Verkuil]

On 08/30/2015 05:07 AM, Mauro Carvalho Chehab wrote:
> Add support for the new MEDIA_IOC_G_TOPOLOGY ioctl, according
> with the RFC for the MC next generation.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 5b2c9f7fcd45..a91e1ec076a6 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -232,6 +232,136 @@ static long media_device_setup_link(struct media_device 
> *mdev,
>   return ret;
>  }
>  
> +static long __media_device_get_topology(struct media_device *mdev,
> +   struct media_v2_topology *topo)
> +{
> + struct media_entity *entity;
> + struct media_interface *intf;
> + struct media_pad *pad;
> + struct media_link *link;
> + struct media_v2_entity uentity;
> + struct media_v2_interface uintf;
> + struct media_v2_pad upad;
> + struct media_v2_link ulink;
> + int ret = 0, i;
> +
> + topo->topology_version = mdev->topology_version;
> +
> + /* Get entities and number of entities */
> + i = 0;
> + media_device_for_each_entity(entity, mdev) {
> + i++;
> +
> + if (ret || !topo->entities)
> + continue;

I would add:

if (i > topo->num_entities)
continue;

The copy_to_user can succeed, even if i > num_entities depending on how the
memory was allocated. So I would always check num_entities and refuse to go
beyond it.

> +
> + /* Copy fields to userspace struct if not error */
> + memset(, 0, sizeof(uentity));
> + uentity.id = entity->graph_obj.id;
> + strncpy(uentity.name, entity->name,
> + sizeof(uentity.name));
> +
> + if (copy_to_user(>entities[i - 1], , 
> sizeof(uentity)))
> + ret = -EFAULT;

I would just return here. If the user gives bogus values for this, then just
give up.

> + }
> + topo->num_entities = i;

What to do if topo->entities != NULL and i > topo->num_entities? Just return 0
or return ENOSPC? I'm in favor of ENOSPC since a partial topology result is
useless. But if ENOSPC is returned, then all num_foo values should be updated
with the current number of elements.

This behavior would be consistent with e.g. VIDIOC_G_EXT_CTRLS.

> +
> + /* Get interfaces and number of interfaces */
> + i = 0;
> + media_device_for_each_intf(intf, mdev) {
> + i++;
> +
> + if (ret || !topo->interfaces)
> + continue;
> +
> + memset(, 0, sizeof(uintf));
> +
> + /* Copy intf fields to userspace struct */
> + uintf.id = intf->graph_obj.id;
> + uintf.intf_type = intf->type;
> + uintf.flags = intf->flags;
> +
> + if (media_type(>graph_obj) == MEDIA_GRAPH_INTF_DEVNODE) {
> + struct media_intf_devnode *devnode;
> +
> + devnode = intf_to_devnode(intf);
> +
> + uintf.devnode.major = devnode->major;
> + uintf.devnode.minor = devnode->minor;
> + }
> +
> + if (copy_to_user(>interfaces[i - 1], , 
> sizeof(uintf)))
> + ret = -EFAULT;
> + }
> + topo->num_interfaces = i;
> +
> + /* Get pads and number of pads */
> + i = 0;
> + media_device_for_each_pad(pad, mdev) {
> + i++;
> +
> + if (ret || !topo->pads)
> + continue;
> +
> + memset(, 0, sizeof(upad));
> +
> + /* Copy pad fields to userspace struct */
> + upad.id = pad->graph_obj.id;
> + upad.entity_id = pad->entity->graph_obj.id;
> + upad.flags = pad->flags;
> +
> + if (copy_to_user(>pads[i - 1], , sizeof(upad)))
> + ret = -EFAULT;
> + }
> + topo->num_pads = i;
> +
> + /* Get links and number of links */
> + i = 0;
> + media_device_for_each_link(link, mdev) {
> + i++;
> +
> + if (ret || !topo->links)
> + continue;
> +
> + memset(, 0, sizeof(ulink));
> +
> + /* Copy link fields to userspace struct */
> + ulink.id = link->graph_obj.id;
> + ulink.source_id = link->gobj0->id;
> + ulink.sink_id = link->gobj1->id;
> + ulink.flags = link->flags;
> +
> + if (media_type(link->gobj0) != MEDIA_GRAPH_PAD)
> + ulink.flags |= MEDIA_NEW_LNK_FL_INTERFACE_LINK;
> +
> + if (copy_to_user(>links[i - 1], , sizeof(ulink)))
> + ret = -EFAULT;
> + }
> + topo->num_links = i;
> +
> + return ret;
> +}
> +
> +static long media_device_get_topology(struct media_device *mdev,
> +   struct media_v2_topology __user *utopo)
> +{
> + struct 

Re: [PATCH v8 51/55] [media] remove interface links at media_entity_unregister()

[Hans Verkuil]

On 08/30/2015 05:07 AM, Mauro Carvalho Chehab wrote:
> Interface links connected to an entity should be removed
> before being able of removing the entity.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index a91e1ec076a6..638c682b79c4 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -618,14 +618,30 @@ void media_device_unregister_entity(struct media_entity 
> *entity)
>   return;
>  
>   spin_lock(>lock);
> +
> + /* Remove interface links with this entity on it */
> + list_for_each_entry_safe(link, tmp, >links, graph_obj.list) {
> + if (media_type(link->gobj1) == MEDIA_GRAPH_ENTITY
> + && link->entity == entity) {

I don't think you need the == MEDIA_GRAPH_ENTITY check here. That should always 
be
true if link->entity == entity.

> + media_gobj_remove(>graph_obj);
> + kfree(link);
> + }
> + }
> +
> + /* Remove all data links that belong to this entity */
>   list_for_each_entry_safe(link, tmp, >links, list) {
>   media_gobj_remove(>graph_obj);
>   list_del(>list);
>   kfree(link);
>   }
> +
> + /* Remove all pads that belong to this entity */
>   for (i = 0; i < entity->num_pads; i++)
>   media_gobj_remove(>pads[i].graph_obj);
> +
> + /* Remove the entity */
>   media_gobj_remove(>graph_obj);
> +
>   spin_unlock(>lock);
>   entity->graph_obj.mdev = NULL;
>  }
> 

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html