[Mesa-dev] [PATCH] panfrost: Backport driver to Mali T600/T700

[Connor Abbott]

On Tue, Feb 19, 2019 at 5:33 PM Emil Velikov <emil.l.velikov at gmail.com>
wrote:

> On Fri, 15 Feb 2019 at 21:39, Alyssa Rosenzweig <alyssa at rosenzweig.io>
> wrote:
> >
> > > - about 1/5 of the patch seems to be white space changes
> >
> > ...Oops. Any tips for avoiding this type of diff churn in the future? I
> > suppose it's not inherently harmful, but maybe it could make merging
> > more difficult than strictly necessary.
> >
> Splitting/polishing patches can be fiddly - but IMHO pays in the long run.
> IIRC not too long ago Dave mentioned that specific work took him more
> time to polish+split than to write+debug the code.
>
> > > - doesn't seem like BIFROST is defined anywhere
> >
> > Indeed it's not; Bifrost is not yet supported, but at least this way we
> > can share headers with the out-of-tree work on Bifrost (is anyone
> > working on these parts right now..?)
> >
> > > - other drivers keep details like is_t6xx, require_sfbd, others in
> > > driver/screen specific struct
> >
> > Aye, that'll be fixed next patch :)
> >
> > > - the __LP64__ checks seems suspicious, no other mesa driver has those
> >
> > Is there a better way to handle mixed bit-ness? We have shared memory
> > (sort of -- separate MMUs, separate address spaces, but they're mapped
> > together with shared physical RAM and we opt for SAME_VA where gpu_va ==
> > user_cpu_va). As such, 32-bit Mali and 64-bit Mali behave differently,
> > since pointers are larger and then some fields get rearranged to pack
> > tighter/less-so depending on pointer sizes. There's no real benefit to
> > support both modes in the same build of the driver; by far, having a
> > 32-bit build for armhf with 32-bit Mali descriptors and a 64-bit build
> > for aarch64 with 64-bit descriptors is the sane approach. Accordingly,
> > I reasoned that __LP64__ is the cleanest way to check what type of
> > system we're building for, and from there which descriptor flavour we
> > should use. Is there something inherently problematic about this scheme?
> >
> I might not be the best person for this, but I think subtle
> differences like these should not be exposed to userspace (be part of
> the UABI).
> In the x86 world running 64bit kernels with 32bit userspace is fairly
> common, but from what I hear it's less so in Arm land.
>

This isn't UABI, since it has absolutely nothing to do with the kernel. The
hardware supports two command stream formats, the 32-bit one where pointers
are expanded from 32-bit to 64-bit internally by the HW and the 64-bit one,
and userspace tells the hardware which one it wants to use by setting a bit
in the job header which is only interpreted by the hardware. Right now the
idea is to select which one based on what bitsize the userspace is compiled
for, hence uintptr_t for pointers, but this could change in the future
without having to notify the kernel. Nothing in the kernel is reading these
pointers at all besides some HW workarounds in the vendor kernel which read
the same "which bitsize am I" bit the HW does.


>
> > In theory we can mix and match, the hardware can do both regardless of
> > the CPU as far as I know, but that complicates things dramatically for
> > little benefit.
> >
> > Keep in mind that Midgard onwards uses descriptors in shared memory,
> > rather than a true command stream, so it's possible no other mesa driver
> > does this since no other mesa-supported hardware needs this.
> >
> I don't know all the drivers but it sounds possible.
>
> Thanks for the extensive reply
> Emil
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190219/194569ae/attachment.html>