[PATCH] drm/hisilicon: Add the load and unload for hibmc_driver

[PATCH] drm/hisilicon: Add the load and unload for hibmc_driver

[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,
 	.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

[PATCH] drm/hisilicon: Add the shutdown for hibmc_pci_driver

[Tian Tao]

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

答复: [PATCH] drm/hisilicon: Add the shutdown for hibmc_pci_driver

[tiantao (H)]

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

[PATCH] drm/hisilicon: Code cleanup for hibmc_drv_vdac

[Tian Tao]

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

答复: [PATCH] drm/hisilicon: Code cleanup for hibmc_drv_vdac

[tiantao (H)]

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

[PATCH 1/4] drm/hisilicon: Enforce 128-byte stride alignment to fix the hardware limitation

[Tian Tao]

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

Re: [PATCH 1/4] drm/hisilicon: Enforce 128-byte stride alignment to fix the hardware limitation

[Xinliang Liu]

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
>

[PATCH 2/4] drm/hisilicon: Code cleanup for hibmc_drv_vdac

[Tian Tao]

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

Re: [PATCH 2/4] drm/hisilicon: Code cleanup for hibmc_drv_vdac

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

-Xinliang

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

Re: [PATCH 2/4] drm/hisilicon: Code cleanup for hibmc_drv_vdac

[Thomas Zimmermann]

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

[PATCH 3/4] drm/hisilicon: Add the load and unload for hibmc_driver

[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,
 	.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

[PATCH 4/4] drm/hisilicon: Add the shutdown for hibmc_pci_driver

[Tian Tao]

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

答复: [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver

[tiantao (H)]

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

Re: [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver

[Thomas Zimmermann]

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

Re: [PATCH] drm/hisilicon: Add the load and unload for hibmc_driver

[kbuild test robot]

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