[PATCH v2 0/3] Expanding the basic vkms features

[PATCH v2 0/3] Expanding the basic vkms features

[Rodrigo Siqueira]

This series of patches add a centralized initialization mechanism, a
single CRTC with a plane, an encoder, and extra module information. 

Changes in v2:
 - Remove unused definitions
 - Improve file names
 - Improve code separation
 - Remove unnecessary formats

Rodrigo Siqueira (3):
  drm/vkms: Add mode_config initialization
  drm/vkms: Add basic CRTC initialization
  drm/vkms: Add extra information about vkms

 drivers/gpu/drm/Kconfig            |  8 ++-
 drivers/gpu/drm/vkms/Makefile      |  2 +-
 drivers/gpu/drm/vkms/vkms_crtc.c   | 35 ++++++++++++
 drivers/gpu/drm/vkms/vkms_drv.c    | 61 +++++++++-----------
 drivers/gpu/drm/vkms/vkms_drv.h    | 24 +++++++-
 drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/vkms/vkms_plane.c  | 46 +++++++++++++++
 7 files changed, 227 insertions(+), 40 deletions(-)
 create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c
 create mode 100644 drivers/gpu/drm/vkms/vkms_output.c
 create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c

-- 
2.17.0

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

[PATCH v2 1/3] drm/vkms: Add mode_config initialization

[Rodrigo Siqueira]

Initialize minimum and maximum width and height of the frame buffers
with default values.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 drivers/gpu/drm/vkms/vkms_drv.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 35517b09538e..aec3f180f96d 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -9,6 +9,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_atomic_helper.h>
 #include "vkms_drv.h"
 
 #define DRIVER_NAME	"vkms"
@@ -17,6 +18,12 @@
 #define DRIVER_MAJOR	1
 #define DRIVER_MINOR	0
 
+#define XRES_MIN    32
+#define YRES_MIN    32
+
+#define XRES_MAX  8192
+#define YRES_MAX  8192
+
 static struct vkms_device *vkms_device;
 
 static const struct file_operations vkms_driver_fops = {
@@ -67,6 +74,11 @@ static const struct drm_connector_funcs vkms_connector_funcs = {
 	.destroy = vkms_connector_destroy,
 };
 
+static const struct drm_mode_config_funcs vkms_mode_funcs = {
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
 static int __init vkms_init(void)
 {
 	int ret;
@@ -87,6 +99,11 @@ static int __init vkms_init(void)
 	}
 
 	drm_mode_config_init(&vkms_device->drm);
+	vkms_device->drm.mode_config.funcs = &vkms_mode_funcs;
+	vkms_device->drm.mode_config.min_width = XRES_MIN;
+	vkms_device->drm.mode_config.min_height = YRES_MIN;
+	vkms_device->drm.mode_config.max_width = XRES_MAX;
+	vkms_device->drm.mode_config.max_height = YRES_MAX;
 
 	ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector,
 				 &vkms_connector_funcs,
-- 
2.17.0

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

[PATCH v2 2/3] drm/vkms: Add basic CRTC initialization

[Rodrigo Siqueira]

This commit adds the essential infrastructure for around CRTCs which
is composed of: a new data struct for output data information, a
function for creating planes, and a simple encoder attached to the
connector. Finally, due to the introduction of a new initialization
function, connectors were moved from vkms_drv.c to vkms_display.c.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 drivers/gpu/drm/vkms/Makefile      |  2 +-
 drivers/gpu/drm/vkms/vkms_crtc.c   | 35 ++++++++++++
 drivers/gpu/drm/vkms/vkms_drv.c    | 60 ++++++--------------
 drivers/gpu/drm/vkms/vkms_drv.h    | 24 +++++++-
 drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/vkms/vkms_plane.c  | 46 +++++++++++++++
 6 files changed, 211 insertions(+), 47 deletions(-)
 create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c
 create mode 100644 drivers/gpu/drm/vkms/vkms_output.c
 create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c

diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
index 2aef948d3a34..3f774a6a9c58 100644
--- a/drivers/gpu/drm/vkms/Makefile
+++ b/drivers/gpu/drm/vkms/Makefile
@@ -1,3 +1,3 @@
-vkms-y := vkms_drv.o
+vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
 
 obj-$(CONFIG_DRM_VKMS) += vkms.o
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
new file mode 100644
index 000000000000..bf76cd39ece7
--- /dev/null
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * 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.
+ */
+
+#include "vkms_drv.h"
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+
+static const struct drm_crtc_funcs vkms_crtc_funcs = {
+	.set_config             = drm_atomic_helper_set_config,
+	.destroy                = drm_crtc_cleanup,
+	.page_flip              = drm_atomic_helper_page_flip,
+	.reset                  = drm_atomic_helper_crtc_reset,
+	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
+};
+
+int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
+		   struct drm_plane *primary, struct drm_plane *cursor)
+{
+	int ret;
+
+	ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor,
+					&vkms_crtc_funcs, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to init CRTC\n");
+		return ret;
+	}
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index aec3f180f96d..070613e32934 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -6,7 +6,6 @@
  */
 
 #include <linux/module.h>
-#include <drm/drmP.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_atomic_helper.h>
@@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = {
 	.minor			= DRIVER_MINOR,
 };
 
-static const u32 vkms_formats[] = {
-	DRM_FORMAT_XRGB8888,
+static const struct drm_mode_config_funcs vkms_mode_funcs = {
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
 };
 
-static void vkms_connector_destroy(struct drm_connector *connector)
+static int vkms_modeset_init(struct vkms_device *vkmsdev)
 {
-	drm_connector_unregister(connector);
-	drm_connector_cleanup(connector);
-}
+	struct drm_device *dev = &vkmsdev->drm;
 
-static const struct drm_connector_funcs vkms_connector_funcs = {
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = vkms_connector_destroy,
-};
+	drm_mode_config_init(dev);
+	dev->mode_config.funcs = &vkms_mode_funcs;
+	dev->mode_config.min_width = XRES_MIN;
+	dev->mode_config.min_height = YRES_MIN;
+	dev->mode_config.max_width = XRES_MAX;
+	dev->mode_config.max_height = YRES_MAX;
 
-static const struct drm_mode_config_funcs vkms_mode_funcs = {
-	.atomic_check = drm_atomic_helper_check,
-	.atomic_commit = drm_atomic_helper_commit,
-};
+	return vkms_output_init(vkmsdev);
+}
 
 static int __init vkms_init(void)
 {
@@ -98,48 +96,24 @@ static int __init vkms_init(void)
 		goto out_fini;
 	}
 
-	drm_mode_config_init(&vkms_device->drm);
-	vkms_device->drm.mode_config.funcs = &vkms_mode_funcs;
-	vkms_device->drm.mode_config.min_width = XRES_MIN;
-	vkms_device->drm.mode_config.min_height = YRES_MIN;
-	vkms_device->drm.mode_config.max_width = XRES_MAX;
-	vkms_device->drm.mode_config.max_height = YRES_MAX;
-
-	ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector,
-				 &vkms_connector_funcs,
-				 DRM_MODE_CONNECTOR_VIRTUAL);
-	if (ret < 0) {
-		DRM_ERROR("Failed to init connector\n");
-		goto out_unregister;
-	}
-
-	ret = drm_simple_display_pipe_init(&vkms_device->drm,
-					   &vkms_device->pipe,
-					   NULL,
-					   vkms_formats,
-					   ARRAY_SIZE(vkms_formats),
-					   NULL,
-					   &vkms_device->connector);
-	if (ret < 0) {
-		DRM_ERROR("Cannot setup simple display pipe\n");
+	ret = vkms_modeset_init(vkms_device);
+	if (ret)
 		goto out_unregister;
-	}
 
 	ret = drm_dev_register(&vkms_device->drm, 0);
 	if (ret)
 		goto out_unregister;
 
-	drm_connector_register(&vkms_device->connector);
-
 	return 0;
 
 out_unregister:
 	platform_device_unregister(vkms_device->platform);
+
 out_fini:
 	drm_dev_fini(&vkms_device->drm);
+
 out_free:
 	kfree(vkms_device);
-
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index c77c5bf5032a..b0f9d2e61a42 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -1,13 +1,31 @@
 #ifndef _VKMS_DRV_H_
 #define _VKMS_DRV_H_
 
-#include <drm/drm_simple_kms_helper.h>
+#include <drm/drmP.h>
+#include <drm/drm.h>
+#include <drm/drm_encoder.h>
+
+static const u32 vkms_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
+
+struct vkms_output {
+	struct drm_crtc crtc;
+	struct drm_encoder encoder;
+	struct drm_connector connector;
+};
 
 struct vkms_device {
 	struct drm_device drm;
 	struct platform_device *platform;
-	struct drm_simple_display_pipe pipe;
-	struct drm_connector connector;
+	struct vkms_output output;
 };
 
+int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
+		   struct drm_plane *primary, struct drm_plane *cursor);
+
+int vkms_output_init(struct vkms_device *vkmsdev);
+
+struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
+
 #endif /* _VKMS_DRV_H_ */
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
new file mode 100644
index 000000000000..48143eac3c12
--- /dev/null
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * 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.
+ */
+
+#include "vkms_drv.h"
+#include <drm/drm_crtc_helper.h>
+
+static void vkms_connector_destroy(struct drm_connector *connector)
+{
+	drm_connector_unregister(connector);
+	drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs vkms_connector_funcs = {
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = vkms_connector_destroy,
+};
+
+static const struct drm_encoder_funcs vkms_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+int vkms_output_init(struct vkms_device *vkmsdev)
+{
+	struct vkms_output *output = &vkmsdev->output;
+	struct drm_device *dev = &vkmsdev->drm;
+	struct drm_connector *connector = &output->connector;
+	struct drm_encoder *encoder = &output->encoder;
+	struct drm_crtc *crtc = &output->crtc;
+	struct drm_plane *primary;
+	int ret;
+
+	primary = vkms_plane_init(vkmsdev);
+	if (IS_ERR(primary))
+		return PTR_ERR(primary);
+
+	ret = vkms_crtc_init(dev, crtc, primary, NULL);
+	if (ret)
+		goto err_crtc;
+
+	ret = drm_connector_init(dev, connector, &vkms_connector_funcs,
+				 DRM_MODE_CONNECTOR_VIRTUAL);
+	if (ret) {
+		DRM_ERROR("Failed to init connector\n");
+		goto err_connector;
+	}
+
+	ret = drm_connector_register(connector);
+	if (ret) {
+		DRM_ERROR("Failed to register connector\n");
+		goto err_connector_register;
+	}
+
+	ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs,
+			       DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to init encoder\n");
+		goto err_encoder;
+	}
+	encoder->possible_crtcs = 1;
+
+	ret = drm_mode_connector_attach_encoder(connector, encoder);
+	if (ret) {
+		DRM_ERROR("Failed to attach connector to encoder\n");
+		goto err_attach;
+	}
+
+	drm_mode_config_reset(dev);
+
+	return 0;
+
+err_attach:
+	drm_encoder_cleanup(encoder);
+
+err_encoder:
+	drm_connector_unregister(connector);
+
+err_connector_register:
+	drm_connector_cleanup(connector);
+
+err_connector:
+	drm_crtc_cleanup(crtc);
+
+err_crtc:
+	drm_plane_cleanup(primary);
+	return ret;
+}
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
new file mode 100644
index 000000000000..2c25b1d6ab5b
--- /dev/null
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * 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.
+ */
+
+#include "vkms_drv.h"
+#include <drm/drm_plane_helper.h>
+#include <drm/drm_atomic_helper.h>
+
+static const struct drm_plane_funcs vkms_plane_funcs = {
+	.update_plane		= drm_atomic_helper_update_plane,
+	.disable_plane		= drm_atomic_helper_disable_plane,
+	.destroy		= drm_plane_cleanup,
+	.reset			= drm_atomic_helper_plane_reset,
+	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
+};
+
+struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev)
+{
+	struct drm_device *dev = &vkmsdev->drm;
+	struct drm_plane *plane;
+	const u32 *formats;
+	int ret, nformats;
+
+	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
+	if (!plane)
+		return ERR_PTR(-ENOMEM);
+
+	formats = vkms_formats;
+	nformats = ARRAY_SIZE(vkms_formats);
+
+	ret = drm_universal_plane_init(dev, plane, 0,
+				       &vkms_plane_funcs,
+				       formats, nformats,
+				       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret) {
+		kfree(plane);
+		return ERR_PTR(ret);
+	}
+
+	return plane;
+}
-- 
2.17.0

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

Re: [PATCH v2 2/3] drm/vkms: Add basic CRTC initialization

[Haneen Mohammed]

On Wed, May 16, 2018 at 08:56:21PM -0300, Rodrigo Siqueira wrote:
> This commit adds the essential infrastructure for around CRTCs which
> is composed of: a new data struct for output data information, a
> function for creating planes, and a simple encoder attached to the
> connector. Finally, due to the introduction of a new initialization
> function, connectors were moved from vkms_drv.c to vkms_display.c.
> 
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---
>  drivers/gpu/drm/vkms/Makefile      |  2 +-
>  drivers/gpu/drm/vkms/vkms_crtc.c   | 35 ++++++++++++
>  drivers/gpu/drm/vkms/vkms_drv.c    | 60 ++++++--------------
>  drivers/gpu/drm/vkms/vkms_drv.h    | 24 +++++++-
>  drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_plane.c  | 46 +++++++++++++++
>  6 files changed, 211 insertions(+), 47 deletions(-)
>  create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_output.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c
> 
> diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> index 2aef948d3a34..3f774a6a9c58 100644
> --- a/drivers/gpu/drm/vkms/Makefile
> +++ b/drivers/gpu/drm/vkms/Makefile
> @@ -1,3 +1,3 @@
> -vkms-y := vkms_drv.o
> +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
>  
>  obj-$(CONFIG_DRM_VKMS) += vkms.o
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> new file mode 100644
> index 000000000000..bf76cd39ece7
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * 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.
> + */
> +
> +#include "vkms_drv.h"
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +static const struct drm_crtc_funcs vkms_crtc_funcs = {
> +	.set_config             = drm_atomic_helper_set_config,
> +	.destroy                = drm_crtc_cleanup,
> +	.page_flip              = drm_atomic_helper_page_flip,
> +	.reset                  = drm_atomic_helper_crtc_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> +	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
> +};
> +
> +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> +		   struct drm_plane *primary, struct drm_plane *cursor)
> +{
> +	int ret;
> +

Just as a follow up, I have noticed that vkms breaks when inspecting
its state in the debugfs, also when running igt tests with kms and core
filters, and adding the following fixed that issue:

1) a drm_crtc_helper_funcs with dummy atomic_check.

> +	ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor,
> +					&vkms_crtc_funcs, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to init CRTC\n");
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index aec3f180f96d..070613e32934 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -6,7 +6,6 @@
>   */
>  
>  #include <linux/module.h>
> -#include <drm/drmP.h>
>  #include <drm/drm_gem.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = {
>  	.minor			= DRIVER_MINOR,
>  };
>  
> -static const u32 vkms_formats[] = {
> -	DRM_FORMAT_XRGB8888,
> +static const struct drm_mode_config_funcs vkms_mode_funcs = {
> +	.atomic_check = drm_atomic_helper_check,
> +	.atomic_commit = drm_atomic_helper_commit,
>  };
>  
> -static void vkms_connector_destroy(struct drm_connector *connector)
> +static int vkms_modeset_init(struct vkms_device *vkmsdev)
>  {
> -	drm_connector_unregister(connector);
> -	drm_connector_cleanup(connector);
> -}
> +	struct drm_device *dev = &vkmsdev->drm;
>  
> -static const struct drm_connector_funcs vkms_connector_funcs = {
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = vkms_connector_destroy,
> -};
> +	drm_mode_config_init(dev);
> +	dev->mode_config.funcs = &vkms_mode_funcs;
> +	dev->mode_config.min_width = XRES_MIN;
> +	dev->mode_config.min_height = YRES_MIN;
> +	dev->mode_config.max_width = XRES_MAX;
> +	dev->mode_config.max_height = YRES_MAX;
>  
> -static const struct drm_mode_config_funcs vkms_mode_funcs = {
> -	.atomic_check = drm_atomic_helper_check,
> -	.atomic_commit = drm_atomic_helper_commit,
> -};
> +	return vkms_output_init(vkmsdev);
> +}
>  
>  static int __init vkms_init(void)
>  {
> @@ -98,48 +96,24 @@ static int __init vkms_init(void)
>  		goto out_fini;
>  	}
>  
> -	drm_mode_config_init(&vkms_device->drm);
> -	vkms_device->drm.mode_config.funcs = &vkms_mode_funcs;
> -	vkms_device->drm.mode_config.min_width = XRES_MIN;
> -	vkms_device->drm.mode_config.min_height = YRES_MIN;
> -	vkms_device->drm.mode_config.max_width = XRES_MAX;
> -	vkms_device->drm.mode_config.max_height = YRES_MAX;
> -
> -	ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector,
> -				 &vkms_connector_funcs,
> -				 DRM_MODE_CONNECTOR_VIRTUAL);
> -	if (ret < 0) {
> -		DRM_ERROR("Failed to init connector\n");
> -		goto out_unregister;
> -	}
> -
> -	ret = drm_simple_display_pipe_init(&vkms_device->drm,
> -					   &vkms_device->pipe,
> -					   NULL,
> -					   vkms_formats,
> -					   ARRAY_SIZE(vkms_formats),
> -					   NULL,
> -					   &vkms_device->connector);
> -	if (ret < 0) {
> -		DRM_ERROR("Cannot setup simple display pipe\n");
> +	ret = vkms_modeset_init(vkms_device);
> +	if (ret)
>  		goto out_unregister;
> -	}
>  
>  	ret = drm_dev_register(&vkms_device->drm, 0);
>  	if (ret)
>  		goto out_unregister;
>  
> -	drm_connector_register(&vkms_device->connector);
> -
>  	return 0;
>  
>  out_unregister:
>  	platform_device_unregister(vkms_device->platform);
> +
>  out_fini:
>  	drm_dev_fini(&vkms_device->drm);
> +
>  out_free:
>  	kfree(vkms_device);
> -
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index c77c5bf5032a..b0f9d2e61a42 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -1,13 +1,31 @@
>  #ifndef _VKMS_DRV_H_
>  #define _VKMS_DRV_H_
>  
> -#include <drm/drm_simple_kms_helper.h>
> +#include <drm/drmP.h>
> +#include <drm/drm.h>
> +#include <drm/drm_encoder.h>
> +
> +static const u32 vkms_formats[] = {
> +	DRM_FORMAT_XRGB8888,
> +};
> +
> +struct vkms_output {
> +	struct drm_crtc crtc;
> +	struct drm_encoder encoder;
> +	struct drm_connector connector;
> +};
>  
>  struct vkms_device {
>  	struct drm_device drm;
>  	struct platform_device *platform;
> -	struct drm_simple_display_pipe pipe;
> -	struct drm_connector connector;
> +	struct vkms_output output;
>  };
>  
> +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> +		   struct drm_plane *primary, struct drm_plane *cursor);
> +
> +int vkms_output_init(struct vkms_device *vkmsdev);
> +
> +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
> +
>  #endif /* _VKMS_DRV_H_ */
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> new file mode 100644
> index 000000000000..48143eac3c12
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -0,0 +1,91 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * 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.
> + */
> +
> +#include "vkms_drv.h"
> +#include <drm/drm_crtc_helper.h>
> +
> +static void vkms_connector_destroy(struct drm_connector *connector)
> +{
> +	drm_connector_unregister(connector);
> +	drm_connector_cleanup(connector);
> +}
> +
> +static const struct drm_connector_funcs vkms_connector_funcs = {
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = vkms_connector_destroy,

2) adding the following hooks with their drm_atomic_helper_connector_*
counterpart to the vkms_connector_funcs:
atomic_duplicate_state, atomic_destroy_state, and reset.

> +};
> +
> +static const struct drm_encoder_funcs vkms_encoder_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
> +int vkms_output_init(struct vkms_device *vkmsdev)
> +{
> +	struct vkms_output *output = &vkmsdev->output;
> +	struct drm_device *dev = &vkmsdev->drm;
> +	struct drm_connector *connector = &output->connector;
> +	struct drm_encoder *encoder = &output->encoder;
> +	struct drm_crtc *crtc = &output->crtc;
> +	struct drm_plane *primary;
> +	int ret;
> +
> +	primary = vkms_plane_init(vkmsdev);
> +	if (IS_ERR(primary))
> +		return PTR_ERR(primary);
> +
> +	ret = vkms_crtc_init(dev, crtc, primary, NULL);
> +	if (ret)
> +		goto err_crtc;
> +

3) add drm_connector_helper_funcs with dummy {get_modes, mode_valid}.

> +	ret = drm_connector_init(dev, connector, &vkms_connector_funcs,
> +				 DRM_MODE_CONNECTOR_VIRTUAL);
> +	if (ret) {
> +		DRM_ERROR("Failed to init connector\n");
> +		goto err_connector;
> +	}
> +
> +	ret = drm_connector_register(connector);
> +	if (ret) {
> +		DRM_ERROR("Failed to register connector\n");
> +		goto err_connector_register;
> +	}
> +
> +	ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs,
> +			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to init encoder\n");
> +		goto err_encoder;
> +	}
> +	encoder->possible_crtcs = 1;
> +
> +	ret = drm_mode_connector_attach_encoder(connector, encoder);
> +	if (ret) {
> +		DRM_ERROR("Failed to attach connector to encoder\n");
> +		goto err_attach;
> +	}
> +
> +	drm_mode_config_reset(dev);
> +
> +	return 0;
> +
> +err_attach:
> +	drm_encoder_cleanup(encoder);
> +
> +err_encoder:
> +	drm_connector_unregister(connector);
> +
> +err_connector_register:
> +	drm_connector_cleanup(connector);
> +
> +err_connector:
> +	drm_crtc_cleanup(crtc);
> +
> +err_crtc:
> +	drm_plane_cleanup(primary);
> +	return ret;
> +}
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> new file mode 100644
> index 000000000000..2c25b1d6ab5b
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * 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.
> + */
> +
> +#include "vkms_drv.h"
> +#include <drm/drm_plane_helper.h>
> +#include <drm/drm_atomic_helper.h>
> +
> +static const struct drm_plane_funcs vkms_plane_funcs = {
> +	.update_plane		= drm_atomic_helper_update_plane,
> +	.disable_plane		= drm_atomic_helper_disable_plane,
> +	.destroy		= drm_plane_cleanup,
> +	.reset			= drm_atomic_helper_plane_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
> +};
> +
> +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev)
> +{
> +	struct drm_device *dev = &vkmsdev->drm;
> +	struct drm_plane *plane;
> +	const u32 *formats;
> +	int ret, nformats;
> +
> +	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
> +	if (!plane)
> +		return ERR_PTR(-ENOMEM);
> +
> +	formats = vkms_formats;
> +	nformats = ARRAY_SIZE(vkms_formats);
> +
> +	ret = drm_universal_plane_init(dev, plane, 0,
> +				       &vkms_plane_funcs,
> +				       formats, nformats,
> +				       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (ret) {
> +		kfree(plane);
> +		return ERR_PTR(ret);
> +	}
> +

4) add drm_plane_helper_funcs with dummy {atomic_check, prepare_fb, cleanup_fb}.

> +	return plane;
> +}
> -- 
> 2.17.0
> 

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

Re: [PATCH v2 2/3] drm/vkms: Add basic CRTC initialization

[Rodrigo Siqueira]

Hi Haneen,

Thanks for the feedback :)

On 05/20, Haneen Mohammed wrote:
> On Wed, May 16, 2018 at 08:56:21PM -0300, Rodrigo Siqueira wrote:
> > This commit adds the essential infrastructure for around CRTCs which
> > is composed of: a new data struct for output data information, a
> > function for creating planes, and a simple encoder attached to the
> > connector. Finally, due to the introduction of a new initialization
> > function, connectors were moved from vkms_drv.c to vkms_display.c.
> > 
> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > ---
> >  drivers/gpu/drm/vkms/Makefile      |  2 +-
> >  drivers/gpu/drm/vkms/vkms_crtc.c   | 35 ++++++++++++
> >  drivers/gpu/drm/vkms/vkms_drv.c    | 60 ++++++--------------
> >  drivers/gpu/drm/vkms/vkms_drv.h    | 24 +++++++-
> >  drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/vkms/vkms_plane.c  | 46 +++++++++++++++
> >  6 files changed, 211 insertions(+), 47 deletions(-)
> >  create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c
> >  create mode 100644 drivers/gpu/drm/vkms/vkms_output.c
> >  create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c
> > 
> > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> > index 2aef948d3a34..3f774a6a9c58 100644
> > --- a/drivers/gpu/drm/vkms/Makefile
> > +++ b/drivers/gpu/drm/vkms/Makefile
> > @@ -1,3 +1,3 @@
> > -vkms-y := vkms_drv.o
> > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
> >  
> >  obj-$(CONFIG_DRM_VKMS) += vkms.o
> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > new file mode 100644
> > index 000000000000..bf76cd39ece7
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -0,0 +1,35 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * 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.
> > + */
> > +
> > +#include "vkms_drv.h"
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_crtc_helper.h>
> > +
> > +static const struct drm_crtc_funcs vkms_crtc_funcs = {
> > +	.set_config             = drm_atomic_helper_set_config,
> > +	.destroy                = drm_crtc_cleanup,
> > +	.page_flip              = drm_atomic_helper_page_flip,
> > +	.reset                  = drm_atomic_helper_crtc_reset,
> > +	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> > +	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
> > +};
> > +
> > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> > +		   struct drm_plane *primary, struct drm_plane *cursor)
> > +{
> > +	int ret;
> > +
> 
> Just as a follow up, I have noticed that vkms breaks when inspecting
> its state in the debugfs, also when running igt tests with kms and core
> filters, and adding the following fixed that issue:

Could you explain me how you tested the state with debugfs?

I will add the fixes suggested in your comments.

Thanks
 
> 1) a drm_crtc_helper_funcs with dummy atomic_check.
> 
> > +	ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor,
> > +					&vkms_crtc_funcs, NULL);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to init CRTC\n");
> > +		return ret;
> > +	}
> > +
> > +	return ret;
> > +}
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> > index aec3f180f96d..070613e32934 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -6,7 +6,6 @@
> >   */
> >  
> >  #include <linux/module.h>
> > -#include <drm/drmP.h>
> >  #include <drm/drm_gem.h>
> >  #include <drm/drm_crtc_helper.h>
> >  #include <drm/drm_atomic_helper.h>
> > @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = {
> >  	.minor			= DRIVER_MINOR,
> >  };
> >  
> > -static const u32 vkms_formats[] = {
> > -	DRM_FORMAT_XRGB8888,
> > +static const struct drm_mode_config_funcs vkms_mode_funcs = {
> > +	.atomic_check = drm_atomic_helper_check,
> > +	.atomic_commit = drm_atomic_helper_commit,
> >  };
> >  
> > -static void vkms_connector_destroy(struct drm_connector *connector)
> > +static int vkms_modeset_init(struct vkms_device *vkmsdev)
> >  {
> > -	drm_connector_unregister(connector);
> > -	drm_connector_cleanup(connector);
> > -}
> > +	struct drm_device *dev = &vkmsdev->drm;
> >  
> > -static const struct drm_connector_funcs vkms_connector_funcs = {
> > -	.fill_modes = drm_helper_probe_single_connector_modes,
> > -	.destroy = vkms_connector_destroy,
> > -};
> > +	drm_mode_config_init(dev);
> > +	dev->mode_config.funcs = &vkms_mode_funcs;
> > +	dev->mode_config.min_width = XRES_MIN;
> > +	dev->mode_config.min_height = YRES_MIN;
> > +	dev->mode_config.max_width = XRES_MAX;
> > +	dev->mode_config.max_height = YRES_MAX;
> >  
> > -static const struct drm_mode_config_funcs vkms_mode_funcs = {
> > -	.atomic_check = drm_atomic_helper_check,
> > -	.atomic_commit = drm_atomic_helper_commit,
> > -};
> > +	return vkms_output_init(vkmsdev);
> > +}
> >  
> >  static int __init vkms_init(void)
> >  {
> > @@ -98,48 +96,24 @@ static int __init vkms_init(void)
> >  		goto out_fini;
> >  	}
> >  
> > -	drm_mode_config_init(&vkms_device->drm);
> > -	vkms_device->drm.mode_config.funcs = &vkms_mode_funcs;
> > -	vkms_device->drm.mode_config.min_width = XRES_MIN;
> > -	vkms_device->drm.mode_config.min_height = YRES_MIN;
> > -	vkms_device->drm.mode_config.max_width = XRES_MAX;
> > -	vkms_device->drm.mode_config.max_height = YRES_MAX;
> > -
> > -	ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector,
> > -				 &vkms_connector_funcs,
> > -				 DRM_MODE_CONNECTOR_VIRTUAL);
> > -	if (ret < 0) {
> > -		DRM_ERROR("Failed to init connector\n");
> > -		goto out_unregister;
> > -	}
> > -
> > -	ret = drm_simple_display_pipe_init(&vkms_device->drm,
> > -					   &vkms_device->pipe,
> > -					   NULL,
> > -					   vkms_formats,
> > -					   ARRAY_SIZE(vkms_formats),
> > -					   NULL,
> > -					   &vkms_device->connector);
> > -	if (ret < 0) {
> > -		DRM_ERROR("Cannot setup simple display pipe\n");
> > +	ret = vkms_modeset_init(vkms_device);
> > +	if (ret)
> >  		goto out_unregister;
> > -	}
> >  
> >  	ret = drm_dev_register(&vkms_device->drm, 0);
> >  	if (ret)
> >  		goto out_unregister;
> >  
> > -	drm_connector_register(&vkms_device->connector);
> > -
> >  	return 0;
> >  
> >  out_unregister:
> >  	platform_device_unregister(vkms_device->platform);
> > +
> >  out_fini:
> >  	drm_dev_fini(&vkms_device->drm);
> > +
> >  out_free:
> >  	kfree(vkms_device);
> > -
> >  	return ret;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> > index c77c5bf5032a..b0f9d2e61a42 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -1,13 +1,31 @@
> >  #ifndef _VKMS_DRV_H_
> >  #define _VKMS_DRV_H_
> >  
> > -#include <drm/drm_simple_kms_helper.h>
> > +#include <drm/drmP.h>
> > +#include <drm/drm.h>
> > +#include <drm/drm_encoder.h>
> > +
> > +static const u32 vkms_formats[] = {
> > +	DRM_FORMAT_XRGB8888,
> > +};
> > +
> > +struct vkms_output {
> > +	struct drm_crtc crtc;
> > +	struct drm_encoder encoder;
> > +	struct drm_connector connector;
> > +};
> >  
> >  struct vkms_device {
> >  	struct drm_device drm;
> >  	struct platform_device *platform;
> > -	struct drm_simple_display_pipe pipe;
> > -	struct drm_connector connector;
> > +	struct vkms_output output;
> >  };
> >  
> > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> > +		   struct drm_plane *primary, struct drm_plane *cursor);
> > +
> > +int vkms_output_init(struct vkms_device *vkmsdev);
> > +
> > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
> > +
> >  #endif /* _VKMS_DRV_H_ */
> > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> > new file mode 100644
> > index 000000000000..48143eac3c12
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -0,0 +1,91 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * 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.
> > + */
> > +
> > +#include "vkms_drv.h"
> > +#include <drm/drm_crtc_helper.h>
> > +
> > +static void vkms_connector_destroy(struct drm_connector *connector)
> > +{
> > +	drm_connector_unregister(connector);
> > +	drm_connector_cleanup(connector);
> > +}
> > +
> > +static const struct drm_connector_funcs vkms_connector_funcs = {
> > +	.fill_modes = drm_helper_probe_single_connector_modes,
> > +	.destroy = vkms_connector_destroy,
> 
> 2) adding the following hooks with their drm_atomic_helper_connector_*
> counterpart to the vkms_connector_funcs:
> atomic_duplicate_state, atomic_destroy_state, and reset.
> 
> > +};
> > +
> > +static const struct drm_encoder_funcs vkms_encoder_funcs = {
> > +	.destroy = drm_encoder_cleanup,
> > +};
> > +
> > +int vkms_output_init(struct vkms_device *vkmsdev)
> > +{
> > +	struct vkms_output *output = &vkmsdev->output;
> > +	struct drm_device *dev = &vkmsdev->drm;
> > +	struct drm_connector *connector = &output->connector;
> > +	struct drm_encoder *encoder = &output->encoder;
> > +	struct drm_crtc *crtc = &output->crtc;
> > +	struct drm_plane *primary;
> > +	int ret;
> > +
> > +	primary = vkms_plane_init(vkmsdev);
> > +	if (IS_ERR(primary))
> > +		return PTR_ERR(primary);
> > +
> > +	ret = vkms_crtc_init(dev, crtc, primary, NULL);
> > +	if (ret)
> > +		goto err_crtc;
> > +
> 
> 3) add drm_connector_helper_funcs with dummy {get_modes, mode_valid}.
> 
> > +	ret = drm_connector_init(dev, connector, &vkms_connector_funcs,
> > +				 DRM_MODE_CONNECTOR_VIRTUAL);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to init connector\n");
> > +		goto err_connector;
> > +	}
> > +
> > +	ret = drm_connector_register(connector);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to register connector\n");
> > +		goto err_connector_register;
> > +	}
> > +
> > +	ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs,
> > +			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to init encoder\n");
> > +		goto err_encoder;
> > +	}
> > +	encoder->possible_crtcs = 1;
> > +
> > +	ret = drm_mode_connector_attach_encoder(connector, encoder);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to attach connector to encoder\n");
> > +		goto err_attach;
> > +	}
> > +
> > +	drm_mode_config_reset(dev);
> > +
> > +	return 0;
> > +
> > +err_attach:
> > +	drm_encoder_cleanup(encoder);
> > +
> > +err_encoder:
> > +	drm_connector_unregister(connector);
> > +
> > +err_connector_register:
> > +	drm_connector_cleanup(connector);
> > +
> > +err_connector:
> > +	drm_crtc_cleanup(crtc);
> > +
> > +err_crtc:
> > +	drm_plane_cleanup(primary);
> > +	return ret;
> > +}
> > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> > new file mode 100644
> > index 000000000000..2c25b1d6ab5b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> > @@ -0,0 +1,46 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * 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.
> > + */
> > +
> > +#include "vkms_drv.h"
> > +#include <drm/drm_plane_helper.h>
> > +#include <drm/drm_atomic_helper.h>
> > +
> > +static const struct drm_plane_funcs vkms_plane_funcs = {
> > +	.update_plane		= drm_atomic_helper_update_plane,
> > +	.disable_plane		= drm_atomic_helper_disable_plane,
> > +	.destroy		= drm_plane_cleanup,
> > +	.reset			= drm_atomic_helper_plane_reset,
> > +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> > +	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
> > +};
> > +
> > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev)
> > +{
> > +	struct drm_device *dev = &vkmsdev->drm;
> > +	struct drm_plane *plane;
> > +	const u32 *formats;
> > +	int ret, nformats;
> > +
> > +	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
> > +	if (!plane)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	formats = vkms_formats;
> > +	nformats = ARRAY_SIZE(vkms_formats);
> > +
> > +	ret = drm_universal_plane_init(dev, plane, 0,
> > +				       &vkms_plane_funcs,
> > +				       formats, nformats,
> > +				       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> > +	if (ret) {
> > +		kfree(plane);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> 
> 4) add drm_plane_helper_funcs with dummy {atomic_check, prepare_fb, cleanup_fb}.
> 
> > +	return plane;
> > +}
> > -- 
> > 2.17.0
> > 
> 
> - Haneen
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/3] drm/vkms: Add basic CRTC initialization

[Haneen Mohammed]

On Sun, May 20, 2018 at 11:28:37AM -0300, Rodrigo Siqueira wrote:
> Hi Haneen,
> 
> Thanks for the feedback :)
> 
> On 05/20, Haneen Mohammed wrote:
> > On Wed, May 16, 2018 at 08:56:21PM -0300, Rodrigo Siqueira wrote:
> > > This commit adds the essential infrastructure for around CRTCs which
> > > is composed of: a new data struct for output data information, a
> > > function for creating planes, and a simple encoder attached to the
> > > connector. Finally, due to the introduction of a new initialization
> > > function, connectors were moved from vkms_drv.c to vkms_display.c.
> > > 
> > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > ---
> > >  drivers/gpu/drm/vkms/Makefile      |  2 +-
> > >  drivers/gpu/drm/vkms/vkms_crtc.c   | 35 ++++++++++++
> > >  drivers/gpu/drm/vkms/vkms_drv.c    | 60 ++++++--------------
> > >  drivers/gpu/drm/vkms/vkms_drv.h    | 24 +++++++-
> > >  drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/vkms/vkms_plane.c  | 46 +++++++++++++++
> > >  6 files changed, 211 insertions(+), 47 deletions(-)
> > >  create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c
> > >  create mode 100644 drivers/gpu/drm/vkms/vkms_output.c
> > >  create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c
> > > 
> > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> > > index 2aef948d3a34..3f774a6a9c58 100644
> > > --- a/drivers/gpu/drm/vkms/Makefile
> > > +++ b/drivers/gpu/drm/vkms/Makefile
> > > @@ -1,3 +1,3 @@
> > > -vkms-y := vkms_drv.o
> > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
> > >  
> > >  obj-$(CONFIG_DRM_VKMS) += vkms.o
> > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > new file mode 100644
> > > index 000000000000..bf76cd39ece7
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > @@ -0,0 +1,35 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * 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.
> > > + */
> > > +
> > > +#include "vkms_drv.h"
> > > +#include <drm/drm_atomic_helper.h>
> > > +#include <drm/drm_crtc_helper.h>
> > > +
> > > +static const struct drm_crtc_funcs vkms_crtc_funcs = {
> > > +	.set_config             = drm_atomic_helper_set_config,
> > > +	.destroy                = drm_crtc_cleanup,
> > > +	.page_flip              = drm_atomic_helper_page_flip,
> > > +	.reset                  = drm_atomic_helper_crtc_reset,
> > > +	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> > > +	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
> > > +};
> > > +
> > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> > > +		   struct drm_plane *primary, struct drm_plane *cursor)
> > > +{
> > > +	int ret;
> > > +
> > 
> > Just as a follow up, I have noticed that vkms breaks when inspecting
> > its state in the debugfs, also when running igt tests with kms and core
> > filters, and adding the following fixed that issue:
> 
> Could you explain me how you tested the state with debugfs?
> 
> I will add the fixes suggested in your comments.
> 
> Thanks
>  

I just tried to read this file: /sys/kernel/debug/dri/0/state, which
then caused a null pointer dereference error.

> > 1) a drm_crtc_helper_funcs with dummy atomic_check.
> > 
> > > +	ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor,
> > > +					&vkms_crtc_funcs, NULL);
> > > +	if (ret) {
> > > +		DRM_ERROR("Failed to init CRTC\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	return ret;
> > > +}
> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> > > index aec3f180f96d..070613e32934 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > > @@ -6,7 +6,6 @@
> > >   */
> > >  
> > >  #include <linux/module.h>
> > > -#include <drm/drmP.h>
> > >  #include <drm/drm_gem.h>
> > >  #include <drm/drm_crtc_helper.h>
> > >  #include <drm/drm_atomic_helper.h>
> > > @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = {
> > >  	.minor			= DRIVER_MINOR,
> > >  };
> > >  
> > > -static const u32 vkms_formats[] = {
> > > -	DRM_FORMAT_XRGB8888,
> > > +static const struct drm_mode_config_funcs vkms_mode_funcs = {
> > > +	.atomic_check = drm_atomic_helper_check,
> > > +	.atomic_commit = drm_atomic_helper_commit,
> > >  };
> > >  
> > > -static void vkms_connector_destroy(struct drm_connector *connector)
> > > +static int vkms_modeset_init(struct vkms_device *vkmsdev)
> > >  {
> > > -	drm_connector_unregister(connector);
> > > -	drm_connector_cleanup(connector);
> > > -}
> > > +	struct drm_device *dev = &vkmsdev->drm;
> > >  
> > > -static const struct drm_connector_funcs vkms_connector_funcs = {
> > > -	.fill_modes = drm_helper_probe_single_connector_modes,
> > > -	.destroy = vkms_connector_destroy,
> > > -};
> > > +	drm_mode_config_init(dev);
> > > +	dev->mode_config.funcs = &vkms_mode_funcs;
> > > +	dev->mode_config.min_width = XRES_MIN;
> > > +	dev->mode_config.min_height = YRES_MIN;
> > > +	dev->mode_config.max_width = XRES_MAX;
> > > +	dev->mode_config.max_height = YRES_MAX;
> > >  
> > > -static const struct drm_mode_config_funcs vkms_mode_funcs = {
> > > -	.atomic_check = drm_atomic_helper_check,
> > > -	.atomic_commit = drm_atomic_helper_commit,
> > > -};
> > > +	return vkms_output_init(vkmsdev);
> > > +}
> > >  
> > >  static int __init vkms_init(void)
> > >  {
> > > @@ -98,48 +96,24 @@ static int __init vkms_init(void)
> > >  		goto out_fini;
> > >  	}
> > >  
> > > -	drm_mode_config_init(&vkms_device->drm);
> > > -	vkms_device->drm.mode_config.funcs = &vkms_mode_funcs;
> > > -	vkms_device->drm.mode_config.min_width = XRES_MIN;
> > > -	vkms_device->drm.mode_config.min_height = YRES_MIN;
> > > -	vkms_device->drm.mode_config.max_width = XRES_MAX;
> > > -	vkms_device->drm.mode_config.max_height = YRES_MAX;
> > > -
> > > -	ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector,
> > > -				 &vkms_connector_funcs,
> > > -				 DRM_MODE_CONNECTOR_VIRTUAL);
> > > -	if (ret < 0) {
> > > -		DRM_ERROR("Failed to init connector\n");
> > > -		goto out_unregister;
> > > -	}
> > > -
> > > -	ret = drm_simple_display_pipe_init(&vkms_device->drm,
> > > -					   &vkms_device->pipe,
> > > -					   NULL,
> > > -					   vkms_formats,
> > > -					   ARRAY_SIZE(vkms_formats),
> > > -					   NULL,
> > > -					   &vkms_device->connector);
> > > -	if (ret < 0) {
> > > -		DRM_ERROR("Cannot setup simple display pipe\n");
> > > +	ret = vkms_modeset_init(vkms_device);
> > > +	if (ret)
> > >  		goto out_unregister;
> > > -	}
> > >  
> > >  	ret = drm_dev_register(&vkms_device->drm, 0);
> > >  	if (ret)
> > >  		goto out_unregister;
> > >  
> > > -	drm_connector_register(&vkms_device->connector);
> > > -
> > >  	return 0;
> > >  
> > >  out_unregister:
> > >  	platform_device_unregister(vkms_device->platform);
> > > +
> > >  out_fini:
> > >  	drm_dev_fini(&vkms_device->drm);
> > > +
> > >  out_free:
> > >  	kfree(vkms_device);
> > > -
> > >  	return ret;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> > > index c77c5bf5032a..b0f9d2e61a42 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > > @@ -1,13 +1,31 @@
> > >  #ifndef _VKMS_DRV_H_
> > >  #define _VKMS_DRV_H_
> > >  
> > > -#include <drm/drm_simple_kms_helper.h>
> > > +#include <drm/drmP.h>
> > > +#include <drm/drm.h>
> > > +#include <drm/drm_encoder.h>
> > > +
> > > +static const u32 vkms_formats[] = {
> > > +	DRM_FORMAT_XRGB8888,
> > > +};
> > > +
> > > +struct vkms_output {
> > > +	struct drm_crtc crtc;
> > > +	struct drm_encoder encoder;
> > > +	struct drm_connector connector;
> > > +};
> > >  
> > >  struct vkms_device {
> > >  	struct drm_device drm;
> > >  	struct platform_device *platform;
> > > -	struct drm_simple_display_pipe pipe;
> > > -	struct drm_connector connector;
> > > +	struct vkms_output output;
> > >  };
> > >  
> > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> > > +		   struct drm_plane *primary, struct drm_plane *cursor);
> > > +
> > > +int vkms_output_init(struct vkms_device *vkmsdev);
> > > +
> > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
> > > +
> > >  #endif /* _VKMS_DRV_H_ */
> > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> > > new file mode 100644
> > > index 000000000000..48143eac3c12
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > > @@ -0,0 +1,91 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * 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.
> > > + */
> > > +
> > > +#include "vkms_drv.h"
> > > +#include <drm/drm_crtc_helper.h>
> > > +
> > > +static void vkms_connector_destroy(struct drm_connector *connector)
> > > +{
> > > +	drm_connector_unregister(connector);
> > > +	drm_connector_cleanup(connector);
> > > +}
> > > +
> > > +static const struct drm_connector_funcs vkms_connector_funcs = {
> > > +	.fill_modes = drm_helper_probe_single_connector_modes,
> > > +	.destroy = vkms_connector_destroy,
> > 
> > 2) adding the following hooks with their drm_atomic_helper_connector_*
> > counterpart to the vkms_connector_funcs:
> > atomic_duplicate_state, atomic_destroy_state, and reset.
> > 
> > > +};
> > > +
> > > +static const struct drm_encoder_funcs vkms_encoder_funcs = {
> > > +	.destroy = drm_encoder_cleanup,
> > > +};
> > > +
> > > +int vkms_output_init(struct vkms_device *vkmsdev)
> > > +{
> > > +	struct vkms_output *output = &vkmsdev->output;
> > > +	struct drm_device *dev = &vkmsdev->drm;
> > > +	struct drm_connector *connector = &output->connector;
> > > +	struct drm_encoder *encoder = &output->encoder;
> > > +	struct drm_crtc *crtc = &output->crtc;
> > > +	struct drm_plane *primary;
> > > +	int ret;
> > > +
> > > +	primary = vkms_plane_init(vkmsdev);
> > > +	if (IS_ERR(primary))
> > > +		return PTR_ERR(primary);
> > > +
> > > +	ret = vkms_crtc_init(dev, crtc, primary, NULL);
> > > +	if (ret)
> > > +		goto err_crtc;
> > > +
> > 
> > 3) add drm_connector_helper_funcs with dummy {get_modes, mode_valid}.
> > 
> > > +	ret = drm_connector_init(dev, connector, &vkms_connector_funcs,
> > > +				 DRM_MODE_CONNECTOR_VIRTUAL);
> > > +	if (ret) {
> > > +		DRM_ERROR("Failed to init connector\n");
> > > +		goto err_connector;
> > > +	}
> > > +
> > > +	ret = drm_connector_register(connector);
> > > +	if (ret) {
> > > +		DRM_ERROR("Failed to register connector\n");
> > > +		goto err_connector_register;
> > > +	}
> > > +
> > > +	ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs,
> > > +			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> > > +	if (ret) {
> > > +		DRM_ERROR("Failed to init encoder\n");
> > > +		goto err_encoder;
> > > +	}
> > > +	encoder->possible_crtcs = 1;
> > > +
> > > +	ret = drm_mode_connector_attach_encoder(connector, encoder);
> > > +	if (ret) {
> > > +		DRM_ERROR("Failed to attach connector to encoder\n");
> > > +		goto err_attach;
> > > +	}
> > > +
> > > +	drm_mode_config_reset(dev);
> > > +
> > > +	return 0;
> > > +
> > > +err_attach:
> > > +	drm_encoder_cleanup(encoder);
> > > +
> > > +err_encoder:
> > > +	drm_connector_unregister(connector);
> > > +
> > > +err_connector_register:
> > > +	drm_connector_cleanup(connector);
> > > +
> > > +err_connector:
> > > +	drm_crtc_cleanup(crtc);
> > > +
> > > +err_crtc:
> > > +	drm_plane_cleanup(primary);
> > > +	return ret;
> > > +}
> > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> > > new file mode 100644
> > > index 000000000000..2c25b1d6ab5b
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> > > @@ -0,0 +1,46 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * 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.
> > > + */
> > > +
> > > +#include "vkms_drv.h"
> > > +#include <drm/drm_plane_helper.h>
> > > +#include <drm/drm_atomic_helper.h>
> > > +
> > > +static const struct drm_plane_funcs vkms_plane_funcs = {
> > > +	.update_plane		= drm_atomic_helper_update_plane,
> > > +	.disable_plane		= drm_atomic_helper_disable_plane,
> > > +	.destroy		= drm_plane_cleanup,
> > > +	.reset			= drm_atomic_helper_plane_reset,
> > > +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> > > +	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
> > > +};
> > > +
> > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev)
> > > +{
> > > +	struct drm_device *dev = &vkmsdev->drm;
> > > +	struct drm_plane *plane;
> > > +	const u32 *formats;
> > > +	int ret, nformats;
> > > +
> > > +	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
> > > +	if (!plane)
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	formats = vkms_formats;
> > > +	nformats = ARRAY_SIZE(vkms_formats);
> > > +
> > > +	ret = drm_universal_plane_init(dev, plane, 0,
> > > +				       &vkms_plane_funcs,
> > > +				       formats, nformats,
> > > +				       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> > > +	if (ret) {
> > > +		kfree(plane);
> > > +		return ERR_PTR(ret);
> > > +	}
> > > +
> > 
> > 4) add drm_plane_helper_funcs with dummy {atomic_check, prepare_fb, cleanup_fb}.
> > 
> > > +	return plane;
> > > +}
> > > -- 
> > > 2.17.0
> > > 
> > 
> > - Haneen
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/3] drm/vkms: Add basic CRTC initialization

[Rodrigo Siqueira]

[-- Attachment #1.1: Type: text/plain, Size: 15931 bytes --]

Ahhhh, I got it!

Thanks a lot Haneen :)

On Sun, May 20, 2018 at 1:44 PM, Haneen Mohammed <hamohammed.sa@gmail.com>
wrote:

> On Sun, May 20, 2018 at 11:28:37AM -0300, Rodrigo Siqueira wrote:
> > Hi Haneen,
> >
> > Thanks for the feedback :)
> >
> > On 05/20, Haneen Mohammed wrote:
> > > On Wed, May 16, 2018 at 08:56:21PM -0300, Rodrigo Siqueira wrote:
> > > > This commit adds the essential infrastructure for around CRTCs which
> > > > is composed of: a new data struct for output data information, a
> > > > function for creating planes, and a simple encoder attached to the
> > > > connector. Finally, due to the introduction of a new initialization
> > > > function, connectors were moved from vkms_drv.c to vkms_display.c.
> > > >
> > > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > > ---
> > > >  drivers/gpu/drm/vkms/Makefile      |  2 +-
> > > >  drivers/gpu/drm/vkms/vkms_crtc.c   | 35 ++++++++++++
> > > >  drivers/gpu/drm/vkms/vkms_drv.c    | 60 ++++++--------------
> > > >  drivers/gpu/drm/vkms/vkms_drv.h    | 24 +++++++-
> > > >  drivers/gpu/drm/vkms/vkms_output.c | 91
> ++++++++++++++++++++++++++++++
> > > >  drivers/gpu/drm/vkms/vkms_plane.c  | 46 +++++++++++++++
> > > >  6 files changed, 211 insertions(+), 47 deletions(-)
> > > >  create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c
> > > >  create mode 100644 drivers/gpu/drm/vkms/vkms_output.c
> > > >  create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c
> > > >
> > > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/
> Makefile
> > > > index 2aef948d3a34..3f774a6a9c58 100644
> > > > --- a/drivers/gpu/drm/vkms/Makefile
> > > > +++ b/drivers/gpu/drm/vkms/Makefile
> > > > @@ -1,3 +1,3 @@
> > > > -vkms-y := vkms_drv.o
> > > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
> > > >
> > > >  obj-$(CONFIG_DRM_VKMS) += vkms.o
> > > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > new file mode 100644
> > > > index 000000000000..bf76cd39ece7
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > @@ -0,0 +1,35 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * 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.
> > > > + */
> > > > +
> > > > +#include "vkms_drv.h"
> > > > +#include <drm/drm_atomic_helper.h>
> > > > +#include <drm/drm_crtc_helper.h>
> > > > +
> > > > +static const struct drm_crtc_funcs vkms_crtc_funcs = {
> > > > + .set_config             = drm_atomic_helper_set_config,
> > > > + .destroy                = drm_crtc_cleanup,
> > > > + .page_flip              = drm_atomic_helper_page_flip,
> > > > + .reset                  = drm_atomic_helper_crtc_reset,
> > > > + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> > > > + .atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
> > > > +};
> > > > +
> > > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> > > > +            struct drm_plane *primary, struct drm_plane *cursor)
> > > > +{
> > > > + int ret;
> > > > +
> > >
> > > Just as a follow up, I have noticed that vkms breaks when inspecting
> > > its state in the debugfs, also when running igt tests with kms and core
> > > filters, and adding the following fixed that issue:
> >
> > Could you explain me how you tested the state with debugfs?
> >
> > I will add the fixes suggested in your comments.
> >
> > Thanks
> >
>
> I just tried to read this file: /sys/kernel/debug/dri/0/state, which
> then caused a null pointer dereference error.
>
> > > 1) a drm_crtc_helper_funcs with dummy atomic_check.
> > >
> > > > + ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor,
> > > > +                                 &vkms_crtc_funcs, NULL);
> > > > + if (ret) {
> > > > +         DRM_ERROR("Failed to init CRTC\n");
> > > > +         return ret;
> > > > + }
> > > > +
> > > > + return ret;
> > > > +}
> > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c
> b/drivers/gpu/drm/vkms/vkms_drv.c
> > > > index aec3f180f96d..070613e32934 100644
> > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > > > @@ -6,7 +6,6 @@
> > > >   */
> > > >
> > > >  #include <linux/module.h>
> > > > -#include <drm/drmP.h>
> > > >  #include <drm/drm_gem.h>
> > > >  #include <drm/drm_crtc_helper.h>
> > > >  #include <drm/drm_atomic_helper.h>
> > > > @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = {
> > > >   .minor                  = DRIVER_MINOR,
> > > >  };
> > > >
> > > > -static const u32 vkms_formats[] = {
> > > > - DRM_FORMAT_XRGB8888,
> > > > +static const struct drm_mode_config_funcs vkms_mode_funcs = {
> > > > + .atomic_check = drm_atomic_helper_check,
> > > > + .atomic_commit = drm_atomic_helper_commit,
> > > >  };
> > > >
> > > > -static void vkms_connector_destroy(struct drm_connector *connector)
> > > > +static int vkms_modeset_init(struct vkms_device *vkmsdev)
> > > >  {
> > > > - drm_connector_unregister(connector);
> > > > - drm_connector_cleanup(connector);
> > > > -}
> > > > + struct drm_device *dev = &vkmsdev->drm;
> > > >
> > > > -static const struct drm_connector_funcs vkms_connector_funcs = {
> > > > - .fill_modes = drm_helper_probe_single_connector_modes,
> > > > - .destroy = vkms_connector_destroy,
> > > > -};
> > > > + drm_mode_config_init(dev);
> > > > + dev->mode_config.funcs = &vkms_mode_funcs;
> > > > + dev->mode_config.min_width = XRES_MIN;
> > > > + dev->mode_config.min_height = YRES_MIN;
> > > > + dev->mode_config.max_width = XRES_MAX;
> > > > + dev->mode_config.max_height = YRES_MAX;
> > > >
> > > > -static const struct drm_mode_config_funcs vkms_mode_funcs = {
> > > > - .atomic_check = drm_atomic_helper_check,
> > > > - .atomic_commit = drm_atomic_helper_commit,
> > > > -};
> > > > + return vkms_output_init(vkmsdev);
> > > > +}
> > > >
> > > >  static int __init vkms_init(void)
> > > >  {
> > > > @@ -98,48 +96,24 @@ static int __init vkms_init(void)
> > > >           goto out_fini;
> > > >   }
> > > >
> > > > - drm_mode_config_init(&vkms_device->drm);
> > > > - vkms_device->drm.mode_config.funcs = &vkms_mode_funcs;
> > > > - vkms_device->drm.mode_config.min_width = XRES_MIN;
> > > > - vkms_device->drm.mode_config.min_height = YRES_MIN;
> > > > - vkms_device->drm.mode_config.max_width = XRES_MAX;
> > > > - vkms_device->drm.mode_config.max_height = YRES_MAX;
> > > > -
> > > > - ret = drm_connector_init(&vkms_device->drm,
> &vkms_device->connector,
> > > > -                          &vkms_connector_funcs,
> > > > -                          DRM_MODE_CONNECTOR_VIRTUAL);
> > > > - if (ret < 0) {
> > > > -         DRM_ERROR("Failed to init connector\n");
> > > > -         goto out_unregister;
> > > > - }
> > > > -
> > > > - ret = drm_simple_display_pipe_init(&vkms_device->drm,
> > > > -                                    &vkms_device->pipe,
> > > > -                                    NULL,
> > > > -                                    vkms_formats,
> > > > -                                    ARRAY_SIZE(vkms_formats),
> > > > -                                    NULL,
> > > > -                                    &vkms_device->connector);
> > > > - if (ret < 0) {
> > > > -         DRM_ERROR("Cannot setup simple display pipe\n");
> > > > + ret = vkms_modeset_init(vkms_device);
> > > > + if (ret)
> > > >           goto out_unregister;
> > > > - }
> > > >
> > > >   ret = drm_dev_register(&vkms_device->drm, 0);
> > > >   if (ret)
> > > >           goto out_unregister;
> > > >
> > > > - drm_connector_register(&vkms_device->connector);
> > > > -
> > > >   return 0;
> > > >
> > > >  out_unregister:
> > > >   platform_device_unregister(vkms_device->platform);
> > > > +
> > > >  out_fini:
> > > >   drm_dev_fini(&vkms_device->drm);
> > > > +
> > > >  out_free:
> > > >   kfree(vkms_device);
> > > > -
> > > >   return ret;
> > > >  }
> > > >
> > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h
> b/drivers/gpu/drm/vkms/vkms_drv.h
> > > > index c77c5bf5032a..b0f9d2e61a42 100644
> > > > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > > > @@ -1,13 +1,31 @@
> > > >  #ifndef _VKMS_DRV_H_
> > > >  #define _VKMS_DRV_H_
> > > >
> > > > -#include <drm/drm_simple_kms_helper.h>
> > > > +#include <drm/drmP.h>
> > > > +#include <drm/drm.h>
> > > > +#include <drm/drm_encoder.h>
> > > > +
> > > > +static const u32 vkms_formats[] = {
> > > > + DRM_FORMAT_XRGB8888,
> > > > +};
> > > > +
> > > > +struct vkms_output {
> > > > + struct drm_crtc crtc;
> > > > + struct drm_encoder encoder;
> > > > + struct drm_connector connector;
> > > > +};
> > > >
> > > >  struct vkms_device {
> > > >   struct drm_device drm;
> > > >   struct platform_device *platform;
> > > > - struct drm_simple_display_pipe pipe;
> > > > - struct drm_connector connector;
> > > > + struct vkms_output output;
> > > >  };
> > > >
> > > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> > > > +            struct drm_plane *primary, struct drm_plane *cursor);
> > > > +
> > > > +int vkms_output_init(struct vkms_device *vkmsdev);
> > > > +
> > > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
> > > > +
> > > >  #endif /* _VKMS_DRV_H_ */
> > > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c
> b/drivers/gpu/drm/vkms/vkms_output.c
> > > > new file mode 100644
> > > > index 000000000000..48143eac3c12
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > > > @@ -0,0 +1,91 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * 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.
> > > > + */
> > > > +
> > > > +#include "vkms_drv.h"
> > > > +#include <drm/drm_crtc_helper.h>
> > > > +
> > > > +static void vkms_connector_destroy(struct drm_connector *connector)
> > > > +{
> > > > + drm_connector_unregister(connector);
> > > > + drm_connector_cleanup(connector);
> > > > +}
> > > > +
> > > > +static const struct drm_connector_funcs vkms_connector_funcs = {
> > > > + .fill_modes = drm_helper_probe_single_connector_modes,
> > > > + .destroy = vkms_connector_destroy,
> > >
> > > 2) adding the following hooks with their drm_atomic_helper_connector_*
> > > counterpart to the vkms_connector_funcs:
> > > atomic_duplicate_state, atomic_destroy_state, and reset.
> > >
> > > > +};
> > > > +
> > > > +static const struct drm_encoder_funcs vkms_encoder_funcs = {
> > > > + .destroy = drm_encoder_cleanup,
> > > > +};
> > > > +
> > > > +int vkms_output_init(struct vkms_device *vkmsdev)
> > > > +{
> > > > + struct vkms_output *output = &vkmsdev->output;
> > > > + struct drm_device *dev = &vkmsdev->drm;
> > > > + struct drm_connector *connector = &output->connector;
> > > > + struct drm_encoder *encoder = &output->encoder;
> > > > + struct drm_crtc *crtc = &output->crtc;
> > > > + struct drm_plane *primary;
> > > > + int ret;
> > > > +
> > > > + primary = vkms_plane_init(vkmsdev);
> > > > + if (IS_ERR(primary))
> > > > +         return PTR_ERR(primary);
> > > > +
> > > > + ret = vkms_crtc_init(dev, crtc, primary, NULL);
> > > > + if (ret)
> > > > +         goto err_crtc;
> > > > +
> > >
> > > 3) add drm_connector_helper_funcs with dummy {get_modes, mode_valid}.
> > >
> > > > + ret = drm_connector_init(dev, connector, &vkms_connector_funcs,
> > > > +                          DRM_MODE_CONNECTOR_VIRTUAL);
> > > > + if (ret) {
> > > > +         DRM_ERROR("Failed to init connector\n");
> > > > +         goto err_connector;
> > > > + }
> > > > +
> > > > + ret = drm_connector_register(connector);
> > > > + if (ret) {
> > > > +         DRM_ERROR("Failed to register connector\n");
> > > > +         goto err_connector_register;
> > > > + }
> > > > +
> > > > + ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs,
> > > > +                        DRM_MODE_ENCODER_VIRTUAL, NULL);
> > > > + if (ret) {
> > > > +         DRM_ERROR("Failed to init encoder\n");
> > > > +         goto err_encoder;
> > > > + }
> > > > + encoder->possible_crtcs = 1;
> > > > +
> > > > + ret = drm_mode_connector_attach_encoder(connector, encoder);
> > > > + if (ret) {
> > > > +         DRM_ERROR("Failed to attach connector to encoder\n");
> > > > +         goto err_attach;
> > > > + }
> > > > +
> > > > + drm_mode_config_reset(dev);
> > > > +
> > > > + return 0;
> > > > +
> > > > +err_attach:
> > > > + drm_encoder_cleanup(encoder);
> > > > +
> > > > +err_encoder:
> > > > + drm_connector_unregister(connector);
> > > > +
> > > > +err_connector_register:
> > > > + drm_connector_cleanup(connector);
> > > > +
> > > > +err_connector:
> > > > + drm_crtc_cleanup(crtc);
> > > > +
> > > > +err_crtc:
> > > > + drm_plane_cleanup(primary);
> > > > + return ret;
> > > > +}
> > > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c
> b/drivers/gpu/drm/vkms/vkms_plane.c
> > > > new file mode 100644
> > > > index 000000000000..2c25b1d6ab5b
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> > > > @@ -0,0 +1,46 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * 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.
> > > > + */
> > > > +
> > > > +#include "vkms_drv.h"
> > > > +#include <drm/drm_plane_helper.h>
> > > > +#include <drm/drm_atomic_helper.h>
> > > > +
> > > > +static const struct drm_plane_funcs vkms_plane_funcs = {
> > > > + .update_plane           = drm_atomic_helper_update_plane,
> > > > + .disable_plane          = drm_atomic_helper_disable_plane,
> > > > + .destroy                = drm_plane_cleanup,
> > > > + .reset                  = drm_atomic_helper_plane_reset,
> > > > + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> > > > + .atomic_destroy_state   = drm_atomic_helper_plane_destroy_state,
> > > > +};
> > > > +
> > > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev)
> > > > +{
> > > > + struct drm_device *dev = &vkmsdev->drm;
> > > > + struct drm_plane *plane;
> > > > + const u32 *formats;
> > > > + int ret, nformats;
> > > > +
> > > > + plane = kzalloc(sizeof(*plane), GFP_KERNEL);
> > > > + if (!plane)
> > > > +         return ERR_PTR(-ENOMEM);
> > > > +
> > > > + formats = vkms_formats;
> > > > + nformats = ARRAY_SIZE(vkms_formats);
> > > > +
> > > > + ret = drm_universal_plane_init(dev, plane, 0,
> > > > +                                &vkms_plane_funcs,
> > > > +                                formats, nformats,
> > > > +                                NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> > > > + if (ret) {
> > > > +         kfree(plane);
> > > > +         return ERR_PTR(ret);
> > > > + }
> > > > +
> > >
> > > 4) add drm_plane_helper_funcs with dummy {atomic_check, prepare_fb,
> cleanup_fb}.
> > >
> > > > + return plane;
> > > > +}
> > > > --
> > > > 2.17.0
> > > >
> > >
> > > - Haneen
>



-- 

Rodrigo Siqueira
Graduate Student
Department of Computer Science
University of São Paulo

[-- Attachment #1.2: Type: text/html, Size: 22191 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v2 2/3] drm/vkms: Add basic CRTC initialization

[Daniel Vetter]

On Sun, May 20, 2018 at 09:22:31AM +0300, Haneen Mohammed wrote:
> On Wed, May 16, 2018 at 08:56:21PM -0300, Rodrigo Siqueira wrote:
> > This commit adds the essential infrastructure for around CRTCs which
> > is composed of: a new data struct for output data information, a
> > function for creating planes, and a simple encoder attached to the
> > connector. Finally, due to the introduction of a new initialization
> > function, connectors were moved from vkms_drv.c to vkms_display.c.
> > 
> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > ---
> >  drivers/gpu/drm/vkms/Makefile      |  2 +-
> >  drivers/gpu/drm/vkms/vkms_crtc.c   | 35 ++++++++++++
> >  drivers/gpu/drm/vkms/vkms_drv.c    | 60 ++++++--------------
> >  drivers/gpu/drm/vkms/vkms_drv.h    | 24 +++++++-
> >  drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/vkms/vkms_plane.c  | 46 +++++++++++++++
> >  6 files changed, 211 insertions(+), 47 deletions(-)
> >  create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c
> >  create mode 100644 drivers/gpu/drm/vkms/vkms_output.c
> >  create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c
> > 
> > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> > index 2aef948d3a34..3f774a6a9c58 100644
> > --- a/drivers/gpu/drm/vkms/Makefile
> > +++ b/drivers/gpu/drm/vkms/Makefile
> > @@ -1,3 +1,3 @@
> > -vkms-y := vkms_drv.o
> > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
> >  
> >  obj-$(CONFIG_DRM_VKMS) += vkms.o
> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > new file mode 100644
> > index 000000000000..bf76cd39ece7
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -0,0 +1,35 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * 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.
> > + */
> > +
> > +#include "vkms_drv.h"
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_crtc_helper.h>
> > +
> > +static const struct drm_crtc_funcs vkms_crtc_funcs = {
> > +	.set_config             = drm_atomic_helper_set_config,
> > +	.destroy                = drm_crtc_cleanup,
> > +	.page_flip              = drm_atomic_helper_page_flip,
> > +	.reset                  = drm_atomic_helper_crtc_reset,
> > +	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> > +	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
> > +};
> > +
> > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> > +		   struct drm_plane *primary, struct drm_plane *cursor)
> > +{
> > +	int ret;
> > +
> 
> Just as a follow up, I have noticed that vkms breaks when inspecting
> its state in the debugfs, also when running igt tests with kms and core
> filters, and adding the following fixed that issue:
> 
> 1) a drm_crtc_helper_funcs with dummy atomic_check.

We're trying to make callbacks as optional as possible, this was probably
just an oversight. Can you try to find the place where this is called, and
add suitable checks for NULL _funcs pointer?

Thanks, Daniel
> 
> > +	ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor,
> > +					&vkms_crtc_funcs, NULL);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to init CRTC\n");
> > +		return ret;
> > +	}
> > +
> > +	return ret;
> > +}
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> > index aec3f180f96d..070613e32934 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -6,7 +6,6 @@
> >   */
> >  
> >  #include <linux/module.h>
> > -#include <drm/drmP.h>
> >  #include <drm/drm_gem.h>
> >  #include <drm/drm_crtc_helper.h>
> >  #include <drm/drm_atomic_helper.h>
> > @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = {
> >  	.minor			= DRIVER_MINOR,
> >  };
> >  
> > -static const u32 vkms_formats[] = {
> > -	DRM_FORMAT_XRGB8888,
> > +static const struct drm_mode_config_funcs vkms_mode_funcs = {
> > +	.atomic_check = drm_atomic_helper_check,
> > +	.atomic_commit = drm_atomic_helper_commit,
> >  };
> >  
> > -static void vkms_connector_destroy(struct drm_connector *connector)
> > +static int vkms_modeset_init(struct vkms_device *vkmsdev)
> >  {
> > -	drm_connector_unregister(connector);
> > -	drm_connector_cleanup(connector);
> > -}
> > +	struct drm_device *dev = &vkmsdev->drm;
> >  
> > -static const struct drm_connector_funcs vkms_connector_funcs = {
> > -	.fill_modes = drm_helper_probe_single_connector_modes,
> > -	.destroy = vkms_connector_destroy,
> > -};
> > +	drm_mode_config_init(dev);
> > +	dev->mode_config.funcs = &vkms_mode_funcs;
> > +	dev->mode_config.min_width = XRES_MIN;
> > +	dev->mode_config.min_height = YRES_MIN;
> > +	dev->mode_config.max_width = XRES_MAX;
> > +	dev->mode_config.max_height = YRES_MAX;
> >  
> > -static const struct drm_mode_config_funcs vkms_mode_funcs = {
> > -	.atomic_check = drm_atomic_helper_check,
> > -	.atomic_commit = drm_atomic_helper_commit,
> > -};
> > +	return vkms_output_init(vkmsdev);
> > +}
> >  
> >  static int __init vkms_init(void)
> >  {
> > @@ -98,48 +96,24 @@ static int __init vkms_init(void)
> >  		goto out_fini;
> >  	}
> >  
> > -	drm_mode_config_init(&vkms_device->drm);
> > -	vkms_device->drm.mode_config.funcs = &vkms_mode_funcs;
> > -	vkms_device->drm.mode_config.min_width = XRES_MIN;
> > -	vkms_device->drm.mode_config.min_height = YRES_MIN;
> > -	vkms_device->drm.mode_config.max_width = XRES_MAX;
> > -	vkms_device->drm.mode_config.max_height = YRES_MAX;
> > -
> > -	ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector,
> > -				 &vkms_connector_funcs,
> > -				 DRM_MODE_CONNECTOR_VIRTUAL);
> > -	if (ret < 0) {
> > -		DRM_ERROR("Failed to init connector\n");
> > -		goto out_unregister;
> > -	}
> > -
> > -	ret = drm_simple_display_pipe_init(&vkms_device->drm,
> > -					   &vkms_device->pipe,
> > -					   NULL,
> > -					   vkms_formats,
> > -					   ARRAY_SIZE(vkms_formats),
> > -					   NULL,
> > -					   &vkms_device->connector);
> > -	if (ret < 0) {
> > -		DRM_ERROR("Cannot setup simple display pipe\n");
> > +	ret = vkms_modeset_init(vkms_device);
> > +	if (ret)
> >  		goto out_unregister;
> > -	}
> >  
> >  	ret = drm_dev_register(&vkms_device->drm, 0);
> >  	if (ret)
> >  		goto out_unregister;
> >  
> > -	drm_connector_register(&vkms_device->connector);
> > -
> >  	return 0;
> >  
> >  out_unregister:
> >  	platform_device_unregister(vkms_device->platform);
> > +
> >  out_fini:
> >  	drm_dev_fini(&vkms_device->drm);
> > +
> >  out_free:
> >  	kfree(vkms_device);
> > -
> >  	return ret;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> > index c77c5bf5032a..b0f9d2e61a42 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -1,13 +1,31 @@
> >  #ifndef _VKMS_DRV_H_
> >  #define _VKMS_DRV_H_
> >  
> > -#include <drm/drm_simple_kms_helper.h>
> > +#include <drm/drmP.h>
> > +#include <drm/drm.h>
> > +#include <drm/drm_encoder.h>
> > +
> > +static const u32 vkms_formats[] = {
> > +	DRM_FORMAT_XRGB8888,
> > +};
> > +
> > +struct vkms_output {
> > +	struct drm_crtc crtc;
> > +	struct drm_encoder encoder;
> > +	struct drm_connector connector;
> > +};
> >  
> >  struct vkms_device {
> >  	struct drm_device drm;
> >  	struct platform_device *platform;
> > -	struct drm_simple_display_pipe pipe;
> > -	struct drm_connector connector;
> > +	struct vkms_output output;
> >  };
> >  
> > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> > +		   struct drm_plane *primary, struct drm_plane *cursor);
> > +
> > +int vkms_output_init(struct vkms_device *vkmsdev);
> > +
> > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
> > +
> >  #endif /* _VKMS_DRV_H_ */
> > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> > new file mode 100644
> > index 000000000000..48143eac3c12
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -0,0 +1,91 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * 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.
> > + */
> > +
> > +#include "vkms_drv.h"
> > +#include <drm/drm_crtc_helper.h>
> > +
> > +static void vkms_connector_destroy(struct drm_connector *connector)
> > +{
> > +	drm_connector_unregister(connector);
> > +	drm_connector_cleanup(connector);
> > +}
> > +
> > +static const struct drm_connector_funcs vkms_connector_funcs = {
> > +	.fill_modes = drm_helper_probe_single_connector_modes,
> > +	.destroy = vkms_connector_destroy,
> 
> 2) adding the following hooks with their drm_atomic_helper_connector_*
> counterpart to the vkms_connector_funcs:
> atomic_duplicate_state, atomic_destroy_state, and reset.
> 
> > +};
> > +
> > +static const struct drm_encoder_funcs vkms_encoder_funcs = {
> > +	.destroy = drm_encoder_cleanup,
> > +};
> > +
> > +int vkms_output_init(struct vkms_device *vkmsdev)
> > +{
> > +	struct vkms_output *output = &vkmsdev->output;
> > +	struct drm_device *dev = &vkmsdev->drm;
> > +	struct drm_connector *connector = &output->connector;
> > +	struct drm_encoder *encoder = &output->encoder;
> > +	struct drm_crtc *crtc = &output->crtc;
> > +	struct drm_plane *primary;
> > +	int ret;
> > +
> > +	primary = vkms_plane_init(vkmsdev);
> > +	if (IS_ERR(primary))
> > +		return PTR_ERR(primary);
> > +
> > +	ret = vkms_crtc_init(dev, crtc, primary, NULL);
> > +	if (ret)
> > +		goto err_crtc;
> > +
> 
> 3) add drm_connector_helper_funcs with dummy {get_modes, mode_valid}.
> 
> > +	ret = drm_connector_init(dev, connector, &vkms_connector_funcs,
> > +				 DRM_MODE_CONNECTOR_VIRTUAL);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to init connector\n");
> > +		goto err_connector;
> > +	}
> > +
> > +	ret = drm_connector_register(connector);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to register connector\n");
> > +		goto err_connector_register;
> > +	}
> > +
> > +	ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs,
> > +			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to init encoder\n");
> > +		goto err_encoder;
> > +	}
> > +	encoder->possible_crtcs = 1;
> > +
> > +	ret = drm_mode_connector_attach_encoder(connector, encoder);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to attach connector to encoder\n");
> > +		goto err_attach;
> > +	}
> > +
> > +	drm_mode_config_reset(dev);
> > +
> > +	return 0;
> > +
> > +err_attach:
> > +	drm_encoder_cleanup(encoder);
> > +
> > +err_encoder:
> > +	drm_connector_unregister(connector);
> > +
> > +err_connector_register:
> > +	drm_connector_cleanup(connector);
> > +
> > +err_connector:
> > +	drm_crtc_cleanup(crtc);
> > +
> > +err_crtc:
> > +	drm_plane_cleanup(primary);
> > +	return ret;
> > +}
> > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> > new file mode 100644
> > index 000000000000..2c25b1d6ab5b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> > @@ -0,0 +1,46 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * 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.
> > + */
> > +
> > +#include "vkms_drv.h"
> > +#include <drm/drm_plane_helper.h>
> > +#include <drm/drm_atomic_helper.h>
> > +
> > +static const struct drm_plane_funcs vkms_plane_funcs = {
> > +	.update_plane		= drm_atomic_helper_update_plane,
> > +	.disable_plane		= drm_atomic_helper_disable_plane,
> > +	.destroy		= drm_plane_cleanup,
> > +	.reset			= drm_atomic_helper_plane_reset,
> > +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> > +	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
> > +};
> > +
> > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev)
> > +{
> > +	struct drm_device *dev = &vkmsdev->drm;
> > +	struct drm_plane *plane;
> > +	const u32 *formats;
> > +	int ret, nformats;
> > +
> > +	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
> > +	if (!plane)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	formats = vkms_formats;
> > +	nformats = ARRAY_SIZE(vkms_formats);
> > +
> > +	ret = drm_universal_plane_init(dev, plane, 0,
> > +				       &vkms_plane_funcs,
> > +				       formats, nformats,
> > +				       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> > +	if (ret) {
> > +		kfree(plane);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> 
> 4) add drm_plane_helper_funcs with dummy {atomic_check, prepare_fb, cleanup_fb}.
> 
> > +	return plane;
> > +}
> > -- 
> > 2.17.0
> > 
> 
> - Haneen

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/3] drm/vkms: Add basic CRTC initialization

[Haneen Mohammed]

On Wed, May 23, 2018 at 10:53:33AM +0200, Daniel Vetter wrote:
> On Sun, May 20, 2018 at 09:22:31AM +0300, Haneen Mohammed wrote:
> > On Wed, May 16, 2018 at 08:56:21PM -0300, Rodrigo Siqueira wrote:
> > > This commit adds the essential infrastructure for around CRTCs which
> > > is composed of: a new data struct for output data information, a
> > > function for creating planes, and a simple encoder attached to the
> > > connector. Finally, due to the introduction of a new initialization
> > > function, connectors were moved from vkms_drv.c to vkms_display.c.
> > > 
> > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > ---
> > >  drivers/gpu/drm/vkms/Makefile      |  2 +-
> > >  drivers/gpu/drm/vkms/vkms_crtc.c   | 35 ++++++++++++
> > >  drivers/gpu/drm/vkms/vkms_drv.c    | 60 ++++++--------------
> > >  drivers/gpu/drm/vkms/vkms_drv.h    | 24 +++++++-
> > >  drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/vkms/vkms_plane.c  | 46 +++++++++++++++
> > >  6 files changed, 211 insertions(+), 47 deletions(-)
> > >  create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c
> > >  create mode 100644 drivers/gpu/drm/vkms/vkms_output.c
> > >  create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c
> > > 
> > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> > > index 2aef948d3a34..3f774a6a9c58 100644
> > > --- a/drivers/gpu/drm/vkms/Makefile
> > > +++ b/drivers/gpu/drm/vkms/Makefile
> > > @@ -1,3 +1,3 @@
> > > -vkms-y := vkms_drv.o
> > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
> > >  
> > >  obj-$(CONFIG_DRM_VKMS) += vkms.o
> > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > new file mode 100644
> > > index 000000000000..bf76cd39ece7
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > @@ -0,0 +1,35 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * 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.
> > > + */
> > > +
> > > +#include "vkms_drv.h"
> > > +#include <drm/drm_atomic_helper.h>
> > > +#include <drm/drm_crtc_helper.h>
> > > +
> > > +static const struct drm_crtc_funcs vkms_crtc_funcs = {
> > > +	.set_config             = drm_atomic_helper_set_config,
> > > +	.destroy                = drm_crtc_cleanup,
> > > +	.page_flip              = drm_atomic_helper_page_flip,
> > > +	.reset                  = drm_atomic_helper_crtc_reset,
> > > +	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> > > +	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
> > > +};
> > > +
> > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> > > +		   struct drm_plane *primary, struct drm_plane *cursor)
> > > +{
> > > +	int ret;
> > > +
> > 
> > Just as a follow up, I have noticed that vkms breaks when inspecting
> > its state in the debugfs, also when running igt tests with kms and core
> > filters, and adding the following fixed that issue:
> > 
> > 1) a drm_crtc_helper_funcs with dummy atomic_check.
> 
> We're trying to make callbacks as optional as possible, this was probably
> just an oversight. Can you try to find the place where this is called, and
> add suitable checks for NULL _funcs pointer?
> 
> Thanks, Daniel

Sure, I'll look for that.

> > 
> > > +	ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor,
> > > +					&vkms_crtc_funcs, NULL);
> > > +	if (ret) {
> > > +		DRM_ERROR("Failed to init CRTC\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	return ret;
> > > +}
> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> > > index aec3f180f96d..070613e32934 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > > @@ -6,7 +6,6 @@
> > >   */
> > >  
> > >  #include <linux/module.h>
> > > -#include <drm/drmP.h>
> > >  #include <drm/drm_gem.h>
> > >  #include <drm/drm_crtc_helper.h>
> > >  #include <drm/drm_atomic_helper.h>
> > > @@ -59,25 +58,24 @@ static struct drm_driver vkms_driver = {
> > >  	.minor			= DRIVER_MINOR,
> > >  };
> > >  
> > > -static const u32 vkms_formats[] = {
> > > -	DRM_FORMAT_XRGB8888,
> > > +static const struct drm_mode_config_funcs vkms_mode_funcs = {
> > > +	.atomic_check = drm_atomic_helper_check,
> > > +	.atomic_commit = drm_atomic_helper_commit,
> > >  };
> > >  
> > > -static void vkms_connector_destroy(struct drm_connector *connector)
> > > +static int vkms_modeset_init(struct vkms_device *vkmsdev)
> > >  {
> > > -	drm_connector_unregister(connector);
> > > -	drm_connector_cleanup(connector);
> > > -}
> > > +	struct drm_device *dev = &vkmsdev->drm;
> > >  
> > > -static const struct drm_connector_funcs vkms_connector_funcs = {
> > > -	.fill_modes = drm_helper_probe_single_connector_modes,
> > > -	.destroy = vkms_connector_destroy,
> > > -};
> > > +	drm_mode_config_init(dev);
> > > +	dev->mode_config.funcs = &vkms_mode_funcs;
> > > +	dev->mode_config.min_width = XRES_MIN;
> > > +	dev->mode_config.min_height = YRES_MIN;
> > > +	dev->mode_config.max_width = XRES_MAX;
> > > +	dev->mode_config.max_height = YRES_MAX;
> > >  
> > > -static const struct drm_mode_config_funcs vkms_mode_funcs = {
> > > -	.atomic_check = drm_atomic_helper_check,
> > > -	.atomic_commit = drm_atomic_helper_commit,
> > > -};
> > > +	return vkms_output_init(vkmsdev);
> > > +}
> > >  
> > >  static int __init vkms_init(void)
> > >  {
> > > @@ -98,48 +96,24 @@ static int __init vkms_init(void)
> > >  		goto out_fini;
> > >  	}
> > >  
> > > -	drm_mode_config_init(&vkms_device->drm);
> > > -	vkms_device->drm.mode_config.funcs = &vkms_mode_funcs;
> > > -	vkms_device->drm.mode_config.min_width = XRES_MIN;
> > > -	vkms_device->drm.mode_config.min_height = YRES_MIN;
> > > -	vkms_device->drm.mode_config.max_width = XRES_MAX;
> > > -	vkms_device->drm.mode_config.max_height = YRES_MAX;
> > > -
> > > -	ret = drm_connector_init(&vkms_device->drm, &vkms_device->connector,
> > > -				 &vkms_connector_funcs,
> > > -				 DRM_MODE_CONNECTOR_VIRTUAL);
> > > -	if (ret < 0) {
> > > -		DRM_ERROR("Failed to init connector\n");
> > > -		goto out_unregister;
> > > -	}
> > > -
> > > -	ret = drm_simple_display_pipe_init(&vkms_device->drm,
> > > -					   &vkms_device->pipe,
> > > -					   NULL,
> > > -					   vkms_formats,
> > > -					   ARRAY_SIZE(vkms_formats),
> > > -					   NULL,
> > > -					   &vkms_device->connector);
> > > -	if (ret < 0) {
> > > -		DRM_ERROR("Cannot setup simple display pipe\n");
> > > +	ret = vkms_modeset_init(vkms_device);
> > > +	if (ret)
> > >  		goto out_unregister;
> > > -	}
> > >  
> > >  	ret = drm_dev_register(&vkms_device->drm, 0);
> > >  	if (ret)
> > >  		goto out_unregister;
> > >  
> > > -	drm_connector_register(&vkms_device->connector);
> > > -
> > >  	return 0;
> > >  
> > >  out_unregister:
> > >  	platform_device_unregister(vkms_device->platform);
> > > +
> > >  out_fini:
> > >  	drm_dev_fini(&vkms_device->drm);
> > > +
> > >  out_free:
> > >  	kfree(vkms_device);
> > > -
> > >  	return ret;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> > > index c77c5bf5032a..b0f9d2e61a42 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > > @@ -1,13 +1,31 @@
> > >  #ifndef _VKMS_DRV_H_
> > >  #define _VKMS_DRV_H_
> > >  
> > > -#include <drm/drm_simple_kms_helper.h>
> > > +#include <drm/drmP.h>
> > > +#include <drm/drm.h>
> > > +#include <drm/drm_encoder.h>
> > > +
> > > +static const u32 vkms_formats[] = {
> > > +	DRM_FORMAT_XRGB8888,
> > > +};
> > > +
> > > +struct vkms_output {
> > > +	struct drm_crtc crtc;
> > > +	struct drm_encoder encoder;
> > > +	struct drm_connector connector;
> > > +};
> > >  
> > >  struct vkms_device {
> > >  	struct drm_device drm;
> > >  	struct platform_device *platform;
> > > -	struct drm_simple_display_pipe pipe;
> > > -	struct drm_connector connector;
> > > +	struct vkms_output output;
> > >  };
> > >  
> > > +int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> > > +		   struct drm_plane *primary, struct drm_plane *cursor);
> > > +
> > > +int vkms_output_init(struct vkms_device *vkmsdev);
> > > +
> > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
> > > +
> > >  #endif /* _VKMS_DRV_H_ */
> > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> > > new file mode 100644
> > > index 000000000000..48143eac3c12
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > > @@ -0,0 +1,91 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * 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.
> > > + */
> > > +
> > > +#include "vkms_drv.h"
> > > +#include <drm/drm_crtc_helper.h>
> > > +
> > > +static void vkms_connector_destroy(struct drm_connector *connector)
> > > +{
> > > +	drm_connector_unregister(connector);
> > > +	drm_connector_cleanup(connector);
> > > +}
> > > +
> > > +static const struct drm_connector_funcs vkms_connector_funcs = {
> > > +	.fill_modes = drm_helper_probe_single_connector_modes,
> > > +	.destroy = vkms_connector_destroy,
> > 
> > 2) adding the following hooks with their drm_atomic_helper_connector_*
> > counterpart to the vkms_connector_funcs:
> > atomic_duplicate_state, atomic_destroy_state, and reset.
> > 
> > > +};
> > > +
> > > +static const struct drm_encoder_funcs vkms_encoder_funcs = {
> > > +	.destroy = drm_encoder_cleanup,
> > > +};
> > > +
> > > +int vkms_output_init(struct vkms_device *vkmsdev)
> > > +{
> > > +	struct vkms_output *output = &vkmsdev->output;
> > > +	struct drm_device *dev = &vkmsdev->drm;
> > > +	struct drm_connector *connector = &output->connector;
> > > +	struct drm_encoder *encoder = &output->encoder;
> > > +	struct drm_crtc *crtc = &output->crtc;
> > > +	struct drm_plane *primary;
> > > +	int ret;
> > > +
> > > +	primary = vkms_plane_init(vkmsdev);
> > > +	if (IS_ERR(primary))
> > > +		return PTR_ERR(primary);
> > > +
> > > +	ret = vkms_crtc_init(dev, crtc, primary, NULL);
> > > +	if (ret)
> > > +		goto err_crtc;
> > > +
> > 
> > 3) add drm_connector_helper_funcs with dummy {get_modes, mode_valid}.
> > 
> > > +	ret = drm_connector_init(dev, connector, &vkms_connector_funcs,
> > > +				 DRM_MODE_CONNECTOR_VIRTUAL);
> > > +	if (ret) {
> > > +		DRM_ERROR("Failed to init connector\n");
> > > +		goto err_connector;
> > > +	}
> > > +
> > > +	ret = drm_connector_register(connector);
> > > +	if (ret) {
> > > +		DRM_ERROR("Failed to register connector\n");
> > > +		goto err_connector_register;
> > > +	}
> > > +
> > > +	ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs,
> > > +			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> > > +	if (ret) {
> > > +		DRM_ERROR("Failed to init encoder\n");
> > > +		goto err_encoder;
> > > +	}
> > > +	encoder->possible_crtcs = 1;
> > > +
> > > +	ret = drm_mode_connector_attach_encoder(connector, encoder);
> > > +	if (ret) {
> > > +		DRM_ERROR("Failed to attach connector to encoder\n");
> > > +		goto err_attach;
> > > +	}
> > > +
> > > +	drm_mode_config_reset(dev);
> > > +
> > > +	return 0;
> > > +
> > > +err_attach:
> > > +	drm_encoder_cleanup(encoder);
> > > +
> > > +err_encoder:
> > > +	drm_connector_unregister(connector);
> > > +
> > > +err_connector_register:
> > > +	drm_connector_cleanup(connector);
> > > +
> > > +err_connector:
> > > +	drm_crtc_cleanup(crtc);
> > > +
> > > +err_crtc:
> > > +	drm_plane_cleanup(primary);
> > > +	return ret;
> > > +}
> > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> > > new file mode 100644
> > > index 000000000000..2c25b1d6ab5b
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> > > @@ -0,0 +1,46 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * 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.
> > > + */
> > > +
> > > +#include "vkms_drv.h"
> > > +#include <drm/drm_plane_helper.h>
> > > +#include <drm/drm_atomic_helper.h>
> > > +
> > > +static const struct drm_plane_funcs vkms_plane_funcs = {
> > > +	.update_plane		= drm_atomic_helper_update_plane,
> > > +	.disable_plane		= drm_atomic_helper_disable_plane,
> > > +	.destroy		= drm_plane_cleanup,
> > > +	.reset			= drm_atomic_helper_plane_reset,
> > > +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> > > +	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
> > > +};
> > > +
> > > +struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev)
> > > +{
> > > +	struct drm_device *dev = &vkmsdev->drm;
> > > +	struct drm_plane *plane;
> > > +	const u32 *formats;
> > > +	int ret, nformats;
> > > +
> > > +	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
> > > +	if (!plane)
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	formats = vkms_formats;
> > > +	nformats = ARRAY_SIZE(vkms_formats);
> > > +
> > > +	ret = drm_universal_plane_init(dev, plane, 0,
> > > +				       &vkms_plane_funcs,
> > > +				       formats, nformats,
> > > +				       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> > > +	if (ret) {
> > > +		kfree(plane);
> > > +		return ERR_PTR(ret);
> > > +	}
> > > +
> > 
> > 4) add drm_plane_helper_funcs with dummy {atomic_check, prepare_fb, cleanup_fb}.
> > 
> > > +	return plane;
> > > +}
> > > -- 
> > > 2.17.0
> > > 
> > 
> > - Haneen
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2,2/3] drm/vkms: Add basic CRTC initialization

[Thomas Gleixner]

Rodrigo,

On Wed, 16 May 2018, Rodrigo Siqueira wrote:

All files added by this commit have inconsistent license information:

> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * 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.
> + */

> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -0,0 +1,91 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * 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.
> + */

> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * 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.
> + */

The SPDX license identifier says GPL v2 only, but the license notice says
GPL v2 or later. This is just wrong.

Please decide which one to use and please also remove the boiler plate
text. It is redundant.

See Documentation/process/license-rules.rst

Please fix ASAP, add a Fixes tag and cc stable.

Thanks,

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

Re: [v2,2/3] drm/vkms: Add basic CRTC initialization

[Rodrigo Siqueira]

On 01/18, Thomas Gleixner wrote:
> Rodrigo,
> 
> On Wed, 16 May 2018, Rodrigo Siqueira wrote:
> 
> All files added by this commit have inconsistent license information:
> 
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -0,0 +1,35 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * 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.
> > + */
> 
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -0,0 +1,91 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * 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.
> > + */
> 
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> > @@ -0,0 +1,46 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * 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.
> > + */
> 
> The SPDX license identifier says GPL v2 only, but the license notice says
> GPL v2 or later. This is just wrong.
> 
> Please decide which one to use and please also remove the boiler plate
> text. It is redundant.
> 
> See Documentation/process/license-rules.rst
> 
> Please fix ASAP, add a Fixes tag and cc stable.

Hi Thomas,

Thanks for your feedback. I'll try to send a patch for fix this issue
today.

Best Regards
Rodrigo Siqueira
 
> Thanks,
> 
>         tglx

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/3] drm/vkms: Add extra information about vkms

[Rodrigo Siqueira]

Add the following additional information: authors and description in
Kconfig.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 drivers/gpu/drm/Kconfig         | 8 ++++++--
 drivers/gpu/drm/vkms/vkms_drv.c | 2 ++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 0893b1d8ede7..a16efdea00fa 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -213,10 +213,14 @@ config DRM_VGEM
 	  If M is selected the module will be called vgem.
 
 config DRM_VKMS
-	tristate "Virtual KMS"
+	tristate "Virtual KMS (EXPERIMENTAL)"
 	depends on DRM
+	default n
 	help
-	  Choose this option to get a virtual kernal mode-setting driver.
+	  Virtual Kernel Mode-Setting (VKMS) is used for testing or for
+	  running GPU in a headless machines. Choose this option to get
+	  a VKMS.
+
 	  If M is selected the module will be called vkms.
 
 source "drivers/gpu/drm/exynos/Kconfig"
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 070613e32934..740a4cbfed91 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -133,5 +133,7 @@ static void __exit vkms_exit(void)
 module_init(vkms_init);
 module_exit(vkms_exit);
 
+MODULE_AUTHOR("Haneen Mohammed <hamohammed.sa@gmail.com>");
+MODULE_AUTHOR("Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>");
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
-- 
2.17.0

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

Re: [PATCH v2 0/3] Expanding the basic vkms features

[Haneen Mohammed]

On Wed, May 16, 2018 at 08:55:06PM -0300, Rodrigo Siqueira wrote:
> This series of patches add a centralized initialization mechanism, a
> single CRTC with a plane, an encoder, and extra module information. 
> 
> Changes in v2:
>  - Remove unused definitions
>  - Improve file names
>  - Improve code separation
>  - Remove unnecessary formats
> 
> Rodrigo Siqueira (3):
>   drm/vkms: Add mode_config initialization
>   drm/vkms: Add basic CRTC initialization
>   drm/vkms: Add extra information about vkms
> 
>  drivers/gpu/drm/Kconfig            |  8 ++-
>  drivers/gpu/drm/vkms/Makefile      |  2 +-
>  drivers/gpu/drm/vkms/vkms_crtc.c   | 35 ++++++++++++
>  drivers/gpu/drm/vkms/vkms_drv.c    | 61 +++++++++-----------
>  drivers/gpu/drm/vkms/vkms_drv.h    | 24 +++++++-
>  drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_plane.c  | 46 +++++++++++++++
>  7 files changed, 227 insertions(+), 40 deletions(-)
>  create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_output.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c
> 
> -- 
> 2.17.0

with my limited knowledge on DRM, these looks good to me.

Reviewed-by: Haneen Mohammed <hamohammed.sa@gmail>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/3] Expanding the basic vkms features

[Daniel Vetter]

On Thu, May 17, 2018 at 10:42:32PM +0300, Haneen Mohammed wrote:
> On Wed, May 16, 2018 at 08:55:06PM -0300, Rodrigo Siqueira wrote:
> > This series of patches add a centralized initialization mechanism, a
> > single CRTC with a plane, an encoder, and extra module information. 
> > 
> > Changes in v2:
> >  - Remove unused definitions
> >  - Improve file names
> >  - Improve code separation
> >  - Remove unnecessary formats
> > 
> > Rodrigo Siqueira (3):
> >   drm/vkms: Add mode_config initialization
> >   drm/vkms: Add basic CRTC initialization
> >   drm/vkms: Add extra information about vkms
> > 
> >  drivers/gpu/drm/Kconfig            |  8 ++-
> >  drivers/gpu/drm/vkms/Makefile      |  2 +-
> >  drivers/gpu/drm/vkms/vkms_crtc.c   | 35 ++++++++++++
> >  drivers/gpu/drm/vkms/vkms_drv.c    | 61 +++++++++-----------
> >  drivers/gpu/drm/vkms/vkms_drv.h    | 24 +++++++-
> >  drivers/gpu/drm/vkms/vkms_output.c | 91 ++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/vkms/vkms_plane.c  | 46 +++++++++++++++
> >  7 files changed, 227 insertions(+), 40 deletions(-)
> >  create mode 100644 drivers/gpu/drm/vkms/vkms_crtc.c
> >  create mode 100644 drivers/gpu/drm/vkms/vkms_output.c
> >  create mode 100644 drivers/gpu/drm/vkms/vkms_plane.c
> > 
> > -- 
> > 2.17.0
> 
> with my limited knowledge on DRM, these looks good to me.
> 
> Reviewed-by: Haneen Mohammed <hamohammed.sa@gmail>

All three applied, thanks for patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel