Re: [RFC PATCH V0 1/7] [media] dt-bindings: mt8183: Add binding for DIP shared memory

[Tomasz Figa <tfiga@chromium.org>]

On Tue, Feb 12, 2019 at 6:37 PM Frederic Chen
<frederic.chen@mediatek.com> wrote:
>
> Dear Laurent and Sakari,
>
> I appreciate your messages.
>
> On Sat, 2019-02-09 at 20:17 +0200, Laurent Pinchart wrote:
> > On Sat, Feb 09, 2019 at 05:59:07PM +0200, Sakari Ailus wrote:
> > > Hi Frederic,
> > >
> > > Could you cc the devicetree list, please?
> > >
> > > On Fri, Feb 01, 2019 at 07:21:25PM +0800, Frederic Chen wrote:
> > > > This patch adds the binding for describing the shared memory
> > > > used to exchange configuration and tuning data between the
> > > > co-processor and Digital Image Processing (DIP) unit of the
> > > > camera ISP system on Mediatek SoCs.
> > > >
> > > > Signed-off-by: Frederic Chen <frederic.chen@mediatek.com>
> > > > ---
> > > >  .../mediatek,reserve-memory-dip_smem.txt           | 45 ++++++++++++++++++++++
> > > >  1 file changed, 45 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/reserved-memory/mediatek,reserve-memory-dip_smem.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/reserved-memory/mediatek,reserve-memory-dip_smem.txt b/Documentation/devicetree/bindings/reserved-memory/mediatek,reserve-memory-dip_smem.txt
> > > > new file mode 100644
> > > > index 0000000..0ded478
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/reserved-memory/mediatek,reserve-memory-dip_smem.txt
> > > > @@ -0,0 +1,45 @@
> > > > +Mediatek DIP Shared Memory binding
> > > > +
> > > > +This binding describes the shared memory, which serves the purpose of
> > > > +describing the shared memory region used to exchange data between Digital
> > > > +Image Processing (DIP) and co-processor in Mediatek SoCs.
> > > > +
> > > > +The co-processor doesn't have the iommu so we need to use the physical
> > > > +address to access the shared buffer in the firmware.
> > > > +
> > > > +The Digital Image Processing (DIP) can access memory through mt8183 IOMMU so
> > > > +it can use dma address to access the memory region.
> > > > +(See iommu/mediatek,iommu.txt for the detailed description of Mediatek IOMMU)
> > >
> > > What kind of purpose is the memory used for? Buffers containing video data,
> > > or something else? Could the buffer objects be mapped on the devices
> > > based on the need instead?
>
> The memory buffers contain camera 3A tuning data, which are used by the
> co-processor and DIP IP. About mapping the buffer based on the need
> instead, I’m not sure I understand this point. Do you mean that
> allocating and mapping the memory dynamically?
>
> >
> > And could CMA be used when physically contiguous memory is needed ?
>
> DIP driver does not use CMA now, because the first version will be used
> by CrOS but CrOS won’t enable CMA.
>

Thanks Frederic for replying. Let me further clarify what's the problem here.

The co-processor is behind a simple MPU (Memory Protection Unit),
which does not have any mapping capabilities, but can only allow or
deny access to particular parts of the physical address space. That
means that we have to either build some scatter gather capability
inside of the firmware or just do with contiguous allocation.

There is also a security aspect here. The MPU can be accessed from
both the co-processor and CPU, but it has a lockdown mode, which makes
it read only until the SoC is reset. If we allocate the memory
dynamically, we need to keep the MPU unlocked, which automatically
grants the firmware access to all the address space.

For security reasons we decided to go with preallocated memory pool,
which lets us pre-program the MPU and lock it down.

Best regards,
Tomasz

> >
> > > > +
> > > > +
> > > > +Required properties:
> > > > +
> > > > +- compatible: must be "mediatek,reserve-memory-dip_smem"
> > > > +
> > > > +- reg: required for static allocation (see reserved-memory.txt for
> > > > +  the detailed usage)
> > > > +
> > > > +- alloc-range: required for dynamic allocation. The range must
> > > > +  between 0x00000400 and 0x100000000 due to the co-processer's
> > > > +  addressing limitation
> > > > +
> > > > +- size: required for dynamic allocation. The unit is bytes.
> > > > +  If you want to enable the full feature of Digital Processing Unit,
> > > > +  you need 20 MB at least.
> > > > +
> > > > +
> > > > +Example:
> > > > +
> > > > +The following example shows the DIP shared memory setup for MT8183.
> > > > +
> > > > + reserved-memory {
> > > > +         #address-cells = <2>;
> > > > +         #size-cells = <2>;
> > > > +         ranges;
> > > > +         reserve-memory-isp_smem {
> > > > +                 compatible = "mediatek,reserve-memory-dip_smem";
> > > > +                 size = <0 0x1400000>;
> > > > +                 alignment = <0 0x1000>;
> > > > +                 alloc-ranges = <0 0x40000000 0 0x50000000>;
> > > > +         };
> > > > + };
> >
>
> Sincerely,
>
> Frederic Chen
>
>