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: Wed Aug 12 04:00:28 CEST 2015 git branch: test git hash: 2696f495bdc046d84da6c909a1e7f535138a2a62 gcc version:i686-linux-gcc (GCC) 5.1.0 sparse version: v0.5.0-51-ga53cea2 smatch version: 0.4.1-3153-g7d56ab3 host hardware: x86_64 host os:4.0.0-3.slh.1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin-bf561: ERRORS linux-git-i686: OK linux-git-m32r: OK linux-git-mips: ERRORS linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: WARNINGS linux-2.6.32.27-i686: ERRORS linux-2.6.33.7-i686: ERRORS linux-2.6.34.7-i686: ERRORS linux-2.6.35.9-i686: ERRORS linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.1.10-i686: ERRORS linux-3.2.37-i686: ERRORS linux-3.3.8-i686: ERRORS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: ERRORS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: ERRORS linux-3.10.1-i686: ERRORS linux-3.11.1-i686: ERRORS linux-3.12.23-i686: ERRORS linux-3.13.11-i686: ERRORS linux-3.14.9-i686: ERRORS linux-3.15.2-i686: ERRORS linux-3.16.7-i686: ERRORS linux-3.17.8-i686: ERRORS linux-3.18.7-i686: ERRORS linux-3.19-i686: ERRORS linux-4.0-i686: ERRORS linux-4.1.1-i686: ERRORS linux-4.2-rc1-i686: WARNINGS linux-2.6.32.27-x86_64: ERRORS linux-2.6.33.7-x86_64: ERRORS linux-2.6.34.7-x86_64: ERRORS linux-2.6.35.9-x86_64: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.37-x86_64: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: ERRORS linux-3.10.1-x86_64: ERRORS linux-3.11.1-x86_64: ERRORS linux-3.12.23-x86_64: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.9-x86_64: ERRORS linux-3.15.2-x86_64: ERRORS linux-3.16.7-x86_64: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.7-x86_64: ERRORS linux-3.19-x86_64: ERRORS linux-4.0-x86_64: ERRORS linux-4.1.1-x86_64: ERRORS linux-4.2-rc1-x86_64: WARNINGS apps: OK spec-git: OK sparse: ERRORS ABI WARNING: change for blackfin-bf561 ABI WARNING: change for mips smatch: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The 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 3/3] [media] c8sectpfe: use a new Kconfig menu for DVB platform drivers
While this is the first DVB platform drivers, let's keep the Kconfig options well organized, adding it on its own DVB menu. Of course, it should depend on MEDIA_DIGITAL_TV_SUPPORT, as this enables all DVB-related menus. Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index ce3eaf050ba7..3adf686e005d 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -293,4 +293,13 @@ config VIDEO_VIM2M framework. endif #V4L_TEST_DRIVERS +menuconfig DVB_PLATFORM_DRIVERS + bool DVB platform devices + depends on MEDIA_DIGITAL_TV_SUPPORT + default n + ---help--- + Say Y here to enable support for platform-specific Digital TV drivers. + +if DVB_PLATFORM_DRIVERS source drivers/media/platform/sti/c8sectpfe/Kconfig +endif #DVB_PLATFORM_DRIVERS -- 2.4.3 -- To unsubscribe from this list: send the line unsubscribe 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/3] [media] tda10071: use div_s64() when dividing a s64 integer
Otherwise, it will break on 32 bits archs.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/dvb-frontends/tda10071.c
b/drivers/media/dvb-frontends/tda10071.c
index ee6653124618..119d47596ac8 100644
--- a/drivers/media/dvb-frontends/tda10071.c
+++ b/drivers/media/dvb-frontends/tda10071.c
@@ -527,7 +527,7 @@ static int tda10071_read_signal_strength(struct
dvb_frontend *fe, u16 *strength)
unsigned int uitmp;
if (c-strength.stat[0].scale == FE_SCALE_DECIBEL) {
- uitmp = c-strength.stat[0].svalue / 1000 + 256;
+ uitmp = div_s64(c-strength.stat[0].svalue, 1000) + 256;
uitmp = clamp(uitmp, 181U, 236U); /* -75dBm - -20dBm */
/* scale value to 0x-0x */
*strength = (uitmp-181) * 0x / (236-181);
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe 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/3] [media] c8sectpfe: fix pinctrl dependencies
compiling on some archs fail with: drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c:540:8: error: implicit declaration of function ‘pinctrl_select_state’ [-Werror=implicit-function-declaration] ret = pinctrl_select_state(fei-pinctrl, tsin-pstate); That's due the need of including pinctrl.h header and because CONFIG_PINCTRL needs to be true. Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com diff --git a/drivers/media/platform/sti/c8sectpfe/Kconfig b/drivers/media/platform/sti/c8sectpfe/Kconfig index a7227d2ab6cc..1b1110d7374f 100644 --- a/drivers/media/platform/sti/c8sectpfe/Kconfig +++ b/drivers/media/platform/sti/c8sectpfe/Kconfig @@ -1,6 +1,7 @@ config DVB_C8SECTPFE tristate STMicroelectronics C8SECTPFE DVB support - depends on DVB_CORE I2C (ARCH_STI || ARCH_MULTIPLATFORM || COMPILE_TEST) + depends on PINCTRL DVB_CORE I2C + depends on ARCH_STI || ARCH_MULTIPLATFORM || COMPILE_TEST select LIBELF_32 select FW_LOADER select FW_LOADER_USER_HELPER_FALLBACK diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c index 955d8daf055f..1586a1e6836d 100644 --- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c +++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c @@ -33,6 +33,7 @@ #include linux/time.h #include linux/version.h #include linux/wait.h +#include linux/pinctrl/pinctrl.h #include c8sectpfe-core.h #include c8sectpfe-common.h -- 2.4.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] [media] tda10071: use div_s64() when dividing a s64 integer
On 08/12/2015 01:39 AM, Mauro Carvalho Chehab wrote:
Otherwise, it will break on 32 bits archs.
Look good!
Antti
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/dvb-frontends/tda10071.c
b/drivers/media/dvb-frontends/tda10071.c
index ee6653124618..119d47596ac8 100644
--- a/drivers/media/dvb-frontends/tda10071.c
+++ b/drivers/media/dvb-frontends/tda10071.c
@@ -527,7 +527,7 @@ static int tda10071_read_signal_strength(struct
dvb_frontend *fe, u16 *strength)
unsigned int uitmp;
if (c-strength.stat[0].scale == FE_SCALE_DECIBEL) {
- uitmp = c-strength.stat[0].svalue / 1000 + 256;
+ uitmp = div_s64(c-strength.stat[0].svalue, 1000) + 256;
uitmp = clamp(uitmp, 181U, 236U); /* -75dBm - -20dBm */
/* scale value to 0x-0x */
*strength = (uitmp-181) * 0x / (236-181);
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: VIMC: API proposal, configuring the topology through user space
Hi Helen, On 08/10/15 19:21, Helen Fornazier wrote: Hi, thanks for your reviews On Mon, Aug 10, 2015 at 10:11 AM, Hans Verkuil hverk...@xs4all.nl wrote: Hi Helen! On 08/08/2015 03:55 AM, Helen Fornazier wrote: Hi! I've made a first sketch about the API of the vimc driver (virtual media controller) to configure the topology through user space. As first suggested by Laurent Pinchart, it is based on configfs. I would like to know you opinion about it, if you have any suggestion to improve it, otherwise I'll start its implementation as soon as possible. This API may change with the MC changes and I may see other possible configurations as I implementing it but here is the first idea of how the API will look like. vimc project link: https://github.com/helen-fornazier/opw-staging/ For more information: http://kernelnewbies.org/LaurentPinchart /*** The API: / In short, a topology like this one: http://goo.gl/Y7eUfu Would look like this filesystem tree: https://goo.gl/tCZPTg Txt version of the filesystem tree: https://goo.gl/42KX8Y * The /configfs/vimc subsystem I haven't checked the latest vimc driver, but doesn't it support multiple instances, just like vivid? I would certainly recommend that. Yes, it does support And if there are multiple instances, then each instance gets its own entry in configfs: /configs/vimc0, /configs/vimc1, etc. You are right, I'll add those The vimc driver registers a subsystem in the configfs with the following contents: ls /configfs/vimc build_topology status The build_topology attribute file will be used to tell the driver the configuration is done and it can build the topology internally echo -n anything here /configfs/vimc/build_topology Reading from the status attribute can have 3 different classes of outputs 1) deployed: the current configured tree is built 2) undeployed: no errors, the user has modified the configfs tree thus the topology was undeployed 3) error error_message: the topology configuration is wrong I don't see the status file in the filesystem tree links above. Sorry, I forgot to add I actually wonder if we can't use build_topology for that: reading it will just return the status. Yes we can, I was just wondering what should be the name of the file, as getting the status from reading the build_topology file doesn't seem much intuitive I'm not opposed to a status file, but it would be good to know what Laurent thinks. What happens if it is deployed and you want to 'undeploy' it? Instead of writing anything to build_topology it might be useful to write a real command to it. E.g. 'deploy', 'destroy'. What happens when you make changes while a topology is deployed? Should such changes be rejected until the usecount of the driver goes to 0, or will it only be rejected when you try to deploy the new configuration? I was thinking that if the user try changing the topology, or it will fail (because the device instance is in use) or it will undeploy the old topology (if the device is not in use). Then a 'destroy' command won't be useful, the user can just unload the driver when it won't be used anymore, If you have multiple vimc instances and you want to 'destroy' the topology of only one instance, then you can't rmmod the module. I think I would prefer to have proper commands for the build_topology file. It would also keep the state handling of the driver simple: it's either deployed or undeployed, and changes to the topology can only take place if it is undeployed. Commands for build_topology can be: deploy: deploy the current topology undeploy: undeploy the current topology. but keep the topology, allowing the user to make changes destroy: undeploy the current topology and remove it, giving the user a clean slate. Feel free to come up with better command names :-) * Creating an entity: Use mkdir in the /configfs/vimc to create an entity representation, e.g.: mkdir /configfs/vimc/sensor_a The attribute files will be created by the driver through configfs: ls /configfs/vimc/sensor_a name role Configure the name that will appear to the /dev/media0 and what this node do (debayer, scaler, capture, input, generic) echo -n Sensor A /configfs/vimc/sensor_a/name echo -n sensor /configfs/vimc/sensor_a/role * Creating a pad: Use mkdir inside an entity's folder, the attribute called direction will be automatically created in the process, for example: mkdir /configfs/vimc/sensor_a/pad_0 ls /configfs/vimc/sensor_a/pad_0 direction echo -n source /configfs/vimc/sensor_a/pad_0/direction The direction attribute can be source or sink Hmm. Aren't the pads+direction dictated by the entity role? E.g. a sensor will only have one pad which is always a source. I think setting the role is what creates the pad
Re: VIMC: API proposal, configuring the topology through user space
Em Tue, 11 Aug 2015 11:28:25 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: Hi Helen, On 08/10/15 19:21, Helen Fornazier wrote: Hi, thanks for your reviews On Mon, Aug 10, 2015 at 10:11 AM, Hans Verkuil hverk...@xs4all.nl wrote: Hi Helen! On 08/08/2015 03:55 AM, Helen Fornazier wrote: Hi! I've made a first sketch about the API of the vimc driver (virtual media controller) to configure the topology through user space. As first suggested by Laurent Pinchart, it is based on configfs. I would like to know you opinion about it, if you have any suggestion to improve it, otherwise I'll start its implementation as soon as possible. This API may change with the MC changes and I may see other possible configurations as I implementing it but here is the first idea of how the API will look like. vimc project link: https://github.com/helen-fornazier/opw-staging/ For more information: http://kernelnewbies.org/LaurentPinchart /*** The API: / In short, a topology like this one: http://goo.gl/Y7eUfu Would look like this filesystem tree: https://goo.gl/tCZPTg Txt version of the filesystem tree: https://goo.gl/42KX8Y * The /configfs/vimc subsystem I haven't checked the latest vimc driver, but doesn't it support multiple instances, just like vivid? I would certainly recommend that. Yes, it does support And if there are multiple instances, then each instance gets its own entry in configfs: /configs/vimc0, /configs/vimc1, etc. You are right, I'll add those The vimc driver registers a subsystem in the configfs with the following contents: ls /configfs/vimc build_topology status The build_topology attribute file will be used to tell the driver the configuration is done and it can build the topology internally echo -n anything here /configfs/vimc/build_topology Reading from the status attribute can have 3 different classes of outputs 1) deployed: the current configured tree is built 2) undeployed: no errors, the user has modified the configfs tree thus the topology was undeployed 3) error error_message: the topology configuration is wrong I don't see the status file in the filesystem tree links above. Sorry, I forgot to add I actually wonder if we can't use build_topology for that: reading it will just return the status. Yes we can, I was just wondering what should be the name of the file, as getting the status from reading the build_topology file doesn't seem much intuitive I'm not opposed to a status file, but it would be good to know what Laurent thinks. What happens if it is deployed and you want to 'undeploy' it? Instead of writing anything to build_topology it might be useful to write a real command to it. E.g. 'deploy', 'destroy'. What happens when you make changes while a topology is deployed? Should such changes be rejected until the usecount of the driver goes to 0, or will it only be rejected when you try to deploy the new configuration? I was thinking that if the user try changing the topology, or it will fail (because the device instance is in use) or it will undeploy the old topology (if the device is not in use). Then a 'destroy' command won't be useful, the user can just unload the driver when it won't be used anymore, Well, we're planning to add support for dynamic addition/removal of entities, interfaces, pads and links. So, instead of undeploy the old topology, one could just do: rm -rf part of the tree If you have multiple vimc instances and you want to 'destroy' the topology of only one instance, then you can't rmmod the module. You can still use rm remove just one entire instance of the topology. I think I would prefer to have proper commands for the build_topology file. It would also keep the state handling of the driver simple: it's either deployed or undeployed, and changes to the topology can only take place if it is undeployed. Commands for build_topology can be: deploy: deploy the current topology undeploy: undeploy the current topology. but keep the topology, allowing the user to make changes destroy: undeploy the current topology and remove it, giving the user a clean slate. Feel free to come up with better command names :-) * Creating an entity: Use mkdir in the /configfs/vimc to create an entity representation, e.g.: mkdir /configfs/vimc/sensor_a The attribute files will be created by the driver through configfs: ls /configfs/vimc/sensor_a name role Configure the name that will appear to the /dev/media0 and what this node do (debayer, scaler, capture, input, generic) echo -n Sensor A /configfs/vimc/sensor_a/name echo -n sensor /configfs/vimc/sensor_a/role * Creating a pad: Use mkdir inside an entity's folder, the attribute called
Re: [RFC PATCH v2 4/5] media: videobuf2: Define vb2_buf_type and vb2_memory
On 08/11/2015 04:19 AM, Junghak Sung wrote:
Hi Hans,
On 08/10/2015 05:22 PM, Hans Verkuil wrote:
On 07/31/2015 10:44 AM, Junghak Sung wrote:
Define enum vb2_buf_type and enum vb2_memory for videobuf2-core. This
change requires translation functions that could covert v4l2-core stuffs
to videobuf2-core stuffs in videobuf2-v4l2.c file.
The v4l2-specific member variables(e.g. type, memory) remains in
struct vb2_queue for backward compatibility and performance of type
translation.
Signed-off-by: Junghak Sung jh1009.s...@samsung.com
Signed-off-by: Geunyoung Kim nenggun@samsung.com
Acked-by: Seung-Woo Kim sw0312@samsung.com
Acked-by: Inki Dae inki@samsung.com
---
drivers/media/v4l2-core/videobuf2-core.c | 139 +++-
drivers/media/v4l2-core/videobuf2-v4l2.c | 209
--
include/media/videobuf2-core.h | 99 +++---
include/media/videobuf2-v4l2.h | 12 +-
4 files changed, 299 insertions(+), 160 deletions(-)
snip
diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c
b/drivers/media/v4l2-core/videobuf2-v4l2.c
index 85527e9..22dd19c 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -30,8 +30,46 @@
#include media/v4l2-common.h
#include media/videobuf2-v4l2.h
+#define CREATE_TRACE_POINTS
#include trace/events/v4l2.h
+static const enum vb2_buf_type _tbl_buf_type[] = {
+ [V4L2_BUF_TYPE_VIDEO_CAPTURE] = VB2_BUF_TYPE_VIDEO_CAPTURE,
+ [V4L2_BUF_TYPE_VIDEO_OUTPUT]= VB2_BUF_TYPE_VIDEO_OUTPUT,
+ [V4L2_BUF_TYPE_VIDEO_OVERLAY] = VB2_BUF_TYPE_VIDEO_OVERLAY,
+ [V4L2_BUF_TYPE_VBI_CAPTURE] = VB2_BUF_TYPE_VBI_CAPTURE,
+ [V4L2_BUF_TYPE_VBI_OUTPUT] = VB2_BUF_TYPE_VBI_OUTPUT,
+ [V4L2_BUF_TYPE_SLICED_VBI_CAPTURE] =
VB2_BUF_TYPE_SLICED_VBI_CAPTURE,
+ [V4L2_BUF_TYPE_SLICED_VBI_OUTPUT] =
VB2_BUF_TYPE_SLICED_VBI_OUTPUT,
+ [V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY]=
VB2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY,
+ [V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE]=
VB2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ [V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE] =
VB2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ [V4L2_BUF_TYPE_SDR_CAPTURE] = VB2_BUF_TYPE_SDR_CAPTURE,
+ [V4L2_BUF_TYPE_PRIVATE] = VB2_BUF_TYPE_PRIVATE,
+};
+
+static const enum vb2_memory _tbl_memory[] = {
+ [V4L2_MEMORY_MMAP] = VB2_MEMORY_MMAP,
+ [V4L2_MEMORY_USERPTR] = VB2_MEMORY_USERPTR,
+ [V4L2_MEMORY_DMABUF]= VB2_MEMORY_DMABUF,
+};
+
+#define to_vb2_buf_type(type) \
+({ \
+ enum vb2_buf_type ret = 0; \
+ if( type 0 type ARRAY_SIZE(_tbl_buf_type) ) \
+ ret = (_tbl_buf_type[type]);\
+ ret;\
+})
+
+#define to_vb2_memory(memory) \
+({ \
+ enum vb2_memory ret = 0;\
+ if( memory 0 memory ARRAY_SIZE(_tbl_memory) )\
+ ret = (_tbl_memory[memory]);\
+ ret;\
+})
+
snip
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index dc405da..871fcc6 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -15,9 +15,47 @@
#include linux/mm_types.h
#include linux/mutex.h
#include linux/poll.h
-#include linux/videodev2.h
#include linux/dma-buf.h
+#define VB2_MAX_FRAME 32
+#define VB2_MAX_PLANES 8
+
+enum vb2_buf_type {
+ VB2_BUF_TYPE_UNKNOWN= 0,
+ VB2_BUF_TYPE_VIDEO_CAPTURE = 1,
+ VB2_BUF_TYPE_VIDEO_OUTPUT = 2,
+ VB2_BUF_TYPE_VIDEO_OVERLAY = 3,
+ VB2_BUF_TYPE_VBI_CAPTURE= 4,
+ VB2_BUF_TYPE_VBI_OUTPUT = 5,
+ VB2_BUF_TYPE_SLICED_VBI_CAPTURE = 6,
+ VB2_BUF_TYPE_SLICED_VBI_OUTPUT = 7,
+ VB2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY = 8,
+ VB2_BUF_TYPE_VIDEO_CAPTURE_MPLANE = 9,
+ VB2_BUF_TYPE_VIDEO_OUTPUT_MPLANE= 10,
+ VB2_BUF_TYPE_SDR_CAPTURE= 11,
+ VB2_BUF_TYPE_DVB_CAPTURE= 12,
+ VB2_BUF_TYPE_PRIVATE= 0x80,
+};
+
+enum vb2_memory {
+ VB2_MEMORY_UNKNOWN = 0,
+ VB2_MEMORY_MMAP = 1,
+ VB2_MEMORY_USERPTR = 2,
+ VB2_MEMORY_DMABUF = 4,
+};
+
+#define VB2_TYPE_IS_MULTIPLANAR(type) \
+ ((type) == VB2_BUF_TYPE_VIDEO_CAPTURE_MPLANE\
+|| (type) == VB2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+
+#define VB2_TYPE_IS_OUTPUT(type) \
+ ((type) ==
Re: [PATCH 09/12] tda10071: use jiffies when poll firmware status
Em Thu, 9 Jul 2015 07:06:29 +0300
Antti Palosaari cr...@iki.fi escreveu:
Use jiffies to set timeout for firmware command status polling.
It is more elegant solution than poll X times with sleep.
Shorten timeout to 30ms as all commands seems to be executed under
10ms.
Signed-off-by: Antti Palosaari cr...@iki.fi
---
drivers/media/dvb-frontends/tda10071.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/media/dvb-frontends/tda10071.c
b/drivers/media/dvb-frontends/tda10071.c
index 6226b57..c1507cc 100644
--- a/drivers/media/dvb-frontends/tda10071.c
+++ b/drivers/media/dvb-frontends/tda10071.c
@@ -53,8 +53,9 @@ static int tda10071_cmd_execute(struct tda10071_dev *dev,
struct tda10071_cmd *cmd)
{
struct i2c_client *client = dev-client;
- int ret, i;
+ int ret;
unsigned int uitmp;
+ unsigned long timeout;
if (!dev-warm) {
ret = -EFAULT;
@@ -72,17 +73,19 @@ static int tda10071_cmd_execute(struct tda10071_dev *dev,
goto error;
/* wait cmd execution terminate */
- for (i = 1000, uitmp = 1; i uitmp; i--) {
+ #define CMD_EXECUTE_TIMEOUT 30
+ timeout = jiffies + msecs_to_jiffies(CMD_EXECUTE_TIMEOUT);
+ for (uitmp = 1; !time_after(jiffies, timeout) uitmp;) {
ret = regmap_read(dev-regmap, 0x1f, uitmp);
if (ret)
goto error;
-
- usleep_range(200, 5000);
Hmm... removing the usleep() doesn't sound a good idea. You'll be
flooding the I2C bus with read commands and spending CPU cycles
for 30ms spending more power than the previous code. That doesn't
sound more elegant solution than poll X times with sleep for me.
So, I would keep the usleep_range() here and add a better
comment on the patch description.
I'll skip this patch from the git pull request, as, from your description,
this is just a cleanup patch. So, it shouldn't affect the other patches
at the series.
}
- dev_dbg(client-dev, loop=%d\n, i);
+ dev_dbg(client-dev, cmd execution took %u ms\n,
+ jiffies_to_msecs(jiffies) -
+ (jiffies_to_msecs(timeout) - CMD_EXECUTE_TIMEOUT));
- if (i == 0) {
+ if (uitmp) {
ret = -ETIMEDOUT;
goto error;
}
--
To unsubscribe from this list: send the line unsubscribe 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 v10.1] media: Add registration helpers for V4L2 flash sub-devices
On 08/11/15 14:56, Jacek Anaszewski wrote:
On 07/28/2015 12:00 PM, Hans Verkuil wrote:
On 06/19/2015 09:31 AM, Jacek Anaszewski wrote:
This patch adds helper functions for registering/unregistering
LED Flash class devices as V4L2 sub-devices. The functions should
be called from the LED subsystem device driver. In case the
support for V4L2 Flash sub-devices is disabled in the kernel
config the functions' empty versions will be used.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Hans Verkuil hans.verk...@cisco.com
---
- fixed possible NULL fled_cdev pointer dereference
in the v4l2_flash_init function
drivers/media/v4l2-core/Kconfig| 11 +
drivers/media/v4l2-core/Makefile |2 +
drivers/media/v4l2-core/v4l2-flash-led-class.c | 710
include/media/v4l2-flash-led-class.h | 148 +
4 files changed, 871 insertions(+)
create mode 100644 drivers/media/v4l2-core/v4l2-flash-led-class.c
create mode 100644 include/media/v4l2-flash-led-class.h
snip
diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c
b/drivers/media/v4l2-core/v4l2-flash-led-class.c
new file mode 100644
index 000..5bdfb8d
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c
snip
+static const struct v4l2_subdev_core_ops v4l2_flash_core_ops = {
+.queryctrl = v4l2_subdev_queryctrl,
+.querymenu = v4l2_subdev_querymenu,
Why are these here? This should not be necessary. As long as the
sd.ctrl_handler
pointer is set, this is handled automatically.
I removed these two lines and indeed driver works well without it.
+};
+
+static const struct v4l2_subdev_ops v4l2_flash_subdev_ops = {
+.core = v4l2_flash_core_ops,
+};
+
And if v4l2_flash_core_ops goes away, then this can go away as well.
What should I pass as the second argument to v4l2_subdev_init then?
It seems that ops can't be NULL:
void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops
*ops)
{
INIT_LIST_HEAD(sd-list);
BUG_ON(!ops); ---
In that case just give an empty ops struct.
I really need to look at this whether I can remove that BUG_ON.
Regards,
Hans
sd-ops = ops;
sd-v4l2_dev = NULL;
sd-flags = 0;
sd-name[0] = '\0';
sd-grp_id = 0;
sd-dev_priv = NULL;
sd-host_priv = NULL;
#if defined(CONFIG_MEDIA_CONTROLLER)
sd-entity.name = sd-name;
sd-entity.type = MEDIA_ENT_T_V4L2_SUBDEV;
#endif
}
I know this driver has been merged, but I just noticed this while looking at
something else.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/13] v4l: subdev: Add pad config allocator and init
Em Fri, 24 Jul 2015 16:12:44 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: On 07/23/2015 02:21 PM, William Towle wrote: From: Laurent Pinchart laurent.pinch...@linaro.org Add a new subdev operation to initialize a subdev pad config array, and a helper function to allocate and initialize the array. This can be used by bridge drivers to implement try format based on subdev pad operations. Signed-off-by: Laurent Pinchart laurent.pinch...@linaro.org Acked-by: Vaibhav Hiremath vaibhav.hirem...@linaro.org Acked-by: Hans Verkuil hans.verk...@cisco.com Won't merge this patch. The Media Controller implementation is currently broken. So, as agreed at the MC workshop, we won't be changing anything related to the MC while we don't rework its implementation in order to fix its mess. In this particular case, we'll very likely need to replace pads from arrays to linked lists, in order to properly support dynamic addition and removal. If we go to that direction, the implementation of this patch will be different. So, it should wait for the changes. Feel free to submit a new version of this change once we finish with the MC rework patches. Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL FOR v4.3] Various fixes
Em Tue, 28 Jul 2015 11:22:15 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: This pull request contains a pile of fixes/enhancements, mostly soc-camera related. Regards, Hans The following changes since commit 4dc102b2f53d63207fa12a6ad49c7b6448bc3301: [media] dvb_core: Replace memset with eth_zero_addr (2015-07-22 13:32:21 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.3e for you to fetch changes up to 9a400ca65ee917dc438cb9b553c11580269b4460: v4l2: export videobuf2 trace points (2015-07-28 11:15:04 +0200) Ezequiel Garcia (1): tw68: Move PCI vendor and device IDs to pci_ids.h Hans Verkuil (13): sh-veu: initialize timestamp_flags and copy timestamp info tw9910: don't use COLORSPACE_JPEG tw9910: init priv-scale and update standard ak881x: simplify standard checks mt9t112: JPEG - SRGB sh_mobile_ceu_camera: fix querycap sh_mobile_ceu_camera: set field to FIELD_NONE soc_camera: fix enum_input soc_camera: fix expbuf support soc_camera: compliance fixes soc_camera: pass on streamoff error soc_camera: always release queue for queue owner mt9v032: fix uninitialized variable warning Laurent Pinchart (1): v4l: subdev: Add pad config allocator and init As explained, we won't be adding any changes at the MC while we don't fix the MC mess. As I don't know what patches here are dependent of this change, I'm stopping handling this patch series at patch #15, with means that the following patches aren't merged: 0015-v4l-subdev-Add-pad-config-allocator-and-init.patch 0016-media-soc_camera-rcar_vin-Add-BT.709-24-bit-RGB888-i.patch 0017-media-soc_camera-pad-aware-driver-initialisation.patch 0018-media-rcar_vin-Use-correct-pad-number-in-try_fmt.patch 0019-media-soc_camera-soc_scale_crop-Use-correct-pad-numb.patch 0020-media-rcar_vin-fill-in-bus_info-field.patch 0021-media-rcar_vin-Reject-videobufs-that-are-too-small-f.patch 0022-mt9v032-fix-uninitialized-variable-warning.patch 0023-tw68-Move-PCI-vendor-and-device-IDs-to-pci_ids.h.patch 0024-v4l2-export-videobuf2-trace-points.patch Feel free to submit the remaining fix patches from this series that aren't related to media controller on a separate pull request. Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL FOR v4.3] Various fixes
Hi Mauro, On 08/11/15 15:08, Mauro Carvalho Chehab wrote: Em Tue, 28 Jul 2015 11:22:15 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: This pull request contains a pile of fixes/enhancements, mostly soc-camera related. Regards, Hans The following changes since commit 4dc102b2f53d63207fa12a6ad49c7b6448bc3301: [media] dvb_core: Replace memset with eth_zero_addr (2015-07-22 13:32:21 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.3e for you to fetch changes up to 9a400ca65ee917dc438cb9b553c11580269b4460: v4l2: export videobuf2 trace points (2015-07-28 11:15:04 +0200) Ezequiel Garcia (1): tw68: Move PCI vendor and device IDs to pci_ids.h Hans Verkuil (13): sh-veu: initialize timestamp_flags and copy timestamp info tw9910: don't use COLORSPACE_JPEG tw9910: init priv-scale and update standard ak881x: simplify standard checks mt9t112: JPEG - SRGB sh_mobile_ceu_camera: fix querycap sh_mobile_ceu_camera: set field to FIELD_NONE soc_camera: fix enum_input soc_camera: fix expbuf support soc_camera: compliance fixes soc_camera: pass on streamoff error soc_camera: always release queue for queue owner mt9v032: fix uninitialized variable warning Laurent Pinchart (1): v4l: subdev: Add pad config allocator and init As explained, we won't be adding any changes at the MC while we don't fix the MC mess. As I don't know what patches here are dependent of this change, I'm stopping handling this patch series at patch #15, with means that the following patches aren't merged: 0015-v4l-subdev-Add-pad-config-allocator-and-init.patch 0016-media-soc_camera-rcar_vin-Add-BT.709-24-bit-RGB888-i.patch 0017-media-soc_camera-pad-aware-driver-initialisation.patch 0018-media-rcar_vin-Use-correct-pad-number-in-try_fmt.patch 0019-media-soc_camera-soc_scale_crop-Use-correct-pad-numb.patch 0020-media-rcar_vin-fill-in-bus_info-field.patch 0021-media-rcar_vin-Reject-videobufs-that-are-too-small-f.patch William, can you take a look at this? Just let me know which patches are independent to patch 0015. 0022-mt9v032-fix-uninitialized-variable-warning.patch 0023-tw68-Move-PCI-vendor-and-device-IDs-to-pci_ids.h.patch 0024-v4l2-export-videobuf2-trace-points.patch Feel free to submit the remaining fix patches from this series that aren't related to media controller on a separate pull request. I actually posted a 4.3 pull request this afternoon that has patches 22 and an improved version of patch 24: https://patchwork.linuxtv.org/patch/30810/ Especially the tracepoint patch is quite important. Patch 23 will be part of some future pull request (no hurry with that one). Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 09/16] media: use media_graph_obj for link endpoints
Em Tue, 11 Aug 2015 14:25:18 +0200
Hans Verkuil hansv...@cisco.com escreveu:
Hi Mauro,
Thanks for posting the missing patches.
Thanks for reviewing this patch series!
On 08/11/15 14:09, Mauro Carvalho Chehab wrote:
As we'll need to create links between entities and interfaces,
we need to identify the link endpoints by the media_graph_obj.
Most of the changes here was done by this small script:
for i in `find drivers/media -type f` `find drivers/staging/media -type f`;
do
perl -ne
's,([\w]+)\-\(source|sink)\-\entity,gobj_to_pad($1-$2)-entity,; print
$_;' $i a mv a $i
done
Please note that, while we're now using graph_obj to reference
the link endpoints, we're still assuming that all endpoints are
pads. This is true for all existing links, so no problems
are expected so far.
Yet, as we introduce links between entities and interfaces,
we may need to change some existing code to work with links
that aren't pad to pad.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
snip
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 403019035424..f6e2136480f1 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -43,6 +43,17 @@ enum media_graph_type {
MEDIA_GRAPH_LINK,
};
+/**
+ * enum media_graph_link_dir - direction of a link
+ *
+ * @MEDIA_LINK_DIR_BIDIRECTIONAL Link is bidirectional
+ * @MEDIA_LINK_DIR_PAD_0_TO_1 Link is unidirectional,
+ * from port 0 (source) to port 1 (sink)
+ */
+enum media_graph_link_dir {
+ MEDIA_LINK_DIR_BIDIRECTIONAL,
+ MEDIA_LINK_DIR_PORT0_TO_PORT1,
+};
1) the comment and the actual enum are out-of-sync
Ah, yes. I was in doubt about using PAD or PORT here. I ended by using
port at the links, as the endpoints can either be an interface/entity
or a pad. So, I decided to use port.
2) why not just make a 'BIRECTIONAL' link flag instead of inventing
a new enum? Adding yet another field seems overkill to me. Have a
'BIDIRECTIONAL' flag seems perfectly OK to me (and useful for the
application as well).
Yeah, we can use flags, instead. I decided to use an enum here just
to make it clearer about the two possible options.
I was actually considering to rename media_link source/sink to
port0/port1, as using source/sink names on a bidirection link
doesn't make sense. I'm still in doubt about such rename, though,
as it would make harder to inspect the graph traversal routines.
Also, I want to force all places that create a link to choose
between either BIRECTIONAL or PORT0_TO_PORT1, as this makes easier
to review if the code is doing the right thing when inspecting it.
In summary, I would prefer to keep this internally as a separate
enum, at least for now. We can latter simplify it and use a flag
for that (or maybe two flags?).
/* Structs to represent the objects that belong to a media graph */
@@ -72,9 +83,9 @@ struct media_pipeline {
struct media_link {
struct list_head list;
- struct media_graph_obj graph_obj;
- struct media_pad *source; /* Source pad */
- struct media_pad *sink; /* Sink pad */
+ struct media_graph_obj graph_obj;
+ enum media_graph_link_dir dir;
+ struct media_graph_obj *source, *sink;
I'm not too keen about all the gobj_to_foo(obj) macros that this requires. It
is rather ugly code.
What about this:
union {
struct media_graph_obj *source;
struct media_pad *source_pad;
struct media_interface *source_intf;
};
union {
struct media_graph_obj *sink;
struct media_pad *sink_pad;
struct media_entity *sink_ent;
};
Now the code can just use -source_pad etc.
good idea. Will do that on a version 3. I think that, in this case, the
best is to write a note that the first element at pad/entity/interface
should be the graph_obj.
I would actually call port0_intf and port1_ent on the above structs,
as it makes no sense to call sink/source for interface-entity links.
struct media_link *reverse; /* Link in the reverse direction */
unsigned long flags;/* Link flags (MEDIA_LNK_FL_*) */
};
@@ -115,6 +126,11 @@ struct media_entity {
u32 group_id; /* Entity group ID */
u16 num_pads; /* Number of sink and source pads */
+
+ /*
+* Both num_links and num_backlinks are used only to report
+* the number of links via MEDIA_IOC_ENUM_ENTITIES at media_device.c
+*/
u16 num_links; /* Number of existing links, both
* enabled and disabled */
u16 num_backlinks; /* Number of backlinks */
@@ -171,6 +187,12 @@ struct media_entity_graph {
Re: VIMC: API proposal, configuring the topology through user space
On 08/11/15 11:36, Mauro Carvalho Chehab wrote: Em Tue, 11 Aug 2015 11:28:25 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: Hi Helen, On 08/10/15 19:21, Helen Fornazier wrote: Hi, thanks for your reviews On Mon, Aug 10, 2015 at 10:11 AM, Hans Verkuil hverk...@xs4all.nl wrote: Hi Helen! On 08/08/2015 03:55 AM, Helen Fornazier wrote: Hi! I've made a first sketch about the API of the vimc driver (virtual media controller) to configure the topology through user space. As first suggested by Laurent Pinchart, it is based on configfs. I would like to know you opinion about it, if you have any suggestion to improve it, otherwise I'll start its implementation as soon as possible. This API may change with the MC changes and I may see other possible configurations as I implementing it but here is the first idea of how the API will look like. vimc project link: https://github.com/helen-fornazier/opw-staging/ For more information: http://kernelnewbies.org/LaurentPinchart /*** The API: / In short, a topology like this one: http://goo.gl/Y7eUfu Would look like this filesystem tree: https://goo.gl/tCZPTg Txt version of the filesystem tree: https://goo.gl/42KX8Y * The /configfs/vimc subsystem I haven't checked the latest vimc driver, but doesn't it support multiple instances, just like vivid? I would certainly recommend that. Yes, it does support And if there are multiple instances, then each instance gets its own entry in configfs: /configs/vimc0, /configs/vimc1, etc. You are right, I'll add those The vimc driver registers a subsystem in the configfs with the following contents: ls /configfs/vimc build_topology status The build_topology attribute file will be used to tell the driver the configuration is done and it can build the topology internally echo -n anything here /configfs/vimc/build_topology Reading from the status attribute can have 3 different classes of outputs 1) deployed: the current configured tree is built 2) undeployed: no errors, the user has modified the configfs tree thus the topology was undeployed 3) error error_message: the topology configuration is wrong I don't see the status file in the filesystem tree links above. Sorry, I forgot to add I actually wonder if we can't use build_topology for that: reading it will just return the status. Yes we can, I was just wondering what should be the name of the file, as getting the status from reading the build_topology file doesn't seem much intuitive I'm not opposed to a status file, but it would be good to know what Laurent thinks. What happens if it is deployed and you want to 'undeploy' it? Instead of writing anything to build_topology it might be useful to write a real command to it. E.g. 'deploy', 'destroy'. What happens when you make changes while a topology is deployed? Should such changes be rejected until the usecount of the driver goes to 0, or will it only be rejected when you try to deploy the new configuration? I was thinking that if the user try changing the topology, or it will fail (because the device instance is in use) or it will undeploy the old topology (if the device is not in use). Then a 'destroy' command won't be useful, the user can just unload the driver when it won't be used anymore, Well, we're planning to add support for dynamic addition/removal of entities, interfaces, pads and links. So, instead of undeploy the old topology, one could just do: rm -rf part of the tree Dynamic entities and interfaces yes, but dynamic pads? The entity defines the number of pads it has, which is related to the hardware or IP properties of the entity. I don't see how that can be dynamic. I certainly wouldn't bother with that in vimc. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10.1] media: Add registration helpers for V4L2 flash sub-devices
On 07/28/2015 12:00 PM, Hans Verkuil wrote:
On 06/19/2015 09:31 AM, Jacek Anaszewski wrote:
This patch adds helper functions for registering/unregistering
LED Flash class devices as V4L2 sub-devices. The functions should
be called from the LED subsystem device driver. In case the
support for V4L2 Flash sub-devices is disabled in the kernel
config the functions' empty versions will be used.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Hans Verkuil hans.verk...@cisco.com
---
- fixed possible NULL fled_cdev pointer dereference
in the v4l2_flash_init function
drivers/media/v4l2-core/Kconfig| 11 +
drivers/media/v4l2-core/Makefile |2 +
drivers/media/v4l2-core/v4l2-flash-led-class.c | 710
include/media/v4l2-flash-led-class.h | 148 +
4 files changed, 871 insertions(+)
create mode 100644 drivers/media/v4l2-core/v4l2-flash-led-class.c
create mode 100644 include/media/v4l2-flash-led-class.h
snip
diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c
b/drivers/media/v4l2-core/v4l2-flash-led-class.c
new file mode 100644
index 000..5bdfb8d
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c
snip
+static const struct v4l2_subdev_core_ops v4l2_flash_core_ops = {
+ .queryctrl = v4l2_subdev_queryctrl,
+ .querymenu = v4l2_subdev_querymenu,
Why are these here? This should not be necessary. As long as the sd.ctrl_handler
pointer is set, this is handled automatically.
I removed these two lines and indeed driver works well without it.
+};
+
+static const struct v4l2_subdev_ops v4l2_flash_subdev_ops = {
+ .core = v4l2_flash_core_ops,
+};
+
And if v4l2_flash_core_ops goes away, then this can go away as well.
What should I pass as the second argument to v4l2_subdev_init then?
It seems that ops can't be NULL:
void v4l2_subdev_init(struct v4l2_subdev *sd, const struct
v4l2_subdev_ops *ops)
{
INIT_LIST_HEAD(sd-list);
BUG_ON(!ops); ---
sd-ops = ops;
sd-v4l2_dev = NULL;
sd-flags = 0;
sd-name[0] = '\0';
sd-grp_id = 0;
sd-dev_priv = NULL;
sd-host_priv = NULL;
#if defined(CONFIG_MEDIA_CONTROLLER)
sd-entity.name = sd-name;
sd-entity.type = MEDIA_ENT_T_V4L2_SUBDEV;
#endif
}
I know this driver has been merged, but I just noticed this while looking at
something else.
Regards,
Hans
--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line unsubscribe 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 08/16] media: convert links from array to list
On 08/07/15 16:20, Mauro Carvalho Chehab wrote:
Using memory realloc to increase the size of an array
is complex and makes harder to remove links. Also, by
embedding the link inside an array at the entity makes harder
to change the code to add interfaces, as interfaces will
also need to use links.
So, convert the links from arrays to lists.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 9fb3f8958265..a95ca981aabb 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
snip
@@ -452,27 +445,21 @@ EXPORT_SYMBOL_GPL(media_entity_put);
static struct media_link *media_entity_add_link(struct media_entity *entity)
{
- if (entity-num_links = entity-max_links) {
- struct media_link *links = entity-links;
- unsigned int max_links = entity-max_links + 2;
- unsigned int i;
+ struct media_link *link;
- links = krealloc(links, max_links * sizeof(*links), GFP_KERNEL);
- if (links == NULL)
- return NULL;
+ link = kzalloc(sizeof(*link), GFP_KERNEL);
+ if (link == NULL)
+ return NULL;
- for (i = 0; i entity-num_links; i++)
- links[i].reverse-reverse = links[i];
-
- entity-max_links = max_links;
- entity-links = links;
- }
+ link-reverse-reverse = link;
Huh? link points to a zeroed struct, so link-reverse will be NULL.
This can't work.
Are you sure this line should be here? The original code doesn't set it
either for the new link, it just updates the reverse links for the
realloced links.
+ INIT_LIST_HEAD(link-list);
+ list_add(entity-links, link-list);
/* Initialize graph object embedded at the new link */
graph_obj_init(entity-parent, MEDIA_GRAPH_LINK,
- entity-links[entity-num_links].graph_obj);
+ link-graph_obj);
- return entity-links[entity-num_links++];
+ return link;
}
int
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 13/16] media: make the internal function to create links more generic
On 08/07/15 16:20, Mauro Carvalho Chehab wrote: In preparation to add a public function to add links, let's make the internal function that creates link more generic. Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c index 96d48aec8381..c68dc421b022 100644 --- a/drivers/media/media-entity.c +++ b/drivers/media/media-entity.c @@ -461,7 +461,12 @@ EXPORT_SYMBOL_GPL(media_entity_put); * Links management */ -static struct media_link *media_entity_add_link(struct media_entity *entity) +static struct media_link *__media_create_link(struct media_device *mdev, + enum media_graph_link_dir dir, Am I blind? I can't find the media_graph_link_dir enum definition anywhere in this patch series... Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: VIMC: API proposal, configuring the topology through user space
Em Tue, 11 Aug 2015 12:34:46 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: On 08/11/15 11:36, Mauro Carvalho Chehab wrote: Em Tue, 11 Aug 2015 11:28:25 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: Hi Helen, On 08/10/15 19:21, Helen Fornazier wrote: Hi, thanks for your reviews On Mon, Aug 10, 2015 at 10:11 AM, Hans Verkuil hverk...@xs4all.nl wrote: Hi Helen! On 08/08/2015 03:55 AM, Helen Fornazier wrote: Hi! I've made a first sketch about the API of the vimc driver (virtual media controller) to configure the topology through user space. As first suggested by Laurent Pinchart, it is based on configfs. I would like to know you opinion about it, if you have any suggestion to improve it, otherwise I'll start its implementation as soon as possible. This API may change with the MC changes and I may see other possible configurations as I implementing it but here is the first idea of how the API will look like. vimc project link: https://github.com/helen-fornazier/opw-staging/ For more information: http://kernelnewbies.org/LaurentPinchart /*** The API: / In short, a topology like this one: http://goo.gl/Y7eUfu Would look like this filesystem tree: https://goo.gl/tCZPTg Txt version of the filesystem tree: https://goo.gl/42KX8Y * The /configfs/vimc subsystem I haven't checked the latest vimc driver, but doesn't it support multiple instances, just like vivid? I would certainly recommend that. Yes, it does support And if there are multiple instances, then each instance gets its own entry in configfs: /configs/vimc0, /configs/vimc1, etc. You are right, I'll add those The vimc driver registers a subsystem in the configfs with the following contents: ls /configfs/vimc build_topology status The build_topology attribute file will be used to tell the driver the configuration is done and it can build the topology internally echo -n anything here /configfs/vimc/build_topology Reading from the status attribute can have 3 different classes of outputs 1) deployed: the current configured tree is built 2) undeployed: no errors, the user has modified the configfs tree thus the topology was undeployed 3) error error_message: the topology configuration is wrong I don't see the status file in the filesystem tree links above. Sorry, I forgot to add I actually wonder if we can't use build_topology for that: reading it will just return the status. Yes we can, I was just wondering what should be the name of the file, as getting the status from reading the build_topology file doesn't seem much intuitive I'm not opposed to a status file, but it would be good to know what Laurent thinks. What happens if it is deployed and you want to 'undeploy' it? Instead of writing anything to build_topology it might be useful to write a real command to it. E.g. 'deploy', 'destroy'. What happens when you make changes while a topology is deployed? Should such changes be rejected until the usecount of the driver goes to 0, or will it only be rejected when you try to deploy the new configuration? I was thinking that if the user try changing the topology, or it will fail (because the device instance is in use) or it will undeploy the old topology (if the device is not in use). Then a 'destroy' command won't be useful, the user can just unload the driver when it won't be used anymore, Well, we're planning to add support for dynamic addition/removal of entities, interfaces, pads and links. So, instead of undeploy the old topology, one could just do: rm -rf part of the tree Dynamic entities and interfaces yes, but dynamic pads? The entity defines the number of pads it has, which is related to the hardware or IP properties of the entity. I don't see how that can be dynamic. It can be dynamic. DVB has usages for that, on two different situations: 1) DVB demux There are some hardware that has an IP block that provides more than one demuxes, but they share a number of TS filters. The amount of TS filters per demux can be dynamically changed. For example, such hardware may have 2 demuxes and 32 shared TS filters. At DVB driver init, let's assume that each demux will have 16 filters. So: demux 0 - 16 TS filters = 16 pads demux 1 - 16 TS buffers = 16 pads On a given time, let's assume that the first demux has all 16 filters already in usage, but the second demux is using only 4 filters. It is possible to dynamically change the hardware to move, let's say, 4 buffers from the second demux to the first one. After such change, we'll have: demux 0 - 20 TS filters = 20 pads demux 1 - 12 TS filters = 12 pads Ok, one might think on initializing both with 32 pads, and create 32 ringbuffers at the Kernelspace for each demux, but if the number of TS filters is
Re: [PATCH RFC v2 16/16] media: add functions to allow creating interfaces
On 08/07/15 16:20, Mauro Carvalho Chehab wrote:
Interfaces are different than entities: they represent a
Kernel-userspace interaction, while entities represent a
piece of hardware/firmware/software that executes a function.
Let's distinguish them by creating a separate structure to
store the interfaces.
Latter patches should change the existing drivers and logic
to split the current interface embedded inside the entity
structure (device nodes) into a separate object of the graph.
So, to be clear, the plan is to replace the embedded media_entity
struct in struct video_device by a struct media_intf_devnode pointer
that is allocated with media_devnode_create()?
Can we keep struct media_intf_devnode embedded in struct video_device?
Or is that impossible since all media_graph_obj structs are expected
to be allocated?
I do have a preference for keeping structs embedded, unless there is a
good reason not to. I just want to make sure there is a good reason.
Regards,
Hans
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 5c0d197fa072..bc806958815e 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -741,3 +741,59 @@ struct media_pad *media_entity_remote_pad(struct
media_pad *pad)
}
EXPORT_SYMBOL_GPL(media_entity_remote_pad);
+
+
+/* Functions related to the media interface via device nodes */
+
+struct media_intf_devnode *media_devnode_create(struct media_device *mdev,
+ u32 major, u32 minor,
+ gfp_t gfp_flags)
+{
+ struct media_intf_devnode *devnode;
+
+ devnode = kzalloc(sizeof(*devnode), gfp_flags);
+ if (!devnode)
+ return NULL;
+
+ devnode-major = major;
+ devnode-minor = minor;
+ INIT_LIST_HEAD(devnode-intf.entity_links);
+
+ graph_obj_init(mdev, MEDIA_GRAPH_INTF_DEVNODE,
+devnode-intf.graph_obj);
+
+ return devnode;
+}
+EXPORT_SYMBOL_GPL(media_devnode_create);
+
+void media_devnode_remove(struct media_intf_devnode *devnode)
+{
+ graph_obj_remove(devnode-intf.graph_obj);
+ kfree(devnode);
+}
+EXPORT_SYMBOL_GPL(media_devnode_remove);
+
+struct media_link *media_create_intf_link(struct media_entity *entity,
+ struct media_graph_obj *gobj,
+ u32 flags)
+{
+ struct media_link *link;
+
+ /* Only accept links between entity==interface */
+ if (gobj-type != MEDIA_GRAPH_INTF_DEVNODE)
+ return NULL;
+
+ link = __media_create_link(entity-parent,
+MEDIA_LINK_DIR_BIDIRECTIONAL,
+entity-graph_obj,
+gobj,
+flags, entity-intf_links);
+ if (link == NULL)
+ return NULL;
+
+ /* Create the link at the interface */
+ list_add(gobj_to_interface(gobj)-entity_links, link-list);
+
+ return link;
+}
+EXPORT_SYMBOL_GPL(media_create_intf_link);
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 5f080ab88399..1faeb74f8489 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -36,11 +36,14 @@
* @MEDIA_GRAPH_ENTITY: Identify a media entity
* @MEDIA_GRAPH_PAD: Identify a media PAD
* @MEDIA_GRAPH_LINK:Identify a media link
+ * @MEDIA_GRAPH_INTF_DEVNODE:Identify a media Kernel API interface
via
+ * a device node
*/
enum media_graph_type {
MEDIA_GRAPH_ENTITY,
MEDIA_GRAPH_PAD,
MEDIA_GRAPH_LINK,
+ MEDIA_GRAPH_INTF_DEVNODE,
};
/**
@@ -136,7 +139,9 @@ struct media_entity {
u16 num_backlinks; /* Number of backlinks */
struct media_pad *pads; /* Pads array (num_pads elements) */
- struct list_head links; /* Links list */
+ struct list_head links; /* PAD links list */
+
+ struct list_head intf_links;/* Interface links list */
const struct media_entity_operations *ops; /* Entity operations */
@@ -161,6 +166,30 @@ struct media_entity {
} info;
};
+/**
+ * struct media_intf_devnode - Define a Kernel API interface
+ *
+ * @graph_obj: embedded graph object
+ * @ent_links: List of links pointing to graph entities
+ */
+struct media_interface {
+ struct media_graph_obj graph_obj;
+ struct list_headentity_links;
+};
+
+/**
+ * struct media_intf_devnode - Define a Kernel API interface via a device
node
+ *
+ * @intf:embedded interface object
+ * @major: Major number of a device node
+ * @minor: Minor number of a device node
+ */
+struct media_intf_devnode {
+
Re: [PATCH RFC v2 13/16] media: make the internal function to create links more generic
Em Tue, 11 Aug 2015 12:57:04 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: On 08/07/15 16:20, Mauro Carvalho Chehab wrote: In preparation to add a public function to add links, let's make the internal function that creates link more generic. Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c index 96d48aec8381..c68dc421b022 100644 --- a/drivers/media/media-entity.c +++ b/drivers/media/media-entity.c @@ -461,7 +461,12 @@ EXPORT_SYMBOL_GPL(media_entity_put); * Links management */ -static struct media_link *media_entity_add_link(struct media_entity *entity) +static struct media_link *__media_create_link(struct media_device *mdev, + enum media_graph_link_dir dir, Am I blind? I can't find the media_graph_link_dir enum definition anywhere in this patch series... There were a few patches that vger didn't seem to get. This one was added by patch 9: 0009-media-use-media_graph_obj-for-link-endpoints.patch I'll try to resend the missing patches. Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] DT support for omap4-iss
* Michael Allwright michael.allwri...@upb.de [150810 08:19]:
+
+/*
+We need a better solution for this
+*/
+#include ../arch/arm/mach-omap2/omap-pm.h
Please let's not do things like this, I end up having to deal with
all these eventually :(
+static void iss_set_constraints(struct iss_device *iss, bool enable)
+{
+if (!iss)
+return;
+
+/* FIXME: Look for something more precise as a good throughtput limit */
+omap_pm_set_min_bus_tput(iss-dev, OCP_INITIATOR_AGENT,
+ enable ? 80 : -1);
+}
+
+static struct iss_platform_data iss_dummy_pdata = {
+.set_constraints = iss_set_constraints,
+};
If this is one time setting, you could do it based on the
compatible string using arch/arm/mach-omap2/pdata-quirks.c.
If you need to toggle it, you could populate a function pointer
in pdata-quirks.c. Those are easy to fix once there is some Linux
generic API available :)
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe 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 08/16] media: convert links from array to list
Em Tue, 11 Aug 2015 12:49:40 +0200
Hans Verkuil hverk...@xs4all.nl escreveu:
On 08/07/15 16:20, Mauro Carvalho Chehab wrote:
Using memory realloc to increase the size of an array
is complex and makes harder to remove links. Also, by
embedding the link inside an array at the entity makes harder
to change the code to add interfaces, as interfaces will
also need to use links.
So, convert the links from arrays to lists.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 9fb3f8958265..a95ca981aabb 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
snip
@@ -452,27 +445,21 @@ EXPORT_SYMBOL_GPL(media_entity_put);
static struct media_link *media_entity_add_link(struct media_entity
*entity)
{
- if (entity-num_links = entity-max_links) {
- struct media_link *links = entity-links;
- unsigned int max_links = entity-max_links + 2;
- unsigned int i;
+ struct media_link *link;
- links = krealloc(links, max_links * sizeof(*links), GFP_KERNEL);
- if (links == NULL)
- return NULL;
+ link = kzalloc(sizeof(*link), GFP_KERNEL);
+ if (link == NULL)
+ return NULL;
- for (i = 0; i entity-num_links; i++)
- links[i].reverse-reverse = links[i];
-
- entity-max_links = max_links;
- entity-links = links;
- }
+ link-reverse-reverse = link;
Huh? link points to a zeroed struct, so link-reverse will be NULL.
This can't work.
Are you sure this line should be here? The original code doesn't set it
either for the new link, it just updates the reverse links for the
realloced links.
You're right. I think I can just remove that line.
I had to confess that I didn't understand well that the reverse links
stuff. I mean, IMHO, the entire concept of storing a second copy of
the link, calling it as reverse on some places and as backlink
on others is messy, and I guess we should get rid of duplicating the
number of links for no good reason.
It is easy to just add the same link to a list at the other entity,
and keep the link just once in the memory.
+ INIT_LIST_HEAD(link-list);
+ list_add(entity-links, link-list);
/* Initialize graph object embedded at the new link */
graph_obj_init(entity-parent, MEDIA_GRAPH_LINK,
- entity-links[entity-num_links].graph_obj);
+ link-graph_obj);
- return entity-links[entity-num_links++];
+ return link;
}
int
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FOR v4.3] Various fixes
A small vivid improvement and two patches that fix compiler/linker warnings. Regards, Hans The following changes since commit 267897a4708fd7a0592333f33a4a7c393c999ab7: [media] tda10071: implement DVBv5 statistics (2015-08-11 07:34:58 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.3f for you to fetch changes up to c947b28da693d6c05c87a3aefc7bd6a261bf07a5: mt9v032: fix uninitialized variable warning (2015-08-11 13:47:47 +0200) Hans Verkuil (1): mt9v032: fix uninitialized variable warning Philipp Zabel (1): v4l2: move tracepoint generation into separate file Prashant Laddha (1): vivid: support cvt, gtf timings for video out drivers/media/i2c/mt9v032.c | 2 +- drivers/media/platform/vivid/vivid-vid-out.c | 15 +-- drivers/media/v4l2-core/Makefile | 3 +++ drivers/media/v4l2-core/v4l2-ioctl.c | 1 - drivers/media/v4l2-core/v4l2-trace.c | 11 +++ 5 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 drivers/media/v4l2-core/v4l2-trace.c -- To unsubscribe from this list: send the line unsubscribe 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] i2c/adv7511: Fix license, set to GPL v2
Header claims GPL v2, so make the MODULE_LICENSE reflect that properly. Signed-off-by: Mike Looijmans mike.looijm...@topic.nl --- drivers/media/i2c/adv7511.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c index 95bcd40..497ee00 100644 --- a/drivers/media/i2c/adv7511.c +++ b/drivers/media/i2c/adv7511.c @@ -40,7 +40,7 @@ MODULE_PARM_DESC(debug, debug level (0-2)); MODULE_DESCRIPTION(Analog Devices ADV7511 HDMI Transmitter Device Driver); MODULE_AUTHOR(Hans Verkuil); -MODULE_LICENSE(GPL); +MODULE_LICENSE(GPL v2); #define MASK_ADV7511_EDID_RDY_INT 0x04 #define MASK_ADV7511_MSEN_INT 0x40 -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 16/16] media: add functions to allow creating interfaces
Em Tue, 11 Aug 2015 13:14:41 +0200
Hans Verkuil hverk...@xs4all.nl escreveu:
On 08/07/15 16:20, Mauro Carvalho Chehab wrote:
Interfaces are different than entities: they represent a
Kernel-userspace interaction, while entities represent a
piece of hardware/firmware/software that executes a function.
Let's distinguish them by creating a separate structure to
store the interfaces.
Latter patches should change the existing drivers and logic
to split the current interface embedded inside the entity
structure (device nodes) into a separate object of the graph.
So, to be clear, the plan is to replace the embedded media_entity
struct in struct video_device by a struct media_intf_devnode pointer
that is allocated with media_devnode_create()?
Yes.
Can we keep struct media_intf_devnode embedded in struct video_device?
Or is that impossible since all media_graph_obj structs are expected
to be allocated?
I do have a preference for keeping structs embedded, unless there is a
good reason not to. I just want to make sure there is a good reason.
My plan is to always allocate the structs, as we'll very likely need to
have kref for all those structs, in order to be sure that they'll be
freed only after having all their usages stopped.
The thing is that it is really complex when we have lots of objects
linked to know when an object is not used anymore.
Let's assume, for example, this simple graph:
entity 0
pad 0
entity 1
pad 0
pad 1
entity 2
pad 0
link 0
entity 0:pad 0 - entity 1: pad 0
link 1
entity 1:pad 1 - entity 2: pad 0
If we need to keep back references for the links, we'll end by
having both links 0 and 1 used twice.
We can't free neither the entities or the pads while link 0
is not freed, but it is used twice: as a link and as a backlink.
Assuming that we want to free all those objects from the memory,
It can be really tricky to find the right order to free them, as
we need first to go into the 3 entities and free the links there,
then we can free the pads and entities.
If, on the other hand, we use kref, we can simply do a for on
all objects and call kref_put(). The object memory will be
freed in an order that will be safe, e. g. when all kref counts
are zero for that specific object.
I actually postponed the usage of kref because it is a little hard to
de-embed the entities on subdevices, but IMHO, using kref is the best
and safest way to be sure that the Kernel won't be accessing a freed
memory as the graph dynamically changes.
Regards,
Mauro
Regards,
Hans
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 5c0d197fa072..bc806958815e 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -741,3 +741,59 @@ struct media_pad *media_entity_remote_pad(struct
media_pad *pad)
}
EXPORT_SYMBOL_GPL(media_entity_remote_pad);
+
+
+/* Functions related to the media interface via device nodes */
+
+struct media_intf_devnode *media_devnode_create(struct media_device *mdev,
+ u32 major, u32 minor,
+ gfp_t gfp_flags)
+{
+ struct media_intf_devnode *devnode;
+
+ devnode = kzalloc(sizeof(*devnode), gfp_flags);
+ if (!devnode)
+ return NULL;
+
+ devnode-major = major;
+ devnode-minor = minor;
+ INIT_LIST_HEAD(devnode-intf.entity_links);
+
+ graph_obj_init(mdev, MEDIA_GRAPH_INTF_DEVNODE,
+ devnode-intf.graph_obj);
+
+ return devnode;
+}
+EXPORT_SYMBOL_GPL(media_devnode_create);
+
+void media_devnode_remove(struct media_intf_devnode *devnode)
+{
+ graph_obj_remove(devnode-intf.graph_obj);
+ kfree(devnode);
+}
+EXPORT_SYMBOL_GPL(media_devnode_remove);
+
+struct media_link *media_create_intf_link(struct media_entity *entity,
+ struct media_graph_obj *gobj,
+ u32 flags)
+{
+ struct media_link *link;
+
+ /* Only accept links between entity==interface */
+ if (gobj-type != MEDIA_GRAPH_INTF_DEVNODE)
+ return NULL;
+
+ link = __media_create_link(entity-parent,
+ MEDIA_LINK_DIR_BIDIRECTIONAL,
+ entity-graph_obj,
+ gobj,
+ flags, entity-intf_links);
+ if (link == NULL)
+ return NULL;
+
+ /* Create the link at the interface */
+ list_add(gobj_to_interface(gobj)-entity_links, link-list);
+
+ return link;
+}
+EXPORT_SYMBOL_GPL(media_create_intf_link);
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index
Re: [PATCH RFC v2 09/16] media: use media_graph_obj for link endpoints
Hi Mauro,
Thanks for posting the missing patches.
On 08/11/15 14:09, Mauro Carvalho Chehab wrote:
As we'll need to create links between entities and interfaces,
we need to identify the link endpoints by the media_graph_obj.
Most of the changes here was done by this small script:
for i in `find drivers/media -type f` `find drivers/staging/media -type f`; do
perl -ne
's,([\w]+)\-\(source|sink)\-\entity,gobj_to_pad($1-$2)-entity,; print
$_;' $i a mv a $i
done
Please note that, while we're now using graph_obj to reference
the link endpoints, we're still assuming that all endpoints are
pads. This is true for all existing links, so no problems
are expected so far.
Yet, as we introduce links between entities and interfaces,
we may need to change some existing code to work with links
that aren't pad to pad.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
snip
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 403019035424..f6e2136480f1 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -43,6 +43,17 @@ enum media_graph_type {
MEDIA_GRAPH_LINK,
};
+/**
+ * enum media_graph_link_dir - direction of a link
+ *
+ * @MEDIA_LINK_DIR_BIDIRECTIONAL Link is bidirectional
+ * @MEDIA_LINK_DIR_PAD_0_TO_1Link is unidirectional,
+ * from port 0 (source) to port 1 (sink)
+ */
+enum media_graph_link_dir {
+ MEDIA_LINK_DIR_BIDIRECTIONAL,
+ MEDIA_LINK_DIR_PORT0_TO_PORT1,
+};
1) the comment and the actual enum are out-of-sync
2) why not just make a 'BIRECTIONAL' link flag instead of inventing
a new enum? Adding yet another field seems overkill to me. Have a
'BIDIRECTIONAL' flag seems perfectly OK to me (and useful for the
application as well).
/* Structs to represent the objects that belong to a media graph */
@@ -72,9 +83,9 @@ struct media_pipeline {
struct media_link {
struct list_head list;
- struct media_graph_obj graph_obj;
- struct media_pad *source; /* Source pad */
- struct media_pad *sink; /* Sink pad */
+ struct media_graph_obj graph_obj;
+ enum media_graph_link_dir dir;
+ struct media_graph_obj *source, *sink;
I'm not too keen about all the gobj_to_foo(obj) macros that this requires. It
is rather ugly code.
What about this:
union {
struct media_graph_obj *source;
struct media_pad *source_pad;
struct media_interface *source_intf;
};
union {
struct media_graph_obj *sink;
struct media_pad *sink_pad;
struct media_entity *sink_ent;
};
Now the code can just use -source_pad etc.
struct media_link *reverse; /* Link in the reverse direction */
unsigned long flags;/* Link flags (MEDIA_LNK_FL_*) */
};
@@ -115,6 +126,11 @@ struct media_entity {
u32 group_id; /* Entity group ID */
u16 num_pads; /* Number of sink and source pads */
+
+ /*
+ * Both num_links and num_backlinks are used only to report
+ * the number of links via MEDIA_IOC_ENUM_ENTITIES at media_device.c
+ */
u16 num_links; /* Number of existing links, both
* enabled and disabled */
u16 num_backlinks; /* Number of backlinks */
@@ -171,6 +187,12 @@ struct media_entity_graph {
#define gobj_to_entity(gobj) \
container_of(gobj, struct media_entity, graph_obj)
+#define gobj_to_link(gobj) \
+ container_of(gobj, struct media_link, graph_obj)
+
+#define gobj_to_pad(gobj) \
+ container_of(gobj, struct media_pad, graph_obj)
+
I saw a lot of type checks (if (link-sink.type != MEDIA_GRAPH_PAD))
that I think would look cleaner if there was a simple static inline
helper:
static inline bool is_pad(const struct media_graph_obj *obj)
{
return obj-type == MEDIA_GRAPH_PAD;
}
media_is_pad() will work as well, but I think brevity is more useful
in this case. Personal opinion, though.
void graph_obj_init(struct media_device *mdev,
enum media_graph_type type,
struct media_graph_obj *gobj);
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] i2c/adv7511: Fix license, set to GPL v2
Hi Mike, Please split up this patch: these are two different drivers with different authors and different subsystems. The media/i2c/adv7511.c patch I can handle, but the patch for the drm driver should go to the dri-devel mailinglist. I can't take that change. Easiest is just to post two patches, one for each driver. Regards, Hans On 07/28/15 12:57, Mike Looijmans wrote: Header claims GPL v2, so make the MODULE_LICENSE reflect that properly. Signed-off-by: Mike Looijmans mike.looijm...@topic.nl --- drivers/gpu/drm/i2c/adv7511_core.c | 2 +- drivers/media/i2c/adv7511.c| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i2c/adv7511_core.c b/drivers/gpu/drm/i2c/adv7511_core.c index 2564b5d..12e8134 100644 --- a/drivers/gpu/drm/i2c/adv7511_core.c +++ b/drivers/gpu/drm/i2c/adv7511_core.c @@ -956,4 +956,4 @@ module_exit(adv7511_exit); MODULE_AUTHOR(Lars-Peter Clausen l...@metafoo.de); MODULE_DESCRIPTION(ADV7511 HDMI transmitter driver); -MODULE_LICENSE(GPL); +MODULE_LICENSE(GPL v2); diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c index 02d76c6..1a4275d 100644 --- a/drivers/media/i2c/adv7511.c +++ b/drivers/media/i2c/adv7511.c @@ -41,7 +41,7 @@ MODULE_PARM_DESC(debug, debug level (0-2)); MODULE_DESCRIPTION(Analog Devices ADV7511 HDMI Transmitter Device Driver); MODULE_AUTHOR(Hans Verkuil); -MODULE_LICENSE(GPL); +MODULE_LICENSE(GPL v2); #define MASK_ADV7511_EDID_RDY_INT 0x04 #define MASK_ADV7511_MSEN_INT 0x40 -- To unsubscribe from this list: send the line unsubscribe 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 10/16] media: rename the function that create pad links
Now that a link can be either between two different graph
objects, we'll need to add more functions to create links.
So, rename the existing one that create links only between
two pads as media_create_pad_link().
No functional changes.
This patch was created via this shell script:
for i in $(find drivers/media -name '*.[ch]' -type f) $(find
drivers/staging/media -name '*.[ch]' -type f) $(find include/ -name '*.h' -type
f) ; do sed s,media_entity_create_link,media_create_pad_link,g $i a mv a
$i; done
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 2fdcbb5f000a..65f59f2124b4 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -412,16 +412,16 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
}
if (tuner fe)
- media_entity_create_link(tuner, 0, fe, 0, 0);
+ media_create_pad_link(tuner, 0, fe, 0, 0);
if (fe demux)
- media_entity_create_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
+ media_create_pad_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
if (demux dvr)
- media_entity_create_link(demux, 1, dvr, 0,
MEDIA_LNK_FL_ENABLED);
+ media_create_pad_link(demux, 1, dvr, 0, MEDIA_LNK_FL_ENABLED);
if (demux ca)
- media_entity_create_link(demux, 1, ca, 0, MEDIA_LNK_FL_ENABLED);
+ media_create_pad_link(demux, 1, ca, 0, MEDIA_LNK_FL_ENABLED);
}
EXPORT_SYMBOL_GPL(dvb_create_media_graph);
#endif
diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
index 6d167428727d..c81bfbfea32f 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
@@ -1482,11 +1482,11 @@ static int s5c73m3_oif_registered(struct v4l2_subdev
*sd)
return ret;
}
- ret = media_entity_create_link(state-sensor_sd.entity,
+ ret = media_create_pad_link(state-sensor_sd.entity,
S5C73M3_ISP_PAD, state-oif_sd.entity, OIF_ISP_PAD,
MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
- ret = media_entity_create_link(state-sensor_sd.entity,
+ ret = media_create_pad_link(state-sensor_sd.entity,
S5C73M3_JPEG_PAD, state-oif_sd.entity, OIF_JPEG_PAD,
MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
index 30a9ca62e034..d3bff30bcb6f 100644
--- a/drivers/media/i2c/s5k5baf.c
+++ b/drivers/media/i2c/s5k5baf.c
@@ -1756,7 +1756,7 @@ static int s5k5baf_registered(struct v4l2_subdev *sd)
v4l2_err(sd, failed to register subdev %s\n,
state-cis_sd.name);
else
- ret = media_entity_create_link(state-cis_sd.entity, PAD_CIS,
+ ret = media_create_pad_link(state-cis_sd.entity, PAD_CIS,
state-sd.entity, PAD_CIS,
MEDIA_LNK_FL_IMMUTABLE |
MEDIA_LNK_FL_ENABLED);
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
b/drivers/media/i2c/smiapp/smiapp-core.c
index 308613ea0aed..5aa49eb393a9 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2495,7 +2495,7 @@ static int smiapp_register_subdevs(struct smiapp_sensor
*sensor)
return rval;
}
- rval = media_entity_create_link(this-sd.entity,
+ rval = media_create_pad_link(this-sd.entity,
this-source_pad,
last-sd.entity,
last-sink_pad,
@@ -2503,7 +2503,7 @@ static int smiapp_register_subdevs(struct smiapp_sensor
*sensor)
MEDIA_LNK_FL_IMMUTABLE);
if (rval) {
dev_err(client-dev,
- media_entity_create_link failed\n);
+ media_create_pad_link failed\n);
return rval;
}
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index c8bbf122f4a6..bdeb6e804c7e 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -481,7 +481,7 @@ static struct media_link *media_entity_add_link(struct
media_entity *entity)
}
int
-media_entity_create_link(struct media_entity *source, u16 source_pad,
+media_create_pad_link(struct media_entity *source, u16 source_pad,
struct media_entity *sink, u16 sink_pad, u32 flags)
{
struct media_link *link;
@@ -519,7 +519,7 @@ media_entity_create_link(struct media_entity *source, u16
[PATCH RFC v2 09/16] media: use media_graph_obj for link endpoints
As we'll need to create links between entities and interfaces,
we need to identify the link endpoints by the media_graph_obj.
Most of the changes here was done by this small script:
for i in `find drivers/media -type f` `find drivers/staging/media -type f`; do
perl -ne
's,([\w]+)\-\(source|sink)\-\entity,gobj_to_pad($1-$2)-entity,; print $_;'
$i a mv a $i
done
Please note that, while we're now using graph_obj to reference
the link endpoints, we're still assuming that all endpoints are
pads. This is true for all existing links, so no problems
are expected so far.
Yet, as we introduce links between entities and interfaces,
we may need to change some existing code to work with links
that aren't pad to pad.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/dvb-core/dvb_frontend.c
b/drivers/media/dvb-core/dvb_frontend.c
index 842b9c8f80c6..4de93d6b203e 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -635,7 +635,9 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
for (i = 0; i entity-num_links; i++) {
link = entity-links[i];
- if (link-sink-entity == entity) {
+ if (link-sink.type != MEDIA_GRAPH_PAD)
+ continue;
+ if (gobj_to_pad(link-sink)-entity == entity) {
found_link = link;
n_links++;
if (link-flags MEDIA_LNK_FL_ENABLED)
@@ -658,14 +660,16 @@ static int dvb_enable_media_tuner(struct dvb_frontend *fe)
return -EINVAL;
}
- source = found_link-source-entity;
+ source = gobj_to_pad(found_link-source)-entity;
fepriv-pipe_start_entity = source;
for (i = 0; i source-num_links; i++) {
struct media_entity *sink;
int flags = 0;
link = source-links[i];
- sink = link-sink-entity;
+ if (link-sink.type != MEDIA_GRAPH_PAD)
+ continue;
+ sink = gobj_to_pad(link-sink)-entity;
if (sink == entity)
flags = MEDIA_LNK_FL_ENABLED;
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index a95ca981aabb..40597e9a0ff0 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -155,14 +155,19 @@ static long __media_device_enum_links(struct media_device
*mdev,
list_for_each_entry(ent_link, entity-links, list) {
struct media_link_desc link;
+ /* Only PAD to PAD links should be enumerated with
legacy API */
+ if (ent_link-source-type != MEDIA_GRAPH_PAD ||
+ ent_link-sink-type != MEDIA_GRAPH_PAD)
+ continue;
+
/* Ignore backlinks. */
- if (ent_link-source-entity != entity)
+ if (gobj_to_pad(ent_link-source)-entity != entity)
continue;
memset(link, 0, sizeof(link));
- media_device_kpad_to_upad(ent_link-source,
+ media_device_kpad_to_upad(gobj_to_pad(ent_link-source),
link.source);
- media_device_kpad_to_upad(ent_link-sink,
+ media_device_kpad_to_upad(gobj_to_pad(ent_link-sink),
link.sink);
link.flags = ent_link-flags;
if (copy_to_user(ulink, link, sizeof(*ulink)))
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 4a42ccfeffab..c8bbf122f4a6 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -121,10 +121,15 @@ EXPORT_SYMBOL_GPL(media_entity_cleanup);
static struct media_entity *
media_entity_other(struct media_entity *entity, struct media_link *link)
{
- if (link-source-entity == entity)
- return link-sink-entity;
+ /* For now, we only do graph traversal with PADs */
+ if (link-sink-type != MEDIA_GRAPH_PAD ||
+ link-source-type != MEDIA_GRAPH_PAD)
+ return NULL;
+
+ if (gobj_to_pad(link-source)-entity == entity)
+ return gobj_to_pad(link-sink)-entity;
else
- return link-source-entity;
+ return gobj_to_pad(link-source)-entity;
}
/* push an entity to traversal stack */
@@ -217,6 +222,10 @@ media_entity_graph_walk_next(struct media_entity_graph
*graph)
/* Get the entity in the other end of the link . */
next = media_entity_other(entity, link);
+ if (!next) {
+ list_rotate_left(link_top(graph));
+ continue;
+ }
if (WARN_ON(next-id
Re: [PATCH RFC v2 16/16] media: add functions to allow creating interfaces
On 08/11/15 14:24, Mauro Carvalho Chehab wrote: Em Tue, 11 Aug 2015 13:14:41 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: On 08/07/15 16:20, Mauro Carvalho Chehab wrote: Interfaces are different than entities: they represent a Kernel-userspace interaction, while entities represent a piece of hardware/firmware/software that executes a function. Let's distinguish them by creating a separate structure to store the interfaces. Latter patches should change the existing drivers and logic to split the current interface embedded inside the entity structure (device nodes) into a separate object of the graph. So, to be clear, the plan is to replace the embedded media_entity struct in struct video_device by a struct media_intf_devnode pointer that is allocated with media_devnode_create()? Yes. Can we keep struct media_intf_devnode embedded in struct video_device? Or is that impossible since all media_graph_obj structs are expected to be allocated? I do have a preference for keeping structs embedded, unless there is a good reason not to. I just want to make sure there is a good reason. My plan is to always allocate the structs, as we'll very likely need to have kref for all those structs, in order to be sure that they'll be freed only after having all their usages stopped. The thing is that it is really complex when we have lots of objects linked to know when an object is not used anymore. Let's assume, for example, this simple graph: entity 0 pad 0 entity 1 pad 0 pad 1 entity 2 pad 0 link 0 entity 0:pad 0 - entity 1: pad 0 link 1 entity 1:pad 1 - entity 2: pad 0 If we need to keep back references for the links, we'll end by having both links 0 and 1 used twice. We can't free neither the entities or the pads while link 0 is not freed, but it is used twice: as a link and as a backlink. Assuming that we want to free all those objects from the memory, It can be really tricky to find the right order to free them, as we need first to go into the 3 entities and free the links there, then we can free the pads and entities. If, on the other hand, we use kref, we can simply do a for on all objects and call kref_put(). The object memory will be freed in an order that will be safe, e. g. when all kref counts are zero for that specific object. I actually postponed the usage of kref because it is a little hard to de-embed the entities on subdevices, but IMHO, using kref is the best and safest way to be sure that the Kernel won't be accessing a freed memory as the graph dynamically changes. OK, I agree. I just wanted to make sure I understood the reason for this change. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/12] tda10071: use jiffies when poll firmware status
On 08/11/2015 01:20 PM, Mauro Carvalho Chehab wrote:
Em Thu, 9 Jul 2015 07:06:29 +0300
Antti Palosaari cr...@iki.fi escreveu:
Use jiffies to set timeout for firmware command status polling.
It is more elegant solution than poll X times with sleep.
/* wait cmd execution terminate */
- for (i = 1000, uitmp = 1; i uitmp; i--) {
+ #define CMD_EXECUTE_TIMEOUT 30
+ timeout = jiffies + msecs_to_jiffies(CMD_EXECUTE_TIMEOUT);
+ for (uitmp = 1; !time_after(jiffies, timeout) uitmp;) {
ret = regmap_read(dev-regmap, 0x1f, uitmp);
if (ret)
goto error;
-
- usleep_range(200, 5000);
Hmm... removing the usleep() doesn't sound a good idea. You'll be
flooding the I2C bus with read commands and spending CPU cycles
for 30ms spending more power than the previous code. That doesn't
sound more elegant solution than poll X times with sleep for me.
So, I would keep the usleep_range() here and add a better
comment on the patch description.
First of all, polling firmware ready status is very common for chips
having firmware. And there is 2 ways to implement it:
1) poll N times in a loop using X sleep, timeout = N * X
2) poll in a loop using jiffies as a timeout
IMHO 2 is more elegant solution and I have started using it recently.
What you now propose is add some throttle in order to slow down polling
interval to reduce I2C I/O. Yes sure less I/O is better, but downside is
that it makes some unneeded extra delay to code path. Usually these sort
firmware ready polling ends rather quickly, in a loop or two.
Sure it eats some extra CPU cycles, but I think extra control messages
are about nothing compared to I/O used for data streaming.
Which kind of throttle delay you think is suitable for polling command
status over I2C bus?
regards
Antti
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 4/5] media: videobuf2: Define vb2_buf_type and vb2_memory
On 08/11/15 15:56, Laurent Pinchart wrote: Hijacking this e-mail thread a bit, would it make sense for the new vb2-core to support different memory allocation for different planes ? I'm foreseeing use cases for buffers that bundle image data with meta-data, where image data should be captured to a dma-buf imported buffer, but meta-data doesn't need to be shared. In that case it wouldn't be easy for userspace to find a dma-buf provider for the meta-data buffers in order to import all planes. Being able to use dma-buf import for the image plane(s) and mmap for the meta-data plane would be easier. Yes, that would make sense, but I'd postpone that until someone actually needs it. The biggest hurdle would be how to adapt the V4L2 API to this, and not the actual vb2 core code. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 09/16] media: use media_graph_obj for link endpoints
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 403019035424..f6e2136480f1 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -43,6 +43,17 @@ enum media_graph_type {
MEDIA_GRAPH_LINK,
};
+/**
+ * enum media_graph_link_dir - direction of a link
+ *
+ * @MEDIA_LINK_DIR_BIDIRECTIONAL Link is bidirectional
+ * @MEDIA_LINK_DIR_PAD_0_TO_1 Link is unidirectional,
+ * from port 0 (source) to port 1 (sink)
+ */
+enum media_graph_link_dir {
+ MEDIA_LINK_DIR_BIDIRECTIONAL,
+ MEDIA_LINK_DIR_PORT0_TO_PORT1,
+};
1) the comment and the actual enum are out-of-sync
Ah, yes. I was in doubt about using PAD or PORT here. I ended by using
port at the links, as the endpoints can either be an interface/entity
or a pad. So, I decided to use port.
It's either bi-directional (between interface and entity) or directional
(between two pads), so I think PAD is better here. We don't use the term
port anywhere else in the MC, so I think it is a bit confusing to
introduce a new name here.
2) why not just make a 'BIRECTIONAL' link flag instead of inventing
a new enum? Adding yet another field seems overkill to me. Have a
'BIDIRECTIONAL' flag seems perfectly OK to me (and useful for the
application as well).
Yeah, we can use flags, instead. I decided to use an enum here just
to make it clearer about the two possible options.
I was actually considering to rename media_link source/sink to
port0/port1, as using source/sink names on a bidirection link
doesn't make sense. I'm still in doubt about such rename, though,
as it would make harder to inspect the graph traversal routines.
Right. I really wouldn't rename it. As suggested below using an
anonymous union would allow you to create proper names.
Also, I want to force all places that create a link to choose
between either BIRECTIONAL or PORT0_TO_PORT1, as this makes easier
to review if the code is doing the right thing when inspecting it.
By creating two different functions? I think that would be very useful.
E.g. make_pad_link() and make_intf_to_ent_link() or something like
that. That would also hide the link direction. I still prefer a flag,
though :-) That's mostly personal preference, though.
In summary, I would prefer to keep this internally as a separate
enum, at least for now. We can latter simplify it and use a flag
for that (or maybe two flags?).
/* Structs to represent the objects that belong to a media graph */
@@ -72,9 +83,9 @@ struct media_pipeline {
struct media_link {
struct list_head list;
- struct media_graph_obj graph_obj;
- struct media_pad *source; /* Source pad */
- struct media_pad *sink; /* Sink pad */
+ struct media_graph_obj graph_obj;
+ enum media_graph_link_dir dir;
+ struct media_graph_obj *source, *sink;
I'm not too keen about all the gobj_to_foo(obj) macros that this requires. It
is rather ugly code.
What about this:
union {
struct media_graph_obj *source;
struct media_pad *source_pad;
struct media_interface *source_intf;
};
union {
struct media_graph_obj *sink;
struct media_pad *sink_pad;
struct media_entity *sink_ent;
};
Now the code can just use -source_pad etc.
good idea. Will do that on a version 3. I think that, in this case, the
best is to write a note that the first element at pad/entity/interface
should be the graph_obj.
I would actually call port0_intf and port1_ent on the above structs,
as it makes no sense to call sink/source for interface-entity links.
How about this:
union {
struct media_graph_obj *port0;
struct media_interface *port0_intf; // perhaps just intf or
interface?
struct media_pad *source;
};
union {
struct media_graph_obj *port1;
struct media_entity *port1_ent; // perhaps just ent or entity?
struct media_pad *sink;
};
This has the advantage that the source/sink pads are still called source and
sink and you don't have to rename the existing code.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mantis: remove an uneeded goto
Gotos makes a little harder to check the code. In this particular case, the goto is doing nothing but jumping into a return. Instead, just replace the goto by the return, making it simpler. Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com diff --git a/drivers/media/pci/mantis/mantis_dma.c b/drivers/media/pci/mantis/mantis_dma.c index 87990ece5848..2ce310b0a022 100644 --- a/drivers/media/pci/mantis/mantis_dma.c +++ b/drivers/media/pci/mantis/mantis_dma.c @@ -140,12 +140,10 @@ int mantis_dma_init(struct mantis_pci *mantis) /* Stop RISC Engine */ mmwrite(0, MANTIS_DMA_CTL); - goto err; + return err; } return 0; -err: - return err; } EXPORT_SYMBOL_GPL(mantis_dma_init); -- 2.4.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] zoran: Use monotonic time
Wall time obtained from do_gettimeofday is susceptible to sudden jumps due to
user setting the time or due to NTP.
Monotonic time is constantly increasing time better suited for comparing two
timestamps.
Signed-off-by: Abhilash Jindal klock.andr...@gmail.com
---
drivers/media/pci/zoran/zoran_device.c | 18 +-
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/media/pci/zoran/zoran_device.c
b/drivers/media/pci/zoran/zoran_device.c
index 40119b3..4d47dda 100644
--- a/drivers/media/pci/zoran/zoran_device.c
+++ b/drivers/media/pci/zoran/zoran_device.c
@@ -31,6 +31,7 @@
#include linux/kernel.h
#include linux/module.h
#include linux/vmalloc.h
+#include linux/ktime.h
#include linux/interrupt.h
#include linux/proc_fs.h
@@ -181,20 +182,11 @@ dump_guests (struct zoran *zr)
}
}
-static inline unsigned long
-get_time (void)
-{
- struct timeval tv;
-
- do_gettimeofday(tv);
- return (100 * tv.tv_sec + tv.tv_usec);
-}
-
void
detect_guest_activity (struct zoran *zr)
{
int timeout, i, j, res, guest[8], guest0[8], change[8][3];
- unsigned long t0, t1;
+ ktime_t t0, t1;
dump_guests(zr);
printk(KERN_INFO %s: Detecting guests activity, please wait...\n,
@@ -205,15 +197,15 @@ detect_guest_activity (struct zoran *zr)
timeout = 0;
j = 0;
- t0 = get_time();
+ t0 = ktime_get();
while (timeout 1) {
udelay(10);
timeout++;
for (i = 1; (i 8) (j 8); i++) {
res = post_office_read(zr, i, 0);
if (res != guest[i]) {
- t1 = get_time();
- change[j][0] = (t1 - t0);
+ t1 = ktime_get();
+ change[j][0] = ktime_to_us(ktime_sub(t1, t0));
t0 = t1;
change[j][1] = i;
change[j][2] = res;
--
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: [RFC PATCH v2 4/5] media: videobuf2: Define vb2_buf_type and vb2_memory
Hi Hans, On Tuesday 11 August 2015 15:59:35 Hans Verkuil wrote: On 08/11/15 15:56, Laurent Pinchart wrote: Hijacking this e-mail thread a bit, would it make sense for the new vb2-core to support different memory allocation for different planes ? I'm foreseeing use cases for buffers that bundle image data with meta-data, where image data should be captured to a dma-buf imported buffer, but meta-data doesn't need to be shared. In that case it wouldn't be easy for userspace to find a dma-buf provider for the meta-data buffers in order to import all planes. Being able to use dma-buf import for the image plane(s) and mmap for the meta-data plane would be easier. Yes, that would make sense, but I'd postpone that until someone actually needs it. I might need it soon. Looks like my cunning plan to let someone else implement it failed ;-) The biggest hurdle would be how to adapt the V4L2 API to this, and not the actual vb2 core code. Changes will be needed in both, but I agree that in-kernel changes should be less of a hassle. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] ov2659: get rid of unused values
Why to store the chosed values for prediv, postdiv and mult if
those won't be used?
drivers/media/i2c/ov2659.c: In function 'ov2659_pll_calc_params':
drivers/media/i2c/ov2659.c:912:35: warning: variable 's_mult' set but not used
[-Wunused-but-set-variable]
u32 s_prediv = 1, s_postdiv = 1, s_mult = 1;
^
drivers/media/i2c/ov2659.c:912:20: warning: variable 's_postdiv' set but not
used [-Wunused-but-set-variable]
u32 s_prediv = 1, s_postdiv = 1, s_mult = 1;
^
drivers/media/i2c/ov2659.c:912:6: warning: variable 's_prediv' set but not used
[-Wunused-but-set-variable]
u32 s_prediv = 1, s_postdiv = 1, s_mult = 1;
^
This is likely some leftover from some past change.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
---
drivers/media/i2c/ov2659.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
index 6edffc7b74e3..49109f4f5bb4 100644
--- a/drivers/media/i2c/ov2659.c
+++ b/drivers/media/i2c/ov2659.c
@@ -909,7 +909,6 @@ static void ov2659_pll_calc_params(struct ov2659 *ov2659)
u8 ctrl1_reg = 0, ctrl2_reg = 0, ctrl3_reg = 0;
struct i2c_client *client = ov2659-client;
unsigned int desired = pdata-link_frequency;
- u32 s_prediv = 1, s_postdiv = 1, s_mult = 1;
u32 prediv, postdiv, mult;
u32 bestdelta = -1;
u32 delta, actual;
@@ -929,9 +928,6 @@ static void ov2659_pll_calc_params(struct ov2659 *ov2659)
if ((delta bestdelta) || (bestdelta == -1)) {
bestdelta = delta;
- s_mult= mult;
- s_prediv = prediv;
- s_postdiv = postdiv;
ctrl1_reg = ctrl1[i].reg;
ctrl2_reg = mult;
ctrl3_reg = ctrl3[j].reg;
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe 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] ov9650: remove an extra space
drivers/media/i2c/ov9650.c:1439 ov965x_detect_sensor() warn: inconsistent indenting Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com --- drivers/media/i2c/ov9650.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c index 2bc473385c91..e691bba1945b 100644 --- a/drivers/media/i2c/ov9650.c +++ b/drivers/media/i2c/ov9650.c @@ -1436,7 +1436,7 @@ static int ov965x_detect_sensor(struct v4l2_subdev *sd) int ret; mutex_lock(ov965x-lock); -__ov965x_set_power(ov965x, 1); + __ov965x_set_power(ov965x, 1); usleep_range(25000, 26000); /* Check sensor revision */ -- 2.4.3 -- To unsubscribe from this list: send the line unsubscribe 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] sr030pc30: don't read a new pointer
sr030pc30_get_fmt() can only succeed if both info-curr_win and
info-curr_fmt are not NULL.
If one of those vars are null, the curent code would call:
ret = sr030pc30_set_params(sd);
If the curr_win is null, it will return -EINVAL, as it would be
expected. However, if curr_fmt is NULL, the function won't
set it.
The code will then try to read from it:
mf-code= info-curr_fmt-code;
mf-colorspace = info-curr_fmt-colorspace;
with obviouly won't work.
This got reported by smatch:
drivers/media/i2c/sr030pc30.c:505 sr030pc30_get_fmt() error: we
previously assumed 'info-curr_win' could be null (see line 499)
drivers/media/i2c/sr030pc30.c:507 sr030pc30_get_fmt() error: we
previously assumed 'info-curr_fmt' could be null (see line 499)
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
---
drivers/media/i2c/sr030pc30.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/media/i2c/sr030pc30.c b/drivers/media/i2c/sr030pc30.c
index 229dc76c44a5..47ebe27cb00e 100644
--- a/drivers/media/i2c/sr030pc30.c
+++ b/drivers/media/i2c/sr030pc30.c
@@ -496,11 +496,8 @@ static int sr030pc30_get_fmt(struct v4l2_subdev *sd,
mf = format-format;
- if (!info-curr_win || !info-curr_fmt) {
- ret = sr030pc30_set_params(sd);
- if (ret)
- return ret;
- }
+ if (!info-curr_win || !info-curr_fmt)
+ return -EINVAL;
mf-width = info-curr_win-width;
mf-height = info-curr_win-height;
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe 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] tc358743: don't use variable length array for I2C writes
drivers/media/i2c/tc358743.c:148:19: warning: Variable length array is used.
As the maximum size is 1026, we can't use dynamic var, as it
would otherwise spend 1056 bytes of the stack at i2c_wr() function.
So, allocate a buffer with the allowed maximum size together with
the state var.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
---
drivers/media/i2c/tc358743.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 2e926317d7e9..fe42c9a1cb78 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -59,6 +59,9 @@ MODULE_LICENSE(GPL);
#define EDID_NUM_BLOCKS_MAX 8
#define EDID_BLOCK_SIZE 128
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE (EDID_NUM_BLOCKS_MAX * EDID_BLOCK_SIZE + 2)
+
static const struct v4l2_dv_timings_cap tc358743_timings_cap = {
.type = V4L2_DV_BT_656_1120,
/* keep this initialization for compatibility with GCC 4.4.6 */
@@ -94,6 +97,9 @@ struct tc358743_state {
/* edid */
u8 edid_blocks_written;
+ /* used by i2c_wr() */
+ u8 wr_data[MAX_XFER_SIZE];
+
struct v4l2_dv_timings timings;
u32 mbus_fmt_code;
@@ -143,9 +149,13 @@ static void i2c_wr(struct v4l2_subdev *sd, u16 reg, u8
*values, u32 n)
{
struct tc358743_state *state = to_state(sd);
struct i2c_client *client = state-i2c_client;
+ u8 *data = state-wr_data;
int err, i;
struct i2c_msg msg;
- u8 data[2 + n];
+
+ if ((2 + n) sizeof(state-wr_data))
+ v4l2_warn(sd, i2c wr reg=%04x: len=%d is too big!\n,
+ reg, 2 + n);
msg.addr = client-addr;
msg.buf = data;
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] media: flash: Don't initialize core ops
queryctrl and querymenu menu ops don't need to be initialized
if sd.ctrl_handler is set. Since no other core ops are required
by the wrapper don't initialize related field of v4l2_flash_subdev_ops.
Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com
Cc: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/v4l2-core/v4l2-flash-led-class.c |6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c
b/drivers/media/v4l2-core/v4l2-flash-led-class.c
index 5bdfb8d..57a1829 100644
--- a/drivers/media/v4l2-core/v4l2-flash-led-class.c
+++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c
@@ -609,13 +609,7 @@ static const struct v4l2_subdev_internal_ops
v4l2_flash_subdev_internal_ops = {
.close = v4l2_flash_close,
};
-static const struct v4l2_subdev_core_ops v4l2_flash_core_ops = {
- .queryctrl = v4l2_subdev_queryctrl,
- .querymenu = v4l2_subdev_querymenu,
-};
-
static const struct v4l2_subdev_ops v4l2_flash_subdev_ops = {
- .core = v4l2_flash_core_ops,
};
struct v4l2_flash *v4l2_flash_init(
--
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: [RFC PATCH v2 3/5] media: videobuf2: Divide videobuf2-core into 2 parts
Hello, On Monday 10 August 2015 15:38:48 Hans Verkuil wrote: On 08/10/2015 02:55 PM, Mauro Carvalho Chehab wrote: Em Mon, 10 Aug 2015 14:07:21 +0200 Hans Verkuil escreveu: Hi Junghak, I'm reviewing the header changes since I think there are several improvements that can be done that will make things more logical and will simplify the code. My comments below are a mix of suggestions for improvement and brainstorming. Feel free to ask for clarification if something is not clear. On 07/31/2015 10:44 AM, Junghak Sung wrote: Divide videobuf2-core into core part and v4l2-specific part - core part: videobuf2 core related with buffer management memory allocation - v4l2-specific part: v4l2-specific stuff Signed-off-by: Junghak Sung jh1009.s...@samsung.com Signed-off-by: Geunyoung Kim nenggun@samsung.com Acked-by: Seung-Woo Kim sw0312@samsung.com Acked-by: Inki Dae inki@samsung.com --- snip ... I noticed that __qbuf_mmap/userptr/dmabuf are all in -v4l2.c. That's a bad sign: those are some of the most complex vb2 functions and they really belong in the core since you'll need it for DVB as well. As suggested above, by moving the index, length and offset/userptr/fd data to the core structs these functions can all be moved back into core.c as far as I can see. Well, that will depend on how the DVB implementation will actually be. Currently, VB2 has lot of V4L2-dependent code on it, with lots of V4L2 structs from videodev2.h that are there. Well, if we want the core to be re-used, it should not include videodev2.h anymore. Also, it should not assume that all non-V4L2 cores would use exactly the same logic for the userspace API. Agreed. In the DVB case, it makes no sense to have anything similar to OVERLAY there. VB2 doesn't support overlay at all, so that's no problem. I also can't see any usage for USERPTR at DVB neither, as either MMAP or DMABUF should fulfill all userspace needs I'm aware of. While USERPTR isn't needed for DVB, the actual handling of such buffers is completely independent of the API. I think it is from an architecture point-of-view a really bad idea if anyone other than the vb2 core would call the memops. So yes, the core would have code that is not needed by DVB, but it still is something that belongs to the core in my view. Anything else is very ugly. I agree, USERPTR handling belongs to the core as that's not dependent on V4L2. However, I'd make USERPTR usage not recommended for new drivers/subsystems (that's a polite way to say don't even think about using it :-)) as DMABUF should support the USERPTR use cases nowadays, and USERPTR wobbly as DMA to userspace memory isn't officially supported on all architectures. Also, the meta data for the DVB upcoming ioctls for MMAP/DMABUF aren't yet proposed. They can be very different than the ones inside the V4L2 ioctls. Well, it's pretty much constrained by mmap() and the dma-buf API. I.e. an offset for for mmap and a fd for dmabuf. You don't have a choice here. So, I guess it is better for now to keep those API-dependent stuff at VB2-v4l2 and, once the DVB code (and the corresponding API bits) are written, revisit it and then move the common code to the VB2 core. I strongly disagree with that. Having API-dependent code calling memops defeats the purpose. There is nothing wrong with having a vb2 core that supports e.g. USERPTR and dma-buf as long as that core is API-independent. But there is a lot wrong if the API-dependent code is bypassing the vb2 core code to call low-level memops. It is good to remember that today the v4l2_buffer struct is used in the vb2 core because vb2 is only used with v4l2, so why duplicate v4l2_buffer fields in the vb2 core structs? We should not have any v4l2_* struct inside VB2 core, as the DVB core should not be dependent on the V4L2 structs. So, everything that it is V4L2-specific should be inside the VB2-v4l2. The reverse is also true: we should not pollute the VB2 core with DVB-specific data structures. I never said anything else. I was talking about what's used today and why it is the way it is now. So, all VB2-specific struct should be at VB2-dvb. But if we want to reuse it for other subsystems, then the vb2 core structs should contain all the core buffer information. This avoids the need for a lot of the ops that you added and makes it possible to keep the __qbuf_mmap/userptr/dmabuf in the core code as well. Adding these fields to the vb2 core structs is something that can be done first, before splitting up core.c into core.c and v4l2.c. I'm afraid that we'll lose the big picture if we try to put the API-dependent parts at the core before adding a non-V4L2 usage on VB2. We can always simplify the code latter, but IMHO we should focus first on adding the new functionality (support for DVB).
Re: [PATCH] [media] i2c/adv7511: Fix license, set to GPL v2
Okay, I split it up and sent it to the proper lists. Just noticed I forgot to set the in-reply-to headers though. Hope that won't be a problem. On 11-08-15 13:45, Hans Verkuil wrote: Hi Mike, Please split up this patch: these are two different drivers with different authors and different subsystems. The media/i2c/adv7511.c patch I can handle, but the patch for the drm driver should go to the dri-devel mailinglist. I can't take that change. Easiest is just to post two patches, one for each driver. Regards, Hans On 07/28/15 12:57, Mike Looijmans wrote: Header claims GPL v2, so make the MODULE_LICENSE reflect that properly. Signed-off-by: Mike Looijmans mike.looijm...@topic.nl --- drivers/gpu/drm/i2c/adv7511_core.c | 2 +- drivers/media/i2c/adv7511.c| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i2c/adv7511_core.c b/drivers/gpu/drm/i2c/adv7511_core.c index 2564b5d..12e8134 100644 --- a/drivers/gpu/drm/i2c/adv7511_core.c +++ b/drivers/gpu/drm/i2c/adv7511_core.c @@ -956,4 +956,4 @@ module_exit(adv7511_exit); MODULE_AUTHOR(Lars-Peter Clausen l...@metafoo.de); MODULE_DESCRIPTION(ADV7511 HDMI transmitter driver); -MODULE_LICENSE(GPL); +MODULE_LICENSE(GPL v2); diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c index 02d76c6..1a4275d 100644 --- a/drivers/media/i2c/adv7511.c +++ b/drivers/media/i2c/adv7511.c @@ -41,7 +41,7 @@ MODULE_PARM_DESC(debug, debug level (0-2)); MODULE_DESCRIPTION(Analog Devices ADV7511 HDMI Transmitter Device Driver); MODULE_AUTHOR(Hans Verkuil); -MODULE_LICENSE(GPL); +MODULE_LICENSE(GPL v2); #define MASK_ADV7511_EDID_RDY_INT 0x04 #define MASK_ADV7511_MSEN_INT 0x40 Kind regards, Mike Looijmans System Expert TOPIC Embedded Products Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: +31 (0) 499 33 69 79 Telefax: +31 (0) 499 33 69 70 E-mail: mike.looijm...@topicproducts.com Website: www.topicproducts.com Please consider the environment before printing this e-mail -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 4/5] media: videobuf2: Define vb2_buf_type and vb2_memory
Hello,
On Monday 10 August 2015 10:22:12 Hans Verkuil wrote:
On 07/31/2015 10:44 AM, Junghak Sung wrote:
Define enum vb2_buf_type and enum vb2_memory for videobuf2-core. This
change requires translation functions that could covert v4l2-core stuffs
to videobuf2-core stuffs in videobuf2-v4l2.c file.
The v4l2-specific member variables(e.g. type, memory) remains in
struct vb2_queue for backward compatibility and performance of type
translation.
Signed-off-by: Junghak Sung jh1009.s...@samsung.com
Signed-off-by: Geunyoung Kim nenggun@samsung.com
Acked-by: Seung-Woo Kim sw0312@samsung.com
Acked-by: Inki Dae inki@samsung.com
---
drivers/media/v4l2-core/videobuf2-core.c | 139 +++-
drivers/media/v4l2-core/videobuf2-v4l2.c | 209 ++---
include/media/videobuf2-core.h | 99 +++---
include/media/videobuf2-v4l2.h | 12 +-
4 files changed, 299 insertions(+), 160 deletions(-)
snip
diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c
b/drivers/media/v4l2-core/videobuf2-v4l2.c index 85527e9..22dd19c 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -30,8 +30,46 @@
#include media/v4l2-common.h
#include media/videobuf2-v4l2.h
+#define CREATE_TRACE_POINTS
#include trace/events/v4l2.h
+static const enum vb2_buf_type _tbl_buf_type[] = {
+ [V4L2_BUF_TYPE_VIDEO_CAPTURE] = VB2_BUF_TYPE_VIDEO_CAPTURE,
+ [V4L2_BUF_TYPE_VIDEO_OUTPUT]= VB2_BUF_TYPE_VIDEO_OUTPUT,
+ [V4L2_BUF_TYPE_VIDEO_OVERLAY] = VB2_BUF_TYPE_VIDEO_OVERLAY,
+ [V4L2_BUF_TYPE_VBI_CAPTURE] = VB2_BUF_TYPE_VBI_CAPTURE,
+ [V4L2_BUF_TYPE_VBI_OUTPUT] = VB2_BUF_TYPE_VBI_OUTPUT,
+ [V4L2_BUF_TYPE_SLICED_VBI_CAPTURE] =
VB2_BUF_TYPE_SLICED_VBI_CAPTURE,
+ [V4L2_BUF_TYPE_SLICED_VBI_OUTPUT] =
VB2_BUF_TYPE_SLICED_VBI_OUTPUT,
+ [V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY]=
VB2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY,
+ [V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE]=
VB2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ [V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE] =
VB2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ [V4L2_BUF_TYPE_SDR_CAPTURE] = VB2_BUF_TYPE_SDR_CAPTURE,
+ [V4L2_BUF_TYPE_PRIVATE] = VB2_BUF_TYPE_PRIVATE,
+};
+
+static const enum vb2_memory _tbl_memory[] = {
+ [V4L2_MEMORY_MMAP] = VB2_MEMORY_MMAP,
+ [V4L2_MEMORY_USERPTR] = VB2_MEMORY_USERPTR,
+ [V4L2_MEMORY_DMABUF]= VB2_MEMORY_DMABUF,
+};
+
+#define to_vb2_buf_type(type) \
+({ \
+ enum vb2_buf_type ret = 0; \
+ if( type 0 type ARRAY_SIZE(_tbl_buf_type) ) \
+ ret = (_tbl_buf_type[type]);\
+ ret;\
+})
+
+#define to_vb2_memory(memory) \
+({ \
+ enum vb2_memory ret = 0;\
+ if( memory 0 memory ARRAY_SIZE(_tbl_memory) )\
+ ret = (_tbl_memory[memory]);\
+ ret;\
+})
+
snip
diff --git a/include/media/videobuf2-core.h
b/include/media/videobuf2-core.h index dc405da..871fcc6 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -15,9 +15,47 @@
#include linux/mm_types.h
#include linux/mutex.h
#include linux/poll.h
-#include linux/videodev2.h
#include linux/dma-buf.h
+#define VB2_MAX_FRAME 32
+#define VB2_MAX_PLANES 8
+
+enum vb2_buf_type {
+ VB2_BUF_TYPE_UNKNOWN= 0,
+ VB2_BUF_TYPE_VIDEO_CAPTURE = 1,
+ VB2_BUF_TYPE_VIDEO_OUTPUT = 2,
+ VB2_BUF_TYPE_VIDEO_OVERLAY = 3,
+ VB2_BUF_TYPE_VBI_CAPTURE= 4,
+ VB2_BUF_TYPE_VBI_OUTPUT = 5,
+ VB2_BUF_TYPE_SLICED_VBI_CAPTURE = 6,
+ VB2_BUF_TYPE_SLICED_VBI_OUTPUT = 7,
+ VB2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY = 8,
+ VB2_BUF_TYPE_VIDEO_CAPTURE_MPLANE = 9,
+ VB2_BUF_TYPE_VIDEO_OUTPUT_MPLANE= 10,
+ VB2_BUF_TYPE_SDR_CAPTURE= 11,
+ VB2_BUF_TYPE_DVB_CAPTURE= 12,
+ VB2_BUF_TYPE_PRIVATE= 0x80,
+};
+
+enum vb2_memory {
+ VB2_MEMORY_UNKNOWN = 0,
+ VB2_MEMORY_MMAP = 1,
+ VB2_MEMORY_USERPTR = 2,
+ VB2_MEMORY_DMABUF = 4,
+};
+
+#define VB2_TYPE_IS_MULTIPLANAR(type) \
+ ((type) == VB2_BUF_TYPE_VIDEO_CAPTURE_MPLANE\
+|| (type) == VB2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+
+#define
Re: [GIT PULL FOR v4.3] Various fixes
Hi Hans, On Tue, 11 Aug 2015, Hans Verkuil wrote: 0015-v4l-subdev-Add-pad-config-allocator-and-init.patch 0016-media-soc_camera-rcar_vin-Add-BT.709-24-bit-RGB888-i.patch 0017-media-soc_camera-pad-aware-driver-initialisation.patch 0018-media-rcar_vin-Use-correct-pad-number-in-try_fmt.patch 0019-media-soc_camera-soc_scale_crop-Use-correct-pad-numb.patch 0020-media-rcar_vin-fill-in-bus_info-field.patch 0021-media-rcar_vin-Reject-videobufs-that-are-too-small-f.patch William, can you take a look at this? Just let me know which patches are independent to patch 0015. Of those, the patches that *do* call the allocator function directly or are otherwise co-dependent are numbers 17-19 inclusive. The independent patches in that list are therefore: [prerequisite work by Laurent Pinchart (1x)...] 0016-media-soc_camera-rcar_vin-Add-BT.709-24-bit-RGB888-i.patch [general rcar_vin enhancements by Rob Taylor (2x)...] 0020-media-rcar_vin-fill-in-bus_info-field.patch 0021-media-rcar_vin-Reject-videobufs-that-are-too-small-f.patch Cheers, Wills. -- To unsubscribe from this list: send the line unsubscribe 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] tc358743: don't use variable length array for I2C writes
drivers/media/i2c/tc358743.c:148:19: warning: Variable length array is used.
As the maximum size is 1026, we can't use dynamic var, as it
would otherwise spend 1056 bytes of the stack at i2c_wr() function.
So, allocate a buffer with the allowed maximum size together with
the state var.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 2e926317d7e9..fe42c9a1cb78 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -59,6 +59,9 @@ MODULE_LICENSE(GPL);
#define EDID_NUM_BLOCKS_MAX 8
#define EDID_BLOCK_SIZE 128
+/* Max transfer size done by I2C transfer functions */
+#define MAX_XFER_SIZE (EDID_NUM_BLOCKS_MAX * EDID_BLOCK_SIZE + 2)
+
static const struct v4l2_dv_timings_cap tc358743_timings_cap = {
.type = V4L2_DV_BT_656_1120,
/* keep this initialization for compatibility with GCC 4.4.6 */
@@ -94,6 +97,9 @@ struct tc358743_state {
/* edid */
u8 edid_blocks_written;
+ /* used by i2c_wr() */
+ u8 wr_data[MAX_XFER_SIZE];
+
struct v4l2_dv_timings timings;
u32 mbus_fmt_code;
@@ -143,9 +149,13 @@ static void i2c_wr(struct v4l2_subdev *sd, u16 reg, u8
*values, u32 n)
{
struct tc358743_state *state = to_state(sd);
struct i2c_client *client = state-i2c_client;
+ u8 *data = state-wr_data;
int err, i;
struct i2c_msg msg;
- u8 data[2 + n];
+
+ if ((2 + n) sizeof(state-wr_data))
+ v4l2_warn(sd, i2c wr reg=%04x: len=%d is too big!\n,
+ reg, 2 + n);
msg.addr = client-addr;
msg.buf = data;
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe 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 09/12] tda10071: use jiffies when poll firmware status
Em Tue, 11 Aug 2015 18:18:39 +0300
Antti Palosaari cr...@iki.fi escreveu:
On 08/11/2015 01:20 PM, Mauro Carvalho Chehab wrote:
Em Thu, 9 Jul 2015 07:06:29 +0300
Antti Palosaari cr...@iki.fi escreveu:
Use jiffies to set timeout for firmware command status polling.
It is more elegant solution than poll X times with sleep.
/* wait cmd execution terminate */
- for (i = 1000, uitmp = 1; i uitmp; i--) {
+ #define CMD_EXECUTE_TIMEOUT 30
+ timeout = jiffies + msecs_to_jiffies(CMD_EXECUTE_TIMEOUT);
+ for (uitmp = 1; !time_after(jiffies, timeout) uitmp;) {
ret = regmap_read(dev-regmap, 0x1f, uitmp);
if (ret)
goto error;
-
- usleep_range(200, 5000);
Hmm... removing the usleep() doesn't sound a good idea. You'll be
flooding the I2C bus with read commands and spending CPU cycles
for 30ms spending more power than the previous code. That doesn't
sound more elegant solution than poll X times with sleep for me.
So, I would keep the usleep_range() here and add a better
comment on the patch description.
First of all, polling firmware ready status is very common for chips
having firmware. And there is 2 ways to implement it:
1) poll N times in a loop using X sleep, timeout = N * X
2) poll in a loop using jiffies as a timeout
IMHO 2 is more elegant solution and I have started using it recently.
Yes, (2) is more elegant.
What you now propose is add some throttle in order to slow down polling
interval to reduce I2C I/O. Yes sure less I/O is better, but downside is
that it makes some unneeded extra delay to code path. Usually these sort
firmware ready polling ends rather quickly, in a loop or two.
If only few interactions is needed, then OK. Please add a comment then,
explaining that.
Sure it eats some extra CPU cycles, but I think extra control messages
are about nothing compared to I/O used for data streaming.
Which kind of throttle delay you think is suitable for polling command
status over I2C bus?
regards
Antti
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] bt8xxx: Use monotonic time
Wall time obtained from do_gettimeofday is susceptible to sudden jumps due to
user setting the time or due to NTP.
Monotonic time is constantly increasing time better suited for comparing two
timestamps.
Signed-off-by: Abhilash Jindal klock.andr...@gmail.com
---
drivers/media/pci/bt8xx/bttv-input.c | 23 +--
drivers/media/pci/bt8xx/bttvp.h |2 +-
2 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/media/pci/bt8xx/bttv-input.c
b/drivers/media/pci/bt8xx/bttv-input.c
index 67c8d6b..0f21771 100644
--- a/drivers/media/pci/bt8xx/bttv-input.c
+++ b/drivers/media/pci/bt8xx/bttv-input.c
@@ -194,21 +194,18 @@ static u32 bttv_rc5_decode(unsigned int code)
static void bttv_rc5_timer_end(unsigned long data)
{
struct bttv_ir *ir = (struct bttv_ir *)data;
- struct timeval tv;
+ ktime_t tv;
u32 gap, rc5, scancode;
u8 toggle, command, system;
/* get time */
- do_gettimeofday(tv);
+ tv = ktime_get();
+ gap = ktime_to_us(ktime_sub(tv, ir-base_time));
/* avoid overflow with gap 1s */
- if (tv.tv_sec - ir-base_time.tv_sec 1) {
+ if (gap USEC_PER_SEC) {
gap = 20;
- } else {
- gap = 100 * (tv.tv_sec - ir-base_time.tv_sec) +
- tv.tv_usec - ir-base_time.tv_usec;
- }
-
+ }
/* signal we're ready to start a new code */
ir-active = false;
@@ -249,7 +246,7 @@ static void bttv_rc5_timer_end(unsigned long data)
static int bttv_rc5_irq(struct bttv *btv)
{
struct bttv_ir *ir = btv-remote;
- struct timeval tv;
+ ktime_t tv;
u32 gpio;
u32 gap;
unsigned long current_jiffies;
@@ -259,14 +256,12 @@ static int bttv_rc5_irq(struct bttv *btv)
/* get time of bit */
current_jiffies = jiffies;
- do_gettimeofday(tv);
+ tv = ktime_get();
+ gap = ktime_to_us(ktime_sub(tv, ir-base_time));
/* avoid overflow with gap 1s */
- if (tv.tv_sec - ir-base_time.tv_sec 1) {
+ if (gap USEC_PER_SEC) {
gap = 20;
- } else {
- gap = 100 * (tv.tv_sec - ir-base_time.tv_sec) +
- tv.tv_usec - ir-base_time.tv_usec;
}
dprintk(RC5 IRQ: gap %d us for %s\n,
diff --git a/drivers/media/pci/bt8xx/bttvp.h b/drivers/media/pci/bt8xx/bttvp.h
index a444cfb..31bf79d 100644
--- a/drivers/media/pci/bt8xx/bttvp.h
+++ b/drivers/media/pci/bt8xx/bttvp.h
@@ -140,7 +140,7 @@ struct bttv_ir {
boolrc5_gpio; /* Is RC5 legacy GPIO enabled? */
u32 last_bit; /* last raw bit seen */
u32 code; /* raw code under construction */
- struct timeval base_time; /* time of last seen code */
+ ktime_t base_time; /* time of
last seen code */
boolactive; /* building raw code */
};
--
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] rc-core: improve the lirc protocol reporting
Hi David,
Em Wed, 22 Jul 2015 22:55:24 +0200
David Härdeman da...@hardeman.nu escreveu:
Commit 275ddb40bcf686d210d86c6718e42425a6a0bc76 removed the lirc
protocol but kept backwards compatibility by always listing
the protocol as present and enabled. This patch further improves
the logic by only listing the protocol if the lirc module is loaded
(or if lirc is builtin).
Makes sense, but see below.
Signed-off-by: David Härdeman da...@hardeman.nu
---
drivers/media/rc/rc-main.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index ecaee02..3f0f71a 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -828,6 +828,23 @@ struct rc_filter_attribute {
.mask = (_mask),\
}
+static bool lirc_is_present(void)
+{
+#if defined(CONFIG_LIRC_MODULE)
+ struct module *lirc;
+
+ mutex_lock(module_mutex);
+ lirc = find_module(lirc_dev);
+ mutex_unlock(module_mutex);
I don't think it would be a good idea to play with the module mutex
lock here or calling find_module(). This is something that no other driver
does (well, except for FB DRM driver, that dynamically loads a module
if not found).
Perhaps we could use some simpler logic, like storing some value if lirc
got loaded or not (worse case, we might use a static atomic var at rc core).
+
+ return lirc ? true : false;
+#elif defined(CONFIG_LIRC)
+ return true;
+#else
+ return false;
+#endif
+}
+
/**
* show_protocols() - shows the current/wakeup IR protocol(s)
* @device: the device descriptor
@@ -882,7 +899,7 @@ static ssize_t show_protocols(struct device *device,
allowed = ~proto_names[i].type;
}
- if (dev-driver_type == RC_DRIVER_IR_RAW)
+ if (dev-driver_type == RC_DRIVER_IR_RAW lirc_is_present())
tmp += sprintf(tmp, [lirc] );
if (tmp != buf)
--
To unsubscribe from this list: send the line unsubscribe 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
[PATCH 3/4] ov2659: get rid of unused values
Why to store the chosed values for prediv, postdiv and mult if
those won't be used?
drivers/media/i2c/ov2659.c: In function 'ov2659_pll_calc_params':
drivers/media/i2c/ov2659.c:912:35: warning: variable 's_mult' set but not used
[-Wunused-but-set-variable]
u32 s_prediv = 1, s_postdiv = 1, s_mult = 1;
^
drivers/media/i2c/ov2659.c:912:20: warning: variable 's_postdiv' set but not
used [-Wunused-but-set-variable]
u32 s_prediv = 1, s_postdiv = 1, s_mult = 1;
^
drivers/media/i2c/ov2659.c:912:6: warning: variable 's_prediv' set but not used
[-Wunused-but-set-variable]
u32 s_prediv = 1, s_postdiv = 1, s_mult = 1;
^
This is likely some leftover from some past change.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
index 6edffc7b74e3..49109f4f5bb4 100644
--- a/drivers/media/i2c/ov2659.c
+++ b/drivers/media/i2c/ov2659.c
@@ -909,7 +909,6 @@ static void ov2659_pll_calc_params(struct ov2659 *ov2659)
u8 ctrl1_reg = 0, ctrl2_reg = 0, ctrl3_reg = 0;
struct i2c_client *client = ov2659-client;
unsigned int desired = pdata-link_frequency;
- u32 s_prediv = 1, s_postdiv = 1, s_mult = 1;
u32 prediv, postdiv, mult;
u32 bestdelta = -1;
u32 delta, actual;
@@ -929,9 +928,6 @@ static void ov2659_pll_calc_params(struct ov2659 *ov2659)
if ((delta bestdelta) || (bestdelta == -1)) {
bestdelta = delta;
- s_mult= mult;
- s_prediv = prediv;
- s_postdiv = postdiv;
ctrl1_reg = ctrl1[i].reg;
ctrl2_reg = mult;
ctrl3_reg = ctrl3[j].reg;
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe 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] ov9650: remove an extra space
drivers/media/i2c/ov9650.c:1439 ov965x_detect_sensor() warn: inconsistent indenting Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c index 2bc473385c91..e691bba1945b 100644 --- a/drivers/media/i2c/ov9650.c +++ b/drivers/media/i2c/ov9650.c @@ -1436,7 +1436,7 @@ static int ov965x_detect_sensor(struct v4l2_subdev *sd) int ret; mutex_lock(ov965x-lock); -__ov965x_set_power(ov965x, 1); + __ov965x_set_power(ov965x, 1); usleep_range(25000, 26000); /* Check sensor revision */ -- 2.4.3 -- To unsubscribe from this list: send the line unsubscribe 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] sr030pc30: don't read a new pointer
sr030pc30_get_fmt() can only succeed if both info-curr_win and
info-curr_fmt are not NULL.
If one of those vars are null, the curent code would call:
ret = sr030pc30_set_params(sd);
If the curr_win is null, it will return -EINVAL, as it would be
expected. However, if curr_fmt is NULL, the function won't
set it.
The code will then try to read from it:
mf-code= info-curr_fmt-code;
mf-colorspace = info-curr_fmt-colorspace;
with obviouly won't work.
This got reported by smatch:
drivers/media/i2c/sr030pc30.c:505 sr030pc30_get_fmt() error: we
previously assumed 'info-curr_win' could be null (see line 499)
drivers/media/i2c/sr030pc30.c:507 sr030pc30_get_fmt() error: we
previously assumed 'info-curr_fmt' could be null (see line 499)
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/i2c/sr030pc30.c b/drivers/media/i2c/sr030pc30.c
index 229dc76c44a5..47ebe27cb00e 100644
--- a/drivers/media/i2c/sr030pc30.c
+++ b/drivers/media/i2c/sr030pc30.c
@@ -496,11 +496,8 @@ static int sr030pc30_get_fmt(struct v4l2_subdev *sd,
mf = format-format;
- if (!info-curr_win || !info-curr_fmt) {
- ret = sr030pc30_set_params(sd);
- if (ret)
- return ret;
- }
+ if (!info-curr_win || !info-curr_fmt)
+ return -EINVAL;
mf-width = info-curr_win-width;
mf-height = info-curr_win-height;
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] cxd2841er: declare static functions as such
drivers/media/dvb-frontends/cxd2841er.c:992:5: warning: no previous prototype
for 'cxd2841er_get_carrier_offset_t2' [-Wmissing-prototypes]
int cxd2841er_get_carrier_offset_t2(
^
drivers/media/dvb-frontends/cxd2841er.c:1032:5: warning: no previous prototype
for 'cxd2841er_get_carrier_offset_c' [-Wmissing-prototypes]
int cxd2841er_get_carrier_offset_c(
^
drivers/media/dvb-frontends/cxd2841er.c:1360:5: warning: no previous prototype
for 'cxd2841er_read_snr_t2' [-Wmissing-prototypes]
int cxd2841er_read_snr_t2(struct cxd2841er_priv *priv, u32 *snr)
^
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
---
drivers/media/dvb-frontends/cxd2841er.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/dvb-frontends/cxd2841er.c
b/drivers/media/dvb-frontends/cxd2841er.c
index d3813cc22770..0d1a15109d1e 100644
--- a/drivers/media/dvb-frontends/cxd2841er.c
+++ b/drivers/media/dvb-frontends/cxd2841er.c
@@ -989,8 +989,8 @@ static int cxd2841er_get_carrier_offset_s_s2(struct
cxd2841er_priv *priv,
return 0;
}
-int cxd2841er_get_carrier_offset_t2(
- struct cxd2841er_priv *priv, u32 bandwidth, int *offset)
+static int cxd2841er_get_carrier_offset_t2(struct cxd2841er_priv *priv,
+ u32 bandwidth, int *offset)
{
u8 data[4];
@@ -1029,8 +1029,8 @@ int cxd2841er_get_carrier_offset_t2(
return 0;
}
-int cxd2841er_get_carrier_offset_c(
- struct cxd2841er_priv *priv, int *offset)
+static int cxd2841er_get_carrier_offset_c(struct cxd2841er_priv *priv,
+ int *offset)
{
u8 data[2];
@@ -1357,7 +1357,7 @@ static int cxd2841er_read_snr_t(struct cxd2841er_priv
*priv, u32 *snr)
return 0;
}
-int cxd2841er_read_snr_t2(struct cxd2841er_priv *priv, u32 *snr)
+static int cxd2841er_read_snr_t2(struct cxd2841er_priv *priv, u32 *snr)
{
u32 reg;
u8 data[2];
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe 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] DT support for omap4-iss
On 11 August 2015 at 13:16, Tony Lindgren t...@atomide.com wrote:
* Michael Allwright michael.allwri...@upb.de [150810 08:19]:
+
+/*
+We need a better solution for this
+*/
+#include ../arch/arm/mach-omap2/omap-pm.h
Please let's not do things like this, I end up having to deal with
all these eventually :(
+static void iss_set_constraints(struct iss_device *iss, bool enable)
+{
+if (!iss)
+return;
+
+/* FIXME: Look for something more precise as a good throughtput limit */
+omap_pm_set_min_bus_tput(iss-dev, OCP_INITIATOR_AGENT,
+ enable ? 80 : -1);
+}
+
+static struct iss_platform_data iss_dummy_pdata = {
+.set_constraints = iss_set_constraints,
+};
If this is one time setting, you could do it based on the
compatible string using arch/arm/mach-omap2/pdata-quirks.c.
If you need to toggle it, you could populate a function pointer
in pdata-quirks.c. Those are easy to fix once there is some Linux
generic API available :)
Regards,
Tony
Hi Tony,
Thanks for the suggestion, I'll move that function into
pdata-quirks.c. Please read on, I really need some help regarding the
following error, I lost 8-9 hours on this today.
[ 141.612609] omap4iss 5200.iss: CSI2: CSI2_96M_FCLK reset timeout!
This comes from the function: int omap4iss_csi2_reset(struct
iss_csi2_device *csi2) in iss_csi2.c. I have found that bit 29 in
REGISTER1 belonging to the CSI2A registers, isn't becoming high after
doing the reset on kernel 4.1.4. However it does come high in 3.17.
This bit is a flag indicating that the reset on the CSI2_96M_FCLK is
complete.
3.17
[ 43.399658] omap4-iss 5200.iss: REGISTER1 = 0x
[ 43.405456] omap4-iss 5200.iss: REGISTER1 = 0xe002e10e
4.1.4
[ 210.331909] omap4iss 5200.iss: REGISTER1 = 0x
[ 210.338470] omap4iss 5200.iss: REGISTER1 = 0xc002e10e
[ 210.342609] omap4iss 5200.iss: CSI2: CSI2_96M_FCLK reset timeout!
Note: the transition from 0x to 0xc002e10e would seem to
indicate that the operation completed, just not successfully...
I have spent the day sampling at different points in the code,
checking the contents of all the registers belonging to the ISS and
CSI PHY to conclude that there are no differences between the two
instances of the driver running on 3.17 and 4.1.4. Using the internal
__clk_is_enabled from clk-provider.h I also checked that the muxes
responsible for providing the clocks to the module were enabled
before, during and after the reset. I have also confirmed the
identical issue also occurs on a different board.
I suspect someone has broken something in the hwmods, or PRCM data
structures. Although I have not yet been able to find any relevant
differences in the source files that I have searched through.
Any suggestions regarding where I should continue to look for this
issue are welcome. Unfortunately if I can't get some support on this
soon, I will have to abandon working on this patch.
Michael Allwright
PhD Student
Paderborn Institute for Advanced Studies in Computer Science and Engineering
University of Paderborn
Office-number 02-10
Zukunftsmeile 1
33102 Paderborn
Germany
--
To unsubscribe from this list: send the line unsubscribe 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/3] [media] horus3a: don't use variable length arrays
The Linux stack is short; we need to be able to count the number
of bytes used at stack on each function. So, we don't like to
use variable-length arrays, as complained by smatch:
drivers/media/dvb-frontends/horus3a.c:57:19: warning: Variable length
array is used.
The max usecase of the driver seems to be 5 bytes + 1 for the
register.
So, let's be safe and allocate 6 bytes for the write buffer.
This should be enough to cover all cases. If not, let's print
an error message.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
---
drivers/media/dvb-frontends/horus3a.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb-frontends/horus3a.c
b/drivers/media/dvb-frontends/horus3a.c
index 46a82dc586d8..5074305b289e 100644
--- a/drivers/media/dvb-frontends/horus3a.c
+++ b/drivers/media/dvb-frontends/horus3a.c
@@ -26,6 +26,8 @@
#include horus3a.h
#include dvb_frontend.h
+#define MAX_WRITE_REGSIZE 5
+
enum horus3a_state {
STATE_UNKNOWN,
STATE_SLEEP,
@@ -54,16 +56,22 @@ static int horus3a_write_regs(struct horus3a_priv *priv,
u8 reg, const u8 *data, u32 len)
{
int ret;
- u8 buf[len+1];
+ u8 buf[MAX_WRITE_REGSIZE + 1];
struct i2c_msg msg[1] = {
{
.addr = priv-i2c_address,
.flags = 0,
- .len = sizeof(buf),
+ .len = len + 1,
.buf = buf,
}
};
+ if (len + 1 = sizeof(buf)) {
+ dev_warn(priv-i2c-dev,wr reg=%04x: len=%d is too big!\n,
+reg, len + 1);
+ return -E2BIG;
+ }
+
horus3a_i2c_debug(priv, reg, 1, data, len);
buf[0] = reg;
memcpy(buf[1], data, len);
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe 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] c8sectpfe: don't go past channel_data array
As reported by smatch:
drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c:365
find_channel() error: buffer overflow 'fei-channel_data' 8 = 63
It seems that a cut-and-paste type of error occurred here:
the channel_data array size is C8SECTPFE_MAX_TSIN_CHAN, and not
C8SECTPFE_MAXCHANNEL.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
index 3a9109356e67..955d8daf055f 100644
--- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
@@ -361,7 +361,7 @@ static struct channel_info *find_channel(struct c8sectpfei
*fei, int tsin_num)
{
int i;
- for (i = 0; i C8SECTPFE_MAXCHANNEL; i++) {
+ for (i = 0; i C8SECTPFE_MAX_TSIN_CHAN; i++) {
if (!fei-channel_data[i])
continue;
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe 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] c8sectpfe: don't go past channel_data array
As reported by smatch:
drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c:365
find_channel() error: buffer overflow 'fei-channel_data' 8 = 63
It seems that a cut-and-paste type of error occurred here:
the channel_data array size is C8SECTPFE_MAX_TSIN_CHAN, and not
C8SECTPFE_MAXCHANNEL.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
---
drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
index 3a9109356e67..955d8daf055f 100644
--- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
@@ -361,7 +361,7 @@ static struct channel_info *find_channel(struct c8sectpfei
*fei, int tsin_num)
{
int i;
- for (i = 0; i C8SECTPFE_MAXCHANNEL; i++) {
+ for (i = 0; i C8SECTPFE_MAX_TSIN_CHAN; i++) {
if (!fei-channel_data[i])
continue;
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe 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] c8sectpfe: Allow compiling it with COMPILE_TEST
While it won't work, it is good to allow it to build with COMPILE_TEST, as we can check if other patches would break compilation for this driver. Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com --- drivers/media/platform/sti/c8sectpfe/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/sti/c8sectpfe/Kconfig b/drivers/media/platform/sti/c8sectpfe/Kconfig index d1bfd4c646ae..a7227d2ab6cc 100644 --- a/drivers/media/platform/sti/c8sectpfe/Kconfig +++ b/drivers/media/platform/sti/c8sectpfe/Kconfig @@ -1,6 +1,6 @@ config DVB_C8SECTPFE tristate STMicroelectronics C8SECTPFE DVB support - depends on DVB_CORE I2C (ARCH_STI || ARCH_MULTIPLATFORM) + depends on DVB_CORE I2C (ARCH_STI || ARCH_MULTIPLATFORM || COMPILE_TEST) select LIBELF_32 select FW_LOADER select FW_LOADER_USER_HELPER_FALLBACK -- 2.4.3 -- To unsubscribe from this list: send the line unsubscribe 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/3] [media] cxd2841er: don't use variable length arrays
The Linux stack is short; we need to be able to count the number
of bytes used at stack on each function. So, we don't like to
use variable-length arrays, as complained by smatch:
drivers/media/dvb-frontends/cxd2841er.c:205:19: warning: Variable
length array is used.
The max usecase of the driver seems to be 15 bytes + 1 for the
register.
So, let's be safe and allocate 17 bytes for the write buffer.
This should be enough to cover all cases. If not, let's print
an error message.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
---
drivers/media/dvb-frontends/cxd2841er.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb-frontends/cxd2841er.c
b/drivers/media/dvb-frontends/cxd2841er.c
index 0d1a15109d1e..fdffb2f0ded8 100644
--- a/drivers/media/dvb-frontends/cxd2841er.c
+++ b/drivers/media/dvb-frontends/cxd2841er.c
@@ -33,6 +33,8 @@
#include cxd2841er.h
#include cxd2841er_priv.h
+#define MAX_WRITE_REGSIZE 16
+
enum cxd2841er_state {
STATE_SHUTDOWN = 0,
STATE_SLEEP_S,
@@ -202,18 +204,24 @@ static int cxd2841er_write_regs(struct cxd2841er_priv
*priv,
u8 addr, u8 reg, const u8 *data, u32 len)
{
int ret;
- u8 buf[len+1];
+ u8 buf[MAX_WRITE_REGSIZE + 1];
u8 i2c_addr = (addr == I2C_SLVX ?
priv-i2c_addr_slvx : priv-i2c_addr_slvt);
struct i2c_msg msg[1] = {
{
.addr = i2c_addr,
.flags = 0,
- .len = sizeof(buf),
+ .len = len + 1,
.buf = buf,
}
};
+ if (len + 1 = sizeof(buf)) {
+ dev_warn(priv-i2c-dev,wr reg=%04x: len=%d is too big!\n,
+reg, len + 1);
+ return -E2BIG;
+ }
+
cxd2841er_i2c_debug(priv, i2c_addr, reg, 1, data, len);
buf[0] = reg;
memcpy(buf[1], data, len);
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe 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/3] [media] ascot2e: don't use variable length arrays
The Linux stack is short; we need to be able to count the number
of bytes used at stack on each function. So, we don't like to
use variable-length arrays, as complained by smatch:
drivers/media/dvb-frontends/horus3a.c:57:19: warning: Variable length
array is used.
The max usecase of the driver seems to be 10 bytes + 1 for the
register.
So, let's be safe and allocate 11 bytes for the write buffer.
This should be enough to cover all cases. If not, let's print
an error message.
Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
---
drivers/media/dvb-frontends/ascot2e.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb-frontends/ascot2e.c
b/drivers/media/dvb-frontends/ascot2e.c
index ae7e463c2f9b..f770f6a2c987 100644
--- a/drivers/media/dvb-frontends/ascot2e.c
+++ b/drivers/media/dvb-frontends/ascot2e.c
@@ -26,6 +26,8 @@
#include ascot2e.h
#include dvb_frontend.h
+#define MAX_WRITE_REGSIZE 10
+
enum ascot2e_state {
STATE_UNKNOWN,
STATE_SLEEP,
@@ -120,16 +122,22 @@ static int ascot2e_write_regs(struct ascot2e_priv *priv,
u8 reg, const u8 *data, u32 len)
{
int ret;
- u8 buf[len+1];
+ u8 buf[MAX_WRITE_REGSIZE + 1];
struct i2c_msg msg[1] = {
{
.addr = priv-i2c_address,
.flags = 0,
- .len = sizeof(buf),
+ .len = len + 1,
.buf = buf,
}
};
+ if (len + 1 = sizeof(buf)) {
+ dev_warn(priv-i2c-dev,wr reg=%04x: len=%d is too big!\n,
+reg, len + 1);
+ return -E2BIG;
+ }
+
ascot2e_i2c_debug(priv, reg, 1, data, len);
buf[0] = reg;
memcpy(buf[1], data, len);
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe 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: VIMC: API proposal, configuring the topology through user space
Hello, thank you for your feedback On Tue, Aug 11, 2015 at 6:36 AM, Mauro Carvalho Chehab mche...@osg.samsung.com wrote: Em Tue, 11 Aug 2015 11:28:25 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: Hi Helen, On 08/10/15 19:21, Helen Fornazier wrote: Hi, thanks for your reviews On Mon, Aug 10, 2015 at 10:11 AM, Hans Verkuil hverk...@xs4all.nl wrote: Hi Helen! On 08/08/2015 03:55 AM, Helen Fornazier wrote: Hi! I've made a first sketch about the API of the vimc driver (virtual media controller) to configure the topology through user space. As first suggested by Laurent Pinchart, it is based on configfs. I would like to know you opinion about it, if you have any suggestion to improve it, otherwise I'll start its implementation as soon as possible. This API may change with the MC changes and I may see other possible configurations as I implementing it but here is the first idea of how the API will look like. vimc project link: https://github.com/helen-fornazier/opw-staging/ For more information: http://kernelnewbies.org/LaurentPinchart /*** The API: / In short, a topology like this one: http://goo.gl/Y7eUfu Would look like this filesystem tree: https://goo.gl/tCZPTg Txt version of the filesystem tree: https://goo.gl/42KX8Y * The /configfs/vimc subsystem I haven't checked the latest vimc driver, but doesn't it support multiple instances, just like vivid? I would certainly recommend that. Yes, it does support And if there are multiple instances, then each instance gets its own entry in configfs: /configs/vimc0, /configs/vimc1, etc. You are right, I'll add those The vimc driver registers a subsystem in the configfs with the following contents: ls /configfs/vimc build_topology status The build_topology attribute file will be used to tell the driver the configuration is done and it can build the topology internally echo -n anything here /configfs/vimc/build_topology Reading from the status attribute can have 3 different classes of outputs 1) deployed: the current configured tree is built 2) undeployed: no errors, the user has modified the configfs tree thus the topology was undeployed 3) error error_message: the topology configuration is wrong I don't see the status file in the filesystem tree links above. Sorry, I forgot to add I actually wonder if we can't use build_topology for that: reading it will just return the status. Yes we can, I was just wondering what should be the name of the file, as getting the status from reading the build_topology file doesn't seem much intuitive I'm not opposed to a status file, but it would be good to know what Laurent thinks. What happens if it is deployed and you want to 'undeploy' it? Instead of writing anything to build_topology it might be useful to write a real command to it. E.g. 'deploy', 'destroy'. What happens when you make changes while a topology is deployed? Should such changes be rejected until the usecount of the driver goes to 0, or will it only be rejected when you try to deploy the new configuration? I was thinking that if the user try changing the topology, or it will fail (because the device instance is in use) or it will undeploy the old topology (if the device is not in use). Then a 'destroy' command won't be useful, the user can just unload the driver when it won't be used anymore, Well, we're planning to add support for dynamic addition/removal of entities, interfaces, pads and links. So, instead of undeploy the old topology, one could just do: rm -rf part of the tree I think I misunderstood the word dynamic here. Do you mean that entities/interfaces/pads/link could be created/removed while their file handlers are opened? While an instance (lets say vimc2) is being used? If you have multiple vimc instances and you want to 'destroy' the topology of only one instance, then you can't rmmod the module. You can still use rm remove just one entire instance of the topology. Just to be clear: They way I was thinking was: the user do the mkdir/rmdir/echo/cat as he likes, if some file handler is opened in some device then rmdir/mkdir/echo would fail in the folder related to that device: Lets say we have /configfs/vimc/vimc0/ent/name /configfs/vimc/vimc1/ent/name /configfs/vimc/vimc1/ent_debayer/name if some file related to vimc0 is opened, then echo foo /configfs/vimc/vimc0/ent/name would fail. But echo foo /configfs/vimc/vimc1/ent/name would work (assuming we are not using the filehandlers of vimc1) If the user wants to remove vimc1 (as he is not using it anyway), then just: $ rmdir -r /configfs/vimc/vimc1 I don't see a big reason to explicitly undeploy a topology (without removing the folders) other then to modify the topology. Then when the user changes the filesystem tree, it undeploys the
