lkml.org 
[lkml]   [2020]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 1/5] media: mt9p031: Add support for 8 bit and 10 bit formats
    Hi Stefan,

    Thank you for the patch.

    On Wed, Sep 30, 2020 at 12:51:29PM +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.

    That's not how it should work though. If you set the format on
    MEDIA_BUS_FMT_SGRBG8_1X8 through the pipeline for instance, you will end
    up capturing the 8 LSB, not the 8 MSB.

    What's your use case for this ?

    > 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>
    > ---
    > Changes in v2:
    > - Use unsigned int for num_fmts and loop variable in find_datafmt
    > - Remove superfluous const qualifier from find_datafmt
    > ---
    > 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..2e6671ef877c 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;
    > + unsigned 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 u32 mt9p031_find_datafmt(struct mt9p031 *mt9p031, u32 code)
    > +{
    > + unsigned 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;

    --
    Regards,

    Laurent Pinchart

    \
     
     \ /
      Last update: 2020-09-30 13:43    [W:3.615 / U:0.528 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site