[PATCH] drm/amdgpu: Fix ENOSYS means 'invalid syscall nr' in amdgpu_device.c
Lazar, Lijo
lijo.lazar at amd.com
Tue Jul 25 14:02:19 UTC 2023
On 7/23/2023 11:45 AM, Srinivasan Shanmugam wrote:
> ENOSYS should be used for nonexistent syscalls only, replace ENOSYS with
> EINVAL & other style fixes
>
> WARNING: ENOSYS means 'invalid syscall nr' and nothing else
> + if (r == -ENOSYS)
>
> WARNING: ENOSYS means 'invalid syscall nr' and nothing else
> + if (r == -ENOSYS)
>
> 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>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 60 +++++++++++-----------
> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 4 +-
> 2 files changed, 33 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 1c786190a84e..ec166c797b9a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -159,7 +159,7 @@ static ssize_t amdgpu_device_get_pcie_replay_count(struct device *dev,
> return sysfs_emit(buf, "%llu\n", cnt);
> }
>
> -static DEVICE_ATTR(pcie_replay_count, S_IRUGO,
> +static DEVICE_ATTR(pcie_replay_count, 0444,
> amdgpu_device_get_pcie_replay_count, NULL);
>
> static void amdgpu_device_get_pcie_info(struct amdgpu_device *adev);
> @@ -183,7 +183,7 @@ static ssize_t amdgpu_device_get_product_name(struct device *dev,
> return sysfs_emit(buf, "%s\n", adev->product_name);
> }
>
> -static DEVICE_ATTR(product_name, S_IRUGO,
> +static DEVICE_ATTR(product_name, 0444,
> amdgpu_device_get_product_name, NULL);
>
> /**
> @@ -205,7 +205,7 @@ static ssize_t amdgpu_device_get_product_number(struct device *dev,
> return sysfs_emit(buf, "%s\n", adev->product_number);
> }
>
> -static DEVICE_ATTR(product_number, S_IRUGO,
> +static DEVICE_ATTR(product_number, 0444,
> amdgpu_device_get_product_number, NULL);
>
> /**
> @@ -227,7 +227,7 @@ static ssize_t amdgpu_device_get_serial_number(struct device *dev,
> return sysfs_emit(buf, "%s\n", adev->serial);
> }
>
> -static DEVICE_ATTR(serial_number, S_IRUGO,
> +static DEVICE_ATTR(serial_number, 0444,
> amdgpu_device_get_serial_number, NULL);
>
> /**
> @@ -481,8 +481,7 @@ uint32_t amdgpu_device_rreg(struct amdgpu_device *adev,
> /*
> * MMIO register read with bytes helper functions
> * @offset:bytes offset from MMIO start
> - *
> -*/
> + */
>
> /**
> * amdgpu_mm_rreg8 - read a memory mapped IO register
> @@ -506,8 +505,8 @@ uint8_t amdgpu_mm_rreg8(struct amdgpu_device *adev, uint32_t offset)
> * MMIO register write with bytes helper functions
> * @offset:bytes offset from MMIO start
> * @value: the value want to be written to the register
> - *
> -*/
> + */
> +
> /**
> * amdgpu_mm_wreg8 - read a memory mapped IO register
> *
> @@ -991,7 +990,7 @@ static void amdgpu_device_mem_scratch_fini(struct amdgpu_device *adev)
> * @registers: pointer to the register array
> * @array_size: size of the register array
> *
> - * Programs an array or registers with and and or masks.
> + * Programs an array or registers with and or masks.
> * This is a helper for setting golden registers.
> */
> void amdgpu_device_program_register_sequence(struct amdgpu_device *adev,
> @@ -1157,7 +1156,7 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
> int rbar_size = pci_rebar_bytes_to_size(adev->gmc.real_vram_size);
> struct pci_bus *root;
> struct resource *res;
> - unsigned i;
> + unsigned int i;
> u16 cmd;
> int r;
>
> @@ -1226,9 +1225,8 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
>
> static bool amdgpu_device_read_bios(struct amdgpu_device *adev)
> {
> - if (hweight32(adev->aid_mask) && (adev->flags & AMD_IS_APU)) {
> + if (hweight32(adev->aid_mask) && (adev->flags & AMD_IS_APU))
> return false;
> - }
>
> return true;
> }
> @@ -1264,6 +1262,7 @@ bool amdgpu_device_need_post(struct amdgpu_device *adev)
> if (adev->asic_type == CHIP_FIJI) {
> int err;
> uint32_t fw_ver;
> +
> err = request_firmware(&adev->pm.fw, "amdgpu/fiji_smc.bin", adev->dev);
> /* force vPost if error occured */
> if (err)
> @@ -1366,6 +1365,7 @@ static unsigned int amdgpu_device_vga_set_decode(struct pci_dev *pdev,
> bool state)
> {
> struct amdgpu_device *adev = drm_to_adev(pci_get_drvdata(pdev));
> +
> amdgpu_asic_set_vga_state(adev, state);
> if (state)
> return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM |
> @@ -1388,7 +1388,8 @@ static void amdgpu_device_check_block_size(struct amdgpu_device *adev)
> {
> /* defines number of bits in page table versus page directory,
> * a page is 4KB so we have 12 bits offset, minimum 9 bits in the
> - * page table and the remaining bits are in the page directory */
> + * page table and the remaining bits are in the page directory
> + */
> if (amdgpu_vm_block_size == -1)
> return;
>
> @@ -1620,7 +1621,7 @@ static bool amdgpu_switcheroo_can_switch(struct pci_dev *pdev)
> {
> struct drm_device *dev = pci_get_drvdata(pdev);
>
> - /*
> + /*
> * FIXME: open_count is protected by drm_global_mutex but that would lead to
> * locking inversion with the driver load path. And the access here is
> * completely racy anyway. So don't bother with locking for now.
> @@ -3265,7 +3266,7 @@ static int amdgpu_device_ip_resume_phase2(struct amdgpu_device *adev)
> *
> * Main resume function for hardware IPs. The hardware IPs
> * are split into two resume functions because they are
> - * are also used in in recovering from a GPU reset and some additional
> + * also used in recovering from a GPU reset and some additional
> * steps need to be take between them. In this case (S3/S4) they are
> * run sequentially.
> * Returns 0 on success, negative error code on failure.
> @@ -3367,8 +3368,7 @@ bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type)
> #else
> default:
> if (amdgpu_dc > 0)
> - DRM_INFO_ONCE("Display Core has been requested via kernel parameter "
> - "but isn't supported by ASIC, ignoring\n");
> + DRM_INFO_ONCE("Display Core has been requested via kernel parameter but isn't supported by ASIC, ignoring\n");
> return false;
> #endif
> }
> @@ -3616,7 +3616,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> pdev->subsystem_vendor, pdev->subsystem_device, pdev->revision);
>
> /* mutex initialization are all done here so we
> - * can recall function without having locking issues */
> + * can recall function without having locking issues
> + */
> mutex_init(&adev->firmware.mutex);
> mutex_init(&adev->pm.mutex);
> mutex_init(&adev->gfx.gpu_clock_mutex);
> @@ -3693,11 +3694,11 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> atomic_set(&adev->pm.pwr_state[i], POWER_STATE_UNKNOWN);
>
> adev->rmmio = ioremap(adev->rmmio_base, adev->rmmio_size);
> - if (adev->rmmio == NULL) {
> + if (!adev->rmmio)
> return -ENOMEM;
> - }
> +
> DRM_INFO("register mmio base: 0x%08X\n", (uint32_t)adev->rmmio_base);
> - DRM_INFO("register mmio size: %u\n", (unsigned)adev->rmmio_size);
> + DRM_INFO("register mmio size: %u\n", (unsigned int)adev->rmmio_size);
>
> /*
> * Reset domain needs to be present early, before XGMI hive discovered
> @@ -3951,7 +3952,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>
> /* if we have > 1 VGA cards, then disable the amdgpu VGA resources */
> /* this will fail for cards that aren't VGA class devices, just
> - * ignore it */
> + * ignore it
> + */
> if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
> vga_client_register(adev->pdev, amdgpu_device_vga_set_decode);
>
> @@ -4034,7 +4036,7 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
>
> /* make sure IB test finished before entering exclusive mode
> * to avoid preemption on IB test
> - * */
> + */
> if (amdgpu_sriov_vf(adev)) {
> amdgpu_virt_request_full_gpu(adev, false);
> amdgpu_virt_fini_data_exchange(adev);
> @@ -4771,8 +4773,9 @@ int amdgpu_device_pre_asic_reset(struct amdgpu_device *adev,
> if (!ring || !ring->sched.thread)
> continue;
>
> - /*clear job fence from fence drv to avoid force_completion
> - *leave NULL and vm flush fence in fence drv */
> + /* Clear job fence from fence drv to avoid force_completion
> + * leave NULL and vm flush fence in fence drv
> + */
> amdgpu_fence_driver_clear_job_fences(ring);
>
> /* after all hw jobs are reset, hw fence is meaningless, so force_completion */
> @@ -4786,7 +4789,7 @@ int amdgpu_device_pre_asic_reset(struct amdgpu_device *adev,
>
> r = amdgpu_reset_prepare_hwcontext(adev, reset_context);
> /* If reset handler not implemented, continue; otherwise return */
> - if (r == -ENOSYS)
> + if (r == -EINVAL)
ENOSYS is defined as /* Function not implemented */ on some platforms.
If not for ENOSYS, please use ENOTSUPP here instead.
> r = 0;
> else
> return r;
> @@ -4904,7 +4907,7 @@ int amdgpu_do_asic_reset(struct list_head *device_list_handle,
> reset_context->reset_device_list = device_list_handle;
> r = amdgpu_reset_perform_reset(tmp_adev, reset_context);
> /* If reset handler not implemented, continue; otherwise return */
> - if (r == -ENOSYS)
> + if (r == -EINVAL)
Please use ENOTSUPP instead.
> r = 0;
> else
> return r;
> @@ -5393,9 +5396,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
> if (adev->enable_mes && adev->ip_versions[GC_HWIP][0] != IP_VERSION(11, 0, 3))
> amdgpu_mes_self_test(tmp_adev);
>
> - if (!drm_drv_uses_atomic_modeset(adev_to_drm(tmp_adev)) && !job_signaled) {
> + if (!drm_drv_uses_atomic_modeset(adev_to_drm(tmp_adev)) && !job_signaled)
> drm_helper_resume_force_mode(adev_to_drm(tmp_adev));
> - }
>
> if (tmp_adev->asic_reset_res)
> r = tmp_adev->asic_reset_res;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> index eec41ad30406..12515e40b693 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> @@ -87,7 +87,7 @@ int amdgpu_reset_prepare_hwcontext(struct amdgpu_device *adev,
> reset_handler = adev->reset_cntl->get_reset_handler(
> adev->reset_cntl, reset_context);
> if (!reset_handler)
> - return -ENOSYS;
> + return -EINVAL;
Please use ENOTSUPP instead.
>
> return reset_handler->prepare_hwcontext(adev->reset_cntl,
> reset_context);
> @@ -103,7 +103,7 @@ int amdgpu_reset_perform_reset(struct amdgpu_device *adev,
> reset_handler = adev->reset_cntl->get_reset_handler(
> adev->reset_cntl, reset_context);
> if (!reset_handler)
> - return -ENOSYS;
> + return -EINVAL;
Please use ENOTSUPP instead.
Thanks,
Lijo
>
> ret = reset_handler->perform_reset(adev->reset_cntl, reset_context);
> if (ret)
More information about the amd-gfx
mailing list