Re: [PATCH 1/9] drm/i915: Check pixel clock when setting mode for DP

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Jul 03, 2015 at 02:35:49PM +0300, Mika Kahola wrote:
> It is possible the we request to have a mode that has
> higher pixel clock than our HW can support. This patch
> checks if requested pixel clock is lower than the one
> supported by the HW. The requested mode is discarded
> if we cannot support the requested pixel clock.
> 
> This patch applies to DisplayPort.
> 
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fcc64e5..2e55dff 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -197,6 +197,26 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
>  	return (max_link_clock * max_lanes * 8) / 10;
>  }
>  
> +static int
> +intel_dp_max_pixclk(struct intel_dp *intel_dp)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_encoder *encoder = &intel_dig_port->base;
> +	struct drm_device *dev = encoder->base.dev;
> +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +	if (IS_CHERRYVIEW(dev))
> +		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);

Maybe we should precompute this stuff and store as max_dotclock into dev_priv?
It has really nothing to do with DP.

> +	else if (IS_VALLEYVIEW(dev))
> +		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 90);
> +	else if (IS_BROADWELL(dev) && intel_crtc->config->ips_enabled)

You can't look at the current crtc config at this point. Here we just
need to consider the max we can support under any conditions and filter
the modes based on that. That does mean that under some circumstances
not all of the validated modes will actually work, but there's really
nothing sensible we can do about that.

In the specific case of IPS, we can always choose to disable it at
modeset time if the mode would otherwise exceed the limit. So IPS is
never a good reason for rejecting a mode.

> +		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
> +	else
> +		return dev_priv->max_cdclk_freq;

90% is the correct limit for all older platforms.

Exceptions are CHV which has the 95% limit, and starting from HSW+
we should be able to handle 100%.

For gen2/3 we also have the option of using double wide mode which means
the limit there should be 2 * cdclk * 9 / 10.

> +}
> +
>  static enum drm_mode_status
>  intel_dp_mode_valid(struct drm_connector *connector,
>  		    struct drm_display_mode *mode)
> @@ -206,6 +226,7 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
>  	int target_clock = mode->clock;
>  	int max_rate, mode_rate, max_lanes, max_link_clock;
> +	int max_pixclk;
>  
>  	if (is_edp(intel_dp) && fixed_mode) {
>  		if (mode->hdisplay > fixed_mode->hdisplay)
> @@ -223,7 +244,9 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
>  	mode_rate = intel_dp_link_required(target_clock, 18);
>  
> -	if (mode_rate > max_rate)
> +	max_pixclk = intel_dp_max_pixclk(intel_dp);
> +
> +	if ((mode_rate > max_rate) || (target_clock > max_pixclk))
>  		return MODE_CLOCK_HIGH;
>  
>  	if (mode->clock < 10000)
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx