On Tue, Aug 09, 2016 at 02:34:09PM +0300, Imre Deak wrote:
> Atm the LVDS encoder depends on the PPS HW context being saved/restored
> from generic suspend/resume code. Since the PPS is specific to the LVDS
> and eDP encoders a cleaner way is to reinitialize it during encoder
> enabling, so do this here for LVDS. Follow-up patches will init the PPS
> for the eDP encoder similarly and remove the suspend/resume time save /
> restore.
>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_lvds.c | 103 +++++++++++++++++++++++++++++++++-----
> 2 files changed, 91 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 889508f..da82744 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3710,6 +3710,7 @@ enum {
>
> #define _PP_ON_DELAYS 0x61208
> #define PP_ON_DELAYS(pps_idx) _MMIO_PPS(pps_idx, _PP_ON_DELAYS)
> +#define PANEL_PORT_SELECT_SHIFT 30
> #define PANEL_PORT_SELECT_MASK (3 << 30)
> #define PANEL_PORT_SELECT_LVDS (0 << 30)
> #define PANEL_PORT_SELECT_DPA (1 << 30)
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index c5739fc..d5158e5 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -48,6 +48,20 @@ struct intel_lvds_connector {
> struct notifier_block lid_notifier;
> };
>
> +struct intel_lvds_pps {
> + /* 100us units */
> + int t1_t2;
> + int t3;
> + int t4;
> + int t5;
> + int tx;
> +
> + int divider;
> +
> + int port;
> + bool reset_on_powerdown;
> +};
> +
> struct intel_lvds_encoder {
> struct intel_encoder base;
>
> @@ -55,6 +69,9 @@ struct intel_lvds_encoder {
> i915_reg_t reg;
> u32 a3_power;
>
> + struct intel_lvds_pps init_pps;
> + u32 init_lvds_val;
> +
> struct intel_lvds_connector *attached_connector;
> };
>
> @@ -136,6 +153,72 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
> pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
> }
>
> +static void intel_lvds_pps_get_hw_state(struct drm_i915_private *dev_priv,
> + struct intel_lvds_pps *pps)
> +{
> + u32 val;
> +
> + pps->reset_on_powerdown = I915_READ(PP_CONTROL(0)) & PANEL_POWER_RESET;
> +
> + val = I915_READ(PP_ON_DELAYS(0));
> + pps->port = (val & PANEL_PORT_SELECT_MASK) >>
> + PANEL_PORT_SELECT_SHIFT;
> + pps->t1_t2 = (val & PANEL_POWER_UP_DELAY_MASK) >>
> + PANEL_POWER_UP_DELAY_SHIFT;
> + pps->t5 = (val & PANEL_LIGHT_ON_DELAY_MASK) >>
> + PANEL_LIGHT_ON_DELAY_SHIFT;
> +
> + val = I915_READ(PP_OFF_DELAYS(0));
> + pps->t3 = (val & PANEL_POWER_DOWN_DELAY_MASK) >>
> + PANEL_POWER_DOWN_DELAY_SHIFT;
> + pps->tx = (val & PANEL_LIGHT_OFF_DELAY_MASK) >>
> + PANEL_LIGHT_OFF_DELAY_SHIFT;
> +
> + val = I915_READ(PP_DIVISOR(0));
> + pps->divider = (val & PP_REFERENCE_DIVIDER_MASK) >>
> + PP_REFERENCE_DIVIDER_SHIFT;
> + /* Convert from 100ms to 100us units */
> + pps->t4 = ((val & PANEL_POWER_CYCLE_DELAY_MASK) >>
> + PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
This should have the +1 offset handling I think. Hmm. Looks like we mess
that up even for eDP :(
> +
> + if (INTEL_INFO(dev_priv)->gen <= 4 &&
> + pps->t1_t2 == 0 && pps->t5 == 0 && pps->t3 == 0 && pps->tx == 0) {
> + DRM_DEBUG_KMS("Panel power timings uninitialized, "
> + "setting defaults\n");
> + /* Set T2 to 40ms and T5 to 200ms in 100 usec units */
> + pps->t1_t2 = 40 * 10;
> + pps->t5 = 200 * 10;
> + /* Set T3 to 35ms and Tx to 200ms in 100 usec units */
> + pps->t3 = 35 * 10;
> + pps->tx = 200 * 10;
Not sure where these came from originally. The spgw spec tells us:
min max
t1 0.5 10
t2 0 50
t3 0 50
t4 200 -
t5 200 -
t6 200 -
t7 0 10
So maybe we should at least set t4=200 here based on the minimum t4
delay in the spec, probably should add the max t7 time there as well
as that's what the power cycle delays seems to be really: t4+t7.
Bspec seems to claim that spgw spec has an upper limit of 400 for this,
but I can't see that at least in the copy I found.
And maybe bump up t1_t2=60 and t3=50 based on the max delay for those
in the spec?
Anyways, this is all material for a separate patch for sure.
> + }
> +
> + DRM_DEBUG_DRIVER("LVDS PPS:t1+t2 %d t3 %d t4 %d t5 %d tx %d "
> + "divider %d port %d reset_on_powerdown %d\n",
> + pps->t1_t2, pps->t3, pps->t4, pps->t5, pps->tx,
> + pps->divider, pps->port, pps->reset_on_powerdown);
> +}
> +
> +static void intel_lvds_pps_init_hw(struct drm_i915_private *dev_priv,
> + struct intel_lvds_pps *pps)
> +{
> + u32 val;
> +
> + val = I915_READ(PP_CONTROL(0));
> + WARN_ON((val & PANEL_UNLOCK_MASK) != PANEL_UNLOCK_REGS);
> + if (pps->reset_on_powerdown)
> + val |= PANEL_POWER_RESET;
Hmm. Maybe we can just always set this bit? Hmm, we also totally fail to
deal with the related IBX w/a in the LVDS code. Should probably fix that
up as well as a followup.
> + I915_WRITE(PP_CONTROL(0), val);
> +
> + I915_WRITE(PP_ON_DELAYS(0), (pps->port << PANEL_PORT_SELECT_SHIFT) |
> + (pps->t1_t2 << PANEL_POWER_UP_DELAY_SHIFT) |
> + (pps->t5 << PANEL_LIGHT_ON_DELAY_SHIFT));
> + I915_WRITE(PP_OFF_DELAYS(0), (pps->t3 << PANEL_POWER_DOWN_DELAY_SHIFT) |
> + (pps->tx << PANEL_LIGHT_OFF_DELAY_SHIFT));
> + I915_WRITE(PP_DIVISOR(0), (pps->divider << PP_REFERENCE_DIVIDER_SHIFT) |
> + ((pps->t4 / 1000) << PANEL_POWER_CYCLE_DELAY_SHIFT));
DIV_ROUND_UP() I think, and maybe the +1 handling. Though I can't really
tell if the spec has intended the +1 to be a replacement for rounding
up, or do we need to do both. Both would seem like the safe choice. Gen2
Bspec doesn't mention the +1, but it says writing 0 will even abort any
ongoing delay.
> +}
> +
> static void intel_pre_enable_lvds(struct intel_encoder *encoder)
> {
> struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
> @@ -154,7 +237,9 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
> assert_pll_disabled(dev_priv, pipe);
> }
>
> - temp = I915_READ(lvds_encoder->reg);
> + intel_lvds_pps_init_hw(dev_priv, &lvds_encoder->init_pps);
> +
> + temp = lvds_encoder->init_lvds_val;
> temp |= LVDS_PORT_EN | LVDS_A0A2_CLKA_POWER_UP;
>
> if (HAS_PCH_CPT(dev)) {
> @@ -922,18 +1007,6 @@ void intel_lvds_init(struct drm_device *dev)
> DRM_DEBUG_KMS("LVDS is not present in VBT, but enabled anyway\n");
> }
>
> - /* Set the Panel Power On/Off timings if uninitialized. */
> - if (INTEL_INFO(dev_priv)->gen < 5 &&
> - I915_READ(PP_ON_DELAYS(0)) == 0 && I915_READ(PP_OFF_DELAYS(0)) == 0) {
> - /* Set T2 to 40ms and T5 to 200ms */
> - I915_WRITE(PP_ON_DELAYS(0), 0x019007d0);
> -
> - /* Set T3 to 35ms and Tx to 200ms */
> - I915_WRITE(PP_OFF_DELAYS(0), 0x015e07d0);
> -
> - DRM_DEBUG_KMS("Panel power timings uninitialized, setting defaults\n");
> - }
> -
> lvds_encoder = kzalloc(sizeof(*lvds_encoder), GFP_KERNEL);
> if (!lvds_encoder)
> return;
> @@ -999,6 +1072,10 @@ void intel_lvds_init(struct drm_device *dev)
> dev->mode_config.scaling_mode_property,
> DRM_MODE_SCALE_ASPECT);
> intel_connector->panel.fitting_mode = DRM_MODE_SCALE_ASPECT;
> +
> + intel_lvds_pps_get_hw_state(dev_priv, &lvds_encoder->init_pps);
> + lvds_encoder->init_lvds_val = lvds;
> +
> /*
> * LVDS discovery:
> * 1) check for EDID on DDC
> --
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
