linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jacopo Mondi <jacopo@jmondi.org>
To: Steve Longerbeam <slongerbeam@gmail.com>
Cc: Jacopo Mondi <jacopo@jmondi.org>,
	laurent.pinchart@ideasonboard.com, sakari.ailus@iki.fi,
	hverkuil-cisco@xs4all.nl, mirela.rabulea@nxp.com,
	xavier.roumegue@oss.nxp.com, tomi.valkeinen@ideasonboard.com,
	hugues.fruchet@st.com, prabhakar.mahadev-lad.rj@bp.renesas.com,
	aford173@gmail.com, festevam@gmail.com,
	Eugen.Hristev@microchip.com, jbrunet@baylibre.com,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-media@vger.kernel.org
Subject: [PATCH 00/17] media: ov5640: Rework the clock tree programming for MIPI
Date: Mon, 31 Jan 2022 15:32:24 +0100	[thread overview]
Message-ID: <20220131143245.128089-1-jacopo@jmondi.org> (raw)

Heya
   this series changes the way the ov5640 clock tree is programmed and moves the
driver to control frame rate by changing the vertical blanking when used in MIPI
mode.

A bit of background:
The OV5640 is -everywhere-. After a long long time since it has been put on the
market it still is one of the most common sensors found in EVKs from several
several vendors. Touching it is scary, it will likely make a lot of people
complain as there are a lot of active users out there.

The current driver implementation as it is barely works at least here. Testing
in UYVY mode I can only capture in 1080p from the few tests I've run. Other
resolutions do not work or have a completely off frame rate.

The current implementation operates in the following way:

- it allows to control frame rate through the s_frame_interval operation
- fixes a set of frame rates supported by each mode (15, 30 or 60FPS)
- tries to adjust the pixel rate to comply with the configured frame rate
  and capture resolution

The idea is then to fix the frame size (visible and blanking) and compute
the pixel rate dynamically and adjust the clock tree accordingly.

As it was hard to obtain correct operations in that way, and that frame rates
were fixed to only a few of the achievable ones I have decided to flip the table
and move to an opposite model:

- Set a fixed pixel clock per each supported mode according to the indications
  reported in the chip manual
- Program the clock tree according to the above pixel rate
- Adjust the frame rate by controlling the frame vertical blankings

With this new configuration I can capture in all the sensor supported modes in
YUYV, RGB565, RGB888 and SBGGR modes with the following limitations:

- 16bpp modes works in all resolutions
- RAW 8bpp mode works for high resolutions from 1280x720 up
- RGB888 mode works for low resolutions up to 1280x720

The blankings for each mode have been adjusted to work by default at 30FPS
in YUYV/RGB565 mode, and can be adjusted to precisely obtain the desired frame
rate. Unfortunately the minimum blanking values are not documented so shrinking
them too much might hinder capture operations.

I tested the series on both i.MX6Q and i.MX8MP and I get the same results on
both platforms.

There remain two issues:

- RGB888 colors are off, both on imx6 and imx8. I tested all the RGB color
  permutations the chip can perform and I am not able to get the color
  ordering "right". It might be due to how I perform decoding or to a bug in
  the sensor manual.

- I was not able to test JPEG, which I would like to support next. If anyone
  has been able to test JPEG capture in the past and would like to share it's
  setup I would be very glad.

Are we there yet ? No, I don't think so. My main concern at the moment is that
this sensor supports -a lot- of different formats, and the pixel rates I
have fixed are compromises to a have all formats working. In example, for
640x480 I could have gone for 96MHz instead of 48. This would allow to
push the sensor up to 60FPS per second in YUYV/RGB565 mode, but would break
capturing in RGB888 mode, for which 96Mhz @24bpp is too high as a pixel rate.
-Ideally-  userspace should be able to select the pixel rate which is
ideal for the mode currently in use. The PIXEL_RATE control is however marked
as READ_ONLY by the framework, and the only alternative I'm left with is to
create per-mode pixel rates but this would make the driver a lot mode
complicated... Ideas ?

The series changes the clock tree programming for MIPI only. I have not tried
to maintain the current behaviour for parallel mode which, as far as I'm aware,
works correctly. As I cannot test parallel, I would really appreciate a
confirmation I have not broken parallel mode :)

Ah, v4l2-compliance seems happy too:
Total for device /dev/v4l-subdev3: 43, Succeeded: 43, Failed: 0, Warnings: 0

Thanks
   j

Jacopo Mondi (21):
  media: ov5640: Add pixel rate to modes
  media: ov5604: Re-arrange modes definition
  media: ov5640: Add is_mipi() function
  media: ov5640: Associate bpp with formats
  media: ov5640: Update pixel_rate and link_freq
  media: ov5640: Rework CSI-2 clock tree
  media: ov5640: Rework timings programming
  media: ov5640: Re-sort per-mode register tables
  media: ov5640: Remove ov5640_mode_init_data
  media: ov5640: Add HBLANK control
  media: ov5640: Add VBLANK control
  media: ov5640: Fix durations to comply with FPS
  media: ov5640: Initialize try format
  media: ov5640: Implement get_selection
  media: ov5640: Limit format to FPS in DVP mode only
  media: ov5640: Disable s_frame_interval in MIPI mode
  media: ov5640: Register device properties
  media: ov5640: Add RGB565_1X16 format
  media: ov5640: Add RGB888/BGR888 formats
  media: ov5640: Restrict sizes to mbus code
  media: ov5640: Adjust format to bpp in s_fmt

 drivers/media/i2c/ov5640.c | 1063 ++++++++++++++++++++++++++----------
 1 file changed, 778 insertions(+), 285 deletions(-)

