On Wed, Apr 17, 2013 at 03:09:50PM -0300, Paulo Zanoni wrote:
> 2013/4/17 Daniel Vetter <daniel.vetter at ffwll.ch>:
> > For a bunch of reason we need to more accurately track this:
> > - hw pipe state readout for Haswell needs the cpu transcoder.
> > - We need to know the right cpu transcoder in a bunch of places in
> > ->disable and other modeset callbacks.
> >
> > In the future we need to add hw state readout&check support, too. But
> > to avoid ugly merge conflicts do the rote sed job now without any
> > functional changes.
> >
> > v2: Preserve the cpu_transcoder value when overwriting crtc->config.
> > Reported by Paulo.
>
> No more WARNs here. Just booted, checked dmesg and checked if there
> are pixels on the screen.
>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
Let's try this again ... thanks for the review.
-Daniel
>
> >
> > Cc: Paulo Zanoni <przanoni at gmail.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > ---
> > drivers/gpu/drm/i915/intel_ddi.c | 8 ++++----
> > drivers/gpu/drm/i915/intel_display.c | 37 +++++++++++++++++++---------------
> > drivers/gpu/drm/i915/intel_drv.h | 6 +++++-
> > drivers/gpu/drm/i915/intel_hdmi.c | 6 +++---
> > 4 files changed, 33 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 22524cb..26a0a57 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -924,7 +924,7 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
> > struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
> > - enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> > + enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
> > int type = intel_encoder->type;
> > uint32_t temp;
> >
> > @@ -958,7 +958,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
> > struct drm_encoder *encoder = &intel_encoder->base;
> > struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> > enum pipe pipe = intel_crtc->pipe;
> > - enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> > + enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
> > enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > int type = intel_encoder->type;
> > uint32_t temp;
> > @@ -1223,7 +1223,7 @@ void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> > struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> > struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
> > enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > - enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> > + enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
> >
> > if (cpu_transcoder != TRANSCODER_EDP)
> > I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
> > @@ -1233,7 +1233,7 @@ void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> > void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc)
> > {
> > struct drm_i915_private *dev_priv = intel_crtc->base.dev->dev_private;
> > - enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> > + enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
> >
> > if (cpu_transcoder != TRANSCODER_EDP)
> > I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index dce643c..948a2c3 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -892,7 +892,7 @@ enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
> > struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >
> > - return intel_crtc->cpu_transcoder;
> > + return intel_crtc->config.cpu_transcoder;
> > }
> >
> > static void ironlake_wait_for_vblank(struct drm_device *dev, int pipe)
> > @@ -3208,7 +3208,7 @@ static void lpt_pch_enable(struct drm_crtc *crtc)
> > struct drm_device *dev = crtc->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > - enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> > + enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
> >
> > assert_transcoder_disabled(dev_priv, TRANSCODER_A);
> >
> > @@ -3583,7 +3583,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> > struct intel_encoder *encoder;
> > int pipe = intel_crtc->pipe;
> > int plane = intel_crtc->plane;
> > - enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> > + enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
> >
> > if (!intel_crtc->active)
> > return;
> > @@ -3643,7 +3643,7 @@ static void haswell_crtc_off(struct drm_crtc *crtc)
> >
> > /* Stop saying we're using TRANSCODER_EDP because some other CRTC might
> > * start using it. */
> > - intel_crtc->cpu_transcoder = (enum transcoder) intel_crtc->pipe;
> > + intel_crtc->config.cpu_transcoder = (enum transcoder) intel_crtc->pipe;
> >
> > intel_ddi_put_crtc_pll(crtc);
> > }
> > @@ -4499,7 +4499,7 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
> > struct drm_device *dev = intel_crtc->base.dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > enum pipe pipe = intel_crtc->pipe;
> > - enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> > + enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
> > uint32_t vsyncshift;
> >
> > if (!IS_GEN2(dev) && adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> > @@ -5237,7 +5237,7 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc,
> > {
> > struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > - enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> > + enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
> > uint32_t val;
> >
> > val = I915_READ(PIPECONF(cpu_transcoder));
> > @@ -5431,7 +5431,7 @@ void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
> > struct drm_device *dev = crtc->base.dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > int pipe = crtc->pipe;
> > - enum transcoder transcoder = crtc->cpu_transcoder;
> > + enum transcoder transcoder = crtc->config.cpu_transcoder;
> >
> > if (INTEL_INFO(dev)->gen >= 5) {
> > I915_WRITE(PIPE_DATA_M1(transcoder), TU_SIZE(m_n->tu) | m_n->gmch_m);
> > @@ -5614,7 +5614,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> > WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)),
> > "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev));
> >
> > - intel_crtc->cpu_transcoder = pipe;
> > + intel_crtc->config.cpu_transcoder = pipe;
> >
> > ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
> > &has_reduced_clock, &reduced_clock);
> > @@ -5796,9 +5796,9 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> > }
> >
> > if (is_cpu_edp)
> > - intel_crtc->cpu_transcoder = TRANSCODER_EDP;
> > + intel_crtc->config.cpu_transcoder = TRANSCODER_EDP;
> > else
> > - intel_crtc->cpu_transcoder = pipe;
> > + intel_crtc->config.cpu_transcoder = pipe;
> >
> > /* We are not sure yet this won't happen. */
> > WARN(!HAS_PCH_LPT(dev), "Unexpected PCH type %d\n",
> > @@ -5807,7 +5807,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> > WARN(num_connectors != 1, "%d connectors attached to pipe %c\n",
> > num_connectors, pipe_name(pipe));
> >
> > - WARN_ON(I915_READ(PIPECONF(intel_crtc->cpu_transcoder)) &
> > + WARN_ON(I915_READ(PIPECONF(intel_crtc->config.cpu_transcoder)) &
> > (PIPECONF_ENABLE | I965_PIPECONF_ACTIVE));
> >
> > WARN_ON(I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE);
> > @@ -5858,7 +5858,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > uint32_t tmp;
> >
> > - tmp = I915_READ(PIPECONF(crtc->cpu_transcoder));
> > + tmp = I915_READ(PIPECONF(crtc->config.cpu_transcoder));
> > if (!(tmp & PIPECONF_ENABLE))
> > return false;
> >
> > @@ -6826,7 +6826,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > - enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> > + enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
> > struct drm_display_mode *mode;
> > int htot = I915_READ(HTOTAL(cpu_transcoder));
> > int hsync = I915_READ(HSYNC(cpu_transcoder));
> > @@ -7989,10 +7989,12 @@ static int __intel_set_mode(struct drm_crtc *crtc,
> > * to set it here already despite that we pass it down the callchain.
> > */
> > if (modeset_pipes) {
> > + enum transcoder tmp = to_intel_crtc(crtc)->config.cpu_transcoder;
> > crtc->mode = *mode;
> > /* mode_set/enable/disable functions rely on a correct pipe
> > * config. */
> > to_intel_crtc(crtc)->config = *pipe_config;
> > + to_intel_crtc(crtc)->config.cpu_transcoder = tmp;
> > }
> >
> > /* Only after disabling all output pipelines that will be changed can we
> > @@ -8403,7 +8405,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> > /* Swap pipes & planes for FBC on pre-965 */
> > intel_crtc->pipe = pipe;
> > intel_crtc->plane = pipe;
> > - intel_crtc->cpu_transcoder = pipe;
> > + intel_crtc->config.cpu_transcoder = pipe;
> > if (IS_MOBILE(dev) && IS_GEN3(dev)) {
> > DRM_DEBUG_KMS("swapping pipes & planes for FBC\n");
> > intel_crtc->plane = !pipe;
> > @@ -9128,7 +9130,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
> > u32 reg;
> >
> > /* Clear any frame start delays used for debugging left by the BIOS */
> > - reg = PIPECONF(crtc->cpu_transcoder);
> > + reg = PIPECONF(crtc->config.cpu_transcoder);
> > I915_WRITE(reg, I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
> >
> > /* We need to sanitize the plane -> pipe mapping first because this will
> > @@ -9294,7 +9296,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> > }
> >
> > crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> > - crtc->cpu_transcoder = TRANSCODER_EDP;
> > + crtc->config.cpu_transcoder = TRANSCODER_EDP;
> >
> > DRM_DEBUG_KMS("Pipe %c using transcoder EDP\n",
> > pipe_name(pipe));
> > @@ -9304,7 +9306,10 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> > setup_pipes:
> > list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> > base.head) {
> > + enum transcoder tmp = crtc->config.cpu_transcoder;
> > memset(&crtc->config, 0, sizeof(crtc->config));
> > + crtc->config.cpu_transcoder = tmp;
> > +
> > crtc->active = dev_priv->display.get_pipe_config(crtc,
> > &crtc->config);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 7b97b9a..1362ca6 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -188,6 +188,10 @@ struct intel_crtc_config {
> > * between pch encoders and cpu encoders. */
> > bool has_pch_encoder;
> >
> > + /* CPU Transcoder for the pipe. Currently this can only differ from the
> > + * pipe on Haswell (where we have a special eDP transcoder). */
> > + enum transcoder cpu_transcoder;
> > +
> > /*
> > * Use reduced/limited/broadcast rbg range, compressing from the full
> > * range fed into the crtcs.
> > @@ -220,13 +224,13 @@ struct intel_crtc_config {
> > int pixel_target_clock;
> > /* Used by SDVO (and if we ever fix it, HDMI). */
> > unsigned pixel_multiplier;
> > +
> > };
> >
> > struct intel_crtc {
> > struct drm_crtc base;
> > enum pipe pipe;
> > enum plane plane;
> > - enum transcoder cpu_transcoder;
> > u8 lut_r[256], lut_g[256], lut_b[256];
> > /*
> > * Whether the crtc and the connected output pipeline is active. Implies
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 8912201..3e6a3ef 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -294,8 +294,8 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
> > struct drm_device *dev = encoder->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> > - u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
> > - u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->cpu_transcoder);
> > + u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
> > + u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->config.cpu_transcoder);
> > unsigned int i, len = DIP_HEADER_SIZE + frame->len;
> > u32 val = I915_READ(ctl_reg);
> >
> > @@ -570,7 +570,7 @@ static void hsw_set_infoframes(struct drm_encoder *encoder,
> > struct drm_i915_private *dev_priv = encoder->dev->dev_private;
> > struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> > struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> > - u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
> > + u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
> > u32 val = I915_READ(reg);
> >
> > assert_hdmi_port_disabled(intel_hdmi);
> > --
> > 1.7.10.4
> >
>
>
>
> --
> Paulo Zanoni
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
