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