drm/hisilicon: Add the load and unload for hibmc_driver
diff mbox series

Message ID 1583466184-7060-1-git-send-email-tiantao6@hisilicon.com
State New, archived
Headers show
Series
  • drm/hisilicon: Add the load and unload for hibmc_driver
Related show

Commit Message

Tian Tao March 6, 2020, 3:42 a.m. UTC
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(-)

Comments

Thomas Zimmermann March 6, 2020, 7:24 a.m. UTC | #1
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);
>  
>
kernel test robot March 6, 2020, 9:11 a.m. UTC | #2
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

Patch
diff mbox series

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