All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V3 0/3] Expanding the basic vkms features
@ 2018-05-22  1:04 Rodrigo Siqueira
  2018-05-22  1:04 ` [PATCH V3 1/3] drm/vkms: Add mode_config initialization Rodrigo Siqueira
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Rodrigo Siqueira @ 2018-05-22  1:04 UTC (permalink / raw)
  To: Gustavo Padovan, Daniel Vetter, Sean Paul, Haneen Mohammed; +Cc: dri-devel

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

Changes in v3:
 - Adds drm_crtc_helper_funcs with a simple atomic_check
 - Adds extra hooks for drm_connector_funcs hooks (reset,
   atomic_duplicate_state, atomic_destroy_state)
 - Adds drm_connector_helper_funcs
 - Adds drm_plane_helper_funcs
 - Changes in the commit messages

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   |  47 ++++++++++++
 drivers/gpu/drm/vkms/vkms_drv.c    |  55 +++++---------
 drivers/gpu/drm/vkms/vkms_drv.h    |  33 +++++++-
 drivers/gpu/drm/vkms/vkms_output.c | 118 +++++++++++++++++++++++++++++
 drivers/gpu/drm/vkms/vkms_plane.c  |  62 +++++++++++++++
 7 files changed, 285 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V3 1/3] drm/vkms: Add mode_config initialization
  2018-05-22  1:04 [PATCH V3 0/3] Expanding the basic vkms features Rodrigo Siqueira
@ 2018-05-22  1:04 ` Rodrigo Siqueira
  2018-05-22  1:06 ` [PATCH V3 2/3] drm/vkms: Add basic CRTC initialization Rodrigo Siqueira
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Rodrigo Siqueira @ 2018-05-22  1:04 UTC (permalink / raw)
  To: Gustavo Padovan, Daniel Vetter, Sean Paul, Haneen Mohammed; +Cc: dri-devel

Set a frame buffer default size for width and height (minimum and
maximum).

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V3 2/3] drm/vkms: Add basic CRTC initialization
  2018-05-22  1:04 [PATCH V3 0/3] Expanding the basic vkms features Rodrigo Siqueira
  2018-05-22  1:04 ` [PATCH V3 1/3] drm/vkms: Add mode_config initialization Rodrigo Siqueira
@ 2018-05-22  1:06 ` Rodrigo Siqueira
  2018-05-22  2:57   ` Haneen Mohammed
  2018-10-22 15:30   ` Emil Velikov
  2018-05-22  1:07 ` [PATCH V3 3/3] drm/vkms: Add extra information about vkms Rodrigo Siqueira
  2018-05-23  9:15 ` [PATCH V3 0/3] Expanding the basic vkms features Daniel Vetter
  3 siblings, 2 replies; 12+ messages in thread
From: Rodrigo Siqueira @ 2018-05-22  1:06 UTC (permalink / raw)
  To: Gustavo Padovan, Daniel Vetter, Sean Paul, Haneen Mohammed; +Cc: dri-devel

This commit appends the essential CRTCs infrastructure for VKMS. CRTCs
demands other elements to work correctly, in this sense, this patch adds
a new data struct that centralizes the data structures related to the
output, simple planes management, and single encoder attached to the
connector.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 drivers/gpu/drm/vkms/Makefile      |   2 +-
 drivers/gpu/drm/vkms/vkms_crtc.c   |  47 ++++++++++++
 drivers/gpu/drm/vkms/vkms_drv.c    |  66 +++++-----------
 drivers/gpu/drm/vkms/vkms_drv.h    |  33 +++++++-
 drivers/gpu/drm/vkms/vkms_output.c | 118 +++++++++++++++++++++++++++++
 drivers/gpu/drm/vkms/vkms_plane.c  |  62 +++++++++++++++
 6 files changed, 275 insertions(+), 53 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..4ec1424ed8b8
--- /dev/null
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -0,0 +1,47 @@
+// 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,
+};
+
+static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
+				  struct drm_crtc_state *state)
+{
+	return 0;
+}
+
+static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
+	.atomic_check  = vkms_crtc_atomic_check,
+};
+
+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;
+	}
+
+	drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index aec3f180f96d..eae5f1f293d0 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>
@@ -18,12 +17,6 @@
 #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 = {
@@ -59,25 +52,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 +90,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..a9f8b6695683 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -1,13 +1,40 @@
 #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>
+
+#define XRES_MIN    32
+#define YRES_MIN    32
+
+#define XRES_DEF  1024
+#define YRES_DEF   768
+
+#define XRES_MAX  8192
+#define YRES_MAX  8192
+
+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..01e701691ea8
--- /dev/null
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -0,0 +1,118 @@
+// 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>
+#include <drm/drm_atomic_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,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int vkms_conn_get_modes(struct drm_connector *connector)
+{
+	int count;
+
+	count = drm_add_modes_noedid(connector, XRES_MAX, YRES_MAX);
+	drm_set_preferred_mode(connector, XRES_DEF, YRES_DEF);
+
+	return count;
+}
+
+static int vkms_conn_mode_valid(struct drm_connector *connector,
+				struct drm_display_mode *mode)
+{
+	return MODE_OK;
+}
+
+static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = {
+	.get_modes    = vkms_conn_get_modes,
+	.mode_valid   = vkms_conn_mode_valid,
+};
+
+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;
+	}
+
+	drm_connector_helper_add(connector, &vkms_conn_helper_funcs);
+
+	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..b6231c1e3462
--- /dev/null
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -0,0 +1,62 @@
+// 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,
+};
+
+static int vkms_plane_atomic_check(struct drm_plane *plane,
+				   struct drm_plane_state *state)
+{
+	return 0;
+}
+
+static void vkms_primary_plane_update(struct drm_plane *plane,
+				      struct drm_plane_state *old_state)
+{
+}
+
+static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = {
+	.atomic_check		= vkms_plane_atomic_check,
+	.atomic_update		= vkms_primary_plane_update,
+};
+
+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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V3 3/3] drm/vkms: Add extra information about vkms
  2018-05-22  1:04 [PATCH V3 0/3] Expanding the basic vkms features Rodrigo Siqueira
  2018-05-22  1:04 ` [PATCH V3 1/3] drm/vkms: Add mode_config initialization Rodrigo Siqueira
  2018-05-22  1:06 ` [PATCH V3 2/3] drm/vkms: Add basic CRTC initialization Rodrigo Siqueira
@ 2018-05-22  1:07 ` Rodrigo Siqueira
  2018-05-23  9:15 ` [PATCH V3 0/3] Expanding the basic vkms features Daniel Vetter
  3 siblings, 0 replies; 12+ messages in thread
From: Rodrigo Siqueira @ 2018-05-22  1:07 UTC (permalink / raw)
  To: Gustavo Padovan, Daniel Vetter, Sean Paul, Haneen Mohammed; +Cc: dri-devel

Add 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 eae5f1f293d0..e8730fd72cdf 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -127,5 +127,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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V3 2/3] drm/vkms: Add basic CRTC initialization
  2018-05-22  1:06 ` [PATCH V3 2/3] drm/vkms: Add basic CRTC initialization Rodrigo Siqueira
