[Intel-gfx] [PATCH] drm/i915: Don't use crtc->config when reading out infoframe state

Ville Syrjälä ville.syrjala at linux.intel.com
Tue Dec 1 06:09:49 PST 2015


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 at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > 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 at linux.intel.com>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC


More information about the Intel-gfx mailing list