Re: [PATCH 2/2] media: atmel-isi: move configure_geometry() to start_streaming()
Hi Josh,
On Wed, 17 Jun 2015, Josh Wu wrote:
As in set_fmt() function we only need to know which format is been set,
we don't need to access the ISI hardware in this moment.
So move the configure_geometry(), which access the ISI hardware, to
start_streaming() will make code more consistent and simpler.
Signed-off-by: Josh Wu josh...@atmel.com
---
drivers/media/platform/soc_camera/atmel-isi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 8bc40ca..b01086d 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -390,6 +390,11 @@ static int start_streaming(struct vb2_queue *vq,
unsigned int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
+ ret = configure_geometry(isi, icd-user_width, icd-user_height,
+ icd-current_fmt-code);
+ if (ret 0)
+ return ret;
No. Firstly, you'd have to pm_runtime_put() here if you fail. Secondly I
think it's better to fail earlier - at S_FMT, than here. Not accessing the
hardware in S_FMT is a good idea, but I'd at least do all the checking
there. So, maybe add a u32 cfg2_cr field to struct atmel_isi, calculate
it in S_FMT but only write to the hardware in start_streaming()?
Thanks
Guennadi
+
spin_lock_irq(isi-lock);
/* Clear any pending interrupt */
isi_readl(isi, ISI_STATUS);
@@ -477,8 +482,6 @@ static int isi_camera_init_videobuf(struct vb2_queue *q,
static int isi_camera_set_fmt(struct soc_camera_device *icd,
struct v4l2_format *f)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
- struct atmel_isi *isi = ici-priv;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = f-fmt.pix;
@@ -511,16 +514,6 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
if (mf-code != xlate-code)
return -EINVAL;
- /* Enable PM and peripheral clock before operate isi registers */
- pm_runtime_get_sync(ici-v4l2_dev.dev);
-
- ret = configure_geometry(isi, pix-width, pix-height, xlate-code);
-
- pm_runtime_put(ici-v4l2_dev.dev);
-
- if (ret 0)
- return ret;
-
pix-width = mf-width;
pix-height = mf-height;
pix-field = mf-field;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] media: atmel-isi: move configure_geometry() to start_streaming()
Yep, I see the thread and updates to this patch now, please, ignore this
mail, sorry.
Thanks
Guennadi
On Sun, 30 Aug 2015, Guennadi Liakhovetski wrote:
Hi Josh,
On Wed, 17 Jun 2015, Josh Wu wrote:
As in set_fmt() function we only need to know which format is been set,
we don't need to access the ISI hardware in this moment.
So move the configure_geometry(), which access the ISI hardware, to
start_streaming() will make code more consistent and simpler.
Signed-off-by: Josh Wu josh...@atmel.com
---
drivers/media/platform/soc_camera/atmel-isi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 8bc40ca..b01086d 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -390,6 +390,11 @@ static int start_streaming(struct vb2_queue *vq,
unsigned int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
+ ret = configure_geometry(isi, icd-user_width, icd-user_height,
+ icd-current_fmt-code);
+ if (ret 0)
+ return ret;
No. Firstly, you'd have to pm_runtime_put() here if you fail. Secondly I
think it's better to fail earlier - at S_FMT, than here. Not accessing the
hardware in S_FMT is a good idea, but I'd at least do all the checking
there. So, maybe add a u32 cfg2_cr field to struct atmel_isi, calculate
it in S_FMT but only write to the hardware in start_streaming()?
Thanks
Guennadi
+
spin_lock_irq(isi-lock);
/* Clear any pending interrupt */
isi_readl(isi, ISI_STATUS);
@@ -477,8 +482,6 @@ static int isi_camera_init_videobuf(struct vb2_queue *q,
static int isi_camera_set_fmt(struct soc_camera_device *icd,
struct v4l2_format *f)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
- struct atmel_isi *isi = ici-priv;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = f-fmt.pix;
@@ -511,16 +514,6 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
if (mf-code != xlate-code)
return -EINVAL;
- /* Enable PM and peripheral clock before operate isi registers */
- pm_runtime_get_sync(ici-v4l2_dev.dev);
-
- ret = configure_geometry(isi, pix-width, pix-height, xlate-code);
-
- pm_runtime_put(ici-v4l2_dev.dev);
-
- if (ret 0)
- return ret;
-
pix-width = mf-width;
pix-height = mf-height;
pix-field = mf-field;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] media: atmel-isi: setup the ISI_CFG2 register directly
Hi Josh,
On Wed, 17 Jun 2015, Josh Wu wrote:
In the function configure_geometry(), we will setup the ISI CFG2
according to the sensor output format.
It make no sense to just read back the CFG2 register and just set part
of it.
So just set up this register directly makes things simpler.
Simpler doesn't necessarily mean better or more correct:) There are other
fields in that register and currently the driver preserves them, with this
patch you overwrite them with 0. 0 happens to be the reset value of that
register. So, you should at least mention that in this patch description,
just saying simpler doesn't convince me. So, at least I'd modify that, I
can do that myself. But in general I'm not even sure why this patch is
needed. Yes, currently those fields of that register are unused, so, we
can assume they stay at their reset values. But firstly the hardware can
change and at some point the reset value can change, or those other fields
will get set indirectly by something. Or the driver will change at some
point to support more fields of that register and then this code will have
to be changed again. So, I'd ask you again - do you really want this
patch? If you insist - I'll take it, but I'd add the reset value
reasoning. Otherwise maybe just drop it?
Thanks
Guennadi
Currently only support YUV format from camera sensor.
Signed-off-by: Josh Wu josh...@atmel.com
---
drivers/media/platform/soc_camera/atmel-isi.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 9070172..8bc40ca 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -105,24 +105,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
static int configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
- u32 cfg2, cr;
+ u32 cfg2;
+ /* According to sensor's output format to set cfg2 */
switch (code) {
/* YUV, including grey */
case MEDIA_BUS_FMT_Y8_1X8:
- cr = ISI_CFG2_GRAYSCALE;
+ cfg2 = ISI_CFG2_GRAYSCALE;
break;
case MEDIA_BUS_FMT_VYUY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_3;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_2;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_2;
break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_1;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_1;
break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cr = ISI_CFG2_YCC_SWAP_DEFAULT;
+ cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
default:
@@ -130,17 +131,10 @@ static int configure_geometry(struct atmel_isi *isi,
u32 width,
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
- cfg2 = isi_readl(isi, ISI_CFG2);
- /* Set YCC swap mode */
- cfg2 = ~ISI_CFG2_YCC_SWAP_MODE_MASK;
- cfg2 |= cr;
/* Set width */
- cfg2 = ~(ISI_CFG2_IM_HSIZE_MASK);
cfg2 |= ((width - 1) ISI_CFG2_IM_HSIZE_OFFSET)
ISI_CFG2_IM_HSIZE_MASK;
/* Set height */
- cfg2 = ~(ISI_CFG2_IM_VSIZE_MASK);
cfg2 |= ((height - 1) ISI_CFG2_IM_VSIZE_OFFSET)
ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] STV0299 has bogus CAN_INVERSION_AUTO flag
On 24/08/15 16:19, Johann Klammer wrote:
from gdb dump:
[...]
info = {
name = ST STV0299 DVB-S, '\000' repeats 111 times, type = FE_QPSK,
frequency_min = 95, frequency_max = 215,
frequency_stepsize = 125, frequency_tolerance = 0,
symbol_rate_min = 100, symbol_rate_max = 4500,
symbol_rate_tolerance = 500, notifier_delay = 0,
caps = (FE_CAN_INVERSION_AUTO | FE_CAN_FEC_1_2 | FE_CAN_FEC_2_3 |
FE_CAN_FEC_3_4 | FE_CAN_FEC_5_6 | FE_CAN_FEC_7_8 | FE_CAN_FEC_AUTO |
FE_CAN_QPSK)},
[...]
when tuning:
[...]
Hi
..
[331020.207352] stv0299 does not support auto-inversion
[331020.507480] stv0299 does not support auto-inversion
[331020.807610] stv0299 does not support auto-inversion
[331021.107747] stv0299 does not support auto-inversion
[...]
(but how the heck should I know?)
I am using the stv0299 with no problems at all, the code hasn't changed
much in years. I am using linux 4.2-rc6
You shouldn't be getting that message as dvb core does the auto
inversion for the driver.
I looked through the code and can't find any fault.
Regards
Malcolm
--
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 v3 3/4] media: pxa_camera: trivial move of dma irq functions
On Wed, 29 Jul 2015, Robert Jarzmik wrote: From: Robert Jarzmik robert.jarz...@intel.com This moves the dma irq handling functions up in the source file, so that they are available before DMA preparation functions. It prepares the conversion to DMA engine, where the descriptors are populated with these functions as callbacks. Signed-off-by: Robert Jarzmik robert.jarz...@intel.com --- Since v1: fixed prototypes change --- drivers/media/platform/soc_camera/pxa_camera.c | 42 +++--- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/drivers/media/platform/soc_camera/pxa_camera.c b/drivers/media/platform/soc_camera/pxa_camera.c index 39c7e7519b3c..cdfb93aaee43 100644 --- a/drivers/media/platform/soc_camera/pxa_camera.c +++ b/drivers/media/platform/soc_camera/pxa_camera.c @@ -313,6 +313,30 @@ static int calculate_dma_sglen(struct scatterlist *sglist, int sglen, [snip] + pxa_camera_dma_irq(pcdev, DMA_Y); [snip] - pxa_camera_dma_irq(channel, pcdev, DMA_Y); This still seems to break compilation to me. Could you compile-test after each your patch, please? Thanks Guennadi -- 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 v3 3/4] media: pxa_camera: trivial move of dma irq functions
Guennadi Liakhovetski g.liakhovet...@gmx.de writes: This still seems to break compilation to me. Could you compile-test after each your patch, please? Ah yes. Ill timing, I had sent the v4 before having these comments, so I'll have to fix it in v5. Cheers. -- Robert -- 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] STV0299 has bogus CAN_INVERSION_AUTO flag
On 30/08/15 16:07, Johann Klammer wrote:
On 08/30/2015 11:53 AM, Malcolm Priestley wrote:
On 24/08/15 16:19, Johann Klammer wrote:
from gdb dump:
[...]
info = {
name = ST STV0299 DVB-S, '\000' repeats 111 times, type = FE_QPSK,
frequency_min = 95, frequency_max = 215,
frequency_stepsize = 125, frequency_tolerance = 0,
symbol_rate_min = 100, symbol_rate_max = 4500,
symbol_rate_tolerance = 500, notifier_delay = 0,
caps = (FE_CAN_INVERSION_AUTO | FE_CAN_FEC_1_2 | FE_CAN_FEC_2_3 |
FE_CAN_FEC_3_4 | FE_CAN_FEC_5_6 | FE_CAN_FEC_7_8 | FE_CAN_FEC_AUTO |
FE_CAN_QPSK)},
[...]
when tuning:
[...]
Hi
..
[331020.207352] stv0299 does not support auto-inversion
[331020.507480] stv0299 does not support auto-inversion
[331020.807610] stv0299 does not support auto-inversion
[331021.107747] stv0299 does not support auto-inversion
[...]
(but how the heck should I know?)
I am using the stv0299 with no problems at all, the code hasn't changed much in
years. I am using linux 4.2-rc6
boot/vmlinuz-4.1.0-1-586
Something must have changed. I updated kernel and got those messages.
I did not get then before. The vmlinuz.old
points to: boot/vmlinuz-3.14.15
vmlinuz points to: boot/vmlinuz-4.1.0-1-586
(debian testing binary .deb)
You shouldn't be getting that message as dvb core does the auto inversion for
the driver.
It may have been exposed by trying oneshot tuning mode... not sure...
Yes, it is with setting oneshot.
Looks like the FE_CAN_INVERSION_AUTO flag is only bogus in
FE_TUNE_MODE_ONESHOT mode.
Obversely don't set INVERSION_AUTO bearing in mind when you do it
manually you have to tune first with it off and if that fails again with
it on.
Regards
Malcolm
--
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 v4 2/4] media: pxa_camera: move interrupt to tasklet
From: Robert Jarzmik robert.jarz...@intel.com
In preparation for dmaengine conversion, move the camera interrupt
handling into a tasklet. This won't change the global flow, as this
interrupt is only used to detect the end of frame and activate DMA fifos
handling.
Signed-off-by: Robert Jarzmik robert.jarz...@intel.com
---
drivers/media/platform/soc_camera/pxa_camera.c | 44 +-
1 file changed, 29 insertions(+), 15 deletions(-)
diff --git a/drivers/media/platform/soc_camera/pxa_camera.c
b/drivers/media/platform/soc_camera/pxa_camera.c
index aa304950bb98..39c7e7519b3c 100644
--- a/drivers/media/platform/soc_camera/pxa_camera.c
+++ b/drivers/media/platform/soc_camera/pxa_camera.c
@@ -223,6 +223,7 @@ struct pxa_camera_dev {
struct pxa_buffer *active;
struct pxa_dma_desc *sg_tail[3];
+ struct tasklet_struct task_eof;
u32 save_cicr[5];
};
@@ -605,6 +606,7 @@ static void pxa_camera_start_capture(struct pxa_camera_dev
*pcdev)
unsigned long cicr0;
dev_dbg(pcdev-soc_host.v4l2_dev.dev, %s\n, __func__);
+ __raw_writel(__raw_readl(pcdev-base + CISR), pcdev-base + CISR);
/* Enable End-Of-Frame Interrupt */
cicr0 = __raw_readl(pcdev-base + CICR0) | CICR0_ENB;
cicr0 = ~CICR0_EOFM;
@@ -922,13 +924,35 @@ static void pxa_camera_deactivate(struct pxa_camera_dev
*pcdev)
clk_disable_unprepare(pcdev-clk);
}
-static irqreturn_t pxa_camera_irq(int irq, void *data)
+static void pxa_camera_eof(unsigned long arg)
{
- struct pxa_camera_dev *pcdev = data;
- unsigned long status, cifr, cicr0;
+ struct pxa_camera_dev *pcdev = (struct pxa_camera_dev *)arg;
+ unsigned long cifr;
struct pxa_buffer *buf;
struct videobuf_buffer *vb;
+ dev_dbg(pcdev-soc_host.v4l2_dev.dev,
+ Camera interrupt status 0x%x\n,
+ __raw_readl(pcdev-base + CISR));
+
+ /* Reset the FIFOs */
+ cifr = __raw_readl(pcdev-base + CIFR) | CIFR_RESET_F;
+ __raw_writel(cifr, pcdev-base + CIFR);
+
+ pcdev-active = list_first_entry(pcdev-capture,
+struct pxa_buffer, vb.queue);
+ vb = pcdev-active-vb;
+ buf = container_of(vb, struct pxa_buffer, vb);
+ pxa_videobuf_set_actdma(pcdev, buf);
+
+ pxa_dma_start_channels(pcdev);
+}
+
+static irqreturn_t pxa_camera_irq(int irq, void *data)
+{
+ struct pxa_camera_dev *pcdev = data;
+ unsigned long status, cicr0;
+
status = __raw_readl(pcdev-base + CISR);
dev_dbg(pcdev-soc_host.v4l2_dev.dev,
Camera interrupt status 0x%lx\n, status);
@@ -939,20 +963,9 @@ static irqreturn_t pxa_camera_irq(int irq, void *data)
__raw_writel(status, pcdev-base + CISR);
if (status CISR_EOF) {
- /* Reset the FIFOs */
- cifr = __raw_readl(pcdev-base + CIFR) | CIFR_RESET_F;
- __raw_writel(cifr, pcdev-base + CIFR);
-
- pcdev-active = list_first_entry(pcdev-capture,
- struct pxa_buffer, vb.queue);
- vb = pcdev-active-vb;
- buf = container_of(vb, struct pxa_buffer, vb);
- pxa_videobuf_set_actdma(pcdev, buf);
-
- pxa_dma_start_channels(pcdev);
-
cicr0 = __raw_readl(pcdev-base + CICR0) | CICR0_EOFM;
__raw_writel(cicr0, pcdev-base + CICR0);
+ tasklet_schedule(pcdev-task_eof);
}
return IRQ_HANDLED;
@@ -1847,6 +1860,7 @@ static int pxa_camera_probe(struct platform_device *pdev)
pcdev-soc_host.priv= pcdev;
pcdev-soc_host.v4l2_dev.dev= pdev-dev;
pcdev-soc_host.nr = pdev-id;
+ tasklet_init(pcdev-task_eof, pxa_camera_eof, (unsigned long)pcdev);
err = soc_camera_host_register(pcdev-soc_host);
if (err)
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/4] media: pxa_camera: conversion to dmaengine
Convert pxa_camera to dmaengine. This removes all DMA registers
manipulation in favor of the more generic dmaengine API.
The functional level should be the same as before. The biggest change is
in the videobuf_sg_splice() function, which splits a videobuf-dma into
several scatterlists for 3 planes captures (Y, U, V).
Signed-off-by: Robert Jarzmik robert.jarz...@free.fr
---
Since v1: Guennadi's fixes
dma tasklet functions prototypes change (trivial move)
Since v2: sglist cut revamped with Guennadi's comments
Since v3: sglist split removed after Andrew's merge in -mm tree in lib/
taken into account Vinod's change to DMA_CTRL_REUSE
---
drivers/media/platform/soc_camera/Kconfig | 1 +
drivers/media/platform/soc_camera/pxa_camera.c | 404 +++--
2 files changed, 172 insertions(+), 233 deletions(-)
diff --git a/drivers/media/platform/soc_camera/Kconfig
b/drivers/media/platform/soc_camera/Kconfig
index f2776cd415ca..e5e2d6cf6638 100644
--- a/drivers/media/platform/soc_camera/Kconfig
+++ b/drivers/media/platform/soc_camera/Kconfig
@@ -30,6 +30,7 @@ config VIDEO_PXA27x
tristate PXA27x Quick Capture Interface driver
depends on VIDEO_DEV PXA27x SOC_CAMERA
select VIDEOBUF_DMA_SG
+ select SG_SPLIT
---help---
This is a v4l2 driver for the PXA27x Quick Capture Interface
diff --git a/drivers/media/platform/soc_camera/pxa_camera.c
b/drivers/media/platform/soc_camera/pxa_camera.c
index cdfb93aaee43..969140634f3d 100644
--- a/drivers/media/platform/soc_camera/pxa_camera.c
+++ b/drivers/media/platform/soc_camera/pxa_camera.c
@@ -28,6 +28,9 @@
#include linux/clk.h
#include linux/sched.h
#include linux/slab.h
+#include linux/dmaengine.h
+#include linux/dma-mapping.h
+#include linux/dma/pxa-dma.h
#include media/v4l2-common.h
#include media/v4l2-dev.h
@@ -38,7 +41,6 @@
#include linux/videodev2.h
-#include mach/dma.h
#include linux/platform_data/camera-pxa.h
#define PXA_CAM_VERSION 0.0.6
@@ -175,21 +177,16 @@ enum pxa_camera_active_dma {
DMA_V = 0x4,
};
-/* descriptor needed for the PXA DMA engine */
-struct pxa_cam_dma {
- dma_addr_t sg_dma;
- struct pxa_dma_desc *sg_cpu;
- size_t sg_size;
- int sglen;
-};
-
/* buffer for one video frame */
struct pxa_buffer {
/* common v4l buffer stuff -- must be first */
struct videobuf_buffer vb;
u32 code;
/* our descriptor lists for Y, U and V channels */
- struct pxa_cam_dma dmas[3];
+ struct dma_async_tx_descriptor *descs[3];
+ dma_cookie_tcookie[3];
+ struct scatterlist *sg[3];
+ int sg_len[3];
int inwork;
enum pxa_camera_active_dma active_dma;
};
@@ -207,7 +204,7 @@ struct pxa_camera_dev {
void __iomem*base;
int channels;
- unsigned intdma_chans[3];
+ struct dma_chan *dma_chans[3];
struct pxacamera_platform_data *pdata;
struct resource *res;
@@ -222,7 +219,6 @@ struct pxa_camera_dev {
spinlock_t lock;
struct pxa_buffer *active;
- struct pxa_dma_desc *sg_tail[3];
struct tasklet_struct task_eof;
u32 save_cicr[5];
@@ -259,7 +255,6 @@ static int pxa_videobuf_setup(struct videobuf_queue *vq,
unsigned int *count,
static void free_buffer(struct videobuf_queue *vq, struct pxa_buffer *buf)
{
struct soc_camera_device *icd = vq-priv_data;
- struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
struct videobuf_dmabuf *dma = videobuf_to_dma(buf-vb);
int i;
@@ -276,61 +271,40 @@ static void free_buffer(struct videobuf_queue *vq, struct
pxa_buffer *buf)
if (buf-vb.state == VIDEOBUF_NEEDS_INIT)
return;
- for (i = 0; i ARRAY_SIZE(buf-dmas); i++) {
- if (buf-dmas[i].sg_cpu)
- dma_free_coherent(ici-v4l2_dev.dev,
- buf-dmas[i].sg_size,
- buf-dmas[i].sg_cpu,
- buf-dmas[i].sg_dma);
- buf-dmas[i].sg_cpu = NULL;
+ for (i = 0; i 3 buf-descs[i]; i++) {
+ dmaengine_desc_free(buf-descs[i]);
+ kfree(buf-sg[i]);
+ buf-descs[i] = NULL;
+ buf-sg[i] = NULL;
+ buf-sg_len[i] = 0;
}
videobuf_dma_unmap(vq-dev, dma);
videobuf_dma_free(dma);
buf-vb.state = VIDEOBUF_NEEDS_INIT;
-}
-
-static int calculate_dma_sglen(struct scatterlist *sglist, int sglen,
- int sg_first_ofs, int size)
-{
- int i, offset, dma_len, xfer_len;
- struct scatterlist
[PATCH v4 0/4] media: pxa_camera: conversion to dmaengine
Hi Guennadi, This is the forth round. This time sg_split() was a consequence of (a) and (b), ie. sg_split() move into kernel's lib/ directory, and following dmaengine reuse flag introduction, change pxa_camera accordingly. Happy review. Cheers. Robert Jarzmik (4): media: pxa_camera: fix the buffer free path media: pxa_camera: move interrupt to tasklet media: pxa_camera: trivial move of dma irq functions media: pxa_camera: conversion to dmaengine drivers/media/platform/soc_camera/Kconfig | 1 + drivers/media/platform/soc_camera/pxa_camera.c | 478 +++-- 2 files changed, 220 insertions(+), 259 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/4] media: pxa_camera: fix the buffer free path
From: Robert Jarzmik robert.jarz...@intel.com
Fix the error path where the video buffer wasn't allocated nor
mapped. In this case, in the driver free path don't try to unmap memory
which was not mapped in the first place.
Signed-off-by: Robert Jarzmik robert.jarz...@free.fr
---
drivers/media/platform/soc_camera/pxa_camera.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/soc_camera/pxa_camera.c
b/drivers/media/platform/soc_camera/pxa_camera.c
index fcb942de0c7f..aa304950bb98 100644
--- a/drivers/media/platform/soc_camera/pxa_camera.c
+++ b/drivers/media/platform/soc_camera/pxa_camera.c
@@ -272,8 +272,8 @@ static void free_buffer(struct videobuf_queue *vq, struct
pxa_buffer *buf)
* longer in STATE_QUEUED or STATE_ACTIVE
*/
videobuf_waiton(vq, buf-vb, 0, 0);
- videobuf_dma_unmap(vq-dev, dma);
- videobuf_dma_free(dma);
+ if (buf-vb.state == VIDEOBUF_NEEDS_INIT)
+ return;
for (i = 0; i ARRAY_SIZE(buf-dmas); i++) {
if (buf-dmas[i].sg_cpu)
@@ -283,6 +283,8 @@ static void free_buffer(struct videobuf_queue *vq, struct
pxa_buffer *buf)
buf-dmas[i].sg_dma);
buf-dmas[i].sg_cpu = NULL;
}
+ videobuf_dma_unmap(vq-dev, dma);
+ videobuf_dma_free(dma);
buf-vb.state = VIDEOBUF_NEEDS_INIT;
}
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/4] media: pxa_camera: trivial move of dma irq functions
From: Robert Jarzmik robert.jarz...@intel.com
This moves the dma irq handling functions up in the source file, so that
they are available before DMA preparation functions. It prepares the
conversion to DMA engine, where the descriptors are populated with these
functions as callbacks.
Signed-off-by: Robert Jarzmik robert.jarz...@intel.com
---
Since v1: fixed prototypes change
---
drivers/media/platform/soc_camera/pxa_camera.c | 42 +++---
1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/media/platform/soc_camera/pxa_camera.c
b/drivers/media/platform/soc_camera/pxa_camera.c
index 39c7e7519b3c..cdfb93aaee43 100644
--- a/drivers/media/platform/soc_camera/pxa_camera.c
+++ b/drivers/media/platform/soc_camera/pxa_camera.c
@@ -313,6 +313,30 @@ static int calculate_dma_sglen(struct scatterlist *sglist,
int sglen,
return i + 1;
}
+static void pxa_camera_dma_irq(struct pxa_camera_dev *pcdev,
+ enum pxa_camera_active_dma act_dma);
+
+static void pxa_camera_dma_irq_y(int channel, void *data)
+{
+ struct pxa_camera_dev *pcdev = data;
+
+ pxa_camera_dma_irq(pcdev, DMA_Y);
+}
+
+static void pxa_camera_dma_irq_u(int channel, void *data)
+{
+ struct pxa_camera_dev *pcdev = data;
+
+ pxa_camera_dma_irq(pcdev, DMA_U);
+}
+
+static void pxa_camera_dma_irq_v(int channel, void *data)
+{
+ struct pxa_camera_dev *pcdev = data;
+
+ pxa_camera_dma_irq(pcdev, DMA_V);
+}
+
/**
* pxa_init_dma_channel - init dma descriptors
* @pcdev: pxa camera device
@@ -810,24 +834,6 @@ out:
spin_unlock_irqrestore(pcdev-lock, flags);
}
-static void pxa_camera_dma_irq_y(int channel, void *data)
-{
- struct pxa_camera_dev *pcdev = data;
- pxa_camera_dma_irq(channel, pcdev, DMA_Y);
-}
-
-static void pxa_camera_dma_irq_u(int channel, void *data)
-{
- struct pxa_camera_dev *pcdev = data;
- pxa_camera_dma_irq(channel, pcdev, DMA_U);
-}
-
-static void pxa_camera_dma_irq_v(int channel, void *data)
-{
- struct pxa_camera_dev *pcdev = data;
- pxa_camera_dma_irq(channel, pcdev, DMA_V);
-}
-
static struct videobuf_queue_ops pxa_videobuf_ops = {
.buf_setup = pxa_videobuf_setup,
.buf_prepare= pxa_videobuf_prepare,
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] media: pxa_camera: conversion to dmaengine
Guennadi Liakhovetski g.liakhovet...@gmx.de writes:
+last_buf = list_entry(pcdev-capture.prev,
+ struct pxa_buffer, vb.queue);
You can use list_last_entry()
Ok.
+last_status = dma_async_is_tx_complete(pcdev-dma_chans[chan],
+ last_buf-cookie[chan],
+ NULL, last_issued);
+if (camera_status overrun
+last_status != DMA_COMPLETE) {
+dev_dbg(dev, FIFO overrun! CISR: %x\n,
+camera_status);
+pxa_camera_stop_capture(pcdev);
+list_for_each_entry(buf, pcdev-capture, vb.queue)
+pxa_dma_add_tail_buf(pcdev, buf);
Why have you added this loop? Is it a bug in the current implementation or
is it only needed with the switch to dmaengine?
It's a consequence of the switch.
With dmaengine, a dmaengine_terminate_all() removes all queued txs. It is
therefore necessary to requeue them. In the previous implementation, the
chaining was still good, and it was enough to just queue the first
videobuffer : the other buffers would follow by chaining.
Cheers.
--
Robert
--
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 v3 4/4] media: pxa_camera: conversion to dmaengine
Hi Robert,
I assume, the next iteration of your patches won't contain a local copy of
the SG splitting code.
On Wed, 29 Jul 2015, Robert Jarzmik wrote:
Convert pxa_camera to dmaengine. This removes all DMA registers
manipulation in favor of the more generic dmaengine API.
The functional level should be the same as before. The biggest change is
in the videobuf_sg_splice() function, which splits a videobuf-dma into
several scatterlists for 3 planes captures (Y, U, V).
Signed-off-by: Robert Jarzmik robert.jarz...@free.fr
---
Since v1: Guennadi's fixes
dma tasklet functions prototypes change (trivial move)
Since v2: sglist cut revamped with Guennadi's comments
---
drivers/media/platform/soc_camera/pxa_camera.c | 492
++---
1 file changed, 267 insertions(+), 225 deletions(-)
diff --git a/drivers/media/platform/soc_camera/pxa_camera.c
b/drivers/media/platform/soc_camera/pxa_camera.c
index cdfb93aaee43..030ed7413bba 100644
--- a/drivers/media/platform/soc_camera/pxa_camera.c
+++ b/drivers/media/platform/soc_camera/pxa_camera.c
[snip]
@@ -806,28 +824,41 @@ static void pxa_camera_dma_irq(int channel, struct
pxa_camera_dev *pcdev,
buf = container_of(vb, struct pxa_buffer, vb);
WARN_ON(buf-inwork || list_empty(vb-queue));
- dev_dbg(dev, %s channel=%d %s%s(vb=0x%p) dma.desc=%x\n,
- __func__, channel, status DCSR_STARTINTR ? SOF : ,
- status DCSR_ENDINTR ? EOF : , vb, DDADR(channel));
-
- if (status DCSR_ENDINTR) {
- /*
- * It's normal if the last frame creates an overrun, as there
- * are no more DMA descriptors to fetch from QCI fifos
- */
- if (camera_status overrun
- !list_is_last(pcdev-capture.next, pcdev-capture)) {
- dev_dbg(dev, FIFO overrun! CISR: %x\n,
- camera_status);
- pxa_camera_stop_capture(pcdev);
- pxa_camera_start_capture(pcdev);
- goto out;
- }
- buf-active_dma = ~act_dma;
- if (!buf-active_dma) {
- pxa_camera_wakeup(pcdev, vb, buf);
- pxa_camera_check_link_miss(pcdev);
- }
+ /*
+ * It's normal if the last frame creates an overrun, as there
+ * are no more DMA descriptors to fetch from QCI fifos
+ */
+ switch (act_dma) {
+ case DMA_U:
+ chan = 1;
+ break;
+ case DMA_V:
+ chan = 2;
+ break;
+ default:
+ chan = 0;
+ break;
+ }
+ last_buf = list_entry(pcdev-capture.prev,
+ struct pxa_buffer, vb.queue);
You can use list_last_entry()
+ last_status = dma_async_is_tx_complete(pcdev-dma_chans[chan],
+last_buf-cookie[chan],
+NULL, last_issued);
+ if (camera_status overrun
+ last_status != DMA_COMPLETE) {
+ dev_dbg(dev, FIFO overrun! CISR: %x\n,
+ camera_status);
+ pxa_camera_stop_capture(pcdev);
+ list_for_each_entry(buf, pcdev-capture, vb.queue)
+ pxa_dma_add_tail_buf(pcdev, buf);
Why have you added this loop? Is it a bug in the current implementation or
is it only needed with the switch to dmaengine?
+ pxa_camera_start_capture(pcdev);
+ goto out;
+ }
+ buf-active_dma = ~act_dma;
+ if (!buf-active_dma) {
+ pxa_camera_wakeup(pcdev, vb, buf);
+ pxa_camera_check_link_miss(pcdev, last_buf-cookie[chan],
+last_issued);
}
out:
@@ -1014,10 +1045,7 @@ static void pxa_camera_clock_stop(struct
soc_camera_host *ici)
__raw_writel(0x3ff, pcdev-base + CICR0);
/* Stop DMA engine */
- DCSR(pcdev-dma_chans[0]) = 0;
- DCSR(pcdev-dma_chans[1]) = 0;
- DCSR(pcdev-dma_chans[2]) = 0;
-
+ pxa_dma_stop_channels(pcdev);
pxa_camera_deactivate(pcdev);
}
Thanks
Guennadi
--
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] STV0299 has bogus CAN_INVERSION_AUTO flag
On 08/30/2015 11:53 AM, Malcolm Priestley wrote:
On 24/08/15 16:19, Johann Klammer wrote:
from gdb dump:
[...]
info = {
name = ST STV0299 DVB-S, '\000' repeats 111 times, type = FE_QPSK,
frequency_min = 95, frequency_max = 215,
frequency_stepsize = 125, frequency_tolerance = 0,
symbol_rate_min = 100, symbol_rate_max = 4500,
symbol_rate_tolerance = 500, notifier_delay = 0,
caps = (FE_CAN_INVERSION_AUTO | FE_CAN_FEC_1_2 | FE_CAN_FEC_2_3 |
FE_CAN_FEC_3_4 | FE_CAN_FEC_5_6 | FE_CAN_FEC_7_8 | FE_CAN_FEC_AUTO |
FE_CAN_QPSK)},
[...]
when tuning:
[...]
Hi
..
[331020.207352] stv0299 does not support auto-inversion
[331020.507480] stv0299 does not support auto-inversion
[331020.807610] stv0299 does not support auto-inversion
[331021.107747] stv0299 does not support auto-inversion
[...]
(but how the heck should I know?)
I am using the stv0299 with no problems at all, the code hasn't changed much
in years. I am using linux 4.2-rc6
boot/vmlinuz-4.1.0-1-586
Something must have changed. I updated kernel and got those messages.
I did not get then before. The vmlinuz.old
points to: boot/vmlinuz-3.14.15
vmlinuz points to: boot/vmlinuz-4.1.0-1-586
(debian testing binary .deb)
You shouldn't be getting that message as dvb core does the auto inversion for
the driver.
It may have been exposed by trying oneshot tuning mode... not sure...
I had to try that 'coz I only got about 50% success on any tuning attempt on
this card
(has always been like that).
got rid of that, it tunes reliably now if I set INVERSION to 0.
and retry a few times instead of uning once, then polling for LOCK
(which often won't happen).
It's entirely possible the bug was in there for a while.
I looked through the code and can't find any fault.
They explicitly set the CAN_INVERSION_AUTO flag in the FE_GET_INFO ioctl,
then fail -EINVAL when you try to tune with INVERSION_AUTO.
At least that's where the error message comes from.
Regards
Malcolm
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: soc-camera: increase the length of clk_name on soc_of_bind()
Hi Josh, Sorry, I missed the 4.3 merge cycle, but isn't this patch a fix? Isn't it fixing soc-camera / atmel-isi on a specific platform, where the clock name is longer, than currently supported? Is this platform in the mainline and its current camera support is broken because of this? In such a case we could still push it in for 4.3 Thanks Guennadi On Tue, 4 Aug 2015, Josh Wu wrote: Since in soc_of_bind() it may use the of node's full name as the clk_name, and this full name may be longer than 32 characters, take at91 i2c sensor as an example, length is 34 bytes: /ahb/apb/i2c@f8028000/camera@0x30 So this patch increase the clk_name[] array size to 64. It seems big enough so far. Signed-off-by: Josh Wu josh...@atmel.com --- drivers/media/platform/soc_camera/soc_camera.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c index d708df4..fcf3e97 100644 --- a/drivers/media/platform/soc_camera/soc_camera.c +++ b/drivers/media/platform/soc_camera/soc_camera.c @@ -1621,7 +1621,7 @@ static int soc_of_bind(struct soc_camera_host *ici, struct soc_camera_async_client *sasc; struct soc_of_info *info; struct i2c_client *client; - char clk_name[V4L2_SUBDEV_NAME_SIZE]; + char clk_name[V4L2_SUBDEV_NAME_SIZE + 32]; int ret; /* allocate a new subdev and add match info to it */ -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 00/55] MC next generation patches
Em Sun, 30 Aug 2015 00:06:11 -0300 Mauro Carvalho Chehab mche...@osg.samsung.com escreveu: That's the 8th version of the MC next generation patches. Differences from version 7: - Patches reworked to make the reviewers happy; - Bug fixes; - ALSA changes got their own separate patches; - Javier patches got integrated into this series; - media-entity.h structs are now properly documented; - Tested on both au0828 and omap3isp. Due to the complexity of this change, other platform drivers may require some fixes. As the patch series sent before, this is not meant to be sent upstream yet. Its goal is to merge it for Kernel 4.4, in order to give people enough time to review and fix pending issues. As on the previous series, the patches are available on my experimental tree: http://git.linuxtv.org/cgit.cgi/mchehab/experimental.git/log/?h=mc_next_gen There's one additional patch there that removes the backlinks from G_TOPOLOGY ioctl. I'll be posting it in separate at the ML. I also added a new branch: http://git.linuxtv.org/cgit.cgi/mchehab/experimental.git/log/?h=mc_next_gen_test Based on the first one. It contains a hack for au0828 that exposes the tuner also via subdev devnode, and reduces the number of output pads of the DVB demux to just 5, as the default is too high to produce a .dot file that would be useful. Of course, this patch should never leave my experimental tree ;) There are a few other from Javier there meant to allow testing the omap3isp on my Beaglebone (that doesn't have any sensor on it) and on his omap3 devices. I added support at the mc_nextgen_test tool to produce Graphviz .dot files. It is still experimental, but it is good enough to already produce some useful graphs. The newest version is at: http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-utils.git/log/?h=mc-next-gen I added some graphs produced by it at: https://mchehab.fedorapeople.org/mc-next-gen/ Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Mon Aug 31 04:00:14 CEST 2015 git branch: test git hash: d071c833a0d30e7aae0ea565d92ef83c79106d6f gcc version:i686-linux-gcc (GCC) 5.1.0 sparse version: v0.5.0-51-ga53cea2 smatch version: 0.4.1-3153-g7d56ab3 host hardware: x86_64 host os:4.0.0-3.slh.1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: WARNINGS linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12.23-i686: OK linux-3.13.11-i686: OK linux-3.14.9-i686: OK linux-3.15.2-i686: OK linux-3.16.7-i686: OK linux-3.17.8-i686: OK linux-3.18.7-i686: OK linux-3.19-i686: OK linux-4.0-i686: OK linux-4.1.1-i686: OK linux-4.2-rc1-i686: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12.23-x86_64: OK linux-3.13.11-x86_64: OK linux-3.14.9-x86_64: OK linux-3.15.2-x86_64: OK linux-3.16.7-x86_64: OK linux-3.17.8-x86_64: OK linux-3.18.7-x86_64: OK linux-3.19-x86_64: OK linux-4.0-x86_64: OK linux-4.1.1-x86_64: OK linux-4.2-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS smatch: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v3 2/5] media: videobuf2: Restructure vb2_buffer
Hello Hans,
Thank you for your review.
I leave some comments in the body for reply.
Regards,
Junghak
On 08/28/2015 10:31 PM, Hans Verkuil wrote:
Hi Junghak,
Thanks for this patch, it looks much better. I do have a number of comments,
though...
On 08/26/2015 01:59 PM, Junghak Sung wrote:
Remove v4l2-specific stuff from struct vb2_buffer and add member variables
related with buffer management.
struct vb2_plane {
snip
/* plane information */
unsigned intbytesused;
unsigned intlength;
union {
unsigned intoffset;
unsigned long userptr;
int fd;
} m;
unsigned intdata_offset;
}
struct vb2_buffer {
snip
/* buffer information */
unsigned intnum_planes;
unsigned intindex;
unsigned inttype;
unsigned intmemory;
struct vb2_planeplanes[VIDEO_MAX_PLANES];
snip
};
And create struct vb2_v4l2_buffer as container buffer for v4l2 use.
struct vb2_v4l2_buffer {
struct vb2_buffer vb2_buf;
__u32 flags;
__u32 field;
struct timeval timestamp;
struct v4l2_timecodetimecode;
__u32 sequence;
};
This patch includes only changes inside of videobuf2. So, it is required to
modify all device drivers which use videobuf2.
Signed-off-by: Junghak Sung jh1009.s...@samsung.com
Signed-off-by: Geunyoung Kim nenggun@samsung.com
Acked-by: Seung-Woo Kim sw0312@samsung.com
Acked-by: Inki Dae inki@samsung.com
---
drivers/media/v4l2-core/videobuf2-core.c | 324 +-
include/media/videobuf2-core.h | 50 ++---
include/media/videobuf2-v4l2.h | 26 +++
3 files changed, 236 insertions(+), 164 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index ab00ea0..9266d50 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -35,10 +35,10 @@
static int debug;
module_param(debug, int, 0644);
-#define dprintk(level, fmt, arg...) \
- do { \
- if (debug = level)\
- pr_info(vb2: %s: fmt, __func__, ## arg); \
+#define dprintk(level, fmt, arg...)\
+ do {\
+ if (debug = level) \
+ pr_info(vb2: %s: fmt, __func__, ## arg); \
These are just whitespace changes, and that is something it see *a lot* in this
patch. And usually for no clear reason.
Please remove those whitespace changes, it makes a difficult patch even harder
to read than it already is.
I just wanted to remove unnecessary white spaces or adjust alignment.
OK, I will revert those whitespace changes for better review.
} while (0)
#ifdef CONFIG_VIDEO_ADV_DEBUG
snip
@@ -656,12 +652,21 @@ static bool __buffers_in_use(struct vb2_queue *q)
*/
static void __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
{
+ struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
struct vb2_queue *q = vb-vb2_queue;
+ unsigned int plane;
/* Copy back data such as timestamp, flags, etc. */
- memcpy(b, vb-v4l2_buf, offsetof(struct v4l2_buffer, m));
- b-reserved2 = vb-v4l2_buf.reserved2;
- b-reserved = vb-v4l2_buf.reserved;
Hmm, I'm not sure why these reserved fields were copied here. I think it was
for compatibility reasons for some old drivers that abused the reserved field.
However, in the new code these reserved fields should probably be explicitly
initialized to 0.
+ b-index = vb-index;
+ b-type = vb-type;
+ b-memory = vb-memory;
+ b-bytesused = 0;
+
+ b-flags = vbuf-flags;
+ b-field = vbuf-field;
+ b-timestamp = vbuf-timestamp;
+ b-timecode = vbuf-timecode;
+ b-sequence = vbuf-sequence;
if (V4L2_TYPE_IS_MULTIPLANAR(q-type)) {
/*
@@ -669,21 +674,33 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb,
struct v4l2_buffer *b)
* for it. The caller has already verified memory and size.
*/
b-length = vb-num_planes;
- memcpy(b-m.planes, vb-v4l2_planes,
- b-length * sizeof(struct v4l2_plane));
A similar problem occurs here: the memcpy would have copied the reserved
field in v4l2_plane as well, but that no longer happens, so you need to
do an explicit memset for the reserved field in the new code.
It means that
