cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Aug 1 05:00:10 CEST 2018 media-tree git hash:1d06352e18ef502e30837cedfe618298816fb48c media_build git hash: e75fbc93fc427f769c3ce5a020bf204e02a45852 v4l-utils git hash: 5583f43ef1a4814c2bd3c43cb06461b7f532b141 edid-decode git hash: ab18befbcacd6cd4dff63faa82e32700369d6f25 gcc version:i686-linux-gcc (GCC) 8.1.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.16.0-1-amd64 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-arm64: OK linux-git-i686: WARNINGS linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: WARNINGS Check COMPILE_TEST: OK linux-2.6.36.4-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-i686: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-i686: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-i686: OK linux-2.6.39.4-x86_64: OK linux-3.0.101-i686: OK linux-3.0.101-x86_64: OK linux-3.1.10-i686: OK linux-3.1.10-x86_64: OK linux-3.2.102-i686: OK linux-3.2.102-x86_64: OK linux-3.3.8-i686: OK linux-3.3.8-x86_64: OK linux-3.4.113-i686: OK linux-3.4.113-x86_64: OK linux-3.5.7-i686: OK linux-3.5.7-x86_64: OK linux-3.6.11-i686: OK linux-3.6.11-x86_64: OK linux-3.7.10-i686: OK linux-3.7.10-x86_64: OK linux-3.8.13-i686: OK linux-3.8.13-x86_64: OK linux-3.9.11-i686: OK linux-3.9.11-x86_64: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.57-i686: OK linux-3.16.57-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.115-i686: OK linux-3.18.115-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.52-i686: OK linux-4.1.52-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.140-i686: OK linux-4.4.140-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.112-i686: OK linux-4.9.112-x86_64: OK linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.55-i686: OK linux-4.14.55-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.6-i686: OK linux-4.17.6-x86_64: OK linux-4.18-rc4-i686: OK linux-4.18-rc4-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
RE: [PATCH v2] media: imx208: Add imx208 camera sensor driver
Hi Tomasz, Please check my comments below. >Hi Ping-chung, >On Mon, Jul 30, 2018 at 6:19 PM Ping-chung Chen >wrote: > > From: "Chen, Ping-chung" > > Add a V4L2 sub-device driver for the Sony IMX208 image sensor. > This is a camera sensor using the I2C bus for control and the > CSI-2 bus for data. > >Please see my comments inline. >[snip] > diff --git a/drivers/media/i2c/imx208.c b/drivers/media/i2c/imx208.c > new file mode 100644 index 000..5adfb79 > --- /dev/null > +++ b/drivers/media/i2c/imx208.c > @@ -0,0 +1,984 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (C) 2018 Intel Corporation > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define IMX208_REG_VALUE_08BIT 1 > +#define IMX208_REG_VALUE_16BIT 2 >We don't need to define these. It's obvious that 8 bits is 1 byte and >16 bits are 2 bytes. Done. > + > +#define IMX208_REG_MODE_SELECT 0x0100 > +#define IMX208_MODE_STANDBY0x00 > +#define IMX208_MODE_STREAMING 0x01 >[snip] > +/* Test Pattern Control */ > +#define IMX208_REG_TEST_PATTERN_MODE 0x0600 > +#define IMX208_TEST_PATTERN_DISABLE0 > +#define IMX208_TEST_PATTERN_SOLID_COLOR1 > +#define IMX208_TEST_PATTERN_COLOR_BARS 2 #define > +IMX208_TEST_PATTERN_GREY_COLOR 3 > +#define IMX208_TEST_PATTERN_PN94 >Please use hexadecimal notation for register values (as already done below). Done. > +#define IMX208_TEST_PATTERN_FIX_1 0x100 > +#define IMX208_TEST_PATTERN_FIX_2 0x101 > +#define IMX208_TEST_PATTERN_FIX_3 0x102 > +#define IMX208_TEST_PATTERN_FIX_4 0x103 > +#define IMX208_TEST_PATTERN_FIX_5 0x104 > +#define IMX208_TEST_PATTERN_FIX_6 0x105 >[snip] > +static const int imx208_test_pattern_val[] = { > + IMX208_TEST_PATTERN_DISABLE, > + IMX208_TEST_PATTERN_SOLID_COLOR, > + IMX208_TEST_PATTERN_COLOR_BARS, > + IMX208_TEST_PATTERN_GREY_COLOR, > + IMX208_TEST_PATTERN_PN9, > + IMX208_TEST_PATTERN_FIX_1, > + IMX208_TEST_PATTERN_FIX_2, > + IMX208_TEST_PATTERN_FIX_3, > + IMX208_TEST_PATTERN_FIX_4, > + IMX208_TEST_PATTERN_FIX_5, > + IMX208_TEST_PATTERN_FIX_6, > +}; > + > +/* Configurations for supported link frequencies */ > +#define IMX208_LINK_FREQ_384MHZ38400ULL > +#define IMX208_LINK_FREQ_96MHZ 9600ULL >nit: If we really need defines for these, then at least they should be somehow >useful, e.g. >#define MHZ (1000*1000ULL) >#define IMX208_LINK_FREQ_384MHZ (384ULL * MHZ) >This at least makes it easy to see that there are no mistakes in the number, >e.g. wrong number of zeroes. Sure, done. > + > +enum { > + IMX208_LINK_FREQ_384MHZ_INDEX, > + IMX208_LINK_FREQ_96MHZ_INDEX, > +}; > + > +/* > + * pixel_rate = link_freq * data-rate * nr_of_lanes / bits_per_sample > + * data rate => double data rate; number of lanes => 2; bits per > +pixel => 10 */ static u64 link_freq_to_pixel_rate(u64 f) { > + f *= 2 * 2; > + do_div(f, 10); >Please add macros for those magic numbers. Done. > + > + return f; > +} > + > +/* Menu items for LINK_FREQ V4L2 control */ static const s64 > +link_freq_menu_items[] = { > + IMX208_LINK_FREQ_384MHZ, > + IMX208_LINK_FREQ_96MHZ, >Since we have an enum already, please use it for explicit indices, to ensure >things are consistent and actually easier to read, i.e. >[IMX208_LINK_FREQ_384MHZ_INDEX] = IMX208_LINK_FREQ_384MHZ, Done. > +}; > + > +/* Link frequency configs */ > +static const struct imx208_link_freq_config link_freq_configs[] = { > + { >Explicit indices, i.e. >[IMX208_LINK_FREQ_384MHZ_INDEX] = { Done. > + .pixels_per_line = IMX208_PPL_384MHZ, > + .reg_list = { > + .num_of_regs = ARRAY_SIZE(mipi_data_rate), > + .regs = mipi_data_rate, > + } > + }, > + { >Ditto. > + .pixels_per_line = IMX208_PPL_96MHZ, > + .reg_list = { > + .num_of_regs = ARRAY_SIZE(mipi_data_rate), > + .regs = mipi_data_rate, >How comes that both link frequencies have the same register values for MIPI >data rate? We have renamed it as pll_ctrl_reg. > + } > + }, > +}; > + > +/* Mode configs */ > +static const struct imx208_mode supported_modes[] = { > + { > + .width = 1936, > + .height = 1096, > + .vts_def = IMX208_VTS_60FPS, > + .vts_min = IMX208_VTS_60FPS_MIN, > + .reg_list = { > + .num_of_regs = ARRAY_SIZE(mode_1936x1096_60fps_regs), > + .regs = mode_1936x1096_60fps_regs, > + }, > + .link_freq_index = 0, >Please use the index that was defined before - IMX208_LINK_FREQ_384MHZ_INDEX. Done. > + }, > + { > + .width =
[PATCH v3] media: imx208: Add imx208 camera sensor driver
From: "Chen, Ping-chung" Add a V4L2 sub-device driver for the Sony IMX208 image sensor. This is a camera sensor using the I2C bus for control and the CSI-2 bus for data. Signed-off-by: Ping-Chung Chen --- since v1: -- Update the function media_entity_pads_init for upstreaming. -- Change the structure name mutex as imx208_mx. -- Refine the control flow of test pattern function. -- vflip/hflip control support (will impact the output bayer order) -- support 4 bayer orders output (via change v/hflip) - SRGGB10(default), SGRBG10, SGBRG10, SBGGR10 -- Simplify error handling in the set_stream function. since v2: -- Refine coding style. -- Fix the if statement to use pm_runtime_get_if_in_use(). -- Print more error log during error handling. -- Remove mutex_destroy() from imx208_free_controls(). -- Add more comments. MAINTAINERS| 7 + drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/imx208.c | 995 + 4 files changed, 1014 insertions(+) create mode 100644 drivers/media/i2c/imx208.c diff --git a/MAINTAINERS b/MAINTAINERS index bbd9b9b..896c1df 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13268,6 +13268,13 @@ S: Maintained F: drivers/ssb/ F: include/linux/ssb/ +SONY IMX208 SENSOR DRIVER +M: Sakari Ailus +L: linux-media@vger.kernel.org +T: git git://linuxtv.org/media_tree.git +S: Maintained +F: drivers/media/i2c/imx208.c + SONY IMX258 SENSOR DRIVER M: Sakari Ailus L: linux-media@vger.kernel.org diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 8669853..ae11f1e 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -595,6 +595,17 @@ config VIDEO_APTINA_PLL config VIDEO_SMIAPP_PLL tristate +config VIDEO_IMX208 + tristate "Sony IMX208 sensor support" + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API + depends on MEDIA_CAMERA_SUPPORT + ---help--- + This is a Video4Linux2 sensor driver for the Sony + IMX208 camera. + + To compile this driver as a module, choose M here: the + module will be called imx208. + config VIDEO_IMX258 tristate "Sony IMX258 sensor support" depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 837c428..47604c2 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -104,6 +104,7 @@ obj-$(CONFIG_VIDEO_I2C) += video-i2c.o obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o obj-$(CONFIG_VIDEO_OV2659) += ov2659.o obj-$(CONFIG_VIDEO_TC358743) += tc358743.o +obj-$(CONFIG_VIDEO_IMX208) += imx208.o obj-$(CONFIG_VIDEO_IMX258) += imx258.o obj-$(CONFIG_VIDEO_IMX274) += imx274.o diff --git a/drivers/media/i2c/imx208.c b/drivers/media/i2c/imx208.c new file mode 100644 index 000..e997193 --- /dev/null +++ b/drivers/media/i2c/imx208.c @@ -0,0 +1,995 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include + +#define IMX208_REG_MODE_SELECT 0x0100 +#define IMX208_MODE_STANDBY0x00 +#define IMX208_MODE_STREAMING 0x01 + +/* Chip ID */ +#define IMX208_REG_CHIP_ID 0x +#define IMX208_CHIP_ID 0x0208 + +/* V_TIMING internal */ +#define IMX208_REG_VTS 0x0340 +#define IMX208_VTS_60FPS 0x0472 +#define IMX208_VTS_BINNING 0x0239 +#define IMX208_VTS_60FPS_MIN 0x0458 +#define IMX208_VTS_BINNING_MIN 0x0230 +#define IMX208_VTS_MAX 0x + +/* HBLANK control - read only */ +#define IMX208_PPL_384MHZ 2248 +#define IMX208_PPL_96MHZ 2248 + +/* Exposure control */ +#define IMX208_REG_EXPOSURE0x0202 +#define IMX208_EXPOSURE_MIN4 +#define IMX208_EXPOSURE_STEP 1 +#define IMX208_EXPOSURE_DEFAULT0x190 +#define IMX208_EXPOSURE_MAX65535 + +/* Analog gain control */ +#define IMX208_REG_ANALOG_GAIN 0x0204 +#define IMX208_ANA_GAIN_MIN0 +#define IMX208_ANA_GAIN_MAX0x00e0 +#define IMX208_ANA_GAIN_STEP 1 +#define IMX208_ANA_GAIN_DEFAULT0x0 + +/* Digital gain control */ +#define IMX208_REG_GR_DIGITAL_GAIN 0x020e +#define IMX208_REG_R_DIGITAL_GAIN 0x0210 +#define IMX208_REG_B_DIGITAL_GAIN 0x0212 +#define IMX208_REG_GB_DIGITAL_GAIN 0x0214 +#define IMX208_DGTL_GAIN_MIN 0 +#define IMX208_DGTL_GAIN_MAX 4096 +#define IMX208_DGTL_GAIN_DEFAULT 0x100 +#define IMX208_DGTL_GAIN_STEP 1 + +/* Orientation */ +#define IMX208_REG_ORIENTATION_CONTROL 0x0101 + +/* Test Pattern Control */ +#define IMX208_REG_TEST_PATTERN_MODE 0x0600 +#define IMX208_TEST_PATTERN_DISABLE0x0 +#define
Re: [PATCH 05/21] dt-bindings: media: Specify bus type for MIPI D-PHY, others, explicitly
On Mon, Jul 23, 2018 at 04:46:50PM +0300, Sakari Ailus wrote: > Allow specifying the bus type explicitly for MIPI D-PHY, parallel and > Bt.656 busses. This is useful for devices that can make use of different > bus types. There are CSI-2 transmitters and receivers but the PHY > selection needs to be made between C-PHY and D-PHY; many devices also > support parallel and Bt.656 interfaces but the means to pass that > information to software wasn't there. > > Autodetection (value 0) is removed as an option as the property could be > simply omitted in that case. Presumably there are users, so you can't remove it. But documenting behavior when absent would be good. > > Signed-off-by: Sakari Ailus > --- > Documentation/devicetree/bindings/media/video-interfaces.txt | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt > b/Documentation/devicetree/bindings/media/video-interfaces.txt > index baf9d9756b3c..f884ada0bffc 100644 > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > @@ -100,10 +100,12 @@ Optional endpoint properties >slave device (data source) by the master device (data sink). In the master >mode the data source device is also the source of the synchronization > signals. > - bus-type: data bus type. Possible values are: > - 0 - autodetect based on other properties (MIPI CSI-2 D-PHY, parallel or > Bt656) >1 - MIPI CSI-2 C-PHY >2 - MIPI CSI1 >3 - CCP2 > + 4 - MIPI CSI-2 D-PHY > + 5 - Parallel Is that really specific enough to be useful? > + 6 - Bt.656 > - bus-width: number of data lines actively used, valid for the parallel > busses. > - data-shift: on the parallel data busses, if bus-width is used to specify > the >number of data lines, data-shift can be used to specify which data lines > are > -- > 2.11.0 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
web and mobile app development
Hello, Are you in search of a suitable app development agency to nurture and transform your creative app idea into reality? Well, your search ends here. We are based in India. We are in the mobile app development domain for last seven years and with a team of 120+ web and mobile app development professionals, we have served the global clientele. Talking about the mobile apps, we have built over 350 mobile apps ranging from a simple business app to complex enterprise app to date. Our expertise in iOS, Android, Ionic, and PhoneGap-based platforms and various libraries can help you reach a huge smartphone-using audience with the following services: • Mobile App Consulting • Mobile App UI/UX Designing • Mobile App Development • Mobile App Maintenance & Support • Website and eCommerce portal Development We render our enterprise-grade app development services across different industry sectors including retail, games and entertainment, lifestyle, social media, healthcare and hospitality, BFSI, eCommerce (m-commerce), food and beverages, and the like. We will gladly share the references of our work on your demand. Here is how you can start expanding your business online: 1. Respond to this email 2. We will provide an Action Plan and Quote 3. Get your Mobile App Development started That's it for now! Thanks for your time. Awaiting your reply, Regards, Vivek Shah
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Em Tue, 31 Jul 2018 15:30:56 +0200 Marco Felsch escreveu: > Hi Javier, > > On 18-07-31 14:52, Javier Martinez Canillas wrote: > > Hi Marco, > > > > On 07/31/2018 02:36 PM, Marco Felsch wrote: > > > > [snip] > > > > >> > > >> Yes, another thing that patch 19/22 should take into account is DTs that > > >> don't have input connectors defined. So probably TVP5150_PORT_YOUT should > > >> be 0 instead of TVP5150_PORT_NUM - 1 as is the case in the current patch. > > >> > > >> In other words, it should work both when input connectors are defined in > > >> the DT and when these are not defined and only an output port is > > >> defined. > > > > > > Yes, it would be a approach to map the output port dynamicaly to the > > > highest port number. I tried to keep things easy by a static mapping. > > > Maybe a follow up patch can change this behaviour. > > > > > > Anyway, input connectors aren't required. There must be at least one > > > port child node with a correct port-number in the DT. > > > > > > > Yes, that was my point. But your patch uses the port child reg property as > > the index for the struct device_node *endpoints[TVP5150_PORT_NUM] array. > > > > If there's only one port child (for the output) then the DT binding says > > that the reg property isn't required, so this will be 0 and your patch will > > wrongly map it to TVP5150_PORT_AIP1A. That's why I said that the output port > > should be the first one in your enum tvp5150_ports and not the last one. > > Yes, now I got you. I implemted this in such a way in my first apporach. > But at the moment I don't know why I changed this. Maybe to keep the > decoder->input number in sync with the em28xx devices, which will set the > port by the s_routing() callback. > > Let me check this. Anyway, with the patchset I sent (with one fix), it will do the right thing with regards to the pad output: https://git.linuxtv.org/mchehab/experimental.git/log/?h=tvp5150 $ mc_nextgen_test -D digraph board { rankdir=TB colorscheme=x11 labelloc="t" label="Grabster AV 350 driver:em28xx, bus: usb-:00:14.0-2 " intf_devnode_7 [label="intf_devnode_7\nvideo\n/dev/video0", shape=box, style=filled, fillcolor=yellow] intf_devnode_10 [label="intf_devnode_10\nvbi\n/dev/vbi0", shape=box, style=filled, fillcolor=yellow] entity_1 [label="{{ 0 | 1 | 2} | entity_1\nATV decoder\ntvp5150 0-005c | { 3}}", shape=Mrecord, style=filled, fillcolor=lightblue] entity_6 [label="{{ 0} | entity_6\nV4L I/O\n2-2:1.0 video}", shape=Mrecord, style=filled, fillcolor=aquamarine] entity_9 [label="{{ 0} | entity_9\nVBI I/O\n2-2:1.0 vbi}", shape=Mrecord, style=filled, fillcolor=aquamarine] entity_14 [label="{entity_14\nunknown entity type\nComposite | { 0}}", shape=Mrecord, style=filled, fillcolor=cadetblue] entity_16 [label="{entity_16\nunknown entity type\nS-Video | { 0}}", shape=Mrecord, style=filled, fillcolor=cadetblue] intf_devnode_7 -> entity_6 [dir="none" color="orange"] intf_devnode_10 -> entity_9 [dir="none" color="orange"] entity_1:pad_5 -> entity_6:pad_12 [color=blue] entity_1:pad_5 -> entity_9:pad_13 [color=blue] entity_14:pad_15 -> entity_1:pad_2 [color=blue] entity_16:pad_17 -> entity_1:pad_2 [color=blue style="dashed"] } It won't do the right thing with regards to the input, though, as the code at v4l2-mc.c expects just one input. So, both composite and S-Video connectors (created outside tvp5150, based on the input entries at em28xx cards table) are linked to pad 0. Thanks, Mauro
web and mobile app development
Hello, Are you in search of a suitable app development agency to nurture and transform your creative app idea into reality? Well, your search ends here. We are based in India. We are in the mobile app development domain for last seven years and with a team of 120+ web and mobile app development professionals, we have served the global clientele. Talking about the mobile apps, we have built over 350 mobile apps ranging from a simple business app to complex enterprise app to date. Our expertise in iOS, Android, Ionic, and PhoneGap-based platforms and various libraries can help you reach a huge smartphone-using audience with the following services: • Mobile App Consulting • Mobile App UI/UX Designing • Mobile App Development • Mobile App Maintenance & Support • Website and eCommerce portal Development We render our enterprise-grade app development services across different industry sectors including retail, games and entertainment, lifestyle, social media, healthcare and hospitality, BFSI, eCommerce (m-commerce), food and beverages, and the like. We will gladly share the references of our work on your demand. Here is how you can start expanding your business online: 1. Respond to this email 2. We will provide an Action Plan and Quote 3. Get your Mobile App Development started That's it for now! Thanks for your time. Awaiting your reply, Regards, Vivek Shah
v4l2_spi_subdev_init vs v4l2_i2c_subdev_init
Hello v4l2 gurus, Documentation/media/kapi/v4l2-subdev.rst states : "Afterwards you need to initialize :c:type:`sd `->name with a unique name and set the module owner. This is done for you if you use the i2c helper functions" I try to write a v4l2 spi driver and use hence v4l2_spi_subdev_init, not v4l2_i2c_subdev_init. In v4l2_i2c_subdev_init, subdev name is initialised by snprintf(sd->name, sizeof(sd->name), "%s %d-%04x", client->dev.driver->name, i2c_adapter_id(client->adapter), client->addr); In v4l2_spi_subdev_init, subdev name is initialised by strlcpy(sd->name, spi->dev.driver->name, sizeof(sd->name)); This does not give similar results :( with i2c, subdev name is set as "xxx %d-%04x", giving a unique name to the subdev. with spi, subdev name is set as "xxx", giving the same name to all similar subdevs on the same host Is that intentional or an oversight, and if so, how should that be fixed ? Best regards Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
[bug report] media: i2c: Add driver for Aptina MT9V111
Hello Jacopo Mondi, The patch aab7ed1c3927: "media: i2c: Add driver for Aptina MT9V111" from Jul 25, 2018, leads to the following static checker warning: drivers/media/i2c/mt9v111.c:1163 mt9v111_probe() warn: passing zero to 'PTR_ERR' drivers/media/i2c/mt9v111.c:1173 mt9v111_probe() warn: passing zero to 'PTR_ERR' drivers/media/i2c/mt9v111.c:1184 mt9v111_probe() warn: passing zero to 'PTR_ERR' drivers/media/i2c/mt9v111.c:1194 mt9v111_probe() warn: passing zero to 'PTR_ERR' drivers/media/i2c/mt9v111.c 1155 v4l2_ctrl_handler_init(>ctrls, 5); 1156 1157 mt9v111->auto_awb = v4l2_ctrl_new_std(>ctrls, 1158_ctrl_ops, 1159 V4L2_CID_AUTO_WHITE_BALANCE, 11600, 1, 1, 1161V4L2_WHITE_BALANCE_AUTO); 1162 if (IS_ERR_OR_NULL(mt9v111->auto_awb)) { 1163 ret = PTR_ERR(mt9v111->auto_awb); This just returns success because v4l2_ctrl_new_std() only return NULL on error, it never returns error pointers. I guess we should set ret to EINVAL? if (!mt9v111->auto_awb) { ret = -EINVAL; goto error_free_ctrls; } 1164 goto error_free_ctrls; 1165 } 1166 1167 mt9v111->auto_exp = v4l2_ctrl_new_std_menu(>ctrls, 1168 _ctrl_ops, 1169 V4L2_CID_EXPOSURE_AUTO, 1170 V4L2_EXPOSURE_MANUAL, 1171 0, V4L2_EXPOSURE_AUTO); 1172 if (IS_ERR_OR_NULL(mt9v111->auto_exp)) { 1173 ret = PTR_ERR(mt9v111->auto_exp); 1174 goto error_free_ctrls; 1175 } 1176 1177 /* Initialize timings */ 1178 mt9v111->hblank = v4l2_ctrl_new_std(>ctrls, _ctrl_ops, 1179 V4L2_CID_HBLANK, 1180 MT9V111_CORE_R05_MIN_HBLANK, 1181 MT9V111_CORE_R05_MAX_HBLANK, 1, 1182 MT9V111_CORE_R05_DEF_HBLANK); 1183 if (IS_ERR_OR_NULL(mt9v111->hblank)) { 1184 ret = PTR_ERR(mt9v111->hblank); 1185 goto error_free_ctrls; 1186 } 1187 1188 mt9v111->vblank = v4l2_ctrl_new_std(>ctrls, _ctrl_ops, 1189 V4L2_CID_VBLANK, 1190 MT9V111_CORE_R06_MIN_VBLANK, 1191 MT9V111_CORE_R06_MAX_VBLANK, 1, 1192 MT9V111_CORE_R06_DEF_VBLANK); 1193 if (IS_ERR_OR_NULL(mt9v111->vblank)) { 1194 ret = PTR_ERR(mt9v111->vblank); 1195 goto error_free_ctrls; 1196 } 1197 1198 /* PIXEL_RATE is fixed: just expose it to user space. */ 1199 v4l2_ctrl_new_std(>ctrls, _ctrl_ops, regards, dan carpenter
[PATCH RFC 2/4] media: v4l2: taint pads with the signal types for consumer devices
Consumer devices are provided with a wide diferent range of types supported by the same driver, allowing different configutations. In order to make easier to setup media controller links, "taint" pads with the signal type it carries. While here, get rid of DEMOD_PAD_VBI_OUT, as the signal it carries is actually the same as the normal video output. The difference happens at the video/VBI interface: - for VBI, only the hidden lines are streamed; - for video, the stream is usually cropped to hide the vbi lines. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/dvb-frontends/au8522_decoder.c | 3 ++ drivers/media/i2c/msp3400-driver.c | 2 ++ drivers/media/i2c/saa7115.c | 2 ++ drivers/media/i2c/tvp5150.c | 2 ++ drivers/media/pci/saa7134/saa7134-core.c | 2 ++ drivers/media/tuners/si2157.c| 3 ++ drivers/media/usb/dvb-usb-v2/mxl111sf.c | 2 ++ drivers/media/v4l2-core/tuner-core.c | 5 +++ include/media/media-entity.h | 33 9 files changed, 54 insertions(+) diff --git a/drivers/media/dvb-frontends/au8522_decoder.c b/drivers/media/dvb-frontends/au8522_decoder.c index 30cd2bd7aec2..f4df9ab3d8b0 100644 --- a/drivers/media/dvb-frontends/au8522_decoder.c +++ b/drivers/media/dvb-frontends/au8522_decoder.c @@ -721,8 +721,11 @@ static int au8522_probe(struct i2c_client *client, #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + state->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_RF; state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; + state->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_ATV_VIDEO; state->pads[DEMOD_PAD_AUDIO_OUT].flags = MEDIA_PAD_FL_SOURCE; + state->pads[DEMOD_PAD_AUDIO_OUT].sig_type = PAD_SIGNAL_AUDIO; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; ret = media_entity_pads_init(>entity, ARRAY_SIZE(state->pads), diff --git a/drivers/media/i2c/msp3400-driver.c b/drivers/media/i2c/msp3400-driver.c index 3db966db83eb..3b9c729fbd52 100644 --- a/drivers/media/i2c/msp3400-driver.c +++ b/drivers/media/i2c/msp3400-driver.c @@ -704,7 +704,9 @@ static int msp_probe(struct i2c_client *client, const struct i2c_device_id *id) #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[IF_AUD_DEC_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + state->pads[IF_AUD_DEC_PAD_IF_INPUT].sig_type = PAD_SIGNAL_AUDIO; state->pads[IF_AUD_DEC_PAD_OUT].flags = MEDIA_PAD_FL_SOURCE; + state->pads[IF_AUD_DEC_PAD_OUT].sig_type = PAD_SIGNAL_AUDIO; sd->entity.function = MEDIA_ENT_F_IF_AUD_DECODER; diff --git a/drivers/media/i2c/saa7115.c b/drivers/media/i2c/saa7115.c index 4c72db58dfd2..0b298aa34a7c 100644 --- a/drivers/media/i2c/saa7115.c +++ b/drivers/media/i2c/saa7115.c @@ -1835,7 +1835,9 @@ static int saa711x_probe(struct i2c_client *client, #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + state->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_RF; state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; + state->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_ATV_VIDEO; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c index 1414c2c14639..dab83a774e73 100644 --- a/drivers/media/i2c/tvp5150.c +++ b/drivers/media/i2c/tvp5150.c @@ -1495,7 +1495,9 @@ static int tvp5150_probe(struct i2c_client *c, #if defined(CONFIG_MEDIA_CONTROLLER) core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + core->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_RF; core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; + core->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_ATV_VIDEO; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; diff --git a/drivers/media/pci/saa7134/saa7134-core.c b/drivers/media/pci/saa7134/saa7134-core.c index 267d143c3a48..322e2ac00066 100644 --- a/drivers/media/pci/saa7134/saa7134-core.c +++ b/drivers/media/pci/saa7134/saa7134-core.c @@ -846,7 +846,9 @@ static void saa7134_create_entities(struct saa7134_dev *dev) if (!decoder) { dev->demod.name = "saa713x"; dev->demod_pad[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + dev->demod_pad[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_RF; dev->demod_pad[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; + dev->demod_pad[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_ATV_VIDEO; dev->demod.function = MEDIA_ENT_F_ATV_DECODER; ret = media_entity_pads_init(>demod, DEMOD_NUM_PADS, diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c index 9e34d31d724d..85e9ea9059a3 100644 --- a/drivers/media/tuners/si2157.c +++ b/drivers/media/tuners/si2157.c @@ -469,8
[PATCH RFC 0/4] Better handle pads for tuning/decoder part of the devices
At PC consumer devices, it is very common that the bridge same driver to be attached to different types of tuners and demods. We need a way for the Kernel to properly identify what kind of signal is provided by each PAD, in order to properly setup the pipelines. The previous approach were to hardcode a fixed number of PADs for all elements of the same type. This is not good, as different devices may actually have a different number of pads. It was acceptable in the past, as there were a promisse of adding "soon" a properties API that would allow to identify the type for each PADs, but this was never merged (or even a patchset got submitted). So, replace this approach by another one: add a "taint" mark to pads that contain different types of signals. I tried to minimize the number of signals, in order to make it simpler to convert from the past way. However, I'm not inspired today to give names to the signals each pad contain. So, feel free to give better suggestions if this one doesn't fit too well. For now, this is just a RFC, compile-tested only, as the main goal here is to discuss about an approach. Once I get enough feedback, I'll do some tests. Mauro Carvalho Chehab (4): media: v4l2: remove VBI output pad media: v4l2: taint pads with the signal types for consumer devices v4l2-mc: switch it to use the new approach to setup pipelines media: dvb: use signals to discover pads drivers/media/dvb-core/dvbdev.c | 37 - drivers/media/dvb-frontends/au8522_decoder.c | 4 +- drivers/media/i2c/msp3400-driver.c | 2 + drivers/media/i2c/saa7115.c | 3 +- drivers/media/i2c/tvp5150.c | 3 +- drivers/media/pci/saa7134/saa7134-core.c | 3 +- drivers/media/tuners/si2157.c| 3 + drivers/media/usb/dvb-usb-v2/mxl111sf.c | 2 + drivers/media/v4l2-core/tuner-core.c | 5 ++ drivers/media/v4l2-core/v4l2-mc.c| 87 include/media/media-entity.h | 33 include/media/v4l2-mc.h | 2 - 12 files changed, 158 insertions(+), 26 deletions(-) -- 2.17.1
[PATCH RFC 4/4] media: dvb: use signals to discover pads
On tuner pads, multiple signals are present. Be sure to get the right PAD by using them. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/dvb-core/dvbdev.c | 37 + 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c index 787fe06df217..5c39bd328835 100644 --- a/drivers/media/dvb-core/dvbdev.c +++ b/drivers/media/dvb-core/dvbdev.c @@ -607,6 +607,28 @@ static int dvb_create_io_intf_links(struct dvb_adapter *adap, return 0; } +static int get_pad_index(struct media_entity *entity, bool is_sink, +enum media_pad_signal_type sig_type) +{ + int i; + bool pad_is_sink; + + for (i = 0; i < entity->num_pads; i++) { + if (entity->pads[i].flags == MEDIA_PAD_FL_SINK) + pad_is_sink = true; + else if (entity->pads[i].flags == MEDIA_PAD_FL_SOURCE) + pad_is_sink = false; + else + continue; /* This is an error! */ + + if (pad_is_sink != is_sink) + continue; + if (entity->pads[i].sig_type == sig_type) + return i; + } + return -EINVAL; +} + int dvb_create_media_graph(struct dvb_adapter *adap, bool create_rf_connector) { @@ -618,7 +640,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap, unsigned demux_pad = 0; unsigned dvr_pad = 0; unsigned ntuner = 0, ndemod = 0; - int ret; + int ret, pad_source, pad_sink; static const char *connector_name = "Television"; if (!mdev) @@ -678,7 +700,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap, if (ret) return ret; - if (!ntuner) + if (!ntuner) { ret = media_create_pad_links(mdev, MEDIA_ENT_F_CONN_RF, conn, 0, @@ -686,19 +708,26 @@ int dvb_create_media_graph(struct dvb_adapter *adap, demod, 0, MEDIA_LNK_FL_ENABLED, false); - else + } else { + pad_sink = get_pad_index(tuner, true, PAD_SIGNAL_RF); + if (pad_sink < 0) + return -EINVAL; ret = media_create_pad_links(mdev, MEDIA_ENT_F_CONN_RF, conn, 0, MEDIA_ENT_F_TUNER, -tuner, TUNER_PAD_RF_INPUT, +tuner, pad_sink, MEDIA_LNK_FL_ENABLED, false); + } if (ret) return ret; } if (ntuner && ndemod) { + pad_source = get_pad_index(tuner, true, PAD_SIGNAL_RF); + if (pad_source) + return -EINVAL; ret = media_create_pad_links(mdev, MEDIA_ENT_F_TUNER, tuner, TUNER_PAD_OUTPUT, -- 2.17.1
[PATCH RFC 3/4] v4l2-mc: switch it to use the new approach to setup pipelines
Instead of relying on a static map for pids, use the new sig_type "taint" type to setup the pipelines with the same tipe between different entities. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/v4l2-core/v4l2-mc.c | 87 +-- 1 file changed, 71 insertions(+), 16 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c index 982bab3530f6..8640f656f9ae 100644 --- a/drivers/media/v4l2-core/v4l2-mc.c +++ b/drivers/media/v4l2-core/v4l2-mc.c @@ -19,6 +19,28 @@ #include #include +static int get_pad_index(struct media_entity *entity, bool is_sink, +enum media_pad_signal_type sig_type) +{ + int i; + bool pad_is_sink; + + for (i = 0; i < entity->num_pads; i++) { + if (entity->pads[i].flags == MEDIA_PAD_FL_SINK) + pad_is_sink = true; + else if (entity->pads[i].flags == MEDIA_PAD_FL_SOURCE) + pad_is_sink = false; + else + continue; /* This is an error! */ + + if (pad_is_sink != is_sink) + continue; + if (entity->pads[i].sig_type == sig_type) + return i; + } + return -EINVAL; +} + int v4l2_mc_create_media_graph(struct media_device *mdev) { @@ -28,7 +50,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev) struct media_entity *io_v4l = NULL, *io_vbi = NULL, *io_swradio = NULL; bool is_webcam = false; u32 flags; - int ret; + int ret, pad_sink, pad_source; if (!mdev) return 0; @@ -97,29 +119,48 @@ int v4l2_mc_create_media_graph(struct media_device *mdev) /* Link the tuner and IF video output pads */ if (tuner) { if (if_vid) { - ret = media_create_pad_link(tuner, TUNER_PAD_OUTPUT, - if_vid, - IF_VID_DEC_PAD_IF_INPUT, + pad_source = get_pad_index(tuner, false, PAD_SIGNAL_RF); + pad_sink = get_pad_index(if_vid, true, PAD_SIGNAL_RF); + if (pad_source < 0 || pad_sink < 0) + return -EINVAL; + ret = media_create_pad_link(tuner, pad_source, + if_vid, pad_sink, MEDIA_LNK_FL_ENABLED); if (ret) return ret; - ret = media_create_pad_link(if_vid, IF_VID_DEC_PAD_OUT, - decoder, DEMOD_PAD_IF_INPUT, + + pad_source = get_pad_index(if_vid, false, + PAD_SIGNAL_ATV_VIDEO); + pad_sink = get_pad_index(decoder, true, +PAD_SIGNAL_ATV_VIDEO); + if (pad_source < 0 || pad_sink < 0) + return -EINVAL; + ret = media_create_pad_link(if_vid, pad_source, + decoder, pad_sink, MEDIA_LNK_FL_ENABLED); if (ret) return ret; } else { - ret = media_create_pad_link(tuner, TUNER_PAD_OUTPUT, - decoder, DEMOD_PAD_IF_INPUT, + pad_source = get_pad_index(tuner, false, PAD_SIGNAL_RF); + pad_sink = get_pad_index(decoder, true, PAD_SIGNAL_RF); + if (pad_source < 0 || pad_sink < 0) + return -EINVAL; + ret = media_create_pad_link(tuner, pad_source, + decoder, pad_sink, MEDIA_LNK_FL_ENABLED); if (ret) return ret; } if (if_aud) { - ret = media_create_pad_link(tuner, TUNER_PAD_AUD_OUT, - if_aud, - IF_AUD_DEC_PAD_IF_INPUT, + pad_source = get_pad_index(tuner, false, + PAD_SIGNAL_AUDIO); + pad_sink = get_pad_index(decoder, true, +PAD_SIGNAL_AUDIO); + if (pad_source < 0 || pad_sink < 0) + return -EINVAL; + ret = media_create_pad_link(tuner, pad_source, + if_aud, pad_sink,
[PATCH RFC 1/4] media: v4l2: remove VBI output pad
The signal there is the same as the video output (well, except for sliced VBI, but let's simplify the model and ignore it, at least for now - as it is routed together with raw VBI). Signed-off-by: Mauro Carvalho Chehab --- drivers/media/dvb-frontends/au8522_decoder.c | 1 - drivers/media/i2c/saa7115.c | 1 - drivers/media/i2c/tvp5150.c | 1 - drivers/media/pci/saa7134/saa7134-core.c | 1 - drivers/media/v4l2-core/v4l2-mc.c| 2 +- include/media/v4l2-mc.h | 2 -- 6 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/media/dvb-frontends/au8522_decoder.c b/drivers/media/dvb-frontends/au8522_decoder.c index 343dc92ef54e..30cd2bd7aec2 100644 --- a/drivers/media/dvb-frontends/au8522_decoder.c +++ b/drivers/media/dvb-frontends/au8522_decoder.c @@ -722,7 +722,6 @@ static int au8522_probe(struct i2c_client *client, state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; - state->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE; state->pads[DEMOD_PAD_AUDIO_OUT].flags = MEDIA_PAD_FL_SOURCE; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; diff --git a/drivers/media/i2c/saa7115.c b/drivers/media/i2c/saa7115.c index b07114b5efb2..4c72db58dfd2 100644 --- a/drivers/media/i2c/saa7115.c +++ b/drivers/media/i2c/saa7115.c @@ -1836,7 +1836,6 @@ static int saa711x_probe(struct i2c_client *client, #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; - state->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c index 1734ed4ede33..1414c2c14639 100644 --- a/drivers/media/i2c/tvp5150.c +++ b/drivers/media/i2c/tvp5150.c @@ -1496,7 +1496,6 @@ static int tvp5150_probe(struct i2c_client *c, #if defined(CONFIG_MEDIA_CONTROLLER) core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; - core->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; diff --git a/drivers/media/pci/saa7134/saa7134-core.c b/drivers/media/pci/saa7134/saa7134-core.c index 9e76de2411ae..267d143c3a48 100644 --- a/drivers/media/pci/saa7134/saa7134-core.c +++ b/drivers/media/pci/saa7134/saa7134-core.c @@ -847,7 +847,6 @@ static void saa7134_create_entities(struct saa7134_dev *dev) dev->demod.name = "saa713x"; dev->demod_pad[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; dev->demod_pad[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; - dev->demod_pad[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE; dev->demod.function = MEDIA_ENT_F_ATV_DECODER; ret = media_entity_pads_init(>demod, DEMOD_NUM_PADS, diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c index 0fc185a2ce90..982bab3530f6 100644 --- a/drivers/media/v4l2-core/v4l2-mc.c +++ b/drivers/media/v4l2-core/v4l2-mc.c @@ -147,7 +147,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev) } if (io_vbi) { - ret = media_create_pad_link(decoder, DEMOD_PAD_VBI_OUT, + ret = media_create_pad_link(decoder, DEMOD_PAD_VID_OUT, io_vbi, 0, MEDIA_LNK_FL_ENABLED); if (ret) diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h index 2634d9dc9916..7c9c781b16a9 100644 --- a/include/media/v4l2-mc.h +++ b/include/media/v4l2-mc.h @@ -89,14 +89,12 @@ enum if_aud_dec_pad_index { * * @DEMOD_PAD_IF_INPUT:IF input sink pad. * @DEMOD_PAD_VID_OUT: Video output source pad. - * @DEMOD_PAD_VBI_OUT: Vertical Blank Interface (VBI) output source pad. * @DEMOD_PAD_AUDIO_OUT: Audio output source pad. * @DEMOD_NUM_PADS:Maximum number of output pads. */ enum demod_pad_index { DEMOD_PAD_IF_INPUT, DEMOD_PAD_VID_OUT, - DEMOD_PAD_VBI_OUT, DEMOD_PAD_AUDIO_OUT, DEMOD_NUM_PADS }; -- 2.17.1
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Hi Javier, On 18-07-31 14:52, Javier Martinez Canillas wrote: > Hi Marco, > > On 07/31/2018 02:36 PM, Marco Felsch wrote: > > [snip] > > >> > >> Yes, another thing that patch 19/22 should take into account is DTs that > >> don't have input connectors defined. So probably TVP5150_PORT_YOUT should > >> be 0 instead of TVP5150_PORT_NUM - 1 as is the case in the current patch. > >> > >> In other words, it should work both when input connectors are defined in > >> the DT and when these are not defined and only an output port is defined. > > > > Yes, it would be a approach to map the output port dynamicaly to the > > highest port number. I tried to keep things easy by a static mapping. > > Maybe a follow up patch can change this behaviour. > > > > Anyway, input connectors aren't required. There must be at least one > > port child node with a correct port-number in the DT. > > > > Yes, that was my point. But your patch uses the port child reg property as > the index for the struct device_node *endpoints[TVP5150_PORT_NUM] array. > > If there's only one port child (for the output) then the DT binding says > that the reg property isn't required, so this will be 0 and your patch will > wrongly map it to TVP5150_PORT_AIP1A. That's why I said that the output port > should be the first one in your enum tvp5150_ports and not the last one. Yes, now I got you. I implemted this in such a way in my first apporach. But at the moment I don't know why I changed this. Maybe to keep the decoder->input number in sync with the em28xx devices, which will set the port by the s_routing() callback. Let me check this. Best Regards, Marco > > Regards, > > Marco > > > > Best regards,
Need mobile apps development?
Hello, Are you in search of a suitable app development agency to nurture and transform your creative app idea into reality? Well, your search ends here. We are based in India. We are in the mobile app development domain for last seven years and with a team of 120+ web and mobile app development professionals, we have served the global clientele. Talking about the mobile apps, we have built over 350 mobile apps ranging from a simple business app to complex enterprise app to date. Our expertise in iOS, Android, Ionic, and PhoneGap-based platforms and various libraries can help you reach a huge smartphone-using audience with the following services: • Mobile App Consulting • Mobile App UI/UX Designing • Mobile App Development • Mobile App Maintenance & Support • Website and eCommerce portal Development We render our enterprise-grade app development services across different industry sectors including retail, games and entertainment, lifestyle, social media, healthcare and hospitality, BFSI, eCommerce (m-commerce), food and beverages, and the like. We will gladly share the references of our work on your demand. Here is how you can start expanding your business online: 1. Respond to this email 2. We will provide an Action Plan and Quote 3. Get your Mobile App Development started That's it for now! Thanks for your time. Awaiting your reply, Regards, Vivek Shah
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Em Tue, 31 Jul 2018 07:06:59 -0300 Mauro Carvalho Chehab escreveu: > Em Tue, 31 Jul 2018 10:52:56 +0200 > Javier Martinez Canillas escreveu: > > The graph is not built correct, as it is linking tvp5150's input pads as > if they were output ones. > > The problem is that now you need to teach drivers/media/v4l2-core/v4l2-mc.c > to do the proper wiring for tvp5150. > > I suspect that fixing v4l2-mc for doing that is not hard, but it may > require changes at the other demods. Thankfully there aren't many > demod drivers, but such patch should be applied before patch 19/22. > > In the specific case of demods that don't support sliced VBI (or > where sliced VBI is not coded), there should be just one source pad. > > On demods with sliced VBI, there are actually two source pads, > although, for simplicity, maybe we could map them as just one. > > If we map as just one source pad, it is probably easy to change the > code at v4l2-mc to do the right thing. > > I'll do some tests here and try to code it. Ok, did some coding. The way to make it more robust and allow having a different number of PADs for different demods/tuners is with an approach like the one below. This is just a RFC sort of patch, as it is incomplete, not covering the dvbdev.c pipeline setup logic. Anyway, it should be useful for further discussions, but some work is needed. Regards, Mauro [RFC] media: v4l2: taint pads with the signal types for consumer devices Consumer devices are provided with a wide diferent range of types supported by the same driver, allowing different configutations. In order to make easier to setup media controller links, "taint" pads with the signal type it carries. While here, get rid of DEMOD_PAD_VBI_OUT, as the signal it carries is actually the same as the normal video output. The difference happens at the video/VBI interface: - for VBI, only the hidden lines are streamed; - for video, the stream is usually cropped to hide the vbi lines. Compile-tested only and incomplete: the dvbdev.c should have a similar change like the one done at v4l2-mc.c. Signed-off-by: Mauro Carvalho Chehab diff --git a/drivers/media/dvb-frontends/au8522_decoder.c b/drivers/media/dvb-frontends/au8522_decoder.c index 343dc92ef54e..f4df9ab3d8b0 100644 --- a/drivers/media/dvb-frontends/au8522_decoder.c +++ b/drivers/media/dvb-frontends/au8522_decoder.c @@ -721,9 +721,11 @@ static int au8522_probe(struct i2c_client *client, #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + state->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_RF; state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; - state->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE; + state->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_ATV_VIDEO; state->pads[DEMOD_PAD_AUDIO_OUT].flags = MEDIA_PAD_FL_SOURCE; + state->pads[DEMOD_PAD_AUDIO_OUT].sig_type = PAD_SIGNAL_AUDIO; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; ret = media_entity_pads_init(>entity, ARRAY_SIZE(state->pads), diff --git a/drivers/media/i2c/msp3400-driver.c b/drivers/media/i2c/msp3400-driver.c index 3db966db83eb..3b9c729fbd52 100644 --- a/drivers/media/i2c/msp3400-driver.c +++ b/drivers/media/i2c/msp3400-driver.c @@ -704,7 +704,9 @@ static int msp_probe(struct i2c_client *client, const struct i2c_device_id *id) #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[IF_AUD_DEC_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + state->pads[IF_AUD_DEC_PAD_IF_INPUT].sig_type = PAD_SIGNAL_AUDIO; state->pads[IF_AUD_DEC_PAD_OUT].flags = MEDIA_PAD_FL_SOURCE; + state->pads[IF_AUD_DEC_PAD_OUT].sig_type = PAD_SIGNAL_AUDIO; sd->entity.function = MEDIA_ENT_F_IF_AUD_DECODER; diff --git a/drivers/media/i2c/saa7115.c b/drivers/media/i2c/saa7115.c index b07114b5efb2..0b298aa34a7c 100644 --- a/drivers/media/i2c/saa7115.c +++ b/drivers/media/i2c/saa7115.c @@ -1835,8 +1835,9 @@ static int saa711x_probe(struct i2c_client *client, #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + state->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_RF; state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; - state->pads[DEMOD_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE; + state->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_ATV_VIDEO; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c index 1734ed4ede33..dab83a774e73 100644 --- a/drivers/media/i2c/tvp5150.c +++ b/drivers/media/i2c/tvp5150.c @@ -1495,8 +1495,9 @@ static int tvp5150_probe(struct i2c_client *c, #if defined(CONFIG_MEDIA_CONTROLLER) core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + core->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_RF; core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; -
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Em Tue, 31 Jul 2018 14:36:52 +0200 Marco Felsch escreveu: > Hi Javier, > Hi Mauro, > > On 18-07-31 13:26, Javier Martinez Canillas wrote: > > Hi Mauro, > > > > On 07/31/2018 12:06 PM, Mauro Carvalho Chehab wrote: > > > Em Tue, 31 Jul 2018 10:52:56 +0200 > > > Javier Martinez Canillas escreveu: > > > > > >> Hello Mauro, > > >> > > >> On 07/30/2018 08:18 PM, Mauro Carvalho Chehab wrote: > > >>> Em Thu, 28 Jun 2018 18:20:50 +0200 > > >>> Marco Felsch escreveu: > > >>> > > From: Javier Martinez Canillas > > > > Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors > > support") > > added input signals support for the tvp5150, but the approach was found > > to be incorrect so the corresponding DT binding commit 82c2ffeb217a > > ("[media] tvp5150: document input connectors DT bindings") was > > reverted. > > > > This left the driver with an undocumented (and wrong) DT parsing logic, > > so lets get rid of this code as well until the input connectors support > > is implemented properly. > > > > It's a partial revert due other patches added on top of mentioned > > commit > > not allowing the commit to be reverted cleanly anymore. But all the > > code > > related to the DT parsing logic and input entities creation are > > removed. > > > > Suggested-by: Laurent Pinchart > > Signed-off-by: Javier Martinez Canillas > > Acked-by: Laurent Pinchart > > [m.fel...@pengutronix.de: rm TVP5150_INPUT_NUM define] > > Signed-off-by: Marco Felsch > > --- > > >>> > > >>> ... > > >>> > > -static int tvp5150_registered(struct v4l2_subdev *sd) > > -{ > > -#ifdef CONFIG_MEDIA_CONTROLLER > > - struct tvp5150 *decoder = to_tvp5150(sd); > > - int ret = 0; > > - int i; > > - > > - for (i = 0; i < TVP5150_INPUT_NUM; i++) { > > - struct media_entity *input = >input_ent[i]; > > - struct media_pad *pad = >input_pad[i]; > > - > > - if (!input->name) > > - continue; > > - > > - decoder->input_pad[i].flags = MEDIA_PAD_FL_SOURCE; > > - > > - ret = media_entity_pads_init(input, 1, pad); > > - if (ret < 0) > > - return ret; > > - > > - ret = media_device_register_entity(sd->v4l2_dev->mdev, > > input); > > - if (ret < 0) > > - return ret; > > - > > - ret = media_create_pad_link(input, 0, >entity, > > - DEMOD_PAD_IF_INPUT, 0); > > - if (ret < 0) { > > - media_device_unregister_entity(input); > > - return ret; > > - } > > - } > > -#endif > > >>> > > >>> Hmm... I suspect that reverting this part may cause problems for drivers > > >>> like em28xx when compiled with MC, as they rely that the supported > > >>> demods > > >>> will have 3 pads (DEMOD_NUM_PADS). > > >>> > > >> > > >> I don't see how this change could affect em28xx and other drivers. The > > >> function > > >> tvp5150_registered() being removed here, only register the media entity > > >> and add > > >> a link if input->name was set. This is set in tvp5150_parse_dt() and > > >> only if a > > >> input connector as defined in the Device Tree file. > > >> > > >> In other words, all the code removed by this patch is DT-only and isn't > > >> used by > > >> any media driver that makes use of the tvp5151. > > >> > > >> As mentioned in the commit message, this code has never been used > > >> (besides from > > >> my testings) and should had been removed when the DT binding was > > >> reverted, but > > >> for some reasons the first patch landed and the second didn't at the > > >> time. > > > > > > Short answer: > > > > > > Yeah, you're right. Yet, patch 19/22 will cause regressions. > > > > > > Long answer: > > > > > > That's easy enough to test. > > > > > > Without this patch, a em28xx-based board (Terratec Grabster AV350) > > > reports: > > > > > > $ ./mc_nextgen_test -D > > > digraph board { > > > rankdir=TB > > > colorscheme=x11 > > > labelloc="t" > > > label="Grabster AV 350 > > > driver:em28xx, bus: usb-:00:14.0-2 > > > " > > > intf_devnode_7 [label="intf_devnode_7\nvideo\n/dev/video0", shape=box, > > > style=filled, fillcolor=yellow] > > > intf_devnode_10 [label="intf_devnode_10\nvbi\n/dev/vbi0", shape=box, > > > style=filled, fillcolor=yellow] > > > entity_1 [label="{{ 0} | entity_1\nATV decoder\ntvp5150 0-005c | > > > { 1 | 2}}", shape=Mrecord, style=filled, > > > fillcolor=lightblue] > > > entity_6 [label="{{ 0} | entity_6\nV4L I/O\n2-2:1.0 video}", > > > shape=Mrecord, style=filled,
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Hi Marco, On 07/31/2018 02:36 PM, Marco Felsch wrote: [snip] >> >> Yes, another thing that patch 19/22 should take into account is DTs that >> don't have input connectors defined. So probably TVP5150_PORT_YOUT should >> be 0 instead of TVP5150_PORT_NUM - 1 as is the case in the current patch. >> >> In other words, it should work both when input connectors are defined in >> the DT and when these are not defined and only an output port is defined. > > Yes, it would be a approach to map the output port dynamicaly to the > highest port number. I tried to keep things easy by a static mapping. > Maybe a follow up patch can change this behaviour. > > Anyway, input connectors aren't required. There must be at least one > port child node with a correct port-number in the DT. > Yes, that was my point. But your patch uses the port child reg property as the index for the struct device_node *endpoints[TVP5150_PORT_NUM] array. If there's only one port child (for the output) then the DT binding says that the reg property isn't required, so this will be 0 and your patch will wrongly map it to TVP5150_PORT_AIP1A. That's why I said that the output port should be the first one in your enum tvp5150_ports and not the last one. > Regards, > Marco > Best regards, -- Javier Martinez Canillas Software Engineer - Desktop Hardware Enablement Red Hat
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Hi Javier, Hi Mauro, On 18-07-31 13:26, Javier Martinez Canillas wrote: > Hi Mauro, > > On 07/31/2018 12:06 PM, Mauro Carvalho Chehab wrote: > > Em Tue, 31 Jul 2018 10:52:56 +0200 > > Javier Martinez Canillas escreveu: > > > >> Hello Mauro, > >> > >> On 07/30/2018 08:18 PM, Mauro Carvalho Chehab wrote: > >>> Em Thu, 28 Jun 2018 18:20:50 +0200 > >>> Marco Felsch escreveu: > >>> > From: Javier Martinez Canillas > > Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support") > added input signals support for the tvp5150, but the approach was found > to be incorrect so the corresponding DT binding commit 82c2ffeb217a > ("[media] tvp5150: document input connectors DT bindings") was reverted. > > This left the driver with an undocumented (and wrong) DT parsing logic, > so lets get rid of this code as well until the input connectors support > is implemented properly. > > It's a partial revert due other patches added on top of mentioned commit > not allowing the commit to be reverted cleanly anymore. But all the code > related to the DT parsing logic and input entities creation are removed. > > Suggested-by: Laurent Pinchart > Signed-off-by: Javier Martinez Canillas > Acked-by: Laurent Pinchart > [m.fel...@pengutronix.de: rm TVP5150_INPUT_NUM define] > Signed-off-by: Marco Felsch > --- > >>> > >>> ... > >>> > -static int tvp5150_registered(struct v4l2_subdev *sd) > -{ > -#ifdef CONFIG_MEDIA_CONTROLLER > -struct tvp5150 *decoder = to_tvp5150(sd); > -int ret = 0; > -int i; > - > -for (i = 0; i < TVP5150_INPUT_NUM; i++) { > -struct media_entity *input = >input_ent[i]; > -struct media_pad *pad = >input_pad[i]; > - > -if (!input->name) > -continue; > - > -decoder->input_pad[i].flags = MEDIA_PAD_FL_SOURCE; > - > -ret = media_entity_pads_init(input, 1, pad); > -if (ret < 0) > -return ret; > - > -ret = media_device_register_entity(sd->v4l2_dev->mdev, > input); > -if (ret < 0) > -return ret; > - > -ret = media_create_pad_link(input, 0, >entity, > -DEMOD_PAD_IF_INPUT, 0); > -if (ret < 0) { > -media_device_unregister_entity(input); > -return ret; > -} > -} > -#endif > >>> > >>> Hmm... I suspect that reverting this part may cause problems for drivers > >>> like em28xx when compiled with MC, as they rely that the supported demods > >>> will have 3 pads (DEMOD_NUM_PADS). > >>> > >> > >> I don't see how this change could affect em28xx and other drivers. The > >> function > >> tvp5150_registered() being removed here, only register the media entity > >> and add > >> a link if input->name was set. This is set in tvp5150_parse_dt() and only > >> if a > >> input connector as defined in the Device Tree file. > >> > >> In other words, all the code removed by this patch is DT-only and isn't > >> used by > >> any media driver that makes use of the tvp5151. > >> > >> As mentioned in the commit message, this code has never been used (besides > >> from > >> my testings) and should had been removed when the DT binding was reverted, > >> but > >> for some reasons the first patch landed and the second didn't at the time. > > > > Short answer: > > > > Yeah, you're right. Yet, patch 19/22 will cause regressions. > > > > Long answer: > > > > That's easy enough to test. > > > > Without this patch, a em28xx-based board (Terratec Grabster AV350) reports: > > > > $ ./mc_nextgen_test -D > > digraph board { > > rankdir=TB > > colorscheme=x11 > > labelloc="t" > > label="Grabster AV 350 > > driver:em28xx, bus: usb-:00:14.0-2 > > " > > intf_devnode_7 [label="intf_devnode_7\nvideo\n/dev/video0", shape=box, > > style=filled, fillcolor=yellow] > > intf_devnode_10 [label="intf_devnode_10\nvbi\n/dev/vbi0", shape=box, > > style=filled, fillcolor=yellow] > > entity_1 [label="{{ 0} | entity_1\nATV decoder\ntvp5150 0-005c | > > { 1 | 2}}", shape=Mrecord, style=filled, fillcolor=lightblue] > > entity_6 [label="{{ 0} | entity_6\nV4L I/O\n2-2:1.0 video}", > > shape=Mrecord, style=filled, fillcolor=aquamarine] > > entity_9 [label="{{ 0} | entity_9\nVBI I/O\n2-2:1.0 vbi}", > > shape=Mrecord, style=filled, fillcolor=aquamarine] > > entity_14 [label="{entity_14\nunknown entity type\nComposite | > > { 0}}", shape=Mrecord, style=filled, fillcolor=cadetblue] > > entity_16 [label="{entity_16\nunknown entity type\nS-Video | { > >
Mobile apps development
We are an image editing service provider having more than 10 years of industry experience. We use latest techniques for photo editing to meet international quality standards. We are committed to deliver reliable photo processing services. Production of 1000+ images within single working day 24×7×365 photo editing services On-demand image editing to meet rush job requirements Service details: Images Masking Photo Clipping Path Photo Cutout Photo Enhancement Vector Conversion Photo Stitching Services Fashion Photo Editing Jewelry Retouching Footwear Photo Editing Furniture Photo Retouching Wedding Photo Editing Real Estate Photo Editing Photo Restoration We provide trials to evaluate our service quality to new clients. Thanks, Edward
Re: [PATCH] media: rc: read out of bounds if bpf reports high protocol number
Hi Hias, On Mon, Jul 30, 2018 at 09:20:18PM +0200, Matthias Reichl wrote: > On Sat, Jul 28, 2018 at 10:11:15AM +0100, Sean Young wrote: > > The repeat period is read from a static array. If a keydown event is > > reported from bpf with a high protocol number, we read out of bounds. This > > is unlikely to end up with a reasonable repeat period at the best of times, > > in which case no timely key up event is generated. > > > > Signed-off-by: Sean Young > > --- > > drivers/media/rc/rc-main.c | 12 ++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c > > index 2e222d9ee01f..a24850be1f4f 100644 > > --- a/drivers/media/rc/rc-main.c > > +++ b/drivers/media/rc/rc-main.c > > @@ -679,6 +679,14 @@ static void ir_timer_repeat(struct timer_list *t) > > spin_unlock_irqrestore(>keylock, flags); > > } > > > > +unsigned int repeat_period(int protocol) > > +{ > > + if (protocol >= ARRAY_SIZE(protocols)) > > + return 100; > > 100 seems a bit arbitrarily chosen to me. Wouldn't it be better to > (re-)use eg protocols[RC_PROTO_UNKNOWN].repeat_period here? That's a good idea! I think the patch is already on its way to be merged, but we can patch this later. What we really need is a way to set the repeat period and minimum timeout for a bpf protocol. Sean
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Hi Mauro, On 07/31/2018 12:06 PM, Mauro Carvalho Chehab wrote: > Em Tue, 31 Jul 2018 10:52:56 +0200 > Javier Martinez Canillas escreveu: > >> Hello Mauro, >> >> On 07/30/2018 08:18 PM, Mauro Carvalho Chehab wrote: >>> Em Thu, 28 Jun 2018 18:20:50 +0200 >>> Marco Felsch escreveu: >>> From: Javier Martinez Canillas Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support") added input signals support for the tvp5150, but the approach was found to be incorrect so the corresponding DT binding commit 82c2ffeb217a ("[media] tvp5150: document input connectors DT bindings") was reverted. This left the driver with an undocumented (and wrong) DT parsing logic, so lets get rid of this code as well until the input connectors support is implemented properly. It's a partial revert due other patches added on top of mentioned commit not allowing the commit to be reverted cleanly anymore. But all the code related to the DT parsing logic and input entities creation are removed. Suggested-by: Laurent Pinchart Signed-off-by: Javier Martinez Canillas Acked-by: Laurent Pinchart [m.fel...@pengutronix.de: rm TVP5150_INPUT_NUM define] Signed-off-by: Marco Felsch --- >>> >>> ... >>> -static int tvp5150_registered(struct v4l2_subdev *sd) -{ -#ifdef CONFIG_MEDIA_CONTROLLER - struct tvp5150 *decoder = to_tvp5150(sd); - int ret = 0; - int i; - - for (i = 0; i < TVP5150_INPUT_NUM; i++) { - struct media_entity *input = >input_ent[i]; - struct media_pad *pad = >input_pad[i]; - - if (!input->name) - continue; - - decoder->input_pad[i].flags = MEDIA_PAD_FL_SOURCE; - - ret = media_entity_pads_init(input, 1, pad); - if (ret < 0) - return ret; - - ret = media_device_register_entity(sd->v4l2_dev->mdev, input); - if (ret < 0) - return ret; - - ret = media_create_pad_link(input, 0, >entity, - DEMOD_PAD_IF_INPUT, 0); - if (ret < 0) { - media_device_unregister_entity(input); - return ret; - } - } -#endif >>> >>> Hmm... I suspect that reverting this part may cause problems for drivers >>> like em28xx when compiled with MC, as they rely that the supported demods >>> will have 3 pads (DEMOD_NUM_PADS). >>> >> >> I don't see how this change could affect em28xx and other drivers. The >> function >> tvp5150_registered() being removed here, only register the media entity and >> add >> a link if input->name was set. This is set in tvp5150_parse_dt() and only if >> a >> input connector as defined in the Device Tree file. >> >> In other words, all the code removed by this patch is DT-only and isn't used >> by >> any media driver that makes use of the tvp5151. >> >> As mentioned in the commit message, this code has never been used (besides >> from >> my testings) and should had been removed when the DT binding was reverted, >> but >> for some reasons the first patch landed and the second didn't at the time. > > Short answer: > > Yeah, you're right. Yet, patch 19/22 will cause regressions. > > Long answer: > > That's easy enough to test. > > Without this patch, a em28xx-based board (Terratec Grabster AV350) reports: > > $ ./mc_nextgen_test -D > digraph board { > rankdir=TB > colorscheme=x11 > labelloc="t" > label="Grabster AV 350 > driver:em28xx, bus: usb-:00:14.0-2 > " > intf_devnode_7 [label="intf_devnode_7\nvideo\n/dev/video0", shape=box, > style=filled, fillcolor=yellow] > intf_devnode_10 [label="intf_devnode_10\nvbi\n/dev/vbi0", shape=box, > style=filled, fillcolor=yellow] > entity_1 [label="{{ 0} | entity_1\nATV decoder\ntvp5150 0-005c | > { 1 | 2}}", shape=Mrecord, style=filled, fillcolor=lightblue] > entity_6 [label="{{ 0} | entity_6\nV4L I/O\n2-2:1.0 video}", > shape=Mrecord, style=filled, fillcolor=aquamarine] > entity_9 [label="{{ 0} | entity_9\nVBI I/O\n2-2:1.0 vbi}", > shape=Mrecord, style=filled, fillcolor=aquamarine] > entity_14 [label="{entity_14\nunknown entity type\nComposite | > { 0}}", shape=Mrecord, style=filled, fillcolor=cadetblue] > entity_16 [label="{entity_16\nunknown entity type\nS-Video | { > 0}}", shape=Mrecord, style=filled, fillcolor=cadetblue] > intf_devnode_7 -> entity_6 [dir="none" color="orange"] > intf_devnode_10 -> entity_9 [dir="none" color="orange"] > entity_1:pad_3 -> entity_6:pad_12 [color=blue] > entity_1:pad_4 -> entity_9:pad_13 [color=blue] > entity_14:pad_15 -> entity_1:pad_2 [color=blue] > entity_16:pad_17 -> entity_1:pad_2 [color=blue style="dashed"] > } > > tvp5150
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Em Tue, 31 Jul 2018 10:52:56 +0200 Javier Martinez Canillas escreveu: > Hello Mauro, > > On 07/30/2018 08:18 PM, Mauro Carvalho Chehab wrote: > > Em Thu, 28 Jun 2018 18:20:50 +0200 > > Marco Felsch escreveu: > > > >> From: Javier Martinez Canillas > >> > >> Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support") > >> added input signals support for the tvp5150, but the approach was found > >> to be incorrect so the corresponding DT binding commit 82c2ffeb217a > >> ("[media] tvp5150: document input connectors DT bindings") was reverted. > >> > >> This left the driver with an undocumented (and wrong) DT parsing logic, > >> so lets get rid of this code as well until the input connectors support > >> is implemented properly. > >> > >> It's a partial revert due other patches added on top of mentioned commit > >> not allowing the commit to be reverted cleanly anymore. But all the code > >> related to the DT parsing logic and input entities creation are removed. > >> > >> Suggested-by: Laurent Pinchart > >> Signed-off-by: Javier Martinez Canillas > >> Acked-by: Laurent Pinchart > >> [m.fel...@pengutronix.de: rm TVP5150_INPUT_NUM define] > >> Signed-off-by: Marco Felsch > >> --- > > > > ... > > > >> -static int tvp5150_registered(struct v4l2_subdev *sd) > >> -{ > >> -#ifdef CONFIG_MEDIA_CONTROLLER > >> - struct tvp5150 *decoder = to_tvp5150(sd); > >> - int ret = 0; > >> - int i; > >> - > >> - for (i = 0; i < TVP5150_INPUT_NUM; i++) { > >> - struct media_entity *input = >input_ent[i]; > >> - struct media_pad *pad = >input_pad[i]; > >> - > >> - if (!input->name) > >> - continue; > >> - > >> - decoder->input_pad[i].flags = MEDIA_PAD_FL_SOURCE; > >> - > >> - ret = media_entity_pads_init(input, 1, pad); > >> - if (ret < 0) > >> - return ret; > >> - > >> - ret = media_device_register_entity(sd->v4l2_dev->mdev, input); > >> - if (ret < 0) > >> - return ret; > >> - > >> - ret = media_create_pad_link(input, 0, >entity, > >> - DEMOD_PAD_IF_INPUT, 0); > >> - if (ret < 0) { > >> - media_device_unregister_entity(input); > >> - return ret; > >> - } > >> - } > >> -#endif > > > > Hmm... I suspect that reverting this part may cause problems for drivers > > like em28xx when compiled with MC, as they rely that the supported demods > > will have 3 pads (DEMOD_NUM_PADS). > > > > I don't see how this change could affect em28xx and other drivers. The > function > tvp5150_registered() being removed here, only register the media entity and > add > a link if input->name was set. This is set in tvp5150_parse_dt() and only if a > input connector as defined in the Device Tree file. > > In other words, all the code removed by this patch is DT-only and isn't used > by > any media driver that makes use of the tvp5151. > > As mentioned in the commit message, this code has never been used (besides > from > my testings) and should had been removed when the DT binding was reverted, but > for some reasons the first patch landed and the second didn't at the time. Short answer: Yeah, you're right. Yet, patch 19/22 will cause regressions. Long answer: That's easy enough to test. Without this patch, a em28xx-based board (Terratec Grabster AV350) reports: $ ./mc_nextgen_test -D digraph board { rankdir=TB colorscheme=x11 labelloc="t" label="Grabster AV 350 driver:em28xx, bus: usb-:00:14.0-2 " intf_devnode_7 [label="intf_devnode_7\nvideo\n/dev/video0", shape=box, style=filled, fillcolor=yellow] intf_devnode_10 [label="intf_devnode_10\nvbi\n/dev/vbi0", shape=box, style=filled, fillcolor=yellow] entity_1 [label="{{ 0} | entity_1\nATV decoder\ntvp5150 0-005c | { 1 | 2}}", shape=Mrecord, style=filled, fillcolor=lightblue] entity_6 [label="{{ 0} | entity_6\nV4L I/O\n2-2:1.0 video}", shape=Mrecord, style=filled, fillcolor=aquamarine] entity_9 [label="{{ 0} | entity_9\nVBI I/O\n2-2:1.0 vbi}", shape=Mrecord, style=filled, fillcolor=aquamarine] entity_14 [label="{entity_14\nunknown entity type\nComposite | { 0}}", shape=Mrecord, style=filled, fillcolor=cadetblue] entity_16 [label="{entity_16\nunknown entity type\nS-Video | { 0}}", shape=Mrecord, style=filled, fillcolor=cadetblue] intf_devnode_7 -> entity_6 [dir="none" color="orange"] intf_devnode_10 -> entity_9 [dir="none" color="orange"] entity_1:pad_3 -> entity_6:pad_12 [color=blue] entity_1:pad_4 -> entity_9:pad_13 [color=blue] entity_14:pad_15 -> entity_1:pad_2 [color=blue] entity_16:pad_17 -> entity_1:pad_2 [color=blue style="dashed"] } tvp5150 reports 3 pads (one input, two output pads), and media core properly connects the source pads. With patch 18/22, I got the same graph. So, yeah,
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Hello Mauro, On 07/30/2018 08:18 PM, Mauro Carvalho Chehab wrote: > Em Thu, 28 Jun 2018 18:20:50 +0200 > Marco Felsch escreveu: > >> From: Javier Martinez Canillas >> >> Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support") >> added input signals support for the tvp5150, but the approach was found >> to be incorrect so the corresponding DT binding commit 82c2ffeb217a >> ("[media] tvp5150: document input connectors DT bindings") was reverted. >> >> This left the driver with an undocumented (and wrong) DT parsing logic, >> so lets get rid of this code as well until the input connectors support >> is implemented properly. >> >> It's a partial revert due other patches added on top of mentioned commit >> not allowing the commit to be reverted cleanly anymore. But all the code >> related to the DT parsing logic and input entities creation are removed. >> >> Suggested-by: Laurent Pinchart >> Signed-off-by: Javier Martinez Canillas >> Acked-by: Laurent Pinchart >> [m.fel...@pengutronix.de: rm TVP5150_INPUT_NUM define] >> Signed-off-by: Marco Felsch >> --- > > ... > >> -static int tvp5150_registered(struct v4l2_subdev *sd) >> -{ >> -#ifdef CONFIG_MEDIA_CONTROLLER >> -struct tvp5150 *decoder = to_tvp5150(sd); >> -int ret = 0; >> -int i; >> - >> -for (i = 0; i < TVP5150_INPUT_NUM; i++) { >> -struct media_entity *input = >input_ent[i]; >> -struct media_pad *pad = >input_pad[i]; >> - >> -if (!input->name) >> -continue; >> - >> -decoder->input_pad[i].flags = MEDIA_PAD_FL_SOURCE; >> - >> -ret = media_entity_pads_init(input, 1, pad); >> -if (ret < 0) >> -return ret; >> - >> -ret = media_device_register_entity(sd->v4l2_dev->mdev, input); >> -if (ret < 0) >> -return ret; >> - >> -ret = media_create_pad_link(input, 0, >entity, >> -DEMOD_PAD_IF_INPUT, 0); >> -if (ret < 0) { >> -media_device_unregister_entity(input); >> -return ret; >> -} >> -} >> -#endif > > Hmm... I suspect that reverting this part may cause problems for drivers > like em28xx when compiled with MC, as they rely that the supported demods > will have 3 pads (DEMOD_NUM_PADS). > I don't see how this change could affect em28xx and other drivers. The function tvp5150_registered() being removed here, only register the media entity and add a link if input->name was set. This is set in tvp5150_parse_dt() and only if a input connector as defined in the Device Tree file. In other words, all the code removed by this patch is DT-only and isn't used by any media driver that makes use of the tvp5151. As mentioned in the commit message, this code has never been used (besides from my testings) and should had been removed when the DT binding was reverted, but for some reasons the first patch landed and the second didn't at the time. > Thanks, > Mauro > Best regards, -- Javier Martinez Canillas Software Engineer - Desktop Hardware Enablement Red Hat
[RFC PATCH ragnatech] media: tvp5150: tvp5150_volatile_reg() can be static
Fixes: 07dff5b8c030 ("media: tvp5150: convert register access to regmap") Signed-off-by: kbuild test robot --- tvp5150.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c index b3c7b65..2bd9500 100644 --- a/drivers/media/i2c/tvp5150.c +++ b/drivers/media/i2c/tvp5150.c @@ -1321,7 +1321,7 @@ static const struct regmap_range tvp5150_readable_ranges[] = { }, }; -bool tvp5150_volatile_reg(struct device *dev, unsigned int reg) +static bool tvp5150_volatile_reg(struct device *dev, unsigned int reg) { switch (reg) { case TVP5150_VERT_LN_COUNT_MSB:
[ragnatech:media-tree 311/324] drivers/media/i2c/tvp5150.c:1324:6: sparse: symbol 'tvp5150_volatile_reg' was not declared. Should it be static?
tree: git://git.ragnatech.se/linux media-tree head: 1d06352e18ef502e30837cedfe618298816fb48c commit: 07dff5b8c03053db6fb6e33fd38c3e5d37f67bc5 [311/324] media: tvp5150: convert register access to regmap reproduce: # apt-get install sparse git checkout 07dff5b8c03053db6fb6e33fd38c3e5d37f67bc5 make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) drivers/media/i2c/tvp5150.c:893:21: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:893:21: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:893:21: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:893:21: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:893:21: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:893:21: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:893:21: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:894:20: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:894:20: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:894:20: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:894:20: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:894:20: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:894:20: sparse: expression using sizeof(void) drivers/media/i2c/tvp5150.c:894:20: sparse: expression using sizeof(void) >> drivers/media/i2c/tvp5150.c:1324:6: sparse: symbol 'tvp5150_volatile_reg' >> was not declared. Should it be static? Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"
Hi Mauro, On 18-07-30 15:18, Mauro Carvalho Chehab wrote: > Em Thu, 28 Jun 2018 18:20:50 +0200 > Marco Felsch escreveu: > > > From: Javier Martinez Canillas > > > > Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support") > > added input signals support for the tvp5150, but the approach was found > > to be incorrect so the corresponding DT binding commit 82c2ffeb217a > > ("[media] tvp5150: document input connectors DT bindings") was reverted. > > > > This left the driver with an undocumented (and wrong) DT parsing logic, > > so lets get rid of this code as well until the input connectors support > > is implemented properly. > > > > It's a partial revert due other patches added on top of mentioned commit > > not allowing the commit to be reverted cleanly anymore. But all the code > > related to the DT parsing logic and input entities creation are removed. > > > > Suggested-by: Laurent Pinchart > > Signed-off-by: Javier Martinez Canillas > > Acked-by: Laurent Pinchart > > [m.fel...@pengutronix.de: rm TVP5150_INPUT_NUM define] > > Signed-off-by: Marco Felsch > > --- > > ... > > > -static int tvp5150_registered(struct v4l2_subdev *sd) > > -{ > > -#ifdef CONFIG_MEDIA_CONTROLLER > > - struct tvp5150 *decoder = to_tvp5150(sd); > > - int ret = 0; > > - int i; > > - > > - for (i = 0; i < TVP5150_INPUT_NUM; i++) { > > - struct media_entity *input = >input_ent[i]; > > - struct media_pad *pad = >input_pad[i]; > > - > > - if (!input->name) > > - continue; > > - > > - decoder->input_pad[i].flags = MEDIA_PAD_FL_SOURCE; > > - > > - ret = media_entity_pads_init(input, 1, pad); > > - if (ret < 0) > > - return ret; > > - > > - ret = media_device_register_entity(sd->v4l2_dev->mdev, input); > > - if (ret < 0) > > - return ret; > > - > > - ret = media_create_pad_link(input, 0, >entity, > > - DEMOD_PAD_IF_INPUT, 0); > > - if (ret < 0) { > > - media_device_unregister_entity(input); > > - return ret; > > - } > > - } > > -#endif > > Hmm... I suspect that reverting this part may cause problems for drivers > like em28xx when compiled with MC, as they rely that the supported demods > will have 3 pads (DEMOD_NUM_PADS). Please, can you test this for me? I have no such usb device. Using the DEMOD_NUM_PADS looked wrong to me since the tvp5150 has more than one input pad. Thanks, Marco > Thanks, > Mauro >
Re: [PATCH 13/22] [media] tvp5150: disable output while signal not locked
Hi Mauro, thanks for your feedback. On 18-07-30 15:00, Mauro Carvalho Chehab wrote: > Em Thu, 28 Jun 2018 18:20:45 +0200 > Marco Felsch escreveu: > > > From: Philipp Zabel > > > > To avoid short frames on stream start, keep output pins at high impedance > > while we are not properly locked onto the input signal. > > > > Signed-off-by: Philipp Zabel > > Signed-off-by: Marco Felsch > > --- > > drivers/media/i2c/tvp5150.c | 39 ++--- > > 1 file changed, 28 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c > > index d8bdbedd8826..27cfd08be3d2 100644 > > --- a/drivers/media/i2c/tvp5150.c > > +++ b/drivers/media/i2c/tvp5150.c > > @@ -60,6 +60,7 @@ struct tvp5150 { > > v4l2_std_id detected_norm; > > u32 input; > > u32 output; > > + u32 oe; > > int enable; > > bool lock; > > > > @@ -799,14 +800,20 @@ static irqreturn_t tvp5150_isr(int irq, void *dev_id) > > { > > struct tvp5150 *decoder = dev_id; > > struct regmap *map = decoder->regmap; > > - unsigned int active = 0, status = 0; > > + unsigned int mask, active = 0, status = 0; > > + > > + mask = TVP5150_MISC_CTL_YCBCR_OE | TVP5150_MISC_CTL_SYNC_OE | > > + TVP5150_MISC_CTL_CLOCK_OE; > > > > regmap_read(map, TVP5150_INT_STATUS_REG_A, ); > > if (status) { > > regmap_write(map, TVP5150_INT_STATUS_REG_A, status); > > > > - if (status & TVP5150_INT_A_LOCK) > > + if (status & TVP5150_INT_A_LOCK) { > > decoder->lock = !!(status & TVP5150_INT_A_LOCK_STATUS); > > + regmap_update_bits(map, TVP5150_MISC_CTL, mask, > > + decoder->lock ? decoder->oe : 0); > > + } > > > > return IRQ_HANDLED; > > } > > @@ -872,10 +879,26 @@ static int tvp5150_enable(struct v4l2_subdev *sd) > > /* Disable autoswitch mode */ > > tvp5150_set_std(sd, std); > > > > - if (decoder->mbus_type == V4L2_MBUS_PARALLEL) > > + /* > > +* Enable the YCbCr and clock outputs. In discrete sync mode > > +* (non-BT.656) additionally enable the the sync outputs. > > +*/ > > + switch (decoder->mbus_type) { > > + case V4L2_MBUS_PARALLEL: > > /* 8-bit 4:2:2 YUV with discrete sync output */ > > regmap_update_bits(decoder->regmap, TVP5150_DATA_RATE_SEL, > >0x7, 0x0); > > + decoder->oe = TVP5150_MISC_CTL_YCBCR_OE | > > + TVP5150_MISC_CTL_CLOCK_OE | > > + TVP5150_MISC_CTL_SYNC_OE; > > + break; > > + case V4L2_MBUS_BT656: > > + decoder->oe = TVP5150_MISC_CTL_YCBCR_OE | > > + TVP5150_MISC_CTL_CLOCK_OE; > > + break; > > + default: > > + return -EINVAL; > > + } > > > > return 0; > > }; > > @@ -1190,14 +1213,8 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd, > > int enable) > > if (enable) { > > tvp5150_enable(sd); > > > > - /* > > -* Enable the YCbCr and clock outputs. In discrete sync mode > > -* (non-BT.656) additionally enable the the sync outputs. > > -*/ > > - val = TVP5150_MISC_CTL_YCBCR_OE | TVP5150_MISC_CTL_CLOCK_OE; > > - if (decoder->mbus_type == V4L2_MBUS_PARALLEL) > > - val |= TVP5150_MISC_CTL_SYNC_OE; > > - > > + /* Enable outputs if decoder is locked */ > > + val = decoder->lock ? decoder->oe : 0; > > Hmm... this only works if the tvp5150 has the interrupts enabled. > > The code should be, instead: > > if (c->irq) > val = decoder->lock ? decoder->oe : 0; > else > val = decoder->oe; The previous patch "[media] tvp5150: Add sync lock interrupt handling" adds the look mechanism. As you can see the lock will be always true if there is no interrupt support during probe(): core->irq = c->irq; tvp5150_reset(sd, 0); /* Calls v4l2_ctrl_handler_setup() */ if (c->irq) { res = devm_request_threaded_irq(>dev, c->irq, NULL, tvp5150_isr, IRQF_TRIGGER_HIGH | IRQF_ONESHOT, "tvp5150", core); if (res) return res; } else { core->lock = true; } I'am with you that your s_stream version looks a bit nicer but adds the check again. Regards, Marco > > > int_val = TVP5150_INT_A_LOCK; > > } > > > > > > Thanks, > Mauro > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |