Re: [Intel-gfx] [PATCH] drm/i915: Avoid divide by zero
On Wed, Jan 23, 2019 at 07:21:57AM +, Kahola, Mika wrote: > On Tue, 2019-01-22 at 21:09 +0200, Ville Syrjälä wrote: > > On Tue, Jan 22, 2019 at 08:09:40PM +0200, Jani Nikula wrote: > > > On Tue, 22 Jan 2019, Ville Syrjälä > > > wrote: > > > > On Tue, Jan 22, 2019 at 02:58:24PM +0200, Mika Kahola wrote: > > > > > Avoid divide by zero warning on static analysis. > > > > > > > > > > Signed-off-by: Mika Kahola > > > > > --- > > > > > drivers/gpu/drm/i915/intel_pm.c | 6 -- > > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > > b/drivers/gpu/drm/i915/intel_pm.c > > > > > index 8b63afa3a221..6a8e8b3f44c2 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > > @@ -3912,8 +3912,10 @@ > > > > > skl_ddb_get_pipe_allocation_limits(struct drm_i915_private > > > > > *dev_priv, > > > > > pipe_width = hdisplay; > > > > > } > > > > > > > > > > - alloc->start = ddb_size * width_before_pipe / > > > > > total_width; > > > > > - alloc->end = ddb_size * (width_before_pipe + > > > > > pipe_width) / total_width; > > > > > + alloc->start = total_width == 0 ? > > > > > + 0 : ddb_size * width_before_pipe / total_width; > > > > > + alloc->end = total_width == 0 ? > > > > > + 0 : ddb_size * (width_before_pipe + pipe_width) > > > > > / total_width; > > > > > > > > Can't happen. > I'd say it's very unlikely to happen but in theory possible. If we > don't have any crtc_state enabled we end up having total_width = 0. In > this case it probably won't matter what numbers we end up with alloc- > >start and alloc->end. The caller bails out early in that case: if (!cstate->base.active) { alloc->start = alloc->end = 0; return 0; } > > Anyway, this was something that caught my eye while looking into crc > mismatch errors and static checker results. > > Like Jani said, warn is sufficient with this case. > > Cheers, > Mika > > > > > > > Yeah, it's about stfu the checker... > > > > Feels like the tip of an iceberg. How many more uglies are we going > > to > > have to add? > > > > > > > > > > > > > > } > > > > > > > > > > static unsigned int skl_cursor_allocation(int num_active) > > > > > -- > > > > > 2.17.1 > > > > > > > > > > ___ > > > > > Intel-gfx mailing list > > > > > Intel-gfx@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > -- > > > Jani Nikula, Intel Open Source Graphics Center > > > > -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid divide by zero
On Tue, 2019-01-22 at 21:09 +0200, Ville Syrjälä wrote: > On Tue, Jan 22, 2019 at 08:09:40PM +0200, Jani Nikula wrote: > > On Tue, 22 Jan 2019, Ville Syrjälä > > wrote: > > > On Tue, Jan 22, 2019 at 02:58:24PM +0200, Mika Kahola wrote: > > > > Avoid divide by zero warning on static analysis. > > > > > > > > Signed-off-by: Mika Kahola > > > > --- > > > > drivers/gpu/drm/i915/intel_pm.c | 6 -- > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > b/drivers/gpu/drm/i915/intel_pm.c > > > > index 8b63afa3a221..6a8e8b3f44c2 100644 > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > @@ -3912,8 +3912,10 @@ > > > > skl_ddb_get_pipe_allocation_limits(struct drm_i915_private > > > > *dev_priv, > > > > pipe_width = hdisplay; > > > > } > > > > > > > > - alloc->start = ddb_size * width_before_pipe / > > > > total_width; > > > > - alloc->end = ddb_size * (width_before_pipe + > > > > pipe_width) / total_width; > > > > + alloc->start = total_width == 0 ? > > > > + 0 : ddb_size * width_before_pipe / total_width; > > > > + alloc->end = total_width == 0 ? > > > > + 0 : ddb_size * (width_before_pipe + pipe_width) > > > > / total_width; > > > > > > Can't happen. I'd say it's very unlikely to happen but in theory possible. If we don't have any crtc_state enabled we end up having total_width = 0. In this case it probably won't matter what numbers we end up with alloc- >start and alloc->end. Anyway, this was something that caught my eye while looking into crc mismatch errors and static checker results. Like Jani said, warn is sufficient with this case. Cheers, Mika > > > > Yeah, it's about stfu the checker... > > Feels like the tip of an iceberg. How many more uglies are we going > to > have to add? > > > > > > > > > > } > > > > > > > > static unsigned int skl_cursor_allocation(int num_active) > > > > -- > > > > 2.17.1 > > > > > > > > ___ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Jani Nikula, Intel Open Source Graphics Center > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid divide by zero
On Tue, Jan 22, 2019 at 08:09:40PM +0200, Jani Nikula wrote: > On Tue, 22 Jan 2019, Ville Syrjälä wrote: > > On Tue, Jan 22, 2019 at 02:58:24PM +0200, Mika Kahola wrote: > >> Avoid divide by zero warning on static analysis. > >> > >> Signed-off-by: Mika Kahola > >> --- > >> drivers/gpu/drm/i915/intel_pm.c | 6 -- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_pm.c > >> b/drivers/gpu/drm/i915/intel_pm.c > >> index 8b63afa3a221..6a8e8b3f44c2 100644 > >> --- a/drivers/gpu/drm/i915/intel_pm.c > >> +++ b/drivers/gpu/drm/i915/intel_pm.c > >> @@ -3912,8 +3912,10 @@ skl_ddb_get_pipe_allocation_limits(struct > >> drm_i915_private *dev_priv, > >>pipe_width = hdisplay; > >>} > >> > >> - alloc->start = ddb_size * width_before_pipe / total_width; > >> - alloc->end = ddb_size * (width_before_pipe + pipe_width) / total_width; > >> + alloc->start = total_width == 0 ? > >> + 0 : ddb_size * width_before_pipe / total_width; > >> + alloc->end = total_width == 0 ? > >> + 0 : ddb_size * (width_before_pipe + pipe_width) / total_width; > > > > Can't happen. > > Yeah, it's about stfu the checker... Feels like the tip of an iceberg. How many more uglies are we going to have to add? > > > > >> } > >> > >> static unsigned int skl_cursor_allocation(int num_active) > >> -- > >> 2.17.1 > >> > >> ___ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Graphics Center -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid divide by zero
On Tue, 22 Jan 2019, Ville Syrjälä wrote: > On Tue, Jan 22, 2019 at 02:58:24PM +0200, Mika Kahola wrote: >> Avoid divide by zero warning on static analysis. >> >> Signed-off-by: Mika Kahola >> --- >> drivers/gpu/drm/i915/intel_pm.c | 6 -- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c >> b/drivers/gpu/drm/i915/intel_pm.c >> index 8b63afa3a221..6a8e8b3f44c2 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -3912,8 +3912,10 @@ skl_ddb_get_pipe_allocation_limits(struct >> drm_i915_private *dev_priv, >> pipe_width = hdisplay; >> } >> >> -alloc->start = ddb_size * width_before_pipe / total_width; >> -alloc->end = ddb_size * (width_before_pipe + pipe_width) / total_width; >> +alloc->start = total_width == 0 ? >> +0 : ddb_size * width_before_pipe / total_width; >> +alloc->end = total_width == 0 ? >> +0 : ddb_size * (width_before_pipe + pipe_width) / total_width; > > Can't happen. Yeah, it's about stfu the checker... > >> } >> >> static unsigned int skl_cursor_allocation(int num_active) >> -- >> 2.17.1 >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid divide by zero
On Tue, Jan 22, 2019 at 02:58:24PM +0200, Mika Kahola wrote: > Avoid divide by zero warning on static analysis. > > Signed-off-by: Mika Kahola > --- > drivers/gpu/drm/i915/intel_pm.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 8b63afa3a221..6a8e8b3f44c2 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3912,8 +3912,10 @@ skl_ddb_get_pipe_allocation_limits(struct > drm_i915_private *dev_priv, > pipe_width = hdisplay; > } > > - alloc->start = ddb_size * width_before_pipe / total_width; > - alloc->end = ddb_size * (width_before_pipe + pipe_width) / total_width; > + alloc->start = total_width == 0 ? > + 0 : ddb_size * width_before_pipe / total_width; > + alloc->end = total_width == 0 ? > + 0 : ddb_size * (width_before_pipe + pipe_width) / total_width; Can't happen. > } > > static unsigned int skl_cursor_allocation(int num_active) > -- > 2.17.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid divide by zero
On Tue, 2019-01-22 at 15:07 +0200, Jani Nikula wrote: > On Tue, 22 Jan 2019, Mika Kahola wrote: > > Avoid divide by zero warning on static analysis. > > > > Signed-off-by: Mika Kahola > > --- > > drivers/gpu/drm/i915/intel_pm.c | 6 -- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c > > index 8b63afa3a221..6a8e8b3f44c2 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -3912,8 +3912,10 @@ skl_ddb_get_pipe_allocation_limits(struct > > drm_i915_private *dev_priv, > > pipe_width = hdisplay; > > } > > if (WARN_ON(total_width == 0)) > return; Warning might be enough here. Switching to use this in this patch. Thanks for the review! Cheers, Mika > > ? > > BR, > Jani. > > > > > > - alloc->start = ddb_size * width_before_pipe / total_width; > > - alloc->end = ddb_size * (width_before_pipe + pipe_width) / > > total_width; > > + alloc->start = total_width == 0 ? > > + 0 : ddb_size * width_before_pipe / total_width; > > + alloc->end = total_width == 0 ? > > + 0 : ddb_size * (width_before_pipe + pipe_width) / > > total_width; > > } > > > > static unsigned int skl_cursor_allocation(int num_active) > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid divide by zero
On Tue, 22 Jan 2019, Mika Kahola wrote: > Avoid divide by zero warning on static analysis. > > Signed-off-by: Mika Kahola > --- > drivers/gpu/drm/i915/intel_pm.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 8b63afa3a221..6a8e8b3f44c2 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3912,8 +3912,10 @@ skl_ddb_get_pipe_allocation_limits(struct > drm_i915_private *dev_priv, > pipe_width = hdisplay; > } if (WARN_ON(total_width == 0)) return; ? BR, Jani. > > - alloc->start = ddb_size * width_before_pipe / total_width; > - alloc->end = ddb_size * (width_before_pipe + pipe_width) / total_width; > + alloc->start = total_width == 0 ? > + 0 : ddb_size * width_before_pipe / total_width; > + alloc->end = total_width == 0 ? > + 0 : ddb_size * (width_before_pipe + pipe_width) / total_width; > } > > static unsigned int skl_cursor_allocation(int num_active) -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Avoid divide by zero
Avoid divide by zero warning on static analysis. Signed-off-by: Mika Kahola --- drivers/gpu/drm/i915/intel_pm.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8b63afa3a221..6a8e8b3f44c2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3912,8 +3912,10 @@ skl_ddb_get_pipe_allocation_limits(struct drm_i915_private *dev_priv, pipe_width = hdisplay; } - alloc->start = ddb_size * width_before_pipe / total_width; - alloc->end = ddb_size * (width_before_pipe + pipe_width) / total_width; + alloc->start = total_width == 0 ? + 0 : ddb_size * width_before_pipe / total_width; + alloc->end = total_width == 0 ? + 0 : ddb_size * (width_before_pipe + pipe_width) / total_width; } static unsigned int skl_cursor_allocation(int num_active) -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx