| Message ID | 20201022050148.27105-2-abhinavk@codeaurora.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add devcoredump support for DPU | expand |
Hi Abhinav,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-exynos/exynos-drm-next]
[also build test WARNING on drm-intel/for-linux-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.9 next-20201022]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Abhinav-Kumar/Add-devcoredump-support-for-DPU/20201022-130507
base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
config: microblaze-randconfig-r003-20201022 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
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/76add8f28420cad0b0d2977abed6e031ef307f32
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Abhinav-Kumar/Add-devcoredump-support-for-DPU/20201022-130507
git checkout 76add8f28420cad0b0d2977abed6e031ef307f32
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=microblaze
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/selftests/test-drm_framebuffer.c:12:
>> drivers/gpu/drm/selftests/../drm_crtc_internal.h:238:10: warning: 'struct drm_printer' declared inside parameter list will not be visible outside of this definition or declaration
238 | struct drm_printer *p);
| ^~~~~~~~~~~
drivers/gpu/drm/selftests/test-drm_framebuffer.c: In function 'execute_drm_mode_fb_cmd2':
drivers/gpu/drm/selftests/test-drm_framebuffer.c:333:26: warning: variable 'fb' set but not used [-Wunused-but-set-variable]
333 | struct drm_framebuffer *fb;
| ^~
vim +238 drivers/gpu/drm/selftests/../drm_crtc_internal.h
231
232 int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
233 struct drm_plane_state *plane_state);
234 int __drm_atomic_helper_set_config(struct drm_mode_set *set,
235 struct drm_atomic_state *state);
236
237 void drm_atomic_print_state(const struct drm_atomic_state *state,
> 238 struct drm_printer *p);
239
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Wed, Oct 21, 2020 at 10:01:45PM -0700, Abhinav Kumar wrote: > Currently drm_atomic_print_state() internally allocates and uses a > drm_info printer. Allow it to accept any drm_printer type so that > the API can be leveraged even for taking drm snapshot. > > Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> > --- > drivers/gpu/drm/drm_atomic.c | 17 ++++++++++++----- > drivers/gpu/drm/drm_atomic_uapi.c | 4 +++- > drivers/gpu/drm/drm_crtc_internal.h | 4 +++- > 3 files changed, 18 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 58527f151984..e7079a5f439c 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1,6 +1,7 @@ > /* > * Copyright (C) 2014 Red Hat > * Copyright (C) 2014 Intel Corp. > + * Copyright (c) 2020, The Linux Foundation. All rights reserved. > * > * Permission is hereby granted, free of charge, to any person obtaining a > * copy of this software and associated documentation files (the "Software"), > @@ -1543,9 +1544,9 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, > } > EXPORT_SYMBOL(__drm_atomic_helper_set_config); > > -void drm_atomic_print_state(const struct drm_atomic_state *state) > +void drm_atomic_print_state(const struct drm_atomic_state *state, > + struct drm_printer *p) Please add a nice kerneldoc for this newly exported function. Specifically this kerneldoc needs to include a warning that state updates after call drm_atomic_state_helper_commit_hw_done() is unsafe to print using this function, because it looks at the new state objects. Only the old state structures will stay like this. So maybe rename the function to say print_new_state() to make this completely clear. That way we can eventually add a print_old_state() when needed. Otherwise I think this makes sense, and nicely avoids the locking issue of looking at ->state pointers without the right locking. -Daniel > { > - struct drm_printer p = drm_info_printer(state->dev->dev); > struct drm_plane *plane; > struct drm_plane_state *plane_state; > struct drm_crtc *crtc; > @@ -1554,17 +1555,23 @@ void drm_atomic_print_state(const struct drm_atomic_state *state) > struct drm_connector_state *connector_state; > int i; > > + if (!p) { > + DRM_ERROR("invalid drm printer\n"); > + return; > + } > + > DRM_DEBUG_ATOMIC("checking %p\n", state); > > for_each_new_plane_in_state(state, plane, plane_state, i) > - drm_atomic_plane_print_state(&p, plane_state); > + drm_atomic_plane_print_state(p, plane_state); > > for_each_new_crtc_in_state(state, crtc, crtc_state, i) > - drm_atomic_crtc_print_state(&p, crtc_state); > + drm_atomic_crtc_print_state(p, crtc_state); > > for_each_new_connector_in_state(state, connector, connector_state, i) > - drm_atomic_connector_print_state(&p, connector_state); > + drm_atomic_connector_print_state(p, connector_state); > } > +EXPORT_SYMBOL(drm_atomic_print_state); > > static void __drm_state_dump(struct drm_device *dev, struct drm_printer *p, > bool take_locks) > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > index 25c269bc4681..d9ae86c92608 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -2,6 +2,7 @@ > * Copyright (C) 2014 Red Hat > * Copyright (C) 2014 Intel Corp. > * Copyright (C) 2018 Intel Corp. > + * Copyright (c) 2020, The Linux Foundation. All rights reserved. > * > * Permission is hereby granted, free of charge, to any person obtaining a > * copy of this software and associated documentation files (the "Software"), > @@ -1294,6 +1295,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, > struct drm_out_fence_state *fence_state; > int ret = 0; > unsigned int i, j, num_fences; > + struct drm_printer p = drm_info_printer(dev->dev); > > /* disallow for drivers not supporting atomic: */ > if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) > @@ -1413,7 +1415,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, > ret = drm_atomic_nonblocking_commit(state); > } else { > if (drm_debug_enabled(DRM_UT_STATE)) > - drm_atomic_print_state(state); > + drm_atomic_print_state(state, &p); > > ret = drm_atomic_commit(state); > } > diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h > index da96b2f64d7e..d34215366936 100644 > --- a/drivers/gpu/drm/drm_crtc_internal.h > +++ b/drivers/gpu/drm/drm_crtc_internal.h > @@ -5,6 +5,7 @@ > * Jesse Barnes <jesse.barnes@intel.com> > * Copyright © 2014 Intel Corporation > * Daniel Vetter <daniel.vetter@ffwll.ch> > + * Copyright (c) 2020, The Linux Foundation. All rights reserved. > * > * Permission is hereby granted, free of charge, to any person obtaining a > * copy of this software and associated documentation files (the "Software"), > @@ -233,7 +234,8 @@ int __drm_atomic_helper_disable_plane(struct drm_plane *plane, > int __drm_atomic_helper_set_config(struct drm_mode_set *set, > struct drm_atomic_state *state); > > -void drm_atomic_print_state(const struct drm_atomic_state *state); > +void drm_atomic_print_state(const struct drm_atomic_state *state, > + struct drm_printer *p); > > /* drm_atomic_uapi.c */ > int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state, > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Abhinav,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-exynos/exynos-drm-next]
[also build test WARNING on drm-intel/for-linux-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.10-rc1 next-20201026]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Abhinav-Kumar/Add-devcoredump-support-for-DPU/20201022-130507
base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
config: mips-randconfig-r014-20201026 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project f2c25c70791de95d2466e09b5b58fc37f6ccd7a4)
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
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/0day-ci/linux/commit/76add8f28420cad0b0d2977abed6e031ef307f32
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Abhinav-Kumar/Add-devcoredump-support-for-DPU/20201022-130507
git checkout 76add8f28420cad0b0d2977abed6e031ef307f32
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/selftests/test-drm_framebuffer.c:12:
>> drivers/gpu/drm/selftests/../drm_crtc_internal.h:238:10: warning: declaration of 'struct drm_printer' will not be visible outside of this function [-Wvisibility]
struct drm_printer *p);
^
1 warning generated.
vim +238 drivers/gpu/drm/selftests/../drm_crtc_internal.h
231
232 int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
233 struct drm_plane_state *plane_state);
234 int __drm_atomic_helper_set_config(struct drm_mode_set *set,
235 struct drm_atomic_state *state);
236
237 void drm_atomic_print_state(const struct drm_atomic_state *state,
> 238 struct drm_printer *p);
239
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Daniel On 2020-10-22 03:38, Daniel Vetter wrote: > On Wed, Oct 21, 2020 at 10:01:45PM -0700, Abhinav Kumar wrote: >> Currently drm_atomic_print_state() internally allocates and uses a >> drm_info printer. Allow it to accept any drm_printer type so that >> the API can be leveraged even for taking drm snapshot. >> >> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> >> --- >> drivers/gpu/drm/drm_atomic.c | 17 ++++++++++++----- >> drivers/gpu/drm/drm_atomic_uapi.c | 4 +++- >> drivers/gpu/drm/drm_crtc_internal.h | 4 +++- >> 3 files changed, 18 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_atomic.c >> b/drivers/gpu/drm/drm_atomic.c >> index 58527f151984..e7079a5f439c 100644 >> --- a/drivers/gpu/drm/drm_atomic.c >> +++ b/drivers/gpu/drm/drm_atomic.c >> @@ -1,6 +1,7 @@ >> /* >> * Copyright (C) 2014 Red Hat >> * Copyright (C) 2014 Intel Corp. >> + * Copyright (c) 2020, The Linux Foundation. All rights reserved. >> * >> * Permission is hereby granted, free of charge, to any person >> obtaining a >> * copy of this software and associated documentation files (the >> "Software"), >> @@ -1543,9 +1544,9 @@ int __drm_atomic_helper_set_config(struct >> drm_mode_set *set, >> } >> EXPORT_SYMBOL(__drm_atomic_helper_set_config); >> >> -void drm_atomic_print_state(const struct drm_atomic_state *state) >> +void drm_atomic_print_state(const struct drm_atomic_state *state, >> + struct drm_printer *p) > > Please add a nice kerneldoc for this newly exported function. > Specifically > this kerneldoc needs to include a warning that state updates after call > drm_atomic_state_helper_commit_hw_done() is unsafe to print using this > function, because it looks at the new state objects. Only the old state > structures will stay like this. > > So maybe rename the function to say print_new_state() to make this > completely clear. That way we can eventually add a print_old_state() > when > needed. > > Otherwise I think this makes sense, and nicely avoids the locking issue > of > looking at ->state pointers without the right locking. > -Daniel > Thanks for the review, I have addressed these comments and posted a V2. -Abhinav >> { >> - struct drm_printer p = drm_info_printer(state->dev->dev); >> struct drm_plane *plane; >> struct drm_plane_state *plane_state; >> struct drm_crtc *crtc; >> @@ -1554,17 +1555,23 @@ void drm_atomic_print_state(const struct >> drm_atomic_state *state) >> struct drm_connector_state *connector_state; >> int i; >> >> + if (!p) { >> + DRM_ERROR("invalid drm printer\n"); >> + return; >> + } >> + >> DRM_DEBUG_ATOMIC("checking %p\n", state); >> >> for_each_new_plane_in_state(state, plane, plane_state, i) >> - drm_atomic_plane_print_state(&p, plane_state); >> + drm_atomic_plane_print_state(p, plane_state); >> >> for_each_new_crtc_in_state(state, crtc, crtc_state, i) >> - drm_atomic_crtc_print_state(&p, crtc_state); >> + drm_atomic_crtc_print_state(p, crtc_state); >> >> for_each_new_connector_in_state(state, connector, connector_state, >> i) >> - drm_atomic_connector_print_state(&p, connector_state); >> + drm_atomic_connector_print_state(p, connector_state); >> } >> +EXPORT_SYMBOL(drm_atomic_print_state); >> >> static void __drm_state_dump(struct drm_device *dev, struct >> drm_printer *p, >> bool take_locks) >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c >> b/drivers/gpu/drm/drm_atomic_uapi.c >> index 25c269bc4681..d9ae86c92608 100644 >> --- a/drivers/gpu/drm/drm_atomic_uapi.c >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c >> @@ -2,6 +2,7 @@ >> * Copyright (C) 2014 Red Hat >> * Copyright (C) 2014 Intel Corp. >> * Copyright (C) 2018 Intel Corp. >> + * Copyright (c) 2020, The Linux Foundation. All rights reserved. >> * >> * Permission is hereby granted, free of charge, to any person >> obtaining a >> * copy of this software and associated documentation files (the >> "Software"), >> @@ -1294,6 +1295,7 @@ int drm_mode_atomic_ioctl(struct drm_device >> *dev, >> struct drm_out_fence_state *fence_state; >> int ret = 0; >> unsigned int i, j, num_fences; >> + struct drm_printer p = drm_info_printer(dev->dev); >> >> /* disallow for drivers not supporting atomic: */ >> if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) >> @@ -1413,7 +1415,7 @@ int drm_mode_atomic_ioctl(struct drm_device >> *dev, >> ret = drm_atomic_nonblocking_commit(state); >> } else { >> if (drm_debug_enabled(DRM_UT_STATE)) >> - drm_atomic_print_state(state); >> + drm_atomic_print_state(state, &p); >> >> ret = drm_atomic_commit(state); >> } >> diff --git a/drivers/gpu/drm/drm_crtc_internal.h >> b/drivers/gpu/drm/drm_crtc_internal.h >> index da96b2f64d7e..d34215366936 100644 >> --- a/drivers/gpu/drm/drm_crtc_internal.h >> +++ b/drivers/gpu/drm/drm_crtc_internal.h >> @@ -5,6 +5,7 @@ >> * Jesse Barnes <jesse.barnes@intel.com> >> * Copyright © 2014 Intel Corporation >> * Daniel Vetter <daniel.vetter@ffwll.ch> >> + * Copyright (c) 2020, The Linux Foundation. All rights reserved. >> * >> * Permission is hereby granted, free of charge, to any person >> obtaining a >> * copy of this software and associated documentation files (the >> "Software"), >> @@ -233,7 +234,8 @@ int __drm_atomic_helper_disable_plane(struct >> drm_plane *plane, >> int __drm_atomic_helper_set_config(struct drm_mode_set *set, >> struct drm_atomic_state *state); >> >> -void drm_atomic_print_state(const struct drm_atomic_state *state); >> +void drm_atomic_print_state(const struct drm_atomic_state *state, >> + struct drm_printer *p); >> >> /* drm_atomic_uapi.c */ >> int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state, >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> Forum, >> a Linux Foundation Collaborative Project >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 58527f151984..e7079a5f439c 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1,6 +1,7 @@ /* * Copyright (C) 2014 Red Hat * Copyright (C) 2014 Intel Corp. + * Copyright (c) 2020, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -1543,9 +1544,9 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, } EXPORT_SYMBOL(__drm_atomic_helper_set_config); -void drm_atomic_print_state(const struct drm_atomic_state *state) +void drm_atomic_print_state(const struct drm_atomic_state *state, + struct drm_printer *p) { - struct drm_printer p = drm_info_printer(state->dev->dev); struct drm_plane *plane; struct drm_plane_state *plane_state; struct drm_crtc *crtc; @@ -1554,17 +1555,23 @@ void drm_atomic_print_state(const struct drm_atomic_state *state) struct drm_connector_state *connector_state; int i; + if (!p) { + DRM_ERROR("invalid drm printer\n"); + return; + } + DRM_DEBUG_ATOMIC("checking %p\n", state); for_each_new_plane_in_state(state, plane, plane_state, i) - drm_atomic_plane_print_state(&p, plane_state); + drm_atomic_plane_print_state(p, plane_state); for_each_new_crtc_in_state(state, crtc, crtc_state, i) - drm_atomic_crtc_print_state(&p, crtc_state); + drm_atomic_crtc_print_state(p, crtc_state); for_each_new_connector_in_state(state, connector, connector_state, i) - drm_atomic_connector_print_state(&p, connector_state); + drm_atomic_connector_print_state(p, connector_state); } +EXPORT_SYMBOL(drm_atomic_print_state); static void __drm_state_dump(struct drm_device *dev, struct drm_printer *p, bool take_locks) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 25c269bc4681..d9ae86c92608 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -2,6 +2,7 @@ * Copyright (C) 2014 Red Hat * Copyright (C) 2014 Intel Corp. * Copyright (C) 2018 Intel Corp. + * Copyright (c) 2020, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -1294,6 +1295,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, struct drm_out_fence_state *fence_state; int ret = 0; unsigned int i, j, num_fences; + struct drm_printer p = drm_info_printer(dev->dev); /* disallow for drivers not supporting atomic: */ if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) @@ -1413,7 +1415,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, ret = drm_atomic_nonblocking_commit(state); } else { if (drm_debug_enabled(DRM_UT_STATE)) - drm_atomic_print_state(state); + drm_atomic_print_state(state, &p); ret = drm_atomic_commit(state); } diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index da96b2f64d7e..d34215366936 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -5,6 +5,7 @@ * Jesse Barnes <jesse.barnes@intel.com> * Copyright © 2014 Intel Corporation * Daniel Vetter <daniel.vetter@ffwll.ch> + * Copyright (c) 2020, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -233,7 +234,8 @@ int __drm_atomic_helper_disable_plane(struct drm_plane *plane, int __drm_atomic_helper_set_config(struct drm_mode_set *set, struct drm_atomic_state *state); -void drm_atomic_print_state(const struct drm_atomic_state *state); +void drm_atomic_print_state(const struct drm_atomic_state *state, + struct drm_printer *p); /* drm_atomic_uapi.c */ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state,
Currently drm_atomic_print_state() internally allocates and uses a drm_info printer. Allow it to accept any drm_printer type so that the API can be leveraged even for taking drm snapshot. Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> --- drivers/gpu/drm/drm_atomic.c | 17 ++++++++++++----- drivers/gpu/drm/drm_atomic_uapi.c | 4 +++- drivers/gpu/drm/drm_crtc_internal.h | 4 +++- 3 files changed, 18 insertions(+), 7 deletions(-)