Message-ID: <20190924162217.GA12974@arm.com>
Date:   Tue, 24 Sep 2019 16:22:18 +0000
From:   Ayan Halder <Ayan.Halder@....com>
To:     Sumit Semwal <sumit.semwal@...aro.org>
CC:     John Stultz <john.stultz@...aro.org>,
        Sudipto Paul <Sudipto.Paul@....com>,
        Vincent Donnefort <Vincent.Donnefort@....com>,
        Chenbo Feng <fengc@...gle.com>,
        lkml <linux-kernel@...r.kernel.org>,
        Liam Mark <lmark@...eaurora.org>,
        "Andrew F . Davis" <afd@...com>,
        Christoph Hellwig <hch@...radead.org>,
        Alistair Strachan <astrachan@...gle.com>,
        DRI mailing list <dri-devel@...ts.freedesktop.org>,
        Hridya Valsaraju <hridya@...gle.com>,
        Pratik Patel <pratikp@...eaurora.org>, nd <nd@....com>
Subject: Re: [RESEND][PATCH v8 0/5] DMA-BUF Heaps (destaging ION)

On Thu, Sep 19, 2019 at 10:21:52PM +0530, Sumit Semwal wrote:
> Hello Christoph, everyone,
> 
> On Sat, 7 Sep 2019 at 00:17, John Stultz <john.stultz@...aro.org> wrote:
> >
> > Here is yet another pass at the dma-buf heaps patchset Andrew
> > and I have been working on which tries to destage a fair chunk
> > of ION functionality.
> >
> > The patchset implements per-heap devices which can be opened
> > directly and then an ioctl is used to allocate a dmabuf from the
> > heap.
> >
> > The interface is similar, but much simpler then IONs, only
> > providing an ALLOC ioctl.
> >
> > Also, I've provided relatively simple system and cma heaps.
> >
> > I've booted and tested these patches with AOSP on the HiKey960
> > using the kernel tree here:
> >   https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/dma-buf-heap
> >
> > And the userspace changes here:
> >   https://android-review.googlesource.com/c/device/linaro/hikey/+/909436
> >
> > Compared to ION, this patchset is missing the system-contig,
> > carveout and chunk heaps, as I don't have a device that uses
> > those, so I'm unable to do much useful validation there.
> > Additionally we have no upstream users of chunk or carveout,
> > and the system-contig has been deprecated in the common/andoid-*
> > kernels, so this should be ok.
> >
> > I've also removed the stats accounting, since any such accounting
> > should be implemented by dma-buf core or the heaps themselves.
> >
> > Most of the changes in this revision are adddressing the more
> > concrete feedback from Christoph (many thanks!). Though I'm not
> > sure if some of the less specific feedback was completely resolved
> > in discussion last time around. Please let me know!
> 
> It looks like most of the feedback has been taken care of. If there's
> no more objection to this series, I'd like to merge it in soon.
> 
> If there are any more review comments, may I request you to please provide them?

I tested these patches using our internal test suite with Arm,komeda
driver and the following node in dts

        reserved-memory {
                #address-cells = <0x2>;
                #size-cells = <0x2>;
                ranges;

                framebuffer@...00000 {
                        compatible = "shared-dma-pool";
                        linux,cma-default;
                        reg = <0x0 0x60000000 0x0 0x8000000>;
                };
        }

The tests went fine. Our tests allocates framebuffers of different
sizes, posts them on screen and the driver writes back to one of the
framebuffers. I havenot tested for any performance, latency or
cache management related stuff. So, it that looks appropriate, feel
free to add:-
Tested-by:- Ayan Kumar Halder <ayan.halder@....com>

Are you planning to write some igt tests for it ?
> 
> >
> > New in v8:
> > * Make struct dma_heap_ops consts (Suggested by Christoph)
> > * Add flush_kernel_vmap_range/invalidate_kernel_vmap_range calls
> >   (suggested by Christoph)
> > * Condense dma_heap_buffer and heap_helper_buffer (suggested by
> >   Christoph)
> > * Get rid of needless struct system_heap (suggested by Christoph)
> > * Fix indentation by using shorter argument names (suggested by
> >   Christoph)
> > * Remove unused private_flags value
> > * Add forgotten include file to fix build issue on x86
> > * Checkpatch whitespace fixups
> >
> > Thoughts and feedback would be greatly appreciated!
> >
> > thanks
> > -john
> Best,
> Sumit.
> >
> > Cc: Laura Abbott <labbott@...hat.com>
> > Cc: Benjamin Gaignard <benjamin.gaignard@...aro.org>
> > Cc: Sumit Semwal <sumit.semwal@...aro.org>
> > Cc: Liam Mark <lmark@...eaurora.org>
> > Cc: Pratik Patel <pratikp@...eaurora.org>
> > Cc: Brian Starkey <Brian.Starkey@....com>
> > Cc: Vincent Donnefort <Vincent.Donnefort@....com>
> > Cc: Sudipto Paul <Sudipto.Paul@....com>
> > Cc: Andrew F. Davis <afd@...com>
> > Cc: Christoph Hellwig <hch@...radead.org>
> > Cc: Chenbo Feng <fengc@...gle.com>
> > Cc: Alistair Strachan <astrachan@...gle.com>
> > Cc: Hridya Valsaraju <hridya@...gle.com>
> > Cc: dri-devel@...ts.freedesktop.org
> >
> >
> > Andrew F. Davis (1):
> >   dma-buf: Add dma-buf heaps framework
> >
> > John Stultz (4):
> >   dma-buf: heaps: Add heap helpers
> >   dma-buf: heaps: Add system heap to dmabuf heaps
> >   dma-buf: heaps: Add CMA heap to dmabuf heaps
> >   kselftests: Add dma-heap test
> >
> >  MAINTAINERS                                   |  18 ++
> >  drivers/dma-buf/Kconfig                       |  11 +
> >  drivers/dma-buf/Makefile                      |   2 +
> >  drivers/dma-buf/dma-heap.c                    | 250 ++++++++++++++++
> >  drivers/dma-buf/heaps/Kconfig                 |  14 +
> >  drivers/dma-buf/heaps/Makefile                |   4 +
> >  drivers/dma-buf/heaps/cma_heap.c              | 164 +++++++++++
> >  drivers/dma-buf/heaps/heap-helpers.c          | 269 ++++++++++++++++++
> >  drivers/dma-buf/heaps/heap-helpers.h          |  55 ++++
> >  drivers/dma-buf/heaps/system_heap.c           | 122 ++++++++
> >  include/linux/dma-heap.h                      |  59 ++++
> >  include/uapi/linux/dma-heap.h                 |  55 ++++
> >  tools/testing/selftests/dmabuf-heaps/Makefile |   9 +
> >  .../selftests/dmabuf-heaps/dmabuf-heap.c      | 230 +++++++++++++++
> >  14 files changed, 1262 insertions(+)
> >  create mode 100644 drivers/dma-buf/dma-heap.c
> >  create mode 100644 drivers/dma-buf/heaps/Kconfig
> >  create mode 100644 drivers/dma-buf/heaps/Makefile
> >  create mode 100644 drivers/dma-buf/heaps/cma_heap.c
> >  create mode 100644 drivers/dma-buf/heaps/heap-helpers.c
> >  create mode 100644 drivers/dma-buf/heaps/heap-helpers.h
> >  create mode 100644 drivers/dma-buf/heaps/system_heap.c
> >  create mode 100644 include/linux/dma-heap.h
> >  create mode 100644 include/uapi/linux/dma-heap.h
> >  create mode 100644 tools/testing/selftests/dmabuf-heaps/Makefile
> >  create mode 100644 tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
> >
> > --
> > 2.17.1
> >
> 
> 
> -- 
> Thanks and regards,
> 
> Sumit Semwal
> Linaro Consumer Group - Kernel Team Lead
> Linaro.org │ Open source software for ARM SoCs
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Powered by blists - more mailing lists