Re: [Qemu-devel] [PATCH 1/8] virtio-gpu/2d: add hardware spec include file

[Dave Airlie]

On 30 September 2014 17:55, Gerd Hoffmann <address@hidden> wrote:
>
> On Di, 2014-09-30 at 10:27 +1000, Dave Airlie wrote:
>> > Triggered by the framebuffer endian issues we have with stdvga I've
>> > started to check where we stand with virtio-gpu and whenever we have to
>> > fix something in the virtio protocol before setting in stone with the
>> > upstream merge.
>>
>> Let me start by saying its not that I don't care about endianness, but
>> its a mess I had hoped to avoid until someone else care more about it.
>
> Understood.  It's a big mess indeed.
>
>> So before we try fixing things, we probably need to design something
>> that defines
>> where all the swapping happens, and what formats the virgl "hw" device 
>> supports.
>
> 2D case is easy.  Everything is little endian.  kernel driver / qemu are
> doing the byteswaps for the structs sent over the control pipe.
>
> Problem with 3D is that both qemu and kernel driver passing through data
> where they don't even know what is inside, so they can't do the
> byteswapping.
>
>> The main problem is getting an efficient solution that avoids double swapping
>> of the major items like texture and vertex data if we can in some scenarios.
>
> Yes.
>
>>
>> Currently the virgl "hw" supports little endian defined formats, as
>> per the gallium
>> interface, i.e. B8G8R8A8 means blue/red/green/alpha,
>>
>> http://gallium.readthedocs.org/en/latest/format.html
>> is the documentation.
>
> Thanks.
>
>> >> +    VIRTIO_GPU_FORMAT_B5G5R5A1_UNORM  = 5,
>> >> +    VIRTIO_GPU_FORMAT_B5G6R5_UNORM    = 7,
>> >
>> > Do we really wanna go down this route?  I'm inclined to just zap 16bpp
>> > support.
>>
>> We have to support 16bpp as an OpenGL format no matter what, and this
>> is why endianness sucks, we have lots of strange ass formats we need
>> to send over the wire, that have no nicely defined endianness, like Z24S8.
>
> Ok.  But for 2D we can just not support it, right?

We can, I expect some pushback at some point, people still want to
test with 16bpp
for other areas, and it would be nice to know they can. But I don't really care
about it personally. I just though we should provide at least a basic
number of working
bpps (8,16,32).


>> > What about 3d mode?  We are passing blobs between virglrenderer and
>> > guest driver:  capabilities and gallium 3d command stream.  What happens
>> > to them in case host and guest endianness don't match?  I think at least
>> > the capabilities have 32bit values which need to be swapped.  Dunno
>> > about the gallium commands ...
>>
>> For 3D we probably need to define the gallium command streams to be
>> little endian, however the big problem is the data that is stored
>> inside objects.
>> Texture and vertex, constants, indices etc. How do we decide to swap these,
>> when do we swap things, on the DMA transfers to the host, do we just
>> swap the formats on the host side etc.
>>
>> I probably need to spend some time working this out with BenH, but
>> I'm not really sure how we can avoid backing ourselves into a large 
>> inefficent
>> hole at some point.
>
> I surely don't wanna go down that route, and I think it is reasonable to
> just not support 3D/virgl mode if we would have to swap data.

I think initially not support 3D mode is the way to go until we can work it out.

>
>
> So, we could define *two* virgl feature bits.  One for little endian,
> one for bigendian.  endianness applies to the gallium command stream and
> to gallium formats using integers in host endianess.
>
> On the host side we'll go set the feature bit matching host endianness.
> qemu handles the virtqueue command struct swapping, and virglrenderer
> should only see native endian.
>
> On the guest side we'll look for the feature bit matching guest
> endianness, and if it isn't set due to guest + host having different
> byte order you'll get 2D support only.
>
> The endianness negotiation is a bit iffy, but that way it is possible to
> have virgl on bigendian without swapping everything twice.
>
>
> The other reasonable way would be to simply define virgl being little
> endian.  Bigendian guests / hosts would either simply not support 3D
> then, or implement swapping.  But in the bigendian-guest on
> bigendian-host case we'll swap everything twice then ...
>

I think we should probably move a few more formats from the 3D side
into the 2D side, so we can have the guests just pick the LE format
it requires

http://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/include/pipe/p_format.h#n354

is what gallium currently does, and we could just provide XRGB, XBGR
formats in both endianness
and have the guest pick the one it wants to use.

The 2D pixman code would need updating to provide 2D support for these
formats as well.

I suspect I could add an endian cap for the 3D bits that I could pass
through from guest to host.

How do you test guests with big endian? Isn't it really slow?

Dave.