diff mbox series

[1/5] drm/i915/backlight: use VLV_DISPLAY_BASE for VLV/CHV backlight registers

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

Commit Message

Jani Nikula Dec. 5, 2022, 2:11 p.m. UTC 
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(-)

Comments

Jani Nikula Dec. 5, 2022, 2:17 p.m. UTC | #1 
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 */
Rodrigo Vivi Dec. 5, 2022, 5:25 p.m. UTC | #2 
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
Jani Nikula Dec. 7, 2022, 9:36 a.m. UTC | #3 
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 mbox series

Patch

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 */