Skip to content

Initial RGBA16F capabilities for vc4#115

Open
nkreeger wants to merge 7 commits into
anholt:masterfrom
nkreeger:vc4-half-float
Open

Initial RGBA16F capabilities for vc4#115
nkreeger wants to merge 7 commits into
anholt:masterfrom
nkreeger:vc4-half-float

Conversation

@nkreeger

Copy link
Copy Markdown

Hi @anholt - here is a few things that I think are needed for #114

I am not a domain expert in drivers so I'm mostly piecing together bits that I see the v3d driver does already. Happy to iterate on this if you are open to some collaboration in helping me bridge some domain knowledge gap here.

@anholt

anholt commented Oct 29, 2018

Copy link
Copy Markdown
Owner

Looks like a reasonable start. Note that src/gallium/driivers/vc4/kernel/is just copies of the kernel code used for the software simulator, and you'll need to modify your actual kernel drivers/gpu/drm/vc4/ code :) (I'll make sure that vc4/kernel is synced up when merging)

@nkreeger

Copy link
Copy Markdown
Author

Oh ok - so the actual change needs to go in here? https://github.com/torvalds/linux/tree/master/drivers/gpu/drm/vc4

@nkreeger

Copy link
Copy Markdown
Author

Or if I'm just targeting Raspberry Pi - here https://github.com/raspberrypi/linux/tree/rpi-4.14.y/drivers/gpu/drm/vc4 ?

@anholt

anholt commented Oct 30, 2018

Copy link
Copy Markdown
Owner

Yeah, feel free to work against downstream rpi's kernel. The validation code hasn't changed in so long that we should be able to apply any patches that you send (to the dri-devel mailing list, since the kernel has an archaic contribution process) to master successfully.

@nkreeger

nkreeger commented Nov 1, 2018

Copy link
Copy Markdown
Author

Great thanks for the info - I just sent the vc4_validate.c change to dri-devel mailing list. I assume it is a good idea to keep these patches small - so I'll try tackling the HDR changes later in another patch.

@nkreeger

nkreeger commented Nov 5, 2018

Copy link
Copy Markdown
Author

@anholt PTAL when you have a chance.

I'm not sure we need the "is float texture" flag - can we just rely on the enum pipe_format format field of vc4_key? Do you just want to avoid a future if condition with a bunch of format == .... checks?

Also - it looks like we can rely on the nir_op-* commands for unpack and packing?

@anholt

anholt commented Nov 5, 2018

Copy link
Copy Markdown
Owner

Yeah, I'd forgotten that we had the format in the key already.

Ideally we would have our texturing unpacks be in NIR, but I haven't done that yet. If we did, there would be a chance for NIR to do some useful optimizations ("oh, hey, you unpacked and then repacked the value, we can just skip that") You'll see at the end of ntq_emit_tex() that we do qir_UNPACK_8_F() for unorm8-to-float conversion, so that's the thing that would need a variant for unpacking 16f instead.

@nkreeger

nkreeger commented Nov 5, 2018

Copy link
Copy Markdown
Author

OK thanks that is helpful - to clarify: Texture packs are handled from NIR but texture unpacks are not?

@anholt

anholt commented Nov 5, 2018

Copy link
Copy Markdown
Owner

You mean packing for TLB writes? Yeah, I moved that to NIR, and some sort of similar logic might work for texturing. (It's kind of weird because we don't get to have NIR have a different number of components returned from the texture instructions, so we'd end up having returned components that are just unused)

@anholt

anholt commented Dec 20, 2018

Copy link
Copy Markdown
Owner

WIP moving the texture unpacking to NIR for v3d, should be straightforward to use in vc4 too:

https://gitlab.freedesktop.org/anholt/mesa/commits/v3d-lower-tex-results

@anholt

anholt commented Jan 5, 2019

Copy link
Copy Markdown
Owner

The NIR unpacking support is now merged:
https://gitlab.freedesktop.org/mesa/mesa/merge_requests/45

@nkreeger

nkreeger commented Jan 6, 2019 via email

Copy link
Copy Markdown
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants