Re: [PATCH v2 1/7] drm/radeon: remove load callback

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Hi

Am 21.06.24 um 16:15 schrieb Wu Hoi Pok:
> This is "drm/radeon: remove load callback" v2, the only changes
> were made are adding "ddev->dev_private = rdev;", right after
> the allocation of "struct radeon_device". Patch v2 2-7 mostly
> describes simple "rdev->ddev" to "rdev_to_drm(rdev)" to suit
> Patch v2 1/7.
>
> Please be aware that these 7 patches depends on each other.

Thanks for the update. In the current form, it's not reviewable, or 
trackable in patchwork 
(https://patchwork.freedesktop.org/project/dri-devel/series/).

For sending patch series, please use 'git send-email' with the 
--cover-letter option. This will provide you with a single email for 
describing the patchset as a whole, and create all actual patch mails as 
replies to the cover letter.

Maybe test this first with your local email account and then please 
resubmit to the mailing list.

Best regards
Thomas

> Thank you.
>
> Signed-off-by: Wu Hoi Pok <wuhoipok@xxxxxxxxx>
> ---
>   drivers/gpu/drm/radeon/radeon.h     | 11 ++++++++---
>   drivers/gpu/drm/radeon/radeon_drv.c | 27 ++++++++++++++++++---------
>   drivers/gpu/drm/radeon/radeon_drv.h |  1 -
>   drivers/gpu/drm/radeon/radeon_kms.c | 18 ++++++------------
>   4 files changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 0999c8eaae94..69bb30ced189 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2297,7 +2297,7 @@ typedef void (*radeon_wreg_t)(struct radeon_device*, uint32_t, uint32_t);
>   
>   struct radeon_device {
>   	struct device			*dev;
> -	struct drm_device		*ddev;
> +	struct drm_device		ddev;
>   	struct pci_dev			*pdev;
>   #ifdef __alpha__
>   	struct pci_controller		*hose;
> @@ -2440,10 +2440,13 @@ struct radeon_device {
>   	u64 gart_pin_size;
>   };
>   
> +static inline struct drm_device *rdev_to_drm(struct radeon_device *rdev)
> +{
> +	return &rdev->ddev;
> +}
> +
>   bool radeon_is_px(struct drm_device *dev);
>   int radeon_device_init(struct radeon_device *rdev,
> -		       struct drm_device *ddev,
> -		       struct pci_dev *pdev,
>   		       uint32_t flags);
>   void radeon_device_fini(struct radeon_device *rdev);
>   int radeon_gpu_wait_for_idle(struct radeon_device *rdev);
> @@ -2818,6 +2821,8 @@ struct radeon_device *radeon_get_rdev(struct ttm_device *bdev);
>   
>   /* KMS */
>   
> +int radeon_driver_load_kms(struct radeon_device *dev, unsigned long flags);
> +
>   u32 radeon_get_vblank_counter_kms(struct drm_crtc *crtc);
>   int radeon_enable_vblank_kms(struct drm_crtc *crtc);
>   void radeon_disable_vblank_kms(struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 7bf08164140e..ae9cadceba83 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -259,7 +259,8 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>   			    const struct pci_device_id *ent)
>   {
>   	unsigned long flags = 0;
> -	struct drm_device *dev;
> +	struct drm_device *ddev;
> +	struct radeon_device *rdev;
>   	int ret;
>   
>   	if (!ent)
> @@ -300,28 +301,37 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>   	if (ret)
>   		return ret;
>   
> -	dev = drm_dev_alloc(&kms_driver, &pdev->dev);
> -	if (IS_ERR(dev))
> -		return PTR_ERR(dev);
> +	rdev = devm_drm_dev_alloc(&pdev->dev, &kms_driver, typeof(*rdev), ddev);
> +	if (IS_ERR(rdev))
> +		return PTR_ERR(rdev);
> +
> +	rdev->dev  = &pdev->dev;
> +	rdev->pdev = pdev;
> +	ddev = rdev_to_drm(rdev);
> +	ddev->dev_private = rdev;
>   
>   	ret = pci_enable_device(pdev);
>   	if (ret)
>   		goto err_free;
>   
> -	pci_set_drvdata(pdev, dev);
> +	pci_set_drvdata(pdev, ddev);
> +
> +	ret = radeon_driver_load_kms(rdev, flags);
> +	if (ret)
> +		goto err_agp;
>   
> -	ret = drm_dev_register(dev, ent->driver_data);
> +	ret = drm_dev_register(ddev, flags);
>   	if (ret)
>   		goto err_agp;
>   
> -	radeon_fbdev_setup(dev->dev_private);
> +	radeon_fbdev_setup(ddev->dev_private);
>   
>   	return 0;
>   
>   err_agp:
>   	pci_disable_device(pdev);
>   err_free:
> -	drm_dev_put(dev);
> +	drm_dev_put(ddev);
>   	return ret;
>   }
>   
> @@ -569,7 +579,6 @@ static const struct drm_ioctl_desc radeon_ioctls_kms[] = {
>   static const struct drm_driver kms_driver = {
>   	.driver_features =
>   	    DRIVER_GEM | DRIVER_RENDER | DRIVER_MODESET,
> -	.load = radeon_driver_load_kms,
>   	.open = radeon_driver_open_kms,
>   	.postclose = radeon_driver_postclose_kms,
>   	.unload = radeon_driver_unload_kms,
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.h b/drivers/gpu/drm/radeon/radeon_drv.h
> index 02a65971d140..6c1eb75a951b 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.h
> +++ b/drivers/gpu/drm/radeon/radeon_drv.h
> @@ -117,7 +117,6 @@
>   long radeon_drm_ioctl(struct file *filp,
>   		      unsigned int cmd, unsigned long arg);
>   
> -int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags);
>   void radeon_driver_unload_kms(struct drm_device *dev);
>   int radeon_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv);
>   void radeon_driver_postclose_kms(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> index a16590c6247f..d2df194393af 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -91,7 +91,7 @@ void radeon_driver_unload_kms(struct drm_device *dev)
>   /**
>    * radeon_driver_load_kms - Main load function for KMS.
>    *
> - * @dev: drm dev pointer
> + * @rdev: radeon dev pointer
>    * @flags: device flags
>    *
>    * This is the main load function for KMS (all asics).
> @@ -101,24 +101,18 @@ void radeon_driver_unload_kms(struct drm_device *dev)
>    * (crtcs, encoders, hotplug detect, etc.).
>    * Returns 0 on success, error on failure.
>    */
> -int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
> +int radeon_driver_load_kms(struct radeon_device *rdev, unsigned long flags)
>   {
> -	struct pci_dev *pdev = to_pci_dev(dev->dev);
> -	struct radeon_device *rdev;
> +	struct pci_dev *pdev = rdev->pdev;
> +	struct drm_device *dev = rdev_to_drm(rdev);
>   	int r, acpi_status;
>   
> -	rdev = kzalloc(sizeof(struct radeon_device), GFP_KERNEL);
> -	if (rdev == NULL) {
> -		return -ENOMEM;
> -	}
> -	dev->dev_private = (void *)rdev;
> -
>   #ifdef __alpha__
>   	rdev->hose = pdev->sysdata;
>   #endif
>   
>   	if (pci_find_capability(pdev, PCI_CAP_ID_AGP))
> -		rdev->agp = radeon_agp_head_init(dev);
> +		rdev->agp = radeon_agp_head_init(rdev_to_drm(rdev));
>   	if (rdev->agp) {
>   		rdev->agp->agp_mtrr = arch_phys_wc_add(
>   			rdev->agp->agp_info.aper_base,
> @@ -147,7 +141,7 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
>   	 * properly initialize the GPU MC controller and permit
>   	 * VRAM allocation
>   	 */
> -	r = radeon_device_init(rdev, dev, pdev, flags);
> +	r = radeon_device_init(rdev, flags);
>   	if (r) {
>   		dev_err(dev->dev, "Fatal error during GPU init\n");
>   		goto out;

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)