Re: [PATCH v2 00/12] media: ov5640: Misc cleanup and improvements

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Mon, May 07, 2018 at 06:00:55PM -0700, Sam Bobrowicz wrote:
> > Hi,
> >
> > Here is a "small" series that mostly cleans up the ov5640 driver code,
> > slowly getting rid of the big data array for more understandable code
> > (hopefully).
> >
> > The biggest addition would be the clock rate computation at runtime,
> > instead of relying on those arrays to setup the clock tree
> > properly. As a side effect, it fixes the framerate that was off by
> > around 10% on the smaller resolutions, and we now support 60fps.
> >
> > This also introduces a bunch of new features.
> >
> > Let me know what you think,
> > Maxime
> >
> > Changes from v1:
> >   - Integrated Hugues' suggestions to fix v4l2-compliance
> >   - Fixed the bus width with JPEG
> >   - Dropped the clock rate calculation loops for something simpler as
> >     suggested by Sakari
> >   - Cache the exposure value instead of using the control value
> >   - Rebased on top of 4.17
> >
> > Maxime Ripard (10):
> >   media: ov5640: Don't force the auto exposure state at start time
> >   media: ov5640: Init properly the SCLK dividers
> >   media: ov5640: Change horizontal and vertical resolutions name
> >   media: ov5640: Add horizontal and vertical totals
> >   media: ov5640: Program the visible resolution
> >   media: ov5640: Adjust the clock based on the expected rate
> >   media: ov5640: Compute the clock rate at runtime
> >   media: ov5640: Enhance FPS handling
> >   media: ov5640: Add 60 fps support
> >   media: ov5640: Remove duplicate auto-exposure setup
> >
> > Mylène Josserand (2):
> >   media: ov5640: Add auto-focus feature
> >   media: ov5640: Add light frequency control
> >
> >  drivers/media/i2c/ov5640.c | 752 +++++++++++++++++++++----------------
> >  1 file changed, 422 insertions(+), 330 deletions(-)
> >
> > --
> > 2.17.0
> >
> 
> As discussed, MIPI required some additional work. Please see the
> patches here which add support for MIPI:
> https://www.dropbox.com/s/73epty7808yzq1t/ov5640_mipi_fixes.zip?dl=0
> 
> The first 3 patches are fixes I believe should be made to earlier
> patches prior to submitting v2 of this series. The remaining 4 patches
> should probably just be added onto the end of this series as-is (or
> with feedback incorporated if needed).
> 
> I will note that this is still not working correctly on my system for
> any resolution that requires a 672 Mbps mipi rate. This includes
> 1080p@30hz, full@15hz, and 720p@60hz. My CSI2 receiver is reporting
> CRC errors though, so this could be an integrity issue on my module.
> I'm curious to hear if others have success at these resolutions.
> 
> Please try this out on other MIPI and DVP platforms with as many
> different resolutions as possible and let me know if it works.

I've took some of your changes to remain as feature stable as possible
in my series. Some other changes (like the PLL2 setup), while totally
welcome, should be in a separate, subsequent series.

DVP works as expected, after looking at the feedback from Loic (and
the clock tree especially), some of the comments you made (like the
bit divider being meaningless for DVP) are not totally accurate, so
I've tried to make the best blend of all the feedback you gave. It
still works properly with DVP, but I still don't have a MIPI camera to
test, so I'm not sure this works, even though I should have the same
setup than the one you reported.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux