On Sat, Sep 16, 2017 at 2:42 AM, Felix Kuehling <Felix.Kuehling at amd.com> wrote:
> From: Yong Zhao <yong.zhao at amd.com>
>
> The idea is to let kfd init and resume function share the same code path
> as much as possible, rather than to have two copies of almost identical
> code. That way improves the code readability and maintainability.
>
> 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 | 78 +++++++++++++++++----------------
> 1 file changed, 40 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 61fff25..cc8af11 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -92,6 +92,8 @@ static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size,
> unsigned int chunk_size);
> static void kfd_gtt_sa_fini(struct kfd_dev *kfd);
>
> +static int kfd_resume(struct kfd_dev *kfd);
> +
> static const struct kfd_device_info *lookup_device_info(unsigned short did)
> {
> size_t i;
> @@ -176,15 +178,8 @@ static bool device_iommu_pasid_init(struct kfd_dev *kfd)
> pasid_limit,
> kfd->doorbell_process_limit - 1);
>
> - err = amd_iommu_init_device(kfd->pdev, pasid_limit);
> - if (err < 0) {
> - dev_err(kfd_device, "error initializing iommu device\n");
> - return false;
> - }
> -
> if (!kfd_set_pasid_limit(pasid_limit)) {
> dev_err(kfd_device, "error setting pasid limit\n");
> - amd_iommu_free_device(kfd->pdev);
> return false;
> }
>
> @@ -280,29 +275,22 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
> goto kfd_interrupt_error;
> }
>
> - if (!device_iommu_pasid_init(kfd)) {
> - dev_err(kfd_device,
> - "Error initializing iommuv2 for device %x:%x\n",
> - kfd->pdev->vendor, kfd->pdev->device);
> - goto device_iommu_pasid_error;
> - }
> - amd_iommu_set_invalidate_ctx_cb(kfd->pdev,
> - iommu_pasid_shutdown_callback);
> - amd_iommu_set_invalid_ppr_cb(kfd->pdev, iommu_invalid_ppr_cb);
> -
> kfd->dqm = device_queue_manager_init(kfd);
> if (!kfd->dqm) {
> dev_err(kfd_device, "Error initializing queue manager\n");
> goto device_queue_manager_error;
> }
>
> - if (kfd->dqm->ops.start(kfd->dqm)) {
> + if (!device_iommu_pasid_init(kfd)) {
> dev_err(kfd_device,
> - "Error starting queue manager for device %x:%x\n",
> + "Error initializing iommuv2 for device %x:%x\n",
> kfd->pdev->vendor, kfd->pdev->device);
> - goto dqm_start_error;
> + goto device_iommu_pasid_error;
> }
>
> + if (kfd_resume(kfd))
> + goto kfd_resume_error;
> +
> kfd->dbgmgr = NULL;
>
> kfd->init_complete = true;
> @@ -314,11 +302,10 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
>
> goto out;
>
> -dqm_start_error:
> +kfd_resume_error:
> +device_iommu_pasid_error:
> device_queue_manager_uninit(kfd->dqm);
> device_queue_manager_error:
> - amd_iommu_free_device(kfd->pdev);
> -device_iommu_pasid_error:
> kfd_interrupt_exit(kfd);
> kfd_interrupt_error:
> kfd_topology_remove_device(kfd);
> @@ -338,8 +325,8 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
> void kgd2kfd_device_exit(struct kfd_dev *kfd)
> {
> if (kfd->init_complete) {
> + kgd2kfd_suspend(kfd);
> device_queue_manager_uninit(kfd->dqm);
> - amd_iommu_free_device(kfd->pdev);
> kfd_interrupt_exit(kfd);
> kfd_topology_remove_device(kfd);
> kfd_doorbell_fini(kfd);
> @@ -362,25 +349,40 @@ void kgd2kfd_suspend(struct kfd_dev *kfd)
>
> int kgd2kfd_resume(struct kfd_dev *kfd)
> {
> - unsigned int pasid_limit;
> - int err;
> + if (!kfd->init_complete)
> + return 0;
>
> - pasid_limit = kfd_get_pasid_limit();
> + return kfd_resume(kfd);
>
> - if (kfd->init_complete) {
> - err = amd_iommu_init_device(kfd->pdev, pasid_limit);
> - 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);
> - kfd->dqm->ops.start(kfd->dqm);
> +static int kfd_resume(struct kfd_dev *kfd)
> +{
> + int err = 0;
> + unsigned int pasid_limit = kfd_get_pasid_limit();
> +
> + err = amd_iommu_init_device(kfd->pdev, pasid_limit);
> + if (err)
> + 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);
> +
> + err = kfd->dqm->ops.start(kfd->dqm);
> + if (err) {
> + dev_err(kfd_device,
> + "Error starting queue manager for device %x:%x\n",
> + kfd->pdev->vendor, kfd->pdev->device);
> + goto dqm_start_error;
> }
>
> - return 0;
> + return err;
> +
> +dqm_start_error:
> + amd_iommu_free_device(kfd->pdev);
> +
> + return err;
> }
>
> /* This is called directly from KGD at ISR. */
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
This patch is:
Reviewed-by: Oded Gabbay <oded.gabbay at gmail.com>
