From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from smtp.nokia.com ([147.243.128.26]:41303 "EHLO mgw-da02.nokia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751778Ab2AOToG (ORCPT ); Sun, 15 Jan 2012 14:44:06 -0500 Message-ID: <4F132C82.9060105@maxwell.research.nokia.com> Date: Sun, 15 Jan 2012 21:44:02 +0200 From: Sakari Ailus MIME-Version: 1.0 To: Laurent Pinchart CC: linux-media@vger.kernel.org, dacohen@gmail.com, snjw23@gmail.com Subject: Re: [RFC 08/17] v4l: Image source control class References: <4EF0EFC9.6080501@maxwell.research.nokia.com> <201201051723.41247.laurent.pinchart@ideasonboard.com> <4F11EAD3.9070004@maxwell.research.nokia.com> <201201150243.05381.laurent.pinchart@ideasonboard.com> In-Reply-To: <201201150243.05381.laurent.pinchart@ideasonboard.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: Hi Laurent, Laurent Pinchart wrote: > On Saturday 14 January 2012 21:51:31 Sakari Ailus wrote: >> Laurent Pinchart wrote: >>> On Tuesday 20 December 2011 21:28:00 Sakari Ailus wrote: >>>> From: Sakari Ailus >>>> >>>> Add image source control class. This control class is intended to >>>> contain low level controls which deal with control of the image capture >>>> process --- the A/D converter in image sensors, for example. >>>> >>>> Signed-off-by: Sakari Ailus >>>> --- >>>> >>>> Documentation/DocBook/media/v4l/controls.xml | 101 >>>> +++++++++++++++++ .../DocBook/media/v4l/vidioc-g-ext-ctrls.xml | >>>> 6 + >>>> drivers/media/video/v4l2-ctrls.c | 10 ++ >>>> include/linux/videodev2.h | 10 ++ >>>> 4 files changed, 127 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/Documentation/DocBook/media/v4l/controls.xml >>>> b/Documentation/DocBook/media/v4l/controls.xml index 3bc5ee8..69ede83 >>>> 100644 >>>> --- a/Documentation/DocBook/media/v4l/controls.xml >>>> +++ b/Documentation/DocBook/media/v4l/controls.xml >>>> @@ -3356,6 +3356,107 @@ interface and may change in the future. >>>> >>>> >>>> >>>> >>>> >>>> + >>>> +
>>>> + Image Source Control Reference >>>> + >>>> + >>>> + Experimental >>>> + >>>> + This is an >>> + linkend="experimental">experimental interface and may >>>> + change in the future. >>>> + >>>> + >>>> + >>>> + The Image Source control class is intended for low-level >>>> + control of image source devices such as image sensors. The >>>> + devices feature an analogue to digital converter and a bus >>> >>> Is the V4L2 documentation written in US or UK English ? US uses analog, >>> UK uses analogue. Analog would be shorter in control names. >> >> Both appear to be used, but the US English appears to be more commonly >> used. I guess it's mostly chosen by whatever happened to be used by the >> author of the patch. I prefer UK spelling which you might have noticed >> already. :-) > > Yes I have. I haven't checked whether V4L2 prefers the UK or US spelling. I'll > trust you on that. I have checked and most seem to have used US spelling. If you wish me to change it, I can do that. >> I remember there was a discussion on this topic years ago within the >> kernel community but I don't remember how it ended up with... LWN.net >> appears to remember better than I do, but by a quick check I can't find >> any definitive conclusion. >> >> >> >> >>>> + transmitter to transmit the image data out of the device. >>>> + >>>> + >>>> + >>>> + Image Source Control IDs >>>> + >>>> + >>>> + >>>> + >>>> + >>>> + >>>> + >>>> + >>>> + >>>> + >>>> + ID >>>> + Type >>>> + >>> align="left">Description + >>>> + >>>> + >>>> + >>>> + >>>> + >>> spanname="id">V4L2_CID_IMAGE_SOURCE_CLASS + >>>> >>>> class >>>> >>>> + >>>> + >>>> + The IMAGE_SOURCE class descriptor. >>>> + >>>> + >>>> + >>> spanname="id">V4L2_CID_IMAGE_SOURCE_VBLANK >>>> + >>>> >>>> integer >>>> >>>> + >>>> + >>>> + Vertical blanking. The idle >>>> + preriod after every frame during which no image data is >>> >>> s/preriod/period/ >>> >>>> + produced. The unit of vertical blanking is a line. Every >>>> + line has length of the image width plus horizontal >>>> + blanking at the pixel clock specified by struct >>>> + v4l2_mbus_framefmt >>> + />. >>>> + >>>> + >>>> + >>> spanname="id">V4L2_CID_IMAGE_SOURCE_HBLANK >>>> + >>>> >>>> integer >>>> >>>> + >>>> + >>>> + Horizontal blanking. The idle >>>> + preriod after every line of image data during which no >>> >>> s/preriod/period/ >>> >>>> + image data is produced. The unit of horizontal blanking is >>>> + pixels. >>>> + >>>> + >>>> + >>> spanname="id">V4L2_CID_IMAGE_SOURCE_LINK_FREQ>>> y> + integer menu >>>> + >>>> + >>>> + Image source's data bus frequency. >>> >>> What's the frequency unit ? Sample/second ? >> >> Hz --- that's the unit of frequency. I fixed that in the new version. > > Is the user supposed to compute the pixel clock from this information ? That's > what the text below seems to imply. Apparently I have forgotten to update this in the new patchset. But yes, these factors do define it. The sensors' internal clock tree will be involved and calculation is non-trivial. This is why we also have the PIXEL_RATE control --- where I will refer to in the next patchset. >>>> + Together with the media bus pixel code, bus type (clock >>>> + cycles per sample), the data bus frequency defines the >>>> + pixel clock. The >>>> + frame rate can be calculated from the pixel clock, image >>>> + width and height and horizontal and vertical blanking. The >>>> + frame rate control is performed by selecting the desired >>>> + horizontal and vertical blanking. >>>> + >>>> + >>>> + >>>> + >>> spanname="id">V4L2_CID_IMAGE_SOURCE_ANALOGUE_GAIN>>> en try> + integer >>>> + >>>> + >>>> + Analogue gain is gain affecting >>>> + all colour components in the pixel matrix. The gain >>>> + operation is performed in the analogue domain before A/D >>>> + conversion. >>> >>> Should we define one gain per color component ? >> >> I think that in the end we may need up to six analogue gains: >> >> - Gain for all components > > Many sensors that provide per-component gains also provide a global gain > control that sets the four component gains. I'm not sure how (and if) we > should handle that. If it directly affects all of them, I don't think we should support it. But if it's independent of the colour-specific ones, then, sure, it should be supported. >> - Blue gain >> - Red gain >> - Green gain (for both greens) >> - Gr gain >> - Gb gain >> >> It may be debatable whether Gr / Gb gain will always be the same or not. >> I'm not fully certain about that. As Hans G. suggested, it might be >> possible to go with just one for green. > > I'm also unsure about that. Having different gains for the two green > components doesn't seem very useful. On the other hand, if we find a use case > later, we'll have to break driver ABIs. Let's add the gains later on. Now we'll need a single analogue gain for all components and that's good for the time being. >>>> + >>>> + >>>> + >>>> + >>>> + >>>> +
>>>> + >>>> +
>>>> + >>>> >>>> >>>> >>>>