On Mon, Oct 02, 2017 at 01:53:07PM +0000, Imre Deak wrote:
> The common lane power down flag of a DPIO PHY has a funky semantic:
> after the initial enabling of the PHY (so from a disabled state) this
> flag will be clear. It will be set only after the PHY will be used for
> the first time (for instance due to enabling the corresponding pipe) and
> then become unused (due to disabling the pipe). During the initial PHY
> enablement we don't know which of the above phases we are in, so move
> the check for the flag where this is known, the HW readout code. This is
> where the rest of lane power down status checks are done anyway.
This makes sense. I just wonder why at first place we were doing that
extra check to see if that was disabled.
I couldn't find anything on spec to justify the previous use of it and
new one is consistent with bit 9 that is very similar, so:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>
> This fixes at least a problem on GLK where after module reloading, the
> common lane power down flag of PHY1 is set, but the PHY is actually
> powered-on and properly set up. The GRC readout code for other PHYs will
> hence think that PHY1 is not powered initially and disable it after the
> GRC readout. This will cause the AUX power well related to PHY1 to get
> disabled in a stuck state, timing out when we try to enable it later.
>
> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
> Fixes: e93da0a0137b ("drm/i915/bxt: Sanitiy check the PHY lane power down status")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102777
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 3 ++-
> drivers/gpu/drm/i915/intel_dpio_phy.c | 20 --------------------
> 2 files changed, 2 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 93cbbcbbc193..65f4b6786791 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1713,7 +1713,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> out:
> if (ret && IS_GEN9_LP(dev_priv)) {
> tmp = I915_READ(BXT_PHY_CTL(port));
> - if ((tmp & (BXT_PHY_LANE_POWERDOWN_ACK |
> + if ((tmp & (BXT_PHY_CMNLANE_POWERDOWN_ACK |
> + BXT_PHY_LANE_POWERDOWN_ACK |
> BXT_PHY_LANE_ENABLED)) != BXT_PHY_LANE_ENABLED)
> DRM_ERROR("Port %c enabled but PHY powered down? "
> "(PHY_CTL %08x)\n", port_name(port), tmp);
> diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
> index 09b670929786..de38d014ed39 100644
> --- a/drivers/gpu/drm/i915/intel_dpio_phy.c
> +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
> @@ -208,12 +208,6 @@ static const struct bxt_ddi_phy_info glk_ddi_phy_info[] = {
> },
> };
>
> -static u32 bxt_phy_port_mask(const struct bxt_ddi_phy_info *phy_info)
> -{
> - return (phy_info->dual_channel * BIT(phy_info->channel[DPIO_CH1].port)) |
> - BIT(phy_info->channel[DPIO_CH0].port);
> -}
> -
> static const struct bxt_ddi_phy_info *
> bxt_get_phy_list(struct drm_i915_private *dev_priv, int *count)
> {
> @@ -313,7 +307,6 @@ bool bxt_ddi_phy_is_enabled(struct drm_i915_private *dev_priv,
> enum dpio_phy phy)
> {
> const struct bxt_ddi_phy_info *phy_info;
> - enum port port;
>
> phy_info = bxt_get_phy_info(dev_priv, phy);
>
> @@ -335,19 +328,6 @@ bool bxt_ddi_phy_is_enabled(struct drm_i915_private *dev_priv,
> return false;
> }
>
> - for_each_port_masked(port, bxt_phy_port_mask(phy_info)) {
> - u32 tmp = I915_READ(BXT_PHY_CTL(port));
> -
> - if (tmp & BXT_PHY_CMNLANE_POWERDOWN_ACK) {
> - DRM_DEBUG_DRIVER("DDI PHY %d powered, but common lane "
> - "for port %c powered down "
> - "(PHY_CTL %08x)\n",
> - phy, port_name(port), tmp);
> -
> - return false;
> - }
> - }
> -
> return true;
> }
>
> --
> 2.13.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
