* [PATCH v2] drm/i915/tv: avoid possible division by zero
@ 2023-07-18 1:32 Su Hui
2023-07-24 17:35 ` [Intel-gfx] " Andi Shyti
0 siblings, 1 reply; 6+ messages in thread
From: Su Hui @ 2023-07-18 1:32 UTC (permalink / raw)
To: jani.nikula, joonas.lahtinen, rodrigo.vivi, tvrtko.ursulin,
airlied, daniel, nathan, ndesaulniers, trix, andrzej.hajda
Cc: suhui, intel-gfx, llvm, kernel-janitors, linux-kernel, dri-devel,
mripard
Clang warning: drivers/gpu/drm/i915/display/intel_tv.c:
line 991, column 22 Division by zero.
Assuming tv_mode->oversample=1 and (!tv_mode->progressive)=1,
then division by zero will happen.
Fixes: 1bba5543e4fe ("drm/i915: Fix TV encoder clock computation")
Signed-off-by: Su Hui <suhui@nfschina.com>
---
drivers/gpu/drm/i915/display/intel_tv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
index 36b479b46b60..f59553f7c132 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -988,7 +988,7 @@ intel_tv_mode_to_mode(struct drm_display_mode *mode,
const struct tv_mode *tv_mode,
int clock)
{
- mode->clock = clock / (tv_mode->oversample >> !tv_mode->progressive);
+ mode->clock = clock / tv_mode->oversample << !tv_mode->progressive;
/*
* tv_mode horizontal timings:
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/tv: avoid possible division by zero
2023-07-18 1:32 [PATCH v2] drm/i915/tv: avoid possible division by zero Su Hui
@ 2023-07-24 17:35 ` Andi Shyti
2023-07-25 4:06 ` Su Hui
0 siblings, 1 reply; 6+ messages in thread
From: Andi Shyti @ 2023-07-24 17:35 UTC (permalink / raw)
To: Su Hui
Cc: mripard, tvrtko.ursulin, kernel-janitors, llvm, andrzej.hajda,
trix, intel-gfx, ndesaulniers, linux-kernel, nathan, dri-devel,
rodrigo.vivi
On Tue, Jul 18, 2023 at 09:32:17AM +0800, Su Hui wrote:
> Clang warning: drivers/gpu/drm/i915/display/intel_tv.c:
> line 991, column 22 Division by zero.
> Assuming tv_mode->oversample=1 and (!tv_mode->progressive)=1,
> then division by zero will happen.
>
> Fixes: 1bba5543e4fe ("drm/i915: Fix TV encoder clock computation")
> Signed-off-by: Su Hui <suhui@nfschina.com>
> ---
> drivers/gpu/drm/i915/display/intel_tv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
> index 36b479b46b60..f59553f7c132 100644
> --- a/drivers/gpu/drm/i915/display/intel_tv.c
> +++ b/drivers/gpu/drm/i915/display/intel_tv.c
> @@ -988,7 +988,7 @@ intel_tv_mode_to_mode(struct drm_display_mode *mode,
> const struct tv_mode *tv_mode,
> int clock)
> {
> - mode->clock = clock / (tv_mode->oversample >> !tv_mode->progressive);
> + mode->clock = clock / tv_mode->oversample << !tv_mode->progressive;
but this does not provide the same value. Try with:
8 / (2 >> 1)
and
8 / 2 << 1
They are definitely different.
The real check could be:
if (!(tv_mode->oversample >> 1))
return ...
But first I would check if that's actually possible.
Andi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/tv: avoid possible division by zero
2023-07-24 17:35 ` [Intel-gfx] " Andi Shyti
@ 2023-07-25 4:06 ` Su Hui
2023-07-25 5:51 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Su Hui @ 2023-07-25 4:06 UTC (permalink / raw)
To: Andi Shyti
Cc: mripard, tvrtko.ursulin, kernel-janitors, llvm, andrzej.hajda,
trix, intel-gfx, ndesaulniers, linux-kernel, nathan, dri-devel,
rodrigo.vivi
On 2023/7/25 01:35, Andi Shyti wrote:
> On Tue, Jul 18, 2023 at 09:32:17AM +0800, Su Hui wrote:
>> Clang warning: drivers/gpu/drm/i915/display/intel_tv.c:
>> line 991, column 22 Division by zero.
>> Assuming tv_mode->oversample=1 and (!tv_mode->progressive)=1,
>> then division by zero will happen.
>>
>> Fixes: 1bba5543e4fe ("drm/i915: Fix TV encoder clock computation")
>> Signed-off-by: Su Hui <suhui@nfschina.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_tv.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
>> index 36b479b46b60..f59553f7c132 100644
>> --- a/drivers/gpu/drm/i915/display/intel_tv.c
>> +++ b/drivers/gpu/drm/i915/display/intel_tv.c
>> @@ -988,7 +988,7 @@ intel_tv_mode_to_mode(struct drm_display_mode *mode,
>> const struct tv_mode *tv_mode,
>> int clock)
>> {
>> - mode->clock = clock / (tv_mode->oversample >> !tv_mode->progressive);
>> + mode->clock = clock / tv_mode->oversample << !tv_mode->progressive;
> but this does not provide the same value. Try with:
>
> 8 / (2 >> 1)
>
> and
>
> 8 / 2 << 1
>
> They are definitely different.
>
> The real check could be:
>
> if (!(tv_mode->oversample >> 1))
> return ...
>
> But first I would check if that's actually possible.
Oh, I have a v3 patch, like this:
- mode->clock = clock / (tv_mode->oversample >>
!tv_mode->progressive);
+ mode->clock = clock;
+ if (tv_mode->oversample >> !tv_mode->progressive)
+ mode->clock /= tv_mode->oversample >> 1;
But I'm not sure does it need to print some error messages or do some
things when
"tv_mode->oversample << !tv_mode->progressive" is zero?
If all right , I will send this v3 patch.
Su Hui
> Andi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/tv: avoid possible division by zero
2023-07-25 4:06 ` Su Hui
@ 2023-07-25 5:51 ` Dan Carpenter
2023-07-26 1:21 ` Su Hui
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2023-07-25 5:51 UTC (permalink / raw)
To: Su Hui
Cc: mripard, tvrtko.ursulin, kernel-janitors, llvm, trix, intel-gfx,
ndesaulniers, linux-kernel, nathan, andrzej.hajda, dri-devel,
Andi Shyti, rodrigo.vivi
The reason why the first five attempts had bugs is because we are
trying to write it in the most complicated way possible, shifting by
logical not what?
regards,
dan carpenter
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
index 36b479b46b60..6997b6cb1df2 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -988,7 +988,13 @@ intel_tv_mode_to_mode(struct drm_display_mode *mode,
const struct tv_mode *tv_mode,
int clock)
{
- mode->clock = clock / (tv_mode->oversample >> !tv_mode->progressive);
+ int div = tv_mode->oversample;
+
+ if (!tv_mode->progressive)
+ div >>= 1;
+ if (div == 0)
+ div = 1;
+ mode->clock = clock / div;
/*
* tv_mode horizontal timings:
@@ -1135,6 +1141,8 @@ intel_tv_get_config(struct intel_encoder *encoder,
break;
default:
tv_mode.oversample = 1;
+ WARN_ON_ONCE(!tv_mode.progressive);
+ tv_mode.progressive = true;
break;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/tv: avoid possible division by zero
2023-07-25 5:51 ` Dan Carpenter
@ 2023-07-26 1:21 ` Su Hui
2023-07-26 4:58 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Su Hui @ 2023-07-26 1:21 UTC (permalink / raw)
To: Dan Carpenter
Cc: mripard, tvrtko.ursulin, kernel-janitors, llvm, trix, intel-gfx,
ndesaulniers, linux-kernel, nathan, andrzej.hajda, dri-devel,
Andi Shyti, rodrigo.vivi
On 2023/7/25 13:51, Dan Carpenter wrote:
> The reason why the first five attempts had bugs is because we are
> trying to write it in the most complicated way possible, shifting by
> logical not what?
Wonderful! Should I add your name as signed-of-by?
I will send a v3 patch later.
Really thanks for your help!
Su Hui
> regards,
> dan carpenter
>
> diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
> index 36b479b46b60..6997b6cb1df2 100644
> --- a/drivers/gpu/drm/i915/display/intel_tv.c
> +++ b/drivers/gpu/drm/i915/display/intel_tv.c
> @@ -988,7 +988,13 @@ intel_tv_mode_to_mode(struct drm_display_mode *mode,
> const struct tv_mode *tv_mode,
> int clock)
> {
> - mode->clock = clock / (tv_mode->oversample >> !tv_mode->progressive);
> + int div = tv_mode->oversample;
> +
> + if (!tv_mode->progressive)
> + div >>= 1;
> + if (div == 0)
> + div = 1;
> + mode->clock = clock / div;
>
> /*
> * tv_mode horizontal timings:
> @@ -1135,6 +1141,8 @@ intel_tv_get_config(struct intel_encoder *encoder,
> break;
> default:
> tv_mode.oversample = 1;
> + WARN_ON_ONCE(!tv_mode.progressive);
> + tv_mode.progressive = true;
> break;
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/tv: avoid possible division by zero
2023-07-26 1:21 ` Su Hui
@ 2023-07-26 4:58 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2023-07-26 4:58 UTC (permalink / raw)
To: Su Hui
Cc: mripard, tvrtko.ursulin, kernel-janitors, llvm, trix, intel-gfx,
ndesaulniers, linux-kernel, nathan, andrzej.hajda, dri-devel,
Andi Shyti, rodrigo.vivi
On Wed, Jul 26, 2023 at 09:21:50AM +0800, Su Hui wrote:
> On 2023/7/25 13:51, Dan Carpenter wrote:
> > The reason why the first five attempts had bugs is because we are
> > trying to write it in the most complicated way possible, shifting by
> > logical not what?
> Wonderful! Should I add your name as signed-of-by?
Sure. Or you can put it as a Suggested-by.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-07-26 4:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-18 1:32 [PATCH v2] drm/i915/tv: avoid possible division by zero Su Hui
2023-07-24 17:35 ` [Intel-gfx] " Andi Shyti
2023-07-25 4:06 ` Su Hui
2023-07-25 5:51 ` Dan Carpenter
2023-07-26 1:21 ` Su Hui
2023-07-26 4:58 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).