Messages in this thread |  | | Date | Sat, 3 Dec 2022 14:41:25 +0100 | Subject | Re: [PATCH 00/14] staging: vc04_services: bcm2835-isp support | From | Stefan Wahren <> |
| |
Hi Dave,
Am 02.12.22 um 14:42 schrieb Dave Stevenson: > Hi Stefan > > On Fri, 2 Dec 2022 at 12:41, Stefan Wahren <stefan.wahren@i2se.com> wrote: >> Hi Dave, >> >> Am 02.12.22 um 12:23 schrieb Dave Stevenson: >>> Hi Laurent, Umang, and Stefan. >>> >>> On Fri, 2 Dec 2022 at 09:17, Laurent Pinchart >>> <laurent.pinchart@ideasonboard.com> wrote: >>>> Hi Umang, >>>> >>>> On Fri, Dec 02, 2022 at 11:57:18AM +0800, Umang Jain wrote: >>>>> On 12/2/22 6:45 AM, Stefan Wahren wrote: >>>>>> Am 30.11.22 um 11:58 schrieb Umang Jain: >>>>>>> On 11/27/22 6:56 AM, Stefan Wahren wrote: >>>>>>>> Am 26.11.22 um 17:26 schrieb Umang Jain: >>>>>>>>> On 11/26/22 8:12 PM, Stefan Wahren wrote: >>>>>>>>>> Am 21.11.22 um 22:47 schrieb Umang Jain: >>>>>>>>>>> This series aims to upport bcm2835-isp from the RPi kernel [1] and is a >>>>>>>>>>> independent subset of earlier series [2] posted to upport CSI-2/CCP2 >>>>>>>>>>> receiver IP core("Unicam) + the ISP driver found in BCM283x and compatible >>>>>>>>>>> SoCs (namely BCM2711). Unicam is still under active development to work >>>>>>>>>>> with multistream support to get into mainline. Hence only the ISP driver >>>>>>>>>>> will remain the primary area of this series. >>>>>>>>>> thanks for working on this. But honestly i would prefer that vchiq >>>>>>>>>> comes out of staging before adding more features. As Greg said >>>>>>>>>> some time ago staging is not a place to "dump code and run away". >>>>>>>>>> These new files are in the same bad shape as the rest of vc04 >>>>>>>>>> before the clean-up here in staging started. >>>>>>>>> Certainly, I am not here to do that - but I am still learning the ropes. >>>>>>>> no problem. >>>>>>>> >>>>>>>>> If the staging issue is becoming a blocker for bcm2835-isp going >>>>>>>>> upstream, I would be happy to help here! Though I must mention that >>>>>>>>> I still have limited visibility so my aim would be to chart out a >>>>>>>>> plan of things needed to be done to get vc04_services out of staging! >>>>>>>> The vchiq driver is in staging since 2016, so every step forwards is >>>>>>>> good. Unfortunately all of the low hanging fruits has been gathered. >>>>>>>> >>>>>>>> For me the most important, but not to tricky steps to get vchiq out >>>>>>>> of staging would be: >>>>>>>> >>>>>>>> * Cleanup logging mechanism >>>>>>>> >>>>>>>> * Get rid of custom function return values >>>>>>>> >>>>>>>> There was already an attempt for this [1] >>>>>>>> >>>>>>>> * Get rid of all non essential global structures and create a proper per >>>>>>>> device structure >>>>>>>> >>>>>>>>>> I agree that VCSM is on the TODO list for vchiq, but this driver >>>>>>>>>> is not necessary for making bcm2835-audio & bcm2835-camera leave >>>>>>>>>> staging. It just binds more resources on a new feature. >>>>>>> bcm2835-camera is the legacy camera stack which probably need to be >>>>>>> dropped from hereon... >>>>>> I don't not know if there any users left, so i would be careful here. >>>>>> Can bcm2835-isp completely replace bcm2835-camera? Sorry, for this >>>>>> dumb question but i'm not expert here. >>>>> I am careful too here and probably need Input from RaspberryPi in order >>>>> to proceed to drop it. But from my perspective - bcm2835-camera is _not_ >>>>> going out of staging - it'll either sit here (or probably dropped) as >>>>> statied from [1] >>>>> >>>>> ``` >>>>> + * There are two camera drivers in the kernel for BCM283x - this one >>>>> + * and bcm2835-camera (currently in staging). >>>>> ``` >>>>> >>>>> The bcm2835-camera is meant to be replaced by unicam [1] , but the ISP >>>>> (bcm2835-isp) is meant to be worked with unicam [1]. In fact, I have >>>>> mentioned in my cover the testing of bcm2835-isp happened on top of >>>>> unicam patches. >>>> To be accurate, the bcm2835-camera driver supports the VC4 >>>> firmware-based camera stack. In that setup, the camera sensors (OV5647 >>>> or IMX219), CSI-2 receiver (Unicam) and ISP are all controlled by the >>>> firmware, which provides a high-level interface towards the kernel. This >>>> architecture has been replaced by Linux-side control of the camera >>>> sensors (through existing drivers in drivers/media/i2c/), Unicam >>>> (through the driver from [1]) and ISP (through this driver). Moving >>>> control to the Linux side requires complex processing in userspace, >>>> handled by libcamera. >>>> >>>> bcm2835-camera is thus replaced by multiple drivers combined with >>>> libcamera, and that is the camera stack that is shipped by Raspberry Pi >>>> these days. While this may affect some userspace use cases), we will not >>>> work on destaging bcm2835-camera, and as far as I'm aware, nobody else >>>> is planning to do so either. I don't mind much if the driver stays in >>>> staging for some more time, but I'd rather drop it if possible. >> Thanks for clarification. Okay, so Unicam + bcm2835-isp are able to >> handle the old camera (OV5647)? > There are kernel drivers in mainline for ov5647 (v1) and imx219 (v2) > cameras, with imx477 (HQ camera) due to be done. Connected to > bcm2835-unicam that gives you raw images from these sensors. > > However you have to use libcamera to get useful images with these > drivers - raw Bayer isn't easy to consume. > Libcamera has support for all three (and more) sensors in the RPi IPA > (image processing algorithms), and will make use of bcm2835-isp to > process the images appropriately. > >>> It would be reasonable to drop it at the point that Libcamera can work >>> to a similar level with at least the following list of applications: >>> - FFmpeg >>> - Gstreamer >>> - Chromium >>> - Firefox >>> - Motion >>> And that still leaves a huge number of existing V4L2 apps out in the cold. >>> >>> Do you wish to make any predictions as to when that would be >>> achievable? Or even when a v1.0 release of libcamera is going to >>> happen? >>> Dropping anything prior to those points would be rather premature in my book. >>> >>> >>> The TODOs on bcm2835-camera are: >>> 1) Zero copy. That comes almost for free as bcm2835-isp already does >>> this, but it does rely on vcsm-cma. >>> The main reason I haven't pushed it is that it then requires >>> reasonable amounts of CMA heap for all the buffers, which until >>> recently haven't been present in the default configurations. With the >>> vc4 DRM driver now being default (at least for the vendor kernel) and >>> also requiring CMA, making the change makes more sense. >>> AFAIK there is no easy way to have one driver choosing between using >>> vb2_vmalloc_memops and vb2_dma_contig_memops at runtime, but I may be >>> wrong. >>> Actually bcm2835_defconfig appears to only allocate a 32MB CMA heap, >>> so perhaps we don't get very far. >> CMA configuration should actually happen in device tree or kernel >> cmdline. The bcm2835_defconfig is limited to make it work even with the >> original Pi. > It's the original 256MB Pis that are the issue as memory is so tight. > Losing a bigger chunk to CMA is a problem there. > > I see that bcm283x.dtsi sets it to 64MB, which is probably sufficient > for most bcm2835-camera use cases, but leaves me worried that you'll > steal it from other things. > >>> 2) This isn't workable within the current V4L2 frameworks. The >>> multi-planar V4L2 pixel formats are currently allocated as independent >>> buffers for each plane, whereas the firmware needs a single buffer >>> with (currently) specific offsets for the chroma planes. The >>> V4L2/videobuf2 core changes required to implement that are going to be >>> significant, and have minimal gain. >>> The specific stride handling is already dealt with (set bytesperline >>> appropriately), it's the padding of the height to a multiple of 16 >>> before the chroma planes on YUV420 and NV12 formats that require the >>> firmware to do a small amount of repacking. The performance hit is >>> actually minimal anyway. >>> >>> If bcm2835-camera is the only thing holding back vc04_services, then I >>> can have a look at it. >> No, it's the vchiq interface which needs the work. > OK, I'll liaise with Umang and may look to deal with a couple of them. > Things like documenting the memory barriers probably falls to Phil.
actually this item is already done. I missed to update the TODO file and send a patch soon.
Best regards
> > Dave > >> Thanks Stefan >> >>> Dave >>> >>>>> [1]: https://lore.kernel.org/linux-media/20220208155027.891055-5-jeanmichel.hautbois@ideasonboard.com/ >>>>> >>>>>>>>> I see two TODO files in vc04_services: >>>>>>>>> ./bcm2835-camera/TODO >>>>>>>>> ./interface/TODO >>>>>>>>> >>>>>>>>> One of the bcm2835-camera TODO points to the vc-sm-cma driver >>>>>>>>> itself. So that's address in the series. The other remaining one - >>>>>>>>> I will need to take a deeper look before commenting on it. >>>>>>>>> >>>>>>>>> The main chunk of TODO are in vc04_services/interfaces/TODO. Doing >>>>>>>>> a cursory reading of them suggests that these apply to *all* >>>>>>>>> vc04_services components? Am I right? >>>>>>>> Actually these applies just for the interfaces directory. Some of >>>>>>>> them could apply to the services, but this is no priority. >>>>>>> By no priority, you mean this doesn't affect the criteria required to >>>>>>> ful-fill to get these out of staging? >>>>>> Correct >>>>>> >>>>>>>>> Are these are the specific bits of cleanup you are referring to in >>>>>>>>> your comment? >>>>>>>> You mean about bcm2835-isp? There were too many changes to vchiq >>>>>>>> that i don't remember them all. The first that come to my mind was >>>>>>>> those fancy comment sections which is not kernel coding style. It >>>>>>>> has been removed. >>>>>>> No, I don't mean the bcm2835-isp changes (those are upcoming / >>>>>>> out-of-tree still so...). I mean what are the specific bits / points >>>>>>> that needs to be addressed to get vc04_services out of the staging. >>>>>> These were the points which i mentioned in my last email. They came >>>>>> from interface/TODO. >>>>>> >>>>>>> You have mentioned it above now, so I'll follow up on those. >>>>>> That would be great :) >>>>>> >>>>>>> The many vchiq changes you referred to above comment (that you don't >>>>>>> remember) are from [1] as well or some other series ? >>>>>> Sorry, for the confusing. The many changes i refer were the dozens of >>>>>> clean up patches for vc04_interfaces in mainline staging since the >>>>>> last years. [1] was just a single patch which has been accepted yet. >>>>> Ah I see. There are many others that I've to dig out then. Thanks for >>>>> clarifying! >>>>> >>>>>>>> [1] - >>>>>>>> https://lore.kernel.org/linux-staging/20220712181928.17547-1-jslebodn@redhat.com/ >>>>>>>> >>>>>>>>>> Unfortuntately i hadn't much time to work on vchiq by myself. >>>>>>>>>> >>>>>>>>>> Just my two cents >>>>>>>>>> Stefan >>>> -- >>>> Regards, >>>> >>>> Laurent Pinchart >>> _______________________________________________ >>> linux-arm-kernel mailing list >>> linux-arm-kernel@lists.infradead.org >>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
|  |