Hi, Ivaylo Sorry for the late response, and Thanks very much for the detailed explanations! On Thu, 18 Aug 2022 at 18:23, Ivaylo Dimitrov <ivo.g.dimitrov.75@xxxxxxxxx> wrote: > > Hi, > > On 17.08.22 г. 7:52 ч., Yongqin Liu wrote: > > Hi, Ivaylo > > > > On Mon, 15 Aug 2022 at 14:23, Ivaylo Dimitrov > > <ivo.g.dimitrov.75@xxxxxxxxx> wrote: > >> > >> Hi Liu, > >> > >> On 14.08.22 г. 17:27 ч., Yongqin Liu wrote: > >>> Hi, IvayIo > >>> > >>> Thanks very much for the reply! > >>> > >>> On Sat, 13 Aug 2022 at 14:58, Ivaylo Dimitrov > >>> <ivo.g.dimitrov.75@xxxxxxxxx> wrote: > >>>> > >>>> Hi Liu, > >>>> > >>>> On 12.08.22 г. 7:35 ч., Yongqin Liu wrote: > >>>>> Hi, Ivaylo, Tomi > >>>>> > >>>>> We have one X15 Android AOSP master build, it could not have the home > >>>>> screen displayed > >>>>> on the hdmi monitor connected with this change, with the following > >>>>> message printed on the serial console > >>>>> [ 607.404205] omapdrm omapdrm.0: Failed to setup plane plane-0 > >>>>> [ 607.410522] omapdrm omapdrm.0: Failed to setup plane plane-1 > >>>>> [ 607.416381] omapdrm omapdrm.0: Failed to setup plane plane-2 > >>>>> [ 607.422088] omapdrm omapdrm.0: Failed to setup plane plane-3 > >>>>> > >>>>> # for details, please check the link here: http://ix.io/47m1 > >>>>> > >>>>> It will work with home screen displayed on the hdmi monitor if this > >>>>> change is reverted. > >>>>> > >>>>> Is this the broken problem you talked about here? > >>>>> > >>>>> And could you please give some suggestions on how to have the x15 > >>>>> Android build work with this change? > >>>>> > >>>> > >>>> Make sure scanout (i.e. those to be displayed) buffers are actually > >>>> allocated as such - OMAP_BO_SCANOUT flag must be set when calling > >>>> omap_bo_new(). > >>> > >>> I am not familiar with this area, I am sorry if I asked quite silly questions:( > >>> I googled omap_bo_new, and found it's a function of libdrm here[1], is > >>> it what you meant here? > >>> > >> > >> Yes, calling this function from userspace ends in kernel code the > >> $subject patch is part of. > >> > >>> If it's the omap_bo_new that we should pass OMAP_BO_SCANOUT when it is called, > >>> then is it the correct way to update omap_bo_new to add the OMAP_BO_SCANOUT flag > >>> before it calls omap_bo_new_impl? > >>> > >> > >> omap_bo_new() is fine and does not need any updates/fixes, it is the > >> code that uses it (whoever it is, I am not familiar with the userspace > >> you are using) that shall pass correct flags (third parameter) when > >> calling it. > > > > Sorry, I do not get the point here. > > Like you said, the code that calls omap_bo_new needs to pass OMAP_BO_SCANOUT, > > then IMO omap_bo_new should be the best place to add the OMAP_BO_SCANOUT flag, > > (like via flags = flags | OMAP_BO_SCANOUT), that could help avoid > > missing the flag by some code, > > and also avoids hacks/changes on the possible blob binaries. > > > > Do I misunderstand somewhere? > > Or is there some case that OMAP_BO_SCANOUT shouldn't be passed when > > omap_bo_new is called? > > > > Exactly. You need to pass OMAP_BO_SCANOUT only when you want your > buffers to be 'scanout' buffers(i.e. buffers that can be displayed on > screen), which is not always the case - there is no need offscreen > buffers or pixmaps to be scanout capable, for example. There are more > cases like that. > > The problem is that scanout buffer on OMAP4 allocate additional > resources in DMM/TILER (a piece of hardware) and those resources are > limited. Not only that, but DMM/TILER memory space eventually gets > fragmented over time (if you have lots of allocataoins/deallocations) > and you will start getting ENOMEM (or similar) errors. > > Ofc, in your particular use case you may never hit such issues. Thanks, I understand the cases now. > >> BTW you shall really find who and how uses OMAP BO API, in theory it > >> might use ioctls directly and not call omap_bo_xxx functions. > > > > Do you mean the DRM_OMAP_GEM_NEW ioctl api? > > There is no place in the AOSP tree to call that except the > > omap_bo_new_impl function, > > which is called by the omap_bo_new and omap_bo_new_tiled functions. > > The omap_bo_new should not be called with the OMAP_BO_TILED flag, > > while the omap_bo_new_tiled should be called with the OMAP_BO_TILED flag > > > > Regarding to the omap_bo_new function, there are 2 places call it in > > the AOSP tree: > > #1 ./external/libkmsxx/kms++/src/omap/omapframebuffer.cpp > > #2 ./device/ti/beagle_x15/gpu/gralloc.am57x.so > > > > #1 seems not used in AOSP yet, and #2 is one blob binary we do not > > have the source for. > > > > I would bet on gralloc.am57x.so. yeah, that's my guess as well. > >> strace > >> would be your friend there. or gdb, or whatever tools are used on > >> android. Or put some printfs() in omap_bo_new() that output the PID of > >> the calling process, etc. > > > > Thanks a lot for these great suggestions! Will use them when possible. > > > >>> And another question is that, since the userspace(libdrm) will be used > >>> to work with different kernel versions, > >>> like the old 4.14, 4.19, etc, do you think there will be problem to > >>> pass OMAP_BO_SCANOUT > >>> from the userspace side with the old kernels(which does not have this change)? > >>> does this change need to be backported to the old kernel versions? > >> > >> There should not be any issue. The changes could be backported if one > >> hits the issues this $series is fixing, but there is no need. > > > > Thanks for the confirmation! > > I just boot-tested with adding OMAP_BO_SCANOUT in the omap_bo_new function, > > and it worked with the current 4.14, 4.19, and the mainline kernels. > > # via adding line "flags = flags | OMAP_BO_SCANOUT" in the omap_bo_new function. > > > > sure, the point is that with this change *every* BO will be allocated as > scanout BO, potentially leading to the above explained issues. get it. > >>> > >>> And the last question is that, omap_bo_new might be called by some > >>> property binaries what not everyone > >>> could get the source to update, for such case what's your suggestions? > >>> > >> > >> Hard to say without knowing what that library would be. > >> > >> When I hit issues with closed blobs, sometimes I reverse-engineer them > >> to fix the issue, example: > >> > >> https://github.com/maemo-leste/sgx-ddk-um/tree/master/dbm > >> > >> This is REed libdbm from sgx-ddk-um 1.17.4948957, that is responsible > >> for allocating BOs (what omap_bo_new() does) but it uses DUMB buffers > >> API, instead of OMAP BO API. > >> > >> I guess you are using some older version of sgx-ddk-um, so you may fix > >> in similar way. Or binary patch. > > > > The blob binary that calls omap_bo_new is the gralloc.am57x.so here[2]: > > any suggestions with it? > > # sorry, I am not able to find out how you did the reverse-engineer > > work# with the dbm repository shared here, > > # not sure if you could give some tutorial steps for the similar > > reverse-engineer# work with gralloc.am57x.so > > > > Sorry, but it is like if you ask me to provide you with a tutorial on > how to do brain surgery :) > > > [2]: https://android.googlesource.com/device/ti/beagle-x15/+/refs/heads/master/gpu/gralloc.am57x.so > > > > I investigated this a bit and it seems it calls omap_bo_new() in a > wrapper function like: > > bo = omap_bo_new(dev, -page_size & (size + page_size - 1), ((param5 & > 0x800000) != 0) | OMAP_BO_WC | OMAP_BO_MEM_CONTIG); > > Didn't investigate further what param5 is, but it controls if > OMAP_BO_SCANOUT is passed to omap_bo_new or not. > > However, this library was not made with upstream kernel in mind, as > AFAIK OMAP_BO_MEM_CONTIG never made it upstream: > > https://yhbt.net/lore/all/2580272.MiZDHyRxZo@avalon/T/ > > @Tomi - any comment? > > So, you have couple of options: > > 1. Ask TI for upstream-compatible library. check is in progress, but it would take quite a long time I guess > 2. Try to push OMAP_BO_MEM_CONTIG patch upstream. hmm, sounds like one impossible thing... > 3. Modify omap_bo_new() to something like: > . > #define OMAP_BO_MEM_CONTIG 0x00000008 /* only use contiguous dma mem */ > . > if (flags & OMAP_BO_MEM_CONTIG) > flags |= OMAP_BO_SCANOUT; > . > This will not achieve exactly what OMAP_BO_MEM_CONTIG is supposed to do, > but should make it work, at least. This looks like the only doable thing at the moment, maybe one change needs to be submitted to the mesa/drm repository. I can submit a request on your #3 change to the mesa/drm repository for discussion after some check if you do not mind. Thanks, Yongqin Liu > >>>>> On Thu, 17 Feb 2022 at 23:29, Ivaylo Dimitrov > >>>>> <ivo.g.dimitrov.75@xxxxxxxxx> wrote: > >>>>>> > >>>>>> > >>>>>> > >>>>>> On 17.02.22 г. 14:46 ч., Tomi Valkeinen wrote: > >>>>>>> Hi, > >>>>>>> > >>>>>>> On 19/01/2022 12:23, Ivaylo Dimitrov wrote: > >>>>>>>> On devices with DMM, all allocations are done through either DMM or > >>>>>>>> TILER. > >>>>>>>> DMM/TILER being a limited resource means that such allocations will start > >>>>>>>> to fail before actual free memory is exhausted. What is even worse is > >>>>>>>> that > >>>>>>>> with time DMM/TILER space gets fragmented to the point that even if we > >>>>>>>> have > >>>>>>>> enough free DMM/TILER space and free memory, allocation fails because > >>>>>>>> there > >>>>>>>> is no big enough free block in DMM/TILER space. > >>>>>>>> > >>>>>>>> Such failures can be easily observed with OMAP xorg DDX, for example - > >>>>>>>> starting few GUI applications (so buffers for their windows are > >>>>>>>> allocated) > >>>>>>>> and then rotating landscape<->portrait while closing and opening new > >>>>>>>> windows soon results in allocation failures. > >>>>>>>> > >>>>>>>> Fix that by mapping buffers through DMM/TILER only when really needed, > >>>>>>>> like, for scanout buffers. > >>>>>>> > >>>>>>> Doesn't this break users that get a buffer from omapdrm and expect it to > >>>>>>> be contiguous? > >>>>>>> > >>>>>> > >>>>>> If you mean dumb buffer, then no, this does not break users as dumb > >>>>>> buffers are allocated as scanout: > >>>>>> > >>>>>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/omapdrm/omap_gem.c#L603 > >>>>>> > >>>>>> If you mean omap_bo allocated buffers, then if users want > >>>>>> linear(scanout) buffer, then they request it explicitly by passing > >>>>>> OMAP_BO_SCANOUT. > >>>>>> > >>>>>> Ivo > >>>>> > >>>>> > >>>>> > >>> > >>> > >>> > > > > > > -- Best Regards, Yongqin Liu --------------------------------------------------------------- #mailing list linaro-android@xxxxxxxxxxxxxxxx http://lists.linaro.org/mailman/listinfo/linaro-android