--
2.35.0

             reply	other threads:[~2022-01-31 14:32 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-31 14:32 Jacopo Mondi [this message]
2022-01-31 14:32 ` [PATCH 01/21] media: ov5640: Add pixel rate to modes Jacopo Mondi
2022-02-01 14:52   ` Laurent Pinchart
2022-01-31 14:32 ` [PATCH 02/21] media: ov5604: Re-arrange modes definition Jacopo Mondi
2022-02-01 14:20   ` Laurent Pinchart
2022-01-31 14:32 ` [PATCH 03/21] media: ov5640: Add is_mipi() function Jacopo Mondi
2022-02-01 14:25   ` Laurent Pinchart
2022-01-31 14:32 ` [PATCH 04/21] media: ov5640: Associate bpp with formats Jacopo Mondi
2022-02-01 14:27   ` Laurent Pinchart
2022-01-31 14:32 ` [PATCH 05/21] media: ov5640: Update pixel_rate and link_freq Jacopo Mondi
2022-02-01 16:52   ` Laurent Pinchart
2022-01-31 14:32 ` [PATCH 06/21] media: ov5640: Rework CSI-2 clock tree Jacopo Mondi
2022-02-01 17:27   ` Laurent Pinchart
2022-02-07 14:07     ` Jacopo Mondi
2022-01-31 14:32 ` [PATCH 07/21] media: ov5640: Rework timings programming Jacopo Mondi
2022-02-01 19:03   ` Laurent Pinchart
2022-02-07 14:37     ` Jacopo Mondi
2022-02-07 16:39       ` Laurent Pinchart
2022-01-31 14:32 ` [PATCH 08/21] media: ov5640: Re-sort per-mode register tables Jacopo Mondi
2022-02-01 19:05   ` Laurent Pinchart
2022-02-07 14:42     ` Jacopo Mondi
2022-01-31 14:32 ` [PATCH 09/21] media: ov5640: Remove ov5640_mode_init_data Jacopo Mondi
2022-02-01 19:07   ` Laurent Pinchart
2022-02-07 14:45     ` Jacopo Mondi
2022-01-31 14:32 ` [PATCH 10/21] media: ov5640: Add HBLANK control Jacopo Mondi
2022-02-02 21:20   ` Laurent Pinchart
2022-01-31 14:32 ` [PATCH 11/21] media: ov5640: Add VBLANK control Jacopo Mondi
2022-02-02 21:35   ` Laurent Pinchart
2022-02-07 15:09     ` Jacopo Mondi
2022-02-07 17:22       ` Laurent Pinchart
2022-02-03  7:58   ` Xavier Roumegue (OSS)
2022-02-07 15:12     ` Jacopo Mondi
2022-01-31 14:44 ` [PATCH 12/21] media: ov5640: Fix durations to comply with FPS Jacopo Mondi
2022-01-31 14:44   ` [PATCH 13/21] media: ov5640: Initialize try format Jacopo Mondi
2022-02-02 21:51     ` Laurent Pinchart
2022-01-31 14:44   ` [PATCH 14/21] media: ov5640: Implement get_selection Jacopo Mondi
2022-02-02 22:29     ` Laurent Pinchart
2022-02-07 15:47       ` Jacopo Mondi
2022-02-07 17:53         ` Laurent Pinchart
2022-02-08 14:18           ` Jacopo Mondi
2022-02-08 14:41             ` Laurent Pinchart
2022-01-31 14:44   ` [PATCH 15/21] media: ov5640: Limit format to FPS in DVP mode only Jacopo Mondi
2022-02-02 22:38     ` Laurent Pinchart
2022-02-07 15:49       ` Jacopo Mondi
2022-01-31 14:44   ` [PATCH 16/21] media: ov5640: Disable s_frame_interval in MIPI mode Jacopo Mondi
2022-02-02 22:40     ` Laurent Pinchart
2022-02-02 21:48   ` [PATCH 12/21] media: ov5640: Fix durations to comply with FPS Laurent Pinchart
2022-02-07 15:58     ` Jacopo Mondi
2022-01-31 14:45 ` [PATCH 17/21] media: ov5640: Register device properties Jacopo Mondi
2022-01-31 14:45   ` [PATCH 18/21] media: ov5640: Add RGB565_1X16 format Jacopo Mondi
2022-02-02 22:48     ` Laurent Pinchart
2022-01-31 14:45   ` [PATCH 19/21] media: ov5640: Add RGB888/BGR888 formats Jacopo Mondi
2022-02-02 22:49     ` Laurent Pinchart
2022-02-02 22:44   ` [PATCH 17/21] media: ov5640: Register device properties Laurent Pinchart
2022-01-31 14:45 ` [PATCH 20/21] media: ov5640: Restrict sizes to mbus code Jacopo Mondi
2022-01-31 14:45   ` [PATCH 21/21] media: ov5640: Adjust format to bpp in s_fmt Jacopo Mondi
2022-02-02 23:03     ` Laurent Pinchart
2022-02-07 16:07       ` Jacopo Mondi
2022-02-07 16:46         ` Laurent Pinchart
2022-02-02 22:57   ` [PATCH 20/21] media: ov5640: Restrict sizes to mbus code Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220131143245.128089-1-jacopo@jmondi.org \
    --to=jacopo@jmondi.org \
    --cc=Eugen.Hristev@microchip.com \
    --cc=aford173@gmail.com \
    --cc=festevam@gmail.com \
    --cc=hugues.fruchet@st.com \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=jbrunet@baylibre.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=mirela.rabulea@nxp.com \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=sakari.ailus@iki.fi \
    --cc=slongerbeam@gmail.com \
    --cc=tomi.valkeinen@ideasonboard.com \
    --cc=xavier.roumegue@oss.nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).