Re: [Freedreno] [PATCH 1/2] iommu/arm-smmu-qcom: Skip the TTBR1 quirk for db820c.

[Eric Anholt]

On Mon, Mar 29, 2021 at 7:47 AM Will Deacon  wrote:
>
> On Fri, Mar 26, 2021 at 04:13:02PM -0700, Eric Anholt wrote:
> > db820c wants to use the qcom smmu path to get HUPCF set (which keeps
> > the GPU from wedging and then sometimes wedging the kernel after a
> > page fault), but it doesn't have separate pagetables support yet in
> > drm/msm so we can't go all the way to the TTBR1 path.
>
> What do you mean by "doesn't have separate pagetables support yet"? The
> compatible string doesn't feel like the right way to determine this.

In my past experience with DT, software looking at the (existing)
board-specific compatibles has been a typical mechanism used to
resolve something like this "ok, but you need to actually get down to
what board is involved here to figure out how to play along with the
rest of Linux that later attaches to other DT nodes".  Do you have a
preferred mechanism here?
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 2/2] arm64: dts: msm8996: Mark the GPU's SMMU as an adreno one.

[Eric Anholt]

This enables the adreno-specific SMMU path that sets HUPCF so
(user-managed) page faults don't wedge the GPU.

Signed-off-by: Eric Anholt 
---

We've been seeing a flaky test per day or so in Mesa CI where the
kernel gets wedged after an iommu fault turns into CP errors.  With
this patch, the CI isn't throwing the string of CP errors on the
faults in any of the ~10 jobs I've run so far.

 arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi 
b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 6de136e3add9..432b87ec9c5e 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -1127,7 +1127,7 @@ cci_i2c1: i2c-bus@1 {
};
 
adreno_smmu: iommu@b4 {
-   compatible = "qcom,msm8996-smmu-v2", "qcom,smmu-v2";
+   compatible = "qcom,msm8996-smmu-v2", 
"qcom,adreno-smmu", "qcom,smmu-v2";
reg = <0x00b4 0x1>;
 
#global-interrupts = <1>;
-- 
2.31.0

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 1/2] iommu/arm-smmu-qcom: Skip the TTBR1 quirk for db820c.

[Eric Anholt]

db820c wants to use the qcom smmu path to get HUPCF set (which keeps
the GPU from wedging and then sometimes wedging the kernel after a
page fault), but it doesn't have separate pagetables support yet in
drm/msm so we can't go all the way to the TTBR1 path.

Signed-off-by: Eric Anholt 
---

We've been seeing a flaky test per day or so in Mesa CI where the
kernel gets wedged after an iommu fault turns into CP errors.  With
this patch, the CI isn't throwing the string of CP errors on the
faults in any of the ~10 jobs I've run so far.

 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index bcda17012aee..51f22193e456 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -130,6 +130,16 @@ static int qcom_adreno_smmu_alloc_context_bank(struct 
arm_smmu_domain *smmu_doma
return __arm_smmu_alloc_bitmap(smmu->context_map, start, count);
 }
 
+static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu)
+{
+   const struct device_node *np = smmu->dev->of_node;
+
+   if (of_device_is_compatible(np, "qcom,msm8996-smmu-v2"))
+   return false;
+
+   return true;
+}
+
 static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
 {
@@ -144,7 +154,8 @@ static int qcom_adreno_smmu_init_context(struct 
arm_smmu_domain *smmu_domain,
 * be AARCH64 stage 1 but double check because the arm-smmu code assumes
 * that is the case when the TTBR1 quirk is enabled
 */
-   if ((smmu_domain->stage == ARM_SMMU_DOMAIN_S1) &&
+   if (qcom_adreno_can_do_ttbr1(smmu_domain->smmu) &&
+   (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) &&
(smmu_domain->cfg.fmt == ARM_SMMU_CTX_FMT_AARCH64))
pgtbl_cfg->quirks |= IO_PGTABLE_QUIRK_ARM_TTBR1;
 
-- 
2.31.0

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH] drm/msm: fix a6xx_gmu_clear_oob

[Eric Anholt]

On Tue, Feb 9, 2021 at 5:24 PM Jordan Crouse  wrote:
>
> On Mon, Feb 08, 2021 at 01:55:54PM -0500, Jonathan Marek wrote:
> > The cleanup patch broke a6xx_gmu_clear_oob, fix it by adding the missing
> > bitshift operation.
> >
> > Fixes: 555c50a4a19b ("drm/msm: Clean up GMU OOB set/clear handling")
> > Signed-off-by: Jonathan Marek 
>
> Thanks.  I feel silly that I missed that.
>
> Reviewed-by: Jordan Crouse 

Yeah, oops.

Reviewed-by: Eric Anholt 
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH v3 3/3] drm/msm: Clean up GMU OOB set/clear handling.

[Eric Anholt]

Now that the bug is fixed in the minimal way for stable, go make the
code table-driven.

Signed-off-by: Eric Anholt 
Reviewed-by: Jordan Crouse 
---
 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 b3318f86aabc..9066e98eb8ef 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)
 {
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 */
@@ -298,7 +317,7 @@ int a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum 
a6xx_gmu_oob_state state)
if (ret)
DRM_DEV_ERROR(gmu->dev,
"Timeout waiting for GMU OOB set %s: 0x%x\n",
-   name,
+   a6xx_gmu_oob_bits[state].name,
gmu_read(gmu, REG_A6XX_GMU_GMU2HOST_INTR_INFO));
 
/* Clear the acknowledge interrupt */
@@ -310,36 +329,17 @@ int a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum 
a6xx_gmu_oob_state state)
 /* 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,
-   

[Freedreno] [PATCH v3 1/3] drm/msm: Fix race of GPU init vs timestamp power management.

[Eric Anholt]

We were using the same force-poweron bit in the two codepaths, so they
could race to have one of them lose GPU power early.

freedreno CI was seeing intermittent errors like:
[drm:_a6xx_gmu_set_oob] *ERROR* Timeout waiting for GMU OOB set GPU_SET: 0x0
and this issue could have contributed to it.

Signed-off-by: Eric Anholt 
Fixes: 4b565ca5a2cb ("drm/msm: Add A6XX device support")
Reviewed-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 25 ++---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h |  8 
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c |  4 ++--
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index e6703ae98760..b3318f86aabc 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -264,6 +264,16 @@ int a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum 
a6xx_gmu_oob_state state)
}
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;
@@ -301,9 +311,14 @@ int a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum 
a6xx_gmu_oob_state state)
 void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state)
 {
if (!gmu->legacy) {
-   WARN_ON(state != GMU_OOB_GPU_SET);
-   gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET,
-   1 << GMU_OOB_GPU_SET_CLEAR_NEW);
+   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);
+   }
return;
}
 
@@ -312,6 +327,10 @@ void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum 
a6xx_gmu_oob_state state)
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);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h 
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index c6d2bced8e5d..9fa278de2106 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -156,6 +156,7 @@ enum a6xx_gmu_oob_state {
GMU_OOB_BOOT_SLUMBER = 0,
GMU_OOB_GPU_SET,
GMU_OOB_DCVS_SET,
+   GMU_OOB_PERFCOUNTER_SET,
 };
 
 /* These are the interrupt / ack bits for each OOB request that are set
@@ -190,6 +191,13 @@ enum a6xx_gmu_oob_state {
 #define GMU_OOB_GPU_SET_ACK_NEW31
 #define GMU_OOB_GPU_SET_CLEAR_NEW  31
 
+#define GMU_OOB_PERFCOUNTER_REQUEST17
+#define GMU_OOB_PERFCOUNTER_ACK25
+#define GMU_OOB_PERFCOUNTER_CLEAR  25
+
+#define GMU_OOB_PERFCOUNTER_REQUEST_NEW28
+#define GMU_OOB_PERFCOUNTER_ACK_NEW30
+#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);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index c8a9010c1a1d..7424a70b9d35 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1177,12 +1177,12 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, 
uint64_t *value)
struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
 
/* Force the GPU power on so we can read this register */
-   a6xx_gmu_set_oob(_gpu->gmu, GMU_OOB_GPU_SET);
+   a6xx_gmu_set_oob(_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
 
*value = gpu_read64(gpu, REG_A6XX_RBBM_PERFCTR_CP_0_LO,
REG_A6XX_RBBM_PERFCTR_CP_0_HI);
 
-   a6xx_gmu_clear_oob(_gpu->gmu, GMU_OOB_GPU_SET);
+   a6xx_gmu_clear_oob(_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
return 0;
 }
 
-- 
2.30.0

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH v3 0/3] drm/msm: fix for "Timeout waiting for GMU OOB set GPU_SET: 0x0"

[Eric Anholt]

Updated commit messages over v2, no code changes.

Eric Anholt (3):
  drm/msm: Fix race of GPU init vs timestamp power management.
  drm/msm: Fix races managing the OOB state for timestamp vs timestamps.
  drm/msm: Clean up GMU OOB set/clear handling.

 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 105 +++---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h |  49 
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c |   8 +-
 3 files changed, 84 insertions(+), 78 deletions(-)

-- 
2.30.0

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH v3 2/3] drm/msm: Fix races managing the OOB state for timestamp vs timestamps.

[Eric Anholt]

Now that we're not racing with GPU setup, also fix races of timestamps
against other timestamps.  In freedreno CI, we were seeing this path trigger
timeouts on setting the GMU bit, producing:

[drm:_a6xx_gmu_set_oob] *ERROR* Timeout waiting for GMU OOB set GPU_SET: 0x0

and this triggered especially on the first set of tests right after
boot (it's probably easier to lose the race than one might think,
given that we start many tests in parallel, and waiting for NFS to
page in code probably means that lots of tests hit the same point of
screen init at the same time).  As of this patch, the message seems to
have completely gone away.

Signed-off-by: Eric Anholt 
Fixes: 4b565ca5a2cb ("drm/msm: Add A6XX device support")
Reviewed-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 7424a70b9d35..e8f0b5325a7f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1175,6 +1175,9 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, 
uint64_t *value)
 {
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+   static DEFINE_MUTEX(perfcounter_oob);
+
+   mutex_lock(_oob);
 
/* Force the GPU power on so we can read this register */
a6xx_gmu_set_oob(_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
@@ -1183,6 +1186,7 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, 
uint64_t *value)
REG_A6XX_RBBM_PERFCTR_CP_0_HI);
 
a6xx_gmu_clear_oob(_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
+   mutex_unlock(_oob);
return 0;
 }
 
-- 
2.30.0

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH 1/3] drm/msm: Fix race of GPU init vs timestamp power management.

[Eric Anholt]

On Thu, Jan 28, 2021 at 10:52 AM Jordan Crouse  wrote:
>
> On Wed, Jan 27, 2021 at 03:39:44PM -0800, Eric Anholt wrote:
> > We were using the same force-poweron bit in the two codepaths, so they
> > could race to have one of them lose GPU power early.
> >
> > Signed-off-by: Eric Anholt 
> > Cc: sta...@vger.kernel.org # v5.9
>
> You can add:
> Fixes: 4b565ca5a2cb ("drm/msm: Add A6XX device support")
>
> Because that was my ugly.
>
> Reviewed-by: Jordan Crouse 

I only pointed it at 5.9 because it looked like it would probably
conflict against older branches.  I can add the fixes tag if you'd
like, though.
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 3/3 v2] drm/msm: Clean up GMU OOB set/clear handling.

[Eric Anholt]

Now that the bug is fixed in the minimal way for stable, go make the
code table-driven.

Signed-off-by: Eric Anholt 
---

Previous version hadn't been rebased off of a bit of debug code I had,
so it wouldn't cleanly apply.

 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 b3318f86aabc..9066e98eb8ef 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)
 {
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 */
@@ -298,7 +317,7 @@ int a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum 
a6xx_gmu_oob_state state)
if (ret)
DRM_DEV_ERROR(gmu->dev,
"Timeout waiting for GMU OOB set %s: 0x%x\n",
-   name,
+   a6xx_gmu_oob_bits[state].name,
gmu_read(gmu, REG_A6XX_GMU_GMU2HOST_INTR_INFO));
 
/* Clear the acknowledge interrupt */
@@ -310,36 +329,17 @@ int a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum 
a6xx_gmu_oob_state state)
 /* 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);
- 

[Freedreno] [PATCH 1/3] drm/msm: Fix race of GPU init vs timestamp power management.

[Eric Anholt]

We were using the same force-poweron bit in the two codepaths, so they
could race to have one of them lose GPU power early.

Signed-off-by: Eric Anholt 
Cc: sta...@vger.kernel.org # v5.9
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 25 ++---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h |  8 
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c |  4 ++--
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 78836b4fb98e..378dc7f190c3 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -264,6 +264,16 @@ int _a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum 
a6xx_gmu_oob_state state, char
}
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;
@@ -302,9 +312,14 @@ int _a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum 
a6xx_gmu_oob_state state, char
 void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state)
 {
if (!gmu->legacy) {
-   WARN_ON(state != GMU_OOB_GPU_SET);
-   gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET,
-   1 << GMU_OOB_GPU_SET_CLEAR_NEW);
+   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);
+   }
return;
}
 
@@ -313,6 +328,10 @@ void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum 
a6xx_gmu_oob_state state)
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);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h 
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index c6d2bced8e5d..9fa278de2106 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -156,6 +156,7 @@ enum a6xx_gmu_oob_state {
GMU_OOB_BOOT_SLUMBER = 0,
GMU_OOB_GPU_SET,
GMU_OOB_DCVS_SET,
+   GMU_OOB_PERFCOUNTER_SET,
 };
 
 /* These are the interrupt / ack bits for each OOB request that are set
@@ -190,6 +191,13 @@ enum a6xx_gmu_oob_state {
 #define GMU_OOB_GPU_SET_ACK_NEW31
 #define GMU_OOB_GPU_SET_CLEAR_NEW  31
 
+#define GMU_OOB_PERFCOUNTER_REQUEST17
+#define GMU_OOB_PERFCOUNTER_ACK25
+#define GMU_OOB_PERFCOUNTER_CLEAR  25
+
+#define GMU_OOB_PERFCOUNTER_REQUEST_NEW28
+#define GMU_OOB_PERFCOUNTER_ACK_NEW30
+#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);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index c8a9010c1a1d..7424a70b9d35 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1177,12 +1177,12 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, 
uint64_t *value)
struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
 
/* Force the GPU power on so we can read this register */
-   a6xx_gmu_set_oob(_gpu->gmu, GMU_OOB_GPU_SET);
+   a6xx_gmu_set_oob(_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
 
*value = gpu_read64(gpu, REG_A6XX_RBBM_PERFCTR_CP_0_LO,
REG_A6XX_RBBM_PERFCTR_CP_0_HI);
 
-   a6xx_gmu_clear_oob(_gpu->gmu, GMU_OOB_GPU_SET);
+   a6xx_gmu_clear_oob(_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
return 0;
 }
 
-- 
2.30.0

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 3/3] drm/msm: Clean up GMU OOB set/clear handling.

[Eric Anholt]

Now that the bug is fixed in the minimal way for stable, go make the
code table-driven.

Signed-off-by: Eric Anholt 
---
 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,
- 

[Freedreno] [PATCH 2/3] drm/msm: Fix races managing the OOB state for timestamp vs timestamps.

[Eric Anholt]

Now that we're not racing with GPU setup, also fix races of timestamps
against other timestamps.  In CI, we were seeing this path trigger
timeouts on setting the GMU bit, especially on the first set of tests
right after boot (it's probably easier to lose the race than one might
think, given that we start many tests in parallel, and waiting for NFS
to page in code probably means that lots of tests hit the same point
of screen init at the same time).

Signed-off-by: Eric Anholt 
Cc: sta...@vger.kernel.org # v5.9
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 7424a70b9d35..e8f0b5325a7f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1175,6 +1175,9 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, 
uint64_t *value)
 {
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+   static DEFINE_MUTEX(perfcounter_oob);
+
+   mutex_lock(_oob);
 
/* Force the GPU power on so we can read this register */
a6xx_gmu_set_oob(_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
@@ -1183,6 +1186,7 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, 
uint64_t *value)
REG_A6XX_RBBM_PERFCTR_CP_0_HI);
 
a6xx_gmu_clear_oob(_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
+   mutex_unlock(_oob);
return 0;
 }
 
-- 
2.30.0

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 2/2] drm/msm: Quiet error during failure in optional resource mappings.

[Eric Anholt]

We don't expect to find vbif_nrt or regdma on cheza, but were clogging
up dmesg with errors about it.

Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c |  4 ++--
 drivers/gpu/drm/msm/msm_drv.c   | 22 ++
 drivers/gpu/drm/msm/msm_drv.h   |  2 ++
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index a4ab802fee6d..d9aef2b5e930 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -838,13 +838,13 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
dpu_kms->vbif[VBIF_RT] = NULL;
goto error;
}
-   dpu_kms->vbif[VBIF_NRT] = msm_ioremap(dpu_kms->pdev, "vbif_nrt", 
"vbif_nrt");
+   dpu_kms->vbif[VBIF_NRT] = msm_ioremap_quiet(dpu_kms->pdev, "vbif_nrt", 
"vbif_nrt");
if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) {
dpu_kms->vbif[VBIF_NRT] = NULL;
DPU_DEBUG("VBIF NRT is not defined");
}
 
-   dpu_kms->reg_dma = msm_ioremap(dpu_kms->pdev, "regdma", "regdma");
+   dpu_kms->reg_dma = msm_ioremap_quiet(dpu_kms->pdev, "regdma", "regdma");
if (IS_ERR(dpu_kms->reg_dma)) {
dpu_kms->reg_dma = NULL;
DPU_DEBUG("REG_DMA is not defined");
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index f6ce40bf3699..df4a3c6a49cd 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -120,8 +120,8 @@ struct clk *msm_clk_get(struct platform_device *pdev, const 
char *name)
return clk;
 }
 
-void __iomem *msm_ioremap(struct platform_device *pdev, const char *name,
-   const char *dbgname)
+void __iomem *_msm_ioremap(struct platform_device *pdev, const char *name,
+  const char *dbgname, bool quiet)
 {
struct resource *res;
unsigned long size;
@@ -133,7 +133,8 @@ void __iomem *msm_ioremap(struct platform_device *pdev, 
const char *name,
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
if (!res) {
-   DRM_DEV_ERROR(>dev, "failed to get memory resource: 
%s\n", name);
+   if (!quiet)
+   DRM_DEV_ERROR(>dev, "failed to get memory 
resource: %s\n", name);
return ERR_PTR(-EINVAL);
}
 
@@ -141,7 +142,8 @@ void __iomem *msm_ioremap(struct platform_device *pdev, 
const char *name,
 
ptr = devm_ioremap(>dev, res->start, size);
if (!ptr) {
-   DRM_DEV_ERROR(>dev, "failed to ioremap: %s\n", name);
+   if (!quiet)
+   DRM_DEV_ERROR(>dev, "failed to ioremap: %s\n", 
name);
return ERR_PTR(-ENOMEM);
}
 
@@ -151,6 +153,18 @@ void __iomem *msm_ioremap(struct platform_device *pdev, 
const char *name,
return ptr;
 }
 
+void __iomem *msm_ioremap(struct platform_device *pdev, const char *name,
+ const char *dbgname)
+{
+   return _msm_ioremap(pdev, name, dbgname, false);
+}
+
+void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name,
+   const char *dbgname)
+{
+   return _msm_ioremap(pdev, name, dbgname, true);
+}
+
 void msm_writel(u32 data, void __iomem *addr)
 {
if (reglog)
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index e2d6a6056418..2687f7a42c15 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -411,6 +411,8 @@ struct clk *msm_clk_bulk_get_clock(struct clk_bulk_data 
*bulk, int count,
const char *name);
 void __iomem *msm_ioremap(struct platform_device *pdev, const char *name,
const char *dbgname);
+void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name,
+   const char *dbgname);
 void msm_writel(u32 data, void __iomem *addr);
 u32 msm_readl(const void __iomem *addr);
 
-- 
2.26.2

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 1/2] drm/msm: Garbage collect unused resource _len fields.

[Eric Anholt]

Nothing was using the lengths of these ioremaps.

Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 21 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h  |  1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c |  9 -
 3 files changed, 31 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 680527e28d09..a4ab802fee6d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -45,20 +45,6 @@
 static int dpu_kms_hw_init(struct msm_kms *kms);
 static void _dpu_kms_mmu_destroy(struct dpu_kms *dpu_kms);
 
-static unsigned long dpu_iomap_size(struct platform_device *pdev,
-   const char *name)
-{
-   struct resource *res;
-
-   res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
-   if (!res) {
-   DRM_ERROR("failed to get memory resource: %s\n", name);
-   return 0;
-   }
-
-   return resource_size(res);
-}
-
 #ifdef CONFIG_DEBUG_FS
 static int _dpu_danger_signal_status(struct seq_file *s,
bool danger_status)
@@ -844,7 +830,6 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
goto error;
}
DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio);
-   dpu_kms->mmio_len = dpu_iomap_size(dpu_kms->pdev, "mdp");
 
dpu_kms->vbif[VBIF_RT] = msm_ioremap(dpu_kms->pdev, "vbif", "vbif");
if (IS_ERR(dpu_kms->vbif[VBIF_RT])) {
@@ -853,22 +838,16 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
dpu_kms->vbif[VBIF_RT] = NULL;
goto error;
}
-   dpu_kms->vbif_len[VBIF_RT] = dpu_iomap_size(dpu_kms->pdev, "vbif");
dpu_kms->vbif[VBIF_NRT] = msm_ioremap(dpu_kms->pdev, "vbif_nrt", 
"vbif_nrt");
if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) {
dpu_kms->vbif[VBIF_NRT] = NULL;
DPU_DEBUG("VBIF NRT is not defined");
-   } else {
-   dpu_kms->vbif_len[VBIF_NRT] = dpu_iomap_size(dpu_kms->pdev,
-"vbif_nrt");
}
 
dpu_kms->reg_dma = msm_ioremap(dpu_kms->pdev, "regdma", "regdma");
if (IS_ERR(dpu_kms->reg_dma)) {
dpu_kms->reg_dma = NULL;
DPU_DEBUG("REG_DMA is not defined");
-   } else {
-   dpu_kms->reg_dma_len = dpu_iomap_size(dpu_kms->pdev, "regdma");
}
 
pm_runtime_get_sync(_kms->pdev->dev);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 4e32d040f1e6..13034cdb8665 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -100,7 +100,6 @@ struct dpu_kms {
 
/* io/register spaces: */
void __iomem *mmio, *vbif[VBIF_MAX], *reg_dma;
-   unsigned long mmio_len, vbif_len[VBIF_MAX], reg_dma_len;
 
struct regulator *vdd;
struct regulator *mmagic;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
index 80d3cfc14007..9f20b84d5c0a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -37,7 +37,6 @@ struct dpu_mdss_hw_init_handler {
 struct dpu_mdss {
struct msm_mdss base;
void __iomem *mmio;
-   unsigned long mmio_len;
struct dss_module_power mp;
struct dpu_irq_controller irq_controller;
struct icc_path *path[2];
@@ -292,7 +291,6 @@ int dpu_mdss_init(struct drm_device *dev)
 {
struct platform_device *pdev = to_platform_device(dev->dev);
struct msm_drm_private *priv = dev->dev_private;
-   struct resource *res;
struct dpu_mdss *dpu_mdss;
struct dss_module_power *mp;
int ret = 0;
@@ -308,13 +306,6 @@ int dpu_mdss_init(struct drm_device *dev)
 
DRM_DEBUG("mapped mdss address space @%pK\n", dpu_mdss->mmio);
 
-   res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mdss");
-   if (!res) {
-   DRM_ERROR("failed to get memory resource for mdss\n");
-   return -ENOMEM;
-   }
-   dpu_mdss->mmio_len = resource_size(res);
-
ret = dpu_mdss_parse_data_bus_icc_path(dev, dpu_mdss);
if (ret)
return ret;
-- 
2.26.2

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 1/2] drm/msm: Fix address space size after refactor.

[Eric Anholt]

Previously the address space went from 16M to ~0u, but with the
refactor one of the 'f's was dropped, limiting us to 256MB.
Additionally, the new interface takes a start and size, not start and
end, so we can't just copy and paste.

Fixes regressions in dEQP-VK.memory.allocation.random.*

Fixes: ccac7ce373c1 ("drm/msm: Refactor address space initialization")
Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 89673c7ed473..5db06b590943 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -194,7 +194,7 @@ adreno_iommu_create_address_space(struct msm_gpu *gpu,
struct msm_gem_address_space *aspace;
 
aspace = msm_gem_address_space_create(mmu, "gpu", SZ_16M,
-   0xfff);
+   0x - SZ_16M);
 
if (IS_ERR(aspace) && !IS_ERR(mmu))
mmu->funcs->destroy(mmu);
-- 
2.26.2

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 2/2] drm/msm: Fix setup of a6xx create_address_space.

[Eric Anholt]

We don't want it under CONFIG_DRM_MSM_GPU_STATE, we need it all the
time (like the other GPUs do).

Fixes: ccac7ce373c1 ("drm/msm: Refactor address space initialization")
Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index a1589e040c57..7768557cdfb2 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -893,8 +893,8 @@ static const struct adreno_gpu_funcs funcs = {
 #if defined(CONFIG_DRM_MSM_GPU_STATE)
.gpu_state_get = a6xx_gpu_state_get,
.gpu_state_put = a6xx_gpu_state_put,
-   .create_address_space = adreno_iommu_create_address_space,
 #endif
+   .create_address_space = adreno_iommu_create_address_space,
},
.get_timestamp = a6xx_get_timestamp,
 };
-- 
2.26.2

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH v6 4/5] drm/msm: Refactor address space initialization

[Eric Anholt]

On Wed, Jun 17, 2020 at 1:16 PM Eric Anholt  wrote:
>
> On Thu, Apr 9, 2020 at 4:34 PM Jordan Crouse  wrote:
> >
> > Refactor how address space initialization works. Instead of having the
> > address space function create the MMU object (and thus require separate but
> > equal functions for gpummu and iommu) use a single function and pass the
> > MMU struct in. Make the generic code cleaner by using target specific
> > functions to create the address space so a2xx can do its own thing in its
> > own space.  For all the other targets use a generic helper to initialize
> > IOMMU but leave the door open for newer targets to use customization
> > if they need it.
>
> I'm seeing regressions in dEQP-VK.memory.allocation.random.* on cheza
> after this commit.   The symptom is that large allocations fail with
> -ENOSPC from MSM_GEM_INFO(IOVA).
>
> Possibly relevant change from having stuffed some debug info in:
>
> before:
> [3.791436] [drm:msm_gem_address_space_create] *ERROR* msmgem
> address space create: 0x100 + 0xfeff
> [3.801672] platform 506a000.gmu: Adding to iommu group 6
> [3.807359] [drm:msm_gem_address_space_create] *ERROR* msmgem
> address space create: 0x0 + 0x7fff
> [3.817140] msm ae0.mdss: bound 500.gpu (ops a3xx_ops)
> [3.823212] msm_dpu ae01000.mdp: [drm:msm_ioremap] *ERROR* failed
> to get memory resource: vbif_nrt
> [3.832429] msm_dpu ae01000.mdp: [drm:msm_ioremap] *ERROR* failed
> to get memory resource: regdma
> [3.841478] [drm:dpu_kms_hw_init:878] dpu hardware revision:0x4000
> [3.848193] [drm:msm_gem_address_space_create] *ERROR* msmgem
> address space create: 0x1000 + 0xefff
>
> after:
>
> [3.798707] [drm:msm_gem_address_space_create] *ERROR* msmgem
> address space create: 0x100 + 0xfff
> [3.808731] platform 506a000.gmu: Adding to iommu group 6
> [3.814440] [drm:msm_gem_address_space_create] *ERROR* msmgem
> address space create: 0x0 + 0x7fff
> [3.820494] hub 2-1:1.0: USB hub found
> [3.824108] msm ae0.mdss: bound 500.gpu (ops a3xx_ops)
> [3.828554] hub 2-1:1.0: 4 ports detected
> [3.833756] msm_dpu ae01000.mdp: [drm:msm_ioremap] *ERROR* failed
> to get memory resource: vbif_nrt
> [3.847038] msm_dpu ae01000.mdp: [drm:msm_ioremap] *ERROR* failed
> to get memory resource: regdma
> [3.856095] [drm:dpu_kms_hw_init:878] dpu hardware revision:0x4000
> [3.862840] [drm:msm_gem_address_space_create] *ERROR* msmgem
> address space create: 0x1000 + 0xfff
>
> 256MB for GMU address space?

Found the bug, fixes at the last 2 commits of
https://github.com/anholt/linux/tree/drm-msm-address-space

I'm going to try having another go at convincing gmail to let git
send-email through, but all the howtos in the world didn't work last
time (gsuite has different behavior from normal gmail).
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH v6 4/5] drm/msm: Refactor address space initialization

[Eric Anholt]

On Thu, Apr 9, 2020 at 4:34 PM Jordan Crouse  wrote:
>
> Refactor how address space initialization works. Instead of having the
> address space function create the MMU object (and thus require separate but
> equal functions for gpummu and iommu) use a single function and pass the
> MMU struct in. Make the generic code cleaner by using target specific
> functions to create the address space so a2xx can do its own thing in its
> own space.  For all the other targets use a generic helper to initialize
> IOMMU but leave the door open for newer targets to use customization
> if they need it.

I'm seeing regressions in dEQP-VK.memory.allocation.random.* on cheza
after this commit.   The symptom is that large allocations fail with
-ENOSPC from MSM_GEM_INFO(IOVA).

Possibly relevant change from having stuffed some debug info in:

before:
[3.791436] [drm:msm_gem_address_space_create] *ERROR* msmgem
address space create: 0x100 + 0xfeff
[3.801672] platform 506a000.gmu: Adding to iommu group 6
[3.807359] [drm:msm_gem_address_space_create] *ERROR* msmgem
address space create: 0x0 + 0x7fff
[3.817140] msm ae0.mdss: bound 500.gpu (ops a3xx_ops)
[3.823212] msm_dpu ae01000.mdp: [drm:msm_ioremap] *ERROR* failed
to get memory resource: vbif_nrt
[3.832429] msm_dpu ae01000.mdp: [drm:msm_ioremap] *ERROR* failed
to get memory resource: regdma
[3.841478] [drm:dpu_kms_hw_init:878] dpu hardware revision:0x4000
[3.848193] [drm:msm_gem_address_space_create] *ERROR* msmgem
address space create: 0x1000 + 0xefff

after:

[3.798707] [drm:msm_gem_address_space_create] *ERROR* msmgem
address space create: 0x100 + 0xfff
[3.808731] platform 506a000.gmu: Adding to iommu group 6
[3.814440] [drm:msm_gem_address_space_create] *ERROR* msmgem
address space create: 0x0 + 0x7fff
[3.820494] hub 2-1:1.0: USB hub found
[3.824108] msm ae0.mdss: bound 500.gpu (ops a3xx_ops)
[3.828554] hub 2-1:1.0: 4 ports detected
[3.833756] msm_dpu ae01000.mdp: [drm:msm_ioremap] *ERROR* failed
to get memory resource: vbif_nrt
[3.847038] msm_dpu ae01000.mdp: [drm:msm_ioremap] *ERROR* failed
to get memory resource: regdma
[3.856095] [drm:dpu_kms_hw_init:878] dpu hardware revision:0x4000
[3.862840] [drm:msm_gem_address_space_create] *ERROR* msmgem
address space create: 0x1000 + 0xfff

256MB for GMU address space?
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH v2] drm/msm: Check for powered down HW in the devfreq callbacks

[Eric Anholt]

On Fri, May 1, 2020 at 12:03 PM Jordan Crouse  wrote:
>
> Writing to the devfreq sysfs nodes while the GPU is powered down can
> result in a system crash (on a5xx) or a nasty GMU error (on a6xx):
>
>  $ /sys/class/devfreq/500.gpu# echo 5 > min_freq
>   [  104.841625] platform 506a000.gmu: [drm:a6xx_gmu_set_oob]
> *ERROR* Timeout waiting for GMU OOB set GPU_DCVS: 0x0
>
> Despite the fact that we carefully try to suspend the devfreq device when
> the hardware is powered down there are lots of holes in the governors that
> don't check for the suspend state and blindly call into the devfreq
> callbacks that end up triggering hardware reads in the GPU driver.
>
> Call pm_runtime_get_if_in_use() in the gpu_busy() and gpu_set_freq()
> callbacks to skip the hardware access if it isn't active.
>
> v2: Use pm_runtime_get_if_in_use() per Eric Anholt
>
> Cc: sta...@vger.kernel.org
> Signed-off-by: Jordan Crouse 
> ---
>
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 6 ++
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 8 
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 7 +++
>  3 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 724024a2243a..4d7f269edfcc 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -1404,6 +1404,10 @@ static unsigned long a5xx_gpu_busy(struct msm_gpu *gpu)
>  {
> u64 busy_cycles, busy_time;
>
> +   /* Only read the gpu busy if the hardware is already active */
> +   if (pm_runtime_get_if_in_use(>pdev->dev) <= 0)
> +   return 0;
> +

RPM's APIs are a bit of a trap and will return a negative errno for
the get functions if runtime PM is disabled in kconfig, even though
usually that would mean that the power domain is not ever disabled by
RPM.  I think in these checks you want "if (pm_runtime_get_if_in_use()
== 0)", and that seems to be a common pattern in other drivers.  With
that,

Reviewed-by: Eric Anholt 

(and tested, too)
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH] drm/msm: Check for powered down HW in the devfreq callbacks

[Eric Anholt]

On Fri, May 1, 2020 at 11:26 AM Jordan Crouse  wrote:
>
> Writing to the devfreq sysfs nodes while the GPU is powered down can
> result in a system crash (on a5xx) or a nasty GMU error (on a6xx):
>
>  $ /sys/class/devfreq/500.gpu# echo 5 > min_freq
>   [  104.841625] platform 506a000.gmu: [drm:a6xx_gmu_set_oob]
> *ERROR* Timeout waiting for GMU OOB set GPU_DCVS: 0x0
>
> Despite the fact that we carefully try to suspend the devfreq device when
> the hardware is powered down there are lots of holes in the governors that
> don't check for the suspend state and blindly call into the devfreq
> callbacks that end up triggering hardware reads in the GPU driver.
>
> Check the power state in the gpu_busy() and gpu_set_freq() callbacks for
> a5xx and a6xx to make sure that the hardware is active before trying to
> access it.

Chatted on IRC -- while this avoids the instaboot on db820c when
setting /sys/class/devfreq/devfreq1/min_freq, I think we should be
using pm_runtime_get_if_in_use() to avoid the races while still
avoiding bringing up the GPU.
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH] drm/msm/a5xx: Always set an OPP supported hardware value

[Eric Anholt]

On Fri, Feb 14, 2020 at 10:36 AM Jordan Crouse  wrote:
>
> If the opp table specifies opp-supported-hw as a property but the driver
> has not set a supported hardware value the OPP subsystem will reject
> all the table entries.
>
> Set a "default" value that will match the default table entries but not
> conflict with any possible real bin values. Also fix a small memory leak
> and free the buffer allocated by nvmem_cell_read().
>
> Signed-off-by: Jordan Crouse 

This does fix my warn at boot on db820c.

Reviewed-by: Eric Anholt 
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH 3/4] drm/msm: Use dma_resv locking wrappers

[Eric Anholt]

On Fri, Dec 13, 2019 at 12:08 PM Daniel Vetter  wrote:
>
> On Mon, Nov 25, 2019 at 10:43:55AM +0100, Daniel Vetter wrote:
> > I'll add more fancy logic to them soon, so everyone really has to use
> > them. Plus they already provide some nice additional debug
> > infrastructure on top of direct ww_mutex usage for the fences tracked
> > by dma_resv.
> >
> > Signed-off-by: Daniel Vetter 
> > Cc: Rob Clark 
> > Cc: Sean Paul 
> > Cc: linux-arm-...@vger.kernel.org
> > Cc: freedreno@lists.freedesktop.org
>
> Ping for some review/acks.
>
> Thanks, Daniel
>
> > ---
> >  drivers/gpu/drm/msm/msm_gem_submit.c | 10 +-
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c 
> > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > index 7d04c47d0023..385d4965a8d0 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > @@ -157,7 +157,7 @@ static void submit_unlock_unpin_bo(struct 
> > msm_gem_submit *submit,
> >   msm_gem_unpin_iova(_obj->base, submit->aspace);
> >
> >   if (submit->bos[i].flags & BO_LOCKED)
> > - ww_mutex_unlock(_obj->base.resv->lock);
> > + dma_resv_unlock(msm_obj->base.resv);
> >
> >   if (backoff && !(submit->bos[i].flags & BO_VALID))
> >   submit->bos[i].iova = 0;
> > @@ -180,8 +180,8 @@ static int submit_lock_objects(struct msm_gem_submit 
> > *submit)
> >   contended = i;
> >
> >   if (!(submit->bos[i].flags & BO_LOCKED)) {
> > - ret = 
> > ww_mutex_lock_interruptible(_obj->base.resv->lock,
> > - >ticket);
> > + ret = dma_resv_lock_interruptible(msm_obj->base.resv,
> > +   >ticket);
> >   if (ret)
> >   goto fail;
> >   submit->bos[i].flags |= BO_LOCKED;
> > @@ -202,8 +202,8 @@ static int submit_lock_objects(struct msm_gem_submit 
> > *submit)
> >   if (ret == -EDEADLK) {
> >   struct msm_gem_object *msm_obj = submit->bos[contended].obj;
> >   /* we lost out in a seqno race, lock and retry.. */
> > - ret = 
> > ww_mutex_lock_slow_interruptible(_obj->base.resv->lock,
> > - >ticket);
> > + ret = dma_resv_lock_slow_interruptible(msm_obj->base.resv,
> > +>ticket);
> >   if (!ret) {
> >   submit->bos[contended].flags |= BO_LOCKED;
> >   slow_locked = contended;
> > --
> > 2.24.0
> >

Reviewed-by: Eric Anholt 
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH] drm/msm/dpu: Add UBWC support for RGB8888 formats

[Eric Anholt]

Rob Clark  writes:

> On Wed, Nov 6, 2019 at 3:26 PM Fritz Koenig  wrote:
>>
>> Hardware only natively supports BGR UBWC.
>> UBWC support for RGB can be had by pretending
>> that the buffer is BGR.
>
> Just to expand, this aligns with how we handle RGB component order in
> mesa for tiled or tiled+ubwc.  If uncompressed to linear the component
> order is RGB, but in tiled or tiled+ubwc, the component order is
> always the hw "native" order (BGR) regardless of what the outside
> world thinks.  But that detail kinda doesn't matter, it's not like
> generic code is going to understand the tiled or tiled+ubwc format in
> the first place.. and code that does understand it, knows enough to
> know that tiled/tiled+ubwc is always in the native component order.
>
>> Signed-off-by: Fritz Koenig 
>
> Reviewed-by: Rob Clark 

Seems like a reasonable workaround to me, and permissible by our fourcc
modifier rules ("you just have to have one way to address the pixels
given a fourcc and a modifier").

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH 1/5] clk: inherit clocks enabled by bootloader

[Eric Anholt]

Rob Clark  writes:

> From: Rob Clark 
>
> The goal here is to support inheriting a display setup by bootloader,
> although there may also be some non-display related use-cases.
>
> Rough idea is to add a flag for clks and power domains that might
> already be enabled when kernel starts, and which should not be
> disabled at late_initcall if the kernel thinks they are "unused".
>
> If bootloader is enabling display, and kernel is using efifb before
> real display driver is loaded (potentially from kernel module after
> userspace starts, in a typical distro kernel), we don't want to kill
> the clocks and power domains that are used by the display before
> userspace starts.
>
> Signed-off-by: Rob Clark 

Raspberry Pi is carrying downstream hacks to do similar stuff, and it
would be great to see CCF finally support this.


signature.asc
Description: PGP signature
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH v2 1/5] drm: Add reservation_object to drm_gem_object

[Eric Anholt]

Rob Herring  writes:

> Many users of drm_gem_object embed a struct reservation_object into
> their subclassed struct, so let's add one to struct drm_gem_object.
> This will allow removing the reservation object from the subclasses
> and removing the ->gem_prime_res_obj callback.
>
> With the addition, add a drm_gem_reservation_object_wait() helper
> function for drivers to use in wait ioctls.

1, 4, 5 are:

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH 5/5] drm: vc4: Switch to use drm_gem_object reservation_object

[Eric Anholt]

Rob Herring  writes:

> Now that the base struct drm_gem_object has a reservation_object, use it
> and remove the private BO one.

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH 4/5] drm: v3d: Switch to use drm_gem_object reservation_object

