diff mbox series

[03/15] dmr/msm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()

Message ID 20210916211552.33490-4-greenfoo@u92.eu (mailing list archive)
State Superseded
Headers show
Series drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible | expand

Commit Message

Fernando Ramos Sept. 16, 2021, 9:15 p.m. UTC 
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(-)

Comments

Sean Paul Sept. 17, 2021, 3:29 p.m. UTC | #1 
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
>
kernel test robot Sept. 20, 2021, 1:54 a.m. UTC | #2 
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
Fernando Ramos Sept. 20, 2021, 9:25 p.m. UTC | #3 
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 mbox series

Patch

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)