[Intel-gfx] [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane

[Nabendu Maiti]

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 at 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 at lists.freedesktop.org
>>>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx