mbox series

[000/123] drm/i915: remove implicit dev_priv local variable use

Message ID cover.1714136165.git.jani.nikula@intel.com (mailing list archive)
Headers show
Series drm/i915: remove implicit dev_priv local variable use | expand

Message

Jani Nikula April 26, 2024, 1:01 p.m. UTC
Hey all, it's time to stop using the implicit dev_priv local variable in
register macros. Yes, this is huge. It's also (almost) completely
scripted.

Thoughts?

BR,
Jani.


Here's the script:


	#!/bin/bash
	
	set -e
	
	# Find all the registers implicitly relying on dev_priv
	REGS=$(git grep -h "#define.*dev_priv" -- drivers/gpu/drm/i915/i915_reg.h |\
		       grep -v '#define[ \t]\+[a-zA-Z0-9_]\+(dev_priv' |\
		       sed 's/#define[ \t]\+\([a-zA-Z0-9_]\+\).*/\1/')
	
	for reg in $REGS; do
		echo $reg
		
		FILES=$(git grep -wl $reg -- drivers/gpu/drm/i915)
	
		cocci=$(mktemp)
		cat >$cocci <<EOF
	@@
	identifier reg =~ "^$reg\$";
	@@
	
	  <...
	(
	  reg(
	+     dev_priv,
	      ...)
	|
	- reg
	+ reg(dev_priv)
	)
	  ...>
	
	EOF
	
		# already function-like macros
		sed -i "s/\(#define *${reg}(\)/\1dev_priv, /" $FILES
	
		# new function-like macros
		sed -i "s/\(#define *${reg}\)\([ \t]\)/\1(dev_priv)\2/" $FILES
	
		spatch --sp-file $cocci --in-place --linux-spacing $FILES >/dev/null
	
		rm -f $cocci
	
		git commit -as -F - <<EOF
	drm/i915: pass dev_priv explicitly to $reg
	
	Avoid the implicit dev_priv local variable use, and pass dev_priv
	explicitly to the $reg register macro.
	
	EOF
		
	done


Jani Nikula (123):
  drm/i915: pass dev_priv explicitly to DPLL
  drm/i915: pass dev_priv explicitly to DPLL_MD
  drm/i915: pass dev_priv explicitly to PALETTE
  drm/i915: pass dev_priv explicitly to PIPE_CRC_CTL
  drm/i915: pass dev_priv explicitly to PIPE_CRC_RES_1_IVB
  drm/i915: pass dev_priv explicitly to PIPE_CRC_RES_2_IVB
  drm/i915: pass dev_priv explicitly to PIPE_CRC_RES_3_IVB
  drm/i915: pass dev_priv explicitly to PIPE_CRC_RES_4_IVB
  drm/i915: pass dev_priv explicitly to PIPE_CRC_RES_5_IVB
  drm/i915: pass dev_priv explicitly to PIPE_CRC_RES_RED
  drm/i915: pass dev_priv explicitly to PIPE_CRC_RES_GREEN
  drm/i915: pass dev_priv explicitly to PIPE_CRC_RES_BLUE
  drm/i915: pass dev_priv explicitly to PIPE_CRC_RES_RES1_I915
  drm/i915: pass dev_priv explicitly to PIPE_CRC_RES_RES2_G4X
  drm/i915: pass dev_priv explicitly to TRANS_HTOTAL
  drm/i915: pass dev_priv explicitly to TRANS_HBLANK
  drm/i915: pass dev_priv explicitly to TRANS_HSYNC
  drm/i915: pass dev_priv explicitly to TRANS_VTOTAL
  drm/i915: pass dev_priv explicitly to TRANS_VBLANK
  drm/i915: pass dev_priv explicitly to TRANS_VSYNC
  drm/i915: pass dev_priv explicitly to BCLRPAT
  drm/i915: pass dev_priv explicitly to TRANS_VSYNCSHIFT
  drm/i915: pass dev_priv explicitly to PIPESRC
  drm/i915: pass dev_priv explicitly to TRANS_MULT
  drm/i915: pass dev_priv explicitly to TRANS_VRR_CTL
  drm/i915: pass dev_priv explicitly to TRANS_VRR_VMAX
  drm/i915: pass dev_priv explicitly to TRANS_VRR_VMIN
  drm/i915: pass dev_priv explicitly to TRANS_VRR_VMAXSHIFT
  drm/i915: pass dev_priv explicitly to TRANS_VRR_STATUS
  drm/i915: pass dev_priv explicitly to TRANS_VRR_VTOTAL_PREV
  drm/i915: pass dev_priv explicitly to TRANS_VRR_FLIPLINE
  drm/i915: pass dev_priv explicitly to TRANS_VRR_STATUS2
  drm/i915: pass dev_priv explicitly to TRANS_PUSH
  drm/i915: pass dev_priv explicitly to TRANS_VRR_VSYNC
  drm/i915: pass dev_priv explicitly to PORT_HOTPLUG_EN
  drm/i915: pass dev_priv explicitly to PORT_HOTPLUG_STAT
  drm/i915: pass dev_priv explicitly to PORT_DFT2_G4X
  drm/i915: pass dev_priv explicitly to PFIT_CONTROL
  drm/i915: pass dev_priv explicitly to PFIT_PGM_RATIOS
  drm/i915: pass dev_priv explicitly to PFIT_AUTO_RATIOS
  drm/i915: pass dev_priv explicitly to TRANSCONF
  drm/i915: pass dev_priv explicitly to PIPEDSL
  drm/i915: pass dev_priv explicitly to PIPEFRAME
  drm/i915: pass dev_priv explicitly to PIPEFRAMEPIXEL
  drm/i915: pass dev_priv explicitly to PIPESTAT
  drm/i915: pass dev_priv explicitly to PIPEGCMAX
  drm/i915: pass dev_priv explicitly to PIPE_ARB_CTL
  drm/i915: pass dev_priv explicitly to ICL_PIPESTATUS
  drm/i915: pass dev_priv explicitly to DSPARB
  drm/i915: pass dev_priv explicitly to DSPFW1
  drm/i915: pass dev_priv explicitly to DSPFW2
  drm/i915: pass dev_priv explicitly to DSPFW3
  drm/i915: pass dev_priv explicitly to PIPE_FRMCOUNT_G4X
  drm/i915: pass dev_priv explicitly to PIPE_FLIPCOUNT_G4X
  drm/i915: pass dev_priv explicitly to CURCNTR
  drm/i915: pass dev_priv explicitly to CURBASE
  drm/i915: pass dev_priv explicitly to CURPOS
  drm/i915: pass dev_priv explicitly to CURPOS_ERLY_TPT
  drm/i915: pass dev_priv explicitly to CURSIZE
  drm/i915: pass dev_priv explicitly to CUR_FBC_CTL
  drm/i915: pass dev_priv explicitly to CUR_CHICKEN
  drm/i915: pass dev_priv explicitly to CURSURFLIVE
  drm/i915: pass dev_priv explicitly to DSPADDR_VLV
  drm/i915: pass dev_priv explicitly to DSPCNTR
  drm/i915: pass dev_priv explicitly to DSPADDR
  drm/i915: pass dev_priv explicitly to DSPSTRIDE
  drm/i915: pass dev_priv explicitly to DSPPOS
  drm/i915: pass dev_priv explicitly to DSPSIZE
  drm/i915: pass dev_priv explicitly to DSPSURF
  drm/i915: pass dev_priv explicitly to DSPTILEOFF
  drm/i915: pass dev_priv explicitly to DSPOFFSET
  drm/i915: pass dev_priv explicitly to DSPSURFLIVE
  drm/i915: pass dev_priv explicitly to DSPGAMC
  drm/i915: pass dev_priv explicitly to CHV_BLEND
  drm/i915: pass dev_priv explicitly to CHV_CANVAS
  drm/i915: pass dev_priv explicitly to PRIMPOS
  drm/i915: pass dev_priv explicitly to PRIMSIZE
  drm/i915: pass dev_priv explicitly to PRIMCNSTALPHA
  drm/i915: pass dev_priv explicitly to SWF0
  drm/i915: pass dev_priv explicitly to SWF1
  drm/i915: pass dev_priv explicitly to SWF3
  drm/i915: pass dev_priv explicitly to _PIPEBDSL
  drm/i915: pass dev_priv explicitly to _TRANSBCONF
  drm/i915: pass dev_priv explicitly to _PIPEBSTAT
  drm/i915: pass dev_priv explicitly to _PIPEB_FRMCOUNT_G4X
  drm/i915: pass dev_priv explicitly to _PIPEB_FLIPCOUNT_G4X
  drm/i915: pass dev_priv explicitly to _DSPBCNTR
  drm/i915: pass dev_priv explicitly to _DSPBADDR
  drm/i915: pass dev_priv explicitly to _DSPBSTRIDE
  drm/i915: pass dev_priv explicitly to _DSPBPOS
  drm/i915: pass dev_priv explicitly to _DSPBSIZE
  drm/i915: pass dev_priv explicitly to _DSPBSURF
  drm/i915: pass dev_priv explicitly to _DSPBTILEOFF
  drm/i915: pass dev_priv explicitly to _DSPBOFFSET
  drm/i915: pass dev_priv explicitly to _DSPBSURFLIVE
  drm/i915: pass dev_priv explicitly to PIPE_DATA_M1
  drm/i915: pass dev_priv explicitly to PIPE_DATA_N1
  drm/i915: pass dev_priv explicitly to PIPE_DATA_M2
  drm/i915: pass dev_priv explicitly to PIPE_DATA_N2
  drm/i915: pass dev_priv explicitly to PIPE_LINK_M1
  drm/i915: pass dev_priv explicitly to PIPE_LINK_N1
  drm/i915: pass dev_priv explicitly to PIPE_LINK_M2
  drm/i915: pass dev_priv explicitly to PIPE_LINK_N2
  drm/i915: pass dev_priv explicitly to HSW_TVIDEO_DIP_CTL
  drm/i915: pass dev_priv explicitly to HSW_TVIDEO_DIP_GCP
  drm/i915: pass dev_priv explicitly to HSW_TVIDEO_DIP_AVI_DATA
  drm/i915: pass dev_priv explicitly to HSW_TVIDEO_DIP_VS_DATA
  drm/i915: pass dev_priv explicitly to HSW_TVIDEO_DIP_SPD_DATA
  drm/i915: pass dev_priv explicitly to HSW_TVIDEO_DIP_GMP_DATA
  drm/i915: pass dev_priv explicitly to HSW_TVIDEO_DIP_VSC_DATA
  drm/i915: pass dev_priv explicitly to GLK_TVIDEO_DIP_DRM_DATA
  drm/i915: pass dev_priv explicitly to ICL_VIDEO_DIP_PPS_DATA
  drm/i915: pass dev_priv explicitly to ICL_VIDEO_DIP_PPS_ECC
  drm/i915: pass dev_priv explicitly to ADL_TVIDEO_DIP_AS_SDP_DATA
  drm/i915: pass dev_priv explicitly to HSW_STEREO_3D_CTL
  drm/i915: pass dev_priv explicitly to TRANS_DDI_FUNC_CTL
  drm/i915: pass dev_priv explicitly to TRANS_DDI_FUNC_CTL2
  drm/i915: pass dev_priv explicitly to TGL_DP_TP_CTL
  drm/i915: pass dev_priv explicitly to TGL_DP_TP_STATUS
  drm/i915: pass dev_priv explicitly to TRANS_MSA_MISC
  drm/i915: pass dev_priv explicitly to TRANS_SET_CONTEXT_LATENCY
  drm/i915: pass dev_priv explicitly to MTL_CLKGATE_DIS_TRANS
  drm/i915: pass dev_priv explicitly to DSPLINOFF

 drivers/gpu/drm/i915/display/g4x_dp.c         |   2 +-
 drivers/gpu/drm/i915/display/i9xx_plane.c     |  62 ++--
 drivers/gpu/drm/i915/display/i9xx_wm.c        |  87 +++---
 drivers/gpu/drm/i915/display/icl_dsi.c        |  46 +--
 drivers/gpu/drm/i915/display/intel_color.c    |  44 +--
 drivers/gpu/drm/i915/display/intel_crt.c      |  50 ++--
 drivers/gpu/drm/i915/display/intel_cursor.c   |  33 ++-
 drivers/gpu/drm/i915/display/intel_ddi.c      |  47 ++--
 drivers/gpu/drm/i915/display/intel_display.c  | 203 ++++++++------
 .../drm/i915/display/intel_display_debugfs.c  |   2 +-
 .../gpu/drm/i915/display/intel_display_irq.c  |  41 +--
 .../drm/i915/display/intel_display_power.c    |   2 +-
 .../i915/display/intel_display_power_well.c   |  14 +-
 drivers/gpu/drm/i915/display/intel_dp.c       |   3 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |   5 +-
 drivers/gpu/drm/i915/display/intel_dpll.c     |  63 +++--
 drivers/gpu/drm/i915/display/intel_drrs.c     |   2 +-
 drivers/gpu/drm/i915/display/intel_dvo.c      |   5 +-
 drivers/gpu/drm/i915/display/intel_fbc.c      |   8 +-
 drivers/gpu/drm/i915/display/intel_fdi.c      |  15 +-
 .../drm/i915/display/intel_fifo_underrun.c    |  13 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c     |  32 ++-
 .../gpu/drm/i915/display/intel_hotplug_irq.c  |  12 +-
 drivers/gpu/drm/i915/display/intel_lspcon.c   |   2 +-
 drivers/gpu/drm/i915/display/intel_lvds.c     |   2 +-
 drivers/gpu/drm/i915/display/intel_overlay.c  |  10 +-
 .../gpu/drm/i915/display/intel_pch_display.c  |  21 +-
 drivers/gpu/drm/i915/display/intel_pipe_crc.c |  20 +-
 drivers/gpu/drm/i915/display/intel_pps.c      |   2 +-
 drivers/gpu/drm/i915/display/intel_psr.c      |  18 +-
 drivers/gpu/drm/i915/display/intel_vblank.c   |  14 +-
 drivers/gpu/drm/i915/display/intel_vrr.c      |  52 ++--
 drivers/gpu/drm/i915/display/vlv_dsi.c        |   3 +-
 drivers/gpu/drm/i915/gvt/cmd_parser.c         |  14 +-
 drivers/gpu/drm/i915/gvt/display.c            |  71 ++---
 drivers/gpu/drm/i915/gvt/fb_decoder.c         |  20 +-
 drivers/gpu/drm/i915/gvt/handlers.c           |  40 +--
 drivers/gpu/drm/i915/i915_irq.c               |   5 +-
 drivers/gpu/drm/i915/i915_reg.h               | 246 ++++++++--------
 drivers/gpu/drm/i915/i915_suspend.c           |  48 ++--
 drivers/gpu/drm/i915/intel_clock_gating.c     |   9 +-
 drivers/gpu/drm/i915/intel_gvt_mmio_table.c   | 264 +++++++++---------
 42 files changed, 906 insertions(+), 746 deletions(-)

Comments

Jani Nikula April 26, 2024, 1:09 p.m. UTC | #1
On Fri, 26 Apr 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> Hey all, it's time to stop using the implicit dev_priv local variable in
> register macros. Yes, this is huge. It's also (almost) completely
> scripted.

Okay, I was first going to send the entire series, but chickened out and
hit ^C when git send-email was going though the patches. You get the
idea with what's here. It's just more of the same. Plus I pushed the lot
to [1].

I think we'll need to do this. The question is how to handle this
churn. Do we want this many patches? If not, how much to squash?


BR,
Jani.


[1] https://gitlab.freedesktop.org/jani/linux/-/commits/regs-mass-dev-priv-removal/?ref_type=heads
Rodrigo Vivi April 29, 2024, 12:43 p.m. UTC | #2
On Fri, Apr 26, 2024 at 04:09:45PM +0300, Jani Nikula wrote:
> On Fri, 26 Apr 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> > Hey all, it's time to stop using the implicit dev_priv local variable in
> > register macros. Yes, this is huge. It's also (almost) completely
> > scripted.
> 
> Okay, I was first going to send the entire series, but chickened out and
> hit ^C when git send-email was going though the patches. You get the
> idea with what's here. It's just more of the same. Plus I pushed the lot
> to [1].

now it makes sense. I was wondering why I was only seeing a few patches
when the series was telling over a hundred.

> 
> I think we'll need to do this. 

Agreed. Let's do this.

> The question is how to handle this
> churn. Do we want this many patches? If not, how much to squash?

From a glance on these initial patches, it sounds really organized in
individual patches and easy to review.
Perhaps if we take this path we might just split the series in blocks
and merge these initial 17, and we continue over the next weeks.

However, if this is automated like you mentioned in the cover letter,
perhaps we can do one patch per directory? (display vs gvt vs gem? vs drm/i915/{.c,.h})

> 
> 
> BR,
> Jani.
> 
> 
> [1] https://gitlab.freedesktop.org/jani/linux/-/commits/regs-mass-dev-priv-removal/?ref_type=heads
> 
> 
> -- 
> Jani Nikula, Intel
Jani Nikula April 29, 2024, 2:08 p.m. UTC | #3
On Mon, 29 Apr 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> From a glance on these initial patches, it sounds really organized in
> individual patches and easy to review.
> Perhaps if we take this path we might just split the series in blocks
> and merge these initial 17, and we continue over the next weeks.

Ack.

> However, if this is automated like you mentioned in the cover letter,
> perhaps we can do one patch per directory? (display vs gvt vs gem? vs drm/i915/{.c,.h})

I'll look into it. The first natural batch came about when I moved some
color regs, so I sent them [1].

BR,
Jani.


[1] https://lore.kernel.org/r/cover.1714399071.git.jani.nikula@intel.com