On Thu, Oct 6, 2011 at 6:05 PM, Javier Martinez Canillas <martinez.javier@xxxxxxxxx> wrote: > On Thu, Oct 6, 2011 at 5:25 PM, Enrico <ebutera@xxxxxxxxxxxxxxxx> wrote: >> - i don't see Deepthy patches, it seems to be based on the >> pre-Deepthy-patches driver and fixed (not that this is a bad thing!); >> i say this because, like Gary, i'm interested in a possible forward >> porting to a more recent kernel so i was searching for a starting >> point >> > > I didn't know there was a more recent version of Deepthy patches, > Since they are not yet in mainline we should decide if we work on top > of that or on top of mainline. Deepthy patches are very good to > separate bt656 and non-bt656 execution inside the ISP, also add a > platform data variable to decide which mode has to be used. > > But reading the documentation and from my experimental validation I > think that there are a few things that can be improved. > > First the assumption that we can use FLDSTAT to check if a frame is > ODD or EVEN I find to not always be true. Also I don't know who sets > this value since in the TRM always talks as it is only used with > discrete syncs. Yes about FLDSTAT i noticed the same thing. And that's why we need someone that knows the ISP better to help us.... > Also, I don't think that we should change the ISP CCDC configuration > inside the VD0 interrupt handler. Since the shadowed registers only > can be accessed during a frame processing, or more formally the new > values are taken at the beginning of a frame execution. > > By the time we change for example the output address memory for the > next buffer in the VD0 handler, the next frame is already being > processed so that value won't be used for the CCDC until that frame > finish. So It is not behaving as the code expect, since for 3 frames > the CCDC output memory address will be the same. > > That is why I move most of the logic to the VD1 interrupt since there > the current frame didn't finish yet and we can configure the CCDC for > the next frame. > > But to do that the buffer for the next frame and the releasing of the > last buffer can't happen simultaneously, that is why I decouple these > two actions. > > Again, this is my own observations and what I understood from the TRM > and I could be wrong. I can't comment on that, i hope Laurent or Deepthy will join the discussion... >> - i don't think that adding the "priv" field in v4l2-mediabus.h will >> be accepted, and since it is related to the default cropping you added >> i think it can be dropped and just let the user choose the appropriate >> cropping >> > > Yes, probably is too much of a hack, but I didn't know of another way > that the subdev could report to the ISP of the standard and since > v4l2_pix_format has also a priv field, I think it could be at least a > temporary solution (remember that we want this to work first and then > we plan to do it right for upstream submission). ...and my hope continues here. Enrico -- 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
