Re: [PATCH 2/6] v4l: omap4iss: Add support for OMAP4 camera interface - Video devices
On 10/03/2013 01:55 AM, Laurent Pinchart wrote:
> From: Sergio Aguirre
>
> This adds a very simplistic driver to utilize the CSI2A interface inside
> the ISS subsystem in OMAP4, and dump the data to memory.
>
> Check Documentation/video4linux/omap4_camera.txt for details.
>
> This commit adds video devices support.
>
> Signed-off-by: Sergio Aguirre
>
> [Port the driver to v3.12-rc3, including the following changes
> - Don't include plat/ headers
> - Don't use cpu_is_omap44xx() macro
> - Don't depend on EXPERIMENTAL
> - Fix s_crop operation prototype
> - Update link_notify prototype
> - Rename media_entity_remote_source to media_entity_remote_pad]
>
> Signed-off-by: Laurent Pinchart
> ---
> drivers/staging/media/omap4iss/iss_video.c | 1129
>
> drivers/staging/media/omap4iss/iss_video.h | 201 +
> 2 files changed, 1330 insertions(+)
> create mode 100644 drivers/staging/media/omap4iss/iss_video.c
> create mode 100644 drivers/staging/media/omap4iss/iss_video.h
>
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c
> new file mode 100644
> index 000..31f1b88
> --- /dev/null
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> +/*
> -
> + * V4L2 ioctls
> + */
> +
> +static int
> +iss_video_querycap(struct file *file, void *fh, struct v4l2_capability *cap)
> +{
> + struct iss_video *video = video_drvdata(file);
> +
> + strlcpy(cap->driver, ISS_VIDEO_DRIVER_NAME, sizeof(cap->driver));
> + strlcpy(cap->card, video->video.name, sizeof(cap->card));
> + strlcpy(cap->bus_info, "media", sizeof(cap->bus_info));
> +
> + if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> + else
> + cap->capabilities = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
Set device_caps instead of capabilities and add:
cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> +
> + return 0;
> +}
> +
> +static int
> +iss_video_get_format(struct file *file, void *fh, struct v4l2_format *format)
> +{
> + struct iss_video_fh *vfh = to_iss_video_fh(fh);
> + struct iss_video *video = video_drvdata(file);
> +
> + if (format->type != video->type)
> + return -EINVAL;
> +
> + mutex_lock(&video->mutex);
> + *format = vfh->format;
> + mutex_unlock(&video->mutex);
> +
> + return 0;
> +}
> +
> +static int
> +iss_video_set_format(struct file *file, void *fh, struct v4l2_format *format)
> +{
> + struct iss_video_fh *vfh = to_iss_video_fh(fh);
> + struct iss_video *video = video_drvdata(file);
> + struct v4l2_mbus_framefmt fmt;
> +
> + if (format->type != video->type)
> + return -EINVAL;
> +
> + mutex_lock(&video->mutex);
> +
> + /* Fill the bytesperline and sizeimage fields by converting to media bus
> + * format and back to pixel format.
> + */
> + iss_video_pix_to_mbus(&format->fmt.pix, &fmt);
> + iss_video_mbus_to_pix(video, &fmt, &format->fmt.pix);
> +
> + vfh->format = *format;
> +
> + mutex_unlock(&video->mutex);
> + return 0;
> +}
> +
> +static int
> +iss_video_try_format(struct file *file, void *fh, struct v4l2_format *format)
> +{
> + struct iss_video *video = video_drvdata(file);
> + struct v4l2_subdev_format fmt;
> + struct v4l2_subdev *subdev;
> + u32 pad;
> + int ret;
> +
> + if (format->type != video->type)
> + return -EINVAL;
> +
> + subdev = iss_video_remote_subdev(video, &pad);
> + if (subdev == NULL)
> + return -EINVAL;
> +
> + iss_video_pix_to_mbus(&format->fmt.pix, &fmt.format);
> +
> + fmt.pad = pad;
> + fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> + ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
> + if (ret)
> + return ret == -ENOIOCTLCMD ? -EINVAL : ret;
Return ENOTTY instead of EINVAL. Even better, use v4l2_subdev_has_op() +
v4l2_disable_ioctl()
to just disable ioctls based on the available subdev ops in probe().
> +
> + iss_video_mbus_to_pix(video, &fmt.format, &format->fmt.pix);
> + return 0;
> +}
> +
> +static int
> +iss_video_cropcap(struct file *file, void *fh, struct v4l2_cropcap *cropcap)
> +{
> + struct iss_video *video = video_drvdata(file);
> + struct v4l2_subdev *subdev;
> + int ret;
> +
> + subdev = iss_video_remote_subdev(video, NULL);
> + if (subdev == NULL)
> + return -EINVAL;
> +
> + mutex_lock(&video->mutex);
> + ret = v4l2_subdev_call(subdev, video, cropcap, cropcap);
> + mutex_unlock(&video->mutex);
> +
> + return ret == -ENOIOCTLCMD ? -EINVAL : ret;
> +}
> +
> +static int
> +iss_video_get_crop(struct file *file, void *fh, struct v4l2_crop *crop)
> +{
> + struct iss_video *video = video_drvdata(file);
> + struct v4l2_subdev_format format;
> +
Re: [PATCH 1/6] v4l: omap4iss: Add support for OMAP4 camera interface - Core
A few small comments below... On 10/03/2013 01:55 AM, Laurent Pinchart wrote: > From: Sergio Aguirre > > This adds a very simplistic driver to utilize the CSI2A interface inside > the ISS subsystem in OMAP4, and dump the data to memory. > > Check Documentation/video4linux/omap4_camera.txt for details. > > This commit adds the driver core, registers definitions and > documentation. > > Signed-off-by: Sergio Aguirre > > [Port the driver to v3.12-rc3, including the following changes > - Don't include plat/ headers > - Don't use cpu_is_omap44xx() macro > - Don't depend on EXPERIMENTAL > - Fix s_crop operation prototype > - Update link_notify prototype > - Rename media_entity_remote_source to media_entity_remote_pad] > > Signed-off-by: Laurent Pinchart > --- > Documentation/video4linux/omap4_camera.txt | 63 ++ > drivers/staging/media/omap4iss/iss.c | 1477 > > drivers/staging/media/omap4iss/iss.h | 153 +++ > drivers/staging/media/omap4iss/iss_regs.h | 883 + > include/media/omap4iss.h | 65 ++ > 5 files changed, 2641 insertions(+) > create mode 100644 Documentation/video4linux/omap4_camera.txt > create mode 100644 drivers/staging/media/omap4iss/iss.c > create mode 100644 drivers/staging/media/omap4iss/iss.h > create mode 100644 drivers/staging/media/omap4iss/iss_regs.h > create mode 100644 include/media/omap4iss.h > > diff --git a/Documentation/video4linux/omap4_camera.txt > b/Documentation/video4linux/omap4_camera.txt > new file mode 100644 > index 000..2ebbd1d > --- /dev/null > +++ b/Documentation/video4linux/omap4_camera.txt > @@ -0,0 +1,63 @@ > + OMAP4 ISS Driver > + > + > +Introduction > + > + > +The OMAP44XX family of chips contains the Imaging SubSystem (a.k.a. ISS), > +Which contains several components that can be categorized in 3 big groups: > + > +- Interfaces (2 Interfaces: CSI2-A & CSI2-B/CCP2) > +- ISP (Image Signal Processor) > +- SIMCOP (Still Image Coprocessor) > + > +For more information, please look in [1] for latest version of: > + "OMAP4430 Multimedia Device Silicon Revision 2.x" > + > +As of Revision AB, the ISS is described in detail in section 8. > + > +This driver is supporting _only_ the CSI2-A/B interfaces for now. > + > +It makes use of the Media Controller framework [2], and inherited most of the > +code from OMAP3 ISP driver (found under drivers/media/video/omap3isp/*), > except Update the omap3isp path. > +that it doesn't need an IOMMU now for ISS buffers memory mapping. > + > +Supports usage of MMAP buffers only (for now). > + > +IMPORTANT: It is recommended to have this patchset: > + Contiguous Memory Allocator (v22) [3]. This note can be removed as CMA has been merged. > + > +Tested platforms > + > + > +- OMAP4430SDP, w/ ES2.1 GP & SEVM4430-CAM-V1-0 (Contains IMX060 & OV5640, in > + which only the last one is supported, outputting YUV422 frames). > + > +- TI Blaze MDP, w/ OMAP4430 ES2.2 EMU (Contains 1 IMX060 & 2 OV5650 sensors, > in > + which only the OV5650 are supported, outputting RAW10 frames). > + > +- PandaBoard, Rev. A2, w/ OMAP4430 ES2.1 GP & OV adapter board, tested with > + following sensors: > + * OV5640 > + * OV5650 > + > +- Tested on mainline kernel: > + > + > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=summary > + > + Tag: v3.3 (commit c16fa4f2ad19908a47c63d8fa436a1178438c7e7) > + > +File list > +- > +drivers/media/video/omap4iss/ Wrong path. > +include/media/omap4iss.h > + > +References > +-- > + > +[1] > http://focus.ti.com/general/docs/wtbu/wtbudocumentcenter.tsp?navigationId=12037&templateId=6123#62 > +[2] http://lwn.net/Articles/420485/ > +[3] http://www.spinics.net/lists/linux-media/msg44370.html > +-- > +Author: Sergio Aguirre > +Copyright (C) 2012, Texas Instruments > diff --git a/drivers/staging/media/omap4iss/iss.c > b/drivers/staging/media/omap4iss/iss.c > new file mode 100644 > index 000..d054d9b > --- /dev/null > +++ b/drivers/staging/media/omap4iss/iss.c > @@ -0,0 +1,1477 @@ > +/* > + * TI OMAP4 ISS V4L2 Driver > + * > + * Copyright (C) 2012, Texas Instruments > + * > + * Author: Sergio Aguirre > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > +#include "iss.h" > +#include "iss_regs.h" > + > +#define ISS_PRINT_REGISTER(iss, name)\ > + dev_dbg(iss->dev, "###ISS " #name "=0x%08x\n", \ > + readl(iss->regs[OMAP4_ISS_MEM_TOP] + ISS_##name)) > + > +s
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Thu Oct 3 04:00:21 CEST 2013 git branch: test git hash: b4559ace2ca8c88666584279f582b998c6591fb0 gcc version:i686-linux-gcc (GCC) 4.8.1 sparse version: 0.4.5-rc1 host hardware: x86_64 host os:3.10.1 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: ERRORS linux-2.6.32.27-i686: ERRORS linux-2.6.33.7-i686: ERRORS linux-2.6.34.7-i686: ERRORS linux-2.6.35.9-i686: ERRORS linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.10.1-i686: OK linux-3.1.10-i686: ERRORS linux-3.11.1-i686: OK linux-3.12-rc1-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-2.6.31.14-x86_64: ERRORS linux-2.6.32.27-x86_64: ERRORS linux-2.6.33.7-x86_64: ERRORS linux-2.6.34.7-x86_64: ERRORS linux-2.6.35.9-x86_64: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.10.1-x86_64: OK linux-3.1.10-x86_64: ERRORS linux-3.11.1-x86_64: OK linux-3.12-rc1-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS 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 apps: WARNINGS spec-git: OK sparse version: 0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.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
Re: [PATCH v2 1/4] media: Add pad flag MEDIA_PAD_FL_MUST_CONNECT
Hi Sakari,
On 10/03/2013 08:29 AM, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Thursday 03 October 2013 02:17:50 Sakari Ailus wrote:
>> Pads that set this flag must be connected by an active link for the entity
>> to stream.
>>
>> Signed-off-by: Sakari Ailus
>> Acked-by: Sylwester Nawrocki
>
> Acked-by: Laurent Pinchart
Looks good, I would just like to ask for changing my e-mail address on those
patches to s.nawro...@samsung.com, sorry for not mentioning it earlier.
Just one doubt below regarding name of the flag.
>> ---
>> Documentation/DocBook/media/v4l/media-ioc-enum-links.xml | 10 ++
>> include/uapi/linux/media.h |1 +
>> 2 files changed, 11 insertions(+)
>>
>> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
>> b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml index
>> 355df43..e357dc9 100644
>> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
>> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
>> @@ -134,6 +134,16 @@
>> Output pad, relative to the entity. Output pads source data
>> and are origins of links.
>>
>> +
>> +MEDIA_PAD_FL_MUST_CONNECT
>> +If this flag is set and the pad is linked to any other
>> +pad, then at least one of those links must be enabled for the
>> +entity to be able to stream. There could be temporary reasons
>> +(e.g. device configuration dependent) for the pad to need
>> +enabled links even when this flag isn't set; the absence of the
>> +flag doesn't imply there is none. The flag has no effect on pads
>> +without connected links.
Probably MEDIA_PAD_FL_MUST_CONNECT name is fine, but isn't it more something
like MEDIA_PAD_FL_NEED_ACTIVE_LINK ? Or presumably MEDIA_PAD_FL_MUST_CONNECT
just doesn't make sense on pads without connected links and should never be
set on such pads ? From the last sentence it feels the situation where a pad
without a connected link has this flags set is allowed and a valid
configuration.
Perhaps the last sentence should be something like:
"The flag should not be used on pads without connected links and has no effect
on such pads."
Regards,
Sylwester
>> +
>>
>>
>>
>> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
>> index ed49574..d847c76 100644
>> --- a/include/uapi/linux/media.h
>> +++ b/include/uapi/linux/media.h
>> @@ -98,6 +98,7 @@ struct media_entity_desc {
>>
>> #define MEDIA_PAD_FL_SINK (1 << 0)
>> #define MEDIA_PAD_FL_SOURCE (1 << 1)
>> +#define MEDIA_PAD_FL_MUST_CONNECT (1 << 2)
>>
>> struct media_pad_desc {
>> __u32 entity; /* entity ID */
--
Sylwester Nawrocki
Samsung R&D Institute Poland
--
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/6] v4l: omap4iss: Add support for OMAP4 camera interface - Resizer
From: Sergio Aguirre
This adds a very simplistic driver to utilize the CSI2A interface inside
the ISS subsystem in OMAP4, and dump the data to memory.
Check Documentation/video4linux/omap4_camera.txt for details.
This commit adds resizer support.
Signed-off-by: Sergio Aguirre
[Port the driver to v3.12-rc3, including the following changes
- Don't include plat/ headers
- Don't use cpu_is_omap44xx() macro
- Don't depend on EXPERIMENTAL
- Fix s_crop operation prototype
- Update link_notify prototype
- Rename media_entity_remote_source to media_entity_remote_pad]
Signed-off-by: Laurent Pinchart
---
drivers/staging/media/omap4iss/iss_resizer.c | 905 +++
drivers/staging/media/omap4iss/iss_resizer.h | 75 +++
2 files changed, 980 insertions(+)
create mode 100644 drivers/staging/media/omap4iss/iss_resizer.c
create mode 100644 drivers/staging/media/omap4iss/iss_resizer.h
diff --git a/drivers/staging/media/omap4iss/iss_resizer.c
b/drivers/staging/media/omap4iss/iss_resizer.c
new file mode 100644
index 000..cb5df52
--- /dev/null
+++ b/drivers/staging/media/omap4iss/iss_resizer.c
@@ -0,0 +1,905 @@
+/*
+ * TI OMAP4 ISS V4L2 Driver - ISP RESIZER module
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ *
+ * Author: Sergio Aguirre
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "iss.h"
+#include "iss_regs.h"
+#include "iss_resizer.h"
+
+static struct v4l2_mbus_framefmt *
+__resizer_get_format(struct iss_resizer_device *resizer, struct v4l2_subdev_fh
*fh,
+ unsigned int pad, enum v4l2_subdev_format_whence which);
+
+static const unsigned int resizer_fmts[] = {
+ V4L2_MBUS_FMT_UYVY8_1X16,
+ V4L2_MBUS_FMT_YUYV8_1X16,
+};
+
+/*
+ * resizer_print_status - Print current RESIZER Module register values.
+ * @resizer: Pointer to ISS ISP RESIZER device.
+ *
+ * Also prints other debug information stored in the RESIZER module.
+ */
+#define RSZ_PRINT_REGISTER(iss, name)\
+ dev_dbg(iss->dev, "###RSZ " #name "=0x%08x\n", \
+ readl(iss->regs[OMAP4_ISS_MEM_ISP_RESIZER] + RSZ_##name))
+
+#define RZA_PRINT_REGISTER(iss, name)\
+ dev_dbg(iss->dev, "###RZA " #name "=0x%08x\n", \
+ readl(iss->regs[OMAP4_ISS_MEM_ISP_RESIZER] + RZA_##name))
+
+static void resizer_print_status(struct iss_resizer_device *resizer)
+{
+ struct iss_device *iss = to_iss_device(resizer);
+
+ dev_dbg(iss->dev, "-RESIZER Register dump-\n");
+
+ RSZ_PRINT_REGISTER(iss, SYSCONFIG);
+ RSZ_PRINT_REGISTER(iss, IN_FIFO_CTRL);
+ RSZ_PRINT_REGISTER(iss, FRACDIV);
+ RSZ_PRINT_REGISTER(iss, SRC_EN);
+ RSZ_PRINT_REGISTER(iss, SRC_MODE);
+ RSZ_PRINT_REGISTER(iss, SRC_FMT0);
+ RSZ_PRINT_REGISTER(iss, SRC_FMT1);
+ RSZ_PRINT_REGISTER(iss, SRC_VPS);
+ RSZ_PRINT_REGISTER(iss, SRC_VSZ);
+ RSZ_PRINT_REGISTER(iss, SRC_HPS);
+ RSZ_PRINT_REGISTER(iss, SRC_HSZ);
+ RSZ_PRINT_REGISTER(iss, DMA_RZA);
+ RSZ_PRINT_REGISTER(iss, DMA_RZB);
+ RSZ_PRINT_REGISTER(iss, DMA_STA);
+ RSZ_PRINT_REGISTER(iss, GCK_MMR);
+ RSZ_PRINT_REGISTER(iss, GCK_SDR);
+ RSZ_PRINT_REGISTER(iss, IRQ_RZA);
+ RSZ_PRINT_REGISTER(iss, IRQ_RZB);
+ RSZ_PRINT_REGISTER(iss, YUV_Y_MIN);
+ RSZ_PRINT_REGISTER(iss, YUV_Y_MAX);
+ RSZ_PRINT_REGISTER(iss, YUV_C_MIN);
+ RSZ_PRINT_REGISTER(iss, YUV_C_MAX);
+ RSZ_PRINT_REGISTER(iss, SEQ);
+
+ RZA_PRINT_REGISTER(iss, EN);
+ RZA_PRINT_REGISTER(iss, MODE);
+ RZA_PRINT_REGISTER(iss, 420);
+ RZA_PRINT_REGISTER(iss, I_VPS);
+ RZA_PRINT_REGISTER(iss, I_HPS);
+ RZA_PRINT_REGISTER(iss, O_VSZ);
+ RZA_PRINT_REGISTER(iss, O_HSZ);
+ RZA_PRINT_REGISTER(iss, V_PHS_Y);
+ RZA_PRINT_REGISTER(iss, V_PHS_C);
+ RZA_PRINT_REGISTER(iss, V_DIF);
+ RZA_PRINT_REGISTER(iss, V_TYP);
+ RZA_PRINT_REGISTER(iss, V_LPF);
+ RZA_PRINT_REGISTER(iss, H_PHS);
+ RZA_PRINT_REGISTER(iss, H_DIF);
+ RZA_PRINT_REGISTER(iss, H_TYP);
+ RZA_PRINT_REGISTER(iss, H_LPF);
+ RZA_PRINT_REGISTER(iss, DWN_EN);
+ RZA_PRINT_REGISTER(iss, SDR_Y_BAD_H);
+ RZA_PRINT_REGISTER(iss, SDR_Y_BAD_L);
+ RZA_PRINT_REGISTER(iss, SDR_Y_SAD_H);
+ RZA_PRINT_REGISTER(iss, SDR_Y_SAD_L);
+ RZA_PRINT_REGISTER(iss, SDR_Y_OFT);
+ RZA_PRINT_REGISTER(iss, SDR_Y_PTR_S);
+ RZA_PRINT_REGISTER(iss, SDR_Y_PTR_E);
+ RZA_PRINT_REGISTER(iss, SDR_C_BAD_H);
+ RZA_PRINT_REGISTER(iss, SDR_C_BAD_L);
+ RZA_PRINT_REGISTER(iss, SDR_C_SAD_H);
+ RZA_PRINT_REGISTER(iss, SDR_C_SAD_L);
+ RZA_PRINT_REGISTER(iss, SDR_C_OFT);
+ R
[PATCH 0/6] OMAP4 ISS driver
Hello, The OMAP4 ISS driver has lived out of tree for more than two years now. This situation is both sad and resource-wasting, as the driver has been used (and thus) hacked since then with nowhere to send patches to. Time has come to fix the problem. As the code is mostly, but not quite ready for prime time, I'd like to request its addition to drivers/staging/. I've added a (pretty small) TODO file and I commit to cleaning up the code and get it to drivers/media/ where it belongs. I've split the driver in six patches to avoid getting caught in vger's size and to make review slightly easier. Sergio Aguirre is the driver author (huge thanks for that!), I've thus kept his authorship on patches 1/6 to 5/6. I don't have much else to add here, let's get this beast to mainline and allow other developers to use the driver and contribute patches. Laurent Pinchart (1): v4l: omap4iss: Add support for OMAP4 camera interface - Build system Sergio Aguirre (5): v4l: omap4iss: Add support for OMAP4 camera interface - Core v4l: omap4iss: Add support for OMAP4 camera interface - Video devices v4l: omap4iss: Add support for OMAP4 camera interface - CSI receivers v4l: omap4iss: Add support for OMAP4 camera interface - IPIPE(IF) v4l: omap4iss: Add support for OMAP4 camera interface - Resizer Documentation/video4linux/omap4_camera.txt | 63 ++ drivers/staging/media/Kconfig|2 + drivers/staging/media/Makefile |1 + drivers/staging/media/omap4iss/Kconfig | 12 + drivers/staging/media/omap4iss/Makefile |6 + drivers/staging/media/omap4iss/TODO |4 + drivers/staging/media/omap4iss/iss.c | 1477 ++ drivers/staging/media/omap4iss/iss.h | 153 +++ drivers/staging/media/omap4iss/iss_csi2.c| 1368 drivers/staging/media/omap4iss/iss_csi2.h| 156 +++ drivers/staging/media/omap4iss/iss_csiphy.c | 278 + drivers/staging/media/omap4iss/iss_csiphy.h | 51 + drivers/staging/media/omap4iss/iss_ipipe.c | 581 ++ drivers/staging/media/omap4iss/iss_ipipe.h | 67 ++ drivers/staging/media/omap4iss/iss_ipipeif.c | 847 +++ drivers/staging/media/omap4iss/iss_ipipeif.h | 92 ++ drivers/staging/media/omap4iss/iss_regs.h| 883 +++ drivers/staging/media/omap4iss/iss_resizer.c | 905 drivers/staging/media/omap4iss/iss_resizer.h | 75 ++ drivers/staging/media/omap4iss/iss_video.c | 1129 drivers/staging/media/omap4iss/iss_video.h | 201 include/media/omap4iss.h | 65 ++ 22 files changed, 8416 insertions(+) create mode 100644 Documentation/video4linux/omap4_camera.txt create mode 100644 drivers/staging/media/omap4iss/Kconfig create mode 100644 drivers/staging/media/omap4iss/Makefile create mode 100644 drivers/staging/media/omap4iss/TODO create mode 100644 drivers/staging/media/omap4iss/iss.c create mode 100644 drivers/staging/media/omap4iss/iss.h create mode 100644 drivers/staging/media/omap4iss/iss_csi2.c create mode 100644 drivers/staging/media/omap4iss/iss_csi2.h create mode 100644 drivers/staging/media/omap4iss/iss_csiphy.c create mode 100644 drivers/staging/media/omap4iss/iss_csiphy.h create mode 100644 drivers/staging/media/omap4iss/iss_ipipe.c create mode 100644 drivers/staging/media/omap4iss/iss_ipipe.h create mode 100644 drivers/staging/media/omap4iss/iss_ipipeif.c create mode 100644 drivers/staging/media/omap4iss/iss_ipipeif.h create mode 100644 drivers/staging/media/omap4iss/iss_regs.h create mode 100644 drivers/staging/media/omap4iss/iss_resizer.c create mode 100644 drivers/staging/media/omap4iss/iss_resizer.h create mode 100644 drivers/staging/media/omap4iss/iss_video.c create mode 100644 drivers/staging/media/omap4iss/iss_video.h create mode 100644 include/media/omap4iss.h -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] v4l: omap4iss: Add support for OMAP4 camera interface - CSI receivers
From: Sergio Aguirre
This adds a very simplistic driver to utilize the CSI2A interface inside
the ISS subsystem in OMAP4, and dump the data to memory.
Check Documentation/video4linux/omap4_camera.txt for details.
This commit adds CSI receivers support.
Signed-off-by: Sergio Aguirre
[Port the driver to v3.12-rc3, including the following changes
- Don't include plat/ headers
- Don't use cpu_is_omap44xx() macro
- Don't depend on EXPERIMENTAL
- Fix s_crop operation prototype
- Update link_notify prototype
- Rename media_entity_remote_source to media_entity_remote_pad]
Signed-off-by: Laurent Pinchart
---
drivers/staging/media/omap4iss/iss_csi2.c | 1368 +++
drivers/staging/media/omap4iss/iss_csi2.h | 156 +++
drivers/staging/media/omap4iss/iss_csiphy.c | 278 ++
drivers/staging/media/omap4iss/iss_csiphy.h | 51 +
4 files changed, 1853 insertions(+)
create mode 100644 drivers/staging/media/omap4iss/iss_csi2.c
create mode 100644 drivers/staging/media/omap4iss/iss_csi2.h
create mode 100644 drivers/staging/media/omap4iss/iss_csiphy.c
create mode 100644 drivers/staging/media/omap4iss/iss_csiphy.h
diff --git a/drivers/staging/media/omap4iss/iss_csi2.c
b/drivers/staging/media/omap4iss/iss_csi2.c
new file mode 100644
index 000..0ee8381
--- /dev/null
+++ b/drivers/staging/media/omap4iss/iss_csi2.c
@@ -0,0 +1,1368 @@
+/*
+ * TI OMAP4 ISS V4L2 Driver - CSI PHY module
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ *
+ * Author: Sergio Aguirre
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "iss.h"
+#include "iss_regs.h"
+#include "iss_csi2.h"
+
+/*
+ * csi2_if_enable - Enable CSI2 Receiver interface.
+ * @enable: enable flag
+ *
+ */
+static void csi2_if_enable(struct iss_csi2_device *csi2, u8 enable)
+{
+ struct iss_csi2_ctrl_cfg *currctrl = &csi2->ctrl;
+
+ writel((readl(csi2->regs1 + CSI2_CTRL) & ~CSI2_CTRL_IF_EN) |
+ (enable ? CSI2_CTRL_IF_EN : 0),
+ csi2->regs1 + CSI2_CTRL);
+
+ currctrl->if_enable = enable;
+}
+
+/*
+ * csi2_recv_config - CSI2 receiver module configuration.
+ * @currctrl: iss_csi2_ctrl_cfg structure
+ *
+ */
+static void csi2_recv_config(struct iss_csi2_device *csi2,
+struct iss_csi2_ctrl_cfg *currctrl)
+{
+ u32 reg = 0;
+
+ if (currctrl->frame_mode)
+ reg |= CSI2_CTRL_FRAME;
+ else
+ reg &= ~CSI2_CTRL_FRAME;
+
+ if (currctrl->vp_clk_enable)
+ reg |= CSI2_CTRL_VP_CLK_EN;
+ else
+ reg &= ~CSI2_CTRL_VP_CLK_EN;
+
+ if (currctrl->vp_only_enable)
+ reg |= CSI2_CTRL_VP_ONLY_EN;
+ else
+ reg &= ~CSI2_CTRL_VP_ONLY_EN;
+
+ reg &= ~CSI2_CTRL_VP_OUT_CTRL_MASK;
+ reg |= currctrl->vp_out_ctrl << CSI2_CTRL_VP_OUT_CTRL_SHIFT;
+
+ if (currctrl->ecc_enable)
+ reg |= CSI2_CTRL_ECC_EN;
+ else
+ reg &= ~CSI2_CTRL_ECC_EN;
+
+ /*
+* Set MFlag assertion boundaries to:
+* Low: 4/8 of FIFO size
+* High: 6/8 of FIFO size
+*/
+ reg &= ~(CSI2_CTRL_MFLAG_LEVH_MASK | CSI2_CTRL_MFLAG_LEVL_MASK);
+ reg |= (2 << CSI2_CTRL_MFLAG_LEVH_SHIFT) |
+ (4 << CSI2_CTRL_MFLAG_LEVL_SHIFT);
+
+ /* Generation of 16x64-bit bursts (Recommended) */
+ reg |= CSI2_CTRL_BURST_SIZE_EXPAND;
+
+ /* Do Non-Posted writes (Recommended) */
+ reg |= CSI2_CTRL_NON_POSTED_WRITE;
+
+ /*
+* Enforce Little endian for all formats, including:
+* YUV4:2:2 8-bit and YUV4:2:0 Legacy
+*/
+ reg |= CSI2_CTRL_ENDIANNESS;
+
+ writel(reg, csi2->regs1 + CSI2_CTRL);
+}
+
+static const unsigned int csi2_input_fmts[] = {
+ V4L2_MBUS_FMT_SGRBG10_1X10,
+ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
+ V4L2_MBUS_FMT_SRGGB10_1X10,
+ V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8,
+ V4L2_MBUS_FMT_SBGGR10_1X10,
+ V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8,
+ V4L2_MBUS_FMT_SGBRG10_1X10,
+ V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8,
+ V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_MBUS_FMT_SGBRG8_1X8,
+ V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_MBUS_FMT_SRGGB8_1X8,
+ V4L2_MBUS_FMT_UYVY8_1X16,
+ V4L2_MBUS_FMT_YUYV8_1X16,
+};
+
+/* To set the format on the CSI2 requires a mapping function that takes
+ * the following inputs:
+ * - 3 different formats (at this time)
+ * - 2 destinations (mem, vp+mem) (vp only handled separately)
+ * - 2 decompression options (on, off)
+ * Output should be CSI2 frame format code
+ * Array indices as follows: [format][dest][decompr]
+ * Not all combinations are valid. 0 means invalid.
+ */
+static const u16 __csi2_fmt_map[][2][2] = {
+ /
[PATCH 2/6] v4l: omap4iss: Add support for OMAP4 camera interface - Video devices
From: Sergio Aguirre
This adds a very simplistic driver to utilize the CSI2A interface inside
the ISS subsystem in OMAP4, and dump the data to memory.
Check Documentation/video4linux/omap4_camera.txt for details.
This commit adds video devices support.
Signed-off-by: Sergio Aguirre
[Port the driver to v3.12-rc3, including the following changes
- Don't include plat/ headers
- Don't use cpu_is_omap44xx() macro
- Don't depend on EXPERIMENTAL
- Fix s_crop operation prototype
- Update link_notify prototype
- Rename media_entity_remote_source to media_entity_remote_pad]
Signed-off-by: Laurent Pinchart
---
drivers/staging/media/omap4iss/iss_video.c | 1129
drivers/staging/media/omap4iss/iss_video.h | 201 +
2 files changed, 1330 insertions(+)
create mode 100644 drivers/staging/media/omap4iss/iss_video.c
create mode 100644 drivers/staging/media/omap4iss/iss_video.h
diff --git a/drivers/staging/media/omap4iss/iss_video.c
b/drivers/staging/media/omap4iss/iss_video.c
new file mode 100644
index 000..31f1b88
--- /dev/null
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -0,0 +1,1129 @@
+/*
+ * TI OMAP4 ISS V4L2 Driver - Generic video node
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ *
+ * Author: Sergio Aguirre
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "iss_video.h"
+#include "iss.h"
+
+
+/*
-
+ * Helper functions
+ */
+
+static struct iss_format_info formats[] = {
+ { V4L2_MBUS_FMT_Y8_1X8, V4L2_MBUS_FMT_Y8_1X8,
+ V4L2_MBUS_FMT_Y8_1X8, V4L2_MBUS_FMT_Y8_1X8,
+ V4L2_PIX_FMT_GREY, 8, },
+ { V4L2_MBUS_FMT_Y10_1X10, V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y10_1X10, V4L2_MBUS_FMT_Y8_1X8,
+ V4L2_PIX_FMT_Y10, 10, },
+ { V4L2_MBUS_FMT_Y12_1X12, V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y12_1X12, V4L2_MBUS_FMT_Y8_1X8,
+ V4L2_PIX_FMT_Y12, 12, },
+ { V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_PIX_FMT_SBGGR8, 8, },
+ { V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_MBUS_FMT_SGBRG8_1X8,
+ V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_MBUS_FMT_SGBRG8_1X8,
+ V4L2_PIX_FMT_SGBRG8, 8, },
+ { V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_PIX_FMT_SGRBG8, 8, },
+ { V4L2_MBUS_FMT_SRGGB8_1X8, V4L2_MBUS_FMT_SRGGB8_1X8,
+ V4L2_MBUS_FMT_SRGGB8_1X8, V4L2_MBUS_FMT_SRGGB8_1X8,
+ V4L2_PIX_FMT_SRGGB8, 8, },
+ { V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
+ V4L2_MBUS_FMT_SGRBG10_1X10, 0,
+ V4L2_PIX_FMT_SGRBG10DPCM8, 8, },
+ { V4L2_MBUS_FMT_SBGGR10_1X10, V4L2_MBUS_FMT_SBGGR10_1X10,
+ V4L2_MBUS_FMT_SBGGR10_1X10, V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_PIX_FMT_SBGGR10, 10, },
+ { V4L2_MBUS_FMT_SGBRG10_1X10, V4L2_MBUS_FMT_SGBRG10_1X10,
+ V4L2_MBUS_FMT_SGBRG10_1X10, V4L2_MBUS_FMT_SGBRG8_1X8,
+ V4L2_PIX_FMT_SGBRG10, 10, },
+ { V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_MBUS_FMT_SGRBG10_1X10,
+ V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_PIX_FMT_SGRBG10, 10, },
+ { V4L2_MBUS_FMT_SRGGB10_1X10, V4L2_MBUS_FMT_SRGGB10_1X10,
+ V4L2_MBUS_FMT_SRGGB10_1X10, V4L2_MBUS_FMT_SRGGB8_1X8,
+ V4L2_PIX_FMT_SRGGB10, 10, },
+ { V4L2_MBUS_FMT_SBGGR12_1X12, V4L2_MBUS_FMT_SBGGR10_1X10,
+ V4L2_MBUS_FMT_SBGGR12_1X12, V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_PIX_FMT_SBGGR12, 12, },
+ { V4L2_MBUS_FMT_SGBRG12_1X12, V4L2_MBUS_FMT_SGBRG10_1X10,
+ V4L2_MBUS_FMT_SGBRG12_1X12, V4L2_MBUS_FMT_SGBRG8_1X8,
+ V4L2_PIX_FMT_SGBRG12, 12, },
+ { V4L2_MBUS_FMT_SGRBG12_1X12, V4L2_MBUS_FMT_SGRBG10_1X10,
+ V4L2_MBUS_FMT_SGRBG12_1X12, V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_PIX_FMT_SGRBG12, 12, },
+ { V4L2_MBUS_FMT_SRGGB12_1X12, V4L2_MBUS_FMT_SRGGB10_1X10,
+ V4L2_MBUS_FMT_SRGGB12_1X12, V4L2_MBUS_FMT_SRGGB8_1X8,
+ V4L2_PIX_FMT_SRGGB12, 12, },
+ { V4L2_MBUS_FMT_UYVY8_1X16, V4L2_MBUS_FMT_UYVY8_1X16,
+ V4L2_MBUS_FMT_UYVY8_1X16, 0,
+ V4L2_PIX_FMT_UYVY, 16, },
+ { V4L2_MBUS_FMT_YUYV8_1X16, V4L2_MBUS_FMT_YUYV8_1X16,
+ V4L2_MBUS_FMT_YUYV8_1X16, 0,
+ V4L2_PIX_FMT_YUYV, 16, },
+ { V4L2_MBUS_FMT_YUYV8_1_5X8, V4L2_MBUS_FMT_YUYV8_1_5X8,
+ V4L2_MBUS_FMT_YUYV8_1_5X8, 0,
+ V4L2_PIX_FMT_NV12, 8, },
+};
+
+const struct iss_format_info *
+omap4iss_video_format_info(enum v4l2_mbus_pixelcode code)
+{
+ unsigned int i;
+
[PATCH 4/6] v4l: omap4iss: Add support for OMAP4 camera interface - IPIPE(IF)
From: Sergio Aguirre
This adds a very simplistic driver to utilize the CSI2A interface inside
the ISS subsystem in OMAP4, and dump the data to memory.
Check Documentation/video4linux/omap4_camera.txt for details.
This commit adds the IPIPEIF and IPIPE processing blocks support.
Signed-off-by: Sergio Aguirre
[Port the driver to v3.12-rc3, including the following changes
- Don't include plat/ headers
- Don't use cpu_is_omap44xx() macro
- Don't depend on EXPERIMENTAL
- Fix s_crop operation prototype
- Update link_notify prototype
- Rename media_entity_remote_source to media_entity_remote_pad]
Signed-off-by: Laurent Pinchart
---
drivers/staging/media/omap4iss/iss_ipipe.c | 581 ++
drivers/staging/media/omap4iss/iss_ipipe.h | 67 +++
drivers/staging/media/omap4iss/iss_ipipeif.c | 847 +++
drivers/staging/media/omap4iss/iss_ipipeif.h | 92 +++
4 files changed, 1587 insertions(+)
create mode 100644 drivers/staging/media/omap4iss/iss_ipipe.c
create mode 100644 drivers/staging/media/omap4iss/iss_ipipe.h
create mode 100644 drivers/staging/media/omap4iss/iss_ipipeif.c
create mode 100644 drivers/staging/media/omap4iss/iss_ipipeif.h
diff --git a/drivers/staging/media/omap4iss/iss_ipipe.c
b/drivers/staging/media/omap4iss/iss_ipipe.c
new file mode 100644
index 000..fc38a5c5
--- /dev/null
+++ b/drivers/staging/media/omap4iss/iss_ipipe.c
@@ -0,0 +1,581 @@
+/*
+ * TI OMAP4 ISS V4L2 Driver - ISP IPIPE module
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ *
+ * Author: Sergio Aguirre
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "iss.h"
+#include "iss_regs.h"
+#include "iss_ipipe.h"
+
+static struct v4l2_mbus_framefmt *
+__ipipe_get_format(struct iss_ipipe_device *ipipe, struct v4l2_subdev_fh *fh,
+ unsigned int pad, enum v4l2_subdev_format_whence which);
+
+static const unsigned int ipipe_fmts[] = {
+ V4L2_MBUS_FMT_SGRBG10_1X10,
+ V4L2_MBUS_FMT_SRGGB10_1X10,
+ V4L2_MBUS_FMT_SBGGR10_1X10,
+ V4L2_MBUS_FMT_SGBRG10_1X10,
+};
+
+/*
+ * ipipe_print_status - Print current IPIPE Module register values.
+ * @ipipe: Pointer to ISS ISP IPIPE device.
+ *
+ * Also prints other debug information stored in the IPIPE module.
+ */
+#define IPIPE_PRINT_REGISTER(iss, name)\
+ dev_dbg(iss->dev, "###IPIPE " #name "=0x%08x\n", \
+ readl(iss->regs[OMAP4_ISS_MEM_ISP_IPIPE] + IPIPE_##name))
+
+static void ipipe_print_status(struct iss_ipipe_device *ipipe)
+{
+ struct iss_device *iss = to_iss_device(ipipe);
+
+ dev_dbg(iss->dev, "-IPIPE Register dump-\n");
+
+ IPIPE_PRINT_REGISTER(iss, SRC_EN);
+ IPIPE_PRINT_REGISTER(iss, SRC_MODE);
+ IPIPE_PRINT_REGISTER(iss, SRC_FMT);
+ IPIPE_PRINT_REGISTER(iss, SRC_COL);
+ IPIPE_PRINT_REGISTER(iss, SRC_VPS);
+ IPIPE_PRINT_REGISTER(iss, SRC_VSZ);
+ IPIPE_PRINT_REGISTER(iss, SRC_HPS);
+ IPIPE_PRINT_REGISTER(iss, SRC_HSZ);
+ IPIPE_PRINT_REGISTER(iss, GCK_MMR);
+ IPIPE_PRINT_REGISTER(iss, YUV_PHS);
+
+ dev_dbg(iss->dev, "---\n");
+}
+
+/*
+ * ipipe_enable - Enable/Disable IPIPE.
+ * @enable: enable flag
+ *
+ */
+static void ipipe_enable(struct iss_ipipe_device *ipipe, u8 enable)
+{
+ struct iss_device *iss = to_iss_device(ipipe);
+
+ writel((readl(iss->regs[OMAP4_ISS_MEM_ISP_IPIPE] + IPIPE_SRC_EN) &
+ ~IPIPE_SRC_EN_EN) |
+ enable ? IPIPE_SRC_EN_EN : 0,
+ iss->regs[OMAP4_ISS_MEM_ISP_IPIPE] + IPIPE_SRC_EN);
+}
+
+/*
-
+ * Format- and pipeline-related configuration helpers
+ */
+
+static void ipipe_configure(struct iss_ipipe_device *ipipe)
+{
+ struct iss_device *iss = to_iss_device(ipipe);
+ struct v4l2_mbus_framefmt *format;
+
+ /* IPIPE_PAD_SINK */
+ format = &ipipe->formats[IPIPE_PAD_SINK];
+
+ /* NOTE: Currently just supporting pipeline IN: RGB, OUT: YUV422 */
+ writel(IPIPE_SRC_FMT_RAW2YUV,
+ iss->regs[OMAP4_ISS_MEM_ISP_IPIPE] + IPIPE_SRC_FMT);
+
+ /* Enable YUV444 -> YUV422 conversion */
+ writel(IPIPE_YUV_PHS_LPF,
+ iss->regs[OMAP4_ISS_MEM_ISP_IPIPE] + IPIPE_YUV_PHS);
+
+ writel(0, iss->regs[OMAP4_ISS_MEM_ISP_IPIPE] + IPIPE_SRC_VPS);
+ writel(0, iss->regs[OMAP4_ISS_MEM_ISP_IPIPE] + IPIPE_SRC_HPS);
+ writel((format->height - 2) & IPIPE_SRC_VSZ_MASK,
+ iss->regs[OMAP4_ISS_MEM_ISP_IPIPE] + IPIPE_SRC_VSZ);
+ writel((format->width - 1) & IPIPE_SRC_HSZ_MASK,
+ iss->regs[OMAP4_ISS_MEM_IS
[PATCH 6/6] v4l: omap4iss: Add support for OMAP4 camera interface - Build system
This adds a very simplistic driver to utilize the CSI2A interface inside the ISS subsystem in OMAP4, and dump the data to memory. Check Documentation/video4linux/omap4_camera.txt for details. This commit adds and updates Kconfig's and Makefile's, as well as a TODO list. Signed-off-by: Laurent Pinchart --- drivers/staging/media/Kconfig | 2 ++ drivers/staging/media/Makefile | 1 + drivers/staging/media/omap4iss/Kconfig | 12 drivers/staging/media/omap4iss/Makefile | 6 ++ drivers/staging/media/omap4iss/TODO | 4 5 files changed, 25 insertions(+) create mode 100644 drivers/staging/media/omap4iss/Kconfig create mode 100644 drivers/staging/media/omap4iss/Makefile create mode 100644 drivers/staging/media/omap4iss/TODO diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig index 46f1e61..bc4c798 100644 --- a/drivers/staging/media/Kconfig +++ b/drivers/staging/media/Kconfig @@ -33,6 +33,8 @@ source "drivers/staging/media/msi3101/Kconfig" source "drivers/staging/media/solo6x10/Kconfig" +source "drivers/staging/media/omap4iss/Kconfig" + # Keep LIRC at the end, as it has sub-menus source "drivers/staging/media/lirc/Kconfig" diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile index eb7f30b..a528d3f 100644 --- a/drivers/staging/media/Makefile +++ b/drivers/staging/media/Makefile @@ -6,3 +6,4 @@ obj-$(CONFIG_VIDEO_DT3155) += dt3155v4l/ obj-$(CONFIG_VIDEO_GO7007) += go7007/ obj-$(CONFIG_USB_MSI3101) += msi3101/ obj-$(CONFIG_VIDEO_DM365_VPFE) += davinci_vpfe/ +obj-$(CONFIG_VIDEO_OMAP4) += omap4iss/ diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig new file mode 100644 index 000..b9fe753 --- /dev/null +++ b/drivers/staging/media/omap4iss/Kconfig @@ -0,0 +1,12 @@ +config VIDEO_OMAP4 + bool "OMAP 4 Camera support" + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4 + select VIDEOBUF2_DMA_CONTIG + ---help--- + Driver for an OMAP 4 ISS controller. + +config VIDEO_OMAP4_DEBUG + bool "OMAP 4 Camera debug messages" + depends on VIDEO_OMAP4 + ---help--- + Enable debug messages on OMAP 4 ISS controller driver. diff --git a/drivers/staging/media/omap4iss/Makefile b/drivers/staging/media/omap4iss/Makefile new file mode 100644 index 000..a716ce9 --- /dev/null +++ b/drivers/staging/media/omap4iss/Makefile @@ -0,0 +1,6 @@ +# Makefile for OMAP4 ISS driver + +omap4-iss-objs += \ + iss.o iss_csi2.o iss_csiphy.o iss_ipipeif.o iss_ipipe.o iss_resizer.o iss_video.o + +obj-$(CONFIG_VIDEO_OMAP4) += omap4-iss.o diff --git a/drivers/staging/media/omap4iss/TODO b/drivers/staging/media/omap4iss/TODO new file mode 100644 index 000..fcde888 --- /dev/null +++ b/drivers/staging/media/omap4iss/TODO @@ -0,0 +1,4 @@ +* Make the driver compile as a module +* Fix FIFO/buffer overflows and underflows +* Replace dummy resizer code with a real implementation +* Fix checkpatch errors and warnings -- 1.8.1.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: [PATCH v2 1/4] media: Add pad flag MEDIA_PAD_FL_MUST_CONNECT
Hi Sakari,
Thank you for the patch.
On Thursday 03 October 2013 02:17:50 Sakari Ailus wrote:
> Pads that set this flag must be connected by an active link for the entity
> to stream.
>
> Signed-off-by: Sakari Ailus
> Acked-by: Sylwester Nawrocki
Acked-by: Laurent Pinchart
> ---
> Documentation/DocBook/media/v4l/media-ioc-enum-links.xml | 10 ++
> include/uapi/linux/media.h |1 +
> 2 files changed, 11 insertions(+)
>
> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
> b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml index
> 355df43..e357dc9 100644
> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
> @@ -134,6 +134,16 @@
> Output pad, relative to the entity. Output pads source data
> and are origins of links.
>
> +
> + MEDIA_PAD_FL_MUST_CONNECT
> + If this flag is set and the pad is linked to any other
> + pad, then at least one of those links must be enabled for the
> + entity to be able to stream. There could be temporary reasons
> + (e.g. device configuration dependent) for the pad to need
> + enabled links even when this flag isn't set; the absence of the
> + flag doesn't imply there is none. The flag has no effect on pads
> + without connected links.
> +
>
>
>
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index ed49574..d847c76 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -98,6 +98,7 @@ struct media_entity_desc {
>
> #define MEDIA_PAD_FL_SINK(1 << 0)
> #define MEDIA_PAD_FL_SOURCE (1 << 1)
> +#define MEDIA_PAD_FL_MUST_CONNECT(1 << 2)
>
> struct media_pad_desc {
> __u32 entity; /* entity ID */
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod.
On 03.10.2013 01:54, Luis Alves wrote: I Antti, I think it's safe to use because the hybrid_tuner_request_state will make sure that the i2c_adapter_id is the same for both demods. I also looked that tuner-i2c.h hybrid_tuner_request_state() and if I am not misunderstanding the criteria to return state is i2c adapter id and i2c slave address. Yea, it seems to be correct even there is quad card used as slave address must be different in that case! regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] omap3isp: Add resizer data rate configuration to resizer_link_validate
The configuration of many other blocks depend on resizer maximum data rate.
Get the value from resizer at link validation time.
Signed-off-by: Sakari Ailus
Acked-by: Laurent Pinchart
---
drivers/media/platform/omap3isp/ispresizer.c | 15 +++
drivers/media/platform/omap3isp/ispvideo.c | 54 --
2 files changed, 15 insertions(+), 54 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispresizer.c
b/drivers/media/platform/omap3isp/ispresizer.c
index fa35f2c..0d36b8b 100644
--- a/drivers/media/platform/omap3isp/ispresizer.c
+++ b/drivers/media/platform/omap3isp/ispresizer.c
@@ -1532,6 +1532,20 @@ static int resizer_set_format(struct v4l2_subdev *sd,
struct v4l2_subdev_fh *fh,
return 0;
}
+static int resizer_link_validate(struct v4l2_subdev *sd,
+struct media_link *link,
+struct v4l2_subdev_format *source_fmt,
+struct v4l2_subdev_format *sink_fmt)
+{
+ struct isp_res_device *res = v4l2_get_subdevdata(sd);
+ struct isp_pipeline *pipe = to_isp_pipeline(&sd->entity);
+
+ omap3isp_resizer_max_rate(res, &pipe->max_rate);
+
+ return v4l2_subdev_link_validate_default(sd, link,
+source_fmt, sink_fmt);
+}
+
/*
* resizer_init_formats - Initialize formats on all pads
* @sd: ISP resizer V4L2 subdevice
@@ -1570,6 +1584,7 @@ static const struct v4l2_subdev_pad_ops
resizer_v4l2_pad_ops = {
.set_fmt = resizer_set_format,
.get_selection = resizer_get_selection,
.set_selection = resizer_set_selection,
+ .link_validate = resizer_link_validate,
};
/* subdev operations */
diff --git a/drivers/media/platform/omap3isp/ispvideo.c
b/drivers/media/platform/omap3isp/ispvideo.c
index d45af5c..9319e4d 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -278,55 +278,6 @@ static int isp_video_get_graph_data(struct isp_video
*video,
return 0;
}
-/*
- * Validate a pipeline by checking both ends of all links for format
- * discrepancies.
- *
- * Compute the minimum time per frame value as the maximum of time per frame
- * limits reported by every block in the pipeline.
- *
- * Return 0 if all formats match, or -EPIPE if at least one link is found with
- * different formats on its two ends or if the pipeline doesn't start with a
- * video source (either a subdev with no input pad, or a non-subdev entity).
- */
-static int isp_video_validate_pipeline(struct isp_pipeline *pipe)
-{
- struct isp_device *isp = pipe->output->isp;
- struct media_pad *pad;
- struct v4l2_subdev *subdev;
-
- subdev = isp_video_remote_subdev(pipe->output, NULL);
- if (subdev == NULL)
- return -EPIPE;
-
- while (1) {
- /* Retrieve the sink format */
- pad = &subdev->entity.pads[0];
- if (!(pad->flags & MEDIA_PAD_FL_SINK))
- break;
-
- /* Update the maximum frame rate */
- if (subdev == &isp->isp_res.subdev)
- omap3isp_resizer_max_rate(&isp->isp_res,
- &pipe->max_rate);
-
- /* Retrieve the source format. Return an error if no source
-* entity can be found, and stop checking the pipeline if the
-* source entity isn't a subdev.
-*/
- pad = media_entity_remote_pad(pad);
- if (pad == NULL)
- return -EPIPE;
-
- if (media_entity_type(pad->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
- break;
-
- subdev = media_entity_to_v4l2_subdev(pad->entity);
- }
-
- return 0;
-}
-
static int
__isp_video_get_format(struct isp_video *video, struct v4l2_format *format)
{
@@ -1054,11 +1005,6 @@ isp_video_streamon(struct file *file, void *fh, enum
v4l2_buf_type type)
if (ret < 0)
goto err_check_format;
- /* Validate the pipeline and update its state. */
- ret = isp_video_validate_pipeline(pipe);
- if (ret < 0)
- goto err_check_format;
-
pipe->error = false;
spin_lock_irqsave(&pipe->lock, flags);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] media: Add pad flag MEDIA_PAD_FL_MUST_CONNECT
Pads that set this flag must be connected by an active link for the entity
to stream.
Signed-off-by: Sakari Ailus
Acked-by: Sylwester Nawrocki
---
Documentation/DocBook/media/v4l/media-ioc-enum-links.xml | 10 ++
include/uapi/linux/media.h |1 +
2 files changed, 11 insertions(+)
diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
index 355df43..e357dc9 100644
--- a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
+++ b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
@@ -134,6 +134,16 @@
Output pad, relative to the entity. Output pads source data
and are origins of links.
+
+ MEDIA_PAD_FL_MUST_CONNECT
+ If this flag is set and the pad is linked to any other
+ pad, then at least one of those links must be enabled for the
+ entity to be able to stream. There could be temporary reasons
+ (e.g. device configuration dependent) for the pad to need
+ enabled links even when this flag isn't set; the absence of the
+ flag doesn't imply there is none. The flag has no effect on pads
+ without connected links.
+
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index ed49574..d847c76 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -98,6 +98,7 @@ struct media_entity_desc {
#define MEDIA_PAD_FL_SINK (1 << 0)
#define MEDIA_PAD_FL_SOURCE(1 << 1)
+#define MEDIA_PAD_FL_MUST_CONNECT (1 << 2)
struct media_pad_desc {
__u32 entity; /* entity ID */
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] media: Check for active links on pads with MEDIA_PAD_FL_MUST_CONNECT flag
Do not allow streaming if a pad with MEDIA_PAD_FL_MUST_CONNECT flag is not
connected by an active link.
This patch makes it possible to avoid drivers having to check for the most
common case of link state validation: a sink pad that must be connected.
Signed-off-by: Sakari Ailus
Tested-by: Sylwester Nawrocki
Acked-by: Laurent Pinchart
---
drivers/media/media-entity.c | 41 ++---
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 2c286c3..37c334e 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -235,6 +235,8 @@ __must_check int media_entity_pipeline_start(struct
media_entity *entity,
media_entity_graph_walk_start(&graph, entity);
while ((entity = media_entity_graph_walk_next(&graph))) {
+ DECLARE_BITMAP(active, entity->num_pads);
+ DECLARE_BITMAP(has_no_links, entity->num_pads);
unsigned int i;
entity->stream_count++;
@@ -248,21 +250,46 @@ __must_check int media_entity_pipeline_start(struct
media_entity *entity,
if (!entity->ops || !entity->ops->link_validate)
continue;
+ bitmap_zero(active, entity->num_pads);
+ bitmap_fill(has_no_links, entity->num_pads);
+
for (i = 0; i < entity->num_links; i++) {
struct media_link *link = &entity->links[i];
-
- /* Is this pad part of an enabled link? */
- if (!(link->flags & MEDIA_LNK_FL_ENABLED))
- continue;
-
- /* Are we the sink or not? */
- if (link->sink->entity != entity)
+ struct media_pad *pad = link->sink->entity == entity
+ ? link->sink : link->source;
+
+ /* Mark that a pad is connected by a link. */
+ bitmap_clear(has_no_links, pad->index, 1);
+
+ /*
+* Pads that either do not need to connect or
+* are connected through an enabled link are
+* fine.
+*/
+ if (!(pad->flags & MEDIA_PAD_FL_MUST_CONNECT) ||
+ link->flags & MEDIA_LNK_FL_ENABLED)
+ bitmap_set(active, pad->index, 1);
+
+ /*
+* Link validation will only take place for
+* sink ends of the link that are enabled.
+*/
+ if (link->sink != pad ||
+ !(link->flags & MEDIA_LNK_FL_ENABLED))
continue;
ret = entity->ops->link_validate(link);
if (ret < 0 && ret != -ENOIOCTLCMD)
goto error;
}
+
+ /* Either no links or validated links are fine. */
+ bitmap_or(active, active, has_no_links, entity->num_pads);
+
+ if (!bitmap_full(active, entity->num_pads)) {
+ ret = -EPIPE;
+ goto error;
+ }
}
mutex_unlock(&mdev->graph_mutex);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/4] Add MEDIA_PAD_FL_MUST_CONNECT pad flag, check it
Hi all, This is the second version of the patchset that adds a new media pad flag to denote that the pad must be connected by an enabled link for the entity to be able to stream. The previous version of the set is available here: http://www.spinics.net/lists/linux-media/msg68105.html> What has changed is - Indentation of the pad flags for the omap3isp driver and assignment of pad in media_entity_pipeline_start() and - Improved the pad flag documentation. Laurent: unless there further comments to the patches, could you take them to your tree as they contain changes to the Media controller framework and the omap3isp driver? -- Kind regards, Sakari -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] omap3isp: Mark which pads must connect
Mark pads that must be connected. Signed-off-by: Sakari Ailus Acked-by: Laurent Pinchart --- drivers/media/platform/omap3isp/ispccdc.c|3 ++- drivers/media/platform/omap3isp/ispccp2.c|3 ++- drivers/media/platform/omap3isp/ispcsi2.c|3 ++- drivers/media/platform/omap3isp/isppreview.c |3 ++- drivers/media/platform/omap3isp/ispresizer.c |3 ++- drivers/media/platform/omap3isp/ispstat.c|2 +- drivers/media/platform/omap3isp/ispvideo.c |6 -- 7 files changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/media/platform/omap3isp/ispccdc.c b/drivers/media/platform/omap3isp/ispccdc.c index 907a205..561c991 100644 --- a/drivers/media/platform/omap3isp/ispccdc.c +++ b/drivers/media/platform/omap3isp/ispccdc.c @@ -2484,7 +2484,8 @@ static int ccdc_init_entities(struct isp_ccdc_device *ccdc) v4l2_set_subdevdata(sd, ccdc); sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS | V4L2_SUBDEV_FL_HAS_DEVNODE; - pads[CCDC_PAD_SINK].flags = MEDIA_PAD_FL_SINK; + pads[CCDC_PAD_SINK].flags = MEDIA_PAD_FL_SINK + | MEDIA_PAD_FL_MUST_CONNECT; pads[CCDC_PAD_SOURCE_VP].flags = MEDIA_PAD_FL_SOURCE; pads[CCDC_PAD_SOURCE_OF].flags = MEDIA_PAD_FL_SOURCE; diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c index e716514..e84fe05 100644 --- a/drivers/media/platform/omap3isp/ispccp2.c +++ b/drivers/media/platform/omap3isp/ispccp2.c @@ -1076,7 +1076,8 @@ static int ccp2_init_entities(struct isp_ccp2_device *ccp2) v4l2_set_subdevdata(sd, ccp2); sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; - pads[CCP2_PAD_SINK].flags = MEDIA_PAD_FL_SINK; + pads[CCP2_PAD_SINK].flags = MEDIA_PAD_FL_SINK + | MEDIA_PAD_FL_MUST_CONNECT; pads[CCP2_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE; me->ops = &ccp2_media_ops; diff --git a/drivers/media/platform/omap3isp/ispcsi2.c b/drivers/media/platform/omap3isp/ispcsi2.c index 6db245d..6205608 100644 --- a/drivers/media/platform/omap3isp/ispcsi2.c +++ b/drivers/media/platform/omap3isp/ispcsi2.c @@ -1245,7 +1245,8 @@ static int csi2_init_entities(struct isp_csi2_device *csi2) sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; pads[CSI2_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE; - pads[CSI2_PAD_SINK].flags = MEDIA_PAD_FL_SINK; + pads[CSI2_PAD_SINK].flags = MEDIA_PAD_FL_SINK + | MEDIA_PAD_FL_MUST_CONNECT; me->ops = &csi2_media_ops; ret = media_entity_init(me, CSI2_PADS_NUM, pads, 0); diff --git a/drivers/media/platform/omap3isp/isppreview.c b/drivers/media/platform/omap3isp/isppreview.c index cd8831a..1c776c1 100644 --- a/drivers/media/platform/omap3isp/isppreview.c +++ b/drivers/media/platform/omap3isp/isppreview.c @@ -2283,7 +2283,8 @@ static int preview_init_entities(struct isp_prev_device *prev) v4l2_ctrl_handler_setup(&prev->ctrls); sd->ctrl_handler = &prev->ctrls; - pads[PREV_PAD_SINK].flags = MEDIA_PAD_FL_SINK; + pads[PREV_PAD_SINK].flags = MEDIA_PAD_FL_SINK + | MEDIA_PAD_FL_MUST_CONNECT; pads[PREV_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE; me->ops = &preview_media_ops; diff --git a/drivers/media/platform/omap3isp/ispresizer.c b/drivers/media/platform/omap3isp/ispresizer.c index d11fb26..fa35f2c 100644 --- a/drivers/media/platform/omap3isp/ispresizer.c +++ b/drivers/media/platform/omap3isp/ispresizer.c @@ -1701,7 +1701,8 @@ static int resizer_init_entities(struct isp_res_device *res) v4l2_set_subdevdata(sd, res); sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; - pads[RESZ_PAD_SINK].flags = MEDIA_PAD_FL_SINK; + pads[RESZ_PAD_SINK].flags = MEDIA_PAD_FL_SINK + | MEDIA_PAD_FL_MUST_CONNECT; pads[RESZ_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE; me->ops = &resizer_media_ops; diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c index 61e17f9..a75407c 100644 --- a/drivers/media/platform/omap3isp/ispstat.c +++ b/drivers/media/platform/omap3isp/ispstat.c @@ -1067,7 +1067,7 @@ static int isp_stat_init_entities(struct ispstat *stat, const char *name, subdev->flags |= V4L2_SUBDEV_FL_HAS_EVENTS | V4L2_SUBDEV_FL_HAS_DEVNODE; v4l2_set_subdevdata(subdev, stat); - stat->pad.flags = MEDIA_PAD_FL_SINK; + stat->pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT; me->ops = NULL; return media_entity_init(me, 1, &stat->pad, 0); diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c index a908d00..d45af5c 100644 --- a/drivers/media/platform/omap3isp/ispvideo.c +++ b/drivers/media/platform/omap3isp/ispvideo.c @@ -1335,11 +1335,13 @@ int omap3isp_video_init(struct isp_video *video, const char *name) switch (video
Re: [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod.
I Antti,
I think it's safe to use because the hybrid_tuner_request_state will
make sure that the i2c_adapter_id is the same for both demods.
On the other hand, I think I need to re-send this changes as one single file.
Regards,
Luis
2013/10/2 Antti Palosaari :
> On 03.10.2013 01:09, Luis Alves wrote:
>>
>> Hi Mike,
>>
>> It's done (also tested and apparently working good)!
>>
>> I didn't know if two separated patches were needed (one for the cx24117
>> and the other for the cx23885) but I've splited it.
>> As you pointed out, this series of patches are to be used against your
>> cx24117 branch.
>>
>> Regards,
>> Luis
>>
>> Signed-off-by: Luis Alves
>
>
> I am not very familiar how that hybrid tuner request works, but it seems to
> be based of driver global list.
>
> I wonder if that works as it should. What happens when you have two cx24117
> demods equipped, having total of 4 frontends? Does it block access only for
> one demod at the time (as it should block only one per chip)?
>
> regards
> Antti
>
>
>
>> ---
>> drivers/media/dvb-frontends/cx24117.c | 72
>> +++--
>> drivers/media/dvb-frontends/cx24117.h |4 +-
>> 2 files changed, 53 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/media/dvb-frontends/cx24117.c
>> b/drivers/media/dvb-frontends/cx24117.c
>> index 3b63913..9087309 100644
>> --- a/drivers/media/dvb-frontends/cx24117.c
>> +++ b/drivers/media/dvb-frontends/cx24117.c
>> @@ -31,6 +31,7 @@
>> #include
>> #include
>>
>> +#include "tuner-i2c.h"
>> #include "dvb_frontend.h"
>> #include "cx24117.h"
>>
>> @@ -145,6 +146,9 @@ enum cmds {
>> CMD_TUNERSLEEP = 0x36,
>> };
>>
>> +static LIST_HEAD(hybrid_tuner_instance_list);
>> +static DEFINE_MUTEX(cx24117_list_mutex);
>> +
>> /* The Demod/Tuner can't easily provide these, we cache them */
>> struct cx24117_tuning {
>> u32 frequency;
>> @@ -176,9 +180,11 @@ struct cx24117_priv {
>> u8 demod_address;
>> struct i2c_adapter *i2c;
>> u8 skip_fw_load;
>> -
>> struct mutex fe_lock;
>> - atomic_t fe_nr;
>> +
>> + /* Used for sharing this struct between demods */
>> + struct tuner_i2c_props i2c_props;
>> + struct list_head hybrid_tuner_instance_list;
>> };
>>
>> /* one per each fe */
>> @@ -536,7 +542,7 @@ static int cx24117_load_firmware(struct dvb_frontend
>> *fe,
>> dev_dbg(&state->priv->i2c->dev,
>> "%s() demod%d FW is %zu bytes (%02x %02x .. %02x %02x)\n",
>> __func__, state->demod, fw->size, fw->data[0],
>> fw->data[1],
>> - fw->data[fw->size-2], fw->data[fw->size-1]);
>> + fw->data[fw->size - 2], fw->data[fw->size - 1]);
>>
>> cx24117_writereg(state, 0xea, 0x00);
>> cx24117_writereg(state, 0xea, 0x01);
>> @@ -1116,37 +1122,64 @@ static int cx24117_diseqc_send_burst(struct
>> dvb_frontend *fe,
>> return 0;
>> }
>>
>> +static int cx24117_get_priv(struct cx24117_priv **priv,
>> + struct i2c_adapter *i2c, u8 client_address)
>> +{
>> + int ret;
>> +
>> + mutex_lock(&cx24117_list_mutex);
>> + ret = hybrid_tuner_request_state(struct cx24117_priv, (*priv),
>> + hybrid_tuner_instance_list, i2c, client_address,
>> "cx24117");
>> + mutex_unlock(&cx24117_list_mutex);
>> +
>> + return ret;
>> +}
>> +
>> +static void cx24117_release_priv(struct cx24117_priv *priv)
>> +{
>> + mutex_lock(&cx24117_list_mutex);
>> + if (priv != NULL)
>> + hybrid_tuner_release_state(priv);
>> + mutex_unlock(&cx24117_list_mutex);
>> +}
>> +
>> static void cx24117_release(struct dvb_frontend *fe)
>> {
>> struct cx24117_state *state = fe->demodulator_priv;
>> dev_dbg(&state->priv->i2c->dev, "%s demod%d\n",
>> __func__, state->demod);
>> - if (!atomic_dec_and_test(&state->priv->fe_nr))
>> - kfree(state->priv);
>> + cx24117_release_priv(state->priv);
>> kfree(state);
>> }
>>
>> static struct dvb_frontend_ops cx24117_ops;
>>
>> struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
>> - struct i2c_adapter *i2c, struct dvb_frontend *fe)
>> + struct i2c_adapter *i2c)
>> {
>> struct cx24117_state *state = NULL;
>> struct cx24117_priv *priv = NULL;
>> int demod = 0;
>>
>> - /* first frontend attaching */
>> - /* allocate shared priv struct */
>> - if (fe == NULL) {
>> - priv = kzalloc(sizeof(struct cx24117_priv), GFP_KERNEL);
>> - if (priv == NULL)
>> - goto error1;
>> + /* get the common data struct for both demods */
>> + demod = cx24117_get_priv(&priv, i2c, config->demod_address);
>> +
>> + switch (demod) {
>> + case 0:
>> + dev_err(&state->priv->i2c->dev,
>> + "%s: Error attaching frontend %d\n",
>> + KB
Re: [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod.
On 03.10.2013 01:09, Luis Alves wrote:
Hi Mike,
It's done (also tested and apparently working good)!
I didn't know if two separated patches were needed (one for the cx24117 and the
other for the cx23885) but I've splited it.
As you pointed out, this series of patches are to be used against your cx24117
branch.
Regards,
Luis
Signed-off-by: Luis Alves
I am not very familiar how that hybrid tuner request works, but it seems
to be based of driver global list.
I wonder if that works as it should. What happens when you have two
cx24117 demods equipped, having total of 4 frontends? Does it block
access only for one demod at the time (as it should block only one per
chip)?
regards
Antti
---
drivers/media/dvb-frontends/cx24117.c | 72 +++--
drivers/media/dvb-frontends/cx24117.h |4 +-
2 files changed, 53 insertions(+), 23 deletions(-)
diff --git a/drivers/media/dvb-frontends/cx24117.c
b/drivers/media/dvb-frontends/cx24117.c
index 3b63913..9087309 100644
--- a/drivers/media/dvb-frontends/cx24117.c
+++ b/drivers/media/dvb-frontends/cx24117.c
@@ -31,6 +31,7 @@
#include
#include
+#include "tuner-i2c.h"
#include "dvb_frontend.h"
#include "cx24117.h"
@@ -145,6 +146,9 @@ enum cmds {
CMD_TUNERSLEEP = 0x36,
};
+static LIST_HEAD(hybrid_tuner_instance_list);
+static DEFINE_MUTEX(cx24117_list_mutex);
+
/* The Demod/Tuner can't easily provide these, we cache them */
struct cx24117_tuning {
u32 frequency;
@@ -176,9 +180,11 @@ struct cx24117_priv {
u8 demod_address;
struct i2c_adapter *i2c;
u8 skip_fw_load;
-
struct mutex fe_lock;
- atomic_t fe_nr;
+
+ /* Used for sharing this struct between demods */
+ struct tuner_i2c_props i2c_props;
+ struct list_head hybrid_tuner_instance_list;
};
/* one per each fe */
@@ -536,7 +542,7 @@ static int cx24117_load_firmware(struct dvb_frontend *fe,
dev_dbg(&state->priv->i2c->dev,
"%s() demod%d FW is %zu bytes (%02x %02x .. %02x %02x)\n",
__func__, state->demod, fw->size, fw->data[0], fw->data[1],
- fw->data[fw->size-2], fw->data[fw->size-1]);
+ fw->data[fw->size - 2], fw->data[fw->size - 1]);
cx24117_writereg(state, 0xea, 0x00);
cx24117_writereg(state, 0xea, 0x01);
@@ -1116,37 +1122,64 @@ static int cx24117_diseqc_send_burst(struct
dvb_frontend *fe,
return 0;
}
+static int cx24117_get_priv(struct cx24117_priv **priv,
+ struct i2c_adapter *i2c, u8 client_address)
+{
+ int ret;
+
+ mutex_lock(&cx24117_list_mutex);
+ ret = hybrid_tuner_request_state(struct cx24117_priv, (*priv),
+ hybrid_tuner_instance_list, i2c, client_address, "cx24117");
+ mutex_unlock(&cx24117_list_mutex);
+
+ return ret;
+}
+
+static void cx24117_release_priv(struct cx24117_priv *priv)
+{
+ mutex_lock(&cx24117_list_mutex);
+ if (priv != NULL)
+ hybrid_tuner_release_state(priv);
+ mutex_unlock(&cx24117_list_mutex);
+}
+
static void cx24117_release(struct dvb_frontend *fe)
{
struct cx24117_state *state = fe->demodulator_priv;
dev_dbg(&state->priv->i2c->dev, "%s demod%d\n",
__func__, state->demod);
- if (!atomic_dec_and_test(&state->priv->fe_nr))
- kfree(state->priv);
+ cx24117_release_priv(state->priv);
kfree(state);
}
static struct dvb_frontend_ops cx24117_ops;
struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
- struct i2c_adapter *i2c, struct dvb_frontend *fe)
+ struct i2c_adapter *i2c)
{
struct cx24117_state *state = NULL;
struct cx24117_priv *priv = NULL;
int demod = 0;
- /* first frontend attaching */
- /* allocate shared priv struct */
- if (fe == NULL) {
- priv = kzalloc(sizeof(struct cx24117_priv), GFP_KERNEL);
- if (priv == NULL)
- goto error1;
+ /* get the common data struct for both demods */
+ demod = cx24117_get_priv(&priv, i2c, config->demod_address);
+
+ switch (demod) {
+ case 0:
+ dev_err(&state->priv->i2c->dev,
+ "%s: Error attaching frontend %d\n",
+ KBUILD_MODNAME, demod);
+ goto error1;
+ break;
+ case 1:
+ /* new priv instance */
priv->i2c = i2c;
priv->demod_address = config->demod_address;
mutex_init(&priv->fe_lock);
- } else {
- demod = 1;
- priv = ((struct cx24117_state *) fe->demodulator_priv)->priv;
+ break;
+ default:
+ /* existing priv instance */
+ break;
}
/* allocate memory for the internal state */
@@ -1154,7 +1187,7 @@ struct dvb_frontend *cx24117_attach(const struct
cx24117_config *con
Re: [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod.
Em Wed, 2 Oct 2013 23:09:11 +0100
Luis Alves escreveu:
> Hi Mike,
>
> It's done (also tested and apparently working good)!
>
> I didn't know if two separated patches were needed (one for the cx24117 and
> the other for the cx23885) but I've splited it.
If the patch causes regression or breaks compilation, it should be folded.
I suspect that this is the case, as you're changing the interface here.
> As you pointed out, this series of patches are to be used against your
> cx24117 branch.
>
> Regards,
> Luis
>
> Signed-off-by: Luis Alves
> ---
> drivers/media/dvb-frontends/cx24117.c | 72
> +++--
> drivers/media/dvb-frontends/cx24117.h |4 +-
> 2 files changed, 53 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/dvb-frontends/cx24117.c
> b/drivers/media/dvb-frontends/cx24117.c
> index 3b63913..9087309 100644
> --- a/drivers/media/dvb-frontends/cx24117.c
> +++ b/drivers/media/dvb-frontends/cx24117.c
> @@ -31,6 +31,7 @@
> #include
> #include
>
> +#include "tuner-i2c.h"
> #include "dvb_frontend.h"
> #include "cx24117.h"
>
> @@ -145,6 +146,9 @@ enum cmds {
> CMD_TUNERSLEEP = 0x36,
> };
>
> +static LIST_HEAD(hybrid_tuner_instance_list);
> +static DEFINE_MUTEX(cx24117_list_mutex);
> +
> /* The Demod/Tuner can't easily provide these, we cache them */
> struct cx24117_tuning {
> u32 frequency;
> @@ -176,9 +180,11 @@ struct cx24117_priv {
> u8 demod_address;
> struct i2c_adapter *i2c;
> u8 skip_fw_load;
> -
> struct mutex fe_lock;
> - atomic_t fe_nr;
> +
> + /* Used for sharing this struct between demods */
> + struct tuner_i2c_props i2c_props;
> + struct list_head hybrid_tuner_instance_list;
> };
>
> /* one per each fe */
> @@ -536,7 +542,7 @@ static int cx24117_load_firmware(struct dvb_frontend *fe,
> dev_dbg(&state->priv->i2c->dev,
> "%s() demod%d FW is %zu bytes (%02x %02x .. %02x %02x)\n",
> __func__, state->demod, fw->size, fw->data[0], fw->data[1],
> - fw->data[fw->size-2], fw->data[fw->size-1]);
> + fw->data[fw->size - 2], fw->data[fw->size - 1]);
>
> cx24117_writereg(state, 0xea, 0x00);
> cx24117_writereg(state, 0xea, 0x01);
> @@ -1116,37 +1122,64 @@ static int cx24117_diseqc_send_burst(struct
> dvb_frontend *fe,
> return 0;
> }
>
> +static int cx24117_get_priv(struct cx24117_priv **priv,
> + struct i2c_adapter *i2c, u8 client_address)
> +{
> + int ret;
> +
> + mutex_lock(&cx24117_list_mutex);
> + ret = hybrid_tuner_request_state(struct cx24117_priv, (*priv),
> + hybrid_tuner_instance_list, i2c, client_address, "cx24117");
> + mutex_unlock(&cx24117_list_mutex);
> +
> + return ret;
> +}
> +
> +static void cx24117_release_priv(struct cx24117_priv *priv)
> +{
> + mutex_lock(&cx24117_list_mutex);
> + if (priv != NULL)
> + hybrid_tuner_release_state(priv);
> + mutex_unlock(&cx24117_list_mutex);
> +}
> +
> static void cx24117_release(struct dvb_frontend *fe)
> {
> struct cx24117_state *state = fe->demodulator_priv;
> dev_dbg(&state->priv->i2c->dev, "%s demod%d\n",
> __func__, state->demod);
> - if (!atomic_dec_and_test(&state->priv->fe_nr))
> - kfree(state->priv);
> + cx24117_release_priv(state->priv);
> kfree(state);
> }
>
> static struct dvb_frontend_ops cx24117_ops;
>
> struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
> - struct i2c_adapter *i2c, struct dvb_frontend *fe)
> + struct i2c_adapter *i2c)
> {
> struct cx24117_state *state = NULL;
> struct cx24117_priv *priv = NULL;
> int demod = 0;
>
> - /* first frontend attaching */
> - /* allocate shared priv struct */
> - if (fe == NULL) {
> - priv = kzalloc(sizeof(struct cx24117_priv), GFP_KERNEL);
> - if (priv == NULL)
> - goto error1;
> + /* get the common data struct for both demods */
> + demod = cx24117_get_priv(&priv, i2c, config->demod_address);
> +
> + switch (demod) {
> + case 0:
> + dev_err(&state->priv->i2c->dev,
> + "%s: Error attaching frontend %d\n",
> + KBUILD_MODNAME, demod);
> + goto error1;
> + break;
> + case 1:
> + /* new priv instance */
> priv->i2c = i2c;
> priv->demod_address = config->demod_address;
> mutex_init(&priv->fe_lock);
> - } else {
> - demod = 1;
> - priv = ((struct cx24117_state *) fe->demodulator_priv)->priv;
> + break;
> + default:
> + /* existing priv instance */
> + break;
> }
>
> /* allocate memory for the internal state */
> @@ -1154,7 +1187,7 @@ struct dvb_frontend *cx24117_attach(const struct
> cx24117_config *config,
> if (state == NULL)
>
[PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod.
Hi Mike,
It's done (also tested and apparently working good)!
I didn't know if two separated patches were needed (one for the cx24117 and the
other for the cx23885) but I've splited it.
As you pointed out, this series of patches are to be used against your cx24117
branch.
Regards,
Luis
Signed-off-by: Luis Alves
---
drivers/media/dvb-frontends/cx24117.c | 72 +++--
drivers/media/dvb-frontends/cx24117.h |4 +-
2 files changed, 53 insertions(+), 23 deletions(-)
diff --git a/drivers/media/dvb-frontends/cx24117.c
b/drivers/media/dvb-frontends/cx24117.c
index 3b63913..9087309 100644
--- a/drivers/media/dvb-frontends/cx24117.c
+++ b/drivers/media/dvb-frontends/cx24117.c
@@ -31,6 +31,7 @@
#include
#include
+#include "tuner-i2c.h"
#include "dvb_frontend.h"
#include "cx24117.h"
@@ -145,6 +146,9 @@ enum cmds {
CMD_TUNERSLEEP = 0x36,
};
+static LIST_HEAD(hybrid_tuner_instance_list);
+static DEFINE_MUTEX(cx24117_list_mutex);
+
/* The Demod/Tuner can't easily provide these, we cache them */
struct cx24117_tuning {
u32 frequency;
@@ -176,9 +180,11 @@ struct cx24117_priv {
u8 demod_address;
struct i2c_adapter *i2c;
u8 skip_fw_load;
-
struct mutex fe_lock;
- atomic_t fe_nr;
+
+ /* Used for sharing this struct between demods */
+ struct tuner_i2c_props i2c_props;
+ struct list_head hybrid_tuner_instance_list;
};
/* one per each fe */
@@ -536,7 +542,7 @@ static int cx24117_load_firmware(struct dvb_frontend *fe,
dev_dbg(&state->priv->i2c->dev,
"%s() demod%d FW is %zu bytes (%02x %02x .. %02x %02x)\n",
__func__, state->demod, fw->size, fw->data[0], fw->data[1],
- fw->data[fw->size-2], fw->data[fw->size-1]);
+ fw->data[fw->size - 2], fw->data[fw->size - 1]);
cx24117_writereg(state, 0xea, 0x00);
cx24117_writereg(state, 0xea, 0x01);
@@ -1116,37 +1122,64 @@ static int cx24117_diseqc_send_burst(struct
dvb_frontend *fe,
return 0;
}
+static int cx24117_get_priv(struct cx24117_priv **priv,
+ struct i2c_adapter *i2c, u8 client_address)
+{
+ int ret;
+
+ mutex_lock(&cx24117_list_mutex);
+ ret = hybrid_tuner_request_state(struct cx24117_priv, (*priv),
+ hybrid_tuner_instance_list, i2c, client_address, "cx24117");
+ mutex_unlock(&cx24117_list_mutex);
+
+ return ret;
+}
+
+static void cx24117_release_priv(struct cx24117_priv *priv)
+{
+ mutex_lock(&cx24117_list_mutex);
+ if (priv != NULL)
+ hybrid_tuner_release_state(priv);
+ mutex_unlock(&cx24117_list_mutex);
+}
+
static void cx24117_release(struct dvb_frontend *fe)
{
struct cx24117_state *state = fe->demodulator_priv;
dev_dbg(&state->priv->i2c->dev, "%s demod%d\n",
__func__, state->demod);
- if (!atomic_dec_and_test(&state->priv->fe_nr))
- kfree(state->priv);
+ cx24117_release_priv(state->priv);
kfree(state);
}
static struct dvb_frontend_ops cx24117_ops;
struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
- struct i2c_adapter *i2c, struct dvb_frontend *fe)
+ struct i2c_adapter *i2c)
{
struct cx24117_state *state = NULL;
struct cx24117_priv *priv = NULL;
int demod = 0;
- /* first frontend attaching */
- /* allocate shared priv struct */
- if (fe == NULL) {
- priv = kzalloc(sizeof(struct cx24117_priv), GFP_KERNEL);
- if (priv == NULL)
- goto error1;
+ /* get the common data struct for both demods */
+ demod = cx24117_get_priv(&priv, i2c, config->demod_address);
+
+ switch (demod) {
+ case 0:
+ dev_err(&state->priv->i2c->dev,
+ "%s: Error attaching frontend %d\n",
+ KBUILD_MODNAME, demod);
+ goto error1;
+ break;
+ case 1:
+ /* new priv instance */
priv->i2c = i2c;
priv->demod_address = config->demod_address;
mutex_init(&priv->fe_lock);
- } else {
- demod = 1;
- priv = ((struct cx24117_state *) fe->demodulator_priv)->priv;
+ break;
+ default:
+ /* existing priv instance */
+ break;
}
/* allocate memory for the internal state */
@@ -1154,7 +1187,7 @@ struct dvb_frontend *cx24117_attach(const struct
cx24117_config *config,
if (state == NULL)
goto error2;
- state->demod = demod;
+ state->demod = demod - 1;
state->priv = priv;
/* test i2c bus for ack */
@@ -1163,12 +1196,9 @@ struct dvb_frontend *cx24117_attach(const struct
cx24117_config *config,
goto error3;
}
- /* nr of frontends using the module */
- atomic
[PATCH 2/2] cx24117: Removed from cx23885 the no longer needed frontend pointer from the dvb_attach function.
cx23885 changes: to be used against mkrufky/cx24117 branch Signed-off-by: Luis Alves --- drivers/media/pci/cx23885/cx23885-dvb.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c index 34120db..0549205 100644 --- a/drivers/media/pci/cx23885/cx23885-dvb.c +++ b/drivers/media/pci/cx23885/cx23885-dvb.c @@ -1058,20 +1058,13 @@ static int dvb_register(struct cx23885_tsport *port) case 1: fe0->dvb.frontend = dvb_attach(cx24117_attach, &tbs_cx24117_config, - &i2c_bus->i2c_adap, NULL); + &i2c_bus->i2c_adap); break; /* PORT C */ case 2: - /* use fe1 pointer as temporary holder */ - /* for the first frontend */ - fe1 = videobuf_dvb_get_frontend( - &port->dev->ts1.frontends, 1); - fe0->dvb.frontend = dvb_attach(cx24117_attach, &tbs_cx24117_config, - &i2c_bus->i2c_adap, fe1->dvb.frontend); - /* we're done, so clear fe1 pointer */ - fe1 = NULL; + &i2c_bus->i2c_adap); break; } break; -- 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: [PATCH 2/5] gpio: samsung: Use CONFIG_ARCH_S3C64XX to check for S3C64xx support
Hi Linus, On Wednesday 02 of October 2013 12:46:51 Linus Walleij wrote: > On Sat, Sep 28, 2013 at 8:21 PM, Tomasz Figa wrote: > > Since CONFIG_PLAT_S3C64XX is going to be removed, this patch modifies > > the gpio-samsung driver to use the proper way of checking for S3C64xx > > support - CONFIG_ARCH_S3C64XX. > > > > Signed-off-by: Tomasz Figa > > Acked-by: Linus Walleij Thanks. > I assume that this will go through ARM SoC? I think so. Best regards, Tomasz -- 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 v2 4/4] v4l: events: Don't sleep in dequeue if none are subscribed
Laurent Pinchart wrote: Hi Sakari, On Wednesday 02 October 2013 16:45:16 Sakari Ailus wrote: Dequeueing events was is entirely possible even if none are subscribed, was or is ? :-) Was. :-) I'll fix that. -- Sakari Ailus sakari.ai...@linux.intel.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 05/26] omap3isp: Make isp_video_buffer_prepare_user() use get_user_pages_fast()
On Wed 02-10-13 21:41:10, Laurent Pinchart wrote:
> Hi Jan,
>
> Thank you for the patch.
>
> On Wednesday 02 October 2013 16:27:46 Jan Kara wrote:
> > CC: Laurent Pinchart
> > CC: linux-media@vger.kernel.org
> > Signed-off-by: Jan Kara
>
> Acked-by: Laurent Pinchart
Thanks!
>
> Could you briefly explain where you're headed with this ?
My motivation is that currently filesystems have to workaround locking
problems with ->page_mkwrite() (the write page fault handler) being called
with mmap_sem held. The plan is to provide ability to ->page_mkwrite()
handler to drop mmap_sem. And that needs an audit of all get_user_pages()
callers to verify they won't be broken by this locking change. So I've
started with making this audit simpler.
> The V4L2 subsystem has suffered for quite a long time from potentiel
> AB-BA deadlocks, due the drivers taking the mmap_sem semaphore while
> holding the V4L2 buffers queue lock in the code path below, and taking
> them in reverse order in the mmap() path (as the mm core takes the
> mmap_sem semaphore before calling the mmap() operation).
Yeah, I've read about this in some comment in V4L2. Also there are some
places (drivers/media/platform/omap/omap_vout.c and
drivers/media/v4l2-core/) which acquire mmap_sem pretty early to avoid lock
inversion with the queue lock. These are actually causing me quite some
headache :)
> We've solved that by releasing the V4L2 buffers queue lock before
> taking the mmap_sem lock below and taking it back after releasing the
> mmap_sem
> lock. Having an explicit indication that the mmap_sem lock was taken helped
> keeping the problem in mind. I'm not against hiding it in
> get_user_pages_fast(), but I'd like to know what the next step is. Maybe it
> would also be worth it adding a /* get_user_pages_fast() takes the mmap_sem
> */
> comment before the call ?
OK, I can add the comment. Thanks for review.
Honza
> > ---
> > drivers/media/platform/omap3isp/ispqueue.c | 10 +++---
> > 1 file changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/media/platform/omap3isp/ispqueue.c
> > b/drivers/media/platform/omap3isp/ispqueue.c index
> > e15f01342058..bed380395e6c 100644
> > --- a/drivers/media/platform/omap3isp/ispqueue.c
> > +++ b/drivers/media/platform/omap3isp/ispqueue.c
> > @@ -331,13 +331,9 @@ static int isp_video_buffer_prepare_user(struct
> > isp_video_buffer *buf) if (buf->pages == NULL)
> > return -ENOMEM;
> >
> > - down_read(¤t->mm->mmap_sem);
> > - ret = get_user_pages(current, current->mm, data & PAGE_MASK,
> > -buf->npages,
> > -buf->vbuf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE, 0,
> > -buf->pages, NULL);
> > - up_read(¤t->mm->mmap_sem);
> > -
> > + ret = get_user_pages_fast(data & PAGE_MASK, buf->npages,
> > + buf->vbuf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE,
> > + buf->pages);
> > if (ret != buf->npages) {
> > buf->npages = ret < 0 ? 0 : ret;
> > isp_video_buffer_cleanup(buf);
> --
> Regards,
>
> Laurent Pinchart
>
--
Jan Kara
SUSE Labs, CR
--
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 v2 0/4]
Hi Laurent, Laurent Pinchart wrote: On Wednesday 02 October 2013 16:45:12 Sakari Ailus wrote: Hi all, This is the second RFC set after the initial patch that makes poll return POLLERR if no events are subscribed. There are other issues as well which these patches address. The original RFC patch is here: http://www.spinics.net/lists/linux-media/msg68077.html> poll(2) and select(2) can both be used for I/O multiplexing. While both provide slightly different semantics. man 2 select: select() and pselect() allow a program to monitor multiple file descriptors, waiting until one or more of the file descriptors become "ready" for some class of I/O operation (e.g., input possible). A file descriptor is considered ready if it is possible to perform the corre‐ sponding I/O operation (e.g., read(2)) without blocking. The two system calls provide slightly different semantics: poll(2) can signal POLLERR related to a file handle but select(2) does not: instead, on POLLERR it sets a bit corresponding to a file handle in the read and write sets. This is somewhat confusing since with the original patch --- using select(2) would suggest that there's something to read or write instead of knowing no further exceptions are coming. Thus, also denying polling a subdev file handle using select(2) will mean the POLLERR never gets through in any form. So the meaningful alternatives I can think of are: 1) Use POLLERR | POLLPRI. When the last event subscription is gone and select(2) IOCTL is issued, all file descriptor sets are set for a file handle. Users of poll(2) will directly see both of the flags, making the case visible to the user immediately in some cases. On sub-devices this is obvious but on V4L2 devices the user should poll(2) (or select(2)) again to know whether there's I/O waiting to be read, written or whether buffers are ready. 2) Use POLLPRI only. While this does not differ from any regular event at the level of poll(2) or select(2), the POLLIN or POLLOUT flags are not adversely affected. In each of the cases to ascertain oneself in a generic way of whether events cannot no longer be obtained one has to call VIDIOC_DQEVENT IOCTL, which currently may block. A patch in the set makes VIDIOC_DQEVENT to signal EIO error code if no events are subscribed. The videobuf2 changes are untested at the moment since I didn't have a device using videobuf2 at hand right now. Comments and questions are very welcome. What's the behaviour of select(2) and poll(2) after this patch set when polling an fd for both read and events, when no event has been subscribed to ? The first one. If you're using select(2), you'll get the fd-specific bit set in all three sets. For poll(2), you'll get POLLERR and POLLPRI set for the fd. No poll nor select can directly tell that there are no further events; instead they intend to say that the corresponding operations on the file descriptor wouldn't block. Events are a little funny in this respect; the difference of behaviour must be documented in select(2) V4L2 documentation which currently is missing entirely (I'll send a patch). An alternative would be indeed use POLLPRI only. The only way the user would know there are no further events coming would be to use VIDIOC_DQEVENT then. As a matter of fact, this way the behaviour of select(2) would better conform to what the POSIX standard specifies but OTOH would not allow to tell about the situation using poll(2) only in any case since it'd look like the same as any event. But I don't think it'd be a problem either. -- Regards, Sakari Ailus sakari.ai...@linux.intel.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 05/26] omap3isp: Make isp_video_buffer_prepare_user() use get_user_pages_fast()
Hi Jan,
Thank you for the patch.
On Wednesday 02 October 2013 16:27:46 Jan Kara wrote:
> CC: Laurent Pinchart
> CC: linux-media@vger.kernel.org
> Signed-off-by: Jan Kara
Acked-by: Laurent Pinchart
Could you briefly explain where you're headed with this ? The V4L2 subsystem
has suffered for quite a long time from potentiel AB-BA deadlocks, due the
drivers taking the mmap_sem semaphore while holding the V4L2 buffers queue
lock in the code path below, and taking them in reverse order in the mmap()
path (as the mm core takes the mmap_sem semaphore before calling the mmap()
operation). We've solved that by releasing the V4L2 buffers queue lock before
taking the mmap_sem lock below and taking it back after releasing the mmap_sem
lock. Having an explicit indication that the mmap_sem lock was taken helped
keeping the problem in mind. I'm not against hiding it in
get_user_pages_fast(), but I'd like to know what the next step is. Maybe it
would also be worth it adding a /* get_user_pages_fast() takes the mmap_sem */
comment before the call ?
> ---
> drivers/media/platform/omap3isp/ispqueue.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/ispqueue.c
> b/drivers/media/platform/omap3isp/ispqueue.c index
> e15f01342058..bed380395e6c 100644
> --- a/drivers/media/platform/omap3isp/ispqueue.c
> +++ b/drivers/media/platform/omap3isp/ispqueue.c
> @@ -331,13 +331,9 @@ static int isp_video_buffer_prepare_user(struct
> isp_video_buffer *buf) if (buf->pages == NULL)
> return -ENOMEM;
>
> - down_read(¤t->mm->mmap_sem);
> - ret = get_user_pages(current, current->mm, data & PAGE_MASK,
> - buf->npages,
> - buf->vbuf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE, 0,
> - buf->pages, NULL);
> - up_read(¤t->mm->mmap_sem);
> -
> + ret = get_user_pages_fast(data & PAGE_MASK, buf->npages,
> + buf->vbuf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE,
> + buf->pages);
> if (ret != buf->npages) {
> buf->npages = ret < 0 ? 0 : ret;
> isp_video_buffer_cleanup(buf);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New USB core API to change interval and max packet size
On Wed, 2 Oct 2013, Alan Stern wrote: > > Ok, so it sounds like we want to keep the changes the endpoints across > > alt setting changes. > > No, just the opposite. That was the point I was trying to make. Any > changes to ep5 in altsetting 0 (for example) will have no effect on ep1 --^^^ Typo: this should have been ep5 as well. > in altsetting 1, because the two altsettings reference the endpoint by > two separate usb_host_endpoint structures. Alan Stern -- 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: New USB core API to change interval and max packet size
On Wed, 2 Oct 2013, Sarah Sharp wrote: > > In particular, do we want to go around changing single endpoints, one > > at a time? Or do we want to change all the endpoints in an interface > > at once? > > > > Given that a change to one endpoint may require the entire schedule to > > be recomputed, it seems to make more sense to do all of them at once. > > For example, drivers could call a routine to set the desired endpoint > > parameters, and then the new parameters could take effect when the > > driver calls usb_set_interface(). > > I think it makes sense to change several endpoints, and then ask the > host to recompute the schedule. Perhaps the driver could issue several > calls to usb_change_ep_bandwidth and then ask the USB core to update the > host's schedule? That's what I had in mind. Note that usb_set_interface() already updates the schedule. > I'm not sure that usb_set_interface() is the right function for the new > parameters to take effect. What if the driver is trying to modify the > current alt setting? Would they need to call usb_set_interface() again? Yes. I can see two disadvantages: You don't want to call usb_set_interface() if there are any transfers in progress for endpoints in that interface -- even endpoints whose bandwidth you aren't trying to change. The transfers would get shut down. usb_set_interface() communicates with the device, which isn't necessary if you're merely updating the host's schedule. The alternatives are either a separate function to do the schedule update, or else pass usb_change_ep_bandwidth() an array or list of endpoint info and have it do all the updates at once. I think a separate function would be easier for drivers to use. > > In any case, the question about what to do when the interface setting > > gets switched never really arises. Each usb_host_endpoint structure is > > referenced from only one altsetting. If the driver wants the new > > parameters applied to an endpoint in several altsettings, it will have > > to change each altsetting separately. > > Ok, so it sounds like we want to keep the changes the endpoints across > alt setting changes. No, just the opposite. That was the point I was trying to make. Any changes to ep5 in altsetting 0 (for example) will have no effect on ep1 in altsetting 1, because the two altsettings reference the endpoint by two separate usb_host_endpoint structures. Furthermore, it generally doesn't make sense to apply a single change across multiple altsettings. An isochronous endpoint, for example, might have maxpacket = 500 in one altsetting and 900 in another. When you reduce the 500 to 400, that doesn't mean you also want to reduce the 900 to 400. > But we still want to reset the values after the > driver unbinds, correct? Absolutely. When the next driver binds, it should get the default values (i.e., the ones stored in the descriptors). This would be analogous to the way we currently set each interface back to altsetting 0 when a driver unbinds. Alan Stern -- 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
Hello My Dear,New Friend
Hello My Dear,New Friend How are you today?? My name is Miss Glory Kuku, I like to be your friends ok; Please send me your e-mail to my inbox at (glory01...@yahoo.com) so i will send you my Photos to you -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New USB core API to change interval and max packet size
On Wed, Oct 02, 2013 at 10:22:52AM -0400, Alan Stern wrote: > On Tue, 1 Oct 2013, Sarah Sharp wrote: > > > > When you say a new API, what do you mean? New functions in usbcore > > > to be used by usb device drivers? > > > > Yes. You would export the function in the USB core, and put a prototype > > in a USB include file (probably in include/linux/usb.h). Let's say that > > function is called usb_change_ep_bandwidth. > > > > Drivers could call into that function when they needed to change either > > the bInterval or wMaxPacketSize of a particular endpoint. This could be > > during the driver's probe function, or before switching alternate > > interface settings, or even after the alt setting is in place, but > > userspace dictates the driver use a different bandwidth. > > > > Drivers should pass usb_change_ep_bandwidth a pointer to the endpoint > > they need to change, along with the bInterval and wMaxPacketSize values > > they would like the endpoint to have. Those values could be stored as > > new values in struct usb_host_endpoint. > > > > usb_change_ep_bandwidth would then call into the xHCI driver to drop the > > endpoint, re-add it, and then issue a bandwidth change. The xHCI driver > > would have to be changed to look at the new fields in usb_host_endpoint, > > and set up the endpoint contexts with the interval and packet size from > > those fields, instead of the endpoint descriptor. > > > > We should probably set the new values in usb_host_endpoint to zero after > > the driver unbinds from the device. Not sure if they should be reset > > after the driver switches interfaces. I would have to see the use cases > > in the driver. > > We should consider this before rushing into a new API. Yes, I agree. :) That's why I'd like to see some cases in the media drivers code where it could benefit from changing the interval or maxpacket size, so that we can see what use cases we have. Mauro, can you point is to places in drivers that would need this? > In particular, do we want to go around changing single endpoints, one > at a time? Or do we want to change all the endpoints in an interface > at once? > > Given that a change to one endpoint may require the entire schedule to > be recomputed, it seems to make more sense to do all of them at once. > For example, drivers could call a routine to set the desired endpoint > parameters, and then the new parameters could take effect when the > driver calls usb_set_interface(). I think it makes sense to change several endpoints, and then ask the host to recompute the schedule. Perhaps the driver could issue several calls to usb_change_ep_bandwidth and then ask the USB core to update the host's schedule? I'm not sure that usb_set_interface() is the right function for the new parameters to take effect. What if the driver is trying to modify the current alt setting? Would they need to call usb_set_interface() again? > In any case, the question about what to do when the interface setting > gets switched never really arises. Each usb_host_endpoint structure is > referenced from only one altsetting. If the driver wants the new > parameters applied to an endpoint in several altsettings, it will have > to change each altsetting separately. Ok, so it sounds like we want to keep the changes the endpoints across alt setting changes. But we still want to reset the values after the driver unbinds, correct? Sarah Sharp -- 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: New USB core API to change interval and max packet size
On Wed, Oct 02, 2013 at 12:16:04AM +0300, Xenia Ragiadakou wrote: > On 10/01/2013 11:45 PM, Sarah Sharp wrote: > >Sure. I would actually suggest you first finish up the patch to issue a > >configure endpoint if userspace wants to clear a halt, but the endpoint > >isn't actually halted. Did your most current patch work? I can't > >remember what the status was. > > > >Sarah Sharp > > Thanks for the clarification, I understand better now. > As far as concerns the reset endpoint fix, I am not sure if it works > I have to send an RFC so that it can be further tested but I have a > lot of pending RFCs for xhci on the mailing list so i was waiting > for those to be reviewed before sending new ones. Or it is not > necessary to wait and just send the RFC based on current usb-next > tree? Go ahead and send that one as well. Sarah Sharp -- 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 v2 0/4]
Hi Sakari, On Wednesday 02 October 2013 16:45:12 Sakari Ailus wrote: > Hi all, > > This is the second RFC set after the initial patch that makes poll return > POLLERR if no events are subscribed. There are other issues as well which > these patches address. > > The original RFC patch is here: > > http://www.spinics.net/lists/linux-media/msg68077.html> > > poll(2) and select(2) can both be used for I/O multiplexing. While both > provide slightly different semantics. man 2 select: > >select() and pselect() allow a program to monitor multiple > file descriptors, waiting until one or more of the file descriptors > become "ready" for some class of I/O operation (e.g., input possible). A > file descriptor is considered ready if it is possible to perform the > corre‐ sponding I/O operation (e.g., read(2)) without blocking. > > The two system calls provide slightly different semantics: poll(2) can > signal POLLERR related to a file handle but select(2) does not: instead, on > POLLERR it sets a bit corresponding to a file handle in the read and write > sets. This is somewhat confusing since with the original patch --- using > select(2) would suggest that there's something to read or write instead of > knowing no further exceptions are coming. > > Thus, also denying polling a subdev file handle using select(2) will mean > the POLLERR never gets through in any form. > > So the meaningful alternatives I can think of are: > > 1) Use POLLERR | POLLPRI. When the last event subscription is gone and > select(2) IOCTL is issued, all file descriptor sets are set for a file > handle. Users of poll(2) will directly see both of the flags, making the > case visible to the user immediately in some cases. On sub-devices this is > obvious but on V4L2 devices the user should poll(2) (or select(2)) again to > know whether there's I/O waiting to be read, written or whether buffers are > ready. > > 2) Use POLLPRI only. While this does not differ from any regular event at > the level of poll(2) or select(2), the POLLIN or POLLOUT flags are not > adversely affected. > > In each of the cases to ascertain oneself in a generic way of whether events > cannot no longer be obtained one has to call VIDIOC_DQEVENT IOCTL, which > currently may block. A patch in the set makes VIDIOC_DQEVENT to signal EIO > error code if no events are subscribed. > > The videobuf2 changes are untested at the moment since I didn't have a > device using videobuf2 at hand right now. > > Comments and questions are very welcome. What's the behaviour of select(2) and poll(2) after this patch set when polling an fd for both read and events, when no event has been subscribed to ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 4/4] v4l: events: Don't sleep in dequeue if none are subscribed
Hi Sakari,
On Wednesday 02 October 2013 16:45:16 Sakari Ailus wrote:
> Dequeueing events was is entirely possible even if none are subscribed,
was or is ? :-)
> leading to sleeping indefinitely. Fix this by returning -ENOENT when no
> events are subscribed.
>
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/v4l2-core/v4l2-event.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-event.c
> b/drivers/media/v4l2-core/v4l2-event.c index b53897e..553a800 100644
> --- a/drivers/media/v4l2-core/v4l2-event.c
> +++ b/drivers/media/v4l2-core/v4l2-event.c
> @@ -77,10 +77,17 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct
> v4l2_event *event, mutex_unlock(fh->vdev->lock);
>
> do {
> - ret = wait_event_interruptible(fh->wait,
> -fh->navailable != 0);
> + bool subscribed;
> + ret = wait_event_interruptible(
> + fh->wait,
> + fh->navailable != 0 ||
> + !(subscribed = v4l2_event_has_subscribed(fh)));
> if (ret < 0)
> break;
> + if (!subscribed) {
> + ret = -EIO;
> + break;
> + }
>
> ret = __v4l2_event_dequeue(fh, event);
> } while (ret == -ENOENT);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control
On 10/02/2013 11:33 AM, Mauro Carvalho Chehab wrote: ... > Well, from userspace PoV, it should have just one devnode for each > TX/RX. I'm fine with that. > So, if the device has N TX and/or RX simultaneous connections, it should > be exposing N device nodes, and the DT should for it should have N entries, > one for each. DT is based on the actual HW construction, not how a particular OS wants to expose that HW through its APIs. If there is a single HW block, there should be a single DT node, even if that HW block supports multiple channels. In some circumstances, it might make sense for the single top-level node that represents the HW-block to have child nodes that represent the channels, depending on what exactly the HW is doing and whether this level of detail is useful in DT. I would qualify this as rare though. -- 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 RFC] media: rc: OF: Add Generic bindings for remote-control
Em Wed, 02 Oct 2013 17:22:06 +0100 Srinivas KANDAGATLA escreveu: > On 01/10/13 15:49, Mauro Carvalho Chehab wrote: > >>> > > > >>> > > Btw, we're even thinking on mapping HDMI-CEC remote controller RX/TX > >>> > > via > >>> > > the RC subsystem. So, another L1 protocol would be "hdmi-cec". > >>> > > > >> > Ok. > >>> > > Yet, it seems unlikely that the very same remote controller IP would > >>> > > use > >>> > > a different protocol for RX and TX, while sharing the same registers. > >> > > >> > ST IRB block has one IR processor which has both TX and RX support and > >> > one UHF Processor which has RX support only. However the register map > >> > for all these support is in single IRB IP block. > >> > > >> > So the driver can configure the IP as TX in "infrared" and RX in "uhf". > >> > This is supported in ST IRB IP. > >> > > >> > This case can not be represented in a single device tree node with > >> > l1-protocol and direction properties. > >> > > >> > IMHO, having tx-mode and rx-mode or tx-protocol and rx-protocol > >> > properties will give more flexibility. > >> > > >> > What do you think? > > Yeah, if they're using the same registers, then your proposal works > > better. > > > > I would prefer to not call it as just protocol, as IR has an > > upper layer protocol that defines how the bits are encoded, e. g. > > RC5, RC6, NEC, SONY, ..., with is what we generally call as protocol > > on rc-core. > > > > A proper naming for it is hard to find. Well, for IR/UHF, it is actually > > Yes I agree. > > > specifying the medium, but for Bluetooth, HDMI-CEC, it defines a > > protocol stack to be used, with covers not only the physical layer of > > the OSI model. > > > > Perhaps the better would be to call it as: tx-proto-stack/rx-proto-stack. > > > How are we going to address use case highlighted by Mark R, like N > Connections on a single IP block? > > This use-case can not be addressed with tx-mode and rx-mode or > tx-proto-stack/rx-proto-stack properties. > > So the idea of generic properties for tx and rx sounds incorrect. > > IMHO, Best thing would be to drop the idea of using tx-mode and rx-mode > as generic properties and use "st,tx-mode" and "st,rx-mode" instead for > st-rc driver. > > What do you think? Well, from userspace PoV, it should have just one devnode for each TX/RX. So, if the device has N TX and/or RX simultaneous connections, it should be exposing N device nodes, and the DT should for it should have N entries, one for each. A completely independent issue is how the driver will prevent to have two simultaneous access to the same resource. As on any other type of resource, there are several alternatives: - block the reads/writes, if some I/O operation is pending; - return -EAGAIN where the API allows (non-block calls), and the error is temporary; - return -EBUSY if the resource is more "permanently" allocated. So, if the very same registers are used by more than one TX/RX unit, then the driver should for example have a mutex/semaphore to lock such I/O while another I/O operation is undergoing. That solution is the same used by I2C devices: the I2C bus has a lock, serializing the access to all devices on that bus. There is another possible scenario: a device that have more than one connection, and that userspace could setup what connection is active, putting all the other ones inactive. On such scenario, we would need to add more bits at RC API, in order to allow userspace to enumerate the possible RX/TX connections, and to change it at runtime. If we had such scenario, then the DT representation for it could be different. So, instead of having a single TX/RX mode/protocol-stack, we would have a connections table. Also, each entry would likely need a name, in order to allow userspace to distinguish between the diferent entries that are wired on a given board. Anyway, we don't have such scenario yet. So, let's not overdesign the API, thinking on a possible scenario that may never happen. > > Thanks, > srini > > > -- > 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 -- Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New USB core API to change interval and max packet size
Em Wed, 02 Oct 2013 12:38:14 -0400 (EDT) Alan Stern escreveu: > On Wed, 2 Oct 2013, Mauro Carvalho Chehab wrote: > > > > > So, there's no need to call usb_change_ep_bandwidth(). > > > > > > That's right. > > > > > > > If so, then usb_change_ep_bandwidth() as a quirk, if bInterval > > > > or wMaxPacketSize were improperly filled. > > > > > > > > Right? > > > > > > Or if the values are correct, but the driver wants to use something > > > different for its own reasons (for example, to get lower latency or > > > because it knows that it will never use packets as large as the > > > descriptor allows). Right. > > > > Ok, so, in this case, usb_change_ep_bandwidth() could be called > > just before usb_alloc_urb(), in order to make it to use the packet > > size that would be expected for that kind of ISOC traffic that > > userspace indirectly selected, by adjusting the streaming > > video resolution selected, right? > > We haven't decided on the final API yet. However, note that > usb_alloc_urb() doesn't depend on the packet size. It requires you to > specify only the number of packets, not their sizes. Therefore it > doesn't matter whether you call usb_change_ep_bandwidth() before or > after usb_alloc_urb(). Sure, but, at least on almost all V4L2 drivers, the number of packets and their sizes should be calculated to be able to receive all URBs needed to store a complete image frame (that generally arrives on every 1/60 Hz or 1/50 Hz - depending on the frames per second rate). On those drivers, the transfer_buffer is allocated at the same loop where usb_alloc_urb() is called. So, it makes sense for them to specify the bandwidth parameters at the function that calls usb_alloc_coherent() and usb_alloc_urb(). Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New USB core API to change interval and max packet size
On Wed, 2 Oct 2013, Mauro Carvalho Chehab wrote: > > > So, there's no need to call usb_change_ep_bandwidth(). > > > > That's right. > > > > > If so, then usb_change_ep_bandwidth() as a quirk, if bInterval > > > or wMaxPacketSize were improperly filled. > > > > > > Right? > > > > Or if the values are correct, but the driver wants to use something > > different for its own reasons (for example, to get lower latency or > > because it knows that it will never use packets as large as the > > descriptor allows). Right. > > Ok, so, in this case, usb_change_ep_bandwidth() could be called > just before usb_alloc_urb(), in order to make it to use the packet > size that would be expected for that kind of ISOC traffic that > userspace indirectly selected, by adjusting the streaming > video resolution selected, right? We haven't decided on the final API yet. However, note that usb_alloc_urb() doesn't depend on the packet size. It requires you to specify only the number of packets, not their sizes. Therefore it doesn't matter whether you call usb_change_ep_bandwidth() before or after usb_alloc_urb(). Alan Stern -- 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 RFC] media: rc: OF: Add Generic bindings for remote-control
On 01/10/13 15:49, Mauro Carvalho Chehab wrote: >>> > > >>> > > Btw, we're even thinking on mapping HDMI-CEC remote controller RX/TX via >>> > > the RC subsystem. So, another L1 protocol would be "hdmi-cec". >>> > > >> > Ok. >>> > > Yet, it seems unlikely that the very same remote controller IP would use >>> > > a different protocol for RX and TX, while sharing the same registers. >> > >> > ST IRB block has one IR processor which has both TX and RX support and >> > one UHF Processor which has RX support only. However the register map >> > for all these support is in single IRB IP block. >> > >> > So the driver can configure the IP as TX in "infrared" and RX in "uhf". >> > This is supported in ST IRB IP. >> > >> > This case can not be represented in a single device tree node with >> > l1-protocol and direction properties. >> > >> > IMHO, having tx-mode and rx-mode or tx-protocol and rx-protocol >> > properties will give more flexibility. >> > >> > What do you think? > Yeah, if they're using the same registers, then your proposal works > better. > > I would prefer to not call it as just protocol, as IR has an > upper layer protocol that defines how the bits are encoded, e. g. > RC5, RC6, NEC, SONY, ..., with is what we generally call as protocol > on rc-core. > > A proper naming for it is hard to find. Well, for IR/UHF, it is actually Yes I agree. > specifying the medium, but for Bluetooth, HDMI-CEC, it defines a > protocol stack to be used, with covers not only the physical layer of > the OSI model. > > Perhaps the better would be to call it as: tx-proto-stack/rx-proto-stack. > How are we going to address use case highlighted by Mark R, like N Connections on a single IP block? This use-case can not be addressed with tx-mode and rx-mode or tx-proto-stack/rx-proto-stack properties. So the idea of generic properties for tx and rx sounds incorrect. IMHO, Best thing would be to drop the idea of using tx-mode and rx-mode as generic properties and use "st,tx-mode" and "st,rx-mode" instead for st-rc driver. What do you think? Thanks, srini -- 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: New USB core API to change interval and max packet size
Em Wed, 02 Oct 2013 11:00:13 -0400 (EDT) Alan Stern escreveu: > On Wed, 2 Oct 2013, Mauro Carvalho Chehab wrote: > > > Let me see if I understand the changes at the media drivers. So, please > > correct me if I got it wrong. > > > > I'm yet to get any USB 3.0 media device, although it is common to connect > > an USB 1.1 or USB 2.0 device on a USB 3.0 host port. > > > > So, for example, on this device: > > > ... > > Endpoint Descriptor: > > bLength 7 > > bDescriptorType 5 > > bEndpointAddress 0x83 EP 3 IN > > bmAttributes3 > > Transfer TypeInterrupt > > Synch Type None > > Usage Type Data > > wMaxPacketSize 0x0004 1x 4 bytes > > bInterval 1 > > ... > > > > connected via this BUS device: > > ... > > > In such situation, and assuming that the USB tables are correct, there's > > nothing that needs to be done there, as bInterval/wMaxPacketSize are > > correct for USB 2.0. > > > > So, there's no need to call usb_change_ep_bandwidth(). > > That's right. > > > If so, then usb_change_ep_bandwidth() as a quirk, if bInterval > > or wMaxPacketSize were improperly filled. > > > > Right? > > Or if the values are correct, but the driver wants to use something > different for its own reasons (for example, to get lower latency or > because it knows that it will never use packets as large as the > descriptor allows). Right. Ok, so, in this case, usb_change_ep_bandwidth() could be called just before usb_alloc_urb(), in order to make it to use the packet size that would be expected for that kind of ISOC traffic that userspace indirectly selected, by adjusting the streaming video resolution selected, right? Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New USB core API to change interval and max packet size
On Wed, 2 Oct 2013, Mauro Carvalho Chehab wrote: > Let me see if I understand the changes at the media drivers. So, please > correct me if I got it wrong. > > I'm yet to get any USB 3.0 media device, although it is common to connect > an USB 1.1 or USB 2.0 device on a USB 3.0 host port. > > So, for example, on this device: > ... > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x83 EP 3 IN > bmAttributes3 > Transfer TypeInterrupt > Synch Type None > Usage Type Data > wMaxPacketSize 0x0004 1x 4 bytes > bInterval 1 > ... > > connected via this BUS device: ... > In such situation, and assuming that the USB tables are correct, there's > nothing that needs to be done there, as bInterval/wMaxPacketSize are > correct for USB 2.0. > > So, there's no need to call usb_change_ep_bandwidth(). That's right. > If so, then usb_change_ep_bandwidth() as a quirk, if bInterval > or wMaxPacketSize were improperly filled. > > Right? Or if the values are correct, but the driver wants to use something different for its own reasons (for example, to get lower latency or because it knows that it will never use packets as large as the descriptor allows). Right. Alan Stern -- 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 v2 4/4] v4l: events: Don't sleep in dequeue if none are subscribed
Hans Verkuil wrote:
...
+if (!subscribed) {
+ret = -EIO;
Shouldn't this be -ENOENT?
If I use -ENOENT, having no events subscribed is indistinguishable
form no events pending condition. Combine that with using select(2),
and you can no longer distinguish having no events subscribed from
the case where you got an event but someone else (another thread or
process) dequeued it.
OK, but then your commit message is out of sync with the actual patch since
the commit log says ENOENT.
-EIO makes that explicit --- this also mirrors the behaviour of
VIDIOC_DQBUF. (And it must be documented as well, which is missing
from the patch currently.)
I don't like using EIO for this. EIO generally is returned if a hardware
error or an unexpected hardware condition occurs. How about -ENOMSG? Or
perhaps EPIPE? (As in: "the pipe containing events is gone").
Thinking about this some more, -ENOENT is probably what we should
return. *But* when there are no events to dequeue, we should instead
return -EAGAIN (i.e. EWOULDBLOCK) which VIDIOC_DQBUF also uses.
However I'm not sure if anything depends on -ENOENT currently (probably
not really) so changing this might require some consideration. No error
codes are currently defined for VIDIOC_DQEVENT; was planning to fix that
while we're at this.
--
Cheers,
Sakari Ailus
sakari.ai...@linux.intel.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: [RFC v2 4/4] v4l: events: Don't sleep in dequeue if none are subscribed
Hi Hans,
Hans Verkuil wrote:
On 10/02/13 16:18, Sakari Ailus wrote:
Hi Hans,
Thanks for the comments!
Hans Verkuil wrote:
On 10/02/13 15:45, Sakari Ailus wrote:
Dequeueing events was is entirely possible even if none are subscribed,
leading to sleeping indefinitely. Fix this by returning -ENOENT when no
events are subscribed.
Signed-off-by: Sakari Ailus
---
drivers/media/v4l2-core/v4l2-event.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index b53897e..553a800 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -77,10 +77,17 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct
v4l2_event *event,
mutex_unlock(fh->vdev->lock);
do {
-ret = wait_event_interruptible(fh->wait,
- fh->navailable != 0);
+bool subscribed;
Can you add an empty line here?
Sure.
+ret = wait_event_interruptible(
+fh->wait,
+fh->navailable != 0 ||
+!(subscribed = v4l2_event_has_subscribed(fh)));
if (ret < 0)
break;
+if (!subscribed) {
+ret = -EIO;
Shouldn't this be -ENOENT?
If I use -ENOENT, having no events subscribed is indistinguishable
form no events pending condition. Combine that with using select(2),
and you can no longer distinguish having no events subscribed from
the case where you got an event but someone else (another thread or
process) dequeued it.
OK, but then your commit message is out of sync with the actual patch since
the commit log says ENOENT.
Right. The error code was the last thing I changed before sending the
patch, and I ignored it was also present in the commit message. :-P
-EIO makes that explicit --- this also mirrors the behaviour of
VIDIOC_DQBUF. (And it must be documented as well, which is missing
from the patch currently.)
I don't like using EIO for this. EIO generally is returned if a hardware
error or an unexpected hardware condition occurs. How about -ENOMSG? Or
perhaps EPIPE? (As in: "the pipe containing events is gone").
There is no pipe (or at least wasn't; it's a queue or rather is
implemented as a fifo :)) so of the two I prefer -ENOMSG. What would you
think of -ENODATA or -EPERM (which is used e.g. when writing read-only
controls)?
--
Cheers,
Sakari Ailus
sakari.ai...@linux.intel.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: [RFC v2 4/4] v4l: events: Don't sleep in dequeue if none are subscribed
On 10/02/13 16:18, Sakari Ailus wrote:
> Hi Hans,
>
> Thanks for the comments!
>
> Hans Verkuil wrote:
>> On 10/02/13 15:45, Sakari Ailus wrote:
>>> Dequeueing events was is entirely possible even if none are subscribed,
>>> leading to sleeping indefinitely. Fix this by returning -ENOENT when no
>>> events are subscribed.
>>>
>>> Signed-off-by: Sakari Ailus
>>> ---
>>> drivers/media/v4l2-core/v4l2-event.c | 11 +--
>>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-event.c
>>> b/drivers/media/v4l2-core/v4l2-event.c
>>> index b53897e..553a800 100644
>>> --- a/drivers/media/v4l2-core/v4l2-event.c
>>> +++ b/drivers/media/v4l2-core/v4l2-event.c
>>> @@ -77,10 +77,17 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct
>>> v4l2_event *event,
>>> mutex_unlock(fh->vdev->lock);
>>>
>>> do {
>>> -ret = wait_event_interruptible(fh->wait,
>>> - fh->navailable != 0);
>>> +bool subscribed;
>>
>> Can you add an empty line here?
>
> Sure.
>
>>> +ret = wait_event_interruptible(
>>> +fh->wait,
>>> +fh->navailable != 0 ||
>>> +!(subscribed = v4l2_event_has_subscribed(fh)));
>>> if (ret < 0)
>>> break;
>>> +if (!subscribed) {
>>> +ret = -EIO;
>>
>> Shouldn't this be -ENOENT?
>
> If I use -ENOENT, having no events subscribed is indistinguishable
> form no events pending condition. Combine that with using select(2),
> and you can no longer distinguish having no events subscribed from
> the case where you got an event but someone else (another thread or
> process) dequeued it.
OK, but then your commit message is out of sync with the actual patch since
the commit log says ENOENT.
> -EIO makes that explicit --- this also mirrors the behaviour of
> VIDIOC_DQBUF. (And it must be documented as well, which is missing
> from the patch currently.)
I don't like using EIO for this. EIO generally is returned if a hardware
error or an unexpected hardware condition occurs. How about -ENOMSG? Or
perhaps EPIPE? (As in: "the pipe containing events is gone").
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 19/26] ivtv: Convert driver to use get_user_pages_unlocked()
CC: Andy Walls
CC: linux-media@vger.kernel.org
Signed-off-by: Jan Kara
---
drivers/media/pci/ivtv/ivtv-udma.c | 6 ++
drivers/media/pci/ivtv/ivtv-yuv.c | 12 ++--
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/media/pci/ivtv/ivtv-udma.c
b/drivers/media/pci/ivtv/ivtv-udma.c
index 7338cb2d0a38..6012e5049076 100644
--- a/drivers/media/pci/ivtv/ivtv-udma.c
+++ b/drivers/media/pci/ivtv/ivtv-udma.c
@@ -124,10 +124,8 @@ int ivtv_udma_setup(struct ivtv *itv, unsigned long
ivtv_dest_addr,
}
/* Get user pages for DMA Xfer */
- down_read(¤t->mm->mmap_sem);
- err = get_user_pages(current, current->mm,
- user_dma.uaddr, user_dma.page_count, 0, 1, dma->map,
NULL);
- up_read(¤t->mm->mmap_sem);
+ err = get_user_pages_unlocked(current, current->mm, user_dma.uaddr,
+ user_dma.page_count, 0, 1, dma->map);
if (user_dma.page_count != err) {
IVTV_DEBUG_WARN("failed to map user pages, returned %d instead
of %d\n",
diff --git a/drivers/media/pci/ivtv/ivtv-yuv.c
b/drivers/media/pci/ivtv/ivtv-yuv.c
index 2ad65eb29832..9365995917d8 100644
--- a/drivers/media/pci/ivtv/ivtv-yuv.c
+++ b/drivers/media/pci/ivtv/ivtv-yuv.c
@@ -75,15 +75,15 @@ static int ivtv_yuv_prep_user_dma(struct ivtv *itv, struct
ivtv_user_dma *dma,
ivtv_udma_get_page_info (&uv_dma, (unsigned long)args->uv_source, 360 *
uv_decode_height);
/* Get user pages for DMA Xfer */
- down_read(¤t->mm->mmap_sem);
- y_pages = get_user_pages(current, current->mm, y_dma.uaddr,
y_dma.page_count, 0, 1, &dma->map[0], NULL);
+ y_pages = get_user_pages_unlocked(current, current->mm, y_dma.uaddr,
+ y_dma.page_count, 0, 1, &dma->map[0]);
uv_pages = 0; /* silence gcc. value is set and consumed only if: */
if (y_pages == y_dma.page_count) {
- uv_pages = get_user_pages(current, current->mm,
- uv_dma.uaddr, uv_dma.page_count, 0, 1,
- &dma->map[y_pages], NULL);
+ uv_pages = get_user_pages_unlocked(current, current->mm,
+ uv_dma.uaddr,
+ uv_dma.page_count, 0, 1,
+ &dma->map[y_pages]);
}
- up_read(¤t->mm->mmap_sem);
if (y_pages != y_dma.page_count || uv_pages != uv_dma.page_count) {
int rc = -EFAULT;
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/26] omap3isp: Make isp_video_buffer_prepare_user() use get_user_pages_fast()
CC: Laurent Pinchart
CC: linux-media@vger.kernel.org
Signed-off-by: Jan Kara
---
drivers/media/platform/omap3isp/ispqueue.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispqueue.c
b/drivers/media/platform/omap3isp/ispqueue.c
index e15f01342058..bed380395e6c 100644
--- a/drivers/media/platform/omap3isp/ispqueue.c
+++ b/drivers/media/platform/omap3isp/ispqueue.c
@@ -331,13 +331,9 @@ static int isp_video_buffer_prepare_user(struct
isp_video_buffer *buf)
if (buf->pages == NULL)
return -ENOMEM;
- down_read(¤t->mm->mmap_sem);
- ret = get_user_pages(current, current->mm, data & PAGE_MASK,
-buf->npages,
-buf->vbuf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE, 0,
-buf->pages, NULL);
- up_read(¤t->mm->mmap_sem);
-
+ ret = get_user_pages_fast(data & PAGE_MASK, buf->npages,
+ buf->vbuf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE,
+ buf->pages);
if (ret != buf->npages) {
buf->npages = ret < 0 ? 0 : ret;
isp_video_buffer_cleanup(buf);
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New USB core API to change interval and max packet size
On Tue, 1 Oct 2013, Sarah Sharp wrote: > > When you say a new API, what do you mean? New functions in usbcore > > to be used by usb device drivers? > > Yes. You would export the function in the USB core, and put a prototype > in a USB include file (probably in include/linux/usb.h). Let's say that > function is called usb_change_ep_bandwidth. > > Drivers could call into that function when they needed to change either > the bInterval or wMaxPacketSize of a particular endpoint. This could be > during the driver's probe function, or before switching alternate > interface settings, or even after the alt setting is in place, but > userspace dictates the driver use a different bandwidth. > > Drivers should pass usb_change_ep_bandwidth a pointer to the endpoint > they need to change, along with the bInterval and wMaxPacketSize values > they would like the endpoint to have. Those values could be stored as > new values in struct usb_host_endpoint. > > usb_change_ep_bandwidth would then call into the xHCI driver to drop the > endpoint, re-add it, and then issue a bandwidth change. The xHCI driver > would have to be changed to look at the new fields in usb_host_endpoint, > and set up the endpoint contexts with the interval and packet size from > those fields, instead of the endpoint descriptor. > > We should probably set the new values in usb_host_endpoint to zero after > the driver unbinds from the device. Not sure if they should be reset > after the driver switches interfaces. I would have to see the use cases > in the driver. We should consider this before rushing into a new API. In particular, do we want to go around changing single endpoints, one at a time? Or do we want to change all the endpoints in an interface at once? Given that a change to one endpoint may require the entire schedule to be recomputed, it seems to make more sense to do all of them at once. For example, drivers could call a routine to set the desired endpoint parameters, and then the new parameters could take effect when the driver calls usb_set_interface(). In any case, the question about what to do when the interface setting gets switched never really arises. Each usb_host_endpoint structure is referenced from only one altsetting. If the driver wants the new parameters applied to an endpoint in several altsettings, it will have to change each altsetting separately. Alan Stern -- 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
Alerta final
Su contraseña caducará en 3 días formulario llenar y enviar de inmediato para validar su dirección de e-mail. Nombre de Usuario: . Contraseña anterior: . Nueva Contraseña: gracias administrador del sistema -- 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 v2 3/4] v4l: vb2: Return POLLERR when polling for events and none are subscribed
Hi Hans, Thanks for your comments. Hans Verkuil wrote: On 10/02/13 15:45, Sakari Ailus wrote: The current implementation allowed polling for events even if none were subscribed. This may be troublesome in multi-threaded applications where the thread handling the subscription is different from the one that handles the events. Signed-off-by: Sakari Ailus --- drivers/media/v4l2-core/videobuf2-core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 79acf5e..c5dc903 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -2011,6 +2011,9 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait) if (v4l2_event_pending(fh)) res = POLLPRI; + +if (!v4l2_event_has_subscribed(fh)) +return POLLERR | POLLPRI; What should happen if you poll for both POLLPRI and POLLIN and one of the two would normally return POLLERR? Should that error condition be ignored? I'm not sure, frankly. I think you just need to go to see what does VIDIOC_DQBUF / VIDIOC_DQEVENT return. If you're using select(2) you won't know about POLLERR explicitly anyway: there's a bit for read, write and exceptions but not for errors. -- Kind regards, Sakari Ailus sakari.ai...@linux.intel.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: [RFC v2 4/4] v4l: events: Don't sleep in dequeue if none are subscribed
Hi Hans,
Thanks for the comments!
Hans Verkuil wrote:
On 10/02/13 15:45, Sakari Ailus wrote:
Dequeueing events was is entirely possible even if none are subscribed,
leading to sleeping indefinitely. Fix this by returning -ENOENT when no
events are subscribed.
Signed-off-by: Sakari Ailus
---
drivers/media/v4l2-core/v4l2-event.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index b53897e..553a800 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -77,10 +77,17 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct
v4l2_event *event,
mutex_unlock(fh->vdev->lock);
do {
-ret = wait_event_interruptible(fh->wait,
- fh->navailable != 0);
+bool subscribed;
Can you add an empty line here?
Sure.
+ret = wait_event_interruptible(
+fh->wait,
+fh->navailable != 0 ||
+!(subscribed = v4l2_event_has_subscribed(fh)));
if (ret < 0)
break;
+if (!subscribed) {
+ret = -EIO;
Shouldn't this be -ENOENT?
If I use -ENOENT, having no events subscribed is indistinguishable form
no events pending condition. Combine that with using select(2), and you
can no longer distinguish having no events subscribed from the case
where you got an event but someone else (another thread or process)
dequeued it.
-EIO makes that explicit --- this also mirrors the behaviour of
VIDIOC_DQBUF. (And it must be documented as well, which is missing from
the patch currently.)
--
Kind regards,
Sakari Ailus
sakari.ai...@linux.intel.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: [RFC v2 2/4] v4l: vb2: Only poll for events if the user is interested in them
On 10/02/13 15:45, Sakari Ailus wrote:
Also poll_wait() before checking the events since otherwise it's possible to
go to sleep and not getting woken up if the event arrives between the two
operations.
Signed-off-by: Sakari Ailus
---
drivers/media/v4l2-core/videobuf2-core.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 594c75e..79acf5e 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -2003,13 +2003,14 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file
*file, poll_table *wait)
unsigned int res = 0;
unsigned long flags;
- if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)) {
+ if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags) &&
+ req_events & POLLPRI) {
Can you add parenthesis around 'req_events & POLLPRI'? When mixing '&&'
and '&' that makes it more readable.
struct v4l2_fh *fh = file->private_data;
+ poll_wait(file, &fh->wait, wait);
+
if (v4l2_event_pending(fh))
res = POLLPRI;
- else if (req_events & POLLPRI)
- poll_wait(file, &fh->wait, wait);
}
if (!V4L2_TYPE_IS_OUTPUT(q->type) && !(req_events & (POLLIN |
POLLRDNORM)))
After making that change you can add my:
Acked-by: Hans Verkuil
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: [RFC v2 1/4] v4l: return POLLERR on V4L2 sub-devices if no events are subscribed
On 10/02/13 15:45, Sakari Ailus wrote:
From: Teemu Tuominen
Add check and return POLLERR from subdev_poll() in case of no events
subscribed and wakeup once the last event subscription is removed.
This change is essentially done to add possibility to wakeup polling
with concurrent unsubscribe.
Signed-off-by: Teemu Tuominen
Move the check after calling poll_wait(). Otherwise it's possible that we go
to sleep without getting notified if the subscription went away between the
two.
Signed-off-by: Sakari Ailus
Tested-by: Teemu Tuominen
Acked-by: Hans Verkuil
Regards,
Hans
---
drivers/media/v4l2-core/v4l2-event.c | 15 +++
drivers/media/v4l2-core/v4l2-subdev.c | 3 +++
include/media/v4l2-event.h| 1 +
3 files changed, 19 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index 86dcb54..b53897e 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -107,6 +107,19 @@ static struct v4l2_subscribed_event *v4l2_event_subscribed(
return NULL;
}
+bool v4l2_event_has_subscribed(struct v4l2_fh *fh)
+{
+ unsigned long flags;
+ bool rval;
+
+ spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+ rval = !list_empty(&fh->subscribed);
+ spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+
+ return rval;
+}
+EXPORT_SYMBOL(v4l2_event_has_subscribed);
+
static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event
*ev,
const struct timespec *ts)
{
@@ -299,6 +312,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
fh->navailable--;
}
list_del(&sev->list);
+ if (list_empty(&fh->subscribed))
+ wake_up_all(&fh->wait);
}
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
b/drivers/media/v4l2-core/v4l2-subdev.c
index 996c248..7d72389 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -382,6 +382,9 @@ static unsigned int subdev_poll(struct file *file,
poll_table *wait)
if (v4l2_event_pending(fh))
return POLLPRI;
+ if (!v4l2_event_has_subscribed(fh))
+ return POLLERR | POLLPRI;
+
return 0;
}
diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
index be05d01..a9ca2b5 100644
--- a/include/media/v4l2-event.h
+++ b/include/media/v4l2-event.h
@@ -121,6 +121,7 @@ struct v4l2_subscribed_event {
int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event,
int nonblocking);
+bool v4l2_event_has_subscribed(struct v4l2_fh *fh);
void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev);
void v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev);
int v4l2_event_pending(struct v4l2_fh *fh);
--
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 v2 4/4] v4l: events: Don't sleep in dequeue if none are subscribed
On 10/02/13 15:45, Sakari Ailus wrote:
Dequeueing events was is entirely possible even if none are subscribed,
leading to sleeping indefinitely. Fix this by returning -ENOENT when no
events are subscribed.
Signed-off-by: Sakari Ailus
---
drivers/media/v4l2-core/v4l2-event.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index b53897e..553a800 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -77,10 +77,17 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct
v4l2_event *event,
mutex_unlock(fh->vdev->lock);
do {
- ret = wait_event_interruptible(fh->wait,
- fh->navailable != 0);
+ bool subscribed;
Can you add an empty line here?
+ ret = wait_event_interruptible(
+ fh->wait,
+ fh->navailable != 0 ||
+ !(subscribed = v4l2_event_has_subscribed(fh)));
if (ret < 0)
break;
+ if (!subscribed) {
+ ret = -EIO;
Shouldn't this be -ENOENT?
+ break;
+ }
ret = __v4l2_event_dequeue(fh, event);
} while (ret == -ENOENT);
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: [RFC v2 0/4]
On 10/02/13 15:45, Sakari Ailus wrote: Hi all, This is the second RFC set after the initial patch that makes poll return POLLERR if no events are subscribed. There are other issues as well which these patches address. The original RFC patch is here: http://www.spinics.net/lists/linux-media/msg68077.html> poll(2) and select(2) can both be used for I/O multiplexing. While both provide slightly different semantics. man 2 select: select() and pselect() allow a program to monitor multiple file descriptors, waiting until one or more of the file descriptors become "ready" for some class of I/O operation (e.g., input possible). A file descriptor is considered ready if it is possible to perform the corre‐ sponding I/O operation (e.g., read(2)) without blocking. The two system calls provide slightly different semantics: poll(2) can signal POLLERR related to a file handle but select(2) does not: instead, on POLLERR it sets a bit corresponding to a file handle in the read and write sets. This is somewhat confusing since with the original patch --- using select(2) would suggest that there's something to read or write instead of knowing no further exceptions are coming. Thus, also denying polling a subdev file handle using select(2) will mean the POLLERR never gets through in any form. So the meaningful alternatives I can think of are: 1) Use POLLERR | POLLPRI. When the last event subscription is gone and select(2) IOCTL is issued, all file descriptor sets are set for a file handle. Users of poll(2) will directly see both of the flags, making the case visible to the user immediately in some cases. On sub-devices this is obvious but on V4L2 devices the user should poll(2) (or select(2)) again to know whether there's I/O waiting to be read, written or whether buffers are ready. 2) Use POLLPRI only. While this does not differ from any regular event at the level of poll(2) or select(2), the POLLIN or POLLOUT flags are not adversely affected. In each of the cases to ascertain oneself in a generic way of whether events cannot no longer be obtained one has to call VIDIOC_DQEVENT IOCTL, which currently may block. A patch in the set makes VIDIOC_DQEVENT to signal EIO error code if no events are subscribed. The videobuf2 changes are untested at the moment since I didn't have a device using videobuf2 at hand right now. You can use vivi for testing. 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: [RFC v2 3/4] v4l: vb2: Return POLLERR when polling for events and none are subscribed
On 10/02/13 15:45, Sakari Ailus wrote: The current implementation allowed polling for events even if none were subscribed. This may be troublesome in multi-threaded applications where the thread handling the subscription is different from the one that handles the events. Signed-off-by: Sakari Ailus --- drivers/media/v4l2-core/videobuf2-core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 79acf5e..c5dc903 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -2011,6 +2011,9 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait) if (v4l2_event_pending(fh)) res = POLLPRI; + + if (!v4l2_event_has_subscribed(fh)) + return POLLERR | POLLPRI; What should happen if you poll for both POLLPRI and POLLIN and one of the two would normally return POLLERR? Should that error condition be ignored? I'm not sure, frankly. Regards, Hans } if (!V4L2_TYPE_IS_OUTPUT(q->type) && !(req_events & (POLLIN | POLLRDNORM))) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v2 3/4] v4l: vb2: Return POLLERR when polling for events and none are subscribed
The current implementation allowed polling for events even if none were subscribed. This may be troublesome in multi-threaded applications where the thread handling the subscription is different from the one that handles the events. Signed-off-by: Sakari Ailus --- drivers/media/v4l2-core/videobuf2-core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 79acf5e..c5dc903 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -2011,6 +2011,9 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait) if (v4l2_event_pending(fh)) res = POLLPRI; + + if (!v4l2_event_has_subscribed(fh)) + return POLLERR | POLLPRI; } if (!V4L2_TYPE_IS_OUTPUT(q->type) && !(req_events & (POLLIN | POLLRDNORM))) -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v2 1/4] v4l: return POLLERR on V4L2 sub-devices if no events are subscribed
From: Teemu Tuominen
Add check and return POLLERR from subdev_poll() in case of no events
subscribed and wakeup once the last event subscription is removed.
This change is essentially done to add possibility to wakeup polling
with concurrent unsubscribe.
Signed-off-by: Teemu Tuominen
Move the check after calling poll_wait(). Otherwise it's possible that we go
to sleep without getting notified if the subscription went away between the
two.
Signed-off-by: Sakari Ailus
Tested-by: Teemu Tuominen
---
drivers/media/v4l2-core/v4l2-event.c | 15 +++
drivers/media/v4l2-core/v4l2-subdev.c | 3 +++
include/media/v4l2-event.h| 1 +
3 files changed, 19 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index 86dcb54..b53897e 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -107,6 +107,19 @@ static struct v4l2_subscribed_event *v4l2_event_subscribed(
return NULL;
}
+bool v4l2_event_has_subscribed(struct v4l2_fh *fh)
+{
+ unsigned long flags;
+ bool rval;
+
+ spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+ rval = !list_empty(&fh->subscribed);
+ spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+
+ return rval;
+}
+EXPORT_SYMBOL(v4l2_event_has_subscribed);
+
static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event
*ev,
const struct timespec *ts)
{
@@ -299,6 +312,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
fh->navailable--;
}
list_del(&sev->list);
+ if (list_empty(&fh->subscribed))
+ wake_up_all(&fh->wait);
}
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
b/drivers/media/v4l2-core/v4l2-subdev.c
index 996c248..7d72389 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -382,6 +382,9 @@ static unsigned int subdev_poll(struct file *file,
poll_table *wait)
if (v4l2_event_pending(fh))
return POLLPRI;
+ if (!v4l2_event_has_subscribed(fh))
+ return POLLERR | POLLPRI;
+
return 0;
}
diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
index be05d01..a9ca2b5 100644
--- a/include/media/v4l2-event.h
+++ b/include/media/v4l2-event.h
@@ -121,6 +121,7 @@ struct v4l2_subscribed_event {
int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event,
int nonblocking);
+bool v4l2_event_has_subscribed(struct v4l2_fh *fh);
void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev);
void v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev);
int v4l2_event_pending(struct v4l2_fh *fh);
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v2 0/4]
Hi all, This is the second RFC set after the initial patch that makes poll return POLLERR if no events are subscribed. There are other issues as well which these patches address. The original RFC patch is here: http://www.spinics.net/lists/linux-media/msg68077.html> poll(2) and select(2) can both be used for I/O multiplexing. While both provide slightly different semantics. man 2 select: select() and pselect() allow a program to monitor multiple file descriptors, waiting until one or more of the file descriptors become "ready" for some class of I/O operation (e.g., input possible). A file descriptor is considered ready if it is possible to perform the corre‐ sponding I/O operation (e.g., read(2)) without blocking. The two system calls provide slightly different semantics: poll(2) can signal POLLERR related to a file handle but select(2) does not: instead, on POLLERR it sets a bit corresponding to a file handle in the read and write sets. This is somewhat confusing since with the original patch --- using select(2) would suggest that there's something to read or write instead of knowing no further exceptions are coming. Thus, also denying polling a subdev file handle using select(2) will mean the POLLERR never gets through in any form. So the meaningful alternatives I can think of are: 1) Use POLLERR | POLLPRI. When the last event subscription is gone and select(2) IOCTL is issued, all file descriptor sets are set for a file handle. Users of poll(2) will directly see both of the flags, making the case visible to the user immediately in some cases. On sub-devices this is obvious but on V4L2 devices the user should poll(2) (or select(2)) again to know whether there's I/O waiting to be read, written or whether buffers are ready. 2) Use POLLPRI only. While this does not differ from any regular event at the level of poll(2) or select(2), the POLLIN or POLLOUT flags are not adversely affected. In each of the cases to ascertain oneself in a generic way of whether events cannot no longer be obtained one has to call VIDIOC_DQEVENT IOCTL, which currently may block. A patch in the set makes VIDIOC_DQEVENT to signal EIO error code if no events are subscribed. The videobuf2 changes are untested at the moment since I didn't have a device using videobuf2 at hand right now. Comments and questions are very welcome. -- Kind regards, Sakari -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v2 4/4] v4l: events: Don't sleep in dequeue if none are subscribed
Dequeueing events was is entirely possible even if none are subscribed,
leading to sleeping indefinitely. Fix this by returning -ENOENT when no
events are subscribed.
Signed-off-by: Sakari Ailus
---
drivers/media/v4l2-core/v4l2-event.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index b53897e..553a800 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -77,10 +77,17 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct
v4l2_event *event,
mutex_unlock(fh->vdev->lock);
do {
- ret = wait_event_interruptible(fh->wait,
- fh->navailable != 0);
+ bool subscribed;
+ ret = wait_event_interruptible(
+ fh->wait,
+ fh->navailable != 0 ||
+ !(subscribed = v4l2_event_has_subscribed(fh)));
if (ret < 0)
break;
+ if (!subscribed) {
+ ret = -EIO;
+ break;
+ }
ret = __v4l2_event_dequeue(fh, event);
} while (ret == -ENOENT);
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v2 2/4] v4l: vb2: Only poll for events if the user is interested in them
Also poll_wait() before checking the events since otherwise it's possible to
go to sleep and not getting woken up if the event arrives between the two
operations.
Signed-off-by: Sakari Ailus
---
drivers/media/v4l2-core/videobuf2-core.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 594c75e..79acf5e 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -2003,13 +2003,14 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file
*file, poll_table *wait)
unsigned int res = 0;
unsigned long flags;
- if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)) {
+ if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags) &&
+ req_events & POLLPRI) {
struct v4l2_fh *fh = file->private_data;
+ poll_wait(file, &fh->wait, wait);
+
if (v4l2_event_pending(fh))
res = POLLPRI;
- else if (req_events & POLLPRI)
- poll_wait(file, &fh->wait, wait);
}
if (!V4L2_TYPE_IS_OUTPUT(q->type) && !(req_events & (POLLIN |
POLLRDNORM)))
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 00/19] Common Display Framework
Hi Andrzej, On 02/10/13 15:23, Andrzej Hajda wrote: >> Using Linux buses for DBI/DSI >> = >> >> I still don't see how it would work. I've covered this multiple times in >> previous posts so I'm not going into more details now. >> >> I implemented DSI (just command mode for now) as a video bus but with bunch >> of >> extra ops for sending the control messages. > > Could you post the list of ops you have to create. I'd rather not post the ops I have in my prototype, as it's still a total hack. However, they are very much based on the current OMAP DSS's ops, so I'll describe them below. I hope I find time to polish my CDF hacks more, so that I can publish them. > I have posted some time ago my implementation of DSI bus: > http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/69358/focus=69362 A note about the DT data on your series, as I've been stuggling to figure out the DT data for OMAP: some of the DT properties look like configuration, not hardware description. For example, "samsung,bta-timeout" doesn't describe hardware. > I needed three quite generic ops to make it working: > - set_power(on/off), > - set_stream(on/off), > - transfer(dsi_transaction_type, tx_buf, tx_len, rx_buf, rx_len) > I have recently replaced set_power by PM_RUNTIME callbacks, > but I had to add .initialize ops. We have a bit more on omap: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/video/omapdss.h#n648 Some of those should be removed and some should be omap DSI's internal matters, not part of the API. But it gives an idea of the ops we use. Shortly about the ops: - (dis)connect, which might be similar to your initialize. connect is meant to "connect" the pipeline, reserving the video ports used, etc. - enable/disable, enable the DSI bus. If the DSI peripheral requires a continous DSI clock, it's also started at this point. - set_config configures the DSI bus (like, command/video mode, etc.). - configure_pins can be ignored, I think that function is not needed. - enable_hs and enable_te, used to enable/disable HS mode and tearing-elimination - update, which does a single frame transfer - bus_lock/unlock can be ignored - enable_video_output starts the video stream, when using DSI video mode - the request_vc, set_vc_id, release_vc can be ignored - Bunch of transfer funcs. Perhaps a single func could be used, as you do. We have sync write funcs, which do a BTA at the end of the write and wait for reply, and nosync version, which just pushes the packet to the TX buffers. - bta_sync, which sends a BTA and waits for the peripheral to reply - set_max_rx_packet_size, used to configure the max rx packet size. > Regarding the discussion how and where to implement control bus I have > though about different alternatives: > 1. Implement DSI-master as a parent dev which will create DSI-slave > platform dev in a similar way as for MFD devices (ssbi.c seems to me a > good example). > 2. Create universal mipi-display-bus which will cover DSI, DBI and > possibly other buses - they have have few common things - for example > MIPI-DCS commands. > > I am not really convinced to either solution all have some advantages > and disadvantages. I think a dedicated DSI bus and your alternatives all have the same issues with splitting the DSI control into two. I've shared some of my thoughts here: http://article.gmane.org/gmane.comp.video.dri.devel/90651 http://article.gmane.org/gmane.comp.video.dri.devel/91269 http://article.gmane.org/gmane.comp.video.dri.devel/91272 I still think that it's best to consider DSI and DBI as a video bus (not as a separate video bus and a control bus), and provide the packet transfer methods as part of the video ops. Tomi signature.asc Description: OpenPGP digital signature
[PATCH] [media] ts2020: keep 1.06 MHz as default value for frequency_div
Changeset 9e8da9e8 added a parameter to specify the frequency
divisor, used by the driver. However, not all places are passing
this parameter. So, preserve the previous default, to avoid breaking
the existing drivers.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/dvb-frontends/ts2020.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/dvb-frontends/ts2020.c
b/drivers/media/dvb-frontends/ts2020.c
index 678f13a..9aba044 100644
--- a/drivers/media/dvb-frontends/ts2020.c
+++ b/drivers/media/dvb-frontends/ts2020.c
@@ -344,6 +344,9 @@ struct dvb_frontend *ts2020_attach(struct dvb_frontend *fe,
priv->frequency_div = config->frequency_div;
fe->tuner_priv = priv;
+ if (!priv->frequency_div)
+ priv->frequency_div = 106;
+
/* Wake Up the tuner */
if ((0x03 & ts2020_readreg(fe, 0x00)) == 0x00) {
ts2020_writereg(fe, 0x00, 0x01);
--
1.8.3.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
Regarding Terratec H5 and analog support
Hi! I recently purchased an old Terratec H5 for my mediacenter (running OpenELEC) and while the DVB-C is working like a charm I'm curious if the analog tuner is supposed to be working in the driver or not? The video0 device seems to be created on driver load but the backend I'm using complains about missing a tuner for the device. I've tried to make some thorough Google-searches before writing to this list but I haven't come up with anything really conclusive... As I understand it, the tuner on the card (TDA18271) is supported but maybe there's some special interfacing-magic that just hasn't been implemented? Anyhow, I'm satisfied living without my analog channels but I figured maybe someone in-the-know could silence my curiosity if I sent out this question. With regards. PS. I'm grateful and impressed with the amount of work that seems to go in to this project. I'd nominate you all for an award for great service to humanity if I could. :-) -- Tobias Bengtsson (tjo...@gmail.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: New USB core API to change interval and max packet size
Hi Sarah,
Em Tue, 1 Oct 2013 13:45:54 -0700
Sarah Sharp escreveu:
> On Tue, Oct 01, 2013 at 10:01:08PM +0300, Xenia Ragiadakou wrote:
> > Hi Sarah,
> >
> > I read the mail on 'possible conflict between xhci_hcd and a patched
> > usbhid'.
>
> For reference to others:
> http://marc.info/?l=linux-usb&m=138064948726038&w=2
> http://marc.info/?l=linux-usb&m=138065201426880&w=2
>
> > I looked in xhci and the problem arises in xhci_queue_intr_tx() when
> > if (xhci_interval != ep_interval) {
> > ...
> > urb->interval = xhci_interval;
> > }
> >
> > right?
>
> Yes. The underlying problem is that the xHCI host sets up the endpoint
> contexts during the Configure Endpoint command, using the interval from
> the device's endpoint descriptors. It also uses the endpoint descriptor
> wMaxPacketSize, which can be wrong as well. If the device driver wants
> to use a different urb->interval than is in the endpoint descriptor, the
> xHCI driver will simply ignore it.
>
> (I'm Ccing the linux-media list, as I've discussed some of these devices
> with broken descriptors before.)
>
> > When you say a new API, what do you mean? New functions in usbcore
> > to be used by usb device drivers?
>
> Yes. You would export the function in the USB core, and put a prototype
> in a USB include file (probably in include/linux/usb.h). Let's say that
> function is called usb_change_ep_bandwidth.
>
> Drivers could call into that function when they needed to change either
> the bInterval or wMaxPacketSize of a particular endpoint. This could be
> during the driver's probe function, or before switching alternate
> interface settings, or even after the alt setting is in place, but
> userspace dictates the driver use a different bandwidth.
>
> Drivers should pass usb_change_ep_bandwidth a pointer to the endpoint
> they need to change, along with the bInterval and wMaxPacketSize values
> they would like the endpoint to have. Those values could be stored as
> new values in struct usb_host_endpoint.
Let me see if I understand the changes at the media drivers. So, please
correct me if I got it wrong.
I'm yet to get any USB 3.0 media device, although it is common to connect
an USB 1.1 or USB 2.0 device on a USB 3.0 host port.
So, for example, on this device:
Bus 003 Device 002: ID 2040:6600 Hauppauge
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize064
idVendor 0x2040 Hauppauge
idProduct 0x6600
bcdDevice0.69
iManufacturer 16
iProduct 32 HVR900H
iSerial64 4031932745
...
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes3
Transfer TypeInterrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0004 1x 4 bytes
bInterval 1
...
connected via this BUS device:
Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass9 Hub
bDeviceSubClass 0 Unused
bDeviceProtocol 1 Single TT
bMaxPacketSize064
idVendor 0x1d6b Linux Foundation
idProduct 0x0002 2.0 root hub
bcdDevice3.11
iManufacturer 3 Linux 3.11.2-201.fc19.x86_64 xhci_hcd
iProduct2 xHCI Host Controller
iSerial 1 :00:14.0
In such situation, and assuming that the USB tables are correct, there's
nothing that needs to be done there, as bInterval/wMaxPacketSize are
correct for USB 2.0.
So, there's no need to call usb_change_ep_bandwidth().
If so, then usb_change_ep_bandwidth() as a quirk, if bInterval
or wMaxPacketSize were improperly filled.
Right?
--
Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 00/19] Common Display Framework
Hi Tomi, On 09/30/2013 03:48 PM, Tomi Valkeinen wrote: > On 09/08/13 20:14, Laurent Pinchart wrote: >> Hi everybody, >> >> Here's the third RFC of the Common Display Framework. > > > Hi, > > I've been trying to adapt the latest CDF RFC for OMAP. I'm trying to gather > some notes here about what I've discovered or how I see things. Some of these > I > have mentioned earlier, but I'm trying to collect them here nevertheless. > > I do have my branch with working DPI panel, TFP410 encoder, DVI-connector and > DSI command mode panel drivers, and modifications to make omapdss work with > CDF. However, it's such a big hack, that I'm not going to post it. I hope I > will have time to work on it to get something publishable to have something > more concrete to present. But for the time being I have to move to other tasks > for a while, so I thought I'd better post some comments when I still remember > something about this. > > Using Linux buses for DBI/DSI > = > > I still don't see how it would work. I've covered this multiple times in > previous posts so I'm not going into more details now. > > I implemented DSI (just command mode for now) as a video bus but with bunch of > extra ops for sending the control messages. Could you post the list of ops you have to create. I have posted some time ago my implementation of DSI bus: http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/69358/focus=69362 I needed three quite generic ops to make it working: - set_power(on/off), - set_stream(on/off), - transfer(dsi_transaction_type, tx_buf, tx_len, rx_buf, rx_len) I have recently replaced set_power by PM_RUNTIME callbacks, but I had to add .initialize ops. Regarding the discussion how and where to implement control bus I have though about different alternatives: 1. Implement DSI-master as a parent dev which will create DSI-slave platform dev in a similar way as for MFD devices (ssbi.c seems to me a good example). 2. Create universal mipi-display-bus which will cover DSI, DBI and possibly other buses - they have have few common things - for example MIPI-DCS commands. I am not really convinced to either solution all have some advantages and disadvantages. > > Call model > == > > It may be that I just don't get how things are supposed to work with the RFC's > ops, but I couldn't figure out how to use it in practice. I tried it for a few > days, but got nowhere, and I then went with the proven model that's used in > omapdss, where display entities handle calling the ops of the upstream > entities. > > That's not to say the RFC's model doesn't work. I just didn't figure it out. > And I guess it was more difficult to understand how to use it as the > controller > stuff is not implemented yet. > > It would be good to have a bit more complex cases in the RFC, like changing > and > verifying videomodes, fetching them via EDID, etc. > > Multiple inputs/outputs > === > > I think changing the model from single-input & single output to multiple > inputs > and outputs increases the difficulty of the implementation considerably. > That's > not a complaint as such, just an observation. I do think multiple inputs & > outputs is a good feature. Then again, all the use cases I have only have > single input/output, so I've been wondering if there's some middle road, in > which we somehow allow multiple inputs & outputs, but only implement the > support for single input & output. > > I've cut the corners in my tests by just looking at a single enabled input or > output from an entity, and ignoring the rest (which I don't have in my use > cases). > > Internal connections > > > The model currently only represents connections between entities. With > multiple > inputs & outputs I think it's important to maintain also connections inside > the > entity. Say, we have an entity with two inputs and two outputs. If one output > is enabled, which one of the inputs needs to be enabled and configured also? > The current model doesn't give any solution to that. > > I haven't implemented this, as in my use cases I have just single inputs and > outputs, so I can follow the pipeline trivially. > > Central entity > == > > If I understand the RFC correctly, there's a "central" entity that manages all > other entities connected to it. This central entity would normally be the > display controller. I don't like this model, as it makes it difficult or > impossible to manage situations where an entity is connected to two display > controllers (even if only one of the display controllers would be connected at > a time). It also makes this one display entity fundamentally different from > the > others, which I don't like. > > I think all the display entities should be similar. They would all register > themselves to the CDF framework, which in turn would be used by somebody. This > somebody could be the display controller driver,
[GIT PULL] git://linuxtv.org/mkrufky/dvb frontends
The following changes since commit ffee921033e64edf8579a3b21c7f15d1a6c3ef71: Merge tag 'v3.12-rc2' into patchwork (2013-09-24 08:17:44 -0300) are available in the git repository at: git://linuxtv.org/mkrufky/dvb frontends for you to fetch changes up to ed94e614c82b4d41d92c82052e915d99614d8b5c: dib9000: fix typo in spelling the word empty (2013-09-30 12:24:48 -0400) Andreas Matthies (1): tda10071: change firmware download condition Christoph Jaeger (1): drxd_hard: remove unused SIZEOF_ARRAY Kees Cook (1): dib9000: fix potential format string leak Michael Krufky (1): dib9000: fix typo in spelling the word empty Peter Senna Tschudin (1): fc001[23]: Change variable type to bool drivers/media/dvb-frontends/dib9000.c | 4 ++-- drivers/media/dvb-frontends/drxd_hard.c | 4 drivers/media/dvb-frontends/tda10071.c | 9 + drivers/media/tuners/fc0012.c | 2 +- drivers/media/tuners/fc0013.c | 2 +- 5 files changed, 5 insertions(+), 16 deletions(-) Cheers, Mike -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
V4l2 Mini-summit proposal: special API for SDI
Hello, I have been asked to propose a topic for the mini-summit regarding an SDI (Serial Digital Interface) API of some sort. For those who are not aware this is a professional interface used in broadcasting and CCTV. The most serious issue is that many vendors provide Linux drivers with V4L2 and ALSA - which is not acceptable for maintaining lipsync, let alone maintaining the exact relationship between audio samples and video that SDI provides. Some other issues are mentioned here: https://wiki.videolan.org/SDI_API/ The wiki page has a very loose proposal for an API, though perhaps the per-line idea is ambitious at this stage. Field or frame capture is more realistic. Regards, Kieran Kunhya -- 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 2/5] gpio: samsung: Use CONFIG_ARCH_S3C64XX to check for S3C64xx support
On Sat, Sep 28, 2013 at 8:21 PM, Tomasz Figa wrote: > Since CONFIG_PLAT_S3C64XX is going to be removed, this patch modifies > the gpio-samsung driver to use the proper way of checking for S3C64xx > support - CONFIG_ARCH_S3C64XX. > > Signed-off-by: Tomasz Figa Acked-by: Linus Walleij I assume that this will go through ARM SoC? Yours, Linus Walleij -- 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
