On Tue, Sep 03, 2013 at 02:43:39PM +0300, Jani Nikula wrote:
> VLV has per-pipe PP registers. Set up power sequencing on mode set. The
> connector init time setup is problematic, since we don't have a pipe at
> that time. Cook up something.
>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 139 +++++++++++++++++++++++++++------------
> 1 file changed, 97 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0ba72f1..95d67fc 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -237,24 +237,47 @@ intel_hrawclk(struct drm_device *dev)
> }
> }
>
> +static void
> +intel_dp_init_panel_power_sequencer(struct drm_device *dev,
> + struct intel_dp *intel_dp,
> + struct edp_power_seq *out);
> +static void
> +intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> + struct intel_dp *intel_dp,
> + struct edp_power_seq *out);
> +
> +static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
> +{
> + struct drm_device *dev = intel_dp_to_dev(intel_dp);
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + int pipe = to_intel_crtc(intel_dig_port->base.base.crtc)->pipe;
> +
> + return HAS_PCH_SPLIT(dev) ? PCH_PP_CONTROL : VLV_PIPE_PP_CONTROL(pipe);
> +}
> +
> +static u32 _pp_stat_reg(struct intel_dp *intel_dp)
> +{
> + struct drm_device *dev = intel_dp_to_dev(intel_dp);
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + int pipe = to_intel_crtc(intel_dig_port->base.base.crtc)->pipe;
> +
> + return HAS_PCH_SPLIT(dev) ? PCH_PP_STATUS : VLV_PIPE_PP_STATUS(pipe);
> +}
> +
> static bool ironlake_edp_have_panel_power(struct intel_dp *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 pp_stat_reg;
>
> - pp_stat_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_STATUS : PCH_PP_STATUS;
> - return (I915_READ(pp_stat_reg) & PP_ON) != 0;
> + return (I915_READ(_pp_stat_reg(intel_dp)) & PP_ON) != 0;
> }
>
> static bool ironlake_edp_have_panel_vdd(struct intel_dp *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 pp_ctrl_reg;
>
> - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL;
> - return (I915_READ(pp_ctrl_reg) & EDP_FORCE_VDD) != 0;
> + return (I915_READ(_pp_ctrl_reg(intel_dp)) & EDP_FORCE_VDD) != 0;
> }
>
> static void
> @@ -262,19 +285,15 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 pp_stat_reg, pp_ctrl_reg;
>
> if (!is_edp(intel_dp))
> return;
>
> - pp_stat_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_STATUS : PCH_PP_STATUS;
> - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL;
> -
> if (!ironlake_edp_have_panel_power(intel_dp) && !ironlake_edp_have_panel_vdd(intel_dp)) {
> WARN(1, "eDP powered off while attempting aux channel communication.\n");
> DRM_DEBUG_KMS("Status 0x%08x Control 0x%08x\n",
> - I915_READ(pp_stat_reg),
> - I915_READ(pp_ctrl_reg));
> + I915_READ(_pp_stat_reg(intel_dp)),
> + I915_READ(_pp_ctrl_reg(intel_dp)));
> }
> }
>
> @@ -948,8 +967,8 @@ static void ironlake_wait_panel_status(struct intel_dp *intel_dp,
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 pp_stat_reg, pp_ctrl_reg;
>
> - pp_stat_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_STATUS : PCH_PP_STATUS;
> - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL;
> + pp_stat_reg = _pp_stat_reg(intel_dp);
> + pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
>
> DRM_DEBUG_KMS("mask %08x value %08x status %08x control %08x\n",
> mask, value,
> @@ -991,11 +1010,8 @@ static u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 control;
> - u32 pp_ctrl_reg;
> -
> - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL;
> - control = I915_READ(pp_ctrl_reg);
>
> + control = I915_READ(_pp_ctrl_reg(intel_dp));
> control &= ~PANEL_UNLOCK_MASK;
> control |= PANEL_UNLOCK_REGS;
> return control;
> @@ -1028,8 +1044,8 @@ void ironlake_edp_panel_vdd_on(struct intel_dp *intel_dp)
> pp = ironlake_get_pp_control(intel_dp);
> pp |= EDP_FORCE_VDD;
>
> - pp_stat_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_STATUS : PCH_PP_STATUS;
> - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL;
> + pp_stat_reg = _pp_stat_reg(intel_dp);
> + pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
>
> I915_WRITE(pp_ctrl_reg, pp);
> POSTING_READ(pp_ctrl_reg);
> @@ -1057,8 +1073,8 @@ static void ironlake_panel_vdd_off_sync(struct intel_dp *intel_dp)
> pp = ironlake_get_pp_control(intel_dp);
> pp &= ~EDP_FORCE_VDD;
>
> - pp_stat_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_STATUS : PCH_PP_STATUS;
> - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL;
> + pp_stat_reg = _pp_ctrl_reg(intel_dp);
> + pp_ctrl_reg = _pp_stat_reg(intel_dp);
>
> I915_WRITE(pp_ctrl_reg, pp);
> POSTING_READ(pp_ctrl_reg);
> @@ -1123,20 +1139,19 @@ void ironlake_edp_panel_on(struct intel_dp *intel_dp)
>
> ironlake_wait_panel_power_cycle(intel_dp);
>
> + pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
> pp = ironlake_get_pp_control(intel_dp);
> if (IS_GEN5(dev)) {
> /* ILK workaround: disable reset around power sequence */
> pp &= ~PANEL_POWER_RESET;
> - I915_WRITE(PCH_PP_CONTROL, pp);
> - POSTING_READ(PCH_PP_CONTROL);
> + I915_WRITE(pp_ctrl_reg, pp);
> + POSTING_READ(pp_ctrl_reg);
> }
>
> pp |= POWER_TARGET_ON;
> if (!IS_GEN5(dev))
> pp |= PANEL_POWER_RESET;
>
> - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL;
> -
> I915_WRITE(pp_ctrl_reg, pp);
> POSTING_READ(pp_ctrl_reg);
>
> @@ -1144,8 +1159,8 @@ void ironlake_edp_panel_on(struct intel_dp *intel_dp)
>
> if (IS_GEN5(dev)) {
> pp |= PANEL_POWER_RESET; /* restore panel reset bit */
> - I915_WRITE(PCH_PP_CONTROL, pp);
> - POSTING_READ(PCH_PP_CONTROL);
> + I915_WRITE(pp_ctrl_reg, pp);
> + POSTING_READ(pp_ctrl_reg);
> }
> }
>
> @@ -1168,7 +1183,7 @@ void ironlake_edp_panel_off(struct intel_dp *intel_dp)
> * panels get very unhappy and cease to work. */
> pp &= ~(POWER_TARGET_ON | EDP_FORCE_VDD | PANEL_POWER_RESET | EDP_BLC_ENABLE);
>
> - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL;
> + pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
>
> I915_WRITE(pp_ctrl_reg, pp);
> POSTING_READ(pp_ctrl_reg);
> @@ -1201,7 +1216,7 @@ void ironlake_edp_backlight_on(struct intel_dp *intel_dp)
> pp = ironlake_get_pp_control(intel_dp);
> pp |= EDP_BLC_ENABLE;
>
> - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL;
> + pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
>
> I915_WRITE(pp_ctrl_reg, pp);
> POSTING_READ(pp_ctrl_reg);
> @@ -1225,7 +1240,7 @@ void ironlake_edp_backlight_off(struct intel_dp *intel_dp)
> pp = ironlake_get_pp_control(intel_dp);
> pp &= ~EDP_BLC_ENABLE;
>
> - pp_ctrl_reg = IS_VALLEYVIEW(dev) ? PIPEA_PP_CONTROL : PCH_PP_CONTROL;
> + pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
>
> I915_WRITE(pp_ctrl_reg, pp);
> POSTING_READ(pp_ctrl_reg);
> @@ -1748,6 +1763,7 @@ static void vlv_pre_enable_dp(struct intel_encoder *encoder)
> struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> int port = vlv_dport_to_channel(dport);
> int pipe = intel_crtc->pipe;
> + struct edp_power_seq power_seq;
> u32 val;
>
> mutex_lock(&dev_priv->dpio_lock);
> @@ -1765,6 +1781,10 @@ static void vlv_pre_enable_dp(struct intel_encoder *encoder)
>
> mutex_unlock(&dev_priv->dpio_lock);
>
> + /* init power sequencer on this pipe and port */
> + intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
> + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, &power_seq);
> +
> intel_enable_dp(encoder);
>
> vlv_wait_port_ready(dev_priv, port);
> @@ -3146,6 +3166,34 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
> }
> }
>
> +static enum pipe
> +vlv_power_sequencer_pipe(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + enum port port = intel_dig_port->port;
> + enum pipe pipe;
> +
> + /* modeset should have pipe */
> + if (crtc)
> + return to_intel_crtc(crtc)->pipe;
> +
> + /* init time, try to find a pipe with this port selected */
> + for (pipe = PIPE_A; pipe <= PIPE_B; pipe++) {
> + u32 port_sel = I915_READ(VLV_PIPE_PP_ON_DELAYS(pipe)) &
> + PANEL_PORT_SELECT_MASK;
> + if (port_sel == PANEL_PORT_SELECT_DPB_VLV && port == PORT_B)
> + return pipe;
> + else if (port_sel == PANEL_PORT_SELECT_DPC && port == PORT_C)
> + return pipe;
> + }
> +
> + /* shrug */
> + return PIPE_A;
Yeah, looks reasonable enough to me. If the BIOS already set up
something keep using it, otherwise just pick A.
The only concern here is whether the power sequencer requires something
else to be set up? Maybe we'd need the DP port register to have the
matching pipe configured too? Would need to test that on real hardware I
suppose.
But to me this seems at least better than the current use pipe A
always situation:
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> +}
> +
> static void
> intel_dp_init_panel_power_sequencer(struct drm_device *dev,
> struct intel_dp *intel_dp,
> @@ -3154,24 +3202,26 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct edp_power_seq cur, vbt, spec, final;
> u32 pp_on, pp_off, pp_div, pp;
> - int pp_control_reg, pp_on_reg, pp_off_reg, pp_div_reg;
> + int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg;
>
> if (HAS_PCH_SPLIT(dev)) {
> - pp_control_reg = PCH_PP_CONTROL;
> + pp_ctrl_reg = PCH_PP_CONTROL;
> pp_on_reg = PCH_PP_ON_DELAYS;
> pp_off_reg = PCH_PP_OFF_DELAYS;
> pp_div_reg = PCH_PP_DIVISOR;
> } else {
> - pp_control_reg = PIPEA_PP_CONTROL;
> - pp_on_reg = PIPEA_PP_ON_DELAYS;
> - pp_off_reg = PIPEA_PP_OFF_DELAYS;
> - pp_div_reg = PIPEA_PP_DIVISOR;
> + enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
> +
> + pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
> + pp_on_reg = VLV_PIPE_PP_ON_DELAYS(pipe);
> + pp_off_reg = VLV_PIPE_PP_OFF_DELAYS(pipe);
> + pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
> }
>
> /* Workaround: Need to write PP_CONTROL with the unlock key as
> * the very first thing. */
> pp = ironlake_get_pp_control(intel_dp);
> - I915_WRITE(pp_control_reg, pp);
> + I915_WRITE(pp_ctrl_reg, pp);
>
> pp_on = I915_READ(pp_on_reg);
> pp_off = I915_READ(pp_off_reg);
> @@ -3259,9 +3309,11 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> pp_off_reg = PCH_PP_OFF_DELAYS;
> pp_div_reg = PCH_PP_DIVISOR;
> } else {
> - pp_on_reg = PIPEA_PP_ON_DELAYS;
> - pp_off_reg = PIPEA_PP_OFF_DELAYS;
> - pp_div_reg = PIPEA_PP_DIVISOR;
> + enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
> +
> + pp_on_reg = VLV_PIPE_PP_ON_DELAYS(pipe);
> + pp_off_reg = VLV_PIPE_PP_OFF_DELAYS(pipe);
> + pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
> }
>
> /* And finally store the new values in the power sequencer. */
> @@ -3278,7 +3330,10 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> /* Haswell doesn't have any port selection bits for the panel
> * power sequencer any more. */
> if (IS_VALLEYVIEW(dev)) {
> - port_sel = I915_READ(pp_on_reg) & 0xc0000000;
> + if (dp_to_dig_port(intel_dp)->port == PORT_B)
> + port_sel = PANEL_PORT_SELECT_DPB_VLV;
> + else
> + port_sel = PANEL_PORT_SELECT_DPC;
> } else if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
> if (dp_to_dig_port(intel_dp)->port == PORT_A)
> port_sel = PANEL_PORT_SELECT_DPA;
> --
> 1.7.9.5
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
