[Intel-gfx] [PATCH] drm/i915: Prevent unbounded wm results in g4x_compute_wm()

[Chris Wilson]

Quoting Chris Wilson (2017-11-07 12:56:23)
> Smatch warns of
> 
> drivers/gpu/drm/i915/intel_pm.c:1161 g4x_compute_wm() warn: signedness bug returning '(-33554430)'
> 
> which is a result of it believing that wm may be INT_MAX following
> g4x_tlb_miss_wa(). Just declaring g4x_tlb_miss_wa() as returning an
> unsigned integer is not sufficient, we need to tell smatch that wm itself
> is unsigned for it to not worry. So mark up the locals we expect to be
> non-negative, and so silence smatch.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Cc: Dan Carpenter <dan.carpenter at oracle.com>

Hi Dan, do you mind sharing any insight as to why smatch generates this
warning and whether the approach in this patch is incorrect?

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6e1358d4e764..403097606322 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -928,7 +928,7 @@ static void pineview_update_wm(struct intel_crtc *unused_crtc)
>   *  and the size of 8 whole lines. This adjustment is always performed
>   *  in the actual pixel depth regardless of whether FBC is enabled or not."
>   */
> -static int g4x_tlb_miss_wa(int fifo_size, int width, int cpp)
> +static unsigned int g4x_tlb_miss_wa(int fifo_size, int width, int cpp)
>  {
>         int tlb_miss = fifo_size * 64 - width * cpp * 8;
>  
> @@ -1105,8 +1105,8 @@ static uint16_t g4x_compute_wm(const struct intel_crtc_state *crtc_state,
>         struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>         const struct drm_display_mode *adjusted_mode =
>                 &crtc_state->base.adjusted_mode;
> -       int clock, htotal, cpp, width, wm;
> -       int latency = dev_priv->wm.pri_latency[level] * 10;
> +       unsigned int latency = dev_priv->wm.pri_latency[level] * 10;
> +       unsigned int clock, htotal, cpp, width, wm;
>  
>         if (latency == 0)
>                 return USHRT_MAX;
> @@ -1145,7 +1145,7 @@ static uint16_t g4x_compute_wm(const struct intel_crtc_state *crtc_state,
>                    level == G4X_WM_LEVEL_NORMAL) {
>                 wm = intel_wm_method1(clock, cpp, latency);
>         } else {
> -               int small, large;
> +               unsigned int small, large;
>  
>                 small = intel_wm_method1(clock, cpp, latency);
>                 large = intel_wm_method2(clock, htotal, width, cpp, latency);
> @@ -1158,7 +1158,7 @@ static uint16_t g4x_compute_wm(const struct intel_crtc_state *crtc_state,
>  
>         wm = DIV_ROUND_UP(wm, 64) + 2;
>  
> -       return min_t(int, wm, USHRT_MAX);
> +       return min_t(unsigned int, wm, USHRT_MAX);
>  }
>  
>  static bool g4x_raw_plane_wm_set(struct intel_crtc_state *crtc_state,
> -- 
> 2.15.0
>