Re: [RFT PATCH v3 6/6] uvcvideo: Move decode processing to process context
Hi Guennadi, Thanks for your review and time on this. I certainly appreciate the extra eyes here! On 12/01/18 09:37, Guennadi Liakhovetski wrote: > Hi Kieran, > > On Fri, 12 Jan 2018, Kieran Bingham wrote: > >> Newer high definition cameras, and cameras with multiple lenses such as >> the range of stereo-vision cameras now available have ever increasing >> data rates. >> >> The inclusion of a variable length packet header in URB packets mean >> that we must memcpy the frame data out to our destination 'manually'. >> This can result in data rates of up to 2 gigabits per second being >> processed. >> >> To improve efficiency, and maximise throughput, handle the URB decode >> processing through a work queue to move it from interrupt context, and >> allow multiple processors to work on URBs in parallel. >> >> Signed-off-by: Kieran Bingham>> >> --- >> v2: >> - Lock full critical section of usb_submit_urb() >> >> v3: >> - Fix race on submitting uvc_video_decode_data_work() to work queue. >> - Rename uvc_decode_op -> uvc_copy_op (Generic to encode/decode) >> - Rename decodes -> copy_operations >> - Don't queue work if there is no async task >> - obtain copy op structure directly in uvc_video_decode_data() >> - uvc_video_decode_data_work() -> uvc_video_copy_data_work() >> --- >> drivers/media/usb/uvc/uvc_queue.c | 12 +++- >> drivers/media/usb/uvc/uvc_video.c | 116 +++ >> drivers/media/usb/uvc/uvcvideo.h | 24 ++- >> 3 files changed, 138 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/media/usb/uvc/uvc_queue.c >> b/drivers/media/usb/uvc/uvc_queue.c >> index 5a9987e547d3..598087eeb5c2 100644 >> --- a/drivers/media/usb/uvc/uvc_queue.c >> +++ b/drivers/media/usb/uvc/uvc_queue.c >> @@ -179,10 +179,22 @@ static void uvc_stop_streaming(struct vb2_queue *vq) >> struct uvc_video_queue *queue = vb2_get_drv_priv(vq); >> struct uvc_streaming *stream = uvc_queue_to_stream(queue); >> >> +/* Prevent new buffers coming in. */ >> +spin_lock_irq(>irqlock); >> +queue->flags |= UVC_QUEUE_STOPPING; >> +spin_unlock_irq(>irqlock); Q_A: >> + >> +/* >> + * All pending work should be completed before disabling the stream, as >> + * all URBs will be free'd during uvc_video_enable(s, 0). >> + */ >> +flush_workqueue(stream->async_wq); > > What if we manage to get one last URB here, then... That will be fine. queue->flags = UVC_QUEUE_STOPPING, and thus no more items can be added to the workqueue. >> + >> uvc_video_enable(stream, 0); >> Q_B: >> spin_lock_irq(>irqlock); >> uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR); >> +queue->flags &= ~UVC_QUEUE_STOPPING; >> spin_unlock_irq(>irqlock); >> } >> >> diff --git a/drivers/media/usb/uvc/uvc_video.c >> b/drivers/media/usb/uvc/uvc_video.c >> index 3878bec3276e..fb6b5af17380 100644 >> --- a/drivers/media/usb/uvc/uvc_video.c >> +++ b/drivers/media/usb/uvc/uvc_video.c > > [snip] > >> +/* >> + * When the stream is stopped, all URBs are freed as part of the call to >> + * uvc_stop_streaming() and must not be handled asynchronously. In that >> + * event we can safely complete the packet work directly in this >> + * context, without resubmitting the URB. >> + */ >> +spin_lock_irqsave(>irqlock, flags); >> +if (!(queue->flags & UVC_QUEUE_STOPPING)) { >> +INIT_WORK(_urb->work, uvc_video_copy_data_work); >> +queue_work(stream->async_wq, _urb->work); >> +} else { >> +uvc_video_copy_packets(uvc_urb); > > Can it not happen, that if the stream is currently being stopped, the > queue has been flushed, possibly the previous URB or a couple of them > don't get decoded, but you do decode this one, creating a corrupted frame? > Wouldn't it be better to just drop this URB too? I don't think so. The only time that this uvc_video_copy_packets() can be called directly in this context is if UVC_QUEUE_STOPPING is set, *AND* we have the lock... Therefore we must be executing between points Q_A and Q_B above. The flush_workqueue() will ensure that all queued work is completed. By calling uvc_video_copy_packets() directly we are ensuring that this last packet is also completed. The headers have already been processed at this stage during the call to ->decode() - so all we are actually doing here is the async memcpy work which has already been promised by the header processing, and releasing the references on the vb2 buffers if applicable. Any buffers not fully completed will be returned marked and returned by the call to : uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR); which happens *after* label Q_B: -- Regards Kieran > >> } >> +spin_unlock_irqrestore(>irqlock, flags); >> } >> >> /* > > Thanks > Guennadi >
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Sat Jan 13 05:00:17 CET 2018 media-tree git hash:e3ee691dbf24096ea51b3200946b11d68ce75361 media_build git hash: 46c9dc0a08499791cedfc7ee0df387e475f075a2 v4l-utils git hash: 351eb68ac235f37f749a1c5d6ed2fae80e9dffe3 gcc version:i686-linux-gcc (GCC) 7.1.0 sparse version: v0.5.0-3911-g6f737e1f smatch version: v0.5.0-3911-g6f737e1f host hardware: x86_64 host os:4.13.0-164 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.1.10-i686: ERRORS linux-3.2.37-i686: ERRORS linux-3.3.8-i686: ERRORS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: ERRORS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: WARNINGS linux-3.10.1-i686: WARNINGS linux-3.11.1-i686: ERRORS linux-3.12.67-i686: ERRORS linux-3.13.11-i686: ERRORS linux-3.14.9-i686: ERRORS linux-3.15.2-i686: ERRORS linux-3.16.7-i686: ERRORS linux-3.17.8-i686: ERRORS linux-3.18.7-i686: ERRORS linux-3.19-i686: ERRORS linux-4.0.9-i686: ERRORS linux-4.1.33-i686: ERRORS linux-4.2.8-i686: ERRORS linux-4.3.6-i686: ERRORS linux-4.4.22-i686: ERRORS linux-4.5.7-i686: ERRORS linux-4.6.7-i686: ERRORS linux-4.7.5-i686: ERRORS linux-4.8-i686: ERRORS linux-4.9.26-i686: ERRORS linux-4.10.14-i686: WARNINGS linux-4.11-i686: WARNINGS linux-4.12.1-i686: WARNINGS linux-4.13-i686: WARNINGS linux-4.14-i686: WARNINGS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.37-x86_64: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: WARNINGS linux-3.10.1-x86_64: WARNINGS linux-3.11.1-x86_64: ERRORS linux-3.12.67-x86_64: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.9-x86_64: ERRORS linux-3.15.2-x86_64: ERRORS linux-3.16.7-x86_64: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.7-x86_64: ERRORS linux-3.19-x86_64: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.33-x86_64: ERRORS linux-4.2.8-x86_64: ERRORS linux-4.3.6-x86_64: ERRORS linux-4.4.22-x86_64: ERRORS linux-4.5.7-x86_64: ERRORS linux-4.6.7-x86_64: ERRORS linux-4.7.5-x86_64: ERRORS linux-4.8-x86_64: ERRORS linux-4.9.26-x86_64: ERRORS linux-4.10.14-x86_64: WARNINGS linux-4.11-x86_64: WARNINGS linux-4.12.1-x86_64: WARNINGS linux-4.13-x86_64: WARNINGS linux-4.14-x86_64: WARNINGS apps: OK spec-git: OK smatch: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
[PATCH RESEND] media: video-i2c: add video-i2c driver
There are several thermal sensors that only have a low-speed bus interface but output valid video data. This patchset enables support for the AMG88xx "Grid-Eye" sensor family. Cc: Luca BarbatoCc: Laurent Pinchart Signed-off-by: Matt Ranostay --- drivers/media/i2c/Kconfig | 9 + drivers/media/i2c/Makefile| 1 + drivers/media/i2c/video-i2c.c | 556 ++ 3 files changed, 566 insertions(+) create mode 100644 drivers/media/i2c/video-i2c.c diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 9f18cd296841..549f1e9fc01e 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -908,6 +908,15 @@ config VIDEO_M52790 To compile this driver as a module, choose M here: the module will be called m52790. + +config VIDEO_I2C + tristate "I2C transport video support" + depends on VIDEO_V4L2 && I2C + select VIDEOBUF2_VMALLOC + ---help--- + Enable the I2C transport video support which supports the + following: + * Panasonic AMG88xx Grid-Eye Sensors endmenu menu "Sensors used on soc_camera driver" diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index c0f94cd8d56d..5ca4c98a4bea 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -90,6 +90,7 @@ obj-$(CONFIG_VIDEO_LM3646)+= lm3646.o obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o obj-$(CONFIG_VIDEO_AK881X) += ak881x.o obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o +obj-$(CONFIG_VIDEO_I2C)+= video-i2c.o obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o obj-$(CONFIG_VIDEO_OV2659) += ov2659.o obj-$(CONFIG_VIDEO_TC358743) += tc358743.o diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c new file mode 100644 index ..9df9b5ebd156 --- /dev/null +++ b/drivers/media/i2c/video-i2c.c @@ -0,0 +1,556 @@ +/* + * video-i2c.c - Support for I2C transport video devices + * + * Copyright (C) 2018 Matt Ranostay + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Supported: + * - Panasonic AMG88xx Grid-Eye Sensors + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define VIDEO_I2C_DRIVER "video-i2c" +#define MAX_BUFFER_SIZE128 + +struct video_i2c_chip; + +struct video_i2c_buffer { + struct vb2_v4l2_buffer vb; + struct list_head list; +}; + +struct video_i2c_data { + struct i2c_client *client; + const struct video_i2c_chip *chip; + struct mutex lock; + spinlock_t slock; + struct mutex queue_lock; + + struct v4l2_device v4l2_dev; + struct video_device vdev; + struct vb2_queue vb_vidq; + + struct task_struct *kthread_vid_cap; + struct list_head vid_cap_active; +}; + +static struct v4l2_fmtdesc amg88xx_format = { + .pixelformat = V4L2_PIX_FMT_Y12, +}; + +static struct v4l2_frmsize_discrete amg88xx_size = { + .width = 8, + .height = 8, +}; + +struct video_i2c_chip { + /* video dimensions */ + const struct v4l2_fmtdesc *format; + const struct v4l2_frmsize_discrete *size; + + /* max frames per second */ + unsigned int max_fps; + + /* pixel buffer size */ + unsigned int buffer_size; + + /* pixel size in bits */ + unsigned int bpp; + + /* xfer function */ + int (*xfer)(struct video_i2c_data *data, char *buf); +}; + +static int amg88xx_xfer(struct video_i2c_data *data, char *buf) +{ + struct i2c_client *client = data->client; + struct i2c_msg msg[2]; + u8 reg = 0x80; + int ret; + + msg[0].addr = client->addr; + msg[0].flags = 0; + msg[0].len = 1; + msg[0].buf = (char *) + + msg[1].addr = client->addr; + msg[1].flags = I2C_M_RD; + msg[1].len = data->chip->buffer_size; + msg[1].buf = (char *) buf; + + ret = i2c_transfer(client->adapter, msg, 2); + + return (ret == 2) ? 0 : -EIO; +} + +static const struct video_i2c_chip video_i2c_chip = { + .size = _size, + .format = _format, + .max_fps= 10, + .buffer_size= 128, + .bpp= 16, + .xfer = _xfer, +}; + +static const struct
Re: [PATCH v2 00/19] prevent bounds-check bypass via speculative execution
On Thu, Jan 11, 2018 at 5:19 PM, Linus Torvaldswrote: > Should the array access in entry_SYSCALL_64_fastpath be made to use > the masking approach? That one has a bounds check for an inline constant. cmpq$__NR_syscall_max, %rax so should be safe. The classic Spectre variant #1 code sequence is: int array_size; if (x < array_size) { something with array[x] } which runs into problems because the array_size variable may not be in cache, and while the CPU core is waiting for the value it speculates inside the "if" body. The syscall entry is more like: #define ARRAY_SIZE 10 if (x < ARRAY_SIZE) { something with array[x] } Here there isn't any reason for speculation. The core has the value of 'x' in a register and the upper bound encoded into the "cmp" instruction. Both are right there, no waiting, no speculation. -Tony
media: tc358743: clk_disable_unprepare(refclk) missed
Hello, tc358743_probe_of() enables refclk and disables it on its error paths. But there is no clk_disable_unprepare(refclk) in tc358743_remove() and on error paths in tc358743_probe(). Is it a problem? If we should fix it, is adding struct clk *refclk; to tc358743_state the reasonable way to keep clk easily available? Found by Linux Driver Verification project (linuxtesting.org). -- Alexey Khoroshilov Linux Verification Center, ISPRAS web: http://linuxtesting.org
Re: dvb usb issues since kernel 4.9
On Fri, 2018-01-12 at 19:13 -0200, Mauro Carvalho Chehab wrote: > > > The .config file used to build the Kernel is at: > https://pastebin.com/wpZghann > Hi Mauro Any chance you can try CONFIG_HZ_1000=y, CONFIG_HZ=1000 ? Thanks.
Re: dvb usb issues since kernel 4.9
Em Tue, 9 Jan 2018 09:48:47 -0800 Linus Torvaldsescreveu: > On Tue, Jan 9, 2018 at 9:27 AM, Eric Dumazet wrote: > > > > So yes, commit 4cd13c21b207 ("softirq: Let ksoftirqd do its job") has > > shown up multiple times in various 'regressions' > > simply because it could surface the problem more often. > > But even if you revert it, you can still make the faulty > > driver/subsystem misbehave by adding more stress to the cpu handling > > the IRQ. > > ..but that's always true. People sometimes live on the edge - often by > design (ie hardware has been designed/selected to be the crappiest > possible that still work). > > That doesn't change anything. A patch that takes "bad things can > happen" to "bad things DO happen" is a bad patch. > > > Maybe the answer is to tune the kernel for small latencies at the > > price of small throughput (situation before the patch) > > Generally we always want to tune for latency. Throughput is "easy", > but almost never interesting. > > Sure, people do batch jobs. And yes, people often _benchmark_ > throughput, because it's easy to benchmark. It's much harder to > benchmark latency, even when it's often much more important. > > A prime example is the SSD benchmarks in the last few years - they > improved _dramatically_ when people noticed that the real problem was > latency, not the idiotic maximum big-block bandwidth numbers that have > almost zero impact on most people. > > Put another way: we already have a very strong implicit bias towards > bandwidth just because it's easier to see and measure. > > That means that we generally should strive to have a explicit bias > towards optimizing for latency when that choice comes up. Just to > balance things out (and just to not take the easy way out: bandwidth > can often be improved by adding more layers of buffering and bigger > buffers, and that often ends up really hurting latency). > > > 1) Revert the patch > > Well, we can revert it only partially - limiting it to just networking > for example. > > Just saying "act the way you used to for tasklets" already seems to > have fixed the issue in DVB. > > > 2) get rid of ksoftirqd since it adds unexpected latencies. > > We can't get rid of it entirely, since the synchronous softirq code > can cause problems too. It's why we have that "maximum of ten > synchronous events" in __do_softirq(). > > And we don't *want* to get rid of it. > > We've _always_ had that small-scale "at some point we can't do it > synchronously any more". > > That is a small-scale "don't have horrible latency for _other_ things" > protection. So it's about latency too, it's just about protecting > latency of the rest of the system. > > The problem with commit 4cd13c21b207 is that it turns the small-scale > latency issues in softirq handling (they get larger latencies for lots > of hardware interrupts or even from non-preemptible kernel code) into > the _huge_ scale latency of scheduling, and does so in a racy way too. > > > 3) Let applications that expect to have high throughput make sure to > > pin their threads on cpus that are not processing IRQ. > > (And make sure to not use irqbalance, and setup IRQ cpu affinities) > > The only people that really deal in "thoughput only" tend to be the > HPC people, and they already do things like this. > > (The other end of the spectrum is the realtime people that have > extreme latency requirements, who do things like that for the reverse > reason: keeping one or more CPU's reserved for the particular > low-latency realtime job). Ok, it took me some time - and a faster microSD - in order to be sure that the data loss weren't due to bad storage performance, but I have now some test results. In summary, indeed the ksoftirq commit 4cd13c21b207 ("softirq: Let ksoftirqd do its job") is causing data losses. On my tests, it generate at least one continuity error on every 1-5 minutes. Either reverting it or applying Linus proposal of partially reverting it fixes the issues. Increasing the number of URBs doesn't seem to help. I'm enclosing the dirty details below. Linus/Eric, Now that I have an environment setup, I can test whatever other alternative that would fix the UDP packet flow attack while won't break the softirq handling code. Regards, Mauro --- All tests below were done on a Raspberry Pi3 with a SanDisk Extreme U3 microSD card with 32GB and a DVBSky S960C DVB-S2 tuner with an external power supply, connected to a TCP/IP network via Ethernet (with uses USB on RPi). It also have a serial cable connected to it. It was installed with LibreELEC 8.2.2, using tvheadend backend. I'm recording one MPEG-TS service/"channel" composed of one audio and one video stream, The total traffic collected by tvheadend was about 4 Mbits/s (audio+video+EPG tables). It is part of a 58 mbits/s MPEG Transport stream, with 23 TV service/"channels" on it. While handling this issue, I found
Re: [PATCH 1/7] cx231xx: Add second frontend option
On Fri, Jan 12, 2018 at 11:19 AM, Brad Lovewrote: > Include ability to add a second dvb attach style frontend to cx231xx > USB bridge. All current boards set to use frontend[0]. Changes are > backwards compatible with current behaviour. > > Signed-off-by: Brad Love > --- > drivers/media/usb/cx231xx/cx231xx-dvb.c | 173 > ++-- > 1 file changed, 97 insertions(+), 76 deletions(-) > > diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c > b/drivers/media/usb/cx231xx/cx231xx-dvb.c > index cb4209f..4c6d2f4 100644 > --- a/drivers/media/usb/cx231xx/cx231xx-dvb.c > +++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c > @@ -55,7 +55,7 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); > #define CX231XX_DVB_MAX_PACKETS 64 > > struct cx231xx_dvb { > - struct dvb_frontend *frontend; > + struct dvb_frontend *frontend[2]; Maybe define something like CX231XX_MAX_FRONTEND and use it here rather than using a hardcoded 2. Alex > > /* feed count management */ > struct mutex lock; > @@ -386,17 +386,17 @@ static int attach_xc5000(u8 addr, struct cx231xx *dev) > cfg.i2c_adap = cx231xx_get_i2c_adap(dev, dev->board.tuner_i2c_master); > cfg.i2c_addr = addr; > > - if (!dev->dvb->frontend) { > + if (!dev->dvb->frontend[0]) { > dev_err(dev->dev, "%s/2: dvb frontend not attached. Can't > attach xc5000\n", > dev->name); > return -EINVAL; > } > > - fe = dvb_attach(xc5000_attach, dev->dvb->frontend, ); > + fe = dvb_attach(xc5000_attach, dev->dvb->frontend[0], ); > if (!fe) { > dev_err(dev->dev, "%s/2: xc5000 attach failed\n", dev->name); > - dvb_frontend_detach(dev->dvb->frontend); > - dev->dvb->frontend = NULL; > + dvb_frontend_detach(dev->dvb->frontend[0]); > + dev->dvb->frontend[0] = NULL; > return -EINVAL; > } > > @@ -408,9 +408,9 @@ static int attach_xc5000(u8 addr, struct cx231xx *dev) > > int cx231xx_set_analog_freq(struct cx231xx *dev, u32 freq) > { > - if ((dev->dvb != NULL) && (dev->dvb->frontend != NULL)) { > + if ((dev->dvb != NULL) && (dev->dvb->frontend[0] != NULL)) { > > - struct dvb_tuner_ops *dops = > >dvb->frontend->ops.tuner_ops; > + struct dvb_tuner_ops *dops = > >dvb->frontend[0]->ops.tuner_ops; > > if (dops->set_analog_params != NULL) { > struct analog_parameters params; > @@ -421,7 +421,7 @@ int cx231xx_set_analog_freq(struct cx231xx *dev, u32 freq) > /*params.audmode = ; */ > > /* Set the analog parameters to set the frequency */ > - dops->set_analog_params(dev->dvb->frontend, ); > + dops->set_analog_params(dev->dvb->frontend[0], > ); > } > > } > @@ -433,15 +433,15 @@ int cx231xx_reset_analog_tuner(struct cx231xx *dev) > { > int status = 0; > > - if ((dev->dvb != NULL) && (dev->dvb->frontend != NULL)) { > + if ((dev->dvb != NULL) && (dev->dvb->frontend[0] != NULL)) { > > - struct dvb_tuner_ops *dops = > >dvb->frontend->ops.tuner_ops; > + struct dvb_tuner_ops *dops = > >dvb->frontend[0]->ops.tuner_ops; > > if (dops->init != NULL && !dev->xc_fw_load_done) { > > dev_dbg(dev->dev, > "Reloading firmware for XC5000\n"); > - status = dops->init(dev->dvb->frontend); > + status = dops->init(dev->dvb->frontend[0]); > if (status == 0) { > dev->xc_fw_load_done = 1; > dev_dbg(dev->dev, > @@ -481,17 +481,29 @@ static int register_dvb(struct cx231xx_dvb *dvb, > dvb_register_media_controller(>adapter, dev->media_dev); > > /* Ensure all frontends negotiate bus access */ > - dvb->frontend->ops.ts_bus_ctrl = cx231xx_dvb_bus_ctrl; > + dvb->frontend[0]->ops.ts_bus_ctrl = cx231xx_dvb_bus_ctrl; > + if (dvb->frontend[1]) > + dvb->frontend[1]->ops.ts_bus_ctrl = cx231xx_dvb_bus_ctrl; > > dvb->adapter.priv = dev; > > /* register frontend */ > - result = dvb_register_frontend(>adapter, dvb->frontend); > + result = dvb_register_frontend(>adapter, dvb->frontend[0]); > if (result < 0) { > dev_warn(dev->dev, >"%s: dvb_register_frontend failed (errno = %d)\n", >dev->name, result); > - goto fail_frontend; > + goto fail_frontend0; > + } > + > + if (dvb->frontend[1]) { > + result = dvb_register_frontend(>adapter, > dvb->frontend[1]); > + if (result < 0) { > +
Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
On 01/12/2018 07:12 PM, Hans Verkuil wrote: > On 01/12/2018 06:52 PM, Ville Syrjälä wrote: >> On Fri, Jan 12, 2018 at 06:14:53PM +0100, Hans Verkuil wrote: >>> On 01/12/2018 05:30 PM, Ville Syrjälä wrote: On Fri, Jan 12, 2018 at 05:19:44PM +0100, Hans Verkuil wrote: > Hi Ville, > > For some strange reason your email disappeared from the Cc list. Perhaps > it's the > ä that confuses something somewhere. > > So I'll just forward this directly to you. > > Can you please take a look? This patch series has been in limbo for too > long. IIRC last I looked we still had some ragistration race to deal with. Was that fixed? >>> >>> That was fixed in v5. >>> Also I think we got stuck on leaving the zombie device lingering around when the display is disconnected. I couldn't understand why that is at all useful since you anyway remove the device eventually. >>> >>> It's not a zombie device. If you disconnect and reconnect the display then >>> the >>> application using the CEC device will see the display disappear and reappear >>> as expected. >>> >>> It helps if you think of the normal situation (as is present in most ARM >>> SoCs) >>> where CEC is integral to the HDMI transmitter. I.e. it is not functionality >>> that >>> can be removed. So the cec device is always there and an application opens >>> the >>> device and can use it, regardless of whether a display is connected or not. >>> >>> If a display is detected, the EDID will be read and the CEC physical >>> address is >>> set. The application is informed of that through an event and the CEC >>> adapter >>> can be used. If the HPD disappears the physical address is reset to f.f.f.f >>> and >>> again the application is informed. And in fact it still has to be able to >>> use >>> the CEC adapter even if there is no HPD since some displays turn off the >>> HPD when >>> in standby, but CEC can still be used to power them up again. >> >> Hmm. So you're saying there are DP devices out there that release HPD >> but still respond to DPCD accesses? That sounds... wrong. > > Not quite. To be precise: there are HDMI displays that release HPD when in > standby > but still respond to CEC commands. > > Such displays are still being made today so if you are building a product like > a media streaming box, then this is something to take into account. > > However, for this specific case (CEC tunneling) it is a non-issue since the > DP CEC protocol simply doesn't support sending CEC commands without HPD. > >> In general I don't think we can assume that a device has retained its >> state if it has deasserted HPD, thus we have to reconfigure everything >> again anyway. >> >>> >>> Now consider a future Intel NUC with an HDMI connector on the backplane and >>> working DP CEC-Tunneling-over-AUX support (e.g. the Megachips MCDP2900): the >>> CEC support is always there (it's built in), but only becomes visible to the >>> kernel when you connect a display. You don't want the cec device to >>> disappear >>> whenever you unplug the display, that makes no sense. Applications would >>> loose the CEC configuration and have to close and reopen (when it reappears) >>> the cec device for no good reason since it is built in. >> >> If the application can't remember its settings across a disconnect it >> sounds broken anwyay. This would anyway happen when you disconenct the >> entire dongle. > > Huh? > >> >>> >>> The same situation is valid when using a USB-C to HDMI adapter: >>> disconnecting >>> or reconnecting a display should not lead to the removal of the CEC device. >>> Only when an adapter with different CEC capabilities is detected is there a >>> need to actually unregister the CEC device. >>> >>> All this is really a workaround of the fact that when the HPD disappears the >>> DP-to-HDMI adapter (either external or built-in) also disappears from the >>> topology, even though it is physically still there. >> >> The dongles I've seen do not disappear. The downstream hpd is >> signalled with short hpd pulses + SINK_COUNT instead. >> >> But I've not actually seen a dongle that implements the >> BRANCH_DEVICE_CTRL DPCD register, so not quite sure what those would >> actually do. The spec does say they should default to using long >> hpd for downstream hpd handling. > > I did a few more experiments and it appears that someone somewhere keeps > track of DP branch devices. I.e. after disconnecting my usb-c to hdmi adapter > it still appears in i915_display_info. At least until something else is > connected. I basically need to hook into the DP branch detection. > > Something to look at this weekend. I found a better place to do the CEC (un)registration: a long HPD pulse indicates that the DPCD registers have changed, so that's when I should detect whether there is a new branch device with CEC capabilities. Regards, Hans
Re: iMX6q/coda encoder failures with ffmpeg/gstreamer m2m encoders
Le mardi 19 décembre 2017 à 13:38 +0100, Neil Armstrong a écrit : > > > > The coda driver does not allow S_FMT anymore, as soon as the > > buffers are > > allocated with REQBUFS: > > > > https://bugzilla.gnome.org/show_bug.cgi?id=791338 > > > > regards > > Philipp > > > > Thanks Philipp, > > It solves the gstreamer encoding. Just to let you know that a fix, though slightly different, was merged into master branch. Let us know if you have any further issues. regards, Nicolas
Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
On 01/12/2018 06:52 PM, Ville Syrjälä wrote: > On Fri, Jan 12, 2018 at 06:14:53PM +0100, Hans Verkuil wrote: >> On 01/12/2018 05:30 PM, Ville Syrjälä wrote: >>> On Fri, Jan 12, 2018 at 05:19:44PM +0100, Hans Verkuil wrote: Hi Ville, For some strange reason your email disappeared from the Cc list. Perhaps it's the ä that confuses something somewhere. So I'll just forward this directly to you. Can you please take a look? This patch series has been in limbo for too long. >>> >>> IIRC last I looked we still had some ragistration race to deal with. >>> Was that fixed? >> >> That was fixed in v5. >> >>> >>> Also I think we got stuck on leaving the zombie device lingering around >>> when the display is disconnected. I couldn't understand why that is >>> at all useful since you anyway remove the device eventually. >> >> It's not a zombie device. If you disconnect and reconnect the display then >> the >> application using the CEC device will see the display disappear and reappear >> as expected. >> >> It helps if you think of the normal situation (as is present in most ARM >> SoCs) >> where CEC is integral to the HDMI transmitter. I.e. it is not functionality >> that >> can be removed. So the cec device is always there and an application opens >> the >> device and can use it, regardless of whether a display is connected or not. >> >> If a display is detected, the EDID will be read and the CEC physical address >> is >> set. The application is informed of that through an event and the CEC adapter >> can be used. If the HPD disappears the physical address is reset to f.f.f.f >> and >> again the application is informed. And in fact it still has to be able to use >> the CEC adapter even if there is no HPD since some displays turn off the HPD >> when >> in standby, but CEC can still be used to power them up again. > > Hmm. So you're saying there are DP devices out there that release HPD > but still respond to DPCD accesses? That sounds... wrong. Not quite. To be precise: there are HDMI displays that release HPD when in standby but still respond to CEC commands. Such displays are still being made today so if you are building a product like a media streaming box, then this is something to take into account. However, for this specific case (CEC tunneling) it is a non-issue since the DP CEC protocol simply doesn't support sending CEC commands without HPD. > In general I don't think we can assume that a device has retained its > state if it has deasserted HPD, thus we have to reconfigure everything > again anyway. > >> >> Now consider a future Intel NUC with an HDMI connector on the backplane and >> working DP CEC-Tunneling-over-AUX support (e.g. the Megachips MCDP2900): the >> CEC support is always there (it's built in), but only becomes visible to the >> kernel when you connect a display. You don't want the cec device to disappear >> whenever you unplug the display, that makes no sense. Applications would >> loose the CEC configuration and have to close and reopen (when it reappears) >> the cec device for no good reason since it is built in. > > If the application can't remember its settings across a disconnect it > sounds broken anwyay. This would anyway happen when you disconenct the > entire dongle. Huh? > >> >> The same situation is valid when using a USB-C to HDMI adapter: disconnecting >> or reconnecting a display should not lead to the removal of the CEC device. >> Only when an adapter with different CEC capabilities is detected is there a >> need to actually unregister the CEC device. >> >> All this is really a workaround of the fact that when the HPD disappears the >> DP-to-HDMI adapter (either external or built-in) also disappears from the >> topology, even though it is physically still there. > > The dongles I've seen do not disappear. The downstream hpd is > signalled with short hpd pulses + SINK_COUNT instead. > > But I've not actually seen a dongle that implements the > BRANCH_DEVICE_CTRL DPCD register, so not quite sure what those would > actually do. The spec does say they should default to using long > hpd for downstream hpd handling. I did a few more experiments and it appears that someone somewhere keeps track of DP branch devices. I.e. after disconnecting my usb-c to hdmi adapter it still appears in i915_display_info. At least until something else is connected. I basically need to hook into the DP branch detection. Something to look at this weekend. Regards, Hans > >> If there was a way to >> detect the adapter when there is no display connected, then this workaround >> wouldn't be needed. >> >> This situation is specific to DisplayPort, this is the only case where the >> HDMI connector disappears in a puff of smoke when you disconnect the HDMI >> cable, even though the actual physical connector is obviously still there. >> >> Regards, >> >> Hans >> >>> >>> Adding the lists back to cc so I don't have to repeat
RE: [PATCH 1/2] media: intel-ipu3: cio2: fix a crash with out-of-bounds access
Hi Yong, > -Original Message- > From: Zhi, Yong > Sent: Wednesday, January 03, 2018 6:57 PM > To: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com > Cc: tf...@chromium.org; Mani, Rajmohan; Zhi, > Yong ; Cao, Bingbu > Subject: [PATCH 1/2] media: intel-ipu3: cio2: fix a crash with out-of-bounds > access > > When dmabuf is used for BLOB type frame, the frame buffers allocated by > gralloc will hold more pages than the valid frame data due to height > alignment. > > In this case, the page numbers in sg list could exceed the FBPT upper limit > value - max_lops(8)*1024 to cause crash. > > Limit the LOP access to the valid data length to avoid FBPT sub-entries > overflow. > > Signed-off-by: Yong Zhi > Signed-off-by: Cao Bing Bu > --- > drivers/media/pci/intel/ipu3/ipu3-cio2.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > index 941caa987dab..949f43d206ad 100644 > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > @@ -838,8 +838,9 @@ static int cio2_vb2_buf_init(struct vb2_buffer *vb) > container_of(vb, struct cio2_buffer, vbb.vb2_buf); > static const unsigned int entries_per_page = > CIO2_PAGE_SIZE / sizeof(u32); > - unsigned int pages = DIV_ROUND_UP(vb->planes[0].length, > CIO2_PAGE_SIZE); > - unsigned int lops = DIV_ROUND_UP(pages + 1, entries_per_page); > + unsigned int pages = DIV_ROUND_UP(vb->planes[0].length, > + CIO2_PAGE_SIZE) + 1; > + unsigned int lops = DIV_ROUND_UP(pages, entries_per_page); > struct sg_table *sg; > struct sg_page_iter sg_iter; > int i, j; > @@ -869,6 +870,8 @@ static int cio2_vb2_buf_init(struct vb2_buffer *vb) > > i = j = 0; Nit: separate assignments are preferred over multiple assignments > for_each_sg_page(sg->sgl, _iter, sg->nents, 0) { > + if (!pages--) > + break; > b->lop[i][j] = sg_page_iter_dma_address(_iter) >> > PAGE_SHIFT; > j++; > if (j == entries_per_page) { > -- > 2.7.4
[PATCH v3 1/2] media: dt-bindings: Add OF properties to ov7670
Describe newly introduced OF properties for ov7670 image sensor. The driver supports two standard properties to configure synchronism signals polarities and one custom property already supported as platform data options by the driver to suppress pixel clock during horizontal blanking. Re-phrase child nodes description while at there. Signed-off-by: Jacopo Mondi--- Documentation/devicetree/bindings/media/i2c/ov7670.txt | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/media/i2c/ov7670.txt b/Documentation/devicetree/bindings/media/i2c/ov7670.txt index 826b656..7c89ea5 100644 --- a/Documentation/devicetree/bindings/media/i2c/ov7670.txt +++ b/Documentation/devicetree/bindings/media/i2c/ov7670.txt @@ -9,14 +9,23 @@ Required Properties: - clocks: reference to the xclk input clock. - clock-names: should be "xclk". +Required Endpoint Properties: +- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH respectively. + Default is active high. +- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH respectively. + Default is active high. + Optional Properties: - reset-gpios: reference to the GPIO connected to the resetb pin, if any. Active is low. - powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any. Active is high. +- ov7670,pclk-hb-disable: a boolean property to suppress pixel clock output + signal during horizontal blankings. -The device node must contain one 'port' child node for its digital output -video port, in accordance with the video interface bindings defined in +The device node must contain one 'port' child node with one 'endpoint' child +sub-node for its digital output video port, in accordance with the video +interface bindings defined in: Documentation/devicetree/bindings/media/video-interfaces.txt. Example: @@ -34,8 +43,13 @@ Example: assigned-clocks = <>; assigned-clock-rates = <2500>; + ov7670,pclk-hb-disable; + port { ov7670_0: endpoint { + hsync-active = <0>; + vsync-active = <0>; + remote-endpoint = <_0>; }; }; -- 2.7.4
[PATCH v3 0/2] media: ov7670: Implement mbus configuration
Hello, round 3 for this series. I have now removed 'pll-bypass' property as suggested by Sakari, and restructured bindings description to list hsync and vsync properties as required. Changelog reported below. Thanks j v2->v3: - Drop 'pll-bypass' property - Make 'plck-hb-disable' a boolean optional property - List 'hsync' and 'vsync' polarities as required endpoint properties - Restructured 'ov7670_parse_dt()' function to reflect the above changes v1->v2: - Split bindings description and implementation - Addressed Sakari's comments on v1 - Check for return values of register writes in set_fmt() - TODO: decide if "pll-bypass" should be an OF property. Jacopo Mondi (2): media: dt-bindings: Add OF properties to ov7670 v4l2: i2c: ov7670: Implement OF mbus configuration .../devicetree/bindings/media/i2c/ov7670.txt | 18 +++- drivers/media/i2c/ov7670.c | 102 ++--- 2 files changed, 104 insertions(+), 16 deletions(-) -- 2.7.4
[PATCH v3 2/2] v4l2: i2c: ov7670: Implement OF mbus configuration
ov7670 driver supports two optional properties supplied through platform data, but currently does not support any standard video interface property. Add support through OF parsing for 2 generic properties (vsync and hsync polarities) and for one custom property already supported through platform data to suppress pixel clock output during horizontal blanking. While at there, check return value of register writes in set_fmt function and rationalize spacings. Signal polarities and pixel clock blanking verified through scope and image capture. Signed-off-by: Jacopo Mondi--- drivers/media/i2c/ov7670.c | 102 ++--- 1 file changed, 88 insertions(+), 14 deletions(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 950a0ac..ad5f9e9 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -237,6 +238,7 @@ struct ov7670_info { struct clk *clk; struct gpio_desc *resetb_gpio; struct gpio_desc *pwdn_gpio; + unsigned int mbus_config; /* Media bus configuration flags */ int min_width; /* Filter out smaller sizes */ int min_height; /* Filter out smaller sizes */ int clock_speed;/* External clock speed (MHz) */ @@ -995,7 +997,7 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd, #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API struct v4l2_mbus_framefmt *mbus_fmt; #endif - unsigned char com7; + unsigned char com7, com10 = 0; int ret; if (format->pad) @@ -1015,7 +1017,6 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd, } ret = ov7670_try_fmt_internal(sd, >format, , ); - if (ret) return ret; /* @@ -1026,16 +1027,41 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd, */ com7 = ovfmt->regs[0].value; com7 |= wsize->com7_bit; - ov7670_write(sd, REG_COM7, com7); + ret = ov7670_write(sd, REG_COM7, com7); + if (ret) + return ret; + + /* +* Configure the media bus through COM10 register +*/ + if (info->mbus_config & V4L2_MBUS_VSYNC_ACTIVE_LOW) + com10 |= COM10_VS_NEG; + if (info->mbus_config & V4L2_MBUS_HSYNC_ACTIVE_LOW) + com10 |= COM10_HREF_REV; + if (info->pclk_hb_disable) + com10 |= COM10_PCLK_HB; + ret = ov7670_write(sd, REG_COM10, com10); + if (ret) + return ret; + /* * Now write the rest of the array. Also store start/stops */ - ov7670_write_array(sd, ovfmt->regs + 1); - ov7670_set_hw(sd, wsize->hstart, wsize->hstop, wsize->vstart, - wsize->vstop); - ret = 0; - if (wsize->regs) + ret = ov7670_write_array(sd, ovfmt->regs + 1); + if (ret) + return ret; + + ret = ov7670_set_hw(sd, wsize->hstart, wsize->hstop, wsize->vstart, + wsize->vstop); + if (ret) + return ret; + + if (wsize->regs) { ret = ov7670_write_array(sd, wsize->regs); + if (ret) + return ret; + } + info->fmt = ovfmt; /* @@ -1048,8 +1074,10 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd, * to write it unconditionally, and that will make the frame * rate persistent too. */ - if (ret == 0) - ret = ov7670_write(sd, REG_CLKRC, info->clkrc); + ret = ov7670_write(sd, REG_CLKRC, info->clkrc); + if (ret) + return ret; + return 0; } @@ -1658,6 +1686,49 @@ static int ov7670_init_gpio(struct i2c_client *client, struct ov7670_info *info) return 0; } +/* + * ov7670_parse_dt() - Parse device tree to collect mbus configuration + * properties + */ +static int ov7670_parse_dt(struct device *dev, + struct ov7670_info *info) +{ + struct fwnode_handle *fwnode = dev_fwnode(dev); + struct v4l2_fwnode_endpoint bus_cfg; + struct fwnode_handle *ep; + int ret; + + if (!fwnode) + return -EINVAL; + + info->pclk_hb_disable = false; + if (fwnode_property_present(fwnode, "ov7670,pclk-hb-disable")) + info->pclk_hb_disable = true; + + ep = fwnode_graph_get_next_endpoint(fwnode, NULL); + if (!ep) { + fwnode_handle_put(fwnode); + return -EINVAL; + } + + ret = v4l2_fwnode_endpoint_parse(ep, _cfg); + if (ret) + goto error_put_fwnodes; + + if (bus_cfg.bus_type != V4L2_MBUS_PARALLEL) { + dev_err(dev, "Unsupported media bus type\n"); + ret = -EINVAL; + goto error_put_fwnodes; + } +
Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
On Fri, Jan 12, 2018 at 06:14:53PM +0100, Hans Verkuil wrote: > On 01/12/2018 05:30 PM, Ville Syrjälä wrote: > > On Fri, Jan 12, 2018 at 05:19:44PM +0100, Hans Verkuil wrote: > >> Hi Ville, > >> > >> For some strange reason your email disappeared from the Cc list. Perhaps > >> it's the > >> ä that confuses something somewhere. > >> > >> So I'll just forward this directly to you. > >> > >> Can you please take a look? This patch series has been in limbo for too > >> long. > > > > IIRC last I looked we still had some ragistration race to deal with. > > Was that fixed? > > That was fixed in v5. > > > > > Also I think we got stuck on leaving the zombie device lingering around > > when the display is disconnected. I couldn't understand why that is > > at all useful since you anyway remove the device eventually. > > It's not a zombie device. If you disconnect and reconnect the display then the > application using the CEC device will see the display disappear and reappear > as expected. > > It helps if you think of the normal situation (as is present in most ARM SoCs) > where CEC is integral to the HDMI transmitter. I.e. it is not functionality > that > can be removed. So the cec device is always there and an application opens the > device and can use it, regardless of whether a display is connected or not. > > If a display is detected, the EDID will be read and the CEC physical address > is > set. The application is informed of that through an event and the CEC adapter > can be used. If the HPD disappears the physical address is reset to f.f.f.f > and > again the application is informed. And in fact it still has to be able to use > the CEC adapter even if there is no HPD since some displays turn off the HPD > when > in standby, but CEC can still be used to power them up again. Hmm. So you're saying there are DP devices out there that release HPD but still respond to DPCD accesses? That sounds... wrong. In general I don't think we can assume that a device has retained its state if it has deasserted HPD, thus we have to reconfigure everything again anyway. > > Now consider a future Intel NUC with an HDMI connector on the backplane and > working DP CEC-Tunneling-over-AUX support (e.g. the Megachips MCDP2900): the > CEC support is always there (it's built in), but only becomes visible to the > kernel when you connect a display. You don't want the cec device to disappear > whenever you unplug the display, that makes no sense. Applications would > loose the CEC configuration and have to close and reopen (when it reappears) > the cec device for no good reason since it is built in. If the application can't remember its settings across a disconnect it sounds broken anwyay. This would anyway happen when you disconenct the entire dongle. > > The same situation is valid when using a USB-C to HDMI adapter: disconnecting > or reconnecting a display should not lead to the removal of the CEC device. > Only when an adapter with different CEC capabilities is detected is there a > need to actually unregister the CEC device. > > All this is really a workaround of the fact that when the HPD disappears the > DP-to-HDMI adapter (either external or built-in) also disappears from the > topology, even though it is physically still there. The dongles I've seen do not disappear. The downstream hpd is signalled with short hpd pulses + SINK_COUNT instead. But I've not actually seen a dongle that implements the BRANCH_DEVICE_CTRL DPCD register, so not quite sure what those would actually do. The spec does say they should default to using long hpd for downstream hpd handling. > If there was a way to > detect the adapter when there is no display connected, then this workaround > wouldn't be needed. > > This situation is specific to DisplayPort, this is the only case where the > HDMI connector disappears in a puff of smoke when you disconnect the HDMI > cable, even though the actual physical connector is obviously still there. > > Regards, > > Hans > > > > > Adding the lists back to cc so I don't have to repeat myself there... > > > >> > >> Regards, > >> > >>Hans > >> > >> > >> Forwarded Message > >> Subject: Re: [PATCHv5 0/3] drm/i915: add DisplayPort > >> CEC-Tunneling-over-AUX support > >> Date: Tue, 9 Jan 2018 13:46:44 +0100 > >> From: Hans Verkuil> >> To: linux-media@vger.kernel.org > >> CC: Daniel Vetter , Carlos Santa > >> , dri-de...@lists.freedesktop.org > >> > >> First of all a Happy New Year for all of you! > >> > >> And secondly: can this v5 patch series be reviewed/merged? It's been > >> waiting > >> for that for a very long time now... > >> > >> Regards, > >> > >>Hans > >> > >> On 12/11/17 09:57, Hans Verkuil wrote: > >>> Ping again. Added a CC to Ville whom I inexplicably forgot to add when > >>> I sent the v5 patch series. > >>> > >>> Regards, > >>> > >>> Hans > >>> > >>> On 01/12/17 08:23, Hans
Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
On 01/12/2018 05:30 PM, Ville Syrjälä wrote: > On Fri, Jan 12, 2018 at 05:19:44PM +0100, Hans Verkuil wrote: >> Hi Ville, >> >> For some strange reason your email disappeared from the Cc list. Perhaps >> it's the >> ä that confuses something somewhere. >> >> So I'll just forward this directly to you. >> >> Can you please take a look? This patch series has been in limbo for too long. > > IIRC last I looked we still had some ragistration race to deal with. > Was that fixed? That was fixed in v5. > > Also I think we got stuck on leaving the zombie device lingering around > when the display is disconnected. I couldn't understand why that is > at all useful since you anyway remove the device eventually. It's not a zombie device. If you disconnect and reconnect the display then the application using the CEC device will see the display disappear and reappear as expected. It helps if you think of the normal situation (as is present in most ARM SoCs) where CEC is integral to the HDMI transmitter. I.e. it is not functionality that can be removed. So the cec device is always there and an application opens the device and can use it, regardless of whether a display is connected or not. If a display is detected, the EDID will be read and the CEC physical address is set. The application is informed of that through an event and the CEC adapter can be used. If the HPD disappears the physical address is reset to f.f.f.f and again the application is informed. And in fact it still has to be able to use the CEC adapter even if there is no HPD since some displays turn off the HPD when in standby, but CEC can still be used to power them up again. Now consider a future Intel NUC with an HDMI connector on the backplane and working DP CEC-Tunneling-over-AUX support (e.g. the Megachips MCDP2900): the CEC support is always there (it's built in), but only becomes visible to the kernel when you connect a display. You don't want the cec device to disappear whenever you unplug the display, that makes no sense. Applications would loose the CEC configuration and have to close and reopen (when it reappears) the cec device for no good reason since it is built in. The same situation is valid when using a USB-C to HDMI adapter: disconnecting or reconnecting a display should not lead to the removal of the CEC device. Only when an adapter with different CEC capabilities is detected is there a need to actually unregister the CEC device. All this is really a workaround of the fact that when the HPD disappears the DP-to-HDMI adapter (either external or built-in) also disappears from the topology, even though it is physically still there. If there was a way to detect the adapter when there is no display connected, then this workaround wouldn't be needed. This situation is specific to DisplayPort, this is the only case where the HDMI connector disappears in a puff of smoke when you disconnect the HDMI cable, even though the actual physical connector is obviously still there. Regards, Hans > > Adding the lists back to cc so I don't have to repeat myself there... > >> >> Regards, >> >> Hans >> >> >> Forwarded Message >> Subject: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX >> support >> Date: Tue, 9 Jan 2018 13:46:44 +0100 >> From: Hans Verkuil>> To: linux-media@vger.kernel.org >> CC: Daniel Vetter , Carlos Santa >> , dri-de...@lists.freedesktop.org >> >> First of all a Happy New Year for all of you! >> >> And secondly: can this v5 patch series be reviewed/merged? It's been waiting >> for that for a very long time now... >> >> Regards, >> >> Hans >> >> On 12/11/17 09:57, Hans Verkuil wrote: >>> Ping again. Added a CC to Ville whom I inexplicably forgot to add when >>> I sent the v5 patch series. >>> >>> Regards, >>> >>> Hans >>> >>> On 01/12/17 08:23, Hans Verkuil wrote: Ping! I really like to get this in for 4.16 so I can move forward with hooking this up for nouveau/amd. Regards, Hans On 11/20/2017 12:42 PM, Hans Verkuil wrote: > This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX > feature. This patch series is based on the 4.14 mainline release but > applies > as well to drm-next. > > This patch series has been tested with my NUC7i5BNK, a Samsung USB-C to > HDMI adapter and a Club 3D DisplayPort MST Hub + modified UpTab DP-to-HDMI > adapter (where the CEC pin is wired up). > > Please note this comment at the start of drm_dp_cec.c: > > -- > Unfortunately it turns out that we have a chicken-and-egg situation > here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI adapters > have a converter chip that supports CEC-Tunneling-over-AUX (usually the > Parade PS176 or MegaChips MCDP2900), but
Re: [PATCH 1/2] MAINTAINERS: linux-media: update Microchip ISI and ISC entries
On 09/01/2018 at 14:46, Nicolas Ferre wrote: > These two image capture interface drivers are now handled > by Wenyou Yang. > I benefit from this change to update the two entries by correcting the > binding documentation link for ISC and moving the ISI to the new > MICROCHIP / ATMEL location. > > Signed-off-by: Nicolas Ferre> --- > Hi, > > Patch against next-20180109. > Note that I didn't find it useful to have several patches for these changes. > Tell me if you feel differently. > > I would like to have the Ack from Ludovic and Wenyou obviously. I don't know > if > Songjun can answer as he's not with Microchip anymore. Update on this patch: Boris took the second patch of the series through NAND/MTD tree so this one can go alone upstream through the linux-media tree. I also have the 2 required Ack. So, do you want me to re-send this one independently or should we wait for 4.16-rc1? Best regards, Nicolas > MAINTAINERS | 19 ++- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index a7d10a2bb980..65c4b59b582f 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2353,13 +2353,6 @@ L: linux-...@vger.kernel.org > S: Supported > F: drivers/i2c/busses/i2c-at91.c > > -ATMEL ISI DRIVER > -M: Ludovic Desroches > -L: linux-media@vger.kernel.org > -S: Supported > -F: drivers/media/platform/atmel/atmel-isi.c > -F: include/media/atmel-isi.h > - > ATMEL LCDFB DRIVER > M: Nicolas Ferre > L: linux-fb...@vger.kernel.org > @@ -9102,12 +9095,20 @@ S:Maintained > F: drivers/crypto/atmel-ecc.* > > MICROCHIP / ATMEL ISC DRIVER > -M: Songjun Wu > +M: Wenyou Yang > L: linux-media@vger.kernel.org > S: Supported > F: drivers/media/platform/atmel/atmel-isc.c > F: drivers/media/platform/atmel/atmel-isc-regs.h > -F: devicetree/bindings/media/atmel-isc.txt > +F: Documentation/devicetree/bindings/media/atmel-isc.txt > + > +MICROCHIP / ATMEL ISI DRIVER > +M: Wenyou Yang > +L: linux-media@vger.kernel.org > +S: Supported > +F: drivers/media/platform/atmel/atmel-isi.c > +F: include/media/atmel-isi.h > +F: Documentation/devicetree/bindings/media/atmel-isi.txt > > MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER > M: Woojung Huh > -- Nicolas Ferre
Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
On Fri, Jan 12, 2018 at 05:19:44PM +0100, Hans Verkuil wrote: > Hi Ville, > > For some strange reason your email disappeared from the Cc list. Perhaps it's > the > ä that confuses something somewhere. > > So I'll just forward this directly to you. > > Can you please take a look? This patch series has been in limbo for too long. IIRC last I looked we still had some ragistration race to deal with. Was that fixed? Also I think we got stuck on leaving the zombie device lingering around when the display is disconnected. I couldn't understand why that is at all useful since you anyway remove the device eventually. Adding the lists back to cc so I don't have to repeat myself there... > > Regards, > > Hans > > > Forwarded Message > Subject: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX > support > Date: Tue, 9 Jan 2018 13:46:44 +0100 > From: Hans Verkuil> To: linux-media@vger.kernel.org > CC: Daniel Vetter , Carlos Santa > , dri-de...@lists.freedesktop.org > > First of all a Happy New Year for all of you! > > And secondly: can this v5 patch series be reviewed/merged? It's been waiting > for that for a very long time now... > > Regards, > > Hans > > On 12/11/17 09:57, Hans Verkuil wrote: > > Ping again. Added a CC to Ville whom I inexplicably forgot to add when > > I sent the v5 patch series. > > > > Regards, > > > > Hans > > > > On 01/12/17 08:23, Hans Verkuil wrote: > >> Ping! > >> > >> I really like to get this in for 4.16 so I can move forward with hooking > >> this up for nouveau/amd. > >> > >> Regards, > >> > >>Hans > >> > >> On 11/20/2017 12:42 PM, Hans Verkuil wrote: > >>> This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX > >>> feature. This patch series is based on the 4.14 mainline release but > >>> applies > >>> as well to drm-next. > >>> > >>> This patch series has been tested with my NUC7i5BNK, a Samsung USB-C to > >>> HDMI adapter and a Club 3D DisplayPort MST Hub + modified UpTab DP-to-HDMI > >>> adapter (where the CEC pin is wired up). > >>> > >>> Please note this comment at the start of drm_dp_cec.c: > >>> > >>> -- > >>> Unfortunately it turns out that we have a chicken-and-egg situation > >>> here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI adapters > >>> have a converter chip that supports CEC-Tunneling-over-AUX (usually the > >>> Parade PS176 or MegaChips MCDP2900), but they do not wire up the CEC pin, > >>> thus making CEC useless. > >>> > >>> Sadly there is no way for this driver to know this. What happens is > >>> that a /dev/cecX device is created that is isolated and unable to see > >>> any of the other CEC devices. Quite literally the CEC wire is cut > >>> (or in this case, never connected in the first place). > >>> > >>> I suspect that the reason so few adapters support this is that this > >>> tunneling protocol was never supported by any OS. So there was no > >>> easy way of testing it, and no incentive to correctly wire up the > >>> CEC pin. > >>> > >>> Hopefully by creating this driver it will be easier for vendors to > >>> finally fix their adapters and test the CEC functionality. > >>> > >>> I keep a list of known working adapters here: > >>> > >>> https://hverkuil.home.xs4all.nl/cec-status.txt > >>> > >>> Please mail me (hverk...@xs4all.nl) if you find an adapter that works > >>> and is not yet listed there. > >>> > >>> Note that the current implementation does not support CEC over an MST hub. > >>> As far as I can see there is no mechanism defined in the DisplayPort > >>> standard to transport CEC interrupts over an MST device. It might be > >>> possible to do this through polling, but I have not been able to get that > >>> to work. > >>> -- > >>> > >>> I really hope that this work will provide an incentive for vendors to > >>> finally connect the CEC pin. It's a shame that there are so few adapters > >>> that work (I found only two USB-C to HDMI adapters that work, and no > >>> (mini-)DP to HDMI adapters at all). > >>> > >>> Hopefully if this gets merged there will be an incentive for vendors > >>> to make adapters where this actually works. It is a very nice feature > >>> for HTPC boxes. > >>> > >>> The main reason why this v5 is delayed by 2 months is due to the fact > >>> that I needed some dedicated time to investigate what happens when an > >>> MST hub is in use. It turns out that this is not working. There is no > >>> mechanism defined in the DisplayPort standard to transport the CEC > >>> interrupt back up the MST chain. I was actually able to send a CEC > >>> message but the interrupt that tells when the transmit finished is > >>> unavailable. > >>> > >>> I attempted to implement this via polling, but I got weird errors > >>> and was not able to read the
[PATCH 3/7] si2157: Add hybrid tuner support
Add ability to share a tuner amongst demodulators. Addtional demods are attached using hybrid_tuner_instance_list. The changes are equivalent to moving all of probe to _attach. Results are backwards compatible with current usage. If the tuner is acquired via attach, then .release cleans state. if the tuner is an i2c driver, then .release is set to NULL, and .remove cleans remaining state. The following file contains a static si2157_attach: - drivers/media/pci/saa7164/saa7164-dvb.c The function name has been appended with _priv to appease the compiler. Signed-off-by: Brad Love--- drivers/media/pci/saa7164/saa7164-dvb.c | 11 +- drivers/media/tuners/si2157.c | 232 +++- drivers/media/tuners/si2157.h | 14 ++ drivers/media/tuners/si2157_priv.h | 5 + 4 files changed, 192 insertions(+), 70 deletions(-) diff --git a/drivers/media/pci/saa7164/saa7164-dvb.c b/drivers/media/pci/saa7164/saa7164-dvb.c index e76d3ba..9522c6c 100644 --- a/drivers/media/pci/saa7164/saa7164-dvb.c +++ b/drivers/media/pci/saa7164/saa7164-dvb.c @@ -110,8 +110,9 @@ static struct si2157_config hauppauge_hvr2255_tuner_config = { .if_port = 1, }; -static int si2157_attach(struct saa7164_port *port, struct i2c_adapter *adapter, - struct dvb_frontend *fe, u8 addr8bit, struct si2157_config *cfg) +static int si2157_attach_priv(struct saa7164_port *port, + struct i2c_adapter *adapter, struct dvb_frontend *fe, + u8 addr8bit, struct si2157_config *cfg) { struct i2c_board_info bi; struct i2c_client *tuner; @@ -624,11 +625,13 @@ int saa7164_dvb_register(struct saa7164_port *port) if (port->dvb.frontend != NULL) { if (port->nr == 0) { - si2157_attach(port, >i2c_bus[0].i2c_adap, + si2157_attach_priv(port, + >i2c_bus[0].i2c_adap, port->dvb.frontend, 0xc0, _hvr2255_tuner_config); } else { - si2157_attach(port, >i2c_bus[1].i2c_adap, + si2157_attach_priv(port, + >i2c_bus[1].i2c_adap, port->dvb.frontend, 0xc0, _hvr2255_tuner_config); } diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c index e35b1fa..9121361 100644 --- a/drivers/media/tuners/si2157.c +++ b/drivers/media/tuners/si2157.c @@ -18,6 +18,11 @@ static const struct dvb_tuner_ops si2157_ops; +static DEFINE_MUTEX(si2157_list_mutex); +static LIST_HEAD(hybrid_tuner_instance_list); + +/*-*/ + /* execute firmware command */ static int si2157_cmd_execute(struct i2c_client *client, struct si2157_cmd *cmd) { @@ -385,6 +390,31 @@ static int si2157_get_if_frequency(struct dvb_frontend *fe, u32 *frequency) return 0; } +static void si2157_release(struct dvb_frontend *fe) +{ + struct i2c_client *client = fe->tuner_priv; + struct si2157_dev *dev = i2c_get_clientdata(client); + + dev_dbg(>dev, "%s()\n", __func__); + + /* only do full cleanup on final instance */ + if (hybrid_tuner_report_instance_count(dev) == 1) { + /* stop statistics polling */ + cancel_delayed_work_sync(>stat_work); +#ifdef CONFIG_MEDIA_CONTROLLER_DVB + if (dev->mdev) + media_device_unregister_entity(>ent); +#endif + i2c_set_clientdata(client, NULL); + } + + mutex_lock(_list_mutex); + hybrid_tuner_release_state(dev); + mutex_unlock(_list_mutex); + + fe->tuner_priv = NULL; +} + static const struct dvb_tuner_ops si2157_ops = { .info = { .name = "Silicon Labs Si2141/Si2146/2147/2148/2157/2158", @@ -396,6 +426,7 @@ static const struct dvb_tuner_ops si2157_ops = { .sleep = si2157_sleep, .set_params = si2157_set_params, .get_if_frequency = si2157_get_if_frequency, + .release = si2157_release, }; static void si2157_stat_work(struct work_struct *work) @@ -431,72 +462,30 @@ static int si2157_probe(struct i2c_client *client, { struct si2157_config *cfg = client->dev.platform_data; struct dvb_frontend *fe = cfg->fe; - struct si2157_dev *dev; - struct si2157_cmd cmd; - int ret; - - dev = kzalloc(sizeof(*dev), GFP_KERNEL); - if (!dev) { - ret = -ENOMEM; - dev_err(>dev, "kzalloc() failed\n"); - goto err; - } - - i2c_set_clientdata(client, dev); - dev->fe = cfg->fe; - dev->inversion = cfg->inversion; - dev->if_port =
[PATCH 7/7] cx231xx: Add second i2c demod to Hauppauge 975
Hauppauge HVR-975 is a dual frontend, single tuner USB device. It contains lgdt3306a and si2168 frontends and one si2157 tuner. The lgdt3306a frontend is currently enabled. This creates the second demodulator and attaches it to the tuner. Enables lgdt3306a|si2168 + si2157 Signed-off-by: Brad Love--- drivers/media/usb/cx231xx/cx231xx-cards.c | 1 + drivers/media/usb/cx231xx/cx231xx-dvb.c | 50 +-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c index 8582568..00e88a8f 100644 --- a/drivers/media/usb/cx231xx/cx231xx-cards.c +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c @@ -979,6 +979,7 @@ struct cx231xx_board cx231xx_boards[] = { .demod_i2c_master = I2C_1_MUX_3, .has_dvb = 1, .demod_addr = 0x59, /* 0xb2 >> 1 */ + .demod_addr2 = 0x64, /* 0xc8 >> 1 */ .norm = V4L2_STD_ALL, .input = {{ diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c index 7201e14..7c947ca 100644 --- a/drivers/media/usb/cx231xx/cx231xx-dvb.c +++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c @@ -1173,14 +1173,17 @@ static int dvb_init(struct cx231xx *dev) { struct i2c_client *client; struct i2c_adapter *adapter; + struct i2c_adapter *adapter2; struct i2c_board_info info = {}; struct si2157_config si2157_config = {}; struct lgdt3306a_config lgdt3306a_config = {}; + struct si2168_config si2168_config = {}; - /* attach demodulator chip */ + /* attach first demodulator chip */ lgdt3306a_config = hauppauge_955q_lgdt3306a_config; lgdt3306a_config.fe = >dvb->frontend[0]; lgdt3306a_config.i2c_adapter = + lgdt3306a_config.deny_i2c_rptr = 0; strlcpy(info.type, "lgdt3306a", sizeof(info.type)); info.addr = dev->board.demod_addr; @@ -1202,10 +1205,39 @@ static int dvb_init(struct cx231xx *dev) } dvb->i2c_client_demod[0] = client; - dev->dvb->frontend[0]->ops.i2c_gate_ctrl = NULL; + + /* attach second demodulator chip */ + si2168_config.ts_mode = SI2168_TS_SERIAL; + si2168_config.fe = >dvb->frontend[1]; + si2168_config.i2c_adapter = + si2168_config.ts_clock_inv = true; + + memset(, 0, sizeof(struct i2c_board_info)); + strlcpy(info.type, "si2168", sizeof(info.type)); + info.addr = dev->board.demod_addr2; + info.platform_data = _config; + + request_module(info.type); + client = i2c_new_device(adapter, ); + if (client == NULL || client->dev.driver == NULL) { + result = -ENODEV; + goto out_free; + } + + if (!try_module_get(client->dev.driver->owner)) { + dev_err(dev->dev, + "Failed to attach %s frontend.\n", info.type); + i2c_unregister_device(client); + result = -ENODEV; + goto out_free; + } + + dvb->i2c_client_demod[1] = client; + dev->dvb->frontend[1]->id = 1; /* define general-purpose callback pointer */ dvb->frontend[0]->callback = cx231xx_tuner_callback; + dvb->frontend[1]->callback = cx231xx_tuner_callback; /* attach tuner */ si2157_config.fe = dev->dvb->frontend[0]; @@ -1221,8 +1253,10 @@ static int dvb_init(struct cx231xx *dev) info.platform_data = _config; request_module("si2157"); - client = i2c_new_device(tuner_i2c, ); + client = i2c_new_device(adapter, ); if (client == NULL || client->dev.driver == NULL) { + module_put(dvb->i2c_client_demod[1]->dev.driver->owner); + i2c_unregister_device(dvb->i2c_client_demod[1]); module_put(dvb->i2c_client_demod[0]->dev.driver->owner); i2c_unregister_device(dvb->i2c_client_demod[0]); result = -ENODEV; @@ -1233,6 +1267,8 @@ static int dvb_init(struct cx231xx *dev) dev_err(dev->dev, "Failed to obtain %s tuner.\n", info.type); i2c_unregister_device(client); + module_put(dvb->i2c_client_demod[1]->dev.driver->owner); + i2c_unregister_device(dvb->i2c_client_demod[1]); module_put(dvb->i2c_client_demod[0]->dev.driver->owner);
[PATCH 4/7] si2168: Add ts bus coontrol, turn off bus on sleep
Includes a function to set TS MODE property os si2168. The function either disables the TS output bus, or sets mode to config option. When going to sleep the TS bus is turned off, this makes the driver compatible with multiple frontend usage. Signed-off-by: Brad Love--- drivers/media/dvb-frontends/si2168.c | 38 drivers/media/dvb-frontends/si2168.h | 1 + 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c index 539399d..429c03a 100644 --- a/drivers/media/dvb-frontends/si2168.c +++ b/drivers/media/dvb-frontends/si2168.c @@ -409,6 +409,30 @@ static int si2168_set_frontend(struct dvb_frontend *fe) return ret; } +static int si2168_ts_bus_ctrl(struct dvb_frontend *fe, int acquire) +{ + struct i2c_client *client = fe->demodulator_priv; + struct si2168_dev *dev = i2c_get_clientdata(client); + struct si2168_cmd cmd; + int ret = 0; + + dev_dbg(>dev, "%s acquire: %d\n", __func__, acquire); + + /* set TS_MODE property */ + memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6); + if (acquire) + cmd.args[4] |= dev->ts_mode; + else + cmd.args[4] |= SI2168_TS_TRISTATE; + if (dev->ts_clock_gapped) + cmd.args[4] |= 0x40; + cmd.wlen = 6; + cmd.rlen = 4; + ret = si2168_cmd_execute(client, ); + + return ret; +} + static int si2168_init(struct dvb_frontend *fe) { struct i2c_client *client = fe->demodulator_priv; @@ -540,14 +564,7 @@ static int si2168_init(struct dvb_frontend *fe) dev->version >> 24 & 0xff, dev->version >> 16 & 0xff, dev->version >> 8 & 0xff, dev->version >> 0 & 0xff); - /* set ts mode */ - memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6); - cmd.args[4] |= dev->ts_mode; - if (dev->ts_clock_gapped) - cmd.args[4] |= 0x40; - cmd.wlen = 6; - cmd.rlen = 4; - ret = si2168_cmd_execute(client, ); + ret = si2168_ts_bus_ctrl(fe, 1); if (ret) goto err; @@ -584,6 +601,9 @@ static int si2168_sleep(struct dvb_frontend *fe) dev->active = false; + /* tri-state data bus */ + si2168_ts_bus_ctrl(fe, 0); + /* Firmware B 4.0-11 or later loses warm state during sleep */ if (dev->version > ('B' << 24 | 4 << 16 | 0 << 8 | 11 << 0)) dev->warm = false; @@ -681,6 +701,8 @@ static const struct dvb_frontend_ops si2168_ops = { .init = si2168_init, .sleep = si2168_sleep, + .ts_bus_ctrl = si2168_ts_bus_ctrl, + .set_frontend = si2168_set_frontend, .read_status = si2168_read_status, diff --git a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h index 3225d0c..f48f0fb 100644 --- a/drivers/media/dvb-frontends/si2168.h +++ b/drivers/media/dvb-frontends/si2168.h @@ -38,6 +38,7 @@ struct si2168_config { /* TS mode */ #define SI2168_TS_PARALLEL 0x06 #define SI2168_TS_SERIAL 0x03 +#define SI2168_TS_TRISTATE 0x00 u8 ts_mode; /* TS clock inverted */ -- 2.7.4
[PATCH 6/7] si2168: Announce frontend creation failure
The driver outputs on success, but is silent on failure. Give one message that probe failed. Signed-off-by: Brad Love--- drivers/media/dvb-frontends/si2168.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c index 429c03a..c1a638c 100644 --- a/drivers/media/dvb-frontends/si2168.c +++ b/drivers/media/dvb-frontends/si2168.c @@ -810,7 +810,7 @@ static int si2168_probe(struct i2c_client *client, err_kfree: kfree(dev); err: - dev_dbg(>dev, "failed=%d\n", ret); + dev_warn(>dev, "probe failed = %d\n", ret); return ret; } -- 2.7.4
[PATCH 0/7] cx231xx: Add multiple frontend USB device
This patch set requires: https://patchwork.linuxtv.org/patch/46396/ https://patchwork.linuxtv.org/patch/46397/ The Hauppauge HVR-975 is a dual frontend, single tuner USB device. The 975 has lgdt3306a (currently enabled) and si2168 demodulators, and one si2157 tuner. It provides analog capture via breakout cable. This patch set adds pieces to the cx231xx USB bridge to allow a second frontend, whether it is old dvb_attach style, or new i2c device style. A new field is added to board config to accomodate second demod address. To accomodate addubg the second demodulator to the si2157 tuner, hybrid tuner instance functionality was added. The contents of probe, moved to attach, and .release is provided for shared instances to clean their state. All changes are backwards compatible and transparent to current usages. The si2168 frontend driver required addition of ts bus control, without this both frontends remain active, after switching between, and the demux provides no data thereafter. Finally the second demod is added to the HVR975 and attached to the si2157. Brad Love (7): cx231xx: Add second frontend option cx231xx: Add second i2c demod client si2157: Add hybrid tuner support si2168: Add ts bus coontrol, turn off bus on sleep si2168: Announce frontend creation failure lgdt3306a: Announce successful creation cx231xx: Add second i2c demod to Hauppauge 975 drivers/media/dvb-frontends/lgdt3306a.c | 4 +- drivers/media/dvb-frontends/si2168.c| 40 - drivers/media/dvb-frontends/si2168.h| 1 + drivers/media/pci/saa7164/saa7164-dvb.c | 11 +- drivers/media/tuners/si2157.c | 232 +--- drivers/media/tuners/si2157.h | 14 ++ drivers/media/tuners/si2157_priv.h | 5 + drivers/media/usb/cx231xx/cx231xx-cards.c | 1 + drivers/media/usb/cx231xx/cx231xx-dvb.c | 269 ++-- drivers/media/usb/cx231xx/cx231xx-dvb.c.rej | 11 ++ drivers/media/usb/cx231xx/cx231xx.h | 1 + 11 files changed, 411 insertions(+), 178 deletions(-) create mode 100644 drivers/media/usb/cx231xx/cx231xx-dvb.c.rej -- 2.7.4
[PATCH 5/7] lgdt3306a: Announce successful creation
The driver is near silent, this adds a simple announcement at the end of probe after the chip has been detected and upgrades a debug message to error if probe has failed. Signed-off-by: Brad Love--- drivers/media/dvb-frontends/lgdt3306a.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c index 3642e6e..ec7d04d 100644 --- a/drivers/media/dvb-frontends/lgdt3306a.c +++ b/drivers/media/dvb-frontends/lgdt3306a.c @@ -2202,6 +2202,8 @@ static int lgdt3306a_probe(struct i2c_client *client, *config->i2c_adapter = state->muxc->adapter[0]; *config->fe = fe; + dev_info(>dev, "LG Electronics LGDT3306A successfully identified\n"); + return 0; err_kfree: @@ -2209,7 +2211,7 @@ static int lgdt3306a_probe(struct i2c_client *client, err_fe: kfree(config); fail: - dev_dbg(>dev, "failed=%d\n", ret); + dev_warn(>dev, "probe failed = %d\n", ret); return ret; } -- 2.7.4
[PATCH 2/7] cx231xx: Add second i2c demod client
Include ability to add a i2c device style frontend to cx231xx USB bridge. All current boards set to use frontend[0]. Changes are backwards compatible with current behaviour. Signed-off-by: Brad Love--- drivers/media/usb/cx231xx/cx231xx-dvb.c | 45 ++--- drivers/media/usb/cx231xx/cx231xx.h | 1 + 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c index 4c6d2f4..7201e14 100644 --- a/drivers/media/usb/cx231xx/cx231xx-dvb.c +++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c @@ -68,7 +68,7 @@ struct cx231xx_dvb { struct dmx_frontend fe_hw; struct dmx_frontend fe_mem; struct dvb_net net; - struct i2c_client *i2c_client_demod; + struct i2c_client *i2c_client_demod[2]; struct i2c_client *i2c_client_tuner; }; @@ -616,7 +616,12 @@ static void unregister_dvb(struct cx231xx_dvb *dvb) i2c_unregister_device(client); } /* remove I2C demod */ - client = dvb->i2c_client_demod; + client = dvb->i2c_client_demod[1]; + if (client) { + module_put(client->dev.driver->owner); + i2c_unregister_device(client); + } + client = dvb->i2c_client_demod[0]; if (client) { module_put(client->dev.driver->owner); i2c_unregister_device(client); @@ -805,7 +810,7 @@ static int dvb_init(struct cx231xx *dev) goto out_free; } - dvb->i2c_client_demod = client; + dvb->i2c_client_demod[0] = client; dev->dvb->frontend[0]->ops.i2c_gate_ctrl = NULL; @@ -852,7 +857,7 @@ static int dvb_init(struct cx231xx *dev) goto out_free; } - dvb->i2c_client_demod = client; + dvb->i2c_client_demod[0] = client; memset(, 0, sizeof(struct i2c_board_info)); @@ -1012,7 +1017,7 @@ static int dvb_init(struct cx231xx *dev) goto out_free; } - dvb->i2c_client_demod = client; + dvb->i2c_client_demod[0] = client; /* attach tuner chip */ si2157_config.fe = dev->dvb->frontend[0]; @@ -1031,16 +1036,16 @@ static int dvb_init(struct cx231xx *dev) client = i2c_new_device(tuner_i2c, ); if (client == NULL || client->dev.driver == NULL) { - module_put(dvb->i2c_client_demod->dev.driver->owner); - i2c_unregister_device(dvb->i2c_client_demod); + module_put(dvb->i2c_client_demod[0]->dev.driver->owner); + i2c_unregister_device(dvb->i2c_client_demod[0]); result = -ENODEV; goto out_free; } if (!try_module_get(client->dev.driver->owner)) { i2c_unregister_device(client); - module_put(dvb->i2c_client_demod->dev.driver->owner); - i2c_unregister_device(dvb->i2c_client_demod); + module_put(dvb->i2c_client_demod[0]->dev.driver->owner); + i2c_unregister_device(dvb->i2c_client_demod[0]); result = -ENODEV; goto out_free; } @@ -1078,7 +1083,7 @@ static int dvb_init(struct cx231xx *dev) goto out_free; } - dvb->i2c_client_demod = client; + dvb->i2c_client_demod[0] = client; /* define general-purpose callback pointer */ dvb->frontend[0]->callback = cx231xx_tuner_callback; @@ -1122,7 +1127,7 @@ static int dvb_init(struct cx231xx *dev) goto out_free; } - dvb->i2c_client_demod = client; + dvb->i2c_client_demod[0] = client; dev->dvb->frontend[0]->ops.i2c_gate_ctrl = NULL; /* define general-purpose callback pointer */ @@ -1144,8 +1149,8 @@ static int dvb_init(struct cx231xx *dev) client = i2c_new_device(adapter, ); if (client == NULL || client->dev.driver == NULL) { - module_put(dvb->i2c_client_demod->dev.driver->owner); - i2c_unregister_device(dvb->i2c_client_demod); + module_put(dvb->i2c_client_demod[0]->dev.driver->owner); + i2c_unregister_device(dvb->i2c_client_demod[0]); result = -ENODEV; goto out_free; } @@ -1154,8 +1159,8 @@ static int dvb_init(struct cx231xx *dev) dev_err(dev->dev, "Failed to obtain %s tuner.\n", info.type); i2c_unregister_device(client); -
[PATCH 1/7] cx231xx: Add second frontend option
Include ability to add a second dvb attach style frontend to cx231xx USB bridge. All current boards set to use frontend[0]. Changes are backwards compatible with current behaviour. Signed-off-by: Brad Love--- drivers/media/usb/cx231xx/cx231xx-dvb.c | 173 ++-- 1 file changed, 97 insertions(+), 76 deletions(-) diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c index cb4209f..4c6d2f4 100644 --- a/drivers/media/usb/cx231xx/cx231xx-dvb.c +++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c @@ -55,7 +55,7 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); #define CX231XX_DVB_MAX_PACKETS 64 struct cx231xx_dvb { - struct dvb_frontend *frontend; + struct dvb_frontend *frontend[2]; /* feed count management */ struct mutex lock; @@ -386,17 +386,17 @@ static int attach_xc5000(u8 addr, struct cx231xx *dev) cfg.i2c_adap = cx231xx_get_i2c_adap(dev, dev->board.tuner_i2c_master); cfg.i2c_addr = addr; - if (!dev->dvb->frontend) { + if (!dev->dvb->frontend[0]) { dev_err(dev->dev, "%s/2: dvb frontend not attached. Can't attach xc5000\n", dev->name); return -EINVAL; } - fe = dvb_attach(xc5000_attach, dev->dvb->frontend, ); + fe = dvb_attach(xc5000_attach, dev->dvb->frontend[0], ); if (!fe) { dev_err(dev->dev, "%s/2: xc5000 attach failed\n", dev->name); - dvb_frontend_detach(dev->dvb->frontend); - dev->dvb->frontend = NULL; + dvb_frontend_detach(dev->dvb->frontend[0]); + dev->dvb->frontend[0] = NULL; return -EINVAL; } @@ -408,9 +408,9 @@ static int attach_xc5000(u8 addr, struct cx231xx *dev) int cx231xx_set_analog_freq(struct cx231xx *dev, u32 freq) { - if ((dev->dvb != NULL) && (dev->dvb->frontend != NULL)) { + if ((dev->dvb != NULL) && (dev->dvb->frontend[0] != NULL)) { - struct dvb_tuner_ops *dops = >dvb->frontend->ops.tuner_ops; + struct dvb_tuner_ops *dops = >dvb->frontend[0]->ops.tuner_ops; if (dops->set_analog_params != NULL) { struct analog_parameters params; @@ -421,7 +421,7 @@ int cx231xx_set_analog_freq(struct cx231xx *dev, u32 freq) /*params.audmode = ; */ /* Set the analog parameters to set the frequency */ - dops->set_analog_params(dev->dvb->frontend, ); + dops->set_analog_params(dev->dvb->frontend[0], ); } } @@ -433,15 +433,15 @@ int cx231xx_reset_analog_tuner(struct cx231xx *dev) { int status = 0; - if ((dev->dvb != NULL) && (dev->dvb->frontend != NULL)) { + if ((dev->dvb != NULL) && (dev->dvb->frontend[0] != NULL)) { - struct dvb_tuner_ops *dops = >dvb->frontend->ops.tuner_ops; + struct dvb_tuner_ops *dops = >dvb->frontend[0]->ops.tuner_ops; if (dops->init != NULL && !dev->xc_fw_load_done) { dev_dbg(dev->dev, "Reloading firmware for XC5000\n"); - status = dops->init(dev->dvb->frontend); + status = dops->init(dev->dvb->frontend[0]); if (status == 0) { dev->xc_fw_load_done = 1; dev_dbg(dev->dev, @@ -481,17 +481,29 @@ static int register_dvb(struct cx231xx_dvb *dvb, dvb_register_media_controller(>adapter, dev->media_dev); /* Ensure all frontends negotiate bus access */ - dvb->frontend->ops.ts_bus_ctrl = cx231xx_dvb_bus_ctrl; + dvb->frontend[0]->ops.ts_bus_ctrl = cx231xx_dvb_bus_ctrl; + if (dvb->frontend[1]) + dvb->frontend[1]->ops.ts_bus_ctrl = cx231xx_dvb_bus_ctrl; dvb->adapter.priv = dev; /* register frontend */ - result = dvb_register_frontend(>adapter, dvb->frontend); + result = dvb_register_frontend(>adapter, dvb->frontend[0]); if (result < 0) { dev_warn(dev->dev, "%s: dvb_register_frontend failed (errno = %d)\n", dev->name, result); - goto fail_frontend; + goto fail_frontend0; + } + + if (dvb->frontend[1]) { + result = dvb_register_frontend(>adapter, dvb->frontend[1]); + if (result < 0) { + dev_warn(dev->dev, + "%s: 2nd dvb_register_frontend failed (errno = %d)\n", + dev->name, result); + goto fail_frontend1; + } } /* register demux stuff */ @@ -569,9 +581,14 @@ static int register_dvb(struct cx231xx_dvb *dvb, fail_dmxdev: dvb_dmx_release(>demux);
[GIT PULL FOR v4.16] Various fixes
Hi Mauro, Some fixes for 4.16. Note the cec-gpio.txt patch: that has a CC to stable for 4.15. It's probably too late to get that in for 4.15 itself, but it definitely should be backported to 4.15. Otherwise you can fry your Rpi if you aren't aware of the voltages involved. Regards, Hans The following changes since commit e3ee691dbf24096ea51b3200946b11d68ce75361: media: ov5640: add support of RGB565 and YUYV formats (2018-01-05 12:54:14 -0500) are available in the Git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.16f for you to fetch changes up to 2b7d6056e7f0e235bc6abe70acf52afdb3a02448: drivers/media/common/videobuf2: rename from videobuf (2018-01-12 16:33:34 +0100) Arnd Bergmann (2): media: staging: tegra-vde: select DMA_SHARED_BUFFER media: cobalt: select CONFIG_SND_PCM Hans Verkuil (2): dt-bindings/media/cec-gpio.txt: mention the CEC/HPD max voltages drivers/media/common/videobuf2: rename from videobuf Documentation/devicetree/bindings/media/cec-gpio.txt| 6 +- drivers/media/common/Kconfig| 2 +- drivers/media/common/Makefile | 2 +- drivers/media/common/{videobuf => videobuf2}/Kconfig| 0 drivers/media/common/{videobuf => videobuf2}/Makefile | 0 drivers/media/common/{videobuf => videobuf2}/videobuf2-core.c | 0 drivers/media/common/{videobuf => videobuf2}/videobuf2-dma-contig.c | 0 drivers/media/common/{videobuf => videobuf2}/videobuf2-dma-sg.c | 0 drivers/media/common/{videobuf => videobuf2}/videobuf2-dvb.c| 0 drivers/media/common/{videobuf => videobuf2}/videobuf2-memops.c | 0 drivers/media/common/{videobuf => videobuf2}/videobuf2-v4l2.c | 0 drivers/media/common/{videobuf => videobuf2}/videobuf2-vmalloc.c| 0 drivers/media/pci/cobalt/Kconfig| 1 + drivers/staging/media/tegra-vde/Kconfig | 1 + 14 files changed, 9 insertions(+), 3 deletions(-) rename drivers/media/common/{videobuf => videobuf2}/Kconfig (100%) rename drivers/media/common/{videobuf => videobuf2}/Makefile (100%) rename drivers/media/common/{videobuf => videobuf2}/videobuf2-core.c (100%) rename drivers/media/common/{videobuf => videobuf2}/videobuf2-dma-contig.c (100%) rename drivers/media/common/{videobuf => videobuf2}/videobuf2-dma-sg.c (100%) rename drivers/media/common/{videobuf => videobuf2}/videobuf2-dvb.c (100%) rename drivers/media/common/{videobuf => videobuf2}/videobuf2-memops.c (100%) rename drivers/media/common/{videobuf => videobuf2}/videobuf2-v4l2.c (100%) rename drivers/media/common/{videobuf => videobuf2}/videobuf2-vmalloc.c (100%)
Re: MT9M131 on I.MX6DL CSI color issue
Le vendredi 12 janvier 2018 à 10:58 +0100, Anatolij Gustschin a écrit : > On Fri, 12 Jan 2018 01:16:03 +0100 > Florian Boor florian.b...@kernelconcepts.de wrote: > ... > > Basically it works pretty well apart from the really strange > > colors. I guess its > > some YUV vs. RGB issue or similar. Here [1] is an example generated > > with the > > following command. > > > > gst-launch v4l2src device=/dev/video4 num-buffers=1 ! jpegenc ! > > filesink > > location=capture1.jpeg > > > > Apart from the colors everything is fine. > > I'm pretty sure I have not seen such an effect before - what might > > be wrong here? > > You need conversion to RGB before JPEG encoding. Try with > > gst-launch v4l2src device=/dev/video4 num-buffers=1 ! \ > videoparse format=5 width=1280 height=1024 framerate=25/1 > ! \ > jpegenc ! filesink location=capture1.jpeg > > For "format" codes see gst-inspect-1.0 videoparse. A properly written driver should never permit this. Nicolas
Re: [PATCH v7 6/6] [media] v4l: Document explicit synchronization behavior
On 01/10/18 17:07, Gustavo Padovan wrote: > From: Gustavo Padovan> > Add section to VIDIOC_QBUF about it > > v5: > - Remove V4L2_CAP_ORDERED > - Add doc about V4L2_FMT_FLAG_UNORDERED > > v4: > - Document ordering behavior for in-fences > - Document V4L2_CAP_ORDERED capability > - Remove doc about OUT_FENCE event > - Document immediate return of out-fence in QBUF > > v3: > - make the out_fence refer to the current buffer (Hans) > - Note what happens when the IN_FENCE is not set (Hans) > > v2: > - mention that fences are files (Hans) > - rework for the new API > > Signed-off-by: Gustavo Padovan > --- > Documentation/media/uapi/v4l/vidioc-qbuf.rst | 47 > +++- > Documentation/media/uapi/v4l/vidioc-querybuf.rst | 9 - > 2 files changed, 54 insertions(+), 2 deletions(-) > > diff --git a/Documentation/media/uapi/v4l/vidioc-qbuf.rst > b/Documentation/media/uapi/v4l/vidioc-qbuf.rst > index 9e448a4aa3aa..8809397fb110 100644 > --- a/Documentation/media/uapi/v4l/vidioc-qbuf.rst > +++ b/Documentation/media/uapi/v4l/vidioc-qbuf.rst > @@ -54,7 +54,7 @@ When the buffer is intended for output (``type`` is > or ``V4L2_BUF_TYPE_VBI_OUTPUT``) applications must also initialize the > ``bytesused``, ``field`` and ``timestamp`` fields, see :ref:`buffer` > for details. Applications must also set ``flags`` to 0. The > -``reserved2`` and ``reserved`` fields must be set to 0. When using the > +``reserved`` field must be set to 0. When using the > :ref:`multi-planar API `, the ``m.planes`` field must > contain a userspace pointer to a filled-in array of struct > :c:type:`v4l2_plane` and the ``length`` field must be set > @@ -118,6 +118,51 @@ immediately with an ``EAGAIN`` error code when no buffer > is available. > The struct :c:type:`v4l2_buffer` structure is specified in > :ref:`buffer`. > > +Explicit Synchronization > + > + > +Explicit Synchronization allows us to control the synchronization of > +shared buffers from userspace by passing fences to the kernel and/or > +receiving them from it. Fences passed to the kernel are named in-fences and > +the kernel should wait on them to signal before using the buffer, i.e., > queueing > +it to the driver. I would drop ", i.e., queueing it to the driver" On the other side, the kernel can create out-fences for the > +buffers it queues to the drivers. Out-fences signal when the driver is > +finished with buffer, i.e., the buffer is ready. The fences are represented > +as a file and passed as a file descriptor to userspace. > + > +The in-fences are communicated to the kernel at the ``VIDIOC_QBUF`` ioctl > +using the ``V4L2_BUF_FLAG_IN_FENCE`` buffer flag and the `fence_fd` field. If > +an in-fence needs to be passed to the kernel, `fence_fd` should be set to the > +fence file descriptor number and the ``V4L2_BUF_FLAG_IN_FENCE`` should be set > +as well. Setting one but not the other will cause ``VIDIOC_QBUF`` to return > +with error. The fence_fd field will be ignored if the with -> with an > +``V4L2_BUF_FLAG_IN_FENCE`` is not set. > + > +The videobuf2-core will guarantee that all buffers queued with in-fence will with -> with an > +be queued to the drivers in the same order. Fence may signal out of order, so Fence -> Fences > +this guarantee at videobuf2 is necessary to not change ordering. Add some text here to make it clear that waiting for a buffer with an in-fence will also block all buffers queued after that buffer. > + > +If the in-fence signals with an error the videobuf2 won't queue the buffer to > +the driver, instead it will flag it with an error. And then wait for the > +previous buffer to complete before asking userspace dequeue the buffer with > +error - to make sure we deliver the buffers back in the correct order. I think there is no need to describe the internal implementation. It is enough to say that the buffer is marked with FLAG_ERROR if the in-fence signals an error. > + > +To get an out-fence back from V4L2 the ``V4L2_BUF_FLAG_OUT_FENCE`` flag > should > +be set to ask for a fence to be attached to the buffer. The out-fence fd is > +sent to userspace as a ``VIDIOC_QBUF`` return argument on the `fence_fd` > field. > + > +Note the the same `fence_fd` field is used for both sending the in-fence as > +input argument to receive the out-fence as a return argument. > + > +At streamoff the out-fences will either signal normally if the driver waits > +for the operations on the buffers to finish or signal with an error if the > +driver cancels the pending operations. Buffers with in-fences won't be queued > +to the driver if their fences signal. It will be cleaned up. It -> They It should be clarified here if you can or cannot use both in and out fences for the same buffer. > + > +The ``V4L2_FMT_FLAG_UNORDERED`` flag in ``VIDIOC_ENUM_FMT`` tells userspace > +that
Re: [PATCH v5 0/9] Renesas Capture Engine Unit (CEU) V4L2 driver
Hi Jacopo, On Friday, 12 January 2018 16:04:00 EET Jacopo Mondi wrote: > Hello, >(hopefully) last round for CEU driver. > > Changelog is quite thin, I have updated CEU driver MODULE_LICENSE to match > SPDX identifier, added Rob's and Laurent's Reviewed-by tags to bindings, and > made variables of "struct ceu_data" type static in the driver. > > All of the patches are now Reviewed/Acked. Time to have this series > included? Yes please ! Hans, could you pick this up ? > v4->v5: > - Added Rob's and Laurent's Reviewed-by tag to DT bindings > - Change CEU driver module license to "GPL v2" to match SPDX identifier as > suggested by Philippe Ombredanne > - Make struct ceu_data static as suggested by Laurent and add his > Reviewed-by to CEU driver. > > v3->v4: > - Drop generic fallback compatible string "renesas,ceu" > - Addressed Laurent's comments on [3/9] > - Fix error messages on irq get/request > - Do not leak ceudev if irq_get fails > - Make irq_mask a const field > > v2->v3: > - Improved DT bindings removing standard properties (pinctrl- ones and > remote-endpoint) not specific to this driver and improved description of > compatible strings > - Remove ov772x's xlkc_rate property and set clock rate in Migo-R board file > - Made 'xclk' clock private to ov772x driver in Migo-R board file > - Change 'rstb' GPIO active output level and changed ov772x and tw9910 > drivers accordingly as suggested by Fabio > - Minor changes in CEU driver to address Laurent's comments > - Moved Migo-R setup patch to the end of the series to silence 0-day bot > - Renamed tw9910 clock to 'xti' as per video decoder manual > - Changed all SPDX identifiers to GPL-2.0 from previous GPL-2.0+ > > v1->v2: > - DT > -- Addressed Geert's comments and added clocks for CEU to mstp6 clock > source -- Specified supported generic video iterfaces properties in > dt-bindings and simplified example > > - CEU driver > -- Re-worked interrupt handler, interrupt management, reset(*) and capture > start operation > -- Re-worked querycap/enum_input/enum_frameintervals to fix some > v4l2_compliance failures > -- Removed soc_camera legacy operations g/s_mbus_format > -- Update to new notifier implementation > -- Fixed several comments from Hans, Laurent and Sakari > > - Migo-R > -- Register clocks and gpios for sensor drivers in Migo-R setup > -- Updated sensors (tw9910 and ov772x) drivers headers and drivers to close > remarks from Hans and Laurent: > --- Removed platform callbacks and handle clocks and gpios from sensor > drivers --- Remove g/s_mbus_config operations > > Jacopo Mondi (9): > dt-bindings: media: Add Renesas CEU bindings > include: media: Add Renesas CEU driver interface > v4l: platform: Add Renesas CEU driver > ARM: dts: r7s72100: Add Capture Engine Unit (CEU) > v4l: i2c: Copy ov772x soc_camera sensor driver > media: i2c: ov772x: Remove soc_camera dependencies > v4l: i2c: Copy tw9910 soc_camera sensor driver > media: i2c: tw9910: Remove soc_camera dependencies > arch: sh: migor: Use new renesas-ceu camera driver > > .../devicetree/bindings/media/renesas,ceu.txt | 81 + > arch/arm/boot/dts/r7s72100.dtsi| 15 +- > arch/sh/boards/mach-migor/setup.c | 225 ++- > arch/sh/kernel/cpu/sh4a/clock-sh7722.c |2 +- > drivers/media/i2c/Kconfig | 20 + > drivers/media/i2c/Makefile |2 + > drivers/media/i2c/ov772x.c | 1181 ++ > drivers/media/i2c/tw9910.c | 1039 > drivers/media/platform/Kconfig |9 + > drivers/media/platform/Makefile|1 + > drivers/media/platform/renesas-ceu.c | 1648 + > include/media/drv-intf/renesas-ceu.h | 26 + > include/media/i2c/ov772x.h |6 +- > include/media/i2c/tw9910.h |9 + > 14 files changed, 4133 insertions(+), 131 deletions(-) > create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt > create mode 100644 drivers/media/i2c/ov772x.c > create mode 100644 drivers/media/i2c/tw9910.c > create mode 100644 drivers/media/platform/renesas-ceu.c > create mode 100644 include/media/drv-intf/renesas-ceu.h -- Regards, Laurent Pinchart
Re: [PATCH v2 2/2] media: ov9650: add device tree binding
On 01/08/2018 10:35 AM, Sakari Ailus wrote: > I was going to say you're missing the MAINTAINERS entry for this newly > added file but then I noticed that the entire driver is missing an entry. > Still this file should have a MAINTAINERS entry added for it independently > of that, in the same patch. > > Cc Sylwester. I don't the hardware and I can't test the patches so Mita-san if you wish so please add yourself as a maintainer of whole driver. -- Regards, Sylwester
Re: [PATCH v2 1/2] media: ov9650: support device tree probing
On 01/07/2018 05:54 PM, Akinobu Mita wrote: > The ov9650 driver currently only supports legacy platform data probe. > This change adds device tree probing. > Signed-off-by: Akinobu Mita> --- > drivers/media/i2c/ov9650.c | 130 > - > 1 file changed, 92 insertions(+), 38 deletions(-) > > diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c > index 69433e1..99a3eab 100644 > --- a/drivers/media/i2c/ov9650.c > +++ b/drivers/media/i2c/ov9650.c > -static void __ov965x_set_power(struct ov965x *ov965x, int on) > +static int __ov965x_set_power(struct ov965x *ov965x, int on) > { > if (on) { > - ov965x_gpio_set(ov965x->gpios[GPIO_PWDN], 0); > - ov965x_gpio_set(ov965x->gpios[GPIO_RST], 0); > + int ret = clk_prepare_enable(ov965x->clk); It seems you rely on the fact clk_prepare_enable() is a nop when passed argument is NULL, which happens in non-DT case. > + if (ret) > + return ret; > + > + gpiod_set_value_cansleep(ov965x->gpios[GPIO_PWDN], 0); > + gpiod_set_value_cansleep(ov965x->gpios[GPIO_RST], 0); > msleep(25); > } else { > - ov965x_gpio_set(ov965x->gpios[GPIO_RST], 1); > - ov965x_gpio_set(ov965x->gpios[GPIO_PWDN], 1); > + gpiod_set_value_cansleep(ov965x->gpios[GPIO_RST], 1); > + gpiod_set_value_cansleep(ov965x->gpios[GPIO_PWDN], 1); > + > + clk_disable_unprepare(ov965x->clk); > } > > ov965x->streaming = 0; > + > + return 0; > } > @@ -1408,16 +1414,17 @@ static const struct v4l2_subdev_ops ov965x_subdev_ops > = { > /* > * Reset and power down GPIOs configuration > */ > -static int ov965x_configure_gpios(struct ov965x *ov965x, > - const struct ov9650_platform_data *pdata) > +static int ov965x_configure_gpios_pdata(struct ov965x *ov965x, > + const struct ov9650_platform_data *pdata) > { > int ret, i; > + int gpios[NUM_GPIOS]; > > - ov965x->gpios[GPIO_PWDN] = pdata->gpio_pwdn; > - ov965x->gpios[GPIO_RST] = pdata->gpio_reset; > + gpios[GPIO_PWDN] = pdata->gpio_pwdn; > + gpios[GPIO_RST] = pdata->gpio_reset; > > for (i = 0; i < ARRAY_SIZE(ov965x->gpios); i++) { > - int gpio = ov965x->gpios[i]; > + int gpio = gpios[i]; > > if (!gpio_is_valid(gpio)) > continue; > @@ -1427,9 +1434,30 @@ static int ov965x_configure_gpios(struct ov965x > *ov965x, > return ret; > v4l2_dbg(1, debug, >sd, "set gpio %d to 1\n", gpio); > > - gpio_set_value(gpio, 1); > + gpio_set_value_cansleep(gpio, 1); > gpio_export(gpio, 0); > - ov965x->gpios[i] = gpio; > + ov965x->gpios[i] = gpio_to_desc(gpio); > + } > + > + return 0; > +} > + > +static int ov965x_configure_gpios(struct ov965x *ov965x) > +{ > + struct device *dev = >client->dev; > + > + ov965x->gpios[GPIO_PWDN] = devm_gpiod_get_optional(dev, "powerdown", > + GPIOD_OUT_HIGH); > + if (IS_ERR(ov965x->gpios[GPIO_PWDN])) { > + dev_info(dev, "can't get %s GPIO\n", "powerdown"); > + return PTR_ERR(ov965x->gpios[GPIO_PWDN]); > + } > + > + ov965x->gpios[GPIO_RST] = devm_gpiod_get_optional(dev, "reset", > + GPIOD_OUT_HIGH); > + if (IS_ERR(ov965x->gpios[GPIO_RST])) { > + dev_info(dev, "can't get %s GPIO\n", "reset"); > + return PTR_ERR(ov965x->gpios[GPIO_RST]); > } > > return 0; > @@ -1443,7 +1471,10 @@ static int ov965x_detect_sensor(struct v4l2_subdev *sd) > int ret; > > mutex_lock(>lock); > - __ov965x_set_power(ov965x, 1); > + ret = __ov965x_set_power(ov965x, 1); > + if (ret) > + goto out; > + > msleep(25); > > /* Check sensor revision */ > @@ -1463,6 +1494,7 @@ static int ov965x_detect_sensor(struct v4l2_subdev *sd) > ret = -ENODEV; > } > } > +out: > mutex_unlock(>lock); > > return ret; > @@ -1476,23 +1508,39 @@ static int ov965x_probe(struct i2c_client *client, > struct ov965x *ov965x; > int ret; > > - if (!pdata) { > - dev_err(>dev, "platform data not specified\n"); > - return -EINVAL; > - } > - > - if (pdata->mclk_frequency == 0) { > - dev_err(>dev, "MCLK frequency not specified\n"); > - return -EINVAL; > - } > - > ov965x = devm_kzalloc(>dev, sizeof(*ov965x), GFP_KERNEL); > if (!ov965x) > return -ENOMEM; > > - mutex_init(>lock); I would leave mutex initialization as first thing after the private data structure allocation, is there a need to
Re: [PATCH v2] MAINTAINERS: mtd/nand: update Microchip nand entry
On Thu, 11 Jan 2018 17:26:59 +0100 Nicolas Ferrewrote: > Update Wenyou Yang email address. > Take advantage of this update to move this entry to the MICROCHIP / ATMEL > location and add the DT binding documentation link. > > Signed-off-by: Nicolas Ferre > Acked-by: Wenyou Yang Applied. Thanks, Boris > --- > v2: - patch agains 09ec417b0ea8 ("mtd: nand: samsung: Disable subpage > writes on E-die NAND") of > http://git.infradead.org/linux-mtd.git/shortlog/refs/heads/nand/next > - Ack by Wenyou added > > > Hi, > > v1 patch was part of a series because it was conflicting with the previous one > named: > "[PATCH 1/2] MAINTAINERS: linux-media: update Microchip ISI and ISC entries" > Boris asked me to rebase it so that they are independent. > So, if this first one goes upstream through another tree, conflicts will have > to be resolved at one point. > > Best regards, > Nicolas > > > MAINTAINERS | 15 --- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index aa71ab52fd76..37ee5ae4bae2 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2382,13 +2382,6 @@ F: > Documentation/devicetree/bindings/input/atmel,maxtouch.txt > F: drivers/input/touchscreen/atmel_mxt_ts.c > F: include/linux/platform_data/atmel_mxt_ts.h > > -ATMEL NAND DRIVER > -M: Wenyou Yang > -M: Josh Wu > -L: linux-...@lists.infradead.org > -S: Supported > -F: drivers/mtd/nand/atmel/* > - > ATMEL SAMA5D2 ADC DRIVER > M: Ludovic Desroches > L: linux-...@vger.kernel.org > @@ -9045,6 +9038,14 @@ F: drivers/media/platform/atmel/atmel-isc.c > F: drivers/media/platform/atmel/atmel-isc-regs.h > F: devicetree/bindings/media/atmel-isc.txt > > +MICROCHIP / ATMEL NAND DRIVER > +M: Wenyou Yang > +M: Josh Wu > +L: linux-...@lists.infradead.org > +S: Supported > +F: drivers/mtd/nand/atmel/* > +F: Documentation/devicetree/bindings/mtd/atmel-nand.txt > + > MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER > M: Woojung Huh > M: Microchip Linux Driver Support
[PATCH v5 0/9] Renesas Capture Engine Unit (CEU) V4L2 driver
Hello, (hopefully) last round for CEU driver. Changelog is quite thin, I have updated CEU driver MODULE_LICENSE to match SPDX identifier, added Rob's and Laurent's Reviewed-by tags to bindings, and made variables of "struct ceu_data" type static in the driver. All of the patches are now Reviewed/Acked. Time to have this series included? Thanks j v4->v5: - Added Rob's and Laurent's Reviewed-by tag to DT bindings - Change CEU driver module license to "GPL v2" to match SPDX identifier as suggested by Philippe Ombredanne - Make struct ceu_data static as suggested by Laurent and add his Reviewed-by to CEU driver. v3->v4: - Drop generic fallback compatible string "renesas,ceu" - Addressed Laurent's comments on [3/9] - Fix error messages on irq get/request - Do not leak ceudev if irq_get fails - Make irq_mask a const field v2->v3: - Improved DT bindings removing standard properties (pinctrl- ones and remote-endpoint) not specific to this driver and improved description of compatible strings - Remove ov772x's xlkc_rate property and set clock rate in Migo-R board file - Made 'xclk' clock private to ov772x driver in Migo-R board file - Change 'rstb' GPIO active output level and changed ov772x and tw9910 drivers accordingly as suggested by Fabio - Minor changes in CEU driver to address Laurent's comments - Moved Migo-R setup patch to the end of the series to silence 0-day bot - Renamed tw9910 clock to 'xti' as per video decoder manual - Changed all SPDX identifiers to GPL-2.0 from previous GPL-2.0+ v1->v2: - DT -- Addressed Geert's comments and added clocks for CEU to mstp6 clock source -- Specified supported generic video iterfaces properties in dt-bindings and simplified example - CEU driver -- Re-worked interrupt handler, interrupt management, reset(*) and capture start operation -- Re-worked querycap/enum_input/enum_frameintervals to fix some v4l2_compliance failures -- Removed soc_camera legacy operations g/s_mbus_format -- Update to new notifier implementation -- Fixed several comments from Hans, Laurent and Sakari - Migo-R -- Register clocks and gpios for sensor drivers in Migo-R setup -- Updated sensors (tw9910 and ov772x) drivers headers and drivers to close remarks from Hans and Laurent: --- Removed platform callbacks and handle clocks and gpios from sensor drivers --- Remove g/s_mbus_config operations Jacopo Mondi (9): dt-bindings: media: Add Renesas CEU bindings include: media: Add Renesas CEU driver interface v4l: platform: Add Renesas CEU driver ARM: dts: r7s72100: Add Capture Engine Unit (CEU) v4l: i2c: Copy ov772x soc_camera sensor driver media: i2c: ov772x: Remove soc_camera dependencies v4l: i2c: Copy tw9910 soc_camera sensor driver media: i2c: tw9910: Remove soc_camera dependencies arch: sh: migor: Use new renesas-ceu camera driver .../devicetree/bindings/media/renesas,ceu.txt | 81 + arch/arm/boot/dts/r7s72100.dtsi| 15 +- arch/sh/boards/mach-migor/setup.c | 225 ++- arch/sh/kernel/cpu/sh4a/clock-sh7722.c |2 +- drivers/media/i2c/Kconfig | 20 + drivers/media/i2c/Makefile |2 + drivers/media/i2c/ov772x.c | 1181 ++ drivers/media/i2c/tw9910.c | 1039 drivers/media/platform/Kconfig |9 + drivers/media/platform/Makefile|1 + drivers/media/platform/renesas-ceu.c | 1648 include/media/drv-intf/renesas-ceu.h | 26 + include/media/i2c/ov772x.h |6 +- include/media/i2c/tw9910.h |9 + 14 files changed, 4133 insertions(+), 131 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt create mode 100644 drivers/media/i2c/ov772x.c create mode 100644 drivers/media/i2c/tw9910.c create mode 100644 drivers/media/platform/renesas-ceu.c create mode 100644 include/media/drv-intf/renesas-ceu.h -- 2.7.4
[PATCH v5 1/9] dt-bindings: media: Add Renesas CEU bindings
Add bindings documentation for Renesas Capture Engine Unit (CEU). Signed-off-by: Jacopo MondiReviewed-by: Rob Herring Reviewed-by: Laurent Pinchart --- .../devicetree/bindings/media/renesas,ceu.txt | 81 ++ 1 file changed, 81 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt b/Documentation/devicetree/bindings/media/renesas,ceu.txt new file mode 100644 index 000..590ee27 --- /dev/null +++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt @@ -0,0 +1,81 @@ +Renesas Capture Engine Unit (CEU) +-- + +The Capture Engine Unit is the image capture interface found in the Renesas +SH Mobile and RZ SoCs. + +The interface supports a single parallel input with data bus width of 8 or 16 +bits. + +Required properties: +- compatible: Shall be "renesas,r7s72100-ceu" for CEU units found in RZ/A1-H + and RZ/A1-M SoCs. +- reg: Registers address base and size. +- interrupts: The interrupt specifier. + +The CEU supports a single parallel input and should contain a single 'port' +subnode with a single 'endpoint'. Connection to input devices are modeled +according to the video interfaces OF bindings specified in: +Documentation/devicetree/bindings/media/video-interfaces.txt + +Optional endpoint properties applicable to parallel input bus described in +the above mentioned "video-interfaces.txt" file are supported. + +- hsync-active: Active state of the HSYNC signal, 0/1 for LOW/HIGH respectively. + If property is not present, default is active high. +- vsync-active: Active state of the VSYNC signal, 0/1 for LOW/HIGH respectively. + If property is not present, default is active high. + +Example: + +The example describes the connection between the Capture Engine Unit and an +OV7670 image sensor connected to i2c1 interface. + +ceu: ceu@e821 { + reg = <0xe821 0x209c>; + compatible = "renesas,r7s72100-ceu"; + interrupts = ; + + pinctrl-names = "default"; + pinctrl-0 = <_pins>; + + status = "okay"; + + port { + ceu_in: endpoint { + remote-endpoint = <_out>; + + hsync-active = <1>; + vsync-active = <0>; + }; + }; +}; + +i2c1: i2c@fcfee400 { + pinctrl-names = "default"; + pinctrl-0 = <_pins>; + + status = "okay"; + + clock-frequency = <10>; + + ov7670: camera@21 { + compatible = "ovti,ov7670"; + reg = <0x21>; + + pinctrl-names = "default"; + pinctrl-0 = <_pins>; + + reset-gpios = < 11 GPIO_ACTIVE_LOW>; + powerdown-gpios = < 12 GPIO_ACTIVE_HIGH>; + + port { + ov7670_out: endpoint { + remote-endpoint = <_in>; + + hsync-active = <1>; + vsync-active = <0>; + }; + }; + }; +}; -- 2.7.4
[PATCH v5 2/9] include: media: Add Renesas CEU driver interface
Add renesas-ceu header file. Do not remove the existing sh_mobile_ceu.h one as long as the original driver does not go away. Signed-off-by: Jacopo MondiReviewed-by: Laurent Pinchart --- include/media/drv-intf/renesas-ceu.h | 26 ++ 1 file changed, 26 insertions(+) create mode 100644 include/media/drv-intf/renesas-ceu.h diff --git a/include/media/drv-intf/renesas-ceu.h b/include/media/drv-intf/renesas-ceu.h new file mode 100644 index 000..52841d1 --- /dev/null +++ b/include/media/drv-intf/renesas-ceu.h @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * renesas-ceu.h - Renesas CEU driver interface + * + * Copyright 2017-2018 Jacopo Mondi + */ + +#ifndef __MEDIA_DRV_INTF_RENESAS_CEU_H__ +#define __MEDIA_DRV_INTF_RENESAS_CEU_H__ + +#define CEU_MAX_SUBDEVS2 + +struct ceu_async_subdev { + unsigned long flags; + unsigned char bus_width; + unsigned char bus_shift; + unsigned int i2c_adapter_id; + unsigned int i2c_address; +}; + +struct ceu_platform_data { + unsigned int num_subdevs; + struct ceu_async_subdev subdevs[CEU_MAX_SUBDEVS]; +}; + +#endif /* ___MEDIA_DRV_INTF_RENESAS_CEU_H__ */ -- 2.7.4
[PATCH v5 4/9] ARM: dts: r7s72100: Add Capture Engine Unit (CEU)
Add Capture Engine Unit (CEU) node to device tree. Signed-off-by: Jacopo MondiReviewed-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart --- arch/arm/boot/dts/r7s72100.dtsi | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi index ab9645a..5fe62f9 100644 --- a/arch/arm/boot/dts/r7s72100.dtsi +++ b/arch/arm/boot/dts/r7s72100.dtsi @@ -135,9 +135,9 @@ #clock-cells = <1>; compatible = "renesas,r7s72100-mstp-clocks", "renesas,cpg-mstp-clocks"; reg = <0xfcfe042c 4>; - clocks = <_clk>; - clock-indices = ; - clock-output-names = "rtc"; + clocks = <_clk>, <_clk>; + clock-indices = ; + clock-output-names = "ceu", "rtc"; }; mstp7_clks: mstp7_clks@fcfe0430 { @@ -667,4 +667,13 @@ power-domains = <_clocks>; status = "disabled"; }; + + ceu: ceu@e821 { + reg = <0xe821 0x3000>; + compatible = "renesas,r7s72100-ceu"; + interrupts = ; + clocks = <_clks R7S72100_CLK_CEU>; + power-domains = <_clocks>; + status = "disabled"; + }; }; -- 2.7.4
[PATCH v5 3/9] v4l: platform: Add Renesas CEU driver
Add driver for Renesas Capture Engine Unit (CEU). The CEU interface supports capturing 'data' (YUV422) and 'images' (NV[12|21|16|61]). This driver aims to replace the soc_camera-based sh_mobile_ceu one. Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ platform GR-Peach. Tested with ov7725 camera sensor on SH4 platform Migo-R. Signed-off-by: Jacopo MondiReviewed-by: Laurent Pinchart --- drivers/media/platform/Kconfig |9 + drivers/media/platform/Makefile |1 + drivers/media/platform/renesas-ceu.c | 1648 ++ 3 files changed, 1658 insertions(+) create mode 100644 drivers/media/platform/renesas-ceu.c diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index fd0c998..fe7bd26 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -144,6 +144,15 @@ config VIDEO_STM32_DCMI To compile this driver as a module, choose M here: the module will be called stm32-dcmi. +config VIDEO_RENESAS_CEU + tristate "Renesas Capture Engine Unit (CEU) driver" + depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA + depends on ARCH_SHMOBILE || ARCH_R7S72100 || COMPILE_TEST + select VIDEOBUF2_DMA_CONTIG + select V4L2_FWNODE + ---help--- + This is a v4l2 driver for the Renesas CEU Interface + source "drivers/media/platform/soc_camera/Kconfig" source "drivers/media/platform/exynos4-is/Kconfig" source "drivers/media/platform/am437x/Kconfig" diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index 003b0bb..6580a6b 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_SH_VOU)+= sh_vou.o obj-$(CONFIG_SOC_CAMERA) += soc_camera/ obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o +obj-$(CONFIG_VIDEO_RENESAS_CEU)+= renesas-ceu.o obj-$(CONFIG_VIDEO_RENESAS_FCP)+= rcar-fcp.o obj-$(CONFIG_VIDEO_RENESAS_FDP1) += rcar_fdp1.o obj-$(CONFIG_VIDEO_RENESAS_JPU)+= rcar_jpu.o diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c new file mode 100644 index 000..ccca838 --- /dev/null +++ b/drivers/media/platform/renesas-ceu.c @@ -0,0 +1,1648 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * V4L2 Driver for Renesas Capture Engine Unit (CEU) interface + * Copyright (C) 2017-2018 Jacopo Mondi + * + * Based on soc-camera driver "soc_camera/sh_mobile_ceu_camera.c" + * Copyright (C) 2008 Magnus Damm + * + * Based on V4L2 Driver for PXA camera host - "pxa_camera.c", + * Copyright (C) 2006, Sascha Hauer, Pengutronix + * Copyright (C) 2008, Guennadi Liakhovetski + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define DRIVER_NAME"renesas-ceu" + +/* CEU registers offsets and masks. */ +#define CEU_CAPSR 0x00 /* Capture start register */ +#define CEU_CAPCR 0x04 /* Capture control register*/ +#define CEU_CAMCR 0x08 /* Capture interface control register */ +#define CEU_CAMOR 0x10 /* Capture interface offset register */ +#define CEU_CAPWR 0x14 /* Capture interface width register*/ +#define CEU_CAIFR 0x18 /* Capture interface input format register */ +#define CEU_CRCNTR 0x28 /* CEU register control register */ +#define CEU_CRCMPR 0x2c /* CEU register forcible control register */ +#define CEU_CFLCR 0x30 /* Capture filter control register */ +#define CEU_CFSZR 0x34 /* Capture filter size clip register */ +#define CEU_CDWDR 0x38 /* Capture destination width register */ +#define CEU_CDAYR 0x3c /* Capture data address Y register */ +#define CEU_CDACR 0x40 /* Capture data address C register */ +#define CEU_CFWCR 0x5c /* Firewall operation control register */ +#define CEU_CDOCR 0x64 /* Capture data output control register*/ +#define CEU_CEIER 0x70 /* Capture event interrupt enable register */ +#define CEU_CETCR 0x74 /* Capture event flag clear register */ +#define CEU_CSTSR 0x7c /* Capture status register */ +#define CEU_CSRTR 0x80 /* Capture software reset register */ + +/* Data synchronous fetch mode. */ +#define CEU_CAMCR_JPEG BIT(4) + +/* Input components ordering: CEU_CAMCR.DTARY field. */ +#define CEU_CAMCR_DTARY_8_UYVY (0x00 << 8) +#define CEU_CAMCR_DTARY_8_VYUY (0x01 << 8) +#define CEU_CAMCR_DTARY_8_YUYV (0x02 << 8) +#define
[PATCH v5 6/9] media: i2c: ov772x: Remove soc_camera dependencies
Remove soc_camera framework dependencies from ov772x sensor driver. - Handle clock and gpios - Register async subdevice - Remove soc_camera specific g/s_mbus_config operations - Change image format colorspace from JPEG to SRGB as the two use the same colorspace information but JPEG makes assumptions on color components quantization that do not apply to the sensor - Remove sizes crop from get_selection as driver can't scale - Add kernel doc to driver interface header file - Adjust build system This commit does not remove the original soc_camera based driver as long as other platforms depends on soc_camera-based CEU driver. Signed-off-by: Jacopo MondiReviewed-by: Laurent Pinchart --- drivers/media/i2c/Kconfig | 11 +++ drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ov772x.c | 177 ++--- include/media/i2c/ov772x.h | 6 +- 4 files changed, 133 insertions(+), 62 deletions(-) diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index cb5d7ff..a61d7f4 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -645,6 +645,17 @@ config VIDEO_OV5670 To compile this driver as a module, choose M here: the module will be called ov5670. +config VIDEO_OV772X + tristate "OmniVision OV772x sensor support" + depends on I2C && VIDEO_V4L2 + depends on MEDIA_CAMERA_SUPPORT + ---help--- + This is a Video4Linux2 sensor-level driver for the OmniVision + OV772x camera. + + To compile this driver as a module, choose M here: the + module will be called ov772x. + config VIDEO_OV7640 tristate "OmniVision OV7640 sensor support" depends on I2C && VIDEO_V4L2 diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 548a9ef..fb99293 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -66,6 +66,7 @@ obj-$(CONFIG_VIDEO_OV5645) += ov5645.o obj-$(CONFIG_VIDEO_OV5647) += ov5647.o obj-$(CONFIG_VIDEO_OV5670) += ov5670.o obj-$(CONFIG_VIDEO_OV6650) += ov6650.o +obj-$(CONFIG_VIDEO_OV772X) += ov772x.o obj-$(CONFIG_VIDEO_OV7640) += ov7640.o obj-$(CONFIG_VIDEO_OV7670) += ov7670.o obj-$(CONFIG_VIDEO_OV9650) += ov9650.o diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c index 8063835..df2516c 100644 --- a/drivers/media/i2c/ov772x.c +++ b/drivers/media/i2c/ov772x.c @@ -1,6 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /* * ov772x Camera Driver * + * Copyright (C) 2017 Jacopo Mondi + * * Copyright (C) 2008 Renesas Solutions Corp. * Kuninori Morimoto * @@ -9,27 +12,25 @@ * Copyright 2006-7 Jonathan Corbet * Copyright (C) 2008 Magnus Damm * Copyright (C) 2008, Guennadi Liakhovetski - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ +#include +#include +#include +#include #include #include #include -#include #include -#include #include #include #include -#include -#include + #include -#include +#include #include +#include /* * register offset @@ -393,8 +394,10 @@ struct ov772x_win_size { struct ov772x_priv { struct v4l2_subdevsubdev; struct v4l2_ctrl_handler hdl; - struct v4l2_clk *clk; + struct clk *clk; struct ov772x_camera_info*info; + struct gpio_desc *pwdn_gpio; + struct gpio_desc *rstb_gpio; const struct ov772x_color_format *cfmt; const struct ov772x_win_size *win; unsigned shortflag_vflip:1; @@ -409,7 +412,7 @@ struct ov772x_priv { static const struct ov772x_color_format ov772x_cfmts[] = { { .code = MEDIA_BUS_FMT_YUYV8_2X8, - .colorspace = V4L2_COLORSPACE_JPEG, + .colorspace = V4L2_COLORSPACE_SRGB, .dsp3 = 0x0, .dsp4 = DSP_OFMT_YUV, .com3 = SWAP_YUV, @@ -417,7 +420,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = { }, { .code = MEDIA_BUS_FMT_YVYU8_2X8, - .colorspace = V4L2_COLORSPACE_JPEG, + .colorspace = V4L2_COLORSPACE_SRGB, .dsp3 = UV_ON, .dsp4 = DSP_OFMT_YUV, .com3 = SWAP_YUV, @@ -425,7 +428,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = { }, { .code = MEDIA_BUS_FMT_UYVY8_2X8, - .colorspace = V4L2_COLORSPACE_JPEG, + .colorspace =
[PATCH v5 5/9] v4l: i2c: Copy ov772x soc_camera sensor driver
Copy the soc_camera based driver in v4l2 sensor driver directory. This commit just copies the original file without modifying it. No modification to KConfig and Makefile as soc_camera framework dependencies need to be removed first in next commit. Signed-off-by: Jacopo MondiAcked-by: Laurent Pinchart --- drivers/media/i2c/ov772x.c | 1124 1 file changed, 1124 insertions(+) create mode 100644 drivers/media/i2c/ov772x.c diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c new file mode 100644 index 000..8063835 --- /dev/null +++ b/drivers/media/i2c/ov772x.c @@ -0,0 +1,1124 @@ +/* + * ov772x Camera Driver + * + * Copyright (C) 2008 Renesas Solutions Corp. + * Kuninori Morimoto + * + * Based on ov7670 and soc_camera_platform driver, + * + * Copyright 2006-7 Jonathan Corbet + * Copyright (C) 2008 Magnus Damm + * Copyright (C) 2008, Guennadi Liakhovetski + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +/* + * register offset + */ +#define GAIN0x00 /* AGC - Gain control gain setting */ +#define BLUE0x01 /* AWB - Blue channel gain setting */ +#define RED 0x02 /* AWB - Red channel gain setting */ +#define GREEN 0x03 /* AWB - Green channel gain setting */ +#define COM10x04 /* Common control 1 */ +#define BAVG0x05 /* U/B Average Level */ +#define GAVG0x06 /* Y/Gb Average Level */ +#define RAVG0x07 /* V/R Average Level */ +#define AECH0x08 /* Exposure Value - AEC MSBs */ +#define COM20x09 /* Common control 2 */ +#define PID 0x0A /* Product ID Number MSB */ +#define VER 0x0B /* Product ID Number LSB */ +#define COM30x0C /* Common control 3 */ +#define COM40x0D /* Common control 4 */ +#define COM50x0E /* Common control 5 */ +#define COM60x0F /* Common control 6 */ +#define AEC 0x10 /* Exposure Value */ +#define CLKRC 0x11 /* Internal clock */ +#define COM70x12 /* Common control 7 */ +#define COM80x13 /* Common control 8 */ +#define COM90x14 /* Common control 9 */ +#define COM10 0x15 /* Common control 10 */ +#define REG16 0x16 /* Register 16 */ +#define HSTART 0x17 /* Horizontal sensor size */ +#define HSIZE 0x18 /* Horizontal frame (HREF column) end high 8-bit */ +#define VSTART 0x19 /* Vertical frame (row) start high 8-bit */ +#define VSIZE 0x1A /* Vertical sensor size */ +#define PSHFT 0x1B /* Data format - pixel delay select */ +#define MIDH0x1C /* Manufacturer ID byte - high */ +#define MIDL0x1D /* Manufacturer ID byte - low */ +#define LAEC0x1F /* Fine AEC value */ +#define COM11 0x20 /* Common control 11 */ +#define BDBASE 0x22 /* Banding filter Minimum AEC value */ +#define DBSTEP 0x23 /* Banding filter Maximum Setp */ +#define AEW 0x24 /* AGC/AEC - Stable operating region (upper limit) */ +#define AEB 0x25 /* AGC/AEC - Stable operating region (lower limit) */ +#define VPT 0x26 /* AGC/AEC Fast mode operating region */ +#define REG28 0x28 /* Register 28 */ +#define HOUTSIZE0x29 /* Horizontal data output size MSBs */ +#define EXHCH 0x2A /* Dummy pixel insert MSB */ +#define EXHCL 0x2B /* Dummy pixel insert LSB */ +#define VOUTSIZE0x2C /* Vertical data output size MSBs */ +#define ADVFL 0x2D /* LSB of insert dummy lines in Vertical direction */ +#define ADVFH 0x2E /* MSG of insert dummy lines in Vertical direction */ +#define YAVE0x2F /* Y/G Channel Average value */ +#define LUMHTH 0x30 /* Histogram AEC/AGC Luminance high level threshold */ +#define LUMLTH 0x31 /* Histogram AEC/AGC Luminance low level threshold */ +#define HREF0x32 /* Image start and size control */ +#define DM_LNL 0x33 /* Dummy line low 8 bits */ +#define DM_LNH 0x34 /* Dummy line high 8 bits */ +#define ADOFF_B 0x35 /* AD offset compensation value for B channel */ +#define ADOFF_R 0x36 /* AD offset compensation value for R channel */ +#define ADOFF_GB0x37 /* AD offset compensation value for Gb channel */ +#define ADOFF_GR0x38 /* AD offset compensation value for Gr channel */ +#define OFF_B 0x39 /* Analog process B channel offset value */ +#define OFF_R 0x3A /* Analog process R channel offset value */ +#define OFF_GB 0x3B /* Analog process Gb channel offset value */ +#define OFF_GR 0x3C /* Analog process Gr channel offset value */ +#define COM12 0x3D
Re: [PATCH v7 5/6] [media] vb2: add out-fence support to QBUF
On 01/10/18 17:07, Gustavo Padovan wrote: > From: Gustavo Padovan> > If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create > an out_fence and send its fd to userspace on the fence_fd field as a > return arg for the QBUF call. > > The fence is signaled on buffer_done(), when the job on the buffer is > finished. > > v8: > - return 0 as fence_fd if OUT_FENCE flag not used (Mauro) > - fix crash when checking not using fences in vb2_buffer_done() > > v7: > - merge patch that add the infrastructure to out-fences into > this one (Alex Courbot) > - Do not install the fd if there is no fence. (Alex Courbot) > - do not report error on requeueing, just WARN_ON_ONCE() (Hans) > > v6 > - get rid of the V4L2_EVENT_OUT_FENCE event. We always keep the > ordering in vb2 for queueing in the driver, so the event is not > necessary anymore and the out_fence_fd is sent back to userspace > on QBUF call return arg > - do not allow requeueing with out-fences, instead mark the buffer > with an error and wake up to userspace. > - send the out_fence_fd back to userspace on the fence_fd field > > v5: > - delay fd_install to DQ_EVENT (Hans) > - if queue is fully ordered send OUT_FENCE event right away > (Brian) > - rename 'q->ordered' to 'q->ordered_in_driver' > - merge change to implement OUT_FENCE event here > > v4: > - return the out_fence_fd in the BUF_QUEUED event(Hans) > > v3: - add WARN_ON_ONCE(q->ordered) on requeueing (Hans) > - set the OUT_FENCE flag if there is a fence pending (Hans) > - call fd_install() after vb2_core_qbuf() (Hans) > - clean up fence if vb2_core_qbuf() fails (Hans) > - add list to store sync_file and fence for the next queued buffer > > v2: check if the queue is ordered. > > Signed-off-by: Gustavo Padovan > --- > drivers/media/common/videobuf/videobuf2-core.c | 101 > +++-- > drivers/media/common/videobuf/videobuf2-v4l2.c | 28 ++- > include/media/videobuf2-core.h | 22 ++ > 3 files changed, 140 insertions(+), 11 deletions(-) > > diff --git a/drivers/media/common/videobuf/videobuf2-core.c > b/drivers/media/common/videobuf/videobuf2-core.c > index 777e3a2bc746..1f30d9efb7c8 100644 > --- a/drivers/media/common/videobuf/videobuf2-core.c > +++ b/drivers/media/common/videobuf/videobuf2-core.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -357,6 +358,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum > vb2_memory memory, > vb->planes[plane].length = plane_sizes[plane]; > vb->planes[plane].min_length = plane_sizes[plane]; > } > + vb->out_fence_fd = -1; > q->bufs[vb->index] = vb; > > /* Allocate video buffer memory for the MMAP type */ > @@ -939,10 +941,22 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum > vb2_buffer_state state) > case VB2_BUF_STATE_QUEUED: > break; > case VB2_BUF_STATE_REQUEUEING: > + /* Requeuing with explicit synchronization, spit warning */ > + WARN_ON_ONCE(vb->out_fence); > + > if (q->start_streaming_called) > __enqueue_in_driver(vb); > - return; > + break; > default: > + if (vb->out_fence) { > + if (state == VB2_BUF_STATE_ERROR) > + dma_fence_set_error(vb->out_fence, -EFAULT); > + dma_fence_signal(vb->out_fence); > + dma_fence_put(vb->out_fence); > + vb->out_fence = NULL; > + vb->out_fence_fd = -1; > + } > + > /* Inform any processes that may be waiting for buffers */ > wake_up(>done_wq); > break; > @@ -1341,6 +1355,65 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned > int index, void *pb) > } > EXPORT_SYMBOL_GPL(vb2_core_prepare_buf); > > +static inline const char *vb2_fence_get_driver_name(struct dma_fence *fence) > +{ > + return "vb2_fence"; > +} > + > +static inline const char *vb2_fence_get_timeline_name(struct dma_fence > *fence) > +{ > + return "vb2_fence_timeline"; > +} > + > +static inline bool vb2_fence_enable_signaling(struct dma_fence *fence) > +{ > + return true; > +} > + > +static const struct dma_fence_ops vb2_fence_ops = { > + .get_driver_name = vb2_fence_get_driver_name, > + .get_timeline_name = vb2_fence_get_timeline_name, > + .enable_signaling = vb2_fence_enable_signaling, > + .wait = dma_fence_default_wait, > +}; > + > +int vb2_setup_out_fence(struct vb2_queue *q, unsigned int index) > +{ > + struct vb2_buffer *vb; > + u64 context; > + > + vb = q->bufs[index]; > +
[PATCH v5 7/9] v4l: i2c: Copy tw9910 soc_camera sensor driver
Copy the soc_camera based driver in v4l2 sensor driver directory. This commit just copies the original file without modifying it. No modification to KConfig and Makefile as soc_camera framework dependencies need to be removed first in next commit. Signed-off-by: Jacopo MondiAcked-by: Laurent Pinchart --- drivers/media/i2c/tw9910.c | 999 + 1 file changed, 999 insertions(+) create mode 100644 drivers/media/i2c/tw9910.c diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c new file mode 100644 index 000..bdb5e0a --- /dev/null +++ b/drivers/media/i2c/tw9910.c @@ -0,0 +1,999 @@ +/* + * tw9910 Video Driver + * + * Copyright (C) 2008 Renesas Solutions Corp. + * Kuninori Morimoto + * + * Based on ov772x driver, + * + * Copyright (C) 2008 Kuninori Morimoto + * Copyright 2006-7 Jonathan Corbet + * Copyright (C) 2008 Magnus Damm + * Copyright (C) 2008, Guennadi Liakhovetski + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#define GET_ID(val) ((val & 0xF8) >> 3) +#define GET_REV(val) (val & 0x07) + +/* + * register offset + */ +#define ID 0x00 /* Product ID Code Register */ +#define STATUS10x01 /* Chip Status Register I */ +#define INFORM 0x02 /* Input Format */ +#define OPFORM 0x03 /* Output Format Control Register */ +#define DLYCTR 0x04 /* Hysteresis and HSYNC Delay Control */ +#define OUTCTR10x05 /* Output Control I */ +#define ACNTL1 0x06 /* Analog Control Register 1 */ +#define CROP_HI0x07 /* Cropping Register, High */ +#define VDELAY_LO 0x08 /* Vertical Delay Register, Low */ +#define VACTIVE_LO 0x09 /* Vertical Active Register, Low */ +#define HDELAY_LO 0x0A /* Horizontal Delay Register, Low */ +#define HACTIVE_LO 0x0B /* Horizontal Active Register, Low */ +#define CNTRL1 0x0C /* Control Register I */ +#define VSCALE_LO 0x0D /* Vertical Scaling Register, Low */ +#define SCALE_HI 0x0E /* Scaling Register, High */ +#define HSCALE_LO 0x0F /* Horizontal Scaling Register, Low */ +#define BRIGHT 0x10 /* BRIGHTNESS Control Register */ +#define CONTRAST 0x11 /* CONTRAST Control Register */ +#define SHARPNESS 0x12 /* SHARPNESS Control Register I */ +#define SAT_U 0x13 /* Chroma (U) Gain Register */ +#define SAT_V 0x14 /* Chroma (V) Gain Register */ +#define HUE0x15 /* Hue Control Register */ +#define CORING10x17 +#define CORING20x18 /* Coring and IF compensation */ +#define VBICNTL0x19 /* VBI Control Register */ +#define ACNTL2 0x1A /* Analog Control 2 */ +#define OUTCTR20x1B /* Output Control 2 */ +#define SDT0x1C /* Standard Selection */ +#define SDTR 0x1D /* Standard Recognition */ +#define TEST 0x1F /* Test Control Register */ +#define CLMPG 0x20 /* Clamping Gain */ +#define IAGC 0x21 /* Individual AGC Gain */ +#define AGCGAIN0x22 /* AGC Gain */ +#define PEAKWT 0x23 /* White Peak Threshold */ +#define CLMPL 0x24 /* Clamp level */ +#define SYNCT 0x25 /* Sync Amplitude */ +#define MISSCNT0x26 /* Sync Miss Count Register */ +#define PCLAMP 0x27 /* Clamp Position Register */ +#define VCNTL1 0x28 /* Vertical Control I */ +#define VCNTL2 0x29 /* Vertical Control II */ +#define CKILL 0x2A /* Color Killer Level Control */ +#define COMB 0x2B /* Comb Filter Control */ +#define LDLY 0x2C /* Luma Delay and H Filter Control */ +#define MISC1 0x2D /* Miscellaneous Control I */ +#define LOOP 0x2E /* LOOP Control Register */ +#define MISC2 0x2F /* Miscellaneous Control II */ +#define MVSN 0x30 /* Macrovision Detection */ +#define STATUS20x31 /* Chip STATUS II */ +#define HFREF 0x32 /* H monitor */ +#define CLMD 0x33 /* CLAMP MODE */ +#define IDCNTL 0x34 /* ID Detection Control */ +#define CLCNTL10x35 /* Clamp Control I */ +#define ANAPLLCTL 0x4C +#define VBIMIN 0x4D +#define HSLOWCTL 0x4E +#define WSS3 0x4F +#define FILLDATA 0x50 +#define SDID 0x51 +#define DID0x52 +#define WSS1 0x53 +#define WSS2 0x54 +#define VVBI 0x55 +#define LCTL6 0x56 +#define LCTL7 0x57 +#define LCTL8 0x58 +#define LCTL9
[PATCH v5 8/9] media: i2c: tw9910: Remove soc_camera dependencies
Remove soc_camera framework dependencies from tw9910 sensor driver. - Handle clock and gpios - Register async subdevice - Remove soc_camera specific g/s_mbus_config operations - Add kernel doc to driver interface header file - Adjust build system This commit does not remove the original soc_camera based driver as long as other platforms depends on soc_camera-based CEU driver. Signed-off-by: Jacopo MondiReviewed-by: Laurent Pinchart --- drivers/media/i2c/Kconfig | 9 +++ drivers/media/i2c/Makefile | 1 + drivers/media/i2c/tw9910.c | 162 - include/media/i2c/tw9910.h | 9 +++ 4 files changed, 120 insertions(+), 61 deletions(-) diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index a61d7f4..804a1bf 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -423,6 +423,15 @@ config VIDEO_TW9906 To compile this driver as a module, choose M here: the module will be called tw9906. +config VIDEO_TW9910 + tristate "Techwell TW9910 video decoder" + depends on VIDEO_V4L2 && I2C + ---help--- + Support for Techwell TW9910 NTSC/PAL/SECAM video decoder. + + To compile this driver as a module, choose M here: the + module will be called tw9910. + config VIDEO_VPX3220 tristate "vpx3220a, vpx3216b & vpx3214c video decoders" depends on VIDEO_V4L2 && I2C diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index fb99293..e26544f 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -48,6 +48,7 @@ obj-$(CONFIG_VIDEO_TVP7002) += tvp7002.o obj-$(CONFIG_VIDEO_TW2804) += tw2804.o obj-$(CONFIG_VIDEO_TW9903) += tw9903.o obj-$(CONFIG_VIDEO_TW9906) += tw9906.o +obj-$(CONFIG_VIDEO_TW9910) += tw9910.o obj-$(CONFIG_VIDEO_CS3308) += cs3308.o obj-$(CONFIG_VIDEO_CS5345) += cs5345.o obj-$(CONFIG_VIDEO_CS53L32A) += cs53l32a.o diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c index bdb5e0a..96792df 100644 --- a/drivers/media/i2c/tw9910.c +++ b/drivers/media/i2c/tw9910.c @@ -1,6 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /* * tw9910 Video Driver * + * Copyright (C) 2017 Jacopo Mondi + * * Copyright (C) 2008 Renesas Solutions Corp. * Kuninori Morimoto * @@ -10,12 +13,10 @@ * Copyright 2006-7 Jonathan Corbet * Copyright (C) 2008 Magnus Damm * Copyright (C) 2008, Guennadi Liakhovetski - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ +#include +#include #include #include #include @@ -25,9 +26,7 @@ #include #include -#include #include -#include #include #define GET_ID(val) ((val & 0xF8) >> 3) @@ -228,8 +227,10 @@ struct tw9910_scale_ctrl { struct tw9910_priv { struct v4l2_subdev subdev; - struct v4l2_clk *clk; + struct clk *clk; struct tw9910_video_info*info; + struct gpio_desc*pdn_gpio; + struct gpio_desc*rstb_gpio; const struct tw9910_scale_ctrl *scale; v4l2_std_id norm; u32 revision; @@ -582,13 +583,66 @@ static int tw9910_s_register(struct v4l2_subdev *sd, } #endif +static int tw9910_power_on(struct tw9910_priv *priv) +{ + struct i2c_client *client = v4l2_get_subdevdata(>subdev); + int ret; + + if (priv->clk) { + ret = clk_prepare_enable(priv->clk); + if (ret) + return ret; + } + + if (priv->pdn_gpio) { + gpiod_set_value(priv->pdn_gpio, 0); + usleep_range(500, 1000); + } + + /* +* FIXME: The reset signal is connected to a shared GPIO on some +* platforms (namely the SuperH Migo-R). Until a framework becomes +* available to handle this cleanly, request the GPIO temporarily +* to avoid conflicts. +*/ + priv->rstb_gpio = gpiod_get_optional(>dev, "rstb", +GPIOD_OUT_LOW); + if (IS_ERR(priv->rstb_gpio)) { + dev_info(>dev, "Unable to get GPIO \"rstb\""); + return PTR_ERR(priv->rstb_gpio); + } + + if (priv->rstb_gpio) { + gpiod_set_value(priv->rstb_gpio, 1); + usleep_range(500, 1000); + gpiod_set_value(priv->rstb_gpio, 0); + usleep_range(500, 1000); + + gpiod_put(priv->rstb_gpio); + } + + return 0; +} + +static int tw9910_power_off(struct tw9910_priv *priv) +{ + clk_disable_unprepare(priv->clk); + + if (priv->pdn_gpio) { +
[PATCH v5 9/9] arch: sh: migor: Use new renesas-ceu camera driver
Migo-R platform uses sh_mobile_ceu camera driver, which is now being replaced by a proper V4L2 camera driver named 'renesas-ceu'. Move Migo-R platform to use the v4l2 renesas-ceu camera driver interface and get rid of soc_camera defined components used to register sensor drivers and of platform specific enable/disable routines. Register clock source and GPIOs for sensor drivers, so they can use clock and gpio APIs. Also, memory for CEU video buffers is now reserved with membocks APIs, and need to be declared as dma_coherent during machine initialization to remove that architecture specific part from CEU driver. Signed-off-by: Jacopo MondiReviewed-by: Laurent Pinchart --- arch/sh/boards/mach-migor/setup.c | 225 +++-- arch/sh/kernel/cpu/sh4a/clock-sh7722.c | 2 +- 2 files changed, 101 insertions(+), 126 deletions(-) diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c index 0bcbe58..ae4a786 100644 --- a/arch/sh/boards/mach-migor/setup.c +++ b/arch/sh/boards/mach-migor/setup.c @@ -1,17 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0 /* * Renesas System Solutions Asia Pte. Ltd - Migo-R * * Copyright (C) 2008 Magnus Damm - * - * This file is subject to the terms and conditions of the GNU General Public - * License. See the file "COPYING" in the main directory of this archive - * for more details. */ +#include #include #include #include #include #include +#include #include #include #include @@ -23,10 +22,11 @@ #include #include #include +#include #include #include #include -#include +#include #include #include #include @@ -45,6 +45,9 @@ * 0x1800 8GB8 NAND Flash (K9K8G08U0A) */ +#define CEU_BUFFER_MEMORY_SIZE (4 << 20) +static phys_addr_t ceu_dma_membase; + static struct smc91x_platdata smc91x_info = { .flags = SMC91X_USE_16BIT | SMC91X_NOWAIT, }; @@ -301,65 +304,24 @@ static struct platform_device migor_lcdc_device = { }, }; -static struct clk *camera_clk; -static DEFINE_MUTEX(camera_lock); - -static void camera_power_on(int is_tw) -{ - mutex_lock(_lock); - - /* Use 10 MHz VIO_CKO instead of 24 MHz to work -* around signal quality issues on Panel Board V2.1. -*/ - camera_clk = clk_get(NULL, "video_clk"); - clk_set_rate(camera_clk, 1000); - clk_enable(camera_clk); /* start VIO_CKO */ - - /* use VIO_RST to take camera out of reset */ - mdelay(10); - if (is_tw) { - gpio_set_value(GPIO_PTT2, 0); - gpio_set_value(GPIO_PTT0, 0); - } else { - gpio_set_value(GPIO_PTT0, 1); - } - gpio_set_value(GPIO_PTT3, 0); - mdelay(10); - gpio_set_value(GPIO_PTT3, 1); - mdelay(10); /* wait to let chip come out of reset */ -} - -static void camera_power_off(void) -{ - clk_disable(camera_clk); /* stop VIO_CKO */ - clk_put(camera_clk); - - gpio_set_value(GPIO_PTT3, 0); - mutex_unlock(_lock); -} - -static int ov7725_power(struct device *dev, int mode) -{ - if (mode) - camera_power_on(0); - else - camera_power_off(); - - return 0; -} - -static int tw9910_power(struct device *dev, int mode) -{ - if (mode) - camera_power_on(1); - else - camera_power_off(); - - return 0; -} - -static struct sh_mobile_ceu_info sh_mobile_ceu_info = { - .flags = SH_CEU_FLAG_USE_8BIT_BUS, +static const struct ceu_platform_data ceu_pdata = { + .num_subdevs= 2, + .subdevs = { + { /* [0] = ov772x */ + .flags = 0, + .bus_width = 8, + .bus_shift = 0, + .i2c_adapter_id = 0, + .i2c_address= 0x21, + }, + { /* [1] = tw9910 */ + .flags = 0, + .bus_width = 8, + .bus_shift = 0, + .i2c_adapter_id = 0, + .i2c_address= 0x45, + }, + }, }; static struct resource migor_ceu_resources[] = { @@ -373,18 +335,32 @@ static struct resource migor_ceu_resources[] = { .start = evt2irq(0x880), .flags = IORESOURCE_IRQ, }, - [2] = { - /* place holder for contiguous memory */ - }, }; static struct platform_device migor_ceu_device = { - .name = "sh_mobile_ceu", - .id = 0, /* "ceu0" clock */ + .name = "renesas-ceu", + .id = 0, /* ceu.0 */ .num_resources = ARRAY_SIZE(migor_ceu_resources), .resource = migor_ceu_resources, .dev= { - .platform_data =
Re: [PATCH v7 4/6] [media] vb2: add in-fence support to QBUF
On 01/10/18 17:07, Gustavo Padovan wrote: > From: Gustavo Padovan> > Receive in-fence from userspace and add support for waiting on them > before queueing the buffer to the driver. Buffers can't be queued to the > driver before its fences signal. And a buffer can't be queue to the driver > out of the order they were queued from userspace. That means that even if > it fence signal it must wait all other buffers, ahead of it in the queue, > to signal first. > > If the fence for some buffer fails we do not queue it to the driver, > instead we mark it as error and wait until the previous buffer is done > to notify userspace of the error. We wait here to deliver the buffers back > to userspace in order. > > v8: - improve comments about fences with errors > > v7: > - get rid of the fence array stuff for ordering and just use > get_num_buffers_ready() (Hans) > - fix issue of queuing the buffer twice (Hans) > - avoid the dma_fence_wait() in core_qbuf() (Alex) > - merge preparation commit in > > v6: > - With fences always keep the order userspace queues the buffers. > - Protect in_fence manipulation with a lock (Brian Starkey) > - check if fences have the same context before adding a fence array > - Fix last_fence ref unbalance in __set_in_fence() (Brian Starkey) > - Clean up fence if __set_in_fence() fails (Brian Starkey) > - treat -EINVAL from dma_fence_add_callback() (Brian Starkey) > > v5: - use fence_array to keep buffers ordered in vb2 core when > needed (Brian Starkey) > - keep backward compat on the reserved2 field (Brian Starkey) > - protect fence callback removal with lock (Brian Starkey) > > v4: > - Add a comment about dma_fence_add_callback() not returning a > error (Hans) > - Call dma_fence_put(vb->in_fence) if fence signaled (Hans) > - select SYNC_FILE under config VIDEOBUF2_CORE (Hans) > - Move dma_fence_is_signaled() check to __enqueue_in_driver() (Hans) > - Remove list_for_each_entry() in __vb2_core_qbuf() (Hans) > - Remove if (vb->state != VB2_BUF_STATE_QUEUED) from > vb2_start_streaming() (Hans) > - set IN_FENCE flags on __fill_v4l2_buffer (Hans) > - Queue buffers to the driver as soon as they are ready (Hans) > - call fill_user_buffer() after queuing the buffer (Hans) > - add err: label to clean up fence > - add dma_fence_wait() before calling vb2_start_streaming() > > v3: - document fence parameter > - remove ternary if at vb2_qbuf() return (Mauro) > - do not change if conditions behaviour (Mauro) > > v2: > - fix vb2_queue_or_prepare_buf() ret check > - remove check for VB2_MEMORY_DMABUF only (Javier) > - check num of ready buffers to start streaming > - when queueing, start from the first ready buffer > - handle queue cancel > > Signed-off-by: Gustavo Padovan > --- > drivers/media/common/videobuf/videobuf2-core.c | 166 > ++--- > drivers/media/common/videobuf/videobuf2-v4l2.c | 29 - > drivers/media/v4l2-core/Kconfig| 33 + > include/media/videobuf2-core.h | 14 ++- > 4 files changed, 221 insertions(+), 21 deletions(-) > > diff --git a/drivers/media/common/videobuf/videobuf2-core.c > b/drivers/media/common/videobuf/videobuf2-core.c > index f7109f827f6e..777e3a2bc746 100644 > --- a/drivers/media/common/videobuf/videobuf2-core.c > +++ b/drivers/media/common/videobuf/videobuf2-core.c > @@ -352,6 +352,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum > vb2_memory memory, > vb->index = q->num_buffers + buffer; > vb->type = q->type; > vb->memory = memory; > + spin_lock_init(>fence_cb_lock); > for (plane = 0; plane < num_planes; ++plane) { > vb->planes[plane].length = plane_sizes[plane]; > vb->planes[plane].min_length = plane_sizes[plane]; > @@ -936,7 +937,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum > vb2_buffer_state state) > > switch (state) { > case VB2_BUF_STATE_QUEUED: > - return; > + break; > case VB2_BUF_STATE_REQUEUEING: > if (q->start_streaming_called) > __enqueue_in_driver(vb); > @@ -946,6 +947,19 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum > vb2_buffer_state state) > wake_up(>done_wq); > break; > } > + > + /* > + * The check below verifies if there is a buffer in queue with an s/in/in the/ > + * error state. They are added to queue in the error state when > + * their in-fence fails to signal. > + * To not mess with buffer ordering we wait until the previous buffer > + * is done to mark the buffer in the error state as done and notify > + * userspace. So
Re: [PATCH v4 3/9] v4l: platform: Add Renesas CEU driver
Hi Philippe, Laurent, Geert, On Fri, Jan 12, 2018 at 11:36:31AM +0100, Philippe Ombredanne wrote: > On Tue, Jan 9, 2018 at 5:25 PM, Jacopo Mondi> wrote: > > Add driver for Renesas Capture Engine Unit (CEU). > > > > > --- /dev/null > > +++ b/drivers/media/platform/renesas-ceu.c > > @@ -0,0 +1,1648 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > > > > +MODULE_DESCRIPTION("Renesas CEU camera driver"); > > +MODULE_AUTHOR("Jacopo Mondi "); > > +MODULE_LICENSE("GPL"); > > Jacopo, > the MODULE_LICENSE does not match the SPDX tag. Per module.h "GPL" > means GPL-2.0 or later ;) > > It should be instead: > > > +MODULE_LICENSE("GPL v2"); > > ... to match your > > > +// SPDX-License-Identifier: GPL-2.0 I will update this in next v5. Laurent, Geert: I'll keep SPDX identifier to "GPL-2.0" until kernel doc does not get updated. Thanks j > > I know this can be confusing, but updating the MODULE_LICENSE tags > definitions in module.h to match SPDX tags is unlikely to happen as it > would create mayhem for everyone and every module loader relying on > this established convention. > > -- > Cordially > Philippe Ombredanne
[PATCH] media: imx258: Add imx258 camera sensor driver
Add a V4L2 sub-device driver for the Sony IMX258 image sensor. This is a camera sensor using the I2C bus for control and the CSI-2 bus for data. Signed-off-by: Andy Yeh--- MAINTAINERS|7 + drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/imx258.c | 1155 4 files changed, 1174 insertions(+) create mode 100644 drivers/media/i2c/imx258.c diff --git a/MAINTAINERS b/MAINTAINERS index d76af75..806aa46 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12640,6 +12640,13 @@ S: Maintained F: drivers/ssb/ F: include/linux/ssb/ +SONY IMX258 SENSOR DRIVER +M: Sakari Ailus +L: linux-media@vger.kernel.org +T: git git://linuxtv.org/media_tree.git +S: Maintained +F: drivers/media/i2c/imx258.c + SONY IMX274 SENSOR DRIVER M: Leon Luo L: linux-media@vger.kernel.org diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index cb5d7ff..cabde37 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -555,6 +555,17 @@ config VIDEO_APTINA_PLL config VIDEO_SMIAPP_PLL tristate +config VIDEO_IMX258 + tristate "Sony IMX258 sensor support" + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API + depends on MEDIA_CAMERA_SUPPORT + ---help--- + This is a Video4Linux2 sensor-level driver for the Sony + IMX258 camera. + + To compile this driver as a module, choose M here: the + module will be called imx258. + config VIDEO_IMX274 tristate "Sony IMX274 sensor support" depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 548a9ef..cf1e0f1 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -93,6 +93,7 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o obj-$(CONFIG_VIDEO_OV2659) += ov2659.o obj-$(CONFIG_VIDEO_TC358743) += tc358743.o +obj-$(CONFIG_VIDEO_IMX258) += imx258.o obj-$(CONFIG_VIDEO_IMX274) += imx274.o obj-$(CONFIG_SDR_MAX2175) += max2175.o diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c new file mode 100644 index 000..87e4b18 --- /dev/null +++ b/drivers/media/i2c/imx258.c @@ -0,0 +1,1155 @@ +// Copyright (C) 2018 Intel Corporation +// SPDX-License-Identifier: GPL-2.0 + +#include +#include +#include +#include +#include +#include +#include + +#define IMX258_REG_VALUE_08BIT 1 +#define IMX258_REG_VALUE_16BIT 2 +#define IMX258_REG_VALUE_24BIT 3 + +#define IMX258_REG_MODE_SELECT 0x0100 +#define IMX258_MODE_STANDBY0x00 +#define IMX258_MODE_STREAMING 0x01 + +/* Chip ID */ +#define IMX258_REG_CHIP_ID 0x0016 +#define IMX258_CHIP_ID 0x0258 + +/* V_TIMING internal */ +#define IMX258_REG_VTS 0x0340 +#define IMX258_VTS_30FPS 0x0c98 +#define IMX258_VTS_MAX 0x7fff + +/* HBLANK control - read only */ +#define IMX258_PPL_650MHZ 5352 +#define IMX258_PPL_325MHZ 2676 + +/* Exposure control */ +#define IMX258_REG_EXPOSURE0x0202 +#define IMX258_EXPOSURE_MIN4 +#define IMX258_EXPOSURE_STEP 1 +#define IMX258_EXPOSURE_DEFAULT0x640 + +/* Analog gain control */ +#define IMX258_REG_ANALOG_GAIN 0x0204 +#define IMX258_ANA_GAIN_MIN0 +#define IMX258_ANA_GAIN_MAX0x1fff +#define IMX258_ANA_GAIN_STEP 1 +#define IMX258_ANA_GAIN_DEFAULT0x0 + +/* Orientation */ +#define REG_MIRROR_FLIP_CONTROL0x0101 +#define REG_CONFIG_MIRROR_FLIP 0x03 + +struct imx258_reg { + u16 address; + u8 val; +}; + +struct imx258_reg_list { + u32 num_of_regs; + const struct imx258_reg *regs; +}; + +/* Link frequency config */ +struct imx258_link_freq_config { + u32 pixels_per_line; + + /* PLL registers for this link frequency */ + struct imx258_reg_list reg_list; +}; + +/* Mode : resolution and related config */ +struct imx258_mode { + /* Frame width */ + u32 width; + /* Frame height */ + u32 height; + + /* V-timing */ + u32 vts_def; + u32 vts_min; + + /* Index of Link frequency config to be used */ + u32 link_freq_index; + /* Default register values */ + struct imx258_reg_list reg_list; +}; + +/* 4208x3118 needs 1300Mbps/lane, 4 lanes */ +static const struct imx258_reg mipi_data_rate_1300mbps[] = { + {0x0301, 0x05}, + {0x0303, 0x02}, + {0x0305, 0x03}, + {0x0306, 0x00}, + {0x0307, 0xCB}, + {0x0309, 0x0A}, + {0x030B, 0x01}, + {0x030D, 0x02}, + {0x030E, 0x00}, + {0x030F, 0xD8}, + {0x0310, 0x00}, + {0x0820, 0x14}, + {0x0821,
RE: [PATCH] media: imx258: Add imx258 camera sensor driver
Hi Sakari, > > + /* stream off */ > > + ret = imx258_write_reg(imx258, IMX258_REG_MODE_SELECT, > > + IMX258_REG_VALUE_08BIT, > > +IMX258_MODE_STANDBY); >> Yes, this is true. But the sensor is already in software standby mode here, >> isn't it? No, it's not. 0x0100 is not set to 0 in default setting. Power on sequence requests sw-standby to be set at T6 (reset + 0.4ms), and streaming on to be set at T7 (reset + 12ms). > Fair enough; to be on the safe side, you indeed need the 12 ms delay here. OK. I will set usleep_range(1000, 2000), which is enough for test on DUT. Regards, Andy -Original Message- From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com] Sent: Friday, January 12, 2018 4:46 PM To: Yeh, AndyCc: linux-media@vger.kernel.org; Chiang, AlanX ; Chen, JasonX Z Subject: Re: [PATCH] media: imx258: Add imx258 camera sensor driver Hi Andy, On Fri, Jan 12, 2018 at 08:18:50AM +, Yeh, Andy wrote: > Thanks Sakari to remind. > Since I would like to attach some snips from the datasheets. But blocked, > I will re-edit to plain table and hope it could be understood. Please wrap the lines at 80 characters. > > Regards, Andy > > From: Yeh, Andy > Sent: Friday, January 12, 2018 3:31 PM > To: 'Sakari Ailus' > Cc: 'linux-media@vger.kernel.org' ; AlanX > Chiang (alanx.chi...@intel.com) ; JasonX Z > Chen (jasonx.z.c...@intel.com) > Subject: RE: [PATCH] media: imx258: Add imx258 camera sensor driver > > Hi Sakari, > > Thanks for the review comments. Please check first then I will update patch > accordingly. > > > + usleep_range(1000, 2000); > > You are right. This should be removed since delayed in ACPI power on > sequence. > https://chromium-review.googlesource.com/c/chromiumos/third_party/core > boot/+/826746/5/src/mainboard/google/poppy/variants/nautilus/include/v > ariant/acpi/mipi_camera.asl#402 > \_SB.PCI0.I2C2.CAM0.CRST(1) Sleep(5) (could be > less so it could be adjusted in coreboot then for shorter launch time. > ) Ack. > > > + /* stream off */ > > + ret = imx258_write_reg(imx258, IMX258_REG_MODE_SELECT, > > + IMX258_REG_VALUE_08BIT, > > +IMX258_MODE_STANDBY); > > I don't think it should be possible that the sensor was streaming > > here. If it was, something must have been really wrong. > > Sensor datasheet claimed before updating register and streaming, need > do SW standby. However Sony implements the function with single bit. > (different from OVT) So I will change the comment to "software standby" > to replace "stream off" to clarify the purpose. > > Address: 0x0100 > Value: > 0: software standby > 1: streaming Yes, this is true. But the sensor is already in software standby mode here, isn't it? ... > And from T6~T7, per the profiling data on DUT, the duration is always > longer than 25ms (removed the manual delay) while loading all required > register lists. I would still prefer to keep a 1~2ms delay range > before streaming on for safety purpose. Fair enough; to be on the safe side, you indeed need the 12 ms delay here. > > > +static int imx258_get_skip_frames(struct v4l2_subdev *sd, u32 > > +*frames) { > > + /* > > + * Sony: The 1st frame is ok when set from SW standby to streaming. > > + */ > > + *frames = 0; > > > If that's the case, then you can drop g_skip_frames callback altogether.. > > If remove g_skip_frames callback, it implies always 0 when user queries. > I would like to keep the flexibly if we will need change the > skip_frames value. How do you think? If it turns out the sensor needs this, then you can re-introduce the function. Please remove it now. > > > + case V4L2_CID_GAIN: > > + /* Todo */ > > + break; > > + case V4L2_CID_TEST_PATTERN: > > + /* Todo */ > > If the control isn't implemented, I suggest to remove it from the driver > > altogether, or alternatively implement it. > > The GAIN control should likely be DIGITAL_GAIN instead, right? > > You are right. Will remove them first. We are almost done > (DIGITAL_GAIN) and will submit separately. Ack. -- Kind regards, Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH v7 3/6] [media] vb2: add explicit fence user API
On 01/10/18 17:07, Gustavo Padovan wrote: > From: Gustavo Padovan> > Turn the reserved2 field into fence_fd that we will use to send > an in-fence to the kernel and return an out-fence from the kernel to > userspace. > > Two new flags were added, V4L2_BUF_FLAG_IN_FENCE, that should be used > when sending a fence to the kernel to be waited on, and > V4L2_BUF_FLAG_OUT_FENCE, to ask the kernel to give back an out-fence. > > v5: > - keep using reserved2 field for cpia2 > - set fence_fd to 0 for now, for compat with userspace(Mauro) > > v4: > - make it a union with reserved2 and fence_fd (Hans Verkuil) > > v3: > - make the out_fence refer to the current buffer (Hans Verkuil) > > v2: add documentation > > Signed-off-by: Gustavo Padovan > --- > Documentation/media/uapi/v4l/buffer.rst| 15 +++ > drivers/media/common/videobuf/videobuf2-v4l2.c | 2 +- > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 4 ++-- > include/uapi/linux/videodev2.h | 7 ++- > 4 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/Documentation/media/uapi/v4l/buffer.rst > b/Documentation/media/uapi/v4l/buffer.rst > index ae6ee73f151c..eeefbd2547e7 100644 > --- a/Documentation/media/uapi/v4l/buffer.rst > +++ b/Documentation/media/uapi/v4l/buffer.rst > @@ -648,6 +648,21 @@ Buffer Flags >- Start Of Exposure. The buffer timestamp has been taken when the > exposure of the frame has begun. This is only valid for the > ``V4L2_BUF_TYPE_VIDEO_CAPTURE`` buffer type. > +* .. _`V4L2-BUF-FLAG-IN-FENCE`: > + > + - ``V4L2_BUF_FLAG_IN_FENCE`` > + - 0x0020 > + - Ask V4L2 to wait on fence passed in ``fence_fd`` field. The buffer > + won't be queued to the driver until the fence signals. I'd also add: "The order in which buffers are queued is guaranteed to be preserved, so any buffers queued after this buffer will also be blocked until this fence signals." Regards, Hans > + > +* .. _`V4L2-BUF-FLAG-OUT-FENCE`: > + > + - ``V4L2_BUF_FLAG_OUT_FENCE`` > + - 0x0040 > + - Request a fence to be attached to the buffer. The ``fence_fd`` > + field on > + :ref:`VIDIOC_QBUF` is used as a return argument to send the out-fence > + fd to userspace. > > > > diff --git a/drivers/media/common/videobuf/videobuf2-v4l2.c > b/drivers/media/common/videobuf/videobuf2-v4l2.c > index fac3cd6f901d..d838524a459e 100644 > --- a/drivers/media/common/videobuf/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf/videobuf2-v4l2.c > @@ -203,7 +203,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, > void *pb) > b->timestamp = ns_to_timeval(vb->timestamp); > b->timecode = vbuf->timecode; > b->sequence = vbuf->sequence; > - b->reserved2 = 0; > + b->fence_fd = 0; > b->reserved = 0; > > if (q->is_multiplanar) { > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > index e48d59046086..a11a0a2bed47 100644 > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > @@ -370,7 +370,7 @@ struct v4l2_buffer32 { > __s32 fd; > } m; > __u32 length; > - __u32 reserved2; > + __s32 fence_fd; > __u32 reserved; > }; > > @@ -533,7 +533,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, > struct v4l2_buffer32 __user > put_user(kp->timestamp.tv_usec, >timestamp.tv_usec) || > copy_to_user(>timecode, >timecode, sizeof(struct > v4l2_timecode)) || > put_user(kp->sequence, >sequence) || > - put_user(kp->reserved2, >reserved2) || > + put_user(kp->fence_fd, >fence_fd) || > put_user(kp->reserved, >reserved) || > put_user(kp->length, >length)) > return -EFAULT; > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 58894cfe9479..2d424aebdd1e 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -933,7 +933,10 @@ struct v4l2_buffer { > __s32 fd; > } m; > __u32 length; > - __u32 reserved2; > + union { > + __s32 fence_fd; > + __u32 reserved2; > + }; > __u32 reserved; > }; > > @@ -970,6 +973,8 @@ struct v4l2_buffer { > #define V4L2_BUF_FLAG_TSTAMP_SRC_SOE 0x0001 > /* mem2mem encoder/decoder */ > #define V4L2_BUF_FLAG_LAST 0x0010 > +#define V4L2_BUF_FLAG_IN_FENCE 0x0020 > +#define V4L2_BUF_FLAG_OUT_FENCE 0x0040 > > /** > * struct v4l2_exportbuffer - export of
Re: [PATCH v7 3/6] [media] vb2: add explicit fence user API
On 01/10/18 17:07, Gustavo Padovan wrote: > From: Gustavo Padovan> > Turn the reserved2 field into fence_fd that we will use to send > an in-fence to the kernel and return an out-fence from the kernel to s/and/or/ > userspace. > > Two new flags were added, V4L2_BUF_FLAG_IN_FENCE, that should be used > when sending a fence to the kernel to be waited on, and > V4L2_BUF_FLAG_OUT_FENCE, to ask the kernel to give back an out-fence. > > v5: > - keep using reserved2 field for cpia2 > - set fence_fd to 0 for now, for compat with userspace(Mauro) > > v4: > - make it a union with reserved2 and fence_fd (Hans Verkuil) > > v3: > - make the out_fence refer to the current buffer (Hans Verkuil) > > v2: add documentation > > Signed-off-by: Gustavo Padovan > --- > Documentation/media/uapi/v4l/buffer.rst| 15 +++ > drivers/media/common/videobuf/videobuf2-v4l2.c | 2 +- > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 4 ++-- > include/uapi/linux/videodev2.h | 7 ++- > 4 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/Documentation/media/uapi/v4l/buffer.rst > b/Documentation/media/uapi/v4l/buffer.rst > index ae6ee73f151c..eeefbd2547e7 100644 > --- a/Documentation/media/uapi/v4l/buffer.rst > +++ b/Documentation/media/uapi/v4l/buffer.rst I'm missing documentation for the new fence_fd field in struct v4l2_buffer. Make sure to mention what value should be used if you don't set the IN_FENCE flag. > @@ -648,6 +648,21 @@ Buffer Flags >- Start Of Exposure. The buffer timestamp has been taken when the > exposure of the frame has begun. This is only valid for the > ``V4L2_BUF_TYPE_VIDEO_CAPTURE`` buffer type. > +* .. _`V4L2-BUF-FLAG-IN-FENCE`: > + > + - ``V4L2_BUF_FLAG_IN_FENCE`` > + - 0x0020 > + - Ask V4L2 to wait on fence passed in ``fence_fd`` field. The buffer s/fence/the fence/ s/in/in the/ > + won't be queued to the driver until the fence signals. > + > +* .. _`V4L2-BUF-FLAG-OUT-FENCE`: > + > + - ``V4L2_BUF_FLAG_OUT_FENCE`` > + - 0x0040 > + - Request a fence to be attached to the buffer. The ``fence_fd`` s/Request/Request for/ > + field on This very short line looks weird. > + :ref:`VIDIOC_QBUF` is used as a return argument to send the out-fence > + fd to userspace. I'd rephrase this: "The driver will fill in the out-fence fd in the ``fence_fd`` field when :ref:`VIDIOC_QBUF` returns." For both flags you also need to be explicit in mentioning that the application sets these flags before calling VIDIOC_QBUF. For other ioctls the driver just reports these flags. Also mention what happens if the fence can't be found or can't be created. Regards, Hans > > > > diff --git a/drivers/media/common/videobuf/videobuf2-v4l2.c > b/drivers/media/common/videobuf/videobuf2-v4l2.c > index fac3cd6f901d..d838524a459e 100644 > --- a/drivers/media/common/videobuf/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf/videobuf2-v4l2.c > @@ -203,7 +203,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, > void *pb) > b->timestamp = ns_to_timeval(vb->timestamp); > b->timecode = vbuf->timecode; > b->sequence = vbuf->sequence; > - b->reserved2 = 0; > + b->fence_fd = 0; > b->reserved = 0; > > if (q->is_multiplanar) { > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > index e48d59046086..a11a0a2bed47 100644 > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > @@ -370,7 +370,7 @@ struct v4l2_buffer32 { > __s32 fd; > } m; > __u32 length; > - __u32 reserved2; > + __s32 fence_fd; > __u32 reserved; > }; > > @@ -533,7 +533,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, > struct v4l2_buffer32 __user > put_user(kp->timestamp.tv_usec, >timestamp.tv_usec) || > copy_to_user(>timecode, >timecode, sizeof(struct > v4l2_timecode)) || > put_user(kp->sequence, >sequence) || > - put_user(kp->reserved2, >reserved2) || > + put_user(kp->fence_fd, >fence_fd) || > put_user(kp->reserved, >reserved) || > put_user(kp->length, >length)) > return -EFAULT; > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 58894cfe9479..2d424aebdd1e 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -933,7 +933,10 @@ struct v4l2_buffer { > __s32 fd; > } m; > __u32 length; > - __u32 reserved2; > + union { > + __s32 fence_fd; > +
Re: [PATCH v7 2/6] [media] v4l: add 'unordered' flag to format description ioctl
On 01/10/18 17:07, Gustavo Padovan wrote: > From: Gustavo Padovan> > For explicit synchronization it important for userspace to know if the > format being used by the driver can deliver the buffers back to userspace > in the same order they were queued with QBUF. > > Ordered streams fits nicely in a pipeline with DRM for example, where > ordered buffer are expected. > > Signed-off-by: Gustavo Padovan > --- > Documentation/media/uapi/v4l/vidioc-enum-fmt.rst | 3 +++ > include/uapi/linux/videodev2.h | 1 + > 2 files changed, 4 insertions(+) > > diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst > b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst > index 019c513df217..368115f44fc0 100644 > --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst > +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst > @@ -116,6 +116,9 @@ one until ``EINVAL`` is returned. >- This format is not native to the device but emulated through > software (usually libv4l2), where possible try to use a native > format instead for better performance. > +* - ``V4L2_FMT_FLAG_UNORDERED`` > + - 0x0004 > + - This is a format that doesn't guarantee timely order of frames. I'd rephrase this: "This format doesn't guarantee ordered buffer handling. I.e. the order in which buffers are dequeued with VIDIOC_DQBUF may be different from the order in which they were queued with VIDIOC_QBUF." (Use proper links to VIDIOC_(D)QBUF) I would also like to see an example of a driver that uses this. The cobalt driver is a candidate for this. Regards, Hans > > > Return Value > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 982718965180..58894cfe9479 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -716,6 +716,7 @@ struct v4l2_fmtdesc { > > #define V4L2_FMT_FLAG_COMPRESSED 0x0001 > #define V4L2_FMT_FLAG_EMULATED 0x0002 > +#define V4L2_FMT_FLAG_UNORDERED 0x0004 > > /* Frame Size and frame rate enumeration */ > /* >
Re: [PATCH v7 1/6] [media] vb2: add is_unordered callback for drivers
On 01/10/18 17:07, Gustavo Padovan wrote: > From: Gustavo Padovan> > Explicit synchronization benefits a lot from ordered queues, they fit > better in a pipeline with DRM for example so create a opt-in way for > drivers notify videobuf2 that the queue is unordered. > > Drivers don't need implement it if the queue is ordered. > > Signed-off-by: Gustavo Padovan > --- > include/media/videobuf2-core.h | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index f3ee4c7c2fb3..583cdc06de79 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -370,6 +370,9 @@ struct vb2_buffer { > * callback by calling vb2_buffer_done() with either > * %VB2_BUF_STATE_DONE or %VB2_BUF_STATE_ERROR; may use > * vb2_wait_for_all_buffers() function > + * @is_unordered:tell if the queue format is unordered. The default is I'd replace the first sentence by this: "tell if the queue is unordered, i.e. buffers can be dequeued in a different order from how they were queued." Regards, Hans > + * assumed to be ordered and this function only needs to > + * be implemented for unordered queues. > * @buf_queue: passes buffer vb to the driver; driver may start > * hardware operation on this buffer; driver should give > * the buffer back by calling vb2_buffer_done() function; > @@ -393,6 +396,7 @@ struct vb2_ops { > > int (*start_streaming)(struct vb2_queue *q, unsigned int count); > void (*stop_streaming)(struct vb2_queue *q); > + int (*is_unordered)(struct vb2_queue *q); > > void (*buf_queue)(struct vb2_buffer *vb); > }; > @@ -566,6 +570,7 @@ struct vb2_queue { > u32 cnt_wait_finish; > u32 cnt_start_streaming; > u32 cnt_stop_streaming; > + u32 cnt_is_unordered; > #endif > }; > >
Re: [RFC PATCH 0/9] media: base request API support
Hi Alexandre, On 12/15/17 08:56, Alexandre Courbot wrote: > Here is a new attempt at the request API, following the UAPI we agreed on in > Prague. Hopefully this can be used as the basis to move forward. > > This series only introduces the very basics of how requests work: allocate a > request, queue buffers to it, queue the request itself, wait for it to > complete, > reuse it. It does *not* yet use Hans' work with controls setting. I have > preferred to submit it this way for now as it allows us to concentrate on the > basic request/buffer flow, which was harder to get properly than I initially > thought. I still have a gut feeling that it can be improved, with less > back-and- > forth into drivers. > > Plugging in controls support should not be too hard a task (basically just > apply > the saved controls when the request starts), and I am looking at it now. > > The resulting vim2m driver can be successfully used with requests, and my > tests > so far have been successful. > > There are still some rougher edges: > > * locking is currently quite coarse-grained > * too many #ifdef CONFIG_MEDIA_CONTROLLER in the code, as the request API > depends on it - I plan to craft the headers so that it becomes unnecessary. > As it is, some of the code will probably not even compile if > CONFIG_MEDIA_CONTROLLER is not set > > But all in all I think the request flow should be clear and easy to review, > and > the possibility of custom queue and entity support implementations should give > us the flexibility we need to support more specific use-cases (I expect the > generic implementations to be sufficient most of the time though). > > A very simple test program exercising this API is available here (don't forget > to adapt the /dev/media0 hardcoding): > https://gist.github.com/Gnurou/dbc3776ed97ea7d4ce6041ea15eb0438 > > Looking forward to your feedback and comments! I think this will become more interesting when the control code is in. The main thing I've noticed with this patch series is that it is very codec oriented. Which in some ways is OK (after all, that's the first type of HW that we want to support), but the vb2 code in particular should be more generic. I would also recommend that you start preparing documentation patches: we can review that and make sure all the corner-cases are correctly documented. The public API changes are (I think) fairly limited, but the devil is in the details, so getting that reviewed early on will help you later. It's a bit unfortunate that the fence patch series is also making vb2 changes, but I hope that will be merged fairly soon so you can develop on top of that series. Regards, Hans > > Alexandre Courbot (8): > media: add request API core and UAPI > media: request: add generic queue > media: request: add generic entity ops > media: vb2: add support for requests > media: vb2: add support for requests in QBUF ioctl > media: v4l2-mem2mem: add request support > media: vim2m: add media device > media: vim2m: add request support > > Hans Verkuil (1): > videodev2.h: Add request field to v4l2_buffer > > drivers/media/Makefile| 4 +- > drivers/media/media-device.c | 6 + > drivers/media/media-request-entity-generic.c | 56 > drivers/media/media-request-queue-generic.c | 150 ++ > drivers/media/media-request.c | 390 > ++ > drivers/media/platform/vim2m.c| 46 +++ > drivers/media/usb/cpia2/cpia2_v4l.c | 2 +- > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 +- > drivers/media/v4l2-core/v4l2-ioctl.c | 99 ++- > drivers/media/v4l2-core/v4l2-mem2mem.c| 34 +++ > drivers/media/v4l2-core/videobuf2-core.c | 59 +++- > drivers/media/v4l2-core/videobuf2-v4l2.c | 32 ++- > include/media/media-device.h | 3 + > include/media/media-entity.h | 6 + > include/media/media-request.h | 282 +++ > include/media/v4l2-mem2mem.h | 19 ++ > include/media/videobuf2-core.h| 25 +- > include/media/videobuf2-v4l2.h| 2 + > include/uapi/linux/media.h| 11 + > include/uapi/linux/videodev2.h| 3 +- > 20 files changed, 1216 insertions(+), 20 deletions(-) > create mode 100644 drivers/media/media-request-entity-generic.c > create mode 100644 drivers/media/media-request-queue-generic.c > create mode 100644 drivers/media/media-request.c > create mode 100644 include/media/media-request.h >
Re: [RFC PATCH 6/9] media: vb2: add support for requests in QBUF ioctl
On 12/15/17 08:56, Alexandre Courbot wrote: > Support the request argument of the QBUF ioctl. > > Signed-off-by: Alexandre Courbot> --- > drivers/media/v4l2-core/v4l2-ioctl.c | 93 > +++- > 1 file changed, 92 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c > b/drivers/media/v4l2-core/v4l2-ioctl.c > index 8d041247e97f..28f9c368563e 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > > #include > > @@ -965,6 +966,81 @@ static int check_fmt(struct file *file, enum > v4l2_buf_type type) > return -EINVAL; > } > > +/* > + * Validate that a given request can be used during an ioctl. > + * > + * When using the request API, request file descriptors must be matched > against > + * the actual request object. User-space can pass any file descriptor, so we > + * need to make sure the call is valid before going further. > + * > + * This function looks up the request and associated data and performs the > + * following sanity checks: > + * > + * * Make sure that the entity supports requests, > + * * Make sure that the entity belongs to the media_device managing the > passed > + * request, > + * * Make sure that the entity data (if any) is associated to the current > file > + * handler. > + * > + * This function returns a pointer to the valid request, or and error code in > + * case of failure. When successful, a reference to the request is acquired > and > + * must be properly released. > + */ > +#ifdef CONFIG_MEDIA_CONTROLLER > +static struct media_request * > +check_request(int request, struct file *file, void *fh) > +{ > + struct media_request *req = NULL; > + struct video_device *vfd = video_devdata(file); > + struct v4l2_fh *vfh = > + test_bit(V4L2_FL_USES_V4L2_FH, >flags) ? fh : NULL; > + struct media_entity *entity = >entity; > + const struct media_entity *ent; > + struct media_request_entity_data *data; > + bool found = false; > + > + if (!entity) > + return ERR_PTR(-EINVAL); > + > + /* Check that the entity supports requests */ > + if (!entity->req_ops) > + return ERR_PTR(-ENOTSUPP); > + > + req = media_request_get_from_fd(request); You can get the media_device from vfd->v4l2_dev->mdev. So it is much easier to just pass the media_device as an argument to media_request_get_from_fd()... > + if (!req) > + return ERR_PTR(-EINVAL); > + > + /* Validate that the entity belongs to the media_device managing > + * the request queue */ > + media_device_for_each_entity(ent, req->queue->mdev) { > + if (entity == ent) { > + found = true; > + break; > + } > + } > + if (!found) { > + media_request_put(req); > + return ERR_PTR(-EINVAL); > + } ...and then you don't need to do this ^^^ extra validation check. > + > + /* Validate that the entity's data belongs to the correct fh */ > + data = media_request_get_entity_data(req, entity, vfh); > + if (IS_ERR(data)) { > + media_request_put(req); > + return ERR_PTR(PTR_ERR(data)); > + } This assumes that each filehandle has its own state. That's true for codecs, but not for most (all?) other devices. There the state is per device instance. I'm not sure if we have a unique identifying mark for such drivers. The closest is checking if fh->m2m_ctx is non-NULL, but I don't know if all drivers with per-filehandle state use that field. An alternative might be to check if fh->ctrl_handler is non-NULL. But again, I'm not sure if that's a 100% valid check. > + > + return req; > +} > +#else /* CONFIG_MEDIA_CONTROLLER */ > +static struct media_request * > +check_request(int request, struct file *file, void *fh) > +{ > + return ERR_PTR(-ENOTSUPP); > +} > + > +#endif /* CONFIG_MEDIA_CONTROLLER */ > + > static void v4l_sanitize_format(struct v4l2_format *fmt) > { > unsigned int offset; > @@ -1902,10 +1978,25 @@ static int v4l_querybuf(const struct v4l2_ioctl_ops > *ops, > static int v4l_qbuf(const struct v4l2_ioctl_ops *ops, > struct file *file, void *fh, void *arg) > { > + struct media_request *req = NULL; > struct v4l2_buffer *p = arg; > int ret = check_fmt(file, p->type); > > - return ret ? ret : ops->vidioc_qbuf(file, fh, p); > + if (ret) > + return ret; > + > + if (p->request > 0) { > + req = check_request(p->request, file, fh); > + if (IS_ERR(req)) > + return PTR_ERR(req); > + } > + > + ret = ops->vidioc_qbuf(file, fh, p); > + > + if (req) > + media_request_put(req); > + > + return ret; > } > > static int v4l_dqbuf(const struct v4l2_ioctl_ops
Re: [RFC PATCH 5/9] media: vb2: add support for requests
On 12/15/17 08:56, Alexandre Courbot wrote: > Add throttling support for buffers when requests are in use on a given > queue. Buffers associated to a request are kept into the vb2 queue until > the request becomes active, at which point all the buffers are passed to > the driver. The queue can also signal that is has processed all of a > request's buffers. > > Also add support for the request parameter when handling the QBUF ioctl. > > Signed-off-by: Alexandre Courbot> --- > drivers/media/v4l2-core/videobuf2-core.c | 59 > > drivers/media/v4l2-core/videobuf2-v4l2.c | 29 +++- > include/media/videobuf2-core.h | 25 +- > 3 files changed, 104 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/v4l2-core/videobuf2-core.c > b/drivers/media/v4l2-core/videobuf2-core.c > index cb115ba6a1d2..c01038b7962a 100644 > --- a/drivers/media/v4l2-core/videobuf2-core.c > +++ b/drivers/media/v4l2-core/videobuf2-core.c > @@ -898,6 +898,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum > vb2_buffer_state state) > state != VB2_BUF_STATE_REQUEUEING)) > state = VB2_BUF_STATE_ERROR; > > + WARN_ON(vb->request != q->cur_req); What's the reason for this WARN_ON? It's not immediately obvious to me. > + > #ifdef CONFIG_VIDEO_ADV_DEBUG > /* >* Although this is not a callback, it still does have to balance > @@ -920,6 +922,13 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum > vb2_buffer_state state) > /* Add the buffer to the done buffers list */ > list_add_tail(>done_entry, >done_list); > vb->state = state; > + > + if (q->cur_req) { > + WARN_ON(q->req_buf_cnt < 1); > + > + if (--q->req_buf_cnt == 0) > + q->cur_req = NULL; > + } > } > atomic_dec(>owned_by_drv_count); > spin_unlock_irqrestore(>done_lock, flags); > @@ -1298,6 +1307,16 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned > int index, void *pb) > } > EXPORT_SYMBOL_GPL(vb2_core_prepare_buf); > > +static void vb2_queue_enqueue_current_buffers(struct vb2_queue *q) > +{ > + struct vb2_buffer *vb; > + > + list_for_each_entry(vb, >queued_list, queued_entry) { > + if (vb->request == q->cur_req) > + __enqueue_in_driver(vb); > + } > +} I think this will clash big time with the v4l2 fence patch series... > + > /** > * vb2_start_streaming() - Attempt to start streaming. > * @q: videobuf2 queue > @@ -1318,8 +1337,7 @@ static int vb2_start_streaming(struct vb2_queue *q) >* If any buffers were queued before streamon, >* we can now pass them to driver for processing. >*/ > - list_for_each_entry(vb, >queued_list, queued_entry) > - __enqueue_in_driver(vb); > + vb2_queue_enqueue_current_buffers(q); > > /* Tell the driver to start streaming */ > q->start_streaming_called = 1; > @@ -1361,7 +1379,8 @@ static int vb2_start_streaming(struct vb2_queue *q) > return ret; > } > > -int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb) > +int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, > + struct media_request *req, void *pb) > { > struct vb2_buffer *vb; > int ret; > @@ -1392,6 +1411,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int > index, void *pb) > q->queued_count++; > q->waiting_for_buffers = false; > vb->state = VB2_BUF_STATE_QUEUED; > + vb->request = req; > > if (pb) > call_void_bufop(q, copy_timestamp, vb, pb); > @@ -1401,8 +1421,11 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int > index, void *pb) > /* >* If already streaming, give the buffer to driver for processing. >* If not, the buffer will be given to driver on next streamon. > + * > + * If using the request API, the buffer will be given to the driver > + * when the request becomes active. >*/ > - if (q->start_streaming_called) > + if (q->start_streaming_called && !req) > __enqueue_in_driver(vb); > > /* Fill buffer information for the userspace */ > @@ -1427,6 +1450,28 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int > index, void *pb) > } > EXPORT_SYMBOL_GPL(vb2_core_qbuf); > > +void vb2_queue_start_request(struct vb2_queue *q, struct media_request *req) > +{ > + struct vb2_buffer *vb; > + > + q->req_buf_cnt = 0; > + list_for_each_entry(vb, >queued_list, queued_entry) { > + if (vb->request == req) > + ++q->req_buf_cnt; > + } > + > + /* only consider the request if we actually have buffers for it */ > + if (q->req_buf_cnt == 0) > + return; > + > + q->cur_req = req; > + > + /* If not streaming yet, we will enqueue
Re: [PATCH v4 3/9] v4l: platform: Add Renesas CEU driver
On Tue, Jan 9, 2018 at 5:25 PM, Jacopo Mondiwrote: > Add driver for Renesas Capture Engine Unit (CEU). > --- /dev/null > +++ b/drivers/media/platform/renesas-ceu.c > @@ -0,0 +1,1648 @@ > +// SPDX-License-Identifier: GPL-2.0 > +MODULE_DESCRIPTION("Renesas CEU camera driver"); > +MODULE_AUTHOR("Jacopo Mondi "); > +MODULE_LICENSE("GPL"); Jacopo, the MODULE_LICENSE does not match the SPDX tag. Per module.h "GPL" means GPL-2.0 or later ;) It should be instead: > +MODULE_LICENSE("GPL v2"); ... to match your > +// SPDX-License-Identifier: GPL-2.0 I know this can be confusing, but updating the MODULE_LICENSE tags definitions in module.h to match SPDX tags is unlikely to happen as it would create mayhem for everyone and every module loader relying on this established convention. -- Cordially Philippe Ombredanne
Re: [PATCH v4 3/9] v4l: platform: Add Renesas CEU driver
Hi Geert, On Friday, 12 January 2018 11:01:18 EET Geert Uytterhoeven wrote: > On Fri, Jan 12, 2018 at 12:12 AM, Laurent Pinchart wrote: > > On Tuesday, 9 January 2018 18:25:25 EET Jacopo Mondi wrote: > >> Add driver for Renesas Capture Engine Unit (CEU). > >> > >> The CEU interface supports capturing 'data' (YUV422) and 'images' > >> (NV[12|21|16|61]). > >> > >> This driver aims to replace the soc_camera-based sh_mobile_ceu one. > >> > >> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ > >> platform GR-Peach. > >> > >> Tested with ov7725 camera sensor on SH4 platform Migo-R. > >> > >> Signed-off-by: Jacopo Mondi> >> --- > >> > >> drivers/media/platform/Kconfig |9 + > >> drivers/media/platform/Makefile |1 + > >> drivers/media/platform/renesas-ceu.c | 1648 > >> 3 files changed, 1658 insertions(+) > >> > >> create mode 100644 drivers/media/platform/renesas-ceu.c > > > > [snip] > > > >> diff --git a/drivers/media/platform/renesas-ceu.c > >> b/drivers/media/platform/renesas-ceu.c new file mode 100644 > >> index 000..d261704 > >> --- /dev/null > >> +++ b/drivers/media/platform/renesas-ceu.c > >> @@ -0,0 +1,1648 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > > > > It was recently brought to my attention that SPDX headers should use > > either GPL-2.0-only or GPL-2.0-or-later, no the ambiguous GPL-2.0. Could > > you please update all patches in this series ? > > IMHO it's a bit premature to do that. > As long as Documentation/process/license-rules.rst isn't updated, I wouldn't > follow this change. > > See also https://www.spinics.net/lists/linux-xfs/msg14536.html Thank you for bringing this to my attention. I agree with you. -- Regards, Laurent Pinchart
Re: [Linaro-mm-sig] [PATCH] dma-buf: add some lockdep asserts to the reservation object implementation
Am Donnerstag, den 11.01.2018, 11:54 +0100 schrieb Christian König: > Yeah, somehow missed that one. > > The patch looks mostly good, except for reservation_object_get_excl(). > > For that one an RCU protection is usually sufficient, so annotating it > with reservation_object_assert_held() sounds incorrect to me. Ah, you are correct. I was confused about this one as reservation_object_get_excl_rcu() exists and and the doc above reservation_object_get_excl() states "The obj->lock must be held.", which is misleading for the read-only case. I'll send a v2 with that fixed. Regards, Lucas > Regards, > Christian. > > Am 11.01.2018 um 11:43 schrieb Lucas Stach: > > Did this fall through the cracks over the holidays? It really has made > > my work much easier while reworking some of the reservation object > > handling in etnaviv and I think it might benefit others. > > > > Regards, > > Lucas > > > > Am Freitag, den 01.12.2017, 12:12 +0100 schrieb Lucas Stach: > > > This adds lockdep asserts to the reservation functions which state in > > > their > > > documentation that obj->lock must be held. Allows builds with > > > PROVE_LOCKING > > > enabled to check that the locking requirements are met. > > > > > > > Signed-off-by: Lucas Stach> > > > > > --- > > > drivers/dma-buf/reservation.c | 8 > > > include/linux/reservation.h | 2 ++ > > > 2 files changed, 10 insertions(+) > > > > > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c > > > index b44d9d7db347..accd398e2ea6 100644 > > > --- a/drivers/dma-buf/reservation.c > > > +++ b/drivers/dma-buf/reservation.c > > > @@ -71,6 +71,8 @@ int reservation_object_reserve_shared(struct > > > reservation_object *obj) > > > > > > > > struct reservation_object_list *fobj, *old; > > > > u32 max; > > > > > > > > > > + reservation_object_assert_held(obj); > > > > > > + > > > > old = reservation_object_get_list(obj); > > > > > > > > > > if (old && old->shared_max) { > > > > > > @@ -211,6 +213,8 @@ void reservation_object_add_shared_fence(struct > > > reservation_object *obj, > > > { > > > > struct reservation_object_list *old, *fobj = obj->staged; > > > > > > > > > > + reservation_object_assert_held(obj); > > > > > > + > > > > > > > > old = reservation_object_get_list(obj); > > > > obj->staged = NULL; > > > > > > > > > @@ -236,6 +240,8 @@ void reservation_object_add_excl_fence(struct > > > reservation_object *obj, > > > > > > > > struct reservation_object_list *old; > > > > u32 i = 0; > > > > > > > > > > + reservation_object_assert_held(obj); > > > > > > + > > > > > > > > old = reservation_object_get_list(obj); > > > > > > > > if (old) > > > > i = old->shared_count; > > > > > > @@ -276,6 +282,8 @@ int reservation_object_copy_fences(struct > > > reservation_object *dst, > > > > > > > > size_t size; > > > > unsigned i; > > > > > > > > > > + reservation_object_assert_held(dst); > > > > > > + > > > > > > > > rcu_read_lock(); > > > > src_list = rcu_dereference(src->fence); > > > > > > > > > diff --git a/include/linux/reservation.h b/include/linux/reservation.h > > > index 21fc84d82d41..55e7318800fd 100644 > > > --- a/include/linux/reservation.h > > > +++ b/include/linux/reservation.h > > > @@ -212,6 +212,8 @@ reservation_object_unlock(struct reservation_object > > > *obj) > > > static inline struct dma_fence * > > > reservation_object_get_excl(struct reservation_object *obj) > > > { > > > > + reservation_object_assert_held(obj); > > > > > > + > > > > > > > > return rcu_dereference_protected(obj->fence_excl, > > > > reservation_object_held(obj)); > > > > > > } > > > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > >
Re: [Linaro-mm-sig] [PATCH] dma-buf: add some lockdep asserts to the reservation object implementation
Did this fall through the cracks over the holidays? It really has made my work much easier while reworking some of the reservation object handling in etnaviv and I think it might benefit others. Regards, Lucas Am Freitag, den 01.12.2017, 12:12 +0100 schrieb Lucas Stach: > This adds lockdep asserts to the reservation functions which state in their > documentation that obj->lock must be held. Allows builds with PROVE_LOCKING > enabled to check that the locking requirements are met. > > > Signed-off-by: Lucas Stach> --- > drivers/dma-buf/reservation.c | 8 > include/linux/reservation.h | 2 ++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c > index b44d9d7db347..accd398e2ea6 100644 > --- a/drivers/dma-buf/reservation.c > +++ b/drivers/dma-buf/reservation.c > @@ -71,6 +71,8 @@ int reservation_object_reserve_shared(struct > reservation_object *obj) > > struct reservation_object_list *fobj, *old; > > u32 max; > > > + reservation_object_assert_held(obj); > + > > old = reservation_object_get_list(obj); > > > if (old && old->shared_max) { > @@ -211,6 +213,8 @@ void reservation_object_add_shared_fence(struct > reservation_object *obj, > { > > struct reservation_object_list *old, *fobj = obj->staged; > > > + reservation_object_assert_held(obj); > + > > old = reservation_object_get_list(obj); > > obj->staged = NULL; > > @@ -236,6 +240,8 @@ void reservation_object_add_excl_fence(struct > reservation_object *obj, > > struct reservation_object_list *old; > > u32 i = 0; > > > + reservation_object_assert_held(obj); > + > > old = reservation_object_get_list(obj); > > if (old) > > i = old->shared_count; > @@ -276,6 +282,8 @@ int reservation_object_copy_fences(struct > reservation_object *dst, > > size_t size; > > unsigned i; > > > + reservation_object_assert_held(dst); > + > > rcu_read_lock(); > > src_list = rcu_dereference(src->fence); > > diff --git a/include/linux/reservation.h b/include/linux/reservation.h > index 21fc84d82d41..55e7318800fd 100644 > --- a/include/linux/reservation.h > +++ b/include/linux/reservation.h > @@ -212,6 +212,8 @@ reservation_object_unlock(struct reservation_object *obj) > static inline struct dma_fence * > reservation_object_get_excl(struct reservation_object *obj) > { > > + reservation_object_assert_held(obj); > + > > return rcu_dereference_protected(obj->fence_excl, > > reservation_object_held(obj)); > }
Re: [RFC PATCH 4/9] videodev2.h: Add request field to v4l2_buffer
On 12/15/17 08:56, Alexandre Courbot wrote: > From: Hans Verkuil> > When queuing buffers allow for passing the request ID that > should be associated with this buffer. > > Signed-off-by: Hans Verkuil > [acour...@chromium.org: make request ID 32-bit] > Signed-off-by: Alexandre Courbot > --- > drivers/media/usb/cpia2/cpia2_v4l.c | 2 +- > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 --- > drivers/media/v4l2-core/v4l2-ioctl.c | 4 ++-- > drivers/media/v4l2-core/videobuf2-v4l2.c | 3 ++- > include/media/videobuf2-v4l2.h| 2 ++ > include/uapi/linux/videodev2.h| 3 ++- > 6 files changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c > b/drivers/media/usb/cpia2/cpia2_v4l.c > index 3dedd83f0b19..7217dde95a8a 100644 > --- a/drivers/media/usb/cpia2/cpia2_v4l.c > +++ b/drivers/media/usb/cpia2/cpia2_v4l.c > @@ -948,7 +948,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, > struct v4l2_buffer *buf) > buf->sequence = cam->buffers[buf->index].seq; > buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer; > buf->length = cam->frame_size; > - buf->reserved2 = 0; > + buf->request = 0; > buf->reserved = 0; > memset(>timecode, 0, sizeof(buf->timecode)); > > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > index 821f2aa299ae..94f07c3b0b53 100644 > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c > @@ -370,7 +370,7 @@ struct v4l2_buffer32 { > __s32 fd; > } m; > __u32 length; > - __u32 reserved2; > + __u32 request; > __u32 reserved; > }; > > @@ -438,7 +438,8 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, > struct v4l2_buffer32 __user > get_user(kp->type, >type) || > get_user(kp->flags, >flags) || > get_user(kp->memory, >memory) || > - get_user(kp->length, >length)) > + get_user(kp->length, >length) || > + get_user(kp->request, >request)) > return -EFAULT; > > if (V4L2_TYPE_IS_OUTPUT(kp->type)) > @@ -533,7 +534,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, > struct v4l2_buffer32 __user > put_user(kp->timestamp.tv_usec, >timestamp.tv_usec) || > copy_to_user(>timecode, >timecode, sizeof(struct > v4l2_timecode)) || > put_user(kp->sequence, >sequence) || > - put_user(kp->reserved2, >reserved2) || > + put_user(kp->request, >request) || > put_user(kp->reserved, >reserved) || > put_user(kp->length, >length)) > return -EFAULT; > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c > b/drivers/media/v4l2-core/v4l2-ioctl.c > index ec4ecd5aa8bf..8d041247e97f 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -437,13 +437,13 @@ static void v4l_print_buffer(const void *arg, bool > write_only) > const struct v4l2_plane *plane; > int i; > > - pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, flags=0x%08x, > field=%s, sequence=%d, memory=%s", > + pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, request=%u, > flags=0x%08x, field=%s, sequence=%d, memory=%s", > p->timestamp.tv_sec / 3600, > (int)(p->timestamp.tv_sec / 60) % 60, > (int)(p->timestamp.tv_sec % 60), > (long)p->timestamp.tv_usec, > p->index, > - prt_names(p->type, v4l2_type_names), > + prt_names(p->type, v4l2_type_names), p->request, > p->flags, prt_names(p->field, v4l2_field_names), > p->sequence, prt_names(p->memory, v4l2_memory_names)); > > diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c > b/drivers/media/v4l2-core/videobuf2-v4l2.c > index 0c0669976bdc..bde7b8a3a303 100644 > --- a/drivers/media/v4l2-core/videobuf2-v4l2.c > +++ b/drivers/media/v4l2-core/videobuf2-v4l2.c > @@ -203,7 +203,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, > void *pb) > b->timestamp = ns_to_timeval(vb->timestamp); > b->timecode = vbuf->timecode; > b->sequence = vbuf->sequence; > - b->reserved2 = 0; > + b->request = vbuf->request; > b->reserved = 0; > > if (q->is_multiplanar) { > @@ -320,6 +320,7 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, > } > vb->timestamp = 0; > vbuf->sequence = 0; > + vbuf->request = b->request; > > if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) { > if (b->memory == VB2_MEMORY_USERPTR) { >
Re: [RFC PATCH 1/9] media: add request API core and UAPI
Hi Alexandre, A quick review: I'm primarily concentrating on the uAPI as that is the most critical part to get right at this stage. On 12/15/17 08:56, Alexandre Courbot wrote: > The request API provides a way to group buffers and device parameters > into units of work to be queued and executed. This patch introduces the > UAPI and core framework. > > This patch is based on the previous work by Laurent Pinchart. The core > has changed considerably, but the UAPI is mostly untouched. > > Signed-off-by: Alexandre Courbot> --- > drivers/media/Makefile | 3 +- > drivers/media/media-device.c | 6 + > drivers/media/media-request.c| 390 > +++ > drivers/media/v4l2-core/v4l2-ioctl.c | 2 +- > include/media/media-device.h | 3 + > include/media/media-entity.h | 6 + > include/media/media-request.h| 269 > include/uapi/linux/media.h | 11 + > 8 files changed, 688 insertions(+), 2 deletions(-) > create mode 100644 drivers/media/media-request.c > create mode 100644 include/media/media-request.h > > diff --git a/include/media/media-request.h b/include/media/media-request.h > new file mode 100644 > index ..ead7fd8898c4 > --- /dev/null > +++ b/include/media/media-request.h > @@ -0,0 +1,269 @@ > +/* > + * Generic request queue. > + * > + * Copyright (C) 2017, The Chromium OS Authors. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#ifndef _MEDIA_REQUEST_H > +#define _MEDIA_REQUEST_H > + > +#include > +#include > +#include > + > +struct media_device; > +struct media_request_queue; > +struct media_request_cmd; > +struct media_entity; > +struct media_request_entity_data; > + > +#ifdef CONFIG_MEDIA_CONTROLLER > + > +enum media_request_state { > + MEDIA_REQUEST_STATE_IDLE, > + MEDIA_REQUEST_STATE_QUEUED, > + MEDIA_REQUEST_STATE_COMPLETE, COMPLETE -> COMPLETED > + MEDIA_REQUEST_STATE_DELETED, > +}; Regards, Hans
Re: MT9M131 on I.MX6DL CSI color issue
On Fri, 12 Jan 2018 10:58:40 +0100 Anatolij Gustschin ag...@denx.de wrote: ... > gst-launch v4l2src device=/dev/video4 num-buffers=1 ! \ >videoparse format=5 width=1280 height=1024 framerate=25/1 ! \ >jpegenc ! filesink location=capture1.jpeg I forgot the videoconvert, sorry. Try gst-launch v4l2src device=/dev/video4 num-buffers=1 ! \ filesink location=frame.raw gst-launch filesrc num-buffers=1 location=frame.raw ! \ videoparse format=5 width=1280 height=1024 framerate=25/1 ! \ videoconvert ! jpegenc ! filesink location=capture1.jpeg Anatolij
Re: [PATCH v2 00/19] prevent bounds-check bypass via speculative execution
Do you think that the appropriate patches could be copied to the appropriate people please? On Thu, Jan 11, 2018 at 04:46:24PM -0800, Dan Williams wrote: > Changes since v1 [1]: > * fixup the ifence definition to use alternative_2 per recent AMD > changes in tip/x86/pti (Tom) > > * drop 'nospec_ptr' (Linus, Mark) > > * rename 'nospec_array_ptr' to 'array_ptr' (Alexei) > > * rename 'nospec_barrier' to 'ifence' (Peter, Ingo) > > * clean up occasions of 'variable assignment in if()' (Sergei, Stephen) > > * make 'array_ptr' use a mask instead of an architectural ifence by > default (Linus, Alexei) > > * provide a command line and compile-time opt-in to the ifence > mechanism, if an architecture provides 'ifence_array_ptr'. > > * provide an optimized mask generation helper, 'array_ptr_mask', for > x86 (Linus) > > * move 'get_user' hardening from '__range_not_ok' to '__uaccess_begin' > (Linus) > > * drop "Thermal/int340x: prevent bounds-check..." since userspace does > not have arbitrary control over the 'trip' index (Srinivas) > > * update the changelog of "net: mpls: prevent bounds-check..." and keep > it in the series to continue the debate about Spectre hygiene patches. > (Eric). > > * record a reviewed-by from Laurent on "[media] uvcvideo: prevent > bounds-check..." > > * update the cover letter > > [1]: https://lwn.net/Articles/743376/ > > --- > > Quoting Mark's original RFC: > > "Recently, Google Project Zero discovered several classes of attack > against speculative execution. One of these, known as variant-1, allows > explicit bounds checks to be bypassed under speculation, providing an > arbitrary read gadget. Further details can be found on the GPZ blog [2] > and the Documentation patch in this series." > > This series incorporates Mark Rutland's latest ARM changes and adds > the x86 specific implementation of 'ifence_array_ptr'. That ifence > based approach is provided as an opt-in fallback, but the default > mitigation, '__array_ptr', uses a 'mask' approach that removes > conditional branches instructions, and otherwise aims to redirect > speculation to use a NULL pointer rather than a user controlled value. > > The mask is generated by the following from Alexei, and Linus: > > mask = ~(long)(_i | (_s - 1 - _i)) >> (BITS_PER_LONG - 1); > > ...and Linus provided an optimized mask generation helper for x86: > > asm ("cmpq %1,%2; sbbq %0,%0;" > :"=r" (mask) > :"r"(sz),"r" (idx) > :"cc"); > > The 'array_ptr' mechanism can be switched between 'mask' and 'ifence' > via the spectre_v1={mask,ifence} command line option, and the > compile-time default is set by selecting either CONFIG_SPECTRE1_MASK or > CONFIG_SPECTRE1_IFENCE. > > The 'array_ptr' infrastructure is the primary focus this patch set. The > individual patches that perform 'array_ptr' conversions are a point in > time (i.e. earlier kernel, early analysis tooling, x86 only etc...) > start at finding some of these gadgets. > > Another consideration for reviewing these patches is the 'hygiene' > argument. When a patch refers to hygiene it is concerned with stopping > speculation on an unconstrained or insufficiently constrained pointer > value under userspace control. That by itself is not sufficient for > attack (per current understanding) [3], but it is a necessary > pre-condition. So 'hygiene' refers to cleaning up those suspect > pointers regardless of whether they are usable as a gadget. > > These patches are also be available via the 'nospec-v2' git branch > here: > > git://git.kernel.org/pub/scm/linux/kernel/git/djbw/linux nospec-v2 > > Note that the BPF fix for Spectre variant1 is merged in the bpf.git > tree [4], and is not included in this branch. > > [2]: > https://googleprojectzero.blogspot.co.uk/2018/01/reading-privileged-memory-with-side.html > [3]: https://spectreattack.com/spectre.pdf > [4]: > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=b2157399cc98 > > --- > > Dan Williams (16): > x86: implement ifence() > x86: implement ifence_array_ptr() and array_ptr_mask() > asm-generic/barrier: mask speculative execution flows > x86: introduce __uaccess_begin_nospec and ASM_IFENCE > x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths > ipv6: prevent bounds-check bypass via speculative execution > ipv4: prevent bounds-check bypass via speculative execution > vfs, fdtable: prevent bounds-check bypass via speculative execution > userns: prevent bounds-check bypass via speculative execution > udf: prevent bounds-check bypass via speculative execution > [media] uvcvideo: prevent bounds-check bypass via speculative execution > carl9170: prevent bounds-check bypass via speculative execution > p54: prevent bounds-check bypass via speculative execution > qla2xxx: prevent bounds-check bypass via speculative execution >
Re: MT9M131 on I.MX6DL CSI color issue
On Fri, 12 Jan 2018 01:16:03 +0100 Florian Boor florian.b...@kernelconcepts.de wrote: ... >Basically it works pretty well apart from the really strange colors. I guess >its >some YUV vs. RGB issue or similar. Here [1] is an example generated with the >following command. > >gst-launch v4l2src device=/dev/video4 num-buffers=1 ! jpegenc ! filesink >location=capture1.jpeg > >Apart from the colors everything is fine. >I'm pretty sure I have not seen such an effect before - what might be wrong >here? You need conversion to RGB before JPEG encoding. Try with gst-launch v4l2src device=/dev/video4 num-buffers=1 ! \ videoparse format=5 width=1280 height=1024 framerate=25/1 ! \ jpegenc ! filesink location=capture1.jpeg For "format" codes see gst-inspect-1.0 videoparse. HTH, Anatolij
[PATCH 2/3] drm/amdgpu: add amdgpu_pasid_free_delayed v2
Free up a pasid after all fences signaled. v2: also handle the case when we can't allocate a fence array. Signed-off-by: Christian König--- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 82 + drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h | 2 + 2 files changed, 84 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c index 5248a3232aff..842caa5ed73b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c @@ -40,6 +40,12 @@ */ static DEFINE_IDA(amdgpu_pasid_ida); +/* Helper to free pasid from a fence callback */ +struct amdgpu_pasid_cb { + struct dma_fence_cb cb; + unsigned int pasid; +}; + /** * amdgpu_pasid_alloc - Allocate a PASID * @bits: Maximum width of the PASID in bits, must be at least 1 @@ -75,6 +81,82 @@ void amdgpu_pasid_free(unsigned int pasid) ida_simple_remove(_pasid_ida, pasid); } +static void amdgpu_pasid_free_cb(struct dma_fence *fence, +struct dma_fence_cb *_cb) +{ + struct amdgpu_pasid_cb *cb = + container_of(_cb, struct amdgpu_pasid_cb, cb); + + amdgpu_pasid_free(cb->pasid); + dma_fence_put(fence); + kfree(cb); +} + +/** + * amdgpu_pasid_free_delayed - free pasid when fences signal + * + * @resv: reservation object with the fences to wait for + * @pasid: pasid to free + * + * Free the pasid only after all the fences in resv are signaled. + */ +void amdgpu_pasid_free_delayed(struct reservation_object *resv, + unsigned int pasid) +{ + struct dma_fence *fence, **fences; + struct amdgpu_pasid_cb *cb; + unsigned count; + int r; + + r = reservation_object_get_fences_rcu(resv, NULL, , ); + if (r) + goto fallback; + + if (count == 0) { + amdgpu_pasid_free(pasid); + return; + } + + if (count == 1) { + fence = fences[0]; + kfree(fences); + } else { + uint64_t context = dma_fence_context_alloc(1); + struct dma_fence_array *array; + + array = dma_fence_array_create(count, fences, context, + 1, false); + if (!array) { + kfree(fences); + goto fallback; + } + fence = >base; + } + + cb = kmalloc(sizeof(*cb), GFP_KERNEL); + if (!cb) { + /* Last resort when we are OOM */ + dma_fence_wait(fence, false); + dma_fence_put(fence); + amdgpu_pasid_free(pasid); + } else { + cb->pasid = pasid; + if (dma_fence_add_callback(fence, >cb, + amdgpu_pasid_free_cb)) + amdgpu_pasid_free_cb(fence, >cb); + } + + return; + +fallback: + /* Not enough memory for the delayed delete, as last resort +* block for all the fences to complete. +*/ + reservation_object_wait_timeout_rcu(resv, true, false, + MAX_SCHEDULE_TIMEOUT); + amdgpu_pasid_free(pasid); +} + /* * VMID manager * diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h index ad931fa570b3..38f37c16fc5e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h @@ -69,6 +69,8 @@ struct amdgpu_vmid_mgr { int amdgpu_pasid_alloc(unsigned int bits); void amdgpu_pasid_free(unsigned int pasid); +void amdgpu_pasid_free_delayed(struct reservation_object *resv, + unsigned int pasid); bool amdgpu_vmid_had_gpu_reset(struct amdgpu_device *adev, struct amdgpu_vmid *id); -- 2.14.1
[PATCH 1/3] dma-buf: make returning the exclusive fence optional
Change reservation_object_get_fences_rcu to make the exclusive fence pointer optional. If not specified the exclusive fence is put into the fence array as well. This is helpful for a couple of cases where we need all fences in a single array. Signed-off-by: Christian König--- drivers/dma-buf/reservation.c | 31 ++- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index b759a569b7b8..461afa9febd4 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences); * @pshared: the array of shared fence ptrs returned (array is krealloc'd to * the required size, and must be freed by caller) * - * RETURNS - * Zero or -errno + * Retrieve all fences from the reservation object. If the pointer for the + * exclusive fence is not specified the fence is put into the array of the + * shared fences as well. Returns either zero or -ENOMEM. */ int reservation_object_get_fences_rcu(struct reservation_object *obj, struct dma_fence **pfence_excl, @@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, do { struct reservation_object_list *fobj; - unsigned seq; - unsigned int i; + unsigned int i, seq; + size_t sz = 0; shared_count = i = 0; @@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, goto unlock; fobj = rcu_dereference(obj->fence); - if (fobj) { + if (fobj) + sz += sizeof(*shared) * fobj->shared_max; + + if (!pfence_excl && fence_excl) + sz += sizeof(*shared); + + if (sz) { struct dma_fence **nshared; - size_t sz = sizeof(*shared) * fobj->shared_max; nshared = krealloc(shared, sz, GFP_NOWAIT | __GFP_NOWARN); @@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, break; } shared = nshared; - shared_count = fobj->shared_count; - + shared_count = fobj ? fobj->shared_count : 0; for (i = 0; i < shared_count; ++i) { shared[i] = rcu_dereference(fobj->shared[i]); if (!dma_fence_get_rcu(shared[i])) break; } + + if (!pfence_excl && fence_excl) { + shared[i] = fence_excl; + fence_excl = NULL; + ++i; + ++shared_count; + } } if (i != shared_count || read_seqcount_retry(>seq, seq)) { @@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, *pshared_count = shared_count; *pshared = shared; - *pfence_excl = fence_excl; + if (pfence_excl) + *pfence_excl = fence_excl; return ret; } -- 2.14.1
[PATCH 3/3] drm/amdgpu: always allocate a PASIDs for each VM v2
Start to always allocate a pasid for each VM. v2: use dev_warn when we run out of PASIDs Signed-off-by: Christian König--- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 43 ++--- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 5773a581761b..a108b30d8186 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -805,7 +805,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv) { struct amdgpu_device *adev = dev->dev_private; struct amdgpu_fpriv *fpriv; - int r; + int r, pasid; file_priv->driver_priv = NULL; @@ -819,28 +819,25 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv) goto out_suspend; } - r = amdgpu_vm_init(adev, >vm, - AMDGPU_VM_CONTEXT_GFX, 0); - if (r) { - kfree(fpriv); - goto out_suspend; + pasid = amdgpu_pasid_alloc(16); + if (pasid < 0) { + dev_warn(adev->dev, "No more PASIDs available!"); + pasid = 0; } + r = amdgpu_vm_init(adev, >vm, AMDGPU_VM_CONTEXT_GFX, pasid); + if (r) + goto error_pasid; fpriv->prt_va = amdgpu_vm_bo_add(adev, >vm, NULL); if (!fpriv->prt_va) { r = -ENOMEM; - amdgpu_vm_fini(adev, >vm); - kfree(fpriv); - goto out_suspend; + goto error_vm; } if (amdgpu_sriov_vf(adev)) { r = amdgpu_map_static_csa(adev, >vm, >csa_va); - if (r) { - amdgpu_vm_fini(adev, >vm); - kfree(fpriv); - goto out_suspend; - } + if (r) + goto error_vm; } mutex_init(>bo_list_lock); @@ -849,6 +846,16 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv) amdgpu_ctx_mgr_init(>ctx_mgr); file_priv->driver_priv = fpriv; + goto out_suspend; + +error_vm: + amdgpu_vm_fini(adev, >vm); + +error_pasid: + if (pasid) + amdgpu_pasid_free(pasid); + + kfree(fpriv); out_suspend: pm_runtime_mark_last_busy(dev->dev); @@ -871,6 +878,8 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev, struct amdgpu_device *adev = dev->dev_private; struct amdgpu_fpriv *fpriv = file_priv->driver_priv; struct amdgpu_bo_list *list; + struct amdgpu_bo *pd; + unsigned int pasid; int handle; if (!fpriv) @@ -895,7 +904,13 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev, amdgpu_bo_unreserve(adev->virt.csa_obj); } + pasid = fpriv->vm.pasid; + pd = amdgpu_bo_ref(fpriv->vm.root.base.bo); + amdgpu_vm_fini(adev, >vm); + if (pasid) + amdgpu_pasid_free_delayed(pd->tbo.resv, pasid); + amdgpu_bo_unref(); idr_for_each_entry(>bo_list_handles, list, handle) amdgpu_bo_list_free(list); -- 2.14.1
Re: [RFT PATCH v3 6/6] uvcvideo: Move decode processing to process context
Hi Kieran, On Fri, 12 Jan 2018, Kieran Bingham wrote: > Newer high definition cameras, and cameras with multiple lenses such as > the range of stereo-vision cameras now available have ever increasing > data rates. > > The inclusion of a variable length packet header in URB packets mean > that we must memcpy the frame data out to our destination 'manually'. > This can result in data rates of up to 2 gigabits per second being > processed. > > To improve efficiency, and maximise throughput, handle the URB decode > processing through a work queue to move it from interrupt context, and > allow multiple processors to work on URBs in parallel. > > Signed-off-by: Kieran Bingham> > --- > v2: > - Lock full critical section of usb_submit_urb() > > v3: > - Fix race on submitting uvc_video_decode_data_work() to work queue. > - Rename uvc_decode_op -> uvc_copy_op (Generic to encode/decode) > - Rename decodes -> copy_operations > - Don't queue work if there is no async task > - obtain copy op structure directly in uvc_video_decode_data() > - uvc_video_decode_data_work() -> uvc_video_copy_data_work() > --- > drivers/media/usb/uvc/uvc_queue.c | 12 +++- > drivers/media/usb/uvc/uvc_video.c | 116 +++ > drivers/media/usb/uvc/uvcvideo.h | 24 ++- > 3 files changed, 138 insertions(+), 14 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_queue.c > b/drivers/media/usb/uvc/uvc_queue.c > index 5a9987e547d3..598087eeb5c2 100644 > --- a/drivers/media/usb/uvc/uvc_queue.c > +++ b/drivers/media/usb/uvc/uvc_queue.c > @@ -179,10 +179,22 @@ static void uvc_stop_streaming(struct vb2_queue *vq) > struct uvc_video_queue *queue = vb2_get_drv_priv(vq); > struct uvc_streaming *stream = uvc_queue_to_stream(queue); > > + /* Prevent new buffers coming in. */ > + spin_lock_irq(>irqlock); > + queue->flags |= UVC_QUEUE_STOPPING; > + spin_unlock_irq(>irqlock); > + > + /* > + * All pending work should be completed before disabling the stream, as > + * all URBs will be free'd during uvc_video_enable(s, 0). > + */ > + flush_workqueue(stream->async_wq); What if we manage to get one last URB here, then... > + > uvc_video_enable(stream, 0); > > spin_lock_irq(>irqlock); > uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR); > + queue->flags &= ~UVC_QUEUE_STOPPING; > spin_unlock_irq(>irqlock); > } > > diff --git a/drivers/media/usb/uvc/uvc_video.c > b/drivers/media/usb/uvc/uvc_video.c > index 3878bec3276e..fb6b5af17380 100644 > --- a/drivers/media/usb/uvc/uvc_video.c > +++ b/drivers/media/usb/uvc/uvc_video.c [snip] > + /* > + * When the stream is stopped, all URBs are freed as part of the call to > + * uvc_stop_streaming() and must not be handled asynchronously. In that > + * event we can safely complete the packet work directly in this > + * context, without resubmitting the URB. > + */ > + spin_lock_irqsave(>irqlock, flags); > + if (!(queue->flags & UVC_QUEUE_STOPPING)) { > + INIT_WORK(_urb->work, uvc_video_copy_data_work); > + queue_work(stream->async_wq, _urb->work); > + } else { > + uvc_video_copy_packets(uvc_urb); Can it not happen, that if the stream is currently being stopped, the queue has been flushed, possibly the previous URB or a couple of them don't get decoded, but you do decode this one, creating a corrupted frame? Wouldn't it be better to just drop this URB too? > } > + spin_unlock_irqrestore(>irqlock, flags); > } > > /* Thanks Guennadi
[RFT PATCH v3 2/6] uvcvideo: Convert decode functions to use new context structure
The URB completion handlers currently reference the stream context. Now that each URB has its own context structure, convert the decode (and one encode) functions to utilise this context for URB management. Signed-off-by: Kieran BinghamReviewed-by: Laurent Pinchart --- v2: - fix checkpatch warning (pre-existing in code) --- drivers/media/usb/uvc/uvc_isight.c | 4 +++- drivers/media/usb/uvc/uvc_video.c | 32 --- drivers/media/usb/uvc/uvcvideo.h | 8 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_isight.c b/drivers/media/usb/uvc/uvc_isight.c index 8510e7259e76..433b8b4f96e2 100644 --- a/drivers/media/usb/uvc/uvc_isight.c +++ b/drivers/media/usb/uvc/uvc_isight.c @@ -99,9 +99,11 @@ static int isight_decode(struct uvc_video_queue *queue, struct uvc_buffer *buf, return 0; } -void uvc_video_decode_isight(struct urb *urb, struct uvc_streaming *stream, +void uvc_video_decode_isight(struct uvc_urb *uvc_urb, struct uvc_buffer *buf) { + struct urb *urb = uvc_urb->urb; + struct uvc_streaming *stream = uvc_urb->stream; int ret, i; for (i = 0; i < urb->number_of_packets; ++i) { diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index e57c5f52c73b..92bd0952a66e 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -1153,9 +1153,11 @@ static void uvc_video_validate_buffer(const struct uvc_streaming *stream, /* * Completion handler for video URBs. */ -static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream, - struct uvc_buffer *buf) +static void uvc_video_decode_isoc(struct uvc_urb *uvc_urb, + struct uvc_buffer *buf) { + struct urb *urb = uvc_urb->urb; + struct uvc_streaming *stream = uvc_urb->stream; u8 *mem; int ret, i; @@ -1199,9 +1201,11 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream, } } -static void uvc_video_decode_bulk(struct urb *urb, struct uvc_streaming *stream, - struct uvc_buffer *buf) +static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb, + struct uvc_buffer *buf) { + struct urb *urb = uvc_urb->urb; + struct uvc_streaming *stream = uvc_urb->stream; u8 *mem; int len, ret; @@ -1266,9 +1270,12 @@ static void uvc_video_decode_bulk(struct urb *urb, struct uvc_streaming *stream, } } -static void uvc_video_encode_bulk(struct urb *urb, struct uvc_streaming *stream, - struct uvc_buffer *buf) +static void uvc_video_encode_bulk(struct uvc_urb *uvc_urb, + struct uvc_buffer *buf) { + struct urb *urb = uvc_urb->urb; + struct uvc_streaming *stream = uvc_urb->stream; + u8 *mem = urb->transfer_buffer; int len = stream->urb_size, ret; @@ -1311,7 +1318,8 @@ static void uvc_video_encode_bulk(struct urb *urb, struct uvc_streaming *stream, static void uvc_video_complete(struct urb *urb) { - struct uvc_streaming *stream = urb->context; + struct uvc_urb *uvc_urb = urb->context; + struct uvc_streaming *stream = uvc_urb->stream; struct uvc_video_queue *queue = >queue; struct uvc_buffer *buf = NULL; unsigned long flags; @@ -1341,7 +1349,7 @@ static void uvc_video_complete(struct urb *urb) queue); spin_unlock_irqrestore(>irqlock, flags); - stream->decode(urb, stream, buf); + stream->decode(uvc_urb, buf); if ((ret = usb_submit_urb(urb, GFP_ATOMIC)) < 0) { uvc_printk(KERN_ERR, "Failed to resubmit video URB (%d).\n", @@ -1419,6 +1427,8 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming *stream, uvc_free_urb_buffers(stream); break; } + + uvc_urb->stream = stream; } if (i == UVC_URBS) { @@ -1517,7 +1527,7 @@ static int uvc_init_video_isoc(struct uvc_streaming *stream, } urb->dev = stream->dev->udev; - urb->context = stream; + urb->context = uvc_urb; urb->pipe = usb_rcvisocpipe(stream->dev->udev, ep->desc.bEndpointAddress); #ifndef CONFIG_DMA_NONCOHERENT @@ -1584,8 +1594,8 @@ static int uvc_init_video_bulk(struct uvc_streaming *stream, return -ENOMEM; } - usb_fill_bulk_urb(urb, stream->dev->udev, pipe, uvc_urb->buffer, - size, uvc_video_complete, stream); + usb_fill_bulk_urb(urb, stream->dev->udev, pipe, uvc_urb->buffer, + size, uvc_video_complete, uvc_urb); #ifndef CONFIG_DMA_NONCOHERENT
[RFT PATCH v3 3/6] uvcvideo: Protect queue internals with helper
The URB completion operation obtains the current buffer by reading directly into the queue internal interface. Protect this queue abstraction by providing a helper uvc_queue_get_current_buffer() which can be used by both the decode task, and the uvc_queue_next_buffer() functions. Signed-off-by: Kieran BinghamReviewed-by: Laurent Pinchart --- v2: - Fix coding style of conditional statements --- drivers/media/usb/uvc/uvc_queue.c | 33 +++- drivers/media/usb/uvc/uvc_video.c | 7 +-- drivers/media/usb/uvc/uvcvideo.h | 2 ++- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c index c8d78b2f3de4..4a581d631525 100644 --- a/drivers/media/usb/uvc/uvc_queue.c +++ b/drivers/media/usb/uvc/uvc_queue.c @@ -399,6 +399,33 @@ void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect) spin_unlock_irqrestore(>irqlock, flags); } +/* + * uvc_queue_get_current_buffer: Obtain the current working output buffer + * + * Buffers may span multiple packets, and even URBs, therefore the active buffer + * remains on the queue until the EOF marker. + */ +static struct uvc_buffer * +__uvc_queue_get_current_buffer(struct uvc_video_queue *queue) +{ + if (list_empty(>irqqueue)) + return NULL; + + return list_first_entry(>irqqueue, struct uvc_buffer, queue); +} + +struct uvc_buffer *uvc_queue_get_current_buffer(struct uvc_video_queue *queue) +{ + struct uvc_buffer *nextbuf; + unsigned long flags; + + spin_lock_irqsave(>irqlock, flags); + nextbuf = __uvc_queue_get_current_buffer(queue); + spin_unlock_irqrestore(>irqlock, flags); + + return nextbuf; +} + struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue, struct uvc_buffer *buf) { @@ -415,11 +442,7 @@ struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue, spin_lock_irqsave(>irqlock, flags); list_del(>queue); - if (!list_empty(>irqqueue)) - nextbuf = list_first_entry(>irqqueue, struct uvc_buffer, - queue); - else - nextbuf = NULL; + nextbuf = __uvc_queue_get_current_buffer(queue); spin_unlock_irqrestore(>irqlock, flags); buf->state = buf->error ? UVC_BUF_STATE_ERROR : UVC_BUF_STATE_DONE; diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index 92bd0952a66e..3878bec3276e 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -1322,7 +1322,6 @@ static void uvc_video_complete(struct urb *urb) struct uvc_streaming *stream = uvc_urb->stream; struct uvc_video_queue *queue = >queue; struct uvc_buffer *buf = NULL; - unsigned long flags; int ret; switch (urb->status) { @@ -1343,11 +1342,7 @@ static void uvc_video_complete(struct urb *urb) return; } - spin_lock_irqsave(>irqlock, flags); - if (!list_empty(>irqqueue)) - buf = list_first_entry(>irqqueue, struct uvc_buffer, - queue); - spin_unlock_irqrestore(>irqlock, flags); + buf = uvc_queue_get_current_buffer(queue); stream->decode(uvc_urb, buf); diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 81a9a419a423..5caa1f4de3cb 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -692,6 +692,8 @@ extern int uvc_queue_streamon(struct uvc_video_queue *queue, extern int uvc_queue_streamoff(struct uvc_video_queue *queue, enum v4l2_buf_type type); extern void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect); +extern struct uvc_buffer * + uvc_queue_get_current_buffer(struct uvc_video_queue *queue); extern struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue, struct uvc_buffer *buf); extern int uvc_queue_mmap(struct uvc_video_queue *queue, -- git-series 0.9.1
[RFT PATCH v3 6/6] uvcvideo: Move decode processing to process context
Newer high definition cameras, and cameras with multiple lenses such as the range of stereo-vision cameras now available have ever increasing data rates. The inclusion of a variable length packet header in URB packets mean that we must memcpy the frame data out to our destination 'manually'. This can result in data rates of up to 2 gigabits per second being processed. To improve efficiency, and maximise throughput, handle the URB decode processing through a work queue to move it from interrupt context, and allow multiple processors to work on URBs in parallel. Signed-off-by: Kieran Bingham--- v2: - Lock full critical section of usb_submit_urb() v3: - Fix race on submitting uvc_video_decode_data_work() to work queue. - Rename uvc_decode_op -> uvc_copy_op (Generic to encode/decode) - Rename decodes -> copy_operations - Don't queue work if there is no async task - obtain copy op structure directly in uvc_video_decode_data() - uvc_video_decode_data_work() -> uvc_video_copy_data_work() --- drivers/media/usb/uvc/uvc_queue.c | 12 +++- drivers/media/usb/uvc/uvc_video.c | 116 +++ drivers/media/usb/uvc/uvcvideo.h | 24 ++- 3 files changed, 138 insertions(+), 14 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c index 5a9987e547d3..598087eeb5c2 100644 --- a/drivers/media/usb/uvc/uvc_queue.c +++ b/drivers/media/usb/uvc/uvc_queue.c @@ -179,10 +179,22 @@ static void uvc_stop_streaming(struct vb2_queue *vq) struct uvc_video_queue *queue = vb2_get_drv_priv(vq); struct uvc_streaming *stream = uvc_queue_to_stream(queue); + /* Prevent new buffers coming in. */ + spin_lock_irq(>irqlock); + queue->flags |= UVC_QUEUE_STOPPING; + spin_unlock_irq(>irqlock); + + /* +* All pending work should be completed before disabling the stream, as +* all URBs will be free'd during uvc_video_enable(s, 0). +*/ + flush_workqueue(stream->async_wq); + uvc_video_enable(stream, 0); spin_lock_irq(>irqlock); uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR); + queue->flags &= ~UVC_QUEUE_STOPPING; spin_unlock_irq(>irqlock); } diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index 3878bec3276e..fb6b5af17380 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -1058,21 +1058,74 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, return data[0]; } -static void uvc_video_decode_data(struct uvc_streaming *stream, +static void uvc_video_copy_packets(struct uvc_urb *uvc_urb) +{ + unsigned int i; + + for (i = 0; i < uvc_urb->async_operations; i++) { + struct uvc_copy_op *op = _urb->copy_operations[i]; + + memcpy(op->dst, op->src, op->len); + + /* Release reference taken on this buffer */ + uvc_queue_buffer_release(op->buf); + } +} + +/* + * uvc_video_decode_data_work: Asynchronous memcpy processing + * + * Perform memcpy tasks in process context, with completion handlers + * to return the URB, and buffer handles. + * + * The work submitter must pre-determine that the work is safe + */ +static void uvc_video_copy_data_work(struct work_struct *work) +{ + struct uvc_urb *uvc_urb = container_of(work, struct uvc_urb, work); + struct uvc_streaming *stream = uvc_urb->stream; + struct uvc_video_queue *queue = >queue; + int ret; + + uvc_video_copy_packets(uvc_urb); + + /* +* Prevent resubmitting URBs when shutting down to ensure that no new +* work item will be scheduled after uvc_stop_streaming() flushes the +* work queue. +*/ + spin_lock_irq(>irqlock); + if (!(queue->flags & UVC_QUEUE_STOPPING)) { + ret = usb_submit_urb(uvc_urb->urb, GFP_ATOMIC); + if (ret < 0) + uvc_printk(KERN_ERR, + "Failed to resubmit video URB (%d).\n", + ret); + } + spin_unlock_irq(>irqlock); +} + +static void uvc_video_decode_data(struct uvc_urb *uvc_urb, struct uvc_buffer *buf, const __u8 *data, int len) { - unsigned int maxlen, nbytes; - void *mem; + unsigned int active_op = uvc_urb->async_operations; + struct uvc_copy_op *decode = _urb->copy_operations[active_op]; + unsigned int maxlen; if (len <= 0) return; - /* Copy the video data to the buffer. */ maxlen = buf->length - buf->bytesused; - mem = buf->mem + buf->bytesused; - nbytes = min((unsigned int)len, maxlen); - memcpy(mem, data, nbytes); - buf->bytesused += nbytes; + + /* Take a buffer reference for async work */ + kref_get(>ref); + + decode->buf = buf; + decode->src = data;
[RFT PATCH v3 1/6] uvcvideo: Refactor URB descriptors
We currently store three separate arrays for each URB reference we hold. Objectify the data needed to track URBs into a single uvc_urb structure, allowing better object management and tracking of the URB. All accesses to the data pointers through stream, are converted to use a uvc_urb pointer for consistency. Signed-off-by: Kieran BinghamReviewed-by: Laurent Pinchart --- v2: - Re-describe URB context structure - Re-name uvc_urb->{urb_buffer,urb_dma}{buffer,dma} --- drivers/media/usb/uvc/uvc_video.c | 49 +++- drivers/media/usb/uvc/uvcvideo.h | 18 ++-- 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index 73cd44e8bd81..e57c5f52c73b 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -1357,14 +1357,16 @@ static void uvc_free_urb_buffers(struct uvc_streaming *stream) unsigned int i; for (i = 0; i < UVC_URBS; ++i) { - if (stream->urb_buffer[i]) { + struct uvc_urb *uvc_urb = >uvc_urb[i]; + + if (uvc_urb->buffer) { #ifndef CONFIG_DMA_NONCOHERENT usb_free_coherent(stream->dev->udev, stream->urb_size, - stream->urb_buffer[i], stream->urb_dma[i]); + uvc_urb->buffer, uvc_urb->dma); #else - kfree(stream->urb_buffer[i]); + kfree(uvc_urb->buffer); #endif - stream->urb_buffer[i] = NULL; + uvc_urb->buffer = NULL; } } @@ -1402,16 +1404,18 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming *stream, /* Retry allocations until one succeed. */ for (; npackets > 1; npackets /= 2) { for (i = 0; i < UVC_URBS; ++i) { + struct uvc_urb *uvc_urb = >uvc_urb[i]; + stream->urb_size = psize * npackets; #ifndef CONFIG_DMA_NONCOHERENT - stream->urb_buffer[i] = usb_alloc_coherent( + uvc_urb->buffer = usb_alloc_coherent( stream->dev->udev, stream->urb_size, - gfp_flags | __GFP_NOWARN, >urb_dma[i]); + gfp_flags | __GFP_NOWARN, _urb->dma); #else - stream->urb_buffer[i] = + uvc_urb->buffer = kmalloc(stream->urb_size, gfp_flags | __GFP_NOWARN); #endif - if (!stream->urb_buffer[i]) { + if (!uvc_urb->buffer) { uvc_free_urb_buffers(stream); break; } @@ -1441,13 +1445,15 @@ static void uvc_uninit_video(struct uvc_streaming *stream, int free_buffers) uvc_video_stats_stop(stream); for (i = 0; i < UVC_URBS; ++i) { - urb = stream->urb[i]; + struct uvc_urb *uvc_urb = >uvc_urb[i]; + + urb = uvc_urb->urb; if (urb == NULL) continue; usb_kill_urb(urb); usb_free_urb(urb); - stream->urb[i] = NULL; + uvc_urb->urb = NULL; } if (free_buffers) @@ -1502,6 +1508,8 @@ static int uvc_init_video_isoc(struct uvc_streaming *stream, size = npackets * psize; for (i = 0; i < UVC_URBS; ++i) { + struct uvc_urb *uvc_urb = >uvc_urb[i]; + urb = usb_alloc_urb(npackets, gfp_flags); if (urb == NULL) { uvc_uninit_video(stream, 1); @@ -1514,12 +1522,12 @@ static int uvc_init_video_isoc(struct uvc_streaming *stream, ep->desc.bEndpointAddress); #ifndef CONFIG_DMA_NONCOHERENT urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP; - urb->transfer_dma = stream->urb_dma[i]; + urb->transfer_dma = uvc_urb->dma; #else urb->transfer_flags = URB_ISO_ASAP; #endif urb->interval = ep->desc.bInterval; - urb->transfer_buffer = stream->urb_buffer[i]; + urb->transfer_buffer = uvc_urb->buffer; urb->complete = uvc_video_complete; urb->number_of_packets = npackets; urb->transfer_buffer_length = size; @@ -1529,7 +1537,7 @@ static int uvc_init_video_isoc(struct uvc_streaming *stream, urb->iso_frame_desc[j].length = psize; } - stream->urb[i] = urb; + uvc_urb->urb = urb; } return 0; @@ -1568,21 +1576,22 @@ static int uvc_init_video_bulk(struct uvc_streaming *stream, size = 0; for (i = 0; i < UVC_URBS; ++i) { +
[RFT PATCH v3 4/6] uvcvideo: queue: Simplify spin-lock usage
Both uvc_start_streaming(), and uvc_stop_streaming() are called from userspace context. As such, they do not need to save the IRQ state, and can use spin_lock_irq() and spin_unlock_irq() respectively. Signed-off-by: Kieran Bingham--- drivers/media/usb/uvc/uvc_queue.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c index 4a581d631525..ddac4d89a291 100644 --- a/drivers/media/usb/uvc/uvc_queue.c +++ b/drivers/media/usb/uvc/uvc_queue.c @@ -158,7 +158,6 @@ static int uvc_start_streaming(struct vb2_queue *vq, unsigned int count) { struct uvc_video_queue *queue = vb2_get_drv_priv(vq); struct uvc_streaming *stream = uvc_queue_to_stream(queue); - unsigned long flags; int ret; queue->buf_used = 0; @@ -167,9 +166,9 @@ static int uvc_start_streaming(struct vb2_queue *vq, unsigned int count) if (ret == 0) return 0; - spin_lock_irqsave(>irqlock, flags); + spin_lock_irq(>irqlock); uvc_queue_return_buffers(queue, UVC_BUF_STATE_QUEUED); - spin_unlock_irqrestore(>irqlock, flags); + spin_unlock_irq(>irqlock); return ret; } @@ -178,13 +177,12 @@ static void uvc_stop_streaming(struct vb2_queue *vq) { struct uvc_video_queue *queue = vb2_get_drv_priv(vq); struct uvc_streaming *stream = uvc_queue_to_stream(queue); - unsigned long flags; uvc_video_enable(stream, 0); - spin_lock_irqsave(>irqlock, flags); + spin_lock_irq(>irqlock); uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR); - spin_unlock_irqrestore(>irqlock, flags); + spin_unlock_irq(>irqlock); } static const struct vb2_ops uvc_queue_qops = { -- git-series 0.9.1
[RFT PATCH v3 5/6] uvcvideo: queue: Support asynchronous buffer handling
The buffer queue interface currently operates sequentially, processing buffers after they have fully completed. In preparation for supporting parallel tasks operating on the buffers, we will need to support buffers being processed on multiple CPUs. Adapt the uvc_queue_next_buffer() such that a reference count tracks the active use of the buffer, returning the buffer to the VB2 stack at completion. Signed-off-by: Kieran Bingham--- drivers/media/usb/uvc/uvc_queue.c | 61 ++-- drivers/media/usb/uvc/uvcvideo.h | 4 ++- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c index ddac4d89a291..5a9987e547d3 100644 --- a/drivers/media/usb/uvc/uvc_queue.c +++ b/drivers/media/usb/uvc/uvc_queue.c @@ -131,6 +131,7 @@ static void uvc_buffer_queue(struct vb2_buffer *vb) spin_lock_irqsave(>irqlock, flags); if (likely(!(queue->flags & UVC_QUEUE_DISCONNECTED))) { + kref_init(>ref); list_add_tail(>queue, >irqqueue); } else { /* If the device is disconnected return the buffer to userspace @@ -424,28 +425,66 @@ struct uvc_buffer *uvc_queue_get_current_buffer(struct uvc_video_queue *queue) return nextbuf; } -struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue, +/* + * uvc_queue_requeue: Requeue a buffer on our internal irqqueue + * + * Reuse a buffer through our internal queue without the need to 'prepare' + * The buffer will be returned to userspace through the uvc_buffer_queue call if + * the device has been disconnected + */ +static void uvc_queue_requeue(struct uvc_video_queue *queue, struct uvc_buffer *buf) { - struct uvc_buffer *nextbuf; - unsigned long flags; + buf->error = 0; + buf->state = UVC_BUF_STATE_QUEUED; + buf->bytesused = 0; + vb2_set_plane_payload(>buf.vb2_buf, 0, 0); + + uvc_buffer_queue(>buf.vb2_buf); +} + +static void uvc_queue_buffer_complete(struct kref *ref) +{ + struct uvc_buffer *buf = container_of(ref, struct uvc_buffer, ref); + struct vb2_buffer *vb = >buf.vb2_buf; + struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue); if ((queue->flags & UVC_QUEUE_DROP_CORRUPTED) && buf->error) { - buf->error = 0; - buf->state = UVC_BUF_STATE_QUEUED; - buf->bytesused = 0; - vb2_set_plane_payload(>buf.vb2_buf, 0, 0); - return buf; + uvc_queue_requeue(queue, buf); + return; } + buf->state = buf->error ? UVC_BUF_STATE_ERROR : UVC_BUF_STATE_DONE; + vb2_set_plane_payload(>buf.vb2_buf, 0, buf->bytesused); + vb2_buffer_done(>buf.vb2_buf, VB2_BUF_STATE_DONE); +} + +/* + * Release a reference on the buffer. Complete the buffer when the last + * reference is released + */ +void uvc_queue_buffer_release(struct uvc_buffer *buf) +{ + kref_put(>ref, uvc_queue_buffer_complete); +} + +/* + * Remove this buffer from the queue. Lifetime will persist while async actions + * are still running (if any), and uvc_queue_buffer_release will give the buffer + * back to VB2 when all users have completed. + */ +struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue, + struct uvc_buffer *buf) +{ + struct uvc_buffer *nextbuf; + unsigned long flags; + spin_lock_irqsave(>irqlock, flags); list_del(>queue); nextbuf = __uvc_queue_get_current_buffer(queue); spin_unlock_irqrestore(>irqlock, flags); - buf->state = buf->error ? UVC_BUF_STATE_ERROR : UVC_BUF_STATE_DONE; - vb2_set_plane_payload(>buf.vb2_buf, 0, buf->bytesused); - vb2_buffer_done(>buf.vb2_buf, VB2_BUF_STATE_DONE); + uvc_queue_buffer_release(buf); return nextbuf; } diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 5caa1f4de3cb..6a18dbfc3e5b 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -404,6 +404,9 @@ struct uvc_buffer { unsigned int bytesused; u32 pts; + + /* asynchronous buffer handling */ + struct kref ref; }; #define UVC_QUEUE_DISCONNECTED (1 << 0) @@ -696,6 +699,7 @@ extern struct uvc_buffer * uvc_queue_get_current_buffer(struct uvc_video_queue *queue); extern struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue, struct uvc_buffer *buf); +extern void uvc_queue_buffer_release(struct uvc_buffer *buf); extern int uvc_queue_mmap(struct uvc_video_queue *queue, struct vm_area_struct *vma); extern unsigned int uvc_queue_poll(struct uvc_video_queue *queue, -- git-series 0.9.1
[RFT PATCH v3 0/6] Asynchronous UVC
The Linux UVC driver has long provided adequate performance capabilities for web-cams and low data rate video devices in Linux while resolutions were low. Modern USB cameras are now capable of high data rates thanks to USB3 with 1080p, and even 4k capture resolutions supported. Cameras such as the Stereolabs ZED (bulk transfers) or the Logitech BRIO (isochronous transfers) can generate more data than an embedded ARM core is able to process on a single core, resulting in frame loss. A large part of this performance impact is from the requirement to ‘memcpy’ frames out from URB packets to destination frames. This unfortunate requirement is due to the UVC protocol allowing a variable length header, and thus it is not possible to provide the target frame buffers directly. Extra throughput is possible by moving the actual memcpy actions to a work queue, and moving the memcpy out of interrupt context thus allowing work tasks to be scheduled across multiple cores. This series has been tested on both the ZED and BRIO cameras on arm64 platforms, however due to the intrinsic changes in the driver I would like to see it tested with other devices and other platforms, so I'd appreciate if anyone can test this on a range of USB cameras. In particular, any iSight devices, or devices which use UVC to encode data (output device) would certainly be great to be tested with these patches. v2: - Fix race reported by Guennadi v3: - Fix similar race reported by Laurent - Only queue work if required (encode/isight do not queue work) - Refactor/Rename variables for clarity Kieran Bingham (6): uvcvideo: Refactor URB descriptors uvcvideo: Convert decode functions to use new context structure uvcvideo: Protect queue internals with helper uvcvideo: queue: Simplify spin-lock usage uvcvideo: queue: Support asynchronous buffer handling uvcvideo: Move decode processing to process context drivers/media/usb/uvc/uvc_isight.c | 4 +- drivers/media/usb/uvc/uvc_queue.c | 114 + drivers/media/usb/uvc/uvc_video.c | 198 ++ drivers/media/usb/uvc/uvcvideo.h | 56 +++- 4 files changed, 296 insertions(+), 76 deletions(-) base-commit: 6f0e5fd39143a59c22d60e7befc4f33f22aeed2f -- git-series 0.9.1
Re: [PATCH v3 00/27] kill devm_ioremap_nocache
Hi Christophe , On 2018/1/4 16:05, Christophe LEROY wrote: > > > Le 25/12/2017 à 02:34, Yisheng Xie a écrit : >> >> >> On 2017/12/24 17:05, christophe leroy wrote: >>> >>> >>> Le 23/12/2017 à 14:48, Greg KH a écrit : On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote: > Hi all, > > When I tried to use devm_ioremap function and review related code, I found > devm_ioremap and devm_ioremap_nocache is almost the same with each other, > except one use ioremap while the other use ioremap_nocache. For all arches? Really? Look at MIPS, and x86, they have different functions. > While ioremap's > default function is ioremap_nocache, so devm_ioremap_nocache also have the > same function with devm_ioremap, which can just be killed to reduce the > size > of devres.o(from 20304 bytes to 18992 bytes in my compile environment). > > I have posted two versions, which use macro instead of function for > devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill > devm_ioremap_nocache for no need to keep a macro around for the duplicate > thing. So here comes v3 and please help to review. I don't think this can be done, what am I missing? These functions are not identical, sorry for missing that before. >>> >>> devm_ioremap() and devm_ioremap_nocache() are quite similar, both use >>> devm_ioremap_release() for the release, why not just defining: >>> >>> static void __iomem *__devm_ioremap(struct device *dev, resource_size_t >>> offset, >>> resource_size_t size, bool nocache) >>> { >>> [...] >>> if (nocache) >>> addr = ioremap_nocache(offset, size); >>> else >>> addr = ioremap(offset, size); >>> [...] >>> } >>> >>> then in include/linux/io.h >>> >>> static inline void __iomem *devm_ioremap(struct device *dev, >>> resource_size_t offset, >>> resource_size_t size) >>> {return __devm_ioremap(dev, offset, size, false);} >>> >>> static inline void __iomem *devm_ioremap_nocache(struct device *dev, >>> resource_size_t offset, >>> resource_size_t size); >>> {return __devm_ioremap(dev, offset, size, true);} >> >> Yeah, this seems good to me, right now we have devm_ioremap, >> devm_ioremap_wc, devm_ioremap_nocache >> May be we can use an enum like: >> typedef enum { >> DEVM_IOREMAP = 0, >> DEVM_IOREMAP_NOCACHE, >> DEVM_IOREMAP_WC, >> } devm_ioremap_type; >> >> static inline void __iomem *devm_ioremap(struct device *dev, resource_size_t >> offset, >> resource_size_t size) >> {return __devm_ioremap(dev, offset, size, DEVM_IOREMAP);} >> >> static inline void __iomem *devm_ioremap_nocache(struct device *dev, >> resource_size_t offset, >> resource_size_t size); >> {return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_NOCACHE);} >> >> static inline void __iomem *devm_ioremap_wc(struct device *dev, >> resource_size_t offset, >> resource_size_t size); >> {return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_WC);} >> >> static void __iomem *__devm_ioremap(struct device *dev, resource_size_t >> offset, >> resource_size_t size, devm_ioremap_type type) >> { >> void __iomem **ptr, *addr = NULL; >> [...] >> switch (type){ >> case DEVM_IOREMAP: >> addr = ioremap(offset, size); >> break; >> case DEVM_IOREMAP_NOCACHE: >> addr = ioremap_nocache(offset, size); >> break; >> case DEVM_IOREMAP_WC: >> addr = ioremap_wc(offset, size); >> break; >> } >> [...] >> } > > > That looks good to me, will you submit a v4 ? Sorry for late response. And I will submit the v4 as your suggestion. Thanks Yisheng > > Christophe > >>
Re: [PATCH v5 0/5] Add OV5640 parallel interface and RGB565/YUYV support
Hi, On Fri, Jan 12, 2018 at 10:18:39AM +0800, Yong wrote: > > On Thu, Jan 11, 2018 at 09:15:08AM +0800, Yong wrote: > > > > On Mon, Jan 08, 2018 at 05:13:39PM +, Hugues FRUCHET wrote: > > > > > I'm using a ST board with OV5640 wired in parallel bus output in > > > > > order > > > > > to interface to my STM32 DCMI parallel interface. > > > > > Perhaps could you describe your setup so I could help on > > > > > understanding > > > > > the problem on your side. From my past experience with this sensor > > > > > module, you can first check hsync/vsync polarities, the datasheet is > > > > > buggy on VSYNC polarity as documented in patch 4/5. > > > > > > > > It turns out that it was indeed a polarity issue. > > > > > > > > It looks like that in order to operate properly, I need to setup the > > > > opposite polarity on HSYNC and VSYNC on the interface. I looked at the > > > > signals under a scope, and VSYNC is obviously inversed as you > > > > described. HSYNC, I'm not so sure since the HBLANK period seems very > > > > long, almost a line. > > > > > > > > Since VSYNC at least looks correct, I'd be inclined to think that the > > > > polarity is inversed on at least the SoC I'm using it on. > > > > > > > > Yong, did you test the V3S CSI driver with a parallel interface? With > > > > what sensor driver? Have you found some polarities issues like this? > > > > > > Did you try it with Allwinner SoCs? > > > > Yes, on an H3. Looking at all the Allwinner datasheet I could get my > > hands on, they are all documented in the same way. However, I really > > start to wonder whether the polarity shouldn't be reversed. > > > > At least the fact that VSYNC is clearly active low on the > > oscilloscope, while I have to set it active high in the controller > > seems like a strong hint :) > > The BSP code of Allwinner also treat V4L2_MBUS_VSYNC_ACTIVE_HIGH as > they documented 'positive'. > Maybe there need some more tests to confirm if the datasheet and BSP > code are both wrong. Indeed, and at the same time they do the same on the ov5640 driver they have. I really think they are in a situation where they report that both the interface and the sensor are active high, while they are actually active low, but since both sides still use the same polarity it works. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
Re: [PATCH v4 3/9] v4l: platform: Add Renesas CEU driver
Hi Laurent, On Fri, Jan 12, 2018 at 12:12 AM, Laurent Pinchartwrote: > On Tuesday, 9 January 2018 18:25:25 EET Jacopo Mondi wrote: >> Add driver for Renesas Capture Engine Unit (CEU). >> >> The CEU interface supports capturing 'data' (YUV422) and 'images' >> (NV[12|21|16|61]). >> >> This driver aims to replace the soc_camera-based sh_mobile_ceu one. >> >> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ >> platform GR-Peach. >> >> Tested with ov7725 camera sensor on SH4 platform Migo-R. >> >> Signed-off-by: Jacopo Mondi >> --- >> drivers/media/platform/Kconfig |9 + >> drivers/media/platform/Makefile |1 + >> drivers/media/platform/renesas-ceu.c | 1648 >> ++ 3 files changed, 1658 insertions(+) >> create mode 100644 drivers/media/platform/renesas-ceu.c > > [snip] > >> diff --git a/drivers/media/platform/renesas-ceu.c >> b/drivers/media/platform/renesas-ceu.c new file mode 100644 >> index 000..d261704 >> --- /dev/null >> +++ b/drivers/media/platform/renesas-ceu.c >> @@ -0,0 +1,1648 @@ >> +// SPDX-License-Identifier: GPL-2.0 > > It was recently brought to my attention that SPDX headers should use either > GPL-2.0-only or GPL-2.0-or-later, no the ambiguous GPL-2.0. Could you please > update all patches in this series ? IMHO it's a bit premature to do that. As long as Documentation/process/license-rules.rst isn't updated, I wouldn't follow this change. See also https://www.spinics.net/lists/linux-xfs/msg14536.html 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
Re: [PATCH] media: imx258: Add imx258 camera sensor driver
Hi Andy, On Fri, Jan 12, 2018 at 08:18:50AM +, Yeh, Andy wrote: > Thanks Sakari to remind. > Since I would like to attach some snips from the datasheets. But blocked, I > will re-edit to plain table and hope it could be understood. Please wrap the lines at 80 characters. > > Regards, Andy > > From: Yeh, Andy > Sent: Friday, January 12, 2018 3:31 PM > To: 'Sakari Ailus'> Cc: 'linux-media@vger.kernel.org' ; AlanX Chiang > (alanx.chi...@intel.com) ; JasonX Z Chen > (jasonx.z.c...@intel.com) > Subject: RE: [PATCH] media: imx258: Add imx258 camera sensor driver > > Hi Sakari, > > Thanks for the review comments. Please check first then I will update patch > accordingly. > > > + usleep_range(1000, 2000); > > You are right. This should be removed since delayed in ACPI power on > sequence. > https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+/826746/5/src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/mipi_camera.asl#402 > \_SB.PCI0.I2C2.CAM0.CRST(1) Sleep(5) (could be > less so it could be adjusted in coreboot then for shorter launch time. ) Ack. > > > + /* stream off */ > > + ret = imx258_write_reg(imx258, IMX258_REG_MODE_SELECT, > > + IMX258_REG_VALUE_08BIT, > > IMX258_MODE_STANDBY); > > I don't think it should be possible that the sensor was streaming here. If > > it was, something must have been really wrong. > > Sensor datasheet claimed before updating register and streaming, need do > SW standby. However Sony implements the function with single bit. > (different from OVT) So I will change the comment to "software standby" > to replace "stream off" to clarify the purpose. > > Address: 0x0100 > Value: > 0: software standby > 1: streaming Yes, this is true. But the sensor is already in software standby mode here, isn't it? ... > And from T6~T7, per the profiling data on DUT, the duration is always > longer than 25ms (removed the manual delay) while loading all required > register lists. I would still prefer to keep a 1~2ms delay range before > streaming on for safety purpose. Fair enough; to be on the safe side, you indeed need the 12 ms delay here. > > > +static int imx258_get_skip_frames(struct v4l2_subdev *sd, u32 *frames) > > +{ > > + /* > > + * Sony: The 1st frame is ok when set from SW standby to streaming. > > + */ > > + *frames = 0; > > > If that's the case, then you can drop g_skip_frames callback altogether.. > > If remove g_skip_frames callback, it implies always 0 when user queries. > I would like to keep the flexibly if we will need change the skip_frames > value. How do you think? If it turns out the sensor needs this, then you can re-introduce the function. Please remove it now. > > > + case V4L2_CID_GAIN: > > + /* Todo */ > > + break; > > + case V4L2_CID_TEST_PATTERN: > > + /* Todo */ > > If the control isn't implemented, I suggest to remove it from the driver > > altogether, or alternatively implement it. > > The GAIN control should likely be DIGITAL_GAIN instead, right? > > You are right. Will remove them first. We are almost done (DIGITAL_GAIN) > and will submit separately. Ack. -- Kind regards, Sakari Ailus sakari.ai...@linux.intel.com
Re: [linux-sunxi] Re: [PATCH v5 2/2] media: V3s: Add support for Allwinner CSI.
On Fri, Jan 12, 2018 at 09:51:14AM +0800, Yong wrote: > Hi Maxime, > > On Thu, 11 Jan 2018 14:28:44 +0100 > Maxime Ripardwrote: > > > Hi Yong, > > > > On Thu, Jan 11, 2018 at 11:06:06AM +0800, Yong Deng wrote: > > > Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > interface and CSI1 is used for parallel interface. This is not > > > documented in datasheet but by test and guess. > > > > > > This patch implement a v4l2 framework driver for it. > > > > > > Currently, the driver only support the parallel interface. MIPI-CSI2, > > > ISP's support are not included in this patch. > > > > > > Signed-off-by: Yong Deng > > > > I've needed this patch in order to fix a NULL pointer dereference: > > http://code.bulix.org/oz6gmb-257359?raw > > > > This is needed because while it's ok to have a NULL pointer to > > v4l2_subdev_pad_config when you call the subdev set_fmt with > > V4L2_SUBDEV_FORMAT_ACTIVE, it's not with V4L2_SUBDEV_FORMAT_TRY, and > > sensors will assume taht it's a valid pointer. > > > > Otherwise, > > Tested-by: Maxime Ripard > > I revisit some code of subdevs and you are right. > > Squash your patch into my driver patch and add your Tested-by in > commit. Is it right? Yep, that's perfect :) Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
RE: [PATCH] media: imx258: Add imx258 camera sensor driver
Thanks Sakari to remind. Since I would like to attach some snips from the datasheets. But blocked, I will re-edit to plain table and hope it could be understood. Regards, Andy From: Yeh, Andy Sent: Friday, January 12, 2018 3:31 PM To: 'Sakari Ailus'Cc: 'linux-media@vger.kernel.org' ; AlanX Chiang (alanx.chi...@intel.com) ; JasonX Z Chen (jasonx.z.c...@intel.com) Subject: RE: [PATCH] media: imx258: Add imx258 camera sensor driver Hi Sakari, Thanks for the review comments. Please check first then I will update patch accordingly. > + usleep_range(1000, 2000); You are right. This should be removed since delayed in ACPI power on sequence. https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+/826746/5/src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/mipi_camera.asl#402 \_SB.PCI0.I2C2.CAM0.CRST(1) Sleep(5) (could be less so it could be adjusted in coreboot then for shorter launch time. ) > + /* stream off */ > + ret = imx258_write_reg(imx258, IMX258_REG_MODE_SELECT, > + IMX258_REG_VALUE_08BIT, > IMX258_MODE_STANDBY); > I don't think it should be possible that the sensor was streaming here. If > it was, something must have been really wrong. Sensor datasheet claimed before updating register and streaming, need do SW standby. However Sony implements the function with single bit. (different from OVT) So I will change the comment to "software standby" to replace "stream off" to clarify the purpose. Address: 0x0100 Value: 0: software standby 1: streaming > + if (ret) { > + dev_err(>dev, "%s failed to set stream off > registers\n", > + __func__); > + return ret; > + } > + usleep_range(13000, 14000); > That's a very long delay. Will preciously adjust the delay here, and let me explain the delay based on sensor datasheet. (under Intel NDA) Also please help review if any comments. Above delay (13~14ms) is the time from XCLR (reset) to streaming. So I was programming it as 13~14ms. (numbering from spec) T6 = 0.4ms: MCLK start to reset rising till CCI Read version ID register wait time. T7 = 12ms: MCLK start and reset rising till Send streaming command wait time (to complete reading all parameters) Since in coreboot, there are 5ms delay after reset, so assume T6" is 5ms. And from T6~T7, per the profiling data on DUT, the duration is always longer than 25ms (removed the manual delay) while loading all required register lists. I would still prefer to keep a 1~2ms delay range before streaming on for safety purpose. > +static int imx258_get_skip_frames(struct v4l2_subdev *sd, u32 *frames) > +{ > + /* > + * Sony: The 1st frame is ok when set from SW standby to streaming. > + */ > + *frames = 0; > If that's the case, then you can drop g_skip_frames callback altogether.. If remove g_skip_frames callback, it implies always 0 when user queries. I would like to keep the flexibly if we will need change the skip_frames value. How do you think? > + case V4L2_CID_GAIN: > + /* Todo */ > + break; > + case V4L2_CID_TEST_PATTERN: > + /* Todo */ > If the control isn't implemented, I suggest to remove it from the driver > altogether, or alternatively implement it. > The GAIN control should likely be DIGITAL_GAIN instead, right? You are right. Will remove them first. We are almost done (DIGITAL_GAIN) and will submit separately. Regards, Andy -Original Message- From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com] Sent: Friday, January 12, 2018 5:25 AM To: Yeh, Andy Cc: linux-media@vger.kernel.org Subject: Re: [PATCH] media: imx258: Add imx258 camera sensor driver Hi Andy, Thanks for the patch. Please see my comments below. On Thu, Jan 11, 2018 at 10:47:39PM +0800, Andy Yeh wrote: > Add a V4L2 sub-device driver for the Sony IMX258 image sensor. > This is a camera sensor using the I2C bus for control and the > CSI-2 bus for data. > > Signed-off-by: Andy Yeh > --- > MAINTAINERS | 7 + > drivers/media/i2c/Kconfig | 11 + > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/imx258.c | 1184 > > 4 files changed, 1203 insertions(+) > create mode 100644 drivers/media/i2c/imx258.c > > diff --git a/MAINTAINERS b/MAINTAINERS index d76af75..806aa46 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12640,6 +12640,13 @@ S: Maintained > F: drivers/ssb/ > F: include/linux/ssb/ > > +SONY IMX258 SENSOR DRIVER > +M: Sakari Ailus > +L: linux-media@vger.kernel.org > +T: git git://linuxtv.org/media_tree.git > +S: Maintained >
Re: [PATCH 2/2] media: intel-ipu3: cio2: fix for wrong vb2buf state warnings
On Thu, Jan 4, 2018 at 11:57 AM, Yong Zhiwrote: > cio2 driver should release buffer with QUEUED state > when start_stream op failed, wrong buffer state will > cause vb2 core throw a warning. > > Signed-off-by: Yong Zhi > Signed-off-by: Cao Bing Bu > --- > drivers/media/pci/intel/ipu3/ipu3-cio2.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > index 949f43d206ad..106d04306372 100644 > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > @@ -785,7 +785,8 @@ static irqreturn_t cio2_irq(int irq, void *cio2_ptr) > > / Videobuf2 interface / > > -static void cio2_vb2_return_all_buffers(struct cio2_queue *q) > +static void cio2_vb2_return_all_buffers(struct cio2_queue *q, > + enum vb2_buffer_state state) > { > unsigned int i; > > @@ -793,7 +794,7 @@ static void cio2_vb2_return_all_buffers(struct cio2_queue > *q) > if (q->bufs[i]) { > atomic_dec(>bufs_queued); > vb2_buffer_done(>bufs[i]->vbb.vb2_buf, > - VB2_BUF_STATE_ERROR); > + state); nit: Does it really exceed 80 characters after folding into previous line? With the nit fixed: Reviewed-by: Tomasz Figa Best regards, Tomasz
Re: [PATCH 1/2] media: intel-ipu3: cio2: fix a crash with out-of-bounds access
On Thu, Jan 4, 2018 at 11:57 AM, Yong Zhiwrote: > When dmabuf is used for BLOB type frame, the frame > buffers allocated by gralloc will hold more pages > than the valid frame data due to height alignment. > > In this case, the page numbers in sg list could exceed the > FBPT upper limit value - max_lops(8)*1024 to cause crash. > > Limit the LOP access to the valid data length > to avoid FBPT sub-entries overflow. > > Signed-off-by: Yong Zhi > Signed-off-by: Cao Bing Bu > --- > drivers/media/pci/intel/ipu3/ipu3-cio2.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > index 941caa987dab..949f43d206ad 100644 > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > @@ -838,8 +838,9 @@ static int cio2_vb2_buf_init(struct vb2_buffer *vb) > container_of(vb, struct cio2_buffer, vbb.vb2_buf); > static const unsigned int entries_per_page = > CIO2_PAGE_SIZE / sizeof(u32); > - unsigned int pages = DIV_ROUND_UP(vb->planes[0].length, > CIO2_PAGE_SIZE); > - unsigned int lops = DIV_ROUND_UP(pages + 1, entries_per_page); > + unsigned int pages = DIV_ROUND_UP(vb->planes[0].length, > + CIO2_PAGE_SIZE) + 1; Why + 1? This would still overflow the buffer, wouldn't it? > + unsigned int lops = DIV_ROUND_UP(pages, entries_per_page); > struct sg_table *sg; > struct sg_page_iter sg_iter; > int i, j; > @@ -869,6 +870,8 @@ static int cio2_vb2_buf_init(struct vb2_buffer *vb) > > i = j = 0; > for_each_sg_page(sg->sgl, _iter, sg->nents, 0) { > + if (!pages--) > + break; Or perhaps we should check here for (pages > 1)? Best regards, Tomasz