Hi DW,
On Thu, Jun 09, 2022 at 06:24:43AM +0200, Gerd Hoffmann wrote:
On Fri, Jun 03, 2022 at 02:18:49PM -0700, Dongwon Kim wrote:
Having one fence for a vgfb would cause conflict in case there are multiple planes referencing the same vgfb (e.g. Xorg screen covering two displays in extended mode) being flushed simultaneously. So it makes sence to use a separated fence for each plane update to prevent this.
vgfb->fence is not required anymore with the suggested code change so both prepare_fb and cleanup_fb are removed since only fence creation/ freeing are done in there.
The fences are allocated and released in prepare_fb + cleanup_fb for a reason: atomic_update must not fail.
In case fence allocation fails, it falls back to non-fence path so it won't fail for primary-plane-update.
For cursor plane update, it returns if fence is NULL but we could change it to just proceed and just make it skip waiting like,
[Kasireddy, Vivek] But cursor plane update is always tied to a fence based on the way it works now and we have to fail if there is no fence.
if (fence) { dma_fence_wait(&fence->f, true); dma_fence_put(&fence->f); }
Or maybe I can limit my suggested changes to primary-plane-update only.
What do you think about these?
I guess virtio-gpu must be fixed to use drm_plane_state->fence correctly ...
I was thinking about this too but current functions (e.g. virtio_gpu_cmd_transfer_to_host_2d) takes "struct virtio_gpu_fence". Not sure what is the best way to connect drm_plane_state->fence to virtio_gpu_fence without changing major function interfaces.
[Kasireddy, Vivek] FWIU, we cannot use drm_plane_state->fence as it is used by drm core to handle explicit fences. So, I think a cleaner way is to subclass base drm_plane_state and move the fence from virtio_gpu_framebuffer to a new struct virtio_gpu_plane_state. This way, we can create the fence in prepare_fb() and use it for synchronization in resource_flush.
Thanks, Vivek
take care, Gerd