Re: [PATCH v4 1/3] [media] of: move graph helpers from drivers/media/v4l2-core to drivers/of
On 08/03/14 14:23, Grant Likely wrote: That's fine. In that case the driver would specifically require the endpoint to be that one node although the above looks a little weird The driver can't require that. It's up to the board designer to decide how many endpoints are used. A driver may say that it has a single input port. But the number of endpoints for that port is up to the use case. Come now, when you're writing a driver you know if it will ever be possible to have more than one port. If that is the case then the binding should be specifically laid out for that. If there will never be multiple ports and the binding is unambiguous, then, and only then, should the shortcut be used, and only the shortcut should be accepted. I was talking about endpoints, not ports. There's no unclarity about the number of ports, that comes directly from the hardware for that specific component. The number of endpoints, however, come from the board hardware. The driver writer cannot know that. to me. I would recommend that if there are other non-port child nodes then the ports should still be encapsulated by a ports node. The device binding should not be ambiguous about which nodes are ports. Hmm, ambiguous in what way? Parsing the binding now consists of a ladder of 'ifs' that gives three distinct different behaviours for no benefit. You don't want that in It considerably lessens the amount of text in the DT for many use cases, making it easier to write and maintain the dts files. bindings because it makes it more difficult to get the parsing right in the first place, and to make sure that all users parse it in the same way (Linux, U-Boot, BSD, etc). Bindings should be as absolutely simple as possible. Well, yes, I agree there. This is not the simplest of bindings. I'd be more than happy if we would come up with simpler version of this, which would still allow us to have the same descriptive power. Just to be clear, I have no problem with having the option in the pattern, but the driver needs to be specific about what layout it expects. If we forget the shortened endpoint format, I think it can be quite specific. A device has either one port, in which case it should require the 'ports' node to be omitted, or the device has more than one port, in which case it should require 'ports' node. Note that the original v4l2 binding doc says that 'ports' is always optional. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 3/3] Documentation: of: Document graph bindings
On 08/03/14 14:25, Grant Likely wrote: Sure. If endpoints are logical, then only create the ones actually hooked up. No problem there. But nor do I see any issue with having empty connections if the board author things it makes sense to have them in the dtsi. I don't think they are usually logical, although they probably might be in some cases. As I see it, a port is a group of pins in a hardware component, and two endpoints define a connection between two ports, which on the HW level are the wires between the ports. So a port with two endpoints is a group of pins, with wires that go from the same pins to two different components. Tomi signature.asc Description: OpenPGP digital signature
Question about set format call check for vb2_is_busy
Hi, I am studying v4l2 m2m driver example. I want to know why the set format function in the example fails when it is called again after user application req_buf? In set format function checks for vb2_is_busy(vq) and that function returns true after user space app calls req_buf. For example in here: http://stuff.mit.edu/afs/sipb/contrib/linux/drivers/media/platform/mem2mem_testdev.c static int vidioc_s_fmt(struct m2mtest_ctx *ctx, struct v4l2_format *f) { //... // Check for vb2_is_busy() here: if (vb2_is_busy(vq)) { v4l2_err(ctx-dev-v4l2_dev, %s queue busy\n, __func__); return -EBUSY; } //... } Why the driver prevents user space application change format after it request buffers? Thank you. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question about set format call check for vb2_is_busy
On 03/10/2014 08:02 AM, m silverstri wrote: Hi, I am studying v4l2 m2m driver example. I want to know why the set format function in the example fails when it is called again after user application req_buf? In set format function checks for vb2_is_busy(vq) and that function returns true after user space app calls req_buf. For example in here: http://stuff.mit.edu/afs/sipb/contrib/linux/drivers/media/platform/mem2mem_testdev.c static int vidioc_s_fmt(struct m2mtest_ctx *ctx, struct v4l2_format *f) { //... // Check for vb2_is_busy() here: if (vb2_is_busy(vq)) { v4l2_err(ctx-dev-v4l2_dev, %s queue busy\n, __func__); return -EBUSY; } //... } Why the driver prevents user space application change format after it request buffers? When you request the buffers they will be allocated based on the current format. Changing the format later will mean a change in buffer size, but once the buffers are allocated that is locked in place. It's generally a bad idea to, say, increase the image size and then watch how DMA overwrites your memory :-) This is not strictly speaking a v4l limitation, but a limitation of almost all hardware. It is possible to allow format changes after reqbufs is called, but that generally requires that the buffers all have the maximum possible size which wastes a lot of memory. And in addition you would have to have some sort of metadata as part of the captured frame so you know the actual size of the image stored in the buffer. None of the drivers in the kernel support this, BTW. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Siano: smsusb - Add a device id for PX-S1UD
Could anyone move forward with this patch? Or is there anything I should do more? Please advise. Thanks, Satoshi (2014/02/10 18:45), Satoshi Nagahama wrote: Add a device id to support for PX-S1UD (PLEX ISDB-T usb dongle) which has sms2270. Signed-off-by: Satoshi Nagahama satt...@aim.com --- drivers/media/usb/siano/smsusb.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c index 05bd91a..1836a41 100644 --- a/drivers/media/usb/siano/smsusb.c +++ b/drivers/media/usb/siano/smsusb.c @@ -653,6 +653,8 @@ static const struct usb_device_id smsusb_id_table[] = { .driver_info = SMS1XXX_BOARD_ZTE_DVB_DATA_CARD }, { USB_DEVICE(0x19D2, 0x0078), .driver_info = SMS1XXX_BOARD_ONDA_MDTV_DATA_CARD }, + { USB_DEVICE(0x3275, 0x0080), + .driver_info = SMS1XXX_BOARD_SIANO_RIO }, { } /* Terminating entry */ }; -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mcam-core.c:undefined reference to `vb2_dma_sg_memops'
On Mon, Mar 10, 2014 at 4:45 AM, kbuild test robot fengguang...@intel.com wrote: tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: ca62eec4e524591b82d9edf7a18e3ae6b691517d commit: 866f321339988293a5bb3ec6634c2c9d8396bf54 Revert staging/solo6x10: depend on CONFIG_FONTS date: 9 months ago config: x86_64-randconfig-tt1-03101102 (attached as .config) All error/warnings: drivers/built-in.o: In function `mcam_v4l_open': mcam-core.c:(.text+0x22b222): undefined reference to `vb2_dma_sg_memops' --- 0-DAY kernel build testing backend Open Source Technology Center http://lists.01.org/mailman/listinfo/kbuild Intel Corporation Patch available and accepted according to patchwork: https://patchwork.linuxtv.org/patch/20263/ But not yet in mainline Also submitted by Randy: https://lkml.org/lkml/2013/10/31/395 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say programmer or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/3] [media] of: move graph helpers from drivers/media/v4l2-core to drivers/of
Hi, On 03/08/2014 04:54 PM, Laurent Pinchart wrote: Hi Philipp, On Saturday 08 March 2014 13:07:23 Philipp Zabel wrote: On Fri, Mar 7, 2014 at 6:18 PM, Grant Likely wrote: On Wed, 26 Feb 2014 16:24:57 +0100, Philipp Zabel wrote: The 'ports' node is optional. It is only needed if the parent node has its own #address-cells and #size-cells properties. If the ports are direct children of the device node, there might be other nodes than ports: device { #address-cells = 1; #size-cells = 0; port@0 { endpoint { ... }; }; port@1 { endpoint { ... }; }; some-other-child { ... }; }; device { #address-cells = x; #size-cells = y; ports { #address-cells = 1; #size-cells = 0; port@0 { endpoint { ... }; }; port@1 { endpoint { ... }; }; }; some-other-child { ... }; }; From a pattern perspective I have no problem with that From an individual driver binding perspective that is just dumb! It's fine for the ports node to be optional, but an individual driver using the binding should be explicit about which it will accept. Please use either a flag or a separate wrapper so that the driver can select the behaviour. If the generic binding exists in both forms, most drivers should be able to cope with both. Maybe it should be mentioned in the bindings that the short form without ports node should be used where possible (i.e. for devices that don't already have #address,size-cells != 1,0). Having a separate wrapper to enforce the ports node for devices that need it might be useful. The helper should find the two endpoints in both cases. Tomi suggests an even more compact form for devices with just one port: device { endpoint { ... }; some-other-child { ... }; }; That's fine. In that case the driver would specifically require the endpoint to be that one node although the above looks a little weird to me. I would recommend that if there are other non-port child nodes then the ports should still be encapsulated by a ports node. The device binding should not be ambiguous about which nodes are ports. Sylwester suggested as an alternative, if I understood correctly, to drop the endpoint node and instead keep the port: device-a { implicit_output_ep: port { remote-endpoint = explicit_input_ep; }; }; device-b { port { explicit_input_ep: endpoint { remote-endpoint = implicit_output_ep; }; }; }; This would have the advantage to reduce verbosity for devices with multiple ports that are only connected via one endport each, and you'd always have the connected ports in the device tree as 'port' nodes. I like that idea. I would prefer making the 'port' nodes mandatory and the 'ports' and 'endpoint' nodes optional. Leaving the 'port' node out slightly decreases readability in my opinion, but making the 'endpoint' node optional increases it. That's just my point of view though. I want to propose another solution to simplify bindings, in fact I have few ideas to consider: 1. Use named ports instead of address-cells/regs. Ie instead of port@number schema, use port-function. This will allow to avoid ports node and #address-cells, #size-cells, reg properties. Additionally it should increase readability of the bindings. device { port-dsi { endpoint { ... }; }; port-rgb { endpoint { ... }; }; }; It is little bit like with gpios vs reset-gpios properties. Another advantage I see we do not need do mappings of port numbers to functions between dts, drivers and documentation. 2. Similar approach can be taken to endpoint nodes, in fact as endpoints are children of port node and as I understand port node have no other children we can use any name instead of endpoint@number, of course some convention can be helpful. device { port-dsi { ep-soc1 { ... }; ep-soc2 { ... }; }; port-rgb { ep-panel { ... }; }; }; I would like to add that those ideas would work nicely with Sylwester's proposition of skipping endpoints nodes in case there is only one endpoint - the most common cases are devices with one or two ports, each port having only one remote endpoint. The complete graph for DSI/LVDS bridge I work recently will look like: dsim {
Re: [PATCH v6 2/8] Documentation: of: Document graph bindings
Hi Grant, Am Freitag, den 07.03.2014, 18:27 + schrieb Grant Likely: On Wed, 5 Mar 2014 10:20:36 +0100, Philipp Zabel p.za...@pengutronix.de wrote: The device tree graph bindings as used by V4L2 and documented in Documentation/device-tree/bindings/media/video-interfaces.txt contain generic parts that are not media specific but could be useful for any subsystem with data flow between multiple devices. This document describes the generic bindings. Signed-off-by: Philipp Zabel p.za...@pengutronix.de See my comments on the previous version. My concerns are the handling of the optional 'ports' node and the usage of reverse links. would this change address your concern about the reverse links? As the preexisting video-interfaces.txt bindings mandate the reverse links, I worry about introducing a second, subtly different binding. It should be noted somewhere in video-interfaces.txt that the reverse links are deprecated for the but still supported by the code for backwards compatibility. diff --git a/Documentation/devicetree/bindings/graph.txt b/Documentation/devicetree/bindings/graph.txt index 1a69c07..eb6cae5 100644 --- a/Documentation/devicetree/bindings/graph.txt +++ b/Documentation/devicetree/bindings/graph.txt @@ -87,12 +87,13 @@ device { Links between endpoints --- -Each endpoint should contain a 'remote-endpoint' phandle property that points -to the corresponding endpoint in the port of the remote device. In turn, the -remote endpoint should contain a 'remote-endpoint' property. If it has one, -it must not point to another than the local endpoint. Two endpoints with their -'remote-endpoint' phandles pointing at each other form a link between the -containing ports. +Two endpoint nodes form a link between the two ports they are contained in +if one contains a 'remote-endpoint' phandle property, pointing to the other +endpoint. The endpoint pointed to should not contain a 'remote-endpoint' +property itself. Which direction the phandle should point in depends on the +device type. In general, links should be pointing outwards from central +devices that provide DMA memory interfaces, such as display controller, +video capture interface, or serial digital audio interface cores. device-1 { port { @@ -104,8 +105,8 @@ device-1 { device-2 { port { -device_2_input: endpoint { -remote-endpoint = device_1_output; +device_2_input: endpoint { }; + /* no remote-endpoint, this endpoint is pointed at */ }; }; }; regards Philipp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/3] [media] of: move graph helpers from drivers/media/v4l2-core to drivers/of
On 10/03/14 10:58, Andrzej Hajda wrote: I want to propose another solution to simplify bindings, in fact I have few ideas to consider: 1. Use named ports instead of address-cells/regs. Ie instead of port@number schema, use port-function. This will allow to avoid ports node and #address-cells, #size-cells, reg properties. Additionally it should increase readability of the bindings. device { port-dsi { endpoint { ... }; }; port-rgb { endpoint { ... }; }; }; It is little bit like with gpios vs reset-gpios properties. Another advantage I see we do not need do mappings of port numbers to functions between dts, drivers and documentation. That makes it more difficult to iterate the ports. You need to go through all the nodes and use partial name matching. I think for things like gpios, the driver always gives the full name, so there's no need for any kind of partial matching or searching. It looks nice when just looking at the DT, though. Tomi signature.asc Description: OpenPGP digital signature
[GIT PULL FOR v3.15] New flash faults, lm3560 cleanups, lm3646 driver
Hi Mauro, Here are some cleanups for lm3560 and a driver for lm3646 flash controller which also adds a few new flash fault control bits. The following changes since commit f2d7313534072a5fe192e7cf46204b413acef479: [media] drx-d: add missing braces in drxd_hard.c:DRXD_init (2014-03-09 09:20:50 -0300) are available in the git repository at: ssh://linuxtv.org/git/sailus/media_tree.git flash-for-3.15 for you to fetch changes up to 4473269ce76242fc13bd837ec299e8b588a43b7a: lm3646: add new dual LED Flash driver (2014-03-10 11:36:04 +0200) Andy Shevchenko (3): lm3560: remove FSF address from the license lm3560: keep style for the comments lm3560: prevent memory leak in case of pdata absence Daniel Jeong (3): v4l2-controls.h: Add addtional Flash fault bits controls.xml: Document addtional Flash fault bits lm3646: add new dual LED Flash driver Documentation/DocBook/media/v4l/controls.xml | 18 ++ drivers/media/i2c/Kconfig|9 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/lm3560.c | 22 +- drivers/media/i2c/lm3646.c | 414 ++ include/media/lm3646.h | 87 ++ include/uapi/linux/v4l2-controls.h |3 + 7 files changed, 541 insertions(+), 13 deletions(-) create mode 100644 drivers/media/i2c/lm3646.c create mode 100644 include/media/lm3646.h -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
problems with Cinergy HTC HD Rev. 2 (0x0ccd:0x0101) Conexant 231xx
Hi, i have some problems using the Terratec Cinergy HTC Stick HD Rev. 2 . The driver or the specification is currenty not implemented into driverset of media-build. All i know is, that there is a buildin Conexant Chip (I think the CX23102) and a SiliconLabs Tuner (SI2173). I already tried to adapt this knownledge combined with the configuration of the windows driver to the driver(cx231xx) but sadly without success. Hopefully someone can help me with that problem or may tell me what i did wrong. Thanks! [cx231xx-cards.c] .name = Terratec CINERGY HTC STICK HD Rev. 2, .tuner_type = TUNER_ABSENT,/**/ .tuner_addr = 0xc0,/**/ .decoder = CX231XX_AVDECODER, .output_mode = OUT_MODE_VIP11, .demod_xfer_mode = 0, .ctl_pin_status_mask = 0xFFC4, .agc_analog_digital_select_gpio = 0x11, .tuner_sif_gpio = -1, .tuner_scl_gpio = -1, .tuner_sda_gpio = -1, .gpio_pin_status_mask = 0x4001000, .tuner_i2c_master = 3, .demod_i2c_master = 2,/**/ .ir_i2c_master = 2, .has_dvb = 1, .demod_addr = 0xc8,/**/ .norm = V4L2_STD_NTSC_M, .input = {{ .type = CX231XX_VMUX_TELEVISION, .vmux = CX231XX_VIN_3_1, .amux = CX231XX_AMUX_VIDEO, .gpio = NULL, }, { .type = CX231XX_VMUX_COMPOSITE1, .vmux = CX231XX_VIN_2_1, .amux = CX231XX_AMUX_LINE_IN, .gpio = NULL, }, { .type = CX231XX_VMUX_SVIDEO, .vmux = CX231XX_VIN_1_1 | (CX231XX_VIN_1_2 8) | CX25840_SVIDEO_ON, .amux = CX231XX_AMUX_LINE_IN, .gpio = NULL, } }, }, == [cinergy_htc_stick_hd.inf (orig: cxPolaris.inf)] ; Sony format = 1, Philips format = 0 HKR,DriverData,I2SInputFormat,0x00010001, 0x00, 0x00, 0x00, 0x00 HKR,DriverData,EnableAFAudio,0x00010001, 0x01, 0x00, 0x00, 0x00 HKR,DriverData,GpioMaskFM,0x00010001, 0x01, 0x00, 0x00, 0x00 HKR,DriverData,GpioSettingFM,0x00010001, 0x01, 0x00, 0x00, 0x00 HKR,DriverData,VideoStandard,0x00010001, 0x20,0x00,0x00,0x00 ;Gain has 7 levels in volume up and down directions. each level gives 6db gain or attenuation ;for attenuation we need to use 2's complement values. ;for Gain we use levels 1-7, for attenuation we use levels 0x9(-7) - 0xF (-1) HKR,DriverData,FMAudioGain,0x00010001, 0x1, 0x00, 0x00, 0x00 ; for Polaris testing, select HANC to transfer Audio HKR,DriverData,EnableHANCAudioOut,0x00010001, 0x00, 0x00, 0x00, 0x00 ; GPIO Pin values - ; IMPORTANT !!! if any GPIO is not used - just delete the corresponding entry !!! ; AGC_Analog_Digitial_Select_Gpio_Bit is controlled by GPIO 28 HKR,DriverData,AGC_Analog_Digitial_Select_Gpio_Bit, 0x00010001, 0x1C, 0x00, 0x00, 0x00 ;gpio pin status mask, useless pin bit is 0 ;useful pin bit is 1 HKR,DriverData,gpio_pin_status_mask,0x00010001, 0x4001000 ;PIN_CTRL pin status mask, useless pin bit is 0 ;useful pin bit is 1 HKR,DriverData,ctrl_pin_status_mask,0x00010001, 0xFFC4 ;Safely_Remove_Hardware=1: enable, by default ;Safely_Remove_Hardware=0: disable, then Polaris will not appear in the safely remove hardware icon list. HKR,DriverData,Safely_Remove_Hardware,0x00010001, 0x01, 0x00, 0x00, 0x00 ;--- ; Crossbar AddReg sections ; ;---Crossbar registry values--- ; ; Note: For each pin on the crossbar, specify the following: ; (1) Pin type ; 0 - SVIDEO ; 1 - Tuner ; 2 - Composite ; 3 - audio tuner in ; 4 - audio line in ; 7 - YUV ; (2) InputMux - input mux to use for the selected pin ; (3) RelatedPinIndex ;--- ;---Crossbar registry values--- ;Pin 0 - Tuner In ; Input Mux : VIN2_1 for Tuner input from NXP18271 HKR,DriverData\XBarPin0,PinType,0x00010001, 0x01,0x00,0x00,0x00 HKR,DriverData\XBarPin0,InputMux,0x00010001, 0x03,0x00,0x00,0x00 HKR,DriverData\XBarPin0,RelatedPinIndex,0x00010001, 0x03,0x00,0x00,0x00 ;Pin 1 - Composite in ; Input Mux : VIN2_1 for Composite HKR,DriverData\XBarPin1,PinType,0x00010001, 0x02,0x00,0x00,0x00 HKR,DriverData\XBarPin1,InputMux,0x00010001, 0x02,0x00,0x00,0x00 HKR,DriverData\XBarPin1,RelatedPinIndex,0x00010001, 0x04,0x00,0x00,0x00 ;Pin 2 - S-Video in ; Input Mux : VIN1_1 for Luma, VIN1_2 for Chroma HKR,DriverData\XBarPin2,PinType,0x00010001, 0x00,0x00,0x00,0x00 HKR,DriverData\XBarPin2,InputMux,0x00010001, 0x01,0x01,0x00,0x00 HKR,DriverData\XBarPin2,RelatedPinIndex,0x00010001, 0x04,0x00,0x00,0x00 ;Pin 3 - Tuner Audio HKR,DriverData\XBarPin3,PinType,0x00010001, 0x03,0x00,0x00,0x00 HKR,DriverData\XBarPin3,InputMux,0x00010001, 0x00,0x00,0x00,0x00
Re: [REVIEW PATCH 11/13] DocBook: document RF tuner bandwidth controls
On 05.03.2014 20:49, Mauro Carvalho Chehab wrote: Em Thu, 27 Feb 2014 02:22:06 +0200 Antti Palosaari cr...@iki.fi escreveu: Add documentation for RF tuner bandwidth controls. These controls are used to set filters on tuner signal path. Cc: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Antti Palosaari cr...@iki.fi --- Documentation/DocBook/media/v4l/controls.xml | 19 +++ 1 file changed, 19 insertions(+) diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml index 6c9dbf6..5550fea 100644 --- a/Documentation/DocBook/media/v4l/controls.xml +++ b/Documentation/DocBook/media/v4l/controls.xml @@ -5007,6 +5007,25 @@ descriptor. Calling VIDIOC-QUERYCTRL; for this control will return a description of this control class./entry /row row + entry spanname=idconstantV4L2_CID_RF_TUNER_BANDWIDTH_AUTO/constantnbsp;/entry + entryboolean/entry +/row +row + entry spanname=descrEnables/disables tuner radio channel +bandwidth configuration. In automatic mode bandwidth configuration is performed +by the driver./entry +/row +row + entry spanname=idconstantV4L2_CID_RF_TUNER_BANDWIDTH/constantnbsp;/entry + entryinteger/entry +/row +row + entry spanname=descrFilter(s) on tuner signal path are used to +filter signal according to receiving party needs. Driver configures filters to +fulfill desired bandwidth requirement. Used when V4L2_CID_RF_TUNER_BANDWIDTH_AUTO is not +set. The range and step are driver-specific./entry Huh? If this is enable/disable, why the range and step are driver-specific? Because there is two controls grouped. That is situation of having AUTO/MANUAL. V4L2_CID_RF_TUNER_BANDWIDTH_AUTO V4L2_CID_RF_TUNER_BANDWIDTH V4L2_CID_RF_TUNER_BANDWIDTH is valid only when V4L2_CID_RF_TUNER_BANDWIDTH_AUTO == false. regards Antti +/row +row entry spanname=idconstantV4L2_CID_RF_TUNER_LNA_GAIN_AUTO/constantnbsp;/entry entryboolean/entry /row -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
On 08/03/14 13:41, Grant Likely wrote: Ok. If we go for single directional link, the question is then: which way? And is the direction different for display and camera, which are kind of reflections of each other? In general I would recommend choosing whichever device you would sensibly think of as a master. In the camera case I would choose the camera controller node instead of the camera itself, and in the display case I would choose the display controller instead of the panel. The binding author needs to choose what she things makes the most sense, but drivers can still use if it it turns out to be 'backwards' I would perhaps choose the same approach, but at the same time I think it's all but clear. The display controller doesn't control the panel any more than a DMA controller controls, say, the display controller. In fact, in earlier versions of OMAP DSS DT support I had a simpler port description, and in that I had the panel as the master (i.e. link from panel to dispc) because the panel driver uses the display controller's features to provide the panel device a data stream. And even with the current OMAP DSS DT version, which uses the v4l2 style ports/endpoints, the driver model is still the same, and only links towards upstream are used. So one reason I'm happy with the dual-linking is that I can easily follow the links from the downstream entities to upstream entities, and other people, who have different driver model, can easily do the opposite. But I agree that single-linking is enough and this can be handled at runtime, even if it makes the code more complex. And perhaps requires extra data in the dts, to give the start points for the graph. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH] rtl2832u_sdr: fixing v4l2-compliance issues
Moikka Hans! I will look these issues today. Actually I forget to ran whole v4l2-compliance after I added these controls... I ran it for basic API and it failed there too in many places, IIRC mostly those were related to new tuner types. regards Antti On 06.03.2014 01:21, Hans Verkuil wrote: Antti, Attached is a patch that fixed all but one v4l2-compliance error: fail: v4l2-test-controls.cpp(295): returned control value out of range fail: v4l2-test-controls.cpp(357): invalid control 00a2090c test VIDIOC_G/S_CTRL: FAIL fail: v4l2-test-controls.cpp(465): returned control value out of range fail: v4l2-test-controls.cpp(573): invalid control 00a2090c test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL That's the BANDWIDTH control and it returned value 320 when the minimum was 600. I couldn't trace where that came from in the limited time I spent on it, I expect you can find it much quicker. I did my testing with this tree: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/sdr which is a merge of your tree: http://git.linuxtv.org/media-tree.git/shortlog/refs/heads/sdr and my ongoing vb2-fixes tree: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/vb2-part4 I leave it to you to process this patch further. Regards, Hans Signed-off-by: Hans Verkuil hans.verk...@cisco.com diff --git a/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c b/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c index ac487cb..3013305 100644 --- a/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c +++ b/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c @@ -120,6 +120,7 @@ struct rtl2832_sdr_state { struct vb2_queue vb_queue; struct list_head queued_bufs; spinlock_t queued_bufs_lock; /* Protects queued_bufs */ + unsigned sequence; /* buffer sequence counter */ /* Note if taking both locks v4l2_lock must always be locked first! */ struct mutex v4l2_lock; /* Protects everything else */ @@ -415,6 +416,8 @@ static void rtl2832_sdr_urb_complete(struct urb *urb) len = rtl2832_sdr_convert_stream(s, ptr, urb-transfer_buffer, urb-actual_length); vb2_set_plane_payload(fbuf-vb, 0, len); + v4l2_get_timestamp(fbuf-vb.v4l2_buf.timestamp); + fbuf-vb.v4l2_buf.sequence = s-sequence++; vb2_buffer_done(fbuf-vb, VB2_BUF_STATE_DONE); } skip: @@ -611,8 +614,9 @@ static int rtl2832_sdr_queue_setup(struct vb2_queue *vq, struct rtl2832_sdr_state *s = vb2_get_drv_priv(vq); dev_dbg(s-udev-dev, %s: *nbuffers=%d\n, __func__, *nbuffers); - /* Absolute min and max number of buffers available for mmap() */ - *nbuffers = clamp_t(unsigned int, *nbuffers, 8, 32); + /* Need at least 8 buffers */ + if (vq-num_buffers + *nbuffers 8) + *nbuffers = 8 - vq-num_buffers; *nplanes = 1; /* 2 = max 16-bit sample returned */ sizes[0] = PAGE_ALIGN(BULK_BUFFER_SIZE * 2); @@ -1013,6 +1017,8 @@ static int rtl2832_sdr_start_streaming(struct vb2_queue *vq, unsigned int count) if (ret) goto err; + s-sequence = 0; + ret = rtl2832_sdr_submit_urbs(s); if (ret) goto err; @@ -1087,6 +1093,8 @@ static int rtl2832_sdr_s_tuner(struct file *file, void *priv, struct rtl2832_sdr_state *s = video_drvdata(file); dev_dbg(s-udev-dev, %s:\n, __func__); + if (v-index 1) + return -EINVAL; return 0; } @@ -1122,12 +1130,15 @@ static int rtl2832_sdr_g_frequency(struct file *file, void *priv, dev_dbg(s-udev-dev, %s: tuner=%d type=%d\n, __func__, f-tuner, f-type); - if (f-tuner == 0) + if (f-tuner == 0) { f-frequency = s-f_adc; - else if (f-tuner == 1) + f-type = V4L2_TUNER_ADC; + } else if (f-tuner == 1) { f-frequency = s-f_tuner; - else + f-type = V4L2_TUNER_RF; + } else { return -EINVAL; + } return ret; } @@ -1161,7 +1172,9 @@ static int rtl2832_sdr_s_frequency(struct file *file, void *priv, __func__, s-f_adc); ret = rtl2832_sdr_set_adc(s); } else if (f-tuner == 1) { - s-f_tuner = f-frequency; + s-f_tuner = clamp_t(unsigned int, f-frequency, + bands_fm[0].rangelow, + bands_fm[0].rangehigh); dev_dbg(s-udev-dev, %s: RF frequency=%u Hz\n, __func__, f-frequency); @@ -1195,6 +1208,7 @@ static int rtl2832_sdr_g_fmt_sdr_cap(struct file *file, void *priv, dev_dbg(s-udev-dev, %s:\n, __func__); f-fmt.sdr.pixelformat = s-pixelformat; +
Re: [PATCH v6 2/8] Documentation: of: Document graph bindings
Hi Philipp, On Monday 10 March 2014 10:28:10 Philipp Zabel wrote: Hi Grant, Am Freitag, den 07.03.2014, 18:27 + schrieb Grant Likely: On Wed, 5 Mar 2014 10:20:36 +0100, Philipp Zabel wrote: The device tree graph bindings as used by V4L2 and documented in Documentation/device-tree/bindings/media/video-interfaces.txt contain generic parts that are not media specific but could be useful for any subsystem with data flow between multiple devices. This document describes the generic bindings. Signed-off-by: Philipp Zabel p.za...@pengutronix.de See my comments on the previous version. My concerns are the handling of the optional 'ports' node and the usage of reverse links. would this change address your concern about the reverse links? As the preexisting video-interfaces.txt bindings mandate the reverse links, I worry about introducing a second, subtly different binding. It should be noted somewhere in video-interfaces.txt that the reverse links are deprecated for the but still supported by the code for backwards compatibility. I'm very much against removing the reverse links. Without them the graph will become much more complex to parse. You can try to convince me, but for now I'm afraid it's a NACK. diff --git a/Documentation/devicetree/bindings/graph.txt b/Documentation/devicetree/bindings/graph.txt index 1a69c07..eb6cae5 100644 --- a/Documentation/devicetree/bindings/graph.txt +++ b/Documentation/devicetree/bindings/graph.txt @@ -87,12 +87,13 @@ device { Links between endpoints --- -Each endpoint should contain a 'remote-endpoint' phandle property that points -to the corresponding endpoint in the port of the remote device. In turn, the -remote endpoint should contain a 'remote-endpoint' property. If it has one, -it must not point to another than the local endpoint. Two endpoints with their -'remote-endpoint' phandles pointing at each other form a link between the -containing ports. +Two endpoint nodes form a link between the two ports they are contained in +if one contains a 'remote-endpoint' phandle property, pointing to the other +endpoint. The endpoint pointed to should not contain a 'remote-endpoint' +property itself. Which direction the phandle should point in depends on the +device type. In general, links should be pointing outwards from central +devices that provide DMA memory interfaces, such as display controller, +video capture interface, or serial digital audio interface cores. device-1 { port { @@ -104,8 +105,8 @@ device-1 { device-2 { port { -device_2_input: endpoint { -remote-endpoint = device_1_output; +device_2_input: endpoint { }; + /* no remote-endpoint, this endpoint is pointed at */ }; }; }; -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/3] [media] of: move graph helpers from drivers/media/v4l2-core to drivers/of
Hi Andrzej, On Monday 10 March 2014 09:58:07 Andrzej Hajda wrote: On 03/08/2014 04:54 PM, Laurent Pinchart wrote: On Saturday 08 March 2014 13:07:23 Philipp Zabel wrote: On Fri, Mar 7, 2014 at 6:18 PM, Grant Likely wrote: On Wed, 26 Feb 2014 16:24:57 +0100, Philipp Zabel wrote: The 'ports' node is optional. It is only needed if the parent node has its own #address-cells and #size-cells properties. If the ports are direct children of the device node, there might be other nodes than ports: device { #address-cells = 1; #size-cells = 0; port@0 { endpoint { ... }; }; port@1 { endpoint { ... }; }; some-other-child { ... }; }; device { #address-cells = x; #size-cells = y; ports { #address-cells = 1; #size-cells = 0; port@0 { endpoint { ... }; }; port@1 { endpoint { ... }; }; }; some-other-child { ... }; }; From a pattern perspective I have no problem with that From an individual driver binding perspective that is just dumb! It's fine for the ports node to be optional, but an individual driver using the binding should be explicit about which it will accept. Please use either a flag or a separate wrapper so that the driver can select the behaviour. If the generic binding exists in both forms, most drivers should be able to cope with both. Maybe it should be mentioned in the bindings that the short form without ports node should be used where possible (i.e. for devices that don't already have #address,size-cells != 1,0). Having a separate wrapper to enforce the ports node for devices that need it might be useful. The helper should find the two endpoints in both cases. Tomi suggests an even more compact form for devices with just one port: device { endpoint { ... }; some-other-child { ... }; }; That's fine. In that case the driver would specifically require the endpoint to be that one node although the above looks a little weird to me. I would recommend that if there are other non-port child nodes then the ports should still be encapsulated by a ports node. The device binding should not be ambiguous about which nodes are ports. Sylwester suggested as an alternative, if I understood correctly, to drop the endpoint node and instead keep the port: device-a { implicit_output_ep: port { remote-endpoint = explicit_input_ep; }; }; device-b { port { explicit_input_ep: endpoint { remote-endpoint = implicit_output_ep; }; }; }; This would have the advantage to reduce verbosity for devices with multiple ports that are only connected via one endport each, and you'd always have the connected ports in the device tree as 'port' nodes. I like that idea. I would prefer making the 'port' nodes mandatory and the 'ports' and 'endpoint' nodes optional. Leaving the 'port' node out slightly decreases readability in my opinion, but making the 'endpoint' node optional increases it. That's just my point of view though. I want to propose another solution to simplify bindings, in fact I have few ideas to consider: 1. Use named ports instead of address-cells/regs. Ie instead of port@number schema, use port-function. This will allow to avoid ports node and #address-cells, #size-cells, reg properties. Additionally it should increase readability of the bindings. device { port-dsi { endpoint { ... }; }; port-rgb { endpoint { ... }; }; }; It is little bit like with gpios vs reset-gpios properties. Another advantage I see we do not need do mappings of port numbers to functions between dts, drivers and documentation. The problem with this approach is that ports are identified by a number inside the kernel, so we would still need to define name to number mappings, or switch to port names internally first. 2. Similar approach can be taken to endpoint nodes, in fact as endpoints are children of port node and as I understand port node have no other children we can use any name instead of endpoint@number, of course some convention can be helpful. device { port-dsi { ep-soc1 { ... }; ep-soc2 { ... }; }; port-rgb { ep-panel { ...
[PATCH 09/15] drx-j: get rid of some unused vars
As reported when compiled with W=1: drivers/media/dvb-frontends/drx39xyj/drxj.c: In function ‘ctrl_set_channel’: drivers/media/dvb-frontends/drx39xyj/drxj.c:10340:26: warning: variable ‘common_attr’ set but not used [-Wunused-but-set-variable] struct drx_common_attr *common_attr = NULL; ^ drivers/media/dvb-frontends/drx39xyj/drxj.c:10336:6: warning: variable ‘intermediate_freq’ set but not used [-Wunused-but-set-variable] s32 intermediate_freq = 0; Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index a26ddc9fa2bc..a36cfa684153 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -10333,11 +10333,9 @@ ctrl_set_channel(struct drx_demod_instance *demod, struct drx_channel *channel) { int rc; s32 tuner_freq_offset = 0; - s32 intermediate_freq = 0; struct drxj_data *ext_attr = NULL; struct i2c_device_addr *dev_addr = NULL; enum drx_standard standard = DRX_STANDARD_UNKNOWN; - struct drx_common_attr *common_attr = NULL; #ifndef DRXJ_VSB_ONLY u32 min_symbol_rate = 0; u32 max_symbol_rate = 0; @@ -10348,7 +10346,6 @@ ctrl_set_channel(struct drx_demod_instance *demod, struct drx_channel *channel) if ((demod == NULL) || (channel == NULL)) return -EINVAL; - common_attr = (struct drx_common_attr *) demod-my_common_attr; dev_addr = demod-my_i2c_dev_addr; ext_attr = (struct drxj_data *) demod-my_ext_attr; standard = ext_attr-standard; @@ -10506,7 +10503,6 @@ ctrl_set_channel(struct drx_demod_instance *demod, struct drx_channel *channel) } tuner_freq_offset = 0; - intermediate_freq = demod-my_common_attr-intermediate_freq; /*== Setup demod for specific standard */ switch (standard) { -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 14/15] drx-j: Fix detection of no signal
When the signal is 7, it means that no signal was received. Value experimentally measured. Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index 9958277dd943..8098d87cda0b 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -9420,6 +9420,9 @@ static int get_sig_strength(struct drx_demod_instance *demod, u16 *sig_strength) *sig_strength = (20 * if_gain / if_agc_sns); } + if (*sig_strength = 7) + *sig_strength = 0; + return 0; rw_error: return -EIO; -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/15] drx-j: Cleanups, fixes and support for DVBv5 stats
This patch series is meant to: 1) fix some reported issues (sparse, smatch); 2) Fix one compilation issue with em28xx when drx-j is not selected; 3) Get rid of unused code. It is always possible to restore the code from git history. Removing the unused code helps to better understand what's actually there. 4) Add support for DVBv5 stats. On my tests here with an AWGN noise generator, the CNR measure made by drx-j was close enough to the SNR injected (with a difference of about 1 dB). Mauro Carvalho Chehab (15): drx-j: Don't use 0 as NULL drx-j: Fix dubious usage of instead of drx39xxj.h: Fix undefined reference to attach function drx-j: don't use mc_info before checking if its not NULL drx-j: get rid of dead code drx-j: remove external symbols drx-j: Fix usage of drxj_close() drx-j: propagate returned error from request_firmware() drx-j: get rid of some unused vars drx-j: Don't use state for DVB lock state drx-j: re-add get_sig_strength() drx-j: Prepare to use DVBv5 stats drx-j: properly handle bit counts on stats drx-j: Fix detection of no signal drx-j: enable DVBv5 stats drivers/media/dvb-frontends/drx39xyj/drx39xxj.h | 6 + drivers/media/dvb-frontends/drx39xyj/drx_driver.h |24 - drivers/media/dvb-frontends/drx39xyj/drxj.c | 11146 +++- drivers/media/dvb-frontends/drx39xyj/drxj.h |30 - 4 files changed, 1343 insertions(+), 9863 deletions(-) -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/15] drx-j: re-add get_sig_strength()
We'll need to use this function. Restore it from the git history. This function will be used on the next patch. Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 77 + 1 file changed, 77 insertions(+) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index 7022a69f56bd..ca807b1fc67c 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -9352,6 +9352,83 @@ rw_error: /**/ /** + * \fn int get_sig_strength() + * \brief Retrieve signal strength for VSB and QAM. + * \param demod Pointer to demod instance + * \param u16-t Pointer to signal strength data; range 0, .. , 100. + * \return int. + * \retval 0 sig_strength contains valid data. + * \retval -EINVAL sig_strength is NULL. + * \retval -EIO Erroneous data, sig_strength contains invalid data. + */ +#define DRXJ_AGC_TOP0x2800 +#define DRXJ_AGC_SNS0x1600 +#define DRXJ_RFAGC_MAX 0x3fff +#define DRXJ_RFAGC_MIN 0x800 + +static int get_sig_strength(struct drx_demod_instance *demod, u16 *sig_strength) +{ + struct i2c_device_addr *dev_addr = demod-my_i2c_dev_addr; + int rc; + u16 rf_gain = 0; + u16 if_gain = 0; + u16 if_agc_sns = 0; + u16 if_agc_top = 0; + u16 rf_agc_max = 0; + u16 rf_agc_min = 0; + + rc = drxj_dap_read_reg16(dev_addr, IQM_AF_AGC_IF__A, if_gain, 0); + if (rc != 0) { + pr_err(error %d\n, rc); + goto rw_error; + } + if_gain = IQM_AF_AGC_IF__M; + rc = drxj_dap_read_reg16(dev_addr, IQM_AF_AGC_RF__A, rf_gain, 0); + if (rc != 0) { + pr_err(error %d\n, rc); + goto rw_error; + } + rf_gain = IQM_AF_AGC_RF__M; + + if_agc_sns = DRXJ_AGC_SNS; + if_agc_top = DRXJ_AGC_TOP; + rf_agc_max = DRXJ_RFAGC_MAX; + rf_agc_min = DRXJ_RFAGC_MIN; + + if (if_gain if_agc_top) { + if (rf_gain rf_agc_max) + *sig_strength = 100; + else if (rf_gain rf_agc_min) { + if (rf_agc_max == rf_agc_min) { + pr_err(error: rf_agc_max == rf_agc_min\n); + return -EIO; + } + *sig_strength = + 75 + 25 * (rf_gain - rf_agc_min) / (rf_agc_max - + rf_agc_min); + } else + *sig_strength = 75; + } else if (if_gain if_agc_sns) { + if (if_agc_top == if_agc_sns) { + pr_err(error: if_agc_top == if_agc_sns\n); + return -EIO; + } + *sig_strength = + 20 + 55 * (if_gain - if_agc_sns) / (if_agc_top - if_agc_sns); + } else { + if (!if_agc_sns) { + pr_err(error: if_agc_sns is zero!\n); + return -EIO; + } + *sig_strength = (20 * if_gain / if_agc_sns); + } + + return 0; + rw_error: + return -EIO; +} + +/** * \fn int ctrl_get_qam_sig_quality() * \brief Retreive QAM signal quality from device. * \param devmod Pointer to demodulator instance. -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/15] drx-j: don't use mc_info before checking if its not NULL
smatch warning: drivers/media/dvb-frontends/drx39xyj/drxj.c:20803 drx_ctrl_u_code() warn: variable dereferenced before check 'mc_info' (see line 20800) Reported-by: Dan Carpenter dan.carpen...@oracle.com Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index 1e6dab7e5892..a8fd53b612ae 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -20208,12 +20208,14 @@ static int drx_ctrl_u_code(struct drx_demod_instance *demod, const u8 *mc_data_init = NULL; u8 *mc_data = NULL; unsigned size; - char *mc_file = mc_info-mc_file; + char *mc_file; /* Check arguments */ - if (!mc_info || !mc_file) + if (!mc_info || !mc_info-mc_file) return -EINVAL; + mc_file = mc_info-mc_file; + if (!demod-firmware) { const struct firmware *fw = NULL; -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/15] drx-j: Fix dubious usage of instead of
Fixes the following warnings: drivers/media/dvb-frontends/drx39xyj/drxj.c:16764:68: warning: dubious: x !y drivers/media/dvb-frontends/drx39xyj/drxj.c:16778:68: warning: dubious: x !y drivers/media/dvb-frontends/drx39xyj/drxj.c:16797:68: warning: dubious: x !y Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index af3b69ce8c16..1e6dab7e5892 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -16762,12 +16762,12 @@ static int ctrl_set_oob(struct drx_demod_instance *demod, struct drxoob *oob_par case DRX_OOB_MODE_A: if ( /* signal is transmitted inverted */ - ((oob_param-spectrum_inverted == true) + ((oob_param-spectrum_inverted == true) /* and tuner is not mirroring the signal */ (!mirror_freq_spect_oob)) | /* or */ /* signal is transmitted noninverted */ - ((oob_param-spectrum_inverted == false) + ((oob_param-spectrum_inverted == false) /* and tuner is mirroring the signal */ (mirror_freq_spect_oob)) ) @@ -16780,12 +16780,12 @@ static int ctrl_set_oob(struct drx_demod_instance *demod, struct drxoob *oob_par case DRX_OOB_MODE_B_GRADE_A: if ( /* signal is transmitted inverted */ - ((oob_param-spectrum_inverted == true) + ((oob_param-spectrum_inverted == true) /* and tuner is not mirroring the signal */ (!mirror_freq_spect_oob)) | /* or */ /* signal is transmitted noninverted */ - ((oob_param-spectrum_inverted == false) + ((oob_param-spectrum_inverted == false) /* and tuner is mirroring the signal */ (mirror_freq_spect_oob)) ) @@ -16799,12 +16799,12 @@ static int ctrl_set_oob(struct drx_demod_instance *demod, struct drxoob *oob_par default: if ( /* signal is transmitted inverted */ - ((oob_param-spectrum_inverted == true) + ((oob_param-spectrum_inverted == true) /* and tuner is not mirroring the signal */ (!mirror_freq_spect_oob)) | /* or */ /* signal is transmitted noninverted */ - ((oob_param-spectrum_inverted == false) + ((oob_param-spectrum_inverted == false) /* and tuner is mirroring the signal */ (mirror_freq_spect_oob)) ) -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/15] drx-j: Prepare to use DVBv5 stats
Convert the stats internally to use DVBv5. For now, it will keep showing everything via DVBv3 API only, as the .len value were not initialized. That allows testing if the new stats code didn't break anything. A latter patch will add the final bits for the DVBv5 stats to fully work. Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drx_driver.h | 24 -- drivers/media/dvb-frontends/drx39xyj/drxj.c | 321 +- 2 files changed, 125 insertions(+), 220 deletions(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drx_driver.h b/drivers/media/dvb-frontends/drx39xyj/drx_driver.h index e54eb35b52d9..9076bf21cc8a 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drx_driver.h +++ b/drivers/media/dvb-frontends/drx39xyj/drx_driver.h @@ -1033,30 +1033,6 @@ struct drx_channel { /**/ -/** -* \struct struct drx_sig_quality * Signal quality metrics. -* -* Used by DRX_CTRL_SIG_QUALITY. -*/ -struct drx_sig_quality { - u16 MER; /** in steps of 0.1 dB*/ - u32 pre_viterbi_ber; - /** in steps of 1/scale_factor_ber */ - u32 post_viterbi_ber; - /** in steps of 1/scale_factor_ber */ - u32 scale_factor_ber; - /** scale factor for BER */ - u16 packet_error; - /** number of packet errors */ - u32 post_reed_solomon_ber; - /** in steps of 1/scale_factor_ber */ - u32 pre_ldpc_ber; - /** in steps of 1/scale_factor_ber */ - u32 aver_iter;/** in steps of 0.01 */ - u16 indicator; - /** indicative signal quality low=0..100=high */ -}; - enum drx_cfg_sqi_speed { DRX_SQI_SPEED_FAST = 0, DRX_SQI_SPEED_MEDIUM, diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index ca807b1fc67c..6005e344f66c 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -8656,10 +8656,12 @@ rw_error: } /**/ -static int -ctrl_get_qam_sig_quality(struct drx_demod_instance *demod, struct drx_sig_quality *sig_quality); +static int ctrl_get_qam_sig_quality(struct drx_demod_instance *demod); + static int qam_flip_spec(struct drx_demod_instance *demod, struct drx_channel *channel) { + struct i2c_device_addr *dev_addr = demod-my_i2c_dev_addr; + struct drxj_data *ext_attr = demod-my_ext_attr; int rc; u32 iqm_fs_rate_ofs = 0; u32 iqm_fs_rate_lo = 0; @@ -8669,11 +8671,6 @@ static int qam_flip_spec(struct drx_demod_instance *demod, struct drx_channel *c u16 fsm_state = 0; int i = 0; int ofsofs = 0; - struct i2c_device_addr *dev_addr = NULL; - struct drxj_data *ext_attr = NULL; - - dev_addr = demod-my_i2c_dev_addr; - ext_attr = (struct drxj_data *) demod-my_ext_attr; /* Silence the controlling of lc, equ, and the acquisition state machine */ rc = drxj_dap_read_reg16(dev_addr, SCU_RAM_QAM_CTL_ENA__A, qam_ctl_ena, 0); @@ -8858,8 +8855,10 @@ qam64auto(struct drx_demod_instance *demod, struct drx_channel *channel, s32 tuner_freq_offset, enum drx_lock_status *lock_status) { - struct drx_sig_quality sig_quality; - struct drxj_data *ext_attr = NULL; + struct drxj_data *ext_attr = demod-my_ext_attr; + struct i2c_device_addr *dev_addr = demod-my_i2c_dev_addr; + struct drx39xxj_state *state = dev_addr-user_data; + struct dtv_frontend_properties *p = state-frontend.dtv_property_cache; int rc; u32 lck_state = NO_LOCK; u32 start_time = 0; @@ -8868,7 +8867,6 @@ qam64auto(struct drx_demod_instance *demod, u16 data = 0; /* external attributes for storing aquired channel constellation */ - ext_attr = (struct drxj_data *) demod-my_ext_attr; *lock_status = DRX_NOT_LOCKED; start_time = jiffies_to_msecs(jiffies); lck_state = NO_LOCK; @@ -8882,12 +8880,12 @@ qam64auto(struct drx_demod_instance *demod, switch (lck_state) { case NO_LOCK: if (*lock_status == DRXJ_DEMOD_LOCK) { - rc = ctrl_get_qam_sig_quality(demod, sig_quality); + rc = ctrl_get_qam_sig_quality(demod); if (rc != 0) { pr_err(error %d\n, rc); goto rw_error; } - if (sig_quality.MER 208) { + if (p-cnr.stat[0].svalue 20800) {
[PATCH 06/15] drx-j: remove external symbols
This driver doesn't export any external symbol, except for the attach() method. Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 12 ++-- drivers/media/dvb-frontends/drx39xyj/drxj.h | 30 - 2 files changed, 6 insertions(+), 36 deletions(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index e8c890800904..828d0527f38d 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -561,7 +561,7 @@ static int drxdap_fasi_write_reg32(struct i2c_device_addr *dev_addr, u32 addr, u32 data, u32 flags); -struct drxj_data drxj_data_g = { +static struct drxj_data drxj_data_g = { false, /* has_lna : true if LNA (aka PGA) present */ false, /* has_oob : true if OOB supported */ false, /* has_ntsc: true if NTSC supported */ @@ -810,7 +810,7 @@ struct drxj_data drxj_data_g = { * \var drxj_default_addr_g * \brief Default I2C address and device identifier. */ -struct i2c_device_addr drxj_default_addr_g = { +static struct i2c_device_addr drxj_default_addr_g = { DRXJ_DEF_I2C_ADDR, /* i2c address */ DRXJ_DEF_DEMOD_DEV_ID /* device id */ }; @@ -819,7 +819,7 @@ struct i2c_device_addr drxj_default_addr_g = { * \var drxj_default_comm_attr_g * \brief Default common attributes of a drxj demodulator instance. */ -struct drx_common_attr drxj_default_comm_attr_g = { +static struct drx_common_attr drxj_default_comm_attr_g = { NULL, /* ucode file */ true, /* ucode verify switch */ {0},/* version record */ @@ -890,7 +890,7 @@ struct drx_common_attr drxj_default_comm_attr_g = { * \var drxj_default_demod_g * \brief Default drxj demodulator instance. */ -struct drx_demod_instance drxj_default_demod_g = { +static struct drx_demod_instance drxj_default_demod_g = { drxj_default_addr_g, /* i2c address device id */ drxj_default_comm_attr_g, /* demod common attributes */ drxj_data_g/* demod device specific attributes */ @@ -11291,7 +11291,7 @@ static int drx_ctrl_u_code(struct drx_demod_instance *demod, * */ -int drxj_open(struct drx_demod_instance *demod) +static int drxj_open(struct drx_demod_instance *demod) { struct i2c_device_addr *dev_addr = NULL; struct drxj_data *ext_attr = NULL; @@ -11504,7 +11504,7 @@ rw_error: * \return Status_t Return status. * */ -int drxj_close(struct drx_demod_instance *demod) +static int drxj_close(struct drx_demod_instance *demod) { struct i2c_device_addr *dev_addr = demod-my_i2c_dev_addr; int rc; diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.h b/drivers/media/dvb-frontends/drx39xyj/drxj.h index 6d46513b7169..55ad535197d2 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.h +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.h @@ -647,34 +647,4 @@ DEFINES (x == DRX_LOCK_STATE_2) ? sync lock : \ (Invalid)) -/*- -ENUM --*/ - -/*- -STRUCTS --*/ - -/*- -Exported FUNCTIONS --*/ - - int drxj_open(struct drx_demod_instance *demod); - int drxj_close(struct drx_demod_instance *demod); - int drxj_ctrl(struct drx_demod_instance *demod, -u32 ctrl, void *ctrl_data); - -/*- -Exported GLOBAL VARIABLES --*/ - extern struct drx_access_func drx_dap_drxj_funct_g; - extern struct drx_demod_func drxj_functions_g; - extern struct drxj_data drxj_data_g; - extern struct i2c_device_addr drxj_default_addr_g; - extern struct drx_common_attr drxj_default_comm_attr_g; - extern struct drx_demod_instance drxj_default_demod_g; - -/*- -THE END --*/ #endif /* __DRXJ_H__ */ -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/15] drx39xxj.h: Fix undefined reference to attach function
As reported by the kbuild test robot fengguang...@intel.com: drivers/built-in.o: In function `em28xx_dvb_init': em28xx-dvb.c:(.text+0x876f2c): undefined reference to `drx39xxj_attach' That happens when CONFIG_VIDEO_EM28XX_DVB is selected, and neither CONFIG_MEDIA_SUBDRV_AUTOSELECT or DVB_DRX39XYJ is selected. Reported-by: kbuild test robot fengguang...@intel.com Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drx39xxj.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/dvb-frontends/drx39xyj/drx39xxj.h b/drivers/media/dvb-frontends/drx39xyj/drx39xxj.h index 2e0c50f0a12a..cfd0b96b6939 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drx39xxj.h +++ b/drivers/media/dvb-frontends/drx39xyj/drx39xxj.h @@ -34,6 +34,12 @@ struct drx39xxj_state { const struct firmware *fw; }; +#if IS_ENABLED(CONFIG_DVB_DRX39XYJ) struct dvb_frontend *drx39xxj_attach(struct i2c_adapter *i2c); +#else +static inline struct dvb_frontend *drx39xxj_attach(struct i2c_adapter *i2c) { + return NULL; +}; +#endif #endif /* DVB_DUMMY_FE_H */ -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/15] drx-j: properly handle bit counts on stats
Instead of just assuming that the min resolution is 1E-6, pass both bit error and bit counts for userspace to calculate BER. The same applies for PER, for 8VSB. It is not clear how to get the packet count for QAM. So, for now, don't expose PER for QAM. Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 89 - 1 file changed, 50 insertions(+), 39 deletions(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index 6005e344f66c..9958277dd943 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -6199,7 +6199,8 @@ rw_error: * \brief Get the values of packet error in 8VSB mode * \return Error code */ -static int get_vsb_post_rs_pck_err(struct i2c_device_addr *dev_addr, u16 *pck_errs) +static int get_vsb_post_rs_pck_err(struct i2c_device_addr *dev_addr, + u16 *pck_errs, u16 *pck_count) { int rc; u16 data = 0; @@ -6224,9 +6225,8 @@ static int get_vsb_post_rs_pck_err(struct i2c_device_addr *dev_addr, u16 *pck_er pr_err(error: period and/or prescale is zero!\n); return -EIO; } - *pck_errs = - (u16) frac_times1e6(packet_errors_mant * (1 packet_errors_exp), -(period * prescale * 77)); + *pck_errs = packet_errors_mant * (1 packet_errors_exp); + *pck_count = period * prescale * 77; return 0; rw_error: @@ -6238,7 +6238,8 @@ rw_error: * \brief Get the values of ber in VSB mode * \return Error code */ -static int get_vs_bpost_viterbi_ber(struct i2c_device_addr *dev_addr, u32 *ber) +static int get_vs_bpost_viterbi_ber(struct i2c_device_addr *dev_addr, + u32 *ber, u32 *cnt) { int rc; u16 data = 0; @@ -6259,19 +6260,17 @@ static int get_vs_bpost_viterbi_ber(struct i2c_device_addr *dev_addr, u32 *ber) bit_errors_exp = (data FEC_RS_NR_BIT_ERRORS_EXP__M) FEC_RS_NR_BIT_ERRORS_EXP__B; + *cnt = period * prescale * 207 * ((bit_errors_exp 2) ? 1 : 8); + if (((bit_errors_mant bit_errors_exp) 3) 68700) - *ber = 26570; + *ber = (*cnt) * 26570; else { if (period * prescale == 0) { pr_err(error: period and/or prescale is zero!\n); return -EIO; } - *ber = - frac_times1e6(bit_errors_mant -((bit_errors_exp - 2) ? (bit_errors_exp - 3) : bit_errors_exp), -period * prescale * 207 * -((bit_errors_exp 2) ? 1 : 8)); + *ber = bit_errors_mant ((bit_errors_exp 2) ? + (bit_errors_exp - 3) : bit_errors_exp); } return 0; @@ -6284,7 +6283,8 @@ rw_error: * \brief Get the values of ber in VSB mode * \return Error code */ -static int get_vs_bpre_viterbi_ber(struct i2c_device_addr *dev_addr, u32 *ber) +static int get_vs_bpre_viterbi_ber(struct i2c_device_addr *dev_addr, + u32 *ber, u32 *cnt) { u16 data = 0; int rc; @@ -6292,15 +6292,12 @@ static int get_vs_bpre_viterbi_ber(struct i2c_device_addr *dev_addr, u32 *ber) rc = drxj_dap_read_reg16(dev_addr, VSB_TOP_NR_SYM_ERRS__A, data, 0); if (rc != 0) { pr_err(error %d\n, rc); - goto rw_error; + return -EIO; } - *ber = - frac_times1e6(data, -VSB_TOP_MEASUREMENT_PERIOD * SYMBOLS_PER_SEGMENT); + *ber = data; + *cnt = VSB_TOP_MEASUREMENT_PERIOD * SYMBOLS_PER_SEGMENT; return 0; -rw_error: - return -EIO; } /** @@ -9289,7 +9286,8 @@ rw_error: * */ static int -get_qamrs_err_count(struct i2c_device_addr *dev_addr, struct drxjrs_errors *rs_errors) +get_qamrs_err_count(struct i2c_device_addr *dev_addr, + struct drxjrs_errors *rs_errors) { int rc; u16 nr_bit_errors = 0, @@ -9474,6 +9472,8 @@ ctrl_get_qam_sig_quality(struct drx_demod_instance *demod) u16 qam_vd_period = 0; /* Viterbi Measurement period */ u32 vd_bit_cnt = 0; /* ViterbiDecoder Bit Count */ + p-block_count.stat[0].scale = FE_SCALE_NOT_AVAILABLE; + /* read the physical registers */ /* Get the RS error data */ rc = get_qamrs_err_count(dev_addr, measuredrs_errors); @@ -9554,9 +9554,9 @@ ctrl_get_qam_sig_quality(struct drx_demod_instance *demod) QAM_VD_NR_SYMBOL_ERRORS_FIXED_MANT__B; if ((m e) 3 549752) - qam_vd_ser = 50; + qam_vd_ser = 50 * vd_bit_cnt * ((e 2) ? 1 : 8) / 8; else - qam_vd_ser = frac_times1e6(m
[PATCH 10/15] drx-j: Don't use state for DVB lock state
State is already used on other places for the state struct. Don't use it here, to avoid troubles with latter patches. Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index a36cfa684153..7022a69f56bd 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -8861,7 +8861,7 @@ qam64auto(struct drx_demod_instance *demod, struct drx_sig_quality sig_quality; struct drxj_data *ext_attr = NULL; int rc; - u32 state = NO_LOCK; + u32 lck_state = NO_LOCK; u32 start_time = 0; u32 d_locked_time = 0; u32 timeout_ofs = 0; @@ -8871,7 +8871,7 @@ qam64auto(struct drx_demod_instance *demod, ext_attr = (struct drxj_data *) demod-my_ext_attr; *lock_status = DRX_NOT_LOCKED; start_time = jiffies_to_msecs(jiffies); - state = NO_LOCK; + lck_state = NO_LOCK; do { rc = ctrl_lock_status(demod, lock_status); if (rc != 0) { @@ -8879,7 +8879,7 @@ qam64auto(struct drx_demod_instance *demod, goto rw_error; } - switch (state) { + switch (lck_state) { case NO_LOCK: if (*lock_status == DRXJ_DEMOD_LOCK) { rc = ctrl_get_qam_sig_quality(demod, sig_quality); @@ -,7 +,7 @@ qam64auto(struct drx_demod_instance *demod, goto rw_error; } if (sig_quality.MER 208) { - state = DEMOD_LOCKED; + lck_state = DEMOD_LOCKED; /* some delay to see if fec_lock possible TODO find the right value */ timeout_ofs += DRXJ_QAM_DEMOD_LOCK_EXT_WAITTIME;/* see something, waiting longer */ d_locked_time = jiffies_to_msecs(jiffies); @@ -8909,7 +8909,7 @@ qam64auto(struct drx_demod_instance *demod, pr_err(error %d\n, rc); goto rw_error; } - state = SYNC_FLIPPED; + lck_state = SYNC_FLIPPED; msleep(10); } break; @@ -8934,7 +8934,7 @@ qam64auto(struct drx_demod_instance *demod, pr_err(error %d\n, rc); goto rw_error; } - state = SPEC_MIRRORED; + lck_state = SPEC_MIRRORED; /* reset timer TODO: still need 500ms? */ start_time = d_locked_time = jiffies_to_msecs(jiffies); @@ -9008,7 +9008,7 @@ qam256auto(struct drx_demod_instance *demod, struct drx_sig_quality sig_quality; struct drxj_data *ext_attr = NULL; int rc; - u32 state = NO_LOCK; + u32 lck_state = NO_LOCK; u32 start_time = 0; u32 d_locked_time = 0; u32 timeout_ofs = DRXJ_QAM_DEMOD_LOCK_EXT_WAITTIME; @@ -9017,14 +9017,14 @@ qam256auto(struct drx_demod_instance *demod, ext_attr = (struct drxj_data *) demod-my_ext_attr; *lock_status = DRX_NOT_LOCKED; start_time = jiffies_to_msecs(jiffies); - state = NO_LOCK; + lck_state = NO_LOCK; do { rc = ctrl_lock_status(demod, lock_status); if (rc != 0) { pr_err(error %d\n, rc); goto rw_error; } - switch (state) { + switch (lck_state) { case NO_LOCK: if (*lock_status == DRXJ_DEMOD_LOCK) { rc = ctrl_get_qam_sig_quality(demod, sig_quality); @@ -9033,7 +9033,7 @@ qam256auto(struct drx_demod_instance *demod, goto rw_error; } if (sig_quality.MER 268) { - state = DEMOD_LOCKED; + lck_state = DEMOD_LOCKED; timeout_ofs += DRXJ_QAM_DEMOD_LOCK_EXT_WAITTIME;/* see something, wait longer */ d_locked_time = jiffies_to_msecs(jiffies); } @@ -9050,7 +9050,7 @@
[PATCH 07/15] drx-j: Fix usage of drxj_close()
This function is currently not used. However, it was meant to be called at device release. So, add it there. While here, remove the bad check, as reported by Dan, as smatch warning: drivers/media/dvb-frontends/drx39xyj/drxj.c:20041 drxj_close() warn: variable dereferenced before check 'demod' (see line 20036) Reported-by: Dan Carpenter dan.carpen...@oracle.com Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index 828d0527f38d..c5205d5c997e 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -11510,8 +11510,7 @@ static int drxj_close(struct drx_demod_instance *demod) int rc; enum drx_power_mode power_mode = DRX_POWER_UP; - if ((demod == NULL) || - (demod-my_common_attr == NULL) || + if ((demod-my_common_attr == NULL) || (demod-my_ext_attr == NULL) || (demod-my_i2c_dev_addr == NULL) || (!demod-my_common_attr-is_opened)) { @@ -12218,6 +12217,8 @@ static void drx39xxj_release(struct dvb_frontend *fe) struct drx39xxj_state *state = fe-demodulator_priv; struct drx_demod_instance *demod = state-demod; + drxj_close(demod); + kfree(demod-my_ext_attr); kfree(demod-my_common_attr); kfree(demod-my_i2c_dev_addr); -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/15] drx-j: Don't use 0 as NULL
Fixes the following warnings: drivers/media/dvb-frontends/drx39xyj/drxj.c:1679:65: warning: Using plain integer as NULL pointer drivers/media/dvb-frontends/drx39xyj/drxj.c:1679:71: warning: Using plain integer as NULL pointer drivers/media/dvb-frontends/drx39xyj/drxj.c:1681:52: warning: Using plain integer as NULL pointer drivers/media/dvb-frontends/drx39xyj/drxj.c:1681:58: warning: Using plain integer as NULL pointer Reported-by: kbuild test robot fengguang...@intel.com Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index 0232b1409ec0..af3b69ce8c16 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -1676,9 +1676,10 @@ static int drxdap_fasi_read_block(struct i2c_device_addr *dev_addr, * In single master mode, split the read and write actions. * No special action is needed for write chunks here. */ - rc = drxbsp_i2c_write_read(dev_addr, bufx, buf, 0, 0, 0); + rc = drxbsp_i2c_write_read(dev_addr, bufx, buf, + NULL, 0, NULL); if (rc == 0) - rc = drxbsp_i2c_write_read(0, 0, 0, dev_addr, todo, data); + rc = drxbsp_i2c_write_read(NULL, 0, NULL, dev_addr, todo, data); #else /* In multi master mode, do everything in one RW action */ rc = drxbsp_i2c_write_read(dev_addr, bufx, buf, dev_addr, todo, -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/15] drx-j: propagate returned error from request_firmware()
Fix a smatch warning: drivers/media/dvb-frontends/drx39xyj/drxj.c:11711 drx_ctrl_u_code() info: why not propagate 'rc' from request_firmware() instead of (-2)? Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index c5205d5c997e..a26ddc9fa2bc 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -11708,7 +11708,7 @@ static int drx_ctrl_u_code(struct drx_demod_instance *demod, rc = request_firmware(fw, mc_file, demod-i2c-dev.parent); if (rc 0) { pr_err(Couldn't read firmware %s\n, mc_file); - return -ENOENT; + return rc; } demod-firmware = fw; -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 15/15] drx-j: enable DVBv5 stats
Now that everything is set, let's enable DVBv5 stats, for applications that support it. DVBv3 apps will still work. Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/dvb-frontends/drx39xyj/drxj.c | 29 + 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index 8098d87cda0b..0c0e9f3b108f 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -6200,7 +6200,7 @@ rw_error: * \return Error code */ static int get_vsb_post_rs_pck_err(struct i2c_device_addr *dev_addr, - u16 *pck_errs, u16 *pck_count) + u32 *pck_errs, u32 *pck_count) { int rc; u16 data = 0; @@ -10664,8 +10664,8 @@ ctrl_sig_quality(struct drx_demod_instance *demod, struct dtv_frontend_properties *p = state-frontend.dtv_property_cache; enum drx_standard standard = ext_attr-standard; int rc; - u32 ber, cnt; - u16 err, pkt, mer, strength; + u32 ber, cnt, err, pkt; + u16 mer, strength; rc = get_sig_strength(demod, strength); if (rc 0) { @@ -12249,11 +12249,11 @@ static struct dvb_frontend_ops drx39xxj_ops; struct dvb_frontend *drx39xxj_attach(struct i2c_adapter *i2c) { struct drx39xxj_state *state = NULL; - struct i2c_device_addr *demod_addr = NULL; struct drx_common_attr *demod_comm_attr = NULL; struct drxj_data *demod_ext_attr = NULL; struct drx_demod_instance *demod = NULL; + struct dtv_frontend_properties *p; struct drxuio_cfg uio_cfg; struct drxuio_data uio_data; int result; @@ -12331,6 +12331,27 @@ struct dvb_frontend *drx39xxj_attach(struct i2c_adapter *i2c) sizeof(struct dvb_frontend_ops)); state-frontend.demodulator_priv = state; + + /* Initialize stats - needed for DVBv5 stats to work */ + p = state-frontend.dtv_property_cache; + p-strength.len = 1; + p-pre_bit_count.len = 1; + p-pre_bit_error.len = 1; + p-post_bit_count.len = 1; + p-post_bit_error.len = 1; + p-block_count.len = 1; + p-block_error.len = 1; + p-cnr.len = 1; + + p-strength.stat[0].scale = FE_SCALE_RELATIVE; + p-pre_bit_count.stat[0].scale = FE_SCALE_NOT_AVAILABLE; + p-pre_bit_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE; + p-post_bit_count.stat[0].scale = FE_SCALE_NOT_AVAILABLE; + p-post_bit_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE; + p-block_count.stat[0].scale = FE_SCALE_NOT_AVAILABLE; + p-block_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE; + p-cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE; + return state-frontend; error: -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW PATCH 2/3] vb2: Add videobuf2-dvb support
From: Hans Verkuil hans.verk...@cisco.com With the new vb2_thread_start/stop core code it is very easy to implement videobuf2-dvb. This should simplify converting existing videobuf drivers to vb2. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/Kconfig | 4 + drivers/media/v4l2-core/Makefile| 1 + drivers/media/v4l2-core/videobuf2-dvb.c | 336 include/media/videobuf2-dvb.h | 58 ++ 4 files changed, 399 insertions(+) create mode 100644 drivers/media/v4l2-core/videobuf2-dvb.c create mode 100644 include/media/videobuf2-dvb.h diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig index 2189bfb..9ca0f8d 100644 --- a/drivers/media/v4l2-core/Kconfig +++ b/drivers/media/v4l2-core/Kconfig @@ -83,3 +83,7 @@ config VIDEOBUF2_DMA_SG #depends on HAS_DMA select VIDEOBUF2_CORE select VIDEOBUF2_MEMOPS + +config VIDEOBUF2_DVB + tristate + select VIDEOBUF2_CORE diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile index c6ae7ba..63d29f2 100644 --- a/drivers/media/v4l2-core/Makefile +++ b/drivers/media/v4l2-core/Makefile @@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o +obj-$(CONFIG_VIDEOBUF2_DVB) += videobuf2-dvb.o ccflags-y += -I$(srctree)/drivers/media/dvb-core ccflags-y += -I$(srctree)/drivers/media/dvb-frontends diff --git a/drivers/media/v4l2-core/videobuf2-dvb.c b/drivers/media/v4l2-core/videobuf2-dvb.c new file mode 100644 index 000..d092698 --- /dev/null +++ b/drivers/media/v4l2-core/videobuf2-dvb.c @@ -0,0 +1,336 @@ +/* + * + * some helper function for simple DVB cards which simply DMA the + * complete transport stream and let the computer sort everything else + * (i.e. we are using the software demux, ...). Also uses the + * video-buf to manage DMA buffers. + * + * (c) 2004 Gerd Knorr kra...@bytesex.org [SUSE Labs] + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include linux/module.h +#include linux/init.h +#include linux/device.h +#include linux/slab.h + +#include media/videobuf2-dvb.h + +/* -- */ + +MODULE_AUTHOR(Gerd Knorr kra...@bytesex.org [SuSE Labs]); +MODULE_LICENSE(GPL); + +/* -- */ + +static int dvb_fnc(struct vb2_buffer *vb, void *priv) +{ + struct vb2_dvb *dvb = priv; + + dvb_dmx_swfilter(dvb-demux, vb2_plane_vaddr(vb, 0), + vb2_get_plane_payload(vb, 0)); + return 0; +} + +static int vb2_dvb_start_feed(struct dvb_demux_feed *feed) +{ + struct dvb_demux *demux = feed-demux; + struct vb2_dvb *dvb = demux-priv; + int rc = 0; + + if (!demux-dmx.frontend) + return -EINVAL; + + mutex_lock(dvb-lock); + dvb-nfeeds++; + + if (!dvb-dvbq.threadio) { + rc = vb2_thread_start(dvb-dvbq, dvb_fnc, dvb, dvb-name); + if (rc) + dvb-nfeeds--; + } + if (!rc) + rc = dvb-nfeeds; + mutex_unlock(dvb-lock); + return rc; +} + +static int vb2_dvb_stop_feed(struct dvb_demux_feed *feed) +{ + struct dvb_demux *demux = feed-demux; + struct vb2_dvb *dvb = demux-priv; + int err = 0; + + mutex_lock(dvb-lock); + dvb-nfeeds--; + if (0 == dvb-nfeeds) + err = vb2_thread_stop(dvb-dvbq); + mutex_unlock(dvb-lock); + return err; +} + +static int vb2_dvb_register_adapter(struct vb2_dvb_frontends *fe, + struct module *module, + void *adapter_priv, + struct device *device, + char *adapter_name, + short *adapter_nr, + int mfe_shared) +{ + int result; + + mutex_init(fe-lock); + + /* register adapter */ + result = dvb_register_adapter(fe-adapter, adapter_name, module, + device, adapter_nr); + if (result 0) { + pr_warn(%s: dvb_register_adapter failed (errno = %d)\n, + adapter_name, result); + } + fe-adapter.priv = adapter_priv; + fe-adapter.mfe_shared = mfe_shared; + + return result; +} + +static int vb2_dvb_register_frontend(struct dvb_adapter *adapter, + struct vb2_dvb *dvb) +{ + int result; + + /* register frontend */ + result = dvb_register_frontend(adapter, dvb-frontend); + if (result
[REVIEW PATCH 1/3] vb2: add thread support
From: Hans Verkuil hans.verk...@cisco.com In order to implement vb2 DVB support you need to be able to start a kernel thread that queues and dequeues buffers, calling a callback function for every buffer. This patch adds support for that. It's based on drivers/media/v4l2-core/videobuf-dvb.c, but with all the DVB specific stuff stripped out, thus making it much more generic. Signed-off-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/v4l2-core/videobuf2-core.c | 147 +++ include/media/videobuf2-core.h | 32 +++ 2 files changed, 179 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index f9059bb..98069f6 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -6,6 +6,9 @@ * Author: Pawel Osciak pa...@osciak.com *Marek Szyprowski m.szyprow...@samsung.com * + * The vb2_thread implementation was based on code from videobuf-dvb.c: + * (c) 2004 Gerd Knorr kra...@bytesex.org [SUSE Labs] + * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation. @@ -18,10 +21,13 @@ #include linux/poll.h #include linux/slab.h #include linux/sched.h +#include linux/freezer.h +#include linux/kthread.h #include media/v4l2-dev.h #include media/v4l2-fh.h #include media/v4l2-event.h +#include media/v4l2-common.h #include media/videobuf2-core.h static int debug; @@ -2890,6 +2896,147 @@ size_t vb2_write(struct vb2_queue *q, const char __user *data, size_t count, } EXPORT_SYMBOL_GPL(vb2_write); +struct vb2_threadio_data { + struct task_struct *thread; + vb2_thread_fnc fnc; + void *priv; + bool stop; +}; + +static int vb2_thread(void *data) +{ + struct vb2_queue *q = data; + struct vb2_threadio_data *threadio = q-threadio; + struct vb2_fileio_data *fileio = q-fileio; + bool set_timestamp = false; + int prequeue = 0; + int index = 0; + int ret = 0; + + if (V4L2_TYPE_IS_OUTPUT(q-type)) { + prequeue = q-num_buffers; + set_timestamp = + (q-timestamp_flags V4L2_BUF_FLAG_TIMESTAMP_MASK) == + V4L2_BUF_FLAG_TIMESTAMP_COPY; + } + + set_freezable(); + + for (;;) { + struct vb2_buffer *vb; + + /* +* Call vb2_dqbuf to get buffer back. +*/ + memset(fileio-b, 0, sizeof(fileio-b)); + fileio-b.type = q-type; + fileio-b.memory = q-memory; + if (prequeue) { + fileio-b.index = index++; + prequeue--; + } else { + call_qop(q, wait_finish, q); + ret = vb2_internal_dqbuf(q, fileio-b, 0); + call_qop(q, wait_prepare, q); + dprintk(5, file io: vb2_dqbuf result: %d\n, ret); + } + if (threadio-stop) + break; + if (ret) + break; + try_to_freeze(); + + vb = q-bufs[fileio-b.index]; + if (!(fileio-b.flags V4L2_BUF_FLAG_ERROR)) + ret = threadio-fnc(vb, threadio-priv); + if (ret) + break; + call_qop(q, wait_finish, q); + if (set_timestamp) + v4l2_get_timestamp(fileio-b.timestamp); + ret = vb2_internal_qbuf(q, fileio-b); + call_qop(q, wait_prepare, q); + if (ret) + break; + } + + /* Hmm, linux becomes *very* unhappy without this ... */ + while (!kthread_should_stop()) { + set_current_state(TASK_INTERRUPTIBLE); + schedule(); + } + return 0; +} + +/* + * This function should not be used for anything else but the videobuf2-dvb + * support. If you think you have another good use-case for this, then please + * contact the linux-media mailinglist first. + */ +int vb2_thread_start(struct vb2_queue *q, vb2_thread_fnc fnc, void *priv, +const char *thread_name) +{ + struct vb2_threadio_data *threadio; + int ret = 0; + + if (q-threadio) + return -EBUSY; + if (vb2_is_busy(q)) + return -EBUSY; + if (WARN_ON(q-fileio)) + return -EBUSY; + + threadio = kzalloc(sizeof(*threadio), GFP_KERNEL); + if (threadio == NULL) + return -ENOMEM; + threadio-fnc = fnc; + threadio-priv = priv; + + ret = __vb2_init_fileio(q, !V4L2_TYPE_IS_OUTPUT(q-type)); + dprintk(3, file io: vb2_init_fileio result: %d\n, ret); +
Re: [PATCH v2 7/7] v4l: ti-vpe: Add selection API in VPE driver
On 03/10/2014 01:12 PM, Archit Taneja wrote: Hi Hans, On Friday 07 March 2014 07:17 PM, Archit Taneja wrote: On Friday 07 March 2014 07:02 PM, Hans Verkuil wrote: On 03/07/2014 02:22 PM, Archit Taneja wrote: Disregard what I said, it's OK to upstream it. But if you could just spend some hours fixing the problems, that would really be best. Sure, I'll try to fix these issues and then post a v3. I fixed most of the compliance errors. There were some things I needed to change in .utils/v4l2-compliance/v4l2-test-buffers.cpp'. I added those with some questions in the comments: diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp index 6576d11..532a5b6 100644 --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp @@ -219,7 +219,13 @@ static int checkQueryBuf(struct node *node, const struct v4l2_buffer buf, fail_on_test(!(buf.flags (V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_ERROR))); if (node-is_video) { fail_on_test(buf.field == V4L2_FIELD_ALTERNATE); - fail_on_test(buf.field == V4L2_FIELD_ANY); + /* + * the OUTPUT buffers are queued with V4L2_FIELD_ANY + * field type by the application. Is it the driver's + * job to change this to NONE in buf_prepare? Yes, it is. Applications may pass in FIELD_ANY, but the driver must never return it, it should always be replaced by what the driver actually uses. + */ + + /* fail_on_test(buf.field == V4L2_FIELD_ANY); */ if (cur_fmt.fmt.pix.field == V4L2_FIELD_ALTERNATE) { fail_on_test(buf.field != V4L2_FIELD_BOTTOM buf.field != V4L2_FIELD_TOP); @@ -651,9 +657,17 @@ static int captureBufs(struct node *node, const struct v4l2_requestbuffers bufs } else if (node-is_m2m timestamp == V4L2_BUF_FLAG_TIMESTAMP_COPY) { fail_on_test(buffer_info.find(buf.timestamp) == buffer_info.end()); struct v4l2_buffer orig_buf = buffer_info[buf.timestamp]; - fail_on_test(buf.field != orig_buf.field); - fail_on_test((buf.flags valid_output_flags) != - (orig_buf.flags valid_output_flags)); + /* same issue as as in checkQueryBuf */ + /* fail_on_test(buf.field != orig_buf.field); */ + + /* + * the queued buffers are filled with flags like + * V4L2_BUF_FLAG_KEYFRAME, these are lost when + * the captured buffers are dequed. How do we + * fix this? Well, the driver has to copy them :-) Note that v4l2-compliance assumes that there is a 1 to 1 mapping between buffers coming into the codec and buffers coming out of the codec. If that's not the case, then copying such flags does not make any sense. On the other hand, in that case using V4L2_BUF_FLAG_TIMESTAMP_COPY probably makes no sense either. Regards, Hans + */ + /*fail_on_test((buf.flags valid_output_flags) != + (orig_buf.flags valid_output_flags)); */ if (buf.flags V4L2_BUF_FLAG_TIMECODE) fail_on_test(memcmp(buf.timecode, orig_buf.timecode, sizeof(buf.timecode))); Thanks, Archit -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FOR v3.15] Add G/S_EDID support for video nodes
Currently the VIDIOC_SUBDEV_G/S_EDID and struct v4l2_subdev_edid are subdev APIs. However, that's in reality quite annoying since for simple video pipelines there is no need to create v4l-subdev device nodes for anything else except for setting or getting EDIDs. What happens in practice is that v4l2 bridge drivers add explicit support for VIDIOC_SUBDEV_G/S_EDID themselves, just to avoid having to create subdev device nodes just for this. So this patch series makes the ioctls available as regular ioctls as well. In that case the pad field is interpreted as the input or output index as returned by ENUMINPUT/OUTPUT. Rebased to the latest master branch, but otherwise unchanged from the REVIEWv1 patch series: http://www.spinics.net/lists/linux-media/msg74022.html Regards, Hans The following changes since commit f2d7313534072a5fe192e7cf46204b413acef479: [media] drx-d: add missing braces in drxd_hard.c:DRXD_init (2014-03-09 09:20:50 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git edid for you to fetch changes up to 66090d7c5b127fa4a461394261c50ca2364a286a: DocBook v4l2: update the G/S_EDID documentation (2014-03-10 13:36:29 +0100) Hans Verkuil (5): v4l2-compat-ioctl32: fix wrong VIDIOC_SUBDEV_G/S_EDID32 support. v4l2: allow v4l2_subdev_edid to be used with video nodes v4l2: add VIDIOC_G/S_EDID support to the v4l2 core. adv*: replace the deprecated v4l2_subdev_edid by v4l2_edid. DocBook v4l2: update the G/S_EDID documentation Documentation/DocBook/media/v4l/v4l2.xml | 2 +- Documentation/DocBook/media/v4l/{vidioc-subdev-g-edid.xml = vidioc-g-edid.xml} | 36 +++- drivers/media/i2c/ad9389b.c | 2 +- drivers/media/i2c/adv7511.c | 2 +- drivers/media/i2c/adv7604.c | 4 ++-- drivers/media/i2c/adv7842.c | 4 ++-- drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 32 drivers/media/v4l2-core/v4l2-dev.c | 2 ++ drivers/media/v4l2-core/v4l2-ioctl.c | 16 +--- drivers/media/v4l2-core/v4l2-subdev.c | 4 ++-- include/media/v4l2-ioctl.h | 2 ++ include/media/v4l2-subdev.h | 4 ++-- include/uapi/linux/v4l2-common.h | 8 include/uapi/linux/v4l2-subdev.h | 14 +- include/uapi/linux/videodev2.h | 2 ++ 15 files changed, 82 insertions(+), 52 deletions(-) rename Documentation/DocBook/media/v4l/{vidioc-subdev-g-edid.xml = vidioc-g-edid.xml} (77%) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FOR v3.14] Three fixes
Three small fixes for 3.14: - while working on the EDID changes Laurent discovered that the ioctl numbers specified in v4l2-compat-ioctl32 were wrong and have been from the beginning. Fix that. NOTE: this patch was also included with the 3.15 EDID pull request I posted today. - The v4l2-dv-timings module was missing the module name, description and license. Because of the missing license any driver that loads that module will taint the kernel. Definitely not what I intended. - Fix a saa7134 suspend bug: https://bugzilla.kernel.org/show_bug.cgi?id=69581 Regards, Hans The following changes since commit bfd0306462fdbc5e0a8c6999aef9dde0f9745399: [media] v4l: Document timestamp buffer flag behaviour (2014-03-05 16:48:28 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v3.14f for you to fetch changes up to b500d6a1664622cb7ba6ad27a1bc2bbce1022587: saa7134: fix WARN_ON during resume. (2014-03-07 12:11:19 +0100) Hans Verkuil (3): v4l2-compat-ioctl32: fix wrong VIDIOC_SUBDEV_G/S_EDID32 support. v4l2-dv-timings: add module name, description, license. saa7134: fix WARN_ON during resume. drivers/media/pci/saa7134/saa7134-cards.c | 4 ++-- drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 4 ++-- drivers/media/v4l2-core/v4l2-dv-timings.c | 4 3 files changed, 8 insertions(+), 4 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL FOR v3.15] Add G/S_EDID support for video nodes
On 03/10/2014 01:40 PM, Hans Verkuil wrote: Currently the VIDIOC_SUBDEV_G/S_EDID and struct v4l2_subdev_edid are subdev APIs. However, that's in reality quite annoying since for simple video pipelines there is no need to create v4l-subdev device nodes for anything else except for setting or getting EDIDs. What happens in practice is that v4l2 bridge drivers add explicit support for VIDIOC_SUBDEV_G/S_EDID themselves, just to avoid having to create subdev device nodes just for this. So this patch series makes the ioctls available as regular ioctls as well. In that case the pad field is interpreted as the input or output index as returned by ENUMINPUT/OUTPUT. Rebased to the latest master branch, but otherwise unchanged from the REVIEWv1 patch series: http://www.spinics.net/lists/linux-media/msg74022.html Please note that the first patch is also part of a 3.14 patch series I just posted. It's a fix that should go to 3.14 as well IMHO. Regards, Hans Regards, Hans The following changes since commit f2d7313534072a5fe192e7cf46204b413acef479: [media] drx-d: add missing braces in drxd_hard.c:DRXD_init (2014-03-09 09:20:50 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git edid for you to fetch changes up to 66090d7c5b127fa4a461394261c50ca2364a286a: DocBook v4l2: update the G/S_EDID documentation (2014-03-10 13:36:29 +0100) Hans Verkuil (5): v4l2-compat-ioctl32: fix wrong VIDIOC_SUBDEV_G/S_EDID32 support. v4l2: allow v4l2_subdev_edid to be used with video nodes v4l2: add VIDIOC_G/S_EDID support to the v4l2 core. adv*: replace the deprecated v4l2_subdev_edid by v4l2_edid. DocBook v4l2: update the G/S_EDID documentation Documentation/DocBook/media/v4l/v4l2.xml | 2 +- Documentation/DocBook/media/v4l/{vidioc-subdev-g-edid.xml = vidioc-g-edid.xml} | 36 +++- drivers/media/i2c/ad9389b.c | 2 +- drivers/media/i2c/adv7511.c | 2 +- drivers/media/i2c/adv7604.c | 4 ++-- drivers/media/i2c/adv7842.c | 4 ++-- drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 32 drivers/media/v4l2-core/v4l2-dev.c | 2 ++ drivers/media/v4l2-core/v4l2-ioctl.c | 16 +--- drivers/media/v4l2-core/v4l2-subdev.c | 4 ++-- include/media/v4l2-ioctl.h | 2 ++ include/media/v4l2-subdev.h | 4 ++-- include/uapi/linux/v4l2-common.h | 8 include/uapi/linux/v4l2-subdev.h | 14 +- include/uapi/linux/videodev2.h | 2 ++ 15 files changed, 82 insertions(+), 52 deletions(-) rename Documentation/DocBook/media/v4l/{vidioc-subdev-g-edid.xml = vidioc-g-edid.xml} (77%) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] v4l: vb2: Add a function to discard all DONE buffers
When suspending a device while a video stream is active all buffers marked as done but not dequeued yet will be kept across suspend and given back to userspace after resume. This will result in outdated buffers being dequeued. Introduce a new vb2 function to mark all done buffers as erroneous instead, to be used by drivers at resume time. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/v4l2-core/videobuf2-core.c | 24 include/media/videobuf2-core.h | 1 + 2 files changed, 25 insertions(+) The OMAP3 ISP driver custom queue implementation has this feature. I'm trying to move it to videobuf2, and thus need something similar in vb2. The function name could probably be improved, I'm open to suggestions. diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 8e6695c..ae2b5b1 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -971,6 +971,30 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) EXPORT_SYMBOL_GPL(vb2_buffer_done); /** + * vb2_discard_done() - discard all buffers marked as DONE + * @q: videobuf2 queue + * + * This function is intended to be used with suspend/resume operations. It + * discards all 'done' buffers as they would be too old to be requested after + * resume. + * + * Drivers must stop the hardware and synchronize with interrupt handlers and/or + * delayed works before calling this function to make sure no buffer will be + * touched by the driver and/or hardware. + */ +void vb2_discard_done(struct vb2_queue *q) +{ + struct vb2_buffer *vb; + unsigned long flags; + + spin_lock_irqsave(q-done_lock, flags); + list_for_each_entry(vb, q-done_list, done_entry) + vb-state = VB2_BUF_STATE_ERROR; + spin_unlock_irqrestore(q-done_lock, flags); +} +EXPORT_SYMBOL_GPL(vb2_discard_done); + +/** * __fill_vb2_buffer() - fill a vb2_buffer with information provided in a * v4l2_buffer by the userspace. The caller has already verified that struct * v4l2_buffer has a valid number of planes. diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index bf6859e..213b555 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -372,6 +372,7 @@ void *vb2_plane_vaddr(struct vb2_buffer *vb, unsigned int plane_no); void *vb2_plane_cookie(struct vb2_buffer *vb, unsigned int plane_no); void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state); +void vb2_discard_done(struct vb2_queue *q); int vb2_wait_for_all_buffers(struct vb2_queue *q); int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b); -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/15] drx-j: Cleanups, fixes and support for DVBv5 stats
Em Mon, 10 Mar 2014 08:58:52 -0300 Mauro Carvalho Chehab m.che...@samsung.com escreveu: This patch series is meant to: 1) fix some reported issues (sparse, smatch); 2) Fix one compilation issue with em28xx when drx-j is not selected; 3) Get rid of unused code. It is always possible to restore the code from git history. Removing the unused code helps to better understand what's actually there. 4) Add support for DVBv5 stats. On my tests here with an AWGN noise generator, the CNR measure made by drx-j was close enough to the SNR injected (with a difference of about 1 dB). Mauro Carvalho Chehab (15): drx-j: Don't use 0 as NULL drx-j: Fix dubious usage of instead of drx39xxj.h: Fix undefined reference to attach function drx-j: don't use mc_info before checking if its not NULL drx-j: get rid of dead code This patch seems to be rejected by vger... likely too big. Well the patch is here: http://git.linuxtv.org/mchehab/experimental.git/commitdiff/49bac5876df20f74238bfccf3ef9cbaefcaa2ca9 For anyone wanting to test, all patches are at: http://git.linuxtv.org/mchehab/experimental.git/shortlog/refs/heads/drx-j-v3 drx-j: remove external symbols drx-j: Fix usage of drxj_close() drx-j: propagate returned error from request_firmware() drx-j: get rid of some unused vars drx-j: Don't use state for DVB lock state drx-j: re-add get_sig_strength() drx-j: Prepare to use DVBv5 stats drx-j: properly handle bit counts on stats drx-j: Fix detection of no signal drx-j: enable DVBv5 stats drivers/media/dvb-frontends/drx39xyj/drx39xxj.h | 6 + drivers/media/dvb-frontends/drx39xyj/drx_driver.h |24 - drivers/media/dvb-frontends/drx39xyj/drxj.c | 11146 +++- drivers/media/dvb-frontends/drx39xyj/drxj.h |30 - 4 files changed, 1343 insertions(+), 9863 deletions(-) -- Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] v4l: vb2: Add a function to discard all DONE buffers
On 03/10/2014 02:04 PM, Laurent Pinchart wrote: When suspending a device while a video stream is active all buffers marked as done but not dequeued yet will be kept across suspend and given back to userspace after resume. This will result in outdated buffers being dequeued. Introduce a new vb2 function to mark all done buffers as erroneous instead, to be used by drivers at resume time. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Makes sense. Acked-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans --- drivers/media/v4l2-core/videobuf2-core.c | 24 include/media/videobuf2-core.h | 1 + 2 files changed, 25 insertions(+) The OMAP3 ISP driver custom queue implementation has this feature. I'm trying to move it to videobuf2, and thus need something similar in vb2. The function name could probably be improved, I'm open to suggestions. diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 8e6695c..ae2b5b1 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -971,6 +971,30 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) EXPORT_SYMBOL_GPL(vb2_buffer_done); /** + * vb2_discard_done() - discard all buffers marked as DONE + * @q: videobuf2 queue + * + * This function is intended to be used with suspend/resume operations. It + * discards all 'done' buffers as they would be too old to be requested after + * resume. + * + * Drivers must stop the hardware and synchronize with interrupt handlers and/or + * delayed works before calling this function to make sure no buffer will be + * touched by the driver and/or hardware. + */ +void vb2_discard_done(struct vb2_queue *q) +{ + struct vb2_buffer *vb; + unsigned long flags; + + spin_lock_irqsave(q-done_lock, flags); + list_for_each_entry(vb, q-done_list, done_entry) + vb-state = VB2_BUF_STATE_ERROR; + spin_unlock_irqrestore(q-done_lock, flags); +} +EXPORT_SYMBOL_GPL(vb2_discard_done); + +/** * __fill_vb2_buffer() - fill a vb2_buffer with information provided in a * v4l2_buffer by the userspace. The caller has already verified that struct * v4l2_buffer has a valid number of planes. diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index bf6859e..213b555 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -372,6 +372,7 @@ void *vb2_plane_vaddr(struct vb2_buffer *vb, unsigned int plane_no); void *vb2_plane_cookie(struct vb2_buffer *vb, unsigned int plane_no); void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state); +void vb2_discard_done(struct vb2_queue *q); int vb2_wait_for_all_buffers(struct vb2_queue *q); int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b); -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] videobuf2-dma-contig broken for IOMMU + USERPTR buffers without struct page mapping
Hello, On some platforms (namely ARM) IOMMUs are handled transparently by the DMA mapping implementation. This requires mapping and unmapping all USERPTR buffers for DMA, regardless of whether they're backed by struct page or not. videobuf2-dma-contig is broken in that regard, as it call dma_map_sg() and dma_unmap_sg() only for buffers backed with struct page. The page-less USERPTR dma-contig support was mostly intended (if I'm not mistaken) to support exporters (in the dmabuf sense, but through mmap() on the exporter side and USERPTR on the V4L2 side) that required large physically contiguous buffers. Allocating such buffers required reserving memory at boot time and resulted in no struct page mappings for that memory. Now that CMA is available I believe that most (if not all) drivers have been converted to CMA using the dma_alloc_* API. My original test case for page- less USERPTR buffers with the OMAP3 ISP, capturing to mmap()ed fbdev memory, now has the memory backed by struct page. I wonder whether we should drop support for this broken feature altogether, or fix it. A fix won't be easy, given that dma_map_sg() assumes that the memory is backed by struct page at least on some platforms. On ARM, for instance, it calls page_to_phys(sg_page()) on sglist entries. Does anyone still have a test case for this features ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] Move device tree graph parsing helpers to drivers/of
Em Fri, 07 Mar 2014 18:23:30 + Grant Likely grant.lik...@linaro.org escreveu: On Thu, 06 Mar 2014 18:13:20 +0100, Philipp Zabel p.za...@pengutronix.de wrote: Hi Mauro, Russell, I have temporarily removed the simplified bindings at Sylwester's request and updated the branch with the acks. The following changes since commit 0414855fdc4a40da05221fc6062cccbc0c30f169: Linux 3.14-rc5 (2014-03-02 18:56:16 -0800) are available in the git repository at: git://git.pengutronix.de/git/pza/linux.git topic/of-graph for you to fetch changes up to d484700a36952c6675aa47dec4d7a536929aa922: of: Warn if of_graph_parse_endpoint is called with the root node (2014-03-06 17:41:54 +0100) Nak. I made comments that haven't been resolved yet. I've replied with more detail tonight. The big issues are how drivers handle the optional 'ports' node and I do not agree to the double-linkage in the binding description. If I understood well, you're requesting to revert all those six patches that were imported via git pull from my tree (and from Greg and Russell), right? E. g. reverting those changesets: d484700a3695 f2a575f67695 4329b93b283c 6ff60d397b17 4d56ed5a009b fd9fdb78a9bf as it seems that there's no easy way to revert a git pull. I suspect that this will likely cause some harm when merging from our trees upstream. Regards, Mauro g. Philipp Zabel (6): [media] of: move graph helpers from drivers/media/v4l2-core to drivers/of Documentation: of: Document graph bindings of: Warn if of_graph_get_next_endpoint is called with the root node of: Reduce indentation in of_graph_get_next_endpoint [media] of: move common endpoint parsing to drivers/of of: Warn if of_graph_parse_endpoint is called with the root node Documentation/devicetree/bindings/graph.txt | 129 ++ drivers/media/i2c/adv7343.c | 4 +- drivers/media/i2c/mt9p031.c | 4 +- drivers/media/i2c/s5k5baf.c | 3 +- drivers/media/i2c/tvp514x.c | 3 +- drivers/media/i2c/tvp7002.c | 3 +- drivers/media/platform/exynos4-is/fimc-is.c | 6 +- drivers/media/platform/exynos4-is/media-dev.c | 13 ++- drivers/media/platform/exynos4-is/mipi-csis.c | 5 +- drivers/media/v4l2-core/v4l2-of.c | 133 +-- drivers/of/base.c | 151 ++ include/linux/of_graph.h | 66 +++ include/media/v4l2-of.h | 33 +- 13 files changed, 375 insertions(+), 178 deletions(-) create mode 100644 Documentation/devicetree/bindings/graph.txt create mode 100644 include/linux/of_graph.h -- Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
On Monday 10 March 2014 12:18:20 Tomi Valkeinen wrote: On 08/03/14 13:41, Grant Likely wrote: Ok. If we go for single directional link, the question is then: which way? And is the direction different for display and camera, which are kind of reflections of each other? In general I would recommend choosing whichever device you would sensibly think of as a master. In the camera case I would choose the camera controller node instead of the camera itself, and in the display case I would choose the display controller instead of the panel. The binding author needs to choose what she things makes the most sense, but drivers can still use if it it turns out to be 'backwards' I would perhaps choose the same approach, but at the same time I think it's all but clear. The display controller doesn't control the panel any more than a DMA controller controls, say, the display controller. In fact, in earlier versions of OMAP DSS DT support I had a simpler port description, and in that I had the panel as the master (i.e. link from panel to dispc) because the panel driver uses the display controller's features to provide the panel device a data stream. And even with the current OMAP DSS DT version, which uses the v4l2 style ports/endpoints, the driver model is still the same, and only links towards upstream are used. So one reason I'm happy with the dual-linking is that I can easily follow the links from the downstream entities to upstream entities, and other people, who have different driver model, can easily do the opposite. But I agree that single-linking is enough and this can be handled at runtime, even if it makes the code more complex. And perhaps requires extra data in the dts, to give the start points for the graph. In theory unidirectional links in DT are indeed enough. However, let's not forget the following. - There's no such thing as single start points for graphs. Sure, in some simple cases the graph will have a single start point, but that's not a generic rule. For instance the camera graphs http://ideasonboard.org/media/omap3isp.ps and http://ideasonboard.org/media/eyecam.ps have two camera sensors, and thus two starting points from a data flow point of view. And if you want a better understanding of how complex media graphs can become, have a look at http://ideasonboard.org/media/vsp1.0.pdf (that's a real world example, albeit all connections are internal to the SoC in that particular case, and don't need to be described in DT). - There's also no such thing as a master device that can just point to slave devices. Once again simple cases exist where that model could work, but real world examples exist of complex pipelines with dozens of elements all implemented by a separate IP core and handled by separate drivers, forming a graph with long chains and branches. We thus need real graph bindings. - Finally, having no backlinks in DT would make the software implementation very complex. We need to be able to walk the graph in a generic way without having any of the IP core drivers loaded, and without any specific starting point. We would thus need to parse the complete DT tree, looking at all nodes and trying to find out whether they're part of the graph we're trying to walk. The complexity of the operation would be at best quadratic to the number of nodes in the whole DT and to the number of nodes in the graph. -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part.
Re: [PATCH v4 1/3] [media] of: move graph helpers from drivers/media/v4l2-core to drivers/of
Hi Tomi, On Monday 10 March 2014 08:00:02 Tomi Valkeinen wrote: On 08/03/14 17:54, Laurent Pinchart wrote: Sylwester suggested as an alternative, if I understood correctly, to drop the endpoint node and instead keep the port: device-a { implicit_output_ep: port { remote-endpoint = explicit_input_ep; }; }; device-b { port { explicit_input_ep: endpoint { remote-endpoint = implicit_output_ep; }; }; }; This would have the advantage to reduce verbosity for devices with multiple ports that are only connected via one endport each, and you'd always have the connected ports in the device tree as 'port' nodes. I like that idea. I would prefer making the 'port' nodes mandatory and the 'ports' and 'endpoint' nodes optional. Leaving the 'port' node out slightly decreases readability in my opinion, but making the 'endpoint' node optional increases it. That's just my point of view though. I, on the other hand, don't like it =). With that format, the remote-endpoint doesn't point to an EP, but a port. And you'll have endpoint's properties in a port node, among the port's properties. We'll need to discuss port and endpoint properties separately, but it might make sense to allow endpoints to override port properties instead of specifying the same value explicitly for each endpoint. Endpoint parsing functions would thus look for properties in endpoints first and then in the parent port node if the property can't be found. This would work with implicit endpoints and would be hidden to the drivers. (Please note that this is just food for thought) -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part.
Re: [PATCH v6 2/8] Documentation: of: Document graph bindings
Am Montag, den 10.03.2014, 12:37 +0100 schrieb Laurent Pinchart: Hi Philipp, On Monday 10 March 2014 10:28:10 Philipp Zabel wrote: Hi Grant, Am Freitag, den 07.03.2014, 18:27 + schrieb Grant Likely: On Wed, 5 Mar 2014 10:20:36 +0100, Philipp Zabel wrote: The device tree graph bindings as used by V4L2 and documented in Documentation/device-tree/bindings/media/video-interfaces.txt contain generic parts that are not media specific but could be useful for any subsystem with data flow between multiple devices. This document describes the generic bindings. Signed-off-by: Philipp Zabel p.za...@pengutronix.de See my comments on the previous version. My concerns are the handling of the optional 'ports' node and the usage of reverse links. would this change address your concern about the reverse links? As the preexisting video-interfaces.txt bindings mandate the reverse links, I worry about introducing a second, subtly different binding. It should be noted somewhere in video-interfaces.txt that the reverse links are deprecated for the but still supported by the code for backwards compatibility. I'm very much against removing the reverse links. Without them the graph will become much more complex to parse. You can try to convince me, but for now I'm afraid it's a NACK. For the record, I'd prefer to keep them, too. Besides the parsing complexity, it just feels more natural to take both ends and connect them together. If phandles are only permitted to point in one direction, there's always the additional question which direction is the right one. Assume, for example, the following setup of two SoC digital audio interfaces connected to an audio codec and a bluetooth chip, respectively. The audio codec has a second audio interface that is connected directly to the bluetooth chip for headset operation: ,---. ,. | dai1 [0]---[0] codec | `---' ,-[1] | | `' | ,-. ,---. `-[1] bt | | dai2 [0]---[0]| `---' `-´ How to decide which direction the codec:1 -- bt:1 link should point in? regards Philipp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv1 PATCH 4/7] mem2mem_testdev: add USERPTR support.
From: Hans Verkuil hans.verk...@cisco.com There is no reason why we shouldn't enable this here. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/mem2mem_testdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index c4b54f8..8e2aed2 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -782,7 +782,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds int ret; src_vq-type = V4L2_BUF_TYPE_VIDEO_OUTPUT; - src_vq-io_modes = VB2_MMAP | VB2_DMABUF; + src_vq-io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF; src_vq-drv_priv = ctx; src_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); src_vq-ops = m2mtest_qops; @@ -795,7 +795,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds return ret; dst_vq-type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - dst_vq-io_modes = VB2_MMAP | VB2_DMABUF; + dst_vq-io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF; dst_vq-drv_priv = ctx; dst_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); dst_vq-ops = m2mtest_qops; -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv1 PATCH 0/7] mem2mem_testdev: fixes and minor enhancements
This patch series fixes a number of v4l2-compliance problems and some small enhancements (a better default transfer time and adding USERPTR support). Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv1 PATCH 6/7] mem2mem_testdev: fix field, sequence and time copying
From: Hans Verkuil hans.verk...@cisco.com - Set the sequence counters correctly. - Copy timestamps, timecode, relevant buffer flags and field from the received buffer to the outgoing buffer. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/mem2mem_testdev.c | 27 --- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index 1ba1a83..dec8092 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -112,6 +112,7 @@ struct m2mtest_q_data { unsigned intwidth; unsigned intheight; unsigned intsizeimage; + unsigned intsequence; struct m2mtest_fmt *fmt; }; @@ -234,12 +235,21 @@ static int device_process(struct m2mtest_ctx *ctx, bytes_left = bytesperline - tile_w * MEM2MEM_NUM_TILES; w = 0; + out_vb-v4l2_buf.sequence = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE)-sequence++; + in_vb-v4l2_buf.sequence = q_data-sequence++; memcpy(out_vb-v4l2_buf.timestamp, in_vb-v4l2_buf.timestamp, sizeof(struct timeval)); - out_vb-v4l2_buf.flags = ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK; - out_vb-v4l2_buf.flags |= - in_vb-v4l2_buf.flags V4L2_BUF_FLAG_TSTAMP_SRC_MASK; + if (in_vb-v4l2_buf.flags V4L2_BUF_FLAG_TIMECODE) + memcpy(out_vb-v4l2_buf.timecode, in_vb-v4l2_buf.timecode, + sizeof(struct v4l2_timecode)); + out_vb-v4l2_buf.field = in_vb-v4l2_buf.field; + out_vb-v4l2_buf.flags = in_vb-v4l2_buf.flags + (V4L2_BUF_FLAG_TIMECODE | +V4L2_BUF_FLAG_KEYFRAME | +V4L2_BUF_FLAG_PFRAME | +V4L2_BUF_FLAG_BFRAME | +V4L2_BUF_FLAG_TSTAMP_SRC_MASK); switch (ctx-mode) { case MEM2MEM_HFLIP | MEM2MEM_VFLIP: @@ -765,9 +775,19 @@ static int m2mtest_buf_prepare(struct vb2_buffer *vb) static void m2mtest_buf_queue(struct vb2_buffer *vb) { struct m2mtest_ctx *ctx = vb2_get_drv_priv(vb-vb2_queue); + v4l2_m2m_buf_queue(ctx-fh.m2m_ctx, vb); } +static int m2mtest_start_streaming(struct vb2_queue *q, unsigned count) +{ + struct m2mtest_ctx *ctx = vb2_get_drv_priv(q); + struct m2mtest_q_data *q_data = get_q_data(ctx, q-type); + + q_data-sequence = 0; + return 0; +} + static int m2mtest_stop_streaming(struct vb2_queue *q) { struct m2mtest_ctx *ctx = vb2_get_drv_priv(q); @@ -792,6 +812,7 @@ static struct vb2_ops m2mtest_qops = { .queue_setup = m2mtest_queue_setup, .buf_prepare = m2mtest_buf_prepare, .buf_queue = m2mtest_buf_queue, + .start_streaming = m2mtest_start_streaming, .stop_streaming = m2mtest_stop_streaming, .wait_prepare= vb2_ops_wait_prepare, .wait_finish = vb2_ops_wait_finish, -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv1 PATCH 5/7] mem2mem_testdev: return pending buffers in stop_streaming()
From: Hans Verkuil hans.verk...@cisco.com To keep the vb2 buffer administration in balance stop_streaming() must return any pending buffers to the vb2 framework. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/mem2mem_testdev.c | 21 + 1 file changed, 21 insertions(+) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index 8e2aed2..1ba1a83 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -768,10 +768,31 @@ static void m2mtest_buf_queue(struct vb2_buffer *vb) v4l2_m2m_buf_queue(ctx-fh.m2m_ctx, vb); } +static int m2mtest_stop_streaming(struct vb2_queue *q) +{ + struct m2mtest_ctx *ctx = vb2_get_drv_priv(q); + struct vb2_buffer *vb; + unsigned long flags; + + for (;;) { + if (V4L2_TYPE_IS_OUTPUT(q-type)) + vb = v4l2_m2m_src_buf_remove(ctx-fh.m2m_ctx); + else + vb = v4l2_m2m_dst_buf_remove(ctx-fh.m2m_ctx); + if (vb == NULL) + return 0; + spin_lock_irqsave(ctx-dev-irqlock, flags); + v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR); + spin_unlock_irqrestore(ctx-dev-irqlock, flags); + } + return 0; +} + static struct vb2_ops m2mtest_qops = { .queue_setup = m2mtest_queue_setup, .buf_prepare = m2mtest_buf_prepare, .buf_queue = m2mtest_buf_queue, + .stop_streaming = m2mtest_stop_streaming, .wait_prepare= vb2_ops_wait_prepare, .wait_finish = vb2_ops_wait_finish, }; -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv1 PATCH 1/7] mem2mem_testdev: use 40ms default transfer time.
From: Hans Verkuil hans.verk...@cisco.com The default of 1 second is a bit painful, switch to a 25 Hz framerate. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/mem2mem_testdev.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index 4bb5e88..5c6067d 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -60,9 +60,7 @@ MODULE_PARM_DESC(debug, activates debug info); #define MEM2MEM_VID_MEM_LIMIT (16 * 1024 * 1024) /* Default transaction time in msec */ -#define MEM2MEM_DEF_TRANSTIME 1000 -/* Default number of buffers per transaction */ -#define MEM2MEM_DEF_TRANSLEN 1 +#define MEM2MEM_DEF_TRANSTIME 40 #define MEM2MEM_COLOR_STEP (0xff 4) #define MEM2MEM_NUM_TILES 8 @@ -804,10 +802,10 @@ static const struct v4l2_ctrl_config m2mtest_ctrl_trans_time_msec = { .id = V4L2_CID_TRANS_TIME_MSEC, .name = Transaction Time (msec), .type = V4L2_CTRL_TYPE_INTEGER, - .def = 1001, + .def = MEM2MEM_DEF_TRANSTIME, .min = 1, .max = 10001, - .step = 100, + .step = 1, }; static const struct v4l2_ctrl_config m2mtest_ctrl_trans_num_bufs = { -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv1 PATCH 2/7] mem2mem_testdev: pick default format with try_fmt.
From: Hans Verkuil hans.verk...@cisco.com This resolves an issue raised by v4l2-compliance: if the given format does not exist, then pick a default format. While there is an exception regarding this for TV capture drivers, this m2m driver should do the right thing. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/mem2mem_testdev.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index 5c6067d..104d863 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -543,7 +543,11 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv, struct m2mtest_ctx *ctx = file2ctx(file); fmt = find_format(f); - if (!fmt || !(fmt-types MEM2MEM_CAPTURE)) { + if (!fmt) { + f-fmt.pix.pixelformat = formats[0].fourcc; + fmt = find_format(f); + } + if (!(fmt-types MEM2MEM_CAPTURE)) { v4l2_err(ctx-dev-v4l2_dev, Fourcc format (0x%08x) invalid.\n, f-fmt.pix.pixelformat); @@ -561,7 +565,11 @@ static int vidioc_try_fmt_vid_out(struct file *file, void *priv, struct m2mtest_ctx *ctx = file2ctx(file); fmt = find_format(f); - if (!fmt || !(fmt-types MEM2MEM_OUTPUT)) { + if (!fmt) { + f-fmt.pix.pixelformat = formats[0].fourcc; + fmt = find_format(f); + } + if (!(fmt-types MEM2MEM_OUTPUT)) { v4l2_err(ctx-dev-v4l2_dev, Fourcc format (0x%08x) invalid.\n, f-fmt.pix.pixelformat); -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv1 PATCH 3/7] mem2mem_testdev: set priv to 0
From: Hans Verkuil hans.verk...@cisco.com v4l2_compliance fix. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/mem2mem_testdev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index 104d863..c4b54f8 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -532,6 +532,7 @@ static int vidioc_try_fmt(struct v4l2_format *f, struct m2mtest_fmt *fmt) f-fmt.pix.width = ~DIM_ALIGN_MASK; f-fmt.pix.bytesperline = (f-fmt.pix.width * fmt-depth) 3; f-fmt.pix.sizeimage = f-fmt.pix.height * f-fmt.pix.bytesperline; + f-fmt.pix.priv = 0; return 0; } -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv1 PATCH 7/7] mem2mem_testdev: improve field handling
From: Hans Verkuil hans.verk...@cisco.com try_fmt should just set field to NONE and not return an error if a different field was passed. buf_prepare should check if the field passed in from userspace has a supported field value. At the moment only NONE is supported and ANY is mapped to NONE. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/mem2mem_testdev.c | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index dec8092..4f3096b 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -516,19 +516,8 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv, static int vidioc_try_fmt(struct v4l2_format *f, struct m2mtest_fmt *fmt) { - enum v4l2_field field; - - field = f-fmt.pix.field; - - if (field == V4L2_FIELD_ANY) - field = V4L2_FIELD_NONE; - else if (V4L2_FIELD_NONE != field) - return -EINVAL; - /* V4L2 specification suggests the driver corrects the format struct * if any of the dimensions is unsupported */ - f-fmt.pix.field = field; - if (f-fmt.pix.height MIN_H) f-fmt.pix.height = MIN_H; else if (f-fmt.pix.height MAX_H) @@ -542,6 +531,7 @@ static int vidioc_try_fmt(struct v4l2_format *f, struct m2mtest_fmt *fmt) f-fmt.pix.width = ~DIM_ALIGN_MASK; f-fmt.pix.bytesperline = (f-fmt.pix.width * fmt-depth) 3; f-fmt.pix.sizeimage = f-fmt.pix.height * f-fmt.pix.bytesperline; + f-fmt.pix.field = V4L2_FIELD_NONE; f-fmt.pix.priv = 0; return 0; @@ -760,6 +750,15 @@ static int m2mtest_buf_prepare(struct vb2_buffer *vb) dprintk(ctx-dev, type: %d\n, vb-vb2_queue-type); q_data = get_q_data(ctx, vb-vb2_queue-type); + if (V4L2_TYPE_IS_OUTPUT(vb-vb2_queue-type)) { + if (vb-v4l2_buf.field == V4L2_FIELD_ANY) + vb-v4l2_buf.field = V4L2_FIELD_NONE; + if (vb-v4l2_buf.field != V4L2_FIELD_NONE) { + dprintk(ctx-dev, %s field isn't supported\n, + __func__); + return -EINVAL; + } + } if (vb2_plane_size(vb, 0) q_data-sizeimage) { dprintk(ctx-dev, %s data will not fit into plane (%lu %lu)\n, -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
On 10/03/14 15:52, Laurent Pinchart wrote: In theory unidirectional links in DT are indeed enough. However, let's not forget the following. - There's no such thing as single start points for graphs. Sure, in some simple cases the graph will have a single start point, but that's not a generic rule. For instance the camera graphs http://ideasonboard.org/media/omap3isp.ps and http://ideasonboard.org/media/eyecam.ps have two camera sensors, and thus two starting points from a data flow point of view. And if you want a better understanding of how complex media graphs can become, have a look at http://ideasonboard.org/media/vsp1.0.pdf (that's a real world example, albeit all connections are internal to the SoC in that particular case, and don't need to be described in DT). - There's also no such thing as a master device that can just point to slave devices. Once again simple cases exist where that model could work, but real world examples exist of complex pipelines with dozens of elements all implemented by a separate IP core and handled by separate drivers, forming a graph with long chains and branches. We thus need real graph bindings. - Finally, having no backlinks in DT would make the software implementation very complex. We need to be able to walk the graph in a generic way without having any of the IP core drivers loaded, and without any specific starting point. We would thus need to parse the complete DT tree, looking at all nodes and trying to find out whether they're part of the graph we're trying to walk. The complexity of the operation would be at best quadratic to the number of nodes in the whole DT and to the number of nodes in the graph. I did use plural when I said to give the start points If you have a list of starting points in the DT, a graph helper or something could create a runtime representation of the graph at some early phase during the boot, which would include backlinks. The individual drivers could use that runtime graph, instead of the DT graph. But it still sounds considerably more complex than double-links in DT. Tomi signature.asc Description: OpenPGP digital signature
Re: [GIT PULL] Move device tree graph parsing helpers to drivers/of
On Mon, 10 Mar 2014 10:26:30 -0300, Mauro Carvalho Chehab m.che...@samsung.com wrote: Em Fri, 07 Mar 2014 18:23:30 + Grant Likely grant.lik...@linaro.org escreveu: On Thu, 06 Mar 2014 18:13:20 +0100, Philipp Zabel p.za...@pengutronix.de wrote: Hi Mauro, Russell, I have temporarily removed the simplified bindings at Sylwester's request and updated the branch with the acks. The following changes since commit 0414855fdc4a40da05221fc6062cccbc0c30f169: Linux 3.14-rc5 (2014-03-02 18:56:16 -0800) are available in the git repository at: git://git.pengutronix.de/git/pza/linux.git topic/of-graph for you to fetch changes up to d484700a36952c6675aa47dec4d7a536929aa922: of: Warn if of_graph_parse_endpoint is called with the root node (2014-03-06 17:41:54 +0100) Nak. I made comments that haven't been resolved yet. I've replied with more detail tonight. The big issues are how drivers handle the optional 'ports' node and I do not agree to the double-linkage in the binding description. If I understood well, you're requesting to revert all those six patches that were imported via git pull from my tree (and from Greg and Russell), right? E. g. reverting those changesets: d484700a3695 f2a575f67695 4329b93b283c 6ff60d397b17 4d56ed5a009b fd9fdb78a9bf as it seems that there's no easy way to revert a git pull. All trees containing the branch would need to be reverted. I suspect that this will likely cause some harm when merging from our trees upstream. It means any tree containing that branch *must* be rewound. See my reply to rmk. I've made a proposal on how I could be happy with leaving the branches alone. I'm not particularly happy, but there is a way to resolve things without reverts or rewinds. g. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
On Mon, 10 Mar 2014 12:18:20 +0200, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 08/03/14 13:41, Grant Likely wrote: Ok. If we go for single directional link, the question is then: which way? And is the direction different for display and camera, which are kind of reflections of each other? In general I would recommend choosing whichever device you would sensibly think of as a master. In the camera case I would choose the camera controller node instead of the camera itself, and in the display case I would choose the display controller instead of the panel. The binding author needs to choose what she things makes the most sense, but drivers can still use if it it turns out to be 'backwards' I would perhaps choose the same approach, but at the same time I think it's all but clear. The display controller doesn't control the panel any more than a DMA controller controls, say, the display controller. In a sense it doesn't actually matter. You sensibly want to choose the most likely direction that drivers would go looking for a device it depends on, but it can be matched up at runtime regardless of the direction chosen by the binding. g. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
On Mon, 10 Mar 2014 14:52:53 +0100, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: On Monday 10 March 2014 12:18:20 Tomi Valkeinen wrote: On 08/03/14 13:41, Grant Likely wrote: Ok. If we go for single directional link, the question is then: which way? And is the direction different for display and camera, which are kind of reflections of each other? In general I would recommend choosing whichever device you would sensibly think of as a master. In the camera case I would choose the camera controller node instead of the camera itself, and in the display case I would choose the display controller instead of the panel. The binding author needs to choose what she things makes the most sense, but drivers can still use if it it turns out to be 'backwards' I would perhaps choose the same approach, but at the same time I think it's all but clear. The display controller doesn't control the panel any more than a DMA controller controls, say, the display controller. In fact, in earlier versions of OMAP DSS DT support I had a simpler port description, and in that I had the panel as the master (i.e. link from panel to dispc) because the panel driver uses the display controller's features to provide the panel device a data stream. And even with the current OMAP DSS DT version, which uses the v4l2 style ports/endpoints, the driver model is still the same, and only links towards upstream are used. So one reason I'm happy with the dual-linking is that I can easily follow the links from the downstream entities to upstream entities, and other people, who have different driver model, can easily do the opposite. But I agree that single-linking is enough and this can be handled at runtime, even if it makes the code more complex. And perhaps requires extra data in the dts, to give the start points for the graph. In theory unidirectional links in DT are indeed enough. However, let's not forget the following. - There's no such thing as single start points for graphs. Sure, in some simple cases the graph will have a single start point, but that's not a generic rule. For instance the camera graphs http://ideasonboard.org/media/omap3isp.ps and http://ideasonboard.org/media/eyecam.ps have two camera sensors, and thus two starting points from a data flow point of view. And if you want a better understanding of how complex media graphs can become, have a look at http://ideasonboard.org/media/vsp1.0.pdf (that's a real world example, albeit all connections are internal to the SoC in that particular case, and don't need to be described in DT). - There's also no such thing as a master device that can just point to slave devices. Once again simple cases exist where that model could work, but real world examples exist of complex pipelines with dozens of elements all implemented by a separate IP core and handled by separate drivers, forming a graph with long chains and branches. We thus need real graph bindings. - Finally, having no backlinks in DT would make the software implementation very complex. We need to be able to walk the graph in a generic way without having any of the IP core drivers loaded, and without any specific starting point. We would thus need to parse the complete DT tree, looking at all nodes and trying to find out whether they're part of the graph we're trying to walk. The complexity of the operation would be at best quadratic to the number of nodes in the whole DT and to the number of nodes in the graph. Not really. To being with, you cannot determine any meaning of a node across the tree (aside from it being an endpoint) without also understanding the binding that the node is a part of. That means you need to have something matching against the compatible string on both ends of the linkage. For instance: panel { compatible = acme,lvds-panel; lvds-port: port { }; }; display-controller { compatible = encom,video; port { remote-endpoint = lvds-port; }; }; In the above example, the encom,video driver has absolutely zero information about what the acme,lvds-panel binding actually implements. There needs to be both a driver for the acme,lvds-panel binding and one for the encom,video binding (even if the acme,lvds-panel binding is very thin and defers the functionality to the video controller). What you want here is the drivers to register each side of the connection. That could be modeled with something like the following (pseudocode): struct of_endpoint { struct list_head list; struct device_node *ep_node; void *context; void (*cb)(struct of_endpoint *ep, void *data); } int of_register_port(struct device *node, void (*cb)(struct of_endpoint *ep, void *data), void *data) { struct of_endpoint *ep = kzalloc(sizeof(*ep), GFP_KERNEL); ep-ep_node = node;
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
Hi Grant, On Monday 10 March 2014 14:58:15 Grant Likely wrote: On Mon, 10 Mar 2014 14:52:53 +0100, Laurent Pinchart wrote: On Monday 10 March 2014 12:18:20 Tomi Valkeinen wrote: On 08/03/14 13:41, Grant Likely wrote: Ok. If we go for single directional link, the question is then: which way? And is the direction different for display and camera, which are kind of reflections of each other? In general I would recommend choosing whichever device you would sensibly think of as a master. In the camera case I would choose the camera controller node instead of the camera itself, and in the display case I would choose the display controller instead of the panel. The binding author needs to choose what she things makes the most sense, but drivers can still use if it it turns out to be 'backwards' I would perhaps choose the same approach, but at the same time I think it's all but clear. The display controller doesn't control the panel any more than a DMA controller controls, say, the display controller. In fact, in earlier versions of OMAP DSS DT support I had a simpler port description, and in that I had the panel as the master (i.e. link from panel to dispc) because the panel driver uses the display controller's features to provide the panel device a data stream. And even with the current OMAP DSS DT version, which uses the v4l2 style ports/endpoints, the driver model is still the same, and only links towards upstream are used. So one reason I'm happy with the dual-linking is that I can easily follow the links from the downstream entities to upstream entities, and other people, who have different driver model, can easily do the opposite. But I agree that single-linking is enough and this can be handled at runtime, even if it makes the code more complex. And perhaps requires extra data in the dts, to give the start points for the graph. In theory unidirectional links in DT are indeed enough. However, let's not forget the following. - There's no such thing as single start points for graphs. Sure, in some simple cases the graph will have a single start point, but that's not a generic rule. For instance the camera graphs http://ideasonboard.org/media/omap3isp.ps and http://ideasonboard.org/media/eyecam.ps have two camera sensors, and thus two starting points from a data flow point of view. And if you want a better understanding of how complex media graphs can become, have a look at http://ideasonboard.org/media/vsp1.0.pdf (that's a real world example, albeit all connections are internal to the SoC in that particular case, and don't need to be described in DT). - There's also no such thing as a master device that can just point to slave devices. Once again simple cases exist where that model could work, but real world examples exist of complex pipelines with dozens of elements all implemented by a separate IP core and handled by separate drivers, forming a graph with long chains and branches. We thus need real graph bindings. - Finally, having no backlinks in DT would make the software implementation very complex. We need to be able to walk the graph in a generic way without having any of the IP core drivers loaded, and without any specific starting point. We would thus need to parse the complete DT tree, looking at all nodes and trying to find out whether they're part of the graph we're trying to walk. The complexity of the operation would be at best quadratic to the number of nodes in the whole DT and to the number of nodes in the graph. Not really. To being with, you cannot determine any meaning of a node across the tree (aside from it being an endpoint) That's the important part. I can assume the target node of the remote-endpoint phandle to be an endpoint, and can thus assume that it implements the of-graph bindings. That's all I need to be able to walk the graph in a generic way. without also understanding the binding that the node is a part of. That means you need to have something matching against the compatible string on both ends of the linkage. For instance: panel { compatible = acme,lvds-panel; lvds-port: port { }; }; display-controller { compatible = encom,video; port { remote-endpoint = lvds-port; }; }; In the above example, the encom,video driver has absolutely zero information about what the acme,lvds-panel binding actually implements. There needs to be both a driver for the acme,lvds-panel binding and one for the encom,video binding (even if the acme,lvds-panel binding is very thin and defers the functionality to the video controller). I absolutely agree with that. We need a driver for each device (in this case the acme panel and the encom display controller), and we need those drivers to register entities (in the generic sense of
Re: [PATCH 7/7] [media] adv7180: Add support for power down
On 03/10/2014 03:37 PM, Hans Verkuil wrote: [...] + +static int adv7180_s_power(struct v4l2_subdev *sd, int on) +{ + struct adv7180_state *state = to_state(sd); + struct i2c_client *client = v4l2_get_subdevdata(sd); + int ret; + + ret = mutex_lock_interruptible(state-mutex); + if (ret) + return ret; + + ret = adv7180_set_power(state, client, on); + if (ret) + goto out; + + state-powered = on; +out: I would change this to: if (!ret) state-powered = on; and drop the 'goto'. ok. [...] static int adv7180_resume(struct device *dev) @@ -656,10 +687,11 @@ static int adv7180_resume(struct device *dev) struct adv7180_state *state = to_state(sd); int ret; - ret = i2c_smbus_write_byte_data(client, ADV7180_PWR_MAN_REG, - ADV7180_PWR_MAN_ON); - if (ret 0) - return ret; + if (state-powered) { + ret = adv7180_set_power(state, client, true); + if (ret) + return ret; + } ret = init_device(client, state); if (ret 0) return ret; What is the initial state of the driver when loaded? Shouldn't probe() set the 'powered' variable to true initially? Yep, st-powered should be set to true by default. What's your process in general, want me to resend the whole series, or are you going to apply the patches that are ok? Thanks for the quick review, - Lars -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] [media] adv7180: Add support for power down
On 03/10/2014 04:24 PM, Lars-Peter Clausen wrote: On 03/10/2014 03:37 PM, Hans Verkuil wrote: [...] + +static int adv7180_s_power(struct v4l2_subdev *sd, int on) +{ + struct adv7180_state *state = to_state(sd); + struct i2c_client *client = v4l2_get_subdevdata(sd); + int ret; + + ret = mutex_lock_interruptible(state-mutex); + if (ret) + return ret; + + ret = adv7180_set_power(state, client, on); + if (ret) + goto out; + + state-powered = on; +out: I would change this to: if (!ret) state-powered = on; and drop the 'goto'. ok. [...] static int adv7180_resume(struct device *dev) @@ -656,10 +687,11 @@ static int adv7180_resume(struct device *dev) struct adv7180_state *state = to_state(sd); int ret; - ret = i2c_smbus_write_byte_data(client, ADV7180_PWR_MAN_REG, - ADV7180_PWR_MAN_ON); - if (ret 0) - return ret; + if (state-powered) { + ret = adv7180_set_power(state, client, true); + if (ret) + return ret; + } ret = init_device(client, state); if (ret 0) return ret; What is the initial state of the driver when loaded? Shouldn't probe() set the 'powered' variable to true initially? Yep, st-powered should be set to true by default. What's your process in general, want me to resend the whole series, or are you going to apply the patches that are ok? When do you think you can have a fixed version of this patch ready? If it is today/tomorrow, then I'll wait for that, otherwise I'll make a pull request for the other 6 patches (which look fine). In any case there is no need to resend the whole series. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rtl2832u_sdr: fixing v4l2-compliance issues
Moikka Hans! On 06.03.2014 01:21, Hans Verkuil wrote: Antti, Attached is a patch that fixed all but one v4l2-compliance error: fail: v4l2-test-controls.cpp(295): returned control value out of range fail: v4l2-test-controls.cpp(357): invalid control 00a2090c test VIDIOC_G/S_CTRL: FAIL fail: v4l2-test-controls.cpp(465): returned control value out of range fail: v4l2-test-controls.cpp(573): invalid control 00a2090c test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL That's the BANDWIDTH control and it returned value 320 when the minimum was 600. I couldn't trace where that came from in the limited time I spent on it, I expect you can find it much quicker. That is because I added native V4L2_CID_RF_TUNER_BANDWIDTH support only for E4000 tuner driver. The others, FC0012, FC0013 and R820T are set via DVB API by rtl2832_sdr driver, which is quite hackish solution. Devices having E4000 works correctly. Dunno if it wise to hack rtl2832_sdr and clamp values to valid per tuner or leave it as it is. Adding V4L2_CID_RF_TUNER_BANDWIDTH to those 3 tuner drivers is also quite trivial... regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] [media] adv7180: Add support for power down
On 03/10/2014 04:28 PM, Hans Verkuil wrote: On 03/10/2014 04:24 PM, Lars-Peter Clausen wrote: On 03/10/2014 03:37 PM, Hans Verkuil wrote: [...] + +static int adv7180_s_power(struct v4l2_subdev *sd, int on) +{ + struct adv7180_state *state = to_state(sd); + struct i2c_client *client = v4l2_get_subdevdata(sd); + int ret; + + ret = mutex_lock_interruptible(state-mutex); + if (ret) + return ret; + + ret = adv7180_set_power(state, client, on); + if (ret) + goto out; + + state-powered = on; +out: I would change this to: if (!ret) state-powered = on; and drop the 'goto'. ok. [...] static int adv7180_resume(struct device *dev) @@ -656,10 +687,11 @@ static int adv7180_resume(struct device *dev) struct adv7180_state *state = to_state(sd); int ret; - ret = i2c_smbus_write_byte_data(client, ADV7180_PWR_MAN_REG, - ADV7180_PWR_MAN_ON); - if (ret 0) - return ret; + if (state-powered) { + ret = adv7180_set_power(state, client, true); + if (ret) + return ret; + } ret = init_device(client, state); if (ret 0) return ret; What is the initial state of the driver when loaded? Shouldn't probe() set the 'powered' variable to true initially? Yep, st-powered should be set to true by default. What's your process in general, want me to resend the whole series, or are you going to apply the patches that are ok? When do you think you can have a fixed version of this patch ready? If it is today/tomorrow, then I'll wait for that, otherwise I'll make a pull request for the other 6 patches (which look fine). I already have the updated version of the patch, will send it out soon. - Lars -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rtl2832u_sdr: fixing v4l2-compliance issues
On 03/10/2014 04:29 PM, Antti Palosaari wrote: Moikka Hans! On 06.03.2014 01:21, Hans Verkuil wrote: Antti, Attached is a patch that fixed all but one v4l2-compliance error: fail: v4l2-test-controls.cpp(295): returned control value out of range fail: v4l2-test-controls.cpp(357): invalid control 00a2090c test VIDIOC_G/S_CTRL: FAIL fail: v4l2-test-controls.cpp(465): returned control value out of range fail: v4l2-test-controls.cpp(573): invalid control 00a2090c test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL That's the BANDWIDTH control and it returned value 320 when the minimum was 600. I couldn't trace where that came from in the limited time I spent on it, I expect you can find it much quicker. That is because I added native V4L2_CID_RF_TUNER_BANDWIDTH support only for E4000 tuner driver. The others, FC0012, FC0013 and R820T are set via DVB API by rtl2832_sdr driver, which is quite hackish solution. Devices having E4000 works correctly. Dunno if it wise to hack rtl2832_sdr and clamp values to valid per tuner or leave it as it is. Adding V4L2_CID_RF_TUNER_BANDWIDTH to those 3 tuner drivers is also quite trivial... I recommend whatever is the best long-term solution :-) It's good practice to fix such compliance errors. One reason is that v4l2-compliance generally stops testing whatever ioctl it is testing once it finds a problem, so there may be other failures lurking behind this one that v4l2-compliance won't find. The other reason is that, well, it's a bug! So it should be fixed anyway. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rtl2832u_sdr: fixing v4l2-compliance issues
On 10.03.2014 17:36, Hans Verkuil wrote: On 03/10/2014 04:29 PM, Antti Palosaari wrote: Moikka Hans! On 06.03.2014 01:21, Hans Verkuil wrote: Antti, Attached is a patch that fixed all but one v4l2-compliance error: fail: v4l2-test-controls.cpp(295): returned control value out of range fail: v4l2-test-controls.cpp(357): invalid control 00a2090c test VIDIOC_G/S_CTRL: FAIL fail: v4l2-test-controls.cpp(465): returned control value out of range fail: v4l2-test-controls.cpp(573): invalid control 00a2090c test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL That's the BANDWIDTH control and it returned value 320 when the minimum was 600. I couldn't trace where that came from in the limited time I spent on it, I expect you can find it much quicker. That is because I added native V4L2_CID_RF_TUNER_BANDWIDTH support only for E4000 tuner driver. The others, FC0012, FC0013 and R820T are set via DVB API by rtl2832_sdr driver, which is quite hackish solution. Devices having E4000 works correctly. Dunno if it wise to hack rtl2832_sdr and clamp values to valid per tuner or leave it as it is. Adding V4L2_CID_RF_TUNER_BANDWIDTH to those 3 tuner drivers is also quite trivial... I recommend whatever is the best long-term solution :-) It's good practice to fix such compliance errors. One reason is that v4l2-compliance generally stops testing whatever ioctl it is testing once it finds a problem, so there may be other failures lurking behind this one that v4l2-compliance won't find. The other reason is that, well, it's a bug! So it should be fixed anyway. Yeah it is bug. But I decided to convert only e4000 and wait some feedback until add v4l2 controls to those other supported drivers... OK, lets see if I can convert at least those FC0012 and FC0013 drivers now. That sdr driver is still on staging ;) regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
Am Montag, den 10.03.2014, 16:15 +0100 schrieb Laurent Pinchart: Hi Grant, On Monday 10 March 2014 14:58:15 Grant Likely wrote: On Mon, 10 Mar 2014 14:52:53 +0100, Laurent Pinchart wrote: On Monday 10 March 2014 12:18:20 Tomi Valkeinen wrote: On 08/03/14 13:41, Grant Likely wrote: Ok. If we go for single directional link, the question is then: which way? And is the direction different for display and camera, which are kind of reflections of each other? In general I would recommend choosing whichever device you would sensibly think of as a master. In the camera case I would choose the camera controller node instead of the camera itself, and in the display case I would choose the display controller instead of the panel. The binding author needs to choose what she things makes the most sense, but drivers can still use if it it turns out to be 'backwards' I would perhaps choose the same approach, but at the same time I think it's all but clear. The display controller doesn't control the panel any more than a DMA controller controls, say, the display controller. In fact, in earlier versions of OMAP DSS DT support I had a simpler port description, and in that I had the panel as the master (i.e. link from panel to dispc) because the panel driver uses the display controller's features to provide the panel device a data stream. And even with the current OMAP DSS DT version, which uses the v4l2 style ports/endpoints, the driver model is still the same, and only links towards upstream are used. So one reason I'm happy with the dual-linking is that I can easily follow the links from the downstream entities to upstream entities, and other people, who have different driver model, can easily do the opposite. But I agree that single-linking is enough and this can be handled at runtime, even if it makes the code more complex. And perhaps requires extra data in the dts, to give the start points for the graph. In theory unidirectional links in DT are indeed enough. However, let's not forget the following. - There's no such thing as single start points for graphs. Sure, in some simple cases the graph will have a single start point, but that's not a generic rule. For instance the camera graphs http://ideasonboard.org/media/omap3isp.ps and http://ideasonboard.org/media/eyecam.ps have two camera sensors, and thus two starting points from a data flow point of view. And if you want a better understanding of how complex media graphs can become, have a look at http://ideasonboard.org/media/vsp1.0.pdf (that's a real world example, albeit all connections are internal to the SoC in that particular case, and don't need to be described in DT). - There's also no such thing as a master device that can just point to slave devices. Once again simple cases exist where that model could work, but real world examples exist of complex pipelines with dozens of elements all implemented by a separate IP core and handled by separate drivers, forming a graph with long chains and branches. We thus need real graph bindings. - Finally, having no backlinks in DT would make the software implementation very complex. We need to be able to walk the graph in a generic way without having any of the IP core drivers loaded, and without any specific starting point. We would thus need to parse the complete DT tree, looking at all nodes and trying to find out whether they're part of the graph we're trying to walk. The complexity of the operation would be at best quadratic to the number of nodes in the whole DT and to the number of nodes in the graph. Not really. To being with, you cannot determine any meaning of a node across the tree (aside from it being an endpoint) That's the important part. I can assume the target node of the remote-endpoint phandle to be an endpoint, and can thus assume that it implements the of-graph bindings. That's all I need to be able to walk the graph in a generic way. without also understanding the binding that the node is a part of. That means you need to have something matching against the compatible string on both ends of the linkage. For instance: panel { compatible = acme,lvds-panel; lvds-port: port { }; }; display-controller { compatible = encom,video; port { remote-endpoint = lvds-port; }; }; In the above example, the encom,video driver has absolutely zero information about what the acme,lvds-panel binding actually implements. There needs to be both a driver for the acme,lvds-panel binding and one for the encom,video binding (even if the acme,lvds-panel binding is very thin and defers the functionality to the video controller). I absolutely agree with that. We need a driver
Re: [REVIEW PATCH 0/3] Add g_tvnorms video op
Hi Hans, Thank you for the patches. On Monday 17 February 2014 12:44:11 Hans Verkuil wrote: This patch series addresses a problem that was exposed by commit a5338190e. The issue is that soc_camera implements s/g_std ioctls and just forwards those to the subdev, whether or not the subdev actually implements them. In addition, tvnorms is never set, so even if the subdev implements the s/g_std the ENUMSTD ioctl will not report anything. The solution is to add a g_tvnorms video op to v4l2_subdev (there was already a g_tvnorms_output, so that fits nicely) and to let soc_camera call that so the video_device tvnorms field is set correctly. Before registering the video node it will check if tvnorms == 0 and disable the STD ioctls if that's the case. While this problem cropped up in soc_camera it is really a problem for any generic bridge driver, so this is useful to have. Note that it is untested. The plan is that Laurent tests and Guennadi pulls it into his tree. I've tested the series on v3.10 with the atmel-isi driver. Without the patches applied ENUMSTD returns -ENODATA, and with the patches applied it returns - ENOTTY. Tested-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question about set format call check for vb2_is_busy
On Mon, Mar 10, 2014 at 12:21 AM, Hans Verkuil hverk...@xs4all.nl wrote: On 03/10/2014 08:02 AM, m silverstri wrote: Hi, I am studying v4l2 m2m driver example. I want to know why the set format function in the example fails when it is called again after user application req_buf? In set format function checks for vb2_is_busy(vq) and that function returns true after user space app calls req_buf. For example in here: http://stuff.mit.edu/afs/sipb/contrib/linux/drivers/media/platform/mem2mem_testdev.c static int vidioc_s_fmt(struct m2mtest_ctx *ctx, struct v4l2_format *f) { //... // Check for vb2_is_busy() here: if (vb2_is_busy(vq)) { v4l2_err(ctx-dev-v4l2_dev, %s queue busy\n, __func__); return -EBUSY; } //... } Why the driver prevents user space application change format after it request buffers? When you request the buffers they will be allocated based on the current format. Changing the format later will mean a change in buffer size, but once the buffers are allocated that is locked in place. It's generally a bad idea to, say, increase the image size and then watch how DMA overwrites your memory :-) This is not strictly speaking a v4l limitation, but a limitation of almost all hardware. It is possible to allow format changes after reqbufs is called, but that generally requires that the buffers all have the maximum possible size which wastes a lot of memory. And in addition you would have to have some sort of metadata as part of the captured frame so you know the actual size of the image stored in the buffer. None of the drivers in the kernel support this, BTW. Regards, Hans Hans, Thank you. Regards, Mike -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] [media] adv7180: Add support for power down
The adv7180 has a low power mode in which the analog and the digital processing section are shut down. Implement the s_power callback to let bridge drivers put the part into low power mode when not needed. Signed-off-by: Lars-Peter Clausen l...@metafoo.de --- Changes since v1: * Set powered to true in probe() since the part is powered on when it comes out of reset * Avoid use ofgoto in adv7180_s_power --- drivers/media/i2c/adv7180.c | 52 - 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c index 623cec5..9cfc9a3 100644 --- a/drivers/media/i2c/adv7180.c +++ b/drivers/media/i2c/adv7180.c @@ -127,6 +127,7 @@ struct adv7180_state { int irq; v4l2_std_id curr_norm; boolautodetect; + boolpowered; u8 input; }; #define to_adv7180_sd(_ctrl) (container_of(_ctrl-handler,\ @@ -311,6 +312,37 @@ out: return ret; } +static int adv7180_set_power(struct adv7180_state *state, + struct i2c_client *client, bool on) +{ + u8 val; + + if (on) + val = ADV7180_PWR_MAN_ON; + else + val = ADV7180_PWR_MAN_OFF; + + return i2c_smbus_write_byte_data(client, ADV7180_PWR_MAN_REG, val); +} + +static int adv7180_s_power(struct v4l2_subdev *sd, int on) +{ + struct adv7180_state *state = to_state(sd); + struct i2c_client *client = v4l2_get_subdevdata(sd); + int ret; + + ret = mutex_lock_interruptible(state-mutex); + if (ret) + return ret; + + ret = adv7180_set_power(state, client, on); + if (ret == 0) + state-powered = on; + + mutex_unlock(state-mutex); + return ret; +} + static int adv7180_s_ctrl(struct v4l2_ctrl *ctrl) { struct v4l2_subdev *sd = to_adv7180_sd(ctrl); @@ -441,6 +473,7 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops = { static const struct v4l2_subdev_core_ops adv7180_core_ops = { .s_std = adv7180_s_std, + .s_power = adv7180_s_power, }; static const struct v4l2_subdev_ops adv7180_ops = { @@ -587,6 +620,7 @@ static int adv7180_probe(struct i2c_client *client, state-irq = client-irq; mutex_init(state-mutex); state-autodetect = true; + state-powered = true; state-input = 0; sd = state-sd; v4l2_i2c_subdev_init(sd, client, adv7180_ops); @@ -640,13 +674,10 @@ static const struct i2c_device_id adv7180_id[] = { static int adv7180_suspend(struct device *dev) { struct i2c_client *client = to_i2c_client(dev); - int ret; + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct adv7180_state *state = to_state(sd); - ret = i2c_smbus_write_byte_data(client, ADV7180_PWR_MAN_REG, - ADV7180_PWR_MAN_OFF); - if (ret 0) - return ret; - return 0; + return adv7180_set_power(state, client, false); } static int adv7180_resume(struct device *dev) @@ -656,10 +687,11 @@ static int adv7180_resume(struct device *dev) struct adv7180_state *state = to_state(sd); int ret; - ret = i2c_smbus_write_byte_data(client, ADV7180_PWR_MAN_REG, - ADV7180_PWR_MAN_ON); - if (ret 0) - return ret; + if (state-powered) { + ret = adv7180_set_power(state, client, true); + if (ret) + return ret; + } ret = init_device(client, state); if (ret 0) return ret; -- 1.8.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FOR v3.15] adv7180 fixes
The following changes since commit f2d7313534072a5fe192e7cf46204b413acef479: [media] drx-d: add missing braces in drxd_hard.c:DRXD_init (2014-03-09 09:20:50 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v3.15e for you to fetch changes up to 72f965c6bbff5f0b3290e5019c90496dcb9462fe: adv7180: Add support for power down (2014-03-10 18:12:26 +0100) Lars-Peter Clausen (7): adv7180: Fix remove order adv7180: Free control handler on remove() adv7180: Remove unnecessary v4l2_device_unregister_subdev() from probe error path adv7180: Remove duplicated probe error message adv7180: Use threaded IRQ instead of IRQ + workqueue adv7180: Add support for async device registration adv7180: Add support for power down drivers/media/i2c/adv7180.c | 100 +--- 1 file changed, 59 insertions(+), 41 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 4/8] of: Reduce indentation in of_graph_get_next_endpoint
Hi Philipp, On Friday 07 March 2014 18:40:54 Philipp Zabel wrote: Am Freitag, den 07.03.2014, 01:12 +0100 schrieb Laurent Pinchart: Hi Philipp, Thank you for the patch. I've submitted a fix for the of_graph_get_next_endpoint() function, but it hasn't been applied yet due to the patch series that contained it needing more work. The patch is available at https://patchwork.linuxtv.org/patch/21946/. I can rebase it on top of this series, but I still wanted to let you know about it in case you would like to integrate it. Thank you for the pointer. A pity about the timing, this will mostly revert my indentation patch. I'd be glad if you could rebase on top of the merged series. While we look at of_graph_get_next_endpoint(), could you explain the reason behind the extra reference count increase on the prev node: /* * Avoid dropping prev node refcount to 0 when getting the next * child below. */ of_node_get(prev); This unfortunately makes using the function in for_each style macros a hassle. If that part wasn't there and all users that want to keep using prev after the call were expected to increase refcount themselves, we could have a #define of_graph_for_each_endpoint(parent, endpoint) \ for (endpoint = of_graph_get_next_endpoint(parent, NULL); \ endpoint != NULL; \ endpoint = of_graph_get_next_endpoint(parent, endpoint)) I don't know what the exact design decision was (Sylwester might know), but I suspect it's mostly about historical reasons. I see no reason that would prevent modifying the current behaviour to make a for-each loop easier to implement. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[FINAL PATCH 6/6] msi3101: fix v4l2-compliance issues
Fix msi3101 driver v4l2-compliance issues. Cc: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/staging/media/msi3101/sdr-msi3101.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/staging/media/msi3101/sdr-msi3101.c b/drivers/staging/media/msi3101/sdr-msi3101.c index 93e6eba..011db2c 100644 --- a/drivers/staging/media/msi3101/sdr-msi3101.c +++ b/drivers/staging/media/msi3101/sdr-msi3101.c @@ -1134,6 +1134,7 @@ static int msi3101_g_fmt_sdr_cap(struct file *file, void *priv, dev_dbg(s-udev-dev, %s: pixelformat fourcc %4.4s\n, __func__, (char *)s-pixelformat); + memset(f-fmt.sdr.reserved, 0, sizeof(f-fmt.sdr.reserved)); f-fmt.sdr.pixelformat = s-pixelformat; return 0; @@ -1151,6 +1152,7 @@ static int msi3101_s_fmt_sdr_cap(struct file *file, void *priv, if (vb2_is_busy(q)) return -EBUSY; + memset(f-fmt.sdr.reserved, 0, sizeof(f-fmt.sdr.reserved)); for (i = 0; i NUM_FORMATS; i++) { if (formats[i].pixelformat == f-fmt.sdr.pixelformat) { s-pixelformat = f-fmt.sdr.pixelformat; @@ -1172,6 +1174,7 @@ static int msi3101_try_fmt_sdr_cap(struct file *file, void *priv, dev_dbg(s-udev-dev, %s: pixelformat fourcc %4.4s\n, __func__, (char *)f-fmt.sdr.pixelformat); + memset(f-fmt.sdr.reserved, 0, sizeof(f-fmt.sdr.reserved)); for (i = 0; i NUM_FORMATS; i++) { if (formats[i].pixelformat == f-fmt.sdr.pixelformat) return 0; @@ -1233,6 +1236,7 @@ static int msi3101_g_frequency(struct file *file, void *priv, f-frequency = s-f_adc; ret = 0; } else if (f-tuner == 1) { + f-type = V4L2_TUNER_RF; ret = v4l2_subdev_call(s-v4l2_subdev, tuner, g_frequency, f); } else { ret = -EINVAL; -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[FINAL PATCH 3/6] rtl2832_sdr: clamp bandwidth to nearest legal value in automode
Clamp bandwidth to nearest legal value in automode in order to pass v4l2-compliance test. Reported-by: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c b/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c index bcc632a..c2c3c3d 100644 --- a/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c +++ b/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c @@ -1324,8 +1324,16 @@ static int rtl2832_sdr_s_ctrl(struct v4l2_ctrl *ctrl) switch (ctrl-id) { case V4L2_CID_RF_TUNER_BANDWIDTH_AUTO: case V4L2_CID_RF_TUNER_BANDWIDTH: - if (s-bandwidth_auto-val) - s-bandwidth-val = s-f_adc; + /* TODO: these controls should be moved to tuner drivers */ + if (s-bandwidth_auto-val) { + /* Round towards the closest legal value */ + s32 val = s-f_adc + s-bandwidth-step / 2; + u32 offset; + val = clamp(val, s-bandwidth-minimum, s-bandwidth-maximum); + offset = val - s-bandwidth-minimum; + offset = s-bandwidth-step * (offset / s-bandwidth-step); + s-bandwidth-val = s-bandwidth-minimum + offset; + } c-bandwidth_hz = s-bandwidth-val; -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[FINAL PATCH 5/6] msi001: fix v4l2-compliance issues
Fix msi001 driver v4l2-compliance issues. Cc: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/staging/media/msi3101/msi001.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/media/msi3101/msi001.c b/drivers/staging/media/msi3101/msi001.c index 25feece..ac43bae 100644 --- a/drivers/staging/media/msi3101/msi001.c +++ b/drivers/staging/media/msi3101/msi001.c @@ -429,6 +429,7 @@ static int msi001_probe(struct spi_device *spi) } s-spi = spi; + s-f_tuner = bands[0].rangelow; v4l2_spi_subdev_init(s-sd, spi, msi001_ops); /* Register controls */ -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[FINAL PATCH 2/6] rtl2832u_sdr: fixing v4l2-compliance issues
From: Hans Verkuil hverk...@xs4all.nl Fix rtl2832u_sdr driver v4l2-compliance issues. Signed-off-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c | 29 +++- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c b/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c index 932716f..bcc632a 100644 --- a/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c +++ b/drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c @@ -120,6 +120,7 @@ struct rtl2832_sdr_state { struct vb2_queue vb_queue; struct list_head queued_bufs; spinlock_t queued_bufs_lock; /* Protects queued_bufs */ + unsigned sequence; /* buffer sequence counter */ /* Note if taking both locks v4l2_lock must always be locked first! */ struct mutex v4l2_lock; /* Protects everything else */ @@ -415,6 +416,8 @@ static void rtl2832_sdr_urb_complete(struct urb *urb) len = rtl2832_sdr_convert_stream(s, ptr, urb-transfer_buffer, urb-actual_length); vb2_set_plane_payload(fbuf-vb, 0, len); + v4l2_get_timestamp(fbuf-vb.v4l2_buf.timestamp); + fbuf-vb.v4l2_buf.sequence = s-sequence++; vb2_buffer_done(fbuf-vb, VB2_BUF_STATE_DONE); } skip: @@ -611,8 +614,9 @@ static int rtl2832_sdr_queue_setup(struct vb2_queue *vq, struct rtl2832_sdr_state *s = vb2_get_drv_priv(vq); dev_dbg(s-udev-dev, %s: *nbuffers=%d\n, __func__, *nbuffers); - /* Absolute min and max number of buffers available for mmap() */ - *nbuffers = clamp_t(unsigned int, *nbuffers, 8, 32); + /* Need at least 8 buffers */ + if (vq-num_buffers + *nbuffers 8) + *nbuffers = 8 - vq-num_buffers; *nplanes = 1; /* 2 = max 16-bit sample returned */ sizes[0] = PAGE_ALIGN(BULK_BUFFER_SIZE * 2); @@ -1013,6 +1017,8 @@ static int rtl2832_sdr_start_streaming(struct vb2_queue *vq, unsigned int count) if (ret) goto err; + s-sequence = 0; + ret = rtl2832_sdr_submit_urbs(s); if (ret) goto err; @@ -1090,6 +1096,8 @@ static int rtl2832_sdr_s_tuner(struct file *file, void *priv, struct rtl2832_sdr_state *s = video_drvdata(file); dev_dbg(s-udev-dev, %s:\n, __func__); + if (v-index 1) + return -EINVAL; return 0; } @@ -1125,12 +1133,15 @@ static int rtl2832_sdr_g_frequency(struct file *file, void *priv, dev_dbg(s-udev-dev, %s: tuner=%d type=%d\n, __func__, f-tuner, f-type); - if (f-tuner == 0) + if (f-tuner == 0) { f-frequency = s-f_adc; - else if (f-tuner == 1) + f-type = V4L2_TUNER_ADC; + } else if (f-tuner == 1) { f-frequency = s-f_tuner; - else + f-type = V4L2_TUNER_RF; + } else { return -EINVAL; + } return ret; } @@ -1164,7 +1175,9 @@ static int rtl2832_sdr_s_frequency(struct file *file, void *priv, __func__, s-f_adc); ret = rtl2832_sdr_set_adc(s); } else if (f-tuner == 1) { - s-f_tuner = f-frequency; + s-f_tuner = clamp_t(unsigned int, f-frequency, + bands_fm[0].rangelow, + bands_fm[0].rangehigh); dev_dbg(s-udev-dev, %s: RF frequency=%u Hz\n, __func__, f-frequency); @@ -1198,6 +1211,7 @@ static int rtl2832_sdr_g_fmt_sdr_cap(struct file *file, void *priv, dev_dbg(s-udev-dev, %s:\n, __func__); f-fmt.sdr.pixelformat = s-pixelformat; + memset(f-fmt.sdr.reserved, 0, sizeof(f-fmt.sdr.reserved)); return 0; } @@ -1214,6 +1228,7 @@ static int rtl2832_sdr_s_fmt_sdr_cap(struct file *file, void *priv, if (vb2_is_busy(q)) return -EBUSY; + memset(f-fmt.sdr.reserved, 0, sizeof(f-fmt.sdr.reserved)); for (i = 0; i NUM_FORMATS; i++) { if (formats[i].pixelformat == f-fmt.sdr.pixelformat) { s-pixelformat = f-fmt.sdr.pixelformat; @@ -1235,6 +1250,7 @@ static int rtl2832_sdr_try_fmt_sdr_cap(struct file *file, void *priv, dev_dbg(s-udev-dev, %s: pixelformat fourcc %4.4s\n, __func__, (char *)f-fmt.sdr.pixelformat); + memset(f-fmt.sdr.reserved, 0, sizeof(f-fmt.sdr.reserved)); for (i = 0; i NUM_FORMATS; i++) { if (formats[i].pixelformat == f-fmt.sdr.pixelformat) return 0; @@ -1365,6 +1381,7 @@ struct dvb_frontend *rtl2832_sdr_attach(struct dvb_frontend *fe, s-i2c = i2c; s-cfg = cfg; s-f_adc = bands_adc[0].rangelow; + s-f_tuner =
[FINAL PATCH 4/6] rtl28xxu: depends on I2C_MUX
We need depend on I2C_MUX as rtl2832 demod used requires it. All error/warnings: warning: (DVB_USB_RTL28XXU) selects DVB_RTL2832 which has unmet direct dependencies (MEDIA_SUPPORT DVB_CORE I2C I2C_MUX) ERROR: i2c_add_mux_adapter [drivers/media/dvb-frontends/rtl2832.ko] undefined! ERROR: i2c_del_mux_adapter [drivers/media/dvb-frontends/rtl2832.ko] undefined! Reported-by: kbuild test robot fengguang...@intel.com Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/usb/dvb-usb-v2/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/dvb-usb-v2/Kconfig b/drivers/media/usb/dvb-usb-v2/Kconfig index bfb7378..037e519 100644 --- a/drivers/media/usb/dvb-usb-v2/Kconfig +++ b/drivers/media/usb/dvb-usb-v2/Kconfig @@ -126,7 +126,7 @@ config DVB_USB_MXL111SF config DVB_USB_RTL28XXU tristate Realtek RTL28xxU DVB USB support - depends on DVB_USB_V2 + depends on DVB_USB_V2 I2C_MUX select DVB_RTL2830 select DVB_RTL2832 select MEDIA_TUNER_QT1010 if MEDIA_SUBDRV_AUTOSELECT -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[FINAL PATCH 1/6] v4l: rename v4l2_format_sdr to v4l2_sdr_format
Rename v4l2_format_sdr to v4l2_sdr_format in order to keep it in line with other formats. Reported-by: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Antti Palosaari cr...@iki.fi --- Documentation/DocBook/media/v4l/dev-sdr.xml | 2 +- drivers/media/v4l2-core/v4l2-ioctl.c| 2 +- include/uapi/linux/videodev2.h | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/DocBook/media/v4l/dev-sdr.xml b/Documentation/DocBook/media/v4l/dev-sdr.xml index ac9f1af..524b9c4 100644 --- a/Documentation/DocBook/media/v4l/dev-sdr.xml +++ b/Documentation/DocBook/media/v4l/dev-sdr.xml @@ -78,7 +78,7 @@ of the data format. /para table pgwide=1 frame=none id=v4l2-format-sdr - titlestruct structnamev4l2_format_sdr/structname/title + titlestruct structnamev4l2_sdr_format/structname/title tgroup cols=3 cs-str; tbody valign=top diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 95dd4f1..e6e86a2 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -246,7 +246,7 @@ static void v4l_print_format(const void *arg, bool write_only) const struct v4l2_vbi_format *vbi; const struct v4l2_sliced_vbi_format *sliced; const struct v4l2_window *win; - const struct v4l2_format_sdr *sdr; + const struct v4l2_sdr_format *sdr; unsigned i; pr_cont(type=%s, prt_names(p-type, v4l2_type_names)); diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index d3187d8..5f10ed9 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1714,10 +1714,10 @@ struct v4l2_pix_format_mplane { } __attribute__ ((packed)); /** - * struct v4l2_format_sdr - SDR format definition + * struct v4l2_sdr_format - SDR format definition * @pixelformat: little endian four character code (fourcc) */ -struct v4l2_format_sdr { +struct v4l2_sdr_format { __u32 pixelformat; __u8reserved[28]; } __attribute__ ((packed)); @@ -1740,7 +1740,7 @@ struct v4l2_format { struct v4l2_window win; /* V4L2_BUF_TYPE_VIDEO_OVERLAY */ struct v4l2_vbi_format vbi; /* V4L2_BUF_TYPE_VBI_CAPTURE */ struct v4l2_sliced_vbi_format sliced; /* V4L2_BUF_TYPE_SLICED_VBI_CAPTURE */ - struct v4l2_format_sdr sdr; /* V4L2_BUF_TYPE_SDR_CAPTURE */ + struct v4l2_sdr_format sdr; /* V4L2_BUF_TYPE_SDR_CAPTURE */ __u8raw_data[200]; /* user-defined */ } fmt; }; -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL] SDR API
That is just same set I sent earlier too, but rebased to latest media/master and 6 small compliance fix. The following changes since commit f2d7313534072a5fe192e7cf46204b413acef479: [media] drx-d: add missing braces in drxd_hard.c:DRXD_init (2014-03-09 09:20:50 -0300) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git sdr_review_v3 for you to fetch changes up to 2a02f2e9cf67c2e083950ea4b1d03a2756c1ee94: msi3101: fix v4l2-compliance issues (2014-03-10 21:30:44 +0200) Antti Palosaari (37): v4l: add RF tuner channel bandwidth control v4l: reorganize RF tuner control ID numbers v4l: uapi: add SDR formats CU8 and CU16LE v4l: add enum_freq_bands support to tuner sub-device v4l: add control for RF tuner PLL lock flag DocBook: V4L: add V4L2_SDR_FMT_CU8 - 'CU08' DocBook: V4L: add V4L2_SDR_FMT_CU16LE - 'CU16' DocBook: document RF tuner bandwidth controls DocBook: media: document PLL lock control DocBook: media: add some general info about RF tuners msi3101: convert to SDR API msi001: Mirics MSi001 silicon tuner driver msi3101: use msi001 tuner driver MAINTAINERS: add msi001 driver MAINTAINERS: add msi3101 driver msi3101: clamp mmap buffers to reasonable level e4000: convert DVB tuner to I2C driver model e4000: implement controls via v4l2 control framework e4000: fix PLL calc to allow higher frequencies e4000: implement PLL lock v4l control e4000: get rid of DVB i2c_gate_ctrl() e4000: convert to Regmap API e4000: rename some variables rtl2832_sdr: Realtek RTL2832 SDR driver module rtl28xxu: constify demod config structs rtl28xxu: attach SDR extension module rtl28xxu: fix switch-case style issue rtl28xxu: use muxed RTL2832 I2C adapters for E4000 and RTL2832_SDR rtl2832_sdr: expose e4000 controls to user r820t: add manual gain controls rtl2832_sdr: expose R820T controls to user MAINTAINERS: add rtl2832_sdr driver v4l: rename v4l2_format_sdr to v4l2_sdr_format rtl2832_sdr: clamp bandwidth to nearest legal value in automode rtl28xxu: depends on I2C_MUX msi001: fix v4l2-compliance issues msi3101: fix v4l2-compliance issues Hans Verkuil (1): rtl2832u_sdr: fixing v4l2-compliance issues Documentation/DocBook/media/v4l/controls.xml | 51 +++- Documentation/DocBook/media/v4l/dev-sdr.xml |2 +- Documentation/DocBook/media/v4l/pixfmt-sdr-cu08.xml | 44 Documentation/DocBook/media/v4l/pixfmt-sdr-cu16le.xml | 46 Documentation/DocBook/media/v4l/pixfmt.xml|3 + MAINTAINERS | 30 +++ drivers/media/tuners/Kconfig |1 + drivers/media/tuners/e4000.c | 598 +-- drivers/media/tuners/e4000.h | 21 +- drivers/media/tuners/e4000_priv.h | 86 ++- drivers/media/tuners/r820t.c | 137 ++- drivers/media/tuners/r820t.h | 10 + drivers/media/usb/dvb-usb-v2/Kconfig |2 +- drivers/media/usb/dvb-usb-v2/Makefile |1 + drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 90 +-- drivers/media/usb/dvb-usb-v2/rtl28xxu.h |2 + drivers/media/v4l2-core/v4l2-ctrls.c |9 + drivers/media/v4l2-core/v4l2-ioctl.c |2 +- drivers/staging/media/Kconfig |2 + drivers/staging/media/Makefile|2 + drivers/staging/media/msi3101/Kconfig |7 +- drivers/staging/media/msi3101/Makefile|1 + drivers/staging/media/msi3101/msi001.c| 500 +++ drivers/staging/media/msi3101/sdr-msi3101.c | 1564 +- drivers/staging/media/rtl2832u_sdr/Kconfig|7 + drivers/staging/media/rtl2832u_sdr/Makefile |6 + drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c | 1501 + drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.h | 51 include/media/v4l2-subdev.h |1 + include/uapi/linux/v4l2-controls.h| 15 +- include/uapi/linux/videodev2.h| 10 +- 31 files changed, 3531 insertions(+), 1271 deletions(-) create mode 100644 Documentation/DocBook/media/v4l/pixfmt-sdr-cu08.xml create mode 100644
vb2: various small fixes/improvements
This patch series contains a list of various vb2 fixes and improvements. These patches were originally part of this RFC patch series: http://www.spinics.net/lists/linux-media/msg73391.html They are now rebased and reordered a bit. It's little stuff for the most part, although the first patch touches on more drivers since it changes the return type of stop_streaming to void. The return value was always ignored by vb2 and you really cannot do anything sensible with it. In general resource allocations can return an error, but freeing up resources should not. That should always succeed. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW PATCH 01/11] vb2: stop_streaming should return void
From: Hans Verkuil hans.verk...@cisco.com The vb2 core ignores any return code from the stop_streaming op. And there really isn't anything it can do anyway in case of an error. So change the return type to void and update any drivers that implement it. The int return gave drivers the idea that this operation could actually fail, but that's really not the case. The pwc amd sdr-msi3101 drivers both had this construction: if (mutex_lock_interruptible(s-v4l2_lock)) return -ERESTARTSYS; This has been updated to just call mutex_lock(). The stop_streaming op expects this to really stop streaming and I very much doubt this will work reliably if stop_streaming just returns without really stopping the DMA. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/pci/sta2x11/sta2x11_vip.c | 3 +-- drivers/media/platform/blackfin/bfin_capture.c | 6 +- drivers/media/platform/coda.c| 4 +--- drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 +--- drivers/media/platform/exynos4-is/fimc-capture.c | 6 +++--- drivers/media/platform/exynos4-is/fimc-lite.c| 6 +++--- drivers/media/platform/exynos4-is/fimc-m2m.c | 3 +-- drivers/media/platform/marvell-ccic/mcam-core.c | 7 +++ drivers/media/platform/s3c-camif/camif-capture.c | 4 ++-- drivers/media/platform/s5p-jpeg/jpeg-core.c | 4 +--- drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 3 +-- drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 3 +-- drivers/media/platform/s5p-tv/mixer_video.c | 3 +-- drivers/media/platform/soc_camera/atmel-isi.c| 6 ++ drivers/media/platform/soc_camera/mx2_camera.c | 4 +--- drivers/media/platform/soc_camera/mx3_camera.c | 4 +--- drivers/media/platform/soc_camera/rcar_vin.c | 4 +--- drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c | 4 ++-- drivers/media/platform/vivi.c| 3 +-- drivers/media/platform/vsp1/vsp1_video.c | 4 +--- drivers/media/usb/em28xx/em28xx-v4l.h| 2 +- drivers/media/usb/em28xx/em28xx-video.c | 8 ++-- drivers/media/usb/pwc/pwc-if.c | 7 ++- drivers/media/usb/s2255/s2255drv.c | 5 ++--- drivers/media/usb/stk1160/stk1160-v4l.c | 4 ++-- drivers/media/usb/usbtv/usbtv-video.c| 9 +++-- drivers/staging/media/davinci_vpfe/vpfe_video.c | 3 +-- drivers/staging/media/dt3155v4l/dt3155v4l.c | 3 +-- drivers/staging/media/go7007/go7007-v4l2.c | 3 +-- drivers/staging/media/msi3101/sdr-msi3101.c | 7 ++- drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c | 3 +-- drivers/staging/media/solo6x10/solo6x10-v4l2.c | 3 +-- include/media/videobuf2-core.h | 2 +- 33 files changed, 49 insertions(+), 95 deletions(-) diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c index bb11443..7559951 100644 --- a/drivers/media/pci/sta2x11/sta2x11_vip.c +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c @@ -357,7 +357,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count) } /* abort streaming and wait for last buffer */ -static int stop_streaming(struct vb2_queue *vq) +static void stop_streaming(struct vb2_queue *vq) { struct sta2x11_vip *vip = vb2_get_drv_priv(vq); struct vip_buffer *vip_buf, *node; @@ -374,7 +374,6 @@ static int stop_streaming(struct vb2_queue *vq) list_del(vip_buf-list); } spin_unlock(vip-lock); - return 0; } static struct vb2_ops vip_video_qops = { diff --git a/drivers/media/platform/blackfin/bfin_capture.c b/drivers/media/platform/blackfin/bfin_capture.c index 200bec9..16f643c 100644 --- a/drivers/media/platform/blackfin/bfin_capture.c +++ b/drivers/media/platform/blackfin/bfin_capture.c @@ -427,15 +427,12 @@ static int bcap_start_streaming(struct vb2_queue *vq, unsigned int count) return 0; } -static int bcap_stop_streaming(struct vb2_queue *vq) +static void bcap_stop_streaming(struct vb2_queue *vq) { struct bcap_device *bcap_dev = vb2_get_drv_priv(vq); struct ppi_if *ppi = bcap_dev-ppi; int ret; - if (!vb2_is_streaming(vq)) - return 0; - bcap_dev-stop = true; wait_for_completion(bcap_dev-comp); ppi-ops-stop(ppi); @@ -452,7 +449,6 @@ static int bcap_stop_streaming(struct vb2_queue *vq) list_del(bcap_dev-cur_frm-list); vb2_buffer_done(bcap_dev-cur_frm-vb, VB2_BUF_STATE_ERROR); } - return 0; } static struct vb2_ops bcap_video_qops = { diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 3e5199e..d9b1a04 100644 ---
[REVIEW PATCH 11/11] vb2: allow read/write as long as the format is single planar
From: Hans Verkuil hans.verk...@cisco.com It was impossible to read() or write() a frame if the queue type was multiplanar. Even if the current format is single planar. Change this to just check whether the number of planes is 1 or more. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 21 + 1 file changed, 21 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 54a4150..8faf1ef 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -2622,6 +2622,7 @@ struct vb2_fileio_buf { */ struct vb2_fileio_data { struct v4l2_requestbuffers req; + struct v4l2_plane p; struct v4l2_buffer b; struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME]; unsigned int cur_index; @@ -2712,13 +2713,21 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) * Read mode requires pre queuing of all buffers. */ if (read) { + bool is_multiplanar = V4L2_TYPE_IS_MULTIPLANAR(q-type); + /* * Queue all buffers. */ for (i = 0; i q-num_buffers; i++) { struct v4l2_buffer *b = fileio-b; + memset(b, 0, sizeof(*b)); b-type = q-type; + if (is_multiplanar) { + memset(fileio-p, 0, sizeof(fileio-p)); + b-m.planes = fileio-p; + b-length = 1; + } b-memory = q-memory; b-index = i; ret = vb2_internal_qbuf(q, b); @@ -2786,6 +2795,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ { struct vb2_fileio_data *fileio; struct vb2_fileio_buf *buf; + bool is_multiplanar = V4L2_TYPE_IS_MULTIPLANAR(q-type); bool set_timestamp = !read (q-timestamp_flags V4L2_BUF_FLAG_TIMESTAMP_MASK) == V4L2_BUF_FLAG_TIMESTAMP_COPY; @@ -2820,6 +2830,11 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ memset(fileio-b, 0, sizeof(fileio-b)); fileio-b.type = q-type; fileio-b.memory = q-memory; + if (is_multiplanar) { + memset(fileio-p, 0, sizeof(fileio-p)); + fileio-b.m.planes = fileio-p; + fileio-b.length = 1; + } ret = vb2_internal_dqbuf(q, fileio-b, nonblock); dprintk(5, file io: vb2_dqbuf result: %d\n, ret); if (ret) @@ -2890,6 +2905,12 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ fileio-b.memory = q-memory; fileio-b.index = index; fileio-b.bytesused = buf-pos; + if (is_multiplanar) { + memset(fileio-p, 0, sizeof(fileio-p)); + fileio-p.bytesused = buf-pos; + fileio-b.m.planes = fileio-p; + fileio-b.length = 1; + } if (set_timestamp) v4l2_get_timestamp(fileio-b.timestamp); ret = vb2_internal_qbuf(q, fileio-b); -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW PATCH 04/11] vb2: use correct prefix
From: Hans Verkuil hans.verk...@cisco.com Many dprintk's in vb2 use a hardcoded prefix with the function name. In many cases that is now outdated. Replace prefixes by the function name using __func__. At least now I know if I see a 'qbuf:' prefix whether that refers to the mmap, userptr or dmabuf variant. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 102 --- 1 file changed, 54 insertions(+), 48 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 83e78e9..71be247 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -371,7 +371,8 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) if (q-bufs[buffer] == NULL) continue; if (q-bufs[buffer]-state == VB2_BUF_STATE_PREPARING) { - dprintk(1, reqbufs: preparing buffers, cannot free\n); + dprintk(1, %s: preparing buffers, cannot free\n, + __func__); return -EAGAIN; } } @@ -656,12 +657,12 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b) int ret; if (b-type != q-type) { - dprintk(1, querybuf: wrong buffer type\n); + dprintk(1, %s: wrong buffer type\n, __func__); return -EINVAL; } if (b-index = q-num_buffers) { - dprintk(1, querybuf: buffer index out of range\n); + dprintk(1, %s: buffer index out of range\n, __func__); return -EINVAL; } vb = q-bufs[b-index]; @@ -721,12 +722,12 @@ static int __verify_memory_type(struct vb2_queue *q, { if (memory != V4L2_MEMORY_MMAP memory != V4L2_MEMORY_USERPTR memory != V4L2_MEMORY_DMABUF) { - dprintk(1, reqbufs: unsupported memory type\n); + dprintk(1, %s: unsupported memory type\n, __func__); return -EINVAL; } if (type != q-type) { - dprintk(1, reqbufs: requested type is incorrect\n); + dprintk(1, %s: requested type is incorrect\n, __func__); return -EINVAL; } @@ -735,17 +736,20 @@ static int __verify_memory_type(struct vb2_queue *q, * are available. */ if (memory == V4L2_MEMORY_MMAP __verify_mmap_ops(q)) { - dprintk(1, reqbufs: MMAP for current setup unsupported\n); + dprintk(1, %s: MMAP for current setup unsupported\n, + __func__); return -EINVAL; } if (memory == V4L2_MEMORY_USERPTR __verify_userptr_ops(q)) { - dprintk(1, reqbufs: USERPTR for current setup unsupported\n); + dprintk(1, %s: USERPTR for current setup unsupported\n, + __func__); return -EINVAL; } if (memory == V4L2_MEMORY_DMABUF __verify_dmabuf_ops(q)) { - dprintk(1, reqbufs: DMABUF for current setup unsupported\n); + dprintk(1, %s: DMABUF for current setup unsupported\n, + __func__); return -EINVAL; } @@ -755,7 +759,7 @@ static int __verify_memory_type(struct vb2_queue *q, * do the memory and type validation. */ if (q-fileio) { - dprintk(1, reqbufs: file io in progress\n); + dprintk(1, %s: file io in progress\n, __func__); return -EBUSY; } return 0; @@ -790,7 +794,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) int ret; if (q-streaming) { - dprintk(1, reqbufs: streaming active\n); + dprintk(1, %s: streaming active\n, __func__); return -EBUSY; } @@ -800,7 +804,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) * are not in use and can be freed. */ if (q-memory == V4L2_MEMORY_MMAP __buffers_in_use(q)) { - dprintk(1, reqbufs: memory in use, cannot free\n); + dprintk(1, %s: memory in use, cannot free\n, __func__); return -EBUSY; } @@ -1272,15 +1276,14 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b) vb-v4l2_planes[plane].length == planes[plane].length) continue; - dprintk(3, qbuf: userspace address for plane %d changed, - reacquiring memory\n, plane); + dprintk(3, %s: userspace address for plane %d changed, reacquiring memory\n, + __func__, plane); /* Check if
[REVIEW PATCH 06/11] vb2: set timestamp when using write()
From: Hans Verkuil hans.verk...@cisco.com When using write() to write data to an output video node the vb2 core should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody else is able to provide this information with the write() operation. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index e38b45e..afd1268 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -22,6 +22,7 @@ #include media/v4l2-dev.h #include media/v4l2-fh.h #include media/v4l2-event.h +#include media/v4l2-common.h #include media/videobuf2-core.h static int debug; @@ -2767,6 +2768,9 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ { struct vb2_fileio_data *fileio; struct vb2_fileio_buf *buf; + bool set_timestamp = !read + (q-timestamp_flags V4L2_BUF_FLAG_TIMESTAMP_MASK) == + V4L2_BUF_FLAG_TIMESTAMP_COPY; int ret, index; dprintk(3, file io: mode %s, offset %ld, count %zd, %sblocking\n, @@ -2868,6 +2872,8 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ fileio-b.memory = q-memory; fileio-b.index = index; fileio-b.bytesused = buf-pos; + if (set_timestamp) + v4l2_get_timestamp(fileio-b.timestamp); ret = vb2_internal_qbuf(q, fileio-b); dprintk(5, file io: vb2_internal_qbuf result: %d\n, ret); if (ret) -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW PATCH 05/11] vb2: move __qbuf_mmap before __qbuf_userptr
From: Hans Verkuil hans.verk...@cisco.com __qbuf_mmap was sort of hidden in between the much larger __qbuf_userptr and __qbuf_dmabuf functions. Move it before __qbuf_userptr which is also conform the usual order these memory models are implemented: first mmap, then userptr, then dmabuf. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 71be247..e38b45e 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -1254,6 +1254,20 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b } /** + * __qbuf_mmap() - handle qbuf of an MMAP buffer + */ +static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b) +{ + int ret; + + __fill_vb2_buffer(vb, b, vb-v4l2_planes); + ret = call_vb_qop(vb, buf_prepare, vb); + if (ret) + fail_vb_qop(vb, buf_prepare); + return ret; +} + +/** * __qbuf_userptr() - handle qbuf of a USERPTR buffer */ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b) @@ -1359,20 +1373,6 @@ err: } /** - * __qbuf_mmap() - handle qbuf of an MMAP buffer - */ -static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b) -{ - int ret; - - __fill_vb2_buffer(vb, b, vb-v4l2_planes); - ret = call_vb_qop(vb, buf_prepare, vb); - if (ret) - fail_vb_qop(vb, buf_prepare); - return ret; -} - -/** * __qbuf_dmabuf() - handle qbuf of a DMABUF buffer */ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b) -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW PATCH 08/11] vb2: simplify a confusing condition.
From: Hans Verkuil hans.verk...@cisco.com q-start_streaming_called is always true, so the WARN_ON check against it being false can be dropped. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 8984187..2ae316b 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -1099,9 +1099,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) if (!q-start_streaming_called) { if (WARN_ON(state != VB2_BUF_STATE_QUEUED)) state = VB2_BUF_STATE_QUEUED; - } else if (!WARN_ON(!q-start_streaming_called)) { - if (WARN_ON(state != VB2_BUF_STATE_DONE - state != VB2_BUF_STATE_ERROR)) + } else if (WARN_ON(state != VB2_BUF_STATE_DONE + state != VB2_BUF_STATE_ERROR)) { state = VB2_BUF_STATE_ERROR; } -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW PATCH 10/11] vb2: set v4l2_buffer.bytesused to 0 for mp buffers
From: Hans Verkuil hans.verk...@cisco.com The bytesused field of struct v4l2_buffer is not used for multiplanar formats, so just zero it to prevent it from having some random value. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index f68a60f..54a4150 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -583,6 +583,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b) * for it. The caller has already verified memory and size. */ b-length = vb-num_planes; + b-bytesused = 0; memcpy(b-m.planes, vb-v4l2_planes, b-length * sizeof(struct v4l2_plane)); } else { -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW PATCH 03/11] vb2: if bytesused is 0, then fill with output buffer length
From: Hans Verkuil hans.verk...@cisco.com The application should really always fill in bytesused for output buffers, unfortunately the vb2 framework never checked for that. So for single planar formats replace a bytesused of 0 by the length of the buffer, and for multiplanar format do the same if bytesused is 0 for ALL planes. This seems to be what the user really intended if v4l2_buffer was just memset to 0. I'm afraid that just checking for this and returning an error would break too many applications. Quite a few drivers never check for bytesused at all and just use the buffer length instead. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 32 +++- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 1a09442..83e78e9 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -1145,19 +1145,35 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b memset(v4l2_planes[plane].reserved, 0, sizeof(v4l2_planes[plane].reserved)); v4l2_planes[plane].data_offset = 0; + v4l2_planes[plane].bytesused = 0; } /* Fill in driver-provided information for OUTPUT types */ if (V4L2_TYPE_IS_OUTPUT(b-type)) { + bool bytesused_is_used; + + /* Check if bytesused == 0 for all planes */ + for (plane = 0; plane vb-num_planes; ++plane) + if (b-m.planes[plane].bytesused) + break; + bytesused_is_used = plane vb-num_planes; + /* * Will have to go up to b-length when API starts * accepting variable number of planes. +* +* If bytesused_is_used is false, then fall back to the +* full buffer size. In that case userspace clearly +* never bothered to set it and it's a safe assumption +* that they really meant to use the full plane sizes. */ for (plane = 0; plane vb-num_planes; ++plane) { - v4l2_planes[plane].bytesused = - b-m.planes[plane].bytesused; - v4l2_planes[plane].data_offset = - b-m.planes[plane].data_offset; + struct v4l2_plane *pdst = v4l2_planes[plane]; + struct v4l2_plane *psrc = b-m.planes[plane]; + + pdst-bytesused = bytesused_is_used ? + psrc-bytesused : psrc-length; + pdst-data_offset = psrc-data_offset; } } @@ -1183,9 +1199,15 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b * so fill in relevant v4l2_buffer struct fields instead. * In videobuf we use our internal V4l2_planes struct for * single-planar buffers as well, for simplicity. +* +* If bytesused == 0, then fall back to the full buffer size +* as that's a sensible default. */ if (V4L2_TYPE_IS_OUTPUT(b-type)) - v4l2_planes[0].bytesused = b-bytesused; + v4l2_planes[0].bytesused = + b-bytesused ? b-bytesused : b-length; + else + v4l2_planes[0].bytesused = 0; /* Single-planar buffers never use data_offset */ v4l2_planes[0].data_offset = 0; -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW PATCH 07/11] vb2: reject output buffers with V4L2_FIELD_ALTERNATE
From: Hans Verkuil hans.verk...@cisco.com This is not allowed by the spec and does in fact not make any sense. Return -EINVAL if this is the case. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index afd1268..8984187 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -1526,6 +1526,15 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b) __func__, ret); return ret; } + if (V4L2_TYPE_IS_OUTPUT(q-type) b-field == V4L2_FIELD_ALTERNATE) { + /* +* If field is ALTERNATE, then we return an error. +* If the format's field is ALTERNATE, then the buffer's field +* should be either TOP or BOTTOM, but using ALTERNATE here as +* well makes no sense. +*/ + return -EINVAL; + } vb-state = VB2_BUF_STATE_PREPARING; vb-v4l2_buf.timestamp.tv_sec = 0; -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW PATCH 02/11] vb2: fix handling of data_offset and v4l2_plane.reserved[]
From: Hans Verkuil hans.verk...@cisco.com The videobuf2-core did not zero the reserved array of v4l2_plane as it should. More serious is the fact that data_offset was not handled correctly: - for capture devices it was never zeroed, which meant that it was uninitialized. Unless the driver sets it it was a completely random number. - __qbuf_dmabuf had a completely incorrect length check that included data_offset. - in the single-planar case data_offset was never correctly set to 0. The single-planar API doesn't support data_offset, so setting it to 0 is the right thing to do. All these issues were found with v4l2-compliance. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index f9059bb..1a09442 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -1141,6 +1141,12 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b unsigned int plane; if (V4L2_TYPE_IS_MULTIPLANAR(b-type)) { + for (plane = 0; plane vb-num_planes; ++plane) { + memset(v4l2_planes[plane].reserved, 0, + sizeof(v4l2_planes[plane].reserved)); + v4l2_planes[plane].data_offset = 0; + } + /* Fill in driver-provided information for OUTPUT types */ if (V4L2_TYPE_IS_OUTPUT(b-type)) { /* @@ -1169,8 +1175,6 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b b-m.planes[plane].m.fd; v4l2_planes[plane].length = b-m.planes[plane].length; - v4l2_planes[plane].data_offset = - b-m.planes[plane].data_offset; } } } else { @@ -1180,10 +1184,10 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b * In videobuf we use our internal V4l2_planes struct for * single-planar buffers as well, for simplicity. */ - if (V4L2_TYPE_IS_OUTPUT(b-type)) { + if (V4L2_TYPE_IS_OUTPUT(b-type)) v4l2_planes[0].bytesused = b-bytesused; - v4l2_planes[0].data_offset = 0; - } + /* Single-planar buffers never use data_offset */ + v4l2_planes[0].data_offset = 0; if (b-memory == V4L2_MEMORY_USERPTR) { v4l2_planes[0].m.userptr = b-m.userptr; @@ -1193,9 +1197,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b if (b-memory == V4L2_MEMORY_DMABUF) { v4l2_planes[0].m.fd = b-m.fd; v4l2_planes[0].length = b-length; - v4l2_planes[0].data_offset = 0; } - } /* Zero flags that the vb2 core handles */ @@ -1374,8 +1376,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b) if (planes[plane].length == 0) planes[plane].length = dbuf-size; - if (planes[plane].length planes[plane].data_offset + - q-plane_sizes[plane]) { + if (planes[plane].length q-plane_sizes[plane]) { dprintk(1, qbuf: invalid dmabuf length for plane %d\n, plane); ret = -EINVAL; -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW PATCH 09/11] vb2: add vb2_fileio_is_active and check it more often
From: Hans Verkuil hans.verk...@cisco.com Added a vb2_fileio_is_active inline function that returns true if fileio is in progress. Check for this too in mmap() (you don't want apps mmap()ing buffers used by fileio) and expbuf() (same reason). In addition drivers should be able to check for this in queue_setup() to return an error if an attempt is made to read() or write() with V4L2_FIELD_ALTERNATE being configured. This is illegal (there is no way to pass the TOP/BOTTOM information around using file I/O). However, in order to be able to check for this the init_fileio function needs to set q-fileio early on, before the buffers are allocated. So switch to using internal functions (__reqbufs, vb2_internal_qbuf and vb2_internal_streamon) to skip the fileio check. Well, that's why the internal functions were created... Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 39 include/media/videobuf2-core.h | 17 ++ 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 2ae316b..f68a60f 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -759,7 +759,7 @@ static int __verify_memory_type(struct vb2_queue *q, * create_bufs is called with count == 0, but count == 0 should still * do the memory and type validation. */ - if (q-fileio) { + if (vb2_fileio_is_active(q)) { dprintk(1, %s: file io in progress\n, __func__); return -EBUSY; } @@ -1628,7 +1628,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b) struct vb2_buffer *vb; int ret; - if (q-fileio) { + if (vb2_fileio_is_active(q)) { dprintk(1, %s(): file io in progress\n, __func__); return -EBUSY; } @@ -1798,7 +1798,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b) */ int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b) { - if (q-fileio) { + if (vb2_fileio_is_active(q)) { dprintk(1, %s(): file io in progress\n, __func__); return -EBUSY; } @@ -2018,7 +2018,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n */ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking) { - if (q-fileio) { + if (vb2_fileio_is_active(q)) { dprintk(1, %s: file io in progress\n, __func__); return -EBUSY; } @@ -2148,7 +2148,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type) */ int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type) { - if (q-fileio) { + if (vb2_fileio_is_active(q)) { dprintk(1, %s: file io in progress\n, __func__); return -EBUSY; } @@ -2195,7 +2195,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type) */ int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type) { - if (q-fileio) { + if (vb2_fileio_is_active(q)) { dprintk(1, %s: file io in progress\n, __func__); return -EBUSY; } @@ -2280,6 +2280,11 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb) return -EINVAL; } + if (vb2_fileio_is_active(q)) { + dprintk(1, expbuf: file io in progress\n); + return -EBUSY; + } + vb_plane = vb-planes[eb-plane]; dbuf = call_memop(vb, get_dmabuf, vb_plane-mem_priv, eb-flags O_ACCMODE); @@ -2356,6 +2361,10 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma) return -EINVAL; } } + if (vb2_fileio_is_active(q)) { + dprintk(1, mmap: file io in progress\n); + return -EBUSY; + } /* * Find the plane corresponding to the offset passed by userspace. @@ -2467,7 +2476,7 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait) /* * Start file I/O emulator only if streaming API has not been used yet. */ - if (q-num_buffers == 0 q-fileio == NULL) { + if (q-num_buffers == 0 !vb2_fileio_is_active(q)) { if (!V4L2_TYPE_IS_OUTPUT(q-type) (q-io_modes VB2_READ) (req_events (POLLIN | POLLRDNORM))) { if (__vb2_init_fileio(q, 1)) @@ -2672,7 +2681,8 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) fileio-req.count = count; fileio-req.memory = V4L2_MEMORY_MMAP; fileio-req.type = q-type; - ret = vb2_reqbufs(q, fileio-req); + q-fileio = fileio; + ret = __reqbufs(q, fileio-req); if (ret)
Re: [PATCH 36/47] adv7604: Make output format configurable through pad format operations
Hi Hans, On Wednesday 12 February 2014 16:01:17 Hans Verkuil wrote: On 02/05/14 17:42, Laurent Pinchart wrote: Replace the dummy video format operations by pad format operations that configure the output format. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/adv7604.c | 243 ++- include/media/adv7604.h | 47 ++--- 2 files changed, 225 insertions(+), 65 deletions(-) snip diff --git a/include/media/adv7604.h b/include/media/adv7604.h index 22811d3..2cc8e16 100644 --- a/include/media/adv7604.h +++ b/include/media/adv7604.h @@ -32,16 +32,6 @@ enum adv7604_ain_sel { ADV7604_AIN9_4_5_6_SYNC_2_1 = 4, }; -/* Bus rotation and reordering (IO register 0x04, [7:5]) */ -enum adv7604_op_ch_sel { - ADV7604_OP_CH_SEL_GBR = 0, - ADV7604_OP_CH_SEL_GRB = 1, - ADV7604_OP_CH_SEL_BGR = 2, - ADV7604_OP_CH_SEL_RGB = 3, - ADV7604_OP_CH_SEL_BRG = 4, - ADV7604_OP_CH_SEL_RBG = 5, -}; - /* Input Color Space (IO register 0x02, [7:4]) */ enum adv7604_inp_color_space { ADV7604_INP_COLOR_SPACE_LIM_RGB = 0, @@ -55,29 +45,11 @@ enum adv7604_inp_color_space { ADV7604_INP_COLOR_SPACE_AUTO = 0xf, }; -/* Select output format (IO register 0x03, [7:0]) */ -enum adv7604_op_format_sel { - ADV7604_OP_FORMAT_SEL_SDR_ITU656_8 = 0x00, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_10 = 0x01, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_12_MODE0 = 0x02, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_12_MODE1 = 0x06, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_12_MODE2 = 0x0a, - ADV7604_OP_FORMAT_SEL_DDR_422_8 = 0x20, - ADV7604_OP_FORMAT_SEL_DDR_422_10 = 0x21, - ADV7604_OP_FORMAT_SEL_DDR_422_12_MODE0 = 0x22, - ADV7604_OP_FORMAT_SEL_DDR_422_12_MODE1 = 0x23, - ADV7604_OP_FORMAT_SEL_DDR_422_12_MODE2 = 0x24, - ADV7604_OP_FORMAT_SEL_SDR_444_24 = 0x40, - ADV7604_OP_FORMAT_SEL_SDR_444_30 = 0x41, - ADV7604_OP_FORMAT_SEL_SDR_444_36_MODE0 = 0x42, - ADV7604_OP_FORMAT_SEL_DDR_444_24 = 0x60, - ADV7604_OP_FORMAT_SEL_DDR_444_30 = 0x61, - ADV7604_OP_FORMAT_SEL_DDR_444_36 = 0x62, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_16 = 0x80, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_20 = 0x81, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_24_MODE0 = 0x82, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_24_MODE1 = 0x86, - ADV7604_OP_FORMAT_SEL_SDR_ITU656_24_MODE2 = 0x8a, +/* Select output format (IO register 0x03, [4:2]) */ +enum adv7604_op_format_mode_sel { + ADV7604_OP_FORMAT_MODE0 = 0x00, + ADV7604_OP_FORMAT_MODE1 = 0x04, + ADV7604_OP_FORMAT_MODE2 = 0x08, }; enum adv7604_drive_strength { @@ -104,11 +76,8 @@ struct adv7604_platform_data { /* Analog input muxing mode */ enum adv7604_ain_sel ain_sel; - /* Bus rotation and reordering */ - enum adv7604_op_ch_sel op_ch_sel; I would keep this as part of the platform_data. This is typically used if things are wired up in a non-standard way and so is specific to the hardware. It is not something that will change from format to format. Right, some level of configuration is needed to account for non-standard wiring. However I'm not sure where that should be handled. With exotic wiring the format at the receiver will be different than the format output by the ADV7604. From a pure ADV7604 point of view, the output format doesn't depend on the wiring. I wonder whether this shouldn't be a link property instead of being a subdev property. There's of course the question of where to store that link property if it's not part of either subdev. Even if we decide that the wiring is a property of the source subdev, I don't think we should duplicate bus reordering code in all subdev drivers. This should thus be handled by the v4l2 core (either directly or as helper functions). Other than this it all looks quite nice! Much more flexible than before. - - /* Select output format */ - enum adv7604_op_format_sel op_format_sel; + /* Select output format mode */ + enum adv7604_op_format_mode_sel op_format_mode_sel; /* Configuration of the INT1 pin */ enum adv7604_int1_config int1_config; @@ -116,14 +85,12 @@ struct adv7604_platform_data { /* IO register 0x02 */ unsigned alt_gamma:1; unsigned op_656_range:1; - unsigned rgb_out:1; unsigned alt_data_sat:1; /* IO register 0x05 */ unsigned blank_data:1; unsigned insert_av_codes:1; unsigned replicate_av_codes:1; - unsigned invert_cbcr:1; /* IO register 0x06 */ unsigned inv_vs_pol:1; -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/48] ADV7611 support
Hello, This patch set implements support for the ADV7611 in the adv7604 driver. It also comes up with new features such as output format configuration through pad format operations, hot-plug detect control through GPIO and DT support. Patches 06/48 to 24/48 and 39/48 replace the subdev video DV timings query cap and enum operations with pad-level equivalents. I've split driver changes in one patch per driver to make review easier, but I can squash them together if desired. I believe I've addressed all comments received on v1, except the one related to op_ch_sel in patch adv7604: Make output format configurable through pad format operations which is still open for discussion. Patches 02/48 to 05/48 have been acked in v1 already, I will send a pull request for them separately if a v3 of this series ends up being needed. I'd like to get patch 01/48 upstream soon as well. Changes compared to v1: - Check the edid and pad fields for various ioctls in the subdev core - Switch to the descriptor-based GPIO API - Leave enum adv7604_pad in header file - Keep the hotplug notifier - Fix compilation breakage when !CONFIG_OF due to directly dereferencing the return value of of_match_node() - Move patch v4l: subdev: Remove deprecated video-level DV timings operations later in the series to avoid bisection breakages - Document struct v4l2_enum_dv_timings reserved field as being set to 0 by both drivers and application - Document pad field of struct v4l2_enum_dv_timings and struct v4l2_dv_timings_cap as being used for subdev nodes only - Typo fixes in documentation Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Tomasz Stanislawski t.stanisl...@samsung.com Cc: Scott Jiang scott.jiang.li...@gmail.com Cc: Prabhakar Lad prabhakar.cse...@gmail.com Lars-Peter Clausen (4): adv7604: Add missing include to linux/types.h adv7604: Add support for asynchronous probing adv7604: Don't put info string arrays on the stack adv7604: Add adv7611 support Laurent Pinchart (44): v4l: of: Support empty port nodes v4l: Add UYVY10_2X10 and VYUY10_2X10 media bus pixel codes v4l: Add UYVY10_1X20 and VYUY10_1X20 media bus pixel codes v4l: Add 12-bit YUV 4:2:0 media bus pixel codes v4l: Add 12-bit YUV 4:2:2 media bus pixel codes v4l: Add pad-level DV timings subdev operations ad9389b: Add pad-level DV timings operations adv7511: Add pad-level DV timings operations adv7842: Add pad-level DV timings operations s5p-tv: hdmi: Add pad-level DV timings operations s5p-tv: hdmiphy: Add pad-level DV timings operations ths8200: Add pad-level DV timings operations tvp7002: Add pad-level DV timings operations media: bfin_capture: Switch to pad-level DV operations media: davinci: vpif: Switch to pad-level DV operations media: staging: davinci: vpfe: Switch to pad-level DV operations s5p-tv: mixer: Switch to pad-level DV operations ad9389b: Remove deprecated video-level DV timings operations adv7511: Remove deprecated video-level DV timings operations adv7842: Remove deprecated video-level DV timings operations s5p-tv: hdmi: Remove deprecated video-level DV timings operations s5p-tv: hdmiphy: Remove deprecated video-level DV timings operation ths8200: Remove deprecated video-level DV timings operations tvp7002: Remove deprecated video-level DV timings operations v4l: Improve readability by not wrapping ioctl number #define's v4l: Add support for DV timings ioctls on subdev nodes v4l: Validate fields in the core code for subdev EDID ioctls adv7604: Add 16-bit read functions for CP and HDMI adv7604: Cache register contents when reading multiple bits adv7604: Remove subdev control handlers adv7604: Add sink pads adv7604: Make output format configurable through pad format operations adv7604: Add pad-level DV timings support adv7604: Remove deprecated video-level DV timings operations v4l: subdev: Remove deprecated video-level DV timings operations adv7604: Inline the to_sd function adv7604: Store I2C addresses and clients in arrays adv7604: Replace *_and_or() functions with *_clr_set() adv7604: Sort headers alphabetically adv7604: Support hot-plug detect control through a GPIO adv7604: Specify the default input through platform data adv7604: Add DT support adv7604: Add LLC polarity configuration adv7604: Add endpoint properties to DT bindings Documentation/DocBook/media/v4l/subdev-formats.xml | 760 +++ .../DocBook/media/v4l/vidioc-dv-timings-cap.xml| 27 +- .../DocBook/media/v4l/vidioc-enum-dv-timings.xml | 30 +- .../devicetree/bindings/media/i2c/adv7604.txt | 69 + drivers/media/i2c/ad9389b.c| 65 +- drivers/media/i2c/adv7511.c| 67 +- drivers/media/i2c/adv7604.c| 1440 ++-- drivers/media/i2c/adv7842.c| 14 +- drivers/media/i2c/ths8200.c| 10 + drivers/media/i2c/tvp7002.c
[PATCH v2 10/48] s5p-tv: hdmi: Add pad-level DV timings operations
The video enum_dv_timings and dv_timings_cap operations are deprecated. Implement the pad-level version of those operations to prepare for the removal of the video version. Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Tomasz Stanislawski t.stanisl...@samsung.com Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/platform/s5p-tv/hdmi_drv.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/media/platform/s5p-tv/hdmi_drv.c b/drivers/media/platform/s5p-tv/hdmi_drv.c index 534722c..3db496c 100644 --- a/drivers/media/platform/s5p-tv/hdmi_drv.c +++ b/drivers/media/platform/s5p-tv/hdmi_drv.c @@ -674,6 +674,8 @@ static int hdmi_g_mbus_fmt(struct v4l2_subdev *sd, static int hdmi_enum_dv_timings(struct v4l2_subdev *sd, struct v4l2_enum_dv_timings *timings) { + if (timings-pad != 0) + return -EINVAL; if (timings-index = ARRAY_SIZE(hdmi_timings)) return -EINVAL; timings-timings = hdmi_timings[timings-index].dv_timings; @@ -687,6 +689,9 @@ static int hdmi_dv_timings_cap(struct v4l2_subdev *sd, { struct hdmi_device *hdev = sd_to_hdmi_dev(sd); + if (cap-pad != 0) + return -EINVAL; + /* Let the phy fill in the pixelclock range */ v4l2_subdev_call(hdev-phy_sd, video, dv_timings_cap, cap); cap-type = V4L2_DV_BT_656_1120; @@ -713,6 +718,11 @@ static const struct v4l2_subdev_video_ops hdmi_sd_video_ops = { .s_stream = hdmi_s_stream, }; +static const struct v4l2_subdev_pad_ops hdmi_sd_pad_ops = { + .enum_dv_timings = hdmi_enum_dv_timings, + .dv_timings_cap = hdmi_dv_timings_cap, +}; + static const struct v4l2_subdev_ops hdmi_sd_ops = { .core = hdmi_sd_core_ops, .video = hdmi_sd_video_ops, -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 23/48] ths8200: Remove deprecated video-level DV timings operations
The video enum_dv_timings and dv_timings_cap operations are deprecated and unused. Remove them. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/i2c/ths8200.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/media/i2c/ths8200.c b/drivers/media/i2c/ths8200.c index c4ec8b2..656d889 100644 --- a/drivers/media/i2c/ths8200.c +++ b/drivers/media/i2c/ths8200.c @@ -432,8 +432,6 @@ static const struct v4l2_subdev_video_ops ths8200_video_ops = { .s_stream = ths8200_s_stream, .s_dv_timings = ths8200_s_dv_timings, .g_dv_timings = ths8200_g_dv_timings, - .enum_dv_timings = ths8200_enum_dv_timings, - .dv_timings_cap = ths8200_dv_timings_cap, }; static const struct v4l2_subdev_pad_ops ths8200_pad_ops = { -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/48] v4l: Add UYVY10_1X20 and VYUY10_1X20 media bus pixel codes
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- Documentation/DocBook/media/v4l/subdev-formats.xml | 104 + include/uapi/linux/v4l2-mediabus.h | 4 +- 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml b/Documentation/DocBook/media/v4l/subdev-formats.xml index 6fb58de..e3cbbb4 100644 --- a/Documentation/DocBook/media/v4l/subdev-formats.xml +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml @@ -2436,6 +2436,110 @@ entryvsubscript1/subscript/entry entryvsubscript0/subscript/entry /row + row id=V4L2-MBUS-FMT-UYVY10-1X20 + entryV4L2_MBUS_FMT_UYVY10_1X20/entry + entry0x201a/entry + entry/entry + dash-ent-12; + entryusubscript9/subscript/entry + entryusubscript8/subscript/entry + entryusubscript7/subscript/entry + entryusubscript6/subscript/entry + entryusubscript5/subscript/entry + entryusubscript4/subscript/entry + entryusubscript3/subscript/entry + entryusubscript2/subscript/entry + entryusubscript1/subscript/entry + entryusubscript0/subscript/entry + entryysubscript9/subscript/entry + entryysubscript8/subscript/entry + entryysubscript7/subscript/entry + entryysubscript6/subscript/entry + entryysubscript5/subscript/entry + entryysubscript4/subscript/entry + entryysubscript3/subscript/entry + entryysubscript2/subscript/entry + entryysubscript1/subscript/entry + entryysubscript0/subscript/entry + /row + row + entry/entry + entry/entry + entry/entry + dash-ent-12; + entryvsubscript9/subscript/entry + entryvsubscript8/subscript/entry + entryvsubscript7/subscript/entry + entryvsubscript6/subscript/entry + entryvsubscript5/subscript/entry + entryvsubscript4/subscript/entry + entryvsubscript3/subscript/entry + entryvsubscript2/subscript/entry + entryvsubscript1/subscript/entry + entryvsubscript0/subscript/entry + entryysubscript9/subscript/entry + entryysubscript8/subscript/entry + entryysubscript7/subscript/entry + entryysubscript6/subscript/entry + entryysubscript5/subscript/entry + entryysubscript4/subscript/entry + entryysubscript3/subscript/entry + entryysubscript2/subscript/entry + entryysubscript1/subscript/entry + entryysubscript0/subscript/entry + /row + row id=V4L2-MBUS-FMT-VYUY10-1X20 + entryV4L2_MBUS_FMT_VYUY10_1X20/entry + entry0x201b/entry + entry/entry + dash-ent-12; + entryvsubscript9/subscript/entry + entryvsubscript8/subscript/entry + entryvsubscript7/subscript/entry + entryvsubscript6/subscript/entry + entryvsubscript5/subscript/entry + entryvsubscript4/subscript/entry + entryvsubscript3/subscript/entry + entryvsubscript2/subscript/entry + entryvsubscript1/subscript/entry + entryvsubscript0/subscript/entry + entryysubscript9/subscript/entry + entryysubscript8/subscript/entry + entryysubscript7/subscript/entry + entryysubscript6/subscript/entry + entryysubscript5/subscript/entry + entryysubscript4/subscript/entry + entryysubscript3/subscript/entry + entryysubscript2/subscript/entry + entryysubscript1/subscript/entry + entryysubscript0/subscript/entry + /row + row + entry/entry + entry/entry + entry/entry + dash-ent-12; + entryusubscript9/subscript/entry + entryusubscript8/subscript/entry + entryusubscript7/subscript/entry + entryusubscript6/subscript/entry + entryusubscript5/subscript/entry + entryusubscript4/subscript/entry + entryusubscript3/subscript/entry + entryusubscript2/subscript/entry + entryusubscript1/subscript/entry + entryusubscript0/subscript/entry + entryysubscript9/subscript/entry + entryysubscript8/subscript/entry + entryysubscript7/subscript/entry + entryysubscript6/subscript/entry + entryysubscript5/subscript/entry + entryysubscript4/subscript/entry + entryysubscript3/subscript/entry +
[PATCH v2 05/48] v4l: Add 12-bit YUV 4:2:2 media bus pixel codes
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- Documentation/DocBook/media/v4l/subdev-formats.xml | 240 + include/uapi/linux/v4l2-mediabus.h | 6 +- 2 files changed, 245 insertions(+), 1 deletion(-) diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml b/Documentation/DocBook/media/v4l/subdev-formats.xml index a0fa7e0..b2d5a03 100644 --- a/Documentation/DocBook/media/v4l/subdev-formats.xml +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml @@ -3006,6 +3006,246 @@ entryusubscript1/subscript/entry entryusubscript0/subscript/entry /row + row id=V4L2-MBUS-FMT-UYVY12-1X24 + entryV4L2_MBUS_FMT_UYVY12_1X24/entry + entry0x2020/entry + entry/entry + dash-ent-8; + entryusubscript11/subscript/entry + entryusubscript10/subscript/entry + entryusubscript9/subscript/entry + entryusubscript8/subscript/entry + entryusubscript7/subscript/entry + entryusubscript6/subscript/entry + entryusubscript5/subscript/entry + entryusubscript4/subscript/entry + entryusubscript3/subscript/entry + entryusubscript2/subscript/entry + entryusubscript1/subscript/entry + entryusubscript0/subscript/entry + entryysubscript11/subscript/entry + entryysubscript10/subscript/entry + entryysubscript9/subscript/entry + entryysubscript8/subscript/entry + entryysubscript7/subscript/entry + entryysubscript6/subscript/entry + entryysubscript5/subscript/entry + entryysubscript4/subscript/entry + entryysubscript3/subscript/entry + entryysubscript2/subscript/entry + entryysubscript1/subscript/entry + entryysubscript0/subscript/entry + /row + row + entry/entry + entry/entry + entry/entry + dash-ent-8; + entryvsubscript11/subscript/entry + entryvsubscript10/subscript/entry + entryvsubscript9/subscript/entry + entryvsubscript8/subscript/entry + entryvsubscript7/subscript/entry + entryvsubscript6/subscript/entry + entryvsubscript5/subscript/entry + entryvsubscript4/subscript/entry + entryvsubscript3/subscript/entry + entryvsubscript2/subscript/entry + entryvsubscript1/subscript/entry + entryvsubscript0/subscript/entry + entryysubscript11/subscript/entry + entryysubscript10/subscript/entry + entryysubscript9/subscript/entry + entryysubscript8/subscript/entry + entryysubscript7/subscript/entry + entryysubscript6/subscript/entry + entryysubscript5/subscript/entry + entryysubscript4/subscript/entry + entryysubscript3/subscript/entry + entryysubscript2/subscript/entry + entryysubscript1/subscript/entry + entryysubscript0/subscript/entry + /row + row id=V4L2-MBUS-FMT-VYUY12-1X24 + entryV4L2_MBUS_FMT_VYUY12_1X24/entry + entry0x2021/entry + entry/entry + dash-ent-8; + entryvsubscript11/subscript/entry + entryvsubscript10/subscript/entry + entryvsubscript9/subscript/entry + entryvsubscript8/subscript/entry + entryvsubscript7/subscript/entry + entryvsubscript6/subscript/entry + entryvsubscript5/subscript/entry + entryvsubscript4/subscript/entry + entryvsubscript3/subscript/entry + entryvsubscript2/subscript/entry + entryvsubscript1/subscript/entry + entryvsubscript0/subscript/entry + entryysubscript11/subscript/entry + entryysubscript10/subscript/entry + entryysubscript9/subscript/entry + entryysubscript8/subscript/entry + entryysubscript7/subscript/entry + entryysubscript6/subscript/entry + entryysubscript5/subscript/entry + entryysubscript4/subscript/entry + entryysubscript3/subscript/entry + entryysubscript2/subscript/entry + entryysubscript1/subscript/entry + entryysubscript0/subscript/entry + /row + row + entry/entry + entry/entry + entry/entry + dash-ent-8; + entryusubscript11/subscript/entry + entryusubscript10/subscript/entry + entryusubscript9/subscript/entry + entryusubscript8/subscript/entry + entryusubscript7/subscript/entry +
[PATCH v2 13/48] tvp7002: Add pad-level DV timings operations
The video enum_dv_timings operation is deprecated. Implement the pad-level version of the operation to prepare for the removal of the video version. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Reviewed-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/i2c/tvp7002.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c index 912e1cc..9f56fd5 100644 --- a/drivers/media/i2c/tvp7002.c +++ b/drivers/media/i2c/tvp7002.c @@ -832,6 +832,9 @@ static int tvp7002_log_status(struct v4l2_subdev *sd) static int tvp7002_enum_dv_timings(struct v4l2_subdev *sd, struct v4l2_enum_dv_timings *timings) { + if (timings-pad != 0) + return -EINVAL; + /* Check requested format index is within range */ if (timings-index = NUM_TIMINGS) return -EINVAL; @@ -937,6 +940,7 @@ static const struct v4l2_subdev_pad_ops tvp7002_pad_ops = { .enum_mbus_code = tvp7002_enum_mbus_code, .get_fmt = tvp7002_get_pad_format, .set_fmt = tvp7002_set_pad_format, + .enum_dv_timings = tvp7002_enum_dv_timings, }; /* V4L2 top level operation handlers */ -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 27/48] v4l: Validate fields in the core code for subdev EDID ioctls
The subdev EDID ioctls receive a pad field that must reference an existing pad and an EDID field that must point to a buffer. Validate both fields in the core code instead of duplicating validation in all drivers. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/ad9389b.c | 2 -- drivers/media/i2c/adv7511.c | 2 -- drivers/media/i2c/adv7604.c | 4 drivers/media/i2c/adv7842.c | 4 drivers/media/v4l2-core/v4l2-subdev.c | 24 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c index 4cdff9e..5b78828 100644 --- a/drivers/media/i2c/ad9389b.c +++ b/drivers/media/i2c/ad9389b.c @@ -683,8 +683,6 @@ static int ad9389b_get_edid(struct v4l2_subdev *sd, return -EINVAL; if (edid-blocks == 0 || edid-blocks 256) return -EINVAL; - if (!edid-edid) - return -EINVAL; if (!state-edid.segments) { v4l2_dbg(1, debug, sd, EDID segment 0 not found\n); return -ENODATA; diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c index de7ddf5..ff1c2cd 100644 --- a/drivers/media/i2c/adv7511.c +++ b/drivers/media/i2c/adv7511.c @@ -784,8 +784,6 @@ static int adv7511_get_edid(struct v4l2_subdev *sd, return -EINVAL; if ((edid-blocks == 0) || (edid-blocks 256)) return -EINVAL; - if (!edid-edid) - return -EINVAL; if (!state-edid.segments) { v4l2_dbg(1, debug, sd, EDID segment 0 not found\n); return -ENODATA; diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index 71c8570..de3db42 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -1673,8 +1673,6 @@ static int adv7604_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi return -EINVAL; if (edid-start_block == 1) edid-blocks = 1; - if (!edid-edid) - return -EINVAL; if (edid-blocks state-edid.blocks) edid-blocks = state-edid.blocks; @@ -1761,8 +1759,6 @@ static int adv7604_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi edid-blocks = 2; return -E2BIG; } - if (!edid-edid) - return -EINVAL; v4l2_dbg(2, debug, sd, %s: write EDID pad %d, edid.present = 0x%x\n, __func__, edid-pad, state-edid.present); diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c index 7fd9325..33558c8 100644 --- a/drivers/media/i2c/adv7842.c +++ b/drivers/media/i2c/adv7842.c @@ -2035,8 +2035,6 @@ static int adv7842_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi return -EINVAL; if (edid-start_block == 1) edid-blocks = 1; - if (!edid-edid) - return -EINVAL; switch (edid-pad) { case ADV7842_EDID_PORT_A: @@ -2071,8 +2069,6 @@ static int adv7842_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *e) return -EINVAL; if (e-blocks 2) return -E2BIG; - if (!e-edid) - return -EINVAL; /* todo, per edid */ state-aspect_ratio = v4l2_calc_aspect_ratio(e-edid[0x15], diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 853fb84..9fff1eb 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -349,11 +349,27 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) sd, pad, set_selection, subdev_fh, sel); } - case VIDIOC_SUBDEV_G_EDID: - return v4l2_subdev_call(sd, pad, get_edid, arg); + case VIDIOC_SUBDEV_G_EDID: { + struct v4l2_subdev_edid *edid = arg; - case VIDIOC_SUBDEV_S_EDID: - return v4l2_subdev_call(sd, pad, set_edid, arg); + if (edid-pad = sd-entity.num_pads) + return -EINVAL; + if (edid-edid == NULL) + return -EINVAL; + + return v4l2_subdev_call(sd, pad, get_edid, edid); + } + + case VIDIOC_SUBDEV_S_EDID: { + struct v4l2_subdev_edid *edid = arg; + + if (edid-pad = sd-entity.num_pads) + return -EINVAL; + if (edid-edid == NULL) + return -EINVAL; + + return v4l2_subdev_call(sd, pad, set_edid, edid); + } case VIDIOC_SUBDEV_DV_TIMINGS_CAP: { struct v4l2_dv_timings_cap *cap = arg; -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at
[PATCH v2 30/48] adv7604: Don't put info string arrays on the stack
From: Lars-Peter Clausen l...@metafoo.de We do not want to modify the info string arrays ever, so no need to waste stack space for them. While we are at it also make them const. Signed-off-by: Lars-Peter Clausen l...@metafoo.de Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/adv7604.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index c3a76ac..81d737c 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -1882,13 +1882,13 @@ static int adv7604_log_status(struct v4l2_subdev *sd) struct stdi_readback stdi; u8 reg_io_0x02 = io_read(sd, 0x02); - char *csc_coeff_sel_rb[16] = { + static const char * const csc_coeff_sel_rb[16] = { bypassed, YPbPr601 - RGB, reserved, YPbPr709 - RGB, reserved, RGB - YPbPr601, reserved, RGB - YPbPr709, reserved, YPbPr709 - YPbPr601, YPbPr601 - YPbPr709, reserved, reserved, reserved, reserved, manual }; - char *input_color_space_txt[16] = { + static const char * const input_color_space_txt[16] = { RGB limited range (16-235), RGB full range (0-255), YCbCr Bt.601 (16-235), YCbCr Bt.709 (16-235), xvYCC Bt.601, xvYCC Bt.709, @@ -1896,12 +1896,12 @@ static int adv7604_log_status(struct v4l2_subdev *sd) invalid, invalid, invalid, invalid, invalid, invalid, invalid, automatic }; - char *rgb_quantization_range_txt[] = { + static const char * const rgb_quantization_range_txt[] = { Automatic, RGB limited range (16-235), RGB full range (0-255), }; - char *deep_color_mode_txt[4] = { + static const char * const deep_color_mode_txt[4] = { 8-bits per channel, 10-bits per channel, 12-bits per channel, -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 35/48] adv7604: Add sink pads
The ADV7604 has sink pads for its HDMI and analog inputs. Report them. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/adv7604.c | 61 ++--- include/media/adv7604.h | 23 - 2 files changed, 46 insertions(+), 38 deletions(-) diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index dc7f6e0..851b350 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -97,13 +97,15 @@ struct adv7604_chip_info { * ** */ + struct adv7604_state { const struct adv7604_chip_info *info; struct adv7604_platform_data pdata; struct v4l2_subdev sd; - struct media_pad pad; + struct media_pad pads[ADV7604_PAD_MAX]; + unsigned int source_pad; struct v4l2_ctrl_handler hdl; - enum adv7604_input_port selected_input; + enum adv7604_pad selected_input; struct v4l2_dv_timings timings; struct { u8 edid[256]; @@ -775,18 +777,18 @@ static inline bool is_analog_input(struct v4l2_subdev *sd) { struct adv7604_state *state = to_state(sd); - return state-selected_input == ADV7604_INPUT_VGA_RGB || - state-selected_input == ADV7604_INPUT_VGA_COMP; + return state-selected_input == ADV7604_PAD_VGA_RGB || + state-selected_input == ADV7604_PAD_VGA_COMP; } static inline bool is_digital_input(struct v4l2_subdev *sd) { struct adv7604_state *state = to_state(sd); - return state-selected_input == ADV7604_INPUT_HDMI_PORT_A || - state-selected_input == ADV7604_INPUT_HDMI_PORT_B || - state-selected_input == ADV7604_INPUT_HDMI_PORT_C || - state-selected_input == ADV7604_INPUT_HDMI_PORT_D; + return state-selected_input == ADV7604_PAD_HDMI_PORT_A || + state-selected_input == ADV7604_PAD_HDMI_PORT_B || + state-selected_input == ADV7604_PAD_HDMI_PORT_C || + state-selected_input == ADV7604_PAD_HDMI_PORT_D; } /* --- */ @@ -1066,14 +1068,14 @@ static void set_rgb_quantization_range(struct v4l2_subdev *sd) switch (state-rgb_quantization_range) { case V4L2_DV_RGB_RANGE_AUTO: - if (state-selected_input == ADV7604_INPUT_VGA_RGB) { + if (state-selected_input == ADV7604_PAD_VGA_RGB) { /* Receiving analog RGB signal * Set RGB full range (0-255) */ io_write_and_or(sd, 0x02, 0x0f, 0x10); break; } - if (state-selected_input == ADV7604_INPUT_VGA_COMP) { + if (state-selected_input == ADV7604_PAD_VGA_COMP) { /* Receiving analog YPbPr signal * Set automode */ io_write_and_or(sd, 0x02, 0x0f, 0xf0); @@ -1106,7 +1108,7 @@ static void set_rgb_quantization_range(struct v4l2_subdev *sd) } break; case V4L2_DV_RGB_RANGE_LIMITED: - if (state-selected_input == ADV7604_INPUT_VGA_COMP) { + if (state-selected_input == ADV7604_PAD_VGA_COMP) { /* YCrCb limited range (16-235) */ io_write_and_or(sd, 0x02, 0x0f, 0x20); break; @@ -1117,7 +1119,7 @@ static void set_rgb_quantization_range(struct v4l2_subdev *sd) break; case V4L2_DV_RGB_RANGE_FULL: - if (state-selected_input == ADV7604_INPUT_VGA_COMP) { + if (state-selected_input == ADV7604_PAD_VGA_COMP) { /* YCrCb full range (0-255) */ io_write_and_or(sd, 0x02, 0x0f, 0x60); break; @@ -1806,7 +1808,7 @@ static int adv7604_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi struct adv7604_state *state = to_state(sd); u8 *data = NULL; - if (edid-pad ADV7604_EDID_PORT_D) + if (edid-pad ADV7604_PAD_HDMI_PORT_D) return -EINVAL; if (edid-blocks == 0) return -EINVAL; @@ -1821,10 +1823,10 @@ static int adv7604_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edi edid-blocks = state-edid.blocks; switch (edid-pad) { - case ADV7604_EDID_PORT_A: - case ADV7604_EDID_PORT_B: - case ADV7604_EDID_PORT_C: - case ADV7604_EDID_PORT_D: + case ADV7604_PAD_HDMI_PORT_A: + case ADV7604_PAD_HDMI_PORT_B: + case ADV7604_PAD_HDMI_PORT_C: + case ADV7604_PAD_HDMI_PORT_D: if (state-edid.present (1 edid-pad)) data = state-edid.edid; break; @@ -1878,7 +1880,7 @@ static int adv7604_set_edid(struct v4l2_subdev *sd,
[PATCH v2 29/48] adv7604: Add support for asynchronous probing
From: Lars-Peter Clausen l...@metafoo.de Signed-off-by: Lars-Peter Clausen l...@metafoo.de Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/i2c/adv7604.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index de3db42..c3a76ac 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -2333,6 +2333,11 @@ static int adv7604_probe(struct i2c_client *client, goto err_entity; v4l2_info(sd, %s found @ 0x%x (%s)\n, client-name, client-addr 1, client-adapter-name); + + err = v4l2_async_register_subdev(sd); + if (err) + goto err_entity; + return 0; err_entity: @@ -2356,6 +2361,7 @@ static int adv7604_remove(struct i2c_client *client) cancel_delayed_work(state-delayed_work_enable_hotplug); destroy_workqueue(state-work_queues); + v4l2_async_unregister_subdev(sd); v4l2_device_unregister_subdev(sd); media_entity_cleanup(sd-entity); adv7604_unregister_clients(to_state(sd)); -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html