| Message ID | 20240821085613.5408-1-hongchi.peng@siengine.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: komeda: Fix an issue related to normalized zpos | expand |
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
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
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 >
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;
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(-)