Re: [Intel-gfx] [PATCH] drm/i915: Start using REG_BIT* macros

From: "Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Start using REG_BIT* macros
Date: Fri, 2 Sep 2022 11:13:19 +0300	[thread overview]
Message-ID: <YxG7HwaE0zSjzDk/@intel.com> (raw)
In-Reply-To: <87wnanb8c3.fsf@intel.com>

On Thu, Sep 01, 2022 at 03:14:04PM +0300, Jani Nikula wrote:
> On Thu, 01 Sep 2022, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote:
> > Lets start to use REG_BIT* macros, instead of (x << 0) like expressions.
> 
> Please be more specific in the commit subject, it's not like we haven't
> started using REG_BIT in general, ever since we introduced it! ;) So
> refer to CDCLK_CTL.

Yeah, agree looks too generic, it was previsouly part of the series, 
so got edited "a bit" :)

> 
> Please just update the subject while pushing, no need to resend for
> that,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> PS. Could I also persuade you to split out some of the cdclk register
> macros to a new display/intel_cdclk_regs.h header? E.g. CDCLK_CTL is
> only used in intel_cdclk.c (and gvt).

Well need to take a look.. Shouldn't be very complex, I hope.

Stan

> 
> >
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 5e6239864c35..d82f14979fdf 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7077,16 +7077,16 @@ enum skl_power_gate {
> >  
> >  /* CDCLK_CTL */
> >  #define CDCLK_CTL			_MMIO(0x46000)
> > -#define  CDCLK_FREQ_SEL_MASK		(3 << 26)
> > -#define  CDCLK_FREQ_450_432		(0 << 26)
> > -#define  CDCLK_FREQ_540			(1 << 26)
> > -#define  CDCLK_FREQ_337_308		(2 << 26)
> > -#define  CDCLK_FREQ_675_617		(3 << 26)
> > -#define  BXT_CDCLK_CD2X_DIV_SEL_MASK	(3 << 22)
> > -#define  BXT_CDCLK_CD2X_DIV_SEL_1	(0 << 22)
> > -#define  BXT_CDCLK_CD2X_DIV_SEL_1_5	(1 << 22)
> > -#define  BXT_CDCLK_CD2X_DIV_SEL_2	(2 << 22)
> > -#define  BXT_CDCLK_CD2X_DIV_SEL_4	(3 << 22)
> > +#define  CDCLK_FREQ_SEL_MASK		REG_GENMASK(27, 26)
> > +#define  CDCLK_FREQ_450_432		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 0)
> > +#define  CDCLK_FREQ_540		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 1)
> > +#define  CDCLK_FREQ_337_308		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 2)
> > +#define  CDCLK_FREQ_675_617		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 3)
> > +#define  BXT_CDCLK_CD2X_DIV_SEL_MASK	REG_GENMASK(23, 22)
> > +#define  BXT_CDCLK_CD2X_DIV_SEL_1	REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 0)
> > +#define  BXT_CDCLK_CD2X_DIV_SEL_1_5	REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 1)
> > +#define  BXT_CDCLK_CD2X_DIV_SEL_2	REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 2)
> > +#define  BXT_CDCLK_CD2X_DIV_SEL_4	REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 3)
> >  #define  BXT_CDCLK_CD2X_PIPE(pipe)	((pipe) << 20)
> >  #define  CDCLK_DIVMUX_CD_OVERRIDE	(1 << 19)
> >  #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center