On Sat, Oct 8, 2011 at 5:51 PM, Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > Hi, > > On Friday 07 October 2011 11:31:46 Javier Martinez Canillas wrote: >> On Fri, Oct 7, 2011 at 10:54 AM, Enrico wrote: >> > On Thu, Oct 6, 2011 at 6:05 PM, Javier Martinez Canillas wrote: >> >> On Thu, Oct 6, 2011 at 5:25 PM, Enrico 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. > > Two quick comments, as I haven't had time to look into this recently. > > 1. I've updated the omap3isp-omap3isp-yuv branch with a new CCDC YUV support > patch which should (hopefully) configure the bridge automatically and report > correct formats at the CCDC output. The patch hasn't been tested as I still > don't have access to YUV hardware. > Hello Laurent, I'm glad to see that you are joining the thread :) > 2. Could you guys please rebase all your patches on top of the omap3isp- > omap3isp-yuv branch ? I will then review them. > Yes, I'll cook a patch today on top on your omap3isp-yuv and send for review. I won't be able to test neither since I don't have proper hardware at home. But at least you will get an idea of the approach we are using to solve this and can point possible flaws. >> >>> - 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. > > -- > Regards, > > Laurent Pinchart > Thanks a lot for your time. -- 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
