[Intel-gfx] [PATCH v2 00/11] lib/string_helpers: Add a few string helpers

Wed Jan 26 09:39:40 UTC 2022

Add some helpers under lib/string_helpers.h so they can be used
throughout the kernel. When I started doing this there were 2 other
previous attempts I know of, not counting the iterations each of them
had:

1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.com/
2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@linux.intel.com/#t

Now there is also the v1 of this same patch series:
https://lore.kernel.org/all/20220119072450.2890107-1-lucas.demarchi@intel.com/

Going through the comments I tried to find some common ground and
justification for what is in here, addressing some of the concerns
raised.

a. This version should be a drop-in replacement for what is currently in
   the tree, with no change in behavior or binary size. For binary
   size what I checked was that the linked objects in the end have the
   same size (gcc 11). From comments in the previous attempts this seems
   also the case for earlier compiler versions

b. I didn't change the function name to choice_* as suggested by Andrew
   Morton in 20191023155619.43e0013f0c8c673a5c508c1e at linux-foundation.org
   because other people argumented in favor of shorter names for these
   simple helpers - if they are long and people simply not use due to
   that, we failed. However as pointed out in v1 of this patchseries,
   onoff(), yesno(), enabledisable(), enableddisabled() have some
   issues: the last 2 are hard to read and for the first 2 it would not
   be hard to have the symbol to clash with variable names.
   From comments in v1, most people were in favor (or at least not
   opposed) to using str_on_off(), str_yes_no(), str_enable_disable()
   and str_enabled_disabled().

c. Use string_helper.h for these helpers - pulling string.h in the
   compilations units was one of the concerns and I think re-using this
   already existing header is better than creating a new string-choice.h

d. One alternative to all of this suggested by Christian König
   (43456ba7-c372-84cc-4949-dcb817188e21 at amd.com) would be to add a
   printk format. But besides the comment, he also seemed to like
   the common function. This brought the argument from others that the
   simple yesno()/enabledisable() already used in the code (or new
   renamed version) is easier to remember and use than e.g. %py[DOY]

Changes in v2:

  - Use str_ prefix and separate other words with underscore: it's a
    little bit longer, but should improve readability

  - Patches we re-split due to the rename: first patch adds all the new
    functions, then additional patches try to do one conversion at a
    time. While doing so, there were some fixes for issues already
    present along the way

  - Style suggestions from v1 were adopted

In v1 it was suggested to apply this in drm-misc. I will leave this to
maintainers to decide: maybe it would be simpler to merge the first
patches on drm-intel-next, wait for the back merge and merge the rest
through drm-misc - my fear is a big conflict with other work going in
drm-intel-next since the bulk of the rename is there.

I tried to figure out acks and reviews from v1 and apply them to how the
patches are now split.

thanks
Lucas De Marchi

Lucas De Marchi (11):
  lib/string_helpers: Consolidate string helpers implementation
  drm/i915: Fix trailing semicolon
  drm/i915: Use str_yes_no()
  drm/i915: Use str_enable_disable()
  drm/i915: Use str_enabled_disabled()
  drm/i915: Use str_on_off()
  drm/amd/display: Use str_yes_no()
  drm/gem: Sort includes alphabetically
  drm: Convert open-coded yes/no strings to yesno()
  tomoyo: Use str_yes_no()
  cxgb4: Use str_yes_no()

 drivers/gpu/drm/amd/amdgpu/atom.c             |   4 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c |  14 +-
 drivers/gpu/drm/dp/drm_dp.c                   |   3 +-
 drivers/gpu/drm/drm_client_modeset.c          |   3 +-
 drivers/gpu/drm/drm_gem.c                     |  23 +-
 drivers/gpu/drm/i915/display/g4x_dp.c         |   6 +-
 .../gpu/drm/i915/display/intel_backlight.c    |   3 +-
 drivers/gpu/drm/i915/display/intel_ddi.c      |   4 +-
 drivers/gpu/drm/i915/display/intel_display.c  |  46 ++--
 .../drm/i915/display/intel_display_debugfs.c  |  74 +++---
 .../drm/i915/display/intel_display_power.c    |   4 +-
 .../drm/i915/display/intel_display_trace.h    |   9 +-
 drivers/gpu/drm/i915/display/intel_dp.c       |  20 +-
 drivers/gpu/drm/i915/display/intel_dpll.c     |   3 +-
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c |   7 +-
 drivers/gpu/drm/i915/display/intel_dsi_vbt.c  |   7 +-
 drivers/gpu/drm/i915/display/intel_fbc.c      |   4 +-
 drivers/gpu/drm/i915/display/intel_fdi.c      |   8 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c     |   3 +-
 drivers/gpu/drm/i915/display/intel_sprite.c   |   6 +-
 drivers/gpu/drm/i915/display/vlv_dsi_pll.c    |   3 +-
 .../gpu/drm/i915/gem/selftests/huge_pages.c   |   9 +-
 .../drm/i915/gem/selftests/i915_gem_context.c |   7 +-
 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c   |   3 +-
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  11 +-
 .../drm/i915/gt/intel_execlists_submission.c  |   7 +-
 drivers/gpu/drm/i915/gt/intel_gt_pm.c         |   3 +-
 drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c |  52 ++--
 drivers/gpu/drm/i915/gt/intel_rc6.c           |   5 +-
 drivers/gpu/drm/i915/gt/intel_reset.c         |   3 +-
 drivers/gpu/drm/i915/gt/intel_rps.c           |  13 +-
 drivers/gpu/drm/i915/gt/intel_sseu.c          |   9 +-
 drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c  |  10 +-
 drivers/gpu/drm/i915/gt/selftest_timeline.c   |   3 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c     |   5 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_log.c    |   5 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c     |   6 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c   |   4 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc.c         |  14 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_debugfs.c |  20 +-
 drivers/gpu/drm/i915/i915_debugfs.c           |  17 +-
 drivers/gpu/drm/i915/i915_driver.c            |   4 +-
 drivers/gpu/drm/i915/i915_gpu_error.c         |   9 +-
 drivers/gpu/drm/i915/i915_params.c            |   5 +-
 drivers/gpu/drm/i915/i915_utils.h             |  21 +-
 drivers/gpu/drm/i915/intel_device_info.c      |   8 +-
 drivers/gpu/drm/i915/intel_dram.c             |  10 +-
 drivers/gpu/drm/i915/intel_pm.c               |  14 +-
 drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c  |   4 +-
 drivers/gpu/drm/i915/selftests/i915_active.c  |   3 +-
 drivers/gpu/drm/i915/vlv_suspend.c            |   3 +-
 drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c |   5 +-
 drivers/gpu/drm/radeon/atom.c                 |   3 +-
 drivers/gpu/drm/v3d/v3d_debugfs.c             |  11 +-
 drivers/gpu/drm/virtio/virtgpu_debugfs.c      |   4 +-
 .../ethernet/chelsio/cxgb4/cxgb4_debugfs.c    | 249 ++++++++++--------
 include/linux/string_helpers.h                |  20 ++
 security/tomoyo/audit.c                       |   2 +-
 security/tomoyo/common.c                      |  19 +-
 security/tomoyo/common.h                      |   1 -
 60 files changed, 482 insertions(+), 373 deletions(-)

-- 
2.34.1