<div dir="ltr"><div>Hi Laurent and Kieran,</div><div>Sent a v2 patch with requested changes.</div><div><br></div><div>Regards,</div><div><i><b>Vedant Paranjape</b></i><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 21, 2021 at 7:18 PM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Jun 21, 2021 at 02:44:56PM +0100, Kieran Bingham wrote:<br>
> Hi Vedant,<br>
> <br>
> On 21/06/2021 14:34, Vedant Paranjape wrote:<br>
> > Brief specifications available at<br>
> > <a href="https://cdn.sparkfun.com/datasheets/Dev/RaspberryPi/ov5647_full.pdf" rel="noreferrer" target="_blank">https://cdn.sparkfun.com/datasheets/Dev/RaspberryPi/ov5647_full.pdf</a><br>
> > <br>
> <br>
> I presume this comes from the features on page 5 stating<br>
> <br>
> """<br>
> 1.4 μm x 1.4 μm pixel with OmniBSI technology for<br>
> high performance (high sensitivity, low crosstalk, low<br>
> noise<br>
> """<br>
> <br>
> It might be nice to reference that specifically rather than the whole<br>
> document, so readers can quickly and easily find the relevant source.<br>
> <br>
> <br>
> > Signed-off-by: Vedant Paranjape <<a href="mailto:vedantparanjape160201@gmail.com" target="_blank">vedantparanjape160201@gmail.com</a>><br>
> > ---<br>
> >  src/libcamera/camera_sensor_properties.cpp | 5 +++++<br>
> >  1 file changed, 5 insertions(+)<br>
> > <br>
> > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp<br>
> > index f660743a..43030e8b 100644<br>
> > --- a/src/libcamera/camera_sensor_properties.cpp<br>
> > +++ b/src/libcamera/camera_sensor_properties.cpp<br>
> > @@ -81,6 +81,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen<br>
> >                             { 1, controls::draft::TestPatternModeColorBars },<br>
> >                     },<br>
> >             } },<br>
> > +           { "ov5647", {<br>
> > +                   .unitCellSize = { 1400, 1400 },<br>
> > +                   /* \todo fill test pattern modes for ov5647. */<br>
> <br>
> It seems this todo is quite involved, as it will require updating the<br>
> kernel driver to actually have test patterns :-S<br>
> <br>
> I can't see any available at:<br>
> <br>
> <a href="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/i2c/ov5647.c" rel="noreferrer" target="_blank">https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/i2c/ov5647.c</a><br>
> <br>
> So the todo is likely sufficient, but I'd be more explicit saying there<br>
> are no current supported test modes in the kernel.<br>
<br>
I'd say there's no \todo needed in that case, if the driver doesn't<br>
support test patterns, the implementation here is enough.<br>
<br>
> Other than those comments,<br>
> <br>
> Reviewed-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>><br>
> <br>
> > +                   .testPatternModes = {},<br>
> > +           } },<br>
> >             { "ov5693", {<br>
> >                     .unitCellSize = { 1400, 1400 },<br>
> >                     .testPatternModes = {<br>
> > <br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div>