[PATCH] staging: bcm2835-camera: Remove explicit cache flush operations

[Dave Stevenson]

On 8 March 2017 at 13:46, Michael Zoran <mzoran at crowfest.net> wrote:
> On Wed, 2017-03-08 at 13:19 +0000, Dave Stevenson wrote:
>
> Hi Michael.
>
> The original issue was on Pi1 with H264 or MJPEG streams. Buffer sizes were
> small enough to fit in the L1 cache and the relevant sections weren't being
> flushed. The resulting stream ended up being corrupt due to a block of stale
> data from the L1. (L2 is common between VideoCore and ARM on 2835, so not an
> issue. IIRC Videocore is generally using uncached access anyway).
>
> It won't show up with any of the raw image formats as they are all too large
> to fit in L1. Low bitrate, low resolution H264 is the most likely to exhibit
> issues as buffers will be a few kB in size.
>
> I haven't looked at recent vc04_services changes, but iff it handles that
> cleanly then there are no qualms from me.
>
>   Dave
>
> vc04_services now uses dma_map_sg/dma_unmap_sg for bulk which is the
> multi-platform method to hand the buffer back and forth. I'm looking at both
> arm and arm64 code in mm, and it appears to have some rather aggressive code
> to deal with flushing the caches. It's also tries to deal with bounce
> buffers that may be needed and any iommu devices on the bus.

So theoretically it does the right thing, although may be over
aggressive in flushing in this context.

> I'll try some different formats, but I gave away my RPI 1 awhile ago. I do
> have a few RPI 2's laying around.

BCM2835 (Pi0/1) is the only processor in the family where the L2 is
shared between VC4 and ARM.
BCM2836 & 2837 (Pi2 & 3) the ARMs have their own L2 cache so I doubt
they would exhibit this issue anyway.

If you need a Pi0/1 then shout and we can find you one. (Email me directly).

  Dave

> On 8 March 2017 at 12:21, Michael Zoran <mzoran at crowfest.net> wrote:
>
> The camera code has an explicit cache flush operation
> which is not portable.  Now that vc04_services is using portable
> DMA APIs that already do the cache flushing, explicit flushes
> should no longer be needed.
>
> The one call to __cpuc_flush_dcache_area has been removed.
>
> Testing:
>         The offical V2 camera for the RPI was tested on a RPI 3
>         running in 32 bit mode(armhf).  The cheese application
>         and ffmpeg was used to view and stream video from the
>         camera.  Nothing new seems to be broken without the
>         cache flushing.
>
> Signed-off-by: Michael Zoran <mzoran at crowfest.net>
> ---
>  drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> index ca6e9ebc0e81..a57eb829c353 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> @@ -276,10 +276,6 @@ static int bulk_receive(struct vchiq_mmal_instance
> *instance,
>         msg_context->u.bulk.dts = msg->u.buffer_from_host.buffer_header.dts;
>         msg_context->u.bulk.pts = msg->u.buffer_from_host.buffer_header.pts;
>
> -       // only need to flush L1 cache here, as VCHIQ takes care of the L2
> -       // cache.
> -       __cpuc_flush_dcache_area(msg_context->u.bulk.buffer->buffer,
> rd_len);
> -
>         /* queue the bulk submission */
>         vchi_service_use(instance->handle);
>         ret = vchi_bulk_queue_receive(instance->handle,
> --
> 2.11.0
>
>
>