| Message ID | 20191025225009.50305-2-john.stultz@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Support non-default CMA regions to the dmabuf heaps interface | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Fri, Oct 25, 2019 at 5:51 PM John Stultz <john.stultz@linaro.org> wrote: > > This binding specifies which CMA regions should be added to the > dmabuf heaps interface. Is this an ION DT binding in disguise? I thought I killed that. ;) > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Laura Abbott <labbott@redhat.com> > Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org> > Cc: Sumit Semwal <sumit.semwal@linaro.org> > Cc: Liam Mark <lmark@codeaurora.org> > Cc: Pratik Patel <pratikp@codeaurora.org> > Cc: Brian Starkey <Brian.Starkey@arm.com> > Cc: Andrew F. Davis <afd@ti.com> > Cc: Chenbo Feng <fengc@google.com> > Cc: Alistair Strachan <astrachan@google.com> > Cc: Sandeep Patil <sspatil@google.com> > Cc: Hridya Valsaraju <hridya@google.com> > Cc: devicetree@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: John Stultz <john.stultz@linaro.org> > --- > .../bindings/dma/dmabuf-heap-cma.txt | 31 +++++++++++++++++++ > 1 file changed, 31 insertions(+) > create mode 100644 Documentation/devicetree/bindings/dma/dmabuf-heap-cma.txt > > diff --git a/Documentation/devicetree/bindings/dma/dmabuf-heap-cma.txt b/Documentation/devicetree/bindings/dma/dmabuf-heap-cma.txt > new file mode 100644 > index 000000000000..bde7b1f1c269 > --- /dev/null > +++ b/Documentation/devicetree/bindings/dma/dmabuf-heap-cma.txt > @@ -0,0 +1,31 @@ > +* DMA-BUF CMA Heap > + > +Specifies a CMA region that should be added to the dma-buf heaps > +interface. > + > +Required properties: > +- compatible: Must be "dmabuf-heap-cma" > +- memory-region: phandle to a CMA reserved memory node > + > +Example: > +This example has a camera CMA node in reserved memory, which is then > +referenced by the dmabuf-heap-cma node. > + > + > + reserved-memory { > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + ... > + cma_camera: cma-camera { > + compatible = "shared-dma-pool"; > + reg = <0x0 0x24C00000 0x0 0x4000000>; > + reusable; > + }; > + ... > + }; > + > + cma_heap { > + compatible = "dmabuf-heap-cma"; > + memory-region = <&cma_camera>; Why the indirection here? Can't you just add a flag property to reserved-memory nodes like we do to flag CMA nodes? As I suspected, it's because in patch 2 you're just abusing DT to instantiate platform devices. We already support binding drivers to reserved-memory nodes directly. Rob
On Fri, Oct 25, 2019 at 4:32 PM Rob Herring <rob.e.herring@gmail.com> wrote: > > On Fri, Oct 25, 2019 at 5:51 PM John Stultz <john.stultz@linaro.org> wrote: > > > > This binding specifies which CMA regions should be added to the > > dmabuf heaps interface. > > Is this an ION DT binding in disguise? I thought I killed that. ;) Maybe? I may not have been paying attention back then. :) > > +Example: > > +This example has a camera CMA node in reserved memory, which is then > > +referenced by the dmabuf-heap-cma node. > > + > > + > > + reserved-memory { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + ranges; > > + ... > > + cma_camera: cma-camera { > > + compatible = "shared-dma-pool"; > > + reg = <0x0 0x24C00000 0x0 0x4000000>; > > + reusable; > > + }; > > + ... > > + }; > > + > > + cma_heap { > > + compatible = "dmabuf-heap-cma"; > > + memory-region = <&cma_camera>; > > Why the indirection here? Can't you just add a flag property to > reserved-memory nodes like we do to flag CMA nodes? Happy to try. Do you mean like with the "reuasable" tag? Or more like the "linux,cma-default" tag? Do you have a preference for the flag name here? > As I suspected, it's because in patch 2 you're just abusing DT to > instantiate platform devices. We already support binding drivers to > reserved-memory nodes directly. Sorry, one of those "when all you know how to do is hammer, everything looks like a nail" issues. Is there a specific example for binding drivers to reserved-memory nodes I can try to follow? Appreciate the review and feedback! thanks -john
On Fri, Oct 25, 2019 at 04:55:35PM -0700, John Stultz wrote: > On Fri, Oct 25, 2019 at 4:32 PM Rob Herring <rob.e.herring@gmail.com> wrote: > > > > On Fri, Oct 25, 2019 at 5:51 PM John Stultz <john.stultz@linaro.org> wrote: > > > > > > This binding specifies which CMA regions should be added to the > > > dmabuf heaps interface. > > > > Is this an ION DT binding in disguise? I thought I killed that. ;) > > Maybe? I may not have been paying attention back then. :) > > > > +Example: > > > +This example has a camera CMA node in reserved memory, which is then > > > +referenced by the dmabuf-heap-cma node. > > > + > > > + > > > + reserved-memory { > > > + #address-cells = <2>; > > > + #size-cells = <2>; > > > + ranges; > > > + ... > > > + cma_camera: cma-camera { > > > + compatible = "shared-dma-pool"; > > > + reg = <0x0 0x24C00000 0x0 0x4000000>; > > > + reusable; > > > + }; > > > + ... > > > + }; > > > + > > > + cma_heap { > > > + compatible = "dmabuf-heap-cma"; > > > + memory-region = <&cma_camera>; > > > > Why the indirection here? Can't you just add a flag property to > > reserved-memory nodes like we do to flag CMA nodes? > > Happy to try. Do you mean like with the "reuasable" tag? Or more like > the "linux,cma-default" tag? Probably like "linux,cma-default" as it is a hint for who to manage it rather than a characteristic of the region. > Do you have a preference for the flag name here? Not really. > > As I suspected, it's because in patch 2 you're just abusing DT to > > instantiate platform devices. We already support binding drivers to > > reserved-memory nodes directly. > > Sorry, one of those "when all you know how to do is hammer, everything > looks like a nail" issues. > Is there a specific example for binding drivers to reserved-memory > nodes I can try to follow? ramoops and I think there's a QCom driver. Rob
diff --git a/Documentation/devicetree/bindings/dma/dmabuf-heap-cma.txt b/Documentation/devicetree/bindings/dma/dmabuf-heap-cma.txt new file mode 100644 index 000000000000..bde7b1f1c269 --- /dev/null +++ b/Documentation/devicetree/bindings/dma/dmabuf-heap-cma.txt @@ -0,0 +1,31 @@ +* DMA-BUF CMA Heap + +Specifies a CMA region that should be added to the dma-buf heaps +interface. + +Required properties: +- compatible: Must be "dmabuf-heap-cma" +- memory-region: phandle to a CMA reserved memory node + +Example: +This example has a camera CMA node in reserved memory, which is then +referenced by the dmabuf-heap-cma node. + + + reserved-memory { + #address-cells = <2>; + #size-cells = <2>; + ranges; + ... + cma_camera: cma-camera { + compatible = "shared-dma-pool"; + reg = <0x0 0x24C00000 0x0 0x4000000>; + reusable; + }; + ... + }; + + cma_heap { + compatible = "dmabuf-heap-cma"; + memory-region = <&cma_camera>; + };
This binding specifies which CMA regions should be added to the dmabuf heaps interface. Cc: Rob Herring <robh+dt@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Laura Abbott <labbott@redhat.com> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org> Cc: Sumit Semwal <sumit.semwal@linaro.org> Cc: Liam Mark <lmark@codeaurora.org> Cc: Pratik Patel <pratikp@codeaurora.org> Cc: Brian Starkey <Brian.Starkey@arm.com> Cc: Andrew F. Davis <afd@ti.com> Cc: Chenbo Feng <fengc@google.com> Cc: Alistair Strachan <astrachan@google.com> Cc: Sandeep Patil <sspatil@google.com> Cc: Hridya Valsaraju <hridya@google.com> Cc: devicetree@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz <john.stultz@linaro.org> --- .../bindings/dma/dmabuf-heap-cma.txt | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 Documentation/devicetree/bindings/dma/dmabuf-heap-cma.txt