On Fri, 27 Nov 2009, Hans Verkuil wrote:
> Hi Guennadi,
>
> > Convert soc-camera core and all soc-camera drivers to the new mediabus
> > API. This also takes soc-camera client drivers one step closer to also be
> > usable with generic v4l2-subdev host drivers.
>
> Just a quick question:
>
> > @@ -323,28 +309,39 @@ static int mt9m001_s_fmt(struct v4l2_subdev *sd,
> > struct v4l2_format *f)
> > /* No support for scaling so far, just crop. TODO: use skipping */
> > ret = mt9m001_s_crop(sd, &a);
> > if (!ret) {
> > - pix->width = mt9m001->rect.width;
> > - pix->height = mt9m001->rect.height;
> > - mt9m001->fourcc = pix->pixelformat;
> > + mf->width = mt9m001->rect.width;
> > + mf->height = mt9m001->rect.height;
> > + mt9m001->fmt = soc_mbus_find_datafmt(mf->code,
> > + mt9m001->fmts, mt9m001->num_fmts);
> > + mf->colorspace = mt9m001->fmt->colorspace;
> > }
> >
> > return ret;
> > }
> >
> > -static int mt9m001_try_fmt(struct v4l2_subdev *sd, struct v4l2_format *f)
> > +static int mt9m001_try_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_mbus_framefmt *mf)
> > {
> > struct i2c_client *client = sd->priv;
> > struct mt9m001 *mt9m001 = to_mt9m001(client);
> > - struct v4l2_pix_format *pix = &f->fmt.pix;
> > + const struct soc_mbus_datafmt *fmt;
> >
> > - v4l_bound_align_image(&pix->width, MT9M001_MIN_WIDTH,
> > + v4l_bound_align_image(&mf->width, MT9M001_MIN_WIDTH,
> > MT9M001_MAX_WIDTH, 1,
> > - &pix->height, MT9M001_MIN_HEIGHT + mt9m001->y_skip_top,
> > + &mf->height, MT9M001_MIN_HEIGHT + mt9m001->y_skip_top,
> > MT9M001_MAX_HEIGHT + mt9m001->y_skip_top, 0, 0);
> >
> > - if (pix->pixelformat == V4L2_PIX_FMT_SBGGR8 ||
> > - pix->pixelformat == V4L2_PIX_FMT_SBGGR16)
> > - pix->height = ALIGN(pix->height - 1, 2);
> > + if (mt9m001->fmts == mt9m001_colour_fmts)
> > + mf->height = ALIGN(mf->height - 1, 2);
> > +
> > + fmt = soc_mbus_find_datafmt(mf->code, mt9m001->fmts,
> > + mt9m001->num_fmts);
> > + if (!fmt) {
> > + fmt = mt9m001->fmt;
> > + mf->code = fmt->code;
> > + }
> > +
> > + mf->colorspace = fmt->colorspace;
> >
> > return 0;
> > }
>
> Why do the sensor drivers use soc_mbus_find_datafmt? They only seem to be
> interested in the colorspace field, but I don't see the reason for that.
> Most if not all sensors have a fixed colorspace depending on the
> pixelcode, so they can just ignore the colorspace that the caller
> requested and replace it with their own.
Right, that's exactly what's done here. mt9m001 and mt9v022 drivers
support different formats, depending on the exact detected or specified by
the user model. That's why they have to search for the requested format in
supported list. and then - yes, they just put the found format into user
request:
> > + mf->colorspace = fmt->colorspace;
> I didn't have time for a full review, so I might have missed something.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
