public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
@ 2015-11-18 12:13 Nabendu Maiti
  2015-11-18 12:15 ` Ville Syrjälä
  2015-11-18 12:19 ` Chris Wilson
  0 siblings, 2 replies; 13+ messages in thread
From: Nabendu Maiti @ 2015-11-18 12:13 UTC (permalink / raw)
  To: intel-gfx

Uninitialized variables (width, Height) in intel_check_sprite_plane
leads to compilererror in O1 level. Initialize all declared variables
to fix this issue.

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 2b96f33..a5af384 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -747,7 +747,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	struct drm_framebuffer *fb = state->base.fb;
 	int crtc_x, crtc_y;
 	unsigned int crtc_w, crtc_h;
-	uint32_t src_x, src_y, src_w, src_h;
+	uint32_t src_x = 0, src_y = 0, src_w = 0, src_h = 0;
 	struct drm_rect *src = &state->src;
 	struct drm_rect *dst = &state->dst;
 	const struct drm_rect *clip = &state->clip;
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
  2015-11-18 12:13 [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane Nabendu Maiti
@ 2015-11-18 12:15 ` Ville Syrjälä
  2015-11-18 12:19 ` Chris Wilson
  1 sibling, 0 replies; 13+ messages in thread
From: Ville Syrjälä @ 2015-11-18 12:15 UTC (permalink / raw)
  To: Nabendu Maiti; +Cc: intel-gfx

On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
> Uninitialized variables (width, Height) in intel_check_sprite_plane
> leads to compilererror in O1 level. Initialize all declared variables
> to fix this issue.
> 
> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 2b96f33..a5af384 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -747,7 +747,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
>  	struct drm_framebuffer *fb = state->base.fb;
>  	int crtc_x, crtc_y;
>  	unsigned int crtc_w, crtc_h;
> -	uint32_t src_x, src_y, src_w, src_h;
> +	uint32_t src_x = 0, src_y = 0, src_w = 0, src_h = 0;

Where exactly are these used without initialization?

>  	struct drm_rect *src = &state->src;
>  	struct drm_rect *dst = &state->dst;
>  	const struct drm_rect *clip = &state->clip;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
  2015-11-18 12:13 [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane Nabendu Maiti
  2015-11-18 12:15 ` Ville Syrjälä
@ 2015-11-18 12:19 ` Chris Wilson
  2015-11-18 12:44   ` Maiti, Nabendu Bikash
  2015-11-18 12:52   ` Ville Syrjälä
  1 sibling, 2 replies; 13+ messages in thread
From: Chris Wilson @ 2015-11-18 12:19 UTC (permalink / raw)
  To: Nabendu Maiti; +Cc: intel-gfx

On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
> Uninitialized variables (width, Height) in intel_check_sprite_plane
> leads to compilererror in O1 level. Initialize all declared variables
> to fix this issue.
> 
> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>

Or perhaps:

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 2b96f336589e..8d7b4eb5b5b9 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
        struct drm_framebuffer *fb = state->base.fb;
        int crtc_x, crtc_y;
        unsigned int crtc_w, crtc_h;
-       uint32_t src_x, src_y, src_w, src_h;
        struct drm_rect *src = &state->src;
        struct drm_rect *dst = &state->dst;
        const struct drm_rect *clip = &state->clip;
@@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
        crtc_h = drm_rect_height(dst);
 
        if (state->visible) {
+               u32 src_x, src_y, src_w, src_h;
+
                /* check again in case clipping clamped the results */
                hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
                if (hscale < 0) {
@@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
                        if (crtc_w == 0)
                                state->visible = false;
                }
-       }
 
                /* Check size restrictions when scaling */
-       if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
+               if (src_w != crtc_w || src_h != crtc_h) {
                        unsigned int width_bytes;
 
                        WARN_ON(!can_scale);
@@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
                        }
                }
 
-       if (state->visible) {
                src->x1 = src_x << 16;
                src->x2 = (src_x + src_w) << 16;
                src->y1 = src_y << 16;

And make both the compiler and reader happier
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
  2015-11-18 12:19 ` Chris Wilson
@ 2015-11-18 12:44   ` Maiti, Nabendu Bikash
  2015-11-18 12:52   ` Ville Syrjälä
  1 sibling, 0 replies; 13+ messages in thread
From: Maiti, Nabendu Bikash @ 2015-11-18 12:44 UTC (permalink / raw)
  To: Chris Wilson, ville.syrjala, intel-gfx

agreed, this is better



On 11/18/2015 5:49 PM, Chris Wilson wrote:
> On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
>> Uninitialized variables (width, Height) in intel_check_sprite_plane
>> leads to compilererror in O1 level. Initialize all declared variables
>> to fix this issue.
>>
>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> Or perhaps:
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 2b96f336589e..8d7b4eb5b5b9 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>          struct drm_framebuffer *fb = state->base.fb;
>          int crtc_x, crtc_y;
>          unsigned int crtc_w, crtc_h;
> -       uint32_t src_x, src_y, src_w, src_h;
>          struct drm_rect *src = &state->src;
>          struct drm_rect *dst = &state->dst;
>          const struct drm_rect *clip = &state->clip;
> @@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
>          crtc_h = drm_rect_height(dst);
>   
>          if (state->visible) {
> +               u32 src_x, src_y, src_w, src_h;
> +
>                  /* check again in case clipping clamped the results */
>                  hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
>                  if (hscale < 0) {
> @@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
>                          if (crtc_w == 0)
>                                  state->visible = false;
>                  }
> -       }
>   
>                  /* Check size restrictions when scaling */
> -       if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
> +               if (src_w != crtc_w || src_h != crtc_h) {
>                          unsigned int width_bytes;
>   
>                          WARN_ON(!can_scale);
> @@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>                          }
>                  }
>   
> -       if (state->visible) {
>                  src->x1 = src_x << 16;
>                  src->x2 = (src_x + src_w) << 16;
>                  src->y1 = src_y << 16;
>
> And make both the compiler and reader happier
> -Chris
>

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
  2015-11-18 12:19 ` Chris Wilson
  2015-11-18 12:44   ` Maiti, Nabendu Bikash
@ 2015-11-18 12:52   ` Ville Syrjälä
  2015-11-18 13:18     ` Maiti, Nabendu Bikash
  1 sibling, 1 reply; 13+ messages in thread
From: Ville Syrjälä @ 2015-11-18 12:52 UTC (permalink / raw)
  To: Chris Wilson, Nabendu Maiti, intel-gfx

On Wed, Nov 18, 2015 at 12:19:06PM +0000, Chris Wilson wrote:
> On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
> > Uninitialized variables (width, Height) in intel_check_sprite_plane
> > leads to compilererror in O1 level. Initialize all declared variables
> > to fix this issue.
> > 
> > Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> 
> Or perhaps:
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 2b96f336589e..8d7b4eb5b5b9 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>         struct drm_framebuffer *fb = state->base.fb;
>         int crtc_x, crtc_y;
>         unsigned int crtc_w, crtc_h;
> -       uint32_t src_x, src_y, src_w, src_h;
>         struct drm_rect *src = &state->src;
>         struct drm_rect *dst = &state->dst;
>         const struct drm_rect *clip = &state->clip;
> @@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
>         crtc_h = drm_rect_height(dst);
>  
>         if (state->visible) {
> +               u32 src_x, src_y, src_w, src_h;
> +
>                 /* check again in case clipping clamped the results */
>                 hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
>                 if (hscale < 0) {
> @@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
>                         if (crtc_w == 0)
>                                 state->visible = false;
>                 }
> -       }
>  
>                 /* Check size restrictions when scaling */
> -       if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
> +               if (src_w != crtc_w || src_h != crtc_h) {

That would change what it does.

>                         unsigned int width_bytes;
>  
>                         WARN_ON(!can_scale);
> @@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>                         }
>                 }
>  
> -       if (state->visible) {
>                 src->x1 = src_x << 16;
>                 src->x2 = (src_x + src_w) << 16;
>                 src->y1 = src_y << 16;
> 
> And make both the compiler and reader happier
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
  2015-11-18 12:52   ` Ville Syrjälä
@ 2015-11-18 13:18     ` Maiti, Nabendu Bikash
  2015-11-18 13:30       ` Ville Syrjälä
  0 siblings, 1 reply; 13+ messages in thread
From: Maiti, Nabendu Bikash @ 2015-11-18 13:18 UTC (permalink / raw)
  To: Ville Syrjälä, Chris Wilson, intel-gfx




On 11/18/2015 6:22 PM, Ville Syrjälä wrote:
> On Wed, Nov 18, 2015 at 12:19:06PM +0000, Chris Wilson wrote:
>> On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
>>> Uninitialized variables (width, Height) in intel_check_sprite_plane
>>> leads to compilererror in O1 level. Initialize all declared variables
>>> to fix this issue.
>>>
>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>> Or perhaps:
>>
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>> index 2b96f336589e..8d7b4eb5b5b9 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>>          struct drm_framebuffer *fb = state->base.fb;
>>          int crtc_x, crtc_y;
>>          unsigned int crtc_w, crtc_h;
>> -       uint32_t src_x, src_y, src_w, src_h;
>>          struct drm_rect *src = &state->src;
>>          struct drm_rect *dst = &state->dst;
>>          const struct drm_rect *clip = &state->clip;
>> @@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
>>          crtc_h = drm_rect_height(dst);
>>   
>>          if (state->visible) {
>> +               u32 src_x, src_y, src_w, src_h;
>> +
>>                  /* check again in case clipping clamped the results */
>>                  hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
>>                  if (hscale < 0) {
>> @@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
>>                          if (crtc_w == 0)
>>                                  state->visible = false;
>>                  }
>> -       }
>>   
>>                  /* Check size restrictions when scaling */
>> -       if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
>> +               if (src_w != crtc_w || src_h != crtc_h) {
> That would change what it does.
yes, checked the code where inside each if condition loop we may be 
changing the state->visible var itself. Next condition check it may be 
false too.

The place giving compiler error
                 src->x1 = src_x << 16;
                 src->x2 = (src_x + src_w) << 16;
                 src->y1 = src_y << 16;
                 src->y2 = (src_y + src_h) << 16;

Then just one line change of initializing the variables is better?

>>                          unsigned int width_bytes;
>>   
>>                          WARN_ON(!can_scale);
>> @@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>>                          }
>>                  }
>>   
>> -       if (state->visible) {
>>                  src->x1 = src_x << 16;
>>                  src->x2 = (src_x + src_w) << 16;
>>                  src->y1 = src_y << 16;
>>
>> And make both the compiler and reader happier
>> -Chris
>>
>> -- 
>> Chris Wilson, Intel Open Source Technology Centre
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
  2015-11-18 13:18     ` Maiti, Nabendu Bikash
@ 2015-11-18 13:30       ` Ville Syrjälä
  2015-11-18 17:03         ` Maiti, Nabendu Bikash
  0 siblings, 1 reply; 13+ messages in thread
From: Ville Syrjälä @ 2015-11-18 13:30 UTC (permalink / raw)
  To: Maiti, Nabendu Bikash; +Cc: intel-gfx

On Wed, Nov 18, 2015 at 06:48:37PM +0530, Maiti, Nabendu Bikash wrote:
> 
> 
> 
> On 11/18/2015 6:22 PM, Ville Syrjälä wrote:
> > On Wed, Nov 18, 2015 at 12:19:06PM +0000, Chris Wilson wrote:
> >> On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
> >>> Uninitialized variables (width, Height) in intel_check_sprite_plane
> >>> leads to compilererror in O1 level. Initialize all declared variables
> >>> to fix this issue.
> >>>
> >>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> >> Or perhaps:
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> >> index 2b96f336589e..8d7b4eb5b5b9 100644
> >> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >> @@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >>          struct drm_framebuffer *fb = state->base.fb;
> >>          int crtc_x, crtc_y;
> >>          unsigned int crtc_w, crtc_h;
> >> -       uint32_t src_x, src_y, src_w, src_h;
> >>          struct drm_rect *src = &state->src;
> >>          struct drm_rect *dst = &state->dst;
> >>          const struct drm_rect *clip = &state->clip;
> >> @@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >>          crtc_h = drm_rect_height(dst);
> >>   
> >>          if (state->visible) {
> >> +               u32 src_x, src_y, src_w, src_h;
> >> +
> >>                  /* check again in case clipping clamped the results */
> >>                  hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> >>                  if (hscale < 0) {
> >> @@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >>                          if (crtc_w == 0)
> >>                                  state->visible = false;
> >>                  }
> >> -       }
> >>   
> >>                  /* Check size restrictions when scaling */
> >> -       if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
> >> +               if (src_w != crtc_w || src_h != crtc_h) {
> > That would change what it does.
> yes, checked the code where inside each if condition loop we may be 
> changing the state->visible var itself. Next condition check it may be 
> false too.
> 
> The place giving compiler error
>                  src->x1 = src_x << 16;
>                  src->x2 = (src_x + src_w) << 16;
>                  src->y1 = src_y << 16;
>                  src->y2 = (src_y + src_h) << 16;
> 
> Then just one line change of initializing the variables is better?

Or maybe fix your compiler instead? I don't get any warning/errors from
this. What version of gcc are you using?

> 
> >>                          unsigned int width_bytes;
> >>   
> >>                          WARN_ON(!can_scale);
> >> @@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >>                          }
> >>                  }
> >>   
> >> -       if (state->visible) {
> >>                  src->x1 = src_x << 16;
> >>                  src->x2 = (src_x + src_w) << 16;
> >>                  src->y1 = src_y << 16;
> >>
> >> And make both the compiler and reader happier
> >> -Chris
> >>
> >> -- 
> >> Chris Wilson, Intel Open Source Technology Centre
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
  2015-11-18 13:30       ` Ville Syrjälä
@ 2015-11-18 17:03         ` Maiti, Nabendu Bikash
  2015-11-18 17:26           ` Ville Syrjälä
  0 siblings, 1 reply; 13+ messages in thread
From: Maiti, Nabendu Bikash @ 2015-11-18 17:03 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx



On 11/18/2015 7:00 PM, Ville Syrjälä wrote:
> On Wed, Nov 18, 2015 at 06:48:37PM +0530, Maiti, Nabendu Bikash wrote:
>>
>>
>> On 11/18/2015 6:22 PM, Ville Syrjälä wrote:
>>> On Wed, Nov 18, 2015 at 12:19:06PM +0000, Chris Wilson wrote:
>>>> On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
>>>>> Uninitialized variables (width, Height) in intel_check_sprite_plane
>>>>> leads to compilererror in O1 level. Initialize all declared variables
>>>>> to fix this issue.
>>>>>
>>>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>>>> Or perhaps:
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>>>> index 2b96f336589e..8d7b4eb5b5b9 100644
>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>>> @@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>>>>           struct drm_framebuffer *fb = state->base.fb;
>>>>           int crtc_x, crtc_y;
>>>>           unsigned int crtc_w, crtc_h;
>>>> -       uint32_t src_x, src_y, src_w, src_h;
>>>>           struct drm_rect *src = &state->src;
>>>>           struct drm_rect *dst = &state->dst;
>>>>           const struct drm_rect *clip = &state->clip;
>>>> @@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
>>>>           crtc_h = drm_rect_height(dst);
>>>>    
>>>>           if (state->visible) {
>>>> +               u32 src_x, src_y, src_w, src_h;
>>>> +
>>>>                   /* check again in case clipping clamped the results */
>>>>                   hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
>>>>                   if (hscale < 0) {
>>>> @@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
>>>>                           if (crtc_w == 0)
>>>>                                   state->visible = false;
>>>>                   }
>>>> -       }
>>>>    
>>>>                   /* Check size restrictions when scaling */
>>>> -       if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
>>>> +               if (src_w != crtc_w || src_h != crtc_h) {
>>> That would change what it does.
>> yes, checked the code where inside each if condition loop we may be
>> changing the state->visible var itself. Next condition check it may be
>> false too.
>>
>> The place giving compiler error
>>                   src->x1 = src_x << 16;
>>                   src->x2 = (src_x + src_w) << 16;
>>                   src->y1 = src_y << 16;
>>                   src->y2 = (src_y + src_h) << 16;
>>
>> Then just one line change of initializing the variables is better?
> Or maybe fix your compiler instead? I don't get any warning/errors from
> this. What version of gcc are you using?
  I still get the warning and error if -Werror is enabled, And on 
Makefile O1 optimization is enabled. (note on Android build by default 
Werror is enabled and gcc-- GCC: (GNU) 4.9.2
)

on generic build, gcc --gcc (Ubuntu 4.9.3-5ubuntu1~14.04) 4.9.3

following are the error..
   CC      drivers/staging/android/timed_gpio.o
/home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c: 
In function 'intel_check_sprite_plane':
/home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1241:20: 
error: 'src_h' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
    src->y2 = (src_y + src_h) << 16;
                     ^
/home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1239:20: 
error: 'src_w' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
    src->x2 = (src_x + src_w) << 16;
                     ^
/home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1241:20: 
error: 'src_y' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
    src->y2 = (src_y + src_h) << 16;
                     ^
/home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1238:19: 
error: 'src_x' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
    src->x1 = src_x << 16;
>
>>>>                           unsigned int width_bytes;
>>>>    
>>>>                           WARN_ON(!can_scale);
>>>> @@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>>>>                           }
>>>>                   }
>>>>    
>>>> -       if (state->visible) {
>>>>                   src->x1 = src_x << 16;
>>>>                   src->x2 = (src_x + src_w) << 16;
>>>>                   src->y1 = src_y << 16;
>>>>
>>>> And make both the compiler and reader happier
>>>> -Chris
>>>>
>>>> -- 
>>>> Chris Wilson, Intel Open Source Technology Centre
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
  2015-11-18 17:03         ` Maiti, Nabendu Bikash
@ 2015-11-18 17:26           ` Ville Syrjälä
  2015-11-26 18:03             ` Nabendu Maiti
  0 siblings, 1 reply; 13+ messages in thread
From: Ville Syrjälä @ 2015-11-18 17:26 UTC (permalink / raw)
  To: Maiti, Nabendu Bikash; +Cc: intel-gfx

On Wed, Nov 18, 2015 at 10:33:55PM +0530, Maiti, Nabendu Bikash wrote:
> 
> 
> On 11/18/2015 7:00 PM, Ville Syrjälä wrote:
> > On Wed, Nov 18, 2015 at 06:48:37PM +0530, Maiti, Nabendu Bikash wrote:
> >>
> >>
> >> On 11/18/2015 6:22 PM, Ville Syrjälä wrote:
> >>> On Wed, Nov 18, 2015 at 12:19:06PM +0000, Chris Wilson wrote:
> >>>> On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
> >>>>> Uninitialized variables (width, Height) in intel_check_sprite_plane
> >>>>> leads to compilererror in O1 level. Initialize all declared variables
> >>>>> to fix this issue.
> >>>>>
> >>>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> >>>> Or perhaps:
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> >>>> index 2b96f336589e..8d7b4eb5b5b9 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>>> @@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >>>>           struct drm_framebuffer *fb = state->base.fb;
> >>>>           int crtc_x, crtc_y;
> >>>>           unsigned int crtc_w, crtc_h;
> >>>> -       uint32_t src_x, src_y, src_w, src_h;
> >>>>           struct drm_rect *src = &state->src;
> >>>>           struct drm_rect *dst = &state->dst;
> >>>>           const struct drm_rect *clip = &state->clip;
> >>>> @@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >>>>           crtc_h = drm_rect_height(dst);
> >>>>    
> >>>>           if (state->visible) {
> >>>> +               u32 src_x, src_y, src_w, src_h;
> >>>> +
> >>>>                   /* check again in case clipping clamped the results */
> >>>>                   hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> >>>>                   if (hscale < 0) {
> >>>> @@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >>>>                           if (crtc_w == 0)
> >>>>                                   state->visible = false;
> >>>>                   }
> >>>> -       }
> >>>>    
> >>>>                   /* Check size restrictions when scaling */
> >>>> -       if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
> >>>> +               if (src_w != crtc_w || src_h != crtc_h) {
> >>> That would change what it does.
> >> yes, checked the code where inside each if condition loop we may be
> >> changing the state->visible var itself. Next condition check it may be
> >> false too.
> >>
> >> The place giving compiler error
> >>                   src->x1 = src_x << 16;
> >>                   src->x2 = (src_x + src_w) << 16;
> >>                   src->y1 = src_y << 16;
> >>                   src->y2 = (src_y + src_h) << 16;
> >>
> >> Then just one line change of initializing the variables is better?
> > Or maybe fix your compiler instead? I don't get any warning/errors from
> > this. What version of gcc are you using?
>   I still get the warning and error if -Werror is enabled, And on 
> Makefile O1 optimization is enabled.

And why exactly are you building with -O1?

> (note on Android build by default 
> Werror is enabled and gcc-- GCC: (GNU) 4.9.2
> )
> 
> on generic build, gcc --gcc (Ubuntu 4.9.3-5ubuntu1~14.04) 4.9.3
> 
> following are the error..
>    CC      drivers/staging/android/timed_gpio.o
> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c: 
> In function 'intel_check_sprite_plane':
> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1241:20: 
> error: 'src_h' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>     src->y2 = (src_y + src_h) << 16;
>                      ^
> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1239:20: 
> error: 'src_w' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>     src->x2 = (src_x + src_w) << 16;
>                      ^
> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1241:20: 
> error: 'src_y' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>     src->y2 = (src_y + src_h) << 16;
>                      ^
> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1238:19: 
> error: 'src_x' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>     src->x1 = src_x << 16;
> >
> >>>>                           unsigned int width_bytes;
> >>>>    
> >>>>                           WARN_ON(!can_scale);
> >>>> @@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >>>>                           }
> >>>>                   }
> >>>>    
> >>>> -       if (state->visible) {
> >>>>                   src->x1 = src_x << 16;
> >>>>                   src->x2 = (src_x + src_w) << 16;
> >>>>                   src->y1 = src_y << 16;
> >>>>
> >>>> And make both the compiler and reader happier
> >>>> -Chris
> >>>>
> >>>> -- 
> >>>> Chris Wilson, Intel Open Source Technology Centre
> >>>> _______________________________________________
> >>>> Intel-gfx mailing list
> >>>> Intel-gfx@lists.freedesktop.org
> >>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
  2015-11-18 17:26           ` Ville Syrjälä
@ 2015-11-26 18:03             ` Nabendu Maiti
  2015-12-07 18:17               ` Nabendu Maiti
  0 siblings, 1 reply; 13+ messages in thread
From: Nabendu Maiti @ 2015-11-26 18:03 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx



On 11/18/2015 10:56 PM, Ville Syrjälä wrote:
> On Wed, Nov 18, 2015 at 10:33:55PM +0530, Maiti, Nabendu Bikash wrote:
>>
>> On 11/18/2015 7:00 PM, Ville Syrjälä wrote:
>>> On Wed, Nov 18, 2015 at 06:48:37PM +0530, Maiti, Nabendu Bikash wrote:
>>>>
>>>> On 11/18/2015 6:22 PM, Ville Syrjälä wrote:
>>>>> On Wed, Nov 18, 2015 at 12:19:06PM +0000, Chris Wilson wrote:
>>>>>> On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
>>>>>>> Uninitialized variables (width, Height) in intel_check_sprite_plane
>>>>>>> leads to compilererror in O1 level. Initialize all declared variables
>>>>>>> to fix this issue.
>>>>>>>
>>>>>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>>>>>> Or perhaps:
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>>>>>> index 2b96f336589e..8d7b4eb5b5b9 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>>>>> @@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>>>>>>            struct drm_framebuffer *fb = state->base.fb;
>>>>>>            int crtc_x, crtc_y;
>>>>>>            unsigned int crtc_w, crtc_h;
>>>>>> -       uint32_t src_x, src_y, src_w, src_h;
>>>>>>            struct drm_rect *src = &state->src;
>>>>>>            struct drm_rect *dst = &state->dst;
>>>>>>            const struct drm_rect *clip = &state->clip;
>>>>>> @@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
>>>>>>            crtc_h = drm_rect_height(dst);
>>>>>>     
>>>>>>            if (state->visible) {
>>>>>> +               u32 src_x, src_y, src_w, src_h;
>>>>>> +
>>>>>>                    /* check again in case clipping clamped the results */
>>>>>>                    hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
>>>>>>                    if (hscale < 0) {
>>>>>> @@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
>>>>>>                            if (crtc_w == 0)
>>>>>>                                    state->visible = false;
>>>>>>                    }
>>>>>> -       }
>>>>>>     
>>>>>>                    /* Check size restrictions when scaling */
>>>>>> -       if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
>>>>>> +               if (src_w != crtc_w || src_h != crtc_h) {
>>>>> That would change what it does.
>>>> yes, checked the code where inside each if condition loop we may be
>>>> changing the state->visible var itself. Next condition check it may be
>>>> false too.
>>>>
>>>> The place giving compiler error
>>>>                    src->x1 = src_x << 16;
>>>>                    src->x2 = (src_x + src_w) << 16;
>>>>                    src->y1 = src_y << 16;
>>>>                    src->y2 = (src_y + src_h) << 16;
>>>>
>>>> Then just one line change of initializing the variables is better?
>>> Or maybe fix your compiler instead? I don't get any warning/errors from
>>> this. What version of gcc are you using?
>>    I still get the warning and error if -Werror is enabled, And on
>> Makefile O1 optimization is enabled.
> And why exactly are you building with -O1?
I am using it for platform level debug symbol inclusion in elf which are 
excluded in Os/O2 . We need it.Else it will break generic kernel.
>> (note on Android build by default
>> Werror is enabled and gcc-- GCC: (GNU) 4.9.2
>> )
>>
>> on generic build, gcc --gcc (Ubuntu 4.9.3-5ubuntu1~14.04) 4.9.3
>>
>> following are the error..
>>     CC      drivers/staging/android/timed_gpio.o
>> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:
>> In function 'intel_check_sprite_plane':
>> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1241:20:
>> error: 'src_h' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>      src->y2 = (src_y + src_h) << 16;
>>                       ^
>> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1239:20:
>> error: 'src_w' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>      src->x2 = (src_x + src_w) << 16;
>>                       ^
>> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1241:20:
>> error: 'src_y' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>      src->y2 = (src_y + src_h) << 16;
>>                       ^
>> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1238:19:
>> error: 'src_x' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>      src->x1 = src_x << 16;
>>>>>>                            unsigned int width_bytes;
>>>>>>     
>>>>>>                            WARN_ON(!can_scale);
>>>>>> @@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>>>>>>                            }
>>>>>>                    }
>>>>>>     
>>>>>> -       if (state->visible) {
>>>>>>                    src->x1 = src_x << 16;
>>>>>>                    src->x2 = (src_x + src_w) << 16;
>>>>>>                    src->y1 = src_y << 16;
>>>>>>
>>>>>> And make both the compiler and reader happier
>>>>>> -Chris
>>>>>>
>>>>>> -- 
>>>>>> Chris Wilson, Intel Open Source Technology Centre
>>>>>> _______________________________________________
>>>>>> Intel-gfx mailing list
>>>>>> Intel-gfx@lists.freedesktop.org
>>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
  2015-11-26 18:03             ` Nabendu Maiti
@ 2015-12-07 18:17               ` Nabendu Maiti
  2015-12-08 11:20                 ` Jani Nikula
  0 siblings, 1 reply; 13+ messages in thread
From: Nabendu Maiti @ 2015-12-07 18:17 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx



On 11/26/2015 11:33 PM, Nabendu Maiti wrote:
>
>
> On 11/18/2015 10:56 PM, Ville Syrjälä wrote:
>> On Wed, Nov 18, 2015 at 10:33:55PM +0530, Maiti, Nabendu Bikash wrote:
>>>
>>> On 11/18/2015 7:00 PM, Ville Syrjälä wrote:
>>>> On Wed, Nov 18, 2015 at 06:48:37PM +0530, Maiti, Nabendu Bikash wrote:
>>>>>
>>>>> On 11/18/2015 6:22 PM, Ville Syrjälä wrote:
>>>>>> On Wed, Nov 18, 2015 at 12:19:06PM +0000, Chris Wilson wrote:
>>>>>>> On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
>>>>>>>> Uninitialized variables (width, Height) in 
>>>>>>>> intel_check_sprite_plane
>>>>>>>> leads to compilererror in O1 level. Initialize all declared 
>>>>>>>> variables
>>>>>>>> to fix this issue.
>>>>>>>>
>>>>>>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>>>>>>> Or perhaps:
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
>>>>>>> b/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>> index 2b96f336589e..8d7b4eb5b5b9 100644
>>>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>> @@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane 
>>>>>>> *plane,
>>>>>>>            struct drm_framebuffer *fb = state->base.fb;
>>>>>>>            int crtc_x, crtc_y;
>>>>>>>            unsigned int crtc_w, crtc_h;
>>>>>>> -       uint32_t src_x, src_y, src_w, src_h;
>>>>>>>            struct drm_rect *src = &state->src;
>>>>>>>            struct drm_rect *dst = &state->dst;
>>>>>>>            const struct drm_rect *clip = &state->clip;
>>>>>>> @@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane 
>>>>>>> *plane,
>>>>>>>            crtc_h = drm_rect_height(dst);
>>>>>>>                if (state->visible) {
>>>>>>> +               u32 src_x, src_y, src_w, src_h;
>>>>>>> +
>>>>>>>                    /* check again in case clipping clamped the 
>>>>>>> results */
>>>>>>>                    hscale = drm_rect_calc_hscale(src, dst, 
>>>>>>> min_scale, max_scale);
>>>>>>>                    if (hscale < 0) {
>>>>>>> @@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane 
>>>>>>> *plane,
>>>>>>>                            if (crtc_w == 0)
>>>>>>>                                    state->visible = false;
>>>>>>>                    }
>>>>>>> -       }
>>>>>>>                        /* Check size restrictions when scaling */
>>>>>>> -       if (state->visible && (src_w != crtc_w || src_h != 
>>>>>>> crtc_h)) {
>>>>>>> +               if (src_w != crtc_w || src_h != crtc_h) {
>>>>>> That would change what it does.
>>>>> yes, checked the code where inside each if condition loop we may be
>>>>> changing the state->visible var itself. Next condition check it 
>>>>> may be
>>>>> false too.
>>>>>
>>>>> The place giving compiler error
>>>>>                    src->x1 = src_x << 16;
>>>>>                    src->x2 = (src_x + src_w) << 16;
>>>>>                    src->y1 = src_y << 16;
>>>>>                    src->y2 = (src_y + src_h) << 16;
>>>>>
>>>>> Then just one line change of initializing the variables is better?
>>>> Or maybe fix your compiler instead? I don't get any warning/errors 
>>>> from
>>>> this. What version of gcc are you using?
>>>    I still get the warning and error if -Werror is enabled, And on
>>> Makefile O1 optimization is enabled.
>> And why exactly are you building with -O1?
> I am using it for platform level debug symbol inclusion in elf which 
> are excluded in Os/O2 . We need it.Else it will break generic kernel.
Any comments? Please let me know if anything need to be done from my side.
>>> (note on Android build by default
>>> Werror is enabled and gcc-- GCC: (GNU) 4.9.2
>>> )
>>>
>>> on generic build, gcc --gcc (Ubuntu 4.9.3-5ubuntu1~14.04) 4.9.3
>>>
>>> following are the error..
>>>     CC      drivers/staging/android/timed_gpio.o
>>> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c: 
>>>
>>> In function 'intel_check_sprite_plane':
>>> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1241:20: 
>>>
>>> error: 'src_h' may be used uninitialized in this function
>>> [-Werror=maybe-uninitialized]
>>>      src->y2 = (src_y + src_h) << 16;
>>>                       ^
>>> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1239:20: 
>>>
>>> error: 'src_w' may be used uninitialized in this function
>>> [-Werror=maybe-uninitialized]
>>>      src->x2 = (src_x + src_w) << 16;
>>>                       ^
>>> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1241:20: 
>>>
>>> error: 'src_y' may be used uninitialized in this function
>>> [-Werror=maybe-uninitialized]
>>>      src->y2 = (src_y + src_h) << 16;
>>>                       ^
>>> /home/nbikash/PROJ_CODEBASE/MDsrt_WW44.2/kernel/bxt/drivers/gpu/drm/i915/intel_sprite.c:1238:19: 
>>>
>>> error: 'src_x' may be used uninitialized in this function
>>> [-Werror=maybe-uninitialized]
>>>      src->x1 = src_x << 16;
>>>>>>> unsigned int width_bytes;
>>>>>>>                                WARN_ON(!can_scale);
>>>>>>> @@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane 
>>>>>>> *plane,
>>>>>>>                            }
>>>>>>>                    }
>>>>>>>     -       if (state->visible) {
>>>>>>>                    src->x1 = src_x << 16;
>>>>>>>                    src->x2 = (src_x + src_w) << 16;
>>>>>>>                    src->y1 = src_y << 16;
>>>>>>>
>>>>>>> And make both the compiler and reader happier
>>>>>>> -Chris
>>>>>>>
>>>>>>> -- 
>>>>>>> Chris Wilson, Intel Open Source Technology Centre
>>>>>>> _______________________________________________
>>>>>>> Intel-gfx mailing list
>>>>>>> Intel-gfx@lists.freedesktop.org
>>>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
  2015-12-07 18:17               ` Nabendu Maiti
@ 2015-12-08 11:20                 ` Jani Nikula
  2015-12-10  8:42                   ` Daniel Vetter
  0 siblings, 1 reply; 13+ messages in thread
From: Jani Nikula @ 2015-12-08 11:20 UTC (permalink / raw)
  To: Nabendu Maiti, Ville Syrjälä; +Cc: intel-gfx

On Mon, 07 Dec 2015, Nabendu Maiti <nabendu.bikash.maiti@intel.com> wrote:
> On 11/26/2015 11:33 PM, Nabendu Maiti wrote:
>>
>>
>> On 11/18/2015 10:56 PM, Ville Syrjälä wrote:
>>> On Wed, Nov 18, 2015 at 10:33:55PM +0530, Maiti, Nabendu Bikash wrote:
>>>>
>>>> On 11/18/2015 7:00 PM, Ville Syrjälä wrote:
>>>>> On Wed, Nov 18, 2015 at 06:48:37PM +0530, Maiti, Nabendu Bikash wrote:
>>>>>> Then just one line change of initializing the variables is better?
>>>>> Or maybe fix your compiler instead? I don't get any warning/errors 
>>>>> from
>>>>> this. What version of gcc are you using?
>>>>    I still get the warning and error if -Werror is enabled, And on
>>>> Makefile O1 optimization is enabled.
>>> And why exactly are you building with -O1?
>> I am using it for platform level debug symbol inclusion in elf which 
>> are excluded in Os/O2 . We need it.Else it will break generic kernel.
> Any comments? Please let me know if anything need to be done from my side.

Nobody is doing anything about this because it's the compiler being
silly.

We could merge your patch (provided it said this was a compiler issue)
but frankly it wouldn't take long for someone to submit a patch to drop
the unnecessary initializations again.

We could probably rearrange the code to avoid warnings (e.g. use a local
temp variable for state->visible, or use "goto out" where state->visible
is set to false) but seems like a lot of churn just to make a silly tool
happy.

Your build is bound to produce a lot of false positives across the whole
kernel build. What do you do with those?


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
  2015-12-08 11:20                 ` Jani Nikula
@ 2015-12-10  8:42                   ` Daniel Vetter
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2015-12-10  8:42 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Tue, Dec 08, 2015 at 01:20:39PM +0200, Jani Nikula wrote:
> On Mon, 07 Dec 2015, Nabendu Maiti <nabendu.bikash.maiti@intel.com> wrote:
> > On 11/26/2015 11:33 PM, Nabendu Maiti wrote:
> >>
> >>
> >> On 11/18/2015 10:56 PM, Ville Syrjälä wrote:
> >>> On Wed, Nov 18, 2015 at 10:33:55PM +0530, Maiti, Nabendu Bikash wrote:
> >>>>
> >>>> On 11/18/2015 7:00 PM, Ville Syrjälä wrote:
> >>>>> On Wed, Nov 18, 2015 at 06:48:37PM +0530, Maiti, Nabendu Bikash wrote:
> >>>>>> Then just one line change of initializing the variables is better?
> >>>>> Or maybe fix your compiler instead? I don't get any warning/errors 
> >>>>> from
> >>>>> this. What version of gcc are you using?
> >>>>    I still get the warning and error if -Werror is enabled, And on
> >>>> Makefile O1 optimization is enabled.
> >>> And why exactly are you building with -O1?
> >> I am using it for platform level debug symbol inclusion in elf which 
> >> are excluded in Os/O2 . We need it.Else it will break generic kernel.
> > Any comments? Please let me know if anything need to be done from my side.
> 
> Nobody is doing anything about this because it's the compiler being
> silly.
> 
> We could merge your patch (provided it said this was a compiler issue)
> but frankly it wouldn't take long for someone to submit a patch to drop
> the unnecessary initializations again.
> 
> We could probably rearrange the code to avoid warnings (e.g. use a local
> temp variable for state->visible, or use "goto out" where state->visible
> is set to false) but seems like a lot of churn just to make a silly tool
> happy.
> 
> Your build is bound to produce a lot of false positives across the whole
> kernel build. What do you do with those?

Also I never had problem compiling with debug symbols. Well I only use
them to let gdb help me read binary code, but it works for that ;-)

What exactly are you trying to do here?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-12-10  8:42 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-18 12:13 [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane Nabendu Maiti
2015-11-18 12:15 ` Ville Syrjälä
2015-11-18 12:19 ` Chris Wilson
2015-11-18 12:44   ` Maiti, Nabendu Bikash
2015-11-18 12:52   ` Ville Syrjälä
2015-11-18 13:18     ` Maiti, Nabendu Bikash
2015-11-18 13:30       ` Ville Syrjälä
2015-11-18 17:03         ` Maiti, Nabendu Bikash
2015-11-18 17:26           ` Ville Syrjälä
2015-11-26 18:03             ` Nabendu Maiti
2015-12-07 18:17               ` Nabendu Maiti
2015-12-08 11:20                 ` Jani Nikula
2015-12-10  8:42                   ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox