* [PATCH] drm: komeda: Fix an issue related to normalized zpos
@ 2024-08-21 8:19 hongchi.peng
2024-08-21 8:41 ` 回复: " Peng Hongchi/彭洪驰
0 siblings, 1 reply; 4+ messages in thread
From: hongchi.peng @ 2024-08-21 8:19 UTC (permalink / raw)
To: liviu.dudau
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
dri-devel, hongchi.peng
We use komeda_crtc_normalize_zpos to normalize zpos of affected planes
to their blending zorder in CU. If there's only one slave plane in
affected planes and its layer_split property is enabled, order++ for
its split layer, so that when calculating the normalized_zpos
of master planes, the split layer of the slave plane is included, but
the max_slave_zorder does not include the split layer and keep zero
because there's only one slave plane in affacted planes, although we
actually use two slave layers in this commit.
In most cases, this bug does not result in a commit failure, but assume
the following situation:
slave_layer 0: zpos = 0, layer split enabled, normalized_zpos =
0;(use slave_layer 2 as its split layer)
master_layer 0: zpos = 2, layer_split enabled, normalized_zpos =
2;(use master_layer 2 as its split layer)
master_layer 1: zpos = 4, normalized_zpos = 4;
master_layer 3: zpos = 5, normalized_zpos = 5;
kcrtc_st->max_slave_zorder = 0;
When we use master_layer 3 as a input of CU in function
komeda_compiz_set_input and check it with function
komeda_component_check_input, the parameter idx is equal to
normailzed_zpos minus max_slave_zorder, the value of idx is 5
and is euqal to CU's max_active_inputs, so that
komeda_component_check_input returns a -EINVAL value.
To fix the bug described above, when calculating the max_slave_zorder
with the layer_split enabled, count the split layer in this calculation
directly.
Signed-off-by: hongchi.peng <hongchi.peng@siengine.com>
---
drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
index fe46b0ebefea..0554954c8cea 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
@@ -159,7 +159,7 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
struct drm_plane_state *plane_st;
struct drm_plane *plane;
struct list_head zorder_list;
- int order = 0, err;
+ int order = 0, slave_zpos, err;
DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
crtc->base.id, crtc->name);
@@ -200,9 +200,11 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
/* calculate max slave zorder */
if (has_bit(drm_plane_index(plane), kcrtc->slave_planes))
+ slave_zpos = plane_st->normalized_zpos;
+ if (to_kplane_st(plane_st)->layer_split)
+ slave_zpos++;
kcrtc_st->max_slave_zorder =
- max(plane_st->normalized_zpos,
- kcrtc_st->max_slave_zorder);
+ max(slave_zpos, kcrtc_st->max_slave_zorder);
}
crtc_st->zpos_changed = true;
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* 回复: [PATCH] drm: komeda: Fix an issue related to normalized zpos
2024-08-21 8:19 [PATCH] drm: komeda: Fix an issue related to normalized zpos hongchi.peng
@ 2024-08-21 8:41 ` Peng Hongchi/彭洪驰
0 siblings, 0 replies; 4+ messages in thread
From: Peng Hongchi/彭洪驰 @ 2024-08-21 8:41 UTC (permalink / raw)
To: Peng Hongchi/彭洪驰, liviu.dudau@arm.com
Cc: maarten.lankhorst@linux.intel.com, mripard@kernel.org,
tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch,
dri-devel@lists.freedesktop.org
Hi, all:
I'm sorry that I forgot to add a pair of brackets, and I'll send it again soon.
Best Regards,
Hongchi Peng
-----邮件原件-----
发件人: hongchi.peng <hongchi.peng@siengine.com>
发送时间: 2024年8月21日 16:19
收件人: liviu.dudau@arm.com
抄送: maarten.lankhorst@linux.intel.com; mripard@kernel.org; tzimmermann@suse.de; airlied@gmail.com; daniel@ffwll.ch; dri-devel@lists.freedesktop.org; Peng Hongchi/彭洪驰 <hongchi.peng@siengine.com>
主题: [PATCH] drm: komeda: Fix an issue related to normalized zpos
We use komeda_crtc_normalize_zpos to normalize zpos of affected planes to their blending zorder in CU. If there's only one slave plane in affected planes and its layer_split property is enabled, order++ for its split layer, so that when calculating the normalized_zpos of master planes, the split layer of the slave plane is included, but the max_slave_zorder does not include the split layer and keep zero because there's only one slave plane in affacted planes, although we actually use two slave layers in this commit.
In most cases, this bug does not result in a commit failure, but assume the following situation:
slave_layer 0: zpos = 0, layer split enabled, normalized_zpos =
0;(use slave_layer 2 as its split layer)
master_layer 0: zpos = 2, layer_split enabled, normalized_zpos =
2;(use master_layer 2 as its split layer)
master_layer 1: zpos = 4, normalized_zpos = 4;
master_layer 3: zpos = 5, normalized_zpos = 5;
kcrtc_st->max_slave_zorder = 0;
When we use master_layer 3 as a input of CU in function komeda_compiz_set_input and check it with function komeda_component_check_input, the parameter idx is equal to normailzed_zpos minus max_slave_zorder, the value of idx is 5 and is euqal to CU's max_active_inputs, so that komeda_component_check_input returns a -EINVAL value.
To fix the bug described above, when calculating the max_slave_zorder with the layer_split enabled, count the split layer in this calculation directly.
Signed-off-by: hongchi.peng <hongchi.peng@siengine.com>
---
drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
index fe46b0ebefea..0554954c8cea 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
@@ -159,7 +159,7 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
struct drm_plane_state *plane_st;
struct drm_plane *plane;
struct list_head zorder_list;
- int order = 0, err;
+ int order = 0, slave_zpos, err;
DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
crtc->base.id, crtc->name);
@@ -200,9 +200,11 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
/* calculate max slave zorder */
if (has_bit(drm_plane_index(plane), kcrtc->slave_planes))
+ slave_zpos = plane_st->normalized_zpos;
+ if (to_kplane_st(plane_st)->layer_split)
+ slave_zpos++;
kcrtc_st->max_slave_zorder =
- max(plane_st->normalized_zpos,
- kcrtc_st->max_slave_zorder);
+ max(slave_zpos, kcrtc_st->max_slave_zorder);
}
crtc_st->zpos_changed = true;
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] drm: komeda: Fix an issue related to normalized zpos
@ 2024-08-21 8:56 hongchi.peng
2024-08-22 14:05 ` Liviu Dudau
0 siblings, 1 reply; 4+ messages in thread
From: hongchi.peng @ 2024-08-21 8:56 UTC (permalink / raw)
To: liviu.dudau
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
dri-devel, hongchi.peng
We use komeda_crtc_normalize_zpos to normalize zpos of affected planes
to their blending zorder in CU. If there's only one slave plane in
affected planes and its layer_split property is enabled, order++ for
its split layer, so that when calculating the normalized_zpos
of master planes, the split layer of the slave plane is included, but
the max_slave_zorder does not include the split layer and keep zero
because there's only one slave plane in affacted planes, although we
actually use two slave layers in this commit.
In most cases, this bug does not result in a commit failure, but assume
the following situation:
slave_layer 0: zpos = 0, layer split enabled, normalized_zpos =
0;(use slave_layer 2 as its split layer)
master_layer 0: zpos = 2, layer_split enabled, normalized_zpos =
2;(use master_layer 2 as its split layer)
master_layer 1: zpos = 4, normalized_zpos = 4;
master_layer 3: zpos = 5, normalized_zpos = 5;
kcrtc_st->max_slave_zorder = 0;
When we use master_layer 3 as a input of CU in function
komeda_compiz_set_input and check it with function
komeda_component_check_input, the parameter idx is equal to
normailzed_zpos minus max_slave_zorder, the value of idx is 5
and is euqal to CU's max_active_inputs, so that
komeda_component_check_input returns a -EINVAL value.
To fix the bug described above, when calculating the max_slave_zorder
with the layer_split enabled, count the split layer in this calculation
directly.
Signed-off-by: hongchi.peng <hongchi.peng@siengine.com>
---
drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
index fe46b0ebefea..b3db828284e4 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
@@ -159,7 +159,7 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
struct drm_plane_state *plane_st;
struct drm_plane *plane;
struct list_head zorder_list;
- int order = 0, err;
+ int order = 0, slave_zpos, err;
DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
crtc->base.id, crtc->name);
@@ -199,10 +199,13 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
plane_st->zpos, plane_st->normalized_zpos);
/* calculate max slave zorder */
- if (has_bit(drm_plane_index(plane), kcrtc->slave_planes))
+ if (has_bit(drm_plane_index(plane), kcrtc->slave_planes)) {
+ slave_zpos = plane_st->normalized_zpos;
+ if (to_kplane_st(plane_st)->layer_split)
+ slave_zpos++;
kcrtc_st->max_slave_zorder =
- max(plane_st->normalized_zpos,
- kcrtc_st->max_slave_zorder);
+ max(slave_zpos, kcrtc_st->max_slave_zorder);
+ }
}
crtc_st->zpos_changed = true;
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm: komeda: Fix an issue related to normalized zpos
2024-08-21 8:56 hongchi.peng
@ 2024-08-22 14:05 ` Liviu Dudau
2024-08-23 2:53 ` 回复: " Peng Hongchi/彭洪驰
0 siblings, 1 reply; 4+ messages in thread
From: Liviu Dudau @ 2024-08-22 14:05 UTC (permalink / raw)
To: hongchi.peng
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
dri-devel
Hi Hongchi,
On Wed, Aug 21, 2024 at 04:56:13PM +0800, hongchi.peng wrote:
> We use komeda_crtc_normalize_zpos to normalize zpos of affected planes
> to their blending zorder in CU. If there's only one slave plane in
> affected planes and its layer_split property is enabled, order++ for
> its split layer, so that when calculating the normalized_zpos
> of master planes, the split layer of the slave plane is included, but
> the max_slave_zorder does not include the split layer and keep zero
> because there's only one slave plane in affacted planes, although we
> actually use two slave layers in this commit.
>
> In most cases, this bug does not result in a commit failure, but assume
> the following situation:
> slave_layer 0: zpos = 0, layer split enabled, normalized_zpos =
> 0;(use slave_layer 2 as its split layer)
> master_layer 0: zpos = 2, layer_split enabled, normalized_zpos =
> 2;(use master_layer 2 as its split layer)
> master_layer 1: zpos = 4, normalized_zpos = 4;
> master_layer 3: zpos = 5, normalized_zpos = 5;
> kcrtc_st->max_slave_zorder = 0;
> When we use master_layer 3 as a input of CU in function
> komeda_compiz_set_input and check it with function
> komeda_component_check_input, the parameter idx is equal to
> normailzed_zpos minus max_slave_zorder, the value of idx is 5
> and is euqal to CU's max_active_inputs, so that
> komeda_component_check_input returns a -EINVAL value.
Yes, indeed, you have found a bug in the calculations when layer_split is set.
But I was also looking through the code trying to find where layer_split gets
set and I could not find it, do you have some extra patches?
>
> To fix the bug described above, when calculating the max_slave_zorder
> with the layer_split enabled, count the split layer in this calculation
> directly.
>
> Signed-off-by: hongchi.peng <hongchi.peng@siengine.com>
> ---
> drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index fe46b0ebefea..b3db828284e4 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -159,7 +159,7 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> struct drm_plane_state *plane_st;
> struct drm_plane *plane;
> struct list_head zorder_list;
> - int order = 0, err;
> + int order = 0, slave_zpos, err;
Also, the build bot has already flagged it, your patch needs some improvements.
slave_zpos needs to be u32 if it's going to be compared against max_slave_zorder.
Best regards,
Liviu
>
> DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
> crtc->base.id, crtc->name);
> @@ -199,10 +199,13 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> plane_st->zpos, plane_st->normalized_zpos);
>
> /* calculate max slave zorder */
> - if (has_bit(drm_plane_index(plane), kcrtc->slave_planes))
> + if (has_bit(drm_plane_index(plane), kcrtc->slave_planes)) {
> + slave_zpos = plane_st->normalized_zpos;
> + if (to_kplane_st(plane_st)->layer_split)
> + slave_zpos++;
> kcrtc_st->max_slave_zorder =
> - max(plane_st->normalized_zpos,
> - kcrtc_st->max_slave_zorder);
> + max(slave_zpos, kcrtc_st->max_slave_zorder);
> + }
> }
>
> crtc_st->zpos_changed = true;
> --
> 2.34.1
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 4+ messages in thread* 回复: [PATCH] drm: komeda: Fix an issue related to normalized zpos
2024-08-22 14:05 ` Liviu Dudau
@ 2024-08-23 2:53 ` Peng Hongchi/彭洪驰
2024-08-24 12:32 ` Liviu Dudau
0 siblings, 1 reply; 4+ messages in thread
From: Peng Hongchi/彭洪驰 @ 2024-08-23 2:53 UTC (permalink / raw)
To: Liviu Dudau
Cc: maarten.lankhorst@linux.intel.com, mripard@kernel.org,
tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch,
dri-devel@lists.freedesktop.org
Hi, Liviu,
I'm sorry for my carelessness and thanks for your correction, the corrected patch is as follows.
And we do have an extra patch to set layer_split, but this part of the code is owned by my colleague,
So that I cannot upload it, sorry about this.
Signed-off-by: hongchi.peng <hongchi.peng@siengine.com>
---
drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
index fe46b0ebefea..ab769a0a4afa 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
@@ -160,6 +160,7 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
struct drm_plane *plane;
struct list_head zorder_list;
int order = 0, err;
+ u32 slave_zpos;
DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
crtc->base.id, crtc->name);
@@ -199,10 +200,13 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
plane_st->zpos, plane_st->normalized_zpos);
/* calculate max slave zorder */
- if (has_bit(drm_plane_index(plane), kcrtc->slave_planes))
+ if (has_bit(drm_plane_index(plane), kcrtc->slave_planes)) {
+ slave_zpos = plane_st->normalized_zpos;
+ if (to_kplane_st(plane_st)->layer_split)
+ slave_zpos++;
kcrtc_st->max_slave_zorder =
- max(plane_st->normalized_zpos,
- kcrtc_st->max_slave_zorder);
+ max(slave_zpos, kcrtc_st->max_slave_zorder);
+ }
}
crtc_st->zpos_changed = true;
--
2.34.1
Best Regards,
Hongchi Peng
-----邮件原件-----
发件人: Liviu Dudau <liviu.dudau@arm.com>
发送时间: 2024年8月22日 22:05
收件人: Peng Hongchi/彭洪驰 <hongchi.peng@siengine.com>
抄送: maarten.lankhorst@linux.intel.com; mripard@kernel.org; tzimmermann@suse.de; airlied@gmail.com; daniel@ffwll.ch; dri-devel@lists.freedesktop.org
主题: Re: [PATCH] drm: komeda: Fix an issue related to normalized zpos
Hi Hongchi,
On Wed, Aug 21, 2024 at 04:56:13PM +0800, hongchi.peng wrote:
> We use komeda_crtc_normalize_zpos to normalize zpos of affected planes
> to their blending zorder in CU. If there's only one slave plane in
> affected planes and its layer_split property is enabled, order++ for
> its split layer, so that when calculating the normalized_zpos of
> master planes, the split layer of the slave plane is included, but the
> max_slave_zorder does not include the split layer and keep zero
> because there's only one slave plane in affacted planes, although we
> actually use two slave layers in this commit.
>
> In most cases, this bug does not result in a commit failure, but
> assume the following situation:
> slave_layer 0: zpos = 0, layer split enabled, normalized_zpos =
> 0;(use slave_layer 2 as its split layer)
> master_layer 0: zpos = 2, layer_split enabled, normalized_zpos =
> 2;(use master_layer 2 as its split layer)
> master_layer 1: zpos = 4, normalized_zpos = 4;
> master_layer 3: zpos = 5, normalized_zpos = 5;
> kcrtc_st->max_slave_zorder = 0;
> When we use master_layer 3 as a input of CU in function
> komeda_compiz_set_input and check it with function
> komeda_component_check_input, the parameter idx is equal to
> normailzed_zpos minus max_slave_zorder, the value of idx is 5 and is
> euqal to CU's max_active_inputs, so that komeda_component_check_input
> returns a -EINVAL value.
Yes, indeed, you have found a bug in the calculations when layer_split is set.
But I was also looking through the code trying to find where layer_split gets set and I could not find it, do you have some extra patches?
>
> To fix the bug described above, when calculating the max_slave_zorder
> with the layer_split enabled, count the split layer in this
> calculation directly.
>
> Signed-off-by: hongchi.peng <hongchi.peng@siengine.com>
> ---
> drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index fe46b0ebefea..b3db828284e4 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -159,7 +159,7 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> struct drm_plane_state *plane_st;
> struct drm_plane *plane;
> struct list_head zorder_list;
> - int order = 0, err;
> + int order = 0, slave_zpos, err;
Also, the build bot has already flagged it, your patch needs some improvements.
slave_zpos needs to be u32 if it's going to be compared against max_slave_zorder.
Best regards,
Liviu
>
> DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
> crtc->base.id, crtc->name);
> @@ -199,10 +199,13 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> plane_st->zpos, plane_st->normalized_zpos);
>
> /* calculate max slave zorder */
> - if (has_bit(drm_plane_index(plane), kcrtc->slave_planes))
> + if (has_bit(drm_plane_index(plane), kcrtc->slave_planes)) {
> + slave_zpos = plane_st->normalized_zpos;
> + if (to_kplane_st(plane_st)->layer_split)
> + slave_zpos++;
> kcrtc_st->max_slave_zorder =
> - max(plane_st->normalized_zpos,
> - kcrtc_st->max_slave_zorder);
> + max(slave_zpos, kcrtc_st->max_slave_zorder);
> + }
> }
>
> crtc_st->zpos_changed = true;
> --
> 2.34.1
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: 回复: [PATCH] drm: komeda: Fix an issue related to normalized zpos
2024-08-23 2:53 ` 回复: " Peng Hongchi/彭洪驰
@ 2024-08-24 12:32 ` Liviu Dudau
0 siblings, 0 replies; 4+ messages in thread
From: Liviu Dudau @ 2024-08-24 12:32 UTC (permalink / raw)
To: Peng Hongchi/彭洪驰
Cc: maarten.lankhorst@linux.intel.com, mripard@kernel.org,
tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch,
dri-devel@lists.freedesktop.org
On Fri, Aug 23, 2024 at 02:53:06AM +0000, Peng Hongchi/彭洪驰 wrote:
> Hi, Liviu,
Hi,
>
> I'm sorry for my carelessness and thanks for your correction, the corrected patch is as follows.
> And we do have an extra patch to set layer_split, but this part of the code is owned by my colleague,
> So that I cannot upload it, sorry about this.
>
> Signed-off-by: hongchi.peng <hongchi.peng@siengine.com>
This cannot be your commit message. If you want to make comments in a patch, I suggest you put them after
a three-line dash, like this:
---
Add your comment here
> ---
Keep this marker as it will signal the start of the patch.
> drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index fe46b0ebefea..ab769a0a4afa 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -160,6 +160,7 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> struct drm_plane *plane;
> struct list_head zorder_list;
> int order = 0, err;
> + u32 slave_zpos;
Can you please initialise this to zero?
>
> DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
> crtc->base.id, crtc->name);
> @@ -199,10 +200,13 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> plane_st->zpos, plane_st->normalized_zpos);
>
> /* calculate max slave zorder */
> - if (has_bit(drm_plane_index(plane), kcrtc->slave_planes))
> + if (has_bit(drm_plane_index(plane), kcrtc->slave_planes)) {
> + slave_zpos = plane_st->normalized_zpos;
> + if (to_kplane_st(plane_st)->layer_split)
> + slave_zpos++;
> kcrtc_st->max_slave_zorder =
> - max(plane_st->normalized_zpos,
> - kcrtc_st->max_slave_zorder);
> + max(slave_zpos, kcrtc_st->max_slave_zorder);
> + }
> }
>
> crtc_st->zpos_changed = true;
> --
> 2.34.1
>
> Best Regards,
> Hongchi Peng
Also, can you version your patches to help me track them better?
Best regards,
Liviu
>
> -----邮件原件-----
> 发件人: Liviu Dudau <liviu.dudau@arm.com>
> 发送时间: 2024年8月22日 22:05
> 收件人: Peng Hongchi/彭洪驰 <hongchi.peng@siengine.com>
> 抄送: maarten.lankhorst@linux.intel.com; mripard@kernel.org; tzimmermann@suse.de; airlied@gmail.com; daniel@ffwll.ch; dri-devel@lists.freedesktop.org
> 主题: Re: [PATCH] drm: komeda: Fix an issue related to normalized zpos
>
> Hi Hongchi,
>
> On Wed, Aug 21, 2024 at 04:56:13PM +0800, hongchi.peng wrote:
> > We use komeda_crtc_normalize_zpos to normalize zpos of affected planes
> > to their blending zorder in CU. If there's only one slave plane in
> > affected planes and its layer_split property is enabled, order++ for
> > its split layer, so that when calculating the normalized_zpos of
> > master planes, the split layer of the slave plane is included, but the
> > max_slave_zorder does not include the split layer and keep zero
> > because there's only one slave plane in affacted planes, although we
> > actually use two slave layers in this commit.
> >
> > In most cases, this bug does not result in a commit failure, but
> > assume the following situation:
> > slave_layer 0: zpos = 0, layer split enabled, normalized_zpos =
> > 0;(use slave_layer 2 as its split layer)
> > master_layer 0: zpos = 2, layer_split enabled, normalized_zpos =
> > 2;(use master_layer 2 as its split layer)
> > master_layer 1: zpos = 4, normalized_zpos = 4;
> > master_layer 3: zpos = 5, normalized_zpos = 5;
> > kcrtc_st->max_slave_zorder = 0;
> > When we use master_layer 3 as a input of CU in function
> > komeda_compiz_set_input and check it with function
> > komeda_component_check_input, the parameter idx is equal to
> > normailzed_zpos minus max_slave_zorder, the value of idx is 5 and is
> > euqal to CU's max_active_inputs, so that komeda_component_check_input
> > returns a -EINVAL value.
>
> Yes, indeed, you have found a bug in the calculations when layer_split is set.
> But I was also looking through the code trying to find where layer_split gets set and I could not find it, do you have some extra patches?
>
> >
> > To fix the bug described above, when calculating the max_slave_zorder
> > with the layer_split enabled, count the split layer in this
> > calculation directly.
> >
> > Signed-off-by: hongchi.peng <hongchi.peng@siengine.com>
> > ---
> > drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > index fe46b0ebefea..b3db828284e4 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > @@ -159,7 +159,7 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> > struct drm_plane_state *plane_st;
> > struct drm_plane *plane;
> > struct list_head zorder_list;
> > - int order = 0, err;
> > + int order = 0, slave_zpos, err;
>
> Also, the build bot has already flagged it, your patch needs some improvements.
> slave_zpos needs to be u32 if it's going to be compared against max_slave_zorder.
>
> Best regards,
> Liviu
>
> >
> > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
> > crtc->base.id, crtc->name);
> > @@ -199,10 +199,13 @@ static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> > plane_st->zpos, plane_st->normalized_zpos);
> >
> > /* calculate max slave zorder */
> > - if (has_bit(drm_plane_index(plane), kcrtc->slave_planes))
> > + if (has_bit(drm_plane_index(plane), kcrtc->slave_planes)) {
> > + slave_zpos = plane_st->normalized_zpos;
> > + if (to_kplane_st(plane_st)->layer_split)
> > + slave_zpos++;
> > kcrtc_st->max_slave_zorder =
> > - max(plane_st->normalized_zpos,
> > - kcrtc_st->max_slave_zorder);
> > + max(slave_zpos, kcrtc_st->max_slave_zorder);
> > + }
> > }
> >
> > crtc_st->zpos_changed = true;
> > --
> > 2.34.1
> >
>
> --
> ====================
> | I would like to |
> | fix the world, |
> | but they're not |
> | giving me the |
> \ source code! /
> ---------------
> ¯\_(ツ)_/¯
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-08-24 12:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-21 8:19 [PATCH] drm: komeda: Fix an issue related to normalized zpos hongchi.peng
2024-08-21 8:41 ` 回复: " Peng Hongchi/彭洪驰
-- strict thread matches above, loose matches on Subject: below --
2024-08-21 8:56 hongchi.peng
2024-08-22 14:05 ` Liviu Dudau
2024-08-23 2:53 ` 回复: " Peng Hongchi/彭洪驰
2024-08-24 12:32 ` Liviu Dudau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.