linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver
@ 2020-03-06  3:42 Tian Tao
  2020-03-06  3:42 ` [PATCH] drm/hisilicon: Add the shutdown for hibmc_pci_driver Tian Tao
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Tian Tao @ 2020-03-06  3:42 UTC (permalink / raw)
  To: puck.chen, airlied, daniel, tzimmermann, kraxel,
	alexander.deucher, tglx, dri-devel, xinliang.liu, linux-kernel
  Cc: linuxarm

using the load and unload function provided by drm framework instead of
doing the same work in the hibmc_pci_probe and do some code cleanup.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
Signed-off-by: Gong junjie <gongjunjie2@huawei.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 62 +++++++++----------------
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h |  2 +
 2 files changed, 24 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 79a180a..51f1c70 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -23,7 +23,7 @@
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_vblank.h>
-
+#include <drm/drm_pci.h>
 #include "hibmc_drm_drv.h"
 #include "hibmc_drm_regs.h"
 
@@ -49,6 +49,8 @@ static irqreturn_t hibmc_drm_interrupt(int irq, void *arg)
 
 static struct drm_driver hibmc_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+	.load			= hibmc_load,
+	.unload			= hibmc_unload,
 	.fops			= &hibmc_fops,
 	.name			= "hibmc",
 	.date			= "20160828",
@@ -232,6 +234,21 @@ static int hibmc_hw_map(struct hibmc_drm_private *priv)
 	return 0;
 }
 
+static void hibmc_hw_unmap(struct hibmc_drm_private *priv)
+{
+	struct drm_device *dev = priv->dev;
+
+	if (priv->mmio) {
+		devm_iounmap(dev->dev, priv->mmio);
+		priv->mmio = NULL;
+	}
+
+	if (priv->fb_map) {
+		devm_iounmap(dev->dev, priv->fb_map);
+		priv->fb_map = NULL;
+	}
+}
+
 static int hibmc_hw_init(struct hibmc_drm_private *priv)
 {
 	int ret;
@@ -245,7 +262,7 @@ static int hibmc_hw_init(struct hibmc_drm_private *priv)
 	return 0;
 }
 
-static int hibmc_unload(struct drm_device *dev)
+void hibmc_unload(struct drm_device *dev)
 {
 	struct hibmc_drm_private *priv = dev->dev_private;
 
@@ -258,11 +275,12 @@ static int hibmc_unload(struct drm_device *dev)
 
 	hibmc_kms_fini(priv);
 	hibmc_mm_fini(priv);
+	hibmc_hw_unmap(priv);
 	dev->dev_private = NULL;
 	return 0;
 }
 
-static int hibmc_load(struct drm_device *dev)
+int hibmc_load(struct drm_device *dev, unsigned long flags)
 {
 	struct hibmc_drm_private *priv;
 	int ret;
@@ -332,43 +350,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		return ret;
 
-	dev = drm_dev_alloc(&hibmc_driver, &pdev->dev);
-	if (IS_ERR(dev)) {
-		DRM_ERROR("failed to allocate drm_device\n");
-		return PTR_ERR(dev);
-	}
-
-	dev->pdev = pdev;
-	pci_set_drvdata(pdev, dev);
-
-	ret = pci_enable_device(pdev);
-	if (ret) {
-		DRM_ERROR("failed to enable pci device: %d\n", ret);
-		goto err_free;
-	}
-
-	ret = hibmc_load(dev);
-	if (ret) {
-		DRM_ERROR("failed to load hibmc: %d\n", ret);
-		goto err_disable;
-	}
-
-	ret = drm_dev_register(dev, 0);
-	if (ret) {
-		DRM_ERROR("failed to register drv for userspace access: %d\n",
-			  ret);
-		goto err_unload;
-	}
-	return 0;
-
-err_unload:
-	hibmc_unload(dev);
-err_disable:
-	pci_disable_device(pdev);
-err_free:
-	drm_dev_put(dev);
-
-	return ret;
+	return drm_get_pci_dev(pdev, ent, &hibmc_driver);
 }
 
 static void hibmc_pci_remove(struct pci_dev *pdev)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index 50a0c1f..4e89cd7 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -37,6 +37,8 @@ void hibmc_set_power_mode(struct hibmc_drm_private *priv,
 void hibmc_set_current_gate(struct hibmc_drm_private *priv,
 			    unsigned int gate);
 
+int hibmc_load(struct drm_device *dev, unsigned long flags);
+void hibmc_unload(struct drm_device *dev);
 int hibmc_de_init(struct hibmc_drm_private *priv);
 int hibmc_vdac_init(struct hibmc_drm_private *priv);
 
-- 
2.7.4


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

* [PATCH] drm/hisilicon: Add the shutdown for hibmc_pci_driver
  2020-03-06  3:42 [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
@ 2020-03-06  3:42 ` Tian Tao
  2020-03-06  3:58   ` 答复: " tiantao (H)
  2020-03-06  3:43 ` [PATCH] drm/hisilicon: Code cleanup for hibmc_drv_vdac Tian Tao
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Tian Tao @ 2020-03-06  3:42 UTC (permalink / raw)
  To: puck.chen, airlied, daniel, tzimmermann, kraxel,
	alexander.deucher, tglx, dri-devel, xinliang.liu, linux-kernel
  Cc: linuxarm

add the shutdown function to release the resource.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 51f1c70..0e58455d 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -357,9 +357,14 @@ static void hibmc_pci_remove(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
-	drm_dev_unregister(dev);
-	hibmc_unload(dev);
 	drm_dev_put(dev);
+	pci_disable_device(pdev);
+
+}
+
+static void hibmc_pci_shutdown(struct pci_dev *pdev)
+{
+	hibmc_pci_remove(pdev);
 }
 
 static struct pci_device_id hibmc_pci_table[] = {
@@ -372,6 +377,7 @@ static struct pci_driver hibmc_pci_driver = {
 	.id_table =	hibmc_pci_table,
 	.probe =	hibmc_pci_probe,
 	.remove =	hibmc_pci_remove,
+	.shutdown = hibmc_pci_shutdown,
 	.driver.pm =    &hibmc_pm_ops,
 };
 
-- 
2.7.4


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

* [PATCH] drm/hisilicon: Code cleanup for hibmc_drv_vdac
  2020-03-06  3:42 [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
  2020-03-06  3:42 ` [PATCH] drm/hisilicon: Add the shutdown for hibmc_pci_driver Tian Tao
@ 2020-03-06  3:43 ` Tian Tao
  2020-03-06  3:59   ` 答复: " tiantao (H)
  2020-03-06  3:43 ` [PATCH 1/4] drm/hisilicon: Enforce 128-byte stride alignment to fix the hardware limitation Tian Tao
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Tian Tao @ 2020-03-06  3:43 UTC (permalink / raw)
  To: puck.chen, airlied, daniel, tzimmermann, kraxel,
	alexander.deucher, tglx, dri-devel, xinliang.liu, linux-kernel
  Cc: linuxarm

code cleanup for hibmc_drv_vdac.c, no actual function changes.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
Signed-off-by: Gong junjie <gongjunjie2@huawei.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 49 ++++++++----------------
 1 file changed, 16 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
index 678ac2e..f0e6bb8 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
@@ -52,32 +52,6 @@ static const struct drm_connector_funcs hibmc_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static struct drm_connector *
-hibmc_connector_init(struct hibmc_drm_private *priv)
-{
-	struct drm_device *dev = priv->dev;
-	struct drm_connector *connector;
-	int ret;
-
-	connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL);
-	if (!connector) {
-		DRM_ERROR("failed to alloc memory when init connector\n");
-		return ERR_PTR(-ENOMEM);
-	}
-
-	ret = drm_connector_init(dev, connector,
-				 &hibmc_connector_funcs,
-				 DRM_MODE_CONNECTOR_VGA);
-	if (ret) {
-		DRM_ERROR("failed to init connector: %d\n", ret);
-		return ERR_PTR(ret);
-	}
-	drm_connector_helper_add(connector,
-				 &hibmc_connector_helper_funcs);
-
-	return connector;
-}
-
 static void hibmc_encoder_mode_set(struct drm_encoder *encoder,
 				   struct drm_display_mode *mode,
 				   struct drm_display_mode *adj_mode)
@@ -109,13 +83,6 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
 	struct drm_connector *connector;
 	int ret;
 
-	connector = hibmc_connector_init(priv);
-	if (IS_ERR(connector)) {
-		DRM_ERROR("failed to create connector: %ld\n",
-			  PTR_ERR(connector));
-		return PTR_ERR(connector);
-	}
-
 	encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL);
 	if (!encoder) {
 		DRM_ERROR("failed to alloc memory when init encoder\n");
@@ -131,6 +98,22 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
 	}
 
 	drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
+	connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL);
+	if (!connector) {
+		DRM_ERROR("failed to alloc memory when init connector\n");
+		return -ENOMEM;
+	}
+
+	ret = drm_connector_init(dev, connector,
+				 &hibmc_connector_funcs,
+				 DRM_MODE_CONNECTOR_VGA);
+	if (ret) {
+		DRM_ERROR("failed to init connector: %d\n", ret);
+		return ret;
+	}
+
+	drm_connector_helper_add(connector, &hibmc_connector_helper_funcs);
+	drm_connector_register(connector);
 	drm_connector_attach_encoder(connector, encoder);
 
 	return 0;
-- 
2.7.4


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

* [PATCH 1/4] drm/hisilicon: Enforce 128-byte stride alignment to fix the hardware limitation
  2020-03-06  3:42 [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
  2020-03-06  3:42 ` [PATCH] drm/hisilicon: Add the shutdown for hibmc_pci_driver Tian Tao
  2020-03-06  3:43 ` [PATCH] drm/hisilicon: Code cleanup for hibmc_drv_vdac Tian Tao
@ 2020-03-06  3:43 ` Tian Tao
  2020-04-03  2:00   ` Xinliang Liu
  2020-03-06  3:43 ` [PATCH 2/4] drm/hisilicon: Code cleanup for hibmc_drv_vdac Tian Tao
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Tian Tao @ 2020-03-06  3:43 UTC (permalink / raw)
  To: puck.chen, airlied, daniel, tzimmermann, kraxel,
	alexander.deucher, tglx, dri-devel, xinliang.liu, linux-kernel
  Cc: linuxarm

because the hardware limitation,The initial color depth must set to 32bpp
and must set the FB Offset of the display hardware to 128Byte alignment,
which is used to solve the display problem at 800x600 and 1440x900
resolution under 16bpp.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
Signed-off-by: Gong junjie <gongjunjie2@huawei.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c  | 9 +++++----
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 4 ++--
 drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c     | 2 +-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
index 55b46a7..cc70e83 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
@@ -94,6 +94,10 @@ static int hibmc_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
+	if (state->fb->pitches[0] % 128 != 0) {
+		DRM_DEBUG_ATOMIC("wrong stride with 128-byte aligned\n");
+		return -EINVAL;
+	}
 	return 0;
 }
 
@@ -119,11 +123,8 @@ static void hibmc_plane_atomic_update(struct drm_plane *plane,
 	writel(gpu_addr, priv->mmio + HIBMC_CRT_FB_ADDRESS);
 
 	reg = state->fb->width * (state->fb->format->cpp[0]);
-	/* now line_pad is 16 */
-	reg = PADDING(16, reg);
 
-	line_l = state->fb->width * state->fb->format->cpp[0];
-	line_l = PADDING(16, line_l);
+	line_l = state->fb->pitches[0];
 	writel(HIBMC_FIELD(HIBMC_CRT_FB_WIDTH_WIDTH, reg) |
 	       HIBMC_FIELD(HIBMC_CRT_FB_WIDTH_OFFS, line_l),
 	       priv->mmio + HIBMC_CRT_FB_WIDTH);
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 222356a..79a180a 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -94,7 +94,7 @@ static int hibmc_kms_init(struct hibmc_drm_private *priv)
 	priv->dev->mode_config.max_height = 1200;
 
 	priv->dev->mode_config.fb_base = priv->fb_base;
-	priv->dev->mode_config.preferred_depth = 24;
+	priv->dev->mode_config.preferred_depth = 32;
 	priv->dev->mode_config.prefer_shadow = 1;
 
 	priv->dev->mode_config.funcs = (void *)&hibmc_mode_funcs;
@@ -307,7 +307,7 @@ static int hibmc_load(struct drm_device *dev)
 	/* reset all the states of crtc/plane/encoder/connector */
 	drm_mode_config_reset(dev);
 
-	ret = drm_fbdev_generic_setup(dev, 16);
+	ret = drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth);
 	if (ret) {
 		DRM_ERROR("failed to initialize fbdev: %d\n", ret);
 		goto err;
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
index 99397ac..322bd54 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
@@ -50,7 +50,7 @@ void hibmc_mm_fini(struct hibmc_drm_private *hibmc)
 int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
 		      struct drm_mode_create_dumb *args)
 {
-	return drm_gem_vram_fill_create_dumb(file, dev, 0, 16, args);
+	return drm_gem_vram_fill_create_dumb(file, dev, 0, 128, args);
 }
 
 const struct drm_mode_config_funcs hibmc_mode_funcs = {
-- 
2.7.4


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

* [PATCH 2/4] drm/hisilicon: Code cleanup for hibmc_drv_vdac
  2020-03-06  3:42 [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
                   ` (2 preceding siblings ...)
  2020-03-06  3:43 ` [PATCH 1/4] drm/hisilicon: Enforce 128-byte stride alignment to fix the hardware limitation Tian Tao
@ 2020-03-06  3:43 ` Tian Tao
  2020-04-03  2:26   ` Xinliang Liu
  2020-03-06  3:43 ` [PATCH 3/4] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Tian Tao @ 2020-03-06  3:43 UTC (permalink / raw)
  To: puck.chen, airlied, daniel, tzimmermann, kraxel,
	alexander.deucher, tglx, dri-devel, xinliang.liu, linux-kernel
  Cc: linuxarm

code cleanup for hibmc_drv_vdac.c, no actual function changes.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
Signed-off-by: Gong junjie <gongjunjie2@huawei.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 49 ++++++++----------------
 1 file changed, 16 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
index 678ac2e..f0e6bb8 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
@@ -52,32 +52,6 @@ static const struct drm_connector_funcs hibmc_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static struct drm_connector *
-hibmc_connector_init(struct hibmc_drm_private *priv)
-{
-	struct drm_device *dev = priv->dev;
-	struct drm_connector *connector;
-	int ret;
-
-	connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL);
-	if (!connector) {
-		DRM_ERROR("failed to alloc memory when init connector\n");
-		return ERR_PTR(-ENOMEM);
-	}
-
-	ret = drm_connector_init(dev, connector,
-				 &hibmc_connector_funcs,
-				 DRM_MODE_CONNECTOR_VGA);
-	if (ret) {
-		DRM_ERROR("failed to init connector: %d\n", ret);
-		return ERR_PTR(ret);
-	}
-	drm_connector_helper_add(connector,
-				 &hibmc_connector_helper_funcs);
-
-	return connector;
-}
-
 static void hibmc_encoder_mode_set(struct drm_encoder *encoder,
 				   struct drm_display_mode *mode,
 				   struct drm_display_mode *adj_mode)
@@ -109,13 +83,6 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
 	struct drm_connector *connector;
 	int ret;
 
-	connector = hibmc_connector_init(priv);
-	if (IS_ERR(connector)) {
-		DRM_ERROR("failed to create connector: %ld\n",
-			  PTR_ERR(connector));
-		return PTR_ERR(connector);
-	}
-
 	encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL);
 	if (!encoder) {
 		DRM_ERROR("failed to alloc memory when init encoder\n");
@@ -131,6 +98,22 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
 	}
 
 	drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
+	connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL);
+	if (!connector) {
+		DRM_ERROR("failed to alloc memory when init connector\n");
+		return -ENOMEM;
+	}
+
+	ret = drm_connector_init(dev, connector,
+				 &hibmc_connector_funcs,
+				 DRM_MODE_CONNECTOR_VGA);
+	if (ret) {
+		DRM_ERROR("failed to init connector: %d\n", ret);
+		return ret;
+	}
+
+	drm_connector_helper_add(connector, &hibmc_connector_helper_funcs);
+	drm_connector_register(connector);
 	drm_connector_attach_encoder(connector, encoder);
 
 	return 0;
-- 
2.7.4


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

* [PATCH 3/4] drm/hisilicon: Add the load and unload for hibmc_driver
  2020-03-06  3:42 [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
                   ` (3 preceding siblings ...)
  2020-03-06  3:43 ` [PATCH 2/4] drm/hisilicon: Code cleanup for hibmc_drv_vdac Tian Tao
@ 2020-03-06  3:43 ` Tian Tao
  2020-03-06  3:43 ` [PATCH 4/4] drm/hisilicon: Add the shutdown for hibmc_pci_driver Tian Tao
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tian Tao @ 2020-03-06  3:43 UTC (permalink / raw)
  To: puck.chen, airlied, daniel, tzimmermann, kraxel,
	alexander.deucher, tglx, dri-devel, xinliang.liu, linux-kernel
  Cc: linuxarm

using the load and unload function provided by drm framework instead of
doing the same work in the hibmc_pci_probe and do some code cleanup.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
Signed-off-by: Gong junjie <gongjunjie2@huawei.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 62 +++++++++----------------
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h |  2 +
 2 files changed, 24 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 79a180a..51f1c70 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -23,7 +23,7 @@
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_vblank.h>
-
+#include <drm/drm_pci.h>
 #include "hibmc_drm_drv.h"
 #include "hibmc_drm_regs.h"
 
@@ -49,6 +49,8 @@ static irqreturn_t hibmc_drm_interrupt(int irq, void *arg)
 
 static struct drm_driver hibmc_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+	.load			= hibmc_load,
+	.unload			= hibmc_unload,
 	.fops			= &hibmc_fops,
 	.name			= "hibmc",
 	.date			= "20160828",
@@ -232,6 +234,21 @@ static int hibmc_hw_map(struct hibmc_drm_private *priv)
 	return 0;
 }
 
+static void hibmc_hw_unmap(struct hibmc_drm_private *priv)
+{
+	struct drm_device *dev = priv->dev;
+
+	if (priv->mmio) {
+		devm_iounmap(dev->dev, priv->mmio);
+		priv->mmio = NULL;
+	}
+
+	if (priv->fb_map) {
+		devm_iounmap(dev->dev, priv->fb_map);
+		priv->fb_map = NULL;
+	}
+}
+
 static int hibmc_hw_init(struct hibmc_drm_private *priv)
 {
 	int ret;
@@ -245,7 +262,7 @@ static int hibmc_hw_init(struct hibmc_drm_private *priv)
 	return 0;
 }
 
-static int hibmc_unload(struct drm_device *dev)
+void hibmc_unload(struct drm_device *dev)
 {
 	struct hibmc_drm_private *priv = dev->dev_private;
 
@@ -258,11 +275,12 @@ static int hibmc_unload(struct drm_device *dev)
 
 	hibmc_kms_fini(priv);
 	hibmc_mm_fini(priv);
+	hibmc_hw_unmap(priv);
 	dev->dev_private = NULL;
 	return 0;
 }
 
-static int hibmc_load(struct drm_device *dev)
+int hibmc_load(struct drm_device *dev, unsigned long flags)
 {
 	struct hibmc_drm_private *priv;
 	int ret;
@@ -332,43 +350,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		return ret;
 
-	dev = drm_dev_alloc(&hibmc_driver, &pdev->dev);
-	if (IS_ERR(dev)) {
-		DRM_ERROR("failed to allocate drm_device\n");
-		return PTR_ERR(dev);
-	}
-
-	dev->pdev = pdev;
-	pci_set_drvdata(pdev, dev);
-
-	ret = pci_enable_device(pdev);
-	if (ret) {
-		DRM_ERROR("failed to enable pci device: %d\n", ret);
-		goto err_free;
-	}
-
-	ret = hibmc_load(dev);
-	if (ret) {
-		DRM_ERROR("failed to load hibmc: %d\n", ret);
-		goto err_disable;
-	}
-
-	ret = drm_dev_register(dev, 0);
-	if (ret) {
-		DRM_ERROR("failed to register drv for userspace access: %d\n",
-			  ret);
-		goto err_unload;
-	}
-	return 0;
-
-err_unload:
-	hibmc_unload(dev);
-err_disable:
-	pci_disable_device(pdev);
-err_free:
-	drm_dev_put(dev);
-
-	return ret;
+	return drm_get_pci_dev(pdev, ent, &hibmc_driver);
 }
 
 static void hibmc_pci_remove(struct pci_dev *pdev)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index 50a0c1f..4e89cd7 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -37,6 +37,8 @@ void hibmc_set_power_mode(struct hibmc_drm_private *priv,
 void hibmc_set_current_gate(struct hibmc_drm_private *priv,
 			    unsigned int gate);
 
+int hibmc_load(struct drm_device *dev, unsigned long flags);
+void hibmc_unload(struct drm_device *dev);
 int hibmc_de_init(struct hibmc_drm_private *priv);
 int hibmc_vdac_init(struct hibmc_drm_private *priv);
 
-- 
2.7.4


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

* [PATCH 4/4] drm/hisilicon: Add the shutdown for hibmc_pci_driver
  2020-03-06  3:42 [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
                   ` (4 preceding siblings ...)
  2020-03-06  3:43 ` [PATCH 3/4] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
@ 2020-03-06  3:43 ` Tian Tao
  2020-03-06  3:59 ` 答复: [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver tiantao (H)
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tian Tao @ 2020-03-06  3:43 UTC (permalink / raw)
  To: puck.chen, airlied, daniel, tzimmermann, kraxel,
	alexander.deucher, tglx, dri-devel, xinliang.liu, linux-kernel
  Cc: linuxarm

add the shutdown function to release the resource.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 51f1c70..0e58455d 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -357,9 +357,14 @@ static void hibmc_pci_remove(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
-	drm_dev_unregister(dev);
-	hibmc_unload(dev);
 	drm_dev_put(dev);
+	pci_disable_device(pdev);
+
+}
+
+static void hibmc_pci_shutdown(struct pci_dev *pdev)
+{
+	hibmc_pci_remove(pdev);
 }
 
 static struct pci_device_id hibmc_pci_table[] = {
@@ -372,6 +377,7 @@ static struct pci_driver hibmc_pci_driver = {
 	.id_table =	hibmc_pci_table,
 	.probe =	hibmc_pci_probe,
 	.remove =	hibmc_pci_remove,
+	.shutdown = hibmc_pci_shutdown,
 	.driver.pm =    &hibmc_pm_ops,
 };
 
-- 
2.7.4


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

* 答复: [PATCH] drm/hisilicon: Add the shutdown for hibmc_pci_driver
  2020-03-06  3:42 ` [PATCH] drm/hisilicon: Add the shutdown for hibmc_pci_driver Tian Tao
@ 2020-03-06  3:58   ` tiantao (H)
  0 siblings, 0 replies; 15+ messages in thread
From: tiantao (H) @ 2020-03-06  3:58 UTC (permalink / raw)
  To: tiantao (H), Chenfeng (puck),
	airlied, daniel, tzimmermann, kraxel, alexander.deucher, tglx,
	dri-devel, xinliang.liu, linux-kernel
  Cc: Linuxarm

Hi All:

	Sorry,please  ignore this patch.

Best

-----邮件原件-----
发件人: Linuxarm [mailto:linuxarm-bounces@huawei.com] 代表 Tian Tao
发送时间: 2020年3月6日 11:43
收件人: Chenfeng (puck) <puck.chen@hisilicon.com>; airlied@linux.ie; daniel@ffwll.ch; tzimmermann@suse.de; kraxel@redhat.com; alexander.deucher@amd.com; tglx@linutronix.de; dri-devel@lists.freedesktop.org; xinliang.liu@linaro.org; linux-kernel@vger.kernel.org
抄送: Linuxarm <linuxarm@huawei.com>
主题: [PATCH] drm/hisilicon: Add the shutdown for hibmc_pci_driver

add the shutdown function to release the resource.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 51f1c70..0e58455d 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -357,9 +357,14 @@ static void hibmc_pci_remove(struct pci_dev *pdev)  {
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
-	drm_dev_unregister(dev);
-	hibmc_unload(dev);
 	drm_dev_put(dev);
+	pci_disable_device(pdev);
+
+}
+
+static void hibmc_pci_shutdown(struct pci_dev *pdev) {
+	hibmc_pci_remove(pdev);
 }
 
 static struct pci_device_id hibmc_pci_table[] = { @@ -372,6 +377,7 @@ static struct pci_driver hibmc_pci_driver = {
 	.id_table =	hibmc_pci_table,
 	.probe =	hibmc_pci_probe,
 	.remove =	hibmc_pci_remove,
+	.shutdown = hibmc_pci_shutdown,
 	.driver.pm =    &hibmc_pm_ops,
 };
 
--
2.7.4

_______________________________________________
Linuxarm mailing list
Linuxarm@huawei.com
http://hulk.huawei.com/mailman/listinfo/linuxarm

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

* 答复: [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver
  2020-03-06  3:42 [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
                   ` (5 preceding siblings ...)
  2020-03-06  3:43 ` [PATCH 4/4] drm/hisilicon: Add the shutdown for hibmc_pci_driver Tian Tao
@ 2020-03-06  3:59 ` tiantao (H)
  2020-03-06  7:24 ` Thomas Zimmermann
  2020-03-06  9:11 ` kbuild test robot
  8 siblings, 0 replies; 15+ messages in thread
From: tiantao (H) @ 2020-03-06  3:59 UTC (permalink / raw)
  To: tiantao (H), Chenfeng (puck),
	airlied, daniel, tzimmermann, kraxel, alexander.deucher, tglx,
	dri-devel, xinliang.liu, linux-kernel
  Cc: Linuxarm

Hi All:

	Sorry,please ignore this patch.

Best

-----邮件原件-----
发件人: Linuxarm [mailto:linuxarm-bounces@huawei.com] 代表 Tian Tao
发送时间: 2020年3月6日 11:43
收件人: Chenfeng (puck) <puck.chen@hisilicon.com>; airlied@linux.ie; daniel@ffwll.ch; tzimmermann@suse.de; kraxel@redhat.com; alexander.deucher@amd.com; tglx@linutronix.de; dri-devel@lists.freedesktop.org; xinliang.liu@linaro.org; linux-kernel@vger.kernel.org
抄送: Linuxarm <linuxarm@huawei.com>
主题: [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver

using the load and unload function provided by drm framework instead of doing the same work in the hibmc_pci_probe and do some code cleanup.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
Signed-off-by: Gong junjie <gongjunjie2@huawei.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 62 +++++++++----------------  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h |  2 +
 2 files changed, 24 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 79a180a..51f1c70 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -23,7 +23,7 @@
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_vblank.h>
-
+#include <drm/drm_pci.h>
 #include "hibmc_drm_drv.h"
 #include "hibmc_drm_regs.h"
 
@@ -49,6 +49,8 @@ static irqreturn_t hibmc_drm_interrupt(int irq, void *arg)
 
 static struct drm_driver hibmc_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+	.load			= hibmc_load,
+	.unload			= hibmc_unload,
 	.fops			= &hibmc_fops,
 	.name			= "hibmc",
 	.date			= "20160828",
@@ -232,6 +234,21 @@ static int hibmc_hw_map(struct hibmc_drm_private *priv)
 	return 0;
 }
 
+static void hibmc_hw_unmap(struct hibmc_drm_private *priv) {
+	struct drm_device *dev = priv->dev;
+
+	if (priv->mmio) {
+		devm_iounmap(dev->dev, priv->mmio);
+		priv->mmio = NULL;
+	}
+
+	if (priv->fb_map) {
+		devm_iounmap(dev->dev, priv->fb_map);
+		priv->fb_map = NULL;
+	}
+}
+
 static int hibmc_hw_init(struct hibmc_drm_private *priv)  {
 	int ret;
@@ -245,7 +262,7 @@ static int hibmc_hw_init(struct hibmc_drm_private *priv)
 	return 0;
 }
 
-static int hibmc_unload(struct drm_device *dev)
+void hibmc_unload(struct drm_device *dev)
 {
 	struct hibmc_drm_private *priv = dev->dev_private;
 
@@ -258,11 +275,12 @@ static int hibmc_unload(struct drm_device *dev)
 
 	hibmc_kms_fini(priv);
 	hibmc_mm_fini(priv);
+	hibmc_hw_unmap(priv);
 	dev->dev_private = NULL;
 	return 0;
 }
 
-static int hibmc_load(struct drm_device *dev)
+int hibmc_load(struct drm_device *dev, unsigned long flags)
 {
 	struct hibmc_drm_private *priv;
 	int ret;
@@ -332,43 +350,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		return ret;
 
-	dev = drm_dev_alloc(&hibmc_driver, &pdev->dev);
-	if (IS_ERR(dev)) {
-		DRM_ERROR("failed to allocate drm_device\n");
-		return PTR_ERR(dev);
-	}
-
-	dev->pdev = pdev;
-	pci_set_drvdata(pdev, dev);
-
-	ret = pci_enable_device(pdev);
-	if (ret) {
-		DRM_ERROR("failed to enable pci device: %d\n", ret);
-		goto err_free;
-	}
-
-	ret = hibmc_load(dev);
-	if (ret) {
-		DRM_ERROR("failed to load hibmc: %d\n", ret);
-		goto err_disable;
-	}
-
-	ret = drm_dev_register(dev, 0);
-	if (ret) {
-		DRM_ERROR("failed to register drv for userspace access: %d\n",
-			  ret);
-		goto err_unload;
-	}
-	return 0;
-
-err_unload:
-	hibmc_unload(dev);
-err_disable:
-	pci_disable_device(pdev);
-err_free:
-	drm_dev_put(dev);
-
-	return ret;
+	return drm_get_pci_dev(pdev, ent, &hibmc_driver);
 }
 
 static void hibmc_pci_remove(struct pci_dev *pdev) diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index 50a0c1f..4e89cd7 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -37,6 +37,8 @@ void hibmc_set_power_mode(struct hibmc_drm_private *priv,  void hibmc_set_current_gate(struct hibmc_drm_private *priv,
 			    unsigned int gate);
 
+int hibmc_load(struct drm_device *dev, unsigned long flags); void 
+hibmc_unload(struct drm_device *dev);
 int hibmc_de_init(struct hibmc_drm_private *priv);  int hibmc_vdac_init(struct hibmc_drm_private *priv);
 
--
2.7.4

_______________________________________________
Linuxarm mailing list
Linuxarm@huawei.com
http://hulk.huawei.com/mailman/listinfo/linuxarm

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

* 答复: [PATCH] drm/hisilicon: Code cleanup for hibmc_drv_vdac
  2020-03-06  3:43 ` [PATCH] drm/hisilicon: Code cleanup for hibmc_drv_vdac Tian Tao
@ 2020-03-06  3:59   ` tiantao (H)
  0 siblings, 0 replies; 15+ messages in thread
From: tiantao (H) @ 2020-03-06  3:59 UTC (permalink / raw)
  To: tiantao (H), Chenfeng (puck),
	airlied, daniel, tzimmermann, kraxel, alexander.deucher, tglx,
	dri-devel, xinliang.liu, linux-kernel
  Cc: Linuxarm

Hi All:

	Sorry,please ignore this patch.

Best

-----邮件原件-----
发件人: Linuxarm [mailto:linuxarm-bounces@huawei.com] 代表 Tian Tao
发送时间: 2020年3月6日 11:43
收件人: Chenfeng (puck) <puck.chen@hisilicon.com>; airlied@linux.ie; daniel@ffwll.ch; tzimmermann@suse.de; kraxel@redhat.com; alexander.deucher@amd.com; tglx@linutronix.de; dri-devel@lists.freedesktop.org; xinliang.liu@linaro.org; linux-kernel@vger.kernel.org
抄送: Linuxarm <linuxarm@huawei.com>
主题: [PATCH] drm/hisilicon: Code cleanup for hibmc_drv_vdac

code cleanup for hibmc_drv_vdac.c, no actual function changes.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
Signed-off-by: Gong junjie <gongjunjie2@huawei.com>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 49 ++++++++----------------
 1 file changed, 16 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
index 678ac2e..f0e6bb8 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
@@ -52,32 +52,6 @@ static const struct drm_connector_funcs hibmc_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static struct drm_connector *
-hibmc_connector_init(struct hibmc_drm_private *priv) -{
-	struct drm_device *dev = priv->dev;
-	struct drm_connector *connector;
-	int ret;
-
-	connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL);
-	if (!connector) {
-		DRM_ERROR("failed to alloc memory when init connector\n");
-		return ERR_PTR(-ENOMEM);
-	}
-
-	ret = drm_connector_init(dev, connector,
-				 &hibmc_connector_funcs,
-				 DRM_MODE_CONNECTOR_VGA);
-	if (ret) {
-		DRM_ERROR("failed to init connector: %d\n", ret);
-		return ERR_PTR(ret);
-	}
-	drm_connector_helper_add(connector,
-				 &hibmc_connector_helper_funcs);
-
-	return connector;
-}
-
 static void hibmc_encoder_mode_set(struct drm_encoder *encoder,
 				   struct drm_display_mode *mode,
 				   struct drm_display_mode *adj_mode) @@ -109,13 +83,6 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
 	struct drm_connector *connector;
 	int ret;
 
-	connector = hibmc_connector_init(priv);
-	if (IS_ERR(connector)) {
-		DRM_ERROR("failed to create connector: %ld\n",
-			  PTR_ERR(connector));
-		return PTR_ERR(connector);
-	}
-
 	encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL);
 	if (!encoder) {
 		DRM_ERROR("failed to alloc memory when init encoder\n"); @@ -131,6 +98,22 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
 	}
 
 	drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
+	connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL);
+	if (!connector) {
+		DRM_ERROR("failed to alloc memory when init connector\n");
+		return -ENOMEM;
+	}
+
+	ret = drm_connector_init(dev, connector,
+				 &hibmc_connector_funcs,
+				 DRM_MODE_CONNECTOR_VGA);
+	if (ret) {
+		DRM_ERROR("failed to init connector: %d\n", ret);
+		return ret;
+	}
+
+	drm_connector_helper_add(connector, &hibmc_connector_helper_funcs);
+	drm_connector_register(connector);
 	drm_connector_attach_encoder(connector, encoder);
 
 	return 0;
--
2.7.4

_______________________________________________
Linuxarm mailing list
Linuxarm@huawei.com
http://hulk.huawei.com/mailman/listinfo/linuxarm

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

* Re: [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver
  2020-03-06  3:42 [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
                   ` (6 preceding siblings ...)
  2020-03-06  3:59 ` 答复: [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver tiantao (H)
@ 2020-03-06  7:24 ` Thomas Zimmermann
  2020-03-06  9:11 ` kbuild test robot
  8 siblings, 0 replies; 15+ messages in thread
From: Thomas Zimmermann @ 2020-03-06  7:24 UTC (permalink / raw)
  To: Tian Tao, puck.chen, airlied, daniel, kraxel, alexander.deucher,
	tglx, dri-devel, xinliang.liu, linux-kernel
  Cc: linuxarm


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

Hi

Am 06.03.20 um 04:42 schrieb Tian Tao:
> using the load and unload function provided by drm framework instead of
> doing the same work in the hibmc_pci_probe and do some code cleanup.
> 
> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> Signed-off-by: Gong junjie <gongjunjie2@huawei.com>
> ---
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 62 +++++++++----------------
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h |  2 +
>  2 files changed, 24 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index 79a180a..51f1c70 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -23,7 +23,7 @@
>  #include <drm/drm_print.h>
>  #include <drm/drm_probe_helper.h>
>  #include <drm/drm_vblank.h>
> -
> +#include <drm/drm_pci.h>
>  #include "hibmc_drm_drv.h"
>  #include "hibmc_drm_regs.h"
>  
> @@ -49,6 +49,8 @@ static irqreturn_t hibmc_drm_interrupt(int irq, void *arg)
>  
>  static struct drm_driver hibmc_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> +	.load			= hibmc_load,
> +	.unload			= hibmc_unload,

These callbacks are deprecated and not to be used. During the last weeks
and months, we've been working on removing them from the other remaining
drivers. Therefore nak.

Best regards
Thomas

>  	.fops			= &hibmc_fops,
>  	.name			= "hibmc",
>  	.date			= "20160828",
> @@ -232,6 +234,21 @@ static int hibmc_hw_map(struct hibmc_drm_private *priv)
>  	return 0;
>  }
>  
> +static void hibmc_hw_unmap(struct hibmc_drm_private *priv)
> +{
> +	struct drm_device *dev = priv->dev;
> +
> +	if (priv->mmio) {
> +		devm_iounmap(dev->dev, priv->mmio);
> +		priv->mmio = NULL;
> +	}
> +
> +	if (priv->fb_map) {
> +		devm_iounmap(dev->dev, priv->fb_map);
> +		priv->fb_map = NULL;
> +	}
> +}
> +
>  static int hibmc_hw_init(struct hibmc_drm_private *priv)
>  {
>  	int ret;
> @@ -245,7 +262,7 @@ static int hibmc_hw_init(struct hibmc_drm_private *priv)
>  	return 0;
>  }
>  
> -static int hibmc_unload(struct drm_device *dev)
> +void hibmc_unload(struct drm_device *dev)
>  {
>  	struct hibmc_drm_private *priv = dev->dev_private;
>  
> @@ -258,11 +275,12 @@ static int hibmc_unload(struct drm_device *dev)
>  
>  	hibmc_kms_fini(priv);
>  	hibmc_mm_fini(priv);
> +	hibmc_hw_unmap(priv);
>  	dev->dev_private = NULL;
>  	return 0;
>  }
>  
> -static int hibmc_load(struct drm_device *dev)
> +int hibmc_load(struct drm_device *dev, unsigned long flags)
>  {
>  	struct hibmc_drm_private *priv;
>  	int ret;
> @@ -332,43 +350,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev,
>  	if (ret)
>  		return ret;
>  
> -	dev = drm_dev_alloc(&hibmc_driver, &pdev->dev);
> -	if (IS_ERR(dev)) {
> -		DRM_ERROR("failed to allocate drm_device\n");
> -		return PTR_ERR(dev);
> -	}
> -
> -	dev->pdev = pdev;
> -	pci_set_drvdata(pdev, dev);
> -
> -	ret = pci_enable_device(pdev);
> -	if (ret) {
> -		DRM_ERROR("failed to enable pci device: %d\n", ret);
> -		goto err_free;
> -	}
> -
> -	ret = hibmc_load(dev);
> -	if (ret) {
> -		DRM_ERROR("failed to load hibmc: %d\n", ret);
> -		goto err_disable;
> -	}
> -
> -	ret = drm_dev_register(dev, 0);
> -	if (ret) {
> -		DRM_ERROR("failed to register drv for userspace access: %d\n",
> -			  ret);
> -		goto err_unload;
> -	}
> -	return 0;
> -
> -err_unload:
> -	hibmc_unload(dev);
> -err_disable:
> -	pci_disable_device(pdev);
> -err_free:
> -	drm_dev_put(dev);
> -
> -	return ret;
> +	return drm_get_pci_dev(pdev, ent, &hibmc_driver);
>  }
>  
>  static void hibmc_pci_remove(struct pci_dev *pdev)
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index 50a0c1f..4e89cd7 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -37,6 +37,8 @@ void hibmc_set_power_mode(struct hibmc_drm_private *priv,
>  void hibmc_set_current_gate(struct hibmc_drm_private *priv,
>  			    unsigned int gate);
>  
> +int hibmc_load(struct drm_device *dev, unsigned long flags);
> +void hibmc_unload(struct drm_device *dev);
>  int hibmc_de_init(struct hibmc_drm_private *priv);
>  int hibmc_vdac_init(struct hibmc_drm_private *priv);
>  
> 

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


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

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

* Re: [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver
  2020-03-06  3:42 [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
                   ` (7 preceding siblings ...)
  2020-03-06  7:24 ` Thomas Zimmermann
@ 2020-03-06  9:11 ` kbuild test robot
  8 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2020-03-06  9:11 UTC (permalink / raw)
  To: Tian Tao
  Cc: kbuild-all, puck.chen, airlied, daniel, tzimmermann, kraxel,
	alexander.deucher, tglx, dri-devel, xinliang.liu, linux-kernel,
	linuxarm

[-- Attachment #1: Type: text/plain, Size: 3360 bytes --]

Hi Tian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20200305]
[cannot apply to linus/master v5.6-rc4 v5.6-rc3 v5.6-rc2 v5.6-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Tian-Tao/drm-hisilicon-Add-the-load-and-unload-for-hibmc_driver/20200306-150600
base:    47466dcf84ee66a973ea7d2fca7e582fe9328932
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c: In function 'hibmc_unload':
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c:280:9: warning: 'return' with a value, in function returning void
     return 0;
            ^
   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c:265:6: note: declared here
    void hibmc_unload(struct drm_device *dev)
         ^~~~~~~~~~~~
   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c: In function 'hibmc_pci_probe':
   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c:345:21: warning: unused variable 'dev' [-Wunused-variable]
     struct drm_device *dev;
                        ^~~

vim +/return +280 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c

5e0df3a08f3d17 Rongrong Zou  2016-11-16  264  
da3ad5dfcca37d Tian Tao      2020-03-06  265  void hibmc_unload(struct drm_device *dev)
5e0df3a08f3d17 Rongrong Zou  2016-11-16  266  {
e4daebc77e7b34 Rongrong Zou  2016-11-16  267  	struct hibmc_drm_private *priv = dev->dev_private;
e4daebc77e7b34 Rongrong Zou  2016-11-16  268  
b3df5e65cc0369 Daniel Vetter 2017-06-21  269  	drm_atomic_helper_shutdown(dev);
b3df5e65cc0369 Daniel Vetter 2017-06-21  270  
1d98b91611725e Rongrong Zou  2016-11-16  271  	if (dev->irq_enabled)
1d98b91611725e Rongrong Zou  2016-11-16  272  		drm_irq_uninstall(dev);
1d98b91611725e Rongrong Zou  2016-11-16  273  	if (priv->msi_enabled)
1d98b91611725e Rongrong Zou  2016-11-16  274  		pci_disable_msi(dev->pdev);
1d98b91611725e Rongrong Zou  2016-11-16  275  
da52605eea8f2c Rongrong Zou  2016-11-16  276  	hibmc_kms_fini(priv);
e4daebc77e7b34 Rongrong Zou  2016-11-16  277  	hibmc_mm_fini(priv);
da3ad5dfcca37d Tian Tao      2020-03-06  278  	hibmc_hw_unmap(priv);
e4daebc77e7b34 Rongrong Zou  2016-11-16  279  	dev->dev_private = NULL;
5e0df3a08f3d17 Rongrong Zou  2016-11-16 @280  	return 0;
5e0df3a08f3d17 Rongrong Zou  2016-11-16  281  }
5e0df3a08f3d17 Rongrong Zou  2016-11-16  282  

:::::: The code at line 280 was first introduced by commit
:::::: 5e0df3a08f3d17485a5081634902424c6834e001 drm/hisilicon/hibmc: Add hisilicon hibmc drm master driver

:::::: TO: Rongrong Zou <zourongrong@gmail.com>
:::::: CC: Rongrong Zou <zourongrong@gmail.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47724 bytes --]

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

* Re: [PATCH 1/4] drm/hisilicon: Enforce 128-byte stride alignment to fix the hardware limitation
  2020-03-06  3:43 ` [PATCH 1/4] drm/hisilicon: Enforce 128-byte stride alignment to fix the hardware limitation Tian Tao
@ 2020-04-03  2:00   ` Xinliang Liu
  0 siblings, 0 replies; 15+ messages in thread
From: Xinliang Liu @ 2020-04-03  2:00 UTC (permalink / raw)
  To: Tian Tao
  Cc: Chen Feng, David Airlie, Daniel Vetter, tzimmermann, kraxel,
	alexander.deucher, tglx, dri-devel, lkml, linuxarm

Hi Tao,

On Fri, 6 Mar 2020 at 11:44, Tian Tao <tiantao6@hisilicon.com> wrote:
>
> because the hardware limitation,The initial color depth must set to 32bpp
> and must set the FB Offset of the display hardware to 128Byte alignment,
> which is used to solve the display problem at 800x600 and 1440x900
> resolution under 16bpp.
>
> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> Signed-off-by: Gong junjie <gongjunjie2@huawei.com>

Thanks for the patch.
Acked-by: Xinliang Liu <xinliang.liu@linaro.org>
Applied to drm-misc.

-Xinliang

> ---
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c  | 9 +++++----
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 4 ++--
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c     | 2 +-
>  3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> index 55b46a7..cc70e83 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> @@ -94,6 +94,10 @@ static int hibmc_plane_atomic_check(struct drm_plane *plane,
>                 return -EINVAL;
>         }
>
> +       if (state->fb->pitches[0] % 128 != 0) {
> +               DRM_DEBUG_ATOMIC("wrong stride with 128-byte aligned\n");
> +               return -EINVAL;
> +       }
>         return 0;
>  }
>
> @@ -119,11 +123,8 @@ static void hibmc_plane_atomic_update(struct drm_plane *plane,
>         writel(gpu_addr, priv->mmio + HIBMC_CRT_FB_ADDRESS);
>
>         reg = state->fb->width * (state->fb->format->cpp[0]);
> -       /* now line_pad is 16 */
> -       reg = PADDING(16, reg);
>
> -       line_l = state->fb->width * state->fb->format->cpp[0];
> -       line_l = PADDING(16, line_l);
> +       line_l = state->fb->pitches[0];
>         writel(HIBMC_FIELD(HIBMC_CRT_FB_WIDTH_WIDTH, reg) |
>                HIBMC_FIELD(HIBMC_CRT_FB_WIDTH_OFFS, line_l),
>                priv->mmio + HIBMC_CRT_FB_WIDTH);
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index 222356a..79a180a 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -94,7 +94,7 @@ static int hibmc_kms_init(struct hibmc_drm_private *priv)
>         priv->dev->mode_config.max_height = 1200;
>
>         priv->dev->mode_config.fb_base = priv->fb_base;
> -       priv->dev->mode_config.preferred_depth = 24;
> +       priv->dev->mode_config.preferred_depth = 32;
>         priv->dev->mode_config.prefer_shadow = 1;
>
>         priv->dev->mode_config.funcs = (void *)&hibmc_mode_funcs;
> @@ -307,7 +307,7 @@ static int hibmc_load(struct drm_device *dev)
>         /* reset all the states of crtc/plane/encoder/connector */
>         drm_mode_config_reset(dev);
>
> -       ret = drm_fbdev_generic_setup(dev, 16);
> +       ret = drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth);
>         if (ret) {
>                 DRM_ERROR("failed to initialize fbdev: %d\n", ret);
>                 goto err;
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> index 99397ac..322bd54 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> @@ -50,7 +50,7 @@ void hibmc_mm_fini(struct hibmc_drm_private *hibmc)
>  int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
>                       struct drm_mode_create_dumb *args)
>  {
> -       return drm_gem_vram_fill_create_dumb(file, dev, 0, 16, args);
> +       return drm_gem_vram_fill_create_dumb(file, dev, 0, 128, args);
>  }
>
>  const struct drm_mode_config_funcs hibmc_mode_funcs = {
> --
> 2.7.4
>

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

* Re: [PATCH 2/4] drm/hisilicon: Code cleanup for hibmc_drv_vdac
  2020-03-06  3:43 ` [PATCH 2/4] drm/hisilicon: Code cleanup for hibmc_drv_vdac Tian Tao
@ 2020-04-03  2:26   ` Xinliang Liu
  2020-04-03  7:00     ` Thomas Zimmermann
  0 siblings, 1 reply; 15+ messages in thread
From: Xinliang Liu @ 2020-04-03  2:26 UTC (permalink / raw)
  To: Tian Tao
  Cc: Chen Feng, David Airlie, Daniel Vetter, tzimmermann, kraxel,
	alexander.deucher, tglx, dri-devel, lkml, linuxarm

Hi Tao,

On Fri, 6 Mar 2020 at 11:44, Tian Tao <tiantao6@hisilicon.com> wrote:
>
> code cleanup for hibmc_drv_vdac.c, no actual function changes.
>
> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> Signed-off-by: Gong junjie <gongjunjie2@huawei.com>
> ---
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 49 ++++++++----------------
>  1 file changed, 16 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> index 678ac2e..f0e6bb8 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> @@ -52,32 +52,6 @@ static const struct drm_connector_funcs hibmc_connector_funcs = {
>         .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  };
>
> -static struct drm_connector *
> -hibmc_connector_init(struct hibmc_drm_private *priv)
> -{
> -       struct drm_device *dev = priv->dev;
> -       struct drm_connector *connector;
> -       int ret;
> -
> -       connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL);
> -       if (!connector) {
> -               DRM_ERROR("failed to alloc memory when init connector\n");
> -               return ERR_PTR(-ENOMEM);
> -       }
> -
> -       ret = drm_connector_init(dev, connector,
> -                                &hibmc_connector_funcs,
> -                                DRM_MODE_CONNECTOR_VGA);
> -       if (ret) {
> -               DRM_ERROR("failed to init connector: %d\n", ret);
> -               return ERR_PTR(ret);
> -       }
> -       drm_connector_helper_add(connector,
> -                                &hibmc_connector_helper_funcs);
> -
> -       return connector;
> -}
> -
>  static void hibmc_encoder_mode_set(struct drm_encoder *encoder,
>                                    struct drm_display_mode *mode,
>                                    struct drm_display_mode *adj_mode)
> @@ -109,13 +83,6 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
>         struct drm_connector *connector;
>         int ret;
>
> -       connector = hibmc_connector_init(priv);
> -       if (IS_ERR(connector)) {
> -               DRM_ERROR("failed to create connector: %ld\n",
> -                         PTR_ERR(connector));
> -               return PTR_ERR(connector);
> -       }
> -
>         encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL);
>         if (!encoder) {
>                 DRM_ERROR("failed to alloc memory when init encoder\n");
> @@ -131,6 +98,22 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
>         }
>
>         drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
> +       connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL);
> +       if (!connector) {
> +               DRM_ERROR("failed to alloc memory when init connector\n");
> +               return -ENOMEM;
> +       }
> +
> +       ret = drm_connector_init(dev, connector,
> +                                &hibmc_connector_funcs,
> +                                DRM_MODE_CONNECTOR_VGA);
> +       if (ret) {
> +               DRM_ERROR("failed to init connector: %d\n", ret);
> +               return ret;
> +       }
> +
> +       drm_connector_helper_add(connector, &hibmc_connector_helper_funcs);
> +       drm_connector_register(connector);

You don't need to register a non-hotplug connector as it will be
registered at drm_dev_register automatically.

See function definition:

 488 /**
 489  * drm_connector_register - register a connector
 490  * @connector: the connector to register
 491  *
 492  * Register userspace interfaces for a connector. Only call this
for connectors
 493  * which can be hotplugged after drm_dev_register() has been
called already,
 494  * e.g. DP MST connectors. All other connectors will be
registered automatically
 495  * when calling drm_dev_register().
 496  *
 497  * Returns:
 498  * Zero on success, error code on failure.
 499  */
 500 int drm_connector_register(struct drm_connector *connector)
 501 {


Besides, I don't think this patch cleans much things.

-Xinliang

>         drm_connector_attach_encoder(connector, encoder);
>
>         return 0;
> --
> 2.7.4
>

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

* Re: [PATCH 2/4] drm/hisilicon: Code cleanup for hibmc_drv_vdac
  2020-04-03  2:26   ` Xinliang Liu
@ 2020-04-03  7:00     ` Thomas Zimmermann
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Zimmermann @ 2020-04-03  7:00 UTC (permalink / raw)
  To: Xinliang Liu, Tian Tao
  Cc: David Airlie, Chen Feng, lkml, dri-devel, linuxarm, kraxel,
	alexander.deucher, tglx


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

Hi

Am 03.04.20 um 04:26 schrieb Xinliang Liu:
> Hi Tao,
> 
> On Fri, 6 Mar 2020 at 11:44, Tian Tao <tiantao6@hisilicon.com> wrote:
>>
>> code cleanup for hibmc_drv_vdac.c, no actual function changes.
>>
>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
>> Signed-off-by: Gong junjie <gongjunjie2@huawei.com>
>> ---
>>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 49 ++++++++----------------
>>  1 file changed, 16 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> index 678ac2e..f0e6bb8 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> @@ -52,32 +52,6 @@ static const struct drm_connector_funcs hibmc_connector_funcs = {
>>         .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>>  };
>>
>> -static struct drm_connector *
>> -hibmc_connector_init(struct hibmc_drm_private *priv)
>> -{
>> -       struct drm_device *dev = priv->dev;
>> -       struct drm_connector *connector;
>> -       int ret;
>> -
>> -       connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL);
>> -       if (!connector) {
>> -               DRM_ERROR("failed to alloc memory when init connector\n");
>> -               return ERR_PTR(-ENOMEM);
>> -       }
>> -
>> -       ret = drm_connector_init(dev, connector,
>> -                                &hibmc_connector_funcs,
>> -                                DRM_MODE_CONNECTOR_VGA);
>> -       if (ret) {
>> -               DRM_ERROR("failed to init connector: %d\n", ret);
>> -               return ERR_PTR(ret);
>> -       }
>> -       drm_connector_helper_add(connector,
>> -                                &hibmc_connector_helper_funcs);
>> -
>> -       return connector;
>> -}
>> -
>>  static void hibmc_encoder_mode_set(struct drm_encoder *encoder,
>>                                    struct drm_display_mode *mode,
>>                                    struct drm_display_mode *adj_mode)
>> @@ -109,13 +83,6 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
>>         struct drm_connector *connector;
>>         int ret;
>>
>> -       connector = hibmc_connector_init(priv);
>> -       if (IS_ERR(connector)) {
>> -               DRM_ERROR("failed to create connector: %ld\n",
>> -                         PTR_ERR(connector));
>> -               return PTR_ERR(connector);
>> -       }
>> -
>>         encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL);
>>         if (!encoder) {
>>                 DRM_ERROR("failed to alloc memory when init encoder\n");
>> @@ -131,6 +98,22 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
>>         }
>>
>>         drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
>> +       connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL);
>> +       if (!connector) {
>> +               DRM_ERROR("failed to alloc memory when init connector\n");
>> +               return -ENOMEM;
>> +       }
>> +
>> +       ret = drm_connector_init(dev, connector,
>> +                                &hibmc_connector_funcs,
>> +                                DRM_MODE_CONNECTOR_VGA);
>> +       if (ret) {
>> +               DRM_ERROR("failed to init connector: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       drm_connector_helper_add(connector, &hibmc_connector_helper_funcs);
>> +       drm_connector_register(connector);
> 
> You don't need to register a non-hotplug connector as it will be
> registered at drm_dev_register automatically.
> 
> See function definition:
> 
>  488 /**
>  489  * drm_connector_register - register a connector
>  490  * @connector: the connector to register
>  491  *
>  492  * Register userspace interfaces for a connector. Only call this
> for connectors
>  493  * which can be hotplugged after drm_dev_register() has been
> called already,
>  494  * e.g. DP MST connectors. All other connectors will be
> registered automatically
>  495  * when calling drm_dev_register().
>  496  *
>  497  * Returns:
>  498  * Zero on success, error code on failure.
>  499  */
>  500 int drm_connector_register(struct drm_connector *connector)
>  501 {
> 
> 
> Besides, I don't think this patch cleans much things.

True. Although the auto-kfree of devm_kzalloc() does not work correctly
with DRM and should be replaced.

I'd like to suggest to either allocate the connector as part of struct
hibmc_drm_private, or implement drmm_connector_create() on top of DRM's
new auto-cleanup functionality.

Best regards
Thomas

> 
> -Xinliang
> 
>>         drm_connector_attach_encoder(connector, encoder);
>>
>>         return 0;
>> --
>> 2.7.4
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

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


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

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

end of thread, other threads:[~2020-04-03  7:00 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-06  3:42 [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
2020-03-06  3:42 ` [PATCH] drm/hisilicon: Add the shutdown for hibmc_pci_driver Tian Tao
2020-03-06  3:58   ` 答复: " tiantao (H)
2020-03-06  3:43 ` [PATCH] drm/hisilicon: Code cleanup for hibmc_drv_vdac Tian Tao
2020-03-06  3:59   ` 答复: " tiantao (H)
2020-03-06  3:43 ` [PATCH 1/4] drm/hisilicon: Enforce 128-byte stride alignment to fix the hardware limitation Tian Tao
2020-04-03  2:00   ` Xinliang Liu
2020-03-06  3:43 ` [PATCH 2/4] drm/hisilicon: Code cleanup for hibmc_drv_vdac Tian Tao
2020-04-03  2:26   ` Xinliang Liu
2020-04-03  7:00     ` Thomas Zimmermann
2020-03-06  3:43 ` [PATCH 3/4] drm/hisilicon: Add the load and unload for hibmc_driver Tian Tao
2020-03-06  3:43 ` [PATCH 4/4] drm/hisilicon: Add the shutdown for hibmc_pci_driver Tian Tao
2020-03-06  3:59 ` 答复: [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver tiantao (H)
2020-03-06  7:24 ` Thomas Zimmermann
2020-03-06  9:11 ` kbuild test robot

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