On Mon, Oct 21, 2019 at 09:18:07AM +0000, Brian Starkey wrote: > On Sat, Oct 19, 2019 at 09:41:27AM -0400, Andrew F. Davis wrote: > > On 10/18/19 2:57 PM, Ayan Halder wrote: > > > On Fri, Oct 18, 2019 at 11:49:22AM -0700, John Stultz wrote: > > >> On Fri, Oct 18, 2019 at 11:41 AM Ayan Halder <Ayan.Halder@xxxxxxx> wrote: > > >>> On Fri, Oct 18, 2019 at 09:55:17AM +0000, Brian Starkey wrote: > > >>>> On Thu, Oct 17, 2019 at 01:57:45PM -0700, John Stultz wrote: > > >>>>> On Thu, Oct 17, 2019 at 12:29 PM Andrew F. Davis <afd@xxxxxx> wrote: > > >>>>>> On 10/17/19 3:14 PM, John Stultz wrote: > > >>>>>>> But if the objection stands, do you have a proposal for an alternative > > >>>>>>> way to enumerate a subset of CMA heaps? > > >>>>>>> > > >>>>>> When in staging ION had to reach into the CMA framework as the other > > >>>>>> direction would not be allowed, so cma_for_each_area() was added. If > > >>>>>> DMA-BUF heaps is not in staging then we can do the opposite, and have > > >>>>>> the CMA framework register heaps itself using our framework. That way > > >>>>>> the CMA system could decide what areas to export or not (maybe based on > > >>>>>> a DT property or similar). > > >>>>> > > >>>>> Ok. Though the CMA core doesn't have much sense of DT details either, > > >>>>> so it would probably have to be done in the reserved_mem logic, which > > >>>>> doesn't feel right to me. > > >>>>> > > >>>>> I'd probably guess we should have some sort of dt binding to describe > > >>>>> a dmabuf cma heap and from that node link to a CMA node via a > > >>>>> memory-region phandle. Along with maybe the default heap as well? Not > > >>>>> eager to get into another binding review cycle, and I'm not sure what > > >>>>> non-DT systems will do yet, but I'll take a shot at it and iterate. > > >>>>> > > >>>>>> The end result is the same so we can make this change later (it has to > > >>>>>> come after DMA-BUF heaps is in anyway). > > >>>>> > > >>>>> Well, I'm hesitant to merge code that exposes all the CMA heaps and > > >>>>> then add patches that becomes more selective, should anyone depend on > > >>>>> the initial behavior. :/ > > >>>> > > >>>> How about only auto-adding the system default CMA region (cma->name == > > >>>> "reserved")? > > >>>> > > >>>> And/or the CMA auto-add could be behind a config option? It seems a > > >>>> shame to further delay this, and the CMA heap itself really is useful. > > >>>> > > >>> A bit of a detour, comming back to the issue why the following node > > >>> was not getting detected by the dma-buf heaps framework. > > >>> > > >>> reserved-memory { > > >>> #address-cells = <2>; > > >>> #size-cells = <2>; > > >>> ranges; > > >>> > > >>> display_reserved: framebuffer@60000000 { > > >>> compatible = "shared-dma-pool"; > > >>> linux,cma-default; > > >>> reusable; <<<<<<<<<<<<-----------This was missing in our > > >>> earlier node > > >>> reg = <0 0x60000000 0 0x08000000>; > > >>> }; > > >> > > >> Right. It has to be a CMA region for us to expose it from the cma heap. > > >> > > >> > > >>> With 'reusable', rmem_cma_setup() succeeds , but the kernel crashes as follows :- > > >>> > > >>> [ 0.450562] WARNING: CPU: 2 PID: 1 at mm/cma.c:110 cma_init_reserved_areas+0xec/0x22c > > >> > > >> Is the value 0x60000000 you're using something you just guessed at? It > > >> seems like the warning here is saying the pfn calculated from the base > > >> address isn't valid. > > > It is a valid memory region we use to allocate framebuffers. > > > > > > But does it have a valid kernel virtual mapping? Most ARM systems (just > > assuming you are working on ARM :)) that I'm familiar with have the DRAM > > space starting at 0x80000000 and so don't start having valid pfns until > > that point. Is this address you are reserving an SRAM? > > > > Yeah, I think you've got it. > > This region is DRAM on an FPGA expansion tile, but as you have noticed > its "below" the start of main RAM, and I expect it's not in any of the > declared /memory/ nodes. > > When "reusable" isn't there, I think we'll end up going the coherent.c > route, with dma_init_coherent_memory() setting up some pages. > > If "reusable" is there, then I think we'll end up in contiguous.c and > that expects us to already have pages. > > So, @Ayan, you could perhaps try adding this region as a /memory/ node > as-well, which should mean the kernel sets up some pages for it as > normal memory. But, I have some ancient recollection that the arm64 > kernel couldn't handle system RAM at addresses below 0x80000000 or > something. That might be different now, I'm talking about several > years ago. > Thanks a lot for your suggestions. I added the following node in the dts. memory@60000000 { device_type = "memory"; reg = <0 0x60000000 0 0x08000000>; }; And kept the 'reusable' property in display_reserved:framebuffer@60000000 {...}; Now the kernel boots fine. I am able to get /dev/dma_heap/framebuffer\@60000000 . :) > Thanks, > -Brian > > > Andrew > > > > > > >> > > >> thanks > > >> -john _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel