[PATCH] [media] ad9389b: fix error return code in ad9389b_probe()
From: Wei Yongjun yongjun_...@trendmicro.com.cn
Fix to return a negative error code from the error handling
case instead of 0, as done elsewhere in this function.
Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn
---
drivers/media/i2c/ad9389b.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c
index 58344b6..decef36 100644
--- a/drivers/media/i2c/ad9389b.c
+++ b/drivers/media/i2c/ad9389b.c
@@ -1251,12 +1251,14 @@ static int ad9389b_probe(struct i2c_client *client,
const struct i2c_device_id *
state-edid_i2c_client = i2c_new_dummy(client-adapter, (0x7e1));
if (state-edid_i2c_client == NULL) {
v4l2_err(sd, failed to register edid i2c client\n);
+ err = -ENOMEM;
goto err_entity;
}
state-work_queue = create_singlethread_workqueue(sd-name);
if (state-work_queue == NULL) {
v4l2_err(sd, could not create workqueue\n);
+ err = -ENOMEM;
goto err_unreg;
}
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] vpif_display: fix error return code in vpif_probe()
Hi Wei, Thanks for the patch. On Mon, May 13, 2013 at 11:27 AM, Wei Yongjun weiyj...@gmail.com wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn Fix to return -ENODEV in the subdevice register error handling case instead of 0, as done elsewhere in this function. Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] vpif_capture: fix error return code in vpif_probe()
Hi Wei Thanks for the patch. On Mon, May 13, 2013 at 11:27 AM, Wei Yongjun weiyj...@gmail.com wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn Fix to return -ENODEV in the subdevice register error handling case instead of 0, as done elsewhere in this function. Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] ad9389b: fix error return code in ad9389b_probe()
On Mon May 13 2013 08:00:10 Wei Yongjun wrote:
From: Wei Yongjun yongjun_...@trendmicro.com.cn
Fix to return a negative error code from the error handling
case instead of 0, as done elsewhere in this function.
Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn
Acked-by: Hans Verkuil hans.verk...@cisco.com
Thanks!
Hans
---
drivers/media/i2c/ad9389b.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c
index 58344b6..decef36 100644
--- a/drivers/media/i2c/ad9389b.c
+++ b/drivers/media/i2c/ad9389b.c
@@ -1251,12 +1251,14 @@ static int ad9389b_probe(struct i2c_client *client,
const struct i2c_device_id *
state-edid_i2c_client = i2c_new_dummy(client-adapter, (0x7e1));
if (state-edid_i2c_client == NULL) {
v4l2_err(sd, failed to register edid i2c client\n);
+ err = -ENOMEM;
goto err_entity;
}
state-work_queue = create_singlethread_workqueue(sd-name);
if (state-work_queue == NULL) {
v4l2_err(sd, could not create workqueue\n);
+ err = -ENOMEM;
goto err_unreg;
}
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] blackfin: fix error return code in bcap_probe()
Hi Wei Yongjun,
drivers/media/platform/blackfin/bfin_capture.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/platform/blackfin/bfin_capture.c
b/drivers/media/platform/blackfin/bfin_capture.c
index 0e55b08..2d1e032 100644
--- a/drivers/media/platform/blackfin/bfin_capture.c
+++ b/drivers/media/platform/blackfin/bfin_capture.c
@@ -1070,6 +1070,7 @@ static int bcap_probe(struct platform_device *pdev)
if (!config-num_inputs) {
v4l2_err(bcap_dev-v4l2_dev,
Unable to work without input\n);
+ ret = -EINVAL;
goto err_unreg_vdev;
}
It's better to move this check to the beginning of this function as I
did in my bfin_display patch.
Scott
--
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: Introduce a new helper framework for buffer synchronization
Op 09-05-13 09:33, Inki Dae schreef: Hi all, This post introduces a new helper framework based on dma fence. And the purpose of this post is to collect other opinions and advices before RFC posting. First of all, this helper framework, called fence helper, is in progress yet so might not have enough comments in codes and also might need to be more cleaned up. Moreover, we might be missing some parts of the dma fence. However, I'd like to say that all things mentioned below has been tested with Linux platform and worked well. And tutorial for user process. just before cpu access struct dma_buf_fence *df; df-type = DMA_BUF_ACCESS_READ or DMA_BUF_ACCESS_WRITE; ioctl(fd, DMA_BUF_GET_FENCE, df); after memset or memcpy ioctl(fd, DMA_BUF_PUT_FENCE, df); NAK. Userspace doesn't need to trigger fences. It can do a buffer idle wait, and postpone submitting new commands until after it's done using the buffer. Kernel space doesn't need the root hole you created by giving a dereferencing a pointer passed from userspace. Your next exercise should be to write a security exploit from the api you created here. It's the only way to learn how to write safe code. Hint: df.ctx = mmap(..); ~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: stk1160: cannot alloc 196608 bytes
Hey Ezequiel, Thank you for taking the time to look at this, it really is appreciated. If you need anything else just let me know. Thanks!! On Sat, May 11, 2013 at 2:28 PM, Ezequiel Garcia elezegar...@gmail.com wrote: On Thu, May 9, 2013 at 1:11 PM, a b genericgroupm...@gmail.com wrote: Hi, I am seeing occasional issues when using an easycap card on our fedora 17 machine. [...] On a very quick look you seem to be getting out of memory (out of blocks of pages large enough for stk1160). Now, this may be some bug in stk1160, maybe not. I'll take a closer look in the next weeks. -- Ezequiel -- 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: stk1160: cannot alloc 196608 bytes
Hi Ezequiel, Sorry, just saw your suggestion RE: keep_buffers, i will definitely try this out and let you know how it goes. Will probably give it a few days worth of runs to see if it re-occurs. Thanks again! On Sat, May 11, 2013 at 3:40 PM, Ezequiel Garcia elezegar...@gmail.com wrote: On Sat, May 11, 2013 at 10:28 AM, Ezequiel Garcia elezegar...@gmail.com wrote: On Thu, May 9, 2013 at 1:11 PM, a b genericgroupm...@gmail.com wrote: Hi, I am seeing occasional issues when using an easycap card on our fedora 17 machine. [...] On a very quick look you seem to be getting out of memory (out of blocks of pages large enough for stk1160). Now, this may be some bug in stk1160, maybe not. I'll take a closer look in the next weeks. Could you try using keep_buffers option? This option should tell the driver to try to not release the video buffers, in an attempt to prevent memory from fragmenting. Like this: $ modprobe stk1160 keep_buffers=1 or like this to make it permanent: $ echo options stk1160 keep_buffers=1 /etc/modprobe.d/stk1160.conf Please try this, see if it solves your issue and report your results. -- Ezequiel -- 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 RFC v2 0/3] added managed media/v4l2 initialization
This is the 2nd version of managed initializations for media/v4l2.
There are small changes documented in separate patches.
Additionally to advertise this solution I suggest to look at all *_remove
functions in drivers/media/i2c/ tree. After conversion to devm_* versions
most of the *_remove routines could be removed completely.
Below grep for showing all *_remove functions from drivers/media/i2c:
grep -rPzo (?s)^(\s*)\N*_remove.*?{.*?^\1} drivers/media/i2c/ --include='*.c'
Andrzej Hajda (3):
media: added managed media entity initialization
media: added managed v4l2 control initialization
media: added managed v4l2 subdevice initialization
drivers/media/media-entity.c | 70 +
drivers/media/v4l2-core/v4l2-common.c | 10 +
drivers/media/v4l2-core/v4l2-ctrls.c | 48 ++
drivers/media/v4l2-core/v4l2-subdev.c | 52
include/media/media-entity.h |6 +++
include/media/v4l2-common.h |2 +
include/media/v4l2-ctrls.h| 31 +++
include/media/v4l2-subdev.h |5 +++
8 files changed, 224 insertions(+)
--
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 RFC v2 2/3] media: added managed v4l2 control initialization
This patch adds managed versions of initialization
and cleanup functions for v4l2 control handler.
Signed-off-by: Andrzej Hajda a.ha...@samsung.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
v2:
- added missing struct device forward declaration,
- corrected few comments
---
drivers/media/v4l2-core/v4l2-ctrls.c | 48 ++
include/media/v4l2-ctrls.h | 31 ++
2 files changed, 79 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index ebb8e48..69c9b95 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1421,6 +1421,54 @@ void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler
*hdl)
}
EXPORT_SYMBOL(v4l2_ctrl_handler_free);
+static void devm_v4l2_ctrl_handler_release(struct device *dev, void *res)
+{
+ struct v4l2_ctrl_handler **hdl = res;
+
+ v4l2_ctrl_handler_free(*hdl);
+}
+
+int devm_v4l2_ctrl_handler_init(struct device *dev,
+ struct v4l2_ctrl_handler *hdl,
+ unsigned nr_of_controls_hint)
+{
+ struct v4l2_ctrl_handler **dr;
+ int rc;
+
+ dr = devres_alloc(devm_v4l2_ctrl_handler_release, sizeof(*dr),
+ GFP_KERNEL);
+ if (!dr)
+ return -ENOMEM;
+
+ rc = v4l2_ctrl_handler_init(hdl, nr_of_controls_hint);
+ if (rc) {
+ devres_free(dr);
+ return rc;
+ }
+
+ *dr = hdl;
+ devres_add(dev, dr);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(devm_v4l2_ctrl_handler_init);
+
+static int devm_v4l2_ctrl_handler_match(struct device *dev, void *res,
+ void *data)
+{
+ struct v4l2_ctrl_handler **this = res, **hdl = data;
+
+ return *this == *hdl;
+}
+
+void devm_v4l2_ctrl_handler_free(struct device *dev,
+struct v4l2_ctrl_handler *hdl)
+{
+ WARN_ON(devres_release(dev, devm_v4l2_ctrl_handler_release,
+ devm_v4l2_ctrl_handler_match, hdl));
+}
+EXPORT_SYMBOL_GPL(devm_v4l2_ctrl_handler_free);
+
/* For backwards compatibility: V4L2_CID_PRIVATE_BASE should no longer
be used except in G_CTRL, S_CTRL, QUERYCTRL and QUERYMENU when dealing
with applications that do not use the NEXT_CTRL flag.
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 7343a27..1986b90 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -25,6 +25,7 @@
#include linux/videodev2.h
/* forward references */
+struct device;
struct file;
struct v4l2_ctrl_handler;
struct v4l2_ctrl_helper;
@@ -306,6 +307,36 @@ int v4l2_ctrl_handler_init_class(struct v4l2_ctrl_handler
*hdl,
*/
void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler *hdl);
+/*
+ * devm_v4l2_ctrl_handler_init - managed control handler initialization
+ *
+ * @dev: Device the @hdl belongs to.
+ * @hdl: The control handler.
+ * @nr_of_controls_hint: A hint of how many controls this handler is
+ * expected to refer to.
+ *
+ * This is a managed version of v4l2_ctrl_handler_init. Handler initialized
with
+ * this function will be automatically cleaned up on driver detach.
+ *
+ * If an handler initialized with this function needs to be cleaned up
+ * separately, devm_v4l2_ctrl_handler_free() must be used.
+ */
+int devm_v4l2_ctrl_handler_init(struct device *dev,
+ struct v4l2_ctrl_handler *hdl,
+ unsigned nr_of_controls_hint);
+
+/**
+ * devm_v4l2_ctrl_handler_free - managed control handler free
+ *
+ * @dev: Device the @hdl belongs to.
+ * @hdl: Handler to be cleaned up.
+ *
+ * This function should be used to manual free of an control handler
+ * initialized with devm_v4l2_ctrl_handler_init().
+ */
+void devm_v4l2_ctrl_handler_free(struct device *dev,
+struct v4l2_ctrl_handler *hdl);
+
/** v4l2_ctrl_handler_setup() - Call the s_ctrl op for all controls belonging
* to the handler to initialize the hardware to the current control values.
* @hdl: The control handler.
--
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 RFC v2 3/3] media: added managed v4l2 subdevice initialization
This patch adds managed versions of initialization
functions for v4l2 subdevices.
Signed-off-by: Andrzej Hajda a.ha...@samsung.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
v2:
- changes of v4l2-ctrls.h moved to proper patch
---
drivers/media/v4l2-core/v4l2-common.c | 10 +++
drivers/media/v4l2-core/v4l2-subdev.c | 52 +
include/media/v4l2-common.h |2 ++
include/media/v4l2-subdev.h |5
4 files changed, 69 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-common.c
b/drivers/media/v4l2-core/v4l2-common.c
index 3fed63f..96aac931 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -301,7 +301,17 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct
i2c_client *client,
}
EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
+int devm_v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client
*client,
+ const struct v4l2_subdev_ops *ops)
+{
+ int ret;
+ ret = devm_v4l2_subdev_bind(client-dev, sd);
+ if (!ret)
+ v4l2_i2c_subdev_init(sd, client, ops);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(devm_v4l2_i2c_subdev_init);
/* Load an i2c sub-device. */
struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
b/drivers/media/v4l2-core/v4l2-subdev.c
index 996c248..87ce2f6 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -474,3 +474,55 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct
v4l2_subdev_ops *ops)
#endif
}
EXPORT_SYMBOL(v4l2_subdev_init);
+
+static void devm_v4l2_subdev_release(struct device *dev, void *res)
+{
+ struct v4l2_subdev **sd = res;
+
+ v4l2_device_unregister_subdev(*sd);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ media_entity_cleanup((*sd)-entity);
+#endif
+}
+
+int devm_v4l2_subdev_bind(struct device *dev, struct v4l2_subdev *sd)
+{
+ struct v4l2_subdev **dr;
+
+ dr = devres_alloc(devm_v4l2_subdev_release, sizeof(*dr), GFP_KERNEL);
+ if (!dr)
+ return -ENOMEM;
+
+ *dr = sd;
+ devres_add(dev, dr);
+
+ return 0;
+}
+EXPORT_SYMBOL(devm_v4l2_subdev_bind);
+
+int devm_v4l2_subdev_init(struct device *dev, struct v4l2_subdev *sd,
+ const struct v4l2_subdev_ops *ops)
+{
+ int ret;
+
+ ret = devm_v4l2_subdev_bind(dev, sd);
+ if (!ret)
+ v4l2_subdev_init(sd, ops);
+ return ret;
+}
+EXPORT_SYMBOL(devm_v4l2_subdev_init);
+
+static int devm_v4l2_subdev_match(struct device *dev, void *res,
+ void *data)
+{
+ struct v4l2_subdev **this = res, **sd = data;
+
+ return *this == *sd;
+}
+
+void devm_v4l2_subdev_free(struct device *dev, struct v4l2_subdev *sd)
+{
+ WARN_ON(devres_release(dev, devm_v4l2_subdev_release,
+ devm_v4l2_subdev_match, sd));
+}
+EXPORT_SYMBOL_GPL(devm_v4l2_subdev_free);
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 1d93c48..da62e2b 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -136,6 +136,8 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct
v4l2_device *v4l2_dev,
/* Initialize a v4l2_subdev with data from an i2c_client struct */
void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
const struct v4l2_subdev_ops *ops);
+int devm_v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client
*client,
+ const struct v4l2_subdev_ops *ops);
/* Return i2c client address of v4l2_subdev. */
unsigned short v4l2_i2c_subdev_addr(struct v4l2_subdev *sd);
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 5298d67..881abdd 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -657,6 +657,11 @@ int v4l2_subdev_link_validate(struct media_link *link);
void v4l2_subdev_init(struct v4l2_subdev *sd,
const struct v4l2_subdev_ops *ops);
+int devm_v4l2_subdev_bind(struct device *dev, struct v4l2_subdev *sd);
+int devm_v4l2_subdev_init(struct device *dev, struct v4l2_subdev *sd,
+ const struct v4l2_subdev_ops *ops);
+void devm_v4l2_subdev_free(struct device *dev, struct v4l2_subdev *sd);
+
/* Call an ops of a v4l2_subdev, doing the right checks against
NULL pointers.
--
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 RFC v2 1/3] media: added managed media entity initialization
This patch adds managed versions of initialization
and cleanup functions for media entity.
Signed-off-by: Andrzej Hajda a.ha...@samsung.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
v2:
- added missing struct device forward declaration
---
drivers/media/media-entity.c | 70 ++
include/media/media-entity.h |6
2 files changed, 76 insertions(+)
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index e1cd132..696de35 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -82,9 +82,79 @@ void
media_entity_cleanup(struct media_entity *entity)
{
kfree(entity-links);
+ entity-links = NULL;
}
EXPORT_SYMBOL_GPL(media_entity_cleanup);
+static void devm_media_entity_release(struct device *dev, void *res)
+{
+ struct media_entity **entity = res;
+
+ media_entity_cleanup(*entity);
+}
+
+/**
+ * devm_media_entity_init - managed media entity initialization
+ *
+ * @dev: Device for which @entity belongs to.
+ * @entity: Entity to be initialized.
+ * @num_pads: Total number of sink and source pads.
+ * @pads: Array of 'num_pads' pads.
+ * @extra_links: Initial estimate of the number of extra links.
+ *
+ * This is a managed version of media_entity_init. Entity initialized with
+ * this function will be automatically cleaned up on driver detach.
+ *
+ * If an entity initialized with this function needs to be cleaned up
+ * separately, devm_media_entity_cleanup() must be used.
+ */
+int
+devm_media_entity_init(struct device *dev, struct media_entity *entity,
+ u16 num_pads, struct media_pad *pads, u16 extra_links)
+{
+ struct media_entity **dr;
+ int rc;
+
+ dr = devres_alloc(devm_media_entity_release, sizeof(*dr), GFP_KERNEL);
+ if (!dr)
+ return -ENOMEM;
+
+ rc = media_entity_init(entity, num_pads, pads, extra_links);
+ if (rc) {
+ devres_free(dr);
+ return rc;
+ }
+
+ *dr = entity;
+ devres_add(dev, dr);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(devm_media_entity_init);
+
+static int devm_media_entity_match(struct device *dev, void *res, void *data)
+{
+ struct media_entity **this = res, **entity = data;
+
+ return *this == *entity;
+}
+
+/**
+ * devm_media_entity_cleanup - managed media entity cleanup
+ *
+ * @dev: Device for which @entity belongs to.
+ * @entity: Entity to be cleaned up.
+ *
+ * This function should be used to manual cleanup of an media entity
+ * initialized with devm_media_entity_init().
+ */
+void devm_media_entity_cleanup(struct device *dev, struct media_entity *entity)
+{
+ WARN_ON(devres_release(dev, devm_media_entity_release,
+ devm_media_entity_match, entity));
+}
+EXPORT_SYMBOL_GPL(devm_media_entity_cleanup);
+
/*
-
* Graph traversal
*/
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 0c16f51..e3f1604 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -26,6 +26,8 @@
#include linux/list.h
#include linux/media.h
+struct device;
+
struct media_pipeline {
};
@@ -126,6 +128,10 @@ int media_entity_init(struct media_entity *entity, u16
num_pads,
struct media_pad *pads, u16 extra_links);
void media_entity_cleanup(struct media_entity *entity);
+int devm_media_entity_init(struct device *dev, struct media_entity *entity,
+ u16 num_pads, struct media_pad *pads, u16 extra_links);
+void devm_media_entity_cleanup(struct device *dev, struct media_entity
*entity);
+
int media_entity_create_link(struct media_entity *source, u16 source_pad,
struct media_entity *sink, u16 sink_pad, u32 flags);
int __media_entity_setup_link(struct media_link *link, u32 flags);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] [media] blackfin: fix error return code in bcap_probe()
From: Wei Yongjun yongjun_...@trendmicro.com.cn
Fix to return a negative error code from the error handling
case instead of 0, as done elsewhere in this function.
Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn
---
v1 - v2: move config-num_inputs check to the beginning of this function
---
drivers/media/platform/blackfin/bfin_capture.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/blackfin/bfin_capture.c
b/drivers/media/platform/blackfin/bfin_capture.c
index 0e55b08..391d9a9 100644
--- a/drivers/media/platform/blackfin/bfin_capture.c
+++ b/drivers/media/platform/blackfin/bfin_capture.c
@@ -960,7 +960,7 @@ static int bcap_probe(struct platform_device *pdev)
int ret;
config = pdev-dev.platform_data;
- if (!config) {
+ if (!config || !config-num_inputs) {
v4l2_err(pdev-dev.driver, Unable to get board config\n);
return -ENODEV;
}
@@ -1067,11 +1067,6 @@ static int bcap_probe(struct platform_device *pdev)
NULL);
if (bcap_dev-sd) {
int i;
- if (!config-num_inputs) {
- v4l2_err(bcap_dev-v4l2_dev,
- Unable to work without input\n);
- goto err_unreg_vdev;
- }
/* update tvnorms from the sub devices */
for (i = 0; i config-num_inputs; i++)
@@ -1079,6 +1074,7 @@ static int bcap_probe(struct platform_device *pdev)
} else {
v4l2_err(bcap_dev-v4l2_dev,
Unable to register sub device\n);
+ ret = -ENODEV;
goto err_unreg_vdev;
}
--
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: Introduce a new helper framework for buffer synchronization
-Original Message- From: Maarten Lankhorst [mailto:maarten.lankho...@canonical.com] Sent: Monday, May 13, 2013 5:01 PM To: Inki Dae Cc: Rob Clark; Daniel Vetter; DRI mailing list; linux-arm- ker...@lists.infradead.org; linux-media@vger.kernel.org; linux-fbdev; Kyungmin Park; myungjoo.ham; YoungJun Cho Subject: Re: Introduce a new helper framework for buffer synchronization Op 09-05-13 09:33, Inki Dae schreef: Hi all, This post introduces a new helper framework based on dma fence. And the purpose of this post is to collect other opinions and advices before RFC posting. First of all, this helper framework, called fence helper, is in progress yet so might not have enough comments in codes and also might need to be more cleaned up. Moreover, we might be missing some parts of the dma fence. However, I'd like to say that all things mentioned below has been tested with Linux platform and worked well. And tutorial for user process. just before cpu access struct dma_buf_fence *df; df-type = DMA_BUF_ACCESS_READ or DMA_BUF_ACCESS_WRITE; ioctl(fd, DMA_BUF_GET_FENCE, df); after memset or memcpy ioctl(fd, DMA_BUF_PUT_FENCE, df); NAK. Userspace doesn't need to trigger fences. It can do a buffer idle wait, and postpone submitting new commands until after it's done using the buffer. Hi Maarten, It seems that you say user should wait for a buffer like KDS does: KDS uses select() to postpone submitting new commands. But I think this way assumes that every data flows a DMA device to a CPU. For example, a CPU should keep polling for the completion of a buffer access by a DMA device. This means that the this way isn't considered for data flow to opposite case; CPU to DMA device. Kernel space doesn't need the root hole you created by giving a dereferencing a pointer passed from userspace. Your next exercise should be to write a security exploit from the api you created here. It's the only way to learn how to write safe code. Hint: df.ctx = mmap(..); Also I'm not clear to use our way yet and that is why I posted. As you mentioned, it seems like that using mmap() is more safe. But there is one issue it makes me confusing. For your hint, df.ctx = mmap(..), the issue is that dmabuf mmap can be used to map a dmabuf with user space. And the dmabuf means a physical memory region allocated by some allocator such as drm gem or ion. There might be my missing point so could you please give me more comments? Thanks, Inki Dae ~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: [PATCH RFC v2 3/3] media: added managed v4l2 subdevice initialization
Hi Andrzej,
On Mon May 13 2013 10:34:46 Andrzej Hajda wrote:
This patch adds managed versions of initialization
functions for v4l2 subdevices.
I figured out what is bothering me about this patch: the fact that it is
tied to the v4l2_i2c_subdev_init/v4l2_subdev_init functions. Normally devm
functions are wrappers around functions that actually allocate some resource.
That's not the case with these subdev_init functions, they just initialize
fields in a struct.
Why not drop those wrappers and just provide the devm_v4l2_subdev_bind
function? That's actually the one that is doing the binding, and is a function
drivers can call explicitly.
The only thing you need to add to devm_v4l2_subdev_bind is a WARN_ON check that
sd-ops != NULL, verifying that v4l2_subdev_init was called before the
bind().
I would be much happier with that solution.
Regards,
Hans
Signed-off-by: Andrzej Hajda a.ha...@samsung.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
v2:
- changes of v4l2-ctrls.h moved to proper patch
---
drivers/media/v4l2-core/v4l2-common.c | 10 +++
drivers/media/v4l2-core/v4l2-subdev.c | 52
+
include/media/v4l2-common.h |2 ++
include/media/v4l2-subdev.h |5
4 files changed, 69 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-common.c
b/drivers/media/v4l2-core/v4l2-common.c
index 3fed63f..96aac931 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -301,7 +301,17 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct
i2c_client *client,
}
EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
+int devm_v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client
*client,
+ const struct v4l2_subdev_ops *ops)
+{
+ int ret;
+ ret = devm_v4l2_subdev_bind(client-dev, sd);
+ if (!ret)
+ v4l2_i2c_subdev_init(sd, client, ops);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(devm_v4l2_i2c_subdev_init);
/* Load an i2c sub-device. */
struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
b/drivers/media/v4l2-core/v4l2-subdev.c
index 996c248..87ce2f6 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -474,3 +474,55 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const
struct v4l2_subdev_ops *ops)
#endif
}
EXPORT_SYMBOL(v4l2_subdev_init);
+
+static void devm_v4l2_subdev_release(struct device *dev, void *res)
+{
+ struct v4l2_subdev **sd = res;
+
+ v4l2_device_unregister_subdev(*sd);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+ media_entity_cleanup((*sd)-entity);
+#endif
+}
+
+int devm_v4l2_subdev_bind(struct device *dev, struct v4l2_subdev *sd)
+{
+ struct v4l2_subdev **dr;
+
+ dr = devres_alloc(devm_v4l2_subdev_release, sizeof(*dr), GFP_KERNEL);
+ if (!dr)
+ return -ENOMEM;
+
+ *dr = sd;
+ devres_add(dev, dr);
+
+ return 0;
+}
+EXPORT_SYMBOL(devm_v4l2_subdev_bind);
+
+int devm_v4l2_subdev_init(struct device *dev, struct v4l2_subdev *sd,
+ const struct v4l2_subdev_ops *ops)
+{
+ int ret;
+
+ ret = devm_v4l2_subdev_bind(dev, sd);
+ if (!ret)
+ v4l2_subdev_init(sd, ops);
+ return ret;
+}
+EXPORT_SYMBOL(devm_v4l2_subdev_init);
+
+static int devm_v4l2_subdev_match(struct device *dev, void *res,
+ void *data)
+{
+ struct v4l2_subdev **this = res, **sd = data;
+
+ return *this == *sd;
+}
+
+void devm_v4l2_subdev_free(struct device *dev, struct v4l2_subdev *sd)
+{
+ WARN_ON(devres_release(dev, devm_v4l2_subdev_release,
+devm_v4l2_subdev_match, sd));
+}
+EXPORT_SYMBOL_GPL(devm_v4l2_subdev_free);
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 1d93c48..da62e2b 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -136,6 +136,8 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct
v4l2_device *v4l2_dev,
/* Initialize a v4l2_subdev with data from an i2c_client struct */
void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
const struct v4l2_subdev_ops *ops);
+int devm_v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client
*client,
+ const struct v4l2_subdev_ops *ops);
/* Return i2c client address of v4l2_subdev. */
unsigned short v4l2_i2c_subdev_addr(struct v4l2_subdev *sd);
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 5298d67..881abdd 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -657,6 +657,11 @@ int v4l2_subdev_link_validate(struct media_link *link);
void
[PATCH] [media] omap3isp: Remove redundant platform_set_drvdata()
Commit 0998d06310 (device-core: Ensure drvdata = NULL when no driver is bound) removes the need to set driver data field to NULL. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Sakari Ailus sakari.ai...@iki.fi --- drivers/media/platform/omap3isp/isp.c |2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index 1d7dbd5..4afa421 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -2291,8 +2291,6 @@ error_isp: isp_xclk_cleanup(isp); omap3isp_put(isp); error: - platform_set_drvdata(pdev, NULL); - mutex_destroy(isp-isp_mutex); return ret; -- 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
[RFCv2] Motion Detection API
This RFC looks at adding support for motion detection to V4L2. This is the main
missing piece that prevents the go7007 and solo6x10 drivers from being moved
into mainline from the staging directory.
This is the second version of this RFC. The changes are:
- Use a new event to signal motion detection
- Make the blocks array a pointer to data to allow for a larger number of blocks
(important when dealing with HDTV in the future).
Step one is to look at existing drivers/hardware:
1) The go7007 driver:
- divides the frame into blocks of 16x16 pixels each (that's 45x36
blocks for PAL)
- each block can be assigned to region 0, 1, 2 or 3
- each region has:
- a pixel change threshold
- a motion vector change threshold
- a trigger level; if this is 0, then motion detection for this
region is disabled
- when streaming the reserved field of v4l2_buffer is used as a bitmask:
one bit for each region where motion is detected.
2) The solo6x10 driver:
- divides the frame into blocks of 16x16 pixels each
- each block has its own threshold
- the driver adds one MOTION_ON buffer flag and one MOTION_DETECTED
buffer
flag.
- motion detection can be disabled or enabled.
- the driver has a global motion detection mode with just one threshold:
in that case all blocks are set to the same threshold.
- there is also support for displaying a border around the image if
motion
is detected (very hardware specific).
3) The tw2804 video encoder (based on the datasheet, not implemented in the
driver):
- divides the image in 12x12 blocks (block size will differ for NTSC vs
PAL)
- motion detection can be enabled or disabled for each block
- there are four controls:
- luminance level change threshold
- spatial sensitivity threshold
- temporal sensitivity threshold
- velocity control (determines how well slow motions are
detected)
- detection is reported by a hardware pin in this case
Comparing these three examples of motion detection I see quite a lot of
similarities,
enough to make a proposal for an API:
- Add a MOTION_DETECTION menu control:
- Disabled
- Global Motion Detection
- Regional Motion Detection
If 'Global Motion Detection' is selected, then various threshold controls become
available. What sort of thresholds are available seems to be quite variable, so
I am inclined to leave this as private controls.
- Use a new event to report motion detection:
The go7007 driver would need 4 bits for the mask field (one for each region),
the others just one. I see no reason for adding a 'MOTION_ON' flag as the
solo6x10 driver does today: just check the MOTION_DETECTION control if you
want
to know if motion detection is on or not.
#define V4L2_EVENT_MOTION_DET 5
/**
* struct v4l2_event_motion_det - motion detection event
* @flags: if set to V4L2_EVENT_MD_VALID_FRAME, then the
* frame_sequence field is valid.
* @frame_sequence:the frame sequence number associated with this
event.
* @mask: which regions detected motion.
*/
struct v4l2_event_motion_det {
__u32 flags;
__u32 frame_sequence;
__u32 mask;
};
- Add two new ioctls to get and set the block data:
#define V4L2_MD_TYPE_REGION (1)
#define V4L2_MD_TYPE_THRESHOLD (2)
struct v4l2_md_blocks {
__u32 type;
struct v4l2_rect rect;
__u32 block_min;
__u32 block_max;
__u32 __user *blocks;
__u32 reserved[32];
} __attribute__ ((packed));
#define VIDIOC_G_MD_BLOCKS_IORW('V', 103, struct v4l2_md_blocks)
#define VIDIOC_S_MD_BLOCKS_IORW('V', 104, struct v4l2_md_blocks)
Apps must fill in type and blocks, then can call G_MD_BLOCKS to get the
current
block values for that type. TYPE_REGION returns to which region each block
belongs,
TYPE_THRESHOLD returns threshold values for each block.
If blocks == NULL, then no data is returned, but all other fields are filled
in.
rect returns the rectangle of valid blocks, and blocks_min and blocks_max are
the
min and max values for each 'blocks' array element. If type == REGION, then
blocks_max is the maximum number of regions - 1.
blocks points to a buffer of size 'sizeof(__u32) * rect.width * rect.height'.
To change the blocks apps call S_MD_BLOCKS, fill in type, rect (rect is useful
here to set only a subset of all blocks) and blocks.
So the go7007 would return 45x36 in rect, only the REGION type would be
supported,
min/max would be 0-3.
solo6x10 would return
[PATCH 1/2] [media] soc_camera/sh_mobile_csi2: Remove redundant platform_set_drvdata()
Commit 0998d06310 (device-core: Ensure drvdata = NULL when no driver is bound) removes the need to set driver data field to NULL. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/soc_camera/sh_mobile_csi2.c |8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/media/platform/soc_camera/sh_mobile_csi2.c b/drivers/media/platform/soc_camera/sh_mobile_csi2.c index 09cb4fc..13a1f8f 100644 --- a/drivers/media/platform/soc_camera/sh_mobile_csi2.c +++ b/drivers/media/platform/soc_camera/sh_mobile_csi2.c @@ -340,18 +340,13 @@ static int sh_csi2_probe(struct platform_device *pdev) ret = v4l2_device_register_subdev(pdata-v4l2_dev, priv-subdev); dev_dbg(pdev-dev, %s(%p): ret(register_subdev) = %d\n, __func__, priv, ret); if (ret 0) - goto esdreg; + return ret; pm_runtime_enable(pdev-dev); dev_dbg(pdev-dev, CSI2 probed.\n); return 0; - -esdreg: - platform_set_drvdata(pdev, NULL); - - return ret; } static int sh_csi2_remove(struct platform_device *pdev) @@ -360,7 +355,6 @@ static int sh_csi2_remove(struct platform_device *pdev) v4l2_device_unregister_subdev(priv-subdev); pm_runtime_disable(pdev-dev); - platform_set_drvdata(pdev, NULL); return 0; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] [media] soc_camera_platform: Remove redundant platform_set_drvdata()
Commit 0998d06310 (device-core: Ensure drvdata = NULL when no driver is bound) removes the need to set driver data field to NULL. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- .../platform/soc_camera/soc_camera_platform.c |7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/media/platform/soc_camera/soc_camera_platform.c b/drivers/media/platform/soc_camera/soc_camera_platform.c index 1b7a88c..bf0bdd0 100644 --- a/drivers/media/platform/soc_camera/soc_camera_platform.c +++ b/drivers/media/platform/soc_camera/soc_camera_platform.c @@ -166,14 +166,8 @@ static int soc_camera_platform_probe(struct platform_device *pdev) strncpy(priv-subdev.name, dev_name(pdev-dev), V4L2_SUBDEV_NAME_SIZE); ret = v4l2_device_register_subdev(ici-v4l2_dev, priv-subdev); - if (ret) - goto evdrs; return ret; - -evdrs: - platform_set_drvdata(pdev, NULL); - return ret; } static int soc_camera_platform_remove(struct platform_device *pdev) @@ -183,7 +177,6 @@ static int soc_camera_platform_remove(struct platform_device *pdev) p-icd-control = NULL; v4l2_device_unregister_subdev(priv-subdev); - platform_set_drvdata(pdev, NULL); return 0; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] [media] timblogiw: Remove redundant platform_set_drvdata()
Commit 0998d06310 (device-core: Ensure drvdata = NULL when no driver is bound) removes the need to set driver data field to NULL. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Cc: Pelagicore AB i...@pelagicore.com --- drivers/media/platform/timblogiw.c |4 1 file changed, 4 deletions(-) diff --git a/drivers/media/platform/timblogiw.c b/drivers/media/platform/timblogiw.c index a2f7bdd..99861c63 100644 --- a/drivers/media/platform/timblogiw.c +++ b/drivers/media/platform/timblogiw.c @@ -834,11 +834,9 @@ static int timblogiw_probe(struct platform_device *pdev) goto err_request; } - return 0; err_request: - platform_set_drvdata(pdev, NULL); v4l2_device_unregister(lw-v4l2_dev); err_register: kfree(lw); @@ -858,8 +856,6 @@ static int timblogiw_remove(struct platform_device *pdev) kfree(lw); - platform_set_drvdata(pdev, NULL); - return 0; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] [media] rc: gpio-ir-recv: Remove redundant platform_set_drvdata()
Commit 0998d06310 (device-core: Ensure drvdata = NULL when no driver is bound) removes the need to set driver data field to NULL. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/rc/gpio-ir-recv.c |2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c index 8b82ae9..07aacfa 100644 --- a/drivers/media/rc/gpio-ir-recv.c +++ b/drivers/media/rc/gpio-ir-recv.c @@ -178,7 +178,6 @@ static int gpio_ir_recv_probe(struct platform_device *pdev) return 0; err_request_irq: - platform_set_drvdata(pdev, NULL); rc_unregister_device(rcdev); rcdev = NULL; err_register_rc_device: @@ -196,7 +195,6 @@ static int gpio_ir_recv_remove(struct platform_device *pdev) struct gpio_rc_dev *gpio_dev = platform_get_drvdata(pdev); free_irq(gpio_to_irq(gpio_dev-gpio_nr), gpio_dev); - platform_set_drvdata(pdev, NULL); rc_unregister_device(gpio_dev-rcdev); gpio_free(gpio_dev-gpio_nr); kfree(gpio_dev); -- 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 v2] [media] blackfin: fix error return code in bcap_probe()
2013/5/13 Wei Yongjun weiyj...@gmail.com:
From: Wei Yongjun yongjun_...@trendmicro.com.cn
Fix to return a negative error code from the error handling
case instead of 0, as done elsewhere in this function.
Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn
---
v1 - v2: move config-num_inputs check to the beginning of this function
---
drivers/media/platform/blackfin/bfin_capture.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/blackfin/bfin_capture.c
b/drivers/media/platform/blackfin/bfin_capture.c
index 0e55b08..391d9a9 100644
--- a/drivers/media/platform/blackfin/bfin_capture.c
+++ b/drivers/media/platform/blackfin/bfin_capture.c
@@ -960,7 +960,7 @@ static int bcap_probe(struct platform_device *pdev)
int ret;
config = pdev-dev.platform_data;
- if (!config) {
+ if (!config || !config-num_inputs) {
v4l2_err(pdev-dev.driver, Unable to get board config\n);
return -ENODEV;
}
@@ -1067,11 +1067,6 @@ static int bcap_probe(struct platform_device *pdev)
NULL);
if (bcap_dev-sd) {
int i;
- if (!config-num_inputs) {
- v4l2_err(bcap_dev-v4l2_dev,
- Unable to work without input\n);
- goto err_unreg_vdev;
- }
/* update tvnorms from the sub devices */
for (i = 0; i config-num_inputs; i++)
@@ -1079,6 +1074,7 @@ static int bcap_probe(struct platform_device *pdev)
} else {
v4l2_err(bcap_dev-v4l2_dev,
Unable to register sub device\n);
+ ret = -ENODEV;
goto err_unreg_vdev;
}
Acked-by: Scott Jiang scott.jiang.li...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v3] media: davinci: kconfig: fix incorrect selects
On Tue, 2013-03-12 at 09:14 +, Sekhar Nori wrote: drivers/media/platform/davinci/Kconfig uses selects where it should be using 'depends on'. This results in warnings of the following sort when doing randconfig builds. warning: (VIDEO_DM6446_CCDC VIDEO_DM355_CCDC VIDEO_ISIF VIDEO_DAVINCI_VPBE_DISPLAY) selects VIDEO_VPSS_SYSTEM which has unmet direct dependencies (MEDIA_SUPPORT V4L_PLATFORM_DRIVERS ARCH_DAVINCI) The VPIF kconfigs had a strange 'select' and 'depends on' cross linkage which have been fixed as well by removing unneeded VIDEO_DAVINCI_VPIF config symbol. Similarly, remove the unnecessary VIDEO_VPSS_SYSTEM and VIDEO_VPFE_CAPTURE. They don't select any independent functionality and were being used to manage code dependencies which can be handled using makefile. Selecting video modules is now dependent on all ARCH_DAVINCI instead of specific EVMs and SoCs earlier. This should help build coverage. Remove unnecessary 'default y' for some config symbols. While at it, fix the Kconfig help text to make it more readable and fix names of modules created during module build. Rename VIDEO_ISIF to VIDEO_DM365_ISIF as per suggestion from Prabhakar. This patch has only been build tested; I have tried to not break any existing assumptions. I do not have the setup to test video, so any test reports welcome. Reported-by: Russell King rmk+ker...@arm.linux.org.uk Signed-off-by: Sekhar Nori nsek...@ti.com Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com This seems to be the patch that ended up as mainline commit 3778d05036cc7ddd983ae2451da579af00acdac2 (which was included in v3.10-rc1). After that commit there's still one reference to VIDEO_VPFE_CAPTURE in the tree: as a (negative) dependency in drivers/staging/media/davinci_vpfe/Kconfig. Can that (negative) dependency now be dropped (as it's currently useless) or should it be replaced with a (negative) dependency on a related symbol? Paul Bolle -- 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: Introduce a new helper framework for buffer synchronization
Op 13-05-13 11:21, Inki Dae schreef: -Original Message- From: Maarten Lankhorst [mailto:maarten.lankho...@canonical.com] Sent: Monday, May 13, 2013 5:01 PM To: Inki Dae Cc: Rob Clark; Daniel Vetter; DRI mailing list; linux-arm- ker...@lists.infradead.org; linux-media@vger.kernel.org; linux-fbdev; Kyungmin Park; myungjoo.ham; YoungJun Cho Subject: Re: Introduce a new helper framework for buffer synchronization Op 09-05-13 09:33, Inki Dae schreef: Hi all, This post introduces a new helper framework based on dma fence. And the purpose of this post is to collect other opinions and advices before RFC posting. First of all, this helper framework, called fence helper, is in progress yet so might not have enough comments in codes and also might need to be more cleaned up. Moreover, we might be missing some parts of the dma fence. However, I'd like to say that all things mentioned below has been tested with Linux platform and worked well. And tutorial for user process. just before cpu access struct dma_buf_fence *df; df-type = DMA_BUF_ACCESS_READ or DMA_BUF_ACCESS_WRITE; ioctl(fd, DMA_BUF_GET_FENCE, df); after memset or memcpy ioctl(fd, DMA_BUF_PUT_FENCE, df); NAK. Userspace doesn't need to trigger fences. It can do a buffer idle wait, and postpone submitting new commands until after it's done using the buffer. Hi Maarten, It seems that you say user should wait for a buffer like KDS does: KDS uses select() to postpone submitting new commands. But I think this way assumes that every data flows a DMA device to a CPU. For example, a CPU should keep polling for the completion of a buffer access by a DMA device. This means that the this way isn't considered for data flow to opposite case; CPU to DMA device. Not really. You do both things the same way. You first wait for the bo to be idle, this could be implemented by adding poll support to the dma-buf fd. Then you either do your read or write. Since userspace is supposed to be the one controlling the bo it should stay idle at that point. If you have another thread queueing the buffer againbefore your thread is done that's a bug in the application, and can be solved with userspace locking primitives. No need for the kernel to get involved. Kernel space doesn't need the root hole you created by giving a dereferencing a pointer passed from userspace. Your next exercise should be to write a security exploit from the api you created here. It's the only way to learn how to write safe code. Hint: df.ctx = mmap(..); Also I'm not clear to use our way yet and that is why I posted. As you mentioned, it seems like that using mmap() is more safe. But there is one issue it makes me confusing. For your hint, df.ctx = mmap(..), the issue is that dmabuf mmap can be used to map a dmabuf with user space. And the dmabuf means a physical memory region allocated by some allocator such as drm gem or ion. There might be my missing point so could you please give me more comments? My point was that userspace could change df.ctx to some mmap'd memory, forcing the kernel to execute some code prepared by userspace. -- 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: [v3] media: davinci: kconfig: fix incorrect selects
Hi Paul, On Mon, May 13, 2013 at 3:11 PM, Paul Bolle pebo...@tiscali.nl wrote: On Tue, 2013-03-12 at 09:14 +, Sekhar Nori wrote: [Snip] This patch has only been build tested; I have tried to not break any existing assumptions. I do not have the setup to test video, so any test reports welcome. Reported-by: Russell King rmk+ker...@arm.linux.org.uk Signed-off-by: Sekhar Nori nsek...@ti.com Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com This seems to be the patch that ended up as mainline commit 3778d05036cc7ddd983ae2451da579af00acdac2 (which was included in v3.10-rc1). After that commit there's still one reference to VIDEO_VPFE_CAPTURE in the tree: as a (negative) dependency in drivers/staging/media/davinci_vpfe/Kconfig. Can that (negative) dependency now be dropped (as it's currently useless) or should it be replaced with a (negative) dependency on a related symbol? Good catch! the dependency can be dropped now. Are you planning to post a patch for it or shall I do it ? Regards, --Prabhakar Lad -- 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: [v3] media: davinci: kconfig: fix incorrect selects
Prabhakar, On Mon, 2013-05-13 at 15:27 +0530, Prabhakar Lad wrote: Good catch! the dependency can be dropped now. Great. Are you planning to post a patch for it or shall I do it ? I don't mind submitting that trivial patch. However, it's probably better if you do that. I can only state that this dependency is now useless, because that is simply how the kconfig system works. But you can probably elaborate why it's OK to not replace it with another (negative) dependency. That would make a more informative commit explanation. Paul Bolle -- 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] drivers/staging: davinci: vpfe: fix dependency for building the driver
From: Lad, Prabhakar prabhakar.cse...@gmail.com from commit 3778d05036cc7ddd983ae2451da579af00acdac2 [media: davinci: kconfig: fix incorrect selects] VIDEO_VPFE_CAPTURE was removed but there was a negative dependancy for building the DM365 VPFE MC based capture driver (VIDEO_DM365_VPFE), This patch fixes this dependency by replacing the VIDEO_VPFE_CAPTURE with VIDEO_DM365_ISIF, so as when older DM365 ISIF v4l driver is selected the newer media controller driver for DM365 isnt visible. Reported-by: Paul Bolle pebo...@tiscali.nl Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- drivers/staging/media/davinci_vpfe/Kconfig |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/staging/media/davinci_vpfe/Kconfig b/drivers/staging/media/davinci_vpfe/Kconfig index 2e4a28b..12f321d 100644 --- a/drivers/staging/media/davinci_vpfe/Kconfig +++ b/drivers/staging/media/davinci_vpfe/Kconfig @@ -1,6 +1,6 @@ config VIDEO_DM365_VPFE tristate DM365 VPFE Media Controller Capture Driver - depends on VIDEO_V4L2 ARCH_DAVINCI_DM365 !VIDEO_VPFE_CAPTURE + depends on VIDEO_V4L2 ARCH_DAVINCI_DM365 !VIDEO_DM365_ISIF select VIDEOBUF2_DMA_CONTIG help Support for DM365 VPFE based Media Controller Capture driver. -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v3] media: davinci: kconfig: fix incorrect selects
Hi Paul, On Mon, May 13, 2013 at 3:35 PM, Paul Bolle pebo...@tiscali.nl wrote: Prabhakar, On Mon, 2013-05-13 at 15:27 +0530, Prabhakar Lad wrote: Good catch! the dependency can be dropped now. Great. Are you planning to post a patch for it or shall I do it ? I don't mind submitting that trivial patch. However, it's probably better if you do that. I can only state that this dependency is now useless, because that is simply how the kconfig system works. But you can probably elaborate why it's OK to not replace it with another (negative) dependency. That would make a more informative commit explanation. Posted the patch fixing it https://patchwork.linuxtv.org/patch/18395/ Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v3] media: i2c: mt9p031: add OF support
On Wed, May 08, 2013 at 12:37:29PM +0200, Laurent Pinchart wrote:
Hi Prabhakar,
On Wednesday 08 May 2013 10:19:57 Prabhakar Lad wrote:
On Wed, May 8, 2013 at 7:32 AM, Laurent Pinchart wrote:
On Tuesday 07 May 2013 15:10:36 Prabhakar Lad wrote:
On Mon, May 6, 2013 at 8:29 PM, Prabhakar Lad wrote:
On Fri, May 3, 2013 at 8:04 PM, Arnd Bergmann a...@arndb.de wrote:
On Friday 03 May 2013, Prabhakar Lad wrote:
[snip]
+}
Ok, good.
@@ -955,7 +998,17 @@ static int mt9p031_probe(struct i2c_client
*client,
mt9p031-pdata = pdata;
mt9p031-output_control = MT9P031_OUTPUT_CONTROL_DEF;
mt9p031-mode2 = MT9P031_READ_MODE_2_ROW_BLC;
- mt9p031-model = did-driver_data;
+
+ if (!client-dev.of_node) {
+ mt9p031-model = (enum
mt9p031_model)did-driver_data;
+ } else {
+ const struct of_device_id *of_id;
+
+ of_id =
of_match_device(of_match_ptr(mt9p031_of_match),
+ client-dev);
+ if (of_id)
+ mt9p031-model = (enum
mt9p031_model)of_id-data;
+ }
mt9p031-reset = -1;
Is this actually required? I thought the i2c core just compared the
part of the compatible value after the first comma to the string, so
mt9p031-model = (enum mt9p031_model)did-driver_data should work
in both cases.
At least on v3.8 I just checked that 'did' is indeed NULL for the
devicetree case. Also I see no indication that i2c starts comparing
after the first comma in the string.
I am OK with mt9p031-model = (enum mt9p031_model)did-driver_data
but I see still few drivers doing this, I am not sure for what reason.
If everyone is OK with it I can drop the above change.
My bad, while booting with DT the i2c_device_id ie did in this case will
be NULL, so the above changes are required :-)
I've just tested your patch, and did isn't NULL when booting my
Beagleboard with DT (on v3.9-rc5).
I am pretty much sure you tested it compatible property as aptina,mt9p031
if the compatible property is set to aptina,mt9p031m the did will be NULL.
I've tested both :-)
Sorry to nag, but did you use aptina,mt9p031[m] as a compatible string or did
you use mt9p031[m]. With aptina,... 'did' should really be NULL.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] HDPVR series of patches to replace Apr 25 series
Hi Hans, This series of patches replace the previous series sent on Apr 25. This adds the check you requested for ret byte count in get_video_info(). Thank you, -Leo. Leonid Kegulskiy (4): [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init() [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info() [media] hdpvr: Added some error handling in hdpvr_start_streaming() [media] hdpvr: Cleaned up error handling drivers/media/usb/hdpvr/hdpvr-control.c | 22 +++--- drivers/media/usb/hdpvr/hdpvr-core.c|8 drivers/media/usb/hdpvr/hdpvr-video.c | 70 +-- drivers/media/usb/hdpvr/hdpvr.h |2 +- 4 files changed, 46 insertions(+), 56 deletions(-) -- 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/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info()
Signed-off-by: Leonid Kegulskiy l...@lumanate.com
---
drivers/media/usb/hdpvr/hdpvr-control.c | 25 ++
drivers/media/usb/hdpvr/hdpvr-video.c | 54 +++
drivers/media/usb/hdpvr/hdpvr.h |2 +-
3 files changed, 37 insertions(+), 44 deletions(-)
diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c
b/drivers/media/usb/hdpvr/hdpvr-control.c
index ae8f229..7d1bfb3 100644
--- a/drivers/media/usb/hdpvr/hdpvr-control.c
+++ b/drivers/media/usb/hdpvr/hdpvr-control.c
@@ -45,20 +45,10 @@ int hdpvr_config_call(struct hdpvr_device *dev, uint value,
u8 valbuf)
return ret 0 ? ret : 0;
}
-struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev)
+int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf)
{
- struct hdpvr_video_info *vidinf = NULL;
-#ifdef HDPVR_DEBUG
- char print_buf[15];
-#endif
int ret;
- vidinf = kzalloc(sizeof(struct hdpvr_video_info), GFP_KERNEL);
- if (!vidinf) {
- v4l2_err(dev-v4l2_dev, out of memory\n);
- goto err;
- }
-
mutex_lock(dev-usbc_mutex);
ret = usb_control_msg(dev-udev,
usb_rcvctrlpipe(dev-udev, 0),
@@ -74,6 +64,7 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device
*dev)
#ifdef HDPVR_DEBUG
if (hdpvr_debug MSG_INFO) {
+ char print_buf[15];
hex_dump_to_buffer(dev-usbc_buf, 5, 16, 1, print_buf,
sizeof(print_buf), 0);
v4l2_dbg(MSG_INFO, hdpvr_debug, dev-v4l2_dev,
@@ -82,12 +73,14 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device
*dev)
#endif
mutex_unlock(dev-usbc_mutex);
- if (!vidinf-width || !vidinf-height || !vidinf-fps) {
- kfree(vidinf);
- vidinf = NULL;
+ if ((ret0 ret!=5) ||/* fail if unexpected byte count returned */
+ !vidinf-width || /* preserve original behavior - */
+ !vidinf-height || /* fail if no signal is detected */
+ !vidinf-fps) {
+ ret = -EFAULT;
}
-err:
- return vidinf;
+
+ return ret 0 ? ret : 0;
}
int get_input_lines_info(struct hdpvr_device *dev)
diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c
b/drivers/media/usb/hdpvr/hdpvr-video.c
index 774ba0e..d9eb8e1 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -277,20 +277,19 @@ error:
static int hdpvr_start_streaming(struct hdpvr_device *dev)
{
int ret;
- struct hdpvr_video_info *vidinf;
+ struct hdpvr_video_info vidinf;
if (dev-status == STATUS_STREAMING)
return 0;
else if (dev-status != STATUS_IDLE)
return -EAGAIN;
- vidinf = get_video_info(dev);
+ ret = get_video_info(dev, vidinf);
- if (vidinf) {
+ if (!ret) {
v4l2_dbg(MSG_BUFFER, hdpvr_debug, dev-v4l2_dev,
-video signal: %dx%d@%dhz\n, vidinf-width,
-vidinf-height, vidinf-fps);
- kfree(vidinf);
+video signal: %dx%d@%dhz\n, vidinf.width,
+vidinf.height, vidinf.fps);
/* start streaming 2 request */
ret = usb_control_msg(dev-udev,
@@ -606,21 +605,22 @@ static int vidioc_g_std(struct file *file, void *_fh,
static int vidioc_querystd(struct file *file, void *_fh, v4l2_std_id *a)
{
struct hdpvr_device *dev = video_drvdata(file);
- struct hdpvr_video_info *vid_info;
+ struct hdpvr_video_info vid_info;
struct hdpvr_fh *fh = _fh;
+ int ret;
*a = V4L2_STD_ALL;
if (dev-options.video_input == HDPVR_COMPONENT)
return fh-legacy_mode ? 0 : -ENODATA;
- vid_info = get_video_info(dev);
- if (vid_info == NULL)
+ ret = get_video_info(dev, vid_info);
+ if (ret)
return 0;
- if (vid_info-width == 720
- (vid_info-height == 480 || vid_info-height == 576)) {
- *a = (vid_info-height == 480) ?
+ if (vid_info.width == 720
+ (vid_info.height == 480 || vid_info.height == 576)) {
+ *a = (vid_info.height == 480) ?
V4L2_STD_525_60 : V4L2_STD_625_50;
}
- kfree(vid_info);
+
return 0;
}
@@ -665,7 +665,7 @@ static int vidioc_query_dv_timings(struct file *file, void
*_fh,
{
struct hdpvr_device *dev = video_drvdata(file);
struct hdpvr_fh *fh = _fh;
- struct hdpvr_video_info *vid_info;
+ struct hdpvr_video_info vid_info;
bool interlaced;
int ret = 0;
int i;
@@ -673,10 +673,10 @@ static int vidioc_query_dv_timings(struct file *file,
void *_fh,
fh-legacy_mode = false;
if (dev-options.video_input)
return -ENODATA;
- vid_info = get_video_info(dev);
-
[PATCH 1/4] [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init()
Signed-off-by: Leonid Kegulskiy l...@lumanate.com
---
drivers/media/usb/hdpvr/hdpvr-core.c |8
1 file changed, 8 deletions(-)
diff --git a/drivers/media/usb/hdpvr/hdpvr-core.c
b/drivers/media/usb/hdpvr/hdpvr-core.c
index 8247c19..cb69405 100644
--- a/drivers/media/usb/hdpvr/hdpvr-core.c
+++ b/drivers/media/usb/hdpvr/hdpvr-core.c
@@ -220,7 +220,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev)
{
int ret;
u8 *buf;
- struct hdpvr_video_info *vidinf;
if (device_authorization(dev))
return -EACCES;
@@ -242,13 +241,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev)
control request returned %d\n, ret);
mutex_unlock(dev-usbc_mutex);
- vidinf = get_video_info(dev);
- if (!vidinf)
- v4l2_dbg(MSG_INFO, hdpvr_debug, dev-v4l2_dev,
- no valid video signal or device init failed\n);
- else
- kfree(vidinf);
-
/* enable fan and bling leds */
mutex_lock(dev-usbc_mutex);
buf[0] = 0x1;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] [media] hdpvr: Added some error handling in hdpvr_start_streaming()
Signed-off-by: Leonid Kegulskiy l...@lumanate.com --- drivers/media/usb/hdpvr/hdpvr-video.c |6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c index d9eb8e1..2d02b49 100644 --- a/drivers/media/usb/hdpvr/hdpvr-video.c +++ b/drivers/media/usb/hdpvr/hdpvr-video.c @@ -297,8 +297,12 @@ static int hdpvr_start_streaming(struct hdpvr_device *dev) 0xb8, 0x38, 0x1, 0, NULL, 0, 8000); v4l2_dbg(MSG_BUFFER, hdpvr_debug, dev-v4l2_dev, encoder start control request returned %d\n, ret); + if (ret 0) + return ret; - hdpvr_config_call(dev, CTRL_START_STREAMING_VALUE, 0x00); + ret = hdpvr_config_call(dev, CTRL_START_STREAMING_VALUE, 0x00); + if (ret) + return ret; dev-status = STATUS_STREAMING; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] [media] hdpvr: Cleaned up error handling
Signed-off-by: Leonid Kegulskiy l...@lumanate.com
---
drivers/media/usb/hdpvr/hdpvr-control.c |5 +
drivers/media/usb/hdpvr/hdpvr-video.c | 10 +++---
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c
b/drivers/media/usb/hdpvr/hdpvr-control.c
index 7d1bfb3..583be15 100644
--- a/drivers/media/usb/hdpvr/hdpvr-control.c
+++ b/drivers/media/usb/hdpvr/hdpvr-control.c
@@ -73,10 +73,7 @@ int get_video_info(struct hdpvr_device *dev, struct
hdpvr_video_info *vidinf)
#endif
mutex_unlock(dev-usbc_mutex);
- if ((ret0 ret!=5) ||/* fail if unexpected byte count returned */
- !vidinf-width || /* preserve original behavior - */
- !vidinf-height || /* fail if no signal is detected */
- !vidinf-fps) {
+ if (ret0 ret!=5) { /* fail if unexpected byte count returned */
ret = -EFAULT;
}
diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c
b/drivers/media/usb/hdpvr/hdpvr-video.c
index 2d02b49..5e8d6c2 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -285,8 +285,10 @@ static int hdpvr_start_streaming(struct hdpvr_device *dev)
return -EAGAIN;
ret = get_video_info(dev, vidinf);
+ if (ret)/* device is dead */
+ return ret; /* let the caller know */
- if (!ret) {
+ if (vidinf.width vidinf.height) {
v4l2_dbg(MSG_BUFFER, hdpvr_debug, dev-v4l2_dev,
video signal: %dx%d@%dhz\n, vidinf.width,
vidinf.height, vidinf.fps);
@@ -618,7 +620,7 @@ static int vidioc_querystd(struct file *file, void *_fh,
v4l2_std_id *a)
return fh-legacy_mode ? 0 : -ENODATA;
ret = get_video_info(dev, vid_info);
if (ret)
- return 0;
+ return ret;
if (vid_info.width == 720
(vid_info.height == 480 || vid_info.height == 576)) {
*a = (vid_info.height == 480) ?
@@ -679,6 +681,8 @@ static int vidioc_query_dv_timings(struct file *file, void
*_fh,
return -ENODATA;
ret = get_video_info(dev, vid_info);
if (ret)
+ return ret;
+ if (vid_info.fps == 0)
return -ENOLCK;
interlaced = vid_info.fps = 30;
for (i = 0; i ARRAY_SIZE(hdpvr_dv_timings); i++) {
@@ -1009,7 +1013,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void
*_fh,
ret = get_video_info(dev, vid_info);
if (ret)
- return -EFAULT;
+ return ret;
f-fmt.pix.width = vid_info.width;
f-fmt.pix.height = vid_info.height;
} else {
--
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: Introduce a new helper framework for buffer synchronization
-Original Message- From: Maarten Lankhorst [mailto:maarten.lankho...@canonical.com] Sent: Monday, May 13, 2013 6:52 PM To: Inki Dae Cc: 'Rob Clark'; 'Daniel Vetter'; 'DRI mailing list'; linux-arm- ker...@lists.infradead.org; linux-media@vger.kernel.org; 'linux-fbdev'; 'Kyungmin Park'; 'myungjoo.ham'; 'YoungJun Cho' Subject: Re: Introduce a new helper framework for buffer synchronization Op 13-05-13 11:21, Inki Dae schreef: -Original Message- From: Maarten Lankhorst [mailto:maarten.lankho...@canonical.com] Sent: Monday, May 13, 2013 5:01 PM To: Inki Dae Cc: Rob Clark; Daniel Vetter; DRI mailing list; linux-arm- ker...@lists.infradead.org; linux-media@vger.kernel.org; linux-fbdev; Kyungmin Park; myungjoo.ham; YoungJun Cho Subject: Re: Introduce a new helper framework for buffer synchronization Op 09-05-13 09:33, Inki Dae schreef: Hi all, This post introduces a new helper framework based on dma fence. And the purpose of this post is to collect other opinions and advices before RFC posting. First of all, this helper framework, called fence helper, is in progress yet so might not have enough comments in codes and also might need to be more cleaned up. Moreover, we might be missing some parts of the dma fence. However, I'd like to say that all things mentioned below has been tested with Linux platform and worked well. And tutorial for user process. just before cpu access struct dma_buf_fence *df; df-type = DMA_BUF_ACCESS_READ or DMA_BUF_ACCESS_WRITE; ioctl(fd, DMA_BUF_GET_FENCE, df); after memset or memcpy ioctl(fd, DMA_BUF_PUT_FENCE, df); NAK. Userspace doesn't need to trigger fences. It can do a buffer idle wait, and postpone submitting new commands until after it's done using the buffer. Hi Maarten, It seems that you say user should wait for a buffer like KDS does: KDS uses select() to postpone submitting new commands. But I think this way assumes that every data flows a DMA device to a CPU. For example, a CPU should keep polling for the completion of a buffer access by a DMA device. This means that the this way isn't considered for data flow to opposite case; CPU to DMA device. Not really. You do both things the same way. You first wait for the bo to be idle, this could be implemented by adding poll support to the dma-buf fd. Then you either do your read or write. Since userspace is supposed to be the one controlling the bo it should stay idle at that point. If you have another thread queueing the buffer againbefore your thread is done that's a bug in the application, and can be solved with userspace locking primitives. No need for the kernel to get involved. Yes, that is how we have synchronized buffer between CPU and DMA device until now without buffer synchronization mechanism. I thought that it's best to make user not considering anything: user can access a buffer regardless of any DMA device controlling and the buffer synchronization is performed in kernel level. Moreover, I think we could optimize graphics and multimedia hardware performance because hardware can do more works: one thread accesses a shared buffer and the other controls DMA device with the shared buffer in parallel. Thus, we could avoid sequential processing and that is my intention. Aren't you think about that we could improve hardware utilization with such way or other? of course, there could be better way. Kernel space doesn't need the root hole you created by giving a dereferencing a pointer passed from userspace. Your next exercise should be to write a security exploit from the api you created here. It's the only way to learn how to write safe code. Hint: df.ctx = mmap(..); Also I'm not clear to use our way yet and that is why I posted. As you mentioned, it seems like that using mmap() is more safe. But there is one issue it makes me confusing. For your hint, df.ctx = mmap(..), the issue is that dmabuf mmap can be used to map a dmabuf with user space. And the dmabuf means a physical memory region allocated by some allocator such as drm gem or ion. There might be my missing point so could you please give me more comments? My point was that userspace could change df.ctx to some mmap'd memory, forcing the kernel to execute some code prepared by userspace. Understood. I have to find a better way. And for this, I'd like to listen attentively more opinions and advices. Thanks for comments, Inki Dae -- 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: Introduce a new helper framework for buffer synchronization
Op 13-05-13 13:24, Inki Dae schreef: and can be solved with userspace locking primitives. No need for the kernel to get involved. Yes, that is how we have synchronized buffer between CPU and DMA device until now without buffer synchronization mechanism. I thought that it's best to make user not considering anything: user can access a buffer regardless of any DMA device controlling and the buffer synchronization is performed in kernel level. Moreover, I think we could optimize graphics and multimedia hardware performance because hardware can do more works: one thread accesses a shared buffer and the other controls DMA device with the shared buffer in parallel. Thus, we could avoid sequential processing and that is my intention. Aren't you think about that we could improve hardware utilization with such way or other? of course, there could be better way. If you don't want to block the hardware the only option is to allocate a temp bo and blit to/from it using the hardware. OpenGL already does that when you want to read back, because otherwise the entire pipeline can get stalled. The overhead of command submission for that shouldn't be high, if it is you should really try to optimize that first before coming up with this crazy scheme. In that case you still wouldn't give userspace control over the fences. I don't see any way that can end well. What if userspace never signals? What if userspace gets killed by oom killer. Who keeps track of that? ~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
[RFC] Support for events with a large payload
Currently the event API allows for a payload of up to 64 bytes. Sometimes we
would like to pass larger payloads to userspace such as metadata associated
with a particular video stream.
A typical example of that would be object detection events.
This RFC describes one approach for doing this.
The event framework has the nice property of being able to use from within
interrupts. Copying large payloads does not fit into that framework, so
such payloads should be adminstrated separately.
In addition, I wouldn't allow large payloads to be filled in from interrupt
context: a worker queue would be much more appropriate.
Note that the event API is only useful for relatively low-bandwidth data
since the data is always copied. When dealing with high-bandwidth data the
data should either be a separate plane or become a special stream I/O buffer
type.
The userspace API enhancements in order to achieve this would be the
following:
- Any event that has a large payload would specify a payload_sequence counter
and a payload size value (in bytes).
- A new VIDIOC_DQEVENT_PAYLOAD ioctl would be added which passes the event type,
the payload_sequence counter and a pointer to a buffer to the kernel, and the
payload is returned, or an error is returned if the payload data is no longer
available.
Optional enhancements:
- Have VIDIOC_SUBSCRIBE_EVENT return the maximum payload size (lets apps
preallocate payload memory, but it might be overkill).
- Add functionality to VIDIOC_SUBSCRIBE_EVENT to define the number of
events in the event queue for the filehandle. If the payload is large,
you may want to limit the number of allocated payload buffers. For
example: when dealing with metadata associated with frame you might want
to limit the number of payload buffers to the number of allocated frames.
I feel that this can always be added later if we decide it is really needed,
and leave it out for now.
So the userspace API would be quite simple.
The internal implementation would look like this:
struct v4l2_event_payload {
u32 payload_size;
u32 payload_sequence;
void *payload;
};
struct v4l2_event_payloads {
// lock serializing access to this struct
struct mutex lock;
// global payload sequence number counter
u32 payload_sequence;
// size of the payload array
unsigned payloads;
// index of the oldest payload
unsigned first;
// number of payloads available
unsigned in_use;
struct v4l2_event_payload payloads[];
};
and a pointer to struct v4l2_event_payloads would be added to struct
v4l2_subscribed_event.
It is up to the driver to decide whether there is one v4l2_event_payloads
struct per filehandle or whether there is a global struct shared by any
filehandle subscribed to this event. This will depend on the event and the
size of the payload. Most likely it will be the latter option (a global
queue of payloads).
Internal functions would look like this:
// Initialize the structs.
void v4l2_event_payloads_init(struct v4l2_event_payloads *p, unsigned payloads);
// Get the first available payload (the mutex must be locked). If no payloads
// are available then the oldest payload will be reused. A new sequence number
// will be generated as well.
struct v4l2_event_payload *v4l2_event_payloads_new(struct v4l2_event_payloads
*p);
// Find the payload with the given sequence number. The mutex must be locked.
struct v4l2_event_payload *v4l2_event_payloads_find(struct v4l2_event_payloads
*p, unsigned seqnr);
So when a new payload arrives the driver would take the mutex, call
v4l2_event_payloads_new(), set payload_size and fill the payload data,
remember the payload_size and payload_sequence values, release the mutex
and queue the event with the remembered size and sequence values. Setting
up the payload part cannot be done from interrupt context.
When calling DQEVENT_PAYLOAD the core will use the pointer to struct
v4l2_event_payloads from struct v4l2_subscribed_event, take the mutex,
find the payload, copy it to userspace and release the mutex.
Right now the mutex is in struct v4l2_event_payloads. This is not optimal:
it might be better to have a spinlock for controlling access to the
v4l2_event_payloads struct and a mutex for each v4l2_event_payload struct.
That way setting and getting two different payload structs wouldn't depend
on one another.
Comments?
Hans
PS: I have no immediate plans to implement this, but based on recent discussions
it seems there is a desire to have support for this at some point in the future,
so I decided to see how this could be implemented.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] sta2x11_vip: fix error return code in sta2x11_vip_init_one()
Hello, I agree with the content of the patch, but I disagree with the commit message. From the commit message it seems that you fixed a bug about the error code, but the aim of this patch is to uniform the code style. I suggest something like: uniform code style in sta2x11_vip_init_one() On Monday 13 May 2013 13:59:45 Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn Fix to return a negative error code from the error handling case instead of 0, as done elsewhere in this function. Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn --- drivers/media/pci/sta2x11/sta2x11_vip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c index 7005695..77edc11 100644 --- a/drivers/media/pci/sta2x11/sta2x11_vip.c +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c @@ -1047,7 +1047,8 @@ static int sta2x11_vip_init_one(struct pci_dev *pdev, ret = sta2x11_vip_init_controls(vip); if (ret) goto free_mem; - if (v4l2_device_register(pdev-dev, vip-v4l2_dev)) + ret = v4l2_device_register(pdev-dev, vip-v4l2_dev); + if (ret) goto free_mem; dev_dbg(pdev-dev, BAR #0 at 0x%lx 0x%lx irq %d\n, -- Federico Vaga -- 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: Introduce a new helper framework for buffer synchronization
-Original Message- From: linux-fbdev-ow...@vger.kernel.org [mailto:linux-fbdev- ow...@vger.kernel.org] On Behalf Of Maarten Lankhorst Sent: Monday, May 13, 2013 8:41 PM To: Inki Dae Cc: 'Rob Clark'; 'Daniel Vetter'; 'DRI mailing list'; linux-arm- ker...@lists.infradead.org; linux-media@vger.kernel.org; 'linux-fbdev'; 'Kyungmin Park'; 'myungjoo.ham'; 'YoungJun Cho' Subject: Re: Introduce a new helper framework for buffer synchronization Op 13-05-13 13:24, Inki Dae schreef: and can be solved with userspace locking primitives. No need for the kernel to get involved. Yes, that is how we have synchronized buffer between CPU and DMA device until now without buffer synchronization mechanism. I thought that it's best to make user not considering anything: user can access a buffer regardless of any DMA device controlling and the buffer synchronization is performed in kernel level. Moreover, I think we could optimize graphics and multimedia hardware performance because hardware can do more works: one thread accesses a shared buffer and the other controls DMA device with the shared buffer in parallel. Thus, we could avoid sequential processing and that is my intention. Aren't you think about that we could improve hardware utilization with such way or other? of course, there could be better way. If you don't want to block the hardware the only option is to allocate a temp bo and blit to/from it using the hardware. OpenGL already does that when you want to read back, because otherwise the entire pipeline can get stalled. The overhead of command submission for that shouldn't be high, if it is you should really try to optimize that first before coming up with this crazy scheme. I have considered all devices sharing buffer with CPU; multimedia, display controller and graphics devices (including GPU). And we could provide easy-to-use user interfaces for buffer synchronization. Of course, the proposed user interfaces may be so ugly yet but at least, I think we need something else for it. In that case you still wouldn't give userspace control over the fences. I don't see any way that can end well. What if userspace never signals? What if userspace gets killed by oom killer. Who keeps track of that? In all cases, all kernel resources to user fence will be released by kernel once the fence is timed out: never signaling and process killing by oom killer makes the fence timed out. And if we use mmap mechanism you mentioned before, I think user resource could also be freed properly. Thanks, Inki Dae ~Maarten -- To unsubscribe from this list: send the line unsubscribe linux-fbdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] sta2x11_vip: fix error return code in sta2x11_vip_init_one()
On 05/13/2013 08:19 PM, Federico Vaga wrote: Hello, I agree with the content of the patch, but I disagree with the commit message. From the commit message it seems that you fixed a bug about the error code, but the aim of this patch is to uniform the code style. I suggest something like: uniform code style in sta2x11_vip_init_one() The orig code will release all the resources if v4l2_device_register() failed and return 0. But what we need in this case is to return an negative error code to let the caller known we are failed. So the patch save the return value of v4l2_device_register() to 'ret' and return it when error. On Monday 13 May 2013 13:59:45 Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn Fix to return a negative error code from the error handling case instead of 0, as done elsewhere in this function. Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn --- drivers/media/pci/sta2x11/sta2x11_vip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c index 7005695..77edc11 100644 --- a/drivers/media/pci/sta2x11/sta2x11_vip.c +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c @@ -1047,7 +1047,8 @@ static int sta2x11_vip_init_one(struct pci_dev *pdev, ret = sta2x11_vip_init_controls(vip); if (ret) goto free_mem; -if (v4l2_device_register(pdev-dev, vip-v4l2_dev)) +ret = v4l2_device_register(pdev-dev, vip-v4l2_dev); +if (ret) goto free_mem; dev_dbg(pdev-dev, BAR #0 at 0x%lx 0x%lx irq %d\n, -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] sta2x11_vip: fix error return code in sta2x11_vip_init_one()
On Monday 13 May 2013 20:40:33 Wei Yongjun wrote: On 05/13/2013 08:19 PM, Federico Vaga wrote: Hello, I agree with the content of the patch, but I disagree with the commit message. From the commit message it seems that you fixed a bug about the error code, but the aim of this patch is to uniform the code style. I suggest something like: uniform code style in sta2x11_vip_init_one() The orig code will release all the resources if v4l2_device_register() failed and return 0. But what we need in this case is to return an negative error code to let the caller known we are failed. So the patch save the return value of v4l2_device_register() to 'ret' and return it when error. Oh sure, you are right :) I did not understand it immediately from the commit message. Can you put this answer as commit message? It is perfectly clear where is the bug now. Thank you -- Federico Vaga -- 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: Introduce a new helper framework for buffer synchronization
On Mon, May 13, 2013 at 8:21 AM, Inki Dae inki@samsung.com wrote: In that case you still wouldn't give userspace control over the fences. I don't see any way that can end well. What if userspace never signals? What if userspace gets killed by oom killer. Who keeps track of that? In all cases, all kernel resources to user fence will be released by kernel once the fence is timed out: never signaling and process killing by oom killer makes the fence timed out. And if we use mmap mechanism you mentioned before, I think user resource could also be freed properly. I tend to agree w/ Maarten here.. there is no good reason for userspace to be *signaling* fences. The exception might be some blob gpu drivers which don't have enough knowledge in the kernel to figure out what to do. (In which case you can add driver private ioctls for that.. still not the right thing to do but at least you don't make a public API out of it.) BR, -R -- 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] [media] sta2x11_vip: fix error return code in sta2x11_vip_init_one()
From: Wei Yongjun yongjun_...@trendmicro.com.cn The orig code will release all the resources if v4l2_device_register() failed and return 0. But what we need in this case is to return an negative error code to let the caller known we are failed. So the patch save the return value of v4l2_device_register() to 'ret' and return it when error. Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn --- v1 - v2: change the commit message --- drivers/media/pci/sta2x11/sta2x11_vip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c index 7005695..77edc11 100644 --- a/drivers/media/pci/sta2x11/sta2x11_vip.c +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c @@ -1047,7 +1047,8 @@ static int sta2x11_vip_init_one(struct pci_dev *pdev, ret = sta2x11_vip_init_controls(vip); if (ret) goto free_mem; - if (v4l2_device_register(pdev-dev, vip-v4l2_dev)) + ret = v4l2_device_register(pdev-dev, vip-v4l2_dev); + if (ret) goto free_mem; dev_dbg(pdev-dev, BAR #0 at 0x%lx 0x%lx irq %d\n, -- 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] [media] sta2x11_vip: fix error return code in sta2x11_vip_init_one()
On Monday 13 May 2013 22:00:01 Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn The orig code will release all the resources if v4l2_device_register() failed and return 0. But what we need in this case is to return an negative error code to let the caller known we are failed. So the patch save the return value of v4l2_device_register() to 'ret' and return it when error. Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn Acked-by: Federico Vaga federico.v...@gmail.com --- v1 - v2: change the commit message --- drivers/media/pci/sta2x11/sta2x11_vip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c index 7005695..77edc11 100644 --- a/drivers/media/pci/sta2x11/sta2x11_vip.c +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c @@ -1047,7 +1047,8 @@ static int sta2x11_vip_init_one(struct pci_dev *pdev, ret = sta2x11_vip_init_controls(vip); if (ret) goto free_mem; - if (v4l2_device_register(pdev-dev, vip-v4l2_dev)) + ret = v4l2_device_register(pdev-dev, vip-v4l2_dev); + if (ret) goto free_mem; dev_dbg(pdev-dev, BAR #0 at 0x%lx 0x%lx irq %d\n, -- Federico Vaga -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] v4l: vb2: fix error return code in __vb2_init_fileio()
Hi Wei,
On Mon, May 13, 2013 at 01:48:45PM +0800, Wei Yongjun wrote:
From: Wei Yongjun yongjun_...@trendmicro.com.cn
Fix to return -EINVAL in the get kernel address error handling
case instead of 0, as done elsewhere in this function.
Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn
---
drivers/media/v4l2-core/videobuf2-core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 7d833ee..7bd3ee6 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -2193,8 +2193,10 @@ static int __vb2_init_fileio(struct vb2_queue *q, int
read)
*/
for (i = 0; i q-num_buffers; i++) {
fileio-bufs[i].vaddr = vb2_plane_vaddr(q-bufs[i], 0);
- if (fileio-bufs[i].vaddr == NULL)
+ if (fileio-bufs[i].vaddr == NULL) {
+ ret = -EINVAL;
goto err_reqbufs;
+ }
fileio-bufs[i].size = vb2_plane_size(q-bufs[i], 0);
}
Nice patch!
Reviewed-by: Sakari Ailus sakari.ai...@iki.fi
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device
Hi all, On Wed, May 08, 2013 at 09:32:17AM +0200, Andrzej Hajda wrote: On 07.05.2013 17:07, Laurent Pinchart wrote: On Tuesday 07 May 2013 02:11:27 Kim, Milo wrote: On Monday, May 06, 2013 6:34 PM Andrzej Hajda wrote: This RFC proposes generic API for exposing flash subdevices via LED framework. Rationale Currently there are two frameworks which are used for exposing LED flash to user space: - V4L2 flash controls, - LED framework(with custom sysfs attributes). The list below shows flash drivers in mainline kernel with initial commit date and typical chip application (according to producer): LED API: lm3642: 2012-09-12, Cameras lm355x: 2012-09-05, Cameras max8997: 2011-12-14, Cameras (?) lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys pca955x: 2008-07-16, Cameras, Indicators (?) V4L2 API: as3645a: 2011-05-05, Cameras adp1653: 2011-05-05, Cameras V4L2 provides richest functionality, but there is often demand from application developers to provide already established LED API. We would like to have an unified user interface for flash devices. Some of devices already have the LED API driver exposing limited set of a Flash IC functionality. In order to support all required features the LED API would have to be extended or the V4L2 API would need to be used. However when switching from a LED to a V4L2 Flash driver existing LED API interface would need to be retained. Proposed solution This patch adds V4L2 helper functions to register existing V4L2 flash subdev as LED class device. After registration via v4l2_leddev_register appropriate entry in /sys/class/leds/ is created. During registration all V4L2 flash controls are enumerated and corresponding attributes are added. I have attached also patch with new max77693-led driver using v4l2_leddev. This patch requires presence of the patch max77693: added device tree support: https://patchwork.kernel.org/patch/2414351/ . Additional features - simple API to access all V4L2 flash controls via sysfs, - V4L2 subdevice should not be registered by V4L2 device to use it, - LED triggers API can be used to control the device, - LED device is optional - it will be created only if V4L2_LEDDEV configuration option is enabled and the subdev driver calls v4l2_leddev_register. Doubts This RFC is a result of a uncertainty which API developers should expose by their flash drivers. It is a try to gluing together both APIs. I am not sure if it is the best solution, but I hope there will be some discussion and hopefully some decisions will be taken which way we should follow. The LED subsystem provides similar APIs for the Camera driver. With LED trigger event, flash and torch are enabled/disabled. I'm not sure this is applicable for you. Could you take a look at LED camera trigger feature? For the camera LED trigger, https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/ ?h=f or-nextid=48a1d032c954b9b06c3adbf35ef4735dd70ab757 Example of camera flash driver, https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/ ?h=f or-nextid=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a I think we should decide on one API. Implementing two APIs for a single device is usually messy, and will result in different feature sets (and different bugs) being implemented through each API, depending on the driver. Interactions between the APIs are also a pain point on the kernel side to properly synchronize calls. I don't like having two APIs either. Especially we shouldn't have multiple drivers implementing different APIs for the same device. That said, I wonder if it's possible to support camera-related use cases using the LED API: it's originally designed for quite different devices. Even if you could handle flash strobing using the LED API, the functionality provided by the Media controller and subdev APIs will always be missing: device enumeration and association with the right camera. The strobe functionality is connected to the sensor (hardware strobe pin) which also is why it's natural for the flash to be accessible over the V4L2 API. This was briefly discussed some time ago: URL:http://www.spinics.net/lists/linux-media/msg53645.html The LED API is too limited for torch and flash usage, but I'm definitely open to moving flash devices to the LED API is we can extend it in a way that it covers all the use cases. Extending LED API IMHO seems to be quite straightforward - by adding attributes for supported functionalities. We just need a specification for standard flash/torch attributes. I could prepare an RFC about it if there is a will to explore this direction. I'm leaning towards providing a wrapper that provides torch functionality using V4L2 flash API unless it's really proven to be insane. ;-) The code supporting that in an individual
Re: Device driver memory 'mmap()' function helper cleanup
Hi Mauro, On Wed, Apr 17, 2013 at 07:43:00AM -0300, Mauro Carvalho Chehab wrote: and a camera anymore. The OMAP2 were used on some Nokia phones. They used to maintain that code, but now that they moved to the dark side of the moon, they lost their interests on it. So, it may not be easily find testers for patches there. There's one more underlying issue there than potentially having both no-one with the device and time to test it: the driver does not function as-is in mainline (nor any recent non-mainline kernel either). Quite some work would be required to update it (both to figure out why the whole system crashes when trying to capture images and change the driver use modern APIs). A small while back we decided to still keep the driver in the tree: URL:http://www.spinics.net/lists/linux-media/msg56237.html (The rest of the discussion took place in #v4l AFAIR.) So, what could be done now is either 1) write a patch that changes the driver to use the right API and take a risk of adding one more bug to the driver; or 2) remove the driver now and bring it back only if someone really has time to make it work first. -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] [media] v4l2: SI476X MFD - Do not use binary constants
On Wed, May 8, 2013 at 1:23 PM, Geert Uytterhoeven ge...@linux-m68k.org wrote: Gcc 4.3 doesn't understand binary constanrs (0b*): drivers/media/radio/radio-si476x.c:862:20: error: invalid suffix b1000 on integer constant Hence use a hexadecimal constant (0x*) instead. Signed-off-by: Geert Uytterhoeven ge...@linux-m68k.org Cc: Mauro Carvalho Chehab mche...@redhat.com Cc: linux-media@vger.kernel.org Acked-by: Andrey Smirnov andrew.smir...@gmail.com --- drivers/media/radio/radio-si476x.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/radio/radio-si476x.c b/drivers/media/radio/radio-si476x.c index 9430c6a..9dc8baf 100644 --- a/drivers/media/radio/radio-si476x.c +++ b/drivers/media/radio/radio-si476x.c @@ -44,7 +44,7 @@ #define FREQ_MUL (1000 / 625) -#define SI476X_PHDIV_STATUS_LINK_LOCKED(status) (0b1000 (status)) +#define SI476X_PHDIV_STATUS_LINK_LOCKED(status) (0x80 (status)) #define DRIVER_NAME si476x-radio #define DRIVER_CARD SI476x AM/FM Receiver -- 1.7.0.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: [PATCH] [media] omap3isp: Remove redundant platform_set_drvdata()
Hi Sachin, On Mon, May 13, 2013 at 02:47:54PM +0530, Sachin Kamat wrote: Commit 0998d06310 (device-core: Ensure drvdata = NULL when no driver is bound) removes the need to set driver data field to NULL. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Sakari Ailus sakari.ai...@iki.fi Thanks! Acked-by: Sakari Ailus sakari.ai...@iki.fi -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- 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: Introduce a new helper framework for buffer synchronization
On Mon, May 13, 2013 at 1:18 PM, Inki Dae inki@samsung.com wrote: 2013/5/13 Rob Clark robdcl...@gmail.com On Mon, May 13, 2013 at 8:21 AM, Inki Dae inki@samsung.com wrote: In that case you still wouldn't give userspace control over the fences. I don't see any way that can end well. What if userspace never signals? What if userspace gets killed by oom killer. Who keeps track of that? In all cases, all kernel resources to user fence will be released by kernel once the fence is timed out: never signaling and process killing by oom killer makes the fence timed out. And if we use mmap mechanism you mentioned before, I think user resource could also be freed properly. I tend to agree w/ Maarten here.. there is no good reason for userspace to be *signaling* fences. The exception might be some blob gpu drivers which don't have enough knowledge in the kernel to figure out what to do. (In which case you can add driver private ioctls for that.. still not the right thing to do but at least you don't make a public API out of it.) Please do not care whether those are generic or not. Let's see the following three things. First, it's cache operation. As you know, ARM SoC has ACP (Accelerator Coherency Port) and can be connected to DMA engine or similar devices. And this port is used for cache coherency between CPU cache and DMA device. However, most devices on ARM based embedded systems don't use the ACP port. So they need proper cache operation before and after of DMA or CPU access in case of using cachable mapping. Actually, I see many Linux based platforms call cache control interfaces directly for that. I think the reason, they do so, is that kernel isn't aware of when and how CPU accessed memory. I think we had kicked around the idea of giving dmabuf's a prepare/finish ioctl quite some time back. This is probably something that should be at least a bit decoupled from fences. (Possibly 'prepare' waits for dma access to complete, but not the other way around.) And I did implement in omapdrm support for simulating coherency via page fault-in / shoot-down.. It is one option that makes it completely transparent to userspace, although there is some performance const, so I suppose it depends a bit on your use-case. And second, user process has to do so many things in case of using shared memory with DMA device. User process should understand how DMA device is operated and when interfaces for controling the DMA device are called. Such things would make user application so complicated. And third, it's performance optimization to multimedia and graphics devices. As I mentioned already, we should consider sequential processing for buffer sharing between CPU and DMA device. This means that CPU should stay with idle until DMA device is completed and vise versa. That is why I proposed such user interfaces. Of course, these interfaces might be so ugly yet: for this, Maarten pointed already out and I agree with him. But there must be another better way. Aren't you think we need similar thing? With such interfaces, cache control and buffer synchronization can be performed in kernel level. Moreover, user applization doesn't need to consider DMA device controlling anymore. Therefore, one thread can access a shared buffer and the other can control DMA device with the shared buffer in parallel. We can really make the best use of CPU and DMA idle time. In other words, we can really make the best use of multi tasking OS, Linux. So could you please tell me about that there is any reason we don't use public API for it? I think we can add and use public API if NECESSARY. well, for cache management, I think it is a better idea.. I didn't really catch that this was the motivation from the initial patch, but maybe I read it too quickly. But cache can be decoupled from synchronization, because CPU access is not asynchronous. For userspace/CPU access to buffer, you should: 1) wait for buffer 2) prepare-access 3) ... do whatever cpu access to buffer ... 4) finish-access 5) submit buffer for new dma-operation I suppose you could combine the syscall for #1 and #2.. not sure if that is a good idea or not. But you don't need to. And there is never really any need for userspace to signal a fence. BR, -R Thanks, Inki Dae BR, -R ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- 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: Media controller versus int device in V4L2
Hans, I fear that in my ignorance of V4L2, I have backed my client into a corner, so to speak. I am developing embedded Linux firmware for boards intended to driver video sensors within a medical device. As such, tried and true versions of everything on board are preferred, even if they are not cutting edge. Applying this philosophy has gotten me into the situation where I am committed, for first human use, to a 2.6.37 kernel which does not have media controller v4l2, only int device. Hence my question about back-porting drivers, and the programming differences. I hope that clears up my situation for you. If a patch exists to make the v4l2 on a 2.6.37 kernel into a media controller version, I am unaware of it, though I have not conducted a search for it (I will as soon as I finish this e-mail). Sincerely, Phillip Norisez Software Design Engineer Creation Technologies Office: 303.835.7494 phillip.nori...@creationtech.com | www.creationtech.com -Original Message- From: Hans Verkuil [mailto:hverk...@xs4all.nl] Sent: Friday, May 10, 2013 5:55 AM To: Phillip Norisez Cc: linux-media@vger.kernel.org Subject: Re: Media controller versus int device in V4L2 On Wed May 8 2013 17:06:17 Phillip Norisez wrote: I have the following question, who or what can help me with some information on the specific differences, from a programming viewpoint, between the media controller and int device frameworks for V4L2? v4l2-int-device is deprecated and should never be used. There is only one remaining driver that uses it. Hopefully one day that will be converted as well and the int-device API will disappear. The int-device API has nothing to do with the media controller. It has been superseded by the v4l2-subdev API. Reasonably detailed information is available in Documentation/video4linux/v4l2-frameworks.txt and in the V4L2 Spec (which also contains the Media Controller documentation). It is not entirely clear to me what you want to achieve, but if you happen to have int-device based drivers then those should be converted to v4l2_subdev based drivers for which there are a ton of examples. Regards, Hans A checklist for forward and back porting is what I seek, but I don't expect such a thing to exist. However, I believe someone on here may have the knowledge to author such a list, and I would be willing to pay reasonably for it. Phillip Norisez Software Design Engineer Creation Technologies Office: 303.835.7494 6833 Joyce Street | Arvada, CO 80007 | USA phillip.nori...@creationtech.com | www.creationtech.com Confidentiality Notice This e-mail and any attachment(s) are intended for the individual or entity to which this email is addressed and may contain information that is confidential. If you are not the intended recipient or an employee or agent responsible for delivering this e-mail to the intended recipient, please be aware that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this in error, please notify the sender by telephone at 604.430.4336 or by return e-mail, and please delete or destroy all copies of this communication. Thank you! P Please consider the impact on the environment before printing this email or its attachments -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Mon May 13 19:00:28 CEST 2013 git branch: test git hash: 02615ed5e1b2283db2495af3cf8f4ee172c77d80 gcc version:i686-linux-gcc (GCC) 4.7.2 host hardware: x86_64 host os:3.8-3.slh.2-amd64 linux-git-arm-davinci: OK linux-git-arm-exynos: WARNINGS linux-git-arm-omap: WARNINGS linux-git-blackfin: WARNINGS 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: WARNINGS linux-2.6.32.27-i686: WARNINGS linux-2.6.33.7-i686: WARNINGS linux-2.6.34.7-i686: WARNINGS linux-2.6.35.9-i686: WARNINGS linux-2.6.36.4-i686: WARNINGS linux-2.6.37.6-i686: WARNINGS linux-2.6.38.8-i686: WARNINGS linux-2.6.39.4-i686: WARNINGS linux-3.0.60-i686: WARNINGS linux-3.1.10-i686: WARNINGS linux-3.2.37-i686: WARNINGS linux-3.3.8-i686: WARNINGS linux-3.4.27-i686: WARNINGS linux-3.5.7-i686: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.7.4-i686: WARNINGS linux-3.8-i686: OK linux-3.9-rc1-i686: OK linux-2.6.31.14-x86_64: WARNINGS linux-2.6.32.27-x86_64: WARNINGS linux-2.6.33.7-x86_64: WARNINGS linux-2.6.34.7-x86_64: WARNINGS linux-2.6.35.9-x86_64: WARNINGS linux-2.6.36.4-x86_64: WARNINGS linux-2.6.37.6-x86_64: WARNINGS linux-2.6.38.8-x86_64: WARNINGS linux-2.6.39.4-x86_64: WARNINGS linux-3.0.60-x86_64: WARNINGS linux-3.1.10-x86_64: WARNINGS linux-3.2.37-x86_64: WARNINGS linux-3.3.8-x86_64: WARNINGS linux-3.4.27-x86_64: WARNINGS linux-3.5.7-x86_64: WARNINGS linux-3.6.11-x86_64: WARNINGS linux-3.7.4-x86_64: WARNINGS linux-3.8-x86_64: OK linux-3.9-rc1-x86_64: OK apps: ERRORS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.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
[PATCH v3] adv7180: add more subdev video ops
From: Vladimir Barinov vladimir.bari...@cogentembedded.com
Add subdev video ops for ADV7180 video decoder. This makes decoder usable on
the soc-camera drivers.
Signed-off-by: Vladimir Barinov vladimir.bari...@cogentembedded.com
Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
---
This patch is against the 'media_tree.git' repo.
Changes from version 2:
- set the field format depending on video standard in try_mbus_fmt() method;
- removed querystd() method calls from try_mbus_fmt() and cropcap() methods;
- removed g_crop() method.
drivers/media/i2c/adv7180.c | 86
1 file changed, 86 insertions(+)
Index: media_tree/drivers/media/i2c/adv7180.c
===
--- media_tree.orig/drivers/media/i2c/adv7180.c
+++ media_tree/drivers/media/i2c/adv7180.c
@@ -1,6 +1,8 @@
/*
* adv7180.c Analog Devices ADV7180 video decoder driver
* Copyright (c) 2009 Intel Corporation
+ * Copyright (C) 2013 Cogent Embedded, Inc.
+ * Copyright (C) 2013 Renesas Solutions Corp.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -128,6 +130,7 @@ struct adv7180_state {
v4l2_std_id curr_norm;
boolautodetect;
u8 input;
+ struct v4l2_mbus_framefmt fmt;
};
#define to_adv7180_sd(_ctrl) (container_of(_ctrl-handler,\
struct adv7180_state, \
@@ -397,10 +400,93 @@ static void adv7180_exit_controls(struct
v4l2_ctrl_handler_free(state-ctrl_hdl);
}
+static int adv7180_enum_mbus_fmt(struct v4l2_subdev *sd, unsigned int index,
+enum v4l2_mbus_pixelcode *code)
+{
+ if (index 0)
+ return -EINVAL;
+
+ *code = V4L2_MBUS_FMT_YUYV8_2X8;
+
+ return 0;
+}
+
+static int adv7180_try_mbus_fmt(struct v4l2_subdev *sd,
+ struct v4l2_mbus_framefmt *fmt)
+{
+ struct adv7180_state *state = to_state(sd);
+
+ fmt-code = V4L2_MBUS_FMT_YUYV8_2X8;
+ fmt-colorspace = V4L2_COLORSPACE_SMPTE170M;
+ fmt-field = state-curr_norm V4L2_STD_525_60 ?
+V4L2_FIELD_INTERLACED_BT : V4L2_FIELD_INTERLACED_TB;
+ fmt-width = 720;
+ fmt-height = state-curr_norm V4L2_STD_525_60 ? 480 : 576;
+
+ return 0;
+}
+
+static int adv7180_g_mbus_fmt(struct v4l2_subdev *sd,
+ struct v4l2_mbus_framefmt *fmt)
+{
+ struct adv7180_state *state = to_state(sd);
+
+ *fmt = state-fmt;
+
+ return 0;
+}
+
+static int adv7180_s_mbus_fmt(struct v4l2_subdev *sd,
+ struct v4l2_mbus_framefmt *fmt)
+{
+ struct adv7180_state *state = to_state(sd);
+
+ adv7180_try_mbus_fmt(sd, fmt);
+ state-fmt = *fmt;
+
+ return 0;
+}
+
+static int adv7180_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
+{
+ struct adv7180_state *state = to_state(sd);
+
+ a-bounds.left = 0;
+ a-bounds.top = 0;
+ a-bounds.width = 720;
+ a-bounds.height = state-curr_norm V4L2_STD_525_60 ? 480 : 576;
+ a-defrect = a-bounds;
+ a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+ a-pixelaspect.numerator = 1;
+ a-pixelaspect.denominator = 1;
+
+ return 0;
+}
+
+static int adv7180_g_mbus_config(struct v4l2_subdev *sd,
+struct v4l2_mbus_config *cfg)
+{
+ /*
+* The ADV7180 sensor supports BT.601/656 output modes.
+* The BT.656 is default and not yet configurable by s/w.
+*/
+ cfg-flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
+V4L2_MBUS_DATA_ACTIVE_HIGH;
+ cfg-type = V4L2_MBUS_BT656;
+
+ return 0;
+}
+
static const struct v4l2_subdev_video_ops adv7180_video_ops = {
.querystd = adv7180_querystd,
.g_input_status = adv7180_g_input_status,
.s_routing = adv7180_s_routing,
+ .enum_mbus_fmt = adv7180_enum_mbus_fmt,
+ .try_mbus_fmt = adv7180_try_mbus_fmt,
+ .g_mbus_fmt = adv7180_g_mbus_fmt,
+ .s_mbus_fmt = adv7180_s_mbus_fmt,
+ .cropcap = adv7180_cropcap,
+ .g_mbus_config = adv7180_g_mbus_config,
};
static const struct v4l2_subdev_core_ops adv7180_core_ops = {
--
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: Introduce a new helper framework for buffer synchronization
Hi, On Monday 13 of May 2013 20:24:01 Inki Dae wrote: -Original Message- From: Maarten Lankhorst [mailto:maarten.lankho...@canonical.com] Sent: Monday, May 13, 2013 6:52 PM To: Inki Dae Cc: 'Rob Clark'; 'Daniel Vetter'; 'DRI mailing list'; linux-arm- ker...@lists.infradead.org; linux-media@vger.kernel.org; 'linux-fbdev'; 'Kyungmin Park'; 'myungjoo.ham'; 'YoungJun Cho' Subject: Re: Introduce a new helper framework for buffer synchronization Op 13-05-13 11:21, Inki Dae schreef: -Original Message- From: Maarten Lankhorst [mailto:maarten.lankho...@canonical.com] Sent: Monday, May 13, 2013 5:01 PM To: Inki Dae Cc: Rob Clark; Daniel Vetter; DRI mailing list; linux-arm- ker...@lists.infradead.org; linux-media@vger.kernel.org; linux-fbdev; Kyungmin Park; myungjoo.ham; YoungJun Cho Subject: Re: Introduce a new helper framework for buffer synchronization Op 09-05-13 09:33, Inki Dae schreef: Hi all, This post introduces a new helper framework based on dma fence. And the purpose of this post is to collect other opinions and advices before RFC posting. First of all, this helper framework, called fence helper, is in progress yet so might not have enough comments in codes and also might need to be more cleaned up. Moreover, we might be missing some parts of the dma fence. However, I'd like to say that all things mentioned below has been tested with Linux platform and worked well. And tutorial for user process. just before cpu access struct dma_buf_fence *df; df-type = DMA_BUF_ACCESS_READ or DMA_BUF_ACCESS_WRITE; ioctl(fd, DMA_BUF_GET_FENCE, df); after memset or memcpy ioctl(fd, DMA_BUF_PUT_FENCE, df); NAK. Userspace doesn't need to trigger fences. It can do a buffer idle wait, and postpone submitting new commands until after it's done using the buffer. Hi Maarten, It seems that you say user should wait for a buffer like KDS does: KDS uses select() to postpone submitting new commands. But I think this way assumes that every data flows a DMA device to a CPU. For example, a CPU should keep polling for the completion of a buffer access by a DMA device. This means that the this way isn't considered for data flow to opposite case; CPU to DMA device. Not really. You do both things the same way. You first wait for the bo to be idle, this could be implemented by adding poll support to the dma-buf fd. Then you either do your read or write. Since userspace is supposed to be the one controlling the bo it should stay idle at that point. If you have another thread queueing the buffer againbefore your thread is done that's a bug in the application, and can be solved with userspace locking primitives. No need for the kernel to get involved. Yes, that is how we have synchronized buffer between CPU and DMA device until now without buffer synchronization mechanism. I thought that it's best to make user not considering anything: user can access a buffer regardless of any DMA device controlling and the buffer synchronization is performed in kernel level. Moreover, I think we could optimize graphics and multimedia hardware performance because hardware can do more works: one thread accesses a shared buffer and the other controls DMA device with the shared buffer in parallel. Could you explain this point? I thought that if there is a shared buffer accessible for user and DMA device, only one of them can use it at the moment, i.e. the buffer is useless for the reading user (or read DMA) until (write) DMA (or writing user) finishes writing for it. Is it incorrect? Or this is not the point here? I'm not an expert here and I'm just trying to understand the idea, so correct me if I'm wrong. Thanks in advance. Best regards, Tomasz Thus, we could avoid sequential processing and that is my intention. Aren't you think about that we could improve hardware utilization with such way or other? of course, there could be better way. Kernel space doesn't need the root hole you created by giving a dereferencing a pointer passed from userspace. Your next exercise should be to write a security exploit from the api you created here. It's the only way to learn how to write safe code. Hint: df.ctx = mmap(..); Also I'm not clear to use our way yet and that is why I posted. As you mentioned, it seems like that using mmap() is more safe. But there is one issue it makes me confusing. For your hint, df.ctx = mmap(..), the issue is that dmabuf mmap can be used to map a dmabuf with user space. And the
Re: [PATCH RFC v3] media: i2c: mt9p031: add OF support
Hi Sascha,
On Monday 13 May 2013 12:46:04 Sascha Hauer wrote:
On Wed, May 08, 2013 at 12:37:29PM +0200, Laurent Pinchart wrote:
Hi Prabhakar,
On Wednesday 08 May 2013 10:19:57 Prabhakar Lad wrote:
On Wed, May 8, 2013 at 7:32 AM, Laurent Pinchart wrote:
On Tuesday 07 May 2013 15:10:36 Prabhakar Lad wrote:
On Mon, May 6, 2013 at 8:29 PM, Prabhakar Lad wrote:
On Fri, May 3, 2013 at 8:04 PM, Arnd Bergmann wrote:
On Friday 03 May 2013, Prabhakar Lad wrote:
[snip]
+}
Ok, good.
@@ -955,7 +998,17 @@ static int mt9p031_probe(struct i2c_client
*client,
mt9p031-pdata = pdata;
mt9p031-output_control = MT9P031_OUTPUT_CONTROL_DEF;
mt9p031-mode2 = MT9P031_READ_MODE_2_ROW_BLC;
- mt9p031-model = did-driver_data;
+
+ if (!client-dev.of_node) {
+ mt9p031-model = (enum
mt9p031_model)did-driver_data;
+ } else {
+ const struct of_device_id *of_id;
+
+ of_id =
of_match_device(of_match_ptr(mt9p031_of_match),
+ client-dev);
+ if (of_id)
+ mt9p031-model = (enum
mt9p031_model)of_id-data;
+ }
mt9p031-reset = -1;
Is this actually required? I thought the i2c core just compared
the
part of the compatible value after the first comma to the
string, so
mt9p031-model = (enum mt9p031_model)did-driver_data should
work
in both cases.
At least on v3.8 I just checked that 'did' is indeed NULL for the
devicetree case. Also I see no indication that i2c starts comparing
after the first comma in the string.
I am OK with mt9p031-model = (enum
mt9p031_model)did-driver_data
but I see still few drivers doing this, I am not sure for what
reason.
If everyone is OK with it I can drop the above change.
My bad, while booting with DT the i2c_device_id ie did in this case
will
be NULL, so the above changes are required :-)
I've just tested your patch, and did isn't NULL when booting my
Beagleboard with DT (on v3.9-rc5).
I am pretty much sure you tested it compatible property as
aptina,mt9p031
if the compatible property is set to aptina,mt9p031m the did will be
NULL.
I've tested both :-)
Sorry to nag, but did you use aptina,mt9p031[m] as a compatible string or
did you use mt9p031[m]. With aptina,... 'did' should really be NULL.
I've used aptina,mt9p031[m].
Please see the of_modalias_node() call in of_i2c_register_devices()
(drivers/of/of-i2c.c), that's where the I2C device type name should be
initialized.
--
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: Introduce a new helper framework for buffer synchronization
-Original Message- From: Rob Clark [mailto:robdcl...@gmail.com] Sent: Tuesday, May 14, 2013 2:58 AM To: Inki Dae Cc: linux-fbdev; DRI mailing list; Kyungmin Park; myungjoo.ham; YoungJun Cho; linux-arm-ker...@lists.infradead.org; linux-media@vger.kernel.org Subject: Re: Introduce a new helper framework for buffer synchronization On Mon, May 13, 2013 at 1:18 PM, Inki Dae inki@samsung.com wrote: 2013/5/13 Rob Clark robdcl...@gmail.com On Mon, May 13, 2013 at 8:21 AM, Inki Dae inki@samsung.com wrote: In that case you still wouldn't give userspace control over the fences. I don't see any way that can end well. What if userspace never signals? What if userspace gets killed by oom killer. Who keeps track of that? In all cases, all kernel resources to user fence will be released by kernel once the fence is timed out: never signaling and process killing by oom killer makes the fence timed out. And if we use mmap mechanism you mentioned before, I think user resource could also be freed properly. I tend to agree w/ Maarten here.. there is no good reason for userspace to be *signaling* fences. The exception might be some blob gpu drivers which don't have enough knowledge in the kernel to figure out what to do. (In which case you can add driver private ioctls for that.. still not the right thing to do but at least you don't make a public API out of it.) Please do not care whether those are generic or not. Let's see the following three things. First, it's cache operation. As you know, ARM SoC has ACP (Accelerator Coherency Port) and can be connected to DMA engine or similar devices. And this port is used for cache coherency between CPU cache and DMA device. However, most devices on ARM based embedded systems don't use the ACP port. So they need proper cache operation before and after of DMA or CPU access in case of using cachable mapping. Actually, I see many Linux based platforms call cache control interfaces directly for that. I think the reason, they do so, is that kernel isn't aware of when and how CPU accessed memory. I think we had kicked around the idea of giving dmabuf's a prepare/finish ioctl quite some time back. This is probably something that should be at least a bit decoupled from fences. (Possibly 'prepare' waits for dma access to complete, but not the other way around.) And I did implement in omapdrm support for simulating coherency via page fault-in / shoot-down.. It is one option that makes it completely transparent to userspace, although there is some performance const, so I suppose it depends a bit on your use-case. And second, user process has to do so many things in case of using shared memory with DMA device. User process should understand how DMA device is operated and when interfaces for controling the DMA device are called. Such things would make user application so complicated. And third, it's performance optimization to multimedia and graphics devices. As I mentioned already, we should consider sequential processing for buffer sharing between CPU and DMA device. This means that CPU should stay with idle until DMA device is completed and vise versa. That is why I proposed such user interfaces. Of course, these interfaces might be so ugly yet: for this, Maarten pointed already out and I agree with him. But there must be another better way. Aren't you think we need similar thing? With such interfaces, cache control and buffer synchronization can be performed in kernel level. Moreover, user applization doesn't need to consider DMA device controlling anymore. Therefore, one thread can access a shared buffer and the other can control DMA device with the shared buffer in parallel. We can really make the best use of CPU and DMA idle time. In other words, we can really make the best use of multi tasking OS, Linux. So could you please tell me about that there is any reason we don't use public API for it? I think we can add and use public API if NECESSARY. well, for cache management, I think it is a better idea.. I didn't really catch that this was the motivation from the initial patch, but maybe I read it too quickly. But cache can be decoupled from synchronization, because CPU access is not asynchronous. For userspace/CPU access to buffer, you should: 1) wait for buffer 2) prepare-access 3) ... do whatever cpu access to buffer ... 4) finish-access 5) submit buffer for new dma-operation For data flow from CPU to DMA device, 1) wait for buffer 2) prepare-access (dma_buf_begin_cpu_access) 3) cpu access to buffer For data flow from DMA device to CPU 1) wait for buffer 2) finish-access (dma_buf_end _cpu_access) 3) dma access to buffer 1) and 2) are coupled with one function: we have implemented fence_helper_commit_reserve() for it. Cache control(cache clean or cache
[PATCH] [media] hdpvr: Disable IR receiver by default.
All of the firmwares I've tested, including 0x1e, will inevitably crash
before recording for even 10 minutes. There must be a race condition of
IR RX vs. video-encoding in the firmware, because if you disable IR receiver
polling, then the firmware is stable again. I'd guess that most people don't
use this feature anyway, so we might as well disable it by default, and
warn them that it might be unstable until Hauppauge fixes it in a future
firmware.
Signed-off-by: Jeff Hansen x...@jeffhansen.com
---
drivers/media/usb/hdpvr/hdpvr-core.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/media/usb/hdpvr/hdpvr-core.c
b/drivers/media/usb/hdpvr/hdpvr-core.c
index 8247c19..3e80202 100644
--- a/drivers/media/usb/hdpvr/hdpvr-core.c
+++ b/drivers/media/usb/hdpvr/hdpvr-core.c
@@ -53,6 +53,10 @@ static bool boost_audio;
module_param(boost_audio, bool, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(boost_audio, boost the audio signal);
+int ir_rx_enable;
+module_param(ir_rx_enable, int, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(ir_rx_enable, Enable HDPVR IR receiver (firmware may be
unstable));
+
/* table of devices that work with this driver */
static struct usb_device_id hdpvr_table[] = {
@@ -394,11 +398,13 @@ static int hdpvr_probe(struct usb_interface *interface,
goto error;
}
- client = hdpvr_register_ir_rx_i2c(dev);
- if (!client) {
- v4l2_err(dev-v4l2_dev, i2c IR RX device register failed\n);
- retval = -ENODEV;
- goto reg_fail;
+ if (ir_rx_enable) {
+ client = hdpvr_register_ir_rx_i2c(dev);
+ if (!client) {
+ v4l2_err(dev-v4l2_dev, i2c IR RX device register
failed\n);
+ retval = -ENODEV;
+ goto reg_fail;
+ }
}
client = hdpvr_register_ir_tx_i2c(dev);
--
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 RFC v3] media: i2c: mt9p031: add OF support
On Tue, May 14, 2013 at 12:59:27AM +0200, Laurent Pinchart wrote:
Hi Sascha,
On Monday 13 May 2013 12:46:04 Sascha Hauer wrote:
On Wed, May 08, 2013 at 12:37:29PM +0200, Laurent Pinchart wrote:
Hi Prabhakar,
On Wednesday 08 May 2013 10:19:57 Prabhakar Lad wrote:
On Wed, May 8, 2013 at 7:32 AM, Laurent Pinchart wrote:
On Tuesday 07 May 2013 15:10:36 Prabhakar Lad wrote:
On Mon, May 6, 2013 at 8:29 PM, Prabhakar Lad wrote:
On Fri, May 3, 2013 at 8:04 PM, Arnd Bergmann wrote:
On Friday 03 May 2013, Prabhakar Lad wrote:
[snip]
+}
Ok, good.
@@ -955,7 +998,17 @@ static int mt9p031_probe(struct i2c_client
*client,
mt9p031-pdata = pdata;
mt9p031-output_control = MT9P031_OUTPUT_CONTROL_DEF;
mt9p031-mode2 = MT9P031_READ_MODE_2_ROW_BLC;
- mt9p031-model = did-driver_data;
+
+ if (!client-dev.of_node) {
+ mt9p031-model = (enum
mt9p031_model)did-driver_data;
+ } else {
+ const struct of_device_id *of_id;
+
+ of_id =
of_match_device(of_match_ptr(mt9p031_of_match),
+ client-dev);
+ if (of_id)
+ mt9p031-model = (enum
mt9p031_model)of_id-data;
+ }
mt9p031-reset = -1;
Is this actually required? I thought the i2c core just compared
the
part of the compatible value after the first comma to the
string, so
mt9p031-model = (enum mt9p031_model)did-driver_data should
work
in both cases.
At least on v3.8 I just checked that 'did' is indeed NULL for the
devicetree case. Also I see no indication that i2c starts comparing
after the first comma in the string.
I am OK with mt9p031-model = (enum
mt9p031_model)did-driver_data
but I see still few drivers doing this, I am not sure for what
reason.
If everyone is OK with it I can drop the above change.
My bad, while booting with DT the i2c_device_id ie did in this case
will
be NULL, so the above changes are required :-)
I've just tested your patch, and did isn't NULL when booting my
Beagleboard with DT (on v3.9-rc5).
I am pretty much sure you tested it compatible property as
aptina,mt9p031
if the compatible property is set to aptina,mt9p031m the did will be
NULL.
I've tested both :-)
Sorry to nag, but did you use aptina,mt9p031[m] as a compatible string or
did you use mt9p031[m]. With aptina,... 'did' should really be NULL.
I've used aptina,mt9p031[m].
Please see the of_modalias_node() call in of_i2c_register_devices()
(drivers/of/of-i2c.c), that's where the I2C device type name should be
initialized.
Ok, got it. I still had the older aptina,mt9p031m-sensor binding in my
patch.
Sorry for the noise.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] media: remove duplicate check for EPERM
From: Lad, Prabhakar prabhakar.cse...@gmail.com This patch series cleanups the check for EPERM in dbg_g/s_register and vidioc_g/s_register. Lad, Prabhakar (4): media: i2c: remove duplicate checks for EPERM in dbg_g/s_register media: dvb-frontends: remove duplicate checks for EPERM in dbg_g/s_register media: usb: remove duplicate checks for EPERM in vidioc_g/s_register media: pci: remove duplicate checks for EPERM drivers/media/dvb-frontends/au8522_decoder.c |4 drivers/media/i2c/ad9389b.c |4 drivers/media/i2c/adv7183.c |4 drivers/media/i2c/adv7604.c |4 drivers/media/i2c/cs5345.c |4 drivers/media/i2c/cx25840/cx25840-core.c |4 drivers/media/i2c/m52790.c |4 drivers/media/i2c/mt9v011.c |4 drivers/media/i2c/ov7670.c |4 drivers/media/i2c/saa7115.c |4 drivers/media/i2c/saa7127.c |4 drivers/media/i2c/saa717x.c |4 drivers/media/i2c/ths7303.c |4 drivers/media/i2c/tvp5150.c |4 drivers/media/i2c/tvp7002.c | 10 ++ drivers/media/i2c/upd64031a.c|4 drivers/media/i2c/upd64083.c |4 drivers/media/i2c/vs6624.c |4 drivers/media/pci/bt8xx/bttv-driver.c|6 -- drivers/media/pci/cx18/cx18-av-core.c|4 drivers/media/pci/cx23885/cx23885-ioctl.c|6 -- drivers/media/pci/cx23885/cx23888-ir.c |4 drivers/media/pci/ivtv/ivtv-ioctl.c |2 -- drivers/media/pci/saa7146/mxb.c |4 drivers/media/pci/saa7164/saa7164-encoder.c |6 -- drivers/media/usb/pvrusb2/pvrusb2-hdw.c |2 -- 26 files changed, 2 insertions(+), 110 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] media: i2c: remove duplicate checks for EPERM in dbg_g/s_register
From: Lad, Prabhakar prabhakar.cse...@gmail.com
This patch removes check for EPERM in dbg_g/s_register of subdevice
drivers as this check is already performed by core.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
drivers/media/i2c/ad9389b.c |4
drivers/media/i2c/adv7183.c |4
drivers/media/i2c/adv7604.c |4
drivers/media/i2c/cs5345.c |4
drivers/media/i2c/cx25840/cx25840-core.c |4
drivers/media/i2c/m52790.c |4
drivers/media/i2c/mt9v011.c |4
drivers/media/i2c/ov7670.c |4
drivers/media/i2c/saa7115.c |4
drivers/media/i2c/saa7127.c |4
drivers/media/i2c/saa717x.c |4
drivers/media/i2c/ths7303.c |4
drivers/media/i2c/tvp5150.c |4
drivers/media/i2c/tvp7002.c | 10 ++
drivers/media/i2c/upd64031a.c|4
drivers/media/i2c/upd64083.c |4
drivers/media/i2c/vs6624.c |4
17 files changed, 2 insertions(+), 72 deletions(-)
diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c
index 58344b6..bd77895 100644
--- a/drivers/media/i2c/ad9389b.c
+++ b/drivers/media/i2c/ad9389b.c
@@ -347,8 +347,6 @@ static int ad9389b_g_register(struct v4l2_subdev *sd,
struct v4l2_dbg_register *
if (!v4l2_chip_match_i2c_client(client, reg-match))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
reg-val = ad9389b_rd(sd, reg-reg 0xff);
reg-size = 1;
return 0;
@@ -360,8 +358,6 @@ static int ad9389b_s_register(struct v4l2_subdev *sd, const
struct v4l2_dbg_regi
if (!v4l2_chip_match_i2c_client(client, reg-match))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
ad9389b_wr(sd, reg-reg 0xff, reg-val 0xff);
return 0;
}
diff --git a/drivers/media/i2c/adv7183.c b/drivers/media/i2c/adv7183.c
index 56a1fa4..fa7bb46 100644
--- a/drivers/media/i2c/adv7183.c
+++ b/drivers/media/i2c/adv7183.c
@@ -500,8 +500,6 @@ static int adv7183_g_register(struct v4l2_subdev *sd,
struct v4l2_dbg_register *
if (!v4l2_chip_match_i2c_client(client, reg-match))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
reg-val = adv7183_read(sd, reg-reg 0xff);
reg-size = 1;
return 0;
@@ -513,8 +511,6 @@ static int adv7183_s_register(struct v4l2_subdev *sd, const
struct v4l2_dbg_regi
if (!v4l2_chip_match_i2c_client(client, reg-match))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
adv7183_write(sd, reg-reg 0xff, reg-val 0xff);
return 0;
}
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 31a63c9..e29e7c8 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -647,8 +647,6 @@ static int adv7604_g_register(struct v4l2_subdev *sd,
if (!v4l2_chip_match_i2c_client(client, reg-match))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
reg-size = 1;
switch (reg-reg 8) {
case 0:
@@ -705,8 +703,6 @@ static int adv7604_s_register(struct v4l2_subdev *sd,
if (!v4l2_chip_match_i2c_client(client, reg-match))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
switch (reg-reg 8) {
case 0:
io_write(sd, reg-reg 0xff, reg-val 0xff);
diff --git a/drivers/media/i2c/cs5345.c b/drivers/media/i2c/cs5345.c
index 1d2f7c8..d27d9b7 100644
--- a/drivers/media/i2c/cs5345.c
+++ b/drivers/media/i2c/cs5345.c
@@ -103,8 +103,6 @@ static int cs5345_g_register(struct v4l2_subdev *sd, struct
v4l2_dbg_register *r
if (!v4l2_chip_match_i2c_client(client, reg-match))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
reg-size = 1;
reg-val = cs5345_read(sd, reg-reg 0x1f);
return 0;
@@ -116,8 +114,6 @@ static int cs5345_s_register(struct v4l2_subdev *sd, const
struct v4l2_dbg_regis
if (!v4l2_chip_match_i2c_client(client, reg-match))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
cs5345_write(sd, reg-reg 0x1f, reg-val 0xff);
return 0;
}
diff --git a/drivers/media/i2c/cx25840/cx25840-core.c
b/drivers/media/i2c/cx25840/cx25840-core.c
index 12fb9b2..33c4e51 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.c
+++ b/drivers/media/i2c/cx25840/cx25840-core.c
@@ -1664,8 +1664,6 @@ static int cx25840_g_register(struct v4l2_subdev *sd,
struct v4l2_dbg_register *
if (!v4l2_chip_match_i2c_client(client, reg-match))
[PATCH 2/4] media: dvb-frontends: remove duplicate checks for EPERM in dbg_g/s_register
From: Lad, Prabhakar prabhakar.cse...@gmail.com This patch removes check for EPERM in dbg_g/s_register as this check is already performed by core. Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- drivers/media/dvb-frontends/au8522_decoder.c |4 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/drivers/media/dvb-frontends/au8522_decoder.c b/drivers/media/dvb-frontends/au8522_decoder.c index 2099f21..9d159b4 100644 --- a/drivers/media/dvb-frontends/au8522_decoder.c +++ b/drivers/media/dvb-frontends/au8522_decoder.c @@ -529,8 +529,6 @@ static int au8522_g_register(struct v4l2_subdev *sd, if (!v4l2_chip_match_i2c_client(client, reg-match)) return -EINVAL; - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; reg-val = au8522_readreg(state, reg-reg 0x); return 0; } @@ -543,8 +541,6 @@ static int au8522_s_register(struct v4l2_subdev *sd, if (!v4l2_chip_match_i2c_client(client, reg-match)) return -EINVAL; - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; au8522_writereg(state, reg-reg, reg-val 0xff); return 0; } -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] media: usb: remove duplicate checks for EPERM in vidioc_g/s_register
From: Lad, Prabhakar prabhakar.cse...@gmail.com This patch removes check for EPERM in vidioc_g/s_register as this check is already performed by core. Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- drivers/media/usb/pvrusb2/pvrusb2-hdw.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c index e11267f..01d1c2d 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c @@ -5173,8 +5173,6 @@ int pvr2_hdw_register_access(struct pvr2_hdw *hdw, int stat = 0; int okFl = 0; - if (!capable(CAP_SYS_ADMIN)) return -EPERM; - req.match = *match; req.reg = reg_id; if (setFl) req.val = *val_ptr; -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] media: pci: remove duplicate checks for EPERM
From: Lad, Prabhakar prabhakar.cse...@gmail.com
This patch removes check for EPERM in dbg_g/s_register and
vidioc_g/s_register as this check is already performed by core.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
drivers/media/pci/bt8xx/bttv-driver.c |6 --
drivers/media/pci/cx18/cx18-av-core.c |4
drivers/media/pci/cx23885/cx23885-ioctl.c |6 --
drivers/media/pci/cx23885/cx23888-ir.c |4
drivers/media/pci/ivtv/ivtv-ioctl.c |2 --
drivers/media/pci/saa7146/mxb.c |4
drivers/media/pci/saa7164/saa7164-encoder.c |6 --
7 files changed, 0 insertions(+), 32 deletions(-)
diff --git a/drivers/media/pci/bt8xx/bttv-driver.c
b/drivers/media/pci/bt8xx/bttv-driver.c
index e7d0884..a334c94 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -1936,9 +1936,6 @@ static int bttv_g_register(struct file *file, void *f,
struct bttv_fh *fh = f;
struct bttv *btv = fh-btv;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
-
if (!v4l2_chip_match_host(reg-match)) {
/* TODO: subdev errors should not be ignored, this should
become a
subdev helper function. */
@@ -1960,9 +1957,6 @@ static int bttv_s_register(struct file *file, void *f,
struct bttv_fh *fh = f;
struct bttv *btv = fh-btv;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
-
if (!v4l2_chip_match_host(reg-match)) {
/* TODO: subdev errors should not be ignored, this should
become a
subdev helper function. */
diff --git a/drivers/media/pci/cx18/cx18-av-core.c
b/drivers/media/pci/cx18/cx18-av-core.c
index 38b1d64..ba8caf0 100644
--- a/drivers/media/pci/cx18/cx18-av-core.c
+++ b/drivers/media/pci/cx18/cx18-av-core.c
@@ -1258,8 +1258,6 @@ static int cx18_av_g_register(struct v4l2_subdev *sd,
return -EINVAL;
if ((reg-reg 0x3) != 0)
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
reg-size = 4;
reg-val = cx18_av_read4(cx, reg-reg 0x0ffc);
return 0;
@@ -1274,8 +1272,6 @@ static int cx18_av_s_register(struct v4l2_subdev *sd,
return -EINVAL;
if ((reg-reg 0x3) != 0)
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
cx18_av_write4(cx, reg-reg 0x0ffc, reg-val);
return 0;
}
diff --git a/drivers/media/pci/cx23885/cx23885-ioctl.c
b/drivers/media/pci/cx23885/cx23885-ioctl.c
index acdb6d5..00f5125 100644
--- a/drivers/media/pci/cx23885/cx23885-ioctl.c
+++ b/drivers/media/pci/cx23885/cx23885-ioctl.c
@@ -138,9 +138,6 @@ int cx23885_g_register(struct file *file, void *fh,
{
struct cx23885_dev *dev = ((struct cx23885_fh *)fh)-dev;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
-
if (reg-match.type == V4L2_CHIP_MATCH_HOST) {
switch (reg-match.addr) {
case 0:
@@ -186,9 +183,6 @@ int cx23885_s_register(struct file *file, void *fh,
{
struct cx23885_dev *dev = ((struct cx23885_fh *)fh)-dev;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
-
if (reg-match.type == V4L2_CHIP_MATCH_HOST) {
switch (reg-match.addr) {
case 0:
diff --git a/drivers/media/pci/cx23885/cx23888-ir.c
b/drivers/media/pci/cx23885/cx23888-ir.c
index fa672fe..cd98651 100644
--- a/drivers/media/pci/cx23885/cx23888-ir.c
+++ b/drivers/media/pci/cx23885/cx23888-ir.c
@@ -1116,8 +1116,6 @@ static int cx23888_ir_g_register(struct v4l2_subdev *sd,
return -EINVAL;
if (addr CX23888_IR_CNTRL_REG || addr CX23888_IR_LEARN_REG)
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
reg-size = 4;
reg-val = cx23888_ir_read4(state-dev, addr);
return 0;
@@ -1135,8 +1133,6 @@ static int cx23888_ir_s_register(struct v4l2_subdev *sd,
return -EINVAL;
if (addr CX23888_IR_CNTRL_REG || addr CX23888_IR_LEARN_REG)
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
cx23888_ir_write4(state-dev, addr, reg-val);
return 0;
}
diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.c
b/drivers/media/pci/ivtv/ivtv-ioctl.c
index 9cbbce0..3e281ec 100644
--- a/drivers/media/pci/ivtv/ivtv-ioctl.c
+++ b/drivers/media/pci/ivtv/ivtv-ioctl.c
@@ -715,8 +715,6 @@ static int ivtv_itvc(struct ivtv *itv, bool get, u64 reg,
u64 *val)
{
volatile u8 __iomem *reg_start;
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
if (reg = IVTV_REG_OFFSET reg IVTV_REG_OFFSET + IVTV_REG_SIZE)
reg_start = itv-reg_mem - IVTV_REG_OFFSET;
else if (itv-has_cx23415 reg = IVTV_DECODER_OFFSET
diff --git
