From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57585) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YRLmW-0001o5-08 for qemu-devel@nongnu.org; Fri, 27 Feb 2015 09:20:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YRLmS-0000kY-Gr for qemu-devel@nongnu.org; Fri, 27 Feb 2015 09:20:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39950) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YRLmS-0000kU-73 for qemu-devel@nongnu.org; Fri, 27 Feb 2015 09:20:40 -0500 Message-ID: <54F07D31.2000100@redhat.com> Date: Fri, 27 Feb 2015 09:20:33 -0500 From: Max Reitz MIME-Version: 1.0 References: <1424687012-18524-1-git-send-email-kraxel@redhat.com> <1424687012-18524-7-git-send-email-kraxel@redhat.com> <54EF44F1.1090507@redhat.com> <1425035430.20883.44.camel@nilsson.home.kraxel.org> In-Reply-To: <1425035430.20883.44.camel@nilsson.home.kraxel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RfC PATCH 06/15] virtio-gpu/2d: add virtio gpu core code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: Dave Airlie , qemu-devel@nongnu.org, Anthony Liguori , "Michael S. Tsirkin" On 2015-02-27 at 06:10, Gerd Hoffmann wrote: > On Do, 2015-02-26 at 11:08 -0500, Max Reitz wrote: >> On 2015-02-23 at 05:23, Gerd Hoffmann wrote: >>> This patch adds the core code for virtio gpu emulation, >>> covering 2d support. >>> >>> Written by Dave Airlie and Gerd Hoffmann. >>> >>> Signed-off-by: Dave Airlie >>> Signed-off-by: Gerd Hoffmann >>> --- >>> hw/display/Makefile.objs | 2 + >>> hw/display/virtio-gpu.c | 903 +++++++++++++++++++++++++++++++++++++++++ >>> include/hw/virtio/virtio-gpu.h | 147 +++++++ >>> trace-events | 14 + >>> 4 files changed, 1066 insertions(+) >>> create mode 100644 hw/display/virtio-gpu.c >>> create mode 100644 include/hw/virtio/virtio-gpu.h >> Again I mostly only have formal complaints... >> >> But one non-formal question: As far as I understood virtio-gpu's mode of >> operation from this patch, it looks like there is one resource per >> scanout, and that resource is basically the whole screen (which can be >> updated partially). > This is correct (for 2d mode, 3d will be different). > >> If that is the case, what do we gain from being able to display a >> resource on multiple scanouts? If we don't associate a scanout to a >> resource with set_scanout, the resource won't ever be displayed on that >> scanout; and if we associate it, the scanout's position and dimension >> will be exactly the same as the resource's, so associating a resource >> with multiple scanouts means that all those scanouts will be duplicates >> of each other, which in turn means we can duplicate heads. But that >> seems more like a frontend feature to me... > It's handled this way to mimic behavior of physical hardware, where you > can configure your scanouts (monitor plugs of gfx hardware) in a simliar > way. > > Main advantage of taking this route is that virtual hardware and > physical hardware can be configued the same way, i.e. you can -- for > example -- setup screen mirroring with xrandr. OK. [snip] >>> + 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); >>> + } >>> + } else { >>> + iov_to_buf(res->iov, res->iov_cnt, 0, >>> + pixman_image_get_data(res->image), >>> + pixman_image_get_stride(res->image) >>> + * pixman_image_get_height(res->image)); >> Will this work with stride != t2d.r.width * bpp? > Those cases should take the if branch above and loop over all lines to > handle it correctly. Exactly, but it looks like to me that it doesn't (at least not always). Max