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

[Emil Velikov]

On Tue, 19 Feb 2019 at 16:59, Connor Abbott <cwabbott0 at gmail.com> wrote:
>
> 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.
>
Thanks for the explanation Connor. Seems like I got carried away
thinking the kernel driver modifies or parses these before feeding
them to the hardware.

-Emil