[Qemu-devel] virtio-gpu: cursor update not in sync with resource update

[Qemu-devel] virtio-gpu: cursor update not in sync with resource update

[Marc-André Lureau]

Hi

Afaik, the goal of the seperate cursor queue is to update cursor
quickly (probably to give good input responsiveness). However, the
VIRTIO_GPU_CMD_UPDATE_CURSOR references a surface that is uploaded via
the ctrl queue. The same resource id may be reused for cursor update.
But there is no synchronization between the two queues. So often, the
cursor set results in last update being shown. Should the kernel wait
for the update to finish? But then why having 2 different queues?
Should the update be done on the cursor queue instead? or should the
different cursor be cached (with different id) and qemu to do the
synchronization?

-- 
Marc-André Lureau

Re: [Qemu-devel] virtio-gpu: cursor update not in sync with resource update

[Gerd Hoffmann]

  Hi,

> Afaik, the goal of the seperate cursor queue is to update cursor
> quickly (probably to give good input responsiveness). However, the
> VIRTIO_GPU_CMD_UPDATE_CURSOR references a surface that is uploaded via
> the ctrl queue. The same resource id may be reused for cursor update.
> But there is no synchronization between the two queues. So often, the
> cursor set results in last update being shown. Should the kernel wait
> for the update to finish?

Yes (for new cursors).

> But then why having 2 different queues?
> Should the update be done on the cursor queue instead? or should the
> different cursor be cached (with different id) and qemu to do the
> synchronization?

The guest can upload different cursors and then switch between them
without re-uploading and therefore without ctrl queue updates.  Thats
why they have an id in the first place ...

cheers,
  Gerd

Re: [Qemu-devel] virtio-gpu: cursor update not in sync with resource update

[Marc-André Lureau]

Hi

On Mon, Sep 14, 2015 at 4:06 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> The guest can upload different cursors and then switch between them
> without re-uploading and therefore without ctrl queue updates.  Thats
> why they have an id in the first place ...

Ok, but the cursor id is reused most of the time. So there are at
least two things to fix: use different cursor ids for different cursor
data (with some cache management probably), fix kernel to only upload
new resource id and wait for data to be transfered. Is that correct?


-- 
Marc-André Lureau

Re: [Qemu-devel] virtio-gpu: cursor update not in sync with resource update

[Dave Airlie]

On 15 September 2015 at 01:17, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> Hi
>
> On Mon, Sep 14, 2015 at 4:06 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> The guest can upload different cursors and then switch between them
>> without re-uploading and therefore without ctrl queue updates.  Thats
>> why they have an id in the first place ...
>
> Ok, but the cursor id is reused most of the time. So there are at
> least two things to fix: use different cursor ids for different cursor
> data (with some cache management probably), fix kernel to only upload
> new resource id and wait for data to be transfered. Is that correct?

Currently the kernel driver does this:

        ret = virtio_gpu_cmd_transfer_to_host_2d(vgdev, qobj->hw_res_handle, 0,
                                                 cpu_to_le32(64),
                                                 cpu_to_le32(64),
                                                 0, 0, &fence);
        if (!ret) {
                reservation_object_add_excl_fence(qobj->tbo.resv,
                                                  &fence->f);
                virtio_gpu_object_wait(qobj, false);
        }

before moving the cursor, shouldn't that suffice?

Do we fail the transfer sometimes?

Dave.

Re: [Qemu-devel] virtio-gpu: cursor update not in sync with resource update

[Marc-André Lureau]

Hi

On Mon, Sep 14, 2015 at 11:08 PM, Dave Airlie <airlied@gmail.com> wrote:
> Currently the kernel driver does this:
>
>         ret = virtio_gpu_cmd_transfer_to_host_2d(vgdev, qobj->hw_res_handle, 0,
>                                                  cpu_to_le32(64),
>                                                  cpu_to_le32(64),
>                                                  0, 0, &fence);
>         if (!ret) {
>                 reservation_object_add_excl_fence(qobj->tbo.resv,
>                                                   &fence->f);
>                 virtio_gpu_object_wait(qobj, false);
>         }
>
> before moving the cursor, shouldn't that suffice?
>
> Do we fail the transfer sometimes?


That's apparently not in Gerd tree.

-- 
Marc-André Lureau

Re: [Qemu-devel] virtio-gpu: cursor update not in sync with resource update

[Dave Airlie]

On 15 September 2015 at 07:14, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> Hi
>
> On Mon, Sep 14, 2015 at 11:08 PM, Dave Airlie <airlied@gmail.com> wrote:
>> Currently the kernel driver does this:
>>
>>         ret = virtio_gpu_cmd_transfer_to_host_2d(vgdev, qobj->hw_res_handle, 0,
>>                                                  cpu_to_le32(64),
>>                                                  cpu_to_le32(64),
>>                                                  0, 0, &fence);
>>         if (!ret) {
>>                 reservation_object_add_excl_fence(qobj->tbo.resv,
>>                                                   &fence->f);
>>                 virtio_gpu_object_wait(qobj, false);
>>         }
>>
>> before moving the cursor, shouldn't that suffice?
>>
>> Do we fail the transfer sometimes?
>
>
> That's apparently not in Gerd tree.

Interesting, I'm not sure where this got lost,

I had it in my clone of Gerd's tree from a while back.

Dave.

Re: [Qemu-devel] virtio-gpu: cursor update not in sync with resource update

[Gerd Hoffmann]

  Hi,

> >>         if (!ret) {
> >>                 reservation_object_add_excl_fence(qobj->tbo.resv,
> >>                                                   &fence->f);
> >>                 virtio_gpu_object_wait(qobj, false);
> >>         }
> >>
> >> before moving the cursor, shouldn't that suffice?
> >>
> >> Do we fail the transfer sometimes?
> >
> >
> > That's apparently not in Gerd tree.
> 
> Interesting, I'm not sure where this got lost,

Probably fallen through the cracks with the recent fence fixups.
I've added it back.

cheers,
  Gerd