| Message ID | 20230828094736.4137092-3-stanislaw.gruszka@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | accel/ivpu: Update for -next 2023.08.25 | expand |
On 8/28/2023 3:47 AM, Stanislaw Gruszka wrote: > From: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com> > > Reduce the number of error messages per single failure in > ivpu_dev_init(). Error messages are already printed by functions > called from ivpu_dev_init(). > > Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com> > Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> > --- > drivers/accel/ivpu/ivpu_drv.c | 54 +++++++-------------------- > drivers/accel/ivpu/ivpu_fw.c | 2 +- > drivers/accel/ivpu/ivpu_ipc.c | 13 +++++-- > drivers/accel/ivpu/ivpu_mmu_context.c | 6 ++- > drivers/accel/ivpu/ivpu_pm.c | 4 +- > drivers/accel/ivpu/ivpu_pm.h | 2 +- > 6 files changed, 29 insertions(+), 52 deletions(-) > > diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c > index ba79f397c9e8..b10b2909f05f 100644 > --- a/drivers/accel/ivpu/ivpu_drv.c > +++ b/drivers/accel/ivpu/ivpu_drv.c > @@ -518,78 +518,52 @@ static int ivpu_dev_init(struct ivpu_device *vdev) > lockdep_set_class(&vdev->submitted_jobs_xa.xa_lock, &submitted_jobs_xa_lock_class_key); > > ret = ivpu_pci_init(vdev); > - if (ret) { > - ivpu_err(vdev, "Failed to initialize PCI device: %d\n", ret); > + if (ret) > goto err_xa_destroy; > - } > > ret = ivpu_irq_init(vdev); > - if (ret) { > - ivpu_err(vdev, "Failed to initialize IRQs: %d\n", ret); > + if (ret) > goto err_xa_destroy; > - } > > /* Init basic HW info based on buttress registers which are accessible before power up */ > ret = ivpu_hw_info_init(vdev); > - if (ret) { > - ivpu_err(vdev, "Failed to initialize HW info: %d\n", ret); > + if (ret) > goto err_xa_destroy; > - } > > /* Power up early so the rest of init code can access VPU registers */ > ret = ivpu_hw_power_up(vdev); > - if (ret) { > - ivpu_err(vdev, "Failed to power up HW: %d\n", ret); > + if (ret) > goto err_xa_destroy; > - } > > ret = ivpu_mmu_global_context_init(vdev); > - if (ret) { > - ivpu_err(vdev, "Failed to initialize global MMU context: %d\n", ret); > + if (ret) > goto err_power_down; > - } > > ret = ivpu_mmu_init(vdev); > - if (ret) { > - ivpu_err(vdev, "Failed to initialize MMU device: %d\n", ret); > + if (ret) > goto err_mmu_gctx_fini; > - } > > ret = ivpu_fw_init(vdev); > - if (ret) { > - ivpu_err(vdev, "Failed to initialize firmware: %d\n", ret); > + if (ret) > goto err_mmu_gctx_fini; > - } > > ret = ivpu_ipc_init(vdev); > - if (ret) { > - ivpu_err(vdev, "Failed to initialize IPC: %d\n", ret); > + if (ret) > goto err_fw_fini; > - } > > - ret = ivpu_pm_init(vdev); > - if (ret) { > - ivpu_err(vdev, "Failed to initialize PM: %d\n", ret); > - goto err_ipc_fini; > - } > + ivpu_pm_init(vdev); > > ret = ivpu_job_done_thread_init(vdev); > - if (ret) { > - ivpu_err(vdev, "Failed to initialize job done thread: %d\n", ret); > + if (ret) > goto err_ipc_fini; > - } > > ret = ivpu_fw_load(vdev); > - if (ret) { > - ivpu_err(vdev, "Failed to load firmware: %d\n", ret); > + if (ret) > goto err_job_done_thread_fini; > - } > > ret = ivpu_boot(vdev); > - if (ret) { > - ivpu_err(vdev, "Failed to boot: %d\n", ret); > + if (ret) > goto err_job_done_thread_fini; > - } > > ivpu_pm_enable(vdev); > > @@ -651,10 +625,8 @@ static int ivpu_probe(struct pci_dev *pdev, const struct pci_device_id *id) > pci_set_drvdata(pdev, vdev); > > ret = ivpu_dev_init(vdev); > - if (ret) { > - dev_err(&pdev->dev, "Failed to initialize VPU device: %d\n", ret); > + if (ret) > return ret; > - } Commit text doesn't mention anything about this change. > > ret = drm_dev_register(&vdev->drm, 0); > if (ret) { > diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c > index 9827ea4d7b83..9b6ecd3e9537 100644 > --- a/drivers/accel/ivpu/ivpu_fw.c > +++ b/drivers/accel/ivpu/ivpu_fw.c > @@ -78,7 +78,7 @@ static int ivpu_fw_request(struct ivpu_device *vdev) > } > } > > - ivpu_err(vdev, "Failed to request firmware: %d\n", ret); > + ivpu_err(vdev, "Failed to load firmware: %d\n", ret); Commit text doesn't mention anything about this. Nor do I understand it as this function doesn't load the firmware. Everything below this point seems to be not related to $SUBJECT and feels like should be a separate patch. > return ret; > } > > diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c > index fa0af59e39ab..6b2e9dbb284a 100644 > --- a/drivers/accel/ivpu/ivpu_ipc.c > +++ b/drivers/accel/ivpu/ivpu_ipc.c > @@ -426,15 +426,20 @@ int ivpu_ipc_irq_handler(struct ivpu_device *vdev) > int ivpu_ipc_init(struct ivpu_device *vdev) > { > struct ivpu_ipc_info *ipc = vdev->ipc; > - int ret = -ENOMEM; > + int ret; > > ipc->mem_tx = ivpu_bo_alloc_internal(vdev, 0, SZ_16K, DRM_IVPU_BO_WC); > - if (!ipc->mem_tx) > - return ret; > + if (!ipc->mem_tx) { > + ivpu_err(vdev, "Failed to allocate mem_tx\n"); > + return -ENOMEM; > + } > > ipc->mem_rx = ivpu_bo_alloc_internal(vdev, 0, SZ_16K, DRM_IVPU_BO_WC); > - if (!ipc->mem_rx) > + if (!ipc->mem_rx) { > + ivpu_err(vdev, "Failed to allocate mem_rx\n"); > + ret = -ENOMEM; > goto err_free_tx; > + } > > ipc->mm_tx = devm_gen_pool_create(vdev->drm.dev, __ffs(IVPU_IPC_ALIGNMENT), > -1, "TX_IPC_JSM"); > diff --git a/drivers/accel/ivpu/ivpu_mmu_context.c b/drivers/accel/ivpu/ivpu_mmu_context.c > index 1d2e554e2c4a..1144d6eb5120 100644 > --- a/drivers/accel/ivpu/ivpu_mmu_context.c > +++ b/drivers/accel/ivpu/ivpu_mmu_context.c > @@ -427,8 +427,10 @@ ivpu_mmu_context_init(struct ivpu_device *vdev, struct ivpu_mmu_context *ctx, u3 > INIT_LIST_HEAD(&ctx->bo_list); > > ret = ivpu_mmu_pgtable_init(vdev, &ctx->pgtable); > - if (ret) > + if (ret) { > + ivpu_err(vdev, "Failed to initialize pgtable for ctx %u: %d\n", context_id, ret); > return ret; > + } > > if (!context_id) { > start = vdev->hw->ranges.global.start; > @@ -488,7 +490,7 @@ int ivpu_mmu_user_context_init(struct ivpu_device *vdev, struct ivpu_mmu_context > > ret = ivpu_mmu_context_init(vdev, ctx, ctx_id); > if (ret) { > - ivpu_err(vdev, "Failed to initialize context: %d\n", ret); > + ivpu_err(vdev, "Failed to initialize context %u: %d\n", ctx_id, ret); > return ret; > } > > diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c > index 954c9ecd3e14..28a0d1111e12 100644 > --- a/drivers/accel/ivpu/ivpu_pm.c > +++ b/drivers/accel/ivpu/ivpu_pm.c > @@ -282,7 +282,7 @@ void ivpu_pm_reset_done_cb(struct pci_dev *pdev) > pm_runtime_put_autosuspend(vdev->drm.dev); > } > > -int ivpu_pm_init(struct ivpu_device *vdev) > +void ivpu_pm_init(struct ivpu_device *vdev) > { > struct device *dev = vdev->drm.dev; > struct ivpu_pm_info *pm = vdev->pm; > @@ -303,8 +303,6 @@ int ivpu_pm_init(struct ivpu_device *vdev) > pm_runtime_set_autosuspend_delay(dev, delay); > > ivpu_dbg(vdev, PM, "Autosuspend delay = %d\n", delay); > - > - return 0; > } > > void ivpu_pm_cancel_recovery(struct ivpu_device *vdev) > diff --git a/drivers/accel/ivpu/ivpu_pm.h b/drivers/accel/ivpu/ivpu_pm.h > index fd4eada1290f..f41c30a14a40 100644 > --- a/drivers/accel/ivpu/ivpu_pm.h > +++ b/drivers/accel/ivpu/ivpu_pm.h > @@ -19,7 +19,7 @@ struct ivpu_pm_info { > u32 suspend_reschedule_counter; > }; > > -int ivpu_pm_init(struct ivpu_device *vdev); > +void ivpu_pm_init(struct ivpu_device *vdev); > void ivpu_pm_enable(struct ivpu_device *vdev); > void ivpu_pm_disable(struct ivpu_device *vdev); > void ivpu_pm_cancel_recovery(struct ivpu_device *vdev);
On Mon, Aug 28, 2023 at 02:41:50PM -0600, Jeffrey Hugo wrote: > > ivpu_pm_enable(vdev); > > @@ -651,10 +625,8 @@ static int ivpu_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > pci_set_drvdata(pdev, vdev); > > ret = ivpu_dev_init(vdev); > > - if (ret) { > > - dev_err(&pdev->dev, "Failed to initialize VPU device: %d\n", ret); > > + if (ret) > > return ret; > > - } > > Commit text doesn't mention anything about this change. Will add to commit message info about this. > > ret = drm_dev_register(&vdev->drm, 0); > > if (ret) { > > diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c > > index 9827ea4d7b83..9b6ecd3e9537 100644 > > --- a/drivers/accel/ivpu/ivpu_fw.c > > +++ b/drivers/accel/ivpu/ivpu_fw.c > > @@ -78,7 +78,7 @@ static int ivpu_fw_request(struct ivpu_device *vdev) > > } > > } > > - ivpu_err(vdev, "Failed to request firmware: %d\n", ret); > > + ivpu_err(vdev, "Failed to load firmware: %d\n", ret); > > Commit text doesn't mention anything about this. Nor do I understand it as > this function doesn't load the firmware. I'll remove this change. > Everything below this point seems to be not related to $SUBJECT and feels > like should be a separate patch. I'll add to commit massage that we also add missed prints in functions called from ivpu_dev_init(). And for remaining changes I'll prepare separate patches, one for ivpu_mmu_context.c context and one for ivpu_pm_init(). Thanks Stanislaw
diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c index ba79f397c9e8..b10b2909f05f 100644 --- a/drivers/accel/ivpu/ivpu_drv.c +++ b/drivers/accel/ivpu/ivpu_drv.c @@ -518,78 +518,52 @@ static int ivpu_dev_init(struct ivpu_device *vdev) lockdep_set_class(&vdev->submitted_jobs_xa.xa_lock, &submitted_jobs_xa_lock_class_key); ret = ivpu_pci_init(vdev); - if (ret) { - ivpu_err(vdev, "Failed to initialize PCI device: %d\n", ret); + if (ret) goto err_xa_destroy; - } ret = ivpu_irq_init(vdev); - if (ret) { - ivpu_err(vdev, "Failed to initialize IRQs: %d\n", ret); + if (ret) goto err_xa_destroy; - } /* Init basic HW info based on buttress registers which are accessible before power up */ ret = ivpu_hw_info_init(vdev); - if (ret) { - ivpu_err(vdev, "Failed to initialize HW info: %d\n", ret); + if (ret) goto err_xa_destroy; - } /* Power up early so the rest of init code can access VPU registers */ ret = ivpu_hw_power_up(vdev); - if (ret) { - ivpu_err(vdev, "Failed to power up HW: %d\n", ret); + if (ret) goto err_xa_destroy; - } ret = ivpu_mmu_global_context_init(vdev); - if (ret) { - ivpu_err(vdev, "Failed to initialize global MMU context: %d\n", ret); + if (ret) goto err_power_down; - } ret = ivpu_mmu_init(vdev); - if (ret) { - ivpu_err(vdev, "Failed to initialize MMU device: %d\n", ret); + if (ret) goto err_mmu_gctx_fini; - } ret = ivpu_fw_init(vdev); - if (ret) { - ivpu_err(vdev, "Failed to initialize firmware: %d\n", ret); + if (ret) goto err_mmu_gctx_fini; - } ret = ivpu_ipc_init(vdev); - if (ret) { - ivpu_err(vdev, "Failed to initialize IPC: %d\n", ret); + if (ret) goto err_fw_fini; - } - ret = ivpu_pm_init(vdev); - if (ret) { - ivpu_err(vdev, "Failed to initialize PM: %d\n", ret); - goto err_ipc_fini; - } + ivpu_pm_init(vdev); ret = ivpu_job_done_thread_init(vdev); - if (ret) { - ivpu_err(vdev, "Failed to initialize job done thread: %d\n", ret); + if (ret) goto err_ipc_fini; - } ret = ivpu_fw_load(vdev); - if (ret) { - ivpu_err(vdev, "Failed to load firmware: %d\n", ret); + if (ret) goto err_job_done_thread_fini; - } ret = ivpu_boot(vdev); - if (ret) { - ivpu_err(vdev, "Failed to boot: %d\n", ret); + if (ret) goto err_job_done_thread_fini; - } ivpu_pm_enable(vdev); @@ -651,10 +625,8 @@ static int ivpu_probe(struct pci_dev *pdev, const struct pci_device_id *id) pci_set_drvdata(pdev, vdev); ret = ivpu_dev_init(vdev); - if (ret) { - dev_err(&pdev->dev, "Failed to initialize VPU device: %d\n", ret); + if (ret) return ret; - } ret = drm_dev_register(&vdev->drm, 0); if (ret) { diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c index 9827ea4d7b83..9b6ecd3e9537 100644 --- a/drivers/accel/ivpu/ivpu_fw.c +++ b/drivers/accel/ivpu/ivpu_fw.c @@ -78,7 +78,7 @@ static int ivpu_fw_request(struct ivpu_device *vdev) } } - ivpu_err(vdev, "Failed to request firmware: %d\n", ret); + ivpu_err(vdev, "Failed to load firmware: %d\n", ret); return ret; } diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c index fa0af59e39ab..6b2e9dbb284a 100644 --- a/drivers/accel/ivpu/ivpu_ipc.c +++ b/drivers/accel/ivpu/ivpu_ipc.c @@ -426,15 +426,20 @@ int ivpu_ipc_irq_handler(struct ivpu_device *vdev) int ivpu_ipc_init(struct ivpu_device *vdev) { struct ivpu_ipc_info *ipc = vdev->ipc; - int ret = -ENOMEM; + int ret; ipc->mem_tx = ivpu_bo_alloc_internal(vdev, 0, SZ_16K, DRM_IVPU_BO_WC); - if (!ipc->mem_tx) - return ret; + if (!ipc->mem_tx) { + ivpu_err(vdev, "Failed to allocate mem_tx\n"); + return -ENOMEM; + } ipc->mem_rx = ivpu_bo_alloc_internal(vdev, 0, SZ_16K, DRM_IVPU_BO_WC); - if (!ipc->mem_rx) + if (!ipc->mem_rx) { + ivpu_err(vdev, "Failed to allocate mem_rx\n"); + ret = -ENOMEM; goto err_free_tx; + } ipc->mm_tx = devm_gen_pool_create(vdev->drm.dev, __ffs(IVPU_IPC_ALIGNMENT), -1, "TX_IPC_JSM"); diff --git a/drivers/accel/ivpu/ivpu_mmu_context.c b/drivers/accel/ivpu/ivpu_mmu_context.c index 1d2e554e2c4a..1144d6eb5120 100644 --- a/drivers/accel/ivpu/ivpu_mmu_context.c +++ b/drivers/accel/ivpu/ivpu_mmu_context.c @@ -427,8 +427,10 @@ ivpu_mmu_context_init(struct ivpu_device *vdev, struct ivpu_mmu_context *ctx, u3 INIT_LIST_HEAD(&ctx->bo_list); ret = ivpu_mmu_pgtable_init(vdev, &ctx->pgtable); - if (ret) + if (ret) { + ivpu_err(vdev, "Failed to initialize pgtable for ctx %u: %d\n", context_id, ret); return ret; + } if (!context_id) { start = vdev->hw->ranges.global.start; @@ -488,7 +490,7 @@ int ivpu_mmu_user_context_init(struct ivpu_device *vdev, struct ivpu_mmu_context ret = ivpu_mmu_context_init(vdev, ctx, ctx_id); if (ret) { - ivpu_err(vdev, "Failed to initialize context: %d\n", ret); + ivpu_err(vdev, "Failed to initialize context %u: %d\n", ctx_id, ret); return ret; } diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c index 954c9ecd3e14..28a0d1111e12 100644 --- a/drivers/accel/ivpu/ivpu_pm.c +++ b/drivers/accel/ivpu/ivpu_pm.c @@ -282,7 +282,7 @@ void ivpu_pm_reset_done_cb(struct pci_dev *pdev) pm_runtime_put_autosuspend(vdev->drm.dev); } -int ivpu_pm_init(struct ivpu_device *vdev) +void ivpu_pm_init(struct ivpu_device *vdev) { struct device *dev = vdev->drm.dev; struct ivpu_pm_info *pm = vdev->pm; @@ -303,8 +303,6 @@ int ivpu_pm_init(struct ivpu_device *vdev) pm_runtime_set_autosuspend_delay(dev, delay); ivpu_dbg(vdev, PM, "Autosuspend delay = %d\n", delay); - - return 0; } void ivpu_pm_cancel_recovery(struct ivpu_device *vdev) diff --git a/drivers/accel/ivpu/ivpu_pm.h b/drivers/accel/ivpu/ivpu_pm.h index fd4eada1290f..f41c30a14a40 100644 --- a/drivers/accel/ivpu/ivpu_pm.h +++ b/drivers/accel/ivpu/ivpu_pm.h @@ -19,7 +19,7 @@ struct ivpu_pm_info { u32 suspend_reschedule_counter; }; -int ivpu_pm_init(struct ivpu_device *vdev); +void ivpu_pm_init(struct ivpu_device *vdev); void ivpu_pm_enable(struct ivpu_device *vdev); void ivpu_pm_disable(struct ivpu_device *vdev); void ivpu_pm_cancel_recovery(struct ivpu_device *vdev);