[PATCH] drm: komeda: Fix an issue related to normalized zpos

[PATCH] drm: komeda: Fix an issue related to normalized zpos

[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

Re: [PATCH] drm: komeda: Fix an issue related to normalized zpos

[kernel test robot]

Hi hongchi.peng,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.11-rc4 next-20240822]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/hongchi-peng/drm-komeda-Fix-an-issue-related-to-normalized-zpos/20240822-045334
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20240821085613.5408-1-hongchi.peng%40siengine.com
patch subject: [PATCH] drm: komeda: Fix an issue related to normalized zpos
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240822/202408222005.Pr9Vj8sj-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240822/202408222005.Pr9Vj8sj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408222005.Pr9Vj8sj-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from <command-line>:
   drivers/gpu/drm/arm/display/komeda/komeda_kms.c: In function 'komeda_crtc_normalize_zpos':
>> include/linux/compiler_types.h:510:45: error: call to '__compiletime_assert_301' declared with attribute error: max(slave_zpos, kcrtc_st->max_slave_zorder) signedness error
     510 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |                                             ^
   include/linux/compiler_types.h:491:25: note: in definition of macro '__compiletime_assert'
     491 |                         prefix ## suffix();                             \
         |                         ^~~~~~
   include/linux/compiler_types.h:510:9: note: in expansion of macro '_compiletime_assert'
     510 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:100:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     100 |         BUILD_BUG_ON_MSG(!__types_ok(x,y,ux,uy),        \
         |         ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:105:9: note: in expansion of macro '__careful_cmp_once'
     105 |         __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
         |         ^~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:136:25: note: in expansion of macro '__careful_cmp'
     136 | #define max(x, y)       __careful_cmp(max, x, y)
         |                         ^~~~~~~~~~~~~
   drivers/gpu/drm/arm/display/komeda/komeda_kms.c:207:33: note: in expansion of macro 'max'
     207 |                                 max(slave_zpos, kcrtc_st->max_slave_zorder);
         |                                 ^~~


vim +/__compiletime_assert_301 +510 include/linux/compiler_types.h

eb5c2d4b45e3d2 Will Deacon 2020-07-21  496  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  497  #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21  498  	__compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  499  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  500  /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21  501   * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  502   * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21  503   * @msg:       a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  504   *
eb5c2d4b45e3d2 Will Deacon 2020-07-21  505   * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  506   * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  507   * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21  508   */
eb5c2d4b45e3d2 Will Deacon 2020-07-21  509  #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @510  	_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  511  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] drm: komeda: Fix an issue related to normalized zpos

[kernel test robot]

Hi hongchi.peng,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.11-rc4 next-20240822]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/hongchi-peng/drm-komeda-Fix-an-issue-related-to-normalized-zpos/20240822-045334
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20240821085613.5408-1-hongchi.peng%40siengine.com
patch subject: [PATCH] drm: komeda: Fix an issue related to normalized zpos
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240822/202408222146.r7E6lCGh-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 26670e7fa4f032a019d23d56c6a02926e854e8af)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240822/202408222146.r7E6lCGh-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408222146.r7E6lCGh-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/arm/display/komeda/komeda_kms.c:7:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/gpu/drm/arm/display/komeda/komeda_kms.c:7:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/gpu/drm/arm/display/komeda/komeda_kms.c:7:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   In file included from drivers/gpu/drm/arm/display/komeda/komeda_kms.c:9:
   In file included from include/drm/drm_atomic.h:31:
   In file included from include/drm/drm_crtc.h:32:
   In file included from include/drm/drm_modes.h:33:
   In file included from include/drm/drm_connector.h:32:
   In file included from include/drm/drm_util.h:36:
   In file included from include/linux/kgdb.h:19:
   In file included from include/linux/kprobes.h:28:
   In file included from include/linux/ftrace.h:13:
   In file included from include/linux/kallsyms.h:13:
   In file included from include/linux/mm.h:2228:
   include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     514 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/gpu/drm/arm/display/komeda/komeda_kms.c:207:5: error: call to '__compiletime_assert_310' declared with 'error' attribute: max(slave_zpos, kcrtc_st->max_slave_zorder) signedness error
     207 |                                 max(slave_zpos, kcrtc_st->max_slave_zorder);
         |                                 ^
   include/linux/minmax.h:136:19: note: expanded from macro 'max'
     136 | #define max(x, y)       __careful_cmp(max, x, y)
         |                         ^
   include/linux/minmax.h:105:2: note: expanded from macro '__careful_cmp'
     105 |         __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
         |         ^
   include/linux/minmax.h:100:2: note: expanded from macro '__careful_cmp_once'
     100 |         BUILD_BUG_ON_MSG(!__types_ok(x,y,ux,uy),        \
         |         ^
   note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:498:2: note: expanded from macro '_compiletime_assert'
     498 |         __compiletime_assert(condition, msg, prefix, suffix)
         |         ^
   include/linux/compiler_types.h:491:4: note: expanded from macro '__compiletime_assert'
     491 |                         prefix ## suffix();                             \
         |                         ^
   <scratch space>:79:1: note: expanded from here
      79 | __compiletime_assert_310
         | ^
   7 warnings and 1 error generated.


vim +207 drivers/gpu/drm/arm/display/komeda/komeda_kms.c

   151	
   152	static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
   153					      struct drm_crtc_state *crtc_st)
   154	{
   155		struct drm_atomic_state *state = crtc_st->state;
   156		struct komeda_crtc *kcrtc = to_kcrtc(crtc);
   157		struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(crtc_st);
   158		struct komeda_plane_state *kplane_st;
   159		struct drm_plane_state *plane_st;
   160		struct drm_plane *plane;
   161		struct list_head zorder_list;
   162		int order = 0, slave_zpos, err;
   163	
   164		DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
   165				 crtc->base.id, crtc->name);
   166	
   167		INIT_LIST_HEAD(&zorder_list);
   168	
   169		/* This loop also added all effected planes into the new state */
   170		drm_for_each_plane_mask(plane, crtc->dev, crtc_st->plane_mask) {
   171			plane_st = drm_atomic_get_plane_state(state, plane);
   172			if (IS_ERR(plane_st))
   173				return PTR_ERR(plane_st);
   174	
   175			/* Build a list by zpos increasing */
   176			err = komeda_plane_state_list_add(plane_st, &zorder_list);
   177			if (err)
   178				return err;
   179		}
   180	
   181		kcrtc_st->max_slave_zorder = 0;
   182	
   183		list_for_each_entry(kplane_st, &zorder_list, zlist_node) {
   184			plane_st = &kplane_st->base;
   185			plane = plane_st->plane;
   186	
   187			plane_st->normalized_zpos = order++;
   188			/* When layer_split has been enabled, one plane will be handled
   189			 * by two separated komeda layers (left/right), which may needs
   190			 * two zorders.
   191			 * - zorder: for left_layer for left display part.
   192			 * - zorder + 1: will be reserved for right layer.
   193			 */
   194			if (to_kplane_st(plane_st)->layer_split)
   195				order++;
   196	
   197			DRM_DEBUG_ATOMIC("[PLANE:%d:%s] zpos:%d, normalized zpos: %d\n",
   198					 plane->base.id, plane->name,
   199					 plane_st->zpos, plane_st->normalized_zpos);
   200	
   201			/* calculate max slave zorder */
   202			if (has_bit(drm_plane_index(plane), kcrtc->slave_planes)) {
   203				slave_zpos = plane_st->normalized_zpos;
   204				if (to_kplane_st(plane_st)->layer_split)
   205					slave_zpos++;
   206				kcrtc_st->max_slave_zorder =
 > 207					max(slave_zpos, kcrtc_st->max_slave_zorder);
   208			}
   209		}
   210	
   211		crtc_st->zpos_changed = true;
   212	
   213		return 0;
   214	}
   215	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] drm: komeda: Fix an issue related to normalized zpos

[Liviu Dudau]

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!  /
  ---------------
    ¯\_(ツ)_/¯

回复: [PATCH] drm: komeda: Fix an issue related to normalized zpos

[Peng Hongchi/彭洪驰]

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!  /
  ---------------
    ¯\_(ツ)_/¯

Re: 回复: [PATCH] drm: komeda: Fix an issue related to normalized zpos

[Liviu Dudau]

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!  /
  ---------------
    ¯\_(ツ)_/¯

回复: 回复: [PATCH] drm: komeda: Fix an issue related to normalized zpos

[Peng Hongchi/彭洪驰]

Hi, Liviu,

	I'll initialize 'slave_zpos' to zero and resend the [patch v2] soon, thanks!

Best Regards,
Hongchi Peng

-----邮件原件-----
发件人: Liviu Dudau <liviu.dudau@arm.com> 
发送时间: 2024年8月24日 20:33
收件人: 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

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!  /
  ---------------
    ¯\_(ツ)_/¯

[PATCH] drm: komeda: Fix an issue related to normalized zpos

[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

回复: [PATCH] drm: komeda: Fix an issue related to normalized zpos

[Peng Hongchi/彭洪驰]

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