答复: [PATCH] gpu: drm: fix an improper check of amdgpu_bo_create_kernel
Comments in line. Thanks JimQu 发件人: amd-gfx 代表 Kangjie Lu 发送时间: 2018年12月26日 14:23 收件人: k...@umn.edu 抄送: Zhou, David(ChunMing); David Airlie; Xu, Feifei; Francis, David; linux-kernel@vger.kernel.org; amd-...@lists.freedesktop.org; Huang, Ray; dri-de...@lists.freedesktop.org; Daniel Vetter; pakki...@umn.edu; Deucher, Alexander; Gao, Likun; Zhu, Rex; Koenig, Christian; Zhang, Hawking 主题: [PATCH] gpu: drm: fix an improper check of amdgpu_bo_create_kernel adev->firmware.fw_buf being not NULL may not indicate kernel buffer is created successful. A better way is to check the status (return value) of it. The fix does so. Signed-off-by: Kangjie Lu --- drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c index 7b33867036e7..ba3c1cfb2c35 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c @@ -422,13 +422,19 @@ static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode, int amdgpu_ucode_create_bo(struct amdgpu_device *adev) { + int ret; + if (adev->firmware.load_type != AMDGPU_FW_LOAD_DIRECT) { - amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, PAGE_SIZE, - amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, - >firmware.fw_buf, - >firmware.fw_buf_mc, - >firmware.fw_buf_ptr); - if (!adev->firmware.fw_buf) { + ret = + amdgpu_bo_create_kernel(adev, + adev->firmware.fw_size, + PAGE_SIZE, + amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : + AMDGPU_GEM_DOMAIN_GTT, + >firmware.fw_buf, + >firmware.fw_buf_mc, + >firmware.fw_buf_ptr); + if (ret) { JimQu: Look into amdgpu_bo_create_kernel(), new bo will be created only if *bo_ptr = NULL, So if you encounter the issue the adev->firmware.fw_buf != NULL but new bo is not created, the only problem I can see are: 1. there is bug in amdgpu_bo_create_kernel() 2. adev->firmware.fw_buf != NULL before it is transferred into amdgpu_bo_create_kernel(). Could you commit what is environment you encounter the issue? Thanks JimQu dev_err(adev->dev, "failed to create kernel buffer for firmware.fw_buf\n"); return -ENOMEM; } else if (amdgpu_sriov_vf(adev)) { -- 2.17.2 (Apple Git-113) ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] gpu: drm: fix an improper check of amdgpu_bo_create_kernel
> -Original Message- > From: Kangjie Lu [mailto:k...@umn.edu] > Sent: Wednesday, December 26, 2018 2:24 PM > To: k...@umn.edu > Cc: pakki...@umn.edu; Deucher, Alexander > ; Koenig, Christian > ; Zhou, David(ChunMing) > ; David Airlie ; Daniel Vetter > ; Zhu, Rex ; Huang, Ray > ; Zhang, Hawking ; Xu, > Feifei ; Gao, Likun ; Francis, > David ; amd-...@lists.freedesktop.org; dri- > de...@lists.freedesktop.org; linux-kernel@vger.kernel.org > Subject: [PATCH] gpu: drm: fix an improper check of > amdgpu_bo_create_kernel > > adev->firmware.fw_buf being not NULL may not indicate kernel buffer is > created successful. A better way is to check the status (return value) > of it. The fix does so. Actually, it is the same. If bo is created successfully, the amdgpu_bo object will be created. But using "ret" to align with other function should be better as the return status. Thanks. Reviewed-by: Huang Rui > > Signed-off-by: Kangjie Lu > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 18 -- > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c > index 7b33867036e7..ba3c1cfb2c35 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c > @@ -422,13 +422,19 @@ static int amdgpu_ucode_patch_jt(struct > amdgpu_firmware_info *ucode, > > int amdgpu_ucode_create_bo(struct amdgpu_device *adev) > { > + int ret; > + > if (adev->firmware.load_type != AMDGPU_FW_LOAD_DIRECT) { > - amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, > PAGE_SIZE, > - amdgpu_sriov_vf(adev) ? > AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, > - >firmware.fw_buf, > - >firmware.fw_buf_mc, > - >firmware.fw_buf_ptr); > - if (!adev->firmware.fw_buf) { > + ret = > + amdgpu_bo_create_kernel(adev, > + adev->firmware.fw_size, > + PAGE_SIZE, > + amdgpu_sriov_vf(adev) ? > AMDGPU_GEM_DOMAIN_VRAM : > + AMDGPU_GEM_DOMAIN_GTT, > + >firmware.fw_buf, > + >firmware.fw_buf_mc, > + >firmware.fw_buf_ptr); > + if (ret) { > dev_err(adev->dev, "failed to create kernel buffer > for firmware.fw_buf\n"); > return -ENOMEM; > } else if (amdgpu_sriov_vf(adev)) { > -- > 2.17.2 (Apple Git-113)
[PATCH] gpu: drm: fix an improper check of amdgpu_bo_create_kernel
adev->firmware.fw_buf being not NULL may not indicate kernel buffer is created successful. A better way is to check the status (return value) of it. The fix does so. Signed-off-by: Kangjie Lu --- drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c index 7b33867036e7..ba3c1cfb2c35 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c @@ -422,13 +422,19 @@ static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode, int amdgpu_ucode_create_bo(struct amdgpu_device *adev) { + int ret; + if (adev->firmware.load_type != AMDGPU_FW_LOAD_DIRECT) { - amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, PAGE_SIZE, - amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT, - >firmware.fw_buf, - >firmware.fw_buf_mc, - >firmware.fw_buf_ptr); - if (!adev->firmware.fw_buf) { + ret = + amdgpu_bo_create_kernel(adev, + adev->firmware.fw_size, + PAGE_SIZE, + amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : + AMDGPU_GEM_DOMAIN_GTT, + >firmware.fw_buf, + >firmware.fw_buf_mc, + >firmware.fw_buf_ptr); + if (ret) { dev_err(adev->dev, "failed to create kernel buffer for firmware.fw_buf\n"); return -ENOMEM; } else if (amdgpu_sriov_vf(adev)) { -- 2.17.2 (Apple Git-113)