On 21/02/18 09:03, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

Introduce skeleton of the para-virtualized Xen display frontend driver. This patch only adds required essential stubs.

Signed-off-by: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/xen/Kconfig | 17 ++++++++ drivers/gpu/drm/xen/Makefile | 5 +++ drivers/gpu/drm/xen/xen_drm_front.c | 83 +++++++++++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+) create mode 100644 drivers/gpu/drm/xen/Kconfig create mode 100644 drivers/gpu/drm/xen/Makefile create mode 100644 drivers/gpu/drm/xen/xen_drm_front.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index deeefa7a1773..757825ac60df 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -289,6 +289,8 @@ source "drivers/gpu/drm/pl111/Kconfig"

source "drivers/gpu/drm/tve200/Kconfig"

+source "drivers/gpu/drm/xen/Kconfig"

# Keep legacy drivers last

menuconfig DRM_LEGACY diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 50093ff4479b..9d66657ea117 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -103,3 +103,4 @@ obj-$(CONFIG_DRM_MXSFB) += mxsfb/ obj-$(CONFIG_DRM_TINYDRM) += tinydrm/ obj-$(CONFIG_DRM_PL111) += pl111/ obj-$(CONFIG_DRM_TVE200) += tve200/ +obj-$(CONFIG_DRM_XEN) += xen/ diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig new file mode 100644 index 000000000000..4cca160782ab --- /dev/null +++ b/drivers/gpu/drm/xen/Kconfig @@ -0,0 +1,17 @@ +config DRM_XEN

• bool "DRM Support for Xen guest OS"
• depends on XEN
• help

•  Choose this option if you want to enable DRM support
  

•  for Xen.
  

+config DRM_XEN_FRONTEND

• tristate "Para-virtualized frontend driver for Xen guest OS"
• depends on DRM_XEN
• depends on DRM
• select DRM_KMS_HELPER
• select VIDEOMODE_HELPERS
• select XEN_XENBUS_FRONTEND
• 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 new file mode 100644 index 000000000000..967074d348f6 --- /dev/null +++ b/drivers/gpu/drm/xen/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0

+drm_xen_front-objs := xen_drm_front.o

+obj-$(CONFIG_DRM_XEN_FRONTEND) += drm_xen_front.o diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c new file mode 100644 index 000000000000..fd372fb464a1 --- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front.c @@ -0,0 +1,83 @@ +/*

• • Xen para-virtual DRM device
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.

Use SPDX identifier instead (same applies for all other new sources):

// SPDX-License-Identifier: GPL-2.0

• • Copyright (C) 2016-2018 EPAM Systems Inc.
• • Author: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com
• */

+#include <drm/drmP.h>

+#include <xen/platform_pci.h> +#include <xen/xen.h> +#include <xen/xenbus.h>

+#include <xen/interface/io/displif.h>

+static void backend_on_changed(struct xenbus_device *xb_dev,

• enum xenbus_state backend_state)
  

+{ +}

+static int xen_drv_probe(struct xenbus_device *xb_dev,

• const struct xenbus_device_id *id)
  

+{

• return 0;

+}

+static int xen_drv_remove(struct xenbus_device *dev) +{

• return 0;

+}

+static const struct xenbus_device_id xen_drv_ids[] = {

• { XENDISPL_DRIVER_NAME },
• { "" }

+};

+static struct xenbus_driver xen_driver = {

• .ids = xen_drv_ids,
• .probe = xen_drv_probe,
• .remove = xen_drv_remove,
• .otherend_changed = backend_on_changed,

+};

+static int __init xen_drv_init(void) +{

• if (!xen_domain())

• return -ENODEV;
  

• if (xen_initial_domain()) {

• DRM_ERROR(XENDISPL_DRIVER_NAME " cannot run in initial domain\n");
  

• return -ENODEV;
  

• }

Why not? Wouldn't that be possible in case of the backend living in a driver domain?

Juergen

On 21/02/18 09:47, Oleksandr Andrushchenko wrote:

On 02/21/2018 10:19 AM, Juergen Gross wrote:

...

On 21/02/18 09:03, Oleksandr Andrushchenko wrote:

...

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

Introduce skeleton of the para-virtualized Xen display frontend driver. This patch only adds required essential stubs.

Signed-off-by: Oleksandr Andrushchenko

oleksandr_andrushchenko@epam.com

drivers/gpu/drm/Kconfig             |  2 +   drivers/gpu/drm/Makefile            |  1 +   drivers/gpu/drm/xen/Kconfig         | 17 ++++++++   drivers/gpu/drm/xen/Makefile        |  5 +++   drivers/gpu/drm/xen/xen_drm_front.c | 83 +++++++++++++++++++++++++++++++++++++   5 files changed, 108 insertions(+)   create mode 100644 drivers/gpu/drm/xen/Kconfig   create mode 100644 drivers/gpu/drm/xen/Makefile   create mode 100644 drivers/gpu/drm/xen/xen_drm_front.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index deeefa7a1773..757825ac60df 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -289,6 +289,8 @@ source "drivers/gpu/drm/pl111/Kconfig"     source "drivers/gpu/drm/tve200/Kconfig"   +source "drivers/gpu/drm/xen/Kconfig"

# Keep legacy drivers last     menuconfig DRM_LEGACY diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 50093ff4479b..9d66657ea117 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -103,3 +103,4 @@ obj-$(CONFIG_DRM_MXSFB)    += mxsfb/   obj-$(CONFIG_DRM_TINYDRM) += tinydrm/   obj-$(CONFIG_DRM_PL111) += pl111/   obj-$(CONFIG_DRM_TVE200) += tve200/ +obj-$(CONFIG_DRM_XEN) += xen/ diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig new file mode 100644 index 000000000000..4cca160782ab --- /dev/null +++ b/drivers/gpu/drm/xen/Kconfig @@ -0,0 +1,17 @@ +config DRM_XEN +    bool "DRM Support for Xen guest OS" +    depends on XEN +    help +      Choose this option if you want to enable DRM support +      for Xen.

+config DRM_XEN_FRONTEND +    tristate "Para-virtualized frontend driver for Xen guest OS" +    depends on DRM_XEN +    depends on DRM +    select DRM_KMS_HELPER +    select VIDEOMODE_HELPERS +    select XEN_XENBUS_FRONTEND +    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 new file mode 100644 index 000000000000..967074d348f6 --- /dev/null +++ b/drivers/gpu/drm/xen/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0

+drm_xen_front-objs := xen_drm_front.o

+obj-$(CONFIG_DRM_XEN_FRONTEND) += drm_xen_front.o diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c new file mode 100644 index 000000000000..fd372fb464a1 --- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front.c @@ -0,0 +1,83 @@ +/*

• *  Xen para-virtual DRM device
• *   This program is free software; you can redistribute it and/or

modify

• *   it under the terms of the GNU General Public License as

published by

• *   the Free Software Foundation; either version 2 of the License, or
• *   (at your option) any later version.
• *   This program is distributed in the hope that it will be useful,
• *   but WITHOUT ANY WARRANTY; without even the implied warranty of
• *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
• *   GNU General Public License for more details.

Use SPDX identifier instead (same applies for all other new sources):

// SPDX-License-Identifier: GPL-2.0

Will update, thank you

...

...

• • Copyright (C) 2016-2018 EPAM Systems Inc.
• • Author: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com
• */

+#include <drm/drmP.h>

+#include <xen/platform_pci.h> +#include <xen/xen.h> +#include <xen/xenbus.h>

+#include <xen/interface/io/displif.h>

+static void backend_on_changed(struct xenbus_device *xb_dev, +        enum xenbus_state backend_state) +{ +}

+static int xen_drv_probe(struct xenbus_device *xb_dev, +        const struct xenbus_device_id *id) +{ +    return 0; +}

+static int xen_drv_remove(struct xenbus_device *dev) +{ +    return 0; +}

+static const struct xenbus_device_id xen_drv_ids[] = { +    { XENDISPL_DRIVER_NAME }, +    { "" } +};

+static struct xenbus_driver xen_driver = { +    .ids = xen_drv_ids, +    .probe = xen_drv_probe, +    .remove = xen_drv_remove, +    .otherend_changed = backend_on_changed, +};

+static int __init xen_drv_init(void) +{ +    if (!xen_domain()) +        return -ENODEV;

+    if (xen_initial_domain()) { +        DRM_ERROR(XENDISPL_DRIVER_NAME " cannot run in initial domain\n"); +        return -ENODEV; +    }

Why not? Wouldn't that be possible in case of the backend living in a driver domain?

It is possible (and in my use-case backend indeed runs in a driver domain). I was just not sure if it is really a good idea to allow that. If you think this is ok, then I'll remove this check

I don't think the driver should decide that. This would be the job of Xen tools IMO.

Juergen

On Wed, Feb 21, 2018 at 10:03:34AM +0200, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

Introduce skeleton of the para-virtualized Xen display frontend driver. This patch only adds required essential stubs.

Signed-off-by: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/xen/Kconfig | 17 ++++++++ drivers/gpu/drm/xen/Makefile | 5 +++ drivers/gpu/drm/xen/xen_drm_front.c | 83 +++++++++++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+) create mode 100644 drivers/gpu/drm/xen/Kconfig create mode 100644 drivers/gpu/drm/xen/Makefile create mode 100644 drivers/gpu/drm/xen/xen_drm_front.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index deeefa7a1773..757825ac60df 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -289,6 +289,8 @@ source "drivers/gpu/drm/pl111/Kconfig"

source "drivers/gpu/drm/tve200/Kconfig"

+source "drivers/gpu/drm/xen/Kconfig"

# Keep legacy drivers last

menuconfig DRM_LEGACY diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 50093ff4479b..9d66657ea117 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -103,3 +103,4 @@ obj-$(CONFIG_DRM_MXSFB) += mxsfb/ obj-$(CONFIG_DRM_TINYDRM) += tinydrm/ obj-$(CONFIG_DRM_PL111) += pl111/ obj-$(CONFIG_DRM_TVE200) += tve200/ +obj-$(CONFIG_DRM_XEN) += xen/ diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig new file mode 100644 index 000000000000..4cca160782ab --- /dev/null +++ b/drivers/gpu/drm/xen/Kconfig @@ -0,0 +1,17 @@ +config DRM_XEN

• bool "DRM Support for Xen guest OS"
• depends on XEN
• help

•  Choose this option if you want to enable DRM support
  

•  for Xen.
  

+config DRM_XEN_FRONTEND

• tristate "Para-virtualized frontend driver for Xen guest OS"
• depends on DRM_XEN
• depends on DRM
• select DRM_KMS_HELPER
• select VIDEOMODE_HELPERS
• select XEN_XENBUS_FRONTEND
• 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 new file mode 100644 index 000000000000..967074d348f6 --- /dev/null +++ b/drivers/gpu/drm/xen/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0

+drm_xen_front-objs := xen_drm_front.o

+obj-$(CONFIG_DRM_XEN_FRONTEND) += drm_xen_front.o diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c new file mode 100644 index 000000000000..fd372fb464a1 --- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front.c @@ -0,0 +1,83 @@ +/*

• • Xen para-virtual DRM device
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.

Most Xen drivers in Linux use a dual GPL/BSD license, so that they can be imported into other non GPL OSes:

This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License version 2 as published by the Free Software Foundation; or, when distributed separately from the Linux kernel or incorporated into other software packages, subject to the following license:

Permission is hereby granted, free of charge, to any person obtaining a copy of this source file (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 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 THE AUTHORS OR COPYRIGHT HOLDERS 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.

IMO it would be good to release this driver under the same license, so it can be incorporated into other OSes.

Thanks, Roger.

On Wed, Feb 21, 2018 at 11:42:23AM +0200, Oleksandr Andrushchenko wrote:

On 02/21/2018 11:17 AM, Roger Pau Monné wrote:

...

On Wed, Feb 21, 2018 at 10:03:34AM +0200, Oleksandr Andrushchenko wrote:

...

--- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front.c @@ -0,0 +1,83 @@ +/*

• • Xen para-virtual DRM device
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.

Most Xen drivers in Linux use a dual GPL/BSD license, so that they can be imported into other non GPL OSes:

This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License version 2 as published by the Free Software Foundation; or, when distributed separately from the Linux kernel or incorporated into other software packages, subject to the following license:

Permission is hereby granted, free of charge, to any person obtaining a copy of this source file (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 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 THE AUTHORS OR COPYRIGHT HOLDERS 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.

IMO it would be good to release this driver under the same license, so it can be incorporated into other OSes.

I am in any way expert in licensing, but the above seems to be /* SPDX-License-Identifier: (GPL-2.0 OR MIT) */ At least this is what I see at [1] for MIT. Could you please tell which license(s) as listed at [1] would be appropriate for Xen drivers in terms of how it is expected to appear in the kernel code, e.g. expected SPDX-License-Identifier?

I would be fine with anything MIT/BSD-*/Apache-* like. In the Xen community we have generally done dual GPL-2.0 MIT, so your proposed tag looks fine IMO (I would also personally be fine with MIT/BSD-*/Apache-* only).

The point is that it would be good to have the code under a more permissive license so it can be integrated into non GPL OSes in the future if needed, and that your code could be used as a reference for that.

Thanks, Roger.

On 02/21/2018 10:23 AM, Juergen Gross wrote:

On 21/02/18 09:03, Oleksandr Andrushchenko wrote:

...

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

Initial handling for Xen bus states: implement Xen bus state machine for the frontend driver according to the state diagram and recovery flow from display para-virtualized protocol: xen/interface/io/displif.h.

Signed-off-by: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

drivers/gpu/drm/xen/xen_drm_front.c | 124 +++++++++++++++++++++++++++++++++++- drivers/gpu/drm/xen/xen_drm_front.h | 26 ++++++++ 2 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/xen/xen_drm_front.h

diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c index fd372fb464a1..d0306f9d660d 100644 --- a/drivers/gpu/drm/xen/xen_drm_front.c +++ b/drivers/gpu/drm/xen/xen_drm_front.c @@ -24,19 +24,141 @@

#include <xen/interface/io/displif.h>

+#include "xen_drm_front.h"

+static void xen_drv_remove_internal(struct xen_drm_front_info *front_info) +{ +}

+static int backend_on_initwait(struct xen_drm_front_info *front_info) +{

• return 0;

+}

+static int backend_on_connected(struct xen_drm_front_info *front_info) +{

• return 0;

+}

+static void backend_on_disconnected(struct xen_drm_front_info *front_info) +{

• xenbus_switch_state(front_info->xb_dev, XenbusStateInitialising);

+}

• static void backend_on_changed(struct xenbus_device *xb_dev, enum xenbus_state backend_state) {

• struct xen_drm_front_info *front_info = dev_get_drvdata(&xb_dev->dev);

• int ret;

• DRM_DEBUG("Backend state is %s, front is %s\n",

• 	xenbus_strstate(backend_state),
  

• 	xenbus_strstate(xb_dev->state));
  

• switch (backend_state) {

• case XenbusStateReconfiguring:

• /* fall through */
  

• case XenbusStateReconfigured:

• /* fall through */
  

• case XenbusStateInitialised:

• break;
  

• case XenbusStateInitialising:

• /* recovering after backend unexpected closure */
  

• backend_on_disconnected(front_info);
  

• break;
  

• case XenbusStateInitWait:

• /* recovering after backend unexpected closure */
  

• backend_on_disconnected(front_info);
  

• if (xb_dev->state != XenbusStateInitialising)
  

• 	break;
  

• ret = backend_on_initwait(front_info);
  

• if (ret < 0)
  

• 	xenbus_dev_fatal(xb_dev, ret, "initializing frontend");
  

• else
  

• 	xenbus_switch_state(xb_dev, XenbusStateInitialised);
  

• break;
  

• case XenbusStateConnected:

• if (xb_dev->state != XenbusStateInitialised)
  

• 	break;
  

• ret = backend_on_connected(front_info);
  

• if (ret < 0)
  

• 	xenbus_dev_fatal(xb_dev, ret, "initializing DRM driver");
  

• else
  

• 	xenbus_switch_state(xb_dev, XenbusStateConnected);
  

• break;
  

• case XenbusStateClosing:

• /*
  

•  * in this state backend starts freeing resources,
  

•  * so let it go into closed state, so we can also
  

•  * remove ours
  

•  */
  

• break;
  

• case XenbusStateUnknown:

• /* fall through */
  

• case XenbusStateClosed:

• if (xb_dev->state == XenbusStateClosed)
  

• 	break;
  

• backend_on_disconnected(front_info);
  

• break;
  

• } }

  static int xen_drv_probe(struct xenbus_device *xb_dev, const struct xenbus_device_id *id) {

• return 0;

• struct xen_drm_front_info *front_info;
• front_info = devm_kzalloc(&xb_dev->dev,

• 	sizeof(*front_info), GFP_KERNEL);
  

• if (!front_info) {

• xenbus_dev_fatal(xb_dev, -ENOMEM, "allocating device memory");
  

No need for message in case of allocation failure: this is handled in memory allocation already.

Will remove, thank you

Juergen

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 oleksandr_andrushchenko@epam.com

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 oleksandr_andrushchenko@epam.com

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 <drm/drmP.h>

+#include <linux/of_device.h>

• #include <xen/platform_pci.h> #include <xen/xen.h> #include <xen/xenbus.h>

@@ -25,15 +27,161 @@ #include <xen/interface/io/displif.h>

#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 from a device tree, so arch_setup_dma_ops
  

• * is not called, thus leaving the device with dummy DMA ops.
  

• * This makes the device return error on PRIME buffer import, which
  

• * is not correct: to fix this call of_dma_configure() with a NULL
  

• * node to set default DMA ops.
  

• */
  

• of_dma_configure(&pdev->dev, NULL);
• return xen_drm_front_drv_probe(pdev, &front_ops);

+}

+static int xen_drm_drv_remove(struct platform_device *pdev) +{

• return xen_drm_front_drv_remove(pdev);

+}

+struct platform_device_info xen_drm_front_platform_info = {

• .name = XENDISPL_DRIVER_NAME,
• .id = 0,
• .num_res = 0,
• .dma_mask = DMA_BIT_MASK(32), };

+static struct platform_driver xen_drm_front_front_info = {

• .probe = xen_drm_drv_probe,
• .remove = xen_drm_drv_remove,
• .driver = {

• .name	= XENDISPL_DRIVER_NAME,
  

• },

+};

+static void xen_drm_drv_deinit(struct xen_drm_front_info *front_info) +{

• if (!front_info->drm_pdrv_registered)

• return;
  

• if (front_info->drm_pdev)

• platform_device_unregister(front_info->drm_pdev);
  

• platform_driver_unregister(&xen_drm_front_front_info);
• front_info->drm_pdrv_registered = false;
• front_info->drm_pdev = NULL;

+}

+static int xen_drm_drv_init(struct xen_drm_front_info *front_info) +{

• int ret;
• ret = platform_driver_register(&xen_drm_front_front_info);
• if (ret < 0)

• return ret;
  

• front_info->drm_pdrv_registered = true;
• /* pass card configuration via platform data */
• xen_drm_front_platform_info.data = &front_info->cfg;
• xen_drm_front_platform_info.size_data = sizeof(front_info->cfg);
• front_info->drm_pdev = platform_device_register_full(

• 	&xen_drm_front_platform_info);
  

• if (IS_ERR_OR_NULL(front_info->drm_pdev)) {

• DRM_ERROR("Failed to register " XENDISPL_DRIVER_NAME " PV DRM driver\n");
  

• front_info->drm_pdev = NULL;
  

• xen_drm_drv_deinit(front_info);
  

• return -ENODEV;
  

• }
• return 0;

+}

• static void xen_drv_remove_internal(struct xen_drm_front_info *front_info) {
• xen_drm_drv_deinit(front_info); xen_drm_front_evtchnl_free_all(front_info); }

@@ -59,13 +207,27 @@ static int backend_on_initwait(struct xen_drm_front_info *front_info) static int backend_on_connected(struct xen_drm_front_info *front_info) { xen_drm_front_evtchnl_set_state(front_info, EVTCHNL_STATE_CONNECTED);

• return 0;

• return xen_drm_drv_init(front_info); }

  static void backend_on_disconnected(struct xen_drm_front_info *front_info) {

• bool removed = true;

• if (front_info->drm_pdev) {

• if (xen_drm_front_drv_is_used(front_info->drm_pdev)) {
  

• 	DRM_WARN("DRM driver still in use, deferring removal\n");
  

• 	removed = false;
  

• } else
  

• 	xen_drv_remove_internal(front_info);
  

• }

• xen_drm_front_evtchnl_set_state(front_info, EVTCHNL_STATE_DISCONNECTED);

• xenbus_switch_state(front_info->xb_dev, XenbusStateInitialising);

• if (removed)

• xenbus_switch_state(front_info->xb_dev, XenbusStateInitialising);
  

• else

• xenbus_switch_state(front_info->xb_dev, XenbusStateReconfiguring);
  

  }

  static void backend_on_changed(struct xenbus_device *xb_dev,

@@ -148,6 +310,7 @@ static int xen_drv_probe(struct xenbus_device *xb_dev,

front_info->xb_dev = xb_dev; spin_lock_init(&front_info->io_lock);

• front_info->drm_pdrv_registered = false; dev_set_drvdata(&xb_dev->dev, front_info); return xenbus_switch_state(xb_dev, XenbusStateInitialising); }

diff --git a/drivers/gpu/drm/xen/xen_drm_front.h b/drivers/gpu/drm/xen/xen_drm_front.h index 13f22736ae02..9ed5bfb248d0 100644 --- a/drivers/gpu/drm/xen/xen_drm_front.h +++ b/drivers/gpu/drm/xen/xen_drm_front.h @@ -19,6 +19,8 @@ #ifndef __XEN_DRM_FRONT_H_ #define __XEN_DRM_FRONT_H_

+#include <linux/scatterlist.h>

• #include "xen_drm_front_cfg.h"

  #ifndef GRANT_INVALID_REF

@@ -30,16 +32,38 @@ #define GRANT_INVALID_REF 0 #endif

+struct xen_drm_front_drm_pipeline;

• struct xen_drm_front_ops {

• int (*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);
  

• int (*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);
  

• int (*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);
  

• int (*dbuf_destroy)(struct xen_drm_front_info *front_info,

• 	uint64_t dbuf_cookie);
  

• int (*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);
  

• int (*fb_detach)(struct xen_drm_front_info *front_info,

• 	uint64_t fb_cookie);
  

• int (*page_flip)(struct xen_drm_front_info *front_info,

• 	int conn_idx, uint64_t fb_cookie);
  

  /* CAUTION! this is called with a spin_lock held! */ void (*on_frame_done)(struct platform_device *pdev, int conn_idx, uint64_t fb_cookie);

• void (*drm_last_close)(struct xen_drm_front_info *front_info); };

  struct xen_drm_front_info { struct xenbus_device *xb_dev; /* to protect data between backend IO code and interrupt handler */ spinlock_t io_lock;

• bool drm_pdrv_registered; /* virtual DRM platform device */ struct platform_device *drm_pdev;

diff --git a/drivers/gpu/drm/xen/xen_drm_front_drv.c b/drivers/gpu/drm/xen/xen_drm_front_drv.c new file mode 100644 index 000000000000..b3764d5ed0f6 --- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front_drv.c @@ -0,0 +1,211 @@ +/*

• • Xen para-virtual DRM device
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • Copyright (C) 2016-2018 EPAM Systems Inc.
• • Author: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com
• */

+#include <drm/drmP.h> +#include <drm/drm_gem.h> +#include <drm/drm_atomic_helper.h>

+#include "xen_drm_front.h" +#include "xen_drm_front_cfg.h" +#include "xen_drm_front_drv.h"

+static int dumb_create(struct drm_file *filp,

• struct drm_device *dev, struct drm_mode_create_dumb *args)
  

+{

• return -EINVAL;

+}

+static void free_object(struct drm_gem_object *obj) +{

• struct xen_drm_front_drm_info *drm_info = obj->dev->dev_private;
• drm_info->front_ops->dbuf_destroy(drm_info->front_info,

• 	xen_drm_front_dbuf_to_cookie(obj));
  

+}

+static void on_frame_done(struct platform_device *pdev,

• int conn_idx, uint64_t fb_cookie)
  

+{ +}

+static void lastclose(struct drm_device *dev) +{

• struct xen_drm_front_drm_info *drm_info = dev->dev_private;
• drm_info->front_ops->drm_last_close(drm_info->front_info);

+}

+static int gem_mmap(struct file *filp, struct vm_area_struct *vma) +{

• return -EINVAL;

+}

+static struct sg_table *prime_get_sg_table(struct drm_gem_object *obj) +{

• return NULL;

+}

+static struct drm_gem_object *prime_import_sg_table(struct drm_device *dev,

• struct dma_buf_attachment *attach, struct sg_table *sgt)
  

+{

• return NULL;

+}

+static void *prime_vmap(struct drm_gem_object *obj) +{

• return NULL;

+}

+static void prime_vunmap(struct drm_gem_object *obj, void *vaddr) +{ +}

+static int prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) +{

• return -EINVAL;

+}

+static const struct file_operations xendrm_fops = {

• .owner = THIS_MODULE,
• .open = drm_open,
• .release = drm_release,
• .unlocked_ioctl = drm_ioctl,

+#ifdef CONFIG_COMPAT

• .compat_ioctl = drm_compat_ioctl,

+#endif

• .poll = drm_poll,
• .read = drm_read,
• .llseek = no_llseek,
• .mmap = gem_mmap,

+};

+static const struct vm_operations_struct xen_drm_vm_ops = {

• .open = drm_gem_vm_open,
• .close = drm_gem_vm_close,

+};

+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",
• .major = 1,
• .minor = 0,

+};

+int xen_drm_front_drv_probe(struct platform_device *pdev,

• struct xen_drm_front_ops *front_ops)
  

+{

• struct xen_drm_front_cfg *cfg = dev_get_platdata(&pdev->dev);
• struct xen_drm_front_drm_info *drm_info;
• struct drm_device *dev;
• int ret;
• DRM_INFO("Creating %s\n", xen_drm_driver.desc);
• drm_info = devm_kzalloc(&pdev->dev, sizeof(*drm_info), GFP_KERNEL);
• if (!drm_info)

• return -ENOMEM;
  

• drm_info->front_ops = front_ops;
• drm_info->front_ops->on_frame_done = on_frame_done;
• drm_info->front_info = cfg->front_info;
• dev = drm_dev_alloc(&xen_drm_driver, &pdev->dev);
• if (!dev)

• return -ENOMEM;
  

• drm_info->drm_dev = dev;
• drm_info->cfg = cfg;
• dev->dev_private = drm_info;
• platform_set_drvdata(pdev, drm_info);
• ret = drm_vblank_init(dev, cfg->num_connectors);
• if (ret) {

• DRM_ERROR("Failed to initialize vblank, ret %d\n", ret);
  

• return ret;
  

• }
• dev->irq_enabled = 1;
• ret = drm_dev_register(dev, 0);
• if (ret)

• goto fail_register;
  

• DRM_INFO("Initialized %s %d.%d.%d %s on minor %d\n",

• 	xen_drm_driver.name, xen_drm_driver.major,
  

• 	xen_drm_driver.minor, xen_drm_driver.patchlevel,
  

• 	xen_drm_driver.date, dev->primary->index);
  

• return 0;

+fail_register:

• drm_dev_unregister(dev);
• drm_mode_config_cleanup(dev);
• return ret;

+}

+int xen_drm_front_drv_remove(struct platform_device *pdev) +{

• struct xen_drm_front_drm_info *drm_info = platform_get_drvdata(pdev);
• struct drm_device *dev = drm_info->drm_dev;
• if (dev) {

• drm_dev_unregister(dev);
  

• drm_atomic_helper_shutdown(dev);
  

• drm_mode_config_cleanup(dev);
  

• drm_dev_unref(dev);
  

• }
• return 0;

+}

+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;

+} diff --git a/drivers/gpu/drm/xen/xen_drm_front_drv.h b/drivers/gpu/drm/xen/xen_drm_front_drv.h new file mode 100644 index 000000000000..aaa476535c13 --- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front_drv.h @@ -0,0 +1,60 @@ +/*

• • Xen para-virtual DRM device
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • Copyright (C) 2016-2018 EPAM Systems Inc.
• • Author: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com
• */

+#ifndef __XEN_DRM_FRONT_DRV_H_ +#define __XEN_DRM_FRONT_DRV_H_

+#include <drm/drmP.h>

+#include "xen_drm_front.h" +#include "xen_drm_front_cfg.h"

+struct xen_drm_front_drm_pipeline {

• struct xen_drm_front_drm_info *drm_info;
• int index;

+};

+struct xen_drm_front_drm_info {

• struct xen_drm_front_info *front_info;
• struct xen_drm_front_ops *front_ops;
• struct drm_device *drm_dev;
• struct xen_drm_front_cfg *cfg;

+};

+static inline uint64_t xen_drm_front_fb_to_cookie(

• struct drm_framebuffer *fb)
  

+{

• return (uint64_t)fb;

+}

+static inline uint64_t xen_drm_front_dbuf_to_cookie(

• struct drm_gem_object *gem_obj)
  

+{

• return (uint64_t)gem_obj;

+}

+int xen_drm_front_drv_probe(struct platform_device *pdev,

• struct xen_drm_front_ops *front_ops);
  

+int xen_drm_front_drv_remove(struct platform_device *pdev);

+bool xen_drm_front_drv_is_used(struct platform_device *pdev);

+#endif /* __XEN_DRM_FRONT_DRV_H_ */

-- 2.7.4

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel

On Wed, Feb 21, 2018 at 10:03:40AM +0200, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

Implement kernel modesetiing/connector handling using DRM simple KMS helper pipeline:

• implement KMS part of the driver with the help of DRM simple pipepline helper which is possible due to the fact that the para-virtualized driver only supports a single (primary) plane:

  • initialize connectors according to XenStore configuration
  • handle frame done events from the backend
  • generate vblank events
  • create and destroy frame buffers and propagate those to the backend
  • propagate set/reset mode configuration to the backend on display enable/disable callbacks
  • send page flip request to the backend and implement logic for reporting backend IO errors on prepare fb callback

• implement virtual connector handling:

  • support only pixel formats suitable for single plane modes
  • make sure the connector is always connected
  • support a single video mode as per para-virtualized driver configuration

Signed-off-by: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

I think once you've removed the midlayer in the previous patch it would makes sense to merge the 2 patches into 1.

Bunch more comments below. -Daniel

---

drivers/gpu/drm/xen/Makefile | 2 + drivers/gpu/drm/xen/xen_drm_front_conn.c | 125 +++++++++++++ drivers/gpu/drm/xen/xen_drm_front_conn.h | 35 ++++ drivers/gpu/drm/xen/xen_drm_front_drv.c | 15 ++ drivers/gpu/drm/xen/xen_drm_front_drv.h | 12 ++ drivers/gpu/drm/xen/xen_drm_front_kms.c | 299 +++++++++++++++++++++++++++++++ drivers/gpu/drm/xen/xen_drm_front_kms.h | 30 ++++ 7 files changed, 518 insertions(+) create mode 100644 drivers/gpu/drm/xen/xen_drm_front_conn.c create mode 100644 drivers/gpu/drm/xen/xen_drm_front_conn.h create mode 100644 drivers/gpu/drm/xen/xen_drm_front_kms.c create mode 100644 drivers/gpu/drm/xen/xen_drm_front_kms.h

diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile index d3068202590f..4fcb0da1a9c5 100644 --- a/drivers/gpu/drm/xen/Makefile +++ b/drivers/gpu/drm/xen/Makefile @@ -2,6 +2,8 @@

drm_xen_front-objs := xen_drm_front.o \ xen_drm_front_drv.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
  

diff --git a/drivers/gpu/drm/xen/xen_drm_front_conn.c b/drivers/gpu/drm/xen/xen_drm_front_conn.c new file mode 100644 index 000000000000..d9986a2e1a3b --- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front_conn.c @@ -0,0 +1,125 @@ +/*

• • Xen para-virtual DRM device
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • Copyright (C) 2016-2018 EPAM Systems Inc.
• • Author: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com
• */

+#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h>

+#include <video/videomode.h>

+#include "xen_drm_front_conn.h" +#include "xen_drm_front_drv.h"

+static struct xen_drm_front_drm_pipeline * +to_xen_drm_pipeline(struct drm_connector *connector) +{

• return container_of(connector, struct xen_drm_front_drm_pipeline, conn);

+}

+static const uint32_t plane_formats[] = {

• DRM_FORMAT_RGB565,
• DRM_FORMAT_RGB888,
• DRM_FORMAT_XRGB8888,
• DRM_FORMAT_ARGB8888,
• DRM_FORMAT_XRGB4444,
• DRM_FORMAT_ARGB4444,
• DRM_FORMAT_XRGB1555,
• DRM_FORMAT_ARGB1555,

+};

+const uint32_t *xen_drm_front_conn_get_formats(int *format_count) +{

• *format_count = ARRAY_SIZE(plane_formats);
• return plane_formats;

+}

+static enum drm_connector_status connector_detect(

• struct drm_connector *connector, bool force)
  

+{

• if (drm_dev_is_unplugged(connector->dev))

• return connector_status_disconnected;
  

• return connector_status_connected;

+}

+#define XEN_DRM_NUM_VIDEO_MODES 1 +#define XEN_DRM_CRTC_VREFRESH_HZ 60

+static int connector_get_modes(struct drm_connector *connector) +{

• struct xen_drm_front_drm_pipeline *pipeline =

• to_xen_drm_pipeline(connector);
  

• struct drm_display_mode *mode;
• struct videomode videomode;
• int width, height;
• mode = drm_mode_create(connector->dev);
• if (!mode)

• return 0;
  

• memset(&videomode, 0, sizeof(videomode));
• videomode.hactive = pipeline->width;
• videomode.vactive = pipeline->height;
• width = videomode.hactive + videomode.hfront_porch +

• videomode.hback_porch + videomode.hsync_len;
  

• height = videomode.vactive + videomode.vfront_porch +

• videomode.vback_porch + videomode.vsync_len;
  

• videomode.pixelclock = width * height * XEN_DRM_CRTC_VREFRESH_HZ;
• mode->type = DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER;
• drm_display_mode_from_videomode(&videomode, mode);
• drm_mode_probed_add(connector, mode);
• return XEN_DRM_NUM_VIDEO_MODES;

+}

+static int connector_mode_valid(struct drm_connector *connector,

• struct drm_display_mode *mode)
  

+{

• struct xen_drm_front_drm_pipeline *pipeline =

• 	to_xen_drm_pipeline(connector);
  

• if (mode->hdisplay != pipeline->width)

• return MODE_ERROR;
  

• if (mode->vdisplay != pipeline->height)

• return MODE_ERROR;
  

• return MODE_OK;

+}

+static const struct drm_connector_helper_funcs connector_helper_funcs = {

• .get_modes = connector_get_modes,
• .mode_valid = connector_mode_valid,

+};

+static const struct drm_connector_funcs connector_funcs = {

• .detect = connector_detect,
• .fill_modes = drm_helper_probe_single_connector_modes,
• .destroy = drm_connector_cleanup,
• .reset = drm_atomic_helper_connector_reset,
• .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
• .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,

+};

+int xen_drm_front_conn_init(struct xen_drm_front_drm_info *drm_info,

• struct drm_connector *connector)
  

+{

• drm_connector_helper_add(connector, &connector_helper_funcs);
• return drm_connector_init(drm_info->drm_dev, connector,

• &connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL);
  

+} diff --git a/drivers/gpu/drm/xen/xen_drm_front_conn.h b/drivers/gpu/drm/xen/xen_drm_front_conn.h new file mode 100644 index 000000000000..708e80d45985 --- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front_conn.h @@ -0,0 +1,35 @@ +/*

• • Xen para-virtual DRM device
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • Copyright (C) 2016-2018 EPAM Systems Inc.
• • Author: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com
• */

+#ifndef __XEN_DRM_FRONT_CONN_H_ +#define __XEN_DRM_FRONT_CONN_H_

+#include <drm/drmP.h> +#include <drm/drm_crtc.h> +#include <drm/drm_encoder.h>

+#include <linux/wait.h>

+struct xen_drm_front_drm_info;

+const uint32_t *xen_drm_front_conn_get_formats(int *format_count);

+int xen_drm_front_conn_init(struct xen_drm_front_drm_info *drm_info,

• struct drm_connector *connector);
  

+#endif /* __XEN_DRM_FRONT_CONN_H_ */ diff --git a/drivers/gpu/drm/xen/xen_drm_front_drv.c b/drivers/gpu/drm/xen/xen_drm_front_drv.c index b3764d5ed0f6..e8862d26ba27 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_drv.c +++ b/drivers/gpu/drm/xen/xen_drm_front_drv.c @@ -23,6 +23,7 @@ #include "xen_drm_front.h" #include "xen_drm_front_cfg.h" #include "xen_drm_front_drv.h" +#include "xen_drm_front_kms.h"

static int dumb_create(struct drm_file *filp, struct drm_device *dev, struct drm_mode_create_dumb *args) @@ -41,6 +42,13 @@ static void free_object(struct drm_gem_object *obj) static void on_frame_done(struct platform_device *pdev, int conn_idx, uint64_t fb_cookie) {

• struct xen_drm_front_drm_info *drm_info = platform_get_drvdata(pdev);
• if (unlikely(conn_idx >= drm_info->cfg->num_connectors))

• return;
  

• xen_drm_front_kms_on_frame_done(&drm_info->pipeline[conn_idx],

• 	fb_cookie);
  

}

static void lastclose(struct drm_device *dev) @@ -157,6 +165,12 @@ int xen_drm_front_drv_probe(struct platform_device *pdev, return ret; }

• ret = xen_drm_front_kms_init(drm_info);

• if (ret) {

• DRM_ERROR("Failed to initialize DRM/KMS, ret %d\n", ret);
  

• goto fail_modeset;
  

• }

• dev->irq_enabled = 1;

  ret = drm_dev_register(dev, 0);

@@ -172,6 +186,7 @@ int xen_drm_front_drv_probe(struct platform_device *pdev,

fail_register: drm_dev_unregister(dev); +fail_modeset: drm_mode_config_cleanup(dev); return ret; } diff --git a/drivers/gpu/drm/xen/xen_drm_front_drv.h b/drivers/gpu/drm/xen/xen_drm_front_drv.h index aaa476535c13..563318b19f34 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_drv.h +++ b/drivers/gpu/drm/xen/xen_drm_front_drv.h @@ -20,14 +20,24 @@ #define __XEN_DRM_FRONT_DRV_H_

#include <drm/drmP.h> +#include <drm/drm_simple_kms_helper.h>

#include "xen_drm_front.h" #include "xen_drm_front_cfg.h" +#include "xen_drm_front_conn.h"

struct xen_drm_front_drm_pipeline { struct xen_drm_front_drm_info *drm_info;

int index;

• struct drm_simple_display_pipe pipe;
• struct drm_connector conn;
• /* these are only for connector mode checking */
• int width, height;
• /* last backend error seen on page flip */
• int pgflip_last_error;

};

struct xen_drm_front_drm_info { @@ -35,6 +45,8 @@ struct xen_drm_front_drm_info { struct xen_drm_front_ops *front_ops; struct drm_device *drm_dev; struct xen_drm_front_cfg *cfg;

• struct xen_drm_front_drm_pipeline pipeline[XEN_DRM_FRONT_MAX_CRTCS];

};

static inline uint64_t xen_drm_front_fb_to_cookie( diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c new file mode 100644 index 000000000000..ad94c28835cd --- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c @@ -0,0 +1,299 @@ +/*

• • Xen para-virtual DRM device
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • Copyright (C) 2016-2018 EPAM Systems Inc.
• • Author: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com
• */

+#include "xen_drm_front_kms.h"

+#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_gem.h> +#include <drm/drm_gem_framebuffer_helper.h>

+#include "xen_drm_front.h" +#include "xen_drm_front_conn.h" +#include "xen_drm_front_drv.h"

+static struct xen_drm_front_drm_pipeline * +to_xen_drm_pipeline(struct drm_simple_display_pipe *pipe) +{

• return container_of(pipe, struct xen_drm_front_drm_pipeline, pipe);

+}

+static void fb_destroy(struct drm_framebuffer *fb) +{

• struct xen_drm_front_drm_info *drm_info = fb->dev->dev_private;
• drm_info->front_ops->fb_detach(drm_info->front_info,

• 	xen_drm_front_fb_to_cookie(fb));
  

• drm_gem_fb_destroy(fb);

+}

+static struct drm_framebuffer_funcs fb_funcs = {

• .destroy = fb_destroy,

+};

+static struct drm_framebuffer *fb_create(struct drm_device *dev,

• struct drm_file *filp, const struct drm_mode_fb_cmd2 *mode_cmd)
  

+{

• struct xen_drm_front_drm_info *drm_info = dev->dev_private;
• static struct drm_framebuffer *fb;
• struct drm_gem_object *gem_obj;
• int ret;
• fb = drm_gem_fb_create_with_funcs(dev, filp, mode_cmd, &fb_funcs);
• if (IS_ERR_OR_NULL(fb))

• return fb;
  

• gem_obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
• if (!gem_obj) {

• DRM_ERROR("Failed to lookup GEM object\n");
  

• ret = -ENOENT;
  

• goto fail;
  

• }
• drm_gem_object_unreference_unlocked(gem_obj);
• ret = drm_info->front_ops->fb_attach(

• 	drm_info->front_info,
  

• 	xen_drm_front_dbuf_to_cookie(gem_obj),
  

• 	xen_drm_front_fb_to_cookie(fb),
  

• 	fb->width, fb->height, fb->format->format);
  

• if (ret < 0) {

• DRM_ERROR("Back failed to attach FB %p: %d\n", fb, ret);
  

• goto fail;
  

• }
• return fb;

+fail:

• drm_gem_fb_destroy(fb);
• return ERR_PTR(ret);

+}

+static const struct drm_mode_config_funcs mode_config_funcs = {

• .fb_create = fb_create,
• .atomic_check = drm_atomic_helper_check,
• .atomic_commit = drm_atomic_helper_commit,

+};

+static int display_set_config(struct drm_simple_display_pipe *pipe,

• struct drm_framebuffer *fb)

+{

• struct xen_drm_front_drm_pipeline *pipeline =

• 	to_xen_drm_pipeline(pipe);
  

• struct drm_crtc *crtc = &pipe->crtc;
• struct xen_drm_front_drm_info *drm_info = pipeline->drm_info;
• int ret;
• if (fb)

• ret = drm_info->front_ops->mode_set(pipeline,
  

• 		crtc->x, crtc->y,
  

• 		fb->width, fb->height, fb->format->cpp[0] * 8,
  

• 		xen_drm_front_fb_to_cookie(fb));
  

• else

• ret = drm_info->front_ops->mode_set(pipeline,
  

• 		0, 0, 0, 0, 0,
  

• 		xen_drm_front_fb_to_cookie(NULL));
  

This is a bit much layering, the if (fb) case corresponds to the display_enable/disable hooks, pls fold that in instead of the indirection. simple helpers guarantee that when the display is on, then you have an fb.

Maybe we need to fix the docs, pls check and if that's not clear, submit a kernel-doc patch for the simple pipe helpers.

• if (ret)

• DRM_ERROR("Failed to set mode to back: %d\n", ret);
  

• return ret;

+}

+static void display_enable(struct drm_simple_display_pipe *pipe,

• struct drm_crtc_state *crtc_state)
  

+{

• struct drm_crtc *crtc = &pipe->crtc;
• struct drm_framebuffer *fb = pipe->plane.state->fb;
• if (display_set_config(pipe, fb) == 0)

• drm_crtc_vblank_on(crtc);
  

I get the impression your driver doesn't support vblanks (the page flip code at least looks like it's only generating a single event), you also don't have a enable/disable_vblank implementation. If there's no vblank handling then this shouldn't be needed.

• else

• DRM_ERROR("Failed to enable display\n");
  

+}

+static void display_disable(struct drm_simple_display_pipe *pipe) +{

• struct drm_crtc *crtc = &pipe->crtc;
• display_set_config(pipe, NULL);
• drm_crtc_vblank_off(crtc);
• /* final check for stalled events */
• if (crtc->state->event && !crtc->state->active) {

• unsigned long flags;
  

• spin_lock_irqsave(&crtc->dev->event_lock, flags);
  

• drm_crtc_send_vblank_event(crtc, crtc->state->event);
  

• spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
  

• crtc->state->event = NULL;
  

• }

+}

+void xen_drm_front_kms_on_frame_done(

• struct xen_drm_front_drm_pipeline *pipeline,
  

• uint64_t fb_cookie)
  

+{

• drm_crtc_handle_vblank(&pipeline->pipe.crtc);

Hm, again this doesn't look like real vblank, but only a page-flip done event. If that's correct then please don't use the vblank machinery, but just store the event internally (protected with your own private spinlock) and send it out using drm_crtc_send_vblank_event directly. No calls to arm_vblank_event or any of the other vblank infrastructure should be needed.

Also please remove the drm_vblank_init() call, since your hw doesn't really have vblanks. And exposing vblanks to userspace without implementing them is confusing.

+}

+static void display_send_page_flip(struct drm_simple_display_pipe *pipe,

• struct drm_plane_state *old_plane_state)
  

+{

• struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(

• 	old_plane_state->state, &pipe->plane);
  

• /*

• * If old_plane_state->fb is NULL and plane_state->fb is not,
  

• * then this is an atomic commit which will enable display.
  

• * If old_plane_state->fb is not NULL and plane_state->fb is,
  

• * then this is an atomic commit which will disable display.
  

• * Ignore these and do not send page flip as this framebuffer will be
  

• * sent to the backend as a part of display_set_config call.
  

• */
  

• if (old_plane_state->fb && plane_state->fb) {

• struct xen_drm_front_drm_pipeline *pipeline =
  

• 		to_xen_drm_pipeline(pipe);
  

• struct xen_drm_front_drm_info *drm_info = pipeline->drm_info;
  

• int ret;
  

• ret = drm_info->front_ops->page_flip(drm_info->front_info,
  

• 		pipeline->index,
  

• 		xen_drm_front_fb_to_cookie(plane_state->fb));
  

• pipeline->pgflip_last_error = ret;
  

• if (ret) {
  

• 	DRM_ERROR("Failed to send page flip request to backend: %d\n", ret);
  

• 	/*
  

• 	 * As we are at commit stage the DRM core will anyways
  

• 	 * wait for the vblank and knows nothing about our
  

• 	 * failure. The best we can do is to handle
  

• 	 * vblank now, so there is no vblank/flip_done
  

• 	 * time outs
  

• 	 */
  

• 	drm_crtc_handle_vblank(&pipeline->pipe.crtc);
  

• }
  

• }

+}

+static int display_prepare_fb(struct drm_simple_display_pipe *pipe,

• struct drm_plane_state *plane_state)
  

+{

• struct xen_drm_front_drm_pipeline *pipeline =

• 	to_xen_drm_pipeline(pipe);
  

• if (pipeline->pgflip_last_error) {

• int ret;
  

• /* if previous page flip didn't succeed then report the error */
  

• ret = pipeline->pgflip_last_error;
  

• /* and let us try to page flip next time */
  

• pipeline->pgflip_last_error = 0;
  

• return ret;
  

• }

Nope, this isn't how the uapi works. If your flips fail then we might need to add some error status thing to the drm events, but you can't make the next flip fail. -Daniel

• return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);

+}

+static void display_update(struct drm_simple_display_pipe *pipe,

• struct drm_plane_state *old_plane_state)
  

+{

• struct drm_crtc *crtc = &pipe->crtc;
• struct drm_pending_vblank_event *event;
• event = crtc->state->event;
• if (event) {

• struct drm_device *dev = crtc->dev;
  

• unsigned long flags;
  

• crtc->state->event = NULL;
  

• spin_lock_irqsave(&dev->event_lock, flags);
  

• if (drm_crtc_vblank_get(crtc) == 0)
  

• 	drm_crtc_arm_vblank_event(crtc, event);
  

• else
  

• 	drm_crtc_send_vblank_event(crtc, event);
  

• spin_unlock_irqrestore(&dev->event_lock, flags);
  

• }
• /*

• * Send page flip request to the backend *after* we have event armed/
  

• * sent above, so on page flip done event from the backend we can
  

• * deliver it while handling vblank.
  

• */
  

• display_send_page_flip(pipe, old_plane_state);

+}

+static const struct drm_simple_display_pipe_funcs display_funcs = {

• .enable = display_enable,
• .disable = display_disable,
• .prepare_fb = display_prepare_fb,
• .update = display_update,

+};

+static int display_pipe_init(struct xen_drm_front_drm_info *drm_info,

• int index, struct xen_drm_front_cfg_connector *cfg,
  

• struct xen_drm_front_drm_pipeline *pipeline)
  

+{

• struct drm_device *dev = drm_info->drm_dev;
• const uint32_t *formats;
• int format_count;
• int ret;
• pipeline->drm_info = drm_info;
• pipeline->index = index;
• pipeline->height = cfg->height;
• pipeline->width = cfg->width;
• ret = xen_drm_front_conn_init(drm_info, &pipeline->conn);
• if (ret)

• return ret;
  

• formats = xen_drm_front_conn_get_formats(&format_count);
• return drm_simple_display_pipe_init(dev, &pipeline->pipe,

• 	&display_funcs, formats, format_count,
  

• 	NULL, &pipeline->conn);
  

+}

+int xen_drm_front_kms_init(struct xen_drm_front_drm_info *drm_info) +{

• struct drm_device *dev = drm_info->drm_dev;
• int i, ret;
• drm_mode_config_init(dev);
• dev->mode_config.min_width = 0;
• dev->mode_config.min_height = 0;
• dev->mode_config.max_width = 4095;
• dev->mode_config.max_height = 2047;
• dev->mode_config.funcs = &mode_config_funcs;
• for (i = 0; i < drm_info->cfg->num_connectors; i++) {

• struct xen_drm_front_cfg_connector *cfg =
  

• 		&drm_info->cfg->connectors[i];
  

• struct xen_drm_front_drm_pipeline *pipeline =
  

• 		&drm_info->pipeline[i];
  

• ret = display_pipe_init(drm_info, i, cfg, pipeline);
  

• if (ret) {
  

• 	drm_mode_config_cleanup(dev);
  

• 	return ret;
  

• }
  

• }
• drm_mode_config_reset(dev);
• return 0;

+} diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.h b/drivers/gpu/drm/xen/xen_drm_front_kms.h new file mode 100644 index 000000000000..65a50033bb9b --- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.h @@ -0,0 +1,30 @@ +/*

• • Xen para-virtual DRM device
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • Copyright (C) 2016-2018 EPAM Systems Inc.
• • Author: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com
• */

+#ifndef __XEN_DRM_FRONT_KMS_H_ +#define __XEN_DRM_FRONT_KMS_H_

+#include "xen_drm_front_drv.h"

+int xen_drm_front_kms_init(struct xen_drm_front_drm_info *drm_info);

+void xen_drm_front_kms_on_frame_done(

• struct xen_drm_front_drm_pipeline *pipeline,
  

• uint64_t fb_cookie);
  

+#endif /* __XEN_DRM_FRONT_KMS_H_ */

2.7.4

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel

On Mon, Mar 05, 2018 at 02:59:23PM +0200, Oleksandr Andrushchenko wrote:

On 03/05/2018 11:23 AM, Daniel Vetter wrote:

...

On Wed, Feb 21, 2018 at 10:03:40AM +0200, Oleksandr Andrushchenko wrote:

...

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

Implement kernel modesetiing/connector handling using DRM simple KMS helper pipeline:

• implement KMS part of the driver with the help of DRM simple pipepline helper which is possible due to the fact that the para-virtualized driver only supports a single (primary) plane:

  • initialize connectors according to XenStore configuration
  • handle frame done events from the backend
  • generate vblank events
  • create and destroy frame buffers and propagate those to the backend
  • propagate set/reset mode configuration to the backend on display enable/disable callbacks
  • send page flip request to the backend and implement logic for reporting backend IO errors on prepare fb callback

• implement virtual connector handling:

  • support only pixel formats suitable for single plane modes
  • make sure the connector is always connected
  • support a single video mode as per para-virtualized driver configuration

Signed-off-by: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

I think once you've removed the midlayer in the previous patch it would makes sense to merge the 2 patches into 1.

ok, will squash the two

...

Bunch more comments below. -Daniel

...

---

drivers/gpu/drm/xen/Makefile | 2 + drivers/gpu/drm/xen/xen_drm_front_conn.c | 125 +++++++++++++ drivers/gpu/drm/xen/xen_drm_front_conn.h | 35 ++++ drivers/gpu/drm/xen/xen_drm_front_drv.c | 15 ++ drivers/gpu/drm/xen/xen_drm_front_drv.h | 12 ++ drivers/gpu/drm/xen/xen_drm_front_kms.c | 299 +++++++++++++++++++++++++++++++ drivers/gpu/drm/xen/xen_drm_front_kms.h | 30 ++++ 7 files changed, 518 insertions(+) create mode 100644 drivers/gpu/drm/xen/xen_drm_front_conn.c create mode 100644 drivers/gpu/drm/xen/xen_drm_front_conn.h create mode 100644 drivers/gpu/drm/xen/xen_drm_front_kms.c create mode 100644 drivers/gpu/drm/xen/xen_drm_front_kms.h

diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile index d3068202590f..4fcb0da1a9c5 100644 --- a/drivers/gpu/drm/xen/Makefile +++ b/drivers/gpu/drm/xen/Makefile @@ -2,6 +2,8 @@ drm_xen_front-objs := xen_drm_front.o \ xen_drm_front_drv.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
  

diff --git a/drivers/gpu/drm/xen/xen_drm_front_conn.c b/drivers/gpu/drm/xen/xen_drm_front_conn.c new file mode 100644 index 000000000000..d9986a2e1a3b --- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front_conn.c @@ -0,0 +1,125 @@ +/*

• • Xen para-virtual DRM device
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • Copyright (C) 2016-2018 EPAM Systems Inc.
• • Author: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com
• */

+#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h>

+#include <video/videomode.h>

+#include "xen_drm_front_conn.h" +#include "xen_drm_front_drv.h"

+static struct xen_drm_front_drm_pipeline * +to_xen_drm_pipeline(struct drm_connector *connector) +{

• return container_of(connector, struct xen_drm_front_drm_pipeline, conn);

+}

+static const uint32_t plane_formats[] = {

• DRM_FORMAT_RGB565,
• DRM_FORMAT_RGB888,
• DRM_FORMAT_XRGB8888,
• DRM_FORMAT_ARGB8888,
• DRM_FORMAT_XRGB4444,
• DRM_FORMAT_ARGB4444,
• DRM_FORMAT_XRGB1555,
• DRM_FORMAT_ARGB1555,

+};

+const uint32_t *xen_drm_front_conn_get_formats(int *format_count) +{

• *format_count = ARRAY_SIZE(plane_formats);
• return plane_formats;

+}

+static enum drm_connector_status connector_detect(

• struct drm_connector *connector, bool force)
  

+{

• if (drm_dev_is_unplugged(connector->dev))

• return connector_status_disconnected;
  

• return connector_status_connected;

+}

+#define XEN_DRM_NUM_VIDEO_MODES 1 +#define XEN_DRM_CRTC_VREFRESH_HZ 60

+static int connector_get_modes(struct drm_connector *connector) +{

• struct xen_drm_front_drm_pipeline *pipeline =

• to_xen_drm_pipeline(connector);
  

• struct drm_display_mode *mode;
• struct videomode videomode;
• int width, height;
• mode = drm_mode_create(connector->dev);
• if (!mode)

• return 0;
  

• memset(&videomode, 0, sizeof(videomode));
• videomode.hactive = pipeline->width;
• videomode.vactive = pipeline->height;
• width = videomode.hactive + videomode.hfront_porch +

• videomode.hback_porch + videomode.hsync_len;
  

• height = videomode.vactive + videomode.vfront_porch +

• videomode.vback_porch + videomode.vsync_len;
  

• videomode.pixelclock = width * height * XEN_DRM_CRTC_VREFRESH_HZ;
• mode->type = DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER;
• drm_display_mode_from_videomode(&videomode, mode);
• drm_mode_probed_add(connector, mode);
• return XEN_DRM_NUM_VIDEO_MODES;

+}

+static int connector_mode_valid(struct drm_connector *connector,

• struct drm_display_mode *mode)
  

+{

• struct xen_drm_front_drm_pipeline *pipeline =

• 	to_xen_drm_pipeline(connector);
  

• if (mode->hdisplay != pipeline->width)

• return MODE_ERROR;
  

• if (mode->vdisplay != pipeline->height)

• return MODE_ERROR;
  

• return MODE_OK;

+}

+static const struct drm_connector_helper_funcs connector_helper_funcs = {

• .get_modes = connector_get_modes,
• .mode_valid = connector_mode_valid,

+};

+static const struct drm_connector_funcs connector_funcs = {

• .detect = connector_detect,
• .fill_modes = drm_helper_probe_single_connector_modes,
• .destroy = drm_connector_cleanup,
• .reset = drm_atomic_helper_connector_reset,
• .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
• .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,

+};

+int xen_drm_front_conn_init(struct xen_drm_front_drm_info *drm_info,

• struct drm_connector *connector)
  

+{

• drm_connector_helper_add(connector, &connector_helper_funcs);
• return drm_connector_init(drm_info->drm_dev, connector,

• &connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL);
  

+} diff --git a/drivers/gpu/drm/xen/xen_drm_front_conn.h b/drivers/gpu/drm/xen/xen_drm_front_conn.h new file mode 100644 index 000000000000..708e80d45985 --- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front_conn.h @@ -0,0 +1,35 @@ +/*

• • Xen para-virtual DRM device
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • Copyright (C) 2016-2018 EPAM Systems Inc.
• • Author: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com
• */

+#ifndef __XEN_DRM_FRONT_CONN_H_ +#define __XEN_DRM_FRONT_CONN_H_

+#include <drm/drmP.h> +#include <drm/drm_crtc.h> +#include <drm/drm_encoder.h>

+#include <linux/wait.h>

+struct xen_drm_front_drm_info;

+const uint32_t *xen_drm_front_conn_get_formats(int *format_count);

+int xen_drm_front_conn_init(struct xen_drm_front_drm_info *drm_info,

• struct drm_connector *connector);
  

+#endif /* __XEN_DRM_FRONT_CONN_H_ */ diff --git a/drivers/gpu/drm/xen/xen_drm_front_drv.c b/drivers/gpu/drm/xen/xen_drm_front_drv.c index b3764d5ed0f6..e8862d26ba27 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_drv.c +++ b/drivers/gpu/drm/xen/xen_drm_front_drv.c @@ -23,6 +23,7 @@ #include "xen_drm_front.h" #include "xen_drm_front_cfg.h" #include "xen_drm_front_drv.h" +#include "xen_drm_front_kms.h" static int dumb_create(struct drm_file *filp, struct drm_device *dev, struct drm_mode_create_dumb *args) @@ -41,6 +42,13 @@ static void free_object(struct drm_gem_object *obj) static void on_frame_done(struct platform_device *pdev, int conn_idx, uint64_t fb_cookie) {

• struct xen_drm_front_drm_info *drm_info = platform_get_drvdata(pdev);
• if (unlikely(conn_idx >= drm_info->cfg->num_connectors))

• return;
  

• xen_drm_front_kms_on_frame_done(&drm_info->pipeline[conn_idx],

• 	fb_cookie);
  

  } static void lastclose(struct drm_device *dev)

@@ -157,6 +165,12 @@ int xen_drm_front_drv_probe(struct platform_device *pdev, return ret; }

• ret = xen_drm_front_kms_init(drm_info);
• if (ret) {

• DRM_ERROR("Failed to initialize DRM/KMS, ret %d\n", ret);
  

• goto fail_modeset;
  

• }
• dev->irq_enabled = 1; ret = drm_dev_register(dev, 0);

@@ -172,6 +186,7 @@ int xen_drm_front_drv_probe(struct platform_device *pdev, fail_register: drm_dev_unregister(dev); +fail_modeset: drm_mode_config_cleanup(dev); return ret; } diff --git a/drivers/gpu/drm/xen/xen_drm_front_drv.h b/drivers/gpu/drm/xen/xen_drm_front_drv.h index aaa476535c13..563318b19f34 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_drv.h +++ b/drivers/gpu/drm/xen/xen_drm_front_drv.h @@ -20,14 +20,24 @@ #define __XEN_DRM_FRONT_DRV_H_ #include <drm/drmP.h> +#include <drm/drm_simple_kms_helper.h> #include "xen_drm_front.h" #include "xen_drm_front_cfg.h" +#include "xen_drm_front_conn.h" struct xen_drm_front_drm_pipeline { struct xen_drm_front_drm_info *drm_info; int index;

• struct drm_simple_display_pipe pipe;
• struct drm_connector conn;
• /* these are only for connector mode checking */
• int width, height;
• /* last backend error seen on page flip */
• int pgflip_last_error; }; struct xen_drm_front_drm_info {

@@ -35,6 +45,8 @@ struct xen_drm_front_drm_info { struct xen_drm_front_ops *front_ops; struct drm_device *drm_dev; struct xen_drm_front_cfg *cfg;

• struct xen_drm_front_drm_pipeline pipeline[XEN_DRM_FRONT_MAX_CRTCS]; }; static inline uint64_t xen_drm_front_fb_to_cookie(

diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c new file mode 100644 index 000000000000..ad94c28835cd --- /dev/null +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c @@ -0,0 +1,299 @@ +/*

• • Xen para-virtual DRM device
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • Copyright (C) 2016-2018 EPAM Systems Inc.
• • Author: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com
• */

+#include "xen_drm_front_kms.h"

+#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_gem.h> +#include <drm/drm_gem_framebuffer_helper.h>

+#include "xen_drm_front.h" +#include "xen_drm_front_conn.h" +#include "xen_drm_front_drv.h"

+static struct xen_drm_front_drm_pipeline * +to_xen_drm_pipeline(struct drm_simple_display_pipe *pipe) +{

• return container_of(pipe, struct xen_drm_front_drm_pipeline, pipe);

+}

+static void fb_destroy(struct drm_framebuffer *fb) +{

• struct xen_drm_front_drm_info *drm_info = fb->dev->dev_private;
• drm_info->front_ops->fb_detach(drm_info->front_info,

• 	xen_drm_front_fb_to_cookie(fb));
  

• drm_gem_fb_destroy(fb);

+}

+static struct drm_framebuffer_funcs fb_funcs = {

• .destroy = fb_destroy,

+};

+static struct drm_framebuffer *fb_create(struct drm_device *dev,

• struct drm_file *filp, const struct drm_mode_fb_cmd2 *mode_cmd)
  

+{

• struct xen_drm_front_drm_info *drm_info = dev->dev_private;
• static struct drm_framebuffer *fb;
• struct drm_gem_object *gem_obj;
• int ret;
• fb = drm_gem_fb_create_with_funcs(dev, filp, mode_cmd, &fb_funcs);
• if (IS_ERR_OR_NULL(fb))

• return fb;
  

• gem_obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
• if (!gem_obj) {

• DRM_ERROR("Failed to lookup GEM object\n");
  

• ret = -ENOENT;
  

• goto fail;
  

• }
• drm_gem_object_unreference_unlocked(gem_obj);
• ret = drm_info->front_ops->fb_attach(

• 	drm_info->front_info,
  

• 	xen_drm_front_dbuf_to_cookie(gem_obj),
  

• 	xen_drm_front_fb_to_cookie(fb),
  

• 	fb->width, fb->height, fb->format->format);
  

• if (ret < 0) {

• DRM_ERROR("Back failed to attach FB %p: %d\n", fb, ret);
  

• goto fail;
  

• }
• return fb;

+fail:

• drm_gem_fb_destroy(fb);
• return ERR_PTR(ret);

+}

+static const struct drm_mode_config_funcs mode_config_funcs = {

• .fb_create = fb_create,
• .atomic_check = drm_atomic_helper_check,
• .atomic_commit = drm_atomic_helper_commit,

+};

+static int display_set_config(struct drm_simple_display_pipe *pipe,

• struct drm_framebuffer *fb)

+{

• struct xen_drm_front_drm_pipeline *pipeline =

• 	to_xen_drm_pipeline(pipe);
  

• struct drm_crtc *crtc = &pipe->crtc;
• struct xen_drm_front_drm_info *drm_info = pipeline->drm_info;
• int ret;
• if (fb)

• ret = drm_info->front_ops->mode_set(pipeline,
  

• 		crtc->x, crtc->y,
  

• 		fb->width, fb->height, fb->format->cpp[0] * 8,
  

• 		xen_drm_front_fb_to_cookie(fb));
  

• else

• ret = drm_info->front_ops->mode_set(pipeline,
  

• 		0, 0, 0, 0, 0,
  

• 		xen_drm_front_fb_to_cookie(NULL));
  

This is a bit much layering, the if (fb) case corresponds to the display_enable/disable hooks, pls fold that in instead of the indirection. simple helpers guarantee that when the display is on, then you have an fb.

1. Ok, the only reason for having this function was to keep

front_ops->mode_set calls at one place (will be refactored to be a direct call, not via front_ops). 2. The if (fb) check was meant not to check if simple helpers may give us some wrong value when we do not expect: there is nothing wrong with them. The check was for 2 cases when this function was called: with fb != NULL on display enable and with fb == NULL on display disable, e.g. fb was used as a flag in this check.