| Message ID | 20230502111357.157442-1-dan.scally@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@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. 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@ideasonboard.com> > }; > > } /* namespace */ > -- > 2.34.1 >
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@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@ideasonboard.com> > > > }; > > > > } /* namespace */
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. 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...) Best regards, 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@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@ideasonboard.com> >> >>> }; >>> >>> } /* namespace */ >
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@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@ideasonboard.com> > >> > >>> }; > >>> > >>> } /* namespace */
On Wed, May 03, 2023 at 03:42:50PM +0300, Laurent Pinchart wrote: > 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. Also, given that the driver is on the list and that I have no reason to believe you won't follow up on review and get it merged, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> for this patch. > > 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@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@ideasonboard.com> > > >> > > >>> }; > > >>> > > >>> } /* namespace */
Hi Laurent, On Wed, May 03, 2023 at 03:42:45PM +0300, Laurent Pinchart wrote: > 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. Thanks a lot. Once we have some first comments I will push a v2 :-) > > 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 ? The idea is to first give a try to merge our driver with the one from pinephone (that is adding the CSI2 part into the pinephone version). I am giving it a try currently and then yes, for sure I'll then upstream the 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@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@ideasonboard.com> > > >> > > >>> }; > > >>> > > >>> } /* namespace */ > > -- > Regards, > > Laurent Pinchart
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", {} }, }; } /* namespace */
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@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? 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(+)