[PATCH v2] drm/amdgpu: Move externs to amdgpu.h file from amdgpu_drv.c

Christian König christian.koenig at amd.com
Thu Jul 27 07:33:06 UTC 2023


Am 25.07.23 um 06:51 schrieb Srinivasan Shanmugam:
> Fixes the following:
>
> WARNING: externs should be avoided in .c files
> +extern const struct attribute_group amdgpu_vram_mgr_attr_group;
>
> WARNING: externs should be avoided in .c files
> +extern const struct attribute_group amdgpu_gtt_mgr_attr_group;
>
> WARNING: externs should be avoided in .c files
> +extern const struct attribute_group amdgpu_flash_attr_group;
>
> And other style fixes:
>
> ERROR: do not initialise globals to 0
> WARNING: Block comments should align the * on each line
> WARNING: void function return statements are not generally useful
> WARNING: braces {} are not necessary for single statement blocks

In general good idea, but we want to slowly decompose amdgpu.h and not 
add new stuff.

Please move the vram and gtt defines into amdgpu_ttm.h and take a look 
where the flash extern can be moved instead.

Thanks,
Christian.

>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> ---
>
> v2:
>   - Updated commit message - Added "ERROR: do not initialise globals to
>     0"
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  4 ++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 28 +++++++++----------------
>   2 files changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index a046160b6a0e..93d0f4c7b560 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1524,4 +1524,8 @@ static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
>   
>   int amdgpu_in_reset(struct amdgpu_device *adev);
>   
> +extern const struct attribute_group amdgpu_vram_mgr_attr_group;
> +extern const struct attribute_group amdgpu_gtt_mgr_attr_group;
> +extern const struct attribute_group amdgpu_flash_attr_group;
> +
>   #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index c315fe390e71..84446bbc3509 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -313,9 +313,7 @@ module_param_named(msi, amdgpu_msi, int, 0444);
>    * jobs is 10000. The timeout for compute is 60000.
>    */
>   MODULE_PARM_DESC(lockup_timeout, "GPU lockup timeout in ms (default: for bare metal 10000 for non-compute jobs and 60000 for compute jobs; "
> -		"for passthrough or sriov, 10000 for all jobs."
> -		" 0: keep default value. negative: infinity timeout), "
> -		"format: for bare metal [Non-Compute] or [GFX,Compute,SDMA,Video]; "
> +		"for passthrough or sriov, 10000 for all jobs. 0: keep default value. negative: infinity timeout), format: for bare metal [Non-Compute] or [GFX,Compute,SDMA,Video]; "
>   		"for passthrough or sriov [all jobs] or [GFX,Compute,SDMA,Video].");
>   module_param_string(lockup_timeout, amdgpu_lockup_timeout, sizeof(amdgpu_lockup_timeout), 0444);
>   
> @@ -585,7 +583,7 @@ module_param_named(timeout_period, amdgpu_watchdog_timer.period, uint, 0644);
>   #ifdef CONFIG_DRM_AMDGPU_SI
>   
>   #if IS_ENABLED(CONFIG_DRM_RADEON) || IS_ENABLED(CONFIG_DRM_RADEON_MODULE)
> -int amdgpu_si_support = 0;
> +int amdgpu_si_support;
>   MODULE_PARM_DESC(si_support, "SI support (1 = enabled, 0 = disabled (default))");
>   #else
>   int amdgpu_si_support = 1;
> @@ -604,7 +602,7 @@ module_param_named(si_support, amdgpu_si_support, int, 0444);
>   #ifdef CONFIG_DRM_AMDGPU_CIK
>   
>   #if IS_ENABLED(CONFIG_DRM_RADEON) || IS_ENABLED(CONFIG_DRM_RADEON_MODULE)
> -int amdgpu_cik_support = 0;
> +int amdgpu_cik_support;
>   MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled (default))");
>   #else
>   int amdgpu_cik_support = 1;
> @@ -620,8 +618,7 @@ module_param_named(cik_support, amdgpu_cik_support, int, 0444);
>    * E.g. 0x1 = 256Mbyte, 0x2 = 512Mbyte, 0x4 = 1 Gbyte, 0x8 = 2GByte. The default is 0 (disabled).
>    */
>   MODULE_PARM_DESC(smu_memory_pool_size,
> -	"reserve gtt for smu debug usage, 0 = disable,"
> -		"0x1 = 256Mbyte, 0x2 = 512Mbyte, 0x4 = 1 Gbyte, 0x8 = 2GByte");
> +	"reserve gtt for smu debug usage, 0 = disable,0x1 = 256Mbyte, 0x2 = 512Mbyte, 0x4 = 1 Gbyte, 0x8 = 2GByte");
>   module_param_named(smu_memory_pool_size, amdgpu_smu_memory_pool_size, uint, 0444);
>   
>   /**
> @@ -791,9 +788,9 @@ module_param(hws_gws_support, bool, 0444);
>   MODULE_PARM_DESC(hws_gws_support, "Assume MEC2 FW supports GWS barriers (false = rely on FW version check (Default), true = force supported)");
>   
>   /**
> -  * DOC: queue_preemption_timeout_ms (int)
> -  * queue preemption timeout in ms (1 = Minimum, 9000 = default)
> -  */
> + * DOC: queue_preemption_timeout_ms (int)
> + * queue preemption timeout in ms (1 = Minimum, 9000 = default)
> + */
>   int queue_preemption_timeout_ms = 9000;
>   module_param(queue_preemption_timeout_ms, int, 0644);
>   MODULE_PARM_DESC(queue_preemption_timeout_ms, "queue preemption timeout in ms (1 = Minimum, 9000 = default)");
> @@ -2417,7 +2414,6 @@ static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work)
>   			amdgpu_amdkfd_device_init(adev);
>   		amdgpu_ttm_set_buffer_funcs_status(adev, true);
>   	}
> -	return;
>   }
>   
>   static int amdgpu_pmops_prepare(struct device *dev)
> @@ -2614,6 +2610,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>   	/* wait for all rings to drain before suspending */
>   	for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
>   		struct amdgpu_ring *ring = adev->rings[i];
> +
>   		if (ring && ring->sched.ready) {
>   			ret = amdgpu_fence_wait_empty(ring);
>   			if (ret)
> @@ -2738,6 +2735,7 @@ long amdgpu_drm_ioctl(struct file *filp,
>   	struct drm_file *file_priv = filp->private_data;
>   	struct drm_device *dev;
>   	long ret;
> +
>   	dev = file_priv->minor->dev;
>   	ret = pm_runtime_get_sync(dev->dev);
>   	if (ret < 0)
> @@ -2802,9 +2800,8 @@ int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv)
>   	if (!filp)
>   		return -EINVAL;
>   
> -	if (filp->f_op != &amdgpu_driver_kms_fops) {
> +	if (filp->f_op != &amdgpu_driver_kms_fops)
>   		return -EINVAL;
> -	}
>   
>   	file = filp->private_data;
>   	*fpriv = file->driver_priv;
> @@ -2894,10 +2891,6 @@ static struct pci_error_handlers amdgpu_pci_err_handler = {
>   	.resume		= amdgpu_pci_resume,
>   };
>   
> -extern const struct attribute_group amdgpu_vram_mgr_attr_group;
> -extern const struct attribute_group amdgpu_gtt_mgr_attr_group;
> -extern const struct attribute_group amdgpu_flash_attr_group;
> -
>   static const struct attribute_group *amdgpu_sysfs_groups[] = {
>   	&amdgpu_vram_mgr_attr_group,
>   	&amdgpu_gtt_mgr_attr_group,
> @@ -2905,7 +2898,6 @@ static const struct attribute_group *amdgpu_sysfs_groups[] = {
>   	NULL,
>   };
>   
> -
>   static struct pci_driver amdgpu_kms_pci_driver = {
>   	.name = DRIVER_NAME,
>   	.id_table = pciidlist,



More information about the amd-gfx mailing list