drivers: video: stm32_dcmipp: Initialize ISP only once

[ABESTM]

This fixes bug where stm32_dcmipp_isp_init is called multiple times.

[erwango]

@ABESTM CI failed due to a SonarCloud issue, nothing related to your PR, but I'm not able to launch it again. Would you mind rebasing your PR to hopefully get it right next time ?

[HaeberleAtBosch]

I have tested the patch with a modified video capture example (can not attach here) - which does change the zoom level every second - and it does work well.

[etienne-lms]

Does that mean caller for this function may return with success while another thread has still not yet completed the pipe initialization through stm32_dcmipp_stream_enable()? Maybe a mutex would be preferable.

[ABESTM]

Good point, this could be a problem, and I will rewrite this.
There is already a mutex on the pipe I can probably reuse. Thank you.

[avolmat-st]

Thanks @ABESTM for this fix.
This isp initialization has to be done only once for the whole device instead of once per pipe, so I think it should be held within the struct stm32_dcmipp_data instead of within the pipe structure in order to avoid having it initialized everytime a pipe is used (in a multi-pipe use case).

[ABESTM]

I updated the mutex part.
I noticed that in stm32_dcmipp_get_fmt the stm32_dcmipp_isp_init is called only for DCMIPP_PIPE1 and DCMIPP_PIPE2, but in stm32_dcmipp_stream_enable it is called always.
Maybe in the stm32_dcmipp_stream_enable it should be also called only for DCMIPP_PIPE1 and DCMIPP_PIPE2?

[avolmat-st]

Yes indeed, it might better be part of the previous if statement right after the set_yuv_conversion.
However in fact, considering that the isp_init is done within the get_fmt function, I am no more sure it is really necessary within the stream_enable function. In most / all (current) cases, it will never do anything in fact.

[ABESTM]

Maybe the optimal solution would be to call it inside stm32_dcmipp_init, but this would require that the camera sensor (config->source_dev) is initialized before. This way we could get rid of the mutex.
This would probably require defining additional priority level in Kconfig.

[ABESTM]

I reworked the PR, to initialized ISP during dcmipp initialization. This is similar approach to what is done in video_mcux_mipi_csi2rx.c driver. I added new initialization priority (VIDEO_ISP_INIT_PRIORITY) to global config.
@ngphibang Maybe this could replace the VIDEO_MCUX_MIPI_CSI2RX_INIT_PRIORITY priority in NXP driver?

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud