[Intel-gfx] [bug report] drm/i915/pmu: Use functions common with sysfs to read actual freq

[Intel-gfx] [bug report] drm/i915/pmu: Use functions common with sysfs to read actual freq

[Dan Carpenter]

Hello Ashutosh Dixit,

The patch 12d4eb20d9d8: "drm/i915/pmu: Use functions common with
sysfs to read actual freq" from Mar 15, 2023, leads to the following
Smatch static checker warning:

	drivers/gpu/drm/i915/gt/intel_rps.c:2110 __read_cagf()
	error: uninitialized symbol 'freq'.

drivers/gpu/drm/i915/gt/intel_rps.c
    2082 static u32 __read_cagf(struct intel_rps *rps, bool take_fw)
    2083 {
    2084         struct drm_i915_private *i915 = rps_to_i915(rps);
    2085         struct intel_uncore *uncore = rps_to_uncore(rps);
    2086         i915_reg_t r = INVALID_MMIO_REG;
    2087         u32 freq;
                 ^^^^^^^^

    2088 
    2089         /*
    2090          * For Gen12+ reading freq from HW does not need a forcewake and
    2091          * registers will return 0 freq when GT is in RC6
    2092          */
    2093         if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) {
    2094                 r = MTL_MIRROR_TARGET_WP1;
    2095         } else if (GRAPHICS_VER(i915) >= 12) {
    2096                 r = GEN12_RPSTAT1;
    2097         } else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) {
    2098                 vlv_punit_get(i915);
    2099                 freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
                         ^^^^^^^
Set here.

    2100                 vlv_punit_put(i915);
    2101         } else if (GRAPHICS_VER(i915) >= 6) {
    2102                 r = GEN6_RPSTAT1;
    2103         } else {
    2104                 r = MEMSTAT_ILK;
    2105         }
    2106 
    2107         if (i915_mmio_reg_valid(r))
    2108                 freq = take_fw ? intel_uncore_read(uncore, r) : intel_uncore_read_fw(uncore, r);
                         ^^^^^^
And here...

    2109 
--> 2110         return intel_rps_get_cagf(rps, freq);
                                                ^^^^
Warning.

    2111 }

regards,
dan carpenter

Re: [Intel-gfx] [bug report] drm/i915/pmu: Use functions common with sysfs to read actual freq

[Tvrtko Ursulin]

Hi Dan,

I feared this might happened while reviewing this patch.. I believe it 
is a false positive, see below:

On 29/03/2023 07:24, Dan Carpenter wrote:
> Hello Ashutosh Dixit,
> 
> The patch 12d4eb20d9d8: "drm/i915/pmu: Use functions common with
> sysfs to read actual freq" from Mar 15, 2023, leads to the following
> Smatch static checker warning:
> 
> 	drivers/gpu/drm/i915/gt/intel_rps.c:2110 __read_cagf()
> 	error: uninitialized symbol 'freq'.
> 
> drivers/gpu/drm/i915/gt/intel_rps.c
>      2082 static u32 __read_cagf(struct intel_rps *rps, bool take_fw)
>      2083 {
>      2084         struct drm_i915_private *i915 = rps_to_i915(rps);
>      2085         struct intel_uncore *uncore = rps_to_uncore(rps);
>      2086         i915_reg_t r = INVALID_MMIO_REG;
>      2087         u32 freq;
>                   ^^^^^^^^
> 
>      2088
>      2089         /*
>      2090          * For Gen12+ reading freq from HW does not need a forcewake and
>      2091          * registers will return 0 freq when GT is in RC6
>      2092          */
>      2093         if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) {
>      2094                 r = MTL_MIRROR_TARGET_WP1;
>      2095         } else if (GRAPHICS_VER(i915) >= 12) {
>      2096                 r = GEN12_RPSTAT1;
>      2097         } else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) {
>      2098                 vlv_punit_get(i915);
>      2099                 freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS);
>                           ^^^^^^^
> Set here.
> 
>      2100                 vlv_punit_put(i915);
>      2101         } else if (GRAPHICS_VER(i915) >= 6) {
>      2102                 r = GEN6_RPSTAT1;
>      2103         } else {
>      2104                 r = MEMSTAT_ILK;
>      2105         }

On all but the IS_VALLEVIEW branch, where freq itself is set, r is set 
instead...

>      2106
>      2107         if (i915_mmio_reg_valid(r))
>      2108                 freq = take_fw ? intel_uncore_read(uncore, r) : intel_uncore_read_fw(uncore, r);
>                           ^^^^^^
> And here...

...therefore freq will be initialized here in all cases.

> 
>      2109
> --> 2110         return intel_rps_get_cagf(rps, freq);
>                                                  ^^^^
> Warning.

So no problem here.

Regards,

Tvrtko

> 
>      2111 }
> 
> regards,
> dan carpenter