RE: [RFC] New controls for codec devices
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
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
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
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
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
> -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
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
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
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
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
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
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
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
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
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
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/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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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