Re: [PATCH 2/4] drm/i915: Respect DP++ adaptor TMDS clock limit

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Feb 23, 2016 at 06:46:26PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Try to detect the max TMDS clock limit for the DP++ adaptor (if any)
> and take it into account when checking the port clock.
> 
> Note that as with the sink (HDMI vs. DVI) TMDS clock limit we'll ignore
> the adaptor TMDS clock limit in the modeset path, in case users are
> already "overclocking" their TMDS links. One subtle change here is that
> we'll have to respect the adaptor TMDS clock limit when we decide whether
> to do 12bpc or 8bpc, otherwise we might end up picking 12bpc and
> accidentally driving the TMDS link out of spec even when the user chose
> a mode that fits wihting the limits at 8bpc. This means you can't
> "overclock" your DP++ dongle at 12bpc anymore, but you can continue to
> do so at 8bpc.
> 
> Note that for simplicity we'll use the I2C access method for all dual
> mode adaptors including type 2. Otherwise we'd have to start mixing
> DP AUX and HDMI together. In the future we may need to do that if we
> come across any board designs that don't hook up the DDC pins to the
> DP++ connectors. Such boards would obviously only work with type 2
> dual mode adaptors, and not type 1.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

And on this patch and patch 1:

Cc: drm-intel-fixes@lists.freedesktop.org

I'll review patch 1 too if someone pities me with the spec ;-)

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/intel_drv.h  |  3 ++
>  drivers/gpu/drm/i915/intel_hdmi.c | 65 ++++++++++++++++++++++++++++++++++-----
>  2 files changed, 60 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4852049c9ab3..944eacbfb6a9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -704,6 +704,9 @@ struct cxsr_latency {
>  struct intel_hdmi {
>  	i915_reg_t hdmi_reg;
>  	int ddc_bus;
> +	struct {
> +		int max_tmds_clock;
> +	} dp_dual_mode;
>  	bool limited_color_range;
>  	bool color_range_auto;
>  	bool has_hdmi_sink;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 80b44c054087..1e8cfdb18dc7 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -34,6 +34,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_edid.h>
> +#include <drm/drm_dp_dual_mode_helper.h>
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> @@ -1168,27 +1169,42 @@ static void pch_post_disable_hdmi(struct intel_encoder *encoder)
>  	intel_disable_hdmi(encoder);
>  }
>  
> -static int hdmi_port_clock_limit(struct intel_hdmi *hdmi, bool respect_dvi_limit)
> +static int intel_hdmi_source_max_tmds_clock(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
> -
> -	if ((respect_dvi_limit && !hdmi->has_hdmi_sink) || IS_G4X(dev))
> +	if (IS_G4X(dev_priv))
>  		return 165000;
> -	else if (IS_HASWELL(dev) || INTEL_INFO(dev)->gen >= 8)
> +	else if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
>  		return 300000;
>  	else
>  		return 225000;
>  }
>  
> +static int hdmi_port_clock_limit(struct intel_hdmi *hdmi,
> +				 bool respect_downstream_limits)
> +{
> +	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
> +	int max_tmds_clock = intel_hdmi_source_max_tmds_clock(to_i915(dev));
> +
> +	if (respect_downstream_limits) {
> +		if (hdmi->dp_dual_mode.max_tmds_clock)
> +			max_tmds_clock = min(max_tmds_clock,
> +					     hdmi->dp_dual_mode.max_tmds_clock);
> +		if (!hdmi->has_hdmi_sink)
> +			max_tmds_clock = min(max_tmds_clock, 165000);
> +	}
> +
> +	return max_tmds_clock;
> +}
> +
>  static enum drm_mode_status
>  hdmi_port_clock_valid(struct intel_hdmi *hdmi,
> -		      int clock, bool respect_dvi_limit)
> +		      int clock, bool respect_downstream_limits)
>  {
>  	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
>  
>  	if (clock < 25000)
>  		return MODE_CLOCK_LOW;
> -	if (clock > hdmi_port_clock_limit(hdmi, respect_dvi_limit))
> +	if (clock > hdmi_port_clock_limit(hdmi, respect_downstream_limits))
>  		return MODE_CLOCK_HIGH;
>  
>  	/* BXT DPLL can't generate 223-240 MHz */
> @@ -1312,7 +1328,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	 * within limits.
>  	 */
>  	if (pipe_config->pipe_bpp > 8*3 && pipe_config->has_hdmi_sink &&
> -	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc, false) == MODE_OK &&
> +	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc, true) == MODE_OK &&
>  	    hdmi_12bpc_possible(pipe_config)) {
>  		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
>  		desired_bpp = 12*3;
> @@ -1352,10 +1368,41 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
>  	intel_hdmi->has_audio = false;
>  	intel_hdmi->rgb_quant_range_selectable = false;
>  
> +	intel_hdmi->dp_dual_mode.max_tmds_clock = 0;
> +
>  	kfree(to_intel_connector(connector)->detect_edid);
>  	to_intel_connector(connector)->detect_edid = NULL;
>  }
>  
> +static void
> +intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> +	struct intel_hdmi *hdmi = intel_attached_hdmi(connector);
> +	struct i2c_adapter *adapter =
> +		intel_gmbus_get_adapter(dev_priv, hdmi->ddc_bus);
> +	enum drm_dp_dual_mode_type type = drm_dp_dual_mode_detect(adapter);
> +
> +	if (type == DRM_DP_DUAL_MODE_NONE)
> +		return;
> +
> +	/*
> +	 * Type 1 adaptors can be broken unfortunately, so let's
> +	 * not trust anything they say beyond the type identification.
> +	 */
> +	if (type == DRM_DP_DUAL_MODE_TYPE2_DVI ||
> +	    type == DRM_DP_DUAL_MODE_TYPE2_HDMI) {
> +		hdmi->dp_dual_mode.max_tmds_clock =
> +			drm_dp_dual_mode_max_tmds_clock(adapter);
> +	} else {
> +		hdmi->dp_dual_mode.max_tmds_clock = 165000;
> +	}
> +
> +	DRM_DEBUG_KMS("DP dual mode adaptor (%s) detected (max TMDS clock: %d kHz)\n",
> +		      drm_dp_get_dual_mode_type_name(type),
> +		      hdmi->dp_dual_mode.max_tmds_clock);
> +}
> +
>  static bool
>  intel_hdmi_set_edid(struct drm_connector *connector, bool force)
>  {
> @@ -1371,6 +1418,8 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force)
>  				    intel_gmbus_get_adapter(dev_priv,
>  				    intel_hdmi->ddc_bus));
>  
> +		intel_hdmi_dp_dual_mode_detect(connector);
> +
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
>  	}
>  
> -- 
> 2.4.10
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx