All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] exynosdrm: Remove .load/.unload midlayer
@ 2016-12-13 19:34 Laurent Pinchart
  2016-12-13 19:34 ` [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time Laurent Pinchart
                   ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-12-13 19:34 UTC (permalink / raw)
  To: dri-devel; +Cc: Kyungmin Park, Seung-Woo Kim

Hello,

Patch 1/2 removes the .load and .unload operations from the exynosdrm driver
and moves the code to the probe/remove handlers. As I don't have access to
exynos hardware the patch has been compile-tested only.

As an added bonus, patch 2/2 removes the now unused drm_platform midlayer. It
depends on the omapdrm and shmobild load/unload removal patches.

Laurent Pinchart (2):
  drm: exynos: Perform initialization/cleanup at probe/remove time
  drm: Remove unused drm_platform midlayer

 drivers/gpu/drm/Makefile                 |   2 +-
 drivers/gpu/drm/drm_platform.c           |  91 ------------
 drivers/gpu/drm/exynos/exynos_dp.c       |   1 -
 drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   1 -
 drivers/gpu/drm/exynos/exynos_drm_drv.c  | 245 ++++++++++++++++---------------
 drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   1 -
 drivers/gpu/drm/exynos/exynos_drm_vidi.c |   1 -
 drivers/gpu/drm/exynos/exynos_hdmi.c     |   1 -
 include/drm/drmP.h                       |   3 -
 9 files changed, 128 insertions(+), 218 deletions(-)
 delete mode 100644 drivers/gpu/drm/drm_platform.c

-- 
Regards,

Laurent Pinchart

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

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

* [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-13 19:34 [PATCH 0/2] exynosdrm: Remove .load/.unload midlayer Laurent Pinchart
@ 2016-12-13 19:34 ` Laurent Pinchart
  2016-12-13 21:03   ` Sean Paul
                     ` (4 more replies)
  2016-12-13 19:34 ` [PATCH 2/2] drm: Remove unused drm_platform midlayer Laurent Pinchart
  2016-12-17 22:39 ` [PATCH] drm: Remove the struct drm_device platformdev field Laurent Pinchart
  2 siblings, 5 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-12-13 19:34 UTC (permalink / raw)
  To: dri-devel; +Cc: Kyungmin Park, Seung-Woo Kim

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

The drm driver .load() operation is prone to race conditions as it
initializes the driver after registering the device nodes. Its usage is
deprecated, inline it in the probe function and call drm_dev_alloc() and
drm_dev_register() explicitly.

For consistency inline the .unload() handler in the remove function as
well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/exynos/exynos_dp.c       |   1 -
 drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   1 -
 drivers/gpu/drm/exynos/exynos_drm_drv.c  | 245 ++++++++++++++++---------------
 drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   1 -
 drivers/gpu/drm/exynos/exynos_drm_vidi.c |   1 -
 drivers/gpu/drm/exynos/exynos_hdmi.c     |   1 -
 6 files changed, 127 insertions(+), 123 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index 528229faffe4..b839f065f4b3 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -102,7 +102,6 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
 	struct drm_encoder *encoder = &dp->encoder;
 	int ret;
 
-	drm_connector_register(connector);
 	dp->connector = connector;
 
 	/* Pre-empt DP connector creation if there's a bridge */
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
index ad6b73c7fc59..3aab71a485ba 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
@@ -114,7 +114,6 @@ static int exynos_dpi_create_connector(struct drm_encoder *encoder)
 	}
 
 	drm_connector_helper_add(connector, &exynos_dpi_connector_helper_funcs);
-	drm_connector_register(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
 	return 0;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 739180ac3da5..bcd3d1db53eb 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -90,120 +90,6 @@ static void exynos_drm_atomic_work(struct work_struct *work)
 
 static struct device *exynos_drm_get_dma_device(void);
 
-static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
-{
-	struct exynos_drm_private *private;
-	struct drm_encoder *encoder;
-	unsigned int clone_mask;
-	int cnt, ret;
-
-	private = kzalloc(sizeof(struct exynos_drm_private), GFP_KERNEL);
-	if (!private)
-		return -ENOMEM;
-
-	init_waitqueue_head(&private->wait);
-	spin_lock_init(&private->lock);
-
-	dev_set_drvdata(dev->dev, dev);
-	dev->dev_private = (void *)private;
-
-	/* the first real CRTC device is used for all dma mapping operations */
-	private->dma_dev = exynos_drm_get_dma_device();
-	if (!private->dma_dev) {
-		DRM_ERROR("no device found for DMA mapping operations.\n");
-		ret = -ENODEV;
-		goto err_free_private;
-	}
-	DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
-		 dev_name(private->dma_dev));
-
-	/* create common IOMMU mapping for all devices attached to Exynos DRM */
-	ret = drm_create_iommu_mapping(dev);
-	if (ret < 0) {
-		DRM_ERROR("failed to create iommu mapping.\n");
-		goto err_free_private;
-	}
-
-	drm_mode_config_init(dev);
-
-	exynos_drm_mode_config_init(dev);
-
-	/* setup possible_clones. */
-	cnt = 0;
-	clone_mask = 0;
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
-		clone_mask |= (1 << (cnt++));
-
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
-		encoder->possible_clones = clone_mask;
-
-	platform_set_drvdata(dev->platformdev, dev);
-
-	/* Try to bind all sub drivers. */
-	ret = component_bind_all(dev->dev, dev);
-	if (ret)
-		goto err_mode_config_cleanup;
-
-	ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
-	if (ret)
-		goto err_unbind_all;
-
-	/* Probe non kms sub drivers and virtual display driver. */
-	ret = exynos_drm_device_subdrv_probe(dev);
-	if (ret)
-		goto err_cleanup_vblank;
-
-	drm_mode_config_reset(dev);
-
-	/*
-	 * enable drm irq mode.
-	 * - with irq_enabled = true, we can use the vblank feature.
-	 *
-	 * P.S. note that we wouldn't use drm irq handler but
-	 *	just specific driver own one instead because
-	 *	drm framework supports only one irq handler.
-	 */
-	dev->irq_enabled = true;
-
-	/* init kms poll for handling hpd */
-	drm_kms_helper_poll_init(dev);
-
-	/* force connectors detection */
-	drm_helper_hpd_irq_event(dev);
-
-	return 0;
-
-err_cleanup_vblank:
-	drm_vblank_cleanup(dev);
-err_unbind_all:
-	component_unbind_all(dev->dev, dev);
-err_mode_config_cleanup:
-	drm_mode_config_cleanup(dev);
-	drm_release_iommu_mapping(dev);
-err_free_private:
-	kfree(private);
-
-	return ret;
-}
-
-static int exynos_drm_unload(struct drm_device *dev)
-{
-	exynos_drm_device_subdrv_remove(dev);
-
-	exynos_drm_fbdev_fini(dev);
-	drm_kms_helper_poll_fini(dev);
-
-	drm_vblank_cleanup(dev);
-	component_unbind_all(dev->dev, dev);
-	drm_mode_config_cleanup(dev);
-	drm_release_iommu_mapping(dev);
-
-	kfree(dev->dev_private);
-	dev->dev_private = NULL;
-
-	return 0;
-}
-
 static int commit_is_pending(struct exynos_drm_private *priv, u32 crtcs)
 {
 	bool pending;
@@ -373,8 +259,6 @@ static const struct file_operations exynos_drm_driver_fops = {
 static struct drm_driver exynos_drm_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME
 				  | DRIVER_ATOMIC | DRIVER_RENDER,
-	.load			= exynos_drm_load,
-	.unload			= exynos_drm_unload,
 	.open			= exynos_drm_open,
 	.preclose		= exynos_drm_preclose,
 	.lastclose		= exynos_drm_lastclose,
@@ -552,12 +436,137 @@ static struct component_match *exynos_drm_match_add(struct device *dev)
 
 static int exynos_drm_bind(struct device *dev)
 {
-	return drm_platform_init(&exynos_drm_driver, to_platform_device(dev));
+	struct exynos_drm_private *private;
+	struct drm_encoder *encoder;
+	struct drm_device *drm;
+	unsigned int clone_mask;
+	int cnt, ret;
+
+	drm = drm_dev_alloc(&exynos_drm_driver, dev);
+	if (IS_ERR(drm))
+		return PTR_ERR(drm);
+
+	private = kzalloc(sizeof(struct exynos_drm_private), GFP_KERNEL);
+	if (!private) {
+		ret = -ENOMEM;
+		goto err_free_drm;
+	}
+
+	init_waitqueue_head(&private->wait);
+	spin_lock_init(&private->lock);
+
+	dev_set_drvdata(drm->dev, drm);
+	drm->dev_private = (void *)private;
+
+	/* the first real CRTC device is used for all dma mapping operations */
+	private->dma_dev = exynos_drm_get_dma_device();
+	if (!private->dma_dev) {
+		DRM_ERROR("no device found for DMA mapping operations.\n");
+		ret = -ENODEV;
+		goto err_free_private;
+	}
+	DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
+		 dev_name(private->dma_dev));
+
+	/* create common IOMMU mapping for all devices attached to Exynos DRM */
+	ret = drm_create_iommu_mapping(drm);
+	if (ret < 0) {
+		DRM_ERROR("failed to create iommu mapping.\n");
+		goto err_free_private;
+	}
+
+	drm_mode_config_init(drm);
+
+	exynos_drm_mode_config_init(drm);
+
+	/* setup possible_clones. */
+	cnt = 0;
+	clone_mask = 0;
+	list_for_each_entry(encoder, &drm->mode_config.encoder_list, head)
+		clone_mask |= (1 << (cnt++));
+
+	list_for_each_entry(encoder, &drm->mode_config.encoder_list, head)
+		encoder->possible_clones = clone_mask;
+
+	platform_set_drvdata(drm->platformdev, drm);
+
+	/* Try to bind all sub drivers. */
+	ret = component_bind_all(drm->dev, drm);
+	if (ret)
+		goto err_mode_config_cleanup;
+
+	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
+	if (ret)
+		goto err_unbind_all;
+
+	/* Probe non kms sub drivers and virtual display driver. */
+	ret = exynos_drm_device_subdrv_probe(drm);
+	if (ret)
+		goto err_cleanup_vblank;
+
+	drm_mode_config_reset(drm);
+
+	/*
+	 * enable drm irq mode.
+	 * - with irq_enabled = true, we can use the vblank feature.
+	 *
+	 * P.S. note that we wouldn't use drm irq handler but
+	 *	just specific driver own one instead because
+	 *	drm framework supports only one irq handler.
+	 */
+	drm->irq_enabled = true;
+
+	/* init kms poll for handling hpd */
+	drm_kms_helper_poll_init(drm);
+
+	/* force connectors detection */
+	drm_helper_hpd_irq_event(drm);
+
+	/* register the DRM device */
+	ret = drm_dev_register(drm, 0);
+	if (ret < 0)
+		goto err_cleanup_fbdev;
+
+	return 0;
+
+err_cleanup_fbdev:
+	exynos_drm_fbdev_fini(drm);
+	drm_kms_helper_poll_fini(drm);
+	exynos_drm_device_subdrv_remove(drm);
+err_cleanup_vblank:
+	drm_vblank_cleanup(drm);
+err_unbind_all:
+	component_unbind_all(drm->dev, drm);
+err_mode_config_cleanup:
+	drm_mode_config_cleanup(drm);
+	drm_release_iommu_mapping(drm);
+err_free_private:
+	kfree(private);
+err_free_drm:
+	drm_dev_unref(drm);
+
+	return ret;
 }
 
 static void exynos_drm_unbind(struct device *dev)
 {
-	drm_put_dev(dev_get_drvdata(dev));
+	struct drm_device *drm = dev_get_drvdata(dev);
+
+	drm_dev_unregister(drm);
+
+	exynos_drm_device_subdrv_remove(drm);
+
+	exynos_drm_fbdev_fini(drm);
+	drm_kms_helper_poll_fini(drm);
+
+	component_unbind_all(drm->dev, drm);
+	drm_mode_config_cleanup(drm);
+	drm_release_iommu_mapping(drm);
+
+	kfree(drm->dev_private);
+	drm->dev_private = NULL;
+
+	drm_dev_unref(drm);
 }
 
 static const struct component_master_ops exynos_drm_ops = {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index e07cb1fe4860..7e803fe2352f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1587,7 +1587,6 @@ static int exynos_dsi_create_connector(struct drm_encoder *encoder)
 	}
 
 	drm_connector_helper_add(connector, &exynos_dsi_connector_helper_funcs);
-	drm_connector_register(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
 	return 0;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index 57fe514d5c5b..6bbb0ea8a6af 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -359,7 +359,6 @@ static int vidi_create_connector(struct drm_encoder *encoder)
 	}
 
 	drm_connector_helper_add(connector, &vidi_connector_helper_funcs);
-	drm_connector_register(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
 	return 0;
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 5ed8b1effe71..3bc70f40cb7d 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -909,7 +909,6 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
 	}
 
 	drm_connector_helper_add(connector, &hdmi_connector_helper_funcs);
-	drm_connector_register(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
 	return 0;
-- 
Regards,

Laurent Pinchart

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

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

* [PATCH 2/2] drm: Remove unused drm_platform midlayer
  2016-12-13 19:34 [PATCH 0/2] exynosdrm: Remove .load/.unload midlayer Laurent Pinchart
  2016-12-13 19:34 ` [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time Laurent Pinchart
@ 2016-12-13 19:34 ` Laurent Pinchart
  2016-12-13 21:03   ` Daniel Vetter
  2016-12-17 22:39 ` [PATCH] drm: Remove the struct drm_device platformdev field Laurent Pinchart
  2 siblings, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2016-12-13 19:34 UTC (permalink / raw)
  To: dri-devel; +Cc: Kyungmin Park, Seung-Woo Kim

Now that the last driver has been converted, the drm_platform midlayer
is unused. Remove it.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/Makefile       |  2 +-
 drivers/gpu/drm/drm_platform.c | 91 ------------------------------------------
 include/drm/drmP.h             |  3 --
 3 files changed, 1 insertion(+), 95 deletions(-)
 delete mode 100644 drivers/gpu/drm/drm_platform.c

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index b9ae4280de9d..914b2d7b2c7d 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -7,7 +7,7 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
 		drm_fops.o drm_gem.o drm_ioctl.o drm_irq.o \
 		drm_lock.o drm_memory.o drm_drv.o drm_vm.o \
 		drm_scatter.o drm_pci.o \
-		drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
+		drm_sysfs.o drm_hashtab.o drm_mm.o \
 		drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
 		drm_info.o drm_encoder_slave.o \
 		drm_trace_points.o drm_global.o drm_prime.o \
diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
deleted file mode 100644
index 026269851ce9..000000000000
--- a/drivers/gpu/drm/drm_platform.c
+++ /dev/null
@@ -1,91 +0,0 @@
-/*
- * Derived from drm_pci.c
- *
- * Copyright 2003 José Fonseca.
- * Copyright 2003 Leif Delgass.
- * Copyright (c) 2009, Code Aurora Forum.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL THE
- * AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
- * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- */
-
-#include <linux/export.h>
-#include <drm/drmP.h>
-
-/*
- * Register.
- *
- * \param platdev - Platform device struture
- * \return zero on success or a negative number on failure.
- *
- * Attempt to gets inter module "drm" information. If we are first
- * then register the character device and inter module information.
- * Try and register, if we fail to register, backout previous work.
- */
-
-static int drm_get_platform_dev(struct platform_device *platdev,
-				struct drm_driver *driver)
-{
-	struct drm_device *dev;
-	int ret;
-
-	DRM_DEBUG("\n");
-
-	dev = drm_dev_alloc(driver, &platdev->dev);
-	if (IS_ERR(dev))
-		return PTR_ERR(dev);
-
-	dev->platformdev = platdev;
-
-	ret = drm_dev_register(dev, 0);
-	if (ret)
-		goto err_free;
-
-	DRM_INFO("Initialized %s %d.%d.%d %s on minor %d\n",
-		 driver->name, driver->major, driver->minor, driver->patchlevel,
-		 driver->date, dev->primary->index);
-
-	return 0;
-
-err_free:
-	drm_dev_unref(dev);
-	return ret;
-}
-
-/**
- * drm_platform_init - Register a platform device with the DRM subsystem
- * @driver: DRM device driver
- * @platform_device: platform device to register
- *
- * Registers the specified DRM device driver and platform device with the DRM
- * subsystem, initializing a drm_device structure and calling the driver's
- * .load() function.
- *
- * NOTE: This function is deprecated, please use drm_dev_alloc() and
- * drm_dev_register() instead and remove your ->load() callback.
- *
- * Return: 0 on success or a negative error code on failure.
- */
-int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device)
-{
-	DRM_DEBUG("\n");
-
-	return drm_get_platform_dev(platform_device, driver);
-}
-EXPORT_SYMBOL(drm_platform_init);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index a9cfd33c7b1a..0d6c1a13f533 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -825,9 +825,6 @@ static inline int drm_pci_set_busid(struct drm_device *dev,
 extern int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *speed_mask);
 extern int drm_pcie_get_max_link_width(struct drm_device *dev, u32 *mlw);
 
-/* platform section */
-extern int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device);
-
 /* returns true if currently okay to sleep */
 static __inline__ bool drm_can_sleep(void)
 {
-- 
Regards,

Laurent Pinchart

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

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

* Re: [PATCH 2/2] drm: Remove unused drm_platform midlayer
  2016-12-13 19:34 ` [PATCH 2/2] drm: Remove unused drm_platform midlayer Laurent Pinchart
@ 2016-12-13 21:03   ` Daniel Vetter
  2016-12-13 21:04     ` Sean Paul
  0 siblings, 1 reply; 26+ messages in thread
From: Daniel Vetter @ 2016-12-13 21:03 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Kyungmin Park, Seung-Woo Kim, dri-devel

On Tue, Dec 13, 2016 at 09:34:06PM +0200, Laurent Pinchart wrote:
> Now that the last driver has been converted, the drm_platform midlayer
> is unused. Remove it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/Makefile       |  2 +-
>  drivers/gpu/drm/drm_platform.c | 91 ------------------------------------------
>  include/drm/drmP.h             |  3 --

Absolutely awesome! Well except you forgot to double-check the kerneldoc
with make DOCBOOKS="" htmldocs and notice that you need to remove
drm_platform.c there too ;-)

With that fixed: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Feel
free to push to drm-misc.
-Daniel

>  3 files changed, 1 insertion(+), 95 deletions(-)
>  delete mode 100644 drivers/gpu/drm/drm_platform.c
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index b9ae4280de9d..914b2d7b2c7d 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -7,7 +7,7 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
>  		drm_fops.o drm_gem.o drm_ioctl.o drm_irq.o \
>  		drm_lock.o drm_memory.o drm_drv.o drm_vm.o \
>  		drm_scatter.o drm_pci.o \
> -		drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
> +		drm_sysfs.o drm_hashtab.o drm_mm.o \
>  		drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
>  		drm_info.o drm_encoder_slave.o \
>  		drm_trace_points.o drm_global.o drm_prime.o \
> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
> deleted file mode 100644
> index 026269851ce9..000000000000
> --- a/drivers/gpu/drm/drm_platform.c
> +++ /dev/null
> @@ -1,91 +0,0 @@
> -/*
> - * Derived from drm_pci.c
> - *
> - * Copyright 2003 José Fonseca.
> - * Copyright 2003 Leif Delgass.
> - * Copyright (c) 2009, Code Aurora Forum.
> - * All Rights Reserved.
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice (including the next
> - * paragraph) shall be included in all copies or substantial portions of the
> - * Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL THE
> - * AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> - * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
> - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> - */
> -
> -#include <linux/export.h>
> -#include <drm/drmP.h>
> -
> -/*
> - * Register.
> - *
> - * \param platdev - Platform device struture
> - * \return zero on success or a negative number on failure.
> - *
> - * Attempt to gets inter module "drm" information. If we are first
> - * then register the character device and inter module information.
> - * Try and register, if we fail to register, backout previous work.
> - */
> -
> -static int drm_get_platform_dev(struct platform_device *platdev,
> -				struct drm_driver *driver)
> -{
> -	struct drm_device *dev;
> -	int ret;
> -
> -	DRM_DEBUG("\n");
> -
> -	dev = drm_dev_alloc(driver, &platdev->dev);
> -	if (IS_ERR(dev))
> -		return PTR_ERR(dev);
> -
> -	dev->platformdev = platdev;
> -
> -	ret = drm_dev_register(dev, 0);
> -	if (ret)
> -		goto err_free;
> -
> -	DRM_INFO("Initialized %s %d.%d.%d %s on minor %d\n",
> -		 driver->name, driver->major, driver->minor, driver->patchlevel,
> -		 driver->date, dev->primary->index);
> -
> -	return 0;
> -
> -err_free:
> -	drm_dev_unref(dev);
> -	return ret;
> -}
> -
> -/**
> - * drm_platform_init - Register a platform device with the DRM subsystem
> - * @driver: DRM device driver
> - * @platform_device: platform device to register
> - *
> - * Registers the specified DRM device driver and platform device with the DRM
> - * subsystem, initializing a drm_device structure and calling the driver's
> - * .load() function.
> - *
> - * NOTE: This function is deprecated, please use drm_dev_alloc() and
> - * drm_dev_register() instead and remove your ->load() callback.
> - *
> - * Return: 0 on success or a negative error code on failure.
> - */
> -int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device)
> -{
> -	DRM_DEBUG("\n");
> -
> -	return drm_get_platform_dev(platform_device, driver);
> -}
> -EXPORT_SYMBOL(drm_platform_init);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index a9cfd33c7b1a..0d6c1a13f533 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -825,9 +825,6 @@ static inline int drm_pci_set_busid(struct drm_device *dev,
>  extern int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *speed_mask);
>  extern int drm_pcie_get_max_link_width(struct drm_device *dev, u32 *mlw);
>  
> -/* platform section */
> -extern int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device);
> -
>  /* returns true if currently okay to sleep */
>  static __inline__ bool drm_can_sleep(void)
>  {
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-13 19:34 ` [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time Laurent Pinchart
@ 2016-12-13 21:03   ` Sean Paul
  2016-12-13 21:10   ` Daniel Vetter
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 26+ messages in thread
From: Sean Paul @ 2016-12-13 21:03 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Kyungmin Park, Seung-Woo Kim, dri-devel

On Tue, Dec 13, 2016 at 2:34 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> The drm driver .load() operation is prone to race conditions as it
> initializes the driver after registering the device nodes. Its usage is
> deprecated, inline it in the probe function and call drm_dev_alloc() and
> drm_dev_register() explicitly.
>
> For consistency inline the .unload() handler in the remove function as
> well.
>

This looks reasonable to me, but I also don't have any exynos hardware
(that's running anything close to mainline).

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/gpu/drm/exynos/exynos_dp.c       |   1 -
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   1 -
>  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 245 ++++++++++++++++---------------
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   1 -
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c |   1 -
>  drivers/gpu/drm/exynos/exynos_hdmi.c     |   1 -
>  6 files changed, 127 insertions(+), 123 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
> index 528229faffe4..b839f065f4b3 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> @@ -102,7 +102,6 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
>         struct drm_encoder *encoder = &dp->encoder;
>         int ret;
>
> -       drm_connector_register(connector);
>         dp->connector = connector;
>
>         /* Pre-empt DP connector creation if there's a bridge */
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> index ad6b73c7fc59..3aab71a485ba 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> @@ -114,7 +114,6 @@ static int exynos_dpi_create_connector(struct drm_encoder *encoder)
>         }
>
>         drm_connector_helper_add(connector, &exynos_dpi_connector_helper_funcs);
> -       drm_connector_register(connector);
>         drm_mode_connector_attach_encoder(connector, encoder);
>
>         return 0;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 739180ac3da5..bcd3d1db53eb 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -90,120 +90,6 @@ static void exynos_drm_atomic_work(struct work_struct *work)
>
>  static struct device *exynos_drm_get_dma_device(void);
>
> -static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
> -{
> -       struct exynos_drm_private *private;
> -       struct drm_encoder *encoder;
> -       unsigned int clone_mask;
> -       int cnt, ret;
> -
> -       private = kzalloc(sizeof(struct exynos_drm_private), GFP_KERNEL);
> -       if (!private)
> -               return -ENOMEM;
> -
> -       init_waitqueue_head(&private->wait);
> -       spin_lock_init(&private->lock);
> -
> -       dev_set_drvdata(dev->dev, dev);
> -       dev->dev_private = (void *)private;
> -
> -       /* the first real CRTC device is used for all dma mapping operations */
> -       private->dma_dev = exynos_drm_get_dma_device();
> -       if (!private->dma_dev) {
> -               DRM_ERROR("no device found for DMA mapping operations.\n");
> -               ret = -ENODEV;
> -               goto err_free_private;
> -       }
> -       DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
> -                dev_name(private->dma_dev));
> -
> -       /* create common IOMMU mapping for all devices attached to Exynos DRM */
> -       ret = drm_create_iommu_mapping(dev);
> -       if (ret < 0) {
> -               DRM_ERROR("failed to create iommu mapping.\n");
> -               goto err_free_private;
> -       }
> -
> -       drm_mode_config_init(dev);
> -
> -       exynos_drm_mode_config_init(dev);
> -
> -       /* setup possible_clones. */
> -       cnt = 0;
> -       clone_mask = 0;
> -       list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
> -               clone_mask |= (1 << (cnt++));
> -
> -       list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
> -               encoder->possible_clones = clone_mask;
> -
> -       platform_set_drvdata(dev->platformdev, dev);
> -
> -       /* Try to bind all sub drivers. */
> -       ret = component_bind_all(dev->dev, dev);
> -       if (ret)
> -               goto err_mode_config_cleanup;
> -
> -       ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
> -       if (ret)
> -               goto err_unbind_all;
> -
> -       /* Probe non kms sub drivers and virtual display driver. */
> -       ret = exynos_drm_device_subdrv_probe(dev);
> -       if (ret)
> -               goto err_cleanup_vblank;
> -
> -       drm_mode_config_reset(dev);
> -
> -       /*
> -        * enable drm irq mode.
> -        * - with irq_enabled = true, we can use the vblank feature.
> -        *
> -        * P.S. note that we wouldn't use drm irq handler but
> -        *      just specific driver own one instead because
> -        *      drm framework supports only one irq handler.
> -        */
> -       dev->irq_enabled = true;
> -
> -       /* init kms poll for handling hpd */
> -       drm_kms_helper_poll_init(dev);
> -
> -       /* force connectors detection */
> -       drm_helper_hpd_irq_event(dev);
> -
> -       return 0;
> -
> -err_cleanup_vblank:
> -       drm_vblank_cleanup(dev);
> -err_unbind_all:
> -       component_unbind_all(dev->dev, dev);
> -err_mode_config_cleanup:
> -       drm_mode_config_cleanup(dev);
> -       drm_release_iommu_mapping(dev);
> -err_free_private:
> -       kfree(private);
> -
> -       return ret;
> -}
> -
> -static int exynos_drm_unload(struct drm_device *dev)
> -{
> -       exynos_drm_device_subdrv_remove(dev);
> -
> -       exynos_drm_fbdev_fini(dev);
> -       drm_kms_helper_poll_fini(dev);
> -
> -       drm_vblank_cleanup(dev);
> -       component_unbind_all(dev->dev, dev);
> -       drm_mode_config_cleanup(dev);
> -       drm_release_iommu_mapping(dev);
> -
> -       kfree(dev->dev_private);
> -       dev->dev_private = NULL;
> -
> -       return 0;
> -}
> -
>  static int commit_is_pending(struct exynos_drm_private *priv, u32 crtcs)
>  {
>         bool pending;
> @@ -373,8 +259,6 @@ static const struct file_operations exynos_drm_driver_fops = {
>  static struct drm_driver exynos_drm_driver = {
>         .driver_features        = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME
>                                   | DRIVER_ATOMIC | DRIVER_RENDER,
> -       .load                   = exynos_drm_load,
> -       .unload                 = exynos_drm_unload,
>         .open                   = exynos_drm_open,
>         .preclose               = exynos_drm_preclose,
>         .lastclose              = exynos_drm_lastclose,
> @@ -552,12 +436,137 @@ static struct component_match *exynos_drm_match_add(struct device *dev)
>
>  static int exynos_drm_bind(struct device *dev)
>  {
> -       return drm_platform_init(&exynos_drm_driver, to_platform_device(dev));
> +       struct exynos_drm_private *private;
> +       struct drm_encoder *encoder;
> +       struct drm_device *drm;
> +       unsigned int clone_mask;
> +       int cnt, ret;
> +
> +       drm = drm_dev_alloc(&exynos_drm_driver, dev);
> +       if (IS_ERR(drm))
> +               return PTR_ERR(drm);
> +
> +       private = kzalloc(sizeof(struct exynos_drm_private), GFP_KERNEL);
> +       if (!private) {
> +               ret = -ENOMEM;
> +               goto err_free_drm;
> +       }
> +
> +       init_waitqueue_head(&private->wait);
> +       spin_lock_init(&private->lock);
> +
> +       dev_set_drvdata(drm->dev, drm);
> +       drm->dev_private = (void *)private;
> +
> +       /* the first real CRTC device is used for all dma mapping operations */
> +       private->dma_dev = exynos_drm_get_dma_device();
> +       if (!private->dma_dev) {
> +               DRM_ERROR("no device found for DMA mapping operations.\n");
> +               ret = -ENODEV;
> +               goto err_free_private;
> +       }
> +       DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
> +                dev_name(private->dma_dev));
> +
> +       /* create common IOMMU mapping for all devices attached to Exynos DRM */
> +       ret = drm_create_iommu_mapping(drm);
> +       if (ret < 0) {
> +               DRM_ERROR("failed to create iommu mapping.\n");
> +               goto err_free_private;
> +       }
> +
> +       drm_mode_config_init(drm);
> +
> +       exynos_drm_mode_config_init(drm);
> +
> +       /* setup possible_clones. */
> +       cnt = 0;
> +       clone_mask = 0;
> +       list_for_each_entry(encoder, &drm->mode_config.encoder_list, head)
> +               clone_mask |= (1 << (cnt++));
> +
> +       list_for_each_entry(encoder, &drm->mode_config.encoder_list, head)
> +               encoder->possible_clones = clone_mask;
> +
> +       platform_set_drvdata(drm->platformdev, drm);
> +
> +       /* Try to bind all sub drivers. */
> +       ret = component_bind_all(drm->dev, drm);
> +       if (ret)
> +               goto err_mode_config_cleanup;
> +
> +       ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
> +       if (ret)
> +               goto err_unbind_all;
> +
> +       /* Probe non kms sub drivers and virtual display driver. */
> +       ret = exynos_drm_device_subdrv_probe(drm);
> +       if (ret)
> +               goto err_cleanup_vblank;
> +
> +       drm_mode_config_reset(drm);
> +
> +       /*
> +        * enable drm irq mode.
> +        * - with irq_enabled = true, we can use the vblank feature.
> +        *
> +        * P.S. note that we wouldn't use drm irq handler but
> +        *      just specific driver own one instead because
> +        *      drm framework supports only one irq handler.
> +        */
> +       drm->irq_enabled = true;
> +
> +       /* init kms poll for handling hpd */
> +       drm_kms_helper_poll_init(drm);
> +
> +       /* force connectors detection */
> +       drm_helper_hpd_irq_event(drm);
> +
> +       /* register the DRM device */
> +       ret = drm_dev_register(drm, 0);
> +       if (ret < 0)
> +               goto err_cleanup_fbdev;
> +
> +       return 0;
> +
> +err_cleanup_fbdev:
> +       exynos_drm_fbdev_fini(drm);
> +       drm_kms_helper_poll_fini(drm);
> +       exynos_drm_device_subdrv_remove(drm);
> +err_cleanup_vblank:
> +       drm_vblank_cleanup(drm);
> +err_unbind_all:
> +       component_unbind_all(drm->dev, drm);
> +err_mode_config_cleanup:
> +       drm_mode_config_cleanup(drm);
> +       drm_release_iommu_mapping(drm);
> +err_free_private:
> +       kfree(private);
> +err_free_drm:
> +       drm_dev_unref(drm);
> +
> +       return ret;
>  }
>
>  static void exynos_drm_unbind(struct device *dev)
>  {
> -       drm_put_dev(dev_get_drvdata(dev));
> +       struct drm_device *drm = dev_get_drvdata(dev);
> +
> +       drm_dev_unregister(drm);
> +
> +       exynos_drm_device_subdrv_remove(drm);
> +
> +       exynos_drm_fbdev_fini(drm);
> +       drm_kms_helper_poll_fini(drm);
> +
> +       component_unbind_all(drm->dev, drm);
> +       drm_mode_config_cleanup(drm);
> +       drm_release_iommu_mapping(drm);
> +
> +       kfree(drm->dev_private);
> +       drm->dev_private = NULL;
> +
> +       drm_dev_unref(drm);
>  }
>
>  static const struct component_master_ops exynos_drm_ops = {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index e07cb1fe4860..7e803fe2352f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1587,7 +1587,6 @@ static int exynos_dsi_create_connector(struct drm_encoder *encoder)
>         }
>
>         drm_connector_helper_add(connector, &exynos_dsi_connector_helper_funcs);
> -       drm_connector_register(connector);
>         drm_mode_connector_attach_encoder(connector, encoder);
>
>         return 0;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> index 57fe514d5c5b..6bbb0ea8a6af 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -359,7 +359,6 @@ static int vidi_create_connector(struct drm_encoder *encoder)
>         }
>
>         drm_connector_helper_add(connector, &vidi_connector_helper_funcs);
> -       drm_connector_register(connector);
>         drm_mode_connector_attach_encoder(connector, encoder);
>
>         return 0;
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 5ed8b1effe71..3bc70f40cb7d 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -909,7 +909,6 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
>         }
>
>         drm_connector_helper_add(connector, &hdmi_connector_helper_funcs);
> -       drm_connector_register(connector);
>         drm_mode_connector_attach_encoder(connector, encoder);
>
>         return 0;
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 2/2] drm: Remove unused drm_platform midlayer
  2016-12-13 21:03   ` Daniel Vetter
@ 2016-12-13 21:04     ` Sean Paul
  0 siblings, 0 replies; 26+ messages in thread
From: Sean Paul @ 2016-12-13 21:04 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Kyungmin Park, Laurent Pinchart, Seung-Woo Kim, dri-devel

On Tue, Dec 13, 2016 at 4:03 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Dec 13, 2016 at 09:34:06PM +0200, Laurent Pinchart wrote:
>> Now that the last driver has been converted, the drm_platform midlayer
>> is unused. Remove it.
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> ---
>>  drivers/gpu/drm/Makefile       |  2 +-
>>  drivers/gpu/drm/drm_platform.c | 91 ------------------------------------------
>>  include/drm/drmP.h             |  3 --
>
> Absolutely awesome! Well except you forgot to double-check the kerneldoc
> with make DOCBOOKS="" htmldocs and notice that you need to remove
> drm_platform.c there too ;-)
>
> With that fixed: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Feel
> free to push to drm-misc.

I'll pile on with my \o/

and my

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> -Daniel
>
>>  3 files changed, 1 insertion(+), 95 deletions(-)
>>  delete mode 100644 drivers/gpu/drm/drm_platform.c
>>
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index b9ae4280de9d..914b2d7b2c7d 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -7,7 +7,7 @@ drm-y       :=        drm_auth.o drm_bufs.o drm_cache.o \
>>               drm_fops.o drm_gem.o drm_ioctl.o drm_irq.o \
>>               drm_lock.o drm_memory.o drm_drv.o drm_vm.o \
>>               drm_scatter.o drm_pci.o \
>> -             drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
>> +             drm_sysfs.o drm_hashtab.o drm_mm.o \
>>               drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
>>               drm_info.o drm_encoder_slave.o \
>>               drm_trace_points.o drm_global.o drm_prime.o \
>> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
>> deleted file mode 100644
>> index 026269851ce9..000000000000
>> --- a/drivers/gpu/drm/drm_platform.c
>> +++ /dev/null
>> @@ -1,91 +0,0 @@
>> -/*
>> - * Derived from drm_pci.c
>> - *
>> - * Copyright 2003 José Fonseca.
>> - * Copyright 2003 Leif Delgass.
>> - * Copyright (c) 2009, Code Aurora Forum.
>> - * All Rights Reserved.
>> - *
>> - * Permission is hereby granted, free of charge, to any person obtaining a
>> - * copy of this software and associated documentation files (the "Software"),
>> - * to deal in the Software without restriction, including without limitation
>> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> - * and/or sell copies of the Software, and to permit persons to whom the
>> - * Software is furnished to do so, subject to the following conditions:
>> - *
>> - * The above copyright notice and this permission notice (including the next
>> - * paragraph) shall be included in all copies or substantial portions of the
>> - * Software.
>> - *
>> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL THE
>> - * AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> - * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
>> - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>> - */
>> -
>> -#include <linux/export.h>
>> -#include <drm/drmP.h>
>> -
>> -/*
>> - * Register.
>> - *
>> - * \param platdev - Platform device struture
>> - * \return zero on success or a negative number on failure.
>> - *
>> - * Attempt to gets inter module "drm" information. If we are first
>> - * then register the character device and inter module information.
>> - * Try and register, if we fail to register, backout previous work.
>> - */
>> -
>> -static int drm_get_platform_dev(struct platform_device *platdev,
>> -                             struct drm_driver *driver)
>> -{
>> -     struct drm_device *dev;
>> -     int ret;
>> -
>> -     DRM_DEBUG("\n");
>> -
>> -     dev = drm_dev_alloc(driver, &platdev->dev);
>> -     if (IS_ERR(dev))
>> -             return PTR_ERR(dev);
>> -
>> -     dev->platformdev = platdev;
>> -
>> -     ret = drm_dev_register(dev, 0);
>> -     if (ret)
>> -             goto err_free;
>> -
>> -     DRM_INFO("Initialized %s %d.%d.%d %s on minor %d\n",
>> -              driver->name, driver->major, driver->minor, driver->patchlevel,
>> -              driver->date, dev->primary->index);
>> -
>> -     return 0;
>> -
>> -err_free:
>> -     drm_dev_unref(dev);
>> -     return ret;
>> -}
>> -
>> -/**
>> - * drm_platform_init - Register a platform device with the DRM subsystem
>> - * @driver: DRM device driver
>> - * @platform_device: platform device to register
>> - *
>> - * Registers the specified DRM device driver and platform device with the DRM
>> - * subsystem, initializing a drm_device structure and calling the driver's
>> - * .load() function.
>> - *
>> - * NOTE: This function is deprecated, please use drm_dev_alloc() and
>> - * drm_dev_register() instead and remove your ->load() callback.
>> - *
>> - * Return: 0 on success or a negative error code on failure.
>> - */
>> -int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device)
>> -{
>> -     DRM_DEBUG("\n");
>> -
>> -     return drm_get_platform_dev(platform_device, driver);
>> -}
>> -EXPORT_SYMBOL(drm_platform_init);
>> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
>> index a9cfd33c7b1a..0d6c1a13f533 100644
>> --- a/include/drm/drmP.h
>> +++ b/include/drm/drmP.h
>> @@ -825,9 +825,6 @@ static inline int drm_pci_set_busid(struct drm_device *dev,
>>  extern int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *speed_mask);
>>  extern int drm_pcie_get_max_link_width(struct drm_device *dev, u32 *mlw);
>>
>> -/* platform section */
>> -extern int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device);
>> -
>>  /* returns true if currently okay to sleep */
>>  static __inline__ bool drm_can_sleep(void)
>>  {
>> --
>> Regards,
>>
>> Laurent Pinchart
>>
>> _______________________________________________
>> 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
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-13 19:34 ` [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time Laurent Pinchart
  2016-12-13 21:03   ` Sean Paul
@ 2016-12-13 21:10   ` Daniel Vetter
  2016-12-13 21:30     ` Laurent Pinchart
  2016-12-16 18:02   ` Daniel Stone
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 26+ messages in thread
From: Daniel Vetter @ 2016-12-13 21:10 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Kyungmin Park, Seung-Woo Kim, dri-devel

On Tue, Dec 13, 2016 at 09:34:05PM +0200, Laurent Pinchart wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> The drm driver .load() operation is prone to race conditions as it
> initializes the driver after registering the device nodes. Its usage is
> deprecated, inline it in the probe function and call drm_dev_alloc() and
> drm_dev_register() explicitly.
> 
> For consistency inline the .unload() handler in the remove function as
> well.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/gpu/drm/exynos/exynos_dp.c       |   1 -
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   1 -
>  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 245 ++++++++++++++++---------------
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   1 -
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c |   1 -
>  drivers/gpu/drm/exynos/exynos_hdmi.c     |   1 -
>  6 files changed, 127 insertions(+), 123 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
> index 528229faffe4..b839f065f4b3 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> @@ -102,7 +102,6 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
>  	struct drm_encoder *encoder = &dp->encoder;
>  	int ret;
>  
> -	drm_connector_register(connector);
>  	dp->connector = connector;
>  
>  	/* Pre-empt DP connector creation if there's a bridge */
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> index ad6b73c7fc59..3aab71a485ba 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> @@ -114,7 +114,6 @@ static int exynos_dpi_create_connector(struct drm_encoder *encoder)
>  	}
>  
>  	drm_connector_helper_add(connector, &exynos_dpi_connector_helper_funcs);
> -	drm_connector_register(connector);
>  	drm_mode_connector_attach_encoder(connector, encoder);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 739180ac3da5..bcd3d1db53eb 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -90,120 +90,6 @@ static void exynos_drm_atomic_work(struct work_struct *work)
>  
>  static struct device *exynos_drm_get_dma_device(void);
>  
> -static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
> -{
> -	struct exynos_drm_private *private;
> -	struct drm_encoder *encoder;
> -	unsigned int clone_mask;
> -	int cnt, ret;
> -
> -	private = kzalloc(sizeof(struct exynos_drm_private), GFP_KERNEL);
> -	if (!private)
> -		return -ENOMEM;
> -
> -	init_waitqueue_head(&private->wait);
> -	spin_lock_init(&private->lock);
> -
> -	dev_set_drvdata(dev->dev, dev);
> -	dev->dev_private = (void *)private;
> -
> -	/* the first real CRTC device is used for all dma mapping operations */
> -	private->dma_dev = exynos_drm_get_dma_device();
> -	if (!private->dma_dev) {
> -		DRM_ERROR("no device found for DMA mapping operations.\n");
> -		ret = -ENODEV;
> -		goto err_free_private;
> -	}
> -	DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
> -		 dev_name(private->dma_dev));
> -
> -	/* create common IOMMU mapping for all devices attached to Exynos DRM */
> -	ret = drm_create_iommu_mapping(dev);
> -	if (ret < 0) {
> -		DRM_ERROR("failed to create iommu mapping.\n");
> -		goto err_free_private;
> -	}
> -
> -	drm_mode_config_init(dev);
> -
> -	exynos_drm_mode_config_init(dev);
> -
> -	/* setup possible_clones. */
> -	cnt = 0;
> -	clone_mask = 0;
> -	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
> -		clone_mask |= (1 << (cnt++));
> -
> -	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
> -		encoder->possible_clones = clone_mask;
> -
> -	platform_set_drvdata(dev->platformdev, dev);
> -
> -	/* Try to bind all sub drivers. */
> -	ret = component_bind_all(dev->dev, dev);
> -	if (ret)
> -		goto err_mode_config_cleanup;
> -
> -	ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
> -	if (ret)
> -		goto err_unbind_all;
> -
> -	/* Probe non kms sub drivers and virtual display driver. */
> -	ret = exynos_drm_device_subdrv_probe(dev);
> -	if (ret)
> -		goto err_cleanup_vblank;
> -
> -	drm_mode_config_reset(dev);
> -
> -	/*
> -	 * enable drm irq mode.
> -	 * - with irq_enabled = true, we can use the vblank feature.
> -	 *
> -	 * P.S. note that we wouldn't use drm irq handler but
> -	 *	just specific driver own one instead because
> -	 *	drm framework supports only one irq handler.
> -	 */
> -	dev->irq_enabled = true;
> -
> -	/* init kms poll for handling hpd */
> -	drm_kms_helper_poll_init(dev);
> -
> -	/* force connectors detection */
> -	drm_helper_hpd_irq_event(dev);
> -
> -	return 0;
> -
> -err_cleanup_vblank:
> -	drm_vblank_cleanup(dev);
> -err_unbind_all:
> -	component_unbind_all(dev->dev, dev);
> -err_mode_config_cleanup:
> -	drm_mode_config_cleanup(dev);
> -	drm_release_iommu_mapping(dev);
> -err_free_private:
> -	kfree(private);
> -
> -	return ret;
> -}
> -
> -static int exynos_drm_unload(struct drm_device *dev)
> -{
> -	exynos_drm_device_subdrv_remove(dev);
> -
> -	exynos_drm_fbdev_fini(dev);
> -	drm_kms_helper_poll_fini(dev);
> -
> -	drm_vblank_cleanup(dev);
> -	component_unbind_all(dev->dev, dev);
> -	drm_mode_config_cleanup(dev);
> -	drm_release_iommu_mapping(dev);
> -
> -	kfree(dev->dev_private);
> -	dev->dev_private = NULL;
> -
> -	return 0;
> -}
> -
>  static int commit_is_pending(struct exynos_drm_private *priv, u32 crtcs)
>  {
>  	bool pending;
> @@ -373,8 +259,6 @@ static const struct file_operations exynos_drm_driver_fops = {
>  static struct drm_driver exynos_drm_driver = {
>  	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME
>  				  | DRIVER_ATOMIC | DRIVER_RENDER,
> -	.load			= exynos_drm_load,
> -	.unload			= exynos_drm_unload,
>  	.open			= exynos_drm_open,
>  	.preclose		= exynos_drm_preclose,
>  	.lastclose		= exynos_drm_lastclose,
> @@ -552,12 +436,137 @@ static struct component_match *exynos_drm_match_add(struct device *dev)
>  
>  static int exynos_drm_bind(struct device *dev)
>  {
> -	return drm_platform_init(&exynos_drm_driver, to_platform_device(dev));
> +	struct exynos_drm_private *private;
> +	struct drm_encoder *encoder;
> +	struct drm_device *drm;
> +	unsigned int clone_mask;
> +	int cnt, ret;
> +
> +	drm = drm_dev_alloc(&exynos_drm_driver, dev);
> +	if (IS_ERR(drm))
> +		return PTR_ERR(drm);
> +
> +	private = kzalloc(sizeof(struct exynos_drm_private), GFP_KERNEL);
> +	if (!private) {
> +		ret = -ENOMEM;
> +		goto err_free_drm;
> +	}
> +
> +	init_waitqueue_head(&private->wait);
> +	spin_lock_init(&private->lock);
> +
> +	dev_set_drvdata(drm->dev, drm);
> +	drm->dev_private = (void *)private;
> +
> +	/* the first real CRTC device is used for all dma mapping operations */
> +	private->dma_dev = exynos_drm_get_dma_device();
> +	if (!private->dma_dev) {
> +		DRM_ERROR("no device found for DMA mapping operations.\n");
> +		ret = -ENODEV;
> +		goto err_free_private;
> +	}
> +	DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
> +		 dev_name(private->dma_dev));
> +
> +	/* create common IOMMU mapping for all devices attached to Exynos DRM */
> +	ret = drm_create_iommu_mapping(drm);
> +	if (ret < 0) {
> +		DRM_ERROR("failed to create iommu mapping.\n");
> +		goto err_free_private;
> +	}
> +
> +	drm_mode_config_init(drm);
> +
> +	exynos_drm_mode_config_init(drm);
> +
> +	/* setup possible_clones. */
> +	cnt = 0;
> +	clone_mask = 0;
> +	list_for_each_entry(encoder, &drm->mode_config.encoder_list, head)
> +		clone_mask |= (1 << (cnt++));
> +
> +	list_for_each_entry(encoder, &drm->mode_config.encoder_list, head)
> +		encoder->possible_clones = clone_mask;
> +
> +	platform_set_drvdata(drm->platformdev, drm);
> +
> +	/* Try to bind all sub drivers. */
> +	ret = component_bind_all(drm->dev, drm);
> +	if (ret)
> +		goto err_mode_config_cleanup;
> +
> +	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
> +	if (ret)
> +		goto err_unbind_all;
> +
> +	/* Probe non kms sub drivers and virtual display driver. */
> +	ret = exynos_drm_device_subdrv_probe(drm);
> +	if (ret)
> +		goto err_cleanup_vblank;
> +
> +	drm_mode_config_reset(drm);
> +
> +	/*
> +	 * enable drm irq mode.
> +	 * - with irq_enabled = true, we can use the vblank feature.
> +	 *
> +	 * P.S. note that we wouldn't use drm irq handler but
> +	 *	just specific driver own one instead because
> +	 *	drm framework supports only one irq handler.
> +	 */
> +	drm->irq_enabled = true;
> +
> +	/* init kms poll for handling hpd */
> +	drm_kms_helper_poll_init(drm);
> +
> +	/* force connectors detection */
> +	drm_helper_hpd_irq_event(drm);
> +
> +	/* register the DRM device */
> +	ret = drm_dev_register(drm, 0);
> +	if (ret < 0)
> +		goto err_cleanup_fbdev;
> +
> +	return 0;
> +
> +err_cleanup_fbdev:
> +	exynos_drm_fbdev_fini(drm);
> +	drm_kms_helper_poll_fini(drm);
> +	exynos_drm_device_subdrv_remove(drm);
> +err_cleanup_vblank:
> +	drm_vblank_cleanup(drm);
> +err_unbind_all:
> +	component_unbind_all(drm->dev, drm);
> +err_mode_config_cleanup:
> +	drm_mode_config_cleanup(drm);
> +	drm_release_iommu_mapping(drm);
> +err_free_private:
> +	kfree(private);
> +err_free_drm:
> +	drm_dev_unref(drm);
> +
> +	return ret;
>  }
>  
>  static void exynos_drm_unbind(struct device *dev)
>  {
> -	drm_put_dev(dev_get_drvdata(dev));
> +	struct drm_device *drm = dev_get_drvdata(dev);
> +
> +	drm_dev_unregister(drm);
> +
> +	exynos_drm_device_subdrv_remove(drm);
> +
> +	exynos_drm_fbdev_fini(drm);
> +	drm_kms_helper_poll_fini(drm);

Unbind order is inverted from the error paths in the probe function, but
meh, preexisting.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>o

... because I really want to see drm_platform.c gone! Feel free to push to
drm-misc, you haz commit rights after all ;-)
-Daniel

> +
> +	component_unbind_all(drm->dev, drm);
> +	drm_mode_config_cleanup(drm);
> +	drm_release_iommu_mapping(drm);
> +
> +	kfree(drm->dev_private);
> +	drm->dev_private = NULL;
> +
> +	drm_dev_unref(drm);
>  }
>  
>  static const struct component_master_ops exynos_drm_ops = {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index e07cb1fe4860..7e803fe2352f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1587,7 +1587,6 @@ static int exynos_dsi_create_connector(struct drm_encoder *encoder)
>  	}
>  
>  	drm_connector_helper_add(connector, &exynos_dsi_connector_helper_funcs);
> -	drm_connector_register(connector);
>  	drm_mode_connector_attach_encoder(connector, encoder);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> index 57fe514d5c5b..6bbb0ea8a6af 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -359,7 +359,6 @@ static int vidi_create_connector(struct drm_encoder *encoder)
>  	}
>  
>  	drm_connector_helper_add(connector, &vidi_connector_helper_funcs);
> -	drm_connector_register(connector);
>  	drm_mode_connector_attach_encoder(connector, encoder);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 5ed8b1effe71..3bc70f40cb7d 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -909,7 +909,6 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
>  	}
>  
>  	drm_connector_helper_add(connector, &hdmi_connector_helper_funcs);
> -	drm_connector_register(connector);
>  	drm_mode_connector_attach_encoder(connector, encoder);
>  
>  	return 0;
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-13 21:10   ` Daniel Vetter
@ 2016-12-13 21:30     ` Laurent Pinchart
  2016-12-13 21:32       ` Sean Paul
  0 siblings, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2016-12-13 21:30 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Kyungmin Park, Laurent Pinchart, Seung-Woo Kim, dri-devel

On Tuesday 13 Dec 2016 22:10:58 Daniel Vetter wrote:
> On Tue, Dec 13, 2016 at 09:34:05PM +0200, Laurent Pinchart wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > The drm driver .load() operation is prone to race conditions as it
> > initializes the driver after registering the device nodes. Its usage is
> > deprecated, inline it in the probe function and call drm_dev_alloc() and
> > drm_dev_register() explicitly.
> > 
> > For consistency inline the .unload() handler in the remove function as
> > well.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/gpu/drm/exynos/exynos_dp.c       |   1 -
> >  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   1 -
> >  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 245 ++++++++++++++------------
> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   1 -
> >  drivers/gpu/drm/exynos/exynos_drm_vidi.c |   1 -
> >  drivers/gpu/drm/exynos/exynos_hdmi.c     |   1 -
> >  6 files changed, 127 insertions(+), 123 deletions(-)

[snip]

> >  static void exynos_drm_unbind(struct device *dev)
> >  {
> > -	drm_put_dev(dev_get_drvdata(dev));
> > +	struct drm_device *drm = dev_get_drvdata(dev);
> > +
> > +	drm_dev_unregister(drm);
> > +
> > +	exynos_drm_device_subdrv_remove(drm);
> > +
> > +	exynos_drm_fbdev_fini(drm);
> > +	drm_kms_helper_poll_fini(drm);
> 
> Unbind order is inverted from the error paths in the probe function, but
> meh, preexisting.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>o
> 
> ... because I really want to see drm_platform.c gone! Feel free to push to
> drm-misc, you haz commit rights after all ;-)

Let's try to get the patch tested first :-) Is exynosdrm merged through drm-
misc ?

> > +
> > +	component_unbind_all(drm->dev, drm);
> > +	drm_mode_config_cleanup(drm);
> > +	drm_release_iommu_mapping(drm);
> > +
> > +	kfree(drm->dev_private);
> > +	drm->dev_private = NULL;
> > +
> > +	drm_dev_unref(drm);
> >  }

-- 
Regards,

Laurent Pinchart

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

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

* Re: [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-13 21:30     ` Laurent Pinchart
@ 2016-12-13 21:32       ` Sean Paul
  2016-12-13 21:49         ` Daniel Vetter
  0 siblings, 1 reply; 26+ messages in thread
From: Sean Paul @ 2016-12-13 21:32 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Kyungmin Park, Laurent Pinchart, Seung-Woo Kim, dri-devel

On Tue, Dec 13, 2016 at 4:30 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tuesday 13 Dec 2016 22:10:58 Daniel Vetter wrote:
>> On Tue, Dec 13, 2016 at 09:34:05PM +0200, Laurent Pinchart wrote:
>> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> >
>> > The drm driver .load() operation is prone to race conditions as it
>> > initializes the driver after registering the device nodes. Its usage is
>> > deprecated, inline it in the probe function and call drm_dev_alloc() and
>> > drm_dev_register() explicitly.
>> >
>> > For consistency inline the .unload() handler in the remove function as
>> > well.
>> >
>> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> > ---
>> >
>> >  drivers/gpu/drm/exynos/exynos_dp.c       |   1 -
>> >  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   1 -
>> >  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 245 ++++++++++++++------------
>> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   1 -
>> >  drivers/gpu/drm/exynos/exynos_drm_vidi.c |   1 -
>> >  drivers/gpu/drm/exynos/exynos_hdmi.c     |   1 -
>> >  6 files changed, 127 insertions(+), 123 deletions(-)
>
> [snip]
>
>> >  static void exynos_drm_unbind(struct device *dev)
>> >  {
>> > -   drm_put_dev(dev_get_drvdata(dev));
>> > +   struct drm_device *drm = dev_get_drvdata(dev);
>> > +
>> > +   drm_dev_unregister(drm);
>> > +
>> > +   exynos_drm_device_subdrv_remove(drm);
>> > +
>> > +   exynos_drm_fbdev_fini(drm);
>> > +   drm_kms_helper_poll_fini(drm);
>>
>> Unbind order is inverted from the error paths in the probe function, but
>> meh, preexisting.
>>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>o
>>
>> ... because I really want to see drm_platform.c gone! Feel free to push to
>> drm-misc, you haz commit rights after all ;-)
>
> Let's try to get the patch tested first :-) Is exynosdrm merged through drm-
> misc ?
>

Since 2/2 should go through misc, it makes sense to take the whole
series through there (instead of lockstepping with exynos tree)

Sean

>> > +
>> > +   component_unbind_all(drm->dev, drm);
>> > +   drm_mode_config_cleanup(drm);
>> > +   drm_release_iommu_mapping(drm);
>> > +
>> > +   kfree(drm->dev_private);
>> > +   drm->dev_private = NULL;
>> > +
>> > +   drm_dev_unref(drm);
>> >  }
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-13 21:32       ` Sean Paul
@ 2016-12-13 21:49         ` Daniel Vetter
  2016-12-13 21:54           ` Laurent Pinchart
  0 siblings, 1 reply; 26+ messages in thread
From: Daniel Vetter @ 2016-12-13 21:49 UTC (permalink / raw)
  To: Sean Paul
  Cc: Laurent Pinchart, Seung-Woo Kim, dri-devel, Kyungmin Park,
	Laurent Pinchart

On Tue, Dec 13, 2016 at 04:32:30PM -0500, Sean Paul wrote:
> On Tue, Dec 13, 2016 at 4:30 PM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Tuesday 13 Dec 2016 22:10:58 Daniel Vetter wrote:
> >> On Tue, Dec 13, 2016 at 09:34:05PM +0200, Laurent Pinchart wrote:
> >> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> >
> >> > The drm driver .load() operation is prone to race conditions as it
> >> > initializes the driver after registering the device nodes. Its usage is
> >> > deprecated, inline it in the probe function and call drm_dev_alloc() and
> >> > drm_dev_register() explicitly.
> >> >
> >> > For consistency inline the .unload() handler in the remove function as
> >> > well.
> >> >
> >> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> > ---
> >> >
> >> >  drivers/gpu/drm/exynos/exynos_dp.c       |   1 -
> >> >  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   1 -
> >> >  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 245 ++++++++++++++------------
> >> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   1 -
> >> >  drivers/gpu/drm/exynos/exynos_drm_vidi.c |   1 -
> >> >  drivers/gpu/drm/exynos/exynos_hdmi.c     |   1 -
> >> >  6 files changed, 127 insertions(+), 123 deletions(-)
> >
> > [snip]
> >
> >> >  static void exynos_drm_unbind(struct device *dev)
> >> >  {
> >> > -   drm_put_dev(dev_get_drvdata(dev));
> >> > +   struct drm_device *drm = dev_get_drvdata(dev);
> >> > +
> >> > +   drm_dev_unregister(drm);
> >> > +
> >> > +   exynos_drm_device_subdrv_remove(drm);
> >> > +
> >> > +   exynos_drm_fbdev_fini(drm);
> >> > +   drm_kms_helper_poll_fini(drm);
> >>
> >> Unbind order is inverted from the error paths in the probe function, but
> >> meh, preexisting.
> >>
> >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>o
> >>
> >> ... because I really want to see drm_platform.c gone! Feel free to push to
> >> drm-misc, you haz commit rights after all ;-)
> >
> > Let's try to get the patch tested first :-) Is exynosdrm merged through drm-
> > misc ?
> >
> 
> Since 2/2 should go through misc, it makes sense to take the whole
> series through there (instead of lockstepping with exynos tree)

Same for the one omapdrm patch if that's ok. drm-misc is resynced with
drm-next-4.10, so there shouldn't be anything pending and it should all
apply cleanly.
-Daniel

> 
> Sean
> 
> >> > +
> >> > +   component_unbind_all(drm->dev, drm);
> >> > +   drm_mode_config_cleanup(drm);
> >> > +   drm_release_iommu_mapping(drm);
> >> > +
> >> > +   kfree(drm->dev_private);
> >> > +   drm->dev_private = NULL;
> >> > +
> >> > +   drm_dev_unref(drm);
> >> >  }
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
> > _______________________________________________
> > 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-13 21:49         ` Daniel Vetter
@ 2016-12-13 21:54           ` Laurent Pinchart
  0 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-12-13 21:54 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Seung-Woo Kim, Kyungmin Park, Laurent Pinchart, dri-devel

Hi Daniel,

On Tuesday 13 Dec 2016 22:49:51 Daniel Vetter wrote:
> On Tue, Dec 13, 2016 at 04:32:30PM -0500, Sean Paul wrote:
> > On Tue, Dec 13, 2016 at 4:30 PM, Laurent Pinchart wrote:
> >> On Tuesday 13 Dec 2016 22:10:58 Daniel Vetter wrote:
> >>> On Tue, Dec 13, 2016 at 09:34:05PM +0200, Laurent Pinchart wrote:
> >>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>> 
> >>>> The drm driver .load() operation is prone to race conditions as it
> >>>> initializes the driver after registering the device nodes. Its usage
> >>>> is deprecated, inline it in the probe function and call
> >>>> drm_dev_alloc() and drm_dev_register() explicitly.
> >>>> 
> >>>> For consistency inline the .unload() handler in the remove function
> >>>> as well.
> >>>> 
> >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>> ---
> >>>> 
> >>>>  drivers/gpu/drm/exynos/exynos_dp.c       |   1 -
> >>>>  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   1 -
> >>>>  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 245 ++++++++++++---------
> >>>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   1 -
> >>>>  drivers/gpu/drm/exynos/exynos_drm_vidi.c |   1 -
> >>>>  drivers/gpu/drm/exynos/exynos_hdmi.c     |   1 -
> >>>>  6 files changed, 127 insertions(+), 123 deletions(-)
> >> 
> >> [snip]
> >> 
> >>>>  static void exynos_drm_unbind(struct device *dev)
> >>>>  {
> >>>> 
> >>>> -   drm_put_dev(dev_get_drvdata(dev));
> >>>> +   struct drm_device *drm = dev_get_drvdata(dev);
> >>>> +
> >>>> +   drm_dev_unregister(drm);
> >>>> +
> >>>> +   exynos_drm_device_subdrv_remove(drm);
> >>>> +
> >>>> +   exynos_drm_fbdev_fini(drm);
> >>>> +   drm_kms_helper_poll_fini(drm);
> >>> 
> >>> Unbind order is inverted from the error paths in the probe function,
> >>> but meh, preexisting.
> >>> 
> >>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>o
> >>> 
> >>> ... because I really want to see drm_platform.c gone! Feel free to push
> >>> to drm-misc, you haz commit rights after all ;-)
> >> 
> >> Let's try to get the patch tested first :-) Is exynosdrm merged through
> >> drm- misc ?
> > 
> > Since 2/2 should go through misc, it makes sense to take the whole
> > series through there (instead of lockstepping with exynos tree)
> 
> Same for the one omapdrm patch if that's ok. drm-misc is resynced with
> drm-next-4.10, so there shouldn't be anything pending and it should all
> apply cleanly.

Except that the omapdrm patch is based on top of a series of ~20 pending 
patches. I'll respin the whole lot soon.

> >>>> +
> >>>> +   component_unbind_all(drm->dev, drm);
> >>>> +   drm_mode_config_cleanup(drm);
> >>>> +   drm_release_iommu_mapping(drm);
> >>>> +
> >>>> +   kfree(drm->dev_private);
> >>>> +   drm->dev_private = NULL;
> >>>> +
> >>>> +   drm_dev_unref(drm);
> >>>>  }

-- 
Regards,

Laurent Pinchart

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

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

* Re: [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-13 19:34 ` [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time Laurent Pinchart
  2016-12-13 21:03   ` Sean Paul
  2016-12-13 21:10   ` Daniel Vetter
@ 2016-12-16 18:02   ` Daniel Stone
  2016-12-17 21:49     ` Laurent Pinchart
  2016-12-17  0:33   ` Inki Dae
  2016-12-17 22:29   ` [PATCH v2] " Laurent Pinchart
  4 siblings, 1 reply; 26+ messages in thread
From: Daniel Stone @ 2016-12-16 18:02 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Kyungmin Park, Seung-Woo Kim, dri-devel

Hey Laurent,

On 13 December 2016 at 19:34, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> The drm driver .load() operation is prone to race conditions as it
> initializes the driver after registering the device nodes. Its usage is
> deprecated, inline it in the probe function and call drm_dev_alloc() and
> drm_dev_register() explicitly.
>
> For consistency inline the .unload() handler in the remove function as
> well.

Almost there: you need to add 'drm->platformdev =
to_platform_device(dev)' next to the drm->dev_private assignment. I
thought about cleaning this up, but my XU3's eMMC is dead, so testing
on Exynos for me has a 5+ minute RTT through LAVA ... so this is:

Reviewed-by: Daniel Stone <daniels@collabora.com>

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

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

* Re: [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-13 19:34 ` [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time Laurent Pinchart
                     ` (2 preceding siblings ...)
  2016-12-16 18:02   ` Daniel Stone
@ 2016-12-17  0:33   ` Inki Dae
  2016-12-17 22:12     ` Laurent Pinchart
  2016-12-17 22:29   ` [PATCH v2] " Laurent Pinchart
  4 siblings, 1 reply; 26+ messages in thread
From: Inki Dae @ 2016-12-17  0:33 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Kyungmin Park, Seung-Woo Kim, DRI mailing list

HI,

Thanks for patch. Reasonable to me and go to misc excepting below one thing.
Please check my comment.

2016-12-14 4:34 GMT+09:00 Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com>:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> The drm driver .load() operation is prone to race conditions as it
> initializes the driver after registering the device nodes. Its usage is
> deprecated, inline it in the probe function and call drm_dev_alloc() and
> drm_dev_register() explicitly.
>
> For consistency inline the .unload() handler in the remove function as
> well.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/gpu/drm/exynos/exynos_dp.c       |   1 -
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   1 -
>  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 245 ++++++++++++++++---------------
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   1 -
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c |   1 -
>  drivers/gpu/drm/exynos/exynos_hdmi.c     |   1 -
>  6 files changed, 127 insertions(+), 123 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
> index 528229faffe4..b839f065f4b3 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> @@ -102,7 +102,6 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
>         struct drm_encoder *encoder = &dp->encoder;
>         int ret;
>
> -       drm_connector_register(connector);

You removed above function from encoder and connector drivers.Is
removing this required?
And is this related to this patch? If not so, it seems this change
should go to another patch with the reason to remove this function
call.

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

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

* Re: [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-16 18:02   ` Daniel Stone
@ 2016-12-17 21:49     ` Laurent Pinchart
  0 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-12-17 21:49 UTC (permalink / raw)
  To: Daniel Stone; +Cc: Kyungmin Park, Laurent Pinchart, Seung-Woo Kim, dri-devel

Hi Daniel,

On Friday 16 Dec 2016 18:02:20 Daniel Stone wrote:
> On 13 December 2016 at 19:34, Laurent Pinchart wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > The drm driver .load() operation is prone to race conditions as it
> > initializes the driver after registering the device nodes. Its usage is
> > deprecated, inline it in the probe function and call drm_dev_alloc() and
> > drm_dev_register() explicitly.
> > 
> > For consistency inline the .unload() handler in the remove function as
> > well.
> 
> Almost there: you need to add 'drm->platformdev =
> to_platform_device(dev)' next to the drm->dev_private assignment.

I don't think that's needed. The field was indeed initialized by 
drm_get_platform_dev() (called from drm_platform_init()), but only used in two 
locations in the driver where the struct device is available through different 
ways.

As struct drm_device contains a separate struct device *dev, I'll write a 
patch to remove the platformdev field.

> I thought about cleaning this up, but my XU3's eMMC is dead, so testing
> on Exynos for me has a 5+ minute RTT through LAVA ... so this is:
> 
> Reviewed-by: Daniel Stone <daniels@collabora.com>

Thanks.

-- 
Regards,

Laurent Pinchart

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

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

* Re: [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-17  0:33   ` Inki Dae
@ 2016-12-17 22:12     ` Laurent Pinchart
  2016-12-19  6:32       ` Inki Dae
  0 siblings, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2016-12-17 22:12 UTC (permalink / raw)
  To: Inki Dae; +Cc: Kyungmin Park, Laurent Pinchart, Seung-Woo Kim, DRI mailing list

Hello Inki,

On Saturday 17 Dec 2016 09:33:31 Inki Dae wrote:
> HI,
> 
> Thanks for patch. Reasonable to me and go to misc excepting below one thing.
> Please check my comment.
> 
> 2016-12-14 4:34 GMT+09:00 Laurent Pinchart:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > The drm driver .load() operation is prone to race conditions as it
> > initializes the driver after registering the device nodes. Its usage is
> > deprecated, inline it in the probe function and call drm_dev_alloc() and
> > drm_dev_register() explicitly.
> > 
> > For consistency inline the .unload() handler in the remove function as
> > well.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/gpu/drm/exynos/exynos_dp.c       |   1 -
> >  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   1 -
> >  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 245 ++++++++++++-------------
> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   1 -
> >  drivers/gpu/drm/exynos/exynos_drm_vidi.c |   1 -
> >  drivers/gpu/drm/exynos/exynos_hdmi.c     |   1 -
> >  6 files changed, 127 insertions(+), 123 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/exynos/exynos_dp.c
> > b/drivers/gpu/drm/exynos/exynos_dp.c index 528229faffe4..b839f065f4b3
> > 100644
> > --- a/drivers/gpu/drm/exynos/exynos_dp.c
> > +++ b/drivers/gpu/drm/exynos/exynos_dp.c
> > @@ -102,7 +102,6 @@ static int exynos_dp_bridge_attach(struct
> > analogix_dp_plat_data *plat_data,> 
> >         struct drm_encoder *encoder = &dp->encoder;
> >         int ret;
> > 
> > -       drm_connector_register(connector);
> 
> You removed above function from encoder and connector drivers.Is
> removing this required?
> And is this related to this patch? If not so, it seems this change
> should go to another patch with the reason to remove this function
> call.

When using the .load() callback, driver initialization is performed from 
drm_dev_register() after the DRM device gets registered with sysfs. With this 
patch driver initialization is moved before drm_dev_register(), and 
registering connectors manually would then trigger a WARN due to the sysfs 
parent not being registered yet.

The connectors are registered by the DRM core (drm_modeset_register_all() 
called from drm_dev_register()), so there's no need to register them manually 
after drm_dev_register(), we can just drop that code.

-- 
Regards,

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

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

* [PATCH v2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-13 19:34 ` [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time Laurent Pinchart
                     ` (3 preceding siblings ...)
  2016-12-17  0:33   ` Inki Dae
@ 2016-12-17 22:29   ` Laurent Pinchart
  2016-12-19  9:29     ` Daniel Stone
  4 siblings, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2016-12-17 22:29 UTC (permalink / raw)
  To: dri-devel; +Cc: Kyungmin Park, Seung-Woo Kim

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

The drm driver .load() operation is prone to race conditions as it
initializes the driver after registering the device nodes. Its usage is
deprecated, inline it in the probe function and call drm_dev_alloc() and
drm_dev_register() explicitly.

For consistency inline the .unload() handler in the remove function as
well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Daniel Stone <daniels@collabora.com>
---
Changes since v1:

- Use drm_device.dev instead of drm_device.platformdev in
  exynos_drm_fbdev_create() as the platformdev field isn't set anymore

 drivers/gpu/drm/exynos/exynos_dp.c        |   1 -
 drivers/gpu/drm/exynos/exynos_drm_dpi.c   |   1 -
 drivers/gpu/drm/exynos/exynos_drm_drv.c   | 243 +++++++++++++++---------------
 drivers/gpu/drm/exynos/exynos_drm_dsi.c   |   1 -
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   3 +-
 drivers/gpu/drm/exynos/exynos_drm_vidi.c  |   1 -
 drivers/gpu/drm/exynos/exynos_hdmi.c      |   1 -
 7 files changed, 126 insertions(+), 125 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index 528229faffe4..b839f065f4b3 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -102,7 +102,6 @@ static int exynos_dp_bridge_attach(struct analogix_dp_plat_data *plat_data,
 	struct drm_encoder *encoder = &dp->encoder;
 	int ret;
 
-	drm_connector_register(connector);
 	dp->connector = connector;
 
 	/* Pre-empt DP connector creation if there's a bridge */
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
index ad6b73c7fc59..3aab71a485ba 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
@@ -114,7 +114,6 @@ static int exynos_dpi_create_connector(struct drm_encoder *encoder)
 	}
 
 	drm_connector_helper_add(connector, &exynos_dpi_connector_helper_funcs);
-	drm_connector_register(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
 	return 0;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 739180ac3da5..28f64bcc1b86 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -90,120 +90,6 @@ static void exynos_drm_atomic_work(struct work_struct *work)
 
 static struct device *exynos_drm_get_dma_device(void);
 
-static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
-{
-	struct exynos_drm_private *private;
-	struct drm_encoder *encoder;
-	unsigned int clone_mask;
-	int cnt, ret;
-
-	private = kzalloc(sizeof(struct exynos_drm_private), GFP_KERNEL);
-	if (!private)
-		return -ENOMEM;
-
-	init_waitqueue_head(&private->wait);
-	spin_lock_init(&private->lock);
-
-	dev_set_drvdata(dev->dev, dev);
-	dev->dev_private = (void *)private;
-
-	/* the first real CRTC device is used for all dma mapping operations */
-	private->dma_dev = exynos_drm_get_dma_device();
-	if (!private->dma_dev) {
-		DRM_ERROR("no device found for DMA mapping operations.\n");
-		ret = -ENODEV;
-		goto err_free_private;
-	}
-	DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
-		 dev_name(private->dma_dev));
-
-	/* create common IOMMU mapping for all devices attached to Exynos DRM */
-	ret = drm_create_iommu_mapping(dev);
-	if (ret < 0) {
-		DRM_ERROR("failed to create iommu mapping.\n");
-		goto err_free_private;
-	}
-
-	drm_mode_config_init(dev);
-
-	exynos_drm_mode_config_init(dev);
-
-	/* setup possible_clones. */
-	cnt = 0;
-	clone_mask = 0;
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
-		clone_mask |= (1 << (cnt++));
-
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
-		encoder->possible_clones = clone_mask;
-
-	platform_set_drvdata(dev->platformdev, dev);
-
-	/* Try to bind all sub drivers. */
-	ret = component_bind_all(dev->dev, dev);
-	if (ret)
-		goto err_mode_config_cleanup;
-
-	ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
-	if (ret)
-		goto err_unbind_all;
-
-	/* Probe non kms sub drivers and virtual display driver. */
-	ret = exynos_drm_device_subdrv_probe(dev);
-	if (ret)
-		goto err_cleanup_vblank;
-
-	drm_mode_config_reset(dev);
-
-	/*
-	 * enable drm irq mode.
-	 * - with irq_enabled = true, we can use the vblank feature.
-	 *
-	 * P.S. note that we wouldn't use drm irq handler but
-	 *	just specific driver own one instead because
-	 *	drm framework supports only one irq handler.
-	 */
-	dev->irq_enabled = true;
-
-	/* init kms poll for handling hpd */
-	drm_kms_helper_poll_init(dev);
-
-	/* force connectors detection */
-	drm_helper_hpd_irq_event(dev);
-
-	return 0;
-
-err_cleanup_vblank:
-	drm_vblank_cleanup(dev);
-err_unbind_all:
-	component_unbind_all(dev->dev, dev);
-err_mode_config_cleanup:
-	drm_mode_config_cleanup(dev);
-	drm_release_iommu_mapping(dev);
-err_free_private:
-	kfree(private);
-
-	return ret;
-}
-
-static int exynos_drm_unload(struct drm_device *dev)
-{
-	exynos_drm_device_subdrv_remove(dev);
-
-	exynos_drm_fbdev_fini(dev);
-	drm_kms_helper_poll_fini(dev);
-
-	drm_vblank_cleanup(dev);
-	component_unbind_all(dev->dev, dev);
-	drm_mode_config_cleanup(dev);
-	drm_release_iommu_mapping(dev);
-
-	kfree(dev->dev_private);
-	dev->dev_private = NULL;
-
-	return 0;
-}
-
 static int commit_is_pending(struct exynos_drm_private *priv, u32 crtcs)
 {
 	bool pending;
@@ -373,8 +259,6 @@ static const struct file_operations exynos_drm_driver_fops = {
 static struct drm_driver exynos_drm_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME
 				  | DRIVER_ATOMIC | DRIVER_RENDER,
-	.load			= exynos_drm_load,
-	.unload			= exynos_drm_unload,
 	.open			= exynos_drm_open,
 	.preclose		= exynos_drm_preclose,
 	.lastclose		= exynos_drm_lastclose,
@@ -552,12 +436,135 @@ static struct component_match *exynos_drm_match_add(struct device *dev)
 
 static int exynos_drm_bind(struct device *dev)
 {
-	return drm_platform_init(&exynos_drm_driver, to_platform_device(dev));
+	struct exynos_drm_private *private;
+	struct drm_encoder *encoder;
+	struct drm_device *drm;
+	unsigned int clone_mask;
+	int cnt, ret;
+
+	drm = drm_dev_alloc(&exynos_drm_driver, dev);
+	if (IS_ERR(drm))
+		return PTR_ERR(drm);
+
+	private = kzalloc(sizeof(struct exynos_drm_private), GFP_KERNEL);
+	if (!private) {
+		ret = -ENOMEM;
+		goto err_free_drm;
+	}
+
+	init_waitqueue_head(&private->wait);
+	spin_lock_init(&private->lock);
+
+	dev_set_drvdata(dev, drm);
+	drm->dev_private = (void *)private;
+
+	/* the first real CRTC device is used for all dma mapping operations */
+	private->dma_dev = exynos_drm_get_dma_device();
+	if (!private->dma_dev) {
+		DRM_ERROR("no device found for DMA mapping operations.\n");
+		ret = -ENODEV;
+		goto err_free_private;
+	}
+	DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
+		 dev_name(private->dma_dev));
+
+	/* create common IOMMU mapping for all devices attached to Exynos DRM */
+	ret = drm_create_iommu_mapping(drm);
+	if (ret < 0) {
+		DRM_ERROR("failed to create iommu mapping.\n");
+		goto err_free_private;
+	}
+
+	drm_mode_config_init(drm);
+
+	exynos_drm_mode_config_init(drm);
+
+	/* setup possible_clones. */
+	cnt = 0;
+	clone_mask = 0;
+	list_for_each_entry(encoder, &drm->mode_config.encoder_list, head)
+		clone_mask |= (1 << (cnt++));
+
+	list_for_each_entry(encoder, &drm->mode_config.encoder_list, head)
+		encoder->possible_clones = clone_mask;
+
+	/* Try to bind all sub drivers. */
+	ret = component_bind_all(drm->dev, drm);
+	if (ret)
+		goto err_mode_config_cleanup;
+
+	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
+	if (ret)
+		goto err_unbind_all;
+
+	/* Probe non kms sub drivers and virtual display driver. */
+	ret = exynos_drm_device_subdrv_probe(drm);
+	if (ret)
+		goto err_cleanup_vblank;
+
+	drm_mode_config_reset(drm);
+
+	/*
+	 * enable drm irq mode.
+	 * - with irq_enabled = true, we can use the vblank feature.
+	 *
+	 * P.S. note that we wouldn't use drm irq handler but
+	 *	just specific driver own one instead because
+	 *	drm framework supports only one irq handler.
+	 */
+	drm->irq_enabled = true;
+
+	/* init kms poll for handling hpd */
+	drm_kms_helper_poll_init(drm);
+
+	/* force connectors detection */
+	drm_helper_hpd_irq_event(drm);
+
+	/* register the DRM device */
+	ret = drm_dev_register(drm, 0);
+	if (ret < 0)
+		goto err_cleanup_fbdev;
+
+	return 0;
+
+err_cleanup_fbdev:
+	exynos_drm_fbdev_fini(drm);
+	drm_kms_helper_poll_fini(drm);
+	exynos_drm_device_subdrv_remove(drm);
+err_cleanup_vblank:
+	drm_vblank_cleanup(drm);
+err_unbind_all:
+	component_unbind_all(drm->dev, drm);
+err_mode_config_cleanup:
+	drm_mode_config_cleanup(drm);
+	drm_release_iommu_mapping(drm);
+err_free_private:
+	kfree(private);
+err_free_drm:
+	drm_dev_unref(drm);
+
+	return ret;
 }
 
 static void exynos_drm_unbind(struct device *dev)
 {
-	drm_put_dev(dev_get_drvdata(dev));
+	struct drm_device *drm = dev_get_drvdata(dev);
+
+	drm_dev_unregister(drm);
+
+	exynos_drm_device_subdrv_remove(drm);
+
+	exynos_drm_fbdev_fini(drm);
+	drm_kms_helper_poll_fini(drm);
+
+	component_unbind_all(drm->dev, drm);
+	drm_mode_config_cleanup(drm);
+	drm_release_iommu_mapping(drm);
+
+	kfree(drm->dev_private);
+	drm->dev_private = NULL;
+
+	drm_dev_unref(drm);
 }
 
 static const struct component_master_ops exynos_drm_ops = {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index e07cb1fe4860..7e803fe2352f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1587,7 +1587,6 @@ static int exynos_dsi_create_connector(struct drm_encoder *encoder)
 	}
 
 	drm_connector_helper_add(connector, &exynos_dsi_connector_helper_funcs);
-	drm_connector_register(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
 	return 0;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 9f35deb56170..6b170aa44eaa 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -120,7 +120,6 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper *helper,
 	struct exynos_drm_gem *exynos_gem;
 	struct drm_device *dev = helper->dev;
 	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
-	struct platform_device *pdev = dev->platformdev;
 	unsigned long size;
 	int ret;
 
@@ -143,7 +142,7 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper *helper,
 	 * memory area.
 	 */
 	if (IS_ERR(exynos_gem) && is_drm_iommu_supported(dev)) {
-		dev_warn(&pdev->dev, "contiguous FB allocation failed, falling back to non-contiguous\n");
+		dev_warn(dev->dev, "contiguous FB allocation failed, falling back to non-contiguous\n");
 		exynos_gem = exynos_drm_gem_create(dev, EXYNOS_BO_NONCONTIG,
 						   size);
 	}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index 57fe514d5c5b..6bbb0ea8a6af 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -359,7 +359,6 @@ static int vidi_create_connector(struct drm_encoder *encoder)
 	}
 
 	drm_connector_helper_add(connector, &vidi_connector_helper_funcs);
-	drm_connector_register(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
 	return 0;
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 5ed8b1effe71..3bc70f40cb7d 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -909,7 +909,6 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
 	}
 
 	drm_connector_helper_add(connector, &hdmi_connector_helper_funcs);
-	drm_connector_register(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
 	return 0;
-- 
Regards,

Laurent Pinchart

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

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

* [PATCH] drm: Remove the struct drm_device platformdev field
  2016-12-13 19:34 [PATCH 0/2] exynosdrm: Remove .load/.unload midlayer Laurent Pinchart
  2016-12-13 19:34 ` [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time Laurent Pinchart
  2016-12-13 19:34 ` [PATCH 2/2] drm: Remove unused drm_platform midlayer Laurent Pinchart
@ 2016-12-17 22:39 ` Laurent Pinchart
  2016-12-18 13:16   ` Daniel Vetter
                     ` (5 more replies)
  2 siblings, 6 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-12-17 22:39 UTC (permalink / raw)
  To: dri-devel
  Cc: Chen Feng, Russell King, Jyri Sarha, Xinliang Liu, Xinwei Kong,
	Ben Skeggs, Rongrong Zou, Vincent Abriou

The field contains a pointer to the parent platform device of the DRM
device. As struct drm_device also contains a dev pointer to the struct
device embedded in the platform_device structure, the platformdev field
is redundant. Remove it and use the dev pointer directly.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/armada/armada_drv.c             | 3 +--
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 5 ++---
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 --
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c         | 2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c         | 2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c        | 2 +-
 drivers/gpu/drm/msm/msm_drv.c                   | 1 -
 drivers/gpu/drm/nouveau/nouveau_drm.c           | 3 +--
 drivers/gpu/drm/sti/sti_drv.c                   | 2 --
 drivers/gpu/drm/tilcdc/tilcdc_drv.c             | 1 -
 include/drm/drmP.h                              | 1 -
 11 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index 07086b427c22..f6442ed23bcd 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -154,10 +154,9 @@ static int armada_drm_bind(struct device *dev)
 		return ret;
 	}
 
-	priv->drm.platformdev = to_platform_device(dev);
 	priv->drm.dev_private = priv;
 
-	platform_set_drvdata(priv->drm.platformdev, &priv->drm);
+	dev_set_drvdata(dev, &priv->drm);
 
 	INIT_WORK(&priv->fb_unref_work, armada_drm_unref_work);
 	INIT_KFIFO(priv->fb_unref);
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
index afc2b5d2d5f0..ec1c70d1b682 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
@@ -975,7 +975,7 @@ static int ade_dts_parse(struct platform_device *pdev, struct ade_hw_ctx *ctx)
 
 static int ade_drm_init(struct drm_device *dev)
 {
-	struct platform_device *pdev = dev->platformdev;
+	struct platform_device *pdev = to_platform_device(dev->dev);
 	struct ade_data *ade;
 	struct ade_hw_ctx *ctx;
 	struct ade_crtc *acrtc;
@@ -1036,8 +1036,7 @@ static int ade_drm_init(struct drm_device *dev)
 
 static void ade_drm_cleanup(struct drm_device *dev)
 {
-	struct platform_device *pdev = dev->platformdev;
-	struct ade_data *ade = platform_get_drvdata(pdev);
+	struct ade_data *ade = dev_get_drvdata(dev->dev);
 	struct drm_crtc *crtc = &ade->acrtc.base;
 
 	drm_crtc_cleanup(crtc);
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index ebd5f4fe4c23..842f70251691 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -209,8 +209,6 @@ static int kirin_drm_bind(struct device *dev)
 	if (IS_ERR(drm_dev))
 		return PTR_ERR(drm_dev);
 
-	drm_dev->platformdev = to_platform_device(dev);
-
 	ret = kirin_drm_kms_init(drm_dev);
 	if (ret)
 		goto err_drm_dev_unref;
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
index b782efd4b95f..e8e14a502d21 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
@@ -438,7 +438,7 @@ static int modeset_init(struct mdp4_kms *mdp4_kms)
 
 struct msm_kms *mdp4_kms_init(struct drm_device *dev)
 {
-	struct platform_device *pdev = dev->platformdev;
+	struct platform_device *pdev = to_platform_device(dev->dev);
 	struct mdp4_platform_config *config = mdp4_get_config(pdev);
 	struct mdp4_kms *mdp4_kms;
 	struct msm_kms *kms = NULL;
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
index 618b2ffed9b4..70eae85cf1de 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
@@ -495,7 +495,7 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
 		uint32_t major, uint32_t minor)
 {
 	struct drm_device *dev = mdp5_kms->dev;
-	struct platform_device *pdev = dev->platformdev;
+	struct platform_device *pdev = to_platform_device(dev->dev);
 	struct mdp5_cfg_handler *cfg_handler;
 	struct mdp5_cfg_platform *pconfig;
 	int i, ret = 0;
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
index d444a6901fff..f8f48d014978 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
@@ -160,7 +160,7 @@ void msm_mdss_destroy(struct drm_device *dev)
 
 int msm_mdss_init(struct drm_device *dev)
 {
-	struct platform_device *pdev = dev->platformdev;
+	struct platform_device *pdev = to_platform_device(dev->dev);
 	struct msm_drm_private *priv = dev->dev_private;
 	struct msm_mdss *mdss;
 	int ret;
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index e29bb66f55b1..12548642b227 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -364,7 +364,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
 	}
 
 	platform_set_drvdata(pdev, ddev);
-	ddev->platformdev = pdev;
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv) {
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 59348fc41c77..6082e184007d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -106,7 +106,7 @@ nouveau_name(struct drm_device *dev)
 	if (dev->pdev)
 		return nouveau_pci_name(dev->pdev);
 	else
-		return nouveau_platform_name(dev->platformdev);
+		return nouveau_platform_name(to_platform_device(dev->dev));
 }
 
 static int
@@ -1089,7 +1089,6 @@ nouveau_platform_device_create(const struct nvkm_device_tegra_func *func,
 		goto err_free;
 	}
 
-	drm->platformdev = pdev;
 	platform_set_drvdata(pdev, drm);
 
 	return drm;
diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index ff71e25ab5bf..0df0de397d2c 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -388,8 +388,6 @@ static int sti_bind(struct device *dev)
 	if (IS_ERR(ddev))
 		return PTR_ERR(ddev);
 
-	ddev->platformdev = to_platform_device(dev);
-
 	ret = sti_init(ddev);
 	if (ret)
 		goto err_drm_dev_unref;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index bd0a3bd07167..8f7f6a54ee68 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -245,7 +245,6 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
 	if (IS_ERR(ddev))
 		return PTR_ERR(ddev);
 
-	ddev->platformdev = pdev;
 	ddev->dev_private = priv;
 	platform_set_drvdata(pdev, ddev);
 	drm_mode_config_init(ddev);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 0d6c1a13f533..4cc27449d6d7 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -609,7 +609,6 @@ struct drm_device {
 	struct pci_controller *hose;
 #endif
 
-	struct platform_device *platformdev; /**< Platform device struture */
 	struct virtio_device *virtdev;
 
 	struct drm_sg_mem *sg;	/**< Scatter gather memory */
-- 
Regards,

Laurent Pinchart

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

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

* Re: [PATCH] drm: Remove the struct drm_device platformdev field
  2016-12-17 22:39 ` [PATCH] drm: Remove the struct drm_device platformdev field Laurent Pinchart
@ 2016-12-18 13:16   ` Daniel Vetter
  2016-12-18 20:24     ` Laurent Pinchart
  2016-12-19  8:30   ` Jyri Sarha
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 26+ messages in thread
From: Daniel Vetter @ 2016-12-18 13:16 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Chen Feng, Xinwei Kong, Jyri Sarha, Xinliang Liu, Russell King,
	dri-devel, Rongrong Zou, Vincent Abriou, Ben Skeggs

On Sun, Dec 18, 2016 at 12:39:16AM +0200, Laurent Pinchart wrote:
> The field contains a pointer to the parent platform device of the DRM
> device. As struct drm_device also contains a dev pointer to the struct
> device embedded in the platform_device structure, the platformdev field
> is redundant. Remove it and use the dev pointer directly.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Oh sweet.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/armada/armada_drv.c             | 3 +--
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 5 ++---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 --
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c         | 2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c         | 2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c        | 2 +-
>  drivers/gpu/drm/msm/msm_drv.c                   | 1 -
>  drivers/gpu/drm/nouveau/nouveau_drm.c           | 3 +--
>  drivers/gpu/drm/sti/sti_drv.c                   | 2 --
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c             | 1 -
>  include/drm/drmP.h                              | 1 -
>  11 files changed, 7 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> index 07086b427c22..f6442ed23bcd 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -154,10 +154,9 @@ static int armada_drm_bind(struct device *dev)
>  		return ret;
>  	}
>  
> -	priv->drm.platformdev = to_platform_device(dev);
>  	priv->drm.dev_private = priv;
>  
> -	platform_set_drvdata(priv->drm.platformdev, &priv->drm);
> +	dev_set_drvdata(dev, &priv->drm);
>  
>  	INIT_WORK(&priv->fb_unref_work, armada_drm_unref_work);
>  	INIT_KFIFO(priv->fb_unref);
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index afc2b5d2d5f0..ec1c70d1b682 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -975,7 +975,7 @@ static int ade_dts_parse(struct platform_device *pdev, struct ade_hw_ctx *ctx)
>  
>  static int ade_drm_init(struct drm_device *dev)
>  {
> -	struct platform_device *pdev = dev->platformdev;
> +	struct platform_device *pdev = to_platform_device(dev->dev);
>  	struct ade_data *ade;
>  	struct ade_hw_ctx *ctx;
>  	struct ade_crtc *acrtc;
> @@ -1036,8 +1036,7 @@ static int ade_drm_init(struct drm_device *dev)
>  
>  static void ade_drm_cleanup(struct drm_device *dev)
>  {
> -	struct platform_device *pdev = dev->platformdev;
> -	struct ade_data *ade = platform_get_drvdata(pdev);
> +	struct ade_data *ade = dev_get_drvdata(dev->dev);
>  	struct drm_crtc *crtc = &ade->acrtc.base;
>  
>  	drm_crtc_cleanup(crtc);
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> index ebd5f4fe4c23..842f70251691 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> @@ -209,8 +209,6 @@ static int kirin_drm_bind(struct device *dev)
>  	if (IS_ERR(drm_dev))
>  		return PTR_ERR(drm_dev);
>  
> -	drm_dev->platformdev = to_platform_device(dev);
> -
>  	ret = kirin_drm_kms_init(drm_dev);
>  	if (ret)
>  		goto err_drm_dev_unref;
> diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> index b782efd4b95f..e8e14a502d21 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> @@ -438,7 +438,7 @@ static int modeset_init(struct mdp4_kms *mdp4_kms)
>  
>  struct msm_kms *mdp4_kms_init(struct drm_device *dev)
>  {
> -	struct platform_device *pdev = dev->platformdev;
> +	struct platform_device *pdev = to_platform_device(dev->dev);
>  	struct mdp4_platform_config *config = mdp4_get_config(pdev);
>  	struct mdp4_kms *mdp4_kms;
>  	struct msm_kms *kms = NULL;
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> index 618b2ffed9b4..70eae85cf1de 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> @@ -495,7 +495,7 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
>  		uint32_t major, uint32_t minor)
>  {
>  	struct drm_device *dev = mdp5_kms->dev;
> -	struct platform_device *pdev = dev->platformdev;
> +	struct platform_device *pdev = to_platform_device(dev->dev);
>  	struct mdp5_cfg_handler *cfg_handler;
>  	struct mdp5_cfg_platform *pconfig;
>  	int i, ret = 0;
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
> index d444a6901fff..f8f48d014978 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
> @@ -160,7 +160,7 @@ void msm_mdss_destroy(struct drm_device *dev)
>  
>  int msm_mdss_init(struct drm_device *dev)
>  {
> -	struct platform_device *pdev = dev->platformdev;
> +	struct platform_device *pdev = to_platform_device(dev->dev);
>  	struct msm_drm_private *priv = dev->dev_private;
>  	struct msm_mdss *mdss;
>  	int ret;
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index e29bb66f55b1..12548642b227 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -364,7 +364,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
>  	}
>  
>  	platform_set_drvdata(pdev, ddev);
> -	ddev->platformdev = pdev;
>  
>  	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>  	if (!priv) {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 59348fc41c77..6082e184007d 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -106,7 +106,7 @@ nouveau_name(struct drm_device *dev)
>  	if (dev->pdev)
>  		return nouveau_pci_name(dev->pdev);
>  	else
> -		return nouveau_platform_name(dev->platformdev);
> +		return nouveau_platform_name(to_platform_device(dev->dev));
>  }
>  
>  static int
> @@ -1089,7 +1089,6 @@ nouveau_platform_device_create(const struct nvkm_device_tegra_func *func,
>  		goto err_free;
>  	}
>  
> -	drm->platformdev = pdev;
>  	platform_set_drvdata(pdev, drm);
>  
>  	return drm;
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index ff71e25ab5bf..0df0de397d2c 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -388,8 +388,6 @@ static int sti_bind(struct device *dev)
>  	if (IS_ERR(ddev))
>  		return PTR_ERR(ddev);
>  
> -	ddev->platformdev = to_platform_device(dev);
> -
>  	ret = sti_init(ddev);
>  	if (ret)
>  		goto err_drm_dev_unref;
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index bd0a3bd07167..8f7f6a54ee68 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -245,7 +245,6 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
>  	if (IS_ERR(ddev))
>  		return PTR_ERR(ddev);
>  
> -	ddev->platformdev = pdev;
>  	ddev->dev_private = priv;
>  	platform_set_drvdata(pdev, ddev);
>  	drm_mode_config_init(ddev);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 0d6c1a13f533..4cc27449d6d7 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -609,7 +609,6 @@ struct drm_device {
>  	struct pci_controller *hose;
>  #endif
>  
> -	struct platform_device *platformdev; /**< Platform device struture */
>  	struct virtio_device *virtdev;
>  
>  	struct drm_sg_mem *sg;	/**< Scatter gather memory */
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH] drm: Remove the struct drm_device platformdev field
  2016-12-18 13:16   ` Daniel Vetter
@ 2016-12-18 20:24     ` Laurent Pinchart
  0 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-12-18 20:24 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Laurent Pinchart, Chen Feng, Xinwei Kong, Jyri Sarha,
	Xinliang Liu, Russell King, dri-devel, Rongrong Zou,
	Vincent Abriou, Ben Skeggs

Hi Daniel,

On Sunday 18 Dec 2016 14:16:26 Daniel Vetter wrote:
> On Sun, Dec 18, 2016 at 12:39:16AM +0200, Laurent Pinchart wrote:
> > The field contains a pointer to the parent platform device of the DRM
> > device. As struct drm_device also contains a dev pointer to the struct
> > device embedded in the platform_device structure, the platformdev field
> > is redundant. Remove it and use the dev pointer directly.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> 
> Oh sweet.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks. Now it's your turn for the struct pci_device pointer ;-)

> > ---
> > 
> >  drivers/gpu/drm/armada/armada_drv.c             | 3 +--
> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 5 ++---
> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 --
> >  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c         | 2 +-
> >  drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c         | 2 +-
> >  drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c        | 2 +-
> >  drivers/gpu/drm/msm/msm_drv.c                   | 1 -
> >  drivers/gpu/drm/nouveau/nouveau_drm.c           | 3 +--
> >  drivers/gpu/drm/sti/sti_drv.c                   | 2 --
> >  drivers/gpu/drm/tilcdc/tilcdc_drv.c             | 1 -
> >  include/drm/drmP.h                              | 1 -
> >  11 files changed, 7 insertions(+), 17 deletions(-)

-- 
Regards,

Laurent Pinchart

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

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

* Re: [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-17 22:12     ` Laurent Pinchart
@ 2016-12-19  6:32       ` Inki Dae
  0 siblings, 0 replies; 26+ messages in thread
From: Inki Dae @ 2016-12-19  6:32 UTC (permalink / raw)
  To: Laurent Pinchart, Inki Dae
  Cc: Kyungmin Park, Laurent Pinchart, Seung-Woo Kim, DRI mailing list



2016년 12월 18일 07:12에 Laurent Pinchart 이(가) 쓴 글:
> Hello Inki,
> 
> On Saturday 17 Dec 2016 09:33:31 Inki Dae wrote:
>> HI,
>>
>> Thanks for patch. Reasonable to me and go to misc excepting below one thing.
>> Please check my comment.
>>
>> 2016-12-14 4:34 GMT+09:00 Laurent Pinchart:
>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>
>>> The drm driver .load() operation is prone to race conditions as it
>>> initializes the driver after registering the device nodes. Its usage is
>>> deprecated, inline it in the probe function and call drm_dev_alloc() and
>>> drm_dev_register() explicitly.
>>>
>>> For consistency inline the .unload() handler in the remove function as
>>> well.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> ---
>>>
>>>  drivers/gpu/drm/exynos/exynos_dp.c       |   1 -
>>>  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   1 -
>>>  drivers/gpu/drm/exynos/exynos_drm_drv.c  | 245 ++++++++++++-------------
>>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   1 -
>>>  drivers/gpu/drm/exynos/exynos_drm_vidi.c |   1 -
>>>  drivers/gpu/drm/exynos/exynos_hdmi.c     |   1 -
>>>  6 files changed, 127 insertions(+), 123 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_dp.c
>>> b/drivers/gpu/drm/exynos/exynos_dp.c index 528229faffe4..b839f065f4b3
>>> 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_dp.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_dp.c
>>> @@ -102,7 +102,6 @@ static int exynos_dp_bridge_attach(struct
>>> analogix_dp_plat_data *plat_data,> 
>>>         struct drm_encoder *encoder = &dp->encoder;
>>>         int ret;
>>>
>>> -       drm_connector_register(connector);
>>
>> You removed above function from encoder and connector drivers.Is
>> removing this required?
>> And is this related to this patch? If not so, it seems this change
>> should go to another patch with the reason to remove this function
>> call.
> 
> When using the .load() callback, driver initialization is performed from 
> drm_dev_register() after the DRM device gets registered with sysfs. With this 
> patch driver initialization is moved before drm_dev_register(), and 
> registering connectors manually would then trigger a WARN due to the sysfs 
> parent not being registered yet.
> 
> The connectors are registered by the DRM core (drm_modeset_register_all() 
> called from drm_dev_register()), so there's no need to register them manually 
> after drm_dev_register(), we can just drop that code.
> 

Acked-by: Inki Dae <inki.dae@samsung.com>

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

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

* Re: [PATCH] drm: Remove the struct drm_device platformdev field
  2016-12-17 22:39 ` [PATCH] drm: Remove the struct drm_device platformdev field Laurent Pinchart
  2016-12-18 13:16   ` Daniel Vetter
@ 2016-12-19  8:30   ` Jyri Sarha
  2017-01-03 13:49   ` Vincent ABRIOU
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 26+ messages in thread
From: Jyri Sarha @ 2016-12-19  8:30 UTC (permalink / raw)
  To: Laurent Pinchart, dri-devel
  Cc: Chen Feng, Russell King, Xinliang Liu, Xinwei Kong, Ben Skeggs,
	Rongrong Zou, Vincent Abriou

On 12/18/16 00:39, Laurent Pinchart wrote:
> The field contains a pointer to the parent platform device of the DRM
> device. As struct drm_device also contains a dev pointer to the struct
> device embedded in the platform_device structure, the platformdev field
> is redundant. Remove it and use the dev pointer directly.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Acked-by: Jyri Sarha <jsarha@ti.com>

> ---
>  drivers/gpu/drm/armada/armada_drv.c             | 3 +--
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 5 ++---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 --
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c         | 2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c         | 2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c        | 2 +-
>  drivers/gpu/drm/msm/msm_drv.c                   | 1 -
>  drivers/gpu/drm/nouveau/nouveau_drm.c           | 3 +--
>  drivers/gpu/drm/sti/sti_drv.c                   | 2 --
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c             | 1 -
>  include/drm/drmP.h                              | 1 -
>  11 files changed, 7 insertions(+), 17 deletions(-)


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

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

* Re: [PATCH v2] drm: exynos: Perform initialization/cleanup at probe/remove time
  2016-12-17 22:29   ` [PATCH v2] " Laurent Pinchart
@ 2016-12-19  9:29     ` Daniel Stone
  0 siblings, 0 replies; 26+ messages in thread
From: Daniel Stone @ 2016-12-19  9:29 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Kyungmin Park, Seung-Woo Kim, dri-devel

Hi Laurent,

On 17 December 2016 at 22:29, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> The drm driver .load() operation is prone to race conditions as it
> initializes the driver after registering the device nodes. Its usage is
> deprecated, inline it in the probe function and call drm_dev_alloc() and
> drm_dev_register() explicitly.
>
> For consistency inline the .unload() handler in the remove function as
> well.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Daniel Stone <daniels@collabora.com>
> ---
> Changes since v1:
>
> - Use drm_device.dev instead of drm_device.platformdev in
>   exynos_drm_fbdev_create() as the platformdev field isn't set anymore

Yeah, that looks good. That was what I was getting at with my last
mail, but after a few roundtrips through LAVA trying to figure out
which extra config options I needed to get the panel to light up, and
being sick as well, I'd lost the will to live and didn't feel much
like figuring that one out myself. ;) Thanks!

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

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

* Re: [PATCH] drm: Remove the struct drm_device platformdev field
  2016-12-17 22:39 ` [PATCH] drm: Remove the struct drm_device platformdev field Laurent Pinchart
  2016-12-18 13:16   ` Daniel Vetter
  2016-12-19  8:30   ` Jyri Sarha
@ 2017-01-03 13:49   ` Vincent ABRIOU
  2017-01-03 14:00   ` Russell King - ARM Linux
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 26+ messages in thread
From: Vincent ABRIOU @ 2017-01-03 13:49 UTC (permalink / raw)
  To: Laurent Pinchart, dri-devel
  Cc: Chen Feng, Russell King, Jyri Sarha, Xinliang Liu, Xinwei Kong,
	Ben Skeggs, Rongrong Zou

Hi Laurent,

Ok for the sti driver.
Acked-by: Vincent Abriou <vincent.abriou@st.com>

On 12/17/2016 11:39 PM, Laurent Pinchart wrote:
> The field contains a pointer to the parent platform device of the DRM
> device. As struct drm_device also contains a dev pointer to the struct
> device embedded in the platform_device structure, the platformdev field
> is redundant. Remove it and use the dev pointer directly.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/armada/armada_drv.c             | 3 +--
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 5 ++---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 --
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c         | 2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c         | 2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c        | 2 +-
>  drivers/gpu/drm/msm/msm_drv.c                   | 1 -
>  drivers/gpu/drm/nouveau/nouveau_drm.c           | 3 +--
>  drivers/gpu/drm/sti/sti_drv.c                   | 2 --
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c             | 1 -
>  include/drm/drmP.h                              | 1 -
>  11 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> index 07086b427c22..f6442ed23bcd 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -154,10 +154,9 @@ static int armada_drm_bind(struct device *dev)
>  		return ret;
>  	}
>
> -	priv->drm.platformdev = to_platform_device(dev);
>  	priv->drm.dev_private = priv;
>
> -	platform_set_drvdata(priv->drm.platformdev, &priv->drm);
> +	dev_set_drvdata(dev, &priv->drm);
>
>  	INIT_WORK(&priv->fb_unref_work, armada_drm_unref_work);
>  	INIT_KFIFO(priv->fb_unref);
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index afc2b5d2d5f0..ec1c70d1b682 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -975,7 +975,7 @@ static int ade_dts_parse(struct platform_device *pdev, struct ade_hw_ctx *ctx)
>
>  static int ade_drm_init(struct drm_device *dev)
>  {
> -	struct platform_device *pdev = dev->platformdev;
> +	struct platform_device *pdev = to_platform_device(dev->dev);
>  	struct ade_data *ade;
>  	struct ade_hw_ctx *ctx;
>  	struct ade_crtc *acrtc;
> @@ -1036,8 +1036,7 @@ static int ade_drm_init(struct drm_device *dev)
>
>  static void ade_drm_cleanup(struct drm_device *dev)
>  {
> -	struct platform_device *pdev = dev->platformdev;
> -	struct ade_data *ade = platform_get_drvdata(pdev);
> +	struct ade_data *ade = dev_get_drvdata(dev->dev);
>  	struct drm_crtc *crtc = &ade->acrtc.base;
>
>  	drm_crtc_cleanup(crtc);
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> index ebd5f4fe4c23..842f70251691 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> @@ -209,8 +209,6 @@ static int kirin_drm_bind(struct device *dev)
>  	if (IS_ERR(drm_dev))
>  		return PTR_ERR(drm_dev);
>
> -	drm_dev->platformdev = to_platform_device(dev);
> -
>  	ret = kirin_drm_kms_init(drm_dev);
>  	if (ret)
>  		goto err_drm_dev_unref;
> diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> index b782efd4b95f..e8e14a502d21 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> @@ -438,7 +438,7 @@ static int modeset_init(struct mdp4_kms *mdp4_kms)
>
>  struct msm_kms *mdp4_kms_init(struct drm_device *dev)
>  {
> -	struct platform_device *pdev = dev->platformdev;
> +	struct platform_device *pdev = to_platform_device(dev->dev);
>  	struct mdp4_platform_config *config = mdp4_get_config(pdev);
>  	struct mdp4_kms *mdp4_kms;
>  	struct msm_kms *kms = NULL;
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> index 618b2ffed9b4..70eae85cf1de 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> @@ -495,7 +495,7 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
>  		uint32_t major, uint32_t minor)
>  {
>  	struct drm_device *dev = mdp5_kms->dev;
> -	struct platform_device *pdev = dev->platformdev;
> +	struct platform_device *pdev = to_platform_device(dev->dev);
>  	struct mdp5_cfg_handler *cfg_handler;
>  	struct mdp5_cfg_platform *pconfig;
>  	int i, ret = 0;
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
> index d444a6901fff..f8f48d014978 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
> @@ -160,7 +160,7 @@ void msm_mdss_destroy(struct drm_device *dev)
>
>  int msm_mdss_init(struct drm_device *dev)
>  {
> -	struct platform_device *pdev = dev->platformdev;
> +	struct platform_device *pdev = to_platform_device(dev->dev);
>  	struct msm_drm_private *priv = dev->dev_private;
>  	struct msm_mdss *mdss;
>  	int ret;
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index e29bb66f55b1..12548642b227 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -364,7 +364,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
>  	}
>
>  	platform_set_drvdata(pdev, ddev);
> -	ddev->platformdev = pdev;
>
>  	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>  	if (!priv) {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 59348fc41c77..6082e184007d 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -106,7 +106,7 @@ nouveau_name(struct drm_device *dev)
>  	if (dev->pdev)
>  		return nouveau_pci_name(dev->pdev);
>  	else
> -		return nouveau_platform_name(dev->platformdev);
> +		return nouveau_platform_name(to_platform_device(dev->dev));
>  }
>
>  static int
> @@ -1089,7 +1089,6 @@ nouveau_platform_device_create(const struct nvkm_device_tegra_func *func,
>  		goto err_free;
>  	}
>
> -	drm->platformdev = pdev;
>  	platform_set_drvdata(pdev, drm);
>
>  	return drm;
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index ff71e25ab5bf..0df0de397d2c 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -388,8 +388,6 @@ static int sti_bind(struct device *dev)
>  	if (IS_ERR(ddev))
>  		return PTR_ERR(ddev);
>
> -	ddev->platformdev = to_platform_device(dev);
> -
>  	ret = sti_init(ddev);
>  	if (ret)
>  		goto err_drm_dev_unref;
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index bd0a3bd07167..8f7f6a54ee68 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -245,7 +245,6 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
>  	if (IS_ERR(ddev))
>  		return PTR_ERR(ddev);
>
> -	ddev->platformdev = pdev;
>  	ddev->dev_private = priv;
>  	platform_set_drvdata(pdev, ddev);
>  	drm_mode_config_init(ddev);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 0d6c1a13f533..4cc27449d6d7 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -609,7 +609,6 @@ struct drm_device {
>  	struct pci_controller *hose;
>  #endif
>
> -	struct platform_device *platformdev; /**< Platform device struture */
>  	struct virtio_device *virtdev;
>
>  	struct drm_sg_mem *sg;	/**< Scatter gather memory */
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH] drm: Remove the struct drm_device platformdev field
  2016-12-17 22:39 ` [PATCH] drm: Remove the struct drm_device platformdev field Laurent Pinchart
                     ` (2 preceding siblings ...)
  2017-01-03 13:49   ` Vincent ABRIOU
@ 2017-01-03 14:00   ` Russell King - ARM Linux
  2017-01-03 15:29   ` Rob Clark
  2017-01-04  1:22   ` Xinwei Kong
  5 siblings, 0 replies; 26+ messages in thread
From: Russell King - ARM Linux @ 2017-01-03 14:00 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Chen Feng, dri-devel, Xinliang Liu, Xinwei Kong, Jyri Sarha,
	Rongrong Zou, Vincent Abriou, Ben Skeggs

On Sun, Dec 18, 2016 at 12:39:16AM +0200, Laurent Pinchart wrote:
> The field contains a pointer to the parent platform device of the DRM
> device. As struct drm_device also contains a dev pointer to the struct
> device embedded in the platform_device structure, the platformdev field
> is redundant. Remove it and use the dev pointer directly.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/armada/armada_drv.c             | 3 +--
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 5 ++---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 --
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c         | 2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c         | 2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c        | 2 +-
>  drivers/gpu/drm/msm/msm_drv.c                   | 1 -
>  drivers/gpu/drm/nouveau/nouveau_drm.c           | 3 +--
>  drivers/gpu/drm/sti/sti_drv.c                   | 2 --
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c             | 1 -
>  include/drm/drmP.h                              | 1 -
>  11 files changed, 7 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> index 07086b427c22..f6442ed23bcd 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -154,10 +154,9 @@ static int armada_drm_bind(struct device *dev)
>  		return ret;
>  	}
>  
> -	priv->drm.platformdev = to_platform_device(dev);
>  	priv->drm.dev_private = priv;
>  
> -	platform_set_drvdata(priv->drm.platformdev, &priv->drm);
> +	dev_set_drvdata(dev, &priv->drm);
>  
>  	INIT_WORK(&priv->fb_unref_work, armada_drm_unref_work);
>  	INIT_KFIFO(priv->fb_unref);

For Armada,

Acked-by: Russell King <rmk+kernel@armlinux.org.uk>

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH] drm: Remove the struct drm_device platformdev field
  2016-12-17 22:39 ` [PATCH] drm: Remove the struct drm_device platformdev field Laurent Pinchart
                     ` (3 preceding siblings ...)
  2017-01-03 14:00   ` Russell King - ARM Linux
@ 2017-01-03 15:29   ` Rob Clark
  2017-01-04  1:22   ` Xinwei Kong
  5 siblings, 0 replies; 26+ messages in thread
From: Rob Clark @ 2017-01-03 15:29 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Chen Feng, Russell King, dri-devel, Xinliang Liu, Xinwei Kong,
	Jyri Sarha, Rongrong Zou, Vincent Abriou, Ben Skeggs

On Sat, Dec 17, 2016 at 5:39 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> The field contains a pointer to the parent platform device of the DRM
> device. As struct drm_device also contains a dev pointer to the struct
> device embedded in the platform_device structure, the platformdev field
> is redundant. Remove it and use the dev pointer directly.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

for msm,

Acked-by: Rob Clark <robdclark@gmail.com>

> ---
>  drivers/gpu/drm/armada/armada_drv.c             | 3 +--
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 5 ++---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 --
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c         | 2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c         | 2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c        | 2 +-
>  drivers/gpu/drm/msm/msm_drv.c                   | 1 -
>  drivers/gpu/drm/nouveau/nouveau_drm.c           | 3 +--
>  drivers/gpu/drm/sti/sti_drv.c                   | 2 --
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c             | 1 -
>  include/drm/drmP.h                              | 1 -
>  11 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> index 07086b427c22..f6442ed23bcd 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -154,10 +154,9 @@ static int armada_drm_bind(struct device *dev)
>                 return ret;
>         }
>
> -       priv->drm.platformdev = to_platform_device(dev);
>         priv->drm.dev_private = priv;
>
> -       platform_set_drvdata(priv->drm.platformdev, &priv->drm);
> +       dev_set_drvdata(dev, &priv->drm);
>
>         INIT_WORK(&priv->fb_unref_work, armada_drm_unref_work);
>         INIT_KFIFO(priv->fb_unref);
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index afc2b5d2d5f0..ec1c70d1b682 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -975,7 +975,7 @@ static int ade_dts_parse(struct platform_device *pdev, struct ade_hw_ctx *ctx)
>
>  static int ade_drm_init(struct drm_device *dev)
>  {
> -       struct platform_device *pdev = dev->platformdev;
> +       struct platform_device *pdev = to_platform_device(dev->dev);
>         struct ade_data *ade;
>         struct ade_hw_ctx *ctx;
>         struct ade_crtc *acrtc;
> @@ -1036,8 +1036,7 @@ static int ade_drm_init(struct drm_device *dev)
>
>  static void ade_drm_cleanup(struct drm_device *dev)
>  {
> -       struct platform_device *pdev = dev->platformdev;
> -       struct ade_data *ade = platform_get_drvdata(pdev);
> +       struct ade_data *ade = dev_get_drvdata(dev->dev);
>         struct drm_crtc *crtc = &ade->acrtc.base;
>
>         drm_crtc_cleanup(crtc);
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> index ebd5f4fe4c23..842f70251691 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> @@ -209,8 +209,6 @@ static int kirin_drm_bind(struct device *dev)
>         if (IS_ERR(drm_dev))
>                 return PTR_ERR(drm_dev);
>
> -       drm_dev->platformdev = to_platform_device(dev);
> -
>         ret = kirin_drm_kms_init(drm_dev);
>         if (ret)
>                 goto err_drm_dev_unref;
> diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> index b782efd4b95f..e8e14a502d21 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> @@ -438,7 +438,7 @@ static int modeset_init(struct mdp4_kms *mdp4_kms)
>
>  struct msm_kms *mdp4_kms_init(struct drm_device *dev)
>  {
> -       struct platform_device *pdev = dev->platformdev;
> +       struct platform_device *pdev = to_platform_device(dev->dev);
>         struct mdp4_platform_config *config = mdp4_get_config(pdev);
>         struct mdp4_kms *mdp4_kms;
>         struct msm_kms *kms = NULL;
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> index 618b2ffed9b4..70eae85cf1de 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> @@ -495,7 +495,7 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
>                 uint32_t major, uint32_t minor)
>  {
>         struct drm_device *dev = mdp5_kms->dev;
> -       struct platform_device *pdev = dev->platformdev;
> +       struct platform_device *pdev = to_platform_device(dev->dev);
>         struct mdp5_cfg_handler *cfg_handler;
>         struct mdp5_cfg_platform *pconfig;
>         int i, ret = 0;
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
> index d444a6901fff..f8f48d014978 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
> @@ -160,7 +160,7 @@ void msm_mdss_destroy(struct drm_device *dev)
>
>  int msm_mdss_init(struct drm_device *dev)
>  {
> -       struct platform_device *pdev = dev->platformdev;
> +       struct platform_device *pdev = to_platform_device(dev->dev);
>         struct msm_drm_private *priv = dev->dev_private;
>         struct msm_mdss *mdss;
>         int ret;
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index e29bb66f55b1..12548642b227 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -364,7 +364,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
>         }
>
>         platform_set_drvdata(pdev, ddev);
> -       ddev->platformdev = pdev;
>
>         priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>         if (!priv) {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 59348fc41c77..6082e184007d 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -106,7 +106,7 @@ nouveau_name(struct drm_device *dev)
>         if (dev->pdev)
>                 return nouveau_pci_name(dev->pdev);
>         else
> -               return nouveau_platform_name(dev->platformdev);
> +               return nouveau_platform_name(to_platform_device(dev->dev));
>  }
>
>  static int
> @@ -1089,7 +1089,6 @@ nouveau_platform_device_create(const struct nvkm_device_tegra_func *func,
>                 goto err_free;
>         }
>
> -       drm->platformdev = pdev;
>         platform_set_drvdata(pdev, drm);
>
>         return drm;
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index ff71e25ab5bf..0df0de397d2c 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -388,8 +388,6 @@ static int sti_bind(struct device *dev)
>         if (IS_ERR(ddev))
>                 return PTR_ERR(ddev);
>
> -       ddev->platformdev = to_platform_device(dev);
> -
>         ret = sti_init(ddev);
>         if (ret)
>                 goto err_drm_dev_unref;
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index bd0a3bd07167..8f7f6a54ee68 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -245,7 +245,6 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
>         if (IS_ERR(ddev))
>                 return PTR_ERR(ddev);
>
> -       ddev->platformdev = pdev;
>         ddev->dev_private = priv;
>         platform_set_drvdata(pdev, ddev);
>         drm_mode_config_init(ddev);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 0d6c1a13f533..4cc27449d6d7 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -609,7 +609,6 @@ struct drm_device {
>         struct pci_controller *hose;
>  #endif
>
> -       struct platform_device *platformdev; /**< Platform device struture */
>         struct virtio_device *virtdev;
>
>         struct drm_sg_mem *sg;  /**< Scatter gather memory */
> --
> Regards,
>
> Laurent Pinchart
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH] drm: Remove the struct drm_device platformdev field
  2016-12-17 22:39 ` [PATCH] drm: Remove the struct drm_device platformdev field Laurent Pinchart
                     ` (4 preceding siblings ...)
  2017-01-03 15:29   ` Rob Clark
@ 2017-01-04  1:22   ` Xinwei Kong
  5 siblings, 0 replies; 26+ messages in thread
From: Xinwei Kong @ 2017-01-04  1:22 UTC (permalink / raw)
  To: Laurent Pinchart, dri-devel
  Cc: Chen Feng, Russell King, Jyri Sarha, Xinliang Liu, Ben Skeggs,
	Rongrong Zou, Vincent Abriou

On 2016/12/18 6:39, Laurent Pinchart wrote:
> The field contains a pointer to the parent platform device of the DRM
> device. As struct drm_device also contains a dev pointer to the struct
> device embedded in the platform_device structure, the platformdev field
> is redundant. Remove it and use the dev pointer directly.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>   drivers/gpu/drm/armada/armada_drv.c             | 3 +--
>   drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 5 ++---
>   drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 --
>   drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c         | 2 +-
>   drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c         | 2 +-
>   drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c        | 2 +-
>   drivers/gpu/drm/msm/msm_drv.c                   | 1 -
>   drivers/gpu/drm/nouveau/nouveau_drm.c           | 3 +--
>   drivers/gpu/drm/sti/sti_drv.c                   | 2 --
>   drivers/gpu/drm/tilcdc/tilcdc_drv.c             | 1 -
>   include/drm/drmP.h                              | 1 -
>   11 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> index 07086b427c22..f6442ed23bcd 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -154,10 +154,9 @@ static int armada_drm_bind(struct device *dev)
>   		return ret;
>   	}
>   
> -	priv->drm.platformdev = to_platform_device(dev);
>   	priv->drm.dev_private = priv;
>   
> -	platform_set_drvdata(priv->drm.platformdev, &priv->drm);
> +	dev_set_drvdata(dev, &priv->drm);
>   
>   	INIT_WORK(&priv->fb_unref_work, armada_drm_unref_work);
>   	INIT_KFIFO(priv->fb_unref);
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index afc2b5d2d5f0..ec1c70d1b682 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -975,7 +975,7 @@ static int ade_dts_parse(struct platform_device *pdev, struct ade_hw_ctx *ctx)
>   
>   static int ade_drm_init(struct drm_device *dev)
>   {
> -	struct platform_device *pdev = dev->platformdev;
> +	struct platform_device *pdev = to_platform_device(dev->dev);
>   	struct ade_data *ade;
>   	struct ade_hw_ctx *ctx;
>   	struct ade_crtc *acrtc;
> @@ -1036,8 +1036,7 @@ static int ade_drm_init(struct drm_device *dev)
>   
>   static void ade_drm_cleanup(struct drm_device *dev)
>   {
> -	struct platform_device *pdev = dev->platformdev;
> -	struct ade_data *ade = platform_get_drvdata(pdev);
> +	struct ade_data *ade = dev_get_drvdata(dev->dev);
>   	struct drm_crtc *crtc = &ade->acrtc.base;
>   
>   	drm_crtc_cleanup(crtc);
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> index ebd5f4fe4c23..842f70251691 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> @@ -209,8 +209,6 @@ static int kirin_drm_bind(struct device *dev)
>   	if (IS_ERR(drm_dev))
>   		return PTR_ERR(drm_dev);
>   
> -	drm_dev->platformdev = to_platform_device(dev);
> -
>   	ret = kirin_drm_kms_init(drm_dev);
>   	if (ret)
>   		goto err_drm_dev_unref;
> diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> index b782efd4b95f..e8e14a502d21 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
> @@ -438,7 +438,7 @@ static int modeset_init(struct mdp4_kms *mdp4_kms)
>   
>   struct msm_kms *mdp4_kms_init(struct drm_device *dev)
>   {
> -	struct platform_device *pdev = dev->platformdev;
> +	struct platform_device *pdev = to_platform_device(dev->dev);
>   	struct mdp4_platform_config *config = mdp4_get_config(pdev);
>   	struct mdp4_kms *mdp4_kms;
>   	struct msm_kms *kms = NULL;
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> index 618b2ffed9b4..70eae85cf1de 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> @@ -495,7 +495,7 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
>   		uint32_t major, uint32_t minor)
>   {
>   	struct drm_device *dev = mdp5_kms->dev;
> -	struct platform_device *pdev = dev->platformdev;
> +	struct platform_device *pdev = to_platform_device(dev->dev);
>   	struct mdp5_cfg_handler *cfg_handler;
>   	struct mdp5_cfg_platform *pconfig;
>   	int i, ret = 0;
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
> index d444a6901fff..f8f48d014978 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c
> @@ -160,7 +160,7 @@ void msm_mdss_destroy(struct drm_device *dev)
>   
>   int msm_mdss_init(struct drm_device *dev)
>   {
> -	struct platform_device *pdev = dev->platformdev;
> +	struct platform_device *pdev = to_platform_device(dev->dev);
>   	struct msm_drm_private *priv = dev->dev_private;
>   	struct msm_mdss *mdss;
>   	int ret;
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index e29bb66f55b1..12548642b227 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -364,7 +364,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
>   	}
>   
>   	platform_set_drvdata(pdev, ddev);
> -	ddev->platformdev = pdev;
>   
>   	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>   	if (!priv) {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 59348fc41c77..6082e184007d 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -106,7 +106,7 @@ nouveau_name(struct drm_device *dev)
>   	if (dev->pdev)
>   		return nouveau_pci_name(dev->pdev);
>   	else
> -		return nouveau_platform_name(dev->platformdev);
> +		return nouveau_platform_name(to_platform_device(dev->dev));
>   }
>   
>   static int
> @@ -1089,7 +1089,6 @@ nouveau_platform_device_create(const struct nvkm_device_tegra_func *func,
>   		goto err_free;
>   	}
>   
> -	drm->platformdev = pdev;
>   	platform_set_drvdata(pdev, drm);
>   
>   	return drm;
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index ff71e25ab5bf..0df0de397d2c 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -388,8 +388,6 @@ static int sti_bind(struct device *dev)
>   	if (IS_ERR(ddev))
>   		return PTR_ERR(ddev);
>   
> -	ddev->platformdev = to_platform_device(dev);
> -
>   	ret = sti_init(ddev);
>   	if (ret)
>   		goto err_drm_dev_unref;
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index bd0a3bd07167..8f7f6a54ee68 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -245,7 +245,6 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
>   	if (IS_ERR(ddev))
>   		return PTR_ERR(ddev);
>   
> -	ddev->platformdev = pdev;
>   	ddev->dev_private = priv;
>   	platform_set_drvdata(pdev, ddev);
>   	drm_mode_config_init(ddev);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 0d6c1a13f533..4cc27449d6d7 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -609,7 +609,6 @@ struct drm_device {
>   	struct pci_controller *hose;
>   #endif
>   
> -	struct platform_device *platformdev; /**< Platform device struture */
>   	struct virtio_device *virtdev;
>   
>   	struct drm_sg_mem *sg;	/**< Scatter gather memory */

Acked-by: Xinwei Kong<kong.kongxinwei@hisilicon.com>


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

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

end of thread, other threads:[~2017-01-04  1:30 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-13 19:34 [PATCH 0/2] exynosdrm: Remove .load/.unload midlayer Laurent Pinchart
2016-12-13 19:34 ` [PATCH 1/2] drm: exynos: Perform initialization/cleanup at probe/remove time Laurent Pinchart
2016-12-13 21:03   ` Sean Paul
2016-12-13 21:10   ` Daniel Vetter
2016-12-13 21:30     ` Laurent Pinchart
2016-12-13 21:32       ` Sean Paul
2016-12-13 21:49         ` Daniel Vetter
2016-12-13 21:54           ` Laurent Pinchart
2016-12-16 18:02   ` Daniel Stone
2016-12-17 21:49     ` Laurent Pinchart
2016-12-17  0:33   ` Inki Dae
2016-12-17 22:12     ` Laurent Pinchart
2016-12-19  6:32       ` Inki Dae
2016-12-17 22:29   ` [PATCH v2] " Laurent Pinchart
2016-12-19  9:29     ` Daniel Stone
2016-12-13 19:34 ` [PATCH 2/2] drm: Remove unused drm_platform midlayer Laurent Pinchart
2016-12-13 21:03   ` Daniel Vetter
2016-12-13 21:04     ` Sean Paul
2016-12-17 22:39 ` [PATCH] drm: Remove the struct drm_device platformdev field Laurent Pinchart
2016-12-18 13:16   ` Daniel Vetter
2016-12-18 20:24     ` Laurent Pinchart
2016-12-19  8:30   ` Jyri Sarha
2017-01-03 13:49   ` Vincent ABRIOU
2017-01-03 14:00   ` Russell King - ARM Linux
2017-01-03 15:29   ` Rob Clark
2017-01-04  1:22   ` Xinwei Kong

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