Skip to content

Commit

Permalink
drm/i915: Prevent PPS stealing from a normal DP port on VLV/CHV
Browse files Browse the repository at this point in the history
VLV apparently gets upset if the PPS for a pipe currently driving an
external DP port gets used for VDD stuff on another eDP port. The DP
port falls over and fails to retrain when this happens, leaving the
user staring at a black screen.

Let's fix it by also tracking which pipe is driving which DP/eDP port.
We'll track this under intel_dp so that we'll share the protection
of the pps_mutex alongside the pps_pipe tracking, since the two
things are intimately related.

I had plans to reduce the protection of pps_mutex to cover only eDP
ports, but with this we can't do that. Well, for for VLV/CHV at least.
For other platforms it should still be possible, which would allow
AUX communication to occur in parallel for multiple DP ports.

v2: Drop stray crap from a comment (Imre)
    Grab pps_mutex when clearing active_pipe
    Fix a typo in the commit message
v3: Make vlv_active_pipe() static

Cc: stable@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1481738423-29738-1-git-send-email-ville.syrjala@linux.intel.com
  • Loading branch information
Ville Syrjälä committed Dec 19, 2016
1 parent bf51997 commit 9f2bdb0
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 47 deletions.
152 changes: 105 additions & 47 deletions drivers/gpu/drm/i915/intel_dp.c
Original file line number Diff line number Diff line change
Expand Up @@ -492,48 +492,71 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp)
}
}

static enum pipe vlv_find_free_pps(struct drm_i915_private *dev_priv)
{
struct intel_encoder *encoder;
unsigned int pipes = (1 << PIPE_A) | (1 << PIPE_B);

/*
* We don't have power sequencer currently.
* Pick one that's not used by other ports.
*/
for_each_intel_encoder(&dev_priv->drm, encoder) {
struct intel_dp *intel_dp;

if (encoder->type != INTEL_OUTPUT_DP &&
encoder->type != INTEL_OUTPUT_EDP)
continue;

intel_dp = enc_to_intel_dp(&encoder->base);

if (encoder->type == INTEL_OUTPUT_EDP) {
WARN_ON(intel_dp->active_pipe != INVALID_PIPE &&
intel_dp->active_pipe != intel_dp->pps_pipe);

if (intel_dp->pps_pipe != INVALID_PIPE)
pipes &= ~(1 << intel_dp->pps_pipe);
} else {
WARN_ON(intel_dp->pps_pipe != INVALID_PIPE);

if (intel_dp->active_pipe != INVALID_PIPE)
pipes &= ~(1 << intel_dp->active_pipe);
}
}

if (pipes == 0)
return INVALID_PIPE;

return ffs(pipes) - 1;
}

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_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_encoder *encoder;
unsigned int pipes = (1 << PIPE_A) | (1 << PIPE_B);
enum pipe pipe;

lockdep_assert_held(&dev_priv->pps_mutex);

/* We should never land here with regular DP ports */
WARN_ON(!is_edp(intel_dp));

WARN_ON(intel_dp->active_pipe != INVALID_PIPE &&
intel_dp->active_pipe != intel_dp->pps_pipe);

if (intel_dp->pps_pipe != INVALID_PIPE)
return intel_dp->pps_pipe;

/*
* We don't have power sequencer currently.
* Pick one that's not used by other ports.
*/
for_each_intel_encoder(dev, encoder) {
struct intel_dp *tmp;

if (encoder->type != INTEL_OUTPUT_EDP)
continue;

tmp = enc_to_intel_dp(&encoder->base);

if (tmp->pps_pipe != INVALID_PIPE)
pipes &= ~(1 << tmp->pps_pipe);
}
pipe = vlv_find_free_pps(dev_priv);

/*
* Didn't find one. This should not happen since there
* are two power sequencers and up to two eDP ports.
*/
if (WARN_ON(pipes == 0))
if (WARN_ON(pipe == INVALID_PIPE))
pipe = PIPE_A;
else
pipe = ffs(pipes) - 1;

vlv_steal_power_sequencer(dev, pipe);
intel_dp->pps_pipe = pipe;
Expand Down Expand Up @@ -689,10 +712,17 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
for_each_intel_encoder(dev, encoder) {
struct intel_dp *intel_dp;

if (encoder->type != INTEL_OUTPUT_EDP)
if (encoder->type != INTEL_OUTPUT_DP &&
encoder->type != INTEL_OUTPUT_EDP)
continue;

intel_dp = enc_to_intel_dp(&encoder->base);

WARN_ON(intel_dp->active_pipe != INVALID_PIPE);

if (encoder->type != INTEL_OUTPUT_EDP)
continue;

if (IS_GEN9_LP(dev_priv))
intel_dp->pps_reset = true;
else
Expand Down Expand Up @@ -2852,6 +2882,8 @@ static void vlv_detach_power_sequencer(struct intel_dp *intel_dp)
enum pipe pipe = intel_dp->pps_pipe;
i915_reg_t pp_on_reg = PP_ON_DELAYS(pipe);

WARN_ON(intel_dp->active_pipe != INVALID_PIPE);

edp_panel_vdd_off_sync(intel_dp);

/*
Expand Down Expand Up @@ -2886,22 +2918,23 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
struct intel_dp *intel_dp;
enum port port;

if (encoder->type != INTEL_OUTPUT_EDP)
if (encoder->type != INTEL_OUTPUT_DP &&
encoder->type != INTEL_OUTPUT_EDP)
continue;

intel_dp = enc_to_intel_dp(&encoder->base);
port = dp_to_dig_port(intel_dp)->port;

WARN(intel_dp->active_pipe == pipe,
"stealing pipe %c power sequencer from active (e)DP port %c\n",
pipe_name(pipe), port_name(port));

if (intel_dp->pps_pipe != pipe)
continue;

DRM_DEBUG_KMS("stealing pipe %c power sequencer from port %c\n",
pipe_name(pipe), port_name(port));

WARN(encoder->base.crtc,
"stealing pipe %c power sequencer from active eDP port %c\n",
pipe_name(pipe), port_name(port));

/* make sure vdd is off before we steal it */
vlv_detach_power_sequencer(intel_dp);
}
Expand All @@ -2917,26 +2950,29 @@ static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp)

lockdep_assert_held(&dev_priv->pps_mutex);

if (!is_edp(intel_dp))
return;
WARN_ON(intel_dp->active_pipe != INVALID_PIPE);

if (intel_dp->pps_pipe == crtc->pipe)
return;

/*
* If another power sequencer was being used on this
* port previously make sure to turn off vdd there while
* we still have control of it.
*/
if (intel_dp->pps_pipe != INVALID_PIPE)
if (intel_dp->pps_pipe != INVALID_PIPE &&
intel_dp->pps_pipe != crtc->pipe) {
/*
* If another power sequencer was being used on this
* port previously make sure to turn off vdd there while
* we still have control of it.
*/
vlv_detach_power_sequencer(intel_dp);
}

/*
* We may be stealing the power
* sequencer from another port.
*/
vlv_steal_power_sequencer(dev, crtc->pipe);

intel_dp->active_pipe = crtc->pipe;

if (!is_edp(intel_dp))
return;

/* now it's all ours */
intel_dp->pps_pipe = crtc->pipe;

Expand Down Expand Up @@ -3523,6 +3559,12 @@ intel_dp_link_down(struct intel_dp *intel_dp)
msleep(intel_dp->panel_power_down_delay);

intel_dp->DP = DP;

if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
pps_lock(intel_dp);
intel_dp->active_pipe = INVALID_PIPE;
pps_unlock(intel_dp);
}
}

bool
Expand Down Expand Up @@ -4791,6 +4833,19 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
edp_panel_vdd_schedule_off(intel_dp);
}

static enum pipe vlv_active_pipe(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));

if ((intel_dp->DP & DP_PORT_EN) == 0)
return INVALID_PIPE;

if (IS_CHERRYVIEW(dev_priv))
return DP_PORT_TO_PIPE_CHV(intel_dp->DP);
else
return PORT_TO_PIPE(intel_dp->DP);
}

void intel_dp_encoder_reset(struct drm_encoder *encoder)
{
struct drm_i915_private *dev_priv = to_i915(encoder->dev);
Expand All @@ -4803,14 +4858,16 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder)
if (lspcon->active)
lspcon_resume(lspcon);

if (to_intel_encoder(encoder)->type != INTEL_OUTPUT_EDP)
return;

pps_lock(intel_dp);

/* Reinit the power sequencer, in case BIOS did something with it. */
intel_dp_pps_init(encoder->dev, intel_dp);
intel_edp_panel_vdd_sanitize(intel_dp);
if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
intel_dp->active_pipe = vlv_active_pipe(intel_dp);

if (is_edp(intel_dp)) {
/* Reinit the power sequencer, in case BIOS did something with it. */
intel_dp_pps_init(encoder->dev, intel_dp);
intel_edp_panel_vdd_sanitize(intel_dp);
}

pps_unlock(intel_dp);
}
Expand Down Expand Up @@ -5637,10 +5694,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
* If the current pipe isn't valid, try the PPS pipe, and if that
* fails just assume pipe A.
*/
if (IS_CHERRYVIEW(dev_priv))
pipe = DP_PORT_TO_PIPE_CHV(intel_dp->DP);
else
pipe = PORT_TO_PIPE(intel_dp->DP);
pipe = vlv_active_pipe(intel_dp);

if (pipe != PIPE_A && pipe != PIPE_B)
pipe = intel_dp->pps_pipe;
Expand Down Expand Up @@ -5689,6 +5743,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
return false;

intel_dp->pps_pipe = INVALID_PIPE;
intel_dp->active_pipe = INVALID_PIPE;

/* intel_dp vfuncs */
if (INTEL_GEN(dev_priv) >= 9)
Expand Down Expand Up @@ -5717,6 +5772,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
else
type = DRM_MODE_CONNECTOR_DisplayPort;

if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
intel_dp->active_pipe = vlv_active_pipe(intel_dp);

/*
* For eDP we always set the encoder type to INTEL_OUTPUT_EDP, but
* for DP the encoder type can be set by the caller to
Expand Down
6 changes: 6 additions & 0 deletions drivers/gpu/drm/i915/intel_drv.h
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,12 @@ struct intel_dp {
* this port. Only relevant on VLV/CHV.
*/
enum pipe pps_pipe;
/*
* Pipe currently driving the port. Used for preventing
* the use of the PPS for any pipe currentrly driving
* external DP as that will mess things up on VLV.
*/
enum pipe active_pipe;
/*
* Set if the sequencer may be reset due to a power transition,
* requiring a reinitialization. Only relevant on BXT.
Expand Down

0 comments on commit 9f2bdb0

Please sign in to comment.