* Re: [PATCH linux-next] drm/i915/display: Remove unused variable in the for loop.
2021-10-18 8:44 [PATCH linux-next] drm/i915/display: Remove unused variable in the for loop luo penghao
@ 2021-10-18 9:11 ` Jani Nikula
2021-10-18 12:14 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: Jani Nikula @ 2021-10-18 9:11 UTC (permalink / raw)
To: luo penghao
Cc: Joonas Lahtinen, Rodrigo Vivi, David Airlie, Daniel Vetter,
Stephen Rothwell, intel-gfx, dri-devel, linux-kernel, luo penghao,
Zeal Robot, Deak, Imre
On Mon, 18 Oct 2021, luo penghao <cgel.zte@gmail.com> wrote:
> Variable is not used in the loop, and its assignment is redundant too.
> So it should be deleted.
>
> The clang_analyzer complains as follows:
>
> drivers/gpu/drm/i915/display/intel_fb.c:1018:3 warning:
>
> Value stored to 'cpp' is never read.
>
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: luo penghao <luo.penghao@zte.com.cn>
> ---
> drivers/gpu/drm/i915/display/intel_fb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
> index fa1f375..b9b6a7a 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> @@ -998,7 +998,7 @@ int intel_fill_fb_info(struct drm_i915_private *i915, struct intel_framebuffer *
> for (i = 0; i < num_planes; i++) {
> struct fb_plane_view_dims view_dims;
> unsigned int width, height;
> - unsigned int cpp, size;
> + unsigned int size;
> u32 offset;
> int x, y;
> int ret;
> @@ -1015,7 +1015,7 @@ int intel_fill_fb_info(struct drm_i915_private *i915, struct intel_framebuffer *
> return -EINVAL;
> }
>
> - cpp = fb->base.format->cpp[i];
> + fb->base.format->cpp[i];
Thanks for the report. However, this "fix" isn't any better than having
the unused variable. It's obviously wrong.
It would be useful to dig into the history of the function, and figure
out when and why the variable became unused, and whether that caused an
actual bug or whether this was just leftovers from some refactoring.
So that's what I did. Some git blame and git log -p revealed commit
d3c5e10b6059 ("drm/i915/intel_fb: Factor out
convert_plane_offset_to_xy()") that moved the check that used the cpp
variable to a separate function, and the local variable and the line
above became unused and useless.
That's the actually helpful part. It's easy to see and verify that the
right fix is to just remove the line completely.
BR,
Jani.
> intel_fb_plane_dims(fb, i, &width, &height);
>
> ret = convert_plane_offset_to_xy(fb, i, width, &x, &y);
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH linux-next] drm/i915/display: Remove unused variable in the for loop.
2021-10-18 8:44 [PATCH linux-next] drm/i915/display: Remove unused variable in the for loop luo penghao
2021-10-18 9:11 ` Jani Nikula
@ 2021-10-18 12:14 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2021-10-18 12:14 UTC (permalink / raw)
To: luo penghao, Jani Nikula
Cc: llvm, kbuild-all, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, intel-gfx, dri-devel, linux-kernel, luo penghao,
Zeal Robot
[-- Attachment #1: Type: text/plain, Size: 4795 bytes --]
Hi luo,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on next-20211015]
url: https://github.com/0day-ci/linux/commits/luo-penghao/drm-i915-display-Remove-unused-variable-in-the-for-loop/20211018-164557
base: 7c832d2f9b959e3181370c8b0dacaf9efe13fc05
config: x86_64-randconfig-r014-20211018 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d245f2e8597bfb52c34810a328d42b990e4af1a4)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/1a3c67fc7e245a1130e2b59fe5a04ce82de697c0
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review luo-penghao/drm-i915-display-Remove-unused-variable-in-the-for-loop/20211018-164557
git checkout 1a3c67fc7e245a1130e2b59fe5a04ce82de697c0
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/i915/display/intel_fb.c:1018:25: error: expression result unused [-Werror,-Wunused-value]
fb->base.format->cpp[i];
~~~~~~~~~~~~~~~~~~~~ ~^
1 error generated.
vim +1018 drivers/gpu/drm/i915/display/intel_fb.c
977
978 int intel_fill_fb_info(struct drm_i915_private *i915, struct intel_framebuffer *fb)
979 {
980 struct drm_i915_gem_object *obj = intel_fb_obj(&fb->base);
981 u32 gtt_offset_rotated = 0;
982 u32 gtt_offset_remapped = 0;
983 unsigned int max_size = 0;
984 int i, num_planes = fb->base.format->num_planes;
985 unsigned int tile_size = intel_tile_size(i915);
986
987 intel_fb_view_init(i915, &fb->normal_view, I915_GGTT_VIEW_NORMAL);
988
989 drm_WARN_ON(&i915->drm,
990 intel_fb_supports_90_270_rotation(fb) &&
991 intel_fb_needs_pot_stride_remap(fb));
992
993 if (intel_fb_supports_90_270_rotation(fb))
994 intel_fb_view_init(i915, &fb->rotated_view, I915_GGTT_VIEW_ROTATED);
995 if (intel_fb_needs_pot_stride_remap(fb))
996 intel_fb_view_init(i915, &fb->remapped_view, I915_GGTT_VIEW_REMAPPED);
997
998 for (i = 0; i < num_planes; i++) {
999 struct fb_plane_view_dims view_dims;
1000 unsigned int width, height;
1001 unsigned int size;
1002 u32 offset;
1003 int x, y;
1004 int ret;
1005
1006 /*
1007 * Plane 2 of Render Compression with Clear Color fb modifier
1008 * is consumed by the driver and not passed to DE. Skip the
1009 * arithmetic related to alignment and offset calculation.
1010 */
1011 if (is_gen12_ccs_cc_plane(&fb->base, i)) {
1012 if (IS_ALIGNED(fb->base.offsets[i], PAGE_SIZE))
1013 continue;
1014 else
1015 return -EINVAL;
1016 }
1017
> 1018 fb->base.format->cpp[i];
1019 intel_fb_plane_dims(fb, i, &width, &height);
1020
1021 ret = convert_plane_offset_to_xy(fb, i, width, &x, &y);
1022 if (ret)
1023 return ret;
1024
1025 init_plane_view_dims(fb, i, width, height, &view_dims);
1026
1027 /*
1028 * First pixel of the framebuffer from
1029 * the start of the normal gtt mapping.
1030 */
1031 fb->normal_view.color_plane[i].x = x;
1032 fb->normal_view.color_plane[i].y = y;
1033 fb->normal_view.color_plane[i].stride = fb->base.pitches[i];
1034
1035 offset = calc_plane_aligned_offset(fb, i, &x, &y);
1036
1037 if (intel_fb_supports_90_270_rotation(fb))
1038 gtt_offset_rotated += calc_plane_remap_info(fb, i, &view_dims,
1039 offset, gtt_offset_rotated, x, y,
1040 &fb->rotated_view);
1041
1042 if (intel_fb_needs_pot_stride_remap(fb))
1043 gtt_offset_remapped += calc_plane_remap_info(fb, i, &view_dims,
1044 offset, gtt_offset_remapped, x, y,
1045 &fb->remapped_view);
1046
1047 size = calc_plane_normal_size(fb, i, &view_dims, x, y);
1048 /* how many tiles in total needed in the bo */
1049 max_size = max(max_size, offset + size);
1050 }
1051
1052 if (mul_u32_u32(max_size, tile_size) > obj->base.size) {
1053 drm_dbg_kms(&i915->drm,
1054 "fb too big for bo (need %llu bytes, have %zu bytes)\n",
1055 mul_u32_u32(max_size, tile_size), obj->base.size);
1056 return -EINVAL;
1057 }
1058
1059 return 0;
1060 }
1061
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32668 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread