Skip to content

panfrost: Replace resource shadowing flush

Alyssa Rosenzweig requested to merge alyssa/mesa:panfrost/duckstation into main

The entire point of resource shadowing is to avoid unnecessary flushing. Flushing readers after shadowing is counterproductive. A refresher on how resource shadowing is supposed to work:

First, we determine if it's beneficial to shadow resources. If so, we create a new backing buffer object. We flush the current writer of the resource, if there is one, so the current contents become known to the CPU. If we are not discarding the original resource, we then copy the existing contents of the buffer to the new shadow buffer on the CPU. Finally, we swap the resource's backing buffer for our shadow. Any batch that reads the resource will continue to read the old copy of the resource, and any future draw calls will see the new copy with the change implemented.

Where did we go wrong?

In 988d5aae ("panfrost: Flush resources when shadowing"), we started flushing all readers. We didn't actually need to flush, we just needed to avoid dangling references on the batches reading the old copy of the resource. But that's easily enough avoided: creating a panfrost_resource for the old copy and rewrite the batch's references to point to that. Then, when those batches are flushed, the old copy will be freed and there will be no negative effects on the new copy.

glmark2 -bbuffer:update-method=subdata improved 314fps to 370fps on Rockchip RK3399. This workload motivated the original flush, but even it would rather not flush. Sometimes benchmarks expose real problems :-)

But what actually motivated this is an apitrace from Duckstation's GLES renderer. With this patch, the in-game portion is improved 3fps to 21fps.

Closes: #4028 (closed) Fixes: 988d5aae ("panfrost: Flush resources when shadowing") Signed-off-by: Alyssa Rosenzweig alyssa@collabora.com

Merge request reports