* [PATCH RFC v2 01/14] iommu: arm-smmu-qcom: Configure lpac device with split address space
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 14:21 ` sashiko-bot
2026-10-29 13:52 ` [PATCH RFC v2 02/14] drm/msm: add support for lpac_domain in msm_mmu Anna Maniscalco
` (12 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
We want the context bank used by LPAC (Low Priority Async Compute) to
use split address space just like GFX in order to be able to handle
userspace submissions with per context page tables.
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index edd41b5a3b6a..48a590aaeb5e 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -35,6 +35,8 @@
static const struct of_device_id qcom_smmu_actlr_client_of_match[] = {
{ .compatible = "qcom,adreno",
.data = (const void *) (PREFETCH_DEEP | CPRE | CMTLB) },
+ { .compatible = "qcom,lpac",
+ .data = (const void *) (PREFETCH_DEEP | CPRE | CMTLB) },
{ .compatible = "qcom,adreno-gmu",
.data = (const void *) (PREFETCH_DEEP | CPRE | CMTLB) },
{ .compatible = "qcom,adreno-smmu",
@@ -217,6 +219,7 @@ static void qcom_adreno_smmu_set_prr_addr(const void *cookie, phys_addr_t page_a
}
#define QCOM_ADRENO_SMMU_GPU_SID 0
+#define QCOM_ADRENO_SMMU_LPAC_SID 1
static bool qcom_adreno_smmu_is_gpu_device(struct device *dev)
{
@@ -230,7 +233,8 @@ static bool qcom_adreno_smmu_is_gpu_device(struct device *dev)
for (i = 0; i < fwspec->num_ids; i++) {
u16 sid = FIELD_GET(ARM_SMMU_SMR_ID, fwspec->ids[i]);
- if (sid == QCOM_ADRENO_SMMU_GPU_SID)
+ if (sid == QCOM_ADRENO_SMMU_GPU_SID ||
+ sid == QCOM_ADRENO_SMMU_LPAC_SID)
return true;
}
@@ -305,9 +309,9 @@ static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_doma
*/
if (qcom_adreno_smmu_is_gpu_device(dev)) {
start = 0;
- count = 1;
+ count = 2;
} else {
- start = 1;
+ start = 2;
count = smmu->num_context_banks;
}
@@ -394,6 +398,7 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = {
{ .compatible = "qcom,adreno" },
+ { .compatible = "qcom,lpac" },
{ .compatible = "qcom,adreno-gmu" },
{ .compatible = "qcom,glymur-mdss" },
{ .compatible = "qcom,mdp4" },
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RFC v2 01/14] iommu: arm-smmu-qcom: Configure lpac device with split address space
2026-10-29 13:52 ` [PATCH RFC v2 01/14] iommu: arm-smmu-qcom: Configure lpac device with split address space Anna Maniscalco
@ 2026-10-29 14:21 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-10-29 14:21 UTC (permalink / raw)
To: Anna Maniscalco; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] iommu/arm-smmu-qcom: qcom_adreno_smmu_alloc_context_bank() shifts GMU to cb 2
- [Medium] iommu/arm-smmu-qcom: undocumented qcom,lpac DT compatible breaks DT ABI
--
commit 89b03a5b08cd9912b4e1abfb59372ce6c00d8186
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>
iommu: arm-smmu-qcom: Configure lpac device with split address space
This commit configures the LPAC device to use a split address space by
matching the "qcom,lpac" compatible string, recognizing stream ID 1 in
qcom_adreno_smmu_is_gpu_device(), and reserving context banks 0 and 1 for
GPU and LPAC devices.
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index edd41b5a3b6ac..48a590aaeb5eb 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -35,6 +35,8 @@
> static const struct of_device_id qcom_smmu_actlr_client_of_match[] = {
> { .compatible = "qcom,adreno",
> .data = (const void *) (PREFETCH_DEEP | CPRE | CMTLB) },
> + { .compatible = "qcom,lpac",
> + .data = (const void *) (PREFETCH_DEEP | CPRE | CMTLB) },
[Severity: Medium]
Does adding "qcom,lpac" here and in qcom_smmu_client_of_match[] introduce
an undocumented Device Tree compatible string and break Device Tree
backwards compatibility?
Neither this commit nor the follow-up commits in this series document
"qcom,lpac" in Documentation/devicetree/bindings/, and
Documentation/devicetree/bindings/display/msm/gpu.yaml sets
additionalProperties: false:
Documentation/devicetree/bindings/display/msm/gpu.yaml:
required:
- compatible
- reg
- interrupts
additionalProperties: false
which causes dtbs_check validation to fail when an lpac child node is added
under the GPU node in "arm64: dts: qcom: sm8650: move smmu sid 1 to new
lpac device".
Furthermore, existing upstream Device Trees (such as sm8350.dtsi,
sc8280xp.dtsi, sm8450.dtsi, sm8550.dtsi, sm8650.dtsi, hamoa.dtsi,
kodiak.dtsi, lemans.dtsi, and monaco.dtsi) already assign Stream ID 1
directly to the main "qcom,adreno" GPU node:
arch/arm64/boot/dts/qcom/sm8650.dtsi:
iommus = <&adreno_smmu 0 0x0>,
<&adreno_smmu 1 0x0>;
When booting with an existing sm8650 DTB that lacks the new "qcom,lpac"
child node, gpu->lpac_pdev is NULL and context bank 1 is never allocated,
yet the later commits "firmware: qcom: scm: Configure LPAC aperture",
"drm/msm: initialize LPAC ring", and "drm/msm: enable LPAC on gen70900"
gate LPAC hardware initialization on ADRENO_QUIRK_LPAC rather than
gpu->lpac_pdev.
[ ... ]
> @@ -305,9 +309,9 @@ static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_doma
> */
> if (qcom_adreno_smmu_is_gpu_device(dev)) {
> start = 0;
> - count = 1;
> + count = 2;
> } else {
> - start = 1;
> + start = 2;
> count = smmu->num_context_banks;
> }
[Severity: High]
Won't unconditionally changing start from 1 to 2 in the else branch of
qcom_adreno_smmu_alloc_context_bank() shift the Adreno GMU ("qcom,adreno-gmu",
SID 5) from context bank 1 to context bank 2 across all Adreno platforms,
and break non-GPU context bank allocation on qcom,sdm630-smmu-v2?
During Adreno GMU initialization on Adreno 6xx/7xx/8xx SoCs, the call chain:
a6xx_gpu_init() ->
a6xx_gmu_init() ->
a6xx_gmu_memory_probe() ->
msm_iommu_new(gmu->dev, NULL, 0) ->
iommu_attach_device() ->
arm_smmu_attach_dev() ->
arm_smmu_init_domain_context() ->
qcom_adreno_smmu_alloc_context_bank()
evaluates gmu->dev (SID 5), which takes the else branch (both here and after
the later commit "iommu: arm-smmu-qcom: Fixed mapping between sid and cb for
gpu and lpac") and allocates cbndx 2 instead of cbndx 1, leaving cbndx 1
unused on all SoCs or DTBs without a separate "qcom,lpac" device.
As noted in commit afc95681c30689 ("iommu/arm-smmu-qcom: Add missing GMU
entry to match table"), firmware on platforms such as sc8280xp expects
context bank 1 to be assigned to the GMU; assigning context bank 2 causes
the SMMU to wedge and the GPU to hang without memory access when a GPU
translation fault occurs.
Additionally, on qcom,sdm630-smmu-v2, qcom_adreno_smmuv2_cfg_probe() limits
smmu->num_context_banks to 2:
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c:qcom_adreno_smmuv2_cfg_probe() {
/* TZ protects several last context banks, hide them from Linux */
if (of_device_is_compatible(smmu->dev->of_node, "qcom,sdm630-smmu-v2") &&
smmu->num_context_banks == 5)
smmu->num_context_banks = 2;
}
With start = 2 and count = 2, __arm_smmu_alloc_bitmap(smmu->context_map, 2, 2)
has an empty allocation range and will unconditionally return -ENOSPC for
any non-GPU device.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850961@gmail.com?part=1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RFC v2 02/14] drm/msm: add support for lpac_domain in msm_mmu
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
2026-10-29 13:52 ` [PATCH RFC v2 01/14] iommu: arm-smmu-qcom: Configure lpac device with split address space Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 14:12 ` sashiko-bot
2026-10-29 13:52 ` [PATCH RFC v2 03/14] drm/msm: look for lpac from dts Anna Maniscalco
` (11 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
LPAC (Low Priority Async Compute) needs to use a separate context bank
in order to be able to update the page table independetly.
Add an iommu_domain for LPAC and keep it in sync with the main domain.
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +-
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
drivers/gpu/drm/msm/msm_gpu.h | 1 +
drivers/gpu/drm/msm/msm_iommu.c | 170 ++++++++++++++++++++++++++++----
drivers/gpu/drm/msm/msm_mmu.h | 7 +-
5 files changed, 160 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 2e5d7b53a0c3..a91d9b03ca88 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -1569,7 +1569,7 @@ static int a6xx_gmu_memory_probe(struct drm_device *drm, struct a6xx_gmu *gmu)
{
struct msm_mmu *mmu;
- mmu = msm_iommu_new(gmu->dev, 0);
+ mmu = msm_iommu_new(gmu->dev, NULL, 0);
if (IS_ERR(mmu))
return PTR_ERR(mmu);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index c62c45bb0ddb..f3fc7032fadc 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -201,7 +201,7 @@ adreno_iommu_create_vm(struct msm_gpu *gpu,
struct drm_gpuvm *vm;
u64 start, size;
- mmu = msm_iommu_gpu_new(&pdev->dev, gpu, quirks);
+ mmu = msm_iommu_gpu_new(&pdev->dev, NULL, gpu, quirks);
if (IS_ERR(mmu))
return ERR_CAST(mmu);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 6c83b8cbbb90..e2291dfe3ca8 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -175,6 +175,7 @@ struct msm_gpu {
const struct msm_gpu_funcs *funcs;
struct adreno_smmu_priv adreno_smmu;
+ struct adreno_smmu_priv lpac_adreno_smmu;
struct msm_ringbuffer *rb[MSM_GPU_MAX_RINGS];
int nr_rings;
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index 058c71c82cf5..0e770ec91a83 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -13,7 +13,7 @@
struct msm_iommu {
struct msm_mmu base;
- struct iommu_domain *domain;
+ struct iommu_domain *domain, *lpac_domain;
struct mutex init_lock; /* protects pagetables counter and prr_page */
int pagetables;
@@ -127,6 +127,8 @@ static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova,
}
iommu_flush_iotlb_all(to_msm_iommu(pagetable->parent)->domain);
+ if (to_msm_iommu(pagetable->parent)->lpac_domain)
+ iommu_flush_iotlb_all(to_msm_iommu(pagetable->parent)->lpac_domain);
return ret;
}
@@ -224,6 +226,10 @@ static void msm_iommu_pagetable_destroy(struct msm_mmu *mmu)
struct msm_iommu *iommu = to_msm_iommu(pagetable->parent);
struct adreno_smmu_priv *adreno_smmu =
dev_get_drvdata(pagetable->parent->dev);
+ struct adreno_smmu_priv *lpac_adreno_smmu = NULL;
+
+ if (pagetable->parent->lpac_dev)
+ lpac_adreno_smmu = dev_get_drvdata(pagetable->parent->lpac_dev);
/*
* If this is the last attached pagetable for the parent,
@@ -232,9 +238,13 @@ static void msm_iommu_pagetable_destroy(struct msm_mmu *mmu)
mutex_lock(&iommu->init_lock);
if (--iommu->pagetables == 0) {
adreno_smmu->set_ttbr0_cfg(adreno_smmu->cookie, NULL);
+ if (lpac_adreno_smmu)
+ lpac_adreno_smmu->set_ttbr0_cfg(lpac_adreno_smmu->cookie, NULL);
if (adreno_smmu->set_prr_bit) {
adreno_smmu->set_prr_bit(adreno_smmu->cookie, false);
+ if (lpac_adreno_smmu && lpac_adreno_smmu->set_prr_bit)
+ lpac_adreno_smmu->set_prr_bit(lpac_adreno_smmu->cookie, false);
__free_page(iommu->prr_page);
iommu->prr_page = NULL;
}
@@ -450,14 +460,22 @@ static void msm_iommu_tlb_flush_all(void *cookie)
struct msm_iommu_pagetable *pagetable = cookie;
struct adreno_smmu_priv *adreno_smmu;
- if (!pm_runtime_get_if_in_use(pagetable->iommu_dev))
- return;
+ if (pm_runtime_get_if_in_use(pagetable->iommu_dev)) {
+ adreno_smmu = dev_get_drvdata(pagetable->parent->dev);
+
+ pagetable->tlb->tlb_flush_all((void *)adreno_smmu->cookie);
+
+ pm_runtime_put_autosuspend(pagetable->iommu_dev);
+ }
- adreno_smmu = dev_get_drvdata(pagetable->parent->dev);
+ if (pagetable->parent->lpac_dev &&
+ pm_runtime_get_if_in_use(pagetable->parent->lpac_dev)) {
+ adreno_smmu = dev_get_drvdata(pagetable->parent->lpac_dev);
- pagetable->tlb->tlb_flush_all((void *)adreno_smmu->cookie);
+ pagetable->tlb->tlb_flush_all((void *)adreno_smmu->cookie);
- pm_runtime_put_autosuspend(pagetable->iommu_dev);
+ pm_runtime_put_autosuspend(pagetable->parent->lpac_dev);
+ }
}
static void msm_iommu_tlb_flush_walk(unsigned long iova, size_t size,
@@ -466,14 +484,23 @@ static void msm_iommu_tlb_flush_walk(unsigned long iova, size_t size,
struct msm_iommu_pagetable *pagetable = cookie;
struct adreno_smmu_priv *adreno_smmu;
- if (!pm_runtime_get_if_in_use(pagetable->iommu_dev))
- return;
- adreno_smmu = dev_get_drvdata(pagetable->parent->dev);
+ if (pm_runtime_get_if_in_use(pagetable->iommu_dev)) {
+ adreno_smmu = dev_get_drvdata(pagetable->parent->dev);
+
+ pagetable->tlb->tlb_flush_walk(iova, size, granule, (void *)adreno_smmu->cookie);
- pagetable->tlb->tlb_flush_walk(iova, size, granule, (void *)adreno_smmu->cookie);
+ pm_runtime_put_autosuspend(pagetable->iommu_dev);
+ }
+
+ if (pagetable->parent->lpac_dev &&
+ pm_runtime_get_if_in_use(pagetable->parent->lpac_dev)) {
+ adreno_smmu = dev_get_drvdata(pagetable->parent->lpac_dev);
- pm_runtime_put_autosuspend(pagetable->iommu_dev);
+ pagetable->tlb->tlb_flush_walk(iova, size, granule, (void *)adreno_smmu->cookie);
+
+ pm_runtime_put_autosuspend(pagetable->parent->lpac_dev);
+ }
}
static void msm_iommu_tlb_add_page(struct iommu_iotlb_gather *gather,
@@ -504,12 +531,16 @@ static size_t get_tblsz(const struct io_pgtable_cfg *cfg)
struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent, bool kernel_managed)
{
struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(parent->dev);
+ struct adreno_smmu_priv *lpac_adreno_smmu = NULL;
struct msm_iommu *iommu = to_msm_iommu(parent);
struct msm_iommu_pagetable *pagetable;
const struct io_pgtable_cfg *ttbr1_cfg = NULL;
struct io_pgtable_cfg ttbr0_cfg;
int ret;
+ if (parent->lpac_dev)
+ lpac_adreno_smmu = dev_get_drvdata(parent->lpac_dev);
+
/* Get the pagetable configuration from the domain */
if (adreno_smmu->cookie)
ttbr1_cfg = adreno_smmu->get_ttbr1_cfg(adreno_smmu->cookie);
@@ -528,6 +559,8 @@ struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent, bool kernel_m
msm_mmu_init(&pagetable->base, parent->dev, &pagetable_funcs,
MSM_MMU_IOMMU_PAGETABLE);
+ pagetable->parent = parent;
+
/* Clone the TTBR1 cfg as starting point for TTBR0 cfg: */
ttbr0_cfg = *ttbr1_cfg;
@@ -588,6 +621,17 @@ struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent, bool kernel_m
return ERR_PTR(ret);
}
+ if (lpac_adreno_smmu) {
+ ret = lpac_adreno_smmu->set_ttbr0_cfg(lpac_adreno_smmu->cookie, &ttbr0_cfg);
+ if (ret) {
+ iommu->pagetables--;
+ mutex_unlock(&iommu->init_lock);
+ free_io_pgtable_ops(pagetable->pgtbl_ops);
+ kfree(pagetable);
+ return ERR_PTR(ret);
+ }
+ }
+
BUG_ON(iommu->prr_page);
if (adreno_smmu->set_prr_bit) {
/*
@@ -606,11 +650,16 @@ struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent, bool kernel_m
page_to_phys(iommu->prr_page));
adreno_smmu->set_prr_bit(adreno_smmu->cookie, true);
}
+
+ if (iommu->prr_page && lpac_adreno_smmu && lpac_adreno_smmu->set_prr_bit) {
+ lpac_adreno_smmu->set_prr_addr(lpac_adreno_smmu->cookie,
+ page_to_phys(iommu->prr_page));
+ lpac_adreno_smmu->set_prr_bit(lpac_adreno_smmu->cookie, true);
+ }
}
mutex_unlock(&iommu->init_lock);
/* Needed later for TLB flush */
- pagetable->parent = parent;
pagetable->tlb = ttbr1_cfg->tlb;
pagetable->pgsize_bitmap = ttbr0_cfg.pgsize_bitmap;
pagetable->ttbr = ttbr0_cfg.arm_lpae_s1_cfg.ttbr;
@@ -646,6 +695,26 @@ static int msm_gpu_fault_handler(struct iommu_domain *domain, struct device *dev
return 0;
}
+static int msm_lpac_fault_handler(struct iommu_domain *domain, struct device *dev,
+ unsigned long iova, int flags, void *arg)
+{
+ struct msm_iommu *iommu = arg;
+ struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(iommu->base.lpac_dev);
+ struct adreno_smmu_fault_info info, *ptr = NULL;
+
+ if (adreno_smmu->get_fault_info) {
+ adreno_smmu->get_fault_info(adreno_smmu->cookie, &info);
+ ptr = &info;
+ }
+
+ if (iommu->base.handler)
+ return iommu->base.handler(iommu->base.arg, iova, flags, ptr);
+
+ pr_warn_ratelimited("*** lpac fault: iova=%16lx, flags=%d\n", iova, flags);
+
+ return 0;
+}
+
static int msm_disp_fault_handler(struct iommu_domain *domain, struct device *dev,
unsigned long iova, int flags, void *arg)
{
@@ -660,9 +729,16 @@ static int msm_disp_fault_handler(struct iommu_domain *domain, struct device *de
static void msm_iommu_set_stall(struct msm_mmu *mmu, bool enable)
{
struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(mmu->dev);
+ struct adreno_smmu_priv *lpac_adreno_smmu = NULL;
+
+ if (mmu->lpac_dev)
+ lpac_adreno_smmu = dev_get_drvdata(mmu->lpac_dev);
if (adreno_smmu->set_stall)
adreno_smmu->set_stall(adreno_smmu->cookie, enable);
+
+ if (lpac_adreno_smmu && lpac_adreno_smmu->set_stall)
+ lpac_adreno_smmu->set_stall(lpac_adreno_smmu->cookie, enable);
}
static void msm_iommu_detach(struct msm_mmu *mmu)
@@ -670,6 +746,9 @@ static void msm_iommu_detach(struct msm_mmu *mmu)
struct msm_iommu *iommu = to_msm_iommu(mmu);
iommu_detach_device(iommu->domain, mmu->dev);
+
+ if (iommu->lpac_domain)
+ iommu_detach_device(iommu->lpac_domain, mmu->lpac_dev);
}
static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
@@ -689,6 +768,15 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
if (ret < 0)
return ret;
+ if (ret != len)
+ return -EINVAL;
+
+ if (iommu->lpac_domain) {
+ ret = iommu_map_sgtable(iommu->lpac_domain, iova, sgt, prot);
+ if (ret < 0)
+ return ret;
+ }
+
return (ret == len) ? 0 : -EINVAL;
}
@@ -701,6 +789,9 @@ static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t iova, size_t len)
iommu_unmap(iommu->domain, iova, len);
+ if (iommu->lpac_domain)
+ iommu_unmap(iommu->lpac_domain, iova, len);
+
return 0;
}
@@ -708,6 +799,8 @@ static void msm_iommu_destroy(struct msm_mmu *mmu)
{
struct msm_iommu *iommu = to_msm_iommu(mmu);
iommu_domain_free(iommu->domain);
+ if (iommu->lpac_domain)
+ iommu_domain_free(iommu->lpac_domain);
kmem_cache_destroy(iommu->pt_cache);
kfree(iommu);
}
@@ -720,9 +813,9 @@ static const struct msm_mmu_funcs funcs = {
.set_stall = msm_iommu_set_stall,
};
-struct msm_mmu *msm_iommu_new(struct device *dev, unsigned long quirks)
+struct msm_mmu *msm_iommu_new(struct device *dev, struct device *lpac_dev, unsigned long quirks)
{
- struct iommu_domain *domain;
+ struct iommu_domain *domain, *lpac_domain = NULL;
struct msm_iommu *iommu;
int ret;
@@ -753,6 +846,39 @@ struct msm_mmu *msm_iommu_new(struct device *dev, unsigned long quirks)
return ERR_PTR(ret);
}
+ if (lpac_dev) {
+ if (!device_iommu_mapped(lpac_dev)) {
+ iommu_detach_device(domain, dev);
+ iommu_domain_free(domain);
+ kfree(iommu);
+ return ERR_PTR(-ENODEV);
+ }
+
+ lpac_domain = iommu_paging_domain_alloc(lpac_dev);
+ if (IS_ERR(lpac_domain)) {
+ iommu_detach_device(domain, dev);
+ iommu_domain_free(domain);
+ kfree(iommu);
+ return ERR_CAST(lpac_domain);
+ }
+
+ iommu_set_pgtable_quirks(lpac_domain, quirks);
+
+ iommu->lpac_domain = lpac_domain;
+ iommu->base.lpac_dev = lpac_dev;
+ }
+
+ if (lpac_domain) {
+ ret = iommu_attach_device(lpac_domain, lpac_dev);
+ if (ret) {
+ iommu_detach_device(domain, dev);
+ iommu_domain_free(domain);
+ iommu_domain_free(lpac_domain);
+ kfree(iommu);
+ return ERR_PTR(ret);
+ }
+ }
+
return &iommu->base;
}
@@ -761,7 +887,7 @@ struct msm_mmu *msm_iommu_disp_new(struct device *dev, unsigned long quirks)
struct msm_iommu *iommu;
struct msm_mmu *mmu;
- mmu = msm_iommu_new(dev, quirks);
+ mmu = msm_iommu_new(dev, NULL, quirks);
if (IS_ERR(mmu))
return mmu;
@@ -771,13 +897,18 @@ struct msm_mmu *msm_iommu_disp_new(struct device *dev, unsigned long quirks)
return mmu;
}
-struct msm_mmu *msm_iommu_gpu_new(struct device *dev, struct msm_gpu *gpu, unsigned long quirks)
+struct msm_mmu *msm_iommu_gpu_new(struct device *dev, struct device *lpac_dev,
+ struct msm_gpu *gpu, unsigned long quirks)
{
struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(dev);
+ struct adreno_smmu_priv *lpac_adreno_smmu = NULL;
struct msm_iommu *iommu;
struct msm_mmu *mmu;
- mmu = msm_iommu_new(dev, quirks);
+ if (lpac_dev)
+ lpac_adreno_smmu = dev_get_drvdata(lpac_dev);
+
+ mmu = msm_iommu_new(dev, lpac_dev, quirks);
if (IS_ERR(mmu))
return mmu;
@@ -791,10 +922,15 @@ struct msm_mmu *msm_iommu_gpu_new(struct device *dev, struct msm_gpu *gpu, unsig
kmem_cache_create("msm-mmu-pt", tblsz, tblsz, 0, NULL);
}
iommu_set_fault_handler(iommu->domain, msm_gpu_fault_handler, iommu);
+ if (iommu->lpac_domain)
+ iommu_set_fault_handler(iommu->lpac_domain, msm_lpac_fault_handler, iommu);
/* Enable stall on iommu fault: */
if (adreno_smmu->set_stall)
adreno_smmu->set_stall(adreno_smmu->cookie, true);
+ if (lpac_adreno_smmu && lpac_adreno_smmu->set_stall)
+ lpac_adreno_smmu->set_stall(lpac_adreno_smmu->cookie, true);
+
return mmu;
}
diff --git a/drivers/gpu/drm/msm/msm_mmu.h b/drivers/gpu/drm/msm/msm_mmu.h
index 8915662fbd4d..1965b98d20db 100644
--- a/drivers/gpu/drm/msm/msm_mmu.h
+++ b/drivers/gpu/drm/msm/msm_mmu.h
@@ -52,7 +52,7 @@ struct msm_mmu_prealloc {
struct msm_mmu {
const struct msm_mmu_funcs *funcs;
- struct device *dev;
+ struct device *dev, *lpac_dev;
int (*handler)(void *arg, unsigned long iova, int flags, void *data);
void *arg;
enum msm_mmu_type type;
@@ -74,8 +74,9 @@ static inline void msm_mmu_init(struct msm_mmu *mmu, struct device *dev,
mmu->type = type;
}
-struct msm_mmu *msm_iommu_new(struct device *dev, unsigned long quirks);
-struct msm_mmu *msm_iommu_gpu_new(struct device *dev, struct msm_gpu *gpu, unsigned long quirks);
+struct msm_mmu *msm_iommu_new(struct device *dev, struct device *lpac_dev, unsigned long quirks);
+struct msm_mmu *msm_iommu_gpu_new(struct device *dev, struct device *lpac_dev,
+ struct msm_gpu *gpu, unsigned long quirks);
struct msm_mmu *msm_iommu_disp_new(struct device *dev, unsigned long quirks);
static inline void msm_mmu_set_fault_handler(struct msm_mmu *mmu, void *arg,
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RFC v2 02/14] drm/msm: add support for lpac_domain in msm_mmu
2026-10-29 13:52 ` [PATCH RFC v2 02/14] drm/msm: add support for lpac_domain in msm_mmu Anna Maniscalco
@ 2026-10-29 14:12 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-10-29 14:12 UTC (permalink / raw)
To: Anna Maniscalco; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] drm/msm: wrong device passed to runtime PM in msm_iommu_tlb_flush_all()
- [High] drm/msm: missing TTBR0 rollback and NULL tlb deref in msm_iommu_pagetable_create()
- [High] drm/msm: missing iommu->domain unmap on error in msm_iommu_map()
- [Medium] drm/msm: race on fault_coredump_done between gpu and lpac fault handlers
--
commit 77527d316e1a6de39276075ddde03245b58f889d
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>
drm/msm: add support for lpac_domain in msm_mmu
This commit adds support for a separate LPAC (Low Priority Async Compute)
iommu_domain in msm_mmu and keeps its mappings, TTBR0 configuration, fault
handling, and TLB invalidations synchronized with the main domain.
> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> index 058c71c82cf54..0e770ec91a83a 100644
> --- a/drivers/gpu/drm/msm/msm_iommu.c
> +++ b/drivers/gpu/drm/msm/msm_iommu.c
[ ... ]
> @@ -450,14 +460,22 @@ static void msm_iommu_tlb_flush_all(void *cookie)
> struct msm_iommu_pagetable *pagetable = cookie;
> struct adreno_smmu_priv *adreno_smmu;
>
> - if (!pm_runtime_get_if_in_use(pagetable->iommu_dev))
> - return;
> + if (pm_runtime_get_if_in_use(pagetable->iommu_dev)) {
> + adreno_smmu = dev_get_drvdata(pagetable->parent->dev);
> +
> + pagetable->tlb->tlb_flush_all((void *)adreno_smmu->cookie);
> +
> + pm_runtime_put_autosuspend(pagetable->iommu_dev);
> + }
>
> - adreno_smmu = dev_get_drvdata(pagetable->parent->dev);
> + if (pagetable->parent->lpac_dev &&
> + pm_runtime_get_if_in_use(pagetable->parent->lpac_dev)) {
> + adreno_smmu = dev_get_drvdata(pagetable->parent->lpac_dev);
>
> - pagetable->tlb->tlb_flush_all((void *)adreno_smmu->cookie);
> + pagetable->tlb->tlb_flush_all((void *)adreno_smmu->cookie);
>
> - pm_runtime_put_autosuspend(pagetable->iommu_dev);
> + pm_runtime_put_autosuspend(pagetable->parent->lpac_dev);
> + }
> }
>
> static void msm_iommu_tlb_flush_walk(unsigned long iova, size_t size,
> @@ -466,14 +484,23 @@ static void msm_iommu_tlb_flush_walk(unsigned long iova, size_t size,
> struct msm_iommu_pagetable *pagetable = cookie;
> struct adreno_smmu_priv *adreno_smmu;
>
> - if (!pm_runtime_get_if_in_use(pagetable->iommu_dev))
> - return;
>
> - adreno_smmu = dev_get_drvdata(pagetable->parent->dev);
> + if (pm_runtime_get_if_in_use(pagetable->iommu_dev)) {
> + adreno_smmu = dev_get_drvdata(pagetable->parent->dev);
> +
> + pagetable->tlb->tlb_flush_walk(iova, size, granule, (void *)adreno_smmu->cookie);
>
> - pagetable->tlb->tlb_flush_walk(iova, size, granule, (void *)adreno_smmu->cookie);
> + pm_runtime_put_autosuspend(pagetable->iommu_dev);
> + }
> +
> + if (pagetable->parent->lpac_dev &&
> + pm_runtime_get_if_in_use(pagetable->parent->lpac_dev)) {
> + adreno_smmu = dev_get_drvdata(pagetable->parent->lpac_dev);
>
> - pm_runtime_put_autosuspend(pagetable->iommu_dev);
> + pagetable->tlb->tlb_flush_walk(iova, size, granule, (void *)adreno_smmu->cookie);
> +
> + pm_runtime_put_autosuspend(pagetable->parent->lpac_dev);
> + }
> }
[Severity: High]
Should msm_iommu_tlb_flush_all() and msm_iommu_tlb_flush_walk() keep the
runtime PM reference on pagetable->iommu_dev across both context bank flushes
instead of calling pm_runtime_get_if_in_use() and
pm_runtime_put_autosuspend() on pagetable->parent->lpac_dev?
In msm_iommu_pagetable_create(), pagetable->iommu_dev is initialized from
ttbr1_cfg->iommu_dev (the SMMU controller device), whereas
pagetable->parent->dev and pagetable->parent->lpac_dev are the GPU and LPAC
client devices. Both adreno_smmu->cookie and lpac_adreno_smmu->cookie access
context banks on that SMMU controller device.
When runtime PM is disabled on pagetable->parent->lpac_dev,
pm_runtime_get_if_in_use(pagetable->parent->lpac_dev) returns -EINVAL, which
evaluates as true in the if condition without incrementing usage_count. After
pm_runtime_put_autosuspend(pagetable->iommu_dev) drops the SMMU reference at
the end of the primary flush block, the LPAC block can access unclocked SMMU
registers while pagetable->iommu_dev is suspended and then underflow the
runtime PM usage count on pagetable->parent->lpac_dev. Conversely, if runtime
PM is enabled on lpac_dev with a zero usage count while pagetable->iommu_dev
is active, pm_runtime_get_if_in_use(pagetable->parent->lpac_dev) returns 0
and skips the LPAC TLB flush.
[ ... ]
> @@ -588,6 +621,17 @@ struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent, bool kernel_m
> return ERR_PTR(ret);
> }
>
> + if (lpac_adreno_smmu) {
> + ret = lpac_adreno_smmu->set_ttbr0_cfg(lpac_adreno_smmu->cookie, &ttbr0_cfg);
> + if (ret) {
> + iommu->pagetables--;
> + mutex_unlock(&iommu->init_lock);
> + free_io_pgtable_ops(pagetable->pgtbl_ops);
> + kfree(pagetable);
> + return ERR_PTR(ret);
> + }
> + }
> +
> BUG_ON(iommu->prr_page);
> if (adreno_smmu->set_prr_bit) {
> /*
> @@ -606,11 +650,16 @@ struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent, bool kernel_m
> page_to_phys(iommu->prr_page));
> adreno_smmu->set_prr_bit(adreno_smmu->cookie, true);
> }
> +
> + if (iommu->prr_page && lpac_adreno_smmu && lpac_adreno_smmu->set_prr_bit) {
> + lpac_adreno_smmu->set_prr_addr(lpac_adreno_smmu->cookie,
> + page_to_phys(iommu->prr_page));
> + lpac_adreno_smmu->set_prr_bit(lpac_adreno_smmu->cookie, true);
> + }
> }
> mutex_unlock(&iommu->init_lock);
>
> /* Needed later for TLB flush */
> - pagetable->parent = parent;
> pagetable->tlb = ttbr1_cfg->tlb;
> pagetable->pgsize_bitmap = ttbr0_cfg.pgsize_bitmap;
> pagetable->ttbr = ttbr0_cfg.arm_lpae_s1_cfg.ttbr;
[Severity: High]
What happens in msm_iommu_pagetable_create() if
lpac_adreno_smmu->set_ttbr0_cfg() fails after adreno_smmu->set_ttbr0_cfg()
has already succeeded?
Because the error path does not call
adreno_smmu->set_ttbr0_cfg(adreno_smmu->cookie, NULL) before freeing
pagetable->pgtbl_ops, the primary context bank's TTBR0 remains enabled and
pointed at the freed root page table physical address, and subsequent calls
to msm_iommu_pagetable_create() will fail in qcom_adreno_smmu_set_ttbr0_cfg()
because ARM_SMMU_TCR_EPD0 is already cleared.
In addition, while pagetable->parent = parent was moved before
alloc_io_pgtable_ops(), pagetable->tlb = ttbr1_cfg->tlb is still assigned
after the init_lock block:
drivers/gpu/drm/msm/msm_iommu.c:msm_iommu_pagetable_create() {
...
if (iommu->pagetables++ == 0) {
...
if (lpac_adreno_smmu) {
ret = lpac_adreno_smmu->set_ttbr0_cfg(...);
if (ret) {
...
free_io_pgtable_ops(pagetable->pgtbl_ops);
...
}
}
}
mutex_unlock(&iommu->init_lock);
/* Needed later for TLB flush */
pagetable->tlb = ttbr1_cfg->tlb;
}
When free_io_pgtable_ops() runs on either set_ttbr0_cfg() error path, it
calls io_pgtable_tlb_flush_all() -> msm_iommu_tlb_flush_all(), which
dereferences pagetable->tlb while it is still NULL. Should pagetable->tlb be
initialized before alloc_io_pgtable_ops(), and should the LPAC error path roll
back adreno_smmu->set_ttbr0_cfg(adreno_smmu->cookie, NULL)?
> @@ -646,6 +695,26 @@ static int msm_gpu_fault_handler(struct iommu_domain *domain, struct device *dev
> return 0;
> }
>
> +static int msm_lpac_fault_handler(struct iommu_domain *domain, struct device *dev,
> + unsigned long iova, int flags, void *arg)
> +{
> + struct msm_iommu *iommu = arg;
> + struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(iommu->base.lpac_dev);
> + struct adreno_smmu_fault_info info, *ptr = NULL;
> +
> + if (adreno_smmu->get_fault_info) {
> + adreno_smmu->get_fault_info(adreno_smmu->cookie, &info);
> + ptr = &info;
> + }
> +
> + if (iommu->base.handler)
> + return iommu->base.handler(iommu->base.arg, iova, flags, ptr);
> +
> + pr_warn_ratelimited("*** lpac fault: iova=%16lx, flags=%d\n", iova, flags);
> +
> + return 0;
> +}
[Severity: Medium]
Can msm_lpac_fault_handler() and msm_gpu_fault_handler() race on
adreno_gpu->fault_coredump_done when both context banks fault concurrently?
Because iommu->domain and iommu->lpac_domain use separate SMMU context banks
with separate threaded fault IRQs, both handlers can invoke
iommu->base.handler (a6xx_fault_handler() -> adreno_fault_handler()) at the
same time. In adreno_fault_handler(), do_devcoredump is evaluated before
gpu->lock is taken in msm_gpu_fault_crashstate_capture(), and
reinit_completion() and complete_all() on adreno_gpu->fault_coredump_done run
outside gpu->lock:
drivers/gpu/drm/msm/adreno/adreno_gpu.c:adreno_fault_handler() {
...
do_devcoredump = info && (info->fsr & ARM_SMMU_FSR_SS) &&
!gpu->crashstate && !READ_ONCE(priv->disable_devcoredump);
...
if (do_devcoredump) {
...
reinit_completion(&adreno_gpu->fault_coredump_done);
...
msm_gpu_fault_crashstate_capture(gpu, &fault_info);
complete_all(&adreno_gpu->fault_coredump_done);
}
}
If both context banks stall on a fault while gpu->crashstate is still NULL,
can one handler call reinit_completion() or complete_all() while the other
context bank is still stalled in the SMMU, causing concurrent GMU waits on
adreno_gpu->fault_coredump_done to wake prematurely or race on the
completion?
[ ... ]
> @@ -689,6 +768,15 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
> if (ret < 0)
> return ret;
>
> + if (ret != len)
> + return -EINVAL;
> +
> + if (iommu->lpac_domain) {
> + ret = iommu_map_sgtable(iommu->lpac_domain, iova, sgt, prot);
> + if (ret < 0)
> + return ret;
> + }
> +
> return (ret == len) ? 0 : -EINVAL;
> }
[Severity: High]
If mapping into iommu->lpac_domain fails or maps fewer than len bytes in
msm_iommu_map(), does this leave the [iova, iova + len) mapping leaked in
iommu->domain?
When msm_iommu_map() returns an error to msm_gem_vma_map(), msm_vma->mapped
remains false, so msm_gem_vma_unmap() will not be called to clean up the
primary domain mapping. Should msm_iommu_map() call
iommu_unmap(iommu->domain, iova, len) when the iommu->lpac_domain mapping
fails?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850961@gmail.com?part=2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RFC v2 03/14] drm/msm: look for lpac from dts
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
2026-10-29 13:52 ` [PATCH RFC v2 01/14] iommu: arm-smmu-qcom: Configure lpac device with split address space Anna Maniscalco
2026-10-29 13:52 ` [PATCH RFC v2 02/14] drm/msm: add support for lpac_domain in msm_mmu Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 14:09 ` sashiko-bot
2026-10-29 13:52 ` [PATCH RFC v2 04/14] arm64: dts: qcom: sm8650: move smmu sid 1 to new lpac device Anna Maniscalco
` (10 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
In order to associate the SID used by LPAC to it's own domain and
context bank we need a separate node on the DTS.
Add the code to look up that node on initialization.
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 10 +++++++++
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 36 ++++++++++++++++++++++++++----
drivers/gpu/drm/msm/msm_gpu.c | 6 +++++
drivers/gpu/drm/msm/msm_gpu.h | 1 +
4 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 7f20320ef66a..4d22223c3323 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -6,6 +6,8 @@
* Copyright (c) 2014,2017 The Linux Foundation. All rights reserved.
*/
+#include <linux/of_platform.h>
+
#include "adreno_gpu.h"
bool hang_debug = false;
@@ -271,6 +273,12 @@ static const struct component_ops a3xx_ops = {
static int adreno_probe(struct platform_device *pdev)
{
+ int ret = 0;
+
+ ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+ if (ret)
+ return ret;
+
if (of_device_is_compatible(pdev->dev.of_node, "amd,imageon") ||
msm_gpu_no_components())
return msm_gpu_probe(pdev, &a3xx_ops);
@@ -282,6 +290,8 @@ static void adreno_remove(struct platform_device *pdev)
{
struct msm_drm_private *priv = platform_get_drvdata(pdev);
+ of_platform_depopulate(&pdev->dev);
+
if (priv->kms_init)
component_del(&pdev->dev, &a3xx_ops);
else
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index f3fc7032fadc..1dedeb19946e 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -11,6 +11,8 @@
#include <linux/firmware/qcom/qcom_scm.h>
#include <linux/kernel.h>
#include <linux/of_reserved_mem.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
#include <linux/pm_opp.h>
#include <linux/slab.h>
#include <linux/soc/qcom/mdt_loader.h>
@@ -201,7 +203,9 @@ adreno_iommu_create_vm(struct msm_gpu *gpu,
struct drm_gpuvm *vm;
u64 start, size;
- mmu = msm_iommu_gpu_new(&pdev->dev, NULL, gpu, quirks);
+ mmu = msm_iommu_gpu_new(&pdev->dev,
+ gpu->lpac_pdev ? &gpu->lpac_pdev->dev : NULL,
+ gpu, quirks);
if (IS_ERR(mmu))
return ERR_CAST(mmu);
@@ -1187,6 +1191,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
const struct adreno_gpu_funcs *funcs, int nr_rings)
{
struct device *dev = &pdev->dev;
+ struct device_node *lpac_node;
struct adreno_platform_config *config = dev->platform_data;
struct msm_gpu_config adreno_gpu_config = { 0 };
struct msm_gpu *gpu = &adreno_gpu->base;
@@ -1200,6 +1205,18 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
gpu->allow_relocs = config->info->family < ADRENO_6XX_GEN1;
gpu->pdev = pdev;
+ lpac_node = of_get_compatible_child(pdev->dev.of_node, "qcom,lpac");
+ if (lpac_node)
+ gpu->lpac_pdev = of_find_device_by_node(lpac_node);
+ if (gpu->lpac_pdev) {
+ pm_runtime_enable(&gpu->lpac_pdev->dev);
+ if (!device_link_add(&pdev->dev, &gpu->lpac_pdev->dev,
+ DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_CONSUMER))
+ dev_err(&gpu->lpac_pdev->dev, "failed to link to lpac device\n");
+ }
+ if (lpac_node)
+ of_node_put(lpac_node);
+
/* Only handle the core clock when GMU is not in use (or is absent). */
if (adreno_has_gmu_wrapper(adreno_gpu) ||
adreno_has_rgmu(adreno_gpu) ||
@@ -1221,8 +1238,10 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
gpu_name = devm_kasprintf(dev, GFP_KERNEL, "%"ADRENO_CHIPID_FMT,
ADRENO_CHIPID_ARGS(config->chip_id));
- if (!gpu_name)
- return -ENOMEM;
+ if (!gpu_name) {
+ ret = -ENOMEM;
+ goto err_put_lpac_pdev;
+ }
adreno_gpu_config.ioname = "kgsl_3d0_reg_memory";
@@ -1230,7 +1249,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
ret = adreno_get_pwrlevels(dev, gpu);
if (ret)
- return ret;
+ goto err_put_lpac_pdev;
init_completion(&adreno_gpu->fault_coredump_done);
complete_all(&adreno_gpu->fault_coredump_done);
@@ -1241,6 +1260,10 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
return msm_gpu_init(drm, pdev, &adreno_gpu->base, &funcs->base,
gpu_name, &adreno_gpu_config);
+err_put_lpac_pdev:
+ if (gpu->lpac_pdev)
+ put_device(&gpu->lpac_pdev->dev);
+ return ret;
}
void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
@@ -1256,4 +1279,9 @@ void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
pm_runtime_disable(&priv->gpu_pdev->dev);
msm_gpu_cleanup(&adreno_gpu->base);
+
+ if (gpu->lpac_pdev) {
+ pm_runtime_disable(&gpu->lpac_pdev->dev);
+ put_device(&gpu->lpac_pdev->dev);
+ }
}
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 18ed00e5f143..20581be10ca7 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -963,6 +963,8 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
gpu->gpu_cx = NULL;
platform_set_drvdata(pdev, &gpu->adreno_smmu);
+ if (gpu->lpac_pdev)
+ platform_set_drvdata(gpu->lpac_pdev, &gpu->lpac_adreno_smmu);
msm_devfreq_init(gpu);
@@ -1032,6 +1034,8 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
msm_gem_kernel_put(gpu->memptrs_bo, gpu->vm);
platform_set_drvdata(pdev, NULL);
+ if (gpu->lpac_pdev)
+ platform_set_drvdata(gpu->lpac_pdev, NULL);
return ret;
}
@@ -1062,4 +1066,6 @@ void msm_gpu_cleanup(struct msm_gpu *gpu)
msm_perfcntr_cleanup(gpu);
platform_set_drvdata(gpu->pdev, NULL);
+ if (gpu->lpac_pdev)
+ platform_set_drvdata(gpu->lpac_pdev, NULL);
}
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index e2291dfe3ca8..271956e7f870 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -172,6 +172,7 @@ struct msm_gpu {
const char *name;
struct drm_device *dev;
struct platform_device *pdev;
+ struct platform_device *lpac_pdev;
const struct msm_gpu_funcs *funcs;
struct adreno_smmu_priv adreno_smmu;
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RFC v2 03/14] drm/msm: look for lpac from dts
2026-10-29 13:52 ` [PATCH RFC v2 03/14] drm/msm: look for lpac from dts Anna Maniscalco
@ 2026-10-29 14:09 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-10-29 14:09 UTC (permalink / raw)
To: Anna Maniscalco; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] drm/msm: adreno_gpu_init() links parent as consumer of child lpac_pdev
- [High] drm/msm: double put_device() on lpac_pdev when adreno_gpu_init() fails
- [High] drm/msm: adreno_remove() depopulates lpac_pdev before unbinding GPU
- [Medium] drm/msm: adreno_probe() leaks populated child devices on probe failure
--
commit 50cc2a02be0fe4cc4c62cff78b28bf0d3bb92f45
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>
drm/msm: look for lpac from dts
This commit populates child platform devices from the Adreno GPU device
tree node and looks up the "qcom,lpac" child device during GPU
initialization so its SMMU context bank and IOMMU domain can be wired up.
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 7f20320ef66af..4d22223c3323c 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
[ ... ]
> @@ -271,6 +273,12 @@ static const struct component_ops a3xx_ops = {
>
> static int adreno_probe(struct platform_device *pdev)
> {
> + int ret = 0;
> +
> + ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> + if (ret)
> + return ret;
> +
> if (of_device_is_compatible(pdev->dev.of_node, "amd,imageon") ||
> msm_gpu_no_components())
> return msm_gpu_probe(pdev, &a3xx_ops);
[Severity: Medium]
If msm_gpu_probe() or component_add() fails in adreno_probe() after
of_platform_populate() succeeds, does this leak the populated child
platform_device structures (such as gpu->lpac_pdev) because
of_platform_depopulate(&pdev->dev) is not called on the error path (or via
devm_of_platform_populate())?
> @@ -282,6 +290,8 @@ static void adreno_remove(struct platform_device *pdev)
> {
> struct msm_drm_private *priv = platform_get_drvdata(pdev);
>
> + of_platform_depopulate(&pdev->dev);
> +
> if (priv->kms_init)
> component_del(&pdev->dev, &a3xx_ops);
> else
[Severity: High]
In adreno_remove(), shouldn't of_platform_depopulate(&pdev->dev) happen
after component_del() / msm_gpu_remove() rather than before?
Calling of_platform_depopulate(&pdev->dev) first unregisters
gpu->lpac_pdev via platform_device_unregister() -> device_del() while the
GPU is still bound and iommu->lpac_domain is still attached to
&gpu->lpac_pdev->dev.
During device_del(&gpu->lpac_pdev->dev), the bus notifier runs:
iommu_release_device()
-> __iommu_group_remove_device()
-> __iommu_group_free_device()
which triggers WARN_ON(group->owner_cnt || group->domain !=
group->default_domain) and tears down the LPAC SMMU master state before
component_del() or msm_gpu_remove() runs adreno_unbind() ->
adreno_gpu_cleanup() -> msm_gpu_cleanup() to detach the IOMMU domain and
disable runtime PM on &gpu->lpac_pdev->dev.
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index f3fc7032fadcd..1dedeb19946ec 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
[ ... ]
> @@ -1200,6 +1205,18 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> gpu->allow_relocs = config->info->family < ADRENO_6XX_GEN1;
> gpu->pdev = pdev;
>
> + lpac_node = of_get_compatible_child(pdev->dev.of_node, "qcom,lpac");
> + if (lpac_node)
> + gpu->lpac_pdev = of_find_device_by_node(lpac_node);
> + if (gpu->lpac_pdev) {
> + pm_runtime_enable(&gpu->lpac_pdev->dev);
> + if (!device_link_add(&pdev->dev, &gpu->lpac_pdev->dev,
> + DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_CONSUMER))
> + dev_err(&gpu->lpac_pdev->dev, "failed to link to lpac device\n");
> + }
> + if (lpac_node)
> + of_node_put(lpac_node);
[Severity: High]
Will device_link_add(&pdev->dev, &gpu->lpac_pdev->dev, ...) always fail and
return NULL here?
In adreno_probe(), of_platform_populate() creates gpu->lpac_pdev with
&pdev->dev as its parent. In adreno_gpu_init(), device_link_add() is called
with &pdev->dev (the parent) as the consumer and &gpu->lpac_pdev->dev (its
child) as the supplier.
Inside drivers/base/core.c:device_link_add():
if (!device_pm_initialized(supplier)
|| (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
device_is_dependent(consumer, supplier))) {
link = NULL;
goto out;
}
Because consumer (&pdev->dev) is the parent of supplier
(&gpu->lpac_pdev->dev), device_is_dependent(consumer, supplier) returns 1
via device_is_ancestor(consumer, supplier), rejecting the link as a
circular dependency. In addition, gpu->lpac_pdev has no driver bound to it,
so passing DL_FLAG_AUTOREMOVE_CONSUMER without DL_FLAG_STATELESS is not
valid for a driverless device.
As a result, the device link is never created and &gpu->lpac_pdev->dev
remains in RPM_SUSPENDED with usage_count == 0, causing
msm_iommu_tlb_flush_all() and msm_iommu_tlb_flush_walk() in
drivers/gpu/drm/msm/msm_iommu.c:
if (pagetable->parent->lpac_dev &&
pm_runtime_get_if_in_use(pagetable->parent->lpac_dev)) {
adreno_smmu = dev_get_drvdata(pagetable->parent->lpac_dev);
pagetable->tlb->tlb_flush_all((void *)adreno_smmu->cookie);
pm_runtime_put_autosuspend(pagetable->parent->lpac_dev);
}
to skip all IOTLB invalidations on the LPAC SMMU context bank when buffers
are unmapped.
[ ... ]
> @@ -1241,6 +1260,10 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>
> return msm_gpu_init(drm, pdev, &adreno_gpu->base, &funcs->base,
> gpu_name, &adreno_gpu_config);
> +err_put_lpac_pdev:
> + if (gpu->lpac_pdev)
> + put_device(&gpu->lpac_pdev->dev);
> + return ret;
> }
>
> void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
> @@ -1256,4 +1279,9 @@ void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
> pm_runtime_disable(&priv->gpu_pdev->dev);
>
> msm_gpu_cleanup(&adreno_gpu->base);
> +
> + if (gpu->lpac_pdev) {
> + pm_runtime_disable(&gpu->lpac_pdev->dev);
> + put_device(&gpu->lpac_pdev->dev);
> + }
> }
[Severity: High]
When adreno_gpu_init() fails via err_put_lpac_pdev (when devm_kasprintf()
or adreno_get_pwrlevels() fails), doesn't this cause a double
put_device(&gpu->lpac_pdev->dev)?
The err_put_lpac_pdev path drops the reference on &gpu->lpac_pdev->dev
without clearing gpu->lpac_pdev (or disabling runtime PM), and the callers
of adreno_gpu_init() (such as a6xx_gpu_init()) then call their destroy
function on failure:
drivers/gpu/drm/msm/adreno/a6xx_gpu.c:a6xx_gpu_init() {
...
ret = adreno_gpu_init(dev, pdev, adreno_gpu, info->funcs, nr_rings);
if (ret) {
a6xx_destroy(&(a6xx_gpu->base.base));
return ERR_PTR(ret);
}
...
}
which calls a6xx_destroy() -> adreno_gpu_cleanup(), seeing gpu->lpac_pdev
still non-NULL and calling put_device(&gpu->lpac_pdev->dev) a second time.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850961@gmail.com?part=3
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RFC v2 04/14] arm64: dts: qcom: sm8650: move smmu sid 1 to new lpac device
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
` (2 preceding siblings ...)
2026-10-29 13:52 ` [PATCH RFC v2 03/14] drm/msm: look for lpac from dts Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 14:17 ` sashiko-bot
2026-10-29 13:52 ` [PATCH RFC v2 05/14] firmware: qcom: scm: Configure LPAC aperture Anna Maniscalco
` (9 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
Previously both SID 0 and 1 where associated with the same domain.
When LPAC is not used this is needed so firmware can acces memory when
initializing using the same page table as GFX.
To use LPAC however we need to move SID 1 to a different device.
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
arch/arm64/boot/dts/qcom/sm8650.dtsi | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index 1604bc8cff37..f47d1dbd06da 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -4144,8 +4144,7 @@ gpu: gpu@3d00000 {
interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH 0>;
- iommus = <&adreno_smmu 0 0x0>,
- <&adreno_smmu 1 0x0>;
+ iommus = <&adreno_smmu 0 0x0>;
operating-points-v2 = <&gpu_opp_table>;
@@ -4158,6 +4157,12 @@ gpu: gpu@3d00000 {
status = "disabled";
+ lpac: lpac {
+ compatible = "qcom,lpac";
+
+ iommus = <&adreno_smmu 1 0x0>;
+ };
+
gpu_zap_shader: zap-shader {
memory-region = <&gpu_micro_code_mem>;
};
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RFC v2 04/14] arm64: dts: qcom: sm8650: move smmu sid 1 to new lpac device
2026-10-29 13:52 ` [PATCH RFC v2 04/14] arm64: dts: qcom: sm8650: move smmu sid 1 to new lpac device Anna Maniscalco
@ 2026-10-29 14:17 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-10-29 14:17 UTC (permalink / raw)
To: Anna Maniscalco; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [Critical] drm/msm: parent-child link failure in adreno_gpu_init() skips LPAC TLB flush
- [High] arm64: dts: qcom: sm8650: moving SID 1 to lpac breaks older kernels
- [High] drm/msm: double put_device() on lpac_pdev in adreno_gpu_init() error path
- [High] drm/msm: premature of_platform_depopulate() in adreno_remove()
- [High] drm/msm: missing TTBR0 rollback and NULL tlb in msm_iommu_pagetable_create()
- [High] drm/msm: leaked primary IOMMU mapping on lpac failure in msm_iommu_map()
- [Medium] arm64: dts: qcom: sm8650: undocumented lpac child node breaks dtbs_check
- [Medium] drm/msm: child platform device leak in adreno_probe() error path
--
commit 6e63347e2bf27a9a295e9b4311c3b2e372545491
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>
arm64: dts: qcom: sm8650: move smmu sid 1 to new lpac device
This commit moves stream id 1 out of the main gpu node on sm8650 and places
it into a new lpac child node so that lpac can be attached to a separate
iommu domain.
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index 1604bc8cff373..f47d1dbd06da4 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -4144,8 +4144,7 @@ gpu: gpu@3d00000 {
>
> interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH 0>;
>
> - iommus = <&adreno_smmu 0 0x0>,
> - <&adreno_smmu 1 0x0>;
> + iommus = <&adreno_smmu 0 0x0>;
[Severity: High]
Will removing <&adreno_smmu 1 0x0> from gpu@3d00000 break device tree
backwards compatibility with existing kernels?
As noted in the commit description, gpu firmware accesses memory through
stream id 1 during initialization using the graphics page table even when
lpac is not used. On any kernel without support for the new qcom,lpac child
node, removing <&adreno_smmu 1 0x0> from gpu@3d00000 leaves stream id 1
unattached to the gpu iommu domain, causing translation faults during
firmware initialization.
Also, since lpac is an internal execution pipe of the gpu rather than a
separate hardware block with its own register space, should the second domain
attachment be handled in the driver via iommu apis rather than splitting
stream id 1 into a virtual child node in the device tree?
>
> operating-points-v2 = <&gpu_opp_table>;
>
> @@ -4158,6 +4157,12 @@ gpu: gpu@3d00000 {
>
> status = "disabled";
>
> + lpac: lpac {
> + compatible = "qcom,lpac";
> +
> + iommus = <&adreno_smmu 1 0x0>;
> + };
[Severity: Medium]
Does adding the lpac child node here cause dtbs_check schema validation to
fail on sm8650 device trees?
In Documentation/devicetree/bindings/display/msm/gpu.yaml, the binding for
qcom,adreno specifies:
required:
- compatible
- reg
- interrupts
additionalProperties: false
Neither the lpac child node nor the qcom,lpac compatible string is documented
in Documentation/devicetree/bindings/display/msm/gpu.yaml, so schema
validation rejects the new node.
[Severity: Critical]
When this lpac child node is populated on sm8650, will device_link_add() in
adreno_gpu_init() always fail and cause msm_iommu_tlb_flush_all() and
msm_iommu_tlb_flush_walk() to skip flushing the lpac context bank?
In drivers/gpu/drm/msm/adreno/adreno_device.c, adreno_probe() populates child
nodes of gpu@3d00000 with &pdev->dev as their parent:
ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
Then in drivers/gpu/drm/msm/adreno/adreno_gpu.c, adreno_gpu_init() attempts
to create a runtime pm link with the parent &pdev->dev as consumer and its
child &gpu->lpac_pdev->dev as supplier:
if (gpu->lpac_pdev) {
pm_runtime_enable(&gpu->lpac_pdev->dev);
if (!device_link_add(&pdev->dev, &gpu->lpac_pdev->dev,
DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_CONSUMER))
dev_err(&gpu->lpac_pdev->dev, "failed to link to lpac device\n");
}
Inside drivers/base/core.c, device_link_add() -> device_is_dependent() checks
whether the consumer is an ancestor of the supplier:
if (dev == target || device_is_ancestor(dev, target))
return 1;
Because &pdev->dev is the parent of &gpu->lpac_pdev->dev, device_link_add()
rejects the link as a dependency cycle and returns NULL. In addition,
gpu->lpac_pdev has no driver bound to it, so a managed link would remain in
DL_STATE_DORMANT.
As a result, gpu->lpac_pdev->dev is never runtime-resumed. When per-context
page tables are flushed in drivers/gpu/drm/msm/msm_iommu.c via
msm_iommu_tlb_flush_all() and msm_iommu_tlb_flush_walk():
if (pagetable->parent->lpac_dev &&
pm_runtime_get_if_in_use(pagetable->parent->lpac_dev)) {
adreno_smmu = dev_get_drvdata(pagetable->parent->lpac_dev);
pagetable->tlb->tlb_flush_walk(iova, size, granule, (void *)adreno_smmu->cookie);
pm_runtime_put_autosuspend(pagetable->parent->lpac_dev);
}
pm_runtime_get_if_in_use(pagetable->parent->lpac_dev) always returns 0, so
tlb invalidations on the lpac context bank are skipped when buffers are
unmapped, leaving stale tlb entries pointing to freed pages.
[Severity: High]
If adreno_gpu_init() fails after acquiring gpu->lpac_pdev on sm8650, can
gpu->lpac_pdev->dev undergo a double put_device()?
In drivers/gpu/drm/msm/adreno/adreno_gpu.c, if devm_kasprintf() or
adreno_get_pwrlevels() fails in adreno_gpu_init(), the error path drops the
reference on gpu->lpac_pdev->dev without disabling runtime pm or clearing
gpu->lpac_pdev:
err_put_lpac_pdev:
if (gpu->lpac_pdev)
put_device(&gpu->lpac_pdev->dev);
return ret;
The caller in drivers/gpu/drm/msm/adreno/a6xx_gpu.c, a6xx_gpu_init(), then
invokes a6xx_destroy() -> adreno_gpu_cleanup():
ret = adreno_gpu_init(dev, pdev, adreno_gpu, info->funcs, nr_rings);
if (ret) {
a6xx_destroy(&(a6xx_gpu->base.base));
return ERR_PTR(ret);
}
In adreno_gpu_cleanup(), gpu->lpac_pdev is still non-NULL, so it calls
pm_runtime_disable() and put_device() a second time on &gpu->lpac_pdev->dev:
if (gpu->lpac_pdev) {
pm_runtime_disable(&gpu->lpac_pdev->dev);
put_device(&gpu->lpac_pdev->dev);
}
[Severity: High]
In drivers/gpu/drm/msm/adreno/adreno_device.c, does calling
of_platform_depopulate() at the start of adreno_remove() unregister
lpac_pdev while iommu->lpac_domain is still attached to it?
In adreno_remove():
static void adreno_remove(struct platform_device *pdev)
{
struct msm_drm_private *priv = platform_get_drvdata(pdev);
of_platform_depopulate(&pdev->dev);
if (priv->kms_init)
component_del(&pdev->dev, &a3xx_ops);
else
msm_gpu_remove(pdev, &a3xx_ops);
}
of_platform_depopulate(&pdev->dev) unregisters the child lpac platform device
before component_del() or msm_gpu_remove() runs adreno_unbind() ->
a6xx_destroy() -> adreno_gpu_cleanup() -> msm_gpu_cleanup() ->
msm_iommu_detach().
Because iommu->lpac_domain is still attached when lpac_pdev is unregistered,
__iommu_group_free_device() in drivers/iommu/iommu.c hits:
if (list_empty(&group->devices))
WARN_ON(group->owner_cnt ||
group->domain != group->default_domain);
and msm_iommu_detach() and adreno_gpu_cleanup() subsequently operate on an
already-unregistered device.
[Severity: Medium]
What happens to the populated lpac child platform device if msm_gpu_probe()
or component_add() fails in adreno_probe()?
In drivers/gpu/drm/msm/adreno/adreno_device.c:
ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
if (ret)
return ret;
if (of_device_is_compatible(pdev->dev.of_node, "amd,imageon") ||
msm_gpu_no_components())
return msm_gpu_probe(pdev, &a3xx_ops);
return component_add(&pdev->dev, &a3xx_ops);
If msm_gpu_probe() or component_add() returns an error,
of_platform_depopulate(&pdev->dev) is not called, leaking the child platform
device created for lpac.
[Severity: High]
In drivers/gpu/drm/msm/msm_iommu.c, if lpac_adreno_smmu->set_ttbr0_cfg()
fails in msm_iommu_pagetable_create(), does the error path dereference a NULL
pagetable->tlb and leave ttbr0 enabled on the primary context bank with freed
page tables?
When creating the first pagetable in msm_iommu_pagetable_create(),
adreno_smmu->set_ttbr0_cfg(adreno_smmu->cookie, &ttbr0_cfg) enables ttbr0 on
the primary context bank before lpac_adreno_smmu->set_ttbr0_cfg() is called:
if (lpac_adreno_smmu) {
ret = lpac_adreno_smmu->set_ttbr0_cfg(lpac_adreno_smmu->cookie, &ttbr0_cfg);
if (ret) {
iommu->pagetables--;
mutex_unlock(&iommu->init_lock);
free_io_pgtable_ops(pagetable->pgtbl_ops);
kfree(pagetable);
return ERR_PTR(ret);
}
}
At this point pagetable->tlb has not been assigned yet, as that happens later
in msm_iommu_pagetable_create():
/* Needed later for TLB flush */
pagetable->tlb = ttbr1_cfg->tlb;
Calling free_io_pgtable_ops(pagetable->pgtbl_ops) invokes
msm_iommu_tlb_flush_all(), which dereferences pagetable->tlb if
pagetable->iommu_dev is runtime-active. In addition, because
adreno_smmu->set_ttbr0_cfg(adreno_smmu->cookie, NULL) is not called to roll
back the primary context bank configuration, context bank 0 retains a ttbr0
register pointing to the freed page table and qcom_adreno_smmu_set_ttbr0_cfg()
in drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c will reject subsequent calls
with -EINVAL because ARM_SMMU_TCR_EPD0 is already cleared:
/* Don't call this again if TTBR0 is already enabled */
if (!(cb->tcr[0] & ARM_SMMU_TCR_EPD0))
return -EINVAL;
[Severity: High]
Can msm_iommu_map() in drivers/gpu/drm/msm/msm_iommu.c leak the mapping in
iommu->domain if mapping into iommu->lpac_domain fails?
In msm_iommu_map():
ret = iommu_map_sgtable(iommu->domain, iova, sgt, prot);
if (ret < 0)
return ret;
if (iommu->lpac_domain) {
ret = iommu_map_sgtable(iommu->lpac_domain, iova, sgt, prot);
if (ret < 0)
return ret;
}
return (ret == len) ? 0 : -EINVAL;
If the second iommu_map_sgtable() call on iommu->lpac_domain returns an error
or maps fewer than len bytes after the first iommu_map_sgtable() on
iommu->domain succeeded, msm_iommu_map() returns an error without calling
iommu_unmap(iommu->domain, iova, len). In drivers/gpu/drm/msm/msm_gem_vma.c,
msm_gem_vma_map() then leaves msm_vma->mapped set to false:
if (ret)
msm_vma->mapped = false;
return ret;
so msm_gem_vma_unmap() will never clean up the mapping left behind in
iommu->domain.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850961@gmail.com?part=4
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RFC v2 05/14] firmware: qcom: scm: Configure LPAC aperture
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
` (3 preceding siblings ...)
2026-10-29 13:52 ` [PATCH RFC v2 04/14] arm64: dts: qcom: sm8650: move smmu sid 1 to new lpac device Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 14:08 ` sashiko-bot
2026-10-29 13:52 ` [PATCH RFC v2 06/14] DEBUGGING: print contextbank and other ttbrs on fault Anna Maniscalco
` (8 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
In order to be able to update the SMMU registers from CP for page table
updates we need to configure the LPAC aperture to include the context
bank that will be associated with SID 1.
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/firmware/qcom/qcom_scm.c | 18 ++++++++++++++++++
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 6 ++++++
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 1 +
include/linux/firmware/qcom/qcom_scm.h | 1 +
4 files changed, 26 insertions(+)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 9b06a69d3a6d..92093399f20d 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1204,6 +1204,7 @@ int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare)
EXPORT_SYMBOL_GPL(qcom_scm_restore_sec_cfg);
#define QCOM_SCM_CP_APERTURE_CONTEXT_MASK GENMASK(7, 0)
+#define QCOM_SCM_LPAC_APERTURE_CONTEXT_MASK GENMASK(7, 1)
bool qcom_scm_set_gpu_smmu_aperture_is_available(void)
{
@@ -1229,6 +1230,23 @@ int qcom_scm_set_gpu_smmu_aperture(unsigned int context_bank)
}
EXPORT_SYMBOL_GPL(qcom_scm_set_gpu_smmu_aperture);
+int qcom_scm_set_gpu_smmu_lpac_aperture(unsigned int context_bank)
+{
+ struct qcom_scm_desc desc = {
+ .svc = QCOM_SCM_SVC_MP,
+ .cmd = QCOM_SCM_MP_CP_SMMU_APERTURE_ID,
+ .arginfo = QCOM_SCM_ARGS(4),
+ .args[0] = 0xffff0000 | (1 << 8 | context_bank),
+ .args[1] = 0xffffffff,
+ .args[2] = 0xffffffff,
+ .args[3] = 0xffffffff,
+ .owner = ARM_SMCCC_OWNER_SIP
+ };
+
+ return qcom_scm_call(__scm->dev, &desc, NULL);
+}
+EXPORT_SYMBOL_GPL(qcom_scm_set_gpu_smmu_lpac_aperture);
+
int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size)
{
struct qcom_scm_desc desc = {
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 1dedeb19946e..65d34d0cfc38 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -658,6 +658,7 @@ struct drm_gem_object *adreno_fw_create_bo(struct msm_gpu *gpu,
int adreno_hw_init(struct msm_gpu *gpu)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ const struct adreno_info *info = adreno_gpu->info;
int ret;
VERB("%s", gpu->name);
@@ -668,6 +669,11 @@ int adreno_hw_init(struct msm_gpu *gpu)
ret = qcom_scm_set_gpu_smmu_aperture(0);
if (ret)
DRM_DEV_ERROR(gpu->dev->dev, "unable to set SMMU aperture: %d\n", ret);
+ if (info->quirks & ADRENO_QUIRK_LPAC) {
+ ret = qcom_scm_set_gpu_smmu_lpac_aperture(1);
+ if (ret)
+ DRM_DEV_ERROR(gpu->dev->dev, "unable to set lpac SMMU aperture: %d\n", ret);
+ }
}
for (int i = 0; i < gpu->nr_rings; i++) {
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 1f201322cb6e..ec234a483036 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -64,6 +64,7 @@ enum adreno_family {
#define ADRENO_QUIRK_4GB_VA BIT(6)
#define ADRENO_QUIRK_IFPC BIT(7)
#define ADRENO_QUIRK_SOFTFUSE BIT(8)
+#define ADRENO_QUIRK_LPAC BIT(9)
/* Helper for formating the chip_id in the way that userspace tools like
* crashdec expect.
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index 5747bd191bf1..70fb93e303e2 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -100,6 +100,7 @@ int qcom_scm_io_writel(phys_addr_t addr, unsigned int val);
bool qcom_scm_restore_sec_cfg_available(void);
int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare);
int qcom_scm_set_gpu_smmu_aperture(unsigned int context_bank);
+int qcom_scm_set_gpu_smmu_lpac_aperture(unsigned int context_bank);
bool qcom_scm_set_gpu_smmu_aperture_is_available(void);
int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size);
int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare);
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RFC v2 05/14] firmware: qcom: scm: Configure LPAC aperture
2026-10-29 13:52 ` [PATCH RFC v2 05/14] firmware: qcom: scm: Configure LPAC aperture Anna Maniscalco
@ 2026-10-29 14:08 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-10-29 14:08 UTC (permalink / raw)
To: Anna Maniscalco; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/msm/adreno: missing lpac_pdev check in adreno_hw_init()
- [Low] firmware: qcom: scm: broken mask macro and unmasked context_bank argument
--
commit ea6cfd100e49bc2229349870a3f6d5f6dfcedd94
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>
firmware: qcom: scm: Configure LPAC aperture
This commit adds qcom_scm_set_gpu_smmu_lpac_aperture() to configure the
SMMU LPAC aperture via SCM and calls it from adreno_hw_init() when
ADRENO_QUIRK_LPAC is set.
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 9b06a69d3a6d7..92093399f20d9 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -1204,6 +1204,7 @@ int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare)
> EXPORT_SYMBOL_GPL(qcom_scm_restore_sec_cfg);
>
> #define QCOM_SCM_CP_APERTURE_CONTEXT_MASK GENMASK(7, 0)
> +#define QCOM_SCM_LPAC_APERTURE_CONTEXT_MASK GENMASK(7, 1)
>
> bool qcom_scm_set_gpu_smmu_aperture_is_available(void)
> {
> @@ -1229,6 +1230,23 @@ int qcom_scm_set_gpu_smmu_aperture(unsigned int context_bank)
> }
> EXPORT_SYMBOL_GPL(qcom_scm_set_gpu_smmu_aperture);
>
> +int qcom_scm_set_gpu_smmu_lpac_aperture(unsigned int context_bank)
> +{
> + struct qcom_scm_desc desc = {
> + .svc = QCOM_SCM_SVC_MP,
> + .cmd = QCOM_SCM_MP_CP_SMMU_APERTURE_ID,
> + .arginfo = QCOM_SCM_ARGS(4),
> + .args[0] = 0xffff0000 | (1 << 8 | context_bank),
> + .args[1] = 0xffffffff,
> + .args[2] = 0xffffffff,
> + .args[3] = 0xffffffff,
> + .owner = ARM_SMCCC_OWNER_SIP
> + };
> +
> + return qcom_scm_call(__scm->dev, &desc, NULL);
> +}
> +EXPORT_SYMBOL_GPL(qcom_scm_set_gpu_smmu_lpac_aperture);
[Severity: Low]
Should QCOM_SCM_LPAC_APERTURE_CONTEXT_MASK be dropped or defined as BIT(8),
and should context_bank be masked with FIELD_PREP() in
qcom_scm_set_gpu_smmu_lpac_aperture()?
Currently QCOM_SCM_LPAC_APERTURE_CONTEXT_MASK is defined as GENMASK(7, 1)
and never used, while qcom_scm_set_gpu_smmu_aperture() masks context_bank
into bits 7:0:
drivers/firmware/qcom/qcom_scm.c:qcom_scm_set_gpu_smmu_aperture() {
...
.args[0] = 0xffff0000 | FIELD_PREP(QCOM_SCM_CP_APERTURE_CONTEXT_MASK, context_bank),
...
}
In qcom_scm_set_gpu_smmu_lpac_aperture(), ORing an unmasked context_bank
with (1 << 8) could clobber bit 8 or upper bits of args[0] if a caller
passes a value larger than 0xff.
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 1dedeb19946ec..65d34d0cfc380 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -658,6 +658,7 @@ struct drm_gem_object *adreno_fw_create_bo(struct msm_gpu *gpu,
> int adreno_hw_init(struct msm_gpu *gpu)
> {
> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> + const struct adreno_info *info = adreno_gpu->info;
> int ret;
>
> VERB("%s", gpu->name);
> @@ -668,6 +669,11 @@ int adreno_hw_init(struct msm_gpu *gpu)
> ret = qcom_scm_set_gpu_smmu_aperture(0);
> if (ret)
> DRM_DEV_ERROR(gpu->dev->dev, "unable to set SMMU aperture: %d\n", ret);
> + if (info->quirks & ADRENO_QUIRK_LPAC) {
> + ret = qcom_scm_set_gpu_smmu_lpac_aperture(1);
> + if (ret)
> + DRM_DEV_ERROR(gpu->dev->dev, "unable to set lpac SMMU aperture: %d\n", ret);
> + }
> }
[Severity: High]
Does adreno_hw_init() also need to check gpu->lpac_pdev before calling
qcom_scm_set_gpu_smmu_lpac_aperture(1)?
In adreno_gpu_init(), gpu->lpac_pdev is only populated when the device tree
contains a qcom,lpac child node:
drivers/gpu/drm/msm/adreno/adreno_gpu.c:adreno_gpu_init() {
...
lpac_node = of_get_compatible_child(pdev->dev.of_node, "qcom,lpac");
if (lpac_node)
gpu->lpac_pdev = of_find_device_by_node(lpac_node);
...
}
And adreno_iommu_create_vm() only creates the separate LPAC IOMMU domain
(which allocates SMMU context bank 1) when gpu->lpac_pdev is non-NULL:
drivers/gpu/drm/msm/adreno/adreno_gpu.c:adreno_iommu_create_vm() {
...
mmu = msm_iommu_gpu_new(&pdev->dev,
gpu->lpac_pdev ? &gpu->lpac_pdev->dev : NULL,
gpu, quirks);
...
}
When booting a GPU with ADRENO_QUIRK_LPAC on an existing DTB that does not
have the qcom,lpac child node, gpu->lpac_pdev is NULL and context bank 1 is
never allocated. Won't calling qcom_scm_set_gpu_smmu_lpac_aperture(1)
unconditionally on ADRENO_QUIRK_LPAC configure the LPAC aperture to an
unallocated context bank?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850961@gmail.com?part=5
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RFC v2 06/14] DEBUGGING: print contextbank and other ttbrs on fault
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
` (4 preceding siblings ...)
2026-10-29 13:52 ` [PATCH RFC v2 05/14] firmware: qcom: scm: Configure LPAC aperture Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 13:52 ` [PATCH RFC v2 07/14] iommu: arm-smmu-qcom: Fixed mapping between sid and cb for gpu and lpac Anna Maniscalco
` (7 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
Faults can now come from different context banks so print that
information as well.
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 ++--
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 4 ++++
include/linux/adreno-smmu-priv.h | 2 ++
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 65d34d0cfc38..4389559e1d17 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -329,8 +329,8 @@ int adreno_fault_handler(struct msm_gpu *gpu, unsigned long iova, int flags,
else if (info->fsr & ARM_SMMU_FSR_EF)
type = "EXTERNAL";
- pr_warn_ratelimited("*** gpu fault: ttbr0=%.16llx iova=%.16lx dir=%s type=%s source=%s (%u,%u,%u,%u)\n",
- info->ttbr0, iova,
+ pr_warn_ratelimited("*** gpu fault: cb=%d ttbr0=%.16llx cb0_ttbr0=%.16llx cb1_ttbr0=%.16llx iova=%.16lx dir=%s type=%s source=%s (%u,%u,%u,%u)\n",
+ info->contextbank, info->ttbr0, info->cb0_ttbr0, info->cb1_ttbr0, iova,
flags & IOMMU_FAULT_WRITE ? "WRITE" : "READ",
type, block,
scratch[0], scratch[1], scratch[2], scratch[3]);
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 48a590aaeb5e..ae6152bddf8f 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -136,6 +136,10 @@ static void qcom_adreno_smmu_get_fault_info(const void *cookie,
info->cbfrsynra = arm_smmu_gr1_read(smmu, ARM_SMMU_GR1_CBFRSYNRA(cfg->cbndx));
info->ttbr0 = arm_smmu_cb_readq(smmu, cfg->cbndx, ARM_SMMU_CB_TTBR0);
info->contextidr = arm_smmu_cb_read(smmu, cfg->cbndx, ARM_SMMU_CB_CONTEXTIDR);
+ info->contextbank = cfg->cbndx;
+
+ info->cb0_ttbr0 = arm_smmu_cb_readq(smmu, 0, ARM_SMMU_CB_TTBR0);
+ info->cb1_ttbr0 = arm_smmu_cb_readq(smmu, 1, ARM_SMMU_CB_TTBR0);
}
static void qcom_adreno_smmu_set_stall(const void *cookie, bool enabled)
diff --git a/include/linux/adreno-smmu-priv.h b/include/linux/adreno-smmu-priv.h
index d83c9175828f..5d6de4b4de06 100644
--- a/include/linux/adreno-smmu-priv.h
+++ b/include/linux/adreno-smmu-priv.h
@@ -32,6 +32,8 @@ struct adreno_smmu_fault_info {
u32 fsynr0;
u32 fsynr1;
u32 cbfrsynra;
+ u32 contextbank;
+ u64 cb0_ttbr0, cb1_ttbr0;
};
/**
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH RFC v2 07/14] iommu: arm-smmu-qcom: Fixed mapping between sid and cb for gpu and lpac
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
` (5 preceding siblings ...)
2026-10-29 13:52 ` [PATCH RFC v2 06/14] DEBUGGING: print contextbank and other ttbrs on fault Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 14:13 ` sashiko-bot
2026-10-29 13:52 ` [PATCH RFC v2 08/14] temp: add LPAC regs Anna Maniscalco
` (6 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
We want to map context bank 0 to SID 0 and context bank 1 to SID 1 as it
makes it simpler to deal with.
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 31 ++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index ae6152bddf8f..9413b748e79d 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -225,6 +225,24 @@ static void qcom_adreno_smmu_set_prr_addr(const void *cookie, phys_addr_t page_a
#define QCOM_ADRENO_SMMU_GPU_SID 0
#define QCOM_ADRENO_SMMU_LPAC_SID 1
+static bool qcom_adreno_smmu_is_lpac_device(struct device *dev)
+{
+ struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+ int i;
+
+ /*
+ * LPAC will always use SID 1
+ */
+ for (i = 0; i < fwspec->num_ids; i++) {
+ u16 sid = FIELD_GET(ARM_SMMU_SMR_ID, fwspec->ids[i]);
+
+ if (sid == QCOM_ADRENO_SMMU_LPAC_SID)
+ return true;
+ }
+
+ return false;
+}
+
static bool qcom_adreno_smmu_is_gpu_device(struct device *dev)
{
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
@@ -237,8 +255,7 @@ static bool qcom_adreno_smmu_is_gpu_device(struct device *dev)
for (i = 0; i < fwspec->num_ids; i++) {
u16 sid = FIELD_GET(ARM_SMMU_SMR_ID, fwspec->ids[i]);
- if (sid == QCOM_ADRENO_SMMU_GPU_SID ||
- sid == QCOM_ADRENO_SMMU_LPAC_SID)
+ if (sid == QCOM_ADRENO_SMMU_GPU_SID)
return true;
}
@@ -313,12 +330,14 @@ static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_doma
*/
if (qcom_adreno_smmu_is_gpu_device(dev)) {
start = 0;
- count = 2;
+ count = start + 1;
+ } else if (qcom_adreno_smmu_is_lpac_device(dev)) {
+ start = 1;
+ count = start + 1;
} else {
start = 2;
count = smmu->num_context_banks;
}
-
return __arm_smmu_alloc_bitmap(smmu->context_map, start, count);
}
@@ -363,8 +382,8 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
if (client_match)
qcom_smmu_set_actlr_dev(dev, smmu, cbndx, client_match);
- /* Only enable split pagetables for the GPU device (SID 0) */
- if (!qcom_adreno_smmu_is_gpu_device(dev))
+ /* Only enable split pagetables for the GPU and LPAC devices (SID 0 and 1) */
+ if (!qcom_adreno_smmu_is_gpu_device(dev) && !qcom_adreno_smmu_is_lpac_device(dev))
return 0;
/*
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RFC v2 07/14] iommu: arm-smmu-qcom: Fixed mapping between sid and cb for gpu and lpac
2026-10-29 13:52 ` [PATCH RFC v2 07/14] iommu: arm-smmu-qcom: Fixed mapping between sid and cb for gpu and lpac Anna Maniscalco
@ 2026-10-29 14:13 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-10-29 14:13 UTC (permalink / raw)
To: Anna Maniscalco; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] iommu/arm-smmu-qcom: unconditional CB 1 reservation for LPAC shifts GMU CB
--
commit 51e5e8227679664f11df23a51921048a054a1310
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>
iommu: arm-smmu-qcom: Fixed mapping between sid and cb for gpu and lpac
This commit splits context bank allocation and initialization in
arm-smmu-qcom so that the gpu device (stream id 0) is assigned context bank 0
and the lpac device (stream id 1) is assigned context bank 1, while other
devices start allocation from context bank 2.
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index ae6152bddf8fe..9413b748e79d1 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
[ ... ]
> @@ -313,12 +330,14 @@ static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_doma
> */
> if (qcom_adreno_smmu_is_gpu_device(dev)) {
> start = 0;
> - count = 2;
> + count = start + 1;
> + } else if (qcom_adreno_smmu_is_lpac_device(dev)) {
> + start = 1;
> + count = start + 1;
> } else {
> start = 2;
> count = smmu->num_context_banks;
> }
> -
> return __arm_smmu_alloc_bitmap(smmu->context_map, start, count);
> }
[Severity: High]
Does unconditionally reserving context bank 1 for a separate lpac device and
forcing all other devices into the else branch with start = 2 cause a
regression on platforms without a separate qcom,lpac device node?
In drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c,
qcom_adreno_smmu_alloc_context_bank() now assigns only context bank 0 when
qcom_adreno_smmu_is_gpu_device(dev) is true, reserves context bank 1 for a
separate device matching qcom_adreno_smmu_is_lpac_device(dev), and starts all
other allocations at context bank 2.
On platforms that do not have stream id 1 (such as sdm845, sc7180, sm8150, and
sm8250), or on platforms where both stream id 0 and stream id 1 are listed on
the main gpu node rather than a separate qcom,lpac child node (such as
sc8280xp, sm8350, sc7280, sm8450, sm8550, x1e80100, or sm8650 with an older
device tree), the main gpu device matches qcom_adreno_smmu_is_gpu_device(dev)
first and allocates context bank 0, leaving context bank 1 unallocated.
When a6xx_gmu_memory_probe() -> msm_iommu_new() -> iommu_attach_device() ->
arm_smmu_init_domain_context() -> qcom_adreno_smmu_alloc_context_bank()
subsequently allocates a context bank for the qcom,adreno-gmu device (stream
id 5), it falls into the else branch with start = 2 and receives cbndx = 2
instead of cbndx = 1. As noted in commit afc95681c306 ("iommu/arm-smmu-qcom:
Add missing GMU entry to match table"), having the gmu on cbndx = 2 instead of
cbndx = 1 on sc8280xp causes the smmu to wedge on gpu translation faults.
In addition, on platforms with only two context banks available to Linux,
such as qcom,sdm630-smmu-v2:
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c:qcom_adreno_smmuv2_cfg_probe() {
/* TZ protects several last context banks, hide them from Linux */
if (of_device_is_compatible(smmu->dev->of_node, "qcom,sdm630-smmu-v2") &&
smmu->num_context_banks == 5)
smmu->num_context_banks = 2;
}
entering the else branch passes start = 2 and count = 2 to
__arm_smmu_alloc_bitmap(), which immediately fails even though context bank 1
is free:
drivers/iommu/arm/arm-smmu/arm-smmu.h:__arm_smmu_alloc_bitmap() {
do {
idx = find_next_zero_bit(map, end, start);
if (idx == end)
return -ENOSPC;
} while (test_and_set_bit(idx, map));
}
Could context bank 1 only be reserved when a separate lpac device is actually
present, or should the else branch still allow allocating from context bank 1
when lpac is not used?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850961@gmail.com?part=7
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RFC v2 08/14] temp: add LPAC regs
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
` (6 preceding siblings ...)
2026-10-29 13:52 ` [PATCH RFC v2 07/14] iommu: arm-smmu-qcom: Fixed mapping between sid and cb for gpu and lpac Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 14:13 ` sashiko-bot
2026-10-29 13:52 ` [PATCH RFC v2 09/14] drm/msm: initialize LPAC ring Anna Maniscalco
` (5 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39416
should we merged and we should sync to mesa instead
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/gpu/drm/msm/registers/adreno/a6xx.xml | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/msm/registers/adreno/a6xx.xml b/drivers/gpu/drm/msm/registers/adreno/a6xx.xml
index 3349c01646e1..bb49f6a61a9a 100644
--- a/drivers/gpu/drm/msm/registers/adreno/a6xx.xml
+++ b/drivers/gpu/drm/msm/registers/adreno/a6xx.xml
@@ -276,6 +276,11 @@ by a particular renderpass/blit.
<bitfield pos="1" name="ACCESS_FAULT_ON_VIOL_EN" type="boolean"/>
<bitfield pos="0" name="ACCESS_PROT_EN" type="boolean"/>
</bitset>
+ <reg32 offset="0x0B09" name="CP_LPAC_PROTECT_CNTL">
+ <bitfield pos="3" name="LAST_SPAN_INF_RANGE" type="boolean"/>
+ <bitfield pos="1" name="ACCESS_FAULT_ON_VIOL_EN" type="boolean"/>
+ <bitfield pos="0" name="ACCESS_PROT_EN" type="boolean"/>
+ </reg32>
<reg32 offset="0x084f" name="CP_PROTECT_CNTL" type="a6xx_cp_protect_cntl" variants="A6XX-A7XX"/>
<bitset name="a8xx_cp_protect_cntl" inline="yes">
@@ -333,6 +338,11 @@ by a particular renderpass/blit.
<reg32 offset="0x0845" name="CP_CRASH_DUMP_STATUS" variants="A8XX-"/>
<reg32 offset="0x0908" name="CP_SQE_STAT_ADDR" variants="A6XX-A7XX"/>
<reg32 offset="0x0909" name="CP_SQE_STAT_DATA" variants="A6XX-A7XX"/>
+ <reg64 offset="0x0b00" name="CP_LPAC_RB_BASE" variants="A7XX-"/>
+ <reg32 offset="0x0b02" name="CP_LPAC_RB_CNTL" variants="A7XX-"/>
+ <reg64 offset="0x0b04" name="CP_LPAC_RB_RPTR_ADDR" variants="A7XX-"/>
+ <reg32 offset="0x0b06" name="CP_LPAC_RB_RPTR" variants="A7XX-"/>
+ <reg32 offset="0x0b07" name="CP_LPAC_RB_WPTR" variants="A7XX-"/>
<reg32 offset="0x090a" name="CP_DRAW_STATE_ADDR" variants="A6XX-A7XX"/>
<reg32 offset="0x090b" name="CP_DRAW_STATE_DATA" variants="A6XX-A7XX"/>
<reg32 offset="0x090c" name="CP_ROQ_DBG_ADDR" variants="A6XX-A7XX"/>
@@ -521,6 +531,7 @@ by a particular renderpass/blit.
<reg32 offset="0x0b29" name="CP_SQE_AC_STAT_ADDR" variants="A7XX"/>
<reg32 offset="0x0b2a" name="CP_SQE_AC_STAT_DATA" variants="A7XX"/>
+ <reg32 offset="0x0b30" name="CP_LPAC_CHICKEN_DBG" variants="A7XX-"/>
<reg32 offset="0x0b31" name="CP_LPAC_APRIV_CNTL" variants="A7XX"/>
<reg32 offset="0x0b34" name="CP_LPAC_PROG_FIFO_SIZE" variants="A7XX"/>
<reg32 offset="0x0b35" name="CP_LPAC_ROQ_DBG_DATA" variants="A7XX"/>
@@ -1207,6 +1218,7 @@ by a particular renderpass/blit.
<reg64 offset="0x0E08" name="UCHE_TRAP_BASE" variants="A8XX-"/>
<reg64 offset="0x0E0B" name="UCHE_GMEM_RANGE_MIN" variants="A6XX-A7XX"/>
<reg64 offset="0x0E0D" name="UCHE_GMEM_RANGE_MAX" variants="A6XX-A7XX"/>
+ <reg32 offset="0x0E12" name="UCHE_DEBUG_CNTL_1" usage="cmd"/>
<reg32 offset="0x0e17" name="UCHE_CACHE_WAYS" variants="A6XX-A7XX" usage="init"/>
<reg32 offset="0x0e04" name="UCHE_CACHE_WAYS" variants="A8XX-"/>
<reg32 offset="0x0E18" name="UCHE_FILTER_CNTL"/>
@@ -4406,6 +4418,8 @@ by a particular renderpass/blit.
<array offset="0xaec0" name="SP_PERFCTR_HLSQ_SEL_2" stride="1" length="6" variants="A7XX-"/>
+ <reg32 offset="0xaf84" name="SP_LPAC_CHICKEN_BITS_2" variants="A7XX-" usage="cmd"/>
+
<!--
The downstream kernel calls the debug cluster of registers
"a6xx_sp_ps_tp_cluster" but this actually specifies the border
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RFC v2 08/14] temp: add LPAC regs
2026-10-29 13:52 ` [PATCH RFC v2 08/14] temp: add LPAC regs Anna Maniscalco
@ 2026-10-29 14:13 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-10-29 14:13 UTC (permalink / raw)
To: Anna Maniscalco; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/msm: conflicting offsets and variants for LPAC regs in a6xx.xml
--
commit cffea449b0ea5b2e3282237ed9e0160eef7997c8
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>
temp: add LPAC regs
This commit adds LPAC and UCHE register definitions to the Adreno a6xx XML
register database.
> diff --git a/drivers/gpu/drm/msm/registers/adreno/a6xx.xml b/drivers/gpu/drm/msm/registers/adreno/a6xx.xml
> index 3349c01646e12..bb49f6a61a9af 100644
> --- a/drivers/gpu/drm/msm/registers/adreno/a6xx.xml
> +++ b/drivers/gpu/drm/msm/registers/adreno/a6xx.xml
> @@ -276,6 +276,11 @@ by a particular renderpass/blit.
> <bitfield pos="1" name="ACCESS_FAULT_ON_VIOL_EN" type="boolean"/>
> <bitfield pos="0" name="ACCESS_PROT_EN" type="boolean"/>
> </bitset>
> + <reg32 offset="0x0B09" name="CP_LPAC_PROTECT_CNTL">
> + <bitfield pos="3" name="LAST_SPAN_INF_RANGE" type="boolean"/>
> + <bitfield pos="1" name="ACCESS_FAULT_ON_VIOL_EN" type="boolean"/>
> + <bitfield pos="0" name="ACCESS_PROT_EN" type="boolean"/>
> + </reg32>
>
> <reg32 offset="0x084f" name="CP_PROTECT_CNTL" type="a6xx_cp_protect_cntl" variants="A6XX-A7XX"/>
> <bitset name="a8xx_cp_protect_cntl" inline="yes">
> @@ -333,6 +338,11 @@ by a particular renderpass/blit.
> <reg32 offset="0x0845" name="CP_CRASH_DUMP_STATUS" variants="A8XX-"/>
> <reg32 offset="0x0908" name="CP_SQE_STAT_ADDR" variants="A6XX-A7XX"/>
> <reg32 offset="0x0909" name="CP_SQE_STAT_DATA" variants="A6XX-A7XX"/>
> + <reg64 offset="0x0b00" name="CP_LPAC_RB_BASE" variants="A7XX-"/>
> + <reg32 offset="0x0b02" name="CP_LPAC_RB_CNTL" variants="A7XX-"/>
> + <reg64 offset="0x0b04" name="CP_LPAC_RB_RPTR_ADDR" variants="A7XX-"/>
> + <reg32 offset="0x0b06" name="CP_LPAC_RB_RPTR" variants="A7XX-"/>
> + <reg32 offset="0x0b07" name="CP_LPAC_RB_WPTR" variants="A7XX-"/>
> <reg32 offset="0x090a" name="CP_DRAW_STATE_ADDR" variants="A6XX-A7XX"/>
> <reg32 offset="0x090b" name="CP_DRAW_STATE_DATA" variants="A6XX-A7XX"/>
> <reg32 offset="0x090c" name="CP_ROQ_DBG_ADDR" variants="A6XX-A7XX"/>
> @@ -521,6 +531,7 @@ by a particular renderpass/blit.
> <reg32 offset="0x0b29" name="CP_SQE_AC_STAT_ADDR" variants="A7XX"/>
> <reg32 offset="0x0b2a" name="CP_SQE_AC_STAT_DATA" variants="A7XX"/>
>
> + <reg32 offset="0x0b30" name="CP_LPAC_CHICKEN_DBG" variants="A7XX-"/>
> <reg32 offset="0x0b31" name="CP_LPAC_APRIV_CNTL" variants="A7XX"/>
> <reg32 offset="0x0b34" name="CP_LPAC_PROG_FIFO_SIZE" variants="A7XX"/>
> <reg32 offset="0x0b35" name="CP_LPAC_ROQ_DBG_DATA" variants="A7XX"/>
> @@ -1207,6 +1218,7 @@ by a particular renderpass/blit.
> <reg64 offset="0x0E08" name="UCHE_TRAP_BASE" variants="A8XX-"/>
> <reg64 offset="0x0E0B" name="UCHE_GMEM_RANGE_MIN" variants="A6XX-A7XX"/>
> <reg64 offset="0x0E0D" name="UCHE_GMEM_RANGE_MAX" variants="A6XX-A7XX"/>
> + <reg32 offset="0x0E12" name="UCHE_DEBUG_CNTL_1" usage="cmd"/>
[Severity: Medium]
Should these new register definitions specify narrower hardware variants to
avoid colliding with existing A6XX, A7XX, and A8XX registers during header
generation in drivers/gpu/drm/msm/registers/gen_header.py?
First, UCHE_DEBUG_CNTL_1 is added at offset 0x0E12 with usage="cmd" and no
variants attribute, while leaving the existing definition for 0x0e12 on
A6XX-A7XX in place and overlapping with UCHE_GBIF_GX_CONFIG on A8XX-:
<reg32 offset="0x0e3a" name="UCHE_GBIF_GX_CONFIG" variants="A6XX-A7XX"/>
<reg32 offset="0x0e12" name="UCHE_GBIF_GX_CONFIG" variants="A8XX-"/>
...
<!-- always 0x03200000 ? -->
<reg32 offset="0x0e12" name="UCHE_UNKNOWN_0E12" variants="A6XX-A7XX" usage="init"/>
Because UCHE_DEBUG_CNTL_1 has no variants restriction, dump_reg_usages() in
gen_header.py emits 0x0e12 into CMD_REGS<A8XX> where that offset is
UCHE_GBIF_GX_CONFIG, and leaves duplicate definitions for 0x0e12 on
A6XX-A7XX.
Second, CP_LPAC_RB_BASE (0x0b00), CP_LPAC_RB_CNTL (0x0b02),
CP_LPAC_RB_RPTR_ADDR (0x0b04), CP_LPAC_RB_RPTR (0x0b06), CP_LPAC_RB_WPTR
(0x0b07), and CP_LPAC_CHICKEN_DBG (0x0b30) use open-ended variants="A7XX-",
which includes A8XX. However, on A8XX- the LPAC ringbuffer registers are at
0x080b-0x0812 and offsets 0x0b00-0x0b01 are reassigned:
<reg64 offset="0x080b" name="CP_RB_BASE_LPAC" variants="A8XX-"/>
<reg32 offset="0x080d" name="CP_RB_CNTL_LPAC" variants="A8XX-"/>
<reg32 offset="0x080e" name="CP_RB_RPTR_WR_LPAC" variants="A8XX-"/>
<reg64 offset="0x080f" name="CP_RB_RPTR_ADDR_LPAC" variants="A8XX-"/>
<reg32 offset="0x0811" name="CP_RB_RPTR_LPAC" variants="A8XX-"/>
<reg32 offset="0x0812" name="CP_RB_WPTR_LPAC" variants="A8XX-"/>
...
<reg32 offset="0x0b00" name="CP_SLICE_MEM_POOL_DBG_ADDR_PIPE" variants="A8XX-"/>
<reg32 offset="0x0b01" name="CP_SLICE_MEM_POOL_DBG_DATA_PIPE" variants="A8XX-"/>
Other A7XX LPAC registers in the 0x0b0a-0x0b40 range use variants="A7XX" or
variants="A6XX-A7XX".
Finally, CP_LPAC_PROTECT_CNTL at offset 0x0B09 omits a variants attribute
(exposing it on A8XX) and duplicates the bitfields inline instead of using
type="a6xx_cp_protect_cntl" defined immediately above it:
<bitset name="a6xx_cp_protect_cntl" inline="yes">
<bitfield pos="3" name="LAST_SPAN_INF_RANGE" type="boolean"/>
<bitfield pos="1" name="ACCESS_FAULT_ON_VIOL_EN" type="boolean"/>
<bitfield pos="0" name="ACCESS_PROT_EN" type="boolean"/>
</bitset>
<reg32 offset="0x084f" name="CP_PROTECT_CNTL" type="a6xx_cp_protect_cntl" variants="A6XX-A7XX"/>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850961@gmail.com?part=8
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RFC v2 09/14] drm/msm: initialize LPAC ring
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
` (7 preceding siblings ...)
2026-10-29 13:52 ` [PATCH RFC v2 08/14] temp: add LPAC regs Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 14:33 ` sashiko-bot
2026-10-29 13:52 ` [PATCH RFC v2 10/14] drm/msm: Add LPAC submitqueue Anna Maniscalco
` (4 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
Program all the register needed to start up LPAC and send initialization
sequence.
Additionally, now that the LPAC ring is live, make the code aware of it.
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 1 +
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 107 +++++++++++++++++++++++++++--
drivers/gpu/drm/msm/adreno/adreno_device.c | 6 ++
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 34 +++++----
drivers/gpu/drm/msm/msm_gpu.c | 47 ++++++++++---
drivers/gpu/drm/msm/msm_gpu.h | 10 +++
6 files changed, 179 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
index 3e6f409d13a2..5ee00eead14c 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
@@ -1394,6 +1394,7 @@ static const u32 a750_ifpc_reglist_regs[] = {
REG_A6XX_SP_NC_MODE_CNTL,
REG_A6XX_CP_DBG_ECO_CNTL,
REG_A6XX_CP_PROTECT_CNTL,
+ REG_A6XX_CP_LPAC_PROTECT_CNTL,
REG_A6XX_CP_PROTECT(0),
REG_A6XX_CP_PROTECT(1),
REG_A6XX_CP_PROTECT(2),
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 8b3bb2fd433b..e7bc12de650c 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -168,6 +168,18 @@ void a6xx_flush(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
update_shadow_rptr(gpu, ring);
+ if (ring == gpu->lpac_rb) {
+ /* Copy the shadow to the actual register */
+ ring->cur = ring->next;
+
+ /* Make sure to wrap wptr if we need to */
+ wptr = get_wptr(ring);
+
+ a6xx_fenced_write(a6xx_gpu, REG_A7XX_CP_LPAC_RB_WPTR, wptr, BIT(3), false);
+
+ return;
+ }
+
spin_lock_irqsave(&ring->preempt_lock, flags);
/* Copy the shadow to the actual register */
@@ -730,6 +742,13 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu)
A6XX_CP_PROTECT_CNTL_ACCESS_FAULT_ON_VIOL_EN |
A6XX_CP_PROTECT_CNTL_LAST_SPAN_INF_RANGE);
+ if (gpu->lpac_rb) {
+ gpu_write(gpu, REG_A6XX_CP_LPAC_PROTECT_CNTL,
+ A6XX_CP_PROTECT_CNTL_ACCESS_PROT_EN |
+ A6XX_CP_PROTECT_CNTL_ACCESS_FAULT_ON_VIOL_EN |
+ A6XX_CP_PROTECT_CNTL_LAST_SPAN_INF_RANGE);
+ }
+
for (i = 0; i < protect->count - 1; i++) {
/* Intentionally skip writing to some registers */
if (protect->regs[i])
@@ -972,6 +991,53 @@ static int a7xx_cp_init(struct msm_gpu *gpu)
return a6xx_idle(gpu, ring) ? 0 : -EINVAL;
}
+static int lpac_cp_init(struct msm_gpu *gpu)
+{
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+ struct msm_ringbuffer *ring = gpu->lpac_rb;
+ u32 mask;
+
+ OUT_PKT7(ring, CP_ME_INIT, 7);
+
+ /* Use multiple HW contexts */
+ mask = BIT(0);
+
+ /* Enable error detection */
+ mask |= BIT(1);
+
+ /* Set default reset state */
+ mask |= BIT(3);
+
+ /* Disable save/restore of performance counters across preemption */
+ mask |= BIT(6);
+
+ /* Enable the register init list with the spinlock */
+ mask |= BIT(8);
+
+ OUT_RING(ring, mask);
+
+ /* Enable multiple hardware contexts */
+ OUT_RING(ring, 0x00000003);
+
+ /* Enable error detection */
+ OUT_RING(ring, 0x20000000);
+
+ /* Operation mode mask */
+ OUT_RING(ring, 0x00000002);
+
+ /* *Don't* send a power up reg list for concurrent binning (TODO) */
+ /* Lo address */
+ OUT_RING(ring, lower_32_bits(a6xx_gpu->pwrup_reglist_iova));
+ /* Hi address */
+ OUT_RING(ring, upper_32_bits(a6xx_gpu->pwrup_reglist_iova));
+ /* BIT(31) set => read the regs from the list */
+ OUT_RING(ring, BIT(31));
+
+ a6xx_flush(gpu, ring);
+ return a6xx_idle(gpu, ring) ? 0 : -EINVAL;
+}
+
/*
* Check that the microcode version is new enough to include several key
* security fixes. Return true if the ucode is safe.
@@ -1096,7 +1162,7 @@ static int a6xx_ucode_load(struct msm_gpu *gpu)
if ((adreno_gpu->base.hw_apriv || a6xx_gpu->has_whereami) &&
!a6xx_gpu->shadow_bo) {
a6xx_gpu->shadow = msm_gem_kernel_new(gpu->dev,
- sizeof(u32) * gpu->nr_rings,
+ sizeof(u32) * (gpu->nr_rings + !!gpu->lpac_rb),
MSM_BO_WC | MSM_BO_MAP_PRIV,
gpu->vm, &a6xx_gpu->shadow_bo,
&a6xx_gpu->shadow_iova);
@@ -1289,6 +1355,9 @@ static int hw_init(struct msm_gpu *gpu)
gpu_write(gpu, REG_A6XX_UCHE_CACHE_WAYS, 0x4);
}
+ if (gpu->lpac_rb)
+ gpu_rmw(gpu, REG_A6XX_UCHE_DEBUG_CNTL_1, BIT(30), BIT(30));
+
if (adreno_is_a640_family(adreno_gpu) || adreno_is_a650_family(adreno_gpu)) {
gpu_write(gpu, REG_A6XX_CP_ROQ_THRESHOLDS_2, 0x02000140);
gpu_write(gpu, REG_A6XX_CP_ROQ_THRESHOLDS_1, 0x8040362c);
@@ -1386,6 +1455,13 @@ static int hw_init(struct msm_gpu *gpu)
gpu_write(gpu, REG_A6XX_CP_CHICKEN_DBG, BIT(24));
}
+ if (gpu->lpac_rb) {
+ gpu_write(gpu, REG_A7XX_CP_LPAC_CHICKEN_DBG, 0x1);
+
+ gpu_write(gpu, REG_A7XX_SP_CHICKEN_BITS_2, BIT(4));
+ gpu_write(gpu, REG_A7XX_SP_LPAC_CHICKEN_BITS_2, BIT(4));
+ }
+
if (adreno_is_a690(adreno_gpu))
gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x90);
/* Set dualQ + disable afull for A660 GPU */
@@ -1437,13 +1513,19 @@ static int hw_init(struct msm_gpu *gpu)
/* Set the ringbuffer address */
gpu_write64(gpu, REG_A6XX_CP_RB_BASE, gpu->rb[0]->iova);
+ /* Set the ringbuffer address for lpac */
+ if (gpu->lpac_rb)
+ gpu_write64(gpu, REG_A7XX_CP_LPAC_RB_BASE, gpu->lpac_rb->iova);
+
/* Targets that support extended APRIV can use the RPTR shadow from
* hardware but all the other ones need to disable the feature. Targets
* that support the WHERE_AM_I opcode can use that instead
*/
- if (adreno_gpu->base.hw_apriv)
+ if (adreno_gpu->base.hw_apriv) {
gpu_write(gpu, REG_A6XX_CP_RB_CNTL, MSM_GPU_RB_CNTL_DEFAULT);
- else
+ if (gpu->lpac_rb)
+ gpu_write(gpu, REG_A7XX_CP_LPAC_RB_CNTL, MSM_GPU_RB_CNTL_DEFAULT);
+ } else
gpu_write(gpu, REG_A6XX_CP_RB_CNTL,
MSM_GPU_RB_CNTL_DEFAULT | AXXX_CP_RB_CNTL_NO_UPDATE);
@@ -1451,7 +1533,10 @@ static int hw_init(struct msm_gpu *gpu)
if (a6xx_gpu->shadow_bo) {
gpu_write64(gpu, REG_A6XX_CP_RB_RPTR_ADDR,
shadowptr(a6xx_gpu, gpu->rb[0]));
- for (unsigned int i = 0; i < gpu->nr_rings; i++)
+ if (gpu->lpac_rb)
+ gpu_write64(gpu, REG_A7XX_CP_LPAC_RB_RPTR_ADDR,
+ shadowptr(a6xx_gpu, gpu->lpac_rb));
+ for (unsigned int i = 0; i < (gpu->nr_rings + !!gpu->lpac_rb); i++)
a6xx_gpu->shadow[i] = 0;
}
@@ -1469,9 +1554,17 @@ static int hw_init(struct msm_gpu *gpu)
for (i = 0; i < gpu->nr_rings; i++)
gpu->rb[i]->cur_ctx_seqno = 0;
+ if (gpu->lpac_rb)
+ gpu->lpac_rb->cur_ctx_seqno = 0;
+
/* Enable the SQE_to start the CP engine */
gpu_write(gpu, REG_A6XX_CP_SQE_CNTL, 1);
+ /* Enable the LPAC SQE_to start the CP engine */
+ //TODO is this needed? Doesn't fw do this at init?
+ if (gpu->lpac_rb)
+ gpu_write(gpu, REG_A6XX_CP_LPAC_SQE_CNTL, 1);
+
if (adreno_is_a7xx(adreno_gpu) && !a6xx_gpu->pwrup_reglist_emitted) {
a7xx_patch_pwrup_reglist(gpu);
a6xx_gpu->pwrup_reglist_emitted = true;
@@ -1481,6 +1574,12 @@ static int hw_init(struct msm_gpu *gpu)
if (ret)
goto out;
+ if (gpu->lpac_rb) {
+ ret = lpac_cp_init(gpu);
+ if (ret)
+ goto out;
+ }
+
/*
* Try to load a zap shader into the secure world. If successful
* we can use the CP to switch out of secure mode. If not then we
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 4d22223c3323..6e2333466d7f 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -359,6 +359,9 @@ static void suspend_scheduler(struct msm_gpu *gpu)
drm_sched_wqueue_stop(sched);
}
+
+ if (gpu->lpac_rb)
+ drm_sched_wqueue_stop(&gpu->lpac_rb->sched);
}
static void resume_scheduler(struct msm_gpu *gpu)
@@ -370,6 +373,9 @@ static void resume_scheduler(struct msm_gpu *gpu)
drm_sched_wqueue_start(sched);
}
+
+ if (gpu->lpac_rb)
+ drm_sched_wqueue_start(&gpu->lpac_rb->sched);
}
static int adreno_system_suspend(struct device *dev)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 4389559e1d17..085e1b9044bc 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -655,6 +655,22 @@ struct drm_gem_object *adreno_fw_create_bo(struct msm_gpu *gpu,
return bo;
}
+static inline void init_ring(struct msm_ringbuffer *ring)
+{
+ ring->cur = ring->start;
+ ring->next = ring->start;
+ ring->memptrs->rptr = 0;
+ ring->memptrs->bv_fence = ring->fctx->completed_fence;
+
+ /* Detect and clean up an impossible fence, ie. if GPU managed
+ * to scribble something invalid, we don't want that to confuse
+ * us into mistakingly believing that submits have completed.
+ */
+ if (fence_before(ring->fctx->last_fence, ring->memptrs->fence)) {
+ ring->memptrs->fence = ring->fctx->last_fence;
+ }
+}
+
int adreno_hw_init(struct msm_gpu *gpu)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
@@ -682,20 +698,12 @@ int adreno_hw_init(struct msm_gpu *gpu)
if (!ring)
continue;
- ring->cur = ring->start;
- ring->next = ring->start;
- ring->memptrs->rptr = 0;
- ring->memptrs->bv_fence = ring->fctx->completed_fence;
-
- /* Detect and clean up an impossible fence, ie. if GPU managed
- * to scribble something invalid, we don't want that to confuse
- * us into mistakingly believing that submits have completed.
- */
- if (fence_before(ring->fctx->last_fence, ring->memptrs->fence)) {
- ring->memptrs->fence = ring->fctx->last_fence;
- }
+ init_ring(ring);
}
+ if (gpu->lpac_rb)
+ init_ring(gpu->lpac_rb);
+
return 0;
}
@@ -1253,6 +1261,8 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
adreno_gpu_config.nr_rings = nr_rings;
+ adreno_gpu_config.has_lpac = !!(config->info->quirks & ADRENO_QUIRK_LPAC);
+
ret = adreno_get_pwrlevels(dev, gpu);
if (ret)
goto err_put_lpac_pdev;
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 20581be10ca7..aa3d6ff456fd 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -739,8 +739,8 @@ static void retire_submits(struct msm_gpu *gpu)
int i;
/* Retire the commits starting with highest priority */
- for (i = 0; i < gpu->nr_rings; i++) {
- struct msm_ringbuffer *ring = gpu->rb[i];
+ for (i = 0; i < gpu->nr_rings + !!gpu->lpac_rb; i++) {
+ struct msm_ringbuffer *ring = i < gpu->nr_rings ? gpu->rb[i] : gpu->lpac_rb;
while (true) {
struct msm_gem_submit *submit = NULL;
@@ -782,6 +782,9 @@ void msm_gpu_retire(struct msm_gpu *gpu)
for (i = 0; i < gpu->nr_rings; i++)
msm_update_fence(gpu->rb[i]->fctx, gpu->rb[i]->memptrs->fence);
+ if (gpu->lpac_rb)
+ msm_update_fence(gpu->lpac_rb->fctx, gpu->lpac_rb->memptrs->fence);
+
kthread_queue_work(gpu->worker, &gpu->retire_work);
}
@@ -975,21 +978,21 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
}
memptrs = msm_gem_kernel_new(drm,
- sizeof(struct msm_rbmemptrs) * nr_rings,
+ sizeof(struct msm_rbmemptrs) * (nr_rings + config->has_lpac),
check_apriv(gpu, MSM_BO_WC), gpu->vm, &gpu->memptrs_bo,
&memptrs_iova);
if (IS_ERR(memptrs)) {
ret = PTR_ERR(memptrs);
- DRM_DEV_ERROR(drm->dev, "could not allocate memptrs: %d\n", ret);
- goto fail;
- }
+ DRM_DEV_ERROR(drm->dev, "could not allocate memptrs: %d\n", ret);
+ goto fail;
+ }
- msm_gem_object_set_name(gpu->memptrs_bo, "memptrs");
+ msm_gem_object_set_name(gpu->memptrs_bo, "memptrs");
- if (nr_rings > ARRAY_SIZE(gpu->rb)) {
- DRM_DEV_INFO_ONCE(drm->dev, "Only creating %zu ringbuffers\n",
- ARRAY_SIZE(gpu->rb));
+ if (nr_rings > ARRAY_SIZE(gpu->rb)) {
+ DRM_DEV_INFO_ONCE(drm->dev, "Only creating %zu ringbuffers\n",
+ ARRAY_SIZE(gpu->rb));
nr_rings = ARRAY_SIZE(gpu->rb);
}
@@ -999,6 +1002,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
if (IS_ERR(gpu->rb[i])) {
ret = PTR_ERR(gpu->rb[i]);
+ gpu->rb[i] = NULL;
DRM_DEV_ERROR(drm->dev,
"could not create ringbuffer %d: %d\n", i, ret);
goto fail;
@@ -1008,6 +1012,20 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
memptrs_iova += sizeof(struct msm_rbmemptrs);
}
+ if (config->has_lpac) {
+ gpu->lpac_rb = msm_ringbuffer_new(gpu, nr_rings, memptrs, memptrs_iova);
+
+ if (IS_ERR(gpu->lpac_rb)) {
+ ret = PTR_ERR(gpu->lpac_rb);
+ gpu->lpac_rb = NULL;
+ DRM_DEV_ERROR(drm->dev,
+ "could not create lpac ringbuffer %d\n", ret);
+ goto fail;
+ }
+
+ //IMPORTANT when rebasing on preempt aware hangcheck initialise lpac_rb hangcheck_timer
+ }
+
gpu->nr_rings = nr_rings;
refcount_set(&gpu->sysprof_active, 1);
@@ -1031,6 +1049,11 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
gpu->rb[i] = NULL;
}
+ if (gpu->lpac_rb) {
+ msm_ringbuffer_destroy(gpu->lpac_rb);
+ gpu->lpac_rb = NULL;
+ }
+
msm_gem_kernel_put(gpu->memptrs_bo, gpu->vm);
platform_set_drvdata(pdev, NULL);
@@ -1050,6 +1073,10 @@ void msm_gpu_cleanup(struct msm_gpu *gpu)
gpu->rb[i] = NULL;
}
+ if (gpu->lpac_rb)
+ msm_ringbuffer_destroy(gpu->lpac_rb);
+ gpu->lpac_rb = NULL;
+
msm_gem_kernel_put(gpu->memptrs_bo, gpu->vm);
if (!IS_ERR_OR_NULL(gpu->vm)) {
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 271956e7f870..2f6a96e560b9 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -30,6 +30,7 @@ struct msm_perfcntr_stream;
struct msm_gpu_config {
const char *ioname;
unsigned int nr_rings;
+ bool has_lpac;
};
/* So far, with hardware that I've seen to date, we can have:
@@ -181,6 +182,8 @@ struct msm_gpu {
struct msm_ringbuffer *rb[MSM_GPU_MAX_RINGS];
int nr_rings;
+ struct msm_ringbuffer *lpac_rb;
+
/**
* sysprof_active:
*
@@ -323,6 +326,13 @@ static inline bool msm_gpu_active(struct msm_gpu *gpu)
return true;
}
+ if (gpu->lpac_rb) {
+ struct msm_ringbuffer *ring = gpu->lpac_rb;
+
+ if (fence_after(ring->fctx->last_fence, ring->memptrs->fence))
+ return true;
+ }
+
return false;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RFC v2 09/14] drm/msm: initialize LPAC ring
2026-10-29 13:52 ` [PATCH RFC v2 09/14] drm/msm: initialize LPAC ring Anna Maniscalco
@ 2026-10-29 14:33 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-10-29 14:33 UTC (permalink / raw)
To: Anna Maniscalco; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] drm/msm: adreno_gpu_init() enables LPAC without checking lpac_pdev
- [Medium] drm/msm/a6xx: shadow RPTR size and clear mismatch on preempt init fallback
- [Medium] drm/msm/a6xx: unhalted LPAC SQE in a6xx_recover() and a6xx_gpu_state_get()
- [Medium] drm/msm/a6xx: missing LPAC GX registers in A7xx IFPC/pwrup reglists
--
commit 35ad03732804997c43b27fa187ec0be66e34e69c
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>
drm/msm: initialize LPAC ring
This commit programs the hardware registers required to start the LPAC ring
and sends the CP initialization sequence. It also updates ringbuffer
management, fence retirement, and scheduler suspend/resume paths to account
for the live LPAC ring.
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> index 3e6f409d13a2a..5ee00eead14c8 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> @@ -1394,6 +1394,7 @@ static const u32 a750_ifpc_reglist_regs[] = {
> REG_A6XX_SP_NC_MODE_CNTL,
> REG_A6XX_CP_DBG_ECO_CNTL,
> REG_A6XX_CP_PROTECT_CNTL,
> + REG_A6XX_CP_LPAC_PROTECT_CNTL,
[Severity: Medium]
Should the other GX domain registers programmed for LPAC in hw_init() in
drivers/gpu/drm/msm/adreno/a6xx_gpu.c also be added to the A7xx IFPC and
power-up restore lists?
Specifically, hw_init() configures these registers when gpu->lpac_rb is
present:
drivers/gpu/drm/msm/adreno/a6xx_gpu.c:hw_init() {
if (gpu->lpac_rb)
gpu_rmw(gpu, REG_A6XX_UCHE_DEBUG_CNTL_1, BIT(30), BIT(30));
...
if (gpu->lpac_rb) {
gpu_write(gpu, REG_A7XX_CP_LPAC_CHICKEN_DBG, 0x1);
gpu_write(gpu, REG_A7XX_SP_CHICKEN_BITS_2, BIT(4));
gpu_write(gpu, REG_A7XX_SP_LPAC_CHICKEN_BITS_2, BIT(4));
}
}
On IFPC-capable A7xx targets such as Adreno 750, the GX power domain
collapses across IFPC without re-running hw_init(), and the CP only restores
registers recorded in a750_ifpc_reglist_regs and a7xx_pwrup_reglist_regs by
a7xx_patch_pwrup_reglist(). Because REG_A6XX_UCHE_DEBUG_CNTL_1,
REG_A7XX_CP_LPAC_CHICKEN_DBG, REG_A7XX_SP_CHICKEN_BITS_2, and
REG_A7XX_SP_LPAC_CHICKEN_BITS_2 are not in those lists, won't their LPAC
settings revert to reset defaults after the first IFPC cycle?
[ ... ]
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 8b3bb2fd433ba..e7bc12de650ca 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
[ ... ]
> @@ -1096,7 +1162,7 @@ static int a6xx_ucode_load(struct msm_gpu *gpu)
> if ((adreno_gpu->base.hw_apriv || a6xx_gpu->has_whereami) &&
> !a6xx_gpu->shadow_bo) {
> a6xx_gpu->shadow = msm_gem_kernel_new(gpu->dev,
> - sizeof(u32) * gpu->nr_rings,
> + sizeof(u32) * (gpu->nr_rings + !!gpu->lpac_rb),
> MSM_BO_WC | MSM_BO_MAP_PRIV,
> gpu->vm, &a6xx_gpu->shadow_bo,
> &a6xx_gpu->shadow_iova);
[ ... ]
> @@ -1451,7 +1533,10 @@ static int hw_init(struct msm_gpu *gpu)
> if (a6xx_gpu->shadow_bo) {
> gpu_write64(gpu, REG_A6XX_CP_RB_RPTR_ADDR,
> shadowptr(a6xx_gpu, gpu->rb[0]));
> - for (unsigned int i = 0; i < gpu->nr_rings; i++)
> + if (gpu->lpac_rb)
> + gpu_write64(gpu, REG_A7XX_CP_LPAC_RB_RPTR_ADDR,
> + shadowptr(a6xx_gpu, gpu->lpac_rb));
> + for (unsigned int i = 0; i < (gpu->nr_rings + !!gpu->lpac_rb); i++)
> a6xx_gpu->shadow[i] = 0;
> }
[Severity: Medium]
Can a6xx_gpu->shadow[] be indexed past gpu->nr_rings + !!gpu->lpac_rb if
a6xx_preempt_init() falls back to a single ring?
During initialization in a6xx_gpu_init() -> adreno_gpu_init() ->
msm_gpu_init(), gpu->lpac_rb is created with ring->id set to nr_rings (4 when
preemption is enabled):
drivers/gpu/drm/msm/msm_gpu.c:msm_gpu_init() {
if (config->has_lpac) {
gpu->lpac_rb = msm_ringbuffer_new(gpu, nr_rings, memptrs, memptrs_iova);
}
After msm_gpu_init() returns, a6xx_gpu_init() calls a6xx_preempt_init(),
which falls back to gpu->nr_rings = 1 on allocation failure without updating
gpu->lpac_rb->id:
drivers/gpu/drm/msm/adreno/a6xx_preempt.c:a6xx_preempt_init() {
fail:
/*
* On failure to init preemption, fall back to a single
* ring (ie. no preemption)
*/
a6xx_preempt_fini(gpu);
gpu->nr_rings = 1;
}
When adreno_load_gpu() then calls a6xx_ucode_load() and hw_init(),
a6xx_gpu->shadow is sized for only 2 entries (gpu->nr_rings + !!gpu->lpac_rb)
and hw_init() only zeroes indices 0 and 1. However, shadowptr(a6xx_gpu,
gpu->lpac_rb) and a6xx_get_rptr() still index a6xx_gpu->shadow[ring->id] at
index 4:
drivers/gpu/drm/msm/adreno/a6xx_gpu.c:a6xx_get_rptr() {
if (adreno_gpu->base.hw_apriv || a6xx_gpu->has_whereami)
return a6xx_gpu->shadow[ring->id];
}
On subsequent hw_init() calls during runtime resume or GPU recovery, index 4
is not zeroed and retains its stale RPTR value (8 from the 8-dword CP_ME_INIT
packet in lpac_cp_init()), which causes adreno_idle() to see get_rptr() ==
wptr and return before the LPAC CP has drained CP_ME_INIT.
>
> @@ -1469,9 +1554,17 @@ static int hw_init(struct msm_gpu *gpu)
> for (i = 0; i < gpu->nr_rings; i++)
> gpu->rb[i]->cur_ctx_seqno = 0;
>
> + if (gpu->lpac_rb)
> + gpu->lpac_rb->cur_ctx_seqno = 0;
> +
> /* Enable the SQE_to start the CP engine */
> gpu_write(gpu, REG_A6XX_CP_SQE_CNTL, 1);
>
> + /* Enable the LPAC SQE_to start the CP engine */
> + //TODO is this needed? Doesn't fw do this at init?
> + if (gpu->lpac_rb)
> + gpu_write(gpu, REG_A6XX_CP_LPAC_SQE_CNTL, 1);
[Severity: Medium]
Now that REG_A6XX_CP_LPAC_SQE_CNTL is enabled here, should a6xx_recover() in
drivers/gpu/drm/msm/adreno/a6xx_gpu.c and a6xx_gpu_state_get() in
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c also halt the LPAC SQE by writing
3 to REG_A6XX_CP_LPAC_SQE_CNTL when gpu->lpac_rb is present?
Currently, a6xx_recover() and a6xx_gpu_state_get() only halt
REG_A6XX_CP_SQE_CNTL:
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:a6xx_gpu_state_get() {
/* Halt SQE first */
gpu_write(gpu, REG_A6XX_CP_SQE_CNTL, 3);
}
Leaving the LPAC SQE running while a6xx_gpu_state_get() reads indexed LPAC CP
registers (such as CP_LPAC_DRAW_STATE, CP_LPAC_ROQ_DBG, CP_SQE_AC_UCODE_DBG,
CP_SQE_AC_STAT, and CP_LPAC_FIFO_DBG) and executes the crashdumper, or while
a6xx_recover() prepares for reset, could result in inconsistent crash dump
state or ongoing LPAC CP activity during recovery.
[ ... ]
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 4389559e1d172..085e1b9044bc7 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
[ ... ]
> @@ -1253,6 +1261,8 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>
> adreno_gpu_config.nr_rings = nr_rings;
>
> + adreno_gpu_config.has_lpac = !!(config->info->quirks & ADRENO_QUIRK_LPAC);
[Severity: High]
What happens on a target with ADRENO_QUIRK_LPAC when booting with an older
Device Tree that lacks the qcom,lpac child node?
Earlier in adreno_gpu_init(), gpu->lpac_pdev is only populated if the
qcom,lpac child node exists:
drivers/gpu/drm/msm/adreno/adreno_gpu.c:adreno_gpu_init() {
lpac_node = of_get_compatible_child(pdev->dev.of_node, "qcom,lpac");
if (lpac_node)
gpu->lpac_pdev = of_find_device_by_node(lpac_node);
}
When gpu->lpac_pdev is NULL, adreno_iommu_create_vm() passes NULL for
lpac_dev to msm_iommu_gpu_new(), so iommu->lpac_domain is never allocated or
attached to SMMU context bank 1.
If adreno_gpu_config.has_lpac is set based solely on ADRENO_QUIRK_LPAC
without checking gpu->lpac_pdev (and adreno_hw_init() likewise calls
qcom_scm_set_gpu_smmu_lpac_aperture(1) based only on ADRENO_QUIRK_LPAC),
msm_gpu_init() will still create gpu->lpac_rb, hw_init() will initialize the
LPAC ring, and msm_submitqueue_create() will allow userspace
MSM_SUBMITQUEUE_LPAC submissions that switch to context bank 1 without an
attached IOMMU domain.
Should adreno_gpu_config.has_lpac require gpu->lpac_pdev != NULL, and should
adreno_hw_init() check gpu->lpac_rb before calling
qcom_scm_set_gpu_smmu_lpac_aperture(1)?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850961@gmail.com?part=9
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RFC v2 10/14] drm/msm: Add LPAC submitqueue
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
` (8 preceding siblings ...)
2026-10-29 13:52 ` [PATCH RFC v2 09/14] drm/msm: initialize LPAC ring Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 14:25 ` sashiko-bot
2026-10-29 13:52 ` [PATCH RFC v2 11/14] drm/msm: set ctxbank and asid based on ring Anna Maniscalco
` (3 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
Add UAPI and infostructure to support sending submission to the LPAC
ring.
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 86 ++++++++++++++++++++++-------------
drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 +-
drivers/gpu/drm/msm/msm_gem_submit.c | 6 ++-
drivers/gpu/drm/msm/msm_gpu.c | 8 ++--
drivers/gpu/drm/msm/msm_gpu.h | 4 +-
drivers/gpu/drm/msm/msm_submitqueue.c | 14 +++++-
include/uapi/drm/msm_drm.h | 2 +
7 files changed, 81 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index e7bc12de650c..e82d0c0bc10f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -204,8 +204,10 @@ void a6xx_flush(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
void
a6xx_flush_yield(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
{
+ bool is_lpac = ring == gpu->lpac_rb;
+
/* If preemption is enabled */
- if (gpu->nr_rings > 1) {
+ if (gpu->nr_rings > 1 && !is_lpac) {
/* Yield the floor on command completion */
OUT_PKT7(ring, CP_CONTEXT_SWITCH_YIELD, 4);
@@ -244,6 +246,7 @@ static void a6xx_set_pagetable(struct a6xx_gpu *a6xx_gpu,
struct drm_gpuvm *vm = msm_context_vm(submit->dev, ctx);
struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
phys_addr_t ttbr;
+ bool is_lpac = ring == a6xx_gpu->base.base.lpac_rb;
u32 asid;
u64 memptr = rbmemptr(ring, ttbr0);
@@ -261,25 +264,27 @@ static void a6xx_set_pagetable(struct a6xx_gpu *a6xx_gpu,
OUT_RING(ring, upper_32_bits(rbmemptr(ring, fence)));
OUT_RING(ring, submit->seqno - 1);
- OUT_PKT7(ring, CP_THREAD_CONTROL, 1);
- OUT_RING(ring, CP_THREAD_CONTROL_0_SYNC_THREADS | CP_SET_THREAD_BOTH);
+ if (!is_lpac) {
+ OUT_PKT7(ring, CP_THREAD_CONTROL, 1);
+ OUT_RING(ring, CP_THREAD_CONTROL_0_SYNC_THREADS | CP_SET_THREAD_BOTH);
- /* Reset state used to synchronize BR and BV */
- OUT_PKT7(ring, CP_RESET_CONTEXT_STATE, 1);
- OUT_RING(ring,
- CP_RESET_CONTEXT_STATE_0_CLEAR_ON_CHIP_TS |
- CP_RESET_CONTEXT_STATE_0_CLEAR_RESOURCE_TABLE |
- CP_RESET_CONTEXT_STATE_0_CLEAR_BV_BR_COUNTER |
- CP_RESET_CONTEXT_STATE_0_RESET_GLOBAL_LOCAL_TS);
+ /* Reset state used to synchronize BR and BV */
+ OUT_PKT7(ring, CP_RESET_CONTEXT_STATE, 1);
+ OUT_RING(ring,
+ CP_RESET_CONTEXT_STATE_0_CLEAR_ON_CHIP_TS |
+ CP_RESET_CONTEXT_STATE_0_CLEAR_RESOURCE_TABLE |
+ CP_RESET_CONTEXT_STATE_0_CLEAR_BV_BR_COUNTER |
+ CP_RESET_CONTEXT_STATE_0_RESET_GLOBAL_LOCAL_TS);
- OUT_PKT7(ring, CP_THREAD_CONTROL, 1);
- OUT_RING(ring, CP_THREAD_CONTROL_0_SYNC_THREADS | CP_SET_THREAD_BOTH);
+ OUT_PKT7(ring, CP_THREAD_CONTROL, 1);
+ OUT_RING(ring, CP_THREAD_CONTROL_0_SYNC_THREADS | CP_SET_THREAD_BOTH);
- OUT_PKT7(ring, CP_EVENT_WRITE, 1);
- OUT_RING(ring, LRZ_FLUSH_INVALIDATE);
+ OUT_PKT7(ring, CP_EVENT_WRITE, 1);
+ OUT_RING(ring, LRZ_FLUSH_INVALIDATE);
- OUT_PKT7(ring, CP_THREAD_CONTROL, 1);
- OUT_RING(ring, CP_THREAD_CONTROL_0_SYNC_THREADS | CP_SET_THREAD_BR);
+ OUT_PKT7(ring, CP_THREAD_CONTROL, 1);
+ OUT_RING(ring, CP_THREAD_CONTROL_0_SYNC_THREADS | CP_SET_THREAD_BR);
+ }
}
if (!sysprof) {
@@ -493,7 +498,10 @@ static void a7xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
struct msm_ringbuffer *ring = submit->ring;
- u32 rbbm_perfctr_cp0, cp_always_on_context;
+ u32 rbbm_perfctr_cp0, cp_always_on_context,
+ cp_always_on_counter;
+ bool is_lpac = ring == gpu->lpac_rb;
+ u64 timestamp_iova;
unsigned int i, ibs = 0;
adreno_check_and_reenable_stall(adreno_gpu);
@@ -511,19 +519,24 @@ static void a7xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
* If preemption is enabled, then set the pseudo register for the save
* sequence
*/
- if (gpu->nr_rings > 1)
+ if (gpu->nr_rings > 1 && !is_lpac)
a6xx_emit_set_pseudo_reg(ring, a6xx_gpu, submit->queue);
if (adreno_is_a8xx(adreno_gpu)) {
rbbm_perfctr_cp0 = REG_A8XX_RBBM_PERFCTR_CP(0);
cp_always_on_context = REG_A8XX_CP_ALWAYS_ON_CONTEXT;
+ cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_COUNTER;
} else {
rbbm_perfctr_cp0 = REG_A7XX_RBBM_PERFCTR_CP(0);
cp_always_on_context = REG_A6XX_CP_ALWAYS_ON_CONTEXT;
+ cp_always_on_counter = REG_A6XX_CP_ALWAYS_ON_COUNTER;
}
get_stats_counter(ring, rbbm_perfctr_cp0, rbmemptr_stats(ring, index, cpcycles_start));
- get_stats_counter(ring, cp_always_on_context, rbmemptr_stats(ring, index, alwayson_start));
+ if (is_lpac)
+ get_stats_counter(ring, cp_always_on_counter, rbmemptr_stats(ring, index, alwayson_start));
+ else
+ get_stats_counter(ring, cp_always_on_context, rbmemptr_stats(ring, index, alwayson_start));
OUT_PKT7(ring, CP_THREAD_CONTROL, 1);
OUT_RING(ring, CP_SET_THREAD_BOTH);
@@ -582,17 +595,19 @@ static void a7xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
OUT_RING(ring, submit->seqno);
}
- OUT_PKT7(ring, CP_THREAD_CONTROL, 1);
- OUT_RING(ring, CP_SET_THREAD_BR);
+ if (!is_lpac) {
+ OUT_PKT7(ring, CP_THREAD_CONTROL, 1);
+ OUT_RING(ring, CP_SET_THREAD_BR);
- OUT_PKT7(ring, CP_EVENT_WRITE, 1);
- OUT_RING(ring, CCU_INVALIDATE_DEPTH);
+ OUT_PKT7(ring, CP_EVENT_WRITE, 1);
+ OUT_RING(ring, CCU_INVALIDATE_DEPTH);
- OUT_PKT7(ring, CP_EVENT_WRITE, 1);
- OUT_RING(ring, CCU_INVALIDATE_COLOR);
+ OUT_PKT7(ring, CP_EVENT_WRITE, 1);
+ OUT_RING(ring, CCU_INVALIDATE_COLOR);
- OUT_PKT7(ring, CP_THREAD_CONTROL, 1);
- OUT_RING(ring, CP_SET_THREAD_BV);
+ OUT_PKT7(ring, CP_THREAD_CONTROL, 1);
+ OUT_RING(ring, CP_SET_THREAD_BV);
+ }
/*
* Make sure the timestamp is committed once BV pipe is
@@ -638,10 +653,12 @@ static void a7xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
a6xx_flush_yield(gpu, ring);
/* Check to see if we need to start preemption */
- if (adreno_is_a8xx(adreno_gpu))
- a8xx_preempt_trigger(gpu);
- else
- a6xx_preempt_trigger(gpu);
+ if (!is_lpac) {
+ if (adreno_is_a8xx(adreno_gpu))
+ a8xx_preempt_trigger(gpu);
+ else
+ a6xx_preempt_trigger(gpu);
+ }
}
static void a6xx_set_hwcg(struct msm_gpu *gpu, bool state)
@@ -1220,6 +1237,7 @@ int a6xx_zap_shader_init(struct msm_gpu *gpu)
A6XX_RBBM_INT_0_MASK_PM4CPINTERRUPT | \
A6XX_RBBM_INT_0_MASK_CP_RB_DONE_TS | \
A6XX_RBBM_INT_0_MASK_CP_CACHE_FLUSH_TS | \
+ A6XX_RBBM_INT_0_MASK_CP_CACHE_FLUSH_TS_LPAC | \
A6XX_RBBM_INT_0_MASK_RBBM_ATB_BUS_OVERFLOW | \
A6XX_RBBM_INT_0_MASK_RBBM_HANG_DETECT | \
A6XX_RBBM_INT_0_MASK_UCHE_OOB_ACCESS | \
@@ -2039,6 +2057,9 @@ static irqreturn_t a6xx_irq(struct msm_gpu *gpu)
a6xx_preempt_trigger(gpu);
}
+ if (status & A6XX_RBBM_INT_0_MASK_CP_CACHE_FLUSH_TS_LPAC)
+ msm_gpu_retire(gpu);
+
if (status & A6XX_RBBM_INT_0_MASK_CP_SW)
a6xx_preempt_irq(gpu);
@@ -2509,6 +2530,9 @@ static bool a6xx_progress(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
if (to_adreno_gpu(gpu)->info->quirks & ADRENO_QUIRK_IFPC)
return true;
+ if (ring == gpu->lpac_rb)
+ return true;
+
cp_state = (struct msm_cp_state) {
.ib1_base = gpu_read64(gpu, REG_A6XX_CP_IB1_BASE),
.ib2_base = gpu_read64(gpu, REG_A6XX_CP_IB2_BASE),
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
index b50c57f427b4..d85f2536551c 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
@@ -75,7 +75,7 @@ struct a6xx_gpu {
struct drm_gem_object *preempt_smmu_bo[MSM_GPU_MAX_RINGS];
void *preempt_smmu[MSM_GPU_MAX_RINGS];
uint64_t preempt_smmu_iova[MSM_GPU_MAX_RINGS];
- uint32_t last_seqno[MSM_GPU_MAX_RINGS];
+ uint32_t last_seqno[MSM_GPU_MAX_RINGS + 1];
atomic_t preempt_state;
spinlock_t eval_lock;
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 26ea8a28be47..45e7f8b72dc9 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -71,7 +71,8 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev,
submit->cmd = (void *)&submit->bos[nr_bos];
submit->queue = queue;
submit->pid = get_pid(task_pid(current));
- submit->ring = gpu->rb[queue->ring_nr];
+ submit->ring = queue->ring_nr == gpu->nr_rings ?
+ gpu->lpac_rb : gpu->rb[queue->ring_nr];
submit->fault_dumped = false;
/* Get a unique identifier for the submission for logging purposes */
@@ -599,7 +600,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
goto out_post_unlock;
}
- ring = gpu->rb[queue->ring_nr];
+ ring = queue->ring_nr == gpu->nr_rings ?
+ gpu->lpac_rb : gpu->rb[queue->ring_nr];
if (args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index aa3d6ff456fd..4244d9324cda 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -537,8 +537,8 @@ static void recover_worker(struct kthread_work *work)
* needs to happen after msm_rd_dump_submit() to ensure that the
* bo's referenced by the offending submit are still around.
*/
- for (i = 0; i < gpu->nr_rings; i++) {
- struct msm_ringbuffer *ring = gpu->rb[i];
+ for (i = 0; i < gpu->nr_rings + !!gpu->lpac_rb; i++) {
+ struct msm_ringbuffer *ring = i < gpu->nr_rings ? gpu->rb[i] : gpu->lpac_rb;
uint32_t fence = ring->memptrs->fence;
@@ -561,8 +561,8 @@ static void recover_worker(struct kthread_work *work)
* Replay all remaining submits starting with highest priority
* ring
*/
- for (i = 0; i < gpu->nr_rings; i++) {
- struct msm_ringbuffer *ring = gpu->rb[i];
+ for (i = 0; i < gpu->nr_rings + !!gpu->lpac_rb; i++) {
+ struct msm_ringbuffer *ring = i < gpu->nr_rings ? gpu->rb[i] : gpu->lpac_rb;
unsigned long flags;
spin_lock_irqsave(&ring->submit_lock, flags);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 2f6a96e560b9..877f88ef90d9 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -491,9 +491,11 @@ struct msm_context {
* create at most one &drm_sched_entity per-process per-priority-
* level.
*/
- struct drm_sched_entity *entities[NR_SCHED_PRIORITIES * MSM_GPU_MAX_RINGS];
+ struct drm_sched_entity *entities[NR_SCHED_PRIORITIES * (MSM_GPU_MAX_RINGS + 1)];
+
/**
* @ctx_mem:
+ *
* Total amount of memory of GEM buffers with handles attached for
* this context.
*/
diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c b/drivers/gpu/drm/msm/msm_submitqueue.c
index 1a5a77b28016..a0249430bd00 100644
--- a/drivers/gpu/drm/msm/msm_submitqueue.c
+++ b/drivers/gpu/drm/msm/msm_submitqueue.c
@@ -175,6 +175,7 @@ int msm_submitqueue_create(struct drm_device *drm, struct msm_context *ctx,
struct msm_drm_private *priv = drm->dev_private;
struct msm_gpu_submitqueue *queue;
enum drm_sched_priority sched_prio;
+ struct msm_ringbuffer *ring;
unsigned ring_nr;
int ret;
@@ -209,6 +210,16 @@ int msm_submitqueue_create(struct drm_device *drm, struct msm_context *ctx,
return ret;
queue = kzalloc_obj(*queue);
+
+ if (flags & MSM_SUBMITQUEUE_LPAC) {
+ ring_nr = priv->gpu->nr_rings;
+ ring = priv->gpu->lpac_rb;
+ } else {
+ ring = priv->gpu->rb[ring_nr];
+ }
+
+ if (!ring)
+ return -EINVAL;
}
if (!queue)
@@ -227,8 +238,7 @@ int msm_submitqueue_create(struct drm_device *drm, struct msm_context *ctx,
} else {
queue->ring_nr = ring_nr;
- queue->entity = get_sched_entity(ctx, priv->gpu->rb[ring_nr],
- ring_nr, sched_prio);
+ queue->entity = get_sched_entity(ctx, ring, ring_nr, sched_prio);
}
if (IS_ERR(queue->entity)) {
diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
index 7f2e594be4eb..058228a56213 100644
--- a/include/uapi/drm/msm_drm.h
+++ b/include/uapi/drm/msm_drm.h
@@ -465,10 +465,12 @@ struct drm_msm_gem_madvise {
#define MSM_SUBMITQUEUE_ALLOW_PREEMPT 0x00000001
#define MSM_SUBMITQUEUE_VM_BIND 0x00000002 /* virtual queue for VM_BIND ops */
+#define MSM_SUBMITQUEUE_LPAC 0x00000004
#define MSM_SUBMITQUEUE_FLAGS ( \
MSM_SUBMITQUEUE_ALLOW_PREEMPT | \
MSM_SUBMITQUEUE_VM_BIND | \
+ MSM_SUBMITQUEUE_LPAC | \
0)
/*
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RFC v2 10/14] drm/msm: Add LPAC submitqueue
2026-10-29 13:52 ` [PATCH RFC v2 10/14] drm/msm: Add LPAC submitqueue Anna Maniscalco
@ 2026-10-29 14:25 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-10-29 14:25 UTC (permalink / raw)
To: Anna Maniscalco; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] drm/msm: memory leak of queue in msm_submitqueue_create() when ring is NULL
- [High] drm/msm: broken GPU hang recovery and hangcheck for LPAC ring
- [Medium] drm/msm/a6xx: mismatched always-on counter for LPAC in a7xx_submit()
- [Low] drm/msm/a6xx: unused variable timestamp_iova in a7xx_submit()
--
commit b513ca2d3a9d2c681ec6e6cf4dcbe0f4500714aa
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>
drm/msm: Add LPAC submitqueue
This commit adds the MSM_SUBMITQUEUE_LPAC UAPI flag and driver support
for creating LPAC submitqueues and dispatching submissions to the LPAC
ringbuffer.
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index e7bc12de650ca..e82d0c0bc10ff 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
[ ... ]
> @@ -493,7 +498,10 @@ static void a7xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> struct msm_ringbuffer *ring = submit->ring;
> - u32 rbbm_perfctr_cp0, cp_always_on_context;
> + u32 rbbm_perfctr_cp0, cp_always_on_context,
> + cp_always_on_counter;
> + bool is_lpac = ring == gpu->lpac_rb;
> + u64 timestamp_iova;
[Severity: Low]
Is timestamp_iova needed in a7xx_submit()? It appears to be unused, which
triggers an unused-variable warning at build time (and breaks the build
when CONFIG_WERROR is enabled).
> unsigned int i, ibs = 0;
>
> adreno_check_and_reenable_stall(adreno_gpu);
> @@ -511,19 +519,24 @@ static void a7xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
> * If preemption is enabled, then set the pseudo register for the save
> * sequence
> */
> - if (gpu->nr_rings > 1)
> + if (gpu->nr_rings > 1 && !is_lpac)
> a6xx_emit_set_pseudo_reg(ring, a6xx_gpu, submit->queue);
>
> if (adreno_is_a8xx(adreno_gpu)) {
> rbbm_perfctr_cp0 = REG_A8XX_RBBM_PERFCTR_CP(0);
> cp_always_on_context = REG_A8XX_CP_ALWAYS_ON_CONTEXT;
> + cp_always_on_counter = REG_A8XX_CP_ALWAYS_ON_COUNTER;
> } else {
> rbbm_perfctr_cp0 = REG_A7XX_RBBM_PERFCTR_CP(0);
> cp_always_on_context = REG_A6XX_CP_ALWAYS_ON_CONTEXT;
> + cp_always_on_counter = REG_A6XX_CP_ALWAYS_ON_COUNTER;
> }
>
> get_stats_counter(ring, rbbm_perfctr_cp0, rbmemptr_stats(ring, index, cpcycles_start));
> - get_stats_counter(ring, cp_always_on_context, rbmemptr_stats(ring, index, alwayson_start));
> + if (is_lpac)
> + get_stats_counter(ring, cp_always_on_counter, rbmemptr_stats(ring, index, alwayson_start));
> + else
> + get_stats_counter(ring, cp_always_on_context, rbmemptr_stats(ring, index, alwayson_start));
[Severity: Medium]
Should the corresponding alwayson_end sample in a7xx_submit() also use
cp_always_on_counter when is_lpac is true?
After emitting the IB commands, a7xx_submit() still unconditionally
reads cp_always_on_context:
get_stats_counter(ring, rbbm_perfctr_cp0, rbmemptr_stats(ring, index, cpcycles_end));
get_stats_counter(ring, cp_always_on_context, rbmemptr_stats(ring, index, alwayson_end));
When retire_submit() later computes:
elapsed = (stats->alwayson_end - stats->alwayson_start) * 10000;
for an LPAC submission, it will subtract CP_ALWAYS_ON_COUNTER from
CP_ALWAYS_ON_CONTEXT, corrupting the per-submit elapsed time,
ctx->elapsed_ns, and frequency statistics.
[ ... ]
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index aa3d6ff456fda..4244d9324cda7 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -537,8 +537,8 @@ static void recover_worker(struct kthread_work *work)
> * needs to happen after msm_rd_dump_submit() to ensure that the
> * bo's referenced by the offending submit are still around.
> */
> - for (i = 0; i < gpu->nr_rings; i++) {
> - struct msm_ringbuffer *ring = gpu->rb[i];
> + for (i = 0; i < gpu->nr_rings + !!gpu->lpac_rb; i++) {
> + struct msm_ringbuffer *ring = i < gpu->nr_rings ? gpu->rb[i] : gpu->lpac_rb;
>
> uint32_t fence = ring->memptrs->fence;
>
> @@ -561,8 +561,8 @@ static void recover_worker(struct kthread_work *work)
> * Replay all remaining submits starting with highest priority
> * ring
> */
> - for (i = 0; i < gpu->nr_rings; i++) {
> - struct msm_ringbuffer *ring = gpu->rb[i];
> + for (i = 0; i < gpu->nr_rings + !!gpu->lpac_rb; i++) {
> + struct msm_ringbuffer *ring = i < gpu->nr_rings ? gpu->rb[i] : gpu->lpac_rb;
> unsigned long flags;
>
> spin_lock_irqsave(&ring->submit_lock, flags);
[Severity: High]
How does recover_worker() handle a hang or fault originating on
gpu->lpac_rb?
At the start of recover_worker(), cur_ring is looked up via
gpu->funcs->active_ring(gpu) (a6xx_active_ring()), which returns
a6xx_gpu->cur_ring:
struct msm_ringbuffer *cur_ring = gpu->funcs->active_ring(gpu);
...
submit = find_submit(cur_ring, cur_ring->memptrs->fence + 1);
if (!submit)
goto out_unlock;
Because a6xx_gpu->cur_ring only tracks the 3D rings (gpu->rb[]) and is
never set to gpu->lpac_rb, two things can go wrong when an LPAC
submission triggers a6xx_fault_detect_irq() or a7xx_sw_fuse_violation_irq():
1. If cur_ring is idle, find_submit(cur_ring, ...) returns NULL and
recover_worker() bails out via goto out_unlock without running
gpu->funcs->recover(gpu), after a6xx_fault_detect_irq() has already
written 0 to REG_A6XX_RBBM_INT_0_MASK, leaving GPU interrupts disabled.
2. If cur_ring has an unrelated 3D submit in flight, recover_worker()
attributes the fault to that 3D submit, marks its VM unusable, and in
the fence update loop:
if (ring == cur_ring)
ring->memptrs->fence = ++fence;
advances cur_ring->memptrs->fence while leaving
gpu->lpac_rb->memptrs->fence unadvanced, so the replay loop re-submits
the hung LPAC job.
Similarly, hangcheck_handler() only checks gpu->funcs->active_ring(gpu),
and in a6xx_irq():
if (priv->disable_err_irq)
status &= A6XX_RBBM_INT_0_MASK_CP_CACHE_FLUSH_TS;
A6XX_RBBM_INT_0_MASK_CP_CACHE_FLUSH_TS_LPAC is masked out when
disable_err_irq is set, dropping LPAC completion interrupts.
> diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c b/drivers/gpu/drm/msm/msm_submitqueue.c
> index 1a5a77b280163..a0249430bd005 100644
> --- a/drivers/gpu/drm/msm/msm_submitqueue.c
> +++ b/drivers/gpu/drm/msm/msm_submitqueue.c
[ ... ]
> @@ -209,6 +210,16 @@ int msm_submitqueue_create(struct drm_device *drm, struct msm_context *ctx,
> return ret;
>
> queue = kzalloc_obj(*queue);
> +
> + if (flags & MSM_SUBMITQUEUE_LPAC) {
> + ring_nr = priv->gpu->nr_rings;
> + ring = priv->gpu->lpac_rb;
> + } else {
> + ring = priv->gpu->rb[ring_nr];
> + }
> +
> + if (!ring)
> + return -EINVAL;
> }
[Severity: High]
Does msm_submitqueue_create() leak queue when ring is NULL?
In the non-MSM_SUBMITQUEUE_VM_BIND path (reachable from userspace via
DRM_IOCTL_MSM_SUBMITQUEUE_NEW -> msm_ioctl_submitqueue_new()), queue is
allocated with kzalloc_obj(*queue) before checking whether ring is
non-NULL. If userspace passes MSM_SUBMITQUEUE_LPAC on a GPU where
priv->gpu->lpac_rb is NULL, returning -EINVAL here leaks queue (and if
kzalloc_obj() also returned NULL, checking !ring before !queue returns
-EINVAL instead of -ENOMEM). Should the ring lookup and !ring check happen
before allocating queue?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850961@gmail.com?part=10
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RFC v2 11/14] drm/msm: set ctxbank and asid based on ring
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
` (9 preceding siblings ...)
2026-10-29 13:52 ` [PATCH RFC v2 10/14] drm/msm: Add LPAC submitqueue Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 13:52 ` [PATCH RFC v2 12/14] drm/msm: add lpac ring to devcoredump Anna Maniscalco
` (2 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
qcom-arm-smmu always maps asid and ctxbank 1:1 and lpac is always 1
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index e82d0c0bc10f..57fc4035a753 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -247,7 +247,7 @@ static void a6xx_set_pagetable(struct a6xx_gpu *a6xx_gpu,
struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
phys_addr_t ttbr;
bool is_lpac = ring == a6xx_gpu->base.base.lpac_rb;
- u32 asid;
+ u32 asid, ctxbank;
u64 memptr = rbmemptr(ring, ttbr0);
if (ctx->seqno == ring->cur_ctx_seqno)
@@ -256,6 +256,9 @@ static void a6xx_set_pagetable(struct a6xx_gpu *a6xx_gpu,
if (msm_iommu_pagetable_params(to_msm_vm(vm)->mmu, &ttbr, &asid))
return;
+ /* qcom-arm-smmu always maps asid and ctxbank 1:1 */
+ ctxbank = asid = is_lpac;
+
if (adreno_gpu->info->family >= ADRENO_7XX_GEN1) {
/* Wait for previous submit to complete before continuing: */
OUT_PKT7(ring, CP_WAIT_TIMESTAMP, 4);
@@ -313,7 +316,7 @@ static void a6xx_set_pagetable(struct a6xx_gpu *a6xx_gpu,
CP_SMMU_TABLE_UPDATE_1_TTBR0_HI(upper_32_bits(ttbr)) |
CP_SMMU_TABLE_UPDATE_1_ASID(asid));
OUT_RING(ring, CP_SMMU_TABLE_UPDATE_2_CONTEXTIDR(0));
- OUT_RING(ring, CP_SMMU_TABLE_UPDATE_3_CONTEXTBANK(0));
+ OUT_RING(ring, CP_SMMU_TABLE_UPDATE_3_CONTEXTBANK(ctxbank));
/*
* Write the new TTBR0 to the memstore. This is good for debugging.
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH RFC v2 12/14] drm/msm: add lpac ring to devcoredump
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
` (10 preceding siblings ...)
2026-10-29 13:52 ` [PATCH RFC v2 11/14] drm/msm: set ctxbank and asid based on ring Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 13:52 ` [PATCH RFC v2 13/14] drm/msm: gate lpac behind modparam Anna Maniscalco
2026-10-29 13:52 ` [PATCH RFC v2 14/14] drm/msm: enable LPAC on gen70900 Anna Maniscalco
13 siblings, 0 replies; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
Add the LPAC ring state to the devcoredump for debugging.
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 19 ++++++++++---------
drivers/gpu/drm/msm/msm_gpu.h | 2 +-
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 085e1b9044bc..1ce172e5f73a 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -785,25 +785,26 @@ int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
ktime_get_real_ts64(&state->time);
- for (i = 0; i < gpu->nr_rings; i++) {
+ for (i = 0; i < gpu->nr_rings + !!gpu->lpac_rb; i++) {
+ struct msm_ringbuffer *ring = i < gpu->nr_rings ? gpu->rb[i] : gpu->lpac_rb;
int size = 0, j;
- state->ring[i].fence = gpu->rb[i]->memptrs->fence;
- state->ring[i].iova = gpu->rb[i]->iova;
- state->ring[i].seqno = gpu->rb[i]->fctx->last_fence;
- state->ring[i].rptr = get_rptr(adreno_gpu, gpu->rb[i]);
- state->ring[i].wptr = get_wptr(gpu->rb[i]);
+ state->ring[i].fence = ring->memptrs->fence;
+ state->ring[i].iova = ring->iova;
+ state->ring[i].seqno = ring->fctx->last_fence;
+ state->ring[i].rptr = get_rptr(adreno_gpu, ring);
+ state->ring[i].wptr = get_wptr(ring);
/* Copy at least 'wptr' dwords of the data */
size = state->ring[i].wptr;
/* After wptr find the last non zero dword to save space */
for (j = state->ring[i].wptr; j < MSM_GPU_RINGBUFFER_SZ >> 2; j++)
- if (gpu->rb[i]->start[j])
+ if (ring->start[j])
size = j + 1;
if (size) {
- state->ring[i].data = kvmemdup(gpu->rb[i]->start, size << 2, GFP_KERNEL);
+ state->ring[i].data = kvmemdup(ring->start, size << 2, GFP_KERNEL);
if (state->ring[i].data)
state->ring[i].data_size = size << 2;
}
@@ -1004,7 +1005,7 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
drm_puts(p, "ringbuffer:\n");
- for (i = 0; i < gpu->nr_rings; i++) {
+ for (i = 0; i < gpu->nr_rings + !!gpu->lpac_rb; i++) {
drm_printf(p, " - id: %d\n", i);
drm_printf(p, " iova: 0x%016llx\n", state->ring[i].iova);
drm_printf(p, " last-fence: %u\n", state->ring[i].seqno);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 877f88ef90d9..6b05c2d45bb3 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -640,7 +640,7 @@ struct msm_gpu_state {
void *data;
int data_size;
bool encoded;
- } ring[MSM_GPU_MAX_RINGS];
+ } ring[MSM_GPU_MAX_RINGS+1];
int nr_registers;
u32 *registers;
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH RFC v2 13/14] drm/msm: gate lpac behind modparam
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
` (11 preceding siblings ...)
2026-10-29 13:52 ` [PATCH RFC v2 12/14] drm/msm: add lpac ring to devcoredump Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 14:27 ` sashiko-bot
2026-10-29 13:52 ` [PATCH RFC v2 14/14] drm/msm: enable LPAC on gen70900 Anna Maniscalco
13 siblings, 1 reply; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
Add a mod param to allow initializing and exposing the LPAC ring.
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 4 ++++
drivers/gpu/drm/msm/msm_gpu.c | 3 ++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 6e2333466d7f..51781a2e8a00 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -30,6 +30,10 @@ static bool skip_gpu;
MODULE_PARM_DESC(no_gpu, "Disable GPU driver register (0=enable GPU driver register (default), 1=skip GPU driver register");
module_param(skip_gpu, bool, 0400);
+int enable_lpac = 0;
+MODULE_PARM_DESC(enable_lpac, "Enable LPAC (A7xx+ only) (1=on , 0=disable, -1=auto (default))");
+module_param(enable_lpac, int, 0600);
+
extern const struct adreno_gpulist a2xx_gpulist;
extern const struct adreno_gpulist a3xx_gpulist;
extern const struct adreno_gpulist a4xx_gpulist;
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 4244d9324cda..f6927c797fa5 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -889,6 +889,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
const char *name, struct msm_gpu_config *config)
{
struct msm_drm_private *priv = drm->dev_private;
+ extern int enable_lpac;
int i, ret, nr_rings = config->nr_rings;
void *memptrs;
uint64_t memptrs_iova;
@@ -1012,7 +1013,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
memptrs_iova += sizeof(struct msm_rbmemptrs);
}
- if (config->has_lpac) {
+ if (config->has_lpac && enable_lpac != 0) {
gpu->lpac_rb = msm_ringbuffer_new(gpu, nr_rings, memptrs, memptrs_iova);
if (IS_ERR(gpu->lpac_rb)) {
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RFC v2 13/14] drm/msm: gate lpac behind modparam
2026-10-29 13:52 ` [PATCH RFC v2 13/14] drm/msm: gate lpac behind modparam Anna Maniscalco
@ 2026-10-29 14:27 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-10-29 14:27 UTC (permalink / raw)
To: Anna Maniscalco; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] drm/msm: broken enable_lpac default and gating logic in msm_gpu_init()
- [High] drm/msm: unfreed queue in msm_submitqueue_create() when lpac_rb is NULL
- [Low] drm/msm: missing kernel-parameters.txt documentation for enable_lpac
--
commit 1335ae6013ec105c8c18047306da8e49457285e5
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>
drm/msm: gate lpac behind modparam
This commit adds the enable_lpac module parameter in adreno_device.c and
checks it in msm_gpu_init() before creating the LPAC ringbuffer.
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 6e2333466d7f1..51781a2e8a00e 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -30,6 +30,10 @@ static bool skip_gpu;
> MODULE_PARM_DESC(no_gpu, "Disable GPU driver register (0=enable GPU driver register (default), 1=skip GPU driver register");
> module_param(skip_gpu, bool, 0400);
>
> +int enable_lpac = 0;
> +MODULE_PARM_DESC(enable_lpac, "Enable LPAC (A7xx+ only) (1=on , 0=disable, -1=auto (default))");
> +module_param(enable_lpac, int, 0600);
[Severity: Low]
Should this new module parameter also be documented in
Documentation/admin-guide/kernel-parameters.txt?
[ ... ]
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 4244d9324cda7..f6927c797fa5c 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
[ ... ]
> @@ -1012,7 +1013,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> memptrs_iova += sizeof(struct msm_rbmemptrs);
> }
>
> - if (config->has_lpac) {
> + if (config->has_lpac && enable_lpac != 0) {
[Severity: High]
Does the default value and tri-state logic for enable_lpac match the
documented behavior?
In drivers/gpu/drm/msm/adreno/adreno_device.c, enable_lpac is initialized
to 0, whereas MODULE_PARM_DESC() documents -1 (auto) as the default:
int enable_lpac = 0;
MODULE_PARM_DESC(enable_lpac, "Enable LPAC (A7xx+ only) (1=on , 0=disable, -1=auto (default))");
Because enable_lpac defaults to 0 instead of -1, LPAC remains disabled by
default even on targets that set ADRENO_QUIRK_LPAC (such as gen70900 in
the subsequent patch "drm/msm: enable LPAC on gen70900").
In addition, adreno_gpu_init() in drivers/gpu/drm/msm/adreno/adreno_gpu.c
computes has_lpac solely from ADRENO_QUIRK_LPAC:
adreno_gpu_config.has_lpac = !!(config->info->quirks & ADRENO_QUIRK_LPAC);
and msm_gpu_init() requires both config->has_lpac and enable_lpac != 0.
Unlike enable_preemption in a6xx_gpu_init(), this does not distinguish
between 1 (force on) and -1 (auto based on ADRENO_QUIRK_LPAC), so passing
msm.enable_lpac=1 cannot enable LPAC on an A7xx+ GPU unless
ADRENO_QUIRK_LPAC is already set in the catalog.
Furthermore, because enable_lpac is only checked when creating
gpu->lpac_rb in msm_gpu_init(), other LPAC paths remain out of sync with
the parameter:
1. msm_gpu_init() still allocates memptrs using config->has_lpac:
memptrs = msm_gem_kernel_new(drm,
sizeof(struct msm_rbmemptrs) * (nr_rings + config->has_lpac),
2. adreno_hw_init() in drivers/gpu/drm/msm/adreno/adreno_gpu.c still
configures the LPAC SMMU aperture based on ADRENO_QUIRK_LPAC rather than
whether LPAC is enabled:
if (info->quirks & ADRENO_QUIRK_LPAC) {
ret = qcom_scm_set_gpu_smmu_lpac_aperture(1);
if (ret)
DRM_DEV_ERROR(gpu->dev->dev, "unable to set lpac SMMU aperture: %d\n", ret);
}
Should enable_lpac be initialized to -1 and evaluated when setting
config->has_lpac (and checked consistently in adreno_hw_init())?
> gpu->lpac_rb = msm_ringbuffer_new(gpu, nr_rings, memptrs, memptrs_iova);
[Severity: High]
When enable_lpac is 0, msm_gpu_init() leaves gpu->lpac_rb as NULL. Can
this cause msm_submitqueue_create() to leak the queue allocation when
userspace requests an LPAC submitqueue?
In drivers/gpu/drm/msm/msm_submitqueue.c, msm_ioctl_submitqueue_new() ->
msm_submitqueue_create() allocates queue before checking whether ring
(priv->gpu->lpac_rb) is NULL:
queue = kzalloc_obj(*queue);
if (flags & MSM_SUBMITQUEUE_LPAC) {
ring_nr = priv->gpu->nr_rings;
ring = priv->gpu->lpac_rb;
} else {
ring = priv->gpu->rb[ring_nr];
}
if (!ring)
return -EINVAL;
When priv->gpu->lpac_rb is NULL and userspace passes MSM_SUBMITQUEUE_LPAC
to DRM_IOCTL_MSM_SUBMITQUEUE_NEW, msm_submitqueue_create() returns -EINVAL
without freeing queue.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850961@gmail.com?part=13
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH RFC v2 14/14] drm/msm: enable LPAC on gen70900
2026-10-29 13:52 [RFC PATCH v2 00/14] Enable LPAC on a7xx series GPUs Anna Maniscalco
` (12 preceding siblings ...)
2026-10-29 13:52 ` [PATCH RFC v2 13/14] drm/msm: gate lpac behind modparam Anna Maniscalco
@ 2026-10-29 13:52 ` Anna Maniscalco
2026-10-29 14:45 ` sashiko-bot
13 siblings, 1 reply; 26+ messages in thread
From: Anna Maniscalco @ 2026-10-29 13:52 UTC (permalink / raw)
To: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel, Sean Paul,
Konrad Dybcio, Akhil P Oommen, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, David Airlie, Simona Vetter,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel, dri-devel,
freedreno, devicetree, Anna Maniscalco
When the enable_lpac mod param is left on auto (-1) LPAC will be enabled
for targets that are known to support it and are known to work.
gen70900 (Adreno 750) is one such target.
Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
---
drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
index 5ee00eead14c..cd1bdb54429d 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
@@ -1591,7 +1591,8 @@ static const struct adreno_info a7xx_gpus[] = {
.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
ADRENO_QUIRK_HAS_HW_APRIV |
ADRENO_QUIRK_PREEMPTION |
- ADRENO_QUIRK_IFPC,
+ ADRENO_QUIRK_IFPC |
+ ADRENO_QUIRK_LPAC,
.funcs = &a7xx_gpu_funcs,
.zapfw = "gen70900_zap.mbn",
.a6xx = &(const struct a6xx_info) {
--
2.56.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH RFC v2 14/14] drm/msm: enable LPAC on gen70900
2026-10-29 13:52 ` [PATCH RFC v2 14/14] drm/msm: enable LPAC on gen70900 Anna Maniscalco
@ 2026-10-29 14:45 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-10-29 14:45 UTC (permalink / raw)
To: Anna Maniscalco; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] drm/msm: LPAC enabled on gen70900 without checking lpac_pdev or SW fuse
- [High] drm/msm: a6xx_active_ring() and suspend ignore lpac_rb when LPAC is enabled
- [High] drm/msm: wrong runtime PM device checked for LPAC in msm_iommu_tlb_flush_all()
- [High] drm/msm: failed parent-to-child device link skips LPAC SMMU TLB flushes
- [Medium] drm/msm: LPAC not enabled on gen70900 due to enable_lpac defaulting to 0
- [Medium] drm/msm: mismatched always-on counter reads for LPAC in a7xx_submit()
- [Medium] drm/msm/adreno: missing LPAC GX registers in A750 IFPC restore lists
--
commit 80cc6b84cce3401809bd7592d96a060ab6cb7da3
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>
drm/msm: enable LPAC on gen70900
This commit adds ADRENO_QUIRK_LPAC to the gen70900 (Adreno 750) GPU entry in
a7xx_gpus[] so that LPAC is enabled on targets known to support it.
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> index 5ee00eead14c8..cd1bdb54429d3 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> @@ -1591,7 +1591,8 @@ static const struct adreno_info a7xx_gpus[] = {
> .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> ADRENO_QUIRK_HAS_HW_APRIV |
> ADRENO_QUIRK_PREEMPTION |
> - ADRENO_QUIRK_IFPC,
> + ADRENO_QUIRK_IFPC |
> + ADRENO_QUIRK_LPAC,
[Severity: Medium]
Does setting ADRENO_QUIRK_LPAC here actually enable LPAC by default on
gen70900 when enable_lpac is not explicitly passed by the user?
In drivers/gpu/drm/msm/adreno/adreno_device.c, enable_lpac is initialized to
0 instead of -1:
int enable_lpac = 0;
MODULE_PARM_DESC(enable_lpac, "Enable LPAC (A7xx+ only) (1=on , 0=disable, -1=auto (default))");
module_param(enable_lpac, int, 0600);
When adreno_gpu_init() in drivers/gpu/drm/msm/adreno/adreno_gpu.c sets:
adreno_gpu_config.has_lpac = !!(config->info->quirks & ADRENO_QUIRK_LPAC);
msm_gpu_init() in drivers/gpu/drm/msm/msm_gpu.c allocates an extra
msm_rbmemptrs slot for config->has_lpac, but then skips creating
gpu->lpac_rb because enable_lpac is 0:
if (config->has_lpac && enable_lpac != 0) {
gpu->lpac_rb = msm_ringbuffer_new(gpu, nr_rings, memptrs, memptrs_iova);
Meanwhile, adreno_hw_init() in drivers/gpu/drm/msm/adreno/adreno_gpu.c only
checks the quirk rather than gpu->lpac_rb:
if (info->quirks & ADRENO_QUIRK_LPAC) {
ret = qcom_scm_set_gpu_smmu_lpac_aperture(1);
so qcom_scm_set_gpu_smmu_lpac_aperture(1) is still called during hardware
initialization even when LPAC is disabled. Additionally, because
config->has_lpac only checks ADRENO_QUIRK_LPAC, passing enable_lpac=1 cannot
force-enable LPAC on A7xx+ targets that do not set the quirk.
Should enable_lpac be initialized to -1, and should adreno_hw_init() check
gpu->lpac_rb instead of ADRENO_QUIRK_LPAC?
[Severity: High]
What happens when booting a kernel with ADRENO_QUIRK_LPAC enabled on an
older SM8650 (gen70900) device tree that lacks the qcom,lpac child node, or
on a device where firmware fuses off LPAC?
In adreno_gpu_init() in drivers/gpu/drm/msm/adreno/adreno_gpu.c,
gpu->lpac_pdev is looked up via of_get_compatible_child(pdev->dev.of_node,
"qcom,lpac"). On an older DTB without that child node, gpu->lpac_pdev is
NULL, and adreno_iommu_create_vm() passes NULL for lpac_dev:
mmu = msm_iommu_gpu_new(&pdev->dev,
gpu->lpac_pdev ? &gpu->lpac_pdev->dev : NULL,
gpu, quirks);
so iommu->lpac_domain (context bank 1) is never allocated and SID 1 remains
attached to context bank 0.
However, adreno_gpu_init() sets adreno_gpu_config.has_lpac solely from
ADRENO_QUIRK_LPAC without checking gpu->lpac_pdev, and adreno_hw_init()
unconditionally calls qcom_scm_set_gpu_smmu_lpac_aperture(1). When
gpu->lpac_rb is created and userspace submits to MSM_SUBMITQUEUE_LPAC,
a7xx_submit() -> a6xx_set_pagetable() in
drivers/gpu/drm/msm/adreno/a6xx_gpu.c executes:
/* qcom-arm-smmu always maps asid and ctxbank 1:1 */
ctxbank = asid = is_lpac;
and emits CP_SMMU_TABLE_UPDATE targeting uninitialized context bank 1 while
SID 1 still translates through context bank 0.
Similarly, in a6xx_gmu_secure_init() in
drivers/gpu/drm/msm/adreno/a6xx_gmu.c, REG_A7XX_CX_MISC_SW_FUSE_VALUE is
read after qcom_scm_gpu_init_regs() to check
A7XX_CX_MISC_SW_FUSE_VALUE_RAYTRACING, but A7XX_CX_MISC_SW_FUSE_VALUE_LPAC
is not checked before hw_init() writes REG_A6XX_CP_LPAC_SQE_CNTL and runs
lpac_cp_init(), which can trigger a7xx_sw_fuse_violation_irq() if firmware
disables the LPAC software fuse.
Should LPAC enablement verify both that gpu->lpac_pdev is present and that
A7XX_CX_MISC_SW_FUSE_VALUE_LPAC is set?
[Severity: High]
With ADRENO_QUIRK_LPAC enabled on gen70900 and gpu->lpac_rb exposed to
userspace, how do hangcheck, fault recovery, and suspend handle the LPAC
ring?
In drivers/gpu/drm/msm/adreno/a6xx_gpu.c, a6xx_active_ring() always returns
a6xx_gpu->cur_ring (one of gpu->rb[0..gpu->nr_rings - 1]) and never returns
gpu->lpac_rb:
static struct msm_ringbuffer *a6xx_active_ring(struct msm_gpu *gpu)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
return a6xx_gpu->cur_ring;
}
In drivers/gpu/drm/msm/msm_gpu.c, hangcheck_handler() only checks
gpu->funcs->active_ring(gpu) for fence progress, so a hung submission on
gpu->lpac_rb while the GFX rings are idle will not be detected by software
hangcheck.
Furthermore, if hardware fault detection (a6xx_fault_detect_irq()) or
a7xx_sw_fuse_violation_irq() triggers on an LPAC job, a6xx_fault_detect_irq()
deletes gpu->hangcheck_timer, masks all GPU interrupts by writing 0 to
REG_A6XX_RBBM_INT_0_MASK, and queues gpu->recover_work. In recover_worker()
in drivers/gpu/drm/msm/msm_gpu.c:
struct msm_ringbuffer *cur_ring = gpu->funcs->active_ring(gpu);
...
submit = find_submit(cur_ring, cur_ring->memptrs->fence + 1);
if (!submit)
goto out_unlock;
If cur_ring is idle when LPAC faults, recover_worker() exits to out_unlock
without calling gpu->funcs->recover(gpu) or restoring
REG_A6XX_RBBM_INT_0_MASK, leaving GPU interrupts masked and the GPU wedged.
If cur_ring has a concurrent GFX submission, recover_worker() blames that
GFX submit, marks its VM unusable, advances cur_ring->memptrs->fence while
leaving gpu->lpac_rb->memptrs->fence unadvanced:
for (i = 0; i < gpu->nr_rings + !!gpu->lpac_rb; i++) {
struct msm_ringbuffer *ring = i < gpu->nr_rings ? gpu->rb[i] : gpu->lpac_rb;
uint32_t fence = ring->memptrs->fence;
if (ring == cur_ring)
ring->memptrs->fence = ++fence;
msm_update_fence(ring->fctx, fence);
}
and then replays the hung LPAC submission.
Also, in a6xx_gmu_pm_suspend() and a6xx_pm_suspend() in
drivers/gpu/drm/msm/adreno/a6xx_gpu.c, the loop clearing a6xx_gpu->shadow[]
only iterates up to gpu->nr_rings:
if (a6xx_gpu->shadow_bo)
for (i = 0; i < gpu->nr_rings; i++)
a6xx_gpu->shadow[i] = 0;
leaving a6xx_gpu->shadow[gpu->nr_rings] for gpu->lpac_rb stale across
suspend.
Could hangcheck_handler(), recover_worker(), and the suspend shadow-clearing
loops be updated to account for gpu->lpac_rb?
[Severity: High]
When LPAC submissions on gen70900 switch context bank 1 (ASID 1) to
per-process TTBR0 pagetables via a6xx_set_pagetable(), will TLB invalidations
on context bank 1 be skipped when those pagetables are unmapped or destroyed?
In msm_iommu_tlb_flush_all() and msm_iommu_tlb_flush_walk() in
drivers/gpu/drm/msm/msm_iommu.c, the primary context bank flushes check
pm_runtime_get_if_in_use(pagetable->iommu_dev), whereas the LPAC flushes
check pagetable->parent->lpac_dev:
if (pagetable->parent->lpac_dev &&
pm_runtime_get_if_in_use(pagetable->parent->lpac_dev)) {
adreno_smmu = dev_get_drvdata(pagetable->parent->lpac_dev);
pagetable->tlb->tlb_flush_all((void *)adreno_smmu->cookie);
pm_runtime_put_autosuspend(pagetable->parent->lpac_dev);
}
However, in adreno_gpu_init() in drivers/gpu/drm/msm/adreno/adreno_gpu.c:
if (gpu->lpac_pdev) {
pm_runtime_enable(&gpu->lpac_pdev->dev);
if (!device_link_add(&pdev->dev, &gpu->lpac_pdev->dev,
DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_CONSUMER))
dev_err(&gpu->lpac_pdev->dev, "failed to link to lpac device\n");
}
Because gpu->lpac_pdev is populated from the child node under the GPU node in
arch/arm64/boot/dts/qcom/sm8650.dtsi via of_platform_populate(...,
&pdev->dev), &pdev->dev is the parent of &gpu->lpac_pdev->dev. Calling
device_link_add() with the parent (&pdev->dev) as consumer and its child
(&gpu->lpac_pdev->dev) as supplier fails in device_is_dependent() because
device_is_ancestor(consumer, supplier) is true.
As a result, lpac_dev has no runtime PM link, no bound driver, and no
autosuspend configuration, so it remains permanently in RPM_SUSPENDED with
usage_count == 0. Even if lpac_dev were suspended while pagetable->iommu_dev
is active, the SMMU hardware still retains cached TLB entries for context
bank 1. Because pm_runtime_get_if_in_use(pagetable->parent->lpac_dev) always
returns 0, tlb_flush_all() and tlb_flush_walk() are never called for context
bank 1 before page table pages are freed in msm_iommu_pagetable_destroy() or
__arm_lpae_unmap().
Should msm_iommu_tlb_flush_all() and msm_iommu_tlb_flush_walk() check
pagetable->iommu_dev (or shouldn't the runtime PM handling for lpac_dev be
fixed so context bank 1 TLBs are invalidated before freeing page tables)?
[Severity: Medium]
When LPAC is enabled on gen70900 alongside ADRENO_QUIRK_IFPC, are the LPAC GX
registers restored after an Inter-Frame Power Collapse (IFPC) cycle?
During hw_init() in drivers/gpu/drm/msm/adreno/a6xx_gpu.c, when gpu->lpac_rb
is present, the driver programs several GX-domain registers:
if (gpu->lpac_rb)
gpu_rmw(gpu, REG_A6XX_UCHE_DEBUG_CNTL_1, BIT(30), BIT(30));
...
if (gpu->lpac_rb) {
gpu_write(gpu, REG_A7XX_CP_LPAC_CHICKEN_DBG, 0x1);
gpu_write(gpu, REG_A7XX_SP_CHICKEN_BITS_2, BIT(4));
gpu_write(gpu, REG_A7XX_SP_LPAC_CHICKEN_BITS_2, BIT(4));
}
In drivers/gpu/drm/msm/adreno/a6xx_catalog.c, a750_ifpc_reglist_regs includes
REG_A6XX_CP_LPAC_PROTECT_CNTL, but REG_A6XX_UCHE_DEBUG_CNTL_1,
REG_A7XX_CP_LPAC_CHICKEN_DBG, REG_A7XX_SP_CHICKEN_BITS_2, and
REG_A7XX_SP_LPAC_CHICKEN_BITS_2 are absent from both a750_ifpc_reglist_regs
and a7xx_pwrup_reglist_regs, so their values will be lost when the GX domain
collapses during IFPC.
Should those four registers be added to the A750 IFPC / power-up register
lists?
> .funcs = &a7xx_gpu_funcs,
[Severity: Medium]
When gen70900 uses a7xx_gpu_funcs and submits LPAC jobs through
a7xx_submit() in drivers/gpu/drm/msm/adreno/a6xx_gpu.c, does alwayson_end
read from the wrong counter for LPAC?
At the start of a7xx_submit(), alwayson_start is sampled from
cp_always_on_counter when is_lpac is true:
if (is_lpac)
get_stats_counter(ring, cp_always_on_counter, rbmemptr_stats(ring, index, alwayson_start));
else
get_stats_counter(ring, cp_always_on_context, rbmemptr_stats(ring, index, alwayson_start));
However, at the end of a7xx_submit(), alwayson_end is unconditionally
sampled from cp_always_on_context:
get_stats_counter(ring, rbbm_perfctr_cp0, rbmemptr_stats(ring, index, cpcycles_end));
get_stats_counter(ring, cp_always_on_context, rbmemptr_stats(ring, index, alwayson_end));
When retire_submit() in drivers/gpu/drm/msm/msm_gpu.c computes:
elapsed = (stats->alwayson_end - stats->alwayson_start) * 10000;
it subtracts CP_ALWAYS_ON_COUNTER from CP_ALWAYS_ON_CONTEXT, corrupting the
elapsed time, clock estimation, trace_msm_gpu_submit_retired(), and
ctx->elapsed_ns for LPAC submissions (note also that a7xx_submit() has an
unused local variable timestamp_iova).
Should the alwayson_end sample in a7xx_submit() also branch on is_lpac to
read cp_always_on_counter?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850961@gmail.com?part=14
^ permalink raw reply [flat|nested] 26+ messages in thread