| Message ID | 20210916211552.33490-1-greenfoo@u92.eu (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers where possible | expand |
On Thu, Sep 16, 2021 at 11:15:37PM +0200, Fernando Ramos wrote: > Hi all, > > One of the things in the DRM TODO list ("Documentation/gpu/todo.rst") was to > "use DRM_MODESET_LOCAL_ALL_* helpers instead of boilerplate". That's what this > patch series is about. > > You will find two types of changes here: > > - Replacing "drm_modeset_lock_all_ctx()" (and surrounding boilerplate) with > "DRM_MODESET_LOCK_ALL_BEGIN()/END()" in the remaining places (as it has > already been done in previous commits such as b7ea04d2) > > - Replacing "drm_modeset_lock_all()" with "DRM_MODESET_LOCK_ALL_BEGIN()/END()" > in the remaining places (as it has already been done in previous commits > such as 57037094) > > Most of the changes are straight forward, except for a few cases in the "amd" > and "i915" drivers where some extra dancing was needed to overcome the > limitation that the DRM_MODESET_LOCK_ALL_BEGIN()/END() macros can only be used > once inside the same function (the reason being that the macro expansion > includes *labels*, and you can not have two labels named the same inside one > function) > > Notice that, even after this patch series, some places remain where > "drm_modeset_lock_all()" and "drm_modeset_lock_all_ctx()" are still present, > all inside drm core (which makes sense), except for two (in "amd" and "i915") > which cannot be replaced due to the way they are being used. Can we at least replace those with drm_modeset_lock_all_ctx and delete drm_modeset_lock_all? That would be really nice goal to make sure these don't spread further. Otherwise great stuff, I'm trying to volunteer a few reviewers. -Daniel > > Fernando Ramos (15): > dmr: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN() > dmr/i915: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN() > dmr/msm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN() > drm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() > drm/vmwgfx: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() > drm/tegra: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() > drm/shmobile: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() > drm/radeon: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() > drm/omapdrm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() > drm/nouveau: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() > drm/msm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() > drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() > drm/gma500: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() > drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() > doc: drm: remove TODO entry regarding DRM_MODSET_LOCK_ALL cleanup > > Documentation/gpu/todo.rst | 17 ------- > Documentation/locking/ww-mutex-design.rst | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 13 +++-- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +++++++++---------- > .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 23 +++++---- > drivers/gpu/drm/drm_client_modeset.c | 14 +++--- > drivers/gpu/drm/drm_crtc_helper.c | 18 ++++--- > drivers/gpu/drm/drm_fb_helper.c | 10 ++-- > drivers/gpu/drm/drm_framebuffer.c | 6 ++- > drivers/gpu/drm/gma500/psb_device.c | 14 ++++-- > drivers/gpu/drm/i915/display/intel_audio.c | 12 +++-- > drivers/gpu/drm/i915/display/intel_display.c | 22 +++----- > .../drm/i915/display/intel_display_debugfs.c | 35 ++++++++----- > drivers/gpu/drm/i915/display/intel_overlay.c | 45 ++++++++--------- > drivers/gpu/drm/i915/display/intel_pipe_crc.c | 5 +- > drivers/gpu/drm/i915/i915_drv.c | 12 +++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 ++- > .../gpu/drm/msm/disp/msm_disp_snapshot_util.c | 10 ++-- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 12 +++-- > drivers/gpu/drm/omapdrm/omap_fb.c | 6 ++- > drivers/gpu/drm/radeon/radeon_device.c | 13 +++-- > drivers/gpu/drm/radeon/radeon_dp_mst.c | 7 ++- > drivers/gpu/drm/shmobile/shmob_drm_drv.c | 6 ++- > drivers/gpu/drm/tegra/dsi.c | 6 ++- > drivers/gpu/drm/tegra/hdmi.c | 5 +- > drivers/gpu/drm/tegra/sor.c | 10 ++-- > drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c | 11 ++-- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 +++-- > 28 files changed, 222 insertions(+), 180 deletions(-) > > -- > 2.33.0 >
On 21/09/17 05:24PM, Daniel Vetter wrote: > > Can we at least replace those with drm_modeset_lock_all_ctx and delete > drm_modeset_lock_all? That would be really nice goal to make sure these > don't spread further. I just checked and turns out no one else is using "drm_modeset_lock_all()" anymore. The only reference is the definition of the function itself, which I did not remove because it was being EXPORT_SYMBOL'ed and I was not sure whether it could be removed or not (to prevent breaking third party modules maybe?) The same goes true for its sibling "dmr_modeset_unlock_all()". But if you give me the green light I'll remove both of them right away :)
Hi all, One of the things in the DRM TODO list ("Documentation/gpu/todo.rst") was to "use DRM_MODESET_LOCAL_ALL_* helpers instead of boilerplate". That's what this patch series is about. You will find two types of changes here: - Replacing "drm_modeset_lock_all_ctx()" (and surrounding boilerplate) with "DRM_MODESET_LOCK_ALL_BEGIN()/END()" in the remaining places (as it has already been done in previous commits such as b7ea04d2) - Replacing "drm_modeset_lock_all()" with "DRM_MODESET_LOCK_ALL_BEGIN()/END()" in the remaining places (as it has already been done in previous commits such as 57037094) Most of the changes are straight forward, except for a few cases in the "amd" and "i915" drivers where some extra dancing was needed to overcome the limitation that the DRM_MODESET_LOCK_ALL_BEGIN()/END() macros can only be used once inside the same function (the reason being that the macro expansion includes *labels*, and you can not have two labels named the same inside one function) Notice that, even after this patch series, some places remain where "drm_modeset_lock_all()" and "drm_modeset_lock_all_ctx()" are still present, all inside drm core (which makes sense), except for two (in "amd" and "i915") which cannot be replaced due to the way they are being used. Fernando Ramos (15): dmr: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN() dmr/i915: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN() dmr/msm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN() drm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() drm/vmwgfx: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() drm/tegra: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() drm/shmobile: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() drm/radeon: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() drm/omapdrm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() drm/nouveau: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() drm/msm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() drm/gma500: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() doc: drm: remove TODO entry regarding DRM_MODSET_LOCK_ALL cleanup Documentation/gpu/todo.rst | 17 ------- Documentation/locking/ww-mutex-design.rst | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 13 +++-- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +++++++++---------- .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 23 +++++---- drivers/gpu/drm/drm_client_modeset.c | 14 +++--- drivers/gpu/drm/drm_crtc_helper.c | 18 ++++--- drivers/gpu/drm/drm_fb_helper.c | 10 ++-- drivers/gpu/drm/drm_framebuffer.c | 6 ++- drivers/gpu/drm/gma500/psb_device.c | 14 ++++-- drivers/gpu/drm/i915/display/intel_audio.c | 12 +++-- drivers/gpu/drm/i915/display/intel_display.c | 22 +++----- .../drm/i915/display/intel_display_debugfs.c | 35 ++++++++----- drivers/gpu/drm/i915/display/intel_overlay.c | 45 ++++++++--------- drivers/gpu/drm/i915/display/intel_pipe_crc.c | 5 +- drivers/gpu/drm/i915/i915_drv.c | 12 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 ++- .../gpu/drm/msm/disp/msm_disp_snapshot_util.c | 10 ++-- drivers/gpu/drm/nouveau/dispnv50/disp.c | 12 +++-- drivers/gpu/drm/omapdrm/omap_fb.c | 6 ++- drivers/gpu/drm/radeon/radeon_device.c | 13 +++-- drivers/gpu/drm/radeon/radeon_dp_mst.c | 7 ++- drivers/gpu/drm/shmobile/shmob_drm_drv.c | 6 ++- drivers/gpu/drm/tegra/dsi.c | 6 ++- drivers/gpu/drm/tegra/hdmi.c | 5 +- drivers/gpu/drm/tegra/sor.c | 10 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c | 11 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 +++-- 28 files changed, 222 insertions(+), 180 deletions(-)