[Intel-gfx] [PATCH 30/49] drm/i915/bxt: add display initialize/uninitialize sequence
From: Vandana Kannan
Add display clock/PHY initialization sequence as per BSpec.
Until GOP/VBIOS provides an upper limit value for CDCLK, comparing clock
value with 624 MHz and returning 0 in case it exceeds.
Note that the CD clock and PHY initialization/uninitialization are done
at their current place only for simplicity, in a future patch - when more
of the runtime PM features will be enabled - these will be moved to
power well#1 and modeset encoder enabling/disabling hooks respectively.
This also means that atm dynamic power gating power well #2 is
effectively disabled.
v1: Added function definitions in header files
v2: Imre's review comments addressed
- Moved CDCLK related definitions to i915_reg.h
- Removed defintions for CDCLK frequency
- Split uninit_cdclk() by adding a phy_uninit function
- Calculate freq and decimal based on input frequency
- Program SSA precharge based on input frequency
- Use wait_for 1ms instead 200us udelay for DE PLL locking
- Removed initial value for divider, freq, decimal, ratio.
- Replaced polling loops with wait_for
- Parameterized latency optim setting
- Fix the parts where DE PLL has to be disabled.
- Call CDCLK selection from mode set
v3: (imre)
- add note about the plan to move the cdclk/phy init to a better place
- take rps.hw_lock around pcode access
- fix DDI PHY timeout value
- squash in Vandana's "PORT_CL2CM_DW6_A BUN fix",
"DDI PHY programming register defn", "Do ddi_phy_init always",
"Check CDCLK upper limit" patches
- move PHY register macros next to the corresponding CHV/VLV macros
- move DE PLL register macros here from another patch since they are
used here first
- add BXT_ prefix to CDCLK flags
- s/COMMON_RESET/COMMON_RESET_DIS/ and clarify related code comments
- fix incorrect read value for the RMW of BXT_PHY_CTL_FAMILY_DDI
- fix using GT_DISPLAY_EDP_POWER_ON vs. GT_DISPLAY_DDI_POWER_ON
when powering on DDI ports
- fix incorrect port when setting BXT_PORT_TX_DW14_LN for DDI ports
- add missing masking when programming CDCLK_FREQ_DECIMAL
- add missing powering on for DDI-C port, rename OCL2_LDOFUSE_PWR_EN
to OCL2_LDOFUSE_PWR_DIS to reduce confusion
- add note about mismatch with bspec in the PORT_REF_DW6 fields
- factor out PHY init code to a new function, so we can call it for
PHY_A and PHY_BC, instead of open-coding the same
Signed-off-by: Vandana Kannan (v2)
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_reg.h | 126 +++
drivers/gpu/drm/i915/intel_ddi.c | 291 +++
drivers/gpu/drm/i915/intel_display.c | 75 +
drivers/gpu/drm/i915/intel_drv.h | 4 +
4 files changed, 496 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b4474d3..a3579c0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1120,6 +1120,110 @@ enum skl_disp_power_wells {
#define DPIO_FRC_LATENCY_SHFIT 8
#define CHV_TX_DW14(ch, lane) _TXLANE(ch, lane, 0xb8)
#define DPIO_UPAR_SHIFT 30
+
+/* BXT PHY registers */
+enum bxt_phy {
+ BXT_PHY_A,
+ BXT_PHY_BC
+};
+
+#define BXT_PHY(phy, a, b) ((a) + (phy) * ((b) - (a)))
+
+#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR 0x138090
+#define _EDP_POWER_ON(1 << 1)
+#define _DDI_POWER_ON(1 << 0)
+#define GT_DISPLAY_POWER_ON(phy) BXT_PHY(phy, _EDP_POWER_ON, \
+ _DDI_POWER_ON)
+
+#define _PHY_CTL_FAMILY_EDP0x64C80
+#define _PHY_CTL_FAMILY_DDI0x64C90
+#define COMMON_RESET_DIS (1 << 31)
+#define BXT_PHY_CTL_FAMILY(phy)BXT_PHY(phy,
_PHY_CTL_FAMILY_EDP, \
+_PHY_CTL_FAMILY_DDI)
+
+/* BXT PHY common lane registers */
+#define _PORT_CL1CM_DW0_A 0x162000
+#define _PORT_CL1CM_DW0_BC 0x6C000
+#define PHY_POWER_GOOD (1 << 16)
+#define BXT_PORT_CL1CM_DW0(phy)BXT_PHY(phy, _PORT_CL1CM_DW0_A,
\
+_PORT_CL1CM_DW0_BC)
+
+#define _PORT_CL1CM_DW9_A 0x162024
+#define _PORT_CL1CM_DW9_BC 0x6C024
+#define IREF0RC_OFFSET_SHIFT 8
+#define IREF0RC_OFFSET_MASK (0xFF << IREF0RC_OFFSET_SHIFT)
+#define BXT_PORT_CL1CM_DW9(phy)BXT_PHY(phy, _PORT_CL1CM_DW9_A,
\
+_PORT_CL1CM_DW9_BC)
+
+#define _PORT_CL1CM_DW10_A 0x162028
+#define _PORT_CL1CM_DW10_BC0x6C028
+#define IREF1RC_OFFSET_SHIFT 8
+#define IREF1RC_OFFSET_MASK (0xFF << IREF1RC_OFFSET_SHIFT)
+#define BXT_PORT_CL1CM_DW10(phy) BXT_PHY(phy, _PORT_CL1CM_DW10_A, \
+_PORT_CL1CM_DW10_BC)
+
+#define _PORT_CL1CM_DW28_A 0x162070
+#define _PORT_CL1
Re: [Intel-gfx] [PATCH 30/49] drm/i915/bxt: add display initialize/uninitialize sequence
On Tue, Mar 17, 2015 at 11:39:56AM +0200, Imre Deak wrote:
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index b91862e..ba2d1ae 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8284,6 +8284,75 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv)
> intel_prepare_ddi(dev);
> }
>
> +static int broxton_calc_cdclk(struct drm_i915_private *dev_priv,
> + int max_pixclk)
> +{
> + /*
> + * CDclks are supported:
> + * 144MHz
> + * 288MHz
> + * 384MHz
> + * 576MHz
> + * 624MHz
> + * Check to see whether we're above 90% of the lower bin and
> + * adjust if needed.
> + */
> +
> + /* If max_pixclk is greater than the max allowed clock, return 0.
> + * FIXME:- The max clock allowed needs to be provided by GOP/VBIOS
> + * via a scratch pad register. Till that is enabled, use 624MHz as max.
> + */
> + if (max_pixclk > 624000)
> + return 0;
> + else if (max_pixclk > 576000*9/10)
> + return 624000;
> + else if (max_pixclk > 384000*9/10)
> + return 576000;
> + else if (max_pixclk > 288000*9/10)
> + return 384000;
> + else if (max_pixclk > 144000*9/10)
Does BXT really need a 10% guarband for CDCLK? Other HSW+ platforms need no
guardband IIRC (assuming we ignore 64bpp and scaling).
I'm not seeing anything specific about BXT max pixel rate in BSpec.
The SKL section says EXCLUDE(BXT). Art, can you clarify?
> + return 288000;
> + else
> + return 144000;
> +}
> +
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 30/49] drm/i915/bxt: add display initialize/uninitialize sequence
On Tue, Mar 17, 2015 at 11:39:56AM +0200, Imre Deak wrote:
> From: Vandana Kannan
>
> Add display clock/PHY initialization sequence as per BSpec.
This should really be two patches I think. I'll go over the cdclk bits
first...
>
> Until GOP/VBIOS provides an upper limit value for CDCLK, comparing clock
> value with 624 MHz and returning 0 in case it exceeds.
>
> Note that the CD clock and PHY initialization/uninitialization are done
> at their current place only for simplicity, in a future patch - when more
> of the runtime PM features will be enabled - these will be moved to
> power well#1 and modeset encoder enabling/disabling hooks respectively.
> This also means that atm dynamic power gating power well #2 is
> effectively disabled.
>
> v1: Added function definitions in header files
> v2: Imre's review comments addressed
> - Moved CDCLK related definitions to i915_reg.h
> - Removed defintions for CDCLK frequency
> - Split uninit_cdclk() by adding a phy_uninit function
> - Calculate freq and decimal based on input frequency
> - Program SSA precharge based on input frequency
> - Use wait_for 1ms instead 200us udelay for DE PLL locking
> - Removed initial value for divider, freq, decimal, ratio.
> - Replaced polling loops with wait_for
> - Parameterized latency optim setting
> - Fix the parts where DE PLL has to be disabled.
> - Call CDCLK selection from mode set
>
> v3: (imre)
> - add note about the plan to move the cdclk/phy init to a better place
> - take rps.hw_lock around pcode access
> - fix DDI PHY timeout value
> - squash in Vandana's "PORT_CL2CM_DW6_A BUN fix",
> "DDI PHY programming register defn", "Do ddi_phy_init always",
> "Check CDCLK upper limit" patches
> - move PHY register macros next to the corresponding CHV/VLV macros
> - move DE PLL register macros here from another patch since they are
> used here first
> - add BXT_ prefix to CDCLK flags
> - s/COMMON_RESET/COMMON_RESET_DIS/ and clarify related code comments
> - fix incorrect read value for the RMW of BXT_PHY_CTL_FAMILY_DDI
> - fix using GT_DISPLAY_EDP_POWER_ON vs. GT_DISPLAY_DDI_POWER_ON
> when powering on DDI ports
> - fix incorrect port when setting BXT_PORT_TX_DW14_LN for DDI ports
> - add missing masking when programming CDCLK_FREQ_DECIMAL
> - add missing powering on for DDI-C port, rename OCL2_LDOFUSE_PWR_EN
> to OCL2_LDOFUSE_PWR_DIS to reduce confusion
> - add note about mismatch with bspec in the PORT_REF_DW6 fields
> - factor out PHY init code to a new function, so we can call it for
> PHY_A and PHY_BC, instead of open-coding the same
>
> Signed-off-by: Vandana Kannan (v2)
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/i915/i915_reg.h | 126 +++
> drivers/gpu/drm/i915/intel_ddi.c | 291
> +++
> drivers/gpu/drm/i915/intel_display.c | 75 +
> drivers/gpu/drm/i915/intel_drv.h | 4 +
> 4 files changed, 496 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b4474d3..a3579c0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5326,6 +5430,9 @@ enum skl_disp_power_wells {
> #define DISP_FBC_WM_DIS (1<<15)
> #define DISP_ARB_CTL20x45004
> #define DISP_DATA_PARTITION_5_6 (1<<6)
> +#define DBUF_CTL 0x45008
> +#define DBUF_POWER_REQUEST (1<<31)
> +#define DBUF_POWER_STATE(1<<30)
> #define GEN7_MSG_CTL 0x45010
> #define WAIT_FOR_PCH_RESET_ACK (1<<1)
> #define WAIT_FOR_PCH_FLR_ACK(1<<0)
> @@ -6271,6 +6378,7 @@ enum skl_disp_power_wells {
> #define GEN6_PCODE_WRITE_D_COMP0x11
> #define GEN6_ENCODE_RC6_VID(mv)(((mv) - 245) / 5)
> #define GEN6_DECODE_RC6_VID(vids) (((vids) * 5) + 245)
> +#define DISPLAY_PCU_CONTROL0x17
I called this HSW_PCODE_DE_WRITE_FREQ_REQ in my HSW/BDW cdclk
patches, which matches the name in Bspec for HSW/BDW.
Given our established practice of naming things based on the oldest
platform supporting them, I'd go with the HSW/BDW name.
> #define DISPLAY_IPS_CONTROL0x19
> #defineHSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A
> #define GEN6_PCODE_DATA 0x138128
> @@ -6748,6 +6856,13 @@ enum skl_disp_power_wells {
> #define CDCLK_FREQ_675_617 (3<<26)
> #define CDCLK_FREQ_DECIMAL_MASK (0x7ff)
>
> +#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 BXT_CDCLK_SSA_PRECHARGE_ENABLE (1<<16)
> +
> /* LCPLL_CTL */
> #define LCPLL1_CTL 0x46010
> #define LCPLL2_CTL 0x46014
> @@ -6812,6 +6927,17 @@ enum skl_disp_power_wells {
> #define GET_CFG_CR1_REG(id) (DPLL1_CFGCR1 + (id - SKL_DPLL1) *
Re: [Intel-gfx] [PATCH 30/49] drm/i915/bxt: add display initialize/uninitialize sequence
On Fri, 2015-03-20 at 16:10 +0200, Ville Syrjälä wrote:
> On Tue, Mar 17, 2015 at 11:39:56AM +0200, Imre Deak wrote:
> > From: Vandana Kannan
> >
> > Add display clock/PHY initialization sequence as per BSpec.
>
> This should really be two patches I think. I'll go over the cdclk bits
> first...
Yes, that would've been a better split. Other patches again like the
related register definitions needed to be squashed.
> > Until GOP/VBIOS provides an upper limit value for CDCLK, comparing clock
> > value with 624 MHz and returning 0 in case it exceeds.
> >
> > Note that the CD clock and PHY initialization/uninitialization are done
> > at their current place only for simplicity, in a future patch - when more
> > of the runtime PM features will be enabled - these will be moved to
> > power well#1 and modeset encoder enabling/disabling hooks respectively.
> > This also means that atm dynamic power gating power well #2 is
> > effectively disabled.
> >
> > v1: Added function definitions in header files
> > v2: Imre's review comments addressed
> > - Moved CDCLK related definitions to i915_reg.h
> > - Removed defintions for CDCLK frequency
> > - Split uninit_cdclk() by adding a phy_uninit function
> > - Calculate freq and decimal based on input frequency
> > - Program SSA precharge based on input frequency
> > - Use wait_for 1ms instead 200us udelay for DE PLL locking
> > - Removed initial value for divider, freq, decimal, ratio.
> > - Replaced polling loops with wait_for
> > - Parameterized latency optim setting
> > - Fix the parts where DE PLL has to be disabled.
> > - Call CDCLK selection from mode set
> >
> > v3: (imre)
> > - add note about the plan to move the cdclk/phy init to a better place
> > - take rps.hw_lock around pcode access
> > - fix DDI PHY timeout value
> > - squash in Vandana's "PORT_CL2CM_DW6_A BUN fix",
> > "DDI PHY programming register defn", "Do ddi_phy_init always",
> > "Check CDCLK upper limit" patches
> > - move PHY register macros next to the corresponding CHV/VLV macros
> > - move DE PLL register macros here from another patch since they are
> > used here first
> > - add BXT_ prefix to CDCLK flags
> > - s/COMMON_RESET/COMMON_RESET_DIS/ and clarify related code comments
> > - fix incorrect read value for the RMW of BXT_PHY_CTL_FAMILY_DDI
> > - fix using GT_DISPLAY_EDP_POWER_ON vs. GT_DISPLAY_DDI_POWER_ON
> > when powering on DDI ports
> > - fix incorrect port when setting BXT_PORT_TX_DW14_LN for DDI ports
> > - add missing masking when programming CDCLK_FREQ_DECIMAL
> > - add missing powering on for DDI-C port, rename OCL2_LDOFUSE_PWR_EN
> > to OCL2_LDOFUSE_PWR_DIS to reduce confusion
> > - add note about mismatch with bspec in the PORT_REF_DW6 fields
> > - factor out PHY init code to a new function, so we can call it for
> > PHY_A and PHY_BC, instead of open-coding the same
> >
> > Signed-off-by: Vandana Kannan (v2)
> > Signed-off-by: Imre Deak
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 126 +++
> > drivers/gpu/drm/i915/intel_ddi.c | 291
> > +++
> > drivers/gpu/drm/i915/intel_display.c | 75 +
> > drivers/gpu/drm/i915/intel_drv.h | 4 +
> > 4 files changed, 496 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index b4474d3..a3579c0 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>
> > @@ -5326,6 +5430,9 @@ enum skl_disp_power_wells {
> > #define DISP_FBC_WM_DIS (1<<15)
> > #define DISP_ARB_CTL2 0x45004
> > #define DISP_DATA_PARTITION_5_6 (1<<6)
> > +#define DBUF_CTL 0x45008
> > +#define DBUF_POWER_REQUEST(1<<31)
> > +#define DBUF_POWER_STATE (1<<30)
> > #define GEN7_MSG_CTL 0x45010
> > #define WAIT_FOR_PCH_RESET_ACK(1<<1)
> > #define WAIT_FOR_PCH_FLR_ACK (1<<0)
> > @@ -6271,6 +6378,7 @@ enum skl_disp_power_wells {
> > #define GEN6_PCODE_WRITE_D_COMP 0x11
> > #define GEN6_ENCODE_RC6_VID(mv) (((mv) - 245) / 5)
> > #define GEN6_DECODE_RC6_VID(vids)(((vids) * 5) + 245)
> > +#define DISPLAY_PCU_CONTROL 0x17
>
> I called this HSW_PCODE_DE_WRITE_FREQ_REQ in my HSW/BDW cdclk
> patches, which matches the name in Bspec for HSW/BDW.
>
> Given our established practice of naming things based on the oldest
> platform supporting them, I'd go with the HSW/BDW name.
Ok, will change it.
>
> > #define DISPLAY_IPS_CONTROL 0x19
> > #define HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A
> > #define GEN6_PCODE_DATA0x138128
> > @@ -6748,6 +6856,13 @@ enum skl_disp_power_wells {
> > #define CDCLK_FREQ_675_617(3<<26)
> > #define CDCLK_FREQ_DECIMAL_MASK (0x7ff)
> >
> > +#define BXT_CDCLK_CD2X_DIV_SEL_MASK (3<<22)
> > +#define BXT_CDCLK_CD2X_DIV_SEL_1 (0<<22)
> > +#define BXT_C
Re: [Intel-gfx] [PATCH 30/49] drm/i915/bxt: add display initialize/uninitialize sequence
On Tue, Mar 17, 2015 at 11:39:56AM +0200, Imre Deak wrote:
> From: Vandana Kannan
>
> Add display clock/PHY initialization sequence as per BSpec.
>
> Until GOP/VBIOS provides an upper limit value for CDCLK, comparing clock
> value with 624 MHz and returning 0 in case it exceeds.
>
> Note that the CD clock and PHY initialization/uninitialization are done
> at their current place only for simplicity, in a future patch - when more
> of the runtime PM features will be enabled - these will be moved to
> power well#1 and modeset encoder enabling/disabling hooks respectively.
> This also means that atm dynamic power gating power well #2 is
> effectively disabled.
OK, I've gone through the PHY stuff a bit now, and skipped the cdclk
stuff this time.
>
> v1: Added function definitions in header files
> v2: Imre's review comments addressed
> - Moved CDCLK related definitions to i915_reg.h
> - Removed defintions for CDCLK frequency
> - Split uninit_cdclk() by adding a phy_uninit function
> - Calculate freq and decimal based on input frequency
> - Program SSA precharge based on input frequency
> - Use wait_for 1ms instead 200us udelay for DE PLL locking
> - Removed initial value for divider, freq, decimal, ratio.
> - Replaced polling loops with wait_for
> - Parameterized latency optim setting
> - Fix the parts where DE PLL has to be disabled.
> - Call CDCLK selection from mode set
>
> v3: (imre)
> - add note about the plan to move the cdclk/phy init to a better place
> - take rps.hw_lock around pcode access
> - fix DDI PHY timeout value
> - squash in Vandana's "PORT_CL2CM_DW6_A BUN fix",
> "DDI PHY programming register defn", "Do ddi_phy_init always",
> "Check CDCLK upper limit" patches
> - move PHY register macros next to the corresponding CHV/VLV macros
> - move DE PLL register macros here from another patch since they are
> used here first
> - add BXT_ prefix to CDCLK flags
> - s/COMMON_RESET/COMMON_RESET_DIS/ and clarify related code comments
> - fix incorrect read value for the RMW of BXT_PHY_CTL_FAMILY_DDI
> - fix using GT_DISPLAY_EDP_POWER_ON vs. GT_DISPLAY_DDI_POWER_ON
> when powering on DDI ports
> - fix incorrect port when setting BXT_PORT_TX_DW14_LN for DDI ports
> - add missing masking when programming CDCLK_FREQ_DECIMAL
> - add missing powering on for DDI-C port, rename OCL2_LDOFUSE_PWR_EN
> to OCL2_LDOFUSE_PWR_DIS to reduce confusion
> - add note about mismatch with bspec in the PORT_REF_DW6 fields
> - factor out PHY init code to a new function, so we can call it for
> PHY_A and PHY_BC, instead of open-coding the same
>
> Signed-off-by: Vandana Kannan (v2)
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/i915/i915_reg.h | 126 +++
> drivers/gpu/drm/i915/intel_ddi.c | 291
> +++
> drivers/gpu/drm/i915/intel_display.c | 75 +
> drivers/gpu/drm/i915/intel_drv.h | 4 +
> 4 files changed, 496 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b4474d3..a3579c0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1120,6 +1120,110 @@ enum skl_disp_power_wells {
> #define DPIO_FRC_LATENCY_SHFIT 8
> #define CHV_TX_DW14(ch, lane) _TXLANE(ch, lane, 0xb8)
> #define DPIO_UPAR_SHIFT30
> +
> +/* BXT PHY registers */
> +enum bxt_phy {
> + BXT_PHY_A,
> + BXT_PHY_BC
> +};
We have enum dpio_phy already. Although here we have defined 0 to be the
single channel PHY and 1 is the two channel PHY, whereas on CHV it's
the other way around. I'm going to suggest we flip BXT over to use the
CHV scheme to avoid any surpises later if we actualy try to unify the
code.
> +
> +#define BXT_PHY(phy, a, b) ((a) + (phy) * ((b) - (a)))
This seems to be just another _PIPE(), should at least have an
underscore so that people don't confuse it with something they are
supposed to use.
> +
> +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR0x138090
We can drop the _0_2_0_GTTMMADR suffix, we've never included it for any
other platforms either.
> +#define _EDP_POWER_ON (1 << 1)
> +#define _DDI_POWER_ON (1 << 0)
> +#define GT_DISPLAY_POWER_ON(phy) BXT_PHY(phy, _EDP_POWER_ON, \
> + _DDI_POWER_ON)
Using a _PIPE() type of macro for register bits seems a bit unusual.
I'd just open code it as (1 << (phy)) or something. That also work
better if we the PHYs around so that PHY0 is the dual channel PHY.
> +
> +#define _PHY_CTL_FAMILY_EDP 0x64C80
> +#define _PHY_CTL_FAMILY_DDI 0x64C90
> +#define COMMON_RESET_DIS (1 << 31)
> +#define BXT_PHY_CTL_FAMILY(phy) BXT_PHY(phy,
> _PHY_CTL_FAMILY_EDP, \
> + _PHY_CTL_FAMILY_DDI)
> +
> +/* BXT PHY common lane registers */
> +#define _PORT_CL1CM_DW0_A0x16200
Re: [Intel-gfx] [PATCH 30/49] drm/i915/bxt: add display initialize/uninitialize sequence
On to, 2015-04-02 at 19:32 +0300, Ville Syrjälä wrote:
> On Tue, Mar 17, 2015 at 11:39:56AM +0200, Imre Deak wrote:
> > From: Vandana Kannan
> >
> > Add display clock/PHY initialization sequence as per BSpec.
> >
> > Until GOP/VBIOS provides an upper limit value for CDCLK, comparing clock
> > value with 624 MHz and returning 0 in case it exceeds.
> >
> > Note that the CD clock and PHY initialization/uninitialization are done
> > at their current place only for simplicity, in a future patch - when more
> > of the runtime PM features will be enabled - these will be moved to
> > power well#1 and modeset encoder enabling/disabling hooks respectively.
> > This also means that atm dynamic power gating power well #2 is
> > effectively disabled.
>
> OK, I've gone through the PHY stuff a bit now, and skipped the cdclk
> stuff this time.
>
> >
> > v1: Added function definitions in header files
> > v2: Imre's review comments addressed
> > - Moved CDCLK related definitions to i915_reg.h
> > - Removed defintions for CDCLK frequency
> > - Split uninit_cdclk() by adding a phy_uninit function
> > - Calculate freq and decimal based on input frequency
> > - Program SSA precharge based on input frequency
> > - Use wait_for 1ms instead 200us udelay for DE PLL locking
> > - Removed initial value for divider, freq, decimal, ratio.
> > - Replaced polling loops with wait_for
> > - Parameterized latency optim setting
> > - Fix the parts where DE PLL has to be disabled.
> > - Call CDCLK selection from mode set
> >
> > v3: (imre)
> > - add note about the plan to move the cdclk/phy init to a better place
> > - take rps.hw_lock around pcode access
> > - fix DDI PHY timeout value
> > - squash in Vandana's "PORT_CL2CM_DW6_A BUN fix",
> > "DDI PHY programming register defn", "Do ddi_phy_init always",
> > "Check CDCLK upper limit" patches
> > - move PHY register macros next to the corresponding CHV/VLV macros
> > - move DE PLL register macros here from another patch since they are
> > used here first
> > - add BXT_ prefix to CDCLK flags
> > - s/COMMON_RESET/COMMON_RESET_DIS/ and clarify related code comments
> > - fix incorrect read value for the RMW of BXT_PHY_CTL_FAMILY_DDI
> > - fix using GT_DISPLAY_EDP_POWER_ON vs. GT_DISPLAY_DDI_POWER_ON
> > when powering on DDI ports
> > - fix incorrect port when setting BXT_PORT_TX_DW14_LN for DDI ports
> > - add missing masking when programming CDCLK_FREQ_DECIMAL
> > - add missing powering on for DDI-C port, rename OCL2_LDOFUSE_PWR_EN
> > to OCL2_LDOFUSE_PWR_DIS to reduce confusion
> > - add note about mismatch with bspec in the PORT_REF_DW6 fields
> > - factor out PHY init code to a new function, so we can call it for
> > PHY_A and PHY_BC, instead of open-coding the same
> >
> > Signed-off-by: Vandana Kannan (v2)
> > Signed-off-by: Imre Deak
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 126 +++
> > drivers/gpu/drm/i915/intel_ddi.c | 291
> > +++
> > drivers/gpu/drm/i915/intel_display.c | 75 +
> > drivers/gpu/drm/i915/intel_drv.h | 4 +
> > 4 files changed, 496 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index b4474d3..a3579c0 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1120,6 +1120,110 @@ enum skl_disp_power_wells {
> > #define DPIO_FRC_LATENCY_SHFIT 8
> > #define CHV_TX_DW14(ch, lane) _TXLANE(ch, lane, 0xb8)
> > #define DPIO_UPAR_SHIFT 30
> > +
> > +/* BXT PHY registers */
> > +enum bxt_phy {
> > + BXT_PHY_A,
> > + BXT_PHY_BC
> > +};
>
> We have enum dpio_phy already. Although here we have defined 0 to be the
> single channel PHY and 1 is the two channel PHY, whereas on CHV it's
> the other way around. I'm going to suggest we flip BXT over to use the
> CHV scheme to avoid any surpises later if we actualy try to unify the
> code.
Yea, I haven't noticed it, will use that instead.
> > +
> > +#define BXT_PHY(phy, a, b) ((a) + (phy) * ((b) - (a)))
>
> This seems to be just another _PIPE(), should at least have an
> underscore so that people don't confuse it with something they are
> supposed to use.
Ok, I can rewrite this to
#define _BXT_PHY(phy, a, b) _PIPE(phy, a, b)
>
> > +
> > +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR 0x138090
>
> We can drop the _0_2_0_GTTMMADR suffix, we've never included it for any
> other platforms either.
Ok.
> > +#define _EDP_POWER_ON(1 << 1)
> > +#define _DDI_POWER_ON(1 << 0)
> > +#define GT_DISPLAY_POWER_ON(phy) BXT_PHY(phy, _EDP_POWER_ON, \
> > + _DDI_POWER_ON)
>
> Using a _PIPE() type of macro for register bits seems a bit unusual.
> I'd just open code it as (1 << (phy)) or something. That also work
> better if we the PHYs around so that PHY0 is the dual channel PHY.
Ok, will rewrite
