Fixing the 5th Display output for DG2.
Jouni Högander (1): drm/i915: Fix for PHY_MISC_TC1 offset
Matt Roper (2): drm/i915/dg2: Enable 5th display drm/i915/dg2: Drop 38.4 MHz MPLLB tables
drivers/gpu/drm/i915/display/intel_gmbus.c | 16 +- drivers/gpu/drm/i915/display/intel_snps_phy.c | 210 +----------------- drivers/gpu/drm/i915/i915_irq.c | 5 +- drivers/gpu/drm/i915/i915_reg.h | 7 +- 4 files changed, 25 insertions(+), 213 deletions(-)
From: Matt Roper matthew.d.roper@intel.com
DG2 supports a 5th display output which the hardware refers to as "TC1," even though it isn't a Type-C output. This behaves similarly to the TC1 on past platforms with just a couple minor differences:
* DG2's TC1 bit in SDEISR is at bit 25 rather than 24 as it is on ICP/TGP/ADP. * DG2 doesn't need the hpd inversion setting that we had to use on DG1
Cc: Swathi Dhanavanthri swathi.dhanavanthri@intel.com Cc: Lucas De Marchi lucas.demarchi@intel.com Cc: José Roberto de Souza jose.souza@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com --- drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++++++++++++++-- drivers/gpu/drm/i915/i915_irq.c | 5 ++++- drivers/gpu/drm/i915/i915_reg.h | 1 + 3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c index 6ce8c10fe975..2fad03250661 100644 --- a/drivers/gpu/drm/i915/display/intel_gmbus.c +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c @@ -98,11 +98,21 @@ static const struct gmbus_pin gmbus_pins_dg1[] = { [GMBUS_PIN_4_CNP] = { "dpd", GPIOE }, };
+static const struct gmbus_pin gmbus_pins_dg2[] = { + [GMBUS_PIN_1_BXT] = { "dpa", GPIOB }, + [GMBUS_PIN_2_BXT] = { "dpb", GPIOC }, + [GMBUS_PIN_3_BXT] = { "dpc", GPIOD }, + [GMBUS_PIN_4_CNP] = { "dpd", GPIOE }, + [GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOJ }, +}; + /* pin is expected to be valid */ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv, unsigned int pin) { - if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) + if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2) + return &gmbus_pins_dg2[pin]; + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) return &gmbus_pins_dg1[pin]; else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) return &gmbus_pins_icp[pin]; @@ -123,7 +133,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv, { unsigned int size;
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) + if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2) + size = ARRAY_SIZE(gmbus_pins_dg2); + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) size = ARRAY_SIZE(gmbus_pins_dg1); else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) size = ARRAY_SIZE(gmbus_pins_icp); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index fdd568ba4a16..4d81063b128c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -179,6 +179,7 @@ static const u32 hpd_sde_dg1[HPD_NUM_PINS] = { [HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_B), [HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_C), [HPD_PORT_D] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_D), + [HPD_PORT_TC1] = SDE_TC_HOTPLUG_DG2(HPD_PORT_TC1), };
static void intel_hpd_init_pins(struct drm_i915_private *dev_priv) @@ -4424,7 +4425,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv) if (I915_HAS_HOTPLUG(dev_priv)) dev_priv->hotplug_funcs = &i915_hpd_funcs; } else { - if (HAS_PCH_DG1(dev_priv)) + if (HAS_PCH_DG2(dev_priv)) + dev_priv->hotplug_funcs = &icp_hpd_funcs; + else if (HAS_PCH_DG1(dev_priv)) dev_priv->hotplug_funcs = &dg1_hpd_funcs; else if (DISPLAY_VER(dev_priv) >= 11) dev_priv->hotplug_funcs = &gen11_hpd_funcs; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4ea1713e6b60..4d12abb2d7ff 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6182,6 +6182,7 @@ /* south display engine interrupt: ICP/TGP */ #define SDE_GMBUS_ICP (1 << 23) #define SDE_TC_HOTPLUG_ICP(hpd_pin) REG_BIT(24 + _HPD_PIN_TC(hpd_pin)) +#define SDE_TC_HOTPLUG_DG2(hpd_pin) REG_BIT(25 + _HPD_PIN_TC(hpd_pin)) /* sigh */ #define SDE_DDI_HOTPLUG_ICP(hpd_pin) REG_BIT(16 + _HPD_PIN_DDI(hpd_pin)) #define SDE_DDI_HOTPLUG_MASK_ICP (SDE_DDI_HOTPLUG_ICP(HPD_PORT_D) | \ SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \
-----Original Message----- From: C, Ramalingam ramalingam.c@intel.com Sent: Tuesday, February 15, 2022 11:22 AM To: intel-gfx intel-gfx@lists.freedesktop.org; dri-devel <dri- devel@lists.freedesktop.org> Cc: Ville Syrjälä ville.syrjala@linux.intel.com; Shankar, Uma uma.shankar@intel.com; Roper, Matthew D matthew.d.roper@intel.com; Dhanavanthri, Swathi swathi.dhanavanthri@intel.com; De Marchi, Lucas lucas.demarchi@intel.com; Souza, Jose jose.souza@intel.com; C, Ramalingam ramalingam.c@intel.com Subject: [PATCH 1/3] drm/i915/dg2: Enable 5th display
From: Matt Roper matthew.d.roper@intel.com
DG2 supports a 5th display output which the hardware refers to as "TC1," even though it isn't a Type-C output. This behaves similarly to the TC1 on past platforms with just a couple minor differences:
- DG2's TC1 bit in SDEISR is at bit 25 rather than 24 as it is on ICP/TGP/ADP.
- DG2 doesn't need the hpd inversion setting that we had to use on DG1
Cc: Swathi Dhanavanthri swathi.dhanavanthri@intel.com Cc: Lucas De Marchi lucas.demarchi@intel.com Cc: José Roberto de Souza jose.souza@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com
drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++++++++++++++-- drivers/gpu/drm/i915/i915_irq.c | 5 ++++- drivers/gpu/drm/i915/i915_reg.h | 1 + 3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c index 6ce8c10fe975..2fad03250661 100644 --- a/drivers/gpu/drm/i915/display/intel_gmbus.c +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c @@ -98,11 +98,21 @@ static const struct gmbus_pin gmbus_pins_dg1[] = { [GMBUS_PIN_4_CNP] = { "dpd", GPIOE }, };
+static const struct gmbus_pin gmbus_pins_dg2[] = {
- [GMBUS_PIN_1_BXT] = { "dpa", GPIOB },
- [GMBUS_PIN_2_BXT] = { "dpb", GPIOC },
- [GMBUS_PIN_3_BXT] = { "dpc", GPIOD },
- [GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
- [GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOJ }, };
/* pin is expected to be valid */ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv, unsigned int pin) {
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
return &gmbus_pins_dg2[pin];- else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) return &gmbus_pins_dg1[pin]; else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) return &gmbus_pins_icp[pin];
@@ -123,7 +133,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv, { unsigned int size;
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
size = ARRAY_SIZE(gmbus_pins_dg2);- else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) size = ARRAY_SIZE(gmbus_pins_dg1); else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) size = ARRAY_SIZE(gmbus_pins_icp);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index fdd568ba4a16..4d81063b128c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -179,6 +179,7 @@ static const u32 hpd_sde_dg1[HPD_NUM_PINS] = { [HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_B), [HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_C), [HPD_PORT_D] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_D),
- [HPD_PORT_TC1] = SDE_TC_HOTPLUG_DG2(HPD_PORT_TC1),
Not sure if this applies to DG1, the 5th TC1 port is only for DG2. Should we not have a separate DG2 struct. Can you please clarify to help understand.
Regards, Uma Shankar
};
static void intel_hpd_init_pins(struct drm_i915_private *dev_priv) @@ -4424,7 +4425,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv) if (I915_HAS_HOTPLUG(dev_priv)) dev_priv->hotplug_funcs = &i915_hpd_funcs; } else {
if (HAS_PCH_DG1(dev_priv))
if (HAS_PCH_DG2(dev_priv))dev_priv->hotplug_funcs = &icp_hpd_funcs;else if (HAS_PCH_DG1(dev_priv)) dev_priv->hotplug_funcs = &dg1_hpd_funcs;a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4ea1713e6b60..4d12abb2d7ff 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6182,6 +6182,7 @@ /* south display engine interrupt: ICP/TGP */ #define SDE_GMBUS_ICP (1 << 23) #define SDE_TC_HOTPLUG_ICP(hpd_pin) REG_BIT(24 + _HPD_PIN_TC(hpd_pin)) +#define SDE_TC_HOTPLUG_DG2(hpd_pin) REG_BIT(25 + _HPD_PIN_TC(hpd_pin)) /* sigh */ #define SDE_DDI_HOTPLUG_ICP(hpd_pin) REG_BIT(16 + _HPD_PIN_DDI(hpd_pin)) #define SDE_DDI_HOTPLUG_MASK_ICP (SDE_DDI_HOTPLUG_ICP(HPD_PORT_D) | \ SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \ -- 2.20.1
On Wed, Feb 16, 2022 at 12:02:31AM -0800, Shankar, Uma wrote:
-----Original Message----- From: C, Ramalingam ramalingam.c@intel.com Sent: Tuesday, February 15, 2022 11:22 AM To: intel-gfx intel-gfx@lists.freedesktop.org; dri-devel <dri- devel@lists.freedesktop.org> Cc: Ville Syrjälä ville.syrjala@linux.intel.com; Shankar, Uma uma.shankar@intel.com; Roper, Matthew D matthew.d.roper@intel.com; Dhanavanthri, Swathi swathi.dhanavanthri@intel.com; De Marchi, Lucas lucas.demarchi@intel.com; Souza, Jose jose.souza@intel.com; C, Ramalingam ramalingam.c@intel.com Subject: [PATCH 1/3] drm/i915/dg2: Enable 5th display
From: Matt Roper matthew.d.roper@intel.com
DG2 supports a 5th display output which the hardware refers to as "TC1," even though it isn't a Type-C output. This behaves similarly to the TC1 on past platforms with just a couple minor differences:
- DG2's TC1 bit in SDEISR is at bit 25 rather than 24 as it is on ICP/TGP/ADP.
- DG2 doesn't need the hpd inversion setting that we had to use on DG1
Cc: Swathi Dhanavanthri swathi.dhanavanthri@intel.com Cc: Lucas De Marchi lucas.demarchi@intel.com Cc: José Roberto de Souza jose.souza@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com
drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++++++++++++++-- drivers/gpu/drm/i915/i915_irq.c | 5 ++++- drivers/gpu/drm/i915/i915_reg.h | 1 + 3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c index 6ce8c10fe975..2fad03250661 100644 --- a/drivers/gpu/drm/i915/display/intel_gmbus.c +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c @@ -98,11 +98,21 @@ static const struct gmbus_pin gmbus_pins_dg1[] = { [GMBUS_PIN_4_CNP] = { "dpd", GPIOE }, };
+static const struct gmbus_pin gmbus_pins_dg2[] = {
- [GMBUS_PIN_1_BXT] = { "dpa", GPIOB },
- [GMBUS_PIN_2_BXT] = { "dpb", GPIOC },
- [GMBUS_PIN_3_BXT] = { "dpc", GPIOD },
- [GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
- [GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOJ }, };
/* pin is expected to be valid */ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv, unsigned int pin) {
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
return &gmbus_pins_dg2[pin];- else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) return &gmbus_pins_dg1[pin]; else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) return &gmbus_pins_icp[pin];
@@ -123,7 +133,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv, { unsigned int size;
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
size = ARRAY_SIZE(gmbus_pins_dg2);- else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) size = ARRAY_SIZE(gmbus_pins_dg1); else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) size = ARRAY_SIZE(gmbus_pins_icp);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index fdd568ba4a16..4d81063b128c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -179,6 +179,7 @@ static const u32 hpd_sde_dg1[HPD_NUM_PINS] = { [HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_B), [HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_C), [HPD_PORT_D] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_D),
- [HPD_PORT_TC1] = SDE_TC_HOTPLUG_DG2(HPD_PORT_TC1),
Not sure if this applies to DG1, the 5th TC1 port is only for DG2. Should we not have a separate DG2 struct. Can you please clarify to help understand.
It's fine to add this to the DG1 structure since it's just a mapping table. DG1 will never use TC1 as an input into the table, but there aren't any conflicting mappings between DG1/DG2 so we can use the same table for both.
Matt
Regards, Uma Shankar
};
static void intel_hpd_init_pins(struct drm_i915_private *dev_priv) @@ -4424,7 +4425,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv) if (I915_HAS_HOTPLUG(dev_priv)) dev_priv->hotplug_funcs = &i915_hpd_funcs; } else {
if (HAS_PCH_DG1(dev_priv))
if (HAS_PCH_DG2(dev_priv))dev_priv->hotplug_funcs = &icp_hpd_funcs;else if (HAS_PCH_DG1(dev_priv)) dev_priv->hotplug_funcs = &dg1_hpd_funcs;a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4ea1713e6b60..4d12abb2d7ff 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6182,6 +6182,7 @@ /* south display engine interrupt: ICP/TGP */ #define SDE_GMBUS_ICP (1 << 23) #define SDE_TC_HOTPLUG_ICP(hpd_pin) REG_BIT(24 + _HPD_PIN_TC(hpd_pin)) +#define SDE_TC_HOTPLUG_DG2(hpd_pin) REG_BIT(25 + _HPD_PIN_TC(hpd_pin)) /* sigh */ #define SDE_DDI_HOTPLUG_ICP(hpd_pin) REG_BIT(16 + _HPD_PIN_DDI(hpd_pin)) #define SDE_DDI_HOTPLUG_MASK_ICP (SDE_DDI_HOTPLUG_ICP(HPD_PORT_D) | \ SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \ -- 2.20.1
Since it apparently caused some confusion on various websites, maybe we should change the title of the patch to "Enable 5th port" to make it more clear that this is only a port, not a pipe.
Also, I believe one last line that we need to add to this patch is an intel_ddi_init() call for TC1 in the intel_setup_outputs() function. I think I previously had that in a separate patch, but it went missing when we reorganized and refactored some of these patches
Matt
On Tue, Feb 15, 2022 at 11:21:52AM +0530, Ramalingam C wrote:
From: Matt Roper matthew.d.roper@intel.com
DG2 supports a 5th display output which the hardware refers to as "TC1," even though it isn't a Type-C output. This behaves similarly to the TC1 on past platforms with just a couple minor differences:
- DG2's TC1 bit in SDEISR is at bit 25 rather than 24 as it is on ICP/TGP/ADP.
- DG2 doesn't need the hpd inversion setting that we had to use on DG1
Cc: Swathi Dhanavanthri swathi.dhanavanthri@intel.com Cc: Lucas De Marchi lucas.demarchi@intel.com Cc: José Roberto de Souza jose.souza@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com
drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++++++++++++++-- drivers/gpu/drm/i915/i915_irq.c | 5 ++++- drivers/gpu/drm/i915/i915_reg.h | 1 + 3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c index 6ce8c10fe975..2fad03250661 100644 --- a/drivers/gpu/drm/i915/display/intel_gmbus.c +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c @@ -98,11 +98,21 @@ static const struct gmbus_pin gmbus_pins_dg1[] = { [GMBUS_PIN_4_CNP] = { "dpd", GPIOE }, };
+static const struct gmbus_pin gmbus_pins_dg2[] = {
- [GMBUS_PIN_1_BXT] = { "dpa", GPIOB },
- [GMBUS_PIN_2_BXT] = { "dpb", GPIOC },
- [GMBUS_PIN_3_BXT] = { "dpc", GPIOD },
- [GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
- [GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOJ },
+};
/* pin is expected to be valid */ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv, unsigned int pin) {
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
return &gmbus_pins_dg2[pin];- else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) return &gmbus_pins_dg1[pin]; else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) return &gmbus_pins_icp[pin];
@@ -123,7 +133,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv, { unsigned int size;
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
size = ARRAY_SIZE(gmbus_pins_dg2);- else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) size = ARRAY_SIZE(gmbus_pins_dg1); else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) size = ARRAY_SIZE(gmbus_pins_icp);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index fdd568ba4a16..4d81063b128c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -179,6 +179,7 @@ static const u32 hpd_sde_dg1[HPD_NUM_PINS] = { [HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_B), [HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_C), [HPD_PORT_D] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_D),
- [HPD_PORT_TC1] = SDE_TC_HOTPLUG_DG2(HPD_PORT_TC1),
};
static void intel_hpd_init_pins(struct drm_i915_private *dev_priv) @@ -4424,7 +4425,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv) if (I915_HAS_HOTPLUG(dev_priv)) dev_priv->hotplug_funcs = &i915_hpd_funcs; } else {
if (HAS_PCH_DG1(dev_priv))
if (HAS_PCH_DG2(dev_priv))dev_priv->hotplug_funcs = &icp_hpd_funcs;else if (HAS_PCH_DG1(dev_priv)) dev_priv->hotplug_funcs = &dg1_hpd_funcs;diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4ea1713e6b60..4d12abb2d7ff 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6182,6 +6182,7 @@ /* south display engine interrupt: ICP/TGP */ #define SDE_GMBUS_ICP (1 << 23) #define SDE_TC_HOTPLUG_ICP(hpd_pin) REG_BIT(24 + _HPD_PIN_TC(hpd_pin)) +#define SDE_TC_HOTPLUG_DG2(hpd_pin) REG_BIT(25 + _HPD_PIN_TC(hpd_pin)) /* sigh */ #define SDE_DDI_HOTPLUG_ICP(hpd_pin) REG_BIT(16 + _HPD_PIN_DDI(hpd_pin)) #define SDE_DDI_HOTPLUG_MASK_ICP (SDE_DDI_HOTPLUG_ICP(HPD_PORT_D) | \ SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \ -- 2.20.1
On 2022-02-17 at 08:37:47 -0800, Matt Roper wrote:
Since it apparently caused some confusion on various websites, maybe we should change the title of the patch to "Enable 5th port" to make it more clear that this is only a port, not a pipe.
Ok sure.
Also, I believe one last line that we need to add to this patch is an intel_ddi_init() call for TC1 in the intel_setup_outputs() function. I think I previously had that in a separate patch, but it went missing when we reorganized and refactored some of these patches
like
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 59961621fe4a..18531ffd4789 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -8760,6 +8760,7 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv) intel_ddi_init(dev_priv, PORT_B); intel_ddi_init(dev_priv, PORT_C); intel_ddi_init(dev_priv, PORT_D_XELPD); + intel_ddi_init(dev_priv, PORT_TC1); } else if (IS_ALDERLAKE_P(dev_priv)) { intel_ddi_init(dev_priv, PORT_A); intel_ddi_init(dev_priv, PORT_B);
Ram
Matt
On Tue, Feb 15, 2022 at 11:21:52AM +0530, Ramalingam C wrote:
From: Matt Roper matthew.d.roper@intel.com
DG2 supports a 5th display output which the hardware refers to as "TC1," even though it isn't a Type-C output. This behaves similarly to the TC1 on past platforms with just a couple minor differences:
- DG2's TC1 bit in SDEISR is at bit 25 rather than 24 as it is on ICP/TGP/ADP.
- DG2 doesn't need the hpd inversion setting that we had to use on DG1
Cc: Swathi Dhanavanthri swathi.dhanavanthri@intel.com Cc: Lucas De Marchi lucas.demarchi@intel.com Cc: José Roberto de Souza jose.souza@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com
drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++++++++++++++-- drivers/gpu/drm/i915/i915_irq.c | 5 ++++- drivers/gpu/drm/i915/i915_reg.h | 1 + 3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c index 6ce8c10fe975..2fad03250661 100644 --- a/drivers/gpu/drm/i915/display/intel_gmbus.c +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c @@ -98,11 +98,21 @@ static const struct gmbus_pin gmbus_pins_dg1[] = { [GMBUS_PIN_4_CNP] = { "dpd", GPIOE }, };
+static const struct gmbus_pin gmbus_pins_dg2[] = {
- [GMBUS_PIN_1_BXT] = { "dpa", GPIOB },
- [GMBUS_PIN_2_BXT] = { "dpb", GPIOC },
- [GMBUS_PIN_3_BXT] = { "dpc", GPIOD },
- [GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
- [GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOJ },
+};
/* pin is expected to be valid */ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv, unsigned int pin) {
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
return &gmbus_pins_dg2[pin];- else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) return &gmbus_pins_dg1[pin]; else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) return &gmbus_pins_icp[pin];
@@ -123,7 +133,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv, { unsigned int size;
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
size = ARRAY_SIZE(gmbus_pins_dg2);- else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) size = ARRAY_SIZE(gmbus_pins_dg1); else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) size = ARRAY_SIZE(gmbus_pins_icp);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index fdd568ba4a16..4d81063b128c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -179,6 +179,7 @@ static const u32 hpd_sde_dg1[HPD_NUM_PINS] = { [HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_B), [HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_C), [HPD_PORT_D] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_D),
- [HPD_PORT_TC1] = SDE_TC_HOTPLUG_DG2(HPD_PORT_TC1),
};
static void intel_hpd_init_pins(struct drm_i915_private *dev_priv) @@ -4424,7 +4425,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv) if (I915_HAS_HOTPLUG(dev_priv)) dev_priv->hotplug_funcs = &i915_hpd_funcs; } else {
if (HAS_PCH_DG1(dev_priv))
if (HAS_PCH_DG2(dev_priv))dev_priv->hotplug_funcs = &icp_hpd_funcs;else if (HAS_PCH_DG1(dev_priv)) dev_priv->hotplug_funcs = &dg1_hpd_funcs;diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4ea1713e6b60..4d12abb2d7ff 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6182,6 +6182,7 @@ /* south display engine interrupt: ICP/TGP */ #define SDE_GMBUS_ICP (1 << 23) #define SDE_TC_HOTPLUG_ICP(hpd_pin) REG_BIT(24 + _HPD_PIN_TC(hpd_pin)) +#define SDE_TC_HOTPLUG_DG2(hpd_pin) REG_BIT(25 + _HPD_PIN_TC(hpd_pin)) /* sigh */ #define SDE_DDI_HOTPLUG_ICP(hpd_pin) REG_BIT(16 + _HPD_PIN_DDI(hpd_pin)) #define SDE_DDI_HOTPLUG_MASK_ICP (SDE_DDI_HOTPLUG_ICP(HPD_PORT_D) | \ SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \ -- 2.20.1
-- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795
From: Matt Roper matthew.d.roper@intel.com
DG2 supports a 5th display output which the hardware refers to as "TC1," even though it isn't a Type-C output. This behaves similarly to the TC1 on past platforms with just a couple minor differences:
* DG2's TC1 bit in SDEISR is at bit 25 rather than 24 as it is on ICP/TGP/ADP. * DG2 doesn't need the hpd inversion setting that we had to use on DG1
v2: intel_ddi_init(dev_priv, PORT_TC1); [Matt]
Cc: Swathi Dhanavanthri swathi.dhanavanthri@intel.com Cc: Lucas De Marchi lucas.demarchi@intel.com Cc: José Roberto de Souza jose.souza@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com --- drivers/gpu/drm/i915/display/intel_display.c | 1 + drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++++++++++++++-- drivers/gpu/drm/i915/i915_irq.c | 5 ++++- drivers/gpu/drm/i915/i915_reg.h | 1 + 4 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 59961621fe4a..18531ffd4789 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -8760,6 +8760,7 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv) intel_ddi_init(dev_priv, PORT_B); intel_ddi_init(dev_priv, PORT_C); intel_ddi_init(dev_priv, PORT_D_XELPD); + intel_ddi_init(dev_priv, PORT_TC1); } else if (IS_ALDERLAKE_P(dev_priv)) { intel_ddi_init(dev_priv, PORT_A); intel_ddi_init(dev_priv, PORT_B); diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c index 6ce8c10fe975..2fad03250661 100644 --- a/drivers/gpu/drm/i915/display/intel_gmbus.c +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c @@ -98,11 +98,21 @@ static const struct gmbus_pin gmbus_pins_dg1[] = { [GMBUS_PIN_4_CNP] = { "dpd", GPIOE }, };
+static const struct gmbus_pin gmbus_pins_dg2[] = { + [GMBUS_PIN_1_BXT] = { "dpa", GPIOB }, + [GMBUS_PIN_2_BXT] = { "dpb", GPIOC }, + [GMBUS_PIN_3_BXT] = { "dpc", GPIOD }, + [GMBUS_PIN_4_CNP] = { "dpd", GPIOE }, + [GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOJ }, +}; + /* pin is expected to be valid */ static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv, unsigned int pin) { - if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) + if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2) + return &gmbus_pins_dg2[pin]; + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) return &gmbus_pins_dg1[pin]; else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) return &gmbus_pins_icp[pin]; @@ -123,7 +133,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv, { unsigned int size;
- if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) + if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2) + size = ARRAY_SIZE(gmbus_pins_dg2); + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) size = ARRAY_SIZE(gmbus_pins_dg1); else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) size = ARRAY_SIZE(gmbus_pins_icp); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index fdd568ba4a16..4d81063b128c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -179,6 +179,7 @@ static const u32 hpd_sde_dg1[HPD_NUM_PINS] = { [HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_B), [HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_C), [HPD_PORT_D] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_D), + [HPD_PORT_TC1] = SDE_TC_HOTPLUG_DG2(HPD_PORT_TC1), };
static void intel_hpd_init_pins(struct drm_i915_private *dev_priv) @@ -4424,7 +4425,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv) if (I915_HAS_HOTPLUG(dev_priv)) dev_priv->hotplug_funcs = &i915_hpd_funcs; } else { - if (HAS_PCH_DG1(dev_priv)) + if (HAS_PCH_DG2(dev_priv)) + dev_priv->hotplug_funcs = &icp_hpd_funcs; + else if (HAS_PCH_DG1(dev_priv)) dev_priv->hotplug_funcs = &dg1_hpd_funcs; else if (DISPLAY_VER(dev_priv) >= 11) dev_priv->hotplug_funcs = &gen11_hpd_funcs; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 2243d9d1d941..b5acbbcc8574 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6065,6 +6065,7 @@ /* south display engine interrupt: ICP/TGP */ #define SDE_GMBUS_ICP (1 << 23) #define SDE_TC_HOTPLUG_ICP(hpd_pin) REG_BIT(24 + _HPD_PIN_TC(hpd_pin)) +#define SDE_TC_HOTPLUG_DG2(hpd_pin) REG_BIT(25 + _HPD_PIN_TC(hpd_pin)) /* sigh */ #define SDE_DDI_HOTPLUG_ICP(hpd_pin) REG_BIT(16 + _HPD_PIN_DDI(hpd_pin)) #define SDE_DDI_HOTPLUG_MASK_ICP (SDE_DDI_HOTPLUG_ICP(HPD_PORT_D) | \ SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \
On Fri, Feb 18, 2022 at 12:12:21AM +0530, Ramalingam C wrote:
From: Matt Roper matthew.d.roper@intel.com
DG2 supports a 5th display output which the hardware refers to as "TC1," even though it isn't a Type-C output. This behaves similarly to the TC1 on past platforms with just a couple minor differences:
- DG2's TC1 bit in SDEISR is at bit 25 rather than 24 as it is on ICP/TGP/ADP.
- DG2 doesn't need the hpd inversion setting that we had to use on DG1
v2: intel_ddi_init(dev_priv, PORT_TC1); [Matt]
Cc: Swathi Dhanavanthri swathi.dhanavanthri@intel.com Cc: Lucas De Marchi lucas.demarchi@intel.com Cc: José Roberto de Souza jose.souza@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com
Reviewed-by: Lucas De Marchi lucas.demarchi@intel.com
Lucas De Marchi
From: Matt Roper matthew.d.roper@intel.com
Our early understanding of DG2 was incorrect; since the 5th display isn't actually a Type-C output, 38.4 MHz input clocks are never used on this platform and we can drop the corresponding MPLLB tables.
Cc: Anusha Srivatsa anusha.srivatsa@intel.com Cc: José Roberto de Souza jose.souza@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com --- drivers/gpu/drm/i915/display/intel_snps_phy.c | 208 +----------------- 1 file changed, 1 insertion(+), 207 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c b/drivers/gpu/drm/i915/display/intel_snps_phy.c index 8573a458811a..c60575cb5368 100644 --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c @@ -250,197 +250,6 @@ static const struct intel_mpllb_state * const dg2_dp_100_tables[] = { NULL, };
-/* - * Basic DP link rates with 38.4 MHz reference clock. - */ - -static const struct intel_mpllb_state dg2_dp_rbr_38_4 = { - .clock = 162000, - .ref_control = - REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1), - .mpllb_cp = - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 5) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 25) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127), - .mpllb_div = - REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_TX_CLK_DIV, 2) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FREQ_VCO, 2), - .mpllb_div2 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 304), - .mpllb_fracn1 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1), - .mpllb_fracn2 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 49152), -}; - -static const struct intel_mpllb_state dg2_dp_hbr1_38_4 = { - .clock = 270000, - .ref_control = - REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1), - .mpllb_cp = - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 5) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 25) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127), - .mpllb_div = - REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_TX_CLK_DIV, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FREQ_VCO, 3), - .mpllb_div2 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 248), - .mpllb_fracn1 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1), - .mpllb_fracn2 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 40960), -}; - -static const struct intel_mpllb_state dg2_dp_hbr2_38_4 = { - .clock = 540000, - .ref_control = - REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1), - .mpllb_cp = - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 5) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 25) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127), - .mpllb_div = - REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FREQ_VCO, 3), - .mpllb_div2 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 248), - .mpllb_fracn1 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1), - .mpllb_fracn2 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 40960), -}; - -static const struct intel_mpllb_state dg2_dp_hbr3_38_4 = { - .clock = 810000, - .ref_control = - REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1), - .mpllb_cp = - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 6) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 26) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127), - .mpllb_div = - REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2), - .mpllb_div2 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 388), - .mpllb_fracn1 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1), - .mpllb_fracn2 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 61440), -}; - -static const struct intel_mpllb_state dg2_dp_uhbr10_38_4 = { - .clock = 1000000, - .ref_control = - REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1), - .mpllb_cp = - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 5) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 26) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127), - .mpllb_div = - REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV_CLK_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV_MULTIPLIER, 8) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_WORD_DIV2_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_DP2_MODE, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_SHIM_DIV32_CLK_SEL, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2), - .mpllb_div2 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 488), - .mpllb_fracn1 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 3), - .mpllb_fracn2 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_REM, 2) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 27306), - - /* - * SSC will be enabled, DP UHBR has a minimum SSC requirement. - */ - .mpllb_sscen = - REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_PEAK, 76800), - .mpllb_sscstep = - REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_STEPSIZE, 129024), -}; - -static const struct intel_mpllb_state dg2_dp_uhbr13_38_4 = { - .clock = 1350000, - .ref_control = - REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1), - .mpllb_cp = - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 6) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 56) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127), - .mpllb_div = - REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV_CLK_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV_MULTIPLIER, 8) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_WORD_DIV2_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_DP2_MODE, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 3), - .mpllb_div2 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 670), - .mpllb_fracn1 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1), - .mpllb_fracn2 = - REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 36864), - - /* - * SSC will be enabled, DP UHBR has a minimum SSC requirement. - */ - .mpllb_sscen = - REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_EN, 1) | - REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_PEAK, 103680), - .mpllb_sscstep = - REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_STEPSIZE, 174182), -}; - -static const struct intel_mpllb_state * const dg2_dp_38_4_tables[] = { - &dg2_dp_rbr_38_4, - &dg2_dp_hbr1_38_4, - &dg2_dp_hbr2_38_4, - &dg2_dp_hbr3_38_4, - &dg2_dp_uhbr10_38_4, - &dg2_dp_uhbr13_38_4, - NULL, -}; - /* * eDP link rates with 100 MHz reference clock. */ @@ -749,22 +558,7 @@ intel_mpllb_tables_get(struct intel_crtc_state *crtc_state, if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) { return dg2_edp_tables; } else if (intel_crtc_has_dp_encoder(crtc_state)) { - /* - * FIXME: Initially we're just enabling the "combo" outputs on - * port A-D. The MPLLB for those ports takes an input from the - * "Display Filter PLL" which always has an output frequency - * of 100 MHz, hence the use of the _100 tables below. - * - * Once we enable port TC1 it will either use the same 100 MHz - * "Display Filter PLL" (when strapped to support a native - * display connection) or different 38.4 MHz "Filter PLL" when - * strapped to support a USB connection, so we'll need to check - * that to determine which table to use. - */ - if (0) - return dg2_dp_38_4_tables; - else - return dg2_dp_100_tables; + return dg2_dp_100_tables; } else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) { return dg2_hdmi_tables; }
-----Original Message----- From: C, Ramalingam ramalingam.c@intel.com Sent: Tuesday, February 15, 2022 11:22 AM To: intel-gfx intel-gfx@lists.freedesktop.org; dri-devel <dri- devel@lists.freedesktop.org> Cc: Ville Syrjälä ville.syrjala@linux.intel.com; Shankar, Uma uma.shankar@intel.com; Roper, Matthew D matthew.d.roper@intel.com; Srivatsa, Anusha anusha.srivatsa@intel.com; Souza, Jose jose.souza@intel.com; C, Ramalingam ramalingam.c@intel.com Subject: [PATCH 2/3] drm/i915/dg2: Drop 38.4 MHz MPLLB tables
From: Matt Roper matthew.d.roper@intel.com
Our early understanding of DG2 was incorrect; since the 5th display isn't actually a Type-C output, 38.4 MHz input clocks are never used on this platform and we can drop the corresponding MPLLB tables.
Looks Good to me. Reviewed-by: Uma Shankar uma.shankar@intel.com
Cc: Anusha Srivatsa anusha.srivatsa@intel.com Cc: José Roberto de Souza jose.souza@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com
drivers/gpu/drm/i915/display/intel_snps_phy.c | 208 +----------------- 1 file changed, 1 insertion(+), 207 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c b/drivers/gpu/drm/i915/display/intel_snps_phy.c index 8573a458811a..c60575cb5368 100644 --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c @@ -250,197 +250,6 @@ static const struct intel_mpllb_state * const dg2_dp_100_tables[] = { NULL, };
-/*
- Basic DP link rates with 38.4 MHz reference clock.
- */
-static const struct intel_mpllb_state dg2_dp_rbr_38_4 = {
- .clock = 162000,
- .ref_control =
REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1),- .mpllb_cp =
REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 5) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 25) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127),- .mpllb_div =
REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_TX_CLK_DIV, 2) |REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FREQ_VCO, 2),- .mpllb_div2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 304),- .mpllb_fracn1 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1),- .mpllb_fracn2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 49152),-};
-static const struct intel_mpllb_state dg2_dp_hbr1_38_4 = {
- .clock = 270000,
- .ref_control =
REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1),- .mpllb_cp =
REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 5) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 25) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127),- .mpllb_div =
REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_TX_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FREQ_VCO, 3),- .mpllb_div2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 248),- .mpllb_fracn1 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1),- .mpllb_fracn2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 40960),-};
-static const struct intel_mpllb_state dg2_dp_hbr2_38_4 = {
- .clock = 540000,
- .ref_control =
REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1),- .mpllb_cp =
REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 5) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 25) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127),- .mpllb_div =
REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FREQ_VCO, 3),- .mpllb_div2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 248),- .mpllb_fracn1 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1),- .mpllb_fracn2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 40960),-};
-static const struct intel_mpllb_state dg2_dp_hbr3_38_4 = {
- .clock = 810000,
- .ref_control =
REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1),- .mpllb_cp =
REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 6) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 26) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127),- .mpllb_div =
REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2),- .mpllb_div2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 388),- .mpllb_fracn1 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1),- .mpllb_fracn2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 61440),-};
-static const struct intel_mpllb_state dg2_dp_uhbr10_38_4 = {
- .clock = 1000000,
- .ref_control =
REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1),- .mpllb_cp =
REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 5) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 26) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127),- .mpllb_div =
REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV_MULTIPLIER, 8) |REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_WORD_DIV2_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_DP2_MODE, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_SHIM_DIV32_CLK_SEL, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2),- .mpllb_div2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 488),- .mpllb_fracn1 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 3),- .mpllb_fracn2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_REM, 2) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 27306),- /*
* SSC will be enabled, DP UHBR has a minimum SSC requirement.*/- .mpllb_sscen =
REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_PEAK, 76800),- .mpllb_sscstep =
REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_STEPSIZE, 129024),-};
-static const struct intel_mpllb_state dg2_dp_uhbr13_38_4 = {
- .clock = 1350000,
- .ref_control =
REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1),- .mpllb_cp =
REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 6) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 56) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127),- .mpllb_div =
REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV_MULTIPLIER, 8) |REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_WORD_DIV2_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_DP2_MODE, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 3),- .mpllb_div2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 670),- .mpllb_fracn1 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1),- .mpllb_fracn2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 36864),- /*
* SSC will be enabled, DP UHBR has a minimum SSC requirement.*/- .mpllb_sscen =
REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_PEAK, 103680),- .mpllb_sscstep =
REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_STEPSIZE, 174182),-};
-static const struct intel_mpllb_state * const dg2_dp_38_4_tables[] = {
- &dg2_dp_rbr_38_4,
- &dg2_dp_hbr1_38_4,
- &dg2_dp_hbr2_38_4,
- &dg2_dp_hbr3_38_4,
- &dg2_dp_uhbr10_38_4,
- &dg2_dp_uhbr13_38_4,
- NULL,
-};
/*
- eDP link rates with 100 MHz reference clock.
*/ @@ -749,22 +558,7 @@ intel_mpllb_tables_get(struct intel_crtc_state *crtc_state, if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) { return dg2_edp_tables; } else if (intel_crtc_has_dp_encoder(crtc_state)) {
/** FIXME: Initially we're just enabling the "combo" outputs on* port A-D. The MPLLB for those ports takes an input from the* "Display Filter PLL" which always has an output frequency* of 100 MHz, hence the use of the _100 tables below.** Once we enable port TC1 it will either use the same 100 MHz* "Display Filter PLL" (when strapped to support a native* display connection) or different 38.4 MHz "Filter PLL" when* strapped to support a USB connection, so we'll need to check* that to determine which table to use.*/if (0)return dg2_dp_38_4_tables;elsereturn dg2_dp_100_tables;
return dg2_dp_100_tables;-- 2.20.1
On Tue, Feb 15, 2022 at 11:21:53AM +0530, Ramalingam C wrote:
From: Matt Roper matthew.d.roper@intel.com
Our early understanding of DG2 was incorrect; since the 5th display isn't actually a Type-C output, 38.4 MHz input clocks are never used on this platform and we can drop the corresponding MPLLB tables.
Cc: Anusha Srivatsa anusha.srivatsa@intel.com Cc: José Roberto de Souza jose.souza@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com
Reviewed-by: Lucas De Marchi lucas.demarchi@intel.com
Lucas De Marchi
drivers/gpu/drm/i915/display/intel_snps_phy.c | 208 +----------------- 1 file changed, 1 insertion(+), 207 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c b/drivers/gpu/drm/i915/display/intel_snps_phy.c index 8573a458811a..c60575cb5368 100644 --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c @@ -250,197 +250,6 @@ static const struct intel_mpllb_state * const dg2_dp_100_tables[] = { NULL, };
-/*
- Basic DP link rates with 38.4 MHz reference clock.
- */
-static const struct intel_mpllb_state dg2_dp_rbr_38_4 = {
- .clock = 162000,
- .ref_control =
REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1),- .mpllb_cp =
REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 5) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 25) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127),- .mpllb_div =
REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_TX_CLK_DIV, 2) |REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FREQ_VCO, 2),- .mpllb_div2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 304),- .mpllb_fracn1 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1),- .mpllb_fracn2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 49152),-};
-static const struct intel_mpllb_state dg2_dp_hbr1_38_4 = {
- .clock = 270000,
- .ref_control =
REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1),- .mpllb_cp =
REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 5) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 25) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127),- .mpllb_div =
REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_TX_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FREQ_VCO, 3),- .mpllb_div2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 248),- .mpllb_fracn1 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1),- .mpllb_fracn2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 40960),-};
-static const struct intel_mpllb_state dg2_dp_hbr2_38_4 = {
- .clock = 540000,
- .ref_control =
REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1),- .mpllb_cp =
REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 5) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 25) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127),- .mpllb_div =
REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FREQ_VCO, 3),- .mpllb_div2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 248),- .mpllb_fracn1 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1),- .mpllb_fracn2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 40960),-};
-static const struct intel_mpllb_state dg2_dp_hbr3_38_4 = {
- .clock = 810000,
- .ref_control =
REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1),- .mpllb_cp =
REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 6) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 26) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127),- .mpllb_div =
REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2),- .mpllb_div2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 388),- .mpllb_fracn1 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1),- .mpllb_fracn2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 61440),-};
-static const struct intel_mpllb_state dg2_dp_uhbr10_38_4 = {
- .clock = 1000000,
- .ref_control =
REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1),- .mpllb_cp =
REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 5) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 26) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127),- .mpllb_div =
REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV_MULTIPLIER, 8) |REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_WORD_DIV2_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_DP2_MODE, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_SHIM_DIV32_CLK_SEL, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 2),- .mpllb_div2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 488),- .mpllb_fracn1 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 3),- .mpllb_fracn2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_REM, 2) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 27306),- /*
* SSC will be enabled, DP UHBR has a minimum SSC requirement.*/- .mpllb_sscen =
REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_PEAK, 76800),- .mpllb_sscstep =
REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_STEPSIZE, 129024),-};
-static const struct intel_mpllb_state dg2_dp_uhbr13_38_4 = {
- .clock = 1350000,
- .ref_control =
REG_FIELD_PREP(SNPS_PHY_REF_CONTROL_REF_RANGE, 1),- .mpllb_cp =
REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT, 6) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP, 56) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_INT_GS, 65) |REG_FIELD_PREP(SNPS_PHY_MPLLB_CP_PROP_GS, 127),- .mpllb_div =
REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV5_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV_CLK_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_DIV_MULTIPLIER, 8) |REG_FIELD_PREP(SNPS_PHY_MPLLB_PMIX_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_WORD_DIV2_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_DP2_MODE, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_V2I, 3),- .mpllb_div2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_REF_CLK_DIV, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_MULTIPLIER, 670),- .mpllb_fracn1 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_CGG_UPDATE_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_DEN, 1),- .mpllb_fracn2 =
REG_FIELD_PREP(SNPS_PHY_MPLLB_FRACN_QUOT, 36864),- /*
* SSC will be enabled, DP UHBR has a minimum SSC requirement.*/- .mpllb_sscen =
REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_EN, 1) |REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_PEAK, 103680),- .mpllb_sscstep =
REG_FIELD_PREP(SNPS_PHY_MPLLB_SSC_STEPSIZE, 174182),-};
-static const struct intel_mpllb_state * const dg2_dp_38_4_tables[] = {
- &dg2_dp_rbr_38_4,
- &dg2_dp_hbr1_38_4,
- &dg2_dp_hbr2_38_4,
- &dg2_dp_hbr3_38_4,
- &dg2_dp_uhbr10_38_4,
- &dg2_dp_uhbr13_38_4,
- NULL,
-};
/*
- eDP link rates with 100 MHz reference clock.
*/ @@ -749,22 +558,7 @@ intel_mpllb_tables_get(struct intel_crtc_state *crtc_state, if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) { return dg2_edp_tables; } else if (intel_crtc_has_dp_encoder(crtc_state)) {
/** FIXME: Initially we're just enabling the "combo" outputs on* port A-D. The MPLLB for those ports takes an input from the* "Display Filter PLL" which always has an output frequency* of 100 MHz, hence the use of the _100 tables below.** Once we enable port TC1 it will either use the same 100 MHz* "Display Filter PLL" (when strapped to support a native* display connection) or different 38.4 MHz "Filter PLL" when* strapped to support a USB connection, so we'll need to check* that to determine which table to use.*/if (0)return dg2_dp_38_4_tables;elsereturn dg2_dp_100_tables;
return dg2_dp_100_tables;-- 2.20.1
From: Jouni Högander jouni.hogander@intel.com
Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E port. Correct offset is 0x64C14.
Fix this by handling PHY_E port seprately.
Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Jouni Högander jouni.hogander@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com --- drivers/gpu/drm/i915/display/intel_snps_phy.c | 2 +- drivers/gpu/drm/i915/i915_reg.h | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c b/drivers/gpu/drm/i915/display/intel_snps_phy.c index c60575cb5368..f08061c748b3 100644 --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c @@ -32,7 +32,7 @@ void intel_snps_phy_wait_for_calibration(struct drm_i915_private *i915) if (!intel_phy_is_snps(i915, phy)) continue;
- if (intel_de_wait_for_clear(i915, ICL_PHY_MISC(phy), + if (intel_de_wait_for_clear(i915, DG2_PHY_MISC(phy), DG2_PHY_DP_TX_ACK_MASK, 25)) drm_err(&i915->drm, "SNPS PHY %c failed to calibrate after 25ms.\n", phy); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4d12abb2d7ff..354c25f483cb 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9559,8 +9559,10 @@ enum skl_power_gate {
#define _ICL_PHY_MISC_A 0x64C00 #define _ICL_PHY_MISC_B 0x64C04 -#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, \ - _ICL_PHY_MISC_B) +#define _DG2_PHY_MISC_TC1 0x64C14 /* TC1="PHY E" but offset as if "PHY F" */ +#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, _ICL_PHY_MISC_B) +#define DG2_PHY_MISC(port) ((port) == PHY_E ? _MMIO(_DG2_PHY_MISC_TC1) : \ + ICL_PHY_MISC(port)) #define ICL_PHY_MISC_MUX_DDID (1 << 28) #define ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN (1 << 23) #define DG2_PHY_DP_TX_ACK_MASK REG_GENMASK(23, 20)
-----Original Message----- From: C, Ramalingam ramalingam.c@intel.com Sent: Tuesday, February 15, 2022 11:22 AM To: intel-gfx intel-gfx@lists.freedesktop.org; dri-devel <dri- devel@lists.freedesktop.org> Cc: Ville Syrjälä ville.syrjala@linux.intel.com; Shankar, Uma uma.shankar@intel.com; Hogander, Jouni jouni.hogander@intel.com; C, Ramalingam ramalingam.c@intel.com Subject: [PATCH 3/3] drm/i915: Fix for PHY_MISC_TC1 offset
From: Jouni Högander jouni.hogander@intel.com
Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E port. Correct offset is 0x64C14.
Fix this by handling PHY_E port seprately.
Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Jouni Högander jouni.hogander@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com
drivers/gpu/drm/i915/display/intel_snps_phy.c | 2 +- drivers/gpu/drm/i915/i915_reg.h | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c b/drivers/gpu/drm/i915/display/intel_snps_phy.c index c60575cb5368..f08061c748b3 100644 --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c @@ -32,7 +32,7 @@ void intel_snps_phy_wait_for_calibration(struct drm_i915_private *i915) if (!intel_phy_is_snps(i915, phy)) continue;
if (intel_de_wait_for_clear(i915, ICL_PHY_MISC(phy),
if (intel_de_wait_for_clear(i915, DG2_PHY_MISC(phy), DG2_PHY_DP_TX_ACK_MASK, 25)) drm_err(&i915->drm, "SNPS PHY %c failed to calibrate after25ms.\n", phy); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4d12abb2d7ff..354c25f483cb 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9559,8 +9559,10 @@ enum skl_power_gate {
#define _ICL_PHY_MISC_A 0x64C00 #define _ICL_PHY_MISC_B 0x64C04 -#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, \
_ICL_PHY_MISC_B)+#define _DG2_PHY_MISC_TC1 0x64C14 /* TC1="PHY E" but offset as if "PHY F" */ +#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, _ICL_PHY_MISC_B)
Nit: Align it as was defined earlier to honor line limit.
Looks Good to me. Reviewed-by: Uma Shankar uma.shankar@intel.com
+#define DG2_PHY_MISC(port) ((port) == PHY_E ? _MMIO(_DG2_PHY_MISC_TC1) : \
ICL_PHY_MISC(port))#define ICL_PHY_MISC_MUX_DDID (1 << 28) #define ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN (1 << 23)
#define DG2_PHY_DP_TX_ACK_MASK REG_GENMASK(23, 20)
2.20.1
On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote:
From: Jouni Högander jouni.hogander@intel.com
Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E port. Correct offset is 0x64C14.
Why is it PHY_E and not PHY_F?
Fix this by handling PHY_E port seprately.
Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Jouni Högander jouni.hogander@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com
drivers/gpu/drm/i915/display/intel_snps_phy.c | 2 +- drivers/gpu/drm/i915/i915_reg.h | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c b/drivers/gpu/drm/i915/display/intel_snps_phy.c index c60575cb5368..f08061c748b3 100644 --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c @@ -32,7 +32,7 @@ void intel_snps_phy_wait_for_calibration(struct drm_i915_private *i915) if (!intel_phy_is_snps(i915, phy)) continue;
if (intel_de_wait_for_clear(i915, ICL_PHY_MISC(phy),
if (intel_de_wait_for_clear(i915, DG2_PHY_MISC(phy), DG2_PHY_DP_TX_ACK_MASK, 25)) drm_err(&i915->drm, "SNPS PHY %c failed to calibrate after 25ms.\n", phy);diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4d12abb2d7ff..354c25f483cb 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9559,8 +9559,10 @@ enum skl_power_gate {
#define _ICL_PHY_MISC_A 0x64C00 #define _ICL_PHY_MISC_B 0x64C04 -#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, \
_ICL_PHY_MISC_B)+#define _DG2_PHY_MISC_TC1 0x64C14 /* TC1="PHY E" but offset as if "PHY F" */ +#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, _ICL_PHY_MISC_B) +#define DG2_PHY_MISC(port) ((port) == PHY_E ? _MMIO(_DG2_PHY_MISC_TC1) : \
ICL_PHY_MISC(port))#define ICL_PHY_MISC_MUX_DDID (1 << 28) #define ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN (1 << 23)
#define DG2_PHY_DP_TX_ACK_MASK REG_GENMASK(23, 20)
2.20.1
On Wed, 2022-02-16 at 10:50 +0200, Ville Syrjälä wrote:
On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote:
From: Jouni Högander jouni.hogander@intel.com
Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E port. Correct offset is 0x64C14.
Why is it PHY_E and not PHY_F?
This is a valid question. It seems we have followed intel_phy_is_snps() here:
// snip else if (IS_DG2(dev_priv)) /* * All four "combo" ports and the TC1 port (PHY E) use * Synopsis PHYs. */ return phy <= PHY_E; // snip
According to spec port E is "No connection". Better place to fix this could be intel_phy_is_snps() itself?
Fix this by handling PHY_E port seprately.
Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Jouni Högander jouni.hogander@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com
drivers/gpu/drm/i915/display/intel_snps_phy.c | 2 +- drivers/gpu/drm/i915/i915_reg.h | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c b/drivers/gpu/drm/i915/display/intel_snps_phy.c index c60575cb5368..f08061c748b3 100644 --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c @@ -32,7 +32,7 @@ void intel_snps_phy_wait_for_calibration(struct drm_i915_private *i915) if (!intel_phy_is_snps(i915, phy)) continue;
if (intel_de_wait_for_clear(i915, ICL_PHY_MISC(phy),
if (intel_de_wait_for_clear(i915, DG2_PHY_MISC(phy), DG2_PHY_DP_TX_ACK_MASK,25)) drm_err(&i915->drm, "SNPS PHY %c failed to calibrate after 25ms.\n", phy); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4d12abb2d7ff..354c25f483cb 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9559,8 +9559,10 @@ enum skl_power_gate {
#define _ICL_PHY_MISC_A 0x64C00 #define _ICL_PHY_MISC_B 0x64C04 -#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, \
_ICL_PHY_MISC_B)+#define _DG2_PHY_MISC_TC1 0x64C14 /* TC1="PHY E" but offset as if "PHY F" */ +#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, _ICL_PHY_MISC_B) +#define DG2_PHY_MISC(port) ((port) == PHY_E ? _MMIO(_DG2_PHY_MISC_TC1) : \
ICL_PHY_MISC(port))#define ICL_PHY_MISC_MUX_DDID (1 << 28) #define ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN (1 << 23) #define DG2_PHY_DP_TX_ACK_MASK REG_GENMASK(23, 20) -- 2.20.1
BR,
Jouni Högander
On Wed, Feb 16, 2022 at 09:36:02AM +0000, Hogander, Jouni wrote:
On Wed, 2022-02-16 at 10:50 +0200, Ville Syrjälä wrote:
On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote:
From: Jouni Högander jouni.hogander@intel.com
Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E port. Correct offset is 0x64C14.
Why is it PHY_E and not PHY_F?
This is a valid question. It seems we have followed intel_phy_is_snps() here:
// snip else if (IS_DG2(dev_priv)) /* * All four "combo" ports and the TC1 port (PHY E) use * Synopsis PHYs. */ return phy <= PHY_E; // snip
According to spec port E is "No connection". Better place to fix this could be intel_phy_is_snps() itself?
I think the crucial question is where are all the places that the results of intel_port_to_phy() get used.
I do see that for all the actual snps phy registers we do want PHY_E, but maybe it would be better to have a local SNPS_PHY enum just for intel_snps_phy.c, and leave the other phy thing for everything else?
Not sure if there is some other register we index with the phy that specifically wants PHY_E?
Also it kinda looks to me like for VBT port mapping we also want PHY_F essentially since the modern platforms make the VBT port mapping PHY based and xelpd_port_mapping() uses PORT_TC1<->DVO_PORT_*F. Not that we actually use enum phy in the VBT code atm, but I'm thinking we probably should since it might allow us to get rid of all those different mapping tables. Though the whole intel_port_to_phy() disaster needs to get cleaned up first IMO.
On Wed, 2022-02-16 at 12:07 +0200, Ville Syrjälä wrote:
On Wed, Feb 16, 2022 at 09:36:02AM +0000, Hogander, Jouni wrote:
On Wed, 2022-02-16 at 10:50 +0200, Ville Syrjälä wrote:
On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote:
From: Jouni Högander jouni.hogander@intel.com
Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E port. Correct offset is 0x64C14.
Why is it PHY_E and not PHY_F?
This is a valid question. It seems we have followed intel_phy_is_snps() here:
// snip else if (IS_DG2(dev_priv)) /* * All four "combo" ports and the TC1 port (PHY E) use * Synopsis PHYs. */ return phy <= PHY_E; // snip
According to spec port E is "No connection". Better place to fix this could be intel_phy_is_snps() itself?
I think the crucial question is where are all the places that the results of intel_port_to_phy() get used.
I do see that for all the actual snps phy registers we do want PHY_E, but maybe it would be better to have a local SNPS_PHY enum just for intel_snps_phy.c, and leave the other phy thing for everything else?
Not sure if there is some other register we index with the phy that specifically wants PHY_E?
I went through registers accesses in intel_snps_phy.c. It is actually only this one register which offset is wrong with PHY_E. Everything else seems to be assuming PHY_E including those SNPS_* registers (as you mentioned). I'm starting to think it would be overkill to open up this phy enum for this purpose. I would propose to stick with current patch. Maybe just update commit message. What do you think?
Also it kinda looks to me like for VBT port mapping we also want PHY_F essentially since the modern platforms make the VBT port mapping PHY based and xelpd_port_mapping() uses PORT_TC1<->DVO_PORT_*F. Not that we actually use enum phy in the VBT code atm, but I'm thinking we probably should since it might allow us to get rid of all those different mapping tables. Though the whole intel_port_to_phy() disaster needs to get cleaned up first IMO.
BR,
Jouni Högander
On Wed, Feb 16, 2022 at 02:11:54PM +0000, Hogander, Jouni wrote:
On Wed, 2022-02-16 at 12:07 +0200, Ville Syrjälä wrote:
On Wed, Feb 16, 2022 at 09:36:02AM +0000, Hogander, Jouni wrote:
On Wed, 2022-02-16 at 10:50 +0200, Ville Syrjälä wrote:
On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote:
From: Jouni Högander jouni.hogander@intel.com
Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E port. Correct offset is 0x64C14.
Why is it PHY_E and not PHY_F?
This is a valid question. It seems we have followed intel_phy_is_snps() here:
// snip else if (IS_DG2(dev_priv)) /* * All four "combo" ports and the TC1 port (PHY E) use * Synopsis PHYs. */ return phy <= PHY_E; // snip
According to spec port E is "No connection". Better place to fix this could be intel_phy_is_snps() itself?
I think the crucial question is where are all the places that the results of intel_port_to_phy() get used.
I do see that for all the actual snps phy registers we do want PHY_E, but maybe it would be better to have a local SNPS_PHY enum just for intel_snps_phy.c, and leave the other phy thing for everything else?
Not sure if there is some other register we index with the phy that specifically wants PHY_E?
I went through registers accesses in intel_snps_phy.c. It is actually only this one register which offset is wrong with PHY_E. Everything else seems to be assuming PHY_E including those SNPS_* registers (as you mentioned). I'm starting to think it would be overkill to open up this phy enum for this purpose. I would propose to stick with current patch. Maybe just update commit message. What do you think?
I would put it the other way. It is *only* the SNPS PHY IP registers that use the wonky offsets (unless you found some others?). Everythting on the Intel IP side wants it to be PHY_F.
So still would make more sense to me to add a new enum for the SNPS PHY instance and remap across the boundary. Otherwise we're just propagating this madness everwhere rather than containing in the SNPS PHY implementation.
On Wed, Feb 16, 2022 at 05:01:35PM +0200, Ville Syrjälä wrote:
On Wed, Feb 16, 2022 at 02:11:54PM +0000, Hogander, Jouni wrote:
On Wed, 2022-02-16 at 12:07 +0200, Ville Syrjälä wrote:
On Wed, Feb 16, 2022 at 09:36:02AM +0000, Hogander, Jouni wrote:
On Wed, 2022-02-16 at 10:50 +0200, Ville Syrjälä wrote:
On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote:
From: Jouni Högander jouni.hogander@intel.com
Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E port. Correct offset is 0x64C14.
Why is it PHY_E and not PHY_F?
This is a valid question. It seems we have followed intel_phy_is_snps() here:
// snip else if (IS_DG2(dev_priv)) /* * All four "combo" ports and the TC1 port (PHY E) use * Synopsis PHYs. */ return phy <= PHY_E; // snip
According to spec port E is "No connection". Better place to fix this could be intel_phy_is_snps() itself?
I think the crucial question is where are all the places that the results of intel_port_to_phy() get used.
I do see that for all the actual snps phy registers we do want PHY_E, but maybe it would be better to have a local SNPS_PHY enum just for intel_snps_phy.c, and leave the other phy thing for everything else?
Not sure if there is some other register we index with the phy that specifically wants PHY_E?
I went through registers accesses in intel_snps_phy.c. It is actually only this one register which offset is wrong with PHY_E. Everything else seems to be assuming PHY_E including those SNPS_* registers (as you mentioned). I'm starting to think it would be overkill to open up this phy enum for this purpose. I would propose to stick with current patch. Maybe just update commit message. What do you think?
I would put it the other way. It is *only* the SNPS PHY IP registers that use the wonky offsets (unless you found some others?). Everythting on the Intel IP side wants it to be PHY_F.
So still would make more sense to me to add a new enum for the SNPS PHY instance and remap across the boundary. Otherwise we're just propagating this madness everwhere rather than containing in the SNPS PHY implementation.
Seems people want this is asap. I suppose it'll do as a temporary measure given the phy stuff is already such mess. Acked-by: Ville Syrjälä ville.syrjala@linux.intel.com
As for the proper way to do stuff, I'm thinking roughly: enum intel_spns_phy { SNPS_PHY_A, ... SNPS_PHY_TC1, // == current PHY_E in value }; and I think that can stay entirely inside intel_snps_phy.c.
As for our currnet enum phy I think we could start with something like this: enum phy { PHY_A, ... PHY_F,
PHY_TC1 = PHY_F, ... };
I think that should make it line up with PHY_MISC stuff and the VBT as well. So in the VBT code we could nuke all those crazy mapping tables and just do: old platform: port -> VBT port new platform: phy -> VBT port
And we could probably have encoder->phy which gets populated in the encoder init per-platform, similar to hpd_pin. That would get rid of the intel_port_to_phy() disaster.
On Wed, Feb 16, 2022 at 09:36:02AM +0000, Hogander, Jouni wrote:
On Wed, 2022-02-16 at 10:50 +0200, Ville Syrjälä wrote:
On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote:
From: Jouni Högander jouni.hogander@intel.com
Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E port. Correct offset is 0x64C14.
Why is it PHY_E and not PHY_F?
This is a valid question. It seems we have followed intel_phy_is_snps() here:
// snip else if (IS_DG2(dev_priv)) /* * All four "combo" ports and the TC1 port (PHY E) use * Synopsis PHYs. */ return phy <= PHY_E; // snip
And this is actually the bug that we had. We wouldn't need to bring the incomplete support for the 5th port if this single had changed: it's often preferred to prepare the driver first and enable the port/phy as the last step:
- return phy <= PHY_E; + return phy <= PHY_D;
With possibly a change in the commit above. Because in drivers/gpu/drm/i915/display/intel_snps_phy.c we do:
intel_snps_phy_wait_for_calibration() { ... for_each_phy_masked(phy, ~0) { if (!intel_phy_is_snps(i915, phy)) continue; ... }
Relying on intel_phy_is_snps() to mask out the unavailable phys.
However, since now we almost have the extra port wired up, I'm not going to push back on it. Let's just add a comment on the commit message. And since going with this approach is also acked by Ville who preferred to contain the additional mapping inside intel_phy_snps.c:
Reviewed-by: Lucas De Marchi lucas.demarchi@intel.com
Lucas De Marchi
According to spec port E is "No connection". Better place to fix this could be intel_phy_is_snps() itself?
Fix this by handling PHY_E port seprately.
Signed-off-by: Matt Roper matthew.d.roper@intel.com Signed-off-by: Jouni Högander jouni.hogander@intel.com Signed-off-by: Ramalingam C ramalingam.c@intel.com
drivers/gpu/drm/i915/display/intel_snps_phy.c | 2 +- drivers/gpu/drm/i915/i915_reg.h | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c b/drivers/gpu/drm/i915/display/intel_snps_phy.c index c60575cb5368..f08061c748b3 100644 --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c @@ -32,7 +32,7 @@ void intel_snps_phy_wait_for_calibration(struct drm_i915_private *i915) if (!intel_phy_is_snps(i915, phy)) continue;
if (intel_de_wait_for_clear(i915, ICL_PHY_MISC(phy),
if (intel_de_wait_for_clear(i915, DG2_PHY_MISC(phy), DG2_PHY_DP_TX_ACK_MASK,25)) drm_err(&i915->drm, "SNPS PHY %c failed to calibrate after 25ms.\n", phy); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4d12abb2d7ff..354c25f483cb 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9559,8 +9559,10 @@ enum skl_power_gate {
#define _ICL_PHY_MISC_A 0x64C00 #define _ICL_PHY_MISC_B 0x64C04 -#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, \
_ICL_PHY_MISC_B)+#define _DG2_PHY_MISC_TC1 0x64C14 /* TC1="PHY E" but offset as if "PHY F" */ +#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, _ICL_PHY_MISC_B) +#define DG2_PHY_MISC(port) ((port) == PHY_E ? _MMIO(_DG2_PHY_MISC_TC1) : \
ICL_PHY_MISC(port))#define ICL_PHY_MISC_MUX_DDID (1 << 28) #define ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN (1 << 23) #define DG2_PHY_DP_TX_ACK_MASK REG_GENMASK(23, 20) -- 2.20.1
BR,
Jouni Högander
On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote:
From: Jouni Högander jouni.hogander@intel.com
Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E port. Correct offset is 0x64C14.
Fix this by handling PHY_E port seprately.
order of the patch here is wrong. This patch should come before the patch initializing the 5th port. Then the commit message is not a fix.
This can be done while applying since it's more an order to avoid breaking the tree.
Lucas De Marchi
dri-devel@lists.freedesktop.org
-
Hogander, Jouni -
Lucas De Marchi -
Matt Roper -
Ramalingam C -
Shankar, Uma -
Ville Syrjälä