mbox series

[v6,00/23] imx258 improvement series

Message ID 20240602201345.328737-1-git@luigi311.com (mailing list archive)
Headers show
Series imx258 improvement series | expand

Message

Luis Garcia June 2, 2024, 8:13 p.m. UTC
From: Luis Garcia <git@luigi311.com>

v6:
  - Drop powerdown-gpio patches
    - per Sakari Ailus request as it is not part or 
      not used by the sensor
    - I tested without it and PPP still works
    - Dave mentioned its not part of the datasheet 
      for the imx258
    - Ondrej Jirman also tested it and it doesnt seem
      to be needed


v5:
  - Changed ownership of a few patches to Ondrej Jirman
  - Implement feedback from Tommy Merciai
    - media: i2c: imx258: Add support for reset gpio
    - media: i2c: imx258: Use v4l2_link_freq_to_bitmap helper
    - media: i2c: imx258: Convert to new CCI register access helpers

v4:
  - Swapped out the use macro patch for a patch that uses the new
    CCI register access helpers per Sakari Ailus
  - Fix order of reset and powerdown gpio before disable regulators
  - Fix formating of all patches
  
  - Implemented feedback from Pavel Machek
    - media: i2c: imx258: Follow normal V4L2 behaviours
    - media: i2c: imx258: Allow configuration of clock
  - Implemented feedback from Sakari Ailus
    - media: i2c: imx258: Add support for powerdown gpio


v3 Luis Garcia

- Add Use v4l2_link_freq_to_bitmap helper patch per Sakari Ailus
- Separate dt-bindings for powerdown per Rob Herring
- Fix dt-bindings for imx258.yaml
- Fix sign offs per Dang Huynh 
- Fix versioning per Conor Dooley and Kieran Bingham
- Use scripts to validate and fix patches
- Implemented feedback from Sakari Ailus
  - media: i2c: imx258: Add support for 24MHz clock
  - media: i2c: imx258: Add support for running on 2 CSI data lanes

- Implemented feedback from Rob Herring
  - dt-bindings: media: imx258: Add alternate compatible strings



v2 Luis Garcia

- Add use macros patch 
- Add support for powerdown gpio patch
- Add support for reset gpio patch
- Dropped Add support for long exposure modes patch
- Implemented feedback from Jacopo Mondi
  - media: i2c: imx258: Add regulator control
  - media: i2c: imx258: Add support for 24MHz clock
  - media: i2c: imx258: Add support for running on 2 CSI data lanes
  - media: i2c: imx258: Add get_selection for pixel array information
  - media: i2c: imx258: Issue reset before starting streaming
  - media: i2c: imx258: Set pixel_rate range to the same as the value
  - dt-bindings: media: imx258: Add alternate compatible strings
  - media: i2c: imx258: Change register settings for variants of the sensor
  - media: i2c: imx258: Make HFLIP and VFLIP controls writable

This adds a few more patches and drops one. The long exposure mode patch
was dropped due to the bug that Jacopo found. The powerdown and reset gpio
patches were added as that fixes support for the Pinephone Pro, without
them the sensor doesn't initialize correctly.

Tested on a Pinephone Pro by forcing 24 mhz clock. The two lower 
resolutions had some artifacts but that is expected as more changes are 
required to fix them for the Pinephone Pro specifically, kept all 
registers the same as Dave's original patch since that works on dedicated
imx258 hardware and the artifacts are PPP specific so it shouldn't 
be a regression.



v1 Dave Stevenson

This is a set of patches for imx258 that allow it to work with alternate
clock frequencies, over either 2 or 4 lanes, and generally adding
flexibility to the driver.

Tested with an IMX258 module from Soho Enterprises that has a 24MHz
oscillator. Both 2 and 4 lane configurations work with correct link
frequencies and pixel rates.

Jacopo has tested on a PinePhone Pro which has an ~19.2MHz clock fed from
the SoC, He confirms that the two lower resolution modes work, but not the
full res mode. Comparing to the BSP it looks like they have some weird
clock configuration in the 4208x3120 mode (nominally 1224Mb/s/lane instead
of 1267). As it has never previously worked directly with the mainline
driver this isn't a regression but may indicate that there is a need for
support of additional link frequencies in the future.

The last patch that makes HFLIP and VFLIP configurable may be contentious
as I've retained the default configuration of inverted from the original
driver. I know this was discussed recently, but I can't recall the final
outcome.

I am relying on someone from Intel testing this out, as correcting the
cropping and supporting flips has changed the Bayer order. Seeing as this
is all above board in V4L2 terms I really hope that the layers above it
behave themselves.

Dave Stevenson (20):
  media: i2c: imx258: Remove unused defines
  media: i2c: imx258: Make image geometry meet sensor requirements
  media: i2c: imx258: Disable digital cropping on binned modes
  media: i2c: imx258: Remove redundant I2C writes.
  media: i2c: imx258: Add regulator control
  media: i2c: imx258: Make V4L2_CID_VBLANK configurable.
  media: i2c: imx258: Split out common registers from the mode based
    ones
  media: i2c: imx258: Add support for 24MHz clock
  media: i2c: imx258: Add support for running on 2 CSI data lanes
  media: i2c: imx258: Follow normal V4L2 behaviours for clipping
    exposure
  media: i2c: imx258: Add get_selection for pixel array information
  media: i2c: imx258: Allow configuration of clock lane behaviour
  media: i2c: imx258: Correct max FRM_LENGTH_LINES value
  media: i2c: imx258: Issue reset before starting streaming
  media: i2c: imx258: Set pixel_rate range to the same as the value
  media: i2c: imx258: Support faster pixel rate on binned modes
  dt-bindings: media: imx258: Rename to include vendor prefix
  dt-bindings: media: imx258: Add alternate compatible strings
  media: i2c: imx258: Change register settings for variants of the
    sensor
  media: i2c: imx258: Make HFLIP and VFLIP controls writable

Luis Garcia (2):
  media: i2c: imx258: Use v4l2_link_freq_to_bitmap helper
  media: i2c: imx258: Convert to new CCI register access helpers

Ondrej Jirman (1):
  media: i2c: imx258: Add support for reset gpio

 .../i2c/{imx258.yaml => sony,imx258.yaml}     |   11 +-
 MAINTAINERS                                   |    2 +-
 drivers/media/i2c/Kconfig                     |    1 +
 drivers/media/i2c/imx258.c                    | 1440 ++++++++++-------
 4 files changed, 835 insertions(+), 619 deletions(-)
 rename Documentation/devicetree/bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} (88%)

Comments

Luis Garcia June 3, 2024, 7:07 a.m. UTC | #1
On 6/2/24 14:13, git@luigi311.com wrote:
> From: Luis Garcia <git@luigi311.com>
> 
> v6:
>   - Drop powerdown-gpio patches
>     - per Sakari Ailus request as it is not part or 
>       not used by the sensor
>     - I tested without it and PPP still works
>     - Dave mentioned its not part of the datasheet 
>       for the imx258
>     - Ondrej Jirman also tested it and it doesnt seem
>       to be needed
> 
> 
> v5:
>   - Changed ownership of a few patches to Ondrej Jirman
>   - Implement feedback from Tommy Merciai
>     - media: i2c: imx258: Add support for reset gpio
>     - media: i2c: imx258: Use v4l2_link_freq_to_bitmap helper
>     - media: i2c: imx258: Convert to new CCI register access helpers
> 
> v4:
>   - Swapped out the use macro patch for a patch that uses the new
>     CCI register access helpers per Sakari Ailus
>   - Fix order of reset and powerdown gpio before disable regulators
>   - Fix formating of all patches
>   
>   - Implemented feedback from Pavel Machek
>     - media: i2c: imx258: Follow normal V4L2 behaviours
>     - media: i2c: imx258: Allow configuration of clock
>   - Implemented feedback from Sakari Ailus
>     - media: i2c: imx258: Add support for powerdown gpio
> 
> 
> v3 Luis Garcia
> 
> - Add Use v4l2_link_freq_to_bitmap helper patch per Sakari Ailus
> - Separate dt-bindings for powerdown per Rob Herring
> - Fix dt-bindings for imx258.yaml
> - Fix sign offs per Dang Huynh 
> - Fix versioning per Conor Dooley and Kieran Bingham
> - Use scripts to validate and fix patches
> - Implemented feedback from Sakari Ailus
>   - media: i2c: imx258: Add support for 24MHz clock
>   - media: i2c: imx258: Add support for running on 2 CSI data lanes
> 
> - Implemented feedback from Rob Herring
>   - dt-bindings: media: imx258: Add alternate compatible strings
> 
> 
> 
> v2 Luis Garcia
> 
> - Add use macros patch 
> - Add support for powerdown gpio patch
> - Add support for reset gpio patch
> - Dropped Add support for long exposure modes patch
> - Implemented feedback from Jacopo Mondi
>   - media: i2c: imx258: Add regulator control
>   - media: i2c: imx258: Add support for 24MHz clock
>   - media: i2c: imx258: Add support for running on 2 CSI data lanes
>   - media: i2c: imx258: Add get_selection for pixel array information
>   - media: i2c: imx258: Issue reset before starting streaming
>   - media: i2c: imx258: Set pixel_rate range to the same as the value
>   - dt-bindings: media: imx258: Add alternate compatible strings
>   - media: i2c: imx258: Change register settings for variants of the sensor
>   - media: i2c: imx258: Make HFLIP and VFLIP controls writable
> 
> This adds a few more patches and drops one. The long exposure mode patch
> was dropped due to the bug that Jacopo found. The powerdown and reset gpio
> patches were added as that fixes support for the Pinephone Pro, without
> them the sensor doesn't initialize correctly.
> 
> Tested on a Pinephone Pro by forcing 24 mhz clock. The two lower 
> resolutions had some artifacts but that is expected as more changes are 
> required to fix them for the Pinephone Pro specifically, kept all 
> registers the same as Dave's original patch since that works on dedicated
> imx258 hardware and the artifacts are PPP specific so it shouldn't 
> be a regression.
> 
> 
> 
> v1 Dave Stevenson
> 
> This is a set of patches for imx258 that allow it to work with alternate
> clock frequencies, over either 2 or 4 lanes, and generally adding
> flexibility to the driver.
> 
> Tested with an IMX258 module from Soho Enterprises that has a 24MHz
> oscillator. Both 2 and 4 lane configurations work with correct link
> frequencies and pixel rates.
> 
> Jacopo has tested on a PinePhone Pro which has an ~19.2MHz clock fed from
> the SoC, He confirms that the two lower resolution modes work, but not the
> full res mode. Comparing to the BSP it looks like they have some weird
> clock configuration in the 4208x3120 mode (nominally 1224Mb/s/lane instead
> of 1267). As it has never previously worked directly with the mainline
> driver this isn't a regression but may indicate that there is a need for
> support of additional link frequencies in the future.
> 
> The last patch that makes HFLIP and VFLIP configurable may be contentious
> as I've retained the default configuration of inverted from the original
> driver. I know this was discussed recently, but I can't recall the final
> outcome.
> 
> I am relying on someone from Intel testing this out, as correcting the
> cropping and supporting flips has changed the Bayer order. Seeing as this
> is all above board in V4L2 terms I really hope that the layers above it
> behave themselves.
> 
> Dave Stevenson (20):
>   media: i2c: imx258: Remove unused defines
>   media: i2c: imx258: Make image geometry meet sensor requirements
>   media: i2c: imx258: Disable digital cropping on binned modes
>   media: i2c: imx258: Remove redundant I2C writes.
>   media: i2c: imx258: Add regulator control
>   media: i2c: imx258: Make V4L2_CID_VBLANK configurable.
>   media: i2c: imx258: Split out common registers from the mode based
>     ones
>   media: i2c: imx258: Add support for 24MHz clock
>   media: i2c: imx258: Add support for running on 2 CSI data lanes
>   media: i2c: imx258: Follow normal V4L2 behaviours for clipping
>     exposure
>   media: i2c: imx258: Add get_selection for pixel array information
>   media: i2c: imx258: Allow configuration of clock lane behaviour
>   media: i2c: imx258: Correct max FRM_LENGTH_LINES value
>   media: i2c: imx258: Issue reset before starting streaming
>   media: i2c: imx258: Set pixel_rate range to the same as the value
>   media: i2c: imx258: Support faster pixel rate on binned modes
>   dt-bindings: media: imx258: Rename to include vendor prefix
>   dt-bindings: media: imx258: Add alternate compatible strings
>   media: i2c: imx258: Change register settings for variants of the
>     sensor
>   media: i2c: imx258: Make HFLIP and VFLIP controls writable
> 
> Luis Garcia (2):
>   media: i2c: imx258: Use v4l2_link_freq_to_bitmap helper
>   media: i2c: imx258: Convert to new CCI register access helpers
> 
> Ondrej Jirman (1):
>   media: i2c: imx258: Add support for reset gpio
> 
>  .../i2c/{imx258.yaml => sony,imx258.yaml}     |   11 +-
>  MAINTAINERS                                   |    2 +-
>  drivers/media/i2c/Kconfig                     |    1 +
>  drivers/media/i2c/imx258.c                    | 1440 ++++++++++-------
>  4 files changed, 835 insertions(+), 619 deletions(-)
>  rename Documentation/devicetree/bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} (88%)
> 

Looks like Sakari has took it forward while i was gone so v6 isnt needed anymore
since they implemented the changes on their side 
https://lore.kernel.org/all/ZleJV5YSl92-13CT@valkosipuli.retiisi.eu/