RE: [PATCH 8/8] ARM: S5PV210: example of CMA private area for FIMC device on Goni board
Hello, On Wednesday, August 24, 2011 5:41 PM Aguirre, Sergio wrote: > On Fri, Aug 19, 2011 at 04:27:44PM +0200, Marek Szyprowski wrote: > > This patch is an example how device private CMA area can be activated. > > It creates one CMA region and assigns it to the first s5p-fimc device on > > Samsung Goni S5PC110 board. > > > > Signed-off-by: Marek Szyprowski > > Signed-off-by: Kyungmin Park > > --- > > arch/arm/mach-s5pv210/mach-goni.c |4 > > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/mach-s5pv210/mach-goni.c b/arch/arm/mach-s5pv210/mach- > goni.c > > index 14578f5..f766c45 100644 > > --- a/arch/arm/mach-s5pv210/mach-goni.c > > +++ b/arch/arm/mach-s5pv210/mach-goni.c > > @@ -26,6 +26,7 @@ > > #include > > #include > > #include > > +#include > > > > #include > > #include > > @@ -857,6 +858,9 @@ static void __init goni_map_io(void) > > static void __init goni_reserve(void) > > { > > s5p_mfc_reserve_mem(0x4300, 8 << 20, 0x5100, 8 << 20); > > + > > + /* Create private 16MiB contiguous memory area for s5p-fimc.0 device */ > > + dma_declare_contiguous(&s5p_device_fimc0.dev, 16*SZ_1M, 0); > > This is broken, since according to patch #0006, dma_declare_contiguous requires > a 4th param (limit) which you're not providing here. You are definitely right, there should be one more parameter. This patch was just cherry-picked from older version just before posting to mailing lists. I'm really sorry for this trivial bug. Best regards -- Marek Szyprowski Samsung Poland R&D Center -- 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: Bug#639161: linux-image-3.0.0-1-686-pae: Upgrade 2.6.39 -> 3.0.0 breaks playback on DiBcom 7000PC
Hi, Afaik, this is fixed with those commits: [media] dib0700: correct error message for_v3.0 [media] dib0700: protect the dib0700 buffer access [media] DiBcom: protect the I2C bufer access from http://git.linuxtv.org/pb/media_tree.git/shortlog/refs/heads/for_v3.0 A pull request has been sent to the media-maintainer, but it seems he is busy. -- Patrick. On Thursday 25 August 2011 00:50:37 Ben Hutchings wrote: Patrick, Please could you take a look at the following bug report on Linux 3.0 as packaged in Debian. Ben. On Wed, 2011-08-24 at 18:59 +0200, Soeren D. Schulze wrote: > Package: linux-2.6 > Version: 3.0.0-1 > Severity: normal > > I usually use tzap/mplayer for TV playback. > > After the upgrade to Linux 3.0.0, tzap command line output still looks > fine, but mplayer does not seem to receive any data (its cache does not > fill up). > > syslog/dmesg output looks the same as in 2.6.39. On the first try to > tune, dmesg receives: > > dib0700: tx buffer length is larger than 4. Not supported. > (for which I find various non-Debian bug reports) > > But that does not seem to be the issue, because the same message appears > in 2.6.39, where everything is fine. > So I do not really have an idea what the problem is, but I certainly > know that it's a regression, because simply booting Linux 2.6.39 rather > than 3.0.0 on the same system avoids the problem. > > -- Package-specific info: > ** Version: > Linux version 3.0.0-1-686-pae (Debian 3.0.0-1) (b...@decadent.org.uk) > (gcc version 4.5.3 (Debian 4.5.3-3) ) #1 SMP Sun Jul 24 14:27:32 UTC 2011 > > ** Command line: > BOOT_IMAGE=/vmlinuz-3.0.0-1-686-pae > root=UUID=3aa0a731-df46-486e-9c1e-258723e14f8f ro > > ** Not tainted > > ** Kernel log: > [ 11.031446] saa7134: card=172 -> RoverMedia TV Link Pro FM >19d1:0138 > [ 11.031580] saa7134: card=173 -> Zolid Hybrid TV Tuner PCI >1131:2004 > [ 11.031713] saa7134: card=174 -> Asus Europa Hybrid OEM >1043:4847 > [ 11.031847] saa7134: card=175 -> Leadtek Winfast DTV1000S >107d:6655 > [ 11.031982] saa7134: card=176 -> Beholder BeholdTV 505 RDS >:5051 > [ 11.032126] saa7134: card=177 -> Hawell HW-404M7 > > [ 11.032217] saa7134: card=178 -> Beholder BeholdTV H7 >5ace:7190 > [ 11.032351] saa7134: card=179 -> Beholder BeholdTV A7 >5ace:7090 > [ 11.032485] saa7134: card=180 -> Avermedia PCI M733A >1461:4155 1461:4255 > [ 11.032656] saa7134: card=181 -> TechoTrend TT-budget T-3000 >13c2:2804 > [ 11.032789] saa7134: card=182 -> Kworld PCI SBTVD/ISDB-T Full-Seg > Hybrid 17de:b136 > [ 11.032923] saa7134: card=183 -> Compro VideoMate Vista M1F >185b:c900 > [ 11.033057] saa7134: card=184 -> Encore ENLTV-FM 3 >1a7f:2108 > [ 11.033192] saa7134: card=185 -> MagicPro ProHDTV Pro2 > DMB-TH/Hybrid 17de:d136 > [ 11.033326] saa7134: card=186 -> Beholder BeholdTV 501 >5ace:5010 > [ 11.033460] saa7134: card=187 -> Beholder BeholdTV 503 FM >5ace:5030 > [ 11.033596] saa7134[0]: subsystem: 1131:, board: UNKNOWN/GENERIC > [card=0,autodetected] > [ 11.075242] IR NEC protocol handler initialized > [ 11.265597] saa7134[0]: board init: gpio is 10020 > [ 11.368582] saa7134[0]: Huh, no eeprom present (err=-5)? > [ 11.381924] saa7134[0]: registered device video0 [v4l2] > [ 11.382055] saa7134[0]: registered device vbi0 > [ 11.417515] IR RC5(x) protocol handler initialized > [ 11.607051] cfg80211: Calling CRDA to update world regulatory domain > [ 11.995773] IR RC6 protocol handler initialized > [ 12.191500] IR JVC protocol handler initialized > [ 12.263839] dib0700: loaded with support for 20 different device-types > [ 12.264209] dvb-usb: found a 'Hauppauge Nova-T Stick' in warm state. > [ 12.265499] dvb-usb: will pass the complete MPEG2 transport stream to > the software demuxer. > [ 12.266158] DVB: registering new adapter (Hauppauge Nova-T Stick) > [ 12.517093] DVB: registering adapter 0 frontend 0 (DiBcom 7000PC)... > [ 12.609760] IR Sony protocol handler initialized > [ 12.790869] DiB0070: successfully identified > [ 12.907896] ACPI: PCI Interrupt Link [ALKC] enabled at IRQ 22 > [ 12.907988] VIA 82xx Audio :00:11.5: PCI INT C -> Link[ALKC] -> > GSI 22 (level, low) -> IRQ 22 > [ 12.908316] VIA 82xx Audio :00:11.5: setting latency timer to 64 > [ 12.932675] saa7134 ALSA driver for DMA sound loaded > [ 12.932807] saa7134[0]/alsa: saa7134[0] at 0xfdffc000 irq 16 > registered as card -1 > [ 12.948789] lirc_dev: IR Remote Control driver registered, major 249 > [ 12.952109] IR LIRC bridge handler initialized > [ 13.380033] Registered IR keymap rc-dib0700-rc5 > [ 13.380514] input: IR-receiver inside an USB DVB receiver as > /devices/pci:00/:00:10.4/usb1/1-1/1-1.3/rc/rc0/input6 > [ 13.380749] rc0: IR-receiver inside an USB DVB receiver as > /devices/pci:00/:00:10.4/usb1/1-1/1-1.3/rc/rc0 > [
[PATCH v2/RFC] media: vb2: change queue initialization order
This patch changes the order of operations during stream on call. Now the buffers are first queued to the driver and then the start_streaming method is called. This resolves the most common case when the driver needs to know buffer addresses to enable dma engine and start streaming. Additional parameters to start_streaming and buffer_queue methods have been added to simplify drivers code. The driver are now obliged to check if the number of queued buffers is enough to enable hardware streaming. If not - it should return an error. In such case all the buffers that have been pre-queued are invalidated. Drivers that are able to start/stop streaming on-fly, can control dma engine directly in buf_queue callback. In this case start_streaming callback can be considered as optional. The driver can also assume that after a few first buf_queue calls with zero 'streaming' parameter, the core will finally call start_streaming callback. This patch also updates some videobuf2 clients (s5p-fimc, s5p-mfc, s5p-tv, mem2mem_testdev and vivi) to work properly with the changed order of operations. Signed-off-by: Marek Szyprowski Signed-off-by: Kyungmin Park CC: Pawel Osciak --- drivers/media/video/mem2mem_testdev.c |2 +- drivers/media/video/s5p-fimc/fimc-capture.c | 66 --- drivers/media/video/s5p-fimc/fimc-core.c|2 +- drivers/media/video/s5p-mfc/s5p_mfc_dec.c |4 +- drivers/media/video/s5p-tv/mixer.h |2 - drivers/media/video/s5p-tv/mixer_video.c| 24 +- drivers/media/video/videobuf2-core.c| 30 + drivers/media/video/vivi.c |4 +- include/media/videobuf2-core.h | 18 +-- 9 files changed, 82 insertions(+), 70 deletions(-) --- Hello, This patch introduces significant changes in the vb2 streamon operation, so all vb2 clients need to be checked and updated. Right now I only updated virtual and Samsung drivers. Once we agree that this patch can be merged, I will update it to include all the required changes for all videobuf2 clients as well. Best regards -- Marek Szyprowski Samsung Poland R&D Center diff --git a/drivers/media/video/mem2mem_testdev.c b/drivers/media/video/mem2mem_testdev.c index 0d0c0d5..0a79c86 100644 --- a/drivers/media/video/mem2mem_testdev.c +++ b/drivers/media/video/mem2mem_testdev.c @@ -787,7 +787,7 @@ static int m2mtest_buf_prepare(struct vb2_buffer *vb) return 0; } -static void m2mtest_buf_queue(struct vb2_buffer *vb) +static void m2mtest_buf_queue(struct vb2_buffer *vb, bool streaming) { struct m2mtest_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); v4l2_m2m_buf_queue(ctx->m2m_ctx, vb); diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c b/drivers/media/video/s5p-fimc/fimc-capture.c index e6afe5f..5f80fb4 100644 --- a/drivers/media/video/s5p-fimc/fimc-capture.c +++ b/drivers/media/video/s5p-fimc/fimc-capture.c @@ -151,27 +151,11 @@ static int fimc_isp_subdev_init(struct fimc_dev *fimc, unsigned int index) return ret; } -static int fimc_stop_capture(struct fimc_dev *fimc) +static void fimc_capture_state_cleanup(struct fimc_dev *fimc) { - unsigned long flags; - struct fimc_vid_cap *cap; + struct fimc_vid_cap *cap = &fimc->vid_cap; struct fimc_vid_buffer *buf; - - cap = &fimc->vid_cap; - - if (!fimc_capture_active(fimc)) - return 0; - - spin_lock_irqsave(&fimc->slock, flags); - set_bit(ST_CAPT_SHUT, &fimc->state); - fimc_deactivate_capture(fimc); - spin_unlock_irqrestore(&fimc->slock, flags); - - wait_event_timeout(fimc->irq_queue, - !test_bit(ST_CAPT_SHUT, &fimc->state), - FIMC_SHUTDOWN_TIMEOUT); - - v4l2_subdev_call(cap->sd, video, s_stream, 0); + unsigned long flags; spin_lock_irqsave(&fimc->slock, flags); fimc->state &= ~(1 << ST_CAPT_RUN | 1 << ST_CAPT_PEND | @@ -191,27 +175,50 @@ static int fimc_stop_capture(struct fimc_dev *fimc) } spin_unlock_irqrestore(&fimc->slock, flags); +} + +static int fimc_stop_capture(struct fimc_dev *fimc) +{ + struct fimc_vid_cap *cap = &fimc->vid_cap; + unsigned long flags; + + if (!fimc_capture_active(fimc)) + return 0; + + spin_lock_irqsave(&fimc->slock, flags); + set_bit(ST_CAPT_SHUT, &fimc->state); + fimc_deactivate_capture(fimc); + spin_unlock_irqrestore(&fimc->slock, flags); + + wait_event_timeout(fimc->irq_queue, + !test_bit(ST_CAPT_SHUT, &fimc->state), + FIMC_SHUTDOWN_TIMEOUT); + v4l2_subdev_call(cap->sd, video, s_stream, 0); + + fimc_capture_state_cleanup(fimc); dbg("state: 0x%lx", fimc->state); return 0; } -static int start_streaming(struct vb2_queue *q) + +static int start_streaming(struct vb2_queue *q, int count) { struct fimc_ctx *ctx = q-
Re: [PATCH v2/RFC] media: vb2: change queue initialization order
On Thursday, August 25, 2011 12:52:11 Marek Szyprowski wrote: > This patch changes the order of operations during stream on call. Now the > buffers are first queued to the driver and then the start_streaming method > is called. > > This resolves the most common case when the driver needs to know buffer > addresses to enable dma engine and start streaming. Additional parameters > to start_streaming and buffer_queue methods have been added to simplify > drivers code. The driver are now obliged to check if the number of queued > buffers is enough to enable hardware streaming. If not - it should return > an error. In such case all the buffers that have been pre-queued are > invalidated. > > Drivers that are able to start/stop streaming on-fly, can control dma > engine directly in buf_queue callback. In this case start_streaming > callback can be considered as optional. The driver can also assume that > after a few first buf_queue calls with zero 'streaming' parameter, the core > will finally call start_streaming callback. Looks good! > This patch also updates some videobuf2 clients (s5p-fimc, s5p-mfc, s5p-tv, > mem2mem_testdev and vivi) to work properly with the changed order of > operations. I assume the final patch will update all vb2 clients? I have a few very minor comments below: > > Signed-off-by: Marek Szyprowski > Signed-off-by: Kyungmin Park > CC: Pawel Osciak > --- > drivers/media/video/mem2mem_testdev.c |2 +- > drivers/media/video/s5p-fimc/fimc-capture.c | 66 > --- > drivers/media/video/s5p-fimc/fimc-core.c|2 +- > drivers/media/video/s5p-mfc/s5p_mfc_dec.c |4 +- > drivers/media/video/s5p-tv/mixer.h |2 - > drivers/media/video/s5p-tv/mixer_video.c| 24 +- > drivers/media/video/videobuf2-core.c| 30 + > drivers/media/video/vivi.c |4 +- > include/media/videobuf2-core.h | 18 +-- > 9 files changed, 82 insertions(+), 70 deletions(-) > --- > > Hello, > > This patch introduces significant changes in the vb2 streamon operation, > so all vb2 clients need to be checked and updated. Right now I only > updated virtual and Samsung drivers. Once we agree that this patch can > be merged, I will update it to include all the required changes for all > videobuf2 clients as well. > > Best regards > > Marek Szyprowski > Samsung Poland R&D Center > > diff --git a/drivers/media/video/videobuf2-core.c > b/drivers/media/video/videobuf2-core.c > index e89fd53..283c6ce 100644 > --- a/drivers/media/video/videobuf2-core.c > +++ b/drivers/media/video/videobuf2-core.c > @@ -823,13 +823,13 @@ static int __qbuf_mmap(struct vb2_buffer *vb, struct > v4l2_buffer *b) > /** > * __enqueue_in_driver() - enqueue a vb2_buffer in driver for processing > */ > -static void __enqueue_in_driver(struct vb2_buffer *vb) > +static void __enqueue_in_driver(struct vb2_buffer *vb, bool streaming) > { > struct vb2_queue *q = vb->vb2_queue; > > vb->state = VB2_BUF_STATE_ACTIVE; > atomic_inc(&q->queued_count); > - q->ops->buf_queue(vb); > + q->ops->buf_queue(vb, streaming); > } > > /** > @@ -916,7 +916,7 @@ int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b) > * If not, the buffer will be given to driver on next streamon. > */ > if (q->streaming) > - __enqueue_in_driver(vb); > + __enqueue_in_driver(vb, 1); Use 'true' instead of 1. > > dprintk(1, "qbuf of buffer %d succeeded\n", vb->v4l2_buf.index); > return 0; > @@ -1110,6 +1110,8 @@ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer > *b, bool nonblocking) > } > EXPORT_SYMBOL_GPL(vb2_dqbuf); > > +static void __vb2_queue_cancel(struct vb2_queue *q); > + Is it possible to move __vb2_queue_cancel forward instead of having to add a forward declaration? In general you don't want forward declarations unless you have some sort of circular dependency. > /** > * vb2_streamon - start streaming > * @q: videobuf2 queue > @@ -1144,34 +1146,24 @@ int vb2_streamon(struct vb2_queue *q, enum > v4l2_buf_type type) > } > > /* > -* Cannot start streaming on an OUTPUT device if no buffers have > -* been queued yet. > +* If any buffers were queued before streamon, > +* we can now pass them to driver for processing. > */ > - if (V4L2_TYPE_IS_OUTPUT(q->type)) { > - if (list_empty(&q->queued_list)) { > - dprintk(1, "streamon: no output buffers queued\n"); > - return -EINVAL; > - } > - } > + list_for_each_entry(vb, &q->queued_list, queued_entry) > + __enqueue_in_driver(vb, 0); And 'false' instead of 0. > > /* > * Let driver notice that streaming state has been enabled. > */ > - ret = call_qop(q, start_streaming, q); > +
[PATCH v2] s5p-csis: Handle all available power supplies
On the SoCs this driver is intended to support the are three separate pins to supply the MIPI-CSIS subsystem: 1.1V or 1.2V, 1.8V and power supply for an internal PLL. This patch adds support for two separate voltage supplies to cover properly board configurations where PMIC requires to configure independently each external supply of the MIPI-CSI device. The 1.8V and PLL supply are assigned a single "vdd18" regulator supply name as it seems more reasonable than creating separate regulator supplies for them. While at here stop using the 'fixed_phy_vdd' platform_data field. It has been introduced for boards where the MIPI-CSIS supplies are not controllable. However it is not needed as those boards can use the dummy regulator. Signed-off-by: Sylwester Nawrocki Signed-off-by: Kyungmin Park --- There is a minor change introduced in this patch comparing to the first version - the not really needed fixed_phy_vdd platform data property is killed which simplifies the code a bit. The struct s5p_platform_mipi_csis definition will be cleaned in a subsequent patch. --- drivers/media/video/s5p-fimc/mipi-csis.c | 49 -- 1 files changed, 26 insertions(+), 23 deletions(-) diff --git a/drivers/media/video/s5p-fimc/mipi-csis.c b/drivers/media/video/s5p-fimc/mipi-csis.c index ef056d6..e34d4ba 100644 --- a/drivers/media/video/s5p-fimc/mipi-csis.c +++ b/drivers/media/video/s5p-fimc/mipi-csis.c @@ -81,6 +81,12 @@ static char *csi_clock_name[] = { }; #define NUM_CSIS_CLOCKSARRAY_SIZE(csi_clock_name) +static const char * const csis_supply_name[] = { + "vdd11", /* 1.1V or 1.2V (s5pc100) MIPI CSI suppply */ + "vdd18", /* VDD 1.8V and MIPI CSI PLL supply */ +}; +#define CSIS_NUM_SUPPLIES ARRAY_SIZE(csis_supply_name) + enum { ST_POWERED = 1, ST_STREAMING= 2, @@ -109,9 +115,9 @@ struct csis_state { struct platform_device *pdev; struct resource *regs_res; void __iomem *regs; + struct regulator_bulk_data supplies[CSIS_NUM_SUPPLIES]; struct clk *clock[NUM_CSIS_CLOCKS]; int irq; - struct regulator *supply; u32 flags; const struct csis_pix_format *csis_fmt; struct v4l2_mbus_framefmt format; @@ -460,6 +466,7 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev) struct resource *regs_res; struct csis_state *state; int ret = -ENOMEM; + int i; state = kzalloc(sizeof(*state), GFP_KERNEL); if (!state) @@ -519,14 +526,13 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev) goto e_clkput; } - if (!pdata->fixed_phy_vdd) { - state->supply = regulator_get(&pdev->dev, "vdd"); - if (IS_ERR(state->supply)) { - ret = PTR_ERR(state->supply); - state->supply = NULL; - goto e_clkput; - } - } + for (i = 0; i < CSIS_NUM_SUPPLIES; i++) + state->supplies[i].supply = csis_supply_name[i]; + + ret = regulator_bulk_get(&pdev->dev, CSIS_NUM_SUPPLIES, +state->supplies); + if (ret) + goto e_clkput; ret = request_irq(state->irq, s5pcsis_irq_handler, 0, dev_name(&pdev->dev), state); @@ -561,8 +567,7 @@ static int __devinit s5pcsis_probe(struct platform_device *pdev) e_irqfree: free_irq(state->irq, state); e_regput: - if (state->supply) - regulator_put(state->supply); + regulator_bulk_free(CSIS_NUM_SUPPLIES, state->supplies); e_clkput: clk_disable(state->clock[CSIS_CLK_MUX]); s5pcsis_clk_put(state); @@ -592,11 +597,10 @@ static int s5pcsis_suspend(struct device *dev) ret = pdata->phy_enable(state->pdev, false); if (ret) goto unlock; - if (state->supply) { - ret = regulator_disable(state->supply); - if (ret) - goto unlock; - } + ret = regulator_bulk_disable(CSIS_NUM_SUPPLIES, +state->supplies); + if (ret) + goto unlock; clk_disable(state->clock[CSIS_CLK_GATE]); state->flags &= ~ST_POWERED; } @@ -622,16 +626,16 @@ static int s5pcsis_resume(struct device *dev) goto unlock; if (!(state->flags & ST_POWERED)) { - if (state->supply) - ret = regulator_enable(state->supply); + ret = regulator_bulk_enable(CSIS_NUM_SUPPLIES, + state->supplies); if (ret) goto unlock; - ret = pdata->phy_enable(state->pdev, true); if (!ret) { state->flags |= S
Re: [PATCH 04/14] [media] winbond-cir: Use current logging styles
Acked-by: David Härdeman On Sun, 21 Aug 2011 15:56:47 -0700, Joe Perches wrote: > Add pr_fmt, convert printks to pr_. > > Signed-off-by: Joe Perches > --- > drivers/media/rc/winbond-cir.c |6 -- > 1 files changed, 4 insertions(+), 2 deletions(-) -- 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/RFC] media: vb2: change queue initialization order
Hello, On Thursday, August 25, 2011 1:12 PM Hans Verkuil wrote: > On Thursday, August 25, 2011 12:52:11 Marek Szyprowski wrote: > > This patch changes the order of operations during stream on call. Now the > > buffers are first queued to the driver and then the start_streaming method > > is called. > > > > This resolves the most common case when the driver needs to know buffer > > addresses to enable dma engine and start streaming. Additional parameters > > to start_streaming and buffer_queue methods have been added to simplify > > drivers code. The driver are now obliged to check if the number of queued > > buffers is enough to enable hardware streaming. If not - it should return > > an error. In such case all the buffers that have been pre-queued are > > invalidated. > > > > Drivers that are able to start/stop streaming on-fly, can control dma > > engine directly in buf_queue callback. In this case start_streaming > > callback can be considered as optional. The driver can also assume that > > after a few first buf_queue calls with zero 'streaming' parameter, the core > > will finally call start_streaming callback. > > Looks good! > > > This patch also updates some videobuf2 clients (s5p-fimc, s5p-mfc, s5p-tv, > > mem2mem_testdev and vivi) to work properly with the changed order of > > operations. > > I assume the final patch will update all vb2 clients? Yes, of cource. I just wanted to post the patch ASAP. Updating Samsung and virtual drivers was easy because I already know a bit about them. Updating other drivers requires a bit more work to double check if I didn't break anything. > I have a few very minor comments below: Thanks for your comments :) I will include them in the final version. (snipped) Best regards -- Marek Szyprowski Samsung Poland R&D Center -- 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
radio-si470x-usb.c warning: can I remove 'buf'?
Hi Tobias, While going through the compile warnings generated in the daily build I came across this one: v4l-dvb-git/drivers/media/radio/si470x/radio-si470x-usb.c: In function 'si470x_int_in_callback': v4l-dvb-git/drivers/media/radio/si470x/radio-si470x-usb.c:398:16: warning: variable 'buf' set but not used [-Wunused-but-set-variable] The 'unsigned char buf[RDS_REPORT_SIZE];' is indeed unused, but can I just remove it? There is this single assignment to buf: 'buf[0] = RDS_REPORT;'. This makes me wonder if it is perhaps supposed to be used after all. Please let me know if I can remove it, or if it is a bug that someone needs to fix. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dma buffers for camera
Hi Guennadi, The mx2_camera driver is allocating one "discard" buffer of the same size, as regular buffers for cases, when the user is not fast enough to queue new buffers for the running capture. Arguably, this could be aliminated and the last submitted buffer could be re-used until either more buffers are available or the streaming is stopped. Otherwise, it could also be possible to stop capture until buffers are available again. In any case, this is the current driver implementation. As for 2 buffers instead of one for the actual capture, I think, gstreamer defines 2 as a minimum number of buffers, which is actually also required for any streaming chance. Thank you for clarification... regards Jan -- Tato zpráva byla vytvořena převratným poštovním klientem Opery: http://www.opera.com/mail/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2/RFC] media: vb2: change queue initialization order
On Thu, 25 Aug 2011, Hans Verkuil wrote: > On Thursday, August 25, 2011 12:52:11 Marek Szyprowski wrote: [snip] > > @@ -1110,6 +1110,8 @@ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer > > *b, bool nonblocking) > > } > > EXPORT_SYMBOL_GPL(vb2_dqbuf); > > > > +static void __vb2_queue_cancel(struct vb2_queue *q); > > + > > Is it possible to move __vb2_queue_cancel forward instead of having to add a > forward declaration? In general you don't want forward declarations unless > you have some sort of circular dependency. IMHO, adding a forward declaration has the advantages of making the patch smaller and showing clearly, that the function has not changed, or making any changes directly visible. If such forward declarations should really be avoided, moving of affected functions could be done in a separate patch, clearly stating, that the function contents have not changed. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Streaming analog video with Leadtek Winfast PxDVR3200 H XC4000
Hello everybody, Following steps of http://istvanv.users.sourceforge.net/v4l/xc4000.html I have been able to receive analog input (composite) and watch it with VLC. Now, I am interested in streaming the MPEG-2 stream generated by the cx23417 encoder chip included with the card. Is this possible with this card Leadtek Winfast PxDVR3200 H XC4000? How could I setup the card for outputing MPEG-2 stream? Thank you very much in advance. Gonzalo Berdeal. -- 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: Embedded device and the V4L2 API support - Was: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 driver updates
Em 24-08-2011 19:29, Sakari Ailus escreveu: > Hi Mauro, > > On Sat, Aug 20, 2011 at 05:12:56AM -0700, Mauro Carvalho Chehab wrote: >> Em 20-08-2011 04:27, Sylwester Nawrocki escreveu: >>> Hi Mauro, >>> >>> On 08/17/2011 08:13 AM, Mauro Carvalho Chehab wrote: It seems that there are too many miss understandings or maybe we're just talking the same thing on different ways. So, instead of answering again, let's re-start this discussion on a different way. One of the requirements that it was discussed a lot on both mailing lists and on the Media Controllers meetings that we had (or, at least in the ones where I've participated) is that: "A pure V4L2 userspace application, knowing about video device nodes only, can still use the driver. Not all advanced features will be available." >>> >>> What does a term "a pure V4L2 userspace application" mean here ? >> >> The above quotation are exactly the Laurent's words that I took from one >> of his replies. > > I would define this as an application which uses V4L2 but does not use Media > controller or the V4L2 subdev interfaces nor is aware of any particular > hardware device. As a general rule, applications should not be aware of any particular hardware. That's why we provide standard ways of doing things. Experience shows that hardware aware applications become obsolete very fast. If you seek at the net, you'll see application-aware tools for bttv, zoran, etc. I doubt that they would still work today, as they were dependent on some particular special case driver behavior that has changed among the time. Also, if you look of the updates timeline for those, you'll see that they were kept maintained for a short period of time. So, I think that all hardware aware dependency should be at the driver and at the libv4l only. As the proper usage of the MC API requires a hardware aware knowledge, I don't think that a generic application should bother to implement the MC API at all. It should be noticed, however, that having a hardware/driver aware libv4l also implies that libv4l should be dependent on an specific kernel version, at the distributions. This already happens somehow there, but, currently, a new version of libv4l can work with an older kernel (as all we currently have there is support for new FOURCC formats and quirk lists of sensors mounted upside down). So, a version made to work with kernel 3.0 will for sure support all webcams found at kernel 2.39. >>> Does it also account an application which is linked to libv4l2 and uses >>> calls specific to a particular hardware which are included there? >> >> That's a good question. We need to properly define what it means, to avoid >> having libv4l abuses. >> >> In other words, it seems ok to use libv4l to set pipelines via the MC API >> at open(), but it isn't ok to have an open() binary only libv4l plugin that >> will hook open and do the complete device initialization on userspace >> (I remember that one vendor once proposed a driver like that). >> >> Also, from my side, I'd like to see both libv4l and kernel drivers being >> submitted together, if the new driver depends on a special libv4l support >> for it to work. > > I agree with the above. > > I do favour using libv4l to do the pipeline setup using MC and V4L2 subdev > interfaces. This has the benefit that the driver provides just one interface > to access different aspects of it, be it pipeline setup (Media controller), > a control to change exposure time (V4L2 subdev) or queueing video buffer > (V4L2). This means more simple and more maintainable drivers and less bugs > in general. I agree. > Apart from what the drivers already provide on video nodea, to support a > general purpose V4L2 application, libv4l plugin can do is (not exhaustive > list): IMO, we need to write a full list of what should be done at libv4l, as it seems that there are different opinions about what should be at the driver, and what should be outside it. > - Perform pipeline setup using MC interface, possibly based on input > selected using S_INPUT so that e.g. multiple sensors can be supported and > - implement {S,G,TRY}_EXT_CTRLS and QUERYCTRL using V4L2 subdev nodes as > backend. > > As the Media controller and V4L2 interfaces are standardised, I see no > reason why this plugin could not be fully generic: only the configuration is > device specific. I don't think you can do everything that is needed on a fully generic plugin. 3A algorithm implementations, for example, seems to be device specific. Of course, the maximum we can do to have a generic implementation that fits on most cases, the better. > This configuration could be stored into a configuration file which is > selected based on the system type. On embedded systems (ARMs at least, but > anyway the vast majority is based on ARM) the board type is easily available > for the user space applications in /proc/cpuinfo --- this exam
dst_ca.c warning: can ca_send_message be removed?
Hi Manu, While going through the daily build compiler warnings I came across this one: v4l-dvb-git/drivers/media/dvb/bt8xx/dst_ca.c: In function 'ca_send_message': v4l-dvb-git/drivers/media/dvb/bt8xx/dst_ca.c:480:15: warning: variable 'ca_message_header_len' set but not used [-Wunused-but-set-variable] This variable is indeed set once but unused afterwards. The assignment is: ca_message_header_len = p_ca_message->length; /* Restore it back when you are done */ Obviously it is not restored as the comment says. Is this a bug or just old code that can be removed? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2/RFC] media: vb2: change queue initialization order
On Thu, 25 Aug 2011 12:52:11 +0200 Marek Szyprowski wrote: > This patch changes the order of operations during stream on call. Now the > buffers are first queued to the driver and then the start_streaming method > is called. This seems good to me (I guess it should, since I'm the guy who griped about it before :). > This resolves the most common case when the driver needs to know buffer > addresses to enable dma engine and start streaming. Additional parameters > to start_streaming and buffer_queue methods have been added to simplify > drivers code. The driver are now obliged to check if the number of queued > buffers is enough to enable hardware streaming. If not - it should return > an error. In such case all the buffers that have been pre-queued are > invalidated. I'd suggest that drivers that worked in the old scheme, where the buffers arrived after the start_streaming() call, should continue to work. Why not? > Drivers that are able to start/stop streaming on-fly, can control dma > engine directly in buf_queue callback. In this case start_streaming > callback can be considered as optional. The driver can also assume that > after a few first buf_queue calls with zero 'streaming' parameter, the core > will finally call start_streaming callback. This part I like a bit less. In your patch, almost none of the changed drivers use that parameter. start_streaming() is a significant state change, I don't think it's asking a lot of a driver to provide a callback and actually remember whether it's supposed to be streaming or not. Beyond that, what happens to a driver without a start_streaming() callback if the application first queues all its buffers, then does its VIDIOC_STREAMON call? I see: > + list_for_each_entry(vb, &q->queued_list, queued_entry) > + __enqueue_in_driver(vb, 0); (So we get streaming=0 for all queued buffers). > /* >* Let driver notice that streaming state has been enabled. >*/ > - ret = call_qop(q, start_streaming, q); > + ret = call_qop(q, start_streaming, q, atomic_read(&q->queued_count)); > if (ret) { > dprintk(1, "streamon: driver refused to start streaming\n"); > + __vb2_queue_cancel(q); > return ret; > } The driver will have gotten all of the buffers with streaming=0, then will never get a call again; I don't think that will lead to the desired result. Am I missing something? Thanks, jon -- 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
dvb_frontend.c warning: can 'timeout' be removed?
This is the warning the daily build gives: v4l-dvb-git/drivers/media/dvb/dvb-core/dvb_frontend.c: In function 'dvb_frontend_thread': v4l-dvb-git/drivers/media/dvb/dvb-core/dvb_frontend.c:540:16: warning: variable 'timeout' set but not used [-Wunused-but-set-variable] The 'timeout' variable is indeed not used, but should it? I'm not familiar enough with this code to decide. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2/RFC] media: vb2: change queue initialization order
Hello, On Thursday, August 25, 2011 3:27 PM Jonathan Corbet wrote: > On Thu, 25 Aug 2011 12:52:11 +0200 > Marek Szyprowski wrote: > > > This patch changes the order of operations during stream on call. Now the > > buffers are first queued to the driver and then the start_streaming method > > is called. > > This seems good to me (I guess it should, since I'm the guy who griped > about it before :). > > > This resolves the most common case when the driver needs to know buffer > > addresses to enable dma engine and start streaming. Additional parameters > > to start_streaming and buffer_queue methods have been added to simplify > > drivers code. The driver are now obliged to check if the number of queued > > buffers is enough to enable hardware streaming. If not - it should return > > an error. In such case all the buffers that have been pre-queued are > > invalidated. > > I'd suggest that drivers that worked in the old scheme, where the buffers > arrived after the start_streaming() call, should continue to work. Why > not? Such change might have some side effect - the logic in buf_queue usually assumed that the code from start_streaming has been called earlier, so some additional checks or changes might be needed. > > Drivers that are able to start/stop streaming on-fly, can control dma > > engine directly in buf_queue callback. In this case start_streaming > > callback can be considered as optional. The driver can also assume that > > after a few first buf_queue calls with zero 'streaming' parameter, the core > > will finally call start_streaming callback. > > This part I like a bit less. In your patch, almost none of the changed > drivers use that parameter. start_streaming() is a significant state > change, I don't think it's asking a lot of a driver to provide a callback > and actually remember whether it's supposed to be streaming or not. I would like to get some more comments on this. Usually this matters only to the drivers that are able to be in streaming state without any buffers (usually some camera capture interfaces), which might need to enable dma engine from buf_queue callback. Other drivers only adds the buffer to the internal list and don't care about streaming state at all. > Beyond that, what happens to a driver without a start_streaming() callback > if the application first queues all its buffers, then does its > VIDIOC_STREAMON call? I see: > > > + list_for_each_entry(vb, &q->queued_list, queued_entry) > > + __enqueue_in_driver(vb, 0); > > (So we get streaming=0 for all queued buffers). > > > /* > > * Let driver notice that streaming state has been enabled. > > */ > > - ret = call_qop(q, start_streaming, q); > > + ret = call_qop(q, start_streaming, q, atomic_read(&q->queued_count)); > > if (ret) { > > dprintk(1, "streamon: driver refused to start streaming\n"); > > + __vb2_queue_cancel(q); > > return ret; > > } > > The driver will have gotten all of the buffers with streaming=0, then will > never get a call again; I don't think that will lead to the desired result. > Am I missing something? Hmmm. Looks that I missed something. The driver will need to ignore streaming parameter in such case... Best regards -- Marek Szyprowski Samsung Poland R&D Center -- 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 PATCH 02/12] mt20xx.c: fix compiler warnings
From: Hans Verkuil v4l-dvb-git/drivers/media/common/tuners/mt20xx.c: In function 'mt2050_set_antenna': v4l-dvb-git/drivers/media/common/tuners/mt20xx.c:433:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable] v4l-dvb-git/drivers/media/common/tuners/mt20xx.c: In function 'mt2050_init': v4l-dvb-git/drivers/media/common/tuners/mt20xx.c:577:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable] Signed-off-by: Hans Verkuil --- drivers/media/common/tuners/mt20xx.c | 24 +++- 1 files changed, 11 insertions(+), 13 deletions(-) diff --git a/drivers/media/common/tuners/mt20xx.c b/drivers/media/common/tuners/mt20xx.c index d0e70e1..0e74e97 100644 --- a/drivers/media/common/tuners/mt20xx.c +++ b/drivers/media/common/tuners/mt20xx.c @@ -430,11 +430,10 @@ static void mt2050_set_antenna(struct dvb_frontend *fe, unsigned char antenna) { struct microtune_priv *priv = fe->tuner_priv; unsigned char buf[2]; - int ret; buf[0] = 6; buf[1] = antenna ? 0x11 : 0x10; - ret=tuner_i2c_xfer_send(&priv->i2c_props,buf,2); + tuner_i2c_xfer_send(&priv->i2c_props, buf, 2); tuner_dbg("mt2050: enabled antenna connector %d\n", antenna); } @@ -574,21 +573,20 @@ static int mt2050_init(struct dvb_frontend *fe) { struct microtune_priv *priv = fe->tuner_priv; unsigned char buf[2]; - int ret; - buf[0]=6; - buf[1]=0x10; - ret=tuner_i2c_xfer_send(&priv->i2c_props,buf,2); // power + buf[0] = 6; + buf[1] = 0x10; + tuner_i2c_xfer_send(&priv->i2c_props, buf, 2); /* power */ - buf[0]=0x0f; - buf[1]=0x0f; - ret=tuner_i2c_xfer_send(&priv->i2c_props,buf,2); // m1lo + buf[0] = 0x0f; + buf[1] = 0x0f; + tuner_i2c_xfer_send(&priv->i2c_props, buf, 2); /* m1lo */ - buf[0]=0x0d; - ret=tuner_i2c_xfer_send(&priv->i2c_props,buf,1); - tuner_i2c_xfer_recv(&priv->i2c_props,buf,1); + buf[0] = 0x0d; + tuner_i2c_xfer_send(&priv->i2c_props, buf, 1); + tuner_i2c_xfer_recv(&priv->i2c_props, buf, 1); - tuner_dbg("mt2050: sro is %x\n",buf[0]); + tuner_dbg("mt2050: sro is %x\n", buf[0]); memcpy(&fe->ops.tuner_ops, &mt2050_tuner_ops, sizeof(struct dvb_tuner_ops)); -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 06/12] mxl5005s: fix compiler warning
From: Hans Verkuil v4l-dvb-git/drivers/media/common/tuners/mxl5005s.c: In function 'MXL_TuneRF': v4l-dvb-git/drivers/media/common/tuners/mxl5005s.c:2327:6: warning: variable 'Xtal_Int' set but not used [-Wunused-but-set-variable] Removed the unused Xtal_Int variable. That made it also possible to remove a related function. However, the code of that function has been preserved in a comment describing an equation. Without that function that comment would have been hard to understand. Signed-off-by: Hans Verkuil --- drivers/media/common/tuners/mxl5005s.c | 22 ++ 1 files changed, 10 insertions(+), 12 deletions(-) diff --git a/drivers/media/common/tuners/mxl5005s.c b/drivers/media/common/tuners/mxl5005s.c index 56fe75c..54be9e6 100644 --- a/drivers/media/common/tuners/mxl5005s.c +++ b/drivers/media/common/tuners/mxl5005s.c @@ -309,7 +309,6 @@ static u16 MXL_ControlWrite_Group(struct dvb_frontend *fe, u16 controlNum, static u16 MXL_SetGPIO(struct dvb_frontend *fe, u8 GPIO_Num, u8 GPIO_Val); static u16 MXL_GetInitRegister(struct dvb_frontend *fe, u8 *RegNum, u8 *RegVal, int *count); -static u32 MXL_GetXtalInt(u32 Xtal_Freq); static u16 MXL_TuneRF(struct dvb_frontend *fe, u32 RF_Freq); static void MXL_SynthIFLO_Calc(struct dvb_frontend *fe); static void MXL_SynthRFTGLO_Calc(struct dvb_frontend *fe); @@ -2307,14 +2306,6 @@ static u16 MXL_IFSynthInit(struct dvb_frontend *fe) return status ; } -static u32 MXL_GetXtalInt(u32 Xtal_Freq) -{ - if ((Xtal_Freq % 100) == 0) - return (Xtal_Freq / 1); - else - return (((Xtal_Freq / 100) + 1)*100); -} - static u16 MXL_TuneRF(struct dvb_frontend *fe, u32 RF_Freq) { struct mxl5005s_state *state = fe->tuner_priv; @@ -2324,13 +2315,10 @@ static u16 MXL_TuneRF(struct dvb_frontend *fe, u32 RF_Freq) u32 Kdbl_RF = 2; u32 tg_divval; u32 tg_lo; - u32 Xtal_Int; u32 Fref_TG; u32 Fvco; - Xtal_Int = MXL_GetXtalInt(state->Fxtal); - state->RF_IN = RF_Freq; MXL_SynthRFTGLO_Calc(fe); @@ -2779,6 +2767,16 @@ static u16 MXL_TuneRF(struct dvb_frontend *fe, u32 RF_Freq) tg_lo = (((Fmax/10 - Fvco)/100)*32) / ((Fmax-Fmin)/1000)+8; /* below equation is same as above but much harder to debug. +* +* static u32 MXL_GetXtalInt(u32 Xtal_Freq) +* { +* if ((Xtal_Freq % 100) == 0) +* return (Xtal_Freq / 1); +* else +* return (((Xtal_Freq / 100) + 1)*100); +* } +* +* u32 Xtal_Int = MXL_GetXtalInt(state->Fxtal); * tg_lo = ( ((Fmax/1 * Xtal_Int)/100) - * ((state->TG_LO/1)*divider_val * * (state->Fxtal/1)/100) )*32/((Fmax-Fmin)/1 * -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 00/12] First round of compiler warning fixes
The daily build is now compiled with gcc 4.6.1 which produces lots of 'variable set but not used' warnings. This is the first round of fixes. If there are no objections, then I'll make a pull request this weekend. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 08/12] tvaudio: fix compiler warnings
From: Hans Verkuil v4l-dvb-git/drivers/media/video/tvaudio.c: In function 'tvaudio_s_ctrl': v4l-dvb-git/drivers/media/video/tvaudio.c:1697:15: warning: variable 'balance' set but not used [-Wunused-but-set-variable] v4l-dvb-git/drivers/media/video/tvaudio.c:1697:7: warning: variable 'volume' set but not used [-Wunused-but-set-variable] This is indeed a bug: balance and volume must be used to set the left and right channel volume. Fixed. Signed-off-by: Hans Verkuil --- drivers/media/video/tvaudio.c |9 ++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/media/video/tvaudio.c b/drivers/media/video/tvaudio.c index c46a3bb..f22dbef 100644 --- a/drivers/media/video/tvaudio.c +++ b/drivers/media/video/tvaudio.c @@ -1695,14 +1695,17 @@ static int tvaudio_s_ctrl(struct v4l2_subdev *sd, case V4L2_CID_AUDIO_BALANCE: { int volume, balance; + if (!(desc->flags & CHIP_HAS_VOLUME)) break; - volume = max(chip->left,chip->right); + volume = max(chip->left, chip->right); balance = ctrl->value; + chip->left = (min(65536 - balance, 32768) * volume) / 32768; + chip->right = (min(balance, volume * (__u16)32768)) / 32768; - chip_write(chip,desc->leftreg,desc->volfunc(chip->left)); - chip_write(chip,desc->rightreg,desc->volfunc(chip->right)); + chip_write(chip, desc->leftreg, desc->volfunc(chip->left)); + chip_write(chip, desc->rightreg, desc->volfunc(chip->right)); return 0; } -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 07/12] af9005-fe: fix compiler warning
From: Hans Verkuil v4l-dvb-git/drivers/media/dvb/dvb-usb/af9005-fe.c: In function 'af9005_write_word_agc': v4l-dvb-git/drivers/media/dvb/dvb-usb/af9005-fe.c:66:5: warning: variable 'temp' set but not used [-Wunused-but-set-variable] Signed-off-by: Hans Verkuil --- drivers/media/dvb/dvb-usb/af9005-fe.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/media/dvb/dvb-usb/af9005-fe.c b/drivers/media/dvb/dvb-usb/af9005-fe.c index 6ad9474..3263e97 100644 --- a/drivers/media/dvb/dvb-usb/af9005-fe.c +++ b/drivers/media/dvb/dvb-usb/af9005-fe.c @@ -63,11 +63,9 @@ static int af9005_write_word_agc(struct dvb_usb_device *d, u16 reghi, u16 reglo, u8 pos, u8 len, u16 value) { int ret; - u8 temp; if ((ret = af9005_write_ofdm_register(d, reglo, (u8) (value & 0xff return ret; - temp = (u8) ((value & 0x0300) >> 8); return af9005_write_register_bits(d, reghi, pos, len, (u8) ((value & 0x300) >> 8)); } -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 12/12] vpx3220, bt819: fix compiler warnings
From: Hans Verkuil v4l-dvb-git/drivers/media/video/vpx3220.c: In function 'vpx3220_status': v4l-dvb-git/drivers/media/video/vpx3220.c:299:6: warning: variable 'res' set but not used [-Wunused-but-set-variable] v4l-dvb-git/drivers/media/video/bt819.c: In function 'bt819_status': v4l-dvb-git/drivers/media/video/bt819.c:219:6: warning: variable 'res' set but not used [-Wunused-but-set-variable] Same status/res mixup. Signed-off-by: Hans Verkuil --- drivers/media/video/bt819.c |2 +- drivers/media/video/vpx3220.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/bt819.c b/drivers/media/video/bt819.c index f872044..859eabf 100644 --- a/drivers/media/video/bt819.c +++ b/drivers/media/video/bt819.c @@ -229,7 +229,7 @@ static int bt819_status(struct v4l2_subdev *sd, u32 *pstatus, v4l2_std_id *pstd) if (pstd) *pstd = std; if (pstatus) - *pstatus = status; + *pstatus = res; v4l2_dbg(1, debug, sd, "get status %x\n", status); return 0; diff --git a/drivers/media/video/vpx3220.c b/drivers/media/video/vpx3220.c index ca372eb..e5cad6f 100644 --- a/drivers/media/video/vpx3220.c +++ b/drivers/media/video/vpx3220.c @@ -331,7 +331,7 @@ static int vpx3220_status(struct v4l2_subdev *sd, u32 *pstatus, v4l2_std_id *pst if (pstd) *pstd = std; if (pstatus) - *pstatus = status; + *pstatus = res; return 0; } -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 03/12] wl128x: fix compiler warning + wrong write() return.
From: Hans Verkuil v4l-dvb-git/drivers/media/radio/wl128x/fmdrv_v4l2.c: In function 'fm_v4l2_fops_write': v4l-dvb-git/drivers/media/radio/wl128x/fmdrv_v4l2.c:81:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable] The fix is to check for ret and return -EFAULT if non-zero. I also noticed that write() didn't return the number of bytes written. Fixed as well. Signed-off-by: Hans Verkuil --- drivers/media/radio/wl128x/fmdrv_v4l2.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/media/radio/wl128x/fmdrv_v4l2.c b/drivers/media/radio/wl128x/fmdrv_v4l2.c index 8c0e192..478d1e9 100644 --- a/drivers/media/radio/wl128x/fmdrv_v4l2.c +++ b/drivers/media/radio/wl128x/fmdrv_v4l2.c @@ -84,12 +84,14 @@ static ssize_t fm_v4l2_fops_write(struct file *file, const char __user * buf, ret = copy_from_user(&rds, buf, sizeof(rds)); fmdbg("(%d)type: %d, text %s, af %d\n", ret, rds.text_type, rds.text, rds.af_freq); + if (ret) + return -EFAULT; fmdev = video_drvdata(file); fm_tx_set_radio_text(fmdev, rds.text, rds.text_type); fm_tx_set_af(fmdev, rds.af_freq); - return 0; + return sizeof(rds); } static u32 fm_v4l2_fops_poll(struct file *file, struct poll_table_struct *pts) -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 01/12] radio-si4713.c: fix compiler warning
From: Hans Verkuil v4l-dvb-git/drivers/media/radio/radio-si4713.c: In function 'radio_si4713_querycap': v4l-dvb-git/drivers/media/radio/radio-si4713.c:95:30: warning: variable 'rsdev' set but not used [-Wunused-but-set-variable] Signed-off-by: Hans Verkuil --- drivers/media/radio/radio-si4713.c |4 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/drivers/media/radio/radio-si4713.c b/drivers/media/radio/radio-si4713.c index 444b4cf..d1fab58 100644 --- a/drivers/media/radio/radio-si4713.c +++ b/drivers/media/radio/radio-si4713.c @@ -92,10 +92,6 @@ static int radio_si4713_s_audout(struct file *file, void *priv, static int radio_si4713_querycap(struct file *file, void *priv, struct v4l2_capability *capability) { - struct radio_si4713_device *rsdev; - - rsdev = video_get_drvdata(video_devdata(file)); - strlcpy(capability->driver, "radio-si4713", sizeof(capability->driver)); strlcpy(capability->card, "Silicon Labs Si4713 Modulator", sizeof(capability->card)); -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 11/12] drxd_hard: fix compiler warnings
From: Hans Verkuil v4l-dvb-git/drivers/media/dvb/frontends/drxd_hard.c: In function 'DownloadMicrocode': v4l-dvb-git/drivers/media/dvb/frontends/drxd_hard.c:933:6: warning: variable 'BlockCRC' set but not used [-Wunused-but-set-variable] v4l-dvb-git/drivers/media/dvb/frontends/drxd_hard.c:929:6: warning: variable 'Flags' set but not used [-Wunused-but-set-variable] Signed-off-by: Hans Verkuil --- drivers/media/dvb/frontends/drxd_hard.c | 11 ++- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/media/dvb/frontends/drxd_hard.c b/drivers/media/dvb/frontends/drxd_hard.c index 2238bf0..5490b48 100644 --- a/drivers/media/dvb/frontends/drxd_hard.c +++ b/drivers/media/dvb/frontends/drxd_hard.c @@ -926,16 +926,15 @@ static int DownloadMicrocode(struct drxd_state *state, const u8 *pMCImage, u32 Length) { u8 *pSrc; - u16 Flags; u32 Address; u16 nBlocks; u16 BlockSize; - u16 BlockCRC; u32 offset = 0; int i, status = 0; pSrc = (u8 *) pMCImage; - Flags = (pSrc[0] << 8) | pSrc[1]; + /* We're not using Flags */ + /* Flags = (pSrc[0] << 8) | pSrc[1]; */ pSrc += sizeof(u16); offset += sizeof(u16); nBlocks = (pSrc[0] << 8) | pSrc[1]; @@ -952,11 +951,13 @@ static int DownloadMicrocode(struct drxd_state *state, pSrc += sizeof(u16); offset += sizeof(u16); - Flags = (pSrc[0] << 8) | pSrc[1]; + /* We're not using Flags */ + /* u16 Flags = (pSrc[0] << 8) | pSrc[1]; */ pSrc += sizeof(u16); offset += sizeof(u16); - BlockCRC = (pSrc[0] << 8) | pSrc[1]; + /* We're not using BlockCRC */ + /* u16 BlockCRC = (pSrc[0] << 8) | pSrc[1]; */ pSrc += sizeof(u16); offset += sizeof(u16); -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 09/12] az6027: fix compiler warnings
From: Hans Verkuil v4l-dvb-git/drivers/media/dvb/dvb-usb/az6027.c: In function 'az6027_set_voltage': v4l-dvb-git/drivers/media/dvb/dvb-usb/az6027.c:785:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable] v4l-dvb-git/drivers/media/dvb/dvb-usb/az6027.c: In function 'az6027_i2c_xfer': v4l-dvb-git/drivers/media/dvb/dvb-usb/az6027.c:957:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable] Signed-off-by: Hans Verkuil --- drivers/media/dvb/dvb-usb/az6027.c | 12 +--- 1 files changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/media/dvb/dvb-usb/az6027.c b/drivers/media/dvb/dvb-usb/az6027.c index d59430c..bf389f4 100644 --- a/drivers/media/dvb/dvb-usb/az6027.c +++ b/drivers/media/dvb/dvb-usb/az6027.c @@ -782,7 +782,6 @@ static int az6027_set_voltage(struct dvb_frontend *fe, fe_sec_voltage_t voltage) { u8 buf; - int ret; struct dvb_usb_adapter *adap = fe->dvb->priv; struct i2c_msg i2c_msg = { @@ -800,17 +799,17 @@ static int az6027_set_voltage(struct dvb_frontend *fe, fe_sec_voltage_t voltage) switch (voltage) { case SEC_VOLTAGE_13: buf = 1; - ret = i2c_transfer(&adap->dev->i2c_adap, &i2c_msg, 1); + i2c_transfer(&adap->dev->i2c_adap, &i2c_msg, 1); break; case SEC_VOLTAGE_18: buf = 2; - ret = i2c_transfer(&adap->dev->i2c_adap, &i2c_msg, 1); + i2c_transfer(&adap->dev->i2c_adap, &i2c_msg, 1); break; case SEC_VOLTAGE_OFF: buf = 0; - ret = i2c_transfer(&adap->dev->i2c_adap, &i2c_msg, 1); + i2c_transfer(&adap->dev->i2c_adap, &i2c_msg, 1); break; default: @@ -954,7 +953,6 @@ static int az6027_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], int n { struct dvb_usb_device *d = i2c_get_adapdata(adap); int i = 0, j = 0, len = 0; - int ret; u16 index; u16 value; int length; @@ -990,7 +988,7 @@ static int az6027_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], int n index = (((msg[i].buf[0] << 8) & 0xff00) | (msg[i].buf[1] & 0x00ff)); value = msg[i].addr + (msg[i].len << 8); length = msg[i + 1].len + 6; - ret = az6027_usb_in_op(d, req, value, index, data, length); + az6027_usb_in_op(d, req, value, index, data, length); len = msg[i + 1].len; for (j = 0; j < len; j++) msg[i + 1].buf[j] = data[j + 5]; @@ -1017,7 +1015,7 @@ static int az6027_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], int n index = 0x0; value = msg[i].addr; length = msg[i].len + 6; - ret = az6027_usb_in_op(d, req, value, index, data, length); + az6027_usb_in_op(d, req, value, index, data, length); len = msg[i].len; for (j = 0; j < len; j++) msg[i].buf[j] = data[j + 5]; -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 10/12] mantis: fix compiler warnings
From: Hans Verkuil v4l-dvb-git/drivers/media/dvb/mantis/hopper_cards.c: In function 'hopper_irq_handler': v4l-dvb-git/drivers/media/dvb/mantis/hopper_cards.c:68:37: warning: variable 'mstat' set but not used [-Wunused-but-set-variable] v4l-dvb-git/drivers/media/dvb/mantis/mantis_cards.c: In function 'mantis_irq_handler': v4l-dvb-git/drivers/media/dvb/mantis/mantis_cards.c:76:37: warning: variable 'mstat' set but not used [-Wunused-but-set-variable] Signed-off-by: Hans Verkuil --- drivers/media/dvb/mantis/hopper_cards.c |4 ++-- drivers/media/dvb/mantis/mantis_cards.c |4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/dvb/mantis/hopper_cards.c b/drivers/media/dvb/mantis/hopper_cards.c index 1402062..8bbeebc 100644 --- a/drivers/media/dvb/mantis/hopper_cards.c +++ b/drivers/media/dvb/mantis/hopper_cards.c @@ -65,7 +65,7 @@ static int devs; static irqreturn_t hopper_irq_handler(int irq, void *dev_id) { - u32 stat = 0, mask = 0, lstat = 0, mstat = 0; + u32 stat = 0, mask = 0, lstat = 0; u32 rst_stat = 0, rst_mask = 0; struct mantis_pci *mantis; @@ -80,7 +80,7 @@ static irqreturn_t hopper_irq_handler(int irq, void *dev_id) stat = mmread(MANTIS_INT_STAT); mask = mmread(MANTIS_INT_MASK); - mstat = lstat = stat & ~MANTIS_INT_RISCSTAT; + lstat = stat & ~MANTIS_INT_RISCSTAT; if (!(stat & mask)) return IRQ_NONE; diff --git a/drivers/media/dvb/mantis/mantis_cards.c b/drivers/media/dvb/mantis/mantis_cards.c index 05cbb9d..e6c8368 100644 --- a/drivers/media/dvb/mantis/mantis_cards.c +++ b/drivers/media/dvb/mantis/mantis_cards.c @@ -73,7 +73,7 @@ static char *label[10] = { static irqreturn_t mantis_irq_handler(int irq, void *dev_id) { - u32 stat = 0, mask = 0, lstat = 0, mstat = 0; + u32 stat = 0, mask = 0, lstat = 0; u32 rst_stat = 0, rst_mask = 0; struct mantis_pci *mantis; @@ -88,7 +88,7 @@ static irqreturn_t mantis_irq_handler(int irq, void *dev_id) stat = mmread(MANTIS_INT_STAT); mask = mmread(MANTIS_INT_MASK); - mstat = lstat = stat & ~MANTIS_INT_RISCSTAT; + lstat = stat & ~MANTIS_INT_RISCSTAT; if (!(stat & mask)) return IRQ_NONE; -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 05/12] ddbridge: fix compiler warnings
From: Hans Verkuil v4l-dvb-git/drivers/media/dvb/ddbridge/ddbridge-core.c: In function 'ddb_input_read': v4l-dvb-git/drivers/media/dvb/ddbridge/ddbridge-core.c:515:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable] v4l-dvb-git/drivers/media/dvb/ddbridge/ddbridge-core.c:514:11: warning: variable 'off' set but not used [-Wunused-but-set-variable] 'off' was unused and 'ret' really had to be used to return -EFAULT. Signed-off-by: Hans Verkuil --- drivers/media/dvb/ddbridge/ddbridge-core.c |9 ++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/media/dvb/ddbridge/ddbridge-core.c b/drivers/media/dvb/ddbridge/ddbridge-core.c index 573d540..d2e85ea 100644 --- a/drivers/media/dvb/ddbridge/ddbridge-core.c +++ b/drivers/media/dvb/ddbridge/ddbridge-core.c @@ -507,15 +507,14 @@ static u32 ddb_input_avail(struct ddb_input *input) return 0; } -static size_t ddb_input_read(struct ddb_input *input, u8 *buf, size_t count) +static ssize_t ddb_input_read(struct ddb_input *input, u8 *buf, size_t count) { struct ddb *dev = input->port->dev; u32 left = count; - u32 idx, off, free, stat = input->stat; + u32 idx, free, stat = input->stat; int ret; idx = (stat >> 11) & 0x1f; - off = (stat & 0x7ff) << 7; while (left) { if (input->cbuf == idx) @@ -525,6 +524,8 @@ static size_t ddb_input_read(struct ddb_input *input, u8 *buf, size_t count) free = left; ret = copy_to_user(buf, input->vbuf[input->cbuf] + input->coff, free); + if (ret) + return -EFAULT; input->coff += free; if (input->coff == input->dma_buf_size) { input->coff = 0; @@ -939,6 +940,8 @@ static ssize_t ts_read(struct file *file, char *buf, break; } read = ddb_input_read(input, buf, left); + if (read < 0) + return read; left -= read; buf += read; } -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 04/12] saa7146: fix compiler warning
From: Hans Verkuil v4l-dvb-git/drivers/media/common/saa7146_video.c: In function 'video_close': v4l-dvb-git/drivers/media/common/saa7146_video.c:1350:6: warning: variable 'err' set but not used [-Wunused-but-set-variable] Signed-off-by: Hans Verkuil --- drivers/media/common/saa7146_video.c | 12 1 files changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/media/common/saa7146_video.c b/drivers/media/common/saa7146_video.c index 9aafa4e..77e232f 100644 --- a/drivers/media/common/saa7146_video.c +++ b/drivers/media/common/saa7146_video.c @@ -1347,18 +1347,14 @@ static void video_close(struct saa7146_dev *dev, struct file *file) struct saa7146_fh *fh = file->private_data; struct saa7146_vv *vv = dev->vv_data; struct videobuf_queue *q = &fh->video_q; - int err; - if (IS_CAPTURE_ACTIVE(fh) != 0) { - err = video_end(fh, file); - } else if (IS_OVERLAY_ACTIVE(fh) != 0) { - err = saa7146_stop_preview(fh); - } + if (IS_CAPTURE_ACTIVE(fh) != 0) + video_end(fh, file); + else if (IS_OVERLAY_ACTIVE(fh) != 0) + saa7146_stop_preview(fh); videobuf_stop(q); - /* hmm, why is this function declared void? */ - /* return err */ } -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Getting started with OMAP3 ISP
Background: I have working video capture drivers based on the TI PSP codebase from 2.6.32. In particular, I managed to get a driver for the TVP5150 (analogue BT656) working with that kernel. Now I need to update to Linux 3.0, so I'm trying to get a driver working with the rewritten ISP code. Sadly, I'm having a hard time with this - probably just missing something basic. I've tried to clone the TVP514x driver which says that it works with the OMAP3 ISP code. I've updated it to use my decoder device, but I can't even seem to get into that code from user land. Here are the problems I've had so far: * udev doesn't create any video devices although they have been registered. I see a full set in /sys/class/video4linux # ls /sys/class/video4linux/ v4l-subdev0 v4l-subdev3 v4l-subdev6 video1 video4 v4l-subdev1 v4l-subdev4 v4l-subdev7 video2 video5 v4l-subdev2 v4l-subdev5 video0 video3 video6 Indeed, if I create /dev/videoX by hand, I can get somewhere, but I don't really understand how this is supposed to work. e.g. # v4l2-dbg --info /dev/video3 Driver info: Driver name : ispvideo Card type : OMAP3 ISP CCP2 input Bus info : media Driver version: 1 Capabilities : 0x0402 Video Output Streaming * If I try to grab video, the ISP layer gets a ton of warnings, but I never see it call down into my driver, e.g. to check the current format, etc. I have some of my own code from before which fails miserably (not a big surprise given the hack level of those programs). I tried something off-the-shelf which also fails pretty bad: # ffmpeg -t 10 -f video4linux2 -s 720x480 -r 30 -i /dev/video2 junk.mp4 I've read through Documentation/video4linux/omap3isp.txt without learning much about what might be wrong. Can someone give me some ideas/guidance, please? Thanks -- Gary Thomas | Consulting for the MLB Associates |Embedded world -- 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] V4L: mx3-camera: prepare to support multi-size buffers
Prepare the mx3_camera friver to support the new VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF ioctl()s. The .queue_setup() vb2 operation must be able to handle buffer sizes, provided by the caller, and the .buf_prepare() operation must not use the currently configured frame format for its operation, which makes it superfluous for this driver. Its functionality is moved into .buf_queue(). Signed-off-by: Guennadi Liakhovetski --- drivers/media/video/mx3_camera.c | 153 +++-- 1 files changed, 79 insertions(+), 74 deletions(-) diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c index 6bfbce9..51eee4e 100644 --- a/drivers/media/video/mx3_camera.c +++ b/drivers/media/video/mx3_camera.c @@ -114,6 +114,7 @@ struct mx3_camera_dev { struct list_headcapture; spinlock_t lock; /* Protects video buffer lists */ struct mx3_camera_buffer *active; + size_t buf_total; struct vb2_alloc_ctx*alloc_ctx; enum v4l2_field field; int sequence; @@ -198,118 +199,117 @@ static int mx3_videobuf_setup(struct vb2_queue *vq, struct soc_camera_device *icd = soc_camera_from_vb2q(vq); struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct mx3_camera_dev *mx3_cam = ici->priv; - int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width, - icd->current_fmt->host_fmt); + int bytes_per_line; + unsigned int height; + if (!mx3_cam->idmac_channel[0]) + return -EINVAL; + + if (fmt) { + const struct soc_camera_format_xlate *xlate = soc_camera_xlate_by_fourcc(icd, + fmt->fmt.pix.pixelformat); + bytes_per_line = soc_mbus_bytes_per_line(fmt->fmt.pix.width, +xlate->host_fmt); + height = fmt->fmt.pix.height; + } else { + /* Called from VIDIOC_REQBUFS or in compatibility mode */ + bytes_per_line = soc_mbus_bytes_per_line(icd->user_width, + icd->current_fmt->host_fmt); + height = icd->user_height; + } if (bytes_per_line < 0) return bytes_per_line; - if (!mx3_cam->idmac_channel[0]) - return -EINVAL; + sizes[0] = bytes_per_line * height; *num_planes = 1; - mx3_cam->sequence = 0; - sizes[0] = bytes_per_line * icd->user_height; alloc_ctxs[0] = mx3_cam->alloc_ctx; + if (!vq->num_buffers) + mx3_cam->sequence = 0; + if (!*count) *count = 32; - if (sizes[0] * *count > MAX_VIDEO_MEM * 1024 * 1024) - *count = MAX_VIDEO_MEM * 1024 * 1024 / sizes[0]; + if (sizes[0] * *count + mx3_cam->buf_total > MAX_VIDEO_MEM * 1024 * 1024) + *count = (MAX_VIDEO_MEM * 1024 * 1024 - mx3_cam->buf_total) / + sizes[0]; return 0; } -static int mx3_videobuf_prepare(struct vb2_buffer *vb) +static enum pixel_fmt fourcc_to_ipu_pix(__u32 fourcc) +{ + /* Add more formats as need arises and test possibilities appear... */ + switch (fourcc) { + case V4L2_PIX_FMT_RGB24: + return IPU_PIX_FMT_RGB24; + case V4L2_PIX_FMT_UYVY: + case V4L2_PIX_FMT_RGB565: + default: + return IPU_PIX_FMT_GENERIC; + } +} + +static void mx3_videobuf_queue(struct vb2_buffer *vb) { struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue); struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct mx3_camera_dev *mx3_cam = ici->priv; + struct mx3_camera_buffer *buf = to_mx3_vb(vb); + struct scatterlist *sg = &buf->sg; + struct dma_async_tx_descriptor *txd; struct idmac_channel *ichan = mx3_cam->idmac_channel[0]; - struct scatterlist *sg; - struct mx3_camera_buffer *buf; + struct idmac_video_param *video = &ichan->params.video; + const struct soc_mbus_pixelfmt *host_fmt = icd->current_fmt->host_fmt; + int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width, host_fmt); + unsigned long flags; + dma_cookie_t cookie; size_t new_size; - int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width, - icd->current_fmt->host_fmt); - - if (bytes_per_line < 0) - return bytes_per_line; - buf = to_mx3_vb(vb); - sg = &buf->sg; + BUG_ON(bytes_per_line <= 0); new_size = bytes_per_line * icd->user_height; if (vb2_plane_size(vb, 0) < new_size) { - dev_err(icd->parent, "Buffer too small (%lu < %zu)\n", - vb2_plane_size(
[PATCH 0/2] i.MX3: support multi-size buffers in V4L
Supporting new V4L2 ioctl()s in the mx3_camera driver also requires an extension for the ipu-idmac dmaengine driver. The original thread, adding the ioctl()s can be found here: http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/37143 Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] dmaengine: ipu-idmac: add support for the DMA_PAUSE control
To support multi-size buffers in the mx3_camera V4L2 driver we have to be able to stop DMA on a channel without releasing descriptors and completely halting the hardware. Use the DMA_PAUSE control to implement this mode. Signed-off-by: Guennadi Liakhovetski --- drivers/dma/ipu/ipu_idmac.c | 65 +++--- 1 files changed, 42 insertions(+), 23 deletions(-) diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c index c1a125e..42cdf1c 100644 --- a/drivers/dma/ipu/ipu_idmac.c +++ b/drivers/dma/ipu/ipu_idmac.c @@ -1306,6 +1306,7 @@ static irqreturn_t idmac_interrupt(int irq, void *dev_id) ipu_submit_buffer(ichan, descnew, sgnew, ichan->active_buffer) < 0) { callback = descnew->txd.callback; callback_param = descnew->txd.callback_param; + list_del_init(&descnew->list); spin_unlock(&ichan->lock); if (callback) callback(callback_param); @@ -1427,39 +1428,58 @@ static int __idmac_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, { struct idmac_channel *ichan = to_idmac_chan(chan); struct idmac *idmac = to_idmac(chan->device); + struct ipu *ipu = to_ipu(idmac); + struct list_head *list, *tmp; unsigned long flags; int i; - /* Only supports DMA_TERMINATE_ALL */ - if (cmd != DMA_TERMINATE_ALL) - return -ENXIO; + switch (cmd) { + case DMA_PAUSE: + spin_lock_irqsave(&ipu->lock, flags); + ipu_ic_disable_task(ipu, chan->chan_id); - ipu_disable_channel(idmac, ichan, - ichan->status >= IPU_CHANNEL_ENABLED); + /* Return all descriptors into "prepared" state */ + list_for_each_safe(list, tmp, &ichan->queue) + list_del_init(list); - tasklet_disable(&to_ipu(idmac)->tasklet); + ichan->sg[0] = NULL; + ichan->sg[1] = NULL; - /* ichan->queue is modified in ISR, have to spinlock */ - spin_lock_irqsave(&ichan->lock, flags); - list_splice_init(&ichan->queue, &ichan->free_list); + spin_unlock_irqrestore(&ipu->lock, flags); - if (ichan->desc) - for (i = 0; i < ichan->n_tx_desc; i++) { - struct idmac_tx_desc *desc = ichan->desc + i; - if (list_empty(&desc->list)) - /* Descriptor was prepared, but not submitted */ - list_add(&desc->list, &ichan->free_list); + ichan->status = IPU_CHANNEL_INITIALIZED; + break; + case DMA_TERMINATE_ALL: + ipu_disable_channel(idmac, ichan, + ichan->status >= IPU_CHANNEL_ENABLED); - async_tx_clear_ack(&desc->txd); - } + tasklet_disable(&ipu->tasklet); - ichan->sg[0] = NULL; - ichan->sg[1] = NULL; - spin_unlock_irqrestore(&ichan->lock, flags); + /* ichan->queue is modified in ISR, have to spinlock */ + spin_lock_irqsave(&ichan->lock, flags); + list_splice_init(&ichan->queue, &ichan->free_list); - tasklet_enable(&to_ipu(idmac)->tasklet); + if (ichan->desc) + for (i = 0; i < ichan->n_tx_desc; i++) { + struct idmac_tx_desc *desc = ichan->desc + i; + if (list_empty(&desc->list)) + /* Descriptor was prepared, but not submitted */ + list_add(&desc->list, &ichan->free_list); - ichan->status = IPU_CHANNEL_INITIALIZED; + async_tx_clear_ack(&desc->txd); + } + + ichan->sg[0] = NULL; + ichan->sg[1] = NULL; + spin_unlock_irqrestore(&ichan->lock, flags); + + tasklet_enable(&ipu->tasklet); + + ichan->status = IPU_CHANNEL_INITIALIZED; + break; + default: + return -ENOSYS; + } return 0; } @@ -1662,7 +1682,6 @@ static void __exit ipu_idmac_exit(struct ipu *ipu) struct idmac_channel *ichan = ipu->channel + i; idmac_control(&ichan->dma_chan, DMA_TERMINATE_ALL, 0); - idmac_prep_slave_sg(&ichan->dma_chan, NULL, 0, DMA_NONE, 0); } dma_async_device_unregister(&idmac->dma); -- 1.7.2.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] V4L: mx3-camera: prepare to support multi-size buffers
Hi Guennadi, On Thursday 25 August 2011 18:46:03 Guennadi Liakhovetski wrote: > Prepare the mx3_camera friver to support the new VIDIOC_CREATE_BUFS and > VIDIOC_PREPARE_BUF ioctl()s. The .queue_setup() vb2 operation must be > able to handle buffer sizes, provided by the caller, and the > .buf_prepare() operation must not use the currently configured frame > format for its operation, which makes it superfluous for this driver. > Its functionality is moved into .buf_queue(). You're moving the ichan->dma_chan.device->device_prep_slave_sg() call from .buf_prepare() to .buf_queue(). Is that call cheap ? Otherwise it would be better to keep the .buf_prepare() callback. -- 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
DD Cine CT DVB-C/T
Hi Which modules do I have to build and install to use this card (DD Cine CT DVB-C/T Rev. V6). I checkd out: hg clone http://linuxtv.org/hg/~endriss/media_build_experimental I would like to build only the needed modules. What do I have to select in "make menuconfig"? Regards, Thomas -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RDS Alternate Frequency support in V4L2 radio
Resending since first time mail-delivery failed to linux-media Hi Hans, Mauro & list, I wants to add FM Radio RDS Alternate Frequency support in V4L2 and following is my proposal. What is Alternate Frequency? Alternative frequency (or AF) is an option that allows a receiver to re-tune to a different frequency that provides the same station, when the first signal becomes too weak (e.g. when moving out of range). This is often used in car stereo systems, enabled by Radio Data System(RDS). My Proposal: FM Transmitter: Add an extra control ID (CID) V4L2_CID_RDS_TX_AF like CID's for Radio_Text, PS_Name etc for Alternate Frequency set. With this CID driver will set the AF and starts transmitting in both the frequencies. FM Receiver: Solution 1: Add support for AF based Seek in ioctl "vidioc_s_hw_freq_seek" by adding AF_SEEK option in "struct v4l2_hw_freq_seek" so that whenever application receives signal strength low or a particular channel it will call the ioctl vidioc_s_hw_freq_seek with AF_SEEK option and driver will do AF seek and returns the seeked new frequency back to app. Solution 2: Whenever application receives signal strength low for a radio channel, it will call the ioctl .vidioc_g_frequency where driver will check the signal strength and if its low then do the AF switch and return the new frequency back to application Solution3: Let Application handle the AF switching, means application will get the available AF frequencies for a channel and whenever application receives signal strength low for the channel it will check the AF list and do the vidioc_s_frequency for each entries in list. -- 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] v4l-dvb daily build: WARNINGS
This message is generated daily by a cron job that builds v4l-dvb for the kernels and architectures in the list below. Results of the daily build of v4l-dvb: date:Thu Aug 25 19:00:32 CEST 2011 git hash:9bed77ee2fb46b74782d0d9d14b92e9d07f3df6e gcc version: i686-linux-gcc (GCC) 4.6.1 host hardware:x86_64 host os: 2.6.32.5 linux-git-armv5: WARNINGS linux-git-armv5-davinci: WARNINGS linux-git-armv5-ixp: WARNINGS linux-git-armv5-omap2: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS linux-2.6.31.12-i686: WARNINGS linux-2.6.32.6-i686: WARNINGS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: WARNINGS linux-2.6.35.3-i686: WARNINGS linux-2.6.36-i686: WARNINGS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-rc1-i686: WARNINGS linux-2.6.31.12-x86_64: WARNINGS linux-2.6.32.6-x86_64: WARNINGS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: WARNINGS linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-rc1-x86_64: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The V4L-DVB specification 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: [RFC PATCH] Modify volatile auto cluster handling as per earlier discussions
Hi, First of all thanks for doing this! Overall it looks good, see below for several (small) remarks which I have. On 08/09/2011 06:40 PM, Hans Verkuil wrote: This patch modifies the way autoclusters work when the 'foo' controls are volatile if autofoo is on. E.g.: if autogain is true, then gain returns the gain set by the autogain circuitry. This patch makes the following changes: 1) The V4L2_CTRL_FLAG_VOLATILE flag is added to let userspace know that a certain control is volatile. Currently this is internal information only. 2) If v4l2_ctrl_auto_cluster() is called with the last 'set_volatile' argument set to true, then the cluster has the following behavior: - when in manual mode you can set the manual values normally. - when in auto mode any new manual values will be ignored. When you read the manual values you will get those as determined by the auto mode. - when switching from auto to manual mode the manual values from the auto mode are obtained through g_volatile_ctrl first. Any manual values explicitly set by the application will replace those obtained from the automode and the final set of values is sent to the driver with s_ctrl. - when in auto mode the V4L2_CTRL_FLAG_VOLATILE and INACTIVE flags are set for the 'foo' controls. These flags are cleared when in manual mode. This patch modifies existing users of is_volatile and simplifies the pwc driver that required this behavior. The only thing missing is the documentation update and some code comments. I have to admit that it works quite well. Hans, can you verify that this does what you wanted it to do? Regards, Hans diff --git a/drivers/media/video/pwc/pwc-v4l.c b/drivers/media/video/pwc/pwc-v4l.c index e9a0e94..4ce00bf 100644 --- a/drivers/media/video/pwc/pwc-v4l.c +++ b/drivers/media/video/pwc/pwc-v4l.c @@ -632,52 +634,28 @@ static int pwc_set_awb(struct pwc_device *pdev) { int ret = 0; - if (pdev->auto_white_balance->is_new) { + if (pdev->auto_white_balance->is_new) ret = pwc_set_u8_ctrl(pdev, SET_CHROM_CTL, - WB_MODE_FORMATTER, - pdev->auto_white_balance->val); - if (ret) - return ret; + WB_MODE_FORMATTER, + pdev->auto_white_balance->val); + if (ret) + return ret; - /* Update val when coming from auto or going to a preset */ - if (pdev->red_balance->is_volatile || - pdev->auto_white_balance->val == awb_indoor || - pdev->auto_white_balance->val == awb_outdoor || - pdev->auto_white_balance->val == awb_fl) { - if (!pdev->red_balance->is_new) - pwc_get_u8_ctrl(pdev, GET_STATUS_CTL, - READ_RED_GAIN_FORMATTER, - &pdev->red_balance->val); - if (!pdev->blue_balance->is_new) - pwc_get_u8_ctrl(pdev, GET_STATUS_CTL, - READ_BLUE_GAIN_FORMATTER, - &pdev->blue_balance->val); - } - if (pdev->auto_white_balance->val == awb_auto) { - pdev->red_balance->is_volatile = true; - pdev->blue_balance->is_volatile = true; - pdev->color_bal_valid = false; /* Force cache update */ - } else { - pdev->red_balance->is_volatile = false; - pdev->blue_balance->is_volatile = false; - } + if (pdev->auto_white_balance->val != awb_manual) { + pdev->color_bal_valid = false; /* Force cache update */ + return 0; } The setting of pdev->color_bal_valid = false should happen inside the "if (pdev->auto_white_balance->is_new)" block (under the same pdev->auto_white_balance->val != awb_manual condition), the way it is now if someone tries to say write blue bal while in awb mode, not only will the write get ignored (good), but this will also invalidate the cached values (bad). - if (ret == 0&& pdev->red_balance->is_new) { - if (pdev->auto_white_balance->val != awb_manual) - return -EBUSY; + if (pdev->red_balance->is_new) ret = pwc_set_u8_ctrl(pdev, SET_CHROM_CTL, - PRESET_MANUAL_RED_GAIN_FORMATTER, - pdev->red_balance->val); - } - - if (ret == 0&& pdev->blue_balance->is_new) { - if (pdev->auto_white_balance->val != awb_manual) - return -EBUSY; + PRESET_MANUAL_RED_GAIN_FORMATTER, + pdev->red_balance->val); + if (ret)
[PATCH v2] [media] vp702x: fix buffer handling
In my previous change to this driver, I was not aware, that dvb_usb_device_init calls the frontend_attach routine which needs a transfer buffer. So we can not setup anything private in the probe routine beforehand but have to allocate when needed. This means also that we cannot use a private buffer mutex to serialize that buffer but instead need to use the dvb_usb_device's usb_mutex. Fixes: https://bugzilla.novell.com/show_bug.cgi?id=709440 Tested-by: Markus Stephan Signed-off-by: Florian Mickler --- So, someone who could test that driver found me after all. I renamed the functions to get rid of that ugly and pointless _unlocked suffix I deliriously added earlier. Markus tested this patch modulo function renaming. I am so shure that this version will still work for him, that I already added his Tested-by. *fingerscrossed* drivers/media/dvb/dvb-usb/vp702x-fe.c | 92 +++ drivers/media/dvb/dvb-usb/vp702x.c| 211 ++--- drivers/media/dvb/dvb-usb/vp702x.h| 13 ++- 3 files changed, 193 insertions(+), 123 deletions(-) diff --git a/drivers/media/dvb/dvb-usb/vp702x-fe.c b/drivers/media/dvb/dvb-usb/vp702x-fe.c index 2bb8d4c..ad16455 100644 --- a/drivers/media/dvb/dvb-usb/vp702x-fe.c +++ b/drivers/media/dvb/dvb-usb/vp702x-fe.c @@ -43,21 +43,30 @@ static int vp702x_fe_refresh_state(struct vp702x_fe_state *st) { struct vp702x_device_state *dst = st->d->priv; u8 *buf; + int ret; if (time_after(jiffies, st->next_status_check)) { - mutex_lock(&dst->buf_mutex); - buf = dst->buf; + ret = mutex_lock_interruptible(&st->d->usb_mutex); + if (ret < 0) + return ret; + + ret = vp702x_buffer_setup(dst, &buf, 10); + if (ret) + goto unlock; vp702x_usb_in_op(st->d, READ_STATUS, 0, 0, buf, 10); st->lock = buf[4]; - vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x11, 0, buf, 1); + vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x11, 0, + buf, 1); st->snr = buf[0]; - vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x15, 0, buf, 1); + vp702x_usb_in_op(st->d, READ_TUNER_REG_REQ, 0x15, 0, + buf, 1); st->sig = buf[0]; - mutex_unlock(&dst->buf_mutex); +unlock: + mutex_unlock(&st->d->usb_mutex); st->next_status_check = jiffies + (st->status_check_interval*HZ)/1000; } return 0; @@ -145,11 +154,15 @@ static int vp702x_fe_set_frontend(struct dvb_frontend* fe, /* u16 frequencyRef[16] = { 2, 4, 8, 16, 32, 64, 128, 256, 24, 5, 10, 20, 40, 80, 160, 320 }; */ u64 sr; u8 *cmd; + int ret; - mutex_lock(&dst->buf_mutex); + ret = mutex_lock_interruptible(&st->d->usb_mutex); + if (ret < 0) + return ret; - cmd = dst->buf; - memset(cmd, 0, 10); + ret = vp702x_buffer_setup(dst, &cmd, 10); + if (ret) + goto unlock; cmd[0] = (freq >> 8) & 0x7f; cmd[1] = freq & 0xff; @@ -191,17 +204,25 @@ static int vp702x_fe_set_frontend(struct dvb_frontend* fe, deb_fe("tuning failed.\n"); else deb_fe("tuning succeeded.\n"); + ret = 0; - mutex_unlock(&dst->buf_mutex); - - return 0; +unlock: + mutex_unlock(&st->d->usb_mutex); + return ret; } static int vp702x_fe_init(struct dvb_frontend *fe) { struct vp702x_fe_state *st = fe->demodulator_priv; + int ret; + deb_fe("%s\n",__func__); + ret = mutex_lock_interruptible(&st->d->usb_mutex); + if (ret < 0) + return ret; vp702x_usb_in_op(st->d, RESET_TUNER, 0, 0, NULL, 0); + + mutex_unlock(&st->d->usb_mutex); return 0; } @@ -224,15 +245,21 @@ static int vp702x_fe_send_diseqc_msg (struct dvb_frontend* fe, u8 *cmd; struct vp702x_fe_state *st = fe->demodulator_priv; struct vp702x_device_state *dst = st->d->priv; + int ret; deb_fe("%s\n",__func__); if (m->msg_len > 4) return -EINVAL; - mutex_lock(&dst->buf_mutex); + ret = mutex_lock_interruptible(&st->d->usb_mutex); + if (ret < 0) + return ret; + + ret = vp702x_buffer_setup(dst, &cmd, 10); + if (ret) + goto unlock; - cmd = dst->buf; cmd[1] = SET_DISEQC_CMD; cmd[2] = m->msg_len; memcpy(&cmd[3], m->msg, m->msg_len); @@ -244,10 +271,12 @@ static int vp702x_fe_send_diseqc_msg (struct dvb_frontend* fe, deb_fe("diseqc cmd failed.\n"); else deb_fe("diseqc cmd succeeded.\n"); + ret = 0; - mutex_unlock(&dst->buf_mutex); +unlock: + mutex_unlock(&st->d->usb_mut
Re: [PATCH 2/2] V4L: mx3-camera: prepare to support multi-size buffers
On Thu, 25 Aug 2011, Laurent Pinchart wrote: > Hi Guennadi, > > On Thursday 25 August 2011 18:46:03 Guennadi Liakhovetski wrote: > > Prepare the mx3_camera friver to support the new VIDIOC_CREATE_BUFS and > > VIDIOC_PREPARE_BUF ioctl()s. The .queue_setup() vb2 operation must be > > able to handle buffer sizes, provided by the caller, and the > > .buf_prepare() operation must not use the currently configured frame > > format for its operation, which makes it superfluous for this driver. > > Its functionality is moved into .buf_queue(). > > You're moving the ichan->dma_chan.device->device_prep_slave_sg() call from > .buf_prepare() to .buf_queue(). Is that call cheap ? Otherwise it would be > better to keep the .buf_prepare() callback. But only if (buf->state == CSI_BUF_NEEDS_INIT), i.e., only on the first invocation. In any case, look at idmac_prep_slave_sg() - it is cheap. To do this in .buf_prepare I'd have to store the frame format from the .queue_setup() with a list of indices, to which it applies, and then use it in .buf_prepare()... Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Is DVB ioctl FE_SET_FRONTEND broken?
Hi, As far as I understand it, the FE_SET_FRONTEND ioctl is supposed to tell a DVB device to tune itself, and will send a poll() event when it completes. The "frequency" parameter of this event will be the frequency of the newly tuned channel, or 0 if tuning failed. http://www.linuxtv.org/docs/dvbapi/DVB_Frontend_API.html#SECTION00328000 I have now tested with 2 different DVB adapters, and I don't think the 3.0.x kernel still behaves like this. A study of the dvb-core/dvb_frontend.c file reveals the following code: In the dvb_frontend_ioctl_legacy() function, switch(cmd) { ... case FE_SET_FRONTEND: { ... fepriv->state = FESTATE_RETUNE; /* Request the search algorithm to search */ fepriv->algo_status |= DVBFE_ALGO_SEARCH_AGAIN; dvb_frontend_wakeup(fe); dvb_frontend_add_event(fe, 0); // <--- HERE fepriv->status = 0; err = 0; break; } So basically, the ioctl always sends an event immediately and does not wait for the tuning to happen first. Presumably, the device still tunes in the background and writes the frequency into the frontend's private structure so that a second FE_SET_FRONTEND ioctl succeeds. But this is not the documented behaviour. The bug is visible when you try to use the device for the very first time. I tested by unloading / reloading the kernel modules, launching xine and then pressing its DVB button. This *always* fails the first time, for the reason described above, and works every time after that. Cheers, Chris -- 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: Is DVB ioctl FE_SET_FRONTEND broken?
Hello Chris, On 26.08.2011 01:27, Chris Rankin wrote: > As far as I understand it, the FE_SET_FRONTEND ioctl is supposed to tell a > DVB device to tune itself, and will send a poll() event when it completes. > The "frequency" parameter of this event will be the frequency of the newly > tuned channel, or 0 if tuning failed. > > http://www.linuxtv.org/docs/dvbapi/DVB_Frontend_API.html#SECTION00328000 > > I have now tested with 2 different DVB adapters, and I don't think the 3.0.x > kernel still behaves like this. A study of the dvb-core/dvb_frontend.c file > reveals the following code: > > In the dvb_frontend_ioctl_legacy() function, > > switch(cmd) { > > ... > > case FE_SET_FRONTEND: { > > ... > > fepriv->state = FESTATE_RETUNE; > > /* Request the search algorithm to search */ > fepriv->algo_status |= DVBFE_ALGO_SEARCH_AGAIN; > > dvb_frontend_wakeup(fe); > dvb_frontend_add_event(fe, 0); // <--- HERE > fepriv->status = 0; > err = 0; > break; > } > > So basically, the ioctl always sends an event immediately and does not wait > for the tuning to happen first. Presumably, the device still tunes in the > background and writes the frequency into the frontend's private structure so > that a second FE_SET_FRONTEND ioctl succeeds. But this is not the documented > behaviour. > > The bug is visible when you try to use the device for the very first time. I > tested by unloading / reloading the kernel modules, launching xine and then > pressing its DVB button. This *always* fails the first time, for the reason > described above, and works every time after that. can you please test whether https://patchwork.kernel.org/patch/1036132/ restores the old behaviour? These three pending patches are also related to frontend events: https://patchwork.kernel.org/patch/1036112/ https://patchwork.kernel.org/patch/1036142/ https://patchwork.kernel.org/patch/1036122/ Regards, Andreas -- 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 02/11] ivtv: use kthread_worker instead of workqueue
From: Tejun Heo Upcoming workqueue updates will no longer guarantee fixed workqueue to worker kthread association, so giving RT priority to the irq worker won't work. Use kthread_worker which guarantees specific kthread association instead. This also makes setting the priority cleaner. Signed-off-by: Tejun Heo Cc: Andy Walls Cc: Andrew Morton Cc: ivtv-de...@ivtvdriver.org Cc: linux-media@vger.kernel.org Signed-off-by: Bhanu Prakash Gollapudi --- drivers/media/video/ivtv/ivtv-driver.c | 26 -- drivers/media/video/ivtv/ivtv-driver.h |8 drivers/media/video/ivtv/ivtv-irq.c| 15 +++ drivers/media/video/ivtv/ivtv-irq.h|2 +- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/drivers/media/video/ivtv/ivtv-driver.c b/drivers/media/video/ivtv/ivtv-driver.c index fe7847b..3994642 100644 --- a/drivers/media/video/ivtv/ivtv-driver.c +++ b/drivers/media/video/ivtv/ivtv-driver.c @@ -706,6 +706,8 @@ done: */ static int __devinit ivtv_init_struct1(struct ivtv *itv) { + struct sched_param param = { .sched_priority = 99 }; + itv->base_addr = pci_resource_start(itv->pdev, 0); itv->enc_mbox.max_mbox = 2; /* the encoder has 3 mailboxes (0-2) */ itv->dec_mbox.max_mbox = 1; /* the decoder has 2 mailboxes (0-1) */ @@ -717,13 +719,17 @@ static int __devinit ivtv_init_struct1(struct ivtv *itv) spin_lock_init(&itv->lock); spin_lock_init(&itv->dma_reg_lock); - itv->irq_work_queues = create_singlethread_workqueue(itv->v4l2_dev.name); - if (itv->irq_work_queues == NULL) { - IVTV_ERR("Could not create ivtv workqueue\n"); + init_kthread_worker(&itv->irq_worker); + itv->irq_worker_task = kthread_run(kthread_worker_fn, &itv->irq_worker, + itv->v4l2_dev.name); + if (IS_ERR(itv->irq_worker_task)) { + IVTV_ERR("Could not create ivtv task\n"); return -1; } + /* must use the FIFO scheduler as it is realtime sensitive */ + sched_setscheduler(itv->irq_worker_task, SCHED_FIFO, ¶m); - INIT_WORK(&itv->irq_work_queue, ivtv_irq_work_handler); + init_kthread_work(&itv->irq_work, ivtv_irq_work_handler); /* start counting open_id at 1 */ itv->open_id = 1; @@ -1013,7 +1019,7 @@ static int __devinit ivtv_probe(struct pci_dev *pdev, /* PCI Device Setup */ retval = ivtv_setup_pci(itv, pdev, pci_id); if (retval == -EIO) - goto free_workqueue; + goto free_worker; if (retval == -ENXIO) goto free_mem; @@ -1241,8 +1247,8 @@ free_mem: release_mem_region(itv->base_addr + IVTV_REG_OFFSET, IVTV_REG_SIZE); if (itv->has_cx23415) release_mem_region(itv->base_addr + IVTV_DECODER_OFFSET, IVTV_DECODER_SIZE); -free_workqueue: - destroy_workqueue(itv->irq_work_queues); +free_worker: + kthread_stop(itv->irq_worker_task); err: if (retval == 0) retval = -ENODEV; @@ -1381,9 +1387,9 @@ static void ivtv_remove(struct pci_dev *pdev) ivtv_set_irq_mask(itv, 0x); del_timer_sync(&itv->dma_timer); - /* Stop all Work Queues */ - flush_workqueue(itv->irq_work_queues); - destroy_workqueue(itv->irq_work_queues); + /* Kill irq worker */ + flush_kthread_worker(&itv->irq_worker); + kthread_stop(itv->irq_worker_task); ivtv_streams_cleanup(itv, 1); ivtv_udma_free(itv); diff --git a/drivers/media/video/ivtv/ivtv-driver.h b/drivers/media/video/ivtv/ivtv-driver.h index e8137a2..04bacdb 100644 --- a/drivers/media/video/ivtv/ivtv-driver.h +++ b/drivers/media/video/ivtv/ivtv-driver.h @@ -51,7 +51,7 @@ #include #include #include -#include +#include #include #include #include @@ -261,7 +261,6 @@ struct ivtv_mailbox_data { #define IVTV_F_I_DEC_PAUSED 20 /* the decoder is paused */ #define IVTV_F_I_INITED 21 /* set after first open */ #define IVTV_F_I_FAILED 22 /* set if first open failed */ -#define IVTV_F_I_WORK_INITED 23 /* worker thread was initialized */ /* Event notifications */ #define IVTV_F_I_EV_DEC_STOPPED 28 /* decoder stopped event */ @@ -668,8 +667,9 @@ struct ivtv { /* Interrupts & DMA */ u32 irqmask;/* active interrupts */ u32 irq_rr_idx; /* round-robin stream index */ - struct workqueue_struct *irq_work_queues; /* workqueue for PIO/YUV/VBI actions */ - struct work_struct irq_work_queue; /* work entry */ + struct kthread_worker irq_worker; /* kthread worker for PIO/YUV/VBI actions */ + struct task_struct *irq_worker_task;/* task for irq_worker */ + struct kthread_work irq_work; /* kthread work entry */ spinlock_t dma_reg_lock;