* [PATCH v4 00/10] Some Panfrost fixes and improvements
@ 2025-10-01 2:20 Adrián Larumbe
2025-10-01 2:20 ` [PATCH v4 01/10] drm/panfrost: Replace DRM driver allocation method with newer one Adrián Larumbe
` (9 more replies)
0 siblings, 10 replies; 20+ messages in thread
From: Adrián Larumbe @ 2025-10-01 2:20 UTC (permalink / raw)
To: linux-kernel
Cc: dri-devel, Steven Price, Boris Brezillon, kernel,
Adrián Larumbe
This is v4 of https://lore.kernel.org/dri-devel/20241128211223.1805830-1-adrian.larumbe@collabora.com/
This patch series is a collection of minor fixes and improvements I came up
with while working on driver related stuff.
Changelog:
v4:
- Rebased older patch series onto latest drm-misc-next
- Added patch for renaming JM functions to reflect their actual role
- Fixed treatment of error code in perfcnt when enabling sample buffer AS
v3:
- Minor convenience fixes to patches 3 and 4 in the series
- Move unmapping of maped range of BO to the function's error path
in case of BO mapping failure, also for putting BO's pages
- Split patch 6/8 into two: one makes sure the Job IRQ enablement mask
isn't recalculated at every device reset and uses the same expression
everywhere in the driver, and another one that breaks the enablement
function into two stages.
v2:
- Removed commit that provided an explicit fence cleanup function
- Added commit for removing unused Panfrost device structure member
- Refactored how optional job interrupt reenabling during reset is handled
- Make the way errors and successful return values are delivered from inside
panfrost_mmu_as_get more according to standard.
- Simplify unmapping of already mapped area when mapping the pages of a BO
- Fixing management of runtime-PM reference counts when failing HW job submission.
Adrián Larumbe (10):
drm/panfrost: Replace DRM driver allocation method with newer one
drm/panfrost: Handle inexistent GPU during probe
drm/panfrost: Handle job HW submit errors
drm/panfrost: Handle error when allocating AS number
drm/panfrost: Handle page mapping failure
drm/panfrost: Don't rework job IRQ enable mask in the enable path
drm/panfrost: Make re-enabling job interrupts at device reset optional
drm/panfrost: Add forward declaration and types header
drm/panfrost: Remove unused device property
drm/panfrost: Rename panfrost_job functions to reflect real role
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 4 +-
drivers/gpu/drm/panfrost/panfrost_device.c | 68 ++++-----
drivers/gpu/drm/panfrost/panfrost_device.h | 13 +-
drivers/gpu/drm/panfrost/panfrost_drv.c | 78 ++++------
drivers/gpu/drm/panfrost/panfrost_dump.c | 8 +-
drivers/gpu/drm/panfrost/panfrost_gem.c | 8 +-
.../gpu/drm/panfrost/panfrost_gem_shrinker.c | 4 +-
drivers/gpu/drm/panfrost/panfrost_gpu.c | 64 +++++----
drivers/gpu/drm/panfrost/panfrost_job.c | 135 +++++++++---------
drivers/gpu/drm/panfrost/panfrost_job.h | 15 +-
drivers/gpu/drm/panfrost/panfrost_mmu.c | 101 +++++++++----
drivers/gpu/drm/panfrost/panfrost_mmu.h | 3 +-
drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 27 ++--
13 files changed, 293 insertions(+), 235 deletions(-)
base-commit: 1a93d70801421ca506807ced45566490976d532a
--
2.51.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 01/10] drm/panfrost: Replace DRM driver allocation method with newer one 2025-10-01 2:20 [PATCH v4 00/10] Some Panfrost fixes and improvements Adrián Larumbe @ 2025-10-01 2:20 ` Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 02/10] drm/panfrost: Handle inexistent GPU during probe Adrián Larumbe ` (8 subsequent siblings) 9 siblings, 0 replies; 20+ messages in thread From: Adrián Larumbe @ 2025-10-01 2:20 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Steven Price, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Philipp Zabel Drop the deprecated DRM driver allocation method in favour of devm_drm_dev_alloc(). Overall just make it the same as in Panthor. Also discard now superfluous generic and platform device pointers inside the main panfrost device structure. Some ancient checkpatch issues unearthed as a result of these changes were also fixed, like lines too long or double assignment in one line. Reviewed-by: Steven Price <steven.price@arm.com> Acked-by: Boris Brezillon <boris.brezillon@collabora.com> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 4 +- drivers/gpu/drm/panfrost/panfrost_device.c | 49 ++++++------ drivers/gpu/drm/panfrost/panfrost_device.h | 6 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 74 ++++++++----------- drivers/gpu/drm/panfrost/panfrost_dump.c | 8 +- drivers/gpu/drm/panfrost/panfrost_gem.c | 8 +- .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 4 +- drivers/gpu/drm/panfrost/panfrost_gpu.c | 49 ++++++------ drivers/gpu/drm/panfrost/panfrost_job.c | 37 +++++----- drivers/gpu/drm/panfrost/panfrost_mmu.c | 46 ++++++------ drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 18 ++--- 11 files changed, 146 insertions(+), 157 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index 5d0dce10336b..ac05df2a54fe 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -74,7 +74,7 @@ static int panfrost_devfreq_get_dev_status(struct device *dev, spin_unlock_irqrestore(&pfdevfreq->lock, irqflags); - dev_dbg(pfdev->dev, "busy %lu total %lu %lu %% freq %lu MHz\n", + dev_dbg(pfdev->base.dev, "busy %lu total %lu %lu %% freq %lu MHz\n", status->busy_time, status->total_time, status->busy_time / (status->total_time / 100), status->current_frequency / 1000 / 1000); @@ -119,7 +119,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) int ret; struct dev_pm_opp *opp; unsigned long cur_freq; - struct device *dev = &pfdev->pdev->dev; + struct device *dev = pfdev->base.dev; struct devfreq *devfreq; struct thermal_cooling_device *cooling; struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq; diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 04bec27449cb..733b728ec75f 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -20,9 +20,9 @@ static int panfrost_reset_init(struct panfrost_device *pfdev) { - pfdev->rstc = devm_reset_control_array_get_optional_exclusive(pfdev->dev); + pfdev->rstc = devm_reset_control_array_get_optional_exclusive(pfdev->base.dev); if (IS_ERR(pfdev->rstc)) { - dev_err(pfdev->dev, "get reset failed %ld\n", PTR_ERR(pfdev->rstc)); + dev_err(pfdev->base.dev, "get reset failed %ld\n", PTR_ERR(pfdev->rstc)); return PTR_ERR(pfdev->rstc); } @@ -39,22 +39,22 @@ static int panfrost_clk_init(struct panfrost_device *pfdev) int err; unsigned long rate; - pfdev->clock = devm_clk_get(pfdev->dev, NULL); + pfdev->clock = devm_clk_get(pfdev->base.dev, NULL); if (IS_ERR(pfdev->clock)) { - dev_err(pfdev->dev, "get clock failed %ld\n", PTR_ERR(pfdev->clock)); + dev_err(pfdev->base.dev, "get clock failed %ld\n", PTR_ERR(pfdev->clock)); return PTR_ERR(pfdev->clock); } rate = clk_get_rate(pfdev->clock); - dev_info(pfdev->dev, "clock rate = %lu\n", rate); + dev_info(pfdev->base.dev, "clock rate = %lu\n", rate); err = clk_prepare_enable(pfdev->clock); if (err) return err; - pfdev->bus_clock = devm_clk_get_optional(pfdev->dev, "bus"); + pfdev->bus_clock = devm_clk_get_optional(pfdev->base.dev, "bus"); if (IS_ERR(pfdev->bus_clock)) { - dev_err(pfdev->dev, "get bus_clock failed %ld\n", + dev_err(pfdev->base.dev, "get bus_clock failed %ld\n", PTR_ERR(pfdev->bus_clock)); err = PTR_ERR(pfdev->bus_clock); goto disable_clock; @@ -62,7 +62,7 @@ static int panfrost_clk_init(struct panfrost_device *pfdev) if (pfdev->bus_clock) { rate = clk_get_rate(pfdev->bus_clock); - dev_info(pfdev->dev, "bus_clock rate = %lu\n", rate); + dev_info(pfdev->base.dev, "bus_clock rate = %lu\n", rate); err = clk_prepare_enable(pfdev->bus_clock); if (err) @@ -87,7 +87,7 @@ static int panfrost_regulator_init(struct panfrost_device *pfdev) { int ret, i; - pfdev->regulators = devm_kcalloc(pfdev->dev, pfdev->comp->num_supplies, + pfdev->regulators = devm_kcalloc(pfdev->base.dev, pfdev->comp->num_supplies, sizeof(*pfdev->regulators), GFP_KERNEL); if (!pfdev->regulators) @@ -96,12 +96,12 @@ static int panfrost_regulator_init(struct panfrost_device *pfdev) for (i = 0; i < pfdev->comp->num_supplies; i++) pfdev->regulators[i].supply = pfdev->comp->supply_names[i]; - ret = devm_regulator_bulk_get(pfdev->dev, + ret = devm_regulator_bulk_get(pfdev->base.dev, pfdev->comp->num_supplies, pfdev->regulators); if (ret < 0) { if (ret != -EPROBE_DEFER) - dev_err(pfdev->dev, "failed to get regulators: %d\n", + dev_err(pfdev->base.dev, "failed to get regulators: %d\n", ret); return ret; } @@ -109,7 +109,7 @@ static int panfrost_regulator_init(struct panfrost_device *pfdev) ret = regulator_bulk_enable(pfdev->comp->num_supplies, pfdev->regulators); if (ret < 0) { - dev_err(pfdev->dev, "failed to enable regulators: %d\n", ret); + dev_err(pfdev->base.dev, "failed to enable regulators: %d\n", ret); return ret; } @@ -144,7 +144,7 @@ static int panfrost_pm_domain_init(struct panfrost_device *pfdev) int err; int i, num_domains; - num_domains = of_count_phandle_with_args(pfdev->dev->of_node, + num_domains = of_count_phandle_with_args(pfdev->base.dev->of_node, "power-domains", "#power-domain-cells"); @@ -156,7 +156,7 @@ static int panfrost_pm_domain_init(struct panfrost_device *pfdev) return 0; if (num_domains != pfdev->comp->num_pm_domains) { - dev_err(pfdev->dev, + dev_err(pfdev->base.dev, "Incorrect number of power domains: %d provided, %d needed\n", num_domains, pfdev->comp->num_pm_domains); return -EINVAL; @@ -168,20 +168,21 @@ static int panfrost_pm_domain_init(struct panfrost_device *pfdev) for (i = 0; i < num_domains; i++) { pfdev->pm_domain_devs[i] = - dev_pm_domain_attach_by_name(pfdev->dev, - pfdev->comp->pm_domain_names[i]); + dev_pm_domain_attach_by_name(pfdev->base.dev, + pfdev->comp->pm_domain_names[i]); if (IS_ERR_OR_NULL(pfdev->pm_domain_devs[i])) { err = PTR_ERR(pfdev->pm_domain_devs[i]) ? : -ENODATA; pfdev->pm_domain_devs[i] = NULL; - dev_err(pfdev->dev, + dev_err(pfdev->base.dev, "failed to get pm-domain %s(%d): %d\n", pfdev->comp->pm_domain_names[i], i, err); goto err; } - pfdev->pm_domain_links[i] = device_link_add(pfdev->dev, - pfdev->pm_domain_devs[i], DL_FLAG_PM_RUNTIME | - DL_FLAG_STATELESS | DL_FLAG_RPM_ACTIVE); + pfdev->pm_domain_links[i] = + device_link_add(pfdev->base.dev, + pfdev->pm_domain_devs[i], DL_FLAG_PM_RUNTIME | + DL_FLAG_STATELESS | DL_FLAG_RPM_ACTIVE); if (!pfdev->pm_domain_links[i]) { dev_err(pfdev->pm_domain_devs[i], "adding device link failed!\n"); @@ -220,20 +221,20 @@ int panfrost_device_init(struct panfrost_device *pfdev) err = panfrost_reset_init(pfdev); if (err) { - dev_err(pfdev->dev, "reset init failed %d\n", err); + dev_err(pfdev->base.dev, "reset init failed %d\n", err); goto out_pm_domain; } err = panfrost_clk_init(pfdev); if (err) { - dev_err(pfdev->dev, "clk init failed %d\n", err); + dev_err(pfdev->base.dev, "clk init failed %d\n", err); goto out_reset; } err = panfrost_devfreq_init(pfdev); if (err) { if (err != -EPROBE_DEFER) - dev_err(pfdev->dev, "devfreq init failed %d\n", err); + dev_err(pfdev->base.dev, "devfreq init failed %d\n", err); goto out_clk; } @@ -244,7 +245,7 @@ int panfrost_device_init(struct panfrost_device *pfdev) goto out_devfreq; } - pfdev->iomem = devm_platform_ioremap_resource(pfdev->pdev, 0); + pfdev->iomem = devm_platform_ioremap_resource(to_platform_device(pfdev->base.dev), 0); if (IS_ERR(pfdev->iomem)) { err = PTR_ERR(pfdev->iomem); goto out_regulator; diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index 1e73efad02a8..474b232bb38e 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -124,9 +124,7 @@ struct panfrost_device_debugfs { }; struct panfrost_device { - struct device *dev; - struct drm_device *ddev; - struct platform_device *pdev; + struct drm_device base; int gpu_irq; int mmu_irq; @@ -222,7 +220,7 @@ static inline bool panfrost_high_prio_allowed(struct drm_file *file) static inline struct panfrost_device *to_panfrost_device(struct drm_device *ddev) { - return ddev->dev_private; + return container_of(ddev, struct panfrost_device, base); } static inline int panfrost_model_cmp(struct panfrost_device *pfdev, s32 id) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 22350ce8a08f..2b57f6813714 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -36,7 +36,7 @@ static int panfrost_ioctl_query_timestamp(struct panfrost_device *pfdev, { int ret; - ret = pm_runtime_resume_and_get(pfdev->dev); + ret = pm_runtime_resume_and_get(pfdev->base.dev); if (ret) return ret; @@ -44,14 +44,14 @@ static int panfrost_ioctl_query_timestamp(struct panfrost_device *pfdev, *arg = panfrost_timestamp_read(pfdev); panfrost_cycle_counter_put(pfdev); - pm_runtime_put(pfdev->dev); + pm_runtime_put(pfdev->base.dev); return 0; } static int panfrost_ioctl_get_param(struct drm_device *ddev, void *data, struct drm_file *file) { struct drm_panfrost_get_param *param = data; - struct panfrost_device *pfdev = ddev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(ddev); int ret; if (param->pad != 0) @@ -283,7 +283,7 @@ panfrost_copy_in_sync(struct drm_device *dev, static int panfrost_ioctl_submit(struct drm_device *dev, void *data, struct drm_file *file) { - struct panfrost_device *pfdev = dev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(dev); struct panfrost_file_priv *file_priv = file->driver_priv; struct drm_panfrost_submit *args = data; struct drm_syncobj *sync_out = NULL; @@ -457,7 +457,7 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data, { struct panfrost_file_priv *priv = file_priv->driver_priv; struct drm_panfrost_madvise *args = data; - struct panfrost_device *pfdev = dev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(dev); struct drm_gem_object *gem_obj; struct panfrost_gem_object *bo; int ret = 0; @@ -590,7 +590,7 @@ static int panfrost_open(struct drm_device *dev, struct drm_file *file) { int ret; - struct panfrost_device *pfdev = dev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(dev); struct panfrost_file_priv *panfrost_priv; panfrost_priv = kzalloc(sizeof(*panfrost_priv), GFP_KERNEL); @@ -690,8 +690,7 @@ static void panfrost_gpu_show_fdinfo(struct panfrost_device *pfdev, static void panfrost_show_fdinfo(struct drm_printer *p, struct drm_file *file) { - struct drm_device *dev = file->minor->dev; - struct panfrost_device *pfdev = dev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(file->minor->dev); panfrost_gpu_show_fdinfo(pfdev, file->driver_priv, p); @@ -708,8 +707,7 @@ static const struct file_operations panfrost_drm_driver_fops = { static int panthor_gems_show(struct seq_file *m, void *data) { struct drm_info_node *node = m->private; - struct drm_device *dev = node->minor->dev; - struct panfrost_device *pfdev = dev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(node->minor->dev); panfrost_gem_debugfs_print_bos(pfdev, m); @@ -758,7 +756,8 @@ static int show_file_jm_ctxs(struct panfrost_file_priv *pfile, } static struct drm_info_list panthor_debugfs_list[] = { - {"gems", panthor_gems_show, 0, NULL}, + {"gems", + panthor_gems_show, 0, NULL}, }; static int panthor_gems_debugfs_init(struct drm_minor *minor) @@ -865,15 +864,12 @@ static const struct drm_driver panfrost_drm_driver = { static int panfrost_probe(struct platform_device *pdev) { struct panfrost_device *pfdev; - struct drm_device *ddev; int err; - pfdev = devm_kzalloc(&pdev->dev, sizeof(*pfdev), GFP_KERNEL); - if (!pfdev) - return -ENOMEM; - - pfdev->pdev = pdev; - pfdev->dev = &pdev->dev; + pfdev = devm_drm_dev_alloc(&pdev->dev, &panfrost_drm_driver, + struct panfrost_device, base); + if (IS_ERR(pfdev)) + return PTR_ERR(pfdev); platform_set_drvdata(pdev, pfdev); @@ -883,14 +879,6 @@ static int panfrost_probe(struct platform_device *pdev) pfdev->coherent = device_get_dma_attr(&pdev->dev) == DEV_DMA_COHERENT; - /* Allocate and initialize the DRM device. */ - ddev = drm_dev_alloc(&panfrost_drm_driver, &pdev->dev); - if (IS_ERR(ddev)) - return PTR_ERR(ddev); - - ddev->dev_private = pfdev; - pfdev->ddev = ddev; - mutex_init(&pfdev->shrinker_lock); INIT_LIST_HEAD(&pfdev->shrinker_list); @@ -901,51 +889,47 @@ static int panfrost_probe(struct platform_device *pdev) goto err_out0; } - pm_runtime_set_active(pfdev->dev); - pm_runtime_mark_last_busy(pfdev->dev); - pm_runtime_enable(pfdev->dev); - pm_runtime_set_autosuspend_delay(pfdev->dev, 50); /* ~3 frames */ - pm_runtime_use_autosuspend(pfdev->dev); + pm_runtime_set_active(pfdev->base.dev); + pm_runtime_mark_last_busy(pfdev->base.dev); + pm_runtime_enable(pfdev->base.dev); + pm_runtime_set_autosuspend_delay(pfdev->base.dev, 50); /* ~3 frames */ + pm_runtime_use_autosuspend(pfdev->base.dev); /* * Register the DRM device with the core and the connectors with * sysfs */ - err = drm_dev_register(ddev, 0); + err = drm_dev_register(&pfdev->base, 0); if (err < 0) goto err_out1; - err = panfrost_gem_shrinker_init(ddev); + err = panfrost_gem_shrinker_init(&pfdev->base); if (err) goto err_out2; return 0; err_out2: - drm_dev_unregister(ddev); + drm_dev_unregister(&pfdev->base); err_out1: - pm_runtime_disable(pfdev->dev); + pm_runtime_disable(pfdev->base.dev); panfrost_device_fini(pfdev); - pm_runtime_set_suspended(pfdev->dev); + pm_runtime_set_suspended(pfdev->base.dev); err_out0: - drm_dev_put(ddev); return err; } static void panfrost_remove(struct platform_device *pdev) { struct panfrost_device *pfdev = platform_get_drvdata(pdev); - struct drm_device *ddev = pfdev->ddev; - drm_dev_unregister(ddev); - panfrost_gem_shrinker_cleanup(ddev); + drm_dev_unregister(&pfdev->base); + panfrost_gem_shrinker_cleanup(&pfdev->base); - pm_runtime_get_sync(pfdev->dev); - pm_runtime_disable(pfdev->dev); + pm_runtime_get_sync(pfdev->base.dev); + pm_runtime_disable(pfdev->base.dev); panfrost_device_fini(pfdev); - pm_runtime_set_suspended(pfdev->dev); - - drm_dev_put(ddev); + pm_runtime_set_suspended(pfdev->base.dev); } static ssize_t profiling_show(struct device *dev, diff --git a/drivers/gpu/drm/panfrost/panfrost_dump.c b/drivers/gpu/drm/panfrost/panfrost_dump.c index 4042afe2fbf4..3ed6c902d0a1 100644 --- a/drivers/gpu/drm/panfrost/panfrost_dump.c +++ b/drivers/gpu/drm/panfrost/panfrost_dump.c @@ -163,7 +163,7 @@ void panfrost_core_dump(struct panfrost_job *job) iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY); if (!iter.start) { - dev_warn(pfdev->dev, "failed to allocate devcoredump file\n"); + dev_warn(pfdev->base.dev, "failed to allocate devcoredump file\n"); return; } @@ -204,14 +204,14 @@ void panfrost_core_dump(struct panfrost_job *job) mapping = job->mappings[i]; if (!bo->base.sgt) { - dev_err(pfdev->dev, "Panfrost Dump: BO has no sgt, cannot dump\n"); + dev_err(pfdev->base.dev, "Panfrost Dump: BO has no sgt, cannot dump\n"); iter.hdr->bomap.valid = 0; goto dump_header; } ret = drm_gem_vmap(&bo->base.base, &map); if (ret) { - dev_err(pfdev->dev, "Panfrost Dump: couldn't map Buffer Object\n"); + dev_err(pfdev->base.dev, "Panfrost Dump: couldn't map Buffer Object\n"); iter.hdr->bomap.valid = 0; goto dump_header; } @@ -237,5 +237,5 @@ dump_header: panfrost_core_dump_header(&iter, PANFROSTDUMP_BUF_BO, iter.data + } panfrost_core_dump_header(&iter, PANFROSTDUMP_BUF_TRAILER, iter.data); - dev_coredumpv(pfdev->dev, iter.start, iter.data - iter.start, GFP_KERNEL); + dev_coredumpv(pfdev->base.dev, iter.start, iter.data - iter.start, GFP_KERNEL); } diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c index 85d6289a6eda..0528de674a4f 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.c +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c @@ -26,7 +26,7 @@ static void panfrost_gem_debugfs_bo_add(struct panfrost_device *pfdev, static void panfrost_gem_debugfs_bo_rm(struct panfrost_gem_object *bo) { - struct panfrost_device *pfdev = bo->base.base.dev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(bo->base.base.dev); if (list_empty(&bo->debugfs.node)) return; @@ -48,7 +48,7 @@ static void panfrost_gem_debugfs_bo_rm(struct panfrost_gem_object *bo) {} static void panfrost_gem_free_object(struct drm_gem_object *obj) { struct panfrost_gem_object *bo = to_panfrost_bo(obj); - struct panfrost_device *pfdev = obj->dev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(obj->dev); /* * Make sure the BO is no longer inserted in the shrinker list before @@ -76,7 +76,7 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) for (i = 0; i < n_sgt; i++) { if (bo->sgts[i].sgl) { - dma_unmap_sgtable(pfdev->dev, &bo->sgts[i], + dma_unmap_sgtable(pfdev->base.dev, &bo->sgts[i], DMA_BIDIRECTIONAL, 0); sg_free_table(&bo->sgts[i]); } @@ -284,7 +284,7 @@ static const struct drm_gem_object_funcs panfrost_gem_funcs = { */ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t size) { - struct panfrost_device *pfdev = dev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(dev); struct panfrost_gem_object *obj; obj = kzalloc(sizeof(*obj), GFP_KERNEL); diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c index 02b60ea1433a..2fe967a90bcb 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c @@ -97,7 +97,7 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) */ int panfrost_gem_shrinker_init(struct drm_device *dev) { - struct panfrost_device *pfdev = dev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(dev); pfdev->shrinker = shrinker_alloc(0, "drm-panfrost"); if (!pfdev->shrinker) @@ -120,7 +120,7 @@ int panfrost_gem_shrinker_init(struct drm_device *dev) */ void panfrost_gem_shrinker_cleanup(struct drm_device *dev) { - struct panfrost_device *pfdev = dev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(dev); if (pfdev->shrinker) shrinker_free(pfdev->shrinker); diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c index 174e190ba40f..f94337a6c302 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -36,12 +36,12 @@ static irqreturn_t panfrost_gpu_irq_handler(int irq, void *data) u64 address = (u64) gpu_read(pfdev, GPU_FAULT_ADDRESS_HI) << 32; address |= gpu_read(pfdev, GPU_FAULT_ADDRESS_LO); - dev_warn(pfdev->dev, "GPU Fault 0x%08x (%s) at 0x%016llx\n", + dev_warn(pfdev->base.dev, "GPU Fault 0x%08x (%s) at 0x%016llx\n", fault_status, panfrost_exception_name(fault_status & 0xFF), address); if (state & GPU_IRQ_MULTIPLE_FAULT) - dev_warn(pfdev->dev, "There were multiple GPU faults - some have not been reported\n"); + dev_warn(pfdev->base.dev, "There were multiple GPU faults - some have not been reported\n"); gpu_write(pfdev, GPU_INT_MASK, 0); } @@ -72,13 +72,13 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev) val, val & GPU_IRQ_RESET_COMPLETED, 10, 10000); if (ret) { - dev_err(pfdev->dev, "gpu soft reset timed out, attempting hard reset\n"); + dev_err(pfdev->base.dev, "gpu soft reset timed out, attempting hard reset\n"); gpu_write(pfdev, GPU_CMD, GPU_CMD_HARD_RESET); ret = readl_relaxed_poll_timeout(pfdev->iomem + GPU_INT_RAWSTAT, val, val & GPU_IRQ_RESET_COMPLETED, 100, 10000); if (ret) { - dev_err(pfdev->dev, "gpu hard reset timed out\n"); + dev_err(pfdev->base.dev, "gpu hard reset timed out\n"); return ret; } } @@ -95,7 +95,7 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev) * All in-flight jobs should have released their cycle * counter references upon reset, but let us make sure */ - if (drm_WARN_ON(pfdev->ddev, atomic_read(&pfdev->cycle_counter.use_count) != 0)) + if (drm_WARN_ON(&pfdev->base, atomic_read(&pfdev->cycle_counter.use_count) != 0)) atomic_set(&pfdev->cycle_counter.use_count, 0); return 0; @@ -330,13 +330,13 @@ static void panfrost_gpu_init_features(struct panfrost_device *pfdev) bitmap_from_u64(pfdev->features.hw_features, hw_feat); bitmap_from_u64(pfdev->features.hw_issues, hw_issues); - dev_info(pfdev->dev, "mali-%s id 0x%x major 0x%x minor 0x%x status 0x%x", + dev_info(pfdev->base.dev, "mali-%s id 0x%x major 0x%x minor 0x%x status 0x%x", name, gpu_id, major, minor, status); - dev_info(pfdev->dev, "features: %64pb, issues: %64pb", + dev_info(pfdev->base.dev, "features: %64pb, issues: %64pb", pfdev->features.hw_features, pfdev->features.hw_issues); - dev_info(pfdev->dev, "Features: L2:0x%08x Shader:0x%08x Tiler:0x%08x Mem:0x%0x MMU:0x%08x AS:0x%x JS:0x%x", + dev_info(pfdev->base.dev, "Features: L2:0x%08x Shader:0x%08x Tiler:0x%08x Mem:0x%0x MMU:0x%08x AS:0x%x JS:0x%x", pfdev->features.l2_features, pfdev->features.core_features, pfdev->features.tiler_features, @@ -345,7 +345,7 @@ static void panfrost_gpu_init_features(struct panfrost_device *pfdev) pfdev->features.as_present, pfdev->features.js_present); - dev_info(pfdev->dev, "shader_present=0x%0llx l2_present=0x%0llx", + dev_info(pfdev->base.dev, "shader_present=0x%0llx l2_present=0x%0llx", pfdev->features.shader_present, pfdev->features.l2_present); } @@ -411,7 +411,7 @@ static u64 panfrost_get_core_mask(struct panfrost_device *pfdev) */ core_mask = ~(pfdev->features.l2_present - 1) & (pfdev->features.l2_present - 2); - dev_info_once(pfdev->dev, "using only 1st core group (%lu cores from %lu)\n", + dev_info_once(pfdev->base.dev, "using only 1st core group (%lu cores from %lu)\n", hweight64(core_mask), hweight64(pfdev->features.shader_present)); @@ -432,7 +432,7 @@ void panfrost_gpu_power_on(struct panfrost_device *pfdev) val, val == (pfdev->features.l2_present & core_mask), 10, 20000); if (ret) - dev_err(pfdev->dev, "error powering up gpu L2"); + dev_err(pfdev->base.dev, "error powering up gpu L2"); gpu_write(pfdev, SHADER_PWRON_LO, pfdev->features.shader_present & core_mask); @@ -440,13 +440,13 @@ void panfrost_gpu_power_on(struct panfrost_device *pfdev) val, val == (pfdev->features.shader_present & core_mask), 10, 20000); if (ret) - dev_err(pfdev->dev, "error powering up gpu shader"); + dev_err(pfdev->base.dev, "error powering up gpu shader"); gpu_write(pfdev, TILER_PWRON_LO, pfdev->features.tiler_present); ret = readl_relaxed_poll_timeout(pfdev->iomem + TILER_READY_LO, val, val == pfdev->features.tiler_present, 10, 1000); if (ret) - dev_err(pfdev->dev, "error powering up gpu tiler"); + dev_err(pfdev->base.dev, "error powering up gpu tiler"); } void panfrost_gpu_power_off(struct panfrost_device *pfdev) @@ -458,19 +458,19 @@ void panfrost_gpu_power_off(struct panfrost_device *pfdev) ret = readl_relaxed_poll_timeout(pfdev->iomem + SHADER_PWRTRANS_LO, val, !val, 1, 2000); if (ret) - dev_err(pfdev->dev, "shader power transition timeout"); + dev_err(pfdev->base.dev, "shader power transition timeout"); gpu_write(pfdev, TILER_PWROFF_LO, pfdev->features.tiler_present); ret = readl_relaxed_poll_timeout(pfdev->iomem + TILER_PWRTRANS_LO, val, !val, 1, 2000); if (ret) - dev_err(pfdev->dev, "tiler power transition timeout"); + dev_err(pfdev->base.dev, "tiler power transition timeout"); gpu_write(pfdev, L2_PWROFF_LO, pfdev->features.l2_present); ret = readl_poll_timeout(pfdev->iomem + L2_PWRTRANS_LO, val, !val, 0, 2000); if (ret) - dev_err(pfdev->dev, "l2 power transition timeout"); + dev_err(pfdev->base.dev, "l2 power transition timeout"); } void panfrost_gpu_suspend_irq(struct panfrost_device *pfdev) @@ -491,21 +491,22 @@ int panfrost_gpu_init(struct panfrost_device *pfdev) panfrost_gpu_init_features(pfdev); - err = dma_set_mask_and_coherent(pfdev->dev, - DMA_BIT_MASK(FIELD_GET(0xff00, pfdev->features.mmu_features))); + err = dma_set_mask_and_coherent(pfdev->base.dev, + DMA_BIT_MASK(FIELD_GET(0xff00, + pfdev->features.mmu_features))); if (err) return err; - dma_set_max_seg_size(pfdev->dev, UINT_MAX); + dma_set_max_seg_size(pfdev->base.dev, UINT_MAX); - pfdev->gpu_irq = platform_get_irq_byname(to_platform_device(pfdev->dev), "gpu"); + pfdev->gpu_irq = platform_get_irq_byname(to_platform_device(pfdev->base.dev), "gpu"); if (pfdev->gpu_irq < 0) return pfdev->gpu_irq; - err = devm_request_irq(pfdev->dev, pfdev->gpu_irq, panfrost_gpu_irq_handler, + err = devm_request_irq(pfdev->base.dev, pfdev->gpu_irq, panfrost_gpu_irq_handler, IRQF_SHARED, KBUILD_MODNAME "-gpu", pfdev); if (err) { - dev_err(pfdev->dev, "failed to request gpu irq"); + dev_err(pfdev->base.dev, "failed to request gpu irq"); return err; } @@ -525,9 +526,9 @@ u32 panfrost_gpu_get_latest_flush_id(struct panfrost_device *pfdev) if (panfrost_has_hw_feature(pfdev, HW_FEATURE_FLUSH_REDUCTION)) { /* Flush reduction only makes sense when the GPU is kept powered on between jobs */ - if (pm_runtime_get_if_in_use(pfdev->dev)) { + if (pm_runtime_get_if_in_use(pfdev->base.dev)) { flush_id = gpu_read(pfdev, GPU_LATEST_FLUSH_ID); - pm_runtime_put(pfdev->dev); + pm_runtime_put(pfdev->base.dev); return flush_id; } } diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index c47d14eabbae..a0123d0a1b7d 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -95,7 +95,7 @@ static struct dma_fence *panfrost_fence_create(struct panfrost_device *pfdev, in if (!fence) return ERR_PTR(-ENOMEM); - fence->dev = pfdev->ddev; + fence->dev = &pfdev->base; fence->queue = js_num; fence->seqno = ++js->queue[js_num].emit_seqno; dma_fence_init(&fence->base, &panfrost_fence_ops, &js->job_lock, @@ -206,7 +206,7 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) panfrost_devfreq_record_busy(&pfdev->pfdevfreq); - ret = pm_runtime_get_sync(pfdev->dev); + ret = pm_runtime_get_sync(pfdev->base.dev); if (ret < 0) return; @@ -257,7 +257,7 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) } job_write(pfdev, JS_COMMAND_NEXT(js), JS_COMMAND_START); - dev_dbg(pfdev->dev, + dev_dbg(pfdev->base.dev, "JS: Submitting atom %p to js[%d][%d] with head=0x%llx AS %d", job, js, subslot, jc_head, cfg & 0xf); } @@ -442,12 +442,12 @@ static void panfrost_job_handle_err(struct panfrost_device *pfdev, bool signal_fence = true; if (!panfrost_exception_is_fault(js_status)) { - dev_dbg(pfdev->dev, "js event, js=%d, status=%s, head=0x%x, tail=0x%x", + dev_dbg(pfdev->base.dev, "js event, js=%d, status=%s, head=0x%x, tail=0x%x", js, exception_name, job_read(pfdev, JS_HEAD_LO(js)), job_read(pfdev, JS_TAIL_LO(js))); } else { - dev_err(pfdev->dev, "js fault, js=%d, status=%s, head=0x%x, tail=0x%x", + dev_err(pfdev->base.dev, "js fault, js=%d, status=%s, head=0x%x, tail=0x%x", js, exception_name, job_read(pfdev, JS_HEAD_LO(js)), job_read(pfdev, JS_TAIL_LO(js))); @@ -479,7 +479,7 @@ static void panfrost_job_handle_err(struct panfrost_device *pfdev, if (signal_fence) dma_fence_signal_locked(job->done_fence); - pm_runtime_put_autosuspend(pfdev->dev); + pm_runtime_put_autosuspend(pfdev->base.dev); if (panfrost_exception_needs_reset(pfdev, js_status)) { atomic_set(&pfdev->reset.pending, 1); @@ -498,7 +498,7 @@ static void panfrost_job_handle_done(struct panfrost_device *pfdev, panfrost_devfreq_record_idle(&pfdev->pfdevfreq); dma_fence_signal_locked(job->done_fence); - pm_runtime_put_autosuspend(pfdev->dev); + pm_runtime_put_autosuspend(pfdev->base.dev); } static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) @@ -607,7 +607,7 @@ static void panfrost_job_handle_irqs(struct panfrost_device *pfdev) u32 status = job_read(pfdev, JOB_INT_RAWSTAT); while (status) { - pm_runtime_mark_last_busy(pfdev->dev); + pm_runtime_mark_last_busy(pfdev->base.dev); spin_lock(&pfdev->js->job_lock); panfrost_job_handle_irq(pfdev, status); @@ -688,7 +688,7 @@ panfrost_reset(struct panfrost_device *pfdev, 10, 10000); if (ret) - dev_err(pfdev->dev, "Soft-stop failed\n"); + dev_err(pfdev->base.dev, "Soft-stop failed\n"); /* Handle the remaining interrupts before we reset. */ panfrost_job_handle_irqs(pfdev); @@ -706,7 +706,7 @@ panfrost_reset(struct panfrost_device *pfdev, if (pfdev->jobs[i][j]->requirements & PANFROST_JD_REQ_CYCLE_COUNT || pfdev->jobs[i][j]->is_profiled) panfrost_cycle_counter_put(pfdev->jobs[i][j]->pfdev); - pm_runtime_put_noidle(pfdev->dev); + pm_runtime_put_noidle(pfdev->base.dev); panfrost_devfreq_record_idle(&pfdev->pfdevfreq); } } @@ -774,11 +774,11 @@ static enum drm_gpu_sched_stat panfrost_job_timedout(struct drm_sched_job synchronize_irq(pfdev->js->irq); if (dma_fence_is_signaled(job->done_fence)) { - dev_warn(pfdev->dev, "unexpectedly high interrupt latency\n"); + dev_warn(pfdev->base.dev, "unexpectedly high interrupt latency\n"); return DRM_GPU_SCHED_STAT_NO_HANG; } - dev_err(pfdev->dev, "gpu sched timeout, js=%d, config=0x%x, status=0x%x, head=0x%x, tail=0x%x, sched_job=%p", + dev_err(pfdev->base.dev, "gpu sched timeout, js=%d, config=0x%x, status=0x%x, head=0x%x, tail=0x%x, sched_job=%p", js, job_read(pfdev, JS_CONFIG(js)), job_read(pfdev, JS_STATUS(js)), @@ -847,7 +847,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) .credit_limit = 2, .timeout = msecs_to_jiffies(JOB_TIMEOUT_MS), .name = "pan_js", - .dev = pfdev->dev, + .dev = pfdev->base.dev, }; struct panfrost_job_slot *js; int ret, j; @@ -859,24 +859,25 @@ int panfrost_job_init(struct panfrost_device *pfdev) if (!panfrost_has_hw_feature(pfdev, HW_FEATURE_JOBCHAIN_DISAMBIGUATION)) args.credit_limit = 1; - pfdev->js = js = devm_kzalloc(pfdev->dev, sizeof(*js), GFP_KERNEL); + js = devm_kzalloc(pfdev->base.dev, sizeof(*js), GFP_KERNEL); if (!js) return -ENOMEM; + pfdev->js = js; INIT_WORK(&pfdev->reset.work, panfrost_reset_work); spin_lock_init(&js->job_lock); - js->irq = platform_get_irq_byname(to_platform_device(pfdev->dev), "job"); + js->irq = platform_get_irq_byname(to_platform_device(pfdev->base.dev), "job"); if (js->irq < 0) return js->irq; - ret = devm_request_threaded_irq(pfdev->dev, js->irq, + ret = devm_request_threaded_irq(pfdev->base.dev, js->irq, panfrost_job_irq_handler, panfrost_job_irq_handler_thread, IRQF_SHARED, KBUILD_MODNAME "-job", pfdev); if (ret) { - dev_err(pfdev->dev, "failed to request job irq"); + dev_err(pfdev->base.dev, "failed to request job irq"); return ret; } @@ -890,7 +891,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) ret = drm_sched_init(&js->queue[j].sched, &args); if (ret) { - dev_err(pfdev->dev, "Failed to create scheduler: %d.", ret); + dev_err(pfdev->base.dev, "Failed to create scheduler: %d.", ret); goto err_sched; } } diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index f6b91c052cfb..1d696eeea2fa 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -81,7 +81,7 @@ static int wait_ready(struct panfrost_device *pfdev, u32 as_nr) if (ret) { /* The GPU hung, let's trigger a reset */ panfrost_device_schedule_reset(pfdev); - dev_err(pfdev->dev, "AS_ACTIVE bit stuck\n"); + dev_err(pfdev->base.dev, "AS_ACTIVE bit stuck\n"); } return ret; @@ -222,7 +222,7 @@ static int mmu_cfg_init_aarch64_4k(struct panfrost_mmu *mmu) struct io_pgtable_cfg *pgtbl_cfg = &mmu->pgtbl_cfg; struct panfrost_device *pfdev = mmu->pfdev; - if (drm_WARN_ON(pfdev->ddev, pgtbl_cfg->arm_lpae_s1_cfg.ttbr & + if (drm_WARN_ON(&pfdev->base, pgtbl_cfg->arm_lpae_s1_cfg.ttbr & ~AS_TRANSTAB_AARCH64_4K_ADDR_MASK)) return -EINVAL; @@ -253,7 +253,7 @@ static int panfrost_mmu_cfg_init(struct panfrost_mmu *mmu, return mmu_cfg_init_mali_lpae(mmu); default: /* This should never happen */ - drm_WARN(pfdev->ddev, 1, "Invalid pgtable format"); + drm_WARN(&pfdev->base, 1, "Invalid pgtable format"); return -EINVAL; } } @@ -315,7 +315,9 @@ u32 panfrost_mmu_as_get(struct panfrost_device *pfdev, struct panfrost_mmu *mmu) atomic_set(&mmu->as_count, 1); list_add(&mmu->list, &pfdev->as_lru_list); - dev_dbg(pfdev->dev, "Assigned AS%d to mmu %p, alloc_mask=%lx", as, mmu, pfdev->as_alloc_mask); + dev_dbg(pfdev->base.dev, + "Assigned AS%d to mmu %p, alloc_mask=%lx", + as, mmu, pfdev->as_alloc_mask); panfrost_mmu_enable(pfdev, mmu); @@ -381,13 +383,13 @@ static void panfrost_mmu_flush_range(struct panfrost_device *pfdev, if (mmu->as < 0) return; - pm_runtime_get_noresume(pfdev->dev); + pm_runtime_get_noresume(pfdev->base.dev); /* Flush the PTs only if we're already awake */ - if (pm_runtime_active(pfdev->dev)) + if (pm_runtime_active(pfdev->base.dev)) mmu_hw_do_operation(pfdev, mmu, iova, size, AS_COMMAND_FLUSH_PT); - pm_runtime_put_autosuspend(pfdev->dev); + pm_runtime_put_autosuspend(pfdev->base.dev); } static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, @@ -402,7 +404,9 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, unsigned long paddr = sg_dma_address(sgl); size_t len = sg_dma_len(sgl); - dev_dbg(pfdev->dev, "map: as=%d, iova=%llx, paddr=%lx, len=%zx", mmu->as, iova, paddr, len); + dev_dbg(pfdev->base.dev, + "map: as=%d, iova=%llx, paddr=%lx, len=%zx", + mmu->as, iova, paddr, len); while (len) { size_t pgcount, mapped = 0; @@ -462,7 +466,7 @@ void panfrost_mmu_unmap(struct panfrost_gem_mapping *mapping) if (WARN_ON(!mapping->active)) return; - dev_dbg(pfdev->dev, "unmap: as=%d, iova=%llx, len=%zx", + dev_dbg(pfdev->base.dev, "unmap: as=%d, iova=%llx, len=%zx", mapping->mmu->as, iova, len); while (unmapped_len < len) { @@ -559,7 +563,7 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, bo = bomapping->obj; if (!bo->is_heap) { - dev_WARN(pfdev->dev, "matching BO is not heap type (GPU VA = %llx)", + dev_WARN(pfdev->base.dev, "matching BO is not heap type (GPU VA = %llx)", bomapping->mmnode.start << PAGE_SHIFT); ret = -EINVAL; goto err_bo; @@ -626,7 +630,7 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, if (ret) goto err_unlock; - ret = dma_map_sgtable(pfdev->dev, sgt, DMA_BIDIRECTIONAL, 0); + ret = dma_map_sgtable(pfdev->base.dev, sgt, DMA_BIDIRECTIONAL, 0); if (ret) goto err_map; @@ -636,7 +640,7 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, bomapping->active = true; bo->heap_rss_size += SZ_2M; - dev_dbg(pfdev->dev, "mapped page fault @ AS%d %llx", as, addr); + dev_dbg(pfdev->base.dev, "mapped page fault @ AS%d %llx", as, addr); out: dma_resv_unlock(obj->resv); @@ -662,10 +666,10 @@ static void panfrost_mmu_release_ctx(struct kref *kref) spin_lock(&pfdev->as_lock); if (mmu->as >= 0) { - pm_runtime_get_noresume(pfdev->dev); - if (pm_runtime_active(pfdev->dev)) + pm_runtime_get_noresume(pfdev->base.dev); + if (pm_runtime_active(pfdev->base.dev)) panfrost_mmu_disable(pfdev, mmu->as); - pm_runtime_put_autosuspend(pfdev->dev); + pm_runtime_put_autosuspend(pfdev->base.dev); clear_bit(mmu->as, &pfdev->as_alloc_mask); clear_bit(mmu->as, &pfdev->as_in_use_mask); @@ -726,7 +730,7 @@ struct panfrost_mmu *panfrost_mmu_ctx_create(struct panfrost_device *pfdev) if (pfdev->comp->gpu_quirks & BIT(GPU_QUIRK_FORCE_AARCH64_PGTABLE)) { if (!panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) { - dev_err_once(pfdev->dev, + dev_err_once(pfdev->base.dev, "AARCH64_4K page table not supported\n"); return ERR_PTR(-EINVAL); } @@ -755,7 +759,7 @@ struct panfrost_mmu *panfrost_mmu_ctx_create(struct panfrost_device *pfdev) .oas = pa_bits, .coherent_walk = pfdev->coherent, .tlb = &mmu_tlb_ops, - .iommu_dev = pfdev->dev, + .iommu_dev = pfdev->base.dev, }; mmu->pgtbl_ops = alloc_io_pgtable_ops(fmt, &mmu->pgtbl_cfg, mmu); @@ -848,7 +852,7 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int irq, void *data) if (ret) { /* terminal fault, print info about the fault */ - dev_err(pfdev->dev, + dev_err(pfdev->base.dev, "Unhandled Page fault in AS%d at VA 0x%016llX\n" "Reason: %s\n" "raw fault status: 0x%X\n" @@ -896,18 +900,18 @@ int panfrost_mmu_init(struct panfrost_device *pfdev) { int err; - pfdev->mmu_irq = platform_get_irq_byname(to_platform_device(pfdev->dev), "mmu"); + pfdev->mmu_irq = platform_get_irq_byname(to_platform_device(pfdev->base.dev), "mmu"); if (pfdev->mmu_irq < 0) return pfdev->mmu_irq; - err = devm_request_threaded_irq(pfdev->dev, pfdev->mmu_irq, + err = devm_request_threaded_irq(pfdev->base.dev, pfdev->mmu_irq, panfrost_mmu_irq_handler, panfrost_mmu_irq_handler_thread, IRQF_SHARED, KBUILD_MODNAME "-mmu", pfdev); if (err) { - dev_err(pfdev->dev, "failed to request mmu irq"); + dev_err(pfdev->base.dev, "failed to request mmu irq"); return err; } diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c index 0dd62e8b2fa7..718eb44b40f8 100644 --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c @@ -84,11 +84,11 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev, else if (perfcnt->user) return -EBUSY; - ret = pm_runtime_get_sync(pfdev->dev); + ret = pm_runtime_get_sync(pfdev->base.dev); if (ret < 0) goto err_put_pm; - bo = drm_gem_shmem_create(pfdev->ddev, perfcnt->bosize); + bo = drm_gem_shmem_create(&pfdev->base, perfcnt->bosize); if (IS_ERR(bo)) { ret = PTR_ERR(bo); goto err_put_pm; @@ -175,7 +175,7 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev, err_put_bo: drm_gem_object_put(&bo->base); err_put_pm: - pm_runtime_put(pfdev->dev); + pm_runtime_put(pfdev->base.dev); return ret; } @@ -203,7 +203,7 @@ static int panfrost_perfcnt_disable_locked(struct panfrost_device *pfdev, panfrost_mmu_as_put(pfdev, perfcnt->mapping->mmu); panfrost_gem_mapping_put(perfcnt->mapping); perfcnt->mapping = NULL; - pm_runtime_put_autosuspend(pfdev->dev); + pm_runtime_put_autosuspend(pfdev->base.dev); return 0; } @@ -211,7 +211,7 @@ static int panfrost_perfcnt_disable_locked(struct panfrost_device *pfdev, int panfrost_ioctl_perfcnt_enable(struct drm_device *dev, void *data, struct drm_file *file_priv) { - struct panfrost_device *pfdev = dev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(dev); struct panfrost_perfcnt *perfcnt = pfdev->perfcnt; struct drm_panfrost_perfcnt_enable *req = data; int ret; @@ -238,7 +238,7 @@ int panfrost_ioctl_perfcnt_enable(struct drm_device *dev, void *data, int panfrost_ioctl_perfcnt_dump(struct drm_device *dev, void *data, struct drm_file *file_priv) { - struct panfrost_device *pfdev = dev->dev_private; + struct panfrost_device *pfdev = to_panfrost_device(dev); struct panfrost_perfcnt *perfcnt = pfdev->perfcnt; struct drm_panfrost_perfcnt_dump *req = data; void __user *user_ptr = (void __user *)(uintptr_t)req->buf_ptr; @@ -273,12 +273,12 @@ void panfrost_perfcnt_close(struct drm_file *file_priv) struct panfrost_device *pfdev = pfile->pfdev; struct panfrost_perfcnt *perfcnt = pfdev->perfcnt; - pm_runtime_get_sync(pfdev->dev); + pm_runtime_get_sync(pfdev->base.dev); mutex_lock(&perfcnt->lock); if (perfcnt->user == pfile) panfrost_perfcnt_disable_locked(pfdev, file_priv); mutex_unlock(&perfcnt->lock); - pm_runtime_put_autosuspend(pfdev->dev); + pm_runtime_put_autosuspend(pfdev->base.dev); } int panfrost_perfcnt_init(struct panfrost_device *pfdev) @@ -316,7 +316,7 @@ int panfrost_perfcnt_init(struct panfrost_device *pfdev) COUNTERS_PER_BLOCK * BYTES_PER_COUNTER; } - perfcnt = devm_kzalloc(pfdev->dev, sizeof(*perfcnt), GFP_KERNEL); + perfcnt = devm_kzalloc(pfdev->base.dev, sizeof(*perfcnt), GFP_KERNEL); if (!perfcnt) return -ENOMEM; -- 2.51.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v4 02/10] drm/panfrost: Handle inexistent GPU during probe 2025-10-01 2:20 [PATCH v4 00/10] Some Panfrost fixes and improvements Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 01/10] drm/panfrost: Replace DRM driver allocation method with newer one Adrián Larumbe @ 2025-10-01 2:20 ` Adrián Larumbe 2025-10-01 10:51 ` Boris Brezillon 2025-10-01 2:20 ` [PATCH v4 03/10] drm/panfrost: Handle job HW submit errors Adrián Larumbe ` (7 subsequent siblings) 9 siblings, 1 reply; 20+ messages in thread From: Adrián Larumbe @ 2025-10-01 2:20 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Steven Price, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter Just in case we're dealing with a yet not recognised device. Reviewed-by: Steven Price <steven.price@arm.com> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_gpu.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c index f94337a6c302..8d049a07d393 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -240,9 +240,10 @@ static const struct panfrost_model gpu_models[] = { /* MediaTek MT8188 Mali-G57 MC3 */ GPU_MODEL(g57, 0x9093, GPU_REV(g57, 0, 0)), + {0}, }; -static void panfrost_gpu_init_features(struct panfrost_device *pfdev) +static int panfrost_gpu_init_features(struct panfrost_device *pfdev) { u32 gpu_id, num_js, major, minor, status, rev; const char *name = "unknown"; @@ -327,6 +328,12 @@ static void panfrost_gpu_init_features(struct panfrost_device *pfdev) break; } + if (!model->name) { + dev_err(pfdev->base.dev, "GPU model not found: mali-%s id rev %#x %#x\n", + name, gpu_id, rev); + return -ENODEV; + } + bitmap_from_u64(pfdev->features.hw_features, hw_feat); bitmap_from_u64(pfdev->features.hw_issues, hw_issues); @@ -347,6 +354,8 @@ static void panfrost_gpu_init_features(struct panfrost_device *pfdev) dev_info(pfdev->base.dev, "shader_present=0x%0llx l2_present=0x%0llx", pfdev->features.shader_present, pfdev->features.l2_present); + + return 0; } void panfrost_cycle_counter_get(struct panfrost_device *pfdev) @@ -489,7 +498,9 @@ int panfrost_gpu_init(struct panfrost_device *pfdev) if (err) return err; - panfrost_gpu_init_features(pfdev); + err = panfrost_gpu_init_features(pfdev); + if (err) + return err; err = dma_set_mask_and_coherent(pfdev->base.dev, DMA_BIT_MASK(FIELD_GET(0xff00, -- 2.51.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v4 02/10] drm/panfrost: Handle inexistent GPU during probe 2025-10-01 2:20 ` [PATCH v4 02/10] drm/panfrost: Handle inexistent GPU during probe Adrián Larumbe @ 2025-10-01 10:51 ` Boris Brezillon 0 siblings, 0 replies; 20+ messages in thread From: Boris Brezillon @ 2025-10-01 10:51 UTC (permalink / raw) To: Adrián Larumbe Cc: linux-kernel, dri-devel, Steven Price, kernel, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On Wed, 1 Oct 2025 03:20:23 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > Just in case we're dealing with a yet not recognised device. > > Reviewed-by: Steven Price <steven.price@arm.com> > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_gpu.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c > index f94337a6c302..8d049a07d393 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c > @@ -240,9 +240,10 @@ static const struct panfrost_model gpu_models[] = { > /* MediaTek MT8188 Mali-G57 MC3 */ > GPU_MODEL(g57, 0x9093, > GPU_REV(g57, 0, 0)), > + {0}, > }; > > -static void panfrost_gpu_init_features(struct panfrost_device *pfdev) > +static int panfrost_gpu_init_features(struct panfrost_device *pfdev) > { > u32 gpu_id, num_js, major, minor, status, rev; > const char *name = "unknown"; > @@ -327,6 +328,12 @@ static void panfrost_gpu_init_features(struct panfrost_device *pfdev) > break; > } > > + if (!model->name) { > + dev_err(pfdev->base.dev, "GPU model not found: mali-%s id rev %#x %#x\n", > + name, gpu_id, rev); > + return -ENODEV; > + } > + > bitmap_from_u64(pfdev->features.hw_features, hw_feat); > bitmap_from_u64(pfdev->features.hw_issues, hw_issues); > > @@ -347,6 +354,8 @@ static void panfrost_gpu_init_features(struct panfrost_device *pfdev) > > dev_info(pfdev->base.dev, "shader_present=0x%0llx l2_present=0x%0llx", > pfdev->features.shader_present, pfdev->features.l2_present); > + > + return 0; > } > > void panfrost_cycle_counter_get(struct panfrost_device *pfdev) > @@ -489,7 +498,9 @@ int panfrost_gpu_init(struct panfrost_device *pfdev) > if (err) > return err; > > - panfrost_gpu_init_features(pfdev); > + err = panfrost_gpu_init_features(pfdev); > + if (err) > + return err; > > err = dma_set_mask_and_coherent(pfdev->base.dev, > DMA_BIT_MASK(FIELD_GET(0xff00, ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 03/10] drm/panfrost: Handle job HW submit errors 2025-10-01 2:20 [PATCH v4 00/10] Some Panfrost fixes and improvements Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 01/10] drm/panfrost: Replace DRM driver allocation method with newer one Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 02/10] drm/panfrost: Handle inexistent GPU during probe Adrián Larumbe @ 2025-10-01 2:20 ` Adrián Larumbe 2025-10-06 16:07 ` Steven Price 2025-10-01 2:20 ` [PATCH v4 04/10] drm/panfrost: Handle error when allocating AS number Adrián Larumbe ` (6 subsequent siblings) 9 siblings, 1 reply; 20+ messages in thread From: Adrián Larumbe @ 2025-10-01 2:20 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Steven Price, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter Avoid waiting for the DRM scheduler job timedout handler, and instead, let the DRM scheduler core signal the error fence immediately when HW job submission fails. That means we must also decrement the runtime-PM refcnt for the device, because the job will never be enqueued or inflight. Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_job.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index a0123d0a1b7d..3f60adc9b69d 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -196,7 +196,7 @@ panfrost_enqueue_job(struct panfrost_device *pfdev, int slot, return 1; } -static void panfrost_job_hw_submit(struct panfrost_job *job, int js) +static int panfrost_job_hw_submit(struct panfrost_job *job, int js) { struct panfrost_device *pfdev = job->pfdev; unsigned int subslot; @@ -208,10 +208,11 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) ret = pm_runtime_get_sync(pfdev->base.dev); if (ret < 0) - return; + goto err_hwsubmit; if (WARN_ON(job_read(pfdev, JS_COMMAND_NEXT(js)))) { - return; + ret = -EINVAL; + goto err_hwsubmit; } cfg = panfrost_mmu_as_get(pfdev, job->mmu); @@ -262,6 +263,12 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) job, js, subslot, jc_head, cfg & 0xf); } spin_unlock(&pfdev->js->job_lock); + + return 0; + +err_hwsubmit: + pm_runtime_put_autosuspend(pfdev->base.dev); + return ret; } static int panfrost_acquire_object_fences(struct drm_gem_object **bos, @@ -384,6 +391,7 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) struct panfrost_device *pfdev = job->pfdev; int slot = panfrost_job_get_slot(job); struct dma_fence *fence = NULL; + int ret; if (job->ctx->destroyed) return ERR_PTR(-ECANCELED); @@ -405,7 +413,11 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) dma_fence_put(job->done_fence); job->done_fence = dma_fence_get(fence); - panfrost_job_hw_submit(job, slot); + ret = panfrost_job_hw_submit(job, slot); + if (ret) { + dma_fence_put(fence); + return ERR_PTR(ret); + } return fence; } -- 2.51.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v4 03/10] drm/panfrost: Handle job HW submit errors 2025-10-01 2:20 ` [PATCH v4 03/10] drm/panfrost: Handle job HW submit errors Adrián Larumbe @ 2025-10-06 16:07 ` Steven Price 2025-10-07 0:34 ` Adrián Larumbe 0 siblings, 1 reply; 20+ messages in thread From: Steven Price @ 2025-10-06 16:07 UTC (permalink / raw) To: Adrián Larumbe, linux-kernel Cc: dri-devel, Boris Brezillon, kernel, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On 01/10/2025 03:20, Adrián Larumbe wrote: > Avoid waiting for the DRM scheduler job timedout handler, and instead, let > the DRM scheduler core signal the error fence immediately when HW job > submission fails. > > That means we must also decrement the runtime-PM refcnt for the device, > because the job will never be enqueued or inflight. > > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_job.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c > index a0123d0a1b7d..3f60adc9b69d 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > @@ -196,7 +196,7 @@ panfrost_enqueue_job(struct panfrost_device *pfdev, int slot, > return 1; > } > > -static void panfrost_job_hw_submit(struct panfrost_job *job, int js) > +static int panfrost_job_hw_submit(struct panfrost_job *job, int js) > { > struct panfrost_device *pfdev = job->pfdev; > unsigned int subslot; > @@ -208,10 +208,11 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) > > ret = pm_runtime_get_sync(pfdev->base.dev); > if (ret < 0) > - return; > + goto err_hwsubmit; > > if (WARN_ON(job_read(pfdev, JS_COMMAND_NEXT(js)))) { > - return; > + ret = -EINVAL; > + goto err_hwsubmit; > } > > cfg = panfrost_mmu_as_get(pfdev, job->mmu); > @@ -262,6 +263,12 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) > job, js, subslot, jc_head, cfg & 0xf); > } > spin_unlock(&pfdev->js->job_lock); > + > + return 0; > + > +err_hwsubmit: > + pm_runtime_put_autosuspend(pfdev->base.dev); I think you're missing something here. You've put a call to pm_runtime_put_autosuspend() here which matches the call to pm_runtime_get_sync() that we do earlier in the function. But there's no corresponding panfrost_devfreq_record_idle() (but the first thing this function does is panfrost_devfreq_record_busy()). So unless I'm missing something (very possible) then this is going to mess up the devfreq accounting. A simple fix would be just to move the panfrost_devfreq_record_busy() call down in the function. Thanks, Steve > + return ret; > } > > static int panfrost_acquire_object_fences(struct drm_gem_object **bos, > @@ -384,6 +391,7 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) > struct panfrost_device *pfdev = job->pfdev; > int slot = panfrost_job_get_slot(job); > struct dma_fence *fence = NULL; > + int ret; > > if (job->ctx->destroyed) > return ERR_PTR(-ECANCELED); > @@ -405,7 +413,11 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) > dma_fence_put(job->done_fence); > job->done_fence = dma_fence_get(fence); > > - panfrost_job_hw_submit(job, slot); > + ret = panfrost_job_hw_submit(job, slot); > + if (ret) { > + dma_fence_put(fence); > + return ERR_PTR(ret); > + } > > return fence; > } ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 03/10] drm/panfrost: Handle job HW submit errors 2025-10-06 16:07 ` Steven Price @ 2025-10-07 0:34 ` Adrián Larumbe 0 siblings, 0 replies; 20+ messages in thread From: Adrián Larumbe @ 2025-10-07 0:34 UTC (permalink / raw) To: Steven Price Cc: linux-kernel, dri-devel, Boris Brezillon, kernel, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On 06.10.2025 17:07, Steven Price wrote: > On 01/10/2025 03:20, Adrián Larumbe wrote: > > Avoid waiting for the DRM scheduler job timedout handler, and instead, let > > the DRM scheduler core signal the error fence immediately when HW job > > submission fails. > > > > That means we must also decrement the runtime-PM refcnt for the device, > > because the job will never be enqueued or inflight. > > > > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > > --- > > drivers/gpu/drm/panfrost/panfrost_job.c | 20 ++++++++++++++++---- > > 1 file changed, 16 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c > > index a0123d0a1b7d..3f60adc9b69d 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > > @@ -196,7 +196,7 @@ panfrost_enqueue_job(struct panfrost_device *pfdev, int slot, > > return 1; > > } > > > > -static void panfrost_job_hw_submit(struct panfrost_job *job, int js) > > +static int panfrost_job_hw_submit(struct panfrost_job *job, int js) > > { > > struct panfrost_device *pfdev = job->pfdev; > > unsigned int subslot; > > @@ -208,10 +208,11 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) > > > > ret = pm_runtime_get_sync(pfdev->base.dev); > > if (ret < 0) > > - return; > > + goto err_hwsubmit; > > > > if (WARN_ON(job_read(pfdev, JS_COMMAND_NEXT(js)))) { > > - return; > > + ret = -EINVAL; > > + goto err_hwsubmit; > > } > > > > cfg = panfrost_mmu_as_get(pfdev, job->mmu); > > @@ -262,6 +263,12 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) > > job, js, subslot, jc_head, cfg & 0xf); > > } > > spin_unlock(&pfdev->js->job_lock); > > + > > + return 0; > > + > > +err_hwsubmit: > > + pm_runtime_put_autosuspend(pfdev->base.dev); > > I think you're missing something here. You've put a call to > pm_runtime_put_autosuspend() here which matches the call to > pm_runtime_get_sync() that we do earlier in the function. But there's no > corresponding panfrost_devfreq_record_idle() (but the first thing this > function does is panfrost_devfreq_record_busy()). > > So unless I'm missing something (very possible) then this is going to > mess up the devfreq accounting. A simple fix would be just to move the > panfrost_devfreq_record_busy() call down in the function. You didn't miss anything, I completely forgot to keep the devfreq busy count balanced after this change. I've moved panfrost_devfreq_record_busy() right after the point the function can no longer result in an error, as you suggested. > Thanks, > Steve > > > + return ret; > > } > > > > static int panfrost_acquire_object_fences(struct drm_gem_object **bos, > > @@ -384,6 +391,7 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) > > struct panfrost_device *pfdev = job->pfdev; > > int slot = panfrost_job_get_slot(job); > > struct dma_fence *fence = NULL; > > + int ret; > > > > if (job->ctx->destroyed) > > return ERR_PTR(-ECANCELED); > > @@ -405,7 +413,11 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) > > dma_fence_put(job->done_fence); > > job->done_fence = dma_fence_get(fence); > > > > - panfrost_job_hw_submit(job, slot); > > + ret = panfrost_job_hw_submit(job, slot); > > + if (ret) { > > + dma_fence_put(fence); > > + return ERR_PTR(ret); > > + } > > > > return fence; > > } Adrian Larumbe ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 04/10] drm/panfrost: Handle error when allocating AS number 2025-10-01 2:20 [PATCH v4 00/10] Some Panfrost fixes and improvements Adrián Larumbe ` (2 preceding siblings ...) 2025-10-01 2:20 ` [PATCH v4 03/10] drm/panfrost: Handle job HW submit errors Adrián Larumbe @ 2025-10-01 2:20 ` Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 05/10] drm/panfrost: Handle page mapping failure Adrián Larumbe ` (5 subsequent siblings) 9 siblings, 0 replies; 20+ messages in thread From: Adrián Larumbe @ 2025-10-01 2:20 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Steven Price, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter If we reach the beginning of the LRU AS list, then return an error. Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_job.c | 6 +++++- drivers/gpu/drm/panfrost/panfrost_mmu.c | 5 +++-- drivers/gpu/drm/panfrost/panfrost_mmu.h | 2 +- drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 9 ++++++++- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 3f60adc9b69d..ba934437a8ea 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -215,7 +215,11 @@ static int panfrost_job_hw_submit(struct panfrost_job *job, int js) goto err_hwsubmit; } - cfg = panfrost_mmu_as_get(pfdev, job->mmu); + ret = panfrost_mmu_as_get(pfdev, job->mmu); + if (ret < 0) + goto err_hwsubmit; + + cfg = ret; job_write(pfdev, JS_HEAD_NEXT_LO(js), lower_32_bits(jc_head)); job_write(pfdev, JS_HEAD_NEXT_HI(js), upper_32_bits(jc_head)); diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index 1d696eeea2fa..cf272b167feb 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -258,7 +258,7 @@ static int panfrost_mmu_cfg_init(struct panfrost_mmu *mmu, } } -u32 panfrost_mmu_as_get(struct panfrost_device *pfdev, struct panfrost_mmu *mmu) +int panfrost_mmu_as_get(struct panfrost_device *pfdev, struct panfrost_mmu *mmu) { int as; @@ -300,7 +300,8 @@ u32 panfrost_mmu_as_get(struct panfrost_device *pfdev, struct panfrost_mmu *mmu) if (!atomic_read(&lru_mmu->as_count)) break; } - WARN_ON(&lru_mmu->list == &pfdev->as_lru_list); + if (WARN_ON(&lru_mmu->list == &pfdev->as_lru_list)) + return -EBUSY; list_del_init(&lru_mmu->list); as = lru_mmu->as; diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.h b/drivers/gpu/drm/panfrost/panfrost_mmu.h index 022a9a74a114..e6e6966a0cca 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.h +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.h @@ -16,7 +16,7 @@ void panfrost_mmu_fini(struct panfrost_device *pfdev); void panfrost_mmu_reset(struct panfrost_device *pfdev); void panfrost_mmu_suspend_irq(struct panfrost_device *pfdev); -u32 panfrost_mmu_as_get(struct panfrost_device *pfdev, struct panfrost_mmu *mmu); +int panfrost_mmu_as_get(struct panfrost_device *pfdev, struct panfrost_mmu *mmu); void panfrost_mmu_as_put(struct panfrost_device *pfdev, struct panfrost_mmu *mmu); struct panfrost_mmu *panfrost_mmu_ctx_get(struct panfrost_mmu *mmu); diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c index 718eb44b40f8..a8d821e1bde2 100644 --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c @@ -132,7 +132,12 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev, perfcnt->user = user; - as = panfrost_mmu_as_get(pfdev, perfcnt->mapping->mmu); + ret = panfrost_mmu_as_get(pfdev, perfcnt->mapping->mmu); + if (ret < 0) + goto err_unsetuser; + + as = ret; + cfg = GPU_PERFCNT_CFG_AS(as) | GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_MANUAL); @@ -166,6 +171,8 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev, return 0; +err_unsetuser: + perfcnt->user = NULL; err_vunmap: drm_gem_vunmap(&bo->base, &map); err_put_mapping: -- 2.51.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v4 05/10] drm/panfrost: Handle page mapping failure 2025-10-01 2:20 [PATCH v4 00/10] Some Panfrost fixes and improvements Adrián Larumbe ` (3 preceding siblings ...) 2025-10-01 2:20 ` [PATCH v4 04/10] drm/panfrost: Handle error when allocating AS number Adrián Larumbe @ 2025-10-01 2:20 ` Adrián Larumbe 2025-10-01 10:58 ` Boris Brezillon 2025-10-01 2:20 ` [PATCH v4 06/10] drm/panfrost: Don't rework job IRQ enable mask in the enable path Adrián Larumbe ` (4 subsequent siblings) 9 siblings, 1 reply; 20+ messages in thread From: Adrián Larumbe @ 2025-10-01 2:20 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Steven Price, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter When mapping the pages of a BO, either a heap type at page fault time or else a non-heap BO at object creation time, if the ARM page table mapping function fails, we unmap what had been mapped so far and bail out. Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_mmu.c | 49 ++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index cf272b167feb..fb17c32855a5 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -393,13 +393,32 @@ static void panfrost_mmu_flush_range(struct panfrost_device *pfdev, pm_runtime_put_autosuspend(pfdev->base.dev); } +static void mmu_unmap_range(struct panfrost_mmu *mmu, u64 iova, size_t len) +{ + struct io_pgtable_ops *ops = mmu->pgtbl_ops; + size_t pgsize, unmapped_len = 0; + size_t unmapped_page, pgcount; + + while (unmapped_len < len) { + pgsize = get_pgsize(iova, len - unmapped_len, &pgcount); + + unmapped_page = ops->unmap_pages(ops, iova, pgsize, pgcount, NULL); + WARN_ON(unmapped_page != pgsize * pgcount); + + iova += pgsize * pgcount; + unmapped_len += pgsize * pgcount; + } +} + static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, u64 iova, int prot, struct sg_table *sgt) { unsigned int count; struct scatterlist *sgl; struct io_pgtable_ops *ops = mmu->pgtbl_ops; + size_t total_mapped = 0; u64 start_iova = iova; + int ret; for_each_sgtable_dma_sg(sgt, sgl, count) { unsigned long paddr = sg_dma_address(sgl); @@ -413,10 +432,14 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, size_t pgcount, mapped = 0; size_t pgsize = get_pgsize(iova | paddr, len, &pgcount); - ops->map_pages(ops, iova, paddr, pgsize, pgcount, prot, + ret = ops->map_pages(ops, iova, paddr, pgsize, pgcount, prot, GFP_KERNEL, &mapped); + if (ret) + goto err_unmap_pages; + /* Don't get stuck if things have gone wrong */ mapped = max(mapped, pgsize); + total_mapped += mapped; iova += mapped; paddr += mapped; len -= mapped; @@ -426,6 +449,10 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, panfrost_mmu_flush_range(pfdev, mmu, start_iova, iova - start_iova); return 0; + +err_unmap_pages: + mmu_unmap_range(mmu, start_iova, total_mapped); + return ret; } int panfrost_mmu_map(struct panfrost_gem_mapping *mapping) @@ -436,6 +463,7 @@ int panfrost_mmu_map(struct panfrost_gem_mapping *mapping) struct panfrost_device *pfdev = to_panfrost_device(obj->dev); struct sg_table *sgt; int prot = IOMMU_READ | IOMMU_WRITE | IOMMU_CACHE; + int ret; if (WARN_ON(mapping->active)) return 0; @@ -447,11 +475,18 @@ int panfrost_mmu_map(struct panfrost_gem_mapping *mapping) if (WARN_ON(IS_ERR(sgt))) return PTR_ERR(sgt); - mmu_map_sg(pfdev, mapping->mmu, mapping->mmnode.start << PAGE_SHIFT, - prot, sgt); + ret = mmu_map_sg(pfdev, mapping->mmu, mapping->mmnode.start << PAGE_SHIFT, + prot, sgt); + if (ret) + goto err_put_pages; + mapping->active = true; return 0; + +err_put_pages: + drm_gem_shmem_put_pages_locked(shmem); + return ret; } void panfrost_mmu_unmap(struct panfrost_gem_mapping *mapping) @@ -635,8 +670,10 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, if (ret) goto err_map; - mmu_map_sg(pfdev, bomapping->mmu, addr, - IOMMU_WRITE | IOMMU_READ | IOMMU_CACHE | IOMMU_NOEXEC, sgt); + ret = mmu_map_sg(pfdev, bomapping->mmu, addr, + IOMMU_WRITE | IOMMU_READ | IOMMU_CACHE | IOMMU_NOEXEC, sgt); + if (ret) + goto err_mmu_map_sg; bomapping->active = true; bo->heap_rss_size += SZ_2M; @@ -650,6 +687,8 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, return 0; +err_mmu_map_sg: + dma_unmap_sgtable(pfdev->base.dev, sgt, DMA_BIDIRECTIONAL, 0); err_map: sg_free_table(sgt); err_unlock: -- 2.51.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v4 05/10] drm/panfrost: Handle page mapping failure 2025-10-01 2:20 ` [PATCH v4 05/10] drm/panfrost: Handle page mapping failure Adrián Larumbe @ 2025-10-01 10:58 ` Boris Brezillon 2025-10-07 1:04 ` Adrián Larumbe 0 siblings, 1 reply; 20+ messages in thread From: Boris Brezillon @ 2025-10-01 10:58 UTC (permalink / raw) To: Adrián Larumbe Cc: linux-kernel, dri-devel, Steven Price, kernel, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On Wed, 1 Oct 2025 03:20:26 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > When mapping the pages of a BO, either a heap type at page fault time or > else a non-heap BO at object creation time, if the ARM page table mapping > function fails, we unmap what had been mapped so far and bail out. > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_mmu.c | 49 ++++++++++++++++++++++--- > 1 file changed, 44 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c > index cf272b167feb..fb17c32855a5 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > @@ -393,13 +393,32 @@ static void panfrost_mmu_flush_range(struct panfrost_device *pfdev, > pm_runtime_put_autosuspend(pfdev->base.dev); > } > > +static void mmu_unmap_range(struct panfrost_mmu *mmu, u64 iova, size_t len) > +{ > + struct io_pgtable_ops *ops = mmu->pgtbl_ops; > + size_t pgsize, unmapped_len = 0; > + size_t unmapped_page, pgcount; > + > + while (unmapped_len < len) { > + pgsize = get_pgsize(iova, len - unmapped_len, &pgcount); > + > + unmapped_page = ops->unmap_pages(ops, iova, pgsize, pgcount, NULL); > + WARN_ON(unmapped_page != pgsize * pgcount); > + > + iova += pgsize * pgcount; > + unmapped_len += pgsize * pgcount; > + } > +} > + > static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, > u64 iova, int prot, struct sg_table *sgt) > { > unsigned int count; > struct scatterlist *sgl; > struct io_pgtable_ops *ops = mmu->pgtbl_ops; > + size_t total_mapped = 0; > u64 start_iova = iova; > + int ret; > > for_each_sgtable_dma_sg(sgt, sgl, count) { > unsigned long paddr = sg_dma_address(sgl); > @@ -413,10 +432,14 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, > size_t pgcount, mapped = 0; > size_t pgsize = get_pgsize(iova | paddr, len, &pgcount); > > - ops->map_pages(ops, iova, paddr, pgsize, pgcount, prot, > + ret = ops->map_pages(ops, iova, paddr, pgsize, pgcount, prot, > GFP_KERNEL, &mapped); > + if (ret) > + goto err_unmap_pages; > + > /* Don't get stuck if things have gone wrong */ > mapped = max(mapped, pgsize); > + total_mapped += mapped; > iova += mapped; > paddr += mapped; > len -= mapped; > @@ -426,6 +449,10 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, > panfrost_mmu_flush_range(pfdev, mmu, start_iova, iova - start_iova); > > return 0; > + > +err_unmap_pages: > + mmu_unmap_range(mmu, start_iova, total_mapped); > + return ret; > } > > int panfrost_mmu_map(struct panfrost_gem_mapping *mapping) > @@ -436,6 +463,7 @@ int panfrost_mmu_map(struct panfrost_gem_mapping *mapping) > struct panfrost_device *pfdev = to_panfrost_device(obj->dev); > struct sg_table *sgt; > int prot = IOMMU_READ | IOMMU_WRITE | IOMMU_CACHE; > + int ret; > > if (WARN_ON(mapping->active)) > return 0; > @@ -447,11 +475,18 @@ int panfrost_mmu_map(struct panfrost_gem_mapping *mapping) > if (WARN_ON(IS_ERR(sgt))) > return PTR_ERR(sgt); > > - mmu_map_sg(pfdev, mapping->mmu, mapping->mmnode.start << PAGE_SHIFT, > - prot, sgt); > + ret = mmu_map_sg(pfdev, mapping->mmu, mapping->mmnode.start << PAGE_SHIFT, > + prot, sgt); > + if (ret) > + goto err_put_pages; > + > mapping->active = true; > > return 0; > + > +err_put_pages: > + drm_gem_shmem_put_pages_locked(shmem); > + return ret; > } > > void panfrost_mmu_unmap(struct panfrost_gem_mapping *mapping) > @@ -635,8 +670,10 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, > if (ret) > goto err_map; > > - mmu_map_sg(pfdev, bomapping->mmu, addr, > - IOMMU_WRITE | IOMMU_READ | IOMMU_CACHE | IOMMU_NOEXEC, sgt); > + ret = mmu_map_sg(pfdev, bomapping->mmu, addr, > + IOMMU_WRITE | IOMMU_READ | IOMMU_CACHE | IOMMU_NOEXEC, sgt); > + if (ret) > + goto err_mmu_map_sg; > > bomapping->active = true; > bo->heap_rss_size += SZ_2M; > @@ -650,6 +687,8 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, > > return 0; > > +err_mmu_map_sg: > + dma_unmap_sgtable(pfdev->base.dev, sgt, DMA_BIDIRECTIONAL, 0); You also need to clear the sgts[]/pages[] entries you added earlier, otherwise the next time you have a fault it will bail-out before attempting an mmu_map_sg(). IIRC, Dmitry had a similar fix in his shmem-shrinker series. > err_map: > sg_free_table(sgt); > err_unlock: ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 05/10] drm/panfrost: Handle page mapping failure 2025-10-01 10:58 ` Boris Brezillon @ 2025-10-07 1:04 ` Adrián Larumbe 2025-10-07 7:51 ` Boris Brezillon 0 siblings, 1 reply; 20+ messages in thread From: Adrián Larumbe @ 2025-10-07 1:04 UTC (permalink / raw) To: Boris Brezillon Cc: linux-kernel, dri-devel, Steven Price, kernel, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter Hi Boris, On 01.10.2025 12:58, Boris Brezillon wrote: > On Wed, 1 Oct 2025 03:20:26 +0100 > Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > > > When mapping the pages of a BO, either a heap type at page fault time or > > else a non-heap BO at object creation time, if the ARM page table mapping > > function fails, we unmap what had been mapped so far and bail out. > > > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > > --- > > drivers/gpu/drm/panfrost/panfrost_mmu.c | 49 ++++++++++++++++++++++--- > > 1 file changed, 44 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c > > index cf272b167feb..fb17c32855a5 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > > @@ -393,13 +393,32 @@ static void panfrost_mmu_flush_range(struct panfrost_device *pfdev, > > pm_runtime_put_autosuspend(pfdev->base.dev); > > } > > > > +static void mmu_unmap_range(struct panfrost_mmu *mmu, u64 iova, size_t len) > > +{ > > + struct io_pgtable_ops *ops = mmu->pgtbl_ops; > > + size_t pgsize, unmapped_len = 0; > > + size_t unmapped_page, pgcount; > > + > > + while (unmapped_len < len) { > > + pgsize = get_pgsize(iova, len - unmapped_len, &pgcount); > > + > > + unmapped_page = ops->unmap_pages(ops, iova, pgsize, pgcount, NULL); > > + WARN_ON(unmapped_page != pgsize * pgcount); > > + > > + iova += pgsize * pgcount; > > + unmapped_len += pgsize * pgcount; > > + } > > +} > > + > > static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, > > u64 iova, int prot, struct sg_table *sgt) > > { > > unsigned int count; > > struct scatterlist *sgl; > > struct io_pgtable_ops *ops = mmu->pgtbl_ops; > > + size_t total_mapped = 0; > > u64 start_iova = iova; > > + int ret; > > > > for_each_sgtable_dma_sg(sgt, sgl, count) { > > unsigned long paddr = sg_dma_address(sgl); > > @@ -413,10 +432,14 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, > > size_t pgcount, mapped = 0; > > size_t pgsize = get_pgsize(iova | paddr, len, &pgcount); > > > > - ops->map_pages(ops, iova, paddr, pgsize, pgcount, prot, > > + ret = ops->map_pages(ops, iova, paddr, pgsize, pgcount, prot, > > GFP_KERNEL, &mapped); > > + if (ret) > > + goto err_unmap_pages; > > + > > /* Don't get stuck if things have gone wrong */ > > mapped = max(mapped, pgsize); > > + total_mapped += mapped; > > iova += mapped; > > paddr += mapped; > > len -= mapped; > > @@ -426,6 +449,10 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, > > panfrost_mmu_flush_range(pfdev, mmu, start_iova, iova - start_iova); > > > > return 0; > > + > > +err_unmap_pages: > > + mmu_unmap_range(mmu, start_iova, total_mapped); > > + return ret; > > } > > > > int panfrost_mmu_map(struct panfrost_gem_mapping *mapping) > > @@ -436,6 +463,7 @@ int panfrost_mmu_map(struct panfrost_gem_mapping *mapping) > > struct panfrost_device *pfdev = to_panfrost_device(obj->dev); > > struct sg_table *sgt; > > int prot = IOMMU_READ | IOMMU_WRITE | IOMMU_CACHE; > > + int ret; > > > > if (WARN_ON(mapping->active)) > > return 0; > > @@ -447,11 +475,18 @@ int panfrost_mmu_map(struct panfrost_gem_mapping *mapping) > > if (WARN_ON(IS_ERR(sgt))) > > return PTR_ERR(sgt); > > > > - mmu_map_sg(pfdev, mapping->mmu, mapping->mmnode.start << PAGE_SHIFT, > > - prot, sgt); > > + ret = mmu_map_sg(pfdev, mapping->mmu, mapping->mmnode.start << PAGE_SHIFT, > > + prot, sgt); > > + if (ret) > > + goto err_put_pages; > > + > > mapping->active = true; > > > > return 0; > > + > > +err_put_pages: > > + drm_gem_shmem_put_pages_locked(shmem); > > + return ret; > > } > > > > void panfrost_mmu_unmap(struct panfrost_gem_mapping *mapping) > > @@ -635,8 +670,10 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, > > if (ret) > > goto err_map; > > > > - mmu_map_sg(pfdev, bomapping->mmu, addr, > > - IOMMU_WRITE | IOMMU_READ | IOMMU_CACHE | IOMMU_NOEXEC, sgt); > > + ret = mmu_map_sg(pfdev, bomapping->mmu, addr, > > + IOMMU_WRITE | IOMMU_READ | IOMMU_CACHE | IOMMU_NOEXEC, sgt); > > + if (ret) > > + goto err_mmu_map_sg; > > > > bomapping->active = true; > > bo->heap_rss_size += SZ_2M; > > @@ -650,6 +687,8 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, > > > > return 0; > > > > +err_mmu_map_sg: > > + dma_unmap_sgtable(pfdev->base.dev, sgt, DMA_BIDIRECTIONAL, 0); > > You also need to clear the sgts[]/pages[] entries you added earlier, > otherwise the next time you have a fault it will bail-out before > attempting an mmu_map_sg(). > > IIRC, Dmitry had a similar fix in his shmem-shrinker series. Went over the mailing list and I think the commit you had in mind was 1fc9af813b25 ("drm/panfrost: Fix the error path in panfrost_mmu_map_fault_addr()") I suspect there's a problem with the present code. If shmem_read_mapping_page() fails for let's say, page_offset+5, then when the interrupt is triggered again, because the page array had already been allocated and pages[page_offset] populated in the first try, then it would bail out immediately even though most pages haven't been retrieved yet. On the other hand, depopulating the array for the IRQ to be triggered again seems wasteful. Because for any virtual address, a fault will map all the pages within its 2MiB boundaries, maybe we could change if (pages[page_offset]) { /* Pages are already mapped, bail out. */ goto out; } to 'pages[page_offset+NUM_FAULT_PAGES-1]' And then, in the event that mmu_map_sg() fails: err_mmu_map_sg: pages[page_offset+NUM_FAULT_PAGES-1] = NULL; dma_unmap_sgtable(pfdev->base.dev, sgt, DMA_BIDIRECTIONAL, 0); So that it'll only fetch the very last page instead of all of them in case of a page fault reattempt. > > err_map: > > sg_free_table(sgt); > > err_unlock: Adrian Larumbe ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 05/10] drm/panfrost: Handle page mapping failure 2025-10-07 1:04 ` Adrián Larumbe @ 2025-10-07 7:51 ` Boris Brezillon 0 siblings, 0 replies; 20+ messages in thread From: Boris Brezillon @ 2025-10-07 7:51 UTC (permalink / raw) To: Adrián Larumbe Cc: linux-kernel, dri-devel, Steven Price, kernel, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On Tue, 7 Oct 2025 02:04:00 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > Hi Boris, > > On 01.10.2025 12:58, Boris Brezillon wrote: > > On Wed, 1 Oct 2025 03:20:26 +0100 > > Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > > > > > When mapping the pages of a BO, either a heap type at page fault time or > > > else a non-heap BO at object creation time, if the ARM page table mapping > > > function fails, we unmap what had been mapped so far and bail out. > > > > > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > > > --- > > > drivers/gpu/drm/panfrost/panfrost_mmu.c | 49 ++++++++++++++++++++++--- > > > 1 file changed, 44 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c > > > index cf272b167feb..fb17c32855a5 100644 > > > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > > > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > > > @@ -393,13 +393,32 @@ static void panfrost_mmu_flush_range(struct panfrost_device *pfdev, > > > pm_runtime_put_autosuspend(pfdev->base.dev); > > > } > > > > > > +static void mmu_unmap_range(struct panfrost_mmu *mmu, u64 iova, size_t len) > > > +{ > > > + struct io_pgtable_ops *ops = mmu->pgtbl_ops; > > > + size_t pgsize, unmapped_len = 0; > > > + size_t unmapped_page, pgcount; > > > + > > > + while (unmapped_len < len) { > > > + pgsize = get_pgsize(iova, len - unmapped_len, &pgcount); > > > + > > > + unmapped_page = ops->unmap_pages(ops, iova, pgsize, pgcount, NULL); > > > + WARN_ON(unmapped_page != pgsize * pgcount); > > > + > > > + iova += pgsize * pgcount; > > > + unmapped_len += pgsize * pgcount; > > > + } > > > +} > > > + > > > static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, > > > u64 iova, int prot, struct sg_table *sgt) > > > { > > > unsigned int count; > > > struct scatterlist *sgl; > > > struct io_pgtable_ops *ops = mmu->pgtbl_ops; > > > + size_t total_mapped = 0; > > > u64 start_iova = iova; > > > + int ret; > > > > > > for_each_sgtable_dma_sg(sgt, sgl, count) { > > > unsigned long paddr = sg_dma_address(sgl); > > > @@ -413,10 +432,14 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, > > > size_t pgcount, mapped = 0; > > > size_t pgsize = get_pgsize(iova | paddr, len, &pgcount); > > > > > > - ops->map_pages(ops, iova, paddr, pgsize, pgcount, prot, > > > + ret = ops->map_pages(ops, iova, paddr, pgsize, pgcount, prot, > > > GFP_KERNEL, &mapped); > > > + if (ret) > > > + goto err_unmap_pages; > > > + > > > /* Don't get stuck if things have gone wrong */ > > > mapped = max(mapped, pgsize); > > > + total_mapped += mapped; > > > iova += mapped; > > > paddr += mapped; > > > len -= mapped; > > > @@ -426,6 +449,10 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu, > > > panfrost_mmu_flush_range(pfdev, mmu, start_iova, iova - start_iova); > > > > > > return 0; > > > + > > > +err_unmap_pages: > > > + mmu_unmap_range(mmu, start_iova, total_mapped); > > > + return ret; > > > } > > > > > > int panfrost_mmu_map(struct panfrost_gem_mapping *mapping) > > > @@ -436,6 +463,7 @@ int panfrost_mmu_map(struct panfrost_gem_mapping *mapping) > > > struct panfrost_device *pfdev = to_panfrost_device(obj->dev); > > > struct sg_table *sgt; > > > int prot = IOMMU_READ | IOMMU_WRITE | IOMMU_CACHE; > > > + int ret; > > > > > > if (WARN_ON(mapping->active)) > > > return 0; > > > @@ -447,11 +475,18 @@ int panfrost_mmu_map(struct panfrost_gem_mapping *mapping) > > > if (WARN_ON(IS_ERR(sgt))) > > > return PTR_ERR(sgt); > > > > > > - mmu_map_sg(pfdev, mapping->mmu, mapping->mmnode.start << PAGE_SHIFT, > > > - prot, sgt); > > > + ret = mmu_map_sg(pfdev, mapping->mmu, mapping->mmnode.start << PAGE_SHIFT, > > > + prot, sgt); > > > + if (ret) > > > + goto err_put_pages; > > > + > > > mapping->active = true; > > > > > > return 0; > > > + > > > +err_put_pages: > > > + drm_gem_shmem_put_pages_locked(shmem); > > > + return ret; > > > } > > > > > > void panfrost_mmu_unmap(struct panfrost_gem_mapping *mapping) > > > @@ -635,8 +670,10 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, > > > if (ret) > > > goto err_map; > > > > > > - mmu_map_sg(pfdev, bomapping->mmu, addr, > > > - IOMMU_WRITE | IOMMU_READ | IOMMU_CACHE | IOMMU_NOEXEC, sgt); > > > + ret = mmu_map_sg(pfdev, bomapping->mmu, addr, > > > + IOMMU_WRITE | IOMMU_READ | IOMMU_CACHE | IOMMU_NOEXEC, sgt); > > > + if (ret) > > > + goto err_mmu_map_sg; > > > > > > bomapping->active = true; > > > bo->heap_rss_size += SZ_2M; > > > @@ -650,6 +687,8 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, > > > > > > return 0; > > > > > > +err_mmu_map_sg: > > > + dma_unmap_sgtable(pfdev->base.dev, sgt, DMA_BIDIRECTIONAL, 0); > > > > You also need to clear the sgts[]/pages[] entries you added earlier, > > otherwise the next time you have a fault it will bail-out before > > attempting an mmu_map_sg(). > > > > IIRC, Dmitry had a similar fix in his shmem-shrinker series. > > Went over the mailing list and I think the commit you had in mind was 1fc9af813b25 ("drm/panfrost: Fix the error path in panfrost_mmu_map_fault_addr()") > > I suspect there's a problem with the present code. If shmem_read_mapping_page() fails for let's say, page_offset+5, then when the interrupt is triggered > again, because the page array had already been allocated and pages[page_offset] populated in the first try, then it would bail out immediately even though > most pages haven't been retrieved yet. > > On the other hand, depopulating the array for the IRQ to be triggered again seems wasteful. Because for any virtual address, a fault will map all the > pages within its 2MiB boundaries, maybe we could change > > if (pages[page_offset]) { > /* Pages are already mapped, bail out. */ > goto out; > } > > to 'pages[page_offset+NUM_FAULT_PAGES-1]' Or, we simply don't check the pages[page_offset..page_offset+NUM_FAULT_PAGES-1] range and we let the following loop walk over all entries and fill the missing ones, if any. > > And then, in the event that mmu_map_sg() fails: > > err_mmu_map_sg: > pages[page_offset+NUM_FAULT_PAGES-1] = NULL; > dma_unmap_sgtable(pfdev->base.dev, sgt, DMA_BIDIRECTIONAL, 0); > > So that it'll only fetch the very last page instead of all of them in case of a page fault reattempt. > > > > err_map: > > > sg_free_table(sgt); > > > err_unlock: > > > Adrian Larumbe ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 06/10] drm/panfrost: Don't rework job IRQ enable mask in the enable path 2025-10-01 2:20 [PATCH v4 00/10] Some Panfrost fixes and improvements Adrián Larumbe ` (4 preceding siblings ...) 2025-10-01 2:20 ` [PATCH v4 05/10] drm/panfrost: Handle page mapping failure Adrián Larumbe @ 2025-10-01 2:20 ` Adrián Larumbe 2025-10-01 11:00 ` Boris Brezillon 2025-10-01 2:20 ` [PATCH v4 07/10] drm/panfrost: Make re-enabling job interrupts at device reset optional Adrián Larumbe ` (3 subsequent siblings) 9 siblings, 1 reply; 20+ messages in thread From: Adrián Larumbe @ 2025-10-01 2:20 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Steven Price, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter Up until now, panfrost_job_enable_interrupts() would always recalculate the same job IRQ enablement mask, which is effectively a constant. Replace it with a compile-time constant value, and also in another couple places where an equivalent expression was being used. Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_device.h | 4 ++++ drivers/gpu/drm/panfrost/panfrost_job.c | 19 ++++--------------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index 474b232bb38e..ac7147ed806b 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -26,6 +26,10 @@ struct panfrost_perfcnt; #define MAX_PM_DOMAINS 5 +#define ALL_JS_INT_MASK \ + (GENMASK(16 + NUM_JOB_SLOTS - 1, 16) | \ + GENMASK(NUM_JOB_SLOTS - 1, 0)) + enum panfrost_drv_comp_bits { PANFROST_COMP_BIT_GPU, PANFROST_COMP_BIT_JOB, diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index ba934437a8ea..54764ce91dea 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -428,17 +428,10 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) void panfrost_job_enable_interrupts(struct panfrost_device *pfdev) { - int j; - u32 irq_mask = 0; - clear_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended); - for (j = 0; j < NUM_JOB_SLOTS; j++) { - irq_mask |= MK_JS_MASK(j); - } - - job_write(pfdev, JOB_INT_CLEAR, irq_mask); - job_write(pfdev, JOB_INT_MASK, irq_mask); + job_write(pfdev, JOB_INT_CLEAR, ALL_JS_INT_MASK); + job_write(pfdev, JOB_INT_MASK, ALL_JS_INT_MASK); } void panfrost_job_suspend_irq(struct panfrost_device *pfdev) @@ -757,9 +750,7 @@ panfrost_reset(struct panfrost_device *pfdev, drm_sched_start(&pfdev->js->queue[i].sched, 0); /* Re-enable job interrupts now that everything has been restarted. */ - job_write(pfdev, JOB_INT_MASK, - GENMASK(16 + NUM_JOB_SLOTS - 1, 16) | - GENMASK(NUM_JOB_SLOTS - 1, 0)); + job_write(pfdev, JOB_INT_MASK, ALL_JS_INT_MASK); dma_fence_end_signalling(cookie); } @@ -832,9 +823,7 @@ static irqreturn_t panfrost_job_irq_handler_thread(int irq, void *data) /* Enable interrupts only if we're not about to get suspended */ if (!test_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended)) - job_write(pfdev, JOB_INT_MASK, - GENMASK(16 + NUM_JOB_SLOTS - 1, 16) | - GENMASK(NUM_JOB_SLOTS - 1, 0)); + job_write(pfdev, JOB_INT_MASK, ALL_JS_INT_MASK); return IRQ_HANDLED; } -- 2.51.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v4 06/10] drm/panfrost: Don't rework job IRQ enable mask in the enable path 2025-10-01 2:20 ` [PATCH v4 06/10] drm/panfrost: Don't rework job IRQ enable mask in the enable path Adrián Larumbe @ 2025-10-01 11:00 ` Boris Brezillon 0 siblings, 0 replies; 20+ messages in thread From: Boris Brezillon @ 2025-10-01 11:00 UTC (permalink / raw) To: Adrián Larumbe Cc: linux-kernel, dri-devel, Steven Price, kernel, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On Wed, 1 Oct 2025 03:20:27 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > Up until now, panfrost_job_enable_interrupts() would always recalculate the > same job IRQ enablement mask, which is effectively a constant. > > Replace it with a compile-time constant value, and also in another couple > places where an equivalent expression was being used. > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_device.h | 4 ++++ > drivers/gpu/drm/panfrost/panfrost_job.c | 19 ++++--------------- > 2 files changed, 8 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h > index 474b232bb38e..ac7147ed806b 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -26,6 +26,10 @@ struct panfrost_perfcnt; > > #define MAX_PM_DOMAINS 5 > > +#define ALL_JS_INT_MASK \ > + (GENMASK(16 + NUM_JOB_SLOTS - 1, 16) | \ > + GENMASK(NUM_JOB_SLOTS - 1, 0)) > + > enum panfrost_drv_comp_bits { > PANFROST_COMP_BIT_GPU, > PANFROST_COMP_BIT_JOB, > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c > index ba934437a8ea..54764ce91dea 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > @@ -428,17 +428,10 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) > > void panfrost_job_enable_interrupts(struct panfrost_device *pfdev) > { > - int j; > - u32 irq_mask = 0; > - > clear_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended); > > - for (j = 0; j < NUM_JOB_SLOTS; j++) { > - irq_mask |= MK_JS_MASK(j); > - } > - > - job_write(pfdev, JOB_INT_CLEAR, irq_mask); > - job_write(pfdev, JOB_INT_MASK, irq_mask); > + job_write(pfdev, JOB_INT_CLEAR, ALL_JS_INT_MASK); > + job_write(pfdev, JOB_INT_MASK, ALL_JS_INT_MASK); > } > > void panfrost_job_suspend_irq(struct panfrost_device *pfdev) > @@ -757,9 +750,7 @@ panfrost_reset(struct panfrost_device *pfdev, > drm_sched_start(&pfdev->js->queue[i].sched, 0); > > /* Re-enable job interrupts now that everything has been restarted. */ > - job_write(pfdev, JOB_INT_MASK, > - GENMASK(16 + NUM_JOB_SLOTS - 1, 16) | > - GENMASK(NUM_JOB_SLOTS - 1, 0)); > + job_write(pfdev, JOB_INT_MASK, ALL_JS_INT_MASK); > > dma_fence_end_signalling(cookie); > } > @@ -832,9 +823,7 @@ static irqreturn_t panfrost_job_irq_handler_thread(int irq, void *data) > > /* Enable interrupts only if we're not about to get suspended */ > if (!test_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended)) > - job_write(pfdev, JOB_INT_MASK, > - GENMASK(16 + NUM_JOB_SLOTS - 1, 16) | > - GENMASK(NUM_JOB_SLOTS - 1, 0)); > + job_write(pfdev, JOB_INT_MASK, ALL_JS_INT_MASK); > > return IRQ_HANDLED; > } ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 07/10] drm/panfrost: Make re-enabling job interrupts at device reset optional 2025-10-01 2:20 [PATCH v4 00/10] Some Panfrost fixes and improvements Adrián Larumbe ` (5 preceding siblings ...) 2025-10-01 2:20 ` [PATCH v4 06/10] drm/panfrost: Don't rework job IRQ enable mask in the enable path Adrián Larumbe @ 2025-10-01 2:20 ` Adrián Larumbe 2025-10-01 11:02 ` Boris Brezillon 2025-10-01 2:20 ` [PATCH v4 08/10] drm/panfrost: Add forward declaration and types header Adrián Larumbe ` (2 subsequent siblings) 9 siblings, 1 reply; 20+ messages in thread From: Adrián Larumbe @ 2025-10-01 2:20 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Steven Price, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter Rather than remasking interrupts after a device reset in the main reset path, allow selecting whether to do this with an additional bool parameter. To this end, split reenabling job interrupts into two functions, one that clears the interrupts and another one which unmasks them conditionally. Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_device.c | 9 ++++++--- drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- drivers/gpu/drm/panfrost/panfrost_job.c | 17 ++++++++--------- drivers/gpu/drm/panfrost/panfrost_job.h | 1 + 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 733b728ec75f..f1d811a6de6c 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -400,13 +400,16 @@ bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev, return false; } -void panfrost_device_reset(struct panfrost_device *pfdev) +void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int) { panfrost_gpu_soft_reset(pfdev); panfrost_gpu_power_on(pfdev); panfrost_mmu_reset(pfdev); - panfrost_job_enable_interrupts(pfdev); + + panfrost_job_reset_interrupts(pfdev); + if (enable_job_int) + panfrost_job_enable_interrupts(pfdev); } static int panfrost_device_runtime_resume(struct device *dev) @@ -430,7 +433,7 @@ static int panfrost_device_runtime_resume(struct device *dev) } } - panfrost_device_reset(pfdev); + panfrost_device_reset(pfdev, true); panfrost_devfreq_resume(pfdev); return 0; diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index ac7147ed806b..45d77cda8b89 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -250,7 +250,7 @@ int panfrost_unstable_ioctl_check(void); int panfrost_device_init(struct panfrost_device *pfdev); void panfrost_device_fini(struct panfrost_device *pfdev); -void panfrost_device_reset(struct panfrost_device *pfdev); +void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int); extern const struct dev_pm_ops panfrost_pm_ops; diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 54764ce91dea..3ae984f6290f 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -426,11 +426,14 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) return fence; } +void panfrost_job_reset_interrupts(struct panfrost_device *pfdev) +{ + job_write(pfdev, JOB_INT_CLEAR, ALL_JS_INT_MASK); +} + void panfrost_job_enable_interrupts(struct panfrost_device *pfdev) { clear_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended); - - job_write(pfdev, JOB_INT_CLEAR, ALL_JS_INT_MASK); job_write(pfdev, JOB_INT_MASK, ALL_JS_INT_MASK); } @@ -723,12 +726,7 @@ panfrost_reset(struct panfrost_device *pfdev, spin_unlock(&pfdev->js->job_lock); /* Proceed with reset now. */ - panfrost_device_reset(pfdev); - - /* panfrost_device_reset() unmasks job interrupts, but we want to - * keep them masked a bit longer. - */ - job_write(pfdev, JOB_INT_MASK, 0); + panfrost_device_reset(pfdev, false); /* GPU has been reset, we can clear the reset pending bit. */ atomic_set(&pfdev->reset.pending, 0); @@ -750,7 +748,7 @@ panfrost_reset(struct panfrost_device *pfdev, drm_sched_start(&pfdev->js->queue[i].sched, 0); /* Re-enable job interrupts now that everything has been restarted. */ - job_write(pfdev, JOB_INT_MASK, ALL_JS_INT_MASK); + panfrost_job_enable_interrupts(pfdev); dma_fence_end_signalling(cookie); } @@ -901,6 +899,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) } } + panfrost_job_reset_interrupts(pfdev); panfrost_job_enable_interrupts(pfdev); return 0; diff --git a/drivers/gpu/drm/panfrost/panfrost_job.h b/drivers/gpu/drm/panfrost/panfrost_job.h index 5a30ff1503c6..30eda74e3c34 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.h +++ b/drivers/gpu/drm/panfrost/panfrost_job.h @@ -67,6 +67,7 @@ void panfrost_job_close(struct drm_file *file); int panfrost_job_get_slot(struct panfrost_job *job); int panfrost_job_push(struct panfrost_job *job); void panfrost_job_put(struct panfrost_job *job); +void panfrost_job_reset_interrupts(struct panfrost_device *pfdev); void panfrost_job_enable_interrupts(struct panfrost_device *pfdev); void panfrost_job_suspend_irq(struct panfrost_device *pfdev); int panfrost_job_is_idle(struct panfrost_device *pfdev); -- 2.51.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v4 07/10] drm/panfrost: Make re-enabling job interrupts at device reset optional 2025-10-01 2:20 ` [PATCH v4 07/10] drm/panfrost: Make re-enabling job interrupts at device reset optional Adrián Larumbe @ 2025-10-01 11:02 ` Boris Brezillon 0 siblings, 0 replies; 20+ messages in thread From: Boris Brezillon @ 2025-10-01 11:02 UTC (permalink / raw) To: Adrián Larumbe Cc: linux-kernel, dri-devel, Steven Price, kernel, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On Wed, 1 Oct 2025 03:20:28 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > Rather than remasking interrupts after a device reset in the main reset > path, allow selecting whether to do this with an additional bool parameter. > > To this end, split reenabling job interrupts into two functions, one that > clears the interrupts and another one which unmasks them conditionally. > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_device.c | 9 ++++++--- > drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- > drivers/gpu/drm/panfrost/panfrost_job.c | 17 ++++++++--------- > drivers/gpu/drm/panfrost/panfrost_job.h | 1 + > 4 files changed, 16 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c > index 733b728ec75f..f1d811a6de6c 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c > @@ -400,13 +400,16 @@ bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev, > return false; > } > > -void panfrost_device_reset(struct panfrost_device *pfdev) > +void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int) > { > panfrost_gpu_soft_reset(pfdev); > > panfrost_gpu_power_on(pfdev); > panfrost_mmu_reset(pfdev); > - panfrost_job_enable_interrupts(pfdev); > + > + panfrost_job_reset_interrupts(pfdev); > + if (enable_job_int) > + panfrost_job_enable_interrupts(pfdev); > } > > static int panfrost_device_runtime_resume(struct device *dev) > @@ -430,7 +433,7 @@ static int panfrost_device_runtime_resume(struct device *dev) > } > } > > - panfrost_device_reset(pfdev); > + panfrost_device_reset(pfdev, true); > panfrost_devfreq_resume(pfdev); > > return 0; > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h > index ac7147ed806b..45d77cda8b89 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -250,7 +250,7 @@ int panfrost_unstable_ioctl_check(void); > > int panfrost_device_init(struct panfrost_device *pfdev); > void panfrost_device_fini(struct panfrost_device *pfdev); > -void panfrost_device_reset(struct panfrost_device *pfdev); > +void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int); > > extern const struct dev_pm_ops panfrost_pm_ops; > > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c > index 54764ce91dea..3ae984f6290f 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > @@ -426,11 +426,14 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) > return fence; > } > > +void panfrost_job_reset_interrupts(struct panfrost_device *pfdev) > +{ > + job_write(pfdev, JOB_INT_CLEAR, ALL_JS_INT_MASK); > +} > + > void panfrost_job_enable_interrupts(struct panfrost_device *pfdev) > { > clear_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended); > - > - job_write(pfdev, JOB_INT_CLEAR, ALL_JS_INT_MASK); > job_write(pfdev, JOB_INT_MASK, ALL_JS_INT_MASK); > } > > @@ -723,12 +726,7 @@ panfrost_reset(struct panfrost_device *pfdev, > spin_unlock(&pfdev->js->job_lock); > > /* Proceed with reset now. */ > - panfrost_device_reset(pfdev); > - > - /* panfrost_device_reset() unmasks job interrupts, but we want to > - * keep them masked a bit longer. > - */ > - job_write(pfdev, JOB_INT_MASK, 0); > + panfrost_device_reset(pfdev, false); > > /* GPU has been reset, we can clear the reset pending bit. */ > atomic_set(&pfdev->reset.pending, 0); > @@ -750,7 +748,7 @@ panfrost_reset(struct panfrost_device *pfdev, > drm_sched_start(&pfdev->js->queue[i].sched, 0); > > /* Re-enable job interrupts now that everything has been restarted. */ > - job_write(pfdev, JOB_INT_MASK, ALL_JS_INT_MASK); > + panfrost_job_enable_interrupts(pfdev); > > dma_fence_end_signalling(cookie); > } > @@ -901,6 +899,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) > } > } > > + panfrost_job_reset_interrupts(pfdev); > panfrost_job_enable_interrupts(pfdev); > > return 0; > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.h b/drivers/gpu/drm/panfrost/panfrost_job.h > index 5a30ff1503c6..30eda74e3c34 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.h > +++ b/drivers/gpu/drm/panfrost/panfrost_job.h > @@ -67,6 +67,7 @@ void panfrost_job_close(struct drm_file *file); > int panfrost_job_get_slot(struct panfrost_job *job); > int panfrost_job_push(struct panfrost_job *job); > void panfrost_job_put(struct panfrost_job *job); > +void panfrost_job_reset_interrupts(struct panfrost_device *pfdev); > void panfrost_job_enable_interrupts(struct panfrost_device *pfdev); > void panfrost_job_suspend_irq(struct panfrost_device *pfdev); > int panfrost_job_is_idle(struct panfrost_device *pfdev); ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 08/10] drm/panfrost: Add forward declaration and types header 2025-10-01 2:20 [PATCH v4 00/10] Some Panfrost fixes and improvements Adrián Larumbe ` (6 preceding siblings ...) 2025-10-01 2:20 ` [PATCH v4 07/10] drm/panfrost: Make re-enabling job interrupts at device reset optional Adrián Larumbe @ 2025-10-01 2:20 ` Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 09/10] drm/panfrost: Remove unused device property Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 10/10] drm/panfrost: Rename panfrost_job functions to reflect real role Adrián Larumbe 9 siblings, 0 replies; 20+ messages in thread From: Adrián Larumbe @ 2025-10-01 2:20 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Steven Price, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, llvm This is to make LLVM syntactic analysers happy. Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_mmu.h | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.h b/drivers/gpu/drm/panfrost/panfrost_mmu.h index e6e6966a0cca..27c3c65ed074 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.h +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.h @@ -4,6 +4,7 @@ #ifndef __PANFROST_MMU_H__ #define __PANFROST_MMU_H__ +struct panfrost_device; struct panfrost_gem_mapping; struct panfrost_file_priv; struct panfrost_mmu; -- 2.51.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v4 09/10] drm/panfrost: Remove unused device property 2025-10-01 2:20 [PATCH v4 00/10] Some Panfrost fixes and improvements Adrián Larumbe ` (7 preceding siblings ...) 2025-10-01 2:20 ` [PATCH v4 08/10] drm/panfrost: Add forward declaration and types header Adrián Larumbe @ 2025-10-01 2:20 ` Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 10/10] drm/panfrost: Rename panfrost_job functions to reflect real role Adrián Larumbe 9 siblings, 0 replies; 20+ messages in thread From: Adrián Larumbe @ 2025-10-01 2:20 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Steven Price, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter The as_in_use_mask device state variable is no longer in use. Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_device.h | 1 - drivers/gpu/drm/panfrost/panfrost_mmu.c | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index 45d77cda8b89..e61c4329fd07 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -147,7 +147,6 @@ struct panfrost_device { DECLARE_BITMAP(is_suspended, PANFROST_COMP_BIT_MAX); spinlock_t as_lock; - unsigned long as_in_use_mask; unsigned long as_alloc_mask; unsigned long as_faulty_mask; struct list_head as_lru_list; diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index fb17c32855a5..d54cb7bbc1a2 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -712,7 +712,6 @@ static void panfrost_mmu_release_ctx(struct kref *kref) pm_runtime_put_autosuspend(pfdev->base.dev); clear_bit(mmu->as, &pfdev->as_alloc_mask); - clear_bit(mmu->as, &pfdev->as_in_use_mask); list_del(&mmu->list); } spin_unlock(&pfdev->as_lock); -- 2.51.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v4 10/10] drm/panfrost: Rename panfrost_job functions to reflect real role 2025-10-01 2:20 [PATCH v4 00/10] Some Panfrost fixes and improvements Adrián Larumbe ` (8 preceding siblings ...) 2025-10-01 2:20 ` [PATCH v4 09/10] drm/panfrost: Remove unused device property Adrián Larumbe @ 2025-10-01 2:20 ` Adrián Larumbe 2025-10-01 15:19 ` Boris Brezillon 9 siblings, 1 reply; 20+ messages in thread From: Adrián Larumbe @ 2025-10-01 2:20 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Steven Price, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter panfrost_job_* prefixed functions in panfrost_job.c deal with both panfrost_job objects and also the more general JM (Job Manager) side of the device itself. This is confusing. Reprefix functions that program the JM to panfrosot_jm_* instead. Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_device.c | 14 +++---- drivers/gpu/drm/panfrost/panfrost_drv.c | 4 +- drivers/gpu/drm/panfrost/panfrost_job.c | 48 +++++++++++----------- drivers/gpu/drm/panfrost/panfrost_job.h | 16 ++++---- 4 files changed, 41 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index f1d811a6de6c..c61b97af120c 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -259,7 +259,7 @@ int panfrost_device_init(struct panfrost_device *pfdev) if (err) goto out_gpu; - err = panfrost_job_init(pfdev); + err = panfrost_jm_init(pfdev); if (err) goto out_mmu; @@ -269,7 +269,7 @@ int panfrost_device_init(struct panfrost_device *pfdev) return 0; out_job: - panfrost_job_fini(pfdev); + panfrost_jm_fini(pfdev); out_mmu: panfrost_mmu_fini(pfdev); out_gpu: @@ -290,7 +290,7 @@ int panfrost_device_init(struct panfrost_device *pfdev) void panfrost_device_fini(struct panfrost_device *pfdev) { panfrost_perfcnt_fini(pfdev); - panfrost_job_fini(pfdev); + panfrost_jm_fini(pfdev); panfrost_mmu_fini(pfdev); panfrost_gpu_fini(pfdev); panfrost_devfreq_fini(pfdev); @@ -407,9 +407,9 @@ void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int) panfrost_gpu_power_on(pfdev); panfrost_mmu_reset(pfdev); - panfrost_job_reset_interrupts(pfdev); + panfrost_jm_reset_interrupts(pfdev); if (enable_job_int) - panfrost_job_enable_interrupts(pfdev); + panfrost_jm_enable_interrupts(pfdev); } static int panfrost_device_runtime_resume(struct device *dev) @@ -451,11 +451,11 @@ static int panfrost_device_runtime_suspend(struct device *dev) { struct panfrost_device *pfdev = dev_get_drvdata(dev); - if (!panfrost_job_is_idle(pfdev)) + if (!panfrost_jm_is_idle(pfdev)) return -EBUSY; panfrost_devfreq_suspend(pfdev); - panfrost_job_suspend_irq(pfdev); + panfrost_jm_suspend_irq(pfdev); panfrost_mmu_suspend_irq(pfdev); panfrost_gpu_suspend_irq(pfdev); panfrost_gpu_power_off(pfdev); diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 2b57f6813714..3b79ebbccdf5 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -606,7 +606,7 @@ panfrost_open(struct drm_device *dev, struct drm_file *file) goto err_free; } - ret = panfrost_job_open(file); + ret = panfrost_jm_open(file); if (ret) goto err_job; @@ -625,7 +625,7 @@ panfrost_postclose(struct drm_device *dev, struct drm_file *file) struct panfrost_file_priv *panfrost_priv = file->driver_priv; panfrost_perfcnt_close(file); - panfrost_job_close(file); + panfrost_jm_close(file); panfrost_mmu_ctx_put(panfrost_priv->mmu); kfree(panfrost_priv); diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 3ae984f6290f..4a213db9962d 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -426,18 +426,18 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) return fence; } -void panfrost_job_reset_interrupts(struct panfrost_device *pfdev) +void panfrost_jm_reset_interrupts(struct panfrost_device *pfdev) { job_write(pfdev, JOB_INT_CLEAR, ALL_JS_INT_MASK); } -void panfrost_job_enable_interrupts(struct panfrost_device *pfdev) +void panfrost_jm_enable_interrupts(struct panfrost_device *pfdev) { clear_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended); job_write(pfdev, JOB_INT_MASK, ALL_JS_INT_MASK); } -void panfrost_job_suspend_irq(struct panfrost_device *pfdev) +void panfrost_jm_suspend_irq(struct panfrost_device *pfdev) { set_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended); @@ -499,8 +499,8 @@ static void panfrost_job_handle_err(struct panfrost_device *pfdev, } } -static void panfrost_job_handle_done(struct panfrost_device *pfdev, - struct panfrost_job *job) +static void panfrost_jm_handle_done(struct panfrost_device *pfdev, + struct panfrost_job *job) { /* Set ->jc to 0 to avoid re-submitting an already finished job (can * happen when we receive the DONE interrupt while doing a GPU reset). @@ -513,7 +513,7 @@ static void panfrost_job_handle_done(struct panfrost_device *pfdev, pm_runtime_put_autosuspend(pfdev->base.dev); } -static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) +static void panfrost_jm_handle_irq(struct panfrost_device *pfdev, u32 status) { struct panfrost_job *done[NUM_JOB_SLOTS][2] = {}; struct panfrost_job *failed[NUM_JOB_SLOTS] = {}; @@ -588,7 +588,7 @@ static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) } for (i = 0; i < ARRAY_SIZE(done[0]) && done[j][i]; i++) - panfrost_job_handle_done(pfdev, done[j][i]); + panfrost_jm_handle_done(pfdev, done[j][i]); } /* And finally we requeue jobs that were waiting in the second slot @@ -606,7 +606,7 @@ static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) struct panfrost_job *canceled = panfrost_dequeue_job(pfdev, j); dma_fence_set_error(canceled->done_fence, -ECANCELED); - panfrost_job_handle_done(pfdev, canceled); + panfrost_jm_handle_done(pfdev, canceled); } else if (!atomic_read(&pfdev->reset.pending)) { /* Requeue the job we removed if no reset is pending */ job_write(pfdev, JS_COMMAND_NEXT(j), JS_COMMAND_START); @@ -614,7 +614,7 @@ static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) } } -static void panfrost_job_handle_irqs(struct panfrost_device *pfdev) +static void panfrost_jm_handle_irqs(struct panfrost_device *pfdev) { u32 status = job_read(pfdev, JOB_INT_RAWSTAT); @@ -622,7 +622,7 @@ static void panfrost_job_handle_irqs(struct panfrost_device *pfdev) pm_runtime_mark_last_busy(pfdev->base.dev); spin_lock(&pfdev->js->job_lock); - panfrost_job_handle_irq(pfdev, status); + panfrost_jm_handle_irq(pfdev, status); spin_unlock(&pfdev->js->job_lock); status = job_read(pfdev, JOB_INT_RAWSTAT); } @@ -703,7 +703,7 @@ panfrost_reset(struct panfrost_device *pfdev, dev_err(pfdev->base.dev, "Soft-stop failed\n"); /* Handle the remaining interrupts before we reset. */ - panfrost_job_handle_irqs(pfdev); + panfrost_jm_handle_irqs(pfdev); /* Remaining interrupts have been handled, but we might still have * stuck jobs. Let's make sure the PM counters stay balanced by @@ -748,7 +748,7 @@ panfrost_reset(struct panfrost_device *pfdev, drm_sched_start(&pfdev->js->queue[i].sched, 0); /* Re-enable job interrupts now that everything has been restarted. */ - panfrost_job_enable_interrupts(pfdev); + panfrost_jm_enable_interrupts(pfdev); dma_fence_end_signalling(cookie); } @@ -813,11 +813,11 @@ static const struct drm_sched_backend_ops panfrost_sched_ops = { .free_job = panfrost_job_free }; -static irqreturn_t panfrost_job_irq_handler_thread(int irq, void *data) +static irqreturn_t panfrost_jm_irq_handler_thread(int irq, void *data) { struct panfrost_device *pfdev = data; - panfrost_job_handle_irqs(pfdev); + panfrost_jm_handle_irqs(pfdev); /* Enable interrupts only if we're not about to get suspended */ if (!test_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended)) @@ -826,7 +826,7 @@ static irqreturn_t panfrost_job_irq_handler_thread(int irq, void *data) return IRQ_HANDLED; } -static irqreturn_t panfrost_job_irq_handler(int irq, void *data) +static irqreturn_t panfrost_jm_irq_handler(int irq, void *data) { struct panfrost_device *pfdev = data; u32 status; @@ -842,7 +842,7 @@ static irqreturn_t panfrost_job_irq_handler(int irq, void *data) return IRQ_WAKE_THREAD; } -int panfrost_job_init(struct panfrost_device *pfdev) +int panfrost_jm_init(struct panfrost_device *pfdev) { struct drm_sched_init_args args = { .ops = &panfrost_sched_ops, @@ -875,8 +875,8 @@ int panfrost_job_init(struct panfrost_device *pfdev) return js->irq; ret = devm_request_threaded_irq(pfdev->base.dev, js->irq, - panfrost_job_irq_handler, - panfrost_job_irq_handler_thread, + panfrost_jm_irq_handler, + panfrost_jm_irq_handler_thread, IRQF_SHARED, KBUILD_MODNAME "-job", pfdev); if (ret) { @@ -899,8 +899,8 @@ int panfrost_job_init(struct panfrost_device *pfdev) } } - panfrost_job_reset_interrupts(pfdev); - panfrost_job_enable_interrupts(pfdev); + panfrost_jm_reset_interrupts(pfdev); + panfrost_jm_enable_interrupts(pfdev); return 0; @@ -912,7 +912,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) return ret; } -void panfrost_job_fini(struct panfrost_device *pfdev) +void panfrost_jm_fini(struct panfrost_device *pfdev) { struct panfrost_job_slot *js = pfdev->js; int j; @@ -927,7 +927,7 @@ void panfrost_job_fini(struct panfrost_device *pfdev) destroy_workqueue(pfdev->reset.wq); } -int panfrost_job_open(struct drm_file *file) +int panfrost_jm_open(struct drm_file *file) { struct panfrost_file_priv *panfrost_priv = file->driver_priv; int ret; @@ -949,7 +949,7 @@ int panfrost_job_open(struct drm_file *file) return 0; } -void panfrost_job_close(struct drm_file *file) +void panfrost_jm_close(struct drm_file *file) { struct panfrost_file_priv *panfrost_priv = file->driver_priv; struct panfrost_jm_ctx *jm_ctx; @@ -961,7 +961,7 @@ void panfrost_job_close(struct drm_file *file) xa_destroy(&panfrost_priv->jm_ctxs); } -int panfrost_job_is_idle(struct panfrost_device *pfdev) +int panfrost_jm_is_idle(struct panfrost_device *pfdev) { struct panfrost_job_slot *js = pfdev->js; int i; diff --git a/drivers/gpu/drm/panfrost/panfrost_job.h b/drivers/gpu/drm/panfrost/panfrost_job.h index 30eda74e3c34..da96c674d62b 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.h +++ b/drivers/gpu/drm/panfrost/panfrost_job.h @@ -60,16 +60,16 @@ void panfrost_jm_ctx_put(struct panfrost_jm_ctx *jm_ctx); struct panfrost_jm_ctx *panfrost_jm_ctx_get(struct panfrost_jm_ctx *jm_ctx); struct panfrost_jm_ctx *panfrost_jm_ctx_from_handle(struct drm_file *file, u32 handle); -int panfrost_job_init(struct panfrost_device *pfdev); -void panfrost_job_fini(struct panfrost_device *pfdev); -int panfrost_job_open(struct drm_file *file); -void panfrost_job_close(struct drm_file *file); +int panfrost_jm_init(struct panfrost_device *pfdev); +void panfrost_jm_fini(struct panfrost_device *pfdev); +int panfrost_jm_open(struct drm_file *file); +void panfrost_jm_close(struct drm_file *file); +void panfrost_jm_reset_interrupts(struct panfrost_device *pfdev); +void panfrost_jm_enable_interrupts(struct panfrost_device *pfdev); +void panfrost_jm_suspend_irq(struct panfrost_device *pfdev); +int panfrost_jm_is_idle(struct panfrost_device *pfdev); int panfrost_job_get_slot(struct panfrost_job *job); int panfrost_job_push(struct panfrost_job *job); void panfrost_job_put(struct panfrost_job *job); -void panfrost_job_reset_interrupts(struct panfrost_device *pfdev); -void panfrost_job_enable_interrupts(struct panfrost_device *pfdev); -void panfrost_job_suspend_irq(struct panfrost_device *pfdev); -int panfrost_job_is_idle(struct panfrost_device *pfdev); #endif -- 2.51.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v4 10/10] drm/panfrost: Rename panfrost_job functions to reflect real role 2025-10-01 2:20 ` [PATCH v4 10/10] drm/panfrost: Rename panfrost_job functions to reflect real role Adrián Larumbe @ 2025-10-01 15:19 ` Boris Brezillon 0 siblings, 0 replies; 20+ messages in thread From: Boris Brezillon @ 2025-10-01 15:19 UTC (permalink / raw) To: Adrián Larumbe Cc: linux-kernel, dri-devel, Steven Price, kernel, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On Wed, 1 Oct 2025 03:20:31 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > panfrost_job_* prefixed functions in panfrost_job.c deal with both > panfrost_job objects and also the more general JM (Job Manager) side of > the device itself. This is confusing. > > Reprefix functions that program the JM to panfrosot_jm_* instead. > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_device.c | 14 +++---- > drivers/gpu/drm/panfrost/panfrost_drv.c | 4 +- > drivers/gpu/drm/panfrost/panfrost_job.c | 48 +++++++++++----------- > drivers/gpu/drm/panfrost/panfrost_job.h | 16 ++++---- > 4 files changed, 41 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c > index f1d811a6de6c..c61b97af120c 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c > @@ -259,7 +259,7 @@ int panfrost_device_init(struct panfrost_device *pfdev) > if (err) > goto out_gpu; > > - err = panfrost_job_init(pfdev); > + err = panfrost_jm_init(pfdev); > if (err) > goto out_mmu; > > @@ -269,7 +269,7 @@ int panfrost_device_init(struct panfrost_device *pfdev) > > return 0; > out_job: > - panfrost_job_fini(pfdev); > + panfrost_jm_fini(pfdev); > out_mmu: > panfrost_mmu_fini(pfdev); > out_gpu: > @@ -290,7 +290,7 @@ int panfrost_device_init(struct panfrost_device *pfdev) > void panfrost_device_fini(struct panfrost_device *pfdev) > { > panfrost_perfcnt_fini(pfdev); > - panfrost_job_fini(pfdev); > + panfrost_jm_fini(pfdev); > panfrost_mmu_fini(pfdev); > panfrost_gpu_fini(pfdev); > panfrost_devfreq_fini(pfdev); > @@ -407,9 +407,9 @@ void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int) > panfrost_gpu_power_on(pfdev); > panfrost_mmu_reset(pfdev); > > - panfrost_job_reset_interrupts(pfdev); > + panfrost_jm_reset_interrupts(pfdev); > if (enable_job_int) > - panfrost_job_enable_interrupts(pfdev); > + panfrost_jm_enable_interrupts(pfdev); > } > > static int panfrost_device_runtime_resume(struct device *dev) > @@ -451,11 +451,11 @@ static int panfrost_device_runtime_suspend(struct device *dev) > { > struct panfrost_device *pfdev = dev_get_drvdata(dev); > > - if (!panfrost_job_is_idle(pfdev)) > + if (!panfrost_jm_is_idle(pfdev)) > return -EBUSY; > > panfrost_devfreq_suspend(pfdev); > - panfrost_job_suspend_irq(pfdev); > + panfrost_jm_suspend_irq(pfdev); > panfrost_mmu_suspend_irq(pfdev); > panfrost_gpu_suspend_irq(pfdev); > panfrost_gpu_power_off(pfdev); > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 2b57f6813714..3b79ebbccdf5 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -606,7 +606,7 @@ panfrost_open(struct drm_device *dev, struct drm_file *file) > goto err_free; > } > > - ret = panfrost_job_open(file); > + ret = panfrost_jm_open(file); > if (ret) > goto err_job; > > @@ -625,7 +625,7 @@ panfrost_postclose(struct drm_device *dev, struct drm_file *file) > struct panfrost_file_priv *panfrost_priv = file->driver_priv; > > panfrost_perfcnt_close(file); > - panfrost_job_close(file); > + panfrost_jm_close(file); > > panfrost_mmu_ctx_put(panfrost_priv->mmu); > kfree(panfrost_priv); > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c > index 3ae984f6290f..4a213db9962d 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > @@ -426,18 +426,18 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) > return fence; > } > > -void panfrost_job_reset_interrupts(struct panfrost_device *pfdev) > +void panfrost_jm_reset_interrupts(struct panfrost_device *pfdev) > { > job_write(pfdev, JOB_INT_CLEAR, ALL_JS_INT_MASK); > } > > -void panfrost_job_enable_interrupts(struct panfrost_device *pfdev) > +void panfrost_jm_enable_interrupts(struct panfrost_device *pfdev) > { > clear_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended); > job_write(pfdev, JOB_INT_MASK, ALL_JS_INT_MASK); > } > > -void panfrost_job_suspend_irq(struct panfrost_device *pfdev) > +void panfrost_jm_suspend_irq(struct panfrost_device *pfdev) > { > set_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended); > > @@ -499,8 +499,8 @@ static void panfrost_job_handle_err(struct panfrost_device *pfdev, > } > } > > -static void panfrost_job_handle_done(struct panfrost_device *pfdev, > - struct panfrost_job *job) > +static void panfrost_jm_handle_done(struct panfrost_device *pfdev, > + struct panfrost_job *job) > { > /* Set ->jc to 0 to avoid re-submitting an already finished job (can > * happen when we receive the DONE interrupt while doing a GPU reset). > @@ -513,7 +513,7 @@ static void panfrost_job_handle_done(struct panfrost_device *pfdev, > pm_runtime_put_autosuspend(pfdev->base.dev); > } > > -static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) > +static void panfrost_jm_handle_irq(struct panfrost_device *pfdev, u32 status) > { > struct panfrost_job *done[NUM_JOB_SLOTS][2] = {}; > struct panfrost_job *failed[NUM_JOB_SLOTS] = {}; > @@ -588,7 +588,7 @@ static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) > } > > for (i = 0; i < ARRAY_SIZE(done[0]) && done[j][i]; i++) > - panfrost_job_handle_done(pfdev, done[j][i]); > + panfrost_jm_handle_done(pfdev, done[j][i]); > } > > /* And finally we requeue jobs that were waiting in the second slot > @@ -606,7 +606,7 @@ static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) > struct panfrost_job *canceled = panfrost_dequeue_job(pfdev, j); > > dma_fence_set_error(canceled->done_fence, -ECANCELED); > - panfrost_job_handle_done(pfdev, canceled); > + panfrost_jm_handle_done(pfdev, canceled); > } else if (!atomic_read(&pfdev->reset.pending)) { > /* Requeue the job we removed if no reset is pending */ > job_write(pfdev, JS_COMMAND_NEXT(j), JS_COMMAND_START); > @@ -614,7 +614,7 @@ static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) > } > } > > -static void panfrost_job_handle_irqs(struct panfrost_device *pfdev) > +static void panfrost_jm_handle_irqs(struct panfrost_device *pfdev) > { > u32 status = job_read(pfdev, JOB_INT_RAWSTAT); > > @@ -622,7 +622,7 @@ static void panfrost_job_handle_irqs(struct panfrost_device *pfdev) > pm_runtime_mark_last_busy(pfdev->base.dev); > > spin_lock(&pfdev->js->job_lock); > - panfrost_job_handle_irq(pfdev, status); > + panfrost_jm_handle_irq(pfdev, status); > spin_unlock(&pfdev->js->job_lock); > status = job_read(pfdev, JOB_INT_RAWSTAT); > } > @@ -703,7 +703,7 @@ panfrost_reset(struct panfrost_device *pfdev, > dev_err(pfdev->base.dev, "Soft-stop failed\n"); > > /* Handle the remaining interrupts before we reset. */ > - panfrost_job_handle_irqs(pfdev); > + panfrost_jm_handle_irqs(pfdev); > > /* Remaining interrupts have been handled, but we might still have > * stuck jobs. Let's make sure the PM counters stay balanced by > @@ -748,7 +748,7 @@ panfrost_reset(struct panfrost_device *pfdev, > drm_sched_start(&pfdev->js->queue[i].sched, 0); > > /* Re-enable job interrupts now that everything has been restarted. */ > - panfrost_job_enable_interrupts(pfdev); > + panfrost_jm_enable_interrupts(pfdev); > > dma_fence_end_signalling(cookie); > } > @@ -813,11 +813,11 @@ static const struct drm_sched_backend_ops panfrost_sched_ops = { > .free_job = panfrost_job_free > }; > > -static irqreturn_t panfrost_job_irq_handler_thread(int irq, void *data) > +static irqreturn_t panfrost_jm_irq_handler_thread(int irq, void *data) > { > struct panfrost_device *pfdev = data; > > - panfrost_job_handle_irqs(pfdev); > + panfrost_jm_handle_irqs(pfdev); > > /* Enable interrupts only if we're not about to get suspended */ > if (!test_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended)) > @@ -826,7 +826,7 @@ static irqreturn_t panfrost_job_irq_handler_thread(int irq, void *data) > return IRQ_HANDLED; > } > > -static irqreturn_t panfrost_job_irq_handler(int irq, void *data) > +static irqreturn_t panfrost_jm_irq_handler(int irq, void *data) > { > struct panfrost_device *pfdev = data; > u32 status; > @@ -842,7 +842,7 @@ static irqreturn_t panfrost_job_irq_handler(int irq, void *data) > return IRQ_WAKE_THREAD; > } > > -int panfrost_job_init(struct panfrost_device *pfdev) > +int panfrost_jm_init(struct panfrost_device *pfdev) > { > struct drm_sched_init_args args = { > .ops = &panfrost_sched_ops, > @@ -875,8 +875,8 @@ int panfrost_job_init(struct panfrost_device *pfdev) > return js->irq; > > ret = devm_request_threaded_irq(pfdev->base.dev, js->irq, > - panfrost_job_irq_handler, > - panfrost_job_irq_handler_thread, > + panfrost_jm_irq_handler, > + panfrost_jm_irq_handler_thread, > IRQF_SHARED, KBUILD_MODNAME "-job", > pfdev); > if (ret) { > @@ -899,8 +899,8 @@ int panfrost_job_init(struct panfrost_device *pfdev) > } > } > > - panfrost_job_reset_interrupts(pfdev); > - panfrost_job_enable_interrupts(pfdev); > + panfrost_jm_reset_interrupts(pfdev); > + panfrost_jm_enable_interrupts(pfdev); > > return 0; > > @@ -912,7 +912,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) > return ret; > } > > -void panfrost_job_fini(struct panfrost_device *pfdev) > +void panfrost_jm_fini(struct panfrost_device *pfdev) > { > struct panfrost_job_slot *js = pfdev->js; > int j; > @@ -927,7 +927,7 @@ void panfrost_job_fini(struct panfrost_device *pfdev) > destroy_workqueue(pfdev->reset.wq); > } > > -int panfrost_job_open(struct drm_file *file) > +int panfrost_jm_open(struct drm_file *file) > { > struct panfrost_file_priv *panfrost_priv = file->driver_priv; > int ret; > @@ -949,7 +949,7 @@ int panfrost_job_open(struct drm_file *file) > return 0; > } > > -void panfrost_job_close(struct drm_file *file) > +void panfrost_jm_close(struct drm_file *file) > { > struct panfrost_file_priv *panfrost_priv = file->driver_priv; > struct panfrost_jm_ctx *jm_ctx; > @@ -961,7 +961,7 @@ void panfrost_job_close(struct drm_file *file) > xa_destroy(&panfrost_priv->jm_ctxs); > } > > -int panfrost_job_is_idle(struct panfrost_device *pfdev) > +int panfrost_jm_is_idle(struct panfrost_device *pfdev) > { > struct panfrost_job_slot *js = pfdev->js; > int i; > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.h b/drivers/gpu/drm/panfrost/panfrost_job.h > index 30eda74e3c34..da96c674d62b 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.h > +++ b/drivers/gpu/drm/panfrost/panfrost_job.h > @@ -60,16 +60,16 @@ void panfrost_jm_ctx_put(struct panfrost_jm_ctx *jm_ctx); > struct panfrost_jm_ctx *panfrost_jm_ctx_get(struct panfrost_jm_ctx *jm_ctx); > struct panfrost_jm_ctx *panfrost_jm_ctx_from_handle(struct drm_file *file, u32 handle); > > -int panfrost_job_init(struct panfrost_device *pfdev); > -void panfrost_job_fini(struct panfrost_device *pfdev); > -int panfrost_job_open(struct drm_file *file); > -void panfrost_job_close(struct drm_file *file); > +int panfrost_jm_init(struct panfrost_device *pfdev); > +void panfrost_jm_fini(struct panfrost_device *pfdev); > +int panfrost_jm_open(struct drm_file *file); > +void panfrost_jm_close(struct drm_file *file); > +void panfrost_jm_reset_interrupts(struct panfrost_device *pfdev); > +void panfrost_jm_enable_interrupts(struct panfrost_device *pfdev); > +void panfrost_jm_suspend_irq(struct panfrost_device *pfdev); > +int panfrost_jm_is_idle(struct panfrost_device *pfdev); > int panfrost_job_get_slot(struct panfrost_job *job); > int panfrost_job_push(struct panfrost_job *job); > void panfrost_job_put(struct panfrost_job *job); > -void panfrost_job_reset_interrupts(struct panfrost_device *pfdev); > -void panfrost_job_enable_interrupts(struct panfrost_device *pfdev); > -void panfrost_job_suspend_irq(struct panfrost_device *pfdev); > -int panfrost_job_is_idle(struct panfrost_device *pfdev); > > #endif ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2025-10-07 7:51 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-01 2:20 [PATCH v4 00/10] Some Panfrost fixes and improvements Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 01/10] drm/panfrost: Replace DRM driver allocation method with newer one Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 02/10] drm/panfrost: Handle inexistent GPU during probe Adrián Larumbe 2025-10-01 10:51 ` Boris Brezillon 2025-10-01 2:20 ` [PATCH v4 03/10] drm/panfrost: Handle job HW submit errors Adrián Larumbe 2025-10-06 16:07 ` Steven Price 2025-10-07 0:34 ` Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 04/10] drm/panfrost: Handle error when allocating AS number Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 05/10] drm/panfrost: Handle page mapping failure Adrián Larumbe 2025-10-01 10:58 ` Boris Brezillon 2025-10-07 1:04 ` Adrián Larumbe 2025-10-07 7:51 ` Boris Brezillon 2025-10-01 2:20 ` [PATCH v4 06/10] drm/panfrost: Don't rework job IRQ enable mask in the enable path Adrián Larumbe 2025-10-01 11:00 ` Boris Brezillon 2025-10-01 2:20 ` [PATCH v4 07/10] drm/panfrost: Make re-enabling job interrupts at device reset optional Adrián Larumbe 2025-10-01 11:02 ` Boris Brezillon 2025-10-01 2:20 ` [PATCH v4 08/10] drm/panfrost: Add forward declaration and types header Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 09/10] drm/panfrost: Remove unused device property Adrián Larumbe 2025-10-01 2:20 ` [PATCH v4 10/10] drm/panfrost: Rename panfrost_job functions to reflect real role Adrián Larumbe 2025-10-01 15:19 ` Boris Brezillon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).