Message ID | 20200925075029.32181-1-s.riedmueller@phytec.de |
---|---|
State | New |
Headers | show |
Series | [1/5] media: mt9p031: Add support for 8 bit and 10 bit formats | expand |
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
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; -- Kind regards, Sakari Ailus
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 --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;