diff mbox series

[1/5] media: mt9p031: Add support for 8 bit and 10 bit formats

Message ID 20200925075029.32181-1-s.riedmueller@phytec.de (mailing list archive)
State New, archived
Headers show
Series [1/5] media: mt9p031: Add support for 8 bit and 10 bit formats | expand

Commit Message

Stefan Riedmüller Sept. 25, 2020, 7:50 a.m. UTC 
From: Christian Hemp <c.hemp@phytec.de>

Aside from 12 bit monochrome or color format the sensor implicitly
supports 10 and 8 bit formats as well by simply dropping the
corresponding LSBs.

Signed-off-by: Christian Hemp <c.hemp@phytec.de>
[jlu@pengutronix.de: simplified by dropping v4l2_colorspace handling]
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
---
 drivers/media/i2c/mt9p031.c | 50 +++++++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 10 deletions(-)

Comments

kernel test robot Sept. 25, 2020, 10:30 a.m. UTC | #1 
Hi Stefan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v5.9-rc6 next-20200924]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Stefan-Riedmueller/media-mt9p031-Add-support-for-8-bit-and-10-bit-formats/20200925-155251
base:   git://linuxtv.org/media_tree.git master
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1863bce6d7b4791c048a25ec07ff8aec8b4847cf
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Stefan-Riedmueller/media-mt9p031-Add-support-for-8-bit-and-10-bit-formats/20200925-155251
        git checkout 1863bce6d7b4791c048a25ec07ff8aec8b4847cf
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/i2c/mt9p031.c:166:8: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
     166 | static const u32 mt9p031_find_datafmt(struct mt9p031 *mt9p031, u32 code)
         |        ^~~~~

vim +166 drivers/media/i2c/mt9p031.c

   165	
 > 166	static const u32 mt9p031_find_datafmt(struct mt9p031 *mt9p031, u32 code)
   167	{
   168		int i;
   169	
   170		for (i = 0; i < mt9p031->num_fmts; i++)
   171			if (mt9p031->fmts[i] == code)
   172				return mt9p031->fmts[i];
   173	
   174		return mt9p031->fmts[mt9p031->num_fmts-1];
   175	}
   176	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Sakari Ailus Sept. 25, 2020, 8:11 p.m. UTC | #2 
Hi Stefan,

On Fri, Sep 25, 2020 at 09:50:25AM +0200, Stefan Riedmueller wrote:
> From: Christian Hemp <c.hemp@phytec.de>
> 
> Aside from 12 bit monochrome or color format the sensor implicitly
> supports 10 and 8 bit formats as well by simply dropping the
> corresponding LSBs.
> 
> Signed-off-by: Christian Hemp <c.hemp@phytec.de>
> [jlu@pengutronix.de: simplified by dropping v4l2_colorspace handling]
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
> ---
>  drivers/media/i2c/mt9p031.c | 50 +++++++++++++++++++++++++++++--------
>  1 file changed, 40 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index dc23b9ed510a..0002dd299ffa 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -116,6 +116,18 @@ enum mt9p031_model {
>  	MT9P031_MODEL_MONOCHROME,
>  };
>  
> +static const u32 mt9p031_color_fmts[] = {
> +	MEDIA_BUS_FMT_SGRBG8_1X8,
> +	MEDIA_BUS_FMT_SGRBG10_1X10,
> +	MEDIA_BUS_FMT_SGRBG12_1X12,
> +};
> +
> +static const u32 mt9p031_monochrome_fmts[] = {
> +	MEDIA_BUS_FMT_Y8_1X8,
> +	MEDIA_BUS_FMT_Y10_1X10,
> +	MEDIA_BUS_FMT_Y12_1X12,
> +};
> +
>  struct mt9p031 {
>  	struct v4l2_subdev subdev;
>  	struct media_pad pad;
> @@ -138,6 +150,9 @@ struct mt9p031 {
>  	struct v4l2_ctrl *blc_auto;
>  	struct v4l2_ctrl *blc_offset;
>  
> +	const u32 *fmts;
> +	int num_fmts;

Unsigned int, please.

> +
>  	/* Registers cache */
>  	u16 output_control;
>  	u16 mode2;
> @@ -148,6 +163,17 @@ static struct mt9p031 *to_mt9p031(struct v4l2_subdev *sd)
>  	return container_of(sd, struct mt9p031, subdev);
>  }
>  
> +static const u32 mt9p031_find_datafmt(struct mt9p031 *mt9p031, u32 code)
> +{
> +	int i;

Same here.

> +
> +	for (i = 0; i < mt9p031->num_fmts; i++)
> +		if (mt9p031->fmts[i] == code)
> +			return mt9p031->fmts[i];
> +
> +	return mt9p031->fmts[mt9p031->num_fmts-1];
> +}
> +
>  static int mt9p031_read(struct i2c_client *client, u8 reg)
>  {
>  	return i2c_smbus_read_word_swapped(client, reg);
> @@ -476,10 +502,11 @@ static int mt9p031_enum_mbus_code(struct v4l2_subdev *subdev,
>  {
>  	struct mt9p031 *mt9p031 = to_mt9p031(subdev);
>  
> -	if (code->pad || code->index)
> +	if (code->pad || code->index >= mt9p031->num_fmts)
>  		return -EINVAL;
>  
> -	code->code = mt9p031->format.code;
> +	code->code = mt9p031->fmts[code->index];
> +
>  	return 0;
>  }
>  
> @@ -573,6 +600,8 @@ static int mt9p031_set_format(struct v4l2_subdev *subdev,
>  	__format->width = __crop->width / hratio;
>  	__format->height = __crop->height / vratio;
>  
> +	__format->code = mt9p031_find_datafmt(mt9p031, format->format.code);
> +
>  	format->format = *__format;
>  
>  	return 0;
> @@ -951,10 +980,7 @@ static int mt9p031_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh)
>  
>  	format = v4l2_subdev_get_try_format(subdev, fh->pad, 0);
>  
> -	if (mt9p031->model == MT9P031_MODEL_MONOCHROME)
> -		format->code = MEDIA_BUS_FMT_Y12_1X12;
> -	else
> -		format->code = MEDIA_BUS_FMT_SGRBG12_1X12;
> +	format->code = mt9p031_find_datafmt(mt9p031, 0);
>  
>  	format->width = MT9P031_WINDOW_WIDTH_DEF;
>  	format->height = MT9P031_WINDOW_HEIGHT_DEF;
> @@ -1121,10 +1147,14 @@ static int mt9p031_probe(struct i2c_client *client,
>  	mt9p031->crop.left = MT9P031_COLUMN_START_DEF;
>  	mt9p031->crop.top = MT9P031_ROW_START_DEF;
>  
> -	if (mt9p031->model == MT9P031_MODEL_MONOCHROME)
> -		mt9p031->format.code = MEDIA_BUS_FMT_Y12_1X12;
> -	else
> -		mt9p031->format.code = MEDIA_BUS_FMT_SGRBG12_1X12;
> +	if (mt9p031->model == MT9P031_MODEL_MONOCHROME) {
> +		mt9p031->fmts = mt9p031_monochrome_fmts;
> +		mt9p031->num_fmts = ARRAY_SIZE(mt9p031_monochrome_fmts);
> +	} else {
> +		mt9p031->fmts = mt9p031_color_fmts;
> +		mt9p031->num_fmts = ARRAY_SIZE(mt9p031_color_fmts);
> +	}
> +	mt9p031->format.code = mt9p031_find_datafmt(mt9p031, 0);
>  
>  	mt9p031->format.width = MT9P031_WINDOW_WIDTH_DEF;
>  	mt9p031->format.height = MT9P031_WINDOW_HEIGHT_DEF;
Stefan Riedmüller Sept. 30, 2020, 10:48 a.m. UTC | #3 
Hi Sakari,

On 25.09.20 22:11, Sakari Ailus wrote:
> Hi Stefan,
> 
> On Fri, Sep 25, 2020 at 09:50:25AM +0200, Stefan Riedmueller wrote:
>> From: Christian Hemp <c.hemp@phytec.de>
>>
>> Aside from 12 bit monochrome or color format the sensor implicitly
>> supports 10 and 8 bit formats as well by simply dropping the
>> corresponding LSBs.
>>
>> Signed-off-by: Christian Hemp <c.hemp@phytec.de>
>> [jlu@pengutronix.de: simplified by dropping v4l2_colorspace handling]
>> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
>> Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
>> ---
>>   drivers/media/i2c/mt9p031.c | 50 +++++++++++++++++++++++++++++--------
>>   1 file changed, 40 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
>> index dc23b9ed510a..0002dd299ffa 100644
>> --- a/drivers/media/i2c/mt9p031.c
>> +++ b/drivers/media/i2c/mt9p031.c
>> @@ -116,6 +116,18 @@ enum mt9p031_model {
>>   	MT9P031_MODEL_MONOCHROME,
>>   };
>>   
>> +static const u32 mt9p031_color_fmts[] = {
>> +	MEDIA_BUS_FMT_SGRBG8_1X8,
>> +	MEDIA_BUS_FMT_SGRBG10_1X10,
>> +	MEDIA_BUS_FMT_SGRBG12_1X12,
>> +};
>> +
>> +static const u32 mt9p031_monochrome_fmts[] = {
>> +	MEDIA_BUS_FMT_Y8_1X8,
>> +	MEDIA_BUS_FMT_Y10_1X10,
>> +	MEDIA_BUS_FMT_Y12_1X12,
>> +};
>> +
>>   struct mt9p031 {
>>   	struct v4l2_subdev subdev;
>>   	struct media_pad pad;
>> @@ -138,6 +150,9 @@ struct mt9p031 {
>>   	struct v4l2_ctrl *blc_auto;
>>   	struct v4l2_ctrl *blc_offset;
>>   
>> +	const u32 *fmts;
>> +	int num_fmts;
> 
> Unsigned int, please.
> 
>> +
>>   	/* Registers cache */
>>   	u16 output_control;
>>   	u16 mode2;
>> @@ -148,6 +163,17 @@ static struct mt9p031 *to_mt9p031(struct v4l2_subdev *sd)
>>   	return container_of(sd, struct mt9p031, subdev);
>>   }
>>   
>> +static const u32 mt9p031_find_datafmt(struct mt9p031 *mt9p031, u32 code)
>> +{
>> +	int i;
> 
> Same here.

I'll fix it in v2.

Thanks,
Stefan

> 
>> +
>> +	for (i = 0; i < mt9p031->num_fmts; i++)
>> +		if (mt9p031->fmts[i] == code)
>> +			return mt9p031->fmts[i];
>> +
>> +	return mt9p031->fmts[mt9p031->num_fmts-1];
>> +}
>> +
>>   static int mt9p031_read(struct i2c_client *client, u8 reg)
>>   {
>>   	return i2c_smbus_read_word_swapped(client, reg);
>> @@ -476,10 +502,11 @@ static int mt9p031_enum_mbus_code(struct v4l2_subdev *subdev,
>>   {
>>   	struct mt9p031 *mt9p031 = to_mt9p031(subdev);
>>   
>> -	if (code->pad || code->index)
>> +	if (code->pad || code->index >= mt9p031->num_fmts)
>>   		return -EINVAL;
>>   
>> -	code->code = mt9p031->format.code;
>> +	code->code = mt9p031->fmts[code->index];
>> +
>>   	return 0;
>>   }
>>   
>> @@ -573,6 +600,8 @@ static int mt9p031_set_format(struct v4l2_subdev *subdev,
>>   	__format->width = __crop->width / hratio;
>>   	__format->height = __crop->height / vratio;
>>   
>> +	__format->code = mt9p031_find_datafmt(mt9p031, format->format.code);
>> +
>>   	format->format = *__format;
>>   
>>   	return 0;
>> @@ -951,10 +980,7 @@ static int mt9p031_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh)
>>   
>>   	format = v4l2_subdev_get_try_format(subdev, fh->pad, 0);
>>   
>> -	if (mt9p031->model == MT9P031_MODEL_MONOCHROME)
>> -		format->code = MEDIA_BUS_FMT_Y12_1X12;
>> -	else
>> -		format->code = MEDIA_BUS_FMT_SGRBG12_1X12;
>> +	format->code = mt9p031_find_datafmt(mt9p031, 0);
>>   
>>   	format->width = MT9P031_WINDOW_WIDTH_DEF;
>>   	format->height = MT9P031_WINDOW_HEIGHT_DEF;
>> @@ -1121,10 +1147,14 @@ static int mt9p031_probe(struct i2c_client *client,
>>   	mt9p031->crop.left = MT9P031_COLUMN_START_DEF;
>>   	mt9p031->crop.top = MT9P031_ROW_START_DEF;
>>   
>> -	if (mt9p031->model == MT9P031_MODEL_MONOCHROME)
>> -		mt9p031->format.code = MEDIA_BUS_FMT_Y12_1X12;
>> -	else
>> -		mt9p031->format.code = MEDIA_BUS_FMT_SGRBG12_1X12;
>> +	if (mt9p031->model == MT9P031_MODEL_MONOCHROME) {
>> +		mt9p031->fmts = mt9p031_monochrome_fmts;
>> +		mt9p031->num_fmts = ARRAY_SIZE(mt9p031_monochrome_fmts);
>> +	} else {
>> +		mt9p031->fmts = mt9p031_color_fmts;
>> +		mt9p031->num_fmts = ARRAY_SIZE(mt9p031_color_fmts);
>> +	}
>> +	mt9p031->format.code = mt9p031_find_datafmt(mt9p031, 0);
>>   
>>   	mt9p031->format.width = MT9P031_WINDOW_WIDTH_DEF;
>>   	mt9p031->format.height = MT9P031_WINDOW_HEIGHT_DEF;
>
diff mbox series

Patch

diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index dc23b9ed510a..0002dd299ffa 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -116,6 +116,18 @@  enum mt9p031_model {
 	MT9P031_MODEL_MONOCHROME,
 };
 
+static const u32 mt9p031_color_fmts[] = {
+	MEDIA_BUS_FMT_SGRBG8_1X8,
+	MEDIA_BUS_FMT_SGRBG10_1X10,
+	MEDIA_BUS_FMT_SGRBG12_1X12,
+};
+
+static const u32 mt9p031_monochrome_fmts[] = {
+	MEDIA_BUS_FMT_Y8_1X8,
+	MEDIA_BUS_FMT_Y10_1X10,
+	MEDIA_BUS_FMT_Y12_1X12,
+};
+
 struct mt9p031 {
 	struct v4l2_subdev subdev;
 	struct media_pad pad;
@@ -138,6 +150,9 @@  struct mt9p031 {
 	struct v4l2_ctrl *blc_auto;
 	struct v4l2_ctrl *blc_offset;
 
+	const u32 *fmts;
+	int num_fmts;
+
 	/* Registers cache */
 	u16 output_control;
 	u16 mode2;
@@ -148,6 +163,17 @@  static struct mt9p031 *to_mt9p031(struct v4l2_subdev *sd)
 	return container_of(sd, struct mt9p031, subdev);
 }
 
+static const u32 mt9p031_find_datafmt(struct mt9p031 *mt9p031, u32 code)
+{
+	int i;
+
+	for (i = 0; i < mt9p031->num_fmts; i++)
+		if (mt9p031->fmts[i] == code)
+			return mt9p031->fmts[i];
+
+	return mt9p031->fmts[mt9p031->num_fmts-1];
+}
+
 static int mt9p031_read(struct i2c_client *client, u8 reg)
 {
 	return i2c_smbus_read_word_swapped(client, reg);
@@ -476,10 +502,11 @@  static int mt9p031_enum_mbus_code(struct v4l2_subdev *subdev,
 {
 	struct mt9p031 *mt9p031 = to_mt9p031(subdev);
 
-	if (code->pad || code->index)
+	if (code->pad || code->index >= mt9p031->num_fmts)
 		return -EINVAL;
 
-	code->code = mt9p031->format.code;
+	code->code = mt9p031->fmts[code->index];
+
 	return 0;
 }
 
@@ -573,6 +600,8 @@  static int mt9p031_set_format(struct v4l2_subdev *subdev,
 	__format->width = __crop->width / hratio;
 	__format->height = __crop->height / vratio;
 
+	__format->code = mt9p031_find_datafmt(mt9p031, format->format.code);
+
 	format->format = *__format;
 
 	return 0;
@@ -951,10 +980,7 @@  static int mt9p031_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh)
 
 	format = v4l2_subdev_get_try_format(subdev, fh->pad, 0);
 
-	if (mt9p031->model == MT9P031_MODEL_MONOCHROME)
-		format->code = MEDIA_BUS_FMT_Y12_1X12;
-	else
-		format->code = MEDIA_BUS_FMT_SGRBG12_1X12;
+	format->code = mt9p031_find_datafmt(mt9p031, 0);
 
 	format->width = MT9P031_WINDOW_WIDTH_DEF;
 	format->height = MT9P031_WINDOW_HEIGHT_DEF;
@@ -1121,10 +1147,14 @@  static int mt9p031_probe(struct i2c_client *client,
 	mt9p031->crop.left = MT9P031_COLUMN_START_DEF;
 	mt9p031->crop.top = MT9P031_ROW_START_DEF;
 
-	if (mt9p031->model == MT9P031_MODEL_MONOCHROME)
-		mt9p031->format.code = MEDIA_BUS_FMT_Y12_1X12;
-	else
-		mt9p031->format.code = MEDIA_BUS_FMT_SGRBG12_1X12;
+	if (mt9p031->model == MT9P031_MODEL_MONOCHROME) {
+		mt9p031->fmts = mt9p031_monochrome_fmts;
+		mt9p031->num_fmts = ARRAY_SIZE(mt9p031_monochrome_fmts);
+	} else {
+		mt9p031->fmts = mt9p031_color_fmts;
+		mt9p031->num_fmts = ARRAY_SIZE(mt9p031_color_fmts);
+	}
+	mt9p031->format.code = mt9p031_find_datafmt(mt9p031, 0);
 
 	mt9p031->format.width = MT9P031_WINDOW_WIDTH_DEF;
 	mt9p031->format.height = MT9P031_WINDOW_HEIGHT_DEF;