RE: [RFC] New controls for codec devices

2011-04-05 Thread Jeongtae Park
AAZABlAHYAaQBjAGUAcw
Ax-cr-puzzleid: {D5185033-3373-475A-A768-FCD559F05F0C}

Hi Laurent,
and Hi Kamil.

I think there are some point that I can reply.

First, How about think adding the prefix of common(decoding & encoding) and
encoding controls? Now, only controls for decoding have DECODER_* prefix.
([COMMON], DECODER, ENCODER)

> -Original Message-
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Laurent Pinchart
> Sent: Monday, April 04, 2011 11:51 PM
> To: Kamil Debski
> Cc: linux-media@vger.kernel.org; 'Kyungmin Park'; jaeryul...@samsung.com;
> hansv...@cisco.com; Marek Szyprowski
> Subject: Re: [RFC] New controls for codec devices
> 
> Hi Kamil,
> 
> On Friday 01 April 2011 17:38:26 Kamil Debski wrote:
> > > From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
> > > On Tuesday 29 March 2011 11:48:03 Kamil Debski wrote:
> 
> [snip]
> 
> > > >  2) DECODER_SLICE_INTERFACE D   all common
> > > >
> > > > If set the decoder can accept separate picture slices on input,
> > > > otherwise it requires a whole frame.
> > >
> > > Isn't that a property instead of a settable control ? I'm not sure to
> > > understand why applications would need to tell the decoder what it can
> > > accept. Or is it supposed to be a read-only control ?
> >
> > I think I have not described this control properly. It is used by the
> > application to set what kind of buffer it will send to the driver.
> >
> > A picture can consist of one or more slice. Our hardware can accept two
> > kinds of input buffers and its behavior is determined by a register. So it
> > is a read/write control.  In the first case, a single input buffer contains
> > a whole picture (one or more slices). In the second case, a single input
> > buffer contains a single slice.
> >
> > A nice figure describing pictures and slices can be found here:
> > http://goo.gl/info/bnFe2
> 
> OK I understand it better now.
> 
> Is this configurable at runtime for each buffer, or is it a decoder
> configuration that must be set before the start of the stream ?
> 

Actually, MFC allows to change slice interface at runtime not during slice 
decoding
(after multi-sliced frame decoding when slice interface is enabled).
But we not recommends this method and there may be no use-case like that.


> > > >  3) DECODER_H264_DISPLAY_DELAY  D   H264MFC
> > > >
> > > > Programmable display delay for H264. MFC will return a decoded frame
> > > > after the set number of frames (this may cause that frames are not
> > > > returned in display order).
> > > >
> > > >  4) DECODER_H264_DISPLAY_DELAY_ENABLE   D   H264MFC
> > > >
> > > > Enable display delay for H264.
> > >
> > > Can't display delay enable/disable be controlled through the
> > > DECODER_H264_DISPLAY_DELAY control ? A zero value would mean no display
> > > delay.
> >
> > Firstly I have had the same idea as you have proposed. But in this case
> > zero value is meaningful. If display delay is disabled then it is up to
> > the decoder to determine how many OUTPUT buffers will it process before a
> > buffer on the CAPTURE is dequeued. This is influenced by the number of B
> > frames and how many reference frames are used by P and B frames. This
> > ensures that after the CAPTURE buffer is dequeued it won't be used by the
> > hardware.
> >
> > If the display delay is enabled then the decoder has to return an CAPTURE
> > buffer after processing a certain number of OUTPUT buffers. If this number
> > is low, then it may result in buffers not being dequeued in display order.
> >
> > Even worse, I have just discovered that the buffer can still be used by the
> > hardware as a reference after it has been dequeued in case of low display
> > delay. So I think that this feature is very specific to our hardware (thus
> > I think it should be hw specific control) and application has to be aware
> > of this.
> 
> If I understand this correctly, drivers need to reference CAPTURE buffers
> (containing decoded images) for some time to decode P and B frames. The
> default behaviour is to return a buffer to userspace only when the buffer
> isn't needed by the driver anymore. This behaviour can be overridden using
> the
> display delay control to return CAPTURE buffers sooner, making it possible to
> display frames without an additional delay.
> 
> If applications need to write to the buffers, they can't use that feature.
> Otherwise, why shouldn't drivers always return frames to userspace ASAP ? We
> would then need a single control to tell the driver whether the application
> wants to write to the buffers (in which case they won't be dequeued until the
> driver doesn't need them anymore), or only wants to read from them (in which
> case the driver will dequeued them ASAP).
> 
> > > What is the display delay used for ?
> >
> > As described above - it is used to force hardware to return an CAPTURE
> > buffer after a set nu

Re: [PATCH 1/5] [media] vb2: redesign the stop_streaming() callback and make it obligatory

2011-04-05 Thread Pawel Osciak
Hi again Marek

On Sun, Apr 3, 2011 at 22:49, Marek Szyprowski  wrote:
> Hello,
>
> On Monday, April 04, 2011 1:51 AM Pawel Osciak wrote:
>
>> Drivers are now required to implement the stop_streaming() callback
>> to ensure that all ongoing hardware operations are finished and their
>> ownership of buffers is ceded.
>> Drivers do not have to call vb2_buffer_done() for each buffer they own
>> anymore.
>> Also remove the return value from the callback.
>>
>> Signed-off-by: Pawel Osciak 
>> ---
>>  drivers/media/video/videobuf2-core.c |   16 ++--
>>  include/media/videobuf2-core.h   |   16 +++-
>>  2 files changed, 21 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/video/videobuf2-core.c 
>> b/drivers/media/video/videobuf2-core.c
>> index 6e69584..59d5e8b 100644
>> --- a/drivers/media/video/videobuf2-core.c
>> +++ b/drivers/media/video/videobuf2-core.c
>> @@ -640,6 +640,9 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
>> vb2_buffer_state state)
>>   struct vb2_queue *q = vb->vb2_queue;
>>   unsigned long flags;
>>
>> + if (atomic_read(&q->queued_count) == 0)
>> + return;
>> +
>>   if (vb->state != VB2_BUF_STATE_ACTIVE)
>>   return;
>>
>> @@ -1178,12 +1181,20 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
>>   unsigned int i;
>>
>>   /*
>> -  * Tell driver to stop all transactions and release all queued
>> +  * Tell the driver to stop all transactions and release all queued
>>* buffers.
>>*/
>>   if (q->streaming)
>>   call_qop(q, stop_streaming, q);
>> +
>> + /*
>> +  * All buffers should now not be in use by the driver anymore, but we
>> +  * have to manually set queued_count to 0, as the driver was not
>> +  * required to call vb2_buffer_done() from stop_streaming() for all
>> +  * buffers it had queued.
>> +  */
>>   q->streaming = 0;
>> + atomic_set(&q->queued_count, 0);
>
> If you removed the need to call vb2_buffer_done() then you must also call
> wake_up_all(&q->done_wq) to wake any other threads/processes that might be
> sleeping waiting for buffers.

You made me doubt myself for a second there, but the patch is correct.
There is a call to wake_up_all a few lines below this.

-- 
Best regards,
Pawel Osciak
--
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: [linux-dvb] DVICO HDTV Dual Express2

2011-04-05 Thread Nathan Stitt
I know I shouldn't have quoted mythtv to identify a problem, so I've
tinkered with this a little more and can reproduce it using the dvb
utilities.

I can reproduce the problem using tzap and cat'ing
/dev/dvb/adapter[23]/dvr0 to files. When one of the tuners tunes the
the bad transponder, the files stop growing. Not always immediately,
but typically within a few seconds.

Turning on debugging in various dvb modules shows nothing obviously
suspicious when it occurs, but I notice that when the problem occurs,
when I kill the offending tzap, the following messages are logged.

Apr  6 10:45:39 media kernel: [95234.332590] cx23885_wakeup: 0 buffers
handled (should be 1)
Apr  6 10:45:39 media kernel: [95234.332606] cx23885_wakeup: 0 buffers
handled (should be 1)

But if I kill it without the problem manifesting (either by using a
non-offending transponder, or by killing it before the failure occurs
with the problematic transponder) these messages don't appear.

Also perhaps noteworthy is that once the offending tzap process is
terminated, the file generated from the other resumes growing.

Here are the modules I thought to turn on debugging. Any other suggestions?

nathan@media:/sys/module$ grep -r 1 */parameters/*debug*
af9013/parameters/debug:1
cx23885/parameters/debug:1
cx23885/parameters/i2c_debug:1
cx23885/parameters/vbi_debug:1
cx23885/parameters/video_debug:1
dvb_core/parameters/cam_debug:1
dvb_core/parameters/debug:1
dvb_core/parameters/dvbdev_debug:1
dvb_core/parameters/frontend_debug:1
dvb_usb_af9015/parameters/debug:1
tuner_xc2028/parameters/debug:1
videobuf_core/parameters/debug:1
videobuf_dma_sg/parameters/debug:1
videobuf_dvb/parameters/debug:1
zl10353/parameters/debug:1
zl10353/parameters/debug_regs:1

I'm happy to help track this down further if anyone can advise how to proceed.

(I hope I haven't hijacked this thread, I never saw the earlier
emails, but from what I could tell, it seems to be related to my
experiences).

(sorry for the repeated email, Daniel, I accidentally didn't Reply-to-all)

Regards,
Nathan.
--
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 5/7] v4l: s5p-fimc: add pm_runtime support

2011-04-05 Thread Jonghun Han
AALQBmAGkAbQBjADoAIA
BhAGQAZAAgAHAAbQBfAHIAdQBuAHQAaQBtAGUAIABzAHUAcABwAG8AcgB0AA==
x-cr-puzzleid: {0DF5696E-C27B-4620-A41E-B97F4C401FEA}


Hi Marek,

runtime_pm is used to minimize current.
In my opinion, the followings will be better.
1. Adds pm_runtime_get_sync before running of the first job.
   IMO, dma_run callback function is the best place for calling in case of
M2M.
2. And then in the ISR, call pm_runtime_put_sync in the ISR bottom-half if
there is no remained job.

I had already implemented and tested.
But it remained code cleanup. I hope I can post it on the next week.

Best regards,
Jonghun Han

On Tuesday, April 05, 2011 11:07 PM Marek Szyprowski wrote:
> This patch adds basic support for pm_runtime to s5p-fimc driver. PM
runtime
> support is required to enable the driver on S5PV310 series with power
domain
> driver enabled.
> 
> Signed-off-by: Marek Szyprowski 
> Signed-off-by: Kyungmin Park 
> ---
>  drivers/media/video/s5p-fimc/fimc-capture.c |5 +
>  drivers/media/video/s5p-fimc/fimc-core.c|   14 ++
>  2 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c
> b/drivers/media/video/s5p-fimc/fimc-capture.c
> index 95f8b4e1..f697ed1 100644
> --- a/drivers/media/video/s5p-fimc/fimc-capture.c
> +++ b/drivers/media/video/s5p-fimc/fimc-capture.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -398,6 +399,8 @@ static int fimc_capture_open(struct file *file)
>   if (fimc_m2m_active(fimc))
>   return -EBUSY;
> 
> + pm_runtime_get_sync(&fimc->pdev->dev);
> +
>   if (++fimc->vid_cap.refcnt == 1) {
>   ret = fimc_isp_subdev_init(fimc, 0);
>   if (ret) {
> @@ -428,6 +431,8 @@ static int fimc_capture_close(struct file *file)
>   fimc_subdev_unregister(fimc);
>   }
> 
> + pm_runtime_put_sync(&fimc->pdev->dev);
> +
>   return 0;
>  }
> 
> diff --git a/drivers/media/video/s5p-fimc/fimc-core.c
> b/drivers/media/video/s5p-fimc/fimc-core.c
> index 6c919b3..ead5c0a 100644
> --- a/drivers/media/video/s5p-fimc/fimc-core.c
> +++ b/drivers/media/video/s5p-fimc/fimc-core.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1410,6 +1411,8 @@ static int fimc_m2m_open(struct file *file)
>   if (fimc->vid_cap.refcnt > 0)
>   return -EBUSY;
> 
> + pm_runtime_get_sync(&fimc->pdev->dev);
> +
>   fimc->m2m.refcnt++;
>   set_bit(ST_OUTDMA_RUN, &fimc->state);
> 
> @@ -1452,6 +1455,8 @@ static int fimc_m2m_release(struct file *file)
>   if (--fimc->m2m.refcnt <= 0)
>   clear_bit(ST_OUTDMA_RUN, &fimc->state);
> 
> + pm_runtime_put_sync(&fimc->pdev->dev);
> +
>   return 0;
>  }
> 
> @@ -1649,6 +1654,11 @@ static int fimc_probe(struct platform_device *pdev)
>   goto err_req_region;
>   }
> 
> + pm_runtime_set_active(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
> +
> + pm_runtime_get_sync(&pdev->dev);
> +
>   fimc->num_clocks = MAX_FIMC_CLOCKS - 1;
> 
>   /* Check if a video capture node needs to be registered. */ @@ -
1706,6
> +1716,8 @@ static int fimc_probe(struct platform_device *pdev)
>   dev_dbg(&pdev->dev, "%s(): fimc-%d registered successfully\n",
>   __func__, fimc->id);
> 
> + pm_runtime_put_sync(&pdev->dev);
> +
>   return 0;
> 
>  err_m2m:
> @@ -1740,6 +1752,8 @@ static int __devexit fimc_remove(struct
platform_device
> *pdev)
> 
>   vb2_dma_contig_cleanup_ctx(fimc->alloc_ctx);
> 
> + pm_runtime_disable(&pdev->dev);
> +
>   iounmap(fimc->regs);
>   release_resource(fimc->regs_res);
>   kfree(fimc->regs_res);
> --
> 1.7.1.569.g6f426
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
the
> body of a message to majord...@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Ngene cam device name

2011-04-05 Thread Issa Gorissen
Hello all,

Here is the patch for the NGene card family and the new caio device

Signed-off-by: Issa Gorissen 
---
 drivers/media/dvb/dvb-core/dvbdev.c  |2 +-
 drivers/media/dvb/dvb-core/dvbdev.h  |1 +
 drivers/media/dvb/ngene/ngene-core.c |2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvbdev.c 
b/drivers/media/dvb/dvb-core/dvbdev.c
index f732877..7a64b81 100644
--- a/drivers/media/dvb/dvb-core/dvbdev.c
+++ b/drivers/media/dvb/dvb-core/dvbdev.c
@@ -47,7 +47,7 @@ static DEFINE_MUTEX(dvbdev_register_lock);
 
 static const char * const dnames[] = {
"video", "audio", "sec", "frontend", "demux", "dvr", "ca",
-   "net", "osd"
+   "net", "osd", "caio"
 };
 
 #ifdef CONFIG_DVB_DYNAMIC_MINORS
diff --git a/drivers/media/dvb/dvb-core/dvbdev.h 
b/drivers/media/dvb/dvb-core/dvbdev.h
index fcc6ae9..c63c70d 100644
--- a/drivers/media/dvb/dvb-core/dvbdev.h
+++ b/drivers/media/dvb/dvb-core/dvbdev.h
@@ -47,6 +47,7 @@
 #define DVB_DEVICE_CA 6
 #define DVB_DEVICE_NET7
 #define DVB_DEVICE_OSD8
+#define DVB_DEVICE_CAIO   9
 
 #define DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr) \
static short adapter_nr[] = \
diff --git a/drivers/media/dvb/ngene/ngene-core.c 
b/drivers/media/dvb/ngene/ngene-core.c
index 175a0f6..17cdd38 100644
--- a/drivers/media/dvb/ngene/ngene-core.c
+++ b/drivers/media/dvb/ngene/ngene-core.c
@@ -1523,7 +1523,7 @@ static int init_channel(struct ngene_channel *chan)
set_transfer(&chan->dev->channel[2], 1);
dvb_register_device(adapter, &chan->ci_dev,
&ngene_dvbdev_ci, (void *) chan,
-   DVB_DEVICE_SEC);
+   DVB_DEVICE_CAIO);
if (!chan->ci_dev)
goto err;
}

--
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: TT-budget S2-3200 cannot tune on HB13E DVBS2 transponder

2011-04-05 Thread COEXSI


> -Original Message-
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Issa Gorissen
> Sent: mardi 5 avril 2011 13:00
> To: linux-media@vger.kernel.org
> Subject: TT-budget S2-3200 cannot tune on HB13E DVBS2 transponder
> 
> Hi,
> 
> Eutelsat made a recent migration from DVB-S to DVB-S2 (since 31/3/2011)
> on two transponders on HB13E
> 
> - HOT BIRD 6 13° Est TP 159 Freq 11,681 Ghz DVB-S2 FEC 3/4 27500 Msymb/s
> 0.2 Pilot off Polar H
> 
> - HOT BIRD 9 13° Est TP 99 Freq 12,692 Ghz DVB-S2 FEC 3/4 27500 Msymb/s
> 0.2 Pilot off Polar H
> 

I can confirm that we have a lot of problem with these two transponders and
the TT-S2-3200 card.

Here are some observations:

- It seems that going from DVB-S to DVB-S2 make the antenna settings very
sensitive. We have some sites where everything is working correctly and on
some other sites where we needed to redo the antenna setup, especially the
LNB tilt.

- The STB0899 driver doesn't seem to work correctly: if the reception isn't
really good, we are missing a lot of TS packets and these packets are
altered (mainly garbage). But, the BER returned from the demodulator stay at
zero! (using FE_READ_BER ioctl). By the way, the FE_READ_UNCORRECTED_BLOCKS
ioctl isn't implemented in this driver.

Does anyone can confirm these observations?


> 
> Before those changes, with my TT S2 3200, I was able to watch TV on
> those transponders. Now, I cannot even tune on those transponders. I
> have tried with
> scan-s2 and w_scan and the latest drivers from git. They both find the
> transponders but cannot tune onto it.
> 
> Something noteworthy is that my other card, a DuoFlex S2 can tune fine
> on those transponders.
> 
> My question is; can someone try this as well with a TT S2 3200 and post
> the results ?
> 
> Thank you a lot,
> --
> Issa
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media"
> in the body of a message to majord...@vger.kernel.org More majordomo
> info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: TT-budget S2-3200 cannot tune on HB13E DVBS2 transponder

2011-04-05 Thread Steffen Barszus
On Tue, 05 Apr 2011 13:00:14 +0200
"Issa Gorissen"  wrote:

> Hi,
> 
> Eutelsat made a recent migration from DVB-S to DVB-S2 (since
> 31/3/2011) on two transponders on HB13E
> 
> - HOT BIRD 6 13° Est TP 159 Freq 11,681 Ghz DVB-S2 FEC 3/4 27500
> Msymb/s 0.2 Pilot off Polar H
> 
> - HOT BIRD 9 13° Est TP 99 Freq 12,692 Ghz DVB-S2 FEC 3/4 27500
> Msymb/s 0.2 Pilot off Polar H
> 
> 
> Before those changes, with my TT S2 3200, I was able to watch TV on
> those transponders. Now, I cannot even tune on those transponders. I
> have tried with scan-s2 and w_scan and the latest drivers from git.
> They both find the transponders but cannot tune onto it.
> 
> Something noteworthy is that my other card, a DuoFlex S2 can tune
> fine on those transponders.
> 
> My question is; can someone try this as well with a TT S2 3200 and
> post the results ?

i read something about it lately here (german!): 
http://www.vdr-portal.de/board16-video-disk-recorder/board85-hdtv-dvb-s2/p977938-stb0899-fec-3-4-tester-gesucht/#post977938

It says in stb0899_drv.c function:
static void stb0899_set_iterations(struct stb0899_state *state) 

This:
reg = STB0899_READ_S2REG(STB0899_S2DEMOD, MAX_ITER);
STB0899_SETFIELD_VAL(MAX_ITERATIONS, reg, iter_scale);
stb0899_write_s2reg(state, STB0899_S2DEMOD, STB0899_BASE_MAX_ITER, 
STB0899_OFF0_MAX_ITER, reg);

should be replaced with this:

reg = STB0899_READ_S2REG(STB0899_S2FEC, MAX_ITER);
STB0899_SETFIELD_VAL(MAX_ITERATIONS, reg, iter_scale);
stb0899_write_s2reg(state, STB0899_S2FEC, STB0899_BASE_MAX_ITER, 
STB0899_OFF0_MAX_ITER, reg);

Basically replace STB0899_S2DEMOD with STB0899_S2FEC in this 2 lines
affected.

Kind Regards 

Steffen

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH v2 0/7] locking fixes for cx88

2011-04-05 Thread Jonathan Nieder
Jonathan Nieder wrote:

> Jonathan Nieder (7):
>   [media] cx88: protect per-device driver list with device lock
>   [media] cx88: fix locking of sub-driver operations
>   [media] cx88: hold device lock during sub-driver initialization
>   [media] cx88: use a mutex to protect cx8802_devlist
>   [media] cx88: handle attempts to use unregistered cx88-blackbird
> driver
>   [media] cx88: don't use atomic_t for core->mpeg_users
>   [media] cx88: don't use atomic_t for core->users

Good news: Andreas Huber  tested on a PC with 2
Hauppauge HVR1300 TV cards.

He writes:

> Hi Jonathan, I'm glad to say it works! No more deadlocks,
> no more reference count issues.
>
> I did some stress testing on the driver load and unload mechanism
> with and without the video devices being in use. And it was
> handled all very well.
>
> Great job, thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[cron job] v4l-dvb daily build: ERRORS

2011-04-05 Thread Hans Verkuil
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:Tue Apr  5 19:00:33 CEST 2011
git hash:0b1b920610a4d41c584e97d38b5ce497c4a303d7
gcc version:  i686-linux-gcc (GCC) 4.5.1
host hardware:x86_64
host os:  2.6.32.5

linux-git-armv5: OK
linux-git-armv5-davinci: OK
linux-git-armv5-ixp: OK
linux-git-armv5-omap2: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: WARNINGS
linux-git-powerpc64: OK
linux-git-x86_64: OK
linux-2.6.31.12-i686: ERRORS
linux-2.6.32.6-i686: ERRORS
linux-2.6.33-i686: ERRORS
linux-2.6.34-i686: ERRORS
linux-2.6.35.3-i686: ERRORS
linux-2.6.36-i686: ERRORS
linux-2.6.37-i686: WARNINGS
linux-2.6.38.2-i686: WARNINGS
linux-2.6.31.12-x86_64: ERRORS
linux-2.6.32.6-x86_64: ERRORS
linux-2.6.33-x86_64: ERRORS
linux-2.6.34-x86_64: ERRORS
linux-2.6.35.3-x86_64: ERRORS
linux-2.6.36-x86_64: ERRORS
linux-2.6.37-x86_64: WARNINGS
linux-2.6.38.2-x86_64: WARNINGS
spec-git: OK
sparse: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.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] V4L2 API for flash devices

2011-04-05 Thread Laurent Pinchart
Hi Nayden,

On Tuesday 05 April 2011 17:07:41 Nayden Kanchev wrote:
> On 04/05/2011 04:35 PM, Sakari Ailus wrote:
> > Laurent Pinchart wrote:
> >> On Tuesday 05 April 2011 13:21:03 Sakari Ailus wrote:
> >>> Laurent Pinchart wrote:
>  On Tuesday 05 April 2011 12:23:51 Sakari Ailus wrote:
> > Sakari Ailus wrote:
> >> Laurent Pinchart wrote:
> >>> On Wednesday 30 March 2011 13:05:54 Sakari Ailus wrote:
>  Laurent Pinchart wrote:
> > On Monday 28 March 2011 14:55:40 Sakari Ailus wrote:
> > 
> > [snip]
> > 
> >>V4L2_CID_FLASH_STROBE_MODE (menu; LED)
> >> 
> >> Use hardware or software strobe. If hardware strobe is selected,
> >> the flash controller is a slave in the system where the sensor
> >> produces the strobe signal to the flash.
> >> 
> >> In this case the flash controller setup is limited to
> >> programming strobe timeout and power (LED flash) and the sensor
> >> controls the timing and length of the strobe.
> >> 
> >> enum v4l2_flash_strobe_mode {
> >> 
> >>V4L2_FLASH_STROBE_MODE_SOFTWARE,
> >>V4L2_FLASH_STROBE_MODE_EXT_STROBE,
> >> 
> >> };
> > 
> > [snip]
> > 
> >>V4L2_CID_FLASH_LED_MODE (menu; LED)
> >> 
> >> enum v4l2_flash_led_mode {
> >> 
> >>V4L2_FLASH_LED_MODE_FLASH = 1,
> >>V4L2_FLASH_LED_MODE_TORCH,
> >> 
> >> };
> > 
> > Thinking about this some more, shouldn't we combine the two
> > controls ? They are basically used to configure how the flash
> > LED is controlled: manually (torch mode), automatically by the
> > flash controller (software strobe mode) or automatically by an
> > external component (external strobe mode).
>  
>  That's a good question.
>  
>  The adp1653 supports also additional control (not implemented in
>  the driver, though) that affect hardware strobe length. Based on
>  register setting, the led will be on after strobe either until
>  the timeout expires, or until the strobe signal is high.
>  
>  Should this be also part of the same control, or a different one?
> >>> 
> >>> That can be controlled by a duration control. If the duration is 0,
> >>> the flash is lit for the duration of the external strobe, otherwise
> >>> it's lit for the programmed duration.
> >> 
> >> Sounds good to me.
> > 
> > Thinking about this again; there won't be a separate duration control
>  
>  Why not ? I think we need two timeouts, a watchdog timeout to prevent
>  flash fire or meltdown, and a normal timeout to lit the flash for a
>  user-selected duration.
> >>> 
> >>> Let's assume that an application wants to expose a frame using flash
> >>> with software strobe.
> >>> 
> >>> 1. strobe flash
> >>> 2. qbuf
> >>> 3. streamon
> >>> 4. dqbuf
> >>> 5. streamoff
> >>> 6. ...
> >>> 
> >>> How does an application know how long is the time between 1 -- 4? I'd
> >>> guess that in 6 the application would like to switch off the flash
> >>> instead of specifying a timeout for it.
> >> 
> >> That's a valid use case, and we need to support it. It requires a way to
> >> lit the flash with no timeout other than the watchdog timeout, and a
> >> way to turn it off.
> >> 
> >> However, I'm not sure we should rule out the usefulness of a
> >> duration-based software flash strobe. Can't the two APIs coexist ? Or
> >> do you think a duration-based API is useless ?
> > 
> > I don't want to rule it out, but I don't see use for it either for the
> > time being and thus don't see a reason to specify an API for it. The
> > adp1653 driver does not implement a software based shutdown timeout and
> > I'm not aware of others implementing it either. It was just an idea to
> > overcome the coarse hardware watchdog timeout.
> > 
> > If there is use for it then I think we could bring up the question
> > again: use V4L2_CID_FLASH_TIMEOUT or create V4L2_CID_FLASH_DURATION for
> > it.
> > 
> > What do you think?
> 
> According to me it is a matter of choice whether flash will be started
> and stopped explicitly or timer will be used.
> 
> My personal opinion is that both APIs could coexist although I wouldn't
> use timebased approach. The problem is that timeout should be always a
> little bit higher than actual required time (Sensor frame rate + sensor
> exposure time) because some delays are expected from starting the flash
> till starting the camera. Those delay could vary depending on the
> implementation and could be several ms. In that case flash will stop
> before sensor is ready. So as result half bright half dark images will
> be captured.
> 
> On the other hand if timer is used for stopping it that would optimize
> the flash usage and lower flash duty cycle. Th

