Re: [PATCH v4 00/12] Intel IPU3 ImgU patchset
On 12/05/2017 04:22 AM, Tomasz Figa wrote: > Hi Raj, > > On Tue, Dec 5, 2017 at 9:13 AM, Mani, Rajmohan > wrote: >> Hi Hans, >> >> Thanks for your patience and sharing your thoughts on this. >> >>> Subject: Re: [PATCH v4 00/12] Intel IPU3 ImgU patchset >>> >>> Hi Rajmohan, >>> >>> On 11/17/2017 03:58 AM, Mani, Rajmohan wrote: Hi Sakari and all, > -Original Message- > From: Zhi, Yong > Sent: Tuesday, October 17, 2017 8:47 PM > To: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com > Cc: Zheng, Jian Xu ; Mani, Rajmohan > ; Toivonen, Tuukka > ; Hu, Jerry W ; > a...@arndb.de; h...@lst.de; robin.mur...@arm.com; iommu@lists.linux- > foundation.org; Zhi, Yong > Subject: [PATCH v4 00/12] Intel IPU3 ImgU patchset > > This patchset adds support for the Intel IPU3 (Image Processing Unit) > ImgU which is essentially a modern memory-to-memory ISP. It > implements raw Bayer to YUV image format conversion as well as a > large number of other pixel processing algorithms for improving the image >>> quality. > > Meta data formats are defined for image statistics (3A, i.e. > automatic white balance, exposure and focus, histogram and local area > contrast > enhancement) as well as for the pixel processing algorithm parameters. > The documentation for these formats is currently not included in the > patchset but will be added in a future version of this set. > Here is an update on the IPU3 documentation that we are currently working >>> on. Image processing in IPU3 relies on the following. 1) HW configuration to enable ISP and 2) setting customer specific 3A Tuning / Algorithm Parameters to achieve >>> desired image quality. We intend to provide documentation on ImgU driver programming interface >>> to help users of this driver to configure and enable ISP HW to meet their >>> needs. This documentation will include details on complete V4L2 Kernel >>> driver >>> interface and IO-Control parameters, except for the ISP internal algorithm >>> and >>> its parameters (which is Intel proprietary IP). We will also provide an user space library in binary form to help users of this >>> driver, to convert the public 3A tuning parameters to IPU3 algorithm >>> parameters. This tool will be released under NDA to the users of this >>> driver. >>> >>> So I discussed this situation with Sakari in Prague during the ELCE. I am >>> not >>> happy with adding a driver to the kernel that needs an NDA to be usable. I >>> can't agree to that. It's effectively the same as firmware that's only >>> available >>> under an NDA and we wouldn't accept such drivers either. >>> >> >> Ack >> >>> There are a few options: >>> >>> 1) Document the ISP parameters and that format they are stored in allowing >>> for >>>open source implementations. While this is the ideal, I suspect that >>> this is >>>a no-go for Intel. >>> >> >> Ack >> >>> 2) The driver can be used without using these ISP parameters and still >>> achieve >>>'OK' quality. I.e., it's usable for basic webcam usage under normal >>> lighting >>>conditions. I'm not sure if this is possible at all, though. >>> >> >> This is something that we have tried and are able to do image capture with >> the default ISP parameters using ov5670 sensor. >> Additionally we had to set optimal values for the ov5670 sensor's exposure >> and >> gain settings. > > Does it mean hardcoding some ov5670-specific settings in the ISP > driver? If not, I guess it might be good enough? It doesn't look too bad, but it's hard to tell from just a single frame. I think you can work with Sakari on this, if he also thinks it's good enough, then I am happy with that. > >> >> Please see if the following image looks to meet the 'OK' quality. >> >> git clone https://github.com/RajmohanMani/ipu3-misc.git >> ipu3-misc/ov5670.jpg is the image file. >> >>> 3) Make the library available without requiring an NDA. >>> >> >> We are also actively exploring this option to see if this can be done. >> >>> 4) Provide a non-NDA library (ideally open source) that achieves at minimum >>>the quality as described in 2: i.e. usable for basic webcam. >>> >> >> I see this is the same as option 3) + open sourcing the library. >> Open sourcing the library does not look to be an option. >> I will reconfirm this. > > In my understanding, that could be quite different from option 3). The > open source library would not have to implement all of the > capabilities, just enough to get the "OK" quality and the implemented > part could use some simpler algorithms not covered by IP. That's correct. This also solves my concerns about long-term maintenance of closed source libraries which in my experience tend to suffer from bit-rot after the vendor no longer sells the hardware it was written for. An open source implementation, albeit not as powerful as the NDA ve
Re: Donation
Good Day, My wife and I have awarded you with a donation of $ 1,000,000.00 Dollars from part of our Jackpot Lottery of 50 Million Dollars, respond with your details for claims to our private email. We await your earliest response and God Bless you. Best Regards, Friedrich And Annand Mayrhofer.
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: Tue Dec 5 05:00:23 CET 2017 media-tree git hash:781b045baefdabf7e0bc9f33672ca830d3db9f27 media_build git hash: 320b9b80ebbf318a67a9479c18a0e4be244c8409 v4l-utils git hash: 1864eb9d5908f87d3922ef627cb6bdd096a77edc gcc version:i686-linux-gcc (GCC) 7.1.0 sparse version: v0.5.0-3911-g6f737e1f smatch version: v0.5.0-3911-g6f737e1f host hardware: x86_64 host os:4.13.0-164 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.1.10-i686: ERRORS linux-3.2.37-i686: ERRORS linux-3.3.8-i686: ERRORS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: ERRORS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: ERRORS linux-3.10.1-i686: ERRORS linux-3.11.1-i686: ERRORS linux-3.12.67-i686: ERRORS linux-3.13.11-i686: ERRORS linux-3.14.9-i686: ERRORS linux-3.15.2-i686: ERRORS linux-3.16.7-i686: ERRORS linux-3.17.8-i686: ERRORS linux-3.18.7-i686: ERRORS linux-3.19-i686: ERRORS linux-4.0.9-i686: ERRORS linux-4.1.33-i686: ERRORS linux-4.2.8-i686: ERRORS linux-4.3.6-i686: ERRORS linux-4.4.22-i686: ERRORS linux-4.5.7-i686: ERRORS linux-4.6.7-i686: ERRORS linux-4.7.5-i686: ERRORS linux-4.8-i686: ERRORS linux-4.9.26-i686: ERRORS linux-4.10.14-i686: ERRORS linux-4.11-i686: ERRORS linux-4.12.1-i686: ERRORS linux-4.13-i686: ERRORS linux-4.14-i686: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.37-x86_64: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: ERRORS linux-3.10.1-x86_64: ERRORS linux-3.11.1-x86_64: ERRORS linux-3.12.67-x86_64: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.9-x86_64: ERRORS linux-3.15.2-x86_64: ERRORS linux-3.16.7-x86_64: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.7-x86_64: ERRORS linux-3.19-x86_64: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.33-x86_64: ERRORS linux-4.2.8-x86_64: ERRORS linux-4.3.6-x86_64: ERRORS linux-4.4.22-x86_64: ERRORS linux-4.5.7-x86_64: ERRORS linux-4.6.7-x86_64: ERRORS linux-4.7.5-x86_64: ERRORS linux-4.8-x86_64: ERRORS linux-4.9.26-x86_64: ERRORS linux-4.10.14-x86_64: ERRORS linux-4.11-x86_64: ERRORS linux-4.12.1-x86_64: ERRORS linux-4.13-x86_64: ERRORS linux-4.14-x86_64: ERRORS apps: OK spec-git: OK smatch: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH v4 00/12] Intel IPU3 ImgU patchset
Hi Raj, On Tue, Dec 5, 2017 at 9:13 AM, Mani, Rajmohan wrote: > Hi Hans, > > Thanks for your patience and sharing your thoughts on this. > >> Subject: Re: [PATCH v4 00/12] Intel IPU3 ImgU patchset >> >> Hi Rajmohan, >> >> On 11/17/2017 03:58 AM, Mani, Rajmohan wrote: >> > Hi Sakari and all, >> > >> >> -Original Message- >> >> From: Zhi, Yong >> >> Sent: Tuesday, October 17, 2017 8:47 PM >> >> To: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com >> >> Cc: Zheng, Jian Xu ; Mani, Rajmohan >> >> ; Toivonen, Tuukka >> >> ; Hu, Jerry W ; >> >> a...@arndb.de; h...@lst.de; robin.mur...@arm.com; iommu@lists.linux- >> >> foundation.org; Zhi, Yong >> >> Subject: [PATCH v4 00/12] Intel IPU3 ImgU patchset >> >> >> >> This patchset adds support for the Intel IPU3 (Image Processing Unit) >> >> ImgU which is essentially a modern memory-to-memory ISP. It >> >> implements raw Bayer to YUV image format conversion as well as a >> >> large number of other pixel processing algorithms for improving the image >> quality. >> >> >> >> Meta data formats are defined for image statistics (3A, i.e. >> >> automatic white balance, exposure and focus, histogram and local area >> >> contrast >> >> enhancement) as well as for the pixel processing algorithm parameters. >> >> The documentation for these formats is currently not included in the >> >> patchset but will be added in a future version of this set. >> >> >> > >> > Here is an update on the IPU3 documentation that we are currently working >> on. >> > >> > Image processing in IPU3 relies on the following. >> > >> > 1) HW configuration to enable ISP and >> > 2) setting customer specific 3A Tuning / Algorithm Parameters to achieve >> desired image quality. >> > >> > We intend to provide documentation on ImgU driver programming interface >> to help users of this driver to configure and enable ISP HW to meet their >> needs. This documentation will include details on complete V4L2 Kernel >> driver >> interface and IO-Control parameters, except for the ISP internal algorithm >> and >> its parameters (which is Intel proprietary IP). >> > >> > We will also provide an user space library in binary form to help users of >> > this >> driver, to convert the public 3A tuning parameters to IPU3 algorithm >> parameters. This tool will be released under NDA to the users of this driver. >> >> So I discussed this situation with Sakari in Prague during the ELCE. I am not >> happy with adding a driver to the kernel that needs an NDA to be usable. I >> can't agree to that. It's effectively the same as firmware that's only >> available >> under an NDA and we wouldn't accept such drivers either. >> > > Ack > >> There are a few options: >> >> 1) Document the ISP parameters and that format they are stored in allowing >> for >>open source implementations. While this is the ideal, I suspect that this >> is >>a no-go for Intel. >> > > Ack > >> 2) The driver can be used without using these ISP parameters and still >> achieve >>'OK' quality. I.e., it's usable for basic webcam usage under normal >> lighting >>conditions. I'm not sure if this is possible at all, though. >> > > This is something that we have tried and are able to do image capture with > the default ISP parameters using ov5670 sensor. > Additionally we had to set optimal values for the ov5670 sensor's exposure and > gain settings. Does it mean hardcoding some ov5670-specific settings in the ISP driver? If not, I guess it might be good enough? > > Please see if the following image looks to meet the 'OK' quality. > > git clone https://github.com/RajmohanMani/ipu3-misc.git > ipu3-misc/ov5670.jpg is the image file. > >> 3) Make the library available without requiring an NDA. >> > > We are also actively exploring this option to see if this can be done. > >> 4) Provide a non-NDA library (ideally open source) that achieves at minimum >>the quality as described in 2: i.e. usable for basic webcam. >> > > I see this is the same as option 3) + open sourcing the library. > Open sourcing the library does not look to be an option. > I will reconfirm this. In my understanding, that could be quite different from option 3). The open source library would not have to implement all of the capabilities, just enough to get the "OK" quality and the implemented part could use some simpler algorithms not covered by IP. Best regards, Tomasz
Re: [PATCH 3/3 v7] uvcvideo: add a metadata device node
Hi Guennadi,
On Tuesday, 5 December 2017 02:24:30 EET Laurent Pinchart wrote:
> On Wednesday, 8 November 2017 18:00:14 EET Guennadi Liakhovetski wrote:
[snip]
> > +static void uvc_video_decode_meta(struct uvc_streaming *stream,
> > + struct uvc_buffer *buf, struct uvc_buffer *meta_buf,
> > + u8 *mem, unsigned int length)
>
> The buf parameter is unused, you can remove it. mem isn't modified, I would
> make it const.
>
> > +{
> > + struct uvc_meta_buf *meta;
> > + size_t len_std = 2;
> > + bool has_pts, has_scr;
> > + unsigned long flags;
> > + struct timespec ts;
> > + u8 *scr;
>
> And scr should be const too.
>
> > +
> > + if (!meta_buf || length == 2 ||
> > + meta_buf->length - meta_buf->bytesused <
> > + length + sizeof(meta->ns) + sizeof(meta->sof))
> > + return;
>
> If the metadata buffer overflows should we also set the error bit like we do
> for video buffers ? I have mixed feelings about this, I'd appreciate your
> input.
>
> > + has_pts = mem[1] & UVC_STREAM_PTS;
> > + has_scr = mem[1] & UVC_STREAM_SCR;
> > +
> > + if (has_pts) {
> > + len_std += 4;
> > + scr = mem + 6;
> > + } else {
> > + scr = mem + 2;
> > + }
> > +
> > + if (has_scr)
> > + len_std += 6;
> > +
> > + if (stream->cur_meta_format == V4L2_META_FMT_UVC)
> > + length = len_std;
> > +
> > + if (length == len_std && (!has_scr ||
> > + !memcmp(scr, stream->clock.last_scr, 6)))
> > + return;
> > +
> > + meta = (struct uvc_meta_buf *)((u8 *)meta_buf->mem +
> > meta_buf->bytesused); + local_irq_save(flags);
> > + uvc_video_get_ts(&ts);
>
> FYI, Arnd has posted https://patchwork.kernel.org/patch/10076887/. If the
> patch gets merged first I can help with the rebasing.
I've reviewed and merged Arnd patches in my tree, and...
> > + meta->sof = usb_get_current_frame_number(stream->dev->udev);
> > + local_irq_restore(flags);
> > + meta->ns = timespec_to_ns(&ts);
>
> The meta pointer can be unaligned as the structure is packed and its size
> isn't a multiple of the size of the largest field (and it can contain an
> unspecified amount of vendor data anyway). You thus can't access it directly
> on all architectures, you will need to use the put_unaligned macro. As I
> haven't checked whether all architectures can handle unaligned accesses
> without generating a trap, I would store the USB frame number in a local
> variable and use the put_unaligned macro output of the IRQ disabled section
> (feel free to show me that I'm unnecessarily cautious :-)).
>
> > + if (has_scr)
> > + memcpy(stream->clock.last_scr, scr, 6);
> > +
> > + memcpy(&meta->length, mem, length);
> > + meta_buf->bytesused += length + sizeof(meta->ns) + sizeof(meta->sof);
> > +
> > + uvc_trace(UVC_TRACE_FRAME,
> > + "%s(): t-sys %lu.%09lus, SOF %u, len %u, flags 0x%x, PTS %u,
> > STC
%u
> > frame SOF %u\n",
> > + __func__, ts.tv_sec, ts.tv_nsec, meta->sof,
> > + meta->length, meta->flags, has_pts ? *(u32 *)meta->buf : 0,
> > + has_scr ? *(u32 *)scr : 0,
> > + has_scr ? *(u32 *)(scr + 4) & 0x7ff : 0);
> > +}
[snip]
> For your convenience I've rebased your patch series on top of the two
> patches I mentioned and added another patch on top that contains fixes for
> all the small issues mentioned above. The result is available at
>
> git://linuxtv.org/pinchartl/media.git uvc/metadata
>
> There are just a handful of issues or questions I haven't addressed, if we
> handle them I think we'll be good to go.
... updated the above branch with a rebased version of the series.
--
Regards,
Laurent Pinchart
Re: [PATCH 2/8] [media] uvc_video: use ktime_t for timestamps
Hi Arnd, On Tuesday, 5 December 2017 02:37:27 EET Laurent Pinchart wrote: > On Monday, 27 November 2017 15:19:54 EET Arnd Bergmann wrote: > > uvc_video_get_ts() returns a 'struct timespec', but all its users > > really want a nanoseconds variable anyway. > > > > Changing the deprecated ktime_get_ts/ktime_get_real_ts to ktime_get > > and ktime_get_real simplifies the code noticeably, while keeping > > the resulting numbers unchanged. > > > > Signed-off-by: Arnd Bergmann > > --- > > > > drivers/media/usb/uvc/uvc_video.c | 37 - > > drivers/media/usb/uvc/uvcvideo.h | 2 +- > > 2 files changed, 13 insertions(+), 26 deletions(-) [snip] > > - struct timespec ts; > > + u64 timestamp; [snip] > > uvc_trace(UVC_TRACE_CLOCK, "%s: SOF %u.%06llu y %llu ts %llu " > > "buf ts %llu (x1 %u/%u/%u x2 %u/%u/%u y1 %u y2 %u)\n", > > stream->dev->name, > > sof >> 16, div_u64(((u64)sof & 0x) * 100LLU, 65536), > > - y, timespec_to_ns(&ts), vbuf->vb2_buf.timestamp, > > + y, timestamp, vbuf->vb2_buf.timestamp, > > x1, first->host_sof, first->dev_sof, > > x2, last->host_sof, last->dev_sof, y1, y2); As you've done lots of work moving code away from timespec I figured out I would ask, what is the preferred way to print a ktime in secs.nsecs format ? Should I use ktime_to_timespec and print ts.tv_sec and ts.tv_nsec, or is there a better way ? -- Regards, Laurent Pinchart
Re: [PATCH 1/8] [media] uvc_video: use ktime_t for stats
Hi Arnd,
Thank you for the patch.
On Monday, 27 November 2017 15:19:53 EET Arnd Bergmann wrote:
> 'struct timespec' works fine here, but we try to migrate
> away from it in favor of ktime_t or timespec64. In this
> case, using ktime_t produces the simplest code.
>
> Signed-off-by: Arnd Bergmann
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/usb/uvc/uvc_video.c | 11 ---
> drivers/media/usb/uvc/uvcvideo.h | 4 ++--
> 2 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index fb86d6af398d..d6bee37cd1b8 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -725,7 +725,7 @@ static void uvc_video_stats_decode(struct uvc_streaming
> *stream,
>
> if (stream->stats.stream.nb_frames == 0 &&
> stream->stats.frame.nb_packets == 0)
> - ktime_get_ts(&stream->stats.stream.start_ts);
> + stream->stats.stream.start_ts = ktime_get();
>
> switch (data[1] & (UVC_STREAM_PTS | UVC_STREAM_SCR)) {
> case UVC_STREAM_PTS | UVC_STREAM_SCR:
> @@ -865,16 +865,13 @@ size_t uvc_video_stats_dump(struct uvc_streaming
> *stream, char *buf, {
> unsigned int scr_sof_freq;
> unsigned int duration;
> - struct timespec ts;
> size_t count = 0;
>
> - ts = timespec_sub(stream->stats.stream.stop_ts,
> - stream->stats.stream.start_ts);
> -
> /* Compute the SCR.SOF frequency estimate. At the nominal 1kHz SOF
>* frequency this will not overflow before more than 1h.
>*/
> - duration = ts.tv_sec * 1000 + ts.tv_nsec / 100;
> + duration = ktime_ms_delta(stream->stats.stream.stop_ts,
> + stream->stats.stream.start_ts);
> if (duration != 0)
> scr_sof_freq = stream->stats.stream.scr_sof_count * 1000
>/ duration;
> @@ -915,7 +912,7 @@ static void uvc_video_stats_start(struct uvc_streaming
> *stream)
>
> static void uvc_video_stats_stop(struct uvc_streaming *stream)
> {
> - ktime_get_ts(&stream->stats.stream.stop_ts);
> + stream->stats.stream.stop_ts = ktime_get();
> }
>
> /*
> diff --git a/drivers/media/usb/uvc/uvcvideo.h
> b/drivers/media/usb/uvc/uvcvideo.h index 05398784d1c8..a2c190937067 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -452,8 +452,8 @@ struct uvc_stats_frame {
> };
>
> struct uvc_stats_stream {
> - struct timespec start_ts; /* Stream start timestamp */
> - struct timespec stop_ts;/* Stream stop timestamp */
> + ktime_t start_ts; /* Stream start timestamp */
> + ktime_t stop_ts;/* Stream stop timestamp */
>
> unsigned int nb_frames; /* Number of frames */
--
Regards,
Laurent Pinchart
Re: [PATCH 5/8] [media] omap3isp: support 64-bit version of omap3isp_stat_data
Hi Arnd,
Thank you for the patch.
I'll try to review this without too much delay. In the meantime, I'm CC'ing
Sakari Ailus who might be faster than me :-)
On Monday, 27 November 2017 15:19:57 EET Arnd Bergmann wrote:
> C libraries with 64-bit time_t use an incompatible format for
> struct omap3isp_stat_data. This changes the kernel code to
> support either version, by moving over the normal handling
> to the 64-bit variant, and adding compatiblity code to handle
> the old binary format with the existing ioctl command code.
>
> Fortunately, the command code includes the size of the structure,
> so the difference gets handled automatically. In the process of
> eliminating the references to 'struct timeval' from the kernel,
> I also change the way the timestamp is generated internally,
> basically by open-coding the v4l2_get_timestamp() call.
>
> Signed-off-by: Arnd Bergmann
> ---
> drivers/media/platform/omap3isp/isph3a_aewb.c | 2 ++
> drivers/media/platform/omap3isp/isph3a_af.c | 2 ++
> drivers/media/platform/omap3isp/isphist.c | 2 ++
> drivers/media/platform/omap3isp/ispstat.c | 21 +++--
> drivers/media/platform/omap3isp/ispstat.h | 4 +++-
> include/uapi/linux/omap3isp.h | 22 ++
> 6 files changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/isph3a_aewb.c
> b/drivers/media/platform/omap3isp/isph3a_aewb.c index
> d44626f20ac6..3c82dea4d375 100644
> --- a/drivers/media/platform/omap3isp/isph3a_aewb.c
> +++ b/drivers/media/platform/omap3isp/isph3a_aewb.c
> @@ -250,6 +250,8 @@ static long h3a_aewb_ioctl(struct v4l2_subdev *sd,
> unsigned int cmd, void *arg) return omap3isp_stat_config(stat, arg);
> case VIDIOC_OMAP3ISP_STAT_REQ:
> return omap3isp_stat_request_statistics(stat, arg);
> + case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
> + return omap3isp_stat_request_statistics_time32(stat, arg);
> case VIDIOC_OMAP3ISP_STAT_EN: {
> unsigned long *en = arg;
> return omap3isp_stat_enable(stat, !!*en);
> diff --git a/drivers/media/platform/omap3isp/isph3a_af.c
> b/drivers/media/platform/omap3isp/isph3a_af.c index
> 99bd6cc21d86..4da25c84f0c6 100644
> --- a/drivers/media/platform/omap3isp/isph3a_af.c
> +++ b/drivers/media/platform/omap3isp/isph3a_af.c
> @@ -314,6 +314,8 @@ static long h3a_af_ioctl(struct v4l2_subdev *sd,
> unsigned int cmd, void *arg) return omap3isp_stat_config(stat, arg);
> case VIDIOC_OMAP3ISP_STAT_REQ:
> return omap3isp_stat_request_statistics(stat, arg);
> + case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
> + return omap3isp_stat_request_statistics_time32(stat, arg);
> case VIDIOC_OMAP3ISP_STAT_EN: {
> int *en = arg;
> return omap3isp_stat_enable(stat, !!*en);
> diff --git a/drivers/media/platform/omap3isp/isphist.c
> b/drivers/media/platform/omap3isp/isphist.c index
> a4ed5d140d48..d4be3d0e06f9 100644
> --- a/drivers/media/platform/omap3isp/isphist.c
> +++ b/drivers/media/platform/omap3isp/isphist.c
> @@ -435,6 +435,8 @@ static long hist_ioctl(struct v4l2_subdev *sd, unsigned
> int cmd, void *arg) return omap3isp_stat_config(stat, arg);
> case VIDIOC_OMAP3ISP_STAT_REQ:
> return omap3isp_stat_request_statistics(stat, arg);
> + case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
> + return omap3isp_stat_request_statistics_time32(stat, arg);
> case VIDIOC_OMAP3ISP_STAT_EN: {
> int *en = arg;
> return omap3isp_stat_enable(stat, !!*en);
> diff --git a/drivers/media/platform/omap3isp/ispstat.c
> b/drivers/media/platform/omap3isp/ispstat.c index
> 47cbc7e3d825..5967dfd0a9f7 100644
> --- a/drivers/media/platform/omap3isp/ispstat.c
> +++ b/drivers/media/platform/omap3isp/ispstat.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
>
> #include "isp.h"
>
> @@ -237,7 +238,7 @@ static int isp_stat_buf_queue(struct ispstat *stat)
> if (!stat->active_buf)
> return STAT_NO_BUF;
>
> - v4l2_get_timestamp(&stat->active_buf->ts);
> + ktime_get_ts64(&stat->active_buf->ts);
>
> stat->active_buf->buf_size = stat->buf_size;
> if (isp_stat_buf_check_magic(stat, stat->active_buf)) {
> @@ -500,7 +501,8 @@ int omap3isp_stat_request_statistics(struct ispstat
> *stat, return PTR_ERR(buf);
> }
>
> - data->ts = buf->ts;
> + data->ts.tv_sec = buf->ts.tv_sec;
> + data->ts.tv_usec = buf->ts.tv_nsec / NSEC_PER_USEC;
> data->config_counter = buf->config_counter;
> data->frame_number = buf->frame_number;
> data->buf_size = buf->buf_size;
> @@ -512,6 +514,21 @@ int omap3isp_stat_request_statistics(struct ispstat
> *stat, return 0;
> }
>
> +int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
> + struct omap3isp_stat_data_time32 *data)
> +{
> + struct omap3isp_stat_data data64;
> +
Re: [PATCH 2/8] [media] uvc_video: use ktime_t for timestamps
Hi Arnd,
Thank you for the patch.
On Monday, 27 November 2017 15:19:54 EET Arnd Bergmann wrote:
> uvc_video_get_ts() returns a 'struct timespec', but all its users
> really want a nanoseconds variable anyway.
>
> Changing the deprecated ktime_get_ts/ktime_get_real_ts to ktime_get
> and ktime_get_real simplifies the code noticeably, while keeping
> the resulting numbers unchanged.
>
> Signed-off-by: Arnd Bergmann
> ---
> drivers/media/usb/uvc/uvc_video.c | 37 ---
> drivers/media/usb/uvc/uvcvideo.h | 2 +-
> 2 files changed, 13 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index d6bee37cd1b8..f7a919490b2b 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -369,12 +369,12 @@ static int uvc_commit_video(struct uvc_streaming
> *stream, * Clocks and timestamps
> */
>
> -static inline void uvc_video_get_ts(struct timespec *ts)
> +static inline ktime_t uvc_video_get_time(void)
> {
> if (uvc_clock_param == CLOCK_MONOTONIC)
> - ktime_get_ts(ts);
> + return ktime_get();
> else
> - ktime_get_real_ts(ts);
> + return ktime_get_real();
> }
>
> static void
> @@ -386,7 +386,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream,
> struct uvc_buffer *buf, bool has_pts = false;
> bool has_scr = false;
> unsigned long flags;
> - struct timespec ts;
> + ktime_t time;
> u16 host_sof;
> u16 dev_sof;
>
> @@ -436,7 +436,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream,
> struct uvc_buffer *buf, stream->clock.last_sof = dev_sof;
>
> host_sof = usb_get_current_frame_number(stream->dev->udev);
> - uvc_video_get_ts(&ts);
> + time = uvc_video_get_time();
>
> /* The UVC specification allows device implementations that can't obtain
>* the USB frame number to keep their own frame counters as long as they
> @@ -473,7 +473,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream,
> struct uvc_buffer *buf, sample->dev_stc =
> get_unaligned_le32(&data[header_size - 6]);
> sample->dev_sof = dev_sof;
> sample->host_sof = host_sof;
> - sample->host_ts = ts;
> + sample->host_time = time;
>
> /* Update the sliding window head and count. */
> stream->clock.head = (stream->clock.head + 1) % stream->clock.size;
> @@ -613,14 +613,12 @@ void uvc_video_clock_update(struct uvc_streaming
> *stream, struct uvc_clock_sample *first;
> struct uvc_clock_sample *last;
> unsigned long flags;
> - struct timespec ts;
> + u64 timestamp;
> u32 delta_stc;
> u32 y1, y2;
> u32 x1, x2;
> u32 mean;
> u32 sof;
> - u32 div;
> - u32 rem;
> u64 y;
>
> if (!uvc_hw_timestamps_param)
> @@ -667,9 +665,8 @@ void uvc_video_clock_update(struct uvc_streaming
> *stream, if (x1 == x2)
> goto done;
>
> - ts = timespec_sub(last->host_ts, first->host_ts);
> y1 = NSEC_PER_SEC;
> - y2 = (ts.tv_sec + 1) * NSEC_PER_SEC + ts.tv_nsec;
> + y2 = (u32)ktime_to_ns(ktime_sub(last->host_time, first->host_time)) +
> y1;
>
> /* Interpolated and host SOF timestamps can wrap around at slightly
>* different times. Handle this by adding or removing 2048 to or from
> @@ -686,24 +683,18 @@ void uvc_video_clock_update(struct uvc_streaming
> *stream, - (u64)y2 * (u64)x1;
> y = div_u64(y, x2 - x1);
>
> - div = div_u64_rem(y, NSEC_PER_SEC, &rem);
> - ts.tv_sec = first->host_ts.tv_sec - 1 + div;
> - ts.tv_nsec = first->host_ts.tv_nsec + rem;
> - if (ts.tv_nsec >= NSEC_PER_SEC) {
> - ts.tv_sec++;
> - ts.tv_nsec -= NSEC_PER_SEC;
> - }
> + timestamp = ktime_to_ns(first->host_time) + y - y1;
It took me a few minutes to see that the -1 and -y1 were equivalent. And then
more time to re-read the code and the comments to understand what I had done.
I'm impressed that you haven't blindly replaced the +1s and -1s by
+NSEC_PER_SEC and -NSEC_PER_SEC, but used +y1 and -y1 which I think improves
readability.
Reviewed-by: Laurent Pinchart
with the highest honors :-)
Should I merge this through my tree ?
> uvc_trace(UVC_TRACE_CLOCK, "%s: SOF %u.%06llu y %llu ts %llu "
> "buf ts %llu (x1 %u/%u/%u x2 %u/%u/%u y1 %u y2 %u)\n",
> stream->dev->name,
> sof >> 16, div_u64(((u64)sof & 0x) * 100LLU, 65536),
> - y, timespec_to_ns(&ts), vbuf->vb2_buf.timestamp,
> + y, timestamp, vbuf->vb2_buf.timestamp,
> x1, first->host_sof, first->dev_sof,
> x2, last->host_sof, last->dev_sof, y1, y2);
>
> /* Update the V4L2 buffer. */
> - vbuf->vb2_buf.timestamp = timespec_to_ns(&ts);
> + vbuf->vb2_buf.timestamp = timestamp;
>
> done:
> spin_unlock_irqrestore(&clock->lock, flags);
> @@ -1007,8 +998,6 @
Re: [PATCH 3/3 v7] uvcvideo: add a metadata device node
Hi Guennadi,
Thank you for the patch. We're getting very close, I only have small comments,
please see below.
On Wednesday, 8 November 2017 18:00:14 EET Guennadi Liakhovetski wrote:
> Some UVC video cameras contain metadata in their payload headers. This
> patch extracts that data, adding more clock synchronisation information,
> on both bulk and isochronous endpoints and makes it available to the
> user space on a separate video node, using the V4L2_CAP_META_CAPTURE
> capability and the V4L2_BUF_TYPE_META_CAPTURE buffer queue type. Even
> though different cameras will have different metadata formats, we use
> the same V4L2_META_FMT_UVC pixel format for all of them. Users have to
> parse data, based on the specific camera model information. This
> version of the patch only creates such metadata nodes for cameras,
> specifying a UVC_QUIRK_METADATA_NODE quirk flag.
I don't think this is correct anymore, as we'll use different 4CCs for
different vendor metadata. How would you like to rephrase the commit message ?
> Signed-off-by: Guennadi Liakhovetski
> ---
>
> v7: support up to two metadata formats per camera - the standard one and
> an optional private one, if specified in device information
>
> drivers/media/usb/uvc/Makefile | 2 +-
> drivers/media/usb/uvc/uvc_driver.c | 15 +++
> drivers/media/usb/uvc/uvc_isight.c | 2 +-
> drivers/media/usb/uvc/uvc_metadata.c | 204
> drivers/media/usb/uvc/uvc_queue.c| 41 +--
> drivers/media/usb/uvc/uvc_video.c| 127 --
> drivers/media/usb/uvc/uvcvideo.h | 19 +++-
> drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
> include/uapi/linux/uvcvideo.h| 26 +
> 9 files changed, 416 insertions(+), 21 deletions(-)
> create mode 100644 drivers/media/usb/uvc/uvc_metadata.c
[snip]
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index cbf79b9..5f7ce97 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1877,6 +1877,7 @@ static void uvc_unregister_video(struct uvc_device
> *dev) continue;
>
> video_unregister_device(&stream->vdev);
> + video_unregister_device(&stream->meta.vdev);
>
> uvc_debugfs_cleanup_stream(stream);
> }
> @@ -1934,6 +1935,11 @@ static int uvc_register_video(struct uvc_device *dev,
> return ret;
> }
>
> + /* Register a metadata node, but ignore a possible failure, complete
> + * registration of video nodes anyway.
> + */
> + uvc_meta_register(stream);
> +
> if (stream->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> stream->chain->caps |= V4L2_CAP_VIDEO_CAPTURE;
> else
> @@ -2003,6 +2009,7 @@ static int uvc_register_chains(struct uvc_device *dev)
>
> struct uvc_device_info {
> u32 quirks;
> + u32 meta_format;
> };
>
> static int uvc_probe(struct usb_interface *intf,
> @@ -2038,8 +2045,16 @@ static int uvc_probe(struct usb_interface *intf,
> dev->udev = usb_get_dev(udev);
> dev->intf = usb_get_intf(intf);
> dev->intfnum = intf->cur_altsetting->desc.bInterfaceNumber;
> + if (uvc_quirks_param != -1 &&
> + uvc_quirks_param & UVC_DEV_FLAG_METADATA_NODE) {
> + uvc_quirks_param &= ~UVC_DEV_FLAG_METADATA_NODE;
> + if (uvc_quirks_param == 0)
> + uvc_quirks_param = -1;
> + }
I think we can remove the UVC_DEV_FLAG_METADATA_NODE flag. We can use the
metadata format specified in the device information structure if present, and
default to the standard metadata format otherwise. A metadata video node will
always be registered, which I think is the right thing to do as exposing
timestamp information to userspace is useful.
Do you see any adverse effect of registering a metadata video node
unconditionally ?
> dev->quirks = (uvc_quirks_param == -1)
> ? quirks : uvc_quirks_param;
> + if (info)
> + dev->meta_format = info->meta_format;
>
> if (udev->product != NULL)
> strlcpy(dev->name, udev->product, sizeof dev->name);
[snip]
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c
> b/drivers/media/usb/uvc/uvc_metadata.c new file mode 100644
> index 000..219
> --- /dev/null
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
[snip]
> +/*
> + * videobuf2 Queue Operations
> + */
> +
You can remove this now that the section is empty :-)
> +/*
> + * V4L2 ioctls
> + */
> +
> +static int uvc_meta_v4l2_querycap(struct file *file, void *fh,
> + struct v4l2_capability *cap)
> +{
> + struct v4l2_fh *vfh = file->private_data;
> + struct uvc_streaming *stream = video_get_drvdata(vfh->vdev);
> +
> + strlcpy(cap->driver, "uvcvideo", sizeof(cap->driver));
> +
RE: [PATCH v4 00/12] Intel IPU3 ImgU patchset
Hi Hans, Thanks for your patience and sharing your thoughts on this. > Subject: Re: [PATCH v4 00/12] Intel IPU3 ImgU patchset > > Hi Rajmohan, > > On 11/17/2017 03:58 AM, Mani, Rajmohan wrote: > > Hi Sakari and all, > > > >> -Original Message- > >> From: Zhi, Yong > >> Sent: Tuesday, October 17, 2017 8:47 PM > >> To: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com > >> Cc: Zheng, Jian Xu ; Mani, Rajmohan > >> ; Toivonen, Tuukka > >> ; Hu, Jerry W ; > >> a...@arndb.de; h...@lst.de; robin.mur...@arm.com; iommu@lists.linux- > >> foundation.org; Zhi, Yong > >> Subject: [PATCH v4 00/12] Intel IPU3 ImgU patchset > >> > >> This patchset adds support for the Intel IPU3 (Image Processing Unit) > >> ImgU which is essentially a modern memory-to-memory ISP. It > >> implements raw Bayer to YUV image format conversion as well as a > >> large number of other pixel processing algorithms for improving the image > quality. > >> > >> Meta data formats are defined for image statistics (3A, i.e. > >> automatic white balance, exposure and focus, histogram and local area > >> contrast > >> enhancement) as well as for the pixel processing algorithm parameters. > >> The documentation for these formats is currently not included in the > >> patchset but will be added in a future version of this set. > >> > > > > Here is an update on the IPU3 documentation that we are currently working > on. > > > > Image processing in IPU3 relies on the following. > > > > 1) HW configuration to enable ISP and > > 2) setting customer specific 3A Tuning / Algorithm Parameters to achieve > desired image quality. > > > > We intend to provide documentation on ImgU driver programming interface > to help users of this driver to configure and enable ISP HW to meet their > needs. This documentation will include details on complete V4L2 Kernel driver > interface and IO-Control parameters, except for the ISP internal algorithm and > its parameters (which is Intel proprietary IP). > > > > We will also provide an user space library in binary form to help users of > > this > driver, to convert the public 3A tuning parameters to IPU3 algorithm > parameters. This tool will be released under NDA to the users of this driver. > > So I discussed this situation with Sakari in Prague during the ELCE. I am not > happy with adding a driver to the kernel that needs an NDA to be usable. I > can't agree to that. It's effectively the same as firmware that's only > available > under an NDA and we wouldn't accept such drivers either. > Ack > There are a few options: > > 1) Document the ISP parameters and that format they are stored in allowing > for >open source implementations. While this is the ideal, I suspect that this > is >a no-go for Intel. > Ack > 2) The driver can be used without using these ISP parameters and still achieve >'OK' quality. I.e., it's usable for basic webcam usage under normal > lighting >conditions. I'm not sure if this is possible at all, though. > This is something that we have tried and are able to do image capture with the default ISP parameters using ov5670 sensor. Additionally we had to set optimal values for the ov5670 sensor's exposure and gain settings. Please see if the following image looks to meet the 'OK' quality. git clone https://github.com/RajmohanMani/ipu3-misc.git ipu3-misc/ov5670.jpg is the image file. > 3) Make the library available without requiring an NDA. > We are also actively exploring this option to see if this can be done. > 4) Provide a non-NDA library (ideally open source) that achieves at minimum >the quality as described in 2: i.e. usable for basic webcam. > I see this is the same as option 3) + open sourcing the library. Open sourcing the library does not look to be an option. I will reconfirm this. > 5) Other solutions I didn't think of? > > I think 4 might be the best option, especially if it is open sourced and just > uses > non-IP 3A algorithms. It could even be added to the v4l-utils git repo. > > A closed source non-NDA library might also work, but you will need to check > what Mauro thinks about that. I believe this is option 3) that you are referring here. Depending on the progress that we make on option 3), we can work on the next steps. > My problem is that such libraries tend to be > abandoned after a few years and then bit-rot sets in. An open-source solution > is much easier to maintain in the long term. > > Regards, > > Hans >
[PATCH 2/2] uvcvideo: Report V4L2 device caps through the video_device structure
The V4L2 core populates the struct v4l2_capability device_caps field
from the same field in video_device. There's no need to handle that
manually in the driver.
Signed-off-by: Laurent Pinchart
---
drivers/media/usb/uvc/uvc_driver.c | 11 +++
drivers/media/usb/uvc/uvc_v4l2.c | 4
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_driver.c
b/drivers/media/usb/uvc/uvc_driver.c
index b832929d3382..0ebdcc8b4ff6 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1925,6 +1925,17 @@ int uvc_register_video_device(struct uvc_device *dev,
vdev->prio = &stream->chain->prio;
if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
vdev->vfl_dir = VFL_DIR_TX;
+
+ switch (type) {
+ case V4L2_BUF_TYPE_VIDEO_CAPTURE:
+ default:
+ vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+ break;
+ case V4L2_BUF_TYPE_VIDEO_OUTPUT:
+ vdev->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
+ break;
+ }
+
strlcpy(vdev->name, dev->name, sizeof vdev->name);
/*
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 3e7e283a44a8..5e0323982577 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -568,10 +568,6 @@ static int uvc_ioctl_querycap(struct file *file, void *fh,
usb_make_path(stream->dev->udev, cap->bus_info, sizeof(cap->bus_info));
cap->capabilities = V4L2_CAP_DEVICE_CAPS | V4L2_CAP_STREAMING
| chain->caps;
- if (stream->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
- cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
- else
- cap->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
return 0;
}
--
Regards,
Laurent Pinchart
[PATCH 0/2] uvcvideo: Refactor code to ease metadata implementation
Hi Guennadi, This small patch series refactors the uvc_video_register() function to extract the code that you need into a new uvc_video_register_device() function. Please let me know if it can help. Laurent Pinchart (2): uvcvideo: Factor out video device registration to a function uvcvideo: Report V4L2 device caps through the video_device structure drivers/media/usb/uvc/uvc_driver.c | 77 +- drivers/media/usb/uvc/uvc_v4l2.c | 4 -- drivers/media/usb/uvc/uvcvideo.h | 8 3 files changed, 60 insertions(+), 29 deletions(-) -- Regards, Laurent Pinchart
[PATCH 1/2] uvcvideo: Factor out video device registration to a function
The function will then be used to register the video device for metadata
capture.
Signed-off-by: Laurent Pinchart
---
drivers/media/usb/uvc/uvc_driver.c | 66 +++---
drivers/media/usb/uvc/uvcvideo.h | 8 +
2 files changed, 49 insertions(+), 25 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_driver.c
b/drivers/media/usb/uvc/uvc_driver.c
index f77e31fcfc57..b832929d3382 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -24,6 +24,7 @@
#include
#include
+#include
#include "uvcvideo.h"
@@ -1895,52 +1896,63 @@ static void uvc_unregister_video(struct uvc_device *dev)
kref_put(&dev->ref, uvc_delete);
}
-static int uvc_register_video(struct uvc_device *dev,
- struct uvc_streaming *stream)
+int uvc_register_video_device(struct uvc_device *dev,
+ struct uvc_streaming *stream,
+ struct video_device *vdev,
+ struct uvc_video_queue *queue,
+ enum v4l2_buf_type type,
+ const struct v4l2_file_operations *fops,
+ const struct v4l2_ioctl_ops *ioctl_ops)
{
- struct video_device *vdev = &stream->vdev;
int ret;
/* Initialize the video buffers queue. */
- ret = uvc_queue_init(&stream->queue, stream->type, !uvc_no_drop_param);
+ ret = uvc_queue_init(queue, type, !uvc_no_drop_param);
if (ret)
return ret;
- /* Initialize the streaming interface with default streaming
-* parameters.
-*/
- ret = uvc_video_init(stream);
- if (ret < 0) {
- uvc_printk(KERN_ERR, "Failed to initialize the device "
- "(%d).\n", ret);
- return ret;
- }
-
- uvc_debugfs_init_stream(stream);
-
/* Register the device with V4L. */
- /* We already hold a reference to dev->udev. The video device will be
+ /*
+* We already hold a reference to dev->udev. The video device will be
* unregistered before the reference is released, so we don't need to
* get another one.
*/
vdev->v4l2_dev = &dev->vdev;
- vdev->fops = &uvc_fops;
- vdev->ioctl_ops = &uvc_ioctl_ops;
+ vdev->fops = fops;
+ vdev->ioctl_ops = ioctl_ops;
vdev->release = uvc_release;
vdev->prio = &stream->chain->prio;
- if (stream->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+ if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
vdev->vfl_dir = VFL_DIR_TX;
strlcpy(vdev->name, dev->name, sizeof vdev->name);
- /* Set the driver data before calling video_register_device, otherwise
-* uvc_v4l2_open might race us.
+ /*
+* Set the driver data before calling video_register_device, otherwise
+* the file open() handler might race us.
*/
video_set_drvdata(vdev, stream);
ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
if (ret < 0) {
- uvc_printk(KERN_ERR, "Failed to register video device (%d).\n",
+ uvc_printk(KERN_ERR, "Failed to register %s device (%d).\n",
+ v4l2_type_names[type], ret);
+ return ret;
+ }
+
+ kref_get(&dev->ref);
+ return 0;
+}
+
+static int uvc_register_video(struct uvc_device *dev,
+ struct uvc_streaming *stream)
+{
+ int ret;
+
+ /* Initialize the streaming interface with default parameters. */
+ ret = uvc_video_init(stream);
+ if (ret < 0) {
+ uvc_printk(KERN_ERR, "Failed to initialize the device (%d).\n",
ret);
return ret;
}
@@ -1950,8 +1962,12 @@ static int uvc_register_video(struct uvc_device *dev,
else
stream->chain->caps |= V4L2_CAP_VIDEO_OUTPUT;
- kref_get(&dev->ref);
- return 0;
+ uvc_debugfs_init_stream(stream);
+
+ /* Register the device with V4L. */
+ return uvc_register_video_device(dev, stream, &stream->vdev,
+&stream->queue, stream->type,
+&uvc_fops, &uvc_ioctl_ops);
}
/*
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index e2169caefe9e..f5e8a9b17296 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -716,6 +716,14 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
struct vb2_v4l2_buffer *vbuf,
struct uvc_buffer *buf);
+int uvc_register_video_device(struct uvc_device *dev,
+ struct uvc_streaming *stream,
+ struct video_device *vdev,
+ struct uvc_video_queue *queue,
+ enum v4l2
[GIT PULL FOR v4.16] uvcvideo
Hi Mauro, The following changes since commit 781b045baefdabf7e0bc9f33672ca830d3db9f27: media: imx274: Fix error handling, add MAINTAINERS entry (2017-11-30 04:45:12 -0500) are available in the git repository at: git://linuxtv.org/pinchartl/media.git uvc/next for you to fetch changes up to ed0c7e626c71120d844b06ce06c561670e0feb32: uvcvideo: Stream error events carry no data (2017-12-05 00:28:50 +0200) Baoyou Xie (1): uvcvideo: Mark buffer error where overflow Jaejoong Kim (1): uvcvideo: Remove duplicate & operation Laurent Pinchart (1): uvcvideo: Stream error events carry no data Nicolas Dufresne (1): uvcvideo: Add D3DFMT_L8 support drivers/media/usb/uvc/uvc_driver.c | 5 + drivers/media/usb/uvc/uvc_status.c | 5 +++-- drivers/media/usb/uvc/uvc_video.c | 5 +++-- drivers/media/usb/uvc/uvcvideo.h | 5 + 4 files changed, 16 insertions(+), 4 deletions(-) -- Regards, Laurent Pinchart
Re: [PATCH v6 0/9] i2c: document DMA handling and add helpers for it
On Sun, Dec 03, 2017 at 08:43:47PM +0100, Wolfram Sang wrote: > > It's a bit different in that it's much more likely that a SPI controller > > will actually do DMA than an I2C one since the speeds are higher and > > there's frequent applications that do large transfers so it's more > > likely that people will do the right thing as issues would tend to come > > up if they don't. > Yes, for SPI this is true. I was thinking more of regmap with its > usage of different transport mechanisms. But I guess they should all be > DMA safe because some of them need to be DMA safe? Possibly. Hopefully. I guess we'll find out. signature.asc Description: PGP signature
[PATCH 1/1] v4l: fwnode: Add debug prints for V4L2 endpoint property parsing
Print debug info as standard V4L2 endpoint are parsed.
Signed-off-by: Sakari Ailus
---
drivers/media/v4l2-core/v4l2-fwnode.c | 101 +++---
1 file changed, 80 insertions(+), 21 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
b/drivers/media/v4l2-core/v4l2-fwnode.c
index 9c17a26d544c..bc927bbd4160 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -66,6 +66,7 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct
fwnode_handle *fwnode,
lanes_used |= BIT(array[i]);
bus->data_lanes[i] = array[i];
+ pr_debug("lane %u position %u\n", i, array[i]);
}
rval = fwnode_property_read_u32_array(fwnode,
@@ -82,8 +83,13 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct
fwnode_handle *fwnode,
"lane-polarities", array,
1 + bus->num_data_lanes);
- for (i = 0; i < 1 + bus->num_data_lanes; i++)
+ for (i = 0; i < 1 + bus->num_data_lanes; i++) {
bus->lane_polarities[i] = array[i];
+ pr_debug("lane %u polarity %sinverted",
+i, array[i] ? "" : "not ");
+ }
+ } else {
+ pr_debug("no lane polarities defined, assuming not
inverted\n");
}
}
@@ -95,12 +101,15 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct
fwnode_handle *fwnode,
bus->clock_lane = v;
have_clk_lane = true;
+ pr_debug("clock lane position %u\n", v);
}
- if (fwnode_property_present(fwnode, "clock-noncontinuous"))
+ if (fwnode_property_present(fwnode, "clock-noncontinuous")) {
flags |= V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
- else if (have_clk_lane || bus->num_data_lanes > 0)
+ pr_debug("contiguous clock\n");
+ } else if (have_clk_lane || bus->num_data_lanes > 0) {
flags |= V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+ }
bus->flags = flags;
vep->bus_type = V4L2_MBUS_CSI2;
@@ -115,44 +124,63 @@ static void v4l2_fwnode_endpoint_parse_parallel_bus(
unsigned int flags = 0;
u32 v;
- if (!fwnode_property_read_u32(fwnode, "hsync-active", &v))
+ if (!fwnode_property_read_u32(fwnode, "hsync-active", &v)) {
flags |= v ? V4L2_MBUS_HSYNC_ACTIVE_HIGH :
V4L2_MBUS_HSYNC_ACTIVE_LOW;
+ pr_debug("hsync-active %s\n", v ? "high" : "low");
+ }
- if (!fwnode_property_read_u32(fwnode, "vsync-active", &v))
+ if (!fwnode_property_read_u32(fwnode, "vsync-active", &v)) {
flags |= v ? V4L2_MBUS_VSYNC_ACTIVE_HIGH :
V4L2_MBUS_VSYNC_ACTIVE_LOW;
+ pr_debug("vsync-active %s\n", v ? "high" : "low");
+ }
- if (!fwnode_property_read_u32(fwnode, "field-even-active", &v))
+ if (!fwnode_property_read_u32(fwnode, "field-even-active", &v)) {
flags |= v ? V4L2_MBUS_FIELD_EVEN_HIGH :
V4L2_MBUS_FIELD_EVEN_LOW;
+ pr_debug("field-even-active %s\n", v ? "high": "low");
+ }
+
if (flags)
vep->bus_type = V4L2_MBUS_PARALLEL;
else
vep->bus_type = V4L2_MBUS_BT656;
- if (!fwnode_property_read_u32(fwnode, "pclk-sample", &v))
+ if (!fwnode_property_read_u32(fwnode, "pclk-sample", &v)) {
flags |= v ? V4L2_MBUS_PCLK_SAMPLE_RISING :
V4L2_MBUS_PCLK_SAMPLE_FALLING;
+ pr_debug("pclk-sample %s\n", v ? "high" : "low");
+ }
- if (!fwnode_property_read_u32(fwnode, "data-active", &v))
+ if (!fwnode_property_read_u32(fwnode, "data-active", &v)) {
flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
V4L2_MBUS_DATA_ACTIVE_LOW;
+ pr_debug("data-active %s\n", v ? "high" : "low");
+ }
- if (fwnode_property_present(fwnode, "slave-mode"))
+ if (fwnode_property_present(fwnode, "slave-mode")) {
+ pr_debug("slave mode\n");
flags |= V4L2_MBUS_SLAVE;
- else
+ } else {
flags |= V4L2_MBUS_MASTER;
+ }
- if (!fwnode_property_read_u32(fwnode, "bus-width", &v))
+ if (!fwnode_property_read_u32(fwnode, "bus-width", &v)) {
bus->bus_width = v;
+ pr_debug("bus-width %u\n", v);
+ }
- if (!fwnode_property_read_u32(fwnode, "data-shift", &v))
+ if (!fwnode_property_read_u32(fwnode, "data-shift", &v)) {
bus->data_shift = v;
+ pr_debug("data-shift %u\n", v);
+ }
- if (!fwnode_prope
Re: [PATCH v6 0/9] i2c: document DMA handling and add helpers for it
On Sat, Nov 04, 2017 at 09:20:00PM +0100, Wolfram Sang wrote:
> So, after revisiting old mail threads, taking part in a similar discussion on
> the USB list, and implementing a not-convincing solution before, here is what
> I
> cooked up to document and ease DMA handling for I2C within Linux. Please have
> a
> look at the documentation introduced in patch 7 for details. And to make it
> clear again: The stuff below is opt-in. If host drivers are not updated, they
> will continue to work like before.
>
> While previous versions until v3 tried to magically apply bounce buffers when
> needed, it became clear that detecting DMA safe buffers is too fragile. This
> approach is now opt-in, a DMA_SAFE flag needs to be set on an i2c_msg. The
> outcome so far is very convincing IMO. The core additions are simple and easy
> to understand. The driver changes for the Renesas IP cores became easy to
> understand, too.
>
> Of course, we must now whitelist DMA safe buffers. This series implements it
> for core functionality:
>
> a) for the I2C_RDWR when messages come from userspace
> b) for i2c_smbus_xfer_emulated(), DMA safe buffers are now allocated for
>block transfers
> c) i2c_master_{send|recv} have now a *_dmasafe variant
>
> I am still not sure how we can teach regmap this new flag. regmap is a heavy
> user of I2C, so broonie's opinion here would be great to have. The rest should
> mostly be updating individual drivers which can be done when needed.
>
> All patches have been tested with a Renesas Salvator-X board (r8a7796/M3-W)
> and
> Renesas Lager board (r8a7790/H2). But more testing is really really welcome.
>
> The branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git
> renesas/topic/i2c-core-dma-v6
>
> It is planned to land upstream in v4.16 and I want to push it to linux-next
> early after v4.15 to get lots of testing for the core changes.
>
> Big kudos to Renesas Electronics for funding this work, thank you very much!
>
> Regards,
>
>Wolfram
Applied to for-next after fixing some cosmetic checkpatch issues!
signature.asc
Description: PGP signature
Re: [PATCH v12 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
Hejssan!!
On Sat, Dec 02, 2017 at 03:50:21PM +0100, Niklas Söderlund wrote:
> Hej Sakari,
>
> Thanks for your feedback.
>
> On 2017-12-02 16:05:08 +0200, Sakari Ailus wrote:
> > Hejssan,
> >
> > On Sat, Dec 02, 2017 at 12:08:21PM +0100, Niklas Söderlund wrote:
> > ...
> > > > > +static int rcar_csi2_parse_dt(struct rcar_csi2 *priv)
> > > > > +{
> > > > > + struct device_node *ep;
> > > > > + struct v4l2_fwnode_endpoint v4l2_ep;
> > > > > + int ret;
> > > > > +
> > > > > + ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0);
> > > > > + if (!ep) {
> > > > > + dev_err(priv->dev, "Not connected to subdevice\n");
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep),
> > > > > &v4l2_ep);
> > > > > + if (ret) {
> > > > > + dev_err(priv->dev, "Could not parse v4l2 endpoint\n");
> > > > > + of_node_put(ep);
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + ret = rcar_csi2_parse_v4l2(priv, &v4l2_ep);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + priv->remote.match.fwnode.fwnode =
> > > > > + fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep));
> > > >
> > > > To continue the discussion from v10 --- how does this work? The V4L2
> > > > async
> > > > framework does still matching for the node of the device, not the
> > > > endpoint.
> > > >
> > > > My suggestion is to handle this in match_fwnode until all drivers have
> > > > been
> > > > converted. The V4L2 fwnode helper should be changed as well, then you
> > > > could
> > > > use it here, too.
> > >
> > > I agree that the V4L2 async framework should be changed to work with
> > > endpoints and not the node of the device. I also agree on how this
> > > change should be introduced.
> > >
> > > But I don't feel that this change of the framework needs to block this
> > > patch-set. Once the framework is updated to work with endpoints it
> > > should be a trivial patch to change rcar-csi2 to use the new helper. Do
> > > you agree whit this or do you feel that this patch-set should depend on
> > > such change of the framework?
> >
> > Without changes to the framework, I don't think this would work since the
> > async framework (or individual drivers) will assign the device's fwnode,
> > not that of the endpoint, to the fwnode against which to match the async
> > sub-device.
> >
> > Therefore you'll need these changes for this driver to work. Or if you
> > introduce a sub-device driver that uses endpoints as well, then we have two
> > non-interoperable sets of ISP (or bridge) and sub-device drivers. That'd be
> > quite undesirable.
>
> Such a driver is already upstream, the adv748x driver register its
> subdevices using endpoints rather then the node of the device itself.
>
>
> int adv748x_csi2_init(...)
> {
>
> ...
>
> /* Ensure that matching is based upon the endpoint fwnodes */
> tx->sd.fwnode = of_fwnode_handle(ep);
>
> ...
> }
>
>
> A related patch to when the adv748x driver was unstreamed where
> 'v4l2-async: Match parent devices' by Kieran to make this change in
> behavior not to cause the non-interoperable sets your mention. It seems
> however that that change have fallen thru the cracks.
Please see the other patch I just sent you (plus linux-media).
>
> >
> > Or, if you don't care whether it'd work with your use case right now, you
> > could use the helper function without changes. :-)
>
> The adv748x is the primary use-case for the rcar-csi2 driver at the
> moment. So I must either keep this workaround in the driver or make this
> patch-set depend on future framework fixes. I would prefer to be able to
> use the helper as it makes the driver less complex. At the same time I
> don't want yet another framework change as a blocker for this patch-set
> :-)
>
> Once I'm back from my short vacation I will give the framework update a
> try and if it turns out OK I will make this patch-set dependent on those
> changes and squash in my patch which converts rcar-csi2 to use the
> helper which is already done in preparation of future framework updates.
>
> If it turns out the changes needed are complex or get stuck in review I
> would prefer if it was possible to move forward with the workaround in
> the driver for now and move it to the helper once it's available. Do
> this sound like a agreeable plan to you?
Yes, but I think changing the framework should be fine.
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
[RFC 1/1] v4l: async: Use endpoint node, not device node, for fwnode match
V4L2 async framework can use both device's fwnode and endpoints's fwnode
for matching the async sub-device with the sub-device. In order to proceed
moving towards endpoint matching assign the endpoint to the async
sub-device.
As most async sub-device drivers (and the related hardware) only supports
a single endpoint, use the first endpoint found. This works for all
current drivers --- we only ever supported a single async sub-device per
device to begin with.
For async devices that have no endpoints, continue to use the fwnode
related to the device. This includes e.g. lens devices.
Signed-off-by: Sakari Ailus
---
Hi Niklas,
What do you think of this one? I've tested this on N9, both sensor and
flash devices work nicely there. No opportunistic checks for backwards
compatibility are needed.
The changes were surprisingly simple, there are only two drivers that
weren't entirely trivial to change (this part is truly weird in exynos4-is
and xilinx-vipp). Converting the two to use the common parsing functions
would be quite a bit more work and would be very nice to test. The changes
in this patch were still relatively simple.
drivers/media/platform/am437x/am437x-vpfe.c| 2 +-
drivers/media/platform/atmel/atmel-isc.c | 2 +-
drivers/media/platform/atmel/atmel-isi.c | 2 +-
drivers/media/platform/davinci/vpif_capture.c | 2 +-
drivers/media/platform/exynos4-is/media-dev.c | 14 ++
drivers/media/platform/pxa_camera.c| 2 +-
drivers/media/platform/qcom/camss-8x16/camss.c | 2 +-
drivers/media/platform/rcar_drif.c | 2 +-
drivers/media/platform/stm32/stm32-dcmi.c | 2 +-
drivers/media/platform/ti-vpe/cal.c| 2 +-
drivers/media/platform/xilinx/xilinx-vipp.c| 16 +---
drivers/media/v4l2-core/v4l2-async.c | 8 ++--
drivers/media/v4l2-core/v4l2-fwnode.c | 2 +-
13 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/drivers/media/platform/am437x/am437x-vpfe.c
b/drivers/media/platform/am437x/am437x-vpfe.c
index 0997c640191d..892d9e935d25 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2493,7 +2493,7 @@ vpfe_get_pdata(struct platform_device *pdev)
if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
sdinfo->vpfe_param.vdpol = 1;
- rem = of_graph_get_remote_port_parent(endpoint);
+ rem = of_graph_get_remote_endpoint(endpoint);
if (!rem) {
dev_err(&pdev->dev, "Remote device at %pOF not found\n",
endpoint);
diff --git a/drivers/media/platform/atmel/atmel-isc.c
b/drivers/media/platform/atmel/atmel-isc.c
index 13f1c1c797b0..c8bb60eeb629 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -2044,7 +2044,7 @@ static int isc_parse_dt(struct device *dev, struct
isc_device *isc)
if (!epn)
break;
- rem = of_graph_get_remote_port_parent(epn);
+ rem = of_graph_get_remote_endpoint(epn);
if (!rem) {
dev_notice(dev, "Remote device at %pOF not found\n",
epn);
diff --git a/drivers/media/platform/atmel/atmel-isi.c
b/drivers/media/platform/atmel/atmel-isi.c
index e900995143a3..eafdf91a4541 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -1119,7 +1119,7 @@ static int isi_graph_parse(struct atmel_isi *isi, struct
device_node *node)
if (!ep)
return -EINVAL;
- remote = of_graph_get_remote_port_parent(ep);
+ remote = of_graph_get_remote_endpoint(ep);
if (!remote) {
of_node_put(ep);
return -EINVAL;
diff --git a/drivers/media/platform/davinci/vpif_capture.c
b/drivers/media/platform/davinci/vpif_capture.c
index fca4dc829f73..7c9c2b2bb710 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1572,7 +1572,7 @@ vpif_capture_get_pdata(struct platform_device *pdev)
if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
chan->vpif_if.vd_pol = 1;
- rem = of_graph_get_remote_port_parent(endpoint);
+ rem = of_graph_get_remote_endpoint(endpoint);
if (!rem) {
dev_dbg(&pdev->dev, "Remote device at %pOF not found\n",
endpoint);
diff --git a/drivers/media/platform/exynos4-is/media-dev.c
b/drivers/media/platform/exynos4-is/media-dev.c
index 0ef583cfc424..ab5dfe6d7ac4 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -411,7 +411,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
pd->mux_id
Re: [PATCH v6 1/2] media: ov7740: Document device tree bindings
On Mon, Dec 04, 2017 at 02:58:57PM +0800, Wenyou Yang wrote: > Add the device tree binding documentation for the ov7740 sensor driver. > > Signed-off-by: Wenyou Yang > --- > > Changes in v6: None > Changes in v5: None > Changes in v4: None > Changes in v3: > - Explicitly document the "remote-endpoint" property. > > Changes in v2: None > > .../devicetree/bindings/media/i2c/ov7740.txt | 47 > ++ > 1 file changed, 47 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7740.txt Please add acks when posting new versions. Rob
Re: [PATCH] v4l: vsp1: Fix function declaration/definition mismatch
Hi Eugeniu,
On Monday, 4 December 2017 20:10:34 EET Eugeniu Rosca wrote:
> On Mon, Dec 04, 2017 at 12:52:17PM +0200, Laurent Pinchart wrote:
> > On Friday, 24 November 2017 20:40:57 EET Kieran Bingham wrote:
> >> Hi Eugeniu,
> >>
> >> Thankyou for the patch,
> >>
> >> Laurent - Any comments on this? Otherwise I'll bundle this in with my
> >> suspend/resume patch for a pull request.
> >>
> >>
> >>
> >> I was going to say: We know the object is an entity by it's type. Isn't
> >> hgo more descriptive for it's name ?
> >>
> >> However to answer my own question - The implementation function goes on
> >> to define a struct vsp1_hgo *hgo, so no ... the Entity object shouldn't
> >> be hgo.
> >
> > And that's exactly why there's a difference between the declaration and
> > implementation :-) Naming the parameter hgo in the declaration makes it
> > clear to the reader what entity is expected. The parameter is then named
> > entity in the function definition to allow for the vsp1_hgo *hgo local
> > variable.
> >
> > Is the mismatch a real issue ? I don't think the kernel coding style
> > mandates parameter names to be identical between function declaration and
> > definition.
>
> You are the DRM/VSP1 and kernel experts, so feel free to drop the patch.
> Still IMO what makes kernel coding style sweet is its simplicity [1].
> Here is some statistics computed with exuberant ctags and cpccheck.
>
> $ git describe HEAD
> v4.15-rc2
>
> $ ctags --version
> Exuberant Ctags 5.9~svn20110310, Copyright (C) 1996-2009 Darren Hiebert
> Addresses: ,
> http://ctags.sourceforge.net
> Optional compiled features: +wildcards, +regex
>
> # Number of function definitions in drivers/media:
> $ find drivers/media -type d | \
> xargs -I {} sh -c "ctags -x --c-types=f {}/*" | wc -l
> 24913
>
> # Number of func declaration/definition mismatches found by cppcheck:
> $ cppcheck --force --enable=all --inconclusive drivers/media/ 2>&1 | \
> grep declaration | wc -l
> 168
>
> It looks like only (168/24913) * 100% = 0,67% of all drivers/media
> functions have a mismatch between function declaration and function
> definition. Why making this number worse?
Of course the goal is not to introduce a mismatch for the sake of it. It makes
sense for the declaration and definition to match in most cases. My point is
that in this particular case I believe the mismatch makes the code more
readable.
--
Regards,
Laurent Pinchart
Re: [PATCH 3/3] media: dvb_frontend: Add commands implementation for compat ioct
Hi Jaedon, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.15-rc2 next-20171204] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Jaedon-Shin/Add-support-compat-in-dvb_frontend-c/20171204-201817 base: git://linuxtv.org/media_tree.git master config: x86_64-randconfig-ne0-12042359 (attached as .config) compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All warnings (new ones prefixed by >>): drivers/media/dvb-core/dvb_frontend.c:1992:4: error: unknown type name 'compat_uptr_t' compat_uptr_t reserved2; ^ drivers/media/dvb-core/dvb_frontend.c:2000:2: error: unknown type name 'compat_uptr_t' compat_uptr_t props; ^ In file included from include/linux/string.h:6:0, from drivers/media/dvb-core/dvb_frontend.c:30: drivers/media/dvb-core/dvb_frontend.c: In function 'dvb_frontend_handle_compat_ioctl': drivers/media/dvb-core/dvb_frontend.c:2018:29: error: implicit declaration of function 'compat_ptr' [-Werror=implicit-function-declaration] if (copy_from_user(&prop, compat_ptr(arg), sizeof(prop))) ^ include/linux/compiler.h:58:30: note: in definition of macro '__trace_if' if (__builtin_constant_p(!!(cond)) ? !!(cond) : \ ^~~~ >> drivers/media/dvb-core/dvb_frontend.c:2018:3: note: in expansion of macro >> 'if' if (copy_from_user(&prop, compat_ptr(arg), sizeof(prop))) ^~ drivers/media/dvb-core/dvb_frontend.c:2018:29: warning: passing argument 2 of 'copy_from_user' makes pointer from integer without a cast [-Wint-conversion] if (copy_from_user(&prop, compat_ptr(arg), sizeof(prop))) ^ include/linux/compiler.h:58:30: note: in definition of macro '__trace_if' if (__builtin_constant_p(!!(cond)) ? !!(cond) : \ ^~~~ >> drivers/media/dvb-core/dvb_frontend.c:2018:3: note: in expansion of macro >> 'if' if (copy_from_user(&prop, compat_ptr(arg), sizeof(prop))) ^~ In file included from include/linux/poll.h:12:0, from drivers/media/dvb-core/dvb_frontend.c:35: include/linux/uaccess.h:144:1: note: expected 'const void *' but argument is of type 'int' copy_from_user(void *to, const void __user *from, unsigned long n) ^~ In file included from include/linux/string.h:6:0, from drivers/media/dvb-core/dvb_frontend.c:30: drivers/media/dvb-core/dvb_frontend.c:2018:29: warning: passing argument 2 of 'copy_from_user' makes pointer from integer without a cast [-Wint-conversion] if (copy_from_user(&prop, compat_ptr(arg), sizeof(prop))) ^ include/linux/compiler.h:58:42: note: in definition of macro '__trace_if' if (__builtin_constant_p(!!(cond)) ? !!(cond) : \ ^~~~ >> drivers/media/dvb-core/dvb_frontend.c:2018:3: note: in expansion of macro >> 'if' if (copy_from_user(&prop, compat_ptr(arg), sizeof(prop))) ^~ In file included from include/linux/poll.h:12:0, from drivers/media/dvb-core/dvb_frontend.c:35: include/linux/uaccess.h:144:1: note: expected 'const void *' but argument is of type 'int' copy_from_user(void *to, const void __user *from, unsigned long n) ^~ In file included from include/linux/string.h:6:0, from drivers/media/dvb-core/dvb_frontend.c:30: drivers/media/dvb-core/dvb_frontend.c:2018:29: warning: passing argument 2 of 'copy_from_user' makes pointer from integer without a cast [-Wint-conversion] if (copy_from_user(&prop, compat_ptr(arg), sizeof(prop))) ^ include/linux/compiler.h:69:16: note: in definition of macro '__trace_if' __r = !!(cond); \ ^~~~ >> drivers/media/dvb-core/dvb_frontend.c:2018:3: note: in expansion of macro >> 'if' if (copy_from_user(&prop, compat_ptr(arg), sizeof(prop))) ^~ In file included from include/linux/poll.h:12:0, from drivers/media/dvb-core/dvb_frontend.c:35: include/linux/uaccess.h:144:1: note: expected 'const void *' but argument is of type 'int' copy_from_user(void *to, const void __user *from, unsigned long n) ^~ drivers/media/dvb-core/dvb_frontend.c:2030:21: warning: passing
Re: [PATCH] v4l: vsp1: Fix function declaration/definition mismatch
Hello Laurent, Kieran,
On Mon, Dec 04, 2017 at 12:52:17PM +0200, Laurent Pinchart wrote:
> Hello,
>
> On Friday, 24 November 2017 20:40:57 EET Kieran Bingham wrote:
> > Hi Eugeniu,
> >
> > Thankyou for the patch,
> >
> > Laurent - Any comments on this? Otherwise I'll bundle this in with my
> > suspend/resume patch for a pull request.
> >
> >
> >
> > I was going to say: We know the object is an entity by it's type. Isn't hgo
> > more descriptive for it's name ?
> >
> > However to answer my own question - The implementation function goes on to
> > define a struct vsp1_hgo *hgo, so no ... the Entity object shouldn't be hgo.
>
> And that's exactly why there's a difference between the declaration and
> implementation :-) Naming the parameter hgo in the declaration makes it clear
> to the reader what entity is expected. The parameter is then named entity in
> the function definition to allow for the vsp1_hgo *hgo local variable.
>
> Is the mismatch a real issue ? I don't think the kernel coding style mandates
> parameter names to be identical between function declaration and definition.
You are the DRM/VSP1 and kernel experts, so feel free to drop the patch.
Still IMO what makes kernel coding style sweet is its simplicity [1].
Here is some statistics computed with exuberant ctags and cpccheck.
$ git describe HEAD
v4.15-rc2
$ ctags --version
Exuberant Ctags 5.9~svn20110310, Copyright (C) 1996-2009 Darren Hiebert
Addresses: ,
http://ctags.sourceforge.net
Optional compiled features: +wildcards, +regex
# Number of function definitions in drivers/media:
$ find drivers/media -type d | \
xargs -I {} sh -c "ctags -x --c-types=f {}/*" | wc -l
24913
# Number of func declaration/definition mismatches found by cppcheck:
$ cppcheck --force --enable=all --inconclusive drivers/media/ 2>&1 | \
grep declaration | wc -l
168
It looks like only (168/24913) * 100% = 0,67% of all drivers/media
functions have a mismatch between function declaration and function
definition. Why making this number worse?
BR,
Eugeniu.
[1] ./Documentation/process/coding-style.rst: Kernel coding style is super
simple.
Re: [PATCH v4 3/5] media: i2c: Add TDA1997x HDMI receiver driver
On Mon, Dec 4, 2017 at 4:50 AM, Hans Verkuil wrote:
> Hi Tim,
>
> Found a few more small issues. After that's fixed and you have the Ack for the
> bindings this can be merged I think.
Hans,
Thanks. Can you weigh in on the bindings? Rob was hoping for some
discussion on making some generic bus format types for video and I'm
not familiar with the other video encoders/decoders enough to know if
there is enough commonality.
>
> On 11/29/2017 10:19 PM, Tim Harvey wrote:
>> +
>> +/* parse an infoframe and do some sanity checks on it */
>> +static unsigned int
>> +tda1997x_parse_infoframe(struct tda1997x_state *state, u16 addr)
>> +{
>> + struct v4l2_subdev *sd = &state->sd;
>> + union hdmi_infoframe frame;
>> + u8 buffer[40];
>> + u8 reg;
>> + int len, err;
>> +
>> + /* read data */
>> + len = io_readn(sd, addr, sizeof(buffer), buffer);
>> + err = hdmi_infoframe_unpack(&frame, buffer);
>> + if (err) {
>> + v4l_err(state->client,
>> + "failed parsing %d byte infoframe: 0x%04x/0x%02x\n",
>> + len, addr, buffer[0]);
>> + return err;
>> + }
>> + hdmi_infoframe_log(KERN_INFO, &state->client->dev, &frame);
>> + switch (frame.any.type) {
>> + /* Audio InfoFrame: see HDMI spec 8.2.2 */
>> + case HDMI_INFOFRAME_TYPE_AUDIO:
>> + /* sample rate */
>> + switch (frame.audio.sample_frequency) {
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_32000:
>> + state->audio_samplerate = 32000;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_44100:
>> + state->audio_samplerate = 44100;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_48000:
>> + state->audio_samplerate = 48000;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_88200:
>> + state->audio_samplerate = 88200;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_96000:
>> + state->audio_samplerate = 96000;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_176400:
>> + state->audio_samplerate = 176400;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_192000:
>> + state->audio_samplerate = 192000;
>> + break;
>> + default:
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM:
>> + break;
>> + }
>> +
>> + /* sample size */
>> + switch (frame.audio.sample_size) {
>> + case HDMI_AUDIO_SAMPLE_SIZE_16:
>> + state->audio_samplesize = 16;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_SIZE_20:
>> + state->audio_samplesize = 20;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_SIZE_24:
>> + state->audio_samplesize = 24;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_SIZE_STREAM:
>> + default:
>> + break;
>> + }
>> +
>> + /* Channel Count */
>> + state->audio_channels = frame.audio.channels;
>> + if (frame.audio.channel_allocation &&
>> + frame.audio.channel_allocation != state->audio_ch_alloc) {
>> + /* use the channel assignment from the infoframe */
>> + state->audio_ch_alloc = frame.audio.channel_allocation;
>> + tda1997x_configure_audout(sd, state->audio_ch_alloc);
>> + /* reset the audio FIFO */
>> + tda1997x_hdmi_info_reset(sd, RESET_AUDIO, false);
>> + }
>> + break;
>> +
>> + /* Auxiliary Video information (AVI) InfoFrame: see HDMI spec 8.2.1 */
>> + case HDMI_INFOFRAME_TYPE_AVI:
>> + state->colorspace = frame.avi.colorspace;
>> + state->colorimetry = frame.avi.colorimetry;
>> + state->content = frame.avi.content_type;
>> + /* Quantization Range */
>> + switch (state->rgb_quantization_range) {
>> + case V4L2_DV_RGB_RANGE_AUTO:
>> + state->range = frame.avi.quantization_range;
>> + break;
>> + case V4L2_DV_RGB_RANGE_LIMITED:
>> + state->range = HDMI_QUANTIZATION_RANGE_LIMITED;
>> + break;
>> + case V4L2_DV_RGB_RANGE_FULL:
>> + state->range = HDMI_QUANTIZATION_RANGE_FULL;
>> + break;
>> + }
>> + if (state->range == HDMI_QUANTIZATION_RANGE_DEFAULT) {
>> + if (frame.avi.video_code <= 1)
>> + state->range = HDMI_QUANTIZATION_RANGE_FULL;
>> + else
>>
Re: [PATCH 2/3] media: dvb_frontend: Add compat_ioctl callback
Hi Jaedon,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.15-rc2 next-20171204]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Jaedon-Shin/Add-support-compat-in-dvb_frontend-c/20171204-201817
base: git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-x014-201749 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/media/dvb-core/dvb_frontend.c: In function
'dvb_frontend_compat_ioctl':
>> drivers/media/dvb-core/dvb_frontend.c:1985:54: error: implicit declaration
>> of function 'compat_ptr'; did you mean 'complete'?
>> [-Werror=implicit-function-declaration]
return dvb_frontend_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
^~
complete
cc1: some warnings being treated as errors
vim +1985 drivers/media/dvb-core/dvb_frontend.c
1980
1981 #ifdef CONFIG_COMPAT
1982 static long dvb_frontend_compat_ioctl(struct file *file, unsigned int
cmd,
1983unsigned long arg)
1984 {
> 1985 return dvb_frontend_ioctl(file, cmd, (unsigned
> long)compat_ptr(arg));
1986 }
1987 #endif
1988
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH v4 3/5] staging: Introduce NVIDIA Tegra video decoder driver
Hi Dmitry, As you already mention in the TODO, this should become a v4l2 codec driver. Good existing examples are the coda, qcom/venus and mtk-vcodec drivers. One thing that is not clear from this code is if the tegra hardware is a stateful or stateless codec, i.e. does it keep track of the decoder state in the hardware, or does the application have to keep track of the state and provide the state information together with the video data? I ask because at the moment only stateful codecs are supported. Work is ongoing to support stateless codecs, but we don't support that for now. Anyway, I'm OK with merging this in staging. Although I think it should go to staging/media since we want to keep track of it. Regards, Hans On 10/19/2017 11:34 PM, Dmitry Osipenko wrote: > NVIDIA Tegra20/30/114/124/132 SoC's have video decoder engine that > supports standard set of video formats like H.264 / MPEG-4 / WMV / VC1. > Currently implemented decoding of CAVLC H.264 on Tegra20 only. > > Signed-off-by: Dmitry Osipenko > --- > drivers/staging/Kconfig|2 + > drivers/staging/Makefile |1 + > drivers/staging/tegra-vde/Kconfig |7 + > drivers/staging/tegra-vde/Makefile |1 + > drivers/staging/tegra-vde/TODO |5 + > drivers/staging/tegra-vde/uapi.h | 101 +++ > drivers/staging/tegra-vde/vde.c| 1209 > > 7 files changed, 1326 insertions(+) > create mode 100644 drivers/staging/tegra-vde/Kconfig > create mode 100644 drivers/staging/tegra-vde/Makefile > create mode 100644 drivers/staging/tegra-vde/TODO > create mode 100644 drivers/staging/tegra-vde/uapi.h > create mode 100644 drivers/staging/tegra-vde/vde.c > > diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig > index 554683912cff..10c982811093 100644 > --- a/drivers/staging/Kconfig > +++ b/drivers/staging/Kconfig > @@ -118,4 +118,6 @@ source "drivers/staging/vboxvideo/Kconfig" > > source "drivers/staging/pi433/Kconfig" > > +source "drivers/staging/tegra-vde/Kconfig" > + > endif # STAGING > diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile > index 8951c37d8d80..c5ef39767f22 100644 > --- a/drivers/staging/Makefile > +++ b/drivers/staging/Makefile > @@ -49,3 +49,4 @@ obj-$(CONFIG_BCM2835_VCHIQ) += vc04_services/ > obj-$(CONFIG_CRYPTO_DEV_CCREE) += ccree/ > obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo/ > obj-$(CONFIG_PI433) += pi433/ > +obj-$(CONFIG_TEGRA_VDE) += tegra-vde/ > diff --git a/drivers/staging/tegra-vde/Kconfig > b/drivers/staging/tegra-vde/Kconfig > new file mode 100644 > index ..ec3ebdaa > --- /dev/null > +++ b/drivers/staging/tegra-vde/Kconfig > @@ -0,0 +1,7 @@ > +config TEGRA_VDE > + tristate "NVIDIA Tegra Video Decoder Engine driver" > + depends on ARCH_TEGRA || COMPILE_TEST > + select SRAM > + help > + Say Y here to enable support for the NVIDIA Tegra video decoder > + driver. > diff --git a/drivers/staging/tegra-vde/Makefile > b/drivers/staging/tegra-vde/Makefile > new file mode 100644 > index ..e7c0df1174bf > --- /dev/null > +++ b/drivers/staging/tegra-vde/Makefile > @@ -0,0 +1 @@ > +obj-$(CONFIG_TEGRA_VDE) += vde.o > diff --git a/drivers/staging/tegra-vde/TODO b/drivers/staging/tegra-vde/TODO > new file mode 100644 > index ..e98bbc7b3c19 > --- /dev/null > +++ b/drivers/staging/tegra-vde/TODO > @@ -0,0 +1,5 @@ > +TODO: > + - Figure out how generic V4L2 API could be utilized by this driver, > + implement it. > + > +Contact: Dmitry Osipenko > diff --git a/drivers/staging/tegra-vde/uapi.h > b/drivers/staging/tegra-vde/uapi.h > new file mode 100644 > index ..8502032b5ee2 > --- /dev/null > +++ b/drivers/staging/tegra-vde/uapi.h > @@ -0,0 +1,101 @@ > +/* > + * Copyright (C) 2016-2017 Dmitry Osipenko > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > + * ARISING FROM, OUT OF OR I
Re: [PATCH 3/3] media: dvb_frontend: Add commands implementation for compat ioct
Hello
Sorry if I say something completely wrong here, I was thinking to
implement the same on my own
As far as I understand from the patch, you have added two "compact"
specific ioctl commands
As far as I know, the compact_ioctl is called automatically when the
userland is 32 bit and kernel is 64 bit and it must be transparent
With compact specific ioctl command, the application should know if it
is running on top of 64 bit kernel and use them in case
I am not sure if this is the correct approach (also the userland
application should be all upgraded to make use of these new commands)
As far as I have understood, the compatc_ioctl body should adapt the
structures in and out, using the compact_ types
Bye
2017-12-01 13:31 GMT+01:00 Jaedon Shin :
> The dtv_properties structure and the dtv_property structure are
> different sizes in 32-bit and 64-bit system. This patch provides
> FE_SET_PROPERTY and FE_GET_PROPERTY ioctl commands implementation for
> 32-bit user space applications.
>
> Signed-off-by: Jaedon Shin
> ---
> drivers/media/dvb-core/dvb_frontend.c | 131
> ++
> 1 file changed, 131 insertions(+)
>
> diff --git a/drivers/media/dvb-core/dvb_frontend.c
> b/drivers/media/dvb-core/dvb_frontend.c
> index 1ae23403a0ab..f3751a573dfe 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -1976,9 +1976,140 @@ static long dvb_frontend_ioctl(struct file *file,
> unsigned int cmd,
> }
>
> #ifdef CONFIG_COMPAT
> +struct compat_dtv_property {
> + __u32 cmd;
> + __u32 reserved[3];
> + union {
> + __u32 data;
> + struct dtv_fe_stats st;
> + struct {
> + __u8 data[32];
> + __u32 len;
> + __u32 reserved1[3];
> + compat_uptr_t reserved2;
> + } buffer;
> + } u;
> + int result;
> +} __attribute__ ((packed));
> +
> +struct compat_dtv_properties {
> + __u32 num;
> + compat_uptr_t props;
> +};
> +
> +#define COMPAT_FE_SET_PROPERTY_IOW('o', 82, struct compat_dtv_properties)
> +#define COMPAT_FE_GET_PROPERTY_IOR('o', 83, struct compat_dtv_properties)
> +
> +static int dvb_frontend_handle_compat_ioctl(struct file *file, unsigned int
> cmd,
> + unsigned long arg)
> +{
> + struct dvb_device *dvbdev = file->private_data;
> + struct dvb_frontend *fe = dvbdev->priv;
> + struct dvb_frontend_private *fepriv = fe->frontend_priv;
> + int i, err = 0;
> +
> + if (cmd == COMPAT_FE_SET_PROPERTY) {
> + struct compat_dtv_properties prop, *tvps = NULL;
> + struct compat_dtv_property *tvp = NULL;
> +
> + if (copy_from_user(&prop, compat_ptr(arg), sizeof(prop)))
> + return -EFAULT;
> +
> + tvps = ∝
> +
> + /*
> +* Put an arbitrary limit on the number of messages that can
> +* be sent at once
> +*/
> + if (!tvps->num || (tvps->num > DTV_IOCTL_MAX_MSGS))
> + return -EINVAL;
> +
> + tvp = memdup_user(compat_ptr(tvps->props), tvps->num *
> sizeof(*tvp));
> + if (IS_ERR(tvp))
> + return PTR_ERR(tvp);
> +
> + for (i = 0; i < tvps->num; i++) {
> + err = dtv_property_process_set(fe, file,
> + (tvp + i)->cmd,
> + (tvp + i)->u.data);
> + if (err < 0) {
> + kfree(tvp);
> + return err;
> + }
> + }
> + kfree(tvp);
> + } else if (cmd == COMPAT_FE_GET_PROPERTY) {
> + struct compat_dtv_properties prop, *tvps = NULL;
> + struct compat_dtv_property *tvp = NULL;
> + struct dtv_frontend_properties getp = fe->dtv_property_cache;
> +
> + if (copy_from_user(&prop, compat_ptr(arg), sizeof(prop)))
> + return -EFAULT;
> +
> + tvps = ∝
> +
> + /*
> +* Put an arbitrary limit on the number of messages that can
> +* be sent at once
> +*/
> + if (!tvps->num || (tvps->num > DTV_IOCTL_MAX_MSGS))
> + return -EINVAL;
> +
> + tvp = memdup_user(compat_ptr(tvps->props), tvps->num *
> sizeof(*tvp));
> + if (IS_ERR(tvp))
> + return PTR_ERR(tvp);
> +
> + /*
> +* Let's use our own copy of property cache, in order to
> +* avoid mangling with DTV zigzag logic, as drivers might
> +* return crap, if they don't check if the data is available
> +
Re: [PATCH 0/9] media: imx: Add better OF graph support
Hi Steve, On 10/28/2017 10:36 PM, Steve Longerbeam wrote: > This is a set of patches that improve support for more complex OF > graphs. Currently the imx-media driver only supports a single device > with a single port connected directly to either the CSI muxes or the > MIPI CSI-2 receiver input ports. There can't be a multi-port device in > between. This patch set removes those limitations. > > For an example taken from automotive, a camera sensor or decoder could > be literally a remote device accessible over a FPD-III link, via TI > DS90Ux9xx deserializer/serializer pairs. This patch set would support > such OF graphs. > > There are still some assumptions and restrictions, regarding the equivalence > of device-tree ports, port parents, and endpoints to media pads, entities, > and links that have been enumerated in the TODO file. Before I merge this patch series I wanted to know if Sakari's async work that has now been merged (see https://www.spinics.net/lists/linux-media/msg124082.html) affects this patch series. It still applies cleanly, but I wondered if the subnotifier improvements would simplify this driver. Of course, any such simplification can also be done after this series has been applied, but I don't know what your thoughts are on this. Regards, Hans > This patch set supersedes the following patches submitted earlier: > > "[PATCH v2] media: staging/imx: do not return error in link_notify for > unknown sources" > "[PATCH RFC] media: staging/imx: fix complete handler" > > Tested by: Steve Longerbeam > on SabreLite with the OV5640 > > Tested-by: Philipp Zabel > on Nitrogen6X with the TC358743. > > Tested-by: Russell King > with the IMX219 > > > Steve Longerbeam (9): > media: staging/imx: get CSI bus type from nearest upstream entity > media: staging/imx: remove static media link arrays > media: staging/imx: of: allow for recursing downstream > media: staging/imx: remove devname string from imx_media_subdev > media: staging/imx: pass fwnode handle to find/add async subdev > media: staging/imx: remove static subdev arrays > media: staging/imx: convert static vdev lists to list_head > media: staging/imx: reorder function prototypes > media: staging/imx: update TODO > > drivers/staging/media/imx/TODO| 63 +++- > drivers/staging/media/imx/imx-ic-prp.c| 4 +- > drivers/staging/media/imx/imx-media-capture.c | 2 + > drivers/staging/media/imx/imx-media-csi.c | 187 +- > drivers/staging/media/imx/imx-media-dev.c | 400 > ++ > drivers/staging/media/imx/imx-media-internal-sd.c | 253 +++--- > drivers/staging/media/imx/imx-media-of.c | 278 --- > drivers/staging/media/imx/imx-media-utils.c | 122 +++ > drivers/staging/media/imx/imx-media.h | 187 -- > 9 files changed, 722 insertions(+), 774 deletions(-) >
Re: [PATCH] v4l: rcar-vin: Implement V4L2 video node release handler
On 12/04/2017 02:34 PM, Niklas Söderlund wrote: > Hi Hans, > > On 2017-12-04 10:05:35 +0100, Hans Verkuil wrote: >> Hi Niklas, >> >> On 11/16/2017 01:27 AM, Laurent Pinchart wrote: >>> Hi Niklas, >>> >>> On Thursday, 16 November 2017 01:34:33 EET Niklas Söderlund wrote: On 2017-11-16 00:49:07 +0200, Laurent Pinchart wrote: > The rvin_dev data structure contains driver private data for an instance > of the VIN. It is allocated dynamically at probe time, and must be freed > once all users are gone. > > The structure is currently allocated with devm_kzalloc(), which results > in memory being freed when the device is unbound. If a userspace > application is currently performing an ioctl call, or just keeps the > device node open and closes it later, this will lead to accessing freed > memory. > > Fix the problem by implementing a V4L2 release handler for the video > node associated with the VIN instance (called when the last user of the > video node releases it), and freeing memory explicitly from the release > handler. > > Signed-off-by: Laurent Pinchart > Acked-by: Niklas Söderlund This patch is based on-top of the VIN Gen3 enablement series not yet upstream. This is OK for me, just wanted to check that this was the intention as to minimize conflicts between the two. >>> >>> Yes, that's my intention. The patch should be included, or possibly >>> squashed >>> in, your development branch. >>> >> >> Has this patch been added in your v8 series? If not, can you add it when you >> post a v9? > > This patch needs more work, with the video device now registered at > complete() time and unregistered at unbind() time. Applying this would > free the rcar-vin private data structure at unbind() which probably not > what we want. > > I think this issue should be addressed but maybe together with a > patch-set targeting the generic problem with video device lifetimes in > v4l2 framework? For now I would be happy to focus on getting Gen3 > support picked-up and observe what Laurent's work on lifetime issues > brings and adept the rcar-vin driver to take advantage of that once it's > ready. OK. I marked the patch as "Obsoleted" so it doesn't stick around in my patch list. Regards, Hans
Re: [PATCH] v4l: rcar-vin: Implement V4L2 video node release handler
Hi Hans, On 2017-12-04 10:05:35 +0100, Hans Verkuil wrote: > Hi Niklas, > > On 11/16/2017 01:27 AM, Laurent Pinchart wrote: > > Hi Niklas, > > > > On Thursday, 16 November 2017 01:34:33 EET Niklas Söderlund wrote: > >> On 2017-11-16 00:49:07 +0200, Laurent Pinchart wrote: > >>> The rvin_dev data structure contains driver private data for an instance > >>> of the VIN. It is allocated dynamically at probe time, and must be freed > >>> once all users are gone. > >>> > >>> The structure is currently allocated with devm_kzalloc(), which results > >>> in memory being freed when the device is unbound. If a userspace > >>> application is currently performing an ioctl call, or just keeps the > >>> device node open and closes it later, this will lead to accessing freed > >>> memory. > >>> > >>> Fix the problem by implementing a V4L2 release handler for the video > >>> node associated with the VIN instance (called when the last user of the > >>> video node releases it), and freeing memory explicitly from the release > >>> handler. > >>> > >>> Signed-off-by: Laurent Pinchart > >>> > >> > >> Acked-by: Niklas Söderlund > >> > >> This patch is based on-top of the VIN Gen3 enablement series not yet > >> upstream. This is OK for me, just wanted to check that this was the > >> intention as to minimize conflicts between the two. > > > > Yes, that's my intention. The patch should be included, or possibly > > squashed > > in, your development branch. > > > > Has this patch been added in your v8 series? If not, can you add it when you > post a v9? This patch needs more work, with the video device now registered at complete() time and unregistered at unbind() time. Applying this would free the rcar-vin private data structure at unbind() which probably not what we want. I think this issue should be addressed but maybe together with a patch-set targeting the generic problem with video device lifetimes in v4l2 framework? For now I would be happy to focus on getting Gen3 support picked-up and observe what Laurent's work on lifetime issues brings and adept the rcar-vin driver to take advantage of that once it's ready. > > Thanks, > > Hans -- Regards, Niklas Söderlund
[PATCH for 4.15] omapdrm/dss/hdmi4_cec: fix interrupt handling
The omap4 CEC hardware cannot tell a Nack from a Low Drive from an
Arbitration Lost error, so just report a Nack, which is almost
certainly the reason for the error anyway.
This also simplifies the implementation. The only three interrupts
that need to be enabled are:
Transmit Buffer Full/Empty Change event: triggered when the
transmit finished successfully and cleared the buffer.
Receiver FIFO Not Empty event: triggered when a message was received.
Frame Retransmit Count Exceeded event: triggered when a transmit
failed repeatedly, usually due to the message being Nacked. Other
reasons are possible (Low Drive, Arbitration Lost) but there is no
way to know. If this happens the TX buffer needs to be cleared
manually.
While testing various error conditions I noticed that the hardware
can receive messages up to 18 bytes in total, which exceeds the legal
maximum of 16. This could cause a buffer overflow, so we check for
this and constrain the size to 16 bytes.
The old incorrect interrupt handler could cause the CEC framework to
enter into a bad state because it mis-detected the "Start Bit Irregularity
event" as an ARB_LOST transmit error when it actually is a receive error
which should be ignored.
Signed-off-by: Hans Verkuil
Reported-by: Henrik Austad
Tested-by: Henrik Austad
Tested-by: Hans Verkuil
---
drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c | 46 +++--
1 file changed, 9 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
index d86873f2abe6..108a84cedaf6 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
@@ -78,6 +78,8 @@ static void hdmi_cec_received_msg(struct hdmi_core_data *core)
/* then read the message */
msg.len = cnt & 0xf;
+ if (msg.len > CEC_MAX_MSG_SIZE - 2)
+ msg.len = CEC_MAX_MSG_SIZE - 2;
msg.msg[0] = hdmi_read_reg(core->base,
HDMI_CEC_RX_CMD_HEADER);
msg.msg[1] = hdmi_read_reg(core->base,
@@ -104,26 +106,6 @@ static void hdmi_cec_received_msg(struct hdmi_core_data
*core)
}
}
-static void hdmi_cec_transmit_fifo_empty(struct hdmi_core_data *core, u32
stat1)
-{
- if (stat1 & 2) {
- u32 dbg3 = hdmi_read_reg(core->base, HDMI_CEC_DBG_3);
-
- cec_transmit_done(core->adap,
- CEC_TX_STATUS_NACK |
- CEC_TX_STATUS_MAX_RETRIES,
- 0, (dbg3 >> 4) & 7, 0, 0);
- } else if (stat1 & 1) {
- cec_transmit_done(core->adap,
- CEC_TX_STATUS_ARB_LOST |
- CEC_TX_STATUS_MAX_RETRIES,
- 0, 0, 0, 0);
- } else if (stat1 == 0) {
- cec_transmit_done(core->adap, CEC_TX_STATUS_OK,
- 0, 0, 0, 0);
- }
-}
-
void hdmi4_cec_irq(struct hdmi_core_data *core)
{
u32 stat0 = hdmi_read_reg(core->base, HDMI_CEC_INT_STATUS_0);
@@ -132,27 +114,21 @@ void hdmi4_cec_irq(struct hdmi_core_data *core)
hdmi_write_reg(core->base, HDMI_CEC_INT_STATUS_0, stat0);
hdmi_write_reg(core->base, HDMI_CEC_INT_STATUS_1, stat1);
- if (stat0 & 0x40)
+ if (stat0 & 0x20) {
+ cec_transmit_done(core->adap, CEC_TX_STATUS_OK,
+ 0, 0, 0, 0);
REG_FLD_MOD(core->base, HDMI_CEC_DBG_3, 0x1, 7, 7);
- else if (stat0 & 0x24)
- hdmi_cec_transmit_fifo_empty(core, stat1);
- if (stat1 & 2) {
+ } else if (stat1 & 0x02) {
u32 dbg3 = hdmi_read_reg(core->base, HDMI_CEC_DBG_3);
cec_transmit_done(core->adap,
CEC_TX_STATUS_NACK |
CEC_TX_STATUS_MAX_RETRIES,
0, (dbg3 >> 4) & 7, 0, 0);
- } else if (stat1 & 1) {
- cec_transmit_done(core->adap,
- CEC_TX_STATUS_ARB_LOST |
- CEC_TX_STATUS_MAX_RETRIES,
- 0, 0, 0, 0);
+ REG_FLD_MOD(core->base, HDMI_CEC_DBG_3, 0x1, 7, 7);
}
if (stat0 & 0x02)
hdmi_cec_received_msg(core);
- if (stat1 & 0x3)
- REG_FLD_MOD(core->base, HDMI_CEC_DBG_3, 0x1, 7, 7);
}
static bool hdmi_cec_clear_tx_fifo(struct cec_adapter *adap)
@@ -231,18 +207,14 @@ static int hdmi_cec_adap_enable(struct cec_adapter *adap,
bool enable)
/*
* Enable CEC interrupts:
* Transmit Buffer Full/Empty Change event
-* Transmitter FIFO Empty event
* Receiver FIFO Not Empty event
*/
- hdmi_write_reg(core->base, HDMI_CEC_INT_ENABLE_0,
[GIT PULL FOR v4.16 v2] Various fixes all over the place
This is the same as the previous version of this pull request (see https://patchwork.linuxtv.org/patch/45733/), except that the patch "media: staging/imx: do not return error in link_notify for unknown sources" from Steve Longerbeam has been dropped. When going through newer imx patches I noticed that those supersede that imx patch. Regards, Hans The following changes since commit 781b045baefdabf7e0bc9f33672ca830d3db9f27: media: imx274: Fix error handling, add MAINTAINERS entry (2017-11-30 04:45:12 -0500) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.16a for you to fetch changes up to ebf1e3656a6455c79063539f59e7cd5d62ceb25e: media: exynos4-is: Use PTR_ERR_OR_ZERO() (2017-12-04 13:57:40 +0100) Arnd Bergmann (5): pulse8-cec: print time using time64_t. vivid: print time in y2038-safe way solo6x10: use ktime_get_ts64() for time sync staging: imx: use ktime_t for timestamps vivid: use ktime_t for timestamp calculation Dan Carpenter (1): cpia2: Fix a couple off by one bugs Dean A (1): s2255drv: update firmware load. Geert Uytterhoeven (1): media: i2c: adv748x: Restore full DT paths in kernel messages Greg Kroah-Hartman (1): media: usbvision: remove unneeded DRIVER_LICENSE #define Gustavo A. R. Silva (2): staging: media: imx: fix inconsistent IS_ERR and PTR_ERR davinci: vpif_capture: add NULL check on devm_kzalloc return value Hans Verkuil (4): vimc: add test_pattern and h/vflip controls to the sensor cec: add the adap_monitor_pin_enable op cec-core.rst: document the new adap_monitor_pin_enable op cec: disable the hardware when unregistered Jesse Chan (3): media: mtk-vcodec: add missing MODULE_LICENSE/DESCRIPTION soc_camera: soc_scale_crop: add missing MODULE_DESCRIPTION/AUTHOR/LICENSE media: tegra-cec: add missing MODULE_DESCRIPTION/AUTHOR/LICENSE Loic Poulain (2): media: venus: venc: configure entropy mode media: venus: venc: Apply inloop deblocking filter Martin Kepplinger (1): media: coda: remove definition of CODA_STD_MJPG Stanimir Varbanov (1): venus: cleanup set_property controls Vasyl Gomonovych (1): media: exynos4-is: Use PTR_ERR_OR_ZERO() Documentation/media/kapi/cec-core.rst | 14 ++ drivers/media/cec/cec-adap.c| 23 drivers/media/cec/cec-api.c | 32 ++ drivers/media/cec/cec-core.c| 15 ++ drivers/media/cec/cec-priv.h| 2 ++ drivers/media/i2c/adv748x/adv748x-core.c| 10 +++ drivers/media/pci/solo6x10/solo6x10-core.c | 17 ++-- drivers/media/platform/coda/coda_regs.h | 1 - drivers/media/platform/davinci/vpif_capture.c | 2 ++ drivers/media/platform/exynos4-is/fimc-lite.c | 5 +--- drivers/media/platform/mtk-vcodec/mtk_vcodec_util.c | 3 ++ drivers/media/platform/qcom/venus/core.h| 4 +-- drivers/media/platform/qcom/venus/hfi_cmds.c| 73 ++--- drivers/media/platform/qcom/venus/hfi_helper.h | 4 +-- drivers/media/platform/qcom/venus/venc.c| 33 ++ drivers/media/platform/soc_camera/soc_scale_crop.c | 4 +++ drivers/media/platform/tegra-cec/tegra_cec.c| 5 drivers/media/platform/vimc/vimc-common.h | 5 drivers/media/platform/vimc/vimc-sensor.c | 63 +- drivers/media/platform/vivid/vivid-core.c | 2 +- drivers/media/platform/vivid/vivid-core.h | 2 +- drivers/media/platform/vivid/vivid-radio-rx.c | 11 drivers/media/platform/vivid/vivid-radio-tx.c | 8 ++ drivers/media/platform/vivid/vivid-rds-gen.c| 2 +- drivers/media/platform/vivid/vivid-rds-gen.h| 1 + drivers/media/platform/vivid/vivid-vbi-gen.c| 2 +- drivers/media/usb/cpia2/cpia2_v4l.c | 4 +-- drivers/media/usb/pulse8-cec/pulse8-cec.c | 4 +-- drivers/media/usb/s2255/s2255drv.c | 13 - drivers/media/usb/usbvision/usbvision-video.c | 3 +- drivers/staging/media/imx/imx-media-csi.c | 10 ++- drivers/staging/media/imx/imx-media-fim.c | 30 +++- drivers/staging/media/imx/imx-media.h | 2 +- include/media/cec.h | 13 + 34 files changed, 260 insertions(+), 162 deletions(-)
Re: [PATCH 3/3] media: dvb_frontend: Add commands implementation for compat ioct
Hi Jaedon,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.15-rc2 next-20171204]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Jaedon-Shin/Add-support-compat-in-dvb_frontend-c/20171204-201817
base: git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-x014-201749 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
>> drivers/media//dvb-core/dvb_frontend.c:1992:4: error: unknown type name
>> 'compat_uptr_t'
compat_uptr_t reserved2;
^
drivers/media//dvb-core/dvb_frontend.c:2000:2: error: unknown type name
'compat_uptr_t'
compat_uptr_t props;
^
drivers/media//dvb-core/dvb_frontend.c: In function
'dvb_frontend_handle_compat_ioctl':
drivers/media//dvb-core/dvb_frontend.c:2018:29: error: implicit declaration
of function 'compat_ptr'; did you mean 'complete'?
[-Werror=implicit-function-declaration]
if (copy_from_user(&prop, compat_ptr(arg), sizeof(prop)))
^~
complete
>> drivers/media//dvb-core/dvb_frontend.c:2018:29: warning: passing argument 2
>> of 'copy_from_user' makes pointer from integer without a cast
>> [-Wint-conversion]
In file included from include/linux/poll.h:12:0,
from drivers/media//dvb-core/dvb_frontend.c:35:
include/linux/uaccess.h:144:1: note: expected 'const void *' but argument is
of type 'int'
copy_from_user(void *to, const void __user *from, unsigned long n)
^~
>> drivers/media//dvb-core/dvb_frontend.c:2030:21: warning: passing argument 1
>> of 'memdup_user' makes pointer from integer without a cast [-Wint-conversion]
tvp = memdup_user(compat_ptr(tvps->props), tvps->num * sizeof(*tvp));
^~
In file included from drivers/media//dvb-core/dvb_frontend.c:30:0:
include/linux/string.h:13:14: note: expected 'const void *' but argument is
of type 'int'
extern void *memdup_user(const void __user *, size_t);
^~~
drivers/media//dvb-core/dvb_frontend.c:2049:29: warning: passing argument 2
of 'copy_from_user' makes pointer from integer without a cast [-Wint-conversion]
if (copy_from_user(&prop, compat_ptr(arg), sizeof(prop)))
^~
In file included from include/linux/poll.h:12:0,
from drivers/media//dvb-core/dvb_frontend.c:35:
include/linux/uaccess.h:144:1: note: expected 'const void *' but argument is
of type 'int'
copy_from_user(void *to, const void __user *from, unsigned long n)
^~
drivers/media//dvb-core/dvb_frontend.c:2061:21: warning: passing argument 1
of 'memdup_user' makes pointer from integer without a cast [-Wint-conversion]
tvp = memdup_user(compat_ptr(tvps->props), tvps->num * sizeof(*tvp));
^~
In file included from drivers/media//dvb-core/dvb_frontend.c:30:0:
include/linux/string.h:13:14: note: expected 'const void *' but argument is
of type 'int'
extern void *memdup_user(const void __user *, size_t);
^~~
>> drivers/media//dvb-core/dvb_frontend.c:2087:20: warning: cast to pointer
>> from integer of different size [-Wint-to-pointer-cast]
if (copy_to_user((void __user *)compat_ptr(tvps->props), tvp,
^
cc1: some warnings being treated as errors
vim +/compat_uptr_t +1992 drivers/media//dvb-core/dvb_frontend.c
1980
1981 #ifdef CONFIG_COMPAT
1982 struct compat_dtv_property {
1983 __u32 cmd;
1984 __u32 reserved[3];
1985 union {
1986 __u32 data;
1987 struct dtv_fe_stats st;
1988 struct {
1989 __u8 data[32];
1990 __u32 len;
1991 __u32 reserved1[3];
> 1992 compat_uptr_t reserved2;
1993 } buffer;
1994 } u;
1995 int result;
1996 } __attribute__ ((packed));
1997
1998 struct compat_dtv_properties {
1999 __u32 num;
> 2000 compat_uptr_t props;
2001 };
2002
2003 #define COMPAT_FE_SET_PROPERTY _IOW('o', 82, struct
compat_dtv_properties)
2004 #define COMPAT_FE_GET_PROPERTY _IOR('o', 83, struct
compat_dtv_properties)
20
Re: [PATCH v4 3/5] media: i2c: Add TDA1997x HDMI receiver driver
Hi Tim, Found a few more small issues. After that's fixed and you have the Ack for the bindings this can be merged I think. On 11/29/2017 10:19 PM, Tim Harvey wrote: > Add support for the TDA1997x HDMI receivers. > > Cc: Hans Verkuil > Signed-off-by: Tim Harvey > --- > v4: > - move include/dt-bindings/media/tda1997x.h to bindings patch > - fix typos > - fix default quant range for VGA > - fix quant range handling and conv matrix > - add additional standards and capabilities to timings_cap > > v3: > - use V4L2_DV_BT_FRAME_WIDTH/HEIGHT macros > - fixed missing break > - use only hdmi_infoframe_log for infoframe logging > - simplify tda1997x_s_stream error handling > - add delayed work proc to handle hotplug enable/disable > - fix set_edid (disable HPD before writing, enable after) > - remove enabling edid by default > - initialize timings > - take quant range into account in colorspace conversion > - remove vendor/product tracking (we provide this in log_status via > infoframes) > - add v4l_controls > - add more detail to log_status > - calculate vhref generator timings > - timing detection fixes (rounding errors, hswidth errors) > - rename configure_input/configure_conv functions > > v2: > - implement dv timings enum/cap > - remove deprecated g_mbus_config op > - fix dv_query_timings > - add EDID get/set handling > - remove max-pixel-rate support > - add audio codec DAI support > - change audio bindings > --- > drivers/media/i2c/Kconfig|9 + > drivers/media/i2c/Makefile |1 + > drivers/media/i2c/tda1997x.c | 3516 > ++ > include/media/i2c/tda1997x.h | 53 + > 4 files changed, 3579 insertions(+) > create mode 100644 drivers/media/i2c/tda1997x.c > create mode 100644 include/media/i2c/tda1997x.h > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index 3c6d642..abf24b9 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -56,6 +56,15 @@ config VIDEO_TDA9840 > To compile this driver as a module, choose M here: the > module will be called tda9840. > > +config VIDEO_TDA1997X > + tristate "NXP TDA1997x HDMI receiver" > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API > + ---help--- > + V4L2 subdevice driver for the NXP TDA1997x HDMI receivers. > + > + To compile this driver as a module, choose M here: the > + module will be called tda1997x. > + > config VIDEO_TEA6415C > tristate "Philips TEA6415C audio processor" > depends on I2C > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index 548a9ef..adfcae9 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -13,6 +13,7 @@ obj-$(CONFIG_VIDEO_TVAUDIO) += tvaudio.o > obj-$(CONFIG_VIDEO_TDA7432) += tda7432.o > obj-$(CONFIG_VIDEO_SAA6588) += saa6588.o > obj-$(CONFIG_VIDEO_TDA9840) += tda9840.o > +obj-$(CONFIG_VIDEO_TDA1997X) += tda1997x.o > obj-$(CONFIG_VIDEO_TEA6415C) += tea6415c.o > obj-$(CONFIG_VIDEO_TEA6420) += tea6420.o > obj-$(CONFIG_VIDEO_SAA7110) += saa7110.o > diff --git a/drivers/media/i2c/tda1997x.c b/drivers/media/i2c/tda1997x.c > new file mode 100644 > index 000..70e9680 > --- /dev/null > +++ b/drivers/media/i2c/tda1997x.c > @@ -0,0 +1,3516 @@ > +/* > + * Copyright (C) 2017 Gateworks Corporation > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +#include > + > +/* debug level */ > +static int debug; > +module_param(debug, int, 0644); > +MODULE_PARM_DESC(debug, "debug level (0-2)"); > + > +/* Page 0x00 - General Control */ > +#define REG_VERSION 0x > +#define REG_INPUT_SEL0x0001 > +#define REG_SVC_MODE 0x0002 > +#define REG_HPD_MAN_CTRL 0x0003 > +#define REG_RT_MAN_CTRL 0x0004 > +#define REG_STANDBY_SOFT_RST 0x000A > +#define REG_HDMI_SOFT_RST0x000B > +#define REG_HDMI_INFO_RST0x000C > +#define REG_INT_FLG_CLR_TOP 0x000E > +#define REG_INT_FLG_CLR_SUS 0x000F > +#define REG_INT_FLG_CLR_DDC 0x0010 > +#define REG_INT_FLG_CLR_RATE 0x0011 > +#define REG_INT_FLG_CLR_MODE 0x0012 > +#define REG_INT_FLG_CLR_INFO 0x0013 > +#define REG_INT_FLG_CLR_AUDIO0x0014 > +#define REG_INT_FLG_CLR_HDCP 0x0015 > +#define REG_INT_FLG_CLR_AFE 0x0016 > +#define REG_INT_MASK_TOP 0x0017 > +#define REG_INT_MASK_SUS 0x0018 > +#define REG_INT_MASK_DDC 0x0019 > +#define REG_INT_MASK_RATE0x001A > +#define R
[GIT PULL FOR v4.16] Various fixes
The usual set of fixes, cleanups and small enhancements. Regards, Hans The following changes since commit 781b045baefdabf7e0bc9f33672ca830d3db9f27: media: imx274: Fix error handling, add MAINTAINERS entry (2017-11-30 04:45:12 -0500) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.16b for you to fetch changes up to 7ca70cd3b7828df33fc85abc36c5ec924ea6b2e5: cx23885: Handle return value of kasprintf (2017-12-04 12:53:14 +0100) Andy Shevchenko (2): media: adv7180: Remove duplicate checks media: i2c: adv748x: Remove duplicate NULL check Arnd Bergmann (1): solo6x10: hide unused variable Arvind Yadav (2): hdpvr: Fix an error handling path in hdpvr_probe() cx23885: Handle return value of kasprintf Colin Ian King (4): tuners: mxl5005s: make arrays static const, reduces object code size cxusb: pass buf as a const u8 * pointer and make buf static const pt3: remove redundant assignment to mask drivers/media/pci/zoran: remove redundant assignment to pointer h Dan Carpenter (1): stk-webcam: Fix use after free on disconnect Hans Verkuil (2): adv7604.c: add missing return cec-adap: add '0x' prefix when printing status Ian Jamison (1): media: imx: Remove incorrect check for queue state in start/stop_streaming Joe Perches (2): gspca: Convert PERR to gspca_err gspca: Convert PDEBUG to gspca_dbg Jérémy Lefaure (1): media: use ARRAY_SIZE Kieran Bingham (2): media: i2c: adv748x: Store the pixel rate ctrl on CSI objects media: vsp1: Prevent suspending and resuming DRM pipelines Romain Reignier (1): media: cx231xx: Add support for The Imaging Source DFG/USB2pro Srishti Sharma (2): Staging: media: omap4iss: Use WARN_ON() instead of BUG_ON(). Staging: media: imx: Prefer using BIT macro Stanimir Varbanov (1): venus: venc: set correctly GOP size and number of B-frames drivers/media/cec/cec-adap.c | 2 +- drivers/media/common/saa7146/saa7146_video.c | 9 ++- drivers/media/dvb-frontends/cxd2841er.c | 7 +-- drivers/media/i2c/adv7180.c | 12 ++-- drivers/media/i2c/adv748x/adv748x-afe.c | 1 + drivers/media/i2c/adv748x/adv748x-core.c | 6 +- drivers/media/i2c/adv748x/adv748x-csi2.c | 13 +++-- drivers/media/i2c/adv748x/adv748x.h | 1 + drivers/media/i2c/adv7604.c | 1 + drivers/media/pci/cx23885/cx23885-input.c| 15 - drivers/media/pci/pt3/pt3_i2c.c | 1 - drivers/media/pci/saa7146/hexium_gemini.c| 3 +- drivers/media/pci/saa7146/hexium_orion.c | 3 +- drivers/media/pci/saa7146/mxb.c | 3 +- drivers/media/pci/solo6x10/solo6x10-gpio.c | 2 + drivers/media/pci/zoran/videocodec.c | 1 - drivers/media/platform/qcom/venus/venc.c | 15 +++-- drivers/media/platform/qcom/venus/venc_ctrls.c | 59 +++- drivers/media/platform/vsp1/vsp1_drv.c | 16 +- drivers/media/tuners/mxl5005s.c | 17 -- drivers/media/usb/cx231xx/cx231xx-cards.c| 28 ++ drivers/media/usb/cx231xx/cx231xx.h | 1 + drivers/media/usb/dvb-usb/cxusb.c| 8 ++- drivers/media/usb/dvb-usb/friio-fe.c | 5 +- drivers/media/usb/gspca/autogain_functions.c | 16 +++--- drivers/media/usb/gspca/benq.c | 8 +-- drivers/media/usb/gspca/conex.c | 13 +++-- drivers/media/usb/gspca/cpia1.c | 74 drivers/media/usb/gspca/dtcs033.c| 28 +- drivers/media/usb/gspca/etoms.c | 38 ++--- drivers/media/usb/gspca/finepix.c| 4 +- drivers/media/usb/gspca/gl860/gl860.c| 37 ++-- drivers/media/usb/gspca/gspca.c | 153 +++--- drivers/media/usb/gspca/gspca.h | 9 +-- drivers/media/usb/gspca/jeilinj.c| 19 --- drivers/media/usb/gspca/jl2005bcd.c | 45 --- drivers/media/usb/gspca/kinect.c | 11 ++-- drivers/media/usb/gspca/konica.c | 28 +- drivers/media/usb/gspca/m5602/m5602_core.c | 34 +-- drivers/media/usb/gspca/m5602/m5602_mt9m111.c| 21 +++ drivers/media/usb/gspca/m5602/m5602_ov7660.c | 11 ++-- drivers/media/usb/gspca/m5602/m5602_ov9650.c | 26 - drivers/media/usb/gspca/m5602/m5602_po1030.c | 27 - drivers/media/usb/gspca/m5602/m5602_s5k4aa.c | 16 +++--- drivers/media/usb/gspca/m5602/m5602_s5k83a.c | 2 +- drivers/media/usb/gspca/mars.c | 4 +- drivers/media/usb/gspca/mr
Re: [PATCH] Support HVR-1200 analog video as a clone of HVR-1500. Tested, composite and s-video inputs.
Hi Nigel,
Can you repost this as a proper patch? It doesn't apply (issues with tabs and
whitespace: please use tabs!), and I am missing a "Signed-off-by" line (see
https://elinux.org/Developer_Certificate_Of_Origin).
Thanks!
Hans
On 09/19/2017 09:16 PM, Nigel Kettlewell wrote:
> [adding kernel mailing lists missed from my reply]
>
> Thank you, yes I think I cribbed too much from the 1500. I think the
> tuner part is not necessary: I have no analog over-the-air signal so I
> cannot test it, hence I have removed the tuner element from the patch
> (below).
>
> I have tested DVB-T which works fine. dmesg shows no errors (attached).
>
> DISPLAY=xxx:0.0 vlc dvb-t://frequency=49800:bandwidth=8
> --dvb-adapter=0 --programs=8373
>
>
> /usr/local/bin/v4l2-ctl --set-input 1
> /usr/local/bin/v4l2-ctl -s 0x00f7
> cat /dev/video0 > /tmp/svideo.raw
>
> ffmpeg -f rawvideo -pix_fmt yuyv422 -r 25 -s:v 720x576 -i
> /tmp/svideo.raw -vcodec mpeg2video -vb 2000k -y /tmp/svideo.mpg
>
>
> Revised patch:
>
> ---
> drivers/media/pci/cx23885/cx23885-cards.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/media/pci/cx23885/cx23885-cards.c
> b/drivers/media/pci/cx23885/cx23885-cards.c
> index 0350f13..1b685f0 100644
> --- a/drivers/media/pci/cx23885/cx23885-cards.c
> +++ b/drivers/media/pci/cx23885/cx23885-cards.c
> @@ -196,7 +196,22 @@ struct cx23885_board cx23885_boards[] = {
> },
> [CX23885_BOARD_HAUPPAUGE_HVR1200] = {
> .name = "Hauppauge WinTV-HVR1200",
> + .porta = CX23885_ANALOG_VIDEO,
> .portc = CX23885_MPEG_DVB,
> + .input = {{
> + .type = CX23885_VMUX_COMPOSITE1,
> + .vmux = CX25840_VIN7_CH3 |
> + CX25840_VIN4_CH2 |
> + CX25840_VIN6_CH1,
> + .gpio0 = 0,
> + }, {
> + .type = CX23885_VMUX_SVIDEO,
> + .vmux = CX25840_VIN7_CH3 |
> + CX25840_VIN4_CH2 |
> + CX25840_VIN8_CH1 |
> + CX25840_SVIDEO_ON,
> + .gpio0 = 0,
> + } },
> },
> [CX23885_BOARD_HAUPPAUGE_HVR1700] = {
> .name = "Hauppauge WinTV-HVR1700",
> @@ -2260,6 +2275,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
> case CX23885_BOARD_HAUPPAUGE_HVR1290:
> case CX23885_BOARD_LEADTEK_WINFAST_PXTV1200:
> case CX23885_BOARD_GOTVIEW_X5_3D_HYBRID:
> + case CX23885_BOARD_HAUPPAUGE_HVR1200:
> case CX23885_BOARD_HAUPPAUGE_HVR1500:
> case CX23885_BOARD_MPX885:
> case CX23885_BOARD_MYGICA_X8507:
> --
> 2.9.4
>
>> Nigel Kettlewell
>> 19 September 2017 12:49
>> Thank you, yes I think I cribbed too much from the 1500. I think the
>> tuner part is not necessary: I have no analog over-the-air signal so I
>> cannot test it, hence I have removed the tuner element from the patch
>> (below).
>>
>> I have tested DVB-T which works fine. dmesg shows no errors (attached).
>>
>> DISPLAY=xxx:0.0 vlc dvb-t://frequency=49800:bandwidth=8
>> --dvb-adapter=0 --programs=8373
>>
>>
>> /usr/local/bin/v4l2-ctl --set-input 1
>> /usr/local/bin/v4l2-ctl -s 0x00f7
>> cat /dev/video0 > /tmp/svideo.raw
>>
>> ffmpeg -f rawvideo -pix_fmt yuyv422 -r 25 -s:v 720x576 -i
>> /tmp/svideo.raw -vcodec mpeg2video -vb 2000k -y /tmp/svideo.mpg
>>
>>
>> Revised patch:
>>
>> ---
>> drivers/media/pci/cx23885/cx23885-cards.c | 16
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/media/pci/cx23885/cx23885-cards.c
>> b/drivers/media/pci/cx23885/cx23885-cards.c
>> index 0350f13..1b685f0 100644
>> --- a/drivers/media/pci/cx23885/cx23885-cards.c
>> +++ b/drivers/media/pci/cx23885/cx23885-cards.c
>> @@ -196,7 +196,22 @@ struct cx23885_board cx23885_boards[] = {
>> },
>> [CX23885_BOARD_HAUPPAUGE_HVR1200] = {
>> .name = "Hauppauge WinTV-HVR1200",
>> + .porta = CX23885_ANALOG_VIDEO,
>> .portc = CX23885_MPEG_DVB,
>> + .input = {{
>> + .type = CX23885_VMUX_COMPOSITE1,
>> + .vmux = CX25840_VIN7_CH3 |
>> + CX25840_VIN4_CH2 |
>> + CX25840_VIN6_CH1,
>> + .gpio0 = 0,
>> + }, {
>> + .type = CX23885_VMUX_SVIDEO,
>> + .vmux = CX25840_VIN7_CH3 |
>> + CX25840_VIN4_CH2 |
>> + CX25840_VIN8_CH1 |
>> +
Re: [PATCH 0/4] Backported amdgpu ttm deadlock fixes for 4.14
On Thu, Nov 30, 2017 at 07:23:02PM -0500, Lyude Paul wrote:
> I haven't gone to see where it started, but as of late a good number of
> pretty nasty deadlock issues have appeared with the kernel. Easy
> reproduction recipe on a laptop with i915/amdgpu prime with lockdep enabled:
>
> DRI_PRIME=1 glxinfo
>
> Additionally, some more race conditions exist that I've managed to
> trigger with piglit and lockdep enabled after applying these patches:
>
> =
> WARNING: suspicious RCU usage
> 4.14.3Lyude-Test+ #2 Not tainted
> -
> ./include/linux/reservation.h:216 suspicious rcu_dereference_protected()
> usage!
>
> other info that might help us debug this:
>
> rcu_scheduler_active = 2, debug_locks = 1
> 1 lock held by ext_image_dma_b/27451:
> #0: (reservation_ww_class_mutex){+.+.}, at: []
> ttm_bo_unref+0x9f/0x3c0 [ttm]
>
> stack backtrace:
> CPU: 0 PID: 27451 Comm: ext_image_dma_b Not tainted 4.14.3Lyude-Test+ #2
> Hardware name: HP HP ZBook 15 G4/8275, BIOS P70 Ver. 01.02 06/09/2017
> Call Trace:
> dump_stack+0x8e/0xce
> lockdep_rcu_suspicious+0xc5/0x100
> reservation_object_copy_fences+0x292/0x2b0
> ? ttm_bo_unref+0x9f/0x3c0 [ttm]
> ttm_bo_unref+0xbd/0x3c0 [ttm]
> amdgpu_bo_unref+0x2a/0x50 [amdgpu]
> amdgpu_gem_object_free+0x4b/0x50 [amdgpu]
> drm_gem_object_free+0x1f/0x40 [drm]
> drm_gem_object_put_unlocked+0x40/0xb0 [drm]
> drm_gem_object_handle_put_unlocked+0x6c/0xb0 [drm]
> drm_gem_object_release_handle+0x51/0x90 [drm]
> drm_gem_handle_delete+0x5e/0x90 [drm]
> ? drm_gem_handle_create+0x40/0x40 [drm]
> drm_gem_close_ioctl+0x20/0x30 [drm]
> drm_ioctl_kernel+0x5d/0xb0 [drm]
> drm_ioctl+0x2f7/0x3b0 [drm]
> ? drm_gem_handle_create+0x40/0x40 [drm]
> ? trace_hardirqs_on_caller+0xf4/0x190
> ? trace_hardirqs_on+0xd/0x10
> amdgpu_drm_ioctl+0x4f/0x90 [amdgpu]
> do_vfs_ioctl+0x93/0x670
> ? __fget+0x108/0x1f0
> SyS_ioctl+0x79/0x90
> entry_SYSCALL_64_fastpath+0x23/0xc2
>
> I've also added the relevant fixes for the issue mentioned above.
>
> Christian König (3):
> drm/ttm: fix ttm_bo_cleanup_refs_or_queue once more
> dma-buf: make reservation_object_copy_fences rcu save
> drm/amdgpu: reserve root PD while releasing it
>
> Michel Dänzer (1):
> drm/ttm: Always and only destroy bo->ttm_resv in ttm_bo_release_list
All now queued up, thanks.
greg k-h
Re: [PATCH 5/9] v4l: vsp1: Document the vsp1_du_atomic_config structure
Hi Sergei,
On Monday, 4 December 2017 11:31:49 EET Sergei Shtylyov wrote:
> On 12/3/2017 1:57 PM, Laurent Pinchart wrote:
> > The structure is used in the API that the VSP1 driver exposes to the DU
> > driver. Documenting it is thus important.
> >
> > Signed-off-by: Laurent Pinchart
> >
> > ---
> >
> > include/media/vsp1.h | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/include/media/vsp1.h b/include/media/vsp1.h
> > index 68a8abe4fac5..7850f96fb708 100644
> > --- a/include/media/vsp1.h
> > +++ b/include/media/vsp1.h
> > @@ -41,6 +41,16 @@ struct vsp1_du_lif_config {
> > int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
> > const struct vsp1_du_lif_config *cfg);
> >
> > +/**
> > + * struct vsp1_du_atomic_config - VSP atomic configuration parameters
> > + * @pixelformat: plan pixel format (V4L2 4CC)
>
> s/plan/plane/?
Of course, my bad. This will be fixed in v2. Thank you for catching the typo.
--
Regards,
Laurent Pinchart
[PATCH v3] media: vsp1: Prevent suspending and resuming DRM pipelines
When used as part of a display pipeline, the VSP is stopped and
restarted explicitly by the DU from its suspend and resume handlers.
There is thus no need to stop or restart pipelines in the VSP suspend
and resume handlers, and doing so would cause the hardware to be
left in a misconfigured state.
Ensure that the VSP suspend and resume handlers do not affect DRM-based
pipelines.
Signed-off-by: Kieran Bingham
Reviewed-by: Laurent Pinchart
---
v3:
- Fixed up with punctuation from Laurent.
drivers/media/platform/vsp1/vsp1_drv.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
b/drivers/media/platform/vsp1/vsp1_drv.c
index 962e4c304076..eed9516e25e1 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -571,7 +571,13 @@ static int __maybe_unused vsp1_pm_suspend(struct device
*dev)
{
struct vsp1_device *vsp1 = dev_get_drvdata(dev);
- vsp1_pipelines_suspend(vsp1);
+ /*
+* When used as part of a display pipeline, the VSP is stopped and
+* restarted explicitly by the DU.
+*/
+ if (!vsp1->drm)
+ vsp1_pipelines_suspend(vsp1);
+
pm_runtime_force_suspend(vsp1->dev);
return 0;
@@ -582,7 +588,13 @@ static int __maybe_unused vsp1_pm_resume(struct device
*dev)
struct vsp1_device *vsp1 = dev_get_drvdata(dev);
pm_runtime_force_resume(vsp1->dev);
- vsp1_pipelines_resume(vsp1);
+
+ /*
+* When used as part of a display pipeline, the VSP is stopped and
+* restarted explicitly by the DU.
+*/
+ if (!vsp1->drm)
+ vsp1_pipelines_resume(vsp1);
return 0;
}
--
2.7.4
Re: [PATCH] v4l: vsp1: Fix function declaration/definition mismatch
Hello, On Friday, 24 November 2017 20:40:57 EET Kieran Bingham wrote: > Hi Eugeniu, > > Thankyou for the patch, > > Laurent - Any comments on this? Otherwise I'll bundle this in with my > suspend/resume patch for a pull request. > > On 20/08/17 13:40, Eugeniu Rosca wrote: > > From: Eugeniu Rosca > > > > Cppcheck v1.81 complains that the parameter names of certain vsp1 > > functions don't match between declaration and definition. Fix this. > > No functional change is confirmed by the empty delta between the > > disassembled object files before and after the change. > > > > Signed-off-by: Eugeniu Rosca > > Reviewed-by: Kieran Bingham > > > --- > > > > drivers/media/platform/vsp1/vsp1_entity.h | 5 +++-- > > drivers/media/platform/vsp1/vsp1_hgo.h| 2 +- > > drivers/media/platform/vsp1/vsp1_hgt.h| 2 +- > > drivers/media/platform/vsp1/vsp1_uds.h| 2 +- > > 4 files changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/media/platform/vsp1/vsp1_entity.h > > b/drivers/media/platform/vsp1/vsp1_entity.h index > > c169a060b6d2..1087d7e5c126 100644 > > --- a/drivers/media/platform/vsp1/vsp1_entity.h > > +++ b/drivers/media/platform/vsp1/vsp1_entity.h > > @@ -161,7 +161,8 @@ int vsp1_subdev_enum_mbus_code(struct v4l2_subdev > > *subdev, > > int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev, > > struct v4l2_subdev_pad_config *cfg, > > struct v4l2_subdev_frame_size_enum *fse, > > - unsigned int min_w, unsigned int min_h, > > - unsigned int max_w, unsigned int max_h); > > + unsigned int min_width, unsigned int min_height, > > + unsigned int max_width, > > + unsigned int max_height); > > This is fine. > > > #endif /* __VSP1_ENTITY_H__ */ > > > > diff --git a/drivers/media/platform/vsp1/vsp1_hgo.h > > b/drivers/media/platform/vsp1/vsp1_hgo.h index c6c0b7a80e0c..76a9cf97b9d3 > > 100644 > > --- a/drivers/media/platform/vsp1/vsp1_hgo.h > > +++ b/drivers/media/platform/vsp1/vsp1_hgo.h > > @@ -40,6 +40,6 @@ static inline struct vsp1_hgo *to_hgo(struct v4l2_subdev > > *subdev)> > > } > > > > struct vsp1_hgo *vsp1_hgo_create(struct vsp1_device *vsp1); > > > > -void vsp1_hgo_frame_end(struct vsp1_entity *hgo); > > +void vsp1_hgo_frame_end(struct vsp1_entity *entity); > > I was going to say: We know the object is an entity by it's type. Isn't hgo > more descriptive for it's name ? > > However to answer my own question - The implementation function goes on to > define a struct vsp1_hgo *hgo, so no ... the Entity object shouldn't be hgo. And that's exactly why there's a difference between the declaration and implementation :-) Naming the parameter hgo in the declaration makes it clear to the reader what entity is expected. The parameter is then named entity in the function definition to allow for the vsp1_hgo *hgo local variable. Is the mismatch a real issue ? I don't think the kernel coding style mandates parameter names to be identical between function declaration and definition. > So this looks reasonable to me, and the same logic applies to the other > instances. > > #endif /* __VSP1_HGO_H__ */ > > > > diff --git a/drivers/media/platform/vsp1/vsp1_hgt.h > > b/drivers/media/platform/vsp1/vsp1_hgt.h index 83f2e130942a..37139d54b6c8 > > 100644 > > --- a/drivers/media/platform/vsp1/vsp1_hgt.h > > +++ b/drivers/media/platform/vsp1/vsp1_hgt.h > > @@ -37,6 +37,6 @@ static inline struct vsp1_hgt *to_hgt(struct v4l2_subdev > > *subdev)> > > } > > > > struct vsp1_hgt *vsp1_hgt_create(struct vsp1_device *vsp1); > > > > -void vsp1_hgt_frame_end(struct vsp1_entity *hgt); > > +void vsp1_hgt_frame_end(struct vsp1_entity *entity); > > > > #endif /* __VSP1_HGT_H__ */ > > > > diff --git a/drivers/media/platform/vsp1/vsp1_uds.h > > b/drivers/media/platform/vsp1/vsp1_uds.h index 7bf3cdcffc65..9c7f8497b5da > > 100644 > > --- a/drivers/media/platform/vsp1/vsp1_uds.h > > +++ b/drivers/media/platform/vsp1/vsp1_uds.h > > @@ -35,7 +35,7 @@ static inline struct vsp1_uds *to_uds(struct v4l2_subdev > > *subdev) > > struct vsp1_uds *vsp1_uds_create(struct vsp1_device *vsp1, unsigned int > > index); > > -void vsp1_uds_set_alpha(struct vsp1_entity *uds, struct vsp1_dl_list *dl, > > +void vsp1_uds_set_alpha(struct vsp1_entity *entity, struct vsp1_dl_list > > *dl, > > unsigned int alpha); > > > > #endif /* __VSP1_UDS_H__ */ -- Regards, Laurent Pinchart
Re: [GIT PULL] SAA716x DVB driver
Em Sun, 3 Dec 2017 15:04:31 + Jemma Denson escreveu: > On 03/12/17 14:11, Soeren Moch wrote: > > On 03.12.2017 11:57, Jemma Denson wrote: > >> On 02/12/17 23:59, Soeren Moch wrote: > > The whole purpose of driver development is bringing support for existing > > hardware to available user applications, preferably with existing APIs. > > And exactly that is in this pull request. > > In the whole discussion I cannot find a single reason, how merging this > > driver would violate the linux development principles. > > That's not really one for me to answer, but Mauro has raised objections > so it can't be merged as is. I'm just trying to find a way forward that > avoids this getting stuck for another few years. Let me write a longer explanation about what are my concerns. The current FF API was designed back in 1999, almost 20 years ago. It was a pioneer design on that time, but it shows its age. I had several discussions in the past with some chipset manufacturers and with people interested on open sourcing their drivers for embedded devices. The point is: the current way it is implemented doesn't fit, for several reasons: - The API itself is not flexible enough to support their hardware, with usually has a different number of frontends than the number of decoders and all sorts of other complexities. On modern chipsets, the same pipeline is used also for other things like hdmi input/output and even webcams/microphones. So, an API that integrates with such components are needed. Also, on modern hardware, the descrambler hardware can dynamically be added/removed from the pipeline, and they usually have a pair of scrambler/descrambler module (using a different crypto algorithm), used when the data is written on a hard disk; - The documentation for the API is incomplete; - The API is broken with 64 bits kernel and 32 bits userspace. Most SoCs nowadays use 64 bits Kernel, but lots of vendors are still using 32 bits userspace; - The DVB core doesn't support the FF API. Everything should be written at the drivers. Having an API for FF that doesn't fit on modern hardware has a practical effect of preventing people to write it on a proper way, as APIs on Linux are forever (in the sense that, while the API is still in usage by non-OOT/staging drivers, it can't be removed). The only driver that implements such API upstream isfor a hardware developed in 1999, whose production ended ages ago. So, the status of this API, from Kernel's PoV, is that it could be removed anytime if needed, by simply removing one obsolete driver. Removing obsolete drivers that are preventing adding new functionalities is allowed. So, it is time to replace it for good, implementing something that works for modern hardware, using APIs that fits on embedded systems needs (as most FF implementation are for STB and TV sets). In other words, I'm simply against adding a new driver that will come with a bold maintenance requirement of keeping alive an obsolete FF API that doesn't fit on modern chipset needs, and would prevent adding a new FF API for another 20 years that would otherwise work with modern hardware. It is time to discuss about a new FF API and implement drivers supporting it. In the specific case of saa716x driver, users seemed to be happy on having it OOT, as it has been working like that for a long time without any attempts to merge it. As new hardware using this chipset aren't manufactured anymore (AFAIKT). So, a trivial alternative would be to keep if OOT. So, the only way I would agree on merging the saa716x driver upstream (specially support for FF) is if we come up with a solution that wouldn't force us to stick with an FF API that doesn't support modern hardware for a long time. As capping saa716x FF support doesn't make much sense, we should migrate it to a new FF API, long term. - From what I understood, Soeren's original proposal at his initial pull request were to add it at staging, keeping it there forever. This is against staging policy. Staging is not a place to keep drivers forever, but a place to add drivers that require work. A driver at staging that isn't maintained (in the sense of solving issues that will allow it to be moved out of staging) can be removed anytime. So, this is not an option. As a summary from this thread's discussion, my understanding is that there are currently two alternatives about a way to move forward: 1) add saa716x driver at staging, with developers committed to solve whatever issues the driver has. On other words, we should have a plan to migrate it to an API capable of attending current needs. Both developers and users should be aware that, if such change never happen, the driver may be removed anytime in the future. 2) add first the parts that won't require a new API, and having developers working on addressing the API issues for FF. Once this is addressed, merge the FF support. While this doesn't happen, the FF pa
Re: [PATCH v8 23/28] rcar-vin: parse Gen3 OF and setup media graph
On 11/29/2017 08:43 PM, Niklas Söderlund wrote:
> Parse the VIN Gen3 OF graph and register all CSI-2 devices in the VIN
> group common media device. Once all CSI-2 subdevices are added to the
> common media device create links between them.
>
> The parsing and registering CSI-2 subdevices with the v4l2 async
> framework is a collaborative effort shared between the VIN instances
> which are part of the group. The first rcar-vin instance parses OF and
> finds all other VIN and CSI-2 nodes which are part of the graph. It
> stores a bit mask of all VIN instances found and handles to all CSI-2
> nodes.
>
> The bit mask is used to figure out when all VIN instances have been
> probed. Once the last VIN instance is probed this is detected and this
> instance registers all CSI-2 subdevices in its private async notifier.
> Once the .complete() callback of this notifier is called it register all
register -> registers
> video devices and creates the media controller links between all
> entities.
>
> Signed-off-by: Niklas Söderlund
With that small change and the fix for the issue you found in rvin_group_init():
Reviewed-by: Hans Verkuil
Regards,
Hans
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 322
> +++-
> drivers/media/platform/rcar-vin/rcar-vin.h | 10 +-
> 2 files changed, 327 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index a6713fd61dd87a88..2081637e493e1941 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -86,6 +86,7 @@ static struct rvin_group *__rvin_group_allocate(struct
> rvin_dev *vin)
> return NULL;
>
> kref_init(&group->refcount);
> + group->notifier = NULL;
> rvin_group_data = group;
>
> vin_dbg(vin, "%s: alloc group=%p\n", __func__, group);
> @@ -392,10 +393,281 @@ static int rvin_digital_graph_init(struct rvin_dev
> *vin)
> * Group async notifier
> */
>
> -static int rvin_group_init(struct rvin_dev *vin)
> +/* group lock should be held when calling this function */
> +static int rvin_group_add_link(struct rvin_dev *vin,
> +struct media_entity *source,
> +unsigned int source_idx,
> +struct media_entity *sink,
> +unsigned int sink_idx,
> +u32 flags)
> +{
> + struct media_pad *source_pad, *sink_pad;
> + int ret = 0;
> +
> + source_pad = &source->pads[source_idx];
> + sink_pad = &sink->pads[sink_idx];
> +
> + if (!media_entity_find_link(source_pad, sink_pad))
> + ret = media_create_pad_link(source, source_idx,
> + sink, sink_idx, flags);
> +
> + if (ret)
> + vin_err(vin, "Error adding link from %s to %s\n",
> + source->name, sink->name);
> +
> + return ret;
> +}
> +
> +static int rvin_group_update_links(struct rvin_dev *vin)
> +{
> + struct media_entity *source, *sink;
> + struct rvin_dev *master;
> + unsigned int i, n, idx, csi;
> + int ret = 0;
> +
> + mutex_lock(&vin->group->lock);
> +
> + for (n = 0; n < RCAR_VIN_NUM; n++) {
> +
> + /* Check that VIN is part of the group */
> + if (!vin->group->vin[n])
> + continue;
> +
> + /* Check that subgroup master is part of the group */
> + master = vin->group->vin[n < 4 ? 0 : 4];
> + if (!master)
> + continue;
> +
> + for (i = 0; i < vin->info->num_chsels; i++) {
> + csi = vin->info->chsels[n][i].csi;
> +
> + /* If the CSI-2 is out of bounds it's a noop, skip */
> + if (csi >= RVIN_CSI_MAX)
> + continue;
> +
> + /* Check that CSI-2 are part of the group */
> + if (!vin->group->csi[csi].subdev)
> + continue;
> +
> + source = &vin->group->csi[csi].subdev->entity;
> + sink = &vin->group->vin[n]->vdev.entity;
> + idx = vin->info->chsels[n][i].chan + 1;
> +
> + ret = rvin_group_add_link(vin, source, idx, sink, 0,
> + 0);
> + if (ret)
> + goto out;
> + }
> + }
> +out:
> + mutex_unlock(&vin->group->lock);
> +
> + return ret;
> +}
> +
> +static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
> {
> + struct rvin_dev *vin = notifier_to_vin(notifier);
> + unsigned int i;
> int ret;
>
> + ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
> + if (ret) {
> + vin_err(vin, "Failed to register subdev nodes\n");
> +
Re: [PATCH v8 28/28] rcar-vin: enable support for r8a77970
On 11/29/2017 08:43 PM, Niklas Söderlund wrote:
> Add the SoC specific information for Renesas r8a77970.
>
> Signed-off-by: Niklas Söderlund
Reviewed-by: Hans Verkuil
Regards,
Hans
> ---
> .../devicetree/bindings/media/rcar_vin.txt | 1 +
> drivers/media/platform/rcar-vin/rcar-core.c| 40
> ++
> 2 files changed, 41 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
> b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 314743532bbb4523..6b98f8a3398fa493 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -21,6 +21,7 @@ on Gen3 to a CSI-2 receiver.
> - "renesas,vin-r8a7794" for the R8A7794 device
> - "renesas,vin-r8a7795" for the R8A7795 device
> - "renesas,vin-r8a7796" for the R8A7796 device
> + - "renesas,vin-r8a77970" for the R8A77970 device
> - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
> device.
> - "renesas,rcar-gen3-vin" for a generic R-Car Gen3 compatible device.
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index 62eb89b36fbb2ee1..bbdf36b5c3c8178d 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -1145,6 +1145,42 @@ static const struct rvin_info rcar_info_r8a7796 = {
> },
> };
>
> +static const struct rvin_info rcar_info_r8a77970 = {
> + .chip = RCAR_GEN3,
> + .use_mc = true,
> + .max_width = 4096,
> + .max_height = 4096,
> +
> + .num_chsels = 5,
> + .chsels = {
> + {
> + { .csi = RVIN_CSI40, .chan = 0 },
> + { .csi = RVIN_NC, .chan = 0 },
> + { .csi = RVIN_NC, .chan = 0 },
> + { .csi = RVIN_CSI40, .chan = 0 },
> + { .csi = RVIN_NC, .chan = 0 },
> + }, {
> + { .csi = RVIN_NC, .chan = 0 },
> + { .csi = RVIN_NC, .chan = 0 },
> + { .csi = RVIN_CSI40, .chan = 0 },
> + { .csi = RVIN_CSI40, .chan = 1 },
> + { .csi = RVIN_NC, .chan = 0 },
> + }, {
> + { .csi = RVIN_NC, .chan = 0 },
> + { .csi = RVIN_CSI40, .chan = 0 },
> + { .csi = RVIN_NC, .chan = 0 },
> + { .csi = RVIN_CSI40, .chan = 2 },
> + { .csi = RVIN_NC, .chan = 0 },
> + }, {
> + { .csi = RVIN_CSI40, .chan = 1 },
> + { .csi = RVIN_NC, .chan = 0 },
> + { .csi = RVIN_NC, .chan = 0 },
> + { .csi = RVIN_CSI40, .chan = 3 },
> + { .csi = RVIN_NC, .chan = 0 },
> + },
> + },
> +};
> +
> static const struct of_device_id rvin_of_id_table[] = {
> {
> .compatible = "renesas,vin-r8a7778",
> @@ -1182,6 +1218,10 @@ static const struct of_device_id rvin_of_id_table[] = {
> .compatible = "renesas,vin-r8a7796",
> .data = &rcar_info_r8a7796,
> },
> + {
> + .compatible = "renesas,vin-r8a77970",
> + .data = &rcar_info_r8a77970,
> + },
> { },
> };
> MODULE_DEVICE_TABLE(of, rvin_of_id_table);
>
Re: [PATCH v8 04/28] rcar-vin: move subdevice handling to async callbacks
On 11/29/2017 08:43 PM, Niklas Söderlund wrote:
> In preparation for Gen3 support move the subdevice initialization and
> clean up from rvin_v4l2_{register,unregister}() directly to the async
> callbacks. This simplifies the addition of Gen3 support as the
> rvin_v4l2_register() can be shared for both Gen2 and Gen3 while direct
> subdevice control are only used on Gen2.
>
> While moving this code drop a large comment which is copied from the
> framework documentation and fold rvin_mbus_supported() into its only
> caller.
>
> Signed-off-by: Niklas Söderlund
Reviewed-by: Hans Verkuil
Regards,
Hans
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 105
> ++--
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 35 --
> 2 files changed, 67 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index 6d99542ec74b49a7..6ab51acd676641ec 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -46,47 +46,11 @@ static int rvin_find_pad(struct v4l2_subdev *sd, int
> direction)
> return -EINVAL;
> }
>
> -static bool rvin_mbus_supported(struct rvin_graph_entity *entity)
> -{
> - struct v4l2_subdev *sd = entity->subdev;
> - struct v4l2_subdev_mbus_code_enum code = {
> - .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> - };
> -
> - code.index = 0;
> - code.pad = entity->source_pad;
> - while (!v4l2_subdev_call(sd, pad, enum_mbus_code, NULL, &code)) {
> - code.index++;
> - switch (code.code) {
> - case MEDIA_BUS_FMT_YUYV8_1X16:
> - case MEDIA_BUS_FMT_UYVY8_2X8:
> - case MEDIA_BUS_FMT_UYVY10_2X10:
> - case MEDIA_BUS_FMT_RGB888_1X24:
> - entity->code = code.code;
> - return true;
> - default:
> - break;
> - }
> - }
> -
> - return false;
> -}
> -
> static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier)
> {
> struct rvin_dev *vin = notifier_to_vin(notifier);
> int ret;
>
> - /* Verify subdevices mbus format */
> - if (!rvin_mbus_supported(vin->digital)) {
> - vin_err(vin, "Unsupported media bus format for %s\n",
> - vin->digital->subdev->name);
> - return -EINVAL;
> - }
> -
> - vin_dbg(vin, "Found media bus format for %s: %d\n",
> - vin->digital->subdev->name, vin->digital->code);
> -
> ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
> if (ret < 0) {
> vin_err(vin, "Failed to register subdev nodes\n");
> @@ -103,8 +67,16 @@ static void rvin_digital_notify_unbind(struct
> v4l2_async_notifier *notifier,
> struct rvin_dev *vin = notifier_to_vin(notifier);
>
> vin_dbg(vin, "unbind digital subdev %s\n", subdev->name);
> +
> + mutex_lock(&vin->lock);
> +
> rvin_v4l2_unregister(vin);
> + v4l2_ctrl_handler_free(&vin->ctrl_handler);
> +
> + vin->vdev.ctrl_handler = NULL;
> vin->digital->subdev = NULL;
> +
> + mutex_unlock(&vin->lock);
> }
>
> static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
> @@ -112,12 +84,14 @@ static int rvin_digital_notify_bound(struct
> v4l2_async_notifier *notifier,
>struct v4l2_async_subdev *asd)
> {
> struct rvin_dev *vin = notifier_to_vin(notifier);
> + struct v4l2_subdev_mbus_code_enum code = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> int ret;
>
> v4l2_set_subdev_hostdata(subdev, vin);
>
> /* Find source and sink pad of remote subdevice */
> -
> ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
> if (ret < 0)
> return ret;
> @@ -126,21 +100,74 @@ static int rvin_digital_notify_bound(struct
> v4l2_async_notifier *notifier,
> ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
> vin->digital->sink_pad = ret < 0 ? 0 : ret;
>
> + /* Find compatible subdevices mbus format */
> + vin->digital->code = 0;
> + code.index = 0;
> + code.pad = vin->digital->source_pad;
> + while (!vin->digital->code &&
> +!v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, &code)) {
> + code.index++;
> + switch (code.code) {
> + case MEDIA_BUS_FMT_YUYV8_1X16:
> + case MEDIA_BUS_FMT_UYVY8_2X8:
> + case MEDIA_BUS_FMT_UYVY10_2X10:
> + case MEDIA_BUS_FMT_RGB888_1X24:
> + vin->digital->code = code.code;
> + vin_dbg(vin, "Found media bus format for %s: %d\n",
> + subdev->name, vin->digital->code);
> + break;
> + default:
> + break;
> + }
> + }
> +
> + if (!vin->digita
Re: [PATCH v8 03/28] rcar-vin: unregister video device on driver removal
On 11/29/2017 08:43 PM, Niklas Söderlund wrote:
> If the video device was registered by the complete() callback it should
> be unregistered when the driver is removed. Protect from printing a
> uninitialized video device node name by adding a checking in
> rvin_v4l2_unregister() by checking that the video device is registered.
>
> Signed-off-by: Niklas Söderlund
Reviewed-by: Hans Verkuil
Regards,
Hans
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 2 ++
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 3 +++
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index f7a4c21909da6923..6d99542ec74b49a7 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -272,6 +272,8 @@ static int rcar_vin_remove(struct platform_device *pdev)
>
> pm_runtime_disable(&pdev->dev);
>
> + rvin_v4l2_unregister(vin);
> +
> v4l2_async_notifier_unregister(&vin->notifier);
> v4l2_async_notifier_cleanup(&vin->notifier);
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 178aecc94962abe2..32a658214f48fa49 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -841,6 +841,9 @@ static const struct v4l2_file_operations rvin_fops = {
>
> void rvin_v4l2_unregister(struct rvin_dev *vin)
> {
> + if (!video_is_registered(&vin->vdev))
> + return;
> +
> v4l2_info(&vin->v4l2_dev, "Removing %s\n",
> video_device_node_name(&vin->vdev));
>
>
Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Hi Maxime, I just noticed that you are using the second iteration? Have you received my third iteration? On Sat, 25 Nov 2017 17:02:33 +0100 Maxime Ripard wrote: > On Thu, Nov 23, 2017 at 09:14:44AM +0800, Yong wrote: > > > On Wed, Nov 22, 2017 at 09:33:06AM +0800, Yong wrote: > > > > > On Thu, Jul 27, 2017 at 01:01:35PM +0800, Yong Deng wrote: > > > > > > Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI > > > > > > interface > > > > > > and CSI1 is used for parallel interface. This is not documented in > > > > > > datasheet but by testing 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 > > > > > > > > > > Thanks again for this driver. > > > > > > > > > > It seems like at least this iteration is behaving in a weird way with > > > > > DMA transfers for at least YU12 and NV12 (and I would assume YV12). > > > > > > > > > > Starting a transfer of multiple frames in either of these formats, > > > > > using either ffmpeg (ffmpeg -f v4l2 -video_size 640x480 -framerate 30 > > > > > -i /dev/video0 output.mkv) or yavta (yavta -c80 -p -F --skip 0 -f NV12 > > > > > -s 640x480 $(media-c tl -e 'sun6i-csi')) will end up in a panic. > > > > > > > > > > The panic seems to be generated with random data going into parts of > > > > > the kernel memory, the pattern being in my case something like > > > > > 0x8287868a which is very odd (always around 0x88) > > > > > > > > > > It turns out that when you cover the sensor, the values change to > > > > > around 0x28, so it really seems like it's pixels that have been copied > > > > > there. > > > > > > > > > > I've looked quickly at the DMA setup, and it seems reasonable to > > > > > me. Do you have the same issue on your side? Have you been able to > > > > > test those formats using your hardware? > > > > > > > > I had tested the following formats with BT1120 input: > > > > V4L2_PIX_FMT_NV12 -> NV12 > > > > V4L2_PIX_FMT_NV21 -> NV21 > > > > V4L2_PIX_FMT_NV16 -> NV16 > > > > V4L2_PIX_FMT_NV61 -> NV61 > > > > V4L2_PIX_FMT_YUV420 -> YU12 > > > > V4L2_PIX_FMT_YVU420 -> YV12 > > > > V4L2_PIX_FMT_YUV422P-> 422P > > > > And they all work fine. > > > > > > Ok, that's good to know. > > > > > > > > Given that they all are planar formats and YUYV and the likes work > > > > > just fine, maybe we can leave them aside for now? > > > > > > > > V4L2_PIX_FMT_YUV422P and V4L2_PIX_FMT_YUYV is OK, and V4L2_PIX_FMT_NV12 > > > > is bad? It's really weird. > > > > > > > > What's your input bus code format, type and width? > > > > > > The sensor is an ov5640, so the MBUS code for the bus is > > > MEDIA_BUS_FMT_YUYV8_2X8. > > > > Did you test on V3s? > > No, this is on an H3, but that would be the first difference so far. > > > I haven't tested it with MEDIA_BUS_FMT_YUYV8_2X8. > > Ok, it's good to know that at least it works on your end, it's useful > for us to debug things :) > > > The Allwinner CSI's DMA is definitely weird. Ondřej Jirman thought > > that CSI has an internal queue (Ondřej's commit has explained in detail). > > I think CSI just pick up the buffer address before the frame done > > interrupt triggered. > > The patch in attachment can deal with this. You can see if it is > > useful to solve your problem. > > I'll test that on monday, thanks! > > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com Thanks, Yong
Re: [PATCH v8 02/28] rcar-vin: rename poorly named initialize and cleanup functions
On 11/29/2017 08:43 PM, Niklas Söderlund wrote:
> The functions to initialize and cleanup the hardware and video device
> where poorly named from the start. Rename them to better describe there
> intended function.
>
> Signed-off-by: Niklas Söderlund
Reviewed-by: Hans Verkuil
Much better, indeed.
Regards,
Hans
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 10 +-
> drivers/media/platform/rcar-vin/rcar-dma.c | 6 +++---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 4 ++--
> drivers/media/platform/rcar-vin/rcar-vin.h | 8
> 4 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index 108d776f32651b27..f7a4c21909da6923 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -93,7 +93,7 @@ static int rvin_digital_notify_complete(struct
> v4l2_async_notifier *notifier)
> return ret;
> }
>
> - return rvin_v4l2_probe(vin);
> + return rvin_v4l2_register(vin);
> }
>
> static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier,
> @@ -103,7 +103,7 @@ static void rvin_digital_notify_unbind(struct
> v4l2_async_notifier *notifier,
> struct rvin_dev *vin = notifier_to_vin(notifier);
>
> vin_dbg(vin, "unbind digital subdev %s\n", subdev->name);
> - rvin_v4l2_remove(vin);
> + rvin_v4l2_unregister(vin);
> vin->digital->subdev = NULL;
> }
>
> @@ -245,7 +245,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
> if (irq < 0)
> return irq;
>
> - ret = rvin_dma_probe(vin, irq);
> + ret = rvin_dma_register(vin, irq);
> if (ret)
> return ret;
>
> @@ -260,7 +260,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
>
> return 0;
> error:
> - rvin_dma_remove(vin);
> + rvin_dma_unregister(vin);
> v4l2_async_notifier_cleanup(&vin->notifier);
>
> return ret;
> @@ -275,7 +275,7 @@ static int rcar_vin_remove(struct platform_device *pdev)
> v4l2_async_notifier_unregister(&vin->notifier);
> v4l2_async_notifier_cleanup(&vin->notifier);
>
> - rvin_dma_remove(vin);
> + rvin_dma_unregister(vin);
>
> return 0;
> }
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c
> index 23fdff7a7370842e..d701b52d198243b5 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1153,14 +1153,14 @@ static const struct vb2_ops rvin_qops = {
> .wait_finish= vb2_ops_wait_finish,
> };
>
> -void rvin_dma_remove(struct rvin_dev *vin)
> +void rvin_dma_unregister(struct rvin_dev *vin)
> {
> mutex_destroy(&vin->lock);
>
> v4l2_device_unregister(&vin->v4l2_dev);
> }
>
> -int rvin_dma_probe(struct rvin_dev *vin, int irq)
> +int rvin_dma_register(struct rvin_dev *vin, int irq)
> {
> struct vb2_queue *q = &vin->queue;
> int i, ret;
> @@ -1208,7 +1208,7 @@ int rvin_dma_probe(struct rvin_dev *vin, int irq)
>
> return 0;
> error:
> - rvin_dma_remove(vin);
> + rvin_dma_unregister(vin);
>
> return ret;
> }
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index b479b882da12f62d..178aecc94962abe2 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -839,7 +839,7 @@ static const struct v4l2_file_operations rvin_fops = {
> .read = vb2_fop_read,
> };
>
> -void rvin_v4l2_remove(struct rvin_dev *vin)
> +void rvin_v4l2_unregister(struct rvin_dev *vin)
> {
> v4l2_info(&vin->v4l2_dev, "Removing %s\n",
> video_device_node_name(&vin->vdev));
> @@ -866,7 +866,7 @@ static void rvin_notify(struct v4l2_subdev *sd,
> }
> }
>
> -int rvin_v4l2_probe(struct rvin_dev *vin)
> +int rvin_v4l2_register(struct rvin_dev *vin)
> {
> struct video_device *vdev = &vin->vdev;
> struct v4l2_subdev *sd = vin_to_source(vin);
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> b/drivers/media/platform/rcar-vin/rcar-vin.h
> index 5382078143fb3869..85cb7ec53d2b08b5 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -153,11 +153,11 @@ struct rvin_dev {
> #define vin_warn(d, fmt, arg...) dev_warn(d->dev, fmt, ##arg)
> #define vin_err(d, fmt, arg...) dev_err(d->dev, fmt, ##arg)
>
> -int rvin_dma_probe(struct rvin_dev *vin, int irq);
> -void rvin_dma_remove(struct rvin_dev *vin);
> +int rvin_dma_register(struct rvin_dev *vin, int irq);
> +void rvin_dma_unregister(struct rvin_dev *vin);
>
> -int rvin_v4l2_probe(struct rvin_dev *vin);
> -void rvin_v4l2_remove(struct rvin_dev *vin);
> +int rvin_v4l2_register(struct rvin_dev *vin);
> +
Re: [PATCH 5/9] v4l: vsp1: Document the vsp1_du_atomic_config structure
Hello!
On 12/3/2017 1:57 PM, Laurent Pinchart wrote:
The structure is used in the API that the VSP1 driver exposes to the DU
driver. Documenting it is thus important.
Signed-off-by: Laurent Pinchart
---
include/media/vsp1.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/include/media/vsp1.h b/include/media/vsp1.h
index 68a8abe4fac5..7850f96fb708 100644
--- a/include/media/vsp1.h
+++ b/include/media/vsp1.h
@@ -41,6 +41,16 @@ struct vsp1_du_lif_config {
int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
const struct vsp1_du_lif_config *cfg);
+/**
+ * struct vsp1_du_atomic_config - VSP atomic configuration parameters
+ * @pixelformat: plan pixel format (V4L2 4CC)
s/plan/plane/?
[...]
MBR, Sergei
Re: [PATCH] v4l: rcar-vin: Implement V4L2 video node release handler
Hi Niklas, On 11/16/2017 01:27 AM, Laurent Pinchart wrote: > Hi Niklas, > > On Thursday, 16 November 2017 01:34:33 EET Niklas Söderlund wrote: >> On 2017-11-16 00:49:07 +0200, Laurent Pinchart wrote: >>> The rvin_dev data structure contains driver private data for an instance >>> of the VIN. It is allocated dynamically at probe time, and must be freed >>> once all users are gone. >>> >>> The structure is currently allocated with devm_kzalloc(), which results >>> in memory being freed when the device is unbound. If a userspace >>> application is currently performing an ioctl call, or just keeps the >>> device node open and closes it later, this will lead to accessing freed >>> memory. >>> >>> Fix the problem by implementing a V4L2 release handler for the video >>> node associated with the VIN instance (called when the last user of the >>> video node releases it), and freeing memory explicitly from the release >>> handler. >>> >>> Signed-off-by: Laurent Pinchart >>> >> >> Acked-by: Niklas Söderlund >> >> This patch is based on-top of the VIN Gen3 enablement series not yet >> upstream. This is OK for me, just wanted to check that this was the >> intention as to minimize conflicts between the two. > > Yes, that's my intention. The patch should be included, or possibly squashed > in, your development branch. > Has this patch been added in your v8 series? If not, can you add it when you post a v9? Thanks, Hans
[PATCH] cec-adap: add '0x' prefix when printing status
It's not clear if the transmit status that is printed when debugging is enabled is hex or decimal. Add 0x prefix to make this explicit. Signed-off-by: Hans Verkuil --- drivers/media/cec/cec-adap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c index 9219dc96d575..2a097e016414 100644 --- a/drivers/media/cec/cec-adap.c +++ b/drivers/media/cec/cec-adap.c @@ -540,7 +540,7 @@ void cec_transmit_done_ts(struct cec_adapter *adap, u8 status, unsigned int attempts_made = arb_lost_cnt + nack_cnt + low_drive_cnt + error_cnt; - dprintk(2, "%s: status %02x\n", __func__, status); + dprintk(2, "%s: status 0x%02x\n", __func__, status); if (attempts_made < 1) attempts_made = 1; -- 2.14.1
