[PATCH v2] [media] atmel-isc: add the isc pipeline function
Image Sensor Controller has an internal image processor. It can convert raw format to the other formats, like RGB565, YUV420P. A module parameter 'sensor_preferred' is used to enable or disable the pipeline function. Some v4l2 controls are added to tuning the image when the pipeline function is enabled. Signed-off-by: Songjun Wu --- Changes in v2: - Fix the gama and contrast controls. - Fix some code style issue. drivers/media/platform/atmel/atmel-isc-regs.h | 102 - drivers/media/platform/atmel/atmel-isc.c | 627 +- 2 files changed, 620 insertions(+), 109 deletions(-) diff --git a/drivers/media/platform/atmel/atmel-isc-regs.h b/drivers/media/platform/atmel/atmel-isc-regs.h index 00c449717cde..6936ac467609 100644 --- a/drivers/media/platform/atmel/atmel-isc-regs.h +++ b/drivers/media/platform/atmel/atmel-isc-regs.h @@ -65,6 +65,7 @@ #define ISC_INTSR 0x0034 #define ISC_INT_DDONE BIT(8) +#define ISC_INT_HISDONEBIT(12) /* ISC White Balance Control Register */ #define ISC_WB_CTRL 0x0058 @@ -72,30 +73,98 @@ /* ISC White Balance Configuration Register */ #define ISC_WB_CFG 0x005c +/* ISC White Balance Offset for R, GR Register */ +#define ISC_WB_O_RGR 0x0060 + +/* ISC White Balance Offset for B, GB Register */ +#define ISC_WB_O_BGR 0x0064 + +/* ISC White Balance Gain for R, GR Register */ +#define ISC_WB_G_RGR 0x0068 + +/* ISC White Balance Gain for B, GB Register */ +#define ISC_WB_G_BGR 0x006c + /* ISC Color Filter Array Control Register */ #define ISC_CFA_CTRL0x0070 /* ISC Color Filter Array Configuration Register */ #define ISC_CFA_CFG 0x0074 +#define ISC_CFA_CFG_EITPOL BIT(4) #define ISC_BAY_CFG_GRGR 0x0 #define ISC_BAY_CFG_RGRG 0x1 #define ISC_BAY_CFG_GBGB 0x2 #define ISC_BAY_CFG_BGBG 0x3 -#define ISC_BAY_CFG_MASK GENMASK(1, 0) /* ISC Color Correction Control Register */ #define ISC_CC_CTRL 0x0078 +/* ISC Color Correction RR RG Register */ +#define ISC_CC_RR_RG 0x007c + +/* ISC Color Correction RB OR Register */ +#define ISC_CC_RB_OR 0x0080 + +/* ISC Color Correction GR GG Register */ +#define ISC_CC_GR_GG 0x0084 + +/* ISC Color Correction GB OG Register */ +#define ISC_CC_GB_OG 0x0088 + +/* ISC Color Correction BR BG Register */ +#define ISC_CC_BR_BG 0x008c + +/* ISC Color Correction BB OB Register */ +#define ISC_CC_BB_OB 0x0090 + /* ISC Gamma Correction Control Register */ #define ISC_GAM_CTRL0x0094 +/* ISC_Gamma Correction Blue Entry Register */ +#define ISC_GAM_BENTRY 0x0098 + +/* ISC_Gamma Correction Green Entry Register */ +#define ISC_GAM_GENTRY 0x0198 + +/* ISC_Gamma Correction Green Entry Register */ +#define ISC_GAM_RENTRY 0x0298 + /* Color Space Conversion Control Register */ #define ISC_CSC_CTRL0x0398 +/* Color Space Conversion YR YG Register */ +#define ISC_CSC_YR_YG 0x039c + +/* Color Space Conversion YB OY Register */ +#define ISC_CSC_YB_OY 0x03a0 + +/* Color Space Conversion CBR CBG Register */ +#define ISC_CSC_CBR_CBG0x03a4 + +/* Color Space Conversion CBB OCB Register */ +#define ISC_CSC_CBB_OCB0x03a8 + +/* Color Space Conversion CRR CRG Register */ +#define ISC_CSC_CRR_CRG0x03ac + +/* Color Space Conversion CRB OCR Register */ +#define ISC_CSC_CRB_OCR0x03b0 + /* Contrast And Brightness Control Register */ #define ISC_CBC_CTRL0x03b4 +/* Contrast And Brightness Configuration Register */ +#define ISC_CBC_CFG0x03b8 + +/* Brightness Register */ +#define ISC_CBC_BRIGHT 0x03bc +#define ISC_CBC_BRIGHT_MASKGENMASK(10, 0) + +/* Contrast Register */ +#define ISC_CBC_CONTRAST 0x03c0 +#define ISC_CBC_CONTRAST_MASK GENMASK(11, 0) + /* Subsampling 4:4:4 to 4:2:2 Control Register */ #define ISC_SUB422_CTRL 0x03c4 @@ -120,6 +189,27 @@ #define ISC_RLP_CFG_MODE_YYCC_LIMITED 0xc #define ISC_RLP_CFG_MODE_MASK GENMASK(3, 0) +/* Histogram Control Register */ +#define ISC_HIS_CTRL 0x03d4 + +#define ISC_HIS_CTRL_ENBIT(0) +#define ISC_HIS_CTRL_DIS 0x0 + +/* Histogram Configuration Register */ +#define ISC_HIS_CFG0x03d8 + +#define ISC_HIS_CFG_MODE_GR0x0 +#define ISC_HIS_CFG_MODE_R 0x1 +#define ISC_HIS_CFG_MODE_GB0x2 +#define ISC_HIS_CFG_MODE_B 0x3 +#define ISC_HIS_CFG_MODE_Y 0x4 +#define ISC_HIS_CFG_MODE_RAW 0x5 +#define ISC_HIS_CFG_MODE_YCCIR656 0x6 + +#define ISC_HIS_CFG_BAYSEL_SHIFT 4 + +#define ISC_HIS_CFG_RARBIT(8) + /* DMA Configuration Register */ #define ISC_DCFG0x03e0 #define ISC_DCFG_IMODE_PACKED8 0x0 @@ -159,7 +249,13 @@ /* DMA Address 0 Register */ #define ISC_DAD00x03ec -/* DMA Stride 0 Register */ -#define ISC_DST0
Re: kernel bug 191891
Marc, I'm c/c the ML, as other people may be experiencing the very same issue. Em Sun, 8 Jan 2017 01:22:41 +0100 Marc Duponcheel escreveu: > Hi Mauro, > > Congratulations with your work on the kernel. Thanks! > > You may have noticed bugzilla kernel bug 191891. > > Initially I suspected commit 7724325a19fb0a51d2a69bd2915b33f0ff197f5a > could correlate but I have revert it and still the usb dvb stick does > not work. So I guess in between 4.8.15 and 4.9.0 some other changes > must have caused the regression. > > Please consider having a look and do not hesitate to ask for help to > proceed towards fix. There were a relevant change from Kernel 4.8 to 4.9: now, USB transfers should not use the stack. Changeset 7724325a19fb0 is part of a series meant to fix it. Unfortunately, I don't have all devices that were affected by such patch series. So, we'll need to manually address the issues on the devices that are still broken. What's weird is that, in this specific case, the patch was tested and reported to be ok. Maybe, on the test device, the existing firmware is enough. Anyway, please test the enclosed patch. > > Greetings > Thanks, Mauro [PATCH] dvb-usb: don't use stack for firmware load As reported by Marc Duponcheel , firmware load on dvb-usb is using the stack, with is not allowed anymore on default Kernel configurations: [ 1025.958836] dvb-usb: found a 'WideView WT-220U PenType Receiver (based on ZL353)' in cold state, will try to load a firmware [ 1025.958853] dvb-usb: downloading firmware from file 'dvb-usb-wt220u-zl0353-01.fw' [ 1025.958855] dvb-usb: could not stop the USB controller CPU. [ 1025.958856] dvb-usb: error while transferring firmware (transferred size: -11, block size: 3) [ 1025.958856] dvb-usb: firmware download failed at 8 with -22 [ 1025.958867] usbcore: registered new interface driver dvb_usb_dtt200u [2.789902] dvb-usb: downloading firmware from file 'dvb-usb-wt220u-zl0353-01.fw' [2.789905] [ cut here ] [2.789911] WARNING: CPU: 3 PID: 2196 at drivers/usb/core/hcd.c:1584 usb_hcd_map_urb_for_dma+0x430/0x560 [usbcore] [2.789912] transfer buffer not dma capable [2.789912] Modules linked in: btusb dvb_usb_dtt200u(+) dvb_usb_af9035(+) btrtl btbcm dvb_usb dvb_usb_v2 btintel dvb_core bluetooth rc_core rfkill x86_pkg_temp_thermal intel_powerclamp coretemp crc32_pclmul aesni_intel aes_x86_64 glue_helper lrw gf128mul ablk_helper cryptd drm_kms_helper syscopyarea sysfillrect pcspkr i2c_i801 sysimgblt fb_sys_fops drm i2c_smbus i2c_core r8169 lpc_ich mfd_core mii thermal fan rtc_cmos video button acpi_cpufreq processor snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm snd_timer snd crc32c_intel ahci libahci libata xhci_pci ehci_pci xhci_hcd ehci_hcd usbcore usb_common dm_mirror dm_region_hash dm_log dm_mod [2.789936] CPU: 3 PID: 2196 Comm: systemd-udevd Not tainted 4.9.0-gentoo #1 [2.789937] Hardware name: ASUS All Series/H81I-PLUS, BIOS 0401 07/23/2013 [2.789938] c9000339b690 812bd397 c9000339b6e0 [2.789939] c9000339b6d0 81055c86 06300339b6a0 880116c0c000 [2.789941] 0001 880116c08000 [2.789942] Call Trace: [2.789945] [] dump_stack+0x4d/0x66 [2.789947] [] __warn+0xc6/0xe0 [2.789948] [] warn_slowpath_fmt+0x4a/0x50 [2.789952] [] usb_hcd_map_urb_for_dma+0x430/0x560 [usbcore] [2.789954] [] ? io_schedule_timeout+0xd8/0x110 [2.789956] [] usb_hcd_submit_urb+0x9c/0x980 [usbcore] [2.789958] [] ? copy_page_to_iter+0x14f/0x2b0 [2.789960] [] ? pagecache_get_page+0x28/0x240 [2.789962] [] ? touch_atime+0x20/0xa0 [2.789964] [] usb_submit_urb+0x2c4/0x520 [usbcore] [2.789967] [] usb_start_wait_urb+0x5a/0xe0 [usbcore] [2.789969] [] usb_control_msg+0xbc/0xf0 [usbcore] [2.789970] [] usb_cypress_writemem+0x3d/0x40 [dvb_usb] [2.789972] [] usb_cypress_load_firmware+0x4f/0x130 [dvb_usb] [2.789973] [] ? console_unlock+0x2fe/0x5d0 [2.789974] [] ? vprintk_emit+0x27c/0x410 [2.789975] [] ? vprintk_default+0x1a/0x20 [2.789976] [] ? printk+0x43/0x4b [2.789977] [] dvb_usb_download_firmware+0x60/0xd0 [dvb_usb] [2.789979] [] dvb_usb_device_init+0x3d8/0x610 [dvb_usb] [2.789981] [] dtt200u_usb_probe+0x92/0xd0 [dvb_usb_dtt200u] [2.789984] [] usb_probe_interface+0xfc/0x270 [usbcore] [2.789985] [] driver_probe_device+0x215/0x2d0 [2.789986] [] __driver_attach+0x96/0xa0 [2.789987] [] ? driver_probe_device+0x2d0/0x2d0 [2.789988] [] bus_for_each_dev+0x5b/0x90 [2.789989] [] driver_attach+0x19/0x20 [2.789990] [] bus_add_driver+0x11c/0x220 [2.789991] [] driver_register+0x5b/0xd0 [2.789994] [] usb_register_driver+0x7c/0x130 [usbcore] [2.789994] [] ? 0xa06a5000 [2.789996] [] dtt200u_usb_driver_init+0x1e/0x20 [dvb_usb_d
Re: [RFC v3 00/21] Make use of kref in media device, grab references as needed
Hi Sakari, Just returned this week from vacations. I'm reading my long e-mail backlog, starting from my main inbox... Em Mon, 2 Jan 2017 09:53:49 +0200 Sakari Ailus escreveu: > Hi Mauro, > > On Mon, Dec 19, 2016 at 07:46:55AM -0200, Mauro Carvalho Chehab wrote: > > Em Fri, 16 Dec 2016 17:07:23 +0200 > > Sakari Ailus escreveu: > > > > > Hi Hans, > > > > > > chrdev_open in fs/char_dev.c increases the refcount on open() and > > > > decreases it > > > > on release(). Thus ensuring that the cdev can never be removed while in > > > > an > > > > ioctl. > > > > > > It does, but it does not affect memory which is allocated separately of > > > that. > > > > > > See this: > > > > > > https://www.mail-archive.com/linux-media@vger.kernel.org/msg106390.html> > > > > > > > That sounds promising. If this bug issues other drivers than OMAP3, > > then indeed the core has a bug. > > > > I'll see if I can reproduce it here with some USB drivers later this week. > > It's not a driver problem so yes, it is reproducible on other hardware. Didn't have time to test it before entering into vacations. I guess I won't have any time this week to test those issues on my hardware, as I suspect that my patch queue is full. Also, we're approaching the next merge window. So, unfortunately, I won't have much time those days to do much testing. Btw, Hans commented that you were planning to working on it this month. Do you have some news with regards to the media controller bind/unbind fixes? > > While IMHO it is overkill trying to support hot plug on omap3, I won't > > mind if you do that, provided that your patch series can be applied in > > a way that it won't cause regressions for real hot-pluggable hardware. > > This is not really about the OMAP3 ISP driver hotplug support; it is indeed > about the framework's ability to support hotpluggable hardware. The current > painpoint is removing hardware; the current frameworks aren't quite up to > that at the moment. The point here is that, while it would be fun to allow unbinding OMAP3 V4L2 drivers, OMAP3 doesn't really require hotplug support. On the other hand, on USB drivers, where unbind is a requirement, the current status of the tree is that hotplug works. I did some massive parallel bind/unbind loops here to double check, when we added such fixup patches. Granted, I won't doubt that there are still some rare race conditions that I was unable to reproduce on the time I tested. I also didn't try to hack the Kernel to introduce extra delays to make those race conditions more likely to happen. Anyway, my main concern with this patch is that it breaks hotplug on devices that really need it, while it fix support only for OMAP3 (with doesn't need). Also, it starts with a series of patches that will cause regressions. I won't matter changing the solution to some other approach that would work, provided that the patches are added on an incremented way, and won't introduce regressions to USB drivers. > > On that matter, just like we use vivid as a testbench and as an > > example for other drivers, it would be great if we could merge > > the vimc driver. What's the status of Helen's patchset? > > That's a good point. I wasn't reviewing that driver back then when the > patches were posted, but should it go in, it should make a good example for > writing other drivers as well. > I saw Laurent's comments about Helen's last patch series. From his comments: "I've reviewed the whole patch but haven't had time to test it. I've also skipped the items marked as TODO or FIXME as they're obviously not ready yet :-) Overall this looks good to me, all the issues are minor." Helen promised a new version fixing those minor issues. Perhaps we should merge her next series upstream with such issues addressed and see how it behaves. Thanks, 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: [PATCH v3 00/24] i.MX Media Driver
On Mon, 2017-01-23 at 12:08 +0100, Hans Verkuil wrote: > On 01/23/2017 12:00 PM, Philipp Zabel wrote: > > On Fri, 2017-01-20 at 21:39 +0100, Hans Verkuil wrote: [...] > As long as it is mentioned in the TODO, and ideally in the Kconfig as well, > then I'm fine with it. > > The big advantage of being in the kernel is that it is much easier to start > providing fixes, improvements, etc. If you use a staging driver you know > that there is no guarantee whatsoever with respect to stable ABI/APIs. Of course, but there should be a clear way how to progress on those issues that are documented as blockers, otherwise the driver will linger in staging. Worse, currently we are not even in agreement what to put into the TODO. 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 v3 00/24] i.MX Media Driver
Hi Steve, Hans, [added Laurent to Cc: who I believe might have an opinion on the media bus formats, too. Sorry for the wall of text, I have put a marker where the MEDIA_BUS argument starts] The central issue seems to be that I think media pad links / media bus formats should describe physical links, such as parallel or serial buses, and the formats of pixels flowing through them, whereas Steve would like to extend them to describe software transports and in-memory formats. On Mon, 2017-01-23 at 15:08 -0800, Steve Longerbeam wrote: [...] > >>> And I'm actually in total agreement with that. I definitely agree that > >>> there > >>> should be a mechanism in the media framework that allows passing video > >>> buffers from a source pad to a sink pad using a software queue, with no > >>> involvement from userland. > > That is the other part of the argument. I do not agree that these > > software queue "links" should be presented to userspace as media pad > > links between two entities of a media device. > > First, that would limit the links to subdevices contained in the same > > media graph, while this should work between any two capture and output > > queues of different devices. > > It sounds like we are talking about two different new proposed features. We are talking about the same thing, but we both want a different user interface. Technically, the issue is to trigger the DMA read channel of a mem2mem device automatically whenever another capture device's DMA write channel signals a finished frame. Where we disagree is how to present this to userspace. You represent the capture DMA write channel and mem2mem DMA read channel as pads on media entites and configure the in-kernel software queue between the two using a media pad link. At the same time a different representation of the same DMA write and read channels (the capture vb2_queue of the capture device and the output vb2_queue of the mem2mem device) would be used for operation in the classic, userspace controlled mode via dmabuf passing. I don't want the software-only link in the media graph, but instead use the vb2_queue representation for both cases, and implement the in-kernel queue link on top of the vb2_queue interface. This would allow userspace to have control over buffer allocations and format, and thus avoid unexpected performance implications: it is impossible for userspace to understand which media entity link, when enabled, will cause a significant increase in memory bandwidth usage, or even how much. Also the same mechanism could then be used to link any two devices in a generic manner, instead of special casing the software queue link for two devices that happen to be part of the same media graph. > My proposal is to implement a software buffer queue between pads. > Beyond enabling the link between pads using the existing media controller > API, userspace is not involved after that. The fact that this link is > accomplished with a software buffer queue is not known, and doesn't > need to be known, by userspace. I don't think this is a good thing for the reasons stated above and below: Since the software buffer queue is opaque to userspace, it is completely out of userspace control which format is chosen and how the buffers are allocated. By using media bus formats to configure software links the kernel pretends to userspace that there is a physical connection where there isn't one. Also, the media entity graph would quickly become very unreadable if we were to add all devices to it that could reasonably be linked with software queues. > Your proposal, if I have it right, is to allow linking two v4l2 device > vb2 queues > (i.e. /dev/videoX -> /dev/videoY), using a new user level API, in a free-run > mode such that v4l2 buffers get passed from one device's vb2 queue to the > other without requiring the v4l2 user program to actively forward those > buffers. Yes. > There isn't anything that would preclude one from the other, they can > both exist. But they are different ideas. One implements software queues > at the _pad level_ and is opaque to userspace, the other links queues > at the _device level_ using a new user API, but once the link is > established, also does not require any involvement from userspace. Well, they are different ideas of how the userspace interface _for the same thing_ should look like. > What I'm saying is we can do _both_. What I am saying is we shouldn't do the pad link interface for the software queues. In my opinion it is the wrong abstraction, and apart from the convenience of being able to switch the links on with a single media-ctl invocation, I see too many downsides. > > Assume for example, we want to encode the captured, deinterlaced video > > to h.264 with the coda VPU driver. A software queue link could be > > established between the CSI capture and the VDIC deinterlacer input, > > That's already available in the media graph. By linking CSI and > VDIC entities. The capture device will
Re: [PATCH v3 16/24] media: Add i.MX media core driver
Hi Steve, On Mon, 2017-01-23 at 17:45 -0800, Steve Longerbeam wrote: > > On 01/23/2017 05:38 PM, Steve Longerbeam wrote: > > > >> > >>> Second, ignoring the above locking issue for a moment, > >>> v4l2_pipeline_pm_use() > >>> will call s_power on the sensor _first_, then the mipi csi-2 s_power, > >>> when executing > >>> media-ctl -l '"ov5640 1-003c":0 -> "imx6-mipi-csi2":0[1]'. Which is the > >>> wrong order. > >>> In my version which enforces the correct power on order, the mipi csi-2 > >>> s_power > >>> is called first in that link setup, followed by the sensor. > >> I don't understand why you want to power up subdevs as soon as the links > >> are established. > > > > Because that is the precedence, all other media drivers do pipeline > > power on/off at link_notify. And v4l2_pipeline_link_notify() was written > > as a link_notify method. > > > >> Shouldn't that rather be done for all subdevices in the > >> pipeline when the corresponding capture device is opened? > > > > that won't work. There's no guarantee the links will be established > > at capture device open time. If the device is opened before the links are established, it won't be usable anyway. And I think the connected pipeline should be locked in place while the video device is opened. Is there any reason to ever open the video device and only then start linking entities? > ugh, maybe v4l2_pipeline_pm_use() would work at open/release. If there are > no links yet, it would basically be a no-op. And stream on requires > opening the > device, and the pipeline links should be established by then, so this > might be > fine, looking into this too. Thanks for looking into it, at least I had that working for the TC358743->MIPI-CSI2 link in my driver. > >> It seems to me that powering up the pipeline should be the last step > >> before userspace actually starts the capture. > > > > Well, I'm ok with moving pipeline power on/off to start/stop streaming. > > I would actually prefer to do it then, I only chose at link_notify > > because of precedence. I'll look into it. That might be too late, though. I would expect STREAMON/STREAMOFF to be a rather fast operation as all the slow preparation could be at open / REQBUFS time. Also, there might be sensors that need to be powered on to handle the v4l2_ctrl passthrough? 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 v3 16/24] media: Add i.MX media core driver
On Wed, 2017-01-18 at 17:44 -0800, Steve Longerbeam wrote: > > On 01/14/2017 02:42 PM, Steve Longerbeam wrote: > > > >>> +/* parse inputs property from a sensor node */ > >>> +static void of_parse_sensor_inputs(struct imx_media_dev *imxmd, > >>> +struct imx_media_subdev *sensor, > >>> +struct device_node *sensor_np) > >>> +{ > >>> + struct imx_media_sensor_input *sinput = &sensor->input; > >>> + int ret, i; > >>> + > >>> + for (i = 0; i < IMX_MEDIA_MAX_SENSOR_INPUTS; i++) { > >>> + const char *input_name; > >>> + u32 val; > >>> + > >>> + ret = of_property_read_u32_index(sensor_np, "inputs", i, &val); > >>> + if (ret) > >>> + break; > >>> + > >>> + sinput->value[i] = val; > >>> + > >>> + ret = of_property_read_string_index(sensor_np, "input-names", > >>> + i, &input_name); > >>> + /* > >>> + * if input-names not provided, they will be set using > >>> + * the subdev name once the sensor is known during > >>> + * async bind > >>> + */ > >>> + if (!ret) > >>> + strncpy(sinput->name[i], input_name, > >>> + sizeof(sinput->name[i])); > >>> + } > >>> + > >>> + sinput->num = i; > >>> + > >>> + /* if no inputs provided just assume a single input */ > >>> + if (sinput->num == 0) > >>> + sinput->num = 1; > >>> +} > >> This should be parsed by the sensor driver, not imx-media. > > > > you're probably right. I'll submit a patch for adv7180.c. > > Actually, the problem here is that this parses an input routing value to > pass to s_routing, and an input name string. There would need to be > another subdev callback, maybe enum_imput, that would return this > information for the bridge driver, if this info were to be parsed and > maintained by the sensor. > > But this info should really be known and parsed by the bridge anyway, > because as the header for s_routing states, > > "An i2c device shouldn't know about whether an input pin is connected > to a Composite connector, because on another board or platform it > might be connected to something else entirely. The calling driver is > responsible for mapping a user-level input to the right pins on the i2c > device." I think this description is for non-DT only, as parsing the DT bindings is the obligation of the bound driver, and with that it information it can very well know its connections. 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 v3 13/24] platform: add video-multiplexer subdevice driver
Hi Hans, On Fri, 2017-01-20 at 15:03 +0100, Hans Verkuil wrote: > On 01/07/2017 03:11 AM, Steve Longerbeam wrote: > > From: Philipp Zabel > > > > This driver can handle SoC internal and external video bus multiplexers, > > controlled either by register bit fields or by a GPIO. The subdevice > > passes through frame interval and mbus configuration of the active input > > to the output side. > > > > Signed-off-by: Sascha Hauer > > Signed-off-by: Philipp Zabel > > > > -- > > > > - fixed a cut&paste error in vidsw_remove(): v4l2_async_register_subdev() > > should be unregister. > > > > - added media_entity_cleanup() and v4l2_device_unregister_subdev() > > to vidsw_remove(). > > > > - there was a line left over from a previous iteration that negated > > the new way of determining the pad count just before it which > > has been removed (num_pads = of_get_child_count(np)). > > > > - Philipp Zabel has developed a set of patches that allow adding > > to the subdev async notifier waiting list using a chaining method > > from the async registered callbacks (v4l2_of_subdev_registered() > > and the prep patches for that). For now, I've removed the use of > > v4l2_of_subdev_registered() for the vidmux driver's registered > > callback. This doesn't affect the functionality of this driver, > > but allows for it to be merged now, before adding the chaining > > support. > > > > Signed-off-by: Steve Longerbeam > > --- > > .../bindings/media/video-multiplexer.txt | 59 +++ > > drivers/media/platform/Kconfig | 8 + > > drivers/media/platform/Makefile| 2 + > > drivers/media/platform/video-multiplexer.c | 472 > > + > > 4 files changed, 541 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/media/video-multiplexer.txt > > create mode 100644 drivers/media/platform/video-multiplexer.c > > > > diff --git a/Documentation/devicetree/bindings/media/video-multiplexer.txt > > b/Documentation/devicetree/bindings/media/video-multiplexer.txt > > new file mode 100644 > > index 000..9d133d9 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/video-multiplexer.txt > > @@ -0,0 +1,59 @@ > > +Video Multiplexer > > += > > + > > +Video multiplexers allow to select between multiple input ports. Video > > received > > +on the active input port is passed through to the output port. Muxes > > described > > +by this binding may be controlled by a syscon register bitfield or by a > > GPIO. > > + > > +Required properties: > > +- compatible : should be "video-multiplexer" > > +- reg: should be register base of the register containing the control > > bitfield > > +- bit-mask: bitmask of the control bitfield in the control register > > +- bit-shift: bit offset of the control bitfield in the control register > > +- gpios: alternatively to reg, bit-mask, and bit-shift, a single GPIO > > phandle > > + may be given to switch between two inputs > > +- #address-cells: should be <1> > > +- #size-cells: should be <0> > > +- port@*: at least three port nodes containing endpoints connecting to the > > + source and sink devices according to of_graph bindings. The last port is > > + the output port, all others are inputs. > > + > > +Example: > > + > > +syscon { > > + compatible = "syscon", "simple-mfd"; > > + > > + mux { > > + compatible = "video-multiplexer"; > > + /* Single bit (1 << 19) in syscon register 0x04: */ > > + reg = <0x04>; > > + bit-mask = <1>; > > + bit-shift = <19>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + > > + mux_in0: endpoint { > > + remote-endpoint = <&video_source0_out>; > > + }; > > + }; > > + > > + port@1 { > > + reg = <1>; > > + > > + mux_in1: endpoint { > > + remote-endpoint = <&video_source1_out>; > > + }; > > + }; > > + > > + port@2 { > > + reg = <2>; > > + > > + mux_out: endpoint { > > + remote-endpoint = <&capture_interface_in>; > > + }; > > + }; > > + }; > > +}; > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > > index d944421..65614b5 100644 > > --- a/drivers/media/platform/Kconfig > > +++ b/drivers/media/platform/Kconfig > > @@ -74,6 +74,14 @@ config VIDEO_M32R_AR_M64278 > > To compile this driver as a module, choose M here: the > > module will be called arv. > > > > +config VIDEO_MULTIPLEXER > > + tristate "Video Multiplexer" > > + depends on VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER > > + help > > + This driver provides support for SoC internal N:1 video bus > > +
Re: [PATCH v3 13/24] platform: add video-multiplexer subdevice driver
Hello Steve, On Fri, Jan 6, 2017 at 11:11 PM, Steve Longerbeam wrote: > From: Philipp Zabel [snip] > > +config VIDEO_MULTIPLEXER > + tristate "Video Multiplexer" > + depends on VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER The driver can be build as a module... > + > +static const struct of_device_id vidsw_dt_ids[] = { > + { .compatible = "video-multiplexer", }, > + { /* sentinel */ } > +}; > + ... so you need a MODULE_DEVICE_TABLE(of, vidsw_dt_ids) here or otherwise module autoloading won't work. Best regards, Javier -- 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] pci: drop link_reset
On Wed, Jan 18, 2017 at 07:19:48PM -0500, Doug Ledford wrote: > On Wed, 2017-01-18 at 23:39 +0200, Michael S. Tsirkin wrote: > > No hardware seems to actually call link_reset, and > > no driver implements it as more than a nop stub. > > > > This drops the mentions of the callback from everywhere. > > It's dropped from the documentation as well, but > > the doc really needs to be updated to reflect > > reality better (e.g. on pcie slot reset is the link reset). > > > > This will be done in a later patch. > > > > Signed-off-by: Michael S. Tsirkin > > This is going to conflict with the two patches I have in my for-next > branch related to this same thing (it drops the stubs from qib and > hfi1). It would be easiest if I just added this to my for-next and > fixed up the conflicts prior to submission. > > > --- > > Documentation/PCI/pci-error-recovery.txt | 24 +++--- > > -- > > drivers/infiniband/hw/hfi1/pcie.c| 10 -- > > drivers/infiniband/hw/qib/qib_pcie.c | 8 > > drivers/media/pci/ngene/ngene-cards.c| 7 --- > > include/linux/pci.h | 3 --- > > 5 files changed, 3 insertions(+), 49 deletions(-) > > > > diff --git a/Documentation/PCI/pci-error-recovery.txt > > b/Documentation/PCI/pci-error-recovery.txt > > index ac26869..da3b217 100644 > > --- a/Documentation/PCI/pci-error-recovery.txt > > +++ b/Documentation/PCI/pci-error-recovery.txt > > @@ -78,7 +78,6 @@ struct pci_error_handlers > > { > > int (*error_detected)(struct pci_dev *dev, enum > > pci_channel_state); > > int (*mmio_enabled)(struct pci_dev *dev); > > - int (*link_reset)(struct pci_dev *dev); > > int (*slot_reset)(struct pci_dev *dev); > > void (*resume)(struct pci_dev *dev); > > }; > > @@ -104,8 +103,7 @@ if it implements any, it must implement > > error_detected(). If a callback > > is not implemented, the corresponding feature is considered > > unsupported. > > For example, if mmio_enabled() and resume() aren't there, then it > > is assumed that the driver is not doing any direct recovery and > > requires > > -a slot reset. If link_reset() is not implemented, the card is > > assumed to > > -not care about link resets. Typically a driver will want to know > > about > > +a slot reset. Typically a driver will want to know about > > a slot_reset(). > > > > The actual steps taken by a platform to recover from a PCI error > > @@ -232,25 +230,9 @@ proceeds to STEP 4 (Slot Reset) > > > > STEP 3: Link Reset > > -- > > -The platform resets the link, and then calls the link_reset() > > callback > > -on all affected device drivers. This is a PCI-Express specific > > state > > +The platform resets the link. This is a PCI-Express specific step > > and is done whenever a non-fatal error has been detected that can be > > -"solved" by resetting the link. This call informs the driver of the > > -reset and the driver should check to see if the device appears to be > > -in working condition. > > - > > -The driver is not supposed to restart normal driver I/O operations > > -at this point. It should limit itself to "probing" the device to > > -check its recoverability status. If all is right, then the platform > > -will call resume() once all drivers have ack'd link_reset(). > > - > > - Result codes: > > - (identical to STEP 3 (MMIO Enabled) > > - > > -The platform then proceeds to either STEP 4 (Slot Reset) or STEP 5 > > -(Resume Operations). > > - > > ->>> The current powerpc implementation does not implement this > > callback. > > +"solved" by resetting the link. > > > > STEP 4: Slot Reset > > -- > > diff --git a/drivers/infiniband/hw/hfi1/pcie.c > > b/drivers/infiniband/hw/hfi1/pcie.c > > index 4ac8f33..ebd941f 100644 > > --- a/drivers/infiniband/hw/hfi1/pcie.c > > +++ b/drivers/infiniband/hw/hfi1/pcie.c > > @@ -598,15 +598,6 @@ pci_slot_reset(struct pci_dev *pdev) > > return PCI_ERS_RESULT_CAN_RECOVER; > > } > > > > -static pci_ers_result_t > > -pci_link_reset(struct pci_dev *pdev) > > -{ > > - struct hfi1_devdata *dd = pci_get_drvdata(pdev); > > - > > - dd_dev_info(dd, "HFI1 link_reset function called, > > ignored\n"); > > - return PCI_ERS_RESULT_CAN_RECOVER; > > -} > > - > > static void > > pci_resume(struct pci_dev *pdev) > > { > > @@ -625,7 +616,6 @@ pci_resume(struct pci_dev *pdev) > > const struct pci_error_handlers hfi1_pci_err_handler = { > > .error_detected = pci_error_detected, > > .mmio_enabled = pci_mmio_enabled, > > - .link_reset = pci_link_reset, > > .slot_reset = pci_slot_reset, > > .resume = pci_resume, > > }; > > diff --git a/drivers/infiniband/hw/qib/qib_pcie.c > > b/drivers/infiniband/hw/qib/qib_pcie.c > > index 6abe1c6..c379b83 100644 > > --- a/drivers/infiniband/hw/qib/qib_pcie.c > > +++ b/drivers/infiniband/hw/qib/qib_pcie.c > > @@ -682,13 +682,6 @@ qib_pci_slot_reset(struct pci_dev *pdev) > > return PCI_ERS_RESULT_CAN_RECOVER; > >
[PATCH v2] pci: drop link_reset
No hardware seems to actually call link_reset, and no driver implements it as more than a nop stub. This drops the mentions of the callback from everywhere. It's dropped from the documentation as well, but the doc really needs to be updated to reflect reality better (e.g. on pcie slot reset is the link reset). This will be done in a later patch. Signed-off-by: Michael S. Tsirkin --- changes from v2: - drop from genwqe as well Note: Doug has patches dropping the implementation from infiniband card drivers in his tree already. This is unlikely to cause conflicts though. Documentation/PCI/pci-error-recovery.txt | 24 +++- drivers/infiniband/hw/hfi1/pcie.c| 10 -- drivers/infiniband/hw/qib/qib_pcie.c | 8 drivers/media/pci/ngene/ngene-cards.c| 7 --- drivers/misc/genwqe/card_base.c | 1 - include/linux/pci.h | 3 --- 6 files changed, 3 insertions(+), 50 deletions(-) diff --git a/Documentation/PCI/pci-error-recovery.txt b/Documentation/PCI/pci-error-recovery.txt index ac26869..da3b217 100644 --- a/Documentation/PCI/pci-error-recovery.txt +++ b/Documentation/PCI/pci-error-recovery.txt @@ -78,7 +78,6 @@ struct pci_error_handlers { int (*error_detected)(struct pci_dev *dev, enum pci_channel_state); int (*mmio_enabled)(struct pci_dev *dev); - int (*link_reset)(struct pci_dev *dev); int (*slot_reset)(struct pci_dev *dev); void (*resume)(struct pci_dev *dev); }; @@ -104,8 +103,7 @@ if it implements any, it must implement error_detected(). If a callback is not implemented, the corresponding feature is considered unsupported. For example, if mmio_enabled() and resume() aren't there, then it is assumed that the driver is not doing any direct recovery and requires -a slot reset. If link_reset() is not implemented, the card is assumed to -not care about link resets. Typically a driver will want to know about +a slot reset. Typically a driver will want to know about a slot_reset(). The actual steps taken by a platform to recover from a PCI error @@ -232,25 +230,9 @@ proceeds to STEP 4 (Slot Reset) STEP 3: Link Reset -- -The platform resets the link, and then calls the link_reset() callback -on all affected device drivers. This is a PCI-Express specific state +The platform resets the link. This is a PCI-Express specific step and is done whenever a non-fatal error has been detected that can be -"solved" by resetting the link. This call informs the driver of the -reset and the driver should check to see if the device appears to be -in working condition. - -The driver is not supposed to restart normal driver I/O operations -at this point. It should limit itself to "probing" the device to -check its recoverability status. If all is right, then the platform -will call resume() once all drivers have ack'd link_reset(). - - Result codes: - (identical to STEP 3 (MMIO Enabled) - -The platform then proceeds to either STEP 4 (Slot Reset) or STEP 5 -(Resume Operations). - ->>> The current powerpc implementation does not implement this callback. +"solved" by resetting the link. STEP 4: Slot Reset -- diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c index 4ac8f33..ebd941f 100644 --- a/drivers/infiniband/hw/hfi1/pcie.c +++ b/drivers/infiniband/hw/hfi1/pcie.c @@ -598,15 +598,6 @@ pci_slot_reset(struct pci_dev *pdev) return PCI_ERS_RESULT_CAN_RECOVER; } -static pci_ers_result_t -pci_link_reset(struct pci_dev *pdev) -{ - struct hfi1_devdata *dd = pci_get_drvdata(pdev); - - dd_dev_info(dd, "HFI1 link_reset function called, ignored\n"); - return PCI_ERS_RESULT_CAN_RECOVER; -} - static void pci_resume(struct pci_dev *pdev) { @@ -625,7 +616,6 @@ pci_resume(struct pci_dev *pdev) const struct pci_error_handlers hfi1_pci_err_handler = { .error_detected = pci_error_detected, .mmio_enabled = pci_mmio_enabled, - .link_reset = pci_link_reset, .slot_reset = pci_slot_reset, .resume = pci_resume, }; diff --git a/drivers/infiniband/hw/qib/qib_pcie.c b/drivers/infiniband/hw/qib/qib_pcie.c index 6abe1c6..c379b83 100644 --- a/drivers/infiniband/hw/qib/qib_pcie.c +++ b/drivers/infiniband/hw/qib/qib_pcie.c @@ -682,13 +682,6 @@ qib_pci_slot_reset(struct pci_dev *pdev) return PCI_ERS_RESULT_CAN_RECOVER; } -static pci_ers_result_t -qib_pci_link_reset(struct pci_dev *pdev) -{ - qib_devinfo(pdev, "QIB link_reset function called, ignored\n"); - return PCI_ERS_RESULT_CAN_RECOVER; -} - static void qib_pci_resume(struct pci_dev *pdev) { @@ -707,7 +700,6 @@ qib_pci_resume(struct pci_dev *pdev) const struct pci_error_handlers qib_pci_err_handler = { .error_detected = qib_pci_error_detected, .mmio_enabled = qib_pci_mmio_enabled, - .link_reset = qib_pci_link_reset, .slot_reset = qib_pci_slot_res
Re: [PATCH v2 00/21] Basic i.MX IPUv3 capture support
Hi Steve, On Mon, 2017-01-09 at 20:43 +0100, Philipp Zabel wrote: > On the other hand, there is currently no way to communicate to userspace > that the IC can't downscale bilinearly, but only to 1/2 or 1/4 of the > input resolution, and then scale up bilinearly for there. This is completely wrong. For some reason I that I can't reconstruct anymore, I didn't realize that the PRPENC/VF_RS_R_V/H coefficient fields for the resizing section are 14 bits wide, so the bilinear scaling factor can in fact be reduced down to 8192/16383 ~= 0.50003 before the /2 downsizing section needs to be engaged. > So instead of > pretending to be able to downscale to any resolution, it would be better > to split each IC task into two subdevs, one for the > 1/2-or-1/4-downscaler, and one for the bilinear upscaler. So this point is moot. I still like the unified PRP subdev because of the single input pad, but splitting the scaling over two subdevs is not useful after all. 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
[PATCH] [media] exynos-gsc: Avoid spamming the log on VIDIOC_TRY_FMT
There isn't an ioctl to enum the supported field orders, so a user-space application can call VIDIOC_TRY_FMT using different field orders to know if one is supported. For example, GStreamer does this so during playback dozens of the following messages appear in the kernel log buffer: [ 442.143393] Not supported field order(4) Instead of printing this as an error, just keep it as debug information. Signed-off-by: Javier Martinez Canillas --- drivers/media/platform/exynos-gsc/gsc-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 8524fe15fa80..678b600f0500 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -408,7 +408,7 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f) if (pix_mp->field == V4L2_FIELD_ANY) pix_mp->field = V4L2_FIELD_NONE; else if (pix_mp->field != V4L2_FIELD_NONE) { - pr_err("Not supported field order(%d)\n", pix_mp->field); + pr_debug("Not supported field order(%d)\n", pix_mp->field); return -EINVAL; } -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 13/24] platform: add video-multiplexer subdevice driver
On 01/24/2017 04:02 AM, Philipp Zabel wrote: Hi Hans, On Fri, 2017-01-20 at 15:03 +0100, Hans Verkuil wrote: + +int vidsw_g_mbus_config(struct v4l2_subdev *sd, struct v4l2_mbus_config *cfg) +{ + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd); + struct media_pad *pad; + int ret; + + if (vidsw->active == -1) { + dev_err(sd->dev, "no configuration for inactive mux\n"); + return -EINVAL; + } + + /* +* Retrieve media bus configuration from the entity connected to the +* active input +*/ + pad = media_entity_remote_pad(&vidsw->pads[vidsw->active]); + if (pad) { + sd = media_entity_to_v4l2_subdev(pad->entity); + ret = v4l2_subdev_call(sd, video, g_mbus_config, cfg); + if (ret == -ENOIOCTLCMD) + pad = NULL; + else if (ret < 0) { + dev_err(sd->dev, "failed to get source configuration\n"); + return ret; + } + } + if (!pad) { + /* Mirror the input side on the output side */ + cfg->type = vidsw->endpoint[vidsw->active].bus_type; + if (cfg->type == V4L2_MBUS_PARALLEL || + cfg->type == V4L2_MBUS_BT656) + cfg->flags = vidsw->endpoint[vidsw->active].bus.parallel.flags; + } + + return 0; +} I am not certain this op is needed at all. In the current kernel this op is only used by soc_camera, pxa_camera and omap3isp (somewhat dubious). Normally this information should come from the device tree and there should be no need for this op. My (tentative) long-term plan was to get rid of this op. If you don't need it, then I recommend it is removed. Hi Hans, the imx-media driver was only calling g_mbus_config to the camera sensor, and it was doing that to determine the sensor's bus type. This info was already available from parsing a v4l2_of_endpoint from the sensor node. So it was simple to remove the g_mbus_config calls, and instead rely on the parsed sensor v4l2_of_endpoint. We currently use this to make the CSI capture interface understand whether its source from the MIPI CSI-2 or from the parallel bus. That is probably something that should be fixed, but I'm not quite sure how. The Synopsys DesignWare MIPI CSI-2 reciever turns the incoming MIPI CSI-2 signal into a 32-bit parallel pixel bus plus some signals for the MIPI specific metadata (virtual channel, data type). Then the CSI2IPU gasket turns this input bus into four separate parallel 16-bit pixel buses plus an 8-bit "mct_di" bus for each of them, that carries the MIPI metadata. The incoming data is split into the four outputs according to the MIPI virtual channel. Two of these 16-bit + 8-bit parallel buses are routed through a multiplexer before finally arriving at the CSI on the other side. We need to configure the CSI to either use or ignore the data from the 8-bit mct_di bus depending on whether the source of the mux is configured to the MIPI CSI-2 receiver / CSI2IPU gasket, or to a parallel input. Philipp, from my experience, the CSI_MIPI_DI register (configured by ipu_csi_set_mipi_datatype()) can only be given a virtual channel 0, otherwise no data is received from the MIPI CSI-2 sensor, regardless of the virtual channel the sensor is transmitting over. So it seems the info on the 8-bit mct_di buses generated by the CSI2IPU gasket are ignored by the CSI's, at least the virtual channel number is ignored. For example, if the sensor is transmitting on vc 1, the gasket routes the sensor data to the parallel bus to CSI1. But if CSI_MIPI_DI on CSI1 is written with vc 1, no data is received. Steve Currently we let g_mbus_config pretend that even the internal 32-bit + metadata and 16-bit + 8-bit metadata parallel buses are of type V4L2_MBUS_CSI so that the CSI can ask the mux, which propagates to the CSI-2 receiver, if connected. Without g_mbus_config we'd need to get that information from somewhere else. One possibility would be to extend MEDIA_BUS formats to describe these "parallelized MIPI data" buses separately. 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 v3 19/24] media: imx: Add IC subdev drivers
On 01/20/2017 06:29 AM, Hans Verkuil wrote: On 01/07/2017 03:11 AM, Steve Longerbeam wrote: + +static const struct v4l2_ctrl_config prpenc_std_ctrl[] = { + { + .id = V4L2_CID_HFLIP, + .name = "Horizontal Flip", + .type = V4L2_CTRL_TYPE_BOOLEAN, + .def = 0, + .min = 0, + .max = 1, + .step = 1, + }, { + .id = V4L2_CID_VFLIP, + .name = "Vertical Flip", + .type = V4L2_CTRL_TYPE_BOOLEAN, + .def = 0, + .min = 0, + .max = 1, + .step = 1, + }, { + .id = V4L2_CID_ROTATE, + .name = "Rotation", + .type = V4L2_CTRL_TYPE_INTEGER, + .def = 0, + .min = 0, + .max = 270, + .step = 90, + }, +}; Use v4l2_ctrl_new_std() instead of this array: this avoids duplicating information like the name and type. If this is also done elsewhere, then it should be changed there as well. done. Steve -- 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
cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Jan 25 05:00:24 CET 2017 media-tree git hash:40eca140c404505c09773d1c6685d818cb55ab1a media_build git hash: 3c6ce4ff75f19adf45869e34b376c5b9dee4d50a v4l-utils git hash: 9df320dd3d1a498fcd6cdeef7d783da609b526e0 gcc version:i686-linux-gcc (GCC) 6.2.0 sparse version: v0.5.0-3553-g78b2ea6 smatch version: v0.5.0-3553-g78b2ea6 host hardware: x86_64 host os:4.8.0-164 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.36.4-i686: WARNINGS linux-2.6.37.6-i686: WARNINGS linux-2.6.38.8-i686: WARNINGS linux-2.6.39.4-i686: WARNINGS linux-3.0.60-i686: WARNINGS linux-3.1.10-i686: WARNINGS linux-3.2.37-i686: WARNINGS linux-3.3.8-i686: WARNINGS linux-3.4.27-i686: WARNINGS linux-3.5.7-i686: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.7.4-i686: WARNINGS linux-3.8-i686: WARNINGS linux-3.9.2-i686: WARNINGS linux-3.10.1-i686: WARNINGS linux-3.11.1-i686: OK linux-3.12.67-i686: OK linux-3.13.11-i686: WARNINGS linux-3.14.9-i686: WARNINGS linux-3.15.2-i686: WARNINGS linux-3.16.7-i686: WARNINGS linux-3.17.8-i686: WARNINGS linux-3.18.7-i686: WARNINGS linux-3.19-i686: WARNINGS linux-4.0.9-i686: WARNINGS linux-4.1.33-i686: WARNINGS linux-4.2.8-i686: WARNINGS linux-4.3.6-i686: WARNINGS linux-4.4.22-i686: WARNINGS linux-4.5.7-i686: WARNINGS linux-4.6.7-i686: WARNINGS linux-4.7.5-i686: WARNINGS linux-4.8-i686: OK linux-4.9-i686: OK linux-4.10-rc3-i686: OK linux-2.6.36.4-x86_64: WARNINGS linux-2.6.37.6-x86_64: WARNINGS linux-2.6.38.8-x86_64: WARNINGS linux-2.6.39.4-x86_64: WARNINGS linux-3.0.60-x86_64: WARNINGS linux-3.1.10-x86_64: WARNINGS linux-3.2.37-x86_64: WARNINGS linux-3.3.8-x86_64: WARNINGS linux-3.4.27-x86_64: WARNINGS linux-3.5.7-x86_64: WARNINGS linux-3.6.11-x86_64: WARNINGS linux-3.7.4-x86_64: WARNINGS linux-3.8-x86_64: WARNINGS linux-3.9.2-x86_64: WARNINGS linux-3.10.1-x86_64: WARNINGS linux-3.11.1-x86_64: OK linux-3.12.67-x86_64: OK linux-3.13.11-x86_64: WARNINGS linux-3.14.9-x86_64: WARNINGS linux-3.15.2-x86_64: WARNINGS linux-3.16.7-x86_64: WARNINGS linux-3.17.8-x86_64: WARNINGS linux-3.18.7-x86_64: WARNINGS linux-3.19-x86_64: WARNINGS linux-4.0.9-x86_64: WARNINGS linux-4.1.33-x86_64: WARNINGS linux-4.2.8-x86_64: WARNINGS linux-4.3.6-x86_64: WARNINGS linux-4.4.22-x86_64: WARNINGS linux-4.5.7-x86_64: WARNINGS linux-4.6.7-x86_64: WARNINGS linux-4.7.5-x86_64: WARNINGS linux-4.8-x86_64: OK linux-4.9-x86_64: OK linux-4.10-rc3-x86_64: OK apps: WARNINGS spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html -- 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] v4l2-ctrls.c: add NULL check
Check that the control whose events we want to delete is still there. Normally this will always be the case, but I am not 100% certain if there aren't any corner cases when a device is forcibly unbound. In any case, this will satisfy static checkers and simply make it more robust. Signed-off-by: Hans Verkuil Reported-by: Shaobo --- diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 47001e25fd9e..b9e08e3d6e0e 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -3367,6 +3367,9 @@ static void v4l2_ctrl_del_event(struct v4l2_subscribed_event *sev) { struct v4l2_ctrl *ctrl = v4l2_ctrl_find(sev->fh->ctrl_handler, sev->id); + if (ctrl == NULL) + return; + v4l2_ctrl_lock(ctrl); list_del(&sev->node); v4l2_ctrl_unlock(ctrl); -- 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