Re: [PATCH 1/2] iommu/arm-smmu-qcom: Skip the TTBR1 quirk for db820c.
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?
[PATCH 2/2] arm64: dts: msm8996: Mark the GPU's SMMU as an adreno one.
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
[PATCH 1/2] iommu/arm-smmu-qcom: Skip the TTBR1 quirk for db820c.
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
Re: [PATCH] drm/msm: fix a6xx_gmu_clear_oob
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
[PATCH v3 2/3] drm/msm: Fix races managing the OOB state for timestamp vs timestamps.
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
[PATCH v3 1/3] drm/msm: Fix race of GPU init vs timestamp power management.
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
[PATCH v3 3/3] drm/msm: Clean up GMU OOB set/clear handling.
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,
-
[PATCH v3 0/3] drm/msm: fix for "Timeout waiting for GMU OOB set GPU_SET: 0x0"
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
[PATCH 3/3 v2] drm/msm: Clean up GMU OOB set/clear handling.
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);
-
[PATCH 2/3] drm/msm: Fix races managing the OOB state for timestamp vs timestamps.
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
[PATCH 3/3] drm/msm: Clean up GMU OOB set/clear handling.
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,
-
[PATCH 1/3] drm/msm: Fix race of GPU init vs timestamp power management.
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
Re: [PATCH v2] drm/vc4: replace idr_init() by idr_init_base()
On Thu, Nov 5, 2020 at 12:21 PM Deepak R Varma wrote:
>
> idr_init() uses base 0 which is an invalid identifier for this driver.
> The idr_alloc for this driver uses VC4_PERFMONID_MIN as start value for
> ID range and it is #defined to 1. The new function idr_init_base allows
> IDR to set the ID lookup from base 1. This avoids all lookups that
> otherwise starts from 0 since 0 is always unused / available.
>
> References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
>
> Signed-off-by: Deepak R Varma
> ---
> Changes since v1:
>- Change suggested by Eric Anholt
> 1. Use VC4_PERFMONID_MIN instead of magic number 1
>
> drivers/gpu/drm/vc4/vc4_perfmon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_perfmon.c
> b/drivers/gpu/drm/vc4/vc4_perfmon.c
> index f4aa75efd16b..18abc06335c1 100644
> --- a/drivers/gpu/drm/vc4/vc4_perfmon.c
> +++ b/drivers/gpu/drm/vc4/vc4_perfmon.c
> @@ -77,7 +77,7 @@ struct vc4_perfmon *vc4_perfmon_find(struct vc4_file
> *vc4file, int id)
> void vc4_perfmon_open_file(struct vc4_file *vc4file)
> {
> mutex_init(>perfmon.lock);
> - idr_init(>perfmon.idr);
> + idr_init_base(>perfmon.idr, VC4_PERFMONID_MIN);
> }
>
> static int vc4_perfmon_idr_del(int id, void *elem, void *data)
> --
> 2.25.1
Reviewed-by: Eric Anholt
hopefully Maxime can apply it.
Re: [PATCH] drm/vc4: replace idr_init() by idr_init_base()
On Thu, Nov 5, 2020 at 10:25 AM Deepak R Varma wrote:
>
> idr_init() uses base 0 which is an invalid identifier for this driver.
> The idr_alloc for this driver uses VC4_PERFMONID_MIN as start value for
> ID range and it is #defined to 1. The new function idr_init_base allows
> IDR to set the ID lookup from base 1. This avoids all lookups that
> otherwise starts from 0 since 0 is always unused / available.
>
> References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
>
> Signed-off-by: Deepak R Varma
> ---
> drivers/gpu/drm/vc4/vc4_perfmon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_perfmon.c
> b/drivers/gpu/drm/vc4/vc4_perfmon.c
> index f4aa75efd16b..7d40f421d922 100644
> --- a/drivers/gpu/drm/vc4/vc4_perfmon.c
> +++ b/drivers/gpu/drm/vc4/vc4_perfmon.c
> @@ -77,7 +77,7 @@ struct vc4_perfmon *vc4_perfmon_find(struct vc4_file
> *vc4file, int id)
> void vc4_perfmon_open_file(struct vc4_file *vc4file)
> {
> mutex_init(>perfmon.lock);
> - idr_init(>perfmon.idr);
> + idr_init_base(>perfmon.idr, 1);
> }
Sounds like you should use VC4_PERFMONID_MIN instead of a magic 1 here.
Re: [PATCH -next] drm/v3d: Remove set but not used variable
On Wed, Sep 23, 2020 at 6:13 AM Dave Stevenson
wrote:
>
> Hi
>
> On Wed, 23 Sep 2020 at 08:53, Li Heng wrote:
> >
> > This addresses the following gcc warning with "make W=1":
> >
> > drivers/gpu/drm/v3d/v3d_drv.c:73:32: warning:
> > ‘v3d_v3d_pm_ops’ defined but not used [-Wunused-const-variable=]
> >
> > Reported-by: Hulk Robot
> > Signed-off-by: Li Heng
> > ---
> > drivers/gpu/drm/v3d/v3d_drv.c | 4
> > 1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
> > index 9f7c261..05140db 100644
> > --- a/drivers/gpu/drm/v3d/v3d_drv.c
> > +++ b/drivers/gpu/drm/v3d/v3d_drv.c
> > @@ -70,10 +70,6 @@ static int v3d_runtime_resume(struct device *dev)
> > }
> > #endif
> >
> > -static const struct dev_pm_ops v3d_v3d_pm_ops = {
> > - SET_RUNTIME_PM_OPS(v3d_runtime_suspend, v3d_runtime_resume, NULL)
> > -};
> > -
>
> This looks to be the wrong approach, and I think a patch has got
> dropped somewhere.
>
> On our Raspberry Pi downstream vendor tree we have a patch [1] from
> Eric that renames v3d_v3d_pm_ops to v3d_pm_ops (don't need the
> duplicated suffix), and adds it to v3d_platform_driver. Why that never
> made it through the mainline trees I don't know.
>
> Eric: How good's your memory on this one?
The RPM stuff ended up abandoned because I didn't have any support in
debugging the power domain driver and I punted for a downstream hack.
We should at least be using these ops, though.
Re: [PATCH AUTOSEL 5.4 001/330] drm/v3d: don't leak bin job if v3d_job_init fails.
On Thu, Sep 17, 2020 at 7:01 PM Sasha Levin wrote:
>
> From: Iago Toral Quiroga
>
> [ Upstream commit 0d352a3a8a1f26168d09f7073e61bb4b328e3bb9 ]
>
> If the initialization of the job fails we need to kfree() it
> before returning.
>
> Signed-off-by: Iago Toral Quiroga
> Signed-off-by: Eric Anholt
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20190916071125.5255-1-ito...@igalia.com
> Fixes: a783a09ee76d ("drm/v3d: Refactor job management.")
> Reviewed-by: Eric Anholt
> Signed-off-by: Sasha Levin
You're double freeing with this patch, the bug is already solved.
> ---
> drivers/gpu/drm/v3d/v3d_gem.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
> index 19c092d75266b..6316bf3646af5 100644
> --- a/drivers/gpu/drm/v3d/v3d_gem.c
> +++ b/drivers/gpu/drm/v3d/v3d_gem.c
> @@ -565,6 +565,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
> ret = v3d_job_init(v3d, file_priv, >base,
>v3d_job_free, args->in_sync_bcl);
> if (ret) {
> + kfree(bin);
> v3d_job_put(>base);
> kfree(bin);
> return ret;
> --
> 2.25.1
>
Re: [PATCH] drm/v3d: Use platform_get_irq_optional() to get optional IRQs
On Thu, Jul 16, 2020 at 7:51 AM Nicolas Saenz Julienne
wrote:
>
> Aside from being more correct, the non optional version of the function
> prints an error when failing to find the IRQ.
>
> Fixes: eea9b97b4504 ("drm/v3d: Add support for V3D v4.2")
> Signed-off-by: Nicolas Saenz Julienne
> ---
> drivers/gpu/drm/v3d/v3d_irq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
> index c88686489b88..0be2eb7876be 100644
> --- a/drivers/gpu/drm/v3d/v3d_irq.c
> +++ b/drivers/gpu/drm/v3d/v3d_irq.c
> @@ -217,7 +217,7 @@ v3d_irq_init(struct v3d_dev *v3d)
> V3D_CORE_WRITE(core, V3D_CTL_INT_CLR, V3D_CORE_IRQS);
> V3D_WRITE(V3D_HUB_INT_CLR, V3D_HUB_IRQS);
>
> - irq1 = platform_get_irq(v3d_to_pdev(v3d), 1);
> + irq1 = platform_get_irq_optional(v3d_to_pdev(v3d), 1);
> if (irq1 == -EPROBE_DEFER)
> return irq1;
> if (irq1 > 0) {
> --
Reviewed-by: Eric Anholt
Re: [PATCH] drm/vc4: dsi: Only register our component once a DSI device is attached
On Tue, Jul 7, 2020 at 3:26 AM Maxime Ripard wrote: > > If the DSI driver is the last to probe, component_add will try to run all > the bind callbacks straight away and return the error code. > > However, since we depend on a power domain, we're pretty much guaranteed to > be in that case on the BCM2711, and are just lucky on the previous SoCs > since the v3d also depends on that power domain and is further in the probe > order. > > In that case, the DSI host will not stick around in the system: the DSI > bind callback will be executed, will not find any DSI device attached and > will return EPROBE_DEFER, and we will then remove the DSI host and ask to > be probed later on. > > But since that host doesn't stick around, DSI devices like the RaspberryPi > touchscreen whose probe is not linked to the DSI host (unlike the usual DSI > devices that will be probed through the call to mipi_dsi_host_register) > cannot attach to the DSI host, and we thus end up in a situation where the > DSI host cannot probe because the panel hasn't probed yet, and the panel > cannot probe because the DSI host hasn't yet. > > In order to break this cycle, let's wait until there's a DSI device that > attaches to the DSI host to register the component and allow to progress > further. > > Suggested-by: Andrzej Hajda > Signed-off-by: Maxime Ripard I feel like I've written this patch before, but I've thankfully forgotten most of my battle with DSI probing. As long as this still lets vc4 probe in the absence of a DSI panel in the DT as well, then this is enthusiastically acked.
Re: [PATCH v4 0/9] drm/vc4: Turn the TXP into a CRTC
On Tue, Jun 30, 2020 at 1:25 AM Maxime Ripard wrote: > > Hi Eric, > > On Thu, Jun 11, 2020 at 03:36:45PM +0200, Maxime Ripard wrote: > > Hi, > > > > This is another part of the rpi4 HDMI series that got promoted to a > > series of its own to try to reduce the main one. > > > > This rework is needed since the bcm2711 used in the rpi4 has a more > > complex routing in the HVS that doesn't allow to use a fairly simple > > mapping like what was used before. > > > > Since that mapping affects both the pixelvalves and the TXP, turning the > > TXP into a CRTC just like pixelvalves are allows to deal with the > > mapping in the CRTC states, which turns out to be pretty convenient. > > > > Let me know what you think, > > Maxime > > > > Changes from v3: > > - Stripped the patches out of the main HDMI series > > - Change the bind order of the HVS to avoid a compatible check > > - Added Eric's tags > > - Rebased on top of drm-misc-next > > > > Maxime Ripard (9): > > drm/vc4: Reorder the bind order of the devices > > drm/vc4: crtc: Move HVS setup code to the HVS driver > > Could you review those two patches? You haven't reviewed them yet and > it's holding back the rest of the patches. LKML email workflow is the worst, the patches never came through to me so I didn't see them until you linked me the patchwork. Patch 2, commit message should say "We'll need the HVS to be bound before the TXP", but other than that, r-b.
[PATCH 2/2] drm/msm: Quiet error during failure in optional resource mappings.
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
[PATCH 1/2] drm/msm: Garbage collect unused resource _len fields.
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
[PATCH 2/2] drm/msm: Fix setup of a6xx create_address_space.
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
[PATCH 1/2] drm/msm: Fix address space size after refactor.
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
Re: [PATCH v6 4/5] drm/msm: Refactor address space initialization
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).
Re: [PATCH v6 4/5] drm/msm: Refactor address space initialization
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?
Re: [PATCH v3 004/105] clk: bcm: Add BCM2711 DVP driver
On Wed, May 27, 2020 at 8:49 AM Maxime Ripard wrote: > > The HDMI block has a block that controls clocks and reset signals to the > HDMI0 and HDMI1 controllers. > > Let's expose that through a clock driver implementing a clock and reset > provider. > > Cc: Michael Turquette > Cc: Stephen Boyd > Cc: Rob Herring > Cc: linux-...@vger.kernel.org > Cc: devicet...@vger.kernel.org > Reviewed-by: Stephen Boyd > Signed-off-by: Maxime Ripard > --- > drivers/clk/bcm/Kconfig | 11 +++- > drivers/clk/bcm/Makefile | 1 +- > drivers/clk/bcm/clk-bcm2711-dvp.c | 127 +++- > 3 files changed, 139 insertions(+) > create mode 100644 drivers/clk/bcm/clk-bcm2711-dvp.c > > diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig > index 8c83977a7dc4..784f12c72365 100644 > --- a/drivers/clk/bcm/Kconfig > +++ b/drivers/clk/bcm/Kconfig > @@ -1,4 +1,15 @@ > # SPDX-License-Identifier: GPL-2.0-only > + > +config CLK_BCM2711_DVP > + tristate "Broadcom BCM2711 DVP support" > + depends on ARCH_BCM2835 ||COMPILE_TEST > + depends on COMMON_CLK > + default ARCH_BCM2835 > + select RESET_SIMPLE > + help > + Enable common clock framework support for the Broadcom BCM2711 > + DVP Controller. > + > config CLK_BCM2835 > bool "Broadcom BCM2835 clock support" > depends on ARCH_BCM2835 || ARCH_BRCMSTB || COMPILE_TEST > diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile > index 0070ddf6cdd2..2c1349062147 100644 > --- a/drivers/clk/bcm/Makefile > +++ b/drivers/clk/bcm/Makefile > @@ -6,6 +6,7 @@ obj-$(CONFIG_CLK_BCM_KONA) += clk-kona-setup.o > obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm281xx.o > obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm21664.o > obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o > clk-iproc-asiu.o > +obj-$(CONFIG_CLK_BCM2835) += clk-bcm2711-dvp.o > obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835.o > obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835-aux.o > obj-$(CONFIG_CLK_RASPBERRYPI) += clk-raspberrypi.o I do think that single driver is the right model here, but I noticed that you're not using your new CONFIG_ symbol. With that fixed, r-b from me. (though I'd also recommend devm_platform_get_and_ioremap_resource and devm_reset_controller_register())
Re: [PATCH v3 032/105] drm/vc4: crtc: Enable and disable the PV in atomic_enable / disable
On Tue, Jun 2, 2020 at 8:02 AM Dave Stevenson wrote: > > Hi Maxime and Eric > > On Tue, 2 Jun 2020 at 15:12, Maxime Ripard wrote: > > > > Hi Eric > > > > On Wed, May 27, 2020 at 09:54:44AM -0700, Eric Anholt wrote: > > > On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote: > > > > > > > > The VIDEN bit in the pixelvalve currently being used to enable or > > > > disable > > > > the pixelvalve seems to not be enough in some situations, which whill > > > > end > > > > up with the pixelvalve stalling. > > > > > > > > In such a case, even re-enabling VIDEN doesn't bring it back and we > > > > need to > > > > clear the FIFO. This can only be done if the pixelvalve is disabled > > > > though. > > > > > > > > In order to overcome this, we can configure the pixelvalve during > > > > mode_set_no_fb, but only enable it in atomic_enable and flush the FIFO > > > > there, and in atomic_disable disable the pixelvalve again. > > > > > > What displays has this been tested with? Getting this sequencing > > > right is so painful, and things like DSI are tricky to get to light > > > up. > > > > That FIFO is between the HVS and the HDMI PVs, so this was obviously > > tested against that. Dave also tested the DSI output IIRC, so we should > > be covered here. > > DSI wasn't working on the first patch set that Maxime sent - I haven't > tested this one as yet but will do so. > DPI was working early on to both an Adafruit 800x480 DPI panel, and > via a VGA666 as VGA. > HDMI is obviously working. > VEC is being ignored now. The clock structure is more restricted than > earlier chips, so to get the required clocks for the VEC without using > fractional divides it compromises the clock that other parts of the > system can run at (IIRC including the ARM). That's why the VEC has to > be explicitly enabled for the firmware to enable it as the only > output. It's annoying, but that's just a restriction of the chip. I'm more concerned with "make sure we don't regress pre-pi4 with this series" than "pi4 displays all work from the beginning"
Re: [PATCH v3 015/105] drm/vc4: hvs: Boost the core clock during modeset
On Tue, Jun 2, 2020 at 5:52 AM Maxime Ripard wrote: > > Hi Eric, > > On Wed, May 27, 2020 at 09:33:44AM -0700, Eric Anholt wrote: > > On Wed, May 27, 2020 at 8:49 AM Maxime Ripard wrote: > > > > > > In order to prevent timeouts and stalls in the pipeline, the core clock > > > needs to be maxed at 500MHz during a modeset on the BCM2711. > > > > Like, the whole system's core clock? > > Yep, unfortunately... > > > How is it reasonable for some device driver to crank the system's core > > clock up and back down to some fixed-in-the-driver frequency? Sounds > > like you need some sort of opp thing here. > > That frequency is the minimum rate of that clock. However, since other > devices have similar requirements (unicam in particular) with different > minimum requirements, we will switch to setting a minimum rate instead > of enforcing a particular rate, so that patch would be essentially > s/clk_set_rate/clk_set_min_rate/. clk_set_min_rate makes a lot more sense to me. r-b with that obvious change. Thanks!
Re: [PATCH v3 055/105] drm/vc4: hvs: Introduce a function to get the assigned FIFO
On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote:
>
> At boot time, if we detect that a pixelvalve has been enabled, we need to
> be able to retrieve the HVS channel it has been assigned to so that we can
> disable that channel too. Let's create that function that returns the FIFO
> or an error from a given output.
>
> Signed-off-by: Maxime Ripard
> ---
> +int vc4_hvs_get_fifo_from_output(struct drm_device *dev, unsigned int output)
> +{
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + u32 reg;
> + int ret;
> +
> + switch (output) {
> + case 0:
> + return 0;
> +
> + case 1:
> + return 1;
> +
> + case 2:
> + reg = HVS_READ(SCALER_DISPECTRL);
> + ret = FIELD_GET(SCALER_DISPECTRL_DSP2_MUX_MASK, reg);
> + if (ret == 0)
> + return 2;
> +
> + return 0;
> +
> + case 3:
> + reg = HVS_READ(SCALER_DISPCTRL);
> + ret = FIELD_GET(SCALER_DISPCTRL_DSP3_MUX_MASK, reg);
> + if (ret == 3)
> + return -EPIPE;
> +
> + return ret;
> +
> + case 4:
> + reg = HVS_READ(SCALER_DISPEOLN);
> + ret = FIELD_GET(SCALER_DISPEOLN_DSP4_MUX_MASK, reg);
> + if (ret == 3)
> + return -EPIPE;
> +
> + return ret;
> +
> + case 5:
> + reg = HVS_READ(SCALER_DISPDITHER);
> + ret = FIELD_GET(SCALER_DISPDITHER_DSP5_MUX_MASK, reg);
> + if (ret == 3)
> + return -EPIPE;
Oh, FIELD_GET is new to me. Looks like we should replace
VC4_GET_FIELD usage with just using that header, and also
VC4_SET_FIELD with WARN_ON(!FIELD_FIT()); FIELD_PREP.
Could you follow up with that? Other than that, 54-67 r-b.
Re: [PATCH v3 059/105] drm/vc4: crtc: Add BCM2711 pixelvalves
On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote:
>
> The BCM2711 has 5 pixelvalves, so now that our driver is ready, let's add
> support for them.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 84 ++-
> drivers/gpu/drm/vc4/vc4_regs.h | 6 +++-
> 2 files changed, 88 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 9efd7cb25590..a577ed8f929f 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -229,6 +229,13 @@ static u32 vc4_get_fifo_full_level(struct vc4_crtc
> *vc4_crtc, u32 format)
> case PV_CONTROL_FORMAT_24:
> case PV_CONTROL_FORMAT_DSIV_24:
> default:
> + /*
> +* For some reason, the pixelvalve4 doesn't work with
> +* the usual formula and will only work with 32.
> +*/
> + if (vc4_crtc->data->hvs_output == 5)
> + return 32;
> +
> return fifo_len_bytes - 3 * HVS_FIFO_LATENCY_PIX;
> }
> }
> @@ -237,9 +244,14 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct
> vc4_crtc *vc4_crtc,
> u32 format)
> {
> u32 level = vc4_get_fifo_full_level(vc4_crtc, format);
> + u32 ret = 0;
>
> - return VC4_SET_FIELD(level & 0x3f,
> -PV_CONTROL_FIFO_LEVEL);
> + if (level > 0x3f)
> + ret |= VC4_SET_FIELD((level >> 6) & 0x3,
> +PV5_CONTROL_FIFO_LEVEL_HIGH);
> +
I would drop the conditional here (ORing in zero is fine), and also
the & 3 because it would be good to get a warning if you picked a fifo
full level that doesn't fit in the field.
> + return ret | VC4_SET_FIELD(level & 0x3f,
> + PV_CONTROL_FIFO_LEVEL);
> }
>
> /*
> @@ -277,6 +289,8 @@ static void vc4_crtc_pixelvalve_reset(struct drm_crtc
> *crtc)
>
> static void vc4_crtc_config_pv(struct drm_crtc *crtc)
> {
> + struct drm_device *dev = crtc->dev;
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc);
> struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> @@ -356,6 +370,10 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc)
> if (is_dsi)
> CRTC_WRITE(PV_HACT_ACT, mode->hdisplay * pixel_rep);
>
> + if (vc4->hvs->hvs5)
> + CRTC_WRITE(PV_MUX_CFG,
> + VC4_SET_FIELD(8, PV_MUX_CFG_RGB_PIXEL_MUX_MODE));
Can we get some #defines in the reg header instead of a magic value?
Other than that, r-b.
Re: [PATCH v3 070/105] drm/vc4: hdmi: rework connectors and encoders
On Wed, May 27, 2020 at 8:51 AM Maxime Ripard wrote: > > the vc4_hdmi driver has some custom structures to hold the data it needs to > associate with the drm_encoder and drm_connector structures. > > However, it allocates them separately from the vc4_hdmi structure which > makes it more complicated than it needs to be. > > Move those structures to be contained by vc4_hdmi and update the code > accordingly. > @@ -1220,7 +1219,7 @@ static int vc4_hdmi_bind(struct device *dev, struct > device *master, void *data) > struct drm_device *drm = dev_get_drvdata(master); > struct vc4_dev *vc4 = drm->dev_private; > struct vc4_hdmi *hdmi; > - struct vc4_hdmi_encoder *vc4_hdmi_encoder; > + struct drm_encoder *encoder; > struct device_node *ddc_node; > u32 value; > int ret; > @@ -1229,14 +1228,10 @@ static int vc4_hdmi_bind(struct device *dev, struct > device *master, void *data) > if (!hdmi) > return -ENOMEM; > > - vc4_hdmi_encoder = devm_kzalloc(dev, sizeof(*vc4_hdmi_encoder), > - GFP_KERNEL); > - if (!vc4_hdmi_encoder) > - return -ENOMEM; > - vc4_hdmi_encoder->base.type = VC4_ENCODER_TYPE_HDMI0; > - hdmi->encoder = _hdmi_encoder->base.base; > - > hdmi->pdev = pdev; > + encoder = >encoder.base.base; > + encoder->base.type = VC4_ENCODER_TYPE_HDMI0; Wait, does this patch build? setting struct drm_encoder->base.type = VC4_* seems very wrong, when previously we were setting struct vc4_hdmi_encoder->base.type (struct vc4_encoder->type). Other than this, patch 68-78 r-b.
Re: [PATCH v3 041/105] drm/vc4: crtc: Move HVS mode config to HVS file
On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote:
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 272 +---
> drivers/gpu/drm/vc4/vc4_drv.h | 5 +-
> drivers/gpu/drm/vc4/vc4_hvs.c | 298 ++-
> 3 files changed, 309 insertions(+), 266 deletions(-)
> static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
> {
> - struct drm_device *dev = crtc->dev;
> - struct vc4_dev *vc4 = to_vc4_dev(dev);
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
> - struct drm_display_mode *mode = >state->adjusted_mode;
> - bool interlace = mode->flags & DRM_MODE_FLAG_INTERLACE;
> bool debug_dump_regs = false;
>
> if (debug_dump_regs) {
> @@ -418,42 +372,10 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc
> *crtc)
> drm_print_regset32(, _crtc->regset);
> }
>
> - if (vc4_crtc->data->hvs_output == 2) {
> - u32 dispctrl;
> - u32 dsp3_mux;
> -
> - /*
> -* SCALER_DISPCTRL_DSP3 = X, where X < 2 means 'connect DSP3
> to
> -* FIFO X'.
> -* SCALER_DISPCTRL_DSP3 = 3 means 'disable DSP 3'.
> -*
> -* DSP3 is connected to FIFO2 unless the transposer is
> -* enabled. In this case, FIFO 2 is directly accessed by the
> -* TXP IP, and we need to disable the FIFO2 -> pixelvalve1
> -* route.
> -*/
> - if (vc4_state->feed_txp)
> - dsp3_mux = VC4_SET_FIELD(3, SCALER_DISPCTRL_DSP3_MUX);
> - else
> - dsp3_mux = VC4_SET_FIELD(2, SCALER_DISPCTRL_DSP3_MUX);
> -
> - dispctrl = HVS_READ(SCALER_DISPCTRL) &
> - ~SCALER_DISPCTRL_DSP3_MUX_MASK;
> - HVS_WRITE(SCALER_DISPCTRL, dispctrl | dsp3_mux);
> - }
I just noticed, this block being moved looks like it should probably
have been removed as part of patch #33. Cleaning this up I think will
impact the following patches.
Re: [PATCH v3 040/105] drm/vc4: crtc: Turn pixelvalve reset into a function
On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote: > > The driver resets the pixelvalve FIFO in a number of occurences without > always using the same sequence. > > Since this will be critical for BCM2711, let's move that sequence to a > function so that we are consistent. > > Signed-off-by: Maxime Ripard Patch 34-40 also r-b. Going to take a little break, this is a lot to try to process at once. Hopefully you can merge reviewed stuff to drm-misc and shorten the series.
Re: [PATCH v3 033/105] drm/vc4: crtc: Assign output to channel automatically
On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote:
>
> The HVS found in the BCM2711 has 6 outputs and 3 FIFOs, with each output
> being connected to a pixelvalve, and some muxing between the FIFOs and
> outputs.
>
> Any output cannot feed from any FIFO though, and they all have a bunch of
> constraints.
>
> In order to support this, let's store the possible FIFOs each output can be
> assigned to in the vc4_crtc_data, and use that information at atomic_check
> time to iterate over all the CRTCs enabled and assign them FIFOs.
>
> The channel assigned is then set in the vc4_crtc_state so that the rest of
> the driver can use it.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 37 +
> drivers/gpu/drm/vc4/vc4_drv.h | 7 +-
> drivers/gpu/drm/vc4/vc4_kms.c | 142 --
> drivers/gpu/drm/vc4/vc4_regs.h | 10 ++-
> 4 files changed, 172 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 580b37ad514d..a6c3f2f907bd 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -88,6 +88,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc
> *crtc,
> struct drm_device *dev = crtc->dev;
> struct vc4_dev *vc4 = to_vc4_dev(dev);
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> + struct vc4_crtc_state *vc4_crtc_state =
> to_vc4_crtc_state(crtc->state);
> unsigned int cob_size;
> u32 val;
> int fifo_lines;
> @@ -104,7 +105,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc
> *crtc,
> * Read vertical scanline which is currently composed for our
> * pixelvalve by the HVS, and also the scaler status.
> */
> - val = HVS_READ(SCALER_DISPSTATX(vc4_crtc->channel));
> + val = HVS_READ(SCALER_DISPSTATX(vc4_crtc_state->assigned_channel));
>
> /* Get optional system timestamp after query. */
> if (etime)
> @@ -124,7 +125,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc
> *crtc,
> *hpos += mode->crtc_htotal / 2;
> }
>
> - cob_size = vc4_crtc_get_cob_allocation(vc4, vc4_crtc->channel);
> + cob_size = vc4_crtc_get_cob_allocation(vc4,
> vc4_crtc_state->assigned_channel);
> /* This is the offset we need for translating hvs -> pv scanout pos.
> */
> fifo_lines = cob_size / mode->crtc_hdisplay;
>
> @@ -211,6 +212,7 @@ vc4_crtc_lut_load(struct drm_crtc *crtc)
> struct drm_device *dev = crtc->dev;
> struct vc4_dev *vc4 = to_vc4_dev(dev);
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> + struct vc4_crtc_state *vc4_crtc_state =
> to_vc4_crtc_state(crtc->state);
> u32 i;
>
> /* The LUT memory is laid out with each HVS channel in order,
> @@ -219,7 +221,7 @@ vc4_crtc_lut_load(struct drm_crtc *crtc)
> */
> HVS_WRITE(SCALER_GAMADDR,
> SCALER_GAMADDR_AUTOINC |
> - (vc4_crtc->channel * 3 * crtc->gamma_size));
> + (vc4_crtc_state->assigned_channel * 3 * crtc->gamma_size));
>
> for (i = 0; i < crtc->gamma_size; i++)
> HVS_WRITE(SCALER_GAMDATA, vc4_crtc->lut_r[i]);
> @@ -392,7 +394,7 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
> drm_print_regset32(, _crtc->regset);
> }
>
> - if (vc4_crtc->channel == 2) {
> + if (vc4_crtc->data->hvs_output == 2) {
> u32 dispctrl;
> u32 dsp3_mux;
Looks like this hunk is maybe supposed to be in the hvs_output rename patch?
> @@ -419,7 +421,7 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
> if (!vc4_state->feed_txp)
> vc4_crtc_config_pv(crtc);
>
> - HVS_WRITE(SCALER_DISPBKGNDX(vc4_crtc->channel),
> + HVS_WRITE(SCALER_DISPBKGNDX(vc4_state->assigned_channel),
> SCALER_DISPBKGND_AUTOHS |
> SCALER_DISPBKGND_GAMMA |
> (interlace ? SCALER_DISPBKGND_INTERLACE : 0));
> @@ -451,7 +453,8 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
> struct drm_device *dev = crtc->dev;
> struct vc4_dev *vc4 = to_vc4_dev(dev);
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> - u32 chan = vc4_crtc->channel;
> + struct vc4_crtc_state *vc4_crtc_state = to_vc4_crtc_state(old_state);
> + u32 chan = vc4_crtc_state->assigned_channel;
> int ret;
> require_hvs_enabled(dev);
>
> @@ -530,12 +533,12 @@ static void vc4_crtc_update_dlist(struct drm_crtc *crtc)
> crtc->state->event = NULL;
> }
>
> - HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel),
> + HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel),
> vc4_state->mm.start);
>
> spin_unlock_irqrestore(>event_lock, flags);
>
Re: [PATCH v3 032/105] drm/vc4: crtc: Enable and disable the PV in atomic_enable / disable
On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote: > > The VIDEN bit in the pixelvalve currently being used to enable or disable > the pixelvalve seems to not be enough in some situations, which whill end > up with the pixelvalve stalling. > > In such a case, even re-enabling VIDEN doesn't bring it back and we need to > clear the FIFO. This can only be done if the pixelvalve is disabled though. > > In order to overcome this, we can configure the pixelvalve during > mode_set_no_fb, but only enable it in atomic_enable and flush the FIFO > there, and in atomic_disable disable the pixelvalve again. What displays has this been tested with? Getting this sequencing right is so painful, and things like DSI are tricky to get to light up.
Re: [PATCH v3 020/105] drm/vc4: plane: Create overlays for any CRTC
On Wed, May 27, 2020 at 8:49 AM Maxime Ripard wrote:
>
> Now that we have everything in place, we can now register all the overlay
> planes that can be assigned to all the CRTCs.
>
> This has two side effects:
>
> - The number of overlay planes is reduced from 24 to 8. This is temporary
> and will be increased again in the next patch.
>
> - The ID of the various planes is changed again, and we will now have all
> the primary planes, then all the overlay planes and finally the cursor
> planes. This shouldn't cause any issue since the ordering between
> primary, overlay and cursor planes is preserved.
>
> Signed-off-by: Maxime Ripard
Honestly, I'd squash this with the previous two patches, the
individual refactors don't make much sense on their own or simplify
this patch I think. Either way, patch 17-29 r-b.
> ---
> drivers/gpu/drm/vc4/vc4_plane.c | 35 +-
> 1 file changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 824c188980b0..5335123ae2a0 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -1378,26 +1378,27 @@ int vc4_plane_create_additional_planes(struct
> drm_device *drm)
> struct drm_crtc *crtc;
> unsigned int i;
>
> - drm_for_each_crtc(crtc, drm) {
> - /* Set up some arbitrary number of planes. We're not limited
> -* by a set number of physical registers, just the space in
> -* the HVS (16k) and how small an plane can be (28 bytes).
> -* However, each plane we set up takes up some memory, and
> -* increases the cost of looping over planes, which atomic
> -* modesetting does quite a bit. As a result, we pick a
> -* modest number of planes to expose, that should hopefully
> -* still cover any sane usecase.
> -*/
> - for (i = 0; i < 8; i++) {
> - struct drm_plane *plane =
> - vc4_plane_init(drm, DRM_PLANE_TYPE_OVERLAY);
> + /* Set up some arbitrary number of planes. We're not limited
> +* by a set number of physical registers, just the space in
> +* the HVS (16k) and how small an plane can be (28 bytes).
> +* However, each plane we set up takes up some memory, and
> +* increases the cost of looping over planes, which atomic
> +* modesetting does quite a bit. As a result, we pick a
> +* modest number of planes to expose, that should hopefully
> +* still cover any sane usecase.
> +*/
> + for (i = 0; i < 8; i++) {
> + struct drm_plane *plane =
> + vc4_plane_init(drm, DRM_PLANE_TYPE_OVERLAY);
>
> - if (IS_ERR(plane))
> - continue;
> + if (IS_ERR(plane))
> + continue;
>
> - plane->possible_crtcs = drm_crtc_mask(crtc);
> - }
> + plane->possible_crtcs =
> + GENMASK(drm->mode_config.num_crtc - 1, 0);
> + }
>
> + drm_for_each_crtc(crtc, drm) {
> /* Set up the legacy cursor after overlay initialization,
> * since we overlay planes on the CRTC in the order they were
> * initialized.
> --
> git-series 0.9.1
Re: [PATCH v3 016/105] drm/vc4: plane: Improve LBM usage
On Wed, May 27, 2020 at 8:49 AM Maxime Ripard wrote:
>
> From: Dave Stevenson
>
> LBM allocations were always taking the worst case sizing of
> max(src_width, dst_width) * 16. This is significantly over
> the required sizing, and stops us rendering multiple 4k images
> to the screen.
>
> Add some of the additional constraints to more accurately
> describe the LBM requirements.
>
> Signed-off-by: Dave Stevenson
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/vc4/vc4_plane.c | 31 ---
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 1575c05e3106..602927745f84 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -142,9 +142,10 @@ static const struct hvs_format *vc4_get_hvs_format(u32
> drm_format)
> return NULL;
> }
>
> -static enum vc4_scaling_mode vc4_get_scaling_mode(u32 src, u32 dst)
> +static enum vc4_scaling_mode vc4_get_scaling_mode(u32 src, u32 dst,
> + bool chroma_vrep)
> {
> - if (dst == src)
> + if (dst == src && !chroma_vrep)
> return VC4_SCALING_NONE;
> if (3 * dst >= 2 * src)
> return VC4_SCALING_PPF;
> @@ -369,9 +370,11 @@ static int vc4_plane_setup_clipping_and_scaling(struct
> drm_plane_state *state)
> return ret;
>
> vc4_state->x_scaling[0] = vc4_get_scaling_mode(vc4_state->src_w[0],
> - vc4_state->crtc_w);
> + vc4_state->crtc_w,
> + false);
> vc4_state->y_scaling[0] = vc4_get_scaling_mode(vc4_state->src_h[0],
> - vc4_state->crtc_h);
> + vc4_state->crtc_h,
> + false);
>
> vc4_state->is_unity = (vc4_state->x_scaling[0] == VC4_SCALING_NONE &&
>vc4_state->y_scaling[0] == VC4_SCALING_NONE);
> @@ -384,10 +387,12 @@ static int vc4_plane_setup_clipping_and_scaling(struct
> drm_plane_state *state)
>
> vc4_state->x_scaling[1] =
> vc4_get_scaling_mode(vc4_state->src_w[1],
> -vc4_state->crtc_w);
> +vc4_state->crtc_w,
> +v_subsample == 2);
> vc4_state->y_scaling[1] =
> vc4_get_scaling_mode(vc4_state->src_h[1],
> -vc4_state->crtc_h);
> +vc4_state->crtc_h,
> +v_subsample == 2);
>
> /* YUV conversion requires that horizontal scaling be enabled
> * on the UV plane even if vc4_get_scaling_mode() returned
The change above isn't mentioned in the commit message and I don't
understand what's going on. It should be split out with an
explanation.
> @@ -437,10 +442,7 @@ static void vc4_write_ppf(struct vc4_plane_state
> *vc4_state, u32 src, u32 dst)
> static u32 vc4_lbm_size(struct drm_plane_state *state)
> {
> struct vc4_plane_state *vc4_state = to_vc4_plane_state(state);
> - /* This is the worst case number. One of the two sizes will
> -* be used depending on the scaling configuration.
> -*/
> - u32 pix_per_line = max(vc4_state->src_w[0], (u32)vc4_state->crtc_w);
> + u32 pix_per_line;
> u32 lbm;
>
> /* LBM is not needed when there's no vertical scaling. */
> @@ -448,6 +450,11 @@ static u32 vc4_lbm_size(struct drm_plane_state *state)
> vc4_state->y_scaling[1] == VC4_SCALING_NONE)
> return 0;
>
> + if (vc4_state->x_scaling[0] == VC4_SCALING_TPZ)
> + pix_per_line = vc4_state->crtc_w;
> + else
> + pix_per_line = vc4_state->src_w[0];
Looks like it's also crtc_w for RGB or 4:4:4 and HPPF in (0.5,1.0].
Maybe drop a note in here that we're not covering that case, but src_w
> crtc_w so it's safe at least.
> +
> if (!vc4_state->is_yuv) {
> if (vc4_state->y_scaling[0] == VC4_SCALING_TPZ)
> lbm = pix_per_line * 8;
> @@ -583,7 +590,9 @@ static int vc4_plane_allocate_lbm(struct drm_plane_state
> *state)
> spin_lock_irqsave(>hvs->mm_lock, irqflags);
> ret = drm_mm_insert_node_generic(>hvs->lbm_mm,
> _state->lbm,
> -lbm_size, 32, 0, 0);
> +lbm_size,
> +vc4->hvs->hvs5 ? 64 : 32,
> +
Re: [PATCH v3 015/105] drm/vc4: hvs: Boost the core clock during modeset
On Wed, May 27, 2020 at 8:49 AM Maxime Ripard wrote: > > In order to prevent timeouts and stalls in the pipeline, the core clock > needs to be maxed at 500MHz during a modeset on the BCM2711. Like, the whole system's core clock? How is it reasonable for some device driver to crank the system's core clock up and back down to some fixed-in-the-driver frequency? Sounds like you need some sort of opp thing here. Patch 13,14 r-b.
Re: [PATCH v3 012/105] drm/vc4: drv: Support BCM2711
On Wed, May 27, 2020 at 8:49 AM Maxime Ripard wrote:
>
> The BCM2711 has a reworked display pipeline, and the load tracker needs
> some adjustement to operate properly. Let's add a compatible for BCM2711
> and disable the load tracker until properly supported.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/vc4/vc4_drv.c | 1 +-
> drivers/gpu/drm/vc4/vc4_drv.h | 3 ++-
> drivers/gpu/drm/vc4/vc4_kms.c | 42 +++---
> drivers/gpu/drm/vc4/vc4_plane.c | 5 -
> 4 files changed, 38 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index 76f93b662766..d7f554a6f0ed 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -364,6 +364,7 @@ static int vc4_platform_drm_remove(struct platform_device
> *pdev)
> }
>
> static const struct of_device_id vc4_of_match[] = {
> + { .compatible = "brcm,bcm2711-vc5", },
> { .compatible = "brcm,bcm2835-vc4", },
> { .compatible = "brcm,cygnus-vc4", },
> {},
Patch 6 Acked-by: Eric Anholt
Patch 7-11 Reviewed-by: Eric Anholt
This one to start probing needs to move later in the series once the
vc5 support is actually present in the driver.
Re: [PATCH v5 24/38] drm: v3d: fix common struct sg_table related issues
On Wed, May 13, 2020 at 6:33 AM Marek Szyprowski
wrote:
>
> The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function
> returns the number of the created entries in the DMA address space.
> However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and
> dma_unmap_sg must be called with the original number of the entries
> passed to the dma_map_sg().
>
> struct sg_table is a common structure used for describing a non-contiguous
> memory buffer, used commonly in the DRM and graphics subsystems. It
> consists of a scatterlist with memory pages and DMA addresses (sgl entry),
> as well as the number of scatterlist entries: CPU pages (orig_nents entry)
> and DMA mapped pages (nents entry).
>
> It turned out that it was a common mistake to misuse nents and orig_nents
> entries, calling DMA-mapping functions with a wrong number of entries or
> ignoring the number of mapped entries returned by the dma_map_sg()
> function.
>
> To avoid such issues, lets use a common dma-mapping wrappers operating
> directly on the struct sg_table objects and use scatterlist page
> iterators where possible. This, almost always, hides references to the
> nents and orig_nents entries, making the code robust, easier to follow
> and copy/paste safe.
>
> Signed-off-by: Marek Szyprowski
Reviewed-by: Eric Anholt
Re: [PATCH] MAINTAINERS: Update Raspberry Pi development repository
On Mon, May 11, 2020 at 4:02 AM Nicolas Saenz Julienne wrote: > > Eric Anholt's repo isn't used anymore. List current one. Acked-by: Eric Anholt
Re: [PATCH v2] drm/msm: Check for powered down HW in the devfreq callbacks
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)
Re: [PATCH] drm/msm: Check for powered down HW in the devfreq callbacks
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.
Re: [PATCH 1/5] clk: inherit clocks enabled by bootloader
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
Re: [PATCH 4/4] cpufreq: add driver for Raspbery Pi
Nicolas Saenz Julienne writes:
> Raspberry Pi's firmware offers and interface though which update it's
> performance requirements. It allows us to request for specific runtime
> frequencies, which the firmware might or might not respect, depending on
> the firmware configuration and thermals.
>
> As the maximum and minimum frequencies are configurable in the firmware
> there is no way to know in advance their values. So the Raspberry Pi
> cpufreq driver queries them, builds an opp frequency table to then
> launch cpufreq-dt.
>
> Signed-off-by: Nicolas Saenz Julienne
> ---
>
> Changes since RFC:
> - Alphabetically ordered relevant stuff
> - Updated Kconfig to select firmware interface
> - Correctly unref clk_dev after use
> - Remove all opps on failure
> - Remove use of dev_pm_opp_set_sharing_cpus()
>
> drivers/cpufreq/Kconfig.arm | 8 +++
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/raspberrypi-cpufreq.c | 84 +++
> 3 files changed, 93 insertions(+)
> create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index f8129edc145e..556d432cc826 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -133,6 +133,14 @@ config ARM_QCOM_CPUFREQ_HW
> The driver implements the cpufreq interface for this HW engine.
> Say Y if you want to support CPUFreq HW.
>
> +config ARM_RASPBERRYPI_CPUFREQ
> + tristate "Raspberry Pi cpufreq support"
> + select RASPBERRYPI_FIRMWARE
> + help
> + This adds the CPUFreq driver for Raspberry Pi
> +
> + If in doubt, say N.
> +
> config ARM_S3C_CPUFREQ
> bool
> help
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 689b26c6f949..121c1acb66c0 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)+= pxa2xx-cpufreq.o
> obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o
> obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW)+= qcom-cpufreq-hw.o
> obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO) += qcom-cpufreq-kryo.o
> +obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ)+= raspberrypi-cpufreq.o
> obj-$(CONFIG_ARM_S3C2410_CPUFREQ)+= s3c2410-cpufreq.o
> obj-$(CONFIG_ARM_S3C2412_CPUFREQ)+= s3c2412-cpufreq.o
> obj-$(CONFIG_ARM_S3C2416_CPUFREQ)+= s3c2416-cpufreq.o
> diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c
> b/drivers/cpufreq/raspberrypi-cpufreq.c
> new file mode 100644
> index ..2b3a195a9d37
> --- /dev/null
> +++ b/drivers/cpufreq/raspberrypi-cpufreq.c
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Raspberry Pi cpufreq driver
> + *
> + * Copyright (C) 2019, Nicolas Saenz Julienne
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static const struct of_device_id machines[] __initconst = {
> + { .compatible = "raspberrypi,3-model-b-plus" },
> + { .compatible = "raspberrypi,3-model-b" },
> + { .compatible = "raspberrypi,2-model-b" },
> + { /* sentinel */ }
> +};
I think I'd skip the compatible string check here. The firmware's
clock-management should be well-tested by folks playing with clocking in
the downstream tree. There aren't any firmware differences in the
processing of these clock management packets, to my recollection.
Other than that, I'm happy with the series and would give it my
acked-by.
signature.asc
Description: PGP signature
Re: [PATCH 1/4] clk: bcm2835: remove pllb
Nicolas Saenz Julienne writes: > Raspberry Pi's firmware controls this pll, we should use the firmware > interface to access it. > > Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt If someone ever has a non-rpi 2835 to support, they can resurrect this. signature.asc Description: PGP signature
Re: [PATCH 3/4] clk: bcm2835: register Raspberry Pi's firmware clk device
Nicolas Saenz Julienne writes: > Registers clk-raspberrypi as a platform device as part of the driver's > probe sequence. Similar to how we have VCHI register platform devices for the services VCHI provides, shouldn't we have the firmware driver register the device for clk_raspberrypi? Or put the clk provider in the fw driver instead of a separate driver (no opinion on my part). signature.asc Description: PGP signature
Re: [PATCH 1/2 v2] drm/doc: Allow new UAPI to be used once it's in drm-next/drm-misc-next.
Daniel Vetter writes: > On Wed, Apr 24, 2019 at 03:06:38PM -0700, Eric Anholt wrote: >> I was trying to figure out if it was permissible to merge the Mesa >> side of V3D's CSD support yet while it's in drm-misc-next but not >> drm-next, and developers on #dri-devel IRC had differing opinions of >> what the requirement was. >> >> v2: Restrict to just drm-next or drm-misc-next on airlied's request. > > Personally I think that's a bit too strict (if people want to screw up, > they will be able to anyway). But since I'm all for clearer rules where > possible, this has my support too. > > Reviewed-by: Daniel Vetter Pushed to drm-misc-next now. signature.asc Description: PGP signature
Re: [PATCH 0/3] pinctrl: bcm: Allow PINCTRL_BCM2835 for ARCH_BRCMSTB
Florian Fainelli writes: > Hi Linus, > > This patch series allows making use of the pinctrl-bcm2835 driver on > ARCH_BRCMSTB where it is also used. Binding document is updated, and > then the Kconfig language is updated to allow selecting this driver with > ARCH_BRCMSTB, finally, Al updates the logic to account for the > additional registers that were added on 7211. As far as platform maintainer goes, patch 1-2 are: Reviewed-by: Eric Anholt and patch 3 is: Acked-by: Eric Anholt signature.asc Description: PGP signature
Re: [PATCH 1/2] clk: bcm: Make BCM2835 clock drivers selectable
Florian Fainelli writes: > Make the BCM2835 clock driver selectable by other > architectures/platforms. ARCH_BRCMSTB will be selecting that driver in > the next commit since new chips like 7211 use the same CPRMAN clock > controller that this driver supports. These two are: Reviewed-by: Eric Anholt signature.asc Description: PGP signature
Re: [PATCH] spi: Allow selecting BCM2835 SPI controllers on ARCH_BRCMSTB
Florian Fainelli writes: > ARCH_BRCMSTB platforms have the BCM2835 SPI controllers (normal and > auxiliary), allow selecting the two drivers on such platforms. > > Signed-off-by: Florian Fainelli Reviewed-by: Eric Anholt signature.asc Description: PGP signature
Re: [PATCH] arm64: bcm2835: Add missing dependency on MFD_CORE.
Stefan Wahren writes:
>> Eric Anholt hat am 8. März 2019 um 22:07 geschrieben:
>>
>>
>> When adding the MFD dependency for power domains and WDT in bcm2835, I
>> added it only on the arm32 side and missed it for arm64.
>>
>> Fixes: 5e6acc3e678e ("bcm2835-pm: Move bcm2835-watchdog's DT probe to an
>> MFD.")
>> Signed-off-by: Eric Anholt
>> Reported-by: Stefan Wahren
>
> Acked-by: Stefan Wahren
Merged to bcm2835-driver-next and sent an updated PR.
signature.asc
Description: PGP signature
[GIT PULL] bcm2835-drivers-next-2019-03-12
Hi Florian, here's an updated PR on top of bcm2835-drivers-next-2019-02-01. The following changes since commit e1dc2b2e1bef7237fd8fc055fe1ec2a6ff001f91: ARM: bcm283x: Switch V3D over to using the PM driver instead of firmware. (2019-02-01 10:34:32 +0100) are available in the Git repository at: git://github.com/anholt/linux tags/bcm2835-drivers-next-2019-03-12 for you to fetch changes up to 7a9b6be9fe58194d9a349159176e8cc0d8f10ef8: arm64: bcm2835: Add missing dependency on MFD_CORE. (2019-03-12 13:01:05 -0700) This pull request brings in a build fix for arm64 with bcm2835 enabled, and fixes the driver in the presence of -EPROBE_DEFER. Eric Anholt (3): soc: bcm: bcm2835-pm: Fix PM_IMAGE_PERI power domain support. soc: bcm: bcm2835-pm: Fix error paths of initialization. arm64: bcm2835: Add missing dependency on MFD_CORE. arch/arm64/Kconfig.platforms| 1 + drivers/soc/bcm/bcm2835-power.c | 49 +++-- 2 files changed, 43 insertions(+), 7 deletions(-)
Re: [PATCH 1/4] drm: Add helpers for locking an array of BO reservations.
Rob Herring writes: > On Fri, Mar 8, 2019 at 10:17 AM Eric Anholt wrote: >> >> Now that we have the reservation object in the GEM object, it's easy >> to provide a helper for this common case. Noticed while reviewing >> panfrost and lima drivers. This particular version came out of v3d, >> which in turn was a copy from vc4. >> >> Signed-off-by: Eric Anholt >> --- >> drivers/gpu/drm/drm_gem.c | 76 +++ >> include/drm/drm_gem.h | 4 +++ >> 2 files changed, 80 insertions(+) > > Sweet! I was about to go write this same patch. You are changing the > license from GPL to MIT copying the v3d version, but I guess you have > rights to do that. > > FWIW, > > Acked-by: Rob Herring Was this just for this one patch, or the series? I don't think I can merge without a consumer. signature.asc Description: PGP signature
Re: [PATCH v4 2/2] drm/v3d: Add support for V3D v4.2.
Dave Emett writes: > On Fri, 8 Mar 2019 at 17:43, Eric Anholt wrote: >> >> No compatible string for it yet, just the version-dependent changes. >> They've now tied the hub and the core interrupt lines into a single >> interrupt line coming out of the block. It also turns out I made a >> mistake in modeling the V3D v3.3 and v4.1 bridge as a part of V3D >> itself -- the bridge is going away in favor of an external reset >> controller in a larger HW module. >> >> v2: Use consistent checks for whether we're on 4.2, and fix a leak in >> an error path. >> v3: Use more general means of determining if the current 4.2 changes >> are in place, as apparently other platforms may switch back (noted >> by Dave). Update the binding doc. >> v4: Improve error handling for IRQ init. >> >> Signed-off-by: Eric Anholt > > Reviewed-by: Dave Emett Pushed to drm-misc-next. Thanks! signature.asc Description: PGP signature
Re: [GIT PULL 2/2] bcm2835-drivers-next-2019-03-04
Florian Fainelli writes: > On 3/4/19 4:02 PM, Eric Anholt wrote: >> The following changes since commit e1dc2b2e1bef7237fd8fc055fe1ec2a6ff001f91: >> >> ARM: bcm283x: Switch V3D over to using the PM driver instead of firmware. >> (2019-02-01 10:34:32 +0100) >> >> are available in the Git repository at: >> >> git://github.com/anholt/linux tags/bcm2835-drivers-next-2019-03-04 >> >> for you to fetch changes up to 4deabfae643d8852c643664d9088a647abfaa5d0: >> >> soc: bcm: bcm2835-pm: Fix error paths of initialization. (2019-03-04 >> 15:33:14 -0800) >> >> >> This pull request brings in two fixes for the new native bcm2835 power >> domains driver. >> >> > > Looks like Stefan found an arm64 build configuration that failed linking > with the driver, do you want to add an additional fix to this pull > request that addresses that? Took a look at it now, sent a fix. signature.asc Description: PGP signature
[PATCH] arm64: bcm2835: Add missing dependency on MFD_CORE.
When adding the MFD dependency for power domains and WDT in bcm2835, I
added it only on the arm32 side and missed it for arm64.
Fixes: 5e6acc3e678e ("bcm2835-pm: Move bcm2835-watchdog's DT probe to an MFD.")
Signed-off-by: Eric Anholt
Reported-by: Stefan Wahren
---
This fix is for the bcm2835-drivers-next branch that has been pulled
to Florian's drivers-next branch.
arch/arm64/Kconfig.platforms | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 251ecf34cb02..50f9fb562059 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -27,6 +27,7 @@ config ARCH_BCM2835
bool "Broadcom BCM2835 family"
select TIMER_OF
select GPIOLIB
+ select MFD_CORE
select PINCTRL
select PINCTRL_BCM2835
select ARM_AMBA
--
2.20.1
Re: [PATCH v3 1/3] drm/v3d: Add support for V3D v4.2.
Eric Anholt writes:
> [ Unknown signature status ]
> Dave Emett writes:
>
>> Sorry, a few things I thought of after sending the Reviewed-by email...
>>
>>> + v3d->reset = devm_reset_control_get_exclusive(dev, NULL);
>>> + if (IS_ERR(v3d->reset)) {
>>> + ret = PTR_ERR(v3d->reset);
>>> +
>>> + if (ret == -EPROBE_DEFER)
>>> + goto dev_free;
>> Might be preferable to make this explicitly check against the
>> not-found error code (whatever that is)? As in if (not found)
>> else . Similarly...
>
> You won't have both a bridge and an external reset controller in the DT,
> so I'm not clear on what functional change you're looking for. You're
> just concerned about what the return code from this function is?
> -EPROBE_DEFER is the only one that matters from a probe, really.
>
>>> + if (platform_get_irq(v3d->pdev, 1) < 0) {
>> This should probably explicitly check for not-found rather than any
>> error. As-is, we might silently go down the single-interrupt-line path
>> on a platform with 2 interrupt lines if platform_get_irq(v3d->pdev, 1)
>> hits some other error.
>
> If I do the -EPROBE_DEFER check here, will that be good enough for you?
>
>>> + ret = devm_request_irq(v3d->dev,
>>> platform_get_irq(v3d->pdev, 0),
>>> + v3d_hub_irq, IRQF_SHARED,
>>> + "v3d_hub", v3d);
>>> + ret = devm_request_irq(v3d->dev,
>>> platform_get_irq(v3d->pdev, 1),
>>> + v3d_irq, IRQF_SHARED,
>>> + "v3d_core0", v3d);
>> Not introduced by this change, but return value from first
>> devm_request_irq ignored here?
>
> True, but let's handle separate bugs separately.
Resubmitted with the fix anyway.
signature.asc
Description: PGP signature
Re: [PATCH 1/4] drm: Add helpers for locking an array of BO reservations.
Rob Herring writes: > On Fri, Mar 8, 2019 at 10:17 AM Eric Anholt wrote: >> >> Now that we have the reservation object in the GEM object, it's easy >> to provide a helper for this common case. Noticed while reviewing >> panfrost and lima drivers. This particular version came out of v3d, >> which in turn was a copy from vc4. >> >> Signed-off-by: Eric Anholt >> --- >> drivers/gpu/drm/drm_gem.c | 76 +++ >> include/drm/drm_gem.h | 4 +++ >> 2 files changed, 80 insertions(+) > > Sweet! I was about to go write this same patch. You are changing the > license from GPL to MIT copying the v3d version, but I guess you have > rights to do that. The original vc4 copy I had was MIT, anyway. signature.asc Description: PGP signature
[PATCH v4 1/2] drm/v3d: Handle errors from IRQ setup.
Noted in review by Dave Emett for V3D 4.2 support.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/v3d_drv.c | 8 ++--
drivers/gpu/drm/v3d/v3d_drv.h | 2 +-
drivers/gpu/drm/v3d/v3d_irq.c | 13 +++--
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 3680ebd229f2..f9906cac7a88 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -311,14 +311,18 @@ static int v3d_platform_drm_probe(struct platform_device
*pdev)
if (ret)
goto dev_destroy;
- v3d_irq_init(v3d);
+ ret = v3d_irq_init(v3d);
+ if (ret)
+ goto gem_destroy;
ret = drm_dev_register(drm, 0);
if (ret)
- goto gem_destroy;
+ goto irq_disable;
return 0;
+irq_disable:
+ v3d_irq_disable(v3d);
gem_destroy:
v3d_gem_destroy(drm);
dev_destroy:
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index d856159bd007..bb58ecb9d9c5 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -304,7 +304,7 @@ void v3d_reset(struct v3d_dev *v3d);
void v3d_invalidate_caches(struct v3d_dev *v3d);
/* v3d_irq.c */
-void v3d_irq_init(struct v3d_dev *v3d);
+int v3d_irq_init(struct v3d_dev *v3d);
void v3d_irq_enable(struct v3d_dev *v3d);
void v3d_irq_disable(struct v3d_dev *v3d);
void v3d_irq_reset(struct v3d_dev *v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index 69338da70ddc..29d746cfce57 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -156,7 +156,7 @@ v3d_hub_irq(int irq, void *arg)
return status;
}
-void
+int
v3d_irq_init(struct v3d_dev *v3d)
{
int ret, core;
@@ -173,13 +173,22 @@ v3d_irq_init(struct v3d_dev *v3d)
ret = devm_request_irq(v3d->dev, platform_get_irq(v3d->pdev, 0),
v3d_hub_irq, IRQF_SHARED,
"v3d_hub", v3d);
+ if (ret)
+ goto fail;
+
ret = devm_request_irq(v3d->dev, platform_get_irq(v3d->pdev, 1),
v3d_irq, IRQF_SHARED,
"v3d_core0", v3d);
if (ret)
- dev_err(v3d->dev, "IRQ setup failed: %d\n", ret);
+ goto fail;
v3d_irq_enable(v3d);
+ return 0;
+
+fail:
+ if (ret != -EPROBE_DEFER)
+ dev_err(v3d->dev, "IRQ setup failed: %d\n", ret);
+ return ret;
}
void
--
2.20.1
[PATCH v4 2/2] drm/v3d: Add support for V3D v4.2.
No compatible string for it yet, just the version-dependent changes.
They've now tied the hub and the core interrupt lines into a single
interrupt line coming out of the block. It also turns out I made a
mistake in modeling the V3D v3.3 and v4.1 bridge as a part of V3D
itself -- the bridge is going away in favor of an external reset
controller in a larger HW module.
v2: Use consistent checks for whether we're on 4.2, and fix a leak in
an error path.
v3: Use more general means of determining if the current 4.2 changes
are in place, as apparently other platforms may switch back (noted
by Dave). Update the binding doc.
v4: Improve error handling for IRQ init.
Signed-off-by: Eric Anholt
---
.../devicetree/bindings/gpu/brcm,bcm-v3d.txt | 11 +++--
drivers/gpu/drm/v3d/v3d_drv.c | 21 +++--
drivers/gpu/drm/v3d/v3d_drv.h | 2 +
drivers/gpu/drm/v3d/v3d_gem.c | 12 -
drivers/gpu/drm/v3d/v3d_irq.c | 45 ++-
5 files changed, 71 insertions(+), 20 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.txt
b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.txt
index c907aa8dd755..b2df82b44625 100644
--- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.txt
+++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.txt
@@ -6,15 +6,20 @@ For V3D 2.x, see brcm,bcm-vc4.txt.
Required properties:
- compatible: Should be "brcm,7268-v3d" or "brcm,7278-v3d"
- reg: Physical base addresses and lengths of the register areas
-- reg-names: Names for the register areas. The "hub", "bridge", and "core0"
+- reg-names: Names for the register areas. The "hub" and "core0"
register areas are always required. The "gca" register area
- is required if the GCA cache controller is present.
+ is required if the GCA cache controller is present. The
+ "bridge" register area is required if an external reset
+ controller is not present.
- interrupts: The interrupt numbers. The first interrupt is for the hub,
- while the following interrupts are for the cores.
+ while the following interrupts are separate interrupt lines
+ for the cores (if they don't share the hub's interrupt).
See bindings/interrupt-controller/interrupts.txt
Optional properties:
- clocks: The core clock the unit runs on
+- resets: The reset line for v3d, if not using a mapping of the bridge
+ See bindings/reset/reset.txt
v3d {
compatible = "brcm,7268-v3d";
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index f9906cac7a88..89b7567d550f 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
@@ -264,10 +265,6 @@ static int v3d_platform_drm_probe(struct platform_device
*pdev)
v3d->pdev = pdev;
drm = >drm;
- ret = map_regs(v3d, >bridge_regs, "bridge");
- if (ret)
- goto dev_free;
-
ret = map_regs(v3d, >hub_regs, "hub");
if (ret)
goto dev_free;
@@ -282,6 +279,22 @@ static int v3d_platform_drm_probe(struct platform_device
*pdev)
v3d->cores = V3D_GET_FIELD(ident1, V3D_HUB_IDENT1_NCORES);
WARN_ON(v3d->cores > 1); /* multicore not yet implemented */
+ v3d->reset = devm_reset_control_get_exclusive(dev, NULL);
+ if (IS_ERR(v3d->reset)) {
+ ret = PTR_ERR(v3d->reset);
+
+ if (ret == -EPROBE_DEFER)
+ goto dev_free;
+
+ v3d->reset = NULL;
+ ret = map_regs(v3d, >bridge_regs, "bridge");
+ if (ret) {
+ dev_err(dev,
+ "Failed to get reset control or bridge regs\n");
+ goto dev_free;
+ }
+ }
+
if (v3d->ver < 41) {
ret = map_regs(v3d, >gca_regs, "gca");
if (ret)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index bb58ecb9d9c5..b5efac7894ca 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -33,6 +33,7 @@ struct v3d_dev {
* and revision.
*/
int ver;
+ bool single_irq_line;
struct device *dev;
struct platform_device *pdev;
@@ -41,6 +42,7 @@ struct v3d_dev {
void __iomem *bridge_regs;
void __iomem *gca_regs;
struct clk *clk;
+ struct reset_control *reset;
/* Virtual and DMA addresses of the single shared page table. */
volatile u32 *pt;
diff --git a/driv
Re: [PATCH v3 1/3] drm/v3d: Add support for V3D v4.2.
Dave Emett writes:
> Sorry, a few things I thought of after sending the Reviewed-by email...
>
>> + v3d->reset = devm_reset_control_get_exclusive(dev, NULL);
>> + if (IS_ERR(v3d->reset)) {
>> + ret = PTR_ERR(v3d->reset);
>> +
>> + if (ret == -EPROBE_DEFER)
>> + goto dev_free;
> Might be preferable to make this explicitly check against the
> not-found error code (whatever that is)? As in if (not found)
> else . Similarly...
You won't have both a bridge and an external reset controller in the DT,
so I'm not clear on what functional change you're looking for. You're
just concerned about what the return code from this function is?
-EPROBE_DEFER is the only one that matters from a probe, really.
>> + if (platform_get_irq(v3d->pdev, 1) < 0) {
> This should probably explicitly check for not-found rather than any
> error. As-is, we might silently go down the single-interrupt-line path
> on a platform with 2 interrupt lines if platform_get_irq(v3d->pdev, 1)
> hits some other error.
If I do the -EPROBE_DEFER check here, will that be good enough for you?
>> + ret = devm_request_irq(v3d->dev, platform_get_irq(v3d->pdev,
>> 0),
>> + v3d_hub_irq, IRQF_SHARED,
>> + "v3d_hub", v3d);
>> + ret = devm_request_irq(v3d->dev, platform_get_irq(v3d->pdev,
>> 1),
>> + v3d_irq, IRQF_SHARED,
>> + "v3d_core0", v3d);
> Not introduced by this change, but return value from first
> devm_request_irq ignored here?
True, but let's handle separate bugs separately.
signature.asc
Description: PGP signature
[PATCH 0/4] drm/v3d: Add drm core helper, use shmem helpers
This was inspired by Rob's respin of the shmem helpers patch for panfrost (which the final patch depends on). And, looking at panfrost and lima, recently I realized that we all had some nasty cargo-cult code for BO reservations that we can share now that the reservation object is in the core GEM struct. Eric Anholt (4): drm: Add helpers for locking an array of BO reservations. drm/v3d: Use drm_gem_lock_reservations()/drm_gem_unlock_reservations() drm/v3d: Remove some dead members of struct v3d_bo. drm/v3d: Use the new shmem helpers to reduce driver boilerplate. drivers/gpu/drm/drm_gem.c | 76 + drivers/gpu/drm/v3d/Kconfig | 1 + drivers/gpu/drm/v3d/v3d_bo.c | 309 ++ drivers/gpu/drm/v3d/v3d_drv.c | 27 +-- drivers/gpu/drm/v3d/v3d_drv.h | 25 +-- drivers/gpu/drm/v3d/v3d_gem.c | 67 ++-- drivers/gpu/drm/v3d/v3d_irq.c | 8 +- drivers/gpu/drm/v3d/v3d_mmu.c | 34 +++- include/drm/drm_gem.h | 4 + 9 files changed, 220 insertions(+), 331 deletions(-) -- 2.20.1
[PATCH 4/4] drm/v3d: Use the new shmem helpers to reduce driver boilerplate.
Depends on
https://patchwork.freedesktop.org/patch/290754/?series=57669=1 --
Rob and I have been talking about adding some more help for the
dma_map_sg() code in v3d_mmu.c (which panfrost has a similar version
of), but this already seems like a good cleanup.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/Kconfig | 1 +
drivers/gpu/drm/v3d/v3d_bo.c | 308 ++
drivers/gpu/drm/v3d/v3d_drv.c | 27 +--
drivers/gpu/drm/v3d/v3d_drv.h | 16 +-
drivers/gpu/drm/v3d/v3d_gem.c | 11 +-
drivers/gpu/drm/v3d/v3d_irq.c | 8 +-
drivers/gpu/drm/v3d/v3d_mmu.c | 34 +++-
7 files changed, 134 insertions(+), 271 deletions(-)
diff --git a/drivers/gpu/drm/v3d/Kconfig b/drivers/gpu/drm/v3d/Kconfig
index 1552bf552c94..75a74c45f109 100644
--- a/drivers/gpu/drm/v3d/Kconfig
+++ b/drivers/gpu/drm/v3d/Kconfig
@@ -5,6 +5,7 @@ config DRM_V3D
depends on COMMON_CLK
depends on MMU
select DRM_SCHED
+ select DRM_GEM_SHMEM_HELPER
help
Choose this option if you have a system that has a Broadcom
V3D 3.x or newer GPU, such as BCM7268.
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index dff38bf36c50..b631ade09853 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -25,158 +25,6 @@
#include "v3d_drv.h"
#include "uapi/drm/v3d_drm.h"
-/* Pins the shmem pages, fills in the .pages and .sgt fields of the BO, and
maps
- * it for DMA.
- */
-static int
-v3d_bo_get_pages(struct v3d_bo *bo)
-{
- struct drm_gem_object *obj = >base;
- struct drm_device *dev = obj->dev;
- int npages = obj->size >> PAGE_SHIFT;
- int ret = 0;
-
- mutex_lock(>lock);
- if (bo->pages_refcount++ != 0)
- goto unlock;
-
- if (!obj->import_attach) {
- bo->pages = drm_gem_get_pages(obj);
- if (IS_ERR(bo->pages)) {
- ret = PTR_ERR(bo->pages);
- goto unlock;
- }
-
- bo->sgt = drm_prime_pages_to_sg(bo->pages, npages);
- if (IS_ERR(bo->sgt)) {
- ret = PTR_ERR(bo->sgt);
- goto put_pages;
- }
-
- /* Map the pages for use by the GPU. */
- dma_map_sg(dev->dev, bo->sgt->sgl,
- bo->sgt->nents, DMA_BIDIRECTIONAL);
- } else {
- bo->pages = kcalloc(npages, sizeof(*bo->pages), GFP_KERNEL);
- if (!bo->pages)
- goto put_pages;
-
- drm_prime_sg_to_page_addr_arrays(bo->sgt, bo->pages,
-NULL, npages);
-
- /* Note that dma-bufs come in mapped. */
- }
-
- mutex_unlock(>lock);
-
- return 0;
-
-put_pages:
- drm_gem_put_pages(obj, bo->pages, true, true);
- bo->pages = NULL;
-unlock:
- bo->pages_refcount--;
- mutex_unlock(>lock);
- return ret;
-}
-
-static void
-v3d_bo_put_pages(struct v3d_bo *bo)
-{
- struct drm_gem_object *obj = >base;
-
- mutex_lock(>lock);
- if (--bo->pages_refcount == 0) {
- if (!obj->import_attach) {
- dma_unmap_sg(obj->dev->dev, bo->sgt->sgl,
-bo->sgt->nents, DMA_BIDIRECTIONAL);
- sg_free_table(bo->sgt);
- kfree(bo->sgt);
- drm_gem_put_pages(obj, bo->pages, true, true);
- } else {
- kfree(bo->pages);
- }
- }
- mutex_unlock(>lock);
-}
-
-static struct v3d_bo *v3d_bo_create_struct(struct drm_device *dev,
- size_t unaligned_size)
-{
- struct v3d_dev *v3d = to_v3d_dev(dev);
- struct drm_gem_object *obj;
- struct v3d_bo *bo;
- size_t size = roundup(unaligned_size, PAGE_SIZE);
- int ret;
-
- if (size == 0)
- return ERR_PTR(-EINVAL);
-
- bo = kzalloc(sizeof(*bo), GFP_KERNEL);
- if (!bo)
- return ERR_PTR(-ENOMEM);
- obj = >base;
-
- INIT_LIST_HEAD(>unref_head);
- mutex_init(>lock);
-
- ret = drm_gem_object_init(dev, obj, size);
- if (ret)
- goto free_bo;
-
- spin_lock(>mm_lock);
- ret = drm_mm_insert_node_generic(>mm, >node,
-obj->size >> PAGE_SHIFT,
-GMP_GRANULARITY >> PAGE_SHIFT, 0, 0);
- spin_unlock(>mm_lock);
- if (ret)
- goto free_obj;
-
- return bo;
-
-free_obj:
- drm_gem_object_release(obj);
-free_bo:
- kfree(bo);
- return ERR_PTR(ret);
-}
-
-struct v3d_bo *v3d_bo_creat
[PATCH 1/4] drm: Add helpers for locking an array of BO reservations.
Now that we have the reservation object in the GEM object, it's easy
to provide a helper for this common case. Noticed while reviewing
panfrost and lima drivers. This particular version came out of v3d,
which in turn was a copy from vc4.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/drm_gem.c | 76 +++
include/drm/drm_gem.h | 4 +++
2 files changed, 80 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index ad124f5a6f4d..9fd804f7d7ca 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1233,3 +1233,79 @@ void drm_gem_vunmap(struct drm_gem_object *obj, void
*vaddr)
obj->dev->driver->gem_prime_vunmap(obj, vaddr);
}
EXPORT_SYMBOL(drm_gem_vunmap);
+
+/**
+ * drm_gem_lock_bo_reservations - Sets up the ww context and acquires
+ * the lock on an array of GEM objects.
+ *
+ * Once you've locked your reservations, you'll want to set up space
+ * for your shared fences (if applicable), submit your job, then
+ * drm_gem_unlock_reservations().
+ *
+ * @acquire_ctx - struct ww_acquire_ctx that will be initialized as
+ * part of tracking this set of locked reservations.
+ */
+int
+drm_gem_lock_reservations(struct drm_gem_object **objs, int count,
+ struct ww_acquire_ctx *acquire_ctx)
+{
+ int contended = -1;
+ int i, ret;
+
+ ww_acquire_init(acquire_ctx, _ww_class);
+
+retry:
+ if (contended != -1) {
+ struct drm_gem_object *obj = objs[contended];
+
+ ret = ww_mutex_lock_slow_interruptible(>resv->lock,
+ acquire_ctx);
+ if (ret) {
+ ww_acquire_done(acquire_ctx);
+ return ret;
+ }
+ }
+
+ for (i = 0; i < count; i++) {
+ if (i == contended)
+ continue;
+
+ ret = ww_mutex_lock_interruptible([i]->resv->lock,
+ acquire_ctx);
+ if (ret) {
+ int j;
+
+ for (j = 0; j < i; j++)
+ ww_mutex_unlock([j]->resv->lock);
+
+ if (contended != -1 && contended >= i)
+ ww_mutex_unlock([contended]->resv->lock);
+
+ if (ret == -EDEADLK) {
+ contended = i;
+ goto retry;
+ }
+
+ ww_acquire_done(acquire_ctx);
+ return ret;
+ }
+ }
+
+ ww_acquire_done(acquire_ctx);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_gem_lock_reservations);
+
+void
+drm_gem_unlock_reservations(struct drm_gem_object **objs, int count,
+ struct ww_acquire_ctx *acquire_ctx)
+{
+ int i;
+
+ for (i = 0; i < count; i++)
+ ww_mutex_unlock([i]->resv->lock);
+
+ ww_acquire_fini(acquire_ctx);
+}
+EXPORT_SYMBOL(drm_gem_unlock_reservations);
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 25f1ff2df464..2955aaab3dca 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -384,6 +384,10 @@ void drm_gem_put_pages(struct drm_gem_object *obj, struct
page **pages,
struct drm_gem_object *drm_gem_object_lookup(struct drm_file *filp, u32
handle);
long drm_gem_reservation_object_wait(struct drm_file *filep, u32 handle,
bool wait_all, unsigned long timeout);
+int drm_gem_lock_reservations(struct drm_gem_object **objs, int count,
+ struct ww_acquire_ctx *acquire_ctx);
+void drm_gem_unlock_reservations(struct drm_gem_object **objs, int count,
+struct ww_acquire_ctx *acquire_ctx);
int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
u32 handle, u64 *offset);
int drm_gem_dumb_destroy(struct drm_file *file,
--
2.20.1
[PATCH 2/4] drm/v3d: Use drm_gem_lock_reservations()/drm_gem_unlock_reservations()
Now that we have core helpers, this gets rid of a lot of boilerplate.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/v3d_gem.c | 56 ---
1 file changed, 6 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 0d1e5e0b8042..a4338d56ff9e 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -199,12 +199,8 @@ v3d_unlock_bo_reservations(struct v3d_bo **bos,
int bo_count,
struct ww_acquire_ctx *acquire_ctx)
{
- int i;
-
- for (i = 0; i < bo_count; i++)
- ww_mutex_unlock([i]->base.resv->lock);
-
- ww_acquire_fini(acquire_ctx);
+ drm_gem_unlock_reservations((struct drm_gem_object **)bos, bo_count,
+ acquire_ctx);
}
/* Takes the reservation lock on all the BOs being referenced, so that
@@ -219,52 +215,12 @@ v3d_lock_bo_reservations(struct v3d_bo **bos,
int bo_count,
struct ww_acquire_ctx *acquire_ctx)
{
- int contended_lock = -1;
int i, ret;
- ww_acquire_init(acquire_ctx, _ww_class);
-
-retry:
- if (contended_lock != -1) {
- struct v3d_bo *bo = bos[contended_lock];
-
- ret = ww_mutex_lock_slow_interruptible(>base.resv->lock,
- acquire_ctx);
- if (ret) {
- ww_acquire_done(acquire_ctx);
- return ret;
- }
- }
-
- for (i = 0; i < bo_count; i++) {
- if (i == contended_lock)
- continue;
-
- ret = ww_mutex_lock_interruptible([i]->base.resv->lock,
- acquire_ctx);
- if (ret) {
- int j;
-
- for (j = 0; j < i; j++)
- ww_mutex_unlock([j]->base.resv->lock);
-
- if (contended_lock != -1 && contended_lock >= i) {
- struct v3d_bo *bo = bos[contended_lock];
-
- ww_mutex_unlock(>base.resv->lock);
- }
-
- if (ret == -EDEADLK) {
- contended_lock = i;
- goto retry;
- }
-
- ww_acquire_done(acquire_ctx);
- return ret;
- }
- }
-
- ww_acquire_done(acquire_ctx);
+ ret = drm_gem_lock_reservations((struct drm_gem_object **)bos,
+ bo_count, acquire_ctx);
+ if (ret)
+ return ret;
/* Reserve space for our shared (read-only) fence references,
* before we commit the CL to the hardware.
--
2.20.1
[PATCH 3/4] drm/v3d: Remove some dead members of struct v3d_bo.
vmas was from the previous model of page table management (one per
fd), and vaddr was left over from vc4.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/v3d_bo.c | 1 -
drivers/gpu/drm/v3d/v3d_drv.h | 9 -
2 files changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index 5c83b392b20a..dff38bf36c50 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -117,7 +117,6 @@ static struct v3d_bo *v3d_bo_create_struct(struct
drm_device *dev,
return ERR_PTR(-ENOMEM);
obj = >base;
- INIT_LIST_HEAD(>vmas);
INIT_LIST_HEAD(>unref_head);
mutex_init(>lock);
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index d856159bd007..f5ed915a0f15 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -108,12 +108,6 @@ struct v3d_file_priv {
struct drm_sched_entity sched_entity[V3D_MAX_QUEUES];
};
-/* Tracks a mapping of a BO into a per-fd address space */
-struct v3d_vma {
- struct v3d_page_table *pt;
- struct list_head list; /* entry in v3d_bo.vmas */
-};
-
struct v3d_bo {
struct drm_gem_object base;
@@ -124,9 +118,6 @@ struct v3d_bo {
u32 pages_refcount;
struct page **pages;
struct sg_table *sgt;
- void *vaddr;
-
- struct list_head vmas;/* list of v3d_vma */
/* List entry for the BO's position in
* v3d_exec_info->unref_list
--
2.20.1
[GIT PULL 2/2] bcm2835-drivers-next-2019-03-04
The following changes since commit e1dc2b2e1bef7237fd8fc055fe1ec2a6ff001f91: ARM: bcm283x: Switch V3D over to using the PM driver instead of firmware. (2019-02-01 10:34:32 +0100) are available in the Git repository at: git://github.com/anholt/linux tags/bcm2835-drivers-next-2019-03-04 for you to fetch changes up to 4deabfae643d8852c643664d9088a647abfaa5d0: soc: bcm: bcm2835-pm: Fix error paths of initialization. (2019-03-04 15:33:14 -0800) This pull request brings in two fixes for the new native bcm2835 power domains driver. Eric Anholt (2): soc: bcm: bcm2835-pm: Fix PM_IMAGE_PERI power domain support. soc: bcm: bcm2835-pm: Fix error paths of initialization. drivers/soc/bcm/bcm2835-power.c | 49 +++-- 1 file changed, 42 insertions(+), 7 deletions(-)
[GIT PULL 1/2] bcm2835-dt-next-2019-03-04
Hi Florian, The following changes since commit ab1b4ef966af90ad79fa3c4c124e47915cddde10: ARM: dts: bcm2835-rpi-zero-w: Drop unnecessary pinctrl (2019-02-01 11:56:32 +0100) are available in the Git repository at: git://github.com/anholt/linux tags/bcm2835-dt-next-2019-03-04 for you to fetch changes up to 544e784188f1dd7c797c70b213385e67d92005b6: ARM: dts: bcm283x: Fix hdmi hpd gpio pull (2019-03-04 15:50:10 -0800) This pull request brings in a fix for detecting HDMI on the Pi B rev 2. Helen Koike (1): ARM: dts: bcm283x: Fix hdmi hpd gpio pull arch/arm/boot/dts/bcm2835-rpi-b-rev2.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Re: [PATCH resend] thermal: bcm2835: Fix crash in bcm2835_thermal_debugfs
Phil Elwell writes:
> "cat /sys/kernel/debug/bcm2835_thermal/regset" causes a NULL pointer
> dereference in bcm2835_thermal_debugfs. The driver makes use of the
> implementation details of the thermal framework to retrieve a pointer
> to its private data from a struct thermal_zone_device, and gets it
> wrong - leading to the crash. Instead, store its private data as the
> drvdata and retrieve the thermal_zone_device pointer from it.
>
> Fixes: bcb7dd9ef206 ("thermal: bcm2835: add thermal driver for bcm2835 SoC")
>
> Signed-off-by: Phil Elwell
Acked-by: Eric Anholt
From the thread that missed Ccing the maintainers there was also:
Reviewed-by: Daniel Lezcano
signature.asc
Description: PGP signature
Re: [PATCH] ARM: dts: bcm283x: Fix hdmi hpd gpio pull
Stefan Wahren writes: > Hi, > > since this patch change only one board, the subject could be more specific: > > ARM: dts: bcm2835-rpi-b-rev2: Fix hdmi hpd gpio pull > >> Helen Koike hat am 4. März 2019 um 22:48 >> geschrieben: >> >> >> Raspberry pi board model B revison 2 have the hot plug detector gpio >> active high (and not low as it was in the dts). >> >> Signed-off-by: Helen Koike >> Reviewed-by: Eric Anholt > > I think we should add a Fixes tag here, so it could be backported. > > @Eric: Feel free to take V2 Added a fixes tag, and I'll submit PRs for this and the PM fixes now. signature.asc Description: PGP signature
Re: rpi-b-rev2 can't detect hdmi display due to wrong hpd gpio pull in dts
Helen Koike writes: > Hello, > > I have a rpi-b-rev2 (two holes, 512M) and when I was trying to test vc4 > driver, the HDMI wans't being detected. > > So I changed the hpd-gpios from ACTIVE_LOW to ACTIVE_HIGH and now it > works correctly. I can see that modeprint from libdrm detects correctly > when the display is plugged and when it is unpluggled. > > So I was wondering if this is just in my board or if there is an error > in the dts (which I think it's unlikely). > > In any case I am dropping this email here in case anyone else see this > problem, if you do, try appliying the patch below. We even have the GPIO labeled as "HDMI_HPD_P" above, and dt-blob.dts agrees. Reviewed-by: Eric Anholt Thanks! If Stefan doesn't pick this up shortly, I can. signature.asc Description: PGP signature
[PATCH v3 3/3] drm/v3d: Make sure the GPU is on when measuring clocks.
You'll get garbage measurements if the registers always read back
0xdeadbeef
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/v3d_debugfs.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c
b/drivers/gpu/drm/v3d/v3d_debugfs.c
index eb2b2d2f8553..a24af2d2f574 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -187,6 +187,11 @@ static int v3d_measure_clock(struct seq_file *m, void
*unused)
uint32_t cycles;
int core = 0;
int measure_ms = 1000;
+ int ret;
+
+ ret = pm_runtime_get_sync(v3d->dev);
+ if (ret < 0)
+ return ret;
if (v3d->ver >= 40) {
V3D_CORE_WRITE(core, V3D_V4_PCTR_0_SRC_0_3,
@@ -210,6 +215,9 @@ static int v3d_measure_clock(struct seq_file *m, void
*unused)
cycles / (measure_ms * 1000),
(cycles / (measure_ms * 100)) % 10);
+ pm_runtime_mark_last_busy(v3d->dev);
+ pm_runtime_put_autosuspend(v3d->dev);
+
return 0;
}
--
2.20.1
[PATCH v3 1/3] drm/v3d: Add support for V3D v4.2.
No compatible string for it yet, just the version-dependent changes.
They've now tied the hub and the core interrupt lines into a single
interrupt line coming out of the block. It also turns out I made a
mistake in modeling the V3D v3.3 and v4.1 bridge as a part of V3D
itself -- the bridge is going away in favor of an external reset
controller in a larger HW module.
v2: Use consistent checks for whether we're on 4.2, and fix a leak in
an error path.
v3: Use more general means of determining if the current 4.2 changes
are in place, as apparently other platforms may switch back (noted
by Dave). Update the binding doc.
Signed-off-by: Eric Anholt
---
.../devicetree/bindings/gpu/brcm,bcm-v3d.txt | 11 ++--
drivers/gpu/drm/v3d/v3d_drv.c | 21 +++---
drivers/gpu/drm/v3d/v3d_drv.h | 2 ++
drivers/gpu/drm/v3d/v3d_gem.c | 12 +++-
drivers/gpu/drm/v3d/v3d_irq.c | 28 +++
5 files changed, 60 insertions(+), 14 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.txt
b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.txt
index c907aa8dd755..b2df82b44625 100644
--- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.txt
+++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.txt
@@ -6,15 +6,20 @@ For V3D 2.x, see brcm,bcm-vc4.txt.
Required properties:
- compatible: Should be "brcm,7268-v3d" or "brcm,7278-v3d"
- reg: Physical base addresses and lengths of the register areas
-- reg-names: Names for the register areas. The "hub", "bridge", and "core0"
+- reg-names: Names for the register areas. The "hub" and "core0"
register areas are always required. The "gca" register area
- is required if the GCA cache controller is present.
+ is required if the GCA cache controller is present. The
+ "bridge" register area is required if an external reset
+ controller is not present.
- interrupts: The interrupt numbers. The first interrupt is for the hub,
- while the following interrupts are for the cores.
+ while the following interrupts are separate interrupt lines
+ for the cores (if they don't share the hub's interrupt).
See bindings/interrupt-controller/interrupts.txt
Optional properties:
- clocks: The core clock the unit runs on
+- resets: The reset line for v3d, if not using a mapping of the bridge
+ See bindings/reset/reset.txt
v3d {
compatible = "brcm,7268-v3d";
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index b189b1a0ae7e..392e458b55bd 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -496,10 +497,6 @@ static int v3d_platform_drm_probe(struct platform_device
*pdev)
v3d->pdev = pdev;
drm = >drm;
- ret = map_regs(v3d, >bridge_regs, "bridge");
- if (ret)
- goto dev_free;
-
ret = map_regs(v3d, >hub_regs, "hub");
if (ret)
goto dev_free;
@@ -514,6 +511,22 @@ static int v3d_platform_drm_probe(struct platform_device
*pdev)
v3d->cores = V3D_GET_FIELD(ident1, V3D_HUB_IDENT1_NCORES);
WARN_ON(v3d->cores > 1); /* multicore not yet implemented */
+ v3d->reset = devm_reset_control_get_exclusive(dev, NULL);
+ if (IS_ERR(v3d->reset)) {
+ ret = PTR_ERR(v3d->reset);
+
+ if (ret == -EPROBE_DEFER)
+ goto dev_free;
+
+ v3d->reset = NULL;
+ ret = map_regs(v3d, >bridge_regs, "bridge");
+ if (ret) {
+ dev_err(dev,
+ "Failed to get reset control or bridge regs\n");
+ goto dev_free;
+ }
+ }
+
if (v3d->ver < 41) {
ret = map_regs(v3d, >gca_regs, "gca");
if (ret)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 9e0e11f57307..fab9979b7e1f 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -39,6 +39,7 @@ struct v3d_dev {
* and revision.
*/
int ver;
+ bool single_irq_line;
struct device *dev;
struct platform_device *pdev;
@@ -47,6 +48,7 @@ struct v3d_dev {
void __iomem *bridge_regs;
void __iomem *gca_regs;
struct clk *clk;
+ struct reset_control *reset;
/* Virtual and DMA addresses of the single shared page table. */
volatile u32 *pt;
diff --git a/drivers/gpu/drm/v3d/v3d_ge
[PATCH v3 2/3] drm/v3d: Don't try to set OVRTMUOUT on V3D 4.x.
The old field is gone and the register now has a different field, QRMAXCNT for how many TMU requests get serviced before thread switch. We were accidentally reducing it from its default of 0x3 (4 requests) to 0x0 (1). v2: Skip setting the reg at all on 4.x, instead of trying to update only the old field. Signed-off-by: Eric Anholt --- drivers/gpu/drm/v3d/v3d_gem.c | 3 ++- drivers/gpu/drm/v3d/v3d_regs.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index 109be31e47ea..449d01ea54a0 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -25,7 +25,8 @@ v3d_init_core(struct v3d_dev *v3d, int core) * type. If you want the default behavior, you can still put * "2" in the indirect texture state's output_type field. */ - V3D_CORE_WRITE(core, V3D_CTL_MISCCFG, V3D_MISCCFG_OVRTMUOUT); + if (v3d->ver < 40) + V3D_CORE_WRITE(core, V3D_CTL_MISCCFG, V3D_MISCCFG_OVRTMUOUT); /* Whenever we flush the L2T cache, we always want to flush * the whole thing. diff --git a/drivers/gpu/drm/v3d/v3d_regs.h b/drivers/gpu/drm/v3d/v3d_regs.h index 6ccdee9d47bd..8e88af237610 100644 --- a/drivers/gpu/drm/v3d/v3d_regs.h +++ b/drivers/gpu/drm/v3d/v3d_regs.h @@ -216,6 +216,8 @@ # define V3D_IDENT2_BCG_INTBIT(28) #define V3D_CTL_MISCCFG0x00018 +# define V3D_CTL_MISCCFG_QRMAXCNT_MASK V3D_MASK(3, 1) +# define V3D_CTL_MISCCFG_QRMAXCNT_SHIFT1 # define V3D_MISCCFG_OVRTMUOUT BIT(0) #define V3D_CTL_L2CACTL0x00020 -- 2.20.1
[PATCH 3/7] drm/vc4: Use common helpers for debugfs setup by the driver components.
The global list of all debugfs entries for the driver was painful: the
list couldn't see into the components' structs, so each component had
its own debugs show function to find the component, then find the
regset and dump it. The components also had to be careful to check
that they were actually registered in vc4 before dereferencing
themselves, in case they weren't probed on a particular platform.
They routinely failed at that.
Instead, we can have the components add their debugfs callbacks to a
little list in vc4 to be registered at drm_dev_register() time, which
gets vc4_debugfs.c out of the business of knowing the whole list of
components.
Thanks to this change, dsi0 (if it existed) would register its node.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/vc4/vc4_bo.c | 6 +--
drivers/gpu/drm/vc4/vc4_crtc.c| 33 +++-
drivers/gpu/drm/vc4/vc4_debugfs.c | 84 ---
drivers/gpu/drm/vc4/vc4_dpi.c | 20 +---
drivers/gpu/drm/vc4/vc4_drv.c | 1 +
drivers/gpu/drm/vc4/vc4_drv.h | 38 ++
drivers/gpu/drm/vc4/vc4_dsi.c | 24 ++---
drivers/gpu/drm/vc4/vc4_hdmi.c| 6 +--
drivers/gpu/drm/vc4/vc4_hvs.c | 16 +-
drivers/gpu/drm/vc4/vc4_txp.c | 20 +---
drivers/gpu/drm/vc4/vc4_v3d.c | 19 ++-
drivers/gpu/drm/vc4/vc4_vec.c | 20 +---
12 files changed, 125 insertions(+), 162 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 92e3f98d8478..8c509d560bf0 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -66,8 +66,7 @@ static void vc4_bo_stats_dump(struct vc4_dev *vc4)
mutex_unlock(>purgeable.lock);
}
-#ifdef CONFIG_DEBUG_FS
-int vc4_bo_stats_debugfs(struct seq_file *m, void *unused)
+static int vc4_bo_stats_debugfs(struct seq_file *m, void *unused)
{
struct drm_info_node *node = (struct drm_info_node *)m->private;
struct drm_device *dev = node->minor->dev;
@@ -99,7 +98,6 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *unused)
return 0;
}
-#endif
/* Takes ownership of *name and returns the appropriate slot for it in
* the bo_labels[] array, extending it as necessary.
@@ -1025,6 +1023,8 @@ int vc4_bo_cache_init(struct drm_device *dev)
mutex_init(>bo_lock);
+ vc4_debugfs_add_file(dev, "bo_stats", vc4_bo_stats_debugfs, NULL);
+
INIT_LIST_HEAD(>bo_cache.time_list);
INIT_WORK(>bo_cache.time_work, vc4_bo_cache_time_work);
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 6d7072026056..9a62f12aca54 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -84,33 +84,6 @@ static const struct debugfs_reg32 crtc_regs[] = {
VC4_REG32(PV_HACT_ACT),
};
-#ifdef CONFIG_DEBUG_FS
-int vc4_crtc_debugfs_regs(struct seq_file *m, void *unused)
-{
- struct drm_info_node *node = (struct drm_info_node *)m->private;
- struct drm_device *dev = node->minor->dev;
- int crtc_index = (uintptr_t)node->info_ent->data;
- struct drm_printer p = drm_seq_file_printer(m);
- struct drm_crtc *crtc;
- struct vc4_crtc *vc4_crtc;
- int i;
-
- i = 0;
- list_for_each_entry(crtc, >mode_config.crtc_list, head) {
- if (i == crtc_index)
- break;
- i++;
- }
- if (!crtc)
- return 0;
- vc4_crtc = to_vc4_crtc(crtc);
-
- drm_print_regset32(, _crtc->regset);
-
- return 0;
-}
-#endif
-
bool vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
bool in_vblank_irq, int *vpos, int *hpos,
ktime_t *stime, ktime_t *etime,
@@ -1062,6 +1035,7 @@ static const struct drm_crtc_helper_funcs
vc4_crtc_helper_funcs = {
static const struct vc4_crtc_data pv0_data = {
.hvs_channel = 0,
+ .debugfs_name = "crtc0_regs",
.encoder_types = {
[PV_CONTROL_CLK_SELECT_DSI] = VC4_ENCODER_TYPE_DSI0,
[PV_CONTROL_CLK_SELECT_DPI_SMI_HDMI] = VC4_ENCODER_TYPE_DPI,
@@ -1070,6 +1044,7 @@ static const struct vc4_crtc_data pv0_data = {
static const struct vc4_crtc_data pv1_data = {
.hvs_channel = 2,
+ .debugfs_name = "crtc1_regs",
.encoder_types = {
[PV_CONTROL_CLK_SELECT_DSI] = VC4_ENCODER_TYPE_DSI1,
[PV_CONTROL_CLK_SELECT_DPI_SMI_HDMI] = VC4_ENCODER_TYPE_SMI,
@@ -1078,6 +1053,7 @@ static const struct vc4_crtc_data pv1_data = {
static const struct vc4_crtc_data pv2_data = {
.hvs_channel = 1,
+ .debugfs_name = "crtc2_regs",
.encoder_types = {
[PV_CONTROL_CLK_SELECT_DPI_SMI_HDMI] = VC4_ENCODER_TYPE_HDMI,
[PV_CONTROL_CLK_SELECT_VEC] = VC4_ENCODER_TYPE_VEC,
@@ -1239,6 +1215,9 @@ static int vc4_crtc_bind(stru
[PATCH 6/7] drm/vc4: Add helpers for pm get/put.
This makes sure the vc4_reset doesn't hit an obscure race with the
GET_PARAM ioctl, fixes a decrement outside of the lock, and prevents
future code from making mistakes with the weird return value of
pm_runtime_get_sync().
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/vc4/vc4_drv.c | 21 +
drivers/gpu/drm/vc4/vc4_drv.h | 2 ++
drivers/gpu/drm/vc4/vc4_gem.c | 21 +
drivers/gpu/drm/vc4/vc4_v3d.c | 33 +
4 files changed, 49 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index de1d0ce11831..f95891a61d52 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -77,28 +77,25 @@ static int vc4_get_param_ioctl(struct drm_device *dev, void
*data,
switch (args->param) {
case DRM_VC4_PARAM_V3D_IDENT0:
- ret = pm_runtime_get_sync(>v3d->pdev->dev);
- if (ret < 0)
+ ret = vc4_v3d_pm_get(vc4);
+ if (ret)
return ret;
args->value = V3D_READ(V3D_IDENT0);
- pm_runtime_mark_last_busy(>v3d->pdev->dev);
- pm_runtime_put_autosuspend(>v3d->pdev->dev);
+ vc4_v3d_pm_put(vc4);
break;
case DRM_VC4_PARAM_V3D_IDENT1:
- ret = pm_runtime_get_sync(>v3d->pdev->dev);
- if (ret < 0)
+ ret = vc4_v3d_pm_get(vc4);
+ if (ret)
return ret;
args->value = V3D_READ(V3D_IDENT1);
- pm_runtime_mark_last_busy(>v3d->pdev->dev);
- pm_runtime_put_autosuspend(>v3d->pdev->dev);
+ vc4_v3d_pm_put(vc4);
break;
case DRM_VC4_PARAM_V3D_IDENT2:
- ret = pm_runtime_get_sync(>v3d->pdev->dev);
- if (ret < 0)
+ ret = vc4_v3d_pm_get(vc4);
+ if (ret)
return ret;
args->value = V3D_READ(V3D_IDENT2);
- pm_runtime_mark_last_busy(>v3d->pdev->dev);
- pm_runtime_put_autosuspend(>v3d->pdev->dev);
+ vc4_v3d_pm_put(vc4);
break;
case DRM_VC4_PARAM_SUPPORTS_BRANCHES:
case DRM_VC4_PARAM_SUPPORTS_ETC1:
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 495c5a13a948..c71988b270bc 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -809,6 +809,8 @@ void vc4_plane_async_set_fb(struct drm_plane *plane,
/* vc4_v3d.c */
extern struct platform_driver vc4_v3d_driver;
int vc4_v3d_get_bin_slot(struct vc4_dev *vc4);
+int vc4_v3d_pm_get(struct vc4_dev *vc4);
+void vc4_v3d_pm_put(struct vc4_dev *vc4);
/* vc4_validate.c */
int
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 8d816e5ed762..4544a478f106 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -969,12 +969,7 @@ vc4_complete_exec(struct drm_device *dev, struct
vc4_exec_info *exec)
/* Release the reference we had on the perf monitor. */
vc4_perfmon_put(exec->perfmon);
- mutex_lock(>power_lock);
- if (--vc4->power_refcount == 0) {
- pm_runtime_mark_last_busy(>v3d->pdev->dev);
- pm_runtime_put_autosuspend(>v3d->pdev->dev);
- }
- mutex_unlock(>power_lock);
+ vc4_v3d_pm_put(vc4);
kfree(exec);
}
@@ -1153,17 +1148,11 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
return -ENOMEM;
}
- mutex_lock(>power_lock);
- if (vc4->power_refcount++ == 0) {
- ret = pm_runtime_get_sync(>v3d->pdev->dev);
- if (ret < 0) {
- mutex_unlock(>power_lock);
- vc4->power_refcount--;
- kfree(exec);
- return ret;
- }
+ ret = vc4_v3d_pm_get(vc4);
+ if (ret) {
+ kfree(exec);
+ return ret;
}
- mutex_unlock(>power_lock);
exec->args = args;
INIT_LIST_HEAD(>unref_list);
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index f008584eb739..7820b8eaaa98 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -124,6 +124,39 @@ static int vc4_v3d_debugfs_ident(struct seq_file *m, void
*unused)
return 0;
}
+/**
+ * Wraps pm_runtime_get_sync() in a refcount, so that we can reliably
+ * get the pm_runtime refcount to 0 in vc4_reset().
+ */
+int
+vc4_v3d_pm_get(struct vc4_dev *vc4)
+{
+ mutex_lock(>power_lock);
+ if (vc4->power_refcount++ == 0) {
+ int ret = pm_runtime_get_sync(>v3d->
[PATCH 7/7] drm/vc4: Make sure that the v3d ident debugfs has vc4's power on.
Otherwise, you sometimes decode the ident fields based on 0xdeadbeef
register reads.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/vc4/vc4_v3d.c | 29 +
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index 7820b8eaaa98..36e6c7086ecf 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -108,18 +108,23 @@ static int vc4_v3d_debugfs_ident(struct seq_file *m, void
*unused)
struct drm_info_node *node = (struct drm_info_node *)m->private;
struct drm_device *dev = node->minor->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
- uint32_t ident1 = V3D_READ(V3D_IDENT1);
- uint32_t nslc = VC4_GET_FIELD(ident1, V3D_IDENT1_NSLC);
- uint32_t tups = VC4_GET_FIELD(ident1, V3D_IDENT1_TUPS);
- uint32_t qups = VC4_GET_FIELD(ident1, V3D_IDENT1_QUPS);
-
- seq_printf(m, "Revision: %d\n",
- VC4_GET_FIELD(ident1, V3D_IDENT1_REV));
- seq_printf(m, "Slices: %d\n", nslc);
- seq_printf(m, "TMUs: %d\n", nslc * tups);
- seq_printf(m, "QPUs: %d\n", nslc * qups);
- seq_printf(m, "Semaphores: %d\n",
- VC4_GET_FIELD(ident1, V3D_IDENT1_NSEM));
+ int ret = vc4_v3d_pm_get(vc4);
+
+ if (ret == 0) {
+ uint32_t ident1 = V3D_READ(V3D_IDENT1);
+ uint32_t nslc = VC4_GET_FIELD(ident1, V3D_IDENT1_NSLC);
+ uint32_t tups = VC4_GET_FIELD(ident1, V3D_IDENT1_TUPS);
+ uint32_t qups = VC4_GET_FIELD(ident1, V3D_IDENT1_QUPS);
+
+ seq_printf(m, "Revision: %d\n",
+ VC4_GET_FIELD(ident1, V3D_IDENT1_REV));
+ seq_printf(m, "Slices: %d\n", nslc);
+ seq_printf(m, "TMUs: %d\n", nslc * tups);
+ seq_printf(m, "QPUs: %d\n", nslc * qups);
+ seq_printf(m, "Semaphores: %d\n",
+ VC4_GET_FIELD(ident1, V3D_IDENT1_NSEM));
+ vc4_v3d_pm_put(vc4);
+ }
return 0;
}
--
2.20.1
[PATCH 5/7] drm/vc4: Disable V3D interactions if the v3d component didn't probe.
One might want to use the VC4 display stack without using Mesa.
Similar to the debugfs fixes for not having all of the possible
display bits enabled, make sure you can't oops in vc4 if v3d isn't
enabled.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/vc4/vc4_drv.c | 11 +++
drivers/gpu/drm/vc4/vc4_gem.c | 10 ++
drivers/gpu/drm/vc4/vc4_irq.c | 9 +
drivers/gpu/drm/vc4/vc4_perfmon.c | 18 ++
4 files changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 1927d1b756f5..de1d0ce11831 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -72,6 +72,9 @@ static int vc4_get_param_ioctl(struct drm_device *dev, void
*data,
if (args->pad != 0)
return -EINVAL;
+ if (!vc4->v3d)
+ return -EINVAL;
+
switch (args->param) {
case DRM_VC4_PARAM_V3D_IDENT0:
ret = pm_runtime_get_sync(>v3d->pdev->dev);
@@ -251,6 +254,7 @@ static int vc4_drm_bind(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct drm_device *drm;
struct vc4_dev *vc4;
+ struct device_node *node;
int ret = 0;
dev->coherent_dma_mask = DMA_BIT_MASK(32);
@@ -259,6 +263,13 @@ static int vc4_drm_bind(struct device *dev)
if (!vc4)
return -ENOMEM;
+ /* If VC4 V3D is missing, don't advertise render nodes. */
+ node = of_find_compatible_node(NULL, NULL, "brcm,bcm2835-v3d");
+ if (node)
+ of_node_put(node);
+ else
+ vc4_drm_driver.driver_features &= ~DRIVER_RENDER;
+
drm = drm_dev_alloc(_drm_driver, dev);
if (IS_ERR(drm))
return PTR_ERR(drm);
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 5ee5bf7fedf7..8d816e5ed762 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -74,6 +74,11 @@ vc4_get_hang_state_ioctl(struct drm_device *dev, void *data,
u32 i;
int ret = 0;
+ if (!vc4->v3d) {
+ DRM_DEBUG("VC4_GET_HANG_STATE with no VC4 V3D probed\n");
+ return -EINVAL;
+ }
+
spin_lock_irqsave(>job_lock, irqflags);
kernel_state = vc4->hang_state;
if (!kernel_state) {
@@ -1124,6 +1129,11 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
struct dma_fence *in_fence;
int ret = 0;
+ if (!vc4->v3d) {
+ DRM_DEBUG("VC4_SUBMIT_CL with no VC4 V3D probed\n");
+ return -EINVAL;
+ }
+
if ((args->flags & ~(VC4_SUBMIT_CL_USE_CLEAR_COLOR |
VC4_SUBMIT_CL_FIXED_RCL_ORDER |
VC4_SUBMIT_CL_RCL_ORDER_INCREASING_X |
diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c
index 4cd2ccfe15f4..ffd0a4388752 100644
--- a/drivers/gpu/drm/vc4/vc4_irq.c
+++ b/drivers/gpu/drm/vc4/vc4_irq.c
@@ -229,6 +229,9 @@ vc4_irq_preinstall(struct drm_device *dev)
{
struct vc4_dev *vc4 = to_vc4_dev(dev);
+ if (!vc4->v3d)
+ return;
+
init_waitqueue_head(>job_wait_queue);
INIT_WORK(>overflow_mem_work, vc4_overflow_mem_work);
@@ -243,6 +246,9 @@ vc4_irq_postinstall(struct drm_device *dev)
{
struct vc4_dev *vc4 = to_vc4_dev(dev);
+ if (!vc4->v3d)
+ return 0;
+
/* Enable both the render done and out of memory interrupts. */
V3D_WRITE(V3D_INTENA, V3D_DRIVER_IRQS);
@@ -254,6 +260,9 @@ vc4_irq_uninstall(struct drm_device *dev)
{
struct vc4_dev *vc4 = to_vc4_dev(dev);
+ if (!vc4->v3d)
+ return;
+
/* Disable sending interrupts for our driver's IRQs. */
V3D_WRITE(V3D_INTDIS, V3D_DRIVER_IRQS);
diff --git a/drivers/gpu/drm/vc4/vc4_perfmon.c
b/drivers/gpu/drm/vc4/vc4_perfmon.c
index 495150415020..6562d3d30b20 100644
--- a/drivers/gpu/drm/vc4/vc4_perfmon.c
+++ b/drivers/gpu/drm/vc4/vc4_perfmon.c
@@ -100,12 +100,18 @@ void vc4_perfmon_close_file(struct vc4_file *vc4file)
int vc4_perfmon_create_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
+ struct vc4_dev *vc4 = to_vc4_dev(dev);
struct vc4_file *vc4file = file_priv->driver_priv;
struct drm_vc4_perfmon_create *req = data;
struct vc4_perfmon *perfmon;
unsigned int i;
int ret;
+ if (!vc4->v3d) {
+ DRM_DEBUG("Creating perfmon no VC4 V3D probed\n");
+ return -EINVAL;
+ }
+
/* Number of monitored counters cannot exceed HW limits. */
if (req->ncounters > DRM_VC4_MAX_PERF_COUNTERS ||
!req->ncounters)
@@ -146,10 +152,16 @@ int vc4_perfmon_create_ioctl(struct drm_device *dev, void
*data
[PATCH 2/7] drm/vc4: Use drm_print_regset32() for our debug register dumping.
This removes a bunch of duplicated boilerplate for the debugfs vs
runtime printk debug dumping.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/vc4/vc4_crtc.c | 68 ++---
drivers/gpu/drm/vc4/vc4_dpi.c | 23 ++---
drivers/gpu/drm/vc4/vc4_drv.h | 7 ++
drivers/gpu/drm/vc4/vc4_dsi.c | 155 +
drivers/gpu/drm/vc4/vc4_hdmi.c | 156 +
drivers/gpu/drm/vc4/vc4_hvs.c | 87 -
drivers/gpu/drm/vc4/vc4_txp.c | 28 +++---
drivers/gpu/drm/vc4/vc4_v3d.c | 173 -
drivers/gpu/drm/vc4/vc4_vec.c | 67 ++---
9 files changed, 350 insertions(+), 414 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 730008d3da76..6d7072026056 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -67,43 +68,29 @@ to_vc4_crtc_state(struct drm_crtc_state *crtc_state)
#define CRTC_WRITE(offset, val) writel(val, vc4_crtc->regs + (offset))
#define CRTC_READ(offset) readl(vc4_crtc->regs + (offset))
-#define CRTC_REG(reg) { reg, #reg }
-static const struct {
- u32 reg;
- const char *name;
-} crtc_regs[] = {
- CRTC_REG(PV_CONTROL),
- CRTC_REG(PV_V_CONTROL),
- CRTC_REG(PV_VSYNCD_EVEN),
- CRTC_REG(PV_HORZA),
- CRTC_REG(PV_HORZB),
- CRTC_REG(PV_VERTA),
- CRTC_REG(PV_VERTB),
- CRTC_REG(PV_VERTA_EVEN),
- CRTC_REG(PV_VERTB_EVEN),
- CRTC_REG(PV_INTEN),
- CRTC_REG(PV_INTSTAT),
- CRTC_REG(PV_STAT),
- CRTC_REG(PV_HACT_ACT),
+static const struct debugfs_reg32 crtc_regs[] = {
+ VC4_REG32(PV_CONTROL),
+ VC4_REG32(PV_V_CONTROL),
+ VC4_REG32(PV_VSYNCD_EVEN),
+ VC4_REG32(PV_HORZA),
+ VC4_REG32(PV_HORZB),
+ VC4_REG32(PV_VERTA),
+ VC4_REG32(PV_VERTB),
+ VC4_REG32(PV_VERTA_EVEN),
+ VC4_REG32(PV_VERTB_EVEN),
+ VC4_REG32(PV_INTEN),
+ VC4_REG32(PV_INTSTAT),
+ VC4_REG32(PV_STAT),
+ VC4_REG32(PV_HACT_ACT),
};
-static void vc4_crtc_dump_regs(struct vc4_crtc *vc4_crtc)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(crtc_regs); i++) {
- DRM_INFO("0x%04x (%s): 0x%08x\n",
-crtc_regs[i].reg, crtc_regs[i].name,
-CRTC_READ(crtc_regs[i].reg));
- }
-}
-
#ifdef CONFIG_DEBUG_FS
int vc4_crtc_debugfs_regs(struct seq_file *m, void *unused)
{
struct drm_info_node *node = (struct drm_info_node *)m->private;
struct drm_device *dev = node->minor->dev;
int crtc_index = (uintptr_t)node->info_ent->data;
+ struct drm_printer p = drm_seq_file_printer(m);
struct drm_crtc *crtc;
struct vc4_crtc *vc4_crtc;
int i;
@@ -118,11 +105,7 @@ int vc4_crtc_debugfs_regs(struct seq_file *m, void *unused)
return 0;
vc4_crtc = to_vc4_crtc(crtc);
- for (i = 0; i < ARRAY_SIZE(crtc_regs); i++) {
- seq_printf(m, "%s (0x%04x): 0x%08x\n",
- crtc_regs[i].name, crtc_regs[i].reg,
- CRTC_READ(crtc_regs[i].reg));
- }
+ drm_print_regset32(, _crtc->regset);
return 0;
}
@@ -434,8 +417,10 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
bool debug_dump_regs = false;
if (debug_dump_regs) {
- DRM_INFO("CRTC %d regs before:\n", drm_crtc_index(crtc));
- vc4_crtc_dump_regs(vc4_crtc);
+ struct drm_printer p = drm_info_printer(_crtc->pdev->dev);
+ dev_info(_crtc->pdev->dev, "CRTC %d regs before:\n",
+drm_crtc_index(crtc));
+ drm_print_regset32(, _crtc->regset);
}
if (vc4_crtc->channel == 2) {
@@ -476,8 +461,10 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
vc4_crtc_lut_load(crtc);
if (debug_dump_regs) {
- DRM_INFO("CRTC %d regs after:\n", drm_crtc_index(crtc));
- vc4_crtc_dump_regs(vc4_crtc);
+ struct drm_printer p = drm_info_printer(_crtc->pdev->dev);
+ dev_info(_crtc->pdev->dev, "CRTC %d regs after:\n",
+drm_crtc_index(crtc));
+ drm_print_regset32(, _crtc->regset);
}
}
@@ -1169,11 +1156,16 @@ static int vc4_crtc_bind(struct device *dev, struct
device *master, void *data)
if (!match)
return -ENODEV;
vc4_crtc->data = match->data;
+ vc4_crtc->pdev = pdev;
vc4_crtc->regs = vc4_ioremap_regs(pdev, 0);
if (IS_ERR(vc4_crtc->regs))
return PTR_ERR(vc4_crtc->regs);
+ vc4_crtc->regset.base = vc4_crtc->regs;
+ vc4_crtc->r
[PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32.
The debugfs_regset32 is nice to use for reducing boilerplate in
dumping a bunch of regs in debugfs, but we also want to be able to
print to dmesg them at runtime for driver debugging. drm_printer lets
us format debugfs and the printk the same way.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/drm_print.c | 16
include/drm/drm_print.h | 2 ++
2 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 0e7fc3e7dfb4..5ecc0f04cd0c 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -253,3 +253,19 @@ void drm_err(const char *format, ...)
va_end(args);
}
EXPORT_SYMBOL(drm_err);
+
+void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset)
+{
+ int namelen = 0;
+ int i;
+
+ for (i = 0; i < regset->nregs; i++)
+ namelen = max(namelen, (int)strlen(regset->regs[i].name));
+
+ for (i = 0; i < regset->nregs; i++) {
+ drm_printf(p, "%*s = 0x%08x\n",
+ namelen, regset->regs[i].name,
+ readl(regset->base + regset->regs[i].offset));
+ }
+}
+EXPORT_SYMBOL(drm_print_regset32);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index afbc3beef089..3a4247319e63 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
/**
* DOC: print
@@ -84,6 +85,7 @@ void __drm_printfn_debug(struct drm_printer *p, struct
va_format *vaf);
__printf(2, 3)
void drm_printf(struct drm_printer *p, const char *f, ...);
void drm_puts(struct drm_printer *p, const char *str);
+void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32
*regset);
__printf(2, 0)
/**
--
2.20.1
[PATCH 4/7] drm/vc4: Use drm_printer for the debugfs and runtime bo stats output.
Now I can extend the stats without more copy and pasting between the
two.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/vc4/vc4_bo.c | 48 +++-
1 file changed, 14 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 8c509d560bf0..88ebd681d7eb 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -40,7 +40,7 @@ static bool is_user_label(int label)
return label >= VC4_BO_TYPE_COUNT;
}
-static void vc4_bo_stats_dump(struct vc4_dev *vc4)
+static void vc4_bo_stats_print(struct drm_printer *p, struct vc4_dev *vc4)
{
int i;
@@ -48,21 +48,21 @@ static void vc4_bo_stats_dump(struct vc4_dev *vc4)
if (!vc4->bo_labels[i].num_allocated)
continue;
- DRM_INFO("%30s: %6dkb BOs (%d)\n",
-vc4->bo_labels[i].name,
-vc4->bo_labels[i].size_allocated / 1024,
-vc4->bo_labels[i].num_allocated);
+ drm_printf(p, "%30s: %6dkb BOs (%d)\n",
+ vc4->bo_labels[i].name,
+ vc4->bo_labels[i].size_allocated / 1024,
+ vc4->bo_labels[i].num_allocated);
}
mutex_lock(>purgeable.lock);
if (vc4->purgeable.num)
- DRM_INFO("%30s: %6zdkb BOs (%d)\n", "userspace BO cache",
-vc4->purgeable.size / 1024, vc4->purgeable.num);
+ drm_printf(p, "%30s: %6zdkb BOs (%d)\n", "userspace BO cache",
+ vc4->purgeable.size / 1024, vc4->purgeable.num);
if (vc4->purgeable.purged_num)
- DRM_INFO("%30s: %6zdkb BOs (%d)\n", "total purged BO",
-vc4->purgeable.purged_size / 1024,
-vc4->purgeable.purged_num);
+ drm_printf(p, "%30s: %6zdkb BOs (%d)\n", "total purged BO",
+ vc4->purgeable.purged_size / 1024,
+ vc4->purgeable.purged_num);
mutex_unlock(>purgeable.lock);
}
@@ -71,30 +71,9 @@ static int vc4_bo_stats_debugfs(struct seq_file *m, void
*unused)
struct drm_info_node *node = (struct drm_info_node *)m->private;
struct drm_device *dev = node->minor->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
- int i;
-
- mutex_lock(>bo_lock);
- for (i = 0; i < vc4->num_labels; i++) {
- if (!vc4->bo_labels[i].num_allocated)
- continue;
-
- seq_printf(m, "%30s: %6dkb BOs (%d)\n",
- vc4->bo_labels[i].name,
- vc4->bo_labels[i].size_allocated / 1024,
- vc4->bo_labels[i].num_allocated);
- }
- mutex_unlock(>bo_lock);
+ struct drm_printer p = drm_seq_file_printer(m);
- mutex_lock(>purgeable.lock);
- if (vc4->purgeable.num)
- seq_printf(m, "%30s: %6zdkb BOs (%d)\n", "userspace BO cache",
- vc4->purgeable.size / 1024, vc4->purgeable.num);
-
- if (vc4->purgeable.purged_num)
- seq_printf(m, "%30s: %6zdkb BOs (%d)\n", "total purged BO",
- vc4->purgeable.purged_size / 1024,
- vc4->purgeable.purged_num);
- mutex_unlock(>purgeable.lock);
+ vc4_bo_stats_print(, vc4);
return 0;
}
@@ -473,8 +452,9 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t
unaligned_size,
}
if (IS_ERR(cma_obj)) {
+ struct drm_printer p = drm_info_printer(vc4->dev->dev);
DRM_ERROR("Failed to allocate from CMA:\n");
- vc4_bo_stats_dump(vc4);
+ vc4_bo_stats_print(, vc4);
return ERR_PTR(-ENOMEM);
}
bo = to_vc4_bo(_obj->base);
--
2.20.1
[PATCH v2 1/2] soc: bcm: bcm2835-pm: Fix PM_IMAGE_PERI power domain support.
We don't have ASB master/slave regs for this domain, so just skip that
step.
Signed-off-by: Eric Anholt
Fixes: 670c672608a1 ("soc: bcm: bcm2835-pm: Add support for power domains under
a new binding.")
---
drivers/soc/bcm/bcm2835-power.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/bcm/bcm2835-power.c b/drivers/soc/bcm/bcm2835-power.c
index 48412957ec7a..4a1b99b773c0 100644
--- a/drivers/soc/bcm/bcm2835-power.c
+++ b/drivers/soc/bcm/bcm2835-power.c
@@ -150,7 +150,12 @@ struct bcm2835_power {
static int bcm2835_asb_enable(struct bcm2835_power *power, u32 reg)
{
- u64 start = ktime_get_ns();
+ u64 start;
+
+ if (!reg)
+ return 0;
+
+ start = ktime_get_ns();
/* Enable the module's async AXI bridges. */
ASB_WRITE(reg, ASB_READ(reg) & ~ASB_REQ_STOP);
@@ -165,7 +170,12 @@ static int bcm2835_asb_enable(struct bcm2835_power *power,
u32 reg)
static int bcm2835_asb_disable(struct bcm2835_power *power, u32 reg)
{
- u64 start = ktime_get_ns();
+ u64 start;
+
+ if (!reg)
+ return 0;
+
+ start = ktime_get_ns();
/* Enable the module's async AXI bridges. */
ASB_WRITE(reg, ASB_READ(reg) | ASB_REQ_STOP);
--
2.20.1
Re: [PATCH 2/2] soc: bcm: bcm2835-pm: Fix error paths of initialization.
Florian Fainelli writes:
> On 2/13/19 2:33 PM, Stefan Wahren wrote:
>>
>>> Eric Anholt hat am 13. Februar 2019 um 19:28 geschrieben:
>>>
>>>
>>> Stefan Wahren writes:
>>>
>>>> Hi Eric,
>>>>
>>>> Am 13.02.19 um 01:33 schrieb Eric Anholt:
>>>>> The clock driver may probe after ours and so we need to pass the
>>>>> -EPROBE_DEFER out. Fix the other error path while we're here.
>>>>>
>>>>> Signed-off-by: Eric Anholt
>>>>> Fixes: 670c672608a1 ("soc: bcm: bcm2835-pm: Add support for power domains
>>>>> under a new binding.")
>>>>> ---
>>>>> drivers/soc/bcm/bcm2835-power.c | 30 +-
>>>>> 1 file changed, 25 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/soc/bcm/bcm2835-power.c
>>>>> b/drivers/soc/bcm/bcm2835-power.c
>>>>> index 4a1b99b773c0..11f9469423f7 100644
>>>>> --- a/drivers/soc/bcm/bcm2835-power.c
>>>>> +++ b/drivers/soc/bcm/bcm2835-power.c
>>>>> @@ -485,7 +485,7 @@ static int bcm2835_power_pd_power_off(struct
>>>>> generic_pm_domain *domain)
>>>>> }
>>>>> }
>>>>>
>>>>> -static void
>>>>> +static int
>>>>> bcm2835_init_power_domain(struct bcm2835_power *power,
>>>>> int pd_xlate_index, const char *name)
>>>>> {
>>>>> @@ -493,6 +493,12 @@ bcm2835_init_power_domain(struct bcm2835_power
>>>>> *power,
>>>>> struct bcm2835_power_domain *dom = >domains[pd_xlate_index];
>>>>>
>>>>> dom->clk = devm_clk_get(dev->parent, name);
>>>>> + if (IS_ERR(dom->clk)) {
>>>>> + int ret = PTR_ERR(dom->clk);
>>>>> +
>>>>> + if (ret == -EPROBE_DEFER)
>>>>> + return ret;
>>>> is it safe to proceed in the other error cases?
>>>> Even it would be more consistent with clk_prepare_enable() to print an
>>>> error here.
>>>
>>> Yes, not all domains have a clk, so we want to ignore the other error.
>>
>> But shouldn't we set dom->clk to NULL instead of keeping the error
>> pointer? AFAIK clk_prepare_enable is aware of NULL instead of error
>> pointer.
>
> If the clock is really optional, then yes, this should be the way to go.
Sigh, error pointers. Fixed, sending a v2.
signature.asc
Description: PGP signature
[PATCH v2 2/2] soc: bcm: bcm2835-pm: Fix error paths of initialization.
The clock driver may probe after ours and so we need to pass the
-EPROBE_DEFER out. Fix the other error path while we're here.
v2: Use dom->name instead of dom->gov as the flag for initialized
domains, since we aren't setting up a governor. Make sure to
clear ->clk when no clk is present in the DT.
Signed-off-by: Eric Anholt
Fixes: 670c672608a1 ("soc: bcm: bcm2835-pm: Add support for power domains under
a new binding.")
---
drivers/soc/bcm/bcm2835-power.c | 35 -
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/soc/bcm/bcm2835-power.c b/drivers/soc/bcm/bcm2835-power.c
index 4a1b99b773c0..241c4ed80899 100644
--- a/drivers/soc/bcm/bcm2835-power.c
+++ b/drivers/soc/bcm/bcm2835-power.c
@@ -485,7 +485,7 @@ static int bcm2835_power_pd_power_off(struct
generic_pm_domain *domain)
}
}
-static void
+static int
bcm2835_init_power_domain(struct bcm2835_power *power,
int pd_xlate_index, const char *name)
{
@@ -493,6 +493,17 @@ bcm2835_init_power_domain(struct bcm2835_power *power,
struct bcm2835_power_domain *dom = >domains[pd_xlate_index];
dom->clk = devm_clk_get(dev->parent, name);
+ if (IS_ERR(dom->clk)) {
+ int ret = PTR_ERR(dom->clk);
+
+ if (ret == -EPROBE_DEFER)
+ return ret;
+
+ /* Some domains don't have a clk, so make sure that we
+* don't deref an error pointer later.
+*/
+ dom->clk = NULL;
+ }
dom->base.name = name;
dom->base.power_on = bcm2835_power_pd_power_on;
@@ -505,6 +516,8 @@ bcm2835_init_power_domain(struct bcm2835_power *power,
pm_genpd_init(>base, NULL, true);
power->pd_xlate.domains[pd_xlate_index] = >base;
+
+ return 0;
}
/** bcm2835_reset_reset - Resets a block that has a reset line in the
@@ -602,7 +615,7 @@ static int bcm2835_power_probe(struct platform_device *pdev)
{ BCM2835_POWER_DOMAIN_IMAGE_PERI, BCM2835_POWER_DOMAIN_CAM0 },
{ BCM2835_POWER_DOMAIN_IMAGE_PERI, BCM2835_POWER_DOMAIN_CAM1 },
};
- int ret, i;
+ int ret = 0, i;
u32 id;
power = devm_kzalloc(dev, sizeof(*power), GFP_KERNEL);
@@ -629,8 +642,11 @@ static int bcm2835_power_probe(struct platform_device
*pdev)
power->pd_xlate.num_domains = ARRAY_SIZE(power_domain_names);
- for (i = 0; i < ARRAY_SIZE(power_domain_names); i++)
- bcm2835_init_power_domain(power, i, power_domain_names[i]);
+ for (i = 0; i < ARRAY_SIZE(power_domain_names); i++) {
+ ret = bcm2835_init_power_domain(power, i,
power_domain_names[i]);
+ if (ret)
+ goto fail;
+ }
for (i = 0; i < ARRAY_SIZE(domain_deps); i++) {
pm_genpd_add_subdomain(>domains[domain_deps[i].parent].base,
@@ -644,12 +660,21 @@ static int bcm2835_power_probe(struct platform_device
*pdev)
ret = devm_reset_controller_register(dev, >reset);
if (ret)
- return ret;
+ goto fail;
of_genpd_add_provider_onecell(dev->parent->of_node, >pd_xlate);
dev_info(dev, "Broadcom BCM2835 power domains driver");
return 0;
+
+fail:
+ for (i = 0; i < ARRAY_SIZE(power_domain_names); i++) {
+ struct generic_pm_domain *dom = >domains[i].base;
+
+ if (dom->name)
+ pm_genpd_remove(dom);
+ }
+ return ret;
}
static int bcm2835_power_remove(struct platform_device *pdev)
--
2.20.1
Re: [PATCH v5 0/3] drm/vc4: Add a load tracker
Paul Kocialkowski writes: > Hi, > > Here is a fourth iteration of the VC4 load tracking series, which was > initially developed by Boris Brezillon and that I have now taken over. > > This new iteration takes in account comments from v3 and comes with a > new approach for avoiding underrun reports when reconfiguring the > pipeline. It is now based on detection instead of delaying the underrun > interrupt unmasking. > > It can be tested with a dedicated IGT GPU Tools series: > VC4 load tracker testing Series is: Reviewed-by: Eric Anholt Thanks for persisting on this! signature.asc Description: PGP signature
Re: [PATCH -next] drm: Remove set but not used variable 'gem'
YueHaibing writes: > Fixes gcc '-Wunused-but-set-variable' warning: > > drivers/gpu/drm/vc4/vc4_txp.c: In function 'vc4_txp_connector_atomic_check': > drivers/gpu/drm/vc4/vc4_txp.c:252:29: warning: > variable 'gem' set but not used [-Wunused-but-set-variable] > struct drm_gem_cma_object *gem; > ^ > > Signed-off-by: YueHaibing Pushed to drm-misc-next. Thanks! signature.asc Description: PGP signature
Re: [PATCH 2/2] soc: bcm: bcm2835-pm: Fix error paths of initialization.
Stefan Wahren writes:
> Hi Eric,
>
> Am 13.02.19 um 01:33 schrieb Eric Anholt:
>> The clock driver may probe after ours and so we need to pass the
>> -EPROBE_DEFER out. Fix the other error path while we're here.
>>
>> Signed-off-by: Eric Anholt
>> Fixes: 670c672608a1 ("soc: bcm: bcm2835-pm: Add support for power domains
>> under a new binding.")
>> ---
>> drivers/soc/bcm/bcm2835-power.c | 30 +-
>> 1 file changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/soc/bcm/bcm2835-power.c
>> b/drivers/soc/bcm/bcm2835-power.c
>> index 4a1b99b773c0..11f9469423f7 100644
>> --- a/drivers/soc/bcm/bcm2835-power.c
>> +++ b/drivers/soc/bcm/bcm2835-power.c
>> @@ -485,7 +485,7 @@ static int bcm2835_power_pd_power_off(struct
>> generic_pm_domain *domain)
>> }
>> }
>>
>> -static void
>> +static int
>> bcm2835_init_power_domain(struct bcm2835_power *power,
>>int pd_xlate_index, const char *name)
>> {
>> @@ -493,6 +493,12 @@ bcm2835_init_power_domain(struct bcm2835_power *power,
>> struct bcm2835_power_domain *dom = >domains[pd_xlate_index];
>>
>> dom->clk = devm_clk_get(dev->parent, name);
>> +if (IS_ERR(dom->clk)) {
>> +int ret = PTR_ERR(dom->clk);
>> +
>> +if (ret == -EPROBE_DEFER)
>> +return ret;
> is it safe to proceed in the other error cases?
> Even it would be more consistent with clk_prepare_enable() to print an
> error here.
Yes, not all domains have a clk, so we want to ignore the other error.
And we shouldn't print for defers, generally.
signature.asc
Description: PGP signature
[PATCH 1/2] soc: bcm: bcm2835-pm: Fix PM_IMAGE_PERI power domain support.
We don't have ASB master/slave regs for this domain, so just skip that
step.
Signed-off-by: Eric Anholt
Fixes: 670c672608a1 ("soc: bcm: bcm2835-pm: Add support for power domains under
a new binding.")
---
drivers/soc/bcm/bcm2835-power.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/bcm/bcm2835-power.c b/drivers/soc/bcm/bcm2835-power.c
index 48412957ec7a..4a1b99b773c0 100644
--- a/drivers/soc/bcm/bcm2835-power.c
+++ b/drivers/soc/bcm/bcm2835-power.c
@@ -150,7 +150,12 @@ struct bcm2835_power {
static int bcm2835_asb_enable(struct bcm2835_power *power, u32 reg)
{
- u64 start = ktime_get_ns();
+ u64 start;
+
+ if (!reg)
+ return 0;
+
+ start = ktime_get_ns();
/* Enable the module's async AXI bridges. */
ASB_WRITE(reg, ASB_READ(reg) & ~ASB_REQ_STOP);
@@ -165,7 +170,12 @@ static int bcm2835_asb_enable(struct bcm2835_power *power,
u32 reg)
static int bcm2835_asb_disable(struct bcm2835_power *power, u32 reg)
{
- u64 start = ktime_get_ns();
+ u64 start;
+
+ if (!reg)
+ return 0;
+
+ start = ktime_get_ns();
/* Enable the module's async AXI bridges. */
ASB_WRITE(reg, ASB_READ(reg) | ASB_REQ_STOP);
--
2.20.1
[PATCH 2/2] soc: bcm: bcm2835-pm: Fix error paths of initialization.
The clock driver may probe after ours and so we need to pass the
-EPROBE_DEFER out. Fix the other error path while we're here.
Signed-off-by: Eric Anholt
Fixes: 670c672608a1 ("soc: bcm: bcm2835-pm: Add support for power domains under
a new binding.")
---
drivers/soc/bcm/bcm2835-power.c | 30 +-
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/soc/bcm/bcm2835-power.c b/drivers/soc/bcm/bcm2835-power.c
index 4a1b99b773c0..11f9469423f7 100644
--- a/drivers/soc/bcm/bcm2835-power.c
+++ b/drivers/soc/bcm/bcm2835-power.c
@@ -485,7 +485,7 @@ static int bcm2835_power_pd_power_off(struct
generic_pm_domain *domain)
}
}
-static void
+static int
bcm2835_init_power_domain(struct bcm2835_power *power,
int pd_xlate_index, const char *name)
{
@@ -493,6 +493,12 @@ bcm2835_init_power_domain(struct bcm2835_power *power,
struct bcm2835_power_domain *dom = >domains[pd_xlate_index];
dom->clk = devm_clk_get(dev->parent, name);
+ if (IS_ERR(dom->clk)) {
+ int ret = PTR_ERR(dom->clk);
+
+ if (ret == -EPROBE_DEFER)
+ return ret;
+ }
dom->base.name = name;
dom->base.power_on = bcm2835_power_pd_power_on;
@@ -505,6 +511,8 @@ bcm2835_init_power_domain(struct bcm2835_power *power,
pm_genpd_init(>base, NULL, true);
power->pd_xlate.domains[pd_xlate_index] = >base;
+
+ return 0;
}
/** bcm2835_reset_reset - Resets a block that has a reset line in the
@@ -602,7 +610,7 @@ static int bcm2835_power_probe(struct platform_device *pdev)
{ BCM2835_POWER_DOMAIN_IMAGE_PERI, BCM2835_POWER_DOMAIN_CAM0 },
{ BCM2835_POWER_DOMAIN_IMAGE_PERI, BCM2835_POWER_DOMAIN_CAM1 },
};
- int ret, i;
+ int ret = 0, i;
u32 id;
power = devm_kzalloc(dev, sizeof(*power), GFP_KERNEL);
@@ -629,8 +637,11 @@ static int bcm2835_power_probe(struct platform_device
*pdev)
power->pd_xlate.num_domains = ARRAY_SIZE(power_domain_names);
- for (i = 0; i < ARRAY_SIZE(power_domain_names); i++)
- bcm2835_init_power_domain(power, i, power_domain_names[i]);
+ for (i = 0; i < ARRAY_SIZE(power_domain_names); i++) {
+ ret = bcm2835_init_power_domain(power, i,
power_domain_names[i]);
+ if (ret)
+ goto fail;
+ }
for (i = 0; i < ARRAY_SIZE(domain_deps); i++) {
pm_genpd_add_subdomain(>domains[domain_deps[i].parent].base,
@@ -644,12 +655,21 @@ static int bcm2835_power_probe(struct platform_device
*pdev)
ret = devm_reset_controller_register(dev, >reset);
if (ret)
- return ret;
+ goto fail;
of_genpd_add_provider_onecell(dev->parent->of_node, >pd_xlate);
dev_info(dev, "Broadcom BCM2835 power domains driver");
return 0;
+
+fail:
+ for (i = 0; i < ARRAY_SIZE(power_domain_names); i++) {
+ struct generic_pm_domain *dom = >domains[i].base;
+
+ if (dom->gov)
+ pm_genpd_remove(dom);
+ }
+ return ret;
}
static int bcm2835_power_remove(struct platform_device *pdev)
--
2.20.1
[PATCH v2 1/3] drm/v3d: Add support for V3D v4.2.
No compatible string for it yet, just the version-dependent changes.
They've now tied the hub and the core interrupt lines into a single
interrupt line coming out of the block. It also turns out I made a
mistake in modeling the V3D v3.3 and v4.1 bridge as a part of V3D
itself -- the bridge is going away in favor of an external reset
controller in a larger HW module.
v2: Use consistent checks for whether we're on 4.2, and fix a leak in
an error path.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/v3d_drv.c | 19 +++
drivers/gpu/drm/v3d/v3d_drv.h | 1 +
drivers/gpu/drm/v3d/v3d_gem.c | 12 +++-
drivers/gpu/drm/v3d/v3d_irq.c | 27 +--
4 files changed, 48 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index dd842af8b531..aef46de55b6a 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -497,10 +498,6 @@ static int v3d_platform_drm_probe(struct platform_device
*pdev)
v3d->pdev = pdev;
drm = >drm;
- ret = map_regs(v3d, >bridge_regs, "bridge");
- if (ret)
- goto dev_free;
-
ret = map_regs(v3d, >hub_regs, "hub");
if (ret)
goto dev_free;
@@ -515,6 +512,20 @@ static int v3d_platform_drm_probe(struct platform_device
*pdev)
v3d->cores = V3D_GET_FIELD(ident1, V3D_HUB_IDENT1_NCORES);
WARN_ON(v3d->cores > 1); /* multicore not yet implemented */
+ if (v3d->ver >= 42) {
+ v3d->reset = devm_reset_control_get_exclusive(dev, NULL);
+ if (IS_ERR(v3d->reset)) {
+ ret = PTR_ERR(v3d->reset);
+ dev_err(dev, "Failed to get reset control: %ld\n",
+ ret);
+ goto dev_free;
+ }
+ } else {
+ ret = map_regs(v3d, >bridge_regs, "bridge");
+ if (ret)
+ goto dev_free;
+ }
+
if (v3d->ver < 41) {
ret = map_regs(v3d, >gca_regs, "gca");
if (ret)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 239b56d76f3e..b5732cf897fe 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -48,6 +48,7 @@ struct v3d_dev {
void __iomem *bridge_regs;
void __iomem *gca_regs;
struct clk *clk;
+ struct reset_control *reset;
/* Virtual and DMA addresses of the single shared page table. */
volatile u32 *pt;
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 803f31467ec1..3ee3ae4d3cac 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -69,7 +70,7 @@ v3d_idle_gca(struct v3d_dev *v3d)
}
static void
-v3d_reset_v3d(struct v3d_dev *v3d)
+v3d_reset_by_bridge(struct v3d_dev *v3d)
{
int version = V3D_BRIDGE_READ(V3D_TOP_GR_BRIDGE_REVISION);
@@ -89,6 +90,15 @@ v3d_reset_v3d(struct v3d_dev *v3d)
V3D_TOP_GR_BRIDGE_SW_INIT_1_V3D_CLK_108_SW_INIT);
V3D_BRIDGE_WRITE(V3D_TOP_GR_BRIDGE_SW_INIT_1, 0);
}
+}
+
+static void
+v3d_reset_v3d(struct v3d_dev *v3d)
+{
+ if (v3d->reset)
+ reset_control_reset(v3d->reset);
+ else
+ v3d_reset_by_bridge(v3d);
v3d_init_hw_state(v3d);
}
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index 69338da70ddc..2ad0c5865edd 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -27,6 +27,9 @@
V3D_HUB_INT_MMU_CAP | \
V3D_HUB_INT_TFUC))
+static irqreturn_t
+v3d_hub_irq(int irq, void *arg);
+
static void
v3d_overflow_mem_work(struct work_struct *work)
{
@@ -112,6 +115,12 @@ v3d_irq(int irq, void *arg)
if (intsts & V3D_INT_GMPV)
dev_err(v3d->dev, "GMP violation\n");
+ /* V3D 4.2 wires the hub and core IRQs together, so if we &
+* didn't see the common one then check hub for MMU IRQs.
+*/
+ if (v3d->ver >= 42 && status == IRQ_NONE)
+ return v3d_hub_irq(irq, arg);
+
return status;
}
@@ -170,12 +179,18 @@ v3d_irq_init(struct v3d_dev *v3d)
V3D_CORE_WRITE(core, V3D_CTL_INT_CLR, V3D_CORE_IRQS);
V3D_WRITE(V3D_HUB_INT_CLR, V3D_HUB_IRQS);
- ret = devm_request_irq(v3d->dev, platform_get_irq(v3d->pdev, 0),
- v3d_hub_irq, IRQF_SHARED,
- "v3d_hub", v3d);
- ret = devm
[PATCH v2 3/3] drm/v3d: Make sure the GPU is on when measuring clocks.
You'll get garbage measurements if the registers always read back
0xdeadbeef
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/v3d_debugfs.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c
b/drivers/gpu/drm/v3d/v3d_debugfs.c
index eb2b2d2f8553..a24af2d2f574 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -187,6 +187,11 @@ static int v3d_measure_clock(struct seq_file *m, void
*unused)
uint32_t cycles;
int core = 0;
int measure_ms = 1000;
+ int ret;
+
+ ret = pm_runtime_get_sync(v3d->dev);
+ if (ret < 0)
+ return ret;
if (v3d->ver >= 40) {
V3D_CORE_WRITE(core, V3D_V4_PCTR_0_SRC_0_3,
@@ -210,6 +215,9 @@ static int v3d_measure_clock(struct seq_file *m, void
*unused)
cycles / (measure_ms * 1000),
(cycles / (measure_ms * 100)) % 10);
+ pm_runtime_mark_last_busy(v3d->dev);
+ pm_runtime_put_autosuspend(v3d->dev);
+
return 0;
}
--
2.20.1
[PATCH v2 2/3] drm/v3d: Don't try to set OVRTMUOUT on V3D 4.x.
The old field is gone and the register now has a different field, QRMAXCNT for how many TMU requests get serviced before thread switch. We were accidentally reducing it from its default of 0x3 (4 requests) to 0x0 (1). v2: Skip setting the reg at all on 4.x, instead of trying to update only the old field. Signed-off-by: Eric Anholt --- drivers/gpu/drm/v3d/v3d_gem.c | 3 ++- drivers/gpu/drm/v3d/v3d_regs.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index 3ee3ae4d3cac..06538ffa3014 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -25,7 +25,8 @@ v3d_init_core(struct v3d_dev *v3d, int core) * type. If you want the default behavior, you can still put * "2" in the indirect texture state's output_type field. */ - V3D_CORE_WRITE(core, V3D_CTL_MISCCFG, V3D_MISCCFG_OVRTMUOUT); + if (v3d->ver < 40) + V3D_CORE_WRITE(core, V3D_CTL_MISCCFG, V3D_MISCCFG_OVRTMUOUT); /* Whenever we flush the L2T cache, we always want to flush * the whole thing. diff --git a/drivers/gpu/drm/v3d/v3d_regs.h b/drivers/gpu/drm/v3d/v3d_regs.h index 6ccdee9d47bd..8e88af237610 100644 --- a/drivers/gpu/drm/v3d/v3d_regs.h +++ b/drivers/gpu/drm/v3d/v3d_regs.h @@ -216,6 +216,8 @@ # define V3D_IDENT2_BCG_INTBIT(28) #define V3D_CTL_MISCCFG0x00018 +# define V3D_CTL_MISCCFG_QRMAXCNT_MASK V3D_MASK(3, 1) +# define V3D_CTL_MISCCFG_QRMAXCNT_SHIFT1 # define V3D_MISCCFG_OVRTMUOUT BIT(0) #define V3D_CTL_L2CACTL0x00020 -- 2.20.1
Re: [PATCH] drm/v3d: Fix BO stats accounting for dma-buf-imported buffers.
Daniel Vetter writes: > On Thu, Feb 07, 2019 at 03:26:13PM -0800, Eric Anholt wrote: >> We always decrement at GEM free, so make sure we increment at GEM >> creation for dma-bufs. > > Indeed. Reviewed-by: Daniel Vetter Merged to drm-misc-next. Thanks! signature.asc Description: PGP signature
Re: [PATCH 1/4] drm/v3d: Update top-level kerneldoc for the addition of TFU.
Thomas Spurden writes: > On Thu, 7 Feb 2019 at 20:10, Eric Anholt wrote: >> >> Signed-off-by: Eric Anholt > > Reviewed-by: Thomas Spurden Merged this one, will send out respins of the others. signature.asc Description: PGP signature
[PATCH] drm/v3d: Fix BO stats accounting for dma-buf-imported buffers.
We always decrement at GEM free, so make sure we increment at GEM
creation for dma-bufs.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/v3d_bo.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index a08766d39eab..b1766f096c4b 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -282,6 +282,7 @@ v3d_prime_import_sg_table(struct drm_device *dev,
struct dma_buf_attachment *attach,
struct sg_table *sgt)
{
+ struct v3d_dev *v3d = to_v3d_dev(dev);
struct drm_gem_object *obj;
struct v3d_bo *bo;
@@ -296,6 +297,11 @@ v3d_prime_import_sg_table(struct drm_device *dev,
obj->import_attach = attach;
v3d_bo_get_pages(bo);
+ mutex_lock(>bo_lock);
+ v3d->bo_stats.num_allocated++;
+ v3d->bo_stats.pages_allocated += obj->size >> PAGE_SHIFT;
+ mutex_unlock(>bo_lock);
+
v3d_mmu_insert_ptes(bo);
return obj;
--
2.20.0.rc1
Re: [PATCH] drm/sched: Always trace the dependencies we wait on, to fix a race.
"Koenig, Christian" writes: > Am 07.12.18 um 20:16 schrieb Eric Anholt: >> The entity->dependency can go away completely once we've called >> drm_sched_entity_add_dependency_cb() (if the cb is called before we >> get around to tracing). The tracepoint is more useful if we trace >> every dependency instead of just ones that get callbacks installed, >> anyway, so just do that. >> >> Fixes any easy-to-produce OOPS when tracing the scheduler on V3D with >> "perf record -a -e gpu_scheduler:.\* glxgears" and DEBUG_SLAB enabled. >> >> Signed-off-by: Eric Anholt > > Reviewed-by: Christian König > > Going to pick that up for upstream and will add with a CC: stable. Looks like this got misplaced. signature.asc Description: PGP signature
Re: [PATCH] drm/vc4: Use struct_size() in kzalloc()
"Gustavo A. R. Silva" writes:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> void *entry[];
> };
>
> instance = kzalloc(sizeof(struct foo) + count * sizeof(struct boo),
> GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva
Reviewed and pushed to drm-misc-next. Thanks!
signature.asc
Description: PGP signature
[PATCH 2/4] drm/v3d: Add support for V3D v4.2.
No compatible string for it yet, just the version-dependent changes.
They've now tied the hub and the core interrupt lines into a single
interrupt line coming out of the block. It also turns out I made a
mistake in modeling the V3D v3.3 and v4.1 bridge as a part of V3D
itself -- the bridge is going away in favor of an external reset
controller in a larger HW module.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/v3d_drv.c | 18 ++
drivers/gpu/drm/v3d/v3d_drv.h | 1 +
drivers/gpu/drm/v3d/v3d_gem.c | 12 +++-
drivers/gpu/drm/v3d/v3d_irq.c | 27 +--
4 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 1f22ce542a04..8b2f156bdd30 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -496,10 +497,6 @@ static int v3d_platform_drm_probe(struct platform_device
*pdev)
v3d->pdev = pdev;
drm = >drm;
- ret = map_regs(v3d, >bridge_regs, "bridge");
- if (ret)
- goto dev_free;
-
ret = map_regs(v3d, >hub_regs, "hub");
if (ret)
goto dev_free;
@@ -514,6 +511,19 @@ static int v3d_platform_drm_probe(struct platform_device
*pdev)
v3d->cores = V3D_GET_FIELD(ident1, V3D_HUB_IDENT1_NCORES);
WARN_ON(v3d->cores > 1); /* multicore not yet implemented */
+ if (v3d->ver < 42) {
+ ret = map_regs(v3d, >bridge_regs, "bridge");
+ if (ret)
+ goto dev_free;
+ } else {
+ v3d->reset = devm_reset_control_get_exclusive(dev, NULL);
+ if (IS_ERR(v3d->reset)) {
+ dev_err(dev, "Failed to get reset control: %ld\n",
+ PTR_ERR(v3d->reset));
+ return PTR_ERR(v3d->reset);
+ }
+ }
+
if (v3d->ver < 41) {
ret = map_regs(v3d, >gca_regs, "gca");
if (ret)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 239b56d76f3e..b5732cf897fe 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -48,6 +48,7 @@ struct v3d_dev {
void __iomem *bridge_regs;
void __iomem *gca_regs;
struct clk *clk;
+ struct reset_control *reset;
/* Virtual and DMA addresses of the single shared page table. */
volatile u32 *pt;
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 803f31467ec1..3ee3ae4d3cac 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -69,7 +70,7 @@ v3d_idle_gca(struct v3d_dev *v3d)
}
static void
-v3d_reset_v3d(struct v3d_dev *v3d)
+v3d_reset_by_bridge(struct v3d_dev *v3d)
{
int version = V3D_BRIDGE_READ(V3D_TOP_GR_BRIDGE_REVISION);
@@ -89,6 +90,15 @@ v3d_reset_v3d(struct v3d_dev *v3d)
V3D_TOP_GR_BRIDGE_SW_INIT_1_V3D_CLK_108_SW_INIT);
V3D_BRIDGE_WRITE(V3D_TOP_GR_BRIDGE_SW_INIT_1, 0);
}
+}
+
+static void
+v3d_reset_v3d(struct v3d_dev *v3d)
+{
+ if (v3d->reset)
+ reset_control_reset(v3d->reset);
+ else
+ v3d_reset_by_bridge(v3d);
v3d_init_hw_state(v3d);
}
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index 69338da70ddc..98dd77acf512 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -27,6 +27,9 @@
V3D_HUB_INT_MMU_CAP | \
V3D_HUB_INT_TFUC))
+static irqreturn_t
+v3d_hub_irq(int irq, void *arg);
+
static void
v3d_overflow_mem_work(struct work_struct *work)
{
@@ -112,6 +115,12 @@ v3d_irq(int irq, void *arg)
if (intsts & V3D_INT_GMPV)
dev_err(v3d->dev, "GMP violation\n");
+ /* V3D 4.2 wires the hub and core IRQs together, so if we &
+* didn't see the common one then check hub for MMU IRQs.
+*/
+ if (v3d->ver >= 42 && status == IRQ_NONE)
+ return v3d_hub_irq(irq, arg);
+
return status;
}
@@ -170,12 +179,18 @@ v3d_irq_init(struct v3d_dev *v3d)
V3D_CORE_WRITE(core, V3D_CTL_INT_CLR, V3D_CORE_IRQS);
V3D_WRITE(V3D_HUB_INT_CLR, V3D_HUB_IRQS);
- ret = devm_request_irq(v3d->dev, platform_get_irq(v3d->pdev, 0),
- v3d_hub_irq, IRQF_SHARED,
- "v3d_hub", v3d);
- ret = devm_request_irq(v3d->dev, platform_get_irq(v3d->pdev, 1),
- v3d_irq, IRQF_SH
[PATCH 1/4] drm/v3d: Update top-level kerneldoc for the addition of TFU.
Signed-off-by: Eric Anholt --- drivers/gpu/drm/v3d/v3d_drv.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c index 22fff0d3aecd..1f22ce542a04 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.c +++ b/drivers/gpu/drm/v3d/v3d_drv.c @@ -7,9 +7,9 @@ * This driver supports the Broadcom V3D 3.3 and 4.1 OpenGL ES GPUs. * For V3D 2.x support, see the VC4 driver. * - * Currently only single-core rendering using the binner and renderer - * is supported. The TFU (texture formatting unit) and V3D 4.x's CSD - * (compute shader dispatch) are not yet supported. + * Currently only single-core rendering using the binner and renderer, + * along with TFU (texture formatting unit) rendering is supported. + * V3D 4.x's CSD (compute shader dispatch) is not yet supported. */ #include -- 2.20.0.rc1
