On Fri, Mar 18, 2016 at 05:05:42PM +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. > > v2: addressing comments by Ville: > - rename the dsi get config function to hsw_get_dsi_transcoder_state > - rebase onto the higher level split of pipe/transcoder functions > - use more has_dsi_encoder as we can now because of the above, > with no need to look at the transcoder so much > - rename IS_DSI_TRANSCODER to transcoder_is_dsi > - use the above a bit more instead of comparing to < TRANSCODER_EDP > > Signed-off-by: Jani Nikula <[email protected]> > --- > drivers/gpu/drm/i915/i915_drv.h | 13 +++++ > drivers/gpu/drm/i915/intel_ddi.c | 6 +++ > drivers/gpu/drm/i915/intel_display.c | 91 > +++++++++++++++++++++++++++++---- > 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, 116 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index f330a53c19b9..5e700770c403 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -127,6 +127,8 @@ enum transcoder { > TRANSCODER_B, > TRANSCODER_C, > TRANSCODER_EDP, > + TRANSCODER_DSI_A, > + TRANSCODER_DSI_C, > I915_MAX_TRANSCODERS > }; > > @@ -141,11 +143,20 @@ 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>"; > } > } > > +static inline bool transcoder_is_dsi(enum transcoder transcoder) > +{ > + return transcoder == TRANSCODER_DSI_A || transcoder == 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, > @@ -196,6 +207,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..e6c3a80e1360 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(transcoder_is_dsi(cpu_transcoder)); > + > 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(transcoder_is_dsi(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 98d8b563b9a1..28ead66ed987 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4900,6 +4900,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_encoder *encoder; > int pipe = intel_crtc->pipe, hsw_workaround_pipe; > + enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder; > struct intel_crtc_state *pipe_config = > to_intel_crtc_state(crtc->state); > > @@ -4916,11 +4917,14 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) > if (intel_crtc->config->has_dp_encoder) > intel_dp_set_m_n(intel_crtc, M1_N1); > > - intel_set_pipe_timings(intel_crtc); > + if (!intel_crtc->config->has_dsi_encoder) > + intel_set_pipe_timings(intel_crtc); > + > intel_set_pipe_src_size(intel_crtc); > > - if (intel_crtc->config->cpu_transcoder != TRANSCODER_EDP) { > - I915_WRITE(PIPE_MULT(intel_crtc->config->cpu_transcoder), > + if (cpu_transcoder != TRANSCODER_EDP && > + !transcoder_is_dsi(cpu_transcoder)) { > + I915_WRITE(PIPE_MULT(cpu_transcoder), > intel_crtc->config->pixel_multiplier - 1); > } > > @@ -4929,7 +4933,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) > &intel_crtc->config->fdi_m_n, NULL); > } > > - haswell_set_pipeconf(crtc); > + if (!intel_crtc->config->has_dsi_encoder) > + haswell_set_pipeconf(crtc); > + > haswell_set_pipe_gamma(crtc); > haswell_set_pipemisc(crtc); > > @@ -4972,7 +4978,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); > @@ -5105,7 +5114,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); > @@ -9957,6 +9968,47 @@ static bool hsw_get_transcoder_state(struct intel_crtc > *crtc, > return tmp & PIPECONF_ENABLE; > } > > +static bool bxt_get_dsi_transcoder_state(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) > { > @@ -10018,12 +10070,22 @@ static bool haswell_get_pipe_config(struct > intel_crtc *crtc, > > active = hsw_get_transcoder_state(crtc, pipe_config, > &power_domain_mask); > > + if (IS_BROXTON(dev_priv)) { > + bxt_get_dsi_transcoder_state(crtc, pipe_config, > + &power_domain_mask); > + WARN_ON(active && pipe_config->has_dsi_encoder);
The warn could use a small comment perhaps, explaining that DSI and DDI transcoders shouldn't be enabled at the same time. Anyway, patch looks good to me Reviewed-by: Ville Syrjälä <[email protected]> > + 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); > + } > > - intel_get_pipe_timings(crtc, pipe_config); > intel_get_pipe_src_size(crtc, pipe_config); > > if (INTEL_INFO(dev)->gen >= 9) { > @@ -10048,7 +10110,8 @@ 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 && > + !transcoder_is_dsi(pipe_config->cpu_transcoder)) { > pipe_config->pixel_multiplier = > I915_READ(PIPE_MULT(pipe_config->cpu_transcoder)) + 1; > } else { > @@ -15520,10 +15583,15 @@ 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 (!transcoder_is_dsi(cpu_transcoder)) { > + i915_reg_t reg = PIPECONF(cpu_transcoder); > + > + I915_WRITE(reg, > + I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK); > + } > > /* restore vblank interrupts to correct state */ > drm_crtc_vblank_reset(&crtc->base); > @@ -16194,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 5136eeffc24e..ba45245ad6c8 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -437,7 +437,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 3562bf337e62..1981212ffc8d 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
