lkml.org 
[lkml]   [2022]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 00/14] staging: vc04_services: bcm2835-isp support
    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.

    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

    \
     
     \ /
      Last update: 2022-12-02 14:43    [W:4.092 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site