| Message ID | 20190110230844.9213-1-aditya.swarup@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915/cnl: Fix CNL macros for Voltage Swing programming | expand |
On Thu, 10 Jan 2019, Aditya Swarup <aditya.swarup@intel.com> wrote: > CNL macros for register groups CNL_PORT_TX_DW2_* / CNL_PORT_TX_DW5_* are > configured incorrectly wrt definition of _CNL_PORT_TX_DW_GRP. > > v2: Jani suggested to keep the macros organized semantically i.e., by > function, secondarily by port/pipe/transcoder.->(dw, port) > > Cc: Clint Taylor <clinton.a.taylor@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Signed-off-by: Aditya Swarup <aditya.swarup@intel.com> Pushed to dinq with the appropriate Fixes: tag added, thanks for the patch. > --- > There are still inconsistencies in some macro definitions. The macros > for MG phy registers are (port, ln) e.g., > MG_TX2_LINK_PARAMS(port, ln) and also CNL_PORT_TX_DW4_LN(port, ln) > whereas for ICL -> _ICL_PORT_PCS_DW_LN(dw, ln, port). > > Do you feel that we need to make these definitions consistent and what > should be the sequence -> (dw, ln, port)/(ln, port) or (dw, port, ln)/ > (port,ln)? Feel free to send the patches. I think (dw, port, ln). BR, Jani. > > drivers/gpu/drm/i915/i915_reg.h | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 44958d994bfa..fad5a9e8b44d 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1814,7 +1814,7 @@ enum i915_power_well_id { > #define _CNL_PORT_TX_C_LN0_OFFSET 0x162C40 > #define _CNL_PORT_TX_D_LN0_OFFSET 0x162E40 > #define _CNL_PORT_TX_F_LN0_OFFSET 0x162840 > -#define _CNL_PORT_TX_DW_GRP(port, dw) (_PICK((port), \ > +#define _CNL_PORT_TX_DW_GRP(dw, port) (_PICK((port), \ > _CNL_PORT_TX_AE_GRP_OFFSET, \ > _CNL_PORT_TX_B_GRP_OFFSET, \ > _CNL_PORT_TX_B_GRP_OFFSET, \ > @@ -1822,7 +1822,7 @@ enum i915_power_well_id { > _CNL_PORT_TX_AE_GRP_OFFSET, \ > _CNL_PORT_TX_F_GRP_OFFSET) + \ > 4 * (dw)) > -#define _CNL_PORT_TX_DW_LN0(port, dw) (_PICK((port), \ > +#define _CNL_PORT_TX_DW_LN0(dw, port) (_PICK((port), \ > _CNL_PORT_TX_AE_LN0_OFFSET, \ > _CNL_PORT_TX_B_LN0_OFFSET, \ > _CNL_PORT_TX_B_LN0_OFFSET, \ > @@ -1858,9 +1858,9 @@ enum i915_power_well_id { > > #define _CNL_PORT_TX_DW4_LN0_AE 0x162450 > #define _CNL_PORT_TX_DW4_LN1_AE 0x1624D0 > -#define CNL_PORT_TX_DW4_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP((port), 4)) > -#define CNL_PORT_TX_DW4_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0((port), 4)) > -#define CNL_PORT_TX_DW4_LN(port, ln) _MMIO(_CNL_PORT_TX_DW_LN0((port), 4) + \ > +#define CNL_PORT_TX_DW4_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP(4, (port))) > +#define CNL_PORT_TX_DW4_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0(4, (port))) > +#define CNL_PORT_TX_DW4_LN(port, ln) _MMIO(_CNL_PORT_TX_DW_LN0(4, (port)) + \ > ((ln) * (_CNL_PORT_TX_DW4_LN1_AE - \ > _CNL_PORT_TX_DW4_LN0_AE))) > #define ICL_PORT_TX_DW4_AUX(port) _MMIO(_ICL_PORT_TX_DW_AUX(4, port)) > @@ -1888,8 +1888,8 @@ enum i915_power_well_id { > #define RTERM_SELECT(x) ((x) << 3) > #define RTERM_SELECT_MASK (0x7 << 3) > > -#define CNL_PORT_TX_DW7_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP((port), 7)) > -#define CNL_PORT_TX_DW7_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0((port), 7)) > +#define CNL_PORT_TX_DW7_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP(7, (port))) > +#define CNL_PORT_TX_DW7_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0(7, (port))) > #define ICL_PORT_TX_DW7_AUX(port) _MMIO(_ICL_PORT_TX_DW_AUX(7, port)) > #define ICL_PORT_TX_DW7_GRP(port) _MMIO(_ICL_PORT_TX_DW_GRP(7, port)) > #define ICL_PORT_TX_DW7_LN0(port) _MMIO(_ICL_PORT_TX_DW_LN(7, 0, port))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 44958d994bfa..fad5a9e8b44d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1814,7 +1814,7 @@ enum i915_power_well_id { #define _CNL_PORT_TX_C_LN0_OFFSET 0x162C40 #define _CNL_PORT_TX_D_LN0_OFFSET 0x162E40 #define _CNL_PORT_TX_F_LN0_OFFSET 0x162840 -#define _CNL_PORT_TX_DW_GRP(port, dw) (_PICK((port), \ +#define _CNL_PORT_TX_DW_GRP(dw, port) (_PICK((port), \ _CNL_PORT_TX_AE_GRP_OFFSET, \ _CNL_PORT_TX_B_GRP_OFFSET, \ _CNL_PORT_TX_B_GRP_OFFSET, \ @@ -1822,7 +1822,7 @@ enum i915_power_well_id { _CNL_PORT_TX_AE_GRP_OFFSET, \ _CNL_PORT_TX_F_GRP_OFFSET) + \ 4 * (dw)) -#define _CNL_PORT_TX_DW_LN0(port, dw) (_PICK((port), \ +#define _CNL_PORT_TX_DW_LN0(dw, port) (_PICK((port), \ _CNL_PORT_TX_AE_LN0_OFFSET, \ _CNL_PORT_TX_B_LN0_OFFSET, \ _CNL_PORT_TX_B_LN0_OFFSET, \ @@ -1858,9 +1858,9 @@ enum i915_power_well_id { #define _CNL_PORT_TX_DW4_LN0_AE 0x162450 #define _CNL_PORT_TX_DW4_LN1_AE 0x1624D0 -#define CNL_PORT_TX_DW4_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP((port), 4)) -#define CNL_PORT_TX_DW4_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0((port), 4)) -#define CNL_PORT_TX_DW4_LN(port, ln) _MMIO(_CNL_PORT_TX_DW_LN0((port), 4) + \ +#define CNL_PORT_TX_DW4_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP(4, (port))) +#define CNL_PORT_TX_DW4_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0(4, (port))) +#define CNL_PORT_TX_DW4_LN(port, ln) _MMIO(_CNL_PORT_TX_DW_LN0(4, (port)) + \ ((ln) * (_CNL_PORT_TX_DW4_LN1_AE - \ _CNL_PORT_TX_DW4_LN0_AE))) #define ICL_PORT_TX_DW4_AUX(port) _MMIO(_ICL_PORT_TX_DW_AUX(4, port)) @@ -1888,8 +1888,8 @@ enum i915_power_well_id { #define RTERM_SELECT(x) ((x) << 3) #define RTERM_SELECT_MASK (0x7 << 3) -#define CNL_PORT_TX_DW7_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP((port), 7)) -#define CNL_PORT_TX_DW7_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0((port), 7)) +#define CNL_PORT_TX_DW7_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP(7, (port))) +#define CNL_PORT_TX_DW7_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0(7, (port))) #define ICL_PORT_TX_DW7_AUX(port) _MMIO(_ICL_PORT_TX_DW_AUX(7, port)) #define ICL_PORT_TX_DW7_GRP(port) _MMIO(_ICL_PORT_TX_DW_GRP(7, port)) #define ICL_PORT_TX_DW7_LN0(port) _MMIO(_ICL_PORT_TX_DW_LN(7, 0, port))
CNL macros for register groups CNL_PORT_TX_DW2_* / CNL_PORT_TX_DW5_* are configured incorrectly wrt definition of _CNL_PORT_TX_DW_GRP. v2: Jani suggested to keep the macros organized semantically i.e., by function, secondarily by port/pipe/transcoder.->(dw, port) Cc: Clint Taylor <clinton.a.taylor@intel.com> Cc: Imre Deak <imre.deak@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com> --- There are still inconsistencies in some macro definitions. The macros for MG phy registers are (port, ln) e.g., MG_TX2_LINK_PARAMS(port, ln) and also CNL_PORT_TX_DW4_LN(port, ln) whereas for ICL -> _ICL_PORT_PCS_DW_LN(dw, ln, port). Do you feel that we need to make these definitions consistent and what should be the sequence -> (dw, ln, port)/(ln, port) or (dw, port, ln)/ (port,ln)? drivers/gpu/drm/i915/i915_reg.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)