[Intel-gfx] [PATCH] drm/i915: Fix erroneous conversion to u8

2014-08-08 Thread Damien Lespiau 
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 deepa...@linux.intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@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;
 
dev_priv-rps.ei_interrupt_count++;
 
-- 
1.8.3.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2014-08-08 Thread Ville Syrjälä 
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 deepa...@linux.intel.com
 Cc: Ville Syrjälä ville.syrj...@linux.intel.com
 Signed-off-by: Damien Lespiau damien.lesp...@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Fix erroneous conversion to u8

2014-08-08 Thread Damien Lespiau 
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!)

v2: Make new_delay an int in case we overflow the u8 in the intermediate
computations. new_delay will get clamped at the end anyway. (Ville)

Cc: Deepak S deepa...@linux.intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/i915/i915_irq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9fdf738..8e6729e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1327,10 +1327,10 @@ static u32 vlv_c0_residency(struct drm_i915_private 
*dev_priv,
  * @dev_priv: DRM device private
  *
  */
-static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
+static int 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;
+   int new_delay, adj;
 
dev_priv-rps.ei_interrupt_count++;
 
-- 
1.8.3.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2014-08-08 Thread Ville Syrjälä 
On Fri, Aug 08, 2014 at 07:25:57PM +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!)
 
 v2: Make new_delay an int in case we overflow the u8 in the intermediate
 computations. new_delay will get clamped at the end anyway. (Ville)
 
 Cc: Deepak S deepa...@linux.intel.com
 Cc: Ville Syrjälä ville.syrj...@linux.intel.com
 Signed-off-by: Damien Lespiau damien.lesp...@intel.com

Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

 ---
  drivers/gpu/drm/i915/i915_irq.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index 9fdf738..8e6729e 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -1327,10 +1327,10 @@ static u32 vlv_c0_residency(struct drm_i915_private 
 *dev_priv,
   * @dev_priv: DRM device private
   *
   */
 -static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
 +static int 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;
 + int new_delay, adj;
  
   dev_priv-rps.ei_interrupt_count++;
  
 -- 
 1.8.3.1

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2014-08-08 Thread Daniel Vetter 
On Fri, Aug 08, 2014 at 09:32:20PM +0300, Ville Syrjälä wrote:
 On Fri, Aug 08, 2014 at 07:25:57PM +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!)
  
  v2: Make new_delay an int in case we overflow the u8 in the intermediate
  computations. new_delay will get clamped at the end anyway. (Ville)
  
  Cc: Deepak S deepa...@linux.intel.com
  Cc: Ville Syrjälä ville.syrj...@linux.intel.com
  Signed-off-by: Damien Lespiau damien.lesp...@intel.com
 
 Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

Queued for -next, thanks for the patch.
-Daniel

 
  ---
   drivers/gpu/drm/i915/i915_irq.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_irq.c 
  b/drivers/gpu/drm/i915/i915_irq.c
  index 9fdf738..8e6729e 100644
  --- a/drivers/gpu/drm/i915/i915_irq.c
  +++ b/drivers/gpu/drm/i915/i915_irq.c
  @@ -1327,10 +1327,10 @@ static u32 vlv_c0_residency(struct drm_i915_private 
  *dev_priv,
* @dev_priv: DRM device private
*
*/
  -static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private 
  *dev_priv)
  +static int 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;
  +   int new_delay, adj;
   
  dev_priv-rps.ei_interrupt_count++;
   
  -- 
  1.8.3.1
 
 -- 
 Ville Syrjälä
 Intel OTC
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx