Hi Sungchun, thanks for your suggestions. I am planning to improve output DMA handling in the fimc camera capture driver, what is done already is only a minimal adaptation to make driver work on S5PV310 SoC. We don't even have a platform support (resource and platform device definitions) for FIMC@S5PV310 yet though. I think it needs to be done first. Your proposed scheme of presetting output DMA buffer adresses before streaming is enabled and then using the CIFCNTSEQ register to mask out buffers passed to user space looks reasonable to me. I could imagine adopting such a method for buffer of V4L2_MEMORY_MMAP memory type. But do you think it is going to work for V4L2_MEMORY_USERPTR? There is no confidence that USERPTR addresses passed by applications in subsequent VIDIOC_QBUF calls will not be changing, right? So we would have to walk the list of buffers' addresses written into CIOYSAn registers and perform a write if our new incoming buffer is not there anyway. Nevertheless I think it would be cleaner and more safe to use your proposed method. Did you observe any problems when DMA buffers addresses are reconfigured within the ISR in S5PV310? I mean are there any good reasons against that? I know there are some issues on S5PV210 about that and I am going fix it too. On 12/03/2010 08:30 AM, Sungchun Kang wrote: > Hi Sylwester Nawrocki, > > I have some suggestion about s5p-fimc camera driver. > As you know, FIMC have 32 ping-pong register(CIOYSA1~CIOYSA32) for output > buffer in v310 chip. It is different from v210. > It is not necessary to change address at ping-pong register. > If request buffer number is 8, 8 buffer can be set at ping-pong register. > > - active_buf_q / pending_buf_q > > The list_head pending_buf_q is not necessary for v310. Because, it is possible > to set output buffer until 32. > There is no case that request buffer number is over 32. Ok, I think it is safe to make such assumption. > So, I think pending_buf_q is not use in v310. Instead, it can be used active_buf_q. > Of course, The use of active_buf_q may be different from the present. > > For intstance, ISR may be changed follow. > > a. Read FrameCnt_before(CISTATUS2) - This is for what is written buffer(index) just. > b. Disable a proper bit of FrameCnt_Seq(CIFCNTSEQ) and send written buffer to done_list. > .... And we we wanted to reschedule same buffer again we would have to search in all 32 registers to match the address of a buffer passed back by the application, right? > > I want to know how about your opinions. In general I would like to avoid significant differences of hardware handling across various SoC variants as much as possible. However also I realize that some differences cannot be avoided for optimal hardware handling. And please feel free to submit patches, however I think the best time for those would be after the driver is adopted to videobuf2. Now it is changing frequently. Regards, Sylwester > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Sylwester Nawrocki Samsung Poland R&D Center -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
