cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Sat Dec 16 05:00:18 CET 2017 media-tree git hash:45267fed3e55845c5b4b279162b273040ae4f587 media_build git hash: 4058fea6b2507661d66af5298c048d7c55338f42 v4l-utils git hash: b638ef8b15813494e82148dccd9c0411daa214d9 gcc version:i686-linux-gcc (GCC) 7.1.0 sparse version: v0.5.0-3911-g6f737e1f smatch version: v0.5.0-3911-g6f737e1f host hardware: x86_64 host os:4.13.0-164 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.1.10-i686: ERRORS linux-3.2.37-i686: ERRORS linux-3.3.8-i686: ERRORS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: ERRORS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: ERRORS linux-3.10.1-i686: ERRORS linux-3.11.1-i686: ERRORS linux-3.12.67-i686: ERRORS linux-3.13.11-i686: ERRORS linux-3.14.9-i686: ERRORS linux-3.15.2-i686: ERRORS linux-3.16.7-i686: ERRORS linux-3.17.8-i686: ERRORS linux-3.18.7-i686: ERRORS linux-3.19-i686: ERRORS linux-4.0.9-i686: ERRORS linux-4.1.33-i686: ERRORS linux-4.2.8-i686: ERRORS linux-4.3.6-i686: ERRORS linux-4.4.22-i686: ERRORS linux-4.5.7-i686: ERRORS linux-4.6.7-i686: ERRORS linux-4.7.5-i686: ERRORS linux-4.8-i686: ERRORS linux-4.9.26-i686: ERRORS linux-4.10.14-i686: ERRORS linux-4.11-i686: ERRORS linux-4.12.1-i686: ERRORS linux-4.13-i686: ERRORS linux-4.14-i686: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.37-x86_64: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: ERRORS linux-3.10.1-x86_64: ERRORS linux-3.11.1-x86_64: ERRORS linux-3.12.67-x86_64: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.9-x86_64: ERRORS linux-3.15.2-x86_64: ERRORS linux-3.16.7-x86_64: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.7-x86_64: ERRORS linux-3.19-x86_64: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.33-x86_64: ERRORS linux-4.2.8-x86_64: ERRORS linux-4.3.6-x86_64: ERRORS linux-4.4.22-x86_64: ERRORS linux-4.5.7-x86_64: ERRORS linux-4.6.7-x86_64: ERRORS linux-4.7.5-x86_64: ERRORS linux-4.8-x86_64: ERRORS linux-4.9.26-x86_64: ERRORS linux-4.10.14-x86_64: ERRORS linux-4.11-x86_64: ERRORS linux-4.12.1-x86_64: ERRORS linux-4.13-x86_64: ERRORS linux-4.14-x86_64: ERRORS apps: OK spec-git: OK smatch: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: more build failures
On Fri, Dec 15, 2017 at 11:41:13PM +1100, Vincent McIntyre wrote: > > ... > > $ make allyesconfig > make -C /home/me/git/clones/media_build/v4l allyesconfig > make[1]: Entering directory '/home/me/git/clones/media_build/v4l' > No version yet, using 4.4.0-103-generic > make[2]: Entering directory '/home/me/git/clones/media_build/linux' > Syncing with dir ../media/ > Applying patches for kernel 4.4.0-103-generic > patch -s -f -N -p1 -i ../backports/api_version.patch > patch -s -f -N -p1 -i ../backports/pr_fmt.patch > patch -s -f -N -p1 -i ../backports/debug.patch > patch -s -f -N -p1 -i ../backports/drx39xxj.patch > patch -s -f -N -p1 -i ../backports/v4.14_compiler_h.patch > patch -s -f -N -p1 -i ../backports/v4.14_saa7146_timer_cast.patch > patch -s -f -N -p1 -i ../backports/v4.14_module_param_call.patch > patch -s -f -N -p1 -i ../backports/v4.12_revert_solo6x10_copykerneluser.patch > patch -s -f -N -p1 -i ../backports/v4.10_sched_signal.patch > 1 out of 1 hunk FAILED > The text leading up to this was: > -- > |diff --git a/drivers/staging/media/lirc/lirc_zilog.c > b/drivers/staging/media/lirc/lirc_zilog.c > |index 015e41bd036e..fd61081b47d9 100644 > |--- a/drivers/staging/media/lirc/lirc_zilog.c > |+++ b/drivers/staging/media/lirc/lirc_zilog.c > -- > No file to patch. Skipping patch. > 1 out of 1 hunk ignored > Makefile:130: recipe for target 'apply_patches' failed > make[2]: *** [apply_patches] Error 1 > make[2]: Leaving directory '/home/me/git/clones/media_build/linux' > Makefile:374: recipe for target 'allyesconfig' failed > make[1]: *** [allyesconfig] Error 2 > make[1]: Leaving directory '/home/me/git/clones/media_build/v4l' > Makefile:26: recipe for target 'allyesconfig' failed > make: *** [allyesconfig] Error 2 > can't select all drivers at ./build line 525 > + status=29 > + date > Friday 15 December 23:29:17 AEDT 2017 > + [ 0 = 29 ] I managed to get past the failure above with this change - media: rc: move ir-lirc-codec.c contents into lirc_dev.c media: lirc: remove last remnants of lirc kapi - Sean removed lirc_zilog.c, so it no longer needs patching --- a/backports/v4.10_sched_signal.patch +++ b/backports/v4.10_sched_signal.patch @@ -195,19 +195,6 @@ index 0e8025b7b4dd..8c59d4f53200 100644 #include #include #include -diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c -index db1e7b70c998..fc03068e22b5 100644 a/drivers/media/rc/lirc_dev.c -+++ b/drivers/media/rc/lirc_dev.c -@@ -18,7 +18,7 @@ - #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt - - #include --#include -+#include - #include - #include - #include diff --git a/drivers/media/usb/cpia2/cpia2_core.c b/drivers/media/usb/cpia2/cpia2_core.c index 0efba0da0a45..5d8aa65ab40b 100644 --- a/drivers/media/usb/cpia2/cpia2_core.c @@ -246,19 +233,6 @@ index 0b5c43f7e020..36bd904946bd 100644 #include #include -diff --git a/drivers/staging/media/lirc/lirc_zilog.c b/drivers/staging/media/lirc/lirc_zilog.c -index 015e41bd036e..fd61081b47d9 100644 a/drivers/staging/media/lirc/lirc_zilog.c -+++ b/drivers/staging/media/lirc/lirc_zilog.c -@@ -42,7 +42,7 @@ - #include - #include - #include --#include -+#include - #include - #include - #include However it falls over later in a way I don't think I can help with. ... CC [M] /home/me/git/clones/media_build/v4l/pvrusb2-i2c-core.o CC [M] /home/me/git/clones/media_build/v4l/pvrusb2-audio.o CC [M] /home/me/git/clones/media_build/v4l/pvrusb2-encoder.o CC [M] /home/me/git/clones/media_build/v4l/pvrusb2-video-v4l.o CC [M] /home/me/git/clones/media_build/v4l/pvrusb2-eeprom.o CC [M] /home/me/git/clones/media_build/v4l/pvrusb2-main.o CC [M] /home/me/git/clones/media_build/v4l/pvrusb2-hdw.o /home/me/git/clones/media_build/v4l/pvrusb2-hdw.c: In function 'pvr2_send_request_ex': /home/me/git/clones/media_build/v4l/pvrusb2-hdw.c:3651:7: error: implicit declaration of function 'usb_urb_ep_type_check' [-Werror=implicit-function-declaration] if (usb_urb_ep_type_check(hdw->ctl_write_urb)) { ^ cc1: some warnings being treated as errors scripts/Makefile.build:258: recipe for target '/home/me/git/clones/media_build/v4l/pvrusb2-hdw.o' failed make[3]: *** [/home/me/git/clones/media_build/v4l/pvrusb2-hdw.o] Error 1 Makefile:1423: recipe for target '_module_/home/me/git/clones/media_build/v4l' failed make[2]: *** [_module_/home/me/git/clones/media_build/v4l] Error 2 make[2]: Leaving directory '/usr/src/linux-headers-4.4.0-103-generic' Makefile:51: recipe for target 'default' failed make[1]: *** [default] Error 2 make[1]: Leaving directory '/home/me/git/clones/media_build/v4l' Makefile:26: recipe for target 'all' failed make: *** [all] Error 2 Cheers Vince
[RFC 4/5] ARM: dts: sun8i: a83t: Add support for the ir interface
The ir interface is like the H3 at 0x01f02000 located and is exactly the same. This patch adds support for the ir interface on the A83T. Signed-off-by: Philipp Rossak --- arch/arm/boot/dts/sun8i-a83t.dtsi | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi index 5edb645b506f..5dbf2f0891c1 100644 --- a/arch/arm/boot/dts/sun8i-a83t.dtsi +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi @@ -470,6 +470,16 @@ #reset-cells = <1>; }; + ir: ir@01f02000 { + compatible = "allwinner,sun5i-a13-ir"; + clocks = <&r_ccu CLK_APB0_IR>, <&r_ccu CLK_IR>; + clock-names = "apb", "ir"; + resets = <&r_ccu RST_APB0_IR>; + interrupts = ; + reg = <0x01f02000 0x40>; + status = "disabled"; + }; + r_pio: pinctrl@1f02c00 { compatible = "allwinner,sun8i-a83t-r-pinctrl"; reg = <0x01f02c00 0x400>; -- 2.11.0
[RFC 5/5] ARM: dts: sun8i: a83t: bananapi-m3: Enable IR controller
The Bananapi M3 has an onboard IR receiver. This enables the onboard IR receiver subnode. Other than the other IR receivers this one needs a base clock frequency of 300 Hz (3 MHz), to be able to work. Signed-off-by: Philipp Rossak --- arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 7 +++ arch/arm/boot/dts/sun8i-a83t.dtsi| 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts index c606af3dbfed..2c92c501cd59 100644 --- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts +++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts @@ -88,6 +88,13 @@ /* TODO GL830 USB-to-SATA bridge downstream w/ GPIO power controls */ }; +&ir { + pinctrl-names = "default"; + pinctrl-0 = <&ir_pins_a>; + base-clk-frequency = <300>; + status = "okay"; +}; + &mmc0 { pinctrl-names = "default"; pinctrl-0 = <&mmc0_pins>; diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi index 5dbf2f0891c1..679ce3a66b4b 100644 --- a/arch/arm/boot/dts/sun8i-a83t.dtsi +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi @@ -470,7 +470,7 @@ #reset-cells = <1>; }; - ir: ir@01f02000 { + ir: ir@1f02000 { compatible = "allwinner,sun5i-a13-ir"; clocks = <&r_ccu CLK_APB0_IR>, <&r_ccu CLK_IR>; clock-names = "apb", "ir"; -- 2.11.0
[RFC 2/5] [media] dt: bindings: Update binding documentation for sunxi IR controller
This patch updates documentation for Device-Tree bindings for sunxi IR controller and adds the new requiered property for the base clock frequency. Signed-off-by: Philipp Rossak --- Documentation/devicetree/bindings/media/sunxi-ir.txt | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt b/Documentation/devicetree/bindings/media/sunxi-ir.txt index 91648c569b1e..5f4960c61245 100644 --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt @@ -1,12 +1,13 @@ Device-Tree bindings for SUNXI IR controller found in sunXi SoC family Required properties: -- compatible : "allwinner,sun4i-a10-ir" or "allwinner,sun5i-a13-ir" -- clocks : list of clock specifiers, corresponding to - entries in clock-names property; -- clock-names : should contain "apb" and "ir" entries; -- interrupts : should contain IR IRQ number; -- reg : should contain IO map address for IR. +- compatible : "allwinner,sun4i-a10-ir" or "allwinner,sun5i-a13-ir" +- clocks : list of clock specifiers, corresponding to + entries in clock-names property; +- clock-names: should contain "apb" and "ir" entries; +- interrupts : should contain IR IRQ number; +- reg: should contain IO map address for IR. +- base-clk-frequency : should contain the base clock frequency Optional properties: - linux,rc-map-name: see rc.txt file in the same directory. @@ -21,5 +22,6 @@ ir0: ir@1c21800 { resets = <&apb0_rst 1>; interrupts = <0 5 1>; reg = <0x01C21800 0x40>; + base-clk-frequency = <800>; linux,rc-map-name = "rc-rc6-mce"; }; -- 2.11.0
[RFC 3/5] ARM: dts: sun8i: a83t: Add the ir pin for the A83T
The CIR Pin of the A83T is located at PL12 Signed-off-by: Philipp Rossak --- arch/arm/boot/dts/sun8i-a83t.dtsi | 5 + 1 file changed, 5 insertions(+) diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi index 19acae1b4089..5edb645b506f 100644 --- a/arch/arm/boot/dts/sun8i-a83t.dtsi +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi @@ -488,6 +488,11 @@ drive-strength = <20>; bias-pull-up; }; + + ir_pins_a: ir@0 { + pins = "PL12"; + function = "s_cir_rx"; + }; }; r_rsb: rsb@1f03400 { -- 2.11.0
[RFC 1/5] [media] rc: update sunxi-ir driver to get base frequency from devicetree
This patch updates the sunxi-ir driver to set the ir base clock from devicetree. This is neccessary since there are different ir recievers on the market, that operate with different frequencys. So this value needs to be set depending on the attached receiver. Signed-off-by: Philipp Rossak --- drivers/media/rc/sunxi-cir.c | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c index 97f367b446c4..55b53d6463e9 100644 --- a/drivers/media/rc/sunxi-cir.c +++ b/drivers/media/rc/sunxi-cir.c @@ -72,12 +72,6 @@ /* CIR_REG register idle threshold */ #define REG_CIR_ITHR(val)(((val) << 8) & (GENMASK(15, 8))) -/* Required frequency for IR0 or IR1 clock in CIR mode */ -#define SUNXI_IR_BASE_CLK 800 -/* Frequency after IR internal divider */ -#define SUNXI_IR_CLK (SUNXI_IR_BASE_CLK / 64) -/* Sample period in ns */ -#define SUNXI_IR_SAMPLE (10ul / SUNXI_IR_CLK) /* Noise threshold in samples */ #define SUNXI_IR_RXNOISE 1 /* Idle Threshold in samples */ @@ -122,7 +116,7 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id) /* for each bit in fifo */ dt = readb(ir->base + SUNXI_IR_RXFIFO_REG); rawir.pulse = (dt & 0x80) != 0; - rawir.duration = ((dt & 0x7f) + 1) * SUNXI_IR_SAMPLE; + rawir.duration = ((dt & 0x7f) + 1) * ir->rc->rx_resolution; ir_raw_event_store_with_filter(ir->rc, &rawir); } } @@ -148,6 +142,7 @@ static int sunxi_ir_probe(struct platform_device *pdev) struct device_node *dn = dev->of_node; struct resource *res; struct sunxi_ir *ir; + u32 b_clk_freq; ir = devm_kzalloc(dev, sizeof(struct sunxi_ir), GFP_KERNEL); if (!ir) @@ -172,6 +167,12 @@ static int sunxi_ir_probe(struct platform_device *pdev) return PTR_ERR(ir->clk); } + /* Required frequency for IR0 or IR1 clock in CIR mode */ + if (of_property_read_u32(dn, "base-clk-frequency", &b_clk_freq)) { + dev_err(dev, "failed to get ir base clock frequency.\n"); + return -ENODATA; + } + /* Reset (optional) */ ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL); if (IS_ERR(ir->rst)) @@ -180,7 +181,7 @@ static int sunxi_ir_probe(struct platform_device *pdev) if (ret) return ret; - ret = clk_set_rate(ir->clk, SUNXI_IR_BASE_CLK); + ret = clk_set_rate(ir->clk, b_clk_freq); if (ret) { dev_err(dev, "set ir base clock failed!\n"); goto exit_reset_assert; @@ -225,7 +226,8 @@ static int sunxi_ir_probe(struct platform_device *pdev) ir->rc->map_name = ir->map_name ?: RC_MAP_EMPTY; ir->rc->dev.parent = dev; ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER; - ir->rc->rx_resolution = SUNXI_IR_SAMPLE; + /* Frequency after IR internal divider with sample period in ns */ + ir->rc->rx_resolution = (10ul / (b_clk_freq / 64)); ir->rc->timeout = MS_TO_NS(SUNXI_IR_TIMEOUT); ir->rc->driver_name = SUNXI_IR_DEV; -- 2.11.0
[RFC 0/5] IR support for A83T - sunxi-ir driver update
This patch series adds support for the sunxi A83T ir module and enhances the sunxi-ir driver. Right now the base clock frequency for the ir driver is a hard coded define and is set to 8 MHz. This works for the most common ir receivers. On the Sinovoip Bananapi M3 the ir receiver needs, a 3 MHz base clock frequency to work without problems with this driver (like in the legacy kernel). To fix this issue I reworked the driver that this value could be set over the devicetree. About 37 devices would have a devicetree change if this patch series would be applied. Therfore I would like to ask you to give me some feedback about the patch series, before I finialize it. Thanks in advance! Philipp Philipp Rossak (5): [media] rc: update sunxi-ir driver to get base frequency from devicetree [media] dt: bindings: Update binding documentation for sunxi IR controller ARM: dts: sun8i: a83t: Add the ir pin for the A83T ARM: dts: sun8i: a83t: Add support for the ir interface ARM: dts: sun8i: a83t: bananapi-m3: Enable IR controller Documentation/devicetree/bindings/media/sunxi-ir.txt | 14 -- arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 7 +++ arch/arm/boot/dts/sun8i-a83t.dtsi| 15 +++ drivers/media/rc/sunxi-cir.c | 20 +++- 4 files changed, 41 insertions(+), 15 deletions(-) -- 2.11.0
Re: [PATCH 1/2] media: dt-bindings: coda: Add compatible for CodaHx4 on i.MX51
On Wed, Dec 13, 2017 at 03:09:17PM +0100, Philipp Zabel wrote: > Add a compatible for the CodaHx4 VPU used on i.MX51. > > Signed-off-by: Philipp Zabel > --- > Documentation/devicetree/bindings/media/coda.txt | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Rob Herring
[BUG] NULL pointer dereference in pvr2_v4l2_internal_check
Dear all, Unplugging the TV tuner (WinTV HVR-1900) from USB port causes a NULL pointer dereference in pvr2_v4l2_internal_check: [ 2128.129776] usb 1-1: USB disconnect, device number 6 [ 2128.129987] pvrusb2: Device being rendered inoperable [ 2128.130055] BUG: unable to handle kernel NULL pointer dereference at 0360 [ 2128.130082] IP: pvr2_v4l2_internal_check+0x41/0x60 [pvrusb2] [ 2128.130085] PGD 0 P4D 0 [ 2128.130092] Oops: [#1] PREEMPT SMP [ 2128.130097] Modules linked in: tda10048 tda18271 tda8290 tuner lirc_zilog(C) lirc_dev cx25840 rc_core pvrusb2(O) tveeprom cx2341x dvb_core v4l2_common rfcomm af_packet 8021q garp mrp stp llc nf_log_ipv6 xt_comment nf_log_ipv4 nf_log_common xt_LOG xt_limit ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 ipt_REJECT nf_reject_ipv4 xt_pkttype xt_tcpudp iptable_filter snd_hda_codec_hdmi ip6table_mangle nf_conntrack_netbios_ns nf_conntrack_broadcast nf_conntrack_ipv4 nf_defrag_ipv4 ip_tables xt_conntrack nf_conntrack ip6table_filter ip6_tables x_tables bnep arc4 xfs libcrc32c snd_hda_codec_realtek intel_rapl snd_hda_codec_generic x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm iwlmvm snd_hda_intel raid1 irqbypass crct10dif_pclmul snd_hda_codec mac80211 crc32_pclmul snd_hda_core [ 2128.130176] ghash_clmulni_intel snd_hwdep pcbc snd_pcm iwlwifi snd_timer dell_laptop aesni_intel md_mod hid_multitouch dell_wmi iTCO_wdt aes_x86_64 snd rtsx_pci_ms iTCO_vendor_support btusb crypto_simd uvcvideo dell_smbios btrtl glue_helper dell_smm_hwmon wmi_bmof dcdbas joydev hci_uart cryptd pcspkr cfg80211 videobuf2_vmalloc memstick btbcm serdev videobuf2_memops r8169 btqca soundcore btintel videobuf2_v4l2 mii int3403_thermal i2c_i801 videobuf2_core videodev bluetooth battery ac sparse_keymap ecdh_generic fan thermal idma64 pinctrl_sunrisepoint pinctrl_intel tpm_crb tpm_tis tpm_tis_core tpm processor_thermal_device intel_lpss_acpi intel_soc_dts_iosf int3402_thermal dell_rbtn int340x_thermal_zone mei_me rfkill int3400_thermal intel_lpss_pci acpi_pad mei acpi_thermal_rel intel_lpss intel_pch_thermal [ 2128.130252] acpi_als kfifo_buf shpchp industrialio btrfs xor zstd_decompress zstd_compress xxhash hid_generic usbhid i915 raid6_pq rtsx_pci_sdmmc mmc_core mxm_wmi crc32c_intel i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt xhci_pci fb_sys_fops serio_raw xhci_hcd rtsx_pci drm usbcore wmi video i2c_hid button sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua [ 2128.130300] CPU: 6 PID: 2310 Comm: pvrusb2-context Tainted: G C O4.14.6-1.g45f120a-default #1 [ 2128.130303] Hardware name: Dell Inc. Inspiron 7559/0H0CC0, BIOS 1.1.8 04/17/2016 [ 2128.130306] task: 880cae4f6000 task.stack: b3a7c2548000 [ 2128.130320] RIP: 0010:pvr2_v4l2_internal_check+0x41/0x60 [pvrusb2] [ 2128.130324] RSP: 0018:b3a7c254bec8 EFLAGS: 00010246 [ 2128.130328] RAX: RBX: 880caf05e780 RCX: c0ffe970 [ 2128.130331] RDX: 880c90ca1b60 RSI: 0001 RDI: [ 2128.130334] RBP: 880cac83eb00 R08: c1016a78 R09: 03d2 [ 2128.130337] R10: 03a9 R11: 003d0900 R12: b3a7c24ffc18 [ 2128.130340] R13: 880cae4f6000 R14: R15: c1000ae0 [ 2128.130344] FS: () GS:880cc1d8() knlGS: [ 2128.130347] CS: 0010 DS: ES: CR0: 80050033 [ 2128.130350] CR2: 0360 CR3: 00024ec09005 CR4: 003606e0 [ 2128.130354] DR0: DR1: DR2: [ 2128.130357] DR3: DR6: fffe0ff0 DR7: 0400 [ 2128.130359] Call Trace: [ 2128.130378] pvr2_context_thread_func+0xa6/0x2a0 [pvrusb2] [ 2128.130388] ? finish_wait+0x80/0x80 [ 2128.130394] kthread+0x118/0x130 [ 2128.130399] ? kthread_create_on_node+0x40/0x40 [ 2128.130406] ret_from_fork+0x25/0x30 [ 2128.130412] Code: 8b 7f 38 e8 d2 e4 ff ff 48 8b 7b 40 e8 c9 e4 ff ff 48 8b 43 38 48 8b 90 60 03 00 00 48 05 60 03 00 00 48 39 d0 75 d6 48 8b 43 40 <48> 8b 90 60 03 00 00 48 05 60 03 00 00 48 39 d0 75 c0 48 89 df [ 2128.130491] RIP: pvr2_v4l2_internal_check+0x41/0x60 [pvrusb2] RSP: b3a7c254bec8 [ 2128.130494] CR2: 0360 [ 2128.130499] ---[ end trace b7d1a2a4867177f2 ]--- Upon reconnect the device is no longer recognized by the driver and no firmware is uploaded: [ 2135.323115] usb 1-1: new high-speed USB device number 7 using xhci_hcd [ 2135.481292] usb 1-1: New USB device found, idVendor=2040, idProduct=7300 [ 2135.481302] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 2135.481306] usb 1-1: Product: WinTV [ 2135.481310] usb 1-1: Manufacturer: Hauppauge [ 2135.481313] usb 1-1: SerialNumber: 7300-00-F04BADA0 [ 2135.482726] pvrusb2: Hardware description: WinTV HVR-1900 Model 73xxx This effectively breaks the driver until after a reboot of the kernel. Can this be fi
[PATCH] [media] netup_unidvb: use PCI_EXP_DEVCTL2_COMP_TIMEOUT macro
From: Bjorn Helgaas Use the existing PCI_EXP_DEVCTL2_COMP_TIMEOUT macro instead of hard-coding the PCIe Completion Timeout Value mask. No functional change intended. Signed-off-by: Bjorn Helgaas --- drivers/media/pci/netup_unidvb/netup_unidvb_core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/pci/netup_unidvb/netup_unidvb_core.c b/drivers/media/pci/netup_unidvb/netup_unidvb_core.c index 11829c0fa138..6cf88a0bf458 100644 --- a/drivers/media/pci/netup_unidvb/netup_unidvb_core.c +++ b/drivers/media/pci/netup_unidvb/netup_unidvb_core.c @@ -862,7 +862,7 @@ static int netup_unidvb_initdev(struct pci_dev *pci_dev, PCI_EXP_DEVCTL_NOSNOOP_EN, 0); /* Adjust PCIe completion timeout. */ pcie_capability_clear_and_set_word(pci_dev, - PCI_EXP_DEVCTL2, 0xf, 0x2); + PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_COMP_TIMEOUT, 0x2); if (netup_unidvb_request_mmio(pci_dev)) { dev_err(&pci_dev->dev,
Re: [PATCH] pvrusb2: correctly return V4L2_PIX_FMT_MPEG in enum_fmt
Dear Hans, With your patch applied to kernel 4.14.6 on openSUSE I can get the video working, changing channels also works. But I don't have any audio output. Even if I just do: $ cat /dev/video0 > test.mpg then when I open test.mpg with vlc there is no sound as if it is muted. I tried 'v4l2-ctl -c mute=0', 'v4l2-ctl --set-audio-input=0; v4l2-ctl --set-audio-input=1' so far but nothing changed. Is there something else I could try to get audio output? Best, Oleksandr BTW, on Mint with kernel 4.13.0 regardless of the patch I get a kernel panic as soon as I plug in the device. Something fishy going on there. On 14.12.17 10:54, Hans Verkuil wrote: On 14/12/17 10:52, Oleksandr Ostrenko wrote: On Thursday, December 14, 2017 12:44:42 AM CET Hans Verkuil wrote: The pvrusb2 code appears to have a some old workaround code for xawtv that causes a WARN() due to an unrecognized pixelformat 0 in v4l2_ioctl.c. Since all other MPEG drivers fill this in correctly, it is a safe assumption that this particular problem no longer exists. While I'm at it, clean up the code a bit. Signed-off-by: Hans Verkuil --- I'll try to give this a spin in the morning with xawtv and my ivtv card (that also uses V4L2_PIX_FMT_MPEG), just to make sure xawtv no longer breaks if it sees it. Oleksandr, are you able to test this as well on your pvrusb2? Thanks, Hans, this fixes the original issue on Linux Mint with kernel 4.8.17. Haven't tried it on openSUSE yet. Still, in xawtv I get no TV reception but just a black screen and error messages like: no way to get: 128x96 32 bit TrueColor (LE: bgr-) no way to get: 128x96 32 bit TrueColor (LE: bgr-) no way to get: 128x96 32 bit TrueColor (LE: bgr-) no way to get: 128x96 32 bit TrueColor (LE: bgr-) no way to get: 384x288 32 bit TrueColor (LE: bgr-) Is this another bug? No. xawtv simply doesn't support MPEG formats. So this is what I would expect. Regards, Hans Best, Oleksandr Regards, Hans --- diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c index 4320bda9352d..cc90be364a30 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c @@ -78,18 +78,6 @@ static int vbi_nr[PVR_NUM] = {[0 ... PVR_NUM-1] = -1}; module_param_array(vbi_nr, int, NULL, 0444); MODULE_PARM_DESC(vbi_nr, "Offset for device's vbi dev minor"); -static struct v4l2_fmtdesc pvr_fmtdesc [] = { - { - .index = 0, - .type = V4L2_BUF_TYPE_VIDEO_CAPTURE, - .flags = V4L2_FMT_FLAG_COMPRESSED, - .description= "MPEG1/2", - // This should really be V4L2_PIX_FMT_MPEG, but xawtv - // breaks when I do that. - .pixelformat= 0, // V4L2_PIX_FMT_MPEG, - } -}; - #define PVR_FORMAT_PIX 0 #define PVR_FORMAT_VBI 1 @@ -99,17 +87,11 @@ static struct v4l2_format pvr_format [] = { .fmt= { .pix= { .width = 720, - .height = 576, - // This should really be V4L2_PIX_FMT_MPEG, - // but xawtv breaks when I do that. - .pixelformat= 0, // V4L2_PIX_FMT_MPEG, + .height = 576, + .pixelformat= V4L2_PIX_FMT_MPEG, .field = V4L2_FIELD_INTERLACED, - .bytesperline = 0, // doesn't make sense - // here - //FIXME : Don't know what to put here... - .sizeimage = (32*1024), - .colorspace = 0, // doesn't make sense here - .priv = 0 + /* FIXME : Don't know what to put here... */ + .sizeimage = 32 * 1024, } } }, @@ -407,11 +389,11 @@ static int pvr2_g_frequency(struct file *file, void *priv, struct v4l2_frequency static int pvr2_enum_fmt_vid_cap(struct file *file, void *priv, struct v4l2_fmtdesc *fd) { - /* Only one format is supported : mpeg.*/ - if (fd->index != 0) + /* Only one format is supported: MPEG. */ + if (fd->index) return -EINVAL; - memcpy(fd, pvr_fmtdesc, sizeof(struct v4l2_fmtdesc)); + fd->pixelformat = V4L2_PIX_FMT_MPEG; return 0; } smime.p7s Description: S/MIME Cryptographic Signature
[GIT PULL for 4.16 v2] Intel IPU3 CIO2 CSI-2 receiver driver
Hi Mauro, Here's the Intel IPU3 CIO2 CSI-2 receiver driver, with the accompanying format definitions. Since the previous pull request dealing with this, I've squashed Yong's patch to the patch introducing the driver, addressing two issues: - unused "phys" variable and - memory allocated on stack based on a function argument. https://patchwork.linuxtv.org/patch/45991/> Please pull. The following changes since commit b32a2b42f76cdfd06b4b58a1ddf987ba329ae34e: media: ddbridge: improve error handling logic on fe attach failures (2017-12-13 10:19:41 -0500) are available in the git repository at: ssh://linuxtv.org/git/sailus/media_tree.git ipu3 for you to fetch changes up to 012b03bf357ecb0807c790e8f0b3bcff7e079ae2: intel-ipu3: cio2: add new MIPI-CSI2 driver (2017-12-15 17:56:04 +0200) Yong Zhi (3): videodev2.h, v4l2-ioctl: add IPU3 raw10 color format doc-rst: add IPU3 raw10 bayer pixel format definitions intel-ipu3: cio2: add new MIPI-CSI2 driver Documentation/media/uapi/v4l/pixfmt-rgb.rst|1 + .../media/uapi/v4l/pixfmt-srggb10-ipu3.rst | 335 MAINTAINERS|8 + drivers/media/pci/Kconfig |2 + drivers/media/pci/Makefile |3 +- drivers/media/pci/intel/Makefile |5 + drivers/media/pci/intel/ipu3/Kconfig | 19 + drivers/media/pci/intel/ipu3/Makefile |1 + drivers/media/pci/intel/ipu3/ipu3-cio2.c | 2048 drivers/media/pci/intel/ipu3/ipu3-cio2.h | 449 + drivers/media/v4l2-core/v4l2-ioctl.c |4 + include/uapi/linux/videodev2.h |6 + 12 files changed, 2880 insertions(+), 1 deletion(-) create mode 100644 Documentation/media/uapi/v4l/pixfmt-srggb10-ipu3.rst create mode 100644 drivers/media/pci/intel/Makefile create mode 100644 drivers/media/pci/intel/ipu3/Kconfig create mode 100644 drivers/media/pci/intel/ipu3/Makefile create mode 100644 drivers/media/pci/intel/ipu3/ipu3-cio2.c create mode 100644 drivers/media/pci/intel/ipu3/ipu3-cio2.h -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Hi, On Fri, Dec 15, 2017 at 07:01:40PM +0800, Yong wrote: > Hi Maxime, > > On Fri, 15 Dec 2017 11:50:47 +0100 > Maxime Ripard wrote: > > > Hi Yong, > > > > On Mon, Dec 04, 2017 at 05:45:11PM +0800, Yong wrote: > > > I just noticed that you are using the second iteration? > > > Have you received my third iteration? > > > > Sorry for the late reply, and for not coming back to you yet about > > that test. No, this is still in your v2. I've definitely received your > > v3, I just didn't have time to update to it yet. > > > > But don't worry, my mail was mostly to know if you had tested that > > setup on your side to try to nail down the issue on my end, not really > > a review or comment that would prevent your patch from going in. > > I mean, > The v2 exactly has a bug which may cause the CSI writing frame to > a wrong memory address. Ah, sorry I misunderstood you then. I'll definitely test your v3.. > BTW, should I send a new version. I have made some improve sine v3. .. or your v4 :) Yes, please send a new version. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [RFC PATCH 0/9] media: base request API support
Le vendredi 15 décembre 2017 à 16:56 +0900, Alexandre Courbot a écrit : > Here is a new attempt at the request API, following the UAPI we agreed on in > Prague. Hopefully this can be used as the basis to move forward. > > This series only introduces the very basics of how requests work: allocate a > request, queue buffers to it, queue the request itself, wait for it to > complete, > reuse it. It does *not* yet use Hans' work with controls setting. I have > preferred to submit it this way for now as it allows us to concentrate on the > basic request/buffer flow, which was harder to get properly than I initially > thought. I still have a gut feeling that it can be improved, with less > back-and- > forth into drivers. > > Plugging in controls support should not be too hard a task (basically just > apply > the saved controls when the request starts), and I am looking at it now. > > The resulting vim2m driver can be successfully used with requests, and my > tests > so far have been successful. > > There are still some rougher edges: > > * locking is currently quite coarse-grained > * too many #ifdef CONFIG_MEDIA_CONTROLLER in the code, as the request API > depends on it - I plan to craft the headers so that it becomes unnecessary. > As it is, some of the code will probably not even compile if > CONFIG_MEDIA_CONTROLLER is not set Would it be possible to explain why this relation between request and the media controller ? Why couldn't request be created from video devices ? > > But all in all I think the request flow should be clear and easy to review, > and > the possibility of custom queue and entity support implementations should give > us the flexibility we need to support more specific use-cases (I expect the > generic implementations to be sufficient most of the time though). > > A very simple test program exercising this API is available here (don't forget > to adapt the /dev/media0 hardcoding): > https://gist.github.com/Gnurou/dbc3776ed97ea7d4ce6041ea15eb0438 > > Looking forward to your feedback and comments! > > Alexandre Courbot (8): > media: add request API core and UAPI > media: request: add generic queue > media: request: add generic entity ops > media: vb2: add support for requests > media: vb2: add support for requests in QBUF ioctl > media: v4l2-mem2mem: add request support > media: vim2m: add media device > media: vim2m: add request support > > Hans Verkuil (1): > videodev2.h: Add request field to v4l2_buffer > > drivers/media/Makefile| 4 +- > drivers/media/media-device.c | 6 + > drivers/media/media-request-entity-generic.c | 56 > drivers/media/media-request-queue-generic.c | 150 ++ > drivers/media/media-request.c | 390 > ++ > drivers/media/platform/vim2m.c| 46 +++ > drivers/media/usb/cpia2/cpia2_v4l.c | 2 +- > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 +- > drivers/media/v4l2-core/v4l2-ioctl.c | 99 ++- > drivers/media/v4l2-core/v4l2-mem2mem.c| 34 +++ > drivers/media/v4l2-core/videobuf2-core.c | 59 +++- > drivers/media/v4l2-core/videobuf2-v4l2.c | 32 ++- > include/media/media-device.h | 3 + > include/media/media-entity.h | 6 + > include/media/media-request.h | 282 +++ > include/media/v4l2-mem2mem.h | 19 ++ > include/media/videobuf2-core.h| 25 +- > include/media/videobuf2-v4l2.h| 2 + > include/uapi/linux/media.h| 11 + > include/uapi/linux/videodev2.h| 3 +- > 20 files changed, 1216 insertions(+), 20 deletions(-) > create mode 100644 drivers/media/media-request-entity-generic.c > create mode 100644 drivers/media/media-request-queue-generic.c > create mode 100644 drivers/media/media-request.c > create mode 100644 include/media/media-request.h > signature.asc Description: This is a digitally signed message part
Re: [RFC PATCH 0/9] media: base request API support
Le vendredi 15 décembre 2017 à 16:56 +0900, Alexandre Courbot a écrit : > Here is a new attempt at the request API, following the UAPI we agreed on in > Prague. Hopefully this can be used as the basis to move forward. > > This series only introduces the very basics of how requests work: allocate a > request, queue buffers to it, queue the request itself, wait for it to > complete, > reuse it. It does *not* yet use Hans' work with controls setting. I have > preferred to submit it this way for now as it allows us to concentrate on the > basic request/buffer flow, which was harder to get properly than I initially > thought. I still have a gut feeling that it can be improved, with less > back-and- > forth into drivers. > > Plugging in controls support should not be too hard a task (basically just > apply > the saved controls when the request starts), and I am looking at it now. > > The resulting vim2m driver can be successfully used with requests, and my > tests > so far have been successful. > > There are still some rougher edges: > > * locking is currently quite coarse-grained > * too many #ifdef CONFIG_MEDIA_CONTROLLER in the code, as the request API > depends on it - I plan to craft the headers so that it becomes unnecessary. > As it is, some of the code will probably not even compile if > CONFIG_MEDIA_CONTROLLER is not set > > But all in all I think the request flow should be clear and easy to review, > and > the possibility of custom queue and entity support implementations should give > us the flexibility we need to support more specific use-cases (I expect the > generic implementations to be sufficient most of the time though). > > A very simple test program exercising this API is available here (don't forget > to adapt the /dev/media0 hardcoding): > https://gist.github.com/Gnurou/dbc3776ed97ea7d4ce6041ea15eb0438 It looks like the example uses Hans control work you just mention. Notably, it uses v4l2_ext_controls ctrls.request. > > Looking forward to your feedback and comments! > > Alexandre Courbot (8): > media: add request API core and UAPI > media: request: add generic queue > media: request: add generic entity ops > media: vb2: add support for requests > media: vb2: add support for requests in QBUF ioctl > media: v4l2-mem2mem: add request support > media: vim2m: add media device > media: vim2m: add request support > > Hans Verkuil (1): > videodev2.h: Add request field to v4l2_buffer > > drivers/media/Makefile| 4 +- > drivers/media/media-device.c | 6 + > drivers/media/media-request-entity-generic.c | 56 > drivers/media/media-request-queue-generic.c | 150 ++ > drivers/media/media-request.c | 390 > ++ > drivers/media/platform/vim2m.c| 46 +++ > drivers/media/usb/cpia2/cpia2_v4l.c | 2 +- > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 +- > drivers/media/v4l2-core/v4l2-ioctl.c | 99 ++- > drivers/media/v4l2-core/v4l2-mem2mem.c| 34 +++ > drivers/media/v4l2-core/videobuf2-core.c | 59 +++- > drivers/media/v4l2-core/videobuf2-v4l2.c | 32 ++- > include/media/media-device.h | 3 + > include/media/media-entity.h | 6 + > include/media/media-request.h | 282 +++ > include/media/v4l2-mem2mem.h | 19 ++ > include/media/videobuf2-core.h| 25 +- > include/media/videobuf2-v4l2.h| 2 + > include/uapi/linux/media.h| 11 + > include/uapi/linux/videodev2.h| 3 +- > 20 files changed, 1216 insertions(+), 20 deletions(-) > create mode 100644 drivers/media/media-request-entity-generic.c > create mode 100644 drivers/media/media-request-queue-generic.c > create mode 100644 drivers/media/media-request.c > create mode 100644 include/media/media-request.h > signature.asc Description: This is a digitally signed message part
[PATCH] staging: android: ion: Fix dma direction for dma_sync_sg_for_cpu/device
Use the direction argument passed into begin_cpu_access and end_cpu_access when calling the dma_sync_sg_for_cpu/device. The actual cache primitive called depends on the direction passed in. Signed-off-by: Sushmita Susheelendra --- drivers/staging/android/ion/ion.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index a7d9b0e..f480885 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -346,7 +346,7 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, mutex_lock(&buffer->lock); list_for_each_entry(a, &buffer->attachments, list) { dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents, - DMA_BIDIRECTIONAL); + direction); } mutex_unlock(&buffer->lock); @@ -368,7 +368,7 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, mutex_lock(&buffer->lock); list_for_each_entry(a, &buffer->attachments, list) { dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents, - DMA_BIDIRECTIONAL); + direction); } mutex_unlock(&buffer->lock); -- 1.9.1
Re: [PATCH] [media] tda18212: fix use-after-free in tda18212_remove()
On Fri, 15 Dec 2017 21:06:32 +0200 Antti Palosaari wrote: > On 12/15/2017 08:40 PM, Daniel Scheller wrote: > > On Fri, 15 Dec 2017 20:12:18 +0200 > > Antti Palosaari wrote: > >> > >> em28xx does it currently just correct. > >> 1) unregister frontend > > > > Note that this is a call to em28xx_unregister_dvb(), which in turn > > does dvb_unregister_frontend() and then dvb_frontend_detach() (at > > this stage, fe resources are gone). > > > >> 2) remove I2C SEC > >> 3) remove I2C tuner > >> 4) remove I2C demod (frees shared frontend data) > > > > Yes, but ie. EM2874_BOARD_KWORLD_UB435Q_V3 is a combination of a > > "legacy" demod frontend - lgdt3305 actually - plus the tda18212 > > i2cclient (just like in ddb with stv0367+tda18212 or > > cxd2841er+tda18212), I'm sure this will yield the same report. > > > > Maybe another approach: Implement the tuner_ops.release callback, > > and then move the memset+NULL assignment right there (instead of > > just removing it), but this likely will cause issues when the i2c > > client is removed before detach if we don't keep track of this ie > > somewhere in tda18212_dev (new state var - if _remove is called, > > check if the tuner was released, and if not, call release > > (memset/set NULL), then free). Still with the two other drivers in > > mind though. If they're wrong aswell, I'll rather fix up ddbridge > > of course. > > Whole memset thing could be removed from tda18212, there is something > likely wrong if those are needed. But it is another issue. On a side note: After some few more glances, there are many other drivers in media/tuners/ that would require such treatment (tbh I just peeked into tda18212 when investigating the KASAN report). > Your main issue is somehow to get order of demod/tuner destroy > correct. I don't even like idea whole shared frontend data is owned > by the demod driver instance, but currently it is there and due to > that this should be released lastly. General design goal is also do > things like register things in order and unregister just > reverse-order. Fully agreeing on the last bit. I'll see how to improve on this in ddbridge. Yet, very likely other drivers (and I have a feeling there are quite some) with this issue (wrong teardown order) remain. It might indeed be better if in frontend_ops, tuner_ops would be a ptr to some struct that is managed by the tuner driver itself, that would save from such issues. Anyway, thank you very much for your input! (and apologies for any noise or nonsense) Best regards, Daniel Scheller -- https://github.com/herrnst
Re: [PATCH] [media] tda18212: fix use-after-free in tda18212_remove()
On 12/15/2017 08:40 PM, Daniel Scheller wrote: On Fri, 15 Dec 2017 20:12:18 +0200 Antti Palosaari wrote: On 12/15/2017 08:00 PM, Daniel Scheller wrote: Hi, On Fri, 15 Dec 2017 19:30:18 +0200 Antti Palosaari wrote: Thanks for your reply. Hello I think shared frontend structure, which is owned by demod driver, should be there and valid on time tuner driver is removed. And thus should not happen. Did you make driver unload on different order eg. not just reverse order than driver load? IMHO these should go always on load: 1) load demod driver (which makes shared frontend structure where also some tuner driver data lives) 2) load tuner driver 3) register frontend on unload 1) unregister frontend 2) remove tuner driver 3) remove demod driver (frees shared data) In ddbridge, we do (like in usb/em28xx and platform/sti/c8sectpfe, both also use some demod+tda18212 combo): dvb_unregister_frontend(); dvb_frontend_detach(); module_put(tda18212client->...owner); i2c_unregister_device(tda18212client); fe_detach() clears out the frontend references and frees/invalidates the allocated resources. tuner_ops obviously isn't there then anymore. yeah, but that's even ideally wrong. frontend design currently relies to shared data which is owned by demod driver and thus it should be last thing to be removed. Sure change like you did prevents issue, but logically it is still wrong and may not work on some other case. The two mentioned drivers will very likely yield the same (or similar) KASAN report. em28xx was even changed lately to do the teardown the way ddbridge does in 910b0797fa9e8 ([1], cc'ing Matthias here). With that commit in mind I'm a bit unsure on what is correct or not. OTOH, as dvb_frontend_detach() cleans up everything, IMHO there's no need for the tuner driver to try to clean up further. Please advise. [1] https://git.linuxtv.org/media_tree.git/commit/?id=910b0797fa9e8. em28xx does it currently just correct. 1) unregister frontend Note that this is a call to em28xx_unregister_dvb(), which in turn does dvb_unregister_frontend() and then dvb_frontend_detach() (at this stage, fe resources are gone). 2) remove I2C SEC 3) remove I2C tuner 4) remove I2C demod (frees shared frontend data) Yes, but ie. EM2874_BOARD_KWORLD_UB435Q_V3 is a combination of a "legacy" demod frontend - lgdt3305 actually - plus the tda18212 i2cclient (just like in ddb with stv0367+tda18212 or cxd2841er+tda18212), I'm sure this will yield the same report. Maybe another approach: Implement the tuner_ops.release callback, and then move the memset+NULL assignment right there (instead of just removing it), but this likely will cause issues when the i2c client is removed before detach if we don't keep track of this ie somewhere in tda18212_dev (new state var - if _remove is called, check if the tuner was released, and if not, call release (memset/set NULL), then free). Still with the two other drivers in mind though. If they're wrong aswell, I'll rather fix up ddbridge of course. Whole memset thing could be removed from tda18212, there is something likely wrong if those are needed. But it is another issue. Your main issue is somehow to get order of demod/tuner destroy correct. I don't even like idea whole shared frontend data is owned by the demod driver instance, but currently it is there and due to that this should be released lastly. General design goal is also do things like register things in order and unregister just reverse-order. regards Antti -- http://palosaari.fi/
Re: [PATCH] [media] tda18212: fix use-after-free in tda18212_remove()
On Fri, 15 Dec 2017 20:12:18 +0200 Antti Palosaari wrote: > On 12/15/2017 08:00 PM, Daniel Scheller wrote: > > Hi, > > > > On Fri, 15 Dec 2017 19:30:18 +0200 > > Antti Palosaari wrote: > > > > Thanks for your reply. > > > >> Hello > >> I think shared frontend structure, which is owned by demod driver, > >> should be there and valid on time tuner driver is removed. And thus > >> should not happen. Did you make driver unload on different order > >> eg. not just reverse order than driver load? > >> > >> IMHO these should go always > >> > >> on load: > >> 1) load demod driver (which makes shared frontend structure where > >> also some tuner driver data lives) > >> 2) load tuner driver > >> 3) register frontend > >> > >> on unload > >> 1) unregister frontend > >> 2) remove tuner driver > >> 3) remove demod driver (frees shared data) > > > > In ddbridge, we do (like in usb/em28xx and platform/sti/c8sectpfe, > > both also use some demod+tda18212 combo): > > > > dvb_unregister_frontend(); > > dvb_frontend_detach(); > > module_put(tda18212client->...owner); > > i2c_unregister_device(tda18212client); > > > > fe_detach() clears out the frontend references and frees/invalidates > > the allocated resources. tuner_ops obviously isn't there then > > anymore. > > yeah, but that's even ideally wrong. frontend design currently relies > to shared data which is owned by demod driver and thus it should be > last thing to be removed. Sure change like you did prevents issue, > but logically it is still wrong and may not work on some other case. > > > > > The two mentioned drivers will very likely yield the same (or > > similar) KASAN report. em28xx was even changed lately to do the > > teardown the way ddbridge does in 910b0797fa9e8 ([1], cc'ing > > Matthias here). > > > > With that commit in mind I'm a bit unsure on what is correct or not. > > OTOH, as dvb_frontend_detach() cleans up everything, IMHO there's no > > need for the tuner driver to try to clean up further. > > > > Please advise. > > > > [1] > > https://git.linuxtv.org/media_tree.git/commit/?id=910b0797fa9e8. > > em28xx does it currently just correct. > 1) unregister frontend Note that this is a call to em28xx_unregister_dvb(), which in turn does dvb_unregister_frontend() and then dvb_frontend_detach() (at this stage, fe resources are gone). > 2) remove I2C SEC > 3) remove I2C tuner > 4) remove I2C demod (frees shared frontend data) Yes, but ie. EM2874_BOARD_KWORLD_UB435Q_V3 is a combination of a "legacy" demod frontend - lgdt3305 actually - plus the tda18212 i2cclient (just like in ddb with stv0367+tda18212 or cxd2841er+tda18212), I'm sure this will yield the same report. Maybe another approach: Implement the tuner_ops.release callback, and then move the memset+NULL assignment right there (instead of just removing it), but this likely will cause issues when the i2c client is removed before detach if we don't keep track of this ie somewhere in tda18212_dev (new state var - if _remove is called, check if the tuner was released, and if not, call release (memset/set NULL), then free). Still with the two other drivers in mind though. If they're wrong aswell, I'll rather fix up ddbridge of course. Best regards, Daniel Scheller -- https://github.com/herrnst
Re: [PATCH] [media] tda18212: fix use-after-free in tda18212_remove()
On 12/15/2017 08:00 PM, Daniel Scheller wrote: Hi, On Fri, 15 Dec 2017 19:30:18 +0200 Antti Palosaari wrote: Thanks for your reply. Hello I think shared frontend structure, which is owned by demod driver, should be there and valid on time tuner driver is removed. And thus should not happen. Did you make driver unload on different order eg. not just reverse order than driver load? IMHO these should go always on load: 1) load demod driver (which makes shared frontend structure where also some tuner driver data lives) 2) load tuner driver 3) register frontend on unload 1) unregister frontend 2) remove tuner driver 3) remove demod driver (frees shared data) In ddbridge, we do (like in usb/em28xx and platform/sti/c8sectpfe, both also use some demod+tda18212 combo): dvb_unregister_frontend(); dvb_frontend_detach(); module_put(tda18212client->...owner); i2c_unregister_device(tda18212client); fe_detach() clears out the frontend references and frees/invalidates the allocated resources. tuner_ops obviously isn't there then anymore. yeah, but that's even ideally wrong. frontend design currently relies to shared data which is owned by demod driver and thus it should be last thing to be removed. Sure change like you did prevents issue, but logically it is still wrong and may not work on some other case. The two mentioned drivers will very likely yield the same (or similar) KASAN report. em28xx was even changed lately to do the teardown the way ddbridge does in 910b0797fa9e8 ([1], cc'ing Matthias here). With that commit in mind I'm a bit unsure on what is correct or not. OTOH, as dvb_frontend_detach() cleans up everything, IMHO there's no need for the tuner driver to try to clean up further. Please advise. [1] https://git.linuxtv.org/media_tree.git/commit/?id=910b0797fa9e8. em28xx does it currently just correct. 1) unregister frontend 2) remove I2C SEC 3) remove I2C tuner 4) remove I2C demod (frees shared frontend data) regards Antti -- http://palosaari.fi/
Re: [PATCH] [media] tda18212: fix use-after-free in tda18212_remove()
Hi, On Fri, 15 Dec 2017 19:30:18 +0200 Antti Palosaari wrote: Thanks for your reply. > Hello > I think shared frontend structure, which is owned by demod driver, > should be there and valid on time tuner driver is removed. And thus > should not happen. Did you make driver unload on different order eg. > not just reverse order than driver load? > > IMHO these should go always > > on load: > 1) load demod driver (which makes shared frontend structure where > also some tuner driver data lives) > 2) load tuner driver > 3) register frontend > > on unload > 1) unregister frontend > 2) remove tuner driver > 3) remove demod driver (frees shared data) In ddbridge, we do (like in usb/em28xx and platform/sti/c8sectpfe, both also use some demod+tda18212 combo): dvb_unregister_frontend(); dvb_frontend_detach(); module_put(tda18212client->...owner); i2c_unregister_device(tda18212client); fe_detach() clears out the frontend references and frees/invalidates the allocated resources. tuner_ops obviously isn't there then anymore. The two mentioned drivers will very likely yield the same (or similar) KASAN report. em28xx was even changed lately to do the teardown the way ddbridge does in 910b0797fa9e8 ([1], cc'ing Matthias here). With that commit in mind I'm a bit unsure on what is correct or not. OTOH, as dvb_frontend_detach() cleans up everything, IMHO there's no need for the tuner driver to try to clean up further. Please advise. [1] https://git.linuxtv.org/media_tree.git/commit/?id=910b0797fa9e8. Best regards, Daniel > On 12/15/2017 06:43 PM, Daniel Scheller wrote: > > From: Daniel Scheller > > > > When the driver gets unloaded via it's tda18212_remove() function, > > all frontend structures may already have been freed as > > controlling/bridge drivers already used dvb_frontend_detach() in > > their teardown process. Since __dvb_frontend_free now releases and > > clears all structures, the memset and the NULL assignment in > > tda18212_remove() leads to this KASAN report (invoked via ddbridge, > > which does dvb_frontend_detach() first, followed by > > i2c_unregister_device()): > > > >[ 154.028353] > > == > > [ 154.028396] BUG: KASAN: use-after-free in > > tda18212_remove+0x5c/0xb0 [tda18212] [ 154.028415] Write of size > > 288 at addr 880108b554d8 by task rmmod/285 > > > >[ 154.028442] CPU: 0 PID: 285 Comm: rmmod Tainted: G > > C 4.15.0-rc1-13682-g1363f325bc44 #1 [ 154.028444] Hardware > > name: Gigabyte Technology Co., Ltd. P35-DS3/P35-DS3, BIOS F3 > > 06/11/2007 [ 154.028445] Call Trace: [ 154.028452] > > dump_stack+0x46/0x61 [ 154.028458] > > print_address_description+0x79/0x270 [ 154.028462] ? > > tda18212_remove+0x5c/0xb0 [tda18212] [ 154.028465] > > kasan_report+0x229/0x340 [ 154.028468] memset+0x1f/0x40 > >[ 154.028472] tda18212_remove+0x5c/0xb0 [tda18212] > >[ 154.028476] i2c_device_remove+0x97/0xe0 > >[ 154.028481] device_release_driver_internal+0x267/0x510 > >[ 154.028484] bus_remove_device+0x296/0x470 > >[ 154.028486] device_del+0x35c/0x890 > >[ 154.028489] ? __device_links_no_driver+0x1c0/0x1c0 > >[ 154.028493] ? cxd2841er_get_algo+0x10/0x10 [cxd2841er] > >[ 154.028497] ? cxd2841er_get_algo+0x10/0x10 [cxd2841er] > >[ 154.028500] ? __module_text_address+0xe/0x140 > >[ 154.028503] device_unregister+0x9/0x20 > >[ 154.028509] dvb_input_detach.isra.24+0x286/0x480 [ddbridge] > >[ 154.028514] ddb_ports_detach+0x176/0x630 [ddbridge] > >[ 154.028519] ddb_remove+0x3c/0xb0 [ddbridge] > >[ 154.028523] pci_device_remove+0x93/0x1d0 > >[ 154.028526] device_release_driver_internal+0x267/0x510 > >[ 154.028529] driver_detach+0xb9/0x1b0 > >[ 154.028532] bus_remove_driver+0xd0/0x1f0 > >[ 154.028536] pci_unregister_driver+0x25/0x210 > >[ 154.028541] module_exit_ddbridge+0xc/0x45 [ddbridge] > >[ 154.028544] SyS_delete_module+0x314/0x440 > >[ 154.028546] ? free_module+0x5b0/0x5b0 > >[ 154.028550] ? exit_to_usermode_loop+0xa9/0xc0 > >[ 154.028552] ? free_module+0x5b0/0x5b0 > >[ 154.028554] do_syscall_64+0x179/0x4c0 > >[ 154.028557] ? do_page_fault+0x1b/0x60 > >[ 154.028560] entry_SYSCALL64_slow_path+0x25/0x25 > >[ 154.028563] RIP: 0033:0x7f6c40930de7 > >[ 154.028565] RSP: 002b:7ffc060d6ab8 EFLAGS: 0206 > > ORIG_RAX: 00b0 [ 154.028569] RAX: ffda > > RBX: RCX: 7f6c40930de7 [ 154.028571] RDX: > > 000a RSI: 0800 RDI: 02053268 > > [ 154.028573] RBP: 02053200 R08: R09: > > 1999 [ 154.028574] R10: 0891 R11: > > 0206 R12: 7ffc060d74ef [ 154.028576] R13: > > R14: 02053200 R15: 02052010 > > > >[ 154.028588] Allocated by task 164: > >[ 154.028603] cxd2841er_attach+0xc3/
Re: [PATCH] [media] tda18212: fix use-after-free in tda18212_remove()
Hello I think shared frontend structure, which is owned by demod driver, should be there and valid on time tuner driver is removed. And thus should not happen. Did you make driver unload on different order eg. not just reverse order than driver load? IMHO these should go always on load: 1) load demod driver (which makes shared frontend structure where also some tuner driver data lives) 2) load tuner driver 3) register frontend on unload 1) unregister frontend 2) remove tuner driver 3) remove demod driver (frees shared data) regards Antti On 12/15/2017 06:43 PM, Daniel Scheller wrote: From: Daniel Scheller When the driver gets unloaded via it's tda18212_remove() function, all frontend structures may already have been freed as controlling/bridge drivers already used dvb_frontend_detach() in their teardown process. Since __dvb_frontend_free now releases and clears all structures, the memset and the NULL assignment in tda18212_remove() leads to this KASAN report (invoked via ddbridge, which does dvb_frontend_detach() first, followed by i2c_unregister_device()): [ 154.028353] == [ 154.028396] BUG: KASAN: use-after-free in tda18212_remove+0x5c/0xb0 [tda18212] [ 154.028415] Write of size 288 at addr 880108b554d8 by task rmmod/285 [ 154.028442] CPU: 0 PID: 285 Comm: rmmod Tainted: G C 4.15.0-rc1-13682-g1363f325bc44 #1 [ 154.028444] Hardware name: Gigabyte Technology Co., Ltd. P35-DS3/P35-DS3, BIOS F3 06/11/2007 [ 154.028445] Call Trace: [ 154.028452] dump_stack+0x46/0x61 [ 154.028458] print_address_description+0x79/0x270 [ 154.028462] ? tda18212_remove+0x5c/0xb0 [tda18212] [ 154.028465] kasan_report+0x229/0x340 [ 154.028468] memset+0x1f/0x40 [ 154.028472] tda18212_remove+0x5c/0xb0 [tda18212] [ 154.028476] i2c_device_remove+0x97/0xe0 [ 154.028481] device_release_driver_internal+0x267/0x510 [ 154.028484] bus_remove_device+0x296/0x470 [ 154.028486] device_del+0x35c/0x890 [ 154.028489] ? __device_links_no_driver+0x1c0/0x1c0 [ 154.028493] ? cxd2841er_get_algo+0x10/0x10 [cxd2841er] [ 154.028497] ? cxd2841er_get_algo+0x10/0x10 [cxd2841er] [ 154.028500] ? __module_text_address+0xe/0x140 [ 154.028503] device_unregister+0x9/0x20 [ 154.028509] dvb_input_detach.isra.24+0x286/0x480 [ddbridge] [ 154.028514] ddb_ports_detach+0x176/0x630 [ddbridge] [ 154.028519] ddb_remove+0x3c/0xb0 [ddbridge] [ 154.028523] pci_device_remove+0x93/0x1d0 [ 154.028526] device_release_driver_internal+0x267/0x510 [ 154.028529] driver_detach+0xb9/0x1b0 [ 154.028532] bus_remove_driver+0xd0/0x1f0 [ 154.028536] pci_unregister_driver+0x25/0x210 [ 154.028541] module_exit_ddbridge+0xc/0x45 [ddbridge] [ 154.028544] SyS_delete_module+0x314/0x440 [ 154.028546] ? free_module+0x5b0/0x5b0 [ 154.028550] ? exit_to_usermode_loop+0xa9/0xc0 [ 154.028552] ? free_module+0x5b0/0x5b0 [ 154.028554] do_syscall_64+0x179/0x4c0 [ 154.028557] ? do_page_fault+0x1b/0x60 [ 154.028560] entry_SYSCALL64_slow_path+0x25/0x25 [ 154.028563] RIP: 0033:0x7f6c40930de7 [ 154.028565] RSP: 002b:7ffc060d6ab8 EFLAGS: 0206 ORIG_RAX: 00b0 [ 154.028569] RAX: ffda RBX: RCX: 7f6c40930de7 [ 154.028571] RDX: 000a RSI: 0800 RDI: 02053268 [ 154.028573] RBP: 02053200 R08: R09: 1999 [ 154.028574] R10: 0891 R11: 0206 R12: 7ffc060d74ef [ 154.028576] R13: R14: 02053200 R15: 02052010 [ 154.028588] Allocated by task 164: [ 154.028603] cxd2841er_attach+0xc3/0x7f0 [cxd2841er] [ 154.028608] demod_attach_cxd28xx+0x14c/0x3f0 [ddbridge] [ 154.028612] dvb_input_attach+0x671/0x1e20 [ddbridge] [ 154.028616] ddb_ports_attach+0x453/0xd00 [ddbridge] [ 154.028620] ddb_init+0x4b3/0xa30 [ddbridge] [ 154.028624] ddb_probe+0xa51/0xfe0 [ddbridge] [ 154.028627] pci_device_probe+0x279/0x480 [ 154.028630] driver_probe_device+0x46f/0x7a0 [ 154.028632] __driver_attach+0x133/0x170 [ 154.028634] bus_for_each_dev+0x10a/0x190 [ 154.028637] bus_add_driver+0x2a3/0x5a0 [ 154.028639] driver_register+0x182/0x3a0 [ 154.028641] 0xa016808f [ 154.028643] do_one_initcall+0x77/0x1d0 [ 154.028646] do_init_module+0x1c2/0x548 [ 154.028648] load_module+0x5e61/0x8df0 [ 154.028650] SyS_finit_module+0x142/0x150 [ 154.028652] do_syscall_64+0x179/0x4c0 [ 154.028654] return_from_SYSCALL_64+0x0/0x65 [ 154.028664] Freed by task 285: [ 154.028676] kfree+0x6c/0xa0 [ 154.028682] __dvb_frontend_free+0x81/0xb0 [dvb_core] [ 154.028687] dvb_input_detach.isra.24+0x17c/0x480 [ddbridge] [ 154.028691] ddb_ports_detach+0x176/0x630 [ddbridge] [ 154
[GIT PULL FOR v4.16] RC fixes
Hi Mauro, Some regressions and improvements since the lirc scancodes changes. Thanks, Sean The following changes since commit 0ca4e3130402caea8731a7b54afde56a6edb17c9: media: pxa_camera: rename the soc_camera_ prefix to pxa_camera_ (2017-12-14 12:40:01 -0500) are available in the Git repository at: git://linuxtv.org/syoung/media_tree.git for-v4.16b for you to fetch changes up to 4cb896c68d5296e2af834033a34575f61fc5228f: media: ir-spi: add SPDX identifier (2017-12-15 16:42:11 +) Andi Shyti (1): media: ir-spi: add SPDX identifier Sean Young (8): media: imon: auto-config ffdc 26 device media: imon: remove unused function tv2int media: rc: bang in ir_do_keyup media: lirc: when transmitting scancodes, block until transmit is done media: rc: iguanair: simplify tx loop media: lirc: do not pass ERR_PTR to kfree media: lirc: no need to recalculate duration media: lirc: release lock before sleep Documentation/media/uapi/rc/lirc-write.rst | 4 +- drivers/media/rc/iguanair.c| 19 - drivers/media/rc/imon.c| 28 +++-- drivers/media/rc/ir-spi.c | 15 +++ drivers/media/rc/lirc_dev.c| 67 ++ drivers/media/rc/rc-main.c | 2 +- 6 files changed, 53 insertions(+), 82 deletions(-)
[GIT PULL] Samsung SoC related updates
Hi Mauro, The following changes since commit 0ca4e3130402caea8731a7b54afde56a6edb17c9: media: pxa_camera: rename the soc_camera_ prefix to pxa_camera_ (2017-12-14 12:40:01 -0500) are available in the git repository at: git://linuxtv.org/snawrocki/samsung.git for-v4.16/media/next for you to fetch changes up to 8d10c3a3fa56badd9d8691b59a88e7f00fdeaa7b: s5p-jpeg: Fix off-by-one problem (2017-12-15 17:33:50 +0100) Arnd Bergmann (1): exynos4-is: properly initialize frame format Flavio Ceolin (1): s5p-jpeg: Fix off-by-one problem Marek Szyprowski (3): exynos-gsc: Drop obsolete capabilities exynos4-is: Drop obsolete capabilities exynos4-is: Remove dependency on obsolete SoC support Shuah Khan (2): s5p-mfc: Remove firmware buf null check in s5p_mfc_load_firmware() s5p-mfc: Fix lock contention - request_firmware() once Simon Shields (1): exynos4-is: Check pipe is valid before calling subdev Sylwester Nawrocki (1): s5p-mfc: Fix encoder menu controls initialization drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 +--- drivers/media/platform/exynos4-is/Kconfig | 2 +- drivers/media/platform/exynos4-is/fimc-core.c | 2 +- drivers/media/platform/exynos4-is/fimc-isp.c| 14 +++--- drivers/media/platform/exynos4-is/fimc-lite.c | 2 +- drivers/media/platform/exynos4-is/fimc-m2m.c| 10 +- drivers/media/platform/s5p-jpeg/jpeg-core.c | 2 +- drivers/media/platform/s5p-mfc/s5p_mfc.c| 6 ++ drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 3 +++ drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 10 +- drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 2 +- include/media/drv-intf/exynos-fimc.h| 3 ++- 12 files changed, 30 insertions(+), 30 deletions(-) -- Regards, Sylwester
[PATCH] [media] tda18212: fix use-after-free in tda18212_remove()
From: Daniel Scheller When the driver gets unloaded via it's tda18212_remove() function, all frontend structures may already have been freed as controlling/bridge drivers already used dvb_frontend_detach() in their teardown process. Since __dvb_frontend_free now releases and clears all structures, the memset and the NULL assignment in tda18212_remove() leads to this KASAN report (invoked via ddbridge, which does dvb_frontend_detach() first, followed by i2c_unregister_device()): [ 154.028353] == [ 154.028396] BUG: KASAN: use-after-free in tda18212_remove+0x5c/0xb0 [tda18212] [ 154.028415] Write of size 288 at addr 880108b554d8 by task rmmod/285 [ 154.028442] CPU: 0 PID: 285 Comm: rmmod Tainted: G C 4.15.0-rc1-13682-g1363f325bc44 #1 [ 154.028444] Hardware name: Gigabyte Technology Co., Ltd. P35-DS3/P35-DS3, BIOS F3 06/11/2007 [ 154.028445] Call Trace: [ 154.028452] dump_stack+0x46/0x61 [ 154.028458] print_address_description+0x79/0x270 [ 154.028462] ? tda18212_remove+0x5c/0xb0 [tda18212] [ 154.028465] kasan_report+0x229/0x340 [ 154.028468] memset+0x1f/0x40 [ 154.028472] tda18212_remove+0x5c/0xb0 [tda18212] [ 154.028476] i2c_device_remove+0x97/0xe0 [ 154.028481] device_release_driver_internal+0x267/0x510 [ 154.028484] bus_remove_device+0x296/0x470 [ 154.028486] device_del+0x35c/0x890 [ 154.028489] ? __device_links_no_driver+0x1c0/0x1c0 [ 154.028493] ? cxd2841er_get_algo+0x10/0x10 [cxd2841er] [ 154.028497] ? cxd2841er_get_algo+0x10/0x10 [cxd2841er] [ 154.028500] ? __module_text_address+0xe/0x140 [ 154.028503] device_unregister+0x9/0x20 [ 154.028509] dvb_input_detach.isra.24+0x286/0x480 [ddbridge] [ 154.028514] ddb_ports_detach+0x176/0x630 [ddbridge] [ 154.028519] ddb_remove+0x3c/0xb0 [ddbridge] [ 154.028523] pci_device_remove+0x93/0x1d0 [ 154.028526] device_release_driver_internal+0x267/0x510 [ 154.028529] driver_detach+0xb9/0x1b0 [ 154.028532] bus_remove_driver+0xd0/0x1f0 [ 154.028536] pci_unregister_driver+0x25/0x210 [ 154.028541] module_exit_ddbridge+0xc/0x45 [ddbridge] [ 154.028544] SyS_delete_module+0x314/0x440 [ 154.028546] ? free_module+0x5b0/0x5b0 [ 154.028550] ? exit_to_usermode_loop+0xa9/0xc0 [ 154.028552] ? free_module+0x5b0/0x5b0 [ 154.028554] do_syscall_64+0x179/0x4c0 [ 154.028557] ? do_page_fault+0x1b/0x60 [ 154.028560] entry_SYSCALL64_slow_path+0x25/0x25 [ 154.028563] RIP: 0033:0x7f6c40930de7 [ 154.028565] RSP: 002b:7ffc060d6ab8 EFLAGS: 0206 ORIG_RAX: 00b0 [ 154.028569] RAX: ffda RBX: RCX: 7f6c40930de7 [ 154.028571] RDX: 000a RSI: 0800 RDI: 02053268 [ 154.028573] RBP: 02053200 R08: R09: 1999 [ 154.028574] R10: 0891 R11: 0206 R12: 7ffc060d74ef [ 154.028576] R13: R14: 02053200 R15: 02052010 [ 154.028588] Allocated by task 164: [ 154.028603] cxd2841er_attach+0xc3/0x7f0 [cxd2841er] [ 154.028608] demod_attach_cxd28xx+0x14c/0x3f0 [ddbridge] [ 154.028612] dvb_input_attach+0x671/0x1e20 [ddbridge] [ 154.028616] ddb_ports_attach+0x453/0xd00 [ddbridge] [ 154.028620] ddb_init+0x4b3/0xa30 [ddbridge] [ 154.028624] ddb_probe+0xa51/0xfe0 [ddbridge] [ 154.028627] pci_device_probe+0x279/0x480 [ 154.028630] driver_probe_device+0x46f/0x7a0 [ 154.028632] __driver_attach+0x133/0x170 [ 154.028634] bus_for_each_dev+0x10a/0x190 [ 154.028637] bus_add_driver+0x2a3/0x5a0 [ 154.028639] driver_register+0x182/0x3a0 [ 154.028641] 0xa016808f [ 154.028643] do_one_initcall+0x77/0x1d0 [ 154.028646] do_init_module+0x1c2/0x548 [ 154.028648] load_module+0x5e61/0x8df0 [ 154.028650] SyS_finit_module+0x142/0x150 [ 154.028652] do_syscall_64+0x179/0x4c0 [ 154.028654] return_from_SYSCALL_64+0x0/0x65 [ 154.028664] Freed by task 285: [ 154.028676] kfree+0x6c/0xa0 [ 154.028682] __dvb_frontend_free+0x81/0xb0 [dvb_core] [ 154.028687] dvb_input_detach.isra.24+0x17c/0x480 [ddbridge] [ 154.028691] ddb_ports_detach+0x176/0x630 [ddbridge] [ 154.028695] ddb_remove+0x3c/0xb0 [ddbridge] [ 154.028697] pci_device_remove+0x93/0x1d0 [ 154.028700] device_release_driver_internal+0x267/0x510 [ 154.028702] driver_detach+0xb9/0x1b0 [ 154.028705] bus_remove_driver+0xd0/0x1f0 [ 154.028707] pci_unregister_driver+0x25/0x210 [ 154.028711] module_exit_ddbridge+0xc/0x45 [ddbridge] [ 154.028714] SyS_delete_module+0x314/0x440 [ 154.028716] do_syscall_64+0x179/0x4c0 [ 154.028718] return_from_SYSCALL_64+0x0/0x65 [ 154.028729] The buggy address belongs to the object at 880108b55340 which belongs to the cache kmalloc-2048 of size 2048 [ 154.028755] The buggy address
Re: [PATCH 5/5] v4l2: async: Add debug output to v4l2-async module
Hi Jacopo, On Wed, Dec 13, 2017 at 07:26:20PM +0100, Jacopo Mondi wrote: > The v4l2-async module operations are quite complex to follow, due to the > asynchronous nature of subdevices and notifiers registration and > matching procedures. In order to help with debugging of failed or > erroneous matching between a subdevice and the notifier collected > async_subdevice it gets matched against, introduce a few dev_dbg() calls > in v4l2_async core operations. > > Protect the debug operations with a Kconfig defined symbol, to make sure > when debugging is disabled, no additional code or data is added to the > module. > > Notifiers are identified by the name of the subdevice or v4l2_dev they are > registered by, while subdevice matching which now happens on endpoints, > need a longer description built walking the fwnode graph backwards > collecting parent nodes names (otherwise we would have had printouts > like: "Matching "endpoint" with "endpoint"" which are not that useful). > > Signed-off-by: Jacopo Mondi > > --- > For fwnodes backed by OF, I may have used the "%pOF" format modifier to > get the full node name instead of parsing the fwnode graph by myself with > "v4l2_async_fwnode_full_name()". Unfortunately I'm not aware of anything > like "%pOF" for ACPI backed fwnodes. Also, walking the fwnode graph by > myself allows me to reduce the depth, to reduce the debug messages output > length which is anyway long enough to result disturbing on a 80columns > terminal window. ACPI doesn't have such at the moment. I think printing the full path would still be better. There isn't that much more to print after all. > --- > > drivers/media/v4l2-core/Kconfig | 8 > drivers/media/v4l2-core/v4l2-async.c | 81 > > 2 files changed, 89 insertions(+) > > diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig > index a35c336..8331736 100644 > --- a/drivers/media/v4l2-core/Kconfig > +++ b/drivers/media/v4l2-core/Kconfig > @@ -17,6 +17,14 @@ config VIDEO_ADV_DEBUG > V4L devices. > In doubt, say N. > > +config VIDEO_V4L2_ASYNC_DEBUG > + bool "Enable debug functionalities for V4L2 async module" > + depends on VIDEO_V4L2 I'm not sure I'd add a Kconfig option. This is adding a fairly simple function only to the kernel. > + default n > + ---help--- > + Say Y here to enable debug output in V4L2 async module. > + In doubt, say N. > + > config VIDEO_FIXED_MINOR_RANGES > bool "Enable old-style fixed minor ranges on drivers/video devices" > default n > diff --git a/drivers/media/v4l2-core/v4l2-async.c > b/drivers/media/v4l2-core/v4l2-async.c > index c13a781..307e1a5 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -8,6 +8,10 @@ > * published by the Free Software Foundation. > */ > > +#if defined(CONFIG_VIDEO_V4L2_ASYNC_DEBUG) > +#define DEBUG Do you need this? > +#endif > + > #include > #include > #include > @@ -25,6 +29,52 @@ > #include > #include > > +#if defined(CONFIG_VIDEO_V4L2_ASYNC_DEBUG) > +#define V4L2_ASYNC_FWNODE_NAME_LEN 512 > + > +static void __v4l2_async_fwnode_full_name(char *name, > + unsigned int len, > + unsigned int max_depth, > + struct fwnode_handle *fwnode) > +{ > + unsigned int buf_len = len < V4L2_ASYNC_FWNODE_NAME_LEN ? > +len : V4L2_ASYNC_FWNODE_NAME_LEN; > + char __tmp[V4L2_ASYNC_FWNODE_NAME_LEN]; That's a bit too much to allocate from the stack I think. > + struct fwnode_handle *parent; > + > + memset(name, 0, buf_len); > + buf_len -= snprintf(__tmp, buf_len, "%s", fwnode_get_name(fwnode)); > + > + parent = fwnode; > + while ((parent = fwnode_get_parent(parent)) && buf_len && > + --max_depth) { > + buf_len -= snprintf(name, buf_len, "%s/%s", > + fwnode_get_name(parent), __tmp); > + strcpy(__tmp, name); > + } > +} > + > +static void v4l2_async_fwnode_full_name(char *name, > + unsigned int len, > + struct fwnode_handle *fwnode) > +{ > + /* > + * Usually 4 as nesting level is sufficient to identify an > + * endpoint firmware node uniquely. > + */ > + __v4l2_async_fwnode_full_name(name, len, 4, fwnode); > +} > + > +#else /* CONFIG_VIDEO_V4L2_ASYNC_DEBUG */ > +#define V4L2_ASYNC_FWNODE_NAME_LEN 0 > + > +static void v4l2_async_fwnode_full_name(char *name, > + unsigned int len, > + struct fwnode_handle *fwnode) > +{ > +} > +#endif /* CONFIG_VIDEO_V4L2_ASYNC_DEBUG */ > + > static struct device *v4l2_async_notifier_dev( > struct v4l2_async_notifier *notifier) >
Re: [PATCH v2] media: s5p-mfc: Add support for V4L2_MEMORY_DMABUF type
Hi Marek, On 14/12/17 15:11, Marek Szyprowski wrote: > Hi Hans, > > I would like to get your opinion on this patch. Do you think it makes sense > to: > > 1. add limited support for USERPTR and DMA-buf import? (limited means driver > will accept setting buffer pointer/fd only once after reqbufs for each buffer > index) I don't like this. It's unexpected almost-but-not-quite behavior that will make life very difficult for userspace. > > 2. add a V4L2 device flag to let userspace to discover if device support > queue buffer reconfiguration on-fly or not? This seems to me a better approach. It should be possible to implement most/all of this in vb2, but we need to find a way to signal this to the user. Is this an MFC limitation for the decoder, encoder or both? And is it a limitation of the capture or output side or both? Regards, Hans > > Here is the discussion: https://patchwork.linuxtv.org/patch/45305/ > > Best regards > Marek Szyprowski, PhD > Samsung R&D Institute Poland > > > On 2017-11-06 20:21, Nicolas Dufresne wrote: >> Le lundi 06 novembre 2017 à 10:28 +0100, Marek Szyprowski a écrit : >>> On 2017-11-03 14:45, Nicolas Dufresne wrote: Le vendredi 03 novembre 2017 à 09:11 +0100, Marek Szyprowski a écrit : > MFC driver supports only MMAP operation mode mainly due to the hardware > restrictions of the addresses of the DMA buffers (MFC v5 hardware can > access buffers only in 128MiB memory region starting from the base address > of its firmware). When IOMMU is available, this requirement is easily > fulfilled even for the buffers located anywhere in the memory - typically > by mapping them in the DMA address space as close as possible to the > firmware. Later hardware revisions don't have this limitations at all. > > The second limitation of the MFC hardware related to the memory buffers > is constant buffer address. Once the hardware has been initialized for > operation on given buffer set, the addresses of the buffers cannot be > changed. > > With the above assumptions, a limited support for USERPTR and DMABUF > operation modes can be added. The main requirement is to have all buffers > known when starting hardware. This has been achieved by postponing > hardware initialization once all the DMABUF or USERPTR buffers have been > queued for the first time. Once then, buffers cannot be modified to point > to other memory area. I am concerned about enabling this support with existing userspace. Userspace application will be left with some driver with this limitation and other drivers without. I think it is harmful to enable that feature without providing userspace the ability to know. This is specially conflicting with let's say UVC driver providing buffers, since UVC driver implementing CREATE_BUFS ioctl. So even if userspace start making an effort to maintain the same DMABuf for the same buffer index, if a new buffer is created, we won't be able to use it. >>> Sorry, but I don't get this as an 'issue'. The typical use scenario is to >>> configure buffer queue and start streaming. Basically ReqBufs, stream on and >>> a sequence of bufq/bufdq. This is handled without any problem by MFC driver >>> with this patch. After releasing a queue with reqbufs(0), one can use >>> different set of buffers. >> In real life, you often have capture code decorelated from the encoder >> code. At least, it's the case in GStreamer. The encoder have no >> information about how many buffers were pre-allocated by let's say the >> capture driver. As a side effect, we cannot make sure the importation >> queue is of the same size (amount of buffer). Specially if you have UVC >> driver that allow allocating more buffers at run-time. This is used in >> real-life to compensate additional latency that get's added when a >> pipeline topology is changed (at run-time). Only workaround I had in >> mind, would be to always prepare the queue with the maximum (32), and >> use this as a cache size, but for now, this is not how the deployed >> userspace works unfortunately. >> >> Your reqbuf(0) technique cause a break in the stream (probably a new >> keyframe), as you are forced to STREAMOFF. This is often unwanted, and >> it may create a time discontinuity in case the allocation took time. >> >>> What is the point of changing the buffers during the streaming? IMHO it was >>> one of the biggest pathology of the V4L2 USERPTR API that the buffers >>> were in >>> fact 'created' on the first queue operation. By creating I mean creating all >>> the kernel all needed structures and mappings between the real memory (user >>> ptr value) and the buffer index. The result of this was userspace, which >>> don't >>> use buffer indices and always queues buffers with index = 0, what means that >>> kernel has to reacquire direct access to each buffer every single frame. >>> That >>> is highly inefficien
[PATCH v8] intel-ipu3: cio2: fix two warnings in the code
Fix two warnings reported by static analysis tool: ipu3-cio2.c:1899:16: warning: Variable length array is used. In function 'cio2_pci_probe': ipu3-cio2.c:1726:14: warning: variable 'phys' set but not used [-Wunused-but-set-variable] Signed-off-by: Yong Zhi --- Hi, Mauro, thanks for catching the warnings. Hi, Sakari, can you squash the patch to your tree? Thanks!! drivers/media/pci/intel/ipu3/ipu3-cio2.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c index 4295bdb8b192..941caa987dab 100644 --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c @@ -1723,7 +1723,6 @@ static int cio2_pci_probe(struct pci_dev *pci_dev, const struct pci_device_id *id) { struct cio2_device *cio2; - phys_addr_t phys; void __iomem *const *iomap; int r; @@ -1741,8 +1740,6 @@ static int cio2_pci_probe(struct pci_dev *pci_dev, dev_info(&pci_dev->dev, "device 0x%x (rev: 0x%x)\n", pci_dev->device, pci_dev->revision); - phys = pci_resource_start(pci_dev, CIO2_PCI_BAR); - r = pcim_iomap_regions(pci_dev, 1 << CIO2_PCI_BAR, pci_name(pci_dev)); if (r) { dev_err(&pci_dev->dev, "failed to remap I/O memory (%d)\n", r); @@ -1896,7 +1893,6 @@ static void arrange(void *ptr, size_t elem_size, size_t elems, size_t start) { 0, start - 1 }, { start, elems - 1 }, }; - u8 tmp[elem_size]; #define arr_size(a) ((a)->end - (a)->begin + 1) @@ -1915,12 +1911,12 @@ static void arrange(void *ptr, size_t elem_size, size_t elems, size_t start) /* Swap the entries in two parts of the array. */ for (i = 0; i < size0; i++) { - memcpy(tmp, ptr + elem_size * (arr[1].begin + i), - elem_size); - memcpy(ptr + elem_size * (arr[1].begin + i), - ptr + elem_size * (arr[0].begin + i), elem_size); - memcpy(ptr + elem_size * (arr[0].begin + i), tmp, - elem_size); + u8 *d = ptr + elem_size * (arr[1].begin + i); + u8 *s = ptr + elem_size * (arr[0].begin + i); + size_t j; + + for (j = 0; j < elem_size; j++) + swap(d[j], s[j]); } if (arr_size(&arr[0]) > arr_size(&arr[1])) { -- 2.7.4
Re: [PATCH 4/5] v4l2: async: Postpone subdev_notifier registration
Hi Jacopo, On Wed, Dec 13, 2017 at 07:26:19PM +0100, Jacopo Mondi wrote: > Currently, subdevice notifiers are tested against all available > subdevices as soon as they get registered. It often happens anyway > that the subdevice they are connected to is not yet initialized, as > it usually gets registered later in drivers' code. This makes debug > of v4l2_async particularly painful, as identifying a notifier with > an unitialized subdevice is tricky as they don't have a valid > 'struct device *' or 'struct fwnode_handle *' to be identified with. > > In order to make sure that the notifier's subdevices is initialized > when the notifier is tesed against available subdevices post-pone the > actual notifier registration at subdevice registration time. > > It is worth noting that post-poning registration of a subdevice notifier > does not impact on the completion of the notifiers chain, as even if a > subdev notifier completes as soon as it gets registered, the complete() > call chain cannot be upscaled as long as the subdevice the notifiers > belongs to is not registered. Let me rephrase to make sure I understand the problem correctly --- A sub-device notifier is registered but the notifier's sub-device is not registered yet, and finding a match for this notifier leads, to, well problems. Is that the reason for this patch? I think there could be simpler solutions to address this. I wonder if we could simply check for sub-device notifier's v4l2_dev field, and fail in matching if it's not set. v4l2_device_register_subdev() would still need to proceed with calling v4l2_async_notifier_try_all_subdevs() and v4l2_async_notifier_try_complete() if that was the case. What do you think? > > Also, it is now safe to access a notifier 'struct device *' as we're now > sure it is properly initialized when the notifier is actually > registered. > > Signed-off-by: Jacopo Mondi > --- > drivers/media/v4l2-core/v4l2-async.c | 65 > +++- > include/media/v4l2-async.h | 2 ++ > 2 files changed, 43 insertions(+), 24 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-async.c > b/drivers/media/v4l2-core/v4l2-async.c > index 0a1bf1d..c13a781 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -25,6 +25,13 @@ > #include > #include > > +static struct device *v4l2_async_notifier_dev( > + struct v4l2_async_notifier *notifier) > +{ > + return notifier->v4l2_dev ? notifier->v4l2_dev->dev : > + notifier->sd->dev; > +} > + > static int v4l2_async_notifier_call_bound(struct v4l2_async_notifier *n, > struct v4l2_subdev *subdev, > struct v4l2_async_subdev *asd) > @@ -124,19 +131,6 @@ static struct v4l2_async_subdev *v4l2_async_find_match( > return NULL; > } > > -/* Find the sub-device notifier registered by a sub-device driver. */ > -static struct v4l2_async_notifier *v4l2_async_find_subdev_notifier( > - struct v4l2_subdev *sd) > -{ > - struct v4l2_async_notifier *n; > - > - list_for_each_entry(n, ¬ifier_list, list) > - if (n->sd == sd) > - return n; > - > - return NULL; > -} > - > /* Get v4l2_device related to the notifier if one can be found. */ > static struct v4l2_device *v4l2_async_notifier_find_v4l2_dev( > struct v4l2_async_notifier *notifier) > @@ -160,7 +154,7 @@ static bool v4l2_async_notifier_can_complete( > > list_for_each_entry(sd, ¬ifier->done, async_list) { > struct v4l2_async_notifier *subdev_notifier = > - v4l2_async_find_subdev_notifier(sd); > + sd->subdev_notifier; > > if (subdev_notifier && > !v4l2_async_notifier_can_complete(subdev_notifier)) > @@ -228,7 +222,7 @@ static int v4l2_async_match_notify(struct > v4l2_async_notifier *notifier, > /* >* See if the sub-device has a notifier. If not, return here. >*/ > - subdev_notifier = v4l2_async_find_subdev_notifier(sd); > + subdev_notifier = sd->subdev_notifier; > if (!subdev_notifier || subdev_notifier->parent) > return 0; > > @@ -294,7 +288,7 @@ static void v4l2_async_notifier_unbind_all_subdevs( > > list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) { > struct v4l2_async_notifier *subdev_notifier = > - v4l2_async_find_subdev_notifier(sd); > + sd->subdev_notifier; > > if (subdev_notifier) > v4l2_async_notifier_unbind_all_subdevs(subdev_notifier); > @@ -371,8 +365,7 @@ static bool v4l2_async_notifier_fwnode_has_async_subdev( > > static int __v4l2_async_notifier_register(struct v4l2_async_notifier > *notifier) > { > - struct devic
RE: [RESEND GIT PULL for 4.16] Intel IPU3 CIO2 CSI-2 receiver driver
Hi, Mauro and Sakari, Sorry for the late response, the fix of the 2 warnings is attached. Best regards, Yong From: Zhi, Yong Sent: Friday, December 08, 2017 3:32 PM To: Mauro Carvalho Chehab; Sakari Ailus Cc: linux-media@vger.kernel.org; Mani, Rajmohan Subject: RE: [RESEND GIT PULL for 4.16] Intel IPU3 CIO2 CSI-2 receiver driver Hi, Mauro, > -Original Message- > From: Mauro Carvalho Chehab [mailto:mche...@s-opensource.com] > Sent: Friday, December 8, 2017 7:00 AM > To: Sakari Ailus > Cc: linux-media@vger.kernel.org; Mani, Rajmohan > ; Zhi, Yong > Subject: Re: [RESEND GIT PULL for 4.16] Intel IPU3 CIO2 CSI-2 receiver driver > > Em Fri, 1 Dec 2017 16:31:36 +0200 > Sakari Ailus escreveu: > > > Hi Mauro, > > > > Here's the Intel IPU3 CIO2 CSI-2 receiver driver, with the > > accompanying format definitions. > > This patch generates two warnings: > > drivers/media/pci/intel/ipu3/ipu3-cio2.c:1899:16: warning: Variable length > array is used. > drivers/media/pci/intel/ipu3/ipu3-cio2.c: In function 'cio2_pci_probe': > drivers/media/pci/intel/ipu3/ipu3-cio2.c:1726:14: warning: variable 'phys' > set but not used [-Wunused-but-set-variable] > phys_addr_t phys; > ^~~~ > > We should never use variable-length array on Kernel, as Linux stack is very > limited, and we have static analyzers to check for it at compilation time. > > Also, the logic should check if pci_resource_start() succeeded, instead of > just ignoring it. > > Please fix. > Thanks for catching the warnings, for the variable size array, from the code context, the size is limited to 128 bytes, maybe this language feature itself is not recommended, we will send a patch to address above soon. > > > > > Please pull. > > > > > > The following changes since commit > be9b53c83792e3898755dce90f8c632d40e7c83e: > > > > media: dvb-frontends: complete kernel-doc markups (2017-11-30 > > 04:19:05 -0500) > > > > are available in the git repository at: > > > > ssh://linuxtv.org/git/sailus/media_tree.git ipu3 > > > > for you to fetch changes up to > f178207daa68e817ab6fd702d81ed7c8637ab72c: > > > > intel-ipu3: cio2: add new MIPI-CSI2 driver (2017-11-30 14:19:47 > > +0200) > > > > > > Yong Zhi (3): > > videodev2.h, v4l2-ioctl: add IPU3 raw10 color format > > doc-rst: add IPU3 raw10 bayer pixel format definitions > > intel-ipu3: cio2: add new MIPI-CSI2 driver > > > > Documentation/media/uapi/v4l/pixfmt-rgb.rst|1 + > > .../media/uapi/v4l/pixfmt-srggb10-ipu3.rst | 335 > > MAINTAINERS|8 + > > drivers/media/pci/Kconfig |2 + > > drivers/media/pci/Makefile |3 +- > > drivers/media/pci/intel/Makefile |5 + > > drivers/media/pci/intel/ipu3/Kconfig | 19 + > > drivers/media/pci/intel/ipu3/Makefile |1 + > > drivers/media/pci/intel/ipu3/ipu3-cio2.c | 2052 > > > drivers/media/pci/intel/ipu3/ipu3-cio2.h | 449 + > > drivers/media/v4l2-core/v4l2-ioctl.c |4 + > > include/uapi/linux/videodev2.h |6 + > > 12 files changed, 2884 insertions(+), 1 deletion(-) create mode > > 100644 Documentation/media/uapi/v4l/pixfmt-srggb10-ipu3.rst > > create mode 100644 drivers/media/pci/intel/Makefile create mode > > 100644 drivers/media/pci/intel/ipu3/Kconfig > > create mode 100644 drivers/media/pci/intel/ipu3/Makefile > > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-cio2.c > > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-cio2.h > > > > > > Thanks, > Mauro From d7e9baf13b1a8c8308906a7ed213f75d12756c24 Mon Sep 17 00:00:00 2001 From: Yong Zhi Date: Tue, 12 Dec 2017 10:16:31 -0600 Subject: [PATCH] [PATCH v8] intel-ipu3: cio2: fix two warnings in the code Fix two warnings reported by Mauro Carvalho Chehab: ipu3-cio2.c:1899:16: warning: Variable length array is used. In function 'cio2_pci_probe': ipu3-cio2.c:1726:14: warning: variable 'phys' set but not used [-Wunused-but-set-variable] Hi, Sakari, can you squash the patch to your tree? Signed-off-by: Yong Zhi --- drivers/media/pci/intel/ipu3/ipu3-cio2.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c index 4295bdb8b192..941caa987dab 100644 --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c @@ -1723,7 +1723,6 @@ static int cio2_pci_probe(struct pci_dev *pci_dev, const struct pci_device_id *id) { struct cio2_device *cio2; - phys_addr_t phys; void __iomem *const *iomap; int r; @@ -1741,8 +1740,6 @@ static int cio2_pci_probe(struct pci_dev *pci_dev, dev_info(&pci_dev->dev, "device 0x%x (rev: 0x%x)\n", pci_dev->device,
[GIT PULL FOR v4.16] Various fixes
The following changes since commit 0ca4e3130402caea8731a7b54afde56a6edb17c9: media: pxa_camera: rename the soc_camera_ prefix to pxa_camera_ (2017-12-14 12:40:01 -0500) are available in the Git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.16c for you to fetch changes up to 03672a5bd2ba76b8b54a296e51e85919529d23b8: bdisp: Fix a possible sleep-in-atomic bug in bdisp_hw_save_request (2017-12-15 15:57:12 +0100) Flavio Ceolin (1): media: pxa_camera: disable and unprepare the clock source on error Hans Verkuil (1): pvrusb2: correctly return V4L2_PIX_FMT_MPEG in enum_fmt Jacopo Mondi (1): v4l: sh_mobile_ceu: Return buffers on streamoff() Jia-Ju Bai (1): bdisp: Fix a possible sleep-in-atomic bug in bdisp_hw_save_request Lucas Stach (1): coda: set min_buffers_needed Philipp Zabel (5): coda: fix capture TRY_FMT for YUYV with non-MB-aligned widths coda: round up frame sizes to multiples of 16 for MPEG-4 decoder coda: allocate space for mpeg4 decoder mvcol buffer coda: use correct offset for mpeg4 decoder mvcol buffer vb2: clear V4L2_BUF_FLAG_LAST when filling vb2_buffer Stanimir Varbanov (1): media: vb2: unify calling of set_page_dirty_lock drivers/media/platform/coda/coda-bit.c | 23 --- drivers/media/platform/coda/coda-common.c| 13 ++--- drivers/media/platform/pxa_camera.c | 4 +++- drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c | 7 ++- drivers/media/platform/sti/bdisp/bdisp-hw.c | 2 +- drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 32 +++- drivers/media/v4l2-core/videobuf2-dma-contig.c | 6 -- drivers/media/v4l2-core/videobuf2-dma-sg.c | 7 +++ drivers/media/v4l2-core/videobuf2-v4l2.c | 2 ++ 9 files changed, 48 insertions(+), 48 deletions(-)
Re: [PATCH 2/2] media: coda: Add i.MX51 (CodaHx4) support
Hi Hans, On Fri, 2017-12-15 at 15:22 +0100, Hans Verkuil wrote: > Hi Philipp, > > On 13/12/17 15:09, Philipp Zabel wrote: > > Add support for the CodaHx4 VPU used on i.MX51. > > > > Decoding h.264, MPEG-4, and MPEG-2 video works, as well as encoding > > h.264. MPEG-4 encoding is not enabled, it currently produces visual > > artifacts. > > > > Signed-off-by: Philipp Zabel > > --- > > drivers/media/platform/coda/coda-bit.c| 45 > > ++- > > drivers/media/platform/coda/coda-common.c | 44 > > +++--- > > drivers/media/platform/coda/coda.h| 1 + > > 3 files changed, 74 insertions(+), 16 deletions(-) > > > > > > > + [CODA_IMX51] = { > > + .firmware = { > > + "vpu_fw_imx51.bin", > > + "vpu/vpu_fw_imx51.bin", > > + "v4l-codahx4-imx51.bin" > > + }, > > + .product = CODA_HX4, > > + .codecs = codahx4_codecs, > > + .num_codecs = ARRAY_SIZE(codahx4_codecs), > > + .vdevs= codahx4_video_devices, > > + .num_vdevs= ARRAY_SIZE(codahx4_video_devices), > > + .workbuf_size = 128 * 1024, > > + .tempbuf_size = 304 * 1024, > > + .iram_size= 0x14000, > > + }, > > What's the status of the firmware? Is it going to be available in some > firmware > repository? I remember when testing other imx devices that it was a bit tricky > to get hold of the firmware. And googling v4l-codahx4-imx51.bin doesn't find > anything other than this patch. As far as I am aware, so far all efforts to get these firmware binaries relicensed in a way that makes them redistributable in linux-firmware have not succeeded. They are distributed by NXP directly in the firmware-imx package. The http://git.yoctoproject.org/cgit/cgit.cgi/meta-fsl-arm/ repository contains links to the latest version: wget http://www.nxp.com/lgfiles/NMG/MAD/YOCTO/firmware-imx-5.4.bin dd if=firmware-imx-5.4.bin bs=34087 skip=1 | tar xj cat firmware-imx-5.4/COPYING regards Philipp
Re: [PATCH 2/2] bdisp: Fix a possible sleep-in-atomic bug in bdisp_hw_save_request
Hi Thank you for the patch. On 12/12/17 14:47, Jia-Ju Bai wrote: > The driver may sleep under a spinlock. > The function call path is: > bdisp_device_run (acquire the spinlock) >bdisp_hw_update > bdisp_hw_save_request >devm_kzalloc(GFP_KERNEL) --> may sleep > > To fix it, GFP_KERNEL is replaced with GFP_ATOMIC. > > This bug is found by my static analysis tool(DSAC) and checked by my code > review. > > Signed-off-by: Jia-Ju Bai Reviewed-by: Fabien Dessenne > --- > drivers/media/platform/sti/bdisp/bdisp-hw.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/sti/bdisp/bdisp-hw.c > b/drivers/media/platform/sti/bdisp/bdisp-hw.c > index 4b62ceb..7b45b43 100644 > --- a/drivers/media/platform/sti/bdisp/bdisp-hw.c > +++ b/drivers/media/platform/sti/bdisp/bdisp-hw.c > @@ -1064,7 +1064,7 @@ static void bdisp_hw_save_request(struct bdisp_ctx *ctx) > if (!copy_node[i]) { > copy_node[i] = devm_kzalloc(ctx->bdisp_dev->dev, > sizeof(*copy_node[i]), > - GFP_KERNEL); > + GFP_ATOMIC); > if (!copy_node[i]) > return; > }
Re: [PATCH 1/2] bdisp: Fix a possible sleep-in-atomic bug in bdisp_hw_reset
Hi On 12/12/17 14:47, Jia-Ju Bai wrote: > The driver may sleep under a spinlock. > The function call path is: > bdisp_device_run (acquire the spinlock) >bdisp_hw_reset > msleep --> may sleep > > To fix it, msleep is replaced with mdelay. May I suggest you to use readl_poll_timeout_atomic (instead of the whole "for" block): this fixes the problem and simplifies the code? > > This bug is found by my static analysis tool(DSAC) and checked by my code > review. > > Signed-off-by: Jia-Ju Bai > --- > drivers/media/platform/sti/bdisp/bdisp-hw.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/sti/bdisp/bdisp-hw.c > b/drivers/media/platform/sti/bdisp/bdisp-hw.c > index b7892f3..4b62ceb 100644 > --- a/drivers/media/platform/sti/bdisp/bdisp-hw.c > +++ b/drivers/media/platform/sti/bdisp/bdisp-hw.c > @@ -382,7 +382,7 @@ int bdisp_hw_reset(struct bdisp_dev *bdisp) > for (i = 0; i < POLL_RST_MAX; i++) { > if (readl(bdisp->regs + BLT_STA1) & BLT_STA1_IDLE) > break; > - msleep(POLL_RST_DELAY_MS); > + mdelay(POLL_RST_DELAY_MS); > } > if (i == POLL_RST_MAX) > dev_err(bdisp->dev, "Reset timeout\n");
Re: [PATCH 3/5] include: v4l2_async: Add 'owner' field to notifier
Hi Jacopo, On Wed, Dec 13, 2017 at 07:26:18PM +0100, Jacopo Mondi wrote: > Notifiers can be registered as root notifiers (identified by a 'struct > v4l2_device *') or subdevice notifiers (identified by a 'struct > v4l2_subdev *'). In order to identify a notifier no matter if it is root > or not, add a 'struct fwnode_handle *owner' field, whose name can be > printed out for debug purposes. > > Signed-off-by: Jacopo Mondi You'll have struct device either through the v4l2_device or v4l2_subdev. Do you need an additional field for this? -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH/RFC v2 10/15] adv748x: csi2: add translation from pixelcode to CSI-2 datatype
Hi Kieran, On Thu, Dec 14, 2017 at 10:25:36PM +, Kieran Bingham wrote: > Hi Niklas, > > On 14/12/17 19:08, Niklas Söderlund wrote: > > This will be needed to fill out the frame descriptor information > > correctly. > > > > Signed-off-by: Niklas Söderlund > > --- > > drivers/media/i2c/adv748x/adv748x-csi2.c | 22 ++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c > > b/drivers/media/i2c/adv748x/adv748x-csi2.c > > index 2a5dff8c571013bf..a2a6d93077204731 100644 > > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c > > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c > > @@ -18,6 +18,28 @@ > > > > #include "adv748x.h" > > > > +struct adv748x_csi2_format { > > + unsigned int code; > > + unsigned int datatype; > > +}; > > + > > +static const struct adv748x_csi2_format adv748x_csi2_formats[] = { > > + { .code = MEDIA_BUS_FMT_RGB888_1X24,.datatype = 0x24, }, > > + { .code = MEDIA_BUS_FMT_UYVY8_1X16, .datatype = 0x1e, }, > > + { .code = MEDIA_BUS_FMT_UYVY8_2X8, .datatype = 0x1e, }, > > + { .code = MEDIA_BUS_FMT_YUYV10_2X10,.datatype = 0x1e, }, YUV 422 10 bit is associated to data type 0x1d in CSI-2 specifications > > +}; > > Is the datatype mapping specific to the ADV748x here? > or are these generic/common CSI2 mappings? > > What do those datatype magic numbers represent? They are fixed mappings defined by CSI-2 specifications and they should probably be generic to all drivers imho > > -- > Kieran > > > + > > +static unsigned int adv748x_csi2_code_to_datatype(unsigned int code) > > +{ > > + unsigned int i; > > + > > + for (i = 0; i < ARRAY_SIZE(adv748x_csi2_formats); i++) > > + if (adv748x_csi2_formats[i].code == code) > > + return adv748x_csi2_formats[i].datatype; > > + return 0; > > +} > > + > > static bool is_txa(struct adv748x_csi2 *tx) > > { > > return tx == &tx->state->txa; > >
Re: [PATCH 2/5] device property: Add fwnode_get_name() operation
Hi Jacopo, Thanks for the patch. Could you cc the next version to linux-a...@vger.kernel.org, please? Cc Mika, too. On Wed, Dec 13, 2017 at 07:26:17PM +0100, Jacopo Mondi wrote: > Add operation to retrieve the device name from a fwnode handle. > > Signed-off-by: Jacopo Mondi > --- > drivers/acpi/property.c | 6 ++ > drivers/base/property.c | 12 > drivers/of/property.c| 6 ++ > include/linux/fwnode.h | 2 ++ > include/linux/property.h | 1 + > 5 files changed, 27 insertions(+) > > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c > index e26ea20..1e3971c 100644 > --- a/drivers/acpi/property.c > +++ b/drivers/acpi/property.c > @@ -1186,6 +1186,11 @@ acpi_fwnode_property_read_string_array(const struct > fwnode_handle *fwnode, > val, nval); > } > > +static const char *acpi_fwnode_get_name(const struct fwnode_handle *fwnode) > +{ > + return acpi_dev_name(to_acpi_device_node(fwnode)); This works for device nodes but will fail miserably for non-device ACPI nodes. The ACPI nodes don't currently have a name such as the DT nodes do, it would certainly help debugging if they did. What you'll at least need to do is to check to_acpi_device_node() does not return NULL. acpi_dev_name() should be made const as well if the function is still used in v2. > +} > + > static struct fwnode_handle * > acpi_fwnode_get_named_child_node(const struct fwnode_handle *fwnode, >const char *childname) > @@ -1281,6 +1286,7 @@ static int acpi_fwnode_graph_parse_endpoint(const > struct fwnode_handle *fwnode, > acpi_fwnode_property_read_string_array, \ > .get_parent = acpi_node_get_parent, \ > .get_next_child_node = acpi_get_next_subnode, \ > + .get_name = acpi_fwnode_get_name, \ > .get_named_child_node = acpi_fwnode_get_named_child_node, \ > .get_reference_args = acpi_fwnode_get_reference_args, \ > .graph_get_next_endpoint = \ > diff --git a/drivers/base/property.c b/drivers/base/property.c > index 851b1b6..a87b4a9 100644 > --- a/drivers/base/property.c > +++ b/drivers/base/property.c > @@ -950,6 +950,18 @@ int device_add_properties(struct device *dev, > EXPORT_SYMBOL_GPL(device_add_properties); > > /** > + * fwnode_get_name - Return the fwnode_handle name > + * @fwnode: Firmware node to get name from > + * > + * Returns a pointer to the firmware node name > + */ > +const char *fwnode_get_name(const struct fwnode_handle *fwnode) > +{ > + return fwnode_call_ptr_op(fwnode, get_name); > +} > +EXPORT_SYMBOL(fwnode_get_name); > + > +/** > * fwnode_get_next_parent - Iterate to the node's parent > * @fwnode: Firmware whose parent is retrieved > * > diff --git a/drivers/of/property.c b/drivers/of/property.c > index 8ad33a4..6c195a8 100644 > --- a/drivers/of/property.c > +++ b/drivers/of/property.c > @@ -875,6 +875,11 @@ of_fwnode_property_read_string_array(const struct > fwnode_handle *fwnode, > of_property_count_strings(node, propname); > } > > +static const char *of_fwnode_get_name(const struct fwnode_handle *fwnode) > +{ > + return of_node_full_name(to_of_node(fwnode)); > +} > + > static struct fwnode_handle * > of_fwnode_get_parent(const struct fwnode_handle *fwnode) > { > @@ -988,6 +993,7 @@ const struct fwnode_operations of_fwnode_ops = { > .property_present = of_fwnode_property_present, > .property_read_int_array = of_fwnode_property_read_int_array, > .property_read_string_array = of_fwnode_property_read_string_array, > + .get_name = of_fwnode_get_name, > .get_parent = of_fwnode_get_parent, > .get_next_child_node = of_fwnode_get_next_child_node, > .get_named_child_node = of_fwnode_get_named_child_node, > diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h > index 411a84c..5d3a8c6 100644 > --- a/include/linux/fwnode.h > +++ b/include/linux/fwnode.h > @@ -57,6 +57,7 @@ struct fwnode_reference_args { > *otherwise. > * @property_read_string_array: Read an array of string properties. Return > zero > * on success, a negative error code otherwise. > + * @get_name: Return the fwnode name. > * @get_parent: Return the parent of an fwnode. > * @get_next_child_node: Return the next child node in an iteration. > * @get_named_child_node: Return a child node with a given name. > @@ -81,6 +82,7 @@ struct fwnode_operations { > (*property_read_string_array)(const struct fwnode_handle *fwnode_handle, > const char *propname, const char **val, > size_t nval); > + const char *(*get_name)(const struct fwnode_handle *fwnode); > struct fwnode_handle *(*get_parent)(const struct fwnode_handle *fwnode); >
Re: [PATCH] v4l: rcar-csi2: Don't bail out from probe on no ep
Hi Hans, On 2017-12-15 15:06:44 +0100, Hans Verkuil wrote: > Niklas, > > Did you look at this? If I should take it, can you Ack it? If you are > going to squash or add it to our of your own patch series, then let me > know and I can remove it from my todo queue. I did look at it and talked to Jacobo about it. I think I have a better solution to his problem which I hope to try out in the near future. If my workaround turns out to not solve his problem I will squash this in when I resend the rcar-csi2 patch-set so in any case I think you can drop it from your todo queue. The reason I'm a bit reluctant to ack this straight away is that I think it's kind of good that rcar-csi2 fails to probe if it finds no endpoints to work with, there is little use for the driver instance then. The problem Jacobo is trying to fix is related to how the rcar-vin Gen3 group parses DT and I made a small mistake there which was discovered by Jacobo. And since the original fault is in the rcar-vin driver I think the issue should be fixed in that driver. > > Regards, > > Hans > > On 05/12/17 21:41, Jacopo Mondi wrote: > > When rcar-csi interface is not connected to any endpoint, it fails and > > bails out from probe before registering its own video subdevice. > > This prevents rcar-vin registered notifier from completing and no > > subdevice is ever registered, also for other properly connected csi > > interfaces. > > > > Fix this not returning an error when no endpoint is connected to a csi > > interface and let the driver complete its probe function and register its > > own video subdevice. > > > > --- > > Niklas, > >please squash this patch in your next rcar-csi2 series (if you like it ;) > > > > As we have discussed this is particularly useful for gmsl setup, where > > adv748x > > is connected to CSI20 and max9286 to CSI40/CSI41. If we disable adv748x > > from DTS > > we need CSI20 probe to complete anyhow otherwise no subdevice gets > > registered > > for the two deserializers. > > > > Please note we cannot disable CSI20 entirely otherwise VIN's graph parsing > > breaks. > > > > Thanks > >j > > > > --- > > drivers/media/platform/rcar-vin/rcar-csi2.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c > > b/drivers/media/platform/rcar-vin/rcar-csi2.c > > index 2793efb..90c4062 100644 > > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c > > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > > @@ -928,8 +928,8 @@ static int rcar_csi2_parse_dt(struct rcar_csi2 *priv) > > > > ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0); > > if (!ep) { > > - dev_err(priv->dev, "Not connected to subdevice\n"); > > - return -EINVAL; > > + dev_dbg(priv->dev, "Not connected to subdevice\n"); > > + return 0; > > } > > > > ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep); > > -- > > 2.7.4 > > > -- Regards, Niklas Söderlund
Re: [PATCH 2/2] media: coda: Add i.MX51 (CodaHx4) support
Hi Philipp, On 13/12/17 15:09, Philipp Zabel wrote: > Add support for the CodaHx4 VPU used on i.MX51. > > Decoding h.264, MPEG-4, and MPEG-2 video works, as well as encoding > h.264. MPEG-4 encoding is not enabled, it currently produces visual > artifacts. > > Signed-off-by: Philipp Zabel > --- > drivers/media/platform/coda/coda-bit.c| 45 > ++- > drivers/media/platform/coda/coda-common.c | 44 +++--- > drivers/media/platform/coda/coda.h| 1 + > 3 files changed, 74 insertions(+), 16 deletions(-) > > + [CODA_IMX51] = { > + .firmware = { > + "vpu_fw_imx51.bin", > + "vpu/vpu_fw_imx51.bin", > + "v4l-codahx4-imx51.bin" > + }, > + .product = CODA_HX4, > + .codecs = codahx4_codecs, > + .num_codecs = ARRAY_SIZE(codahx4_codecs), > + .vdevs= codahx4_video_devices, > + .num_vdevs= ARRAY_SIZE(codahx4_video_devices), > + .workbuf_size = 128 * 1024, > + .tempbuf_size = 304 * 1024, > + .iram_size= 0x14000, > + }, What's the status of the firmware? Is it going to be available in some firmware repository? I remember when testing other imx devices that it was a bit tricky to get hold of the firmware. And googling v4l-codahx4-imx51.bin doesn't find anything other than this patch. Regards, Hans
Re: [PATCH 1/2] bdisp: Fix a possible sleep-in-atomic bug in bdisp_hw_reset
Fabien or Benjamin, can you take a look at these two patches? I'm a bit hesitant applying this since e.g. this bdisp_hw_reset() function might wait for up to a second, which is a mite long for an interrupt :-) Regards, Hans On 12/12/17 14:47, Jia-Ju Bai wrote: > The driver may sleep under a spinlock. > The function call path is: > bdisp_device_run (acquire the spinlock) > bdisp_hw_reset > msleep --> may sleep > > To fix it, msleep is replaced with mdelay. > > This bug is found by my static analysis tool(DSAC) and checked by my code > review. > > Signed-off-by: Jia-Ju Bai > --- > drivers/media/platform/sti/bdisp/bdisp-hw.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/sti/bdisp/bdisp-hw.c > b/drivers/media/platform/sti/bdisp/bdisp-hw.c > index b7892f3..4b62ceb 100644 > --- a/drivers/media/platform/sti/bdisp/bdisp-hw.c > +++ b/drivers/media/platform/sti/bdisp/bdisp-hw.c > @@ -382,7 +382,7 @@ int bdisp_hw_reset(struct bdisp_dev *bdisp) > for (i = 0; i < POLL_RST_MAX; i++) { > if (readl(bdisp->regs + BLT_STA1) & BLT_STA1_IDLE) > break; > - msleep(POLL_RST_DELAY_MS); > + mdelay(POLL_RST_DELAY_MS); > } > if (i == POLL_RST_MAX) > dev_err(bdisp->dev, "Reset timeout\n"); >
Re: [PATCH/RFC v2 07/15] rcar-csi2: use frame description information to configure CSI-2 bus
Hi Niklas, thanks for the patch! On Thu, Dec 14, 2017 at 08:08:27PM +0100, Niklas Söderlund wrote: > The driver now have access to frame descriptor information, use it. Only > enable the virtual channels which are described in the frame descriptor > and calculate the link based on all enabled streams. > > With multiplexed stream support it's now possible to have different > formats on the different source pads. Make source formats independent > off each other and disallowing a format on the multiplexed sink. > > Signed-off-by: Niklas Söderlund > --- > drivers/media/platform/rcar-vin/rcar-csi2.c | 112 > ++-- > 1 file changed, 58 insertions(+), 54 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c > b/drivers/media/platform/rcar-vin/rcar-csi2.c > index 6b607b2e31e26063..2dd7d03d622d5510 100644 > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > @@ -296,24 +296,22 @@ static const struct phtw_testdin_data > testdin_data_v3m_e3[] = { > #define CSI0CLKFREQRANGE(n) ((n & 0x3f) << 16) > > struct rcar_csi2_format { > - unsigned int code; > unsigned int datatype; > unsigned int bpp; > }; > > static const struct rcar_csi2_format rcar_csi2_formats[] = { > - { .code = MEDIA_BUS_FMT_RGB888_1X24,.datatype = 0x24, .bpp = 24 }, > - { .code = MEDIA_BUS_FMT_UYVY8_1X16, .datatype = 0x1e, .bpp = 16 }, > - { .code = MEDIA_BUS_FMT_UYVY8_2X8, .datatype = 0x1e, .bpp = 16 }, > - { .code = MEDIA_BUS_FMT_YUYV10_2X10,.datatype = 0x1e, .bpp = 16 }, > + { .datatype = 0x1e, .bpp = 16 }, > + { .datatype = 0x24, .bpp = 24 }, > }; > > -static const struct rcar_csi2_format *rcar_csi2_code_to_fmt(unsigned int > code) > +static const struct rcar_csi2_format > +*rcar_csi2_datatype_to_fmt(unsigned int datatype) > { > unsigned int i; > > for (i = 0; i < ARRAY_SIZE(rcar_csi2_formats); i++) > - if (rcar_csi2_formats[i].code == code) > + if (rcar_csi2_formats[i].datatype == datatype) > return rcar_csi2_formats + i; > > return NULL; > @@ -355,7 +353,7 @@ struct rcar_csi2 { > struct v4l2_async_notifier notifier; > struct v4l2_async_subdev remote; > > - struct v4l2_mbus_framefmt mf; > + struct v4l2_mbus_framefmt mf[4]; > > struct mutex lock; > int stream_count[4]; > @@ -411,25 +409,14 @@ static int rcar_csi2_wait_phy_start(struct rcar_csi2 > *priv) > return -ETIMEDOUT; > } > > -static int rcar_csi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp) > +static int rcar_csi2_calc_mbps(struct rcar_csi2 *priv, > +struct v4l2_subdev *source, > +struct v4l2_mbus_frame_desc *fd) > { > - struct media_pad *pad, *source_pad; > - struct v4l2_subdev *source = NULL; > struct v4l2_ctrl *ctrl; > + unsigned int i, bpp = 0; > u64 mbps; > > - /* Get remote subdevice */ > - pad = &priv->subdev.entity.pads[RCAR_CSI2_SINK]; > - source_pad = media_entity_remote_pad(pad); > - if (!source_pad) { > - dev_err(priv->dev, "Could not find remote source pad\n"); > - return -ENODEV; > - } > - source = media_entity_to_v4l2_subdev(source_pad->entity); > - > - dev_dbg(priv->dev, "Using source %s pad: %u\n", source->name, > - source_pad->index); > - > /* Read the pixel rate control from remote */ > ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE); > if (!ctrl) { > @@ -438,6 +425,21 @@ static int rcar_csi2_calc_mbps(struct rcar_csi2 *priv, > unsigned int bpp) > return -EINVAL; > } > > + /* Calculate total bpp */ > + for (i = 0; i < fd->num_entries; i++) { > + const struct rcar_csi2_format *format; > + > + format = rcar_csi2_datatype_to_fmt( > + fd->entry[i].bus.csi2.data_type); > + if (!format) { > + dev_err(priv->dev, "Unknown data type: %d\n", > + fd->entry[i].bus.csi2.data_type); > + return -EINVAL; > + } > + > + bpp += format->bpp; > + } > + > /* Calculate the phypll */ > mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp; > do_div(mbps, priv->lanes * 100); > @@ -489,39 +491,33 @@ static int rcar_csi2_set_phtw(struct rcar_csi2 *priv, > unsigned int mbps) > return 0; > } > > -static int rcar_csi2_start(struct rcar_csi2 *priv) > +static int rcar_csi2_start(struct rcar_csi2 *priv, struct v4l2_subdev > *source, > +struct v4l2_mbus_frame_desc *fd) I'm not sure I got this right, but with the new s_stream pad operation, and with rcar-csi2 endpoints connected to differenct VIN instances, is it possible for "rcar_csi2_s_stream()" to be called on the same rcar-csi2 instance from different VIN ins
Re: [PATCH] v4l: rcar-csi2: Don't bail out from probe on no ep
Niklas, Did you look at this? If I should take it, can you Ack it? If you are going to squash or add it to our of your own patch series, then let me know and I can remove it from my todo queue. Regards, Hans On 05/12/17 21:41, Jacopo Mondi wrote: > When rcar-csi interface is not connected to any endpoint, it fails and > bails out from probe before registering its own video subdevice. > This prevents rcar-vin registered notifier from completing and no > subdevice is ever registered, also for other properly connected csi > interfaces. > > Fix this not returning an error when no endpoint is connected to a csi > interface and let the driver complete its probe function and register its > own video subdevice. > > --- > Niklas, >please squash this patch in your next rcar-csi2 series (if you like it ;) > > As we have discussed this is particularly useful for gmsl setup, where adv748x > is connected to CSI20 and max9286 to CSI40/CSI41. If we disable adv748x from > DTS > we need CSI20 probe to complete anyhow otherwise no subdevice gets registered > for the two deserializers. > > Please note we cannot disable CSI20 entirely otherwise VIN's graph parsing > breaks. > > Thanks >j > > --- > drivers/media/platform/rcar-vin/rcar-csi2.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c > b/drivers/media/platform/rcar-vin/rcar-csi2.c > index 2793efb..90c4062 100644 > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > @@ -928,8 +928,8 @@ static int rcar_csi2_parse_dt(struct rcar_csi2 *priv) > > ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0); > if (!ep) { > - dev_err(priv->dev, "Not connected to subdevice\n"); > - return -EINVAL; > + dev_dbg(priv->dev, "Not connected to subdevice\n"); > + return 0; > } > > ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep); > -- > 2.7.4 >
Re: [PATCH v10 3/4] [media] platform: Add Synopsys DesignWare HDMI RX PHY e405 Driver
On 11/12/17 18:41, Jose Abreu wrote: > This adds support for the Synopsys DesignWare HDMI RX PHY e405. This > phy receives and decodes HDMI video that is delivered to a controller. > > Main features included in this driver are: > - Equalizer algorithm that chooses the phy best settings > according to the detected HDMI cable characteristics. > - Support for scrambling > - Support for HDMI 2.0 modes up to 6G (HDMI 4k@60Hz). > > The driver was implemented as a standalone V4L2 subdevice and the > phy interface with the controller was implemented using V4L2 ioctls. I > do not know if this is the best option but it is not possible to use the > existing API functions directly as we need specific functions that will > be called by the controller at specific configuration stages. > > There is also a bidirectional communication between controller and phy: > The phy must provide functions that the controller will call (i.e. > configuration functions) and the controller must provide read/write > callbacks, as well as other specific functions. > > Signed-off-by: Jose Abreu > Cc: Joao Pinto > Cc: Mauro Carvalho Chehab > Cc: Hans Verkuil > Cc: Sylwester Nawrocki > Cc: Philippe Ombredanne Reviewed-by: Hans Verkuil Regards, Hans > --- > Changes from v9: > - Use SPDX License ID (Philippe) > Changes from v4: > - Use usleep_range (Sylwester) > - Remove some comments (Sylwester) > - Parse phy version from of_device_id (Sylwester) > - Use "cfg" instead of "cfg-clk" (Sylwester, Rob) > - Change some messages to dev_dbg (Sylwester) > Changes from v3: > - Use v4l2 async API (Sylwester) > - Use clock API (Sylwester) > - Add compatible string (Sylwester) > Changes from RFC: > - Remove a bunch of functions that can be collapsed into > a single config() function > - Add comments for the callbacks and structures (Hans) > --- > drivers/media/platform/Kconfig| 2 + > drivers/media/platform/Makefile | 2 + > drivers/media/platform/dwc/Kconfig| 8 + > drivers/media/platform/dwc/Makefile | 1 + > drivers/media/platform/dwc/dw-hdmi-phy-e405.c | 822 > ++ > drivers/media/platform/dwc/dw-hdmi-phy-e405.h | 41 ++ > include/media/dwc/dw-hdmi-phy-pdata.h | 106 > 7 files changed, 982 insertions(+) > create mode 100644 drivers/media/platform/dwc/Kconfig > create mode 100644 drivers/media/platform/dwc/Makefile > create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.c > create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.h > create mode 100644 include/media/dwc/dw-hdmi-phy-pdata.h > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index fd0c998..0187cbd 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -33,6 +33,8 @@ source "drivers/media/platform/omap/Kconfig" > > source "drivers/media/platform/blackfin/Kconfig" > > +source "drivers/media/platform/dwc/Kconfig" > + > config VIDEO_SH_VOU > tristate "SuperH VOU video output driver" > depends on MEDIA_CAMERA_SUPPORT > diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile > index 003b0bb..2e879c6 100644 > --- a/drivers/media/platform/Makefile > +++ b/drivers/media/platform/Makefile > @@ -97,3 +97,5 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS) += > qcom/camss-8x16/ > obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/ > > obj-y+= meson/ > + > +obj-y+= dwc/ > diff --git a/drivers/media/platform/dwc/Kconfig > b/drivers/media/platform/dwc/Kconfig > new file mode 100644 > index 000..361d38d > --- /dev/null > +++ b/drivers/media/platform/dwc/Kconfig > @@ -0,0 +1,8 @@ > +config VIDEO_DWC_HDMI_PHY_E405 > + tristate "Synopsys Designware HDMI RX PHY e405 driver" > + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > + help > + Support for Synopsys Designware HDMI RX PHY. Version is e405. > + > + To compile this driver as a module, choose M here. The module > + will be called dw-hdmi-phy-e405. > diff --git a/drivers/media/platform/dwc/Makefile > b/drivers/media/platform/dwc/Makefile > new file mode 100644 > index 000..fc3b62c > --- /dev/null > +++ b/drivers/media/platform/dwc/Makefile > @@ -0,0 +1 @@ > +obj-$(CONFIG_VIDEO_DWC_HDMI_PHY_E405) += dw-hdmi-phy-e405.o > diff --git a/drivers/media/platform/dwc/dw-hdmi-phy-e405.c > b/drivers/media/platform/dwc/dw-hdmi-phy-e405.c > new file mode 100644 > index 000..e781cc1 > --- /dev/null > +++ b/drivers/media/platform/dwc/dw-hdmi-phy-e405.c > @@ -0,0 +1,822 @@ > +// SPDX-License-Identifier: (GPL-2.0 OR MIT) > +// Copyright (c) 2017 Synopsys, Inc. and/or its affiliates. > +// Synopsys DesignWare HDMI PHY e405 driver > + > +#include > +#include > +#include > +#include > +#include >
Re: [PATCH v10 2/4] MAINTAINERS: Add entry for Synopsys DesignWare HDMI drivers
On 15/12/17 14:35, Hans Verkuil wrote: > On 11/12/17 18:41, Jose Abreu wrote: >> Add an entry for Synopsys DesignWare HDMI Receivers drivers >> and phys. >> >> Signed-off-by: Jose Abreu >> Cc: Joao Pinto > > Reviewed-by: Hans Verkuil Oops, I was a bit too quick here. You're missing this file: Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt Regards, Hans > > Regards, > > Hans > > > >> --- >> MAINTAINERS | 7 +++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 7a52a66..a1675bc 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -13108,6 +13108,13 @@ L: net...@vger.kernel.org >> S: Supported >> F: drivers/net/ethernet/synopsys/ >> >> +SYNOPSYS DESIGNWARE HDMI RECEIVERS AND PHY DRIVERS >> +M: Jose Abreu >> +L: linux-media@vger.kernel.org >> +S: Maintained >> +F: drivers/media/platform/dwc/* >> +F: include/media/dwc/* >> + >> SYNOPSYS DESIGNWARE I2C DRIVER >> M: Jarkko Nikula >> R: Andy Shevchenko >> >
Re: [PATCH v10 1/4] dt-bindings: media: Document Synopsys DesignWare HDMI RX
On 11/12/17 18:41, Jose Abreu wrote: > Document the bindings for the Synopsys DesignWare HDMI RX. > > Signed-off-by: Jose Abreu > Acked-by: Rob Herring (v8) > Cc: Joao Pinto > Cc: Rob Herring > Cc: Mark Rutland > Cc: Mauro Carvalho Chehab > Cc: Hans Verkuil > Cc: Sylwester Nawrocki > Cc: devicet...@vger.kernel.org Reviewed-by: Hans Verkuil Regards, Hans > --- > Changes from v7: > - Remove SoC specific bindings (Rob) > Changes from v6: > - Document which properties are required/optional (Sylwester) > - Drop compatible string for SoC (Sylwester) > - Reword edid-phandle property (Sylwester) > - Typo fixes (Sylwester) > Changes from v4: > - Use "cfg" instead of "cfg-clk" (Rob) > - Change node names (Rob) > Changes from v3: > - Document the new DT bindings suggested by Sylwester > Changes from v2: > - Document edid-phandle property > --- > .../devicetree/bindings/media/snps,dw-hdmi-rx.txt | 58 > ++ > 1 file changed, 58 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt > > diff --git a/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt > b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt > new file mode 100644 > index 000..1dc09c6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt > @@ -0,0 +1,58 @@ > +Synopsys DesignWare HDMI RX Decoder > +=== > + > +This document defines device tree properties for the Synopsys DesignWare HDMI > +RX Decoder (DWC HDMI RX). > + > +The properties bellow belong to the Synopsys DesignWare HDMI RX Decoder node. > + > +Required properties: > + > +- compatible: Shall be "snps,dw-hdmi-rx". > +- reg: Memory mapped base address and length of the DWC HDMI RX registers. > +- interrupts: Reference to the DWC HDMI RX interrupt and the HDMI 5V sense > +interrupt. > +- clocks: Reference to the config clock. > +- clock-names: Shall be "cfg". > +- #address-cells: Shall be 1. > +- #size-cells: Shall be 0. > + > +Optional properties: > + > +- edid-phandle: Reference to the EDID handler block; if this property is not > +specified it is assumed that EDID is handled by device described by parent > +node of the HDMI RX node. You should not specify this property if your HDMI > RX > +controller does not have CEC. > + > +You also have to create a subnode for the PHY device. PHY node properties are > +as follows. > + > +Required properties: > + > +- compatible: Shall be "snps,dw-hdmi-phy-e405". > +- reg: Shall be the JTAG address of the PHY. > +- clocks: Reference to the config clock. > +- clock-names: Shall be "cfg". > + > +Example: > + > +hdmi_rx: hdmi-rx@0 { > + compatible = "snps,dw-hdmi-rx"; > + reg = <0x0 0x1>; > + interrupts = <1 2>; > + edid-phandle = <&dw_hdmi_edid>; > + > + clocks = <&dw_hdmi_refclk>; > + clock-names = "cfg"; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + hdmi-phy@fc { > + compatible = "snps,dw-hdmi-phy-e405"; > + reg = <0xfc>; > + > + clocks = <&dw_hdmi_refclk>; > + clock-names = "cfg"; > + }; > +}; >
Re: [PATCH v10 2/4] MAINTAINERS: Add entry for Synopsys DesignWare HDMI drivers
On 11/12/17 18:41, Jose Abreu wrote: > Add an entry for Synopsys DesignWare HDMI Receivers drivers > and phys. > > Signed-off-by: Jose Abreu > Cc: Joao Pinto Reviewed-by: Hans Verkuil Regards, Hans > --- > MAINTAINERS | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7a52a66..a1675bc 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -13108,6 +13108,13 @@ L: net...@vger.kernel.org > S: Supported > F: drivers/net/ethernet/synopsys/ > > +SYNOPSYS DESIGNWARE HDMI RECEIVERS AND PHY DRIVERS > +M: Jose Abreu > +L: linux-media@vger.kernel.org > +S: Maintained > +F: drivers/media/platform/dwc/* > +F: include/media/dwc/* > + > SYNOPSYS DESIGNWARE I2C DRIVER > M: Jarkko Nikula > R: Andy Shevchenko >
RE: [PATCH v9 0/2] media: ov7740: Add a V4L2 sensor-level driver
Hi Sakari, Do you have some comments on this version? Best Regards, Wenyou Yang > -Original Message- > From: linux-media-ow...@vger.kernel.org [mailto:linux-media- > ow...@vger.kernel.org] On Behalf Of Wenyou Yang > Sent: 2017年12月11日 9:32 > To: Mauro Carvalho Chehab ; Rob Herring > ; Mark Rutland > Cc: linux-ker...@vger.kernel.org; Nicolas Ferre - M43238 > ; devicet...@vger.kernel.org; Sakari Ailus > ; Jonathan Corbet ; Hans Verkuil > ; linux-arm-ker...@lists.infradead.org; Linux Media > Mailing List > ; Wenyou Yang - A41535 > > Subject: [PATCH v9 0/2] media: ov7740: Add a V4L2 sensor-level driver > > Add a Video4Linux2 sensor-level driver for the OmniVision OV7740 VGA camera > image sensor. > > Changes in v9: > - Use the new SPDX ids. > > Changes in v8: > - As the registers are written at stream start, remove the written >code from the set fmt function. > > Changes in v7: > - Add Acked-by tag. > - Fix the wrong handle of default register configuration. > - Add the missed assignment of ov7740->frmsize. > > Changes in v6: > - Remove unnecessary #include . > - Remove unnecessary comments and extra newline. > - Add const for some structures. > - Add the check of the return value from regmap_write(). > - Simplify the calling of __v4l2_ctrl_handler_setup(). > - Add the default format initialization function. > - Integrate the set_power() and enable/disable the clock into >one function. > > Changes in v5: > - Squash the driver and MAINTAINERS entry patches to one. > - Precede the driver patch with the bindings patch. > > Changes in v4: > - Assign 'val' a initial value to avoid warning: 'val' may be >used uninitialized. > - Rename REG_REG15 to avoid warning: "REG_REG15" redefined. > > Changes in v3: > - Explicitly document the "remote-endpoint" property. > - Put the MAINTAINERS change to a separate patch. > > Changes in v2: > - Split off the bindings into a separate patch. > - Add a new entry to the MAINTAINERS file. > > Wenyou Yang (2): > media: ov7740: Document device tree bindings > media: i2c: Add the ov7740 image sensor driver > > .../devicetree/bindings/media/i2c/ov7740.txt | 47 + > MAINTAINERS|8 + > drivers/media/i2c/Kconfig |8 + > drivers/media/i2c/Makefile |1 + > drivers/media/i2c/ov7740.c | 1216 > > 5 files changed, 1280 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7740.txt > create mode 100644 drivers/media/i2c/ov7740.c > > -- > 2.15.0
Re: [PATCH/RFC v2 06/15] rcar-csi2: use frame description information when propagating .s_stream()
Hi Niklas, On Thu, Dec 14, 2017 at 08:08:26PM +0100, Niklas Söderlund wrote: > Use the frame description from the remote subdevice of the rcar-csi2's > sink pad to get the remote pad and stream pad needed to propagate the > .s_stream() operation. > > The CSI-2 virtual channel which should be acted upon can be determined > by looking at which of the rcar-csi2 source pad the .s_stream() was > called on. This is because the rcar-csi2 acts as a demultiplexer for the > CSI-2 link on the one sink pad and outputs each virtual channel on a > distinct and known source pad. > > Signed-off-by: Niklas Söderlund > --- > drivers/media/platform/rcar-vin/rcar-csi2.c | 58 > - > 1 file changed, 41 insertions(+), 17 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c > b/drivers/media/platform/rcar-vin/rcar-csi2.c > index e0f56cc3d25179a9..6b607b2e31e26063 100644 > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > @@ -614,20 +614,31 @@ static void rcar_csi2_stop(struct rcar_csi2 *priv) > rcar_csi2_reset(priv); > } > > -static int rcar_csi2_sd_info(struct rcar_csi2 *priv, struct v4l2_subdev **sd) > +static int rcar_csi2_get_source_info(struct rcar_csi2 *priv, > + struct v4l2_subdev **subdev, I wonder if using struct media_pad for this would be cleaner. > + unsigned int *pad, > + struct v4l2_mbus_frame_desc *fd) > { > - struct media_pad *pad; > + struct media_pad *remote_pad; > > - pad = media_entity_remote_pad(&priv->pads[RCAR_CSI2_SINK]); > - if (!pad) { > - dev_err(priv->dev, "Could not find remote pad\n"); > + /* Get source subdevice and pad */ > + remote_pad = media_entity_remote_pad(&priv->pads[RCAR_CSI2_SINK]); > + if (!remote_pad) { > + dev_err(priv->dev, "Could not find remote source pad\n"); > return -ENODEV; > } > + *subdev = media_entity_to_v4l2_subdev(remote_pad->entity); > + *pad = remote_pad->index; > > - *sd = media_entity_to_v4l2_subdev(pad->entity); > - if (!*sd) { > - dev_err(priv->dev, "Could not find remote subdevice\n"); > - return -ENODEV; > + /* Get frame descriptor */ > + if (v4l2_subdev_call(*subdev, pad, get_frame_desc, *pad, fd)) { > + dev_err(priv->dev, "Could not read frame desc\n"); > + return -EINVAL; > + } > + > + if (fd->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) { > + dev_err(priv->dev, "Frame desc do not describe CSI-2 link"); > + return -EINVAL; I think this should also work with drivers that do not support frame descriptors. Alternatively support could be added for all drivers. In practice this would mean having a few bus specific implementations of get_frame_desc op that would dig the information from the frame format. Perhaps the former option would make sense here, for now. > } > > return 0; > @@ -637,9 +648,10 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, > unsigned int pad, > unsigned int stream, int enable) > { > struct rcar_csi2 *priv = sd_to_csi2(sd); > + struct v4l2_mbus_frame_desc fd; > struct v4l2_subdev *nextsd; > - unsigned int i, count = 0; > - int ret, vc; > + unsigned int i, rpad, count = 0; > + int ret, vc, rstream = -1; > > /* Only allow stream control on source pads and valid vc */ > vc = rcar_csi2_pad_to_vc(pad); > @@ -650,11 +662,23 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, > unsigned int pad, > if (stream != 0) > return -EINVAL; > > - mutex_lock(&priv->lock); > - > - ret = rcar_csi2_sd_info(priv, &nextsd); > + /* Get information about multiplexed link */ > + ret = rcar_csi2_get_source_info(priv, &nextsd, &rpad, &fd); > if (ret) > - goto out; > + return ret; > + > + /* Get stream on multiplexed link */ > + for (i = 0; i < fd.num_entries; i++) > + if (fd.entry[i].bus.csi2.channel == vc) > + rstream = fd.entry[i].stream; Virtual channel does not equate to the stream. You'll need the data type, too. You should actually obtain this from the configured routes instead. How does this work btw. if you have several streams on the same virtual channel that only have different data types? > + > + if (rstream < 0) { > + dev_err(priv->dev, "Could not find stream for vc %u\n", vc); > + return -EINVAL; > + } > + > + /* Start or stop the requested stream */ > + mutex_lock(&priv->lock); > > for (i = 0; i < 4; i++) > count += priv->stream_count[i]; > @@ -673,14 +697,14 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, > unsigned int pad, > } > > if (enable && priv->stream_count[vc]
more build failures
Hi, Don't get me wrong, I appreciate the vast amount of work going on here. Just letting you know what I'm seeing. + date Friday 15 December 23:28:52 AEDT 2017 + uname -a Linux ubuntu 4.4.0-103-generic #126-Ubuntu SMP Mon Dec 4 16:23:28 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux + cat /proc/version_signature Ubuntu 4.4.0-103.126-generic 4.4.98 ... + git --no-pager log -1 commit 4058fea6b2507661d66af5298c048d7c55338f42 Author: Jasmin Jessich Date: Tue Dec 12 12:49:01 2017 -0500 fixup v3.13_ddbridge_pcimsi.patch Required after the ddbridge 0.9.32 bump in media_tree. Signed-off-by: Jasmin Jessich Signed-off-by: Daniel Scheller ... + git --no-pager log -1 commit 0393e735649dc41358adb7b603bd57dad1ed3260 Author: Laurent Pinchart Date: Tue Oct 17 13:15:54 2017 -0400 media: uvcvideo: Stream error events carry no data According to the UVC specification, stream error events carry no data. Fix a buffer overflow (that should be harmless given data alignment) when reporting the stream error event by removing the data byte from the message. Signed-off-by: Laurent Pinchart Signed-off-by: Mauro Carvalho Chehab ... $ make allyesconfig make -C /home/me/git/clones/media_build/v4l allyesconfig make[1]: Entering directory '/home/me/git/clones/media_build/v4l' No version yet, using 4.4.0-103-generic make[2]: Entering directory '/home/me/git/clones/media_build/linux' Syncing with dir ../media/ Applying patches for kernel 4.4.0-103-generic patch -s -f -N -p1 -i ../backports/api_version.patch patch -s -f -N -p1 -i ../backports/pr_fmt.patch patch -s -f -N -p1 -i ../backports/debug.patch patch -s -f -N -p1 -i ../backports/drx39xxj.patch patch -s -f -N -p1 -i ../backports/v4.14_compiler_h.patch patch -s -f -N -p1 -i ../backports/v4.14_saa7146_timer_cast.patch patch -s -f -N -p1 -i ../backports/v4.14_module_param_call.patch patch -s -f -N -p1 -i ../backports/v4.12_revert_solo6x10_copykerneluser.patch patch -s -f -N -p1 -i ../backports/v4.10_sched_signal.patch 1 out of 1 hunk FAILED The text leading up to this was: -- |diff --git a/drivers/staging/media/lirc/lirc_zilog.c b/drivers/staging/media/lirc/lirc_zilog.c |index 015e41bd036e..fd61081b47d9 100644 |--- a/drivers/staging/media/lirc/lirc_zilog.c |+++ b/drivers/staging/media/lirc/lirc_zilog.c -- No file to patch. Skipping patch. 1 out of 1 hunk ignored Makefile:130: recipe for target 'apply_patches' failed make[2]: *** [apply_patches] Error 1 make[2]: Leaving directory '/home/me/git/clones/media_build/linux' Makefile:374: recipe for target 'allyesconfig' failed make[1]: *** [allyesconfig] Error 2 make[1]: Leaving directory '/home/me/git/clones/media_build/v4l' Makefile:26: recipe for target 'allyesconfig' failed make: *** [allyesconfig] Error 2 can't select all drivers at ./build line 525 + status=29 + date Friday 15 December 23:29:17 AEDT 2017 + [ 0 = 29 ]
Re: [GIT PULL FOR v4.16] staging/media: add NVIDIA Tegra video decoder driver
On 14.12.2017 14:06, Mauro Carvalho Chehab wrote: > Em Tue, 12 Dec 2017 16:28:40 +0100 > Hans Verkuil escreveu: > >> This adds a new NVIDIA Tegra video decoder driver. It is depending on the >> request API work since it is a stateless codec, so for now park this in >> staging. >> >> The dts patches should go through nvidia's tree. >> >> Regards, >> >> Hans >> >> The following changes since commit 330dada5957e3ca0c8811b14c45e3ac42c694651: >> >> media: dvb_frontend: fix return error code (2017-12-12 07:50:14 -0500) >> >> are available in the Git repository at: >> >> git://linuxtv.org/hverkuil/media_tree.git tegradec >> >> for you to fetch changes up to c3c530f45e48b33a2cc49cdeec246d255a5ca7db: >> >> staging: media: Introduce NVIDIA Tegra video decoder driver (2017-12-12 >> 16:06:06 +0100) >> >> >> Dmitry Osipenko (2): >> media: dt: bindings: Add binding for NVIDIA Tegra Video Decoder Engine >> staging: media: Introduce NVIDIA Tegra video decoder driver > > Ok, clearly, there are some things that are not OK on the driver, > otherwise, it won't be merging at staging. Yet, there warnings > there that should be considered before moving it out of staging: Sure, I'm aware of the checkpatch warnings and some of them aren't legit, others aren't very important and would be corrected later. The main reason of going into staging should be the lack of V4L2 interface support in the driver (necessary V4L API isn't there yet), see TODO. Certainly there are other things to be done besides the V4L interface before de-staging, going into staging is a very good variant right now, thanks for allowing to do it! [snip]
Re: [PATCH/RFC v2 05/15] rcar-csi2: count usage for each source pad
On Thu, Dec 14, 2017 at 08:08:25PM +0100, Niklas Söderlund wrote: > The R-Car CSI-2 hardware can output the same virtual channel > simultaneously to more then one R-Car VIN. For this reason we need to > move the usage counting from the global device to each source pad. > > If a source pads usage count go from 0 to 1 we need to signal that a new > stream should start, likewise if it go from 1 to 0 we need to stop a > stream. At the same time we only want to start or stop the R-Car > CSI-2 device only on the first or last stream. > > Signed-off-by: Niklas Söderlund > --- > drivers/media/platform/rcar-vin/rcar-csi2.c | 38 > +++-- > 1 file changed, 30 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c > b/drivers/media/platform/rcar-vin/rcar-csi2.c > index 8ce0bfeef1113f9c..e0f56cc3d25179a9 100644 > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > @@ -328,6 +328,14 @@ enum rcar_csi2_pads { > NR_OF_RCAR_CSI2_PAD, > }; > > +static int rcar_csi2_pad_to_vc(unsigned int pad) > +{ > + if (pad < RCAR_CSI2_SOURCE_VC0 || pad > RCAR_CSI2_SOURCE_VC3) > + return -EINVAL; > + > + return pad - RCAR_CSI2_SOURCE_VC0; > +} > + > struct rcar_csi2_info { > const struct phypll_hsfreqrange *hsfreqrange; > const struct phtw_testdin_data *testdin_data; > @@ -350,7 +358,7 @@ struct rcar_csi2 { > struct v4l2_mbus_framefmt mf; > > struct mutex lock; > - int stream_count; > + int stream_count[4]; Why 4? > > unsigned short lanes; > unsigned char lane_swap[4]; > @@ -630,7 +638,13 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, > unsigned int pad, > { > struct rcar_csi2 *priv = sd_to_csi2(sd); > struct v4l2_subdev *nextsd; > - int ret; > + unsigned int i, count = 0; > + int ret, vc; > + > + /* Only allow stream control on source pads and valid vc */ > + vc = rcar_csi2_pad_to_vc(pad); > + if (vc < 0) > + return vc; > > /* Only one stream on each source pad */ > if (stream != 0) > @@ -642,7 +656,10 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, > unsigned int pad, > if (ret) > goto out; > > - if (enable && priv->stream_count == 0) { > + for (i = 0; i < 4; i++) > + count += priv->stream_count[i]; > + > + if (enable && count == 0) { > pm_runtime_get_sync(priv->dev); > > ret = rcar_csi2_start(priv); > @@ -650,20 +667,23 @@ static int rcar_csi2_s_stream(struct v4l2_subdev *sd, > unsigned int pad, > pm_runtime_put(priv->dev); > goto out; > } > + } else if (!enable && count == 1) { > + rcar_csi2_stop(priv); > + pm_runtime_put(priv->dev); > + } > > + if (enable && priv->stream_count[vc] == 0) { > ret = v4l2_subdev_call(nextsd, video, s_stream, 1); > if (ret) { > rcar_csi2_stop(priv); > pm_runtime_put(priv->dev); > goto out; > } > - } else if (!enable && priv->stream_count == 1) { > - rcar_csi2_stop(priv); > + } else if (!enable && priv->stream_count[vc] == 1) { Do I understand correctly that you can start and streams here one by one, independently of each other? Sometimes this might not be the case. I wonder if we should have a way to tell that to the caller. > ret = v4l2_subdev_call(nextsd, video, s_stream, 0); > - pm_runtime_put(priv->dev); > } > > - priv->stream_count += enable ? 1 : -1; > + priv->stream_count[vc] += enable ? 1 : -1; > out: > mutex_unlock(&priv->lock); > > @@ -919,7 +939,9 @@ static int rcar_csi2_probe(struct platform_device *pdev) > priv->dev = &pdev->dev; > > mutex_init(&priv->lock); > - priv->stream_count = 0; > + > + for (i = 0; i < 4; i++) > + priv->stream_count[i] = 0; > > ret = rcar_csi2_probe_resources(priv, pdev); > if (ret) { > -- > 2.15.1 > -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH/RFC v2 03/15] rcar-vin: use the pad and stream aware s_stream
Hej, On Thu, Dec 14, 2017 at 08:08:23PM +0100, Niklas Söderlund wrote: > To work with multiplexed streams the pad and stream aware s_stream > operation needs to be used. > > Signed-off-by: Niklas Söderlund > --- > drivers/media/platform/rcar-vin/rcar-dma.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c > b/drivers/media/platform/rcar-vin/rcar-dma.c > index cf30e5fceb1d493a..8435491535060eae 100644 > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > @@ -1180,7 +1180,7 @@ static int rvin_set_stream(struct rvin_dev *vin, int on) > > if (!on) { > media_pipeline_stop(vin->vdev.entity.pads); > - return v4l2_subdev_call(sd, video, s_stream, 0); > + return v4l2_subdev_call(sd, pad, s_stream, pad->index, 0, 0); Have you thought of adding a wrapper for the s_stream callback? I think you should either change all s_stream callbacks from video to pad, or add a wrapper which then calls the video op instead of the pad op if the pad op does not exist. Otherwise we again have two non-interoperable classes of drivers for no good reason. Thinking about it, I'm not all that certain changing all instances would be that much work in the end; it should be done anyway. Devices that have a single stream (i.e. everything right now) just don't care about the pad number. > } > > fmt.pad = pad->index; > @@ -1239,12 +1239,14 @@ static int rvin_set_stream(struct rvin_dev *vin, int > on) > if (media_pipeline_start(vin->vdev.entity.pads, pipe)) > return -EPIPE; > > - ret = v4l2_subdev_call(sd, video, s_stream, 1); > + ret = v4l2_subdev_call(sd, pad, s_stream, pad->index, 0, 1); > if (ret == -ENOIOCTLCMD) > ret = 0; > if (ret) > media_pipeline_stop(vin->vdev.entity.pads); > > + vin_dbg(vin, "pad: %u stream: 0 enable: %d\n", pad->index, on); > + > return ret; > } > > -- > 2.15.1 > -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH/RFC v2 02/15] rcar-vin: use pad as the starting point for a pipeline
On Thu, Dec 14, 2017 at 08:08:22PM +0100, Niklas Söderlund wrote: > The pipeline will be moved from the entity to the pads; reflect this in > the media pipeline function API. I'll merge this to "media: entity: Use pad as the starting point for a pipeline" if you're fine with that. I haven't compiled everything for some time, and newly added drivers may be lacking these changes. I'll re-check that soonish. > > Signed-off-by: Niklas Söderlund > --- > drivers/media/platform/rcar-vin/rcar-dma.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c > b/drivers/media/platform/rcar-vin/rcar-dma.c > index 03a914361a33125c..cf30e5fceb1d493a 100644 > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > @@ -1179,7 +1179,7 @@ static int rvin_set_stream(struct rvin_dev *vin, int on) > return -EPIPE; > > if (!on) { > - media_pipeline_stop(&vin->vdev.entity); > + media_pipeline_stop(vin->vdev.entity.pads); > return v4l2_subdev_call(sd, video, s_stream, 0); > } > > @@ -1235,15 +1235,15 @@ static int rvin_set_stream(struct rvin_dev *vin, int > on) > fmt.format.height != vin->format.height) > return -EPIPE; > > - pipe = sd->entity.pipe ? sd->entity.pipe : &vin->vdev.pipe; > - if (media_pipeline_start(&vin->vdev.entity, pipe)) > + pipe = sd->entity.pads->pipe ? sd->entity.pads->pipe : &vin->vdev.pipe; > + if (media_pipeline_start(vin->vdev.entity.pads, pipe)) > return -EPIPE; > > ret = v4l2_subdev_call(sd, video, s_stream, 1); > if (ret == -ENOIOCTLCMD) > ret = 0; > if (ret) > - media_pipeline_stop(&vin->vdev.entity); > + media_pipeline_stop(vin->vdev.entity.pads); > > return ret; > } > -- > 2.15.1 > -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH/RFC v2 01/15] v4l2-subdev.h: add pad and stream aware s_stream
Hejssan Niklas, Tack för uppdaterade lappor! On Thu, Dec 14, 2017 at 08:08:21PM +0100, Niklas Söderlund wrote: > To be able to start and stop individual streams of a multiplexed pad the > s_stream operation needs to be both pad and stream aware. Add a new > operation to pad ops to facilitate this. > > Signed-off-by: Niklas Söderlund > --- > include/media/v4l2-subdev.h | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index a30a94fad8dbacde..7288209338a48fda 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -669,6 +669,9 @@ struct v4l2_subdev_pad_config { > * > * @set_frame_desc: set the low level media bus frame parameters, @fd array > * may be adjusted by the subdev driver to device > capabilities. > + * > + * @s_stream: used to notify the driver that a stream will start or has > + * stopped. This is actually the callback which is used to control the stream state. The above suggests that it's a notification of something that has happened (or about to happen). How about: Enable or disable streaming on a sub-device pad. > */ > struct v4l2_subdev_pad_ops { > int (*init_cfg)(struct v4l2_subdev *sd, > @@ -713,6 +716,8 @@ struct v4l2_subdev_pad_ops { > struct v4l2_subdev_routing *route); > int (*set_routing)(struct v4l2_subdev *sd, > struct v4l2_subdev_routing *route); > + int (*s_stream)(struct v4l2_subdev *sd, unsigned int pad, > + unsigned int stream, int enable); How about bool for enable? > }; > > /** -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH v10 4/4] [media] platform: Add Synopsys DesignWare HDMI RX Controller Driver
Hi Hans, On 13-12-2017 20:49, Hans Verkuil wrote: > On 13/12/17 15:00, Jose Abreu wrote: >> >> Indeed. I compared the values with the spec and they are not >> correct. Even hsync is wrong. I already corrected in the code the >> hsync but regarding interlace I'm not seeing an easy way to do >> this without using interrupts in each vsync because the register >> I was toggling does not behave as I expected (I misunderstood the >> databook). Maybe we should not detect interlaced modes for now? >> Or not fill the il_ fields? > As I mentioned above you as long as you get a good backporch value you > can deduce from whether it is an odd or even number to which field it > belongs and fill in the other values. So I think you only need to read > these values for one field. > > Filling in good values here (at least as far as is possible since not all > hardware can give it) will help debugging issues, even if you otherwise do > not support interlaced. Ok, I will fill the fields. Until the end of January I will be quite busy in another project so if you could review the remaining patches of this series I would appreciate very much ... This way when I have the time I can code all the changes and send them at once. Thanks and Best Regards, Jose Miguel Abreu > > Regards, > > Hans
Re: [PATCH] v4l2-dv-timings: add v4l2_hdmi_colorimetry()
Hi Hans, On 15-12-2017 09:48, Hans Verkuil wrote: > Add the v4l2_hdmi_colorimetry() function so we have a single function > that determines the colorspace, YCbCr encoding, quantization range and > transfer function from the InfoFrame data. You could also make AVI infoframe optional and return RGB in that case. Anyway, I took a quick look at the spec and everything seems ok. Reviewed-by: Jose Abreu Best Regards, Jose Miguel Abreu > > Signed-off-by: Hans Verkuil > --- > Tim, you can add this patch to your driver patch series and just call it. > Note that this colorspace information is what the HDMI gives you, if you > need to convert this than you will need to update this information accordingly > (e.g. lim to full range, 601 to 709 conversions, etc.) before giving it to > userspace. > > Jose, I'm not sure if you need it in your driver, but this is probably useful > regardless. > > Regards, > > Hans > --- > drivers/media/v4l2-core/v4l2-dv-timings.c | 141 > ++ > include/media/v4l2-dv-timings.h | 21 + > 2 files changed, 162 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c > b/drivers/media/v4l2-core/v4l2-dv-timings.c > index 930f9c53a64e..0182d3d3f807 100644 > --- a/drivers/media/v4l2-core/v4l2-dv-timings.c > +++ b/drivers/media/v4l2-core/v4l2-dv-timings.c > @@ -27,6 +27,7 @@ > #include > #include > #include > +#include > > MODULE_AUTHOR("Hans Verkuil"); > MODULE_DESCRIPTION("V4L2 DV Timings Helper Functions"); > @@ -814,3 +815,143 @@ struct v4l2_fract v4l2_calc_aspect_ratio(u8 > hor_landscape, u8 vert_portrait) > return aspect; > } > EXPORT_SYMBOL_GPL(v4l2_calc_aspect_ratio); > + > +/** v4l2_hdmi_rx_colorimetry - determine HDMI colorimetry information > + * based on various InfoFrames. > + * @avi - the AVI InfoFrame > + * @hdmi - the HDMI Vendor InfoFrame, may be NULL > + * @height - the frame height > + * > + * Determines the HDMI colorimetry information, i.e. how the HDMI > + * pixel color data should be interpreted. > + * > + * Note that some of the newer features (DCI-P3, HDR) are not yet > + * implemented: the hdmi.h header needs to be updated to the HDMI 2.0 > + * and CTA-861-G standards. > + */ > +struct v4l2_hdmi_colorimetry > +v4l2_hdmi_rx_colorimetry(const struct hdmi_avi_infoframe *avi, > + const struct hdmi_vendor_infoframe *hdmi, > + unsigned int height) > +{ > + struct v4l2_hdmi_colorimetry c = { > + V4L2_COLORSPACE_SRGB, > + V4L2_YCBCR_ENC_DEFAULT, > + V4L2_QUANTIZATION_FULL_RANGE, > + V4L2_XFER_FUNC_SRGB > + }; > + bool is_ce = avi->video_code || (hdmi && hdmi->vic); > + bool is_sdtv = height <= 576; > + bool default_is_lim_range_rgb = avi->video_code > 1; > + > + switch (avi->colorspace) { > + case HDMI_COLORSPACE_RGB: > + /* RGB pixel encoding */ > + switch (avi->colorimetry) { > + case HDMI_COLORIMETRY_EXTENDED: > + switch (avi->extended_colorimetry) { > + case HDMI_EXTENDED_COLORIMETRY_ADOBE_RGB: > + c.colorspace = V4L2_COLORSPACE_ADOBERGB; > + c.xfer_func = V4L2_XFER_FUNC_ADOBERGB; > + break; > + case HDMI_EXTENDED_COLORIMETRY_BT2020: > + c.colorspace = V4L2_COLORSPACE_BT2020; > + c.xfer_func = V4L2_XFER_FUNC_709; > + break; > + default: > + break; > + } > + break; > + default: > + break; > + } > + switch (avi->quantization_range) { > + case HDMI_QUANTIZATION_RANGE_LIMITED: > + c.quantization = V4L2_QUANTIZATION_LIM_RANGE; > + break; > + case HDMI_QUANTIZATION_RANGE_FULL: > + break; > + default: > + if (default_is_lim_range_rgb) > + c.quantization = V4L2_QUANTIZATION_LIM_RANGE; > + break; > + } > + break; > + > + default: > + /* YCbCr pixel encoding */ > + c.quantization = V4L2_QUANTIZATION_LIM_RANGE; > + switch (avi->colorimetry) { > + case HDMI_COLORIMETRY_NONE: > + if (!is_ce) > + break; > + if (is_sdtv) { > + c.colorspace = V4L2_COLORSPACE_SMPTE170M; > + c.ycbcr_enc = V4L2_YCBCR_ENC_601; > + } else { > + c.colorspace = V4L2_COLORSPACE_REC709; > + c.ycbcr_enc = V4L2_YCBCR_ENC_709; > + } > + c.xf
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Hi Maxime, On Fri, 15 Dec 2017 11:50:47 +0100 Maxime Ripard wrote: > Hi Yong, > > On Mon, Dec 04, 2017 at 05:45:11PM +0800, Yong wrote: > > I just noticed that you are using the second iteration? > > Have you received my third iteration? > > Sorry for the late reply, and for not coming back to you yet about > that test. No, this is still in your v2. I've definitely received your > v3, I just didn't have time to update to it yet. > > But don't worry, my mail was mostly to know if you had tested that > setup on your side to try to nail down the issue on my end, not really > a review or comment that would prevent your patch from going in. I mean, The v2 exactly has a bug which may cause the CSI writing frame to a wrong memory address. BTW, should I send a new version. I have made some improve sine v3. > > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com Thanks, Yong
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Hi Yong, On Mon, Dec 04, 2017 at 05:45:11PM +0800, Yong wrote: > I just noticed that you are using the second iteration? > Have you received my third iteration? Sorry for the late reply, and for not coming back to you yet about that test. No, this is still in your v2. I've definitely received your v3, I just didn't have time to update to it yet. But don't worry, my mail was mostly to know if you had tested that setup on your side to try to nail down the issue on my end, not really a review or comment that would prevent your patch from going in. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
Re: [RFC PATCH 1/9] media: add request API core and UAPI
Alexandre: On Fri, Dec 15, 2017 at 8:56 AM, Alexandre Courbot wrote: > The request API provides a way to group buffers and device parameters > into units of work to be queued and executed. This patch introduces the > UAPI and core framework. > > This patch is based on the previous work by Laurent Pinchart. The core > has changed considerably, but the UAPI is mostly untouched. > > Signed-off-by: Alexandre Courbot --- /dev/null > +++ b/drivers/media/media-request.c > @@ -0,0 +1,390 @@ > +/* > + * Request and request queue base management > + * > + * Copyright (C) 2017, The Chromium OS Authors. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ Have you considered using the new SPDX tags instead of this fine but long legalese? And if other Chromium contributors could follow suit and you could spread the word that would be even better! See Thomas doc patches [1] for details. Thanks! [1] https://lkml.org/lkml/2017/12/4/934 -- Cordially Philippe Ombredanne
Re: [PATCH 2/9] v4l: vsp1: Print the correct blending unit name in debug messages
Hi Laurent, On 03/12/17 10:57, Laurent Pinchart wrote: > The DRM pipelines can use either the BRU or the BRS for blending. Make > sure the right name is used in debugging messages to avoid confusion. This could likely tag along with the preceding [PATCH 1/9] on it's short cut to mainline before the rest of the CRC series is reviewed. > Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham > --- > drivers/media/platform/vsp1/vsp1_drm.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c > b/drivers/media/platform/vsp1/vsp1_drm.c > index ac85942162c1..0fe541084f5c 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/vsp1/vsp1_drm.c > @@ -400,8 +400,11 @@ static int vsp1_du_setup_rpf_pipe(struct vsp1_device > *vsp1, > struct v4l2_subdev_selection sel; > struct v4l2_subdev_format format; > const struct v4l2_rect *crop; > + const char *bru_name; > int ret; > > + bru_name = pipe->bru->type == VSP1_ENTITY_BRU ? "BRU" : "BRS"; > + > /* >* Configure the format on the RPF sink pad and propagate it up to the >* BRU sink pad. > @@ -473,9 +476,9 @@ static int vsp1_du_setup_rpf_pipe(struct vsp1_device > *vsp1, > if (ret < 0) > return ret; > > - dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on BRU pad %u\n", > + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n", > __func__, format.format.width, format.format.height, > - format.format.code, format.pad); > + format.format.code, bru_name, format.pad); > > sel.pad = bru_input; > sel.target = V4L2_SEL_TGT_COMPOSE; > @@ -486,10 +489,9 @@ static int vsp1_du_setup_rpf_pipe(struct vsp1_device > *vsp1, > if (ret < 0) > return ret; > > - dev_dbg(vsp1->dev, > - "%s: set selection (%u,%u)/%ux%u on BRU pad %u\n", > + dev_dbg(vsp1->dev, "%s: set selection (%u,%u)/%ux%u on %s pad %u\n", > __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height, > - sel.pad); > + bru_name, sel.pad); > > return 0; > } >
Re: [PATCH 1/9] v4l: vsp1: Fix display stalls when requesting too many inputs
Hi Laurent, As this is a prevents hardware hangs, and is a distinct patch on it's own - I feel it should be on an accelerated path to integration, and should be merged separately from the rest of the CRC feature series. On 03/12/17 10:57, Laurent Pinchart wrote: > Make sure we don't accept more inputs than the hardware can handle. This > is a temporary fix to avoid display stall, we need to instead allocate > the BRU or BRS to display pipelines dynamically based on the number of > planes they each use. > > Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham > --- > drivers/media/platform/vsp1/vsp1_drm.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c > b/drivers/media/platform/vsp1/vsp1_drm.c > index 7ce69f23f50a..ac85942162c1 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/vsp1/vsp1_drm.c > @@ -530,6 +530,15 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned > int pipe_index) > struct vsp1_rwpf *rpf = vsp1->rpf[i]; > unsigned int j; > > + /* > + * Make sure we don't accept more inputs than the hardware can > + * handle. This is a temporary fix to avoid display stall, we > + * need to instead allocate the BRU or BRS to display pipelines > + * dynamically based on the number of planes they each use. > + */ > + if (pipe->num_inputs >= pipe->bru->source_pad) > + pipe->inputs[i] = NULL; > + > if (!pipe->inputs[i]) > continue; > >
[PATCH] v4l2-dv-timings: add v4l2_hdmi_colorimetry()
Add the v4l2_hdmi_colorimetry() function so we have a single function that determines the colorspace, YCbCr encoding, quantization range and transfer function from the InfoFrame data. Signed-off-by: Hans Verkuil --- Tim, you can add this patch to your driver patch series and just call it. Note that this colorspace information is what the HDMI gives you, if you need to convert this than you will need to update this information accordingly (e.g. lim to full range, 601 to 709 conversions, etc.) before giving it to userspace. Jose, I'm not sure if you need it in your driver, but this is probably useful regardless. Regards, Hans --- drivers/media/v4l2-core/v4l2-dv-timings.c | 141 ++ include/media/v4l2-dv-timings.h | 21 + 2 files changed, 162 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c b/drivers/media/v4l2-core/v4l2-dv-timings.c index 930f9c53a64e..0182d3d3f807 100644 --- a/drivers/media/v4l2-core/v4l2-dv-timings.c +++ b/drivers/media/v4l2-core/v4l2-dv-timings.c @@ -27,6 +27,7 @@ #include #include #include +#include MODULE_AUTHOR("Hans Verkuil"); MODULE_DESCRIPTION("V4L2 DV Timings Helper Functions"); @@ -814,3 +815,143 @@ struct v4l2_fract v4l2_calc_aspect_ratio(u8 hor_landscape, u8 vert_portrait) return aspect; } EXPORT_SYMBOL_GPL(v4l2_calc_aspect_ratio); + +/** v4l2_hdmi_rx_colorimetry - determine HDMI colorimetry information + * based on various InfoFrames. + * @avi - the AVI InfoFrame + * @hdmi - the HDMI Vendor InfoFrame, may be NULL + * @height - the frame height + * + * Determines the HDMI colorimetry information, i.e. how the HDMI + * pixel color data should be interpreted. + * + * Note that some of the newer features (DCI-P3, HDR) are not yet + * implemented: the hdmi.h header needs to be updated to the HDMI 2.0 + * and CTA-861-G standards. + */ +struct v4l2_hdmi_colorimetry +v4l2_hdmi_rx_colorimetry(const struct hdmi_avi_infoframe *avi, +const struct hdmi_vendor_infoframe *hdmi, +unsigned int height) +{ + struct v4l2_hdmi_colorimetry c = { + V4L2_COLORSPACE_SRGB, + V4L2_YCBCR_ENC_DEFAULT, + V4L2_QUANTIZATION_FULL_RANGE, + V4L2_XFER_FUNC_SRGB + }; + bool is_ce = avi->video_code || (hdmi && hdmi->vic); + bool is_sdtv = height <= 576; + bool default_is_lim_range_rgb = avi->video_code > 1; + + switch (avi->colorspace) { + case HDMI_COLORSPACE_RGB: + /* RGB pixel encoding */ + switch (avi->colorimetry) { + case HDMI_COLORIMETRY_EXTENDED: + switch (avi->extended_colorimetry) { + case HDMI_EXTENDED_COLORIMETRY_ADOBE_RGB: + c.colorspace = V4L2_COLORSPACE_ADOBERGB; + c.xfer_func = V4L2_XFER_FUNC_ADOBERGB; + break; + case HDMI_EXTENDED_COLORIMETRY_BT2020: + c.colorspace = V4L2_COLORSPACE_BT2020; + c.xfer_func = V4L2_XFER_FUNC_709; + break; + default: + break; + } + break; + default: + break; + } + switch (avi->quantization_range) { + case HDMI_QUANTIZATION_RANGE_LIMITED: + c.quantization = V4L2_QUANTIZATION_LIM_RANGE; + break; + case HDMI_QUANTIZATION_RANGE_FULL: + break; + default: + if (default_is_lim_range_rgb) + c.quantization = V4L2_QUANTIZATION_LIM_RANGE; + break; + } + break; + + default: + /* YCbCr pixel encoding */ + c.quantization = V4L2_QUANTIZATION_LIM_RANGE; + switch (avi->colorimetry) { + case HDMI_COLORIMETRY_NONE: + if (!is_ce) + break; + if (is_sdtv) { + c.colorspace = V4L2_COLORSPACE_SMPTE170M; + c.ycbcr_enc = V4L2_YCBCR_ENC_601; + } else { + c.colorspace = V4L2_COLORSPACE_REC709; + c.ycbcr_enc = V4L2_YCBCR_ENC_709; + } + c.xfer_func = V4L2_XFER_FUNC_709; + break; + case HDMI_COLORIMETRY_ITU_601: + c.colorspace = V4L2_COLORSPACE_SMPTE170M; + c.ycbcr_enc = V4L2_YCBCR_ENC_601; + c.xfer_func = V4L2_XFER_FUNC_709; + break; + case HDMI_COLORIMETRY_IT
Re: [PATCH] media: v4l: xilinx: Use SPDX-License-Identifier
Em Fri, 15 Dec 2017 00:02:21 +0200 Laurent Pinchart escreveu: > Hi Mauro, > > On Thursday, 14 December 2017 23:50:03 EET Mauro Carvalho Chehab wrote: > > Em Thu, 14 Dec 2017 21:57:06 +0100 Greg KH escreveu: > > > On Thu, Dec 14, 2017 at 10:44:16PM +0200, Laurent Pinchart wrote: > > >> On Thursday, 14 December 2017 22:08:51 EET Greg KH wrote: > > >>> On Thu, Dec 14, 2017 at 09:05:27PM +0200, Laurent Pinchart wrote: > > On Thursday, 14 December 2017 20:54:39 EET Joe Perches wrote: > > > On Thu, 2017-12-14 at 20:37 +0200, Laurent Pinchart wrote: > > >> On Thursday, 14 December 2017 20:32:20 EET Joe Perches wrote: > > >>> On Thu, 2017-12-14 at 20:28 +0200, Laurent Pinchart wrote: > > On Thursday, 14 December 2017 19:05:27 EET Mauro Carvalho Chehab > > wrote: > > > Em Fri, 8 Dec 2017 18:05:37 +0530 Dhaval Shah escreveu: > > >> SPDX-License-Identifier is used for the Xilinx Video IP and > > >> related drivers. > > >> > > >> Signed-off-by: Dhaval Shah > > > > > > Hi Dhaval, > > > > > > You're not listed as one of the Xilinx driver maintainers. I'm > > > afraid that, without their explicit acks, sent to the ML, I > > > can't accept a patch touching at the driver's license tags. > > > > The patch doesn't change the license, I don't see why it would > > cause any issue. Greg isn't listed as the maintainer or copyright > > holder of any of the 10k+ files to which he added an SPDX license > > header in the last kernel release. > > >>> > > >>> Adding a comment line that describes an implicit or > > >>> explicit license is different than removing the license > > >>> text itself. > > >> > > >> The SPDX license header is meant to be equivalent to the license > > >> text. > > > > > > I understand that. > > > At a minimum, removing BSD license text is undesirable > > > > > > as that license states: > > > ** Redistributions of source code must retain the above copyright > > > * notice, this list of conditions and the following disclaimer. > > > > > > etc... > > > > But this patch only removes the following text: > > > > - * This program is free software; you can redistribute it and/or > > modify > > - * it under the terms of the GNU General Public License version 2 as > > - * published by the Free Software Foundation. > > > > and replaces it by the corresponding SPDX header. > > > > >> The only reason why the large SPDX patch didn't touch the whole > > >> kernel in one go was that it was easier to split in in multiple > > >> chunks. > > > > > > Not really, it was scripted. > > > > But still manually reviewed as far as I know. > > > > >> This is no different than not including the full GPL license in > > >> every header file but only pointing to it through its name and > > >> reference, as every kernel source file does. > > > > > > Not every kernel source file had a license text > > > or a reference to another license file. > > > > Correct, but the files touched by this patch do. > > > > This issue is in no way specific to linux-media and should be > > decided upon at the top level, not on a per-subsystem basis. Greg, > > could you comment on this ? > > >>> > > >>> Comment on what exactly? I don't understand the problem here, care to > > >>> summarize it? > > >> > > >> In a nutshell (if I understand it correctly), Dhaval Shah submitted > > >> https:// patchwork.kernel.org/patch/10102451/ which replaces > > >> > > >> +// SPDX-License-Identifier: GPL-2.0 > > >> [...] > > >> - * > > >> - * This program is free software; you can redistribute it and/or modify > > >> - * it under the terms of the GNU General Public License version 2 as > > >> - * published by the Free Software Foundation. > > >> > > >> in all .c and .h files of the Xilinx V4L2 driver > > >> (drivers/media/platform/ > > >> xilinx). I have reviewed the patch and acked it. Mauro then rejected it, > > >> stating that he can't accept a change to license text without an > > >> explicit ack from the official driver's maintainers. My position is > > >> that such a change doesn't change the license and thus doesn't need to > > >> track all copyright holders, and can be merged without an explicit ack > > >> from the respective maintainers. > > > > > > Yes, I agree with you, no license is being changed here, and no > > > copyright is either. > > > > > > BUT, I know that most major companies are reviewing this process right > > > now. We have gotten approval from almost all of the major kernel > > > developer companies to do this, which is great, and supports this work > > > as being acceptable. > > > > > > So it's nice
Re: [PATCH] em28xx: split up em28xx_dvb_init to reduce stack size
On Thu, Dec 14, 2017 at 6:10 PM, Mauro Carvalho Chehab wrote: > Em Mon, 11 Dec 2017 13:05:02 +0100 > Arnd Bergmann escreveu: > >> With CONFIG_KASAN, the init function uses a large amount of kernel stack: >> >> drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init.part.4': >> drivers/media/usb/em28xx/em28xx-dvb.c:2061:1: error: the frame size of 3232 >> bytes is larger than 2048 bytes [-Werror=frame-larger-than=] >> >> Using gcc-7 with -fsanitize-address-use-after-scope makes this even worse: >> >> drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init': >> drivers/media/usb/em28xx/em28xx-dvb.c:2069:1: error: the frame size of 4280 >> bytes is larger than 3072 bytes [-Werror=frame-larger-than=] >> >> By splitting out each part of the switch/case statement that has its own >> local >> variables into a separate function, no single one of them uses more than 500 >> bytes, >> and with a noinline_for_stack annotation we can ensure that they are not >> merged >> back together. > > The right fix here is really to find a way to simplify the new method > of binding I2C devices by using some ancillary routines. > > I'll keep this patch on my queue for now, but my plan is to try to solve > this issue instead of applying it, maybe on the next weeks (as the > volume of patches reduce due to end of year vacations and Seasons). That's ok, thanks. We have a workaround in linux-mm that partially solves this problem to the point where the stack size goes down to 1600 bytes with KASAN, that by itself would be sufficient to let us enable CONFIG_FRAME_WARN again for 64-bit platforms with the default 2048 byte warning limit. I reposted that patch mostly since I want to lower the frame sizes further so we can reduce the warning limit to 1280 bytes for 64-bit architectures in the future. There around 10 patches needed for that, and they mostly seem to address actual issues, so I'd like them to get addressed eventually and set the limit low enough that the warnings we get on 64-bit are more useful again. However, could you please revisit two other patches: https://patchwork.linuxtv.org/patch/45716/ dvb-frontends: fix i2c access helpers for KASAN https://patchwork.linuxtv.org/patch/45709/ r820t: fix r820t_write_reg for KASAN These are currently the ones I'm most interested in getting merged into v4.15 and LTS kernels for the stack size reduction, since they are blocking the patch that enables CONFIG_FRAME_WARN for allmodconfig. Thanks, Arnd
Re: [PATCH] media: v4l: xilinx: Use SPDX-License-Identifier
Em Fri, 15 Dec 2017 10:55:26 +0530 Dhaval Shah escreveu: > Hi Laurent/Mauro/Greg, > > On Fri, Dec 15, 2017 at 3:32 AM, Laurent Pinchart > wrote: > > Hi Mauro, > > > > On Thursday, 14 December 2017 23:50:03 EET Mauro Carvalho Chehab wrote: > >> Em Thu, 14 Dec 2017 21:57:06 +0100 Greg KH escreveu: > >> > On Thu, Dec 14, 2017 at 10:44:16PM +0200, Laurent Pinchart wrote: > >> >> On Thursday, 14 December 2017 22:08:51 EET Greg KH wrote: > >> >>> On Thu, Dec 14, 2017 at 09:05:27PM +0200, Laurent Pinchart wrote: > >> On Thursday, 14 December 2017 20:54:39 EET Joe Perches wrote: > >> > On Thu, 2017-12-14 at 20:37 +0200, Laurent Pinchart wrote: > >> >> On Thursday, 14 December 2017 20:32:20 EET Joe Perches wrote: > >> >>> On Thu, 2017-12-14 at 20:28 +0200, Laurent Pinchart wrote: > >> On Thursday, 14 December 2017 19:05:27 EET Mauro Carvalho Chehab > >> wrote: > >> > Em Fri, 8 Dec 2017 18:05:37 +0530 Dhaval Shah escreveu: > >> >> SPDX-License-Identifier is used for the Xilinx Video IP and > >> >> related drivers. > >> >> > >> >> Signed-off-by: Dhaval Shah > >> > > >> > Hi Dhaval, > >> > > >> > You're not listed as one of the Xilinx driver maintainers. I'm > >> > afraid that, without their explicit acks, sent to the ML, I > >> > can't accept a patch touching at the driver's license tags. > >> > >> The patch doesn't change the license, I don't see why it would > >> cause any issue. Greg isn't listed as the maintainer or copyright > >> holder of any of the 10k+ files to which he added an SPDX license > >> header in the last kernel release. > >> >>> > >> >>> Adding a comment line that describes an implicit or > >> >>> explicit license is different than removing the license > >> >>> text itself. > >> >> > >> >> The SPDX license header is meant to be equivalent to the license > >> >> text. > >> > > >> > I understand that. > >> > At a minimum, removing BSD license text is undesirable > >> > > >> > as that license states: > >> > ** Redistributions of source code must retain the above > >> > copyright > >> > * notice, this list of conditions and the following disclaimer. > >> > > >> > etc... > >> > >> But this patch only removes the following text: > >> > >> - * This program is free software; you can redistribute it and/or > >> modify > >> - * it under the terms of the GNU General Public License version 2 as > >> - * published by the Free Software Foundation. > >> > >> and replaces it by the corresponding SPDX header. > >> > >> >> The only reason why the large SPDX patch didn't touch the whole > >> >> kernel in one go was that it was easier to split in in multiple > >> >> chunks. > >> > > >> > Not really, it was scripted. > >> > >> But still manually reviewed as far as I know. > >> > >> >> This is no different than not including the full GPL license in > >> >> every header file but only pointing to it through its name and > >> >> reference, as every kernel source file does. > >> > > >> > Not every kernel source file had a license text > >> > or a reference to another license file. > >> > >> Correct, but the files touched by this patch do. > >> > >> This issue is in no way specific to linux-media and should be > >> decided upon at the top level, not on a per-subsystem basis. Greg, > >> could you comment on this ? > >> >>> > >> >>> Comment on what exactly? I don't understand the problem here, care to > >> >>> summarize it? > >> >> > >> >> In a nutshell (if I understand it correctly), Dhaval Shah submitted > >> >> https:// patchwork.kernel.org/patch/10102451/ which replaces > >> >> > >> >> +// SPDX-License-Identifier: GPL-2.0 > >> >> [...] > >> >> - * > >> >> - * This program is free software; you can redistribute it and/or modify > >> >> - * it under the terms of the GNU General Public License version 2 as > >> >> - * published by the Free Software Foundation. > >> >> > >> >> in all .c and .h files of the Xilinx V4L2 driver > >> >> (drivers/media/platform/ > >> >> xilinx). I have reviewed the patch and acked it. Mauro then rejected it, > >> >> stating that he can't accept a change to license text without an > >> >> explicit ack from the official driver's maintainers. My position is > >> >> that such a change doesn't change the license and thus doesn't need to > >> >> track all copyright holders, and can be merged without an explicit ack > >> >> from the respective maintainers. > >> > > >> > Yes, I agree with you, no license is being changed here, and no > >> > copyright is either. > >> > > >> > BUT, I know that most major companies are reviewing this process right > >> > now. We have gotten approval from almost all of the major kernel > >
Re: [RFC PATCH] media: v4l2-device: Link subdevices to their parent devices if available
On Fri, Dec 15, 2017 at 01:32:21PM +0900, Tomasz Figa wrote: > Currently v4l2_device_register_subdev_nodes() does not initialize the > dev_parent field of the video_device structs it creates for subdevices > being registered. This leads to __video_register_device() falling back > to the parent device of associated v4l2_device struct, which often does > not match the physical device the subdevice is registered for. > > Due to the problem above, the links between real devices and v4l-subdev > nodes cannot be obtained from sysfs, which might be confusing for the > userspace trying to identify the hardware. > > Fix this by initializing the dev_parent field of the video_device struct > with the value of dev field of the v4l2_subdev struct. In case of > subdevices without a parent struct device, the field will be NULL and the > old behavior will be preserved by the semantics of > __video_register_device(). > > Signed-off-by: Tomasz Figa Acked-by: Sakari Ailus -- Sakari Ailus sakari.ai...@linux.intel.com
[GIT PULL FOR v4.16, v2] media: imx: Add better OF graph support
Note: the new v4l2-async work makes it possible to simplify this. That will be done in follow-up patches. It's easier to do that if this is in first. This v2 is just a rebased version of v1 to fix merge conflicts. Regards, Hans The following changes since commit 0ca4e3130402caea8731a7b54afde56a6edb17c9: media: pxa_camera: rename the soc_camera_ prefix to pxa_camera_ (2017-12-14 12:40:01 -0500) are available in the Git repository at: git://linuxtv.org/hverkuil/media_tree.git imx for you to fetch changes up to c7db71987c4fdbfcc62cb01f5f88fee25e2d0df0: media: staging/imx: update TODO (2017-12-15 09:46:20 +0100) Steve Longerbeam (9): media: staging/imx: get CSI bus type from nearest upstream entity media: staging/imx: remove static media link arrays media: staging/imx: of: allow for recursing downstream media: staging/imx: remove devname string from imx_media_subdev media: staging/imx: pass fwnode handle to find/add async subdev media: staging/imx: remove static subdev arrays media: staging/imx: convert static vdev lists to list_head media: staging/imx: reorder function prototypes media: staging/imx: update TODO drivers/staging/media/imx/TODO| 63 +++-- drivers/staging/media/imx/imx-ic-prp.c| 4 +- drivers/staging/media/imx/imx-media-capture.c | 2 + drivers/staging/media/imx/imx-media-csi.c | 187 +++ drivers/staging/media/imx/imx-media-dev.c | 401 +- drivers/staging/media/imx/imx-media-internal-sd.c | 253 ++-- drivers/staging/media/imx/imx-media-of.c | 278 drivers/staging/media/imx/imx-media-utils.c | 122 -- drivers/staging/media/imx/imx-media.h | 187 ++- 9 files changed, 721 insertions(+), 776 deletions(-)
[RFC PATCH 0/9] media: base request API support
Here is a new attempt at the request API, following the UAPI we agreed on in Prague. Hopefully this can be used as the basis to move forward. This series only introduces the very basics of how requests work: allocate a request, queue buffers to it, queue the request itself, wait for it to complete, reuse it. It does *not* yet use Hans' work with controls setting. I have preferred to submit it this way for now as it allows us to concentrate on the basic request/buffer flow, which was harder to get properly than I initially thought. I still have a gut feeling that it can be improved, with less back-and- forth into drivers. Plugging in controls support should not be too hard a task (basically just apply the saved controls when the request starts), and I am looking at it now. The resulting vim2m driver can be successfully used with requests, and my tests so far have been successful. There are still some rougher edges: * locking is currently quite coarse-grained * too many #ifdef CONFIG_MEDIA_CONTROLLER in the code, as the request API depends on it - I plan to craft the headers so that it becomes unnecessary. As it is, some of the code will probably not even compile if CONFIG_MEDIA_CONTROLLER is not set But all in all I think the request flow should be clear and easy to review, and the possibility of custom queue and entity support implementations should give us the flexibility we need to support more specific use-cases (I expect the generic implementations to be sufficient most of the time though). A very simple test program exercising this API is available here (don't forget to adapt the /dev/media0 hardcoding): https://gist.github.com/Gnurou/dbc3776ed97ea7d4ce6041ea15eb0438 Looking forward to your feedback and comments! Alexandre Courbot (8): media: add request API core and UAPI media: request: add generic queue media: request: add generic entity ops media: vb2: add support for requests media: vb2: add support for requests in QBUF ioctl media: v4l2-mem2mem: add request support media: vim2m: add media device media: vim2m: add request support Hans Verkuil (1): videodev2.h: Add request field to v4l2_buffer drivers/media/Makefile| 4 +- drivers/media/media-device.c | 6 + drivers/media/media-request-entity-generic.c | 56 drivers/media/media-request-queue-generic.c | 150 ++ drivers/media/media-request.c | 390 ++ drivers/media/platform/vim2m.c| 46 +++ drivers/media/usb/cpia2/cpia2_v4l.c | 2 +- drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 +- drivers/media/v4l2-core/v4l2-ioctl.c | 99 ++- drivers/media/v4l2-core/v4l2-mem2mem.c| 34 +++ drivers/media/v4l2-core/videobuf2-core.c | 59 +++- drivers/media/v4l2-core/videobuf2-v4l2.c | 32 ++- include/media/media-device.h | 3 + include/media/media-entity.h | 6 + include/media/media-request.h | 282 +++ include/media/v4l2-mem2mem.h | 19 ++ include/media/videobuf2-core.h| 25 +- include/media/videobuf2-v4l2.h| 2 + include/uapi/linux/media.h| 11 + include/uapi/linux/videodev2.h| 3 +- 20 files changed, 1216 insertions(+), 20 deletions(-) create mode 100644 drivers/media/media-request-entity-generic.c create mode 100644 drivers/media/media-request-queue-generic.c create mode 100644 drivers/media/media-request.c create mode 100644 include/media/media-request.h -- 2.15.1.504.g5279b80103-goog