I'll turn it into a dev_warn and I'll make the error message more
helpful. I think this happens when a new DID has been added to amdgpu,
but not to KFD yet. That's probably worth a warning.
Regards,
Felix
On 2017-08-14 11:50 PM, Zhao, Yong wrote:
>
> Oded, I agree with you. When I made the change, there was WARN_ON
> already in the same function lookup_device_info(), so I followed the
> suit and used WARN again. It is indeed a bit overkill.
>
>
> Felix, do I need to fix it or can you fix it directly?
>
>
> Yong
>
> ------------------------------------------------------------------------
> *From:* Oded Gabbay <oded.gabbay at gmail.com>
> *Sent:* Saturday, August 12, 2017 10:54:41 AM
> *To:* Kuehling, Felix
> *Cc:* amd-gfx list; Zhao, Yong
> *Subject:* Re: [PATCH 14/19] drm/amdkfd: Add more error printing to
> help bringup
>
> On Sat, Aug 12, 2017 at 12:56 AM, Felix Kuehling
> <Felix.Kuehling at amd.com> wrote:
> > From: Yong Zhao <Yong.Zhao at amd.com>
> >
> > Signed-off-by: Yong Zhao <Yong.Zhao at amd.com>
> > Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_device.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > index f628ac3..e1c2ad2 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > @@ -103,6 +103,8 @@ static const struct kfd_device_info
> *lookup_device_info(unsigned short did)
> > }
> > }
> >
> > + WARN(1, "device is not added to supported_devices\n");
> > +
> I think WARN is a bit excessive here. Its not actually a warning - an
> AMD gpu device is present but not supported in amdkfd.
> Maybe a dev_info is more appropriate here.
>
> Oded
>
> > return NULL;
> > }
> >
> > @@ -114,8 +116,10 @@ struct kfd_dev *kgd2kfd_probe(struct kgd_dev *kgd,
> > const struct kfd_device_info *device_info =
> >
> lookup_device_info(pdev->device);
> >
> > - if (!device_info)
> > + if (!device_info) {
> > + dev_err(kfd_device, "kgd2kfd_probe failed\n");
> > return NULL;
> > + }
> >
> > kfd = kzalloc(sizeof(*kfd), GFP_KERNEL);
> > if (!kfd)
> > @@ -364,8 +368,11 @@ int kgd2kfd_resume(struct kfd_dev *kfd)
> >
> > if (kfd->init_complete) {
> > err = amd_iommu_init_device(kfd->pdev, pasid_limit);
> > - if (err < 0)
> > + if (err < 0) {
> > + dev_err(kfd_device, "failed to initialize
> iommu\n");
> > return -ENXIO;
> > + }
> > +
> > amd_iommu_set_invalidate_ctx_cb(kfd->pdev,
> >
> iommu_pasid_shutdown_callback);
> > amd_iommu_set_invalid_ppr_cb(kfd->pdev,
> iommu_invalid_ppr_cb);
> > --
> > 2.7.4
> >
> With the above fixed, this patch is:
> Reviewed-by: Oded Gabbay <oded.gabbay at gmail.com>
