Skip to content

Conversation

@avolmat-st
Copy link

@avolmat-st avolmat-st commented Jun 22, 2025

This PR address the issue #89567 and cleanup few points related to registers access.

Few more commits added to perform some cleanups. Moreover moved to the CCI API functions to avoid having to limit gc2145 specific functions and allow having the driver instanciated multiple times.

@github-actions github-actions bot added the area: Video Video subsystem label Jun 22, 2025
{0x15, 0x37},
{0x16, 0x45},
{GC2145_REG_OUTPUT_FMT, 0x53},
{0x84, 0x53},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like #define GC2145_REG_OUTPUT_FMT 0x84 above would lead to the same value. Any reason to change it?

Copy link
Author

@avolmat-st avolmat-st Jun 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REG_OUTPUT_FMT register is within the PAGE0. Here at that moment, we are accessing to another page hence, we souldn't call this REG_OUTPUT_FMT since this is another register with another purpose.
(I couldn't confirm which one since it is undocumented register based on the spec I have)

Copy link
Contributor

@josuah josuah Jun 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes that is right good point, I did not spot it... I understand the commit message now. Thank you!

@avolmat-st
Copy link
Author

I tested that on a CSI based sensor so I'd be interested in having some tests done on a parallel interface based GC2145.

Alain Volmat added 7 commits June 22, 2025 21:38
Help identify register page change by using GC2145_REG_RESET macro.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Avoid some register page change when the right page is
already being accessible.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Correct wrong macro GC2145_REG_OUTPUT_FMT / GC2145_REG_SYNC_MODE
being used while a different page is being accessed.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
This bit would be used in order to generate several variants
of Bayer formats, however it shouldn't be enabled for YUV/RGB
formats by default.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Use video cci helper functions to access to the sensor.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Set the default format at the moment of variable declaration
in gc2145_init function.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Allow the driver to be instanciated multiple times.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
@avolmat-st avolmat-st force-pushed the pr_gc2145_color_swap_fix branch from ed72fa4 to 47020db Compare June 22, 2025 21:26
@avolmat-st avolmat-st changed the title Small GC2145 cleanup/fixes & fix of B/R (U/V) color swap GC2145: fixes (fix of B/R (U/V) color swap) / cleanups, cci API usage and allow driver multi-instance Jun 22, 2025
@sonarqubecloud
Copy link

@josuah
Copy link
Contributor

josuah commented Jun 23, 2025

This works well on DVP too!
mpv-shot0001

However, I had to apply this fix to DCMI: #92018

Copy link
Contributor

@ngphibang ngphibang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@dkalowsk dkalowsk merged commit d9c935c into zephyrproject-rtos:main Jun 25, 2025
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Video Video subsystem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants