Re: [PATCH 6/9] drm/xen-front: Introduce DRM/KMS virtual display driver
On 03/05/2018 11:13 AM, Daniel Vetter wrote:
On Wed, Feb 21, 2018 at 10:03:39AM +0200, Oleksandr Andrushchenko wrote:
From: Oleksandr Andrushchenko
Implement essential initialization of the display driver:
- introduce required data structures
- handle DRM/KMS driver registration
- perform basic DRM driver initialization
- register driver on backend connection
- remove driver on backend disconnect
- introduce essential callbacks required by DRM/KMS core
- introduce essential callbacks required for frontend operations
Signed-off-by: Oleksandr Andrushchenko
---
drivers/gpu/drm/xen/Makefile| 1 +
drivers/gpu/drm/xen/xen_drm_front.c | 169 -
drivers/gpu/drm/xen/xen_drm_front.h | 24
drivers/gpu/drm/xen/xen_drm_front_drv.c | 211
drivers/gpu/drm/xen/xen_drm_front_drv.h | 60 +
5 files changed, 462 insertions(+), 3 deletions(-)
create mode 100644 drivers/gpu/drm/xen/xen_drm_front_drv.c
create mode 100644 drivers/gpu/drm/xen/xen_drm_front_drv.h
diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
index f1823cb596c5..d3068202590f 100644
--- a/drivers/gpu/drm/xen/Makefile
+++ b/drivers/gpu/drm/xen/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
drm_xen_front-objs := xen_drm_front.o \
+ xen_drm_front_drv.o \
xen_drm_front_evtchnl.o \
xen_drm_front_shbuf.o \
xen_drm_front_cfg.o
diff --git a/drivers/gpu/drm/xen/xen_drm_front.c
b/drivers/gpu/drm/xen/xen_drm_front.c
index 0d94ff272da3..8de88e359d5e 100644
--- a/drivers/gpu/drm/xen/xen_drm_front.c
+++ b/drivers/gpu/drm/xen/xen_drm_front.c
@@ -18,6 +18,8 @@
#include
+#include
+
#include
#include
#include
@@ -25,15 +27,161 @@
#include
#include "xen_drm_front.h"
+#include "xen_drm_front_drv.h"
#include "xen_drm_front_evtchnl.h"
#include "xen_drm_front_shbuf.h"
+static int be_mode_set(struct xen_drm_front_drm_pipeline *pipeline, uint32_t x,
+ uint32_t y, uint32_t width, uint32_t height, uint32_t bpp,
+ uint64_t fb_cookie)
+
+{
+ return 0;
+}
+
+static int be_dbuf_create_int(struct xen_drm_front_info *front_info,
+ uint64_t dbuf_cookie, uint32_t width, uint32_t height,
+ uint32_t bpp, uint64_t size, struct page **pages,
+ struct sg_table *sgt)
+{
+ return 0;
+}
+
+static int be_dbuf_create_from_sgt(struct xen_drm_front_info *front_info,
+ uint64_t dbuf_cookie, uint32_t width, uint32_t height,
+ uint32_t bpp, uint64_t size, struct sg_table *sgt)
+{
+ return be_dbuf_create_int(front_info, dbuf_cookie, width, height,
+ bpp, size, NULL, sgt);
+}
+
+static int be_dbuf_create_from_pages(struct xen_drm_front_info *front_info,
+ uint64_t dbuf_cookie, uint32_t width, uint32_t height,
+ uint32_t bpp, uint64_t size, struct page **pages)
+{
+ return be_dbuf_create_int(front_info, dbuf_cookie, width, height,
+ bpp, size, pages, NULL);
+}
+
+static int be_dbuf_destroy(struct xen_drm_front_info *front_info,
+ uint64_t dbuf_cookie)
+{
+ return 0;
+}
+
+static int be_fb_attach(struct xen_drm_front_info *front_info,
+ uint64_t dbuf_cookie, uint64_t fb_cookie, uint32_t width,
+ uint32_t height, uint32_t pixel_format)
+{
+ return 0;
+}
+
+static int be_fb_detach(struct xen_drm_front_info *front_info,
+ uint64_t fb_cookie)
+{
+ return 0;
+}
+
+static int be_page_flip(struct xen_drm_front_info *front_info, int conn_idx,
+ uint64_t fb_cookie)
+{
+ return 0;
+}
+
+static void xen_drm_drv_unload(struct xen_drm_front_info *front_info)
+{
+ if (front_info->xb_dev->state != XenbusStateReconfiguring)
+ return;
+
+ DRM_DEBUG("Can try removing driver now\n");
+ xenbus_switch_state(front_info->xb_dev, XenbusStateInitialising);
+}
+
static struct xen_drm_front_ops front_ops = {
- /* placeholder for now */
+ .mode_set = be_mode_set,
+ .dbuf_create_from_pages = be_dbuf_create_from_pages,
+ .dbuf_create_from_sgt = be_dbuf_create_from_sgt,
+ .dbuf_destroy = be_dbuf_destroy,
+ .fb_attach = be_fb_attach,
+ .fb_detach = be_fb_detach,
+ .page_flip = be_page_flip,
+ .drm_last_close = xen_drm_drv_unload,
+};
This looks like a midlayer/DRM-abstraction in your driver. Please remove,
and instead directly hook your xen-front code into the relevant drm
callbacks.
ok, will do
In general also pls make sure you don't implement dummy callbacks that do
nothing, we've tried really hard to make them all optional in the drm
infrastructure.
sure
-Daniel
+
+static int xen_drm_drv_probe(struct platform_device *pdev)
+{
+ /*
+* The device is not spawn f
Re: [PATCH 6/9] drm/xen-front: Introduce DRM/KMS virtual display driver
On Wed, Feb 21, 2018 at 10:03:39AM +0200, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko
>
> Implement essential initialization of the display driver:
> - introduce required data structures
> - handle DRM/KMS driver registration
> - perform basic DRM driver initialization
> - register driver on backend connection
> - remove driver on backend disconnect
> - introduce essential callbacks required by DRM/KMS core
> - introduce essential callbacks required for frontend operations
>
> Signed-off-by: Oleksandr Andrushchenko
> ---
> drivers/gpu/drm/xen/Makefile| 1 +
> drivers/gpu/drm/xen/xen_drm_front.c | 169 -
> drivers/gpu/drm/xen/xen_drm_front.h | 24
> drivers/gpu/drm/xen/xen_drm_front_drv.c | 211
>
> drivers/gpu/drm/xen/xen_drm_front_drv.h | 60 +
> 5 files changed, 462 insertions(+), 3 deletions(-)
> create mode 100644 drivers/gpu/drm/xen/xen_drm_front_drv.c
> create mode 100644 drivers/gpu/drm/xen/xen_drm_front_drv.h
>
> diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
> index f1823cb596c5..d3068202590f 100644
> --- a/drivers/gpu/drm/xen/Makefile
> +++ b/drivers/gpu/drm/xen/Makefile
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
>
> drm_xen_front-objs := xen_drm_front.o \
> + xen_drm_front_drv.o \
> xen_drm_front_evtchnl.o \
> xen_drm_front_shbuf.o \
> xen_drm_front_cfg.o
> diff --git a/drivers/gpu/drm/xen/xen_drm_front.c
> b/drivers/gpu/drm/xen/xen_drm_front.c
> index 0d94ff272da3..8de88e359d5e 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front.c
> @@ -18,6 +18,8 @@
>
> #include
>
> +#include
> +
> #include
> #include
> #include
> @@ -25,15 +27,161 @@
> #include
>
> #include "xen_drm_front.h"
> +#include "xen_drm_front_drv.h"
> #include "xen_drm_front_evtchnl.h"
> #include "xen_drm_front_shbuf.h"
>
> +static int be_mode_set(struct xen_drm_front_drm_pipeline *pipeline, uint32_t
> x,
> + uint32_t y, uint32_t width, uint32_t height, uint32_t bpp,
> + uint64_t fb_cookie)
> +
> +{
> + return 0;
> +}
> +
> +static int be_dbuf_create_int(struct xen_drm_front_info *front_info,
> + uint64_t dbuf_cookie, uint32_t width, uint32_t height,
> + uint32_t bpp, uint64_t size, struct page **pages,
> + struct sg_table *sgt)
> +{
> + return 0;
> +}
> +
> +static int be_dbuf_create_from_sgt(struct xen_drm_front_info *front_info,
> + uint64_t dbuf_cookie, uint32_t width, uint32_t height,
> + uint32_t bpp, uint64_t size, struct sg_table *sgt)
> +{
> + return be_dbuf_create_int(front_info, dbuf_cookie, width, height,
> + bpp, size, NULL, sgt);
> +}
> +
> +static int be_dbuf_create_from_pages(struct xen_drm_front_info *front_info,
> + uint64_t dbuf_cookie, uint32_t width, uint32_t height,
> + uint32_t bpp, uint64_t size, struct page **pages)
> +{
> + return be_dbuf_create_int(front_info, dbuf_cookie, width, height,
> + bpp, size, pages, NULL);
> +}
> +
> +static int be_dbuf_destroy(struct xen_drm_front_info *front_info,
> + uint64_t dbuf_cookie)
> +{
> + return 0;
> +}
> +
> +static int be_fb_attach(struct xen_drm_front_info *front_info,
> + uint64_t dbuf_cookie, uint64_t fb_cookie, uint32_t width,
> + uint32_t height, uint32_t pixel_format)
> +{
> + return 0;
> +}
> +
> +static int be_fb_detach(struct xen_drm_front_info *front_info,
> + uint64_t fb_cookie)
> +{
> + return 0;
> +}
> +
> +static int be_page_flip(struct xen_drm_front_info *front_info, int conn_idx,
> + uint64_t fb_cookie)
> +{
> + return 0;
> +}
> +
> +static void xen_drm_drv_unload(struct xen_drm_front_info *front_info)
> +{
> + if (front_info->xb_dev->state != XenbusStateReconfiguring)
> + return;
> +
> + DRM_DEBUG("Can try removing driver now\n");
> + xenbus_switch_state(front_info->xb_dev, XenbusStateInitialising);
> +}
> +
> static struct xen_drm_front_ops front_ops = {
> - /* placeholder for now */
> + .mode_set = be_mode_set,
> + .dbuf_create_from_pages = be_dbuf_create_from_pages,
> + .dbuf_create_from_sgt = be_dbuf_create_from_sgt,
> + .dbuf_destroy = be_dbuf_destroy,
> + .fb_attach = be_fb_attach,
> + .fb_detach = be_fb_detach,
> + .page_flip = be_page_flip,
> + .drm_last_close = xen_drm_drv_unload,
> +};
This looks like a midlayer/DRM-abstraction in your driver. Please remove,
and instead directly hook your xen-front code into the relevant drm
callbacks.
In general also pls make sure you don't implement dummy callbacks that do
nothing, we've tried really hard to make them all optional in the drm
infrastructure.
-Daniel
> +
> +static int xen_drm_d
Re: [PATCH 6/9] drm/xen-front: Introduce DRM/KMS virtual display driver
On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:
> +
> +struct drm_driver xen_drm_driver = {
> + .driver_features = DRIVER_GEM | DRIVER_MODESET |
> + DRIVER_PRIME | DRIVER_ATOMIC,
> + .lastclose = lastclose,
> + .gem_free_object_unlocked = free_object,
> + .gem_vm_ops= &xen_drm_vm_ops,
> + .prime_handle_to_fd= drm_gem_prime_handle_to_fd,
> + .prime_fd_to_handle= drm_gem_prime_fd_to_handle,
> + .gem_prime_import = drm_gem_prime_import,
> + .gem_prime_export = drm_gem_prime_export,
> + .gem_prime_get_sg_table= prime_get_sg_table,
> + .gem_prime_import_sg_table = prime_import_sg_table,
> + .gem_prime_vmap= prime_vmap,
> + .gem_prime_vunmap = prime_vunmap,
> + .gem_prime_mmap= prime_mmap,
> + .dumb_create = dumb_create,
> + .fops = &xendrm_fops,
> + .name = "xendrm-du",
> + .desc = "Xen PV DRM Display Unit",
> + .date = "20161109",
You must have been working on this for a while ;-)
I assume this needs to be updated.
> +bool xen_drm_front_drv_is_used(struct platform_device *pdev)
> +{
> + struct xen_drm_front_drm_info *drm_info = platform_get_drvdata(pdev);
> + struct drm_device *dev;
> +
> + if (!drm_info)
> + return false;
> +
> + dev = drm_info->drm_dev;
> + if (!dev)
> + return false;
> +
> + /*
> + * FIXME: the code below must be protected by drm_global_mutex,
> + * but it is not accessible to us. Anyways there is a race condition,
> + * but we will re-try.
> + */
> + return dev->open_count != 0;
Would it be a problem, given the race, if you report that the frontend
is not in use, while it actually is?
-boris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/9] drm/xen-front: Introduce DRM/KMS virtual display driver
On 02/23/2018 05:12 PM, Boris Ostrovsky wrote:
On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:
+
+struct drm_driver xen_drm_driver = {
+ .driver_features = DRIVER_GEM | DRIVER_MODESET |
+DRIVER_PRIME | DRIVER_ATOMIC,
+ .lastclose = lastclose,
+ .gem_free_object_unlocked = free_object,
+ .gem_vm_ops= &xen_drm_vm_ops,
+ .prime_handle_to_fd= drm_gem_prime_handle_to_fd,
+ .prime_fd_to_handle= drm_gem_prime_fd_to_handle,
+ .gem_prime_import = drm_gem_prime_import,
+ .gem_prime_export = drm_gem_prime_export,
+ .gem_prime_get_sg_table= prime_get_sg_table,
+ .gem_prime_import_sg_table = prime_import_sg_table,
+ .gem_prime_vmap= prime_vmap,
+ .gem_prime_vunmap = prime_vunmap,
+ .gem_prime_mmap= prime_mmap,
+ .dumb_create = dumb_create,
+ .fops = &xendrm_fops,
+ .name = "xendrm-du",
+ .desc = "Xen PV DRM Display Unit",
+ .date = "20161109",
You must have been working on this for a while ;-)
yes, this is true ;)
I assume this needs to be updated.
It can be, but I would either stick to the current value
for historical reasons or would update it in the final version
of the driver, so it reflects the date of issuing ;)
+bool xen_drm_front_drv_is_used(struct platform_device *pdev)
+{
+ struct xen_drm_front_drm_info *drm_info = platform_get_drvdata(pdev);
+ struct drm_device *dev;
+
+ if (!drm_info)
+ return false;
+
+ dev = drm_info->drm_dev;
+ if (!dev)
+ return false;
+
+ /*
+* FIXME: the code below must be protected by drm_global_mutex,
+* but it is not accessible to us. Anyways there is a race condition,
+* but we will re-try.
+*/
+ return dev->open_count != 0;
Would it be a problem, given the race, if you report that the frontend
is not in use, while it actually is?
no, backend will not be able to activate us again
-boris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
