Re: [PATCH 1/1] smiapp: Add driver-specific control class, test pattern controls

Sakari Ailus
2014-05-28T12:25:25Z

Laurent Pinchart wrote:
> Hi Sakari,
>
> On Wednesday 28 May 2014 15:16:58 Sakari Ailus wrote:
>> Laurent Pinchart wrote:
>>> On Wednesday 28 May 2014 12:00:38 Sakari Ailus wrote:
>>>> Add smiapp driver specific control sub-class for test pattern controls.
>>>> More controls are expected since a fair amount of the standard
>>>> functionality is still unsupported. There are sensor model specific
>>>> functionality as well and expectedly thus also sensor specific controls.
>>>> So reserve 128 controls for this driver.
>>>>
>>>> This patch also adds test pattern controls for the four colour
>>>> components.
>>>>
>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>>> ---
>>>> This patch comes before the previous patch I sent to the thread. I missed
>>>> this when sending it.
>>>>
>>>> include/uapi/linux/smiapp.h | 34 +++++++++++++++++++++++++++++++
>>>> include/uapi/linux/v4l2-controls.h | 4 ++++
>>>> 2 files changed, 38 insertions(+)
>>>> create mode 100644 include/uapi/linux/smiapp.h
>>>>
>>>> diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h
>>>> new file mode 100644
>>>> index 0000000..116fc69
>>>> --- /dev/null
>>>> +++ b/include/uapi/linux/smiapp.h
>>>> @@ -0,0 +1,34 @@
>>>> +/*
>>>> + * include/media/smiapp.h
>>>> + *
>>>> + * Generic driver for SMIA/SMIA++ compliant camera modules
>>>> + *
>>>> + * Copyright (C) 2014 Intel Corporation
>>>> + * Contact: Sakari Ailus <sakari.ailus@iki.fi>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU General Public License
>>>> + * version 2 as published by the Free Software Foundation.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful, but
>>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>>> + * General Public License for more details.
>>>> + *
>>>> + */
>>>> +
>>>> +#ifndef __UAPI_LINUX_SMIAPP_H_
>>>> +#define __UAPI_LINUX_SMIAPP_H_
>>>> +
>>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED 0
>>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR 1
>>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS 2
>>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY 3
>>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9 4
>>>> +
>>>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_RED (V4L2_CID_USER_SMIAPP_BASE |
>>>> 0x01)
>>>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENR (V4L2_CID_USER_SMIAPP_BASE |
>>>> 0x02)
>>>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_BLUE (V4L2_CID_USER_SMIAPP_BASE |
>>>> 0x03)
>>>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENB (V4L2_CID_USER_SMIAPP_BASE |
>>>> 0x04)
>>>
>>> Wouldn't it make sense to create a standard test pattern color control
>>> instead ? Several sensors can control the test pattern color in a way or
>>> another. Some of them might need more than one color though, so I'm not
>>> sure how much standardization would be possible.
>>
>> Now that you mention it, I'd guess many raw bayer sensors can set
>> colours for the test pattern (or image). The menu control has no
>> standardised values so I didn't think of standardising controls that
>> depend on it.
>>
>> I'll update the patches (and add a new one for the standard controls).
>
> The color format might differ between devices though, some might not be able
> to differentiate between Gr and Gb for the test pattern. A standard test
> pattern color control should thus be flexible in the color format I suppose.

I think we should simply create a separate control for each component.
There aren't that many components after all.

--
Sakari Ailus
sakari.ailus@linux.intel.com