[PATCH v4 0/3] drm/amd/display: Stop control flow if the divisior is zero

Tiezhu Yang posted 3 patches 11 months ago
drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c  |  2 +-
drivers/gpu/drm/amd/display/dc/os_types.h           |  7 +++++++
drivers/gpu/drm/amd/display/dc/spl/spl_debug.h      | 11 +++++++++++
drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c |  2 +-
4 files changed, 20 insertions(+), 2 deletions(-)
[PATCH v4 0/3] drm/amd/display: Stop control flow if the divisior is zero
Posted by Tiezhu Yang 11 months ago
As far as I can tell, with the current existing macro definitions, there
is no better way to do the minimal and proper changes to stop the control
flow if the divisior is zero.

In order to keep the current ability for the aim of debugging and avoid
printing the warning message twice, it is better to only use ASSERT_BUG()
and SPL_ASSERT_BUG() directly after doing the following two steps:

(1) Add ASSERT_BUG() macro definition
(2) Add SPL_ASSERT_BUG() macro definition

This version is based on 6.13-rc7, tested on x86 and LoongArch.

Tiezhu Yang (3):
  drm/amd/display: Add ASSERT_BUG() macro definition
  drm/amd/display: Add SPL_ASSERT_BUG() macro definition
  drm/amd/display: Harden callers of division functions

 drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c  |  2 +-
 drivers/gpu/drm/amd/display/dc/os_types.h           |  7 +++++++
 drivers/gpu/drm/amd/display/dc/spl/spl_debug.h      | 11 +++++++++++
 drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c |  2 +-
 4 files changed, 20 insertions(+), 2 deletions(-)

-- 
2.42.0
Re: [PATCH v4 0/3] drm/amd/display: Stop control flow if the divisior is zero
Posted by Mingcong Bai 7 months, 3 weeks ago
Hi all,

在 2025/1/14 21:28, Tiezhu Yang 写道:
> As far as I can tell, with the current existing macro definitions, there
> is no better way to do the minimal and proper changes to stop the control
> flow if the divisior is zero.
> 
> In order to keep the current ability for the aim of debugging and avoid
> printing the warning message twice, it is better to only use ASSERT_BUG()
> and SPL_ASSERT_BUG() directly after doing the following two steps:
> 
> (1) Add ASSERT_BUG() macro definition
> (2) Add SPL_ASSERT_BUG() macro definition
> 
> This version is based on 6.13-rc7, tested on x86 and LoongArch.
> 
> Tiezhu Yang (3):
>    drm/amd/display: Add ASSERT_BUG() macro definition
>    drm/amd/display: Add SPL_ASSERT_BUG() macro definition
>    drm/amd/display: Harden callers of division functions
> 
>   drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c  |  2 +-
>   drivers/gpu/drm/amd/display/dc/os_types.h           |  7 +++++++
>   drivers/gpu/drm/amd/display/dc/spl/spl_debug.h      | 11 +++++++++++
>   drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c |  2 +-
>   4 files changed, 20 insertions(+), 2 deletions(-)
> 

Gentle ping on this series...

Harry and Huacai, can you please take a look at this updated series 
(since you have both provided comments on previous revisions?), thank you!

Best Regards,
Mingcong Bai
Re: [PATCH v4 0/3] drm/amd/display: Stop control flow if the divisior is zero
Posted by Alex Deucher 7 months, 3 weeks ago
On Tue, Apr 22, 2025 at 11:55 PM Mingcong Bai <jeffbai@aosc.io> wrote:
>
> Hi all,
>
> 在 2025/1/14 21:28, Tiezhu Yang 写道:
> > As far as I can tell, with the current existing macro definitions, there
> > is no better way to do the minimal and proper changes to stop the control
> > flow if the divisior is zero.
> >
> > In order to keep the current ability for the aim of debugging and avoid
> > printing the warning message twice, it is better to only use ASSERT_BUG()
> > and SPL_ASSERT_BUG() directly after doing the following two steps:
> >
> > (1) Add ASSERT_BUG() macro definition
> > (2) Add SPL_ASSERT_BUG() macro definition
> >
> > This version is based on 6.13-rc7, tested on x86 and LoongArch.
> >
> > Tiezhu Yang (3):
> >    drm/amd/display: Add ASSERT_BUG() macro definition
> >    drm/amd/display: Add SPL_ASSERT_BUG() macro definition
> >    drm/amd/display: Harden callers of division functions
> >
> >   drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c  |  2 +-
> >   drivers/gpu/drm/amd/display/dc/os_types.h           |  7 +++++++
> >   drivers/gpu/drm/amd/display/dc/spl/spl_debug.h      | 11 +++++++++++
> >   drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c |  2 +-
> >   4 files changed, 20 insertions(+), 2 deletions(-)
> >
>
> Gentle ping on this series...
>
> Harry and Huacai, can you please take a look at this updated series
> (since you have both provided comments on previous revisions?), thank you!

I grabbed this series, but after reading this thread:
https://lists.freedesktop.org/archives/dri-devel/2025-April/503021.html
I'm wondering if it wouldn't be better to just allow the divide by 0
rather than triggering BUG().

Alex
Re: [PATCH v4 0/3] drm/amd/display: Stop control flow if the divisior is zero
Posted by Huacai Chen 7 months, 3 weeks ago
On Wed, Apr 23, 2025 at 11:48 AM Mingcong Bai <jeffbai@aosc.io> wrote:
>
> Hi all,
>
> 在 2025/1/14 21:28, Tiezhu Yang 写道:
> > As far as I can tell, with the current existing macro definitions, there
> > is no better way to do the minimal and proper changes to stop the control
> > flow if the divisior is zero.
> >
> > In order to keep the current ability for the aim of debugging and avoid
> > printing the warning message twice, it is better to only use ASSERT_BUG()
> > and SPL_ASSERT_BUG() directly after doing the following two steps:
> >
> > (1) Add ASSERT_BUG() macro definition
> > (2) Add SPL_ASSERT_BUG() macro definition
> >
> > This version is based on 6.13-rc7, tested on x86 and LoongArch.
> >
> > Tiezhu Yang (3):
> >    drm/amd/display: Add ASSERT_BUG() macro definition
> >    drm/amd/display: Add SPL_ASSERT_BUG() macro definition
> >    drm/amd/display: Harden callers of division functions
> >
> >   drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c  |  2 +-
> >   drivers/gpu/drm/amd/display/dc/os_types.h           |  7 +++++++
> >   drivers/gpu/drm/amd/display/dc/spl/spl_debug.h      | 11 +++++++++++
> >   drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c |  2 +-
> >   4 files changed, 20 insertions(+), 2 deletions(-)
> >
>
> Gentle ping on this series...
>
> Harry and Huacai, can you please take a look at this updated series
> (since you have both provided comments on previous revisions?), thank you!
I have already given my R-b.

Huacai

>
> Best Regards,
> Mingcong Bai
>
Re: [PATCH v4 0/3] drm/amd/display: Stop control flow if the divisior is zero
Posted by Huacai Chen 9 months, 3 weeks ago
On Tue, Jan 14, 2025 at 9:29 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> As far as I can tell, with the current existing macro definitions, there
> is no better way to do the minimal and proper changes to stop the control
> flow if the divisior is zero.
>
> In order to keep the current ability for the aim of debugging and avoid
> printing the warning message twice, it is better to only use ASSERT_BUG()
> and SPL_ASSERT_BUG() directly after doing the following two steps:
>
> (1) Add ASSERT_BUG() macro definition
> (2) Add SPL_ASSERT_BUG() macro definition
>
> This version is based on 6.13-rc7, tested on x86 and LoongArch.
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>

>
> Tiezhu Yang (3):
>   drm/amd/display: Add ASSERT_BUG() macro definition
>   drm/amd/display: Add SPL_ASSERT_BUG() macro definition
>   drm/amd/display: Harden callers of division functions
>
>  drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c  |  2 +-
>  drivers/gpu/drm/amd/display/dc/os_types.h           |  7 +++++++
>  drivers/gpu/drm/amd/display/dc/spl/spl_debug.h      | 11 +++++++++++
>  drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c |  2 +-
>  4 files changed, 20 insertions(+), 2 deletions(-)
>
> --
> 2.42.0
>
>