[PATCH 2/3] drm/amdgpu:fix vf_error_put

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Liu, Monk
> Sent: Thursday, October 19, 2017 10:51 PM
> To: Alex Deucher
> Cc: amd-gfx list
> Subject: RE: [PATCH 2/3] drm/amdgpu:fix vf_error_put
> 
> ”amdgpu_vf_error_put“ now is already changed to protected by
> SRIOV_VF(), so the whole logic is okay
> 

Well, you removed all the other calls to error_put in the patch so it seems like we should protect this as well for consistency.  Either way:
Acked-by: Alex Deucher <alexander.deucher at amd.com>

Alex

> 
> -----Original Message-----
> From: Alex Deucher [mailto:alexdeucher at gmail.com]
> Sent: 2017年10月19日 10:58
> To: Liu, Monk <Monk.Liu at amd.com>
> Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>
> Subject: Re: [PATCH 2/3] drm/amdgpu:fix vf_error_put
> 
> On Wed, Oct 18, 2017 at 5:59 AM, Monk Liu <Monk.Liu at amd.com> wrote:
> > it should not work on non-SR-IOV case
> >
> > Change-Id: Icf34f88a0642e4b04c857a07b72df3bb772075d6
> > Signed-off-by: Monk Liu <Monk.Liu at amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   | 7 +++----
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vf_error.c | 7 ++++++-
> >  2 files changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 8354af6..c43f8d4 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -2049,6 +2049,9 @@ static void
> amdgpu_device_detect_sriov_bios(struct amdgpu_device *adev)
> >                 if (amdgpu_atombios_has_gpu_virtualization_table(adev))
> >                         adev->virt.caps |= AMDGPU_SRIOV_CAPS_SRIOV_VBIOS;
> >         }
> > +
> > +       if (!(adev->virt.caps & AMDGPU_SRIOV_CAPS_SRIOV_VBIOS))
> > +               amdgpu_vf_error_put(adev, AMDGIM_ERROR_VF_NO_VBIOS, 0,
> > + 0);
> 
> Shouldn't this be protected with amdgpu_sriov_vf()?  Other than that, looks
> good to me.
> 
> Alex
> 
> >  }
> >
> >  bool amdgpu_device_asic_has_dc_support(enum amd_asic_type
> asic_type)
> > @@ -2260,7 +2263,6 @@ int amdgpu_device_init(struct amdgpu_device
> *adev,
> >         if (amdgpu_vpost_needed(adev)) {
> >                 if (!adev->bios) {
> >                         dev_err(adev->dev, "no vBIOS found\n");
> > -                       amdgpu_vf_error_put(adev,
> AMDGIM_ERROR_VF_NO_VBIOS, 0, 0);
> >                         r = -EINVAL;
> >                         goto failed;
> >                 }
> > @@ -2268,7 +2270,6 @@ int amdgpu_device_init(struct amdgpu_device
> *adev,
> >                 r = amdgpu_atom_asic_init(adev->mode_info.atom_context);
> >                 if (r) {
> >                         dev_err(adev->dev, "gpu post error!\n");
> > -                       amdgpu_vf_error_put(adev,
> AMDGIM_ERROR_VF_GPU_POST_ERROR, 0, 0);
> >                         goto failed;
> >                 }
> >         } else {
> > @@ -3096,7 +3097,6 @@ int amdgpu_gpu_reset(struct amdgpu_device
> *adev)
> >                 }
> >         } else {
> >                 dev_err(adev->dev, "asic resume failed (%d).\n", r);
> > -               amdgpu_vf_error_put(adev,
> AMDGIM_ERROR_VF_ASIC_RESUME_FAIL, 0, r);
> >                 for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> >                         if (adev->rings[i] && adev->rings[i]->sched.thread) {
> >
> > kthread_unpark(adev->rings[i]->sched.thread);
> > @@ -3114,7 +3114,6 @@ int amdgpu_gpu_reset(struct amdgpu_device
> *adev)
> >         if (r) {
> >                 /* bad news, how to tell it to userspace ? */
> >                 dev_info(adev->dev, "GPU reset failed\n");
> > -               amdgpu_vf_error_put(adev,
> AMDGIM_ERROR_VF_GPU_RESET_FAIL, 0, r);
> >         }
> >         else {
> >                 dev_info(adev->dev, "GPU reset successed!\n"); diff
> > --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vf_error.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vf_error.c
> > index 746b813..7f70979 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vf_error.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vf_error.c
> > @@ -31,7 +31,12 @@ void amdgpu_vf_error_put(struct amdgpu_device
> *adev,
> >                          uint64_t error_data)  {
> >         int index;
> > -       uint16_t error_code =
> AMDGIM_ERROR_CODE(AMDGIM_ERROR_CATEGORY_VF,
> sub_error_code);
> > +       uint16_t error_code;
> > +
> > +       if (!amdgpu_sriov_vf(adev))
> > +               return;
> > +
> > +       error_code =
> AMDGIM_ERROR_CODE(AMDGIM_ERROR_CATEGORY_VF,
> > + sub_error_code);
> >
> >         mutex_lock(&adev->virt.vf_errors.lock);
> >         index = adev->virt.vf_errors.write_count %
> > AMDGPU_VF_ERROR_ENTRY_SIZE;
> > --
> > 2.7.4
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx