Re: Multiple Mantis devices gives me glitches
Hello, I have three Cinergy C (DVB-C cards) like this: 05:04.0 Multimedia controller: Twinhan Technology Co. Ltd Mantis DTV PCI Bridge Controller [Ver 1.0] (rev 01) Subsystem: TERRATEC Electronic GmbH Device 1178 Flags: bus master, medium devsel, latency 128, IRQ 20 Memory at fdcfe000 (32-bit, prefetchable) [size=4K] Kernel driver in use: Mantis Kernel modules: mantis Kernel: 2.6.38-13-generic-pae (Ubuntu Natty stock) Motherboard: P43-ES3G CPU: Intel(R) Core(TM)2 Quad CPUQ8400 At some point i started having glitches (I would from time to time get an 'old' frame displayed and sometimes audio noise when this happened). I tried pretty much every trick I could find: * CPU affinity * Dedicated IRQ for each card (only shared with USB, which has no units attached) * Various process priorities (also for the kdvb-processes) * pci latency (from 0x20 to 0xff) I have quite decent results when I only have 2 DVB cards present, and the results became even better when running the irqbalancer-dæmon as well. The glitches are not completely gone, but much more manageble now. So the problem seems to be caused by too many interrupts for my system to handle, however this is where I am in over my head. I know 2.6.38 isn't the freshest brew, but I could not find any changes to the driver since then that seemed relevant (which could just be my lack of source-fu). So, any ideas on how to improve the performance? I am suffering from some hardware incompatibility or is the driver this resource hungry? Hi, I noticed some SMP problems with the mantis driver as well (see my post "Mantis CAM not SMP safe / Activating CAM on Technisat Skystar HD2 (DVB-S2)"). One workaround for me is to limit the CPU to one core (to be sure disable the hyperthreading cores as well). That can be done via BIOS *or* adding maxcpus=1 as kernel parameter *or* you can disable the cores one by one via "|echo 0 > /sys/devices/system/cpu/cpuX/online|" where X is the core to disable. Since you need to be root for this, I did "sudo su" first. But of course our problems might be completely unrelated and limiting to one core won't change a thing ;) Manuel -- 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: Multiple Mantis devices gives me glitches
Hi
Here is a patch that went into Linus GIT this year.
It reduces the number of DMA transfer interrupts into one third.
Linus released 2.6.38.8 doesn't seem to have this patch yet.
I had a single Mantis card and a single CPU.
I used VDR to deliver HDTV channel via network
into a faster computer for viewing.
One known latency point with Mantis is I2C transfer.
With my measure, latency is about 0.33ms per I2C transfer byte.
Basic read / write (address+value) minimal latency is thus 0.66ms.
Latency amount depends on I2C bus speed on the card.
I don't have a working patch to fix this yet though.
Regards, Marko Ristola
---
Refactor Mantis DMA transfer to deliver 16Kb TS data per interrupt
https://patchwork.kernel.org/patch/107036/
With VDR streaming HDTV into network, generating an interrupt once per 16kb,
implemented in this patch, seems to support more robust throughput with HDTV.
Fix leaking almost 64kb data from the previous TS after changing the TS.
One effect of the old version was, that the DMA transfer and driver's
DMA buffer access might happen at the same time - a race condition.
Signed-off-by: Marko M. Ristola
Regards,
Marko Ristola
diff --git a/drivers/media/dvb/mantis/hopper_cards.c
b/drivers/media/dvb/mantis/hopper_cards.c
index 09e9fc7..3b7e376 100644
--- a/drivers/media/dvb/mantis/hopper_cards.c
+++ b/drivers/media/dvb/mantis/hopper_cards.c
@@ -126,7 +126,7 @@ static irqreturn_t hopper_irq_handler(int irq, void *dev_id)
}
if (stat & MANTIS_INT_RISCI) {
dprintk(MANTIS_DEBUG, 0, "<%s>", label[8]);
- mantis->finished_block = (stat & MANTIS_INT_RISCSTAT) >> 28;
+ mantis->busy_block = (stat & MANTIS_INT_RISCSTAT) >> 28;
tasklet_schedule(&mantis->tasklet);
}
if (stat & MANTIS_INT_I2CDONE) {
diff --git a/drivers/media/dvb/mantis/mantis_cards.c
b/drivers/media/dvb/mantis/mantis_cards.c
index cf4b39f..8f048d5 100644
--- a/drivers/media/dvb/mantis/mantis_cards.c
+++ b/drivers/media/dvb/mantis/mantis_cards.c
@@ -134,7 +134,7 @@ static irqreturn_t mantis_irq_handler(int irq, void *dev_id)
}
if (stat & MANTIS_INT_RISCI) {
dprintk(MANTIS_DEBUG, 0, "<%s>", label[8]);
- mantis->finished_block = (stat & MANTIS_INT_RISCSTAT) >> 28;
+ mantis->busy_block = (stat & MANTIS_INT_RISCSTAT) >> 28;
tasklet_schedule(&mantis->tasklet);
}
if (stat & MANTIS_INT_I2CDONE) {
diff --git a/drivers/media/dvb/mantis/mantis_common.h
b/drivers/media/dvb/mantis/mantis_common.h
index d0b645a..23b23b7 100644
--- a/drivers/media/dvb/mantis/mantis_common.h
+++ b/drivers/media/dvb/mantis/mantis_common.h
@@ -122,11 +122,8 @@ struct mantis_pci {
unsigned intnum;
/* RISC Core */
- u32 finished_block;
+ u32 busy_block;
u32 last_block;
- u32 line_bytes;
- u32 line_count;
- u32 risc_pos;
u8 *buf_cpu;
dma_addr_t buf_dma;
u32 *risc_cpu;
diff --git a/drivers/media/dvb/mantis/mantis_dma.c
b/drivers/media/dvb/mantis/mantis_dma.c
index 46202a4..c61ca7d 100644
--- a/drivers/media/dvb/mantis/mantis_dma.c
+++ b/drivers/media/dvb/mantis/mantis_dma.c
@@ -43,13 +43,17 @@
#define RISC_IRQ (0x01 << 24)
#define RISC_STATUS(status) ~status) & 0x0f) << 20) | ((status & 0x0f) << 16))
-#define RISC_FLUSH() (mantis->risc_pos = 0)
-#define RISC_INSTR(opcode) (mantis->risc_cpu[mantis->risc_pos++] =
cpu_to_le32(opcode))
+#define RISC_FLUSH(risc_pos) (risc_pos = 0)
+#define RISC_INSTR(risc_pos, opcode) (mantis->risc_cpu[risc_pos++] =
cpu_to_le32(opcode))
#define MANTIS_BUF_SIZE (64 * 1024)
-#define MANTIS_BLOCK_BYTES (MANTIS_BUF_SIZE >> 4)
-#define MANTIS_BLOCK_COUNT (1 << 4)
-#define MANTIS_RISC_SIZE PAGE_SIZE
+#define MANTIS_BLOCK_BYTES (MANTIS_BUF_SIZE / 4)
+#define MANTIS_DMA_TR_BYTES (2 * 1024) /* upper limit: 4095 bytes. */
+#define MANTIS_BLOCK_COUNT (MANTIS_BUF_SIZE / MANTIS_BLOCK_BYTES)
+
+#define MANTIS_DMA_TR_UNITS (MANTIS_BLOCK_BYTES / MANTIS_DMA_TR_BYTES)
+/* MANTIS_BUF_SIZE / MANTIS_DMA_TR_UNITS must not exceed MANTIS_RISC_SIZE (4k
RISC cmd buffer) */
+#define MANTIS_RISC_SIZE PAGE_SIZE /* RISC program must fit here. */
int mantis_dma_exit(struct mantis_pci *mantis)
{
@@ -124,27 +128,6 @@ err:
return -ENOMEM;
}
-static inline int mantis_calc_lines(struct mantis_pci *mantis)
-{
- mantis->line_bytes = MANTIS_BLOCK_BYTES;
- mantis->line_count = MANTIS_BLOCK_COUNT;
-
- while (mantis->line_bytes > 4095) {
- mantis->line_bytes >>= 1;
- mantis->line_count <<= 1;
- }
-
- dpr
Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
On Tue, Dec 13, 2011 at 1:59 PM, HeungJun, Kim wrote: > Hi Ming and Sylwester, > > Thanks for the reply. > >> -Original Message- >> From: Ming Lei [mailto:ming@canonical.com] >> Sent: Tuesday, December 13, 2011 1:02 PM >> To: HeungJun, Kim >> Cc: Sylwester Nawrocki; linux-o...@vger.kernel.org; linux-arm- >> ker...@lists.infradead.org; linux-ker...@vger.kernel.org; linux- >> me...@vger.kernel.org >> Subject: Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver >> module >> >> Hi, >> >> On Mon, Dec 12, 2011 at 8:08 PM, HeungJun, Kim > wrote: >> > Hi Ming, >> > >> > It's maybe late, but I want to suggest one thing about FD API. >> > >> > This OMAP FD block looks detection ability of just face. >> > But, It's possible to occur another device which can detect >> > specific "object" or "patterns". Moreover, this API can expand >> > "object recognition" area. So, I think it's good to change the API name >> > like "v4l2_recog". >> >> IMO, object detection is better, at least now OMAP4 and samsung has >> face detection IP module, and face recognition is often done on results >> of face detection and more complicated interfaces will be involved. > Actually, the detection point is different, I guess. > The OMAP has the detection block separately, named FDIF. But, Samsung > Exynos doing detection process with externel sensor - m5mols, in our case. > This sensor(m5mols) has ISP function, and these ISP can process detection. > The expert of FIMC is Sylwester. Probably, he can tell the differences > between both in more details. :) > >> >> > >> > Actually, I'm preparing similar control class for mainline with m5mols >> > camera sensor driver. The m5mols camera sensor has the function about >> > "face detection". But, I has experienced about Robot Recognition, and I >> > remember the image processing chip which can detect spefic "pattern". >> > So, I hesitated naming the API(control or ioctl whatever) with "face". >> > It can be possible to provide just "object" or "pattern", not face. >> > Even user library on windows, there is famous "OpenCV". And this is also >> > support not only "face", but also "object". >> >> Yes, object is better than face, and we can use enum flag to describe that >> the objects detected are which kind of objects. In fact, I plan to rename the >> face detection generic driver as object detection generic driver and let >> hardware driver to handle the object detection details. >> >> > >> > The function of OMAP FDIF looks like m5mols ISP's one. >> > please understand I don't have experience about OMAP AP. But, I can tell >> > you it's better to use the name "object recognition", not the "face >> detection", >> > for any other device or driver. >> > >> > In a few days, I'll share the CIDs I have thought for m5mols driver. >> > And, I hope to discuss about this with OMAP FDIF. >> >> You have been doing it already, :-) > Yeah, actually I did. :) > But, until I see OMAP FDIF case, I did not recognize AP(like OMAP) can > have detection capability. :) So, although I did not think about at that time, > I also think we should re-consider this case for now. > > As I look around your patch quickly, the functions is very similar with ours. > (even detection of left/right eye) > So, the problem is there are two ways to proceed "recognition". > - Does it handle at the level of IOCTLs? or CIDs? The patch introduces two IOCTL to retrieve object detection result. I think it should be handled by IOCTL. The interface is a little complex, so it is not easy to handle it by CIDs, IMO. > > If the detection algorithm is proceeded at the level of HW block, > it's right the platform driver provide APIs as IOCTLs(as you're FDIF patches). > On the other hand, if it is proceeded at the sensor(subdevice) level, > I think it's more right to control using CIDs. Certainly, some generic CIDs for object detection will be introduced later and will be handled in the object detection(the current fdif generic driver, patch 6/7) generic driver. > We need the solution including those two cases. > And the name - object detection? object recognition? I think object detection is better. For example, face detection is very different with face recognition, isn't it? thanks, -- Ming Lei > > So, do you have any good ideas? > > ps. There can be another not-matched HW block level issues. > But, the problem which I can check is just above issue for now. > > > Thanks, > Heungjun Kim > > >> >> thanks, >> -- >> Ming Lei >> >> > >> > Thank you. >> > >> > Regards, >> > Heungjun Kim >> > >> > >> >> -Original Message- >> >> From: linux-media-ow...@vger.kernel.org [mailto:linux-media- >> >> ow...@vger.kernel.org] On Behalf Of Ming Lei >> >> Sent: Monday, December 12, 2011 6:50 PM >> >> To: Sylwester Nawrocki >> >> Cc: linux-o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > linux- >> >> ker...@vger.kernel.org; linux-media@vger.kernel.org >> >> Subject: Re: [RFC PATCH v1 6/7] media: video: introduce
RE: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
Hi Ming and Sylwester, Thanks for the reply. > -Original Message- > From: Ming Lei [mailto:ming@canonical.com] > Sent: Tuesday, December 13, 2011 1:02 PM > To: HeungJun, Kim > Cc: Sylwester Nawrocki; linux-o...@vger.kernel.org; linux-arm- > ker...@lists.infradead.org; linux-ker...@vger.kernel.org; linux- > me...@vger.kernel.org > Subject: Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver > module > > Hi, > > On Mon, Dec 12, 2011 at 8:08 PM, HeungJun, Kim wrote: > > Hi Ming, > > > > It's maybe late, but I want to suggest one thing about FD API. > > > > This OMAP FD block looks detection ability of just face. > > But, It's possible to occur another device which can detect > > specific "object" or "patterns". Moreover, this API can expand > > "object recognition" area. So, I think it's good to change the API name > > like "v4l2_recog". > > IMO, object detection is better, at least now OMAP4 and samsung has > face detection IP module, and face recognition is often done on results > of face detection and more complicated interfaces will be involved. Actually, the detection point is different, I guess. The OMAP has the detection block separately, named FDIF. But, Samsung Exynos doing detection process with externel sensor - m5mols, in our case. This sensor(m5mols) has ISP function, and these ISP can process detection. The expert of FIMC is Sylwester. Probably, he can tell the differences between both in more details. :) > > > > > Actually, I'm preparing similar control class for mainline with m5mols > > camera sensor driver. The m5mols camera sensor has the function about > > "face detection". But, I has experienced about Robot Recognition, and I > > remember the image processing chip which can detect spefic "pattern". > > So, I hesitated naming the API(control or ioctl whatever) with "face". > > It can be possible to provide just "object" or "pattern", not face. > > Even user library on windows, there is famous "OpenCV". And this is also > > support not only "face", but also "object". > > Yes, object is better than face, and we can use enum flag to describe that > the objects detected are which kind of objects. In fact, I plan to rename the > face detection generic driver as object detection generic driver and let > hardware driver to handle the object detection details. > > > > > The function of OMAP FDIF looks like m5mols ISP's one. > > please understand I don't have experience about OMAP AP. But, I can tell > > you it's better to use the name "object recognition", not the "face > detection", > > for any other device or driver. > > > > In a few days, I'll share the CIDs I have thought for m5mols driver. > > And, I hope to discuss about this with OMAP FDIF. > > You have been doing it already, :-) Yeah, actually I did. :) But, until I see OMAP FDIF case, I did not recognize AP(like OMAP) can have detection capability. :) So, although I did not think about at that time, I also think we should re-consider this case for now. As I look around your patch quickly, the functions is very similar with ours. (even detection of left/right eye) So, the problem is there are two ways to proceed "recognition". - Does it handle at the level of IOCTLs? or CIDs? If the detection algorithm is proceeded at the level of HW block, it's right the platform driver provide APIs as IOCTLs(as you're FDIF patches). On the other hand, if it is proceeded at the sensor(subdevice) level, I think it's more right to control using CIDs. We need the solution including those two cases. And the name - object detection? object recognition? So, do you have any good ideas? ps. There can be another not-matched HW block level issues. But, the problem which I can check is just above issue for now. Thanks, Heungjun Kim > > thanks, > -- > Ming Lei > > > > > Thank you. > > > > Regards, > > Heungjun Kim > > > > > >> -Original Message- > >> From: linux-media-ow...@vger.kernel.org [mailto:linux-media- > >> ow...@vger.kernel.org] On Behalf Of Ming Lei > >> Sent: Monday, December 12, 2011 6:50 PM > >> To: Sylwester Nawrocki > >> Cc: linux-o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux- > >> ker...@vger.kernel.org; linux-media@vger.kernel.org > >> Subject: Re: [RFC PATCH v1 6/7] media: video: introduce face detection > driver > >> module > >> > >> Hi, > >> > >> On Mon, Dec 12, 2011 at 1:43 AM, Sylwester Nawrocki > >> > >> >> For OMAP4 FD, it is not needed to include FD into MC framework since a > >> >> intermediate buffer is always required. If your HW doesn't belong to this > >> >> case, what is the output of your HW FD in the link? Also sounds FD > results > >> >> may not be needed at all for use space application in the case. > >> > > >> > The result data is similar to OMAP4 one, plus a few other attributes. > >> > User buffers may be filled by other than FD device driver. > >> > >> OK. > >> > >> > >> >> Could you provide some practical use cases about these? > >> > > >> > As abov
Re: cx231xx kernel oops
Yan Seiner wrote:
Andy Walls wrote:
800 MB for 320x420 frames? It sounds like your app has gooned its
requested buffer size.
That's an understatement. :-)
This might be due to endianess differences between MIPS abd x86 and
your app only being written and tested on x86.
My speculation too. I don't know where that number comes from; the
same app works fine with the saa7115 driver if I switch frame
grabbers. I'll have to do some fiddling with the code to figure out
where the problem lies. It's some interaction between the app and the
cx231xx driver.
HAH!
This simple patch fixes it on my MIPS platform - not tested on other
architectures as I don't have them readily available running a newer kernel:
---
/data10/home/yan/openwrt/backfire/trunk/build_dir/linux-brcm47xx/linux-3.0.3/drivers/media/video/cx231xx/cx231xx-pcb-cfg.c
2011-08-17 10:57:16.0 -0700
+++ cx231xx-pcb-cfg.c2011-12-12 20:16:23.0 -0800
@@ -672,7 +672,9 @@
pcb config it is related to */
cx231xx_read_ctrl_reg(dev, VRT_GET_REGISTER, BOARD_CFG_STAT, data, 4);
-config_info = *((u32 *) data);
+//config_info = *((u32 *) data);
+config_info = *(data);
+cx231xx_info("config_info %x\n",config_info);
usb_speed = (u8) (config_info & 0x1);
/* Verify this device belongs to Bus power or Self power device */
No more errors and the frame grabber works up to 480x320 even on a slow
MIPS board.
[ 33.64] cx231xx v4l2 driver loaded.
[ 33.65] cx231xx #0: New device Hauppauge Hauppauge Device @ 480
Mbps (2040:c200) with 5 interfaces
[ 33.66] cx231xx #0: registering interface 1
[ 33.66] cx231xx #0: config_info c9
[ 33.67] cx231xx #0: can't change interface 3 alt no. to 3: Max.
Pkt size = 0
[ 33.68] cx231xx #0: can't change interface 4 alt no. to 1: Max.
Pkt size = 0
[ 33.69] cx231xx #0: Identified as Hauppauge USB Live 2 (card=9)
[ 33.80] cx231xx #0: cx231xx_dif_set_standard: setStandard to
[ 33.82] cx231xx #0: Changing the i2c master port to 3
[ 33.82] cx25840 0-0044: cx23102 A/V decoder found @ 0x88 (cx231xx #0)
[ 33.85] cx25840 0-0044: Firmware download size changed to 16
bytes max length
[ 35.88] cx25840 0-0044: loaded v4l-cx231xx-avcore-01.fw firmware
(16382 bytes)
[ 35.92] cx231xx #0: cx231xx #0: v4l2 driver version 0.0.1
[ 35.95] cx231xx #0: cx231xx_dif_set_standard: setStandard to
[ 36.00] cx231xx #0: video_mux : 0
[ 36.01] cx231xx #0: do_mode_ctrl_overrides : 0xb000
[ 36.01] cx231xx #0: do_mode_ctrl_overrides NTSC
[ 36.02] cx231xx #0: cx231xx #0/0: registered device video0 [v4l2]
[ 36.03] cx231xx #0: cx231xx #0/0: registered device vbi0
[ 36.04] cx231xx #0: V4L2 device registered as video0 and vbi0
[ 36.04] cx231xx #0: EndPoint Addr 0x8400, Alternate settings: 5
[ 36.05] cx231xx #0: Alternate setting 0, max size= 512
[ 36.05] cx231xx #0: Alternate setting 1, max size= 184
[ 36.06] cx231xx #0: Alternate setting 2, max size= 728
[ 36.07] cx231xx #0: Alternate setting 3, max size= 2892
[ 36.07] cx231xx #0: Alternate setting 4, max size= 1800
[ 36.08] cx231xx #0: EndPoint Addr 0x8500, Alternate settings: 2
[ 36.08] cx231xx #0: Alternate setting 0, max size= 512
[ 36.09] cx231xx #0: Alternate setting 1, max size= 512
[ 36.09] cx231xx #0: EndPoint Addr 0x8600, Alternate settings: 2
[ 36.10] cx231xx #0: Alternate setting 0, max size= 512
[ 36.11] cx231xx #0: Alternate setting 1, max size= 576
[ 36.11] usbcore: registered new interface driver cx231xx
[ 36.32] ar71xx-wdt: enabling watchdog timer
--
Few people are capable of expressing with equanimity opinions which differ from
the prejudices of their social environment. Most people are even incapable of
forming such opinions.
Albert Einstein
--
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 v1 6/7] media: video: introduce face detection driver module
Hi, On Mon, Dec 12, 2011 at 8:08 PM, HeungJun, Kim wrote: > Hi Ming, > > It's maybe late, but I want to suggest one thing about FD API. > > This OMAP FD block looks detection ability of just face. > But, It's possible to occur another device which can detect > specific "object" or "patterns". Moreover, this API can expand > "object recognition" area. So, I think it's good to change the API name > like "v4l2_recog". IMO, object detection is better, at least now OMAP4 and samsung has face detection IP module, and face recognition is often done on results of face detection and more complicated interfaces will be involved. > > Actually, I'm preparing similar control class for mainline with m5mols > camera sensor driver. The m5mols camera sensor has the function about > "face detection". But, I has experienced about Robot Recognition, and I > remember the image processing chip which can detect spefic "pattern". > So, I hesitated naming the API(control or ioctl whatever) with "face". > It can be possible to provide just "object" or "pattern", not face. > Even user library on windows, there is famous "OpenCV". And this is also > support not only "face", but also "object". Yes, object is better than face, and we can use enum flag to describe that the objects detected are which kind of objects. In fact, I plan to rename the face detection generic driver as object detection generic driver and let hardware driver to handle the object detection details. > > The function of OMAP FDIF looks like m5mols ISP's one. > please understand I don't have experience about OMAP AP. But, I can tell > you it's better to use the name "object recognition", not the "face > detection", > for any other device or driver. > > In a few days, I'll share the CIDs I have thought for m5mols driver. > And, I hope to discuss about this with OMAP FDIF. You have been doing it already, :-) thanks, -- Ming Lei > > Thank you. > > Regards, > Heungjun Kim > > >> -Original Message- >> From: linux-media-ow...@vger.kernel.org [mailto:linux-media- >> ow...@vger.kernel.org] On Behalf Of Ming Lei >> Sent: Monday, December 12, 2011 6:50 PM >> To: Sylwester Nawrocki >> Cc: linux-o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux- >> ker...@vger.kernel.org; linux-media@vger.kernel.org >> Subject: Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver >> module >> >> Hi, >> >> On Mon, Dec 12, 2011 at 1:43 AM, Sylwester Nawrocki >> >> >> For OMAP4 FD, it is not needed to include FD into MC framework since a >> >> intermediate buffer is always required. If your HW doesn't belong to this >> >> case, what is the output of your HW FD in the link? Also sounds FD results >> >> may not be needed at all for use space application in the case. >> > >> > The result data is similar to OMAP4 one, plus a few other attributes. >> > User buffers may be filled by other than FD device driver. >> >> OK. >> >> >> >> Could you provide some practical use cases about these? >> > >> > As above, and any device with a camera that controls something and makes >> > decision according to presence of human face in his view. >> >> Sounds a reasonable case, :-) >> >> >> >> If FD result is associated with a frame, how can user space get the frame >> seq >> >> if no v4l2 buffer is involved? Without a frame sequence, it is a bit >> >> difficult to retrieve FD results from user space. >> > >> > If you pass image data in memory buffers from user space, yes, it could be >> > impossible. >> >> It is easy to get the frame sequence from v4l2_buffer for the case too, :-) >> >> > >> > Not really, still v4l2_buffer may be used by other (sub)driver within same >> video >> > processing pipeline. >> >> OK. >> >> A related question: how can we make one application to support the two kinds > of >> devices(input from user space data as OMAP4, input from SoC bus as Samsung) >> at the same time? Maybe some capability info is to be exported to user space? >> or other suggestions? >> >> And will your Samsung FD HW support to detect faces from memory? or just only >> detect from SoC bus? >> >> >> > It will be included in the FD result... or in a dedicated v4l2 event data >> structure. >> > More important, at the end of the day, we'll be getting buffers with image >> data >> > at some stage of a video pipeline, which would contain same frame >> > identifier >> > (I think we can ignore v4l2_buffer.field for FD purpose). >> >> OK, I will associate FD result with frame identifier, and not invent a >> dedicated v4l2 event for query frame seq now until a specific requirement >> for it is proposed. >> >> I will convert/integrate recent discussions into patches of v2 for further >> review, and sub device support will be provided. But before starting to do >> it, >> I am still not clear how to integrate FD into MC framework. I understand FD >> sub device is only a media entity, so how can FD sub device find the media >> device(struct media_device)? or just needn't to care abo
[GIT PULL FOR v3.3] Media, OMAP3 ISP & AS3645A
Hi Mauro, The following changes since commit bcc072756e4467dc30e502a311b1c3adec96a0e4: [media] STV0900: Query DVB frontend delivery capabilities (2011-12-12 15:04:34 -0200) are available in the git repository at: git://linuxtv.org/pinchartl/media.git omap3isp-omap3isp-next Clemens Ladisch (1): media: fix truncated entity specification Dmitry Artamonow (1): omap3isp: video: Fix compilation of ispvideo.c Laurent Pinchart (8): omap3isp: preview: Rename max output sizes defines omap3isp: ccdc: Fix crash in HS/VS interrupt handler omap3isp: Fix crash caused by subdevs now having a pointer to devnodes omap3isp: Clarify the clk_pol field in platform data v4l: Add over-current and indicator flash fault bits as3645a: Add driver for LED flash controller omap3isp: video: Don't WARN() on unknown pixel formats omap3isp: Mark next captured frame as faulty when an SBL overflow occurs Documentation/DocBook/media/v4l/controls.xml | 10 + drivers/media/media-device.c |3 +- drivers/media/video/Kconfig |7 + drivers/media/video/Makefile |1 + drivers/media/video/as3645a.c| 904 + drivers/media/video/omap3isp/isp.c | 53 +- drivers/media/video/omap3isp/ispccdc.c | 12 +- drivers/media/video/omap3isp/ispccdc.h |2 - drivers/media/video/omap3isp/ispccp2.c | 20 +- drivers/media/video/omap3isp/ispccp2.h |3 +- drivers/media/video/omap3isp/ispcsi2.c | 18 +- drivers/media/video/omap3isp/ispcsi2.h |2 +- drivers/media/video/omap3isp/isppreview.c| 25 +- drivers/media/video/omap3isp/isppreview.h|2 - drivers/media/video/omap3isp/ispresizer.c|7 +- drivers/media/video/omap3isp/ispresizer.h|1 - drivers/media/video/omap3isp/ispstat.c |2 +- drivers/media/video/omap3isp/ispvideo.c | 22 +- drivers/media/video/omap3isp/ispvideo.h |8 +- drivers/media/video/v4l2-dev.c |4 +- drivers/media/video/v4l2-device.c|4 +- include/linux/videodev2.h|2 + include/media/as3645a.h | 71 ++ include/media/media-entity.h |2 +- include/media/omap3isp.h |2 +- 25 files changed, 1086 insertions(+), 101 deletions(-) create mode 100644 drivers/media/video/as3645a.c create mode 100644 include/media/as3645a.h -- 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 v4 0/3] fbdev: Add FOURCC-based format configuration API
Hi Florian, On Monday 12 December 2011 19:49:38 Florian Tobias Schandinat wrote: > On 12/12/2011 04:08 PM, Laurent Pinchart wrote: > > On Tuesday 29 November 2011 11:26:56 Laurent Pinchart wrote: > >> Hi everybody, > >> > >> Here's the fourth version of the fbdev FOURCC-based format configuration > >> API. > > > > Is there a chance this will make it to v3.3 ? > > Yes, that's likely. I thought you wanted to post a new version of 2/3? Oops, seems I forgot to send it. Sorry :-/ I'll post a new version tomorrow. > I think you also want to do something with red, green, blue, transp when > entering FOURCC mode, at least setting them to zero or maybe even requiring > that they are zero to enter FOURCC mode (as additional safety barrier). Agreed. The FOURCC mode documentation already requires those fields to be set to 0 by applications. I'll enforce this in fb_set_var() if info->fix has the FB_CAP_FOURCC capability flag set. -- 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
Multiple Mantis devices gives me glitches
Hello, I have three Cinergy C (DVB-C cards) like this: 05:04.0 Multimedia controller: Twinhan Technology Co. Ltd Mantis DTV PCI Bridge Controller [Ver 1.0] (rev 01) Subsystem: TERRATEC Electronic GmbH Device 1178 Flags: bus master, medium devsel, latency 128, IRQ 20 Memory at fdcfe000 (32-bit, prefetchable) [size=4K] Kernel driver in use: Mantis Kernel modules: mantis Kernel: 2.6.38-13-generic-pae (Ubuntu Natty stock) Motherboard: P43-ES3G CPU: Intel(R) Core(TM)2 Quad CPUQ8400 At some point i started having glitches (I would from time to time get an 'old' frame displayed and sometimes audio noise when this happened). I tried pretty much every trick I could find: * CPU affinity * Dedicated IRQ for each card (only shared with USB, which has no units attached) * Various process priorities (also for the kdvb-processes) * pci latency (from 0x20 to 0xff) I have quite decent results when I only have 2 DVB cards present, and the results became even better when running the irqbalancer-dæmon as well. The glitches are not completely gone, but much more manageble now. So the problem seems to be caused by too many interrupts for my system to handle, however this is where I am in over my head. I know 2.6.38 isn't the freshest brew, but I could not find any changes to the driver since then that seemed relevant (which could just be my lack of source-fu). So, any ideas on how to improve the performance? I am suffering from some hardware incompatibility or is the driver this resource hungry? -- Vidar Tyldum vi...@tyldum.com PGP: 0x3110AA98 -- 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: [Linaro-mm-sig] [RFC v2 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Sat, Dec 10, 2011 at 03:13:06AM -0800, Mauro Carvalho Chehab wrote: > On 09-12-2011 20:50, Robert Morell wrote: > > On Mon, Dec 05, 2011 at 09:18:48AM -0800, Arnd Bergmann wrote: > >> On Friday 02 December 2011, Sumit Semwal wrote: > >>> This is the first step in defining a dma buffer sharing mechanism. > >> > > [...] > >> > >>> + return dmabuf; > >>> +} > >>> +EXPORT_SYMBOL(dma_buf_export); > >> > >> I agree with Konrad, this should definitely be EXPORT_SYMBOL_GPL, > >> because it's really a low-level function that I would expect > >> to get used by in-kernel subsystems providing the feature to > >> users and having back-end drivers, but it's not the kind of thing > >> we want out-of-tree drivers to mess with. > > > > Is this really necessary? If this is intended to be a > > lowest-common-denominator between many drivers to allow buffer sharing, > > it seems like it needs to be able to be usable by all drivers. > > > > If the interface is not accessible then I fear many drivers will be > > forced to continue to roll their own buffer sharing mechanisms (which is > > exactly what we're trying to avoid here, needless to say). > > Doing a buffer sharing with something that is not GPL is not fun, as, if any > issue rises there, it would be impossible to discover if the problem is either > at the closed-source driver or at the open source one. At the time I was using > the Nvidia proprietary driver, it was very common to have unexplained issues > caused likely by bad code there at the buffer management code, causing X > applications and extensions (like xv) to break. > > We should really make this EXPORT_SYMBOL_GPL(), in order to be able to latter > debug future share buffer issues, when needed. Sorry, I don't buy this argument. Making these exports GPL-only is not likely to cause anybody to open-source their driver, but will rather just cause them to use yet more closed-source code that is even less debuggable than this would be, to those without access to the source. Thanks, Robert > Regards, > Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
Hi, On 12/12/2011 10:49 AM, Ming Lei wrote: >>> If FD result is associated with a frame, how can user space get the frame >>> seq if no v4l2 buffer is involved? Without a frame sequence, it is a bit >>> difficult to retrieve FD results from user space. >> >> If you pass image data in memory buffers from user space, yes, it could be >> impossible. > > It is easy to get the frame sequence from v4l2_buffer for the case too, :-) Oops, have mixed something up ;) >> Not really, still v4l2_buffer may be used by other (sub)driver within same >> video processing pipeline. > > OK. > > A related question: how can we make one application to support the two kinds > of devices(input from user space data as OMAP4, input from SoC bus as Samsung) > at the same time? Maybe some capability info is to be exported to user space? > or other suggestions? Good question. To let applications know that a video device is not just an ordinary video output device I suppose we'll need a new object detection/recognition capability flag. V4L2_CAPS_OBJECT_DETECTION, V4L2_CAP_OBJECT_RECOGNITION or something similar. It's probably safe to assume the SoC will support either input method at time, not both simultaneously. Then it could be, for example, modelled with a video node and a subdev: user image data video capture for FDstream +-+ +-+ | /dev/video0 | | /dev/video0 | | OUTPUT| | CAPTURE| +--+--+ +--+--+ || v^ ..--++--+--+--+-+| image | link0 | pad | face | pad || sensor +-->-+ 0 | detector | 1 || sub-dev +-->-+ | | sub-dev | || ..--+| +-+--+-+| | | | +--+--++-+ | | | pad | image | pad | | +---+ 0 | processing | 1 +--+ link1 | | sub-dev| | +-++-+ User space can control state of link0. If the link is active (streaming) then access to /dev/video0 would be blocked by the driver, e.g. with EBUSY errno. This means that only one data source can be attached to an input pad (pad0). These are intrinsic properties of Media Controller/v4l2 subdev API. > And will your Samsung FD HW support to detect faces from memory? or just only > detect from SoC bus? I think we should be prepared for both configurations, as on a diagram above. [...] > OK, I will associate FD result with frame identifier, and not invent a > dedicated v4l2 event for query frame seq now until a specific requirement > for it is proposed. > > I will convert/integrate recent discussions into patches of v2 for further Sure, sounds like a good idea. > review, and sub device support will be provided. But before starting to do it, > I am still not clear how to integrate FD into MC framework. I understand FD > sub device is only a media entity, so how can FD sub device find the media > device(struct media_device)? or just needn't to care about it now? The media device driver will register all entities that belong to it and will create relevant links between entities' pads, which then can be activated by applications. How the entities are registered is another topic, that we don't need to be concerned about at the moment. If you're curious see drivers/media/video/omap3isp or driver/media/video/s5p-fimc for example media device drivers. -- Regards, Sylwester -- 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: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C
On 12.12.2011 14:56, Mauro Carvalho Chehab wrote:
> On 12-12-2011 11:40, Manu Abraham wrote:
>> On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
>
>>> This also means that just doing an alias from FE_QAM and
>>> SYS_DVBC_ANNEX_AC
>>> to
>>> SYS_DVBC_ANNEX_A may break something, as, for most devices,
>>> SYS_DVBC_ANNEX_AC
>>> really means both Annex A and C.
>>
>>
>>
>> With the current approach, the application can determine whether
>> the hardware supports through the DELSYS enumeration.
>>
>> So, if you have a device that needs to support both ANNEX_A and
>> ANNEX_C, it should be rather doing
>>
>> case DTV_ENUM_DELSYS:
>> buffer.data[0] = SYS_DVBC_ANNEX_A;
>> buffer.data[1] = SYS_DVBC_ANNEX_C;
>> break;
>
> Sure, but we'll need a logic to handle queries for SYS_DVBC_ANNEX_AC
> anyway, if any of the existing DVB-C drivers is currently prepared to
> support both.
>
> I'm not concerned with drx-k. The support for both standards are for
> kernel 3.3. So, no backward compatibility is needed here.
>
> While there is no explicit option, the code for stv0297, stv0367,
> tda10021 and tda10023 drivers are not clear if they support both
> (maybe roll-off might be auto-detected?) or just SYS_DVBC_ANNEX_A.
tda10021: Driver sets roll-off to 0.15. No auto-detection.
tda10023: Driver sets roll-off to 0.18. No auto-detection.
In general, auto-detection seems unlikely. Do you know any chip that
does it? Unless you do, we shouldn't expect it to exist. stv0297 doesn't
even detect IQ inversion.
> That's said, the difference between a 0.15 and a 0.13 rolloff is not big.
> I won't doubt that a demod set to 0.15 rolloff would be capable of working
> (non-optimized) with a 0.13 rolloff.
>
> What I'm saing is that, if any of the existing drivers currently works
> with both Annex A and Annex C, we'll need something equivalent to:
>
> if (delsys == SYS_DVBC_ANNEX_AC) {
> int ret = try_annex_a();
> if (ret < 0)
> ret = try_annex_c();
> }
I'd prefer treating ANNEX_AC just like ANNEX_A. It won't break anything,
because register writes for ANNEX_A will be the same. i.e. applications
using SYS_DVBC_ANNEX_AC will still get the same result as before.
What may change for a user: An updated application may allow him to
select between A and C, if the frontend advertises both. In this case,
both A and C are supposed to work, depending on the location of the
user. Someone who successfully used his tuner in Japan before, and who's
frontend doesn't advertise C, will still be able to choose A and thus
use the same register settings as before.
>>> I didn't look inside the drivers for stv0297, stv0367, tda10021 and
>>> tda10023.
>>> I suspect that some will need an additional code to change the roll-off,
>>> based on
>>> the delivery system.
>>
>>
>>
>> Of course, yes this would need to make the change across multiple
>> drivers.
>>
>> We can fix the drivers, that's no issue at all, as it is a small change.
>
> Indeed, it is a small change. Tuners are trivial to change, but, at the
> demod, we need to discover if roll-off is auto-detected somehow, or if
> they require manual settings, in order to fix the demod drivers.
tda10021: Register 0x3d & 0x01: 0 -> 0.15; 1 -> 0.13
tda10023: Register 0x3d & 0x03: 2 -> 0.15; 3 -> 0.13
Regards,
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Query] V4L2 Integer (?) menu control
Hi Sylwester, Sylwester Nawrocki wrote: Hi Sakari, On 11/26/2011 10:36 AM, Sakari Ailus wrote: On Fri, Nov 25, 2011 at 12:53:16AM +0100, Sylwester Nawrocki wrote: On 11/24/2011 09:57 PM, Sakari Ailus wrote: Sylwester Nawrocki wrote: On 11/24/2011 09:50 AM, Sakari Ailus wrote: There is not currently, but I have patches for it. The issue is that I need them myself but the driver I need them for isn't ready to be released yet. And as usual, I assume others than vivo is required to show they're really useful so I haven't sent them. That's great news. Then I might not need to do all the work on my own;) I hope mine will do. ;-) I'm working on 2.6.32 kernel (ouch!) so I haven't been able to test them properly yet. Please provide feedback on them if you find any issues. Sure, I just need to reserve some time to try these things out. Good that you asked so we won't end up writing essentially the same code again. I'll try to send the patches today. All right, there is no rush. I was just looking around how to support the camera scene mode with m5mols sort of sensors. The scene mode is essentially a compilation of several different parameters, for some of which there are standard controls in V4L2 but for many there are not. I fully agree with this approach. Scene modes should not be implemented at the level of the V4L2 API. Instead, the parameters that the scene modes consist of must be shown separately on the V4L2 API, if that is the level of API they belong to. Depending on your camera stack control algorithms could reside in the user space, which I believe is however not the case with the M5-MOLS. No, with these hybrid camera devices the algorithms are built in their own ISP. And there is quite many advanced algorithms, e.g. auto focus/face detection that are difficult to control at the subdevice API level. Can you tell what makes it difficult? I think the problem boils down to the following issues: 1) lack of controls for several parameters at the camera control class, e.g. - V4L2_CID_METERING_MODE (menu), - V4L2_CID_AUTO_EXPOSURE_BIAS (integer), - V4L2_CID_ISO (integer menu), - various auto focus modes, - capture mode: single, multi, auto bracketing, anti hand-shake, etc. These are indeed quite high level controls. You need some of this when your camera control algorithms run on the camera module itself. I think it's just a matter of adding them to the control class, or to export them as private controls. Some autofocus modes might require a private control, for example, if they need additional information from the user. 2) separate H/W contexts (image format, some controls) for viewfinder, snapshot, preview; introducing the SNAPSHOT control could partially resolve this; This could be more tricky. Guennadi did measurements on how much time could be saved by doing this, and the answer was in his case some 30 milliseconds. The sensor he was using used archaic 20 kHz bus speed instead of the 400 kHz one standardised in the beginning of 90s. How about M5-MOLS? 3) proper support for encoded data on the media bus 4) some sensors with embedded ISPs provide features which might be considered "high level", like for example scene mode (white balance) presets, exposure bracketing, drawing some additional markers in viewfinder video stream, e.g. a face detection rectangle, etc. Much of the time this kind of features bind together many parameters and it may be best to provide a private interface to these. That's also how it often is in the higher layers --- if your functionality is something that no-one else has chances are low that you can provide a fully standardised interface for it. That's just what comes to my mind right now. 1), 2), 3) shouldn't be difficult to solve, only 4) seems to be a real issue. I was planning to prepare an RFC on controls listed in 1). Some of them would need your recent integer menu control patches. The issue is that the subdev API seems to low level for the device but it's the only API available at the user space ;) ... This makes your user space to depend both on the sensor and the ISP, but there's really no way around that if both do non-trivial hardware-specific things. I guess a dedicated library for the sensor itself is needed on top of subdevice API to be able to use advanced features. And even then subdevice/V4L2 API is a limitation. How is it a limitation? Whe whole intent is to provide as standard as possible way to access the hardware features through an interface provided by the driver. So what is missing in your opinion? :-) What I tried to say was that there might different logic implemented at each hybrid sensor/ISP device. Some device specific sequences might be needed, which might not be possible to abstract in a v4l2 subdevice driver itself. With regards to what is missing please see above. Indeed. The V4L2 subdev API provides access to sub
Re: HVR-930C DVB-T mode report
On Mon, Dec 12, 2011 at 3:16 PM, Eddi De Pieri wrote: > Hi to all, > > with latest git, w_scan partially working only if adding -t2 or t3. > > It seems that scan quality of w_scan is lower if compared to dvb_app scan Hi Eddi, A really useful test would be to run the exact same scan twice in a row and see if you get consistent results (in terms of the number of frequencies are found and which ones). It would be worthwhile to know if there's a consistent problem locking to certain frequencies, or whether you get a lock for any given frequency is random. Run the scan with the exact same parameters again and see if you get the same results. Such information will drive whether the developers need to investigate why certain frequencies always fail to lock, or whether there is a more general problem with tuning that results in lock failure regardless of frequency. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.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: HVR-930C DVB-T mode report
Hi to all, with latest git, w_scan partially working only if adding -t2 or t3. It seems that scan quality of w_scan is lower if compared to dvb_app scan # w_scan -f t -t2 -x >it-Venice-w # w_scan -f t -t2 -I it-Venice-w >stdout wc -l stdout 73 # w_scan -f t -t3 >stdout 130 it-Venice-w.stdout # scan -f1 it-Venice-w - > it-Venice.stdout [..] dumping lists (62 services) Done. modprobe dvb-core dvb_mfe_wait_time=1 # scan -f1 it-Venice-w - > it-Venice.stdout [..] dumping lists (77 services) Done. # w_scan -F -f t -t2 -x -vvv -a /dev/dvb/adapter0/frontend1 > it-Venice-w_v1 [...] dumping lists (115 services) Done. Using other device (hauppauge hvr-900h) # w_scan -f t -x - > it-venice_hvr900h [...] dumping lists (461 services) Done. I can't figure out why these big differences... regards, Eddi -- 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: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C
On Tue, Dec 13, 2011 at 1:09 AM, Thomas Kernen wrote: > On 12/12/11 2:40 PM, Manu Abraham wrote: > or is it multiplied by a factor of 10 or was it 100 ? (Oh, my god my application Y uses a factor of 10, the X application uses 100 and the Z application uses 1000). What a lovely confusing scenario. ;-) (Other than for the mentioned issue that the rolloff can be read from the SI, which is available after tuning; for tuning you need rolloff). >>> >>> >>> >>> Sorry, but this argument doesn't make any sense to me. The same problem >>> exists on DVB-S2 already, where several rolloffs are supported. Except >>> if someone would code a channels.conf line in hand, the roll-off is not >>> visible by the end user. >> >> >> >> >> DVB-S2 as what we see as broadcast has just a single rolloff. The same >> rolloff is used in the SI alone. It's a mistake to handle rollolff as a >> user >> input field. The other rolloff's are used for very specific applications, >> such as DSNG, DVB-RCS etc, where bandwidth has to be really >> conserved considering uplinks from trucks, vans etc; for which we don't >> even have applications or users. > > > AFAIK there is at least one card (TBS 6925) that is supporting DVB-S2 > applications aimed normally at contribution markets and whereby the rolloff > may need to be specified. As far as I am aware, that card uses a STV0900 or a 903, more likely it is a STV0903 being a single input device. The STV0900/903 chips are capable of auto detecting the rolloff. All it needs is frequency and symbol rate. Even if it is another different demodulator: All the devices that I have seen which support the advanced MODCOD's, they support auto detection of rolloff. AFAIK, this is readable of the BBHEADER: MATYPE1, represented by 2 bits, as specified as in the DVB-S2 specification. There are other fields along with such as Single/Multiple Input Streams etc. Therefore no user intervention is required to determine rolloff on such devices. (It is read directly off the BBHEADER by the demod) and is available to the driver. Regards, Manu -- 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] it913x add support for IT9135 9006 devices
Support for IT1935 9006 devices.
9006 have version 2 type chip.
9006 devices should use dvb-usb-it9135-02.fw firmware.
On the device tested the tuner id was set to 0 which meant
the driver used tuner id 0x38. The device functioned normally.
Signed-off-by: Malcolm Priestley
---
drivers/media/dvb/dvb-usb/dvb-usb-ids.h |1 +
drivers/media/dvb/dvb-usb/it913x.c | 17 +++--
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/dvb-usb-ids.h
b/drivers/media/dvb/dvb-usb/dvb-usb-ids.h
index 3cce13b..d390dda 100644
--- a/drivers/media/dvb/dvb-usb/dvb-usb-ids.h
+++ b/drivers/media/dvb/dvb-usb/dvb-usb-ids.h
@@ -132,6 +132,7 @@
#define USB_PID_INTEL_CE9500 0x9500
#define USB_PID_ITETECH_IT9135 0x9135
#define USB_PID_ITETECH_IT9135_90050x9005
+#define USB_PID_ITETECH_IT9135_90060x9006
#define USB_PID_KWORLD_399U0xe399
#define USB_PID_KWORLD_399U_2 0xe400
#define USB_PID_KWORLD_395U0xe396
diff --git a/drivers/media/dvb/dvb-usb/it913x.c
b/drivers/media/dvb/dvb-usb/it913x.c
index 3ddf82a..6f6072b 100644
--- a/drivers/media/dvb/dvb-usb/it913x.c
+++ b/drivers/media/dvb/dvb-usb/it913x.c
@@ -387,6 +387,7 @@ static int it913x_rc_query(struct dvb_usb_device *d)
/* Firmware sets raw */
const char fw_it9135_v1[] = "dvb-usb-it9135-01.fw";
+const char fw_it9135_v2[] = "dvb-usb-it9135-02.fw";
const char fw_it9137[] = "dvb-usb-it9137-01.fw";
static int ite_firmware_select(struct usb_device *udev,
@@ -400,6 +401,9 @@ static int ite_firmware_select(struct usb_device *udev,
else if (le16_to_cpu(udev->descriptor.idProduct) ==
USB_PID_ITETECH_IT9135_9005)
sw = IT9135_V1_FW;
+ else if (le16_to_cpu(udev->descriptor.idProduct) ==
+ USB_PID_ITETECH_IT9135_9006)
+ sw = IT9135_V2_FW;
else
sw = IT9137_FW;
@@ -413,6 +417,11 @@ static int ite_firmware_select(struct usb_device *udev,
it913x_config.adc_x2 = 1;
props->firmware = fw_it9135_v1;
break;
+ case IT9135_V2_FW:
+ it913x_config.firmware_ver = 1;
+ it913x_config.adc_x2 = 1;
+ props->firmware = fw_it9135_v2;
+ break;
case IT9137_FW:
default:
it913x_config.firmware_ver = 0;
@@ -701,6 +710,7 @@ static struct usb_device_id it913x_table[] = {
{ USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9135) },
{ USB_DEVICE(USB_VID_KWORLD_2, USB_PID_SVEON_STV22_IT9137) },
{ USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9135_9005) },
+ { USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9135_9006) },
{} /* Terminating entry */
};
@@ -776,7 +786,7 @@ static struct dvb_usb_device_properties it913x_properties =
{
.rc_codes = RC_MAP_MSI_DIGIVOX_III,
},
.i2c_algo = &it913x_i2c_algo,
- .num_device_descs = 4,
+ .num_device_descs = 5,
.devices = {
{ "Kworld UB499-2T T09(IT9137)",
{ &it913x_table[0], NULL },
@@ -790,6 +800,9 @@ static struct dvb_usb_device_properties it913x_properties =
{
{ "ITE 9135(9005) Generic",
{ &it913x_table[3], NULL },
},
+ { "ITE 9135(9006) Generic",
+ { &it913x_table[4], NULL },
+ },
}
};
@@ -823,5 +836,5 @@ module_exit(it913x_module_exit);
MODULE_AUTHOR("Malcolm Priestley ");
MODULE_DESCRIPTION("it913x USB 2 Driver");
-MODULE_VERSION("1.14");
+MODULE_VERSION("1.17");
MODULE_LICENSE("GPL");
--
1.7.7.3
--
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 v4 0/3] fbdev: Add FOURCC-based format configuration API
Hi Laurent, On 12/12/2011 04:08 PM, Laurent Pinchart wrote: > Hi Florian, > > On Tuesday 29 November 2011 11:26:56 Laurent Pinchart wrote: >> Hi everybody, >> >> Here's the fourth version of the fbdev FOURCC-based format configuration >> API. > > Is there a chance this will make it to v3.3 ? Yes, that's likely. I thought you wanted to post a new version of 2/3? I think you also want to do something with red, green, blue, transp when entering FOURCC mode, at least setting them to zero or maybe even requiring that they are zero to enter FOURCC mode (as additional safety barrier). Best regards, Florian Tobias Schandinat -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date:Mon Dec 12 19:00:17 CET 2011 git hash:bcc072756e4467dc30e502a311b1c3adec96a0e4 gcc version: i686-linux-gcc (GCC) 4.6.1 host hardware:x86_64 host os: 3.1-2.slh.1-amd64 linux-git-arm-eabi-enoxys: ERRORS linux-git-arm-eabi-omap: ERRORS linux-git-armv5-ixp: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS linux-2.6.31.12-i686: ERRORS linux-2.6.32.6-i686: WARNINGS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: WARNINGS linux-2.6.35.3-i686: WARNINGS linux-2.6.36-i686: WARNINGS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-i686: WARNINGS linux-3.2-rc1-i686: WARNINGS linux-2.6.31.12-x86_64: ERRORS linux-2.6.32.6-x86_64: WARNINGS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: WARNINGS linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-x86_64: WARNINGS linux-3.2-rc1-x86_64: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2 The 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: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C
On Mon, Dec 12, 2011 at 9:52 PM, Mauro Carvalho Chehab
wrote:
> On 12-12-2011 13:00, Manu Abraham wrote:
>>
>> On Mon, Dec 12, 2011 at 7:26 PM, Mauro Carvalho Chehab
>> wrote:
>>>
>>> On 12-12-2011 11:40, Manu Abraham wrote:
On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
>>>
>>>
>>>
> This also means that just doing an alias from FE_QAM and
> SYS_DVBC_ANNEX_AC
> to
> SYS_DVBC_ANNEX_A may break something, as, for most devices,
> SYS_DVBC_ANNEX_AC
> really means both Annex A and C.
With the current approach, the application can determine whether
the hardware supports through the DELSYS enumeration.
So, if you have a device that needs to support both ANNEX_A and
ANNEX_C, it should be rather doing
case DTV_ENUM_DELSYS:
buffer.data[0] = SYS_DVBC_ANNEX_A;
buffer.data[1] = SYS_DVBC_ANNEX_C;
break;
>>>
>>>
>>>
>>> Sure, but we'll need a logic to handle queries for SYS_DVBC_ANNEX_AC
>>> anyway, if any of the existing DVB-C drivers is currently prepared to
>>> support both.
>>>
>>> I'm not concerned with drx-k. The support for both standards are for
>>> kernel 3.3. So, no backward compatibility is needed here.
>>>
>>> While there is no explicit option, the code for stv0297, stv0367,
>>> tda10021 and tda10023 drivers are not clear if they support both
>>> (maybe roll-off might be auto-detected?) or just SYS_DVBC_ANNEX_A.
>>>
>>> That's said, the difference between a 0.15 and a 0.13 rolloff is not big.
>>> I won't doubt that a demod set to 0.15 rolloff would be capable of
>>> working
>>> (non-optimized) with a 0.13 rolloff.
>>>
>>> What I'm saing is that, if any of the existing drivers currently works
>>> with both Annex A and Annex C, we'll need something equivalent to:
>>>
>>> if (delsys == SYS_DVBC_ANNEX_AC) {
>>> int ret = try_annex_a();
>>> if (ret< 0)
>>> ret = try_annex_c();
>>> }
>>>
>>> For FE_SET_FRONTEND (and the corresponding v5 logic), in order to avoid
>>> regressions.
>>
>>
>>
>> What I was implying:
>>
>> set_frontend/search()
>> {
>> case SYS_DVBC_ANNEX_A:
>> // do whatever you need to do for annex A tuning and return
>> break;
>> case SYS_DVBC_ANNEX_C:
>> // do whatever you need to do for annex C tuning and return
>> break;
>> }
>>
>>
>> ANNEX_AC is a link to ANNEX_A;
>
>
> Yes, I saw your approach.
>
>
>> We never had any ? users to ANNEX_C, so
>> that issue might be simple to ignore.
>
>
> This is hard to say. What I'm saying is that, if any of the current
> drivers works as-is with Annex C, we should assume that someone is using,
> as we don't have any evidence otherwise.
>
> I'm sure there are lots of people running Linux in Japan.
>
> How many of them are using the DVB subsystem is hard to say. The low message
> traffic at the ML for people *.jp is not a good measure, as due to language
> barriers, people may not be posting things there.
>
> A quick grep here on my local copy of the ML traffic (it currently has
> stored
> about 380 days of email, as I moved the older ones to a separate storage
> space)
> still shows 90 messages that has ".jp" inside:
>
> $ grep -l "\.jp" * |wc -l
> 90
>
> 41 of them has the word DVB inside. Ok, there are some false positives there
> too (due to *.jpg), but there are some valid hits also,
>
> Including a commit on this changeset:
> e38030f3ff02684eb9e25e983a03ad318a10a2ea.
> As the cx23885 driver does support DVB-C with stv0367, maybe the committer
> might be using it for DVB-C.
>
> Even if not, I suspect that it is likely to have some DVB-C Annex C users
> out there.
As far as I am aware, most of the services use BCAS2 encryption. There
is no BCAS2 support available as Open Source, other than with sundtek.
Regards,
Manu
--
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] m5mols: Change auto exposure control default value to AUTO
Enabling automatic exposure yields better image quality. With this setting
the anti-flicker algorithm is also enabled in automatic frequency detection
mode which effectively eliminates distortion from fluctuations of light
intensity at power line frequency.
Acked-by: HeungJun Kim
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/video/m5mols/m5mols_core.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/m5mols/m5mols_core.c
b/drivers/media/video/m5mols/m5mols_core.c
index 2f544cb..91cabf5 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -847,7 +847,7 @@ static int m5mols_init_controls(struct m5mols_info *info)
4, (1 << V4L2_COLORFX_BW), V4L2_COLORFX_NONE);
info->autoexposure = v4l2_ctrl_new_std_menu(&info->handle,
&m5mols_ctrl_ops, V4L2_CID_EXPOSURE_AUTO,
- 1, 0, V4L2_EXPOSURE_MANUAL);
+ 1, 0, V4L2_EXPOSURE_AUTO);
sd->ctrl_handler = &info->handle;
if (info->handle.error) {
--
1.7.8
--
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: [GIT PULL for 3.2-rc5] media fixes
On 12-12-2011 15:27, Sylwester Nawrocki wrote: Hi Mauro, On 12/12/2011 05:47 PM, Mauro Carvalho Chehab wrote: Hi Linus, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media v4l_for_linus For a couple fixes for media drivers. The changes are: - ati_remote (new driver for 3.2): Fix the scancode tables; - af9015: fix some issues with firmware load; - au0828: (trivial) add device ID's for some devices; - omap3 and s5p: several small fixes; - Update MAINTAINERS entry to cover /drivers/staging/media and media DocBook; - a few other small trivial fixes. Thanks! Mauro [...] Sylwester Nawrocki (10): [media] s5p-fimc: Fix wrong pointer dereference when unregistering sensors [media] s5p-fimc: Fix error in the capture subdev deinitialization [media] s5p-fimc: Fix initialization for proper system suspend support [media] s5p-fimc: Fix buffer dequeue order issue [media] s5p-fimc: Allow probe() to succeed with null platform data [media] s5p-fimc: Adjust pixel height alignments according to the IP revision [media] s5p-fimc: Fail driver probing when sensor configuration is wrong [media] s5p-fimc: Use correct fourcc for RGB565 colour format [media] m5mols: Fix set_fmt to return proper pixel format code [media] s5p-fimc: Fix camera input configuration in subdev operations There is one more quite important patch left out: http://git.infradead.org/users/kmpark/linux-2.6-samsung/commitdiff/817069678ab57150b21cd0705e2f3a3b2b6509f4 It was included in my recent pull request. It would be good to have in v3.2 too. Are you planning to add it to a next pull request to Linus ? Not likely, but it might be possible. I'll be out the rest of this week due to a travel for a series of meetings at the office. I might not be able to prepare another pull request until next week. After having it done, I should wait for a couple days for it to be at -next, but then it will almost be Xmas. Let's see. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/14] m5mols camera driver updates
Hello, This changset includes optimization for M-5MOLS booting time and changes needed for proper support of subdev user space interface. The last patch enables subdev node at s5p-csis MIPI receiver. This series depends on http://patchwork.linuxtv.org/patch/8694/ which adds framesamples field to the media bus format data structure. HeungJun Kim (4): m5mols: Extend the busy wait helper m5mols: Improve the interrupt handling routines m5mols: Add support for the system initialization interrupt m5mols: Optimize the capture set up sequence Sylwester Nawrocki (10): m5mols: Simplify the I2C registers definition m5mols: Add buffer size configuration support for compressed streams m5mols: Remove mode_save field from struct m5mols_info m5mols: Change the end of frame v4l2_subdev notification id m5mols: Don't ignore v4l2_ctrl_handler_setup() return value m5mols: Move the control handler initialization to probe() m5mols: Do not reset the configured pixel format when unexpected m5mols: Change auto exposure control default value to AUTO m5mols: Enable v4l subdev device node s5p-csis: Enable v4l subdev device node drivers/media/video/m5mols/m5mols.h | 51 +++-- drivers/media/video/m5mols/m5mols_capture.c | 87 +++- drivers/media/video/m5mols/m5mols_core.c| 303 --- drivers/media/video/m5mols/m5mols_reg.h | 248 +- drivers/media/video/s5p-fimc/mipi-csis.c| 22 ++ drivers/media/video/s5p-fimc/mipi-csis.h|3 + 6 files changed, 364 insertions(+), 350 deletions(-) -- 1.7.8 -- Thanks, Sylwester -- 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] m5mols: Change the end of frame v4l2_subdev notification id
Change the v4l2_device notifications id to S5P_FIMC_TX_END_NOTIFY.
Moreover, when frame capture fails, send an 'end of frame' notification
with size set to 0 to let the host driver return a buffer back to the
user and prevent applications waiting forever on DQBUF.
The notification is needed only for the s5p-fimc driver.
Acked-by: HeungJun Kim
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/video/m5mols/m5mols_capture.c | 13 +++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/m5mols/m5mols_capture.c
b/drivers/media/video/m5mols/m5mols_capture.c
index 8bb284d..038b349 100644
--- a/drivers/media/video/m5mols/m5mols_capture.c
+++ b/drivers/media/video/m5mols/m5mols_capture.c
@@ -1,3 +1,4 @@
+
/*
* The Capture code for Fujitsu M-5MOLS ISP
*
@@ -25,6 +26,7 @@
#include
#include
#include
+#include
#include "m5mols.h"
#include "m5mols_reg.h"
@@ -142,13 +144,20 @@ int m5mols_start_capture(struct m5mols_info *info)
if (!ret)
ret = m5mols_write(sd, CAPC_START, REG_CAP_START_MAIN);
if (!ret) {
+ bool captured = false;
+ unsigned int size;
+
/* Wait for the capture completion interrupt */
ret = m5mols_wait_interrupt(sd, REG_INT_CAPTURE, 2000);
if (!ret) {
+ captured = true;
ret = m5mols_capture_info(info);
- if (!ret)
- v4l2_subdev_notify(sd, 0, &info->cap.total);
}
+ size = captured ? info->cap.main : 0;
+ v4l2_dbg(1, m5mols_debug, sd, "%s: size: %d, thumb.: %d B\n",
+__func__, size, info->cap.thumb);
+
+ v4l2_subdev_notify(sd, S5P_FIMC_TX_END_NOTIFY, &size);
}
return ret;
--
1.7.8
--
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] m5mols: Don't ignore v4l2_ctrl_handler_setup() return value
v4l2_ctrl_handler_setup() may fail so check its return value when
restoring controls after device is powered on. While at it simplify
the m5mols_restore_function() a bit.
Acked-by: HeungJun Kim
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/video/m5mols/m5mols.h | 13 ++-
drivers/media/video/m5mols/m5mols_capture.c |2 +-
drivers/media/video/m5mols/m5mols_core.c| 31 +--
3 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/media/video/m5mols/m5mols.h
b/drivers/media/video/m5mols/m5mols.h
index 3cf9af3..78f82c9 100644
--- a/drivers/media/video/m5mols/m5mols.h
+++ b/drivers/media/video/m5mols/m5mols.h
@@ -196,15 +196,16 @@ struct m5mols_info {
struct media_pad pad;
struct v4l2_mbus_framefmt ffmt[M5MOLS_RESTYPE_MAX];
int res_type;
+
wait_queue_head_t irq_waitq;
atomic_t irq_done;
struct v4l2_ctrl_handler handle;
+
/* Autoexposure/exposure control cluster */
- struct {
- struct v4l2_ctrl *autoexposure;
- struct v4l2_ctrl *exposure;
- };
+ struct v4l2_ctrl *autoexposure;
+ struct v4l2_ctrl *exposure;
+
struct v4l2_ctrl *autowb;
struct v4l2_ctrl *colorfx;
struct v4l2_ctrl *saturation;
@@ -221,10 +222,10 @@ struct m5mols_info {
bool lock_awb;
u8 resolution;
u8 mode;
+
int (*set_power)(struct device *dev, int on);
};
-#define is_ctrl_synced(__info) (__info->ctrl_sync)
#define is_available_af(__info)(__info->ver.af)
#define is_code(__code, __type) (__code == m5mols_default_ffmt[__type].code)
#define is_manufacturer(__info, __manufacturer)\
@@ -290,7 +291,7 @@ int m5mols_mode(struct m5mols_info *info, u8 mode);
int m5mols_enable_interrupt(struct v4l2_subdev *sd, u8 reg);
int m5mols_wait_interrupt(struct v4l2_subdev *sd, u8 condition, u32 timeout);
-int m5mols_sync_controls(struct m5mols_info *info);
+int m5mols_restore_controls(struct m5mols_info *info);
int m5mols_start_capture(struct m5mols_info *info);
int m5mols_do_scenemode(struct m5mols_info *info, u8 mode);
int m5mols_lock_3a(struct m5mols_info *info, bool lock);
diff --git a/drivers/media/video/m5mols/m5mols_capture.c
b/drivers/media/video/m5mols/m5mols_capture.c
index 038b349..0fea2ad 100644
--- a/drivers/media/video/m5mols/m5mols_capture.c
+++ b/drivers/media/video/m5mols/m5mols_capture.c
@@ -117,7 +117,7 @@ int m5mols_start_capture(struct m5mols_info *info)
*/
ret = m5mols_mode(info, REG_MONITOR);
if (!ret)
- ret = m5mols_sync_controls(info);
+ ret = m5mols_restore_controls(info);
if (!ret)
ret = m5mols_write(sd, CAPP_YUVOUT_MAIN, REG_JPEG);
if (!ret)
diff --git a/drivers/media/video/m5mols/m5mols_core.c
b/drivers/media/video/m5mols/m5mols_core.c
index f47d406..99a096de 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -625,26 +625,25 @@ static struct v4l2_subdev_pad_ops m5mols_pad_ops = {
};
/**
- * m5mols_sync_controls - Apply default scene mode and the current controls
+ * m5mols_restore_controls - Apply current control values to the registers
*
- * This is used only streaming for syncing between v4l2_ctrl framework and
- * m5mols's controls. First, do the scenemode to the sensor, then call
- * v4l2_ctrl_handler_setup. It can be same between some commands and
- * the scenemode's in the default v4l2_ctrls. But, such commands of control
- * should be prior to the scenemode's one.
+ * m5mols_do_scenemode() handles all parameters for which there is yet no
+ * standard control in V4L2. It should be replaced at some point by setting
+ * each control individually, in required register set up order.
*/
-int m5mols_sync_controls(struct m5mols_info *info)
+int m5mols_restore_controls(struct m5mols_info *info)
{
- int ret = -EINVAL;
+ int ret;
- if (!is_ctrl_synced(info)) {
- ret = m5mols_do_scenemode(info, REG_SCENE_NORMAL);
- if (ret)
- return ret;
+ if (info->ctrl_sync)
+ return 0;
- v4l2_ctrl_handler_setup(&info->handle);
- info->ctrl_sync = 1;
- }
+ ret = m5mols_do_scenemode(info, REG_SCENE_NORMAL);
+ if (ret)
+ return ret;
+
+ ret = v4l2_ctrl_handler_setup(&info->handle);
+ info->ctrl_sync = !ret;
return ret;
}
@@ -668,7 +667,7 @@ static int m5mols_start_monitor(struct m5mols_info *info)
if (!ret)
ret = m5mols_mode(info, REG_MONITOR);
if (!ret)
- ret = m5mols_sync_controls(info);
+ ret = m5mols_restore_controls(info);
return ret;
}
--
1.7.8
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
[PATCH 14/14] s5p-csis: Enable v4l subdev device node
Set v4l2_subdev flags for a host driver to create a sub-device
node for the driver so the subdev can be directly configured
by applications. Add the subdev open() handler.
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/video/s5p-fimc/mipi-csis.c | 22 ++
drivers/media/video/s5p-fimc/mipi-csis.h |3 +++
2 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/s5p-fimc/mipi-csis.c
b/drivers/media/video/s5p-fimc/mipi-csis.c
index 59d79bc..130335c 100644
--- a/drivers/media/video/s5p-fimc/mipi-csis.c
+++ b/drivers/media/video/s5p-fimc/mipi-csis.c
@@ -427,6 +427,23 @@ static int s5pcsis_get_fmt(struct v4l2_subdev *sd, struct
v4l2_subdev_fh *fh,
return 0;
}
+static int s5pcsis_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+ struct v4l2_mbus_framefmt *format = v4l2_subdev_get_try_format(fh, 0);
+
+ format->colorspace = V4L2_COLORSPACE_JPEG;
+ format->code = s5pcsis_formats[0].code;
+ format->width = S5PCSIS_DEF_PIX_WIDTH;
+ format->height = S5PCSIS_DEF_PIX_HEIGHT;
+ format->field = V4L2_FIELD_NONE;
+
+ return 0;
+}
+
+static const struct v4l2_subdev_internal_ops s5pcsis_sd_internal_ops = {
+ .open = s5pcsis_open,
+};
+
static struct v4l2_subdev_core_ops s5pcsis_core_ops = {
.s_power = s5pcsis_s_power,
};
@@ -544,8 +561,13 @@ static int __devinit s5pcsis_probe(struct platform_device
*pdev)
v4l2_subdev_init(&state->sd, &s5pcsis_subdev_ops);
state->sd.owner = THIS_MODULE;
strlcpy(state->sd.name, dev_name(&pdev->dev), sizeof(state->sd.name));
+ state->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
state->csis_fmt = &s5pcsis_formats[0];
+ state->format.code = s5pcsis_formats[0].code;
+ state->format.width = S5PCSIS_DEF_PIX_WIDTH;
+ state->format.height = S5PCSIS_DEF_PIX_HEIGHT;
+
state->pads[CSIS_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
state->pads[CSIS_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_init(&state->sd.entity,
diff --git a/drivers/media/video/s5p-fimc/mipi-csis.h
b/drivers/media/video/s5p-fimc/mipi-csis.h
index f569133..2709286 100644
--- a/drivers/media/video/s5p-fimc/mipi-csis.h
+++ b/drivers/media/video/s5p-fimc/mipi-csis.h
@@ -19,4 +19,7 @@
#define CSIS_PAD_SOURCE1
#define CSIS_PADS_NUM 2
+#define S5PCSIS_DEF_PIX_WIDTH 640
+#define S5PCSIS_DEF_PIX_HEIGHT 480
+
#endif
--
1.7.8
--
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] m5mols: Move the control handler initialization to probe()
This is prerequisite for enabling the sub-device node.
The control handler is now initialized in driver's probe callback
in order to allow the user space access controls before the device
power is enabled with s_power. This is needed due to s_power being
currently called only by the host driver.
It also adds the subdev internal operations, only open() for now
for the TRY format initialization.
Acked-by: HeungJun Kim
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/video/m5mols/m5mols_core.c | 61 ++---
1 files changed, 46 insertions(+), 15 deletions(-)
diff --git a/drivers/media/video/m5mols/m5mols_core.c
b/drivers/media/video/m5mols/m5mols_core.c
index 99a096de..8a935a3 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -699,16 +699,25 @@ static int m5mols_s_ctrl(struct v4l2_ctrl *ctrl)
{
struct v4l2_subdev *sd = to_sd(ctrl);
struct m5mols_info *info = to_m5mols(sd);
- int isp_state = info->mode;
- int ret = 0;
+ int ispstate = info->mode;
+ int ret;
- ret = m5mols_mode(info, REG_PARAMETER);
- if (!ret)
- ret = m5mols_set_ctrl(ctrl);
- if (!ret)
- ret = m5mols_mode(info, isp_state);
+ /*
+* If needed, defer restoring the controls until
+* the device is fully initialized.
+*/
+ if (!info->isp_ready) {
+ info->ctrl_sync = 0;
+ return 0;
+ }
- return ret;
+ ret = m5mols_mode(info, REG_PARAMETER);
+ if (ret < 0)
+ return ret;
+ ret = m5mols_set_ctrl(ctrl);
+ if (ret < 0)
+ return ret;
+ return m5mols_mode(info, ispstate);
}
static const struct v4l2_ctrl_ops m5mols_ctrl_ops = {
@@ -868,8 +877,6 @@ static int m5mols_s_power(struct v4l2_subdev *sd, int on)
ret = m5mols_sensor_power(info, true);
if (!ret)
ret = m5mols_fw_start(sd);
- if (!ret)
- ret = m5mols_init_controls(info);
if (ret)
return ret;
@@ -894,10 +901,7 @@ static int m5mols_s_power(struct v4l2_subdev *sd, int on)
}
ret = m5mols_sensor_power(info, false);
- if (!ret) {
- v4l2_ctrl_handler_free(&info->handle);
- info->ctrl_sync = 0;
- }
+ info->ctrl_sync = 0;
return ret;
}
@@ -923,6 +927,21 @@ static const struct v4l2_subdev_core_ops m5mols_core_ops =
{
.log_status = m5mols_log_status,
};
+/*
+ * V4L2 subdev internal operations
+ */
+static int m5mols_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+ struct v4l2_mbus_framefmt *format = v4l2_subdev_get_try_format(fh, 0);
+
+ *format = m5mols_default_ffmt[0];
+ return 0;
+}
+
+static const struct v4l2_subdev_internal_ops m5mols_subdev_internal_ops = {
+ .open = m5mols_open,
+};
+
static const struct v4l2_subdev_ops m5mols_ops = {
.core = &m5mols_core_ops,
.pad= &m5mols_pad_ops,
@@ -986,6 +1005,7 @@ static int __devinit m5mols_probe(struct i2c_client
*client,
strlcpy(sd->name, MODULE_NAME, sizeof(sd->name));
v4l2_i2c_subdev_init(sd, client, &m5mols_ops);
+ sd->internal_ops = &m5mols_subdev_internal_ops;
info->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_init(&sd->entity, 1, &info->pad, 0);
if (ret < 0)
@@ -1002,7 +1022,17 @@ static int __devinit m5mols_probe(struct i2c_client
*client,
info->res_type = M5MOLS_RESTYPE_MONITOR;
atomic_set(&info->irq_done, 0);
- return 0;
+ ret = m5mols_sensor_power(info, true);
+ if (ret)
+ goto out_me;
+
+ ret = m5mols_fw_start(sd);
+ if (!ret)
+ ret = m5mols_init_controls(info);
+
+ m5mols_sensor_power(info, false);
+ if (!ret)
+ return 0;
out_me:
media_entity_cleanup(&sd->entity);
out_reg:
@@ -1020,6 +1050,7 @@ static int __devexit m5mols_remove(struct i2c_client
*client)
struct m5mols_info *info = to_m5mols(sd);
v4l2_device_unregister_subdev(sd);
+ v4l2_ctrl_handler_free(sd->ctrl_handler);
free_irq(client->irq, sd);
regulator_bulk_free(ARRAY_SIZE(supplies), supplies);
--
1.7.8
--
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] m5mols: Enable v4l subdev device node
Set V4L2_SUBDEV_FL_HAS_DEVNODE flag for the host driver to create the sensor device node. Acked-by: HeungJun Kim Signed-off-by: Sylwester Nawrocki Signed-off-by: Kyungmin Park --- drivers/media/video/m5mols/m5mols_core.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/m5mols/m5mols_core.c b/drivers/media/video/m5mols/m5mols_core.c index 91cabf5..e4801e9 100644 --- a/drivers/media/video/m5mols/m5mols_core.c +++ b/drivers/media/video/m5mols/m5mols_core.c @@ -997,6 +997,7 @@ static int __devinit m5mols_probe(struct i2c_client *client, sd = &info->sd; strlcpy(sd->name, MODULE_NAME, sizeof(sd->name)); v4l2_i2c_subdev_init(sd, client, &m5mols_ops); + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; sd->internal_ops = &m5mols_subdev_internal_ops; info->pad.flags = MEDIA_PAD_FL_SOURCE; -- 1.7.8 -- 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] m5mols: Do not reset the configured pixel format when unexpected
Initialize default pixel format in driver probe() rather than in s_power handler. This also prevents resetting the configuration applied before the device was powered on. Acked-by: HeungJun Kim Signed-off-by: Sylwester Nawrocki Signed-off-by: Kyungmin Park --- drivers/media/video/m5mols/m5mols_core.c | 10 +++--- 1 files changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/media/video/m5mols/m5mols_core.c b/drivers/media/video/m5mols/m5mols_core.c index 8a935a3..2f544cb 100644 --- a/drivers/media/video/m5mols/m5mols_core.c +++ b/drivers/media/video/m5mols/m5mols_core.c @@ -877,13 +877,6 @@ static int m5mols_s_power(struct v4l2_subdev *sd, int on) ret = m5mols_sensor_power(info, true); if (!ret) ret = m5mols_fw_start(sd); - if (ret) - return ret; - - info->ffmt[M5MOLS_RESTYPE_MONITOR] = - m5mols_default_ffmt[M5MOLS_RESTYPE_MONITOR]; - info->ffmt[M5MOLS_RESTYPE_CAPTURE] = - m5mols_default_ffmt[M5MOLS_RESTYPE_CAPTURE]; return ret; } @@ -1020,6 +1013,9 @@ static int __devinit m5mols_probe(struct i2c_client *client, goto out_me; } info->res_type = M5MOLS_RESTYPE_MONITOR; + info->ffmt[0] = m5mols_default_ffmt[0]; + info->ffmt[1] = m5mols_default_ffmt[1]; + atomic_set(&info->irq_done, 0); ret = m5mols_sensor_power(info, true); -- 1.7.8 -- 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] m5mols: Improve the interrupt handling routines
From: HeungJun Kim
The work struct based interrupt handling is not flexible enough
as the M-5MOLS control sequence involves I2C access sequences
before and after an interrupt is generated. A single waitqueue is
enough for the job so remove the work struct based code.
Signed-off-by: HeungJun Kim
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/video/m5mols/m5mols.h |9 +---
drivers/media/video/m5mols/m5mols_capture.c | 34 ++--
drivers/media/video/m5mols/m5mols_core.c| 56 +--
3 files changed, 25 insertions(+), 74 deletions(-)
diff --git a/drivers/media/video/m5mols/m5mols.h
b/drivers/media/video/m5mols/m5mols.h
index 2461a44..13da3f2 100644
--- a/drivers/media/video/m5mols/m5mols.h
+++ b/drivers/media/video/m5mols/m5mols.h
@@ -171,7 +171,6 @@ struct m5mols_version {
* @ffmt: current fmt according to resolution type
* @res_type: current resolution type
* @irq_waitq: waitqueue for the capture
- * @work_irq: workqueue for the IRQ
* @flags: state variable for the interrupt handler
* @handle: control handler
* @autoexposure: Auto Exposure control
@@ -188,7 +187,6 @@ struct m5mols_version {
* @lock_ae: true means the Auto Exposure is locked
* @lock_awb: true means the Aut WhiteBalance is locked
* @resolution:register value for current resolution
- * @interrupt: register value for current interrupt status
* @mode: register value for current operation mode
* @mode_save: register value for current operation mode for saving
* @set_power: optional power callback to the board code
@@ -200,8 +198,7 @@ struct m5mols_info {
struct v4l2_mbus_framefmt ffmt[M5MOLS_RESTYPE_MAX];
int res_type;
wait_queue_head_t irq_waitq;
- struct work_struct work_irq;
- unsigned long flags;
+ atomic_t irq_done;
struct v4l2_ctrl_handler handle;
/* Autoexposure/exposure control cluster */
@@ -221,14 +218,11 @@ struct m5mols_info {
bool lock_ae;
bool lock_awb;
u8 resolution;
- u8 interrupt;
u8 mode;
u8 mode_save;
int (*set_power)(struct device *dev, int on);
};
-#define ST_CAPT_IRQ 0
-
#define is_powered(__info) (__info->power)
#define is_ctrl_synced(__info) (__info->ctrl_sync)
#define is_available_af(__info)(__info->ver.af)
@@ -295,6 +289,7 @@ int m5mols_busy_wait(struct v4l2_subdev *sd, u32 reg, u32
value, u32 mask,
int m5mols_mode(struct m5mols_info *info, u8 mode);
int m5mols_enable_interrupt(struct v4l2_subdev *sd, u8 reg);
+int m5mols_wait_interrupt(struct v4l2_subdev *sd, u8 condition, u32 timeout);
int m5mols_sync_controls(struct m5mols_info *info);
int m5mols_start_capture(struct m5mols_info *info);
int m5mols_do_scenemode(struct m5mols_info *info, u8 mode);
diff --git a/drivers/media/video/m5mols/m5mols_capture.c
b/drivers/media/video/m5mols/m5mols_capture.c
index c8da22f..6409c3f 100644
--- a/drivers/media/video/m5mols/m5mols_capture.c
+++ b/drivers/media/video/m5mols/m5mols_capture.c
@@ -29,22 +29,6 @@
#include "m5mols.h"
#include "m5mols_reg.h"
-static int m5mols_capture_error_handler(struct m5mols_info *info,
- int timeout)
-{
- int ret;
-
- /* Disable all interrupts and clear relevant interrupt staus bits */
- ret = m5mols_write(&info->sd, SYSTEM_INT_ENABLE,
- info->interrupt & ~(REG_INT_CAPTURE));
- if (ret)
- return ret;
-
- if (timeout == 0)
- return -ETIMEDOUT;
-
- return 0;
-}
/**
* m5mols_read_rational - I2C read of a rational number
*
@@ -122,7 +106,6 @@ int m5mols_start_capture(struct m5mols_info *info)
struct v4l2_mbus_framefmt *mf = &info->ffmt[info->res_type];
struct v4l2_subdev *sd = &info->sd;
u8 resolution = info->resolution;
- int timeout;
int ret;
/*
@@ -143,14 +126,9 @@ int m5mols_start_capture(struct m5mols_info *info)
ret = m5mols_enable_interrupt(sd, REG_INT_CAPTURE);
if (!ret)
ret = m5mols_mode(info, REG_CAPTURE);
- if (!ret) {
+ if (!ret)
/* Wait for capture interrupt, after changing capture mode */
- timeout = wait_event_interruptible_timeout(info->irq_waitq,
- test_bit(ST_CAPT_IRQ, &info->flags),
- msecs_to_jiffies(2000));
- if (test_and_clear_bit(ST_CAPT_IRQ, &info->flags))
- ret = m5mols_capture_error_handler(info, timeout);
- }
+ ret = m5mols_wait_interrupt(sd, REG_INT_CAPTURE, 2000);
if (!ret)
ret = m5mols_lock_3a(info, false);
if (ret)
@@ -179,15 +157,13 @@ int m5mols_start_capture(struct m5mols_info *info)
ret = m5mols_write(sd, CAPC_START, REG_CAP_START_MAIN);
if (!ret) {
[PATCH 05/14] m5mols: Remove mode_save field from struct m5mols_info
There is no need to keep this in the drivers' private data structure,
an on the stack variable is enough. Also simplify a bit the ISP state
switching function.
Acked-by: HeungJun Kim
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/video/m5mols/m5mols.h |2 --
drivers/media/video/m5mols/m5mols_core.c | 13 ++---
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/media/video/m5mols/m5mols.h
b/drivers/media/video/m5mols/m5mols.h
index 13da3f2..cf9701c 100644
--- a/drivers/media/video/m5mols/m5mols.h
+++ b/drivers/media/video/m5mols/m5mols.h
@@ -188,7 +188,6 @@ struct m5mols_version {
* @lock_awb: true means the Aut WhiteBalance is locked
* @resolution:register value for current resolution
* @mode: register value for current operation mode
- * @mode_save: register value for current operation mode for saving
* @set_power: optional power callback to the board code
*/
struct m5mols_info {
@@ -219,7 +218,6 @@ struct m5mols_info {
bool lock_awb;
u8 resolution;
u8 mode;
- u8 mode_save;
int (*set_power)(struct device *dev, int on);
};
diff --git a/drivers/media/video/m5mols/m5mols_core.c
b/drivers/media/video/m5mols/m5mols_core.c
index a2b44ad..8ee5e81 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -369,13 +369,13 @@ int m5mols_mode(struct m5mols_info *info, u8 mode)
return ret;
ret = m5mols_read_u8(sd, SYSTEM_SYSMODE, ®);
- if ((!ret && reg == mode) || ret)
+ if (ret || reg == mode)
return ret;
switch (reg) {
case REG_PARAMETER:
ret = m5mols_reg_mode(sd, REG_MONITOR);
- if (!ret && mode == REG_MONITOR)
+ if (mode == REG_MONITOR)
break;
if (!ret)
ret = m5mols_reg_mode(sd, REG_CAPTURE);
@@ -392,7 +392,7 @@ int m5mols_mode(struct m5mols_info *info, u8 mode)
case REG_CAPTURE:
ret = m5mols_reg_mode(sd, REG_MONITOR);
- if (!ret && mode == REG_MONITOR)
+ if (mode == REG_MONITOR)
break;
if (!ret)
ret = m5mols_reg_mode(sd, REG_PARAMETER);
@@ -691,15 +691,14 @@ static int m5mols_s_ctrl(struct v4l2_ctrl *ctrl)
{
struct v4l2_subdev *sd = to_sd(ctrl);
struct m5mols_info *info = to_m5mols(sd);
- int ret;
-
- info->mode_save = info->mode;
+ int isp_state = info->mode;
+ int ret = 0;
ret = m5mols_mode(info, REG_PARAMETER);
if (!ret)
ret = m5mols_set_ctrl(ctrl);
if (!ret)
- ret = m5mols_mode(info, info->mode_save);
+ ret = m5mols_mode(info, isp_state);
return ret;
}
--
1.7.8
--
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] m5mols: Add support for the system initialization interrupt
From: HeungJun Kim
The M-5MOLS internal controller's initialization time depends on the
hardware and firmware revision. Currently the driver just waits for
worst case time period, after applying the voltage supplies, for
the device to be ready. The M-5MOLS supports "System initialization"
interrupt which is triggered after the controller finished booting.
So use this interrupt to optimize the initialization sequence.
After the voltage supplies are applied the I2C communication will
fail, until the internal controller initializes to Flash Writer
state. For the period when the I2C is not accessible use the
isp_ready flag to suppress the error logs.
Signed-off-by: HeungJun Kim
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/video/m5mols/m5mols.h | 12 +++--
drivers/media/video/m5mols/m5mols_core.c | 77 +
drivers/media/video/m5mols/m5mols_reg.h |3 +-
3 files changed, 54 insertions(+), 38 deletions(-)
diff --git a/drivers/media/video/m5mols/m5mols.h
b/drivers/media/video/m5mols/m5mols.h
index cf9701c..3cf9af3 100644
--- a/drivers/media/video/m5mols/m5mols.h
+++ b/drivers/media/video/m5mols/m5mols.h
@@ -182,8 +182,8 @@ struct m5mols_version {
* @ver: information of the version
* @cap: the capture mode attributes
* @power: current sensor's power status
- * @ctrl_sync: true means all controls of the sensor are initialized
- * @int_capture: true means the capture interrupt is issued once
+ * @isp_ready: 1 when the ISP controller has completed booting
+ * @ctrl_sync: 1 when the control handler state is restored in H/W
* @lock_ae: true means the Auto Exposure is locked
* @lock_awb: true means the Aut WhiteBalance is locked
* @resolution:register value for current resolution
@@ -212,8 +212,11 @@ struct m5mols_info {
struct m5mols_version ver;
struct m5mols_capture cap;
- bool power;
- bool ctrl_sync;
+
+ unsigned int isp_ready:1;
+ unsigned int power:1;
+ unsigned int ctrl_sync:1;
+
bool lock_ae;
bool lock_awb;
u8 resolution;
@@ -221,7 +224,6 @@ struct m5mols_info {
int (*set_power)(struct device *dev, int on);
};
-#define is_powered(__info) (__info->power)
#define is_ctrl_synced(__info) (__info->ctrl_sync)
#define is_available_af(__info)(__info->ver.af)
#define is_code(__code, __type) (__code == m5mols_default_ffmt[__type].code)
diff --git a/drivers/media/video/m5mols/m5mols_core.c
b/drivers/media/video/m5mols/m5mols_core.c
index 8ee5e81..3298c6f 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -135,10 +135,13 @@ static u32 m5mols_swap_byte(u8 *data, u8 length)
* @reg: combination of size, category and command for the I2C packet
* @size: desired size of I2C packet
* @val: read value
+ *
+ * Returns 0 on success, or else negative errno.
*/
static int m5mols_read(struct v4l2_subdev *sd, u32 size, u32 reg, u32 *val)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct m5mols_info *info = to_m5mols(sd);
u8 rbuf[M5MOLS_I2C_MAX_SIZE + 1];
u8 category = I2C_CATEGORY(reg);
u8 cmd = I2C_COMMAND(reg);
@@ -168,15 +171,17 @@ static int m5mols_read(struct v4l2_subdev *sd, u32 size,
u32 reg, u32 *val)
usleep_range(200, 200);
ret = i2c_transfer(client->adapter, msg, 2);
- if (ret < 0) {
- v4l2_err(sd, "read failed: size:%d cat:%02x cmd:%02x. %d\n",
-size, category, cmd, ret);
- return ret;
+
+ if (ret == 2) {
+ *val = m5mols_swap_byte(&rbuf[1], size);
+ return 0;
}
- *val = m5mols_swap_byte(&rbuf[1], size);
+ if (info->isp_ready)
+ v4l2_err(sd, "read failed: size:%d cat:%02x cmd:%02x. %d\n",
+size, category, cmd, ret);
- return 0;
+ return ret < 0 ? ret : -EIO;
}
int m5mols_read_u8(struct v4l2_subdev *sd, u32 reg, u8 *val)
@@ -229,10 +234,13 @@ int m5mols_read_u32(struct v4l2_subdev *sd, u32 reg, u32
*val)
* m5mols_write - I2C command write function
* @reg: combination of size, category and command for the I2C packet
* @val: value to write
+ *
+ * Returns 0 on success, or else negative errno.
*/
int m5mols_write(struct v4l2_subdev *sd, u32 reg, u32 val)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct m5mols_info *info = to_m5mols(sd);
u8 wbuf[M5MOLS_I2C_MAX_SIZE + 4];
u8 category = I2C_CATEGORY(reg);
u8 cmd = I2C_COMMAND(reg);
@@ -263,13 +271,14 @@ int m5mols_write(struct v4l2_subdev *sd, u32 reg, u32 val)
usleep_range(200, 200);
ret = i2c_transfer(client->adapter, msg, 1);
- if (ret < 0) {
- v4l2_err(sd, "write failed: size:%d cat:%02x cmd:%02x. %d\n",
- size, category, cmd, ret);
- return ret;
-
[PATCH 07/14] m5mols: Optimize the capture set up sequence
From: HeungJun Kim
Improve the single frame capture set up sequence. Since there is
no need to re-enable the interrupts in each capture sequence, unmask
the required interrupts once at the device initialization time.
Signed-off-by: HeungJun Kim
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/video/m5mols/m5mols_capture.c | 40 +--
drivers/media/video/m5mols/m5mols_core.c|3 +-
2 files changed, 15 insertions(+), 28 deletions(-)
diff --git a/drivers/media/video/m5mols/m5mols_capture.c
b/drivers/media/video/m5mols/m5mols_capture.c
index 6409c3f..8bb284d 100644
--- a/drivers/media/video/m5mols/m5mols_capture.c
+++ b/drivers/media/video/m5mols/m5mols_capture.c
@@ -109,51 +109,37 @@ int m5mols_start_capture(struct m5mols_info *info)
int ret;
/*
-* Preparing capture. Setting control & interrupt before entering
-* capture mode
-*
-* 1) change to MONITOR mode for operating control & interrupt
-* 2) set controls (considering v4l2_control value & lock 3A)
-* 3) set interrupt
-* 4) change to CAPTURE mode
+* Synchronize the controls, set the capture frame resolution and color
+* format. The frame capture is initiated during switching from Monitor
+* to Capture mode.
*/
ret = m5mols_mode(info, REG_MONITOR);
if (!ret)
ret = m5mols_sync_controls(info);
if (!ret)
- ret = m5mols_lock_3a(info, true);
+ ret = m5mols_write(sd, CAPP_YUVOUT_MAIN, REG_JPEG);
if (!ret)
- ret = m5mols_enable_interrupt(sd, REG_INT_CAPTURE);
+ ret = m5mols_write(sd, CAPP_MAIN_IMAGE_SIZE, resolution);
+ if (!ret)
+ ret = m5mols_write(sd, CAPP_JPEG_SIZE_MAX,
+ mf->framesamples - M5MOLS_JPEG_TAGS_SIZE);
+ if (!ret)
+ ret = m5mols_lock_3a(info, true);
if (!ret)
ret = m5mols_mode(info, REG_CAPTURE);
if (!ret)
- /* Wait for capture interrupt, after changing capture mode */
+ /* Wait until a frame is captured to ISP internal memory */
ret = m5mols_wait_interrupt(sd, REG_INT_CAPTURE, 2000);
if (!ret)
ret = m5mols_lock_3a(info, false);
if (ret)
return ret;
+
/*
-* Starting capture. Setting capture frame count and resolution and
-* the format(available format: JPEG, Bayer RAW, YUV).
-*
-* 1) select single or multi(enable to 25), format, size
-* 2) set interrupt
-* 3) start capture(for main image, now)
-* 4) get information
-* 5) notify file size to v4l2 device(e.g, to s5p-fimc v4l2 device)
+* Initiate the captured data transfer to a MIPI-CSI receiver.
*/
ret = m5mols_write(sd, CAPC_SEL_FRAME, 1);
if (!ret)
- ret = m5mols_write(sd, CAPP_YUVOUT_MAIN, REG_JPEG);
- if (!ret)
- ret = m5mols_write(sd, CAPP_MAIN_IMAGE_SIZE, resolution);
- if (!ret)
- ret = m5mols_write(sd, CAPP_JPEG_SIZE_MAX,
- mf->framesamples - M5MOLS_JPEG_TAGS_SIZE);
- if (!ret)
- ret = m5mols_enable_interrupt(sd, REG_INT_CAPTURE);
- if (!ret)
ret = m5mols_write(sd, CAPC_START, REG_CAP_START_MAIN);
if (!ret) {
/* Wait for the capture completion interrupt */
diff --git a/drivers/media/video/m5mols/m5mols_core.c
b/drivers/media/video/m5mols/m5mols_core.c
index 3298c6f..f47d406 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -801,7 +801,8 @@ static int m5mols_fw_start(struct v4l2_subdev *sd)
ret = m5mols_write(sd, PARM_INTERFACE, REG_INTERFACE_MIPI);
if (!ret)
- ret = m5mols_enable_interrupt(sd, REG_INT_AF);
+ ret = m5mols_enable_interrupt(sd,
+ REG_INT_AF | REG_INT_CAPTURE);
return ret;
}
--
1.7.8
--
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] m5mols: Simplify the I2C registers definition
The redundant definitions of the m5mols I2C register addresses within
the pages (categories) are removed. In place of symbolic definitions
plain numbers are used which simplifies the code and eases identifying
the registers in the documentation.
Also make the m5mols_busy() function accept I2C_REG() value as a register
address, like all other functions, rather than using the category and
command values.
Acked-by: HeungJun Kim
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/video/m5mols/m5mols.h |2 +-
drivers/media/video/m5mols/m5mols_core.c |9 +-
drivers/media/video/m5mols/m5mols_reg.h | 244 --
3 files changed, 102 insertions(+), 153 deletions(-)
diff --git a/drivers/media/video/m5mols/m5mols.h
b/drivers/media/video/m5mols/m5mols.h
index 82c8817..42c494c 100644
--- a/drivers/media/video/m5mols/m5mols.h
+++ b/drivers/media/video/m5mols/m5mols.h
@@ -257,7 +257,7 @@ int m5mols_read_u8(struct v4l2_subdev *sd, u32 reg_comb, u8
*val);
int m5mols_read_u16(struct v4l2_subdev *sd, u32 reg_comb, u16 *val);
int m5mols_read_u32(struct v4l2_subdev *sd, u32 reg_comb, u32 *val);
int m5mols_write(struct v4l2_subdev *sd, u32 reg_comb, u32 val);
-int m5mols_busy(struct v4l2_subdev *sd, u8 category, u8 cmd, u8 value);
+int m5mols_busy(struct v4l2_subdev *sd, u32 reg, u8 value);
/*
* Mode operation of the M-5MOLS
diff --git a/drivers/media/video/m5mols/m5mols_core.c
b/drivers/media/video/m5mols/m5mols_core.c
index e0f09e5..10170b8 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -272,14 +272,14 @@ int m5mols_write(struct v4l2_subdev *sd, u32 reg, u32 val)
return 0;
}
-int m5mols_busy(struct v4l2_subdev *sd, u8 category, u8 cmd, u8 mask)
+int m5mols_busy(struct v4l2_subdev *sd, u32 reg, u8 mask)
{
u8 busy;
int i;
int ret;
for (i = 0; i < M5MOLS_I2C_CHECK_RETRY; i++) {
- ret = m5mols_read_u8(sd, I2C_REG(category, cmd, 1), &busy);
+ ret = m5mols_read_u8(sd, reg, &busy);
if (ret < 0)
return ret;
if ((busy & mask) == mask)
@@ -317,7 +317,7 @@ static int m5mols_reg_mode(struct v4l2_subdev *sd, u8 mode)
{
int ret = m5mols_write(sd, SYSTEM_SYSMODE, mode);
- return ret ? ret : m5mols_busy(sd, CAT_SYSTEM, CAT0_SYSMODE, mode);
+ return ret ? ret : m5mols_busy(sd, mode, SYSTEM_SYSMODE);
}
/**
@@ -829,8 +829,7 @@ static int m5mols_s_power(struct v4l2_subdev *sd, int on)
if (!ret)
ret = m5mols_write(sd, AF_MODE, REG_AF_POWEROFF);
if (!ret)
- ret = m5mols_busy(sd, CAT_SYSTEM, CAT0_STATUS,
- REG_AF_IDLE);
+ ret = m5mols_busy(sd, SYSTEM_STATUS, REG_AF_IDLE);
if (!ret)
v4l2_info(sd, "Success soft-landing lens\n");
}
diff --git a/drivers/media/video/m5mols/m5mols_reg.h
b/drivers/media/video/m5mols/m5mols_reg.h
index c755bd6..d488add 100644
--- a/drivers/media/video/m5mols/m5mols_reg.h
+++ b/drivers/media/video/m5mols/m5mols_reg.h
@@ -55,39 +55,31 @@
* There is many registers between customer version address and awb one. For
* more specific contents, see definition if file m5mols.h.
*/
-#define CAT0_VER_CUSTOMER 0x00/* customer version */
-#define CAT0_VER_PROJECT 0x01/* project version */
-#define CAT0_VER_FIRMWARE 0x02/* Firmware version */
-#define CAT0_VER_HARDWARE 0x04/* Hardware version */
-#define CAT0_VER_PARAMETER 0x06/* Parameter version */
-#define CAT0_VER_AWB 0x08/* Auto WB version */
-#define CAT0_VER_STRING0x0a/* string including M-5MOLS */
-#define CAT0_SYSMODE 0x0b/* SYSTEM mode register */
-#define CAT0_STATUS0x0c/* SYSTEM mode status register */
-#define CAT0_INT_FACTOR0x10/* interrupt pending register */
-#define CAT0_INT_ENABLE0x11/* interrupt enable register */
-
-#define SYSTEM_VER_CUSTOMERI2C_REG(CAT_SYSTEM, CAT0_VER_CUSTOMER, 1)
-#define SYSTEM_VER_PROJECT I2C_REG(CAT_SYSTEM, CAT0_VER_PROJECT, 1)
-#define SYSTEM_VER_FIRMWAREI2C_REG(CAT_SYSTEM, CAT0_VER_FIRMWARE, 2)
-#define SYSTEM_VER_HARDWAREI2C_REG(CAT_SYSTEM, CAT0_VER_HARDWARE, 2)
-#define SYSTEM_VER_PARAMETER I2C_REG(CAT_SYSTEM, CAT0_VER_PARAMETER, 2)
-#define SYSTEM_VER_AWB I2C_REG(CAT_SYSTEM, CAT0_VER_AWB, 2)
-
-#define SYSTEM_SYSMODE I2C_REG(CAT_SYSTEM, CAT0_SYSMODE, 1)
+#define SYSTEM_VER_CUSTOMERI2C_REG(CAT_SYSTEM, 0x00, 1)
+#define SYSTEM_VER_PROJECT I2C_REG(CAT_SYSTEM, 0x01, 1)
+#define SYSTEM_VER_FIRMWAREI2C_REG(CAT_SYSTEM, 0x02, 2)
+#define SYSTEM_VER_HARDWAREI2C_REG(CAT_SYSTEM, 0x04, 2)
+#define SYSTEM_VER_PARAMETER I2C_REG(CAT_SYSTEM, 0x06, 2)
+#define SYSTEM_VER_AWB
[PATCH 03/14] m5mols: Extend the busy wait helper
From: HeungJun Kim
Make m5mols_busy_wait function jiffies based rather than relying
on some fixed number of I2C read iterations while busy waiting
for the device to execute a request. With fixed number of iterations
we may be getting different wait times, depending on the I2C speed.
In some conditions we have to wait even if the I2C communications
fails, in those cases M5MOLS_I2C_RDY_WAIT_MASK should be passed
as the mask argument to m5mols_busy_wait().
Signed-off-by: HeungJun Kim
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/video/m5mols/m5mols.h | 11 ++-
drivers/media/video/m5mols/m5mols_core.c | 47 +-
2 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/drivers/media/video/m5mols/m5mols.h
b/drivers/media/video/m5mols/m5mols.h
index 21ccc98..2461a44 100644
--- a/drivers/media/video/m5mols/m5mols.h
+++ b/drivers/media/video/m5mols/m5mols.h
@@ -23,6 +23,10 @@
#define M5MOLS_JPEG_TAGS_SIZE 0x2
#define M5MOLS_MAIN_JPEG_SIZE_MAX (5 * SZ_1M)
+/* ISP state transition timeout, in ms */
+#define M5MOLS_MODE_CHANGE_TIMEOUT 200
+#define M5MOLS_BUSY_WAIT_DEF_TIMEOUT 250
+
extern int m5mols_debug;
#define to_m5mols(__sd)container_of(__sd, struct m5mols_info, sd)
@@ -261,7 +265,12 @@ int m5mols_read_u8(struct v4l2_subdev *sd, u32 reg_comb,
u8 *val);
int m5mols_read_u16(struct v4l2_subdev *sd, u32 reg_comb, u16 *val);
int m5mols_read_u32(struct v4l2_subdev *sd, u32 reg_comb, u32 *val);
int m5mols_write(struct v4l2_subdev *sd, u32 reg_comb, u32 val);
-int m5mols_busy(struct v4l2_subdev *sd, u32 reg, u8 value);
+
+int m5mols_busy_wait(struct v4l2_subdev *sd, u32 reg, u32 value, u32 mask,
+int timeout);
+
+/* Mask value for busy waiting until M-5MOLS I2C interface is initialized */
+#define M5MOLS_I2C_RDY_WAIT_MASK ((u32)~0)
/*
* Mode operation of the M-5MOLS
diff --git a/drivers/media/video/m5mols/m5mols_core.c
b/drivers/media/video/m5mols/m5mols_core.c
index 59903ed..a1302dc 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -272,19 +272,35 @@ int m5mols_write(struct v4l2_subdev *sd, u32 reg, u32 val)
return 0;
}
-int m5mols_busy(struct v4l2_subdev *sd, u32 reg, u8 mask)
+/**
+ * m5mols_busy_wait - Busy waiting with I2C register polling
+ * @reg: the I2C_REG() address of an 8-bit status register to check
+ * @value: expected status register value
+ * @mask: bit mask for the read status register value
+ * @timeout: timeout in miliseconds, or -1 for default timeout
+ *
+ * The @reg register value is ORed with @mask before comparing with @value.
+ *
+ * Return: 0 if the requested condition became true within less than
+ * @timeout ms, or else negative errno.
+ */
+int m5mols_busy_wait(struct v4l2_subdev *sd, u32 reg, u32 value, u32 mask,
+int timeout)
{
- u8 busy;
- int i;
- int ret;
+ int ms = timeout < 0 ? M5MOLS_BUSY_WAIT_DEF_TIMEOUT : timeout;
+ unsigned long end = jiffies + msecs_to_jiffies(ms);
+ u8 status;
- for (i = 0; i < M5MOLS_I2C_CHECK_RETRY; i++) {
- ret = m5mols_read_u8(sd, reg, &busy);
- if (ret < 0)
+ do {
+ int ret = m5mols_read_u8(sd, reg, &status);
+
+ if (ret < 0 && mask <= 0xff)
return ret;
- if ((busy & mask) == mask)
+ if (!ret && (status & mask) == value)
return 0;
- }
+ usleep_range(100, 250);
+ } while (ms > 0 && time_is_after_jiffies(end));
+
return -EBUSY;
}
@@ -316,8 +332,10 @@ int m5mols_enable_interrupt(struct v4l2_subdev *sd, u8 reg)
static int m5mols_reg_mode(struct v4l2_subdev *sd, u8 mode)
{
int ret = m5mols_write(sd, SYSTEM_SYSMODE, mode);
-
- return ret ? ret : m5mols_busy(sd, mode, SYSTEM_SYSMODE);
+ if (ret < 0)
+ return ret;
+ return m5mols_busy_wait(sd, SYSTEM_SYSMODE, mode, 0xff,
+ M5MOLS_MODE_CHANGE_TIMEOUT);
}
/**
@@ -844,9 +862,10 @@ static int m5mols_s_power(struct v4l2_subdev *sd, int on)
if (!ret)
ret = m5mols_write(sd, AF_MODE, REG_AF_POWEROFF);
if (!ret)
- ret = m5mols_busy(sd, SYSTEM_STATUS, REG_AF_IDLE);
- if (!ret)
- v4l2_info(sd, "Success soft-landing lens\n");
+ ret = m5mols_busy_wait(sd, SYSTEM_STATUS, REG_AF_IDLE,
+ 0xff, -1);
+ if (ret < 0)
+ v4l2_warn(sd, "Soft landing lens failed\n");
}
ret = m5mols_sensor_power(info, false);
--
1.7.8
--
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 h
[PATCH 02/14] m5mols: Add buffer size configuration support for compressed streams
Use new struct v4l2_mbus_framefmt framesamples field to return
maximum size of data transmitted per single frame. The framesamples
value is adjusted according to pixel format and configured at the
ISP registers.
Except the pixel width and height, the frame size can also be made
dependent on JPEG quality ratio, when corresponding control is
available at the sub-device interface.
Acked-by: HeungJun Kim
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
drivers/media/video/m5mols/m5mols.h |4
drivers/media/video/m5mols/m5mols_capture.c |4
drivers/media/video/m5mols/m5mols_core.c| 15 +++
drivers/media/video/m5mols/m5mols_reg.h |1 +
4 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/m5mols/m5mols.h
b/drivers/media/video/m5mols/m5mols.h
index 42c494c..21ccc98 100644
--- a/drivers/media/video/m5mols/m5mols.h
+++ b/drivers/media/video/m5mols/m5mols.h
@@ -16,9 +16,13 @@
#ifndef M5MOLS_H
#define M5MOLS_H
+#include
#include
#include "m5mols_reg.h"
+#define M5MOLS_JPEG_TAGS_SIZE 0x2
+#define M5MOLS_MAIN_JPEG_SIZE_MAX (5 * SZ_1M)
+
extern int m5mols_debug;
#define to_m5mols(__sd)container_of(__sd, struct m5mols_info, sd)
diff --git a/drivers/media/video/m5mols/m5mols_capture.c
b/drivers/media/video/m5mols/m5mols_capture.c
index 3248ac8..c8da22f 100644
--- a/drivers/media/video/m5mols/m5mols_capture.c
+++ b/drivers/media/video/m5mols/m5mols_capture.c
@@ -119,6 +119,7 @@ static int m5mols_capture_info(struct m5mols_info *info)
int m5mols_start_capture(struct m5mols_info *info)
{
+ struct v4l2_mbus_framefmt *mf = &info->ffmt[info->res_type];
struct v4l2_subdev *sd = &info->sd;
u8 resolution = info->resolution;
int timeout;
@@ -170,6 +171,9 @@ int m5mols_start_capture(struct m5mols_info *info)
if (!ret)
ret = m5mols_write(sd, CAPP_MAIN_IMAGE_SIZE, resolution);
if (!ret)
+ ret = m5mols_write(sd, CAPP_JPEG_SIZE_MAX,
+ mf->framesamples - M5MOLS_JPEG_TAGS_SIZE);
+ if (!ret)
ret = m5mols_enable_interrupt(sd, REG_INT_CAPTURE);
if (!ret)
ret = m5mols_write(sd, CAPC_START, REG_CAP_START_MAIN);
diff --git a/drivers/media/video/m5mols/m5mols_core.c
b/drivers/media/video/m5mols/m5mols_core.c
index 10170b8..59903ed 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -505,6 +505,16 @@ static struct v4l2_mbus_framefmt *__find_format(struct
m5mols_info *info,
return &info->ffmt[type];
}
+static inline void m5mols_get_buffer_size(struct m5mols_info *info,
+ struct v4l2_mbus_framefmt *mf)
+{
+ u32 min_sz = mf->width * mf->height + M5MOLS_JPEG_TAGS_SIZE;
+
+ /* Clamp provided value to the minimum needed */
+ mf->framesamples = clamp_t(u32, mf->framesamples, min_sz,
+ M5MOLS_MAIN_JPEG_SIZE_MAX);
+}
+
static int m5mols_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
struct v4l2_subdev_format *fmt)
{
@@ -515,6 +525,9 @@ static int m5mols_get_fmt(struct v4l2_subdev *sd, struct
v4l2_subdev_fh *fh,
if (!format)
return -EINVAL;
+ if (format->code == V4L2_MBUS_FMT_JPEG_1X8)
+ m5mols_get_buffer_size(info, format);
+
fmt->format = *format;
return 0;
}
@@ -537,6 +550,8 @@ static int m5mols_set_fmt(struct v4l2_subdev *sd, struct
v4l2_subdev_fh *fh,
if (!sfmt)
return 0;
+ if (format->code == V4L2_MBUS_FMT_JPEG_1X8)
+ m5mols_get_buffer_size(info, format);
format->code = m5mols_default_ffmt[type].code;
format->colorspace = V4L2_COLORSPACE_JPEG;
diff --git a/drivers/media/video/m5mols/m5mols_reg.h
b/drivers/media/video/m5mols/m5mols_reg.h
index d488add..03d1c47 100644
--- a/drivers/media/video/m5mols/m5mols_reg.h
+++ b/drivers/media/video/m5mols/m5mols_reg.h
@@ -310,6 +310,7 @@
#define REG_JPEG 0x10
#define CAPP_MAIN_IMAGE_SIZE I2C_REG(CAT_CAPT_PARM, 0x01, 1)
+#define CAPP_JPEG_SIZE_MAX I2C_REG(CAT_CAPT_PARM, 0x0f, 4)
#define CAPP_MCC_MODE I2C_REG(CAT_CAPT_PARM, 0x1d, 1)
#define REG_MCC_OFF0x00
--
1.7.8
--
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: [GIT PULL for 3.2-rc5] media fixes
Hi Mauro, On 12/12/2011 05:47 PM, Mauro Carvalho Chehab wrote: > Hi Linus, > > Please pull from: > git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media > v4l_for_linus > > For a couple fixes for media drivers. > The changes are: > - ati_remote (new driver for 3.2): Fix the scancode tables; > - af9015: fix some issues with firmware load; > - au0828: (trivial) add device ID's for some devices; > - omap3 and s5p: several small fixes; > - Update MAINTAINERS entry to cover /drivers/staging/media and > media DocBook; > - a few other small trivial fixes. > > Thanks! > Mauro [...] > > Sylwester Nawrocki (10): > [media] s5p-fimc: Fix wrong pointer dereference when unregistering > sensors > [media] s5p-fimc: Fix error in the capture subdev deinitialization > [media] s5p-fimc: Fix initialization for proper system suspend support > [media] s5p-fimc: Fix buffer dequeue order issue > [media] s5p-fimc: Allow probe() to succeed with null platform data > [media] s5p-fimc: Adjust pixel height alignments according to the IP > revision > [media] s5p-fimc: Fail driver probing when sensor configuration is wrong > [media] s5p-fimc: Use correct fourcc for RGB565 colour format > [media] m5mols: Fix set_fmt to return proper pixel format code > [media] s5p-fimc: Fix camera input configuration in subdev operations There is one more quite important patch left out: http://git.infradead.org/users/kmpark/linux-2.6-samsung/commitdiff/817069678ab57150b21cd0705e2f3a3b2b6509f4 It was included in my recent pull request. It would be good to have in v3.2 too. Are you planning to add it to a next pull request to Linus ? -- Thanks, Sylwester -- 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 02/11] mm: compaction: introduce isolate_{free,migrate}pages_range().
On Mon, Dec 12, 2011 at 05:46:13PM +0100, Michal Nazarewicz wrote:
> On Mon, 12 Dec 2011 17:30:52 +0100, Mel Gorman wrote:
>
> >On Mon, Dec 12, 2011 at 04:22:39PM +0100, Michal Nazarewicz wrote:
> >>>
> >>>
> + if (!pfn_valid_within(pfn))
> + goto skip;
> >>>
> >>>The flow of this function in general with gotos of skipped and next
> >>>is confusing in comparison to the existing function. For example,
> >>>if this PFN is not valid, and no freelist is provided, then we call
> >>>__free_page() on a PFN that is known to be invalid.
> >>>
> + ++nr_scanned;
> +
> + if (!PageBuddy(page)) {
> +skip:
> + if (freelist)
> + goto next;
> + for (; start < pfn; ++start)
> + __free_page(pfn_to_page(pfn));
> + return 0;
> + }
> >>>
> >>>So if a PFN is valid and !PageBuddy and no freelist is provided, we
> >>>call __free_page() on it regardless of reference count. That does not
> >>>sound safe.
> >>
> >>Sorry about that. It's a bug in the code which was caught later on. The
> >>code should read ???__free_page(pfn_to_page(start))???.
>
> On Mon, 12 Dec 2011 17:30:52 +0100, Mel Gorman wrote:
> >That will call free on valid PFNs but why is it safe to call
> >__free_page() at all? You say later that CMA requires that all
> >pages in the range be valid but if the pages are in use, that does
> >not mean that calling __free_page() is safe. I suspect you have not
> >seen a problem because the pages in the range were free as expected
> >and not in use because of MIGRATE_ISOLATE.
>
> All pages from [start, pfn) have passed through the loop body which
> means that they are valid and they have been removed from buddy (for
> caller's use). Also, because of split_free_page(), all of those pages
> have been split into 0-order pages.
Ah, I see. Even though you are not putting the pages on a freelist, the
function still returns with an elevated reference count and it's up to
the caller to find them again.
> Therefore, in error recovery, to
> undo what the loop has done so far, we put give back to buddy by
> calling __free_page() on each 0-order page.
>
Ok.
> /* Found a free page, break it into order-0 pages */
> isolated = split_free_page(page);
> total_isolated += isolated;
> - for (i = 0; i < isolated; i++) {
> - list_add(&page->lru, freelist);
> - page++;
> + if (freelist) {
> + struct page *p = page;
> + for (i = isolated; i; --i, ++p)
> + list_add(&p->lru, freelist);
> }
>
> - /* If a page was split, advance to the end of it */
> - if (isolated) {
> - blockpfn += isolated - 1;
> - cursor += isolated - 1;
> - }
> +next:
> + pfn += isolated;
> + page += isolated;
> >>>
> >>>The name isolated is now confusing because it can mean either
> >>>pages isolated or pages scanned depending on context. Your patch
> >>>appears to be doing a lot more than is necessary to convert
> >>>isolate_freepages_block into isolate_freepages_range and at this point,
> >>>it's unclear why you did that.
> >>
> >>When CMA uses this function, it requires all pages in the range to be valid
> >>and free. (Both conditions should be met but you never know.)
>
> To be clear, I meant that the CMA expects pages to be in buddy when the
> function
> is called but after the function finishes, all the pages in the range are
> removed
> from buddy. This, among other things, is why the call to split_free_page() is
> necessary.
>
Understood.
--
Mel Gorman
SUSE Labs
--
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: [GIT PULL for 3.2-rc5] media fixes
On 12/12/2011 06:47 PM, Mauro Carvalho Chehab wrote: Hi Linus, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media v4l_for_linus For a couple fixes for media drivers. The changes are: - ati_remote (new driver for 3.2): Fix the scancode tables; - af9015: fix some issues with firmware load; - au0828: (trivial) add device ID's for some devices; - omap3 and s5p: several small fixes; - Update MAINTAINERS entry to cover /drivers/staging/media and media DocBook; - a few other small trivial fixes. Thanks! Mauro - The following changes since commit dc47ce90c3a822cd7c9e9339fe4d5f61dcb26b50: Linux 3.2-rc5 (2011-12-09 15:09:32 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media v4l_for_linus Antti Palosaari (3): [media] af9015: limit I2C access to keep FW happy Hey, now there is misunderstanding. I don't want that patch to be 3.2. It is surely kinda a bug fix, but I see it too risky at that phase. Please delay that for the 3.3 and apply with my latest af9013 rewrite. [media] tda18218: fix 6 MHz default IF frequency [media] mxl5007t: fix reg read regards Antti -- http://palosaari.fi/ -- 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: [Linaro-mm-sig] [RFC v2 1/2] dma-buf: Introduce dma buffer sharing mechanism
On Saturday 10 December 2011, Daniel Vetter wrote: > If userspace (through some driver calls) > tries to do stupid things, it'll just get garbage. See > Message-ID: > > for my reasons why it think this is the right way to go forward. So in > essence I'm really interested in the reasons why you want the kernel > to enforce this (or I'm completely missing what's the contentious > issue here). This has nothing to do with user space mappings. Whatever user space does, you get garbage if you don't invalidate cache lines that were introduced through speculative prefetching before you access cache lines that were DMA'd from a device. Arnd -- 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
[GIT PULL for 3.2-rc5] media fixes
Hi Linus, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media v4l_for_linus For a couple fixes for media drivers. The changes are: - ati_remote (new driver for 3.2): Fix the scancode tables; - af9015: fix some issues with firmware load; - au0828: (trivial) add device ID's for some devices; - omap3 and s5p: several small fixes; - Update MAINTAINERS entry to cover /drivers/staging/media and media DocBook; - a few other small trivial fixes. Thanks! Mauro - The following changes since commit dc47ce90c3a822cd7c9e9339fe4d5f61dcb26b50: Linux 3.2-rc5 (2011-12-09 15:09:32 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media v4l_for_linus Anssi Hannula (1): [media] ati_remote: switch to single-byte scancodes Antti Palosaari (3): [media] af9015: limit I2C access to keep FW happy [media] tda18218: fix 6 MHz default IF frequency [media] mxl5007t: fix reg read Dan Carpenter (1): [media] V4L: mt9t112: use after free in mt9t112_probe() Dmitry Artamonow (1): [media] omap3isp: fix compilation of ispvideo.c Gary Thomas (1): [media] omap_vout: Fix compile error in 3.1 Guennadi Liakhovetski (2): [media] V4L: soc-camera: fix compiler warnings on 64-bit platforms [media] V4L: mt9m111: fix uninitialised mutex Janusz Krzysztofik (1): [media] V4L: omap1_camera: fix missing include Joe Perches (1): [media] [trivial] omap24xxcam-dma: Fix logical test Marek Szyprowski (1): [media] media: video: s5p-tv: fix build break Mauro Carvalho Chehab (1): MAINTAINERS: Update media entries Michael Krufky (3): [media] au0828: add missing USB ID 2040:7260 [media] au0828: add missing USB ID 2040:7213 [media] au0828: add missing models 72101, 72201 & 72261 to the model matrix Peter Korsgaard (1): [media] s5p_mfc_enc: fix s/H264/H263/ typo Randy Dunlap (1): [media] media/staging: fix allyesconfig build error Sylwester Nawrocki (10): [media] s5p-fimc: Fix wrong pointer dereference when unregistering sensors [media] s5p-fimc: Fix error in the capture subdev deinitialization [media] s5p-fimc: Fix initialization for proper system suspend support [media] s5p-fimc: Fix buffer dequeue order issue [media] s5p-fimc: Allow probe() to succeed with null platform data [media] s5p-fimc: Adjust pixel height alignments according to the IP revision [media] s5p-fimc: Fail driver probing when sensor configuration is wrong [media] s5p-fimc: Use correct fourcc for RGB565 colour format [media] m5mols: Fix set_fmt to return proper pixel format code [media] s5p-fimc: Fix camera input configuration in subdev operations Thomas Jarosch (1): [media] m5mols: Fix logic in sanity check Tomi Valkeinen (1): [media] omap_vout: fix crash if no driver for a display MAINTAINERS |2 + drivers/media/common/tuners/mxl5007t.c |3 +- drivers/media/common/tuners/tda18218.c |2 +- drivers/media/dvb/dvb-usb/af9015.c | 97 drivers/media/dvb/dvb-usb/af9015.h |7 + drivers/media/rc/ati_remote.c| 111 +-- drivers/media/rc/keymaps/rc-ati-x10.c| 96 drivers/media/rc/keymaps/rc-medion-x10.c | 128 +++--- drivers/media/rc/keymaps/rc-snapstream-firefly.c | 114 ++-- drivers/media/video/au0828/au0828-cards.c|7 + drivers/media/video/m5mols/m5mols.h |2 - drivers/media/video/m5mols/m5mols_core.c | 22 ++-- drivers/media/video/mt9m111.c|1 + drivers/media/video/mt9t112.c|4 +- drivers/media/video/omap/omap_vout.c |9 ++ drivers/media/video/omap1_camera.c |1 + drivers/media/video/omap24xxcam-dma.c|2 +- drivers/media/video/omap3isp/ispvideo.c |1 + drivers/media/video/ov6650.c |2 +- drivers/media/video/s5p-fimc/fimc-capture.c | 14 ++- drivers/media/video/s5p-fimc/fimc-core.c | 24 ++-- drivers/media/video/s5p-fimc/fimc-core.h |2 + drivers/media/video/s5p-fimc/fimc-mdevice.c | 43 +--- drivers/media/video/s5p-fimc/fimc-reg.c | 15 ++- drivers/media/video/s5p-mfc/s5p_mfc_enc.c|2 +- drivers/media/video/s5p-tv/mixer_video.c |1 + drivers/media/video/sh_mobile_ceu_camera.c | 34 -- drivers/media/video/sh_mobile_csi2.c |4 +- drivers/media/video/soc_camera.c |3 +- drivers/staging/media/as102/as102_drv.c |4 +- drivers/staging/media/as102/as102_drv.h |3 +- include/media/soc_camera.h |
Re: [PATCH 02/11] mm: compaction: introduce isolate_{free,migrate}pages_range().
On Mon, 12 Dec 2011 17:30:52 +0100, Mel Gorman wrote:
On Mon, Dec 12, 2011 at 04:22:39PM +0100, Michal Nazarewicz wrote:
>
>
>>+ if (!pfn_valid_within(pfn))
>>+ goto skip;
>
>The flow of this function in general with gotos of skipped and next
>is confusing in comparison to the existing function. For example,
>if this PFN is not valid, and no freelist is provided, then we call
>__free_page() on a PFN that is known to be invalid.
>
>>+ ++nr_scanned;
>>+
>>+ if (!PageBuddy(page)) {
>>+skip:
>>+ if (freelist)
>>+ goto next;
>>+ for (; start < pfn; ++start)
>>+ __free_page(pfn_to_page(pfn));
>>+ return 0;
>>+ }
>
>So if a PFN is valid and !PageBuddy and no freelist is provided, we
>call __free_page() on it regardless of reference count. That does not
>sound safe.
Sorry about that. It's a bug in the code which was caught later on. The
code should read ???__free_page(pfn_to_page(start))???.
On Mon, 12 Dec 2011 17:30:52 +0100, Mel Gorman wrote:
That will call free on valid PFNs but why is it safe to call
__free_page() at all? You say later that CMA requires that all
pages in the range be valid but if the pages are in use, that does
not mean that calling __free_page() is safe. I suspect you have not
seen a problem because the pages in the range were free as expected
and not in use because of MIGRATE_ISOLATE.
All pages from [start, pfn) have passed through the loop body which
means that they are valid and they have been removed from buddy (for
caller's use). Also, because of split_free_page(), all of those pages
have been split into 0-order pages. Therefore, in error recovery, to
undo what the loop has done so far, we put give back to buddy by
calling __free_page() on each 0-order page.
>>/* Found a free page, break it into order-0 pages */
>>isolated = split_free_page(page);
>>total_isolated += isolated;
>>- for (i = 0; i < isolated; i++) {
>>- list_add(&page->lru, freelist);
>>- page++;
>>+ if (freelist) {
>>+ struct page *p = page;
>>+ for (i = isolated; i; --i, ++p)
>>+ list_add(&p->lru, freelist);
>>}
>>
>>- /* If a page was split, advance to the end of it */
>>- if (isolated) {
>>- blockpfn += isolated - 1;
>>- cursor += isolated - 1;
>>- }
>>+next:
>>+ pfn += isolated;
>>+ page += isolated;
>
>The name isolated is now confusing because it can mean either
>pages isolated or pages scanned depending on context. Your patch
>appears to be doing a lot more than is necessary to convert
>isolate_freepages_block into isolate_freepages_range and at this point,
>it's unclear why you did that.
When CMA uses this function, it requires all pages in the range to be valid
and free. (Both conditions should be met but you never know.)
To be clear, I meant that the CMA expects pages to be in buddy when the function
is called but after the function finishes, all the pages in the range are
removed
from buddy. This, among other things, is why the call to split_free_page() is
necessary.
It seems racy but I guess you are depending on MIGRATE_ISOLATE to keep
things sane which is fine. However, I strongly suspect that if there
is a race and a page is in use, then you will need to retry the
migration step. Calling __free_page does not look right because
something still has a reference to the page.
This change
adds a second way isolate_freepages_range() works, which is when freelist is
not specified, abort on invalid or non-free page, but continue as usual if
freelist is provided.
Ok, I think you should be able to do that by not calling split_free_page
or adding to the list if !freelist with a comment explaining why the
pages are left on the buddy lists for the caller to figure out. Bail if
a page-in-use is found and have the caller check that the return value
of isolate_freepages_block == end_pfn - start_pfn.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--
--
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 02/11] mm: compaction: introduce isolate_{free,migrate}pages_range().
On Mon, Dec 12, 2011 at 04:22:39PM +0100, Michal Nazarewicz wrote:
> >
> >
> >>+ if (!pfn_valid_within(pfn))
> >>+ goto skip;
> >
> >The flow of this function in general with gotos of skipped and next
> >is confusing in comparison to the existing function. For example,
> >if this PFN is not valid, and no freelist is provided, then we call
> >__free_page() on a PFN that is known to be invalid.
> >
> >>+ ++nr_scanned;
> >>+
> >>+ if (!PageBuddy(page)) {
> >>+skip:
> >>+ if (freelist)
> >>+ goto next;
> >>+ for (; start < pfn; ++start)
> >>+ __free_page(pfn_to_page(pfn));
> >>+ return 0;
> >>+ }
> >
> >So if a PFN is valid and !PageBuddy and no freelist is provided, we
> >call __free_page() on it regardless of reference count. That does not
> >sound safe.
>
> Sorry about that. It's a bug in the code which was caught later on. The
> code should read ???__free_page(pfn_to_page(start))???.
>
That will call free on valid PFNs but why is it safe to call
__free_page() at all? You say later that CMA requires that all
pages in the range be valid but if the pages are in use, that does
not mean that calling __free_page() is safe. I suspect you have not
seen a problem because the pages in the range were free as expected
and not in use because of MIGRATE_ISOLATE.
> >>/* Found a free page, break it into order-0 pages */
> >>isolated = split_free_page(page);
> >>total_isolated += isolated;
> >>- for (i = 0; i < isolated; i++) {
> >>- list_add(&page->lru, freelist);
> >>- page++;
> >>+ if (freelist) {
> >>+ struct page *p = page;
> >>+ for (i = isolated; i; --i, ++p)
> >>+ list_add(&p->lru, freelist);
> >>}
> >>
> >>- /* If a page was split, advance to the end of it */
> >>- if (isolated) {
> >>- blockpfn += isolated - 1;
> >>- cursor += isolated - 1;
> >>- }
> >>+next:
> >>+ pfn += isolated;
> >>+ page += isolated;
> >
> >The name isolated is now confusing because it can mean either
> >pages isolated or pages scanned depending on context. Your patch
> >appears to be doing a lot more than is necessary to convert
> >isolate_freepages_block into isolate_freepages_range and at this point,
> >it's unclear why you did that.
>
> When CMA uses this function, it requires all pages in the range to be valid
> and free. (Both conditions should be met but you never know.)
It seems racy but I guess you are depending on MIGRATE_ISOLATE to keep
things sane which is fine. However, I strongly suspect that if there
is a race and a page is in use, then you will need to retry the
migration step. Calling __free_page does not look right because
something still has a reference to the page.
> This change
> adds a second way isolate_freepages_range() works, which is when freelist is
> not specified, abort on invalid or non-free page, but continue as usual if
> freelist is provided.
>
Ok, I think you should be able to do that by not calling split_free_page
or adding to the list if !freelist with a comment explaining why the
pages are left on the buddy lists for the caller to figure out. Bail if
a page-in-use is found and have the caller check that the return value
of isolate_freepages_block == end_pfn - start_pfn.
> I can try and restructure this function a bit so that there are fewer
> ???gotos???,
> but without the above change, CMA won't really be able to use it effectively
> (it would have to provide a freelist and then validate if pages on it are
> added in order).
>
Please do and double check that __free_page logic too.
--
Mel Gorman
SUSE Labs
--
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: cx231xx kernel oops
On Mon, December 12, 2011 8:22 am, Devin Heitmueller wrote: > On Mon, Dec 12, 2011 at 10:58 AM, Yan Seiner wrote: >>> Also, just to be clear, the USB Live 2 doesn't have any onboard >>> hardware compression. It has comparable requirements related to USB >>> bus utilization as any other USB framegrabber. The only possible >>> advantage you might get is that it does have an onboard scaler, so if >>> you're willing to compromise on quality you can change the capture >>> resolution to a lower value such as 320x240. Also, bear in mind that >>> the cx231xx driver may not be properly tuned to reduce the alternate >>> it uses dependent on resolution. To my knowledge that functionality >>> has not been thoroughly tested (as it's an unpopular use case). >> >> OK, thanks. I was hoping this was a hardware framegrabber; the info on >> the website is so ambiguous as to be nearly useless. > > I think you're just confused about the terminology. The term > "framegrabber" inherently means that it's delivering raw video (as > opposed to having onboard compression and providing MPEG or some other > compressed format). All framegrabbers are hardware framegrabbers. Aha. Thanks for the explanation. > > You may wish to look at the HVR-1950, which is well supported under > Linux and does deliver MPEG video. It's obviously more expensive that > the USB Live 2 and it has a tuner which you probably don't need, but > it does avoid the issue if you have USB bus constraints. I had looked at the HVR-1950 but the power consumption was prohibitive for my application. :-( --Yan -- Pain is temporary. It may last a minute, or an hour, or a day, or a year, but eventually it will subside and something else will take its place. If I quit, however, it lasts forever. -- 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/11] mm: compaction: export some of the functions
On Monday 12 December 2011, Mel Gorman wrote: > The bloat exists either way. I don't believe the linker strips it out so > overall it would make more sense to depend on compaction to keep the > vmstat counters for debugging reasons if nothing else. It's not > something I feel very strongly about though. There were some previous attempts to use -fgc-sections to strip out unused functions from the kernel, but I think they were never merged because of regressions. Arnd -- 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: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C
On 12-12-2011 13:00, Manu Abraham wrote:
On Mon, Dec 12, 2011 at 7:26 PM, Mauro Carvalho Chehab
wrote:
On 12-12-2011 11:40, Manu Abraham wrote:
On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
This also means that just doing an alias from FE_QAM and
SYS_DVBC_ANNEX_AC
to
SYS_DVBC_ANNEX_A may break something, as, for most devices,
SYS_DVBC_ANNEX_AC
really means both Annex A and C.
With the current approach, the application can determine whether
the hardware supports through the DELSYS enumeration.
So, if you have a device that needs to support both ANNEX_A and
ANNEX_C, it should be rather doing
case DTV_ENUM_DELSYS:
buffer.data[0] = SYS_DVBC_ANNEX_A;
buffer.data[1] = SYS_DVBC_ANNEX_C;
break;
Sure, but we'll need a logic to handle queries for SYS_DVBC_ANNEX_AC
anyway, if any of the existing DVB-C drivers is currently prepared to
support both.
I'm not concerned with drx-k. The support for both standards are for
kernel 3.3. So, no backward compatibility is needed here.
While there is no explicit option, the code for stv0297, stv0367,
tda10021 and tda10023 drivers are not clear if they support both
(maybe roll-off might be auto-detected?) or just SYS_DVBC_ANNEX_A.
That's said, the difference between a 0.15 and a 0.13 rolloff is not big.
I won't doubt that a demod set to 0.15 rolloff would be capable of working
(non-optimized) with a 0.13 rolloff.
What I'm saing is that, if any of the existing drivers currently works
with both Annex A and Annex C, we'll need something equivalent to:
if (delsys == SYS_DVBC_ANNEX_AC) {
int ret = try_annex_a();
if (ret< 0)
ret = try_annex_c();
}
For FE_SET_FRONTEND (and the corresponding v5 logic), in order to avoid
regressions.
What I was implying:
set_frontend/search()
{
case SYS_DVBC_ANNEX_A:
// do whatever you need to do for annex A tuning and return
break;
case SYS_DVBC_ANNEX_C:
// do whatever you need to do for annex C tuning and return
break;
}
ANNEX_AC is a link to ANNEX_A;
Yes, I saw your approach.
We never had any ? users to ANNEX_C, so
that issue might be simple to ignore.
This is hard to say. What I'm saying is that, if any of the current
drivers works as-is with Annex C, we should assume that someone is using,
as we don't have any evidence otherwise.
I'm sure there are lots of people running Linux in Japan.
How many of them are using the DVB subsystem is hard to say. The low message
traffic at the ML for people *.jp is not a good measure, as due to language
barriers, people may not be posting things there.
A quick grep here on my local copy of the ML traffic (it currently has stored
about 380 days of email, as I moved the older ones to a separate storage space)
still shows 90 messages that has ".jp" inside:
$ grep -l "\.jp" * |wc -l
90
41 of them has the word DVB inside. Ok, there are some false positives there
too (due to *.jpg), but there are some valid hits also,
Including a commit on this changeset: e38030f3ff02684eb9e25e983a03ad318a10a2ea.
As the cx23885 driver does support DVB-C with stv0367, maybe the committer
might be using it for DVB-C.
Even if not, I suspect that it is likely to have some DVB-C Annex C users
out there.
Regards,
Mauro.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cx231xx kernel oops
On Mon, Dec 12, 2011 at 10:58 AM, Yan Seiner wrote: >> Also, just to be clear, the USB Live 2 doesn't have any onboard >> hardware compression. It has comparable requirements related to USB >> bus utilization as any other USB framegrabber. The only possible >> advantage you might get is that it does have an onboard scaler, so if >> you're willing to compromise on quality you can change the capture >> resolution to a lower value such as 320x240. Also, bear in mind that >> the cx231xx driver may not be properly tuned to reduce the alternate >> it uses dependent on resolution. To my knowledge that functionality >> has not been thoroughly tested (as it's an unpopular use case). > > OK, thanks. I was hoping this was a hardware framegrabber; the info on > the website is so ambiguous as to be nearly useless. I think you're just confused about the terminology. The term "framegrabber" inherently means that it's delivering raw video (as opposed to having onboard compression and providing MPEG or some other compressed format). All framegrabbers are hardware framegrabbers. There were some *really* old devices that delivered the frames with JPEG or proprietary compression so that they fit within USB 1.1, but those designs are almost entirely gone given the hardware cost and the lack of need since almost everything nowadays is USB 2.0. You may wish to look at the HVR-1950, which is well supported under Linux and does deliver MPEG video. It's obviously more expensive that the USB Live 2 and it has a tuner which you probably don't need, but it does avoid the issue if you have USB bus constraints. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.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 v4 0/3] fbdev: Add FOURCC-based format configuration API
Hi Florian, On Tuesday 29 November 2011 11:26:56 Laurent Pinchart wrote: > Hi everybody, > > Here's the fourth version of the fbdev FOURCC-based format configuration > API. Is there a chance this will make it to v3.3 ? -- 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
UK Sudbury post Digital Switchover updated scan data.
Hello, [dvb-t/uk-Sudbury] has now gone Digital Switchover. I wonder if the scan data could be updated? Is this the correct way to do it, I noticed someone else supplied their changes as a diff file format? Thanks, Steve. #-- # file automatically generated by w_scan # (http://wirbel.htpc-forum.de/w_scan/index2.html) #! 20110702 1 0 OFDM GB #-- # location and provider: <[dvb-t/uk-Sudbury]> # date (-mm-dd): 2011-11-25 # provided by (opt): # # T[2] [# comment] #-- T 63400 8MHz AUTO AUTO AUTO AUTO AUTO NONE # East Anglia.. T 65800 8MHz AUTO AUTO AUTO AUTO AUTO NONE # East Anglia.. T 698167000 8MHz AUTO AUTO AUTO AUTO AUTO NONE # East Anglia.. T 73800 8MHz AUTO AUTO AUTO AUTO AUTO NONE # East Anglia.. T 81000 8MHz AUTO AUTO AUTO AUTO AUTO NONE # East Anglia.. -- 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: cx231xx kernel oops
On Mon, December 12, 2011 6:23 am, Devin Heitmueller wrote: > > For what it's worth, I did do quite a bit of work on cx231xx, > including work for mips and arm platforms. That said, all the work > done was on the control interfaces rather than the buffer management > (my particular use case didn't have the video coming back over the USB > bus). > > How does your app setup the buffers? Is it doing MMAP? Userptr? > It's possible userptr support is broken, as that's something that is > much less common. > > And as Andy suggested, if you can test your app under x86, knowing > whether the app works with cx231xx under x86 is useful in knowing if > you have a mips issue or something that your app in particular is > doing. OK, I just tried this with motion (still on the MIPS platform), a totally different app. It tries to allocate a reasonable amount of memory: [1] mmap information: [1] frames=4 [1] 0 length=153600 [1] 1 length=153600 [1] 2 length=153600 [1] 3 length=153600 [1] buffer index 0 VIDIOC_QBUF: Cannot allocate memory [1] ioctl (VIDIOCGCAP): Invalid argument but the driver still tries to grab 800MB. So it's somewhere between the app and the driver. > > Also, just to be clear, the USB Live 2 doesn't have any onboard > hardware compression. It has comparable requirements related to USB > bus utilization as any other USB framegrabber. The only possible > advantage you might get is that it does have an onboard scaler, so if > you're willing to compromise on quality you can change the capture > resolution to a lower value such as 320x240. Also, bear in mind that > the cx231xx driver may not be properly tuned to reduce the alternate > it uses dependent on resolution. To my knowledge that functionality > has not been thoroughly tested (as it's an unpopular use case). OK, thanks. I was hoping this was a hardware framegrabber; the info on the website is so ambiguous as to be nearly useless. > > And finally, there were fixes for the USB Live 2 specifically which > you may not have in 3.0.3. You should check the changelogs. It's > possible that the failure to set the USB alternate is leaving the > driver is an unknown state, which causes it to crash once actually > trying to allocate the buffers. Will do. -- Pain is temporary. It may last a minute, or an hour, or a day, or a year, but eventually it will subside and something else will take its place. If I quit, however, it lasts forever. -- 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 03/11] mm: mmzone: introduce zone_pfn_same_memmap()
On Mon, Dec 12, 2011 at 03:51:55PM +0100, Michal Nazarewicz wrote: > >On Fri, Nov 18, 2011 at 05:43:10PM +0100, Marek Szyprowski wrote: > >>From: Michal Nazarewicz > >>diff --git a/mm/compaction.c b/mm/compaction.c > >>index 6afae0e..09c9702 100644 > >>--- a/mm/compaction.c > >>+++ b/mm/compaction.c > >>@@ -111,7 +111,10 @@ skip: > >> > >> next: > >>pfn += isolated; > >>- page += isolated; > >>+ if (zone_pfn_same_memmap(pfn - isolated, pfn)) > >>+ page += isolated; > >>+ else > >>+ page = pfn_to_page(pfn); > >>} > > On Mon, 12 Dec 2011 15:19:53 +0100, Mel Gorman wrote: > >Is this necessary? > > > >We are isolating pages, the largest of which is a MAX_ORDER_NR_PAGES > >page. [...] > > On Mon, 12 Dec 2011 15:40:30 +0100, Mel Gorman wrote: > >To be clear, I'm referring to a single page being isolated here. It may > >or may not be a high-order page but it's still going to be less then > >MAX_ORDER_NR_PAGES so you should be able check when a new block is > >entered and pfn_to_page is necessary. > > Do you mean something like: > > if (same pageblock) > just do arithmetic; > else > use pfn_to_page; > something like the following untested snippet. /* * Resolve pfn_to_page every MAX_ORDER_NR_PAGES to handle the case where * memmap is not contiguous such as with SPARSEMEM memory model without * VMEMMAP */ pfn += isolated; page += isolated; if ((pfn & ~(MAX_ORDER_NR_PAGES-1)) == 0) page = pfn_to_page(pfn); That would be closer to what other PFN walkers do > ? > > I've discussed it with Dave and he suggested that approach as an > optimisation since in some configurations zone_pfn_same_memmap() > is always true thus compiler will strip the else part, whereas > same pageblock test will be false on occasions regardless of kernel > configuration. > Ok, while I recognise it's an optimisation, it's a very small optimisation and I'm not keen on introducing something new for CMA that has been coped with in the past by always walking PFNs in pageblock-sized ranges with pfn_valid checks where necessary. See setup_zone_migrate_reserve as one example where pfn_to_page is only called once per pageblock and calls pageblock_is_reserved() for examining pages within a pageblock. Still, if you really want the helper, at least keep it in compaction.c as there should be no need to have it in mmzone.h -- Mel Gorman SUSE Labs -- 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/11] mm: compaction: export some of the functions
On Mon, 12 Dec 2011 15:29:07 +0100, Mel Gorman wrote: Overall, this patch implies that CMA is always compiled in. On Mon, Dec 12, 2011 at 03:41:04PM +0100, Michal Nazarewicz wrote: Not really. But yes, it produces some bloat when neither CMA nor compaction are compiled. I assume that linker will be able to deal with that (since the functions are not EXPORT_SYMBOL'ed). On Mon, 12 Dec 2011 16:40:15 +0100, Mel Gorman wrote: The bloat exists either way. I don't believe the linker strips it out so overall it would make more sense to depend on compaction to keep the vmstat counters for debugging reasons if nothing else. It's not something I feel very strongly about though. KK, I'll do that then. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- -- 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/11] mm: compaction: export some of the functions
On Mon, Dec 12, 2011 at 03:41:04PM +0100, Michal Nazarewicz wrote: > On Mon, 12 Dec 2011 15:29:07 +0100, Mel Gorman wrote: > > >On Fri, Nov 18, 2011 at 05:43:11PM +0100, Marek Szyprowski wrote: > >>From: Michal Nazarewicz > >> > >>This commit exports some of the functions from compaction.c file > >>outside of it adding their declaration into internal.h header > >>file so that other mm related code can use them. > >> > >>This forced compaction.c to always be compiled (as opposed to being > >>compiled only if CONFIG_COMPACTION is defined) but as to avoid > >>introducing code that user did not ask for, part of the compaction.c > >>is now wrapped in on #ifdef. > >> > >>Signed-off-by: Michal Nazarewicz > >>Signed-off-by: Marek Szyprowski > >>--- > >> mm/Makefile |3 +- > >> mm/compaction.c | 112 > >> +++ > >> mm/internal.h | 35 + > >> 3 files changed, 83 insertions(+), 67 deletions(-) > >> > >>diff --git a/mm/Makefile b/mm/Makefile > >>index 50ec00e..24ed801 100644 > >>--- a/mm/Makefile > >>+++ b/mm/Makefile > >>@@ -13,7 +13,7 @@ obj-y := filemap.o mempool.o > >>oom_kill.o fadvise.o \ > >> readahead.o swap.o truncate.o vmscan.o shmem.o \ > >> prio_tree.o util.o mmzone.o vmstat.o backing-dev.o \ > >> page_isolation.o mm_init.o mmu_context.o percpu.o \ > >>- $(mmu-y) > >>+ $(mmu-y) compaction.o > > > >That should be > > > >compaction.o $(mmu-y) > > > >for consistency. > > > >Overall, this patch implies that CMA is always compiled in. > > Not really. But yes, it produces some bloat when neither CMA nor > compaction are compiled. I assume that linker will be able to deal > with that (since the functions are not EXPORT_SYMBOL'ed). > The bloat exists either way. I don't believe the linker strips it out so overall it would make more sense to depend on compaction to keep the vmstat counters for debugging reasons if nothing else. It's not something I feel very strongly about though. -- Mel Gorman SUSE Labs -- 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 02/11] mm: compaction: introduce isolate_{free,migrate}pages_range().
On Fri, Nov 18, 2011 at 05:43:09PM +0100, Marek Szyprowski wrote:
From: Michal Nazarewicz
diff --git a/mm/compaction.c b/mm/compaction.c
index 899d956..6afae0e 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -54,51 +54,64 @@ static unsigned long release_freepages(struct list_head
*freelist)
return count;
}
-/* Isolate free pages onto a private freelist. Must hold zone->lock */
-static unsigned long isolate_freepages_block(struct zone *zone,
- unsigned long blockpfn,
- struct list_head *freelist)
+/**
+ * isolate_freepages_range() - isolate free pages, must hold zone->lock.
+ * @zone: Zone pages are in.
+ * @start: The first PFN to start isolating.
+ * @end: The one-past-last PFN.
+ * @freelist: A list to save isolated pages to.
+ *
+ * If @freelist is not provided, holes in range (either non-free pages
+ * or invalid PFNs) are considered an error and function undos its
+ * actions and returns zero.
+ *
+ * If @freelist is provided, function will simply skip non-free and
+ * missing pages and put only the ones isolated on the list.
+ *
+ * Returns number of isolated pages. This may be more then end-start
+ * if end fell in a middle of a free page.
+ */
+static unsigned long
+isolate_freepages_range(struct zone *zone,
+ unsigned long start, unsigned long end,
+ struct list_head *freelist)
On Mon, 12 Dec 2011 15:07:28 +0100, Mel Gorman wrote:
Use start_pfn and end_pfn to keep it consistent with the rest of
compaction.c.
Will do.
{
- unsigned long zone_end_pfn, end_pfn;
- int nr_scanned = 0, total_isolated = 0;
- struct page *cursor;
-
- /* Get the last PFN we should scan for free pages at */
- zone_end_pfn = zone->zone_start_pfn + zone->spanned_pages;
- end_pfn = min(blockpfn + pageblock_nr_pages, zone_end_pfn);
+ unsigned long nr_scanned = 0, total_isolated = 0;
+ unsigned long pfn = start;
+ struct page *page;
- /* Find the first usable PFN in the block to initialse page cursor */
- for (; blockpfn < end_pfn; blockpfn++) {
- if (pfn_valid_within(blockpfn))
- break;
- }
- cursor = pfn_to_page(blockpfn);
+ VM_BUG_ON(!pfn_valid(pfn));
+ page = pfn_to_page(pfn);
/* Isolate free pages. This assumes the block is valid */
- for (; blockpfn < end_pfn; blockpfn++, cursor++) {
- int isolated, i;
- struct page *page = cursor;
-
- if (!pfn_valid_within(blockpfn))
- continue;
- nr_scanned++;
-
- if (!PageBuddy(page))
- continue;
+ while (pfn < end) {
+ unsigned isolated = 1, i;
+
Do not use implcit types. These are unsigned ints, call them unsigned
ints.
Will do.
+ if (!pfn_valid_within(pfn))
+ goto skip;
The flow of this function in general with gotos of skipped and next
is confusing in comparison to the existing function. For example,
if this PFN is not valid, and no freelist is provided, then we call
__free_page() on a PFN that is known to be invalid.
+ ++nr_scanned;
+
+ if (!PageBuddy(page)) {
+skip:
+ if (freelist)
+ goto next;
+ for (; start < pfn; ++start)
+ __free_page(pfn_to_page(pfn));
+ return 0;
+ }
So if a PFN is valid and !PageBuddy and no freelist is provided, we
call __free_page() on it regardless of reference count. That does not
sound safe.
Sorry about that. It's a bug in the code which was caught later on. The
code should read “__free_page(pfn_to_page(start))”.
/* Found a free page, break it into order-0 pages */
isolated = split_free_page(page);
total_isolated += isolated;
- for (i = 0; i < isolated; i++) {
- list_add(&page->lru, freelist);
- page++;
+ if (freelist) {
+ struct page *p = page;
+ for (i = isolated; i; --i, ++p)
+ list_add(&p->lru, freelist);
}
- /* If a page was split, advance to the end of it */
- if (isolated) {
- blockpfn += isolated - 1;
- cursor += isolated - 1;
- }
+next:
+ pfn += isolated;
+ page += isolated;
The name isolated is now confusing because it can mean either
pages isolated or pages scanned depending on context. Your patch
appears to be doing a lot more than is necessary to convert
isolate_freepages_block into isolate_freepages_range and at this point,
it's unclear why you did that.
When CMA uses this func
Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C
On Mon, Dec 12, 2011 at 7:26 PM, Mauro Carvalho Chehab
wrote:
> On 12-12-2011 11:40, Manu Abraham wrote:
>>
>> On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
>
>
>>> This also means that just doing an alias from FE_QAM and
>>> SYS_DVBC_ANNEX_AC
>>> to
>>> SYS_DVBC_ANNEX_A may break something, as, for most devices,
>>> SYS_DVBC_ANNEX_AC
>>> really means both Annex A and C.
>>
>>
>>
>>
>> With the current approach, the application can determine whether
>> the hardware supports through the DELSYS enumeration.
>>
>> So, if you have a device that needs to support both ANNEX_A and
>> ANNEX_C, it should be rather doing
>>
>> case DTV_ENUM_DELSYS:
>> buffer.data[0] = SYS_DVBC_ANNEX_A;
>> buffer.data[1] = SYS_DVBC_ANNEX_C;
>> break;
>
>
> Sure, but we'll need a logic to handle queries for SYS_DVBC_ANNEX_AC
> anyway, if any of the existing DVB-C drivers is currently prepared to
> support both.
>
> I'm not concerned with drx-k. The support for both standards are for
> kernel 3.3. So, no backward compatibility is needed here.
>
> While there is no explicit option, the code for stv0297, stv0367,
> tda10021 and tda10023 drivers are not clear if they support both
> (maybe roll-off might be auto-detected?) or just SYS_DVBC_ANNEX_A.
>
> That's said, the difference between a 0.15 and a 0.13 rolloff is not big.
> I won't doubt that a demod set to 0.15 rolloff would be capable of working
> (non-optimized) with a 0.13 rolloff.
>
> What I'm saing is that, if any of the existing drivers currently works
> with both Annex A and Annex C, we'll need something equivalent to:
>
> if (delsys == SYS_DVBC_ANNEX_AC) {
> int ret = try_annex_a();
> if (ret < 0)
> ret = try_annex_c();
> }
>
> For FE_SET_FRONTEND (and the corresponding v5 logic), in order to avoid
> regressions.
What I was implying:
set_frontend/search()
{
case SYS_DVBC_ANNEX_A:
// do whatever you need to do for annex A tuning and return
break;
case SYS_DVBC_ANNEX_C:
// do whatever you need to do for annex C tuning and return
break;
}
ANNEX_AC is a link to ANNEX_A; We never had any ? users to ANNEX_C, so
that issue might be simple to ignore.
--
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 03/11] mm: mmzone: introduce zone_pfn_same_memmap()
On Fri, Nov 18, 2011 at 05:43:10PM +0100, Marek Szyprowski wrote: From: Michal Nazarewicz diff --git a/mm/compaction.c b/mm/compaction.c index 6afae0e..09c9702 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -111,7 +111,10 @@ skip: next: pfn += isolated; - page += isolated; + if (zone_pfn_same_memmap(pfn - isolated, pfn)) + page += isolated; + else + page = pfn_to_page(pfn); } On Mon, 12 Dec 2011 15:19:53 +0100, Mel Gorman wrote: Is this necessary? We are isolating pages, the largest of which is a MAX_ORDER_NR_PAGES page. [...] On Mon, 12 Dec 2011 15:40:30 +0100, Mel Gorman wrote: To be clear, I'm referring to a single page being isolated here. It may or may not be a high-order page but it's still going to be less then MAX_ORDER_NR_PAGES so you should be able check when a new block is entered and pfn_to_page is necessary. Do you mean something like: if (same pageblock) just do arithmetic; else use pfn_to_page; ? I've discussed it with Dave and he suggested that approach as an optimisation since in some configurations zone_pfn_same_memmap() is always true thus compiler will strip the else part, whereas same pageblock test will be false on occasions regardless of kernel configuration. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- -- 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 01/11] mm: page_alloc: handle MIGRATE_ISOLATE in free_pcppages_bulk()
On Mon, Dec 12, 2011 at 03:23:02PM +0100, Michal Nazarewicz wrote: > >On Fri, Nov 18, 2011 at 05:43:08PM +0100, Marek Szyprowski wrote: > >>From: Michal Nazarewicz > >>diff --git a/mm/page_alloc.c b/mm/page_alloc.c > >>index 9dd443d..58d1a2e 100644 > >>--- a/mm/page_alloc.c > >>+++ b/mm/page_alloc.c > >>@@ -628,6 +628,18 @@ static void free_pcppages_bulk(struct zone *zone, int > >>count, > >>page = list_entry(list->prev, struct page, lru); > >>/* must delete as __free_one_page list manipulates */ > >>list_del(&page->lru); > >>+ > >>+ /* > >>+* When page is isolated in set_migratetype_isolate() > >>+* function it's page_private is not changed since the > >>+* function has no way of knowing if it can touch it. > >>+* This means that when a page is on PCP list, it's > >>+* page_private no longer matches the desired migrate > >>+* type. > >>+*/ > >>+ if (get_pageblock_migratetype(page) == MIGRATE_ISOLATE) > >>+ set_page_private(page, MIGRATE_ISOLATE); > >>+ > > On Mon, 12 Dec 2011 14:42:35 +0100, Mel Gorman wrote: > >How much of a problem is this in practice? > > IIRC, this lead to allocation being made from area marked as isolated > or some such. > And I believe that nothing prevents that from happening. I was just wondering how common it was in practice. Draining the per-cpu lists should work as a substitute either way. -- Mel Gorman SUSE Labs -- 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/11] mm: compaction: export some of the functions
On Mon, 12 Dec 2011 15:29:07 +0100, Mel Gorman wrote: On Fri, Nov 18, 2011 at 05:43:11PM +0100, Marek Szyprowski wrote: From: Michal Nazarewicz This commit exports some of the functions from compaction.c file outside of it adding their declaration into internal.h header file so that other mm related code can use them. This forced compaction.c to always be compiled (as opposed to being compiled only if CONFIG_COMPACTION is defined) but as to avoid introducing code that user did not ask for, part of the compaction.c is now wrapped in on #ifdef. Signed-off-by: Michal Nazarewicz Signed-off-by: Marek Szyprowski --- mm/Makefile |3 +- mm/compaction.c | 112 +++ mm/internal.h | 35 + 3 files changed, 83 insertions(+), 67 deletions(-) diff --git a/mm/Makefile b/mm/Makefile index 50ec00e..24ed801 100644 --- a/mm/Makefile +++ b/mm/Makefile @@ -13,7 +13,7 @@ obj-y := filemap.o mempool.o oom_kill.o fadvise.o \ readahead.o swap.o truncate.o vmscan.o shmem.o \ prio_tree.o util.o mmzone.o vmstat.o backing-dev.o \ page_isolation.o mm_init.o mmu_context.o percpu.o \ - $(mmu-y) + $(mmu-y) compaction.o That should be compaction.o $(mmu-y) for consistency. Overall, this patch implies that CMA is always compiled in. Not really. But yes, it produces some bloat when neither CMA nor compaction are compiled. I assume that linker will be able to deal with that (since the functions are not EXPORT_SYMBOL'ed). Note also that the was majority of compaction.c is #ifdef'd though so only a handful of functions are compiled. Why not just make CMA depend on COMPACTION to keep things simplier? I could imagine that someone would want to have CMA but not compaction, hence I decided to give that choice. For example, if you enable CMA and do not enable COMPACTION, you lose things like the vmstat counters that can aid debugging. In fact, as parts of compaction.c are using defines like COMPACTBLOCKS, I'm not even sure compaction.c can compile without CONFIG_COMPACTION because of the vmstat stuff. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- -- 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 03/11] mm: mmzone: introduce zone_pfn_same_memmap()
On Mon, Dec 12, 2011 at 03:35:03PM +0100, Michal Nazarewicz wrote: > >On Fri, Nov 18, 2011 at 05:43:10PM +0100, Marek Szyprowski wrote: > >>From: Michal Nazarewicz > >>diff --git a/mm/compaction.c b/mm/compaction.c > >>index 6afae0e..09c9702 100644 > >>--- a/mm/compaction.c > >>+++ b/mm/compaction.c > >>@@ -111,7 +111,10 @@ skip: > >> > >> next: > >>pfn += isolated; > >>- page += isolated; > >>+ if (zone_pfn_same_memmap(pfn - isolated, pfn)) > >>+ page += isolated; > >>+ else > >>+ page = pfn_to_page(pfn); > >>} > > On Mon, 12 Dec 2011 15:19:53 +0100, Mel Gorman wrote: > >Is this necessary? > > > >We are isolating pages, the largest of which is a MAX_ORDER_NR_PAGES > >page. [...] > > This is not true for CMA. > To be clear, I'm referring to a single page being isolated here. It may or may not be a high-order page but it's still going to be less then MAX_ORDER_NR_PAGES so you should be able check when a new block is entered and pfn_to_page is necessary. -- Mel Gorman SUSE Labs -- 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 v3 4/4] v4l: Update subdev drivers to handle framesamples parameter
Hi Laurent,
On 12/12/2011 01:31 AM, Laurent Pinchart wrote:
> On Friday 09 December 2011 18:59:52 Sylwester Nawrocki wrote:
>> Update the sub-device drivers having a devnode enabled so they properly
>> handle the new framesamples field of struct v4l2_mbus_framefmt.
>> These drivers don't support compressed (entropy encoded) formats so the
>> framesamples field is simply initialized to 0, altogether with the
>> reserved structure member.
>>
>> There is a few other drivers that expose a devnode (mt9p031, mt9t001,
>> mt9v032), but they already implicitly initialize the new data structure
>> field to 0, so they don't need to be touched.
>>
>> Signed-off-by: Sylwester Nawrocki
>> Signed-off-by: Kyungmin Park
>> ---
>> Hi,
>>
>> In this version the whole reserved field in struct v4l2_mbus_framefmt
>> is also cleared, rather than setting only framesamples to 0.
>>
>> The omap3isp driver changes have been only compile tested.
>>
>> Thanks,
>> Sylwester
>> ---
>> drivers/media/video/noon010pc30.c |5 -
>> drivers/media/video/omap3isp/ispccdc.c|2 ++
>> drivers/media/video/omap3isp/ispccp2.c|2 ++
>> drivers/media/video/omap3isp/ispcsi2.c|2 ++
>> drivers/media/video/omap3isp/isppreview.c |2 ++
>> drivers/media/video/omap3isp/ispresizer.c |2 ++
>> drivers/media/video/s5k6aa.c |2 ++
>> 7 files changed, 16 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/media/video/noon010pc30.c
>> b/drivers/media/video/noon010pc30.c index 50838bf..5af9b60 100644
>> --- a/drivers/media/video/noon010pc30.c
>> +++ b/drivers/media/video/noon010pc30.c
>> @@ -519,13 +519,14 @@ static int noon010_get_fmt(struct v4l2_subdev *sd,
>> struct v4l2_subdev_fh *fh, mf = &fmt->format;
>>
>> mutex_lock(&info->lock);
>> +memset(mf, 0, sizeof(mf));
>> mf->width = info->curr_win->width;
>> mf->height = info->curr_win->height;
>> mf->code = info->curr_fmt->code;
>> mf->colorspace = info->curr_fmt->colorspace;
>> mf->field = V4L2_FIELD_NONE;
>> -
>> mutex_unlock(&info->lock);
>> +
>> return 0;
>> }
>>
>> @@ -546,12 +547,14 @@ static const struct noon010_format
>> *noon010_try_fmt(struct v4l2_subdev *sd, static int noon010_set_fmt(struct
>> v4l2_subdev *sd, struct v4l2_subdev_fh *fh, struct v4l2_subdev_format
>> *fmt)
>> {
>> +const int offset = offsetof(struct v4l2_mbus_framefmt, framesamples);
>> struct noon010_info *info = to_noon010(sd);
>> const struct noon010_frmsize *size = NULL;
>> const struct noon010_format *nf;
>> struct v4l2_mbus_framefmt *mf;
>> int ret = 0;
>>
>> +memset(&fmt->format + offset, 0, sizeof(fmt->format) - offset);
>
> I'm not sure this is a good idea, as it will break when a new field will be
> added to struct v4l2_mbus_framefmt.
I'm not sure it will break. Now there everything cleared after (and including)
framesamples field.
struct v4l2_mbus_framefmt {
__u32 width;
__u32 height;
__u32 code;
__u32 field;
__u32 colorspace;
__u32 framesamples;
__u32 reserved[6];
};
Assuming we convert reserved[0] to new_field
struct v4l2_mbus_framefmt {
__u32 width;
__u32 height;
__u32 code;
__u32 field;
__u32 colorspace;
__u32 framesamples;
__u32 new_field;
__u32 reserved[5];
};
the code:
const int offset = offsetof(struct v4l2_mbus_framefmt, framesamples);
memset(&fmt->format + offset, 0, sizeof(fmt->format) - offset);
would still clear 7 u32' at the structure end, wouldn't it?
>
> Wouldn't it be better to zero the whoel structure in the callers instead ?
--
Regards
Sylwester
--
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 03/11] mm: mmzone: introduce zone_pfn_same_memmap()
On Fri, Nov 18, 2011 at 05:43:10PM +0100, Marek Szyprowski wrote: From: Michal Nazarewicz diff --git a/mm/compaction.c b/mm/compaction.c index 6afae0e..09c9702 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -111,7 +111,10 @@ skip: next: pfn += isolated; - page += isolated; + if (zone_pfn_same_memmap(pfn - isolated, pfn)) + page += isolated; + else + page = pfn_to_page(pfn); } On Mon, 12 Dec 2011 15:19:53 +0100, Mel Gorman wrote: Is this necessary? We are isolating pages, the largest of which is a MAX_ORDER_NR_PAGES page. [...] This is not true for CMA. That said, everywhere else managed to avoid checks like this by always scanning in units of pageblocks. Maybe this should be structured the same way to guarantee pfn_valid is called at least per pageblock (even though only once per MAX_ORDER_NR_PAGES is necessary). I'll look into that. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- -- 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/11] mm: compaction: export some of the functions
On Fri, Nov 18, 2011 at 05:43:11PM +0100, Marek Szyprowski wrote: > From: Michal Nazarewicz > > This commit exports some of the functions from compaction.c file > outside of it adding their declaration into internal.h header > file so that other mm related code can use them. > > This forced compaction.c to always be compiled (as opposed to being > compiled only if CONFIG_COMPACTION is defined) but as to avoid > introducing code that user did not ask for, part of the compaction.c > is now wrapped in on #ifdef. > > Signed-off-by: Michal Nazarewicz > Signed-off-by: Marek Szyprowski > --- > mm/Makefile |3 +- > mm/compaction.c | 112 > +++ > mm/internal.h | 35 + > 3 files changed, 83 insertions(+), 67 deletions(-) > > diff --git a/mm/Makefile b/mm/Makefile > index 50ec00e..24ed801 100644 > --- a/mm/Makefile > +++ b/mm/Makefile > @@ -13,7 +13,7 @@ obj-y := filemap.o mempool.o > oom_kill.o fadvise.o \ > readahead.o swap.o truncate.o vmscan.o shmem.o \ > prio_tree.o util.o mmzone.o vmstat.o backing-dev.o \ > page_isolation.o mm_init.o mmu_context.o percpu.o \ > -$(mmu-y) > +$(mmu-y) compaction.o That should be compaction.o $(mmu-y) for consistency. Overall, this patch implies that CMA is always compiled in. Why not just make CMA depend on COMPACTION to keep things simplier? For example, if you enable CMA and do not enable COMPACTION, you lose things like the vmstat counters that can aid debugging. In fact, as parts of compaction.c are using defines like COMPACTBLOCKS, I'm not even sure compaction.c can compile without CONFIG_COMPACTION because of the vmstat stuff. -- Mel Gorman SUSE Labs -- 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: cx231xx kernel oops
On Mon, Dec 12, 2011 at 7:46 AM, Yan Seiner wrote: > Andy Walls wrote: >> >> 800 MB for 320x420 frames? It sounds like your app has gooned its >> requested buffer size. >> > > > That's an understatement. :-) > > >> >> This might be due to endianess differences between MIPS abd x86 and your >> app only being written and tested on x86. >> >> > > > My speculation too. I don't know where that number comes from; the same app > works fine with the saa7115 driver if I switch frame grabbers. I'll have to > do some fiddling with the code to figure out where the problem lies. It's > some interaction between the app and the cx231xx driver. > > > > >> You still appear to USB stack problems, but not as severe (can't change >> device config to some bogus config). >> > > > The requested buffer size is the result of multiplying max_pkt_size * > max_packets and the rejected config shows a max_packet_size of 0, maybe > ithere;'s a problem with either endianness or int size... ??? Something to > follow up on. For what it's worth, I did do quite a bit of work on cx231xx, including work for mips and arm platforms. That said, all the work done was on the control interfaces rather than the buffer management (my particular use case didn't have the video coming back over the USB bus). How does your app setup the buffers? Is it doing MMAP? Userptr? It's possible userptr support is broken, as that's something that is much less common. And as Andy suggested, if you can test your app under x86, knowing whether the app works with cx231xx under x86 is useful in knowing if you have a mips issue or something that your app in particular is doing. Also, just to be clear, the USB Live 2 doesn't have any onboard hardware compression. It has comparable requirements related to USB bus utilization as any other USB framegrabber. The only possible advantage you might get is that it does have an onboard scaler, so if you're willing to compromise on quality you can change the capture resolution to a lower value such as 320x240. Also, bear in mind that the cx231xx driver may not be properly tuned to reduce the alternate it uses dependent on resolution. To my knowledge that functionality has not been thoroughly tested (as it's an unpopular use case). And finally, there were fixes for the USB Live 2 specifically which you may not have in 3.0.3. You should check the changelogs. It's possible that the failure to set the USB alternate is leaving the driver is an unknown state, which causes it to crash once actually trying to allocate the buffers. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.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 01/11] mm: page_alloc: handle MIGRATE_ISOLATE in free_pcppages_bulk()
On Fri, Nov 18, 2011 at 05:43:08PM +0100, Marek Szyprowski wrote: From: Michal Nazarewicz diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 9dd443d..58d1a2e 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -628,6 +628,18 @@ static void free_pcppages_bulk(struct zone *zone, int count, page = list_entry(list->prev, struct page, lru); /* must delete as __free_one_page list manipulates */ list_del(&page->lru); + + /* +* When page is isolated in set_migratetype_isolate() +* function it's page_private is not changed since the +* function has no way of knowing if it can touch it. +* This means that when a page is on PCP list, it's +* page_private no longer matches the desired migrate +* type. +*/ + if (get_pageblock_migratetype(page) == MIGRATE_ISOLATE) + set_page_private(page, MIGRATE_ISOLATE); + On Mon, 12 Dec 2011 14:42:35 +0100, Mel Gorman wrote: How much of a problem is this in practice? IIRC, this lead to allocation being made from area marked as isolated or some such. [...] I'd go as far to say that it would be preferable to drain the per-CPU lists after you set pageblocks MIGRATE_ISOLATE. The IPIs also have overhead but it will be incurred for the rare rather than the common case. I'll look into that. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- -- 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 03/11] mm: mmzone: introduce zone_pfn_same_memmap()
On Fri, Nov 18, 2011 at 05:43:10PM +0100, Marek Szyprowski wrote:
> From: Michal Nazarewicz
>
> This commit introduces zone_pfn_same_memmap() function which checkes
s/checkes/checks/
> whether two PFNs within the same zone have struct pages within the
> same memmap.
s/memmap/same sparsemem section/
> This check is needed because in general pointer
> arithmetic on struct pages may lead to invalid pointers.
>
> On memory models that are not affected, zone_pfn_same_memmap() is
> defined as always returning true so the call should be optimised
> at compile time.
>
> Signed-off-by: Michal Nazarewicz
> Signed-off-by: Marek Szyprowski
> ---
> include/linux/mmzone.h | 16
> mm/compaction.c|5 -
> 2 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 188cb2f..84e07d0 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -1166,6 +1166,22 @@ static inline int memmap_valid_within(unsigned long
> pfn,
> }
> #endif /* CONFIG_ARCH_HAS_HOLES_MEMORYMODEL */
>
> +#if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
> +/*
> + * Both PFNs must be from the same zone! If this function returns
from the same sparsemem section, not the same zone.
> + * true, pfn_to_page(pfn1) + (pfn2 - pfn1) == pfn_to_page(pfn2).
> + */
> +static inline bool zone_pfn_same_memmap(unsigned long pfn1, unsigned long
> pfn2)
> +{
> + return pfn_to_section_nr(pfn1) == pfn_to_section_nr(pfn2);
> +}
> +
> +#else
> +
> +#define zone_pfn_same_memmap(pfn1, pfn2) (true)
> +
> +#endif
> +
> #endif /* !__GENERATING_BOUNDS.H */
> #endif /* !__ASSEMBLY__ */
> #endif /* _LINUX_MMZONE_H */
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 6afae0e..09c9702 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -111,7 +111,10 @@ skip:
>
> next:
> pfn += isolated;
> - page += isolated;
> + if (zone_pfn_same_memmap(pfn - isolated, pfn))
> + page += isolated;
> + else
> + page = pfn_to_page(pfn);
> }
Is this necessary?
We are isolating pages, the largest of which is a MAX_ORDER_NR_PAGES
page. Sections are never smaller than MAX_ORDER_NR_PAGES so the end
of the free range of pages should never be in another section. That
should mean that the PFN walk will always consider the first
PFN of every section and you can implement a simplier check than
zone_pfn_same_memmap based on pfn & PAGE_SECTION_MASK and contain it
within mm/compaction.c
That said, everywhere else managed to avoid checks like this by always
scanning in units of pageblocks. Maybe this should be structured
the same way to guarantee pfn_valid is called at least per pageblock
(even though only once per MAX_ORDER_NR_PAGES is necessary).
--
Mel Gorman
SUSE Labs
--
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 02/11] mm: compaction: introduce isolate_{free,migrate}pages_range().
On Fri, Nov 18, 2011 at 05:43:09PM +0100, Marek Szyprowski wrote:
> From: Michal Nazarewicz
>
> This commit introduces isolate_freepages_range() and
> isolate_migratepages_range() functions. The first one replaces
> isolate_freepages_block() and the second one extracts functionality
> from isolate_migratepages().
>
> They are more generic and instead of operating on pageblocks operate
> on PFN ranges.
>
> Signed-off-by: Michal Nazarewicz
> Signed-off-by: Marek Szyprowski
> ---
> mm/compaction.c | 170
> ---
> 1 files changed, 111 insertions(+), 59 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 899d956..6afae0e 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -54,51 +54,64 @@ static unsigned long release_freepages(struct list_head
> *freelist)
> return count;
> }
>
> -/* Isolate free pages onto a private freelist. Must hold zone->lock */
> -static unsigned long isolate_freepages_block(struct zone *zone,
> - unsigned long blockpfn,
> - struct list_head *freelist)
> +/**
> + * isolate_freepages_range() - isolate free pages, must hold zone->lock.
> + * @zone:Zone pages are in.
> + * @start: The first PFN to start isolating.
> + * @end: The one-past-last PFN.
> + * @freelist:A list to save isolated pages to.
> + *
> + * If @freelist is not provided, holes in range (either non-free pages
> + * or invalid PFNs) are considered an error and function undos its
> + * actions and returns zero.
> + *
> + * If @freelist is provided, function will simply skip non-free and
> + * missing pages and put only the ones isolated on the list.
> + *
> + * Returns number of isolated pages. This may be more then end-start
> + * if end fell in a middle of a free page.
> + */
> +static unsigned long
> +isolate_freepages_range(struct zone *zone,
> + unsigned long start, unsigned long end,
> + struct list_head *freelist)
Use start_pfn and end_pfn to keep it consistent with the rest of
compaction.c.
> {
> - unsigned long zone_end_pfn, end_pfn;
> - int nr_scanned = 0, total_isolated = 0;
> - struct page *cursor;
> -
> - /* Get the last PFN we should scan for free pages at */
> - zone_end_pfn = zone->zone_start_pfn + zone->spanned_pages;
> - end_pfn = min(blockpfn + pageblock_nr_pages, zone_end_pfn);
> + unsigned long nr_scanned = 0, total_isolated = 0;
> + unsigned long pfn = start;
> + struct page *page;
>
> - /* Find the first usable PFN in the block to initialse page cursor */
> - for (; blockpfn < end_pfn; blockpfn++) {
> - if (pfn_valid_within(blockpfn))
> - break;
> - }
> - cursor = pfn_to_page(blockpfn);
> + VM_BUG_ON(!pfn_valid(pfn));
> + page = pfn_to_page(pfn);
>
> /* Isolate free pages. This assumes the block is valid */
> - for (; blockpfn < end_pfn; blockpfn++, cursor++) {
> - int isolated, i;
> - struct page *page = cursor;
> -
> - if (!pfn_valid_within(blockpfn))
> - continue;
> - nr_scanned++;
> -
> - if (!PageBuddy(page))
> - continue;
> + while (pfn < end) {
> + unsigned isolated = 1, i;
> +
Do not use implcit types. These are unsigned ints, call them unsigned
ints.
> + if (!pfn_valid_within(pfn))
> + goto skip;
The flow of this function in general with gotos of skipped and next
is confusing in comparison to the existing function. For example,
if this PFN is not valid, and no freelist is provided, then we call
__free_page() on a PFN that is known to be invalid.
> + ++nr_scanned;
> +
> + if (!PageBuddy(page)) {
> +skip:
> + if (freelist)
> + goto next;
> + for (; start < pfn; ++start)
> + __free_page(pfn_to_page(pfn));
> + return 0;
> + }
So if a PFN is valid and !PageBuddy and no freelist is provided, we
call __free_page() on it regardless of reference count. That does not
sound safe.
>
> /* Found a free page, break it into order-0 pages */
> isolated = split_free_page(page);
> total_isolated += isolated;
> - for (i = 0; i < isolated; i++) {
> - list_add(&page->lru, freelist);
> - page++;
> + if (freelist) {
> + struct page *p = page;
> + for (i = isolated; i; --i, ++p)
> + list_add(&p->lru, freelist);
> }
>
> - /* If a page was split, advance to the end of it */
> - if (isolated) {
> - blockpfn += isolated - 1;
> - cursor += isolated - 1;
>
Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities
On Mon, Dec 12, 2011 at 7:11 PM, Antti Palosaari wrote:
> On 12/12/2011 02:55 PM, Manu Abraham wrote:
>>
>> On Mon, Dec 12, 2011 at 6:13 PM, Andreas Oberritter
>> wrote:
>>>
>>> On 12.12.2011 05:28, Manu Abraham wrote:
On Sat, Dec 10, 2011 at 5:17 PM, Antti Palosaari wrote:
>
> On 12/10/2011 06:44 AM, Manu Abraham wrote:
>>
>>
>> static int cxd2820r_set_frontend(struct dvb_frontend *fe,
>
>
> [...]
>>
>>
>> + switch (c->delivery_system) {
>> + case SYS_DVBT:
>> + ret = cxd2820r_init_t(fe);
>
>
>
>> + ret = cxd2820r_set_frontend_t(fe, p);
>
>
>
>
> Anyhow, I don't now like idea you have put .init() calls to
> .set_frontend().
> Could you move .init() happen in .init() callback as it was earlier?
This was there in the earlier patch as well. Maybe you have a
new issue now ? ;-)
>
>
> You mean I didn't mentioned it when you send first version? Sorry, I didn't
> looked it very carefully since I first meet stuff that was not related whole
> thing, I mean there was that code changing from .set_params() to
> .set_state(). And I stopped reading rest of the patch.
>
>
>
ok.
The argument what you make doesn't hold well, Why ?
int cxd2820r_init_t(struct dvb_frontend *fe)
{
ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
}
int cxd2820r_init_c(struct dvb_frontend *fe)
{
ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
}
Now, you might like to point that, the Base I2C address location
is different comparing DVB-T/DVBT2 to DVB-C
So, If you have the init as in earlier with a common init, then you
will likely init the wrong device at .init(), as init is called open().
So, this might result in an additional register write, which could
be avoided altogether. One register access is not definitely
something to brag about, but is definitely a small incremental
difference. Other than that this register write doesn't do anything
more than an ADC_START. So starting the ADC at init doesn't
make sense. But does so when you want to select the right ADC.
So definitely, this change is an improvement. Also, you can
compare the time taken for the device to tune now. It is quite
a lot faster compared to without this patch. So you or any other
user should be happy. :-)
I don't think that in any way, the init should be used at init as
you say, which sounds pretty much incorrect.
>>>
>>>
>>> Maybe the function names should be modified to avoid confusion with the
>>> init driver callback.
>>
>>
>>
>> On another tangential thought, Is it really worth to wrap that single
>> register write with another function name ?
>>
>> instead of the current usage; ie,
>>
>> ret = cxd2820r_wr_reg(priv, 0x00085, 0x07); /* Start ADC */
>>
>> within set_frontend()
>>
>> in set_frontend(), another thing that's wrapped up similarly is
>> the set_frontend() within the search() callback, which causes
>> another set of confusions within the driver.
>
>
> Actually there was was a lot more code first but because I ran problems
> selsys needed for T/T2 init was not known at the time .init() was called I
> moved those set_frontend. I left that in a hope I can later fix properly
> adding more stuff back to init.
>
> That is not functionality issue, it is issue about naming callbacks and what
> is functionality of each callback.
> As for these days it have been in my understanding initialization stuff are
> done in .init() and leave as less as possible code to .set_frontend().
> Leaving set_frontend() handle only tuning requests and reconfigure IF
> control etc. And if you look most demod drivers there is rather similar
> logic used.
>
> So I would like to ask what is meaning of:
> .attach()
> * create FE
> * no HW init
> * as less as possible HW I/O, mainly reading chip ID and nothing more
Generally it should be simply create a DVB frontend data structure,
if it finds a valid device.
In some clunky cases, the demodulator clock would be from the tuner,
in which case the tuner has to be attached prior to the demod; and
then later on read device details, once clocks are setup.
>
> .init()
> * do nothing here?
> * download firmware?
>
init should simply initialize the device in the lowest power
mode, where it is ready to do a tune. (in some cases tuner also
might need an init. In such cases, the tuner_ops.init can be just
called).
> .set_frontend()
> * program tuner
> * init demod?
> * tune demod
> * download firmware?
Ideally set_frontend should just setup the frontend as a whole
(tuner, demod, or maybe more devices) and return status.
In some clunky cases, there could be cases where each tune
needs a firmware download. Maybe this download can be
optimized in some paths. This depends on the device.
>
> .sleep()
> * pu
Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C
On 12-12-2011 11:40, Manu Abraham wrote:
On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
This also means that just doing an alias from FE_QAM and SYS_DVBC_ANNEX_AC
to
SYS_DVBC_ANNEX_A may break something, as, for most devices,
SYS_DVBC_ANNEX_AC
really means both Annex A and C.
With the current approach, the application can determine whether
the hardware supports through the DELSYS enumeration.
So, if you have a device that needs to support both ANNEX_A and
ANNEX_C, it should be rather doing
case DTV_ENUM_DELSYS:
buffer.data[0] = SYS_DVBC_ANNEX_A;
buffer.data[1] = SYS_DVBC_ANNEX_C;
break;
Sure, but we'll need a logic to handle queries for SYS_DVBC_ANNEX_AC
anyway, if any of the existing DVB-C drivers is currently prepared to
support both.
I'm not concerned with drx-k. The support for both standards are for
kernel 3.3. So, no backward compatibility is needed here.
While there is no explicit option, the code for stv0297, stv0367,
tda10021 and tda10023 drivers are not clear if they support both
(maybe roll-off might be auto-detected?) or just SYS_DVBC_ANNEX_A.
That's said, the difference between a 0.15 and a 0.13 rolloff is not big.
I won't doubt that a demod set to 0.15 rolloff would be capable of working
(non-optimized) with a 0.13 rolloff.
What I'm saing is that, if any of the existing drivers currently works
with both Annex A and Annex C, we'll need something equivalent to:
if (delsys == SYS_DVBC_ANNEX_AC) {
int ret = try_annex_a();
if (ret < 0)
ret = try_annex_c();
}
For FE_SET_FRONTEND (and the corresponding v5 logic), in order to avoid
regressions.
I didn't look inside the drivers for stv0297, stv0367, tda10021 and
tda10023.
I suspect that some will need an additional code to change the roll-off,
based on
the delivery system.
Of course, yes this would need to make the change across multiple
drivers.
We can fix the drivers, that's no issue at all, as it is a small change.
Indeed, it is a small change. Tuners are trivial to change, but, at the
demod, we need to discover if roll-off is auto-detected somehow, or if
they require manual settings, in order to fix the demod drivers.
Regards,
Manu
--
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 01/11] mm: page_alloc: handle MIGRATE_ISOLATE in free_pcppages_bulk()
On Fri, Nov 18, 2011 at 05:43:08PM +0100, Marek Szyprowski wrote: > From: Michal Nazarewicz > > If page is on PCP list while pageblock it belongs to gets isolated, > the page's private still holds the old migrate type. This means > that free_pcppages_bulk() will put the page on a freelist of the > old migrate type instead of MIGRATE_ISOLATE. > > This commit changes that by explicitly checking whether page's > pageblock's migrate type is MIGRATE_ISOLATE and if it is, overwrites > page's private data. > > Signed-off-by: Michal Nazarewicz > Signed-off-by: Marek Szyprowski > --- > mm/page_alloc.c | 12 > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 9dd443d..58d1a2e 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -628,6 +628,18 @@ static void free_pcppages_bulk(struct zone *zone, int > count, > page = list_entry(list->prev, struct page, lru); > /* must delete as __free_one_page list manipulates */ > list_del(&page->lru); > + > + /* > + * When page is isolated in set_migratetype_isolate() > + * function it's page_private is not changed since the > + * function has no way of knowing if it can touch it. > + * This means that when a page is on PCP list, it's > + * page_private no longer matches the desired migrate > + * type. > + */ > + if (get_pageblock_migratetype(page) == MIGRATE_ISOLATE) > + set_page_private(page, MIGRATE_ISOLATE); > + How much of a problem is this in practice? It's adding overhead to the free path for what should be a very rare case which is undesirable. I know we are already calling get_pageblock_migrate() when freeing pages but it should be unnecessary to call it again. I'd go as far to say that it would be preferable to drain the per-CPU lists after you set pageblocks MIGRATE_ISOLATE. The IPIs also have overhead but it will be incurred for the rare rather than the common case. -- Mel Gorman SUSE Labs -- 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: v4 [PATCH 00/10] Query DVB frontend delivery capabilities
On 12-12-2011 04:35, Manu Abraham wrote: On Sat, Dec 10, 2011 at 6:38 PM, Mauro Carvalho Chehab wrote: On 10-12-2011 02:41, Manu Abraham wrote: Hi, As discussed prior, the following changes help to advertise a frontend's delivery system capabilities. Sending out the patches as they are being worked out. The following patch series are applied against media_tree.git after the following commit commit e9eb0dadba932940f721f9d27544a7818b2fa1c5 Author: Hans Verkuil Date: Tue Nov 8 11:02:34 2011 -0300 [media] V4L menu: add submenu for platform devices A separate issue: please, don't send patches like that as attachment. It makes hard for people review. Instead, you should use git send-email. There's even an example there (at least on git version 1.7.8) showing how to set it for Google: I don't have net access configured for the box where I do tests/on the testbox. The outgoing mail from my side is through the gmail web interface. If I don't attach the patches, gmail garbles those patches. Not sure what you've meant by "net". Internet, or network, in general? If you don't have an ethernet interface configured on your test box, you can still put your git tree into a removable media (pen-driver or whatever) and use it to transfer to your main machine, and then call git from it. If you have Ethernet there, it is even simpler: from your main machine: $ git pull remote:/patch/to/git my_branch $ git send-email Regards, Mauro. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities
On 12/12/2011 02:55 PM, Manu Abraham wrote:
On Mon, Dec 12, 2011 at 6:13 PM, Andreas Oberritter wrote:
On 12.12.2011 05:28, Manu Abraham wrote:
On Sat, Dec 10, 2011 at 5:17 PM, Antti Palosaari wrote:
On 12/10/2011 06:44 AM, Manu Abraham wrote:
static int cxd2820r_set_frontend(struct dvb_frontend *fe,
[...]
+ switch (c->delivery_system) {
+ case SYS_DVBT:
+ ret = cxd2820r_init_t(fe);
+ ret = cxd2820r_set_frontend_t(fe, p);
Anyhow, I don't now like idea you have put .init() calls to .set_frontend().
Could you move .init() happen in .init() callback as it was earlier?
This was there in the earlier patch as well. Maybe you have a
new issue now ? ;-)
You mean I didn't mentioned it when you send first version? Sorry, I
didn't looked it very carefully since I first meet stuff that was not
related whole thing, I mean there was that code changing from
.set_params() to .set_state(). And I stopped reading rest of the patch.
ok.
The argument what you make doesn't hold well, Why ?
int cxd2820r_init_t(struct dvb_frontend *fe)
{
ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
}
int cxd2820r_init_c(struct dvb_frontend *fe)
{
ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
}
Now, you might like to point that, the Base I2C address location
is different comparing DVB-T/DVBT2 to DVB-C
So, If you have the init as in earlier with a common init, then you
will likely init the wrong device at .init(), as init is called open().
So, this might result in an additional register write, which could
be avoided altogether. One register access is not definitely
something to brag about, but is definitely a small incremental
difference. Other than that this register write doesn't do anything
more than an ADC_START. So starting the ADC at init doesn't
make sense. But does so when you want to select the right ADC.
So definitely, this change is an improvement. Also, you can
compare the time taken for the device to tune now. It is quite
a lot faster compared to without this patch. So you or any other
user should be happy. :-)
I don't think that in any way, the init should be used at init as
you say, which sounds pretty much incorrect.
Maybe the function names should be modified to avoid confusion with the
init driver callback.
On another tangential thought, Is it really worth to wrap that single
register write with another function name ?
instead of the current usage; ie,
ret = cxd2820r_wr_reg(priv, 0x00085, 0x07); /* Start ADC */
within set_frontend()
in set_frontend(), another thing that's wrapped up similarly is
the set_frontend() within the search() callback, which causes
another set of confusions within the driver.
Actually there was was a lot more code first but because I ran problems
selsys needed for T/T2 init was not known at the time .init() was called
I moved those set_frontend. I left that in a hope I can later fix
properly adding more stuff back to init.
That is not functionality issue, it is issue about naming callbacks and
what is functionality of each callback.
As for these days it have been in my understanding initialization stuff
are done in .init() and leave as less as possible code to
.set_frontend(). Leaving set_frontend() handle only tuning requests and
reconfigure IF control etc. And if you look most demod drivers there is
rather similar logic used.
So I would like to ask what is meaning of:
.attach()
* create FE
* no HW init
* as less as possible HW I/O, mainly reading chip ID and nothing more
.init()
* do nothing here?
* download firmware?
.set_frontend()
* program tuner
* init demod?
* tune demod
* download firmware?
.sleep()
* put device sleep mode
* powersave
After all it is just fine for me apply that patch, but I would like to
get clear idea what is meaning of every single callback we have. And if
we really end up .init() is not needed and all should be put to
.set_frontend() when possible it means I have to change all my demod
drivers and maybe tuner drivers too.
regards
Antti
--
http://palosaari.fi/
--
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: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C
On Mon, Dec 12, 2011 at 6:49 PM, Mauro Carvalho Chehab
wrote:
> On 12-12-2011 01:59, Manu Abraham wrote:
>>
>> On Sat, Dec 10, 2011 at 5:59 PM, Mauro Carvalho Chehab
>> wrote:
>>>
>>> On 10-12-2011 02:43, Manu Abraham wrote:
>>>
>>>
From 92a79a1e0a1b5403f06f60661f00ede365b10108 Mon Sep 17 00:00:00 2001
From: Manu Abraham
Date: Thu, 24 Nov 2011 17:09:09 +0530
Subject: [PATCH 06/10] DVB: Use a unique delivery system identifier for
DVBC_ANNEX_C,
just like any other. DVBC_ANNEX_A and DVBC_ANNEX_C have slightly
different parameters and used in 2 geographically different
locations.
Signed-off-by: Manu Abraham
---
include/linux/dvb/frontend.h | 7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/include/linux/dvb/frontend.h b/include/linux/dvb/frontend.h
index f80b863..a3c7623 100644
--- a/include/linux/dvb/frontend.h
+++ b/include/linux/dvb/frontend.h
@@ -335,7 +335,7 @@ typedef enum fe_rolloff {
typedef enum fe_delivery_system {
SYS_UNDEFINED,
- SYS_DVBC_ANNEX_AC,
+ SYS_DVBC_ANNEX_A,
SYS_DVBC_ANNEX_B,
SYS_DVBT,
SYS_DSS,
@@ -352,8 +352,13 @@ typedef enum fe_delivery_system {
SYS_DAB,
SYS_DVBT2,
SYS_TURBO,
+ SYS_DVBC_ANNEX_C,
} fe_delivery_system_t;
+
+#define SYS_DVBC_ANNEX_AC SYS_DVBC_ANNEX_A
+
+
struct dtv_cmds_h {
char *name; /* A display name for debugging purposes
*/
>>>
>>>
>>>
>>> This patch conflicts with the approach given by this patch:
>>>
>>>
>>> http://www.mail-archive.com/linux-media@vger.kernel.org/msg39262.html
>>>
>>> merged as commit 39ce61a846c8e1fa00cb57ad5af021542e6e8403.
>>>
>>
>> - For correct delivery system handling, the delivery system identifier
>> should be unique. Otherwise patch 01/10 is meaningless for devices
>> with DVBC_ANNEX_C, facing the same situations.
>
>
> This is a good point.
>
>>
>> - Rolloff is provided only in the SI table and is not known prior to a
>> tune. So users must struggle to find the correct rolloff. So users
>> must know beforehand their experience what rolloff it is rather than
>> reading Service Information, which is broken by approach. It is much
>> easier for a user to state that he is living in Japan or some other
>> place which is using ANNEX_C, rather than creating confusion that
>> he has to use DVBC and rolloff of 0.15
>
>
> Userspace can present it as Japan and hide the technical details. Most
> applications do that already: kaffeine, w_scan, ...
>
> The dvb-apps utils don't do it, but the scan file format it produces
> doesn't support anything besides DVB-C/DVB-S/DVB-T/ATSC anyway.
>
>
>> or is it multiplied by a factor
>> of 10 or was it 100 ? (Oh, my god my application Y uses a factor
>> of 10, the X application uses 100 and the Z application uses 1000).
>> What a lovely confusing scenario. ;-) (Other than for the mentioned
>> issue that the rolloff can be read from the SI, which is available after
>> tuning; for tuning you need rolloff).
>
>
> Sorry, but this argument doesn't make any sense to me. The same problem
> exists on DVB-S2 already, where several rolloffs are supported. Except
> if someone would code a channels.conf line in hand, the roll-off is not
> visible by the end user.
DVB-S2 as what we see as broadcast has just a single rolloff. The same
rolloff is used in the SI alone. It's a mistake to handle rollolff as a user
input field. The other rolloff's are used for very specific applications,
such as DSNG, DVB-RCS etc, where bandwidth has to be really
conserved considering uplinks from trucks, vans etc; for which we don't
even have applications or users.
>
>> Really sexy setup indeed. ;-)
>>
>> One thing that I should warn/mention to you is the lack of clarity on
>> what you say. You say that you want more discussion, but you
>> whack in patches which is never discussed, breaking many logical
>> concepts and ideas and broken by nature. How do you justify
>> yourself ? I don't think you can justify yourself.
>
>
> If we have a consensus around your approach, I'm OK to move for it,
> provided that it doesn't cause regressions upstream.
>
> As I said, this requires reviewing all DVB frontends to be sure that
> they won't break, especially if is there any that it is capable of
> auto-detecting the roll-off factor.
>
> Both approaches have advantages and disadvantages.
>
> The main advantage of my approach is that it is coherent with the current
> DVBv5 API (e. g. SYS_DVBC_ANNEX_AC). So, the only changes are at the
> frontends that need to decide between Annex A and Annex C (currently, only
> drx-k - and the tuners used with it).
>
> Advantages on your approach:
> - Cleaner for the userspace API;
> - It is possible to add Annex C only devices.
> Disadvantages:
> - Need to deprecate the
Re: v4 [PATCH 08/10] TDA18271c2dd: Allow frontend to set DELSYS
On 12-12-2011 03:01, Manu Abraham wrote:
On Sat, Dec 10, 2011 at 6:20 PM, Mauro Carvalho Chehab
wrote:
On 10-12-2011 02:44, Manu Abraham wrote:
From 707877f5a61b3259704d42e7dd5e647e9196e9a4 Mon Sep 17 00:00:00 2001
From: Manu Abraham
Date: Thu, 24 Nov 2011 19:56:34 +0530
Subject: [PATCH 08/10] TDA18271c2dd: Allow frontend to set DELSYS, rather
than querying fe->ops.info.type
With any tuner that can tune to multiple delivery systems/standards, it
does
query fe->ops.info.type to determine frontend type and set the delivery
system type. fe->ops.info.type can handle only 4 delivery systems, viz
FE_QPSK,
FE_QAM, FE_OFDM and FE_ATSC.
Signed-off-by: Manu Abraham
---
drivers/media/dvb/frontends/tda18271c2dd.c | 42
1 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/drivers/media/dvb/frontends/tda18271c2dd.c
b/drivers/media/dvb/frontends/tda18271c2dd.c
index 1b1bf20..43a3dd4 100644
--- a/drivers/media/dvb/frontends/tda18271c2dd.c
+++ b/drivers/media/dvb/frontends/tda18271c2dd.c
@@ -1145,28 +1145,46 @@ static int set_params(struct dvb_frontend *fe,
int status = 0;
int Standard;
- state->m_Frequency = params->frequency;
+ u32 bw;
+ fe_delivery_system_t delsys;
- if (fe->ops.info.type == FE_OFDM)
- switch (params->u.ofdm.bandwidth) {
- case BANDWIDTH_6_MHZ:
+ delsys = fe->dtv_property_cache.delivery_system;
+ bw = fe->dtv_property_cache.bandwidth_hz;
+
+ state->m_Frequency = fe->dtv_property_cache.frequency;
+
+ if (!delsys || !state->m_Frequency) {
+ printk(KERN_ERR "Invalid delsys:%d freq:%d\n", delsys,
state->m_Frequency);
+ return -EINVAL;
+ }
+
+ switch (delsys) {
+ case SYS_DVBT:
+ case SYS_DVBT2:
+ if (!bw)
+ return -EINVAL;
+ switch (bw) {
+ case 600:
Standard = HF_DVBT_6MHZ;
break;
- case BANDWIDTH_7_MHZ:
+ case 700:
Standard = HF_DVBT_7MHZ;
break;
default:
- case BANDWIDTH_8_MHZ:
+ case 800:
Standard = HF_DVBT_8MHZ;
break;
}
- else if (fe->ops.info.type == FE_QAM) {
- if (params->u.qam.symbol_rate<= MAX_SYMBOL_RATE_6MHz)
- Standard = HF_DVBC_6MHZ;
- else
- Standard = HF_DVBC_8MHZ;
- } else
+ break;
+ case SYS_DVBC_ANNEX_A:
+ Standard = HF_DVBC_6MHZ;
+ break;
+ case SYS_DVBC_ANNEX_C:
+ Standard = HF_DVBC_8MHZ;
+ break;
No, this is wrong. This patch doesn't apply anymore, due to the recent
changes that estimate the bandwidth based on the roll-off factor. Reverting
it breaks for DVB-C @ 6MHz spaced channels (and likely decreases quality
or breaks for 7MHz spaced ones too).
The changes that which you mention (which you state breaks 7MHz
spacing) is much newer than these patch series. Alas, how do you
expect people to push out patches every now and then, when you
simply whack patches in. It is not the issue with the people sending
patches, but how you handled the patch series. I have reworked the
patches the 4th time, while you simply whack patches without any
feedback.
Time after time, lot of different people have argued with
you not to simply whack in patches as you seem fit. Who is to blame ?
Patches were sent to the ML, and were tested by the ones complaining
about tuner issues with DVB-C. It were merged only after having feedback.
In any case, this is not a reason for me to not merge this patch. Conflicts
like that happens all the time when merging stuff from different authors.
The issue here is that it assumes that Annex A is 6MHz and Annex C is 8MHz.
This is a wrong assumption: there's nothing at ITU-T J.83 associating Annex C
(or even Annex A) with a given bandwidth.
What is said at the spec is that Annex A is "optimized" for 6MHz, but I
won't be really surprised if some broadcaster decides to use it for other
bandwidths.
The logic there should be, instead:
u32 roll_off = 0;
switch (delsys) {
case SYS_DVBC_ANNEX_A:
roll_off = 115;
break;
case SYS_DVBC_ANNEX_C:
roll_off = 113;
break;
}
if (roll_off) {
bw = symbol_rate * roll_off / 100;
if (bw <= 600)
Standard = HF_DVBC_6MHZ;
else if (bw <= 700)
Standard = HF_DVBC_7MHZ;
else
Standard = HF_DVBC_8MHZ;
}
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: v4 [PATCH 06/10] DVB: Use a unique delivery system identifier for DVBC_ANNEX_C
On 12-12-2011 01:59, Manu Abraham wrote:
On Sat, Dec 10, 2011 at 5:59 PM, Mauro Carvalho Chehab
wrote:
On 10-12-2011 02:43, Manu Abraham wrote:
From 92a79a1e0a1b5403f06f60661f00ede365b10108 Mon Sep 17 00:00:00 2001
From: Manu Abraham
Date: Thu, 24 Nov 2011 17:09:09 +0530
Subject: [PATCH 06/10] DVB: Use a unique delivery system identifier for
DVBC_ANNEX_C,
just like any other. DVBC_ANNEX_A and DVBC_ANNEX_C have slightly
different parameters and used in 2 geographically different
locations.
Signed-off-by: Manu Abraham
---
include/linux/dvb/frontend.h |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/include/linux/dvb/frontend.h b/include/linux/dvb/frontend.h
index f80b863..a3c7623 100644
--- a/include/linux/dvb/frontend.h
+++ b/include/linux/dvb/frontend.h
@@ -335,7 +335,7 @@ typedef enum fe_rolloff {
typedef enum fe_delivery_system {
SYS_UNDEFINED,
- SYS_DVBC_ANNEX_AC,
+ SYS_DVBC_ANNEX_A,
SYS_DVBC_ANNEX_B,
SYS_DVBT,
SYS_DSS,
@@ -352,8 +352,13 @@ typedef enum fe_delivery_system {
SYS_DAB,
SYS_DVBT2,
SYS_TURBO,
+ SYS_DVBC_ANNEX_C,
} fe_delivery_system_t;
+
+#define SYS_DVBC_ANNEX_AC SYS_DVBC_ANNEX_A
+
+
struct dtv_cmds_h {
char*name; /* A display name for debugging purposes */
This patch conflicts with the approach given by this patch:
http://www.mail-archive.com/linux-media@vger.kernel.org/msg39262.html
merged as commit 39ce61a846c8e1fa00cb57ad5af021542e6e8403.
- For correct delivery system handling, the delivery system identifier
should be unique. Otherwise patch 01/10 is meaningless for devices
with DVBC_ANNEX_C, facing the same situations.
This is a good point.
- Rolloff is provided only in the SI table and is not known prior to a
tune. So users must struggle to find the correct rolloff. So users
must know beforehand their experience what rolloff it is rather than
reading Service Information, which is broken by approach. It is much
easier for a user to state that he is living in Japan or some other
place which is using ANNEX_C, rather than creating confusion that
he has to use DVBC and rolloff of 0.15
Userspace can present it as Japan and hide the technical details. Most
applications do that already: kaffeine, w_scan, ...
The dvb-apps utils don't do it, but the scan file format it produces
doesn't support anything besides DVB-C/DVB-S/DVB-T/ATSC anyway.
or is it multiplied by a factor
of 10 or was it 100 ? (Oh, my god my application Y uses a factor
of 10, the X application uses 100 and the Z application uses 1000).
What a lovely confusing scenario. ;-) (Other than for the mentioned
issue that the rolloff can be read from the SI, which is available after
tuning; for tuning you need rolloff).
Sorry, but this argument doesn't make any sense to me. The same problem
exists on DVB-S2 already, where several rolloffs are supported. Except
if someone would code a channels.conf line in hand, the roll-off is not
visible by the end user.
Really sexy setup indeed. ;-)
One thing that I should warn/mention to you is the lack of clarity on
what you say. You say that you want more discussion, but you
whack in patches which is never discussed, breaking many logical
concepts and ideas and broken by nature. How do you justify
yourself ? I don't think you can justify yourself.
If we have a consensus around your approach, I'm OK to move for it,
provided that it doesn't cause regressions upstream.
As I said, this requires reviewing all DVB frontends to be sure that
they won't break, especially if is there any that it is capable of
auto-detecting the roll-off factor.
Both approaches have advantages and disadvantages.
The main advantage of my approach is that it is coherent with the current
DVBv5 API (e. g. SYS_DVBC_ANNEX_AC). So, the only changes are at the
frontends that need to decide between Annex A and Annex C (currently, only
drx-k - and the tuners used with it).
Advantages on your approach:
- Cleaner for the userspace API;
- It is possible to add Annex C only devices.
Disadvantages:
- Need to deprecate the existing SYS_DVBC_ANNEX_AC;
- The alias that SYS_DVBC_ANNEX_AC means only SYS_DVBC_ANNEX_A might
break some thing;
- Requires further changes at the DocBook API description;
- Need to review all DVB-C frontends.
If we're willing to take your approach, we need a patch series that addresses
all DVB-C frontends, to be sure that no regressions were added due to the change
ofSYS_DVBC_ANNEX_AC meaning.
It also requires that FE_QAM to be mapped to be both SYS_DVBC_ANNEX_A and
SYS_DVBC_ANNEX_C,
if isn't there any issue for it to work with Annex C mode, or to just
SYS_DVBC_ANNEX_A, if there is enough confidence that such device doesn't
work at allwith annex C.
The approach there were to allow calls to SYS_DVBC_ANNEX_AC to replace the
Annex A
roll-off factor of 0.15 by the one used
Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities
On Mon, Dec 12, 2011 at 6:13 PM, Andreas Oberritter wrote:
> On 12.12.2011 05:28, Manu Abraham wrote:
>> On Sat, Dec 10, 2011 at 5:17 PM, Antti Palosaari wrote:
>>> On 12/10/2011 06:44 AM, Manu Abraham wrote:
static int cxd2820r_set_frontend(struct dvb_frontend *fe,
>>>
>>> [...]
+ switch (c->delivery_system) {
+ case SYS_DVBT:
+ ret = cxd2820r_init_t(fe);
>>>
>>>
+ ret = cxd2820r_set_frontend_t(fe, p);
>>>
>>>
>>>
>>> Anyhow, I don't now like idea you have put .init() calls to .set_frontend().
>>> Could you move .init() happen in .init() callback as it was earlier?
>>
>> This was there in the earlier patch as well. Maybe you have a
>> new issue now ? ;-)
>>
>> ok.
>>
>> The argument what you make doesn't hold well, Why ?
>>
>> int cxd2820r_init_t(struct dvb_frontend *fe)
>> {
>> ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
>> }
>>
>>
>> int cxd2820r_init_c(struct dvb_frontend *fe)
>> {
>> ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
>> }
>>
>>
>> Now, you might like to point that, the Base I2C address location
>> is different comparing DVB-T/DVBT2 to DVB-C
>>
>> So, If you have the init as in earlier with a common init, then you
>> will likely init the wrong device at .init(), as init is called open().
>> So, this might result in an additional register write, which could
>> be avoided altogether. One register access is not definitely
>> something to brag about, but is definitely a small incremental
>> difference. Other than that this register write doesn't do anything
>> more than an ADC_START. So starting the ADC at init doesn't
>> make sense. But does so when you want to select the right ADC.
>> So definitely, this change is an improvement. Also, you can
>> compare the time taken for the device to tune now. It is quite
>> a lot faster compared to without this patch. So you or any other
>> user should be happy. :-)
>>
>>
>> I don't think that in any way, the init should be used at init as
>> you say, which sounds pretty much incorrect.
>
> Maybe the function names should be modified to avoid confusion with the
> init driver callback.
On another tangential thought, Is it really worth to wrap that single
register write with another function name ?
instead of the current usage; ie,
ret = cxd2820r_wr_reg(priv, 0x00085, 0x07); /* Start ADC */
within set_frontend()
in set_frontend(), another thing that's wrapped up similarly is
the set_frontend() within the search() callback, which causes
another set of confusions within the driver.
Regards,
Manu
--
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: cx231xx kernel oops
Andy Walls wrote: 800 MB for 320x420 frames? It sounds like your app has gooned its requested buffer size. That's an understatement. :-) This might be due to endianess differences between MIPS abd x86 and your app only being written and tested on x86. My speculation too. I don't know where that number comes from; the same app works fine with the saa7115 driver if I switch frame grabbers. I'll have to do some fiddling with the code to figure out where the problem lies. It's some interaction between the app and the cx231xx driver. You still appear to USB stack problems, but not as severe (can't change device config to some bogus config). The requested buffer size is the result of multiplying max_pkt_size * max_packets and the rejected config shows a max_packet_size of 0, maybe ithere;'s a problem with either endianness or int size... ??? Something to follow up on. Regards, Andy !DSPAM:4ee5f4e4112206551461313! -- Few people are capable of expressing with equanimity opinions which differ from the prejudices of their social environment. Most people are even incapable of forming such opinions. Albert Einstein -- 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: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities
On 12.12.2011 05:28, Manu Abraham wrote:
> On Sat, Dec 10, 2011 at 5:17 PM, Antti Palosaari wrote:
>> On 12/10/2011 06:44 AM, Manu Abraham wrote:
>>>
>>> static int cxd2820r_set_frontend(struct dvb_frontend *fe,
>>
>> [...]
>>>
>>> + switch (c->delivery_system) {
>>> + case SYS_DVBT:
>>> + ret = cxd2820r_init_t(fe);
>>
>>
>>> + ret = cxd2820r_set_frontend_t(fe, p);
>>
>>
>>
>> Anyhow, I don't now like idea you have put .init() calls to .set_frontend().
>> Could you move .init() happen in .init() callback as it was earlier?
>
> This was there in the earlier patch as well. Maybe you have a
> new issue now ? ;-)
>
> ok.
>
> The argument what you make doesn't hold well, Why ?
>
> int cxd2820r_init_t(struct dvb_frontend *fe)
> {
> ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
> }
>
>
> int cxd2820r_init_c(struct dvb_frontend *fe)
> {
> ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
> }
>
>
> Now, you might like to point that, the Base I2C address location
> is different comparing DVB-T/DVBT2 to DVB-C
>
> So, If you have the init as in earlier with a common init, then you
> will likely init the wrong device at .init(), as init is called open().
> So, this might result in an additional register write, which could
> be avoided altogether. One register access is not definitely
> something to brag about, but is definitely a small incremental
> difference. Other than that this register write doesn't do anything
> more than an ADC_START. So starting the ADC at init doesn't
> make sense. But does so when you want to select the right ADC.
> So definitely, this change is an improvement. Also, you can
> compare the time taken for the device to tune now. It is quite
> a lot faster compared to without this patch. So you or any other
> user should be happy. :-)
>
>
> I don't think that in any way, the init should be used at init as
> you say, which sounds pretty much incorrect.
Maybe the function names should be modified to avoid confusion with the
init driver callback.
Regards,
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cx231xx kernel oops
Yan Seiner wrote: >I'm resurrecting an older thread. I have a Hauppage USB Live2 >connected >to a MIPS box running openWRT. I tried this earlier on a older >hardware >running the 3.0.3 kernel. This is with newer hardware running >2.6.39.4. The driver attempts to allocate 800MB (!!!) of memory for >the >buffer and fails with a kernel crash. I'm not including any kernel >crash stuff as it has all the symbols stripped. > >This seems to be the key message: > >[ 514.77] unable to allocate 805398992 bytes for transfer buffer 0 > >What can I do to narrow down the allocation problem? > >system type: Atheros AR9132 rev 2 >machine: Buffalo WZR-HP-G300NH >processor: 0 >cpu model: MIPS 24Kc V7.4 >BogoMIPS: 265.42 > >Bus 001 Device 005: ID 2040:c200 Hauppauge > >[ 34.56] cx231xx v4l2 driver loaded. >[ 34.57] cx231xx #0: New device Hauppauge Hauppauge Device @ 480 >Mbps (2040:c200) with 5 interfaces >[ 34.58] cx231xx #0: registering interface 1 >[ 34.58] cx231xx #0: bad senario! >[ 34.59] cx231xx #0: config_info=0 >[ 34.59] cx231xx #0: can't change interface 1 alt no. to 3: Max. >Pkt size = 0 >[ 34.60] usb 1-1.2: selecting invalid altsetting 3 >[ 34.60] cx231xx #0: can't change interface 1 alt no. to 3 >(err=-22) >[ 34.61] cx231xx #0: can't change interface 1 alt no. to 1: Max. >Pkt size = 0 >[ 34.62] usb 1-1.2: selecting invalid altsetting 1 >[ 34.62] cx231xx #0: can't change interface 1 alt no. to 1 >(err=-22) >[ 34.63] cx231xx #0: Identified as Hauppauge USB Live 2 (card=9) >[ 34.74] cx231xx #0: cx231xx_dif_set_standard: setStandard to > >[ 34.76] cx231xx #0: Changing the i2c master port to 3 >[ 34.76] cx25840 0-0044: cx23102 A/V decoder found @ 0x88 >(cx231xx #0) >[ 34.79] cx25840 0-0044: Firmware download size changed to 16 >bytes max length >[ 36.77] cx25840 0-0044: loaded v4l-cx231xx-avcore-01.fw firmware > >(16382 bytes) >[ 36.81] cx231xx #0: cx231xx #0: v4l2 driver version 0.0.1 >[ 36.84] cx231xx #0: cx231xx_dif_set_standard: setStandard to > >[ 36.89] cx231xx #0: video_mux : 0 >[ 36.90] cx231xx #0: do_mode_ctrl_overrides : 0xb000 >[ 36.90] cx231xx #0: do_mode_ctrl_overrides NTSC >[ 36.91] cx231xx #0: cx231xx #0/0: registered device video0 >[v4l2] >[ 36.92] cx231xx #0: cx231xx #0/0: registered device vbi0 >[ 36.93] cx231xx #0: V4L2 device registered as video0 and vbi0 >[ 36.93] cx231xx #0: EndPoint Addr 0x8f00, Alternate settings: 1 >[ 36.94] cx231xx #0: Alternate setting 0, max size= 8 >[ 36.94] cx231xx #0: EndPoint Addr 0x8f00, Alternate settings: 1 >[ 36.95] cx231xx #0: Alternate setting 0, max size= 8 >[ 36.96] cx231xx #0: EndPoint Addr 0x8f00, Alternate settings: 1 >[ 36.96] cx231xx #0: Alternate setting 0, max size= 8 >[ 36.97] cx231xx #0: EndPoint Addr 0x8f00, Alternate settings: 1 >[ 36.97] cx231xx #0: Alternate setting 0, max size= 8 >[ 36.98] usbcore: registered new interface driver cx231xx >[ 37.23] ar71xx-wdt: enabling watchdog timer > > >root@rtmovies:/www/tmp/root/etc# fswebcam -r 320x240 >--- Opening /dev/video0... >Trying source module v4l2... >/dev/video0 opened. >No input was specified, using the first. >VIDIOC_QBUF: Cannot allocate memory >Unable to use mmap. Using read instead. >--- Capturing frame... >VIDIOC_DQBUF: Invalid argument >No frames captured. > > > >-- >Few people are capable of expressing with equanimity opinions which >differ from the prejudices of their social environment. Most people are >even incapable of forming such opinions. >Albert Einstein > >-- >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 800 MB for 320x420 frames? It sounds like your app has gooned its requested buffer size. This might be due to endianess differences between MIPS abd x86 and your app only being written and tested on x86. You still appear to USB stack problems, but not as severe (can't change device config to some bogus config). 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: I2C and mt9p031 on Overo
On 2011-12-11 13:25, Robert Åkerblom-Andersson wrote: Hi, I trying to get the mt9p031 to work on the Overo board. So far I have it working in the Beagleboard xM, and now I have sort of ported/used the same files to get it to work with Overo. My problem now is that when I probe the camera board (LI-5M03 with an adapter board in between providing extra voltage levels) it seams fine. OMAP3ISP loads without any bigger error but the mt9p031 driver can't find the device, but it does not seam to be a driver problem rather a board problem. I think this since I've been debugging with "i2cdetect -y -r 3" to scan the bus for the camera. Most of the times I get nothing, but a couple of times (out of hundreds or more, I used a while loop with i2cdetect and then a sleep 1) it showed up with it's address. I think it happens when I just inserted the board but I'm sure or if I get it into some "weird" state just adding it. It could be a contact error but I have a felling it is something else I have missed. Some pin configuration or something that stops it from working. Do you have any tips on how to debug further or on what might the my problem? I have tried to lower the i2c speed to 100 KHz but it did not seam to make any difference. I too had problems with this device. I have [yet another] different DM3750 board which uses this sensor, so my experience is not exactly the same as yours on the Overo. However, I found that I had to have a pretty substantial delay (500ms) between the time that the MT9P031 was taken out of reset (I have this on a GPIO pin) and when the I2C bus is scanned for the device (mt9p031_probe called). With the delay, the device is discovered and works great. Without it, the device is never seen on the I2C bus. -- Gary Thomas | Consulting for the MLB Associates |Embedded world -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
Hi Ming, It's maybe late, but I want to suggest one thing about FD API. This OMAP FD block looks detection ability of just face. But, It's possible to occur another device which can detect specific "object" or "patterns". Moreover, this API can expand "object recognition" area. So, I think it's good to change the API name like "v4l2_recog". Actually, I'm preparing similar control class for mainline with m5mols camera sensor driver. The m5mols camera sensor has the function about "face detection". But, I has experienced about Robot Recognition, and I remember the image processing chip which can detect spefic "pattern". So, I hesitated naming the API(control or ioctl whatever) with "face". It can be possible to provide just "object" or "pattern", not face. Even user library on windows, there is famous "OpenCV". And this is also support not only "face", but also "object". The function of OMAP FDIF looks like m5mols ISP's one. please understand I don't have experience about OMAP AP. But, I can tell you it's better to use the name "object recognition", not the "face detection", for any other device or driver. In a few days, I'll share the CIDs I have thought for m5mols driver. And, I hope to discuss about this with OMAP FDIF. Thank you. Regards, Heungjun Kim > -Original Message- > From: linux-media-ow...@vger.kernel.org [mailto:linux-media- > ow...@vger.kernel.org] On Behalf Of Ming Lei > Sent: Monday, December 12, 2011 6:50 PM > To: Sylwester Nawrocki > Cc: linux-o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux- > ker...@vger.kernel.org; linux-media@vger.kernel.org > Subject: Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver > module > > Hi, > > On Mon, Dec 12, 2011 at 1:43 AM, Sylwester Nawrocki > > >> For OMAP4 FD, it is not needed to include FD into MC framework since a > >> intermediate buffer is always required. If your HW doesn't belong to this > >> case, what is the output of your HW FD in the link? Also sounds FD results > >> may not be needed at all for use space application in the case. > > > > The result data is similar to OMAP4 one, plus a few other attributes. > > User buffers may be filled by other than FD device driver. > > OK. > > > >> Could you provide some practical use cases about these? > > > > As above, and any device with a camera that controls something and makes > > decision according to presence of human face in his view. > > Sounds a reasonable case, :-) > > > >> If FD result is associated with a frame, how can user space get the frame > seq > >> if no v4l2 buffer is involved? Without a frame sequence, it is a bit > >> difficult to retrieve FD results from user space. > > > > If you pass image data in memory buffers from user space, yes, it could be > > impossible. > > It is easy to get the frame sequence from v4l2_buffer for the case too, :-) > > > > > Not really, still v4l2_buffer may be used by other (sub)driver within same > video > > processing pipeline. > > OK. > > A related question: how can we make one application to support the two kinds of > devices(input from user space data as OMAP4, input from SoC bus as Samsung) > at the same time? Maybe some capability info is to be exported to user space? > or other suggestions? > > And will your Samsung FD HW support to detect faces from memory? or just only > detect from SoC bus? > > > > It will be included in the FD result... or in a dedicated v4l2 event data > structure. > > More important, at the end of the day, we'll be getting buffers with image > data > > at some stage of a video pipeline, which would contain same frame identifier > > (I think we can ignore v4l2_buffer.field for FD purpose). > > OK, I will associate FD result with frame identifier, and not invent a > dedicated v4l2 event for query frame seq now until a specific requirement > for it is proposed. > > I will convert/integrate recent discussions into patches of v2 for further > review, and sub device support will be provided. But before starting to do it, > I am still not clear how to integrate FD into MC framework. I understand FD > sub device is only a media entity, so how can FD sub device find the media > device(struct media_device)? or just needn't to care about it now? > > > thanks, > -- > Ming Lei > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/2] MX2: Add platform definitions for eMMa-PrP device.
eMMa-PrP device included in Freescale i.MX2 chips can also
be used separately to process memory buffers. This patch
provides arch glue code for the driver which provides this
functionality.
Acked-by: Sascha Hauer
Signed-off-by: Javier Martin
---
arch/arm/mach-imx/clock-imx27.c |2 +-
arch/arm/mach-imx/devices-imx27.h |2 ++
arch/arm/plat-mxc/devices/platform-mx2-camera.c | 18 ++
arch/arm/plat-mxc/include/mach/devices-common.h |2 ++
4 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-imx/clock-imx27.c b/arch/arm/mach-imx/clock-imx27.c
index 88fe00a..dc2d7a5 100644
--- a/arch/arm/mach-imx/clock-imx27.c
+++ b/arch/arm/mach-imx/clock-imx27.c
@@ -661,7 +661,7 @@ static struct clk_lookup lookups[] = {
_REGISTER_CLOCK(NULL, "dma", dma_clk)
_REGISTER_CLOCK(NULL, "rtic", rtic_clk)
_REGISTER_CLOCK(NULL, "brom", brom_clk)
- _REGISTER_CLOCK(NULL, "emma", emma_clk)
+ _REGISTER_CLOCK("m2m-emmaprp.0", NULL, emma_clk)
_REGISTER_CLOCK(NULL, "slcdc", slcdc_clk)
_REGISTER_CLOCK("imx27-fec.0", NULL, fec_clk)
_REGISTER_CLOCK(NULL, "emi", emi_clk)
diff --git a/arch/arm/mach-imx/devices-imx27.h
b/arch/arm/mach-imx/devices-imx27.h
index 2f727d7..28537a5 100644
--- a/arch/arm/mach-imx/devices-imx27.h
+++ b/arch/arm/mach-imx/devices-imx27.h
@@ -50,6 +50,8 @@ extern const struct imx_imx_uart_1irq_data
imx27_imx_uart_data[];
extern const struct imx_mx2_camera_data imx27_mx2_camera_data;
#define imx27_add_mx2_camera(pdata)\
imx_add_mx2_camera(&imx27_mx2_camera_data, pdata)
+#define imx27_add_mx2_emmaprp(pdata) \
+ imx_add_mx2_emmaprp(&imx27_mx2_camera_data)
extern const struct imx_mxc_ehci_data imx27_mxc_ehci_otg_data;
#define imx27_add_mxc_ehci_otg(pdata) \
diff --git a/arch/arm/plat-mxc/devices/platform-mx2-camera.c
b/arch/arm/plat-mxc/devices/platform-mx2-camera.c
index b3f4828..11eace9 100644
--- a/arch/arm/plat-mxc/devices/platform-mx2-camera.c
+++ b/arch/arm/plat-mxc/devices/platform-mx2-camera.c
@@ -62,3 +62,21 @@ struct platform_device *__init imx_add_mx2_camera(
res, data->iobaseemmaprp ? 4 : 2,
pdata, sizeof(*pdata), DMA_BIT_MASK(32));
}
+
+struct platform_device *__init imx_add_mx2_emmaprp(
+ const struct imx_mx2_camera_data *data)
+{
+ struct resource res[] = {
+ {
+ .start = data->iobaseemmaprp,
+ .end = data->iobaseemmaprp + data->iosizeemmaprp - 1,
+ .flags = IORESOURCE_MEM,
+ }, {
+ .start = data->irqemmaprp,
+ .end = data->irqemmaprp,
+ .flags = IORESOURCE_IRQ,
+ },
+ };
+ return imx_add_platform_device_dmamask("m2m-emmaprp", 0,
+ res, 2, NULL, 0, DMA_BIT_MASK(32));
+}
diff --git a/arch/arm/plat-mxc/include/mach/devices-common.h
b/arch/arm/plat-mxc/include/mach/devices-common.h
index def9ba5..1b2258d 100644
--- a/arch/arm/plat-mxc/include/mach/devices-common.h
+++ b/arch/arm/plat-mxc/include/mach/devices-common.h
@@ -223,6 +223,8 @@ struct imx_mx2_camera_data {
struct platform_device *__init imx_add_mx2_camera(
const struct imx_mx2_camera_data *data,
const struct mx2_camera_platform_data *pdata);
+struct platform_device *__init imx_add_mx2_emmaprp(
+ const struct imx_mx2_camera_data *data);
#include
struct imx_mxc_ehci_data {
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/2] MEM2MEM: Add support for eMMa-PrP mem2mem operations.
i.MX2x SoCs have a PrP which is capable of resizing and format
conversion of video frames. This driver provides support for
resizing and format conversion from YUYV to YUV420.
This operation is of the utmost importance since some of these
SoCs like i.MX27 include an H.264 video codec which only
accepts YUV420 as input.
Reviewed-by: Sylwester Nawrocki
Signed-off-by: Javier Martin
---
drivers/media/video/Kconfig | 10 +
drivers/media/video/Makefile |2 +
drivers/media/video/mx2_emmaprp.c | 1008 +
3 files changed, 1020 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/video/mx2_emmaprp.c
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index b303a3f..e8ce0c2 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -1107,4 +1107,14 @@ config VIDEO_SAMSUNG_S5P_MFC
help
MFC 5.1 driver for V4L2.
+config VIDEO_MX2_EMMAPRP
+ tristate "MX2 eMMa-PrP support"
+ depends on VIDEO_DEV && VIDEO_V4L2 && SOC_IMX27
+ select VIDEOBUF2_DMA_CONTIG
+ select V4L2_MEM2MEM_DEV
+ help
+ MX2X chips have a PrP that can be used to process buffers from
+ memory to memory. Operations include resizing and format
+ conversion.
+
endif # V4L_MEM2MEM_DRIVERS
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index 117f9c4..7ae711e 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -176,6 +176,8 @@ obj-$(CONFIG_VIDEO_SH_MOBILE_CEU) +=
sh_mobile_ceu_camera.o
obj-$(CONFIG_VIDEO_OMAP1) += omap1_camera.o
obj-$(CONFIG_VIDEO_ATMEL_ISI) += atmel-isi.o
+obj-$(CONFIG_VIDEO_MX2_EMMAPRP)+= mx2_emmaprp.o
+
obj-$(CONFIG_VIDEO_SAMSUNG_S5P_FIMC) += s5p-fimc/
obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC)+= s5p-mfc/
obj-$(CONFIG_VIDEO_SAMSUNG_S5P_TV) += s5p-tv/
diff --git a/drivers/media/video/mx2_emmaprp.c
b/drivers/media/video/mx2_emmaprp.c
new file mode 100644
index 000..ba89a74
--- /dev/null
+++ b/drivers/media/video/mx2_emmaprp.c
@@ -0,0 +1,1008 @@
+/*
+ * Support eMMa-PrP through mem2mem framework.
+ *
+ * eMMa-PrP is a piece of HW that allows fetching buffers
+ * from one memory location and do several operations on
+ * them such as scaling or format conversion giving, as a result
+ * a new processed buffer in another memory location.
+ *
+ * Based on mem2mem_testdev.c by Pawel Osciak.
+ *
+ * Copyright (c) 2011 Vista Silicon S.L.
+ * Javier Martin
+ *
+ * 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; either version 2 of the
+ * License, or (at your option) any later version
+ */
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define EMMAPRP_MODULE_NAME "mem2mem-emmaprp"
+
+MODULE_DESCRIPTION("Mem-to-mem device which supports eMMa-PrP present in mx2
SoCs");
+MODULE_AUTHOR("Javier Martin v4l2_dev, "%s: " fmt, __func__, ## arg)
+
+/* EMMA PrP */
+#define PRP_CNTL0x00
+#define PRP_INTR_CNTL 0x04
+#define PRP_INTRSTATUS 0x08
+#define PRP_SOURCE_Y_PTR0x0c
+#define PRP_SOURCE_CB_PTR 0x10
+#define PRP_SOURCE_CR_PTR 0x14
+#define PRP_DEST_RGB1_PTR 0x18
+#define PRP_DEST_RGB2_PTR 0x1c
+#define PRP_DEST_Y_PTR 0x20
+#define PRP_DEST_CB_PTR 0x24
+#define PRP_DEST_CR_PTR 0x28
+#define PRP_SRC_FRAME_SIZE 0x2c
+#define PRP_DEST_CH1_LINE_STRIDE0x30
+#define PRP_SRC_PIXEL_FORMAT_CNTL 0x34
+#define PRP_CH1_PIXEL_FORMAT_CNTL 0x38
+#define PRP_CH1_OUT_IMAGE_SIZE 0x3c
+#define PRP_CH2_OUT_IMAGE_SIZE 0x40
+#define PRP_SRC_LINE_STRIDE 0x44
+#define PRP_CSC_COEF_0120x48
+#define PRP_CSC_COEF_3450x4c
+#define PRP_CSC_COEF_6780x50
+#define PRP_CH1_RZ_HORI_COEF1 0x54
+#define PRP_CH1_RZ_HORI_COEF2 0x58
+#define PRP_CH1_RZ_HORI_VALID 0x5c
+#define PRP_CH1_RZ_VERT_COEF1 0x60
+#define PRP_CH1_RZ_VERT_COEF2 0x64
+#define PRP_CH1_RZ_VERT_VALID 0x68
+#define PRP_CH2_RZ_HORI_COEF1 0x6c
+#define PRP_CH2_RZ_HORI_COEF2 0x70
+#define PRP_CH2_RZ_HORI_VALID 0x74
+#define PRP_CH2_RZ_VERT_COEF1 0x78
+#define PRP_CH2_RZ_VERT_COEF2 0x7c
+#define PRP_CH2_RZ_VERT_VALID 0x80
+
+#define PRP_CNTL_CH1EN (1 << 0)
+#define PRP_CNTL_CH2EN (1 << 1)
+#define PRP_CNTL_CSIEN (1 << 2)
+#define PRP_CNTL_DATA_IN_YUV420 (0 << 3)
+#define PRP_CNTL_DATA_IN_YUV422 (1 << 3)
+#define PRP_CNTL_DATA_IN_RGB16 (2 << 3)
+#define PRP_CNTL_DATA_IN_RGB32 (3 << 3)
+#define PRP_CNTL_CH1_OUT
[PATCH v4 0/2] Add support form eMMa-PrP in i.MX2 chips as a mem2mem device.
Changes since v3: Patch order inverted as requested by Mauro. Now adjusting of the image dimensions is made using v4l_bound_align_image(). Some coding style fixes. -- 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: Mantis CAM not SMP safe / Activating CAM on Technisat Skystar HD2 (DVB-S2)
On 12/10/2011 01:57 AM, Ninja wrote:
Hi,
has anyone an idea how the SMP problems could be fixed?
You could turn on Mantis Kernel module's debug messages.
It could tell you the emitted interrupts.
One risky thing with the Interrupt handler code is that
MANTIS_GPIF_STATUS is cleared, even though IRQ0 isn't active yet.
This could lead to a rare starvation of the wait queue you described.
I supplied a patch below. Does it help?
I did some further investigation. When comparing the number of interrupts with
all cores enabled and the interrupts with only one core enabled it seems like
only the IRQ0 changed, the other IRQs and the total number stays quite the same:
4 Cores:
All IRQ/sec: 493
Masked IRQ/sec: 400
Unknown IRQ/sec: 0
DMA/sec: 400
IRQ-0/sec: 143
IRQ-1/sec: 0
OCERR/sec: 0
PABRT/sec: 0
RIPRR/sec: 0
PPERR/sec: 0
FTRGT/sec: 0
RISCI/sec: 258
RACK/sec: 0
1 Core:
All IRQ/sec: 518
Masked IRQ/sec: 504
Unknown IRQ/sec: 0
DMA/sec: 504
IRQ-0/sec: 246
IRQ-1/sec: 0
OCERR/sec: 0
PABRT/sec: 0
RIPRR/sec: 0
PPERR/sec: 0
FTRGT/sec: 0
RISCI/sec: 258
RACK/sec: 0
So, where might be the problem?
Turning on Mantis debug messages, might tell the difference between these
interrupts.
I hope somebody can help, because I think we are very close to a fully
functional CAM here.
I ran out of things to test to get closer to the solution :(
Btw: Is there any documentation available for the mantis PCI bridge?
Not that I know.
Manuel
--
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
Regards,
Marko Ristola
--- PATCH --
Mantis/Hopper: Check and clear GPIF status bits only when IRQ0 bit is active.
Signed-off-by: Marko Ristola
diff --git a/drivers/media/dvb/mantis/hopper_cards.c
b/drivers/media/dvb/mantis/hopper_cards.c
index 71622f6..c2084e9 100644
--- a/drivers/media/dvb/mantis/hopper_cards.c
+++ b/drivers/media/dvb/mantis/hopper_cards.c
@@ -84,15 +84,6 @@ static irqreturn_t hopper_irq_handler(int irq, void *dev_id)
if (!(stat & mask))
return IRQ_NONE;
- rst_mask = MANTIS_GPIF_WRACK |
- MANTIS_GPIF_OTHERR |
- MANTIS_SBUF_WSTO |
- MANTIS_GPIF_EXTIRQ;
-
- rst_stat = mmread(MANTIS_GPIF_STATUS);
- rst_stat &= rst_mask;
- mmwrite(rst_stat, MANTIS_GPIF_STATUS);
-
mantis->mantis_int_stat = stat;
mantis->mantis_int_mask = mask;
dprintk(MANTIS_DEBUG, 0, "\n-- Stat=<%02x> Mask=<%02x> --", stat, mask);
@@ -101,6 +92,16 @@ static irqreturn_t hopper_irq_handler(int irq, void *dev_id)
}
if (stat & MANTIS_INT_IRQ0) {
dprintk(MANTIS_DEBUG, 0, "<%s>", label[1]);
+
+ rst_mask = MANTIS_GPIF_WRACK |
+ MANTIS_GPIF_OTHERR |
+ MANTIS_SBUF_WSTO |
+ MANTIS_GPIF_EXTIRQ;
+
+ rst_stat = mmread(MANTIS_GPIF_STATUS);
+ rst_stat &= rst_mask;
+ mmwrite(rst_stat, MANTIS_GPIF_STATUS);
+
mantis->gpif_status = rst_stat;
wake_up(&ca->hif_write_wq);
schedule_work(&ca->hif_evm_work);
diff --git a/drivers/media/dvb/mantis/mantis_cards.c
b/drivers/media/dvb/mantis/mantis_cards.c
index c2bb90b..109a5fb 100644
--- a/drivers/media/dvb/mantis/mantis_cards.c
+++ b/drivers/media/dvb/mantis/mantis_cards.c
@@ -92,15 +92,6 @@ static irqreturn_t mantis_irq_handler(int irq, void *dev_id)
if (!(stat & mask))
return IRQ_NONE;
- rst_mask = MANTIS_GPIF_WRACK |
- MANTIS_GPIF_OTHERR |
- MANTIS_SBUF_WSTO |
- MANTIS_GPIF_EXTIRQ;
-
- rst_stat = mmread(MANTIS_GPIF_STATUS);
- rst_stat &= rst_mask;
- mmwrite(rst_stat, MANTIS_GPIF_STATUS);
-
mantis->mantis_int_stat = stat;
mantis->mantis_int_mask = mask;
dprintk(MANTIS_DEBUG, 0, "\n-- Stat=<%02x> Mask=<%02x> --", stat, mask);
@@ -109,6 +100,15 @@ static irqreturn_t mantis_irq_handler(int irq, void
*dev_id)
}
if (stat & MANTIS_INT_IRQ0) {
dprintk(MANTIS_DEBUG, 0, "<%s>", label[1]);
+ rst_mask = MANTIS_GPIF_WRACK |
+ MANTIS_GPIF_OTHERR |
+ MANTIS_SBUF_WSTO |
+ MANTIS_GPIF_EXTIRQ;
+
+ rst_stat = mmread(MANTIS_GPIF_STATUS);
+ rst_stat &= rst_mask;
+ mmwrite(rst_stat, MANTIS_GPIF_STATUS);
+
mantis->gpif_status = rst_stat;
wake_up(&ca->hif_write_wq);
schedule_work(&ca->hif_evm_work);
--
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.ke
Re: [RFC] Resolution change support in video codecs in v4l2
Hi Mauro, On Tuesday 06 December 2011 16:40:11 Mauro Carvalho Chehab wrote: > On 06-12-2011 13:19, Kamil Debski wrote: > > On 06 December 2011 15:42 Mauro Carvalho Chehab wrote: > >> On 06-12-2011 12:28, 'Sakari Ailus' wrote: > >>> On Tue, Dec 06, 2011 at 01:00:59PM +0100, Laurent Pinchart wrote: > >>> ... > >>> > > 2) new requirement is for a bigger buffer. DMA transfers need to > > be stopped before actually writing inside the buffer (otherwise, > > memory will be corrupted). > > > > In this case, all queued buffers should be marked with an error > > flag. So, both V4L2_BUF_FLAG_FORMATCHANGED and > > V4L2_BUF_FLAG_ERROR should raise. The new format should be > > available via G_FMT. > > I'd like to reword this as follows: > > 1. In all cases, the application needs to be informed that the format > has changed. > > V4L2_BUF_FLAG_FORMATCHANGED (or a similar flag) is all we need. G_FMT > will report the new format. > > 2. In all cases, the application must have the option of reallocating > buffers if it wishes. > > In order to support this, the driver needs to wait until the > application acknowledged the format change before it starts decoding > the stream. Otherwise, if the codec started decoding the new stream > to the existing buffers by itself, applications wouldn't have the > option of freeing the existing buffers and allocating smaller ones. > > STREAMOFF/STREAMON is one way of acknowledging the format change. I'm > not opposed to other ways of doing that, but I think we need an > acknowledgment API to tell the driver to proceed. > >>> > >>> Forcing STRAEMOFF/STRAEMON has two major advantages: > >>> > >>> 1) The application will have an ability to free and reallocate buffers > >>> if it wishes so, and > >>> > >>> 2) It will get explicit information on the changed format. Alternative > >>> would require an additional API to query the format of buffers in cases > >>> the information isn't implicitly available. > >> > >> As already said, a simple flag may give this meaning. Alternatively (or > >> complementary, an event may be generated, containing the new format). > >> > >>> If we do not require STRAEMOFF/STREAMON, the stream would have to be > >>> paused until the application chooses to continue it after dealing with > >>> its buffers and formats. > >> > >> No. STREAMOFF is always used to stop the stream. We can't make it mean > >> otherwise. > >> > >> So, after calling it, application should assume that frames will be > >> lost, while the DMA engine doesn't start again. For live capture devices, sure, but we're talking about memory-to-memory here. If the hardware is stopped compressed buffers on the OUTPUT queue won't be decoded anymore, and userspace won't be able to queue more buffers to the driver. There will be no frame loss on the kernel side. > > Do you mean all buffers or just those that are queued in hardware? > > Of course the ones queued. > > > What has been processed stays processed, it should not matter to the > > buffers that have been processed. > > Sure. > > > The compressed buffer that is queued in the driver and that caused the > > resolution change is on the OUTPUT queue. > > Not necessarily. If the buffer is smaller than the size needed for the > resolution change, what is there is trash, as it could be a partially > filled buffer or an empty buffer, depending if the driver detected about > the format change after or before start filling it. Were's talking about video decoding. On the OUTPUT queue the driver receives compressed buffers. At some point the compressed buffers contain information to notifies the driver and/or hardware of a resolution change in the compressed stream. At the point the buffers on the CAPTURE queue might not be suitable anymore, but there's no issue with buffers on the OUTPUT queue. If STREAMOFF is then called on the CAPTURE queue only, buffers already queued on the OUTPUT queue will stay queued. > > STREMOFF is only done on the CAPTURE queue, so it stays queued and > > information is retained. > > > > From CAPTURE all processed buffers have already been dequeued, so yes > > the content of the buffers queued in hw is lost. But this is ok, because > > after the resolution change the previous frames are not used in > > prediction. > > No. According with the spec: > > The VIDIOC_STREAMON and VIDIOC_STREAMOFF ioctl start and stop the > capture > or output process during streaming (memory mapping or user pointer) I/O. > > Specifically the capture hardware is disabled and no input buffers are > filled (if there are any empty buffers in the incoming queue) until > VIDIOC_STREAMON has been called. Accordingly the output hardware is > disabled, no video signal is produced until VIDIOC_STREAMON has been > called. The ioctl will succeed only when at
RE: [RFC] Resolution change support in video codecs in v4l2
Hi, > From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] > Sent: 12 December 2011 11:59 > > Hi Kamil, > > On Tuesday 06 December 2011 16:03:33 Kamil Debski wrote: > > On 06 December 2011 15:36 Sakari Ailus wrote: > > > On Fri, Dec 02, 2011 at 02:50:17PM -0200, Mauro Carvalho Chehab wrote: > > > > On 02-12-2011 11:57, Sakari Ailus wrote: > > > > > Some codecs need to be able to access buffers which have already > been > > > > > decoded to decode more buffers. Key frames, simply. > > > > > > > > Ok, but let's not add unneeded things at the API if you're not sure. > If > > > > we have such need for a given hardware, then add it. Otherwise, keep > it > > > > simple. > > > > > > This is not so much dependent on hardware but on the standards which the > > > cdoecs implement. > > > > > > > > The user space still wants to be able to show these buffers, so a > new > > > > > flag would likely be required --- V4L2_BUF_FLAG_READ_ONLY, for > > > > > example. > > > > > > > > Huh? Assuming a capture device, when kernel makes a buffer available > to > > > > userspace, kernel should not touch on it anymore (not even for read - > > > > although reading from it probably won't cause any issues, as video > > > > applications in general don't write into those buffers). The opposite > is > > > > true for output devices: once userspace fills it, and queues, it > should > > > > not touch that buffer again. > > > > > > > > This is part of the queue/dequeue logic. I can't see any need for an > > > > extra flag to explicitly say that. > > > > > > There is a reason to do so. An example of this is below. The > > > memory-to-memory device has two queues, output can capture. A video > > > decoder memory-to-memory device's output queue handles compressed video > > > and the capture queue provides the application decoded frames. > > > > > > Certain frames in the stream are key frames, meaning that the decoding > of > > > the following non-key frames requires access to the key frame. The > number > > > of non-key frame can be relatively large, say 16, depending on the > > > codec. > > > > > > If the user should wait for all the frames to be decoded before the key > > > frame can be shown, then either the key frame is to be skipped or > > > delayed. Both of the options are highly undesirable. > > > > I don't think that such a delay is worrisome. This is only initial delay. > > The hw will process these N buffers and after that it works exactly the > > same as it would without the delay in terms of processing time. > > For offline video decoding (such as playing a movie for instance) that's > probably not a big issue. For online video decoding (video conferencing) > where > you want to minimize latency it can be. In this use case it would be good to setup the encoder to use as little reference frames as possible. The lesser reference frames are used the shorter is the delay. The stream used for video conferencing should definitely be different from the one used in DVD/Blu-ray. Also you can set the display delay to 0 then you will get the frames as soon as possible, but it's up to the application to display them in the right order and to make sure that they are not modified. > > > Alternatively one could allocate the double number of buffers required. > > > At 1080p and 16 buffers this could be roughly 66 MB. Additionally, > > > starting the playback is delayed for the duration for the decoding of > > > those frames. I think we should not force users to do so. > > > > I really don't think it is necessary to allocate twice as many buffers. > > Assuming that hw needs K buffers you may alloc N (= K + L) and the > > application may use all these L buffers at a time. > Best wishes, -- Kamil Debski Linux Platform Group 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: [RFC] Resolution change support in video codecs in v4l2
Hi Kamil, On Tuesday 06 December 2011 16:03:33 Kamil Debski wrote: > On 06 December 2011 15:36 Sakari Ailus wrote: > > On Fri, Dec 02, 2011 at 02:50:17PM -0200, Mauro Carvalho Chehab wrote: > > > On 02-12-2011 11:57, Sakari Ailus wrote: > > > > Some codecs need to be able to access buffers which have already been > > > > decoded to decode more buffers. Key frames, simply. > > > > > > Ok, but let's not add unneeded things at the API if you're not sure. If > > > we have such need for a given hardware, then add it. Otherwise, keep it > > > simple. > > > > This is not so much dependent on hardware but on the standards which the > > cdoecs implement. > > > > > > The user space still wants to be able to show these buffers, so a new > > > > flag would likely be required --- V4L2_BUF_FLAG_READ_ONLY, for > > > > example. > > > > > > Huh? Assuming a capture device, when kernel makes a buffer available to > > > userspace, kernel should not touch on it anymore (not even for read - > > > although reading from it probably won't cause any issues, as video > > > applications in general don't write into those buffers). The opposite is > > > true for output devices: once userspace fills it, and queues, it should > > > not touch that buffer again. > > > > > > This is part of the queue/dequeue logic. I can't see any need for an > > > extra flag to explicitly say that. > > > > There is a reason to do so. An example of this is below. The > > memory-to-memory device has two queues, output can capture. A video > > decoder memory-to-memory device's output queue handles compressed video > > and the capture queue provides the application decoded frames. > > > > Certain frames in the stream are key frames, meaning that the decoding of > > the following non-key frames requires access to the key frame. The number > > of non-key frame can be relatively large, say 16, depending on the > > codec. > > > > If the user should wait for all the frames to be decoded before the key > > frame can be shown, then either the key frame is to be skipped or > > delayed. Both of the options are highly undesirable. > > I don't think that such a delay is worrisome. This is only initial delay. > The hw will process these N buffers and after that it works exactly the > same as it would without the delay in terms of processing time. For offline video decoding (such as playing a movie for instance) that's probably not a big issue. For online video decoding (video conferencing) where you want to minimize latency it can be. > > Alternatively one could allocate the double number of buffers required. > > At 1080p and 16 buffers this could be roughly 66 MB. Additionally, > > starting the playback is delayed for the duration for the decoding of > > those frames. I think we should not force users to do so. > > I really don't think it is necessary to allocate twice as many buffers. > Assuming that hw needs K buffers you may alloc N (= K + L) and the > application may use all these L buffers at a time. -- 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] Resolution change support in video codecs in v4l2
> -Original Message- > From: 'Sakari Ailus' [mailto:sakari.ai...@iki.fi] > Sent: 09 December 2011 20:55 > To: Kamil Debski > Cc: 'Mauro Carvalho Chehab'; linux-media@vger.kernel.org; 'Laurent Pinchart'; > 'Sebastian Dröge'; Sylwester Nawrocki; Marek Szyprowski > Subject: Re: [RFC] Resolution change support in video codecs in v4l2 > > Hi Kamil, > > On Tue, Dec 06, 2011 at 04:03:33PM +0100, Kamil Debski wrote: > ... > > > > >The user space still wants to be able to show these buffers, so a new > > > flag > > > > >would likely be required --- V4L2_BUF_FLAG_READ_ONLY, for example. > > > > > > > > Huh? Assuming a capture device, when kernel makes a buffer available > to > > > userspace, > > > > kernel should not touch on it anymore (not even for read - although > > > reading from > > > > it probably won't cause any issues, as video applications in general > don't > > > write > > > > into those buffers). The opposite is true for output devices: once > > > userspace fills it, > > > > and queues, it should not touch that buffer again. > > > > > > > > This is part of the queue/dequeue logic. I can't see any need for an > extra > > > > flag to explicitly say that. > > > > > > There is a reason to do so. An example of this is below. The > > > memory-to-memory device has two queues, output can capture. A video > decoder > > > memory-to-memory device's output queue handles compressed video and the > > > capture queue provides the application decoded frames. > > > > > > Certain frames in the stream are key frames, meaning that the decoding > of > > > the following non-key frames requires access to the key frame. The > number of > > > non-key frame can be relatively large, say 16, depending on the codec. > > > > > > If the user should wait for all the frames to be decoded before the key > > > frame can be shown, then either the key frame is to be skipped or > delayed. > > > Both of the options are highly undesirable. > > > > I don't think that such a delay is worrisome. This is only initial delay. > > The hw will process these N buffers and after that it works exactly the > same > > as it would without the delay in terms of processing time. > > Well, yes, but consider that the decoder also processes key frames when the > decoding is in progress. The dequeueing of the key frames (and any further > frames as long as the key frame is needed by the decoder) will be delayed > until the key frame is no longer required. > > You need extra buffers to cope with such a situation, and in the worst case, > or when the decoder is just as fast as you want to show the frames on the > display, you need double the amount of buffers compared to what you'd really > need for decoding. To make matters worse, this tends to happen at largest > resolutions. > > I think we'd like to avoid this. I really, really, dont see why you say that we would need double the number of buffers? Let's suppose that the stream may reference 2 previous frames. Frame number: 123456789ABCDEF Returned frame: 123456789ABCDEF Buffers returned: 123123123123... (in case we have only 3 buffers) See? After we decode frame number 3 we can return frame number 3. Thus we need minimum of 3 buffers. If we want to have 4 for simultaneous the use of application we allocate 7. The current codec handling system has been build on the following assumptions: - the buffers should be dequeued in order - the buffers should be only dequeued when they are no longer is use This takes care of the delay related problems by requiring more buffers. You have an initial delay then the frames are returned with a constant rate. Dequeuing of any frame will be delayed until it is no longer used - it doesn't matter whether it is a key (I) frame, P frame o r a B frame. Actually B frames shouldn't be used as reference. Usually a frame is referencing only 2-3 previous and maybe 1 ahead (for B-frames) frames and they don't need to be I-frames. Still the interval between I-frames may be 16 or even many, many, more. In your other email you have mentioned "acceleration". I can agree with you that it makes the process faster than decoding the same compressed stream many times. I have never seen any implementation that would process the same compressed stream more than once. Thus I would not say it's purely for acceleration. This is the way it is done - you keep older decompressed frames for reference. Reprocessing the compressed stream would be too computation demanding I suppose. Anyway I can definitely recommend the book "H.264 and MPEG-4 video compression: Video coding for next-generation multimedia" by Iain E.G. Richardson. It is a good book about video coding and modern codecs with many things explained. It would help to get you around with codecs and could answer many of your questions. http://www.amazon.com/H-264-MPEG-4-Video-Compression-Generation/dp/0470848375 Best wishes, -- Kamil Debski Linux Platform Group Samsung Poland R&D Center -- To unsubscribe from this
Re: [RFC PATCH v1 6/7] media: video: introduce face detection driver module
Hi, On Mon, Dec 12, 2011 at 1:43 AM, Sylwester Nawrocki >> For OMAP4 FD, it is not needed to include FD into MC framework since a >> intermediate buffer is always required. If your HW doesn't belong to this >> case, what is the output of your HW FD in the link? Also sounds FD results >> may not be needed at all for use space application in the case. > > The result data is similar to OMAP4 one, plus a few other attributes. > User buffers may be filled by other than FD device driver. OK. >> Could you provide some practical use cases about these? > > As above, and any device with a camera that controls something and makes > decision according to presence of human face in his view. Sounds a reasonable case, :-) >> If FD result is associated with a frame, how can user space get the frame seq >> if no v4l2 buffer is involved? Without a frame sequence, it is a bit >> difficult to retrieve FD results from user space. > > If you pass image data in memory buffers from user space, yes, it could be > impossible. It is easy to get the frame sequence from v4l2_buffer for the case too, :-) > > Not really, still v4l2_buffer may be used by other (sub)driver within same > video > processing pipeline. OK. A related question: how can we make one application to support the two kinds of devices(input from user space data as OMAP4, input from SoC bus as Samsung) at the same time? Maybe some capability info is to be exported to user space? or other suggestions? And will your Samsung FD HW support to detect faces from memory? or just only detect from SoC bus? > It will be included in the FD result... or in a dedicated v4l2 event data > structure. > More important, at the end of the day, we'll be getting buffers with image > data > at some stage of a video pipeline, which would contain same frame identifier > (I think we can ignore v4l2_buffer.field for FD purpose). OK, I will associate FD result with frame identifier, and not invent a dedicated v4l2 event for query frame seq now until a specific requirement for it is proposed. I will convert/integrate recent discussions into patches of v2 for further review, and sub device support will be provided. But before starting to do it, I am still not clear how to integrate FD into MC framework. I understand FD sub device is only a media entity, so how can FD sub device find the media device(struct media_device)? or just needn't to care about it now? thanks, -- Ming Lei -- 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
