On Fri, 15 Feb 2019 at 15:06, Rob Clark via dri-devel <dri-devel@xxxxxxxxxxxxxxxxxxxxx> wrote: > > On Fri, Feb 15, 2019 at 8:42 AM Eric Engestrom <eric.engestrom@xxxxxxxxx> wrote: > > > > On Friday, 2019-02-15 13:36:39 +0000, Eric Engestrom wrote: > > > On Friday, 2019-02-15 07:11:55 -0500, Rob Clark wrote: > > > > On Fri, Feb 15, 2019 at 3:55 AM Daniel Drake <drake@xxxxxxxxxxxx> wrote: > > > > > > > > > > Hi, > > > > > > > > > > Using libdrm-2.4.97, mesa fails to build on ARM with: > > > > > > > > > > [ 456s] In file included from > > > > > ../../../../../src/gallium/drivers/freedreno/freedreno_util.h:33, > > > > > [ 456s] from > > > > > ../../../../../src/gallium/drivers/freedreno/freedreno_batch.h:34, > > > > > [ 456s] from > > > > > ../../../../../src/gallium/drivers/freedreno/freedreno_context.h:39, > > > > > [ 456s] from > > > > > ../../../../../src/gallium/drivers/freedreno/freedreno_program.c:33: > > > > > [ 456s] /usr/include/freedreno/freedreno_ringbuffer.h:32:10: fatal > > > > > error: xf86atomic.h: No such file or directory > > > > > > > > > > The freedreno headers were recently modified to use xf86atomic.h: > > > > > https://gitlab.freedesktop.org/mesa/drm/commit/b541d21a0a908bf98d44375720f4430297720743 > > > > > > > > > > > > > oh, that union/ifdef hack was specifically to avoid this issue.. > > > > probably the patch removing it should be reverted. > > > > > > Right, I messed up with that commit, I didn't realise freedreno_ringbuffer.h > > > was installed. We need to remove that include. > > > > > > That said, I'm confused as to how freedreno_ringbuffer.h users in Mesa > > > knows whether it's safe to use refcnt from that union? > > > It doesn't check for HAS_ATOMIC_OPS, so it can't know whether it > > > contains garbage padding or a refcount, can it? > > > > No, that wouldn't even compile? > > > > (with the code before my messed up commit:) > > Mesa includes freedreno_ringbuffer.h but doesn't define HAS_ATOMIC_OPS, > > so fd_ringbuffer::refcnt doesn't get compiled in (but the padding is > > still there), so code in mesa can't use ->refcnt because the compiler > > wouldn't know what that is. > > > > I must be missing something, how did this ever compile? > > So, these days, mesa has it's own copy of the libdrm code, > libdrm_freedreno really only exists so that you can still build old > mesa with new libdrm. And for a handful of small standalone utilities > (fdperf, and some test code I use to poke the hw standalone).. > > But the way it works is that mesa never needs to access the refcnt, it > mostly only needs to access cur/end (and it wants to do that in a way > that can be inlined, not fxn call into a different dso, since that is > a hot path). The only code that accesses the refcnt is in the libdrm > code itself. Hence this ugly union hack, just to make the struct the > same size both for mesa and for libdrm. > Ouch, I did not see the header was installed either. Just skimmed through Mesa prior to the libdrm_freedreno merge - there are no references of fd_ringbuffer::refcnt. So a simple revert will do the job. To avoid repeating this mistake, we want to add an inline comment. -Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
