Re: [PATCH 09/11] drm/i915: Fix port_clock and adjusted_mode.clock readout all over

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Sep 06, 2013 at 11:29:06PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> Now that adjusted_mode.clock no longer contains the pixel_multiplier, we
> can kill the get_clock() callback and instead do the clock readout
> in get_pipe_config().
> 
> Also i9xx_crtc_clock_get() can now extract the frequency of the PCH
> DPLL, so use it to populate port_clock accurately for PCH encoders.
> For DP in port A the encoder is still responsible for filling in
> port_clock. The FDI adjusted_mode.clock extraction is kept in place
> for some extra sanity checking, but we no longer need to pretend it's
> also the port_clock.
> 
> In the encoder get_config() functions fill out adjusted_mode.clock
> based on port_clock and other details such as the DP M/N values,
> HDMI 12bpc and SDVO pixel_multiplier. For PCH encoders we will then
> do an extra sanity check to make sure the dotclock we derived from
> the FDI configuratiuon matches the one we derive from port_clock.
> 
> DVO doesn't exist on PCH platforms, so it doesn't need to anything
> but assign adjusted_mode.clock=port_clock. And DDI is HSW only, so
> none of the changes apply there.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 -
>  drivers/gpu/drm/i915/intel_crt.c     |  8 ++++++
>  drivers/gpu/drm/i915/intel_display.c | 47 ++++++++++++++++++++++++------------
>  drivers/gpu/drm/i915/intel_dp.c      | 11 ++++++++-
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_dvo.c     |  2 ++
>  drivers/gpu/drm/i915/intel_hdmi.c    | 11 +++++++++
>  drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++++++
>  8 files changed, 72 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 769c138..09fc308 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -369,7 +369,6 @@ struct drm_i915_display_funcs {
>  	 * fills out the pipe-config with the hw state. */
>  	bool (*get_pipe_config)(struct intel_crtc *,
>  				struct intel_crtc_config *);
> -	void (*get_clock)(struct intel_crtc *, struct intel_crtc_config *);
>  	int (*crtc_mode_set)(struct drm_crtc *crtc,
>  			     int x, int y,
>  			     struct drm_framebuffer *old_fb);
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index ea9022e..be99cf2 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -89,6 +89,7 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>  	struct intel_crt *crt = intel_encoder_to_crt(encoder);
>  	u32 tmp, flags = 0;
> +	int dotclock;
>  
>  	tmp = I915_READ(crt->adpa_reg);
>  
> @@ -103,6 +104,13 @@ static void intel_crt_get_config(struct intel_encoder *encoder,
>  		flags |= DRM_MODE_FLAG_NVSYNC;
>  
>  	pipe_config->adjusted_mode.flags |= flags;
> +
> +	dotclock = pipe_config->port_clock;
> +
> +	if (HAS_PCH_SPLIT(dev_priv->dev))
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
>  }
>  
>  /* Note: The caller is required to filter out dpms modes not supported by the
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d89ea94..26f0269 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5047,6 +5047,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  						     DPLL_PORTB_READY_MASK);
>  	}
>  
> +	i9xx_crtc_clock_get(crtc, pipe_config);
> +
>  	return true;
>  }
>  
> @@ -6007,6 +6009,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>  		pipe_config->pixel_multiplier =
>  			((tmp & PLL_REF_SDVO_HDMI_MULTIPLIER_MASK)
>  			 >> PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT) + 1;
> +
> +		ironlake_crtc_clock_get(crtc, pipe_config);
>  	} else {
>  		pipe_config->pixel_multiplier = 1;
>  	}
> @@ -7408,7 +7412,12 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>  		}
>  	}
>  
> -	pipe_config->adjusted_mode.clock = clock.dot;
> +	/*
> +	 * This value includes pixel_multiplier. We will use
> +	 * port_clock to compute adjusted_mode.clock in the
> +	 * encoder's get_config() function.
> +	 */
> +	pipe_config->port_clock = clock.dot;
>  }
>  
>  int intel_dotclock_calculate(int link_freq,
> @@ -7436,6 +7445,9 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
>  	struct drm_device *dev = crtc->base.dev;
>  	int link_freq;
>  
> +	/* read out port_clock from the DPLL */
> +	i9xx_crtc_clock_get(crtc, pipe_config);
> +
>  	/*
>  	 * We need to get the FDI or DP link clock here to derive
>  	 * the M/N dividers.
> @@ -7446,6 +7458,12 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
>  	 */
>  	link_freq = intel_fdi_link_freq(dev) * 10000;
>  
> +	/*
> +	 * This value does not include pixel_multiplier.
> +	 * We will check that port_clock and adjusted_mode.clock
> +	 * agree once we know their relationship in the encoder's
> +	 * get_config() function.
> +	 */
>  	pipe_config->adjusted_mode.clock =
>  		intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n);
>  }
> @@ -8859,9 +8877,6 @@ check_crtc_state(struct drm_device *dev)
>  				encoder->get_config(encoder, &pipe_config);
>  		}
>  
> -		if (dev_priv->display.get_clock)
> -			dev_priv->display.get_clock(crtc, &pipe_config);
> -
>  		WARN(crtc->active != active,
>  		     "crtc active state doesn't match with hw state "
>  		     "(expected %i, found %i)\n", crtc->active, active);
> @@ -8936,6 +8951,18 @@ intel_modeset_check_state(struct drm_device *dev)
>  	check_shared_dpll_state(dev);
>  }
>  
> +void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
> +				     int dotclock)
> +{
> +	/*
> +	 * FDI already provided one idea for the dotclock.
> +	 * Yell if the encoder disagrees.
> +	 */
> +	WARN(!intel_fuzzy_clock_check(pipe_config->adjusted_mode.clock, dotclock),
> +	     "FDI dotclock and encoder dotclock mismatch, fdi: %i, encoder: %i\n",
> +	     pipe_config->adjusted_mode.clock, dotclock);
> +}
> +
>  static int __intel_set_mode(struct drm_crtc *crtc,
>  			    struct drm_display_mode *mode,
>  			    int x, int y, struct drm_framebuffer *fb)
> @@ -9887,7 +9914,6 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.update_plane = ironlake_update_plane;
>  	} else if (HAS_PCH_SPLIT(dev)) {
>  		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
> -		dev_priv->display.get_clock = ironlake_crtc_clock_get;
>  		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
>  		dev_priv->display.crtc_enable = ironlake_crtc_enable;
>  		dev_priv->display.crtc_disable = ironlake_crtc_disable;
> @@ -9895,7 +9921,6 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.update_plane = ironlake_update_plane;
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> -		dev_priv->display.get_clock = i9xx_crtc_clock_get;
>  		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
>  		dev_priv->display.crtc_enable = valleyview_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -9903,7 +9928,6 @@ static void intel_init_display(struct drm_device *dev)
>  		dev_priv->display.update_plane = i9xx_update_plane;
>  	} else {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> -		dev_priv->display.get_clock = i9xx_crtc_clock_get;
>  		dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
>  		dev_priv->display.crtc_enable = i9xx_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -10517,15 +10541,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  			      pipe);
>  	}
>  
> -	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> -			    base.head) {
> -		if (!crtc->active)
> -			continue;
> -		if (dev_priv->display.get_clock)
> -			dev_priv->display.get_clock(crtc,
> -						    &crtc->config);
> -	}
> -
>  	list_for_each_entry(connector, &dev->mode_config.connector_list,
>  			    base.head) {
>  		if (connector->get_hw_state(connector)) {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index be0449b..509a9e4 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1372,6 +1372,7 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> +	int dotclock;
>  
>  	if ((port == PORT_A) || !HAS_PCH_CPT(dev)) {
>  		tmp = I915_READ(intel_dp->output_reg);
> @@ -1403,12 +1404,20 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  
>  	intel_dp_get_m_n(crtc, pipe_config);
>  
> -	if (dp_to_dig_port(intel_dp)->port == PORT_A) {
> +	if (port == PORT_A) {
>  		if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
>  			pipe_config->port_clock = 162000;
>  		else
>  			pipe_config->port_clock = 270000;
>  	}
> +
> +	dotclock = intel_dotclock_calculate(pipe_config->port_clock,
> +					    &pipe_config->dp_m_n);
> +
> +	if (HAS_PCH_SPLIT(dev_priv->dev) && port != PORT_A)
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
>  }
>  
>  static bool is_edp_psr(struct intel_dp *intel_dp)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4cf8898..6e1204f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -805,5 +805,7 @@ extern void intel_dp_get_m_n(struct intel_crtc *crtc,
>  			     struct intel_crtc_config *pipe_config);
>  extern int intel_dotclock_calculate(int link_freq,
>  				    const struct intel_link_m_n *m_n);
> +extern void ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
> +					    int dotclock);
>  
>  #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 406303b..92487ce 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -153,6 +153,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
>  		flags |= DRM_MODE_FLAG_NVSYNC;
>  
>  	pipe_config->adjusted_mode.flags |= flags;
> +
> +	pipe_config->adjusted_mode.clock = pipe_config->port_clock;
>  }
>  
>  static void intel_disable_dvo(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4148cc8..56e6191 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -713,6 +713,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>  	u32 tmp, flags = 0;
> +	int dotclock;
>  
>  	tmp = I915_READ(intel_hdmi->hdmi_reg);
>  
> @@ -727,6 +728,16 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>  		flags |= DRM_MODE_FLAG_NVSYNC;
>  
>  	pipe_config->adjusted_mode.flags |= flags;
> +
> +	if (pipe_config->pipe_bpp == 12*3)

I think you need to check for the 12BPC bit in the hdmi/sdvo reg here, you
can't rely on the pipe bpc value. At least on fdi configs we might end up
with sending a differnt pipe bpp (mayb just 10bpc) accross the wire than
what the port will output (if we aim for the 12bpc mode).
-Daniel

> +		dotclock = pipe_config->port_clock * 2 / 3;
> +	else
> +		dotclock = pipe_config->port_clock;
> +
> +	if (HAS_PCH_SPLIT(dev_priv->dev))
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
>  }
>  
>  static void intel_enable_hdmi(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 74042c6..7148fdd 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1319,6 +1319,7 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
>  	struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
>  	struct intel_sdvo_dtd dtd;
>  	int encoder_pixel_multiplier = 0;
> +	int dotclock;
>  	u32 flags = 0, sdvox;
>  	u8 val;
>  	bool ret;
> @@ -1357,6 +1358,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
>  			 >> SDVO_PORT_MULTIPLY_SHIFT) + 1;
>  	}
>  
> +	dotclock = pipe_config->port_clock / pipe_config->pixel_multiplier;
> +
> +	if (HAS_PCH_SPLIT(dev))
> +		ironlake_check_encoder_dotclock(pipe_config, dotclock);
> +
> +	pipe_config->adjusted_mode.clock = dotclock;
> +
>  	/* Cross check the port pixel multiplier with the sdvo encoder state. */
>  	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_CLOCK_RATE_MULT,
>  				 &val, 1)) {
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux