virtualization.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 26/59] drm/qxl: Use devm_drm_dev_alloc
       [not found] <20200415074034.175360-1-daniel.vetter@ffwll.ch>
@ 2020-04-15  7:40 ` Daniel Vetter
  2020-04-24 15:09   ` Sam Ravnborg
  2020-04-15  7:40 ` [PATCH 27/59] drm/qxl: Don't use drm_device->dev_private Daniel Vetter
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2020-04-15  7:40 UTC (permalink / raw)
  To: Intel Graphics Development
  Cc: spice-devel, Daniel Vetter, DRI Development, virtualization,
	Daniel Vetter, Dave Airlie

Also need to remove the drm_dev_put from the remove hook.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
 drivers/gpu/drm/qxl/qxl_drv.c | 15 ++++++++-------
 drivers/gpu/drm/qxl/qxl_drv.h |  3 +--
 drivers/gpu/drm/qxl/qxl_kms.c | 12 +-----------
 3 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 09102e2efabc..6b4ae4c5fb76 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return -EINVAL; /* TODO: ENODEV ? */
 	}
 
-	qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
-	if (!qdev)
+	qdev = devm_drm_dev_alloc(&pdev->dev, &qxl_driver,
+				  struct qxl_device, ddev);
+	if (IS_ERR(qdev)) {
+		pr_err("Unable to init drm dev");
 		return -ENOMEM;
+	}
 
 	ret = pci_enable_device(pdev);
 	if (ret)
-		goto free_dev;
+		return ret;
 
 	ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "qxl");
 	if (ret)
@@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		}
 	}
 
-	ret = qxl_device_init(qdev, &qxl_driver, pdev);
+	ret = qxl_device_init(qdev, pdev);
 	if (ret)
 		goto put_vga;
 
@@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		vga_put(pdev, VGA_RSRC_LEGACY_IO);
 disable_pci:
 	pci_disable_device(pdev);
-free_dev:
-	kfree(qdev);
+
 	return ret;
 }
 
@@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
 	drm_atomic_helper_shutdown(dev);
 	if (is_vga(pdev))
 		vga_put(pdev, VGA_RSRC_LEGACY_IO);
-	drm_dev_put(dev);
 }
 
 DEFINE_DRM_GEM_FOPS(qxl_fops);
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 435126facc9b..86ac191d9205 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -276,8 +276,7 @@ struct qxl_device {
 extern const struct drm_ioctl_desc qxl_ioctls[];
 extern int qxl_max_ioctl;
 
-int qxl_device_init(struct qxl_device *qdev, struct drm_driver *drv,
-		    struct pci_dev *pdev);
+int qxl_device_init(struct qxl_device *qdev, struct pci_dev *pdev);
 void qxl_device_fini(struct qxl_device *qdev);
 
 int qxl_modeset_init(struct qxl_device *qdev);
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 9eed1a375f24..91a34dd835d7 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -108,21 +108,13 @@ static void qxl_gc_work(struct work_struct *work)
 }
 
 int qxl_device_init(struct qxl_device *qdev,
-		    struct drm_driver *drv,
 		    struct pci_dev *pdev)
 {
 	int r, sb;
 
-	r = drm_dev_init(&qdev->ddev, drv, &pdev->dev);
-	if (r) {
-		pr_err("Unable to init drm dev");
-		goto error;
-	}
-
 	qdev->ddev.pdev = pdev;
 	pci_set_drvdata(pdev, &qdev->ddev);
 	qdev->ddev.dev_private = qdev;
-	drmm_add_final_kfree(&qdev->ddev, qdev);
 
 	mutex_init(&qdev->gem.mutex);
 	mutex_init(&qdev->update_area_mutex);
@@ -138,8 +130,7 @@ int qxl_device_init(struct qxl_device *qdev,
 	qdev->vram_mapping = io_mapping_create_wc(qdev->vram_base, pci_resource_len(pdev, 0));
 	if (!qdev->vram_mapping) {
 		pr_err("Unable to create vram_mapping");
-		r = -ENOMEM;
-		goto error;
+		return -ENOMEM;
 	}
 
 	if (pci_resource_len(pdev, 4) > 0) {
@@ -293,7 +284,6 @@ int qxl_device_init(struct qxl_device *qdev,
 	io_mapping_free(qdev->surface_mapping);
 vram_mapping_free:
 	io_mapping_free(qdev->vram_mapping);
-error:
 	return r;
 }
 
-- 
2.25.1

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

* [PATCH 27/59] drm/qxl: Don't use drm_device->dev_private
       [not found] <20200415074034.175360-1-daniel.vetter@ffwll.ch>
  2020-04-15  7:40 ` [PATCH 26/59] drm/qxl: Use devm_drm_dev_alloc Daniel Vetter
@ 2020-04-15  7:40 ` Daniel Vetter
  2020-04-24 15:12   ` Sam Ravnborg
  2020-04-15  7:40 ` [PATCH 35/59] drm/cirrus: Use devm_drm_dev_alloc Daniel Vetter
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2020-04-15  7:40 UTC (permalink / raw)
  To: Intel Graphics Development
  Cc: spice-devel, Daniel Vetter, DRI Development, virtualization,
	Daniel Vetter, Dave Airlie

Upcasting using a container_of macro is more typesafe, faster and
easier for the compiler to optimize.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
 drivers/gpu/drm/qxl/qxl_debugfs.c |  7 +++----
 drivers/gpu/drm/qxl/qxl_display.c | 32 +++++++++++++++----------------
 drivers/gpu/drm/qxl/qxl_drv.c     |  8 ++++----
 drivers/gpu/drm/qxl/qxl_drv.h     |  4 ++--
 drivers/gpu/drm/qxl/qxl_dumb.c    |  2 +-
 drivers/gpu/drm/qxl/qxl_gem.c     |  2 +-
 drivers/gpu/drm/qxl/qxl_ioctl.c   | 14 +++++++-------
 drivers/gpu/drm/qxl/qxl_irq.c     |  2 +-
 drivers/gpu/drm/qxl/qxl_kms.c     |  1 -
 drivers/gpu/drm/qxl/qxl_object.c  |  2 +-
 drivers/gpu/drm/qxl/qxl_release.c |  2 +-
 drivers/gpu/drm/qxl/qxl_ttm.c     |  2 +-
 12 files changed, 38 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c b/drivers/gpu/drm/qxl/qxl_debugfs.c
index 88123047fdd4..524d35b648d8 100644
--- a/drivers/gpu/drm/qxl/qxl_debugfs.c
+++ b/drivers/gpu/drm/qxl/qxl_debugfs.c
@@ -39,7 +39,7 @@ static int
 qxl_debugfs_irq_received(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct qxl_device *qdev = node->minor->dev->dev_private;
+	struct qxl_device *qdev = to_qxl(node->minor->dev);
 
 	seq_printf(m, "%d\n", atomic_read(&qdev->irq_received));
 	seq_printf(m, "%d\n", atomic_read(&qdev->irq_received_display));
@@ -53,7 +53,7 @@ static int
 qxl_debugfs_buffers_info(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct qxl_device *qdev = node->minor->dev->dev_private;
+	struct qxl_device *qdev = to_qxl(node->minor->dev);
 	struct qxl_bo *bo;
 
 	list_for_each_entry(bo, &qdev->gem.objects, list) {
@@ -83,8 +83,7 @@ void
 qxl_debugfs_init(struct drm_minor *minor)
 {
 #if defined(CONFIG_DEBUG_FS)
-	struct qxl_device *dev =
-		(struct qxl_device *) minor->dev->dev_private;
+	struct qxl_device *dev = to_qxl(minor->dev);
 
 	drm_debugfs_create_files(qxl_debugfs_list, QXL_DEBUGFS_ENTRIES,
 				 minor->debugfs_root, minor);
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 09583a08e141..1082cd5d2fd4 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -221,7 +221,7 @@ static int qxl_add_mode(struct drm_connector *connector,
 			bool preferred)
 {
 	struct drm_device *dev = connector->dev;
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct drm_display_mode *mode = NULL;
 	int rc;
 
@@ -242,7 +242,7 @@ static int qxl_add_mode(struct drm_connector *connector,
 static int qxl_add_monitors_config_modes(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct qxl_output *output = drm_connector_to_qxl_output(connector);
 	int h = output->index;
 	struct qxl_head *head;
@@ -310,7 +310,7 @@ static void qxl_crtc_update_monitors_config(struct drm_crtc *crtc,
 					    const char *reason)
 {
 	struct drm_device *dev = crtc->dev;
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct qxl_crtc *qcrtc = to_qxl_crtc(crtc);
 	struct qxl_head head;
 	int oldcount, i = qcrtc->index;
@@ -400,7 +400,7 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
 					 unsigned int num_clips)
 {
 	/* TODO: vmwgfx where this was cribbed from had locking. Why? */
-	struct qxl_device *qdev = fb->dev->dev_private;
+	struct qxl_device *qdev = to_qxl(fb->dev);
 	struct drm_clip_rect norect;
 	struct qxl_bo *qobj;
 	bool is_primary;
@@ -462,7 +462,7 @@ static const struct drm_crtc_helper_funcs qxl_crtc_helper_funcs = {
 static int qxl_primary_atomic_check(struct drm_plane *plane,
 				    struct drm_plane_state *state)
 {
-	struct qxl_device *qdev = plane->dev->dev_private;
+	struct qxl_device *qdev = to_qxl(plane->dev);
 	struct qxl_bo *bo;
 
 	if (!state->crtc || !state->fb)
@@ -476,7 +476,7 @@ static int qxl_primary_atomic_check(struct drm_plane *plane,
 static int qxl_primary_apply_cursor(struct drm_plane *plane)
 {
 	struct drm_device *dev = plane->dev;
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct drm_framebuffer *fb = plane->state->fb;
 	struct qxl_crtc *qcrtc = to_qxl_crtc(plane->state->crtc);
 	struct qxl_cursor_cmd *cmd;
@@ -523,7 +523,7 @@ static int qxl_primary_apply_cursor(struct drm_plane *plane)
 static void qxl_primary_atomic_update(struct drm_plane *plane,
 				      struct drm_plane_state *old_state)
 {
-	struct qxl_device *qdev = plane->dev->dev_private;
+	struct qxl_device *qdev = to_qxl(plane->dev);
 	struct qxl_bo *bo = gem_to_qxl_bo(plane->state->fb->obj[0]);
 	struct qxl_bo *primary;
 	struct drm_clip_rect norect = {
@@ -554,7 +554,7 @@ static void qxl_primary_atomic_update(struct drm_plane *plane,
 static void qxl_primary_atomic_disable(struct drm_plane *plane,
 				       struct drm_plane_state *old_state)
 {
-	struct qxl_device *qdev = plane->dev->dev_private;
+	struct qxl_device *qdev = to_qxl(plane->dev);
 
 	if (old_state->fb) {
 		struct qxl_bo *bo = gem_to_qxl_bo(old_state->fb->obj[0]);
@@ -570,7 +570,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,
 				     struct drm_plane_state *old_state)
 {
 	struct drm_device *dev = plane->dev;
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct drm_framebuffer *fb = plane->state->fb;
 	struct qxl_crtc *qcrtc = to_qxl_crtc(plane->state->crtc);
 	struct qxl_release *release;
@@ -679,7 +679,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,
 static void qxl_cursor_atomic_disable(struct drm_plane *plane,
 				      struct drm_plane_state *old_state)
 {
-	struct qxl_device *qdev = plane->dev->dev_private;
+	struct qxl_device *qdev = to_qxl(plane->dev);
 	struct qxl_release *release;
 	struct qxl_cursor_cmd *cmd;
 	int ret;
@@ -762,7 +762,7 @@ static void qxl_calc_dumb_shadow(struct qxl_device *qdev,
 static int qxl_plane_prepare_fb(struct drm_plane *plane,
 				struct drm_plane_state *new_state)
 {
-	struct qxl_device *qdev = plane->dev->dev_private;
+	struct qxl_device *qdev = to_qxl(plane->dev);
 	struct drm_gem_object *obj;
 	struct qxl_bo *user_bo;
 	struct qxl_surface surf;
@@ -923,7 +923,7 @@ static int qdev_crtc_init(struct drm_device *dev, int crtc_id)
 {
 	struct qxl_crtc *qxl_crtc;
 	struct drm_plane *primary, *cursor;
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	int r;
 
 	qxl_crtc = kzalloc(sizeof(struct qxl_crtc), GFP_KERNEL);
@@ -965,7 +965,7 @@ static int qdev_crtc_init(struct drm_device *dev, int crtc_id)
 static int qxl_conn_get_modes(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct qxl_output *output = drm_connector_to_qxl_output(connector);
 	unsigned int pwidth = 1024;
 	unsigned int pheight = 768;
@@ -991,7 +991,7 @@ static enum drm_mode_status qxl_conn_mode_valid(struct drm_connector *connector,
 			       struct drm_display_mode *mode)
 {
 	struct drm_device *ddev = connector->dev;
-	struct qxl_device *qdev = ddev->dev_private;
+	struct qxl_device *qdev = to_qxl(ddev);
 
 	if (qxl_check_mode(qdev, mode->hdisplay, mode->vdisplay) != 0)
 		return MODE_BAD;
@@ -1021,7 +1021,7 @@ static enum drm_connector_status qxl_conn_detect(
 	struct qxl_output *output =
 		drm_connector_to_qxl_output(connector);
 	struct drm_device *ddev = connector->dev;
-	struct qxl_device *qdev = ddev->dev_private;
+	struct qxl_device *qdev = to_qxl(ddev);
 	bool connected = false;
 
 	/* The first monitor is always connected */
@@ -1071,7 +1071,7 @@ static int qxl_mode_create_hotplug_mode_update_property(struct qxl_device *qdev)
 
 static int qdev_output_init(struct drm_device *dev, int num_output)
 {
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct qxl_output *qxl_output;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 6b4ae4c5fb76..13872b882775 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -137,7 +137,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 static void qxl_drm_release(struct drm_device *dev)
 {
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 
 	/*
 	 * TODO: qxl_device_fini() call should be in qxl_pci_remove(),
@@ -164,7 +164,7 @@ DEFINE_DRM_GEM_FOPS(qxl_fops);
 static int qxl_drm_freeze(struct drm_device *dev)
 {
 	struct pci_dev *pdev = dev->pdev;
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	int ret;
 
 	ret = drm_mode_config_helper_suspend(dev);
@@ -186,7 +186,7 @@ static int qxl_drm_freeze(struct drm_device *dev)
 
 static int qxl_drm_resume(struct drm_device *dev, bool thaw)
 {
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 
 	qdev->ram_header->int_mask = QXL_INTERRUPT_MASK;
 	if (!thaw) {
@@ -245,7 +245,7 @@ static int qxl_pm_restore(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
-	struct qxl_device *qdev = drm_dev->dev_private;
+	struct qxl_device *qdev = to_qxl(drm_dev);
 
 	qxl_io_reset(qdev);
 	return qxl_drm_resume(drm_dev, false);
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 86ac191d9205..31e35f787df2 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -192,8 +192,6 @@ struct qxl_debugfs {
 
 int qxl_debugfs_fence_init(struct qxl_device *rdev);
 
-struct qxl_device;
-
 struct qxl_device {
 	struct drm_device ddev;
 
@@ -273,6 +271,8 @@ struct qxl_device {
 	int monitors_config_height;
 };
 
+#define to_qxl(dev) container_of(dev, struct qxl_device, ddev)
+
 extern const struct drm_ioctl_desc qxl_ioctls[];
 extern int qxl_max_ioctl;
 
diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c
index 272d19b677d8..24e903383aa1 100644
--- a/drivers/gpu/drm/qxl/qxl_dumb.c
+++ b/drivers/gpu/drm/qxl/qxl_dumb.c
@@ -32,7 +32,7 @@ int qxl_mode_dumb_create(struct drm_file *file_priv,
 			    struct drm_device *dev,
 			    struct drm_mode_create_dumb *args)
 {
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct qxl_bo *qobj;
 	uint32_t handle;
 	int r;
diff --git a/drivers/gpu/drm/qxl/qxl_gem.c b/drivers/gpu/drm/qxl/qxl_gem.c
index 69f37db1027a..5ff6fa9b799c 100644
--- a/drivers/gpu/drm/qxl/qxl_gem.c
+++ b/drivers/gpu/drm/qxl/qxl_gem.c
@@ -34,7 +34,7 @@ void qxl_gem_object_free(struct drm_gem_object *gobj)
 	struct qxl_device *qdev;
 	struct ttm_buffer_object *tbo;
 
-	qdev = (struct qxl_device *)gobj->dev->dev_private;
+	qdev = to_qxl(gobj->dev);
 
 	qxl_surface_evict(qdev, qobj, false);
 
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 8117a45b3610..d9a583966949 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -36,7 +36,7 @@
 static int qxl_alloc_ioctl(struct drm_device *dev, void *data,
 			   struct drm_file *file_priv)
 {
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct drm_qxl_alloc *qxl_alloc = data;
 	int ret;
 	struct qxl_bo *qobj;
@@ -64,7 +64,7 @@ static int qxl_alloc_ioctl(struct drm_device *dev, void *data,
 static int qxl_map_ioctl(struct drm_device *dev, void *data,
 			 struct drm_file *file_priv)
 {
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct drm_qxl_map *qxl_map = data;
 
 	return qxl_mode_dumb_mmap(file_priv, &qdev->ddev, qxl_map->handle,
@@ -279,7 +279,7 @@ static int qxl_process_single_command(struct qxl_device *qdev,
 static int qxl_execbuffer_ioctl(struct drm_device *dev, void *data,
 				struct drm_file *file_priv)
 {
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct drm_qxl_execbuffer *execbuffer = data;
 	struct drm_qxl_command user_cmd;
 	int cmd_num;
@@ -304,7 +304,7 @@ static int qxl_execbuffer_ioctl(struct drm_device *dev, void *data,
 static int qxl_update_area_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file)
 {
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct drm_qxl_update_area *update_area = data;
 	struct qxl_rect area = {.left = update_area->left,
 				.top = update_area->top,
@@ -354,7 +354,7 @@ static int qxl_update_area_ioctl(struct drm_device *dev, void *data,
 static int qxl_getparam_ioctl(struct drm_device *dev, void *data,
 		       struct drm_file *file_priv)
 {
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct drm_qxl_getparam *param = data;
 
 	switch (param->param) {
@@ -373,7 +373,7 @@ static int qxl_getparam_ioctl(struct drm_device *dev, void *data,
 static int qxl_clientcap_ioctl(struct drm_device *dev, void *data,
 				  struct drm_file *file_priv)
 {
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct drm_qxl_clientcap *param = data;
 	int byte, idx;
 
@@ -394,7 +394,7 @@ static int qxl_clientcap_ioctl(struct drm_device *dev, void *data,
 static int qxl_alloc_surf_ioctl(struct drm_device *dev, void *data,
 				struct drm_file *file)
 {
-	struct qxl_device *qdev = dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	struct drm_qxl_alloc_surf *param = data;
 	struct qxl_bo *qobj;
 	int handle;
diff --git a/drivers/gpu/drm/qxl/qxl_irq.c b/drivers/gpu/drm/qxl/qxl_irq.c
index 8435af108632..1ba5a702d763 100644
--- a/drivers/gpu/drm/qxl/qxl_irq.c
+++ b/drivers/gpu/drm/qxl/qxl_irq.c
@@ -32,7 +32,7 @@
 irqreturn_t qxl_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;
-	struct qxl_device *qdev = (struct qxl_device *)dev->dev_private;
+	struct qxl_device *qdev = to_qxl(dev);
 	uint32_t pending;
 
 	pending = xchg(&qdev->ram_header->int_pending, 0);
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 91a34dd835d7..a6d873052cd4 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -114,7 +114,6 @@ int qxl_device_init(struct qxl_device *qdev,
 
 	qdev->ddev.pdev = pdev;
 	pci_set_drvdata(pdev, &qdev->ddev);
-	qdev->ddev.dev_private = qdev;
 
 	mutex_init(&qdev->gem.mutex);
 	mutex_init(&qdev->update_area_mutex);
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index ab72dc3476e9..edc8a9916872 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -33,7 +33,7 @@ static void qxl_ttm_bo_destroy(struct ttm_buffer_object *tbo)
 	struct qxl_device *qdev;
 
 	bo = to_qxl_bo(tbo);
-	qdev = (struct qxl_device *)bo->tbo.base.dev->dev_private;
+	qdev = to_qxl(bo->tbo.base.dev);
 
 	qxl_surface_evict(qdev, bo, false);
 	WARN_ON_ONCE(bo->map_count > 0);
diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index 2feca734c7b1..4fae3e393da1 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -243,7 +243,7 @@ static int qxl_release_validate_bo(struct qxl_bo *bo)
 		return ret;
 
 	/* allocate a surface for reserved + validated buffers */
-	ret = qxl_bo_check_id(bo->tbo.base.dev->dev_private, bo);
+	ret = qxl_bo_check_id(to_qxl(bo->tbo.base.dev), bo);
 	if (ret)
 		return ret;
 	return 0;
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 93a2eb14844b..f09a712b1ed2 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -243,7 +243,7 @@ static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
 	if (!qxl_ttm_bo_is_qxl_bo(bo))
 		return;
 	qbo = to_qxl_bo(bo);
-	qdev = qbo->tbo.base.dev->dev_private;
+	qdev = to_qxl(qbo->tbo.base.dev);
 
 	if (bo->mem.mem_type == TTM_PL_PRIV && qbo->surface_id)
 		qxl_surface_evict(qdev, qbo, new_mem ? true : false);
-- 
2.25.1

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

* [PATCH 35/59] drm/cirrus: Use devm_drm_dev_alloc
       [not found] <20200415074034.175360-1-daniel.vetter@ffwll.ch>
  2020-04-15  7:40 ` [PATCH 26/59] drm/qxl: Use devm_drm_dev_alloc Daniel Vetter
  2020-04-15  7:40 ` [PATCH 27/59] drm/qxl: Don't use drm_device->dev_private Daniel Vetter
@ 2020-04-15  7:40 ` Daniel Vetter
  2020-04-15  7:40 ` [PATCH 36/59] drm/cirrus: Don't use drm_device->dev_private Daniel Vetter
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2020-04-15  7:40 UTC (permalink / raw)
  To: Intel Graphics Development
  Cc: Rob Herring, Daniel Vetter, DRI Development, virtualization,
	Noralf Trønnes, Thomas Zimmermann, Daniel Vetter,
	Dave Airlie, Sam Ravnborg, Emil Velikov

Already using devm_drm_dev_init, so very simple replacment.

Acked-by: Noralf Trønnes <noralf@tronnes.org>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: virtualization@lists.linux-foundation.org
Cc: Emil Velikov <emil.velikov@collabora.com>
---
 drivers/gpu/drm/cirrus/cirrus.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index a36269717c3b..4b65637147ba 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -567,18 +567,13 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
 		return ret;
 
 	ret = -ENOMEM;
-	cirrus = kzalloc(sizeof(*cirrus), GFP_KERNEL);
-	if (cirrus == NULL)
-		return ret;
+	cirrus = devm_drm_dev_alloc(&pdev->dev, &cirrus_driver,
+				    struct cirrus_device, dev);
+	if (IS_ERR(cirrus))
+		return PTR_ERR(cirrus);
 
 	dev = &cirrus->dev;
-	ret = devm_drm_dev_init(&pdev->dev, dev, &cirrus_driver);
-	if (ret) {
-		kfree(cirrus);
-		return ret;
-	}
 	dev->dev_private = cirrus;
-	drmm_add_final_kfree(dev, cirrus);
 
 	cirrus->vram = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0),
 				    pci_resource_len(pdev, 0));
-- 
2.25.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* [PATCH 36/59] drm/cirrus: Don't use drm_device->dev_private
       [not found] <20200415074034.175360-1-daniel.vetter@ffwll.ch>
                   ` (2 preceding siblings ...)
  2020-04-15  7:40 ` [PATCH 35/59] drm/cirrus: Use devm_drm_dev_alloc Daniel Vetter
@ 2020-04-15  7:40 ` Daniel Vetter
  2020-04-15  7:40 ` [PATCH 37/59] drm/cirrus: Move to drm/tiny Daniel Vetter
  2020-04-15  7:40 ` [PATCH 59/59] drm/bochs: Remove explicit drm_connector_register Daniel Vetter
  5 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2020-04-15  7:40 UTC (permalink / raw)
  To: Intel Graphics Development
  Cc: Daniel Vetter, DRI Development, virtualization, Eric Anholt,
	Noralf Trønnes, Thomas Zimmermann, Dave Airlie,
	Daniel Vetter, Sam Ravnborg

Upcasting using a container_of macro is more typesafe, faster and
easier for the compiler to optimize.

Acked-by: Eric Anholt <eric@anholt.net>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Noralf Trønnes" <noralf@tronnes.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: virtualization@lists.linux-foundation.org
---
 drivers/gpu/drm/cirrus/cirrus.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index 4b65637147ba..744a8e337e41 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -59,6 +59,8 @@ struct cirrus_device {
 	void __iomem		       *mmio;
 };
 
+#define to_cirrus(_dev) container_of(_dev, struct cirrus_device, dev)
+
 /* ------------------------------------------------------------------ */
 /*
  * The meat of this driver. The core passes us a mode and we have to program
@@ -311,7 +313,7 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
 			       struct drm_rect *rect)
 {
-	struct cirrus_device *cirrus = fb->dev->dev_private;
+	struct cirrus_device *cirrus = to_cirrus(fb->dev);
 	void *vmap;
 	int idx, ret;
 
@@ -436,7 +438,7 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe,
 			       struct drm_crtc_state *crtc_state,
 			       struct drm_plane_state *plane_state)
 {
-	struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
+	struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev);
 
 	cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb);
 	cirrus_fb_blit_fullscreen(plane_state->fb);
@@ -445,7 +447,7 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe,
 static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
 			       struct drm_plane_state *old_state)
 {
-	struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
+	struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev);
 	struct drm_plane_state *state = pipe->plane.state;
 	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_rect rect;
@@ -573,7 +575,6 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
 		return PTR_ERR(cirrus);
 
 	dev = &cirrus->dev;
-	dev->dev_private = cirrus;
 
 	cirrus->vram = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0),
 				    pci_resource_len(pdev, 0));
-- 
2.25.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* [PATCH 37/59] drm/cirrus: Move to drm/tiny
       [not found] <20200415074034.175360-1-daniel.vetter@ffwll.ch>
                   ` (3 preceding siblings ...)
  2020-04-15  7:40 ` [PATCH 36/59] drm/cirrus: Don't use drm_device->dev_private Daniel Vetter
@ 2020-04-15  7:40 ` Daniel Vetter
  2020-04-15  8:01   ` Thomas Zimmermann
                     ` (2 more replies)
  2020-04-15  7:40 ` [PATCH 59/59] drm/bochs: Remove explicit drm_connector_register Daniel Vetter
  5 siblings, 3 replies; 19+ messages in thread
From: Daniel Vetter @ 2020-04-15  7:40 UTC (permalink / raw)
  To: Intel Graphics Development
  Cc: Daniel Vetter, DRI Development, virtualization, Daniel Vetter,
	Dave Airlie

Because it is. Huge congrats to everyone who made this kind of
refactoring happen!

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
---
 MAINTAINERS                               |  2 +-
 drivers/gpu/drm/Kconfig                   |  2 --
 drivers/gpu/drm/Makefile                  |  1 -
 drivers/gpu/drm/cirrus/Kconfig            | 19 -------------------
 drivers/gpu/drm/cirrus/Makefile           |  2 --
 drivers/gpu/drm/tiny/Kconfig              | 19 +++++++++++++++++++
 drivers/gpu/drm/tiny/Makefile             |  1 +
 drivers/gpu/drm/{cirrus => tiny}/cirrus.c |  0
 8 files changed, 21 insertions(+), 25 deletions(-)
 delete mode 100644 drivers/gpu/drm/cirrus/Kconfig
 delete mode 100644 drivers/gpu/drm/cirrus/Makefile
 rename drivers/gpu/drm/{cirrus => tiny}/cirrus.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7b3255d96d1d..0a5cf105ee37 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5397,7 +5397,7 @@ L:	virtualization@lists.linux-foundation.org
 S:	Obsolete
 W:	https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
 T:	git git://anongit.freedesktop.org/drm/drm-misc
-F:	drivers/gpu/drm/cirrus/
+F:	drivers/gpu/drm/tiny/cirrus.c
 
 DRM DRIVER FOR QXL VIRTUAL GPU
 M:	Dave Airlie <airlied@redhat.com>
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 43594978958e..4f4e7fa001c1 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -310,8 +310,6 @@ source "drivers/gpu/drm/ast/Kconfig"
 
 source "drivers/gpu/drm/mgag200/Kconfig"
 
-source "drivers/gpu/drm/cirrus/Kconfig"
-
 source "drivers/gpu/drm/armada/Kconfig"
 
 source "drivers/gpu/drm/atmel-hlcdc/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index f34d08c83485..2c0e5a7e5953 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -74,7 +74,6 @@ obj-$(CONFIG_DRM_I915)	+= i915/
 obj-$(CONFIG_DRM_MGAG200) += mgag200/
 obj-$(CONFIG_DRM_V3D)  += v3d/
 obj-$(CONFIG_DRM_VC4)  += vc4/
-obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
 obj-$(CONFIG_DRM_SIS)   += sis/
 obj-$(CONFIG_DRM_SAVAGE)+= savage/
 obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
diff --git a/drivers/gpu/drm/cirrus/Kconfig b/drivers/gpu/drm/cirrus/Kconfig
deleted file mode 100644
index c6bbd988b0e5..000000000000
--- a/drivers/gpu/drm/cirrus/Kconfig
+++ /dev/null
@@ -1,19 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only
-config DRM_CIRRUS_QEMU
-	tristate "Cirrus driver for QEMU emulated device"
-	depends on DRM && PCI && MMU
-	select DRM_KMS_HELPER
-	select DRM_GEM_SHMEM_HELPER
-	help
-	 This is a KMS driver for emulated cirrus device in qemu.
-	 It is *NOT* intended for real cirrus devices. This requires
-	 the modesetting userspace X.org driver.
-
-	 Cirrus is obsolete, the hardware was designed in the 90ies
-	 and can't keep up with todays needs.  More background:
-	 https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
-
-	 Better alternatives are:
-	   - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+)
-	   - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
-	   - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
diff --git a/drivers/gpu/drm/cirrus/Makefile b/drivers/gpu/drm/cirrus/Makefile
deleted file mode 100644
index 0c1ed3f99725..000000000000
--- a/drivers/gpu/drm/cirrus/Makefile
+++ /dev/null
@@ -1,2 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index 4160e74e4751..2b6414f0fa75 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -1,5 +1,24 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
+config DRM_CIRRUS_QEMU
+	tristate "Cirrus driver for QEMU emulated device"
+	depends on DRM && PCI && MMU
+	select DRM_KMS_HELPER
+	select DRM_GEM_SHMEM_HELPER
+	help
+	 This is a KMS driver for emulated cirrus device in qemu.
+	 It is *NOT* intended for real cirrus devices. This requires
+	 the modesetting userspace X.org driver.
+
+	 Cirrus is obsolete, the hardware was designed in the 90ies
+	 and can't keep up with todays needs.  More background:
+	 https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
+
+	 Better alternatives are:
+	   - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+)
+	   - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
+	   - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
+
 config DRM_GM12U320
 	tristate "GM12U320 driver for USB projectors"
 	depends on DRM && USB
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index c96ceee71453..6ae4e9e5a35f 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
+obj-$(CONFIG_DRM_CIRRUS_QEMU)		+= cirrus.o
 obj-$(CONFIG_DRM_GM12U320)		+= gm12u320.o
 obj-$(CONFIG_TINYDRM_HX8357D)		+= hx8357d.o
 obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
similarity index 100%
rename from drivers/gpu/drm/cirrus/cirrus.c
rename to drivers/gpu/drm/tiny/cirrus.c
-- 
2.25.1

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

* [PATCH 59/59] drm/bochs: Remove explicit drm_connector_register
       [not found] <20200415074034.175360-1-daniel.vetter@ffwll.ch>
                   ` (4 preceding siblings ...)
  2020-04-15  7:40 ` [PATCH 37/59] drm/cirrus: Move to drm/tiny Daniel Vetter
@ 2020-04-15  7:40 ` Daniel Vetter
  2020-04-21  7:39   ` Gerd Hoffmann
  2020-04-24 18:11   ` Sam Ravnborg
  5 siblings, 2 replies; 19+ messages in thread
From: Daniel Vetter @ 2020-04-15  7:40 UTC (permalink / raw)
  To: Intel Graphics Development
  Cc: Daniel Vetter, virtualization, DRI Development, Daniel Vetter

This is leftovers from the old drm_driver->load callback
upside-down issues. It doesn't do anything for not-hotplugged
connectors since drm_dev_register takes care of that.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
---
 drivers/gpu/drm/bochs/bochs_kms.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
index 7f4bcfad87e9..05d8373888e8 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -104,7 +104,6 @@ static void bochs_connector_init(struct drm_device *dev)
 			   DRM_MODE_CONNECTOR_VIRTUAL);
 	drm_connector_helper_add(connector,
 				 &bochs_connector_connector_helper_funcs);
-	drm_connector_register(connector);
 
 	bochs_hw_load_edid(bochs);
 	if (bochs->edid) {
-- 
2.25.1

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

* Re: [PATCH 37/59] drm/cirrus: Move to drm/tiny
  2020-04-15  7:40 ` [PATCH 37/59] drm/cirrus: Move to drm/tiny Daniel Vetter
@ 2020-04-15  8:01   ` Thomas Zimmermann
  2020-04-15  8:19     ` Daniel Vetter
  2020-04-21  7:37   ` Gerd Hoffmann
  2020-04-24 16:37   ` Sam Ravnborg
  2 siblings, 1 reply; 19+ messages in thread
From: Thomas Zimmermann @ 2020-04-15  8:01 UTC (permalink / raw)
  To: Daniel Vetter, Intel Graphics Development
  Cc: Daniel Vetter, Dave Airlie, Gerd Hoffmann, DRI Development,
	virtualization


[-- Attachment #1.1.1: Type: text/plain, Size: 6152 bytes --]



Am 15.04.20 um 09:40 schrieb Daniel Vetter:
> Because it is. Huge congrats to everyone who made this kind of
> refactoring happen!

Every other week, I felt an urge to send out this patch. Thank you so
much, Daniel! There are more candidates for tiny/. They are all <20k
LOCs and all we'd have to do is to move their code into a single file.
bochs or arc come into my mind.

> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: virtualization@lists.linux-foundation.org

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>  MAINTAINERS                               |  2 +-
>  drivers/gpu/drm/Kconfig                   |  2 --
>  drivers/gpu/drm/Makefile                  |  1 -
>  drivers/gpu/drm/cirrus/Kconfig            | 19 -------------------
>  drivers/gpu/drm/cirrus/Makefile           |  2 --
>  drivers/gpu/drm/tiny/Kconfig              | 19 +++++++++++++++++++
>  drivers/gpu/drm/tiny/Makefile             |  1 +
>  drivers/gpu/drm/{cirrus => tiny}/cirrus.c |  0
>  8 files changed, 21 insertions(+), 25 deletions(-)
>  delete mode 100644 drivers/gpu/drm/cirrus/Kconfig
>  delete mode 100644 drivers/gpu/drm/cirrus/Makefile
>  rename drivers/gpu/drm/{cirrus => tiny}/cirrus.c (100%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7b3255d96d1d..0a5cf105ee37 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5397,7 +5397,7 @@ L:	virtualization@lists.linux-foundation.org
>  S:	Obsolete
>  W:	https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
>  T:	git git://anongit.freedesktop.org/drm/drm-misc
> -F:	drivers/gpu/drm/cirrus/
> +F:	drivers/gpu/drm/tiny/cirrus.c
>  
>  DRM DRIVER FOR QXL VIRTUAL GPU
>  M:	Dave Airlie <airlied@redhat.com>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 43594978958e..4f4e7fa001c1 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -310,8 +310,6 @@ source "drivers/gpu/drm/ast/Kconfig"
>  
>  source "drivers/gpu/drm/mgag200/Kconfig"
>  
> -source "drivers/gpu/drm/cirrus/Kconfig"
> -
>  source "drivers/gpu/drm/armada/Kconfig"
>  
>  source "drivers/gpu/drm/atmel-hlcdc/Kconfig"
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index f34d08c83485..2c0e5a7e5953 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -74,7 +74,6 @@ obj-$(CONFIG_DRM_I915)	+= i915/
>  obj-$(CONFIG_DRM_MGAG200) += mgag200/
>  obj-$(CONFIG_DRM_V3D)  += v3d/
>  obj-$(CONFIG_DRM_VC4)  += vc4/
> -obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
>  obj-$(CONFIG_DRM_SIS)   += sis/
>  obj-$(CONFIG_DRM_SAVAGE)+= savage/
>  obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
> diff --git a/drivers/gpu/drm/cirrus/Kconfig b/drivers/gpu/drm/cirrus/Kconfig
> deleted file mode 100644
> index c6bbd988b0e5..000000000000
> --- a/drivers/gpu/drm/cirrus/Kconfig
> +++ /dev/null
> @@ -1,19 +0,0 @@
> -# SPDX-License-Identifier: GPL-2.0-only
> -config DRM_CIRRUS_QEMU
> -	tristate "Cirrus driver for QEMU emulated device"
> -	depends on DRM && PCI && MMU
> -	select DRM_KMS_HELPER
> -	select DRM_GEM_SHMEM_HELPER
> -	help
> -	 This is a KMS driver for emulated cirrus device in qemu.
> -	 It is *NOT* intended for real cirrus devices. This requires
> -	 the modesetting userspace X.org driver.
> -
> -	 Cirrus is obsolete, the hardware was designed in the 90ies
> -	 and can't keep up with todays needs.  More background:
> -	 https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
> -
> -	 Better alternatives are:
> -	   - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+)
> -	   - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
> -	   - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
> diff --git a/drivers/gpu/drm/cirrus/Makefile b/drivers/gpu/drm/cirrus/Makefile
> deleted file mode 100644
> index 0c1ed3f99725..000000000000
> --- a/drivers/gpu/drm/cirrus/Makefile
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -# SPDX-License-Identifier: GPL-2.0-only
> -obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index 4160e74e4751..2b6414f0fa75 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -1,5 +1,24 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  
> +config DRM_CIRRUS_QEMU
> +	tristate "Cirrus driver for QEMU emulated device"
> +	depends on DRM && PCI && MMU
> +	select DRM_KMS_HELPER
> +	select DRM_GEM_SHMEM_HELPER
> +	help
> +	 This is a KMS driver for emulated cirrus device in qemu.
> +	 It is *NOT* intended for real cirrus devices. This requires
> +	 the modesetting userspace X.org driver.
> +
> +	 Cirrus is obsolete, the hardware was designed in the 90ies
> +	 and can't keep up with todays needs.  More background:
> +	 https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
> +
> +	 Better alternatives are:
> +	   - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+)
> +	   - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
> +	   - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
> +
>  config DRM_GM12U320
>  	tristate "GM12U320 driver for USB projectors"
>  	depends on DRM && USB
> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> index c96ceee71453..6ae4e9e5a35f 100644
> --- a/drivers/gpu/drm/tiny/Makefile
> +++ b/drivers/gpu/drm/tiny/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  
> +obj-$(CONFIG_DRM_CIRRUS_QEMU)		+= cirrus.o
>  obj-$(CONFIG_DRM_GM12U320)		+= gm12u320.o
>  obj-$(CONFIG_TINYDRM_HX8357D)		+= hx8357d.o
>  obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
> similarity index 100%
> rename from drivers/gpu/drm/cirrus/cirrus.c
> rename to drivers/gpu/drm/tiny/cirrus.c
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 37/59] drm/cirrus: Move to drm/tiny
  2020-04-15  8:01   ` Thomas Zimmermann
@ 2020-04-15  8:19     ` Daniel Vetter
  2020-04-15  8:46       ` Thomas Zimmermann
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2020-04-15  8:19 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Intel Graphics Development, DRI Development,
	open list:VIRTIO CORE, NET..., Gerd Hoffmann, Dave Airlie,
	Daniel Vetter

On Wed, Apr 15, 2020 at 10:01 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
>
>
> Am 15.04.20 um 09:40 schrieb Daniel Vetter:
> > Because it is. Huge congrats to everyone who made this kind of
> > refactoring happen!
>
> Every other week, I felt an urge to send out this patch. Thank you so
> much, Daniel! There are more candidates for tiny/. They are all <20k
> LOCs and all we'd have to do is to move their code into a single file.
> bochs or arc come into my mind.

arc I have (later in the series), bochs I feel like is maybe a bit too
big. I'd put the limit for tiny well below 1kloc including whitespace
and all that. bochs might be a candidate once we've helperized a few
more things perhaps.

btw I drmm_ version of vram helpers would help a bunch of these drivers I think.
-Daniel

>
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: virtualization@lists.linux-foundation.org
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> > ---
> >  MAINTAINERS                               |  2 +-
> >  drivers/gpu/drm/Kconfig                   |  2 --
> >  drivers/gpu/drm/Makefile                  |  1 -
> >  drivers/gpu/drm/cirrus/Kconfig            | 19 -------------------
> >  drivers/gpu/drm/cirrus/Makefile           |  2 --
> >  drivers/gpu/drm/tiny/Kconfig              | 19 +++++++++++++++++++
> >  drivers/gpu/drm/tiny/Makefile             |  1 +
> >  drivers/gpu/drm/{cirrus => tiny}/cirrus.c |  0
> >  8 files changed, 21 insertions(+), 25 deletions(-)
> >  delete mode 100644 drivers/gpu/drm/cirrus/Kconfig
> >  delete mode 100644 drivers/gpu/drm/cirrus/Makefile
> >  rename drivers/gpu/drm/{cirrus => tiny}/cirrus.c (100%)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 7b3255d96d1d..0a5cf105ee37 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -5397,7 +5397,7 @@ L:      virtualization@lists.linux-foundation.org
> >  S:   Obsolete
> >  W:   https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
> >  T:   git git://anongit.freedesktop.org/drm/drm-misc
> > -F:   drivers/gpu/drm/cirrus/
> > +F:   drivers/gpu/drm/tiny/cirrus.c
> >
> >  DRM DRIVER FOR QXL VIRTUAL GPU
> >  M:   Dave Airlie <airlied@redhat.com>
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 43594978958e..4f4e7fa001c1 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -310,8 +310,6 @@ source "drivers/gpu/drm/ast/Kconfig"
> >
> >  source "drivers/gpu/drm/mgag200/Kconfig"
> >
> > -source "drivers/gpu/drm/cirrus/Kconfig"
> > -
> >  source "drivers/gpu/drm/armada/Kconfig"
> >
> >  source "drivers/gpu/drm/atmel-hlcdc/Kconfig"
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index f34d08c83485..2c0e5a7e5953 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -74,7 +74,6 @@ obj-$(CONFIG_DRM_I915)      += i915/
> >  obj-$(CONFIG_DRM_MGAG200) += mgag200/
> >  obj-$(CONFIG_DRM_V3D)  += v3d/
> >  obj-$(CONFIG_DRM_VC4)  += vc4/
> > -obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
> >  obj-$(CONFIG_DRM_SIS)   += sis/
> >  obj-$(CONFIG_DRM_SAVAGE)+= savage/
> >  obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
> > diff --git a/drivers/gpu/drm/cirrus/Kconfig b/drivers/gpu/drm/cirrus/Kconfig
> > deleted file mode 100644
> > index c6bbd988b0e5..000000000000
> > --- a/drivers/gpu/drm/cirrus/Kconfig
> > +++ /dev/null
> > @@ -1,19 +0,0 @@
> > -# SPDX-License-Identifier: GPL-2.0-only
> > -config DRM_CIRRUS_QEMU
> > -     tristate "Cirrus driver for QEMU emulated device"
> > -     depends on DRM && PCI && MMU
> > -     select DRM_KMS_HELPER
> > -     select DRM_GEM_SHMEM_HELPER
> > -     help
> > -      This is a KMS driver for emulated cirrus device in qemu.
> > -      It is *NOT* intended for real cirrus devices. This requires
> > -      the modesetting userspace X.org driver.
> > -
> > -      Cirrus is obsolete, the hardware was designed in the 90ies
> > -      and can't keep up with todays needs.  More background:
> > -      https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
> > -
> > -      Better alternatives are:
> > -        - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+)
> > -        - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
> > -        - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
> > diff --git a/drivers/gpu/drm/cirrus/Makefile b/drivers/gpu/drm/cirrus/Makefile
> > deleted file mode 100644
> > index 0c1ed3f99725..000000000000
> > --- a/drivers/gpu/drm/cirrus/Makefile
> > +++ /dev/null
> > @@ -1,2 +0,0 @@
> > -# SPDX-License-Identifier: GPL-2.0-only
> > -obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o
> > diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> > index 4160e74e4751..2b6414f0fa75 100644
> > --- a/drivers/gpu/drm/tiny/Kconfig
> > +++ b/drivers/gpu/drm/tiny/Kconfig
> > @@ -1,5 +1,24 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >
> > +config DRM_CIRRUS_QEMU
> > +     tristate "Cirrus driver for QEMU emulated device"
> > +     depends on DRM && PCI && MMU
> > +     select DRM_KMS_HELPER
> > +     select DRM_GEM_SHMEM_HELPER
> > +     help
> > +      This is a KMS driver for emulated cirrus device in qemu.
> > +      It is *NOT* intended for real cirrus devices. This requires
> > +      the modesetting userspace X.org driver.
> > +
> > +      Cirrus is obsolete, the hardware was designed in the 90ies
> > +      and can't keep up with todays needs.  More background:
> > +      https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
> > +
> > +      Better alternatives are:
> > +        - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+)
> > +        - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
> > +        - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
> > +
> >  config DRM_GM12U320
> >       tristate "GM12U320 driver for USB projectors"
> >       depends on DRM && USB
> > diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> > index c96ceee71453..6ae4e9e5a35f 100644
> > --- a/drivers/gpu/drm/tiny/Makefile
> > +++ b/drivers/gpu/drm/tiny/Makefile
> > @@ -1,5 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >
> > +obj-$(CONFIG_DRM_CIRRUS_QEMU)                += cirrus.o
> >  obj-$(CONFIG_DRM_GM12U320)           += gm12u320.o
> >  obj-$(CONFIG_TINYDRM_HX8357D)                += hx8357d.o
> >  obj-$(CONFIG_TINYDRM_ILI9225)                += ili9225.o
> > diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
> > similarity index 100%
> > rename from drivers/gpu/drm/cirrus/cirrus.c
> > rename to drivers/gpu/drm/tiny/cirrus.c
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 37/59] drm/cirrus: Move to drm/tiny
  2020-04-15  8:19     ` Daniel Vetter
@ 2020-04-15  8:46       ` Thomas Zimmermann
  2020-04-15  9:31         ` Daniel Vetter
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Zimmermann @ 2020-04-15  8:46 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, DRI Development,
	open list:VIRTIO CORE, NET..., Gerd Hoffmann, Dave Airlie,
	Daniel Vetter


[-- Attachment #1.1.1: Type: text/plain, Size: 7571 bytes --]



Am 15.04.20 um 10:19 schrieb Daniel Vetter:
> On Wed, Apr 15, 2020 at 10:01 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>>
>>
>> Am 15.04.20 um 09:40 schrieb Daniel Vetter:
>>> Because it is. Huge congrats to everyone who made this kind of
>>> refactoring happen!
>>
>> Every other week, I felt an urge to send out this patch. Thank you so
>> much, Daniel! There are more candidates for tiny/. They are all <20k
>> LOCs and all we'd have to do is to move their code into a single file.

I meant <20k file size, not LOCs.

>> bochs or arc come into my mind.
> 
> arc I have (later in the series), bochs I feel like is maybe a bit too
> big. I'd put the limit for tiny well below 1kloc including whitespace
> and all that. bochs might be a candidate once we've helperized a few
> more things perhaps.

True. The largest tiny driver is repaper with ~1.1k LOCS. Reading this
code, it seems like it has reached an upper bound of what is feasible.

Best regards
Thomas

> 
> btw I drmm_ version of vram helpers would help a bunch of these drivers I think.
> -Daniel
> 
>>
>>>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> Cc: Dave Airlie <airlied@redhat.com>
>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>> Cc: virtualization@lists.linux-foundation.org
>>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>>
>>> ---
>>>  MAINTAINERS                               |  2 +-
>>>  drivers/gpu/drm/Kconfig                   |  2 --
>>>  drivers/gpu/drm/Makefile                  |  1 -
>>>  drivers/gpu/drm/cirrus/Kconfig            | 19 -------------------
>>>  drivers/gpu/drm/cirrus/Makefile           |  2 --
>>>  drivers/gpu/drm/tiny/Kconfig              | 19 +++++++++++++++++++
>>>  drivers/gpu/drm/tiny/Makefile             |  1 +
>>>  drivers/gpu/drm/{cirrus => tiny}/cirrus.c |  0
>>>  8 files changed, 21 insertions(+), 25 deletions(-)
>>>  delete mode 100644 drivers/gpu/drm/cirrus/Kconfig
>>>  delete mode 100644 drivers/gpu/drm/cirrus/Makefile
>>>  rename drivers/gpu/drm/{cirrus => tiny}/cirrus.c (100%)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 7b3255d96d1d..0a5cf105ee37 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -5397,7 +5397,7 @@ L:      virtualization@lists.linux-foundation.org
>>>  S:   Obsolete
>>>  W:   https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
>>>  T:   git git://anongit.freedesktop.org/drm/drm-misc
>>> -F:   drivers/gpu/drm/cirrus/
>>> +F:   drivers/gpu/drm/tiny/cirrus.c
>>>
>>>  DRM DRIVER FOR QXL VIRTUAL GPU
>>>  M:   Dave Airlie <airlied@redhat.com>
>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>> index 43594978958e..4f4e7fa001c1 100644
>>> --- a/drivers/gpu/drm/Kconfig
>>> +++ b/drivers/gpu/drm/Kconfig
>>> @@ -310,8 +310,6 @@ source "drivers/gpu/drm/ast/Kconfig"
>>>
>>>  source "drivers/gpu/drm/mgag200/Kconfig"
>>>
>>> -source "drivers/gpu/drm/cirrus/Kconfig"
>>> -
>>>  source "drivers/gpu/drm/armada/Kconfig"
>>>
>>>  source "drivers/gpu/drm/atmel-hlcdc/Kconfig"
>>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>>> index f34d08c83485..2c0e5a7e5953 100644
>>> --- a/drivers/gpu/drm/Makefile
>>> +++ b/drivers/gpu/drm/Makefile
>>> @@ -74,7 +74,6 @@ obj-$(CONFIG_DRM_I915)      += i915/
>>>  obj-$(CONFIG_DRM_MGAG200) += mgag200/
>>>  obj-$(CONFIG_DRM_V3D)  += v3d/
>>>  obj-$(CONFIG_DRM_VC4)  += vc4/
>>> -obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
>>>  obj-$(CONFIG_DRM_SIS)   += sis/
>>>  obj-$(CONFIG_DRM_SAVAGE)+= savage/
>>>  obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
>>> diff --git a/drivers/gpu/drm/cirrus/Kconfig b/drivers/gpu/drm/cirrus/Kconfig
>>> deleted file mode 100644
>>> index c6bbd988b0e5..000000000000
>>> --- a/drivers/gpu/drm/cirrus/Kconfig
>>> +++ /dev/null
>>> @@ -1,19 +0,0 @@
>>> -# SPDX-License-Identifier: GPL-2.0-only
>>> -config DRM_CIRRUS_QEMU
>>> -     tristate "Cirrus driver for QEMU emulated device"
>>> -     depends on DRM && PCI && MMU
>>> -     select DRM_KMS_HELPER
>>> -     select DRM_GEM_SHMEM_HELPER
>>> -     help
>>> -      This is a KMS driver for emulated cirrus device in qemu.
>>> -      It is *NOT* intended for real cirrus devices. This requires
>>> -      the modesetting userspace X.org driver.
>>> -
>>> -      Cirrus is obsolete, the hardware was designed in the 90ies
>>> -      and can't keep up with todays needs.  More background:
>>> -      https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
>>> -
>>> -      Better alternatives are:
>>> -        - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+)
>>> -        - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
>>> -        - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
>>> diff --git a/drivers/gpu/drm/cirrus/Makefile b/drivers/gpu/drm/cirrus/Makefile
>>> deleted file mode 100644
>>> index 0c1ed3f99725..000000000000
>>> --- a/drivers/gpu/drm/cirrus/Makefile
>>> +++ /dev/null
>>> @@ -1,2 +0,0 @@
>>> -# SPDX-License-Identifier: GPL-2.0-only
>>> -obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o
>>> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
>>> index 4160e74e4751..2b6414f0fa75 100644
>>> --- a/drivers/gpu/drm/tiny/Kconfig
>>> +++ b/drivers/gpu/drm/tiny/Kconfig
>>> @@ -1,5 +1,24 @@
>>>  # SPDX-License-Identifier: GPL-2.0-only
>>>
>>> +config DRM_CIRRUS_QEMU
>>> +     tristate "Cirrus driver for QEMU emulated device"
>>> +     depends on DRM && PCI && MMU
>>> +     select DRM_KMS_HELPER
>>> +     select DRM_GEM_SHMEM_HELPER
>>> +     help
>>> +      This is a KMS driver for emulated cirrus device in qemu.
>>> +      It is *NOT* intended for real cirrus devices. This requires
>>> +      the modesetting userspace X.org driver.
>>> +
>>> +      Cirrus is obsolete, the hardware was designed in the 90ies
>>> +      and can't keep up with todays needs.  More background:
>>> +      https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
>>> +
>>> +      Better alternatives are:
>>> +        - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+)
>>> +        - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
>>> +        - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
>>> +
>>>  config DRM_GM12U320
>>>       tristate "GM12U320 driver for USB projectors"
>>>       depends on DRM && USB
>>> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
>>> index c96ceee71453..6ae4e9e5a35f 100644
>>> --- a/drivers/gpu/drm/tiny/Makefile
>>> +++ b/drivers/gpu/drm/tiny/Makefile
>>> @@ -1,5 +1,6 @@
>>>  # SPDX-License-Identifier: GPL-2.0-only
>>>
>>> +obj-$(CONFIG_DRM_CIRRUS_QEMU)                += cirrus.o
>>>  obj-$(CONFIG_DRM_GM12U320)           += gm12u320.o
>>>  obj-$(CONFIG_TINYDRM_HX8357D)                += hx8357d.o
>>>  obj-$(CONFIG_TINYDRM_ILI9225)                += ili9225.o
>>> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
>>> similarity index 100%
>>> rename from drivers/gpu/drm/cirrus/cirrus.c
>>> rename to drivers/gpu/drm/tiny/cirrus.c
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>>
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 37/59] drm/cirrus: Move to drm/tiny
  2020-04-15  8:46       ` Thomas Zimmermann
@ 2020-04-15  9:31         ` Daniel Vetter
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2020-04-15  9:31 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Intel Graphics Development, DRI Development,
	open list:VIRTIO CORE, NET..., Gerd Hoffmann, Dave Airlie,
	Daniel Vetter

On Wed, Apr 15, 2020 at 10:46 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
>
>
> Am 15.04.20 um 10:19 schrieb Daniel Vetter:
> > On Wed, Apr 15, 2020 at 10:01 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >>
> >>
> >> Am 15.04.20 um 09:40 schrieb Daniel Vetter:
> >>> Because it is. Huge congrats to everyone who made this kind of
> >>> refactoring happen!
> >>
> >> Every other week, I felt an urge to send out this patch. Thank you so
> >> much, Daniel! There are more candidates for tiny/. They are all <20k
> >> LOCs and all we'd have to do is to move their code into a single file.
>
> I meant <20k file size, not LOCs.
>
> >> bochs or arc come into my mind.
> >
> > arc I have (later in the series), bochs I feel like is maybe a bit too
> > big. I'd put the limit for tiny well below 1kloc including whitespace
> > and all that. bochs might be a candidate once we've helperized a few
> > more things perhaps.
>
> True. The largest tiny driver is repaper with ~1.1k LOCS. Reading this
> code, it seems like it has reached an upper bound of what is feasible.

Yeah, and I think the trouble there is that it contains an
epaper-specific panel abstraction. I think that's pushing it over the
edge. I think we've talked about subclassing drm_panel with
bus/device-specific stuff, that might be an option if this gets out of
hand.

Also I think we should be much stricter with putting drivers into
tiny/ than moving them out. Just so we avoid endless ping-pong, e.g.
for drivers that I can't make really tiny I wont bother with moving
them to tiny/
-Daniel

>
> Best regards
> Thomas
>
> >
> > btw I drmm_ version of vram helpers would help a bunch of these drivers I think.
> > -Daniel
> >
> >>
> >>>
> >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >>> Cc: Dave Airlie <airlied@redhat.com>
> >>> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >>> Cc: virtualization@lists.linux-foundation.org
> >>
> >> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> >>
> >>> ---
> >>>  MAINTAINERS                               |  2 +-
> >>>  drivers/gpu/drm/Kconfig                   |  2 --
> >>>  drivers/gpu/drm/Makefile                  |  1 -
> >>>  drivers/gpu/drm/cirrus/Kconfig            | 19 -------------------
> >>>  drivers/gpu/drm/cirrus/Makefile           |  2 --
> >>>  drivers/gpu/drm/tiny/Kconfig              | 19 +++++++++++++++++++
> >>>  drivers/gpu/drm/tiny/Makefile             |  1 +
> >>>  drivers/gpu/drm/{cirrus => tiny}/cirrus.c |  0
> >>>  8 files changed, 21 insertions(+), 25 deletions(-)
> >>>  delete mode 100644 drivers/gpu/drm/cirrus/Kconfig
> >>>  delete mode 100644 drivers/gpu/drm/cirrus/Makefile
> >>>  rename drivers/gpu/drm/{cirrus => tiny}/cirrus.c (100%)
> >>>
> >>> diff --git a/MAINTAINERS b/MAINTAINERS
> >>> index 7b3255d96d1d..0a5cf105ee37 100644
> >>> --- a/MAINTAINERS
> >>> +++ b/MAINTAINERS
> >>> @@ -5397,7 +5397,7 @@ L:      virtualization@lists.linux-foundation.org
> >>>  S:   Obsolete
> >>>  W:   https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
> >>>  T:   git git://anongit.freedesktop.org/drm/drm-misc
> >>> -F:   drivers/gpu/drm/cirrus/
> >>> +F:   drivers/gpu/drm/tiny/cirrus.c
> >>>
> >>>  DRM DRIVER FOR QXL VIRTUAL GPU
> >>>  M:   Dave Airlie <airlied@redhat.com>
> >>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> >>> index 43594978958e..4f4e7fa001c1 100644
> >>> --- a/drivers/gpu/drm/Kconfig
> >>> +++ b/drivers/gpu/drm/Kconfig
> >>> @@ -310,8 +310,6 @@ source "drivers/gpu/drm/ast/Kconfig"
> >>>
> >>>  source "drivers/gpu/drm/mgag200/Kconfig"
> >>>
> >>> -source "drivers/gpu/drm/cirrus/Kconfig"
> >>> -
> >>>  source "drivers/gpu/drm/armada/Kconfig"
> >>>
> >>>  source "drivers/gpu/drm/atmel-hlcdc/Kconfig"
> >>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> >>> index f34d08c83485..2c0e5a7e5953 100644
> >>> --- a/drivers/gpu/drm/Makefile
> >>> +++ b/drivers/gpu/drm/Makefile
> >>> @@ -74,7 +74,6 @@ obj-$(CONFIG_DRM_I915)      += i915/
> >>>  obj-$(CONFIG_DRM_MGAG200) += mgag200/
> >>>  obj-$(CONFIG_DRM_V3D)  += v3d/
> >>>  obj-$(CONFIG_DRM_VC4)  += vc4/
> >>> -obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
> >>>  obj-$(CONFIG_DRM_SIS)   += sis/
> >>>  obj-$(CONFIG_DRM_SAVAGE)+= savage/
> >>>  obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
> >>> diff --git a/drivers/gpu/drm/cirrus/Kconfig b/drivers/gpu/drm/cirrus/Kconfig
> >>> deleted file mode 100644
> >>> index c6bbd988b0e5..000000000000
> >>> --- a/drivers/gpu/drm/cirrus/Kconfig
> >>> +++ /dev/null
> >>> @@ -1,19 +0,0 @@
> >>> -# SPDX-License-Identifier: GPL-2.0-only
> >>> -config DRM_CIRRUS_QEMU
> >>> -     tristate "Cirrus driver for QEMU emulated device"
> >>> -     depends on DRM && PCI && MMU
> >>> -     select DRM_KMS_HELPER
> >>> -     select DRM_GEM_SHMEM_HELPER
> >>> -     help
> >>> -      This is a KMS driver for emulated cirrus device in qemu.
> >>> -      It is *NOT* intended for real cirrus devices. This requires
> >>> -      the modesetting userspace X.org driver.
> >>> -
> >>> -      Cirrus is obsolete, the hardware was designed in the 90ies
> >>> -      and can't keep up with todays needs.  More background:
> >>> -      https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
> >>> -
> >>> -      Better alternatives are:
> >>> -        - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+)
> >>> -        - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
> >>> -        - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
> >>> diff --git a/drivers/gpu/drm/cirrus/Makefile b/drivers/gpu/drm/cirrus/Makefile
> >>> deleted file mode 100644
> >>> index 0c1ed3f99725..000000000000
> >>> --- a/drivers/gpu/drm/cirrus/Makefile
> >>> +++ /dev/null
> >>> @@ -1,2 +0,0 @@
> >>> -# SPDX-License-Identifier: GPL-2.0-only
> >>> -obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o
> >>> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> >>> index 4160e74e4751..2b6414f0fa75 100644
> >>> --- a/drivers/gpu/drm/tiny/Kconfig
> >>> +++ b/drivers/gpu/drm/tiny/Kconfig
> >>> @@ -1,5 +1,24 @@
> >>>  # SPDX-License-Identifier: GPL-2.0-only
> >>>
> >>> +config DRM_CIRRUS_QEMU
> >>> +     tristate "Cirrus driver for QEMU emulated device"
> >>> +     depends on DRM && PCI && MMU
> >>> +     select DRM_KMS_HELPER
> >>> +     select DRM_GEM_SHMEM_HELPER
> >>> +     help
> >>> +      This is a KMS driver for emulated cirrus device in qemu.
> >>> +      It is *NOT* intended for real cirrus devices. This requires
> >>> +      the modesetting userspace X.org driver.
> >>> +
> >>> +      Cirrus is obsolete, the hardware was designed in the 90ies
> >>> +      and can't keep up with todays needs.  More background:
> >>> +      https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
> >>> +
> >>> +      Better alternatives are:
> >>> +        - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+)
> >>> +        - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
> >>> +        - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
> >>> +
> >>>  config DRM_GM12U320
> >>>       tristate "GM12U320 driver for USB projectors"
> >>>       depends on DRM && USB
> >>> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> >>> index c96ceee71453..6ae4e9e5a35f 100644
> >>> --- a/drivers/gpu/drm/tiny/Makefile
> >>> +++ b/drivers/gpu/drm/tiny/Makefile
> >>> @@ -1,5 +1,6 @@
> >>>  # SPDX-License-Identifier: GPL-2.0-only
> >>>
> >>> +obj-$(CONFIG_DRM_CIRRUS_QEMU)                += cirrus.o
> >>>  obj-$(CONFIG_DRM_GM12U320)           += gm12u320.o
> >>>  obj-$(CONFIG_TINYDRM_HX8357D)                += hx8357d.o
> >>>  obj-$(CONFIG_TINYDRM_ILI9225)                += ili9225.o
> >>> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
> >>> similarity index 100%
> >>> rename from drivers/gpu/drm/cirrus/cirrus.c
> >>> rename to drivers/gpu/drm/tiny/cirrus.c
> >>>
> >>
> >> --
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Software Solutions Germany GmbH
> >> Maxfeldstr. 5, 90409 Nürnberg, Germany
> >> (HRB 36809, AG Nürnberg)
> >> Geschäftsführer: Felix Imendörffer
> >>
> >
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 37/59] drm/cirrus: Move to drm/tiny
  2020-04-15  7:40 ` [PATCH 37/59] drm/cirrus: Move to drm/tiny Daniel Vetter
  2020-04-15  8:01   ` Thomas Zimmermann
@ 2020-04-21  7:37   ` Gerd Hoffmann
  2020-04-24 16:37   ` Sam Ravnborg
  2 siblings, 0 replies; 19+ messages in thread
From: Gerd Hoffmann @ 2020-04-21  7:37 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, Intel Graphics Development, virtualization,
	DRI Development, Dave Airlie

On Wed, Apr 15, 2020 at 09:40:12AM +0200, Daniel Vetter wrote:
> Because it is.

Indeed.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd

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

* Re: [PATCH 59/59] drm/bochs: Remove explicit drm_connector_register
  2020-04-15  7:40 ` [PATCH 59/59] drm/bochs: Remove explicit drm_connector_register Daniel Vetter
@ 2020-04-21  7:39   ` Gerd Hoffmann
  2020-04-24 18:11   ` Sam Ravnborg
  1 sibling, 0 replies; 19+ messages in thread
From: Gerd Hoffmann @ 2020-04-21  7:39 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	virtualization

On Wed, Apr 15, 2020 at 09:40:34AM +0200, Daniel Vetter wrote:
> This is leftovers from the old drm_driver->load callback
> upside-down issues. It doesn't do anything for not-hotplugged
> connectors since drm_dev_register takes care of that.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: virtualization@lists.linux-foundation.org

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

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

* Re: [PATCH 26/59] drm/qxl: Use devm_drm_dev_alloc
  2020-04-15  7:40 ` [PATCH 26/59] drm/qxl: Use devm_drm_dev_alloc Daniel Vetter
@ 2020-04-24 15:09   ` Sam Ravnborg
  2020-04-28 14:00     ` Daniel Vetter
  0 siblings, 1 reply; 19+ messages in thread
From: Sam Ravnborg @ 2020-04-24 15:09 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Dave Airlie, Intel Graphics Development, DRI Development,
	virtualization, Gerd Hoffmann, spice-devel, Daniel Vetter

Hi Daniel

On Wed, Apr 15, 2020 at 09:40:01AM +0200, Daniel Vetter wrote:
> Also need to remove the drm_dev_put from the remove hook.
> 
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: spice-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/qxl/qxl_drv.c | 15 ++++++++-------
>  drivers/gpu/drm/qxl/qxl_drv.h |  3 +--
>  drivers/gpu/drm/qxl/qxl_kms.c | 12 +-----------
>  3 files changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 09102e2efabc..6b4ae4c5fb76 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		return -EINVAL; /* TODO: ENODEV ? */
>  	}
>  
> -	qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
> -	if (!qdev)
> +	qdev = devm_drm_dev_alloc(&pdev->dev, &qxl_driver,
> +				  struct qxl_device, ddev);
> +	if (IS_ERR(qdev)) {
> +		pr_err("Unable to init drm dev");
>  		return -ENOMEM;
> +	}

The other patches do not add any error message when devm_drm_dev_alloc()
fails and driver core will log that driver init failed.

So the pr_err() above should be dropped.
I know it comes from qxl_device_init() but that does not make it a good
idea.

With this fixed:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

>  
>  	ret = pci_enable_device(pdev);
>  	if (ret)
> -		goto free_dev;
> +		return ret;
>  
>  	ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "qxl");
>  	if (ret)
> @@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		}
>  	}
>  
> -	ret = qxl_device_init(qdev, &qxl_driver, pdev);
> +	ret = qxl_device_init(qdev, pdev);
>  	if (ret)
>  		goto put_vga;
>  
> @@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		vga_put(pdev, VGA_RSRC_LEGACY_IO);
>  disable_pci:
>  	pci_disable_device(pdev);
> -free_dev:
> -	kfree(qdev);
> +
>  	return ret;
>  }
>  
> @@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
>  	drm_atomic_helper_shutdown(dev);
>  	if (is_vga(pdev))
>  		vga_put(pdev, VGA_RSRC_LEGACY_IO);
> -	drm_dev_put(dev);
>  }
>  
>  DEFINE_DRM_GEM_FOPS(qxl_fops);
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> index 435126facc9b..86ac191d9205 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.h
> +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> @@ -276,8 +276,7 @@ struct qxl_device {
>  extern const struct drm_ioctl_desc qxl_ioctls[];
>  extern int qxl_max_ioctl;
>  
> -int qxl_device_init(struct qxl_device *qdev, struct drm_driver *drv,
> -		    struct pci_dev *pdev);
> +int qxl_device_init(struct qxl_device *qdev, struct pci_dev *pdev);
>  void qxl_device_fini(struct qxl_device *qdev);
>  
>  int qxl_modeset_init(struct qxl_device *qdev);
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index 9eed1a375f24..91a34dd835d7 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -108,21 +108,13 @@ static void qxl_gc_work(struct work_struct *work)
>  }
>  
>  int qxl_device_init(struct qxl_device *qdev,
> -		    struct drm_driver *drv,
>  		    struct pci_dev *pdev)
>  {
>  	int r, sb;
>  
> -	r = drm_dev_init(&qdev->ddev, drv, &pdev->dev);
> -	if (r) {
> -		pr_err("Unable to init drm dev");
> -		goto error;
> -	}
> -
>  	qdev->ddev.pdev = pdev;
>  	pci_set_drvdata(pdev, &qdev->ddev);
>  	qdev->ddev.dev_private = qdev;
> -	drmm_add_final_kfree(&qdev->ddev, qdev);
>  
>  	mutex_init(&qdev->gem.mutex);
>  	mutex_init(&qdev->update_area_mutex);
> @@ -138,8 +130,7 @@ int qxl_device_init(struct qxl_device *qdev,
>  	qdev->vram_mapping = io_mapping_create_wc(qdev->vram_base, pci_resource_len(pdev, 0));
>  	if (!qdev->vram_mapping) {
>  		pr_err("Unable to create vram_mapping");
> -		r = -ENOMEM;
> -		goto error;
> +		return -ENOMEM;
>  	}
>  
>  	if (pci_resource_len(pdev, 4) > 0) {
> @@ -293,7 +284,6 @@ int qxl_device_init(struct qxl_device *qdev,
>  	io_mapping_free(qdev->surface_mapping);
>  vram_mapping_free:
>  	io_mapping_free(qdev->vram_mapping);
> -error:
>  	return r;
>  }
>  
> -- 
> 2.25.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 27/59] drm/qxl: Don't use drm_device->dev_private
  2020-04-15  7:40 ` [PATCH 27/59] drm/qxl: Don't use drm_device->dev_private Daniel Vetter
@ 2020-04-24 15:12   ` Sam Ravnborg
  0 siblings, 0 replies; 19+ messages in thread
From: Sam Ravnborg @ 2020-04-24 15:12 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Dave Airlie, Intel Graphics Development, DRI Development,
	virtualization, Gerd Hoffmann, spice-devel, Daniel Vetter

On Wed, Apr 15, 2020 at 09:40:02AM +0200, Daniel Vetter wrote:
> Upcasting using a container_of macro is more typesafe, faster and
> easier for the compiler to optimize.
> 
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: spice-devel@lists.freedesktop.org
Acked-by: Sam Ravnborg <sam@ravnborg.org>

> ---
>  drivers/gpu/drm/qxl/qxl_debugfs.c |  7 +++----
>  drivers/gpu/drm/qxl/qxl_display.c | 32 +++++++++++++++----------------
>  drivers/gpu/drm/qxl/qxl_drv.c     |  8 ++++----
>  drivers/gpu/drm/qxl/qxl_drv.h     |  4 ++--
>  drivers/gpu/drm/qxl/qxl_dumb.c    |  2 +-
>  drivers/gpu/drm/qxl/qxl_gem.c     |  2 +-
>  drivers/gpu/drm/qxl/qxl_ioctl.c   | 14 +++++++-------
>  drivers/gpu/drm/qxl/qxl_irq.c     |  2 +-
>  drivers/gpu/drm/qxl/qxl_kms.c     |  1 -
>  drivers/gpu/drm/qxl/qxl_object.c  |  2 +-
>  drivers/gpu/drm/qxl/qxl_release.c |  2 +-
>  drivers/gpu/drm/qxl/qxl_ttm.c     |  2 +-
>  12 files changed, 38 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c b/drivers/gpu/drm/qxl/qxl_debugfs.c
> index 88123047fdd4..524d35b648d8 100644
> --- a/drivers/gpu/drm/qxl/qxl_debugfs.c
> +++ b/drivers/gpu/drm/qxl/qxl_debugfs.c
> @@ -39,7 +39,7 @@ static int
>  qxl_debugfs_irq_received(struct seq_file *m, void *data)
>  {
>  	struct drm_info_node *node = (struct drm_info_node *) m->private;
> -	struct qxl_device *qdev = node->minor->dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(node->minor->dev);
>  
>  	seq_printf(m, "%d\n", atomic_read(&qdev->irq_received));
>  	seq_printf(m, "%d\n", atomic_read(&qdev->irq_received_display));
> @@ -53,7 +53,7 @@ static int
>  qxl_debugfs_buffers_info(struct seq_file *m, void *data)
>  {
>  	struct drm_info_node *node = (struct drm_info_node *) m->private;
> -	struct qxl_device *qdev = node->minor->dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(node->minor->dev);
>  	struct qxl_bo *bo;
>  
>  	list_for_each_entry(bo, &qdev->gem.objects, list) {
> @@ -83,8 +83,7 @@ void
>  qxl_debugfs_init(struct drm_minor *minor)
>  {
>  #if defined(CONFIG_DEBUG_FS)
> -	struct qxl_device *dev =
> -		(struct qxl_device *) minor->dev->dev_private;
> +	struct qxl_device *dev = to_qxl(minor->dev);
>  
>  	drm_debugfs_create_files(qxl_debugfs_list, QXL_DEBUGFS_ENTRIES,
>  				 minor->debugfs_root, minor);
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
> index 09583a08e141..1082cd5d2fd4 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -221,7 +221,7 @@ static int qxl_add_mode(struct drm_connector *connector,
>  			bool preferred)
>  {
>  	struct drm_device *dev = connector->dev;
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct drm_display_mode *mode = NULL;
>  	int rc;
>  
> @@ -242,7 +242,7 @@ static int qxl_add_mode(struct drm_connector *connector,
>  static int qxl_add_monitors_config_modes(struct drm_connector *connector)
>  {
>  	struct drm_device *dev = connector->dev;
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct qxl_output *output = drm_connector_to_qxl_output(connector);
>  	int h = output->index;
>  	struct qxl_head *head;
> @@ -310,7 +310,7 @@ static void qxl_crtc_update_monitors_config(struct drm_crtc *crtc,
>  					    const char *reason)
>  {
>  	struct drm_device *dev = crtc->dev;
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct qxl_crtc *qcrtc = to_qxl_crtc(crtc);
>  	struct qxl_head head;
>  	int oldcount, i = qcrtc->index;
> @@ -400,7 +400,7 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
>  					 unsigned int num_clips)
>  {
>  	/* TODO: vmwgfx where this was cribbed from had locking. Why? */
> -	struct qxl_device *qdev = fb->dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(fb->dev);
>  	struct drm_clip_rect norect;
>  	struct qxl_bo *qobj;
>  	bool is_primary;
> @@ -462,7 +462,7 @@ static const struct drm_crtc_helper_funcs qxl_crtc_helper_funcs = {
>  static int qxl_primary_atomic_check(struct drm_plane *plane,
>  				    struct drm_plane_state *state)
>  {
> -	struct qxl_device *qdev = plane->dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(plane->dev);
>  	struct qxl_bo *bo;
>  
>  	if (!state->crtc || !state->fb)
> @@ -476,7 +476,7 @@ static int qxl_primary_atomic_check(struct drm_plane *plane,
>  static int qxl_primary_apply_cursor(struct drm_plane *plane)
>  {
>  	struct drm_device *dev = plane->dev;
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct drm_framebuffer *fb = plane->state->fb;
>  	struct qxl_crtc *qcrtc = to_qxl_crtc(plane->state->crtc);
>  	struct qxl_cursor_cmd *cmd;
> @@ -523,7 +523,7 @@ static int qxl_primary_apply_cursor(struct drm_plane *plane)
>  static void qxl_primary_atomic_update(struct drm_plane *plane,
>  				      struct drm_plane_state *old_state)
>  {
> -	struct qxl_device *qdev = plane->dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(plane->dev);
>  	struct qxl_bo *bo = gem_to_qxl_bo(plane->state->fb->obj[0]);
>  	struct qxl_bo *primary;
>  	struct drm_clip_rect norect = {
> @@ -554,7 +554,7 @@ static void qxl_primary_atomic_update(struct drm_plane *plane,
>  static void qxl_primary_atomic_disable(struct drm_plane *plane,
>  				       struct drm_plane_state *old_state)
>  {
> -	struct qxl_device *qdev = plane->dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(plane->dev);
>  
>  	if (old_state->fb) {
>  		struct qxl_bo *bo = gem_to_qxl_bo(old_state->fb->obj[0]);
> @@ -570,7 +570,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,
>  				     struct drm_plane_state *old_state)
>  {
>  	struct drm_device *dev = plane->dev;
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct drm_framebuffer *fb = plane->state->fb;
>  	struct qxl_crtc *qcrtc = to_qxl_crtc(plane->state->crtc);
>  	struct qxl_release *release;
> @@ -679,7 +679,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,
>  static void qxl_cursor_atomic_disable(struct drm_plane *plane,
>  				      struct drm_plane_state *old_state)
>  {
> -	struct qxl_device *qdev = plane->dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(plane->dev);
>  	struct qxl_release *release;
>  	struct qxl_cursor_cmd *cmd;
>  	int ret;
> @@ -762,7 +762,7 @@ static void qxl_calc_dumb_shadow(struct qxl_device *qdev,
>  static int qxl_plane_prepare_fb(struct drm_plane *plane,
>  				struct drm_plane_state *new_state)
>  {
> -	struct qxl_device *qdev = plane->dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(plane->dev);
>  	struct drm_gem_object *obj;
>  	struct qxl_bo *user_bo;
>  	struct qxl_surface surf;
> @@ -923,7 +923,7 @@ static int qdev_crtc_init(struct drm_device *dev, int crtc_id)
>  {
>  	struct qxl_crtc *qxl_crtc;
>  	struct drm_plane *primary, *cursor;
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	int r;
>  
>  	qxl_crtc = kzalloc(sizeof(struct qxl_crtc), GFP_KERNEL);
> @@ -965,7 +965,7 @@ static int qdev_crtc_init(struct drm_device *dev, int crtc_id)
>  static int qxl_conn_get_modes(struct drm_connector *connector)
>  {
>  	struct drm_device *dev = connector->dev;
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct qxl_output *output = drm_connector_to_qxl_output(connector);
>  	unsigned int pwidth = 1024;
>  	unsigned int pheight = 768;
> @@ -991,7 +991,7 @@ static enum drm_mode_status qxl_conn_mode_valid(struct drm_connector *connector,
>  			       struct drm_display_mode *mode)
>  {
>  	struct drm_device *ddev = connector->dev;
> -	struct qxl_device *qdev = ddev->dev_private;
> +	struct qxl_device *qdev = to_qxl(ddev);
>  
>  	if (qxl_check_mode(qdev, mode->hdisplay, mode->vdisplay) != 0)
>  		return MODE_BAD;
> @@ -1021,7 +1021,7 @@ static enum drm_connector_status qxl_conn_detect(
>  	struct qxl_output *output =
>  		drm_connector_to_qxl_output(connector);
>  	struct drm_device *ddev = connector->dev;
> -	struct qxl_device *qdev = ddev->dev_private;
> +	struct qxl_device *qdev = to_qxl(ddev);
>  	bool connected = false;
>  
>  	/* The first monitor is always connected */
> @@ -1071,7 +1071,7 @@ static int qxl_mode_create_hotplug_mode_update_property(struct qxl_device *qdev)
>  
>  static int qdev_output_init(struct drm_device *dev, int num_output)
>  {
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct qxl_output *qxl_output;
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 6b4ae4c5fb76..13872b882775 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -137,7 +137,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  static void qxl_drm_release(struct drm_device *dev)
>  {
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  
>  	/*
>  	 * TODO: qxl_device_fini() call should be in qxl_pci_remove(),
> @@ -164,7 +164,7 @@ DEFINE_DRM_GEM_FOPS(qxl_fops);
>  static int qxl_drm_freeze(struct drm_device *dev)
>  {
>  	struct pci_dev *pdev = dev->pdev;
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	int ret;
>  
>  	ret = drm_mode_config_helper_suspend(dev);
> @@ -186,7 +186,7 @@ static int qxl_drm_freeze(struct drm_device *dev)
>  
>  static int qxl_drm_resume(struct drm_device *dev, bool thaw)
>  {
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  
>  	qdev->ram_header->int_mask = QXL_INTERRUPT_MASK;
>  	if (!thaw) {
> @@ -245,7 +245,7 @@ static int qxl_pm_restore(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct drm_device *drm_dev = pci_get_drvdata(pdev);
> -	struct qxl_device *qdev = drm_dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(drm_dev);
>  
>  	qxl_io_reset(qdev);
>  	return qxl_drm_resume(drm_dev, false);
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> index 86ac191d9205..31e35f787df2 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.h
> +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> @@ -192,8 +192,6 @@ struct qxl_debugfs {
>  
>  int qxl_debugfs_fence_init(struct qxl_device *rdev);
>  
> -struct qxl_device;
> -
>  struct qxl_device {
>  	struct drm_device ddev;
>  
> @@ -273,6 +271,8 @@ struct qxl_device {
>  	int monitors_config_height;
>  };
>  
> +#define to_qxl(dev) container_of(dev, struct qxl_device, ddev)
> +
>  extern const struct drm_ioctl_desc qxl_ioctls[];
>  extern int qxl_max_ioctl;
>  
> diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c
> index 272d19b677d8..24e903383aa1 100644
> --- a/drivers/gpu/drm/qxl/qxl_dumb.c
> +++ b/drivers/gpu/drm/qxl/qxl_dumb.c
> @@ -32,7 +32,7 @@ int qxl_mode_dumb_create(struct drm_file *file_priv,
>  			    struct drm_device *dev,
>  			    struct drm_mode_create_dumb *args)
>  {
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct qxl_bo *qobj;
>  	uint32_t handle;
>  	int r;
> diff --git a/drivers/gpu/drm/qxl/qxl_gem.c b/drivers/gpu/drm/qxl/qxl_gem.c
> index 69f37db1027a..5ff6fa9b799c 100644
> --- a/drivers/gpu/drm/qxl/qxl_gem.c
> +++ b/drivers/gpu/drm/qxl/qxl_gem.c
> @@ -34,7 +34,7 @@ void qxl_gem_object_free(struct drm_gem_object *gobj)
>  	struct qxl_device *qdev;
>  	struct ttm_buffer_object *tbo;
>  
> -	qdev = (struct qxl_device *)gobj->dev->dev_private;
> +	qdev = to_qxl(gobj->dev);
>  
>  	qxl_surface_evict(qdev, qobj, false);
>  
> diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
> index 8117a45b3610..d9a583966949 100644
> --- a/drivers/gpu/drm/qxl/qxl_ioctl.c
> +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
> @@ -36,7 +36,7 @@
>  static int qxl_alloc_ioctl(struct drm_device *dev, void *data,
>  			   struct drm_file *file_priv)
>  {
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct drm_qxl_alloc *qxl_alloc = data;
>  	int ret;
>  	struct qxl_bo *qobj;
> @@ -64,7 +64,7 @@ static int qxl_alloc_ioctl(struct drm_device *dev, void *data,
>  static int qxl_map_ioctl(struct drm_device *dev, void *data,
>  			 struct drm_file *file_priv)
>  {
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct drm_qxl_map *qxl_map = data;
>  
>  	return qxl_mode_dumb_mmap(file_priv, &qdev->ddev, qxl_map->handle,
> @@ -279,7 +279,7 @@ static int qxl_process_single_command(struct qxl_device *qdev,
>  static int qxl_execbuffer_ioctl(struct drm_device *dev, void *data,
>  				struct drm_file *file_priv)
>  {
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct drm_qxl_execbuffer *execbuffer = data;
>  	struct drm_qxl_command user_cmd;
>  	int cmd_num;
> @@ -304,7 +304,7 @@ static int qxl_execbuffer_ioctl(struct drm_device *dev, void *data,
>  static int qxl_update_area_ioctl(struct drm_device *dev, void *data,
>  				 struct drm_file *file)
>  {
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct drm_qxl_update_area *update_area = data;
>  	struct qxl_rect area = {.left = update_area->left,
>  				.top = update_area->top,
> @@ -354,7 +354,7 @@ static int qxl_update_area_ioctl(struct drm_device *dev, void *data,
>  static int qxl_getparam_ioctl(struct drm_device *dev, void *data,
>  		       struct drm_file *file_priv)
>  {
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct drm_qxl_getparam *param = data;
>  
>  	switch (param->param) {
> @@ -373,7 +373,7 @@ static int qxl_getparam_ioctl(struct drm_device *dev, void *data,
>  static int qxl_clientcap_ioctl(struct drm_device *dev, void *data,
>  				  struct drm_file *file_priv)
>  {
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct drm_qxl_clientcap *param = data;
>  	int byte, idx;
>  
> @@ -394,7 +394,7 @@ static int qxl_clientcap_ioctl(struct drm_device *dev, void *data,
>  static int qxl_alloc_surf_ioctl(struct drm_device *dev, void *data,
>  				struct drm_file *file)
>  {
> -	struct qxl_device *qdev = dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	struct drm_qxl_alloc_surf *param = data;
>  	struct qxl_bo *qobj;
>  	int handle;
> diff --git a/drivers/gpu/drm/qxl/qxl_irq.c b/drivers/gpu/drm/qxl/qxl_irq.c
> index 8435af108632..1ba5a702d763 100644
> --- a/drivers/gpu/drm/qxl/qxl_irq.c
> +++ b/drivers/gpu/drm/qxl/qxl_irq.c
> @@ -32,7 +32,7 @@
>  irqreturn_t qxl_irq_handler(int irq, void *arg)
>  {
>  	struct drm_device *dev = (struct drm_device *) arg;
> -	struct qxl_device *qdev = (struct qxl_device *)dev->dev_private;
> +	struct qxl_device *qdev = to_qxl(dev);
>  	uint32_t pending;
>  
>  	pending = xchg(&qdev->ram_header->int_pending, 0);
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index 91a34dd835d7..a6d873052cd4 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -114,7 +114,6 @@ int qxl_device_init(struct qxl_device *qdev,
>  
>  	qdev->ddev.pdev = pdev;
>  	pci_set_drvdata(pdev, &qdev->ddev);
> -	qdev->ddev.dev_private = qdev;
>  
>  	mutex_init(&qdev->gem.mutex);
>  	mutex_init(&qdev->update_area_mutex);
> diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
> index ab72dc3476e9..edc8a9916872 100644
> --- a/drivers/gpu/drm/qxl/qxl_object.c
> +++ b/drivers/gpu/drm/qxl/qxl_object.c
> @@ -33,7 +33,7 @@ static void qxl_ttm_bo_destroy(struct ttm_buffer_object *tbo)
>  	struct qxl_device *qdev;
>  
>  	bo = to_qxl_bo(tbo);
> -	qdev = (struct qxl_device *)bo->tbo.base.dev->dev_private;
> +	qdev = to_qxl(bo->tbo.base.dev);
>  
>  	qxl_surface_evict(qdev, bo, false);
>  	WARN_ON_ONCE(bo->map_count > 0);
> diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
> index 2feca734c7b1..4fae3e393da1 100644
> --- a/drivers/gpu/drm/qxl/qxl_release.c
> +++ b/drivers/gpu/drm/qxl/qxl_release.c
> @@ -243,7 +243,7 @@ static int qxl_release_validate_bo(struct qxl_bo *bo)
>  		return ret;
>  
>  	/* allocate a surface for reserved + validated buffers */
> -	ret = qxl_bo_check_id(bo->tbo.base.dev->dev_private, bo);
> +	ret = qxl_bo_check_id(to_qxl(bo->tbo.base.dev), bo);
>  	if (ret)
>  		return ret;
>  	return 0;
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index 93a2eb14844b..f09a712b1ed2 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -243,7 +243,7 @@ static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
>  	if (!qxl_ttm_bo_is_qxl_bo(bo))
>  		return;
>  	qbo = to_qxl_bo(bo);
> -	qdev = qbo->tbo.base.dev->dev_private;
> +	qdev = to_qxl(qbo->tbo.base.dev);
>  
>  	if (bo->mem.mem_type == TTM_PL_PRIV && qbo->surface_id)
>  		qxl_surface_evict(qdev, qbo, new_mem ? true : false);
> -- 
> 2.25.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 37/59] drm/cirrus: Move to drm/tiny
  2020-04-15  7:40 ` [PATCH 37/59] drm/cirrus: Move to drm/tiny Daniel Vetter
  2020-04-15  8:01   ` Thomas Zimmermann
  2020-04-21  7:37   ` Gerd Hoffmann
@ 2020-04-24 16:37   ` Sam Ravnborg
  2 siblings, 0 replies; 19+ messages in thread
From: Sam Ravnborg @ 2020-04-24 16:37 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, DRI Development, virtualization,
	Gerd Hoffmann, Dave Airlie, Daniel Vetter

On Wed, Apr 15, 2020 at 09:40:12AM +0200, Daniel Vetter wrote:
> Because it is. Huge congrats to everyone who made this kind of
> refactoring happen!
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>  MAINTAINERS                               |  2 +-
>  drivers/gpu/drm/Kconfig                   |  2 --
>  drivers/gpu/drm/Makefile                  |  1 -
>  drivers/gpu/drm/cirrus/Kconfig            | 19 -------------------
>  drivers/gpu/drm/cirrus/Makefile           |  2 --
>  drivers/gpu/drm/tiny/Kconfig              | 19 +++++++++++++++++++
>  drivers/gpu/drm/tiny/Makefile             |  1 +
>  drivers/gpu/drm/{cirrus => tiny}/cirrus.c |  0
>  8 files changed, 21 insertions(+), 25 deletions(-)
>  delete mode 100644 drivers/gpu/drm/cirrus/Kconfig
>  delete mode 100644 drivers/gpu/drm/cirrus/Makefile
>  rename drivers/gpu/drm/{cirrus => tiny}/cirrus.c (100%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7b3255d96d1d..0a5cf105ee37 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5397,7 +5397,7 @@ L:	virtualization@lists.linux-foundation.org
>  S:	Obsolete
>  W:	https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
>  T:	git git://anongit.freedesktop.org/drm/drm-misc
> -F:	drivers/gpu/drm/cirrus/
> +F:	drivers/gpu/drm/tiny/cirrus.c
>  
>  DRM DRIVER FOR QXL VIRTUAL GPU
>  M:	Dave Airlie <airlied@redhat.com>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 43594978958e..4f4e7fa001c1 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -310,8 +310,6 @@ source "drivers/gpu/drm/ast/Kconfig"
>  
>  source "drivers/gpu/drm/mgag200/Kconfig"
>  
> -source "drivers/gpu/drm/cirrus/Kconfig"
> -
>  source "drivers/gpu/drm/armada/Kconfig"
>  
>  source "drivers/gpu/drm/atmel-hlcdc/Kconfig"
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index f34d08c83485..2c0e5a7e5953 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -74,7 +74,6 @@ obj-$(CONFIG_DRM_I915)	+= i915/
>  obj-$(CONFIG_DRM_MGAG200) += mgag200/
>  obj-$(CONFIG_DRM_V3D)  += v3d/
>  obj-$(CONFIG_DRM_VC4)  += vc4/
> -obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
>  obj-$(CONFIG_DRM_SIS)   += sis/
>  obj-$(CONFIG_DRM_SAVAGE)+= savage/
>  obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
> diff --git a/drivers/gpu/drm/cirrus/Kconfig b/drivers/gpu/drm/cirrus/Kconfig
> deleted file mode 100644
> index c6bbd988b0e5..000000000000
> --- a/drivers/gpu/drm/cirrus/Kconfig
> +++ /dev/null
> @@ -1,19 +0,0 @@
> -# SPDX-License-Identifier: GPL-2.0-only
> -config DRM_CIRRUS_QEMU
> -	tristate "Cirrus driver for QEMU emulated device"
> -	depends on DRM && PCI && MMU
> -	select DRM_KMS_HELPER
> -	select DRM_GEM_SHMEM_HELPER
> -	help
> -	 This is a KMS driver for emulated cirrus device in qemu.
> -	 It is *NOT* intended for real cirrus devices. This requires
> -	 the modesetting userspace X.org driver.
> -
> -	 Cirrus is obsolete, the hardware was designed in the 90ies
> -	 and can't keep up with todays needs.  More background:
> -	 https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
> -
> -	 Better alternatives are:
> -	   - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+)
> -	   - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
> -	   - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
> diff --git a/drivers/gpu/drm/cirrus/Makefile b/drivers/gpu/drm/cirrus/Makefile
> deleted file mode 100644
> index 0c1ed3f99725..000000000000
> --- a/drivers/gpu/drm/cirrus/Makefile
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -# SPDX-License-Identifier: GPL-2.0-only
> -obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index 4160e74e4751..2b6414f0fa75 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -1,5 +1,24 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  
> +config DRM_CIRRUS_QEMU
> +	tristate "Cirrus driver for QEMU emulated device"
> +	depends on DRM && PCI && MMU
> +	select DRM_KMS_HELPER
> +	select DRM_GEM_SHMEM_HELPER
> +	help
> +	 This is a KMS driver for emulated cirrus device in qemu.
> +	 It is *NOT* intended for real cirrus devices. This requires
> +	 the modesetting userspace X.org driver.
> +
> +	 Cirrus is obsolete, the hardware was designed in the 90ies
> +	 and can't keep up with todays needs.  More background:
> +	 https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
> +
> +	 Better alternatives are:
> +	   - stdvga (DRM_BOCHS, qemu -vga std, default in qemu 2.2+)
> +	   - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
> +	   - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
> +
>  config DRM_GM12U320
>  	tristate "GM12U320 driver for USB projectors"
>  	depends on DRM && USB
> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> index c96ceee71453..6ae4e9e5a35f 100644
> --- a/drivers/gpu/drm/tiny/Makefile
> +++ b/drivers/gpu/drm/tiny/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  
> +obj-$(CONFIG_DRM_CIRRUS_QEMU)		+= cirrus.o
>  obj-$(CONFIG_DRM_GM12U320)		+= gm12u320.o
>  obj-$(CONFIG_TINYDRM_HX8357D)		+= hx8357d.o
>  obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
> similarity index 100%
> rename from drivers/gpu/drm/cirrus/cirrus.c
> rename to drivers/gpu/drm/tiny/cirrus.c
> -- 
> 2.25.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 59/59] drm/bochs: Remove explicit drm_connector_register
  2020-04-15  7:40 ` [PATCH 59/59] drm/bochs: Remove explicit drm_connector_register Daniel Vetter
  2020-04-21  7:39   ` Gerd Hoffmann
@ 2020-04-24 18:11   ` Sam Ravnborg
  1 sibling, 0 replies; 19+ messages in thread
From: Sam Ravnborg @ 2020-04-24 18:11 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, Intel Graphics Development, Gerd Hoffmann,
	DRI Development, virtualization

On Wed, Apr 15, 2020 at 09:40:34AM +0200, Daniel Vetter wrote:
> This is leftovers from the old drm_driver->load callback
> upside-down issues. It doesn't do anything for not-hotplugged
> connectors since drm_dev_register takes care of that.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>  drivers/gpu/drm/bochs/bochs_kms.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
> index 7f4bcfad87e9..05d8373888e8 100644
> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> @@ -104,7 +104,6 @@ static void bochs_connector_init(struct drm_device *dev)
>  			   DRM_MODE_CONNECTOR_VIRTUAL);
>  	drm_connector_helper_add(connector,
>  				 &bochs_connector_connector_helper_funcs);
> -	drm_connector_register(connector);
>  
>  	bochs_hw_load_edid(bochs);
>  	if (bochs->edid) {
> -- 
> 2.25.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 26/59] drm/qxl: Use devm_drm_dev_alloc
  2020-04-24 15:09   ` Sam Ravnborg
@ 2020-04-28 14:00     ` Daniel Vetter
  2020-04-28 17:00       ` Sam Ravnborg
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2020-04-28 14:00 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Dave Airlie, Daniel Vetter, Intel Graphics Development,
	DRI Development, virtualization, Gerd Hoffmann, Daniel Vetter,
	spice-devel

On Fri, Apr 24, 2020 at 05:09:11PM +0200, Sam Ravnborg wrote:
> Hi Daniel
> 
> On Wed, Apr 15, 2020 at 09:40:01AM +0200, Daniel Vetter wrote:
> > Also need to remove the drm_dev_put from the remove hook.
> > 
> > Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: spice-devel@lists.freedesktop.org
> > ---
> >  drivers/gpu/drm/qxl/qxl_drv.c | 15 ++++++++-------
> >  drivers/gpu/drm/qxl/qxl_drv.h |  3 +--
> >  drivers/gpu/drm/qxl/qxl_kms.c | 12 +-----------
> >  3 files changed, 10 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> > index 09102e2efabc..6b4ae4c5fb76 100644
> > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > @@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  		return -EINVAL; /* TODO: ENODEV ? */
> >  	}
> >  
> > -	qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
> > -	if (!qdev)
> > +	qdev = devm_drm_dev_alloc(&pdev->dev, &qxl_driver,
> > +				  struct qxl_device, ddev);
> > +	if (IS_ERR(qdev)) {
> > +		pr_err("Unable to init drm dev");
> >  		return -ENOMEM;
> > +	}
> 
> The other patches do not add any error message when devm_drm_dev_alloc()
> fails and driver core will log that driver init failed.
> 
> So the pr_err() above should be dropped.
> I know it comes from qxl_device_init() but that does not make it a good
> idea.

Hm I know we're inconsistent here, but some drivers have error logging on
all branches, some dont. I'm just trying to go with the prevailing style.

> With this fixed:

Insisting on this or ok as-is?
-Daniel

> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> 
> >  
> >  	ret = pci_enable_device(pdev);
> >  	if (ret)
> > -		goto free_dev;
> > +		return ret;
> >  
> >  	ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "qxl");
> >  	if (ret)
> > @@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  		}
> >  	}
> >  
> > -	ret = qxl_device_init(qdev, &qxl_driver, pdev);
> > +	ret = qxl_device_init(qdev, pdev);
> >  	if (ret)
> >  		goto put_vga;
> >  
> > @@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  		vga_put(pdev, VGA_RSRC_LEGACY_IO);
> >  disable_pci:
> >  	pci_disable_device(pdev);
> > -free_dev:
> > -	kfree(qdev);
> > +
> >  	return ret;
> >  }
> >  
> > @@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
> >  	drm_atomic_helper_shutdown(dev);
> >  	if (is_vga(pdev))
> >  		vga_put(pdev, VGA_RSRC_LEGACY_IO);
> > -	drm_dev_put(dev);
> >  }
> >  
> >  DEFINE_DRM_GEM_FOPS(qxl_fops);
> > diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> > index 435126facc9b..86ac191d9205 100644
> > --- a/drivers/gpu/drm/qxl/qxl_drv.h
> > +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> > @@ -276,8 +276,7 @@ struct qxl_device {
> >  extern const struct drm_ioctl_desc qxl_ioctls[];
> >  extern int qxl_max_ioctl;
> >  
> > -int qxl_device_init(struct qxl_device *qdev, struct drm_driver *drv,
> > -		    struct pci_dev *pdev);
> > +int qxl_device_init(struct qxl_device *qdev, struct pci_dev *pdev);
> >  void qxl_device_fini(struct qxl_device *qdev);
> >  
> >  int qxl_modeset_init(struct qxl_device *qdev);
> > diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> > index 9eed1a375f24..91a34dd835d7 100644
> > --- a/drivers/gpu/drm/qxl/qxl_kms.c
> > +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> > @@ -108,21 +108,13 @@ static void qxl_gc_work(struct work_struct *work)
> >  }
> >  
> >  int qxl_device_init(struct qxl_device *qdev,
> > -		    struct drm_driver *drv,
> >  		    struct pci_dev *pdev)
> >  {
> >  	int r, sb;
> >  
> > -	r = drm_dev_init(&qdev->ddev, drv, &pdev->dev);
> > -	if (r) {
> > -		pr_err("Unable to init drm dev");
> > -		goto error;
> > -	}
> > -
> >  	qdev->ddev.pdev = pdev;
> >  	pci_set_drvdata(pdev, &qdev->ddev);
> >  	qdev->ddev.dev_private = qdev;
> > -	drmm_add_final_kfree(&qdev->ddev, qdev);
> >  
> >  	mutex_init(&qdev->gem.mutex);
> >  	mutex_init(&qdev->update_area_mutex);
> > @@ -138,8 +130,7 @@ int qxl_device_init(struct qxl_device *qdev,
> >  	qdev->vram_mapping = io_mapping_create_wc(qdev->vram_base, pci_resource_len(pdev, 0));
> >  	if (!qdev->vram_mapping) {
> >  		pr_err("Unable to create vram_mapping");
> > -		r = -ENOMEM;
> > -		goto error;
> > +		return -ENOMEM;
> >  	}
> >  
> >  	if (pci_resource_len(pdev, 4) > 0) {
> > @@ -293,7 +284,6 @@ int qxl_device_init(struct qxl_device *qdev,
> >  	io_mapping_free(qdev->surface_mapping);
> >  vram_mapping_free:
> >  	io_mapping_free(qdev->vram_mapping);
> > -error:
> >  	return r;
> >  }
> >  
> > -- 
> > 2.25.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH 26/59] drm/qxl: Use devm_drm_dev_alloc
  2020-04-28 14:00     ` Daniel Vetter
@ 2020-04-28 17:00       ` Sam Ravnborg
  2020-04-28 18:04         ` Daniel Vetter
  0 siblings, 1 reply; 19+ messages in thread
From: Sam Ravnborg @ 2020-04-28 17:00 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Dave Airlie, Daniel Vetter, Intel Graphics Development,
	DRI Development, virtualization, Gerd Hoffmann, Daniel Vetter,
	spice-devel

On Tue, Apr 28, 2020 at 04:00:11PM +0200, Daniel Vetter wrote:
> On Fri, Apr 24, 2020 at 05:09:11PM +0200, Sam Ravnborg wrote:
> > Hi Daniel
> > 
> > On Wed, Apr 15, 2020 at 09:40:01AM +0200, Daniel Vetter wrote:
> > > Also need to remove the drm_dev_put from the remove hook.
> > > 
> > > Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Dave Airlie <airlied@redhat.com>
> > > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > > Cc: virtualization@lists.linux-foundation.org
> > > Cc: spice-devel@lists.freedesktop.org
> > > ---
> > >  drivers/gpu/drm/qxl/qxl_drv.c | 15 ++++++++-------
> > >  drivers/gpu/drm/qxl/qxl_drv.h |  3 +--
> > >  drivers/gpu/drm/qxl/qxl_kms.c | 12 +-----------
> > >  3 files changed, 10 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> > > index 09102e2efabc..6b4ae4c5fb76 100644
> > > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > > @@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >  		return -EINVAL; /* TODO: ENODEV ? */
> > >  	}
> > >  
> > > -	qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
> > > -	if (!qdev)
> > > +	qdev = devm_drm_dev_alloc(&pdev->dev, &qxl_driver,
> > > +				  struct qxl_device, ddev);
> > > +	if (IS_ERR(qdev)) {
> > > +		pr_err("Unable to init drm dev");
> > >  		return -ENOMEM;
> > > +	}
> > 
> > The other patches do not add any error message when devm_drm_dev_alloc()
> > fails and driver core will log that driver init failed.
> > 
> > So the pr_err() above should be dropped.
> > I know it comes from qxl_device_init() but that does not make it a good
> > idea.
> 
> Hm I know we're inconsistent here, but some drivers have error logging on
> all branches, some dont. I'm just trying to go with the prevailing style.
> 
> > With this fixed:
> 
> Insisting on this or ok as-is?
OK as-is.

	Sam

> -Daniel
> 
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > 
> > >  
> > >  	ret = pci_enable_device(pdev);
> > >  	if (ret)
> > > -		goto free_dev;
> > > +		return ret;
> > >  
> > >  	ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "qxl");
> > >  	if (ret)
> > > @@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >  		}
> > >  	}
> > >  
> > > -	ret = qxl_device_init(qdev, &qxl_driver, pdev);
> > > +	ret = qxl_device_init(qdev, pdev);
> > >  	if (ret)
> > >  		goto put_vga;
> > >  
> > > @@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >  		vga_put(pdev, VGA_RSRC_LEGACY_IO);
> > >  disable_pci:
> > >  	pci_disable_device(pdev);
> > > -free_dev:
> > > -	kfree(qdev);
> > > +
> > >  	return ret;
> > >  }
> > >  
> > > @@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
> > >  	drm_atomic_helper_shutdown(dev);
> > >  	if (is_vga(pdev))
> > >  		vga_put(pdev, VGA_RSRC_LEGACY_IO);
> > > -	drm_dev_put(dev);
> > >  }
> > >  
> > >  DEFINE_DRM_GEM_FOPS(qxl_fops);
> > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> > > index 435126facc9b..86ac191d9205 100644
> > > --- a/drivers/gpu/drm/qxl/qxl_drv.h
> > > +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> > > @@ -276,8 +276,7 @@ struct qxl_device {
> > >  extern const struct drm_ioctl_desc qxl_ioctls[];
> > >  extern int qxl_max_ioctl;
> > >  
> > > -int qxl_device_init(struct qxl_device *qdev, struct drm_driver *drv,
> > > -		    struct pci_dev *pdev);
> > > +int qxl_device_init(struct qxl_device *qdev, struct pci_dev *pdev);
> > >  void qxl_device_fini(struct qxl_device *qdev);
> > >  
> > >  int qxl_modeset_init(struct qxl_device *qdev);
> > > diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> > > index 9eed1a375f24..91a34dd835d7 100644
> > > --- a/drivers/gpu/drm/qxl/qxl_kms.c
> > > +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> > > @@ -108,21 +108,13 @@ static void qxl_gc_work(struct work_struct *work)
> > >  }
> > >  
> > >  int qxl_device_init(struct qxl_device *qdev,
> > > -		    struct drm_driver *drv,
> > >  		    struct pci_dev *pdev)
> > >  {
> > >  	int r, sb;
> > >  
> > > -	r = drm_dev_init(&qdev->ddev, drv, &pdev->dev);
> > > -	if (r) {
> > > -		pr_err("Unable to init drm dev");
> > > -		goto error;
> > > -	}
> > > -
> > >  	qdev->ddev.pdev = pdev;
> > >  	pci_set_drvdata(pdev, &qdev->ddev);
> > >  	qdev->ddev.dev_private = qdev;
> > > -	drmm_add_final_kfree(&qdev->ddev, qdev);
> > >  
> > >  	mutex_init(&qdev->gem.mutex);
> > >  	mutex_init(&qdev->update_area_mutex);
> > > @@ -138,8 +130,7 @@ int qxl_device_init(struct qxl_device *qdev,
> > >  	qdev->vram_mapping = io_mapping_create_wc(qdev->vram_base, pci_resource_len(pdev, 0));
> > >  	if (!qdev->vram_mapping) {
> > >  		pr_err("Unable to create vram_mapping");
> > > -		r = -ENOMEM;
> > > -		goto error;
> > > +		return -ENOMEM;
> > >  	}
> > >  
> > >  	if (pci_resource_len(pdev, 4) > 0) {
> > > @@ -293,7 +284,6 @@ int qxl_device_init(struct qxl_device *qdev,
> > >  	io_mapping_free(qdev->surface_mapping);
> > >  vram_mapping_free:
> > >  	io_mapping_free(qdev->vram_mapping);
> > > -error:
> > >  	return r;
> > >  }
> > >  
> > > -- 
> > > 2.25.1
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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

* Re: [PATCH 26/59] drm/qxl: Use devm_drm_dev_alloc
  2020-04-28 17:00       ` Sam Ravnborg
@ 2020-04-28 18:04         ` Daniel Vetter
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2020-04-28 18:04 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Dave Airlie, Daniel Vetter, Intel Graphics Development,
	DRI Development, virtualization, Gerd Hoffmann, Daniel Vetter,
	spice-devel

On Tue, Apr 28, 2020 at 07:00:26PM +0200, Sam Ravnborg wrote:
> On Tue, Apr 28, 2020 at 04:00:11PM +0200, Daniel Vetter wrote:
> > On Fri, Apr 24, 2020 at 05:09:11PM +0200, Sam Ravnborg wrote:
> > > Hi Daniel
> > > 
> > > On Wed, Apr 15, 2020 at 09:40:01AM +0200, Daniel Vetter wrote:
> > > > Also need to remove the drm_dev_put from the remove hook.
> > > > 
> > > > Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > Cc: Dave Airlie <airlied@redhat.com>
> > > > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > > > Cc: virtualization@lists.linux-foundation.org
> > > > Cc: spice-devel@lists.freedesktop.org
> > > > ---
> > > >  drivers/gpu/drm/qxl/qxl_drv.c | 15 ++++++++-------
> > > >  drivers/gpu/drm/qxl/qxl_drv.h |  3 +--
> > > >  drivers/gpu/drm/qxl/qxl_kms.c | 12 +-----------
> > > >  3 files changed, 10 insertions(+), 20 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> > > > index 09102e2efabc..6b4ae4c5fb76 100644
> > > > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > > > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > > > @@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > > >  		return -EINVAL; /* TODO: ENODEV ? */
> > > >  	}
> > > >  
> > > > -	qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
> > > > -	if (!qdev)
> > > > +	qdev = devm_drm_dev_alloc(&pdev->dev, &qxl_driver,
> > > > +				  struct qxl_device, ddev);
> > > > +	if (IS_ERR(qdev)) {
> > > > +		pr_err("Unable to init drm dev");
> > > >  		return -ENOMEM;
> > > > +	}
> > > 
> > > The other patches do not add any error message when devm_drm_dev_alloc()
> > > fails and driver core will log that driver init failed.
> > > 
> > > So the pr_err() above should be dropped.
> > > I know it comes from qxl_device_init() but that does not make it a good
> > > idea.
> > 
> > Hm I know we're inconsistent here, but some drivers have error logging on
> > all branches, some dont. I'm just trying to go with the prevailing style.
> > 
> > > With this fixed:
> > 
> > Insisting on this or ok as-is?
> OK as-is.

Ok, merged the two qxl patches too, plus a lot of the driver conversions.

Thanks a lot to everyone who commented, reviewed and tested thus far.
-Daniel

> 
> 	Sam
> 
> > -Daniel
> > 
> > > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > > 
> > > >  
> > > >  	ret = pci_enable_device(pdev);
> > > >  	if (ret)
> > > > -		goto free_dev;
> > > > +		return ret;
> > > >  
> > > >  	ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "qxl");
> > > >  	if (ret)
> > > > @@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > > >  		}
> > > >  	}
> > > >  
> > > > -	ret = qxl_device_init(qdev, &qxl_driver, pdev);
> > > > +	ret = qxl_device_init(qdev, pdev);
> > > >  	if (ret)
> > > >  		goto put_vga;
> > > >  
> > > > @@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > > >  		vga_put(pdev, VGA_RSRC_LEGACY_IO);
> > > >  disable_pci:
> > > >  	pci_disable_device(pdev);
> > > > -free_dev:
> > > > -	kfree(qdev);
> > > > +
> > > >  	return ret;
> > > >  }
> > > >  
> > > > @@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
> > > >  	drm_atomic_helper_shutdown(dev);
> > > >  	if (is_vga(pdev))
> > > >  		vga_put(pdev, VGA_RSRC_LEGACY_IO);
> > > > -	drm_dev_put(dev);
> > > >  }
> > > >  
> > > >  DEFINE_DRM_GEM_FOPS(qxl_fops);
> > > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> > > > index 435126facc9b..86ac191d9205 100644
> > > > --- a/drivers/gpu/drm/qxl/qxl_drv.h
> > > > +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> > > > @@ -276,8 +276,7 @@ struct qxl_device {
> > > >  extern const struct drm_ioctl_desc qxl_ioctls[];
> > > >  extern int qxl_max_ioctl;
> > > >  
> > > > -int qxl_device_init(struct qxl_device *qdev, struct drm_driver *drv,
> > > > -		    struct pci_dev *pdev);
> > > > +int qxl_device_init(struct qxl_device *qdev, struct pci_dev *pdev);
> > > >  void qxl_device_fini(struct qxl_device *qdev);
> > > >  
> > > >  int qxl_modeset_init(struct qxl_device *qdev);
> > > > diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> > > > index 9eed1a375f24..91a34dd835d7 100644
> > > > --- a/drivers/gpu/drm/qxl/qxl_kms.c
> > > > +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> > > > @@ -108,21 +108,13 @@ static void qxl_gc_work(struct work_struct *work)
> > > >  }
> > > >  
> > > >  int qxl_device_init(struct qxl_device *qdev,
> > > > -		    struct drm_driver *drv,
> > > >  		    struct pci_dev *pdev)
> > > >  {
> > > >  	int r, sb;
> > > >  
> > > > -	r = drm_dev_init(&qdev->ddev, drv, &pdev->dev);
> > > > -	if (r) {
> > > > -		pr_err("Unable to init drm dev");
> > > > -		goto error;
> > > > -	}
> > > > -
> > > >  	qdev->ddev.pdev = pdev;
> > > >  	pci_set_drvdata(pdev, &qdev->ddev);
> > > >  	qdev->ddev.dev_private = qdev;
> > > > -	drmm_add_final_kfree(&qdev->ddev, qdev);
> > > >  
> > > >  	mutex_init(&qdev->gem.mutex);
> > > >  	mutex_init(&qdev->update_area_mutex);
> > > > @@ -138,8 +130,7 @@ int qxl_device_init(struct qxl_device *qdev,
> > > >  	qdev->vram_mapping = io_mapping_create_wc(qdev->vram_base, pci_resource_len(pdev, 0));
> > > >  	if (!qdev->vram_mapping) {
> > > >  		pr_err("Unable to create vram_mapping");
> > > > -		r = -ENOMEM;
> > > > -		goto error;
> > > > +		return -ENOMEM;
> > > >  	}
> > > >  
> > > >  	if (pci_resource_len(pdev, 4) > 0) {
> > > > @@ -293,7 +284,6 @@ int qxl_device_init(struct qxl_device *qdev,
> > > >  	io_mapping_free(qdev->surface_mapping);
> > > >  vram_mapping_free:
> > > >  	io_mapping_free(qdev->vram_mapping);
> > > > -error:
> > > >  	return r;
> > > >  }
> > > >  
> > > > -- 
> > > > 2.25.1
> > > > 
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

end of thread, other threads:[~2020-04-28 18:04 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20200415074034.175360-1-daniel.vetter@ffwll.ch>
2020-04-15  7:40 ` [PATCH 26/59] drm/qxl: Use devm_drm_dev_alloc Daniel Vetter
2020-04-24 15:09   ` Sam Ravnborg
2020-04-28 14:00     ` Daniel Vetter
2020-04-28 17:00       ` Sam Ravnborg
2020-04-28 18:04         ` Daniel Vetter
2020-04-15  7:40 ` [PATCH 27/59] drm/qxl: Don't use drm_device->dev_private Daniel Vetter
2020-04-24 15:12   ` Sam Ravnborg
2020-04-15  7:40 ` [PATCH 35/59] drm/cirrus: Use devm_drm_dev_alloc Daniel Vetter
2020-04-15  7:40 ` [PATCH 36/59] drm/cirrus: Don't use drm_device->dev_private Daniel Vetter
2020-04-15  7:40 ` [PATCH 37/59] drm/cirrus: Move to drm/tiny Daniel Vetter
2020-04-15  8:01   ` Thomas Zimmermann
2020-04-15  8:19     ` Daniel Vetter
2020-04-15  8:46       ` Thomas Zimmermann
2020-04-15  9:31         ` Daniel Vetter
2020-04-21  7:37   ` Gerd Hoffmann
2020-04-24 16:37   ` Sam Ravnborg
2020-04-15  7:40 ` [PATCH 59/59] drm/bochs: Remove explicit drm_connector_register Daniel Vetter
2020-04-21  7:39   ` Gerd Hoffmann
2020-04-24 18:11   ` Sam Ravnborg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).