Re: [patch] [media] stv090x: remove indent levels
On Wed, Feb 19, 2014 at 10:52:32AM +0530, Manu Abraham wrote:
> On Tue, Feb 18, 2014 at 2:26 PM, Dan Carpenter
> wrote:
> > On Tue, Feb 18, 2014 at 09:25:36AM +0530, Manu Abraham wrote:
> >> Hi Dan,
> >>
> >> On Thu, Feb 6, 2014 at 2:58 PM, Dan Carpenter
> >> wrote:
> >> > 1) We can flip the "if (!lock)" check to "if (lock) return lock;" and
> >> >then remove a big chunk of indenting.
> >> > 2) There is a redundant "if (!lock)" which we can remove since we
> >> >already know that lock is zero. This removes another indent level.
> >>
> >>
> >> The stv090x driver is a mature, but slightly complex driver supporting
> >> quite some
> >> different configurations. Is it that some bug you are trying to fix in
> >> there ?
> >> I wouldn't prefer unnecessary code churn in such a driver for
> >> something as simple
> >> as gain in an indentation level.
> >
> > I thought the cleanup was jusitification enough, but the real reason I
> > wrote this patch is that testing:
> >
> > if (!lock) {
> > if (!lock) {
> >
> > sets off a static checker warning. That kind of code is puzzling and if
> > we don't clean it up then it wastes a lot of reviewer time.
> >
> > Also when you're reviewing these patches please consider that the
> > original code might be buggy and not simply messy. Perhaps something
> > other than "if (!lock) {" was intended?
> >
>
> I can't seem to find the possible bug in there..
>
> The code:
>
> lock = fn();
> if (!lock) {
> if (condition1) {
>lock = fn()
> } else {
>if (!lock) {
>}
> }
> }
>
> looks harmless to me, AFAICS.
Yes. I thought so too. It's just a messy, but not broken. Let's just
fix the static checker warning so that we don't have to keep reviewing
it every several months.
> Also, please do note that, if the
> function coldlock exits due to some reason for not finding valid symbols,
> stv090x_search is again fired up from the kernel frontend thread.
This sentence really scares me. Are you trying to say that the second
check on lock is valid for certain use cases? That is not possibly
true because it is a stack variable so (ignoring memory corruption)
it can't be modified from outside the code.
Hm...
The code actually looks like this:
lock = fn();
if (!lock) {
if (condition1) {
lock = fn()
} else {
if (!lock) {
while ((cur_step <= steps) && (!lock)) {
lock = stv090x_get_dmdlock(state, (timeout_dmd / 3));
}
}
}
}
Are you sure it's not buggy? Maybe the if statement should be moved
inside the while () condition?
> It is easy to make such cleanup patches and cause breakages, but a
> lot time consuming to fix such issues. My 2c.
Greg K-H and I review more of these cleanup patches than any other
kernel maintainers so I'm aware of the challenges. If you want to write
a smaller patch to fix the static checker warning then I will review it
for you. If you do that then please give me a:
Reported-by: Dan Carpenter
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v3,2/3] controls.xml : add addtional Flash fault bits
Hi Sakari.
Thank you for you comments.
Hi Daniel,
Thanks for the update.
Daniel Jeong wrote:
Add addtional falult bits for FLASH
V4L2_FLASH_FAULT_UNDER_VOLTAGE : UVLO
V4L2_FLASH_FAULT_INPUT_VOLTAGE : input voltage is adjusted by IVFM
V4L2_FLASH_FAULT_LED_OVER_TEMPERATURE : NTC Trip point is crossed.
Signed-off-by: Daniel Jeong
---
Documentation/DocBook/media/v4l/controls.xml | 16
1 file changed, 16 insertions(+)
diff --git a/Documentation/DocBook/media/v4l/controls.xml
b/Documentation/DocBook/media/v4l/controls.xml
index a5a3188..8121f7e 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -4370,6 +4370,22 @@ interface and may change in the future.
The flash controller has detected a short or open
circuit condition on the indicator LED.
+
+
V4L2_FLASH_FAULT_UNDER_VOLTAGE
+ Flash controller voltage to the flash LED
+ has been below the minimum limit specific to the flash
+ controller.
+
+
+
V4L2_FLASH_FAULT_INPUT_VOLTAGE
+ The flash controller has detected adjustment of input
+ voltage by Input Volage Flash Monitor(IVFM).
Volage -> Voltage; space before "(".
I still feel uncomfortable with the reference to the IVFM. That appears
clearely an implementation specific term.
You previously mentioned the flash current may be adjusted by the flash
controller. It should be mentioned here.
Is it possible to read the adjusted value from the chip?
Unfornatley it is NOT possible.
Usually thresholds can be selected,for example 2.9V, 3.0V, 3.1V, and 3.2V.
Chip adjusts the current value if the input voltage cross the thresholds.
We just read this flault flag from chip. So we need this.
I will describe more next patch.
+
+
+
V4L2_FLASH_FAULT_LED_OVER_TEMPERATURE
+ The flash controller has detected that TEMP input has
+ crossed NTC Trip Voltage.
Even if the NTC resistor might be the actual implementation, I wouldn't
refer to it here. There could be a real temperature sensor, for instance.
I will fix it.
+
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [media] stv090x: remove indent levels
On Tue, Feb 18, 2014 at 2:26 PM, Dan Carpenter wrote:
> On Tue, Feb 18, 2014 at 09:25:36AM +0530, Manu Abraham wrote:
>> Hi Dan,
>>
>> On Thu, Feb 6, 2014 at 2:58 PM, Dan Carpenter
>> wrote:
>> > 1) We can flip the "if (!lock)" check to "if (lock) return lock;" and
>> >then remove a big chunk of indenting.
>> > 2) There is a redundant "if (!lock)" which we can remove since we
>> >already know that lock is zero. This removes another indent level.
>>
>>
>> The stv090x driver is a mature, but slightly complex driver supporting
>> quite some
>> different configurations. Is it that some bug you are trying to fix in there
>> ?
>> I wouldn't prefer unnecessary code churn in such a driver for
>> something as simple
>> as gain in an indentation level.
>
> I thought the cleanup was jusitification enough, but the real reason I
> wrote this patch is that testing:
>
> if (!lock) {
> if (!lock) {
>
> sets off a static checker warning. That kind of code is puzzling and if
> we don't clean it up then it wastes a lot of reviewer time.
>
> Also when you're reviewing these patches please consider that the
> original code might be buggy and not simply messy. Perhaps something
> other than "if (!lock) {" was intended?
>
I can't seem to find the possible bug in there..
The code:
lock = fn();
if (!lock) {
if (condition1) {
lock = fn()
} else {
if (!lock) {
}
}
}
looks harmless to me, AFAICS. Also, please do note that, if the
function coldlock exits due to some reason for not finding valid symbols,
stv090x_search is again fired up from the kernel frontend thread.
It is easy to make such cleanup patches and cause breakages, but a
lot time consuming to fix such issues. My 2c.
Best Regards,
Manu
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Feb 19 04:00:35 CET 2014 git branch: test git hash: 37e59f876bc710d67a30b660826a5e83e07101ce gcc version:i686-linux-gcc (GCC) 4.8.2 sparse version: 0.4.5-rc1 host hardware: x86_64 host os:3.12-6.slh.2-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: WARNINGS linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.31.14-i686: WARNINGS linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12-i686: OK linux-3.13-i686: OK linux-3.14-rc1-i686: OK linux-2.6.31.14-x86_64: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12-x86_64: OK linux-3.13-x86_64: OK linux-3.14-rc1-x86_64: OK apps: OK spec-git: OK sparse version: 0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Some questions timeout in rc_dev
On 18/02/14 15:02, Sean Young wrote: On Sun, Feb 16, 2014 at 10:54:01PM +0100, Rune Petersen wrote: The intent of the timeout member in the rc_dev struct is a little unclear to me. In rc-core.h it is described as: @timeout: optional time after which device stops sending data. But if I look at the usage, it is used to detect idle in ir_raw.c which again is used by the RC-6 decoder to detect end of RC-6 6A transmissions. This leaves me with a few questions: - Without the timeout (which is optional) the RC-6 decoder will not work properly with RC-6 6A transmissions wouldn't that make it required? That sounds like a bug to me. The decoders shouldn't rely on the length of trailing space, probably it would be best to not rely on receiving the trailing space if possible. I can find no specs on RC-6 6A, but the length is supposed to be variable 8-128 bits and there doesn't appear to be any bits for length in the transmission. So the only way to detect the end is a space of 2667+us. - Why are the timeout set in the individual drivers so varied, shouldn't it depend on the encoding rather then the hardware used? The timeout set in the drivers ranges from 2750us(redrat3) to 100us(fintek_cir) and all the way to weird(streamzap) The various devices have different timeouts; they will stop sending IR data when there has been no activity for that amount of time. Thought experiment: Suppose the ir_raw_store_with_filter() code has a timeout _longer_ than this device timeout, it will never enter the idle state. Suppose the ir_raw_store_with_filter() code has a timeout _shorter_ than this device timeout, it will enter the idle state, just faster. Wouldn't this mean that the device timeout cannot be exceeded which I thought was the purpose of max_timeout in dev_rc? - Why is the timeout value controlled by the IR driver, when it us only usedby the rc-core. Wouldn't it make sense to have the timeout initialized to a sane value in a single place? I guess the rc_dev->timeout is used for different things: 1) So that the drivers can advertise what timeout the hardware uses 2) The ttusbir and iguanair are devices which never timeout, so they rely on ir_raw_store_with_filter() to do timeout handling for them. Some drivers have both hardware timeouts and use ir_raw_store_with_filter() so timeout handling is done both in hardware and software. I would like to get rc to a state where it just works for me without modifications, I "just" need to know which changes I can get away without breaking it for everybody else =) As things are right now the RC input feel very sluggish and unresponsive using a RC-6 6A remote and a ite-cir receiver. Sean Rune -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] media: soc-camera: OF cameras
On Wed, Feb 12, 2014 at 12:05 PM, Bryan Wu wrote:
> From: Guennadi Liakhovetski
>
> With OF we aren't getting platform data any more. To minimise changes we
> create all the missing data ourselves, including compulsory struct
> soc_camera_link objects. Host-client linking is now done, based on the OF
> data. Media bus numbers also have to be assigned dynamically.
>
> OF probing reuses the V4L2 Async API which is used by async non-OF probing.
>
> Signed-off-by: Guennadi Liakhovetski
Hi Guennadi,
Do you have chance to review my v2 patch?
Thanks,
-Bryan
> Signed-off-by: Bryan Wu
> ---
> v2:
> - move to use V4L2 Async API
> - cleanup some coding style issue
> - allocate struct soc_camera_desc sdesc on stack
> - cleanup unbalanced mutex operation
>
> drivers/media/platform/soc_camera/soc_camera.c | 232
> -
> include/media/soc_camera.h | 5 +
> 2 files changed, 233 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/soc_camera/soc_camera.c
> b/drivers/media/platform/soc_camera/soc_camera.c
> index 4b8c024..ffe1254 100644
> --- a/drivers/media/platform/soc_camera/soc_camera.c
> +++ b/drivers/media/platform/soc_camera/soc_camera.c
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -36,6 +37,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -49,6 +51,7 @@
> vb2_is_streaming(&(icd)->vb2_vidq))
>
> #define MAP_MAX_NUM 32
> +static DECLARE_BITMAP(host_map, MAP_MAX_NUM);
> static DECLARE_BITMAP(device_map, MAP_MAX_NUM);
> static LIST_HEAD(hosts);
> static LIST_HEAD(devices);
> @@ -65,6 +68,17 @@ struct soc_camera_async_client {
> struct list_head list; /* needed for clean up */
> };
>
> +struct soc_camera_of_client {
> + struct soc_camera_desc *sdesc;
> + struct soc_camera_async_client sasc;
> + struct v4l2_of_endpoint node;
> + struct dev_archdata archdata;
> + struct device_node *link_node;
> + union {
> + struct i2c_board_info i2c_info;
> + };
> +};
> +
> static int soc_camera_video_start(struct soc_camera_device *icd);
> static int video_dev_create(struct soc_camera_device *icd);
>
> @@ -1336,6 +1350,10 @@ static int soc_camera_i2c_init(struct
> soc_camera_device *icd,
> return -EPROBE_DEFER;
> }
>
> + /* OF probing skips following I2C init */
> + if (shd->host_wait)
> + return 0;
> +
> ici = to_soc_camera_host(icd->parent);
> adap = i2c_get_adapter(shd->i2c_adapter_id);
> if (!adap) {
> @@ -1573,10 +1591,180 @@ static void scan_async_host(struct soc_camera_host
> *ici)
> asd += ici->asd_sizes[j];
> }
> }
> +
> +static void soc_camera_of_i2c_ifill(struct soc_camera_of_client *sofc,
> + struct i2c_client *client)
> +{
> + struct i2c_board_info *info = &sofc->i2c_info;
> + struct soc_camera_desc *sdesc = sofc->sdesc;
> +
> + /* on OF I2C devices platform_data == NULL */
> + info->flags = client->flags;
> + info->addr = client->addr;
> + info->irq = client->irq;
> + info->archdata = &sofc->archdata;
> +
> + /* archdata is always empty on OF I2C devices */
> + strlcpy(info->type, client->name, sizeof(info->type));
> +
> + sdesc->host_desc.i2c_adapter_id = client->adapter->nr;
> +}
> +
> +static void soc_camera_of_i2c_info(struct device_node *node,
> + struct soc_camera_of_client *sofc)
> +{
> + struct i2c_client *client;
> + struct soc_camera_desc *sdesc = sofc->sdesc;
> + struct i2c_board_info *info = &sofc->i2c_info;
> + struct device_node *port, *sensor, *bus;
> +
> + port = v4l2_of_get_remote_port(node);
> + if (!port)
> + return;
> +
> + /* Check the bus */
> + sensor = of_get_parent(port);
> + bus = of_get_parent(sensor);
> +
> + if (of_node_cmp(bus->name, "i2c")) {
> + of_node_put(port);
> + of_node_put(sensor);
> + of_node_put(bus);
> + return;
> + }
> +
> + info->of_node = sensor;
> + sdesc->host_desc.board_info = info;
> +
> + client = of_find_i2c_device_by_node(sensor);
> + /*
> +* of_i2c_register_devices() took a reference to the OF node, it is
> not
> +* dropped, when the I2C device is removed, so, we don't need an
> +* additional reference.
> +*/
> + of_node_put(sensor);
> + if (client) {
> + soc_camera_of_i2c_ifill(sofc, client);
> + sofc->link_node = sensor;
> + put_device(&client->dev);
> + }
> +
> + /* client hasn't attached to I2C yet */
> +}
> +
> +static struct soc_camera_of_client *soc_camera_of_alloc_client(const struct
> soc_camera_host *ici,
> +
Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
On Tue, 18 Feb 2014 08:06:24 +0100, Sascha Hauer wrote: > Hi Grant, > > On Mon, Feb 17, 2014 at 06:14:51PM +, Grant Likely wrote: > > On Tue, 11 Feb 2014 07:56:33 -0600, Rob Herring > > wrote: > > > On Tue, Feb 11, 2014 at 5:45 AM, Philipp Zabel > > > wrote: > > > > From: Philipp Zabel > > > > > > > > This patch moves the parsing helpers used to parse connected graphs > > > > in the device tree, like the video interface bindings documented in > > > > Documentation/devicetree/bindings/media/video-interfaces.txt, from > > > > drivers/media/v4l2-core to drivers/of. > > > > > > This is the opposite direction things have been moving... > > > > > > > This allows to reuse the same parser code from outside the V4L2 > > > > framework, > > > > most importantly from display drivers. There have been patches that > > > > duplicate > > > > the code (and I am going to send one of my own), such as > > > > http://lists.freedesktop.org/archives/dri-devel/2013-August/043308.html > > > > and others that parse the same binding in a different way: > > > > https://www.mail-archive.com/linux-omap@vger.kernel.org/msg100761.html > > > > > > > > I think that all common video interface parsing helpers should be moved > > > > to a > > > > single place, outside of the specific subsystems, so that it can be > > > > reused > > > > by all drivers. > > > > > > Perhaps that should be done rather than moving to drivers/of now and > > > then again to somewhere else. > > > > This is just parsing helpers though, isn't it? I have no problem pulling > > helper functions into drivers/of if they are usable by multiple > > subsystems. I don't really understand the model being used though. I > > would appreciate a description of the usage model for these functions > > for poor folks like me who can't keep track of what is going on in > > subsystems. > > You can find it under > Documentation/devicetree/bindings/media/video-interfaces.txt Okay, I think I'm okay with moving the helpers, but I will make one requirement. I would like to have a short binding document describing the pattern being used. The document above talks a lot about video specific issues, but the helpers appear to be specifically about the tree walking properties of the API. g. -- To unsubscribe from this list: send the line "unsubscribe 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] V4L: s5k6a3: Add DT binding documentation
On 22/12/13 22:27, Sylwester Nawrocki wrote: > This patch adds DT binding documentation for the Samsung S5K6A3(YX) > raw image sensor. > > Signed-off-by: Sylwester Nawrocki > Signed-off-by: Kyungmin Park > --- > This patch adds missing documentation [1] for the "samsung,s5k6a3" > compatible. Rob, can you please merge it through your tree if it > looks OK ? Anyone cares to Ack this patch so it can be merged through the media tree ? > Thanks, > Sylwester > > [1] http://www.spinics.net/lists/devicetree/msg10693.html > > Changes since v3 (https://linuxtv.org/patch/20429): > - rephrased 'clocks' and 'clock-names' properties' description. > --- > .../devicetree/bindings/media/samsung-s5k6a3.txt | 33 > > 1 files changed, 33 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k6a3.txt > > diff --git a/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt > b/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt > new file mode 100644 > index 000..cce01e8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt > @@ -0,0 +1,33 @@ > +Samsung S5K6A3(YX) raw image sensor > +- > + > +S5K6A3(YX) is a raw image sensor with MIPI CSI-2 and CCP2 image data > interfaces > +and CCI (I2C compatible) control bus. > + > +Required properties: > + > +- compatible : "samsung,s5k6a3"; > +- reg: I2C slave address of the sensor; > +- svdda-supply : core voltage supply; > +- svddio-supply : I/O voltage supply; > +- afvdd-supply : AF (actuator) voltage supply; > +- gpios : specifier of a GPIO connected to the RESET pin; > +- clocks : should contain list of phandle and clock specifier pairs > + according to common clock bindings for the clocks described > + in the clock-names property; > +- clock-names: should contain "extclk" entry for the sensor's EXTCLK > clock; > + > +Optional properties: > + > +- clock-frequency : the frequency at which the "extclk" clock should be > + configured to operate, in Hz; if this property is not > + specified default 24 MHz value will be used. > + > +The common video interfaces bindings (see video-interfaces.txt) should be > +used to specify link to the image data receiver. The S5K6A3(YX) device > +node should contain one 'port' child node with an 'endpoint' subnode. > + > +Following properties are valid for the endpoint node: > + > +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in > + video-interfaces.txt. The sensor supports only one data lane. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] uvcvideo: Enable VIDIOC_CREATE_BUFS
From: Philipp Zabel
This patch enables the ioctl to create additional buffers on the
videobuf2 capture queue.
Signed-off-by: Philipp Zabel
[laurent.pinch...@ideasonboard.com: Acquire privileges instead of just
checking them in VIDIOC_CREATE_BUFS implementation]
Signed-off-by: Laurent Pinchart
---
drivers/media/usb/uvc/uvc_queue.c | 12
drivers/media/usb/uvc/uvc_v4l2.c | 11 +++
drivers/media/usb/uvc/uvcvideo.h | 2 ++
3 files changed, 25 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index d46dd70..ff7be97 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -198,6 +198,18 @@ int uvc_query_buffer(struct uvc_video_queue *queue, struct
v4l2_buffer *buf)
return ret;
}
+int uvc_create_buffers(struct uvc_video_queue *queue,
+ struct v4l2_create_buffers *cb)
+{
+ int ret;
+
+ mutex_lock(&queue->mutex);
+ ret = vb2_create_bufs(&queue->queue, cb);
+ mutex_unlock(&queue->mutex);
+
+ return ret;
+}
+
int uvc_queue_buffer(struct uvc_video_queue *queue, struct v4l2_buffer *buf)
{
int ret;
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 3afff92..378ae02 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1000,6 +1000,17 @@ static long uvc_v4l2_do_ioctl(struct file *file,
unsigned int cmd, void *arg)
return uvc_query_buffer(&stream->queue, buf);
}
+ case VIDIOC_CREATE_BUFS:
+ {
+ struct v4l2_create_buffers *cb = arg;
+
+ ret = uvc_acquire_privileges(handle);
+ if (ret < 0)
+ return ret;
+
+ return uvc_create_buffers(&stream->queue, cb);
+ }
+
case VIDIOC_QBUF:
if (!uvc_has_privileges(handle))
return -EBUSY;
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 6173632..143d5e5 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -614,6 +614,8 @@ extern int uvc_alloc_buffers(struct uvc_video_queue *queue,
extern void uvc_free_buffers(struct uvc_video_queue *queue);
extern int uvc_query_buffer(struct uvc_video_queue *queue,
struct v4l2_buffer *v4l2_buf);
+extern int uvc_create_buffers(struct uvc_video_queue *queue,
+ struct v4l2_create_buffers *v4l2_cb);
extern int uvc_queue_buffer(struct uvc_video_queue *queue,
struct v4l2_buffer *v4l2_buf);
extern int uvc_dequeue_buffer(struct uvc_video_queue *queue,
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] uvcvideo: Support allocating buffers larger than the current frame size
The queue_setup handler takes an optional format argument that can be used to allocate buffers for a format different than the current format. The uvcvideo driver doesn't support changing the format when buffers have been allocated, but there's no reason not to support allocating buffers larger than the minimum size. When the format argument isn't NULL verify that the requested image size is large enough for the current format and use it for the buffer size. Signed-off-by: Laurent Pinchart Tested-by: Philipp Zabel --- drivers/media/usb/uvc/uvc_queue.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c index 254bc34..d46dd70 100644 --- a/drivers/media/usb/uvc/uvc_queue.c +++ b/drivers/media/usb/uvc/uvc_queue.c @@ -48,9 +48,14 @@ static int uvc_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt, struct uvc_streaming *stream = container_of(queue, struct uvc_streaming, queue); + /* Make sure the image size is large enough. */ + if (fmt && fmt->fmt.pix.sizeimage < stream->ctrl.dwMaxVideoFrameSize) + return -EINVAL; + *nplanes = 1; - sizes[0] = stream->ctrl.dwMaxVideoFrameSize; + sizes[0] = fmt ? fmt->fmt.pix.sizeimage +: stream->ctrl.dwMaxVideoFrameSize; return 0; } -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] uvcvideo: Remove duplicate check for number of buffers in queue_setup
videobuf2 already ensures that the number of buffers will not exceed VIDEO_MAX_FRAME, which is equal to our arbitraty limit of UVC_MAX_VIDEO_BUFFERS. Remove the duplicate check. Signed-off-by: Laurent Pinchart Tested-by: Philipp Zabel --- drivers/media/usb/uvc/uvc_queue.c | 3 --- drivers/media/usb/uvc/uvcvideo.h | 2 -- 2 files changed, 5 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c index cd962be..254bc34 100644 --- a/drivers/media/usb/uvc/uvc_queue.c +++ b/drivers/media/usb/uvc/uvc_queue.c @@ -48,9 +48,6 @@ static int uvc_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt, struct uvc_streaming *stream = container_of(queue, struct uvc_streaming, queue); - if (*nbuffers > UVC_MAX_VIDEO_BUFFERS) - *nbuffers = UVC_MAX_VIDEO_BUFFERS; - *nplanes = 1; sizes[0] = stream->ctrl.dwMaxVideoFrameSize; diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 9e35982..6173632 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -115,8 +115,6 @@ #define UVC_URBS 5 /* Maximum number of packets per URB. */ #define UVC_MAX_PACKETS32 -/* Maximum number of video buffers. */ -#define UVC_MAX_VIDEO_BUFFERS 32 /* Maximum status buffer size in bytes of interrupt URB. */ #define UVC_MAX_STATUS_SIZE16 -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/3] uvcvideo VIDIOC_CREATE_BUFS support
Hello, Here's the second version of the VIDIOC_CREATE_BUFS support for uvcvideo patch set. Compared to v1, patch 3/3 acquires privileges instead of merely checking for them. Now the driver passes the VIDIOC_CREATE_BUFS v4l2-compliance test. Laurent Pinchart (2): uvcvideo: Remove duplicate check for number of buffers in queue_setup uvcvideo: Support allocating buffers larger than the current frame size Philipp Zabel (1): uvcvideo: Enable VIDIOC_CREATE_BUFS drivers/media/usb/uvc/uvc_queue.c | 20 +--- drivers/media/usb/uvc/uvc_v4l2.c | 11 +++ drivers/media/usb/uvc/uvcvideo.h | 4 ++-- 3 files changed, 30 insertions(+), 5 deletions(-) -- 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
easyCap on 12.04
Hy guys, there's someone can help me? I need to get stream by easyCap 12.04. With lsusb i get Bus 003 Device 003: ID 1b71:3002 Now, what i do? tnks to all! =) -- Andrea Rossi -- To unsubscribe from this list: send the line "unsubscribe 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] gspca_stv06xx: remove an unneeded check
"err" is zero here so we don't need to check again. Signed-off-by: Dan Carpenter diff --git a/drivers/media/usb/gspca/stv06xx/stv06xx_vv6410.c b/drivers/media/usb/gspca/stv06xx/stv06xx_vv6410.c index bf3e5c317a26..e60cbb3aa609 100644 --- a/drivers/media/usb/gspca/stv06xx/stv06xx_vv6410.c +++ b/drivers/media/usb/gspca/stv06xx/stv06xx_vv6410.c @@ -178,7 +178,7 @@ static int vv6410_stop(struct sd *sd) PDEBUG(D_STREAM, "Halting stream"); - return (err < 0) ? err : 0; + return 0; } static int vv6410_dump(struct sd *sd) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] ARM: omap2: cm-t35: Add regulators and clock for camera sensor
Hi Igor,
On Tuesday 18 February 2014 16:03:44 Igor Grinberg wrote:
> On 02/18/14 14:47, Laurent Pinchart wrote:
> > On Monday 10 February 2014 22:54:40 Laurent Pinchart wrote:
> >> The camera sensor will soon require regulators and clocks. Register
> >> fixed regulators for its VAA and VDD power supplies and a fixed rate
> >> clock for its master clock.
> >
> > This patch is a prerequisite for a set of 4 patches that need to go
> > through the linux-media tree. It would simpler if it could go through the
> > same tree as well. Given that arch/arm/mach-omap2/board-cm-t35.c has seen
> > very little activity recently I believe the risk of conflict is pretty
> > low.
>
> Indeed, as we work on DT stuff of cm-t35/3730 and pretty much stopped
> updating the board-cm-t35.c file.
DT support for the OMAP3 ISP is coming. Too slowly, but it's coming :-)
> > Tony, would
> > that be fine with you ?
> >
> >> Signed-off-by: Laurent Pinchart
>
> Acked-by: Igor Grinberg
Thank you. Tony, could I get your ack as well to push this through Mauro's
tree ?
> >> ---
> >>
> >> arch/arm/mach-omap2/board-cm-t35.c | 16
> >> 1 file changed, 16 insertions(+)
> >>
> >> diff --git a/arch/arm/mach-omap2/board-cm-t35.c
> >> b/arch/arm/mach-omap2/board-cm-t35.c index 8dd0ec8..018353d 100644
> >> --- a/arch/arm/mach-omap2/board-cm-t35.c
> >> +++ b/arch/arm/mach-omap2/board-cm-t35.c
> >> @@ -16,6 +16,8 @@
> >>
> >> *
> >> */
> >>
> >> +#include
> >> +#include
> >>
> >> #include
> >> #include
> >> #include
> >>
> >> @@ -542,8 +544,22 @@ static struct isp_platform_data cm_t35_isp_pdata = {
> >>
> >>.subdevs = cm_t35_isp_subdevs,
> >>
> >> };
> >>
> >> +static struct regulator_consumer_supply cm_t35_camera_supplies[] = {
> >> + REGULATOR_SUPPLY("vaa", "3-005d"),
> >> + REGULATOR_SUPPLY("vdd", "3-005d"),
> >> +};
> >> +
> >>
> >> static void __init cm_t35_init_camera(void)
> >> {
> >>
> >> + struct clk *clk;
> >> +
> >> + clk = clk_register_fixed_rate(NULL, "mt9t001-clkin", NULL,
CLK_IS_ROOT,
> >> +4800);
> >> + clk_register_clkdev(clk, NULL, "3-005d");
> >> +
> >> + regulator_register_fixed(2, cm_t35_camera_supplies,
> >> + ARRAY_SIZE(cm_t35_camera_supplies));
> >> +
> >>
> >>if (omap3_init_camera(&cm_t35_isp_pdata) < 0)
> >>
> >>pr_warn("CM-T3x: Failed registering camera device!\n");
> >>
> >> }
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] uvcvideo VIDIOC_CREATE_BUFS support
Hi Laurent, Am Dienstag, den 18.02.2014, 15:27 +0100 schrieb Laurent Pinchart: > Hi Philipp, > > Here's a patch set that enables VIDIOC_CREATE_BUFS support in the uvcvideo > driver. It's based on the patch you've submitted (3/3), with two additional > cleanup patches to simplify the queue_setup implementation and supporting > allocation of buffers larger than the current frame size. > > As you've submitted patch 3/3 I assume you have a use case, could you then > please test the patch set to make sure 1/3 and 2/3 don't break anything ? Tested-by: Philipp Zabel Using a Logitech C920 UVC camera connected to i.MX6 SabreLite running GStreamer 1.2.3: gst-launch-1.0 v4l2src device=/dev/video8 \ ! image/jpeg,width=1280,height=720 ! queue max-size-buffers=5 min-threshold-buffers=5 \ ! rtpjpegpay ! udpsink host=... port=... regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pinnacle PCTV 340e no longer working in kernel 3.11
This USB stick was working with 3.8, now with 3.11 the kernel loads the driver correctly but you can't tune any channels (tried with kaffeine, tv-me and VLC) I've loaded the xc4000 driver enabling verbose output and no error messages appear during scanning, merely it does not detect any signal. I haven't tried any other kernel than these, however. Anyone had any luck making it work with the 3.11 kernel? Thanks -- To unsubscribe from this list: send the line "unsubscribe 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] Added bayer 8-bit patterns to uvcvideo
Hi Edgar,
Thank you for the patch.
On Monday 20 January 2014 09:11:28 Edgar Thier wrote:
> Add bayer 8-bit GUIDs to uvcvideo and
> associate them with the corresponding V4L2 pixel formats.
>
> Signed-off-by: Edgar Thier
Acked-by: Laurent Pinchart
Out of curiosity, could you please send me the lsusb -v output of the camera
you need this for ?
> ---
> drivers/media/usb/uvc/uvc_driver.c | 22 +-
> drivers/media/usb/uvc/uvcvideo.h | 12
> 2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c
> index c3bb250..84da426 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -108,11 +108,31 @@ static struct uvc_format_desc uvc_fmts[] = {
> .fcc= V4L2_PIX_FMT_Y16,
> },
> {
> -.name= "RGB Bayer",
> +.name= "RGB Bayer (bggr)",
> .guid= UVC_GUID_FORMAT_BY8,
> .fcc= V4L2_PIX_FMT_SBGGR8,
It looks like your mailer has corrupted the patch by replacing tabs with
spaces. Could you please fix that and resubmit ? While you're at it, could you
please prefix the commit subject with "uvcvideo: " ?
> },
> {
> +.name= "RGB Bayer (bggr)",
> +.guid= UVC_GUID_FORMAT_BY8_BA81,
> +.fcc= V4L2_PIX_FMT_SBGGR8,
> +},
> +{
> +.name= "RGB Bayer (grbg)",
> +.guid= UVC_GUID_FORMAT_BY8_GRBG,
> +.fcc= V4L2_PIX_FMT_SGRBG8,
> +},
> +{
> +.name= "RGB Bayer (gbrg)",
> +.guid= UVC_GUID_FORMAT_BY8_GBRG,
> +.fcc= V4L2_PIX_FMT_SGBRG8,
> +},
> +{
> +.name= "RGB Bayer (rggb)",
> +.guid= UVC_GUID_FORMAT_BY8_RGGB,
> +.fcc= V4L2_PIX_FMT_SRGGB8,
> +},
> +{
> .name= "RGB565",
> .guid= UVC_GUID_FORMAT_RGBP,
> .fcc= V4L2_PIX_FMT_RGB565,
> diff --git a/drivers/media/usb/uvc/uvcvideo.h
> b/drivers/media/usb/uvc/uvcvideo.h index 9e35982..57357d9 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -94,6 +94,18 @@
> #define UVC_GUID_FORMAT_BY8 \
> { 'B', 'Y', '8', ' ', 0x00, 0x00, 0x10, 0x00, \
> 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_BY8_BA81 \
> +{ 'B', 'A', '8', '1', 0x00, 0x00, 0x10, 0x00, \
> + 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_BY8_GRBG \
> +{ 'G', 'R', 'B', 'G', 0x00, 0x00, 0x10, 0x00, \
> + 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_BY8_GBRG \
> +{ 'G', 'B', 'R', 'G', 0x00, 0x00, 0x10, 0x00, \
> + 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> +#define UVC_GUID_FORMAT_BY8_RGGB \
> +{ 'R', 'G', 'G', 'B', 0x00, 0x00, 0x10, 0x00, \
> + 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> #define UVC_GUID_FORMAT_RGBP \
> { 'R', 'G', 'B', 'P', 0x00, 0x00, 0x10, 0x00, \
> 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: davinci_vpfe: fix error check
Hi Prabhakar,
(Removing Greg, Mauro and the de...@driverdev.osuosl.org list to avoid
spamming them)
On Saturday 15 February 2014 09:16:19 Josh Triplett wrote:
> On Sat, Feb 15, 2014 at 11:17:11AM +0100, Levente Kurusa wrote:
> > The check would check the pointer, which is never less than 0.
> > According to the error message, the correct check would be
> > to check the return value of ipipe_mode. Check that instead.
> >
> > Reported-by: David Binderman
> > Signed-off-by: Levente Kurusa
>
> Reviewed-by: Josh Triplett
Could you please handle this patch ?
> > drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> > b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c index
> > 2d36b60..b2daf5e 100644
> > --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> > +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> > @@ -267,7 +267,7 @@ int config_ipipe_hw(struct vpfe_ipipe_device *ipipe)
> >
> > }
> >
> > ipipe_mode = get_ipipe_mode(ipipe);
> > - if (ipipe < 0) {
> > + if (ipipe_mode < 0) {
> > pr_err("Failed to get ipipe mode");
> > return -EINVAL;
> > }
--
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/3] uvcvideo: Enable VIDIOC_CREATE_BUFS
From: Philipp Zabel
This patch enables the ioctl to create additional buffers on the
videobuf2 capture queue.
Signed-off-by: Philipp Zabel
Signed-off-by: Laurent Pinchart
---
drivers/media/usb/uvc/uvc_queue.c | 12
drivers/media/usb/uvc/uvc_v4l2.c | 10 ++
drivers/media/usb/uvc/uvcvideo.h | 2 ++
3 files changed, 24 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index d46dd70..ff7be97 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -198,6 +198,18 @@ int uvc_query_buffer(struct uvc_video_queue *queue, struct
v4l2_buffer *buf)
return ret;
}
+int uvc_create_buffers(struct uvc_video_queue *queue,
+ struct v4l2_create_buffers *cb)
+{
+ int ret;
+
+ mutex_lock(&queue->mutex);
+ ret = vb2_create_bufs(&queue->queue, cb);
+ mutex_unlock(&queue->mutex);
+
+ return ret;
+}
+
int uvc_queue_buffer(struct uvc_video_queue *queue, struct v4l2_buffer *buf)
{
int ret;
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 3afff92..fa58131 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1000,6 +1000,16 @@ static long uvc_v4l2_do_ioctl(struct file *file,
unsigned int cmd, void *arg)
return uvc_query_buffer(&stream->queue, buf);
}
+ case VIDIOC_CREATE_BUFS:
+ {
+ struct v4l2_create_buffers *cb = arg;
+
+ if (!uvc_has_privileges(handle))
+ return -EBUSY;
+
+ return uvc_create_buffers(&stream->queue, cb);
+ }
+
case VIDIOC_QBUF:
if (!uvc_has_privileges(handle))
return -EBUSY;
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 6173632..143d5e5 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -614,6 +614,8 @@ extern int uvc_alloc_buffers(struct uvc_video_queue *queue,
extern void uvc_free_buffers(struct uvc_video_queue *queue);
extern int uvc_query_buffer(struct uvc_video_queue *queue,
struct v4l2_buffer *v4l2_buf);
+extern int uvc_create_buffers(struct uvc_video_queue *queue,
+ struct v4l2_create_buffers *v4l2_cb);
extern int uvc_queue_buffer(struct uvc_video_queue *queue,
struct v4l2_buffer *v4l2_buf);
extern int uvc_dequeue_buffer(struct uvc_video_queue *queue,
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] uvcvideo: Support allocating buffers larger than the current frame size
The queue_setup handler takes an optional format argument that can be used to allocate buffers for a format different than the current format. The uvcvideo driver doesn't support changing the format when buffers have been allocated, but there's no reason not to support allocating buffers larger than the minimum size. When the format argument isn't NULL verify that the requested image size is large enough for the current format and use it for the buffer size. Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_queue.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c index 254bc34..d46dd70 100644 --- a/drivers/media/usb/uvc/uvc_queue.c +++ b/drivers/media/usb/uvc/uvc_queue.c @@ -48,9 +48,14 @@ static int uvc_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt, struct uvc_streaming *stream = container_of(queue, struct uvc_streaming, queue); + /* Make sure the image size is large enough. */ + if (fmt && fmt->fmt.pix.sizeimage < stream->ctrl.dwMaxVideoFrameSize) + return -EINVAL; + *nplanes = 1; - sizes[0] = stream->ctrl.dwMaxVideoFrameSize; + sizes[0] = fmt ? fmt->fmt.pix.sizeimage +: stream->ctrl.dwMaxVideoFrameSize; return 0; } -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] uvcvideo VIDIOC_CREATE_BUFS support
Hi Philipp, Here's a patch set that enables VIDIOC_CREATE_BUFS support in the uvcvideo driver. It's based on the patch you've submitted (3/3), with two additional cleanup patches to simplify the queue_setup implementation and supporting allocation of buffers larger than the current frame size. As you've submitted patch 3/3 I assume you have a use case, could you then please test the patch set to make sure 1/3 and 2/3 don't break anything ? Laurent Pinchart (2): uvcvideo: Remove duplicate check for number of buffers in queue_setup uvcvideo: Support allocating buffers larger than the current frame size Philipp Zabel (1): uvcvideo: Enable VIDIOC_CREATE_BUFS drivers/media/usb/uvc/uvc_queue.c | 20 +--- drivers/media/usb/uvc/uvc_v4l2.c | 10 ++ drivers/media/usb/uvc/uvcvideo.h | 4 ++-- 3 files changed, 29 insertions(+), 5 deletions(-) -- 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 1/3] uvcvideo: Remove duplicate check for number of buffers in queue_setup
videobuf2 already ensures that the number of buffers will not exceed VIDEO_MAX_FRAME, which is equal to our arbitraty limit of UVC_MAX_VIDEO_BUFFERS. Remove the duplicate check. Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_queue.c | 3 --- drivers/media/usb/uvc/uvcvideo.h | 2 -- 2 files changed, 5 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c index cd962be..254bc34 100644 --- a/drivers/media/usb/uvc/uvc_queue.c +++ b/drivers/media/usb/uvc/uvc_queue.c @@ -48,9 +48,6 @@ static int uvc_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt, struct uvc_streaming *stream = container_of(queue, struct uvc_streaming, queue); - if (*nbuffers > UVC_MAX_VIDEO_BUFFERS) - *nbuffers = UVC_MAX_VIDEO_BUFFERS; - *nplanes = 1; sizes[0] = stream->ctrl.dwMaxVideoFrameSize; diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 9e35982..6173632 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -115,8 +115,6 @@ #define UVC_URBS 5 /* Maximum number of packets per URB. */ #define UVC_MAX_PACKETS32 -/* Maximum number of video buffers. */ -#define UVC_MAX_VIDEO_BUFFERS 32 /* Maximum status buffer size in bytes of interrupt URB. */ #define UVC_MAX_STATUS_SIZE16 -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] ARM: omap2: cm-t35: Add regulators and clock for camera sensor
Hi Laurent,
On 02/18/14 14:47, Laurent Pinchart wrote:
> Mauro, Tony,
>
> On Monday 10 February 2014 22:54:40 Laurent Pinchart wrote:
>> The camera sensor will soon require regulators and clocks. Register
>> fixed regulators for its VAA and VDD power supplies and a fixed rate
>> clock for its master clock.
>
> This patch is a prerequisite for a set of 4 patches that need to go through
> the linux-media tree. It would simpler if it could go through the same tree
> as
> well. Given that arch/arm/mach-omap2/board-cm-t35.c has seen very little
> activity recently I believe the risk of conflict is pretty low.
Indeed, as we work on DT stuff of cm-t35/3730 and pretty much stopped
updating the board-cm-t35.c file.
> Tony, would
> that be fine with you ?
>
>> Signed-off-by: Laurent Pinchart
Acked-by: Igor Grinberg
>> ---
>> arch/arm/mach-omap2/board-cm-t35.c | 16
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/arch/arm/mach-omap2/board-cm-t35.c
>> b/arch/arm/mach-omap2/board-cm-t35.c index 8dd0ec8..018353d 100644
>> --- a/arch/arm/mach-omap2/board-cm-t35.c
>> +++ b/arch/arm/mach-omap2/board-cm-t35.c
>> @@ -16,6 +16,8 @@
>> *
>> */
>>
>> +#include
>> +#include
>> #include
>> #include
>> #include
>> @@ -542,8 +544,22 @@ static struct isp_platform_data cm_t35_isp_pdata = {
>> .subdevs = cm_t35_isp_subdevs,
>> };
>>
>> +static struct regulator_consumer_supply cm_t35_camera_supplies[] = {
>> +REGULATOR_SUPPLY("vaa", "3-005d"),
>> +REGULATOR_SUPPLY("vdd", "3-005d"),
>> +};
>> +
>> static void __init cm_t35_init_camera(void)
>> {
>> +struct clk *clk;
>> +
>> +clk = clk_register_fixed_rate(NULL, "mt9t001-clkin", NULL, CLK_IS_ROOT,
>> + 4800);
>> +clk_register_clkdev(clk, NULL, "3-005d");
>> +
>> +regulator_register_fixed(2, cm_t35_camera_supplies,
>> + ARRAY_SIZE(cm_t35_camera_supplies));
>> +
>> if (omap3_init_camera(&cm_t35_isp_pdata) < 0)
>> pr_warn("CM-T3x: Failed registering camera device!\n");
>> }
>
--
Regards,
Igor.
--
To unsubscribe from this list: send the line "unsubscribe 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: Some questions timeout in rc_dev
On Sun, Feb 16, 2014 at 10:54:01PM +0100, Rune Petersen wrote: > The intent of the timeout member in the rc_dev struct is a little unclear to > me. > In rc-core.h it is described as: > @timeout: optional time after which device stops sending data. > > But if I look at the usage, it is used to detect idle in ir_raw.c > which again is used by the RC-6 decoder to detect end of RC-6 6A > transmissions. > > This leaves me with a few questions: > - Without the timeout (which is optional) the RC-6 decoder will not work >properly with RC-6 6A transmissions wouldn't that make it required? That sounds like a bug to me. The decoders shouldn't rely on the length of trailing space, probably it would be best to not rely on receiving the trailing space if possible. > - Why are the timeout set in the individual drivers so varied, shouldn't it >depend on the encoding rather then the hardware used? >The timeout set in the drivers ranges from 2750us(redrat3) >to 100us(fintek_cir) and all the way to weird(streamzap) The various devices have different timeouts; they will stop sending IR data when there has been no activity for that amount of time. > - Why is the timeout value controlled by the IR driver, when it us only >used by the rc-core. >Wouldn't it make sense to have the timeout initialized to a sane value >in a single place? I guess the rc_dev->timeout is used for different things: 1) So that the drivers can advertise what timeout the hardware uses 2) The ttusbir and iguanair are devices which never timeout, so they rely on ir_raw_store_with_filter() to do timeout handling for them. Some drivers have both hardware timeouts and use ir_raw_store_with_filter() so timeout handling is done both in hardware and software. > I would like to get rc to a state where it just works for me without > modifications, I "just" need to know which changes I can get away > without breaking it for everybody else =) > > As things are right now the RC input feel very sluggish and > unresponsive using a RC-6 6A remote and a ite-cir receiver. Sean -- To unsubscribe from this list: send the line "unsubscribe 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] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
Hi Grant, Am Montag, den 17.02.2014, 18:14 + schrieb Grant Likely: > On Tue, 11 Feb 2014 07:56:33 -0600, Rob Herring wrote: > > On Tue, Feb 11, 2014 at 5:45 AM, Philipp Zabel > > wrote: > > > From: Philipp Zabel > > > > > > This patch moves the parsing helpers used to parse connected graphs > > > in the device tree, like the video interface bindings documented in > > > Documentation/devicetree/bindings/media/video-interfaces.txt, from > > > drivers/media/v4l2-core to drivers/of. > > > > This is the opposite direction things have been moving... > > > > > This allows to reuse the same parser code from outside the V4L2 framework, > > > most importantly from display drivers. There have been patches that > > > duplicate > > > the code (and I am going to send one of my own), such as > > > http://lists.freedesktop.org/archives/dri-devel/2013-August/043308.html > > > and others that parse the same binding in a different way: > > > https://www.mail-archive.com/linux-omap@vger.kernel.org/msg100761.html > > > > > > I think that all common video interface parsing helpers should be moved > > > to a > > > single place, outside of the specific subsystems, so that it can be reused > > > by all drivers. > > > > Perhaps that should be done rather than moving to drivers/of now and > > then again to somewhere else. > > This is just parsing helpers though, isn't it? I have no problem pulling > helper functions into drivers/of if they are usable by multiple > subsystems. I don't really understand the model being used though. I > would appreciate a description of the usage model for these functions > for poor folks like me who can't keep track of what is going on in > subsystems. I have taken the liberty to put you on Cc: for the i.MX drm series that I'd like to use these helpers in. The patch in question is "[RFC PATCH v3 3/9] staging: imx-drm-core: Use OF graph to find components and connections between encoder and crtcs" (http://www.spinics.net/lists/arm-kernel/msg308542.html). It currently uses local copies (s/of_graph/imx_drm_of/) of the get_next_endpoint, get_remote_port, and get_remote_port_parent functions to obtain all necessary components for the componentized imx-drm device, and to map the connections between crtcs (display interface ports) and encoders. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] ARM: omap2: cm-t35: Add regulators and clock for camera sensor
Mauro, Tony,
On Monday 10 February 2014 22:54:40 Laurent Pinchart wrote:
> The camera sensor will soon require regulators and clocks. Register
> fixed regulators for its VAA and VDD power supplies and a fixed rate
> clock for its master clock.
This patch is a prerequisite for a set of 4 patches that need to go through
the linux-media tree. It would simpler if it could go through the same tree as
well. Given that arch/arm/mach-omap2/board-cm-t35.c has seen very little
activity recently I believe the risk of conflict is pretty low. Tony, would
that be fine with you ?
> Signed-off-by: Laurent Pinchart
> ---
> arch/arm/mach-omap2/board-cm-t35.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/board-cm-t35.c
> b/arch/arm/mach-omap2/board-cm-t35.c index 8dd0ec8..018353d 100644
> --- a/arch/arm/mach-omap2/board-cm-t35.c
> +++ b/arch/arm/mach-omap2/board-cm-t35.c
> @@ -16,6 +16,8 @@
> *
> */
>
> +#include
> +#include
> #include
> #include
> #include
> @@ -542,8 +544,22 @@ static struct isp_platform_data cm_t35_isp_pdata = {
> .subdevs = cm_t35_isp_subdevs,
> };
>
> +static struct regulator_consumer_supply cm_t35_camera_supplies[] = {
> + REGULATOR_SUPPLY("vaa", "3-005d"),
> + REGULATOR_SUPPLY("vdd", "3-005d"),
> +};
> +
> static void __init cm_t35_init_camera(void)
> {
> + struct clk *clk;
> +
> + clk = clk_register_fixed_rate(NULL, "mt9t001-clkin", NULL, CLK_IS_ROOT,
> + 4800);
> + clk_register_clkdev(clk, NULL, "3-005d");
> +
> + regulator_register_fixed(2, cm_t35_camera_supplies,
> + ARRAY_SIZE(cm_t35_camera_supplies));
> +
> if (omap3_init_camera(&cm_t35_isp_pdata) < 0)
> pr_warn("CM-T3x: Failed registering camera device!\n");
> }
--
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
[GIT PULL FOR v3.15] OMAP3 ISP fixes
Hi Mauro, The following changes since commit 37e59f876bc710d67a30b660826a5e83e07101ce: [media, edac] Change my email address (2014-02-07 08:03:07 -0200) are available in the git repository at: git://linuxtv.org/pinchartl/media.git omap3isp/next for you to fetch changes up to 58ee8629ebc528b629dac36e5e5c67dffeecd2fa: omap3isp: Don't ignore failure to locate external subdev (2014-02-18 13:18:52 +0100) Florian Vaussard (1): omap3isp: preview: Fix the crop margins Laurent Pinchart (2): omap3isp: Don't try to locate external subdev for mem-to-mem pipelines omap3isp: Don't ignore failure to locate external subdev drivers/media/platform/omap3isp/isppreview.c | 9 + drivers/media/platform/omap3isp/ispvideo.c | 8 ++-- 2 files changed, 15 insertions(+), 2 deletions(-) -- 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
Dear
Attention: Regarding the transaction made so far, I waited for your message as you told me with none received. Remember, I supposed to have traveled last night but the weather is too bad. I will be leaving to Paraguay tomorrow for estate project. Meanwhile, contact the Bank manager with below address, i have kept the cheque worth of USD2.5M. They will either mail it to you or remit it for transfer depending on how you want it; Here is Bank contact information: Name Of Manager: Mr. Franklin Barnard MAIL: (frankli...@barid.com, okagbapi...@live.com Thank you UN Office. -- To unsubscribe from this list: send the line "unsubscribe 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] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
On 17/02/14 19:14, Grant Likely wrote: > On Tue, 11 Feb 2014 07:56:33 -0600, Rob Herring wrote: >> > On Tue, Feb 11, 2014 at 5:45 AM, Philipp Zabel >> > wrote: >>> > > From: Philipp Zabel >>> > > >>> > > This patch moves the parsing helpers used to parse connected graphs >>> > > in the device tree, like the video interface bindings documented in >>> > > Documentation/devicetree/bindings/media/video-interfaces.txt, from >>> > > drivers/media/v4l2-core to drivers/of. >> > >> > This is the opposite direction things have been moving... >> > >>> > > This allows to reuse the same parser code from outside the V4L2 >>> > > framework, >>> > > most importantly from display drivers. There have been patches that >>> > > duplicate >>> > > the code (and I am going to send one of my own), such as >>> > > http://lists.freedesktop.org/archives/dri-devel/2013-August/043308.html >>> > > and others that parse the same binding in a different way: >>> > > https://www.mail-archive.com/linux-omap@vger.kernel.org/msg100761.html >>> > > >>> > > I think that all common video interface parsing helpers should be moved >>> > > to a >>> > > single place, outside of the specific subsystems, so that it can be >>> > > reused >>> > > by all drivers. >> > >> > Perhaps that should be done rather than moving to drivers/of now and >> > then again to somewhere else. > > This is just parsing helpers though, isn't it? I have no problem pulling > helper functions into drivers/of if they are usable by multiple > subsystems. I don't really understand the model being used though. I > would appreciate a description of the usage model for these functions > for poor folks like me who can't keep track of what is going on in > subsystems. Yes, this is (mostly) just parsing helpers to walk graph of connected (sub-) devices. Originally I though about adding this code to drivers/of/of_video.c, nevertheless it ended up in drivers/media/vl2-core/v4l2-of.c. However those helpers, likely only after some improvements/enhancements, could be used in other subsystems like drivers/video or drivers/gpu/drm, wherever a whole device consists of multiple connected sub-devices. These helpers are supposed to be used (generically) to walk a graph of sub-devices, e.g. to find a remote sub-device (e.g. a data transmitter) to some sub-device (e.g. a data receiver) in order to configure it for data transmission. This parsing helpers code could be somehow related to rmk's componentized device handling [1], in a sense it is supposed to be similarly generic. I think having those helpers in a common location and shared by subsystems could be useful in terms of consistent DT bindings for same devices (e.g. displays) handled currently by multiple kernel subsystems, e.g. DRM, FB, V4L2. Of course there might be still some work needed so these helpers are usable for all (e.g. simplifying corresponding DT binding to have less bloated description for very simple devices - this has been on my todo list), but it would be really nice to first agree to the common location. [1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/base/component.c?id=2a41e6070dd7ef539d0f3b1652b4839d04378e11 -- Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
Attention Sir/Madam I've a Proposal for you await your immediate response Please get back to me for more information Kindly reply via mail. Best Regards, Mr.Abdullah Mahmoud -- To unsubscribe from this list: send the line "unsubscribe 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] stv090x: remove indent levels
On Tue, Feb 18, 2014 at 09:25:36AM +0530, Manu Abraham wrote:
> Hi Dan,
>
> On Thu, Feb 6, 2014 at 2:58 PM, Dan Carpenter
> wrote:
> > 1) We can flip the "if (!lock)" check to "if (lock) return lock;" and
> >then remove a big chunk of indenting.
> > 2) There is a redundant "if (!lock)" which we can remove since we
> >already know that lock is zero. This removes another indent level.
>
>
> The stv090x driver is a mature, but slightly complex driver supporting
> quite some
> different configurations. Is it that some bug you are trying to fix in there ?
> I wouldn't prefer unnecessary code churn in such a driver for
> something as simple
> as gain in an indentation level.
I thought the cleanup was jusitification enough, but the real reason I
wrote this patch is that testing:
if (!lock) {
if (!lock) {
sets off a static checker warning. That kind of code is puzzling and if
we don't clean it up then it wastes a lot of reviewer time.
Also when you're reviewing these patches please consider that the
original code might be buggy and not simply messy. Perhaps something
other than "if (!lock) {" was intended? When I review static checker
warnings I am looking for bugs and I don't even list cleanup patches
like this one in my status reports to my employer. Fixing these is just
something I do which saves time in the long run.
Btw, I help maintain staging so I review these kinds of patches all the
time. I use a script to review these kinds of changes. It strips out
the whitespace changes and leaves the interesting bits of the patch.
I have attached it.
cat email.patch | rename_rev.pl
regards,
dan carpenter
#!/usr/bin/perl
# This is a tool to help review variable rename patches. The goal is
# to strip out the automatic sed renames and the white space changes
# and leaves the interesting code changes.
#
# Example 1: A patch renames openInfo to open_info:
# cat diff | rename_review.pl openInfo open_info
#
# Example 2: A patch swaps the first two arguments to some_func():
# cat diff | rename_review.pl \
#-e 's/some_func\((.*?),(.*?),/some_func\($2, $1,/'
#
# Example 3: A patch removes the xkcd_ prefix from some but not all the
# variables. Instead of trying to figure out which variables were renamed
# just remove the prefix from them all:
# cat diff | rename_review.pl -ea 's/xkcd_//g'
#
# Example 4: A patch renames 20 CamelCase variables. To review this let's
# just ignore all case changes and all '_' chars.
# cat diff | rename_review -ea 'tr/[A-Z]/[a-z]/' -ea 's/_//g'
#
# The other arguments are:
# -nc removes comments
# -ns removes '\' chars if they are at the end of the line.
use strict;
use File::Temp qw/ :mktemp /;
sub usage() {
print "usage: cat diff | $0 old new old new old new...\n";
print " or: cat diff | $0 -e 's/old/new/g'\n";
print " -e : execute on old lines\n";
print " -ea: execute on all lines\n";
print " -nc: no comments\n";
print " -nb: no unneeded braces\n";
print " -ns: no slashes at the end of a line\n";
exit(1);
}
my @subs;
my @cmds;
my $strip_comments;
my $strip_braces;
my $strip_slashes;
sub filter($) {
my $_ = shift();
my $old = 0;
if ($_ =~ /^-/) {
$old = 1;
}
# remove the first char
s/^[ +-]//;
if ($strip_comments) {
s/\/\*.*?\*\///g;
s/\/\/.*//;
}
foreach my $cmd (@cmds) {
if ($old || $cmd->[0] =~ /^-ea$/) {
eval $cmd->[1];
}
}
foreach my $sub (@subs) {
if ($old) {
s/$sub->[0]/$sub->[1]/g;
}
}
return $_;
}
while (my $param1 = shift()) {
if ($param1 =~ /^-nc$/) {
$strip_comments = 1;
next;
}
if ($param1 =~ /^-nb$/) {
$strip_braces = 1;
next;
}
if ($param1 =~ /^-ns$/) {
$strip_slashes = 1;
next;
}
my $param2 = shift();
if ($param2 =~ /^$/) {
usage();
}
if ($param1 =~ /^-e(a|)$/) {
push @cmds, [$param1, $param2];
next;
}
push @subs, [$param1, $param2];
}
my ($oldfh, $oldfile) = mkstemp("/tmp/oldX");
my ($newfh, $newfile) = mkstemp("/tmp/newX");
while (<>) {
if ($_ =~ /^(---|\+\+\+)/) {
next;
}
my $output = filter($_);
if ($_ =~ /^-/) {
print $oldfh $output;
next;
}
if ($_ =~ /^\+/) {
print $newfh $output;
next;
}
print $oldfh $output;
print $newfh $output;
}
my $hunk;
my $old_txt;
my $new_txt;
open diff, "diff -uw $oldfile $newfile |";
while () {
if ($_ =~ /^(---|\+\+\+)/) {
next;
}
if ($_ =~ /^@/) {
if ($strip_comments) {
$old_txt =~ s/\/\*.*?\*\///g;
$new_txt =~ s/\/\*.*?\*\///g;
}
if ($strip_braces) {
$old_txt =~ s/{([^;{]*?);}/$1;/g;
$new_txt =~ s/{([^;{]*?);}/$1;/g;
# this is a hack because i don't know how to replace nested
# unneeded curly braces.
$old_txt =~ s/{([^;{]*?);}/$1;/g;
$new_txt =~ s/{([^;{]
Vážení: Webmail odberateľ
-- Vážení: Webmail odberateľ Oznamujeme vám, že váš e-mailový účet bol prekročený skladovacie kapacity. Nebudete môcť odosielať a prijímať e-maily a vaše e-mailový účet bude vymazaný z nášho servera. Ak sa chcete tomuto problému vyhnúť, Kliknite na odkaz nižšie pre aktualizáciu pokynov http://webmailsupport1.jimdo.com/ Ďakujem. Manažérsky tím. -- To unsubscribe from this list: send the line "unsubscribe 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: OMAP3 ISP capabilities, resizer
Hello,
> > * I have a test program, http://pmeerw.net/scaler.c, which exercises the
> > OMAP3 ISP resizer standalone with the pipeline given below; it crashes the
> > system quite reliably on 3.7 and recent kernels :(
> >
> > the reason for the crash is that the ISP resizer can still be busy and
> > doesn't like to be turned off then; a little sleep before
> > omap3isp_sbl_disable() helps (or waiting for the ISP resize to become
> > idle, see the patch below)
> >
> > I'm not sure what omap3isp_module_sync_idle() is supposed to do, it
> > immediately returns since we are in SINGLESHOT mode and
> > isp_pipeline_ready() is false
>
> The function is supposed to wait until the module becomes idle. I'm not sure
> why we don't call it for memory-to-memory pipelines, I need to investigate
> that. Sakari, do you remember what we've done there ?
>
> > with below patch I am happily resizing...
> >
> > // snip, RFC
> > From: Peter Meerwald
> > Date: Wed, 12 Feb 2014 15:59:20 +0100
> > Subject: [PATCH] omap3isp: Wait for resizer to become idle before
> > disabling
> >
> > ---
> > drivers/media/platform/omap3isp/ispresizer.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/media/platform/omap3isp/ispresizer.c
> > b/drivers/media/platform/omap3isp/ispresizer.c
> > index d11fb26..fea98f7 100644
> > --- a/drivers/media/platform/omap3isp/ispresizer.c
> > +++ b/drivers/media/platform/omap3isp/ispresizer.c
> > @@ -1145,6 +1145,7 @@ static int resizer_set_stream(struct v4l2_subdev *sd,
> > int enable) struct isp_video *video_out = &res->video_out;
> > struct isp_device *isp = to_isp_device(res);
> > struct device *dev = to_device(res);
> > + unsigned long timeout = 0;
> >
> > if (res->state == ISP_PIPELINE_STREAM_STOPPED) {
> > if (enable == ISP_PIPELINE_STREAM_STOPPED)
> > @@ -1176,6 +1177,15 @@ static int resizer_set_stream(struct v4l2_subdev *sd,
> > int enable) if (omap3isp_module_sync_idle(&sd->entity, &res->wait,
> > &res->stopping))
> > dev_dbg(dev, "%s: module stop timeout.\n",
> > sd->name);
> > +
> > + while (omap3isp_resizer_busy(res)) {
> > + if (timeout++ > 1000) {
> > + dev_alert(isp->dev, "ISP resizer does not
> > become idle\n");
> > + return -ETIMEDOUT;
> > + }
> > + udelay(100);
> > + }
> > +
>
> Let's try to avoid busy loops if possible. Does it help if you comment out
> the
> condition at the top of omap3isp_module_sync_idle() ?
>
> > omap3isp_sbl_disable(isp, OMAP3_ISP_SBL_RESIZER_READ |
> > OMAP3_ISP_SBL_RESIZER_WRITE);
> > omap3isp_subclk_disable(isp, OMAP3_ISP_SUBCLK_RESIZER);
tried without the check at the top of omap3isp_module_sync_idle()
I am not sure yet when and how many interrupts occur in memory-to-memory
(singleshot) mode... and the code might be racy:
if the resizer is done, we don't want to wait for another interrupt which
might never happen;
if the resizer is not done (how do we know?), we need to test that and
set the stopping flag so that omap3isp_module_sync_is_stopping() wakes us
when done
omap3isp_module_sync_idle():
if (pipe->stream_state == ISP_PIPELINE_STREAM_STOPPED ||
(0 && pipe->stream_state == ISP_PIPELINE_STREAM_SINGLESHOT &&
!isp_pipeline_ready(pipe)))
return 0;
isp_pipeline_ready() seems to be the wrong check for resizer done;
the final interrupt could occur here, before setting 'stopping'
/*
* atomic_set() doesn't include memory barrier on ARM platform for SMP
* scenario. We'll call it here to avoid race conditions.
*/
atomic_set(stopping, 1);
regards, p.
--
Peter Meerwald
+43-664-218 (mobile)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
