From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
Subject: Re: [PATCH] drm/i915: Fix erroneous conversion to u8
Date: Fri, 8 Aug 2014 20:56:23 +0300
Message-ID: <20140808175623.GH4193@intel.com>
References: <1407519288-13181-1-git-send-email-damien.lespiau@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Return-path: <intel-gfx-bounces@lists.freedesktop.org>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
 by gabe.freedesktop.org (Postfix) with ESMTP id C41676E1AD
 for <intel-gfx@lists.freedesktop.org>; Fri,  8 Aug 2014 10:58:59 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <1407519288-13181-1-git-send-email-damien.lespiau@intel.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
 <mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
 <mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>
To: Damien Lespiau <damien.lespiau@intel.com>
Cc: intel-gfx@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.org

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 =3D 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=E4l=E4 <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 =3D 0, residency_C0_down =3D 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=E4l=E4
Intel OTC