Re: HVR-1600 (model 74351 rev F1F5) analog Red Screen

2011-04-05 Thread Eric B Munson
On Tue, Apr 5, 2011 at 10:58 AM, Andy Walls  wrote:
> On Mon, 2011-04-04 at 14:36 -0400, Eric B Munson wrote:
>> On Mon, Apr 4, 2011 at 11:16 AM, Eric B Munson  wrote:
>> > On Mon, Apr 4, 2011 at 9:12 AM, Andy Walls  wrote:
>> >> On Mon, 2011-04-04 at 08:20 -0400, Eric B Munson wrote:
>> >>> I the above mentioned capture card and the digital side of the card
>> >>> works well.  However, when I try to get video from the analog side of
>> >>> the card, all I get is a red screen and no sound regardless of channel
>> >>> requested.  This is a problem I see in 2.6.39-rc1 though I typically
>> >>> run the ubuntu 10.10 kernel with the newest drivers built from source.
>> >>>  Is there something in setup or configuration that I may be missing?
>> >>
>> >> Eric,
>> >>
>> >> You are likely missing the last 3 fixes here:
>> >>
>> >> http://git.linuxtv.org/awalls/media_tree.git?a=shortlog;h=refs/heads/cx18_39
>> >>
>> >> (one of which is critical for analog to work).
>> >>
>> >> Also check the ivtv-users and ivtv-devel list for past discussions on
>> >> the "red screen" showing up for known well supported models and what to
>> >> try.
>> >>
>> > Thanks, I will try hand applying these.
>> >
>>
>> I don't have a red screen anymore, now all get from analog static and
>> mythtv's digital channel scanner now seems broken.
>
> Hmmm.
>
> 1. Please provide the output of dmesg when the cx18 driver loads.
>
> 2. Please provide the output of v4l2-ctl -d /dev/video0 --log status
> when tuned to an analog channel.
>
> 3. Please provide the relevant portion of the mythbackend log where
> where the digital scanner starts and then fails.
>
> 4. Does digital tuning still work in MythTV despite the digital scanner
> not working?
>
> 5. Please don't use MythTV to troubleshoot; it is too complex to
> properly eliminate variables.  Test digital with the dvb utilities
> described here:
>
> http://www.linuxtv.org/wiki/index.php/LinuxTV_dvb-apps
> http://www.linuxtv.org/wiki/index.php/Testing_your_DVB_device
>
> Once I have a channels.conf file made, I usually use azap (ATSC) and
> femon to check that I can tune to a digital channel and get a lock.
> Then I use mplayer to check that the content is viewable.
>
>
> The things that spring to mind that could be wrong:
>
> 1. I didn't check that digital still worked when I added my analog
> changes.  Shame on me, but honestly they *shouldn't* have broken it.
> (Famous last words...)
>
> 2. The tda8290 driver module for the new analog demodulator had an I2C
> address bug introduced recently (hardcoded to the wrong address in
> tda8290 module), but a fix was also applied recently.  You may have the
> bug, but not the fix.
>
> 3. The new HVR-1600 has a worldwide analog tuner that the cx18 driver
> defaults to NTSC-M.  If you use another analog standard, you will need
> to use v4l2-ctl to set the proper standard (PAL-B/G/H/I, SECAM-L/L',
> etc.)
>
>
> (Be advised, I have no time to look at any of this at the moment. The
> soonest would be 11 April.)
>
> Regards,
> Andy
>

No worries on the April 11 date, hard to get upset over the scheduling
of free help :)

I will fetch everything that you requested and make sure to post it
before the 11th.

Thanks for your help.
Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] [media] vb2: redesign the stop_streaming() callback and make it obligatory

2011-04-05 Thread Pawel Osciak
On Sun, Apr 3, 2011 at 22:49, Marek Szyprowski  wrote:
> Hello,
>
> On Monday, April 04, 2011 1:51 AM Pawel Osciak wrote:
>
>> Drivers are now required to implement the stop_streaming() callback
>> to ensure that all ongoing hardware operations are finished and their
>> ownership of buffers is ceded.
>> Drivers do not have to call vb2_buffer_done() for each buffer they own
>> anymore.
>> Also remove the return value from the callback.
>>
>> Signed-off-by: Pawel Osciak 
>> ---
>>  drivers/media/video/videobuf2-core.c |   16 ++--
>>  include/media/videobuf2-core.h       |   16 +++-
>>  2 files changed, 21 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/video/videobuf2-core.c 
>> b/drivers/media/video/videobuf2-core.c
>> index 6e69584..59d5e8b 100644
>> --- a/drivers/media/video/videobuf2-core.c
>> +++ b/drivers/media/video/videobuf2-core.c
>> @@ -640,6 +640,9 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
>> vb2_buffer_state state)
>>       struct vb2_queue *q = vb->vb2_queue;
>>       unsigned long flags;
>>
>> +     if (atomic_read(&q->queued_count) == 0)
>> +             return;
>> +
>>       if (vb->state != VB2_BUF_STATE_ACTIVE)
>>               return;
>>
>> @@ -1178,12 +1181,20 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
>>       unsigned int i;
>>
>>       /*
>> -      * Tell driver to stop all transactions and release all queued
>> +      * Tell the driver to stop all transactions and release all queued
>>        * buffers.
>>        */
>>       if (q->streaming)
>>               call_qop(q, stop_streaming, q);
>> +
>> +     /*
>> +      * All buffers should now not be in use by the driver anymore, but we
>> +      * have to manually set queued_count to 0, as the driver was not
>> +      * required to call vb2_buffer_done() from stop_streaming() for all
>> +      * buffers it had queued.
>> +      */
>>       q->streaming = 0;
>> +     atomic_set(&q->queued_count, 0);
>
> If you removed the need to call vb2_buffer_done() then you must also call
> wake_up_all(&q->done_wq) to wake any other threads/processes that might be
> sleeping waiting for buffers.

True, setting queued_count to 0 is not enough. Hm, I'm wondering why
tests on vivi and qv4l2 didn't catch this, it uses poll as well...

-- 
Best regards,
Pawel Osciak
--
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: vb2: stop_streaming() callback redesign

2011-04-05 Thread Pawel Osciak
Hi Laurent,

On Mon, Apr 4, 2011 at 03:27, Laurent Pinchart
 wrote:
> Hi Pawel,
>
> On Monday 04 April 2011 01:51:05 Pawel Osciak wrote:
>> Hi,
>>
>> This series implements a slight redesign of the stop_streaming() callback
>> in vb2. The callback has been made obligatory. The drivers are expected to
>> finish all hardware operations and cede ownership of all buffers before
>> returning, but are not required to call vb2_buffer_done() for any of them.
>> The return value from this callback has also been removed.
>
> What's the rationale behind this patch set ? I've always been against vb2
> controlling the stream state (vb2 should handle buffer management only in my
> opinion) and I'd like to understand why you want to make it required.
>

I might have overstated the intention saying it was a 'redesign'. It
actually doesn't change the overall stop_streaming callback idea, I am
just simplifying it with this patch, while also emphasizing its role
by making it obligatory. Drivers were always required to finish
everything they were doing with the buffers before returning from
stop_streaming. But until now, stop_streaming was expecting the driver
to call vb2_buffer_done for all buffers it received via buf_queue.
We've decided it's superfluous, so I am removing this requirement.
Also, I didn't see any use for the return value from stop_streaming so
I removed it as well. Apart from the above, nothing has really
changed.

> I plan to use vb2 in the uvcvideo driver (when vb2 will provide a way to
> handle device disconnection), and uvcvideo will stop the stream before calling
> vb2_queue_release() and vb2_streamoff(). Would will I need a stop_stream
> operation ?

I actually just yesterday noticed your response from a couple of weeks
ago to my comments to your original buf_queue proposal in my ever
growing pile of mail, sorry about that, I will reply to that as soon
as I have time to properly read it and think about it. Nevertheless, I
have the same question as Marek here, would there be anything
preventing you from doing that in stop_streaming?

-- 
Best regards,
Pawel Osciak
--
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 13/34] media/radio-maxiradio: Drop __TIME__ usage

2011-04-05 Thread Michal Marek
The kernel already prints its build timestamp during boot, no need to
repeat it in random drivers and produce different object files each
time.

Cc: Mauro Carvalho Chehab 
Cc: linux-media@vger.kernel.org
Signed-off-by: Michal Marek 
---
 drivers/media/radio/radio-maxiradio.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/media/radio/radio-maxiradio.c 
b/drivers/media/radio/radio-maxiradio.c
index 5c2a905..e83e840 100644
--- a/drivers/media/radio/radio-maxiradio.c
+++ b/drivers/media/radio/radio-maxiradio.c
@@ -412,8 +412,7 @@ static int __devinit maxiradio_init_one(struct pci_dev 
*pdev, const struct pci_d
goto err_out_free_region;
}
 
-   v4l2_info(v4l2_dev, "version " DRIVER_VERSION
-   " time " __TIME__ "  " __DATE__ "\n");
+   v4l2_info(v4l2_dev, "version " DRIVER_VERSION "\n");
 
v4l2_info(v4l2_dev, "found Guillemot MAXI Radio device (io = 0x%x)\n",
   dev->io);
-- 
1.7.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 14/34] media/cx231xx: Drop __TIME__ usage

2011-04-05 Thread Michal Marek
The kernel already prints its build timestamp during boot, no need to
repeat it in random drivers and produce different object files each
time.

Cc: Mauro Carvalho Chehab 
Cc: linux-media@vger.kernel.org
Signed-off-by: Michal Marek 
---
 drivers/media/video/cx231xx/cx231xx-avcore.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/cx231xx/cx231xx-avcore.c 
b/drivers/media/video/cx231xx/cx231xx-avcore.c
index c53e972..ff5cb50 100644
--- a/drivers/media/video/cx231xx/cx231xx-avcore.c
+++ b/drivers/media/video/cx231xx/cx231xx-avcore.c
@@ -1356,7 +1356,7 @@ void cx231xx_dump_SC_reg(struct cx231xx *dev)
 {
u8 value[4] = { 0, 0, 0, 0 };
int status = 0;
-   cx231xx_info("cx231xx_dump_SC_reg %s!\n", __TIME__);
+   cx231xx_info("cx231xx_dump_SC_reg!\n");
 
status = cx231xx_read_ctrl_reg(dev, VRT_GET_REGISTER, BOARD_CFG_STAT,
 value, 4);
-- 
1.7.4.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: HVR-1600 (model 74351 rev F1F5) analog Red Screen

2011-04-05 Thread Andy Walls
On Mon, 2011-04-04 at 14:36 -0400, Eric B Munson wrote:
> On Mon, Apr 4, 2011 at 11:16 AM, Eric B Munson  wrote:
> > On Mon, Apr 4, 2011 at 9:12 AM, Andy Walls  wrote:
> >> On Mon, 2011-04-04 at 08:20 -0400, Eric B Munson wrote:
> >>> I the above mentioned capture card and the digital side of the card
> >>> works well.  However, when I try to get video from the analog side of
> >>> the card, all I get is a red screen and no sound regardless of channel
> >>> requested.  This is a problem I see in 2.6.39-rc1 though I typically
> >>> run the ubuntu 10.10 kernel with the newest drivers built from source.
> >>>  Is there something in setup or configuration that I may be missing?
> >>
> >> Eric,
> >>
> >> You are likely missing the last 3 fixes here:
> >>
> >> http://git.linuxtv.org/awalls/media_tree.git?a=shortlog;h=refs/heads/cx18_39
> >>
> >> (one of which is critical for analog to work).
> >>
> >> Also check the ivtv-users and ivtv-devel list for past discussions on
> >> the "red screen" showing up for known well supported models and what to
> >> try.
> >>
> > Thanks, I will try hand applying these.
> >
> 
> I don't have a red screen anymore, now all get from analog static and
> mythtv's digital channel scanner now seems broken.

Hmmm.

1. Please provide the output of dmesg when the cx18 driver loads.

2. Please provide the output of v4l2-ctl -d /dev/video0 --log status
when tuned to an analog channel.

3. Please provide the relevant portion of the mythbackend log where
where the digital scanner starts and then fails.

4. Does digital tuning still work in MythTV despite the digital scanner
not working?

5. Please don't use MythTV to troubleshoot; it is too complex to
properly eliminate variables.  Test digital with the dvb utilities
described here: 

http://www.linuxtv.org/wiki/index.php/LinuxTV_dvb-apps
http://www.linuxtv.org/wiki/index.php/Testing_your_DVB_device

Once I have a channels.conf file made, I usually use azap (ATSC) and
femon to check that I can tune to a digital channel and get a lock.
Then I use mplayer to check that the content is viewable.


The things that spring to mind that could be wrong:

1. I didn't check that digital still worked when I added my analog
changes.  Shame on me, but honestly they *shouldn't* have broken it.
(Famous last words...)

2. The tda8290 driver module for the new analog demodulator had an I2C
address bug introduced recently (hardcoded to the wrong address in
tda8290 module), but a fix was also applied recently.  You may have the
bug, but not the fix.

3. The new HVR-1600 has a worldwide analog tuner that the cx18 driver
defaults to NTSC-M.  If you use another analog standard, you will need
to use v4l2-ctl to set the proper standard (PAL-B/G/H/I, SECAM-L/L',
etc.)


(Be advised, I have no time to look at any of this at the moment. The
soonest would be 11 April.)

Regards,
Andy



--
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: DVB-T frequencies for Tenerife, Spain

2011-04-05 Thread Christoph Pfister
2011/4/3 Cédric Martínez Campos :
> Hi there,
>
> attached to this e-mail, you will find the DVB-T frequencies for my
> present region: Tenerife in Spain. It would be great if you could
> include it in the dvb-apps package together.

Added, thanks.

Christoph


> Sincerely yours,
>
> Dr. Cédric M. Campos
> Dept. Matemática Fundamental
> Universidad de La Laguna, ULL
> Avda. Astrofísico Fco. Sánchez, s/n.
> 38206 San Cristóbal de La Laguna (SPAIN)
>
> Tel:  +34 922 318 163
> Fax: +34 922 318 145
>
> Antes de imprimir este correo-e, por favor ten en cuenta el medio ambiente.
> Please bear in mind the Environment before printing this email.
--
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: soc_camera dynamically cropping and scaling

2011-04-05 Thread Paolo Santinelli
Hi Guennadi,

thank you for the useful explanation. I will try!

Paolo.

2011/4/5 Guennadi Liakhovetski :
> On Mon, 4 Apr 2011, Paolo Santinelli wrote:
>
>> Hi Guennadi,
>>
>> I have tried to implement the changes you suggested me in order to get
>> the new live cropping capabilities. I am able to successfully compile
>> the patched driver even though I still have problems  when an
>> application program tries to do live cropping (calling run time the
>> same ."ioctl VIDIOC_S_CROP" but with different cropping parameters).
>>
>> This is the run time error I get when  I call the "ioctl
>> VIDIOC_S_CROP" in order to change live the  cropping area:
>
> Yes, this is correct, my patches don't add any new ioctl()s.
>
>> ov9655 0-0030: Scaled for 320x240 (320x240) : 0x0
>> ov9655 0-0030: Adjusted for 320x240 : hstart 240, hstop 560 = 320,
>> vstart 11, vstop 252 = 241
>> pxa27x-camera pxa27x-camera.0: Output after crop: 320x240
>>
>> pxa27x-camera pxa27x-camera.0: DMA Bus Error IRQ!
>> pxa27x-camera pxa27x-camera.0: DMA Bus Error IRQ!
>> pxa27x-camera pxa27x-camera.0: DMA Bus Error IRQ!
>> select timeout
>
> Well, you certainly realise, that sh-mobile and PXA270 are two absolutely
> different architectures, using different IPs for there video capture
> interfaces and DMA engines. What I have provided to you is a generic patch
> for soc-camera, which you have to take as is, which you've also done. But
> the sh-mobile part was just an example, which you could only take as a
> basis for your PXA work. But for PXA you would actually have to _develop_
> (test and debug) your own implementation, which may well end up pretty
> different from the sh-mobile version. PXA270 doesn't implement any video
> scaling, and the driver currently doesn't implement any host-side
> cropping. So, you have to make sure, that your sensor produces exactly the
> same size output in both your cropped modes. And then just go wild and
> experiment with the driver until you get the desired result:-)
>
> Good luck
> Guennadi
>
>> Here is what I have done:
>>
>> 1) I have used the kernel  linux-2.6.39-rc1;
>> 2) I have carefully  added the patch at the  soc_camera.c and
>> soc_camera.h files as you indicated in your mail "[PATCH 1/2] V4L:
>> soc-camera: add a livecrop host operation";
>> 4) I have changed pxa_camera.c file trying to put the patch as you
>> have indicated for the sh_mobile_ceu_camera.c, that is:
>>
>> -I have changed the "pxa_camera_dev" struct  instead of
>> "sh_mobile_ceu_dev" adding the elements "struct completion complete"
>> and "unsigned int frozen:1;"
>>
>> -I have added the .set_livecrop() method. Inside this method I  have
>> replaced the  prefix "sh_mobile_ceu_"  with "pxa_camera_". This method
>> calls the function "sh_mobile_ceu_capture()" but doesn't exist an
>> equivalent function named "pxa_camera_capture()", instead there are
>> the two functions "pxa_camera_start_capture()" and
>> "pxa_camera_stop_capture()". So I call "pxa_camera_start_capture()"
>> instead of "sh_mobile_ceu_capture()", but I am not sure this is the
>> right solution;
>>
>> -I have changed the  "pxa_camera_start_capture()" function instead of
>> "sh_mobile_ceu_capture()" adding at the end "if (pcdev->frozen)
>> complete(&pcdev->complete);" even if I'm afraid that is not good.
>>
>> -Of course I have  added the line ".set_livecrop      =
>> sh_mobile_ceu_set_livecrop," at the structure  "static struct
>> soc_camera_host_ops pxa_soc_camera_host_ops" instead of  "static
>> struct soc_camera_host_ops sh_mobile_ceu_host_ops".
>>
>> -I didn't change anything else.
>>
>> How to do the live cropping from the application program ? Do I have
>> to invoke the usual "ioctl VIDIOC_S_CROP" or I have to invoke the
>> ".livecrop()" method adding a new "ioctl VIDIOC_S_LIVECROP" ?
>>
>>
>> Thank you very much.
>>
>> Paolo
>>
>>
>>
>> 2011/3/30 Paolo Santinelli :
>> > OK!
>> >
>> > Thanks
>> >
>> > Paolo
>> >
>> > 2011/3/30 Guennadi Liakhovetski :
>> >> On Wed, 30 Mar 2011, Paolo Santinelli wrote:
>> >>
>> >>> Hi Guennadi,
>> >>>
>> >>> Am I wrong or do  I have to add some functions ?
>> >>>
>> >>> I have hand applied the changes at the soc_camera.c and soc_camera.h
>> >>> files. At a fist glance to these files seems that I have to add the
>> >>> function:
>> >>>
>> >>>  .set_livecrop()
>> >>
>> >> Yes, I think, I mentioned this in my last mail, that's what my sh-ceu
>> >> example should have illustrated.
>> >>
>> >>>
>> >>> and probably even something more:
>> >>>
>> >>>   CC      drivers/media/video/soc_camera.o
>> >>> drivers/media/video/soc_camera.c: In function 'soc_camera_s_fmt_vid_cap':
>> >>> drivers/media/video/soc_camera.c:545: error: implicit declaration of
>> >>> function 'vb2_is_streaming'
>> >>> drivers/media/video/soc_camera.c:545: error: 'struct
>> >>> soc_camera_device' has no member named 'vb2_vidq'
>> >>> drivers/media/video/soc_camera.c: In function 'soc_camera_s_crop':
>> >>> drivers/media/video/soc_camera.c:799: error: 'struct
>> >>> soc_camer

Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Sakari Ailus
Laurent Pinchart wrote:
> Hi Guennadi,

Hi all,

> On Tuesday 05 April 2011 14:39:19 Guennadi Liakhovetski wrote:
>> On Tue, 5 Apr 2011, Laurent Pinchart wrote:
>>> On Friday 01 April 2011 10:13:02 Guennadi Liakhovetski wrote:
 A possibility to preallocate and initialise buffers of different sizes
 in V4L2 is required for an efficient implementation of asnapshot mode.
 This patch adds three new ioctl()s: VIDIOC_CREATE_BUFS,
 VIDIOC_DESTROY_BUFS, and VIDIOC_SUBMIT_BUF and defines respective data
 structures.
> 
> [snip]
> 
 diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
 index aa6c393..b6ef46e 100644
 --- a/include/linux/videodev2.h
 +++ b/include/linux/videodev2.h
 @@ -1847,6 +1847,26 @@ struct v4l2_dbg_chip_ident {
> 
> [snip]
> 
 +/* struct v4l2_createbuffers::flags */
 +#define V4L2_BUFFER_FLAG_NO_CACHE_INVALIDATE  (1 << 0)
>>>
>>> Shouldn't cache management be handled at submit/qbuf time instead of
>>> being a buffer property ?
>>
>> hmm, I'd prefer fixing it at create. Or do you want to be able to create
>> buffers and then submit / queue them with different flags?...
> 
> That's the idea, yes. I'm not sure yet how useful that would be though.

I agree that it'd be nice to support this. It depends on where the data
is being used.

For example, you could have an algorithm on the host side which does
process the image data but only uses every second frame, with no other
processing done on the host CPU. In this case the cache would be flushed
needlessly for the frames that are not touched by the CPU.

I admit that this is fine optimisation but I don't think that should be
ruled out either.

The default cache behaviour would still be to flush not to break
existing applications, so I don't think this would be a significant
burden for applications.

Cheers,

-- 
Sakari Ailus
sakari.ai...@maxwell.research.nokia.com
--
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: vb2: stop_streaming() callback redesign

2011-04-05 Thread Marek Szyprowski
Hello,

On Monday, April 04, 2011 12:27 PM Laurent Pinchart wrote:

> Hi Pawel,
> 
> On Monday 04 April 2011 01:51:05 Pawel Osciak wrote:
> > Hi,
> >
> > This series implements a slight redesign of the stop_streaming() callback
> > in vb2. The callback has been made obligatory. The drivers are expected to
> > finish all hardware operations and cede ownership of all buffers before
> > returning, but are not required to call vb2_buffer_done() for any of them.
> > The return value from this callback has also been removed.
> 
> What's the rationale behind this patch set ? I've always been against vb2
> controlling the stream state (vb2 should handle buffer management only in my
> opinion) and I'd like to understand why you want to make it required.

Let me remind the rationale behind {start,stop}_streaming. Basically there are
more than one place where you should change the DMA streaming state, some of
which are quite obvious (like stream_{on,off}), the others are a bit more
surprising (like the recently discussed first call to poll()). Also some of
the vb2 operations behaves differently if streaming is enabled or not
(like dqbuf), so vb2 needs to be aware of streaming state change.

The idea is also to simplify the drivers and provide a one-to-one functions for
all typical v4l2 operations: req_bufs, query_bufs, q_buf, dq_buf, stream_on,
stream_off, mmap, read/write, poll, so implementation of all from this list can
be a simple 4 lines of code, like the following:

static int vidioc_streamon(struct file *file, void *priv, enum v4l2_buf_type i)
{
struct vivi_dev *dev = video_drvdata(file);
return vb2_streamon(&dev->vb_vidq, i);
}

> I plan to use vb2 in the uvcvideo driver (when vb2 will provide a way to
> handle device disconnection), and uvcvideo will stop the stream before calling
> vb2_queue_release() and vb2_streamoff(). Would will I need a stop_stream
> operation ?

What's prevents you from moving the dma streaming stop call from stop_streaming
ioctl and release file operation?

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: [PATCH 0/2] V4L: Extended crop/compose API

2011-04-05 Thread Tomasz Stanislawski

Laurent Pinchart wrote:

Hi Tomasz,

On Tuesday 29 March 2011 12:38:50 Tomasz Stanislawski wrote:

Hans Verkuil wrote:

On Tuesday, March 29, 2011 11:22:17 Tomasz Stanislawski wrote:

Hans Verkuil wrote:

On Monday, March 28, 2011 17:19:54 Tomasz Stanislawski wrote:

Hello everyone,

This patch-set introduces new ioctls to V4L2 API. The new method for
configuration of cropping and composition is presented.

There is some confusion in understanding of a cropping in current
version of V4L2. For CAPTURE devices cropping refers to choosing only a
part of input data stream and processing it and storing it in a memory
buffer. The buffer is fully filled by data. It is not possible to
choose only a part of a buffer for being updated by hardware.

In case of OUTPUT devices, the whole content of a buffer is passed by
hardware to output display. Cropping means selecting only a part of an
output display/signal. It is not possible to choose only a part for a
memory buffer to be processed.

The overmentioned flaws in cropping API were discussed in post:
http://article.gmane.org/gmane.linux.drivers.video-input-

infrastructure/28945

A solution was proposed during brainstorming session in Warsaw.

Hello. Thank you for a quick comment.


I don't have time right now to review this RFC in-depth, but one thing
that needs more attention is the relationship between these new ioctls
and CROPCAP.
And also how this relates to analog inputs (I don't think analog
outputs make any sense). And would a COMPOSECAP ioctl make sense?

Maybe two new ioctl COMPOSECAP and EXTCROPCAP should be added.
For input CROPCAP maps to EXTCROPCAP, for output it maps to COMPOSECAP.
The output EXTCROPCAP would return dimentions of a buffer.
But in my opinion field v4l2_selection::bounds should be added to
structure below. In such a case G_EXTCROP could be used to obtain
cropping bounds.


Using flags to tell G_EXTCROP and G_COMPOSE whether we want to retrieve the 
bounds, default rectangle or current rectangle would be a sensible option in 
my opinion.


I am preparing second version of this RFC that added support for 
obtaining bounds and defrect using G_{EXTCROP / COMPOSE}. It will be 
posted soon.

There is more in CROPCAP than just the bounds. I'd have to think about
this myself.


[snip]


3. Hints

The v4l2_selection::flags field is used to give a driver a hint about
coordinate adjustments.  Below one can find the proposition of
adjustment flags. The syntax is V4L2_SEL_{name}_{LE/GE}, where {name}
refer to a

field in


struct v4l2_rect. The LE is abbreviation from "lesser or equal".  It

prevents


the driver form increasing a parameter. In similar fashion GE means

"greater or


equal" and it disallows decreasing. Combining LE and GE flags prevents

the


driver from any adjustments of parameters.  In such a manner, setting

flags


field to zero would give a driver a free hand in coordinate
adjustment.

#define V4L2_SEL_WIDTH_GE   0x0001
#define V4L2_SEL_WIDTH_LE   0x0002
#define V4L2_SEL_HEIGHT_GE  0x0004
#define V4L2_SEL_HEIGHT_LE  0x0008
#define V4L2_SEL_LEFT_GE0x0010
#define V4L2_SEL_LEFT_LE0x0020
#define V4L2_SEL_TOP_GE 0x0040
#define V4L2_SEL_TOP_LE 0x0080

Wouldn't you also need similar flags for RIGHT and BOTTOM?

Regards,

Hans

Proposed flags refer to fields in v4l2_rect structure. These are left,
top, width and height. Fields bottom and right and not present in
v4l2_rect structure.

But what if I want to keep the bottom right corner fixed, and allow other
parameters to change? I don't think you can do that with the current set
of flags.

Regards,

Hans

You are right. New flags should be added:

#define V4L2_SEL_RIGHT_GE   0x0100
#define V4L2_SEL_RIGHT_LE   0x0200
#define V4L2_SEL_BOTTOM_GE  0x0400
#define V4L2_SEL_BOTTOM_LE  0x0800

They  would be used to inform a driver about adjusting a bottom-right
corner. Right and bottom would be defined as:

right = v4l2_rect::left + v4l2_rect::width
bottom = v4l2_rect::top + v4l2_rect::height


What if you want to allow the rectangle to be slightly enlarged or reduced, 
but want to keep it centered ? I feel like this would get out of hands quite 
fast. Hints are not a bad idea, but they will become very complex to implement 
in drivers, especially when the hardware forces all kind of weird constraints 
(and we all know that hardware designers get extremely creative in that area 
:-)).
Yes.. it may be complex. Unfortunately, currently there is no way to 
control a crop adjustment. The proposed solution is not perfect but I 
think it is sufficient to deal with the most common situations.
Quite probably some simple framework or helper functions have to be 
added to V4L2 kernel API to help driver to deal with a business logic 
hidden behind crop adjustments.


Regards,
Tomasz Stanislawski




#define V4L2_SEL_WIDTH_FIXED0x0003
#define V4L2_SEL_HEIGHT_FIXED   0x0

[PATCH 4/7] v4l: videobuf2: add IOMMU based DMA memory allocator

2011-04-05 Thread Marek Szyprowski
From: Andrzej Pietrasiewicz 

This patch adds new videobuf2 memory allocator dedicated to devices that
supports IOMMU DMA mappings. A device with IOMMU module and a driver
with include/iommu.h compatible interface is required. This allocator
aquires memory with standard alloc_page() call and doesn't suffer from
memory fragmentation issues.

Signed-off-by: Andrzej Pietrasiewicz 
Signed-off-by: Kyungmin Park 
Signed-off-by: Marek Szyprowski 
---
 drivers/media/video/Kconfig   |8 +-
 drivers/media/video/Makefile  |1 +
 drivers/media/video/videobuf2-dma-iommu.c |  469 +
 include/media/videobuf2-dma-iommu.h   |   48 +++
 4 files changed, 525 insertions(+), 1 deletions(-)
 create mode 100644 drivers/media/video/videobuf2-dma-iommu.c
 create mode 100644 include/media/videobuf2-dma-iommu.h

diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 4498b94..40d7bcc 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -60,12 +60,18 @@ config VIDEOBUF2_VMALLOC
select VIDEOBUF2_MEMOPS
tristate
 
-
 config VIDEOBUF2_DMA_SG
#depends on HAS_DMA
select VIDEOBUF2_CORE
select VIDEOBUF2_MEMOPS
tristate
+
+config VIDEOBUF2_DMA_IOMMU
+   select GENERIC_ALLOCATOR
+   select VIDEOBUF2_CORE
+   select VIDEOBUF2_MEMOPS
+   tristate
+
 #
 # Multimedia Video device configuration
 #
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index ace5d8b..04136f6 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -118,6 +118,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS)  += 
videobuf2-memops.o
 obj-$(CONFIG_VIDEOBUF2_VMALLOC)+= videobuf2-vmalloc.o
 obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
 obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
+obj-$(CONFIG_VIDEOBUF2_DMA_IOMMU)  += videobuf2-dma-iommu.o
 
 obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
 
diff --git a/drivers/media/video/videobuf2-dma-iommu.c 
b/drivers/media/video/videobuf2-dma-iommu.c
new file mode 100644
index 000..c208e6d
--- /dev/null
+++ b/drivers/media/video/videobuf2-dma-iommu.c
@@ -0,0 +1,469 @@
+/*
+ * videobuf2-dma-iommu.c - IOMMU based memory allocator for videobuf2
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ *
+ * Author: Andrzej Pietrasiewicz 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/*
+ * 17: single piece of memory (one bitmap entry) equals 128k,
+ * so by default the genalloc's bitmap occupies 4kB (one page
+ * for a number of architectures)
+ */
+#define VB2_DMA_IOMMU_PIECE_ORDER  17
+
+/* -1: use default node id to allocate gen_pool/gen_pool_chunk structure from 
*/
+#define VB2_DMA_IOMMU_NODE_ID  -1
+
+/*
+ * starting address of the virtual address space of the client device
+ * must not be zero
+ */
+#define VB2_DMA_IOMMU_MEM_BASE 0x3000
+
+/* size of the virtual address space of the client device */
+#define VB2_DMA_IOMMU_MEM_SIZE 0x4000
+
+struct vb2_dma_iommu_alloc_ctx {
+   struct device *dev;
+   struct gen_pool *pool;
+   struct iommu_domain *domain;
+};
+
+struct vb2_dma_iommu_desc {
+   unsigned long   size;
+   unsigned intnum_pages;
+   struct page **pages;
+};
+
+struct vb2_dma_iommu_buf {
+   unsigned long   drv_addr;
+   unsigned long   vaddr;
+
+   struct vb2_dma_iommu_desc   info;
+   int offset;
+   atomic_trefcount;
+   int write;
+   struct vm_area_struct   *vma;
+   boolcontig;
+
+   struct vb2_vmarea_handler   handler;
+
+   struct vb2_dma_iommu_alloc_ctx  *ctx;
+};
+
+static void vb2_dma_iommu_put(void *buf_priv);
+
+static void *vb2_dma_iommu_alloc(void *alloc_ctx, unsigned long size)
+{
+   struct vb2_dma_iommu_alloc_ctx *ctx = alloc_ctx;
+   struct vb2_dma_iommu_buf *buf;
+   unsigned long tmp;
+   int i, ret;
+
+   BUG_ON(NULL == alloc_ctx);
+
+   buf = kzalloc(sizeof *buf, GFP_KERNEL);
+   if (!buf)
+   return NULL;
+
+   buf->drv_addr = gen_pool_alloc(ctx->pool, size);
+   if (0 == buf->drv_addr)
+   goto gen_pool_alloc_fail;
+
+   buf->info.size = size;
+   buf->info.num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
+   buf->ctx = ctx;
+   buf->info.pages = kzalloc(
+   buf->info.num_pages * sizeof(struct page *), GFP_KERNEL);
+   if (!buf->info.pages)
+   got

[PATCH 5/7] v4l: s5p-fimc: add pm_runtime support

2011-04-05 Thread Marek Szyprowski
This patch adds basic support for pm_runtime to s5p-fimc driver. PM
runtime support is required to enable the driver on S5PV310 series with
power domain driver enabled.

Signed-off-by: Marek Szyprowski 
Signed-off-by: Kyungmin Park 
---
 drivers/media/video/s5p-fimc/fimc-capture.c |5 +
 drivers/media/video/s5p-fimc/fimc-core.c|   14 ++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c 
b/drivers/media/video/s5p-fimc/fimc-capture.c
index 95f8b4e1..f697ed1 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -398,6 +399,8 @@ static int fimc_capture_open(struct file *file)
if (fimc_m2m_active(fimc))
return -EBUSY;
 
+   pm_runtime_get_sync(&fimc->pdev->dev);
+
if (++fimc->vid_cap.refcnt == 1) {
ret = fimc_isp_subdev_init(fimc, 0);
if (ret) {
@@ -428,6 +431,8 @@ static int fimc_capture_close(struct file *file)
fimc_subdev_unregister(fimc);
}
 
+   pm_runtime_put_sync(&fimc->pdev->dev);
+
return 0;
 }
 
diff --git a/drivers/media/video/s5p-fimc/fimc-core.c 
b/drivers/media/video/s5p-fimc/fimc-core.c
index 6c919b3..ead5c0a 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1410,6 +1411,8 @@ static int fimc_m2m_open(struct file *file)
if (fimc->vid_cap.refcnt > 0)
return -EBUSY;
 
+   pm_runtime_get_sync(&fimc->pdev->dev);
+
fimc->m2m.refcnt++;
set_bit(ST_OUTDMA_RUN, &fimc->state);
 
@@ -1452,6 +1455,8 @@ static int fimc_m2m_release(struct file *file)
if (--fimc->m2m.refcnt <= 0)
clear_bit(ST_OUTDMA_RUN, &fimc->state);
 
+   pm_runtime_put_sync(&fimc->pdev->dev);
+
return 0;
 }
 
@@ -1649,6 +1654,11 @@ static int fimc_probe(struct platform_device *pdev)
goto err_req_region;
}
 
+   pm_runtime_set_active(&pdev->dev);
+   pm_runtime_enable(&pdev->dev);
+
+   pm_runtime_get_sync(&pdev->dev);
+
fimc->num_clocks = MAX_FIMC_CLOCKS - 1;
 
/* Check if a video capture node needs to be registered. */
@@ -1706,6 +1716,8 @@ static int fimc_probe(struct platform_device *pdev)
dev_dbg(&pdev->dev, "%s(): fimc-%d registered successfully\n",
__func__, fimc->id);
 
+   pm_runtime_put_sync(&pdev->dev);
+
return 0;
 
 err_m2m:
@@ -1740,6 +1752,8 @@ static int __devexit fimc_remove(struct platform_device 
*pdev)
 
vb2_dma_contig_cleanup_ctx(fimc->alloc_ctx);
 
+   pm_runtime_disable(&pdev->dev);
+
iounmap(fimc->regs);
release_resource(fimc->regs_res);
kfree(fimc->regs_res);
-- 
1.7.1.569.g6f426
--
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 7/7] ARM: EXYNOS4: enable FIMC on Universal_C210

2011-04-05 Thread Marek Szyprowski
This patch adds definitions to enable support for s5p-fimc driver
together with required power domains and sysmmu controller on Universal
C210 board.

Signed-off-by: Marek Szyprowski 
Signed-off-by: Kyungmin Park 
---
 arch/arm/mach-exynos4/Kconfig   |6 ++
 arch/arm/mach-exynos4/mach-universal_c210.c |   22 ++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
index e849f67..544a594 100644
--- a/arch/arm/mach-exynos4/Kconfig
+++ b/arch/arm/mach-exynos4/Kconfig
@@ -148,12 +148,18 @@ config MACH_ARMLEX4210
 config MACH_UNIVERSAL_C210
bool "Mobile UNIVERSAL_C210 Board"
select CPU_EXYNOS4210
+   select S5P_DEV_FIMC0
+   select S5P_DEV_FIMC1
+   select S5P_DEV_FIMC2
+   select S5P_DEV_FIMC3
select S3C_DEV_HSMMC
select S3C_DEV_HSMMC2
select S3C_DEV_HSMMC3
select S3C_DEV_I2C1
select S3C_DEV_I2C5
select S5P_DEV_ONENAND
+   select EXYNOS4_DEV_PD
+   select EXYNOS4_DEV_SYSMMU
select EXYNOS4_SETUP_I2C1
select EXYNOS4_SETUP_I2C5
select EXYNOS4_SETUP_SDHCI
diff --git a/arch/arm/mach-exynos4/mach-universal_c210.c 
b/arch/arm/mach-exynos4/mach-universal_c210.c
index 97d329f..7ff2f5f 100644
--- a/arch/arm/mach-exynos4/mach-universal_c210.c
+++ b/arch/arm/mach-exynos4/mach-universal_c210.c
@@ -27,9 +27,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 
 #include 
+#include 
 
 /* Following are default values for UCON, ULCON and UFCON UART registers */
 #define UNIVERSAL_UCON_DEFAULT (S3C2410_UCON_TXILEVEL |\
@@ -613,6 +616,15 @@ static struct platform_device *universal_devices[] 
__initdata = {
&s3c_device_hsmmc2,
&s3c_device_hsmmc3,
&s3c_device_i2c5,
+   &s5p_device_fimc0,
+   &s5p_device_fimc1,
+   &s5p_device_fimc2,
+   &s5p_device_fimc3,
+   &exynos4_device_pd[PD_CAM],
+   &exynos4_device_sysmmu[S5P_SYSMMU_FIMC0],
+   &exynos4_device_sysmmu[S5P_SYSMMU_FIMC1],
+   &exynos4_device_sysmmu[S5P_SYSMMU_FIMC2],
+   &exynos4_device_sysmmu[S5P_SYSMMU_FIMC3],
 
/* Universal Devices */
&universal_gpio_keys,
@@ -638,6 +650,16 @@ static void __init universal_machine_init(void)
 
/* Last */
platform_add_devices(universal_devices, ARRAY_SIZE(universal_devices));
+
+   s5p_device_fimc0.dev.parent = &exynos4_device_pd[PD_CAM].dev;
+   s5p_device_fimc1.dev.parent = &exynos4_device_pd[PD_CAM].dev;
+   s5p_device_fimc2.dev.parent = &exynos4_device_pd[PD_CAM].dev;
+   s5p_device_fimc3.dev.parent = &exynos4_device_pd[PD_CAM].dev;
+   exynos4_device_sysmmu[S5P_SYSMMU_FIMC0].dev.parent = 
&exynos4_device_pd[PD_CAM].dev;
+   exynos4_device_sysmmu[S5P_SYSMMU_FIMC1].dev.parent = 
&exynos4_device_pd[PD_CAM].dev;
+   exynos4_device_sysmmu[S5P_SYSMMU_FIMC2].dev.parent = 
&exynos4_device_pd[PD_CAM].dev;
+   exynos4_device_sysmmu[S5P_SYSMMU_FIMC3].dev.parent = 
&exynos4_device_pd[PD_CAM].dev;
+
 }
 
 MACHINE_START(UNIVERSAL_C210, "UNIVERSAL_C210")
-- 
1.7.1.569.g6f426
--
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 6/7] v4l: s5p-fimc: Add support for vb2-dma-iommu allocator

2011-04-05 Thread Marek Szyprowski
This patch adds support for videobuf2-dma-iommu allocator to s5p-fimc
driver. This allocator is selected only on systems that contains support
for S5P SYSMMU module (like EXYNOS4 platform). Otherwise the standard
videobuf2-dma-contig is used.

Signed-off-by: Marek Szyprowski 
Signed-off-by: Kyungmin Park 
---
 drivers/media/video/Kconfig |3 +-
 drivers/media/video/s5p-fimc/fimc-capture.c |4 +-
 drivers/media/video/s5p-fimc/fimc-core.c|   24 ---
 drivers/media/video/s5p-fimc/fimc-core.h|1 +
 drivers/media/video/s5p-fimc/fimc-mem.h |  103 +++
 5 files changed, 122 insertions(+), 13 deletions(-)
 create mode 100644 drivers/media/video/s5p-fimc/fimc-mem.h

diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 40d7bcc..bf2d55d 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -1031,7 +1031,8 @@ config VIDEO_MEM2MEM_TESTDEV
 config  VIDEO_SAMSUNG_S5P_FIMC
tristate "Samsung S5P FIMC (video postprocessor) driver"
depends on VIDEO_DEV && VIDEO_V4L2 && PLAT_S5P
-   select VIDEOBUF2_DMA_CONTIG
+   select VIDEOBUF2_DMA_IOMMU if S5P_SYSTEM_MMU
+   select VIDEOBUF2_DMA_CONTIG if !S5P_SYSTEM_MMU
select V4L2_MEM2MEM_DEV
help
  This is a v4l2 driver for the S5P camera interface
diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c 
b/drivers/media/video/s5p-fimc/fimc-capture.c
index f697ed1..714f0df 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -29,9 +29,9 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "fimc-core.h"
+#include "fimc-mem.h"
 
 static struct v4l2_subdev *fimc_subdev_register(struct fimc_dev *fimc,
struct s5p_fimc_isp_info *isp_info)
@@ -884,7 +884,7 @@ int fimc_register_capture_device(struct fimc_dev *fimc)
q->io_modes = VB2_MMAP | VB2_USERPTR;
q->drv_priv = fimc->vid_cap.ctx;
q->ops = &fimc_capture_qops;
-   q->mem_ops = &vb2_dma_contig_memops;
+   q->mem_ops = &fimc_vb2_allocator_memops;
q->buf_struct_size = sizeof(struct fimc_vid_buffer);
 
