[libcamera-devel] [PATCH] libcamera: pipeline: simple: Add support for ST's DCMIPP

[Laurent Pinchart]

Hi Hugues,

On Wed, May 03, 2023 at 10:18:01AM +0200, Hugues FRUCHET wrote:
> Hi Laurent,
> 
> DCMIPP kernel driver has been submitted but no feedback received yet:
> https://www.spinics.net/lists/linux-media/msg218378.html
> Would be nice to get at least a first level of review before pushing a 
> new version.

OK, I'll take the blame :-) Dan told me he will review the patch series,
I'll try to do so too, but will possibly wait for a v2 if he reports
many issues to be addressed.

> For GC2145, we are in the middle of the bridge with two versions of 
> driver that can be found on the net: a pinephone version  with a 
> parallel interface support and our driver which is a CSI-2 version (we 
> have a small amount of support from GalaxyCore, not amazing but better 
> than nothing...)

Do you plan to upstream your driver ?

> On 5/2/23 14:33, Laurent Pinchart wrote:
> > On Tue, May 02, 2023 at 01:29:31PM +0100, Kieran Bingham via libcamera-devel wrote:
> >> Quoting Daniel Scally via libcamera-devel (2023-05-02 12:13:57)
> >>> The STM32 contains a camera pipeline known as the DCMIPP (Digital
> >>> Camera-Memory Interface Pixel Processor) which receives data from a
> >>> parallel interface and dumps the post-processed data to memory. The
> >>> pipeline is capable of some processing in the form of downscaling
> >>> captured data through cropping or skipping the sensor's output.
> >>>
> >>> The simple pipeline handler is quite capable of handling the DCMIPP
> >>> given its operation is handled entirely through configuring the pads
> >>> of a media graph, so add support for the driver to the pipeline's
> >>> supportedDevices array.
> >>>
> >>> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> >>> ---
> >>> Support for the dcmipp has been tested on an stm32mp135f-dk with a gc2145
> >>> camera sensor. Note that the dcmipp driver **is not** in mainline at the moment,
> >>> and nor is the gc2145's driver. @Hugues, @Alain, is there a plan to move those
> >>> drivers upstream?
> >>
> >> As I understand it, the drivers are open, and as long as there is an
> >> expectation that they are going upstream - that's 'enough' to get this
> >> merged in libcamera I believe.
> > 
> > We don't have to wait for the dcmipp driver to land upstream before
> > merging this, but I'd like to understand the upstreaming plan first.
> > 
> >> It's also a really easily 'line' to maintain ;-)
> >>
> >>> A few changes to ST's kernel were needed to fix some format propagation problems
> >>> and some missing controls for the GC2145 for this to work. Those can be found in
> >>> my tree linked below [1], along with some fixes to the CSI-2 to Parallel bridge
> >>> driver which _is_ upstream and which I posted to linux-media [2].
> >>>
> >>> [1] https://github.com/djrscally/linux/tree/v5.15-stm32mp
> >>> [2] https://lore.kernel.org/linux-media/20230502103547.150918-1-dan.scally@ideasonboard.com/T/#t
> >>>
> >>>   src/libcamera/pipeline/simple/simple.cpp | 1 +
> >>>   1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> >>> index e1f8b989..01516bc3 100644
> >>> --- a/src/libcamera/pipeline/simple/simple.cpp
> >>> +++ b/src/libcamera/pipeline/simple/simple.cpp
> >>> @@ -194,6 +194,7 @@ static const SimplePipelineInfo supportedDevices[] = {
> >>>          { "mxc-isi", {} },
> >>>          { "qcom-camss", {} },
> >>>          { "sun6i-csi", {} },
> >>> +       { "dcmipp", {} },
> >>
> >> Alphabetical sort order ;-)
> >>
> >> With that...
> >>
> >> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >>
> >>>   };
> >>>   
> >>>   } /* namespace */

-- 
Regards,

Laurent Pinchart