Re: [PATCH 4/5] drm/mgag200: Init and finalize devices in mgag200_device_{init,fini}()

2020-05-06 Thread Thomas Zimmermann
Hi Sam

Am 05.05.20 um 18:49 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Tue, May 05, 2020 at 11:56:48AM +0200, Thomas Zimmermann wrote:
>> Device initialization is now done in mgag200_device_init(). Specifically,
>> the function allocates the DRM device and sets up the respective fields
>> in struct mga_device.
>>
>> A call to mgag200_device_fini() finalizes struct mga_device.
>>
>> The old function mgag200_driver_load() and mgag200_driver_unload() were
>> left over from the DRM driver's load callbacks and have now been removed.
> 
> Not too big fan of this patch, due to the changes allocation.
> I would prefer if you merged patch 4+5 and then take it from there.
> 
> You have patch 1+2+3 and they are now reviewed so why not push them
> and work on top of them.

Good idea. I'll do this and postpone patches 4 and 5 to a later patch set.

Best regards
Thomas

> And then you could also push the patch that removes the cursor stuff
> so we do not need to look at that anymore.
> 
>   Sam
>>
>> Signed-off-by: Thomas Zimmermann 
>> ---
>>  drivers/gpu/drm/mgag200/mgag200_drv.c  | 27 ++--
>>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  5 ++--
>>  drivers/gpu/drm/mgag200/mgag200_main.c | 34 --
>>  3 files changed, 37 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c 
>> b/drivers/gpu/drm/mgag200/mgag200_drv.c
>> index c2f0e4b40b052..ad12c1b7c66cc 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
>> @@ -51,6 +51,7 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
>>  
>>  static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id 
>> *ent)
>>  {
>> +struct mga_device *mdev;
>>  struct drm_device *dev;
>>  int ret;
>>  
>> @@ -60,31 +61,28 @@ static int mga_pci_probe(struct pci_dev *pdev, const 
>> struct pci_device_id *ent)
>>  if (ret)
>>  return ret;
>>  
>> -dev = drm_dev_alloc(, >dev);
>> -if (IS_ERR(dev)) {
>> -ret = PTR_ERR(dev);
>> +mdev = devm_kzalloc(>dev, sizeof(*mdev), GFP_KERNEL);
>> +if (!mdev) {
>> +ret = -ENOMEM;
>>  goto err_pci_disable_device;
>>  }
>>  
>> -dev->pdev = pdev;
>> -pci_set_drvdata(pdev, dev);
>> -
>> -ret = mgag200_driver_load(dev, ent->driver_data);
>> +ret = mgag200_device_init(mdev, , pdev, ent->driver_data);
>>  if (ret)
>> -goto err_drm_dev_put;
>> +goto err_pci_disable_device;
>> +
>> +dev = mdev->dev;
>>  
>>  ret = drm_dev_register(dev, ent->driver_data);
>>  if (ret)
>> -goto err_mgag200_driver_unload;
>> +goto err_mgag200_device_fini;
>>  
>>  drm_fbdev_generic_setup(dev, 0);
>>  
>>  return 0;
>>  
>> -err_mgag200_driver_unload:
>> -mgag200_driver_unload(dev);
>> -err_drm_dev_put:
>> -drm_dev_put(dev);
>> +err_mgag200_device_fini:
>> +mgag200_device_fini(mdev);
>>  err_pci_disable_device:
>>  pci_disable_device(pdev);
>>  return ret;
>> @@ -93,9 +91,10 @@ static int mga_pci_probe(struct pci_dev *pdev, const 
>> struct pci_device_id *ent)
>>  static void mga_pci_remove(struct pci_dev *pdev)
>>  {
>>  struct drm_device *dev = pci_get_drvdata(pdev);
>> +struct mga_device *mdev = to_mga_device(dev);
>>  
>>  drm_dev_unregister(dev);
>> -mgag200_driver_unload(dev);
>> +mgag200_device_fini(mdev);
>>  drm_dev_put(dev);
>>  }
>>  
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h 
>> b/drivers/gpu/drm/mgag200/mgag200_drv.h
>> index 632bbb50465c9..1ce0386669ffa 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
>> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
>> @@ -200,8 +200,9 @@ int mgag200_modeset_init(struct mga_device *mdev);
>>  void mgag200_modeset_fini(struct mga_device *mdev);
>>  
>>  /* mgag200_main.c */
>> -int mgag200_driver_load(struct drm_device *dev, unsigned long flags);
>> -void mgag200_driver_unload(struct drm_device *dev);
>> +int mgag200_device_init(struct mga_device *mdev, struct drm_driver *drv,
>> +struct pci_dev *pdev, unsigned long flags);
>> +void mgag200_device_fini(struct mga_device *mdev);
>>  
>>  /* mgag200_i2c.c */
>>  struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev);
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c 
>> b/drivers/gpu/drm/mgag200/mgag200_main.c
>> index 010b309c01fc4..070ff1f433df2 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_main.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_main.c
>> @@ -11,6 +11,7 @@
>>  #include 
>>  
>>  #include 
>> +#include 
>>  #include 
>>  
>>  #include "mgag200_drv.h"
>> @@ -96,17 +97,21 @@ static int mga_vram_init(struct mga_device *mdev)
>>   */
>>  
>>  
>> -int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
>> +int mgag200_device_init(struct mga_device *mdev, struct drm_driver *drv,
>> +struct pci_dev *pdev, unsigned 

Re: [PATCH 4/5] drm/mgag200: Init and finalize devices in mgag200_device_{init,fini}()

2020-05-06 Thread Thomas Zimmermann
Hi

Am 05.05.20 um 16:14 schrieb Daniel Vetter:
> On Tue, May 05, 2020 at 11:56:48AM +0200, Thomas Zimmermann wrote:
>> Device initialization is now done in mgag200_device_init(). Specifically,
>> the function allocates the DRM device and sets up the respective fields
>> in struct mga_device.
>>
>> A call to mgag200_device_fini() finalizes struct mga_device.
>>
>> The old function mgag200_driver_load() and mgag200_driver_unload() were
>> left over from the DRM driver's load callbacks and have now been removed.
>>
>> Signed-off-by: Thomas Zimmermann 
>> ---
>>  drivers/gpu/drm/mgag200/mgag200_drv.c  | 27 ++--
>>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  5 ++--
>>  drivers/gpu/drm/mgag200/mgag200_main.c | 34 --
>>  3 files changed, 37 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c 
>> b/drivers/gpu/drm/mgag200/mgag200_drv.c
>> index c2f0e4b40b052..ad12c1b7c66cc 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
>> @@ -51,6 +51,7 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
>>  
>>  static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id 
>> *ent)
>>  {
>> +struct mga_device *mdev;
>>  struct drm_device *dev;
>>  int ret;
>>  
>> @@ -60,31 +61,28 @@ static int mga_pci_probe(struct pci_dev *pdev, const 
>> struct pci_device_id *ent)
>>  if (ret)
>>  return ret;
>>  
>> -dev = drm_dev_alloc(, >dev);
>> -if (IS_ERR(dev)) {
>> -ret = PTR_ERR(dev);
>> +mdev = devm_kzalloc(>dev, sizeof(*mdev), GFP_KERNEL);
>> +if (!mdev) {
>> +ret = -ENOMEM;
>>  goto err_pci_disable_device;
>>  }
>>  
>> -dev->pdev = pdev;
>> -pci_set_drvdata(pdev, dev);
>> -
>> -ret = mgag200_driver_load(dev, ent->driver_data);
>> +ret = mgag200_device_init(mdev, , pdev, ent->driver_data);
>>  if (ret)
>> -goto err_drm_dev_put;
>> +goto err_pci_disable_device;
>> +
>> +dev = mdev->dev;
>>  
>>  ret = drm_dev_register(dev, ent->driver_data);
>>  if (ret)
>> -goto err_mgag200_driver_unload;
>> +goto err_mgag200_device_fini;
>>  
>>  drm_fbdev_generic_setup(dev, 0);
>>  
>>  return 0;
>>  
>> -err_mgag200_driver_unload:
>> -mgag200_driver_unload(dev);
>> -err_drm_dev_put:
>> -drm_dev_put(dev);
> 
> Moving the drm_dev_put away from here will make the conversion to
> devm_drm_dev_alloc a bit more tricky I think. I'm not sure whether this is
> actually better than just directly going to devm_drm_dev_alloc and then
> cleaning up the fallout, that's at least what I've done in the conversions
> I've attempted thus far.

It's only a first step. Sam suggested to take patches 1 to 3 and build
the atomic conversion on top of that. That's probably what I'll do.

In the longer term, I'd like to use fully managed DRM functions, but
that requires another major patchset. The code in mgag200_main.c and
_ttm.c would go to either _drv.c or _mode.c. From there, the
initialization and shut down can be rewritten with managed helpers.
That's best done after the atomic conversion. Patches 4 and 5 can be
part of this. The VRAM helpers and cursor code will be gone then, which
also helps a bit.

Best regards
Thomas

> 
> Either way, this looks correct.
> 
> Reviewed-by: Daniel Vetter 
> 
>> +err_mgag200_device_fini:
>> +mgag200_device_fini(mdev);
>>  err_pci_disable_device:
>>  pci_disable_device(pdev);
>>  return ret;
>> @@ -93,9 +91,10 @@ static int mga_pci_probe(struct pci_dev *pdev, const 
>> struct pci_device_id *ent)
>>  static void mga_pci_remove(struct pci_dev *pdev)
>>  {
>>  struct drm_device *dev = pci_get_drvdata(pdev);
>> +struct mga_device *mdev = to_mga_device(dev);
>>  
>>  drm_dev_unregister(dev);
>> -mgag200_driver_unload(dev);
>> +mgag200_device_fini(mdev);
>>  drm_dev_put(dev);
>>  }
>>  
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h 
>> b/drivers/gpu/drm/mgag200/mgag200_drv.h
>> index 632bbb50465c9..1ce0386669ffa 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
>> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
>> @@ -200,8 +200,9 @@ int mgag200_modeset_init(struct mga_device *mdev);
>>  void mgag200_modeset_fini(struct mga_device *mdev);
>>  
>>  /* mgag200_main.c */
>> -int mgag200_driver_load(struct drm_device *dev, unsigned long flags);
>> -void mgag200_driver_unload(struct drm_device *dev);
>> +int mgag200_device_init(struct mga_device *mdev, struct drm_driver *drv,
>> +struct pci_dev *pdev, unsigned long flags);
>> +void mgag200_device_fini(struct mga_device *mdev);
>>  
>>  /* mgag200_i2c.c */
>>  struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev);
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c 
>> b/drivers/gpu/drm/mgag200/mgag200_main.c
>> index 010b309c01fc4..070ff1f433df2 100644
>> --- 

Re: [PATCH 4/5] drm/mgag200: Init and finalize devices in mgag200_device_{init,fini}()

2020-05-05 Thread Sam Ravnborg
Hi Thomas.

On Tue, May 05, 2020 at 11:56:48AM +0200, Thomas Zimmermann wrote:
> Device initialization is now done in mgag200_device_init(). Specifically,
> the function allocates the DRM device and sets up the respective fields
> in struct mga_device.
> 
> A call to mgag200_device_fini() finalizes struct mga_device.
> 
> The old function mgag200_driver_load() and mgag200_driver_unload() were
> left over from the DRM driver's load callbacks and have now been removed.

Not too big fan of this patch, due to the changes allocation.
I would prefer if you merged patch 4+5 and then take it from there.

You have patch 1+2+3 and they are now reviewed so why not push them
and work on top of them.
And then you could also push the patch that removes the cursor stuff
so we do not need to look at that anymore.

Sam
> 
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/mgag200/mgag200_drv.c  | 27 ++--
>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  5 ++--
>  drivers/gpu/drm/mgag200/mgag200_main.c | 34 --
>  3 files changed, 37 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c 
> b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index c2f0e4b40b052..ad12c1b7c66cc 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -51,6 +51,7 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
>  
>  static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id 
> *ent)
>  {
> + struct mga_device *mdev;
>   struct drm_device *dev;
>   int ret;
>  
> @@ -60,31 +61,28 @@ static int mga_pci_probe(struct pci_dev *pdev, const 
> struct pci_device_id *ent)
>   if (ret)
>   return ret;
>  
> - dev = drm_dev_alloc(, >dev);
> - if (IS_ERR(dev)) {
> - ret = PTR_ERR(dev);
> + mdev = devm_kzalloc(>dev, sizeof(*mdev), GFP_KERNEL);
> + if (!mdev) {
> + ret = -ENOMEM;
>   goto err_pci_disable_device;
>   }
>  
> - dev->pdev = pdev;
> - pci_set_drvdata(pdev, dev);
> -
> - ret = mgag200_driver_load(dev, ent->driver_data);
> + ret = mgag200_device_init(mdev, , pdev, ent->driver_data);
>   if (ret)
> - goto err_drm_dev_put;
> + goto err_pci_disable_device;
> +
> + dev = mdev->dev;
>  
>   ret = drm_dev_register(dev, ent->driver_data);
>   if (ret)
> - goto err_mgag200_driver_unload;
> + goto err_mgag200_device_fini;
>  
>   drm_fbdev_generic_setup(dev, 0);
>  
>   return 0;
>  
> -err_mgag200_driver_unload:
> - mgag200_driver_unload(dev);
> -err_drm_dev_put:
> - drm_dev_put(dev);
> +err_mgag200_device_fini:
> + mgag200_device_fini(mdev);
>  err_pci_disable_device:
>   pci_disable_device(pdev);
>   return ret;
> @@ -93,9 +91,10 @@ static int mga_pci_probe(struct pci_dev *pdev, const 
> struct pci_device_id *ent)
>  static void mga_pci_remove(struct pci_dev *pdev)
>  {
>   struct drm_device *dev = pci_get_drvdata(pdev);
> + struct mga_device *mdev = to_mga_device(dev);
>  
>   drm_dev_unregister(dev);
> - mgag200_driver_unload(dev);
> + mgag200_device_fini(mdev);
>   drm_dev_put(dev);
>  }
>  
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h 
> b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index 632bbb50465c9..1ce0386669ffa 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -200,8 +200,9 @@ int mgag200_modeset_init(struct mga_device *mdev);
>  void mgag200_modeset_fini(struct mga_device *mdev);
>  
>   /* mgag200_main.c */
> -int mgag200_driver_load(struct drm_device *dev, unsigned long flags);
> -void mgag200_driver_unload(struct drm_device *dev);
> +int mgag200_device_init(struct mga_device *mdev, struct drm_driver *drv,
> + struct pci_dev *pdev, unsigned long flags);
> +void mgag200_device_fini(struct mga_device *mdev);
>  
>   /* mgag200_i2c.c */
>  struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c 
> b/drivers/gpu/drm/mgag200/mgag200_main.c
> index 010b309c01fc4..070ff1f433df2 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_main.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_main.c
> @@ -11,6 +11,7 @@
>  #include 
>  
>  #include 
> +#include 
>  #include 
>  
>  #include "mgag200_drv.h"
> @@ -96,17 +97,21 @@ static int mga_vram_init(struct mga_device *mdev)
>   */
>  
>  
> -int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
> +int mgag200_device_init(struct mga_device *mdev, struct drm_driver *drv,
> + struct pci_dev *pdev, unsigned long flags)
>  {
> - struct mga_device *mdev;
> + struct drm_device *dev = mdev->dev;
>   int ret, option;
>  
> - mdev = devm_kzalloc(dev->dev, sizeof(struct mga_device), GFP_KERNEL);
> - if (mdev == NULL)
> - return 

Re: [PATCH 4/5] drm/mgag200: Init and finalize devices in mgag200_device_{init,fini}()

2020-05-05 Thread Daniel Vetter
On Tue, May 05, 2020 at 11:56:48AM +0200, Thomas Zimmermann wrote:
> Device initialization is now done in mgag200_device_init(). Specifically,
> the function allocates the DRM device and sets up the respective fields
> in struct mga_device.
> 
> A call to mgag200_device_fini() finalizes struct mga_device.
> 
> The old function mgag200_driver_load() and mgag200_driver_unload() were
> left over from the DRM driver's load callbacks and have now been removed.
> 
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/mgag200/mgag200_drv.c  | 27 ++--
>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  5 ++--
>  drivers/gpu/drm/mgag200/mgag200_main.c | 34 --
>  3 files changed, 37 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c 
> b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index c2f0e4b40b052..ad12c1b7c66cc 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -51,6 +51,7 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
>  
>  static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id 
> *ent)
>  {
> + struct mga_device *mdev;
>   struct drm_device *dev;
>   int ret;
>  
> @@ -60,31 +61,28 @@ static int mga_pci_probe(struct pci_dev *pdev, const 
> struct pci_device_id *ent)
>   if (ret)
>   return ret;
>  
> - dev = drm_dev_alloc(, >dev);
> - if (IS_ERR(dev)) {
> - ret = PTR_ERR(dev);
> + mdev = devm_kzalloc(>dev, sizeof(*mdev), GFP_KERNEL);
> + if (!mdev) {
> + ret = -ENOMEM;
>   goto err_pci_disable_device;
>   }
>  
> - dev->pdev = pdev;
> - pci_set_drvdata(pdev, dev);
> -
> - ret = mgag200_driver_load(dev, ent->driver_data);
> + ret = mgag200_device_init(mdev, , pdev, ent->driver_data);
>   if (ret)
> - goto err_drm_dev_put;
> + goto err_pci_disable_device;
> +
> + dev = mdev->dev;
>  
>   ret = drm_dev_register(dev, ent->driver_data);
>   if (ret)
> - goto err_mgag200_driver_unload;
> + goto err_mgag200_device_fini;
>  
>   drm_fbdev_generic_setup(dev, 0);
>  
>   return 0;
>  
> -err_mgag200_driver_unload:
> - mgag200_driver_unload(dev);
> -err_drm_dev_put:
> - drm_dev_put(dev);

Moving the drm_dev_put away from here will make the conversion to
devm_drm_dev_alloc a bit more tricky I think. I'm not sure whether this is
actually better than just directly going to devm_drm_dev_alloc and then
cleaning up the fallout, that's at least what I've done in the conversions
I've attempted thus far.

Either way, this looks correct.

Reviewed-by: Daniel Vetter 

> +err_mgag200_device_fini:
> + mgag200_device_fini(mdev);
>  err_pci_disable_device:
>   pci_disable_device(pdev);
>   return ret;
> @@ -93,9 +91,10 @@ static int mga_pci_probe(struct pci_dev *pdev, const 
> struct pci_device_id *ent)
>  static void mga_pci_remove(struct pci_dev *pdev)
>  {
>   struct drm_device *dev = pci_get_drvdata(pdev);
> + struct mga_device *mdev = to_mga_device(dev);
>  
>   drm_dev_unregister(dev);
> - mgag200_driver_unload(dev);
> + mgag200_device_fini(mdev);
>   drm_dev_put(dev);
>  }
>  
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h 
> b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index 632bbb50465c9..1ce0386669ffa 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -200,8 +200,9 @@ int mgag200_modeset_init(struct mga_device *mdev);
>  void mgag200_modeset_fini(struct mga_device *mdev);
>  
>   /* mgag200_main.c */
> -int mgag200_driver_load(struct drm_device *dev, unsigned long flags);
> -void mgag200_driver_unload(struct drm_device *dev);
> +int mgag200_device_init(struct mga_device *mdev, struct drm_driver *drv,
> + struct pci_dev *pdev, unsigned long flags);
> +void mgag200_device_fini(struct mga_device *mdev);
>  
>   /* mgag200_i2c.c */
>  struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c 
> b/drivers/gpu/drm/mgag200/mgag200_main.c
> index 010b309c01fc4..070ff1f433df2 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_main.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_main.c
> @@ -11,6 +11,7 @@
>  #include 
>  
>  #include 
> +#include 
>  #include 
>  
>  #include "mgag200_drv.h"
> @@ -96,17 +97,21 @@ static int mga_vram_init(struct mga_device *mdev)
>   */
>  
>  
> -int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
> +int mgag200_device_init(struct mga_device *mdev, struct drm_driver *drv,
> + struct pci_dev *pdev, unsigned long flags)
>  {
> - struct mga_device *mdev;
> + struct drm_device *dev = mdev->dev;
>   int ret, option;
>  
> - mdev = devm_kzalloc(dev->dev, sizeof(struct mga_device), GFP_KERNEL);
> - if (mdev == NULL)
> -   

[PATCH 4/5] drm/mgag200: Init and finalize devices in mgag200_device_{init, fini}()

2020-05-05 Thread Thomas Zimmermann
Device initialization is now done in mgag200_device_init(). Specifically,
the function allocates the DRM device and sets up the respective fields
in struct mga_device.

A call to mgag200_device_fini() finalizes struct mga_device.

The old function mgag200_driver_load() and mgag200_driver_unload() were
left over from the DRM driver's load callbacks and have now been removed.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/mgag200/mgag200_drv.c  | 27 ++--
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  5 ++--
 drivers/gpu/drm/mgag200/mgag200_main.c | 34 --
 3 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c 
b/drivers/gpu/drm/mgag200/mgag200_drv.c
index c2f0e4b40b052..ad12c1b7c66cc 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -51,6 +51,7 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
 
 static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
+   struct mga_device *mdev;
struct drm_device *dev;
int ret;
 
@@ -60,31 +61,28 @@ static int mga_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
if (ret)
return ret;
 
-   dev = drm_dev_alloc(, >dev);
-   if (IS_ERR(dev)) {
-   ret = PTR_ERR(dev);
+   mdev = devm_kzalloc(>dev, sizeof(*mdev), GFP_KERNEL);
+   if (!mdev) {
+   ret = -ENOMEM;
goto err_pci_disable_device;
}
 
-   dev->pdev = pdev;
-   pci_set_drvdata(pdev, dev);
-
-   ret = mgag200_driver_load(dev, ent->driver_data);
+   ret = mgag200_device_init(mdev, , pdev, ent->driver_data);
if (ret)
-   goto err_drm_dev_put;
+   goto err_pci_disable_device;
+
+   dev = mdev->dev;
 
ret = drm_dev_register(dev, ent->driver_data);
if (ret)
-   goto err_mgag200_driver_unload;
+   goto err_mgag200_device_fini;
 
drm_fbdev_generic_setup(dev, 0);
 
return 0;
 
-err_mgag200_driver_unload:
-   mgag200_driver_unload(dev);
-err_drm_dev_put:
-   drm_dev_put(dev);
+err_mgag200_device_fini:
+   mgag200_device_fini(mdev);
 err_pci_disable_device:
pci_disable_device(pdev);
return ret;
@@ -93,9 +91,10 @@ static int mga_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
 static void mga_pci_remove(struct pci_dev *pdev)
 {
struct drm_device *dev = pci_get_drvdata(pdev);
+   struct mga_device *mdev = to_mga_device(dev);
 
drm_dev_unregister(dev);
-   mgag200_driver_unload(dev);
+   mgag200_device_fini(mdev);
drm_dev_put(dev);
 }
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h 
b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 632bbb50465c9..1ce0386669ffa 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -200,8 +200,9 @@ int mgag200_modeset_init(struct mga_device *mdev);
 void mgag200_modeset_fini(struct mga_device *mdev);
 
/* mgag200_main.c */
-int mgag200_driver_load(struct drm_device *dev, unsigned long flags);
-void mgag200_driver_unload(struct drm_device *dev);
+int mgag200_device_init(struct mga_device *mdev, struct drm_driver *drv,
+   struct pci_dev *pdev, unsigned long flags);
+void mgag200_device_fini(struct mga_device *mdev);
 
/* mgag200_i2c.c */
 struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev);
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c 
b/drivers/gpu/drm/mgag200/mgag200_main.c
index 010b309c01fc4..070ff1f433df2 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -11,6 +11,7 @@
 #include 
 
 #include 
+#include 
 #include 
 
 #include "mgag200_drv.h"
@@ -96,17 +97,21 @@ static int mga_vram_init(struct mga_device *mdev)
  */
 
 
-int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
+int mgag200_device_init(struct mga_device *mdev, struct drm_driver *drv,
+   struct pci_dev *pdev, unsigned long flags)
 {
-   struct mga_device *mdev;
+   struct drm_device *dev = mdev->dev;
int ret, option;
 
-   mdev = devm_kzalloc(dev->dev, sizeof(struct mga_device), GFP_KERNEL);
-   if (mdev == NULL)
-   return -ENOMEM;
+   dev = drm_dev_alloc(drv, >dev);
+   if (IS_ERR(dev))
+   return PTR_ERR(dev);
dev->dev_private = (void *)mdev;
mdev->dev = dev;
 
+   dev->pdev = pdev;
+   pci_set_drvdata(pdev, dev);
+
mdev->flags = mgag200_flags_from_driver_data(flags);
mdev->type = mgag200_type_from_driver_data(flags);
 
@@ -123,12 +128,15 @@ int mgag200_driver_load(struct drm_device *dev, unsigned 
long flags)
if (!devm_request_mem_region(dev->dev, mdev->rmmio_base,
 mdev->rmmio_size, "mgadrmfb_mmio")) {