Set Keepalive votes at appropriate places to block IFPC power collapse
until we access all the required registers. This is required during gpu
IRQ handling and also during preemption.
Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 26 +++++++++++++++++---------
drivers/gpu/drm/msm/adreno/a6xx_preempt.c | 20 ++++++++++++++++++++
2 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 8c004fc3abd2896d467a9728b34e99e4ed944dc4..6770f0363e7284e4596b1188637a4615d2c0779b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1752,8 +1752,6 @@ static void a6xx_cp_hw_err_irq(struct msm_gpu *gpu)
static void a6xx_fault_detect_irq(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->funcs->active_ring(gpu);
/*
@@ -1765,13 +1763,6 @@ static void a6xx_fault_detect_irq(struct msm_gpu *gpu)
if (gpu_read(gpu, REG_A6XX_RBBM_STATUS3) & A6XX_RBBM_STATUS3_SMMU_STALLED_ON_FAULT)
return;
- /*
- * Force the GPU to stay on until after we finish
- * collecting information
- */
- if (!adreno_has_gmu_wrapper(adreno_gpu))
- gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_GMU_PWR_COL_KEEPALIVE, 1);
-
DRM_DEV_ERROR(&gpu->pdev->dev,
"gpu fault ring %d fence %x status %8.8X rb %4.4x/%4.4x ib1 %16.16llX/%4.4x ib2 %16.16llX/%4.4x\n",
ring ? ring->id : -1, ring ? ring->fctx->last_fence : 0,
@@ -1810,9 +1801,24 @@ static void a7xx_sw_fuse_violation_irq(struct msm_gpu *gpu)
}
}
+static void set_keepalive_vote(struct msm_gpu *gpu, bool on)
+{
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+
+ if (adreno_has_gmu_wrapper(adreno_gpu))
+ return;
+
+ gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_GMU_PWR_COL_KEEPALIVE, on);
+}
+
static irqreturn_t a6xx_irq(struct msm_gpu *gpu)
{
struct msm_drm_private *priv = gpu->dev->dev_private;
+
+ /* Set keepalive vote to avoid power collapse after RBBM_INT_0_STATUS is read */
+ set_keepalive_vote(gpu, true);
+
u32 status = gpu_read(gpu, REG_A6XX_RBBM_INT_0_STATUS);
gpu_write(gpu, REG_A6XX_RBBM_INT_CLEAR_CMD, status);
@@ -1849,6 +1855,8 @@ static irqreturn_t a6xx_irq(struct msm_gpu *gpu)
if (status & A6XX_RBBM_INT_0_MASK_CP_SW)
a6xx_preempt_irq(gpu);
+ set_keepalive_vote(gpu, false);
+
return IRQ_HANDLED;
}
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_preempt.c b/drivers/gpu/drm/msm/adreno/a6xx_preempt.c
index 5b0fd510ff58d989ab285f1a2497f6f522a6b187..1c8ec1911010c00a000d195116fc950c4d947cac 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_preempt.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_preempt.c
@@ -136,6 +136,21 @@ static void preempt_disable_postamble(struct a6xx_gpu *a6xx_gpu)
a6xx_gpu->postamble_enabled = false;
}
+/*
+ * Set preemption keepalive vote. Please note that this vote is different from the one used in
+ * a6xx_irq()
+ */
+static void set_keepalive_vote(struct msm_gpu *gpu, bool on)
+{
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+
+ if (adreno_has_gmu_wrapper(adreno_gpu))
+ return;
+
+ gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_PWR_COL_PREEMPT_KEEPALIVE, on);
+}
+
void a6xx_preempt_irq(struct msm_gpu *gpu)
{
uint32_t status;
@@ -176,6 +191,8 @@ void a6xx_preempt_irq(struct msm_gpu *gpu)
set_preempt_state(a6xx_gpu, PREEMPT_NONE);
+ set_keepalive_vote(gpu, false);
+
trace_msm_gpu_preemption_irq(a6xx_gpu->cur_ring->id);
/*
@@ -302,6 +319,9 @@ void a6xx_preempt_trigger(struct msm_gpu *gpu)
spin_unlock_irqrestore(&ring->preempt_lock, flags);
+ /* Set the keepalive bit to keep the GPU ON until preemption is complete */
+ set_keepalive_vote(gpu, true);
+
a6xx_fenced_write(a6xx_gpu,
REG_A6XX_CP_CONTEXT_SWITCH_SMMU_INFO, a6xx_gpu->preempt_smmu_iova[ring->id],
BIT(1), true);
--
2.50.1On Sun, Jul 20, 2025 at 05:46:09PM +0530, Akhil P Oommen wrote:
> Set Keepalive votes at appropriate places to block IFPC power collapse
> until we access all the required registers. This is required during gpu
> IRQ handling and also during preemption.
>
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 26 +++++++++++++++++---------
> drivers/gpu/drm/msm/adreno/a6xx_preempt.c | 20 ++++++++++++++++++++
> 2 files changed, 37 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 8c004fc3abd2896d467a9728b34e99e4ed944dc4..6770f0363e7284e4596b1188637a4615d2c0779b 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1752,8 +1752,6 @@ static void a6xx_cp_hw_err_irq(struct msm_gpu *gpu)
>
> static void a6xx_fault_detect_irq(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->funcs->active_ring(gpu);
>
> /*
> @@ -1765,13 +1763,6 @@ static void a6xx_fault_detect_irq(struct msm_gpu *gpu)
> if (gpu_read(gpu, REG_A6XX_RBBM_STATUS3) & A6XX_RBBM_STATUS3_SMMU_STALLED_ON_FAULT)
> return;
>
> - /*
> - * Force the GPU to stay on until after we finish
> - * collecting information
> - */
> - if (!adreno_has_gmu_wrapper(adreno_gpu))
> - gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_GMU_PWR_COL_KEEPALIVE, 1);
> -
> DRM_DEV_ERROR(&gpu->pdev->dev,
> "gpu fault ring %d fence %x status %8.8X rb %4.4x/%4.4x ib1 %16.16llX/%4.4x ib2 %16.16llX/%4.4x\n",
> ring ? ring->id : -1, ring ? ring->fctx->last_fence : 0,
> @@ -1810,9 +1801,24 @@ static void a7xx_sw_fuse_violation_irq(struct msm_gpu *gpu)
> }
> }
>
> +static void set_keepalive_vote(struct msm_gpu *gpu, bool on)
a6xx_set_keepalive_vote()
> +{
> + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> + struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> +
> + if (adreno_has_gmu_wrapper(adreno_gpu))
> + return;
> +
> + gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_GMU_PWR_COL_KEEPALIVE, on);
> +}
> +
> static irqreturn_t a6xx_irq(struct msm_gpu *gpu)
> {
> struct msm_drm_private *priv = gpu->dev->dev_private;
> +
> + /* Set keepalive vote to avoid power collapse after RBBM_INT_0_STATUS is read */
> + set_keepalive_vote(gpu, true);
> +
> u32 status = gpu_read(gpu, REG_A6XX_RBBM_INT_0_STATUS);
>
> gpu_write(gpu, REG_A6XX_RBBM_INT_CLEAR_CMD, status);
> @@ -1849,6 +1855,8 @@ static irqreturn_t a6xx_irq(struct msm_gpu *gpu)
> if (status & A6XX_RBBM_INT_0_MASK_CP_SW)
> a6xx_preempt_irq(gpu);
>
> + set_keepalive_vote(gpu, false);
> +
> return IRQ_HANDLED;
> }
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_preempt.c b/drivers/gpu/drm/msm/adreno/a6xx_preempt.c
> index 5b0fd510ff58d989ab285f1a2497f6f522a6b187..1c8ec1911010c00a000d195116fc950c4d947cac 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_preempt.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_preempt.c
> @@ -136,6 +136,21 @@ static void preempt_disable_postamble(struct a6xx_gpu *a6xx_gpu)
> a6xx_gpu->postamble_enabled = false;
> }
>
> +/*
> + * Set preemption keepalive vote. Please note that this vote is different from the one used in
> + * a6xx_irq()
> + */
> +static void set_keepalive_vote(struct msm_gpu *gpu, bool on)
a6xx_set_preempt_keepalive_vote();
> +{
> + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> + struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> +
> + if (adreno_has_gmu_wrapper(adreno_gpu))
> + return;
> +
> + gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_PWR_COL_PREEMPT_KEEPALIVE, on);
> +}
> +
> void a6xx_preempt_irq(struct msm_gpu *gpu)
> {
> uint32_t status;
> @@ -176,6 +191,8 @@ void a6xx_preempt_irq(struct msm_gpu *gpu)
>
> set_preempt_state(a6xx_gpu, PREEMPT_NONE);
>
> + set_keepalive_vote(gpu, false);
> +
> trace_msm_gpu_preemption_irq(a6xx_gpu->cur_ring->id);
>
> /*
> @@ -302,6 +319,9 @@ void a6xx_preempt_trigger(struct msm_gpu *gpu)
>
> spin_unlock_irqrestore(&ring->preempt_lock, flags);
>
> + /* Set the keepalive bit to keep the GPU ON until preemption is complete */
> + set_keepalive_vote(gpu, true);
> +
> a6xx_fenced_write(a6xx_gpu,
> REG_A6XX_CP_CONTEXT_SWITCH_SMMU_INFO, a6xx_gpu->preempt_smmu_iova[ring->id],
> BIT(1), true);
>
> --
> 2.50.1
>
--
With best wishes
Dmitry
On 7/22/2025 7:14 PM, Dmitry Baryshkov wrote:
> On Sun, Jul 20, 2025 at 05:46:09PM +0530, Akhil P Oommen wrote:
>> Set Keepalive votes at appropriate places to block IFPC power collapse
>> until we access all the required registers. This is required during gpu
>> IRQ handling and also during preemption.
>>
>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>> ---
>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 26 +++++++++++++++++---------
>> drivers/gpu/drm/msm/adreno/a6xx_preempt.c | 20 ++++++++++++++++++++
>> 2 files changed, 37 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> index 8c004fc3abd2896d467a9728b34e99e4ed944dc4..6770f0363e7284e4596b1188637a4615d2c0779b 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> @@ -1752,8 +1752,6 @@ static void a6xx_cp_hw_err_irq(struct msm_gpu *gpu)
>>
>> static void a6xx_fault_detect_irq(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->funcs->active_ring(gpu);
>>
>> /*
>> @@ -1765,13 +1763,6 @@ static void a6xx_fault_detect_irq(struct msm_gpu *gpu)
>> if (gpu_read(gpu, REG_A6XX_RBBM_STATUS3) & A6XX_RBBM_STATUS3_SMMU_STALLED_ON_FAULT)
>> return;
>>
>> - /*
>> - * Force the GPU to stay on until after we finish
>> - * collecting information
>> - */
>> - if (!adreno_has_gmu_wrapper(adreno_gpu))
>> - gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_GMU_PWR_COL_KEEPALIVE, 1);
>> -
>> DRM_DEV_ERROR(&gpu->pdev->dev,
>> "gpu fault ring %d fence %x status %8.8X rb %4.4x/%4.4x ib1 %16.16llX/%4.4x ib2 %16.16llX/%4.4x\n",
>> ring ? ring->id : -1, ring ? ring->fctx->last_fence : 0,
>> @@ -1810,9 +1801,24 @@ static void a7xx_sw_fuse_violation_irq(struct msm_gpu *gpu)
>> }
>> }
>>
>> +static void set_keepalive_vote(struct msm_gpu *gpu, bool on)
>
> a6xx_set_keepalive_vote()
static fn! Why do we need prefix here?
-Akhil
>
>> +{
>> + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
>> + struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
>> +
>> + if (adreno_has_gmu_wrapper(adreno_gpu))
>> + return;
>> +
>> + gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_GMU_PWR_COL_KEEPALIVE, on);
>> +}
>> +
>> static irqreturn_t a6xx_irq(struct msm_gpu *gpu)
>> {
>> struct msm_drm_private *priv = gpu->dev->dev_private;
>> +
>> + /* Set keepalive vote to avoid power collapse after RBBM_INT_0_STATUS is read */
>> + set_keepalive_vote(gpu, true);
>> +
>> u32 status = gpu_read(gpu, REG_A6XX_RBBM_INT_0_STATUS);
>>
>> gpu_write(gpu, REG_A6XX_RBBM_INT_CLEAR_CMD, status);
>> @@ -1849,6 +1855,8 @@ static irqreturn_t a6xx_irq(struct msm_gpu *gpu)
>> if (status & A6XX_RBBM_INT_0_MASK_CP_SW)
>> a6xx_preempt_irq(gpu);
>>
>> + set_keepalive_vote(gpu, false);
>> +
>> return IRQ_HANDLED;
>> }
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_preempt.c b/drivers/gpu/drm/msm/adreno/a6xx_preempt.c
>> index 5b0fd510ff58d989ab285f1a2497f6f522a6b187..1c8ec1911010c00a000d195116fc950c4d947cac 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_preempt.c
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_preempt.c
>> @@ -136,6 +136,21 @@ static void preempt_disable_postamble(struct a6xx_gpu *a6xx_gpu)
>> a6xx_gpu->postamble_enabled = false;
>> }
>>
>> +/*
>> + * Set preemption keepalive vote. Please note that this vote is different from the one used in
>> + * a6xx_irq()
>> + */
>> +static void set_keepalive_vote(struct msm_gpu *gpu, bool on)
>
> a6xx_set_preempt_keepalive_vote();
>
>> +{
>> + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
>> + struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
>> +
>> + if (adreno_has_gmu_wrapper(adreno_gpu))
>> + return;
>> +
>> + gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_PWR_COL_PREEMPT_KEEPALIVE, on);
>> +}
>> +
>> void a6xx_preempt_irq(struct msm_gpu *gpu)
>> {
>> uint32_t status;
>> @@ -176,6 +191,8 @@ void a6xx_preempt_irq(struct msm_gpu *gpu)
>>
>> set_preempt_state(a6xx_gpu, PREEMPT_NONE);
>>
>> + set_keepalive_vote(gpu, false);
>> +
>> trace_msm_gpu_preemption_irq(a6xx_gpu->cur_ring->id);
>>
>> /*
>> @@ -302,6 +319,9 @@ void a6xx_preempt_trigger(struct msm_gpu *gpu)
>>
>> spin_unlock_irqrestore(&ring->preempt_lock, flags);
>>
>> + /* Set the keepalive bit to keep the GPU ON until preemption is complete */
>> + set_keepalive_vote(gpu, true);
>> +
>> a6xx_fenced_write(a6xx_gpu,
>> REG_A6XX_CP_CONTEXT_SWITCH_SMMU_INFO, a6xx_gpu->preempt_smmu_iova[ring->id],
>> BIT(1), true);
>>
>> --
>> 2.50.1
>>
>
On Wed, Jul 23, 2025 at 02:54:59AM +0530, Akhil P Oommen wrote:
> On 7/22/2025 7:14 PM, Dmitry Baryshkov wrote:
> > On Sun, Jul 20, 2025 at 05:46:09PM +0530, Akhil P Oommen wrote:
> >> Set Keepalive votes at appropriate places to block IFPC power collapse
> >> until we access all the required registers. This is required during gpu
> >> IRQ handling and also during preemption.
> >>
> >> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> >> ---
> >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 26 +++++++++++++++++---------
> >> drivers/gpu/drm/msm/adreno/a6xx_preempt.c | 20 ++++++++++++++++++++
> >> 2 files changed, 37 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >> index 8c004fc3abd2896d467a9728b34e99e4ed944dc4..6770f0363e7284e4596b1188637a4615d2c0779b 100644
> >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >> @@ -1752,8 +1752,6 @@ static void a6xx_cp_hw_err_irq(struct msm_gpu *gpu)
> >>
> >> static void a6xx_fault_detect_irq(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->funcs->active_ring(gpu);
> >>
> >> /*
> >> @@ -1765,13 +1763,6 @@ static void a6xx_fault_detect_irq(struct msm_gpu *gpu)
> >> if (gpu_read(gpu, REG_A6XX_RBBM_STATUS3) & A6XX_RBBM_STATUS3_SMMU_STALLED_ON_FAULT)
> >> return;
> >>
> >> - /*
> >> - * Force the GPU to stay on until after we finish
> >> - * collecting information
> >> - */
> >> - if (!adreno_has_gmu_wrapper(adreno_gpu))
> >> - gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_GMU_PWR_COL_KEEPALIVE, 1);
> >> -
> >> DRM_DEV_ERROR(&gpu->pdev->dev,
> >> "gpu fault ring %d fence %x status %8.8X rb %4.4x/%4.4x ib1 %16.16llX/%4.4x ib2 %16.16llX/%4.4x\n",
> >> ring ? ring->id : -1, ring ? ring->fctx->last_fence : 0,
> >> @@ -1810,9 +1801,24 @@ static void a7xx_sw_fuse_violation_irq(struct msm_gpu *gpu)
> >> }
> >> }
> >>
> >> +static void set_keepalive_vote(struct msm_gpu *gpu, bool on)
> >
> > a6xx_set_keepalive_vote()
>
> static fn! Why do we need prefix here?
It's really a good custom. Also, note that I suggested two different
names to your functions (otherwise it's too easy to get confused).
> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_preempt.c b/drivers/gpu/drm/msm/adreno/a6xx_preempt.c
> >> index 5b0fd510ff58d989ab285f1a2497f6f522a6b187..1c8ec1911010c00a000d195116fc950c4d947cac 100644
> >> --- a/drivers/gpu/drm/msm/adreno/a6xx_preempt.c
> >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_preempt.c
> >> @@ -136,6 +136,21 @@ static void preempt_disable_postamble(struct a6xx_gpu *a6xx_gpu)
> >> a6xx_gpu->postamble_enabled = false;
> >> }
> >>
> >> +/*
> >> + * Set preemption keepalive vote. Please note that this vote is different from the one used in
> >> + * a6xx_irq()
> >> + */
> >> +static void set_keepalive_vote(struct msm_gpu *gpu, bool on)
> >
> > a6xx_set_preempt_keepalive_vote();
> >
--
With best wishes
Dmitry
On 7/22/25 11:24 PM, Akhil P Oommen wrote: > On 7/22/2025 7:14 PM, Dmitry Baryshkov wrote: >> On Sun, Jul 20, 2025 at 05:46:09PM +0530, Akhil P Oommen wrote: >>> Set Keepalive votes at appropriate places to block IFPC power collapse >>> until we access all the required registers. This is required during gpu >>> IRQ handling and also during preemption. >>> >>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com> >>> --- [...] >>> +static void set_keepalive_vote(struct msm_gpu *gpu, bool on) >> >> a6xx_set_keepalive_vote() > > static fn! Why do we need prefix here? It's good practice to namespace-prefix your functions, so that you can more easily find them, e.g. in a backtrace. For a prefix, adreno_gmu_ would be even better, as the register doesn't seem to have changed across generations Konrad
On 7/23/2025 3:35 PM, Konrad Dybcio wrote: > On 7/22/25 11:24 PM, Akhil P Oommen wrote: >> On 7/22/2025 7:14 PM, Dmitry Baryshkov wrote: >>> On Sun, Jul 20, 2025 at 05:46:09PM +0530, Akhil P Oommen wrote: >>>> Set Keepalive votes at appropriate places to block IFPC power collapse >>>> until we access all the required registers. This is required during gpu >>>> IRQ handling and also during preemption. >>>> >>>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com> >>>> --- > > [...] > >>>> +static void set_keepalive_vote(struct msm_gpu *gpu, bool on) >>> >>> a6xx_set_keepalive_vote() >> >> static fn! Why do we need prefix here? > > It's good practice to namespace-prefix your functions, so that you > can more easily find them, e.g. in a backtrace. It would be obvious from the parent functions in the backtrace, right? A bit subjective! I feel that the prefixes are unnecessary for small local helper fns. Prefix for *every* single routine in a source file makes the code look a little bit bloated. -Akhil. > > For a prefix, adreno_gmu_ would be even better, as the register doesn't > seem to have changed across generations > > Konrad
On Thu, Jul 24, 2025 at 02:52:52AM +0530, Akhil P Oommen wrote: > On 7/23/2025 3:35 PM, Konrad Dybcio wrote: > > On 7/22/25 11:24 PM, Akhil P Oommen wrote: > >> On 7/22/2025 7:14 PM, Dmitry Baryshkov wrote: > >>> On Sun, Jul 20, 2025 at 05:46:09PM +0530, Akhil P Oommen wrote: > >>>> Set Keepalive votes at appropriate places to block IFPC power collapse > >>>> until we access all the required registers. This is required during gpu > >>>> IRQ handling and also during preemption. > >>>> > >>>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com> > >>>> --- > > > > [...] > > > >>>> +static void set_keepalive_vote(struct msm_gpu *gpu, bool on) > >>> > >>> a6xx_set_keepalive_vote() > >> > >> static fn! Why do we need prefix here? > > > > It's good practice to namespace-prefix your functions, so that you > > can more easily find them, e.g. in a backtrace. > > It would be obvious from the parent functions in the backtrace, right? Think about jumping to a tag, etc. > > A bit subjective! I feel that the prefixes are unnecessary for small > local helper fns. Prefix for *every* single routine in a source file > makes the code look a little bit bloated. > > -Akhil. > > > > > For a prefix, adreno_gmu_ would be even better, as the register doesn't > > seem to have changed across generations > > > > Konrad > -- With best wishes Dmitry
© 2016 - 2025 Red Hat, Inc.