[Eric Anholt]

Rob Herring  writes:

> Now that the base struct drm_gem_object has a reservation_object, use it
> and remove the private BO one.
>
> Cc: Eric Anholt 
> Cc: Daniel Vetter 
> Cc: David Airlie 
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: Rob Herring 

> @@ -453,8 +453,6 @@ v3d_wait_bo_ioctl(struct drm_device *dev, void *data,
>  {
>   int ret;
>   struct drm_v3d_wait_bo *args = data;
> - struct drm_gem_object *gem_obj;
> - struct v3d_bo *bo;
>   ktime_t start = ktime_get();
>   u64 delta_ns;
>   unsigned long timeout_jiffies =
> @@ -463,21 +461,8 @@ v3d_wait_bo_ioctl(struct drm_device *dev, void *data,
>   if (args->pad != 0)
>   return -EINVAL;
>  
> - gem_obj = drm_gem_object_lookup(file_priv, args->handle);
> - if (!gem_obj) {
> - DRM_DEBUG("Failed to look up GEM BO %d\n", args->handle);
> - return -EINVAL;
> - }
> - bo = to_v3d_bo(gem_obj);
> -
> - ret = reservation_object_wait_timeout_rcu(bo->resv,
> -   true, true,
> -   timeout_jiffies);
> -
> - if (ret == 0)
> - ret = -ETIME;
> - else if (ret > 0)
> - ret = 0;
> + ret = drm_gem_reservation_object_wait(file_priv, args->handle,
> +   true, timeout_jiffies);

Looks like you lost my ret handling in the change.

Honestly, I'd love to see drm_gem_reservation_object_wait return
sensible values like this (0 for success, -ETIME for timeouts) instead
of drivers having to stubmle over this API.


signature.asc
Description: PGP signature
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [Mesa-dev] [RFC 2/4] nir: Add a new ALU nir_op_imad

[Eric Anholt]

Eduardo Lima Mitev  writes:

> ir3 compiler has an integer multiply-add instruction (IMAD_S24)
> that is used for different offset calculations in the backend.
> Since we intend to move some of these calculations to NIR, we need
> a new ALU op that can represent it.
> ---
>  src/compiler/nir/nir_opcodes.py | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py
> index d32005846a6..b61845fd514 100644
> --- a/src/compiler/nir/nir_opcodes.py
> +++ b/src/compiler/nir/nir_opcodes.py
> @@ -754,6 +754,7 @@ def triop_horiz(name, output_size, src1_size, src2_size, 
> src3_size, const_expr):
> [tuint, tuint, tuint], "", const_expr)
>  
>  triop("ffma", tfloat, "src0 * src1 + src2")
> +triop("imad", tint, "src0 * src1 + src2")

The constant-folding expression should be corrected to what the backend
operation actually does, and you should probably call it imad24 or
something in that case.


signature.asc
Description: PGP signature
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [Mesa-dev] [RFC 1/4] nir: Add a new intrinsic 'load_image_stride'

[Eric Anholt]

Eduardo Lima Mitev  writes:

> This is an internal intrinsic intended to be injected by a
> (freedreno-specific) 'lower_sampler_io' pass that will be introduced
> later in this series; and consumed by ir3_compiler_nir.
>
> The intrinsic will load in SSA values for various constants
> for images (image_dims), namely the format's bytes-per-pixel,
> y-stride and z-stride; for which the backend compiler will emit
> the corresponding uniforms.
>
> const_index[0] is the image index, and const_index[1] is the index
> into image_dims' register file for a given image: 0 for bpp, 1 to
> y-stride and 2 for z-stride.

Can you move this documentation of the meaning of the intrinsic into a
comment in the file?


signature.asc
Description: PGP signature
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH libdrm v2] libdrm: add msm drm uapi header

[Eric Anholt]

Tanmay Shah  writes:

> msm_drm.h file derived from drm-next kernel uapi header.
>
> Remove freedreno/msm/msm_drm.h to maintain only
> one copy of msm_drm.h and change freedreno Makefile
> accordingly.
>
> Signed-off-by: Tanmay Shah 

Looks like this is missing the meson.build update, and leaves a stale
note in include/drm/README.


signature.asc
Description: PGP signature
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH] drm/fourcc: add msm compressed format modifiers

[Eric Anholt]

Jeykumar Sankaran  writes:

> Qualcomm Snapdragon chipsets uses compressed format
> to optimize BW across multiple IP's. This change adds
> needed modifier support in drm for a simple 4x4 tile
> based compressed variants of base formats.
>
> Signed-off-by: Jeykumar Sankaran 

This seems like a reasonable enough description.

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [Intel-gfx] [PATCH 01/10] include: Move ascii85 functions from i915 to linux/ascii85.h

[Eric Anholt]

Chris Wilson  writes:

> Quoting Jordan Crouse (2018-04-05 23:06:53)
>> On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote:
>> > The i915 DRM driver very cleverly used ascii85 encoding for their
>> > GPU state file. Move the encode functions to a general header file to
>> > support other drivers that might be interested in the same
>> > functionality.
>> 
>> In a previous version of this patch, Chris asked what tree I wanted this 
>> applied
>> to, and the answer is: I'm not sure?  I'm hoping that Rob will be cool with
>> picking the rest up for msm-next for 4.18 but I'm okay with putting this
>> particular patch wherever it is easiest for the maintainers.
>
> We are a bit late to sneak it into the 4.17 drm base via i915. I don't
> anticipate any problems (for i915) with this patch going in through
> msm-next, so happy to have it land there and percolate back to i915 3
> months later.

I'd love to have it in drm-misc-next, so I can build a similar hang dump
interface for vc5.  But most importantly, I'd like to have it somewhere
soon.


signature.asc
Description: PGP signature
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH v2 3/5] drm/msm: Reuse dma_fence_release.

[Eric Anholt]

If we follow the typical pattern of the base class being the first
member, we can use the default dma_fence_free function.

Signed-off-by: Eric Anholt <e...@anholt.net>
Cc: Rob Clark <robdcl...@gmail.com>
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
---
 drivers/gpu/drm/msm/msm_fence.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
index 3f299c537b77..a2f89bac9c16 100644
--- a/drivers/gpu/drm/msm/msm_fence.c
+++ b/drivers/gpu/drm/msm/msm_fence.c
@@ -99,8 +99,8 @@ void msm_update_fence(struct msm_fence_context *fctx, 
uint32_t fence)
 }
 
 struct msm_fence {
-   struct msm_fence_context *fctx;
struct dma_fence base;
+   struct msm_fence_context *fctx;
 };
 
 static inline struct msm_fence *to_msm_fence(struct dma_fence *fence)
@@ -130,19 +130,13 @@ static bool msm_fence_signaled(struct dma_fence *fence)
return fence_completed(f->fctx, f->base.seqno);
 }
 
-static void msm_fence_release(struct dma_fence *fence)
-{
-   struct msm_fence *f = to_msm_fence(fence);
-   kfree_rcu(f, base.rcu);
-}
-
 static const struct dma_fence_ops msm_fence_ops = {
.get_driver_name = msm_fence_get_driver_name,
.get_timeline_name = msm_fence_get_timeline_name,
.enable_signaling = msm_fence_enable_signaling,
.signaled = msm_fence_signaled,
.wait = dma_fence_default_wait,
-   .release = msm_fence_release,
+   .release = dma_fence_free,
 };
 
 struct dma_fence *
-- 
2.11.0

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH v2 1/5] drm/msm: Expose our reservation object when exporting a dmabuf.

[Eric Anholt]

Without this, polling on the dma-buf (and presumably other devices
synchronizing against our rendering) would return immediately, even
while the BO was busy.

Signed-off-by: Eric Anholt <e...@anholt.net>
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Cc: sta...@vger.kernel.org
Cc: Rob Clark <robdcl...@gmail.com>
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
---
 drivers/gpu/drm/msm/msm_drv.c   | 1 +
 drivers/gpu/drm/msm/msm_drv.h   | 1 +
 drivers/gpu/drm/msm/msm_gem_prime.c | 7 +++
 3 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 9208e67be453..fe728a134e16 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -829,6 +829,7 @@ static struct drm_driver msm_driver = {
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_export   = drm_gem_prime_export,
.gem_prime_import   = drm_gem_prime_import,
+   .gem_prime_res_obj  = msm_gem_prime_res_obj,
.gem_prime_pin  = msm_gem_prime_pin,
.gem_prime_unpin= msm_gem_prime_unpin,
.gem_prime_get_sg_table = msm_gem_prime_get_sg_table,
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index b885c3d5ae4d..5e67fa66d483 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -223,6 +223,7 @@ struct sg_table *msm_gem_prime_get_sg_table(struct 
drm_gem_object *obj);
 void *msm_gem_prime_vmap(struct drm_gem_object *obj);
 void msm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
 int msm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
+struct reservation_object *msm_gem_prime_res_obj(struct drm_gem_object *obj);
 struct drm_gem_object *msm_gem_prime_import_sg_table(struct drm_device *dev,
struct dma_buf_attachment *attach, struct sg_table *sg);
 int msm_gem_prime_pin(struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/msm/msm_gem_prime.c 
b/drivers/gpu/drm/msm/msm_gem_prime.c
index 60bb290700ce..13403c6da6c7 100644
--- a/drivers/gpu/drm/msm/msm_gem_prime.c
+++ b/drivers/gpu/drm/msm/msm_gem_prime.c
@@ -70,3 +70,10 @@ void msm_gem_prime_unpin(struct drm_gem_object *obj)
if (!obj->import_attach)
msm_gem_put_pages(obj);
 }
+
+struct reservation_object *msm_gem_prime_res_obj(struct drm_gem_object *obj)
+{
+   struct msm_gem_object *msm_obj = to_msm_bo(obj);
+
+   return msm_obj->resv;
+}
-- 
2.11.0

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH 1/4] drm/msm: remove qcom, gpu-pwrlevels bindings

[Eric Anholt]

Rob Clark  writes:

> The plan is to use the OPP bindings.  For now, remove the documentation
> for qcom,gpu-pwrlevels, and make the driver fall back to a safe low
> clock if the node is not present.
>
> Note that no upstream dtb use this node.  For now we keep compatibility
> with this node to avoid breaking compatibility with downstream android
> dt files.
>
> Signed-off-by: Rob Clark 

Will we need the bus frequency knobs that I see in the old pwrlevels
node?  If so, what would the plan be for doing that within OPP?


signature.asc
Description: PGP signature
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno