diff mbox series

[1/3] virtio-gpu: rutabaga: Properly set stride when copying resources

Message ID 20240605152832.11618-2-weifeng.liu.z@gmail.com (mailing list archive)
State New, archived
Headers show
Series virtio-gpu: Enable virglrenderer backend for rutabaga | expand

Commit Message

Weifeng Liu June 5, 2024, 3:28 p.m. UTC
The stride is not correctly assigned when copying pixel data, causing
images being displayed incomplete when using 2d component of rutabaga.

Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
---
 hw/display/virtio-gpu-rutabaga.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Marc-André Lureau June 10, 2024, 7:44 a.m. UTC | #1
Hi

On Wed, Jun 5, 2024 at 7:30 PM Weifeng Liu <weifeng.liu.z@gmail.com> wrote:

> The stride is not correctly assigned when copying pixel data, causing
> images being displayed incomplete when using 2d component of rutabaga.
>
> Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> ---
>  hw/display/virtio-gpu-rutabaga.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/display/virtio-gpu-rutabaga.c
> b/hw/display/virtio-gpu-rutabaga.c
> index 17bf701a21..2ba6869606 100644
> --- a/hw/display/virtio-gpu-rutabaga.c
> +++ b/hw/display/virtio-gpu-rutabaga.c
> @@ -53,6 +53,7 @@ virtio_gpu_rutabaga_update_cursor(VirtIOGPU *g, struct
> virtio_gpu_scanout *s,
>      transfer.z = 0;
>      transfer.w = res->width;
>      transfer.h = res->height;
> +    transfer.stride = res->width * 4;
>

ok, stride defined by QEMUCursor layout


>      transfer.d = 1;
>
>      transfer_iovec.iov_base = s->current_cursor->data;
> @@ -273,6 +274,7 @@ rutabaga_cmd_resource_flush(VirtIOGPU *g, struct
> virtio_gpu_ctrl_command *cmd)
>      transfer.z = 0;
>      transfer.w = res->width;
>      transfer.h = res->height;
> +    transfer.stride = pixman_image_get_stride(res->image);
>      transfer.d = 1;
>

ok (destination image stride)


>      transfer_iovec.iov_base = pixman_image_get_data(res->image);
> @@ -382,6 +384,7 @@ rutabaga_cmd_transfer_to_host_2d(VirtIOGPU *g,
>      transfer.z = 0;
>      transfer.w = t2d.r.width;
>      transfer.h = t2d.r.height;
> +    transfer.stride = t2d.r.width * 4;
>

here however, it's unclear to me what the stride could be, I think it could
depend on resource format (virgl doesn't set stride either).

Gurchetan?



>      transfer.d = 1;
>
>      result = rutabaga_resource_transfer_write(vr->rutabaga, 0,
> t2d.resource_id,
> --
> 2.45.0
>
>
>
Gurchetan Singh June 14, 2024, 11:50 p.m. UTC | #2
On Mon, Jun 10, 2024 at 12:44 AM Marc-André Lureau <
marcandre.lureau@gmail.com> wrote:

> Hi
>
> On Wed, Jun 5, 2024 at 7:30 PM Weifeng Liu <weifeng.liu.z@gmail.com>
> wrote:
>
>> The stride is not correctly assigned when copying pixel data, causing
>> images being displayed incomplete when using 2d component of rutabaga.
>>
>> Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
>> ---
>>  hw/display/virtio-gpu-rutabaga.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/display/virtio-gpu-rutabaga.c
>> b/hw/display/virtio-gpu-rutabaga.c
>> index 17bf701a21..2ba6869606 100644
>> --- a/hw/display/virtio-gpu-rutabaga.c
>> +++ b/hw/display/virtio-gpu-rutabaga.c
>> @@ -53,6 +53,7 @@ virtio_gpu_rutabaga_update_cursor(VirtIOGPU *g, struct
>> virtio_gpu_scanout *s,
>>      transfer.z = 0;
>>      transfer.w = res->width;
>>      transfer.h = res->height;
>> +    transfer.stride = res->width * 4;
>>
>
> ok, stride defined by QEMUCursor layout
>
>
>>      transfer.d = 1;
>>
>>      transfer_iovec.iov_base = s->current_cursor->data;
>> @@ -273,6 +274,7 @@ rutabaga_cmd_resource_flush(VirtIOGPU *g, struct
>> virtio_gpu_ctrl_command *cmd)
>>      transfer.z = 0;
>>      transfer.w = res->width;
>>      transfer.h = res->height;
>> +    transfer.stride = pixman_image_get_stride(res->image);
>>      transfer.d = 1;
>>
>
> ok (destination image stride)
>
>
>>      transfer_iovec.iov_base = pixman_image_get_data(res->image);
>> @@ -382,6 +384,7 @@ rutabaga_cmd_transfer_to_host_2d(VirtIOGPU *g,
>>      transfer.z = 0;
>>      transfer.w = t2d.r.width;
>>      transfer.h = t2d.r.height;
>> +    transfer.stride = t2d.r.width * 4;
>>
>
> here however, it's unclear to me what the stride could be, I think it
> could depend on resource format (virgl doesn't set stride either).
>
> Gurchetan?
>

gfxstream does more or less the same thing internally to deal with the lack
of stride.  Since virtio-gpu KMS only supports 4-bpp formats, this is fine,
so r-b for this particular patch.

That said, we actually don't use virtio-gpu-rutabaga for 2D mode or VirGL
mode, preferring to use the other more supported modes in QEMU for those
use cases.  I think there's actually some CI support for those in many
places.  So suggest just waiting until virglrenderer uprev lands in QEMU.



>
>
>
>>      transfer.d = 1;
>>
>>      result = rutabaga_resource_transfer_write(vr->rutabaga, 0,
>> t2d.resource_id,
>> --
>> 2.45.0
>>
>>
>>
>
> --
> Marc-André Lureau
>
diff mbox series

Patch

diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 17bf701a21..2ba6869606 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -53,6 +53,7 @@  virtio_gpu_rutabaga_update_cursor(VirtIOGPU *g, struct virtio_gpu_scanout *s,
     transfer.z = 0;
     transfer.w = res->width;
     transfer.h = res->height;
+    transfer.stride = res->width * 4;
     transfer.d = 1;
 
     transfer_iovec.iov_base = s->current_cursor->data;
@@ -273,6 +274,7 @@  rutabaga_cmd_resource_flush(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd)
     transfer.z = 0;
     transfer.w = res->width;
     transfer.h = res->height;
+    transfer.stride = pixman_image_get_stride(res->image);
     transfer.d = 1;
 
     transfer_iovec.iov_base = pixman_image_get_data(res->image);
@@ -382,6 +384,7 @@  rutabaga_cmd_transfer_to_host_2d(VirtIOGPU *g,
     transfer.z = 0;
     transfer.w = t2d.r.width;
     transfer.h = t2d.r.height;
+    transfer.stride = t2d.r.width * 4;
     transfer.d = 1;
 
     result = rutabaga_resource_transfer_write(vr->rutabaga, 0, t2d.resource_id,