[Spice-devel] [PATCH v3 5/7] drm/qxl: use drm_gem_object_funcs callbacks

2019-09-03 Thread Gerd Hoffmann
Switch qxl to use drm_gem_object_funcs callbacks
instead of drm_driver callbacks.

Signed-off-by: Gerd Hoffmann 
Acked-by: Thomas Zimmermann 
---
 drivers/gpu/drm/qxl/qxl_drv.c|  8 
 drivers/gpu/drm/qxl/qxl_object.c | 12 
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 2b726a51a302..996d428fa7e6 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -258,16 +258,8 @@ static struct drm_driver qxl_driver = {
 #endif
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
-   .gem_prime_pin = qxl_gem_prime_pin,
-   .gem_prime_unpin = qxl_gem_prime_unpin,
-   .gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
.gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
-   .gem_prime_vmap = qxl_gem_prime_vmap,
-   .gem_prime_vunmap = qxl_gem_prime_vunmap,
.gem_prime_mmap = qxl_gem_prime_mmap,
-   .gem_free_object_unlocked = qxl_gem_object_free,
-   .gem_open_object = qxl_gem_object_open,
-   .gem_close_object = qxl_gem_object_close,
.fops = _fops,
.ioctls = qxl_ioctls,
.irq_handler = qxl_irq_handler,
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 548dfe6f3b26..29aab7b14513 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -77,6 +77,17 @@ void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32 
domain, bool pinned)
}
 }
 
+static const struct drm_gem_object_funcs qxl_object_funcs = {
+   .free = qxl_gem_object_free,
+   .open = qxl_gem_object_open,
+   .close = qxl_gem_object_close,
+   .pin = qxl_gem_prime_pin,
+   .unpin = qxl_gem_prime_unpin,
+   .get_sg_table = qxl_gem_prime_get_sg_table,
+   .vmap = qxl_gem_prime_vmap,
+   .vunmap = qxl_gem_prime_vunmap,
+};
+
 int qxl_bo_create(struct qxl_device *qdev,
  unsigned long size, bool kernel, bool pinned, u32 domain,
  struct qxl_surface *surf,
@@ -100,6 +111,7 @@ int qxl_bo_create(struct qxl_device *qdev,
kfree(bo);
return r;
}
+   bo->tbo.base.funcs = _object_funcs;
bo->type = domain;
bo->pin_count = pinned ? 1 : 0;
bo->surface_id = 0;
-- 
2.18.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH v3 6/7] drm/qxl: use drm_gem_ttm_print_info

2019-09-03 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
Acked-by: Thomas Zimmermann 
---
 drivers/gpu/drm/qxl/qxl_drv.h| 1 +
 drivers/gpu/drm/qxl/qxl_object.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 9e034c5fa87d..d4051409ce64 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 29aab7b14513..c013c516f561 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -86,6 +86,7 @@ static const struct drm_gem_object_funcs qxl_object_funcs = {
.get_sg_table = qxl_gem_prime_get_sg_table,
.vmap = qxl_gem_prime_vmap,
.vunmap = qxl_gem_prime_vunmap,
+   .print_info = drm_gem_ttm_print_info,
 };
 
 int qxl_bo_create(struct qxl_device *qdev,
-- 
2.18.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk v4 24/29] usb-backend: Rewrite USB emulation support

2019-09-03 Thread Frediano Ziglio
> 
> On 9/3/19 3:08 PM, Frediano Ziglio wrote:
> >>
> >> Hi,
> >>
> >> On 8/27/19 12:22 PM, Frediano Ziglio wrote:
> >>> Make initialisation easier.
> >>> Always initialise parser.
> >>> Initialise both parser and host during spice_usb_backend_channel_new.
> >>> Support not having libusb context (no physical devices).
> >>> Avoids too much state variables.
> >>> parser is always initialised after creation making sure the state
> >>> is consistent.
> >>
> >> If usbredirhost is used, why is there a need for a parser too ?
> >> Also below in initialize_parser there is a check that
> >> ch->usbredirhost is NULL (possibly because the parser is always
> >> being initialized before the host).
> >>
> > 
> > For a larger explanation see former patch from Yuri.
> > For a shorter to parse data directed to emulated devices.
> > So host devices (physical) through usbredirhost, emulated devices
> > through usbparser.
> 
> The commit log is saying the parser is always being initialized.
> I'm asking if it is needed also for physical devices.
> 

No, it's used only for emulated devices.

> > 
> > The check for NULL as this path is supposed to initialize the
> > parse only if usbredirhost is not initialize > Maybe would be worth a
> > comment or another name. Or maybe just inline
> > it now, not very long. What do you suggest?
> 
> Just leave it as is. I'll look at previous patches to better understand.
> 
> > 
> >>> Use a single state variable, data flows into/from a single parser.
> >>>
> >>> Signed-off-by: Frediano Ziglio 
> >>> ---
> >>>src/usb-backend.c | 236 +++---
> >>>1 file changed, 116 insertions(+), 120 deletions(-)
> >>>
> >>> diff --git a/src/usb-backend.c b/src/usb-backend.c
> >>> index 36a73a89..d614e693 100644
> >>> --- a/src/usb-backend.c
> >>> +++ b/src/usb-backend.c
> >>> @@ -78,21 +78,21 @@ struct _SpiceUsbBackend
> >>>uint32_t own_devices_mask;
> >>>};
> >>>
> >>> +typedef enum {
> >>> +USB_CHANNEL_STATE_INITIALIZING,
> >>> +USB_CHANNEL_STATE_HOST,
> >>> +USB_CHANNEL_STATE_PARSER,
> >>> +} SpiceUsbBackendChannelState;
> >>> +
> >>>struct _SpiceUsbBackendChannel
> >>>{
> >>>struct usbredirhost *usbredirhost;
> >>> -struct usbredirhost *hidden_host;
> >>>struct usbredirparser *parser;
> >>> -struct usbredirparser *hidden_parser;
> >>> +SpiceUsbBackendChannelState state;
> >>>uint8_t *read_buf;
> >>>int read_buf_size;
> >>>struct usbredirfilter_rule *rules;
> >>>int rules_count;
> >>> -uint32_t host_caps;
> >>> -uint32_t parser_init_done  : 1;
> >>> -uint32_t parser_with_hello : 1;
> >>> -uint32_t hello_done_parser : 1;
> >>> -uint32_t hello_sent: 1;
> >>>uint32_t rejected  : 1;
> >>>uint32_t wait_disconnect_ack : 1;
> >>>SpiceUsbBackendDevice *attached;
> >>> @@ -405,15 +405,16 @@ from both the main thread as well as from the usb
> >>> event handling thread */
> >>>static void usbredir_write_flush_callback(void *user_data)
> >>>{
> >>>SpiceUsbBackendChannel *ch = user_data;
> >>> +if (ch->parser == NULL) {
> >>> +return;
> >>> +}
> >>>if (is_channel_ready(ch->user_data)) {
> >>> -if (ch->usbredirhost) {
> >>
> >> Do you need the following g_assert, or is the ch->parser==NULL enough
> >>  g_assert(ch->state != USB_CHANNEL_STATE_INITIALIZING);
> >>
> > 
> > Which one are you referring to?
> > 
> >>> +if (ch->state == USB_CHANNEL_STATE_HOST) {
> >>>SPICE_DEBUG("%s ch %p -> usbredirhost", __FUNCTION__, ch);
> >>>usbredirhost_write_guest_data(ch->usbredirhost);
> >>> -} else if (ch->parser) {
> >>> +} else {
> >>>SPICE_DEBUG("%s ch %p -> parser", __FUNCTION__, ch);
> >>>usbredirparser_do_write(ch->parser);
> >>> -} else {
> >>> -SPICE_DEBUG("%s ch %p (NOT ACTIVE)", __FUNCTION__, ch);
> >>>}
> >>>} else {
> >>>SPICE_DEBUG("%s ch %p (not ready)", __FUNCTION__, ch);
> >>> @@ -673,21 +674,42 @@ static void usbredir_log(void *user_data, int
> >>> level,
> >>> const char *msg)
> >>>}
> >>>}
> >>>
> >>> +static struct usbredirparser *create_parser(SpiceUsbBackendChannel *ch);
> >>> +
> >>>static int usbredir_write_callback(void *user_data, uint8_t *data, int
> >>>count)
> >>>{
> >>>SpiceUsbBackendChannel *ch = user_data;
> >>>int res;
> >>>SPICE_DEBUG("%s ch %p, %d bytes", __FUNCTION__, ch, count);
> >>> -if (!ch->hello_sent) {
> >>> -/* hello is short header (12) + hello struct (64) + capabilities
> >>> (4) */
> >>> -int hello_size = sizeof(struct usb_redir_header) + sizeof(struct
> >>> usb_redir_hello_header);
> >>> -ch->hello_sent = 1;
> >>> -if (count == hello_size) {
> >>> -memcpy(>host_caps, data + hello_size -
> >>> 

Re: [Spice-devel] [PATCH spice-gtk v4 24/29] usb-backend: Rewrite USB emulation support

2019-09-03 Thread Uri Lublin

On 9/3/19 3:08 PM, Frediano Ziglio wrote:


Hi,

On 8/27/19 12:22 PM, Frediano Ziglio wrote:

Make initialisation easier.
Always initialise parser.
Initialise both parser and host during spice_usb_backend_channel_new.
Support not having libusb context (no physical devices).
Avoids too much state variables.
parser is always initialised after creation making sure the state
is consistent.


If usbredirhost is used, why is there a need for a parser too ?
Also below in initialize_parser there is a check that
ch->usbredirhost is NULL (possibly because the parser is always
being initialized before the host).



For a larger explanation see former patch from Yuri.
For a shorter to parse data directed to emulated devices.
So host devices (physical) through usbredirhost, emulated devices
through usbparser.


The commit log is saying the parser is always being initialized.
I'm asking if it is needed also for physical devices.



The check for NULL as this path is supposed to initialize the
parse only if usbredirhost is not initialize > Maybe would be worth a comment 
or another name. Or maybe just inline
it now, not very long. What do you suggest?


Just leave it as is. I'll look at previous patches to better understand.




Use a single state variable, data flows into/from a single parser.

Signed-off-by: Frediano Ziglio 
---
   src/usb-backend.c | 236 +++---
   1 file changed, 116 insertions(+), 120 deletions(-)

diff --git a/src/usb-backend.c b/src/usb-backend.c
index 36a73a89..d614e693 100644
--- a/src/usb-backend.c
+++ b/src/usb-backend.c
@@ -78,21 +78,21 @@ struct _SpiceUsbBackend
   uint32_t own_devices_mask;
   };
   
+typedef enum {

+USB_CHANNEL_STATE_INITIALIZING,
+USB_CHANNEL_STATE_HOST,
+USB_CHANNEL_STATE_PARSER,
+} SpiceUsbBackendChannelState;
+
   struct _SpiceUsbBackendChannel
   {
   struct usbredirhost *usbredirhost;
-struct usbredirhost *hidden_host;
   struct usbredirparser *parser;
-struct usbredirparser *hidden_parser;
+SpiceUsbBackendChannelState state;
   uint8_t *read_buf;
   int read_buf_size;
   struct usbredirfilter_rule *rules;
   int rules_count;
-uint32_t host_caps;
-uint32_t parser_init_done  : 1;
-uint32_t parser_with_hello : 1;
-uint32_t hello_done_parser : 1;
-uint32_t hello_sent: 1;
   uint32_t rejected  : 1;
   uint32_t wait_disconnect_ack : 1;
   SpiceUsbBackendDevice *attached;
@@ -405,15 +405,16 @@ from both the main thread as well as from the usb
event handling thread */
   static void usbredir_write_flush_callback(void *user_data)
   {
   SpiceUsbBackendChannel *ch = user_data;
+if (ch->parser == NULL) {
+return;
+}
   if (is_channel_ready(ch->user_data)) {
-if (ch->usbredirhost) {


Do you need the following g_assert, or is the ch->parser==NULL enough
 g_assert(ch->state != USB_CHANNEL_STATE_INITIALIZING);



Which one are you referring to?


+if (ch->state == USB_CHANNEL_STATE_HOST) {
   SPICE_DEBUG("%s ch %p -> usbredirhost", __FUNCTION__, ch);
   usbredirhost_write_guest_data(ch->usbredirhost);
-} else if (ch->parser) {
+} else {
   SPICE_DEBUG("%s ch %p -> parser", __FUNCTION__, ch);
   usbredirparser_do_write(ch->parser);
-} else {
-SPICE_DEBUG("%s ch %p (NOT ACTIVE)", __FUNCTION__, ch);
   }
   } else {
   SPICE_DEBUG("%s ch %p (not ready)", __FUNCTION__, ch);
@@ -673,21 +674,42 @@ static void usbredir_log(void *user_data, int level,
const char *msg)
   }
   }
   
+static struct usbredirparser *create_parser(SpiceUsbBackendChannel *ch);

+
   static int usbredir_write_callback(void *user_data, uint8_t *data, int
   count)
   {
   SpiceUsbBackendChannel *ch = user_data;
   int res;
   SPICE_DEBUG("%s ch %p, %d bytes", __FUNCTION__, ch, count);
-if (!ch->hello_sent) {
-/* hello is short header (12) + hello struct (64) + capabilities
(4) */
-int hello_size = sizeof(struct usb_redir_header) + sizeof(struct
usb_redir_hello_header);
-ch->hello_sent = 1;
-if (count == hello_size) {
-memcpy(>host_caps, data + hello_size -
sizeof(ch->host_caps),
-   sizeof(ch->host_caps));
-SPICE_DEBUG("%s ch %p, sending first hello, caps %08X",
-__FUNCTION__, ch, ch->host_caps);
+
+// handle first packet (HELLO) creating parser from capabilities
+if (SPICE_UNLIKELY(ch->parser == NULL)) {
+// we are still initializing the host
+if (ch->usbredirhost == NULL) {
+return 0;
   }
+
+ch->parser = create_parser(ch);
+if (!ch->parser) {
+return 0;
+}
+
+/* hello is short header (12) + hello struct (64) */
+const int hello_size = 12 + sizeof(struct usb_redir_hello_header);


sizeof(struct usb_redir_header) 

Re: [Spice-devel] [PATCH spice-gtk v4 24/29] usb-backend: Rewrite USB emulation support

2019-09-03 Thread Frediano Ziglio
> 
> Hi,
> 
> On 8/27/19 12:22 PM, Frediano Ziglio wrote:
> > Make initialisation easier.
> > Always initialise parser.
> > Initialise both parser and host during spice_usb_backend_channel_new.
> > Support not having libusb context (no physical devices).
> > Avoids too much state variables.
> > parser is always initialised after creation making sure the state
> > is consistent.
> 
> If usbredirhost is used, why is there a need for a parser too ?
> Also below in initialize_parser there is a check that
> ch->usbredirhost is NULL (possibly because the parser is always
> being initialized before the host).
> 

For a larger explanation see former patch from Yuri.
For a shorter to parse data directed to emulated devices.
So host devices (physical) through usbredirhost, emulated devices
through usbparser.

The check for NULL as this path is supposed to initialize the
parse only if usbredirhost is not initialized.
Maybe would be wotrth a comment or another name. Or maybe just inline
it now, not very long. What do you suggest?

> > Use a single state variable, data flows into/from a single parser.
> > 
> > Signed-off-by: Frediano Ziglio 
> > ---
> >   src/usb-backend.c | 236 +++---
> >   1 file changed, 116 insertions(+), 120 deletions(-)
> > 
> > diff --git a/src/usb-backend.c b/src/usb-backend.c
> > index 36a73a89..d614e693 100644
> > --- a/src/usb-backend.c
> > +++ b/src/usb-backend.c
> > @@ -78,21 +78,21 @@ struct _SpiceUsbBackend
> >   uint32_t own_devices_mask;
> >   };
> >   
> > +typedef enum {
> > +USB_CHANNEL_STATE_INITIALIZING,
> > +USB_CHANNEL_STATE_HOST,
> > +USB_CHANNEL_STATE_PARSER,
> > +} SpiceUsbBackendChannelState;
> > +
> >   struct _SpiceUsbBackendChannel
> >   {
> >   struct usbredirhost *usbredirhost;
> > -struct usbredirhost *hidden_host;
> >   struct usbredirparser *parser;
> > -struct usbredirparser *hidden_parser;
> > +SpiceUsbBackendChannelState state;
> >   uint8_t *read_buf;
> >   int read_buf_size;
> >   struct usbredirfilter_rule *rules;
> >   int rules_count;
> > -uint32_t host_caps;
> > -uint32_t parser_init_done  : 1;
> > -uint32_t parser_with_hello : 1;
> > -uint32_t hello_done_parser : 1;
> > -uint32_t hello_sent: 1;
> >   uint32_t rejected  : 1;
> >   uint32_t wait_disconnect_ack : 1;
> >   SpiceUsbBackendDevice *attached;
> > @@ -405,15 +405,16 @@ from both the main thread as well as from the usb
> > event handling thread */
> >   static void usbredir_write_flush_callback(void *user_data)
> >   {
> >   SpiceUsbBackendChannel *ch = user_data;
> > +if (ch->parser == NULL) {
> > +return;
> > +}
> >   if (is_channel_ready(ch->user_data)) {
> > -if (ch->usbredirhost) {
> 
> Do you need the following g_assert, or is the ch->parser==NULL enough
> g_assert(ch->state != USB_CHANNEL_STATE_INITIALIZING);
> 

Which one are you referring to?

> > +if (ch->state == USB_CHANNEL_STATE_HOST) {
> >   SPICE_DEBUG("%s ch %p -> usbredirhost", __FUNCTION__, ch);
> >   usbredirhost_write_guest_data(ch->usbredirhost);
> > -} else if (ch->parser) {
> > +} else {
> >   SPICE_DEBUG("%s ch %p -> parser", __FUNCTION__, ch);
> >   usbredirparser_do_write(ch->parser);
> > -} else {
> > -SPICE_DEBUG("%s ch %p (NOT ACTIVE)", __FUNCTION__, ch);
> >   }
> >   } else {
> >   SPICE_DEBUG("%s ch %p (not ready)", __FUNCTION__, ch);
> > @@ -673,21 +674,42 @@ static void usbredir_log(void *user_data, int level,
> > const char *msg)
> >   }
> >   }
> >   
> > +static struct usbredirparser *create_parser(SpiceUsbBackendChannel *ch);
> > +
> >   static int usbredir_write_callback(void *user_data, uint8_t *data, int
> >   count)
> >   {
> >   SpiceUsbBackendChannel *ch = user_data;
> >   int res;
> >   SPICE_DEBUG("%s ch %p, %d bytes", __FUNCTION__, ch, count);
> > -if (!ch->hello_sent) {
> > -/* hello is short header (12) + hello struct (64) + capabilities
> > (4) */
> > -int hello_size = sizeof(struct usb_redir_header) + sizeof(struct
> > usb_redir_hello_header);
> > -ch->hello_sent = 1;
> > -if (count == hello_size) {
> > -memcpy(>host_caps, data + hello_size -
> > sizeof(ch->host_caps),
> > -   sizeof(ch->host_caps));
> > -SPICE_DEBUG("%s ch %p, sending first hello, caps %08X",
> > -__FUNCTION__, ch, ch->host_caps);
> > +
> > +// handle first packet (HELLO) creating parser from capabilities
> > +if (SPICE_UNLIKELY(ch->parser == NULL)) {
> > +// we are still initializing the host
> > +if (ch->usbredirhost == NULL) {
> > +return 0;
> >   }
> > +
> > +ch->parser = create_parser(ch);
> > +if (!ch->parser) {
> > +return 0;
> > +}
> > +
> > +   

Re: [Spice-devel] [PATCH spice-gtk v4 24/29] usb-backend: Rewrite USB emulation support

2019-09-03 Thread Uri Lublin

Hi,

On 8/27/19 12:22 PM, Frediano Ziglio wrote:

Make initialisation easier.
Always initialise parser.
Initialise both parser and host during spice_usb_backend_channel_new.
Support not having libusb context (no physical devices).
Avoids too much state variables.
parser is always initialised after creation making sure the state
is consistent.


If usbredirhost is used, why is there a need for a parser too ?
Also below in initialize_parser there is a check that
ch->usbredirhost is NULL (possibly because the parser is always
being initialized before the host).


Use a single state variable, data flows into/from a single parser.

Signed-off-by: Frediano Ziglio 
---
  src/usb-backend.c | 236 +++---
  1 file changed, 116 insertions(+), 120 deletions(-)

diff --git a/src/usb-backend.c b/src/usb-backend.c
index 36a73a89..d614e693 100644
--- a/src/usb-backend.c
+++ b/src/usb-backend.c
@@ -78,21 +78,21 @@ struct _SpiceUsbBackend
  uint32_t own_devices_mask;
  };
  
+typedef enum {

+USB_CHANNEL_STATE_INITIALIZING,
+USB_CHANNEL_STATE_HOST,
+USB_CHANNEL_STATE_PARSER,
+} SpiceUsbBackendChannelState;
+
  struct _SpiceUsbBackendChannel
  {
  struct usbredirhost *usbredirhost;
-struct usbredirhost *hidden_host;
  struct usbredirparser *parser;
-struct usbredirparser *hidden_parser;
+SpiceUsbBackendChannelState state;
  uint8_t *read_buf;
  int read_buf_size;
  struct usbredirfilter_rule *rules;
  int rules_count;
-uint32_t host_caps;
-uint32_t parser_init_done  : 1;
-uint32_t parser_with_hello : 1;
-uint32_t hello_done_parser : 1;
-uint32_t hello_sent: 1;
  uint32_t rejected  : 1;
  uint32_t wait_disconnect_ack : 1;
  SpiceUsbBackendDevice *attached;
@@ -405,15 +405,16 @@ from both the main thread as well as from the usb event 
handling thread */
  static void usbredir_write_flush_callback(void *user_data)
  {
  SpiceUsbBackendChannel *ch = user_data;
+if (ch->parser == NULL) {
+return;
+}
  if (is_channel_ready(ch->user_data)) {
-if (ch->usbredirhost) {


Do you need the following g_assert, or is the ch->parser==NULL enough
   g_assert(ch->state != USB_CHANNEL_STATE_INITIALIZING);


+if (ch->state == USB_CHANNEL_STATE_HOST) {
  SPICE_DEBUG("%s ch %p -> usbredirhost", __FUNCTION__, ch);
  usbredirhost_write_guest_data(ch->usbredirhost);
-} else if (ch->parser) {
+} else {
  SPICE_DEBUG("%s ch %p -> parser", __FUNCTION__, ch);
  usbredirparser_do_write(ch->parser);
-} else {
-SPICE_DEBUG("%s ch %p (NOT ACTIVE)", __FUNCTION__, ch);
  }
  } else {
  SPICE_DEBUG("%s ch %p (not ready)", __FUNCTION__, ch);
@@ -673,21 +674,42 @@ static void usbredir_log(void *user_data, int level, 
const char *msg)
  }
  }
  
+static struct usbredirparser *create_parser(SpiceUsbBackendChannel *ch);

+
  static int usbredir_write_callback(void *user_data, uint8_t *data, int count)
  {
  SpiceUsbBackendChannel *ch = user_data;
  int res;
  SPICE_DEBUG("%s ch %p, %d bytes", __FUNCTION__, ch, count);
-if (!ch->hello_sent) {
-/* hello is short header (12) + hello struct (64) + capabilities (4) */
-int hello_size = sizeof(struct usb_redir_header) + sizeof(struct 
usb_redir_hello_header);
-ch->hello_sent = 1;
-if (count == hello_size) {
-memcpy(>host_caps, data + hello_size - sizeof(ch->host_caps),
-   sizeof(ch->host_caps));
-SPICE_DEBUG("%s ch %p, sending first hello, caps %08X",
-__FUNCTION__, ch, ch->host_caps);
+
+// handle first packet (HELLO) creating parser from capabilities
+if (SPICE_UNLIKELY(ch->parser == NULL)) {
+// we are still initializing the host
+if (ch->usbredirhost == NULL) {
+return 0;
  }
+
+ch->parser = create_parser(ch);
+if (!ch->parser) {
+return 0;
+}
+
+/* hello is short header (12) + hello struct (64) */
+const int hello_size = 12 + sizeof(struct usb_redir_hello_header);


sizeof(struct usb_redir_header) instead of 12 (and btw it's 16)


+g_assert(count >= hello_size + 4);


nit: Maybe replace 4 with
  const size_t caps_size = sizeof(uint32_t) * USB_REDIR_CAPS_SIZE;
  g_assert(count >= hello_size + caps_size);


Uri.


+g_assert(SPICE_ALIGNED_CAST(struct usb_redir_header *, data)->type == 
usb_redir_hello);
+
+const uint32_t flags =
+usbredirparser_fl_write_cb_owns_buffer | 
usbredirparser_fl_usb_host |
+usbredirparser_fl_no_hello;
+
+usbredirparser_init(ch->parser,
+PACKAGE_STRING,
+SPICE_ALIGNED_CAST(uint32_t *, data + hello_size),
+(count - hello_size) / sizeof(uint32_t),
+  

[Spice-devel] [PATCH v2 5/6] drm/qxl: use drm_gem_object_funcs callbacks

2019-09-03 Thread Gerd Hoffmann
Switch qxl to use drm_gem_object_funcs callbacks
instead of drm_driver callbacks.

Signed-off-by: Gerd Hoffmann 
Acked-by: Thomas Zimmermann 
---
 drivers/gpu/drm/qxl/qxl_drv.c|  8 
 drivers/gpu/drm/qxl/qxl_object.c | 12 
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 2b726a51a302..996d428fa7e6 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -258,16 +258,8 @@ static struct drm_driver qxl_driver = {
 #endif
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
-   .gem_prime_pin = qxl_gem_prime_pin,
-   .gem_prime_unpin = qxl_gem_prime_unpin,
-   .gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
.gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
-   .gem_prime_vmap = qxl_gem_prime_vmap,
-   .gem_prime_vunmap = qxl_gem_prime_vunmap,
.gem_prime_mmap = qxl_gem_prime_mmap,
-   .gem_free_object_unlocked = qxl_gem_object_free,
-   .gem_open_object = qxl_gem_object_open,
-   .gem_close_object = qxl_gem_object_close,
.fops = _fops,
.ioctls = qxl_ioctls,
.irq_handler = qxl_irq_handler,
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 548dfe6f3b26..29aab7b14513 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -77,6 +77,17 @@ void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32 
domain, bool pinned)
}
 }
 
+static const struct drm_gem_object_funcs qxl_object_funcs = {
+   .free = qxl_gem_object_free,
+   .open = qxl_gem_object_open,
+   .close = qxl_gem_object_close,
+   .pin = qxl_gem_prime_pin,
+   .unpin = qxl_gem_prime_unpin,
+   .get_sg_table = qxl_gem_prime_get_sg_table,
+   .vmap = qxl_gem_prime_vmap,
+   .vunmap = qxl_gem_prime_vunmap,
+};
+
 int qxl_bo_create(struct qxl_device *qdev,
  unsigned long size, bool kernel, bool pinned, u32 domain,
  struct qxl_surface *surf,
@@ -100,6 +111,7 @@ int qxl_bo_create(struct qxl_device *qdev,
kfree(bo);
return r;
}
+   bo->tbo.base.funcs = _object_funcs;
bo->type = domain;
bo->pin_count = pinned ? 1 : 0;
bo->surface_id = 0;
-- 
2.18.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH v2 6/6] drm/qxl: use drm_gem_ttm_print_info

2019-09-03 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
Acked-by: Thomas Zimmermann 
---
 drivers/gpu/drm/qxl/qxl_drv.h| 1 +
 drivers/gpu/drm/qxl/qxl_object.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 9e034c5fa87d..d4051409ce64 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 29aab7b14513..c013c516f561 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -86,6 +86,7 @@ static const struct drm_gem_object_funcs qxl_object_funcs = {
.get_sg_table = qxl_gem_prime_get_sg_table,
.vmap = qxl_gem_prime_vmap,
.vunmap = qxl_gem_prime_vunmap,
+   .print_info = drm_gem_ttm_print_info,
 };
 
 int qxl_bo_create(struct qxl_device *qdev,
-- 
2.18.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 4/5] drm/qxl: use drm_gem_object_funcs callbacks

2019-09-03 Thread Thomas Zimmermann
Hi

Am 03.09.19 um 08:24 schrieb Gerd Hoffmann:
> On Mon, Sep 02, 2019 at 04:34:49PM +0200, Thomas Zimmermann wrote:
>> This patch seems unrelated.
> 
> Well, patch 5/5 depends on it because it hooks the
> drm_gem_ttm_print_info helper into the new
> qxl_object_funcs added by this patch.

Acked-by: Thomas Zimmermann 


> 
>> Am 02.09.19 um 14:41 schrieb Gerd Hoffmann:
>>> Switch qxl to use drm_gem_object_funcs callbacks
>>> instead of drm_driver callbacks.
>>>
>>> Signed-off-by: Gerd Hoffmann 
>>> ---
>>>  drivers/gpu/drm/qxl/qxl_drv.c|  8 
>>>  drivers/gpu/drm/qxl/qxl_object.c | 12 
>>>  2 files changed, 12 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
>>> index 2b726a51a302..996d428fa7e6 100644
>>> --- a/drivers/gpu/drm/qxl/qxl_drv.c
>>> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
>>> @@ -258,16 +258,8 @@ static struct drm_driver qxl_driver = {
>>>  #endif
>>> .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>>> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>>> -   .gem_prime_pin = qxl_gem_prime_pin,
>>> -   .gem_prime_unpin = qxl_gem_prime_unpin,
>>> -   .gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
>>> .gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
>>> -   .gem_prime_vmap = qxl_gem_prime_vmap,
>>> -   .gem_prime_vunmap = qxl_gem_prime_vunmap,
>>> .gem_prime_mmap = qxl_gem_prime_mmap,
>>> -   .gem_free_object_unlocked = qxl_gem_object_free,
>>> -   .gem_open_object = qxl_gem_object_open,
>>> -   .gem_close_object = qxl_gem_object_close,
>>> .fops = _fops,
>>> .ioctls = qxl_ioctls,
>>> .irq_handler = qxl_irq_handler,
>>> diff --git a/drivers/gpu/drm/qxl/qxl_object.c 
>>> b/drivers/gpu/drm/qxl/qxl_object.c
>>> index 548dfe6f3b26..29aab7b14513 100644
>>> --- a/drivers/gpu/drm/qxl/qxl_object.c
>>> +++ b/drivers/gpu/drm/qxl/qxl_object.c
>>> @@ -77,6 +77,17 @@ void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, 
>>> u32 domain, bool pinned)
>>> }
>>>  }
>>>  
>>> +static const struct drm_gem_object_funcs qxl_object_funcs = {
>>> +   .free = qxl_gem_object_free,
>>> +   .open = qxl_gem_object_open,
>>> +   .close = qxl_gem_object_close,
>>> +   .pin = qxl_gem_prime_pin,
>>> +   .unpin = qxl_gem_prime_unpin,
>>> +   .get_sg_table = qxl_gem_prime_get_sg_table,
>>> +   .vmap = qxl_gem_prime_vmap,
>>> +   .vunmap = qxl_gem_prime_vunmap,
>>> +};
>>> +
>>>  int qxl_bo_create(struct qxl_device *qdev,
>>>   unsigned long size, bool kernel, bool pinned, u32 domain,
>>>   struct qxl_surface *surf,
>>> @@ -100,6 +111,7 @@ int qxl_bo_create(struct qxl_device *qdev,
>>> kfree(bo);
>>> return r;
>>> }
>>> +   bo->tbo.base.funcs = _object_funcs;
>>> bo->type = domain;
>>> bo->pin_count = pinned ? 1 : 0;
>>> bo->surface_id = 0;
>>>
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
>> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
>> HRB 21284 (AG Nürnberg)
>>
> 
> 
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)



signature.asc
Description: OpenPGP digital signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 4/5] drm/qxl: use drm_gem_object_funcs callbacks

2019-09-03 Thread Gerd Hoffmann
On Mon, Sep 02, 2019 at 04:34:49PM +0200, Thomas Zimmermann wrote:
> This patch seems unrelated.

Well, patch 5/5 depends on it because it hooks the
drm_gem_ttm_print_info helper into the new
qxl_object_funcs added by this patch.

> Am 02.09.19 um 14:41 schrieb Gerd Hoffmann:
> > Switch qxl to use drm_gem_object_funcs callbacks
> > instead of drm_driver callbacks.
> > 
> > Signed-off-by: Gerd Hoffmann 
> > ---
> >  drivers/gpu/drm/qxl/qxl_drv.c|  8 
> >  drivers/gpu/drm/qxl/qxl_object.c | 12 
> >  2 files changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> > index 2b726a51a302..996d428fa7e6 100644
> > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > @@ -258,16 +258,8 @@ static struct drm_driver qxl_driver = {
> >  #endif
> > .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> > .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> > -   .gem_prime_pin = qxl_gem_prime_pin,
> > -   .gem_prime_unpin = qxl_gem_prime_unpin,
> > -   .gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
> > .gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
> > -   .gem_prime_vmap = qxl_gem_prime_vmap,
> > -   .gem_prime_vunmap = qxl_gem_prime_vunmap,
> > .gem_prime_mmap = qxl_gem_prime_mmap,
> > -   .gem_free_object_unlocked = qxl_gem_object_free,
> > -   .gem_open_object = qxl_gem_object_open,
> > -   .gem_close_object = qxl_gem_object_close,
> > .fops = _fops,
> > .ioctls = qxl_ioctls,
> > .irq_handler = qxl_irq_handler,
> > diff --git a/drivers/gpu/drm/qxl/qxl_object.c 
> > b/drivers/gpu/drm/qxl/qxl_object.c
> > index 548dfe6f3b26..29aab7b14513 100644
> > --- a/drivers/gpu/drm/qxl/qxl_object.c
> > +++ b/drivers/gpu/drm/qxl/qxl_object.c
> > @@ -77,6 +77,17 @@ void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, 
> > u32 domain, bool pinned)
> > }
> >  }
> >  
> > +static const struct drm_gem_object_funcs qxl_object_funcs = {
> > +   .free = qxl_gem_object_free,
> > +   .open = qxl_gem_object_open,
> > +   .close = qxl_gem_object_close,
> > +   .pin = qxl_gem_prime_pin,
> > +   .unpin = qxl_gem_prime_unpin,
> > +   .get_sg_table = qxl_gem_prime_get_sg_table,
> > +   .vmap = qxl_gem_prime_vmap,
> > +   .vunmap = qxl_gem_prime_vunmap,
> > +};
> > +
> >  int qxl_bo_create(struct qxl_device *qdev,
> >   unsigned long size, bool kernel, bool pinned, u32 domain,
> >   struct qxl_surface *surf,
> > @@ -100,6 +111,7 @@ int qxl_bo_create(struct qxl_device *qdev,
> > kfree(bo);
> > return r;
> > }
> > +   bo->tbo.base.funcs = _object_funcs;
> > bo->type = domain;
> > bo->pin_count = pinned ? 1 : 0;
> > bo->surface_id = 0;
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
> HRB 21284 (AG Nürnberg)
> 



___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk v3] streaming: make draw-area visible on MJPEG encoder creation

2019-09-03 Thread Snir Sheriber

Hi,

On 9/2/19 6:50 PM, Kevin Pouget wrote:

This patch allows the MJPEG encoder to inform the spice-widget that
its video drawing area (draw-area) should be made visible on screen.

This is required to switch from GST video decoding to native MJPEG
decoding, otherwise the gst-area remained on top and the MJPEG video
stream was never shown.

Signed-off-by: Kevin Pouget 
---
v2 -> v3: address Snir comments:

- comment about NULL in the signal description
- move 'if (pipeline_ptr == NULL) ...' outside  of 'if 
defined(GDK_WINDOWING_X11)'

regarding EGL note, it might be something like this:


gtk_stack_set_visible_child_name(d->stack, egl_enabled(d) ?
"gl-area" : "draw-area");



Yes, but AFAIU this should never happen

So I'll ack also without it.

Snir.


but it's just a wild guess!

---
  src/channel-display-mjpeg.c |  3 +++
  src/channel-display.c   |  4 +++-
  src/spice-widget.c  | 11 +--
  3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
index 647d31b..636a98b 100644
--- a/src/channel-display-mjpeg.c
+++ b/src/channel-display-mjpeg.c
@@ -300,5 +300,8 @@ VideoDecoder* create_mjpeg_decoder(int codec_type, 
display_stream *stream)

  /* All the other fields are initialized to zero by g_new0(). */

+/* makes the draw-area visible */
+hand_pipeline_to_widget(stream, NULL);
+
  return (VideoDecoder*)decoder;
  }
diff --git a/src/channel-display.c b/src/channel-display.c
index 59c809d..cd87c7c 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -485,7 +485,9 @@ static void 
spice_display_channel_class_init(SpiceDisplayChannelClass *klass)
   *
   * The #SpiceDisplayChannel::gst-video-overlay signal is emitted when
   * pipeline is ready and can be passed to widget to register GStreamer
- * overlay interface and other GStreamer callbacks.
+ * overlay interface and other GStreamer callbacks. If the pipeline
+ * pointer is NULL, the drawing area of the native renderer is set
+ * visible.
   *
   * Returns: %TRUE if the overlay is being set
   *
diff --git a/src/spice-widget.c b/src/spice-widget.c
index a9ba1f1..d73e02f 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -2780,14 +2780,20 @@ static void gst_size_allocate(GtkWidget *widget, 
GdkRectangle *a, gpointer data)
  }

  /* This callback should pass to the widget a pointer of the pipeline
- * so that we can set pipeline and overlay related calls from here.
+ * so that we can the set GST pipeline and overlay related calls from
+ * here.  If the pipeline pointer is NULL, the drawing area of the
+ * native renderer is set visible.
   */
  static gboolean set_overlay(SpiceChannel *channel, void* pipeline_ptr, 
SpiceDisplay *display)
  {
-#if defined(GDK_WINDOWING_X11)
  SpiceDisplayPrivate *d = display->priv;

+if (pipeline_ptr == NULLg0) {
+gtk_stack_set_visible_child_name(d->stack, "draw-area");
+return true;
+}
+
+#if defined(GDK_WINDOWING_X11)
  /* GstVideoOverlay is currently used only under x */
  if (!g_getenv("DISABLE_GSTVIDEOOVERLAY") &&
  GDK_IS_X11_DISPLAY(gdk_display_get_default())) {
--
2.21.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel