Re: [PATCH/RFC] tmio_mmc: keep card-detect interrupts enabled
Well if they are only masked they shouldnt stop being asserted. But we should unmask them again. Im not really sure we should mask them anyway, with the card possibly being gone... Will need to look into it further. 2009/11/9 Guennadi Liakhovetski : > (re-adding accidentally dropped ML) > > On Mon, 9 Nov 2009, Ian Molton wrote: > >> Well, I presume we want to know when the card gets removed :) > > Sure, that's why we shouldn't mask those interrupts:-) If they do get > masked and missed, I do not know, if the interrupt remains pending in this > case, because they never get detected then:) > >> >> 2009/11/9 Guennadi Liakhovetski : >> > Hi Ian >> > >> > Why did you drop all CCs? >> > >> > On Mon, 9 Nov 2009, Ian Molton wrote: >> > >> >> I havent looked at the consequences for the driver if a insert IRQ >> >> occurs during IO, however it seems logical that we should not >> >> permanently mask the IRQ. >> >> >> >> I presume that the IRQ remains pending? >> > >> > Don't know, never checked. Is this important to know? >> > >> > Thanks >> > Guennadi >> > >> >> >> >> 2009/11/6 Guennadi Liakhovetski : >> >> > On SuperH platforms the SDHI controller does not produce any command >> >> > IRQs >> >> > after a completed IO. This leads to card-detect interrupts staying >> >> > disabled. Do not disable card-detect interrupts on DATA IRQs. >> >> > >> >> > Signed-off-by: Guennadi Liakhovetski >> >> > --- >> >> > >> >> > Marked as RFC because I'm not really sure this is a correct approach to >> >> > fix this problem, and whether this will have negative effect on other >> >> > tmio_mmc MFD users. >> >> > >> >> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h >> >> > index c676767..0b31d44 100644 >> >> > --- a/drivers/mmc/host/tmio_mmc.h >> >> > +++ b/drivers/mmc/host/tmio_mmc.h >> >> > @@ -55,10 +55,8 @@ >> >> > /* Define some IRQ masks */ >> >> > /* This is the mask used at reset by the chip */ >> >> > #define TMIO_MASK_ALL 0x837f031d >> >> > -#define TMIO_MASK_READOP (TMIO_STAT_RXRDY | TMIO_STAT_DATAEND | \ >> >> > - TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT) >> >> > -#define TMIO_MASK_WRITEOP (TMIO_STAT_TXRQ | TMIO_STAT_DATAEND | \ >> >> > - TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT) >> >> > +#define TMIO_MASK_READOP (TMIO_STAT_RXRDY | TMIO_STAT_DATAEND) >> >> > +#define TMIO_MASK_WRITEOP (TMIO_STAT_TXRQ | TMIO_STAT_DATAEND) >> >> > #define TMIO_MASK_CMD (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT >> >> > | \ >> >> > TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT) >> >> > #define TMIO_MASK_IRQ (TMIO_MASK_READOP | TMIO_MASK_WRITEOP | >> >> > TMIO_MASK_CMD) > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ > -- Ian Molton Linux, Automotive, and other hacking: http://www.mnementh.co.uk/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] media: rcar_vin: Fix race condition terminating stream
On Mon, 04 Aug 2014 23:27:24 +0200
Hans Verkuil wrote:
> > +/*
> > + * Wait for capture to stop and all in-flight buffers to be finished with
> > by
> > + * the video hardware. This must be called under &priv->lock
> > + *
> > + */
> > +static void rcar_vin_wait_stop_streaming(struct rcar_vin_priv *priv)
> > +{
> > + while (priv->state != STOPPED) {
> > +
> > + /* issue stop if running */
> > + if (priv->state == RUNNING)
> > + rcar_vin_request_capture_stop(priv);
> > +
> > + /* wait until capturing has been stopped */
> > + if (priv->state == STOPPING) {
> > + priv->request_to_stop = true;
> > + spin_unlock_irq(&priv->lock);
> > + wait_for_completion(&priv->capture_stop);
> > + spin_lock_irq(&priv->lock);
> > + }
> > + }
> > +}
> > +
> > static void rcar_vin_videobuf_release(struct vb2_buffer *vb)
> > {
> > struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
> > @@ -462,7 +485,6 @@ static void rcar_vin_videobuf_release(struct vb2_buffer
> > *vb)
> > struct rcar_vin_priv *priv = ici->priv;
> > unsigned int i;
> > int buf_in_use = 0;
> > -
> > spin_lock_irq(&priv->lock);
> >
> > /* Is the buffer in use by the VIN hardware? */
> > @@ -474,20 +496,8 @@ static void rcar_vin_videobuf_release(struct
> > vb2_buffer *vb)
> > }
> >
> > if (buf_in_use) {
> > - while (priv->state != STOPPED) {
> > -
> > - /* issue stop if running */
> > - if (priv->state == RUNNING)
> > - rcar_vin_request_capture_stop(priv);
> > -
> > - /* wait until capturing has been stopped */
> > - if (priv->state == STOPPING) {
> > - priv->request_to_stop = true;
> > - spin_unlock_irq(&priv->lock);
> > - wait_for_completion(&priv->capture_stop);
> > - spin_lock_irq(&priv->lock);
> > - }
> > - }
> > + rcar_vin_wait_stop_streaming(priv);
> > +
>
> Why on earth would videobuf_release call stop_streaming()?
It doesn't. But it appears it can be called whilst the driver still possesses
the buffer, in which case, the driver (as was) would request capture stop, and
wait for the buffer to be returned from the driver.
This logic here has not been changed, merely moved out to an appropriately
named function, so that it can be re-used in rcar_vin_stop_streaming().
> You start streaming in the start_streaming op, not in the buf_queue op. If you
> need a certain minimum of buffers before start_streaming can be called, then
> just
> set min_buffers_needed in struct vb2_queue.
I can submit an additional patch to correct this behaviour in the rcar_vin
driver, if that would be helpful?
> And stop streaming happens in stop_streaming. The various vb2 queue ops
> should just
> do what the op name says. That way everything works nicely together and it
> makes
> your driver much easier to understand.
Agreed. It was particularly difficult to understand WTH this driver was doing.
> Sorry I am late in reviewing this, but I only now stumbled on these patches.
Thanks for the review!
-Ian
> > /*
> > * Capturing has now stopped. The buffer we have been asked
> > * to release could be any of the current buffers in use, so
> > @@ -517,12 +527,15 @@ static void rcar_vin_stop_streaming(struct vb2_queue
> > *vq)
> >
> > spin_lock_irq(&priv->lock);
> >
> > + rcar_vin_wait_stop_streaming(priv);
> > +
> > for (i = 0; i < vq->num_buffers; ++i)
> > if (vq->bufs[i]->state == VB2_BUF_STATE_ACTIVE)
> > vb2_buffer_done(vq->bufs[i], VB2_BUF_STATE_ERROR);
> >
> > list_for_each_safe(buf_head, tmp, &priv->capture)
> > list_del_init(buf_head);
> > +
> > spin_unlock_irq(&priv->lock);
> > }
> >
> >
>
>
>
--
Ian Molton
--
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/1 v1] adv7604: Add adv7612 support
This small patch series adds initial support for the adv7612 dual HDMI input decoder chip and adds a device-tree option allowing the default input to be selected. Please review / apply, -Ian -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] media: adv7604: Add ability to read default input port from DT
This patch adds support to the adv7604 driver for reading the default
selected input from the Device tree. If none is provided, the driver will not
select an input without help from userspace.
Tested-by: William Towle
Signed-off-by: Ian Molton
---
Documentation/devicetree/bindings/media/i2c/adv7604.txt | 5 -
drivers/media/i2c/adv7604.c | 9 +++--
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
index cc0708c..6e8ace0 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
@@ -41,11 +41,12 @@ Optional Endpoint Properties:
- hsync-active: Horizontal synchronization polarity. Defaults to active low.
- vsync-active: Vertical synchronization polarity. Defaults to active low.
- - pclk-sample: Pixel clock polarity. Defaults to output on the falling edge.
+ - pclk-sample: Pixel clock polarity. Defaults to output on the falling edge.
If none of hsync-active, vsync-active and pclk-sample is specified the
endpoint will use embedded BT.656 synchronization.
+ - default-input: Select which input is selected after reset.
Example:
@@ -59,6 +60,8 @@ Example:
#address-cells = <1>;
#size-cells = <0>;
+ default-input = <0>;
+
port@0 {
reg = <0>;
};
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 9f73a7f..75e1942 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -2732,7 +2732,7 @@ static int adv7604_parse_dt(struct adv7604_state *state)
struct v4l2_of_endpoint bus_cfg;
struct device_node *endpoint;
struct device_node *np;
- unsigned int flags;
+ unsigned int flags, v;
np = state->i2c_clients[ADV7604_PAGE_IO]->dev.of_node;
@@ -2742,6 +2742,12 @@ static int adv7604_parse_dt(struct adv7604_state *state)
return -EINVAL;
v4l2_of_parse_endpoint(endpoint, &bus_cfg);
+
+if (!of_property_read_u32(endpoint, "default_input", &v))
+ state->pdata.default_input = v;
+ else
+ state->pdata.default_input = -1;
+
of_node_put(endpoint);
flags = bus_cfg.bus.parallel.flags;
@@ -2780,7 +2786,6 @@ static int adv7604_parse_dt(struct adv7604_state *state)
/* Hardcode the remaining platform data fields. */
state->pdata.disable_pwrdnb = 0;
state->pdata.disable_cable_det_rst = 0;
- state->pdata.default_input = -1;
state->pdata.blank_data = 1;
state->pdata.alt_data_sat = 1;
state->pdata.op_format_mode_sel = ADV7604_OP_FORMAT_MODE0;
--
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
[PATCH 1/2] media: adv7604: Add support for ADV7612 dual HDMI input decoder.
This patch adds necessary support for the ADV7612 dual HDMI decoder / repeater
chip.
This was tested using a heavily modified rcar_vin/soc_camera capture driver.
Tested-by: William Towle
Signed-off-by: Ian Molton
---
.../devicetree/bindings/media/i2c/adv7604.txt | 17 +++
drivers/media/i2c/adv7604.c| 54 +-
2 files changed, 61 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
index c27cede..cc0708c 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
@@ -1,15 +1,16 @@
-* Analog Devices ADV7604/11 video decoder with HDMI receiver
+* Analog Devices ADV7604/11/12 video decoder with HDMI receiver
-The ADV7604 and ADV7611 are multiformat video decoders with an integrated HDMI
-receiver. The ADV7604 has four multiplexed HDMI inputs and one analog input,
-and the ADV7611 has one HDMI input and no analog input.
+The ADV7604 and ADV7611/12 are multiformat video decoders with an integrated
+HDMI receiver. The ADV7604 has four multiplexed HDMI inputs and one analog
+input, and the ADV7611 has one HDMI input and no analog input. The 7612 is
similar to the 7611 but has 2 HDMI inputs.
-These device tree bindings support the ADV7611 only at the moment.
+These device tree bindings support the ADV7611/12 only at the moment.
Required Properties:
- compatible: Must contain one of the following
- "adi,adv7611" for the ADV7611
+- "adi,adv7612" for the ADV7612
- reg: I2C slave address
@@ -22,10 +23,10 @@ port, in accordance with the video interface bindings
defined in
Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes
are numbered as follows.
- Port ADV7611
+ Port ADV7611ADV7612
- HDMI 0
- Digital output 1
+ HDMI 0 0, 1
+ Digital output 12
The digital output port node must contain at least one endpoint.
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 1778d32..9f73a7f 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -80,6 +80,7 @@ MODULE_LICENSE("GPL");
enum adv7604_type {
ADV7604,
ADV7611,
+ ADV7612,
};
struct adv7604_reg_seq {
@@ -2517,6 +2518,11 @@ static void adv7611_setup_irqs(struct v4l2_subdev *sd)
io_write(sd, 0x41, 0xd0); /* STDI irq for any change, disable INT2 */
}
+static void adv7612_setup_irqs(struct v4l2_subdev *sd)
+{
+ io_write(sd, 0x41, 0xd0); /* disable INT2 */
+}
+
static void adv7604_unregister_clients(struct adv7604_state *state)
{
unsigned int i;
@@ -2601,6 +2607,19 @@ static const struct adv7604_reg_seq
adv7611_recommended_settings_hdmi[] = {
{ ADV7604_REG_SEQ_TERM, 0 },
};
+static const struct adv7604_reg_seq adv7612_recommended_settings_hdmi[] = {
+ { ADV7604_REG(ADV7604_PAGE_CP, 0x6c), 0x00 },
+ { ADV7604_REG(ADV7604_PAGE_HDMI, 0x9b), 0x03 },
+ { ADV7604_REG(ADV7604_PAGE_HDMI, 0x6f), 0x08 },
+ { ADV7604_REG(ADV7604_PAGE_HDMI, 0x85), 0x1f },
+ { ADV7604_REG(ADV7604_PAGE_HDMI, 0x87), 0x70 },
+ { ADV7604_REG(ADV7604_PAGE_HDMI, 0x57), 0xda },
+ { ADV7604_REG(ADV7604_PAGE_HDMI, 0x58), 0x01 },
+ { ADV7604_REG(ADV7604_PAGE_HDMI, 0x03), 0x98 },
+ { ADV7604_REG(ADV7604_PAGE_HDMI, 0x4c), 0x44 },
+ { ADV7604_REG_SEQ_TERM, 0 },
+};
+
static const struct adv7604_chip_info adv7604_chip_info[] = {
[ADV7604] = {
.type = ADV7604,
@@ -2663,17 +2682,47 @@ static const struct adv7604_chip_info
adv7604_chip_info[] = {
BIT(ADV7604_PAGE_REP) | BIT(ADV7604_PAGE_EDID) |
BIT(ADV7604_PAGE_HDMI) | BIT(ADV7604_PAGE_CP),
},
+ [ADV7612] = {
+ .type = ADV7612,
+ .has_afe = false,
+ .max_port = ADV7604_PAD_HDMI_PORT_B,
+ .num_dv_ports = 2,
+ .edid_enable_reg = 0x74,
+ .edid_status_reg = 0x76,
+ .lcf_reg = 0xa3,
+ .tdms_lock_mask = 0x43,
+ .cable_det_mask = 0x01,
+ .fmt_change_digital_mask = 0x03,
+ .formats = adv7604_formats,
+ .nformats = ARRAY_SIZE(adv7604_formats),
+ .set_termination = adv7611_set_termination,
+ .setup_irqs = adv7612_setup_irqs,
+ .read_hdmi_pixelclock = adv7611_read_hdmi_pixelclock,
+ .read_cable_det = adv7611_read_cable_det,
+ .recommended_settings = {
+ [1] = adv7612_recommended_settings_hdmi,
+ },
+ .num_recommended_settings = {
+
rcar_vin: rcar_vin_get_formats()
rcar_vin_get_formats() Can anyone explain to me what on earth this function is trying to achieve? I'm finding it rather impenetrable, and it works for me on one driver (adv7180) but not with another (modified 7612 driver). It appears that its doing some sort of magic with the builtin array of formats, to allow the 7180 driver to select a YUV mode?? but I cant for the life of me understand what. I'm fairly new to v4l2, so I dont really know whats legit and what isnt. particularly, the code appears to abuse one "code" to provide several (incompatible?) formats. Help? -- Ian Molton -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] media: adv7604: Add ability to read default input port from DT
On Mon, 11 Aug 2014 13:19:02 +0100
Mark Rutland wrote:
> > - - pclk-sample: Pixel clock polarity. Defaults to output on the falling
> > edge.
> > + - pclk-sample: Pixel clock polarity. Defaults to output on the falling
> > edge.
>
> Unrelated whitespace change?
Is there a sensible way to get miniscule whitespace changes in?
> >If none of hsync-active, vsync-active and pclk-sample is specified the
> >endpoint will use embedded BT.656 synchronization.
> >
> > + - default-input: Select which input is selected after reset.
>
> Valid values are?
Chip dependent. 0 for 7611, 0-1 for 7612, I expect there are other chips in the
family with differing numbers of inputs.
> > +if (!of_property_read_u32(endpoint, "default_input", &v))
>
> This doesn't match the binding ('_' vs '-').
Good catch!
--
Ian Molton
--
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/4] media: rcar_vin: Ensure all in-flight buffers are returned to error state before stopping.
> Fixed kernel WARNINGs for me! \o/ > Ian, perhaps it makes sense for me to take these patches into my hands? I'm planning to respin these tomorrow - is that OK? I have test hardware with two different frontends here. -Ian -- 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
Scaling and rounding issues
Hi folks,
I've been discussing some issues with the CODA driver on gstreamer-devel and
the thread seems better suited to this list;
Here's a copy of what's been said thus far:
I wrote:
I've located the cause of the "giant oops" I noted a couple of days ago.
because ctx->icc is recycled, it must be a valid or NULL pointer for kfree().
When an error occurs, the pointer is not reset to NULL, and kfree() crashes.
This helps:
@@ -208,10 +208,11 @@ static void ipu_scaler_work(struct work_struct *work)
kfree(ctx->icc);
ctx->icc = ipu_image_convert_prepare(ipu_scaler->ipu, &in,
&out, ctx->ctrl,
&ctx->num_tiles);
if (IS_ERR(ctx->icc)) {
+ctx->icc = NULL;
schedule_work(&ctx->skip_run);
return;
}
}
This fix "band-aids" it, but I don't think its complete, as IMO, this really
should result in gstreamer giving up, not displaying blank frames.
On the plus side, it does make the whole thing a lot more reliable.
It seems also like this case should be caught a lot earlier.
I've also noticed odd behaviour below width=480; (height=272)
479: works
478: works
477: gstreamer errors out
476: works (but no video output)
475: ditto
747: ditto
743: gstreamer errors out
The error is:
0:00:00.284476334 1216 0xd01e30 ERROR v4l2transform
gstv4l2transform.c:239:gst_v4l2_transform_set_caps: failed
to set output caps: video/x-raw, pixel-aspect-ratio=(fraction)1/1,
interlace-mode=(string)progressive, framerate=(fraction)24/1, format=(string)BGRx,
width=(int)473, height=(int)272
ERROR: from element
/GstPipeline:pipeline0/v4l2video0convert:v4l2video0convert0: Device
'/dev/video0' cannot capture at 473x272
Additional debug info:
gstv4l2object.c(2967): gst_v4l2_object_set_format ():
/GstPipeline:pipeline0/v4l2video0convert:v4l2video0convert0:
Tried to capture at 473x272, but device returned size 472x272
ERROR: pipeline doesn't want to preroll.
which smells like a shift or bitwise test thats not right.
I'm guessing 478 and 479 are rounding up to 480, but the others aren't.
--
Nicholas dufresne replied thus:
This is a famous case. The driver rounds middle, where only rounding up
can be managed by application. There is an RFC to add some flags to VB2
to let the driver decide what's ideal. When you crop, you want to round
down, when you allocate space, you want to round up (so one can crop on
top). The middle rounding is usually not very useful. I think this
discussion should move to linux-media ML.
--
So the question is what should be done about this?
-Ian
--
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] coda: drop dma_sync_single_for_device in coda_bitstream_queue
On 25/03/15 16:45, Philipp Zabel wrote: Issuing a cache flush for the whole bitstream buffer is not optimal in the first place when only a part of it was written. But given that the buffer is mapped in writecombine mode, it is not needed at all. Signed-off-by: Philipp Zabel Tested-by: Ian Molton --- drivers/media/platform/coda/coda-bit.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c index d39789d..d336cb6 100644 --- a/drivers/media/platform/coda/coda-bit.c +++ b/drivers/media/platform/coda/coda-bit.c @@ -181,10 +181,6 @@ static int coda_bitstream_queue(struct coda_ctx *ctx, if (n < src_size) return -ENOSPC; - dma_sync_single_for_device(&ctx->dev->plat_dev->dev, - ctx->bitstream.paddr, ctx->bitstream.size, - DMA_TO_DEVICE); - src_buf->v4l2_buf.sequence = ctx->qsequence++; return 0; -- 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
rcar_vin, soc_camera and videobuf2
Hi folks,
A colleague and I have been attempting to debug issues with rcar_vin,
soc_camera, and videobuf2.
Presently, We're using the adv7180 frontend to feed composite video to the rcar
hardware.
Using the streamer utility from xawtv3 (as packaged by wheezy), we have been
able to capture both stills and video, however there are significant issues.
There are a lot of warnings triggered in the videobuf2 code, primarily in
videobuf2-core.c
in vb2_buffer_done() we found that it appears that q->start_streaming_called is
0, due to the following code in __vb2_queue_cancel().
q->streaming = 0;
q->start_streaming_called = 0;
q->queued_count = 0;
if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
for (i = 0; i < q->num_buffers; ++i) {
if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE)
vb2_buffer_done(q->bufs[i],
VB2_BUF_STATE_ERROR);
}
/* Must be zero now */
WARN_ON(atomic_read(&q->owned_by_drv_count));
}
This causes the code in vb2_buffer_done() to follow the "DMA path" and thus
generate a warning.
Moving the first three lines afer the if() clause means that we no longer see
the WARN_ON(state != VB2_BUF_STATE_QUEUED) in vb2_buffer_done(), however, we
still see the WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE) later in the process.
When capturing video, the driver also exhibits very odd behaviour, including
apparently time running backwards, which appears to be the result of queuing
buffers in the wrong order. We also see a lot of messages re: buffers being
queued twice.
Looking at the rcar_vin driver, it appears to perform its allocations quite
differently from other soc_camera drivers.
Does anyone have any guidance on how to proceed? Clearly the state machine is
being violated here, but I'm at a loss as to how its actually *supposed* to
operate. Is there any good documentation?
--
Ian Molton
--
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: rcar_vin, soc_camera and videobuf2
[[sorry for the repost - fixing CC's]]
Hi folks,
A colleague and I have been attempting to debug issues with rcar_vin,
soc_camera, and videobuf2.
Presently, We're using the adv7180 frontend to feed composite video to the rcar
hardware.
Using the streamer utility from xawtv3 (as packaged by wheezy), we have been
able to capture both stills and video, however there are significant issues.
There are a lot of warnings triggered in the videobuf2 code, primarily in
videobuf2-core.c
in vb2_buffer_done() we found that it appears that q->start_streaming_called is
0, due to the following code in __vb2_queue_cancel().
q->streaming = 0;
q->start_streaming_called = 0;
q->queued_count = 0;
if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
for (i = 0; i < q->num_buffers; ++i) {
if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE)
vb2_buffer_done(q->bufs[i],
VB2_BUF_STATE_ERROR);
}
/* Must be zero now */
WARN_ON(atomic_read(&q->owned_by_drv_count));
}
This causes the code in vb2_buffer_done() to follow the "DMA path" and thus
generate a warning.
Moving the first three lines afer the if() clause means that we no longer see
the WARN_ON(state != VB2_BUF_STATE_QUEUED) in vb2_buffer_done(), however, we
still see the WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE) later in the process.
When capturing video, the driver also exhibits very odd behaviour, including
apparently time running backwards, which appears to be the result of queuing
buffers in the wrong order. We also see a lot of messages re: buffers being
queued twice.
Looking at the rcar_vin driver, it appears to perform its allocations quite
differently from other soc_camera drivers.
Does anyone have any guidance on how to proceed? Clearly the state machine is
being violated here, but I'm at a loss as to how its actually *supposed* to
operate. Is there any good documentation?
--
Ian Molton
--
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] media: rcar_vin: Fix race condition terminating stream
This patch fixes a race condition whereby a frame being captured may generate an
interrupt between requesting capture to halt and freeing buffers.
This condition is exposed by the earlier patch that explicitly calls
vb2_buffer_done() during stop streaming.
The solution is to wait for capture to finish prior to finalising these buffers.
Signed-off-by: Ian Molton
Signed-off-by: William Towle
---
drivers/media/platform/soc_camera/rcar_vin.c | 43 ++--
1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
b/drivers/media/platform/soc_camera/rcar_vin.c
index 06ce705..aeda4e2 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -455,6 +455,29 @@ error:
vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
}
+/*
+ * Wait for capture to stop and all in-flight buffers to be finished with by
+ * the video hardware. This must be called under &priv->lock
+ *
+ */
+static void rcar_vin_wait_stop_streaming(struct rcar_vin_priv *priv)
+{
+ while (priv->state != STOPPED) {
+
+ /* issue stop if running */
+ if (priv->state == RUNNING)
+ rcar_vin_request_capture_stop(priv);
+
+ /* wait until capturing has been stopped */
+ if (priv->state == STOPPING) {
+ priv->request_to_stop = true;
+ spin_unlock_irq(&priv->lock);
+ wait_for_completion(&priv->capture_stop);
+ spin_lock_irq(&priv->lock);
+ }
+ }
+}
+
static void rcar_vin_videobuf_release(struct vb2_buffer *vb)
{
struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
@@ -462,7 +485,6 @@ static void rcar_vin_videobuf_release(struct vb2_buffer *vb)
struct rcar_vin_priv *priv = ici->priv;
unsigned int i;
int buf_in_use = 0;
-
spin_lock_irq(&priv->lock);
/* Is the buffer in use by the VIN hardware? */
@@ -474,20 +496,8 @@ static void rcar_vin_videobuf_release(struct vb2_buffer
*vb)
}
if (buf_in_use) {
- while (priv->state != STOPPED) {
-
- /* issue stop if running */
- if (priv->state == RUNNING)
- rcar_vin_request_capture_stop(priv);
-
- /* wait until capturing has been stopped */
- if (priv->state == STOPPING) {
- priv->request_to_stop = true;
- spin_unlock_irq(&priv->lock);
- wait_for_completion(&priv->capture_stop);
- spin_lock_irq(&priv->lock);
- }
- }
+ rcar_vin_wait_stop_streaming(priv);
+
/*
* Capturing has now stopped. The buffer we have been asked
* to release could be any of the current buffers in use, so
@@ -517,12 +527,15 @@ static void rcar_vin_stop_streaming(struct vb2_queue *vq)
spin_lock_irq(&priv->lock);
+ rcar_vin_wait_stop_streaming(priv);
+
for (i = 0; i < vq->num_buffers; ++i)
if (vq->bufs[i]->state == VB2_BUF_STATE_ACTIVE)
vb2_buffer_done(vq->bufs[i], VB2_BUF_STATE_ERROR);
list_for_each_safe(buf_head, tmp, &priv->capture)
list_del_init(buf_head);
+
spin_unlock_irq(&priv->lock);
}
--
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
[PATCH 4/4] media: rcar_vin: Clean up rcar_vin_irq
This patch makes the rcar_vin IRQ handler a little more readable.
Removes an else clause, and simplifies the buffer handling.
Signed-off-by: Ian Molton
Reviewed-by: William Towle
---
drivers/media/platform/soc_camera/rcar_vin.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
b/drivers/media/platform/soc_camera/rcar_vin.c
index aeda4e2..a8d2785 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -554,7 +554,6 @@ static irqreturn_t rcar_vin_irq(int irq, void *data)
struct rcar_vin_priv *priv = data;
u32 int_status;
bool can_run = false, hw_stopped;
- int slot;
unsigned int handled = 0;
spin_lock(&priv->lock);
@@ -573,17 +572,22 @@ static irqreturn_t rcar_vin_irq(int irq, void *data)
hw_stopped = !(ioread32(priv->base + VNMS_REG) & VNMS_CA);
if (!priv->request_to_stop) {
+ struct vb2_buffer **q_entry = priv->queue_buf;
+ struct vb2_buffer *vb;
+
if (is_continuous_transfer(priv))
- slot = (ioread32(priv->base + VNMS_REG) &
- VNMS_FBS_MASK) >> VNMS_FBS_SHIFT;
- else
- slot = 0;
+ q_entry += (ioread32(priv->base + VNMS_REG) &
+ VNMS_FBS_MASK) >> VNMS_FBS_SHIFT;
+
+ vb = *q_entry;
+
+ vb->v4l2_buf.field = priv->field;
+ vb->v4l2_buf.sequence = priv->sequence++;
+ do_gettimeofday(&vb->v4l2_buf.timestamp);
+
+ vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
- priv->queue_buf[slot]->v4l2_buf.field = priv->field;
- priv->queue_buf[slot]->v4l2_buf.sequence = priv->sequence++;
- do_gettimeofday(&priv->queue_buf[slot]->v4l2_buf.timestamp);
- vb2_buffer_done(priv->queue_buf[slot], VB2_BUF_STATE_DONE);
- priv->queue_buf[slot] = NULL;
+ *q_entry = NULL;
if (priv->state != STOPPING)
can_run = rcar_vin_fill_hw_slot(priv);
--
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
[PATCH 2/4] media: rcar_vin: Ensure all in-flight buffers are returned to error state before stopping.
Videobuf2 complains about buffers that are still marked ACTIVE (in use by the driver) following a call to stop_streaming(). This patch returns all active buffers to state ERROR prior to stopping. Note: this introduces a (non fatal) race condition as the stream is not guaranteed to be stopped at this point. Signed-off-by: Ian Molton Signed-off-by: William Towle --- drivers/media/platform/soc_camera/rcar_vin.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c index 7154500..06ce705 100644 --- a/drivers/media/platform/soc_camera/rcar_vin.c +++ b/drivers/media/platform/soc_camera/rcar_vin.c @@ -513,8 +513,14 @@ static void rcar_vin_stop_streaming(struct vb2_queue *vq) struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct rcar_vin_priv *priv = ici->priv; struct list_head *buf_head, *tmp; + int i; spin_lock_irq(&priv->lock); + + for (i = 0; i < vq->num_buffers; ++i) + if (vq->bufs[i]->state == VB2_BUF_STATE_ACTIVE) + vb2_buffer_done(vq->bufs[i], VB2_BUF_STATE_ERROR); + list_for_each_safe(buf_head, tmp, &priv->capture) list_del_init(buf_head); spin_unlock_irq(&priv->lock); -- 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
[PATCH 0/4] rcar_vin: fix soc_camera WARN_ON() issues.
This patch series provides fixes that allow the rcar_vin driver to function without triggering dozens of warnings from the videobuf2 and soc_camera layers. Patches 2/3 should probably be merged into a single, atomic change, although patch 2 does not make the existing situation /worse/ in and of itself. Patch 4 does not change the code logic, but is cleaner and less prone to breakage caused by furtutre modification. Also, more consistent with the use of vb pointers elsewhere in the driver. Comments welcome! -Ian -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] media: rcar_vin: Dont aggressively retire buffers
rcar_vin_videobuf_release() is called once per buffer from the buf_cleanup hook.
There is no need to look up the queue and free all buffers at this point.
Signed-off-by: Ian Molton
Signed-off-by: William Towle
---
drivers/media/platform/soc_camera/rcar_vin.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
b/drivers/media/platform/soc_camera/rcar_vin.c
index e594230..7154500 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -493,17 +493,11 @@ static void rcar_vin_videobuf_release(struct vb2_buffer
*vb)
* to release could be any of the current buffers in use, so
* release all buffers that are in use by HW
*/
- for (i = 0; i < MAX_BUFFER_NUM; i++) {
- if (priv->queue_buf[i]) {
- vb2_buffer_done(priv->queue_buf[i],
- VB2_BUF_STATE_ERROR);
- priv->queue_buf[i] = NULL;
- }
- }
- } else {
- list_del_init(to_buf_list(vb));
+ priv->queue_buf[i] = NULL;
}
+ list_del_init(to_buf_list(vb));
+
spin_unlock_irq(&priv->lock);
}
--
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
Resend: [PATCH 0/4] rcar_vin: fix soc_camera WARN_ON() issues.
Resent to include the author and a couple of other interested parties :) This patch series provides fixes that allow the rcar_vin driver to function without triggering dozens of warnings from the videobuf2 and soc_camera layers. Patches 2/3 should probably be merged into a single, atomic change, although patch 2 does not make the existing situation /worse/ in and of itself. Patch 4 does not change the code logic, but is cleaner and less prone to breakage caused by furtutre modification. Also, more consistent with the use of vb pointers elsewhere in the driver. Comments welcome! -Ian -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] media: rcar_vin: Ensure all in-flight buffers are returned to error state before stopping.
Videobuf2 complains about buffers that are still marked ACTIVE (in use by the driver) following a call to stop_streaming(). This patch returns all active buffers to state ERROR prior to stopping. Note: this introduces a (non fatal) race condition as the stream is not guaranteed to be stopped at this point. Signed-off-by: Ian Molton Signed-off-by: William Towle --- drivers/media/platform/soc_camera/rcar_vin.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c index 7154500..06ce705 100644 --- a/drivers/media/platform/soc_camera/rcar_vin.c +++ b/drivers/media/platform/soc_camera/rcar_vin.c @@ -513,8 +513,14 @@ static void rcar_vin_stop_streaming(struct vb2_queue *vq) struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct rcar_vin_priv *priv = ici->priv; struct list_head *buf_head, *tmp; + int i; spin_lock_irq(&priv->lock); + + for (i = 0; i < vq->num_buffers; ++i) + if (vq->bufs[i]->state == VB2_BUF_STATE_ACTIVE) + vb2_buffer_done(vq->bufs[i], VB2_BUF_STATE_ERROR); + list_for_each_safe(buf_head, tmp, &priv->capture) list_del_init(buf_head); spin_unlock_irq(&priv->lock); -- 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
[PATCH 1/4] media: rcar_vin: Dont aggressively retire buffers
rcar_vin_videobuf_release() is called once per buffer from the buf_cleanup hook.
There is no need to look up the queue and free all buffers at this point.
Signed-off-by: Ian Molton
Signed-off-by: William Towle
---
drivers/media/platform/soc_camera/rcar_vin.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
b/drivers/media/platform/soc_camera/rcar_vin.c
index e594230..7154500 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -493,17 +493,11 @@ static void rcar_vin_videobuf_release(struct vb2_buffer
*vb)
* to release could be any of the current buffers in use, so
* release all buffers that are in use by HW
*/
- for (i = 0; i < MAX_BUFFER_NUM; i++) {
- if (priv->queue_buf[i]) {
- vb2_buffer_done(priv->queue_buf[i],
- VB2_BUF_STATE_ERROR);
- priv->queue_buf[i] = NULL;
- }
- }
- } else {
- list_del_init(to_buf_list(vb));
+ priv->queue_buf[i] = NULL;
}
+ list_del_init(to_buf_list(vb));
+
spin_unlock_irq(&priv->lock);
}
--
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
[PATCH 3/4] media: rcar_vin: Fix race condition terminating stream
This patch fixes a race condition whereby a frame being captured may generate an
interrupt between requesting capture to halt and freeing buffers.
This condition is exposed by the earlier patch that explicitly calls
vb2_buffer_done() during stop streaming.
The solution is to wait for capture to finish prior to finalising these buffers.
Signed-off-by: Ian Molton
Signed-off-by: William Towle
---
drivers/media/platform/soc_camera/rcar_vin.c | 43 ++--
1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
b/drivers/media/platform/soc_camera/rcar_vin.c
index 06ce705..aeda4e2 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -455,6 +455,29 @@ error:
vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
}
+/*
+ * Wait for capture to stop and all in-flight buffers to be finished with by
+ * the video hardware. This must be called under &priv->lock
+ *
+ */
+static void rcar_vin_wait_stop_streaming(struct rcar_vin_priv *priv)
+{
+ while (priv->state != STOPPED) {
+
+ /* issue stop if running */
+ if (priv->state == RUNNING)
+ rcar_vin_request_capture_stop(priv);
+
+ /* wait until capturing has been stopped */
+ if (priv->state == STOPPING) {
+ priv->request_to_stop = true;
+ spin_unlock_irq(&priv->lock);
+ wait_for_completion(&priv->capture_stop);
+ spin_lock_irq(&priv->lock);
+ }
+ }
+}
+
static void rcar_vin_videobuf_release(struct vb2_buffer *vb)
{
struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
@@ -462,7 +485,6 @@ static void rcar_vin_videobuf_release(struct vb2_buffer *vb)
struct rcar_vin_priv *priv = ici->priv;
unsigned int i;
int buf_in_use = 0;
-
spin_lock_irq(&priv->lock);
/* Is the buffer in use by the VIN hardware? */
@@ -474,20 +496,8 @@ static void rcar_vin_videobuf_release(struct vb2_buffer
*vb)
}
if (buf_in_use) {
- while (priv->state != STOPPED) {
-
- /* issue stop if running */
- if (priv->state == RUNNING)
- rcar_vin_request_capture_stop(priv);
-
- /* wait until capturing has been stopped */
- if (priv->state == STOPPING) {
- priv->request_to_stop = true;
- spin_unlock_irq(&priv->lock);
- wait_for_completion(&priv->capture_stop);
- spin_lock_irq(&priv->lock);
- }
- }
+ rcar_vin_wait_stop_streaming(priv);
+
/*
* Capturing has now stopped. The buffer we have been asked
* to release could be any of the current buffers in use, so
@@ -517,12 +527,15 @@ static void rcar_vin_stop_streaming(struct vb2_queue *vq)
spin_lock_irq(&priv->lock);
+ rcar_vin_wait_stop_streaming(priv);
+
for (i = 0; i < vq->num_buffers; ++i)
if (vq->bufs[i]->state == VB2_BUF_STATE_ACTIVE)
vb2_buffer_done(vq->bufs[i], VB2_BUF_STATE_ERROR);
list_for_each_safe(buf_head, tmp, &priv->capture)
list_del_init(buf_head);
+
spin_unlock_irq(&priv->lock);
}
--
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
[PATCH 4/4] media: rcar_vin: Clean up rcar_vin_irq
This patch makes the rcar_vin IRQ handler a little more readable.
Removes an else clause, and simplifies the buffer handling.
Signed-off-by: Ian Molton
Reviewed-by: William Towle
---
drivers/media/platform/soc_camera/rcar_vin.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
b/drivers/media/platform/soc_camera/rcar_vin.c
index aeda4e2..a8d2785 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -554,7 +554,6 @@ static irqreturn_t rcar_vin_irq(int irq, void *data)
struct rcar_vin_priv *priv = data;
u32 int_status;
bool can_run = false, hw_stopped;
- int slot;
unsigned int handled = 0;
spin_lock(&priv->lock);
@@ -573,17 +572,22 @@ static irqreturn_t rcar_vin_irq(int irq, void *data)
hw_stopped = !(ioread32(priv->base + VNMS_REG) & VNMS_CA);
if (!priv->request_to_stop) {
+ struct vb2_buffer **q_entry = priv->queue_buf;
+ struct vb2_buffer *vb;
+
if (is_continuous_transfer(priv))
- slot = (ioread32(priv->base + VNMS_REG) &
- VNMS_FBS_MASK) >> VNMS_FBS_SHIFT;
- else
- slot = 0;
+ q_entry += (ioread32(priv->base + VNMS_REG) &
+ VNMS_FBS_MASK) >> VNMS_FBS_SHIFT;
+
+ vb = *q_entry;
+
+ vb->v4l2_buf.field = priv->field;
+ vb->v4l2_buf.sequence = priv->sequence++;
+ do_gettimeofday(&vb->v4l2_buf.timestamp);
+
+ vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
- priv->queue_buf[slot]->v4l2_buf.field = priv->field;
- priv->queue_buf[slot]->v4l2_buf.sequence = priv->sequence++;
- do_gettimeofday(&priv->queue_buf[slot]->v4l2_buf.timestamp);
- vb2_buffer_done(priv->queue_buf[slot], VB2_BUF_STATE_DONE);
- priv->queue_buf[slot] = NULL;
+ *q_entry = NULL;
if (priv->state != STOPPING)
can_run = rcar_vin_fill_hw_slot(priv);
--
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
RFC: soc_camera, rcar_vin, and adv7604
Hi folks, My colleague and I are trying to work out what to do to support the following combination: soc_camera + rcar_vin for capture, and the mainline adv7604 driver (which we have modified to successfully drive the adv7612). The problem we face is that the 7604 driver uses the new "pads" API, but soc_camera based drivers like rcar_vin do not. Obviously, there are a few approaches we could take, but we could use some guidance on this. One approach would be to bodge some non-pads older API support into the 7604 driver. This would probably be the easiest solution. A better approach might be to add pad API support to soc_camera, but it seems to me that the soc_camera API does not abstract away all of the areas that might need to be touched, which would lead to much pad-related churn in all the other soc_camera drivers. The codebase is rather large, and we're struggling to see a clear path through this. Whatever we do, we would like to be acceptable upstream, so we'd like to open a discussion. Perhaps a soc_camera2 with pads support? -- Ian Molton -- 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: soc_camera, rcar_vin, and adv7604
On Wed, 9 Jul 2014 22:34:07 +0200 (CEST) Guennadi Liakhovetski wrote: > Maybe we dould add some support, say, to > help with (fake) file handles just to aid the transition. Indeed - the filehandles are probably the biggest sticking point. I already have the soc_camera code able to select mutually acceptable data formats, but the calls to get/set resolution seem to use fh's I will persist with approach 2 then for now. -- Ian Molton -- 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 3/4] media: rcar_vin: Fix race condition terminating stream
On Tue, 08 Jul 2014 20:09:58 +0400 Sergei Shtylyov wrote: > Hello. Hi, > > Signed-off-by: Ian Molton > > Signed-off-by: William Towle > > --- > > drivers/media/platform/soc_camera/rcar_vin.c | 43 > > ++-- > > 1 file changed, 28 insertions(+), 15 deletions(-) > > > diff --git a/drivers/media/platform/soc_camera/rcar_vin.c > > b/drivers/media/platform/soc_camera/rcar_vin.c > > index 06ce705..aeda4e2 100644 > > --- a/drivers/media/platform/soc_camera/rcar_vin.c > > +++ b/drivers/media/platform/soc_camera/rcar_vin.c > [...] > > @@ -462,7 +485,6 @@ static void rcar_vin_videobuf_release(struct vb2_buffer > > *vb) > > struct rcar_vin_priv *priv = ici->priv; > > unsigned int i; > > int buf_in_use = 0; > > - > > spin_lock_irq(&priv->lock); > > This seems like a random whitespace change. This empty should be present. Agreed. > [...] > > @@ -517,12 +527,15 @@ static void rcar_vin_stop_streaming(struct vb2_queue > > *vq) > > > > spin_lock_irq(&priv->lock); > > > > + rcar_vin_wait_stop_streaming(priv); > > + > > for (i = 0; i < vq->num_buffers; ++i) > > if (vq->bufs[i]->state == VB2_BUF_STATE_ACTIVE) > > vb2_buffer_done(vq->bufs[i], VB2_BUF_STATE_ERROR); > > > > list_for_each_safe(buf_head, tmp, &priv->capture) > > list_del_init(buf_head); > > + > > Also quite a random "drove-by" change. Agreed. Any further comments? If not, I can re-spin this ready for upstreaming. -- Ian Molton -- 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
