Re: [PATCH] drm/i915: Fix erroneous conversion to u8

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Aug 08, 2014 at 06:34:48PM +0100, Damien Lespiau wrote:
> adj was defined as u8. The issue is last_adj can be negative and adj is
> initialized with:
> 
>   adj = dev_priv->rps.last_adj;
> 
> and we were also happily doing things like:
> 
>   if (adj < 0)
> 
> (thank static analysers!)
> 
> Cc: Deepak S <deepak.s@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 9fdf738..89e633f 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1330,7 +1330,8 @@ static u32 vlv_c0_residency(struct drm_i915_private *dev_priv,
>  static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
>  {
>  	u32 residency_C0_up = 0, residency_C0_down = 0;
> -	u8 new_delay, adj;
> +	u8 new_delay;
> +	int adj;

Might be better to make new_delay int too, so that we don't accidentally
under/overflow it due to the adj acceleration thing. And the return
value too. I'm feeling too lazy to think what kind of conditions that
would required, so just making it all int seems easier.

>  
>  	dev_priv->rps.ei_interrupt_count++;
>  
> -- 
> 1.8.3.1

-- 
Ville Syrjälä
Intel OTC