On Tue, Mar 15, 2016 at 09:51:16PM +0200, Jani Nikula wrote:
> The BXT display connections have DSI transcoders A and C that can be
> muxed to any pipe, not unlike the eDP transcoder. Add the notion of DSI
> transcoders.
>
> The "normal" transcoders A, B and C are not used with BXT DSI, so care
> must be taken to avoid accessing those registers with DSI transcoders in
> the hardware state readout, modeset, and generally everywhere.
>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 10 ++++
> drivers/gpu/drm/i915/intel_ddi.c | 6 +++
> drivers/gpu/drm/i915/intel_display.c | 90 +++++++++++++++++++++++++++++----
> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_dsi.c | 9 ++++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 6 +++
> 6 files changed, 112 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5e4a42d996d8..0ddd0f492ca9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -123,6 +123,8 @@ enum transcoder {
> TRANSCODER_B,
> TRANSCODER_C,
> TRANSCODER_EDP,
> + TRANSCODER_DSI_A,
> + TRANSCODER_DSI_C,
> I915_MAX_TRANSCODERS
> };
>
> @@ -137,11 +139,17 @@ static inline const char *transcoder_name(enum transcoder transcoder)
> return "C";
> case TRANSCODER_EDP:
> return "eDP";
> + case TRANSCODER_DSI_A:
> + return "DSI A";
> + case TRANSCODER_DSI_C:
> + return "DSI C";
> default:
> return "<invalid>";
> }
> }
>
> +#define IS_DSI_TRANSCODER(t) ((t) == TRANSCODER_DSI_A || (t) == TRANSCODER_DSI_C)
> +
> /*
> * I915_MAX_PLANES in the enum below is the maximum (across all platforms)
> * number of planes per CRTC. Not all platforms really have this many planes,
> @@ -192,6 +200,8 @@ enum intel_display_power_domain {
> POWER_DOMAIN_TRANSCODER_B,
> POWER_DOMAIN_TRANSCODER_C,
> POWER_DOMAIN_TRANSCODER_EDP,
> + POWER_DOMAIN_TRANSCODER_DSI_A,
> + POWER_DOMAIN_TRANSCODER_DSI_C,
> POWER_DOMAIN_PORT_DDI_A_LANES,
> POWER_DOMAIN_PORT_DDI_B_LANES,
> POWER_DOMAIN_PORT_DDI_C_LANES,
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 91654ffc3a42..8726711864eb 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1061,6 +1061,8 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
> uint32_t temp;
>
> if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP_MST) {
> + WARN_ON(IS_DSI_TRANSCODER(cpu_transcoder));
That all caps IS_DSI_TRANSCODER doesn't really agree with me for some
reason. static inline with lower caps name maybe?
> +
> temp = TRANS_MSA_SYNC_CLK;
> switch (intel_crtc->config->pipe_bpp) {
> case 18:
> @@ -1942,6 +1944,10 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> struct intel_hdmi *intel_hdmi;
> u32 temp, flags = 0;
>
> + /* XXX: DSI transcoder paranoia */
> + if (WARN_ON(IS_DSI_TRANSCODER(cpu_transcoder)))
> + return;
> +
> temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> if (temp & TRANS_DDI_PHSYNC)
> flags |= DRM_MODE_FLAG_PHSYNC;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 842467528892..f1d0a868e80c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4906,7 +4906,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>
> intel_set_pipe_timings(intel_crtc);
>
> - if (intel_crtc->config->cpu_transcoder != TRANSCODER_EDP) {
> + if (intel_crtc->config->cpu_transcoder < TRANSCODER_EDP) {
> I915_WRITE(PIPE_MULT(intel_crtc->config->cpu_transcoder),
> intel_crtc->config->pixel_multiplier - 1);
> }
> @@ -4957,7 +4957,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> dev_priv->display.initial_watermarks(pipe_config);
> else
> intel_update_watermarks(crtc);
> - intel_enable_pipe(intel_crtc);
> +
> + /* XXX: Do the pipe assertions at the right place for BXT DSI. */
> + if (!intel_crtc->config->has_dsi_encoder)
> + intel_enable_pipe(intel_crtc);
>
> if (intel_crtc->config->has_pch_encoder)
> lpt_pch_enable(crtc);
> @@ -5090,7 +5093,9 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> drm_crtc_vblank_off(crtc);
> assert_vblank_disabled(crtc);
>
> - intel_disable_pipe(intel_crtc);
> + /* XXX: Do the pipe assertions at the right place for BXT DSI. */
> + if (!intel_crtc->config->has_dsi_encoder)
> + intel_disable_pipe(intel_crtc);
>
> if (intel_crtc->config->dp_encoder_is_mst)
> intel_ddi_set_vc_payload_alloc(crtc, false);
> @@ -7707,7 +7712,8 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
> enum pipe pipe = intel_crtc->pipe;
> enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>
> - intel_set_trans_timings(intel_crtc);
> + if (!IS_DSI_TRANSCODER(cpu_transcoder))
> + intel_set_trans_timings(intel_crtc);
I think we should put all the DSI checks to the top level
.crtc_enable/disable hooks. Will need to do the function splitting a bit
differently perhaps, but I've already commented on those. The main
benefit would be a more immediate view on the modeset sequence without
having to go through all the functions that get called. Also I would
expect it would then be easier to start shifting stuff into the encoder
hooks if we choose to go that route.
>
> /* Workaround: when the EDP input selection is B, the VTOTAL_B must be
> * programmed with the VTOTAL_EDP value. Same for VTOTAL_C. This is
> @@ -7765,9 +7771,11 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> uint32_t tmp;
>
> - intel_get_trans_timings(crtc, pipe_config);
> + if (!IS_DSI_TRANSCODER(cpu_transcoder))
> + intel_get_trans_timings(crtc, pipe_config);
>
> tmp = I915_READ(PIPESRC(crtc->pipe));
> pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> @@ -8776,9 +8784,11 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> enum pipe pipe = intel_crtc->pipe;
> + enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> u32 val;
>
> - haswell_set_transconf(crtc);
> + if (!IS_DSI_TRANSCODER(cpu_transcoder))
> + haswell_set_transconf(crtc);
>
> I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
> POSTING_READ(GAMMA_MODE(intel_crtc->pipe));
> @@ -9936,6 +9946,47 @@ static bool hsw_get_trans_config(struct intel_crtc *crtc,
> return tmp & PIPECONF_ENABLE;
> }
>
> +static bool bxt_get_dsi_trans_config(struct intel_crtc *crtc,
> + struct intel_crtc_state *pipe_config,
> + unsigned long *power_domain_mask)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + enum intel_display_power_domain power_domain;
> + enum port port;
> + enum transcoder cpu_transcoder;
> + u32 tmp;
> +
> + pipe_config->has_dsi_encoder = false;
> +
> + for_each_port_masked(port, BIT(PORT_A) | BIT(PORT_C)) {
> + if (port == PORT_A)
> + cpu_transcoder = TRANSCODER_DSI_A;
> + else
> + cpu_transcoder = TRANSCODER_DSI_C;
> +
> + power_domain = POWER_DOMAIN_TRANSCODER(cpu_transcoder);
> + if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> + continue;
> + *power_domain_mask |= BIT(power_domain);
> +
> + /* XXX: this works for video mode only */
> + tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
> + if (!(tmp & DPI_ENABLE))
> + continue;
> +
> + tmp = I915_READ(MIPI_CTRL(port));
> + if ((tmp & BXT_PIPE_SELECT_MASK) != BXT_PIPE_SELECT(crtc->pipe))
> + continue;
> +
> + pipe_config->cpu_transcoder = cpu_transcoder;
> + pipe_config->has_dsi_encoder = true;
> + break;
> + }
> +
> + return pipe_config->has_dsi_encoder;
> +}
> +
> static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config)
> {
> @@ -9997,10 +10048,18 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>
> active = hsw_get_trans_config(crtc, pipe_config, &power_domain_mask);
>
> + if (IS_BROXTON(dev_priv)) {
> + bxt_get_dsi_trans_config(crtc, pipe_config, &power_domain_mask);
> + WARN_ON(active && pipe_config->has_dsi_encoder);
> + if (pipe_config->has_dsi_encoder)
> + active = true;
> + }
> +
> if (!active)
> goto out;
>
> - haswell_get_ddi_port_state(crtc, pipe_config);
> + if (!pipe_config->has_dsi_encoder)
> + haswell_get_ddi_port_state(crtc, pipe_config);
>
> intel_get_pipe_timings(crtc, pipe_config);
>
> @@ -10026,7 +10085,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
> (I915_READ(IPS_CTL) & IPS_ENABLE);
>
> - if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
> + if (pipe_config->cpu_transcoder < TRANSCODER_EDP) {
> pipe_config->pixel_multiplier =
> I915_READ(PIPE_MULT(pipe_config->cpu_transcoder)) + 1;
> } else {
> @@ -15516,10 +15575,18 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - i915_reg_t reg = PIPECONF(crtc->config->cpu_transcoder);
> + enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
>
> - /* Clear any frame start delays used for debugging left by the BIOS */
> - I915_WRITE(reg, I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
> + if (!IS_DSI_TRANSCODER(cpu_transcoder)) {
> + i915_reg_t reg = PIPECONF(cpu_transcoder);
> +
> + /*
> + * Clear any frame start delays used for debugging left by the
> + * BIOS.
> + */
> + I915_WRITE(reg,
> + I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
> + }
>
> /* restore vblank interrupts to correct state */
> drm_crtc_vblank_reset(&crtc->base);
> @@ -16195,6 +16262,7 @@ intel_display_capture_error_state(struct drm_device *dev)
> error->pipe[i].stat = I915_READ(PIPESTAT(i));
> }
>
> + /* Note: this does not include DSI transcoders. */
> error->num_transcoders = INTEL_INFO(dev)->num_pipes;
> if (HAS_DDI(dev_priv->dev))
> error->num_transcoders++; /* Account for eDP. */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 02b3d22862a1..6e8c1386129e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -436,7 +436,8 @@ struct intel_crtc_state {
> bool has_infoframe;
>
> /* CPU Transcoder for the pipe. Currently this can only differ from the
> - * pipe on Haswell (where we have a special eDP transcoder). */
> + * pipe on Haswell and later (where we have a special eDP transcoder)
> + * and Broxton (where we have special DSI transcoders). */
> enum transcoder cpu_transcoder;
>
> /*
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 73c15210fdb1..47fd0296a05a 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -268,6 +268,7 @@ static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)
> static bool intel_dsi_compute_config(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config)
> {
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> struct intel_dsi *intel_dsi = container_of(encoder, struct intel_dsi,
> base);
> struct intel_connector *intel_connector = intel_dsi->attached_connector;
> @@ -284,6 +285,14 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
> /* DSI uses short packets for sync events, so clear mode flags for DSI */
> adjusted_mode->flags = 0;
>
> + if (IS_BROXTON(dev_priv)) {
> + /* Dual link goes to DSI transcoder A. */
> + if (intel_dsi->ports == BIT(PORT_C))
> + pipe_config->cpu_transcoder = TRANSCODER_DSI_C;
> + else
> + pipe_config->cpu_transcoder = TRANSCODER_DSI_A;
> + }
> +
> return true;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 2e88a5e06884..d189a0012277 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -89,6 +89,10 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> return "TRANSCODER_C";
> case POWER_DOMAIN_TRANSCODER_EDP:
> return "TRANSCODER_EDP";
> + case POWER_DOMAIN_TRANSCODER_DSI_A:
> + return "TRANSCODER_DSI_A";
> + case POWER_DOMAIN_TRANSCODER_DSI_C:
> + return "TRANSCODER_DSI_C";
> case POWER_DOMAIN_PORT_DDI_A_LANES:
> return "PORT_DDI_A_LANES";
> case POWER_DOMAIN_PORT_DDI_B_LANES:
> @@ -419,6 +423,8 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS | \
> BIT(POWER_DOMAIN_PIPE_A) | \
> BIT(POWER_DOMAIN_TRANSCODER_EDP) | \
> + BIT(POWER_DOMAIN_TRANSCODER_DSI_A) | \
> + BIT(POWER_DOMAIN_TRANSCODER_DSI_C) | \
> BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) | \
> BIT(POWER_DOMAIN_PORT_DDI_A_LANES) | \
> BIT(POWER_DOMAIN_PORT_DSI) | \
> --
> 2.1.4
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
