Hi Hans, > From: Hans Verkuil [mailto:hverkuil@xxxxxxxxx] > Hi Kamil, > > I still need to review this carefully since this is the first codec > driver. > I had hoped to do this during the weekend, but I didn't manage it. I > hope I > can get to it on Friday. This would be great. > One thing I noticed: you aren't using the control framework in this > driver. > Please switch to that. From now on I will NACK any new driver that is > not > using that framework. I'm in the process of converting all existing > drivers > to it, and I don't want to have to fix new drivers as well :-) > > Documentation is in Documentation/video4linux/v4l2-controls.txt. > I am aware of that. I think my reply to one of your previous comments might have got lost in your inbox. I have doubts about the point of using the control framework in my case. I agree with you that for the majority of drivers it will be very useful. In case of MFC or mem2mem FIMC - those two drivers use per file handle contexts and that's the thing that stops me from using your framework. If I understand the control framework correctly, there is no support for such use case. When S_CTRL is run on an MFC driver then the value passed to the driver is stored in s5p_mfc_ctx structure instead of writing this to hardware directly. This value is then used, when it is necessary to setup hardware. For example before running codec initialization for that context. In case of the control framework, the I could implement my .s_ctrl callback which sets the appropriate field in s5p_mfc_ctx structure. The thing I don't like about this is the redundancy of storing the control value. In the one instance scenario it is stored in the driver in s5p_mfc_ctx and in the control fw. In multiple instance scenario the values in s5p_mfc_ctx and in the control fw will be different. I know that there are volatile controls in the framework, but in this case all controls would have to be volatile. Correct me if I am wrong, but I don't see that the control framework is for mem2mem drivers that can have multiple contexts. Again, I agree with you that it is generally a good idea, but not for every driver. I am open to discuss this use case with you - maybe some functionality could be added to the control framework? Best wishes, -- Kamil Debski Linux Platform Group Samsung Poland R&D Center > > On Friday, January 07, 2011 17:25:30 Kamil Debski wrote: > > Hello, > > > > This is the sixth version of the MFC 5.1 driver, decoding part. The > suggestions > > and comments from the group members have been very helpful. > > > > I would be grateful for your comments. Original cover letter ant > detailed change > > log has been attached below. > > > > Best regards, > > Kamil Debski <snip> -- 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
