| Message ID | 20210127233946.1286386-3-eric@anholt.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/msm: Fix race of GPU init vs timestamp power management. | expand |
On Wed, Jan 27, 2021 at 03:39:46PM -0800, Eric Anholt wrote: > Now that the bug is fixed in the minimal way for stable, go make the > code table-driven. > > Signed-off-by: Eric Anholt <eric@anholt.net> There shouldn't be too many more OOB bits, but this is a good cleanup regardless. Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 124 +++++++++++++------------- > drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 55 ++++-------- > 2 files changed, 77 insertions(+), 102 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index 378dc7f190c3..c497e0942141 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -245,47 +245,66 @@ static int a6xx_gmu_hfi_start(struct a6xx_gmu *gmu) > return ret; > } > > +struct a6xx_gmu_oob_bits { > + int set, ack, set_new, ack_new; > + const char *name; > +}; > + > +/* These are the interrupt / ack bits for each OOB request that are set > + * in a6xx_gmu_set_oob and a6xx_clear_oob > + */ > +static const struct a6xx_gmu_oob_bits a6xx_gmu_oob_bits[] = { > + [GMU_OOB_GPU_SET] = { > + .name = "GPU_SET", > + .set = 16, > + .ack = 24, > + .set_new = 30, > + .ack_new = 31, > + }, > + > + [GMU_OOB_PERFCOUNTER_SET] = { > + .name = "PERFCOUNTER", > + .set = 17, > + .ack = 25, > + .set_new = 28, > + .ack_new = 30, > + }, > + > + [GMU_OOB_BOOT_SLUMBER] = { > + .name = "BOOT_SLUMBER", > + .set = 22, > + .ack = 30, > + }, > + > + [GMU_OOB_DCVS_SET] = { > + .name = "GPU_DCVS", > + .set = 23, > + .ack = 31, > + }, > +}; > + > /* Trigger a OOB (out of band) request to the GMU */ > int _a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state, char *file, int line) > { > int ret; > u32 val; > int request, ack; > - const char *name; > > - switch (state) { > - case GMU_OOB_GPU_SET: > - if (gmu->legacy) { > - request = GMU_OOB_GPU_SET_REQUEST; > - ack = GMU_OOB_GPU_SET_ACK; > - } else { > - request = GMU_OOB_GPU_SET_REQUEST_NEW; > - ack = GMU_OOB_GPU_SET_ACK_NEW; > - } > - name = "GPU_SET"; > - break; > - case GMU_OOB_PERFCOUNTER_SET: > - if (gmu->legacy) { > - request = GMU_OOB_PERFCOUNTER_REQUEST; > - ack = GMU_OOB_PERFCOUNTER_ACK; > - } else { > - request = GMU_OOB_PERFCOUNTER_REQUEST_NEW; > - ack = GMU_OOB_PERFCOUNTER_ACK_NEW; > - } > - name = "PERFCOUNTER"; > - break; > - case GMU_OOB_BOOT_SLUMBER: > - request = GMU_OOB_BOOT_SLUMBER_REQUEST; > - ack = GMU_OOB_BOOT_SLUMBER_ACK; > - name = "BOOT_SLUMBER"; > - break; > - case GMU_OOB_DCVS_SET: > - request = GMU_OOB_DCVS_REQUEST; > - ack = GMU_OOB_DCVS_ACK; > - name = "GPU_DCVS"; > - break; > - default: > + if (state >= ARRAY_SIZE(a6xx_gmu_oob_bits)) > return -EINVAL; > + > + if (gmu->legacy) { > + request = a6xx_gmu_oob_bits[state].set; > + ack = a6xx_gmu_oob_bits[state].ack; > + } else { > + request = a6xx_gmu_oob_bits[state].set_new; > + ack = a6xx_gmu_oob_bits[state].ack_new; > + if (!request || !ack) { > + DRM_DEV_ERROR(gmu->dev, > + "Invalid non-legacy GMU request %s\n", > + a6xx_gmu_oob_bits[state].name); > + return -EINVAL; > + } > } > > /* Trigger the equested OOB operation */ > @@ -299,7 +318,7 @@ int _a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state, char > DRM_DEV_ERROR(gmu->dev, > "%s:%d Timeout waiting for GMU OOB set %s: 0x%x\n", > file, line, > - name, > + a6xx_gmu_oob_bits[state].name, > gmu_read(gmu, REG_A6XX_GMU_GMU2HOST_INTR_INFO)); > > /* Clear the acknowledge interrupt */ > @@ -311,36 +330,17 @@ int _a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state, char > /* Clear a pending OOB state in the GMU */ > void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state) > { > - if (!gmu->legacy) { > - if (state == GMU_OOB_GPU_SET) { > - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, > - 1 << GMU_OOB_GPU_SET_CLEAR_NEW); > - } else { > - WARN_ON(state != GMU_OOB_PERFCOUNTER_SET); > - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, > - 1 << GMU_OOB_PERFCOUNTER_CLEAR_NEW); > - } > + int bit; > + > + if (state >= ARRAY_SIZE(a6xx_gmu_oob_bits)) > return; > - } > > - switch (state) { > - case GMU_OOB_GPU_SET: > - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, > - 1 << GMU_OOB_GPU_SET_CLEAR); > - break; > - case GMU_OOB_PERFCOUNTER_SET: > - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, > - 1 << GMU_OOB_PERFCOUNTER_CLEAR); > - break; > - case GMU_OOB_BOOT_SLUMBER: > - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, > - 1 << GMU_OOB_BOOT_SLUMBER_CLEAR); > - break; > - case GMU_OOB_DCVS_SET: > - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, > - 1 << GMU_OOB_DCVS_CLEAR); > - break; > - } > + if (gmu->legacy) > + bit = a6xx_gmu_oob_bits[state].ack; > + else > + bit = a6xx_gmu_oob_bits[state].ack_new; > + > + gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, bit); > } > > /* Enable CPU control of SPTP power power collapse */ > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > index 9fa278de2106..71dfa60070cc 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > @@ -153,52 +153,27 @@ static inline void gmu_write_rscc(struct a6xx_gmu *gmu, u32 offset, u32 value) > */ > > enum a6xx_gmu_oob_state { > + /* > + * Let the GMU know that a boot or slumber operation has started. The value in > + * REG_A6XX_GMU_BOOT_SLUMBER_OPTION lets the GMU know which operation we are > + * doing > + */ > GMU_OOB_BOOT_SLUMBER = 0, > + /* > + * Let the GMU know to not turn off any GPU registers while the CPU is in a > + * critical section > + */ > GMU_OOB_GPU_SET, > + /* > + * Set a new power level for the GPU when the CPU is doing frequency scaling > + */ > GMU_OOB_DCVS_SET, > + /* > + * Used to keep the GPU on for CPU-side reads of performance counters. > + */ > GMU_OOB_PERFCOUNTER_SET, > }; > > -/* These are the interrupt / ack bits for each OOB request that are set > - * in a6xx_gmu_set_oob and a6xx_clear_oob > - */ > - > -/* > - * Let the GMU know that a boot or slumber operation has started. The value in > - * REG_A6XX_GMU_BOOT_SLUMBER_OPTION lets the GMU know which operation we are > - * doing > - */ > -#define GMU_OOB_BOOT_SLUMBER_REQUEST 22 > -#define GMU_OOB_BOOT_SLUMBER_ACK 30 > -#define GMU_OOB_BOOT_SLUMBER_CLEAR 30 > - > -/* > - * Set a new power level for the GPU when the CPU is doing frequency scaling > - */ > -#define GMU_OOB_DCVS_REQUEST 23 > -#define GMU_OOB_DCVS_ACK 31 > -#define GMU_OOB_DCVS_CLEAR 31 > - > -/* > - * Let the GMU know to not turn off any GPU registers while the CPU is in a > - * critical section > - */ > -#define GMU_OOB_GPU_SET_REQUEST 16 > -#define GMU_OOB_GPU_SET_ACK 24 > -#define GMU_OOB_GPU_SET_CLEAR 24 > - > -#define GMU_OOB_GPU_SET_REQUEST_NEW 30 > -#define GMU_OOB_GPU_SET_ACK_NEW 31 > -#define GMU_OOB_GPU_SET_CLEAR_NEW 31 > - > -#define GMU_OOB_PERFCOUNTER_REQUEST 17 > -#define GMU_OOB_PERFCOUNTER_ACK 25 > -#define GMU_OOB_PERFCOUNTER_CLEAR 25 > - > -#define GMU_OOB_PERFCOUNTER_REQUEST_NEW 28 > -#define GMU_OOB_PERFCOUNTER_ACK_NEW 30 > -#define GMU_OOB_PERFCOUNTER_CLEAR_NEW 30 > - > void a6xx_hfi_init(struct a6xx_gmu *gmu); > int a6xx_hfi_start(struct a6xx_gmu *gmu, int boot_state); > void a6xx_hfi_stop(struct a6xx_gmu *gmu); > -- > 2.30.0 >
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 378dc7f190c3..c497e0942141 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -245,47 +245,66 @@ static int a6xx_gmu_hfi_start(struct a6xx_gmu *gmu) return ret; } +struct a6xx_gmu_oob_bits { + int set, ack, set_new, ack_new; + const char *name; +}; + +/* These are the interrupt / ack bits for each OOB request that are set + * in a6xx_gmu_set_oob and a6xx_clear_oob + */ +static const struct a6xx_gmu_oob_bits a6xx_gmu_oob_bits[] = { + [GMU_OOB_GPU_SET] = { + .name = "GPU_SET", + .set = 16, + .ack = 24, + .set_new = 30, + .ack_new = 31, + }, + + [GMU_OOB_PERFCOUNTER_SET] = { + .name = "PERFCOUNTER", + .set = 17, + .ack = 25, + .set_new = 28, + .ack_new = 30, + }, + + [GMU_OOB_BOOT_SLUMBER] = { + .name = "BOOT_SLUMBER", + .set = 22, + .ack = 30, + }, + + [GMU_OOB_DCVS_SET] = { + .name = "GPU_DCVS", + .set = 23, + .ack = 31, + }, +}; + /* Trigger a OOB (out of band) request to the GMU */ int _a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state, char *file, int line) { int ret; u32 val; int request, ack; - const char *name; - switch (state) { - case GMU_OOB_GPU_SET: - if (gmu->legacy) { - request = GMU_OOB_GPU_SET_REQUEST; - ack = GMU_OOB_GPU_SET_ACK; - } else { - request = GMU_OOB_GPU_SET_REQUEST_NEW; - ack = GMU_OOB_GPU_SET_ACK_NEW; - } - name = "GPU_SET"; - break; - case GMU_OOB_PERFCOUNTER_SET: - if (gmu->legacy) { - request = GMU_OOB_PERFCOUNTER_REQUEST; - ack = GMU_OOB_PERFCOUNTER_ACK; - } else { - request = GMU_OOB_PERFCOUNTER_REQUEST_NEW; - ack = GMU_OOB_PERFCOUNTER_ACK_NEW; - } - name = "PERFCOUNTER"; - break; - case GMU_OOB_BOOT_SLUMBER: - request = GMU_OOB_BOOT_SLUMBER_REQUEST; - ack = GMU_OOB_BOOT_SLUMBER_ACK; - name = "BOOT_SLUMBER"; - break; - case GMU_OOB_DCVS_SET: - request = GMU_OOB_DCVS_REQUEST; - ack = GMU_OOB_DCVS_ACK; - name = "GPU_DCVS"; - break; - default: + if (state >= ARRAY_SIZE(a6xx_gmu_oob_bits)) return -EINVAL; + + if (gmu->legacy) { + request = a6xx_gmu_oob_bits[state].set; + ack = a6xx_gmu_oob_bits[state].ack; + } else { + request = a6xx_gmu_oob_bits[state].set_new; + ack = a6xx_gmu_oob_bits[state].ack_new; + if (!request || !ack) { + DRM_DEV_ERROR(gmu->dev, + "Invalid non-legacy GMU request %s\n", + a6xx_gmu_oob_bits[state].name); + return -EINVAL; + } } /* Trigger the equested OOB operation */ @@ -299,7 +318,7 @@ int _a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state, char DRM_DEV_ERROR(gmu->dev, "%s:%d Timeout waiting for GMU OOB set %s: 0x%x\n", file, line, - name, + a6xx_gmu_oob_bits[state].name, gmu_read(gmu, REG_A6XX_GMU_GMU2HOST_INTR_INFO)); /* Clear the acknowledge interrupt */ @@ -311,36 +330,17 @@ int _a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state, char /* Clear a pending OOB state in the GMU */ void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state) { - if (!gmu->legacy) { - if (state == GMU_OOB_GPU_SET) { - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, - 1 << GMU_OOB_GPU_SET_CLEAR_NEW); - } else { - WARN_ON(state != GMU_OOB_PERFCOUNTER_SET); - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, - 1 << GMU_OOB_PERFCOUNTER_CLEAR_NEW); - } + int bit; + + if (state >= ARRAY_SIZE(a6xx_gmu_oob_bits)) return; - } - switch (state) { - case GMU_OOB_GPU_SET: - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, - 1 << GMU_OOB_GPU_SET_CLEAR); - break; - case GMU_OOB_PERFCOUNTER_SET: - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, - 1 << GMU_OOB_PERFCOUNTER_CLEAR); - break; - case GMU_OOB_BOOT_SLUMBER: - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, - 1 << GMU_OOB_BOOT_SLUMBER_CLEAR); - break; - case GMU_OOB_DCVS_SET: - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, - 1 << GMU_OOB_DCVS_CLEAR); - break; - } + if (gmu->legacy) + bit = a6xx_gmu_oob_bits[state].ack; + else + bit = a6xx_gmu_oob_bits[state].ack_new; + + gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, bit); } /* Enable CPU control of SPTP power power collapse */ diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h index 9fa278de2106..71dfa60070cc 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h @@ -153,52 +153,27 @@ static inline void gmu_write_rscc(struct a6xx_gmu *gmu, u32 offset, u32 value) */ enum a6xx_gmu_oob_state { + /* + * Let the GMU know that a boot or slumber operation has started. The value in + * REG_A6XX_GMU_BOOT_SLUMBER_OPTION lets the GMU know which operation we are + * doing + */ GMU_OOB_BOOT_SLUMBER = 0, + /* + * Let the GMU know to not turn off any GPU registers while the CPU is in a + * critical section + */ GMU_OOB_GPU_SET, + /* + * Set a new power level for the GPU when the CPU is doing frequency scaling + */ GMU_OOB_DCVS_SET, + /* + * Used to keep the GPU on for CPU-side reads of performance counters. + */ GMU_OOB_PERFCOUNTER_SET, }; -/* These are the interrupt / ack bits for each OOB request that are set - * in a6xx_gmu_set_oob and a6xx_clear_oob - */ - -/* - * Let the GMU know that a boot or slumber operation has started. The value in - * REG_A6XX_GMU_BOOT_SLUMBER_OPTION lets the GMU know which operation we are - * doing - */ -#define GMU_OOB_BOOT_SLUMBER_REQUEST 22 -#define GMU_OOB_BOOT_SLUMBER_ACK 30 -#define GMU_OOB_BOOT_SLUMBER_CLEAR 30 - -/* - * Set a new power level for the GPU when the CPU is doing frequency scaling - */ -#define GMU_OOB_DCVS_REQUEST 23 -#define GMU_OOB_DCVS_ACK 31 -#define GMU_OOB_DCVS_CLEAR 31 - -/* - * Let the GMU know to not turn off any GPU registers while the CPU is in a - * critical section - */ -#define GMU_OOB_GPU_SET_REQUEST 16 -#define GMU_OOB_GPU_SET_ACK 24 -#define GMU_OOB_GPU_SET_CLEAR 24 - -#define GMU_OOB_GPU_SET_REQUEST_NEW 30 -#define GMU_OOB_GPU_SET_ACK_NEW 31 -#define GMU_OOB_GPU_SET_CLEAR_NEW 31 - -#define GMU_OOB_PERFCOUNTER_REQUEST 17 -#define GMU_OOB_PERFCOUNTER_ACK 25 -#define GMU_OOB_PERFCOUNTER_CLEAR 25 - -#define GMU_OOB_PERFCOUNTER_REQUEST_NEW 28 -#define GMU_OOB_PERFCOUNTER_ACK_NEW 30 -#define GMU_OOB_PERFCOUNTER_CLEAR_NEW 30 - void a6xx_hfi_init(struct a6xx_gmu *gmu); int a6xx_hfi_start(struct a6xx_gmu *gmu, int boot_state); void a6xx_hfi_stop(struct a6xx_gmu *gmu);
Now that the bug is fixed in the minimal way for stable, go make the code table-driven. Signed-off-by: Eric Anholt <eric@anholt.net> --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 124 +++++++++++++------------- drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 55 ++++-------- 2 files changed, 77 insertions(+), 102 deletions(-)