Re: [PATCHv8 18/26] v4l: add buffer exporting via dmabuf
On Thu August 23 2012 01:39:34 Laurent Pinchart wrote:
> Hi Hans,
>
> On Wednesday 22 August 2012 13:41:05 Hans Verkuil wrote:
> > On Tue August 14 2012 17:34:48 Tomasz Stanislawski wrote:
> > > This patch adds extension to V4L2 api. It allow to export a mmap buffer as
> > > file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer
> > > offset used by mmap and return a file descriptor on success.
> > >
> > > Signed-off-by: Tomasz Stanislawski
> > > Signed-off-by: Kyungmin Park
>
> [snip]
>
> > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > > index 7f918dc..b5d058b 100644
> > > --- a/include/linux/videodev2.h
> > > +++ b/include/linux/videodev2.h
> > > @@ -688,6 +688,31 @@ struct v4l2_buffer {
> > >
> > > #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE0x0800
> > > #define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000
> > >
> > > +/**
> > > + * struct v4l2_exportbuffer - export of video buffer as DMABUF file
> > > descriptor + *
> > > + * @fd: file descriptor associated with DMABUF (set by driver)
> > > + * @mem_offset: buffer memory offset as returned by VIDIOC_QUERYBUF in
> > > struct + *v4l2_buffer::m.offset (for single-plane
> > > formats) or
> > > + * v4l2_plane::m.offset (for multi-planar formats)
> > > + * @flags: flags for newly created file, currently only O_CLOEXEC
> > > is
> > > + * supported, refer to manual of open syscall for more
> > > details
> > > + *
> > > + * Contains data used for exporting a video buffer as DMABUF file
> > > descriptor. + * The buffer is identified by a 'cookie' returned by
> > > VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to
> > > userspace). All + * reserved fields must be set to zero. The field
> > > reserved0 is expected to + * become a structure 'type' allowing an
> > > alternative layout of the structure + * content. Therefore this field
> > > should not be used for any other extensions. + */
> > > +struct v4l2_exportbuffer {
> > > + __u32 fd;
> > > + __u32 reserved0;
> > > + __u32 mem_offset;
> > > + __u32 flags;
> > > + __u32 reserved[12];
> > > +};
> >
> > OK, I realized that we also need a type field here: you need the type field
> > (same as in v4l2_buffer) to know which queue the mem_offset refers to. For
> > M2M devices you have two queues, so you need this information.
> >
> > Is there any reason not to use __u32 memory instead of __u32 reserved0?
> > I really dislike 'reserved0'. It's also very inconsistent with the other
> > buffer ioctls which all have type+memory fields.
>
> I'm concerned that we might need to export buffers in the future based on
> something else than the memory type. That's probably an irrational fear
> though.
We're exporting buffers from the V4L2 core. The only method of creating such
buffers is through REQBUFS/CREATE_BUFS, both of which use the memory field.
Even if we need something else in the future, then there is nothing preventing
us from extending the memory enum.
> > Regarding mem_offset: I would prefer a union (possibly anonymous):
> >
> > union {
> > __u32 mem_offset;
> > unsigned long reserved;
> > } m;
> >
> > Again, it's more consistent with the existing buffer ioctls, and it prepares
> > the API for future pointer values. 'reserved' in the union above could even
> > safely be renamed to userptr, even though userptr isn't supported at the
> > moment.
>
> Once again I would really want to see compeling use cases before we export a
> userptr buffer as a dmabuf object. As Mauro previously stated, userptr for
> buffer sharing was a hack in the first place (although a pretty successful
> one
> so far).
I don't want to export a userptr, I want to make sure we *can* export a
pointer-sized thing in the future. Which is why in my proposal above I'm
calling it reserved and not userptr.
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 0/5] Generic panel framework
Hi Laurent, Do you plan to add an API to get and parse EDID to mode list? video mode is tightly coupled with panel that is capable of hot-plug. Or you are busy on modifying EDID parsing code for sharing it amoung DRM/FB/etc? I see you mentioned this in Mar. It is great if you are considering add more info into video mode, such as pixel repeating, 3D timing related parameter. I have some code for CEA modes filtering and 3D parsing, but still tight coupled with FB and with a little hack style. My HDMI driver is implemented as lcd device as you mentioned here. But more complex than other lcd devices for a kthread is handling hot-plug/EDID/HDCP/ASoC etc. I also feel a little weird to add code parsing HDMI audio related info in fbmod.c in my current implementation, thought it is the only place to handle EDID in kernel. Your panel framework provide a better place to add panel related audio/HDCP code. panel notifier can also trigger hot-plug related feature, such as HDCP start. Looking forward to your hot-plug panel patch. Or I can help add it if you would like me to. Thanks! Jun -- 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] m2m-deinterlace: Add V4L2_CAP_VIDEO_M2M capability flag
On 22 August 2012 23:00, Sylwester Nawrocki wrote: > New mem-to-mem video drivers should use V4L2_CAP_VIDEO_M2M capability, rather > than ORed V4L2_CAP_VIDEO_CAPTURE and V4L2_CAP_VIDEO_OUTPUT flags, as outlined > in commit a1367f1b260d29e9b9fb20d8e2f39f1e74fa6c3b. > > Cc: Javier Martin > Signed-off-by: Sylwester Nawrocki > --- > drivers/media/platform/m2m-deinterlace.c |9 +++-- > 1 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/m2m-deinterlace.c > b/drivers/media/platform/m2m-deinterlace.c > index a38c152..5c7df67 100644 > --- a/drivers/media/platform/m2m-deinterlace.c > +++ b/drivers/media/platform/m2m-deinterlace.c > @@ -456,8 +456,13 @@ static int vidioc_querycap(struct file *file, void *priv, > strlcpy(cap->driver, MEM2MEM_NAME, sizeof(cap->driver)); > strlcpy(cap->card, MEM2MEM_NAME, sizeof(cap->card)); > strlcpy(cap->bus_info, MEM2MEM_NAME, sizeof(cap->card)); > - cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_OUTPUT > - | V4L2_CAP_STREAMING; > + /* > +* This is only a mem-to-mem video device. The capture and output > +* device capability flags are left only for backward compatibility > +* and are scheduled for removal. > +*/ > + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_OUTPUT | > + V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING; > cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; > > return 0; > -- > 1.7.4.1 > Acked-by: Javier Martin -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.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] coda: Add V4L2_CAP_VIDEO_M2M capability flag
On 22 August 2012 23:00, Sylwester Nawrocki wrote: > New mem-to-mem video drivers should use V4L2_CAP_VIDEO_M2M capability, rather > than ORed V4L2_CAP_VIDEO_CAPTURE and V4L2_CAP_VIDEO_OUTPUT flags, as outlined > in commit a1367f1b260d29e9b9fb20d8e2f39f1e74fa6c3b. > > Cc: Javier Martin > Signed-off-by: Sylwester Nawrocki > --- > drivers/media/platform/coda.c |9 +++-- > 1 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c > index 0d6e0a0..e74705c 100644 > --- a/drivers/media/platform/coda.c > +++ b/drivers/media/platform/coda.c > @@ -287,8 +287,13 @@ static int vidioc_querycap(struct file *file, void *priv, > strlcpy(cap->driver, CODA_NAME, sizeof(cap->driver)); > strlcpy(cap->card, CODA_NAME, sizeof(cap->card)); > strlcpy(cap->bus_info, CODA_NAME, sizeof(cap->bus_info)); > - cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_OUTPUT > - | V4L2_CAP_STREAMING; > + /* > +* This is only a mem-to-mem video device. The capture and output > +* device capability flags are left only for backward compatibility > +* and are scheduled for removal. > +*/ > + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_OUTPUT | > + V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING; > cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; > > return 0; > -- > 1.7.4.1 > Acked-by: Javier Martin -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.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
[PATCH 1/2] dvb_usb_v2: add debug macro dvb_usb_dbg_usb_control_msg
For dumping usb_control_msg().
Signed-off-by: Antti Palosaari
---
drivers/media/usb/dvb-usb-v2/dvb_usb.h | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb.h
b/drivers/media/usb/dvb-usb-v2/dvb_usb.h
index 5a53c62..bae16a1 100644
--- a/drivers/media/usb/dvb-usb-v2/dvb_usb.h
+++ b/drivers/media/usb/dvb-usb-v2/dvb_usb.h
@@ -63,6 +63,17 @@
#define fe_to_priv(fe) (fe_to_d(fe)->priv)
#define d_to_priv(d) (d->priv)
+#define dvb_usb_dbg_usb_control_msg(udev, r, t, v, i, b, l) { \
+ char *direction; \
+ if (t == (USB_TYPE_VENDOR | USB_DIR_OUT)) \
+ direction = ">>>"; \
+ else \
+ direction = "<<<"; \
+ dev_dbg(&udev->dev, "%s: %02x %02x %02x %02x %02x %02x %02x %02x " \
+ "%s %*ph\n", __func__, t, r, v & 0xff, v >> 8, \
+ i & 0xff, i >> 8, l & 0xff, l >> 8, direction, l, b); \
+}
+
#define DVB_USB_STREAM_BULK(endpoint_, count_, size_) { \
.type = USB_BULK, \
.count = count_, \
--
1.7.11.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 2/2] dvb_usb_v2: use dvb_usb_dbg_usb_control_msg()
Convert drivers: au6610, ce6230, ec168, rtl28xxu for
dvb_usb_dbg_usb_control_msg() macro.
Signed-off-by: Antti Palosaari
---
drivers/media/usb/dvb-usb-v2/au6610.c | 5 +
drivers/media/usb/dvb-usb-v2/ce6230.c | 4 ++--
drivers/media/usb/dvb-usb-v2/ce6230.h | 11 ---
drivers/media/usb/dvb-usb-v2/ec168.c| 4 ++--
drivers/media/usb/dvb-usb-v2/ec168.h| 11 ---
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 6 --
drivers/media/usb/dvb-usb-v2/rtl28xxu.h | 11 ---
7 files changed, 13 insertions(+), 39 deletions(-)
diff --git a/drivers/media/usb/dvb-usb-v2/au6610.c
b/drivers/media/usb/dvb-usb-v2/au6610.c
index c126b70..f309fd8 100644
--- a/drivers/media/usb/dvb-usb-v2/au6610.c
+++ b/drivers/media/usb/dvb-usb-v2/au6610.c
@@ -56,6 +56,11 @@ static int au6610_usb_msg(struct dvb_usb_device *d, u8
operation, u8 addr,
ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0), operation,
USB_TYPE_VENDOR|USB_DIR_IN, addr << 1, index,
usb_buf, 6, AU6610_USB_TIMEOUT);
+
+ dvb_usb_dbg_usb_control_msg(d->udev, operation,
+ (USB_TYPE_VENDOR|USB_DIR_IN), addr << 1, index,
+ usb_buf, 6);
+
if (ret < 0)
goto error;
diff --git a/drivers/media/usb/dvb-usb-v2/ce6230.c
b/drivers/media/usb/dvb-usb-v2/ce6230.c
index 819db9c..1c4357d 100644
--- a/drivers/media/usb/dvb-usb-v2/ce6230.c
+++ b/drivers/media/usb/dvb-usb-v2/ce6230.c
@@ -74,8 +74,8 @@ static int ce6230_ctrl_msg(struct dvb_usb_device *d, struct
usb_req *req)
ret = usb_control_msg(d->udev, pipe, request, requesttype, value, index,
buf, req->data_len, CE6230_USB_TIMEOUT);
- ce6230_debug_dump(request, requesttype, value, index, buf,
- req->data_len);
+ dvb_usb_dbg_usb_control_msg(d->udev, request, requesttype, value, index,
+ buf, req->data_len);
if (ret < 0)
pr_err("%s: usb_control_msg() failed=%d\n", KBUILD_MODNAME,
diff --git a/drivers/media/usb/dvb-usb-v2/ce6230.h
b/drivers/media/usb/dvb-usb-v2/ce6230.h
index 42d7544..299e57e 100644
--- a/drivers/media/usb/dvb-usb-v2/ce6230.h
+++ b/drivers/media/usb/dvb-usb-v2/ce6230.h
@@ -26,17 +26,6 @@
#include "zl10353.h"
#include "mxl5005s.h"
-#define ce6230_debug_dump(r, t, v, i, b, l) { \
- char *direction; \
- if (t == (USB_TYPE_VENDOR | USB_DIR_OUT)) \
- direction = ">>>"; \
- else \
- direction = "<<<"; \
- pr_debug("%s: %02x %02x %02x %02x %02x %02x %02x %02x %s [%d bytes]\n",
\
-__func__, t, r, v & 0xff, v >> 8, i & 0xff, i >> 8, \
- l & 0xff, l >> 8, direction, l); \
-}
-
#define CE6230_USB_TIMEOUT 1000
struct usb_req {
diff --git a/drivers/media/usb/dvb-usb-v2/ec168.c
b/drivers/media/usb/dvb-usb-v2/ec168.c
index ab77622..b74c810 100644
--- a/drivers/media/usb/dvb-usb-v2/ec168.c
+++ b/drivers/media/usb/dvb-usb-v2/ec168.c
@@ -86,8 +86,8 @@ static int ec168_ctrl_msg(struct dvb_usb_device *d, struct
ec168_req *req)
ret = usb_control_msg(d->udev, pipe, request, requesttype, req->value,
req->index, buf, req->size, EC168_USB_TIMEOUT);
- ec168_debug_dump(request, requesttype, req->value, req->index, buf,
- req->size);
+ dvb_usb_dbg_usb_control_msg(d->udev, request, requesttype, req->value,
+ req->index, buf, req->size);
if (ret < 0)
goto err_dealloc;
diff --git a/drivers/media/usb/dvb-usb-v2/ec168.h
b/drivers/media/usb/dvb-usb-v2/ec168.h
index 9181236..f651808 100644
--- a/drivers/media/usb/dvb-usb-v2/ec168.h
+++ b/drivers/media/usb/dvb-usb-v2/ec168.h
@@ -24,17 +24,6 @@
#include "dvb_usb.h"
-#define ec168_debug_dump(r, t, v, i, b, l) { \
- char *direction; \
- if (t == (USB_TYPE_VENDOR | USB_DIR_OUT)) \
- direction = ">>>"; \
- else \
- direction = "<<<"; \
- pr_debug("%s: %02x %02x %02x %02x %02x %02x %02x %02x %s\n", \
-__func__, t, r, v & 0xff, v >> 8, i & 0xff, i >> 8, \
- l & 0xff, l >> 8, direction); \
-}
-
#define EC168_USB_TIMEOUT 1000
struct ec168_req {
diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index c246c50..e29fca2 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -59,11 +59,13 @@ static int rtl28xxu_ctrl_msg(struct dvb_usb_device *d,
struct rtl28xxu_req *req)
ret = usb_control_msg(d->udev, pipe, 0, requesttype, req->value,
req->index, buf, req->size, 1000);
+
+ dvb_usb_dbg_usb_control_msg(d->udev, 0, requesttype, req->value,
+ req->index, buf, req->size);
+
if (ret > 0)
ret = 0;
- deb_dump(0, requesttype, req->
Re: [PATCHv8 18/26] v4l: add buffer exporting via dmabuf
Hi Hans,
On Wednesday 22 August 2012 13:41:05 Hans Verkuil wrote:
> On Tue August 14 2012 17:34:48 Tomasz Stanislawski wrote:
> > This patch adds extension to V4L2 api. It allow to export a mmap buffer as
> > file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer
> > offset used by mmap and return a file descriptor on success.
> >
> > Signed-off-by: Tomasz Stanislawski
> > Signed-off-by: Kyungmin Park
[snip]
> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > index 7f918dc..b5d058b 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -688,6 +688,31 @@ struct v4l2_buffer {
> >
> > #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800
> > #define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000
> >
> > +/**
> > + * struct v4l2_exportbuffer - export of video buffer as DMABUF file
> > descriptor + *
> > + * @fd:file descriptor associated with DMABUF (set by driver)
> > + * @mem_offset:buffer memory offset as returned by VIDIOC_QUERYBUF in
> > struct + * v4l2_buffer::m.offset (for single-plane formats) or
> > + * v4l2_plane::m.offset (for multi-planar formats)
> > + * @flags: flags for newly created file, currently only O_CLOEXEC is
> > + * supported, refer to manual of open syscall for more details
> > + *
> > + * Contains data used for exporting a video buffer as DMABUF file
> > descriptor. + * The buffer is identified by a 'cookie' returned by
> > VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to
> > userspace). All + * reserved fields must be set to zero. The field
> > reserved0 is expected to + * become a structure 'type' allowing an
> > alternative layout of the structure + * content. Therefore this field
> > should not be used for any other extensions. + */
> > +struct v4l2_exportbuffer {
> > + __u32 fd;
> > + __u32 reserved0;
> > + __u32 mem_offset;
> > + __u32 flags;
> > + __u32 reserved[12];
> > +};
>
> OK, I realized that we also need a type field here: you need the type field
> (same as in v4l2_buffer) to know which queue the mem_offset refers to. For
> M2M devices you have two queues, so you need this information.
>
> Is there any reason not to use __u32 memory instead of __u32 reserved0?
> I really dislike 'reserved0'. It's also very inconsistent with the other
> buffer ioctls which all have type+memory fields.
I'm concerned that we might need to export buffers in the future based on
something else than the memory type. That's probably an irrational fear
though.
> Regarding mem_offset: I would prefer a union (possibly anonymous):
>
> union {
> __u32 mem_offset;
> unsigned long reserved;
> } m;
>
> Again, it's more consistent with the existing buffer ioctls, and it prepares
> the API for future pointer values. 'reserved' in the union above could even
> safely be renamed to userptr, even though userptr isn't supported at the
> moment.
Once again I would really want to see compeling use cases before we export a
userptr buffer as a dmabuf object. As Mauro previously stated, userptr for
buffer sharing was a hack in the first place (although a pretty successful one
so far).
--
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] au6610: define reset_resume
After qt1010 change that device seems to survive from reset resume.
Signed-off-by: Antti Palosaari
---
drivers/media/usb/dvb-usb-v2/au6610.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/usb/dvb-usb-v2/au6610.c
b/drivers/media/usb/dvb-usb-v2/au6610.c
index 05f2a86..c126b70 100644
--- a/drivers/media/usb/dvb-usb-v2/au6610.c
+++ b/drivers/media/usb/dvb-usb-v2/au6610.c
@@ -194,6 +194,7 @@ static struct usb_driver au6610_driver = {
.disconnect = dvb_usbv2_disconnect,
.suspend = dvb_usbv2_suspend,
.resume = dvb_usbv2_resume,
+ .reset_resume = dvb_usbv2_reset_resume,
.no_dynamic_id = 1,
.soft_unbind = 1,
};
--
1.7.11.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: [Workshop-2011] Media summit at the Kernel Summit - was: Fwd: Re: [Ksummit-2012-discuss] Organising Mini Summits within the Kernel Summit
Hi Mauro, Can you please provide the schedule for the linuxTV discussions this time at San Diego? Which day will it be on? If you have a detailed schedule, please pass on. I am planning to be in only on that day as I cannot stay back more than that this time around. Regards Naveen -Original Message- From: workshop-2011-boun...@linuxtv.org [mailto:workshop-2011-boun...@linuxtv.org] On Behalf Of Mauro Carvalho Chehab Sent: Tuesday, July 31, 2012 10:23 AM To: Guennadi Liakhovetski Cc: workshop-2...@linuxtv.org; Linux Media Mailing List Subject: Re: [Workshop-2011] Media summit at the Kernel Summit - was: Fwd: Re: [Ksummit-2012-discuss] Organising Mini Summits within the Kernel Summit Em 21-07-2012 02:06, Guennadi Liakhovetski escreveu: > Hi Mauro > > On Tue, 17 Jul 2012, Guennadi Liakhovetski wrote: > >> Hi Mauro >> >> On Tue, 17 Jul 2012, Mauro Carvalho Chehab wrote: >> >>> As we did in 2012, we're planning to do a media summit again at KS/2012. >>> >>> The KS/2012 will happen in San Diego, CA, US, between Aug 26-28, just >>> before the LinuxCon North America. >>> >>> In order to do it, I'd like to know who is interested on participate, >>> and to get proposals about what subjects will be discussed there, >>> in order to start planning the agenda. >> >> I'd love to attend, especially since, as you have seen, I've started doing >> some work on V4L DT bindings, but so far it very much looks like I won't >> be able to do so unfortunately. > > Things change and sometimes also to the better:-) Looks like I'll be able > to attend actually. So, please add me to the list. Great to know! Regards, Mauro ___ Workshop-2011 mailing list workshop-2...@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/workshop-2011 -- 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] soc-camera: Use new selection target definitions
Replace the deprecated V4L2_SEL_TGT_*_ACTIVE selection target
names with their new unified counterparts.
Compatibility definitions are already in linux/v4l2-common.h.
Signed-off-by: Sylwester Nawrocki
---
drivers/media/platform/soc_camera/soc_camera.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index 10b57f8..ba62960 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -950,11 +950,11 @@ static int soc_camera_s_selection(struct file *file, void
*fh,
/* In all these cases cropping emulation will not help */
if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE ||
- (s->target != V4L2_SEL_TGT_COMPOSE_ACTIVE &&
-s->target != V4L2_SEL_TGT_CROP_ACTIVE))
+ (s->target != V4L2_SEL_TGT_COMPOSE &&
+s->target != V4L2_SEL_TGT_CROP))
return -EINVAL;
- if (s->target == V4L2_SEL_TGT_COMPOSE_ACTIVE) {
+ if (s->target == V4L2_SEL_TGT_COMPOSE) {
/* No output size change during a running capture! */
if (is_streaming(ici, icd) &&
(icd->user_width != s->r.width ||
@@ -974,7 +974,7 @@ static int soc_camera_s_selection(struct file *file, void
*fh,
ret = ici->ops->set_selection(icd, s);
if (!ret &&
- s->target == V4L2_SEL_TGT_COMPOSE_ACTIVE) {
+ s->target == V4L2_SEL_TGT_COMPOSE) {
icd->user_width = s->r.width;
icd->user_height = s->r.height;
if (!icd->streamer)
--
1.7.4.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
[PATCH] m2m-deinterlace: Add V4L2_CAP_VIDEO_M2M capability flag
New mem-to-mem video drivers should use V4L2_CAP_VIDEO_M2M capability, rather than ORed V4L2_CAP_VIDEO_CAPTURE and V4L2_CAP_VIDEO_OUTPUT flags, as outlined in commit a1367f1b260d29e9b9fb20d8e2f39f1e74fa6c3b. Cc: Javier Martin Signed-off-by: Sylwester Nawrocki --- drivers/media/platform/m2m-deinterlace.c |9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/m2m-deinterlace.c b/drivers/media/platform/m2m-deinterlace.c index a38c152..5c7df67 100644 --- a/drivers/media/platform/m2m-deinterlace.c +++ b/drivers/media/platform/m2m-deinterlace.c @@ -456,8 +456,13 @@ static int vidioc_querycap(struct file *file, void *priv, strlcpy(cap->driver, MEM2MEM_NAME, sizeof(cap->driver)); strlcpy(cap->card, MEM2MEM_NAME, sizeof(cap->card)); strlcpy(cap->bus_info, MEM2MEM_NAME, sizeof(cap->card)); - cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_OUTPUT - | V4L2_CAP_STREAMING; + /* +* This is only a mem-to-mem video device. The capture and output +* device capability flags are left only for backward compatibility +* and are scheduled for removal. +*/ + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_OUTPUT | + V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING; cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; return 0; -- 1.7.4.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
[PATCH] coda: Add V4L2_CAP_VIDEO_M2M capability flag
New mem-to-mem video drivers should use V4L2_CAP_VIDEO_M2M capability, rather than ORed V4L2_CAP_VIDEO_CAPTURE and V4L2_CAP_VIDEO_OUTPUT flags, as outlined in commit a1367f1b260d29e9b9fb20d8e2f39f1e74fa6c3b. Cc: Javier Martin Signed-off-by: Sylwester Nawrocki --- drivers/media/platform/coda.c |9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 0d6e0a0..e74705c 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -287,8 +287,13 @@ static int vidioc_querycap(struct file *file, void *priv, strlcpy(cap->driver, CODA_NAME, sizeof(cap->driver)); strlcpy(cap->card, CODA_NAME, sizeof(cap->card)); strlcpy(cap->bus_info, CODA_NAME, sizeof(cap->bus_info)); - cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_OUTPUT - | V4L2_CAP_STREAMING; + /* +* This is only a mem-to-mem video device. The capture and output +* device capability flags are left only for backward compatibility +* and are scheduled for removal. +*/ + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_OUTPUT | + V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING; cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; return 0; -- 1.7.4.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
cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date:Wed Aug 22 19:00:25 CEST 2012 git hash:a47b6118e134b51562de520d644d3979b3d99e44 gcc version: i686-linux-gcc (GCC) 4.7.1 host hardware:x86_64 host os: 3.4.07-marune linux-git-arm-eabi-davinci: WARNINGS linux-git-arm-eabi-exynos: OK linux-git-arm-eabi-omap: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: WARNINGS linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS linux-2.6.31.12-x86_64: WARNINGS linux-2.6.32.6-x86_64: WARNINGS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: WARNINGS linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-x86_64: WARNINGS linux-3.2.1-x86_64: WARNINGS linux-3.3-x86_64: WARNINGS linux-3.4-x86_64: WARNINGS linux-3.5-x86_64: WARNINGS linux-3.6-rc2-x86_64: WARNINGS linux-2.6.31.12-i686: WARNINGS linux-2.6.32.6-i686: WARNINGS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: WARNINGS linux-2.6.35.3-i686: WARNINGS linux-2.6.36-i686: WARNINGS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-i686: WARNINGS linux-3.2.1-i686: WARNINGS linux-3.3-i686: WARNINGS linux-3.4-i686: WARNINGS linux-3.5-i686: WARNINGS linux-3.6-rc2-i686: WARNINGS apps: WARNINGS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The V4L-DVB specification 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
[PATCH 4/8] ir-rx51: Clean up timer initialization code
Remove a redundant macro definition. This is unneeded and becomes more
readable once the actual timer code is refactored a little.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 468c8a1..3d2911b 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -105,11 +105,9 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
return 0;
}
-#define tics_after(a, b) ((long)(b) - (long)(a) < 0)
-
static int pulse_timer_set_timeout(struct lirc_rx51 *lirc_rx51, int usec)
{
- int counter;
+ int counter, counter_now;
BUG_ON(usec < 0);
@@ -122,11 +120,8 @@ static int pulse_timer_set_timeout(struct lirc_rx51
*lirc_rx51, int usec)
omap_dm_timer_set_match(lirc_rx51->pulse_timer, 1, counter);
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer,
OMAP_TIMER_INT_MATCH);
- if (tics_after(omap_dm_timer_read_counter(lirc_rx51->pulse_timer),
- counter)) {
- return 1;
- }
- return 0;
+ counter_now = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
+ return (counter - counter_now) < 0;
}
static irqreturn_t lirc_rx51_interrupt_handler(int irq, void *ptr)
--
1.7.12
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/8] ir-rx51: Trivial fixes
-Fix typo
-Change pwm_timer_num type to match type in platform data
-Remove extra parenthesis
-Replace magic constant with proper bit defintions
-Remove duplicate exit pointer
Signed-off-by: Timo Kokkonen
trivial
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/Kconfig | 2 +-
drivers/media/rc/ir-rx51.c | 10 ++
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 093982b..4a68014 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -278,7 +278,7 @@ config IR_RX51
Say Y or M here if you want to enable support for the IR
transmitter diode built in the Nokia N900 (RX51) device.
- The driver uses omap DM timers for gereating the carrier
+ The driver uses omap DM timers for generating the carrier
wave and pulses.
config RC_LOOPBACK
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index a7b787a..468c8a1 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -59,7 +59,7 @@ struct lirc_rx51 {
int wbuf[WBUF_LEN];
int wbuf_index;
unsigned long device_is_open;
- unsigned intpwm_timer_num;
+ int pwm_timer_num;
};
static void lirc_rx51_on(struct lirc_rx51 *lirc_rx51)
@@ -138,11 +138,14 @@ static irqreturn_t lirc_rx51_interrupt_handler(int irq,
void *ptr)
if (!retval)
return IRQ_NONE;
- if ((retval & ~OMAP_TIMER_INT_MATCH))
+ if (retval & ~OMAP_TIMER_INT_MATCH)
dev_err_ratelimited(lirc_rx51->dev,
": Unexpected interrupt source: %x\n", retval);
- omap_dm_timer_write_status(lirc_rx51->pulse_timer, 7);
+ omap_dm_timer_write_status(lirc_rx51->pulse_timer,
+ OMAP_TIMER_INT_MATCH|
+ OMAP_TIMER_INT_OVERFLOW |
+ OMAP_TIMER_INT_CAPTURE);
if (lirc_rx51->wbuf_index < 0) {
dev_err_ratelimited(lirc_rx51->dev,
": BUG wbuf_index has value of %i\n",
@@ -489,7 +492,6 @@ struct platform_driver lirc_rx51_platform_driver = {
.remove = __exit_p(lirc_rx51_remove),
.suspend= lirc_rx51_suspend,
.resume = lirc_rx51_resume,
- .remove = __exit_p(lirc_rx51_remove),
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
--
1.7.12
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/8] ir-rx51: Handle signals properly
The lirc-dev expects the ir-code to be transmitted when the write call
returns back to the user space. We should not leave TX ongoing no
matter what is the reason we return to the user space. Easiest
solution for that is to simply remove interruptible sleeps.
The first wait_event_interruptible is thus replaced with return -EBUSY
in case there is still ongoing transfer. This should suffice as the
concept of sending multiple codes in parallel does not make sense.
The second wait_event_interruptible call is replaced with
wait_even_timeout with a fixed and safe timeout that should prevent
the process from getting stuck in kernel for too long.
Also, from now on we will force the TX to stop before we return from
write call. If the TX happened to time out for some reason, we should
not leave the HW transmitting anything.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 39 ---
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 9487dd3..a7b787a 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
OMAP_TIMER_TRIGGER_NONE);
}
+static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
+{
+ if (lirc_rx51->wbuf_index < 0)
+ return;
+
+ lirc_rx51_off(lirc_rx51);
+ lirc_rx51->wbuf_index = -1;
+ omap_dm_timer_stop(lirc_rx51->pwm_timer);
+ omap_dm_timer_stop(lirc_rx51->pulse_timer);
+ omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
+ wake_up_interruptible(&lirc_rx51->wqueue);
+}
+
static int init_timing_params(struct lirc_rx51 *lirc_rx51)
{
u32 load, match;
@@ -160,13 +173,7 @@ static irqreturn_t lirc_rx51_interrupt_handler(int irq,
void *ptr)
return IRQ_HANDLED;
end:
- /* Stop TX here */
- lirc_rx51_off(lirc_rx51);
- lirc_rx51->wbuf_index = -1;
- omap_dm_timer_stop(lirc_rx51->pwm_timer);
- omap_dm_timer_stop(lirc_rx51->pulse_timer);
- omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
- wake_up_interruptible(&lirc_rx51->wqueue);
+ lirc_rx51_stop_tx(lirc_rx51);
return IRQ_HANDLED;
}
@@ -246,8 +253,9 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
if ((count > WBUF_LEN) || (count % 2 == 0))
return -EINVAL;
- /* Wait any pending transfers to finish */
- wait_event_interruptible(lirc_rx51->wqueue, lirc_rx51->wbuf_index < 0);
+ /* We can have only one transmit at a time */
+ if (lirc_rx51->wbuf_index >= 0)
+ return -EBUSY;
if (copy_from_user(lirc_rx51->wbuf, buf, n))
return -EFAULT;
@@ -273,9 +281,18 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
/*
* Don't return back to the userspace until the transfer has
-* finished
+* finished. However, we wish to not spend any more than 500ms
+* in kernel. No IR code TX should ever take that long.
+*/
+ i = wait_event_timeout(lirc_rx51->wqueue, lirc_rx51->wbuf_index < 0,
+ HZ / 2);
+
+ /*
+* Ensure transmitting has really stopped, even if the timers
+* went mad or something else happened that caused it still
+* sending out something.
*/
- wait_event_interruptible(lirc_rx51->wqueue, lirc_rx51->wbuf_index < 0);
+ lirc_rx51_stop_tx(lirc_rx51);
/* We can sleep again */
lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, -1);
--
1.7.12
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] ir-rx51: Remove MPU wakeup latency adjustments
The ir-rx51 driver calls omap_pm_set_max_mpu_wakeup_lat() in order to
avoid problems that occur when MPU goes to sleep in the middle of
sending an IR code. Without such calls it takes ridiculously long for
the MPU to wake up from a sleep, which distorts the IR signal
completely.
However, the actual problem is that probably the GP timers are not
able to wake up the MPU at all. That is, adjusting the latency
requirements is not the correct way to solve the issue either. The
reason why this used to work with the original 2.6.28 based N900
kernel that is shipped with the product is that placing strict latency
requirements prevents the MPU from going to sleep at all. Furthermore,
the only PM layer imlementation available at the moment for OMAP3
doesn't do anything with the latency requirement placed with
omap_pm_set_max_mpu_wakeup_lat() calls.
A more appropriate fix for the problem would be to modify the idle
layer so that it does not allow MPU going to too deep sleep modes when
it is expected that the timers need to wake up MPU.
Therefore, it makes sense to actually remove this call entirely from
the ir-rx51 driver as it is both wrong and does nothing useful at the
moment.
Signed-off-by: Timo Kokkonen
---
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 --
drivers/media/rc/ir-rx51.c | 9 -
include/media/ir-rx51.h | 2 --
3 files changed, 13 deletions(-)
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index ca07264..e0750cb 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -34,7 +34,6 @@
#include
#include
#include
-#include
#include
@@ -1227,7 +1226,6 @@ static void __init rx51_init_tsc2005(void)
#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
static struct lirc_rx51_platform_data rx51_lirc_data = {
- .set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
.pwm_timer = 9, /* Use GPT 9 for CIR */
};
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 7eed541..ac7d3f0 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -267,12 +267,6 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
if (count < WBUF_LEN)
lirc_rx51->wbuf[count] = -1; /* Insert termination mark */
- /*
-* Adjust latency requirements so the device doesn't go in too
-* deep sleep states
-*/
- lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, 50);
-
lirc_rx51_on(lirc_rx51);
lirc_rx51->wbuf_index = 1;
pulse_timer_set_timeout(lirc_rx51, lirc_rx51->wbuf[0]);
@@ -292,9 +286,6 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
*/
lirc_rx51_stop_tx(lirc_rx51);
- /* We can sleep again */
- lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, -1);
-
return n;
}
diff --git a/include/media/ir-rx51.h b/include/media/ir-rx51.h
index 104aa89..57523f2 100644
--- a/include/media/ir-rx51.h
+++ b/include/media/ir-rx51.h
@@ -3,8 +3,6 @@
struct lirc_rx51_platform_data {
int pwm_timer;
-
- int(*set_max_mpu_wakeup_lat)(struct device *dev, long t);
};
#endif
--
1.7.12
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] ir-rx51: Replace module_{init,exit} macros with module_platform_driver
No reason to avoid using the existing helpers.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 46628c0..7eed541 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -495,17 +495,7 @@ struct platform_driver lirc_rx51_platform_driver = {
},
};
-static int __init lirc_rx51_init(void)
-{
- return platform_driver_register(&lirc_rx51_platform_driver);
-}
-module_init(lirc_rx51_init);
-
-static void __exit lirc_rx51_exit(void)
-{
- platform_driver_unregister(&lirc_rx51_platform_driver);
-}
-module_exit(lirc_rx51_exit);
+module_platform_driver(lirc_rx51_platform_driver);
MODULE_DESCRIPTION("LIRC TX driver for Nokia RX51");
MODULE_AUTHOR("Nokia Corporation");
--
1.7.12
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/8] ir-rx51: Move platform data checking into probe function
This driver is useless without proper platform data. If data is not
available, we should not register the driver at all. Once this check
is done, the BUG_ON check during device open is no longer needed.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 3d2911b..46628c0 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -378,7 +378,6 @@ static long lirc_rx51_ioctl(struct file *filep,
static int lirc_rx51_open(struct inode *inode, struct file *file)
{
struct lirc_rx51 *lirc_rx51 = lirc_get_pdata(file);
- BUG_ON(!lirc_rx51);
file->private_data = lirc_rx51;
@@ -458,6 +457,9 @@ static int lirc_rx51_resume(struct platform_device *dev)
static int __devinit lirc_rx51_probe(struct platform_device *dev)
{
+ if (!dev->dev.platform_data)
+ return -ENODEV;
+
lirc_rx51_driver.features = LIRC_RX51_DRIVER_FEATURES;
lirc_rx51.pdata = dev->dev.platform_data;
lirc_rx51.pwm_timer_num = lirc_rx51.pdata->pwm_timer;
--
1.7.12
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/8] ir-rx51: Remove useless variable from struct lirc_rx51
As clearly visible from the patch, this variable has no useful purpose
what so ever. Thus, it can be removed altogether without any side
effects.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index ac7d3f0..23bc8c0 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -55,7 +55,6 @@ struct lirc_rx51 {
unsigned intfreq; /* carrier frequency */
unsigned intduty_cycle; /* carrier duty cycle */
unsigned intirq_num;
- unsigned intmatch;
int wbuf[WBUF_LEN];
int wbuf_index;
unsigned long device_is_open;
@@ -100,8 +99,6 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
omap_dm_timer_start(lirc_rx51->pulse_timer);
- lirc_rx51->match = 0;
-
return 0;
}
@@ -111,11 +108,7 @@ static int pulse_timer_set_timeout(struct lirc_rx51
*lirc_rx51, int usec)
BUG_ON(usec < 0);
- if (lirc_rx51->match == 0)
- counter = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
- else
- counter = lirc_rx51->match;
-
+ counter = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
counter += (u32)(lirc_rx51->fclk_khz * usec / (1000));
omap_dm_timer_set_match(lirc_rx51->pulse_timer, 1, counter);
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer,
--
1.7.12
--
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 0/8] ir-rx51: Fixes in response to review comments
These patches fix most of the issues pointed out in the patch review
by Sean Young and Sakari Ailus. The most noticeable change after these
patch set is that the IR transmission no longer times out even if the
timers are not waking up the MPU as it should be. However, the
transmission itself is still as badly mangled as before, unless there
is some background load preventing the MPU from going into
sleep. Otherwise the patches are mostly clean ups and rather trivial
stuff.
All comments are welcome. Thanks!
Timo Kokkonen (8):
ir-rx51: Adjust dependencies
ir-rx51: Handle signals properly
ir-rx51: Trivial fixes
ir-rx51: Clean up timer initialization code
ir-rx51: Move platform data checking into probe function
ir-rx51: Replace module_{init,exit} macros with
module_platform_driver
ir-rx51: Remove MPU wakeup latency adjustments
ir-rx51: Remove useless variable from struct lirc_rx51
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 -
drivers/media/rc/Kconfig | 4 +-
drivers/media/rc/ir-rx51.c | 92 +---
include/media/ir-rx51.h | 2 -
4 files changed, 43 insertions(+), 57 deletions(-)
--
1.7.12
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/8] ir-rx51: Adjust dependencies
Although this kind of IR diode circuitry is known to exist only in N900 hardware, nothing prevents making similar circuitry on any OMAP based board. The MACH_NOKIA_RX51 dependency is thus not something we want to be there. Also, this should depend on LIRC as it is a LIRC driver. Signed-off-by: Timo Kokkonen --- drivers/media/rc/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig index ffef8b4..093982b 100644 --- a/drivers/media/rc/Kconfig +++ b/drivers/media/rc/Kconfig @@ -273,7 +273,7 @@ config IR_IGUANA config IR_RX51 tristate "Nokia N900 IR transmitter diode - depends on MACH_NOKIA_RX51 && OMAP_DM_TIMER + depends on OMAP_DM_TIMER && LIRC ---help--- Say Y or M here if you want to enable support for the IR transmitter diode built in the Nokia N900 (RX51) device. -- 1.7.12 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media/radio/shark2: Fix build error caused by missing dependencies
On Wednesday 22 August 2012, Guenter Roeck wrote: > On Wed, Aug 22, 2012 at 05:22:26PM +0200, Hans de Goede wrote: > > Hi, > > > > I've a better fix for this here: > > http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/media-for_v3.6 > > > > I already send a pull-req for this to Mauro a while ago, Mauro? > > > Looks like it found its way into mainline in the last couple of days. > Should have updated my tree first. Sorry for the noise. > I found another issue with the shark driver while doing randconfig tests. Here is my semi-automated log file for the problem. Has this also made it in already? Arnd --- Without this patch, building rand-0y2jSKT results in: WARNING: drivers/usb/musb/musb_hdrc.o(.devinit.text+0x9b8): Section mismatch in reference from the function musb_init_controller() to the function .init.text:dma_controller_create() The function __devinit musb_init_controller() references a function __init dma_controller_create(). If dma_controller_create is only used by musb_init_controller then annotate dma_controller_create with a matching annotation. ERROR: "snd_tea575x_init" [drivers/media/radio/radio-shark.ko] undefined! ERROR: "snd_tea575x_exit" [drivers/media/radio/radio-shark.ko] undefined! make[2]: *** [__modpost] Error 1 make[1]: *** [modules] Error 2 make: *** [sub-make] Error 2 --- sound/pci/Kconfig |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig index ff3af6e..f99fa25 100644 --- a/sound/pci/Kconfig +++ b/sound/pci/Kconfig @@ -2,8 +2,8 @@ config SND_TEA575X tristate - depends on SND_FM801_TEA575X_BOOL || SND_ES1968_RADIO || RADIO_SF16FMR2 || RADIO_MAXIRADIO - default SND_FM801 || SND_ES1968 || RADIO_SF16FMR2 || RADIO_MAXIRADIO + depends on SND_FM801_TEA575X_BOOL || SND_ES1968_RADIO || RADIO_SF16FMR2 || RADIO_MAXIRADIO || RADIO_SHARK + default SND_FM801 || SND_ES1968 || RADIO_SF16FMR2 || RADIO_MAXIRADIO || RADIO_SHARK menuconfig SND_PCI bool "PCI sound devices" -- 1.7.10 -- 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 API] Renumber subdev ioctls
Em 22-08-2012 05:52, Hans Verkuil escreveu: > On Tue August 21 2012 12:44:15 Sakari Ailus wrote: >> Hi Hans, >> >> On Tue, Aug 21, 2012 at 08:39:53AM +0200, Hans Verkuil wrote: >>> On Mon August 20 2012 22:46:04 Sakari Ailus wrote: Hi Mauro and Hans, On Mon, Aug 20, 2012 at 04:05:03PM -0300, Mauro Carvalho Chehab wrote: > Em 20-08-2012 05:30, Hans Verkuil escreveu: >> Hi all! >> >> Recently I had to add two new ioctls for the subdev API >> (include/linux/v4l2-subdev.h) >> and I noticed that the numbering of the ioctls was somewhat random. >> >> In most cases the ioctl number was the same as the V4L2 API counterpart, >> but for >> subdev-specific ioctls no rule exists. >> >> There are a few problems with this: because of the lack of rules there >> is a chance >> that in the future a subdev ioctl may end up to be identical to an >> existing V4L2 >> ioctl. Also, because the numbering isn't nicely increasing it makes it >> hard to create >> a lookup table as was done for the V4L2 ioctls. Well, you could do it, >> but it would >> be a very sparse array, wasting a lot of memory. >> >> Lookup tables have proven to be very useful, so we might want to >> introduce them for >> the subdev core code as well in the future. >> >> Since the subdev API is still marked experimental, I propose to renumber >> the ioctls >> and use the letter 'v' instead of 'V'. 'v' was used for V4L1, and so it >> is now >> available for reuse. > > 'v' is already used (mainly by fs): > > 'v' 00-1F linux/ext2_fs.h conflict! > 'v' 00-1F linux/fs.h conflict! > 'v' 00-0F linux/sonypi.h conflict! > 'v' C0-FF linux/meye.hconflict! > > Reusing the ioctl numbering is a bad thing, as tracing code like strace > will likely > say that a different type of ioctl was called. > > (Yeah, unfortunately, this end by merging with duplicated stuff :< ) > > Also, I don't like the idea of deprecating it just because of that: > interfaces are > supposed to be stable. > > It should be noticed that there are very few ioctls there. So, > using a lookup table is overkill. > > IMO, the better is to sort the ioctl's there at the header file, in order > to > avoid ioctl duplicaton. Many of the V4L2 IOCTLs are being used on subdevs, too, to the extent that subdev_do_ioctl() in drivers/media/v4l2-core/v4l2-subdev.c has a switch statement with over 20 cases. We'll get rid of two once the old crop IOCTLs are removed but we've still got over 20, and the number is likely to grow in the future. Still it's just a fraction of what V4L2 has. We decided to use 'V' also for subdev IOCTLs for a reason I no longer remember. It's true there can be clashes with regular V4L2 IOCTLs in terms of IOCTL codes if the size of the argument struct matches. One of the reasons to use 'V' might have been that then some of the IOCTLs on a device would have different type (the letter in question) which wasn't considered pretty. 'V' is for V4L2 after all, and V4L2 subdev interface is part of the V4L2. The numbering is based on using V4L2 IOCTLs as such if they were applicable to subdevs as such (controls) in which case they're defined in videodev2.h, and if there was even a loosely corresponding IOCTL in V4L2 then use the same number (e.g. formats vs. media bus pixel codes) and otherwise something else. The "something else" case hasn't happened yet. It might have made sense to use a different type for the IOCTLs that aren't V4L2 IOCTLs (i.e. are subdev IOCTLs) for clarity but it's quite late for such a change. However if we think we definitely should do it then it should be done now or not at all... If we want to just improve the efficiency of the switch statement in subdev_do_ioctl() we could divide the IOCTLs based on e.g. a few last bits of the IOCTL number into buckets. >>> >>> It's not so much the switch efficiency. In practice there will be no >>> measurable >>> speed difference. But a lookup table allows one to easily look up >>> information >>> about the ioctl. >>> >>> But the main goal would be to guarantee that subdev ioctls and V4L2 ioctls >>> will never clash, since both types of ioctls can be used with a subdev node. >> >> It's indeed possible to have clashes between the IOCTL codes but that does >> not matter so much: all IOCTLs related to buffers belong to V4L2 and >> anything related to pads belongs to subdevs only. >> >> As long as a little care is taken when choosing the IOCTL number we >> shouldn't have issues any more we have now. Well, that said, the IOCTLs >> belonging to the something else category are more
Re: How to add support for the em2765 webcam Speedlink VAD Laplace to the kernel ?
Em 22-08-2012 04:53, Frank Schäfer escreveu:
> Am 21.08.2012 19:29, schrieb Mauro Carvalho Chehab:
>> Hmm... before reading the rest of this email... I found some doc saying that
>> em2765 is UVC compliant. Doesn't the uvcdriver work with this device?
>
> Yeah, I stumbled over that, too. :D
> But this device is definitely not UVC compliant. Take a look at the
> lsusb output.
> Maybe they are using a different firmware or something like that, but I
> have no idea why the hell they should make a UVC compliant device
> non-UVC-compliant...
> Another notable difference to the devices we've seen so far is the
> em25xx-style EEPROM. Maybe there is a connection.
>
> Btw, do we know any em25xx devices ???
No, I never heard about em25xx. It seems that there are some new em275xx
chips, but I don't have any technical data.
> I have to admit that I didn't open my device to check which chip it
> uses. This information comes from the guy who created the inital wiki
> page. But I think we can trust this information, because he provided the
> full chip label content and also photos. And according to the chip id,
> it's none of the devices we already know.
> I wouldn't hesitate to open my device to verify it, if the chance to
> damage the device wouldn't be that high...
>
>
>>
>> Em 21-08-2012 13:04, Frank Schäfer escreveu:
>>> Am 21.08.2012 14:32, schrieb Mauro Carvalho Chehab:
Em 21-08-2012 08:35, Frank Schäfer escreveu:
> Am 20.08.2012 21:21, schrieb Mauro Carvalho Chehab:
>> Em 20-08-2012 10:02, Hans de Goede escreveu:
>>> Hi,
>>>
>>> On 08/20/2012 01:41 PM, Frank Schäfer wrote:
Hi,
after a break of 2 1/2 kernel releases (sorry, I was busy with another
project), I would like to bring up again the question how to add
support
for this device to the kernel.
See
http://www.mail-archive.com/linux-media@vger.kernel.org/msg44417.html
("Move em27xx/em28xx webcams to a gspca subdriver ?") for the previous
discussion.
Current status is, that I've reverse-engineered the Windows driver and
written a new gspca-subdriver for testing, which is feature complete
and
working stable (will send a patch shortly !).
The device uses an em2765-bridge, so my first idea was of course to
modify/extend the em28xx-driver.
But during the reverse-engineering-process, it turned out that writing
a
new gspca-subdriver was much easier than modifying the em28xx-driver.
The device has the following special characteristics:
- supports only bulk transfers (em28xx driver supports ISOC only)
>> Em28xx driver supports both isoc and bulk transfers, as bulk is
>> required by DVB.
> Hmm... are you 100% sure ? Must have been added recently then...
>
> I did a quick check of the current code, but can't find anything. Could
> you please give me a pointer to the code parts ?
> Btw, if I'm not understanding the code wrong, em28xx_usb_probe() still
> seems to return -ENODEV if no isoc-in endpoint is found, so bulk-ep-only
> devices should not work...
Perhaps I'm tricked by tm6000 code... both codes are similar.
There are a few differences with regards to isoc/bulk hanlding
there.
- uses "proprietary" read/write procedures for the sensor
>> Sure about that? It doesn't use I2C?
> According to the datasheet of the OV2640 it should be SCCB.
SCCB is a variant of I2C.
>>> I'm not sure if Omnivison would admit that. :D
>> Maybe there are trademarks envolved ;)
>
> Yes, it's a funny game. ;)
>
> Anyway, I'm referring to how communication works on the USB level.
> Take a look at http://linuxtv.org/wiki/index.php/VAD_Laplace section
> "Reverse Engineering (evaluation of USB-logs)" to see how it is working.
Ok. From your logs, it seems that em2765 uses a different bus for
sensor communication. It is not unusual to have more than one bus on
some modern devices (cx231xx has 3 I2C buses, plus one extra bus that
can be switched).
>>> Yes, but I'm wondering why.
>>> Shouldn't it be possible to connect both (sensor and eeprom) to the same
>>> bus ? Or are i2c and sccb devices incompatible ?
>>> We've seen so many em28xx devices (some of them beeing much more
>>> complex) and none of them has used two busses so far.
>>> Strange...
>> Most devices nowadays have 2 i2c buses. TV boards typically uses an
>> I2C switch on them. The rationale is to avoid receiving signal
>> interference. Some newer em28xx devices have more than one bus, although,
>> on TV chipsets, this is controlled via one register, that commands
>> bus switch:
>>
>> /* em28xx I2C Clock Register (0x06) */
>> #define EM2874_I2C_SECONDARY_BUS_SELECT 0x04 /* em2874 has two i2c
>> busses */
>>
>> On those devices (em2874/em2875, afaikt), hardware manufacturers p
Re: RFC: Core + Radio profile
Em 22-08-2012 12:19, Mike Isely escreveu:
> On Wed, 22 Aug 2012, Mauro Carvalho Chehab wrote:
>
>> Em 22-08-2012 07:11, Hans Verkuil escreveu:
>>> I've added some more core profile requirements.
>>
Streaming I/O is not supported by radio nodes.
>>
>> Hmm... pvrusb2/ivtv? Ok, it makes sense to move it to use the alsa
>> mpeg API there. If we're enforcing it, we should deprecate the current way
>> there, and make it use ALSA.
>
> I am unaware of any ALSA MPEG API. It's entirely likely that this is
> because I haven't been paying attention. Nevertheless, can you please
> point me at any documentation on this so I can get up to speed?
I don't know much about that. A grep at sound might help:
$ git grep -i mpeg sound/
sound/core/oss/pcm_oss.c: case AFMT_MPEG: return SNDRV_PCM_FORMAT_MPEG;
sound/core/oss/pcm_oss.c: case SNDRV_PCM_FORMAT_MPEG: return
AFMT_MPEG;
sound/core/pcm.c: FORMAT(MPEG),
sound/core/pcm.c: case AFMT_MPEG:
sound/core/pcm.c: return "MPEG";
sound/core/pcm_misc.c:[SNDRV_PCM_FORMAT_MPEG] = {
sound/drivers/vx/vx_cmd.h:#define MASK_VALID_PIPE_MPEG_PARAM 0x40
sound/drivers/vx/vx_cmd.h:#define MASK_SET_PIPE_MPEG_PARAM0x02
sound/drivers/vx/vx_cmd.h:#define P_PREPARE_FOR_MPEG3_MASK
sound/drivers/vx/vx_core.c: if (chip->audio_info & VX_AUDIO_INFO_MPEG1)
sound/drivers/vx/vx_core.c: snd_iprintf(buffer, " mpeg1");
sound/drivers/vx/vx_core.c: if (chip->audio_info & VX_AUDIO_INFO_MPEG2)
sound/drivers/vx/vx_core.c: snd_iprintf(buffer, " mpeg2");
sound/pci/asihpi/asihpi.c:-1, /* HPI_FORMAT_MPEG_L1
3 */
sound/pci/asihpi/asihpi.c:SNDRV_PCM_FORMAT_MPEG, /* HPI_FORMAT_MPEG_L2
4 */
sound/pci/asihpi/asihpi.c:SNDRV_PCM_FORMAT_MPEG, /* HPI_FORMAT_MPEG_L3
5 */
sound/pci/asihpi/hpi.h:/** MPEG-1 Layer-1. */
sound/pci/asihpi/hpi.h: HPI_FORMAT_MPEG_L1 = 3,
sound/pci/asihpi/hpi.h:/** MPEG-1 Layer-2.
sound/pci/asihpi/hpi.h:Windows equivalent is WAVE_FORMAT_MPEG.
sound/pci/asihpi/hpi.h: HPI_FORMAT_MPEG_L2 = 4,
sound/pci/asihpi/hpi.h:/** MPEG-1 Layer-3.
sound/pci/asihpi/hpi.h:Windows equivalent is WAVE_FORMAT_MPEG.
sound/pci/asihpi/hpi.h: HPI_FORMAT_MPEG_L3 = 5,
sound/pci/asihpi/hpi.h:#define HPI_CAPABILITY_MPEG_LAYER3 (1)
sound/pci/asihpi/hpi.h:/** MPEG Ancillary Data modes
sound/pci/asihpi/hpi.h:enum HPI_MPEG_ANC_MODES {
sound/pci/asihpi/hpi.h: /** the MPEG frames have energy information stored in
them (5 bytes per stereo frame, 3 per mono) */
sound/pci/asihpi/hpi.h: HPI_MPEG_ANC_HASENERGY = 0,
sound/pci/asihpi/hpi.h: HPI_MPEG_ANC_RAW = 1
sound/pci/asihpi/hpi.h:enum HPI_ISTREAM_MPEG_ANC_ALIGNS {
sound/pci/asihpi/hpi.h: HPI_MPEG_ANC_ALIGN_LEFT = 0,
sound/pci/asihpi/hpi.h: HPI_MPEG_ANC_ALIGN_RIGHT = 1
sound/pci/asihpi/hpi.h:/** MPEG modes
sound/pci/asihpi/hpi.h:MPEG modes - can be used optionally for
HPI_FormatCreate()
sound/pci/asihpi/hpi.h:Using any mode setting other than HPI_MPEG_MODE_DEFAULT
sound/pci/asihpi/hpi.h:enum HPI_MPEG_MODES {
sound/pci/asihpi/hpi.h:/** Causes the MPEG-1 Layer II bitstream to be recorded
sound/pci/asihpi/hpi.h: HPI_MPEG_MODE_DEFAULT = 0,
sound/pci/asihpi/hpi.h: HPI_MPEG_MODE_STEREO = 1,
sound/pci/asihpi/hpi.h: HPI_MPEG_MODE_JOINTSTEREO = 2,
sound/pci/asihpi/hpi.h: HPI_MPEG_MODE_DUALCHANNEL = 3
sound/pci/asihpi/hpi.h:/** maximum number of ancillary bytes per MPEG frame */
sound/pci/asihpi/hpi.h: u32 bit_rate; /**< for MPEG */
sound/pci/asihpi/hpi.h: u16 format; /**< HPI_FORMAT_PCM16, _MPEG etc.
see #HPI_FORMATS. */
sound/pci/asihpi/hpi_internal.h: u32 bit_rate; /**< for MPEG */
sound/pci/asihpi/hpi_internal.h: u16 format; /**< HPI_FORMAT_PCM16, _MPEG etc.
see \ref HPI_FORMATS. */
sound/pci/asihpi/hpifunc.c: case HPI_FORMAT_MPEG_L1:
sound/pci/asihpi/hpifunc.c: case HPI_FORMAT_MPEG_L2:
sound/pci/asihpi/hpifunc.c: case HPI_FORMAT_MPEG_L3:
sound/pci/asihpi/hpifunc.c: case HPI_FORMAT_MPEG_L1:
sound/pci/asihpi/hpifunc.c: case HPI_FORMAT_MPEG_L2:
sound/pci/asihpi/hpifunc.c: case HPI_FORMAT_MPEG_L3:
sound/pci/asihpi/hpifunc.c: case HPI_FORMAT_MPEG_L2:
sound/pci/asihpi/hpifunc.c: && (attributes !=
HPI_MPEG_MODE_DEFAULT)) {
sound/pci/asihpi/hpifunc.c: attributes =
HPI_MPEG_MODE_DEFAULT;
sound/pci/asihpi/hpifunc.c: } else if (attributes >
HPI_MPEG_MODE_DUALCHANNEL) {
sound/pci/asihpi/hpifunc.c: attributes =
HPI_MPEG_MODE_DEFAULT;
sound/pci/asihpi/hpifunc.c: case HPI_FORMAT_MPEG_L1:
sound/pci/asihpi/hpifunc.c: case HPI_FORMAT_MPEG_L2:
sound/pci/asihpi/hpifunc.c: case HPI_FORMAT_MPEG_L3:
sound/pci/bt87x.c: * (DVB cards use the audio function to transfer MPEG data) */
sound/pci/ens1370.c:#define ES_MSFMTSEL (1<<15) /* MPEG
serial data format; 0 = SONY, 1 = I2S */
sound/pci/ens1370.c:#de
RE: [PATCH v4 4/4] [media] s5p-mfc: Update MFC v4l2 driver to support MFC6.x
Hi Arun,
Please find the comments in the patch contents.
Best wishes,
--
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center
> From: Arun Kumar K [mailto:arun...@samsung.com]
> Sent: 09 August 2012 12:29
>
> From: Jeongtae Park
>
[snip]
> diff --git a/drivers/media/video/s5p-mfc/s5p_mfc.c
b/drivers/media/video/s5p-
> mfc/s5p_mfc.c
> index be8d689..75b9026 100644
> --- a/drivers/media/video/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/video/s5p-mfc/s5p_mfc.c
> @@ -278,12 +278,13 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx
*ctx,
>
> dst_frame_status = s5p_mfc_get_dspl_status(dev)
> & S5P_FIMV_DEC_STATUS_DECODING_STATUS_MASK;
> - res_change = s5p_mfc_get_dspl_status(dev)
> - & S5P_FIMV_DEC_STATUS_RESOLUTION_MASK;
> + res_change = (s5p_mfc_get_dspl_status(dev)
> + & S5P_FIMV_DEC_STATUS_RESOLUTION_MASK)
> + >> S5P_FIMV_DEC_STATUS_RESOLUTION_SHIFT;
> mfc_debug(2, "Frame Status: %x\n", dst_frame_status);
> if (ctx->state == MFCINST_RES_CHANGE_INIT)
> ctx->state = MFCINST_RES_CHANGE_FLUSH;
> - if (res_change) {
> + if (res_change == 1 || res_change == 2) {
Here there should be a define instead of these magic numbers.
(something like S5P_FIMV_RES_INCREASE/DECREASE)
> ctx->state = MFCINST_RES_CHANGE_INIT;
> s5p_mfc_clear_int_flags(dev);
> wake_up_ctx(ctx, reason, err);
> @@ -435,10 +436,26 @@ static void s5p_mfc_handle_seq_done(struct s5p_mfc_ctx
> *ctx,
> s5p_mfc_dec_calc_dpb_size(ctx);
>
> ctx->dpb_count = s5p_mfc_get_dpb_count(dev);
> + ctx->mv_count = s5p_mfc_get_mv_count(dev);
> if (ctx->img_width == 0 || ctx->img_height == 0)
> ctx->state = MFCINST_ERROR;
> else
> ctx->state = MFCINST_HEAD_PARSED;
> +
> + if ((ctx->codec_mode == S5P_MFC_CODEC_H264_DEC ||
> + ctx->codec_mode == S5P_MFC_CODEC_H264_MVC_DEC) &&
> + !list_empty(&ctx->src_queue)) {
> + struct s5p_mfc_buf *src_buf;
> + src_buf = list_entry(ctx->src_queue.next,
> + struct s5p_mfc_buf, list);
> + mfc_debug(2, "Check consumed size of header. ");
> + mfc_debug(2, "source : %d, consumed : %d\n",
> + s5p_mfc_get_consumed_stream(dev),
> + src_buf->b->v4l2_planes[0].bytesused);
> + if (s5p_mfc_get_consumed_stream(dev) <
> + src_buf->b->v4l2_planes[0].bytesused)
> + ctx->remained = 1;
> + }
> }
I have already commented about the name remained. In addition the flag should
be
set here to both 0 an 1, as we cannot take previous value for granted.
> s5p_mfc_clear_int_flags(dev);
> clear_work_bit(ctx);
> @@ -469,7 +486,7 @@ static void s5p_mfc_handle_init_buffers(struct
s5p_mfc_ctx
> *ctx,
> spin_unlock(&dev->condlock);
> if (err == 0) {
> ctx->state = MFCINST_RUNNING;
> - if (!ctx->dpb_flush_flag) {
> + if (!ctx->dpb_flush_flag && !ctx->remained) {
> spin_lock_irqsave(&dev->irqlock, flags);
> if (!list_empty(&ctx->src_queue)) {
> src_buf = list_entry(ctx->src_queue.next,
> @@ -1199,12 +1216,47 @@ static struct s5p_mfc_variant mfc_drvdata_v5 = {
> .port_num = 2,
> .buf_size = &buf_size_v5,
> .buf_align = &mfc_buf_align_v5,
> + .mclk_name = "sclk_mfc",
> + .fw_name= "s5p-mfc.fw",
> +};
> +
> +struct s5p_mfc_buf_size_v6 mfc_buf_size_v6 = {
> + .dev_ctx= 0x7000, /* 28KB */
> + .h264_dec_ctx = 0x20, /* 1.6MB */
> + .other_dec_ctx = 0x5000, /* 20KB */
> + .h264_enc_ctx = 0x19000, /* 100KB */
> + .other_enc_ctx = 0x3000, /* 12KB */
> +};
> +
> +struct s5p_mfc_buf_size buf_size_v6 = {
> + .fw = 0x10, /* 1MB */
> + .cpb= 0x30, /* 3MB */
> + .priv = &mfc_buf_size_v6,
> +};
I have already commented about using defines here in the previous patch.
I think it would look much better than numbers.
[snip]
> --- a/drivers/media/video/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/video/s5p-mfc/s5p_mfc_common.h
> @@ -16,13 +16,14 @@
> #ifndef S5P_MFC_COMMON_H_
> #define S5P_MFC_COMMON_H_
>
> -#include "regs-mfc.h"
> #include
> #include
> #include
> #include
> #include
> #include
> +#include "regs-mfc.h"
> +#include "regs-mfc-v6.h"
>
> /* Definitions related to MFC memory */
>
> @@ -210,6 +211,14 @@ struct s5p_mfc_buf_size_v5 {
> unsigned int shm;
> };
>
> +struct s5p_mfc_bu
RE: [PATCH v4 3/4] [media] s5p-mfc: MFCv6 register definitions
Hi Arun, Please find the comments in the patch contents. Best wishes, -- Kamil Debski Linux Platform Group Samsung Poland R&D Center > From: Arun Kumar K [mailto:arun...@samsung.com] > Sent: 09 August 2012 12:28 > > From: Jeongtae Park > > Adds register definitions for MFC v6.x firmware > > Signed-off-by: Jeongtae Park > Signed-off-by: Janghyuck Kim > Signed-off-by: Jaeryul Oh > Signed-off-by: Naveen Krishna Chatradhi > Signed-off-by: Arun Kumar K > --- > drivers/media/video/s5p-mfc/regs-mfc-v6.h | 429 + > 1 files changed, 429 insertions(+), 0 deletions(-) > create mode 100644 drivers/media/video/s5p-mfc/regs-mfc-v6.h > > diff --git a/drivers/media/video/s5p-mfc/regs-mfc-v6.h > b/drivers/media/video/s5p-mfc/regs-mfc-v6.h > new file mode 100644 > index 000..2f25c5d > --- /dev/null > +++ b/drivers/media/video/s5p-mfc/regs-mfc-v6.h > @@ -0,0 +1,429 @@ > +/* > + * Register definition file for Samsung MFC V6.x Interface (FIMV) driver > + * > + * Copyright (c) 2012 Samsung Electronics > + * http://www.samsung.com/ > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#ifndef _REGS_FIMV_V6_H > +#define _REGS_FIMV_V6_H > + > +#define S5P_FIMV_REG_SIZE_V6 (S5P_FIMV_END_ADDR - S5P_FIMV_START_ADDR) > +#define S5P_FIMV_REG_COUNT_V6((S5P_FIMV_END_ADDR - S5P_FIMV_START_ADDR) / 4) > + > +/* Number of bits that the buffer address should be shifted for particular > + * MFC buffers. */ > +#define S5P_FIMV_MEM_OFFSET_V6 0 > + > +#define S5P_FIMV_START_ADDR_V6 0x > +#define S5P_FIMV_END_ADDR_V6 0xfd80 > + > +#define S5P_FIMV_REG_CLEAR_BEGIN_V6 0xf000 > +#define S5P_FIMV_REG_CLEAR_COUNT_V6 1024 > + > +/* Codec Common Registers */ > +#define S5P_FIMV_RISC_ON_V6 0x > +#define S5P_FIMV_RISC2HOST_INT_V60x003C > +#define S5P_FIMV_HOST2RISC_INT_V60x0044 > +#define S5P_FIMV_RISC_BASE_ADDRESS_V60x0054 > + > +#define S5P_FIMV_MFC_RESET_V60x1070 > + > +#define S5P_FIMV_HOST2RISC_CMD_V60x1100 > +#define S5P_FIMV_H2R_CMD_EMPTY_V60 > +#define S5P_FIMV_H2R_CMD_SYS_INIT_V6 1 > +#define S5P_FIMV_H2R_CMD_OPEN_INSTANCE_V62 > +#define S5P_FIMV_CH_SEQ_HEADER_V63 > +#define S5P_FIMV_CH_INIT_BUFS_V6 4 > +#define S5P_FIMV_CH_FRAME_START_V6 5 > +#define S5P_FIMV_H2R_CMD_CLOSE_INSTANCE_V6 6 > +#define S5P_FIMV_H2R_CMD_SLEEP_V67 > +#define S5P_FIMV_H2R_CMD_WAKEUP_V6 8 > +#define S5P_FIMV_CH_LAST_FRAME_V69 > +#define S5P_FIMV_H2R_CMD_FLUSH_V610 > +/* RMVME: REALLOC used? */ > +#define S5P_FIMV_CH_FRAME_START_REALLOC_V6 5 > + > +#define S5P_FIMV_RISC2HOST_CMD_V60x1104 > +#define S5P_FIMV_R2H_CMD_EMPTY_V60 > +#define S5P_FIMV_R2H_CMD_SYS_INIT_RET_V6 1 > +#define S5P_FIMV_R2H_CMD_OPEN_INSTANCE_RET_V62 > +#define S5P_FIMV_R2H_CMD_SEQ_DONE_RET_V6 3 > +#define S5P_FIMV_R2H_CMD_INIT_BUFFERS_RET_V6 4 > + > +#define S5P_FIMV_R2H_CMD_CLOSE_INSTANCE_RET_V6 6 > +#define S5P_FIMV_R2H_CMD_SLEEP_RET_V67 > +#define S5P_FIMV_R2H_CMD_WAKEUP_RET_V6 8 > +#define S5P_FIMV_R2H_CMD_COMPLETE_SEQ_RET_V6 9 > +#define S5P_FIMV_R2H_CMD_DPB_FLUSH_RET_V610 > +#define S5P_FIMV_R2H_CMD_NAL_ABORT_RET_V611 > +#define S5P_FIMV_R2H_CMD_FW_STATUS_RET_V612 > +#define S5P_FIMV_R2H_CMD_FRAME_DONE_RET_V6 13 > +#define S5P_FIMV_R2H_CMD_FIELD_DONE_RET_V6 14 > +#define S5P_FIMV_R2H_CMD_SLICE_DONE_RET_V6 15 > +#define S5P_FIMV_R2H_CMD_ENC_BUFFER_FUL_RET_V6 16 > +#define S5P_FIMV_R2H_CMD_ERR_RET_V6 32 > + > +#define S5P_FIMV_FW_VERSION_V6 0xF000 > + > +#define S5P_FIMV_INSTANCE_ID_V6 0xF008 > +#define S5P_FIMV_CODEC_TYPE_V6 0xF00C > +#define S5P_FIMV_CONTEXT_MEM_ADDR_V6 0xF014 > +#define S5P_FIMV_CONTEXT_MEM_SIZE_V6 0xF018 > +#define S5P_FIMV_PIXEL_FORMAT_V6 0xF020 > + > +#define S5P_FIMV_METADATA_ENABLE_V6 0xF024 > +#define S5P_FIMV_DBG_BUFFER_ADDR_V6 0xF030 > +#define S5P_FIMV_DBG_BUFFER_SIZE_V6 0xF034 > +#define S5P_FIMV_RET_INSTANCE_ID_V6 0xF070 > + > +#define S5P_FIMV_ERROR_CODE_V6 0xF074 > +#define S5P_FIMV_ERR_WARNINGS_START_V6 160 > +#define S5P_FIMV_ERR_DEC_MASK_V6 0x > +#define S5P_FIMV_ERR_DEC_SHIFT_V60 > +#define S5P_FIMV_ERR_DSPL_MASK_V60x > +#define S5P_FIMV_ERR_DSPL_SHIFT_V6 16 > + > +#define S5P_FIMV_DBG_BUFFER_OUTPUT_SIZE_V6 0xF078 > +#define S5P_FIMV_METADATA_STATUS_V6 0xF07C > +#define S5P_FIMV_METADATA_ADDR_MB_INFO_V60xF080 > +#define S5P_FIMV_METADATA_SIZE_MB_INFO_V6
RE: [PATCH v4 2/4] [media] s5p-mfc: Add MFC variant data to device context
Hi Arun,
Please find the comments in the patch contents.
Best wishes,
--
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center
> From: Arun Kumar K [mailto:arun...@samsung.com]
> Sent: 09 August 2012 12:28
>
> From: Jeongtae Park
>
> MFC variant data replaces various macros used in the driver
> which will change in a different version of MFC hardware.
> Also does a cleanup of MFC context structure and common files.
>
> Signed-off-by: Jeongtae Park
> Signed-off-by: Janghyuck Kim
> Signed-off-by: Jaeryul Oh
> Signed-off-by: Naveen Krishna Chatradhi
> Signed-off-by: Arun Kumar K
> ---
> drivers/media/video/s5p-mfc/regs-mfc.h |2 +-
> drivers/media/video/s5p-mfc/s5p_mfc.c| 78 +-
> drivers/media/video/s5p-mfc/s5p_mfc_cmd_v5.c |4 +-
> drivers/media/video/s5p-mfc/s5p_mfc_common.h | 66 ++---
> drivers/media/video/s5p-mfc/s5p_mfc_ctrl.c |7 +-
> drivers/media/video/s5p-mfc/s5p_mfc_enc.c| 44 +-
> drivers/media/video/s5p-mfc/s5p_mfc_opr_v5.c | 213
+-
> 7 files changed, 243 insertions(+), 171 deletions(-)
>
[snip]
> diff --git a/drivers/media/video/s5p-mfc/s5p_mfc.c
b/drivers/media/video/s5p-
> mfc/s5p_mfc.c
> index ab66680..be8d689 100644
> --- a/drivers/media/video/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/video/s5p-mfc/s5p_mfc.c
[snip]
> @@ -1207,9 +1177,43 @@ static const struct dev_pm_ops s5p_mfc_pm_ops = {
> NULL)
> };
>
> +struct s5p_mfc_buf_size_v5 mfc_buf_size_v5 = {
> + .h264_ctx = 0x96000,
> + .non_h264_ctx = 0x2800,
> + .dsc= 0x2,
> + .shm= 0x1000,
> +};
> +
> +struct s5p_mfc_buf_size buf_size_v5 = {
> + .fw = 0x6,
> + .cpb= 0x40, /* 4MB */
> + .priv = &mfc_buf_size_v5,
> +};
In s5p_mfc_common.h you have the macros that define (most of) the used above
values. Please use the defines.
> +
> +struct s5p_mfc_buf_align mfc_buf_align_v5 = {
> + .base = 17,
> +};
> +
MFC_BASE_ALIGN_ORDER?
> +static struct s5p_mfc_variant mfc_drvdata_v5 = {
> + .version= 0x51,
> + .port_num = 2,
> + .buf_size = &buf_size_v5,
> + .buf_align = &mfc_buf_align_v5,
> +};
> +
[snip]
> diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_common.h
> b/drivers/media/video/s5p-mfc/s5p_mfc_common.h
> index e705938..512e84e 100644
> --- a/drivers/media/video/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/video/s5p-mfc/s5p_mfc_common.h
> @@ -34,10 +34,6 @@
> #define MFC_OFFSET_SHIFT 11
>
> #define FIRMWARE_ALIGN 0x2 /* 128KB */
> -#define MFC_H264_CTX_BUF_SIZE0x96000 /* 600KB per H264
instance
> */
> -#define MFC_CTX_BUF_SIZE 0x2800 /* 10KB per instance */
> -#define DESC_BUF_SIZE0x2 /* 128KB for DESC
buffer */
> -#define SHARED_BUF_SIZE 0x2000 /* 8KB for shared
buffer */
>
> #define DEF_CPB_SIZE 0x4 /* 512KB */
Here are the defines.
I have noticed that you remove them in future patches, but I think it is a
better
idea to keep them here.
>
> @@ -207,6 +203,47 @@ struct s5p_mfc_pm {
> struct device *device;
> };
>
> +struct s5p_mfc_buf_size_v5 {
> + unsigned int h264_ctx;
> + unsigned int non_h264_ctx;
> + unsigned int dsc;
> + unsigned int shm;
> +};
> +
> +struct s5p_mfc_buf_size {
> + unsigned int fw;
> + unsigned int cpb;
> + void *priv;
> +};
> +
> +struct s5p_mfc_buf_align {
> + unsigned int base;
> +};
> +
> +struct s5p_mfc_variant {
> + unsigned int version;
> + unsigned int port_num;
> + struct s5p_mfc_buf_size *buf_size;
> + struct s5p_mfc_buf_align *buf_align;
> +};
> +
> +/**
> + * struct s5p_mfc_priv_buf - represents internal used buffer
> + * @alloc: allocation-specific context for each buffer
> + * (videobuf2 allocator)
> + * @ofs: offset of each buffer, will be used for MFC
> + * @virt:kernel virtual address, only valid when the
> + * buffer accessed by driver
> + * @dma: DMA address, only valid when kernel DMA API used
> + */
> +struct s5p_mfc_priv_buf {
> + void*alloc;
> + unsigned long ofs;
> + void*virt;
> + dma_addr_t dma;
> + size_t size;
> +};
> +
> /**
> * struct s5p_mfc_dev - The struct containing driver internal parameters.
> *
> @@ -257,6 +294,7 @@ struct s5p_mfc_dev {
> struct v4l2_ctrl_handler dec_ctrl_handler;
> struct v4l2_ctrl_handler enc_ctrl_handler;
> struct s5p_mfc_pm pm;
> + struct s5p_mfc_variant *variant;
> int num_inst;
> spinlock_t irqlock; /* lock when operating on videobuf2 queues */
> spinlock_t condlock;/* lock when changing/checking if a context is
> @@ -295,7 +333,6 @@ struct s5p_mfc_h264_enc_params {
> u8 max_ref_pic;
> u8 num_ref_
RE: [PATCH v4 1/4] [media] s5p-mfc: Update MFCv5 driver for callback based architecture
Hi Arun,
First of all - thank you for all the patches, they are getting better and
better
with every version.
Two things:
1) A minor one - I suggest that the choice of the default pixel format should
be
done in a different manner. I suggest using a V4L2_PIX_FMT_* in the
DEF_SRC_FMT_DEC
define and then using find_format in s5p_mfc_dec_init. Same goes for encoding.
2) A major one - the ops mechanism could be improved. I see that there are
many functions that only call an ops. Such as:
> int s5p_mfc_alloc_dec_temp_buffers(struct s5p_mfc_ctx *ctx)
> {
>return s5p_mfc_ops->s5p_mfc_alloc_dec_temp_buffers(ctx);
> }
I suggest dropping the s5p_mfc_ prefix in all the fields of s5p_mfc_hw_ops and
using a macro to call the ops. Like this:
+#define fimc_pipeline_call(f, op, p, args...) \
+ (!(f) ? -ENODEV : (((f)->pipeline_ops && (f)->pipeline_ops->op) ? \
+ (f)->pipeline_ops->op((p), ##args) :
-ENOIOCTLCMD))
(from commit by Sylwester Nawrocki http://goo.gl/Q657j)
In addition, your code does not check whether the ops pointer is null and I
think that it should be done.
Best wishes,
--
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center
[snip the code]
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PATCHES FOR v3.7] V4L: Exynos5 SoC GScaler driver
Hi Mauro, The following changes since commit 01b0c11a1ba49ac96f58b7bc92772c2b469d6caa: [media] Kconfig: Fix b2c2 common code selection (2012-08-21 08:38:31 -0300) are available in the git repository at: git://git.infradead.org/users/kmpark/linux-samsung v4l-exynos-gsc for you to fetch changes up to 231560807f44daf9d1c2913e749c8a8609fc3c66: gscaler: Add Makefile for G-Scaler Driver (2012-08-22 10:36:49 +0200) This is a mem-to-mem driver for Exynos5 SoC series GScaler devices and an addition of multi-planar YVU420 fourcc. Shaik Ameer Basha (2): v4l: Add new YVU420 multi planar fourcc definition gscaler: Add Makefile for G-Scaler Driver Sungchun Kang (3): gscaler: Add new driver for generic scaler gscaler: Add core functionality for the G-Scaler driver gscaler: Add m2m functionality for the G-Scaler driver Documentation/DocBook/media/v4l/pixfmt-yvu420m.xml | 154 Documentation/DocBook/media/v4l/pixfmt.xml |1 + drivers/media/platform/Kconfig |8 + drivers/media/platform/Makefile|1 + drivers/media/platform/exynos-gsc/gsc-core.c | 1253 ++ drivers/media/platform/exynos-gsc/gsc-core.h | 527 + drivers/media/platform/exynos-gsc/gsc-m2m.c| 771 ++ drivers/media/platform/exynos-gsc/gsc-regs.c | 425 ++ drivers/media/platform/exynos-gsc/gsc-regs.h | 172 include/linux/videodev2.h |1 + 10 files changed, 3313 insertions(+) create mode 100644 Documentation/DocBook/media/v4l/pixfmt-yvu420m.xml create mode 100644 drivers/media/platform/exynos-gsc/gsc-core.c create mode 100644 drivers/media/platform/exynos-gsc/gsc-core.h create mode 100644 drivers/media/platform/exynos-gsc/gsc-m2m.c create mode 100644 drivers/media/platform/exynos-gsc/gsc-regs.c create mode 100644 drivers/media/platform/exynos-gsc/gsc-regs.h --- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media/radio/shark2: Fix build error caused by missing dependencies
On Wed, Aug 22, 2012 at 05:22:26PM +0200, Hans de Goede wrote: > Hi, > > I've a better fix for this here: > http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/media-for_v3.6 > > I already send a pull-req for this to Mauro a while ago, Mauro? > Looks like it found its way into mainline in the last couple of days. Should have updated my tree first. Sorry for the noise. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media/radio/shark2: Fix build error caused by missing dependencies
Hi, I've a better fix for this here: http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/media-for_v3.6 I already send a pull-req for this to Mauro a while ago, Mauro? Regards, Hans On 08/22/2012 05:16 PM, Guenter Roeck wrote: Fix build error: ERROR: "led_classdev_register" [drivers/media/radio/shark2.ko] undefined! ERROR: "led_classdev_unregister" [drivers/media/radio/shark2.ko] undefined! which is seen if RADIO_SHARK2 is enabled, but LEDS_CLASS is not. Since RADIO_SHARK2 depends on NEW_LEDS and LEDS_CLASS, select both if it is enabled. Cc: Hans de Goede Signed-off-by: Guenter Roeck --- drivers/media/radio/Kconfig |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig index 8090b87..bee4bee 100644 --- a/drivers/media/radio/Kconfig +++ b/drivers/media/radio/Kconfig @@ -77,6 +77,8 @@ config RADIO_SHARK config RADIO_SHARK2 tristate "Griffin radioSHARK2 USB radio receiver" depends on USB + select NEW_LEDS + select LEDS_CLASS ---help--- Choose Y here if you have this radio receiver. -- 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/1] driver-core: Shut up dev_dbg_reatelimited() without DEBUG
On 08/22/2012 04:57 PM, Hin-Tak Leung wrote:
Antti Palosaari wrote:
Hello Hiroshi,
On 08/21/2012 10:02 AM, Hiroshi Doyu wrote:
Antti Palosaari wrote @ Mon, 20 Aug 2012 23:29:34 +0200:
On 08/20/2012 02:14 PM, Hiroshi Doyu wrote:
Hi Antti,
Antti Palosaari wrote @ Sat, 18 Aug 2012 02:11:56
+0200:
On 08/17/2012 09:04 AM, Hiroshi Doyu wrote:
dev_dbg_reatelimited() without DEBUG printed "217078 callbacks
suppressed". This shouldn't print anything without DEBUG.
Signed-off-by: Hiroshi Doyu
Reported-by: Antti Palosaari
---
include/linux/device.h |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index eb945e1..d4dc26e 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -962,9 +962,13 @@ do {\
dev_level_ratelimited(dev_notice, dev, fmt, ##__VA_ARGS__)
#define dev_info_ratelimited(dev, fmt, ...)\
dev_level_ratelimited(dev_info, dev, fmt, ##__VA_ARGS__)
+#if defined(DEBUG)
#define dev_dbg_ratelimited(dev, fmt, ...)\
dev_level_ratelimited(dev_dbg, dev, fmt, ##__VA_ARGS__)
-
+#else
+#define dev_dbg_ratelimited(dev, fmt, ...)\
+no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#endif
/*
* Stupid hackaround for existing uses of non-printk uses
dev_info
*
NACK. I don't think that's correct behavior. After that patch it
kills
all output of dev_dbg_ratelimited(). If I use dynamic debugs and
order
debugs, I expect to see debugs as earlier.
You are right. I attached the update patch, just moving *_ratelimited
functions after dev_dbg() definitions.
With DEBUG defined/undefined in your "test.ko", it works fine. With
CONFIG_DYNAMIC_DEBUG, it works with "+p", but with "-p", still
"..callbacks suppressed" is printed.
I am using dynamic debugs and behavior is now just same as it was when
reported that bug. OK, likely for static debug it is now correct.
The following patch can also refrain "..callbacks suppressed" with
"-p". I think that it's ok for all cases.
From b4c6aa9160f03b61ed17975c73db36c983a48927 Mon Sep 17 00:00:00 2001
From: Hiroshi Doyu
Date: Mon, 20 Aug 2012 13:49:19 +0300
Subject: [v3 1/1] driver-core: Shut up dev_dbg_reatelimited() without
DEBUG
dev_dbg_reatelimited() without DEBUG printed "217078 callbacks
suppressed". This shouldn't print anything without DEBUG.
With CONFIG_DYNAMIC_DEBUG, the print should be configured as expected.
Signed-off-by: Hiroshi Doyu
Reported-by: Antti Palosaari
---
include/linux/device.h | 62
+--
1 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index 9648331..bb6ffcb 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -932,6 +932,32 @@ int _dev_info(const struct device *dev, const
char *fmt,
...)
#endif
+/*
+ * Stupid hackaround for existing uses of non-printk uses dev_info
+ *
+ * Note that the definition of dev_info below is actually _dev_info
+ * and a macro is used to avoid redefining dev_info
+ */
+
+#define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
+
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define dev_dbg(dev, format, ...) \
+do { \
+dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
+} while (0)
+#elif defined(DEBUG)
+#define dev_dbg(dev, format, arg...)\
+dev_printk(KERN_DEBUG, dev, format, ##arg)
+#else
+#define dev_dbg(dev, format, arg...)\
+({\
+if (0)\
+dev_printk(KERN_DEBUG, dev, format, ##arg);\
+0;\
+})
+#endif
+
#define dev_level_ratelimited(dev_level, dev, fmt, ...)\
do {\
static DEFINE_RATELIMIT_STATE(_rs,\
@@ -955,33 +981,21 @@ do {\
dev_level_ratelimited(dev_notice, dev, fmt, ##__VA_ARGS__)
#define dev_info_ratelimited(dev, fmt, ...)\
dev_level_ratelimited(dev_info, dev, fmt, ##__VA_ARGS__)
+#if defined(CONFIG_DYNAMIC_DEBUG) || defined(DEBUG)
#define dev_dbg_ratelimited(dev, fmt, ...)\
-dev_level_ratelimited(dev_dbg, dev, fmt, ##__VA_ARGS__)
-
-/*
- * Stupid hackaround for existing uses of non-printk uses dev_info
- *
- * Note that the definition of dev_info below is actually _dev_info
- * and a macro is used to avoid redefining dev_info
- */
-
-#define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
-
-#if defined(CONFIG_DYNAMIC_DEBUG)
-#define dev_dbg(dev, format, ...) \
-do { \
-dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
+do {\
+static DEFINE_RATELIMIT_STATE(_rs,\
+ DEFAULT_RATELIMIT_INTERVAL,\
+
Re: RFC: Core + Radio profile
On Wed, 22 Aug 2012, Mauro Carvalho Chehab wrote: > Em 22-08-2012 07:11, Hans Verkuil escreveu: > > I've added some more core profile requirements. > > >> > >> Streaming I/O is not supported by radio nodes. > > Hmm... pvrusb2/ivtv? Ok, it makes sense to move it to use the alsa > mpeg API there. If we're enforcing it, we should deprecate the current way > there, and make it use ALSA. I am unaware of any ALSA MPEG API. It's entirely likely that this is because I haven't been paying attention. Nevertheless, can you please point me at any documentation on this so I can get up to speed? Currently the pvrusb2 driver does not attempt to perform any processing or filtering of the data stream, so radio data is just the same mpeg stream as video (but without any real embedded video data). If I have to get into the business of processing the MPEG data in order to adhere to this proposal, then that will be a very big deal for this driver. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] media/radio/shark2: Fix build error caused by missing dependencies
Fix build error: ERROR: "led_classdev_register" [drivers/media/radio/shark2.ko] undefined! ERROR: "led_classdev_unregister" [drivers/media/radio/shark2.ko] undefined! which is seen if RADIO_SHARK2 is enabled, but LEDS_CLASS is not. Since RADIO_SHARK2 depends on NEW_LEDS and LEDS_CLASS, select both if it is enabled. Cc: Hans de Goede Signed-off-by: Guenter Roeck --- drivers/media/radio/Kconfig |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig index 8090b87..bee4bee 100644 --- a/drivers/media/radio/Kconfig +++ b/drivers/media/radio/Kconfig @@ -77,6 +77,8 @@ config RADIO_SHARK config RADIO_SHARK2 tristate "Griffin radioSHARK2 USB radio receiver" depends on USB + select NEW_LEDS + select LEDS_CLASS ---help--- Choose Y here if you have this radio receiver. -- 1.7.9.7 -- 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 patch 4/4] Re: dma-buf-mgr: multiple dma-buf synchronization (v3)
On Wed, Aug 22, 2012 at 02:52:10PM +0200, Thomas Hellstrom wrote: > Hi, Maarten, > please see some comments inline. > > On 08/22/2012 01:50 PM, Maarten Lankhorst wrote: > >Hey Dan, > > > >Op 16-08-12 01:12, Daniel Vetter schreef: > >>Hi Maarten, > >> > >>Ok, here comes the promised review (finally!), but it's rather a > >>high-level thingy. I've mostly thought about how we could create a neat > >>api with the following points. For a bit of clarity, I've grouped the > >>different considerations a bit. > >> > >Thanks, I have significantly reworked the api based on your comments. > > > >Documentation is currently lacking, and will get updated again for the final > >version. > > > >Full patch series also includes some ttm changes to make use of > >dma-reservation, > >with the intention of moving out fencing from ttm too, but that requires > >more work. > > > >For the full series see: > >http://cgit.freedesktop.org/~mlankhorst/linux/log/?h=v10-wip > > > >My plan is to add a pointer for dma_reservation to a dma-buf, > >so all users of dma-reservation can perform reservations across > >multiple devices as well. Since the default for ttm likely will > >mean only a few buffers are shared I didn't want to complicate > >the abi for ttm much further so only added a pointer that can be > >null to use ttm's reservation_object structure. > > > >The major difference with ttm is that each reservation object > >gets its own lock for fencing and reservations, but they can > >be merged: > > TTM previously had a lock on each buffer object which protected > sync_obj and sync_obj_arg, however > when fencing multiple buffers, say 100 buffers or so in a single > command submission, it meant 100 > locks / unlocks that weren't really necessary, since just updating > the sync_obj and sync_obj_arg members > is a pretty quick operation, whereas locking may be a pretty slow > operation, so those locks were removed > for efficiency. > The reason a single lock (the lru lock) is used to protect > reservation is that a TTM object that is being reserved > *atomically* needs to be taken off LRU lists, since processes > performing LRU eviction don't take a ticket > when evicting, and may thus cause deadlocks; It might be possible to > fix this within TTM by requiring a ticket > for all reservation, but then that ticket needs to be passed down > the call chain for all functions that may perform > a reservation. It would perhaps be simpler (but perhaps not so fair) > to use the current thread info pointer as a ticket > sequence number. While discussing this stuff with Maarten I've read through the generic mutex code, and I think we could adapt the ideas from in there (which would boil down to a single atomice op for the fastpath for both reserve and unreserve, which even have per-arch optimized asm). So I think we can make the per-obj lock as fast as it's possible, since the current ttm fences already use that atomic op. For passing the reservation_ticket down the callstacks I guess with a common reservation systems used for shared buffers (which is the idea here) we can make a good case to add a pointer to the current thread info. Especially for cross-device reservations through dma_buf I think that would simplify the interfaces quite a bit. Wrt the dma_ prefix I agree it's not a stellar name, but since the intention is to use this together with dma_buf and dma_fence to faciliate cross-device madness it does fit somewhat ... Fyi I hopefully get around to play with Maarten's patches a bit, too. One of the things I'd like to add to the current reservation framework is lockdep annotations. Since if we use this across devices it's way too easy to nest reservations improperly, or to create deadlocks because one thread grabs another lock while holding reservations, while another tries to reserve buffers while holding that lock. > >spin_lock(obj->resv) __dma_object_reserve() grab a ref to all > >obj->fences spin_unlock(obj->resv) > > > >spin_lock(obj->resv) assign new fence to obj->fences > >__dma_object_unreserve() spin_unlock(obj->resv) > > > >There's only one thing about fences I haven't been able to map yet > >properly. vmwgfx has sync_obj_flush, but as far as I can tell it has > >not much to do with sync objects, but is rather a generic 'flush before > >release'. Maybe one of the vmwgfx devs could confirm whether that call > >is really needed there? And if so, if there could be some other way do > >that, because it seems to be the ttm_bo_wait call before that would be > >enough, if not it might help more to move the flush to some other call. > > The fence flush should be interpreted as an operation for fencing > mechanisms that aren't otherwise required to signal in finite time, and > where the time from flush to signal might be substantial. TTM is then > supposed to issue a fence flush when it knows ahead of time that it will > soon perform a periodical poll for a buffer to be idle, but not block > waiting for the buffer to be idle. T
Re: [RFC patch 4/4] Re: dma-buf-mgr: multiple dma-buf synchronization (v3)
On 08/22/2012 03:32 PM, Maarten Lankhorst wrote: Hey, Op 22-08-12 14:52, Thomas Hellstrom schreef: Hi, Maarten, please see some comments inline. On 08/22/2012 01:50 PM, Maarten Lankhorst wrote: Hey Dan, Op 16-08-12 01:12, Daniel Vetter schreef: Hi Maarten, Ok, here comes the promised review (finally!), but it's rather a high-level thingy. I've mostly thought about how we could create a neat api with the following points. For a bit of clarity, I've grouped the different considerations a bit. Thanks, I have significantly reworked the api based on your comments. Documentation is currently lacking, and will get updated again for the final version. Full patch series also includes some ttm changes to make use of dma-reservation, with the intention of moving out fencing from ttm too, but that requires more work. For the full series see: http://cgit.freedesktop.org/~mlankhorst/linux/log/?h=v10-wip My plan is to add a pointer for dma_reservation to a dma-buf, so all users of dma-reservation can perform reservations across multiple devices as well. Since the default for ttm likely will mean only a few buffers are shared I didn't want to complicate the abi for ttm much further so only added a pointer that can be null to use ttm's reservation_object structure. The major difference with ttm is that each reservation object gets its own lock for fencing and reservations, but they can be merged: TTM previously had a lock on each buffer object which protected sync_obj and sync_obj_arg, however when fencing multiple buffers, say 100 buffers or so in a single command submission, it meant 100 locks / unlocks that weren't really necessary, since just updating the sync_obj and sync_obj_arg members is a pretty quick operation, whereas locking may be a pretty slow operation, so those locks were removed for efficiency. Speaking of which, mind if I kill sync_obj_arg? Only user is again vmwgfx and it always seems to pass the same for flags, namely DRM_VMW_FENCE_FLAG_EXEC. I guess so, although I've always thought it to be a great idea :), but nobody really understands or care what it's for. Which is a single fence might have multiple definitions of signaled, depending on the user: Consider an awkward GPU with a single command stream that feeds multiple engines. The command parser signals when it has parsed the commands, the 2D engine signals when it is done with the 2D commands it has been fed, and the 3D engine signals when the 3D engine is done, and finally the flush engine signals when all rendered data is flushed. Depending on which engines touch a buffer, each buffer may have a different view on when the attached fence is signaled. But anyway. No in-tree driver is using it, (the old unichrome driver did), and I guess the same functionality can be implemented with multiple fences attached to a single buffer, so feel free to get rid of it. The reason a single lock (the lru lock) is used to protect reservation is that a TTM object that is being reserved *atomically* needs to be taken off LRU lists, since processes performing LRU eviction don't take a ticket when evicting, and may thus cause deadlocks; It might be possible to fix this within TTM by requiring a ticket for all reservation, but then that ticket needs to be passed down the call chain for all functions that may perform a reservation. It would perhaps be simpler (but perhaps not so fair) to use the current thread info pointer as a ticket sequence number. Yeah, that's why the ttm patch for ttm_bo_reserve_locked always calls dma_object_reserve with no_wait set to true. :) It does its own EBUSY handling for the no_wait case, so there should be no functional changes. I need to look a bit deeper into the TTM patches, but as long as nothing breaks I've nothing against it using dma reservation objects. OTOH, it might be worthwhile thinking about the 'dma' prefix, since the reservation objects may find use elsewhere as well, for example for vmwgfx resources, that really have little to do with dma-buffers or buffers at all. I've been toying with the idea of always requiring a sequence number, I just didn't in the current patch yet since it would mean converting every driver, so for a preliminary patch based on a unmerged api it was not worth the time. spin_lock(obj->resv) __dma_object_reserve() grab a ref to all obj->fences spin_unlock(obj->resv) spin_lock(obj->resv) assign new fence to obj->fences __dma_object_unreserve() spin_unlock(obj->resv) There's only one thing about fences I haven't been able to map yet properly. vmwgfx has sync_obj_flush, but as far as I can tell it has not much to do with sync objects, but is rather a generic 'flush before release'. Maybe one of the vmwgfx devs could confirm whether that call is really needed there? And if so, if there could be some other way do that, because it seems to be the ttm_bo_wait call before that would be enough, if not it might help more to move the flush to some
Re: [PATCH 1/1] driver-core: Shut up dev_dbg_reatelimited() without DEBUG
Antti Palosaari wrote:
Hello Hiroshi,
On 08/21/2012 10:02 AM, Hiroshi Doyu wrote:
Antti Palosaari wrote @ Mon, 20 Aug 2012 23:29:34 +0200:
On 08/20/2012 02:14 PM, Hiroshi Doyu wrote:
Hi Antti,
Antti Palosaari wrote @ Sat, 18 Aug 2012 02:11:56 +0200:
On 08/17/2012 09:04 AM, Hiroshi Doyu wrote:
dev_dbg_reatelimited() without DEBUG printed "217078 callbacks
suppressed". This shouldn't print anything without DEBUG.
Signed-off-by: Hiroshi Doyu
Reported-by: Antti Palosaari
---
include/linux/device.h |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index eb945e1..d4dc26e 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -962,9 +962,13 @@ do {\
dev_level_ratelimited(dev_notice, dev, fmt, ##__VA_ARGS__)
#define dev_info_ratelimited(dev, fmt, ...)\
dev_level_ratelimited(dev_info, dev, fmt, ##__VA_ARGS__)
+#if defined(DEBUG)
#define dev_dbg_ratelimited(dev, fmt, ...)\
dev_level_ratelimited(dev_dbg, dev, fmt, ##__VA_ARGS__)
-
+#else
+#define dev_dbg_ratelimited(dev, fmt, ...)\
+no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#endif
/*
* Stupid hackaround for existing uses of non-printk uses dev_info
*
NACK. I don't think that's correct behavior. After that patch it kills
all output of dev_dbg_ratelimited(). If I use dynamic debugs and order
debugs, I expect to see debugs as earlier.
You are right. I attached the update patch, just moving *_ratelimited
functions after dev_dbg() definitions.
With DEBUG defined/undefined in your "test.ko", it works fine. With
CONFIG_DYNAMIC_DEBUG, it works with "+p", but with "-p", still
"..callbacks suppressed" is printed.
I am using dynamic debugs and behavior is now just same as it was when
reported that bug. OK, likely for static debug it is now correct.
The following patch can also refrain "..callbacks suppressed" with
"-p". I think that it's ok for all cases.
From b4c6aa9160f03b61ed17975c73db36c983a48927 Mon Sep 17 00:00:00 2001
From: Hiroshi Doyu
Date: Mon, 20 Aug 2012 13:49:19 +0300
Subject: [v3 1/1] driver-core: Shut up dev_dbg_reatelimited() without DEBUG
dev_dbg_reatelimited() without DEBUG printed "217078 callbacks
suppressed". This shouldn't print anything without DEBUG.
With CONFIG_DYNAMIC_DEBUG, the print should be configured as expected.
Signed-off-by: Hiroshi Doyu
Reported-by: Antti Palosaari
---
include/linux/device.h | 62 +--
1 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index 9648331..bb6ffcb 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -932,6 +932,32 @@ int _dev_info(const struct device *dev, const char *fmt,
...)
#endif
+/*
+ * Stupid hackaround for existing uses of non-printk uses dev_info
+ *
+ * Note that the definition of dev_info below is actually _dev_info
+ * and a macro is used to avoid redefining dev_info
+ */
+
+#define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
+
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define dev_dbg(dev, format, ...) \
+do { \
+dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
+} while (0)
+#elif defined(DEBUG)
+#define dev_dbg(dev, format, arg...)\
+dev_printk(KERN_DEBUG, dev, format, ##arg)
+#else
+#define dev_dbg(dev, format, arg...)\
+({\
+if (0)\
+dev_printk(KERN_DEBUG, dev, format, ##arg);\
+0;\
+})
+#endif
+
#define dev_level_ratelimited(dev_level, dev, fmt, ...)\
do {\
static DEFINE_RATELIMIT_STATE(_rs,\
@@ -955,33 +981,21 @@ do {\
dev_level_ratelimited(dev_notice, dev, fmt, ##__VA_ARGS__)
#define dev_info_ratelimited(dev, fmt, ...)\
dev_level_ratelimited(dev_info, dev, fmt, ##__VA_ARGS__)
+#if defined(CONFIG_DYNAMIC_DEBUG) || defined(DEBUG)
#define dev_dbg_ratelimited(dev, fmt, ...)\
-dev_level_ratelimited(dev_dbg, dev, fmt, ##__VA_ARGS__)
-
-/*
- * Stupid hackaround for existing uses of non-printk uses dev_info
- *
- * Note that the definition of dev_info below is actually _dev_info
- * and a macro is used to avoid redefining dev_info
- */
-
-#define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
-
-#if defined(CONFIG_DYNAMIC_DEBUG)
-#define dev_dbg(dev, format, ...) \
-do { \
-dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
+do {\
+static DEFINE_RATELIMIT_STATE(_rs,\
+ DEFAULT_RATELIMIT_INTERVAL,\
+ DEFAULT_RATELIMIT_BURST);
Re: RFC: Core + Radio profile
Em 22-08-2012 07:11, Hans Verkuil escreveu: > I've added some more core profile requirements. > > Regards, > > Hans > > On Wed August 22 2012 11:40:25 Hans Verkuil wrote: >> Hi all! >> >> The v4l2-compliance utility checks whether device drivers follows the V4L2 >> specification. What is missing are 'profiles' detailing what device drivers >> should and shouldn't implement for particular device classes. >> >> This has been discussed several times, but until now no attempt was made to >> actually write this down. >> >> This RFC is my attempt to start this process by describing three profiles: >> the core profile that all drivers must adhere to, a profile for a radio >> receiver and a profile for a radio transmitter. >> >> Missing in this RFC is a description of how tuner ownership is handled if a >> tuner is shared between a radio and video driver. This will be discussed >> during the workshop next week. DVB x V4L2 tunership discussion is also needed. It also makes sense to document it somewhere. >> I am not certain where these profile descriptions should go. Should we add >> this to DocBook, or describe it in Documentation/video4linux? Or perhaps >> somehow add it to v4l2-compliance, since that tool effectively verifies >> the profile. The strongest document we have, IMO, is the DocBook API one, as both Kernel and userspace developers use it. As we want userspace apps to be aware and benefit of the profiles, this should be there at DocBook. >> Also note that the core profile description is more strict than the spec. IMO, that's the right approach: The specs should define the several API aspects; the profile should mandate what's optional and what's mandatory. >> For example, G/S_PRIORITY are currently optional, After putting the profiles there, we should remove "optional" tags elsewhere, as the profiles section will tell what's mandatory and what is optional, as different profiles require different mandatory arguments. >> but I feel that all new drivers should implement this, especially since it >> is very easy to add provided you use struct v4l2_fh. I agree on making G/S_PRIORITY mandatory. >> Similar with respect to the control requirements which >> assume you use the control framework. IMO, we should split the internal API requirements (use the control framework) from the external API ones, as they're addressed to different audiences, e. g. external API requirements are needed by all application developers, while the internal ones are only for Kernel hackers. The internal API requirements should be together with the internal API descriptions, so Documentation/video4linux is probably the best place for them. >> Note that these profiles are primarily meant for driver developers. The V4L2 >> specification is leading for application developers. This is where we diverge ;) We need profiles primary for application developers, for them to know what is expected to be there on any media driver of a certain kind. Ok, internal driver-developer profiles is also needed, in order for them to use the right internal API's and internal core functions. >> While writing down these profiles I noticed one thing that was very much >> missing for radio devices: there is no capability bit to tell the application >> whether there is an associated ALSA device or whether the audio goes through >> a line-in/out. Personally I think that this should be fixed. Yes, this is one bit that it is missing. Well, this is currently handled this at xawtv3 "radio" aplication by using the libmedia_dev API there. It likely makes sense, for a version 2 of the profiles (e. g. after we finish merging the basic stuff there), to add a chapter at the profiles section recommending the usage of the libraries provided by v4l-utils, with some description or code examples. >> >> Comments are welcome! >> >> Regards, >> >> Hans >> >> >> Core Profile >> >> >> All V4L2 device nodes must support the core profile. >> >> VIDIOC_QUERYCAP must be supported and must set the device_caps field. >> bus_info must be a unique string and must not be empty (pending result of >> the upcoming workshop). >> >> VIDIOC_G/S_PRIORITY must be supported. >> >> If you have controls then both the VIDIOC_G/S_CTRL and the >> VIDIOC_G/S/TRY_EXT_CTRLS >> ioctls must be supported, together with poll(), VIDIOC_DQEVENT and >> VIDIOC_(UN)SUBSCRIBE_EVENT to handle control events. >> Use the control framework to implement this. Now looking at the concrete case, I don't see any troubles on adding it to the V4L2 API, but writing it as: "Driver developers must use the control framework to implement this." >> VIDIOC_LOG_STATUS is optional, but recommended for complex drivers. >> >> VIDIOC_DBG_G_CHIP_IDENT, VIDIOC_DBG_G/S_REGISTER are optional and are >> primarily used to debug drivers. The above ones require a note for application developers: "Applications must not use VIDIOC_LOG_STATUS during its normal behaviour;
Re: [RFC patch 4/4] Re: dma-buf-mgr: multiple dma-buf synchronization (v3)
Hey,
Op 22-08-12 14:52, Thomas Hellstrom schreef:
> Hi, Maarten,
> please see some comments inline.
>
> On 08/22/2012 01:50 PM, Maarten Lankhorst wrote:
>> Hey Dan,
>>
>> Op 16-08-12 01:12, Daniel Vetter schreef:
>>> Hi Maarten,
>>>
>>> Ok, here comes the promised review (finally!), but it's rather a
>>> high-level thingy. I've mostly thought about how we could create a neat
>>> api with the following points. For a bit of clarity, I've grouped the
>>> different considerations a bit.
>>>
>> Thanks, I have significantly reworked the api based on your comments.
>>
>> Documentation is currently lacking, and will get updated again for the final
>> version.
>>
>> Full patch series also includes some ttm changes to make use of
>> dma-reservation,
>> with the intention of moving out fencing from ttm too, but that requires
>> more work.
>>
>> For the full series see:
>> http://cgit.freedesktop.org/~mlankhorst/linux/log/?h=v10-wip
>>
>> My plan is to add a pointer for dma_reservation to a dma-buf,
>> so all users of dma-reservation can perform reservations across
>> multiple devices as well. Since the default for ttm likely will
>> mean only a few buffers are shared I didn't want to complicate
>> the abi for ttm much further so only added a pointer that can be
>> null to use ttm's reservation_object structure.
>>
>> The major difference with ttm is that each reservation object
>> gets its own lock for fencing and reservations, but they can
>> be merged:
>
> TTM previously had a lock on each buffer object which protected sync_obj and
> sync_obj_arg, however
> when fencing multiple buffers, say 100 buffers or so in a single command
> submission, it meant 100
> locks / unlocks that weren't really necessary, since just updating the
> sync_obj and sync_obj_arg members
> is a pretty quick operation, whereas locking may be a pretty slow operation,
> so those locks were removed
> for efficiency.
Speaking of which, mind if I kill sync_obj_arg? Only user is again vmwgfx and
it always seems to pass the same
for flags, namely DRM_VMW_FENCE_FLAG_EXEC.
> The reason a single lock (the lru lock) is used to protect reservation is
> that a TTM object that is being reserved
> *atomically* needs to be taken off LRU lists, since processes performing LRU
> eviction don't take a ticket
> when evicting, and may thus cause deadlocks; It might be possible to fix this
> within TTM by requiring a ticket
> for all reservation, but then that ticket needs to be passed down the call
> chain for all functions that may perform
> a reservation. It would perhaps be simpler (but perhaps not so fair) to use
> the current thread info pointer as a ticket
> sequence number.
Yeah, that's why the ttm patch for ttm_bo_reserve_locked always calls
dma_object_reserve with no_wait set to true. :)
It does its own EBUSY handling for the no_wait case, so there should be no
functional changes.
I've been toying with the idea of always requiring a sequence number, I just
didn't in the current patch yet
since it would mean converting every driver, so for a preliminary patch based
on a unmerged api it was
not worth the time.
>> spin_lock(obj->resv)
>> __dma_object_reserve()
>> grab a ref to all obj->fences
>> spin_unlock(obj->resv)
>>
>> spin_lock(obj->resv)
>> assign new fence to obj->fences
>> __dma_object_unreserve()
>> spin_unlock(obj->resv)
>>
>> There's only one thing about fences I haven't been able to map
>> yet properly. vmwgfx has sync_obj_flush, but as far as I can
>> tell it has not much to do with sync objects, but is rather a
>> generic 'flush before release'. Maybe one of the vmwgfx devs
>> could confirm whether that call is really needed there? And if
>> so, if there could be some other way do that, because it seems
>> to be the ttm_bo_wait call before that would be enough, if not
>> it might help more to move the flush to some other call.
>
> The fence flush should be interpreted as an operation for fencing mechanisms
> that aren't otherwise required to
> signal in finite time, and where the time from flush to signal might be
> substantial. TTM is then supposed to
> issue a fence flush when it knows ahead of time that it will soon perform a
> periodical poll for a buffer to be
> idle, but not block waiting for the buffer to be idle. The delayed buffer
> delete mechanism is, I think, the only user
> currently.
> For hardware that always signal fences immediately, the flush mechanism is
> not needed.
So if I understand it correctly it is the same as I'm doing in fences with
dma_fence::enable_sw_signals?
Great, I don't need to add another op then. Although it looks like I should
export a function to manually
enable it for those cases. :)
>>
>> +
>> +int
>> +__dma_object_reserve(struct dma_reservation_object *obj, bool intr,
>> + bool no_wait, dma_reservation_ticket_t *ticket)
>> +{
>> +int ret;
>> +u64 sequence = ticket ? ticket->seqno : 0;
>> +
>> +while (unlikely(atomic_cmpxchg(&obj->reserved, 0, 1) !=
Re: [RFC patch 4/4] Re: dma-buf-mgr: multiple dma-buf synchronization (v3)
Hi, Maarten, please see some comments inline. On 08/22/2012 01:50 PM, Maarten Lankhorst wrote: Hey Dan, Op 16-08-12 01:12, Daniel Vetter schreef: Hi Maarten, Ok, here comes the promised review (finally!), but it's rather a high-level thingy. I've mostly thought about how we could create a neat api with the following points. For a bit of clarity, I've grouped the different considerations a bit. Thanks, I have significantly reworked the api based on your comments. Documentation is currently lacking, and will get updated again for the final version. Full patch series also includes some ttm changes to make use of dma-reservation, with the intention of moving out fencing from ttm too, but that requires more work. For the full series see: http://cgit.freedesktop.org/~mlankhorst/linux/log/?h=v10-wip My plan is to add a pointer for dma_reservation to a dma-buf, so all users of dma-reservation can perform reservations across multiple devices as well. Since the default for ttm likely will mean only a few buffers are shared I didn't want to complicate the abi for ttm much further so only added a pointer that can be null to use ttm's reservation_object structure. The major difference with ttm is that each reservation object gets its own lock for fencing and reservations, but they can be merged: TTM previously had a lock on each buffer object which protected sync_obj and sync_obj_arg, however when fencing multiple buffers, say 100 buffers or so in a single command submission, it meant 100 locks / unlocks that weren't really necessary, since just updating the sync_obj and sync_obj_arg members is a pretty quick operation, whereas locking may be a pretty slow operation, so those locks were removed for efficiency. The reason a single lock (the lru lock) is used to protect reservation is that a TTM object that is being reserved *atomically* needs to be taken off LRU lists, since processes performing LRU eviction don't take a ticket when evicting, and may thus cause deadlocks; It might be possible to fix this within TTM by requiring a ticket for all reservation, but then that ticket needs to be passed down the call chain for all functions that may perform a reservation. It would perhaps be simpler (but perhaps not so fair) to use the current thread info pointer as a ticket sequence number. spin_lock(obj->resv) __dma_object_reserve() grab a ref to all obj->fences spin_unlock(obj->resv) spin_lock(obj->resv) assign new fence to obj->fences __dma_object_unreserve() spin_unlock(obj->resv) There's only one thing about fences I haven't been able to map yet properly. vmwgfx has sync_obj_flush, but as far as I can tell it has not much to do with sync objects, but is rather a generic 'flush before release'. Maybe one of the vmwgfx devs could confirm whether that call is really needed there? And if so, if there could be some other way do that, because it seems to be the ttm_bo_wait call before that would be enough, if not it might help more to move the flush to some other call. The fence flush should be interpreted as an operation for fencing mechanisms that aren't otherwise required to signal in finite time, and where the time from flush to signal might be substantial. TTM is then supposed to issue a fence flush when it knows ahead of time that it will soon perform a periodical poll for a buffer to be idle, but not block waiting for the buffer to be idle. The delayed buffer delete mechanism is, I think, the only user currently. For hardware that always signal fences immediately, the flush mechanism is not needed. PS: For ttm devs some of the code may look familiar, I don't know if the kernel accepts I-told-you-so tag or not, but if it does you might want to add them now. :-) PPS: I'm aware that I still need to add a signaled op to fences diff --git a/Documentation/DocBook/device-drivers.tmpl b/Documentation/DocBook/device-drivers.tmpl index 030f705..7da9637 100644 --- a/Documentation/DocBook/device-drivers.tmpl +++ b/Documentation/DocBook/device-drivers.tmpl @@ -129,6 +129,8 @@ X!Edrivers/base/interface.c !Edrivers/base/dma-fence.c !Iinclude/linux/dma-fence.h !Iinclude/linux/dma-seqno-fence.h +!Edrivers/base/dma-reservation.c +!Iinclude/linux/dma-reservation.h !Edrivers/base/dma-coherent.c !Edrivers/base/dma-mapping.c diff --git a/drivers/base/Makefile b/drivers/base/Makefile index 6e9f217..b26e639 100644 --- a/drivers/base/Makefile +++ b/drivers/base/Makefile @@ -10,7 +10,7 @@ obj-$(CONFIG_CMA) += dma-contiguous.o obj-y += power/ obj-$(CONFIG_HAS_DMA) += dma-mapping.o obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o -obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o dma-fence.o +obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o dma-fence.o dma-reservation.o obj-$(CONFIG_ISA) += isa.o obj-$(CONFIG_FW_LOADER) += firmware_class.o obj-$(CONFIG_NUMA)+= node.o diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c index 24e88fe..
Re: [PATCHv8 01/26] v4l: Add DMABUF as a memory type
On Wed August 22 2012 14:09:18 Tomasz Stanislawski wrote:
> Hi Hans,
> Thank your for the review.
> Please refer to the comments below.
>
> On 08/22/2012 12:27 PM, Hans Verkuil wrote:
> > On Tue August 14 2012 17:34:31 Tomasz Stanislawski wrote:
> >> From: Sumit Semwal
> >>
> >> Adds DMABUF memory type to v4l framework. Also adds the related file
> >> descriptor in v4l2_plane and v4l2_buffer.
> >>
> >> Signed-off-by: Tomasz Stanislawski
> >>[original work in the PoC for buffer sharing]
> >> Signed-off-by: Sumit Semwal
> >> Signed-off-by: Sumit Semwal
> >> Acked-by: Laurent Pinchart
> >> ---
> >> drivers/media/video/v4l2-compat-ioctl32.c | 18 ++
> >> drivers/media/video/v4l2-ioctl.c |1 +
> >> include/linux/videodev2.h |7 +++
> >> 3 files changed, 26 insertions(+)
> >>
> >> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
> >> b/drivers/media/video/v4l2-compat-ioctl32.c
> >> index 9ebd5c5..a2e0549 100644
> >> --- a/drivers/media/video/v4l2-compat-ioctl32.c
> >> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> >> @@ -304,6 +304,7 @@ struct v4l2_plane32 {
> >>union {
> >>__u32 mem_offset;
> >>compat_long_t userptr;
> >> + __u32 fd;
> >
> > Shouldn't this be int?
> >
>
> Notice that this field should be consistent with fd field used in
> 'struct v4l2_exportbuffer'. Therefore I prefer to use fixed-size types.
> One could use __s32 here but notice that file descriptors are defined
> as small, nonnegative integers according to POSIX spec. The type __u32
> suits well for this purpose. The negative values returned by open
> syscall are used only to indicate failures.
>
> On the other hand, using __s32 may help to avoid compiler warning while
> building userspace apps due to 'signed-vs-unsigned comparisons'.
>
> However, I do not have any strong opinion about 'int vs __u32' issue :).
> Do you think that using __s32 for both QUERYBUF and EXPBUF is a good
> compromise?
The type of a fd is highly variable in the kernel. Just try this for fun:
grep [^a-z]fd[^a-z] -rsI include/linux/
'int' or 'unsigned int' are by far the most common types.
But in structs I did see a few __s32 types, so I think __s32 is a decent
type to use. Just make sure it is __s32 everywhere. Right now __u32 and
int are both used.
Regards,
Hans
>
> >>} m;
> >>__u32 data_offset;
> >>__u32 reserved[11];
> >> @@ -325,6 +326,7 @@ struct v4l2_buffer32 {
> >>__u32 offset;
> >>compat_long_t userptr;
> >>compat_caddr_t planes;
> >> + __u32 fd;
> >
> > Ditto.
> >
> >>} m;
> >>__u32 length;
> >>__u32 reserved2;
>
> > Regards,
> >
> > Hans
> >
>
> 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
>
--
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: [PATCHv8 01/26] v4l: Add DMABUF as a memory type
Hi Hans,
Thank your for the review.
Please refer to the comments below.
On 08/22/2012 12:27 PM, Hans Verkuil wrote:
> On Tue August 14 2012 17:34:31 Tomasz Stanislawski wrote:
>> From: Sumit Semwal
>>
>> Adds DMABUF memory type to v4l framework. Also adds the related file
>> descriptor in v4l2_plane and v4l2_buffer.
>>
>> Signed-off-by: Tomasz Stanislawski
>>[original work in the PoC for buffer sharing]
>> Signed-off-by: Sumit Semwal
>> Signed-off-by: Sumit Semwal
>> Acked-by: Laurent Pinchart
>> ---
>> drivers/media/video/v4l2-compat-ioctl32.c | 18 ++
>> drivers/media/video/v4l2-ioctl.c |1 +
>> include/linux/videodev2.h |7 +++
>> 3 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
>> b/drivers/media/video/v4l2-compat-ioctl32.c
>> index 9ebd5c5..a2e0549 100644
>> --- a/drivers/media/video/v4l2-compat-ioctl32.c
>> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
>> @@ -304,6 +304,7 @@ struct v4l2_plane32 {
>> union {
>> __u32 mem_offset;
>> compat_long_t userptr;
>> +__u32 fd;
>
> Shouldn't this be int?
>
Notice that this field should be consistent with fd field used in
'struct v4l2_exportbuffer'. Therefore I prefer to use fixed-size types.
One could use __s32 here but notice that file descriptors are defined
as small, nonnegative integers according to POSIX spec. The type __u32
suits well for this purpose. The negative values returned by open
syscall are used only to indicate failures.
On the other hand, using __s32 may help to avoid compiler warning while
building userspace apps due to 'signed-vs-unsigned comparisons'.
However, I do not have any strong opinion about 'int vs __u32' issue :).
Do you think that using __s32 for both QUERYBUF and EXPBUF is a good
compromise?
>> } m;
>> __u32 data_offset;
>> __u32 reserved[11];
>> @@ -325,6 +326,7 @@ struct v4l2_buffer32 {
>> __u32 offset;
>> compat_long_t userptr;
>> compat_caddr_t planes;
>> +__u32 fd;
>
> Ditto.
>
>> } m;
>> __u32 length;
>> __u32 reserved2;
> Regards,
>
> Hans
>
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
[RFC patch 4/4] Re: dma-buf-mgr: multiple dma-buf synchronization (v3)
Hey Dan,
Op 16-08-12 01:12, Daniel Vetter schreef:
> Hi Maarten,
>
> Ok, here comes the promised review (finally!), but it's rather a
> high-level thingy. I've mostly thought about how we could create a neat
> api with the following points. For a bit of clarity, I've grouped the
> different considerations a bit.
>
Thanks, I have significantly reworked the api based on your comments.
Documentation is currently lacking, and will get updated again for the final
version.
Full patch series also includes some ttm changes to make use of dma-reservation,
with the intention of moving out fencing from ttm too, but that requires more
work.
For the full series see:
http://cgit.freedesktop.org/~mlankhorst/linux/log/?h=v10-wip
My plan is to add a pointer for dma_reservation to a dma-buf,
so all users of dma-reservation can perform reservations across
multiple devices as well. Since the default for ttm likely will
mean only a few buffers are shared I didn't want to complicate
the abi for ttm much further so only added a pointer that can be
null to use ttm's reservation_object structure.
The major difference with ttm is that each reservation object
gets its own lock for fencing and reservations, but they can
be merged:
spin_lock(obj->resv)
__dma_object_reserve()
grab a ref to all obj->fences
spin_unlock(obj->resv)
spin_lock(obj->resv)
assign new fence to obj->fences
__dma_object_unreserve()
spin_unlock(obj->resv)
There's only one thing about fences I haven't been able to map
yet properly. vmwgfx has sync_obj_flush, but as far as I can
tell it has not much to do with sync objects, but is rather a
generic 'flush before release'. Maybe one of the vmwgfx devs
could confirm whether that call is really needed there? And if
so, if there could be some other way do that, because it seems
to be the ttm_bo_wait call before that would be enough, if not
it might help more to move the flush to some other call.
PS: For ttm devs some of the code may look familiar, I don't know
if the kernel accepts I-told-you-so tag or not, but if it does
you might want to add them now. :-)
PPS: I'm aware that I still need to add a signaled op to fences
diff --git a/Documentation/DocBook/device-drivers.tmpl
b/Documentation/DocBook/device-drivers.tmpl
index 030f705..7da9637 100644
--- a/Documentation/DocBook/device-drivers.tmpl
+++ b/Documentation/DocBook/device-drivers.tmpl
@@ -129,6 +129,8 @@ X!Edrivers/base/interface.c
!Edrivers/base/dma-fence.c
!Iinclude/linux/dma-fence.h
!Iinclude/linux/dma-seqno-fence.h
+!Edrivers/base/dma-reservation.c
+!Iinclude/linux/dma-reservation.h
!Edrivers/base/dma-coherent.c
!Edrivers/base/dma-mapping.c
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 6e9f217..b26e639 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_CMA) += dma-contiguous.o
obj-y += power/
obj-$(CONFIG_HAS_DMA) += dma-mapping.o
obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
-obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o dma-fence.o
+obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o dma-fence.o dma-reservation.o
obj-$(CONFIG_ISA) += isa.o
obj-$(CONFIG_FW_LOADER)+= firmware_class.o
obj-$(CONFIG_NUMA) += node.o
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 24e88fe..3c84ead 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -25,8 +25,10 @@
#include
#include
#include
+#include
#include
#include
+#include
static inline int is_dma_buf_file(struct file *);
@@ -40,6 +42,9 @@ static int dma_buf_release(struct inode *inode, struct file
*file)
dmabuf = file->private_data;
dmabuf->ops->release(dmabuf);
+
+ if (dmabuf->resv == (struct dma_reservation_object*)&dmabuf[1])
+ dma_reservation_object_fini(dmabuf->resv);
kfree(dmabuf);
return 0;
}
@@ -94,6 +99,8 @@ struct dma_buf *dma_buf_export(void *priv, const struct
dma_buf_ops *ops,
{
struct dma_buf *dmabuf;
struct file *file;
+ size_t alloc_size = sizeof(struct dma_buf);
+ alloc_size += sizeof(struct dma_reservation_object);
if (WARN_ON(!priv || !ops
|| !ops->map_dma_buf
@@ -105,13 +112,15 @@ struct dma_buf *dma_buf_export(void *priv, const struct
dma_buf_ops *ops,
return ERR_PTR(-EINVAL);
}
- dmabuf = kzalloc(sizeof(struct dma_buf), GFP_KERNEL);
+ dmabuf = kzalloc(alloc_size, GFP_KERNEL);
if (dmabuf == NULL)
return ERR_PTR(-ENOMEM);
dmabuf->priv = priv;
dmabuf->ops = ops;
dmabuf->size = size;
+ dmabuf->resv = (struct dma_reservation_object*)&dmabuf[1];
+ dma_reservation_object_init(dmabuf->resv);
file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf, flags);
diff --git a/drivers/base/dma-reservation.c b/drivers/base/dma-reservation.c
new file mode 100644
index 000..e7cf4fa
--- /dev/null
+++ b/dr
Re: [PATCHv8 13/26] v4l: vivi: support for dmabuf importing
On Wed August 22 2012 12:56:30 Hans Verkuil wrote: > On Tue August 14 2012 17:34:43 Tomasz Stanislawski wrote: > > This patch enhances VIVI driver with a support for importing a buffer > > from DMABUF file descriptors. > > Thanks for adding DMABUF support to vivi. > > What would be great is if DMABUF support is also added to mem2mem_testdev. > It would make an excellent test case to take the vivi output, pass it > through mem2mem_testdev, and finally output the image using the gpu, all > using dmabuf. > > It's also very useful for application developers to test dmabuf support > without requiring special hardware (other than a dmabuf-enabled gpu > driver). Adding VIDIOC_EXPBUF support to vivi and mem2mem_testdev would be welcome as well for the same reasons. Regards, Hans > > Regards, > > Hans > > > > > Signed-off-by: Tomasz Stanislawski > > Signed-off-by: Kyungmin Park > > --- > > drivers/media/video/Kconfig |1 + > > drivers/media/video/vivi.c |2 +- > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig > > index 966954d..8fa81be 100644 > > --- a/drivers/media/video/Kconfig > > +++ b/drivers/media/video/Kconfig > > @@ -653,6 +653,7 @@ config VIDEO_VIVI > > depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE > > select FONT_8x16 > > select VIDEOBUF2_VMALLOC > > + select DMA_SHARED_BUFFER > > default n > > ---help--- > > Enables a virtual video driver. This device shows a color bar > > diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c > > index a6351c4..37d8fd4 100644 > > --- a/drivers/media/video/vivi.c > > +++ b/drivers/media/video/vivi.c > > @@ -1308,7 +1308,7 @@ static int __init vivi_create_instance(int inst) > > q = &dev->vb_vidq; > > memset(q, 0, sizeof(dev->vb_vidq)); > > q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; > > - q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_READ; > > + q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_READ; > > q->drv_priv = dev; > > q->buf_struct_size = sizeof(struct vivi_buffer); > > q->ops = &vivi_video_qops; > > > -- > 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 > -- 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: [PATCHv8 23/26] v4l: vb2: add support for DMA_ATTR_NO_KERNEL_MAPPING
On Tue August 14 2012 17:34:53 Tomasz Stanislawski wrote:
> From: Marek Szyprowski
Please add some more information in the commit message. I've no idea what's
going on here or why :-)
Regards,
Hans
> Signed-off-by: Marek Szyprowski
> ---
> drivers/media/video/atmel-isi.c |2 +-
> drivers/media/video/blackfin/bfin_capture.c |2 +-
> drivers/media/video/marvell-ccic/mcam-core.c |3 ++-
> drivers/media/video/mx2_camera.c |2 +-
> drivers/media/video/mx2_emmaprp.c|2 +-
> drivers/media/video/mx3_camera.c |2 +-
> drivers/media/video/s5p-fimc/fimc-core.c |2 +-
> drivers/media/video/s5p-fimc/fimc-lite.c |2 +-
> drivers/media/video/s5p-g2d/g2d.c|2 +-
> drivers/media/video/s5p-jpeg/jpeg-core.c |2 +-
> drivers/media/video/s5p-mfc/s5p_mfc.c|5 ++--
> drivers/media/video/s5p-tv/mixer_video.c |2 +-
> drivers/media/video/sh_mobile_ceu_camera.c |2 +-
> drivers/media/video/videobuf2-dma-contig.c | 33
> +++---
> drivers/staging/media/dt3155v4l/dt3155v4l.c |2 +-
> include/media/videobuf2-dma-contig.h |4 +++-
> 16 files changed, 44 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/video/atmel-isi.c b/drivers/media/video/atmel-isi.c
> index 6274a91..9fb5283 100644
> --- a/drivers/media/video/atmel-isi.c
> +++ b/drivers/media/video/atmel-isi.c
> @@ -1000,7 +1000,7 @@ static int __devinit atmel_isi_probe(struct
> platform_device *pdev)
> list_add(&isi->dma_desc[i].list, &isi->dma_desc_head);
> }
>
> - isi->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
> + isi->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev, 0);
> if (IS_ERR(isi->alloc_ctx)) {
> ret = PTR_ERR(isi->alloc_ctx);
> goto err_alloc_ctx;
> diff --git a/drivers/media/video/blackfin/bfin_capture.c
> b/drivers/media/video/blackfin/bfin_capture.c
> index 1677623..7e90b65 100644
> --- a/drivers/media/video/blackfin/bfin_capture.c
> +++ b/drivers/media/video/blackfin/bfin_capture.c
> @@ -893,7 +893,7 @@ static int __devinit bcap_probe(struct platform_device
> *pdev)
> }
> bcap_dev->ppi->priv = bcap_dev;
>
> - bcap_dev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
> + bcap_dev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev, 0);
> if (IS_ERR(bcap_dev->alloc_ctx)) {
> ret = PTR_ERR(bcap_dev->alloc_ctx);
> goto err_free_ppi;
> diff --git a/drivers/media/video/marvell-ccic/mcam-core.c
> b/drivers/media/video/marvell-ccic/mcam-core.c
> index ce2b7b4..10d4db5 100644
> --- a/drivers/media/video/marvell-ccic/mcam-core.c
> +++ b/drivers/media/video/marvell-ccic/mcam-core.c
> @@ -,7 +,8 @@ static int mcam_setup_vb2(struct mcam_camera *cam)
> #ifdef MCAM_MODE_DMA_CONTIG
> vq->ops = &mcam_vb2_ops;
> vq->mem_ops = &vb2_dma_contig_memops;
> - cam->vb_alloc_ctx = vb2_dma_contig_init_ctx(cam->dev);
> + cam->vb_alloc_ctx = vb2_dma_contig_init_ctx(cam->dev,
> + VB2_CREATE_VADDR);
> vq->io_modes = VB2_MMAP | VB2_USERPTR;
> cam->dma_setup = mcam_ctlr_dma_contig;
> cam->frame_complete = mcam_dma_contig_done;
> diff --git a/drivers/media/video/mx2_camera.c
> b/drivers/media/video/mx2_camera.c
> index 637bde8..5c30302 100644
> --- a/drivers/media/video/mx2_camera.c
> +++ b/drivers/media/video/mx2_camera.c
> @@ -1766,7 +1766,7 @@ static int __devinit mx2_camera_probe(struct
> platform_device *pdev)
> if (cpu_is_mx25())
> pcdev->soc_host.capabilities = SOCAM_HOST_CAP_STRIDE;
>
> - pcdev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
> + pcdev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev, 0);
> if (IS_ERR(pcdev->alloc_ctx)) {
> err = PTR_ERR(pcdev->alloc_ctx);
> goto eallocctx;
> diff --git a/drivers/media/video/mx2_emmaprp.c
> b/drivers/media/video/mx2_emmaprp.c
> index 2810015..23c6c42 100644
> --- a/drivers/media/video/mx2_emmaprp.c
> +++ b/drivers/media/video/mx2_emmaprp.c
> @@ -962,7 +962,7 @@ static int emmaprp_probe(struct platform_device *pdev)
>0, MEM2MEM_NAME, pcdev) < 0)
> goto rel_vdev;
>
> - pcdev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
> + pcdev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev, 0);
> if (IS_ERR(pcdev->alloc_ctx)) {
> v4l2_err(&pcdev->v4l2_dev, "Failed to alloc vb2 context\n");
> ret = PTR_ERR(pcdev->alloc_ctx);
> diff --git a/drivers/media/video/mx3_camera.c
> b/drivers/media/video/mx3_camera.c
> index f13643d..882026f 100644
> --- a/drivers/media/video/mx3_camera.c
> +++ b/drivers/media/video/mx3_camera.c
> @@ -1227,7 +1227,7 @@ static int __devinit mx3_camera_probe(struct
> platform_device *pdev)
> soc_host->v4l2_dev.dev = &pdev-
Re: [PATCHv8 19/26] v4l: vb2: add buffer exporting via dmabuf
On Tue August 14 2012 17:34:49 Tomasz Stanislawski wrote:
> This patch adds extension to videobuf2-core. It allow to export a mmap buffer
> as a file descriptor.
>
> Signed-off-by: Tomasz Stanislawski
> Signed-off-by: Kyungmin Park
> Acked-by: Laurent Pinchart
> ---
> drivers/media/video/videobuf2-core.c | 67
> ++
> include/media/videobuf2-core.h |2 +
> 2 files changed, 69 insertions(+)
>
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c
> index aed21e4..61354ec 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -1743,6 +1743,73 @@ static int __find_plane_by_offset(struct vb2_queue *q,
> unsigned long off,
> }
>
> /**
> + * vb2_expbuf() - Export a buffer as a file descriptor
> + * @q: videobuf2 queue
> + * @eb: export buffer structure passed from userspace to
> vidioc_expbuf
> + * handler in driver
> + *
> + * The return values from this function are intended to be directly returned
> + * from vidioc_expbuf handler in driver.
> + */
> +int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
> +{
> + struct vb2_buffer *vb = NULL;
> + struct vb2_plane *vb_plane;
> + unsigned int buffer, plane;
> + int ret;
> + struct dma_buf *dbuf;
> +
> + if (q->memory != V4L2_MEMORY_MMAP) {
> + dprintk(1, "Queue is not currently set up for mmap\n");
> + return -EINVAL;
> + }
> +
> + if (!q->mem_ops->get_dmabuf) {
> + dprintk(1, "Queue does not support DMA buffer exporting\n");
> + return -EINVAL;
> + }
> +
> + if (eb->flags & ~O_CLOEXEC) {
> + dprintk(1, "Queue does support only O_CLOEXEC flag\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * Find the plane corresponding to the offset passed by userspace.
> + */
> + ret = __find_plane_by_offset(q, eb->mem_offset, &buffer, &plane);
> + if (ret) {
> + dprintk(1, "invalid offset %u\n", eb->mem_offset);
> + return ret;
> + }
> +
> + vb = q->bufs[buffer];
> + vb_plane = &vb->planes[plane];
> +
> + dbuf = call_memop(q, get_dmabuf, vb_plane->mem_priv);
> + if (IS_ERR_OR_NULL(dbuf)) {
> + dprintk(1, "Failed to export buffer %d, plane %d\n",
> + buffer, plane);
> + return -EINVAL;
> + }
> +
> + ret = dma_buf_fd(dbuf, eb->flags);
> + if (ret < 0) {
> + dprintk(3, "buffer %d, plane %d failed to export (%d)\n",
> + buffer, plane, ret);
> + dma_buf_put(dbuf);
> + return ret;
> + }
> +
> + dprintk(3, "buffer %d, plane %d exported as %d descriptor\n",
> + buffer, plane, ret);
> + eb->fd = ret;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(vb2_expbuf);
> +
> +/**
> * vb2_mmap() - map video buffers into application address space
> * @q: videobuf2 queue
> * @vma: vma passed to the mmap file operation handler in the driver
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index c306fec..b034424 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -81,6 +81,7 @@ struct vb2_fileio_data;
> struct vb2_mem_ops {
> void*(*alloc)(void *alloc_ctx, unsigned long size);
> void(*put)(void *buf_priv);
> + struct dma_buf *(*get_dmabuf)(void *buf_priv);
>
> void*(*get_userptr)(void *alloc_ctx, unsigned long vaddr,
> unsigned long size, int write);
> @@ -363,6 +364,7 @@ int vb2_queue_init(struct vb2_queue *q);
> void vb2_queue_release(struct vb2_queue *q);
>
> int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b);
> +int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb);
> int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking);
>
> int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type);
>
Please add a vb2_ioctl_expbuf helper function as well!
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: [PATCHv8 18/26] v4l: add buffer exporting via dmabuf
On Tue August 14 2012 17:34:48 Tomasz Stanislawski wrote:
> This patch adds extension to V4L2 api. It allow to export a mmap buffer as
> file
> descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
> mmap and return a file descriptor on success.
>
> Signed-off-by: Tomasz Stanislawski
> Signed-off-by: Kyungmin Park
> ---
> drivers/media/video/v4l2-compat-ioctl32.c |1 +
> drivers/media/video/v4l2-dev.c|1 +
> drivers/media/video/v4l2-ioctl.c | 15 +++
> include/linux/videodev2.h | 26 ++
> include/media/v4l2-ioctl.h|2 ++
> 5 files changed, 45 insertions(+)
>
> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
> b/drivers/media/video/v4l2-compat-ioctl32.c
> index a2e0549..7689c4a 100644
> --- a/drivers/media/video/v4l2-compat-ioctl32.c
> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> @@ -971,6 +971,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int
> cmd, unsigned long arg)
> case VIDIOC_S_FBUF32:
> case VIDIOC_OVERLAY32:
> case VIDIOC_QBUF32:
> + case VIDIOC_EXPBUF:
> case VIDIOC_DQBUF32:
> case VIDIOC_STREAMON32:
> case VIDIOC_STREAMOFF32:
> diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
> index 71237f5..f6e7ea5 100644
> --- a/drivers/media/video/v4l2-dev.c
> +++ b/drivers/media/video/v4l2-dev.c
> @@ -607,6 +607,7 @@ static void determine_valid_ioctls(struct video_device
> *vdev)
> SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
> SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
> SET_VALID_IOCTL(ops, VIDIOC_QBUF, vidioc_qbuf);
> + SET_VALID_IOCTL(ops, VIDIOC_EXPBUF, vidioc_expbuf);
> SET_VALID_IOCTL(ops, VIDIOC_DQBUF, vidioc_dqbuf);
> SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
> SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
> diff --git a/drivers/media/video/v4l2-ioctl.c
> b/drivers/media/video/v4l2-ioctl.c
> index dffd3c9..c4e8c7e 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -458,6 +458,14 @@ static void v4l_print_buffer(const void *arg, bool
> write_only)
> tc->type, tc->flags, tc->frames, *(__u32
> *)tc->userbits);
> }
>
> +static void v4l_print_exportbuffer(const void *arg, bool write_only)
> +{
> + const struct v4l2_exportbuffer *p = arg;
> +
> + pr_cont("fd=%d, mem_offset=%lx, flags=%lx\n",
> + p->fd, (unsigned long)p->mem_offset, (unsigned long)p->flags);
Why the unsigned long casts?
> +}
> +
> static void v4l_print_create_buffers(const void *arg, bool write_only)
> {
> const struct v4l2_create_buffers *p = arg;
> @@ -1254,6 +1262,12 @@ static int v4l_streamoff(const struct v4l2_ioctl_ops
> *ops,
> return ops->vidioc_streamoff(file, fh, *(unsigned int *)arg);
> }
>
> +static int v4l_expbuf(const struct v4l2_ioctl_ops *ops,
> + struct file *file, void *fh, void *arg)
> +{
> + return ops->vidioc_expbuf(file, fh, arg);
> +}
Not needed...
> +
> static int v4l_g_tuner(const struct v4l2_ioctl_ops *ops,
> struct file *file, void *fh, void *arg)
> {
> @@ -1947,6 +1961,7 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
> IOCTL_INFO_STD(VIDIOC_S_FBUF, vidioc_s_fbuf, v4l_print_framebuffer,
> INFO_FL_PRIO),
> IOCTL_INFO_STD(VIDIOC_OVERLAY, vidioc_overlay, v4l_print_u32,
> INFO_FL_PRIO),
> IOCTL_INFO_FNC(VIDIOC_QBUF, v4l_qbuf, v4l_print_buffer, INFO_FL_QUEUE),
> + IOCTL_INFO_FNC(VIDIOC_EXPBUF, v4l_expbuf, v4l_print_exportbuffer,
> INFO_FL_CLEAR(v4l2_exportbuffer, flags)),
...use IOCTL_INFO_STD instead.
> IOCTL_INFO_FNC(VIDIOC_DQBUF, v4l_dqbuf, v4l_print_buffer,
> INFO_FL_QUEUE),
> IOCTL_INFO_FNC(VIDIOC_STREAMON, v4l_streamon, v4l_print_buftype,
> INFO_FL_PRIO | INFO_FL_QUEUE),
> IOCTL_INFO_FNC(VIDIOC_STREAMOFF, v4l_streamoff, v4l_print_buftype,
> INFO_FL_PRIO | INFO_FL_QUEUE),
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 7f918dc..b5d058b 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -688,6 +688,31 @@ struct v4l2_buffer {
> #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE0x0800
> #define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000
>
> +/**
> + * struct v4l2_exportbuffer - export of video buffer as DMABUF file
> descriptor
> + *
> + * @fd: file descriptor associated with DMABUF (set by driver)
> + * @mem_offset: buffer memory offset as returned by VIDIOC_QUERYBUF in
> struct
> + * v4l2_buffer::m.offset (for single-plane formats) or
> + * v4l2_plane::m.offset (for multi-planar formats)
> + * @flags: flags for newly created file, currently only O_CLOEXEC is
> + * supported, refer to manual of open syscall for more details
> + *
> + * Contains data used for exporting a video bu
Re: [PATCHv8 17/26] Documentation: media: description of DMABUF exporting in V4L2
On Tue August 14 2012 17:34:47 Tomasz Stanislawski wrote:
> This patch adds description and usage examples for exporting
> DMABUF file descriptor in V4L2.
>
> Signed-off-by: Tomasz Stanislawski
> Signed-off-by: Kyungmin Park
> CC: linux-...@vger.kernel.org
> ---
> Documentation/DocBook/media/v4l/compat.xml|3 +
> Documentation/DocBook/media/v4l/io.xml|3 +
> Documentation/DocBook/media/v4l/v4l2.xml |1 +
> Documentation/DocBook/media/v4l/vidioc-expbuf.xml | 223
> +
> 4 files changed, 230 insertions(+)
> create mode 100644 Documentation/DocBook/media/v4l/vidioc-expbuf.xml
>
> diff --git a/Documentation/DocBook/media/v4l/compat.xml
> b/Documentation/DocBook/media/v4l/compat.xml
> index ff45330..802c1ab 100644
> --- a/Documentation/DocBook/media/v4l/compat.xml
> +++ b/Documentation/DocBook/media/v4l/compat.xml
> @@ -2609,6 +2609,9 @@ ioctls.
> Importing DMABUF file descriptors as a new IO method described
> in .
>
> +
> + Exporting DMABUF files using &VIDIOC-EXPBUF; ioctl.
> +
>
>
>
> diff --git a/Documentation/DocBook/media/v4l/io.xml
> b/Documentation/DocBook/media/v4l/io.xml
> index 98253ee..c27e59b 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -488,6 +488,9 @@ buffer from userspace using a file descriptor previously
> exported for a
> different or the same device (known as the importer role), or both. This
> section describes the DMABUF importer role API in V4L2.
>
> +Refer to DMABUF exporting for
> +details about exporting a V4L2 buffers as DMABUF file descriptors.
> +
> Input and output devices support the streaming I/O method when the
> V4L2_CAP_STREAMING flag in the
> capabilities field of &v4l2-capability; returned
> by
> diff --git a/Documentation/DocBook/media/v4l/v4l2.xml
> b/Documentation/DocBook/media/v4l/v4l2.xml
> index 0292ed1..874c085 100644
> --- a/Documentation/DocBook/media/v4l/v4l2.xml
> +++ b/Documentation/DocBook/media/v4l/v4l2.xml
> @@ -568,6 +568,7 @@ and discussions on the V4L mailing list.
> &sub-overlay;
> &sub-prepare-buf;
> &sub-qbuf;
> +&sub-expbuf;
This list is sorted alphabetically, so sub-expbuf should go after sub-enumstd.
> &sub-querybuf;
> &sub-querycap;
> &sub-queryctrl;
> diff --git a/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
> b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
> new file mode 100644
> index 000..30ebf67
> --- /dev/null
> +++ b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
> @@ -0,0 +1,223 @@
> +
> +
> +
> +ioctl VIDIOC_EXPBUF
> +&manvol;
> +
> +
> +
> +VIDIOC_EXPBUF
> +Export a buffer as a DMABUF file descriptor.
> +
> +
> +
> +
> +
> + int ioctl
> + int fd
> + int request
> + struct v4l2_exportbuffer
> *argp
> +
> +
> +
> +
> +
> +Arguments
> +
> +
> +
> + fd
> +
> + &fd;
> +
> +
> +
> + request
> +
> + VIDIOC_EXPBUF
> +
> +
> +
> + argp
> +
> +
> +
> +
> +
> +
> +
> +
> +Description
> +
> +
> + Experimental
> + This is an experimental
> + interface and may change in the future.
> +
> +
> +This ioctl is an extension to the memory
> +mapping I/O method therefore it is available only for
> +V4L2_MEMORY_MMAP buffers. It can be used to export a
> +buffer as DMABUF file at any time after buffers have been allocated with the
> +&VIDIOC-REQBUFS; ioctl.
> +
> +Prior to exporting an application calls +linkend="vidioc-querybuf">VIDIOC_QUERYBUF to obtain memory offsets.
> When
> +using the multi-planar API every plane has
> +own offset.
> +
> +To export a buffer, the application fills &v4l2-exportbuffer;. The
> + mem_offset field is set to the offset obtained
> +from VIDIOC_QUERYBUF . Additional flags may be posted
> in
> +the flags field. Refer to manual for open
> syscall
Better IMHO: 'Refer to the manual for open()'
> +for details. Currently only O_CLOEXEC is guaranteed to be supported. All
> other
> +fields must be set to zero. In a case of multi-planar API, every plane is
> +exported separately using multiple VIDIOC_EXPBUF
> +calls.
> +
> + After calling VIDIOC_EXPBUF the fd
> + field will be set by a driver. This is a DMABUF file
> +descriptor. The application may pass it to other API. Refer to +linkend="dmabuf">DMABUF importing for details about importing DMABUF
> +files into V4L2 nodes. A developer is encouraged to close a DMABUF file when
> it
> +is no longer used.
Some explanation of why this is recommended would be useful.
> +
> +
> +
> +
> + Examples
> +
> +
> + Exporting a buffer.
> +
> +int buffer_export(int v4lfd, &v4l2-buf-type; bt, int index, int *dmafd)
> +{
> + &v4l2-buffer; buf;
> + &v4l2-exportbuffer; expbuf;
> +
>
Re: [Workshop-2011] V4L2 API ambiguities: workshop presentation
On Fri August 17 2012 12:35:58 Hans Verkuil wrote: > Hi all, > > I've prepared a presentation for the upcoming workshop based on my RFC and the > comments I received. > > It is available here: > > http://hverkuil.home.xs4all.nl/presentations/v4l2-workshop-2012.odp > http://hverkuil.home.xs4all.nl/presentations/v4l2-workshop-2012.pdf > > Attendees of the workshop: please review this before the workshop starts. I > want to go through this list fairly quickly (particularly slides 1-14) so we > can have more time for other topics. One additional topic: The V4L2 API has a number of experimental API elements, see: http://hverkuil.home.xs4all.nl/spec/media.html#experimental The following have been in use for a considerable amount of time I and propose to drop the experimental tag: Video Output Overlay (OSD) Interface, the section called “Video Output Overlay Interface”. V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY, enum v4l2_buf_type, Table 3.3, “enum v4l2_buf_type”. V4L2_CAP_VIDEO_OUTPUT_OVERLAY, VIDIOC_QUERYCAP ioctl, Table A.92, “Device Capabilities Flags”. VIDIOC_ENUM_FRAMESIZES and VIDIOC_ENUM_FRAMEINTERVALS ioctls. VIDIOC_G_ENC_INDEX ioctl. VIDIOC_ENCODER_CMD and VIDIOC_TRY_ENCODER_CMD ioctls. VIDIOC_DECODER_CMD and VIDIOC_TRY_DECODER_CMD ioctls. While the (TRY_)DECODER_CMD ioctls are strictly speaking new to V4L2 (appearing in 3.4) they started life as identical ioctls (although with a different name) in dvb/video.h. Other than being renamed and folded into the V4L2 specification they are quite old as well. 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: [PATCHv8 13/26] v4l: vivi: support for dmabuf importing
Hi Hans, On Wednesday 22 August 2012 12:56:30 Hans Verkuil wrote: > On Tue August 14 2012 17:34:43 Tomasz Stanislawski wrote: > > This patch enhances VIVI driver with a support for importing a buffer > > from DMABUF file descriptors. > > Thanks for adding DMABUF support to vivi. > > What would be great is if DMABUF support is also added to mem2mem_testdev. > It would make an excellent test case to take the vivi output, pass it > through mem2mem_testdev, and finally output the image using the gpu, all > using dmabuf. > > It's also very useful for application developers to test dmabuf support > without requiring special hardware (other than a dmabuf-enabled gpu > driver). One important missing feature is support for exporting GPU buffers as dmabuf file descriptors in the userspace APIs. I'm not sure where that would plug in the graphics stack, but we probably need at least a Linux-specific OpenGL extension for that. I've heard from Rob Clark that work was ongoing in that direction. I believe that https://wiki.linaro.org/OfficeofCTO/MemoryManagement?action=AttachFile&do=get&target=linux- video.pdf is also related. -- 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: [PATCHv8 13/26] v4l: vivi: support for dmabuf importing
On Tue August 14 2012 17:34:43 Tomasz Stanislawski wrote: > This patch enhances VIVI driver with a support for importing a buffer > from DMABUF file descriptors. Thanks for adding DMABUF support to vivi. What would be great is if DMABUF support is also added to mem2mem_testdev. It would make an excellent test case to take the vivi output, pass it through mem2mem_testdev, and finally output the image using the gpu, all using dmabuf. It's also very useful for application developers to test dmabuf support without requiring special hardware (other than a dmabuf-enabled gpu driver). Regards, Hans > > Signed-off-by: Tomasz Stanislawski > Signed-off-by: Kyungmin Park > --- > drivers/media/video/Kconfig |1 + > drivers/media/video/vivi.c |2 +- > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig > index 966954d..8fa81be 100644 > --- a/drivers/media/video/Kconfig > +++ b/drivers/media/video/Kconfig > @@ -653,6 +653,7 @@ config VIDEO_VIVI > depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE > select FONT_8x16 > select VIDEOBUF2_VMALLOC > + select DMA_SHARED_BUFFER > default n > ---help--- > Enables a virtual video driver. This device shows a color bar > diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c > index a6351c4..37d8fd4 100644 > --- a/drivers/media/video/vivi.c > +++ b/drivers/media/video/vivi.c > @@ -1308,7 +1308,7 @@ static int __init vivi_create_instance(int inst) > q = &dev->vb_vidq; > memset(q, 0, sizeof(dev->vb_vidq)); > q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; > - q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_READ; > + q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_READ; > q->drv_priv = dev; > q->buf_struct_size = sizeof(struct vivi_buffer); > q->ops = &vivi_video_qops; > -- 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: [PATCHv8 02/26] Documentation: media: description of DMABUF importing in V4L2
On Tue August 14 2012 17:34:32 Tomasz Stanislawski wrote:
> This patch adds description and usage examples for importing
> DMABUF file descriptor in V4L2.
>
> Signed-off-by: Tomasz Stanislawski
> Signed-off-by: Kyungmin Park
> CC: linux-...@vger.kernel.org
> ---
> Documentation/DocBook/media/v4l/compat.xml |4 +
> Documentation/DocBook/media/v4l/io.xml | 180
>
> .../DocBook/media/v4l/vidioc-create-bufs.xml |3 +-
> Documentation/DocBook/media/v4l/vidioc-qbuf.xml| 15 ++
> Documentation/DocBook/media/v4l/vidioc-reqbufs.xml | 47 ++---
> 5 files changed, 226 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/DocBook/media/v4l/compat.xml
> b/Documentation/DocBook/media/v4l/compat.xml
> index 98e8d08..ff45330 100644
> --- a/Documentation/DocBook/media/v4l/compat.xml
> +++ b/Documentation/DocBook/media/v4l/compat.xml
> @@ -2605,6 +2605,10 @@ ioctls.
>
> Support for frequency band enumeration:
> &VIDIOC-ENUM-FREQ-BANDS; ioctl.
>
> +
> + Importing DMABUF file descriptors as a new IO method described
> + in .
> +
>
>
>
> diff --git a/Documentation/DocBook/media/v4l/io.xml
> b/Documentation/DocBook/media/v4l/io.xml
> index 1885cc0..98253ee 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -472,6 +472,163 @@ rest should be evident.
>
>
>
> +
> +Streaming I/O (DMA buffer importing)
> +
> +
> + Experimental
> + This is an experimental
> + interface and may change in the future.
> +
> +
> +The DMABUF framework provides a generic mean for sharing buffers
> between
s/mean/method/
> + multiple devices. Device drivers that support DMABUF can export a DMA buffer
> +to userspace as a file descriptor (known as the exporter role), import a DMA
> +buffer from userspace using a file descriptor previously exported for a
> +different or the same device (known as the importer role), or both. This
> +section describes the DMABUF importer role API in V4L2.
> +
> +Input and output devices support the streaming I/O method when the
> +V4L2_CAP_STREAMING flag in the
> +capabilities field of &v4l2-capability; returned
> by
> +the &VIDIOC-QUERYCAP; ioctl is set. Whether importing DMA buffers through
> +DMABUF file descriptors is supported is determined by calling the
> +&VIDIOC-REQBUFS; ioctl with the memory type set to
> +V4L2_MEMORY_DMABUF.
> +
> +This I/O method is dedicated for sharing DMA buffers between V4L
> and
> +other APIs. Buffers (planes) are allocated by a driver on behalf of the
> +application, and exported to the application as file descriptors using an API
> +specific to the allocator driver. Only those file descriptor are exchanged,
> +these files and meta-information are passed in &v4l2-buffer; (or in
This sentence doesn't work well. It's unclear what is meant by 'these files'. Do
you mean 'these file descriptors'?
> +&v4l2-plane; in the multi-planar API case). The driver must be switched into
> +DMABUF I/O mode by calling the &VIDIOC-REQBUFS; with the desired buffer type.
> +No buffers (planes) are allocated beforehand, consequently they are not
> indexed
> +and cannot be queried like mapped buffers with the
> +VIDIOC_QUERYBUF ioctl.
I disagree with that. Userptr buffers can use QUERYBUF just fine. Even for the
userptr you still have to fill in the buffer index when calling QBUF.
So I see no reason why you couldn't use QUERYBUF in the DMABUF case. The only
difference is that the fd field is undefined (set to -1 perhaps?) if the bufffer
isn't queued.
QUERYBUF can be very useful for debugging, for example to see what the status
is of each buffer and how many are queued.
> +
> +
> + Initiating streaming I/O with DMABUF file descriptors
> +
> +
> +&v4l2-requestbuffers; reqbuf;
> +
> +memset (&reqbuf, 0, sizeof (reqbuf));
> +reqbuf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +reqbuf.memory = V4L2_MEMORY_DMABUF;
> +reqbuf.count = 1;
> +
> +if (ioctl (fd, &VIDIOC-REQBUFS;, &reqbuf) == -1) {
> + if (errno == EINVAL)
> + printf ("Video capturing or DMABUF streaming is not
> supported\n");
> + else
> + perror ("VIDIOC_REQBUFS");
> +
> + exit (EXIT_FAILURE);
Let's stick to the kernel coding style, so no ' ' before '(' in function calls.
Same for the other program examples below.
> +}
> +
> +
> +
> +Buffer (plane) file descriptor is passed on the fly with the
s/Buffer/The buffer/
> +&VIDIOC-QBUF; ioctl. In case of multiplanar buffers, every plane can be
'Can be', 'should be' or 'must be'? Does it ever make sense to have the same
fd for different planes? Do we have restrictions on this in the userptr case?
> +associated with a different DMABUF descriptor. Although buffers are commonly
> +cycled, applications can pass a different DMABUF descriptor at each
> +VIDIOC_QBUF call.
> +
> +
> + Q
Re: [PATCHv8 01/26] v4l: Add DMABUF as a memory type
On Tue August 14 2012 17:34:31 Tomasz Stanislawski wrote:
> From: Sumit Semwal
>
> Adds DMABUF memory type to v4l framework. Also adds the related file
> descriptor in v4l2_plane and v4l2_buffer.
>
> Signed-off-by: Tomasz Stanislawski
>[original work in the PoC for buffer sharing]
> Signed-off-by: Sumit Semwal
> Signed-off-by: Sumit Semwal
> Acked-by: Laurent Pinchart
> ---
> drivers/media/video/v4l2-compat-ioctl32.c | 18 ++
> drivers/media/video/v4l2-ioctl.c |1 +
> include/linux/videodev2.h |7 +++
> 3 files changed, 26 insertions(+)
>
> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
> b/drivers/media/video/v4l2-compat-ioctl32.c
> index 9ebd5c5..a2e0549 100644
> --- a/drivers/media/video/v4l2-compat-ioctl32.c
> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> @@ -304,6 +304,7 @@ struct v4l2_plane32 {
> union {
> __u32 mem_offset;
> compat_long_t userptr;
> + __u32 fd;
Shouldn't this be int?
> } m;
> __u32 data_offset;
> __u32 reserved[11];
> @@ -325,6 +326,7 @@ struct v4l2_buffer32 {
> __u32 offset;
> compat_long_t userptr;
> compat_caddr_t planes;
> + __u32 fd;
Ditto.
> } m;
> __u32 length;
> __u32 reserved2;
> @@ -348,6 +350,9 @@ static int get_v4l2_plane32(struct v4l2_plane *up, struct
> v4l2_plane32 *up32,
> up_pln = compat_ptr(p);
> if (put_user((unsigned long)up_pln, &up->m.userptr))
> return -EFAULT;
> + } else if (memory == V4L2_MEMORY_DMABUF) {
> + if (copy_in_user(&up->m.fd, &up32->m.fd, sizeof(int)))
> + return -EFAULT;
> } else {
> if (copy_in_user(&up->m.mem_offset, &up32->m.mem_offset,
> sizeof(__u32)))
> @@ -371,6 +376,11 @@ static int put_v4l2_plane32(struct v4l2_plane *up,
> struct v4l2_plane32 *up32,
> if (copy_in_user(&up32->m.mem_offset, &up->m.mem_offset,
> sizeof(__u32)))
> return -EFAULT;
> + /* For DMABUF, driver might've set up the fd, so copy it back. */
> + if (memory == V4L2_MEMORY_DMABUF)
> + if (copy_in_user(&up32->m.fd, &up->m.fd,
> + sizeof(int)))
> + return -EFAULT;
>
> return 0;
> }
> @@ -453,6 +463,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp,
> struct v4l2_buffer32 __user
> if (get_user(kp->m.offset, &up->m.offset))
> return -EFAULT;
> break;
> + case V4L2_MEMORY_DMABUF:
> + if (get_user(kp->m.fd, &up->m.fd))
> + return -EFAULT;
> + break;
> }
> }
>
> @@ -517,6 +531,10 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp,
> struct v4l2_buffer32 __user
> if (put_user(kp->m.offset, &up->m.offset))
> return -EFAULT;
> break;
> + case V4L2_MEMORY_DMABUF:
> + if (put_user(kp->m.fd, &up->m.fd))
> + return -EFAULT;
> + break;
> }
> }
>
> diff --git a/drivers/media/video/v4l2-ioctl.c
> b/drivers/media/video/v4l2-ioctl.c
> index 6bc47fc..dffd3c9 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -155,6 +155,7 @@ static const char *v4l2_memory_names[] = {
> [V4L2_MEMORY_MMAP]= "mmap",
> [V4L2_MEMORY_USERPTR] = "userptr",
> [V4L2_MEMORY_OVERLAY] = "overlay",
> + [V4L2_MEMORY_DMABUF] = "dmabuf",
> };
>
> #define prt_names(a, arr) a) >= 0) && ((a) < ARRAY_SIZE(arr))) ? \
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 7a147c8..7f918dc 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -186,6 +186,7 @@ enum v4l2_memory {
> V4L2_MEMORY_MMAP = 1,
> V4L2_MEMORY_USERPTR = 2,
> V4L2_MEMORY_OVERLAY = 3,
> + V4L2_MEMORY_DMABUF = 4,
> };
>
> /* see also http://vektor.theorem.ca/graphics/ycbcr/ */
> @@ -596,6 +597,8 @@ struct v4l2_requestbuffers {
> * should be passed to mmap() called on the video node)
> * @userptr: when memory is V4L2_MEMORY_USERPTR, a userspace pointer
> * pointing to this plane
> + * @fd: when memory is V4L2_MEMORY_DMABUF, a userspace
> file
> + * descriptor associated with this plane
> * @data_offset: offset in the plane to the start of data; usually 0,
> * unless the
Re: RFC: Core + Radio profile
I've added some more core profile requirements. Regards, Hans On Wed August 22 2012 11:40:25 Hans Verkuil wrote: > Hi all! > > The v4l2-compliance utility checks whether device drivers follows the V4L2 > specification. What is missing are 'profiles' detailing what device drivers > should and shouldn't implement for particular device classes. > > This has been discussed several times, but until now no attempt was made to > actually write this down. > > This RFC is my attempt to start this process by describing three profiles: > the core profile that all drivers must adhere to, a profile for a radio > receiver and a profile for a radio transmitter. > > Missing in this RFC is a description of how tuner ownership is handled if a > tuner is shared between a radio and video driver. This will be discussed > during the workshop next week. > > I am not certain where these profile descriptions should go. Should we add > this to DocBook, or describe it in Documentation/video4linux? Or perhaps > somehow add it to v4l2-compliance, since that tool effectively verifies > the profile. > > Also note that the core profile description is more strict than the spec. For > example, G/S_PRIORITY are currently optional, but I feel that all new drivers > should implement this, especially since it is very easy to add provided you > use struct v4l2_fh. Similar with respect to the control requirements which > assume you use the control framework. > > Note that these profiles are primarily meant for driver developers. The V4L2 > specification is leading for application developers. > > While writing down these profiles I noticed one thing that was very much > missing for radio devices: there is no capability bit to tell the application > whether there is an associated ALSA device or whether the audio goes through > a line-in/out. Personally I think that this should be fixed. > > Comments are welcome! > > Regards, > > Hans > > > Core Profile > > > All V4L2 device nodes must support the core profile. > > VIDIOC_QUERYCAP must be supported and must set the device_caps field. > bus_info must be a unique string and must not be empty (pending result of > the upcoming workshop). > > VIDIOC_G/S_PRIORITY must be supported. > > If you have controls then both the VIDIOC_G/S_CTRL and the > VIDIOC_G/S/TRY_EXT_CTRLS > ioctls must be supported, together with poll(), VIDIOC_DQEVENT and > VIDIOC_(UN)SUBSCRIBE_EVENT to handle control events. Use the control framework > to implement this. > > VIDIOC_LOG_STATUS is optional, but recommended for complex drivers. > > VIDIOC_DBG_G_CHIP_IDENT, VIDIOC_DBG_G/S_REGISTER are optional and are > primarily used to debug drivers. > > Video, Radio and VBI nodes are for input or output only. The only exception > are video nodes that have the V4L2_CAP_VIDEO_M2M(_MPLANE) capability set. > > Video nodes only control video, radio nodes only control radio and RDS, vbi > nodes only control VBI (raw and/or sliced). > > Streaming I/O is not supported by radio nodes. It should be possible to open device nodes more than once. Just opening a node and querying information from the driver should not change the state of the driver. The file handle that called VIDIOC_REQBUFS, VIDIOC_CREATE_BUFS, read/write or select (for reading or writing) first is the only one that can do I/O. Attempts by other file handles to call these ioctls/file operations will get an EBUSY error. This file handle remains the owner of the I/O until the file handle is closed, or until VIDIOC_REQBUFS is called with count == 0. > > Radio Receiver Profile > == > > Radio devices have a tuner and (usually) a demodulator. The audio is either > send out to a line-out connector or uses ALSA to stream the audio. > > It implements VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and > VIDIOC_ENUM_FREQ_BANDS. > > If hardware seek is supported, then VIDIOC_S_HW_FREQ_SEEK is implemented. > > It does *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO. > > If RDS_BLOCK_IO is supported, then read() and poll() are implemented as well. > > If a mute control exists and the audio is output using a line-out, then the > audio must be muted on driver load and unload. > > On driver load the frequency must be initialized to a valid frequency. > > Note: There are a few drivers that use a radio (pvrusb2) or video (ivtv) > node to stream the audio. These are legacy exceptions to the rule. > > FM Transmitter Profile > == > > FM transmitter devices have a transmitter and modulator. The audio is > transferred using ALSA or a line-in connector. > > It implements VIDIOC_G/S_MODULATOR, VIDIOC_G/S_FREQUENCY and > VIDIOC_ENUM_FREQ_BANDS. > > It does *not* implement VIDIOC_ENUM/G/S_OUTPUT or VIDIOC_ENUM/G/S_AUDOUT. > > If RDS_BLOCK_IO is supported, then write() and poll() are implemented > as well. > > On driver load the frequency must be initialized to a valid frequency. > -- > To unsu
Re: RFC: Core + Radio profile
Hi, On 08/22/2012 11:40 AM, Hans Verkuil wrote: Hi all! While writing down these profiles I noticed one thing that was very much missing for radio devices: there is no capability bit to tell the application whether there is an associated ALSA device or whether the audio goes through a line-in/out. Personally I think that this should be fixed. The question with generic tuner drivers like the tea57XX series is do we always know this ? One could argue to proper way to find this out for applications is by looking at the device topology, either through the media controller framework, or through sysfs. This is for example what xawtv currently does. We need a better library to handle this, which also hides from the user whether the media controller or sysfs is used. > Comments are welcome! Looks good! 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
RFC: Core + Radio profile
Hi all! The v4l2-compliance utility checks whether device drivers follows the V4L2 specification. What is missing are 'profiles' detailing what device drivers should and shouldn't implement for particular device classes. This has been discussed several times, but until now no attempt was made to actually write this down. This RFC is my attempt to start this process by describing three profiles: the core profile that all drivers must adhere to, a profile for a radio receiver and a profile for a radio transmitter. Missing in this RFC is a description of how tuner ownership is handled if a tuner is shared between a radio and video driver. This will be discussed during the workshop next week. I am not certain where these profile descriptions should go. Should we add this to DocBook, or describe it in Documentation/video4linux? Or perhaps somehow add it to v4l2-compliance, since that tool effectively verifies the profile. Also note that the core profile description is more strict than the spec. For example, G/S_PRIORITY are currently optional, but I feel that all new drivers should implement this, especially since it is very easy to add provided you use struct v4l2_fh. Similar with respect to the control requirements which assume you use the control framework. Note that these profiles are primarily meant for driver developers. The V4L2 specification is leading for application developers. While writing down these profiles I noticed one thing that was very much missing for radio devices: there is no capability bit to tell the application whether there is an associated ALSA device or whether the audio goes through a line-in/out. Personally I think that this should be fixed. Comments are welcome! Regards, Hans Core Profile All V4L2 device nodes must support the core profile. VIDIOC_QUERYCAP must be supported and must set the device_caps field. bus_info must be a unique string and must not be empty (pending result of the upcoming workshop). VIDIOC_G/S_PRIORITY must be supported. If you have controls then both the VIDIOC_G/S_CTRL and the VIDIOC_G/S/TRY_EXT_CTRLS ioctls must be supported, together with poll(), VIDIOC_DQEVENT and VIDIOC_(UN)SUBSCRIBE_EVENT to handle control events. Use the control framework to implement this. VIDIOC_LOG_STATUS is optional, but recommended for complex drivers. VIDIOC_DBG_G_CHIP_IDENT, VIDIOC_DBG_G/S_REGISTER are optional and are primarily used to debug drivers. Video, Radio and VBI nodes are for input or output only. The only exception are video nodes that have the V4L2_CAP_VIDEO_M2M(_MPLANE) capability set. Video nodes only control video, radio nodes only control radio and RDS, vbi nodes only control VBI (raw and/or sliced). Streaming I/O is not supported by radio nodes. Radio Receiver Profile == Radio devices have a tuner and (usually) a demodulator. The audio is either send out to a line-out connector or uses ALSA to stream the audio. It implements VIDIOC_G/S_TUNER, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS. If hardware seek is supported, then VIDIOC_S_HW_FREQ_SEEK is implemented. It does *not* implement VIDIOC_ENUM/G/S_INPUT or VIDIOC_ENUM/G/S_AUDIO. If RDS_BLOCK_IO is supported, then read() and poll() are implemented as well. If a mute control exists and the audio is output using a line-out, then the audio must be muted on driver load and unload. On driver load the frequency must be initialized to a valid frequency. Note: There are a few drivers that use a radio (pvrusb2) or video (ivtv) node to stream the audio. These are legacy exceptions to the rule. FM Transmitter Profile == FM transmitter devices have a transmitter and modulator. The audio is transferred using ALSA or a line-in connector. It implements VIDIOC_G/S_MODULATOR, VIDIOC_G/S_FREQUENCY and VIDIOC_ENUM_FREQ_BANDS. It does *not* implement VIDIOC_ENUM/G/S_OUTPUT or VIDIOC_ENUM/G/S_AUDOUT. If RDS_BLOCK_IO is supported, then write() and poll() are implemented as well. On driver load the frequency must be initialized to a valid frequency. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] davinci: vpfe: Add documentation
Hi Sakari,
On Thursday 16 August 2012 09:53 PM, Sakari Ailus wrote:
> Hi Manju,
>
> On Thu, Aug 09, 2012 at 09:13:52AM +0530, Manjunath Hadli wrote:
>> Hi Sakari,
>>
>> Thank you for the comments.
>
> Thanks for the graphs!
>
>> On Thursday 02 August 2012 05:37 AM, Sakari Ailus wrote:
>>> Hi Manju,
>>>
>>> Thanks for the patch.
>>>
>>> Please make sure these patches reach linux-media next time. If they do
>>> not,
>>> it severely limits the number of potential reviewers. I don't know
>>> why, but
>>> the original patch isn't on linux-media even if the list was cc'd.
>>>
>>> Dropping linux-kernel from cc.
>>>
>>> Manjunath Hadli wrote:
Add documentation on the Davinci VPFE driver. Document the subdevs,
and private IOTCLs the driver implements
Signed-off-by: Manjunath Hadli
Signed-off-by: Lad, Prabhakar
---
Documentation/video4linux/davinci-vpfe-mc.txt | 263
+
1 files changed, 263 insertions(+), 0 deletions(-)
create mode 100644 Documentation/video4linux/davinci-vpfe-mc.txt
diff --git a/Documentation/video4linux/davinci-vpfe-mc.txt
b/Documentation/video4linux/davinci-vpfe-mc.txt
new file mode 100644
index 000..968194f
--- /dev/null
+++ b/Documentation/video4linux/davinci-vpfe-mc.txt
@@ -0,0 +1,263 @@
+Davinci Video processing Front End (VPFE) driver
+
+Copyright (C) 2012 Texas Instruments Inc
+
+Contacts: Manjunath Hadli
+
+Introduction
+
+
+This file documents the Texas Instruments Davinci Video processing
Front End
+(VPFE) driver located under drivers/media/video/davinci. The
original driver
+exists for Davinci VPFE, which is now being changed to Media Controller
+Framework.
+
+Currently the driver has been successfully used on the following
version of Davinci:
+
+DM365/DM368
+
+The driver implements V4L2, Media controller and v4l2_subdev
interfaces.
+Sensor, lens and flash drivers using the v4l2_subdev interface in
the kernel
+are supported.
+
+
+Split to subdevs
+
+
+The Davinic VPFE is split into V4L2 subdevs, each of the blocks
inside the VPFE
+having one subdev to represent it. Each of the subdevs provide a
V4L2 subdev
+interface to userspace.
+
+DAVINCI CCDC
+DAVINCI PREVIEWER
+DAVINCI RESIZER
+DAVINCI AEW
+DAVINCI AF
+
+Each possible link in the VPFE is modeled by a link in the Media
controller
+interface. For an example program see [1].
+
+
+Private IOCTLs
+==
+
+The Davinci Video processing Front End (VPFE) driver supports
standard V4L2
+IOCTLs and controls where possible and practical. Much of the
functions provided
+by the VPFE, however, does not fall under the standard IOCTLs.
+
+In general, there is a private ioctl for configuring each of the blocks
+containing hardware-dependent functions.
+
+The following private IOCTLs are supported:
+
+1: IOCTL: PREV_S_PARAM/PREV_G_PARAM
+Description:
+Sets/Gets the parameters required by the previewer module
+Parameter:
+/**
+ * struct prev_module_param- structure to configure preview modules
+ * @version: Version of the preview module
+ * @len: Length of the module config structure
+ * @module_id: Module id
+ * @param: pointer to module config parameter.
+ */
+struct prev_module_param {
+char version[IMP_MAX_NAME_SIZE];
+unsigned short len;
+unsigned short module_id;
+void *param;
+};
>>>
>>> In addition to what Laurent commented on this, could the version
>>> information be passed in struct media_entity_desc instead?
>> I plan to leave out the version.
>>>
>>> As a general comment, it's a bad idea to design an API that allows
>>> passing
>>> blobs, especially when the expected size of the blobs isn't known. That
>>> really equals to asking for trouble.
>>>
>>> That said, I know this is an area where complete documentation is acarce,
>>> but I think that at least the memory layout of the current blob pointers
>>> should be visible in the struct definitions whenever possible. See
>>> e.g. the
>>> OMAP 3 ISP driver.
>> I have proposed using a union of structures instead of the void blob.
>> I also saw the OMAP implementation, and they are pointers (but not void).
>> To me the union approach looks better as it keeps the architecture
>> intact and does not necessitate an
>> explicit copy_from_user. Which of these ways do you suggest?
>
> I would suggest to use the approach taken in the OMAP 3 ISP driver as it has
> one obvious advantage over the union of pointers: it makes it possible to
> perform ato
Re: [RFC API] Renumber subdev ioctls
On Tue August 21 2012 12:44:15 Sakari Ailus wrote: > Hi Hans, > > On Tue, Aug 21, 2012 at 08:39:53AM +0200, Hans Verkuil wrote: > > On Mon August 20 2012 22:46:04 Sakari Ailus wrote: > > > Hi Mauro and Hans, > > > > > > On Mon, Aug 20, 2012 at 04:05:03PM -0300, Mauro Carvalho Chehab wrote: > > > > Em 20-08-2012 05:30, Hans Verkuil escreveu: > > > > > Hi all! > > > > > > > > > > Recently I had to add two new ioctls for the subdev API > > > > > (include/linux/v4l2-subdev.h) > > > > > and I noticed that the numbering of the ioctls was somewhat random. > > > > > > > > > > In most cases the ioctl number was the same as the V4L2 API > > > > > counterpart, but for > > > > > subdev-specific ioctls no rule exists. > > > > > > > > > > There are a few problems with this: because of the lack of rules > > > > > there is a chance > > > > > that in the future a subdev ioctl may end up to be identical to an > > > > > existing V4L2 > > > > > ioctl. Also, because the numbering isn't nicely increasing it makes > > > > > it hard to create > > > > > a lookup table as was done for the V4L2 ioctls. Well, you could do > > > > > it, but it would > > > > > be a very sparse array, wasting a lot of memory. > > > > > > > > > > Lookup tables have proven to be very useful, so we might want to > > > > > introduce them for > > > > > the subdev core code as well in the future. > > > > > > > > > > Since the subdev API is still marked experimental, I propose to > > > > > renumber the ioctls > > > > > and use the letter 'v' instead of 'V'. 'v' was used for V4L1, and so > > > > > it is now > > > > > available for reuse. > > > > > > > > 'v' is already used (mainly by fs): > > > > > > > > 'v' 00-1F linux/ext2_fs.h conflict! > > > > 'v' 00-1F linux/fs.h conflict! > > > > 'v' 00-0F linux/sonypi.h conflict! > > > > 'v' C0-FF linux/meye.hconflict! > > > > > > > > Reusing the ioctl numbering is a bad thing, as tracing code like strace > > > > will likely > > > > say that a different type of ioctl was called. > > > > > > > > (Yeah, unfortunately, this end by merging with duplicated stuff :< ) > > > > > > > > Also, I don't like the idea of deprecating it just because of that: > > > > interfaces are > > > > supposed to be stable. > > > > > > > > It should be noticed that there are very few ioctls there. So, > > > > using a lookup table is overkill. > > > > > > > > IMO, the better is to sort the ioctl's there at the header file, in > > > > order to > > > > avoid ioctl duplicaton. > > > > > > Many of the V4L2 IOCTLs are being used on subdevs, too, to the extent that > > > subdev_do_ioctl() in drivers/media/v4l2-core/v4l2-subdev.c has a switch > > > statement with over 20 cases. We'll get rid of two once the old crop > > > IOCTLs > > > are removed but we've still got over 20, and the number is likely to grow > > > in > > > the future. Still it's just a fraction of what V4L2 has. > > > > > > We decided to use 'V' also for subdev IOCTLs for a reason I no longer > > > remember. It's true there can be clashes with regular V4L2 IOCTLs in terms > > > of IOCTL codes if the size of the argument struct matches. One of the > > > reasons to use 'V' might have been that then some of the IOCTLs on a > > > device > > > would have different type (the letter in question) which wasn't considered > > > pretty. 'V' is for V4L2 after all, and V4L2 subdev interface is part of > > > the > > > V4L2. > > > > > > The numbering is based on using V4L2 IOCTLs as such if they were > > > applicable > > > to subdevs as such (controls) in which case they're defined in > > > videodev2.h, > > > and if there was even a loosely corresponding IOCTL in V4L2 then use the > > > same number (e.g. formats vs. media bus pixel codes) and otherwise > > > something > > > else. The "something else" case hasn't happened yet. > > > > > > It might have made sense to use a different type for the IOCTLs that > > > aren't > > > V4L2 IOCTLs (i.e. are subdev IOCTLs) for clarity but it's quite late for > > > such a change. However if we think we definitely should do it then it > > > should > > > be done now or not at all... > > > > > > If we want to just improve the efficiency of the switch statement in > > > subdev_do_ioctl() we could divide the IOCTLs based on e.g. a few last bits > > > of the IOCTL number into buckets. > > > > It's not so much the switch efficiency. In practice there will be no > > measurable > > speed difference. But a lookup table allows one to easily look up > > information > > about the ioctl. > > > > But the main goal would be to guarantee that subdev ioctls and V4L2 ioctls > > will never clash, since both types of ioctls can be used with a subdev node. > > It's indeed possible to have clashes between the IOCTL codes but that does > not matter so much: all IOCTLs related to buffers belong to V4L2 and > anything related to pads belongs to subdevs only. > > As long as a little c
[GIT PATCHES FOR v3.6] Samsung media driver fixes
Hi Mauro, The following changes since commit f9cd49033b349b8be3bb1f01b39eed837853d880: Merge tag 'v3.6-rc1' into staging/for_v3.6 (2012-08-03 22:41:33 -0300) are available in the git repository at: git://git.infradead.org/users/kmpark/linux-samsung v4l-fixes for you to fetch changes up to 0e59db054e30658c6955d6e27b0a252cef9bfafc: s5p-mfc: Fix second memory bank alignment (2012-08-16 19:12:19 +0200) Kamil Debski (1): s5p-mfc: Fix second memory bank alignment Sylwester Nawrocki (7): s5p-fimc: Enable FIMC-LITE driver only for SOC_EXYNOS4x12 s5p-fimc: Don't allocate fimc-lite video device structure dynamically s5p-fimc: Don't allocate fimc-capture video device dynamically s5p-fimc: Don't allocate fimc-m2m video device dynamically m5mols: Add missing free_irq() on error path m5mols: Fix cast warnings from m5mols_[set/get]_ctrl_mode s5p-fimc: Fix setup of initial links to FIMC entities drivers/media/video/m5mols/m5mols.h | 4 +-- drivers/media/video/m5mols/m5mols_controls.c | 4 +-- drivers/media/video/m5mols/m5mols_core.c | 4 ++- drivers/media/video/s5p-fimc/Kconfig | 2 +- drivers/media/video/s5p-fimc/fimc-capture.c | 31 +++--- drivers/media/video/s5p-fimc/fimc-core.h | 4 +-- drivers/media/video/s5p-fimc/fimc-lite-reg.c | 2 +- drivers/media/video/s5p-fimc/fimc-lite.c | 42 ++--- drivers/media/video/s5p-fimc/fimc-lite.h | 2 +- drivers/media/video/s5p-fimc/fimc-m2m.c | 40 --- drivers/media/video/s5p-fimc/fimc-mdevice.c | 9 -- drivers/media/video/s5p-fimc/fimc-mdevice.h | 6 ++-- drivers/media/video/s5p-fimc/fimc-reg.c | 6 ++-- drivers/media/video/s5p-mfc/s5p_mfc_ctrl.c | 2 +- 14 files changed, 65 insertions(+), 93 deletions(-) --- Thanks, Sylwester -- 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 04/10] staging: media: go7007: Add IDENT for TW2802/2804
On Wed August 22 2012 12:45:13 Volokh Konstantin wrote:
> Signed-off-by: Volokh Konstantin
> ---
> include/media/v4l2-chip-ident.h |4
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/include/media/v4l2-chip-ident.h b/include/media/v4l2-chip-ident.h
> index 7395c81..5395495 100644
> --- a/include/media/v4l2-chip-ident.h
> +++ b/include/media/v4l2-chip-ident.h
> @@ -113,6 +113,10 @@ enum {
> /* module vp27smpx: just ident 2700 */
> V4L2_IDENT_VP27SMPX = 2700,
>
> + /* module wis-tw2804: 2802/2804 */
> + V4L2_IDENT_TW2802 = 2802,
> + V4L2_IDENT_TW2804 = 2804,
> +
> /* module vpx3220: reserved range: 3210-3229 */
> V4L2_IDENT_VPX3214C = 3214,
> V4L2_IDENT_VPX3216B = 3216,
>
There is no need to add this, unless the g/s_register or g_chip_ident ops
are also implemented.
In this case I'd just drop this patch.
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: [PATCH 01/10] staging: media: go7007: Some additional code for TW2804 driver functionality
Hi Volokh!
Thanks for working on this!
I have some quick review notes below.
On Wed August 22 2012 12:45:10 Volokh Konstantin wrote:
> - using new v4l2 framework controls
> - function for reading volatile controls via i2c bus
> - separate V4L2_CID_ ctrls into each V4L2 calls
>
> Signed-off-by: Volokh Konstantin
> ---
> drivers/staging/media/go7007/wis-tw2804.c | 248
> +
> 1 files changed, 248 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/staging/media/go7007/wis-tw2804.c
> b/drivers/staging/media/go7007/wis-tw2804.c
> index 9134f03..05851d3 100644
> --- a/drivers/staging/media/go7007/wis-tw2804.c
> +++ b/drivers/staging/media/go7007/wis-tw2804.c
> @@ -21,10 +21,18 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
> +#include
>
> #include "wis-i2c.h"
>
> struct wis_tw2804 {
> + struct v4l2_subdev sd;
> + struct v4l2_ctrl_handler hdl;
> + u8 channel:2;
> + u8 input:1;
> int channel;
> int norm;
> int brightness;
> @@ -116,9 +124,246 @@ static int write_regs(struct i2c_client *client, u8
> *regs, int channel)
> if (i2c_smbus_write_byte_data(client,
> regs[i] | (channel << 6), regs[i + 1]) < 0)
> return -1;
> +static s32 read_reg(struct i2c_client *client, u8 reg, u8 channel)
> +{
> + return i2c_smbus_read_byte_data(client, (reg) | (channel << 6));
> +}
> +
> +inline struct wis_tw2804 *to_state(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct wis_tw2804, sd);
> +}
> +
> +inline struct wis_tw2804 *to_state_from_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + return container_of(ctrl->handler, struct wis_tw2804, hdl);
> +}
> +
> +static int tw2804_log_status(struct v4l2_subdev *sd)
> +{
> + struct wis_tw2804 *state = to_state(sd);
> + v4l2_info(sd, "Standard: %s\n",
> + state->norm == V4L2_STD_NTSC ? "NTSC" :
> + state->norm == V4L2_STD_PAL ? "PAL" : "unknown");
> + v4l2_info(sd, "Channel: %d\n", state->channel);
> + v4l2_info(sd, "Input: %d\n", state->input);
> + v4l2_ctrl_handler_log_status(&state->hdl, sd->name);
> + return 0;
> +}
> +
> +static s32 get_ctrl_addr(int ctrl)
> +{
> + switch (ctrl) {
> + case V4L2_CID_BRIGHTNESS:
> + return 0x12;
> + case V4L2_CID_CONTRAST:
> + return 0x11;
> + case V4L2_CID_SATURATION:
> + return 0x10;
> + case V4L2_CID_HUE:
> + return 0x0f;
> + case V4L2_CID_AUTOGAIN:
> + return 0x02;
> + case V4L2_CID_COLOR_KILLER:
> + return 0x14;
> + case V4L2_CID_GAIN:
> + return 0x3c;
> + case V4L2_CID_CHROMA_GAIN:
> + return 0x3d;
> + case V4L2_CID_RED_BALANCE:
> + return 0x3f;
> + case V4L2_CID_BLUE_BALANCE:
> + return 0x3e;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int tw2804_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct v4l2_subdev *sd = &to_state_from_ctrl(ctrl)->sd;
> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> + s32 addr = get_ctrl_addr(ctrl->id);
> +
> + if (addr == -EINVAL)
> + return -EINVAL;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_GAIN:
> + case V4L2_CID_CHROMA_GAIN:
> + case V4L2_CID_RED_BALANCE:
> + case V4L2_CID_BLUE_BALANCE:
> + ctrl->cur.val = read_reg(client, addr, 0);
That should be ctrl->val, not ctrl->cur.val.
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int tw2804_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct wis_tw2804 *state = to_state_from_ctrl(ctrl);
> + struct v4l2_subdev *sd = &state->sd;
> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> + s32 reg = ctrl->val;
> + s32 addr = get_ctrl_addr(ctrl->id);
> +
> + if (addr == -EINVAL)
> + return -EINVAL;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_AUTOGAIN:
> + reg = read_reg(client, addr, state->channel);
> + if (reg > 0) {
> + if (ctrl->val == 0)
> + reg &= ~(1<<7);
> + else
> + reg |= 1<<7;
> + } else
> + return reg;
> + break;
> + case V4L2_CID_COLOR_KILLER:
> + reg = read_reg(client, addr, state->channel);
> + if (reg > 0)
> + reg = (reg & ~(0x03)) | (ctrl->val == 0 ? 0x02 : 0x03);
> + else
> + return reg;
> + break;
> + default:
> + break;
> + }
> +
> + reg = reg > 255 ? 255 : (reg < 0 ? 0 : reg);
> + reg = write_reg(client, addr, (u8)reg,
> + ctrl->id == V4L2_CID_GAIN ||
> + ctrl->id == V4L2_CID_CHROMA_GAIN ||
> +
Re: How to add support for the em2765 webcam Speedlink VAD Laplace to the kernel ?
Am 21.08.2012 19:29, schrieb Mauro Carvalho Chehab:
> Hmm... before reading the rest of this email... I found some doc saying that
> em2765 is UVC compliant. Doesn't the uvcdriver work with this device?
Yeah, I stumbled over that, too. :D
But this device is definitely not UVC compliant. Take a look at the
lsusb output.
Maybe they are using a different firmware or something like that, but I
have no idea why the hell they should make a UVC compliant device
non-UVC-compliant...
Another notable difference to the devices we've seen so far is the
em25xx-style EEPROM. Maybe there is a connection.
Btw, do we know any em25xx devices ???
I have to admit that I didn't open my device to check which chip it
uses. This information comes from the guy who created the inital wiki
page. But I think we can trust this information, because he provided the
full chip label content and also photos. And according to the chip id,
it's none of the devices we already know.
I wouldn't hesitate to open my device to verify it, if the chance to
damage the device wouldn't be that high...
>
> Em 21-08-2012 13:04, Frank Schäfer escreveu:
>> Am 21.08.2012 14:32, schrieb Mauro Carvalho Chehab:
>>> Em 21-08-2012 08:35, Frank Schäfer escreveu:
Am 20.08.2012 21:21, schrieb Mauro Carvalho Chehab:
> Em 20-08-2012 10:02, Hans de Goede escreveu:
>> Hi,
>>
>> On 08/20/2012 01:41 PM, Frank Schäfer wrote:
>>> Hi,
>>>
>>> after a break of 2 1/2 kernel releases (sorry, I was busy with another
>>> project), I would like to bring up again the question how to add support
>>> for this device to the kernel.
>>> See
>>> http://www.mail-archive.com/linux-media@vger.kernel.org/msg44417.html
>>> ("Move em27xx/em28xx webcams to a gspca subdriver ?") for the previous
>>> discussion.
>>>
>>> Current status is, that I've reverse-engineered the Windows driver and
>>> written a new gspca-subdriver for testing, which is feature complete and
>>> working stable (will send a patch shortly !).
>>>
>>> The device uses an em2765-bridge, so my first idea was of course to
>>> modify/extend the em28xx-driver.
>>> But during the reverse-engineering-process, it turned out that writing a
>>> new gspca-subdriver was much easier than modifying the em28xx-driver.
>>>
>>> The device has the following special characteristics:
>>> - supports only bulk transfers (em28xx driver supports ISOC only)
> Em28xx driver supports both isoc and bulk transfers, as bulk is
> required by DVB.
Hmm... are you 100% sure ? Must have been added recently then...
I did a quick check of the current code, but can't find anything. Could
you please give me a pointer to the code parts ?
Btw, if I'm not understanding the code wrong, em28xx_usb_probe() still
seems to return -ENODEV if no isoc-in endpoint is found, so bulk-ep-only
devices should not work...
>>> Perhaps I'm tricked by tm6000 code... both codes are similar.
>>> There are a few differences with regards to isoc/bulk hanlding
>>> there.
>>>
>>> - uses "proprietary" read/write procedures for the sensor
> Sure about that? It doesn't use I2C?
According to the datasheet of the OV2640 it should be SCCB.
>>> SCCB is a variant of I2C.
>> I'm not sure if Omnivison would admit that. :D
> Maybe there are trademarks envolved ;)
Yes, it's a funny game. ;)
Anyway, I'm referring to how communication works on the USB level.
Take a look at http://linuxtv.org/wiki/index.php/VAD_Laplace section
"Reverse Engineering (evaluation of USB-logs)" to see how it is working.
>>> Ok. From your logs, it seems that em2765 uses a different bus for
>>> sensor communication. It is not unusual to have more than one bus on
>>> some modern devices (cx231xx has 3 I2C buses, plus one extra bus that
>>> can be switched).
>> Yes, but I'm wondering why.
>> Shouldn't it be possible to connect both (sensor and eeprom) to the same
>> bus ? Or are i2c and sccb devices incompatible ?
>> We've seen so many em28xx devices (some of them beeing much more
>> complex) and none of them has used two busses so far.
>> Strange...
> Most devices nowadays have 2 i2c buses. TV boards typically uses an
> I2C switch on them. The rationale is to avoid receiving signal
> interference. Some newer em28xx devices have more than one bus, although,
> on TV chipsets, this is controlled via one register, that commands
> bus switch:
>
> /* em28xx I2C Clock Register (0x06) */
> #define EM2874_I2C_SECONDARY_BUS_SELECT 0x04 /* em2874 has two i2c
> busses */
>
> On those devices (em2874/em2875, afaikt), hardware manufacturers put
> the TV tuner and demod at the second bus, keeping the first bus
> for remote controller and eeprom.
Ok, I didn't notice that there a two i2c busses.
I wouldn't wonder if the em2765 doesn't support this bus switch and
that's why different USB reads/writes are used instead.
Shouldn't be too difficult to find
