Re: [PATCH] Add virtio gpu driver.

[Pekka Paalanen]

On Tue, 31 May 2016 08:29:20 +0200
Gerd Hoffmann  wrote:

>   Hi,
> 
> > > Why attach the hotspot to the plane?  Wouldn't it make more sense to
> > > make it a framebuffer property?  
> > 
> > We don't have properties on the framebuffer. I guess you /could/ just add
> > it internally to struct drm_framebuffer, and not bother exposing to
> > userspace. I guess that would be a lot simpler,  
> 
> Yes.  I can simply stick the hotspot into drm_framebuffer in
> drm_mode_cursor_universal() and pick up the values in the driver's plane
> update function.
> 
> >  but it also means that
> > atomic userspace can't use hotspots before we add properties to fbs. And
> > doing that is a bit tricky since drm_framebuffer objects are meant to be
> > invariant - this assumption is deeply in-grained into the code all over
> > the place, everything just compares pointers when semantically it means to
> > compare the entire fb (including backing storage pointer/offsets and
> > everything).  
> 
> Hmm, the hotspot location for a given cursor image is invariant too, so
> I don't think that would be a big issue.
> 
> I'd expect userspace define a bunch of cursors, then switch between them
> (instead of defining a single cursor, then constantly updating it).

Except updating a single cursor (well, two alternating buffers) is
exactly what Weston does, since there is no "set of cursors". On
Wayland, a cursor is just a regular surface like any other with
arbitrary content from a client, except it happens to be associated
with a pointer device.

Furthermore, in Weston a cursor plane is not special in any way. *Any*
client surface can go on the cursor plane if it fits. Universal planes,
and all that.

That's one existing userspace. I suppose that is sub-optimal for
virtual drivers, isn't it? But what else could Weston do without having
separate paths for "normal DRM" vs. "virtual DRM"?


Thanks,
pq


pgplEC03Kmkyf.pgp
Description: OpenPGP digital signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Daniel Vetter]

On Tue, May 31, 2016 at 8:29 AM, Gerd Hoffmann  wrote:
>>  but it also means that
>> atomic userspace can't use hotspots before we add properties to fbs. And
>> doing that is a bit tricky since drm_framebuffer objects are meant to be
>> invariant - this assumption is deeply in-grained into the code all over
>> the place, everything just compares pointers when semantically it means to
>> compare the entire fb (including backing storage pointer/offsets and
>> everything).
>
> Hmm, the hotspot location for a given cursor image is invariant too, so
> I don't think that would be a big issue.
>
> I'd expect userspace define a bunch of cursors, then switch between them
> (instead of defining a single cursor, then constantly updating it).

Agreed, conceptually it would be a really nice fit to put the hotspot
into the invariant drm_framebuffer. I just meant to say that you need
to add a bit of new uapi to make it happen, since we can't reuse our
existing get/set_prop functions (since those only change props after
the object is created). And we also can't use our existing ioctls to
enumerate properties of an object (since chicken-egg: you want the
list of properties before creating a new framebuffer, so can't use
that framebuffer to enumerate them).

But really not a big concern.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

  Hi,

> > Why attach the hotspot to the plane?  Wouldn't it make more sense to
> > make it a framebuffer property?
> 
> We don't have properties on the framebuffer. I guess you /could/ just add
> it internally to struct drm_framebuffer, and not bother exposing to
> userspace. I guess that would be a lot simpler,

Yes.  I can simply stick the hotspot into drm_framebuffer in
drm_mode_cursor_universal() and pick up the values in the driver's plane
update function.

>  but it also means that
> atomic userspace can't use hotspots before we add properties to fbs. And
> doing that is a bit tricky since drm_framebuffer objects are meant to be
> invariant - this assumption is deeply in-grained into the code all over
> the place, everything just compares pointers when semantically it means to
> compare the entire fb (including backing storage pointer/offsets and
> everything).

Hmm, the hotspot location for a given cursor image is invariant too, so
I don't think that would be a big issue.

I'd expect userspace define a bunch of cursors, then switch between them
(instead of defining a single cursor, then constantly updating it).

cheers,
  Gerd

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

  Hi,

> - add a small core function to registerr HOT_X/HOT_Y for a (cursor) plane,
>   e.g. drm_plane_register_hotspot(). That should allocate the properties
>   (if they don't exist yet) and then attach those props to the cursor. We
>   don't want those props everywhere, but only on drivers that support/need
>   them, aka virtual hw.

Hmm, why is this special to virtual hw?

>   if (crtc->cursor) {
> - ret = drm_mode_cursor_universal(crtc, req, file_priv);
> + if (drm_core_check_feature(DRIVER_ATOMIC))
> + ret = drm_mode_cursor_atomic(crtc, req, file_priv);
> + else
> + ret = drm_mode_cursor_universal(crtc, req, file_priv);
>   goto out;

>   drm_mode_cursor_atomic would simply be a fusing of
>   drm_mode_cursor_universal + drm_atomic_helper_update_plane (dump all the
>   intermediate variables and store directly in the plane state), with the
>   addition of also storing hot_x/y into the plane state.

Hmm, that'll either make drm_mode_cursor_atomic a big cut+pasted
function, or need quite some refactoring to move common code into
functions callable from both drm_mode_cursor_atomic
+drm_mode_cursor_universal ...

Why attach the hotspot to the plane?  Wouldn't it make more sense to
make it a framebuffer property?

cheers,
  Gerd

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Daniel Vetter]

On Fri, May 27, 2016 at 09:48:22AM +0200, Gerd Hoffmann wrote:
> > I guess I didn't do a good job at looking at your v2: Cursor is still
> > using legacy interfaces and not a proper plane. Would be awesome if
> > you could fix that up. Atomic drivers really shouldn't use the legacy
> > cursor interfaces any more at all.
> > -Daniel
> 
> Figured that one for the most part, see attached draft.
> 
> The only thing I'm wondering is how the hotspot is handled.
> drm_mode_cursor_universal doesn't even look at req->hot_{x,y}.
> 
> /me looks confused.

No need to, you're simply the first virtual driver to wire up atomic
cursors. Hence some gaps to be filled out.

First we need to wire up the state tracking scaffolding for atomic:
- add hot_x/y to drm_plane_state
- add property pointers for hot_x/y to dev->mode_config (like we have for
  all the other atomic props like "SRC_X").
- add encode/decode support for these properties to
  drm_atomic_plane_get_property and drm_atomic_plane_set_property, similar
  again to "SRC_X" and friends
- add a small core function to registerr HOT_X/HOT_Y for a (cursor) plane,
  e.g. drm_plane_register_hotspot(). That should allocate the properties
  (if they don't exist yet) and then attach those props to the cursor. We
  don't want those props everywhere, but only on drivers that support/need
  them, aka virtual hw.

With that a real atomic driver will be able to move the cursor and it's
hotspot around, all nicely done in an atomic commit. But it won't work yet
for userspace for legacy applications. For that we need a notch more:
- one option would be to add hot_x/hot_y to the ->update_plane hook, but
  that has massive trickling effects throughout the subsystem. Probably
  not what we want to do.
- 2nd option would be to add a DRIVER_ATOMIC check to
  drm_mode_cursor_common and call a new drm_mode_cursor_atomic in that
  case:

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 3e52a6ecf6c0..2f15ce2c6bf4 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3046,7 +3046,10 @@ static int drm_mode_cursor_common(struct drm_device *dev,
 */
drm_modeset_lock_crtc(crtc, crtc->cursor);
if (crtc->cursor) {
-   ret = drm_mode_cursor_universal(crtc, req, file_priv);
+   if (drm_core_check_feature(DRIVER_ATOMIC))
+   ret = drm_mode_cursor_atomic(crtc, req, file_priv);
+   else
+   ret = drm_mode_cursor_universal(crtc, req, file_priv);
goto out;
}
 
  drm_mode_cursor_atomic would simply be a fusing of
  drm_mode_cursor_universal + drm_atomic_helper_update_plane (dump all the
  intermediate variables and store directly in the plane state), with the
  addition of also storing hot_x/y into the plane state.

Sorry that this turned into a bit of a project, I've forgotten that we
haven't wired up hot_x/y at all for atomic ...

If you don't want to bother with the atomic properties (only needed for
atomic userspace), then just adding hot_x/y to drm_plane_state is all you
need from the first group of tasks.

Your patch below to implement the atomic cursor looks reasonable. Although
personally I'd go with a separate vfunc table for the cursor so that you
can avoid that ugly switch in the atomic_update hook.

Cheers, Daniel

> 
> cheers,
>   Gerd
> 

> From fb1d0700a46d850ec9f931304a9e99854a3ce5e9 Mon Sep 17 00:00:00 2001
> From: Gerd Hoffmann 
> Date: Thu, 26 May 2016 11:42:52 +0200
> Subject: [PATCH] [wip] virtio-gpu: switch to atomic cursor interfaces
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  drivers/gpu/drm/virtio/virtgpu_display.c | 102 +++---
>  drivers/gpu/drm/virtio/virtgpu_drv.h |   1 +
>  drivers/gpu/drm/virtio/virtgpu_plane.c   | 122 
> ---
>  3 files changed, 109 insertions(+), 116 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c 
> b/drivers/gpu/drm/virtio/virtgpu_display.c
> index 5990cab..d6b16d1 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> @@ -29,8 +29,8 @@
>  #include 
>  #include 
>  
> -#define XRES_MIN   320
> -#define YRES_MIN   200
> +#define XRES_MIN32
> +#define YRES_MIN32
>  
>  #define XRES_DEF  1024
>  #define YRES_DEF   768
> @@ -38,86 +38,6 @@
>  #define XRES_MAX  8192
>  #define YRES_MAX  8192
>  
> -static void
> -virtio_gpu_hide_cursor(struct virtio_gpu_device *vgdev,
> -struct virtio_gpu_output *output)
> -{
> - output->cursor.hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_UPDATE_CURSOR);
> - output->cursor.resource_id = 0;
> - virtio_gpu_cursor_ping(vgdev, output);
> -}
> -
> -static int virtio_gpu_crtc_cursor_set(struct drm_crtc *crtc,
> -   struct drm_file *file_priv,
> -   uint32_t handle,
> -   

Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

  Hi,

> I guess I didn't do a good job at looking at your v2: Cursor is still
> using legacy interfaces and not a proper plane. Would be awesome if
> you could fix that up. Atomic drivers really shouldn't use the legacy
> cursor interfaces any more at all.
> -Daniel

Figured that one for the most part, see attached draft.

The only thing I'm wondering is how the hotspot is handled.
drm_mode_cursor_universal doesn't even look at req->hot_{x,y}.

/me looks confused.

cheers,
  Gerd

From fb1d0700a46d850ec9f931304a9e99854a3ce5e9 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann 
Date: Thu, 26 May 2016 11:42:52 +0200
Subject: [PATCH] [wip] virtio-gpu: switch to atomic cursor interfaces

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/virtio/virtgpu_display.c | 102 +++---
 drivers/gpu/drm/virtio/virtgpu_drv.h |   1 +
 drivers/gpu/drm/virtio/virtgpu_plane.c   | 122 ---
 3 files changed, 109 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index 5990cab..d6b16d1 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -29,8 +29,8 @@
 #include 
 #include 
 
-#define XRES_MIN   320
-#define YRES_MIN   200
+#define XRES_MIN32
+#define YRES_MIN32
 
 #define XRES_DEF  1024
 #define YRES_DEF   768
@@ -38,86 +38,6 @@
 #define XRES_MAX  8192
 #define YRES_MAX  8192
 
-static void
-virtio_gpu_hide_cursor(struct virtio_gpu_device *vgdev,
-		   struct virtio_gpu_output *output)
-{
-	output->cursor.hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_UPDATE_CURSOR);
-	output->cursor.resource_id = 0;
-	virtio_gpu_cursor_ping(vgdev, output);
-}
-
-static int virtio_gpu_crtc_cursor_set(struct drm_crtc *crtc,
-  struct drm_file *file_priv,
-  uint32_t handle,
-  uint32_t width,
-  uint32_t height,
-  int32_t hot_x, int32_t hot_y)
-{
-	struct virtio_gpu_device *vgdev = crtc->dev->dev_private;
-	struct virtio_gpu_output *output =
-		container_of(crtc, struct virtio_gpu_output, crtc);
-	struct drm_gem_object *gobj = NULL;
-	struct virtio_gpu_object *qobj = NULL;
-	struct virtio_gpu_fence *fence = NULL;
-	int ret = 0;
-
-	if (handle == 0) {
-		virtio_gpu_hide_cursor(vgdev, output);
-		return 0;
-	}
-
-	/* lookup the cursor */
-	gobj = drm_gem_object_lookup(crtc->dev, file_priv, handle);
-	if (gobj == NULL)
-		return -ENOENT;
-
-	qobj = gem_to_virtio_gpu_obj(gobj);
-
-	if (!qobj->hw_res_handle) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	virtio_gpu_cmd_transfer_to_host_2d(vgdev, qobj->hw_res_handle, 0,
-	   cpu_to_le32(64),
-	   cpu_to_le32(64),
-	   0, 0, );
-	ret = virtio_gpu_object_reserve(qobj, false);
-	if (!ret) {
-		reservation_object_add_excl_fence(qobj->tbo.resv,
-		  >f);
-		fence_put(>f);
-		virtio_gpu_object_unreserve(qobj);
-		virtio_gpu_object_wait(qobj, false);
-	}
-
-	output->cursor.hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_UPDATE_CURSOR);
-	output->cursor.resource_id = cpu_to_le32(qobj->hw_res_handle);
-	output->cursor.hot_x = cpu_to_le32(hot_x);
-	output->cursor.hot_y = cpu_to_le32(hot_y);
-	virtio_gpu_cursor_ping(vgdev, output);
-	ret = 0;
-
-out:
-	drm_gem_object_unreference_unlocked(gobj);
-	return ret;
-}
-
-static int virtio_gpu_crtc_cursor_move(struct drm_crtc *crtc,
-int x, int y)
-{
-	struct virtio_gpu_device *vgdev = crtc->dev->dev_private;
-	struct virtio_gpu_output *output =
-		container_of(crtc, struct virtio_gpu_output, crtc);
-
-	output->cursor.hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_MOVE_CURSOR);
-	output->cursor.pos.x = cpu_to_le32(x);
-	output->cursor.pos.y = cpu_to_le32(y);
-	virtio_gpu_cursor_ping(vgdev, output);
-	return 0;
-}
-
 static int virtio_gpu_page_flip(struct drm_crtc *crtc,
 struct drm_framebuffer *fb,
 struct drm_pending_vblank_event *event,
@@ -164,8 +84,6 @@ static int virtio_gpu_page_flip(struct drm_crtc *crtc,
 }
 
 static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
-	.cursor_set2= virtio_gpu_crtc_cursor_set,
-	.cursor_move= virtio_gpu_crtc_cursor_move,
 	.set_config = drm_atomic_helper_set_config,
 	.destroy= drm_crtc_cleanup,
 
@@ -406,7 +324,7 @@ static int vgdev_output_init(struct virtio_gpu_device *vgdev, int index)
 	struct drm_connector *connector = >conn;
 	struct drm_encoder *encoder = >enc;
 	struct drm_crtc *crtc = >crtc;
-	struct drm_plane *plane;
+	struct drm_plane *primary, *cursor;
 
 	output->index = index;
 	if (index == 0) {
@@ -415,14 +333,18 @@ static int vgdev_output_init(struct virtio_gpu_device *vgdev, int index)
 		output->info.r.height = cpu_to_le32(YRES_DEF);
 	}
 
-	plane = virtio_gpu_plane_init(vgdev, index);
-	if (IS_ERR(plane))
-		return PTR_ERR(plane);
-	drm_crtc_init_with_planes(dev, crtc, plane, NULL,
+	primary = virtio_gpu_plane_init(vgdev, DRM_PLANE_TYPE_PRIMARY, index);
+	if (IS_ERR(primary))
+		return 

Re: [PATCH] Add virtio gpu driver.

[Emil Velikov]

On 25 May 2016 at 17:40, Daniel Vetter  wrote:
> On Mon, Mar 30, 2015 at 4:49 PM, Daniel Vetter  wrote:
>> On Mon, Mar 30, 2015 at 02:23:47PM +0200, Gerd Hoffmann wrote:
>>> > > Signed-off-by: Dave Airlie 
>>> > > Signed-off-by: Gerd Hoffmann 
>>> >
>>> > Standard request from my side for new drm drivers (especially if they're
>>> > this simple): Can you please update the drivers to latest drm internal
>>> > interfaces, i.e. using universal planes and atomic?
>>>
>>> Up'n'running.  Incremental patch:
>>>
>>> https://www.kraxel.org/cgit/linux/commit/?h=virtio-gpu-2d=b8edf4f38a1ec5a50f6ac8948521a12f862d3d5a
>>>
>>> v2 coming, but I'll go over the other reviews first.
>>
>> Looking good. Wrt pageflip the current MO is to handroll it in your
>> driver, common approach is to use the msm async commit implementation
>> msm_atomic_commit. The issue is simply that right now there's still no
>> useable generic vblank callback support (drm_irq.c is a mess) hence why
>> the core helpers don't support async flips yet.
>
> I guess I didn't do a good job at looking at your v2: Cursor is still
> using legacy interfaces and not a proper plane. Would be awesome if
> you could fix that up. Atomic drivers really shouldn't use the legacy
> cursor interfaces any more at all.

Wild idea:
Worth adding if (drm_core_check_feature(dev, DRIVER_ATOMIC)  {
printf("abort abort"); return; }
style of checks for the legacy (preatomic) kms helpers ?

Or does it feel like an overkill ?

-Emil
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Daniel Vetter]

On Mon, Mar 30, 2015 at 4:49 PM, Daniel Vetter  wrote:
> On Mon, Mar 30, 2015 at 02:23:47PM +0200, Gerd Hoffmann wrote:
>> > > Signed-off-by: Dave Airlie 
>> > > Signed-off-by: Gerd Hoffmann 
>> >
>> > Standard request from my side for new drm drivers (especially if they're
>> > this simple): Can you please update the drivers to latest drm internal
>> > interfaces, i.e. using universal planes and atomic?
>>
>> Up'n'running.  Incremental patch:
>>
>> https://www.kraxel.org/cgit/linux/commit/?h=virtio-gpu-2d=b8edf4f38a1ec5a50f6ac8948521a12f862d3d5a
>>
>> v2 coming, but I'll go over the other reviews first.
>
> Looking good. Wrt pageflip the current MO is to handroll it in your
> driver, common approach is to use the msm async commit implementation
> msm_atomic_commit. The issue is simply that right now there's still no
> useable generic vblank callback support (drm_irq.c is a mess) hence why
> the core helpers don't support async flips yet.

I guess I didn't do a good job at looking at your v2: Cursor is still
using legacy interfaces and not a proper plane. Would be awesome if
you could fix that up. Atomic drivers really shouldn't use the legacy
cursor interfaces any more at all.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

  Hi,

  Signed-off-by: Dave Airlie airl...@redhat.com
  Signed-off-by: Gerd Hoffmann kra...@redhat.com
 
 Standard request from my side for new drm drivers (especially if they're
 this simple): Can you please update the drivers to latest drm internal
 interfaces, i.e. using universal planes and atomic?

Up'n'running.  Incremental patch:

https://www.kraxel.org/cgit/linux/commit/?h=virtio-gpu-2did=b8edf4f38a1ec5a50f6ac8948521a12f862d3d5a

v2 coming, but I'll go over the other reviews first.

cheers,
  Gerd


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Daniel Vetter]

On Mon, Mar 30, 2015 at 02:23:47PM +0200, Gerd Hoffmann wrote:
   Hi,
 
   Signed-off-by: Dave Airlie airl...@redhat.com
   Signed-off-by: Gerd Hoffmann kra...@redhat.com
  
  Standard request from my side for new drm drivers (especially if they're
  this simple): Can you please update the drivers to latest drm internal
  interfaces, i.e. using universal planes and atomic?
 
 Up'n'running.  Incremental patch:
 
 https://www.kraxel.org/cgit/linux/commit/?h=virtio-gpu-2did=b8edf4f38a1ec5a50f6ac8948521a12f862d3d5a
 
 v2 coming, but I'll go over the other reviews first.

Looking good. Wrt pageflip the current MO is to handroll it in your
driver, common approach is to use the msm async commit implementation
msm_atomic_commit. The issue is simply that right now there's still no
useable generic vblank callback support (drm_irq.c is a mess) hence why
the core helpers don't support async flips yet.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.

[One Thousand Gnomes]

 It's not about virtio at all.  It's about vga compatibility, so we have
 a simple framebuffer as boot display.  Only used when virtio is *not*
 enabled.

VGA can be a separate device altogether.

In fact there were *real* PCI graphics cards that did this and had a
register than flipped the output source over.

Alan
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

  Hi,

  
  Completely different thing crossing my mind:  I think we can make
  virtio-vga fully compatible with stdvga.  stdvga has two bars, memory
  (#0) and mmio (#2).  We can make the mmio bar larger and place all the
  virtio regions there.
  
 
 Full compatibility with some standard sounds like a better motivation,
 yes.

Ok, I'll look into it.

  I think in any case I'll go split off the vga compatibility bits to a
  different patch (and possible a separate patch series).
  
  cheers,
Gerd
 
 Will you still need me to change core to claim specific memory only?

That would be great, yes.  The resource conflict with vesafb/efifb will
stay no matter how we design the pci resource layout of virtio-vga.

cheers,
  Gerd



___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

  Hi,

 And is it possible to use offset within BAR and/or memory BARs?
 If yes I'd strongly prefer this.

What is the point?  Do you want place virtio regions and vga framebuffer
in the same pci bar?  Why?  virtio is mmio and traps into qemu on
access, whereas the vga framebuffer is memory-backed (with dirty
tracking turned on).  Don't think this is a good idea, even though the
memory api would probably allow to do this.

cheers,
  Gerd


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Michael S. Tsirkin]

On Thu, Mar 26, 2015 at 08:12:39AM +0100, Gerd Hoffmann wrote:
 On Mi, 2015-03-25 at 18:09 +0100, Michael S. Tsirkin wrote:
  On Wed, Mar 25, 2015 at 04:37:16PM +0100, Gerd Hoffmann wrote:
 Hi,
   
BTW can we teach virtio-gpu to look for framebuffer using
virtio pci caps?
   
   The virtio-gpu driver doesn't matter much here, it doesn't use it
   anyway.
   
 Or are there limitations such as only
using IO port BARs, or compatibility with
BIOS code etc that limit us to specific BARs anyway?
   
   Yes, vgabios code needs to know.  Currently it has bar #2 for the vga
   framebuffer bar hardcoded.  It's 16bit code.  I don't feel like making
   the probing more complicated ...
   
   cheers,
 Gerd
  
  OK - you are saying all VGA cards use bar #2 for this
  functionality, so we are just following
  established practice here?
 
 vgabios checks pci ids to figure.  qxl+stdvga use bar #0, vmware-vga bar
 #1, virtio-vga bar #2.
 
 cheers,
   Gerd
 

And is it possible to use offset within BAR and/or memory BARs?
If yes I'd strongly prefer this.
As for writing 16 bit code, I need to do this for virtio scsi/blk
anyway, so we'll be able to share code.

-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.

[Michael S. Tsirkin]

On Thu, Mar 26, 2015 at 09:42:47AM +0100, Gerd Hoffmann wrote:
   Hi,
 
  And is it possible to use offset within BAR and/or memory BARs?
  If yes I'd strongly prefer this.
 
 What is the point?  Do you want place virtio regions and vga framebuffer
 in the same pci bar?  Why?  virtio is mmio and traps into qemu on
 access, whereas the vga framebuffer is memory-backed (with dirty
 tracking turned on).  Don't think this is a good idea, even though the
 memory api would probably allow to do this.
 
 cheers,
   Gerd

Absolutely, it's pretty common to mix regions in a BAR.
For example, we have virtio kick (ioeventfd backed,
handled in kernel) in same BAR as common and device
specific configuration.

We did the same thing you are now doing with the
virtio BAR, and now we have to maintain two code
bases, virtio pci config was designed to be future proof
so why not use it?

This is mostly just making sure we don't paint ourselves into a corner.

-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.

[Michael S. Tsirkin]

On Thu, Mar 26, 2015 at 12:38:43PM +0100, Gerd Hoffmann wrote:
   Hi,
 
  Absolutely, it's pretty common to mix regions in a BAR.
  For example, we have virtio kick (ioeventfd backed,
  handled in kernel) in same BAR as common and device
  specific configuration.
 
  We did the same thing you are now doing with the
  virtio BAR, and now we have to maintain two code
  bases, virtio pci config was designed to be future proof
  so why not use it?
 
 It's not about virtio at all.  It's about vga compatibility, so we have
 a simple framebuffer as boot display.  Only used when virtio is *not*
 enabled.
 

I don't know. This seems exactly like the kind of thing
we had in mind when we added the virtio pci capability.
For example, we have text in spec that requires drivers
to skip unknown capabilities.

And yes, if bios pokes at a specific bar then we do
need to list this info in the virtio spec so this makes
it an issue that is virtio related.


  This is mostly just making sure we don't paint ourselves into a corner.
 
 It's a simple memory bar.  vga cards have that since pci was invented
 (standalone ones, chipset graphics aside), and there havn't been
 fundamental changes ...
 
 cheers,
   Gerd
 

Yes, it's not about what we put there now. It's about being able
to move things about in the future without breaking guests.

-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

  Hi,

 Absolutely, it's pretty common to mix regions in a BAR.
 For example, we have virtio kick (ioeventfd backed,
 handled in kernel) in same BAR as common and device
 specific configuration.

 We did the same thing you are now doing with the
 virtio BAR, and now we have to maintain two code
 bases, virtio pci config was designed to be future proof
 so why not use it?

It's not about virtio at all.  It's about vga compatibility, so we have
a simple framebuffer as boot display.  Only used when virtio is *not*
enabled.

 This is mostly just making sure we don't paint ourselves into a corner.

It's a simple memory bar.  vga cards have that since pci was invented
(standalone ones, chipset graphics aside), and there havn't been
fundamental changes ...

cheers,
  Gerd


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Daniel Vetter]