vb2_queue_init(q);
diff --git a/drivers/media/video/s5p-fimc/fimc-core.c 
b/drivers/media/video/s5p-fimc/fimc-core.c
index ead5c0a..594c471 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -27,9 +27,9 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "fimc-core.h"
+#include "fimc-mem.h"
 
 static char *fimc_clocks[MAX_FIMC_CLOCKS] = {
"sclk_fimc", "fimc", "sclk_cam"
@@ -457,7 +457,7 @@ int fimc_prepare_addr(struct fimc_ctx *ctx, struct 
vb2_buffer *vb,
dbg("memplanes= %d, colplanes= %d, pix_size= %d",
frame->fmt->memplanes, frame->fmt->colplanes, pix_size);
 
-   paddr->y = vb2_dma_contig_plane_paddr(vb, 0);
+   paddr->y = fimc_vb2_plane_addr(vb, 0);
 
if (frame->fmt->memplanes == 1) {
switch (frame->fmt->colplanes) {
@@ -485,10 +485,10 @@ int fimc_prepare_addr(struct fimc_ctx *ctx, struct 
vb2_buffer *vb,
}
} else {
if (frame->fmt->memplanes >= 2)
-   paddr->cb = vb2_dma_contig_plane_paddr(vb, 1);
+   paddr->cb = fimc_vb2_plane_addr(vb, 1);
 
if (frame->fmt->memplanes == 3)
-   paddr->cr = vb2_dma_contig_plane_paddr(vb, 2);
+   paddr->cr = fimc_vb2_plane_addr(vb, 2);
}
 
dbg("PHYS_ADDR: y= 0x%X  cb= 0x%X cr= 0x%X ret= %d",
@@ -1378,7 +1378,7 @@ static int queue_init(void *priv, struct vb2_queue 
*src_vq,
src_vq->io_modes = VB2_MMAP | VB2_USERPTR;
src_vq->drv_priv = ctx;
src_vq->ops = &fimc_qops;
-   src_vq->mem_ops = &vb2_dma_contig_memops;
+   src_vq->mem_ops = &fimc_vb2_allocator_memops;
src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
 
ret = vb2_queue_init(src_vq);
@@ -1390,7 +1390,7 @@ static int queue_init(void *priv, struct vb2_queue 
*src_vq,
dst_vq->io_modes = VB2_MMAP | VB2_USERPTR;
dst_vq->drv_priv = ctx;
dst_vq->ops = &fimc_qops;
-   dst_vq->mem_ops = &vb2_dma_contig_memops;
+   dst_vq->mem_ops = &fimc_vb2_allocator_memops;
dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
 
return vb2_queue_init(dst_vq);
@@ -1688,12 +1688,15 @@ static int fimc_probe(struct platform_device *pdev)
goto err_clk;
}
 
-   /* Initialize contiguous memory allocator */
-   fimc->alloc_ctx = vb2_dma_contig_init_ctx(&fimc->pdev->dev);
+   /* Initialize memory allocator */
+   fimc->alloc_ctx = fimc_vb2_allocator_init(pdev, fimc);
if (IS_ERR(fimc->alloc_ctx)) {
ret = PTR_ERR(fimc->alloc_ctx);
goto err_irq;
}
+   ret = fimc_vb2_allocator_enable(fimc->alloc_ctx);
+   if 

[PATCH 3/7] v4l: videobuf2: dma-sg: move some generic functions to memops

2011-04-05 Thread Marek Szyprowski
From: Andrzej Pietrasiewicz 

This patch moves some generic code to videobuf2-memops. This code will
be later used by the iommu allocator. This patch adds also vma locking
in user pointer mode.

Signed-off-by: Andrzej Pietrasiewicz 
Signed-off-by: Kyungmin Park 
Signed-off-by: Marek Szyprowski 
CC: Pawel Osciak 
---
 drivers/media/video/videobuf2-dma-sg.c |   37 +--
 drivers/media/video/videobuf2-memops.c |   76 
 include/media/videobuf2-memops.h   |5 ++
 3 files changed, 93 insertions(+), 25 deletions(-)

diff --git a/drivers/media/video/videobuf2-dma-sg.c 
b/drivers/media/video/videobuf2-dma-sg.c
index b2d9485..240abaa 100644
--- a/drivers/media/video/videobuf2-dma-sg.c
+++ b/drivers/media/video/videobuf2-dma-sg.c
@@ -29,6 +29,7 @@ struct vb2_dma_sg_buf {
struct vb2_dma_sg_desc  sg_desc;
atomic_trefcount;
struct vb2_vmarea_handler   handler;
+   struct vm_area_struct   *vma;
 };
 
 static void vb2_dma_sg_put(void *buf_priv);
@@ -150,15 +151,9 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, 
unsigned long vaddr,
if (!buf->pages)
goto userptr_fail_pages_array_alloc;
 
-   down_read(¤t->mm->mmap_sem);
-   num_pages_from_user = get_user_pages(current, current->mm,
-vaddr & PAGE_MASK,
-buf->sg_desc.num_pages,
-write,
-1, /* force */
-buf->pages,
-NULL);
-   up_read(¤t->mm->mmap_sem);
+   num_pages_from_user = vb2_get_user_pages(vaddr, buf->sg_desc.num_pages,
+buf->pages, write, &buf->vma);
+
if (num_pages_from_user != buf->sg_desc.num_pages)
goto userptr_fail_get_user_pages;
 
@@ -177,6 +172,8 @@ userptr_fail_get_user_pages:
   num_pages_from_user, buf->sg_desc.num_pages);
while (--num_pages_from_user >= 0)
put_page(buf->pages[num_pages_from_user]);
+   if (buf->vma)
+   vb2_put_vma(buf->vma);
kfree(buf->pages);
 
 userptr_fail_pages_array_alloc:
@@ -200,6 +197,8 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
   __func__, buf->sg_desc.num_pages);
if (buf->vaddr)
vm_unmap_ram(buf->vaddr, buf->sg_desc.num_pages);
+   if (buf->vma)
+   vb2_put_vma(buf->vma);
while (--i >= 0) {
if (buf->write)
set_page_dirty_lock(buf->pages[i]);
@@ -236,28 +235,16 @@ static unsigned int vb2_dma_sg_num_users(void *buf_priv)
 static int vb2_dma_sg_mmap(void *buf_priv, struct vm_area_struct *vma)
 {
struct vb2_dma_sg_buf *buf = buf_priv;
-   unsigned long uaddr = vma->vm_start;
-   unsigned long usize = vma->vm_end - vma->vm_start;
-   int i = 0;
+   int ret;
 
if (!buf) {
printk(KERN_ERR "No memory to map\n");
return -EINVAL;
}
 
-   do {
-   int ret;
-
-   ret = vm_insert_page(vma, uaddr, buf->pages[i++]);
-   if (ret) {
-   printk(KERN_ERR "Remapping memory, error: %d\n", ret);
-   return ret;
-   }
-
-   uaddr += PAGE_SIZE;
-   usize -= PAGE_SIZE;
-   } while (usize > 0);
-
+   ret = vb2_insert_pages(vma, buf->pages);
+   if (ret)
+   return ret;
 
/*
 * Use common vm_area operations to track buffer refcount.
diff --git a/drivers/media/video/videobuf2-memops.c 
b/drivers/media/video/videobuf2-memops.c
index 5370a3a..9d44473 100644
--- a/drivers/media/video/videobuf2-memops.c
+++ b/drivers/media/video/videobuf2-memops.c
@@ -185,6 +185,82 @@ int vb2_mmap_pfn_range(struct vm_area_struct *vma, 
unsigned long paddr,
 EXPORT_SYMBOL_GPL(vb2_mmap_pfn_range);
 
 /**
+ * vb2_get_user_pages() - pin user pages
+ * @vaddr: virtual address from which to start
+ * @num_pages: number of pages to pin
+ * @pages: table of pointers to struct pages to pin
+ * @write: if 0, the pages must not be written to
+ * @vma:   output parameter, copy of the vma or NULL
+ * if get_user_pages fails
+ *
+ * This function just forwards invocation to get_user_pages, but eases using
+ * the latter in videobuf2 allocators.
+ */
+int vb2_get_user_pages(unsigned long vaddr, unsigned int num_pages,
+  struct page **pages, int write, struct vm_area_struct 
**vma)
+{
+   struct vm_area_struct *found_vma;
+   struct mm_struct *mm = current->mm;
+   int ret = -EFAULT;
+
+   down_read(¤t->mm->mmap_sem);
+
+   found_vma = find_vma(mm, vaddr);
+   if (NULL == found_vma || found_vma->vm_end < (vaddr + num_pages * 
PAGE_SIZE)

[PATCH 1/7] ARM: EXYNOS4: power domains: fixes and code cleanup

2011-04-05 Thread Marek Szyprowski
From: Tomasz Stanislawski 

This patch extends power domain driver with support for enabling and
disabling modules in S5P_CLKGATE_BLOCK register. It also performs a
little code cleanup to avoid confusion between exynos4_device_pd array
index and power domain id.

Signed-off-by: Tomasz Stanislawski 
Signed-off-by: Kyungmin Park 
Signed-off-by: Marek Szyprowski 
---
 arch/arm/mach-exynos4/dev-pd.c  |   93 +--
 arch/arm/mach-exynos4/include/mach/regs-clock.h |7 ++
 arch/arm/plat-samsung/include/plat/pd.h |1 +
 3 files changed, 79 insertions(+), 22 deletions(-)

diff --git a/arch/arm/mach-exynos4/dev-pd.c b/arch/arm/mach-exynos4/dev-pd.c
index 3273f25..44c6597 100644
--- a/arch/arm/mach-exynos4/dev-pd.c
+++ b/arch/arm/mach-exynos4/dev-pd.c
@@ -16,13 +16,17 @@
 #include 
 
 #include 
+#include 
 
 #include 
 
+static DEFINE_SPINLOCK(gate_block_slock);
+
 static int exynos4_pd_enable(struct device *dev)
 {
struct samsung_pd_info *pdata =  dev->platform_data;
u32 timeout;
+   int ret = 0;
 
__raw_writel(S5P_INT_LOCAL_PWR_EN, pdata->base);
 
@@ -31,21 +35,39 @@ static int exynos4_pd_enable(struct device *dev)
while ((__raw_readl(pdata->base + 0x4) & S5P_INT_LOCAL_PWR_EN)
!= S5P_INT_LOCAL_PWR_EN) {
if (timeout == 0) {
-   printk(KERN_ERR "Power domain %s enable failed.\n",
-   dev_name(dev));
-   return -ETIMEDOUT;
+   dev_err(dev, "enable failed\n");
+   ret = -ETIMEDOUT;
+   goto done;
}
timeout--;
udelay(100);
}
 
-   return 0;
+   /* configure clk gate mask if it is present */
+   if (pdata->gate_mask) {
+   unsigned long flags;
+   unsigned long value;
+
+   spin_lock_irqsave(&gate_block_slock, flags);
+
+   value  = __raw_readl(S5P_CLKGATE_BLOCK);
+   value |= pdata->gate_mask;
+   __raw_writel(value, S5P_CLKGATE_BLOCK);
+
+   spin_unlock_irqrestore(&gate_block_slock, flags);
+   }
+
+done:
+   dev_info(dev, "enable finished\n");
+
+   return ret;
 }
 
 static int exynos4_pd_disable(struct device *dev)
 {
struct samsung_pd_info *pdata =  dev->platform_data;
u32 timeout;
+   int ret = 0;
 
__raw_writel(0, pdata->base);
 
@@ -53,81 +75,108 @@ static int exynos4_pd_disable(struct device *dev)
timeout = 10;
while (__raw_readl(pdata->base + 0x4) & S5P_INT_LOCAL_PWR_EN) {
if (timeout == 0) {
-   printk(KERN_ERR "Power domain %s disable failed.\n",
-   dev_name(dev));
-   return -ETIMEDOUT;
+   dev_err(dev, "disable failed\n");
+   ret = -ETIMEDOUT;
+   goto done;
}
timeout--;
udelay(100);
}
 
-   return 0;
+   if (pdata->gate_mask) {
+   unsigned long flags;
+   unsigned long value;
+
+   spin_lock_irqsave(&gate_block_slock, flags);
+
+   value  = __raw_readl(S5P_CLKGATE_BLOCK);
+   value &= ~pdata->gate_mask;
+   __raw_writel(value, S5P_CLKGATE_BLOCK);
+
+   spin_unlock_irqrestore(&gate_block_slock, flags);
+   }
+done:
+   dev_info(dev, "disable finished\n");
+
+   return ret;
 }
 
 struct platform_device exynos4_device_pd[] = {
-   {
+   [PD_MFC] = {
.name   = "samsung-pd",
-   .id = 0,
+   .id = PD_MFC,
.dev = {
.platform_data = &(struct samsung_pd_info) {
.enable = exynos4_pd_enable,
.disable= exynos4_pd_disable,
.base   = S5P_PMU_MFC_CONF,
+   .gate_mask  = S5P_CLKGATE_BLOCK_MFC,
},
},
-   }, {
+   },
+   [PD_G3D] = {
.name   = "samsung-pd",
-   .id = 1,
+   .id = PD_G3D,
.dev = {
.platform_data = &(struct samsung_pd_info) {
.enable = exynos4_pd_enable,
.disable= exynos4_pd_disable,
.base   = S5P_PMU_G3D_CONF,
+   .gate_mask  = S5P_CLKGATE_BLOCK_G3D,
},
},
-   }, {
+   },
+   [PD_LCD0] = {
.name   = "samsung-pd",
-   .id = 2,
+   .id = PD_LCD0,
.dev 

[RFC/PATCH v2 0/7] Samsung IOMMU videobuf2 allocator and s5p-fimc update

2011-04-05 Thread Marek Szyprowski
Hello,

This is a second version of the Samsung IOMMU driver, videobuf2
allocator for IOMMU mapped memory and FIMC driver update. The main
change from the previous version is a complete rewrite of the IOMMU
driver API. As suggested by Arnd Bergmann we decided to drop the custom
interface and use the kernel wide, common iommu API defined in
linux/include/iommu.h. This way the videobuf2 iommu allocator become
much more generic framework, because it is no longer tied to any
particular iommu implementation.

This patch series introduces new type of videbuf2 memory allocator -
vb2-dma-iommu. This allocator can be used on the platforms that support
linux/include/iommu.h style IOMMU driver. An IOMMU driver for Samsung
EXYNOS4 (called SYSMMU) platform is also included. The allocator and
IOMMU driver is then used by s5p-fimc driver. To make it possible some
additional changes are required. Mainly the platform support for s5p-fimc
for EXYNOS4 machines need to be defined. The proposed solution has been
tested on Universal C210 board (Samsung S5PC210/EXYNOS4 based).
This IOMMU allocator has no dependences on any external subsystems.

We also ported s5p-mfc and s5p-tv drivers to this allocator, they will
be posted in separate patch series. This will enable to get them working
on mainline Linux kernel for EXYNOS4 platform. Support for
S5PV210/S5PC110 platform still depends on CMA allocator that needs more
discussion on memory management mailing list and further development.
The patches with updated s5p-mfc and s5p-tv drivers will follow.

To get FIMC module working on EXYNOS4 platform on UniversalC210 board we
also added support for power domains and power gating.

This patch series contains a collection of patches for various platform
subsystems. Here is a detailed list:

[PATCH 1/7] ARM: EXYNOS4: power domains: fixes and code cleanup
- adds support for block gating in Samsung power domain driver and
  performs some cleanup

[PATCH 2/7] ARM: Samsung: update/rewrite Samsung SYSMMU (IOMMU) driver
- a complete rewrite of sysmmu driver for Samsung platform, now uses
  linux/include/iommu.h api

[PATCH 3/7] v4l: videobuf2: dma-sg: move some generic functions to memops
- a little cleanup and preparations for the dma-iommu allocator

[PATCH 4/7] v4l: videobuf2: add IOMMU based DMA memory allocator
- introduces new memory allocator for videobuf2 for drivers that support
  iommu dma memory mappings

[PATCH 5/7] v4l: s5p-fimc: add pm_runtime support
- adds support for pm_runtime in s5p-fimc driver

[PATCH 6/7] v4l: s5p-fimc: Add support for vb2-dma-iommu allocator
- adds support for the newly introduces videbuf2-s5p-iommu allocator
  on EXYNOS4 platform

[PATCH 7/7] ARM: EXYNOS4: enable FIMC on Universal_C210
- adds all required machine definitions to get FIMC modules working
  on Universal C210 boards


Changelog:

V2:
 - custom SYSMMU interface has been dropped in favour of linux/include/iommu.h
   and rewritten SYSMMU driver again
 - added support to SYSMMU for mapping pages larger than 4Kb
 - dropped ARM shared mode
 - videobuf2-s5p-iommu allocator has been renamed to videobuf2-dma-iommu,
   because it has no dependenco on any Samsung platform specific API,
   the allocator still uses only 4Kb pages, but this will be changed in the
   next version
 - dropped FIMC platform patch that have been merged mainline
 - rebased all patches onto Linux kernel v2.6.39-rc1

V1: http://www.spinics.net/lists/linux-media/msg29751.html

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center



Complete patch summary:

Andrzej Pietrasiewicz (3):
  ARM: Samsung: update/rewrite Samsung SYSMMU (IOMMU) driver
  v4l: videobuf2: dma-sg: move some generic functions to memops
  v4l: videobuf2: add IOMMU based DMA memory allocator

Marek Szyprowski (3):
  v4l: s5p-fimc: add pm_runtime support
  v4l: s5p-fimc: Add support for vb2-dma-iommu allocator
  ARM: EXYNOS4: enable FIMC on Universal_C210

Tomasz Stanislawski (1):
  ARM: EXYNOS4: power domains: fixes and code cleanup

 arch/arm/mach-exynos4/Kconfig   |6 +
 arch/arm/mach-exynos4/clock.c   |   68 +-
 arch/arm/mach-exynos4/dev-pd.c  |   93 ++-
 arch/arm/mach-exynos4/dev-sysmmu.c  |  615 -
 arch/arm/mach-exynos4/include/mach/irqs.h   |   35 +-
 arch/arm/mach-exynos4/include/mach/regs-clock.h |7 +
 arch/arm/mach-exynos4/include/mach/sysmmu.h |   46 -
 arch/arm/mach-exynos4/mach-universal_c210.c |   22 +
 arch/arm/plat-s5p/Kconfig   |   20 +-
 arch/arm/plat-s5p/include/plat/sysmmu.h |  165 ++--
 arch/arm/plat-s5p/sysmmu.c  | 1103 ---
 arch/arm/plat-samsung/include/plat/devs.h   |2 +-
 arch/arm/plat-samsung/include/plat/pd.h |1 +
 drivers/media/video/Kconfig |   11 +-
 drivers/media/video/Makefile|1 +
 drivers/media/video/s5p-fimc/fimc-capture.c |9 +-
 drivers/media/vide

Re: [PATCH 0/2] V4L: Extended crop/compose API

2011-04-05 Thread Laurent Pinchart
Hi Tomasz,

On Tuesday 29 March 2011 12:38:50 Tomasz Stanislawski wrote:
> Hans Verkuil wrote:
> > On Tuesday, March 29, 2011 11:22:17 Tomasz Stanislawski wrote:
> >> Hans Verkuil wrote:
> >>> On Monday, March 28, 2011 17:19:54 Tomasz Stanislawski wrote:
>  Hello everyone,
>  
>  This patch-set introduces new ioctls to V4L2 API. The new method for
>  configuration of cropping and composition is presented.
>  
>  There is some confusion in understanding of a cropping in current
>  version of V4L2. For CAPTURE devices cropping refers to choosing only a
>  part of input data stream and processing it and storing it in a memory
>  buffer. The buffer is fully filled by data. It is not possible to
>  choose only a part of a buffer for being updated by hardware.
>  
>  In case of OUTPUT devices, the whole content of a buffer is passed by
>  hardware to output display. Cropping means selecting only a part of an
>  output display/signal. It is not possible to choose only a part for a
>  memory buffer to be processed.
>  
>  The overmentioned flaws in cropping API were discussed in post:
>  http://article.gmane.org/gmane.linux.drivers.video-input-
infrastructure/28945
> 
>  A solution was proposed during brainstorming session in Warsaw.
> >> 
> >> Hello. Thank you for a quick comment.
> >> 
> >>> I don't have time right now to review this RFC in-depth, but one thing
> >>> that needs more attention is the relationship between these new ioctls
> >>> and CROPCAP.
> > 
> >>> And also how this relates to analog inputs (I don't think analog
> >>> outputs make any sense). And would a COMPOSECAP ioctl make sense?
> >> 
> >> Maybe two new ioctl COMPOSECAP and EXTCROPCAP should be added.
> >> For input CROPCAP maps to EXTCROPCAP, for output it maps to COMPOSECAP.
> >> The output EXTCROPCAP would return dimentions of a buffer.
> >> But in my opinion field v4l2_selection::bounds should be added to
> >> structure below. In such a case G_EXTCROP could be used to obtain
> >> cropping bounds.

Using flags to tell G_EXTCROP and G_COMPOSE whether we want to retrieve the 
bounds, default rectangle or current rectangle would be a sensible option in 
my opinion.

> > There is more in CROPCAP than just the bounds. I'd have to think about
> > this myself.

[snip]

>  3. Hints
>  
>  The v4l2_selection::flags field is used to give a driver a hint about
>  coordinate adjustments.  Below one can find the proposition of
>  adjustment flags. The syntax is V4L2_SEL_{name}_{LE/GE}, where {name}
>  refer to a
> > 
> > field in
> > 
>  struct v4l2_rect. The LE is abbreviation from "lesser or equal".  It
> > 
> > prevents
> > 
>  the driver form increasing a parameter. In similar fashion GE means
> > 
> > "greater or
> > 
>  equal" and it disallows decreasing. Combining LE and GE flags prevents
> > 
> > the
> > 
>  driver from any adjustments of parameters.  In such a manner, setting
> > 
> > flags
> > 
>  field to zero would give a driver a free hand in coordinate
>  adjustment.
>  
>  #define V4L2_SEL_WIDTH_GE0x0001
>  #define V4L2_SEL_WIDTH_LE0x0002
>  #define V4L2_SEL_HEIGHT_GE   0x0004
>  #define V4L2_SEL_HEIGHT_LE   0x0008
>  #define V4L2_SEL_LEFT_GE 0x0010
>  #define V4L2_SEL_LEFT_LE 0x0020
>  #define V4L2_SEL_TOP_GE  0x0040
>  #define V4L2_SEL_TOP_LE  0x0080
> >>> 
> >>> Wouldn't you also need similar flags for RIGHT and BOTTOM?
> >>> 
> >>> Regards,
> >>> 
> >>>   Hans
> >> 
> >> Proposed flags refer to fields in v4l2_rect structure. These are left,
> >> top, width and height. Fields bottom and right and not present in
> >> v4l2_rect structure.
> > 
> > But what if I want to keep the bottom right corner fixed, and allow other
> > parameters to change? I don't think you can do that with the current set
> > of flags.
> > 
> > Regards,
> > 
> > Hans
> 
> You are right. New flags should be added:
> 
> #define V4L2_SEL_RIGHT_GE 0x0100
> #define V4L2_SEL_RIGHT_LE 0x0200
> #define V4L2_SEL_BOTTOM_GE0x0400
> #define V4L2_SEL_BOTTOM_LE0x0800
> 
> They  would be used to inform a driver about adjusting a bottom-right
> corner. Right and bottom would be defined as:
> 
> right = v4l2_rect::left + v4l2_rect::width
> bottom = v4l2_rect::top + v4l2_rect::height

What if you want to allow the rectangle to be slightly enlarged or reduced, 
but want to keep it centered ? I feel like this would get out of hands quite 
fast. Hints are not a bad idea, but they will become very complex to implement 
in drivers, especially when the hardware forces all kind of weird constraints 
(and we all know that hardware designers get extremely creative in that area 
:-)).

>  #define V4L2_SEL_WIDTH_FIXED 0x0003
>  #define V4L2_SEL_HEIGHT_FIXED0x000c
>  #define V4L2_SEL_LEFT_FIXED  

Re: [RFC] V4L2 API for flash devices

2011-04-05 Thread Sakari Ailus
Laurent Pinchart wrote:
> Hi Sakari,

Hi,

> On Tuesday 05 April 2011 13:21:03 Sakari Ailus wrote:
>> Laurent Pinchart wrote:
>>> On Tuesday 05 April 2011 12:23:51 Sakari Ailus wrote:
 Sakari Ailus wrote:
> Laurent Pinchart wrote:
>> On Wednesday 30 March 2011 13:05:54 Sakari Ailus wrote:
>>> Laurent Pinchart wrote:
 On Monday 28 March 2011 14:55:40 Sakari Ailus wrote:

 [snip]

>   V4L2_CID_FLASH_STROBE_MODE (menu; LED)
>
> Use hardware or software strobe. If hardware strobe is selected,
> the flash controller is a slave in the system where the sensor
> produces the strobe signal to the flash.
>
> In this case the flash controller setup is limited to programming
> strobe timeout and power (LED flash) and the sensor controls the
> timing and length of the strobe.
>
> enum v4l2_flash_strobe_mode {
>
>   V4L2_FLASH_STROBE_MODE_SOFTWARE,
>   V4L2_FLASH_STROBE_MODE_EXT_STROBE,
>
> };

 [snip]

>   V4L2_CID_FLASH_LED_MODE (menu; LED)
>
> enum v4l2_flash_led_mode {
>
>   V4L2_FLASH_LED_MODE_FLASH = 1,
>   V4L2_FLASH_LED_MODE_TORCH,
>
> };

 Thinking about this some more, shouldn't we combine the two controls
 ? They are basically used to configure how the flash LED is
 controlled: manually (torch mode), automatically by the flash
 controller (software strobe mode) or automatically by an external
 component (external strobe mode).
>>>
>>> That's a good question.
>>>
>>> The adp1653 supports also additional control (not implemented in the
>>> driver, though) that affect hardware strobe length. Based on register
>>> setting, the led will be on after strobe either until the timeout
>>> expires, or until the strobe signal is high.
>>>
>>> Should this be also part of the same control, or a different one?
>>
>> That can be controlled by a duration control. If the duration is 0,
>> the flash is lit for the duration of the external strobe, otherwise
>> it's lit for the programmed duration.
>
> Sounds good to me.

 Thinking about this again; there won't be a separate duration control
>>>
>>> Why not ? I think we need two timeouts, a watchdog timeout to prevent
>>> flash fire or meltdown, and a normal timeout to lit the flash for a
>>> user-selected duration.
>>
>> Let's assume that an application wants to expose a frame using flash
>> with software strobe.
>>
>> 1. strobe flash
>> 2. qbuf
>> 3. streamon
>> 4. dqbuf
>> 5. streamoff
>> 6. ...
>>
>> How does an application know how long is the time between 1 -- 4? I'd
>> guess that in 6 the application would like to switch off the flash
>> instead of specifying a timeout for it.
> 
> That's a valid use case, and we need to support it. It requires a way to lit 
> the flash with no timeout other than the watchdog timeout, and a way to turn 
> it off.
> 
> However, I'm not sure we should rule out the usefulness of a duration-based 
> software flash strobe. Can't the two APIs coexist ? Or do you think a 
> duration-based API is useless ?

I don't want to rule it out, but I don't see use for it either for the
time being and thus don't see a reason to specify an API for it. The
adp1653 driver does not implement a software based shutdown timeout and
I'm not aware of others implementing it either. It was just an idea to
overcome the coarse hardware watchdog timeout.

If there is use for it then I think we could bring up the question
again: use V4L2_CID_FLASH_TIMEOUT or create V4L2_CID_FLASH_DURATION for it.

What do you think?

-- 
Sakari Ailus
sakari.ai...@maxwell.research.nokia.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 2/4 v2] V4L: add videobuf2 helper functions to support multi-size video-buffers

2011-04-05 Thread Guennadi Liakhovetski
On Tue, 5 Apr 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Friday 01 April 2011 16:06:42 Guennadi Liakhovetski wrote:
> > This patch extends the videobuf2 framework with new helper functions and
> > modifies existing ones to support multi-size video-buffers.
> 
> [snip]
> 
> > diff --git a/include/media/videobuf2-core.h
> > b/include/media/videobuf2-core.h index f87472a..88076fc 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h
> > @@ -177,6 +177,10 @@ struct vb2_buffer {
> >   * plane should be set in the sizes[] array and optional
> >   * per-plane allocator specific context in alloc_ctxs[]
> >   * array
> > + * @queue_add: like above, but called from VIDIOC_CREATE_BUFS, 
> > but if
> > + * there are already buffers on the queue, it won't replace
> > + * them, but add new ones, possibly with a different format
> > + * and plane sizes
> 
> I don't think drivers will need to perform different operations in 
> queue_setup 
> and queue_add. You could merge the two operations.

In principle yes. That's also what I ended up doing internally in the 
sh-mobile driver. Would have to change existing drivers then...

> >   * @wait_prepare:  release any locks taken while calling vb2 functions;
> >   * it is called before an ioctl needs to wait for a new
> >   * buffer to arrive; required to avoid a deadlock in
> > @@ -194,6 +198,8 @@ struct vb2_buffer {
> >   * each hardware operation in this callback;
> >   * if an error is returned, the buffer will not be queued
> >   * in driver; optional
> > + * @buf_submit:called primarily to invalidate buffer caches 
> > for faster
> > + * consequent queuing; optional
> 
> What's the difference between buf_submit and buf_prepare ?

My confusion;) Clarified this with Hans too already.

> >   * @buf_finish:called before every dequeue of the buffer back 
> > to
> >   * userspace; drivers may perform any operations required
> >   * before userspace accesses the buffer; optional
> 
> -- 
> Regards,
> 
> Laurent Pinchart

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


Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Laurent Pinchart
On Tuesday 05 April 2011 14:52:20 Guennadi Liakhovetski wrote:
> On Tue, 5 Apr 2011, Hans Verkuil wrote:
> > On Tuesday, April 05, 2011 14:21:03 Laurent Pinchart wrote:
> > > On Friday 01 April 2011 10:13:02 Guennadi Liakhovetski wrote:
> [snip]
> 
> > > >  /*
> > > >  
> > > >   * I O C T L   C O D E S   F O R   V I D E O   D E V I C E S
> > > >   *
> > > > 
> > > > @@ -1937,6 +1957,10 @@ struct v4l2_dbg_chip_ident {
> > > > 
> > > >  #defineVIDIOC_SUBSCRIBE_EVENT   _IOW('V', 90, struct
> > > > 
> > > > v4l2_event_subscription) #defineVIDIOC_UNSUBSCRIBE_EVENT 
> > > > _IOW('V',
> > > > 91, struct v4l2_event_subscription)
> > > > 
> > > > +#define VIDIOC_CREATE_BUFS _IOWR('V', 92, struct
> > > > v4l2_create_buffers) +#define VIDIOC_DESTROY_BUFS   _IOWR('V', 93,
> > > > struct v4l2_buffer_span) +#define VIDIOC_SUBMIT_BUF  _IOW('V', 94,
> > > > int)
> > > > +
> > > 
> > > In case we later need to pass other information (such as flags) to
> > > VIDIOC_SUBMIT_BUF, you should use a structure instead of an int.
> > 
> > I would just pass struct v4l2_buffer to this ioctl, just like QBUF/DQBUF
> > do.
> 
> Why??? You do not need all that extra information. Well, we could, of
> course, make a struct with reserved fields... but it seems nice to me to
> have the clarity here - this ioctl() _only_ gives buffer ownership to the
> kernel. No more configuration...

Right now you're correct. In the future we might need extra fields, so we need 
a structure with reserved fields.

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Laurent Pinchart
Hi Guennadi,

On Tuesday 05 April 2011 14:39:19 Guennadi Liakhovetski wrote:
> On Tue, 5 Apr 2011, Laurent Pinchart wrote:
> > On Friday 01 April 2011 10:13:02 Guennadi Liakhovetski wrote:
> > > A possibility to preallocate and initialise buffers of different sizes
> > > in V4L2 is required for an efficient implementation of asnapshot mode.
> > > This patch adds three new ioctl()s: VIDIOC_CREATE_BUFS,
> > > VIDIOC_DESTROY_BUFS, and VIDIOC_SUBMIT_BUF and defines respective data
> > > structures.

[snip]

> > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > > index aa6c393..b6ef46e 100644
> > > --- a/include/linux/videodev2.h
> > > +++ b/include/linux/videodev2.h
> > > @@ -1847,6 +1847,26 @@ struct v4l2_dbg_chip_ident {

[snip]

> > > +/* struct v4l2_createbuffers::flags */
> > > +#define V4L2_BUFFER_FLAG_NO_CACHE_INVALIDATE (1 << 0)
> > 
> > Shouldn't cache management be handled at submit/qbuf time instead of
> > being a buffer property ?
> 
> hmm, I'd prefer fixing it at create. Or do you want to be able to create
> buffers and then submit / queue them with different flags?...

That's the idea, yes. I'm not sure yet how useful that would be though.

[snip]

> > > +
> > > 
> > >  /*
> > >  
> > >   *   I O C T L   C O D E S   F O R   V I D E O   D E V I C E S
> > >   *
> > > 
> > > @@ -1937,6 +1957,10 @@ struct v4l2_dbg_chip_ident {
> > > 
> > >  #define  VIDIOC_SUBSCRIBE_EVENT   _IOW('V', 90, struct
> > > 
> > > v4l2_event_subscription) #define  VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91,
> > > struct v4l2_event_subscription)
> > > 
> > > +#define VIDIOC_CREATE_BUFS   _IOWR('V', 92, struct 
> > > v4l2_create_buffers)
> > > +#define VIDIOC_DESTROY_BUFS  _IOWR('V', 93, struct v4l2_buffer_span)
> > 
> > Just throwing an idea in here, what about using the same structure for
> > both ioctls ? Or even a single ioctl for both create and destroy, like
> > we do with REQBUFS ?
> 
> Personally, tbh, I don't like either of them. The first one seems an
> overkill - you don't need all those fields for destroy. The second one is
> a particular case of the first one, plus it adds confusion by re-using the
> ioctl:-) Where with REQBUFS we could just set count = 0 to say - release
> all buffers, with this one we need index and count, so, we'd need one more
> flag to distinguish between create / destroy...

OK, idea dismissed :-)

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Laurent Pinchart
On Tuesday 05 April 2011 14:40:16 Hans Verkuil wrote:
> On Tuesday, April 05, 2011 14:02:17 Laurent Pinchart wrote:
> > On Monday 04 April 2011 10:06:47 Hans Verkuil wrote:
> > > > On Mon, 4 Apr 2011, Hans Verkuil wrote:
> > > >> On Friday, April 01, 2011 10:13:02 Guennadi Liakhovetski wrote:
> > [snip]
> > 
> > > BTW, REQBUFS and CREATE/DESTROY_BUFS should definitely co-exist.
> > > REQBUFS is compulsory, while CREATE/DESTROY are optional.
> > 
> > Drivers must support REQBUFS and should support CREATE/DESTROY, but I
> > think applications should not be allowed to mix calls.
> 
> Why not? The videobuf2-core.c implementation shouldn't care about that, so
> I don't see why userspace should care.

Because it makes the API semantics much more complex to define. We would have 
to precisely define interactions between REQBUFS and CREATE/DESTROY. That will 
be error-prone, for very little benefits (if any at all). If an application is 
aware of the CREATE/DESTROY ioctls and wants to use them, why would it need to 
use REQBUFS ?

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Guennadi Liakhovetski
On Tue, 5 Apr 2011, Hans Verkuil wrote:

> On Tuesday, April 05, 2011 14:21:03 Laurent Pinchart wrote:
> > On Friday 01 April 2011 10:13:02 Guennadi Liakhovetski wrote:

[snip]

> > >  /*
> > >   *   I O C T L   C O D E S   F O R   V I D E O   D E V I C E S
> > >   *
> > > @@ -1937,6 +1957,10 @@ struct v4l2_dbg_chip_ident {
> > >  #define  VIDIOC_SUBSCRIBE_EVENT   _IOW('V', 90, struct
> > > v4l2_event_subscription) #define  VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91,
> > > struct v4l2_event_subscription)
> > > 
> > > +#define VIDIOC_CREATE_BUFS   _IOWR('V', 92, struct 
> > > v4l2_create_buffers)
> > > +#define VIDIOC_DESTROY_BUFS  _IOWR('V', 93, struct v4l2_buffer_span)
> > > +#define VIDIOC_SUBMIT_BUF _IOW('V', 94, int)
> > > +
> > 
> > In case we later need to pass other information (such as flags) to 
> > VIDIOC_SUBMIT_BUF, you should use a structure instead of an int.
> 
> I would just pass struct v4l2_buffer to this ioctl, just like QBUF/DQBUF do.

Why??? You do not need all that extra information. Well, we could, of 
course, make a struct with reserved fields... but it seems nice to me to 
have the clarity here - this ioctl() _only_ gives buffer ownership to the 
kernel. No more configuration...

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


Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Laurent Pinchart
On Tuesday 05 April 2011 14:34:57 Hans Verkuil wrote:
> On Tuesday, April 05, 2011 14:21:03 Laurent Pinchart wrote:
> > On Friday 01 April 2011 10:13:02 Guennadi Liakhovetski wrote:

[snip]

> > > @@ -1937,6 +1957,10 @@ struct v4l2_dbg_chip_ident {
> > > 
> > >  #define  VIDIOC_SUBSCRIBE_EVENT   _IOW('V', 90, struct
> > > 
> > > v4l2_event_subscription) #define  VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91,
> > > struct v4l2_event_subscription)
> > > 
> > > +#define VIDIOC_CREATE_BUFS   _IOWR('V', 92, struct 
> > > v4l2_create_buffers)
> > > +#define VIDIOC_DESTROY_BUFS  _IOWR('V', 93, struct v4l2_buffer_span)
> > > +#define VIDIOC_SUBMIT_BUF _IOW('V', 94, int)
> > > +
> > 
> > In case we later need to pass other information (such as flags) to
> > VIDIOC_SUBMIT_BUF, you should use a structure instead of an int.
> 
> I would just pass struct v4l2_buffer to this ioctl, just like QBUF/DQBUF
> do.

v4l2_buffer has a single reserved field (we could reclaim the input field, 
it's not used by any in-tree driver). Shouldn't we a bit more future-proof ?

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 0/4] V4L: new ioctl()s to support multi-sized video-buffers

2011-04-05 Thread Guennadi Liakhovetski
On Tue, 5 Apr 2011, Laurent Pinchart wrote:

[snip]

> > > - Are "holes" in buffer indexes allowed? I don't like the ability to
> > > free an arbitrary span of buffers in the queue, it complicates checks
> > > in many places and I don't think is worth it...
> > 
> > That's how this ioctl() has been proposed at the Warsaw meeting.
> 
> If my memory is correct, we agreed that buffers created with a single CREATE 
> call had to be freed all at once by DESTROY. This won't prevent holes though, 
> as applications could call CREATE three times and then free buffers allocated 
> by the second call.

Yes, I think, you're right. Currently I don't track those creation sets... 
Do we really want that? What does it give us?

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


Re: [PATCH/RFC 2/4 v2] V4L: add videobuf2 helper functions to support multi-size video-buffers

2011-04-05 Thread Laurent Pinchart
On Monday 04 April 2011 09:55:17 Guennadi Liakhovetski wrote:
> On Sun, 3 Apr 2011, Pawel Osciak wrote:
> > On Fri, Apr 1, 2011 at 07:06, Guennadi Liakhovetski wrote:
> > > This patch extends the videobuf2 framework with new helper functions
> > > and modifies existing ones to support multi-size video-buffers.

[snip]

> > > diff --git a/drivers/media/video/videobuf2-core.c
> > > b/drivers/media/video/videobuf2-core.c index 71734a4..20e1572 100644
> > > --- a/drivers/media/video/videobuf2-core.c
> > > +++ b/drivers/media/video/videobuf2-core.c

[snip]

> > > @@ -241,16 +250,36 @@ static void __vb2_queue_free(struct vb2_queue *q)
> > >}
> > > 
> > >/* Release video buffer memory */
> > > -   __vb2_free_mem(q);
> > > +   __vb2_free_mem(q, span);
> > > 
> > >/* Free videobuf buffers */
> > > -   for (buffer = 0; buffer < q->num_buffers; ++buffer) {
> > > +   for (buffer = span->index;
> > > +buffer < span->index + span->count; ++buffer) {
> > >kfree(q->bufs[buffer]);
> > >q->bufs[buffer] = NULL;
> > >}
> > > 
> > > -   q->num_buffers = 0;
> > > -   q->memory = 0;
> > > +   q->num_buffers -= span->count;
> > > +   if (!q->num_buffers)
> > > +   q->memory = 0;
> > > +
> > > +   return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(vb2_destroy_bufs);
> > > +
> > 
> > This messes up q->num_buffers, which is used in other places as the
> > upper bound for indexes and assumes <0; q->num_buffers> is contiguous.
> > Examples include querybufs, buffers_in_use. In general, I find it
> > unnecessary complicated and dangerous to allow introducing "holes" in
> > buffer indexes.
> 
> Yes, there are issues... I think, we can just return -EBUSY on such
> fragmenting DESTROY calls. Anything left over will be freed on last
> close() anyway. Let's decide how we want to handle these and I'll prepare
> a v2, probably with documentation this time:-)

I also feel a bit uneasy about introducing holes in buffer indexes. Do we have 
use cases for that right now ?

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Hans Verkuil
On Tuesday, April 05, 2011 14:02:17 Laurent Pinchart wrote:
> On Monday 04 April 2011 10:06:47 Hans Verkuil wrote:
> > > On Mon, 4 Apr 2011, Hans Verkuil wrote:
> > >> On Friday, April 01, 2011 10:13:02 Guennadi Liakhovetski wrote:
> 
> [snip]
> 
> > BTW, REQBUFS and CREATE/DESTROY_BUFS should definitely co-exist. REQBUFS
> > is compulsory, while CREATE/DESTROY are optional.
> 
> Drivers must support REQBUFS and should support CREATE/DESTROY, but I think 
> applications should not be allowed to mix calls.

Why not? The videobuf2-core.c implementation shouldn't care about that, so
I don't see why userspace should care.

Regards,

Hans

> 
> -- 
> 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
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Guennadi Liakhovetski
On Tue, 5 Apr 2011, Laurent Pinchart wrote:

> On Monday 04 April 2011 10:06:47 Hans Verkuil wrote:
> > > On Mon, 4 Apr 2011, Hans Verkuil wrote:
> > >> On Friday, April 01, 2011 10:13:02 Guennadi Liakhovetski wrote:
> 
> [snip]
> 
> > BTW, REQBUFS and CREATE/DESTROY_BUFS should definitely co-exist. REQBUFS
> > is compulsory, while CREATE/DESTROY are optional.
> 
> Drivers must support REQBUFS and should support CREATE/DESTROY, but I think 
> applications should not be allowed to mix calls.

So, you want to track it down in the kernel which mode is being used?... 
Hans?

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


Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Guennadi Liakhovetski
Hi Laurent

On Tue, 5 Apr 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Friday 01 April 2011 10:13:02 Guennadi Liakhovetski wrote:
> > A possibility to preallocate and initialise buffers of different sizes
> > in V4L2 is required for an efficient implementation of asnapshot mode.
> > This patch adds three new ioctl()s: VIDIOC_CREATE_BUFS,
> > VIDIOC_DESTROY_BUFS, and VIDIOC_SUBMIT_BUF and defines respective data
> > structures.
> 
> [snip]
> 
> > diff --git a/drivers/media/video/v4l2-ioctl.c
> > b/drivers/media/video/v4l2-ioctl.c index a01ed39..b80a211 100644
> > --- a/drivers/media/video/v4l2-ioctl.c
> > +++ b/drivers/media/video/v4l2-ioctl.c
> 
> [snip]
> 
> > @@ -2184,6 +2187,46 @@ static long __video_do_ioctl(struct file *file,
> > dbgarg(cmd, "type=0x%8.8x", sub->type);
> > break;
> > }
> > +   case VIDIOC_CREATE_BUFS:
> > +   {
> > +   struct v4l2_create_buffers *create = arg;
> > +
> > +   if (!ops->vidioc_create_bufs)
> > +   break;
> > +   ret = check_fmt(ops, create->format.type);
> > +   if (ret)
> > +   break;
> > +
> > +   if (create->size)
> > +   CLEAR_AFTER_FIELD(create, count);
> 
> Why only when create->size is > 0 ?

Because otherwise create->format contains the frame format, that will be 
used for plane-size calculations.

> > +   ret = ops->vidioc_create_bufs(file, fh, create);
> > +
> > +   dbgarg(cmd, "count=%d\n", create->count);
> > +   break;
> > +   }
> > +   case VIDIOC_DESTROY_BUFS:
> > +   {
> > +   struct v4l2_buffer_span *span = arg;
> > +
> > +   if (!ops->vidioc_destroy_bufs)
> > +   break;
> > +
> > +   ret = ops->vidioc_destroy_bufs(file, fh, span);
> > +
> > +   dbgarg(cmd, "count=%d", span->count);
> > +   break;
> > +   }
> > +   case VIDIOC_SUBMIT_BUF:
> > +   {
> > +   unsigned int *i = arg;
> > +
> > +   if (!ops->vidioc_submit_buf)
> > +   break;
> > +   ret = ops->vidioc_submit_buf(file, fh, *i);
> > +   dbgarg(cmd, "index=%d", *i);
> > +   break;
> > +   }
> > default:
> > {
> > bool valid_prio = true;
> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > index aa6c393..b6ef46e 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -1847,6 +1847,26 @@ struct v4l2_dbg_chip_ident {
> > __u32 revision;/* chip revision, chip specific */
> >  } __attribute__ ((packed));
> > 
> > +/* VIDIOC_DESTROY_BUFS */
> > +struct v4l2_buffer_span {
> > +   __u32   index;  /* output: buffers index...index + 
> > count - 1 have been
> > created */ +__u32   count;
> > +   __u32   reserved[2];
> > +};
> > +
> > +/* struct v4l2_createbuffers::flags */
> > +#define V4L2_BUFFER_FLAG_NO_CACHE_INVALIDATE   (1 << 0)
> 
> Shouldn't cache management be handled at submit/qbuf time instead of being a 
> buffer property ?

hmm, I'd prefer fixing it at create. Or do you want to be able to create 
buffers and then submit / queue them with different flags?...

> > +/* VIDIOC_CREATE_BUFS */
> > +struct v4l2_create_buffers {
> > +   __u32   index;  /* output: buffers 
> > index...index + count - 1 have been
> > created */ +__u32   count;
> > +   __u32   flags;  /* V4L2_BUFFER_FLAG_* */
> > +   enum v4l2_memorymemory;
> > +   __u32   size;   /* Explicit size, e.g., for 
> > compressed streams */
> > +   struct v4l2_format  format; /* "type" is used always, the 
> > rest if size 
> ==
> > 0 */ +};
> 
> You need reserved fields here.

Yes, already discussed with Hans, will add.

> 
> > +
> >  /*
> >   * I O C T L   C O D E S   F O R   V I D E O   D E V I C E S
> >   *
> > @@ -1937,6 +1957,10 @@ struct v4l2_dbg_chip_ident {
> >  #defineVIDIOC_SUBSCRIBE_EVENT   _IOW('V', 90, struct
> > v4l2_event_subscription) #defineVIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91,
> > struct v4l2_event_subscription)
> > 
> > +#define VIDIOC_CREATE_BUFS _IOWR('V', 92, struct v4l2_create_buffers)
> > +#define VIDIOC_DESTROY_BUFS_IOWR('V', 93, struct v4l2_buffer_span)
> 
> Just throwing an idea in here, what about using the same structure for both 
> ioctls ? Or even a single ioctl for both create and destroy, like we do with 
> REQBUFS ?

Personally, tbh, I don't like either of them. The first one seems an 
overkill - you don't need all those fields for destroy. The second one is 
a particular case of the first one, plus it adds confusion by re-using the 
ioctl:-) Where with REQBUFS we could just set count = 0 to say - release 
all buffers, with this one we need index and count, so, we'd need one more 
flag to distinguish between create / destroy...

> > +#define VIDIOC_SUBMIT_BUF   _IOW('V', 94, int)
> > +
> 

Re: [PATCH/RFC 2/4 v2] V4L: add videobuf2 helper functions to support multi-size video-buffers

2011-04-05 Thread Laurent Pinchart
Hi Guennadi,

On Friday 01 April 2011 16:06:42 Guennadi Liakhovetski wrote:
> This patch extends the videobuf2 framework with new helper functions and
> modifies existing ones to support multi-size video-buffers.

[snip]

> diff --git a/include/media/videobuf2-core.h
> b/include/media/videobuf2-core.h index f87472a..88076fc 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -177,6 +177,10 @@ struct vb2_buffer {
>   *   plane should be set in the sizes[] array and optional
>   *   per-plane allocator specific context in alloc_ctxs[]
>   *   array
> + * @queue_add:   like above, but called from VIDIOC_CREATE_BUFS, 
> but if
> + *   there are already buffers on the queue, it won't replace
> + *   them, but add new ones, possibly with a different format
> + *   and plane sizes

I don't think drivers will need to perform different operations in queue_setup 
and queue_add. You could merge the two operations.

>   * @wait_prepare:release any locks taken while calling vb2 functions;
>   *   it is called before an ioctl needs to wait for a new
>   *   buffer to arrive; required to avoid a deadlock in
> @@ -194,6 +198,8 @@ struct vb2_buffer {
>   *   each hardware operation in this callback;
>   *   if an error is returned, the buffer will not be queued
>   *   in driver; optional
> + * @buf_submit:  called primarily to invalidate buffer caches 
> for faster
> + *   consequent queuing; optional

What's the difference between buf_submit and buf_prepare ?

>   * @buf_finish:  called before every dequeue of the buffer back 
> to
>   *   userspace; drivers may perform any operations required
>   *   before userspace accesses the buffer; optional

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Hans Verkuil
On Tuesday, April 05, 2011 14:21:03 Laurent Pinchart wrote:
> On Friday 01 April 2011 10:13:02 Guennadi Liakhovetski wrote:
> > A possibility to preallocate and initialise buffers of different sizes
> > in V4L2 is required for an efficient implementation of asnapshot mode.
> > This patch adds three new ioctl()s: VIDIOC_CREATE_BUFS,
> > VIDIOC_DESTROY_BUFS, and VIDIOC_SUBMIT_BUF and defines respective data
> > structures.
> > 
> > Signed-off-by: Guennadi Liakhovetski 
> > ---
> >  drivers/media/video/v4l2-compat-ioctl32.c |3 ++
> >  drivers/media/video/v4l2-ioctl.c  |   43
> > + include/linux/videodev2.h | 
> >  24  include/media/v4l2-ioctl.h|3 ++
> >  4 files changed, 73 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
> > b/drivers/media/video/v4l2-compat-ioctl32.c index 7c26947..d71b289 100644
> > --- a/drivers/media/video/v4l2-compat-ioctl32.c
> > +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> > @@ -922,6 +922,9 @@ long v4l2_compat_ioctl32(struct file *file, unsigned
> > int cmd, unsigned long arg) case VIDIOC_DQEVENT:
> > case VIDIOC_SUBSCRIBE_EVENT:
> > case VIDIOC_UNSUBSCRIBE_EVENT:
> > +   case VIDIOC_CREATE_BUFS:
> > +   case VIDIOC_DESTROY_BUFS:
> > +   case VIDIOC_SUBMIT_BUF:
> > ret = do_video_ioctl(file, cmd, arg);
> > break;
> > 
> > diff --git a/drivers/media/video/v4l2-ioctl.c
> > b/drivers/media/video/v4l2-ioctl.c index a01ed39..b80a211 100644
> > --- a/drivers/media/video/v4l2-ioctl.c
> > +++ b/drivers/media/video/v4l2-ioctl.c
> > @@ -259,6 +259,9 @@ static const char *v4l2_ioctls[] = {
> > [_IOC_NR(VIDIOC_DQEVENT)]  = "VIDIOC_DQEVENT",
> > [_IOC_NR(VIDIOC_SUBSCRIBE_EVENT)]  = "VIDIOC_SUBSCRIBE_EVENT",
> > [_IOC_NR(VIDIOC_UNSUBSCRIBE_EVENT)] = "VIDIOC_UNSUBSCRIBE_EVENT",
> > +   [_IOC_NR(VIDIOC_CREATE_BUFS)]  = "VIDIOC_CREATE_BUFS",
> > +   [_IOC_NR(VIDIOC_DESTROY_BUFS)] = "VIDIOC_DESTROY_BUFS",
> > +   [_IOC_NR(VIDIOC_SUBMIT_BUF)]   = "VIDIOC_SUBMIT_BUF",
> >  };
> >  #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)
> > 
> > @@ -2184,6 +2187,46 @@ static long __video_do_ioctl(struct file *file,
> > dbgarg(cmd, "type=0x%8.8x", sub->type);
> > break;
> > }
> > +   case VIDIOC_CREATE_BUFS:
> > +   {
> > +   struct v4l2_create_buffers *create = arg;
> > +
> > +   if (!ops->vidioc_create_bufs)
> > +   break;
> > +   ret = check_fmt(ops, create->format.type);
> > +   if (ret)
> > +   break;
> > +
> > +   if (create->size)
> > +   CLEAR_AFTER_FIELD(create, count);
> > +
> > +   ret = ops->vidioc_create_bufs(file, fh, create);
> > +
> > +   dbgarg(cmd, "count=%d\n", create->count);
> > +   break;
> > +   }
> > +   case VIDIOC_DESTROY_BUFS:
> > +   {
> > +   struct v4l2_buffer_span *span = arg;
> > +
> > +   if (!ops->vidioc_destroy_bufs)
> > +   break;
> > +
> > +   ret = ops->vidioc_destroy_bufs(file, fh, span);
> > +
> > +   dbgarg(cmd, "count=%d", span->count);
> > +   break;
> > +   }
> > +   case VIDIOC_SUBMIT_BUF:
> > +   {
> > +   unsigned int *i = arg;
> > +
> > +   if (!ops->vidioc_submit_buf)
> > +   break;
> > +   ret = ops->vidioc_submit_buf(file, fh, *i);
> > +   dbgarg(cmd, "index=%d", *i);
> > +   break;
> > +   }
> > default:
> > {
> > bool valid_prio = true;
> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > index aa6c393..b6ef46e 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -1847,6 +1847,26 @@ struct v4l2_dbg_chip_ident {
> > __u32 revision;/* chip revision, chip specific */
> >  } __attribute__ ((packed));
> > 
> > +/* VIDIOC_DESTROY_BUFS */
> > +struct v4l2_buffer_span {
> > +   __u32   index;  /* output: buffers index...index + 
> > count 
- 1 have been
> > created */ +__u32   count;
> > +   __u32   reserved[2];
> > +};
> > +
> > +/* struct v4l2_createbuffers::flags */
> > +#define V4L2_BUFFER_FLAG_NO_CACHE_INVALIDATE   (1 << 0)
> > +
> > +/* VIDIOC_CREATE_BUFS */
> > +struct v4l2_create_buffers {
> > +   __u32   index;  /* output: buffers 
> > index...index + 
count - 1 have been
> > created */ +__u32   count;
> > +   __u32   flags;  /* V4L2_BUFFER_FLAG_* */
> > +   enum v4l2_memorymemory;
> > +   __u32   size;   /* Explicit size, e.g., for 
compressed streams */
> > +   struct v4l2_format  format; /* "type" is used always, the 
> > rest if 
size 
> ==
> > 0 */ +};
> > +
> >  /*
> >   * I O C T L   C O D E S   F O R   V I D E O   D E V I C E S
> >   *
> > @@ -1937,6 +1

Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Laurent Pinchart
On Friday 01 April 2011 10:13:02 Guennadi Liakhovetski wrote:
> A possibility to preallocate and initialise buffers of different sizes
> in V4L2 is required for an efficient implementation of asnapshot mode.
> This patch adds three new ioctl()s: VIDIOC_CREATE_BUFS,
> VIDIOC_DESTROY_BUFS, and VIDIOC_SUBMIT_BUF and defines respective data
> structures.
> 
> Signed-off-by: Guennadi Liakhovetski 
> ---
>  drivers/media/video/v4l2-compat-ioctl32.c |3 ++
>  drivers/media/video/v4l2-ioctl.c  |   43
> + include/linux/videodev2.h | 
>  24  include/media/v4l2-ioctl.h|3 ++
>  4 files changed, 73 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
> b/drivers/media/video/v4l2-compat-ioctl32.c index 7c26947..d71b289 100644
> --- a/drivers/media/video/v4l2-compat-ioctl32.c
> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> @@ -922,6 +922,9 @@ long v4l2_compat_ioctl32(struct file *file, unsigned
> int cmd, unsigned long arg) case VIDIOC_DQEVENT:
>   case VIDIOC_SUBSCRIBE_EVENT:
>   case VIDIOC_UNSUBSCRIBE_EVENT:
> + case VIDIOC_CREATE_BUFS:
> + case VIDIOC_DESTROY_BUFS:
> + case VIDIOC_SUBMIT_BUF:
>   ret = do_video_ioctl(file, cmd, arg);
>   break;
> 
> diff --git a/drivers/media/video/v4l2-ioctl.c
> b/drivers/media/video/v4l2-ioctl.c index a01ed39..b80a211 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -259,6 +259,9 @@ static const char *v4l2_ioctls[] = {
>   [_IOC_NR(VIDIOC_DQEVENT)]  = "VIDIOC_DQEVENT",
>   [_IOC_NR(VIDIOC_SUBSCRIBE_EVENT)]  = "VIDIOC_SUBSCRIBE_EVENT",
>   [_IOC_NR(VIDIOC_UNSUBSCRIBE_EVENT)] = "VIDIOC_UNSUBSCRIBE_EVENT",
> + [_IOC_NR(VIDIOC_CREATE_BUFS)]  = "VIDIOC_CREATE_BUFS",
> + [_IOC_NR(VIDIOC_DESTROY_BUFS)] = "VIDIOC_DESTROY_BUFS",
> + [_IOC_NR(VIDIOC_SUBMIT_BUF)]   = "VIDIOC_SUBMIT_BUF",
>  };
>  #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)
> 
> @@ -2184,6 +2187,46 @@ static long __video_do_ioctl(struct file *file,
>   dbgarg(cmd, "type=0x%8.8x", sub->type);
>   break;
>   }
> + case VIDIOC_CREATE_BUFS:
> + {
> + struct v4l2_create_buffers *create = arg;
> +
> + if (!ops->vidioc_create_bufs)
> + break;
> + ret = check_fmt(ops, create->format.type);
> + if (ret)
> + break;
> +
> + if (create->size)
> + CLEAR_AFTER_FIELD(create, count);
> +
> + ret = ops->vidioc_create_bufs(file, fh, create);
> +
> + dbgarg(cmd, "count=%d\n", create->count);
> + break;
> + }
> + case VIDIOC_DESTROY_BUFS:
> + {
> + struct v4l2_buffer_span *span = arg;
> +
> + if (!ops->vidioc_destroy_bufs)
> + break;
> +
> + ret = ops->vidioc_destroy_bufs(file, fh, span);
> +
> + dbgarg(cmd, "count=%d", span->count);
> + break;
> + }
> + case VIDIOC_SUBMIT_BUF:
> + {
> + unsigned int *i = arg;
> +
> + if (!ops->vidioc_submit_buf)
> + break;
> + ret = ops->vidioc_submit_buf(file, fh, *i);
> + dbgarg(cmd, "index=%d", *i);
> + break;
> + }
>   default:
>   {
>   bool valid_prio = true;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index aa6c393..b6ef46e 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1847,6 +1847,26 @@ struct v4l2_dbg_chip_ident {
>   __u32 revision;/* chip revision, chip specific */
>  } __attribute__ ((packed));
> 
> +/* VIDIOC_DESTROY_BUFS */
> +struct v4l2_buffer_span {
> + __u32   index;  /* output: buffers index...index + 
> count - 1 have been
> created */ +  __u32   count;
> + __u32   reserved[2];
> +};
> +
> +/* struct v4l2_createbuffers::flags */
> +#define V4L2_BUFFER_FLAG_NO_CACHE_INVALIDATE (1 << 0)
> +
> +/* VIDIOC_CREATE_BUFS */
> +struct v4l2_create_buffers {
> + __u32   index;  /* output: buffers 
> index...index + count - 1 have been
> created */ +  __u32   count;
> + __u32   flags;  /* V4L2_BUFFER_FLAG_* */
> + enum v4l2_memorymemory;
> + __u32   size;   /* Explicit size, e.g., for 
> compressed streams */
> + struct v4l2_format  format; /* "type" is used always, the 
> rest if size 
==
> 0 */ +};
> +
>  /*
>   *   I O C T L   C O D E S   F O R   V I D E O   D E V I C E S
>   *
> @@ -1937,6 +1957,10 @@ struct v4l2_dbg_chip_ident {
>  #define  VIDIOC_SUBSCRIBE_EVENT   _IOW('V', 90, struct
> v4l2_event_subscription) #define  VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91,
> struct v4l2_event_subscriptio

Re: [PATCH/RFC 0/4] V4L: new ioctl()s to support multi-sized video-buffers

2011-04-05 Thread Laurent Pinchart
Hi Guennadi,

On Monday 04 April 2011 09:15:55 Guennadi Liakhovetski wrote:
> On Sun, 3 Apr 2011, Pawel Osciak wrote:
> > On Fri, Apr 1, 2011 at 01:12, Guennadi Liakhovetski wrote:
> > > Hi all
> > > 
> > > As discussed at the last V4L2 meeting in Warsaw, one of the
> > > prerequisites to support fast switching between different image
> > > formats is an ability to preallocate buffers of different sizes and
> > > handle them over to the driver in advance. This avoids the need to
> > > allocate buffers at the time of switching. This patch series is a
> > > first implementation of these ioctl()s, implemented for the
> > > sh_mobile_ceu_camera soc-camera host driver. Tested on an sh7722 migor
> > > SuperH platform. Yes, I know, documentation is missing yet;-)
> > 
> > I will have to wait for documentation before doing a full review, it's
> > hard to comment without it. Also, please mention how the new ioctls
> > influence the state machine.
> 
> Ok, I wanted to wait with the documentation until we have the API settled,
> because modifying the code is easier, than modifying the documentation:-)
> But right, I'll try to put something together.
> 
> > Some questions and doubts I'm having:
> > - Can you call CREATE more than once, before/after REQBUFS, for all
> > streaming states? What about reading/writing?
> 
> The idea was to use CREATE/DESTROY _instead_ of REQBUFS. And yes, one of
> the purposes of CREATE is to be able to call it multiple times with
> different parameters. The new API should provide at least all the
> functionality, that REQBUFS provides, i.e., you should be able to use it
> with MMAP and USERPTR memory.
> 
> > - Can driver decline CREATE if it is not supported? What if the format
> > is not supported?
> 
> Sure, if .vidioc_create_bufs() is not implemented by the driver, the
> ioctl() will just error out. Of course you're allowed to do any checks you
> see fit in your driver, like unsupported formats and return an error in
> case of a problem.
> 
> > - If we fail allocating in CREATE, should the whole queue be freed (as
> > it is done in your patch I believe)?
> 
> No, that's a bug, thanks for spotting!
> 
> > - I'm assuming REQBUFS(0) is to free buffers allocated with CREATE too?
> 
> Currently it is possible to mix CREATE/DESTROY and REQBUFS. Not sure if
> this is good, maybe we have to allow the use of only one API. I'd probably
> prefer the latter, but I'm open for suggestions here.
> 
> > - Are we allowing DESTROY to free arbitrary span of buffers (i.e.
> > those created with REQBUFS as well)?
> 
> Again, we can decide, whether we want to support mixing of these APIs or
> not.
> 
> > - Are "holes" in buffer indexes allowed? I don't like the ability to
> > free an arbitrary span of buffers in the queue, it complicates checks
> > in many places and I don't think is worth it...
> 
> That's how this ioctl() has been proposed at the Warsaw meeting.

If my memory is correct, we agreed that buffers created with a single CREATE 
call had to be freed all at once by DESTROY. This won't prevent holes though, 
as applications could call CREATE three times and then free buffers allocated 
by the second call.

[snip]

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] V4L2 API for flash devices

2011-04-05 Thread David Cohen
On Mon, Mar 28, 2011 at 3:55 PM, Sakari Ailus
 wrote:
> Hi,

Hi Sakari,

[snip]

> This is a bitmask containing the fault information for the flash. This
> assumes the proposed V4L2 bit mask controls [5]; otherwise this would
> likely need to be a set of controls.
>
> #define V4L2_FLASH_FAULT_OVER_VOLTAGE           0x0001
> #define V4L2_FLASH_FAULT_TIMEOUT                0x0002
> #define V4L2_FLASH_FAULT_OVER_TEMPERATURE       0x0004
> #define V4L2_FLASH_FAULT_SHORT_CIRCUIT          0x0008

Sorry for bringing this a bit late.  As we already talked directly,
IMO (1 << 0), (1 << 1), ... could have a better readability to expose
how you want to define an expand these macros.

Br,

David Cohen
--
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] v4l: Add mt9v032 sensor driver

2011-04-05 Thread Laurent Pinchart
From: Detlev Casanova 

The MT9V032 is a parallel wide VGA sensor from Aptina (formerly Micron)
controlled through I2C.

The driver creates a V4L2 subdevice. It currently supports binning and
cropping, and the gain, auto gain, exposure, auto exposure and test
pattern controls.

Signed-off-by: Detlev Casanova 
Signed-off-by: Laurent Pinchart 
---
 drivers/media/video/Kconfig   |7 +
 drivers/media/video/Makefile  |1 +
 drivers/media/video/mt9v032.c |  773 +
 include/media/mt9v032.h   |   12 +
 4 files changed, 793 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/video/mt9v032.c
 create mode 100644 include/media/mt9v032.h

diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 4498b94..fa1da7d 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -337,6 +337,13 @@ config VIDEO_MT9V011
  mt0v011 1.3 Mpixel camera.  It currently only works with the
  em28xx driver.
 
+config VIDEO_MT9V032
+   tristate "Micron MT9V032 sensor support"
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the Micron
+ MT9V032 752x480 CMOS sensor.
+
 config VIDEO_TCM825X
tristate "TCM825x camera sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index ace5d8b..a10e4c3 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_VIDEO_OV7670)+= ov7670.o
 obj-$(CONFIG_VIDEO_TCM825X) += tcm825x.o
 obj-$(CONFIG_VIDEO_TVEEPROM) += tveeprom.o
 obj-$(CONFIG_VIDEO_MT9V011) += mt9v011.o
+obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o
 obj-$(CONFIG_VIDEO_SR030PC30)  += sr030pc30.o
 obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o
 
diff --git a/drivers/media/video/mt9v032.c b/drivers/media/video/mt9v032.c
new file mode 100644
index 000..1319c2c
--- /dev/null
+++ b/drivers/media/video/mt9v032.c
@@ -0,0 +1,773 @@
+/*
+ * Driver for MT9V032 CMOS Image Sensor from Micron
+ *
+ * Copyright (C) 2010, Laurent Pinchart 
+ *
+ * Based on the MT9M001 driver,
+ *
+ * Copyright (C) 2008, Guennadi Liakhovetski 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define MT9V032_PIXEL_ARRAY_HEIGHT 492
+#define MT9V032_PIXEL_ARRAY_WIDTH  782
+
+#define MT9V032_CHIP_VERSION   0x00
+#defineMT9V032_CHIP_ID_REV10x1311
+#defineMT9V032_CHIP_ID_REV30x1313
+#define MT9V032_ROW_START  0x01
+#defineMT9V032_ROW_START_MIN   4
+#defineMT9V032_ROW_START_DEF   10
+#defineMT9V032_ROW_START_MAX   482
+#define MT9V032_COLUMN_START   0x02
+#defineMT9V032_COLUMN_START_MIN1
+#defineMT9V032_COLUMN_START_DEF2
+#defineMT9V032_COLUMN_START_MAX752
+#define MT9V032_WINDOW_HEIGHT  0x03
+#defineMT9V032_WINDOW_HEIGHT_MIN   1
+#defineMT9V032_WINDOW_HEIGHT_DEF   480
+#defineMT9V032_WINDOW_HEIGHT_MAX   480
+#define MT9V032_WINDOW_WIDTH   0x04
+#defineMT9V032_WINDOW_WIDTH_MIN1
+#defineMT9V032_WINDOW_WIDTH_DEF752
+#defineMT9V032_WINDOW_WIDTH_MAX752
+#define MT9V032_HORIZONTAL_BLANKING0x05
+#defineMT9V032_HORIZONTAL_BLANKING_MIN 43
+#defineMT9V032_HORIZONTAL_BLANKING_MAX 1023
+#define MT9V032_VERTICAL_BLANKING  0x06
+#defineMT9V032_VERTICAL_BLANKING_MIN   4
+#defineMT9V032_VERTICAL_BLANKING_MAX   3000
+#define MT9V032_CHIP_CONTROL   0x07
+#defineMT9V032_CHIP_CONTROL_MASTER_MODE(1 << 3)
+#defineMT9V032_CHIP_CONTROL_DOUT_ENABLE(1 << 7)
+#defineMT9V032_CHIP_CONTROL_SEQUENTIAL (1 << 8)
+#define MT9V032_SHUTTER_WIDTH1 0x08
+#define MT9V032_SHUTTER_WIDTH2 0x09
+#define MT9V032_SHUTTER_WIDTH_CONTROL  0x0a
+#define MT9V032_TOTAL_SHUTTER_WIDTH0x0b
+#defineMT9V032_TOTAL_SHUTTER_WIDTH_MIN 1
+#defineMT9V032_TOTAL_SHUTTER_WIDTH_DEF 

Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Laurent Pinchart
On Monday 04 April 2011 10:06:47 Hans Verkuil wrote:
> > On Mon, 4 Apr 2011, Hans Verkuil wrote:
> >> On Friday, April 01, 2011 10:13:02 Guennadi Liakhovetski wrote:

[snip]

> BTW, REQBUFS and CREATE/DESTROY_BUFS should definitely co-exist. REQBUFS
> is compulsory, while CREATE/DESTROY are optional.

Drivers must support REQBUFS and should support CREATE/DESTROY, but I think 
applications should not be allowed to mix calls.

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 1/4] V4L: add three new ioctl()s for multi-size videobuffer management

2011-04-05 Thread Laurent Pinchart
Hi Guennadi,

On Friday 01 April 2011 10:13:02 Guennadi Liakhovetski wrote:
> A possibility to preallocate and initialise buffers of different sizes
> in V4L2 is required for an efficient implementation of asnapshot mode.
> This patch adds three new ioctl()s: VIDIOC_CREATE_BUFS,
> VIDIOC_DESTROY_BUFS, and VIDIOC_SUBMIT_BUF and defines respective data
> structures.

[snip]

> diff --git a/drivers/media/video/v4l2-ioctl.c
> b/drivers/media/video/v4l2-ioctl.c index a01ed39..b80a211 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c

[snip]

> @@ -2184,6 +2187,46 @@ static long __video_do_ioctl(struct file *file,
>   dbgarg(cmd, "type=0x%8.8x", sub->type);
>   break;
>   }
> + case VIDIOC_CREATE_BUFS:
> + {
> + struct v4l2_create_buffers *create = arg;
> +
> + if (!ops->vidioc_create_bufs)
> + break;
> + ret = check_fmt(ops, create->format.type);
> + if (ret)
> + break;
> +
> + if (create->size)
> + CLEAR_AFTER_FIELD(create, count);

Why only when create->size is > 0 ?

> + ret = ops->vidioc_create_bufs(file, fh, create);
> +
> + dbgarg(cmd, "count=%d\n", create->count);
> + break;
> + }
> + case VIDIOC_DESTROY_BUFS:
> + {
> + struct v4l2_buffer_span *span = arg;
> +
> + if (!ops->vidioc_destroy_bufs)
> + break;
> +
> + ret = ops->vidioc_destroy_bufs(file, fh, span);
> +
> + dbgarg(cmd, "count=%d", span->count);
> + break;
> + }
> + case VIDIOC_SUBMIT_BUF:
> + {
> + unsigned int *i = arg;
> +
> + if (!ops->vidioc_submit_buf)
> + break;
> + ret = ops->vidioc_submit_buf(file, fh, *i);
> + dbgarg(cmd, "index=%d", *i);
> + break;
> + }
>   default:
>   {
>   bool valid_prio = true;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index aa6c393..b6ef46e 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1847,6 +1847,26 @@ struct v4l2_dbg_chip_ident {
>   __u32 revision;/* chip revision, chip specific */
>  } __attribute__ ((packed));
> 
> +/* VIDIOC_DESTROY_BUFS */
> +struct v4l2_buffer_span {
> + __u32   index;  /* output: buffers index...index + 
> count - 1 have been
> created */ +  __u32   count;
> + __u32   reserved[2];
> +};
> +
> +/* struct v4l2_createbuffers::flags */
> +#define V4L2_BUFFER_FLAG_NO_CACHE_INVALIDATE (1 << 0)

Shouldn't cache management be handled at submit/qbuf time instead of being a 
buffer property ?

> +/* VIDIOC_CREATE_BUFS */
> +struct v4l2_create_buffers {
> + __u32   index;  /* output: buffers 
> index...index + count - 1 have been
> created */ +  __u32   count;
> + __u32   flags;  /* V4L2_BUFFER_FLAG_* */
> + enum v4l2_memorymemory;
> + __u32   size;   /* Explicit size, e.g., for 
> compressed streams */
> + struct v4l2_format  format; /* "type" is used always, the 
> rest if size 
==
> 0 */ +};

You need reserved fields here.

> +
>  /*
>   *   I O C T L   C O D E S   F O R   V I D E O   D E V I C E S
>   *
> @@ -1937,6 +1957,10 @@ struct v4l2_dbg_chip_ident {
>  #define  VIDIOC_SUBSCRIBE_EVENT   _IOW('V', 90, struct
> v4l2_event_subscription) #define  VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91,
> struct v4l2_event_subscription)
> 
> +#define VIDIOC_CREATE_BUFS   _IOWR('V', 92, struct v4l2_create_buffers)
> +#define VIDIOC_DESTROY_BUFS  _IOWR('V', 93, struct v4l2_buffer_span)

Just throwing an idea in here, what about using the same structure for both 
ioctls ? Or even a single ioctl for both create and destroy, like we do with 
REQBUFS ?

> +#define VIDIOC_SUBMIT_BUF _IOW('V', 94, int)
> +
>  /* Reminder: when adding new ioctls please add support for them to
> drivers/media/video/v4l2-compat-ioctl32.c as well! */
> 

[snip]

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] V4L2 API for flash devices

2011-04-05 Thread Laurent Pinchart
Hi Sakari,

On Tuesday 05 April 2011 13:21:03 Sakari Ailus wrote:
> Laurent Pinchart wrote:
> > On Tuesday 05 April 2011 12:23:51 Sakari Ailus wrote:
> >> Sakari Ailus wrote:
> >>> Laurent Pinchart wrote:
>  On Wednesday 30 March 2011 13:05:54 Sakari Ailus wrote:
> > Laurent Pinchart wrote:
> >> On Monday 28 March 2011 14:55:40 Sakari Ailus wrote:
> >> 
> >> [snip]
> >> 
> >>>   V4L2_CID_FLASH_STROBE_MODE (menu; LED)
> >>> 
> >>> Use hardware or software strobe. If hardware strobe is selected,
> >>> the flash controller is a slave in the system where the sensor
> >>> produces the strobe signal to the flash.
> >>> 
> >>> In this case the flash controller setup is limited to programming
> >>> strobe timeout and power (LED flash) and the sensor controls the
> >>> timing and length of the strobe.
> >>> 
> >>> enum v4l2_flash_strobe_mode {
> >>> 
> >>>   V4L2_FLASH_STROBE_MODE_SOFTWARE,
> >>>   V4L2_FLASH_STROBE_MODE_EXT_STROBE,
> >>> 
> >>> };
> >> 
> >> [snip]
> >> 
> >>>   V4L2_CID_FLASH_LED_MODE (menu; LED)
> >>> 
> >>> enum v4l2_flash_led_mode {
> >>> 
> >>>   V4L2_FLASH_LED_MODE_FLASH = 1,
> >>>   V4L2_FLASH_LED_MODE_TORCH,
> >>> 
> >>> };
> >> 
> >> Thinking about this some more, shouldn't we combine the two controls
> >> ? They are basically used to configure how the flash LED is
> >> controlled: manually (torch mode), automatically by the flash
> >> controller (software strobe mode) or automatically by an external
> >> component (external strobe mode).
> > 
> > That's a good question.
> > 
> > The adp1653 supports also additional control (not implemented in the
> > driver, though) that affect hardware strobe length. Based on register
> > setting, the led will be on after strobe either until the timeout
> > expires, or until the strobe signal is high.
> > 
> > Should this be also part of the same control, or a different one?
>  
>  That can be controlled by a duration control. If the duration is 0,
>  the flash is lit for the duration of the external strobe, otherwise
>  it's lit for the programmed duration.
> >>> 
> >>> Sounds good to me.
> >> 
> >> Thinking about this again; there won't be a separate duration control
> > 
> > Why not ? I think we need two timeouts, a watchdog timeout to prevent
> > flash fire or meltdown, and a normal timeout to lit the flash for a
> > user-selected duration.
> 
> Let's assume that an application wants to expose a frame using flash
> with software strobe.
> 
> 1. strobe flash
> 2. qbuf
> 3. streamon
> 4. dqbuf
> 5. streamoff
> 6. ...
> 
> How does an application know how long is the time between 1 -- 4? I'd
> guess that in 6 the application would like to switch off the flash
> instead of specifying a timeout for it.

That's a valid use case, and we need to support it. It requires a way to lit 
the flash with no timeout other than the watchdog timeout, and a way to turn 
it off.

However, I'm not sure we should rule out the usefulness of a duration-based 
software flash strobe. Can't the two APIs coexist ? Or do you think a 
duration-based API is useless ?

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] V4L2 API for flash devices

2011-04-05 Thread Sakari Ailus
Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tuesday 05 April 2011 12:23:51 Sakari Ailus wrote:
>> Sakari Ailus wrote:
>>> Laurent Pinchart wrote:
 On Wednesday 30 March 2011 13:05:54 Sakari Ailus wrote:
> Laurent Pinchart wrote:
>> On Monday 28 March 2011 14:55:40 Sakari Ailus wrote:
>>
>> [snip]
>>
>>> V4L2_CID_FLASH_STROBE_MODE (menu; LED)
>>>
>>> Use hardware or software strobe. If hardware strobe is selected, the
>>> flash controller is a slave in the system where the sensor produces
>>> the strobe signal to the flash.
>>>
>>> In this case the flash controller setup is limited to programming
>>> strobe timeout and power (LED flash) and the sensor controls the
>>> timing and length of the strobe.
>>>
>>> enum v4l2_flash_strobe_mode {
>>>
>>> V4L2_FLASH_STROBE_MODE_SOFTWARE,
>>> V4L2_FLASH_STROBE_MODE_EXT_STROBE,
>>>
>>> };
>>
>> [snip]
>>
>>> V4L2_CID_FLASH_LED_MODE (menu; LED)
>>>
>>> enum v4l2_flash_led_mode {
>>>
>>> V4L2_FLASH_LED_MODE_FLASH = 1,
>>> V4L2_FLASH_LED_MODE_TORCH,
>>>
>>> };
>>
>> Thinking about this some more, shouldn't we combine the two controls ?
>> They are basically used to configure how the flash LED is controlled:
>> manually (torch mode), automatically by the flash controller (software
>> strobe mode) or automatically by an external component (external
>> strobe mode).
>
> That's a good question.
>
> The adp1653 supports also additional control (not implemented in the
> driver, though) that affect hardware strobe length. Based on register
> setting, the led will be on after strobe either until the timeout
> expires, or until the strobe signal is high.
>
> Should this be also part of the same control, or a different one?

 That can be controlled by a duration control. If the duration is 0, the
 flash is lit for the duration of the external strobe, otherwise it's
 lit for the programmed duration.
>>>
>>> Sounds good to me.
>>
>> Thinking about this again; there won't be a separate duration control
> 
> Why not ? I think we need two timeouts, a watchdog timeout to prevent flash 
> fire or meltdown, and a normal timeout to lit the flash for a user-selected 
> duration.

Let's assume that an application wants to expose a frame using flash
with software strobe.

1. strobe flash
2. qbuf
3. streamon
4. dqbuf
5. streamoff
6. ...

How does an application know how long is the time between 1 -- 4? I'd
guess that in 6 the application would like to switch off the flash
instead of specifying a timeout for it.

-- 
Sakari Ailus
sakari.ai...@maxwell.research.nokia.com
--
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


TT-budget S2-3200 cannot tune on HB13E DVBS2 transponder

2011-04-05 Thread Issa Gorissen
Hi,

Eutelsat made a recent migration from DVB-S to DVB-S2 (since 31/3/2011) on two
transponders on HB13E

- HOT BIRD 6 13° Est TP 159 Freq 11,681 Ghz DVB-S2 FEC 3/4 27500 Msymb/s 0.2
Pilot off Polar H

- HOT BIRD 9 13° Est TP 99 Freq 12,692 Ghz DVB-S2 FEC 3/4 27500 Msymb/s 0.2
Pilot off Polar H


Before those changes, with my TT S2 3200, I was able to watch TV on those
transponders. Now, I cannot even tune on those transponders. I have tried with
scan-s2 and w_scan and the latest drivers from git. They both find the
transponders but cannot tune onto it.

Something noteworthy is that my other card, a DuoFlex S2 can tune fine on
those transponders.

My question is; can someone try this as well with a TT S2 3200 and post the
results ?

Thank you a lot,
--
Issa

--
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 v18 08/13] davinci: eliminate use of IO_ADDRESS() on sysmod

2011-04-05 Thread Nori, Sekhar
Hi Manju,

On Sat, Apr 02, 2011 at 15:13:17, Hadli, Manjunath wrote:
> Current devices.c file has a number of instances where
> IO_ADDRESS() is used for system module register
> access. Eliminate this in favor of a ioremap()
> based access.
> 
> Consequent to this, a new global pointer davinci_sysmodbase
> has been introduced which gets initialized during
> the initialization of each relevant SoC
> 
> Signed-off-by: Manjunath Hadli 
> Acked-by: Sekhar Nori 
> ---

> diff --git a/arch/arm/mach-davinci/include/mach/hardware.h 
> b/arch/arm/mach-davinci/include/mach/hardware.h
> index 414e0b9..2a6b560 100644
> --- a/arch/arm/mach-davinci/include/mach/hardware.h
> +++ b/arch/arm/mach-davinci/include/mach/hardware.h
> @@ -21,6 +21,12 @@
>   */
>  #define DAVINCI_SYSTEM_MODULE_BASE0x01C4
>  
> +#ifndef __ASSEMBLER__
> +extern void __iomem *davinci_sysmodbase;
> +#define DAVINCI_SYSMODULE_VIRT(x)(davinci_sysmodbase + (x))
> +void davinci_map_sysmod(void);
> +#endif

Russell has posted[1] that the hardware.h file should
not be polluted with platform private stuff like this.

Your patch 7/13 actually helped towards that goal, but
this one takes us back. This patch cannot be used in
the current form.

Currently there are separate header files for dm644x,
dm355, dm646x and dm365. I would like to start by
removing unnecessary code from these files and trying
to consolidate them into a single file.

Example, the EMAC base address definitions in dm365.h
should be moved into dm365.c. Similarly, there is a lot
of VPIF specific stuff in dm646x.h which is not really
specific to dm646x.h and so should probably be moved to
include/media/ or arch/arm/mach-davinci/include/mach/vpif.h

Once consolidated into a single file, davinci_sysmodbase
can be moved into that file.

Also, Russell has said[2] that at least for this merge
window only consolidation and bug fixes will go through
his tree. This means that as far as mach-davinci is
concerned, the clean-up part of this series can go to
2.6.40 - but not the stuff which adds new support.

Thanks,
Sekhar

[1] http://www.spinics.net/lists/arm-kernel/msg120410.html
[2] http://www.spinics.net/lists/arm-kernel/msg120606.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] V4L2 API for flash devices

2011-04-05 Thread Laurent Pinchart
Hi Sakari,

On Tuesday 05 April 2011 12:23:51 Sakari Ailus wrote:
> Sakari Ailus wrote:
> > Laurent Pinchart wrote:
> >> On Wednesday 30 March 2011 13:05:54 Sakari Ailus wrote:
> >>> Laurent Pinchart wrote:
>  On Monday 28 March 2011 14:55:40 Sakari Ailus wrote:
>  
>  [snip]
>  
> > V4L2_CID_FLASH_STROBE_MODE (menu; LED)
> > 
> > Use hardware or software strobe. If hardware strobe is selected, the
> > flash controller is a slave in the system where the sensor produces
> > the strobe signal to the flash.
> > 
> > In this case the flash controller setup is limited to programming
> > strobe timeout and power (LED flash) and the sensor controls the
> > timing and length of the strobe.
> > 
> > enum v4l2_flash_strobe_mode {
> > 
> > V4L2_FLASH_STROBE_MODE_SOFTWARE,
> > V4L2_FLASH_STROBE_MODE_EXT_STROBE,
> > 
> > };
>  
>  [snip]
>  
> > V4L2_CID_FLASH_LED_MODE (menu; LED)
> > 
> > enum v4l2_flash_led_mode {
> > 
> > V4L2_FLASH_LED_MODE_FLASH = 1,
> > V4L2_FLASH_LED_MODE_TORCH,
> > 
> > };
>  
>  Thinking about this some more, shouldn't we combine the two controls ?
>  They are basically used to configure how the flash LED is controlled:
>  manually (torch mode), automatically by the flash controller (software
>  strobe mode) or automatically by an external component (external
>  strobe mode).
> >>> 
> >>> That's a good question.
> >>> 
> >>> The adp1653 supports also additional control (not implemented in the
> >>> driver, though) that affect hardware strobe length. Based on register
> >>> setting, the led will be on after strobe either until the timeout
> >>> expires, or until the strobe signal is high.
> >>> 
> >>> Should this be also part of the same control, or a different one?
> >> 
> >> That can be controlled by a duration control. If the duration is 0, the
> >> flash is lit for the duration of the external strobe, otherwise it's
> >> lit for the programmed duration.
> > 
> > Sounds good to me.
> 
> Thinking about this again; there won't be a separate duration control

Why not ? I think we need two timeouts, a watchdog timeout to prevent flash 
fire or meltdown, and a normal timeout to lit the flash for a user-selected 
duration.

> and the hardware timeout can't be zero in a general case.
> 
> So this is not an option, and I don't think we'd want to add duration
> control for this purpose.
> 
> What about V4L2_CID_FLASH_EXTERNAL_STROBE_EDGE?

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] V4L2 API for flash devices

2011-04-05 Thread Sakari Ailus
Hi Laurent,

Sakari Ailus wrote:
> Laurent Pinchart wrote:
>> On Wednesday 30 March 2011 13:05:54 Sakari Ailus wrote:
>>> Laurent Pinchart wrote:
 Hi Sakari,
>>>
>>> Hi Laurent,
>>>
>>> Thanks for the comments!
>>>
 On Monday 28 March 2011 14:55:40 Sakari Ailus wrote:

 [snip]

>   V4L2_CID_FLASH_STROBE_MODE (menu; LED)
>
> Use hardware or software strobe. If hardware strobe is selected, the
> flash controller is a slave in the system where the sensor produces the
> strobe signal to the flash.
>
> In this case the flash controller setup is limited to programming strobe
> timeout and power (LED flash) and the sensor controls the timing and
> length of the strobe.
>
> enum v4l2_flash_strobe_mode {
>
>   V4L2_FLASH_STROBE_MODE_SOFTWARE,
>   V4L2_FLASH_STROBE_MODE_EXT_STROBE,
>
> };

 [snip]

>   V4L2_CID_FLASH_LED_MODE (menu; LED)
>
> enum v4l2_flash_led_mode {
>
>   V4L2_FLASH_LED_MODE_FLASH = 1,
>   V4L2_FLASH_LED_MODE_TORCH,
>
> };

 Thinking about this some more, shouldn't we combine the two controls ?
 They are basically used to configure how the flash LED is controlled:
 manually (torch mode), automatically by the flash controller (software
 strobe mode) or automatically by an external component (external strobe
 mode).
>>>
>>> That's a good question.
>>>
>>> The adp1653 supports also additional control (not implemented in the
>>> driver, though) that affect hardware strobe length. Based on register
>>> setting, the led will be on after strobe either until the timeout
>>> expires, or until the strobe signal is high.
>>>
>>> Should this be also part of the same control, or a different one?
>>
>> That can be controlled by a duration control. If the duration is 0, the 
>> flash 
>> is lit for the duration of the external strobe, otherwise it's lit for the 
>> programmed duration.
> 
> Sounds good to me.

Thinking about this again; there won't be a separate duration control
and the hardware timeout can't be zero in a general case.

So this is not an option, and I don't think we'd want to add duration
control for this purpose.

What about V4L2_CID_FLASH_EXTERNAL_STROBE_EDGE?

-- 
Sakari Ailus
sakari.ai...@maxwell.research.nokia.com
--
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] rc-core: int to bool conversion for winbond-cir

2011-04-05 Thread David Härdeman
Using bool instead of an int helps readability a bit.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/winbond-cir.c |   16 
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
index b0a5fdc..cdb5ef4 100644
--- a/drivers/media/rc/winbond-cir.c
+++ b/drivers/media/rc/winbond-cir.c
@@ -382,7 +382,7 @@ wbcir_shutdown(struct pnp_dev *device)
 {
struct device *dev = &device->dev;
struct wbcir_data *data = pnp_get_drvdata(device);
-   int do_wake = 1;
+   bool do_wake = true;
u8 match[11];
u8 mask[11];
u8 rc6_csl = 0;
@@ -392,14 +392,14 @@ wbcir_shutdown(struct pnp_dev *device)
memset(mask, 0, sizeof(mask));
 
if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev)) {
-   do_wake = 0;
+   do_wake = false;
goto finish;
}
 
switch (protocol) {
case IR_PROTOCOL_RC5:
if (wake_sc > 0xFFF) {
-   do_wake = 0;
+   do_wake = false;
dev_err(dev, "RC5 - Invalid wake scancode\n");
break;
}
@@ -418,7 +418,7 @@ wbcir_shutdown(struct pnp_dev *device)
 
case IR_PROTOCOL_NEC:
if (wake_sc > 0xFF) {
-   do_wake = 0;
+   do_wake = false;
dev_err(dev, "NEC - Invalid wake scancode\n");
break;
}
@@ -440,7 +440,7 @@ wbcir_shutdown(struct pnp_dev *device)
 
if (wake_rc6mode == 0) {
if (wake_sc > 0x) {
-   do_wake = 0;
+   do_wake = false;
dev_err(dev, "RC6 - Invalid wake scancode\n");
break;
}
@@ -496,7 +496,7 @@ wbcir_shutdown(struct pnp_dev *device)
} else if (wake_sc <= 0x007F) {
rc6_csl = 60;
} else {
-   do_wake = 0;
+   do_wake = false;
dev_err(dev, "RC6 - Invalid wake scancode\n");
break;
}
@@ -508,14 +508,14 @@ wbcir_shutdown(struct pnp_dev *device)
mask[i++] = 0x0F;
 
} else {
-   do_wake = 0;
+   do_wake = false;
dev_err(dev, "RC6 - Invalid wake mode\n");
}
 
break;
 
default:
-   do_wake = 0;
+   do_wake = false;
break;
}
 

--
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] rc-core: set mode and default keymap for winbond-cir

2011-04-05 Thread David Härdeman
Not having the correct mode and a default keymap is not
very user-friendly (and rc-core won't allow it).

Signed-off-by: David Härdeman 
---
 drivers/media/rc/winbond-cir.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
index 16f4178..b0a5fdc 100644
--- a/drivers/media/rc/winbond-cir.c
+++ b/drivers/media/rc/winbond-cir.c
@@ -790,6 +790,7 @@ wbcir_probe(struct pnp_dev *device, const struct 
pnp_device_id *dev_id)
goto exit_unregister_led;
}
 
+   data->dev->driver_type = RC_DRIVER_IR_RAW;
data->dev->driver_name = WBCIR_NAME;
data->dev->input_name = WBCIR_NAME;
data->dev->input_phys = "wbcir/cir0";
@@ -797,6 +798,7 @@ wbcir_probe(struct pnp_dev *device, const struct 
pnp_device_id *dev_id)
data->dev->input_id.vendor = PCI_VENDOR_ID_WINBOND;
data->dev->input_id.product = WBCIR_ID_FAMILY;
data->dev->input_id.version = WBCIR_ID_CHIP;
+   data->dev->map_name = RC_MAP_RC6_MCE;
data->dev->priv = data;
data->dev->dev.parent = &device->dev;
 

