Re: [REVIEWv2 PATCH 02/13] vb2: fix handling of data_offset and v4l2_plane.reserved[]
On 04/10/2014 02:46 AM, Pawel Osciak wrote:
Looks good to me, just a small nit below.
On Mon, Apr 7, 2014 at 10:11 PM, Hans Verkuil hverk...@xs4all.nl wrote:
From: Hans Verkuil hans.verk...@cisco.com
The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
array in struct v4l2_plane would be non-zero, causing v4l2-compliance
errors.
More serious is the fact that data_offset was not handled correctly:
- for capture devices it was never zeroed, which meant that it was
uninitialized. Unless the driver sets it it was a completely random
number. With the memset above this is now fixed.
- __qbuf_dmabuf had a completely incorrect length check that included
data_offset.
- in __fill_vb2_buffer in the DMABUF case the data_offset field was
unconditionally copied from v4l2_buffer to v4l2_plane when this
should only happen in the output case.
- in the single-planar case data_offset was never correctly set to 0.
The single-planar API doesn't support data_offset, so setting it
to 0 is the right thing to do. This too is now solved by the memset.
All these issues were found with v4l2-compliance.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Acked-by: Pawel Osciak pa...@osciak.com
---
drivers/media/v4l2-core/videobuf2-core.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index f9059bb..596998e 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1169,8 +1169,6 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
const struct v4l2_buffer *b
b-m.planes[plane].m.fd;
v4l2_planes[plane].length =
b-m.planes[plane].length;
- v4l2_planes[plane].data_offset =
- b-m.planes[plane].data_offset;
}
}
} else {
@@ -1180,10 +1178,8 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
const struct v4l2_buffer *b
* In videobuf we use our internal V4l2_planes struct for
* single-planar buffers as well, for simplicity.
*/
- if (V4L2_TYPE_IS_OUTPUT(b-type)) {
+ if (V4L2_TYPE_IS_OUTPUT(b-type))
v4l2_planes[0].bytesused = b-bytesused;
- v4l2_planes[0].data_offset = 0;
- }
if (b-memory == V4L2_MEMORY_USERPTR) {
v4l2_planes[0].m.userptr = b-m.userptr;
@@ -1193,9 +1189,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb,
const struct v4l2_buffer *b
if (b-memory == V4L2_MEMORY_DMABUF) {
v4l2_planes[0].m.fd = b-m.fd;
v4l2_planes[0].length = b-length;
- v4l2_planes[0].data_offset = 0;
}
-
}
/* Zero flags that the vb2 core handles */
@@ -1238,6 +1232,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
int write = !V4L2_TYPE_IS_OUTPUT(q-type);
bool reacquired = vb-planes[0].mem_priv == NULL;
+ memset(planes, 0, sizeof(planes[0]) * vb-num_planes);
memset(planes, 0, sizeof(planes));
Should we really do this? This array is for 8 planes, whereas today we do not
have more than 2 planes worst case. So zeroing all planes for every qbuf seems
excessive to me.
I fact, looking at the code only the actual planes are copied back anyway:
/*
* Now that everything is in order, copy relevant information
* provided by userspace.
*/
for (plane = 0; plane vb-num_planes; ++plane)
vb-v4l2_planes[plane] = planes[plane];
so memsetting more than the actual number of planes is pointless.
Unless I am missing something?
Regards,
Hans
/* Copy relevant information provided by the userspace */
__fill_vb2_buffer(vb, b, planes);
@@ -1357,6 +1352,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
int write = !V4L2_TYPE_IS_OUTPUT(q-type);
bool reacquired = vb-planes[0].mem_priv == NULL;
+ memset(planes, 0, sizeof(planes[0]) * vb-num_planes);
memset(planes, 0, sizeof(planes));
/* Copy relevant information provided by the userspace */
__fill_vb2_buffer(vb, b, planes);
@@ -1374,8 +1370,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
if (planes[plane].length == 0)
planes[plane].length = dbuf-size;
- if (planes[plane].length planes[plane].data_offset +
-
[PATCH v2 6/8] s5p-jpeg: Fix sysmmu page fault
This patch fixes jpeg sysmmu page fault on Exynos4x12 SoCs.
During encoding Exynos4x12 SoCs access wider memory area
than it results from Image_x and Image_y values written to
the JPEG_IMAGE_SIZE register. In order to avoid sysmmu page
fault apply proper output buffer size alignment.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 46 +--
1 file changed, 43 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 541f03e..393f3fd 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1106,6 +1106,32 @@ static int s5p_jpeg_try_fmt_vid_out(struct file *file,
void *priv,
return vidioc_try_fmt(f, fmt, ctx, FMT_TYPE_OUTPUT);
}
+static int exynos4_jpeg_get_output_buffer_size(struct s5p_jpeg_ctx *ctx,
+ struct v4l2_format *f,
+ int fmt_depth)
+{
+ struct v4l2_pix_format *pix = f-fmt.pix;
+ u32 pix_fmt = f-fmt.pix.pixelformat;
+ int w = pix-width, h = pix-height, wh_align;
+
+ if (pix_fmt == V4L2_PIX_FMT_RGB32 ||
+ pix_fmt == V4L2_PIX_FMT_NV24 ||
+ pix_fmt == V4L2_PIX_FMT_NV42 ||
+ pix_fmt == V4L2_PIX_FMT_NV12 ||
+ pix_fmt == V4L2_PIX_FMT_NV21 ||
+ pix_fmt == V4L2_PIX_FMT_YUV420)
+ wh_align = 4;
+ else
+ wh_align = 1;
+
+ jpeg_bound_align_image(w, S5P_JPEG_MIN_WIDTH,
+ S5P_JPEG_MAX_WIDTH, wh_align,
+ h, S5P_JPEG_MIN_HEIGHT,
+ S5P_JPEG_MAX_HEIGHT, wh_align);
+
+ return w * h * fmt_depth 3;
+}
+
static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, struct v4l2_format *f)
{
struct vb2_queue *vq;
@@ -1132,10 +1158,24 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct,
struct v4l2_format *f)
q_data-fmt = s5p_jpeg_find_format(ct, pix-pixelformat, f_type);
q_data-w = pix-width;
q_data-h = pix-height;
- if (q_data-fmt-fourcc != V4L2_PIX_FMT_JPEG)
- q_data-size = q_data-w * q_data-h * q_data-fmt-depth 3;
- else
+ if (q_data-fmt-fourcc != V4L2_PIX_FMT_JPEG) {
+ /*
+* During encoding Exynos4x12 SoCs access wider memory area
+* than it results from Image_x and Image_y values written to
+* the JPEG_IMAGE_SIZE register. In order to avoid sysmmu
+* page fault calculate proper buffer size in such a case.
+*/
+ if (ct-jpeg-variant-version == SJPEG_EXYNOS4
+ f_type == FMT_TYPE_OUTPUT ct-mode == S5P_JPEG_ENCODE)
+ q_data-size = exynos4_jpeg_get_output_buffer_size(ct,
+ f,
+ q_data-fmt-depth);
+ else
+ q_data-size = q_data-w * q_data-h *
+ q_data-fmt-depth 3;
+ } else {
q_data-size = pix-sizeimage;
+ }
if (f_type == FMT_TYPE_OUTPUT) {
ctrl_subs = v4l2_ctrl_find(ct-ctrl_handler,
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/8] s5p-jpeg: g_selection callback should always succeed
Remove erroneous guard preventing successful execution of g_selection callback in case the driver variant is different from SJPEG_S5P. Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/s5p-jpeg/jpeg-core.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c index 04260c2..541f03e 100644 --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c @@ -1177,8 +1177,7 @@ static int s5p_jpeg_g_selection(struct file *file, void *priv, struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv); if (s-type != V4L2_BUF_TYPE_VIDEO_OUTPUT - s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE - ctx-jpeg-variant-version != SJPEG_S5P) + s-type != V4L2_BUF_TYPE_VIDEO_CAPTURE) return -EINVAL; /* For JPEG blob active == default == bounds */ -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/8] s5p_jpeg: Prevent JPEG 4:2:0 YUV 4:2:0 decompression
Prevent decompression of a JPEG 4:2:0 with odd width to
the YUV 4:2:0 compliant formats for Exynos4x12 SoCs and
adjust capture format to RGB565 in such a case. This is
required because the configuration would produce a raw
image with broken luma component.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 25 ++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 393f3fd..24545bd 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1064,15 +1064,17 @@ static int s5p_jpeg_try_fmt_vid_cap(struct file *file,
void *priv,
return -EINVAL;
}
+ if ((ctx-jpeg-variant-version != SJPEG_EXYNOS4) ||
+ (ctx-mode != S5P_JPEG_DECODE))
+ goto exit;
+
/*
* The exynos4x12 device requires resulting YUV image
* subsampling not to be lower than the input jpeg subsampling.
* If this requirement is not met then downgrade the requested
* capture format to the one with subsampling equal to the input jpeg.
*/
- if ((ctx-jpeg-variant-version == SJPEG_EXYNOS4)
- (ctx-mode == S5P_JPEG_DECODE)
- (fmt-flags SJPEG_FMT_NON_RGB)
+ if ((fmt-flags SJPEG_FMT_NON_RGB)
(fmt-subsampling ctx-subsampling)) {
ret = s5p_jpeg_adjust_fourcc_to_subsampling(ctx-subsampling,
fmt-fourcc,
@@ -1085,6 +1087,23 @@ static int s5p_jpeg_try_fmt_vid_cap(struct file *file,
void *priv,
FMT_TYPE_CAPTURE);
}
+ /*
+* Decompression of a JPEG file with 4:2:0 subsampling and odd
+* width to the YUV 4:2:0 compliant formats produces a raw image
+* with broken luma component. Adjust capture format to RGB565
+* in such a case.
+*/
+ if (ctx-subsampling == V4L2_JPEG_CHROMA_SUBSAMPLING_420
+ (ctx-out_q.w 1)
+ (pix-pixelformat == V4L2_PIX_FMT_NV12 ||
+pix-pixelformat == V4L2_PIX_FMT_NV21 ||
+pix-pixelformat == V4L2_PIX_FMT_YUV420)) {
+ pix-pixelformat = V4L2_PIX_FMT_RGB565;
+ fmt = s5p_jpeg_find_format(ctx, pix-pixelformat,
+ FMT_TYPE_CAPTURE);
+ }
+
+exit:
return vidioc_try_fmt(f, fmt, ctx, FMT_TYPE_CAPTURE);
}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/8] s5p-jpeg: Perform fourcc downgrade only for Exynos4x12 SoCs
Change the driver variant check from is not S5PC210
to is Exynos4 while checking whether YUV format needs
to be downgraded in order to prevent upsampling which
is not supported by Exynos4 SoCs family.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/s5p-jpeg/jpeg-core.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 3ae9210..d307c0f 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1070,7 +1070,7 @@ static int s5p_jpeg_try_fmt_vid_cap(struct file *file,
void *priv,
* If this requirement is not met then downgrade the requested
* capture format to the one with subsampling equal to the input jpeg.
*/
- if ((ctx-jpeg-variant-version != SJPEG_S5P)
+ if ((ctx-jpeg-variant-version == SJPEG_EXYNOS4)
(ctx-mode == S5P_JPEG_DECODE)
(fmt-flags SJPEG_FMT_NON_RGB)
(fmt-subsampling ctx-subsampling)) {
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/8] s5p-jpeg: Add m2m_ops field to the s5p_jpeg_variant structure
Simplify the code by adding m2m_ops field to the
s5p_jpeg_variant structure which allows to avoid
if statement in the s5p_jpeg_probe function.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 12
drivers/media/platform/s5p-jpeg/jpeg-core.h |7 ---
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index d307c0f..1b69b69 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1566,7 +1566,7 @@ static struct v4l2_m2m_ops s5p_jpeg_m2m_ops = {
.job_abort = s5p_jpeg_job_abort,
}
;
-static struct v4l2_m2m_ops exynos_jpeg_m2m_ops = {
+static struct v4l2_m2m_ops exynos4_jpeg_m2m_ops = {
.device_run = exynos4_jpeg_device_run,
.job_ready = s5p_jpeg_job_ready,
.job_abort = s5p_jpeg_job_abort,
@@ -1852,7 +1852,6 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
{
struct s5p_jpeg *jpeg;
struct resource *res;
- struct v4l2_m2m_ops *samsung_jpeg_m2m_ops;
int ret;
if (!pdev-dev.of_node)
@@ -1906,13 +1905,8 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
goto clk_get_rollback;
}
- if (jpeg-variant-version == SJPEG_S5P)
- samsung_jpeg_m2m_ops = s5p_jpeg_m2m_ops;
- else
- samsung_jpeg_m2m_ops = exynos_jpeg_m2m_ops;
-
/* mem2mem device */
- jpeg-m2m_dev = v4l2_m2m_init(samsung_jpeg_m2m_ops);
+ jpeg-m2m_dev = v4l2_m2m_init(jpeg-variant-m2m_ops);
if (IS_ERR(jpeg-m2m_dev)) {
v4l2_err(jpeg-v4l2_dev, Failed to init mem2mem device\n);
ret = PTR_ERR(jpeg-m2m_dev);
@@ -2101,12 +2095,14 @@ static const struct dev_pm_ops s5p_jpeg_pm_ops = {
static struct s5p_jpeg_variant s5p_jpeg_drvdata = {
.version= SJPEG_S5P,
.jpeg_irq = s5p_jpeg_irq,
+ .m2m_ops= s5p_jpeg_m2m_ops,
.fmt_ver_flag = SJPEG_FMT_FLAG_S5P,
};
static struct s5p_jpeg_variant exynos4_jpeg_drvdata = {
.version= SJPEG_EXYNOS4,
.jpeg_irq = exynos4_jpeg_irq,
+ .m2m_ops= exynos4_jpeg_m2m_ops,
.fmt_ver_flag = SJPEG_FMT_FLAG_EXYNOS4,
};
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.h
b/drivers/media/platform/s5p-jpeg/jpeg-core.h
index c222436..3e47863 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.h
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.h
@@ -117,9 +117,10 @@ struct s5p_jpeg {
};
struct s5p_jpeg_variant {
- unsigned intversion;
- unsigned intfmt_ver_flag;
- irqreturn_t (*jpeg_irq)(int irq, void *priv);
+ unsigned intversion;
+ unsigned intfmt_ver_flag;
+ struct v4l2_m2m_ops *m2m_ops;
+ irqreturn_t (*jpeg_irq)(int irq, void *priv);
};
/**
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/8] s5p-jpeg: Add fmt_ver_flag field to the s5p_jpeg_variant structure
Simplify the code by adding fmt_ver_flag field
to the s5p_jpeg_variant structure which allows
to avoid if statement in the s5p_jpeg_find_format
function.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 11 ---
drivers/media/platform/s5p-jpeg/jpeg-core.h |1 +
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 8a18972..3ae9210 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -959,7 +959,7 @@ static int s5p_jpeg_g_fmt(struct file *file, void *priv,
struct v4l2_format *f)
static struct s5p_jpeg_fmt *s5p_jpeg_find_format(struct s5p_jpeg_ctx *ctx,
u32 pixelformat, unsigned int fmt_type)
{
- unsigned int k, fmt_flag, ver_flag;
+ unsigned int k, fmt_flag;
if (ctx-mode == S5P_JPEG_ENCODE)
fmt_flag = (fmt_type == FMT_TYPE_OUTPUT) ?
@@ -970,16 +970,11 @@ static struct s5p_jpeg_fmt *s5p_jpeg_find_format(struct
s5p_jpeg_ctx *ctx,
SJPEG_FMT_FLAG_DEC_OUTPUT :
SJPEG_FMT_FLAG_DEC_CAPTURE;
- if (ctx-jpeg-variant-version == SJPEG_S5P)
- ver_flag = SJPEG_FMT_FLAG_S5P;
- else
- ver_flag = SJPEG_FMT_FLAG_EXYNOS4;
-
for (k = 0; k ARRAY_SIZE(sjpeg_formats); k++) {
struct s5p_jpeg_fmt *fmt = sjpeg_formats[k];
if (fmt-fourcc == pixelformat
fmt-flags fmt_flag
- fmt-flags ver_flag) {
+ fmt-flags ctx-jpeg-variant-fmt_ver_flag) {
return fmt;
}
}
@@ -2106,11 +2101,13 @@ static const struct dev_pm_ops s5p_jpeg_pm_ops = {
static struct s5p_jpeg_variant s5p_jpeg_drvdata = {
.version= SJPEG_S5P,
.jpeg_irq = s5p_jpeg_irq,
+ .fmt_ver_flag = SJPEG_FMT_FLAG_S5P,
};
static struct s5p_jpeg_variant exynos4_jpeg_drvdata = {
.version= SJPEG_EXYNOS4,
.jpeg_irq = exynos4_jpeg_irq,
+ .fmt_ver_flag = SJPEG_FMT_FLAG_EXYNOS4,
};
static const struct of_device_id samsung_jpeg_match[] = {
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.h
b/drivers/media/platform/s5p-jpeg/jpeg-core.h
index f482dbf..c222436 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.h
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.h
@@ -118,6 +118,7 @@ struct s5p_jpeg {
struct s5p_jpeg_variant {
unsigned intversion;
+ unsigned intfmt_ver_flag;
irqreturn_t (*jpeg_irq)(int irq, void *priv);
};
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/8] s5p-jpeg: Fix build break when CONFIG_OF is undefined
This patch fixes build break occurring when
there is no support for Device Tree turned on
in the kernel configuration. In such a case only
the driver variant for S5PC210 SoC will be available.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 1b69b69..04260c2 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1840,7 +1840,7 @@ static irqreturn_t exynos4_jpeg_irq(int irq, void *priv)
return IRQ_HANDLED;
}
-static void *jpeg_get_drv_data(struct platform_device *pdev);
+static void *jpeg_get_drv_data(struct device *dev);
/*
*
@@ -1854,15 +1854,12 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
struct resource *res;
int ret;
- if (!pdev-dev.of_node)
- return -ENODEV;
-
/* JPEG IP abstraction struct */
jpeg = devm_kzalloc(pdev-dev, sizeof(struct s5p_jpeg), GFP_KERNEL);
if (!jpeg)
return -ENOMEM;
- jpeg-variant = jpeg_get_drv_data(pdev);
+ jpeg-variant = jpeg_get_drv_data(pdev-dev);
mutex_init(jpeg-lock);
spin_lock_init(jpeg-slock);
@@ -2091,7 +2088,6 @@ static const struct dev_pm_ops s5p_jpeg_pm_ops = {
SET_RUNTIME_PM_OPS(s5p_jpeg_runtime_suspend, s5p_jpeg_runtime_resume,
NULL)
};
-#ifdef CONFIG_OF
static struct s5p_jpeg_variant s5p_jpeg_drvdata = {
.version= SJPEG_S5P,
.jpeg_irq = s5p_jpeg_irq,
@@ -2122,19 +2118,21 @@ static const struct of_device_id samsung_jpeg_match[] =
{
MODULE_DEVICE_TABLE(of, samsung_jpeg_match);
-static void *jpeg_get_drv_data(struct platform_device *pdev)
+static void *jpeg_get_drv_data(struct device *dev)
{
struct s5p_jpeg_variant *driver_data = NULL;
const struct of_device_id *match;
- match = of_match_node(of_match_ptr(samsung_jpeg_match),
-pdev-dev.of_node);
+ if (!IS_ENABLED(CONFIG_OF) || !dev-of_node)
+ return s5p_jpeg_drvdata;
+
+ match = of_match_node(samsung_jpeg_match, dev-of_node);
+
if (match)
driver_data = (struct s5p_jpeg_variant *)match-data;
return driver_data;
}
-#endif
static struct platform_driver s5p_jpeg_driver = {
.probe = s5p_jpeg_probe,
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 8/8] s5p_jpeg: Fix NV12 format entry related to S5C2120 SoC
S5PC210 SoC doesn't support encoding NV12 raw images. Remove
the relavant flag from the respective entry in the sjpeg_formats
array.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/s5p-jpeg/jpeg-core.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 24545bd..2c8481e 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -192,8 +192,7 @@ static struct s5p_jpeg_fmt sjpeg_formats[] = {
.colplanes = 2,
.h_align= 4,
.v_align= 4,
- .flags = SJPEG_FMT_FLAG_ENC_OUTPUT |
- SJPEG_FMT_FLAG_DEC_CAPTURE |
+ .flags = SJPEG_FMT_FLAG_DEC_CAPTURE |
SJPEG_FMT_FLAG_S5P |
SJPEG_FMT_NON_RGB,
.subsampling= V4L2_JPEG_CHROMA_SUBSAMPLING_420,
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] dma-buf: update exp_name when using dma_buf_export()
On Thu, Apr 10, 2014 at 01:30:06AM +0200, Javier Martinez Canillas wrote: commit c0b00a5 (dma-buf: update debugfs output) modified the default exporter name to be the KBUILD_MODNAME pre-processor macro instead of __FILE__ but the documentation was not updated. Also the Supporting existing mmap interfaces in exporters section title seems wrong since talks about the interface used by importers. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch --- Documentation/dma-buf-sharing.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/dma-buf-sharing.txt b/Documentation/dma-buf-sharing.txt index 505e711..7d61cef 100644 --- a/Documentation/dma-buf-sharing.txt +++ b/Documentation/dma-buf-sharing.txt @@ -66,7 +66,7 @@ The dma_buf buffer sharing API usage contains the following steps: Exporting modules which do not wish to provide any specific name may use the helper define 'dma_buf_export()', with the same arguments as above, but - without the last argument; a __FILE__ pre-processor directive will be + without the last argument; a KBUILD_MODNAME pre-processor directive will be inserted in place of 'exp_name' instead. 2. Userspace gets a handle to pass around to potential buffer-users @@ -352,7 +352,7 @@ Being able to mmap an export dma-buf buffer object has 2 main use-cases: No special interfaces, userspace simply calls mmap on the dma-buf fd. -2. Supporting existing mmap interfaces in exporters +2. Supporting existing mmap interfaces in importers Similar to the motivation for kernel cpu access it is again important that the userspace code of a given importing subsystem can use the same interfaces -- 1.9.0 ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [RFC] reservation: add suppport for read-only access using rcu
Hi!
Ugh. This became more complicated than I thought, but I'm OK with moving
TTM over to fence while we sort out
how / if we're going to use this.
While reviewing, it struck me that this is kind of error-prone, and hard
to follow since we're operating on a structure that may be
continually updated under us, needing a lot of RCU-specific macros and
barriers.
Also the rcu wait appears to not complete until there are no busy fences
left (new ones can be added while we wait) rather than
waiting on a snapshot of busy fences.
I wonder if these issues can be addressed by having a function that
provides a snapshot of all busy fences: This can be accomplished
either by including the exclusive fence in the fence_list structure and
allocate a new such structure each time it is updated. The RCU reader
could then just make a copy of the current fence_list structure pointed
to by obj-fence, but I'm not sure we want to reallocate *each* time we
update the fence pointer.
The other approach uses a seqlock to obtain a consistent snapshot, and
I've attached an incomplete outline, and I'm not 100% whether it's OK to
combine RCU and seqlocks in this way...
Both these approaches have the benefit of hiding the RCU snapshotting in
a single function, that can then be used by any waiting
or polling function.
/Thomas
On 04/09/2014 04:49 PM, Maarten Lankhorst wrote:
This adds 3 more functions to deal with rcu.
reservation_object_wait_timeout_rcu() will wait on all fences of the
reservation_object, without obtaining the ww_mutex.
reservation_object_test_signaled_rcu() will test if all fences of the
reservation_object are signaled without using the ww_mutex.
reservation_object_get_excl() is added because touching the fence_excl
member directly will trigger a sparse warning.
Signed-off-by: Maarten Lankhorst maarten.lankho...@canonical.com
---
drivers/base/dma-buf.c | 46 +++--
drivers/base/reservation.c | 147
+--
include/linux/fence.h | 22 ++
include/linux/reservation.h | 40
4 files changed, 224 insertions(+), 31 deletions(-)
diff --git a/drivers/base/reservation.c b/drivers/base/reservation.c
index b82a5b6..c4bcf10 100644
--- a/drivers/base/reservation.c
+++ b/drivers/base/reservation.c
@@ -82,6 +82,8 @@ reservation_object_add_shared_inplace(struct reservation_object *obj,
{
u32 i;
+ preempt_disable();
+ write_seqcount_begin(obj-seq);
for (i = 0; i fobj-shared_count; ++i) {
if (fobj-shared[i]-context == fence-context) {
struct fence *old_fence = fobj-shared[i];
@@ -90,6 +92,8 @@ reservation_object_add_shared_inplace(struct reservation_object *obj,
fobj-shared[i] = fence;
+ write_seqcount_end(obj-seq);
+ preempt_enable();
fence_put(old_fence);
return;
}
@@ -101,8 +105,9 @@ reservation_object_add_shared_inplace(struct reservation_object *obj,
* make the new fence visible before incrementing
* fobj-shared_count
*/
- smp_wmb();
fobj-shared_count++;
+ write_seqcount_end(obj-seq);
+ preempt_enable();
}
static void
@@ -141,7 +146,11 @@ reservation_object_add_shared_replace(struct reservation_object *obj,
fobj-shared[fobj-shared_count++] = fence;
done:
+ preempt_disable();
+ write_seqcount_begin(obj-seq);
obj-fence = fobj;
+ write_seqcount_end(obj-seq);
+ preempt_enable();
kfree(old);
}
@@ -173,6 +182,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
u32 i = 0;
old = reservation_object_get_list(obj);
+ preempt_disable();
+ write_seqcount_begin(obj-seq);
if (old) {
i = old-shared_count;
old-shared_count = 0;
@@ -182,7 +193,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
fence_get(fence);
obj-fence_excl = fence;
-
+ write_seqcount_end(obj-seq);
+ preempt_enable();
/* inplace update, no shared fences */
while (i--)
fence_put(old-shared[i]);
@@ -191,3 +203,76 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
fence_put(old_fence);
}
EXPORT_SYMBOL(reservation_object_add_excl_fence);
+
+struct unsignaled {
+ unsigned shared_max;
+ unsigned shared_count;
+ struct fence **shared;
+ struct fence *exclusive;
+};
+
+static int reservation_object_unsignaled_rcu(struct reservation_object *obj,
+ struct unsignaled *us)
+{
+ unsigned seq;
+ struct reservation_object_list *fobj, list;
+ struct fence *fence;
+
+retry:
+ seq = read_seqcount_begin(obj-seq);
+ rcu_read_lock();
+
+ fobj = obj-fence;
+ fence = obj-exclusive;
+
+ /* Check pointers for validity */
+ if (read_seqcount_retry(obj-seq, seq)) {
+ rcu_read_unlock();
+ goto retry;
+ }
+
+ list = *fobj;
+
+ /* Check list for validity */
+ if (read_seqcount_retry(obj-seq, seq)) {
+ rcu_read_unlock();
+ goto retry;
+ }
+
+ if (list.shared_count == 0) {
+ if (fence
+ !test_bit(FENCE_FLAG_SIGNALED_BIT, fence-flags)
+ fence_get_rcu(fence))
+ us-exclusive = exclusive;
+ rcu_read_unlock();
+
[PATCH 1/2] staging: media: omap24xx: fix up checkpatch error message
tcm825x.c and tcm825x.h:
fixing ERROR: Macros with complex values should be enclosed in parenthesis
Signed-off-by: Vitaly Osipov vitaly.osi...@gmail.com
---
drivers/staging/media/omap24xx/tcm825x.c |8
drivers/staging/media/omap24xx/tcm825x.h |4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/media/omap24xx/tcm825x.c
b/drivers/staging/media/omap24xx/tcm825x.c
index f4dd32d..2326481 100644
--- a/drivers/staging/media/omap24xx/tcm825x.c
+++ b/drivers/staging/media/omap24xx/tcm825x.c
@@ -89,10 +89,10 @@ static const struct tcm825x_reg rgb565 = { 0x02,
TCM825X_PICFMT };
/* Our own specific controls */
#define V4L2_CID_ALC V4L2_CID_PRIVATE_BASE
-#define V4L2_CID_H_EDGE_EN V4L2_CID_PRIVATE_BASE + 1
-#define V4L2_CID_V_EDGE_EN V4L2_CID_PRIVATE_BASE + 2
-#define V4L2_CID_LENS V4L2_CID_PRIVATE_BASE + 3
-#define V4L2_CID_MAX_EXPOSURE_TIME V4L2_CID_PRIVATE_BASE + 4
+#define V4L2_CID_H_EDGE_EN (V4L2_CID_PRIVATE_BASE + 1)
+#define V4L2_CID_V_EDGE_EN (V4L2_CID_PRIVATE_BASE + 2)
+#define V4L2_CID_LENS (V4L2_CID_PRIVATE_BASE + 3)
+#define V4L2_CID_MAX_EXPOSURE_TIME (V4L2_CID_PRIVATE_BASE + 4)
#define V4L2_CID_LAST_PRIV V4L2_CID_MAX_EXPOSURE_TIME
/* Video controls */
diff --git a/drivers/staging/media/omap24xx/tcm825x.h
b/drivers/staging/media/omap24xx/tcm825x.h
index 9970fb1..4a41127 100644
--- a/drivers/staging/media/omap24xx/tcm825x.h
+++ b/drivers/staging/media/omap24xx/tcm825x.h
@@ -21,8 +21,8 @@
#define TCM825X_NAME tcm825x
-#define TCM825X_MASK(x) x 0x00ff
-#define TCM825X_ADDR(x) (x 0xff00) 8
+#define TCM825X_MASK(x) (x 0x00ff)
+#define TCM825X_ADDR(x) ((x 0xff00) 8)
/* The TCM825X I2C sensor chip has a fixed slave address of 0x3d. */
#define TCM825X_I2C_ADDR 0x3d
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] staging: media: omap24xx: fix up a checkpatch.pl warning
tcm825x.c: changed printk to pr_info Signed-off-by: Vitaly Osipov vitaly.osi...@gmail.com --- drivers/staging/media/omap24xx/tcm825x.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/omap24xx/tcm825x.c b/drivers/staging/media/omap24xx/tcm825x.c index 2326481..3367ccd 100644 --- a/drivers/staging/media/omap24xx/tcm825x.c +++ b/drivers/staging/media/omap24xx/tcm825x.c @@ -914,8 +914,8 @@ static int __init tcm825x_init(void) rval = i2c_add_driver(tcm825x_i2c_driver); if (rval) - printk(KERN_INFO %s: failed registering TCM825X_NAME \n, - __func__); + pr_info(%s: failed registering TCM825X_NAME \n, + __func__); return rval; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] staging: media: omap24xx: fix up a checkpatch.pl warning
The two subjects are really close to being the same. You should choose better subjects. Like: [PATCH 2/2] staging: media: omap24xx: use pr_info() instead of KERN_INFO (All the checkpatch.pl people use the exact same subject for everything though, so you're not alone in this). regards, dan carpenter -- 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] Helper to abstract vma handling in media layer
Hello, On 2014-03-17 20:49, Jan Kara wrote: The following patch series is my first stab at abstracting vma handling from the various media drivers. After this patch set drivers have to know much less details about vmas, their types, and locking. My motivation for the series is that I want to change get_user_pages() locking and I want to handle subtle locking details in as few places as possible. The core of the series is the new helper get_vaddr_pfns() which is given a virtual address and it fills in PFNs into provided array. If PFNs correspond to normal pages it also grabs references to these pages. The difference from get_user_pages() is that this function can also deal with pfnmap, mixed, and io mappings which is what the media drivers need. The patches are just compile tested (since I don't have any of the hardware I'm afraid I won't be able to do any more testing anyway) so please handle with care. I'm grateful for any comments. Thanks for posting this series! I will check if it works with our hardware soon. This is something I wanted to introduce some time ago to simplify buffer handling in dma-buf, but I had no time to start working. However I would like to go even further with integration of your pfn vector idea. This structure looks like a best solution for a compact representation of the memory buffer, which should be considered by the hardware as contiguous (either contiguous in physical memory or mapped contiguously into dma address space by the respective iommu). As you already noticed it is widely used by graphics and video drivers. I would also like to add support for pfn vector directly to the dma-mapping subsystem. This can be done quite easily (even with a fallback for architectures which don't provide method for it). I will try to prepare rfc soon. This will finally remove the need for hacks in media/v4l2-core/videobuf2-dma-contig.c Thanks for motivating me to finally start working on this! Best regards -- Marek Szyprowski, PhD Samsung RD Institute Poland -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [RFC] reservation: add suppport for read-only access using rcu
Hey,
op 10-04-14 10:46, Thomas Hellstrom schreef:
Hi!
Ugh. This became more complicated than I thought, but I'm OK with moving
TTM over to fence while we sort out
how / if we're going to use this.
While reviewing, it struck me that this is kind of error-prone, and hard
to follow since we're operating on a structure that may be
continually updated under us, needing a lot of RCU-specific macros and
barriers.
Yeah, but with the exception of dma_buf_poll I don't think there is anything
else
outside drivers/base/reservation.c has to deal with rcu.
Also the rcu wait appears to not complete until there are no busy fences
left (new ones can be added while we wait) rather than
waiting on a snapshot of busy fences.
This has been by design, because 'wait for bo idle' type of functions only care
if the bo is completely idle or not.
It would be easy to make a snapshot even without seqlocks, just copy
reservation_object_test_signaled_rcu to return a shared list if test_all is
set, or return pointer to exclusive otherwise.
I wonder if these issues can be addressed by having a function that
provides a snapshot of all busy fences: This can be accomplished
either by including the exclusive fence in the fence_list structure and
allocate a new such structure each time it is updated. The RCU reader
could then just make a copy of the current fence_list structure pointed
to by obj-fence, but I'm not sure we want to reallocate *each* time we
update the fence pointer.
No, the most common operation is updating fence pointers, which is why
the current design makes that cheap. It's also why doing rcu reads is more
expensive.
The other approach uses a seqlock to obtain a consistent snapshot, and
I've attached an incomplete outline, and I'm not 100% whether it's OK to
combine RCU and seqlocks in this way...
Both these approaches have the benefit of hiding the RCU snapshotting in
a single function, that can then be used by any waiting
or polling function.
I think the middle way with using seqlocks to protect the fence_excl pointer
and shared list combination,
and using RCU to protect the refcounts for fences and the availability of the
list could work for our usecase
and might remove a bunch of memory barriers. But yeah that depends on layering
rcu and seqlocks.
No idea if that is allowed. But I suppose it is.
Also, you're being overly paranoid with seqlock reading, we would only need
something like this:
rcu_read_lock()
preempt_disable()
seq = read_seqcount_begin();
read fence_excl, shared_count = ACCESS_ONCE(fence-shared_count)
copy shared to a struct.
if (read_seqcount_retry()) { unlock and retry }
preempt_enable();
use fence_get_rcu() to bump refcount on everything, if that fails unlock,
put, and retry
rcu_read_unlock()
But the shared list would still need to be RCU'd, to make sure we're not
reading freed garbage.
~Maarten
--
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] Helper to abstract vma handling in media layer
Hello, On Thu 10-04-14 12:02:50, Marek Szyprowski wrote: On 2014-03-17 20:49, Jan Kara wrote: The following patch series is my first stab at abstracting vma handling from the various media drivers. After this patch set drivers have to know much less details about vmas, their types, and locking. My motivation for the series is that I want to change get_user_pages() locking and I want to handle subtle locking details in as few places as possible. The core of the series is the new helper get_vaddr_pfns() which is given a virtual address and it fills in PFNs into provided array. If PFNs correspond to normal pages it also grabs references to these pages. The difference from get_user_pages() is that this function can also deal with pfnmap, mixed, and io mappings which is what the media drivers need. The patches are just compile tested (since I don't have any of the hardware I'm afraid I won't be able to do any more testing anyway) so please handle with care. I'm grateful for any comments. Thanks for posting this series! I will check if it works with our hardware soon. This is something I wanted to introduce some time ago to simplify buffer handling in dma-buf, but I had no time to start working. Thanks for having a look in the series. However I would like to go even further with integration of your pfn vector idea. This structure looks like a best solution for a compact representation of the memory buffer, which should be considered by the hardware as contiguous (either contiguous in physical memory or mapped contiguously into dma address space by the respective iommu). As you already noticed it is widely used by graphics and video drivers. I would also like to add support for pfn vector directly to the dma-mapping subsystem. This can be done quite easily (even with a fallback for architectures which don't provide method for it). I will try to prepare rfc soon. This will finally remove the need for hacks in media/v4l2-core/videobuf2-dma-contig.c That would be a worthwhile thing to do. When I was reading the code this seemed like something which could be done but I delibrately avoided doing more unification than necessary for my purposes as I don't have any hardware to test and don't know all the subtleties in the code... BTW, is there some way to test the drivers without the physical video HW? Honza -- Jan Kara j...@suse.cz SUSE Labs, CR -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hauppauge ImpactVCB-e 01385
Hi Guys, Sorry to go on about the Hauppauge ImpactVCB-e 01385, but it's a couple of years and version 12.04, since I last asked. This page: http://linuxtv.org/wiki/index.php/Hauppauge Now shows: Table of analog-only devices sold by Hauppauge Model Standard Interface Supported Comments ImpactVCB-e Video PCIe ✔ Yes No tuners, only video-in. S-Video Capture works with kernel 3.5.0 (Ubuntu 12.10). As my distribution is 13.10 with kernel 3.11, I believe it should work. uname -a gives: 3.11.0-19-generic #33-Ubuntu SMP Tue Mar 11 18:48:34 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux When I plug in my 01385 I get the same old stuff in dmseg, ie: cx23885 driver version 0.0.3 loaded [ 8.921390] cx23885[0]: Your board isn't known (yet) to the driver. [ 8.921390] cx23885[0]: Try to pick one of the existing card configs via [ 8.921390] cx23885[0]: card=n insmod option. Updating to the latest [ 8.921390] cx23885[0]: version might help as well. [ 8.921393] cx23885[0]: Here is a list of valid choices for the card=n insmod option: Etc. Does anyone have any idea of the issue here? Best regards Steve -- 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 0/2] managed token devres interfaces
- Construct string with (dev is struct em28xx *dev) format: tuner:%s-%s-%d with the following: dev_name(dev-udev-dev) dev-udev-bus-bus_name dev-tuner_addr What guarantees this won't get confused by hot plugging and re-use of the bus slot ? I'm also not sure I understand why you can't have a shared parent device and simply attach the resources to that. This sounds like a problem mfd already solved ? Alan -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [RFC] reservation: add suppport for read-only access using rcu
On 04/10/2014 12:07 PM, Maarten Lankhorst wrote:
Hey,
op 10-04-14 10:46, Thomas Hellstrom schreef:
Hi!
Ugh. This became more complicated than I thought, but I'm OK with moving
TTM over to fence while we sort out
how / if we're going to use this.
While reviewing, it struck me that this is kind of error-prone, and hard
to follow since we're operating on a structure that may be
continually updated under us, needing a lot of RCU-specific macros and
barriers.
Yeah, but with the exception of dma_buf_poll I don't think there is
anything else
outside drivers/base/reservation.c has to deal with rcu.
Also the rcu wait appears to not complete until there are no busy fences
left (new ones can be added while we wait) rather than
waiting on a snapshot of busy fences.
This has been by design, because 'wait for bo idle' type of functions
only care
if the bo is completely idle or not.
No, not when using RCU, because the bo may be busy again before the
function returns :)
Complete idleness can only be guaranteed if holding the reservation, or
otherwise making sure
that no new rendering is submitted to the buffer, so it's an overkill to
wait for complete idleness here.
It would be easy to make a snapshot even without seqlocks, just copy
reservation_object_test_signaled_rcu to return a shared list if
test_all is set, or return pointer to exclusive otherwise.
I wonder if these issues can be addressed by having a function that
provides a snapshot of all busy fences: This can be accomplished
either by including the exclusive fence in the fence_list structure and
allocate a new such structure each time it is updated. The RCU reader
could then just make a copy of the current fence_list structure pointed
to by obj-fence, but I'm not sure we want to reallocate *each* time we
update the fence pointer.
No, the most common operation is updating fence pointers, which is why
the current design makes that cheap. It's also why doing rcu reads is
more expensive.
The other approach uses a seqlock to obtain a consistent snapshot, and
I've attached an incomplete outline, and I'm not 100% whether it's OK to
combine RCU and seqlocks in this way...
Both these approaches have the benefit of hiding the RCU snapshotting in
a single function, that can then be used by any waiting
or polling function.
I think the middle way with using seqlocks to protect the fence_excl
pointer and shared list combination,
and using RCU to protect the refcounts for fences and the availability
of the list could work for our usecase
and might remove a bunch of memory barriers. But yeah that depends on
layering rcu and seqlocks.
No idea if that is allowed. But I suppose it is.
Also, you're being overly paranoid with seqlock reading, we would only
need something like this:
rcu_read_lock()
preempt_disable()
seq = read_seqcount_begin()
read fence_excl, shared_count = ACCESS_ONCE(fence-shared_count)
copy shared to a struct.
if (read_seqcount_retry()) { unlock and retry }
preempt_enable();
use fence_get_rcu() to bump refcount on everything, if that fails
unlock, put, and retry
rcu_read_unlock()
But the shared list would still need to be RCU'd, to make sure we're
not reading freed garbage.
Ah. OK,
But I think we should use rcu inside seqcount, because
read_seqcount_begin() may spin for a long time if there are
many writers. Also I don't think the preempt_disable() is needed for
read_seq critical sections other than they might
decrease the risc of retries..
Thanks,
Thomas
~Maarten
--
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] Helper to abstract vma handling in media layer
On 04/10/14 12:32, Jan Kara wrote: Hello, On Thu 10-04-14 12:02:50, Marek Szyprowski wrote: On 2014-03-17 20:49, Jan Kara wrote: The following patch series is my first stab at abstracting vma handling from the various media drivers. After this patch set drivers have to know much less details about vmas, their types, and locking. My motivation for the series is that I want to change get_user_pages() locking and I want to handle subtle locking details in as few places as possible. The core of the series is the new helper get_vaddr_pfns() which is given a virtual address and it fills in PFNs into provided array. If PFNs correspond to normal pages it also grabs references to these pages. The difference from get_user_pages() is that this function can also deal with pfnmap, mixed, and io mappings which is what the media drivers need. The patches are just compile tested (since I don't have any of the hardware I'm afraid I won't be able to do any more testing anyway) so please handle with care. I'm grateful for any comments. Thanks for posting this series! I will check if it works with our hardware soon. This is something I wanted to introduce some time ago to simplify buffer handling in dma-buf, but I had no time to start working. Thanks for having a look in the series. However I would like to go even further with integration of your pfn vector idea. This structure looks like a best solution for a compact representation of the memory buffer, which should be considered by the hardware as contiguous (either contiguous in physical memory or mapped contiguously into dma address space by the respective iommu). As you already noticed it is widely used by graphics and video drivers. I would also like to add support for pfn vector directly to the dma-mapping subsystem. This can be done quite easily (even with a fallback for architectures which don't provide method for it). I will try to prepare rfc soon. This will finally remove the need for hacks in media/v4l2-core/videobuf2-dma-contig.c That would be a worthwhile thing to do. When I was reading the code this seemed like something which could be done but I delibrately avoided doing more unification than necessary for my purposes as I don't have any hardware to test and don't know all the subtleties in the code... BTW, is there some way to test the drivers without the physical video HW? You can use the vivi driver (drivers/media/platform/vivi) for this. However, while the vivi driver can import dma buffers it cannot export them. If you want that, then you have to use this tree: http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/log/?h=vb2-part4 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 2/2] [RFC] reservation: add suppport for read-only access using rcu
On 04/10/2014 01:08 PM, Thomas Hellstrom wrote: On 04/10/2014 12:07 PM, Maarten Lankhorst wrote: Hey, op 10-04-14 10:46, Thomas Hellstrom schreef: Hi! Ugh. This became more complicated than I thought, but I'm OK with moving TTM over to fence while we sort out how / if we're going to use this. While reviewing, it struck me that this is kind of error-prone, and hard to follow since we're operating on a structure that may be continually updated under us, needing a lot of RCU-specific macros and barriers. Yeah, but with the exception of dma_buf_poll I don't think there is anything else outside drivers/base/reservation.c has to deal with rcu. Also the rcu wait appears to not complete until there are no busy fences left (new ones can be added while we wait) rather than waiting on a snapshot of busy fences. This has been by design, because 'wait for bo idle' type of functions only care if the bo is completely idle or not. No, not when using RCU, because the bo may be busy again before the function returns :) Complete idleness can only be guaranteed if holding the reservation, or otherwise making sure that no new rendering is submitted to the buffer, so it's an overkill to wait for complete idleness here. Although, if we fail to get a refcount for a fence, and it's still busy we need to do a seq retry, because the fence might have been replaced by another fence from the same context, without being idle. That check is not present in the snapshot code I sent. /Thomas -- 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 0/2] managed token devres interfaces
Hi Alan, Em Thu, 10 Apr 2014 12:04:35 +0100 One Thousand Gnomes gno...@lxorguk.ukuu.org.uk escreveu: - Construct string with (dev is struct em28xx *dev) format: tuner:%s-%s-%d with the following: dev_name(dev-udev-dev) dev-udev-bus-bus_name dev-tuner_addr What guarantees this won't get confused by hot plugging and re-use of the bus slot ? Good point. Yes, this should be addressed. I'm also not sure I understand why you can't have a shared parent device and simply attach the resources to that. This sounds like a problem mfd already solved ? There are some devices that have lots of different functions spread out on several subsystems. For example, some devices provide standard USB Audio Class, handled by snd-usb-audio for the audio stream, while the video stream is handled via a separate driver, like some em28xx devices. There are even more complex devices that provide 3G modem, storage and digital TV, whose USB ID changes when either the 3G modem starts or when the digital TV firmware is loaded. So, we need to find a way to lock some hardware resources among different subsystems that don't share anything in common. Not sure if mfd has the same type of problem of a non-mfd driver using another function of the same device that has some shared hardware resources between the separate functions, and, if so, how they solved it. Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/2] managed token devres interfaces
For example, some devices provide standard USB Audio Class, handled by snd-usb-audio for the audio stream, while the video stream is handled via a separate driver, like some em28xx devices. Which is what mfd is designed to handle. There are even more complex devices that provide 3G modem, storage and digital TV, whose USB ID changes when either the 3G modem starts or when the digital TV firmware is loaded. But presumably you only have one driver at a time then ? So, we need to find a way to lock some hardware resources among different subsystems that don't share anything in common. Not sure if mfd has the same type of problem of a non-mfd driver using another function of the same device The MFD device provides subdevices for all the functions. That is the whole underlying concept. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] staging: media: omap24xx: fix up a checkpatch.pl warning
Thanks, that's helpful - I for some reason thought that multi-part patch had to have more or less uniform subject. We the checkpatch.pl people come from http://www.eudyptula-challenge.org/, where at some stage we are told to submit a patch for a single style issue in the staging tree. All newbies... Hoping to be back with more substantial contributions soon. Regards, Vitaly On Thu, Apr 10, 2014 at 7:48 PM, Dan Carpenter dan.carpen...@oracle.com wrote: The two subjects are really close to being the same. You should choose better subjects. Like: [PATCH 2/2] staging: media: omap24xx: use pr_info() instead of KERN_INFO (All the checkpatch.pl people use the exact same subject for everything though, so you're not alone in this). regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL 3.15] rtl2832_sdr attach fixes
The following changes since commit a83b93a7480441a47856dc9104bea970e84cda87: [media] em28xx-dvb: fix PCTV 461e tuner I2C binding (2014-03-31 08:02:16 -0300) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git fixes3.15_rtl2832_sdr_attach for you to fetch changes up to 613f11d8dee62e4e7ecdc7670a54560773a324f9: rtl28xxu: silence error log about disabled rtl2832_sdr module (2014-04-10 01:01:24 +0300) Antti Palosaari (2): rtl28xxu: do not hard depend on staging SDR module rtl28xxu: silence error log about disabled rtl2832_sdr module drivers/media/usb/dvb-usb-v2/Makefile | 1 - drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 48 +++- 2 files changed, 43 insertions(+), 6 deletions(-) -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Helper to abstract vma handling in media layer
On Thu 10-04-14 13:07:42, Hans Verkuil wrote: On 04/10/14 12:32, Jan Kara wrote: Hello, On Thu 10-04-14 12:02:50, Marek Szyprowski wrote: On 2014-03-17 20:49, Jan Kara wrote: The following patch series is my first stab at abstracting vma handling from the various media drivers. After this patch set drivers have to know much less details about vmas, their types, and locking. My motivation for the series is that I want to change get_user_pages() locking and I want to handle subtle locking details in as few places as possible. The core of the series is the new helper get_vaddr_pfns() which is given a virtual address and it fills in PFNs into provided array. If PFNs correspond to normal pages it also grabs references to these pages. The difference from get_user_pages() is that this function can also deal with pfnmap, mixed, and io mappings which is what the media drivers need. The patches are just compile tested (since I don't have any of the hardware I'm afraid I won't be able to do any more testing anyway) so please handle with care. I'm grateful for any comments. Thanks for posting this series! I will check if it works with our hardware soon. This is something I wanted to introduce some time ago to simplify buffer handling in dma-buf, but I had no time to start working. Thanks for having a look in the series. However I would like to go even further with integration of your pfn vector idea. This structure looks like a best solution for a compact representation of the memory buffer, which should be considered by the hardware as contiguous (either contiguous in physical memory or mapped contiguously into dma address space by the respective iommu). As you already noticed it is widely used by graphics and video drivers. I would also like to add support for pfn vector directly to the dma-mapping subsystem. This can be done quite easily (even with a fallback for architectures which don't provide method for it). I will try to prepare rfc soon. This will finally remove the need for hacks in media/v4l2-core/videobuf2-dma-contig.c That would be a worthwhile thing to do. When I was reading the code this seemed like something which could be done but I delibrately avoided doing more unification than necessary for my purposes as I don't have any hardware to test and don't know all the subtleties in the code... BTW, is there some way to test the drivers without the physical video HW? You can use the vivi driver (drivers/media/platform/vivi) for this. However, while the vivi driver can import dma buffers it cannot export them. If you want that, then you have to use this tree: http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/log/?h=vb2-part4 Thanks for the pointer that looks good. I've also found drivers/media/platform/mem2mem_testdev.c which seems to do even more testing of the area I made changes to. So now I have to find some userspace tool which can issue proper ioctls to setup and use the buffers and I can start testing what I wrote :) Honza -- Jan Kara j...@suse.cz SUSE Labs, CR -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Helper to abstract vma handling in media layer
On 04/10/14 14:15, Jan Kara wrote: On Thu 10-04-14 13:07:42, Hans Verkuil wrote: On 04/10/14 12:32, Jan Kara wrote: Hello, On Thu 10-04-14 12:02:50, Marek Szyprowski wrote: On 2014-03-17 20:49, Jan Kara wrote: The following patch series is my first stab at abstracting vma handling from the various media drivers. After this patch set drivers have to know much less details about vmas, their types, and locking. My motivation for the series is that I want to change get_user_pages() locking and I want to handle subtle locking details in as few places as possible. The core of the series is the new helper get_vaddr_pfns() which is given a virtual address and it fills in PFNs into provided array. If PFNs correspond to normal pages it also grabs references to these pages. The difference from get_user_pages() is that this function can also deal with pfnmap, mixed, and io mappings which is what the media drivers need. The patches are just compile tested (since I don't have any of the hardware I'm afraid I won't be able to do any more testing anyway) so please handle with care. I'm grateful for any comments. Thanks for posting this series! I will check if it works with our hardware soon. This is something I wanted to introduce some time ago to simplify buffer handling in dma-buf, but I had no time to start working. Thanks for having a look in the series. However I would like to go even further with integration of your pfn vector idea. This structure looks like a best solution for a compact representation of the memory buffer, which should be considered by the hardware as contiguous (either contiguous in physical memory or mapped contiguously into dma address space by the respective iommu). As you already noticed it is widely used by graphics and video drivers. I would also like to add support for pfn vector directly to the dma-mapping subsystem. This can be done quite easily (even with a fallback for architectures which don't provide method for it). I will try to prepare rfc soon. This will finally remove the need for hacks in media/v4l2-core/videobuf2-dma-contig.c That would be a worthwhile thing to do. When I was reading the code this seemed like something which could be done but I delibrately avoided doing more unification than necessary for my purposes as I don't have any hardware to test and don't know all the subtleties in the code... BTW, is there some way to test the drivers without the physical video HW? You can use the vivi driver (drivers/media/platform/vivi) for this. However, while the vivi driver can import dma buffers it cannot export them. If you want that, then you have to use this tree: http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/log/?h=vb2-part4 Thanks for the pointer that looks good. I've also found drivers/media/platform/mem2mem_testdev.c which seems to do even more testing of the area I made changes to. So now I have to find some userspace tool which can issue proper ioctls to setup and use the buffers and I can start testing what I wrote :) Get the v4l-utils.git repository (http://git.linuxtv.org/cgit.cgi/v4l-utils.git/). You want the v4l2-ctl tool. Don't use the version supplied by your distro, that's often too old. 'v4l2-ctl --help-streaming' gives the available options for doing streaming. So simple capturing from vivi is 'v4l2-ctl --stream-mmap' or '--stream-user'. You can't test dmabuf unless you switch to the vb2-part4 branch of my tree. If you need help with testing it's easiest to contact me on the #v4l irc channel. 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 2/2] staging: media: omap24xx: fix up a checkpatch.pl warning
On Thu, Apr 10, 2014 at 09:57:23PM +1000, Vitaly Osipov wrote: Thanks, that's helpful - I for some reason thought that multi-part patch had to have more or less uniform subject. We the checkpatch.pl people come from http://www.eudyptula-challenge.org/, where at some stage we are told to submit a patch for a single style issue in the staging tree. All newbies... Hoping to be back with more substantial contributions soon. Yeah, I know about eudyptula. No worries. Newbies are welcome in staging. regards, dan carpenter -- 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 0/2] managed token devres interfaces
On 04/10/2014 05:38 AM, Mauro Carvalho Chehab wrote: Hi Alan, Em Thu, 10 Apr 2014 12:04:35 +0100 One Thousand Gnomes gno...@lxorguk.ukuu.org.uk escreveu: - Construct string with (dev is struct em28xx *dev) format: tuner:%s-%s-%d with the following: dev_name(dev-udev-dev) dev-udev-bus-bus_name dev-tuner_addr What guarantees this won't get confused by hot plugging and re-use of the bus slot ? Good point. Yes, this should be addressed. This resource should be destroyed when em28xx_ handles unplug from its em28xx_usb_disconnect() or in generic words, in its uninit. It is a matter of making sure this resource is handled in the uninit for this media device/driver(s) like any other shared resource. Would that cover the hot plugging and re-use of the bus slot scenario? -- Shuah -- Shuah Khan Senior Linux Kernel Developer - Open Source Group Samsung Research America(Silicon Valley) shuah...@samsung.com | (970) 672-0658 -- 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 0/2] managed token devres interfaces
Em Thu, 10 Apr 2014 12:46:53 +0100 One Thousand Gnomes gno...@lxorguk.ukuu.org.uk escreveu: For example, some devices provide standard USB Audio Class, handled by snd-usb-audio for the audio stream, while the video stream is handled via a separate driver, like some em28xx devices. Which is what mfd is designed to handle. There are even more complex devices that provide 3G modem, storage and digital TV, whose USB ID changes when either the 3G modem starts or when the digital TV firmware is loaded. But presumably you only have one driver at a time then ? In this specific device, before USB ID changes, only storage is available, as it contains the manufacturer's Windows driver on it (and the device firmware). After the USB ID changes and after the 3G chip manufacturer is recognized (if the device is locked to an specific 3G carrier), the USB storage switches to work as a micro SD memory reader and the TV and 3G modem functions also become available. The Kernel drivers that are used by this device are: smsdvb 18471 0 dvb_core 114974 1 smsdvb option 42468 0 smsusb 17819 0 usb_wwan 19510 1 option smsmdtv52283 2 smsdvb,smsusb rc_core27210 1 smsmdtv usb_storage56690 0 Those are the logs: [210431.241488] usb 1-1.3: new high-speed USB device number 7 using ehci-pci [210431.340676] usb 1-1.3: New USB device found, idVendor=19d2, idProduct=2000 [210431.340683] usb 1-1.3: New USB device strings: Mfr=3, Product=2, SerialNumber=4 [210431.340687] usb 1-1.3: Product: ZTE WCDMA Technologies MSM [210431.340691] usb 1-1.3: Manufacturer: ZTE,Incorporated [210431.340695] usb 1-1.3: SerialNumber: P675B1ZTED01 [210431.344778] usb-storage 1-1.3:1.0: USB Mass Storage device detected [210431.344891] scsi10 : usb-storage 1-1.3:1.0 [210432.701801] usb 1-1.3: USB disconnect, device number 7 [210438.735217] usb 1-1.3: new high-speed USB device number 8 using ehci-pci [210438.834309] usb 1-1.3: New USB device found, idVendor=19d2, idProduct=0086 [210438.834314] usb 1-1.3: New USB device strings: Mfr=3, Product=2, SerialNumber=4 [210438.834317] usb 1-1.3: Product: ZTE WCDMA Technologies MSM [210438.834319] usb 1-1.3: Manufacturer: ZTE,Incorporated [210438.834321] usb 1-1.3: SerialNumber: P675B1ZTED01 [210438.839042] option 1-1.3:1.0: GSM modem (1-port) converter detected [210438.839369] usb 1-1.3: GSM modem (1-port) converter now attached to ttyUSB1 [210438.839566] option 1-1.3:1.1: GSM modem (1-port) converter detected [210438.839749] usb 1-1.3: GSM modem (1-port) converter now attached to ttyUSB2 [210438.839880] usb-storage 1-1.3:1.2: USB Mass Storage device detected [210438.840116] scsi11 : usb-storage 1-1.3:1.2 [210438.840407] option 1-1.3:1.3: GSM modem (1-port) converter detected [210438.840593] usb 1-1.3: GSM modem (1-port) converter now attached to ttyUSB3 [210438.840811] option 1-1.3:1.4: GSM modem (1-port) converter detected [210438.840961] usb 1-1.3: GSM modem (1-port) converter now attached to ttyUSB4 [210439.844891] scsi 11:0:0:0: Direct-Access ZTE MMC Storage 2.31 PQ: 0 ANSI: 2 [210439.845516] sd 11:0:0:0: Attached scsi generic sg4 type 0 [210439.854702] sd 11:0:0:0: [sdd] Attached SCSI removable disk [210449.269010] smscore_detect_mode: line: 1274: MSG_SMS_GET_VERSION_EX_REQ failed first try [210454.266114] smscore_set_device_mode: line: 1354: mode detect failed -62 [210454.266121] smsusb_init_device: line: 436: smscore_start_device(...) failed [210454.266371] smsusb_onresponse: line: 143: error, urb status -2, 0 bytes [210454.266618] smsusb_onresponse: line: 143: error, urb status -2, 0 bytes [210454.266855] smsusb_onresponse: line: 143: error, urb status -2, 0 bytes [210454.267090] smsusb_onresponse: line: 143: error, urb status -2, 0 bytes [210454.267341] smsusb_onresponse: line: 143: error, urb status -2, 0 bytes [210454.267593] smsusb_onresponse: line: 143: error, urb status -2, 0 bytes [210454.267868] smsusb_onresponse: line: 143: error, urb status -2, 0 bytes [210454.268106] smsusb_onresponse: line: 143: error, urb status -2, 0 bytes [210454.268340] smsusb_onresponse: line: 143: error, urb status -2, 0 bytes [210454.268609] smsusb_onresponse: line: 143: error, urb status -2, 0 bytes [210454.268625] sms_ir_exit: [210454.274338] smsusb: probe of 1-1.3:1.5 failed with error -62 [210454.274413] option 1-1.3:1.5: GSM modem (1-port) converter detected [210454.274768] usb 1-1.3: GSM modem (1-port) converter now attached to ttyUSB5 [210454.310745] sd 11:0:0:0: [sdd] 7744512 512-byte logical blocks: (3.96 GB/3.69 GiB) [210454.316753] sd 11:0:0:0: [sdd] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [210454.334348] sdd: sdd1 [210455.709432] EXT4-fs (sdd1): mounting ext3 file system using the ext4 subsystem So, we need to find a way to lock some hardware resources among different subsystems that don't share
[PATCH 2/2] [RFC v2 with seqcount] reservation: add suppport for read-only access using rcu
op 10-04-14 13:08, Thomas Hellstrom schreef:
On 04/10/2014 12:07 PM, Maarten Lankhorst wrote:
Hey,
op 10-04-14 10:46, Thomas Hellstrom schreef:
Hi!
Ugh. This became more complicated than I thought, but I'm OK with moving
TTM over to fence while we sort out
how / if we're going to use this.
While reviewing, it struck me that this is kind of error-prone, and hard
to follow since we're operating on a structure that may be
continually updated under us, needing a lot of RCU-specific macros and
barriers.
Yeah, but with the exception of dma_buf_poll I don't think there is
anything else
outside drivers/base/reservation.c has to deal with rcu.
Also the rcu wait appears to not complete until there are no busy fences
left (new ones can be added while we wait) rather than
waiting on a snapshot of busy fences.
This has been by design, because 'wait for bo idle' type of functions
only care
if the bo is completely idle or not.
No, not when using RCU, because the bo may be busy again before the
function returns :)
Complete idleness can only be guaranteed if holding the reservation, or
otherwise making sure
that no new rendering is submitted to the buffer, so it's an overkill to
wait for complete idleness here.
You're probably right, but it makes waiting a lot easier if I don't have to
deal with memory allocations. :P
It would be easy to make a snapshot even without seqlocks, just copy
reservation_object_test_signaled_rcu to return a shared list if
test_all is set, or return pointer to exclusive otherwise.
I wonder if these issues can be addressed by having a function that
provides a snapshot of all busy fences: This can be accomplished
either by including the exclusive fence in the fence_list structure and
allocate a new such structure each time it is updated. The RCU reader
could then just make a copy of the current fence_list structure pointed
to by obj-fence, but I'm not sure we want to reallocate *each* time we
update the fence pointer.
No, the most common operation is updating fence pointers, which is why
the current design makes that cheap. It's also why doing rcu reads is
more expensive.
The other approach uses a seqlock to obtain a consistent snapshot, and
I've attached an incomplete outline, and I'm not 100% whether it's OK to
combine RCU and seqlocks in this way...
Both these approaches have the benefit of hiding the RCU snapshotting in
a single function, that can then be used by any waiting
or polling function.
I think the middle way with using seqlocks to protect the fence_excl
pointer and shared list combination,
and using RCU to protect the refcounts for fences and the availability
of the list could work for our usecase
and might remove a bunch of memory barriers. But yeah that depends on
layering rcu and seqlocks.
No idea if that is allowed. But I suppose it is.
Also, you're being overly paranoid with seqlock reading, we would only
need something like this:
rcu_read_lock()
preempt_disable()
seq = read_seqcount_begin()
read fence_excl, shared_count = ACCESS_ONCE(fence-shared_count)
copy shared to a struct.
if (read_seqcount_retry()) { unlock and retry }
preempt_enable();
use fence_get_rcu() to bump refcount on everything, if that fails
unlock, put, and retry
rcu_read_unlock()
But the shared list would still need to be RCU'd, to make sure we're
not reading freed garbage.
Ah. OK,
But I think we should use rcu inside seqcount, because
read_seqcount_begin() may spin for a long time if there are
many writers. Also I don't think the preempt_disable() is needed for
read_seq critical sections other than they might
decrease the risc of retries..
Reading the seqlock code makes me suspect that's the case too. The lockdep code
calls
local_irq_disable, so it's probably safe without preemption disabled.
~Maarten
I like the ability of not allocating memory, so I kept
reservation_object_wait_timeout_rcu mostly
the way it was. This code appears to fail on nouveau when using the shared
members,
but I'm not completely sure whether the error is in nouveau or this code yet.
--8
[RFC v2] reservation: add suppport for read-only access using rcu
This adds 4 more functions to deal with rcu.
reservation_object_get_fences_rcu() will obtain the list of shared
and exclusive fences without obtaining the ww_mutex.
reservation_object_wait_timeout_rcu() will wait on all fences of the
reservation_object, without obtaining the ww_mutex.
reservation_object_test_signaled_rcu() will test if all fences of the
reservation_object are signaled without using the ww_mutex.
reservation_object_get_excl() is added because touching the fence_excl
member directly will trigger a sparse warning.
Signed-off-by: Maarten Lankhorst maarten.lankho...@canonical.com
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index d89a98d2c37b..ca6ef0c4b358 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -137,7 +137,7 @@ static unsigned int dma_buf_poll(struct file
Re: Hauppauge ImpactVCB-e 01385
When I plug in my 01385 I get the same old stuff in dmseg, ie: cx23885 driver version 0.0.3 loaded [ 8.921390] cx23885[0]: Your board isn't known (yet) to the driver. [ 8.921390] cx23885[0]: Try to pick one of the existing card configs via [ 8.921390] cx23885[0]: card=n insmod option. Updating to the latest [ 8.921390] cx23885[0]: version might help as well. [ 8.921393] cx23885[0]: Here is a list of valid choices for the card=n insmod option: Etc. Does anyone have any idea of the issue here? Sure. The issue is nobody cares enough to update the driver to support your card and make it work out of the box. - Steve -- Steven Toth - Kernel Labs http://www.kernellabs.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: Hauppauge ImpactVCB-e 01385
The issue is nobody cares enough to update the driver to support your card and make it work out of the box. Hi Steve, Thanks for your response. Well not nobody because I do. How do I do it? If you would guide me through, I will happily update it. Regards Steve. Steven Toth st...@kernellabs.com wrote: When I plug in my 01385 I get the same old stuff in dmseg, ie: cx23885 driver version 0.0.3 loaded [ 8.921390] cx23885[0]: Your board isn't known (yet) to the driver. [ 8.921390] cx23885[0]: Try to pick one of the existing card configs via [ 8.921390] cx23885[0]: card=n insmod option. Updating to the latest [ 8.921390] cx23885[0]: version might help as well. [ 8.921393] cx23885[0]: Here is a list of valid choices for the card=n insmod option: Etc. Does anyone have any idea of the issue here? Sure. The issue is nobody cares enough to update the driver to support your card and make it work out of the box. - Steve -- Steven Toth - Kernel Labs http://www.kernellabs.com
Re: Hauppauge ImpactVCB-e 01385
Hi Steve, Well not nobody because I do. How do I do it? If you would guide me through, I will happily update it. I seem to recall Kernel Labs offered you some commercial help with this product in the past and you declined, you apparently wanted to tackle it on your own. How's it coming along? The mailing list will welcome any patches you choose to submit. Best, - Steve -- Steven Toth - Kernel Labs http://www.kernellabs.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: [yavta PATCH 3/9] Allow supporting mem2mem devices by adding forced OUTPUT device type
Laurent Pinchart wrote: On Saturday 01 March 2014 18:18:04 Sakari Ailus wrote: The option is --output, or -o. Wouldn't it make sense to have an option to force the device type to a user- specified value instead of just an option for the output type ? -o is also usually used to select an output file, I'd like to keep it for that. Sounds good. I'll use Q for the queue type. -- Sakari Ailus sakari.ai...@iki.fi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [yavta PATCH 5/9] Allow passing file descriptors to yavta
Hi Laurent,
Thanks for the comments.
Laurent Pinchart wrote:
...
@@ -196,6 +192,16 @@ static int video_open(struct device *dev, const char
*devname, int no_query)
printf(Device %s opened.\n, devname);
+ dev-opened = 1;
+
+ return 0;
+}
+
+static int video_querycap(struct device *dev, int no_query) {
+ struct v4l2_capability cap;
+ unsigned int capabilities;
+ int ret;
+
video_querycap ends up setting the dev-type field, which isn't really the job
of a query function. Would there be a clean way to pass the fd to the
video_open() function instead ? Maybe video_open() could be split and/or
renamed to video_init() ?
Agreed. I'll separate queue type selection from querycap. As the
querycap needs to be done after opening the device, I'll put it into
another function. I'm ok with video_init(), but what would you think
about e.g. video_set_queue_type() as the function does nothing else.
--
Regards,
Sakari Ailus
sakari.ai...@iki.fi
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FOR v3.16] VSP1 fixes and features
Hi Mauro, The following changes since commit a83b93a7480441a47856dc9104bea970e84cda87: [media] em28xx-dvb: fix PCTV 461e tuner I2C binding (2014-03-31 08:02:16 -0300) are available in the git repository at: git://linuxtv.org/pinchartl/media.git vsp1/next for you to fetch changes up to 3d9b1cabc31b86f01e396a9de036398e140270e0: v4l: vsp1: Add DT support (2014-04-08 18:43:03 +0200) Given that the DT bindings are simple, that they have been acked by Sylwester and that the DT bindings maintainers have been largely unresponsive, I believe this can go in as-is. Laurent Pinchart (6): v4l: vsp1: Remove unexisting rt clocks v4l: vsp1: uds: Enable scaling of alpha layer v4l: vsp1: Support multi-input entities v4l: vsp1: Add BRU support v4l: vsp1: Add DT bindings documentation v4l: vsp1: Add DT support .../devicetree/bindings/media/renesas,vsp1.txt | 43 +++ drivers/media/platform/vsp1/Makefile| 2 +- drivers/media/platform/vsp1/vsp1.h | 3 +- drivers/media/platform/vsp1/vsp1_bru.c | 395 +++ drivers/media/platform/vsp1/vsp1_bru.h | 39 +++ drivers/media/platform/vsp1/vsp1_drv.c | 101 +++--- drivers/media/platform/vsp1/vsp1_entity.c | 57 ++-- drivers/media/platform/vsp1/vsp1_entity.h | 24 +- drivers/media/platform/vsp1/vsp1_hsit.c | 7 +- drivers/media/platform/vsp1/vsp1_lif.c | 1 - drivers/media/platform/vsp1/vsp1_lut.c | 1 - drivers/media/platform/vsp1/vsp1_regs.h | 98 ++ drivers/media/platform/vsp1/vsp1_rpf.c | 7 +- drivers/media/platform/vsp1/vsp1_rwpf.h | 4 + drivers/media/platform/vsp1/vsp1_sru.c | 1 - drivers/media/platform/vsp1/vsp1_uds.c | 4 +- drivers/media/platform/vsp1/vsp1_video.c| 26 +- drivers/media/platform/vsp1/vsp1_video.h| 1 + drivers/media/platform/vsp1/vsp1_wpf.c | 13 +- 19 files changed, 733 insertions(+), 94 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/renesas,vsp1.txt create mode 100644 drivers/media/platform/vsp1/vsp1_bru.c create mode 100644 drivers/media/platform/vsp1/vsp1_bru.h -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [yavta PATCH 6/9] Timestamp source for output buffers
Hi Laurent,
Laurent Pinchart wrote:
...
@@ -1298,7 +1302,8 @@ static struct option opts[] = {
{sleep-forever, 0, 0, OPT_SLEEP_FOREVER},
{stride, 1, 0, OPT_STRIDE},
{time-per-frame, 1, 0, 't'},
- {userptr, 0, 0, 'u'},
+ {timestamp-source, 1, 0, OPT_TSTAMP_SRC},
+ {userptr, 1, 0, 'u'},
This seems to be an unrelated change.
Oops! My bad.
{0, 0, 0, 0}
};
@@ -1487,6 +1492,17 @@ int main(int argc, char *argv[])
case OPT_STRIDE:
stride = atoi(optarg);
break;
+ case OPT_TSTAMP_SRC:
+ if (!strcmp(optarg, eof)) {
+ dev.buffer_output_flags |=
V4L2_BUF_FLAG_TSTAMP_SRC_EOF;
As the buffer_output_flags isn't used for anything else, would it make sense
to just name it timestamp_source ?
Currently not. But it could. I'm fine with the change if you insist. :-)
+ } else if (!strcmp(optarg, soe)) {
+ dev.buffer_output_flags |=
V4L2_BUF_FLAG_TSTAMP_SRC_SOE;
+ } else {
+ printf(Invalid timestamp source %s\n, optarg);
+ return 1;
+ }
+ printf(Using %s timestamp source\n, optarg);
Do we really need this printf ?
Time to add a verbose option? :-D
I'll remove it.
--
Cheers,
Sakari Ailus
sakari.ai...@iki.fi
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [yavta PATCH 7/9] Print timestamp type and source for dequeued buffers
Hi Laurent, Laurent Pinchart wrote: Hi Sakari, Thank you for the patch. Given that the timestamp type and source are not supposed to change during streaming, do we really need to print them for every frame ? When processing frames from memory to memory (COPY timestamp type), the it is entirely possible that the timestamp source changes as the flags are copied from the OUTPUT buffer to the CAPTURE buffer. These patches do not support it but it is allowed. One option would be to print the source on every frame only when the type is COPY. For a program like yavta this might be overly sophisticated IMO. :-) -- Kind regards, Sakari Ailus sakari.ai...@iki.fi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2] media: soc-camera: OF cameras
Hi Bryan, On Tue, 8 Apr 2014, Bryan Wu wrote: Thanks Josh, I think I will take you point and rework my patch again. But I need Guennadi's review firstly, Guennadi, could you please help to review it? Ok, let me double check the situation: 1. We've got this patch from you, aiming at adding OF probing support to soc-camra 2. We've got an alternative patch from Ben to do the same, his last reply to a comment to his patch was Thanks, I will look into this. 3. We've got Ben's patches for rcar-vin, that presumably work with his patch from (2) above 4. We've got Josh's patches to add OF / async probing to atmel-isi and ov2640, that are not known to work with either (1) or (2) above, so, they don't work at all, right? So, to summarise, there is a core patch from Ben, that he possibly wants to adjust, and that works with his rcar-vin OF, there is a patch from you that isn't known to work with any driver, and there are patches from Josh, that don't work, because there isn't a suitable patch available for them. I will have a look at your and Ben's soc-camera OF patches to compare them and compare them with my early code (hopefully this coming weekend), but so far it looks like only Ben's solution has a complete working stack. Am I missing something? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 41/49] rc-core: rename mutex
On Friday 04 April 2014 01:34:43 David Härdeman wrote:
Having a mutex named lock is a bit misleading.
Why? A mutex is a type of lock so what's the problem?
A little grep'ing and sed'ing reveals that out of the 1578 unique mutex names
in the kernel source I have to hand, 540 contain lock, and 921 contain
mutex.
Cheers
James
Signed-off-by: David Härdeman da...@hardeman.nu
---
drivers/media/rc/img-ir/img-ir-hw.c |4 ++-
drivers/media/rc/rc-main.c | 42
++- include/media/rc-core.h |
5 ++--
3 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c index 5bc7903..a9abbb4 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -666,11 +666,11 @@ static void img_ir_set_protocol(struct img_ir_priv
*priv, u64 proto) {
struct rc_dev *rdev = priv-hw.rdev;
- mutex_lock(rdev-lock);
+ mutex_lock(rdev-mutex);
rdev-enabled_protocols = proto;
rdev-allowed_wakeup_protocols = proto;
rdev-enabled_wakeup_protocols = proto;
- mutex_unlock(rdev-lock);
+ mutex_unlock(rdev-mutex);
}
/* Set up IR decoders */
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 7caca4f..bd4dfab 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -109,7 +109,7 @@ int rc_open(struct rc_dev *dev)
{
int err = 0;
- mutex_lock(dev-lock);
+ mutex_lock(dev-mutex);
if (dev-dead)
err = -ENODEV;
@@ -119,7 +119,7 @@ int rc_open(struct rc_dev *dev)
dev-users--;
}
- mutex_unlock(dev-lock);
+ mutex_unlock(dev-mutex);
return err;
}
@@ -127,12 +127,12 @@ EXPORT_SYMBOL_GPL(rc_open);
void rc_close(struct rc_dev *dev)
{
- mutex_lock(dev-lock);
+ mutex_lock(dev-mutex);
if (!dev-dead !--dev-users dev-close)
dev-close(dev);
- mutex_unlock(dev-lock);
+ mutex_unlock(dev-mutex);
}
EXPORT_SYMBOL_GPL(rc_close);
@@ -322,7 +322,7 @@ struct rc_filter_attribute {
* It returns the protocol names of supported protocols.
* Enabled protocols are printed in brackets.
*
- * dev-lock is taken to guard against races between store_protocols and
+ * dev-mutex is taken to guard against races between store_protocols and
* show_protocols.
*/
static ssize_t show_protocols(struct device *device,
@@ -339,7 +339,7 @@ static ssize_t show_protocols(struct device *device,
return -EINVAL;
rc_event(dev, RC_KEY, RC_KEY_REPEAT, 1);
- mutex_lock(dev-lock);
+ mutex_lock(dev-mutex);
if (fattr-type == RC_FILTER_NORMAL) {
enabled = dev-enabled_protocols;
@@ -349,7 +349,7 @@ static ssize_t show_protocols(struct device *device,
allowed = dev-allowed_wakeup_protocols;
}
- mutex_unlock(dev-lock);
+ mutex_unlock(dev-mutex);
IR_dprintk(1, %s: allowed - 0x%llx, enabled - 0x%llx\n,
__func__, (long long)allowed, (long long)enabled);
@@ -449,7 +449,7 @@ static int parse_protocol_change(u64 *protocols, const
char *buf) * See parse_protocol_change() for the valid commands.
* Returns @len on success or a negative error code.
*
- * dev-lock is taken to guard against races between store_protocols and
+ * dev-mutex is taken to guard against races between store_protocols and
* show_protocols.
*/
static ssize_t store_protocols(struct device *device,
@@ -488,7 +488,7 @@ static ssize_t store_protocols(struct device *device,
return -EINVAL;
}
- mutex_lock(dev-lock);
+ mutex_lock(dev-mutex);
old_protocols = *current_protocols;
new_protocols = old_protocols;
@@ -532,7 +532,7 @@ static ssize_t store_protocols(struct device *device,
rc = len;
out:
- mutex_unlock(dev-lock);
+ mutex_unlock(dev-mutex);
return rc;
}
@@ -550,7 +550,7 @@ out:
* Bits of the filter value corresponding to set bits in the filter mask
are * compared against input scancodes and non-matching scancodes are
discarded. *
- * dev-lock is taken to guard against races between store_filter and
+ * dev-mutex is taken to guard against races between store_filter and
* show_filter.
*/
static ssize_t show_filter(struct device *device,
@@ -571,12 +571,12 @@ static ssize_t show_filter(struct device *device,
else
filter = dev-scancode_wakeup_filter;
- mutex_lock(dev-lock);
+ mutex_lock(dev-mutex);
if (fattr-mask)
val = filter-mask;
else
val = filter-data;
- mutex_unlock(dev-lock);
+ mutex_unlock(dev-mutex);
return sprintf(buf, %#x\n, val);
}
@@ -597,7 +597,7 @@ static ssize_t show_filter(struct device *device,
* Bits of the filter value corresponding to set
Re: [RFC] Helper to abstract vma handling in media layer
On Thu 10-04-14 14:22:20, Hans Verkuil wrote: On 04/10/14 14:15, Jan Kara wrote: On Thu 10-04-14 13:07:42, Hans Verkuil wrote: On 04/10/14 12:32, Jan Kara wrote: Hello, On Thu 10-04-14 12:02:50, Marek Szyprowski wrote: On 2014-03-17 20:49, Jan Kara wrote: The following patch series is my first stab at abstracting vma handling from the various media drivers. After this patch set drivers have to know much less details about vmas, their types, and locking. My motivation for the series is that I want to change get_user_pages() locking and I want to handle subtle locking details in as few places as possible. The core of the series is the new helper get_vaddr_pfns() which is given a virtual address and it fills in PFNs into provided array. If PFNs correspond to normal pages it also grabs references to these pages. The difference from get_user_pages() is that this function can also deal with pfnmap, mixed, and io mappings which is what the media drivers need. The patches are just compile tested (since I don't have any of the hardware I'm afraid I won't be able to do any more testing anyway) so please handle with care. I'm grateful for any comments. Thanks for posting this series! I will check if it works with our hardware soon. This is something I wanted to introduce some time ago to simplify buffer handling in dma-buf, but I had no time to start working. Thanks for having a look in the series. However I would like to go even further with integration of your pfn vector idea. This structure looks like a best solution for a compact representation of the memory buffer, which should be considered by the hardware as contiguous (either contiguous in physical memory or mapped contiguously into dma address space by the respective iommu). As you already noticed it is widely used by graphics and video drivers. I would also like to add support for pfn vector directly to the dma-mapping subsystem. This can be done quite easily (even with a fallback for architectures which don't provide method for it). I will try to prepare rfc soon. This will finally remove the need for hacks in media/v4l2-core/videobuf2-dma-contig.c That would be a worthwhile thing to do. When I was reading the code this seemed like something which could be done but I delibrately avoided doing more unification than necessary for my purposes as I don't have any hardware to test and don't know all the subtleties in the code... BTW, is there some way to test the drivers without the physical video HW? You can use the vivi driver (drivers/media/platform/vivi) for this. However, while the vivi driver can import dma buffers it cannot export them. If you want that, then you have to use this tree: http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/log/?h=vb2-part4 Thanks for the pointer that looks good. I've also found drivers/media/platform/mem2mem_testdev.c which seems to do even more testing of the area I made changes to. So now I have to find some userspace tool which can issue proper ioctls to setup and use the buffers and I can start testing what I wrote :) Get the v4l-utils.git repository (http://git.linuxtv.org/cgit.cgi/v4l-utils.git/). You want the v4l2-ctl tool. Don't use the version supplied by your distro, that's often too old. 'v4l2-ctl --help-streaming' gives the available options for doing streaming. So simple capturing from vivi is 'v4l2-ctl --stream-mmap' or '--stream-user'. You can't test dmabuf unless you switch to the vb2-part4 branch of my tree. Great, it seems to be doing something and it shows there's some bug in my code. Thanks a lot for help. Honza -- Jan Kara j...@suse.cz SUSE Labs, CR -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/9] Provide -Q option for setting the queue type
Instead of guessing the queue type, allow setting it explicitly.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
yavta.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/yavta.c b/yavta.c
index 739463d..d7bccfd 100644
--- a/yavta.c
+++ b/yavta.c
@@ -1501,6 +1501,8 @@ static void usage(const char *argv0)
printf(-n, --nbufs n Set the number of video
buffers\n);
printf(-p, --pause Pause before starting the video
stream\n);
printf(-q, --quality n MJPEG quality (0-100)\n);
+ printf(-Q, --queue-typeBuffer queue type (\capture\,
\overlay\ or a\n);
+ printf(number)\n);
printf(-r, --get-control ctrl Get control 'ctrl'\n);
printf(-R, --realtime=[priority] Enable realtime RR
scheduling\n);
printf(-s, --size WxH Set the frame size\n);
@@ -1543,6 +1545,7 @@ static struct option opts[] = {
{offset, 1, 0, OPT_USERPTR_OFFSET},
{pause, 0, 0, 'p'},
{quality, 1, 0, 'q'},
+ {queue-type, 1, 0, 'Q'},
{get-control, 1, 0, 'r'},
{requeue-last, 0, 0, OPT_REQUEUE_LAST},
{realtime, 2, 0, 'R'},
@@ -1564,7 +1567,7 @@ int main(int argc, char *argv[])
/* Options parsings */
const struct v4l2_format_info *info;
- int do_file = 0, do_capture = 0, do_pause = 0;
+ int do_file = 0, do_capture = 0, do_pause = 0, queue_type = -1;
int do_set_time_per_frame = 0;
int do_enum_formats = 0, do_set_format = 0;
int do_enum_inputs = 0, do_set_input = 0;
@@ -1600,7 +1603,7 @@ int main(int argc, char *argv[])
unsigned int rt_priority = 1;
opterr = 0;
- while ((c = getopt_long(argc, argv,
c::Cd:f:F::hi:Iln:pq:r:R::s:t:uw:, opts, NULL)) != -1) {
+ while ((c = getopt_long(argc, argv,
c::Cd:f:F::hi:Iln:pq:Q:r:R::s:t:uw:, opts, NULL)) != -1) {
switch (c) {
case 'c':
@@ -1652,6 +1655,14 @@ int main(int argc, char *argv[])
case 'q':
quality = atoi(optarg);
break;
+ case 'Q':
+ if (!strcmp(optarg, capture))
+ queue_type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+ else if (!strcmp(optarg, output))
+ queue_type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
+ else
+ queue_type = atoi(optarg);
+ break;
case 'r':
ctrl_name = strtol(optarg, endptr, 0);
if (*endptr != 0) {
@@ -1761,6 +1772,9 @@ int main(int argc, char *argv[])
if (dev.type == (enum v4l2_buf_type)-1)
no_query = 1;
+ if (queue_type != -1)
+ dev.type = queue_type;
+
dev.memtype = memtype;
if (do_get_control) {
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/9] Timestamp source for output buffers
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
yavta.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/yavta.c b/yavta.c
index 6f80311..a2e0666 100644
--- a/yavta.c
+++ b/yavta.c
@@ -72,6 +72,7 @@ struct device
unsigned int width;
unsigned int height;
+ uint32_t buffer_output_flags;
unsigned char num_planes;
struct v4l2_plane_pix_format plane_fmt[VIDEO_MAX_PLANES];
@@ -809,6 +810,9 @@ static int video_queue_buffer(struct device *dev, int
index, enum buffer_fill_mo
buf.type = dev-type;
buf.memory = dev-memtype;
+ if (dev-type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+ buf.flags = dev-buffer_output_flags;
+
if (video_is_mplane(dev)) {
buf.m.planes = planes;
buf.length = dev-num_planes;
@@ -1516,6 +1520,7 @@ static void usage(const char *argv0)
printf(--no-query Don't query capabilities on
open\n);
printf(--offsetUser pointer buffer offset from
page start\n);
printf(--requeue-last Requeue the last buffers before
streamoff\n);
+ printf(--timestamp-source Set timestamp source on output
buffers [eof, soe]\n);
printf(--skip nSkip the first n frames\n);
printf(--sleep-forever Sleep forever after configuring
the device\n);
printf(--stride value Line stride in bytes\n);
@@ -1530,6 +1535,7 @@ static void usage(const char *argv0)
#define OPT_REQUEUE_LAST 262
#define OPT_STRIDE 263
#define OPT_FD 264
+#define OPT_TSTAMP_SRC 265
static struct option opts[] = {
{capture, 2, 0, 'c'},
@@ -1559,6 +1565,7 @@ static struct option opts[] = {
{sleep-forever, 0, 0, OPT_SLEEP_FOREVER},
{stride, 1, 0, OPT_STRIDE},
{time-per-frame, 1, 0, 't'},
+ {timestamp-source, 1, 0, OPT_TSTAMP_SRC},
{userptr, 0, 0, 'u'},
{0, 0, 0, 0}
};
@@ -1757,6 +1764,16 @@ int main(int argc, char *argv[])
case OPT_STRIDE:
stride = atoi(optarg);
break;
+ case OPT_TSTAMP_SRC:
+ if (!strcmp(optarg, eof)) {
+ dev.buffer_output_flags |=
V4L2_BUF_FLAG_TSTAMP_SRC_EOF;
+ } else if (!strcmp(optarg, soe)) {
+ dev.buffer_output_flags |=
V4L2_BUF_FLAG_TSTAMP_SRC_SOE;
+ } else {
+ printf(Invalid timestamp source %s\n, optarg);
+ return 1;
+ }
+ break;
case OPT_USERPTR_OFFSET:
userptr_offset = atoi(optarg);
break;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/9] Separate querying capabilities and determining buffer queue type
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
yavta.c | 78 ++-
1 file changed, 42 insertions(+), 36 deletions(-)
diff --git a/yavta.c b/yavta.c
index 5a35bab..12a6719 100644
--- a/yavta.c
+++ b/yavta.c
@@ -220,12 +220,8 @@ static const char *v4l2_format_name(unsigned int fourcc)
return name;
}
-static int video_open(struct device *dev, const char *devname, int no_query)
+static int video_open(struct device *dev, const char *devname)
{
- struct v4l2_capability cap;
- unsigned int capabilities;
- int ret;
-
dev-fd = -1;
dev-memtype = V4L2_MEMORY_MMAP;
dev-buffers = NULL;
@@ -240,39 +236,46 @@ static int video_open(struct device *dev, const char
*devname, int no_query)
printf(Device %s opened.\n, devname);
- if (no_query) {
- /* Assume capture device. */
- dev-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- return 0;
- }
+ return 0;
+}
+
+static int video_querycap(struct device *dev, unsigned int *capabilities)
+{
+ struct v4l2_capability cap;
+ int ret;
memset(cap, 0, sizeof cap);
ret = ioctl(dev-fd, VIDIOC_QUERYCAP, cap);
if (ret 0)
return 0;
- capabilities = cap.capabilities V4L2_CAP_DEVICE_CAPS
+ *capabilities = cap.capabilities V4L2_CAP_DEVICE_CAPS
? cap.device_caps : cap.capabilities;
- if (capabilities V4L2_CAP_VIDEO_CAPTURE_MPLANE)
+ printf(Device `%s' on `%s' is a video %s (%s mplanes) device.\n,
+ cap.card, cap.bus_info,
+ video_is_capture(dev) ? capture : output,
+ video_is_mplane(dev) ? with : without);
+ return 0;
+}
+
+static int video_set_queue_type(struct device *dev, unsigned int capabilities)
+{
+ if (dev-type != -1) {
+ } else if (capabilities V4L2_CAP_VIDEO_CAPTURE_MPLANE) {
dev-type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
- else if (capabilities V4L2_CAP_VIDEO_OUTPUT_MPLANE)
+ } else if (capabilities V4L2_CAP_VIDEO_OUTPUT_MPLANE) {
dev-type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
- else if (capabilities V4L2_CAP_VIDEO_CAPTURE)
+ } else if (capabilities V4L2_CAP_VIDEO_CAPTURE) {
dev-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- else if (capabilities V4L2_CAP_VIDEO_OUTPUT)
+ } else if (capabilities V4L2_CAP_VIDEO_OUTPUT) {
dev-type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
- else {
- printf(Error opening device %s: neither video capture
- nor video output supported.\n, devname);
- close(dev-fd);
+ } else {
+ printf(Device supports neither capture nor output.\n);
return -EINVAL;
}
+ printf(Using buffer queue type %d\n, dev-type);
- printf(Device `%s' on `%s' is a video %s (%s mplanes) device.\n,
- cap.card, cap.bus_info,
- video_is_capture(dev) ? capture : output,
- video_is_mplane(dev) ? with : without);
return 0;
}
@@ -1561,12 +1564,14 @@ static struct option opts[] = {
int main(int argc, char *argv[])
{
struct sched_param sched;
- struct device dev = { 0 };
+ struct device dev = { .type = -1 };
int ret;
/* Options parsings */
const struct v4l2_format_info *info;
- int do_file = 0, do_capture = 0, do_pause = 0, queue_type = -1;
+ /* Use video capture by default if query isn't done. */
+ unsigned int capabilities = V4L2_CAP_VIDEO_CAPTURE;
+ int do_file = 0, do_capture = 0, do_pause = 0;
int do_set_time_per_frame = 0;
int do_enum_formats = 0, do_set_format = 0;
int do_enum_inputs = 0, do_set_input = 0;
@@ -1656,11 +1661,11 @@ int main(int argc, char *argv[])
break;
case 'Q':
if (!strcmp(optarg, capture))
- queue_type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+ dev.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
else if (!strcmp(optarg, output))
- queue_type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
+ dev.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
else
- queue_type = atoi(optarg);
+ dev.type = atoi(optarg);
break;
case 'r':
ctrl_name = strtol(optarg, endptr, 0);
@@ -1761,18 +1766,19 @@ int main(int argc, char *argv[])
if (!do_file)
filename = NULL;
- /* Open the video device. If the device type isn't recognized, set the
-* --no-query option to avoid querying V4L2 subdevs.
-*/
- ret = video_open(dev, argv[optind], no_query);
+ ret =
[PATCH v2 8/9] Support copy timestamps
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi --- yavta.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/yavta.c b/yavta.c index 9810dcf..124dd1d 100644 --- a/yavta.c +++ b/yavta.c @@ -668,6 +668,9 @@ static void get_ts_flags(uint32_t flags, const char **ts_type, const char **ts_s case V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC: *ts_type = monotonic; break; + case V4L2_BUF_FLAG_TIMESTAMP_COPY: + *ts_type = copy; + break; default: *ts_type = invalid; } -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 9/9] Set timestamp for output buffers if the timestamp type is copy
Copy timestamp type will mean the timestamp is be copied from the source to
the destination buffer on mem-to-mem devices.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
yavta.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/yavta.c b/yavta.c
index 124dd1d..251fe40 100644
--- a/yavta.c
+++ b/yavta.c
@@ -73,6 +73,7 @@ struct device
unsigned int width;
unsigned int height;
uint32_t buffer_output_flags;
+ uint32_t timestamp_type;
unsigned char num_planes;
struct v4l2_plane_pix_format plane_fmt[VIDEO_MAX_PLANES];
@@ -756,6 +757,7 @@ static int video_alloc_buffers(struct device *dev, int
nbufs,
return ret;
}
+ dev-timestamp_type = buf.flags V4L2_BUF_FLAG_TIMESTAMP_MASK;
dev-buffers = buffers;
dev-nbufs = rb.count;
return 0;
@@ -818,8 +820,16 @@ static int video_queue_buffer(struct device *dev, int
index, enum buffer_fill_mo
buf.type = dev-type;
buf.memory = dev-memtype;
- if (dev-type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+ if (dev-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
buf.flags = dev-buffer_output_flags;
+ if (dev-timestamp_type == V4L2_BUF_FLAG_TIMESTAMP_COPY) {
+ struct timespec ts;
+
+ clock_gettime(CLOCK_MONOTONIC, ts);
+ buf.timestamp.tv_sec = ts.tv_sec;
+ buf.timestamp.tv_usec = ts.tv_nsec / 1000;
+ }
+ }
if (video_is_mplane(dev)) {
buf.m.planes = planes;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/9] Use int for buffer queue type
This makes comparisons nicer.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
yavta.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/yavta.c b/yavta.c
index 18e1709..5a35bab 100644
--- a/yavta.c
+++ b/yavta.c
@@ -64,7 +64,7 @@ struct device
{
int fd;
- enum v4l2_buf_type type;
+ int type; /* buffer queue type */
enum v4l2_memory memtype;
unsigned int nbufs;
struct buffer *buffers;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/9] Allow passing file descriptors to yavta
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
yavta.c | 46 +++---
1 file changed, 31 insertions(+), 15 deletions(-)
diff --git a/yavta.c b/yavta.c
index 12a6719..6f80311 100644
--- a/yavta.c
+++ b/yavta.c
@@ -63,6 +63,7 @@ struct buffer
struct device
{
int fd;
+ int opened;
int type; /* buffer queue type */
enum v4l2_memory memtype;
@@ -222,11 +223,6 @@ static const char *v4l2_format_name(unsigned int fourcc)
static int video_open(struct device *dev, const char *devname)
{
- dev-fd = -1;
- dev-memtype = V4L2_MEMORY_MMAP;
- dev-buffers = NULL;
- dev-type = (enum v4l2_buf_type)-1;
-
dev-fd = open(devname, O_RDWR);
if (dev-fd 0) {
printf(Error opening device %s: %s (%d).\n, devname,
@@ -236,6 +232,8 @@ static int video_open(struct device *dev, const char
*devname)
printf(Device %s opened.\n, devname);
+ dev-opened = 1;
+
return 0;
}
@@ -287,7 +285,8 @@ static void video_close(struct device *dev)
free(dev-pattern[i]);
free(dev-buffers);
- close(dev-fd);
+ if (dev-opened)
+ close(dev-fd);
}
static unsigned int get_control_type(struct device *dev, unsigned int id)
@@ -1513,6 +1512,7 @@ static void usage(const char *argv0)
printf(-w, --set-control 'ctrl value' Set control 'ctrl' to
'value'\n);
printf(--enum-formats Enumerate formats\n);
printf(--enum-inputs Enumerate inputs\n);
+ printf(--fdUse a numeric file descriptor
insted of a device\n);
printf(--no-query Don't query capabilities on
open\n);
printf(--offsetUser pointer buffer offset from
page start\n);
printf(--requeue-last Requeue the last buffers before
streamoff\n);
@@ -1529,6 +1529,7 @@ static void usage(const char *argv0)
#define OPT_USERPTR_OFFSET 261
#define OPT_REQUEUE_LAST 262
#define OPT_STRIDE 263
+#define OPT_FD 264
static struct option opts[] = {
{capture, 2, 0, 'c'},
@@ -1536,6 +1537,7 @@ static struct option opts[] = {
{delay, 1, 0, 'd'},
{enum-formats, 0, 0, OPT_ENUM_FORMATS},
{enum-inputs, 0, 0, OPT_ENUM_INPUTS},
+ {fd, 1, 0, OPT_FD},
{file, 2, 0, 'F'},
{fill-frames, 0, 0, 'I'},
{format, 1, 0, 'f'},
@@ -1564,7 +1566,11 @@ static struct option opts[] = {
int main(int argc, char *argv[])
{
struct sched_param sched;
- struct device dev = { .type = -1 };
+ struct device dev = {
+ .fd = -1,
+ .memtype = V4L2_MEMORY_MMAP,
+ .type = -1,
+ };
int ret;
/* Options parsings */
@@ -1728,6 +1734,14 @@ int main(int argc, char *argv[])
case OPT_ENUM_INPUTS:
do_enum_inputs = 1;
break;
+ case OPT_FD:
+ dev.fd = atoi(optarg);
+ if (dev.fd 0) {
+ printf(Bad file descriptor %d\n, dev.fd);
+ return 1;
+ }
+ printf(Using file descriptor %d\n, dev.fd);
+ break;
case OPT_NO_QUERY:
no_query = 1;
break;
@@ -1758,17 +1772,19 @@ int main(int argc, char *argv[])
return 1;
}
- if (optind = argc) {
- usage(argv[0]);
- return 1;
- }
-
if (!do_file)
filename = NULL;
- ret = video_open(dev, argv[optind]);
- if (ret 0)
- return 1;
+ if (dev.fd == -1) {
+ if (optind = argc) {
+ usage(argv[0]);
+ return 1;
+ } else {
+ ret = video_open(dev, argv[optind]);
+ if (ret 0)
+ return 1;
+ }
+ }
if (!no_query) {
ret = video_querycap(dev, capabilities);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/9] Print timestamp type and source for dequeued buffers
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
yavta.c | 52 ++--
1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/yavta.c b/yavta.c
index a2e0666..9810dcf 100644
--- a/yavta.c
+++ b/yavta.c
@@ -659,6 +659,30 @@ static void video_buffer_fill_userptr(struct device *dev,
struct buffer *buffer,
v4l2buf-m.planes[i].m.userptr = (unsigned long)buffer-mem[i];
}
+static void get_ts_flags(uint32_t flags, const char **ts_type, const char
**ts_source)
+{
+ switch (flags V4L2_BUF_FLAG_TIMESTAMP_MASK) {
+ case V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN:
+ *ts_type = unknown;
+ break;
+ case V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC:
+ *ts_type = monotonic;
+ break;
+ default:
+ *ts_type = invalid;
+ }
+ switch (flags V4L2_BUF_FLAG_TSTAMP_SRC_MASK) {
+ case V4L2_BUF_FLAG_TSTAMP_SRC_EOF:
+ *ts_source = EoF;
+ break;
+ case V4L2_BUF_FLAG_TSTAMP_SRC_SOE:
+ *ts_source = SoE;
+ break;
+ default:
+ *ts_source = invalid;
+ }
+}
+
static int video_alloc_buffers(struct device *dev, int nbufs,
unsigned int offset, unsigned int padding)
{
@@ -706,26 +730,7 @@ static int video_alloc_buffers(struct device *dev, int
nbufs,
strerror(errno), errno);
return ret;
}
- switch (buf.flags V4L2_BUF_FLAG_TIMESTAMP_MASK) {
- case V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN:
- ts_type = unknown;
- break;
- case V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC:
- ts_type = monotonic;
- break;
- default:
- ts_type = invalid;
- }
- switch (buf.flags V4L2_BUF_FLAG_TSTAMP_SRC_MASK) {
- case V4L2_BUF_FLAG_TSTAMP_SRC_EOF:
- ts_source = EoF;
- break;
- case V4L2_BUF_FLAG_TSTAMP_SRC_SOE:
- ts_source = SoE;
- break;
- default:
- ts_source = invalid;
- }
+ get_ts_flags(buf.flags, ts_type, ts_source);
printf(length: %u offset: %u timestamp type/source: %s/%s\n,
buf.length, buf.m.offset, ts_type, ts_source);
@@ -1393,6 +1398,7 @@ static int video_do_capture(struct device *dev, unsigned
int nframes,
last.tv_usec = start.tv_nsec / 1000;
for (i = 0; i nframes; ++i) {
+ const char *ts_type, *ts_source;
/* Dequeue a buffer. */
memset(buf, 0, sizeof buf);
memset(planes, 0, sizeof planes);
@@ -1425,10 +1431,12 @@ static int video_do_capture(struct device *dev,
unsigned int nframes,
fps = fps ? 100.0 / fps : 0.0;
clock_gettime(CLOCK_MONOTONIC, ts);
- printf(%u (%u) [%c] %u %u bytes %ld.%06ld %ld.%06ld %.3f
fps\n, i, buf.index,
+ get_ts_flags(buf.flags, ts_type, ts_source);
+ printf(%u (%u) [%c] %u %u bytes %ld.%06ld %ld.%06ld %.3f fps
tstamp type/src %s/%s\n, i, buf.index,
(buf.flags V4L2_BUF_FLAG_ERROR) ? 'E' : '-',
buf.sequence, buf.bytesused, buf.timestamp.tv_sec,
- buf.timestamp.tv_usec, ts.tv_sec, ts.tv_nsec/1000, fps);
+ buf.timestamp.tv_usec, ts.tv_sec, ts.tv_nsec/1000, fps,
+ ts_type, ts_source);
last = buf.timestamp;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/9] Zero dev in main()
This is necessary since video_open() may not be always called soon
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
yavta.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/yavta.c b/yavta.c
index d7bccfd..18e1709 100644
--- a/yavta.c
+++ b/yavta.c
@@ -226,7 +226,6 @@ static int video_open(struct device *dev, const char
*devname, int no_query)
unsigned int capabilities;
int ret;
- memset(dev, 0, sizeof *dev);
dev-fd = -1;
dev-memtype = V4L2_MEMORY_MMAP;
dev-buffers = NULL;
@@ -1562,7 +1561,7 @@ static struct option opts[] = {
int main(int argc, char *argv[])
{
struct sched_param sched;
- struct device dev;
+ struct device dev = { 0 };
int ret;
/* Options parsings */
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [yavta PATCH 5/9] Allow passing file descriptors to yavta
Hi Sakari,
On Thursday 10 April 2014 21:48:55 Sakari Ailus wrote:
Hi Laurent,
Thanks for the comments.
Laurent Pinchart wrote:
...
@@ -196,6 +192,16 @@ static int video_open(struct device *dev, const char
*devname, int no_query)
printf(Device %s opened.\n, devname);
+ dev-opened = 1;
+
+ return 0;
+}
+
+static int video_querycap(struct device *dev, int no_query) {
+ struct v4l2_capability cap;
+ unsigned int capabilities;
+ int ret;
+
video_querycap ends up setting the dev-type field, which isn't really the
job of a query function. Would there be a clean way to pass the fd to the
video_open() function instead ? Maybe video_open() could be split and/or
renamed to video_init() ?
Agreed. I'll separate queue type selection from querycap. As the
querycap needs to be done after opening the device, I'll put it into
another function. I'm ok with video_init(), but what would you think
about e.g. video_set_queue_type() as the function does nothing else.
Just thinking out loud, we need to
- initialize the device structure,
- open the device or use an externally provided fd,
- optionally query the device capabilities,
- optionally override the queue type.
Initializing the device structure must be performed unconditionally, I would
create a video_init() function for that.
Opening the device or using an externally provided fd are exclusive
operations, I would create two functions (that wouldn't do much).
Querying the device capabilities is also optional, I would create one function
for that.
Finally, overriding the queue type is of course optional and should be
implemented in its own function. We should probably return an error if the
user tries to set a queue type not reported by QUERYCAP (assuming QUERYCAP has
been called).
Ideally I'd also like to make the --no-query argument non-mandatory when
operating on subdev nodes. It has been introduced because QUERYCAP isn't
supported by subdev nodes, and it would be nice if we could detect somehow
that the device node corresponds to a subdev and automatically skip QUERYCAP
in that case.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Helper to abstract vma handling in media layer
On Thu 10-04-14 23:57:38, Jan Kara wrote: On Thu 10-04-14 14:22:20, Hans Verkuil wrote: On 04/10/14 14:15, Jan Kara wrote: On Thu 10-04-14 13:07:42, Hans Verkuil wrote: On 04/10/14 12:32, Jan Kara wrote: Hello, On Thu 10-04-14 12:02:50, Marek Szyprowski wrote: On 2014-03-17 20:49, Jan Kara wrote: The following patch series is my first stab at abstracting vma handling from the various media drivers. After this patch set drivers have to know much less details about vmas, their types, and locking. My motivation for the series is that I want to change get_user_pages() locking and I want to handle subtle locking details in as few places as possible. The core of the series is the new helper get_vaddr_pfns() which is given a virtual address and it fills in PFNs into provided array. If PFNs correspond to normal pages it also grabs references to these pages. The difference from get_user_pages() is that this function can also deal with pfnmap, mixed, and io mappings which is what the media drivers need. The patches are just compile tested (since I don't have any of the hardware I'm afraid I won't be able to do any more testing anyway) so please handle with care. I'm grateful for any comments. Thanks for posting this series! I will check if it works with our hardware soon. This is something I wanted to introduce some time ago to simplify buffer handling in dma-buf, but I had no time to start working. Thanks for having a look in the series. However I would like to go even further with integration of your pfn vector idea. This structure looks like a best solution for a compact representation of the memory buffer, which should be considered by the hardware as contiguous (either contiguous in physical memory or mapped contiguously into dma address space by the respective iommu). As you already noticed it is widely used by graphics and video drivers. I would also like to add support for pfn vector directly to the dma-mapping subsystem. This can be done quite easily (even with a fallback for architectures which don't provide method for it). I will try to prepare rfc soon. This will finally remove the need for hacks in media/v4l2-core/videobuf2-dma-contig.c That would be a worthwhile thing to do. When I was reading the code this seemed like something which could be done but I delibrately avoided doing more unification than necessary for my purposes as I don't have any hardware to test and don't know all the subtleties in the code... BTW, is there some way to test the drivers without the physical video HW? You can use the vivi driver (drivers/media/platform/vivi) for this. However, while the vivi driver can import dma buffers it cannot export them. If you want that, then you have to use this tree: http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/log/?h=vb2-part4 Thanks for the pointer that looks good. I've also found drivers/media/platform/mem2mem_testdev.c which seems to do even more testing of the area I made changes to. So now I have to find some userspace tool which can issue proper ioctls to setup and use the buffers and I can start testing what I wrote :) Get the v4l-utils.git repository (http://git.linuxtv.org/cgit.cgi/v4l-utils.git/). You want the v4l2-ctl tool. Don't use the version supplied by your distro, that's often too old. 'v4l2-ctl --help-streaming' gives the available options for doing streaming. So simple capturing from vivi is 'v4l2-ctl --stream-mmap' or '--stream-user'. You can't test dmabuf unless you switch to the vb2-part4 branch of my tree. Great, it seems to be doing something and it shows there's some bug in my code. Thanks a lot for help. OK, so after a small fix the basic functionality seems to be working. It doesn't seem there's a way to test multiplanar buffers with vivi, is there? Honza -- Jan Kara j...@suse.cz SUSE Labs, CR -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/9] Provide -Q option for setting the queue type
Hi, Sakari Ailus wrote: Instead of guessing the queue type, allow setting it explicitly. These got out a little bit too fast... what would you expect git send-email to do if you press ^C? Changes since v1: - Replace --output (-o, for which getopt handling was actually missing) option with a more generic --queue-type (-Q) option. - Cleaned up queue type setting; there are now separate functions for opening the video device, querying its capabilities and setting the queue type - The user-set timestamp source flags are no longer printed - Removed the accidental --userptr option handling change -- Kind regards, Sakari Ailus sakari.ai...@iki.fi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [yavta PATCH 7/9] Print timestamp type and source for dequeued buffers
On Thursday 10 April 2014 21:58:41 Sakari Ailus wrote: Laurent Pinchart wrote: Hi Sakari, Thank you for the patch. Given that the timestamp type and source are not supposed to change during streaming, do we really need to print them for every frame ? When processing frames from memory to memory (COPY timestamp type), the it is entirely possible that the timestamp source changes as the flags are copied from the OUTPUT buffer to the CAPTURE buffer. It's possible, but is it allowed by the V4L2 API ? These patches do not support it but it is allowed. One option would be to print the source on every frame only when the type is COPY. For a program like yavta this might be overly sophisticated IMO. :-) My concern is that this makes the lines output by yavta pretty long. -- 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: [yavta PATCH 5/9] Allow passing file descriptors to yavta
Hi Laurent, Laurent Pinchart wrote: Just thinking out loud, we need to - initialize the device structure, - open the device or use an externally provided fd, - optionally query the device capabilities, - optionally override the queue type. Initializing the device structure must be performed unconditionally, I would create a video_init() function for that. This is now performed in the beginning of main(). video_open() no longer initialises anything. Opening the device or using an externally provided fd are exclusive operations, I would create two functions (that wouldn't do much). Currently this is a few lines in main(). Querying the device capabilities is also optional, I would create one function for that. Patch 4/9. Finally, overriding the queue type is of course optional and should be implemented in its own function. We should probably return an error if the user tries to set a queue type not reported by QUERYCAP (assuming QUERYCAP has been called). The same check already done in the driver, and an error is returned if it's wrong. I'm leaning towards thinking this isn't necessary in yavta. Ideally I'd also like to make the --no-query argument non-mandatory when operating on subdev nodes. It has been introduced because QUERYCAP isn't supported by subdev nodes, and it would be nice if we could detect somehow that the device node corresponds to a subdev and automatically skip QUERYCAP in that case. It's already non-mandatory. We don't try to rocognise sub-device nodes, but failing querycap is a non-error already without this patchset. -- Regards, Sakari Ailus sakari.ai...@iki.fi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [yavta PATCH 7/9] Print timestamp type and source for dequeued buffers
Laurent Pinchart wrote: On Thursday 10 April 2014 21:58:41 Sakari Ailus wrote: Laurent Pinchart wrote: Hi Sakari, Thank you for the patch. Given that the timestamp type and source are not supposed to change during streaming, do we really need to print them for every frame ? When processing frames from memory to memory (COPY timestamp type), the it is entirely possible that the timestamp source changes as the flags are copied from the OUTPUT buffer to the CAPTURE buffer. It's possible, but is it allowed by the V4L2 API ? The spec states that: The V4L2_BUF_FLAG_TIMESTAMP_COPY timestamp type which is used by e.g. on mem-to-mem devices is an exception to the rule: the timestamp source flags are copied from the OUTPUT video buffer to the CAPTURE video buffer. These patches do not support it but it is allowed. One option would be to print the source on every frame only when the type is COPY. For a program like yavta this might be overly sophisticated IMO. :-) My concern is that this makes the lines output by yavta pretty long. True as well. I could remove type/src from the timestamp source information. That's mostly redundant anyway. Then we shouldn't exceed 80 characters per line that easily anymore. Could this be the time to add a verbose option? :-) -- Regards, Sakari Ailus sakari.ai...@iki.fi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] v4l: vsp1: Add DT support
On Tue, Mar 25, 2014 at 01:18:22PM +0100, Laurent Pinchart wrote: Hello, Gentle ping. I'll send a pull request in a week if I don't receive any comment on the DT bindings in the meantime. Hi Laurent, one way or another can you let me know if/when the bindings are accepted and I'll queue-up the shmobile patches. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] v4l: vsp1: Add DT support
Hi Simon, On Friday 11 April 2014 08:04:18 Simon Horman wrote: On Tue, Mar 25, 2014 at 01:18:22PM +0100, Laurent Pinchart wrote: Hello, Gentle ping. I'll send a pull request in a week if I don't receive any comment on the DT bindings in the meantime. Hi Laurent, one way or another can you let me know if/when the bindings are accepted and I'll queue-up the shmobile patches. Sure. The DT bindings have been acked and I've sent a pull request. I'll let you know when the patches get pulled, until then there's no guarantee that the bindings won't change. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] v4l: vsp1: Add DT support
On Fri, Apr 11, 2014 at 01:37:11AM +0200, Laurent Pinchart wrote: Hi Simon, On Friday 11 April 2014 08:04:18 Simon Horman wrote: On Tue, Mar 25, 2014 at 01:18:22PM +0100, Laurent Pinchart wrote: Hello, Gentle ping. I'll send a pull request in a week if I don't receive any comment on the DT bindings in the meantime. Hi Laurent, one way or another can you let me know if/when the bindings are accepted and I'll queue-up the shmobile patches. Sure. The DT bindings have been acked and I've sent a pull request. I'll let you know when the patches get pulled, until then there's no guarantee that the bindings won't change. Thanks, got it. -- 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: OK
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Fri Apr 11 04:00:16 CEST 2014 git branch: test git hash: a83b93a7480441a47856dc9104bea970e84cda87 gcc version:i686-linux-gcc (GCC) 4.8.2 sparse version: v0.5.0-11-g38d1124 host hardware: x86_64 host os:3.13-7.slh.1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.31.14-i686: OK linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12-i686: OK linux-3.13-i686: OK linux-3.14-i686: OK linux-2.6.31.14-x86_64: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12-x86_64: OK linux-3.13-x86_64: OK linux-3.14-x86_64: OK apps: OK spec-git: OK sparse version: v0.5.0-11-g38d1124 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2] media: soc-camera: OF cameras
Hi, Guennadi On 4/11/2014 5:18 AM, Guennadi Liakhovetski wrote: Hi Bryan, On Tue, 8 Apr 2014, Bryan Wu wrote: Thanks Josh, I think I will take you point and rework my patch again. But I need Guennadi's review firstly, Guennadi, could you please help to review it? Ok, let me double check the situation: 1. We've got this patch from you, aiming at adding OF probing support to soc-camra 2. We've got an alternative patch from Ben to do the same, his last reply to a comment to his patch was Thanks, I will look into this. 3. We've got Ben's patches for rcar-vin, that presumably work with his patch from (2) above 4. We've got Josh's patches to add OF / async probing to atmel-isi and ov2640, that are not known to work with either (1) or (2) above, so, they don't work at all, right? Right, the atmel-isi dt cannot work in those two patches unless the mclk stuff is added or adjusted. So, to summarise, there is a core patch from Ben, that he possibly wants to adjust, and that works with his rcar-vin OF, there is a patch from you that isn't known to work with any driver, and there are patches from Josh, that don't work, because there isn't a suitable patch available for them. I will have a look at your and Ben's soc-camera OF patches to compare them and compare them with my early code (hopefully this coming weekend), but so far it looks like only Ben's solution has a complete working stack. Am I missing something? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ Best Regards, Josh Wu -- 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
