Message ID | 5f01d9c209298ab53f62a3f9a52473469d91ad4a.1670249413.git.jani.nikula@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/i915/backlight: drop implict dev_priv etc. | expand |
On Mon, 05 Dec 2022, Jani Nikula <jani.nikula@intel.com> wrote: > Since the VLV/CHV backlight registers are only used on VLV/CHV, there's > no need to dynamically look up DISPLAY_MMIO_BASE(). We know it's > VLV_DISPLAY_BASE. Use it statically, reducing the implicit dev_priv > references. Hmm, I spotted this, but looks like I didn't spot that none of the *other* backlight register apparently aren't used on VLV/CHV. Could we just drop DISPLAY_MMIO_BASE() from them altogether? BR, Jani. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > .../drm/i915/display/intel_backlight_regs.h | 21 ++++++++----------- > 1 file changed, 9 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_backlight_regs.h b/drivers/gpu/drm/i915/display/intel_backlight_regs.h > index 344eb8096bd2..02bd1f8201bf 100644 > --- a/drivers/gpu/drm/i915/display/intel_backlight_regs.h > +++ b/drivers/gpu/drm/i915/display/intel_backlight_regs.h > @@ -8,20 +8,17 @@ > > #include "intel_display_reg_defs.h" > > -#define _VLV_BLC_PWM_CTL2_A (DISPLAY_MMIO_BASE(dev_priv) + 0x61250) > -#define _VLV_BLC_PWM_CTL2_B (DISPLAY_MMIO_BASE(dev_priv) + 0x61350) > -#define VLV_BLC_PWM_CTL2(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL2_A, \ > - _VLV_BLC_PWM_CTL2_B) > +#define _VLV_BLC_PWM_CTL2_A (VLV_DISPLAY_BASE + 0x61250) > +#define _VLV_BLC_PWM_CTL2_B (VLV_DISPLAY_BASE + 0x61350) > +#define VLV_BLC_PWM_CTL2(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL2_A, _VLV_BLC_PWM_CTL2_B) > > -#define _VLV_BLC_PWM_CTL_A (DISPLAY_MMIO_BASE(dev_priv) + 0x61254) > -#define _VLV_BLC_PWM_CTL_B (DISPLAY_MMIO_BASE(dev_priv) + 0x61354) > -#define VLV_BLC_PWM_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL_A, \ > - _VLV_BLC_PWM_CTL_B) > +#define _VLV_BLC_PWM_CTL_A (VLV_DISPLAY_BASE + 0x61254) > +#define _VLV_BLC_PWM_CTL_B (VLV_DISPLAY_BASE + 0x61354) > +#define VLV_BLC_PWM_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL_A, _VLV_BLC_PWM_CTL_B) > > -#define _VLV_BLC_HIST_CTL_A (DISPLAY_MMIO_BASE(dev_priv) + 0x61260) > -#define _VLV_BLC_HIST_CTL_B (DISPLAY_MMIO_BASE(dev_priv) + 0x61360) > -#define VLV_BLC_HIST_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_HIST_CTL_A, \ > - _VLV_BLC_HIST_CTL_B) > +#define _VLV_BLC_HIST_CTL_A (VLV_DISPLAY_BASE + 0x61260) > +#define _VLV_BLC_HIST_CTL_B (VLV_DISPLAY_BASE + 0x61360) > +#define VLV_BLC_HIST_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_HIST_CTL_A, _VLV_BLC_HIST_CTL_B) > > /* Backlight control */ > #define BLC_PWM_CTL2 _MMIO(DISPLAY_MMIO_BASE(dev_priv) + 0x61250) /* 965+ only */
On Mon, Dec 05, 2022 at 04:17:16PM +0200, Jani Nikula wrote: > On Mon, 05 Dec 2022, Jani Nikula <jani.nikula@intel.com> wrote: > > Since the VLV/CHV backlight registers are only used on VLV/CHV, there's > > no need to dynamically look up DISPLAY_MMIO_BASE(). We know it's > > VLV_DISPLAY_BASE. Use it statically, reducing the implicit dev_priv > > references. > > Hmm, I spotted this, but looks like I didn't spot that none of the > *other* backlight register apparently aren't used on VLV/CHV. > > Could we just drop DISPLAY_MMIO_BASE() from them altogether? That would be neat. I hope Ville agrees. Did you use any sed or coccinele on patch 3? Feel free to use: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> for the series, if you decide to go with it and have this extra removal on a following up work. > > BR, > Jani. > > > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > .../drm/i915/display/intel_backlight_regs.h | 21 ++++++++----------- > > 1 file changed, 9 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_backlight_regs.h b/drivers/gpu/drm/i915/display/intel_backlight_regs.h > > index 344eb8096bd2..02bd1f8201bf 100644 > > --- a/drivers/gpu/drm/i915/display/intel_backlight_regs.h > > +++ b/drivers/gpu/drm/i915/display/intel_backlight_regs.h > > @@ -8,20 +8,17 @@ > > > > #include "intel_display_reg_defs.h" > > > > -#define _VLV_BLC_PWM_CTL2_A (DISPLAY_MMIO_BASE(dev_priv) + 0x61250) > > -#define _VLV_BLC_PWM_CTL2_B (DISPLAY_MMIO_BASE(dev_priv) + 0x61350) > > -#define VLV_BLC_PWM_CTL2(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL2_A, \ > > - _VLV_BLC_PWM_CTL2_B) > > +#define _VLV_BLC_PWM_CTL2_A (VLV_DISPLAY_BASE + 0x61250) > > +#define _VLV_BLC_PWM_CTL2_B (VLV_DISPLAY_BASE + 0x61350) > > +#define VLV_BLC_PWM_CTL2(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL2_A, _VLV_BLC_PWM_CTL2_B) > > > > -#define _VLV_BLC_PWM_CTL_A (DISPLAY_MMIO_BASE(dev_priv) + 0x61254) > > -#define _VLV_BLC_PWM_CTL_B (DISPLAY_MMIO_BASE(dev_priv) + 0x61354) > > -#define VLV_BLC_PWM_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL_A, \ > > - _VLV_BLC_PWM_CTL_B) > > +#define _VLV_BLC_PWM_CTL_A (VLV_DISPLAY_BASE + 0x61254) > > +#define _VLV_BLC_PWM_CTL_B (VLV_DISPLAY_BASE + 0x61354) > > +#define VLV_BLC_PWM_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL_A, _VLV_BLC_PWM_CTL_B) > > > > -#define _VLV_BLC_HIST_CTL_A (DISPLAY_MMIO_BASE(dev_priv) + 0x61260) > > -#define _VLV_BLC_HIST_CTL_B (DISPLAY_MMIO_BASE(dev_priv) + 0x61360) > > -#define VLV_BLC_HIST_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_HIST_CTL_A, \ > > - _VLV_BLC_HIST_CTL_B) > > +#define _VLV_BLC_HIST_CTL_A (VLV_DISPLAY_BASE + 0x61260) > > +#define _VLV_BLC_HIST_CTL_B (VLV_DISPLAY_BASE + 0x61360) > > +#define VLV_BLC_HIST_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_HIST_CTL_A, _VLV_BLC_HIST_CTL_B) > > > > /* Backlight control */ > > #define BLC_PWM_CTL2 _MMIO(DISPLAY_MMIO_BASE(dev_priv) + 0x61250) /* 965+ only */ > > -- > Jani Nikula, Intel Open Source Graphics Center
On Mon, 05 Dec 2022, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > On Mon, Dec 05, 2022 at 04:17:16PM +0200, Jani Nikula wrote: >> On Mon, 05 Dec 2022, Jani Nikula <jani.nikula@intel.com> wrote: >> > Since the VLV/CHV backlight registers are only used on VLV/CHV, there's >> > no need to dynamically look up DISPLAY_MMIO_BASE(). We know it's >> > VLV_DISPLAY_BASE. Use it statically, reducing the implicit dev_priv >> > references. >> >> Hmm, I spotted this, but looks like I didn't spot that none of the >> *other* backlight register apparently aren't used on VLV/CHV. >> >> Could we just drop DISPLAY_MMIO_BASE() from them altogether? > > That would be neat. I hope Ville agrees. Sent v2 with that. > > Did you use any sed or coccinele on patch 3? Nah, I usually just use query-replace-regexp in Emacs for the one-off replacements. > > Feel free to use: > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > for the series, if you decide to go with it and have this > extra removal on a following up work. Thanks, I tagged them as # v1 for the patches I had to rebase. BR, Jani. > >> >> BR, >> Jani. >> >> > >> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> > --- >> > .../drm/i915/display/intel_backlight_regs.h | 21 ++++++++----------- >> > 1 file changed, 9 insertions(+), 12 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_backlight_regs.h b/drivers/gpu/drm/i915/display/intel_backlight_regs.h >> > index 344eb8096bd2..02bd1f8201bf 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_backlight_regs.h >> > +++ b/drivers/gpu/drm/i915/display/intel_backlight_regs.h >> > @@ -8,20 +8,17 @@ >> > >> > #include "intel_display_reg_defs.h" >> > >> > -#define _VLV_BLC_PWM_CTL2_A (DISPLAY_MMIO_BASE(dev_priv) + 0x61250) >> > -#define _VLV_BLC_PWM_CTL2_B (DISPLAY_MMIO_BASE(dev_priv) + 0x61350) >> > -#define VLV_BLC_PWM_CTL2(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL2_A, \ >> > - _VLV_BLC_PWM_CTL2_B) >> > +#define _VLV_BLC_PWM_CTL2_A (VLV_DISPLAY_BASE + 0x61250) >> > +#define _VLV_BLC_PWM_CTL2_B (VLV_DISPLAY_BASE + 0x61350) >> > +#define VLV_BLC_PWM_CTL2(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL2_A, _VLV_BLC_PWM_CTL2_B) >> > >> > -#define _VLV_BLC_PWM_CTL_A (DISPLAY_MMIO_BASE(dev_priv) + 0x61254) >> > -#define _VLV_BLC_PWM_CTL_B (DISPLAY_MMIO_BASE(dev_priv) + 0x61354) >> > -#define VLV_BLC_PWM_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL_A, \ >> > - _VLV_BLC_PWM_CTL_B) >> > +#define _VLV_BLC_PWM_CTL_A (VLV_DISPLAY_BASE + 0x61254) >> > +#define _VLV_BLC_PWM_CTL_B (VLV_DISPLAY_BASE + 0x61354) >> > +#define VLV_BLC_PWM_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL_A, _VLV_BLC_PWM_CTL_B) >> > >> > -#define _VLV_BLC_HIST_CTL_A (DISPLAY_MMIO_BASE(dev_priv) + 0x61260) >> > -#define _VLV_BLC_HIST_CTL_B (DISPLAY_MMIO_BASE(dev_priv) + 0x61360) >> > -#define VLV_BLC_HIST_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_HIST_CTL_A, \ >> > - _VLV_BLC_HIST_CTL_B) >> > +#define _VLV_BLC_HIST_CTL_A (VLV_DISPLAY_BASE + 0x61260) >> > +#define _VLV_BLC_HIST_CTL_B (VLV_DISPLAY_BASE + 0x61360) >> > +#define VLV_BLC_HIST_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_HIST_CTL_A, _VLV_BLC_HIST_CTL_B) >> > >> > /* Backlight control */ >> > #define BLC_PWM_CTL2 _MMIO(DISPLAY_MMIO_BASE(dev_priv) + 0x61250) /* 965+ only */ >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_backlight_regs.h b/drivers/gpu/drm/i915/display/intel_backlight_regs.h index 344eb8096bd2..02bd1f8201bf 100644 --- a/drivers/gpu/drm/i915/display/intel_backlight_regs.h +++ b/drivers/gpu/drm/i915/display/intel_backlight_regs.h @@ -8,20 +8,17 @@ #include "intel_display_reg_defs.h" -#define _VLV_BLC_PWM_CTL2_A (DISPLAY_MMIO_BASE(dev_priv) + 0x61250) -#define _VLV_BLC_PWM_CTL2_B (DISPLAY_MMIO_BASE(dev_priv) + 0x61350) -#define VLV_BLC_PWM_CTL2(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL2_A, \ - _VLV_BLC_PWM_CTL2_B) +#define _VLV_BLC_PWM_CTL2_A (VLV_DISPLAY_BASE + 0x61250) +#define _VLV_BLC_PWM_CTL2_B (VLV_DISPLAY_BASE + 0x61350) +#define VLV_BLC_PWM_CTL2(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL2_A, _VLV_BLC_PWM_CTL2_B) -#define _VLV_BLC_PWM_CTL_A (DISPLAY_MMIO_BASE(dev_priv) + 0x61254) -#define _VLV_BLC_PWM_CTL_B (DISPLAY_MMIO_BASE(dev_priv) + 0x61354) -#define VLV_BLC_PWM_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL_A, \ - _VLV_BLC_PWM_CTL_B) +#define _VLV_BLC_PWM_CTL_A (VLV_DISPLAY_BASE + 0x61254) +#define _VLV_BLC_PWM_CTL_B (VLV_DISPLAY_BASE + 0x61354) +#define VLV_BLC_PWM_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_PWM_CTL_A, _VLV_BLC_PWM_CTL_B) -#define _VLV_BLC_HIST_CTL_A (DISPLAY_MMIO_BASE(dev_priv) + 0x61260) -#define _VLV_BLC_HIST_CTL_B (DISPLAY_MMIO_BASE(dev_priv) + 0x61360) -#define VLV_BLC_HIST_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_HIST_CTL_A, \ - _VLV_BLC_HIST_CTL_B) +#define _VLV_BLC_HIST_CTL_A (VLV_DISPLAY_BASE + 0x61260) +#define _VLV_BLC_HIST_CTL_B (VLV_DISPLAY_BASE + 0x61360) +#define VLV_BLC_HIST_CTL(pipe) _MMIO_PIPE(pipe, _VLV_BLC_HIST_CTL_A, _VLV_BLC_HIST_CTL_B) /* Backlight control */ #define BLC_PWM_CTL2 _MMIO(DISPLAY_MMIO_BASE(dev_priv) + 0x61250) /* 965+ only */
Since the VLV/CHV backlight registers are only used on VLV/CHV, there's no need to dynamically look up DISPLAY_MMIO_BASE(). We know it's VLV_DISPLAY_BASE. Use it statically, reducing the implicit dev_priv references. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- .../drm/i915/display/intel_backlight_regs.h | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-)