[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: virtio-gpu: Clarification regarding VIRTIO_GPU_CMD_TRANSFER_TO_HOST_2D
Hello, I believe there is a bug in QEMU's implementation of VIRTIO_GPU_CMD_TRANSFER_TO_HOST_2D. However it is not clear to me what the correct behaviour should be. The specification says the driver must "allocate a framebuffer from guest ram, and attach it as backing storage to the resource just created". To me this implies that the size of the backing storage must match the size of the resource. Consequently I intuitively expect that to update a rectangle R in the resource the same rectangle must be updated in the backing store, i.e: +--------------------+ +--------------------+ | | | | | Host | | Backing | | Resource | | Storage | | +-----+ | | +-----+ | | | | | | | | | | | R | | | | R | | | | | | | | | | | +-----+ | | +-----+ | +--------------------+ +--------------------+ However, QEMU actually starts drawing from the upper-left corner of the backing storage: +--------------------+ +-----+--------------+ | | | | | | Host | | R | Backing | | Resource | | | Storage | | +-----+ | +-----+ | | | | | | | | | R | | | | | | | | | | | +-----+ | | | +--------------------+ +--------------------+ Reading the specification again I realized it does not explicitly mandate that the size of the backing storage must match the size of the resource. But if that were the case I would expect QEMU to behave like this: +--------------------+ +--------------------+ | | | R +-------+ | Host | +------------+ | | Resource | | | | +-----+ | | Backing | | | | | | Storage | | | R | | | | | | | | | | | +-----+ | | | +--------------------+ +--------------------+ The relevant code in QEMU is in virtio_gpu_transfer_to_host_2d: format = pixman_image_get_format(res->image); bpp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(format), 8); stride = pixman_image_get_stride(res->image); if (t2d.offset || t2d.r.x || t2d.r.y || t2d.r.width != pixman_image_get_width(res->image)) { void *img_data = pixman_image_get_data(res->image); for (h = 0; h < t2d.r.height; h++) { src_offset = t2d.offset + stride * h; dst_offset = (t2d.r.y + h) * stride + (t2d.r.x * bpp); iov_to_buf(res->iov, res->iov_cnt, src_offset, (uint8_t *)img_data + dst_offset, t2d.r.width * bpp); I would expect src_offset to either be src_offset = dst_offset; if the backing storage's size is supposed to match the resource's size. If not, I'd expect src_offset to be src_offset = t2d.offset + t2d.r.width * h; My question boils down to: - Is QEMU's implementation correct? - If yes, what is the motivation behind it? Especially, what is the purpose of the explicit offset? - If not, what is the expected behaviour? With kind regards, David Hoppenbrouwers
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]