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@xxxxxxxxxxxx>
---
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@xxxxxxx>
发送时间: 2024年8月22日 22:05
收件人: Peng Hongchi/彭洪驰 <hongchi.peng@xxxxxxxxxxxx>
抄送: maarten.lankhorst@xxxxxxxxxxxxxxx; mripard@xxxxxxxxxx; tzimmermann@xxxxxxx; airlied@xxxxxxxxx; daniel@xxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx
主题: 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@xxxxxxxxxxxx>
> ---
> 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! /
---------------
¯\_(ツ)_/¯