@ 2018-05-22  2:57   ` Haneen Mohammed
  2018-05-22 12:02     ` Rodrigo Siqueira
  2018-10-22 15:30   ` Emil Velikov
  1 sibling, 1 reply; 12+ messages in thread
From: Haneen Mohammed @ 2018-05-22  2:57 UTC (permalink / raw)
  To: Rodrigo Siqueira; +Cc: dri-devel, Sean Paul, Daniel Vetter

On Mon, May 21, 2018 at 10:06:40PM -0300, Rodrigo Siqueira wrote:
> This commit appends the essential CRTCs infrastructure for VKMS. CRTCs
> demands other elements to work correctly, in this sense, this patch adds
> a new data struct that centralizes the data structures related to the
> output, simple planes management, and single encoder attached to the
> connector.
> 
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---
>  drivers/gpu/drm/vkms/Makefile      |   2 +-
>  drivers/gpu/drm/vkms/vkms_crtc.c   |  47 ++++++++++++
>  drivers/gpu/drm/vkms/vkms_drv.c    |  66 +++++-----------
>  drivers/gpu/drm/vkms/vkms_drv.h    |  33 +++++++-
>  drivers/gpu/drm/vkms/vkms_output.c | 118 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_plane.c  |  62 +++++++++++++++
>  6 files changed, 275 insertions(+), 53 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..4ec1424ed8b8
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -0,0 +1,47 @@
> +// 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,
> +};
> +
> +static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
> +				  struct drm_crtc_state *state)
> +{
> +	return 0;
> +}
> +
> +static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
> +	.atomic_check  = vkms_crtc_atomic_check,
> +};
> +
> +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;
> +	}
> +
> +	drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
> +
> +	return ret;
> +}
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index aec3f180f96d..eae5f1f293d0 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>
> @@ -18,12 +17,6 @@
>  #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 = {
> @@ -59,25 +52,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 +90,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..a9f8b6695683 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -1,13 +1,40 @@
>  #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>
> +
> +#define XRES_MIN    32
> +#define YRES_MIN    32
> +
> +#define XRES_DEF  1024
> +#define YRES_DEF   768
> +
> +#define XRES_MAX  8192
> +#define YRES_MAX  8192
> +
> +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..01e701691ea8
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -0,0 +1,118 @@
> +// 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>
> +#include <drm/drm_atomic_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,
> +	.reset = drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int vkms_conn_get_modes(struct drm_connector *connector)
> +{
> +	int count;
> +
> +	count = drm_add_modes_noedid(connector, XRES_MAX, YRES_MAX);
> +	drm_set_preferred_mode(connector, XRES_DEF, YRES_DEF);
> +
> +	return count;
> +}
> +
> +static int vkms_conn_mode_valid(struct drm_connector *connector,
> +				struct drm_display_mode *mode)
> +{
> +	return MODE_OK;
> +}
> +
> +static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = {
> +	.get_modes    = vkms_conn_get_modes,
> +	.mode_valid   = vkms_conn_mode_valid,
> +};
> +
> +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;
> +	}
> +
> +	drm_connector_helper_add(connector, &vkms_conn_helper_funcs);
> +
> +	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..b6231c1e3462
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -0,0 +1,62 @@
> +// 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,
> +};
> +
> +static int vkms_plane_atomic_check(struct drm_plane *plane,
> +				   struct drm_plane_state *state)
> +{
> +	return 0;
> +}
> +
> +static void vkms_primary_plane_update(struct drm_plane *plane,
> +				      struct drm_plane_state *old_state)
> +{
> +}
> +
> +static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = {
> +	.atomic_check		= vkms_plane_atomic_check,
> +	.atomic_update		= vkms_primary_plane_update,
> +};
> +

I think you forgot to use the plane helper below :)

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V3 2/3] drm/vkms: Add basic CRTC initialization
  2018-05-22  2:57   ` Haneen Mohammed
@ 2018-05-22 12:02     ` Rodrigo Siqueira
  0 siblings, 0 replies; 12+ messages in thread
From: Rodrigo Siqueira @ 2018-05-22 12:02 UTC (permalink / raw)
  To: Haneen Mohammed; +Cc: dri-devel, Sean Paul, Daniel Vetter


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

I fixed it, and I'm preparing the v4 patchset.

Thanks again for the review :)

On Mon, May 21, 2018 at 11:57 PM, Haneen Mohammed <hamohammed.sa@gmail.com>
wrote:

> On Mon, May 21, 2018 at 10:06:40PM -0300, Rodrigo Siqueira wrote:
> > This commit appends the essential CRTCs infrastructure for VKMS. CRTCs
> > demands other elements to work correctly, in this sense, this patch adds
> > a new data struct that centralizes the data structures related to the
> > output, simple planes management, and single encoder attached to the
> > connector.
> >
> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > ---
> >  drivers/gpu/drm/vkms/Makefile      |   2 +-
> >  drivers/gpu/drm/vkms/vkms_crtc.c   |  47 ++++++++++++
> >  drivers/gpu/drm/vkms/vkms_drv.c    |  66 +++++-----------
> >  drivers/gpu/drm/vkms/vkms_drv.h    |  33 +++++++-
> >  drivers/gpu/drm/vkms/vkms_output.c | 118 +++++++++++++++++++++++++++++
> >  drivers/gpu/drm/vkms/vkms_plane.c  |  62 +++++++++++++++
> >  6 files changed, 275 insertions(+), 53 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..4ec1424ed8b8
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -0,0 +1,47 @@
> > +// 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,
> > +};
> > +
> > +static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
> > +                               struct drm_crtc_state *state)
> > +{
> > +     return 0;
> > +}
> > +
> > +static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
> > +     .atomic_check  = vkms_crtc_atomic_check,
> > +};
> > +
> > +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;
> > +     }
> > +
> > +     drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
> > +
> > +     return ret;
> > +}
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c
> b/drivers/gpu/drm/vkms/vkms_drv.c
> > index aec3f180f96d..eae5f1f293d0 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>
> > @@ -18,12 +17,6 @@
> >  #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 = {
> > @@ -59,25 +52,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 +90,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..a9f8b6695683 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -1,13 +1,40 @@
> >  #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>
> > +
> > +#define XRES_MIN    32
> > +#define YRES_MIN    32
> > +
> > +#define XRES_DEF  1024
> > +#define YRES_DEF   768
> > +
> > +#define XRES_MAX  8192
> > +#define YRES_MAX  8192
> > +
> > +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..01e701691ea8
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -0,0 +1,118 @@
> > +// 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>
> > +#include <drm/drm_atomic_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,
> > +     .reset = drm_atomic_helper_connector_reset,
> > +     .atomic_duplicate_state = drm_atomic_helper_connector_
> duplicate_state,
> > +     .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > +};
> > +
> > +static int vkms_conn_get_modes(struct drm_connector *connector)
> > +{
> > +     int count;
> > +
> > +     count = drm_add_modes_noedid(connector, XRES_MAX, YRES_MAX);
> > +     drm_set_preferred_mode(connector, XRES_DEF, YRES_DEF);
> > +
> > +     return count;
> > +}
> > +
> > +static int vkms_conn_mode_valid(struct drm_connector *connector,
> > +                             struct drm_display_mode *mode)
> > +{
> > +     return MODE_OK;
> > +}
> > +
> > +static const struct drm_connector_helper_funcs vkms_conn_helper_funcs =
> {
> > +     .get_modes    = vkms_conn_get_modes,
> > +     .mode_valid   = vkms_conn_mode_valid,
> > +};
> > +
> > +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;
> > +     }
> > +
> > +     drm_connector_helper_add(connector, &vkms_conn_helper_funcs);
> > +
> > +     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..b6231c1e3462
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> > @@ -0,0 +1,62 @@
> > +// 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,
> > +};
> > +
> > +static int vkms_plane_atomic_check(struct drm_plane *plane,
> > +                                struct drm_plane_state *state)
> > +{
> > +     return 0;
> > +}
> > +
> > +static void vkms_primary_plane_update(struct drm_plane *plane,
> > +                                   struct drm_plane_state *old_state)
> > +{
> > +}
> > +
> > +static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = {
> > +     .atomic_check           = vkms_plane_atomic_check,
> > +     .atomic_update          = vkms_primary_plane_update,
> > +};
> > +
>
> I think you forgot to use the plane helper below :)
>
> > +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
> >
>



-- 

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

[-- Attachment #1.2: Type: text/html, Size: 21330 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V3 0/3] Expanding the basic vkms features
  2018-05-22  1:04 [PATCH V3 0/3] Expanding the basic vkms features Rodrigo Siqueira
                   ` (2 preceding siblings ...)
  2018-05-22  1:07 ` [PATCH V3 3/3] drm/vkms: Add extra information about vkms Rodrigo Siqueira
@ 2018-05-23  9:15 ` Daniel Vetter
  2018-05-24  2:42   ` Haneen Mohammed
  3 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2018-05-23  9:15 UTC (permalink / raw)
  To: Rodrigo Siqueira; +Cc: Haneen Mohammed, dri-devel, Sean Paul, Daniel Vetter

On Mon, May 21, 2018 at 10:04:23PM -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

Oops, I merged v2 already. I think we need follow-up patches.
 
> Changes in v3:
>  - Adds drm_crtc_helper_funcs with a simple atomic_check

Sorry for the late comment on v2, but I think we need to figure out why
this goes boom first. It imo shouldn't.

>  - Adds extra hooks for drm_connector_funcs hooks (reset,
>    atomic_duplicate_state, atomic_destroy_state)

Hm, reset shouldn't be required. Why do you need it?

Wrt duplicate/destroy state, those are mandatory. I think it'd be good to
have checks for those in the drm_*_init functions, but only for atomic
drivers. You can use drm_drv_uses_atomic_modeset() and WARN_ON(). There's
a bunch of examples already for checking for this stuff, see e.g.
dma_fence_init().

>  - Adds drm_connector_helper_funcs
>  - Adds drm_plane_helper_funcs

Same here, would be good to add WARN_ON to the relevant _init() functions
to make sure all the mandatory stuff is there to begin with.

Since Rodrigo has typed the fixes to vkms already, could you Haneen look
into adding these checks to the core drm core?

Thanks, Daniel

>  - Changes in the commit messages
> 
> 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   |  47 ++++++++++++
>  drivers/gpu/drm/vkms/vkms_drv.c    |  55 +++++---------
>  drivers/gpu/drm/vkms/vkms_drv.h    |  33 +++++++-
>  drivers/gpu/drm/vkms/vkms_output.c | 118 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_plane.c  |  62 +++++++++++++++
>  7 files changed, 285 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
> 

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V3 0/3] Expanding the basic vkms features
  2018-05-23  9:15 ` [PATCH V3 0/3] Expanding the basic vkms features Daniel Vetter
@ 2018-05-24  2:42   ` Haneen Mohammed
  2018-05-24  8:15     ` Daniel Vetter
  0 siblings, 1 reply; 12+ messages in thread
From: Haneen Mohammed @ 2018-05-24  2:42 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel, Sean Paul, Rodrigo Siqueira, Daniel Vetter

On Wed, May 23, 2018 at 11:15:18AM +0200, Daniel Vetter wrote:
> On Mon, May 21, 2018 at 10:04:23PM -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
> 
> Oops, I merged v2 already. I think we need follow-up patches.
>  
> > Changes in v3:
> >  - Adds drm_crtc_helper_funcs with a simple atomic_check
> 
> Sorry for the late comment on v2, but I think we need to figure out why
> this goes boom first. It imo shouldn't.
> 
> >  - Adds extra hooks for drm_connector_funcs hooks (reset,
> >    atomic_duplicate_state, atomic_destroy_state)
> 
> Hm, reset shouldn't be required. Why do you need it?
> 

I've checked the code, and I think the memory for [plane/connector/crtc]
states that are used later in duplicate/destroy are allocated in drm_atomic_helper_*_reset. 
Otherwise checking /sys/kernel/debug/dri/0/state or running the igt
tests causes null pointer dereference error.

> Wrt duplicate/destroy state, those are mandatory. I think it'd be good to
> have checks for those in the drm_*_init functions, but only for atomic
> drivers. You can use drm_drv_uses_atomic_modeset() and WARN_ON(). There's
> a bunch of examples already for checking for this stuff, see e.g.
> dma_fence_init().
>
> >  - Adds drm_connector_helper_funcs
> >  - Adds drm_plane_helper_funcs
> 
> Same here, would be good to add WARN_ON to the relevant _init() functions
> to make sure all the mandatory stuff is there to begin with.
> 
> Since Rodrigo has typed the fixes to vkms already, could you Haneen look
> into adding these checks to the core drm core?
>

Sure, I'll work on that.

> Thanks, Daniel
> 
> >  - Changes in the commit messages
> > 
> > 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   |  47 ++++++++++++
> >  drivers/gpu/drm/vkms/vkms_drv.c    |  55 +++++---------
> >  drivers/gpu/drm/vkms/vkms_drv.h    |  33 +++++++-
> >  drivers/gpu/drm/vkms/vkms_output.c | 118 +++++++++++++++++++++++++++++
> >  drivers/gpu/drm/vkms/vkms_plane.c  |  62 +++++++++++++++
> >  7 files changed, 285 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
> > 
> 
> -- 
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V3 0/3] Expanding the basic vkms features
  2018-05-24  2:42   ` Haneen Mohammed
@ 2018-05-24  8:15     ` Daniel Vetter
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2018-05-24  8:15 UTC (permalink / raw)
  To: Haneen Mohammed; +Cc: Rodrigo Siqueira, Sean Paul, dri-devel, Daniel Vetter

On Thu, May 24, 2018 at 05:42:02AM +0300, Haneen Mohammed wrote:
> On Wed, May 23, 2018 at 11:15:18AM +0200, Daniel Vetter wrote:
> > On Mon, May 21, 2018 at 10:04:23PM -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
> > 
> > Oops, I merged v2 already. I think we need follow-up patches.
> >  
> > > Changes in v3:
> > >  - Adds drm_crtc_helper_funcs with a simple atomic_check
> > 
> > Sorry for the late comment on v2, but I think we need to figure out why
> > this goes boom first. It imo shouldn't.
> > 
> > >  - Adds extra hooks for drm_connector_funcs hooks (reset,
> > >    atomic_duplicate_state, atomic_destroy_state)
> > 
> > Hm, reset shouldn't be required. Why do you need it?
> > 
> 
> I've checked the code, and I think the memory for [plane/connector/crtc]
> states that are used later in duplicate/destroy are allocated in drm_atomic_helper_*_reset. 
> Otherwise checking /sys/kernel/debug/dri/0/state or running the igt
> tests causes null pointer dereference error.

Yes, but drm_mode_config_reset is a helper, and not mandatory. Most
drivers use that to initialize the software state structures, but e.g.
i915 reads out the actual hardware state, and uses that to reconstruct the
software state. This allows for smooth take over of the boot loader screen
state, avoiding flickering.

That's why the ->reset hooks aren't mandatory, but duplicate/destroy hooks
are.

> 
> > Wrt duplicate/destroy state, those are mandatory. I think it'd be good to
> > have checks for those in the drm_*_init functions, but only for atomic
> > drivers. You can use drm_drv_uses_atomic_modeset() and WARN_ON(). There's
> > a bunch of examples already for checking for this stuff, see e.g.
> > dma_fence_init().
> >
> > >  - Adds drm_connector_helper_funcs
> > >  - Adds drm_plane_helper_funcs
> > 
> > Same here, would be good to add WARN_ON to the relevant _init() functions
> > to make sure all the mandatory stuff is there to begin with.
> > 
> > Since Rodrigo has typed the fixes to vkms already, could you Haneen look
> > into adding these checks to the core drm core?
> >
> 
> Sure, I'll work on that.

Cool. When you do that, pls also double-check that the kerneldoc comments
are accurate and up-to-date.

Thanks, Daniel

> 
> > Thanks, Daniel
> > 
> > >  - Changes in the commit messages
> > > 
> > > 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   |  47 ++++++++++++
> > >  drivers/gpu/drm/vkms/vkms_drv.c    |  55 +++++---------
> > >  drivers/gpu/drm/vkms/vkms_drv.h    |  33 +++++++-
> > >  drivers/gpu/drm/vkms/vkms_output.c | 118 +++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/vkms/vkms_plane.c  |  62 +++++++++++++++
> > >  7 files changed, 285 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
> > > 
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V3 2/3] drm/vkms: Add basic CRTC initialization
  2018-05-22  1:06 ` [PATCH V3 2/3] drm/vkms: Add basic CRTC initialization Rodrigo Siqueira
  2018-05-22  2:57   ` Haneen Mohammed
@ 2018-10-22 15:30   ` Emil Velikov
  2018-10-22 23:57     ` Rodrigo Siqueira
  1 sibling, 1 reply; 12+ messages in thread
From: Emil Velikov @ 2018-10-22 15:30 UTC (permalink / raw)
  To: rodrigosiqueiramelo; +Cc: hamohammed.sa, ML dri-devel, Sean Paul, Daniel Vetter

Hi all,

On Tue, 22 May 2018 at 02:07, Rodrigo Siqueira
<rodrigosiqueiramelo@gmail.com> wrote:

> +// SPDX-License-Identifier: GPL-2.0

This

> +/*
> + * 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.
> + */

and this are not the same thing.

The former is 2.0-only, while the explicit text is 2.0-or-later.

Personally, I'd opt for 2.0-only, or even make it a dual 2.0-only OR MIT.
Yet again, this is not my code, so not my decision to make.

From a quick glance other vkms files have the same mismatch.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V3 2/3] drm/vkms: Add basic CRTC initialization
  2018-10-22 15:30   ` Emil Velikov
@ 2018-10-22 23:57     ` Rodrigo Siqueira
  2018-10-23 12:40       ` Emil Velikov
  0 siblings, 1 reply; 12+ messages in thread
From: Rodrigo Siqueira @ 2018-10-22 23:57 UTC (permalink / raw)
  To: Emil Velikov; +Cc: hamohammed.sa, ML dri-devel, Sean Paul, Daniel Vetter

On 10/22, Emil Velikov wrote:
> Hi all,

Hi Emil,

Thanks for your feedback.
 
> On Tue, 22 May 2018 at 02:07, Rodrigo Siqueira
> <rodrigosiqueiramelo@gmail.com> wrote:
> 
> > +// SPDX-License-Identifier: GPL-2.0
> 
> This
> 
> > +/*
> > + * 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.
> > + */
> 
> and this are not the same thing.
> 
> The former is 2.0-only, while the explicit text is 2.0-or-later.
> 
> Personally, I'd opt for 2.0-only, or even make it a dual 2.0-only OR MIT.
> Yet again, this is not my code, so not my decision to make.

I have to admit that I do not understand too much about these issues,
however, now it is time to for learning more about it :)

Do you have any documentation to recommend me about the issue? I want to
read it, and fix the problems that you highlighted.

Best Regards

> From a quick glance other vkms files have the same mismatch.
> 
> HTH
> Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V3 2/3] drm/vkms: Add basic CRTC initialization
  2018-10-22 23:57     ` Rodrigo Siqueira
@ 2018-10-23 12:40       ` Emil Velikov
  0 siblings, 0 replies; 12+ messages in thread
From: Emil Velikov @ 2018-10-23 12:40 UTC (permalink / raw)
  To: Rodrigo Siqueira; +Cc: Haneen Mohammed, ML dri-devel, Sean Paul, Daniel Vetter

On Tue, 23 Oct 2018 at 00:57, Rodrigo Siqueira
<rodrigosiqueiramelo@gmail.com> wrote:
>
> On 10/22, Emil Velikov wrote:
> > Hi all,
>
> Hi Emil,
>
> Thanks for your feedback.
>
> > On Tue, 22 May 2018 at 02:07, Rodrigo Siqueira
> > <rodrigosiqueiramelo@gmail.com> wrote:
> >
> > > +// SPDX-License-Identifier: GPL-2.0
> >
> > This
> >
> > > +/*
> > > + * 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.
> > > + */
> >
> > and this are not the same thing.
> >
> > The former is 2.0-only, while the explicit text is 2.0-or-later.
> >
> > Personally, I'd opt for 2.0-only, or even make it a dual 2.0-only OR MIT.
> > Yet again, this is not my code, so not my decision to make.
>
> I have to admit that I do not understand too much about these issues,
> however, now it is time to for learning more about it :)
>
> Do you have any documentation to recommend me about the issue? I want to
> read it, and fix the problems that you highlighted.
>
WRT the SPDX identifiers you can check the official website
https://spdx.org/licenses/

For some reason the kernel seems to be using the "Deprecated Licenses"
 GPL-2.0 instead of GPL-2.0-only, etc.
IMHO it's better to stay consistent, and use the Deprecated for now.
For each entry there's also a link to the full license text, etc.

To provide some meat for my personal views/suggestion:
 - 2.0-only is narrowly scoped, while 2.0-or-later allows using any
future, _yet to be written_, version of the license
 - MIT historically all of DRM (and graphics stack as a whole) has
been MIT so that others - BSD, Solaris etc could reuse it.

But as I said, it's not my call.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-10-23 12:41 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-22  1:04 [PATCH V3 0/3] Expanding the basic vkms features Rodrigo Siqueira
2018-05-22  1:04 ` [PATCH V3 1/3] drm/vkms: Add mode_config initialization Rodrigo Siqueira
2018-05-22  1:06 ` [PATCH V3 2/3] drm/vkms: Add basic CRTC initialization Rodrigo Siqueira
2018-05-22  2:57   ` Haneen Mohammed
2018-05-22 12:02     ` Rodrigo Siqueira
2018-10-22 15:30   ` Emil Velikov
2018-10-22 23:57     ` Rodrigo Siqueira
2018-10-23 12:40       ` Emil Velikov
2018-05-22  1:07 ` [PATCH V3 3/3] drm/vkms: Add extra information about vkms Rodrigo Siqueira
2018-05-23  9:15 ` [PATCH V3 0/3] Expanding the basic vkms features Daniel Vetter
2018-05-24  2:42   ` Haneen Mohammed
2018-05-24  8:15     ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.