| Message ID | 20210916211552.33490-4-greenfoo@u92.eu (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible | expand |
On Thu, Sep 16, 2021 at 11:15:40PM +0200, Fernando Ramos wrote: > As requested in Documentation/gpu/todo.rst, replace the boilerplate code > surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() > and DRM_MODESET_LOCK_ALL_END() > With the subject fixed (s/dmr/drm/), Reviewed-by: Sean Paul <sean@poorly.run> > Signed-off-by: Fernando Ramos <greenfoo@u92.eu> > --- > drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c > index cabe15190ec1..c83db90b0e02 100644 > --- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c > +++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c > @@ -99,20 +99,18 @@ static void msm_disp_capture_atomic_state(struct msm_disp_state *disp_state) > { > struct drm_device *ddev; > struct drm_modeset_acquire_ctx ctx; > + int ret; > > disp_state->timestamp = ktime_get(); > > ddev = disp_state->drm_dev; > > - drm_modeset_acquire_init(&ctx, 0); > - > - while (drm_modeset_lock_all_ctx(ddev, &ctx) != 0) > - drm_modeset_backoff(&ctx); > + DRM_MODESET_LOCK_ALL_BEGIN(ddev, ctx, 0, ret); > > disp_state->atomic_state = drm_atomic_helper_duplicate_state(ddev, > &ctx); > - drm_modeset_drop_locks(&ctx); > - drm_modeset_acquire_fini(&ctx); > + > + DRM_MODESET_LOCK_ALL_END(ddev, ctx, ret); > } > > void msm_disp_snapshot_capture_state(struct msm_disp_state *disp_state) > -- > 2.33.0 >
Hi Fernando,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-exynos/exynos-drm-next]
[also build test ERROR on tegra-drm/drm/tegra/for-next linus/master v5.15-rc2 next-20210917]
[cannot apply to drm-intel/for-linux-next tegra/for-next drm-tip/drm-tip airlied/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/Fernando-Ramos/drm-cleanup-Use-DRM_MODESET_LOCK_ALL_-helpers-where-possible/20210917-051842
base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.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/0da6630f04d8f86f9d3c9a65fe61a6c6d182c87f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Fernando-Ramos/drm-cleanup-Use-DRM_MODESET_LOCK_ALL_-helpers-where-possible/20210917-051842
git checkout 0da6630f04d8f86f9d3c9a65fe61a6c6d182c87f
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arm
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 >>):
In file included from include/drm/drm_crtc.h:36,
from include/drm/drm_atomic_helper.h:31,
from drivers/gpu/drm/msm/disp/msm_disp_snapshot.h:9,
from drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c:8:
drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c: In function 'msm_disp_capture_atomic_state':
>> include/drm/drm_modeset_lock.h:167:14: error: implicit declaration of function 'drm_drv_uses_atomic_modeset' [-Werror=implicit-function-declaration]
167 | if (!drm_drv_uses_atomic_modeset(dev)) \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c:108:9: note: in expansion of macro 'DRM_MODESET_LOCK_ALL_BEGIN'
108 | DRM_MODESET_LOCK_ALL_BEGIN(ddev, ctx, 0, ret);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/drm_drv_uses_atomic_modeset +167 include/drm/drm_modeset_lock.h
a6a8bb848d5ca4 Daniel Vetter 2014-07-25 138
06eaae46381737 Thierry Reding 2015-12-02 139 int drm_modeset_lock_all_ctx(struct drm_device *dev,
51fd371bbaf940 Rob Clark 2013-11-19 140 struct drm_modeset_acquire_ctx *ctx);
51fd371bbaf940 Rob Clark 2013-11-19 141
b7ea04d299c78b Sean Paul 2018-11-29 142 /**
b7ea04d299c78b Sean Paul 2018-11-29 143 * DRM_MODESET_LOCK_ALL_BEGIN - Helper to acquire modeset locks
b7ea04d299c78b Sean Paul 2018-11-29 144 * @dev: drm device
b7ea04d299c78b Sean Paul 2018-11-29 145 * @ctx: local modeset acquire context, will be dereferenced
b7ea04d299c78b Sean Paul 2018-11-29 146 * @flags: DRM_MODESET_ACQUIRE_* flags to pass to drm_modeset_acquire_init()
b7ea04d299c78b Sean Paul 2018-11-29 147 * @ret: local ret/err/etc variable to track error status
b7ea04d299c78b Sean Paul 2018-11-29 148 *
b7ea04d299c78b Sean Paul 2018-11-29 149 * Use these macros to simplify grabbing all modeset locks using a local
b7ea04d299c78b Sean Paul 2018-11-29 150 * context. This has the advantage of reducing boilerplate, but also properly
b7ea04d299c78b Sean Paul 2018-11-29 151 * checking return values where appropriate.
b7ea04d299c78b Sean Paul 2018-11-29 152 *
b7ea04d299c78b Sean Paul 2018-11-29 153 * Any code run between BEGIN and END will be holding the modeset locks.
b7ea04d299c78b Sean Paul 2018-11-29 154 *
b7ea04d299c78b Sean Paul 2018-11-29 155 * This must be paired with DRM_MODESET_LOCK_ALL_END(). We will jump back and
b7ea04d299c78b Sean Paul 2018-11-29 156 * forth between the labels on deadlock and error conditions.
b7ea04d299c78b Sean Paul 2018-11-29 157 *
b7ea04d299c78b Sean Paul 2018-11-29 158 * Drivers can acquire additional modeset locks. If any lock acquisition
b7ea04d299c78b Sean Paul 2018-11-29 159 * fails, the control flow needs to jump to DRM_MODESET_LOCK_ALL_END() with
b7ea04d299c78b Sean Paul 2018-11-29 160 * the @ret parameter containing the return value of drm_modeset_lock().
b7ea04d299c78b Sean Paul 2018-11-29 161 *
b7ea04d299c78b Sean Paul 2018-11-29 162 * Returns:
b7ea04d299c78b Sean Paul 2018-11-29 163 * The only possible value of ret immediately after DRM_MODESET_LOCK_ALL_BEGIN()
b7ea04d299c78b Sean Paul 2018-11-29 164 * is 0, so no error checking is necessary
b7ea04d299c78b Sean Paul 2018-11-29 165 */
b7ea04d299c78b Sean Paul 2018-11-29 166 #define DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, flags, ret) \
77ef38574beb3e Daniel Vetter 2020-08-14 @167 if (!drm_drv_uses_atomic_modeset(dev)) \
77ef38574beb3e Daniel Vetter 2020-08-14 168 mutex_lock(&dev->mode_config.mutex); \
b7ea04d299c78b Sean Paul 2018-11-29 169 drm_modeset_acquire_init(&ctx, flags); \
b7ea04d299c78b Sean Paul 2018-11-29 170 modeset_lock_retry: \
b7ea04d299c78b Sean Paul 2018-11-29 171 ret = drm_modeset_lock_all_ctx(dev, &ctx); \
b7ea04d299c78b Sean Paul 2018-11-29 172 if (ret) \
b7ea04d299c78b Sean Paul 2018-11-29 173 goto modeset_lock_fail;
b7ea04d299c78b Sean Paul 2018-11-29 174
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On 21/09/20 09:54AM, kernel test robot wrote: > > [auto build test ERROR on drm-exynos/exynos-drm-next] > [also build test ERROR on tegra-drm/drm/tegra/for-next linus/master v5.15-rc2 next-20210917] I forgot to #include <drm/drm_drv.h> for those platforms and didn't notice because I only tried to build for X86. I'll fix it. > [cannot apply to drm-intel/for-linux-next tegra/for-next drm-tip/drm-tip airlied/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'. I built this patch against drm-next, which currently points to v5.15-rc1. Should I be targeting a different branch? In any case, as suggested, I'll remember to use "--base" in the future to make it easier to apply. Thanks for the hint. > All errors (new ones prefixed by >>): > > In file included from include/drm/drm_crtc.h:36, > from include/drm/drm_atomic_helper.h:31, > from drivers/gpu/drm/msm/disp/msm_disp_snapshot.h:9, > from drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c:8: > drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c: In function 'msm_disp_capture_atomic_state': > >> include/drm/drm_modeset_lock.h:167:14: error: implicit declaration of function 'drm_drv_uses_atomic_modeset' [-Werror=implicit-function-declaration] > 167 | if (!drm_drv_uses_atomic_modeset(dev)) \ > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c:108:9: note: in expansion of macro 'DRM_MODESET_LOCK_ALL_BEGIN' > 108 | DRM_MODESET_LOCK_ALL_BEGIN(ddev, ctx, 0, ret); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~ > cc1: some warnings being treated as errors Out of curiosity: The top comment says there were two build errors (one on exynos and another one on tegra), but there is only one reported bug (on msm). Is this because the bot only reports the first error found? Is there a link to a report with each of the build errors on each of the platforms? Thanks.
diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c index cabe15190ec1..c83db90b0e02 100644 --- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c +++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c @@ -99,20 +99,18 @@ static void msm_disp_capture_atomic_state(struct msm_disp_state *disp_state) { struct drm_device *ddev; struct drm_modeset_acquire_ctx ctx; + int ret; disp_state->timestamp = ktime_get(); ddev = disp_state->drm_dev; - drm_modeset_acquire_init(&ctx, 0); - - while (drm_modeset_lock_all_ctx(ddev, &ctx) != 0) - drm_modeset_backoff(&ctx); + DRM_MODESET_LOCK_ALL_BEGIN(ddev, ctx, 0, ret); disp_state->atomic_state = drm_atomic_helper_duplicate_state(ddev, &ctx); - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); + + DRM_MODESET_LOCK_ALL_END(ddev, ctx, ret); } void msm_disp_snapshot_capture_state(struct msm_disp_state *disp_state)
As requested in Documentation/gpu/todo.rst, replace the boilerplate code surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and DRM_MODESET_LOCK_ALL_END() Signed-off-by: Fernando Ramos <greenfoo@u92.eu> --- drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)