--
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] V4L2 API for flash devices

2011-04-05 Thread Sakari Ailus
Laurent Pinchart wrote:
> Hi Sakari,

Hi Laurent,

> On Tuesday 29 March 2011 13:51:38 Sakari Ailus wrote:
>> Sakari Ailus wrote:
>>> Hans Verkuil wrote:
 On Tuesday, March 29, 2011 11:35:19 Sakari Ailus wrote:
> Hi Hans,
>
> Many thanks for the comments!
>>>
>>> ...
>>>
> It occurred to me that an application might want to turn off a flash
> which has been strobed on software. That can't be done on a single
> button control.
>
> V4L2_CID_FLASH_SHUTDOWN?
>
> The application would know the flash strobe is ongoing before it
> receives a timeout fault. I somehow feel that there should be a control
> telling that directly.
>
> What about using a bool control for the strobe?

 It depends: is the strobe signal just a pulse that kicks off the flash,
 or is it active throughout the flash duration? In the latter case a
 bool makes sense, in the first case an extra button control makes
 sense.
>>>
>>> I like buttons since I associate them with action (like strobing) but on
>>> the other hand buttons don't allow querying the current state. On the
>>> other hand, the current state isn't always determinable, e.g. in the
>>> absence of the interrupt line from the flash controller interrupt pin
>>> (e.g. N900!).
>>
>> Oh, I need to take my words back a bit.
>>
>> There indeed is a way to get the on/off status for the flash, but that
>> involves I2C register access --- when you read the fault registers, you
>> do get the state, even if the interrupt linke is missing from the
>> device. At least I can't see why this wouldn't work, at least on this
>> particular chip.
>>
>> What you can't have in this case is the event.
>>
>> So, in my opinion this suggests that a single boolean control is the way
>> to go.
> 
> Why would an application want to turn off a flash that has been strobbed in 
> software ? Applications should set the flash duration and then strobe it.

The applications won't know beforehand the exact timing of the exposure
of the frames on the sensor and the latencies of the operating system
possibly affected by other processes running on the system. Thus it's
impossible to know exactly how long flash strobe (on software, that is!)
is required.

So, as far as I see there should be a way to turn the flash off and the
timeout would mostly function as a safeguard. This is likely dependent
on the flash controller as well.

-- 
Sakari Ailus
sakari.ai...@maxwell.research.nokia.com
--
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 13/14] omap3isp: lane shifter support

2011-04-05 Thread Laurent Pinchart
From: Michael Jones 

To use the lane shifter, set different pixel formats at each end of
the link at the CCDC input.

Signed-off-by: Michael Jones 
Acked-by: Laurent Pinchart 
---
 drivers/media/video/omap3isp/isp.c  |7 ++-
 drivers/media/video/omap3isp/isp.h  |5 +-
 drivers/media/video/omap3isp/ispccdc.c  |   27 ++--
 drivers/media/video/omap3isp/ispvideo.c |  108 +--
 drivers/media/video/omap3isp/ispvideo.h |3 +
 5 files changed, 120 insertions(+), 30 deletions(-)

diff --git a/drivers/media/video/omap3isp/isp.c 
b/drivers/media/video/omap3isp/isp.c
index 78240c0..8190511 100644
--- a/drivers/media/video/omap3isp/isp.c
+++ b/drivers/media/video/omap3isp/isp.c
@@ -287,7 +287,8 @@ static void isp_power_settings(struct isp_device *isp, int 
idle)
  */
 void omap3isp_configure_bridge(struct isp_device *isp,
   enum ccdc_input_entity input,
-  const struct isp_parallel_platform_data *pdata)
+  const struct isp_parallel_platform_data *pdata,
+  unsigned int shift)
 {
u32 ispctrl_val;
 
@@ -300,9 +301,9 @@ void omap3isp_configure_bridge(struct isp_device *isp,
switch (input) {
case CCDC_INPUT_PARALLEL:
ispctrl_val |= ISPCTRL_PAR_SER_CLK_SEL_PARALLEL;
-   ispctrl_val |= pdata->data_lane_shift << ISPCTRL_SHIFT_SHIFT;
ispctrl_val |= pdata->clk_pol << ISPCTRL_PAR_CLK_POL_SHIFT;
ispctrl_val |= pdata->bridge << ISPCTRL_PAR_BRIDGE_SHIFT;
+   shift += pdata->data_lane_shift * 2;
break;
 
case CCDC_INPUT_CSI2A:
@@ -321,6 +322,8 @@ void omap3isp_configure_bridge(struct isp_device *isp,
return;
}
 
+   ispctrl_val |= ((shift/2) << ISPCTRL_SHIFT_SHIFT) & ISPCTRL_SHIFT_MASK;
+
ispctrl_val &= ~ISPCTRL_SYNC_DETECT_MASK;
ispctrl_val |= ISPCTRL_SYNC_DETECT_VSRISE;
 
diff --git a/drivers/media/video/omap3isp/isp.h 
b/drivers/media/video/omap3isp/isp.h
index 00075c6..2620c40 100644
--- a/drivers/media/video/omap3isp/isp.h
+++ b/drivers/media/video/omap3isp/isp.h
@@ -132,7 +132,6 @@ struct isp_reg {
 
 /**
  * struct isp_parallel_platform_data - Parallel interface platform data
- * @width: Parallel bus width in bits (8, 10, 11 or 12)
  * @data_lane_shift: Data lane shifter
  * 0 - CAMEXT[13:0] -> CAM[13:0]
  * 1 - CAMEXT[13:2] -> CAM[11:0]
@@ -146,7 +145,6 @@ struct isp_reg {
  * ISPCTRL_PAR_BRIDGE_BENDIAN - Big endian
  */
 struct isp_parallel_platform_data {
-   unsigned int width;
unsigned int data_lane_shift:2;
unsigned int clk_pol:1;
unsigned int bridge:4;
@@ -312,7 +310,8 @@ int omap3isp_pipeline_set_stream(struct isp_pipeline *pipe,
 enum isp_pipeline_stream_state state);
 void omap3isp_configure_bridge(struct isp_device *isp,
   enum ccdc_input_entity input,
-  const struct isp_parallel_platform_data *pdata);
+  const struct isp_parallel_platform_data *pdata,
+  unsigned int shift);
 
 #define ISP_XCLK_NONE  0
 #define ISP_XCLK_A 1
diff --git a/drivers/media/video/omap3isp/ispccdc.c 
b/drivers/media/video/omap3isp/ispccdc.c
index 68941ed..39d501b 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -1116,21 +1116,38 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
struct isp_parallel_platform_data *pdata = NULL;
struct v4l2_subdev *sensor;
struct v4l2_mbus_framefmt *format;
+   const struct isp_format_info *fmt_info;
+   struct v4l2_subdev_format fmt_src;
+   unsigned int depth_out;
+   unsigned int depth_in = 0;
struct media_pad *pad;
unsigned long flags;
+   unsigned int shift;
u32 syn_mode;
u32 ccdc_pattern;
 
-   if (ccdc->input == CCDC_INPUT_PARALLEL) {
-   pad = media_entity_remote_source(&ccdc->pads[CCDC_PAD_SINK]);
-   sensor = media_entity_to_v4l2_subdev(pad->entity);
+   pad = media_entity_remote_source(&ccdc->pads[CCDC_PAD_SINK]);
+   sensor = media_entity_to_v4l2_subdev(pad->entity);
+   if (ccdc->input == CCDC_INPUT_PARALLEL)
pdata = &((struct isp_v4l2_subdevs_group *)sensor->host_priv)
->bus.parallel;
+
+   /* Compute shift value for lane shifter to configure the bridge. */
+   fmt_src.pad = pad->index;
+   fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+   if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) {
+   fmt_info = omap3isp_video_format_info(fmt_src.format.code);
+   depth_in = fmt_info->bpp;
}
 
-   omap3isp_configure_bridge(isp, ccdc->input, pdata);
+   fmt_info = o

[PATCH 14/14] omap3isp: Don't increment node entity use count when poweron fails

2011-04-05 Thread Laurent Pinchart
When open a device node, all entities part of the same pipeline are
powered on. If one of the entities fails to be powered on, the open
operations fails. In that case the device node entity use count must not
be incremented.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/video/omap3isp/isp.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/omap3isp/isp.c 
b/drivers/media/video/omap3isp/isp.c
index 8190511..f4edbf0 100644
--- a/drivers/media/video/omap3isp/isp.c
+++ b/drivers/media/video/omap3isp/isp.c
@@ -663,6 +663,8 @@ int omap3isp_pipeline_pm_use(struct media_entity *entity, 
int use)
 
/* Apply power change to connected non-nodes. */
ret = isp_pipeline_pm_power(entity, change);
+   if (ret < 0)
+   entity->use_count -= change;
 
mutex_unlock(&entity->parent->graph_mutex);
 
-- 
1.7.3.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 11/14] media: add missing 8-bit bayer formats and Y12

2011-04-05 Thread Laurent Pinchart
From: Michael Jones 

8-bit SGBRG and SRGGB media bus formats are missing, as well as the
12-bit grey format. Add them.

Signed-off-by: Michael Jones 
Acked-by: Laurent Pinchart 
---
 Documentation/DocBook/v4l/subdev-formats.xml |   59 ++
 include/linux/v4l2-mediabus.h|7 ++-
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/v4l/subdev-formats.xml 
b/Documentation/DocBook/v4l/subdev-formats.xml
index 7041127..d7ccd25 100644
--- a/Documentation/DocBook/v4l/subdev-formats.xml
+++ b/Documentation/DocBook/v4l/subdev-formats.xml
@@ -456,6 +456,23 @@
  b1
  b0

+   
+ V4L2_MBUS_FMT_SGBRG8_1X8
+ 0x3013
+ 
+ -
+ -
+ -
+ -
+ g7
+ g6
+ g5
+ g4
+ g3
+ g2
+ g1
+ g0
+   

  V4L2_MBUS_FMT_SGRBG8_1X8
  0x3002
@@ -473,6 +490,23 @@
  g1
  g0

+   
+ V4L2_MBUS_FMT_SRGGB8_1X8
+ 0x3014
+ 
+ -
+ -
+ -
+ -
+ r7
+ r6
+ r5
+ r4
+ r3
+ r2
+ r1
+ r0
+   

  V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8
  0x300b
@@ -2159,6 +2193,31 @@
  u1
  u0

+   
+ V4L2_MBUS_FMT_Y12_1X12
+ 0x2013
+ 
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ y11
+ y10
+ y9
+ y8
+ y7
+ y6
+ y5
+ y4
+ y3
+ y2
+ y1
+ y0
+   

  V4L2_MBUS_FMT_UYVY8_1X16
  0x200f
diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h
index 7054a7a..de5c159 100644
--- a/include/linux/v4l2-mediabus.h
+++ b/include/linux/v4l2-mediabus.h
@@ -47,7 +47,7 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_RGB565_2X8_BE = 0x1007,
V4L2_MBUS_FMT_RGB565_2X8_LE = 0x1008,
 
-   /* YUV (including grey) - next is 0x2013 */
+   /* YUV (including grey) - next is 0x2014 */
V4L2_MBUS_FMT_Y8_1X8 = 0x2001,
V4L2_MBUS_FMT_UYVY8_1_5X8 = 0x2002,
V4L2_MBUS_FMT_VYUY8_1_5X8 = 0x2003,
@@ -60,6 +60,7 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_Y10_1X10 = 0x200a,
V4L2_MBUS_FMT_YUYV10_2X10 = 0x200b,
V4L2_MBUS_FMT_YVYU10_2X10 = 0x200c,
+   V4L2_MBUS_FMT_Y12_1X12 = 0x2013,
V4L2_MBUS_FMT_UYVY8_1X16 = 0x200f,
V4L2_MBUS_FMT_VYUY8_1X16 = 0x2010,
V4L2_MBUS_FMT_YUYV8_1X16 = 0x2011,
@@ -67,9 +68,11 @@ enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_YUYV10_1X20 = 0x200d,
V4L2_MBUS_FMT_YVYU10_1X20 = 0x200e,
 
-   /* Bayer - next is 0x3013 */
+   /* Bayer - next is 0x3015 */
V4L2_MBUS_FMT_SBGGR8_1X8 = 0x3001,
+   V4L2_MBUS_FMT_SGBRG8_1X8 = 0x3013,
V4L2_MBUS_FMT_SGRBG8_1X8 = 0x3002,
+   V4L2_MBUS_FMT_SRGGB8_1X8 = 0x3014,
V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8 = 0x300b,
V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8 = 0x300c,
V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8 = 0x3009,
-- 
1.7.3.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 10/14] v4l: add V4L2_PIX_FMT_Y12 format

2011-04-05 Thread Laurent Pinchart
From: Michael Jones 

Y12 is a grey-scale format with a depth of 12 bits per pixel stored in
16-bit words.

Signed-off-by: Michael Jones 
Acked-by: Laurent Pinchart 
---
 Documentation/DocBook/media-entities.tmpl |1 +
 Documentation/DocBook/v4l/pixfmt-y12.xml  |   79 +
 Documentation/DocBook/v4l/pixfmt.xml  |1 +
 include/linux/videodev2.h |1 +
 4 files changed, 82 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/DocBook/v4l/pixfmt-y12.xml

diff --git a/Documentation/DocBook/media-entities.tmpl 
b/Documentation/DocBook/media-entities.tmpl
index 5d259c6..fea63b4 100644
--- a/Documentation/DocBook/media-entities.tmpl
+++ b/Documentation/DocBook/media-entities.tmpl
@@ -294,6 +294,7 @@
 
 
 
+
 
 
 
diff --git a/Documentation/DocBook/v4l/pixfmt-y12.xml 
b/Documentation/DocBook/v4l/pixfmt-y12.xml
new file mode 100644
index 000..ff417b8
--- /dev/null
+++ b/Documentation/DocBook/v4l/pixfmt-y12.xml
@@ -0,0 +1,79 @@
+
+  
+V4L2_PIX_FMT_Y12 ('Y12 ')
+&manvol;
+  
+  
+V4L2_PIX_FMT_Y12
+Grey-scale image
+  
+  
+Description
+
+This is a grey-scale image with a depth of 12 bits per pixel. Pixels
+are stored in 16-bit words with unused high bits padded with 0. The least
+significant byte is stored at lower memory addresses (little-endian).
+
+
+  V4L2_PIX_FMT_Y12 4 × 4
+pixel image
+
+  
+   Byte Order.
+   Each cell is one byte.
+ 
+   
+ 
+ 
+   
+ start + 0:
+ Y'00low
+ Y'00high
+ Y'01low
+ Y'01high
+ Y'02low
+ Y'02high
+ Y'03low
+ Y'03high
+   
+   
+ start + 8:
+ Y'10low
+ Y'10high
+ Y'11low
+ Y'11high
+ Y'12low
+ Y'12high
+ Y'13low
+ Y'13high
+   
+   
+ start + 16:
+ Y'20low
+ Y'20high
+ Y'21low
+ Y'21high
+ Y'22low
+ Y'22high
+ Y'23low
+ Y'23high
+   
+   
+ start + 24:
+ Y'30low
+ Y'30high
+ Y'31low
+ Y'31high
+ Y'32low
+ Y'32high
+ Y'33low
+ Y'33high
+   
+ 
+   
+ 
+   
+  
+
+  
+
diff --git a/Documentation/DocBook/v4l/pixfmt.xml 
b/Documentation/DocBook/v4l/pixfmt.xml
index c6fdcbb..40af4be 100644
--- a/Documentation/DocBook/v4l/pixfmt.xml
+++ b/Documentation/DocBook/v4l/pixfmt.xml
@@ -696,6 +696,7 @@ information.
 &sub-packed-yuv;
 &sub-grey;
 &sub-y10;
+&sub-y12;
 &sub-y16;
 &sub-yuyv;
 &sub-uyvy;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index aa6c393..be82c8e 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -308,6 +308,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_Y4  v4l2_fourcc('Y', '0', '4', ' ') /*  4  Greyscale  
   */
 #define V4L2_PIX_FMT_Y6  v4l2_fourcc('Y', '0', '6', ' ') /*  6  Greyscale  
   */
 #define V4L2_PIX_FMT_Y10 v4l2_fourcc('Y', '1', '0', ' ') /* 10  Greyscale  
   */
+#define V4L2_PIX_FMT_Y12 v4l2_fourcc('Y', '1', '2', ' ') /* 12  Greyscale  
   */
 #define V4L2_PIX_FMT_Y16 v4l2_fourcc('Y', '1', '6', ' ') /* 16  Greyscale  
   */
 
 /* Palette formats */
-- 
1.7.3.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 12/14] omap3isp: ccdc: support Y10/12, 8-bit bayer fmts

2011-04-05 Thread Laurent Pinchart
From: Michael Jones 

Add support for 8-bit bayer and 10- and 12-bit grey formats at the CCDC
input. Y12 is truncated to Y10 at the CCDC output.

Signed-off-by: Michael Jones 
Acked-by: Laurent Pinchart 
---
 drivers/media/video/omap3isp/ispccdc.c  |6 ++
 drivers/media/video/omap3isp/ispvideo.c |   12 
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispccdc.c 
b/drivers/media/video/omap3isp/ispccdc.c
index fd811ea..68941ed 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -43,6 +43,12 @@ __ccdc_get_format(struct isp_ccdc_device *ccdc, struct 
v4l2_subdev_fh *fh,
 
 static const unsigned int ccdc_fmts[] = {
V4L2_MBUS_FMT_Y8_1X8,
+   V4L2_MBUS_FMT_Y10_1X10,
+   V4L2_MBUS_FMT_Y12_1X12,
+   V4L2_MBUS_FMT_SGRBG8_1X8,
+   V4L2_MBUS_FMT_SRGGB8_1X8,
+   V4L2_MBUS_FMT_SBGGR8_1X8,
+   V4L2_MBUS_FMT_SGBRG8_1X8,
V4L2_MBUS_FMT_SGRBG10_1X10,
V4L2_MBUS_FMT_SRGGB10_1X10,
V4L2_MBUS_FMT_SBGGR10_1X10,
diff --git a/drivers/media/video/omap3isp/ispvideo.c 
b/drivers/media/video/omap3isp/ispvideo.c
index a0bb5db..9ade735 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -48,6 +48,18 @@
 static struct isp_format_info formats[] = {
{ V4L2_MBUS_FMT_Y8_1X8, V4L2_MBUS_FMT_Y8_1X8,
  V4L2_MBUS_FMT_Y8_1X8, V4L2_PIX_FMT_GREY, 8, },
+   { V4L2_MBUS_FMT_Y10_1X10, V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y10_1X10, V4L2_PIX_FMT_Y10, 10, },
+   { V4L2_MBUS_FMT_Y12_1X12, V4L2_MBUS_FMT_Y10_1X10,
+ V4L2_MBUS_FMT_Y12_1X12, V4L2_PIX_FMT_Y12, 12, },
+   { V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_MBUS_FMT_SBGGR8_1X8,
+ V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_PIX_FMT_SBGGR8, 8, },
+   { V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_MBUS_FMT_SGBRG8_1X8,
+ V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_PIX_FMT_SGBRG8, 8, },
+   { V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_MBUS_FMT_SGRBG8_1X8,
+ V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_PIX_FMT_SGRBG8, 8, },
+   { V4L2_MBUS_FMT_SRGGB8_1X8, V4L2_MBUS_FMT_SRGGB8_1X8,
+ V4L2_MBUS_FMT_SRGGB8_1X8, V4L2_PIX_FMT_SRGGB8, 8, },
{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
  V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_PIX_FMT_SGRBG10DPCM8, 8, },
{ V4L2_MBUS_FMT_SBGGR10_1X10, V4L2_MBUS_FMT_SBGGR10_1X10,
-- 
1.7.3.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 07/14] omap3isp: stat: update struct ispstat_generic_config's comments

2011-04-05 Thread Laurent Pinchart
From: David Cohen 

struct ispstat_generic_config's comments refers to isph3a_aewb_config,
isph3a_af_config and isphist_config. But those structs have had their
names prefixed with 'omap3'. So, let's update the comments.

Signed-off-by: David Cohen 
---
 drivers/media/video/omap3isp/ispstat.h |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispstat.h 
b/drivers/media/video/omap3isp/ispstat.h
index 820950c..d86da94 100644
--- a/drivers/media/video/omap3isp/ispstat.h
+++ b/drivers/media/video/omap3isp/ispstat.h
@@ -131,9 +131,9 @@ struct ispstat {
 struct ispstat_generic_config {
/*
 * Fields must be in the same order as in:
-*  - isph3a_aewb_config
-*  - isph3a_af_config
-*  - isphist_config
+*  - omap3isp_h3a_aewb_config
+*  - omap3isp_h3a_af_config
+*  - omap3isp_hist_config
 */
u32 buf_size;
u16 config_counter;
-- 
1.7.3.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 08/14] omap3isp: isp: Reset the ISP when the pipeline can't be stopped

2011-04-05 Thread Laurent Pinchart
When a failure to stop a module in the pipeline is detected, the only
way to recover is to reset the ISP. However, as other users can be using
a different pipeline with other modules, the ISP can't be reset
synchronously with the error detection.

Mark the ISP as needing a reset when a failure to stop a pipeline is
detected, and reset the ISP when the last user releases the last
reference to the ISP.

Modify the omap3isp_pipeline_set_stream() function to record the new ISP
pipeline state only when no error occurs, except when stopping the
pipeline in which case the pipeline is still marked as stopped.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/video/omap3isp/isp.c |   14 --
 drivers/media/video/omap3isp/isp.h |1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/omap3isp/isp.c 
b/drivers/media/video/omap3isp/isp.c
index 7d68b10..f380f09 100644
--- a/drivers/media/video/omap3isp/isp.c
+++ b/drivers/media/video/omap3isp/isp.c
@@ -872,6 +872,9 @@ static int isp_pipeline_disable(struct isp_pipeline *pipe)
}
}
 
+   if (failure < 0)
+   isp->needs_reset = true;
+
return failure;
 }
 
@@ -884,7 +887,8 @@ static int isp_pipeline_disable(struct isp_pipeline *pipe)
  * single-shot or continuous mode.
  *
  * Return 0 if successful, or the return value of the failed video::s_stream
- * operation otherwise.
+ * operation otherwise. The pipeline state is not updated when the operation
+ * fails, except when stopping the pipeline.
  */
 int omap3isp_pipeline_set_stream(struct isp_pipeline *pipe,
 enum isp_pipeline_stream_state state)
@@ -895,7 +899,9 @@ int omap3isp_pipeline_set_stream(struct isp_pipeline *pipe,
ret = isp_pipeline_disable(pipe);
else
ret = isp_pipeline_enable(pipe, state);
-   pipe->stream_state = state;
+
+   if (ret == 0 || state == ISP_PIPELINE_STREAM_STOPPED)
+   pipe->stream_state = state;
 
return ret;
 }
@@ -1481,6 +1487,10 @@ void omap3isp_put(struct isp_device *isp)
if (--isp->ref_count == 0) {
isp_disable_interrupts(isp);
isp_save_ctx(isp);
+   if (isp->needs_reset) {
+   isp_reset(isp);
+   isp->needs_reset = false;
+   }
isp_disable_clocks(isp);
}
mutex_unlock(&isp->isp_mutex);
diff --git a/drivers/media/video/omap3isp/isp.h 
b/drivers/media/video/omap3isp/isp.h
index cf5214e..5f87645 100644
--- a/drivers/media/video/omap3isp/isp.h
+++ b/drivers/media/video/omap3isp/isp.h
@@ -262,6 +262,7 @@ struct isp_device {
/* ISP Obj */
spinlock_t stat_lock;   /* common lock for statistic drivers */
struct mutex isp_mutex; /* For handling ref_count field */
+   bool needs_reset;
int has_context;
int ref_count;
unsigned int autoidle;
-- 
1.7.3.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 09/14] omap3isp: Use isp xclk defines

2011-04-05 Thread Laurent Pinchart
From: Stanimir Varbanov 

Use isp defines for isp xclk selection in isp_set_xclk().

Signed-off-by: Stanimir Varbanov 
---
 drivers/media/video/omap3isp/isp.c |   12 +++-
 drivers/media/video/omap3isp/isp.h |6 +++---
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/omap3isp/isp.c 
b/drivers/media/video/omap3isp/isp.c
index f380f09..78240c0 100644
--- a/drivers/media/video/omap3isp/isp.c
+++ b/drivers/media/video/omap3isp/isp.c
@@ -215,20 +215,21 @@ static u32 isp_set_xclk(struct isp_device *isp, u32 xclk, 
u8 xclksel)
}
 
switch (xclksel) {
-   case 0:
+   case ISP_XCLK_A:
isp_reg_clr_set(isp, OMAP3_ISP_IOMEM_MAIN, ISP_TCTRL_CTRL,
ISPTCTRL_CTRL_DIVA_MASK,
divisor << ISPTCTRL_CTRL_DIVA_SHIFT);
dev_dbg(isp->dev, "isp_set_xclk(): cam_xclka set to %d Hz\n",
currentxclk);
break;
-   case 1:
+   case ISP_XCLK_B:
isp_reg_clr_set(isp, OMAP3_ISP_IOMEM_MAIN, ISP_TCTRL_CTRL,
ISPTCTRL_CTRL_DIVB_MASK,
divisor << ISPTCTRL_CTRL_DIVB_SHIFT);
dev_dbg(isp->dev, "isp_set_xclk(): cam_xclkb set to %d Hz\n",
currentxclk);
break;
+   case ISP_XCLK_NONE:
default:
omap3isp_put(isp);
dev_dbg(isp->dev, "ISP_ERR: isp_set_xclk(): Invalid requested "
@@ -237,13 +238,13 @@ static u32 isp_set_xclk(struct isp_device *isp, u32 xclk, 
u8 xclksel)
}
 
/* Do we go from stable whatever to clock? */
-   if (divisor >= 2 && isp->xclk_divisor[xclksel] < 2)
+   if (divisor >= 2 && isp->xclk_divisor[xclksel - 1] < 2)
omap3isp_get(isp);
/* Stopping the clock. */
-   else if (divisor < 2 && isp->xclk_divisor[xclksel] >= 2)
+   else if (divisor < 2 && isp->xclk_divisor[xclksel - 1] >= 2)
omap3isp_put(isp);
 
-   isp->xclk_divisor[xclksel] = divisor;
+   isp->xclk_divisor[xclksel - 1] = divisor;
 
omap3isp_put(isp);
 
diff --git a/drivers/media/video/omap3isp/isp.h 
b/drivers/media/video/omap3isp/isp.h
index 5f87645..00075c6 100644
--- a/drivers/media/video/omap3isp/isp.h
+++ b/drivers/media/video/omap3isp/isp.h
@@ -314,9 +314,9 @@ void omap3isp_configure_bridge(struct isp_device *isp,
   enum ccdc_input_entity input,
   const struct isp_parallel_platform_data *pdata);
 
-#define ISP_XCLK_NONE  -1
-#define ISP_XCLK_A 0
-#define ISP_XCLK_B 1
+#define ISP_XCLK_NONE  0
+#define ISP_XCLK_A 1
+#define ISP_XCLK_B 2
 
 struct isp_device *omap3isp_get(struct isp_device *isp);
 void omap3isp_put(struct isp_device *isp);
-- 
1.7.3.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 04/14] omap3isp: resizer: Improved resizer rsz factor formula

2011-04-05 Thread Laurent Pinchart
From: Sakari Ailus 

Round properly the rsz factor so that we get highest rsz so that the input
width (or height) is highest possible smaller or equal to what the user
asks.

Signed-off-by: Sakari Ailus 
Signed-off-by: Laurent Pinchart 
---
 drivers/media/video/omap3isp/ispresizer.c |   40 +---
 1 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispresizer.c 
b/drivers/media/video/omap3isp/ispresizer.c
index 40b2db8..70897ce 100644
--- a/drivers/media/video/omap3isp/ispresizer.c
+++ b/drivers/media/video/omap3isp/ispresizer.c
@@ -714,16 +714,36 @@ static void resizer_print_status(struct isp_res_device 
*res)
  * iw and ih are the input width and height after cropping. Those equations 
need
  * to be satisfied exactly for the resizer to work correctly.
  *
- * Reverting the equations, we can compute the resizing ratios with
+ * The equations can't be easily reverted, as the >> 8 operation is not linear.
+ * In addition, not all input sizes can be achieved for a given output size. To
+ * get the highest input size lower than or equal to the requested input size,
+ * we need to compute the highest resizing ratio that satisfies the following
+ * inequality (taking the 4-tap mode width equation as an example)
+ *
+ * iw >= (32 * sph + (ow - 1) * hrsz + 16) >> 8 - 7
+ *
+ * (where iw is the requested input width) which can be rewritten as
+ *
+ *   iw - 7>= (32 * sph + (ow - 1) * hrsz + 16) >> 8
+ *  (iw - 7) << 8  >=  32 * sph + (ow - 1) * hrsz + 16 - b
+ * ((iw - 7) << 8) + b >=  32 * sph + (ow - 1) * hrsz + 16
+ *
+ * where b is the value of the 8 least significant bits of the right hand side
+ * expression of the last inequality. The highest resizing ratio value will be
+ * achieved when b is equal to its maximum value of 255. That resizing ratio
+ * value will still satisfy the original inequality, as b will disappear when
+ * the expression will be shifted right by 8.
+ *
+ * The reverted the equations thus become
  *
  * - 8-phase, 4-tap mode
- * hrsz = ((iw - 7) * 256 - 16 - 32 * sph) / (ow - 1)
- * vrsz = ((ih - 4) * 256 - 16 - 32 * spv) / (oh - 1)
+ * hrsz = ((iw - 7) * 256 + 255 - 16 - 32 * sph) / (ow - 1)
+ * vrsz = ((ih - 4) * 256 + 255 - 16 - 32 * spv) / (oh - 1)
  * - 4-phase, 7-tap mode
- * hrsz = ((iw - 7) * 256 - 32 - 64 * sph) / (ow - 1)
- * vrsz = ((ih - 7) * 256 - 32 - 64 * spv) / (oh - 1)
+ * hrsz = ((iw - 7) * 256 + 255 - 32 - 64 * sph) / (ow - 1)
+ * vrsz = ((ih - 7) * 256 + 255 - 32 - 64 * spv) / (oh - 1)
  *
- * The ratios are integer values, and must be rounded down to ensure that the
+ * The ratios are integer values, and are rounded down to ensure that the
  * cropped input size is not bigger than the uncropped input size.
  *
  * As the number of phases/taps, used to select the correct equations to 
compute
@@ -799,10 +819,10 @@ static void resizer_calc_ratios(struct isp_res_device 
*res,
max_height = min_t(unsigned int, max_height, MAX_OUT_HEIGHT);
output->height = clamp(output->height, min_height, max_height);
 
-   ratio->vert = ((input->height - 4) * 256 - 16 - 32 * spv)
+   ratio->vert = ((input->height - 4) * 256 + 255 - 16 - 32 * spv)
/ (output->height - 1);
if (ratio->vert > MID_RESIZE_VALUE)
-   ratio->vert = ((input->height - 7) * 256 - 32 - 64 * spv)
+   ratio->vert = ((input->height - 7) * 256 + 255 - 32 - 64 * spv)
/ (output->height - 1);
ratio->vert = clamp_t(unsigned int, ratio->vert,
  MIN_RESIZE_VALUE, MAX_RESIZE_VALUE);
@@ -870,10 +890,10 @@ static void resizer_calc_ratios(struct isp_res_device 
*res,
  max_width & ~(width_alignment - 1));
output->width = ALIGN(output->width, width_alignment);
 
-   ratio->horz = ((input->width - 7) * 256 - 16 - 32 * sph)
+   ratio->horz = ((input->width - 7) * 256 + 255 - 16 - 32 * sph)
/ (output->width - 1);
if (ratio->horz > MID_RESIZE_VALUE)
-   ratio->horz = ((input->width - 7) * 256 - 32 - 64 * sph)
+   ratio->horz = ((input->width - 7) * 256 + 255 - 32 - 64 * sph)
/ (output->width - 1);
ratio->horz = clamp_t(unsigned int, ratio->horz,
  MIN_RESIZE_VALUE, MAX_RESIZE_VALUE);
-- 
1.7.3.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 03/14] omap3isp: resizer: Use 4-tap mode equations when the ratio is <= 512

2011-04-05 Thread Laurent Pinchart
As the number of phases/taps, used to select the correct equations to
compute the ratio, depends on the ratio, start with the 7-tap mode
equations to compute an approximation of the ratio, and switch to the
4-tap mode equations if the approximation is lower than or equal to 512.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/video/omap3isp/ispresizer.c |   27 ++-
 1 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispresizer.c 
b/drivers/media/video/omap3isp/ispresizer.c
index 829d7bf..40b2db8 100644
--- a/drivers/media/video/omap3isp/ispresizer.c
+++ b/drivers/media/video/omap3isp/ispresizer.c
@@ -724,9 +724,20 @@ static void resizer_print_status(struct isp_res_device 
*res)
  * vrsz = ((ih - 7) * 256 - 32 - 64 * spv) / (oh - 1)
  *
  * The ratios are integer values, and must be rounded down to ensure that the
- * cropped input size is not bigger than the uncropped input size. As the ratio
- * in 7-tap mode is always smaller than the ratio in 4-tap mode, we can use the
- * 7-tap mode equations to compute a ratio approximation.
+ * cropped input size is not bigger than the uncropped input size.
+ *
+ * As the number of phases/taps, used to select the correct equations to 
compute
+ * the ratio, depends on the ratio, we start with the 4-tap mode equations to
+ * compute an approximation of the ratio, and switch to the 7-tap mode 
equations
+ * if the approximation is higher than the ratio threshold.
+ *
+ * As the 7-tap mode equations will return a ratio smaller than or equal to the
+ * 4-tap mode equations, the resulting ratio could become lower than or equal 
to
+ * the ratio threshold. This 'equations loop' isn't an issue as long as the
+ * correct equations are used to compute the final input size. Starting with 
the
+ * 4-tap mode equations ensure that, in case of values resulting in a 'ratio
+ * loop', the smallest of the ratio values will be used, never exceeding the
+ * requested input size.
  *
  * We first clamp the output size according to the hardware capabilitie to 
avoid
  * auto-cropping the input more than required to satisfy the TRM equations. The
@@ -788,8 +799,11 @@ static void resizer_calc_ratios(struct isp_res_device *res,
max_height = min_t(unsigned int, max_height, MAX_OUT_HEIGHT);
output->height = clamp(output->height, min_height, max_height);
 
-   ratio->vert = ((input->height - 7) * 256 - 32 - 64 * spv)
+   ratio->vert = ((input->height - 4) * 256 - 16 - 32 * spv)
/ (output->height - 1);
+   if (ratio->vert > MID_RESIZE_VALUE)
+   ratio->vert = ((input->height - 7) * 256 - 32 - 64 * spv)
+   / (output->height - 1);
ratio->vert = clamp_t(unsigned int, ratio->vert,
  MIN_RESIZE_VALUE, MAX_RESIZE_VALUE);
 
@@ -856,8 +870,11 @@ static void resizer_calc_ratios(struct isp_res_device *res,
  max_width & ~(width_alignment - 1));
output->width = ALIGN(output->width, width_alignment);
 
-   ratio->horz = ((input->width - 7) * 256 - 32 - 64 * sph)
+   ratio->horz = ((input->width - 7) * 256 - 16 - 32 * sph)
/ (output->width - 1);
+   if (ratio->horz > MID_RESIZE_VALUE)
+   ratio->horz = ((input->width - 7) * 256 - 32 - 64 * sph)
+   / (output->width - 1);
ratio->horz = clamp_t(unsigned int, ratio->horz,
  MIN_RESIZE_VALUE, MAX_RESIZE_VALUE);
 
-- 
1.7.3.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 05/14] omap3isp: Fix trivial typos

2011-04-05 Thread Laurent Pinchart
From: Michael Jones 

It doesn't get more trivial than these.

Signed-off-by: Michael Jones 
---
 drivers/media/video/omap3isp/isp.c|4 ++--
 drivers/media/video/omap3isp/ispccdc.c|4 ++--
 drivers/media/video/omap3isp/isppreview.c |2 +-
 drivers/media/video/omap3isp/ispqueue.c   |4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/video/omap3isp/isp.c 
b/drivers/media/video/omap3isp/isp.c
index 1a9963bd..7d68b10 100644
--- a/drivers/media/video/omap3isp/isp.c
+++ b/drivers/media/video/omap3isp/isp.c
@@ -715,7 +715,7 @@ static int isp_pipeline_link_notify(struct media_pad 
*source,
  * Walk the entities chain starting at the pipeline output video node and start
  * all modules in the chain in the given mode.
  *
- * Return 0 if successfull, or the return value of the failed video::s_stream
+ * Return 0 if successful, or the return value of the failed video::s_stream
  * operation otherwise.
  */
 static int isp_pipeline_enable(struct isp_pipeline *pipe,
@@ -883,7 +883,7 @@ static int isp_pipeline_disable(struct isp_pipeline *pipe)
  * Set the pipeline to the given stream state. Pipelines can be started in
  * single-shot or continuous mode.
  *
- * Return 0 if successfull, or the return value of the failed video::s_stream
+ * Return 0 if successful, or the return value of the failed video::s_stream
  * operation otherwise.
  */
 int omap3isp_pipeline_set_stream(struct isp_pipeline *pipe,
diff --git a/drivers/media/video/omap3isp/ispccdc.c 
b/drivers/media/video/omap3isp/ispccdc.c
index 5ff9d14..fd811ea 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -1338,7 +1338,7 @@ static int ccdc_sbl_wait_idle(struct isp_ccdc_device 
*ccdc,
  * @ccdc: Pointer to ISP CCDC device.
  * @event: Pointing which event trigger handler
  *
- * Return 1 when the event and stopping request combination is satisfyied,
+ * Return 1 when the event and stopping request combination is satisfied,
  * zero otherwise.
  */
 static int __ccdc_handle_stopping(struct isp_ccdc_device *ccdc, u32 event)
@@ -1618,7 +1618,7 @@ static int ccdc_video_queue(struct isp_video *video, 
struct isp_buffer *buffer)
 
ccdc_set_outaddr(ccdc, buffer->isp_addr);
 
-   /* We now have a buffer queued on the output, restart the pipeline in
+   /* We now have a buffer queued on the output, restart the pipeline
 * on the next CCDC interrupt if running in continuous mode (or when
 * starting the stream).
 */
diff --git a/drivers/media/video/omap3isp/isppreview.c 
b/drivers/media/video/omap3isp/isppreview.c
index baf9374..6af0f23 100644
--- a/drivers/media/video/omap3isp/isppreview.c
+++ b/drivers/media/video/omap3isp/isppreview.c
@@ -755,7 +755,7 @@ static struct preview_update update_attrs[] = {
  * @configs - pointer to update config structure.
  * @config - return pointer to appropriate structure field.
  * @bit - for which feature to return pointers.
- * Return size of coresponding prev_params member
+ * Return size of corresponding prev_params member
  */
 static u32
 __preview_get_ptrs(struct prev_params *params, void **param,
diff --git a/drivers/media/video/omap3isp/ispqueue.c 
b/drivers/media/video/omap3isp/ispqueue.c
index 8fddc58..c91e56a 100644
--- a/drivers/media/video/omap3isp/ispqueue.c
+++ b/drivers/media/video/omap3isp/ispqueue.c
@@ -408,8 +408,8 @@ done:
  * isp_video_buffer_prepare_vm_flags - Get VMA flags for a userspace address
  *
  * This function locates the VMAs for the buffer's userspace address and checks
- * that their flags match. The onlflag that we need to care for at the moment 
is
- * VM_PFNMAP.
+ * that their flags match. The only flag that we need to care for at the moment
+ * is VM_PFNMAP.
  *
  * The buffer vm_flags field is set to the first VMA flags.
  *
-- 
1.7.3.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 06/14] media: Properly handle link flags in link setup, link notify callback

2011-04-05 Thread Laurent Pinchart
The link flags were not properly handled in __media_entity_setup_link
which could lead to link_notify callback to be called when the flags are
not modified. Fix this.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/media-entity.c |8 ++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 23640ed..056138f 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -378,7 +378,6 @@ EXPORT_SYMBOL_GPL(media_entity_create_link);
 
 static int __media_entity_setup_link_notify(struct media_link *link, u32 flags)
 {
-   const u32 mask = MEDIA_LNK_FL_ENABLED;
int ret;
 
/* Notify both entities. */
@@ -395,7 +394,7 @@ static int __media_entity_setup_link_notify(struct 
media_link *link, u32 flags)
return ret;
}
 
-   link->flags = (link->flags & ~mask) | (flags & mask);
+   link->flags = flags;
link->reverse->flags = link->flags;
 
return 0;
@@ -417,6 +416,7 @@ static int __media_entity_setup_link_notify(struct 
media_link *link, u32 flags)
  */
 int __media_entity_setup_link(struct media_link *link, u32 flags)
 {
+   const u32 mask = MEDIA_LNK_FL_ENABLED;
struct media_device *mdev;
struct media_entity *source, *sink;
int ret = -EBUSY;
@@ -424,6 +424,10 @@ int __media_entity_setup_link(struct media_link *link, u32 
flags)
if (link == NULL)
return -EINVAL;
 
+   /* The non-modifiable link flags must not be modified. */
+   if ((link->flags & ~mask) != (flags & ~mask))
+   return -EINVAL;
+
if (link->flags & MEDIA_LNK_FL_IMMUTABLE)
return link->flags == flags ? 0 : -EINVAL;
 
-- 
1.7.3.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 02/14] omap3isp: resizer: Center the crop rectangle

2011-04-05 Thread Laurent Pinchart
When the crop rectangle needs to be modified to match hardware
requirements, center the resulting rectangle on the requested rectangle
instead of removing pixels from the right and bottom sides only.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/video/omap3isp/ispresizer.c |   16 
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispresizer.c 
b/drivers/media/video/omap3isp/ispresizer.c
index 75d39b1..829d7bf 100644
--- a/drivers/media/video/omap3isp/ispresizer.c
+++ b/drivers/media/video/omap3isp/ispresizer.c
@@ -775,6 +775,8 @@ static void resizer_calc_ratios(struct isp_res_device *res,
unsigned int max_width;
unsigned int max_height;
unsigned int width_alignment;
+   unsigned int width;
+   unsigned int height;
 
/*
 * Clamp the output height based on the hardware capabilities and
@@ -794,11 +796,11 @@ static void resizer_calc_ratios(struct isp_res_device 
*res,
if (ratio->vert <= MID_RESIZE_VALUE) {
upscaled_height = (output->height - 1) * ratio->vert
+ 32 * spv + 16;
-   input->height = (upscaled_height >> 8) + 4;
+   height = (upscaled_height >> 8) + 4;
} else {
upscaled_height = (output->height - 1) * ratio->vert
+ 64 * spv + 32;
-   input->height = (upscaled_height >> 8) + 7;
+   height = (upscaled_height >> 8) + 7;
}
 
/*
@@ -862,12 +864,18 @@ static void resizer_calc_ratios(struct isp_res_device 
*res,
if (ratio->horz <= MID_RESIZE_VALUE) {
upscaled_width = (output->width - 1) * ratio->horz
   + 32 * sph + 16;
-   input->width = (upscaled_width >> 8) + 7;
+   width = (upscaled_width >> 8) + 7;
} else {
upscaled_width = (output->width - 1) * ratio->horz
   + 64 * sph + 32;
-   input->width = (upscaled_width >> 8) + 7;
+   width = (upscaled_width >> 8) + 7;
}
+
+   /* Center the new crop rectangle. */
+   input->left += (input->width - width) / 2;
+   input->top += (input->height - height) / 2;
+   input->width = width;
+   input->height = height;
 }
 
 /*
-- 
1.7.3.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 01/14] media: Use correct ioctl name in MEDIA_IOC_SETUP_LINK documentation

2011-04-05 Thread Laurent Pinchart
The documentation incorrectly refers to MEDIA_IOC_ENUM_LINKS, fix it.

Signed-off-by: Laurent Pinchart 
---
 Documentation/DocBook/v4l/media-ioc-setup-link.xml |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/DocBook/v4l/media-ioc-setup-link.xml 
b/Documentation/DocBook/v4l/media-ioc-setup-link.xml
index 2331e76..cec97af 100644
--- a/Documentation/DocBook/v4l/media-ioc-setup-link.xml
+++ b/Documentation/DocBook/v4l/media-ioc-setup-link.xml
@@ -34,7 +34,7 @@
   
request

- MEDIA_IOC_ENUM_LINKS
+ MEDIA_IOC_SETUP_LINK

   
   
-- 
1.7.3.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 00/14] OMAP3 ISP and media controller patches for 2.6.39

2011-04-05 Thread Laurent Pinchart
Hi everybody,

Here's a set of OMAP3 ISP patches for 2.6.39, including two media controller
patches and a V4L2 core patch.

Most of those patches fix code and documentation. Patches 10/14 to 13/14
implement lane shifter support in the OMAP3 ISP driver. That's a new feature,
but there's no risk of regression from 2.6.38 as the OMAP3 ISP driver has been
merged into 2.6.39-r1.

I'll send a pull request at the end of the week if there's no comment on the
patches.

David Cohen (1):
  omap3isp: stat: update struct ispstat_generic_config's comments

Laurent Pinchart (6):
  media: Use correct ioctl name in MEDIA_IOC_SETUP_LINK documentation
  omap3isp: resizer: Center the crop rectangle
  omap3isp: resizer: Use 4-tap mode equations when the ratio is <= 512
  media: Properly handle link flags in link setup, link notify callback
  omap3isp: isp: Reset the ISP when the pipeline can't be stopped
  omap3isp: Don't increment node entity use count when poweron fails

Michael Jones (5):
  omap3isp: Fix trivial typos
  v4l: add V4L2_PIX_FMT_Y12 format
  media: add missing 8-bit bayer formats and Y12
  omap3isp: ccdc: support Y10/12, 8-bit bayer fmts
  omap3isp: lane shifter support

Sakari Ailus (1):
  omap3isp: resizer: Improved resizer rsz factor formula

Stanimir Varbanov (1):
  omap3isp: Use isp xclk defines

 Documentation/DocBook/media-entities.tmpl  |1 +
 Documentation/DocBook/v4l/media-ioc-setup-link.xml |2 +-
 Documentation/DocBook/v4l/pixfmt-y12.xml   |   79 ++
 Documentation/DocBook/v4l/pixfmt.xml   |1 +
 Documentation/DocBook/v4l/subdev-formats.xml   |   59 +++
 drivers/media/media-entity.c   |8 +-
 drivers/media/video/omap3isp/isp.c |   39 +--
 drivers/media/video/omap3isp/isp.h |   12 +-
 drivers/media/video/omap3isp/ispccdc.c |   37 ++--
 drivers/media/video/omap3isp/isppreview.c  |2 +-
 drivers/media/video/omap3isp/ispqueue.c|4 +-
 drivers/media/video/omap3isp/ispresizer.c  |   75 +++---
 drivers/media/video/omap3isp/ispstat.h |6 +-
 drivers/media/video/omap3isp/ispvideo.c|  108 +---
 drivers/media/video/omap3isp/ispvideo.h|3 +
 include/linux/v4l2-mediabus.h  |7 +-
 include/linux/videodev2.h  |1 +
 17 files changed, 380 insertions(+), 64 deletions(-)
 create mode 100644 Documentation/DocBook/v4l/pixfmt-y12.xml

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 04/12] mm: alloc_contig_freed_pages() added

2011-04-05 Thread Marek Szyprowski
Hello,

On Monday, April 04, 2011 3:15 PM Michał Nazarewicz wrote:

> > What kind of success have you had running this in practice?  I'd be
> > worried that some silly task or a sticky dentry would end up in the
> > range that you want to allocate in.
> 
> I'm not sure what you are asking.
> 
> The function requires the range to be marked as MIGRATE_ISOLATE and all
> pages being free, so nothing can be allocated there while the function
> is running.
> 
> If you are asking about CMA in general, the range that CMA uses is marked
> as MIGRATE_CMA (a new migrate type) which means that only MIGRATE_MOVABLE
> pages can be allocated there.  This means, that in theory, if there is
> enough memory the pages can always be moved out of the region.  At leasts
> that's my understanding of the type.  If this is correct, the allocation
> should always succeed provided enough memory for the pages within the
> region to be moved to is available.
> 
> As of practice, I have run some simple test to see if the code works and
> they succeeded.  Also, Marek has run some test with actual hardware and
> those worked well as well (but I'll let Marek talk about any details).

We did the tests with real multimedia drivers - video codec and video
converter (s5p-mfc and s5p-fimc). These drivers allocate large contiguous 
buffers for video data. The allocation is performed when driver is opened
by user space application. 

First we consumed system memory by running a set of simple applications
that just did some malloc() and filled memory with random pattern to consume
free pages. Then some of that memory has been freed and we ran the video
decoding application. Multimedia drivers successfully managed to allocate
required contiguous buffers from MIGRATE_CMA ranges.

The tests have been performed with different system usage patterns (malloc(),
heavy filesystem load, anonymous memory mapping). In all these cases CMA
worked surprisingly good allowing the drivers to allocate the required 
contiguous buffers.

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