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

[Emil Velikov]

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.

> 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