[PATCH RFC 087/111] staging: etnaviv: align command stream size to 64 bit

[Christian Gmeiner]

2015-04-02 18:45 GMT+02:00 Russell King - ARM Linux <linux at arm.linux.org.uk>:
> On Thu, Apr 02, 2015 at 06:29:24PM +0200, Lucas Stach wrote:
>> The start of the commands must always be 64bit aligned, it's the same
>> for all pipes. The size can be dword aligned if the last command in the
>> stream is something like SET_STATE with a length of 2. In that case one
>> needs to insert another padding dword, but as it's the end of the stream
>> userspace may omit that.
>>
>> So that more a question of policy: do we want userspace to always
>> specify the size including the padding and reject streams with a size
>> not double-dword aligned, or do we just fix it up in the kernel, as it's
>> equally easily done there. I was a bit on the fence here and decided to
>> go the "let the kernel fix things" route.
>
> Without really knowing the hardware, it's hard to say.  However, they
> seem to be designed around a 64-bit architecture, and I would not be
> surprised if the front end DMA always fetches from the command buffer
> in 64-bit quantities.

Thats what the hardware does.

>
> You mention SET_STATE, but the same applies to NOP, WAIT and all of the
> other commands for the 2D cores - the command word must always be in
> the first 32-bits of each 64-bit naturally aligned word in memory.
>

Yup - see https://github.com/laanwj/etna_viv/blob/master/doc/hardware.md#command-stream

> Given that, my feeling (and it's only a feeling) is that it would be
> potentially dangerous to allow userspace to pass a buffer which isn't
> aligned.  As you've found, the kernel would have to align the buffer
> size up to a 64-bit quantity to add the LINK command at the end anyway.
>
> So, I would err on the side of having userspace always do that, and
> we initially enforce that on the kernel side.  If we find that's too
> strict, we can always relax the kernel side - and we still remain
> compatible with userspace.  If we do the reverse, it's harder for the
> kernel to become more strict without breaking userspace.  Given the
> "no regressions" rule, that's something we all want to avoid. :)
>

--
Christian Gmeiner, MSc

https://soundcloud.com/christian-gmeiner