Re: [Xen-devel] [PATCH 2/3] drm/xen-front: Use Xen common shared buffer implementation
Hi, Daniel!
On 11/22/18 4:33 PM, Daniel Vetter wrote:
On Thu, Nov 22, 2018 at 12:02:29PM +0200, Oleksandr Andrushchenko wrote:
From: Oleksandr Andrushchenko
Use page directory based shared buffer implementation
now available as common code for Xen frontend drivers.
Signed-off-by: Oleksandr Andrushchenko
---
drivers/gpu/drm/xen/Kconfig | 1 +
drivers/gpu/drm/xen/Makefile | 1 -
drivers/gpu/drm/xen/xen_drm_front.c | 60 ++--
drivers/gpu/drm/xen/xen_drm_front_gem.c | 1 -
drivers/gpu/drm/xen/xen_drm_front_shbuf.c | 414 --
drivers/gpu/drm/xen/xen_drm_front_shbuf.h | 64
6 files changed, 30 insertions(+), 511 deletions(-)
delete mode 100644 drivers/gpu/drm/xen/xen_drm_front_shbuf.c
delete mode 100644 drivers/gpu/drm/xen/xen_drm_front_shbuf.h
diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig
index 4cca160782ab..f969d486855d 100644
--- a/drivers/gpu/drm/xen/Kconfig
+++ b/drivers/gpu/drm/xen/Kconfig
@@ -12,6 +12,7 @@ config DRM_XEN_FRONTEND
select DRM_KMS_HELPER
select VIDEOMODE_HELPERS
select XEN_XENBUS_FRONTEND
+ select XEN_FRONT_PGDIR_SHBUF
help
Choose this option if you want to enable a para-virtualized
frontend DRM/KMS driver for Xen guest OSes.
diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
index 712afff5ffc3..825905f67faa 100644
--- a/drivers/gpu/drm/xen/Makefile
+++ b/drivers/gpu/drm/xen/Makefile
@@ -4,7 +4,6 @@ drm_xen_front-objs := xen_drm_front.o \
xen_drm_front_kms.o \
xen_drm_front_conn.o \
xen_drm_front_evtchnl.o \
- xen_drm_front_shbuf.o \
xen_drm_front_cfg.o \
xen_drm_front_gem.o
diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c
index 6b6d5ab82ec3..9597544fecc1 100644
--- a/drivers/gpu/drm/xen/xen_drm_front.c
+++ b/drivers/gpu/drm/xen/xen_drm_front.c
@@ -19,6 +19,7 @@
#include
#include
+#include
#include
#include "xen_drm_front.h"
@@ -26,28 +27,20 @@
#include "xen_drm_front_evtchnl.h"
#include "xen_drm_front_gem.h"
#include "xen_drm_front_kms.h"
-#include "xen_drm_front_shbuf.h"
struct xen_drm_front_dbuf {
struct list_head list;
u64 dbuf_cookie;
u64 fb_cookie;
- struct xen_drm_front_shbuf *shbuf;
+
+ struct xen_front_pgdir_shbuf shbuf;
};
-static int dbuf_add_to_list(struct xen_drm_front_info *front_info,
- struct xen_drm_front_shbuf *shbuf, u64 dbuf_cookie)
+static void dbuf_add_to_list(struct xen_drm_front_info *front_info,
+struct xen_drm_front_dbuf *dbuf, u64 dbuf_cookie)
{
- struct xen_drm_front_dbuf *dbuf;
-
- dbuf = kzalloc(sizeof(*dbuf), GFP_KERNEL);
- if (!dbuf)
- return -ENOMEM;
-
dbuf->dbuf_cookie = dbuf_cookie;
- dbuf->shbuf = shbuf;
list_add(>list, _info->dbuf_list);
- return 0;
}
static struct xen_drm_front_dbuf *dbuf_get(struct list_head *dbuf_list,
@@ -64,11 +57,14 @@ static struct xen_drm_front_dbuf *dbuf_get(struct list_head
*dbuf_list,
static void dbuf_flush_fb(struct list_head *dbuf_list, u64 fb_cookie)
{
+#if IS_ENABLED(CONFIG_X86)
struct xen_drm_front_dbuf *buf, *q;
list_for_each_entry_safe(buf, q, dbuf_list, list)
if (buf->fb_cookie == fb_cookie)
- xen_drm_front_shbuf_flush(buf->shbuf);
+ drm_clflush_pages(buf->shbuf.pages,
+ buf->shbuf.num_pages);
+#endif
Why do we need to clflush here only on x86? Feels fairly fishy, but I
think we've discussed this problem for long time with the original
submission already.
First of all sorry for the late response: it took me quite some time
to dig deeper into the flushing issue and better understand the
root cause of this. At the moment my understanding is that this
flushing just hides the real problem and must be removed totally.
So, in v2 of this patch I will remove dbuf_flush_fb.
I am about to start a dedicated discussion on dri-devel with this respect
as I am still concerned about the way this can be solved: I have a suspect
and couple of solutions/workarounds, but I do also need some advise
from DRI community on this.
Anyway, I'm all for code duplication removal, so if the Xen folks are
happy with patch 1, this one here has my ack. Might also be best to merge
all three through the Xen tree. Fallback would be xen folks sending a
topic pull request with these 3 patches to drm-misc and takashi's sound
tree.
-Daniel
}
static void dbuf_free(struct list_head *dbuf_list, u64 dbuf_cookie)
@@ -78,8 +74,8 @@ static void dbuf_free(struct list_head *dbuf_list, u64
dbuf_cookie)
list_for_each_entry_safe(buf, q, dbuf_list, list)
Re: [Xen-devel] [PATCH 2/3] drm/xen-front: Use Xen common shared buffer implementation
On 22/11/2018 15:33, Daniel Vetter wrote: > On Thu, Nov 22, 2018 at 12:02:29PM +0200, Oleksandr Andrushchenko wrote: >> From: Oleksandr Andrushchenko >> >> Use page directory based shared buffer implementation >> now available as common code for Xen frontend drivers. >> >> Signed-off-by: Oleksandr Andrushchenko >> --- >> drivers/gpu/drm/xen/Kconfig | 1 + >> drivers/gpu/drm/xen/Makefile | 1 - >> drivers/gpu/drm/xen/xen_drm_front.c | 60 ++-- >> drivers/gpu/drm/xen/xen_drm_front_gem.c | 1 - >> drivers/gpu/drm/xen/xen_drm_front_shbuf.c | 414 -- >> drivers/gpu/drm/xen/xen_drm_front_shbuf.h | 64 >> 6 files changed, 30 insertions(+), 511 deletions(-) >> delete mode 100644 drivers/gpu/drm/xen/xen_drm_front_shbuf.c >> delete mode 100644 drivers/gpu/drm/xen/xen_drm_front_shbuf.h > Anyway, I'm all for code duplication removal, so if the Xen folks are > happy with patch 1, this one here has my ack. Might also be best to merge > all three through the Xen tree. Fallback would be xen folks sending a > topic pull request with these 3 patches to drm-misc and takashi's sound > tree. I'm fine with taking it through the Xen tree. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/3] drm/xen-front: Use Xen common shared buffer implementation
On Thu, Nov 22, 2018 at 12:02:29PM +0200, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko
>
> Use page directory based shared buffer implementation
> now available as common code for Xen frontend drivers.
>
> Signed-off-by: Oleksandr Andrushchenko
> ---
> drivers/gpu/drm/xen/Kconfig | 1 +
> drivers/gpu/drm/xen/Makefile | 1 -
> drivers/gpu/drm/xen/xen_drm_front.c | 60 ++--
> drivers/gpu/drm/xen/xen_drm_front_gem.c | 1 -
> drivers/gpu/drm/xen/xen_drm_front_shbuf.c | 414 --
> drivers/gpu/drm/xen/xen_drm_front_shbuf.h | 64
> 6 files changed, 30 insertions(+), 511 deletions(-)
> delete mode 100644 drivers/gpu/drm/xen/xen_drm_front_shbuf.c
> delete mode 100644 drivers/gpu/drm/xen/xen_drm_front_shbuf.h
>
> diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig
> index 4cca160782ab..f969d486855d 100644
> --- a/drivers/gpu/drm/xen/Kconfig
> +++ b/drivers/gpu/drm/xen/Kconfig
> @@ -12,6 +12,7 @@ config DRM_XEN_FRONTEND
> select DRM_KMS_HELPER
> select VIDEOMODE_HELPERS
> select XEN_XENBUS_FRONTEND
> + select XEN_FRONT_PGDIR_SHBUF
> help
> Choose this option if you want to enable a para-virtualized
> frontend DRM/KMS driver for Xen guest OSes.
> diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
> index 712afff5ffc3..825905f67faa 100644
> --- a/drivers/gpu/drm/xen/Makefile
> +++ b/drivers/gpu/drm/xen/Makefile
> @@ -4,7 +4,6 @@ drm_xen_front-objs := xen_drm_front.o \
> xen_drm_front_kms.o \
> xen_drm_front_conn.o \
> xen_drm_front_evtchnl.o \
> - xen_drm_front_shbuf.o \
> xen_drm_front_cfg.o \
> xen_drm_front_gem.o
>
> diff --git a/drivers/gpu/drm/xen/xen_drm_front.c
> b/drivers/gpu/drm/xen/xen_drm_front.c
> index 6b6d5ab82ec3..9597544fecc1 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front.c
> @@ -19,6 +19,7 @@
> #include
> #include
>
> +#include
> #include
>
> #include "xen_drm_front.h"
> @@ -26,28 +27,20 @@
> #include "xen_drm_front_evtchnl.h"
> #include "xen_drm_front_gem.h"
> #include "xen_drm_front_kms.h"
> -#include "xen_drm_front_shbuf.h"
>
> struct xen_drm_front_dbuf {
> struct list_head list;
> u64 dbuf_cookie;
> u64 fb_cookie;
> - struct xen_drm_front_shbuf *shbuf;
> +
> + struct xen_front_pgdir_shbuf shbuf;
> };
>
> -static int dbuf_add_to_list(struct xen_drm_front_info *front_info,
> - struct xen_drm_front_shbuf *shbuf, u64 dbuf_cookie)
> +static void dbuf_add_to_list(struct xen_drm_front_info *front_info,
> + struct xen_drm_front_dbuf *dbuf, u64 dbuf_cookie)
> {
> - struct xen_drm_front_dbuf *dbuf;
> -
> - dbuf = kzalloc(sizeof(*dbuf), GFP_KERNEL);
> - if (!dbuf)
> - return -ENOMEM;
> -
> dbuf->dbuf_cookie = dbuf_cookie;
> - dbuf->shbuf = shbuf;
> list_add(>list, _info->dbuf_list);
> - return 0;
> }
>
> static struct xen_drm_front_dbuf *dbuf_get(struct list_head *dbuf_list,
> @@ -64,11 +57,14 @@ static struct xen_drm_front_dbuf *dbuf_get(struct
> list_head *dbuf_list,
>
> static void dbuf_flush_fb(struct list_head *dbuf_list, u64 fb_cookie)
> {
> +#if IS_ENABLED(CONFIG_X86)
> struct xen_drm_front_dbuf *buf, *q;
>
> list_for_each_entry_safe(buf, q, dbuf_list, list)
> if (buf->fb_cookie == fb_cookie)
> - xen_drm_front_shbuf_flush(buf->shbuf);
> + drm_clflush_pages(buf->shbuf.pages,
> + buf->shbuf.num_pages);
> +#endif
Why do we need to clflush here only on x86? Feels fairly fishy, but I
think we've discussed this problem for long time with the original
submission already.
Anyway, I'm all for code duplication removal, so if the Xen folks are
happy with patch 1, this one here has my ack. Might also be best to merge
all three through the Xen tree. Fallback would be xen folks sending a
topic pull request with these 3 patches to drm-misc and takashi's sound
tree.
-Daniel
> }
>
> static void dbuf_free(struct list_head *dbuf_list, u64 dbuf_cookie)
> @@ -78,8 +74,8 @@ static void dbuf_free(struct list_head *dbuf_list, u64
> dbuf_cookie)
> list_for_each_entry_safe(buf, q, dbuf_list, list)
> if (buf->dbuf_cookie == dbuf_cookie) {
> list_del(>list);
> - xen_drm_front_shbuf_unmap(buf->shbuf);
> - xen_drm_front_shbuf_free(buf->shbuf);
> + xen_front_pgdir_shbuf_unmap(>shbuf);
> + xen_front_pgdir_shbuf_free(>shbuf);
> kfree(buf);
> break;
> }
> @@ -91,8 +87,8 @@ static void dbuf_free_all(struct list_head *dbuf_list)
>
>
