freedreno header uses not installed xf86atomic.h

[Rob Clark]

On Fri, Feb 15, 2019 at 8:42 AM Eric Engestrom <eric.engestrom at intel.com> 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 at endlessm.com> 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.

BR,
-R