Re: [PATCH v2] drm/msm/dpu: fix encoder irq wait skip
On Thu, 9 May 2024 at 22:40, Barnabás Czémán wrote:
>
> The irq_idx is unsigned so it cannot be lower than zero, better
> to change the condition to check if it is equal with zero.
> It could not cause any issue because a valid irq index starts from one.
>
> Fixes: 5a9d50150c2c ("drm/msm/dpu: shift IRQ indices by 1")
> Signed-off-by: Barnabás Czémán
In the previous revision you have got the Reviewed-by tag, which you
didn't include here for some reason.
This revision is:
Reviewed-by: Dmitry Baryshkov
Note, there is no need to send a next iteration just for these tags.
They usually get picked up by the patch management software (including
the Fixes tag).
> ---
> Changes in v2:
> - Add Fixes in commit message.
> - Link to v1:
> https://lore.kernel.org/r/20240509-irq_wait-v1-1-41d653e37...@gmail.com
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 119f3ea50a7c..cf7d769ab3b9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -428,7 +428,7 @@ int dpu_encoder_helper_wait_for_irq(struct
> dpu_encoder_phys *phys_enc,
> return -EWOULDBLOCK;
> }
>
> - if (irq_idx < 0) {
> + if (irq_idx == 0) {
> DRM_DEBUG_KMS("skip irq wait id=%u, callback=%ps\n",
> DRMID(phys_enc->parent), func);
> return 0;
>
> ---
> base-commit: 704ba27ac55579704ba1289392448b0c66b56258
> change-id: 20240509-irq_wait-49444cea77e2
>
> Best regards,
> --
> Barnabás Czémán
>
--
With best wishes
Dmitry
Re: [PATCH v2] drm/msm/dpu: fix encoder irq wait skip
On 5/9/2024 12:40 PM, Barnabás Czémán wrote:
The irq_idx is unsigned so it cannot be lower than zero, better
to change the condition to check if it is equal with zero.
It could not cause any issue because a valid irq index starts from one.
Fixes: 5a9d50150c2c ("drm/msm/dpu: shift IRQ indices by 1")
Signed-off-by: Barnabás Czémán
---
Changes in v2:
- Add Fixes in commit message.
- Link to v1:
https://lore.kernel.org/r/20240509-irq_wait-v1-1-41d653e37...@gmail.com
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
no functional change, so you could have retained by R-b, but here it is
again,
Reviewed-by: Abhinav Kumar
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 119f3ea50a7c..cf7d769ab3b9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -428,7 +428,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys
*phys_enc,
return -EWOULDBLOCK;
}
- if (irq_idx < 0) {
+ if (irq_idx == 0) {
DRM_DEBUG_KMS("skip irq wait id=%u, callback=%ps\n",
DRMID(phys_enc->parent), func);
return 0;
---
base-commit: 704ba27ac55579704ba1289392448b0c66b56258
change-id: 20240509-irq_wait-49444cea77e2
Best regards,
Re: [PATCH] drm/msm/dpu: fix encoder irq wait skip
On 5/9/2024 10:39 AM, Barnabás Czémán wrote:
The irq_idx is unsigned so it cannot be lower than zero, better
to change the condition to check if it is equal with zero.
It could not cause any issue because a valid irq index starts from one.
Signed-off-by: Barnabás Czémán
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
I think we also need
Fixes: 5a9d50150c2c ("drm/msm/dpu: shift IRQ indices by 1")
With that,
Reviewed-by: Abhinav Kumar
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 119f3ea50a7c..cf7d769ab3b9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -428,7 +428,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys
*phys_enc,
return -EWOULDBLOCK;
}
- if (irq_idx < 0) {
+ if (irq_idx == 0) {
DRM_DEBUG_KMS("skip irq wait id=%u, callback=%ps\n",
DRMID(phys_enc->parent), func);
return 0;
---
base-commit: 704ba27ac55579704ba1289392448b0c66b56258
change-id: 20240509-irq_wait-49444cea77e2
Best regards,
Re: [PATCH] drm/msm/dpu: guard ctl irq callback register/unregister
On 5/9/2024 10:52 AM, Barnabás Czémán wrote: CTLs on older qualcomm SOCs like msm8953 and msm8996 has not got interrupts, so better to skip CTL irq callback register/unregister make dpu_ctl_cfg be able to define without intr_start. Thanks for the patch. Have msm8953 and msm8996 migrated to DPU or is there a series planned to migrate them? The change itself is correct but without the catalogs for those chipsets merged, we will never hit this path. Signed-off-by: Barnabás Czémán --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 489be1c0c704..250d83af53a4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -298,7 +298,7 @@ static void dpu_encoder_phys_cmd_irq_enable(struct dpu_encoder_phys *phys_enc) phys_enc); dpu_encoder_phys_cmd_control_vblank_irq(phys_enc, true); - if (dpu_encoder_phys_cmd_is_master(phys_enc)) + if (dpu_encoder_phys_cmd_is_master(phys_enc) && phys_enc->irq[INTR_IDX_CTL_START]) dpu_core_irq_register_callback(phys_enc->dpu_kms, phys_enc->irq[INTR_IDX_CTL_START], dpu_encoder_phys_cmd_ctl_start_irq, @@ -311,7 +311,7 @@ static void dpu_encoder_phys_cmd_irq_disable(struct dpu_encoder_phys *phys_enc) phys_enc->hw_pp->idx - PINGPONG_0, phys_enc->vblank_refcount); - if (dpu_encoder_phys_cmd_is_master(phys_enc)) + if (dpu_encoder_phys_cmd_is_master(phys_enc) && phys_enc->irq[INTR_IDX_CTL_START]) dpu_core_irq_unregister_callback(phys_enc->dpu_kms, phys_enc->irq[INTR_IDX_CTL_START]); --- base-commit: 704ba27ac55579704ba1289392448b0c66b56258 change-id: 20240509-ctl_irq-a90b2d7a0bf5 Best regards,
Re: [PATCH] docs: document python version used for compilation
On 5/9/2024 9:48 AM, Jonathan Corbet wrote: Dmitry Baryshkov writes: The drm/msm driver had adopted using Python3 script to generate register header files instead of shipping pre-generated header files. Document the minimal Python version supported by the script. Signed-off-by: Dmitry Baryshkov --- Documentation/process/changes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst index 5685d7bfe4d0..8d225a9f65a2 100644 --- a/Documentation/process/changes.rst +++ b/Documentation/process/changes.rst @@ -63,6 +63,7 @@ cpio any cpio --version GNU tar1.28 tar --version gtags (optional) 6.6.5gtags --version mkimage (optional) 2017.01 mkimage --version +Python (optional) 3.5.xpython3 --version == === Is it really optional - can you build the driver without it? True, we cannot build the driver now without it. So we should be dropping the optional tag. With that addressed, Reviewed-by: Abhinav Kumar This document needs some help... I'm missing a number of things that are *not* marked as "optional" (jfsutils, reiserfsprogs, pcmciautils, ppp, ...) and somehow my system works fine :) It would be nice to document *why* users might need a specific tool. But I guess we aren't going to do that now. I can apply this, but I do wonder about the "optional" marking. Thanks, jon
[PATCH v2] drm/msm/adreno: De-spaghettify the use of memory barriers
return readl(gpu->mmio + (reg << 2)); + gpu_write(gpu, reg, data); + gpu_read(gpu, reg); } static inline void gpu_rmw(struct msm_gpu *gpu, u32 reg, u32 mask, u32 or) --- base-commit: ec2d9beb604a623a9f5308f7abcff3561e08c155 change-id: 20240509-topic-adreno-a8939b92f625 Best regards, -- Konrad Dybcio
Re: [PATCH] docs: document python version used for compilation
Dmitry Baryshkov writes: > The drm/msm driver had adopted using Python3 script to generate register > header files instead of shipping pre-generated header files. Document > the minimal Python version supported by the script. > > Signed-off-by: Dmitry Baryshkov > --- > Documentation/process/changes.rst | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/process/changes.rst > b/Documentation/process/changes.rst > index 5685d7bfe4d0..8d225a9f65a2 100644 > --- a/Documentation/process/changes.rst > +++ b/Documentation/process/changes.rst > @@ -63,6 +63,7 @@ cpio any cpio --version > GNU tar1.28 tar --version > gtags (optional) 6.6.5gtags --version > mkimage (optional) 2017.01 mkimage --version > +Python (optional) 3.5.xpython3 --version > == === > Is it really optional - can you build the driver without it? This document needs some help... I'm missing a number of things that are *not* marked as "optional" (jfsutils, reiserfsprogs, pcmciautils, ppp, ...) and somehow my system works fine :) It would be nice to document *why* users might need a specific tool. But I guess we aren't going to do that now. I can apply this, but I do wonder about the "optional" marking. Thanks, jon
[PATCH 1/2] drm/msm: Use a7xx family directly in gpu_state
With a7xx, we need to import a new header for each new generation and
switch to a different list of registers, instead of making
backwards-compatible changes. Using the helpers inadvertently made a750
use the a740 list of registers, instead use the family directly to fix
this.
Fixes: f3f8207d8aed ("drm/msm: Add devcoredump support for a750")
Signed-off-by: Connor Abbott
---
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 41 ++---
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
index 77146d30bcaa..c641ee7dec78 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
@@ -390,18 +390,18 @@ static void a7xx_get_debugbus_blocks(struct msm_gpu *gpu,
const u32 *debugbus_blocks, *gbif_debugbus_blocks;
int i;
- if (adreno_is_a730(adreno_gpu)) {
+ if (adreno_gpu->info->family == ADRENO_7XX_GEN1) {
debugbus_blocks = gen7_0_0_debugbus_blocks;
debugbus_blocks_count = ARRAY_SIZE(gen7_0_0_debugbus_blocks);
gbif_debugbus_blocks = a7xx_gbif_debugbus_blocks;
gbif_debugbus_blocks_count =
ARRAY_SIZE(a7xx_gbif_debugbus_blocks);
- } else if (adreno_is_a740_family(adreno_gpu)) {
+ } else if (adreno_gpu->info->family == ADRENO_7XX_GEN2) {
debugbus_blocks = gen7_2_0_debugbus_blocks;
debugbus_blocks_count = ARRAY_SIZE(gen7_2_0_debugbus_blocks);
gbif_debugbus_blocks = a7xx_gbif_debugbus_blocks;
gbif_debugbus_blocks_count =
ARRAY_SIZE(a7xx_gbif_debugbus_blocks);
} else {
- BUG_ON(!adreno_is_a750(adreno_gpu));
+ BUG_ON(adreno_gpu->info->family != ADRENO_7XX_GEN3);
debugbus_blocks = gen7_9_0_debugbus_blocks;
debugbus_blocks_count = ARRAY_SIZE(gen7_9_0_debugbus_blocks);
gbif_debugbus_blocks = gen7_9_0_gbif_debugbus_blocks;
@@ -511,7 +511,7 @@ static void a6xx_get_debugbus(struct msm_gpu *gpu,
const struct a6xx_debugbus_block *cx_debugbus_blocks;
if (adreno_is_a7xx(adreno_gpu)) {
- BUG_ON(!(adreno_is_a730(adreno_gpu) ||
adreno_is_a740_family(adreno_gpu)));
+ BUG_ON(adreno_gpu->info->family > ADRENO_7XX_GEN3);
cx_debugbus_blocks = a7xx_cx_debugbus_blocks;
nr_cx_debugbus_blocks =
ARRAY_SIZE(a7xx_cx_debugbus_blocks);
} else {
@@ -662,11 +662,11 @@ static void a7xx_get_dbgahb_clusters(struct msm_gpu *gpu,
const struct gen7_sptp_cluster_registers *dbgahb_clusters;
unsigned dbgahb_clusters_size;
- if (adreno_is_a730(adreno_gpu)) {
+ if (adreno_gpu->info->family == ADRENO_7XX_GEN1) {
dbgahb_clusters = gen7_0_0_sptp_clusters;
dbgahb_clusters_size = ARRAY_SIZE(gen7_0_0_sptp_clusters);
} else {
- BUG_ON(!adreno_is_a740_family(adreno_gpu));
+ BUG_ON(adreno_gpu->info->family > ADRENO_7XX_GEN3);
dbgahb_clusters = gen7_2_0_sptp_clusters;
dbgahb_clusters_size = ARRAY_SIZE(gen7_2_0_sptp_clusters);
}
@@ -820,14 +820,14 @@ static void a7xx_get_clusters(struct msm_gpu *gpu,
const struct gen7_cluster_registers *clusters;
unsigned clusters_size;
- if (adreno_is_a730(adreno_gpu)) {
+ if (adreno_gpu->info->family == ADRENO_7XX_GEN1) {
clusters = gen7_0_0_clusters;
clusters_size = ARRAY_SIZE(gen7_0_0_clusters);
- } else if (adreno_is_a740_family(adreno_gpu)) {
+ } else if (adreno_gpu->info->family == ADRENO_7XX_GEN2) {
clusters = gen7_2_0_clusters;
clusters_size = ARRAY_SIZE(gen7_2_0_clusters);
} else {
- BUG_ON(!adreno_is_a750(adreno_gpu));
+ BUG_ON(adreno_gpu->info->family != ADRENO_7XX_GEN3);
clusters = gen7_9_0_clusters;
clusters_size = ARRAY_SIZE(gen7_9_0_clusters);
}
@@ -895,7 +895,7 @@ static void a7xx_get_shader_block(struct msm_gpu *gpu,
if (WARN_ON(datasize > A6XX_CD_DATA_SIZE))
return;
- if (adreno_is_a730(adreno_gpu)) {
+ if (adreno_gpu->info->family == ADRENO_7XX_GEN1) {
gpu_rmw(gpu, REG_A7XX_SP_DBG_CNTL, GENMASK(1, 0), 3);
}
@@ -925,7 +925,7 @@ static void a7xx_get_shader_block(struct msm_gpu *gpu,
datasize);
out:
- if (adreno_is_a730(adreno_gpu)) {
+ if (adreno_gpu->info->family == ADRENO_7XX_GEN1) {
gpu_rmw(gpu, REG_A7XX_SP_DBG_CNTL, GENMASK(1, 0), 0);
}
}
@@ -958,14 +958,14 @@ static void a7xx_get_shaders(struct msm_gpu *gpu,
unsigned num_shader_blocks;
int i;
- if (adreno_is_a730(adreno_gpu)) {
+ if
[PATCH 2/2] drm/msm: Dump correct dbgahb clusters on a750
This was missed thanks to the family mixup fixed in the previous commit.
Fixes: f3f8207d8aed ("drm/msm: Add devcoredump support for a750")
Signed-off-by: Connor Abbott
---
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
index c641ee7dec78..69f3b942ce9d 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
@@ -665,10 +665,13 @@ static void a7xx_get_dbgahb_clusters(struct msm_gpu *gpu,
if (adreno_gpu->info->family == ADRENO_7XX_GEN1) {
dbgahb_clusters = gen7_0_0_sptp_clusters;
dbgahb_clusters_size = ARRAY_SIZE(gen7_0_0_sptp_clusters);
- } else {
- BUG_ON(adreno_gpu->info->family > ADRENO_7XX_GEN3);
+ } else if (adreno_gpu->info->family == ADRENO_7XX_GEN2) {
dbgahb_clusters = gen7_2_0_sptp_clusters;
dbgahb_clusters_size = ARRAY_SIZE(gen7_2_0_sptp_clusters);
+ } else {
+ BUG_ON(adreno_gpu->info->family != ADRENO_7XX_GEN3);
+ dbgahb_clusters = gen7_9_0_sptp_clusters;
+ dbgahb_clusters_size = ARRAY_SIZE(gen7_9_0_sptp_clusters);
}
a6xx_state->dbgahb_clusters = state_kcalloc(a6xx_state,
--
2.31.1
[PATCH 0/2] drm/msm: Fixes for devcoredump on a750
Unfortunately the first time around I made the mistake of not checking the devcoredump file closely enough to make sure it had the right contents. This makes sure we're actually using the a750 register lists on a750. Signed-off-by: Connor Abbott --- Connor Abbott (2): drm/msm: Use a7xx family directly in gpu_state drm/msm: Dump correct dbgahb clusters on a750 drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 46 +++-- 1 file changed, 24 insertions(+), 22 deletions(-) --- base-commit: 5acf1f91d74433cbfffd9df962b6e45f5d3ef253 change-id: 20240509-a750-devcoredump-fixes-e59bb5563b1a Best regards, -- Connor Abbott
[PATCH] docs: document python version used for compilation
The drm/msm driver had adopted using Python3 script to generate register header files instead of shipping pre-generated header files. Document the minimal Python version supported by the script. Signed-off-by: Dmitry Baryshkov --- Documentation/process/changes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst index 5685d7bfe4d0..8d225a9f65a2 100644 --- a/Documentation/process/changes.rst +++ b/Documentation/process/changes.rst @@ -63,6 +63,7 @@ cpio any cpio --version GNU tar1.28 tar --version gtags (optional) 6.6.5gtags --version mkimage (optional) 2017.01 mkimage --version +Python (optional) 3.5.xpython3 --version == === .. [#f1] Sphinx is needed only to build the Kernel documentation --- base-commit: 704ba27ac55579704ba1289392448b0c66b56258 change-id: 20240509-python-version-a8b6ca2125ff Best regards, -- Dmitry Baryshkov
Re: [PATCH] drm/msm: remove python 3.9 dependency for compiling msm
On Wed, 8 May 2024 at 02:05, Abhinav Kumar wrote:
>
> Since commit 5acf49119630 ("drm/msm: import gen_header.py script from Mesa"),
> compilation is broken on machines having python versions older than 3.9
> due to dependency on argparse.BooleanOptionalAction.
>
> Switch to use simple bool for the validate flag to remove the dependency.
>
> Fixes: 5acf49119630 ("drm/msm: import gen_header.py script from Mesa")
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/registers/gen_header.py | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