On Wed, Mar 25, 2015 at 03:53:09PM +0100, Gerd Hoffmann wrote:
   Signed-off-by: Dave Airlie airl...@redhat.com
   Signed-off-by: Gerd Hoffmann kra...@redhat.com
  
  Standard request from my side for new drm drivers (especially if they're
  this simple): Can you please update the drivers to latest drm internal
  interfaces, i.e. using universal planes and atomic?
 
 Have a docs / sample code pointer for me?

Picking any of the recently converted drivers or recently merged atomic
drivers should be fairly informative. Overall conversion procedure is
detailed in

http://blog.ffwll.ch/2014/11/atomic-modeset-support-for-kms-drivers.html
http://blog.ffwll.ch/2015/01/update-for-atomic-display-updates.html

And ofc there's the kerneldoc stuff in the drm docbook. If you have
questions probably best to ask them in #dri-devel irc, most of the people
who've done atomic conversions hang out there and can help out.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.

[Michael S. Tsirkin]

On Thu, Mar 26, 2015 at 04:07:16PM +0100, Gerd Hoffmann wrote:
   Hi,
 
  I don't know. This seems exactly like the kind of thing
  we had in mind when we added the virtio pci capability.
  For example, we have text in spec that requires drivers
  to skip unknown capabilities.
  
  And yes, if bios pokes at a specific bar then we do
  need to list this info in the virtio spec so this makes
  it an issue that is virtio related.
 
 Hmm, virtio-vga is a two-in-one device basically.  When virtio is
 enabled it behaves like virtio-gpu-pci, otherwise it behaves very
 simliar to stdvga.  So you need to know nothing about virtio to handle
 the vga side, and I want keep it that way.
 
 When no vga compatibility is needed there always is the option to just
 use virtio-gpu-pci instead.
 
  Yes, it's not about what we put there now. It's about being able
  to move things about in the future without breaking guests.
 
 We don't have that today for stdvga, and I still fail to see what this
 buys us.
 
 
 Completely different thing crossing my mind:  I think we can make
 virtio-vga fully compatible with stdvga.  stdvga has two bars, memory
 (#0) and mmio (#2).  We can make the mmio bar larger and place all the
 virtio regions there.
 

Full compatibility with some standard sounds like a better motivation,
yes.

 
 I think in any case I'll go split off the vga compatibility bits to a
 different patch (and possible a separate patch series).
 
 cheers,
   Gerd

Will you still need me to change core to claim specific memory only?


-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

On Mi, 2015-03-25 at 18:09 +0100, Michael S. Tsirkin wrote:
 On Wed, Mar 25, 2015 at 04:37:16PM +0100, Gerd Hoffmann wrote:
Hi,
  
   BTW can we teach virtio-gpu to look for framebuffer using
   virtio pci caps?
  
  The virtio-gpu driver doesn't matter much here, it doesn't use it
  anyway.
  
Or are there limitations such as only
   using IO port BARs, or compatibility with
   BIOS code etc that limit us to specific BARs anyway?
  
  Yes, vgabios code needs to know.  Currently it has bar #2 for the vga
  framebuffer bar hardcoded.  It's 16bit code.  I don't feel like making
  the probing more complicated ...
  
  cheers,
Gerd
 
 OK - you are saying all VGA cards use bar #2 for this
 functionality, so we are just following
 established practice here?

vgabios checks pci ids to figure.  qxl+stdvga use bar #0, vmware-vga bar
#1, virtio-vga bar #2.

cheers,
  Gerd


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [virtio-dev] Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

  Hi,

 I don't know. This seems exactly like the kind of thing
 we had in mind when we added the virtio pci capability.
 For example, we have text in spec that requires drivers
 to skip unknown capabilities.
 
 And yes, if bios pokes at a specific bar then we do
 need to list this info in the virtio spec so this makes
 it an issue that is virtio related.

Hmm, virtio-vga is a two-in-one device basically.  When virtio is
enabled it behaves like virtio-gpu-pci, otherwise it behaves very
simliar to stdvga.  So you need to know nothing about virtio to handle
the vga side, and I want keep it that way.

When no vga compatibility is needed there always is the option to just
use virtio-gpu-pci instead.

 Yes, it's not about what we put there now. It's about being able
 to move things about in the future without breaking guests.

We don't have that today for stdvga, and I still fail to see what this
buys us.


Completely different thing crossing my mind:  I think we can make
virtio-vga fully compatible with stdvga.  stdvga has two bars, memory
(#0) and mmio (#2).  We can make the mmio bar larger and place all the
virtio regions there.


I think in any case I'll go split off the vga compatibility bits to a
different patch (and possible a separate patch series).

cheers,
  Gerd


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Michael S. Tsirkin]

On Wed, Mar 25, 2015 at 03:52:01PM +0100, Gerd Hoffmann wrote:
   Hi,
 
   diff --git a/drivers/virtio/virtio_pci_common.c 
   b/drivers/virtio/virtio_pci_common.c
   index e894eb2..a3167fa 100644
   --- a/drivers/virtio/virtio_pci_common.c
   +++ b/drivers/virtio/virtio_pci_common.c
   @@ -510,7 +510,7 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
 goto err_enable_device;

 rc = pci_request_regions(pci_dev, virtio-pci);
   - if (rc)
   + if (rc  ((pci_dev-class  8) != PCI_CLASS_DISPLAY_VGA))
 goto err_request_regions;

 if (force_legacy) {
  
  This is probably what you described as the only concern?
 
 Ahem, no, forgot that one,

What does the concern refer to then?

 but it is related.  With vesafb using and
 registering the vga compat framebuffer bar pci_request_regions will not
 succeed.
 
 vesafb will be unregistered later on (this is what I was refering to) by
 the virtio-gpu driver.
 
  If we only need to request specific
  regions, I think we should do exactly that, requesting only parts of
  regions that are covered by the virtio capabilities.
 
 That should work too.
 
 cheers,
   Gerd

BTW can we teach virtio-gpu to look for framebuffer using
virtio pci caps? Or are there limitations such as only
using IO port BARs, or compatibility with
BIOS code etc that limit us to specific BARs anyway?

-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

  Hi,

  diff --git a/drivers/virtio/virtio_pci_common.c 
  b/drivers/virtio/virtio_pci_common.c
  index e894eb2..a3167fa 100644
  --- a/drivers/virtio/virtio_pci_common.c
  +++ b/drivers/virtio/virtio_pci_common.c
  @@ -510,7 +510,7 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
  goto err_enable_device;
   
  rc = pci_request_regions(pci_dev, virtio-pci);
  -   if (rc)
  +   if (rc  ((pci_dev-class  8) != PCI_CLASS_DISPLAY_VGA))
  goto err_request_regions;
   
  if (force_legacy) {
 
 This is probably what you described as the only concern?

Ahem, no, forgot that one, but it is related.  With vesafb using and
registering the vga compat framebuffer bar pci_request_regions will not
succeed.

vesafb will be unregistered later on (this is what I was refering to) by
the virtio-gpu driver.

 If we only need to request specific
 regions, I think we should do exactly that, requesting only parts of
 regions that are covered by the virtio capabilities.

That should work too.

cheers,
  Gerd


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

  Hi,

 BTW can we teach virtio-gpu to look for framebuffer using
 virtio pci caps?

The virtio-gpu driver doesn't matter much here, it doesn't use it
anyway.

  Or are there limitations such as only
 using IO port BARs, or compatibility with
 BIOS code etc that limit us to specific BARs anyway?

Yes, vgabios code needs to know.  Currently it has bar #2 for the vga
framebuffer bar hardcoded.  It's 16bit code.  I don't feel like making
the probing more complicated ...

cheers,
  Gerd

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

  Signed-off-by: Dave Airlie airl...@redhat.com
  Signed-off-by: Gerd Hoffmann kra...@redhat.com
 
 Standard request from my side for new drm drivers (especially if they're
 this simple): Can you please update the drivers to latest drm internal
 interfaces, i.e. using universal planes and atomic?

Have a docs / sample code pointer for me?

thanks,
  Gerd


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

On Di, 2015-03-24 at 22:50 +, Daniel Stone wrote:
 Hi,
 
 On 24 March 2015 at 16:07, Gerd Hoffmann kra...@redhat.com wrote:
  +static int virtio_gpu_crtc_page_flip(struct drm_crtc *crtc,
  +struct drm_framebuffer *fb,
  +struct drm_pending_vblank_event *event,
  +uint32_t flags)
  +{
  +   return -EINVAL;
  +}
 
 I'm not going to lie, I was really hoping the 5th (?) GPU option for
 Qemu would support pageflipping.

As Dave already pointed out there is a WIP patch for that, it'll be
there.

While being at it:
 - bochsdrm (qemu -vga std driver) supports pageflip since 3.19.
 - cirrus is more or less a lost case, we mimic existing hardware
   from the 90ies here and it simply isn't up to todays needs for
   many reasons.  Just stop using it.
 - qxl -- hmm, not sure, there is this primary surface concept in
   the virtual hardware design, which doesn't mix very well with
   pageflip I suspect.

 Daniel's comment about conversion to
 atomic is relevant, but: do you have a mechanism which allows you to
 post updates (e.g. 'start displaying this buffer now please') that
 allows you to get events back when they have actually been displayed?

It's possible to fence the framebuffer update requests, so you'll be
notified when the update has reached the qemu ui code.  Typically the ui
code has queued the update at that point.  So with a local display (sdl,
gtk) showing up on the screen should be just a pageflip (on the host)
away.  With a remote display (vnc) it will take a little longer until
the user will actually see the update.

cheers,
  Gerd


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Daniel Stone]

Hi,

On 24 March 2015 at 16:07, Gerd Hoffmann kra...@redhat.com wrote:
 +static int virtio_gpu_crtc_page_flip(struct drm_crtc *crtc,
 +struct drm_framebuffer *fb,
 +struct drm_pending_vblank_event *event,
 +uint32_t flags)
 +{
 +   return -EINVAL;
 +}

I'm not going to lie, I was really hoping the 5th (?) GPU option for
Qemu would support pageflipping. Daniel's comment about conversion to
atomic is relevant, but: do you have a mechanism which allows you to
post updates (e.g. 'start displaying this buffer now please') that
allows you to get events back when they have actually been displayed?

Cheers,
Daniel
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Daniel Stone]

Hi,

On Wednesday, March 25, 2015, Dave Airlie airl...@gmail.com wrote:

 On 25 March 2015 at 08:50, Daniel Stone dan...@fooishbar.org
 javascript:; wrote:
  I'm not going to lie, I was really hoping the 5th (?) GPU option for
  Qemu would support pageflipping. Daniel's comment about conversion to
  atomic is relevant, but: do you have a mechanism which allows you to
  post updates (e.g. 'start displaying this buffer now please') that
  allows you to get events back when they have actually been displayed?

 Page flip is implemented in a later patch,


 https://www.kraxel.org/cgit/linux/commit/?h=virtio-gpuid=1e167e8e964f8e08100d315dd354cc0a4b090841

 Since its a long way from an actual display, finding out when
 something is actually displayed is hard,
 but when we've posted it to the frontbuffer should be fine.


Oh nice. 100% exact timings aren't critical; it's more just having
something to gate to 60fps that we can actually drive Weston's repaint loop
off, as that's (not unreasonably) driven by pageflip events.

Cheers,
Daniel
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Michael S. Tsirkin]

On Wed, Mar 25, 2015 at 04:37:16PM +0100, Gerd Hoffmann wrote:
   Hi,
 
  BTW can we teach virtio-gpu to look for framebuffer using
  virtio pci caps?
 
 The virtio-gpu driver doesn't matter much here, it doesn't use it
 anyway.
 
   Or are there limitations such as only
  using IO port BARs, or compatibility with
  BIOS code etc that limit us to specific BARs anyway?
 
 Yes, vgabios code needs to know.  Currently it has bar #2 for the vga
 framebuffer bar hardcoded.  It's 16bit code.  I don't feel like making
 the probing more complicated ...
 
 cheers,
   Gerd

OK - you are saying all VGA cards use bar #2 for this
functionality, so we are just following
established practice here?

-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Dave Airlie]

On 25 March 2015 at 08:50, Daniel Stone dan...@fooishbar.org wrote:
 Hi,

 On 24 March 2015 at 16:07, Gerd Hoffmann kra...@redhat.com wrote:
 +static int virtio_gpu_crtc_page_flip(struct drm_crtc *crtc,
 +struct drm_framebuffer *fb,
 +struct drm_pending_vblank_event *event,
 +uint32_t flags)
 +{
 +   return -EINVAL;
 +}

 I'm not going to lie, I was really hoping the 5th (?) GPU option for
 Qemu would support pageflipping. Daniel's comment about conversion to
 atomic is relevant, but: do you have a mechanism which allows you to
 post updates (e.g. 'start displaying this buffer now please') that
 allows you to get events back when they have actually been displayed?

Page flip is implemented in a later patch,

https://www.kraxel.org/cgit/linux/commit/?h=virtio-gpuid=1e167e8e964f8e08100d315dd354cc0a4b090841

Since its a long way from an actual display, finding out when
something is actually displayed is hard,
but when we've posted it to the frontbuffer should be fine.

Dave.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH] Add virtio gpu driver.

[Gerd Hoffmann]

From: Dave Airlie airl...@gmail.com

This patch adds a kms driver for the virtio gpu.  The xorg modesetting
driver can handle the device just fine, the framebuffer for fbcon is
there too.

Qemu patches for the host side are under review currently.

The pci version of the device comes in two variants: with and without
vga compatibility.  The former has a extra memory bar for the vga
framebuffer, the later is a pure virtio device.  The only concern for
this driver is that in the virtio-vga case we have to kick out the
firmware framebuffer.

Initial revision has only 2d support, 3d (virgl) support requires
some more work on the qemu side and will be added later.

Signed-off-by: Dave Airlie airl...@redhat.com
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 drivers/gpu/drm/Kconfig  |   2 +
 drivers/gpu/drm/Makefile |   1 +
 drivers/gpu/drm/virtio/Kconfig   |  11 +
 drivers/gpu/drm/virtio/Makefile  |   9 +
 drivers/gpu/drm/virtio/virtgpu_debugfs.c |  64 
 drivers/gpu/drm/virtio/virtgpu_display.c | 527 ++
 drivers/gpu/drm/virtio/virtgpu_drm_bus.c |  68 
 drivers/gpu/drm/virtio/virtgpu_drv.c | 132 
 drivers/gpu/drm/virtio/virtgpu_drv.h | 326 +++
 drivers/gpu/drm/virtio/virtgpu_fb.c  | 415 
 drivers/gpu/drm/virtio/virtgpu_fence.c   |  95 ++
 drivers/gpu/drm/virtio/virtgpu_gem.c | 120 +++
 drivers/gpu/drm/virtio/virtgpu_kms.c | 125 +++
 drivers/gpu/drm/virtio/virtgpu_object.c  | 174 ++
 drivers/gpu/drm/virtio/virtgpu_ttm.c | 451 ++
 drivers/gpu/drm/virtio/virtgpu_vq.c  | 540 +++
 drivers/virtio/virtio_pci_common.c   |   2 +-
 include/drm/drmP.h   |   1 +
 include/uapi/linux/Kbuild|   1 +
 include/uapi/linux/virtio_gpu.h  | 203 
 include/uapi/linux/virtio_ids.h  |   2 +-
 21 files changed, 3267 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/virtio/Kconfig
 create mode 100644 drivers/gpu/drm/virtio/Makefile
 create mode 100644 drivers/gpu/drm/virtio/virtgpu_debugfs.c
 create mode 100644 drivers/gpu/drm/virtio/virtgpu_display.c
 create mode 100644 drivers/gpu/drm/virtio/virtgpu_drm_bus.c
 create mode 100644 drivers/gpu/drm/virtio/virtgpu_drv.c
 create mode 100644 drivers/gpu/drm/virtio/virtgpu_drv.h
 create mode 100644 drivers/gpu/drm/virtio/virtgpu_fb.c
 create mode 100644 drivers/gpu/drm/virtio/virtgpu_fence.c
 create mode 100644 drivers/gpu/drm/virtio/virtgpu_gem.c
 create mode 100644 drivers/gpu/drm/virtio/virtgpu_kms.c
 create mode 100644 drivers/gpu/drm/virtio/virtgpu_object.c
 create mode 100644 drivers/gpu/drm/virtio/virtgpu_ttm.c
 create mode 100644 drivers/gpu/drm/virtio/virtgpu_vq.c
 create mode 100644 include/uapi/linux/virtio_gpu.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 151a050..f2388ea 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -197,6 +197,8 @@ source drivers/gpu/drm/qxl/Kconfig
 
 source drivers/gpu/drm/bochs/Kconfig
 
+source drivers/gpu/drm/virtio/Kconfig
+
 source drivers/gpu/drm/msm/Kconfig
 
 source drivers/gpu/drm/tegra/Kconfig
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 2c239b9..083d443 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_DRM_OMAP)+= omapdrm/
 obj-$(CONFIG_DRM_TILCDC)   += tilcdc/
 obj-$(CONFIG_DRM_QXL) += qxl/
 obj-$(CONFIG_DRM_BOCHS) += bochs/
+obj-$(CONFIG_DRM_VIRTIO_GPU) += virtio/
 obj-$(CONFIG_DRM_MSM) += msm/
 obj-$(CONFIG_DRM_TEGRA) += tegra/
 obj-$(CONFIG_DRM_STI) += sti/
diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig
new file mode 100644
index 000..55868e2
--- /dev/null
+++ b/drivers/gpu/drm/virtio/Kconfig
@@ -0,0 +1,11 @@
+config DRM_VIRTIO_GPU
+   tristate QEMU Virtio GPU
+   depends on DRM  VIRTIO
+   select FB_SYS_FILLRECT
+   select FB_SYS_COPYAREA
+   select FB_SYS_IMAGEBLIT
+select DRM_KMS_HELPER
+select DRM_KMS_FB_HELPER
+select DRM_TTM
+   help
+  QEMU based virtio GPU.
diff --git a/drivers/gpu/drm/virtio/Makefile b/drivers/gpu/drm/virtio/Makefile
new file mode 100644
index 000..57d59ee
--- /dev/null
+++ b/drivers/gpu/drm/virtio/Makefile
@@ -0,0 +1,9 @@
+#
+# Makefile for the drm device driver.  This driver provides support for the
+# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
+
+ccflags-y := -Iinclude/drm
+
+virtio-gpu-y := virtgpu_drv.o virtgpu_kms.o virtgpu_drm_bus.o virtgpu_gem.o 
virtgpu_fb.o virtgpu_display.o virtgpu_vq.o virtgpu_ttm.o virtgpu_fence.o 
virtgpu_object.o virtgpu_debugfs.o
+
+obj-$(CONFIG_DRM_VIRTIO_GPU) += virtio-gpu.o
diff --git a/drivers/gpu/drm/virtio/virtgpu_debugfs.c 
b/drivers/gpu/drm/virtio/virtgpu_debugfs.c
new file mode 100644
index 000..dbc497d
--- 

Re: [PATCH] Add virtio gpu driver.

[Michael S. Tsirkin]

On Tue, Mar 24, 2015 at 05:07:18PM +0100, Gerd Hoffmann wrote:
 From: Dave Airlie airl...@gmail.com
 
 This patch adds a kms driver for the virtio gpu.  The xorg modesetting
 driver can handle the device just fine, the framebuffer for fbcon is
 there too.
 
 Qemu patches for the host side are under review currently.
 
 The pci version of the device comes in two variants: with and without
 vga compatibility.  The former has a extra memory bar for the vga
 framebuffer, the later is a pure virtio device.  The only concern for
 this driver is that in the virtio-vga case we have to kick out the
 firmware framebuffer.
 
 Initial revision has only 2d support, 3d (virgl) support requires
 some more work on the qemu side and will be added later.
 
 Signed-off-by: Dave Airlie airl...@redhat.com
 Signed-off-by: Gerd Hoffmann kra...@redhat.com

I did a quick scan of the code, below are mostly cosmetic
issues.

 ---
  drivers/gpu/drm/Kconfig  |   2 +
  drivers/gpu/drm/Makefile |   1 +
  drivers/gpu/drm/virtio/Kconfig   |  11 +
  drivers/gpu/drm/virtio/Makefile  |   9 +
  drivers/gpu/drm/virtio/virtgpu_debugfs.c |  64 
  drivers/gpu/drm/virtio/virtgpu_display.c | 527 ++
  drivers/gpu/drm/virtio/virtgpu_drm_bus.c |  68 
  drivers/gpu/drm/virtio/virtgpu_drv.c | 132 
  drivers/gpu/drm/virtio/virtgpu_drv.h | 326 +++
  drivers/gpu/drm/virtio/virtgpu_fb.c  | 415 
  drivers/gpu/drm/virtio/virtgpu_fence.c   |  95 ++
  drivers/gpu/drm/virtio/virtgpu_gem.c | 120 +++
  drivers/gpu/drm/virtio/virtgpu_kms.c | 125 +++
  drivers/gpu/drm/virtio/virtgpu_object.c  | 174 ++
  drivers/gpu/drm/virtio/virtgpu_ttm.c | 451 ++
  drivers/gpu/drm/virtio/virtgpu_vq.c  | 540 
 +++
  drivers/virtio/virtio_pci_common.c   |   2 +-
  include/drm/drmP.h   |   1 +
  include/uapi/linux/Kbuild|   1 +
  include/uapi/linux/virtio_gpu.h  | 203 
  include/uapi/linux/virtio_ids.h  |   2 +-
  21 files changed, 3267 insertions(+), 2 deletions(-)
  create mode 100644 drivers/gpu/drm/virtio/Kconfig
  create mode 100644 drivers/gpu/drm/virtio/Makefile
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_debugfs.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_display.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_drm_bus.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_drv.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_drv.h
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_fb.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_fence.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_gem.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_kms.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_object.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_ttm.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_vq.c
  create mode 100644 include/uapi/linux/virtio_gpu.h
 
 diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
 index 151a050..f2388ea 100644
 --- a/drivers/gpu/drm/Kconfig
 +++ b/drivers/gpu/drm/Kconfig
 @@ -197,6 +197,8 @@ source drivers/gpu/drm/qxl/Kconfig
  
  source drivers/gpu/drm/bochs/Kconfig
  
 +source drivers/gpu/drm/virtio/Kconfig
 +
  source drivers/gpu/drm/msm/Kconfig
  
  source drivers/gpu/drm/tegra/Kconfig
 diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
 index 2c239b9..083d443 100644
 --- a/drivers/gpu/drm/Makefile
 +++ b/drivers/gpu/drm/Makefile
 @@ -62,6 +62,7 @@ obj-$(CONFIG_DRM_OMAP)  += omapdrm/
  obj-$(CONFIG_DRM_TILCDC) += tilcdc/
  obj-$(CONFIG_DRM_QXL) += qxl/
  obj-$(CONFIG_DRM_BOCHS) += bochs/
 +obj-$(CONFIG_DRM_VIRTIO_GPU) += virtio/
  obj-$(CONFIG_DRM_MSM) += msm/
  obj-$(CONFIG_DRM_TEGRA) += tegra/
  obj-$(CONFIG_DRM_STI) += sti/
 diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig
 new file mode 100644
 index 000..55868e2
 --- /dev/null
 +++ b/drivers/gpu/drm/virtio/Kconfig
 @@ -0,0 +1,11 @@
 +config DRM_VIRTIO_GPU
 + tristate QEMU Virtio GPU

I think it should be Virtio GPU driver.

 + depends on DRM  VIRTIO
 + select FB_SYS_FILLRECT
 + select FB_SYS_COPYAREA
 + select FB_SYS_IMAGEBLIT
 +select DRM_KMS_HELPER
 +select DRM_KMS_FB_HELPER
 +select DRM_TTM
 + help
 +QEMU based virtio GPU.

How about:
This is the virtual GPU driver for virtio.  It can be used with
lguest or QEMU based VMMs (like KVM or Xen).  Say Y or M.


 diff --git a/drivers/gpu/drm/virtio/Makefile b/drivers/gpu/drm/virtio/Makefile
 new file mode 100644
 index 000..57d59ee
 --- /dev/null
 +++ b/drivers/gpu/drm/virtio/Makefile
 @@ -0,0 +1,9 @@
 +#
 +# Makefile for the drm device driver.  This driver provides support for the
 +# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 

Re: [PATCH] Add virtio gpu driver.

[Daniel Vetter]

On Tue, Mar 24, 2015 at 05:07:18PM +0100, Gerd Hoffmann wrote:
 From: Dave Airlie airl...@gmail.com
 
 This patch adds a kms driver for the virtio gpu.  The xorg modesetting
 driver can handle the device just fine, the framebuffer for fbcon is
 there too.
 
 Qemu patches for the host side are under review currently.
 
 The pci version of the device comes in two variants: with and without
 vga compatibility.  The former has a extra memory bar for the vga
 framebuffer, the later is a pure virtio device.  The only concern for
 this driver is that in the virtio-vga case we have to kick out the
 firmware framebuffer.
 
 Initial revision has only 2d support, 3d (virgl) support requires
 some more work on the qemu side and will be added later.
 
 Signed-off-by: Dave Airlie airl...@redhat.com
 Signed-off-by: Gerd Hoffmann kra...@redhat.com

Standard request from my side for new drm drivers (especially if they're
this simple): Can you please update the drivers to latest drm internal
interfaces, i.e. using universal planes and atomic?

Thanks, Daniel

 ---
  drivers/gpu/drm/Kconfig  |   2 +
  drivers/gpu/drm/Makefile |   1 +
  drivers/gpu/drm/virtio/Kconfig   |  11 +
  drivers/gpu/drm/virtio/Makefile  |   9 +
  drivers/gpu/drm/virtio/virtgpu_debugfs.c |  64 
  drivers/gpu/drm/virtio/virtgpu_display.c | 527 ++
  drivers/gpu/drm/virtio/virtgpu_drm_bus.c |  68 
  drivers/gpu/drm/virtio/virtgpu_drv.c | 132 
  drivers/gpu/drm/virtio/virtgpu_drv.h | 326 +++
  drivers/gpu/drm/virtio/virtgpu_fb.c  | 415 
  drivers/gpu/drm/virtio/virtgpu_fence.c   |  95 ++
  drivers/gpu/drm/virtio/virtgpu_gem.c | 120 +++
  drivers/gpu/drm/virtio/virtgpu_kms.c | 125 +++
  drivers/gpu/drm/virtio/virtgpu_object.c  | 174 ++
  drivers/gpu/drm/virtio/virtgpu_ttm.c | 451 ++
  drivers/gpu/drm/virtio/virtgpu_vq.c  | 540 
 +++
  drivers/virtio/virtio_pci_common.c   |   2 +-
  include/drm/drmP.h   |   1 +
  include/uapi/linux/Kbuild|   1 +
  include/uapi/linux/virtio_gpu.h  | 203 
  include/uapi/linux/virtio_ids.h  |   2 +-
  21 files changed, 3267 insertions(+), 2 deletions(-)
  create mode 100644 drivers/gpu/drm/virtio/Kconfig
  create mode 100644 drivers/gpu/drm/virtio/Makefile
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_debugfs.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_display.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_drm_bus.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_drv.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_drv.h
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_fb.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_fence.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_gem.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_kms.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_object.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_ttm.c
  create mode 100644 drivers/gpu/drm/virtio/virtgpu_vq.c
  create mode 100644 include/uapi/linux/virtio_gpu.h
 
 diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
 index 151a050..f2388ea 100644
 --- a/drivers/gpu/drm/Kconfig
 +++ b/drivers/gpu/drm/Kconfig
 @@ -197,6 +197,8 @@ source drivers/gpu/drm/qxl/Kconfig
  
  source drivers/gpu/drm/bochs/Kconfig
  
 +source drivers/gpu/drm/virtio/Kconfig
 +
  source drivers/gpu/drm/msm/Kconfig
  
  source drivers/gpu/drm/tegra/Kconfig
 diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
 index 2c239b9..083d443 100644
 --- a/drivers/gpu/drm/Makefile
 +++ b/drivers/gpu/drm/Makefile
 @@ -62,6 +62,7 @@ obj-$(CONFIG_DRM_OMAP)  += omapdrm/
  obj-$(CONFIG_DRM_TILCDC) += tilcdc/
  obj-$(CONFIG_DRM_QXL) += qxl/
  obj-$(CONFIG_DRM_BOCHS) += bochs/
 +obj-$(CONFIG_DRM_VIRTIO_GPU) += virtio/
  obj-$(CONFIG_DRM_MSM) += msm/
  obj-$(CONFIG_DRM_TEGRA) += tegra/
  obj-$(CONFIG_DRM_STI) += sti/
 diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig
 new file mode 100644
 index 000..55868e2
 --- /dev/null
 +++ b/drivers/gpu/drm/virtio/Kconfig
 @@ -0,0 +1,11 @@
 +config DRM_VIRTIO_GPU
 + tristate QEMU Virtio GPU
 + depends on DRM  VIRTIO
 + select FB_SYS_FILLRECT
 + select FB_SYS_COPYAREA
 + select FB_SYS_IMAGEBLIT
 +select DRM_KMS_HELPER
 +select DRM_KMS_FB_HELPER
 +select DRM_TTM
 + help
 +QEMU based virtio GPU.
 diff --git a/drivers/gpu/drm/virtio/Makefile b/drivers/gpu/drm/virtio/Makefile
 new file mode 100644
 index 000..57d59ee
 --- /dev/null
 +++ b/drivers/gpu/drm/virtio/Makefile
 @@ -0,0 +1,9 @@
 +#
 +# Makefile for the drm device driver.  This driver provides support for the
 +# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 +
 +ccflags-y := -Iinclude/drm
 +
 

Re: [PATCH] Add virtio gpu driver.

[Michael S. Tsirkin]

On Tue, Mar 24, 2015 at 05:07:18PM +0100, Gerd Hoffmann wrote:
 From: Dave Airlie airl...@gmail.com
 
 This patch adds a kms driver for the virtio gpu.  The xorg modesetting
 driver can handle the device just fine, the framebuffer for fbcon is
 there too.
 
 Qemu patches for the host side are under review currently.
 
 The pci version of the device comes in two variants: with and without
 vga compatibility.  The former has a extra memory bar for the vga
 framebuffer, the later is a pure virtio device.  The only concern for
 this driver is that in the virtio-vga case we have to kick out the
 firmware framebuffer.
 
 Initial revision has only 2d support, 3d (virgl) support requires
 some more work on the qemu side and will be added later.
 
 Signed-off-by: Dave Airlie airl...@redhat.com
 Signed-off-by: Gerd Hoffmann kra...@redhat.com

...

 diff --git a/drivers/virtio/virtio_pci_common.c 
 b/drivers/virtio/virtio_pci_common.c
 index e894eb2..a3167fa 100644
 --- a/drivers/virtio/virtio_pci_common.c
 +++ b/drivers/virtio/virtio_pci_common.c
 @@ -510,7 +510,7 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
   goto err_enable_device;
  
   rc = pci_request_regions(pci_dev, virtio-pci);
 - if (rc)
 + if (rc  ((pci_dev-class  8) != PCI_CLASS_DISPLAY_VGA))
   goto err_request_regions;
  
   if (force_legacy) {

This is probably what you described as the only concern?  Can you
explain why you are doing this?  If we only need to request specific
regions, I think we should do exactly that, requesting only parts of
regions that are covered by the virtio capabilities.

Seems cleaner than looking for a specific class.

Didn't look at device code in depth yet.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Add virtio gpu driver.

[Paul Bolle]

Just a license nit.

On Tue, 2015-03-24 at 17:07 +0100, Gerd Hoffmann wrote:

 --- /dev/null
 +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
 @@ -0,0 +1,132 @@
 +/*
 + * 2011 Red Hat, Inc.
 + * All Rights Reserved.
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a
 + * copy of this software and associated documentation files (the Software),
 + * to deal in the Software without restriction, including without limitation
 + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 + * and/or sell copies of the Software, and to permit persons to whom the
 + * Software is furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice (including the next
 + * paragraph) shall be included in all copies or substantial portions of the
 + * Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
 + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
 + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
 + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
 + * OTHER DEALINGS IN THE SOFTWARE.

I wouldn't know (without further, well, research) which license this is.

 +MODULE_LICENSE(GPL);

But I'm pretty sure it's not GPL v2 or later. So I think the license
mentioned in the comment at the top of this file and the license ident
used in this macro do not match.


Paul Bolle

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization