On Fri, Oct 7, 2011 at 10:54 AM, Enrico <ebutera@xxxxxxxxxxxxxxxx> wrote: > 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.... > Great, good to know that I'm not the only one that noticed this behavior. > >> 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 second you on that, we need someone who knows the ISP better than we do. I have to fix this anyway, so it is better if I can do it the right way and the code gos upstream, so we don't have to internally maintain a separate patch-set and forward port for each kernel release we do. >>> - 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 > -- Javier Martínez Canillas (+34) 682 39 81 69 Barcelona, Spain -- 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
