Re: [PATCH 3/4] gsc-m2m: report correct format bytesperline and sizeimage
Hi John Sheu,
Thanks for the patch.
Please find the review comments inline.
On Wed, Mar 12, 2014 at 4:22 AM, John Sheu wrote:
> Explicitly specify sampling period for subsampled chroma formats, so
> stride and image size are properly reported through VIDIOC_{S,G}_FMT.
>
> Signed-off-by: John Sheu
> ---
> drivers/media/platform/exynos-gsc/gsc-core.c | 154
> +++
> drivers/media/platform/exynos-gsc/gsc-core.h | 16 +--
> drivers/media/platform/exynos-gsc/gsc-regs.c | 40 +++
> drivers/media/platform/exynos-gsc/gsc-regs.h | 4 +-
> 4 files changed, 116 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c
> b/drivers/media/platform/exynos-gsc/gsc-core.c
> index 9d0cc04d..c02addef 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
> @@ -34,167 +34,185 @@ static const struct gsc_fmt gsc_formats[] = {
> {
> .name = "RGB565",
> .pixelformat= V4L2_PIX_FMT_RGB565X,
> - .depth = { 16 },
> .color = GSC_RGB,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 16 },
> + .sampling = { { 1, 1 } },
> }, {
> .name = "XRGB-8-8-8-8, 32 bpp",
> .pixelformat= V4L2_PIX_FMT_RGB32,
> - .depth = { 32 },
> .color = GSC_RGB,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 32 },
> + .sampling = { { 1, 1 } },
> }, {
> .name = "YUV 4:2:2 packed, YCbYCr",
> .pixelformat= V4L2_PIX_FMT_YUYV,
> - .depth = { 16 },
> .color = GSC_YUV422,
> .yorder = GSC_LSB_Y,
> .corder = GSC_CBCR,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 16 },
> + .sampling = { { 1, 1 } },
> .mbus_code = V4L2_MBUS_FMT_YUYV8_2X8,
> }, {
> .name = "YUV 4:2:2 packed, CbYCrY",
> .pixelformat= V4L2_PIX_FMT_UYVY,
> - .depth = { 16 },
> .color = GSC_YUV422,
> .yorder = GSC_LSB_C,
> .corder = GSC_CBCR,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 16 },
> + .sampling = { { 1, 1 } },
> .mbus_code = V4L2_MBUS_FMT_UYVY8_2X8,
> }, {
> .name = "YUV 4:2:2 packed, CrYCbY",
> .pixelformat= V4L2_PIX_FMT_VYUY,
> - .depth = { 16 },
> .color = GSC_YUV422,
> .yorder = GSC_LSB_C,
> .corder = GSC_CRCB,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 16 },
> + .sampling = { { 1, 1 } },
> .mbus_code = V4L2_MBUS_FMT_VYUY8_2X8,
> }, {
> .name = "YUV 4:2:2 packed, YCrYCb",
> .pixelformat= V4L2_PIX_FMT_YVYU,
> - .depth = { 16 },
> .color = GSC_YUV422,
> .yorder = GSC_LSB_Y,
> .corder = GSC_CRCB,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 16 },
> + .sampling = { { 1, 1 } },
> .mbus_code = V4L2_MBUS_FMT_YVYU8_2X8,
> }, {
> .name = "YUV 4:4:4 planar, YCbYCr",
> .pixelformat= V4L2_PIX_FMT_YUV32,
> - .depth = { 32 },
> .color = GSC_YUV444,
> .yorder = GSC_LSB_Y,
> .corder = GSC_CBCR,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 32 },
> + .sampling = { { 1, 1 } },
> }, {
> .name = "YUV 4:2:2 planar, Y/Cb/Cr",
> .pixelformat= V4L2_PIX_FMT_YUV422P,
> - .depth = { 16 },
> .color = GSC_YUV422,
> .yorder = GSC_LSB_Y,
> .corder = GSC_CBCR,
> .num_planes = 1,
> .num_comp = 3,
> + .depth = { 8, 8, 8 },
> + .sampling = { { 1, 1 }, { 2, 1 }, { 2, 1 } },
> }, {
> .name = "YUV 4:2:2 p
Re: [PATCH v12][ 07/12] drm: drm_display_mode: add signal polarity flags
Hi Denis,
On 04/07/2014 02:44 PM, Denis Carikli wrote:
> We need a way to pass signal polarity informations
> between DRM panels, and the display drivers.
>
> To do that, a pol_flags field was added to drm_display_mode.
>
> Signed-off-by: Denis Carikli
> ---
> ChangeLog v11->v12:
> - Rebased: This patch now applies against drm_modes.h
> - Rebased: It now uses the new DRM_MODE_FLAG_POL_DE flags defines names
>
> ChangeLog v10->v11:
> - Since the imx-drm won't be able to retrive its regulators
> from the device tree when using display-timings nodes,
> and that I was told that the drm simple-panel driver
> already supported that, I then, instead, added what was
> lacking to make the eukrea displays work with the
> drm-simple-panel driver.
>
> That required a way to get back the display polarity
> informations from the imx-drm driver without affecting
> userspace.
> ---
> include/drm/drm_modes.h |8
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 2dbbf99..b3789e2 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -93,6 +93,13 @@ enum drm_mode_status {
>
> #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF
>
> +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGEBIT(1)
> +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGEBIT(2)
> +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE BIT(3)
What is the purpose of DRM_MODE_FLAG_POL_PIXDATA_PRESERVE?
If 'preserve' means 'ignore' we can set to zero negedge and posedge bits
instead of adding new bit. If it is something different please describe it.
> +#define DRM_MODE_FLAG_POL_DE_LOW BIT(4)
> +#define DRM_MODE_FLAG_POL_DE_HIGHBIT(5)
> +#define DRM_MODE_FLAG_POL_DE_PRESERVEBIT(6)
> +
The same comments here.
> struct drm_display_mode {
> /* Header */
> struct list_head head;
> @@ -144,6 +151,7 @@ struct drm_display_mode {
> int vrefresh; /* in Hz */
> int hsync; /* in kHz */
> enum hdmi_picture_aspect picture_aspect_ratio;
> + unsigned int pol_flags;
Adding field and macros description to the DocBook would be nice.
Regards
Andrzej
> };
>
> /* mode specified on the command line */
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC] Fix interrupted recording with Hauppauge HD-PVR
Thanks Hans for getting back to me.
I've been trying out your patch and I found the device wasn't actually
restarting the streaming/recording properly after a channel
change. I changed "msecs_to_jiffies(500))" to "msecs_to_jiffies(1000))" and
had the same issue, but "msecs_to_jiffies(2000))"
seems to be working well. I'll let it keep going for a few more hours, but
at the moment it seems to be working well. The recordings
can be ended without anything hanging, and turning off the device ends the
recording properly (mine neglected this occurrence).
I'll let you know if I have any more issues,
ryley
-Original Message-
From: Hans Verkuil [mailto:hverk...@xs4all.nl]
Sent: Tuesday, April 08, 2014 12:07 AM
To: Ryley Angus; linux-media@vger.kernel.org
Subject: Re: [RFC] Fix interrupted recording with Hauppauge HD-PVR
Hi Ryley,
Thank you for the patch. Your analysis seems sound. The patch is actually
not bad for a first attempt, but I did it a bit differently.
Can you test my patch?
Regards,
Hans
Signed-off-by: Hans Verkuil
diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c
b/drivers/media/usb/hdpvr/hdpvr-video.c
index 0500c417..a61373e 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -454,6 +454,8 @@ static ssize_t hdpvr_read(struct file *file, char __user
*buffer, size_t count,
if (buf->status != BUFSTAT_READY &&
dev->status != STATUS_DISCONNECTED) {
+ int err;
+
/* return nonblocking */
if (file->f_flags & O_NONBLOCK) {
if (!ret)
@@ -461,11 +463,23 @@ static ssize_t hdpvr_read(struct file *file, char
__user *buffer, size_t count,
goto err;
}
- if (wait_event_interruptible(dev->wait_data,
- buf->status == BUFSTAT_READY))
{
- ret = -ERESTARTSYS;
+ err =
wait_event_interruptible_timeout(dev->wait_data,
+ buf->status == BUFSTAT_READY,
+ msecs_to_jiffies(500));
+ if (err < 0) {
+ ret = err;
goto err;
}
+ if (!err) {
+ v4l2_dbg(MSG_INFO, hdpvr_debug,
&dev->v4l2_dev,
+ "timeout: restart streaming\n");
+ hdpvr_stop_streaming(dev);
+ err = hdpvr_start_streaming(dev);
+ if (err) {
+ ret = err;
+ goto err;
+ }
+ }
}
if (buf->status != BUFSTAT_READY)
On 04/07/2014 02:04 AM, Ryley Angus wrote:
> (Sorry in advance for probably breaking a few conventions of the
> mailing lists. First time using one so please let me know what I'm
> doing wrong)
>
> I'm writing this because of an issue I had with my Hauppauge HD-PVR.
> I record from my satellite set top box using component video and
> optical audio input. I basically use "cat /dev/video0 > ~/video.ts"
> or use dd. The box is set to output audio as AC-3 over optical, but
> most channels are actually output as stereo PCM. When the channel is
> changed between a PCM channel and (typically to a movie channel) to a
> channel utilising AC-3, the HD-PVR stops the recording as the set top
> box momentarily outputs no audio. Changing between PCM channels
> doesn't cause any issues.
>
> My main problem was that when this happens, cat/dd doesn't actually
> exit. When going through the hdpvr driver source and the dmesg output,
> I found the hdpvr driver wasn't actually shutting down the device
> properly until I manually killed cat/dd.
>
> I've seen references to this issue being a hardware issue from as far
> back as 2010:
> http://forums.gbpvr.com/showthread.php?46429-HD-PVR-drops-recording-on
> -channel-change-Hauppauge-says-too-bad
> .
>
> I tracked my issue to the file "hdpvr-video.c". Specifically, "if
> (wait_event_interruptible(dev->wait_data, buf->status =
> BUFSTAT_READY)) {" (line ~450). The device seems to get stuck waiting
> for the buffer to become ready. But as far as I can tell, when the
> channel is changed between a PCM and AC-3 broadcast the buffer status
> will never actually become ready.
>
> I haven't really ever done much coding, but I wrote a really nasty
> patch for this which tracks the devices buffer status and stops/starts
> the devices recording when the buffer is not ready for a period of
> time. In my limited testing it has worked perfectly, no output is
> overwritten, the output is in sync and the recording changes
cron job: media_tree daily build: OK
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 Apr 8 04:05:59 CEST 2014 git branch: test git hash: a83b93a7480441a47856dc9104bea970e84cda87 gcc version:i686-linux-gcc (GCC) 4.8.2 sparse version: v0.5.0-11-g38d1124 host hardware: x86_64 host os:3.13-7.slh.1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: 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.31.14-i686: OK linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12-i686: OK linux-3.13-i686: OK linux-3.14-i686: OK linux-2.6.31.14-x86_64: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12-x86_64: OK linux-3.13-x86_64: OK linux-3.14-x86_64: OK apps: OK spec-git: OK sparse version: v0.5.0-11-g38d1124 sparse: ERRORS 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/media.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
LOANS
Do you need a loan at 2% with fast approval?If interested Email us:via paydayloan...@yahoo.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/4] gsc-m2m: report correct format bytesperline and sizeimage
Hi Shaik, Sungchun,
Exynos-gsc has no maintainers mentioned in the MAINTAINERS file.
I see your activity in the development of this driver. Do you have
any comments on this patch?
Best wishes,
--
Kamil Debski
Samsung R&D Institute Poland
> -Original Message-
> From: John Sheu [mailto:s...@google.com]
> Sent: Tuesday, March 11, 2014 11:52 PM
> To: linux-media@vger.kernel.org
> Cc: m.che...@samsung.com; k.deb...@samsung.com; posc...@google.com;
> aru...@samsung.com; kgene@samsung.com; John Sheu
> Subject: [PATCH 3/4] gsc-m2m: report correct format bytesperline and
> sizeimage
>
> Explicitly specify sampling period for subsampled chroma formats, so
> stride and image size are properly reported through VIDIOC_{S,G}_FMT.
>
> Signed-off-by: John Sheu
> ---
> drivers/media/platform/exynos-gsc/gsc-core.c | 154 +++
> drivers/media/platform/exynos-gsc/gsc-core.h | 16 +--
> drivers/media/platform/exynos-gsc/gsc-regs.c | 40 +++
> drivers/media/platform/exynos-gsc/gsc-regs.h | 4 +-
> 4 files changed, 116 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c
> b/drivers/media/platform/exynos-gsc/gsc-core.c
> index 9d0cc04d..c02addef 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
> @@ -34,167 +34,185 @@ static const struct gsc_fmt gsc_formats[] = {
> {
> .name = "RGB565",
> .pixelformat= V4L2_PIX_FMT_RGB565X,
> - .depth = { 16 },
> .color = GSC_RGB,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 16 },
> + .sampling = { { 1, 1 } },
> }, {
> .name = "XRGB-8-8-8-8, 32 bpp",
> .pixelformat= V4L2_PIX_FMT_RGB32,
> - .depth = { 32 },
> .color = GSC_RGB,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 32 },
> + .sampling = { { 1, 1 } },
> }, {
> .name = "YUV 4:2:2 packed, YCbYCr",
> .pixelformat= V4L2_PIX_FMT_YUYV,
> - .depth = { 16 },
> .color = GSC_YUV422,
> .yorder = GSC_LSB_Y,
> .corder = GSC_CBCR,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 16 },
> + .sampling = { { 1, 1 } },
> .mbus_code = V4L2_MBUS_FMT_YUYV8_2X8,
> }, {
> .name = "YUV 4:2:2 packed, CbYCrY",
> .pixelformat= V4L2_PIX_FMT_UYVY,
> - .depth = { 16 },
> .color = GSC_YUV422,
> .yorder = GSC_LSB_C,
> .corder = GSC_CBCR,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 16 },
> + .sampling = { { 1, 1 } },
> .mbus_code = V4L2_MBUS_FMT_UYVY8_2X8,
> }, {
> .name = "YUV 4:2:2 packed, CrYCbY",
> .pixelformat= V4L2_PIX_FMT_VYUY,
> - .depth = { 16 },
> .color = GSC_YUV422,
> .yorder = GSC_LSB_C,
> .corder = GSC_CRCB,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 16 },
> + .sampling = { { 1, 1 } },
> .mbus_code = V4L2_MBUS_FMT_VYUY8_2X8,
> }, {
> .name = "YUV 4:2:2 packed, YCrYCb",
> .pixelformat= V4L2_PIX_FMT_YVYU,
> - .depth = { 16 },
> .color = GSC_YUV422,
> .yorder = GSC_LSB_Y,
> .corder = GSC_CRCB,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 16 },
> + .sampling = { { 1, 1 } },
> .mbus_code = V4L2_MBUS_FMT_YVYU8_2X8,
> }, {
> .name = "YUV 4:4:4 planar, YCbYCr",
> .pixelformat= V4L2_PIX_FMT_YUV32,
> - .depth = { 32 },
> .color = GSC_YUV444,
> .yorder = GSC_LSB_Y,
> .corder = GSC_CBCR,
> .num_planes = 1,
> .num_comp = 1,
> + .depth = { 32 },
> + .sampling = { { 1, 1 } },
> }, {
> .name = "YUV 4:2:2 planar, Y/Cb/Cr",
> .pixelformat= V4L2_PIX_FMT_YUV422P,
> - .depth = { 16 },
> .color = GSC_YUV422,
> .yorder = GSC_LSB_Y,
>
Re: [PATCH] [media] stk1160: warrant a NUL terminated string
On Apr 07, Mauro Carvalho Chehab wrote: > strncpy() doesn't warrant a NUL terminated string. Use > strlcpy() instead. > > Fixes Coverity bug CID#1195195. > > Signed-off-by: Mauro Carvalho Chehab > --- > drivers/media/usb/stk1160/stk1160-ac97.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c > b/drivers/media/usb/stk1160/stk1160-ac97.c > index c46c8be89602..2dd308f9541f 100644 > --- a/drivers/media/usb/stk1160/stk1160-ac97.c > +++ b/drivers/media/usb/stk1160/stk1160-ac97.c > @@ -108,7 +108,7 @@ int stk1160_ac97_register(struct stk1160 *dev) >"stk1160-mixer"); > snprintf(card->longname, sizeof(card->longname), >"stk1160 ac97 codec mixer control"); > - strncpy(card->driver, dev->dev->driver->name, sizeof(card->driver)); > + strlcpy(card->driver, dev->dev->driver->name, sizeof(card->driver)); > > rc = snd_ac97_bus(card, 0, &stk1160_ac97_ops, NULL, &ac97_bus); > if (rc) Shame on me. Acked-by: Ezequiel Garcia -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [GIT PULL for 3.15 fixes] VPE fixes
Hi Archit, Thank you for separating the fix patches. I am waiting for 3.15-rc1 to be released. Best wishes, -- Kamil Debski Samsung R&D Institute Poland > -Original Message- > From: Archit Taneja [mailto:arc...@ti.com] > Sent: Monday, April 07, 2014 11:31 AM > To: Kamil Debski > Cc: linux-media@vger.kernel.org > Subject: [GIT PULL for 3.15 fixes] VPE fixes > > Hi Kamil, > > Since the VPE m2m patch set couldn't make it on time, I've separated > out the fixes from the series so that they can be taken in one of the > 3.15-rc series. > > Thanks, > Archit > > The following changes since commit > a83b93a7480441a47856dc9104bea970e84cda87: > >[media] em28xx-dvb: fix PCTV 461e tuner I2C binding (2014-03-31 > 08:02:16 -0300) > > are available in the git repository at: > >git://github.com/boddob/linux.git vpe-fixes-315-rc > > for you to fetch changes up to > 3c7d629f0aa98ed587306913831e5a8968504f7a: > >v4l: ti-vpe: retain v4l2_buffer flags for captured buffers > (2014-04-07 12:56:47 +0530) > > > Archit Taneja (9): >v4l: ti-vpe: Make sure in job_ready that we have the needed > number of dst_bufs >v4l: ti-vpe: Use video_device_release_empty >v4l: ti-vpe: Allow usage of smaller images >v4l: ti-vpe: report correct capabilities in querycap >v4l: ti-vpe: Use correct bus_info name for the device in > querycap >v4l: ti-vpe: Fix initial configuration queue data >v4l: ti-vpe: zero out reserved fields in try_fmt >v4l: ti-vpe: Set correct field parameter for output and capture > buffers >v4l: ti-vpe: retain v4l2_buffer flags for captured buffers > > drivers/media/platform/ti-vpe/vpe.c | 45 > ++--- > 1 file changed, 32 insertions(+), 13 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/4] v4l2-mem2mem: allow reqbufs(0) with "in use" MMAP buffers
Pawel, Marek,
Before taking this to my tree I wanted to get an ACK from one of the
videobuf2 maintainers. Could you spare a moment to look through this
patch?
Best wishes,
--
Kamil Debski
Samsung R&D Institute Poland
> -Original Message-
> From: John Sheu [mailto:s...@google.com]
> Sent: Tuesday, March 11, 2014 11:52 PM
> To: linux-media@vger.kernel.org
> Cc: m.che...@samsung.com; k.deb...@samsung.com; posc...@google.com;
> aru...@samsung.com; kgene@samsung.com; John Sheu
> Subject: [PATCH 4/4] v4l2-mem2mem: allow reqbufs(0) with "in use" MMAP
> buffers
>
> v4l2-mem2mem presently does not allow VIDIOC_REQBUFS to destroy
> outstanding buffers if the queue is of type V4L2_MEMORY_MMAP, and if
> the buffers are considered "in use". This is different behavior than
> for other memory types, and prevents us for deallocating buffers in a
> few
> cases:
>
> * In the case that there are outstanding mmap()ed views on the buffer,
> refcounting on the videobuf2 buffer backing the vm_area will track
> lifetime appropriately,
> * In the case that the buffer has been exported as a DMABUF,
> refcounting
> on the videobuf2 bufer backing the DMABUF will track lifetime
> appropriately.
>
> Remove the specific check for type V4L2_MEMOMRY_MMAP when freeing all
> buffers through VIDIOC_REQBUFS.
>
> Signed-off-by: John Sheu
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 26 +---
> --
> 1 file changed, 1 insertion(+), 25 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c
> index 8e6695c9..5b6f9da6 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -414,8 +414,7 @@ static int __verify_length(struct vb2_buffer *vb,
> const struct v4l2_buffer *b) }
>
> /**
> - * __buffer_in_use() - return true if the buffer is in use and
> - * the queue cannot be freed (by the means of REQBUFS(0)) call
> + * __buffer_in_use() - return true if the buffer is in use.
> */
> static bool __buffer_in_use(struct vb2_queue *q, struct vb2_buffer
> *vb) { @@ -435,20 +434,6 @@ static bool __buffer_in_use(struct
> vb2_queue *q, struct vb2_buffer *vb) }
>
> /**
> - * __buffers_in_use() - return true if any buffers on the queue are in
> use and
> - * the queue cannot be freed (by the means of REQBUFS(0)) call
> - */
> -static bool __buffers_in_use(struct vb2_queue *q) -{
> - unsigned int buffer;
> - for (buffer = 0; buffer < q->num_buffers; ++buffer) {
> - if (__buffer_in_use(q, q->bufs[buffer]))
> - return true;
> - }
> - return false;
> -}
> -
> -/**
> * __fill_v4l2_buffer() - fill in a struct v4l2_buffer with
> information to be
> * returned to userspace
> */
> @@ -681,15 +666,6 @@ static int __reqbufs(struct vb2_queue *q, struct
> v4l2_requestbuffers *req)
> }
>
> if (req->count == 0 || q->num_buffers != 0 || q->memory != req-
> >memory) {
> - /*
> - * We already have buffers allocated, so first check if
> they
> - * are not in use and can be freed.
> - */
> - if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) {
> - dprintk(1, "reqbufs: memory in use, cannot free\n");
> - return -EBUSY;
> - }
> -
> ret = __vb2_queue_free(q, q->num_buffers);
> if (ret)
> return ret;
> --
> 1.9.0.279.gdc9e3eb
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Fix interrupted recording with Hauppauge HD-PVR
Hi Ryley,
Thank you for the patch. Your analysis seems sound. The patch is
actually not bad for a first attempt, but I did it a bit differently.
Can you test my patch?
Regards,
Hans
Signed-off-by: Hans Verkuil
diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c
b/drivers/media/usb/hdpvr/hdpvr-video.c
index 0500c417..a61373e 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -454,6 +454,8 @@ static ssize_t hdpvr_read(struct file *file, char __user
*buffer, size_t count,
if (buf->status != BUFSTAT_READY &&
dev->status != STATUS_DISCONNECTED) {
+ int err;
+
/* return nonblocking */
if (file->f_flags & O_NONBLOCK) {
if (!ret)
@@ -461,11 +463,23 @@ static ssize_t hdpvr_read(struct file *file, char __user
*buffer, size_t count,
goto err;
}
- if (wait_event_interruptible(dev->wait_data,
- buf->status == BUFSTAT_READY)) {
- ret = -ERESTARTSYS;
+ err = wait_event_interruptible_timeout(dev->wait_data,
+ buf->status == BUFSTAT_READY,
+ msecs_to_jiffies(500));
+ if (err < 0) {
+ ret = err;
goto err;
}
+ if (!err) {
+ v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev,
+ "timeout: restart streaming\n");
+ hdpvr_stop_streaming(dev);
+ err = hdpvr_start_streaming(dev);
+ if (err) {
+ ret = err;
+ goto err;
+ }
+ }
}
if (buf->status != BUFSTAT_READY)
On 04/07/2014 02:04 AM, Ryley Angus wrote:
> (Sorry in advance for probably breaking a few conventions of the
> mailing lists. First time using one so please let me know what I’m
> doing wrong)
>
> I’m writing this because of an issue I had with my Hauppauge HD-PVR.
> I record from my satellite set top box using component video and
> optical audio input. I basically use "cat /dev/video0 > ~/video.ts”
> or use dd. The box is set to output audio as AC-3 over optical, but
> most channels are actually output as stereo PCM. When the channel is
> changed between a PCM channel and (typically to a movie channel) to a
> channel utilising AC-3, the HD-PVR stops the recording as the set top
> box momentarily outputs no audio. Changing between PCM channels
> doesn’t cause any issues.
>
> My main problem was that when this happens, cat/dd doesn’t actually
> exit. When going through the hdpvr driver source and the dmesg
> output, I found the hdpvr driver wasn’t actually shutting down the
> device properly until I manually killed cat/dd.
>
> I’ve seen references to this issue being a hardware issue from as far
> back as 2010:
> http://forums.gbpvr.com/showthread.php?46429-HD-PVR-drops-recording-on-channel-change-Hauppauge-says-too-bad
> .
>
> I tracked my issue to the file “hdpvr-video.c”. Specifically, “if
> (wait_event_interruptible(dev->wait_data, buf->status =
> BUFSTAT_READY)) {“ (line ~450). The device seems to get stuck waiting
> for the buffer to become ready. But as far as I can tell, when the
> channel is changed between a PCM and AC-3 broadcast the buffer status
> will never actually become ready.
>
> I haven’t really ever done much coding, but I wrote a really nasty
> patch for this which tracks the devices buffer status and
> stops/starts the devices recording when the buffer is not ready for a
> period of time. In my limited testing it has worked perfectly, no
> output is overwritten, the output is in sync and the recording
> changes perfectly between stereo AC-3 (PCM input is encoded to AC-3
> by the hardware) and 5.1 AC-3 no matter how frequently I change the
> channels back and forth. All changes are transparent to cat/dd and
> neither exit prematurely. Manually killing cat/dd seems to properly
> shut down the device. There is a half-second glitch in the resultant
> where the recording restarts, but this amounts to less than a second
> of lost footage during the change and compared to having the device
> hang, I can live with it. I haven’t had the device restart recording
> when it shouldn’t have.
>
> So considering my code is really messy, I’d love it if someone could
> make some suggestions to make the code better and make sure I don’t
> have too many logic errors. I don’t really know too much about kernel
> coding practices either. And if anyone who’s experienced my
[PATCH] [media] gpsca: remove the risk of a division by zero
As reported by Coverity, there's a potential risk of a division
by zero on some calls to jpeg_set_qual(), if quality is zero.
As quality can't be 0 or lower than that, adds an extra clause
to cover this special case.
Coverity reports: CID#11922280, CID#11922293, CID#11922295
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/usb/gspca/jpeg.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/gspca/jpeg.h b/drivers/media/usb/gspca/jpeg.h
index ab54910418b4..0aa2b671faa4 100644
--- a/drivers/media/usb/gspca/jpeg.h
+++ b/drivers/media/usb/gspca/jpeg.h
@@ -154,7 +154,9 @@ static void jpeg_set_qual(u8 *jpeg_hdr,
{
int i, sc;
- if (quality < 50)
+ if (quality <= 0)
+ sc = 5000;
+ else if (quality < 50)
sc = 5000 / quality;
else
sc = 200 - quality * 2;
--
1.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] stk1160: warrant a NUL terminated string
On 04/07/2014 03:42 PM, Mauro Carvalho Chehab wrote: > strncpy() doesn't warrant a NUL terminated string. Use > strlcpy() instead. > > Fixes Coverity bug CID#1195195. > > Signed-off-by: Mauro Carvalho Chehab My good deed for the day: Acked-by: Hans Verkuil Regards, Hans > --- > drivers/media/usb/stk1160/stk1160-ac97.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c > b/drivers/media/usb/stk1160/stk1160-ac97.c > index c46c8be89602..2dd308f9541f 100644 > --- a/drivers/media/usb/stk1160/stk1160-ac97.c > +++ b/drivers/media/usb/stk1160/stk1160-ac97.c > @@ -108,7 +108,7 @@ int stk1160_ac97_register(struct stk1160 *dev) >"stk1160-mixer"); > snprintf(card->longname, sizeof(card->longname), >"stk1160 ac97 codec mixer control"); > - strncpy(card->driver, dev->dev->driver->name, sizeof(card->driver)); > + strlcpy(card->driver, dev->dev->driver->name, sizeof(card->driver)); > > rc = snd_ac97_bus(card, 0, &stk1160_ac97_ops, NULL, &ac97_bus); > if (rc) > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] stk1160: warrant a NUL terminated string
strncpy() doesn't warrant a NUL terminated string. Use strlcpy() instead. Fixes Coverity bug CID#1195195. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/stk1160/stk1160-ac97.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c b/drivers/media/usb/stk1160/stk1160-ac97.c index c46c8be89602..2dd308f9541f 100644 --- a/drivers/media/usb/stk1160/stk1160-ac97.c +++ b/drivers/media/usb/stk1160/stk1160-ac97.c @@ -108,7 +108,7 @@ int stk1160_ac97_register(struct stk1160 *dev) "stk1160-mixer"); snprintf(card->longname, sizeof(card->longname), "stk1160 ac97 codec mixer control"); - strncpy(card->driver, dev->dev->driver->name, sizeof(card->driver)); + strlcpy(card->driver, dev->dev->driver->name, sizeof(card->driver)); rc = snd_ac97_bus(card, 0, &stk1160_ac97_ops, NULL, &ac97_bus); if (rc) -- 1.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] [media] s5p_jpeg: Prevent JPEG 4:2:0 > YUV 4:2:0 decompression
Prevent decompression of a JPEG 4:2:0 with odd width to
the YUV 4:2:0 compliant formats for Exynos4x12 SoCs and
adjust capture format to RGB565 in such a case. This is
required because the configuration would produce a raw
image with broken luma component.
Signed-off-by: Jacek Anaszewski
Signed-off-by: Kyungmin Park
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index d266e78..9228bcb 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1064,15 +1064,17 @@ static int s5p_jpeg_try_fmt_vid_cap(struct file *file,
void *priv,
return -EINVAL;
}
+ if ((ctx->jpeg->variant->version != SJPEG_EXYNOS4) ||
+ (ctx->mode != S5P_JPEG_DECODE))
+ goto exit;
+
/*
* The exynos4x12 device requires resulting YUV image
* subsampling not to be lower than the input jpeg subsampling.
* If this requirement is not met then downgrade the requested
* capture format to the one with subsampling equal to the input jpeg.
*/
- if ((ctx->jpeg->variant->version == SJPEG_EXYNOS4) &&
- (ctx->mode == S5P_JPEG_DECODE) &&
- (fmt->flags & SJPEG_FMT_NON_RGB) &&
+ if ((fmt->flags & SJPEG_FMT_NON_RGB) &&
(fmt->subsampling < ctx->subsampling)) {
ret = s5p_jpeg_adjust_fourcc_to_subsampling(ctx->subsampling,
fmt->fourcc,
@@ -1085,6 +1087,22 @@ static int s5p_jpeg_try_fmt_vid_cap(struct file *file,
void *priv,
FMT_TYPE_CAPTURE);
}
+ if (ctx->subsampling == V4L2_JPEG_CHROMA_SUBSAMPLING_420 &&
+ (ctx->out_q.w & 1) &&
+ (pix->pixelformat == V4L2_PIX_FMT_NV12 ||
+pix->pixelformat == V4L2_PIX_FMT_NV21 ||
+pix->pixelformat == V4L2_PIX_FMT_YUV420)) {
+ pix->pixelformat = V4L2_PIX_FMT_RGB565;
+ fmt = s5p_jpeg_find_format(ctx, pix->pixelformat,
+ FMT_TYPE_CAPTURE);
+ v4l2_info(&ctx->jpeg->v4l2_dev,
+ "Adjusted capture fourcc to RGB565. Decompression\n"
+ "of a JPEG file with 4:2:0 subsampling and odd\n"
+ "width to the YUV 4:2:0 compliant formats produces\n"
+ "a raw image with broken luma component.\n");
+ }
+
+exit:
return vidioc_try_fmt(f, fmt, ctx, FMT_TYPE_CAPTURE);
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/8] [media] s5p_jpeg: Fix NV12 format entry related to S5C2120 SoC
S5PC210 SoC doesn't support encoding NV12 raw images. Remove
the relavant flag from the respective entry in the sjpeg_formats
array.
Signed-off-by: Jacek Anaszewski
Signed-off-by: Kyungmin Park
---
drivers/media/platform/s5p-jpeg/jpeg-core.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 9228bcb..a57a1e0 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -192,8 +192,7 @@ static struct s5p_jpeg_fmt sjpeg_formats[] = {
.colplanes = 2,
.h_align= 4,
.v_align= 4,
- .flags = SJPEG_FMT_FLAG_ENC_OUTPUT |
- SJPEG_FMT_FLAG_DEC_CAPTURE |
+ .flags = SJPEG_FMT_FLAG_DEC_CAPTURE |
SJPEG_FMT_FLAG_S5P |
SJPEG_FMT_NON_RGB,
.subsampling= V4L2_JPEG_CHROMA_SUBSAMPLING_420,
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] [media] s5p-jpeg: Fix sysmmu page fault
This patch fixes jpeg sysmmu page fault on Exynos4x12 SoCs.
During encoding Exynos4x12 SoCs access wider memory area
than it results from Image_x and Image_y values written to
the JPEG_IMAGE_SIZE register. In order to avoid sysmmu page
fault apply proper output buffer size alignment.
Signed-off-by: Jacek Anaszewski
Signed-off-by: Kyungmin Park
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 46 +--
1 file changed, 43 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 8a15c4a..d266e78 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1106,6 +1106,32 @@ static int s5p_jpeg_try_fmt_vid_out(struct file *file,
void *priv,
return vidioc_try_fmt(f, fmt, ctx, FMT_TYPE_OUTPUT);
}
+static int exynos4_jpeg_get_output_buffer_size(struct s5p_jpeg_ctx *ctx,
+ struct v4l2_format *f,
+ int fmt_depth)
+{
+ struct v4l2_pix_format *pix = &f->fmt.pix;
+ u32 pix_fmt = f->fmt.pix.pixelformat;
+ int w = pix->width, h = pix->height, wh_align;
+
+ if (pix_fmt == V4L2_PIX_FMT_RGB32 ||
+ pix_fmt == V4L2_PIX_FMT_NV24 ||
+ pix_fmt == V4L2_PIX_FMT_NV42 ||
+ pix_fmt == V4L2_PIX_FMT_NV12 ||
+ pix_fmt == V4L2_PIX_FMT_NV21 ||
+ pix_fmt == V4L2_PIX_FMT_YUV420)
+ wh_align = 4;
+ else
+ wh_align = 1;
+
+ jpeg_bound_align_image(&w, S5P_JPEG_MIN_WIDTH,
+ S5P_JPEG_MAX_WIDTH, wh_align,
+ &h, S5P_JPEG_MIN_HEIGHT,
+ S5P_JPEG_MAX_HEIGHT, wh_align);
+
+ return w * h * fmt_depth >> 3;
+}
+
static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, struct v4l2_format *f)
{
struct vb2_queue *vq;
@@ -1132,10 +1158,24 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct,
struct v4l2_format *f)
q_data->fmt = s5p_jpeg_find_format(ct, pix->pixelformat, f_type);
q_data->w = pix->width;
q_data->h = pix->height;
- if (q_data->fmt->fourcc != V4L2_PIX_FMT_JPEG)
- q_data->size = q_data->w * q_data->h * q_data->fmt->depth >> 3;
- else
+ if (q_data->fmt->fourcc != V4L2_PIX_FMT_JPEG) {
+ /*
+* During encoding Exynos4x12 SoCs access wider memory area
+* than it results from Image_x and Image_y values written to
+* the JPEG_IMAGE_SIZE register. In order to avoid sysmmu
+* page fault calculate proper buffer size in such a case.
+*/
+ if (ct->jpeg->variant->version == SJPEG_EXYNOS4 &&
+ f_type == FMT_TYPE_OUTPUT && ct->mode == S5P_JPEG_ENCODE)
+ q_data->size = exynos4_jpeg_get_output_buffer_size(ct,
+ f,
+ q_data->fmt->depth);
+ else
+ q_data->size = q_data->w * q_data->h *
+ q_data->fmt->depth >> 3;
+ } else {
q_data->size = pix->sizeimage;
+ }
if (f_type == FMT_TYPE_OUTPUT) {
ctrl_subs = v4l2_ctrl_find(&ct->ctrl_handler,
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] [media] s5p-jpeg: Fix build break when CONFIG_OF is undefined
This patch fixes build break occurring when
there is no support for Device Tree turned on
in the kernel configuration. In such a case only
the driver variant for S5PC210 SoC will be available.
Signed-off-by: Jacek Anaszewski
Signed-off-by: Kyungmin Park
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 4f4dc81..913a027 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1837,7 +1837,7 @@ static irqreturn_t exynos4_jpeg_irq(int irq, void *priv)
return IRQ_HANDLED;
}
-static void *jpeg_get_drv_data(struct platform_device *pdev);
+static void *jpeg_get_drv_data(struct device *dev);
/*
*
@@ -1851,15 +1851,12 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
struct resource *res;
int ret;
- if (!pdev->dev.of_node)
- return -ENODEV;
-
/* JPEG IP abstraction struct */
jpeg = devm_kzalloc(&pdev->dev, sizeof(struct s5p_jpeg), GFP_KERNEL);
if (!jpeg)
return -ENOMEM;
- jpeg->variant = jpeg_get_drv_data(pdev);
+ jpeg->variant = jpeg_get_drv_data(&pdev->dev);
mutex_init(&jpeg->lock);
spin_lock_init(&jpeg->slock);
@@ -2088,7 +2085,6 @@ static const struct dev_pm_ops s5p_jpeg_pm_ops = {
SET_RUNTIME_PM_OPS(s5p_jpeg_runtime_suspend, s5p_jpeg_runtime_resume,
NULL)
};
-#ifdef CONFIG_OF
static struct s5p_jpeg_variant s5p_jpeg_drvdata = {
.version= SJPEG_S5P,
.jpeg_irq = s5p_jpeg_irq,
@@ -2119,19 +2115,21 @@ static const struct of_device_id samsung_jpeg_match[] =
{
MODULE_DEVICE_TABLE(of, samsung_jpeg_match);
-static void *jpeg_get_drv_data(struct platform_device *pdev)
+static void *jpeg_get_drv_data(struct device *dev)
{
struct s5p_jpeg_variant *driver_data = NULL;
const struct of_device_id *match;
+ if (!IS_ENABLED(CONFIG_OF) || dev->of_node == NULL)
+ return &s5p_jpeg_drvdata;
+
match = of_match_node(of_match_ptr(samsung_jpeg_match),
-pdev->dev.of_node);
+ dev->of_node);
if (match)
driver_data = (struct s5p_jpeg_variant *)match->data;
return driver_data;
}
-#endif
static struct platform_driver s5p_jpeg_driver = {
.probe = s5p_jpeg_probe,
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/8] [media] s5p-jpeg: g_selection callback should always succeed
Remove erroneous guard preventing successful execution of g_selection callback in case the driver variant is different from SJPEG_S5P. Signed-off-by: Jacek Anaszewski Signed-off-by: Kyungmin Park --- drivers/media/platform/s5p-jpeg/jpeg-core.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c index 913a027..8a15c4a 100644 --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c @@ -1177,8 +1177,7 @@ static int s5p_jpeg_g_selection(struct file *file, void *priv, struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv); if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT && - s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE && - ctx->jpeg->variant->version != SJPEG_S5P) + s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) return -EINVAL; /* For JPEG blob active == default == bounds */ -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/8] [media] s5p-jpeg: Add m2m_ops field to the s5p_jpeg_variant structure
Simplify the code by adding m2m_ops field to the
s5p_jpeg_variant structure which allows to avoid
"if" statement in the s5p_jpeg_probe function.
Signed-off-by: Jacek Anaszewski
Signed-off-by: Kyungmin Park
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 12
drivers/media/platform/s5p-jpeg/jpeg-core.h |7 ---
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index c675c90..4f4dc81 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1566,7 +1566,7 @@ static struct v4l2_m2m_ops s5p_jpeg_m2m_ops = {
.job_abort = s5p_jpeg_job_abort,
}
;
-static struct v4l2_m2m_ops exynos_jpeg_m2m_ops = {
+static struct v4l2_m2m_ops exynos4_jpeg_m2m_ops = {
.device_run = exynos4_jpeg_device_run,
.job_ready = s5p_jpeg_job_ready,
.job_abort = s5p_jpeg_job_abort,
@@ -1849,7 +1849,6 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
{
struct s5p_jpeg *jpeg;
struct resource *res;
- struct v4l2_m2m_ops *samsung_jpeg_m2m_ops;
int ret;
if (!pdev->dev.of_node)
@@ -1903,13 +1902,8 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
goto clk_get_rollback;
}
- if (jpeg->variant->version == SJPEG_S5P)
- samsung_jpeg_m2m_ops = &s5p_jpeg_m2m_ops;
- else
- samsung_jpeg_m2m_ops = &exynos_jpeg_m2m_ops;
-
/* mem2mem device */
- jpeg->m2m_dev = v4l2_m2m_init(samsung_jpeg_m2m_ops);
+ jpeg->m2m_dev = v4l2_m2m_init(jpeg->variant->m2m_ops);
if (IS_ERR(jpeg->m2m_dev)) {
v4l2_err(&jpeg->v4l2_dev, "Failed to init mem2mem device\n");
ret = PTR_ERR(jpeg->m2m_dev);
@@ -2098,12 +2092,14 @@ static const struct dev_pm_ops s5p_jpeg_pm_ops = {
static struct s5p_jpeg_variant s5p_jpeg_drvdata = {
.version= SJPEG_S5P,
.jpeg_irq = s5p_jpeg_irq,
+ .m2m_ops= &s5p_jpeg_m2m_ops,
.fmt_ver_flag = SJPEG_FMT_FLAG_S5P,
};
static struct s5p_jpeg_variant exynos4_jpeg_drvdata = {
.version= SJPEG_EXYNOS4,
.jpeg_irq = exynos4_jpeg_irq,
+ .m2m_ops= &exynos4_jpeg_m2m_ops,
.fmt_ver_flag = SJPEG_FMT_FLAG_EXYNOS4,
};
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.h
b/drivers/media/platform/s5p-jpeg/jpeg-core.h
index c222436..3e47863 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.h
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.h
@@ -117,9 +117,10 @@ struct s5p_jpeg {
};
struct s5p_jpeg_variant {
- unsigned intversion;
- unsigned intfmt_ver_flag;
- irqreturn_t (*jpeg_irq)(int irq, void *priv);
+ unsigned intversion;
+ unsigned intfmt_ver_flag;
+ struct v4l2_m2m_ops *m2m_ops;
+ irqreturn_t (*jpeg_irq)(int irq, void *priv);
};
/**
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/8] [media] s5p-jpeg: Perform fourcc downgrade only for Exynos4x12 SoCs
Change the driver variant check from "is not S5PC210"
to "is Exynos4" while checking whether YUV format needs
to be downgraded in order to prevent upsampling which
is not supported by Exynos4 SoCs family.
Signed-off-by: Jacek Anaszewski
Signed-off-by: Kyungmin Park
---
drivers/media/platform/s5p-jpeg/jpeg-core.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 9b0102d..c675c90 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1070,7 +1070,7 @@ static int s5p_jpeg_try_fmt_vid_cap(struct file *file,
void *priv,
* If this requirement is not met then downgrade the requested
* capture format to the one with subsampling equal to the input jpeg.
*/
- if ((ctx->jpeg->variant->version != SJPEG_S5P) &&
+ if ((ctx->jpeg->variant->version == SJPEG_EXYNOS4) &&
(ctx->mode == S5P_JPEG_DECODE) &&
(fmt->flags & SJPEG_FMT_NON_RGB) &&
(fmt->subsampling < ctx->subsampling)) {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/8] [media] s5p-jpeg: Add fmt_ver_flag field to the s5p_jpeg_variant structure
Simplify the code by adding fmt_ver_flag field
to the s5p_jpeg_variant structure which allows
to avoid "if" statement in the s5p_jpeg_find_format
function.
Signed-off-by: Jacek Anaszewski
Signed-off-by: Kyungmin Park
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 11 ---
drivers/media/platform/s5p-jpeg/jpeg-core.h |1 +
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 6db4d5e..9b0102d 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -959,7 +959,7 @@ static int s5p_jpeg_g_fmt(struct file *file, void *priv,
struct v4l2_format *f)
static struct s5p_jpeg_fmt *s5p_jpeg_find_format(struct s5p_jpeg_ctx *ctx,
u32 pixelformat, unsigned int fmt_type)
{
- unsigned int k, fmt_flag, ver_flag;
+ unsigned int k, fmt_flag;
if (ctx->mode == S5P_JPEG_ENCODE)
fmt_flag = (fmt_type == FMT_TYPE_OUTPUT) ?
@@ -970,16 +970,11 @@ static struct s5p_jpeg_fmt *s5p_jpeg_find_format(struct
s5p_jpeg_ctx *ctx,
SJPEG_FMT_FLAG_DEC_OUTPUT :
SJPEG_FMT_FLAG_DEC_CAPTURE;
- if (ctx->jpeg->variant->version == SJPEG_S5P)
- ver_flag = SJPEG_FMT_FLAG_S5P;
- else
- ver_flag = SJPEG_FMT_FLAG_EXYNOS4;
-
for (k = 0; k < ARRAY_SIZE(sjpeg_formats); k++) {
struct s5p_jpeg_fmt *fmt = &sjpeg_formats[k];
if (fmt->fourcc == pixelformat &&
fmt->flags & fmt_flag &&
- fmt->flags & ver_flag) {
+ fmt->flags & ctx->jpeg->variant->fmt_ver_flag) {
return fmt;
}
}
@@ -2103,11 +2098,13 @@ static const struct dev_pm_ops s5p_jpeg_pm_ops = {
static struct s5p_jpeg_variant s5p_jpeg_drvdata = {
.version= SJPEG_S5P,
.jpeg_irq = s5p_jpeg_irq,
+ .fmt_ver_flag = SJPEG_FMT_FLAG_S5P,
};
static struct s5p_jpeg_variant exynos4_jpeg_drvdata = {
.version= SJPEG_EXYNOS4,
.jpeg_irq = exynos4_jpeg_irq,
+ .fmt_ver_flag = SJPEG_FMT_FLAG_EXYNOS4,
};
static const struct of_device_id samsung_jpeg_match[] = {
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.h
b/drivers/media/platform/s5p-jpeg/jpeg-core.h
index f482dbf..c222436 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.h
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.h
@@ -118,6 +118,7 @@ struct s5p_jpeg {
struct s5p_jpeg_variant {
unsigned intversion;
+ unsigned intfmt_ver_flag;
irqreturn_t (*jpeg_irq)(int irq, void *priv);
};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv2 PATCH 01/13] vb2: stop_streaming should return void
From: Hans Verkuil
The vb2 core ignores any return code from the stop_streaming op.
And there really isn't anything it can do anyway in case of an error.
So change the return type to void and update any drivers that implement it.
The int return gave drivers the idea that this operation could actually
fail, but that's really not the case.
The pwc amd sdr-msi3101 drivers both had this construction:
if (mutex_lock_interruptible(&s->v4l2_lock))
return -ERESTARTSYS;
This has been updated to just call mutex_lock(). The stop_streaming op
expects this to really stop streaming and I very much doubt this will
work reliably if stop_streaming just returns without really stopping the
DMA.
Signed-off-by: Hans Verkuil
Acked-by: Pawel Osciak
---
drivers/media/pci/sta2x11/sta2x11_vip.c | 3 +--
drivers/media/platform/blackfin/bfin_capture.c | 6 +-
drivers/media/platform/coda.c| 4 +---
drivers/media/platform/davinci/vpbe_display.c| 6 ++
drivers/media/platform/davinci/vpif_capture.c| 6 ++
drivers/media/platform/davinci/vpif_display.c| 6 ++
drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 +---
drivers/media/platform/exynos4-is/fimc-capture.c | 6 +++---
drivers/media/platform/exynos4-is/fimc-lite.c| 6 +++---
drivers/media/platform/exynos4-is/fimc-m2m.c | 3 +--
drivers/media/platform/marvell-ccic/mcam-core.c | 7 +++
drivers/media/platform/mem2mem_testdev.c | 5 ++---
drivers/media/platform/s3c-camif/camif-capture.c | 4 ++--
drivers/media/platform/s5p-jpeg/jpeg-core.c | 4 +---
drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 3 +--
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 3 +--
drivers/media/platform/s5p-tv/mixer_video.c | 3 +--
drivers/media/platform/soc_camera/atmel-isi.c| 6 ++
drivers/media/platform/soc_camera/mx2_camera.c | 4 +---
drivers/media/platform/soc_camera/mx3_camera.c | 4 +---
drivers/media/platform/soc_camera/rcar_vin.c | 4 +---
drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c | 4 ++--
drivers/media/platform/vivi.c| 3 +--
drivers/media/platform/vsp1/vsp1_video.c | 4 +---
drivers/media/usb/em28xx/em28xx-v4l.h| 2 +-
drivers/media/usb/em28xx/em28xx-video.c | 8 ++--
drivers/media/usb/pwc/pwc-if.c | 7 ++-
drivers/media/usb/s2255/s2255drv.c | 5 ++---
drivers/media/usb/stk1160/stk1160-v4l.c | 4 ++--
drivers/media/usb/usbtv/usbtv-video.c| 9 +++--
drivers/staging/media/davinci_vpfe/vpfe_video.c | 5 ++---
drivers/staging/media/dt3155v4l/dt3155v4l.c | 3 +--
drivers/staging/media/go7007/go7007-v4l2.c | 3 +--
drivers/staging/media/msi3101/sdr-msi3101.c | 7 ++-
drivers/staging/media/rtl2832u_sdr/rtl2832_sdr.c | 7 ++-
drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c | 3 +--
drivers/staging/media/solo6x10/solo6x10-v4l2.c | 3 +--
include/media/videobuf2-core.h | 2 +-
38 files changed, 60 insertions(+), 116 deletions(-)
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index bb11443..7559951 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -357,7 +357,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
}
/* abort streaming and wait for last buffer */
-static int stop_streaming(struct vb2_queue *vq)
+static void stop_streaming(struct vb2_queue *vq)
{
struct sta2x11_vip *vip = vb2_get_drv_priv(vq);
struct vip_buffer *vip_buf, *node;
@@ -374,7 +374,6 @@ static int stop_streaming(struct vb2_queue *vq)
list_del(&vip_buf->list);
}
spin_unlock(&vip->lock);
- return 0;
}
static struct vb2_ops vip_video_qops = {
diff --git a/drivers/media/platform/blackfin/bfin_capture.c
b/drivers/media/platform/blackfin/bfin_capture.c
index 200bec9..16f643c 100644
--- a/drivers/media/platform/blackfin/bfin_capture.c
+++ b/drivers/media/platform/blackfin/bfin_capture.c
@@ -427,15 +427,12 @@ static int bcap_start_streaming(struct vb2_queue *vq,
unsigned int count)
return 0;
}
-static int bcap_stop_streaming(struct vb2_queue *vq)
+static void bcap_stop_streaming(struct vb2_queue *vq)
{
struct bcap_device *bcap_dev = vb2_get_drv_priv(vq);
struct ppi_if *ppi = bcap_dev->ppi;
int ret;
- if (!vb2_is_streaming(vq))
- return 0;
-
bcap_dev->stop = true;
wait_for_completion(&bcap_dev->comp);
ppi->ops->stop(ppi);
@@ -452,7 +449,6 @@ static int bcap_stop_streaming
[REVIEWv2 PATCH 13/13] DocBook media: update bytesused field description
From: Hans Verkuil For output buffers the application has to set the bytesused field. In reality applications often do not set this since drivers that deal with fix image sizes just override it anyway. The vb2 framework will replace this field with the length field if bytesused was set to 0 by the application, which is what happens in practice. Document this behavior. Signed-off-by: Hans Verkuil --- Documentation/DocBook/media/v4l/io.xml | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml index 97a69bf..188e621 100644 --- a/Documentation/DocBook/media/v4l/io.xml +++ b/Documentation/DocBook/media/v4l/io.xml @@ -699,7 +699,12 @@ linkend="v4l2-buf-type" /> buffer. It depends on the negotiated data format and may change with each buffer for compressed variable size data like JPEG images. Drivers must set this field when type -refers to an input stream, applications when it refers to an output stream. +refers to an input stream, applications when it refers to an output stream. +If the application sets this to 0 for an output stream, then +bytesused will be set to the size of the +buffer (see the length field of this struct) by +the driver. For multiplanar formats this field is ignored and the +planes pointer is used instead. __u32 @@ -861,7 +866,11 @@ should set this to 0. The number of bytes occupied by data in the plane (its payload). Drivers must set this field when type - refers to an input stream, applications when it refers to an output stream. + refers to an input stream, applications when it refers to an output stream. + If the application sets this to 0 for an output stream, then + bytesused will be set to the size of the + plane (see the length field of this struct) + by the driver. __u32 -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv2 PATCH 05/13] vb2: move __qbuf_mmap before __qbuf_userptr
From: Hans Verkuil
__qbuf_mmap was sort of hidden in between the much larger __qbuf_userptr
and __qbuf_dmabuf functions. Move it before __qbuf_userptr which is
also conform the usual order these memory models are implemented: first
mmap, then userptr, then dmabuf.
Signed-off-by: Hans Verkuil
Acked-by: Pawel Osciak
---
drivers/media/v4l2-core/videobuf2-core.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 1421075..2e448a7 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1240,6 +1240,20 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
const struct v4l2_buffer *b
}
/**
+ * __qbuf_mmap() - handle qbuf of an MMAP buffer
+ */
+static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
+{
+ int ret;
+
+ __fill_vb2_buffer(vb, b, vb->v4l2_planes);
+ ret = call_vb_qop(vb, buf_prepare, vb);
+ if (ret)
+ fail_vb_qop(vb, buf_prepare);
+ return ret;
+}
+
+/**
* __qbuf_userptr() - handle qbuf of a USERPTR buffer
*/
static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
@@ -1346,20 +1360,6 @@ err:
}
/**
- * __qbuf_mmap() - handle qbuf of an MMAP buffer
- */
-static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
-{
- int ret;
-
- __fill_vb2_buffer(vb, b, vb->v4l2_planes);
- ret = call_vb_qop(vb, buf_prepare, vb);
- if (ret)
- fail_vb_qop(vb, buf_prepare);
- return ret;
-}
-
-/**
* __qbuf_dmabuf() - handle qbuf of a DMABUF buffer
*/
static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv2 PATCH 08/13] vb2: simplify a confusing condition.
From: Hans Verkuil
q->start_streaming_called is always true, so the WARN_ON check against
it being false can be dropped.
Signed-off-by: Hans Verkuil
Acked-by: Pawel Osciak
---
drivers/media/v4l2-core/videobuf2-core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index c662ad9..89147d2 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1094,9 +1094,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum
vb2_buffer_state state)
if (!q->start_streaming_called) {
if (WARN_ON(state != VB2_BUF_STATE_QUEUED))
state = VB2_BUF_STATE_QUEUED;
- } else if (!WARN_ON(!q->start_streaming_called)) {
- if (WARN_ON(state != VB2_BUF_STATE_DONE &&
- state != VB2_BUF_STATE_ERROR))
+ } else if (WARN_ON(state != VB2_BUF_STATE_DONE &&
+ state != VB2_BUF_STATE_ERROR)) {
state = VB2_BUF_STATE_ERROR;
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv2 PATCH 12/13] vb2: start messages with a lower-case for consistency.
From: Hans Verkuil
The kernel debug messages produced by vb2 started either with a
lower or an upper case character. Switched all to use lower-case
which seemed to be what was used in the majority of the messages.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/videobuf2-core.c | 58
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index d33c69b..e79d70c 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -151,7 +151,7 @@ static void __vb2_buf_mem_free(struct vb2_buffer *vb)
for (plane = 0; plane < vb->num_planes; ++plane) {
call_memop(vb, put, vb->planes[plane].mem_priv);
vb->planes[plane].mem_priv = NULL;
- dprintk(3, "Freed plane %d of buffer %d\n", plane,
+ dprintk(3, "freed plane %d of buffer %d\n", plane,
vb->v4l2_buf.index);
}
}
@@ -246,7 +246,7 @@ static void __setup_offsets(struct vb2_queue *q, unsigned
int n)
for (plane = 0; plane < vb->num_planes; ++plane) {
vb->v4l2_planes[plane].m.mem_offset = off;
- dprintk(3, "Buffer %d, plane %d offset 0x%08lx\n",
+ dprintk(3, "buffer %d, plane %d offset 0x%08lx\n",
buffer, plane, off);
off += vb->v4l2_planes[plane].length;
@@ -273,7 +273,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
v4l2_memory memory,
/* Allocate videobuf buffer structures */
vb = kzalloc(q->buf_struct_size, GFP_KERNEL);
if (!vb) {
- dprintk(1, "Memory alloc for buffer struct failed\n");
+ dprintk(1, "memory alloc for buffer struct failed\n");
break;
}
@@ -292,7 +292,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
v4l2_memory memory,
if (memory == V4L2_MEMORY_MMAP) {
ret = __vb2_buf_mem_alloc(vb);
if (ret) {
- dprintk(1, "Failed allocating memory for "
+ dprintk(1, "failed allocating memory for "
"buffer %d\n", buffer);
kfree(vb);
break;
@@ -304,7 +304,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
v4l2_memory memory,
*/
ret = call_vb_qop(vb, buf_init, vb);
if (ret) {
- dprintk(1, "Buffer %d %p initialization"
+ dprintk(1, "buffer %d %p initialization"
" failed\n", buffer, vb);
fail_vb_qop(vb, buf_init);
__vb2_buf_mem_free(vb);
@@ -320,7 +320,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
v4l2_memory memory,
if (memory == V4L2_MEMORY_MMAP)
__setup_offsets(q, buffer);
- dprintk(1, "Allocated %d buffers, %d plane(s) each\n",
+ dprintk(1, "allocated %d buffers, %d plane(s) each\n",
buffer, num_planes);
return buffer;
@@ -477,13 +477,13 @@ static int __verify_planes_array(struct vb2_buffer *vb,
const struct v4l2_buffer
/* Is memory for copying plane information present? */
if (NULL == b->m.planes) {
- dprintk(1, "Multi-planar buffer passed but "
+ dprintk(1, "multi-planar buffer passed but "
"planes array not provided\n");
return -EINVAL;
}
if (b->length < vb->num_planes || b->length > VIDEO_MAX_PLANES) {
- dprintk(1, "Incorrect planes array length, "
+ dprintk(1, "incorrect planes array length, "
"expected %d, got %d\n", vb->num_planes, b->length);
return -EINVAL;
}
@@ -847,7 +847,7 @@ static int __reqbufs(struct vb2_queue *q, struct
v4l2_requestbuffers *req)
/* Finally, allocate buffers and video memory */
allocated_buffers = __vb2_queue_alloc(q, req->memory, num_buffers,
num_planes);
if (allocated_buffers == 0) {
- dprintk(1, "Memory allocation failed\n");
+ dprintk(1, "memory allocation failed\n");
return -ENOMEM;
}
@@ -960,7 +960,7 @@ static int __create_bufs(struct vb2_queue *q, struct
v4l2_create_buffers *create
allocated_buffers = __vb2_queue_alloc(q, create->memory, num_buffers,
num_planes);
if (allocated_buffers == 0) {
- dprintk(1, "Memory allocation failed\n");
+ dprintk(1, "memo
[REVIEWv2 PATCH 03/13] vb2: if bytesused is 0, then fill with output buffer length
From: Hans Verkuil
The application should really always fill in bytesused for output
buffers, unfortunately the vb2 framework never checked for that.
So for single planar formats replace a bytesused of 0 by the length
of the buffer, and for multiplanar format do the same if bytesused is
0 for ALL planes.
This seems to be what the user really intended if v4l2_buffer was
just memset to 0.
I'm afraid that just checking for this and returning an error would
break too many applications. Quite a few drivers never check for bytesused
at all and just use the buffer length instead.
Signed-off-by: Hans Verkuil
Acked-by: Pawel Osciak
---
drivers/media/v4l2-core/videobuf2-core.c | 31 ++-
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 596998e..b2582cb 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1143,15 +1143,30 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
const struct v4l2_buffer *b
if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
/* Fill in driver-provided information for OUTPUT types */
if (V4L2_TYPE_IS_OUTPUT(b->type)) {
+ bool bytesused_is_used;
+
+ /* Check if bytesused == 0 for all planes */
+ for (plane = 0; plane < vb->num_planes; ++plane)
+ if (b->m.planes[plane].bytesused)
+ break;
+ bytesused_is_used = plane < vb->num_planes;
+
/*
* Will have to go up to b->length when API starts
* accepting variable number of planes.
+*
+* If bytesused_is_used is false, then fall back to the
+* full buffer size. In that case userspace clearly
+* never bothered to set it and it's a safe assumption
+* that they really meant to use the full plane sizes.
*/
for (plane = 0; plane < vb->num_planes; ++plane) {
- v4l2_planes[plane].bytesused =
- b->m.planes[plane].bytesused;
- v4l2_planes[plane].data_offset =
- b->m.planes[plane].data_offset;
+ struct v4l2_plane *pdst = &v4l2_planes[plane];
+ struct v4l2_plane *psrc = &b->m.planes[plane];
+
+ pdst->bytesused = bytesused_is_used ?
+ psrc->bytesused : psrc->length;
+ pdst->data_offset = psrc->data_offset;
}
}
@@ -1177,9 +1192,15 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
const struct v4l2_buffer *b
* so fill in relevant v4l2_buffer struct fields instead.
* In videobuf we use our internal V4l2_planes struct for
* single-planar buffers as well, for simplicity.
+*
+* If bytesused == 0, then fall back to the full buffer size
+* as that's a sensible default.
*/
if (V4L2_TYPE_IS_OUTPUT(b->type))
- v4l2_planes[0].bytesused = b->bytesused;
+ v4l2_planes[0].bytesused =
+ b->bytesused ? b->bytesused : b->length;
+ else
+ v4l2_planes[0].bytesused = 0;
if (b->memory == V4L2_MEMORY_USERPTR) {
v4l2_planes[0].m.userptr = b->m.userptr;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv2 PATCH 10/13] vb2: set v4l2_buffer.bytesused to 0 for mp buffers
From: Hans Verkuil
The bytesused field of struct v4l2_buffer is not used for multiplanar
formats, so just zero it to prevent it from having some random value.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/videobuf2-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 08152dd..ef7ef82 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -582,6 +582,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb,
struct v4l2_buffer *b)
* for it. The caller has already verified memory and size.
*/
b->length = vb->num_planes;
+ b->bytesused = 0;
memcpy(b->m.planes, vb->v4l2_planes,
b->length * sizeof(struct v4l2_plane));
} else {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv2 PATCH 09/13] vb2: add vb2_fileio_is_active and check it more often
From: Hans Verkuil
Added a vb2_fileio_is_active inline function that returns true if fileio
is in progress. Check for this too in mmap() (you don't want apps mmap()ing
buffers used by fileio) and expbuf() (same reason).
In addition drivers should be able to check for this in queue_setup() to
return an error if an attempt is made to read() or write() with
V4L2_FIELD_ALTERNATE being configured. This is illegal (there is no way
to pass the TOP/BOTTOM information around using file I/O).
However, in order to be able to check for this the init_fileio function
needs to set q->fileio early on, before the buffers are allocated. So switch
to using internal functions (__reqbufs, vb2_internal_qbuf and
vb2_internal_streamon) to skip the fileio check. Well, that's why the internal
functions were created...
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/videobuf2-core.c | 39
include/media/videobuf2-core.h | 17 ++
2 files changed, 41 insertions(+), 15 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 89147d2..08152dd 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -755,7 +755,7 @@ static int __verify_memory_type(struct vb2_queue *q,
* create_bufs is called with count == 0, but count == 0 should still
* do the memory and type validation.
*/
- if (q->fileio) {
+ if (vb2_fileio_is_active(q)) {
dprintk(1, "file io in progress\n");
return -EBUSY;
}
@@ -1617,7 +1617,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct
v4l2_buffer *b)
struct vb2_buffer *vb;
int ret;
- if (q->fileio) {
+ if (vb2_fileio_is_active(q)) {
dprintk(1, "file io in progress\n");
return -EBUSY;
}
@@ -1786,7 +1786,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct
v4l2_buffer *b)
*/
int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
{
- if (q->fileio) {
+ if (vb2_fileio_is_active(q)) {
dprintk(1, "file io in progress\n");
return -EBUSY;
}
@@ -2006,7 +2006,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct
v4l2_buffer *b, bool n
*/
int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
{
- if (q->fileio) {
+ if (vb2_fileio_is_active(q)) {
dprintk(1, "file io in progress\n");
return -EBUSY;
}
@@ -2136,7 +2136,7 @@ static int vb2_internal_streamon(struct vb2_queue *q,
enum v4l2_buf_type type)
*/
int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
{
- if (q->fileio) {
+ if (vb2_fileio_is_active(q)) {
dprintk(1, "file io in progress\n");
return -EBUSY;
}
@@ -2183,7 +2183,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q,
enum v4l2_buf_type type)
*/
int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
{
- if (q->fileio) {
+ if (vb2_fileio_is_active(q)) {
dprintk(1, "file io in progress\n");
return -EBUSY;
}
@@ -2268,6 +2268,11 @@ int vb2_expbuf(struct vb2_queue *q, struct
v4l2_exportbuffer *eb)
return -EINVAL;
}
+ if (vb2_fileio_is_active(q)) {
+ dprintk(1, "expbuf: file io in progress\n");
+ return -EBUSY;
+ }
+
vb_plane = &vb->planes[eb->plane];
dbuf = call_memop(vb, get_dmabuf, vb_plane->mem_priv, eb->flags &
O_ACCMODE);
@@ -2344,6 +2349,10 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct
*vma)
return -EINVAL;
}
}
+ if (vb2_fileio_is_active(q)) {
+ dprintk(1, "mmap: file io in progress\n");
+ return -EBUSY;
+ }
/*
* Find the plane corresponding to the offset passed by userspace.
@@ -2455,7 +2464,7 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file
*file, poll_table *wait)
/*
* Start file I/O emulator only if streaming API has not been used yet.
*/
- if (q->num_buffers == 0 && q->fileio == NULL) {
+ if (q->num_buffers == 0 && !vb2_fileio_is_active(q)) {
if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ) &&
(req_events & (POLLIN | POLLRDNORM))) {
if (__vb2_init_fileio(q, 1))
@@ -2660,7 +2669,8 @@ static int __vb2_init_fileio(struct vb2_queue *q, int
read)
fileio->req.count = count;
fileio->req.memory = V4L2_MEMORY_MMAP;
fileio->req.type = q->type;
- ret = vb2_reqbufs(q, &fileio->req);
+ q->fileio = fileio;
+ ret = __reqbufs(q, &fileio->req);
if (ret)
goto err_kfree;
@@ -2698,7 +2708,7 @@ static int __vb2_init_fileio(struct
[REVIEWv2 PATCH 06/13] vb2: set timestamp when using write()
From: Hans Verkuil
When using write() to write data to an output video node the vb2 core
should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody
else is able to provide this information with the write() operation.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/videobuf2-core.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 2e448a7..b7de6be 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
static int debug;
@@ -2751,6 +2752,9 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q,
char __user *data, size_
{
struct vb2_fileio_data *fileio;
struct vb2_fileio_buf *buf;
+ bool set_timestamp = !read &&
+ (q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) ==
+ V4L2_BUF_FLAG_TIMESTAMP_COPY;
int ret, index;
dprintk(3, "mode %s, offset %ld, count %zd, %sblocking\n",
@@ -2852,6 +2856,8 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q,
char __user *data, size_
fileio->b.memory = q->memory;
fileio->b.index = index;
fileio->b.bytesused = buf->pos;
+ if (set_timestamp)
+ v4l2_get_timestamp(&fileio->b.timestamp);
ret = vb2_internal_qbuf(q, &fileio->b);
dprintk(5, "vb2_dbuf result: %d\n", ret);
if (ret)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv2 PATCH 07/13] vb2: reject output buffers with V4L2_FIELD_ALTERNATE
From: Hans Verkuil
This is not allowed by the spec and does in fact not make any sense.
Return -EINVAL if this is the case.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/videobuf2-core.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index b7de6be..c662ad9 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1511,6 +1511,19 @@ static int __buf_prepare(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
dprintk(1, "plane parameters verification failed: %d\n", ret);
return ret;
}
+ if (b->field == V4L2_FIELD_ALTERNATE && V4L2_TYPE_IS_OUTPUT(q->type)) {
+ /*
+* If the format's field is ALTERNATE, then the buffer's field
+* should be either TOP or BOTTOM, not ALTERNATE since that
+* makes no sense. The driver has to know whether the
+* buffer represents a top or a bottom field in order to
+* program any DMA correctly. Using ALTERNATE is wrong, since
+* that just says that it is either a top or a bottom field,
+* but not which of the two it is.
+*/
+ dprintk(1, "the field is incorrectly set to ALTERNATE for an
output buffer\n");
+ return -EINVAL;
+ }
vb->state = VB2_BUF_STATE_PREPARING;
vb->v4l2_buf.timestamp.tv_sec = 0;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv2 PATCH 11/13] vb2: allow read/write as long as the format is single planar
From: Hans Verkuil
It was impossible to read() or write() a frame if the queue type was
multiplanar.
Even if the current format is single planar. Change this to just check whether
the number of planes is 1 or more.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/videobuf2-core.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index ef7ef82..d33c69b 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -2610,6 +2610,7 @@ struct vb2_fileio_buf {
*/
struct vb2_fileio_data {
struct v4l2_requestbuffers req;
+ struct v4l2_plane p;
struct v4l2_buffer b;
struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME];
unsigned int cur_index;
@@ -2700,13 +2701,21 @@ static int __vb2_init_fileio(struct vb2_queue *q, int
read)
* Read mode requires pre queuing of all buffers.
*/
if (read) {
+ bool is_multiplanar = V4L2_TYPE_IS_MULTIPLANAR(q->type);
+
/*
* Queue all buffers.
*/
for (i = 0; i < q->num_buffers; i++) {
struct v4l2_buffer *b = &fileio->b;
+
memset(b, 0, sizeof(*b));
b->type = q->type;
+ if (is_multiplanar) {
+ memset(&fileio->p, 0, sizeof(fileio->p));
+ b->m.planes = &fileio->p;
+ b->length = 1;
+ }
b->memory = q->memory;
b->index = i;
ret = vb2_internal_qbuf(q, b);
@@ -2774,6 +2783,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q,
char __user *data, size_
{
struct vb2_fileio_data *fileio;
struct vb2_fileio_buf *buf;
+ bool is_multiplanar = V4L2_TYPE_IS_MULTIPLANAR(q->type);
bool set_timestamp = !read &&
(q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) ==
V4L2_BUF_FLAG_TIMESTAMP_COPY;
@@ -2808,6 +2818,11 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q,
char __user *data, size_
memset(&fileio->b, 0, sizeof(fileio->b));
fileio->b.type = q->type;
fileio->b.memory = q->memory;
+ if (is_multiplanar) {
+ memset(&fileio->p, 0, sizeof(fileio->p));
+ fileio->b.m.planes = &fileio->p;
+ fileio->b.length = 1;
+ }
ret = vb2_internal_dqbuf(q, &fileio->b, nonblock);
dprintk(5, "vb2_dqbuf result: %d\n", ret);
if (ret)
@@ -2878,6 +2893,12 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q,
char __user *data, size_
fileio->b.memory = q->memory;
fileio->b.index = index;
fileio->b.bytesused = buf->pos;
+ if (is_multiplanar) {
+ memset(&fileio->p, 0, sizeof(fileio->p));
+ fileio->p.bytesused = buf->pos;
+ fileio->b.m.planes = &fileio->p;
+ fileio->b.length = 1;
+ }
if (set_timestamp)
v4l2_get_timestamp(&fileio->b.timestamp);
ret = vb2_internal_qbuf(q, &fileio->b);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv2 PATCH 00/13] vb2: various small fixes/improvements
This is the second version of this review patch series. The first can be found here: http://www.spinics.net/lists/linux-media/msg74220.html Changes since v2: - Rebased to latest master branch - Incorporated all suggestions/remarks from Pawel (patches 2, 4 and 7) - Added patch 12 (follow-up of patch 4) and 13 (suggested by Pawel) Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv2 PATCH 04/13] vb2: use correct prefix
From: Hans Verkuil
Many dprintk's in vb2 use a hardcoded prefix with the function name. In
many cases that is now outdated. To keep things consistent the dprintk
macro has been changed to print the function name in addition to the "vb2:"
prefix. Superfluous prefixes elsewhere in the code have been removed.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/videobuf2-core.c | 133 +++
1 file changed, 65 insertions(+), 68 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index b2582cb..1421075 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -27,10 +27,10 @@
static int debug;
module_param(debug, int, 0644);
-#define dprintk(level, fmt, arg...)\
- do {\
- if (debug >= level) \
- printk(KERN_DEBUG "vb2: " fmt, ## arg); \
+#define dprintk(level, fmt, arg...) \
+ do { \
+ if (debug >= level) \
+ pr_debug("vb2: %s: " fmt, __func__, ## arg); \
} while (0)
#ifdef CONFIG_VIDEO_ADV_DEBUG
@@ -371,7 +371,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned
int buffers)
if (q->bufs[buffer] == NULL)
continue;
if (q->bufs[buffer]->state == VB2_BUF_STATE_PREPARING) {
- dprintk(1, "reqbufs: preparing buffers, cannot free\n");
+ dprintk(1, "preparing buffers, cannot free\n");
return -EAGAIN;
}
}
@@ -656,12 +656,12 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer
*b)
int ret;
if (b->type != q->type) {
- dprintk(1, "querybuf: wrong buffer type\n");
+ dprintk(1, "wrong buffer type\n");
return -EINVAL;
}
if (b->index >= q->num_buffers) {
- dprintk(1, "querybuf: buffer index out of range\n");
+ dprintk(1, "buffer index out of range\n");
return -EINVAL;
}
vb = q->bufs[b->index];
@@ -721,12 +721,12 @@ static int __verify_memory_type(struct vb2_queue *q,
{
if (memory != V4L2_MEMORY_MMAP && memory != V4L2_MEMORY_USERPTR &&
memory != V4L2_MEMORY_DMABUF) {
- dprintk(1, "reqbufs: unsupported memory type\n");
+ dprintk(1, "unsupported memory type\n");
return -EINVAL;
}
if (type != q->type) {
- dprintk(1, "reqbufs: requested type is incorrect\n");
+ dprintk(1, "requested type is incorrect\n");
return -EINVAL;
}
@@ -735,17 +735,17 @@ static int __verify_memory_type(struct vb2_queue *q,
* are available.
*/
if (memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
- dprintk(1, "reqbufs: MMAP for current setup unsupported\n");
+ dprintk(1, "MMAP for current setup unsupported\n");
return -EINVAL;
}
if (memory == V4L2_MEMORY_USERPTR && __verify_userptr_ops(q)) {
- dprintk(1, "reqbufs: USERPTR for current setup unsupported\n");
+ dprintk(1, "USERPTR for current setup unsupported\n");
return -EINVAL;
}
if (memory == V4L2_MEMORY_DMABUF && __verify_dmabuf_ops(q)) {
- dprintk(1, "reqbufs: DMABUF for current setup unsupported\n");
+ dprintk(1, "DMABUF for current setup unsupported\n");
return -EINVAL;
}
@@ -755,7 +755,7 @@ static int __verify_memory_type(struct vb2_queue *q,
* do the memory and type validation.
*/
if (q->fileio) {
- dprintk(1, "reqbufs: file io in progress\n");
+ dprintk(1, "file io in progress\n");
return -EBUSY;
}
return 0;
@@ -790,7 +790,7 @@ static int __reqbufs(struct vb2_queue *q, struct
v4l2_requestbuffers *req)
int ret;
if (q->streaming) {
- dprintk(1, "reqbufs: streaming active\n");
+ dprintk(1, "streaming active\n");
return -EBUSY;
}
@@ -800,7 +800,7 @@ static int __reqbufs(struct vb2_queue *q, struct
v4l2_requestbuffers *req)
* are not in use and can be freed.
*/
if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) {
- dprintk(1, "reqbufs: memory in use, cannot free\n");
+ dprintk(1, "memory in use, cannot free\n");
return -EBUSY;
}
@@ -931,8 +931,7 @@ stat
[REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]
From: Hans Verkuil
The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
array in struct v4l2_plane would be non-zero, causing v4l2-compliance
errors.
More serious is the fact that data_offset was not handled correctly:
- for capture devices it was never zeroed, which meant that it was
uninitialized. Unless the driver sets it it was a completely random
number. With the memset above this is now fixed.
- __qbuf_dmabuf had a completely incorrect length check that included
data_offset.
- in __fill_vb2_buffer in the DMABUF case the data_offset field was
unconditionally copied from v4l2_buffer to v4l2_plane when this
should only happen in the output case.
- in the single-planar case data_offset was never correctly set to 0.
The single-planar API doesn't support data_offset, so setting it
to 0 is the right thing to do. This too is now solved by the memset.
All these issues were found with v4l2-compliance.
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/videobuf2-core.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index f9059bb..596998e 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1169,8 +1169,6 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
const struct v4l2_buffer *b
b->m.planes[plane].m.fd;
v4l2_planes[plane].length =
b->m.planes[plane].length;
- v4l2_planes[plane].data_offset =
- b->m.planes[plane].data_offset;
}
}
} else {
@@ -1180,10 +1178,8 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
const struct v4l2_buffer *b
* In videobuf we use our internal V4l2_planes struct for
* single-planar buffers as well, for simplicity.
*/
- if (V4L2_TYPE_IS_OUTPUT(b->type)) {
+ if (V4L2_TYPE_IS_OUTPUT(b->type))
v4l2_planes[0].bytesused = b->bytesused;
- v4l2_planes[0].data_offset = 0;
- }
if (b->memory == V4L2_MEMORY_USERPTR) {
v4l2_planes[0].m.userptr = b->m.userptr;
@@ -1193,9 +1189,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
const struct v4l2_buffer *b
if (b->memory == V4L2_MEMORY_DMABUF) {
v4l2_planes[0].m.fd = b->m.fd;
v4l2_planes[0].length = b->length;
- v4l2_planes[0].data_offset = 0;
}
-
}
/* Zero flags that the vb2 core handles */
@@ -1238,6 +1232,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
int write = !V4L2_TYPE_IS_OUTPUT(q->type);
bool reacquired = vb->planes[0].mem_priv == NULL;
+ memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
/* Copy relevant information provided by the userspace */
__fill_vb2_buffer(vb, b, planes);
@@ -1357,6 +1352,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
int write = !V4L2_TYPE_IS_OUTPUT(q->type);
bool reacquired = vb->planes[0].mem_priv == NULL;
+ memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
/* Copy relevant information provided by the userspace */
__fill_vb2_buffer(vb, b, planes);
@@ -1374,8 +1370,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
if (planes[plane].length == 0)
planes[plane].length = dbuf->size;
- if (planes[plane].length < planes[plane].data_offset +
- q->plane_sizes[plane]) {
+ if (planes[plane].length < q->plane_sizes[plane]) {
dprintk(1, "qbuf: invalid dmabuf length for plane %d\n",
plane);
ret = -EINVAL;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: videobuf2-vmalloc suspect for corrupted data
On 04/07/2014 01:20 PM, Divneil Wadhawan wrote: > Hi Hans, >> Two more questions: >> >> Which kernel version are you using? > 3.4.58 That should be new enough, I see no important differences between 3.4 and 3.14 in this respect. But really, 3.4? That's over two years old! If you have control over what kernel you use then I recommend you upgrade. >> Which capture driver are you using? > It's a TSMUX driver, written locally. I have not seen any reports of problems with vmalloc with arm in a long time. I know the uvc driver uses vmalloc, and that's used frequently. Question: if you use MEMORY_MMAP instead of USERPTR, does that work? Have you tried to stream with v4l2-ctl? It's available here: http://git.linuxtv.org/cgit.cgi/v4l-utils.git/. It's the reference implementation of how to stream, so if that fails as well, then at least its not your application. Testing whether you see the same when capturing from a usb uvc webcam (most webcams are uvc these days) would be useful as well. If it works with a uvc webcam, but not with your driver, then I suspect the driver. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12][ 12/12] ARM: imx_v6_v7_defconfig: Add more drm drivers.
The DRM_PANEL_SIMPLE is needed by the eukrea mbimxsd51's displays. Signed-off-by: Denis Carikli --- ChangeLog v10->v11: - New patch, splitting it would be overkill. --- arch/arm/configs/imx_v6_v7_defconfig |2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig index 09e9743..0316926 100644 --- a/arch/arm/configs/imx_v6_v7_defconfig +++ b/arch/arm/configs/imx_v6_v7_defconfig @@ -183,6 +183,7 @@ CONFIG_V4L_MEM2MEM_DRIVERS=y CONFIG_VIDEO_CODA=y CONFIG_SOC_CAMERA_OV2640=y CONFIG_DRM=y +CONFIG_DRM_PANEL_SIMPLE=y CONFIG_BACKLIGHT_LCD_SUPPORT=y CONFIG_LCD_CLASS_DEVICE=y CONFIG_LCD_L4F00242T03=y @@ -245,6 +246,7 @@ CONFIG_DRM_IMX_TVE=y CONFIG_DRM_IMX_LDB=y CONFIG_DRM_IMX_IPUV3_CORE=y CONFIG_DRM_IMX_IPUV3=y +CONFIG_DRM_IMX_HDMI=y CONFIG_COMMON_CLK_DEBUG=y # CONFIG_IOMMU_SUPPORT is not set CONFIG_PWM=y -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12][ 10/12] ARM: dts: mbimx51sd: Add display support.
The CMO-QVGA, DVI-SVGA and DVI-VGA are added.
Signed-off-by: Denis Carikli
---
ChangeLog v10->v11:
- Now uses the drm-panel instead of the display-timings.
This is to get regulator support, which is lacking in
the imx-drm driver when using the display-timings.
ChangeLog v9->v10:
- Rebased
- Now enables the cmo-qvga regulator at boot.
ChangeLog v8->v9:
- Removed the Cc. They are now set in git-send-email directly.
- updated pixelclk-active after the following patch:
"imx-drm: Match ipu_di_signal_cfg's clk_pol with its description."
ChangeLog v7->v8:
- Rebased the patch: added the now required imx-drm node.
- Adapted the svga clock-frequency value in order to still
be able to display an image after the following commit:
"imx-drm: ipu-v3: more inteligent DI clock selection"
ChangeLog v6->v7:
- Shrinked even more the Cc list.
- Since the pingrp headers were removed, the references
to it where replaced by the actual pins.
- Added the targets to arch/arm/boot/dts/Makefile
ChangeLog v5->v6:
- Reordered the Cc list.
ChangeLog v3->v5:
- Updated to new GPIO defines.
- Updated to new licenses checkpatch requirements.
- one whitespace cleanup.
ChangeLog v2->v3:
- Splitted out from the patch that added support for the cpuimx51/mbimxsd51
boards.
- This patch now only adds display support.
- Added some interested people in the Cc list, and removed some people that
might be annoyed by the receiving of that patch which is unrelated to their
subsystem.
- rebased and reworked the dts displays addition.
- Also rebased and reworked the fsl,pins usage.
---
arch/arm/boot/dts/Makefile |3 ++
.../imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts | 41
.../imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts | 28 +++
.../imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts | 28 +++
.../boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts | 49
5 files changed, 149 insertions(+)
create mode 100644
arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
create mode 100644
arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts
create mode 100644
arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 2145af6..4ac8aeb 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -166,6 +166,9 @@ dtb-$(CONFIG_ARCH_MXC) += \
imx51-apf51dev.dtb \
imx51-babbage.dtb \
imx51-eukrea-mbimxsd51-baseboard.dtb \
+ imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dtb \
+ imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dtb \
+ imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dtb \
imx53-ard.dtb \
imx53-m53evk.dtb \
imx53-mba53.dtb \
diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
new file mode 100644
index 000..7c280f8
--- /dev/null
+++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2013 Eukréa Electromatique
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include "imx51-eukrea-mbimxsd51-baseboard.dts"
+
+/ {
+ model = "Eukrea MBIMXSD51 with the CMO-QVGA Display";
+ compatible = "eukrea,mbimxsd51-baseboard-cmo-qvga",
"eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51";
+
+ panel: panel {
+ compatible = "eukrea,mbimxsd51-cmo-qvga", "simple-panel";
+ power-supply = <®_lcd_3v3>;
+ };
+
+ reg_lcd_3v3: lcd-en {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_reg_lcd_3v3>;
+ regulator-name = "lcd-3v3";
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ gpio = <&gpio3 13 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ regulator-boot-on;
+ };
+};
+
+&display {
+ status = "okay";
+ fsl,panel = <&panel>;
+};
diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts
b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts
new file mode 100644
index 000..323ebf4
--- /dev/null
+++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts
@@ -0,0 +1,28 @@
+/*
+ * Copyright 2013 Eukréa Electromatique
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify
[PATCH v12][ 11/12] ARM: dts: mbimx51sd: Add CMO-QVGA backlight support.
Signed-off-by: Denis Carikli
---
ChangeLog v9->v11:
- Now uses the drm-panel instead of the display-timings.
ChangeLog v8->v9:
- Removed the Cc. They are now set in git-send-email directly.
- The backlight is now on at boot.
ChangeLog v6->v7:
- Shrinked even more the Cc list.
ChangeLog v5->v6:
- Reordered the Cc list.
ChangeLog v3->v5:
- Updated to the new GPIO defines.
ChangeLog v2->v3:
- Splitted out from the patch that added support for the cpuimx51/mbimxsd51
boards.
- This patch now only adds backlight support.
- Added some interested people in the Cc list, and removed some people that
might be annoyed by the receiving of that patch which is unrelated to their
subsystem.
---
.../imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts | 10 ++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
index 7c280f8..4a3f805 100644
--- a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
+++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
@@ -17,9 +17,19 @@
model = "Eukrea MBIMXSD51 with the CMO-QVGA Display";
compatible = "eukrea,mbimxsd51-baseboard-cmo-qvga",
"eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51";
+ backlight: backlight {
+ compatible = "gpio-backlight";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_backlight_1>;
+ gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>;
+ default-brightness-level = <1>;
+ default-on;
+ };
+
panel: panel {
compatible = "eukrea,mbimxsd51-cmo-qvga", "simple-panel";
power-supply = <®_lcd_3v3>;
+ backlight = <&backlight>;
};
reg_lcd_3v3: lcd-en {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12][ 09/12] drm/panel: Add Eukrea mbimxsd51 displays.
Signed-off-by: Denis Carikli
---
ChangeLog v11->v12:
- Rebased: It now uses the new DRM_MODE_FLAG_POL_DE flags defines names
ChangeLog v10->v11:
- New patch.
---
.../bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt |7 ++
.../bindings/panel/eukrea,mbimxsd51-dvi-svga.txt |7 ++
.../bindings/panel/eukrea,mbimxsd51-dvi-vga.txt|7 ++
drivers/gpu/drm/panel/panel-simple.c | 81
4 files changed, 102 insertions(+)
create mode 100644
Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
create mode 100644
Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
create mode 100644
Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
diff --git
a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
new file mode 100644
index 000..03679d0
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
@@ -0,0 +1,7 @@
+Eukrea CMO-QVGA (320x240 pixels) TFT LCD panel
+
+Required properties:
+- compatible: should be "eukrea,mbimxsd51-cmo-qvga"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git
a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
new file mode 100644
index 000..f408c9a
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
@@ -0,0 +1,7 @@
+Eukrea DVI-SVGA (800x600 pixels) DVI output.
+
+Required properties:
+- compatible: should be "eukrea,mbimxsd51-dvi-svga"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git
a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
new file mode 100644
index 000..8ea90da
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
@@ -0,0 +1,7 @@
+Eukrea DVI-VGA (640x480 pixels) DVI output.
+
+Required properties:
+- compatible: should be "eukrea,mbimxsd51-dvi-vga"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 309f29e..45797d8 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -328,6 +328,78 @@ static const struct panel_desc chunghwa_claa101wb01 = {
},
};
+static const struct drm_display_mode eukrea_mbimxsd51_cmoqvga_mode = {
+ .clock = 6500,
+ .hdisplay = 320,
+ .hsync_start = 320 + 38,
+ .hsync_end = 320 + 38 + 20,
+ .htotal = 320 + 38 + 20 + 30,
+ .vdisplay = 240,
+ .vsync_start = 240 + 15,
+ .vsync_end = 240 + 15 + 4,
+ .vtotal = 240 + 15 + 4 + 3,
+ .vrefresh = 60,
+ .pol_flags = DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE |
+DRM_MODE_FLAG_POL_DE_LOW,
+};
+
+static const struct panel_desc eukrea_mbimxsd51_cmoqvga = {
+ .modes = &eukrea_mbimxsd51_cmoqvga_mode,
+ .num_modes = 1,
+ .size = {
+ .width = 73,
+ .height = 56,
+ },
+};
+
+static const struct drm_display_mode eukrea_mbimxsd51_dvisvga_mode = {
+ .clock = 44333,
+ .hdisplay = 800,
+ .hsync_start = 800 + 112,
+ .hsync_end = 800 + 112 + 32,
+ .htotal = 800 + 112 + 32 + 80,
+ .vdisplay = 600,
+ .vsync_start = 600 + 3,
+ .vsync_end = 600 + 3 + 17,
+ .vtotal = 600 + 3 + 17 + 4,
+ .vrefresh = 60,
+ .pol_flags = DRM_MODE_FLAG_POL_PIXDATA_POSEDGE |
+DRM_MODE_FLAG_POL_DE_HIGH,
+};
+
+static const struct panel_desc eukrea_mbimxsd51_dvisvga = {
+ .modes = &eukrea_mbimxsd51_dvisvga_mode,
+ .num_modes = 1,
+ .size = {
+ .width = 0,
+ .height = 0,
+ },
+};
+
+static const struct drm_display_mode eukrea_mbimxsd51_dvivga_mode = {
+ .clock = 23750,
+ .hdisplay = 640,
+ .hsync_start = 640 + 80,
+ .hsync_end = 640 + 80 + 16,
+ .htotal = 640 + 80 + 16 + 64,
+ .vdisplay = 480,
+ .vsync_start = 480 + 3,
+ .vsync_end = 480 + 3 + 13,
+ .vtotal = 480 + 3 + 13 + 4,
+ .vrefresh = 60,
+ .pol_flags = DRM_MODE_FLAG_POL_PIXDATA_POSEDGE |
+DRM_MODE_FLAG_POL_DE_HIGH,
+};
+
+static const struct panel_desc eukrea_mbimxsd51_dvivga = {
+ .modes = &eukrea_mbimxsd51_dvivga_mode,
+ .num_modes = 1,
+ .size = {
+ .width = 0,
+ .height = 0,
+ },
+};
+
static const struct drm_display_mode lg_lp129qe_mode = {
.clock = 285250,
.hdisplay = 2560,
@@ -380,6 +452,15 @@ static const struct of_device_
[PATCH v12][ 06/12] ARM: dts: imx5*, imx6*: correct display-timings nodes.
The imx-drm driver can't use the de-active and
pixelclk-active display-timings properties yet.
Instead the data-enable and the pixel data clock
polarity are hardcoded in the imx-drm driver.
So theses properties are now set to keep
the same behaviour when imx-drm will start
using them.
Signed-off-by: Denis Carikli
---
ChangeLog v9->v10:
- New patch that was splitted out of:
"staging imx-drm: Use de-active and pixelclk-active
display-timings."
---
arch/arm/boot/dts/imx51-babbage.dts |2 ++
arch/arm/boot/dts/imx53-m53evk.dts|2 ++
arch/arm/boot/dts/imx53-tx53-x03x.dts |2 +-
arch/arm/boot/dts/imx6qdl-gw53xx.dtsi |2 ++
arch/arm/boot/dts/imx6qdl-gw54xx.dtsi |2 ++
arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi |2 ++
arch/arm/boot/dts/imx6qdl-sabreauto.dtsi |2 ++
arch/arm/boot/dts/imx6qdl-sabrelite.dtsi |2 ++
arch/arm/boot/dts/imx6qdl-sabresd.dtsi|2 ++
9 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx51-babbage.dts
b/arch/arm/boot/dts/imx51-babbage.dts
index 2dda06b..91ef454 100644
--- a/arch/arm/boot/dts/imx51-babbage.dts
+++ b/arch/arm/boot/dts/imx51-babbage.dts
@@ -38,6 +38,8 @@
vfront-porch = <7>;
hsync-len = <60>;
vsync-len = <10>;
+ de-active = <1>;
+ pixelclk-active = <0>;
};
};
diff --git a/arch/arm/boot/dts/imx53-m53evk.dts
b/arch/arm/boot/dts/imx53-m53evk.dts
index 4b036b4..d03ced7 100644
--- a/arch/arm/boot/dts/imx53-m53evk.dts
+++ b/arch/arm/boot/dts/imx53-m53evk.dts
@@ -41,6 +41,8 @@
vfront-porch = <9>;
vsync-len = <3>;
vsync-active = <1>;
+ de-active = <1>;
+ pixelclk-active = <0>;
};
};
};
diff --git a/arch/arm/boot/dts/imx53-tx53-x03x.dts
b/arch/arm/boot/dts/imx53-tx53-x03x.dts
index 0217dde3..4092a81 100644
--- a/arch/arm/boot/dts/imx53-tx53-x03x.dts
+++ b/arch/arm/boot/dts/imx53-tx53-x03x.dts
@@ -93,7 +93,7 @@
hsync-active = <0>;
vsync-active = <0>;
de-active = <1>;
- pixelclk-active = <1>;
+ pixelclk-active = <0>;
};
ET0500 {
diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
index c8e5ae0..43f48f2 100644
--- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
@@ -494,6 +494,8 @@
vfront-porch = <7>;
hsync-len = <60>;
vsync-len = <10>;
+ de-active = <1>;
+ pixelclk-active = <0>;
};
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
index 2795dfc..59ecfd1 100644
--- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
@@ -516,6 +516,8 @@
vfront-porch = <7>;
hsync-len = <60>;
vsync-len = <10>;
+ de-active = <1>;
+ pixelclk-active = <0>;
};
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
index 99be301..e9419a2 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
@@ -349,6 +349,8 @@
vfront-porch = <7>;
hsync-len = <60>;
vsync-len = <10>;
+ de-active = <1>;
+ pixelclk-active = <0>;
};
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd6..230bbc6 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -405,6 +405,8 @@
vfront-porch = <7>;
hsync-len = <60>;
vsync-len = <10>;
+ de-active = <1>;
+ pixelclk-active = <0>;
};
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
b/arch/arm/boot/dts/imx6qdl-sa
[PATCH v12][ 04/12] imx-drm: Match ipu_di_signal_cfg's clk_pol with its description.
According to the datasheet, setting the di0_polarity_disp_clk field in the GENERAL di register sets the output clock polarity to active high. Signed-off-by: Denis Carikli --- ChangeLog v9->v10: - New patch that is now needed by the "staging: imx-drm: Use de-active and pixelclk-active" patch. --- drivers/staging/imx-drm/ipu-v3/ipu-di.c |2 +- drivers/staging/imx-drm/ipuv3-crtc.c|2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-di.c b/drivers/staging/imx-drm/ipu-v3/ipu-di.c index 82a9eba..849b3e1 100644 --- a/drivers/staging/imx-drm/ipu-v3/ipu-di.c +++ b/drivers/staging/imx-drm/ipu-v3/ipu-di.c @@ -595,7 +595,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig) } } - if (!sig->clk_pol) + if (sig->clk_pol) di_gen |= DI_GEN_POLARITY_DISP_CLK; ipu_di_write(di, di_gen, DI_GENERAL); diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c index c48f640..f2c9cd0 100644 --- a/drivers/staging/imx-drm/ipuv3-crtc.c +++ b/drivers/staging/imx-drm/ipuv3-crtc.c @@ -158,7 +158,7 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc, sig_cfg.Vsync_pol = 1; sig_cfg.enable_pol = 1; - sig_cfg.clk_pol = 1; + sig_cfg.clk_pol = 0; sig_cfg.width = mode->hdisplay; sig_cfg.height = mode->vdisplay; sig_cfg.pixel_fmt = out_pixel_fmt; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12][ 08/12] imx-drm: Use drm_display_mode timings flags.
The previous hardware behaviour was kept if the
flags are not set.
Signed-off-by: Denis Carikli
---
ChangeLog v11->v12:
- Rebased: It now uses the following new flags defines names:
CLK_POL, ENABLE_POL
- The inversions in ipuv3-crtc.c are now fixed.
- ipuv3-crtc.c was still using mode->private_flags
from the previous versions of this patchset, that's now fixed.
ChangeLog v10->v11:
- This patch was splitted-out and adapted from:
"Prepare imx-drm for extra display-timings retrival."
- The display-timings dt specific part was removed.
- The flags names were changed to use the DRM ones from:
"drm: drm_display_mode: add signal polarity flags"
---
drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h |6 --
drivers/staging/imx-drm/ipu-v3/ipu-di.c |7 ++-
drivers/staging/imx-drm/ipuv3-crtc.c| 20 ++--
3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
index eba8893..c934394 100644
--- a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
+++ b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
@@ -29,9 +29,11 @@ enum ipuv3_type {
#define CLK_POL_NEGEDGE0
#define CLK_POL_POSEDGE1
+#define CLK_POL_PRESERVE 2
#define ENABLE_POL_LOW 0
#define ENABLE_POL_HIGH1
+#define ENABLE_POL_PRESERVE2
/*
* Bitfield of Display Interface signal polarities.
@@ -43,10 +45,10 @@ struct ipu_di_signal_cfg {
unsigned clksel_en:1;
unsigned clkidle_en:1;
unsigned data_pol:1;/* true = inverted */
- unsigned clk_pol:1;
- unsigned enable_pol:1;
unsigned Hsync_pol:1; /* true = active high */
unsigned Vsync_pol:1;
+ u8 clk_pol;
+ u8 enable_pol;
u16 width;
u16 height;
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-di.c
b/drivers/staging/imx-drm/ipu-v3/ipu-di.c
index 0ce3f52..c00b0ba 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-di.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-di.c
@@ -597,6 +597,8 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct
ipu_di_signal_cfg *sig)
if (sig->clk_pol == CLK_POL_POSEDGE)
di_gen |= DI_GEN_POLARITY_DISP_CLK;
+ else if (sig->clk_pol == CLK_POL_NEGEDGE)
+ di_gen &= ~DI_GEN_POLARITY_DISP_CLK;
ipu_di_write(di, di_gen, DI_GENERAL);
@@ -604,10 +606,13 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct
ipu_di_signal_cfg *sig)
DI_SYNC_AS_GEN);
reg = ipu_di_read(di, DI_POL);
- reg &= ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15);
+ reg &= ~(DI_POL_DRDY_DATA_POLARITY);
if (sig->enable_pol == ENABLE_POL_HIGH)
reg |= DI_POL_DRDY_POLARITY_15;
+ else if (sig->enable_pol == ENABLE_POL_LOW)
+ reg &= ~DI_POL_DRDY_POLARITY_15;
+
if (sig->data_pol)
reg |= DI_POL_DRDY_DATA_POLARITY;
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c
b/drivers/staging/imx-drm/ipuv3-crtc.c
index 9ba089c..b59b745 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -157,8 +157,24 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
if (mode->flags & DRM_MODE_FLAG_PVSYNC)
sig_cfg.Vsync_pol = 1;
- sig_cfg.enable_pol = ENABLE_POL_HIGH;
- sig_cfg.clk_pol = CLK_POL_NEGEDGE;
+ if (mode->pol_flags & DRM_MODE_FLAG_POL_PIXDATA_POSEDGE)
+ sig_cfg.clk_pol = CLK_POL_POSEDGE;
+ else if (mode->pol_flags & DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE)
+ sig_cfg.clk_pol = CLK_POL_NEGEDGE;
+ else if (mode->pol_flags & DRM_MODE_FLAG_POL_PIXDATA_PRESERVE)
+ sig_cfg.clk_pol = CLK_POL_PRESERVE;
+ else
+ sig_cfg.clk_pol = CLK_POL_NEGEDGE;
+
+ if (mode->pol_flags & DRM_MODE_FLAG_POL_DE_HIGH)
+ sig_cfg.enable_pol = ENABLE_POL_HIGH;
+ else if (mode->pol_flags & DRM_MODE_FLAG_POL_DE_LOW)
+ sig_cfg.enable_pol = ENABLE_POL_LOW;
+ else if (mode->pol_flags & DRM_MODE_FLAG_POL_DE_PRESERVE)
+ sig_cfg.enable_pol = ENABLE_POL_PRESERVE;
+ else
+ sig_cfg.enable_pol = ENABLE_POL_HIGH;
+
sig_cfg.width = mode->hdisplay;
sig_cfg.height = mode->vdisplay;
sig_cfg.pixel_fmt = out_pixel_fmt;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12][ 07/12] drm: drm_display_mode: add signal polarity flags
We need a way to pass signal polarity informations
between DRM panels, and the display drivers.
To do that, a pol_flags field was added to drm_display_mode.
Signed-off-by: Denis Carikli
---
ChangeLog v11->v12:
- Rebased: This patch now applies against drm_modes.h
- Rebased: It now uses the new DRM_MODE_FLAG_POL_DE flags defines names
ChangeLog v10->v11:
- Since the imx-drm won't be able to retrive its regulators
from the device tree when using display-timings nodes,
and that I was told that the drm simple-panel driver
already supported that, I then, instead, added what was
lacking to make the eukrea displays work with the
drm-simple-panel driver.
That required a way to get back the display polarity
informations from the imx-drm driver without affecting
userspace.
---
include/drm/drm_modes.h |8
1 file changed, 8 insertions(+)
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 2dbbf99..b3789e2 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -93,6 +93,13 @@ enum drm_mode_status {
#define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF
+#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE BIT(1)
+#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGE BIT(2)
+#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE BIT(3)
+#define DRM_MODE_FLAG_POL_DE_LOW BIT(4)
+#define DRM_MODE_FLAG_POL_DE_HIGH BIT(5)
+#define DRM_MODE_FLAG_POL_DE_PRESERVE BIT(6)
+
struct drm_display_mode {
/* Header */
struct list_head head;
@@ -144,6 +151,7 @@ struct drm_display_mode {
int vrefresh; /* in Hz */
int hsync; /* in kHz */
enum hdmi_picture_aspect picture_aspect_ratio;
+ unsigned int pol_flags;
};
/* mode specified on the command line */
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12][ 05/12] imx-drm: use defines for clock polarity settings
Signed-off-by: Denis Carikli
---
ChangeLog 11->v12:
- Improved the define names to match the hardware:
ENABLE_POL is not a clock signal but instead an enable signal.
ChangeLog v9->v10:
- New patch which was splitted out from:
"staging: imx-drm: Use de-active and pixelclk-active display-timings.".
- Fixes many issues in "staging: imx-drm: Use de-active and pixelclk-active
display-timings.":
- More clear meaning of the polarity settings.
- The SET_CLK_POL and SET_DE_POL masks are not
needed anymore.
---
drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h |8 +++-
drivers/staging/imx-drm/ipu-v3/ipu-di.c |4 ++--
drivers/staging/imx-drm/ipuv3-crtc.c|4 ++--
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
index c4d14ea..eba8893 100644
--- a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
+++ b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
@@ -27,6 +27,12 @@ enum ipuv3_type {
#define IPU_PIX_FMT_GBR24 v4l2_fourcc('G', 'B', 'R', '3')
+#define CLK_POL_NEGEDGE0
+#define CLK_POL_POSEDGE1
+
+#define ENABLE_POL_LOW 0
+#define ENABLE_POL_HIGH1
+
/*
* Bitfield of Display Interface signal polarities.
*/
@@ -37,7 +43,7 @@ struct ipu_di_signal_cfg {
unsigned clksel_en:1;
unsigned clkidle_en:1;
unsigned data_pol:1;/* true = inverted */
- unsigned clk_pol:1; /* true = rising edge */
+ unsigned clk_pol:1;
unsigned enable_pol:1;
unsigned Hsync_pol:1; /* true = active high */
unsigned Vsync_pol:1;
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-di.c
b/drivers/staging/imx-drm/ipu-v3/ipu-di.c
index 849b3e1..0ce3f52 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-di.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-di.c
@@ -595,7 +595,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct
ipu_di_signal_cfg *sig)
}
}
- if (sig->clk_pol)
+ if (sig->clk_pol == CLK_POL_POSEDGE)
di_gen |= DI_GEN_POLARITY_DISP_CLK;
ipu_di_write(di, di_gen, DI_GENERAL);
@@ -606,7 +606,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct
ipu_di_signal_cfg *sig)
reg = ipu_di_read(di, DI_POL);
reg &= ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15);
- if (sig->enable_pol)
+ if (sig->enable_pol == ENABLE_POL_HIGH)
reg |= DI_POL_DRDY_POLARITY_15;
if (sig->data_pol)
reg |= DI_POL_DRDY_DATA_POLARITY;
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c
b/drivers/staging/imx-drm/ipuv3-crtc.c
index f2c9cd0..9ba089c 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -157,8 +157,8 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
if (mode->flags & DRM_MODE_FLAG_PVSYNC)
sig_cfg.Vsync_pol = 1;
- sig_cfg.enable_pol = 1;
- sig_cfg.clk_pol = 0;
+ sig_cfg.enable_pol = ENABLE_POL_HIGH;
+ sig_cfg.clk_pol = CLK_POL_NEGEDGE;
sig_cfg.width = mode->hdisplay;
sig_cfg.height = mode->vdisplay;
sig_cfg.pixel_fmt = out_pixel_fmt;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12][ 03/12] imx-drm: Correct BGR666 and the board's dts that use them.
The current BGR666 is not consistent with the other color mapings like BGR24.
BGR666 should be in the same byte order than BGR24.
Signed-off-by: Denis Carikli
Acked-by: Philipp Zabel
---
ChangeLog v9->v10:
- Rebased.
- Added Philipp Zabel's Ack.
- Included Lothar Waßmann's suggestion about imx-ldb.c.
- Shortened the patch title
ChangeLog v8->v9:
- Removed the Cc. They are now set in git-send-email directly.
ChangeLog v7->v8:
- Shrinked even more the Cc list.
ChangeLog v6->v7:
- Shrinked even more the Cc list.
ChangeLog v5->v6:
- Remove people not concerned by this patch from the Cc list.
- Added a better explanation of the change.
ChangeLog v5:
- New patch.
---
arch/arm/boot/dts/imx51-apf51dev.dts|2 +-
arch/arm/boot/dts/imx53-m53evk.dts |2 +-
drivers/staging/imx-drm/imx-ldb.c |4 ++--
drivers/staging/imx-drm/ipu-v3/ipu-dc.c |4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/imx51-apf51dev.dts
b/arch/arm/boot/dts/imx51-apf51dev.dts
index c5a9a24..7b3851d 100644
--- a/arch/arm/boot/dts/imx51-apf51dev.dts
+++ b/arch/arm/boot/dts/imx51-apf51dev.dts
@@ -18,7 +18,7 @@
display@di1 {
compatible = "fsl,imx-parallel-display";
- interface-pix-fmt = "bgr666";
+ interface-pix-fmt = "rgb666";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_ipu_disp1>;
diff --git a/arch/arm/boot/dts/imx53-m53evk.dts
b/arch/arm/boot/dts/imx53-m53evk.dts
index d5d146a..4b036b4 100644
--- a/arch/arm/boot/dts/imx53-m53evk.dts
+++ b/arch/arm/boot/dts/imx53-m53evk.dts
@@ -24,7 +24,7 @@
soc {
display1: display@di1 {
compatible = "fsl,imx-parallel-display";
- interface-pix-fmt = "bgr666";
+ interface-pix-fmt = "rgb666";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_ipu_disp1>;
diff --git a/drivers/staging/imx-drm/imx-ldb.c
b/drivers/staging/imx-drm/imx-ldb.c
index fe4c1ef..9141293 100644
--- a/drivers/staging/imx-drm/imx-ldb.c
+++ b/drivers/staging/imx-drm/imx-ldb.c
@@ -188,11 +188,11 @@ static void imx_ldb_encoder_prepare(struct drm_encoder
*encoder)
switch (imx_ldb_ch->chno) {
case 0:
pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
- V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
+ V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_RGB666;
break;
case 1:
pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH1_24) ?
- V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
+ V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_RGB666;
break;
default:
dev_err(ldb->dev, "unable to config di%d panel format\n",
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
index 6f9abe8..154d293 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
@@ -397,9 +397,9 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
/* bgr666 */
ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666);
- ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 5, 0xfc); /* blue */
+ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 17, 0xfc); /* blue */
ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /* green */
- ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 17, 0xfc); /* red */
+ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 5, 0xfc); /* red */
/* bgr24 */
ipu_dc_map_clear(priv, IPU_DC_MAP_BGR24);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12][ 02/12] imx-drm: Add RGB666 support for parallel display.
Signed-off-by: Denis Carikli
Acked-by: Philipp Zabel
---
ChangeLog v8->v9:
- Rebased.
- Added Philipp Zabel's ack.
- Shortened the patch title.
ChangeLog v8->v9:
- Removed the Cc. They are now set in git-send-email directly.
- Rebased.
ChangeLog v7->v8:
- Shrinked even more the Cc list.
ChangeLog v6->v7:
- Shrinked even more the Cc list.
ChangeLog v5->v6:
- Remove people not concerned by this patch from the Cc list.
ChangeLog v3->v5:
- Use the correct RGB order.
ChangeLog v2->v3:
- Added some interested people in the Cc list.
- Removed the commit message long desciption that was just a copy of the short
description.
- Rebased the patch.
- Fixed a copy-paste error in the ipu_dc_map_clear parameter.
---
.../bindings/staging/imx-drm/fsl-imx-drm.txt |3 ++-
drivers/staging/imx-drm/ipu-v3/ipu-dc.c|9 +
drivers/staging/imx-drm/parallel-display.c |2 ++
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
index 3be5ce7..83137ef 100644
--- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
+++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
@@ -60,7 +60,8 @@ Required properties:
- compatible: Should be "fsl,imx-parallel-display"
Optional properties:
- interface_pix_fmt: How this display is connected to the
- display interface. Currently supported types: "rgb24", "rgb565", "bgr666"
+ display interface. Currently supported types: "rgb24", "rgb565", "bgr666",
+ "rgb666"
- edid: verbatim EDID data block describing attached display.
- ddc: phandle describing the i2c bus handling the display data
channel
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
index d5de8bb..6f9abe8 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
@@ -92,6 +92,7 @@ enum ipu_dc_map {
IPU_DC_MAP_GBR24, /* TVEv2 */
IPU_DC_MAP_BGR666,
IPU_DC_MAP_BGR24,
+ IPU_DC_MAP_RGB666,
};
struct ipu_dc {
@@ -155,6 +156,8 @@ static int ipu_pixfmt_to_map(u32 fmt)
return IPU_DC_MAP_BGR666;
case V4L2_PIX_FMT_BGR24:
return IPU_DC_MAP_BGR24;
+ case V4L2_PIX_FMT_RGB666:
+ return IPU_DC_MAP_RGB666;
default:
return -EINVAL;
}
@@ -404,6 +407,12 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 1, 15, 0xff); /* green */
ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 0, 23, 0xff); /* blue */
+ /* rgb666 */
+ ipu_dc_map_clear(priv, IPU_DC_MAP_RGB666);
+ ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 0, 5, 0xfc); /* blue */
+ ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */
+ ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */
+
return 0;
}
diff --git a/drivers/staging/imx-drm/parallel-display.c
b/drivers/staging/imx-drm/parallel-display.c
index c60b6c6..01b7ce5 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -219,6 +219,8 @@ static int imx_pd_bind(struct device *dev, struct device
*master, void *data)
imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB565;
else if (!strcmp(fmt, "bgr666"))
imxpd->interface_pix_fmt = V4L2_PIX_FMT_BGR666;
+ else if (!strcmp(fmt, "rgb666"))
+ imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB666;
}
panel_node = of_parse_phandle(np, "fsl,panel", 0);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12][ 01/12] [media] v4l2: add new V4L2_PIX_FMT_RGB666 pixel format.
That new macro is needed by the imx_drm staging driver
for supporting the QVGA display of the eukrea-cpuimx51 board.
Signed-off-by: Denis Carikli
Acked-by: Mauro Carvalho Chehab
Acked-by: Laurent Pinchart
Acked-by: Philipp Zabel
---
ChangeLog v9->v10:
- Rebased on top of:
"211e7f2 [media] DocBook media: drop the old incorrect packed RGB table"
- Added Philipp Zabel's Ack.
ChangeLog v8->v9:
- Removed the Cc. They are now set in git-send-email directly.
ChangeLog v7->v8:
- Added Mauro Carvalho Chehab back to the list of Cc
ChangeLog v6->v7:
- Shrinked even more the Cc list.
ChangeLog v5->v6:
- Remove people not concerned by this patch from the Cc list.
ChangeLog v3->v4:
- Added Laurent Pinchart's Ack.
ChangeLog v2->v3:
- Added some interested people in the Cc list.
- Added Mauro Carvalho Chehab's Ack.
- Added documentation.
---
.../DocBook/media/v4l/pixfmt-packed-rgb.xml| 39
include/uapi/linux/videodev2.h |1 +
2 files changed, 40 insertions(+)
diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
index e1c4f8b..88a7fe1 100644
--- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
@@ -279,6 +279,45 @@ colorspace
V4L2_COLORSPACE_SRGB.
+
+ V4L2_PIX_FMT_RGB666
+ 'RGBH'
+
+ r5
+ r4
+ r3
+ r2
+ r1
+ r0
+ g5
+ g4
+
+ g3
+ g2
+ g1
+ g0
+ b5
+ b4
+ b3
+ b2
+
+ b1
+ b0
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
V4L2_PIX_FMT_BGR24
'BGR3'
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index ea468ee..d5d818a 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -299,6 +299,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_RGB555X v4l2_fourcc('R', 'G', 'B', 'Q') /* 16 RGB-5-5-5
BE */
#define V4L2_PIX_FMT_RGB565X v4l2_fourcc('R', 'G', 'B', 'R') /* 16 RGB-5-6-5
BE */
#define V4L2_PIX_FMT_BGR666 v4l2_fourcc('B', 'G', 'R', 'H') /* 18 BGR-6-6-6
*/
+#define V4L2_PIX_FMT_RGB666 v4l2_fourcc('R', 'G', 'B', 'H') /* 18 RGB-6-6-6
*/
#define V4L2_PIX_FMT_BGR24 v4l2_fourcc('B', 'G', 'R', '3') /* 24 BGR-8-8-8
*/
#define V4L2_PIX_FMT_RGB24 v4l2_fourcc('R', 'G', 'B', '3') /* 24 RGB-8-8-8
*/
#define V4L2_PIX_FMT_BGR32 v4l2_fourcc('B', 'G', 'R', '4') /* 32
BGR-8-8-8-8 */
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH 02/11] vb2: fix handling of data_offset and v4l2_plane.reserved[]
On 04/07/2014 07:11 AM, Pawel Osciak wrote:
> On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil wrote:
>> From: Hans Verkuil
>>
>> The videobuf2-core did not zero the reserved array of v4l2_plane as it
>> should.
>>
>> More serious is the fact that data_offset was not handled correctly:
>>
>> - for capture devices it was never zeroed, which meant that it was
>> uninitialized. Unless the driver sets it it was a completely random
>> number.
>>
>> - __qbuf_dmabuf had a completely incorrect length check that included
>> data_offset.
>>
>> - in the single-planar case data_offset was never correctly set to 0.
>> The single-planar API doesn't support data_offset, so setting it
>> to 0 is the right thing to do.
>>
>> All these issues were found with v4l2-compliance.
>>
>> Signed-off-by: Hans Verkuil
>> ---
>> drivers/media/v4l2-core/videobuf2-core.c | 19 ++-
>> 1 file changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index f9059bb..1a09442 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -1141,6 +1141,12 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
>> const struct v4l2_buffer *b
>> unsigned int plane;
>>
>> if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
>> + for (plane = 0; plane < vb->num_planes; ++plane) {
>> + memset(v4l2_planes[plane].reserved, 0,
>> + sizeof(v4l2_planes[plane].reserved));
>> + v4l2_planes[plane].data_offset = 0;
>> + }
>> +
>
> Perhaps we should just memset the whole v4l2_planes array to 0 over
> all elements (ARRAY_SIZE)?
You can't do that here since in the mmap case the v4l2_planes array has already
fields filled in. However, by doing a memset in __qbuf_userptr and
__qbuf_dmabuf,
before calling __fill_vb2_buffer, this can be solved neatly and correctly.
> Also I would extract this above the if and zero out everything for
> both multi and singleplanar.
> You shouldn't need to zero it out below then.
>
>> /* Fill in driver-provided information for OUTPUT types */
>> if (V4L2_TYPE_IS_OUTPUT(b->type)) {
>> /*
>> @@ -1169,8 +1175,6 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
>> const struct v4l2_buffer *b
>> b->m.planes[plane].m.fd;
>> v4l2_planes[plane].length =
>> b->m.planes[plane].length;
>> - v4l2_planes[plane].data_offset =
>> - b->m.planes[plane].data_offset;
I've added an explicit explanation for this change to the commit log as
well:
- in __fill_vb2_buffer in the DMABUF case the data_offset field was
unconditionally copied from v4l2_buffer to v4l2_plane when this
should only happen in the output case.
Regards,
Hans
>> }
>> }
>> } else {
>> @@ -1180,10 +1184,10 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
>> const struct v4l2_buffer *b
>> * In videobuf we use our internal V4l2_planes struct for
>> * single-planar buffers as well, for simplicity.
>> */
>> - if (V4L2_TYPE_IS_OUTPUT(b->type)) {
>> + if (V4L2_TYPE_IS_OUTPUT(b->type))
>> v4l2_planes[0].bytesused = b->bytesused;
>> - v4l2_planes[0].data_offset = 0;
>> - }
>> + /* Single-planar buffers never use data_offset */
>> + v4l2_planes[0].data_offset = 0;
>>
>> if (b->memory == V4L2_MEMORY_USERPTR) {
>> v4l2_planes[0].m.userptr = b->m.userptr;
>> @@ -1193,9 +1197,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
>> const struct v4l2_buffer *b
>> if (b->memory == V4L2_MEMORY_DMABUF) {
>> v4l2_planes[0].m.fd = b->m.fd;
>> v4l2_planes[0].length = b->length;
>> - v4l2_planes[0].data_offset = 0;
>> }
>> -
>> }
>>
>> /* Zero flags that the vb2 core handles */
>> @@ -1374,8 +1376,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const
>> struct v4l2_buffer *b)
>> if (planes[plane].length == 0)
>> planes[plane].length = dbuf->size;
>>
>> - if (planes[plane].length < planes[plane].data_offset +
>> - q->plane_sizes[plane]) {
>> + if (planes[plane].length < q->plane_sizes[plane]) {
>
> Good catch!
>
>> dprintk(1, "qbuf: invalid dmabuf length for plane
>> %d\n",
>> plane);
>> ret = -EINVAL;
>> --
>> 1.9.0
>>
>
>
>
RE: videobuf2-vmalloc suspect for corrupted data
>> Which capture driver are you using? > It's a TSMUX driver, written locally. In complete, it's a Multi-INPUT, single output (MUXER) driver, but, currently, it's the capture side fault here. Regards, Divneil -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: videobuf2-vmalloc suspect for corrupted data
Hi Hans, > Two more questions: > > Which kernel version are you using? 3.4.58 > Which capture driver are you using? It's a TSMUX driver, written locally. Regards, Divneil -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: videobuf2-vmalloc suspect for corrupted data
On 04/07/2014 12:49 PM, Divneil Wadhawan wrote: > Hi Pawel, > > Thanks for the quick response. > >> Is it possible that your userspace is not always queuing the same >> userptr memory areas with the same v4l2_buffer index values? > No, userptr is always consistent with the index. > In fact, when we dump the captured buffer (Transport Stream) in this case, > kernel space data and user-space are different. > When that TS is played, macroblocks are observed from user-space and not from > the kernel space dump. > Although, user-space bad data is random, but, I have never seen kernel space > dumped TS as bad. > >> In other words, if you have 2 buffers in use, under userspace mapping >> at addr1 and addr2, if you queue addr1 with index=0 and addr2 with >> index=1 initially, >> you should always keep queuing addr1 with index=0 and never 1, etc. > Yeah! this is the same rule which is being followed. > >> Also, what architecture are you running this on? > ARM Cortex A9 SMP Two more questions: Which kernel version are you using? Which capture driver are you using? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: videobuf2-vmalloc suspect for corrupted data
Hi Pawel, Thanks for the quick response. > Is it possible that your userspace is not always queuing the same > userptr memory areas with the same v4l2_buffer index values? No, userptr is always consistent with the index. In fact, when we dump the captured buffer (Transport Stream) in this case, kernel space data and user-space are different. When that TS is played, macroblocks are observed from user-space and not from the kernel space dump. Although, user-space bad data is random, but, I have never seen kernel space dumped TS as bad. > In other words, if you have 2 buffers in use, under userspace mapping > at addr1 and addr2, if you queue addr1 with index=0 and addr2 with > index=1 initially, > you should always keep queuing addr1 with index=0 and never 1, etc. Yeah! this is the same rule which is being followed. > Also, what architecture are you running this on? ARM Cortex A9 SMP Regards, Divneil -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: videobuf2-vmalloc suspect for corrupted data
Hi Divneil,
On Mon, Apr 7, 2014 at 6:56 PM, Divneil Wadhawan wrote:
> Hi,
>
> I have a V4L2 capture driver accepting a malloc'ed buffer.
> The driver is using vb2_vmalloc_memops
> (../drivers/media/v4l2-core/videobuf2-vmalloc.c) for user-space to kvaddr
> translation.
> Randomly, corrupted data is received by user-app.
>
> So, the question is regarding the handling of get_userptr, put_userptr by
> v4l2-core:
>
> const struct vb2_mem_ops vb2_vmalloc_memops = {
>
> .get_userptr= vb2_vmalloc_get_userptr, (get_user_pages() and
> vm_map_ram())
> .put_userptr= vb2_vmalloc_put_userptr, (set_page_dirty_lock()
> and put_page())
> .
> };
>
> The driver prepares for the transaction by virtue of v4l2-core calling
> get_userptr (QBUF)
> After data is filled, driver puts on a done list (DQBUF)
>
> We never mark the pages as dirty (or put_userptr) after a transaction is
> complete.
> Here, in v4l2 core (videobuf2-core.c) , we conditionally put_userptr - when a
> QBUF with a different userptr on same index, or releasing buffers.
>
> Is it correct? Probably seems to be the reason for corrupted data.
This is an optimization and requires the mapping of userspace buffer
to v4l2_buffer index to not change.
Is it possible that your userspace is not always queuing the same
userptr memory areas with the same v4l2_buffer index values?
In other words, if you have 2 buffers in use, under userspace mapping
at addr1 and addr2, if you queue addr1 with index=0 and addr2 with
index=1 initially,
you should always keep queuing addr1 with index=0 and never 1, etc.
Also, what architecture are you running this on?
> Regards,
> Divneil --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best regards,
Pawel Osciak
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
videobuf2-vmalloc suspect for corrupted data
Hi,
I have a V4L2 capture driver accepting a malloc'ed buffer.
The driver is using vb2_vmalloc_memops
(../drivers/media/v4l2-core/videobuf2-vmalloc.c) for user-space to kvaddr
translation.
Randomly, corrupted data is received by user-app.
So, the question is regarding the handling of get_userptr, put_userptr by
v4l2-core:
const struct vb2_mem_ops vb2_vmalloc_memops = {
.get_userptr= vb2_vmalloc_get_userptr, (get_user_pages() and
vm_map_ram())
.put_userptr= vb2_vmalloc_put_userptr, (set_page_dirty_lock() and
put_page())
.
};
The driver prepares for the transaction by virtue of v4l2-core calling
get_userptr (QBUF)
After data is filled, driver puts on a done list (DQBUF)
We never mark the pages as dirty (or put_userptr) after a transaction is
complete.
Here, in v4l2 core (videobuf2-core.c) , we conditionally put_userptr - when a
QBUF with a different userptr on same index, or releasing buffers.
Is it correct? Probably seems to be the reason for corrupted data.
Regards,
Divneil --
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL for 3.15 fixes] VPE fixes
Hi Kamil, Since the VPE m2m patch set couldn't make it on time, I've separated out the fixes from the series so that they can be taken in one of the 3.15-rc series. Thanks, Archit The following changes since commit a83b93a7480441a47856dc9104bea970e84cda87: [media] em28xx-dvb: fix PCTV 461e tuner I2C binding (2014-03-31 08:02:16 -0300) are available in the git repository at: git://github.com/boddob/linux.git vpe-fixes-315-rc for you to fetch changes up to 3c7d629f0aa98ed587306913831e5a8968504f7a: v4l: ti-vpe: retain v4l2_buffer flags for captured buffers (2014-04-07 12:56:47 +0530) Archit Taneja (9): v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs v4l: ti-vpe: Use video_device_release_empty v4l: ti-vpe: Allow usage of smaller images v4l: ti-vpe: report correct capabilities in querycap v4l: ti-vpe: Use correct bus_info name for the device in querycap v4l: ti-vpe: Fix initial configuration queue data v4l: ti-vpe: zero out reserved fields in try_fmt v4l: ti-vpe: Set correct field parameter for output and capture buffers v4l: ti-vpe: retain v4l2_buffer flags for captured buffers drivers/media/platform/ti-vpe/vpe.c | 45 ++--- 1 file changed, 32 insertions(+), 13 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH 06/11] vb2: set timestamp when using write()
On 04/07/2014 10:32 AM, Pawel Osciak wrote:
> On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil wrote:
>> From: Hans Verkuil
>>
>> When using write() to write data to an output video node the vb2 core
>> should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody
>
> I'm confused. Shouldn't we be saving the existing timestamp from the buffer if
> V4L2_BUF_FLAG_TIMESTAMP_COPY is true, instead of getting it from
> v4l2_get_timestamp()?
When using the write() file operation the application has no way of setting the
timestamp. So it is uninitialized and the reader on the other side receives an
uninitialized (or 0, I'm not sure) timestamp. So __vb2_perform_fileio() has to
fill in a valid timestamp instead.
It's a corner case.
Regards,
Hans
>
>> else is able to provide this information with the write() operation.
>>
>> Signed-off-by: Hans Verkuil
>> ---
>> drivers/media/v4l2-core/videobuf2-core.c | 6 ++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index e38b45e..afd1268 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -22,6 +22,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>
>> static int debug;
>> @@ -2767,6 +2768,9 @@ static size_t __vb2_perform_fileio(struct vb2_queue
>> *q, char __user *data, size_
>> {
>> struct vb2_fileio_data *fileio;
>> struct vb2_fileio_buf *buf;
>> + bool set_timestamp = !read &&
>> + (q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) ==
>> + V4L2_BUF_FLAG_TIMESTAMP_COPY;
>> int ret, index;
>>
>> dprintk(3, "file io: mode %s, offset %ld, count %zd, %sblocking\n",
>> @@ -2868,6 +2872,8 @@ static size_t __vb2_perform_fileio(struct vb2_queue
>> *q, char __user *data, size_
>> fileio->b.memory = q->memory;
>> fileio->b.index = index;
>> fileio->b.bytesused = buf->pos;
>> + if (set_timestamp)
>> + v4l2_get_timestamp(&fileio->b.timestamp);
>> ret = vb2_internal_qbuf(q, &fileio->b);
>> dprintk(5, "file io: vb2_internal_qbuf result: %d\n", ret);
>> if (ret)
>> --
>> 1.9.0
>>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH 08/11] vb2: simplify a confusing condition.
On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil wrote:
> From: Hans Verkuil
>
> q->start_streaming_called is always true, so the WARN_ON check against
> it being false can be dropped.
>
> Signed-off-by: Hans Verkuil
Acked-by: Pawel Osciak
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c
> index 8984187..2ae316b 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1099,9 +1099,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum
> vb2_buffer_state state)
> if (!q->start_streaming_called) {
> if (WARN_ON(state != VB2_BUF_STATE_QUEUED))
> state = VB2_BUF_STATE_QUEUED;
> - } else if (!WARN_ON(!q->start_streaming_called)) {
> - if (WARN_ON(state != VB2_BUF_STATE_DONE &&
> - state != VB2_BUF_STATE_ERROR))
> + } else if (WARN_ON(state != VB2_BUF_STATE_DONE &&
> + state != VB2_BUF_STATE_ERROR)) {
> state = VB2_BUF_STATE_ERROR;
> }
>
> --
> 1.9.0
>
--
Best regards,
Pawel Osciak
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH 07/11] vb2: reject output buffers with V4L2_FIELD_ALTERNATE
On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil wrote:
> From: Hans Verkuil
>
> This is not allowed by the spec and does in fact not make any sense.
> Return -EINVAL if this is the case.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c
> index afd1268..8984187 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1526,6 +1526,15 @@ static int __buf_prepare(struct vb2_buffer *vb, const
> struct v4l2_buffer *b)
> __func__, ret);
> return ret;
> }
> + if (V4L2_TYPE_IS_OUTPUT(q->type) && b->field == V4L2_FIELD_ALTERNATE)
> {
Checking for field first would probably eliminate the additional
OUTPUT check most of the time.
I'd swap them.
> + /*
> +* If field is ALTERNATE, then we return an error.
I'd drop this line, doesn't really add anything.
> +* If the format's field is ALTERNATE, then the buffer's field
> +* should be either TOP or BOTTOM, but using ALTERNATE here as
> +* well makes no sense.
This doesn't really explain why this is an error and is confusing,
since we don't check TOP/BOTTOM
anyway. I think it would be better to say why ALTERNATE doesn't make
sense instead.
> +*/
> + return -EINVAL;
> + }
>
> vb->state = VB2_BUF_STATE_PREPARING;
> vb->v4l2_buf.timestamp.tv_sec = 0;
> --
> 1.9.0
>
--
Best regards,
Pawel Osciak
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH 06/11] vb2: set timestamp when using write()
On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil wrote:
> From: Hans Verkuil
>
> When using write() to write data to an output video node the vb2 core
> should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody
I'm confused. Shouldn't we be saving the existing timestamp from the buffer if
V4L2_BUF_FLAG_TIMESTAMP_COPY is true, instead of getting it from
v4l2_get_timestamp()?
> else is able to provide this information with the write() operation.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c
> index e38b45e..afd1268 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> static int debug;
> @@ -2767,6 +2768,9 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q,
> char __user *data, size_
> {
> struct vb2_fileio_data *fileio;
> struct vb2_fileio_buf *buf;
> + bool set_timestamp = !read &&
> + (q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) ==
> + V4L2_BUF_FLAG_TIMESTAMP_COPY;
> int ret, index;
>
> dprintk(3, "file io: mode %s, offset %ld, count %zd, %sblocking\n",
> @@ -2868,6 +2872,8 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q,
> char __user *data, size_
> fileio->b.memory = q->memory;
> fileio->b.index = index;
> fileio->b.bytesused = buf->pos;
> + if (set_timestamp)
> + v4l2_get_timestamp(&fileio->b.timestamp);
> ret = vb2_internal_qbuf(q, &fileio->b);
> dprintk(5, "file io: vb2_internal_qbuf result: %d\n", ret);
> if (ret)
> --
> 1.9.0
>
--
Best regards,
Pawel Osciak
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH 05/11] vb2: move __qbuf_mmap before __qbuf_userptr
On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil wrote:
> From: Hans Verkuil
>
> __qbuf_mmap was sort of hidden in between the much larger __qbuf_userptr
> and __qbuf_dmabuf functions. Move it before __qbuf_userptr which is
> also conform the usual order these memory models are implemented: first
> mmap, then userptr, then dmabuf.
>
> Signed-off-by: Hans Verkuil
Acked-by: Pawel Osciak
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 28 ++--
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c
> index 71be247..e38b45e 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1254,6 +1254,20 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
> const struct v4l2_buffer *b
> }
>
> /**
> + * __qbuf_mmap() - handle qbuf of an MMAP buffer
> + */
> +static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
> +{
> + int ret;
> +
> + __fill_vb2_buffer(vb, b, vb->v4l2_planes);
> + ret = call_vb_qop(vb, buf_prepare, vb);
> + if (ret)
> + fail_vb_qop(vb, buf_prepare);
> + return ret;
> +}
> +
> +/**
> * __qbuf_userptr() - handle qbuf of a USERPTR buffer
> */
> static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
> @@ -1359,20 +1373,6 @@ err:
> }
>
> /**
> - * __qbuf_mmap() - handle qbuf of an MMAP buffer
> - */
> -static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
> -{
> - int ret;
> -
> - __fill_vb2_buffer(vb, b, vb->v4l2_planes);
> - ret = call_vb_qop(vb, buf_prepare, vb);
> - if (ret)
> - fail_vb_qop(vb, buf_prepare);
> - return ret;
> -}
> -
> -/**
> * __qbuf_dmabuf() - handle qbuf of a DMABUF buffer
> */
> static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
> --
> 1.9.0
>
--
Best regards,
Pawel Osciak
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH 03/11] vb2: if bytesused is 0, then fill with output buffer length
On Mon, Apr 7, 2014 at 4:39 PM, Hans Verkuil wrote: > On 04/07/2014 09:20 AM, Pawel Osciak wrote: >> I'm thinking, that if we are doing this, perhaps we should just update >> the API to allow this case, i.e. say that if the bytesused is not set > > With 'not set' you mean 'is 0', right? Yes, correct. > >> for any planes, length will be used by default? >> This would be backwards-compatible. > > I agree with that. I'll update the doc. > > Regards, > > Hans > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEW PATCH 04/11] vb2: use correct prefix
On 04/07/2014 09:30 AM, Pawel Osciak wrote:
> The idea behind not using __func__ was that it was much more
> informative when debugging to see a "reqbufs" prefix instead of, for
> example "__verify_memory_type". But since some of the functions are
> shared across multiple ioctl impls now (e.g. __verify_memory_type is
> used by both reqbufs and createbufs), as much as I would prefer to
> keep this convention, it'd probably be too much to maintain.
>
> But if we want to do this, we should move "__func__" to dprintk()
> definition, instead of adding it to the call sites.
Good idea. I'll do that.
Regards,
Hans
>
> On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil wrote:
>> From: Hans Verkuil
>>
>> Many dprintk's in vb2 use a hardcoded prefix with the function name. In
>> many cases that is now outdated. Replace prefixes by the function name using
>> __func__. At least now I know if I see a 'qbuf:' prefix whether that refers
>> to the mmap, userptr or dmabuf variant.
>>
>> Signed-off-by: Hans Verkuil
>> ---
>> drivers/media/v4l2-core/videobuf2-core.c | 102
>> ---
>> 1 file changed, 54 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index 83e78e9..71be247 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -371,7 +371,8 @@ static int __vb2_queue_free(struct vb2_queue *q,
>> unsigned int buffers)
>> if (q->bufs[buffer] == NULL)
>> continue;
>> if (q->bufs[buffer]->state == VB2_BUF_STATE_PREPARING) {
>> - dprintk(1, "reqbufs: preparing buffers, cannot
>> free\n");
>> + dprintk(1, "%s: preparing buffers, cannot free\n",
>> + __func__);
>> return -EAGAIN;
>> }
>> }
>> @@ -656,12 +657,12 @@ int vb2_querybuf(struct vb2_queue *q, struct
>> v4l2_buffer *b)
>> int ret;
>>
>> if (b->type != q->type) {
>> - dprintk(1, "querybuf: wrong buffer type\n");
>> + dprintk(1, "%s: wrong buffer type\n", __func__);
>> return -EINVAL;
>> }
>>
>> if (b->index >= q->num_buffers) {
>> - dprintk(1, "querybuf: buffer index out of range\n");
>> + dprintk(1, "%s: buffer index out of range\n", __func__);
>> return -EINVAL;
>> }
>> vb = q->bufs[b->index];
>> @@ -721,12 +722,12 @@ static int __verify_memory_type(struct vb2_queue *q,
>> {
>> if (memory != V4L2_MEMORY_MMAP && memory != V4L2_MEMORY_USERPTR &&
>> memory != V4L2_MEMORY_DMABUF) {
>> - dprintk(1, "reqbufs: unsupported memory type\n");
>> + dprintk(1, "%s: unsupported memory type\n", __func__);
>> return -EINVAL;
>> }
>>
>> if (type != q->type) {
>> - dprintk(1, "reqbufs: requested type is incorrect\n");
>> + dprintk(1, "%s: requested type is incorrect\n", __func__);
>> return -EINVAL;
>> }
>>
>> @@ -735,17 +736,20 @@ static int __verify_memory_type(struct vb2_queue *q,
>> * are available.
>> */
>> if (memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
>> - dprintk(1, "reqbufs: MMAP for current setup unsupported\n");
>> + dprintk(1, "%s: MMAP for current setup unsupported\n",
>> + __func__);
>> return -EINVAL;
>> }
>>
>> if (memory == V4L2_MEMORY_USERPTR && __verify_userptr_ops(q)) {
>> - dprintk(1, "reqbufs: USERPTR for current setup
>> unsupported\n");
>> + dprintk(1, "%s: USERPTR for current setup unsupported\n",
>> + __func__);
>> return -EINVAL;
>> }
>>
>> if (memory == V4L2_MEMORY_DMABUF && __verify_dmabuf_ops(q)) {
>> - dprintk(1, "reqbufs: DMABUF for current setup
>> unsupported\n");
>> + dprintk(1, "%s: DMABUF for current setup unsupported\n",
>> + __func__);
>> return -EINVAL;
>> }
>>
>> @@ -755,7 +759,7 @@ static int __verify_memory_type(struct vb2_queue *q,
>> * do the memory and type validation.
>> */
>> if (q->fileio) {
>> - dprintk(1, "reqbufs: file io in progress\n");
>> + dprintk(1, "%s: file io in progress\n", __func__);
>> return -EBUSY;
>> }
>> return 0;
>> @@ -790,7 +794,7 @@ static int __reqbufs(struct vb2_queue *q, struct
>> v4l2_requestbuffers *req)
>> int ret;
>>
>> if (q->streaming) {
>> - dprintk(1, "reqbufs: streaming active\n");
>> + dprintk(1, "%s: streaming active\n", __func__);
>>
Re: [REVIEW PATCH 03/11] vb2: if bytesused is 0, then fill with output buffer length
On 04/07/2014 09:20 AM, Pawel Osciak wrote:
> I'm thinking, that if we are doing this, perhaps we should just update
> the API to allow this case, i.e. say that if the bytesused is not set
With 'not set' you mean 'is 0', right?
> for any planes, length will be used by default?
> This would be backwards-compatible.
I agree with that. I'll update the doc.
Regards,
Hans
>
> On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil wrote:
>> From: Hans Verkuil
>>
>> The application should really always fill in bytesused for output
>> buffers, unfortunately the vb2 framework never checked for that.
>>
>> So for single planar formats replace a bytesused of 0 by the length
>> of the buffer, and for multiplanar format do the same if bytesused is
>> 0 for ALL planes.
>>
>> This seems to be what the user really intended if v4l2_buffer was
>> just memset to 0.
>>
>> I'm afraid that just checking for this and returning an error would
>> break too many applications. Quite a few drivers never check for bytesused
>> at all and just use the buffer length instead.
>>
>> Signed-off-by: Hans Verkuil
>
> Acked-by: Pawel Osciak
>
>> ---
>> drivers/media/v4l2-core/videobuf2-core.c | 32
>> +++-
>> 1 file changed, 27 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index 1a09442..83e78e9 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -1145,19 +1145,35 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
>> const struct v4l2_buffer *b
>> memset(v4l2_planes[plane].reserved, 0,
>>sizeof(v4l2_planes[plane].reserved));
>> v4l2_planes[plane].data_offset = 0;
>> + v4l2_planes[plane].bytesused = 0;
>> }
>>
>> /* Fill in driver-provided information for OUTPUT types */
>> if (V4L2_TYPE_IS_OUTPUT(b->type)) {
>> + bool bytesused_is_used;
>> +
>> + /* Check if bytesused == 0 for all planes */
>> + for (plane = 0; plane < vb->num_planes; ++plane)
>> + if (b->m.planes[plane].bytesused)
>> + break;
>> + bytesused_is_used = plane < vb->num_planes;
>> +
>> /*
>> * Will have to go up to b->length when API starts
>> * accepting variable number of planes.
>> +*
>> +* If bytesused_is_used is false, then fall back to
>> the
>> +* full buffer size. In that case userspace clearly
>> +* never bothered to set it and it's a safe
>> assumption
>> +* that they really meant to use the full plane
>> sizes.
>> */
>> for (plane = 0; plane < vb->num_planes; ++plane) {
>> - v4l2_planes[plane].bytesused =
>> - b->m.planes[plane].bytesused;
>> - v4l2_planes[plane].data_offset =
>> - b->m.planes[plane].data_offset;
>> + struct v4l2_plane *pdst =
>> &v4l2_planes[plane];
>> + struct v4l2_plane *psrc =
>> &b->m.planes[plane];
>> +
>> + pdst->bytesused = bytesused_is_used ?
>> + psrc->bytesused : psrc->length;
>> + pdst->data_offset = psrc->data_offset;
>> }
>> }
>>
>> @@ -1183,9 +1199,15 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
>> const struct v4l2_buffer *b
>> * so fill in relevant v4l2_buffer struct fields instead.
>> * In videobuf we use our internal V4l2_planes struct for
>> * single-planar buffers as well, for simplicity.
>> +*
>> +* If bytesused == 0, then fall back to the full buffer size
>> +* as that's a sensible default.
>> */
>> if (V4L2_TYPE_IS_OUTPUT(b->type))
>> - v4l2_planes[0].bytesused = b->bytesused;
>> + v4l2_planes[0].bytesused =
>> + b->bytesused ? b->bytesused : b->length;
>> + else
>> + v4l2_planes[0].bytesused = 0;
>> /* Single-planar buffers never use data_offset */
>> v4l2_planes[0].data_offset = 0;
>>
>> --
>> 1.9.0
>>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.ke
Re: [REVIEW PATCH 04/11] vb2: use correct prefix
The idea behind not using __func__ was that it was much more
informative when debugging to see a "reqbufs" prefix instead of, for
example "__verify_memory_type". But since some of the functions are
shared across multiple ioctl impls now (e.g. __verify_memory_type is
used by both reqbufs and createbufs), as much as I would prefer to
keep this convention, it'd probably be too much to maintain.
But if we want to do this, we should move "__func__" to dprintk()
definition, instead of adding it to the call sites.
On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil wrote:
> From: Hans Verkuil
>
> Many dprintk's in vb2 use a hardcoded prefix with the function name. In
> many cases that is now outdated. Replace prefixes by the function name using
> __func__. At least now I know if I see a 'qbuf:' prefix whether that refers
> to the mmap, userptr or dmabuf variant.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 102
> ---
> 1 file changed, 54 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c
> index 83e78e9..71be247 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -371,7 +371,8 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned
> int buffers)
> if (q->bufs[buffer] == NULL)
> continue;
> if (q->bufs[buffer]->state == VB2_BUF_STATE_PREPARING) {
> - dprintk(1, "reqbufs: preparing buffers, cannot
> free\n");
> + dprintk(1, "%s: preparing buffers, cannot free\n",
> + __func__);
> return -EAGAIN;
> }
> }
> @@ -656,12 +657,12 @@ int vb2_querybuf(struct vb2_queue *q, struct
> v4l2_buffer *b)
> int ret;
>
> if (b->type != q->type) {
> - dprintk(1, "querybuf: wrong buffer type\n");
> + dprintk(1, "%s: wrong buffer type\n", __func__);
> return -EINVAL;
> }
>
> if (b->index >= q->num_buffers) {
> - dprintk(1, "querybuf: buffer index out of range\n");
> + dprintk(1, "%s: buffer index out of range\n", __func__);
> return -EINVAL;
> }
> vb = q->bufs[b->index];
> @@ -721,12 +722,12 @@ static int __verify_memory_type(struct vb2_queue *q,
> {
> if (memory != V4L2_MEMORY_MMAP && memory != V4L2_MEMORY_USERPTR &&
> memory != V4L2_MEMORY_DMABUF) {
> - dprintk(1, "reqbufs: unsupported memory type\n");
> + dprintk(1, "%s: unsupported memory type\n", __func__);
> return -EINVAL;
> }
>
> if (type != q->type) {
> - dprintk(1, "reqbufs: requested type is incorrect\n");
> + dprintk(1, "%s: requested type is incorrect\n", __func__);
> return -EINVAL;
> }
>
> @@ -735,17 +736,20 @@ static int __verify_memory_type(struct vb2_queue *q,
> * are available.
> */
> if (memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
> - dprintk(1, "reqbufs: MMAP for current setup unsupported\n");
> + dprintk(1, "%s: MMAP for current setup unsupported\n",
> + __func__);
> return -EINVAL;
> }
>
> if (memory == V4L2_MEMORY_USERPTR && __verify_userptr_ops(q)) {
> - dprintk(1, "reqbufs: USERPTR for current setup
> unsupported\n");
> + dprintk(1, "%s: USERPTR for current setup unsupported\n",
> + __func__);
> return -EINVAL;
> }
>
> if (memory == V4L2_MEMORY_DMABUF && __verify_dmabuf_ops(q)) {
> - dprintk(1, "reqbufs: DMABUF for current setup unsupported\n");
> + dprintk(1, "%s: DMABUF for current setup unsupported\n",
> + __func__);
> return -EINVAL;
> }
>
> @@ -755,7 +759,7 @@ static int __verify_memory_type(struct vb2_queue *q,
> * do the memory and type validation.
> */
> if (q->fileio) {
> - dprintk(1, "reqbufs: file io in progress\n");
> + dprintk(1, "%s: file io in progress\n", __func__);
> return -EBUSY;
> }
> return 0;
> @@ -790,7 +794,7 @@ static int __reqbufs(struct vb2_queue *q, struct
> v4l2_requestbuffers *req)
> int ret;
>
> if (q->streaming) {
> - dprintk(1, "reqbufs: streaming active\n");
> + dprintk(1, "%s: streaming active\n", __func__);
> return -EBUSY;
> }
>
> @@ -800,7 +804,7 @@ static int __reqbufs(struct vb2_queue *q, struct
> v4l2_requestbuffers *req)
> * are not in use and can be freed.
> */
>
Re: [REVIEW PATCH 03/11] vb2: if bytesused is 0, then fill with output buffer length
I'm thinking, that if we are doing this, perhaps we should just update
the API to allow this case, i.e. say that if the bytesused is not set
for any planes, length will be used by default?
This would be backwards-compatible.
On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil wrote:
> From: Hans Verkuil
>
> The application should really always fill in bytesused for output
> buffers, unfortunately the vb2 framework never checked for that.
>
> So for single planar formats replace a bytesused of 0 by the length
> of the buffer, and for multiplanar format do the same if bytesused is
> 0 for ALL planes.
>
> This seems to be what the user really intended if v4l2_buffer was
> just memset to 0.
>
> I'm afraid that just checking for this and returning an error would
> break too many applications. Quite a few drivers never check for bytesused
> at all and just use the buffer length instead.
>
> Signed-off-by: Hans Verkuil
Acked-by: Pawel Osciak
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 32
> +++-
> 1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c
> index 1a09442..83e78e9 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1145,19 +1145,35 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
> const struct v4l2_buffer *b
> memset(v4l2_planes[plane].reserved, 0,
>sizeof(v4l2_planes[plane].reserved));
> v4l2_planes[plane].data_offset = 0;
> + v4l2_planes[plane].bytesused = 0;
> }
>
> /* Fill in driver-provided information for OUTPUT types */
> if (V4L2_TYPE_IS_OUTPUT(b->type)) {
> + bool bytesused_is_used;
> +
> + /* Check if bytesused == 0 for all planes */
> + for (plane = 0; plane < vb->num_planes; ++plane)
> + if (b->m.planes[plane].bytesused)
> + break;
> + bytesused_is_used = plane < vb->num_planes;
> +
> /*
> * Will have to go up to b->length when API starts
> * accepting variable number of planes.
> +*
> +* If bytesused_is_used is false, then fall back to
> the
> +* full buffer size. In that case userspace clearly
> +* never bothered to set it and it's a safe assumption
> +* that they really meant to use the full plane sizes.
> */
> for (plane = 0; plane < vb->num_planes; ++plane) {
> - v4l2_planes[plane].bytesused =
> - b->m.planes[plane].bytesused;
> - v4l2_planes[plane].data_offset =
> - b->m.planes[plane].data_offset;
> + struct v4l2_plane *pdst = &v4l2_planes[plane];
> + struct v4l2_plane *psrc = &b->m.planes[plane];
> +
> + pdst->bytesused = bytesused_is_used ?
> + psrc->bytesused : psrc->length;
> + pdst->data_offset = psrc->data_offset;
> }
> }
>
> @@ -1183,9 +1199,15 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
> const struct v4l2_buffer *b
> * so fill in relevant v4l2_buffer struct fields instead.
> * In videobuf we use our internal V4l2_planes struct for
> * single-planar buffers as well, for simplicity.
> +*
> +* If bytesused == 0, then fall back to the full buffer size
> +* as that's a sensible default.
> */
> if (V4L2_TYPE_IS_OUTPUT(b->type))
> - v4l2_planes[0].bytesused = b->bytesused;
> + v4l2_planes[0].bytesused =
> + b->bytesused ? b->bytesused : b->length;
> + else
> + v4l2_planes[0].bytesused = 0;
> /* Single-planar buffers never use data_offset */
> v4l2_planes[0].data_offset = 0;
>
> --
> 1.9.0
>
--
Best regards,
Pawel Osciak
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
