On Tue, Jun 14, 2022 at 04:02:16PM +0200, Jacopo Mondi wrote:
> Hi Laurent,
> one more comment, mostly for the record if anyone else will
> encounter the same problem I found
>
> On Fri, May 13, 2022 at 12:27:16PM +0200, Jacopo Mondi wrote:
> > Hi Laurent,
>
> [snip]
>
> > > +#define MT9M114_CAM_SENSOR_CONTROL_Y_READ_OUT_AVERAGE (2 << 8)
>
> The version of the datasheet I have documents this bit as "RESERVED"
Indeed, so does mine. I'll drop it.
> [snip]
>
> > > +
> > > +static int mt9m114_configure(struct mt9m114 *sensor)
> > > +{
> > > + u32 value;
> > > + int ret = 0;
> > > +
> > > + /*
> > > + * Pixel array crop and binning. The CAM_SENSOR_CFG_CPIPE_LAST_ROW
> > > + * register isn't clearly documented, but is always set to the number
> > > + * of output rows minus 4 in all example sensor modes.
> > > + */
> > > + mt9m114_write(sensor, MT9M114_CAM_SENSOR_CFG_X_ADDR_START,
> > > + sensor->pa.crop.left, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_SENSOR_CFG_Y_ADDR_START,
> > > + sensor->pa.crop.top, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_SENSOR_CFG_X_ADDR_END,
> > > + sensor->pa.crop.width + sensor->pa.crop.left - 1, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_SENSOR_CFG_Y_ADDR_END,
> > > + sensor->pa.crop.height + sensor->pa.crop.top - 1, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_SENSOR_CFG_CPIPE_LAST_ROW,
> > > + sensor->pa.format.height - 4 - 1, &ret);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + ret = mt9m114_read(sensor, MT9M114_CAM_SENSOR_CONTROL_READ_MODE,
> > > + &value);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + value &= ~(MT9M114_CAM_SENSOR_CONTROL_X_READ_OUT_MASK |
> > > + MT9M114_CAM_SENSOR_CONTROL_Y_READ_OUT_MASK);
> > > +
> > > + if (sensor->pa.crop.width != sensor->pa.format.width)
> > > + value |= MT9M114_CAM_SENSOR_CONTROL_X_READ_OUT_SUMMING;
> > > + if (sensor->pa.crop.height != sensor->pa.format.height)
> > > + value |= MT9M114_CAM_SENSOR_CONTROL_Y_READ_OUT_SUMMING;
>
> While applying 2x2 subsampling, I found SUMMING to mangle the color
> output possibly because the gains should be adjusted accordingly to
> the fact the SUMMING:
> "will add the charge or voltage values of the neighboring pixels
> together"
>
> I have found the combination that works better for me out of the box
> to be:
>
> if (sensor->pa.crop.width != sensor->pa.format.width)
> value |= MT9M114_CAM_SENSOR_CONTROL_X_READ_OUT_AVERAGE;
> if (sensor->pa.crop.height != sensor->pa.format.height)
> value |= MT9M114_CAM_SENSOR_CONTROL_Y_READ_OUT_SKIPPING;
>
> Have you tested 2x2 binning with CSI-2 ?
I don't think I have. I'll give it a try.
> > > +
> > > + mt9m114_write(sensor, MT9M114_CAM_SENSOR_CONTROL_READ_MODE, value,
> > > + &ret);
> > > +
> > > + /*
> > > + * Color pipeline (IFP) cropping and scaling. Subtract 4 from the left
> > > + * and top coordinates to compensate for the lines and columns removed
> > > + * by demosaicing that are taken into account in the crop rectangle but
> > > + * not in the hardware.
> > > + */
> > > + mt9m114_write(sensor, MT9M114_CAM_CROP_WINDOW_XOFFSET,
> > > + sensor->ifp.crop.left - 4, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_CROP_WINDOW_YOFFSET,
> > > + sensor->ifp.crop.top - 4, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_CROP_WINDOW_WIDTH,
> > > + sensor->ifp.crop.width, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_CROP_WINDOW_HEIGHT,
> > > + sensor->ifp.crop.height, &ret);
> > > +
> > > + mt9m114_write(sensor, MT9M114_CAM_OUTPUT_WIDTH,
> > > + sensor->ifp.compose.width, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_OUTPUT_HEIGHT,
> > > + sensor->ifp.compose.height, &ret);
> > > +
> > > + /* AWB and AE windows, use the full frame. */
> > > + mt9m114_write(sensor, MT9M114_CAM_STAT_AWB_CLIP_WINDOW_XSTART, 0, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_STAT_AWB_CLIP_WINDOW_YSTART, 0, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_STAT_AWB_CLIP_WINDOW_XEND,
> > > + sensor->ifp.compose.width - 1, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_STAT_AWB_CLIP_WINDOW_YEND,
> > > + sensor->ifp.compose.height - 1, &ret);
> > > +
> > > + mt9m114_write(sensor, MT9M114_CAM_STAT_AE_INITIAL_WINDOW_XSTART,
> > > + 0, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_STAT_AE_INITIAL_WINDOW_YSTART,
> > > + 0, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_STAT_AE_INITIAL_WINDOW_XEND,
> > > + sensor->ifp.compose.width / 5 - 1, &ret);
> > > + mt9m114_write(sensor, MT9M114_CAM_STAT_AE_INITIAL_WINDOW_YEND,
> > > + sensor->ifp.compose.height / 5 - 1, &ret);
> > > +
> > > + mt9m114_write(sensor, MT9M114_CAM_CROP_CROPMODE,
> > > + MT9M114_CAM_CROP_MODE_AWB_AUTO_CROP_EN |
> > > + MT9M114_CAM_CROP_MODE_AE_AUTO_CROP_EN, &ret);
> > > +
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + /* Set the media bus code. */
> > > + ret = mt9m114_read(sensor, MT9M114_CAM_OUTPUT_FORMAT, &value);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + value &= ~(MT9M114_CAM_OUTPUT_FORMAT_RGB_FORMAT_MASK |
> > > + MT9M114_CAM_OUTPUT_FORMAT_BAYER_FORMAT_MASK |
> > > + MT9M114_CAM_OUTPUT_FORMAT_FORMAT_MASK |
> > > + MT9M114_CAM_OUTPUT_FORMAT_SWAP_BYTES |
> > > + MT9M114_CAM_OUTPUT_FORMAT_SWAP_RED_BLUE);
> > > + value |= sensor->ifp.info->output_format;
> > > +
> > > + mt9m114_write(sensor, MT9M114_CAM_OUTPUT_FORMAT, value, &ret);
> > > + return ret;
> > > +}
[snip]
--
Regards,
Laurent Pinchart
