RE: [PATCH v2 2/2] drm/virtio: fence created per cursor/plane update

["Kasireddy, Vivek" <vivek.kasireddy@intel.com>]

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
> >