[PATCH] drm/amdgpu: move some release handles into fail labels

[Huang Rui]

Ah, sorry, this is my mistake to miss the "return 0;". Marek, thanks a lot
to point out. I will take care next time.

Christian, I see you already send the patch to fix it. Thanks.

Thanks,
Rui

On Wed, Sep 14, 2016 at 10:36:16PM +0800, Koenig, Christian wrote:
> Thanks for that info, I was busy most of the day bisecting this 
> regression as well.
> 
> Yeah, the bug is rather obvious, there is a "return 0;" in the good case 
> missing here.
> 
> So after loading the firmware we release it again. I will hack together 
> a patch, just give me a minute.
> 
> Christian.
> 
> Am 14.09.2016 um 16:21 schrieb Marek Olšák:
> > Hi,
> >
> > This patch causes a massive performance drop (by a factor of 100)
> > followed by GPU hangs with F1 2015 (it needs more than 3GB of memory,
> > the card has 2GB). The game normally works really well (30+ FPS on
> > Polaris11 2GB, 40+ FPS on Tonga 2GB).
> >
> > It should be fixed or the commit should be reverted really quickly.
> >
> > Marek
> >
> >
> > On Mon, Sep 12, 2016 at 4:16 AM, Huang Rui <ray.huang at amd.com> wrote:
> >> Clean up the codes to move the release handles into fail labels.
> >>
> >> Signed-off-by: Huang Rui <ray.huang at amd.com>
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 21 ++++++++++-----------
> >>   1 file changed, 10 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> >> index 5cc95f1..4a5c211 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> >> @@ -247,35 +247,28 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
> >>          const struct common_firmware_header *header = NULL;
> >>
> >>          err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true,
> >> -                       AMDGPU_GEM_DOMAIN_GTT, 0, NULL, NULL, bo);
> >> +                              AMDGPU_GEM_DOMAIN_GTT, 0, NULL, NULL, bo);
> >>          if (err) {
> >>                  dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err);
> >> -               err = -ENOMEM;
> >>                  goto failed;
> >>          }
> >>
> >>          err = amdgpu_bo_reserve(*bo, false);
> >>          if (err) {
> >> -               amdgpu_bo_unref(bo);
> >>                  dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err);
> >> -               goto failed;
> >> +               goto failed_reserve;
> >>          }
> >>
> >>          err = amdgpu_bo_pin(*bo, AMDGPU_GEM_DOMAIN_GTT, &fw_mc_addr);
> >>          if (err) {
> >> -               amdgpu_bo_unreserve(*bo);
> >> -               amdgpu_bo_unref(bo);
> >>                  dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err);
> >> -               goto failed;
> >> +               goto failed_pin;
> >>          }
> >>
> >>          err = amdgpu_bo_kmap(*bo, &fw_buf_ptr);
> >>          if (err) {
> >>                  dev_err(adev->dev, "(%d) Firmware buffer kmap failed\n", err);
> >> -               amdgpu_bo_unpin(*bo);
> >> -               amdgpu_bo_unreserve(*bo);
> >> -               amdgpu_bo_unref(bo);
> >> -               goto failed;
> >> +               goto failed_kmap;
> >>          }
> >>
> >>          amdgpu_bo_unreserve(*bo);
> >> @@ -291,6 +284,12 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
> >>                  }
> >>          }
> >>
> >> +failed_kmap:
> >> +       amdgpu_bo_unpin(*bo);
> >> +failed_pin:
> >> +       amdgpu_bo_unreserve(*bo);
> >> +failed_reserve:
> >> +       amdgpu_bo_unref(bo);
> >>   failed:
> >>          if (err)
> >>                  adev->firmware.smu_load = false;
> >> --
> >> 2.7.4
> >>
> >> _______________________________________________
> >> amd-gfx mailing list
> >> amd-gfx at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
>