On Mon, Nov 30, 2015 at 09:26:42AM +0100, Daniel Vetter wrote: > On Thu, Nov 26, 2015 at 06:27:07PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > The .get_config() hooks should not reference anything in crtc->config, > > everything should be based on the passed in pipe_config instead. So > > don't dig out the cpu_transcoder from crtc->config on ddi platfforms, > > and also avoid using the encoder->crtc link and instead look up the > > pipe via pipe_config->base.crtc. > > > > I don't think this will actually fix anything since during the initial > > state readout we set up the encoder->crtc link prior to calling > > .get_config(), and during the modeset state check the encoder->crtc > > ought to be correct anyway since it's that state we just programmed. > > But this seems the right thing to do anyway. > > > > While at it, do some house cleaning on the local variables in the > > .infoframe_enabled() hooks. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> Pushed to dinq. Thanks for the review. > > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 2 +- > > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > > drivers/gpu/drm/i915/intel_hdmi.c | 47 +++++++++++++++++++-------------------- > > 3 files changed, 26 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index 76ce7c2960b6..7f618cf5289c 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -3151,7 +3151,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder, > > pipe_config->has_hdmi_sink = true; > > intel_hdmi = enc_to_intel_hdmi(&encoder->base); > > > > - if (intel_hdmi->infoframe_enabled(&encoder->base)) > > + if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config)) > > pipe_config->has_infoframe = true; > > break; > > case TRANS_DDI_MODE_SELECT_DVI: > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 6ef456427db5..b2e81403c494 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -711,7 +711,8 @@ struct intel_hdmi { > > void (*set_infoframes)(struct drm_encoder *encoder, > > bool enable, > > const struct drm_display_mode *adjusted_mode); > > - bool (*infoframe_enabled)(struct drm_encoder *encoder); > > + bool (*infoframe_enabled)(struct drm_encoder *encoder, > > + const struct intel_crtc_state *pipe_config); > > }; > > > > struct intel_dp_mst_encoder; > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > > index bdd462e7c690..c3978bad5ca0 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -169,10 +169,10 @@ static void g4x_write_infoframe(struct drm_encoder *encoder, > > POSTING_READ(VIDEO_DIP_CTL); > > } > > > > -static bool g4x_infoframe_enabled(struct drm_encoder *encoder) > > +static bool g4x_infoframe_enabled(struct drm_encoder *encoder, > > + const struct intel_crtc_state *pipe_config) > > { > > - struct drm_device *dev = encoder->dev; > > - struct drm_i915_private *dev_priv = dev->dev_private; > > + struct drm_i915_private *dev_priv = to_i915(encoder->dev); > > struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder); > > u32 val = I915_READ(VIDEO_DIP_CTL); > > > > @@ -225,13 +225,13 @@ static void ibx_write_infoframe(struct drm_encoder *encoder, > > POSTING_READ(reg); > > } > > > > -static bool ibx_infoframe_enabled(struct drm_encoder *encoder) > > +static bool ibx_infoframe_enabled(struct drm_encoder *encoder, > > + const struct intel_crtc_state *pipe_config) > > { > > - 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); > > + struct drm_i915_private *dev_priv = to_i915(encoder->dev); > > struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder); > > - i915_reg_t reg = TVIDEO_DIP_CTL(intel_crtc->pipe); > > + enum pipe pipe = to_intel_crtc(pipe_config->base.crtc)->pipe; > > + i915_reg_t reg = TVIDEO_DIP_CTL(pipe); > > u32 val = I915_READ(reg); > > > > if ((val & VIDEO_DIP_ENABLE) == 0) > > @@ -287,12 +287,12 @@ static void cpt_write_infoframe(struct drm_encoder *encoder, > > POSTING_READ(reg); > > } > > > > -static bool cpt_infoframe_enabled(struct drm_encoder *encoder) > > +static bool cpt_infoframe_enabled(struct drm_encoder *encoder, > > + const struct intel_crtc_state *pipe_config) > > { > > - 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 val = I915_READ(TVIDEO_DIP_CTL(intel_crtc->pipe)); > > + struct drm_i915_private *dev_priv = to_i915(encoder->dev); > > + enum pipe pipe = to_intel_crtc(pipe_config->base.crtc)->pipe; > > + u32 val = I915_READ(TVIDEO_DIP_CTL(pipe)); > > > > if ((val & VIDEO_DIP_ENABLE) == 0) > > return false; > > @@ -341,13 +341,13 @@ static void vlv_write_infoframe(struct drm_encoder *encoder, > > POSTING_READ(reg); > > } > > > > -static bool vlv_infoframe_enabled(struct drm_encoder *encoder) > > +static bool vlv_infoframe_enabled(struct drm_encoder *encoder, > > + const struct intel_crtc_state *pipe_config) > > { > > - 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); > > + struct drm_i915_private *dev_priv = to_i915(encoder->dev); > > struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder); > > - u32 val = I915_READ(VLV_TVIDEO_DIP_CTL(intel_crtc->pipe)); > > + enum pipe pipe = to_intel_crtc(pipe_config->base.crtc)->pipe; > > + u32 val = I915_READ(VLV_TVIDEO_DIP_CTL(pipe)); > > > > if ((val & VIDEO_DIP_ENABLE) == 0) > > return false; > > @@ -398,12 +398,11 @@ static void hsw_write_infoframe(struct drm_encoder *encoder, > > POSTING_READ(ctl_reg); > > } > > > > -static bool hsw_infoframe_enabled(struct drm_encoder *encoder) > > +static bool hsw_infoframe_enabled(struct drm_encoder *encoder, > > + const struct intel_crtc_state *pipe_config) > > { > > - 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 val = I915_READ(HSW_TVIDEO_DIP_CTL(intel_crtc->config->cpu_transcoder)); > > + struct drm_i915_private *dev_priv = to_i915(encoder->dev); > > + u32 val = I915_READ(HSW_TVIDEO_DIP_CTL(pipe_config->cpu_transcoder)); > > > > return val & (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW | > > VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW | > > @@ -927,7 +926,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder, > > if (tmp & HDMI_MODE_SELECT_HDMI) > > pipe_config->has_hdmi_sink = true; > > > > - if (intel_hdmi->infoframe_enabled(&encoder->base)) > > + if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config)) > > pipe_config->has_infoframe = true; > > > > if (tmp & SDVO_AUDIO_ENABLE) > > -- > > 2.4.10 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx