| Message ID | 20181029232315.15539-1-anusha.srivatsa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/fia: FIA registers offset implementation. | expand |
On Mon, Oct 29, 2018 at 04:23:15PM -0700, Anusha Srivatsa wrote: > The registers DPCSSS,DPSP,DPMLE1 and DPPMS are all at an offset > from the base - which is the FLexi IO Adaptor. Lets follow the > offset calculation while accessing these registers. > > v2: > - Follow spec for numbering - s/0/1(Lucas) > - s/FIA_1/FIA1_BASE (Anusha) > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> thanks Lucas De Marchi > --- > drivers/gpu/drm/i915/i915_reg.h | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index bcee91bcfba6..dd74bc01c64e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2057,8 +2057,15 @@ enum i915_power_well_id { > #define BXT_PORT_CL2CM_DW6(phy) _BXT_PHY((phy), _PORT_CL2CM_DW6_BC) > #define DW6_OLDO_DYN_PWR_DOWN_EN (1 << 28) > > +/* FIA Offsets */ > +#define FIA1_BASE 0x163000 > +#define PORT_TX_DFLEXDPMLE1_OFFSET 0x008C0 > +#define PORT_TX_DFLEXDPPMS_OFFSET 0x00890 > +#define PORT_TX_DFLEXDPCSSS_OFFSET 0x00894 > +#define PORT_TX_DFLEXDPSP_OFFSET 0x008A0 > + > /* ICL PHY DFLEX registers */ > -#define PORT_TX_DFLEXDPMLE1 _MMIO(0x1638C0) > +#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + PORT_TX_DFLEXDPMLE1_OFFSET) > #define DFLEXDPMLE1_DPMLETC_MASK(n) (0xf << (4 * (n))) > #define DFLEXDPMLE1_DPMLETC(n, x) ((x) << (4 * (n))) > > @@ -10988,17 +10995,17 @@ enum skl_power_gate { > _ICL_DSC1_RC_BUF_THRESH_1_UDW_PB, \ > _ICL_DSC1_RC_BUF_THRESH_1_UDW_PC) > > -#define PORT_TX_DFLEXDPSP _MMIO(0x1638A0) > +#define PORT_TX_DFLEXDPSP _MMIO(FIA1_BASE + PORT_TX_DFLEXDPSP_OFFSET) > #define TC_LIVE_STATE_TBT(tc_port) (1 << ((tc_port) * 8 + 6)) > #define TC_LIVE_STATE_TC(tc_port) (1 << ((tc_port) * 8 + 5)) > #define DP_LANE_ASSIGNMENT_SHIFT(tc_port) ((tc_port) * 8) > #define DP_LANE_ASSIGNMENT_MASK(tc_port) (0xf << ((tc_port) * 8)) > #define DP_LANE_ASSIGNMENT(tc_port, x) ((x) << ((tc_port) * 8)) > > -#define PORT_TX_DFLEXDPPMS _MMIO(0x163890) > +#define PORT_TX_DFLEXDPPMS _MMIO(FIA1_BASE + PORT_TX_DFLEXDPPMS_OFFSET) > #define DP_PHY_MODE_STATUS_COMPLETED(tc_port) (1 << (tc_port)) > > -#define PORT_TX_DFLEXDPCSSS _MMIO(0x163894) > +#define PORT_TX_DFLEXDPCSSS _MMIO(FIA1_BASE + PORT_TX_DFLEXDPCSSS_OFFSET) > #define DP_PHY_MODE_STATUS_NOT_SAFE(tc_port) (1 << (tc_port)) > > #endif /* _I915_REG_H_ */ > -- > 2.17.1 >
On Mon, 29 Oct 2018, Anusha Srivatsa <anusha.srivatsa@intel.com> wrote: > The registers DPCSSS,DPSP,DPMLE1 and DPPMS are all at an offset > from the base - which is the FLexi IO Adaptor. Lets follow the > offset calculation while accessing these registers. Why? If I search the specs or i915_reg.h for, say, 0x1638c0 I'll find what I'm looking for. We generally don't follow this type of definitions for registers. We may have some, but those are exceptions. Please don't do this without some pretty good rationale written in the commit message. BR, Jani. > > v2: > - Follow spec for numbering - s/0/1(Lucas) > - s/FIA_1/FIA1_BASE (Anusha) > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index bcee91bcfba6..dd74bc01c64e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2057,8 +2057,15 @@ enum i915_power_well_id { > #define BXT_PORT_CL2CM_DW6(phy) _BXT_PHY((phy), _PORT_CL2CM_DW6_BC) > #define DW6_OLDO_DYN_PWR_DOWN_EN (1 << 28) > > +/* FIA Offsets */ > +#define FIA1_BASE 0x163000 > +#define PORT_TX_DFLEXDPMLE1_OFFSET 0x008C0 > +#define PORT_TX_DFLEXDPPMS_OFFSET 0x00890 > +#define PORT_TX_DFLEXDPCSSS_OFFSET 0x00894 > +#define PORT_TX_DFLEXDPSP_OFFSET 0x008A0 > + > /* ICL PHY DFLEX registers */ > -#define PORT_TX_DFLEXDPMLE1 _MMIO(0x1638C0) > +#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + PORT_TX_DFLEXDPMLE1_OFFSET) > #define DFLEXDPMLE1_DPMLETC_MASK(n) (0xf << (4 * (n))) > #define DFLEXDPMLE1_DPMLETC(n, x) ((x) << (4 * (n))) > > @@ -10988,17 +10995,17 @@ enum skl_power_gate { > _ICL_DSC1_RC_BUF_THRESH_1_UDW_PB, \ > _ICL_DSC1_RC_BUF_THRESH_1_UDW_PC) > > -#define PORT_TX_DFLEXDPSP _MMIO(0x1638A0) > +#define PORT_TX_DFLEXDPSP _MMIO(FIA1_BASE + PORT_TX_DFLEXDPSP_OFFSET) > #define TC_LIVE_STATE_TBT(tc_port) (1 << ((tc_port) * 8 + 6)) > #define TC_LIVE_STATE_TC(tc_port) (1 << ((tc_port) * 8 + 5)) > #define DP_LANE_ASSIGNMENT_SHIFT(tc_port) ((tc_port) * 8) > #define DP_LANE_ASSIGNMENT_MASK(tc_port) (0xf << ((tc_port) * 8)) > #define DP_LANE_ASSIGNMENT(tc_port, x) ((x) << ((tc_port) * 8)) > > -#define PORT_TX_DFLEXDPPMS _MMIO(0x163890) > +#define PORT_TX_DFLEXDPPMS _MMIO(FIA1_BASE + PORT_TX_DFLEXDPPMS_OFFSET) > #define DP_PHY_MODE_STATUS_COMPLETED(tc_port) (1 << (tc_port)) > > -#define PORT_TX_DFLEXDPCSSS _MMIO(0x163894) > +#define PORT_TX_DFLEXDPCSSS _MMIO(FIA1_BASE + PORT_TX_DFLEXDPCSSS_OFFSET) > #define DP_PHY_MODE_STATUS_NOT_SAFE(tc_port) (1 << (tc_port)) > > #endif /* _I915_REG_H_ */
On Tue, Oct 30, 2018 at 6:56 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Mon, 29 Oct 2018, Anusha Srivatsa <anusha.srivatsa@intel.com> wrote: > > The registers DPCSSS,DPSP,DPMLE1 and DPPMS are all at an offset > > from the base - which is the FLexi IO Adaptor. Lets follow the > > offset calculation while accessing these registers. > > Why? > > If I search the specs or i915_reg.h for, say, 0x1638c0 I'll find what > I'm looking for. > > We generally don't follow this type of definitions for registers. We may > have some, but those are exceptions. Because spec 29550 treats those registers as a base + offset to be more future proof regarding a change of the base. And yes, I think something like that needs to be stated in the commit message. Is this enough? Lucas De Marchi > > Please don't do this without some pretty good rationale written in the > commit message. > > BR, > Jani. > > > > > v2: > > - Follow spec for numbering - s/0/1(Lucas) > > - s/FIA_1/FIA1_BASE (Anusha) > > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 15 +++++++++++---- > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index bcee91bcfba6..dd74bc01c64e 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -2057,8 +2057,15 @@ enum i915_power_well_id { > > #define BXT_PORT_CL2CM_DW6(phy) _BXT_PHY((phy), _PORT_CL2CM_DW6_BC) > > #define DW6_OLDO_DYN_PWR_DOWN_EN (1 << 28) > > > > +/* FIA Offsets */ > > +#define FIA1_BASE 0x163000 > > +#define PORT_TX_DFLEXDPMLE1_OFFSET 0x008C0 > > +#define PORT_TX_DFLEXDPPMS_OFFSET 0x00890 > > +#define PORT_TX_DFLEXDPCSSS_OFFSET 0x00894 > > +#define PORT_TX_DFLEXDPSP_OFFSET 0x008A0 > > + > > /* ICL PHY DFLEX registers */ > > -#define PORT_TX_DFLEXDPMLE1 _MMIO(0x1638C0) > > +#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + PORT_TX_DFLEXDPMLE1_OFFSET) > > #define DFLEXDPMLE1_DPMLETC_MASK(n) (0xf << (4 * (n))) > > #define DFLEXDPMLE1_DPMLETC(n, x) ((x) << (4 * (n))) > > > > @@ -10988,17 +10995,17 @@ enum skl_power_gate { > > _ICL_DSC1_RC_BUF_THRESH_1_UDW_PB, \ > > _ICL_DSC1_RC_BUF_THRESH_1_UDW_PC) > > > > -#define PORT_TX_DFLEXDPSP _MMIO(0x1638A0) > > +#define PORT_TX_DFLEXDPSP _MMIO(FIA1_BASE + PORT_TX_DFLEXDPSP_OFFSET) > > #define TC_LIVE_STATE_TBT(tc_port) (1 << ((tc_port) * 8 + 6)) > > #define TC_LIVE_STATE_TC(tc_port) (1 << ((tc_port) * 8 + 5)) > > #define DP_LANE_ASSIGNMENT_SHIFT(tc_port) ((tc_port) * 8) > > #define DP_LANE_ASSIGNMENT_MASK(tc_port) (0xf << ((tc_port) * 8)) > > #define DP_LANE_ASSIGNMENT(tc_port, x) ((x) << ((tc_port) * 8)) > > > > -#define PORT_TX_DFLEXDPPMS _MMIO(0x163890) > > +#define PORT_TX_DFLEXDPPMS _MMIO(FIA1_BASE + PORT_TX_DFLEXDPPMS_OFFSET) > > #define DP_PHY_MODE_STATUS_COMPLETED(tc_port) (1 << (tc_port)) > > > > -#define PORT_TX_DFLEXDPCSSS _MMIO(0x163894) > > +#define PORT_TX_DFLEXDPCSSS _MMIO(FIA1_BASE + PORT_TX_DFLEXDPCSSS_OFFSET) > > #define DP_PHY_MODE_STATUS_NOT_SAFE(tc_port) (1 << (tc_port)) > > > > #endif /* _I915_REG_H_ */ > > -- > Jani Nikula, Intel Open Source Graphics Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 30 Oct 2018, Lucas De Marchi <lucas.de.marchi@gmail.com> wrote: > On Tue, Oct 30, 2018 at 6:56 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: >> >> On Mon, 29 Oct 2018, Anusha Srivatsa <anusha.srivatsa@intel.com> wrote: >> > The registers DPCSSS,DPSP,DPMLE1 and DPPMS are all at an offset >> > from the base - which is the FLexi IO Adaptor. Lets follow the >> > offset calculation while accessing these registers. >> >> Why? >> >> If I search the specs or i915_reg.h for, say, 0x1638c0 I'll find what >> I'm looking for. >> >> We generally don't follow this type of definitions for registers. We may >> have some, but those are exceptions. > > Because spec 29550 treats those registers as a base + offset to be more > future proof regarding a change of the base. And yes, I think something > like that needs to be stated in the commit message. Is this enough? Fair enough. Please also see comments in-line. > > Lucas De Marchi > >> >> Please don't do this without some pretty good rationale written in the >> commit message. >> >> BR, >> Jani. >> >> > >> > v2: >> > - Follow spec for numbering - s/0/1(Lucas) >> > - s/FIA_1/FIA1_BASE (Anusha) >> > >> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> >> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> >> > --- >> > drivers/gpu/drm/i915/i915_reg.h | 15 +++++++++++---- >> > 1 file changed, 11 insertions(+), 4 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> > index bcee91bcfba6..dd74bc01c64e 100644 >> > --- a/drivers/gpu/drm/i915/i915_reg.h >> > +++ b/drivers/gpu/drm/i915/i915_reg.h >> > @@ -2057,8 +2057,15 @@ enum i915_power_well_id { >> > #define BXT_PORT_CL2CM_DW6(phy) _BXT_PHY((phy), _PORT_CL2CM_DW6_BC) >> > #define DW6_OLDO_DYN_PWR_DOWN_EN (1 << 28) >> > >> > +/* FIA Offsets */ >> > +#define FIA1_BASE 0x163000 Ok. >> > +#define PORT_TX_DFLEXDPMLE1_OFFSET 0x008C0 >> > +#define PORT_TX_DFLEXDPPMS_OFFSET 0x00890 >> > +#define PORT_TX_DFLEXDPCSSS_OFFSET 0x00894 >> > +#define PORT_TX_DFLEXDPSP_OFFSET 0x008A0 These should not be grouped here, instead please add above each macro below or just leave out, see below. >> > + >> > /* ICL PHY DFLEX registers */ >> > -#define PORT_TX_DFLEXDPMLE1 _MMIO(0x1638C0) >> > +#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + PORT_TX_DFLEXDPMLE1_OFFSET) IMO either: #define _PORT_TX_DFLEXDPMLE1 0x008C0 #define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + _PORT_TX_DFLEXDPMLE1) or just: #define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + 0x008C0) BR, Jani. >> > #define DFLEXDPMLE1_DPMLETC_MASK(n) (0xf << (4 * (n))) >> > #define DFLEXDPMLE1_DPMLETC(n, x) ((x) << (4 * (n))) >> > >> > @@ -10988,17 +10995,17 @@ enum skl_power_gate { >> > _ICL_DSC1_RC_BUF_THRESH_1_UDW_PB, \ >> > _ICL_DSC1_RC_BUF_THRESH_1_UDW_PC) >> > >> > -#define PORT_TX_DFLEXDPSP _MMIO(0x1638A0) >> > +#define PORT_TX_DFLEXDPSP _MMIO(FIA1_BASE + PORT_TX_DFLEXDPSP_OFFSET) >> > #define TC_LIVE_STATE_TBT(tc_port) (1 << ((tc_port) * 8 + 6)) >> > #define TC_LIVE_STATE_TC(tc_port) (1 << ((tc_port) * 8 + 5)) >> > #define DP_LANE_ASSIGNMENT_SHIFT(tc_port) ((tc_port) * 8) >> > #define DP_LANE_ASSIGNMENT_MASK(tc_port) (0xf << ((tc_port) * 8)) >> > #define DP_LANE_ASSIGNMENT(tc_port, x) ((x) << ((tc_port) * 8)) >> > >> > -#define PORT_TX_DFLEXDPPMS _MMIO(0x163890) >> > +#define PORT_TX_DFLEXDPPMS _MMIO(FIA1_BASE + PORT_TX_DFLEXDPPMS_OFFSET) >> > #define DP_PHY_MODE_STATUS_COMPLETED(tc_port) (1 << (tc_port)) >> > >> > -#define PORT_TX_DFLEXDPCSSS _MMIO(0x163894) >> > +#define PORT_TX_DFLEXDPCSSS _MMIO(FIA1_BASE + PORT_TX_DFLEXDPCSSS_OFFSET) >> > #define DP_PHY_MODE_STATUS_NOT_SAFE(tc_port) (1 << (tc_port)) >> > >> > #endif /* _I915_REG_H_ */ >> >> -- >> Jani Nikula, Intel Open Source Graphics Center >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>-----Original Message----- >From: Jani Nikula [mailto:jani.nikula@linux.intel.com] >Sent: Wednesday, October 31, 2018 2:29 AM >To: Lucas De Marchi <lucas.de.marchi@gmail.com> >Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel- >gfx@lists.freedesktop.org; De Marchi, Lucas <lucas.demarchi@intel.com> >Subject: Re: [Intel-gfx] [PATCH] drm/i915/fia: FIA registers offset >implementation. > >On Tue, 30 Oct 2018, Lucas De Marchi <lucas.de.marchi@gmail.com> wrote: >> On Tue, Oct 30, 2018 at 6:56 AM Jani Nikula <jani.nikula@linux.intel.com> >wrote: >>> >>> On Mon, 29 Oct 2018, Anusha Srivatsa <anusha.srivatsa@intel.com> wrote: >>> > The registers DPCSSS,DPSP,DPMLE1 and DPPMS are all at an offset >>> > from the base - which is the FLexi IO Adaptor. Lets follow the >>> > offset calculation while accessing these registers. >>> >>> Why? >>> >>> If I search the specs or i915_reg.h for, say, 0x1638c0 I'll find what >>> I'm looking for. >>> >>> We generally don't follow this type of definitions for registers. We >>> may have some, but those are exceptions. >> >> Because spec 29550 treats those registers as a base + offset to be >> more future proof regarding a change of the base. And yes, I think >> something like that needs to be stated in the commit message. Is this enough? > >Fair enough. Please also see comments in-line. > >> >> Lucas De Marchi >> >>> >>> Please don't do this without some pretty good rationale written in >>> the commit message. >>> >>> BR, >>> Jani. >>> >>> > >>> > v2: >>> > - Follow spec for numbering - s/0/1(Lucas) >>> > - s/FIA_1/FIA1_BASE (Anusha) >>> > >>> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> >>> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> >>> > --- >>> > drivers/gpu/drm/i915/i915_reg.h | 15 +++++++++++---- >>> > 1 file changed, 11 insertions(+), 4 deletions(-) >>> > >>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h >>> > b/drivers/gpu/drm/i915/i915_reg.h index bcee91bcfba6..dd74bc01c64e >>> > 100644 >>> > --- a/drivers/gpu/drm/i915/i915_reg.h >>> > +++ b/drivers/gpu/drm/i915/i915_reg.h >>> > @@ -2057,8 +2057,15 @@ enum i915_power_well_id { >>> > #define BXT_PORT_CL2CM_DW6(phy) _BXT_PHY((phy), >_PORT_CL2CM_DW6_BC) >>> > #define DW6_OLDO_DYN_PWR_DOWN_EN (1 << 28) >>> > >>> > +/* FIA Offsets */ >>> > +#define FIA1_BASE 0x163000 > >Ok. > >>> > +#define PORT_TX_DFLEXDPMLE1_OFFSET 0x008C0 >>> > +#define PORT_TX_DFLEXDPPMS_OFFSET 0x00890 >>> > +#define PORT_TX_DFLEXDPCSSS_OFFSET 0x00894 >>> > +#define PORT_TX_DFLEXDPSP_OFFSET 0x008A0 > >These should not be grouped here, instead please add above each macro below >or just leave out, see below. > >>> > + >>> > /* ICL PHY DFLEX registers */ >>> > -#define PORT_TX_DFLEXDPMLE1 _MMIO(0x1638C0) >>> > +#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + >PORT_TX_DFLEXDPMLE1_OFFSET) > >IMO either: > >#define _PORT_TX_DFLEXDPMLE1 0x008C0 >#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + >_PORT_TX_DFLEXDPMLE1) > >or just: > >#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + 0x008C0) Makes sense. I ll make this change... Anusha > >BR, >Jani. > >>> > #define DFLEXDPMLE1_DPMLETC_MASK(n) (0xf << (4 * (n))) >>> > #define DFLEXDPMLE1_DPMLETC(n, x) ((x) << (4 * (n))) >>> > >>> > @@ -10988,17 +10995,17 @@ enum skl_power_gate { >>> > _ICL_DSC1_RC_BUF_THRESH_1_UDW_PB, \ >>> > >>> > _ICL_DSC1_RC_BUF_THRESH_1_UDW_PC) >>> > >>> > -#define PORT_TX_DFLEXDPSP _MMIO(0x1638A0) >>> > +#define PORT_TX_DFLEXDPSP _MMIO(FIA1_BASE + >PORT_TX_DFLEXDPSP_OFFSET) >>> > #define TC_LIVE_STATE_TBT(tc_port) (1 << ((tc_port) * 8 + 6)) >>> > #define TC_LIVE_STATE_TC(tc_port) (1 << ((tc_port) * 8 + 5)) >>> > #define DP_LANE_ASSIGNMENT_SHIFT(tc_port) ((tc_port) * 8) >>> > #define DP_LANE_ASSIGNMENT_MASK(tc_port) (0xf << ((tc_port) * 8)) >>> > #define DP_LANE_ASSIGNMENT(tc_port, x) ((x) << ((tc_port) * 8)) >>> > >>> > -#define PORT_TX_DFLEXDPPMS _MMIO(0x163890) >>> > +#define PORT_TX_DFLEXDPPMS _MMIO(FIA1_BASE + >PORT_TX_DFLEXDPPMS_OFFSET) >>> > #define DP_PHY_MODE_STATUS_COMPLETED(tc_port) (1 << >(tc_port)) >>> > >>> > -#define PORT_TX_DFLEXDPCSSS _MMIO(0x163894) >>> > +#define PORT_TX_DFLEXDPCSSS _MMIO(FIA1_BASE + >PORT_TX_DFLEXDPCSSS_OFFSET) >>> > #define DP_PHY_MODE_STATUS_NOT_SAFE(tc_port) (1 << >(tc_port)) >>> > >>> > #endif /* _I915_REG_H_ */ >>> >>> -- >>> Jani Nikula, Intel Open Source Graphics Center >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Jani Nikula, Intel Open Source Graphics Center
On Wed, Oct 31, 2018 at 04:58:27PM +0000, Srivatsa, Anusha wrote: > >>> > /* ICL PHY DFLEX registers */ > >>> > -#define PORT_TX_DFLEXDPMLE1 _MMIO(0x1638C0) > >>> > +#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + > >PORT_TX_DFLEXDPMLE1_OFFSET) > > > >IMO either: > > > >#define _PORT_TX_DFLEXDPMLE1 0x008C0 > >#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + > >_PORT_TX_DFLEXDPMLE1) > > > >or just: > > > >#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + 0x008C0) > > Makes sense. I ll make this change... I think the second option makes more sense. The additional underscored name doesn't improve readability. Lucas De Marchi
>-----Original Message----- >From: Lucas De Marchi [mailto:lucas.de.marchi@gmail.com] >Sent: Wednesday, October 31, 2018 10:23 AM >To: Srivatsa, Anusha <anusha.srivatsa@intel.com> >Cc: Jani Nikula <jani.nikula@linux.intel.com>; intel-gfx@lists.freedesktop.org; De >Marchi, Lucas <lucas.demarchi@intel.com> >Subject: Re: [Intel-gfx] [PATCH] drm/i915/fia: FIA registers offset >implementation. > >On Wed, Oct 31, 2018 at 04:58:27PM +0000, Srivatsa, Anusha wrote: >> >>> > /* ICL PHY DFLEX registers */ >> >>> > -#define PORT_TX_DFLEXDPMLE1 _MMIO(0x1638C0) >> >>> > +#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + >> >PORT_TX_DFLEXDPMLE1_OFFSET) >> > >> >IMO either: >> > >> >#define _PORT_TX_DFLEXDPMLE1 0x008C0 >> >#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + >> >_PORT_TX_DFLEXDPMLE1) >> > >> >or just: >> > >> >#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + 0x008C0) >> >> Makes sense. I ll make this change... > >I think the second option makes more sense. The additional underscored name >doesn't improve readability. Yes, so no special additional defines for offsets, just - #define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + 0x008C0) Anusha >Lucas De Marchi
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index bcee91bcfba6..dd74bc01c64e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2057,8 +2057,15 @@ enum i915_power_well_id { #define BXT_PORT_CL2CM_DW6(phy) _BXT_PHY((phy), _PORT_CL2CM_DW6_BC) #define DW6_OLDO_DYN_PWR_DOWN_EN (1 << 28) +/* FIA Offsets */ +#define FIA1_BASE 0x163000 +#define PORT_TX_DFLEXDPMLE1_OFFSET 0x008C0 +#define PORT_TX_DFLEXDPPMS_OFFSET 0x00890 +#define PORT_TX_DFLEXDPCSSS_OFFSET 0x00894 +#define PORT_TX_DFLEXDPSP_OFFSET 0x008A0 + /* ICL PHY DFLEX registers */ -#define PORT_TX_DFLEXDPMLE1 _MMIO(0x1638C0) +#define PORT_TX_DFLEXDPMLE1 _MMIO(FIA1_BASE + PORT_TX_DFLEXDPMLE1_OFFSET) #define DFLEXDPMLE1_DPMLETC_MASK(n) (0xf << (4 * (n))) #define DFLEXDPMLE1_DPMLETC(n, x) ((x) << (4 * (n))) @@ -10988,17 +10995,17 @@ enum skl_power_gate { _ICL_DSC1_RC_BUF_THRESH_1_UDW_PB, \ _ICL_DSC1_RC_BUF_THRESH_1_UDW_PC) -#define PORT_TX_DFLEXDPSP _MMIO(0x1638A0) +#define PORT_TX_DFLEXDPSP _MMIO(FIA1_BASE + PORT_TX_DFLEXDPSP_OFFSET) #define TC_LIVE_STATE_TBT(tc_port) (1 << ((tc_port) * 8 + 6)) #define TC_LIVE_STATE_TC(tc_port) (1 << ((tc_port) * 8 + 5)) #define DP_LANE_ASSIGNMENT_SHIFT(tc_port) ((tc_port) * 8) #define DP_LANE_ASSIGNMENT_MASK(tc_port) (0xf << ((tc_port) * 8)) #define DP_LANE_ASSIGNMENT(tc_port, x) ((x) << ((tc_port) * 8)) -#define PORT_TX_DFLEXDPPMS _MMIO(0x163890) +#define PORT_TX_DFLEXDPPMS _MMIO(FIA1_BASE + PORT_TX_DFLEXDPPMS_OFFSET) #define DP_PHY_MODE_STATUS_COMPLETED(tc_port) (1 << (tc_port)) -#define PORT_TX_DFLEXDPCSSS _MMIO(0x163894) +#define PORT_TX_DFLEXDPCSSS _MMIO(FIA1_BASE + PORT_TX_DFLEXDPCSSS_OFFSET) #define DP_PHY_MODE_STATUS_NOT_SAFE(tc_port) (1 << (tc_port)) #endif /* _I915_REG_H_ */
The registers DPCSSS,DPSP,DPMLE1 and DPPMS are all at an offset from the base - which is the FLexi IO Adaptor. Lets follow the offset calculation while accessing these registers. v2: - Follow spec for numbering - s/0/1(Lucas) - s/FIA_1/FIA1_BASE (Anusha) Cc: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)