[PATCH 0/9] drm/i915: Check pixel clock when setting mode

[PATCH 0/9] drm/i915: Check pixel clock when setting mode

[Mika Kahola]

From EDID we can read and request higher pixel clock than
our HW can support. This set of patches add 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. For example for Cherryview

'cvt 2560 1600 60' gives

# 2560x1600 59.99 Hz (CVT 4.10MA) hsync: 99.46 kHz; pclk: 348.50 MHz
Modeline "2560x1600_60.00"  348.50  2560 2760 3032 3504  1600 1603 1609 1658 -hsync +vsync

where pixel clock 348.50 MHz is higher than the supported 304 MHz.

The checks are implemented for DisplayPort, HDMI, LVDS, DVO, SDVO, DSI,
CRT, TV, and DP-MST.

Mika Kahola (9):
  drm/i915: Check pixel clock when setting mode for DP
  drm/i915: Check pixel clock when setting mode for HDMI
  drm/i915: Check pixel clock when setting mode for LVDS
  drm/i915: Check pixel clock when setting mode for DVO
  drm/i915: Check pixel clock when setting mode for SDVO
  drm/i915: Check pixel clock when setting mode for DSI
  drm/i915: Check pixel clock when setting mode for CRT
  drm/i915: Check pixel clock when setting mode for TV
  drm/i915: Check pixel clock when setting mode for DP-MST

 drivers/gpu/drm/i915/intel_crt.c    | 24 +++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_dp.c     | 25 ++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_dp_mst.c | 28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dsi.c    | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dvo.c    | 21 ++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_hdmi.c   | 28 +++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_lvds.c   | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_sdvo.c   | 21 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_tv.c     | 21 +++++++++++++++++++++
 9 files changed, 206 insertions(+), 4 deletions(-)

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Mika Kahola]

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);
+	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)
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	else
+		return dev_priv->max_cdclk_freq;
+}
+
 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

[PATCH 2/9] drm/i915: Check pixel clock when setting mode for HDMI

[Mika Kahola]

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 HDMI.

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 00c4b40..1886cd4c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1163,14 +1163,40 @@ static int hdmi_portclock_limit(struct intel_hdmi *hdmi, bool respect_dvi_limit)
 		return 225000;
 }
 
+static int
+intel_hdmi_max_pixclk(struct intel_hdmi *hdmi)
+{
+	struct intel_encoder *encoder =	&hdmi_to_dig_port(hdmi)->base;
+	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+
+	if (IS_CHERRYVIEW(dev))
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	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)
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	else
+		return dev_priv->max_cdclk_freq;
+}
+
 static enum drm_mode_status
 intel_hdmi_mode_valid(struct drm_connector *connector,
 		      struct drm_display_mode *mode)
 {
+	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+
 	int clock = mode->clock;
+	int max_pixclk = intel_hdmi_max_pixclk(intel_hdmi);
 
-	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
+	if (mode->flags & DRM_MODE_FLAG_DBLCLK) {
 		clock *= 2;
+		max_pixclk *= 2;
+	}
+
+	if (clock > max_pixclk)
+		return MODE_CLOCK_HIGH;
 
 	if (clock > hdmi_portclock_limit(intel_attached_hdmi(connector),
 					 true))
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/9] drm/i915: Check pixel clock when setting mode for LVDS

[Mika Kahola]

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 LVDS.

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_lvds.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index ea85547..3280413 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -262,13 +262,31 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
 	POSTING_READ(lvds_encoder->reg);
 }
 
+static int
+intel_lvds_max_pixclk(struct drm_connector *connector)
+{
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+
+	if (IS_CHERRYVIEW(connector->dev))
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	else if (IS_VALLEYVIEW(connector->dev))
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 90);
+	else
+		return dev_priv->max_cdclk_freq;
+}
+
 static enum drm_mode_status
 intel_lvds_mode_valid(struct drm_connector *connector,
 		      struct drm_display_mode *mode)
 {
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
+	int max_pixclk;
+
+	max_pixclk = intel_lvds_max_pixclk(connector);
 
+	if (mode->clock > max_pixclk)
+		return MODE_PANEL;
 	if (mode->hdisplay > fixed_mode->hdisplay)
 		return MODE_PANEL;
 	if (mode->vdisplay > fixed_mode->vdisplay)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/9] drm/i915: Check pixel clock when setting mode for DVO

[Mika Kahola]

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 DVO.

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_dvo.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index ece5bd7..82cbcea 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -240,16 +240,35 @@ static void intel_dvo_dpms(struct drm_connector *connector, int mode)
 	intel_modeset_check_state(connector->dev);
 }
 
+static int
+intel_dvo_max_pixclk(struct intel_dvo *intel_dvo)
+{
+	struct drm_i915_private *dev_priv = intel_dvo->dev.dev_priv;
+	struct intel_crtc *intel_crtc = to_intel_crtc(intel_dvo->base.base.crtc);
+
+	if (IS_CHERRYVIEW(dev_priv))
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	else if (IS_VALLEYVIEW(dev_priv))
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 90);
+	else if (IS_BROADWELL(dev_priv) && intel_crtc->config->ips_enabled)
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	else
+		return dev_priv->max_cdclk_freq;
+}
+
 static enum drm_mode_status
 intel_dvo_mode_valid(struct drm_connector *connector,
 		     struct drm_display_mode *mode)
 {
 	struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
+	int max_pixclk = intel_dvo_max_pixclk(intel_dvo);
 
 	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return MODE_NO_DBLESCAN;
 
-	/* XXX: Validate clock range */
+	/* Validate clock range */
+	if (mode->clock > max_pixclk)
+		return MODE_PANEL;
 
 	if (intel_dvo->panel_fixed_mode) {
 		if (mode->hdisplay > intel_dvo->panel_fixed_mode->hdisplay)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/9] drm/i915: Check pixel clock when setting mode for SDVO

[Mika Kahola]

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 SDVO.

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_sdvo.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index aa2fd75..34aa657 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1553,11 +1553,32 @@ static void intel_sdvo_dpms(struct drm_connector *connector, int mode)
 	intel_modeset_check_state(connector->dev);
 }
 
+static int
+intel_sdvo_max_pixclk(struct intel_sdvo *intel_sdvo)
+{
+	struct drm_device *dev = intel_sdvo->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(intel_sdvo->base.base.crtc);
+
+	if (IS_CHERRYVIEW(dev))
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	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)
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	else
+		return dev_priv->max_cdclk_freq;
+}
+
 static enum drm_mode_status
 intel_sdvo_mode_valid(struct drm_connector *connector,
 		      struct drm_display_mode *mode)
 {
 	struct intel_sdvo *intel_sdvo = intel_attached_sdvo(connector);
+	int max_pixclk = intel_sdvo_max_pixclk(intel_sdvo);
+
+	if (mode->clock > max_pixclk)
+		return MODE_CLOCK_HIGH;
 
 	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return MODE_NO_DBLESCAN;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/9] drm/i915: Check pixel clock when setting mode for DSI

[Mika Kahola]

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 DSI.

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 98998e9..1cf35b8 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -635,15 +635,39 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
 	pipe_config->port_clock = pclk;
 }
 
+static int
+intel_dsi_max_pixclk(struct intel_dsi *intel_dsi)
+{
+	struct drm_device *dev = intel_dsi->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(intel_dsi->base.base.crtc);
+
+	if (IS_CHERRYVIEW(dev))
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	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)
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	else
+		return dev_priv->max_cdclk_freq;
+}
+
 static enum drm_mode_status
 intel_dsi_mode_valid(struct drm_connector *connector,
 		     struct drm_display_mode *mode)
 {
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
+	struct intel_encoder *intel_encoder = intel_connector->encoder;
+	struct drm_encoder *encoder = &intel_encoder->base;
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
+	int max_pixclk = intel_dsi_max_pixclk(intel_dsi);
 
 	DRM_DEBUG_KMS("\n");
 
+	if (mode->clock > max_pixclk)
+		return MODE_PANEL;
+
 	if (mode->flags & DRM_MODE_FLAG_DBLSCAN) {
 		DRM_DEBUG_KMS("MODE_NO_DBLESCAN\n");
 		return MODE_NO_DBLESCAN;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/9] drm/i915: Check pixel clock when setting mode for CRT

[Mika Kahola]

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 CRT.

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 521af2c..0980f32 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -283,13 +283,32 @@ static void intel_crt_dpms(struct drm_connector *connector, int mode)
 	intel_modeset_check_state(connector->dev);
 }
 
+static int
+intel_crt_max_pixclk(struct intel_crt *intel_crt)
+{
+	struct drm_device *dev = intel_crt->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(intel_crt->base.base.crtc);
+
+	if (IS_CHERRYVIEW(dev))
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	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)
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	else
+		return dev_priv->max_cdclk_freq;
+}
+
 static enum drm_mode_status
 intel_crt_mode_valid(struct drm_connector *connector,
 		     struct drm_display_mode *mode)
 {
 	struct drm_device *dev = connector->dev;
-
+	struct intel_crt *intel_crt = intel_attached_crt(connector);
+	int max_pixclk = intel_crt_max_pixclk(intel_crt);
 	int max_clock = 0;
+
 	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return MODE_NO_DBLESCAN;
 
@@ -303,6 +322,9 @@ intel_crt_mode_valid(struct drm_connector *connector,
 	if (mode->clock > max_clock)
 		return MODE_CLOCK_HIGH;
 
+	if (mode->clock > max_pixclk)
+		return MODE_CLOCK_HIGH;
+
 	/* The FDI receiver on LPT only supports 8bpc and only has 2 lanes. */
 	if (HAS_PCH_LPT(dev) &&
 	    (ironlake_get_lanes_required(mode->clock, 270000, 24) > 2))
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 8/9] drm/i915: Check pixel clock when setting mode for TV

[Mika Kahola]

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 TV.

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_tv.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 8b9d325..076c1c1 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -891,12 +891,33 @@ intel_tv_mode_find(struct intel_tv *intel_tv)
 	return intel_tv_mode_lookup(intel_tv->tv_format);
 }
 
+static int
+intel_tv_max_pixclk(struct intel_tv *intel_tv)
+{
+	struct drm_device *dev = intel_tv->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(intel_tv->base.base.crtc);
+
+	if (IS_CHERRYVIEW(dev))
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	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)
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	else
+		return dev_priv->max_cdclk_freq;
+}
+
 static enum drm_mode_status
 intel_tv_mode_valid(struct drm_connector *connector,
 		    struct drm_display_mode *mode)
 {
 	struct intel_tv *intel_tv = intel_attached_tv(connector);
 	const struct tv_mode *tv_mode = intel_tv_mode_find(intel_tv);
+	int max_pixclk = intel_tv_max_pixclk(intel_tv);
+
+	if (mode->clock > max_pixclk)
+		return MODE_CLOCK_HIGH;
 
 	/* Ensure TV refresh is close to desired refresh */
 	if (tv_mode && abs(tv_mode->refresh - drm_mode_vrefresh(mode) * 1000)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Mika Kahola]

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 MST.

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 6e4cc53..0fb9a3d 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -343,10 +343,38 @@ static int intel_dp_mst_get_modes(struct drm_connector *connector)
 	return intel_dp_mst_get_ddc_modes(connector);
 }
 
+static int
+intel_dp_mst_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);
+	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)
+		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
+	else
+		return dev_priv->max_cdclk_freq;
+}
+
 static enum drm_mode_status
 intel_dp_mst_mode_valid(struct drm_connector *connector,
 			struct drm_display_mode *mode)
 {
+
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct intel_dp *intel_dp = intel_connector->mst_port;
+	int max_pixclk = intel_dp_mst_max_pixclk(intel_dp);
+
+	if (mode->clock > max_pixclk)
+		return MODE_CLOCK_HIGH;
+
 	/* TODO - validate mode against available PBN for link */
 	if (mode->clock < 10000)
 		return MODE_CLOCK_LOW;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

On Fri, Jul 03, 2015 at 02:35:51PM +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 LVDS.
> 
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lvds.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index ea85547..3280413 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -262,13 +262,31 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
>  	POSTING_READ(lvds_encoder->reg);
>  }
>  
> +static int
> +intel_lvds_max_pixclk(struct drm_connector *connector)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> +
> +	if (IS_CHERRYVIEW(connector->dev))
> +		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
> +	else if (IS_VALLEYVIEW(connector->dev))
> +		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 90);
> +	else
> +		return dev_priv->max_cdclk_freq;
> +}
> +
>  static enum drm_mode_status
>  intel_lvds_mode_valid(struct drm_connector *connector,
>  		      struct drm_display_mode *mode)
>  {
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
>  	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
> +	int max_pixclk;
> +
> +	max_pixclk = intel_lvds_max_pixclk(connector);
>  
> +	if (mode->clock > max_pixclk)
> +		return MODE_PANEL;

return MODE_CLOCK_HIGH;
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

On Fri, Jul 03, 2015 at 02:35:54PM +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 DSI.
> 
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 98998e9..1cf35b8 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -635,15 +635,39 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
>  	pipe_config->port_clock = pclk;
>  }
>  
> +static int
> +intel_dsi_max_pixclk(struct intel_dsi *intel_dsi)
> +{
> +	struct drm_device *dev = intel_dsi->base.base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(intel_dsi->base.base.crtc);
> +
> +	if (IS_CHERRYVIEW(dev))
> +		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
> +	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)
> +		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
> +	else
> +		return dev_priv->max_cdclk_freq;
> +}
> +
>  static enum drm_mode_status
>  intel_dsi_mode_valid(struct drm_connector *connector,
>  		     struct drm_display_mode *mode)
>  {
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
>  	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
> +	struct intel_encoder *intel_encoder = intel_connector->encoder;
> +	struct drm_encoder *encoder = &intel_encoder->base;
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> +	int max_pixclk = intel_dsi_max_pixclk(intel_dsi);
>  
>  	DRM_DEBUG_KMS("\n");
>  
> +	if (mode->clock > max_pixclk)
> +		return MODE_PANEL;

return MODE_CLOCK_HIGH;
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/9] drm/i915: Check pixel clock when setting mode

[Chris Wilson]

On Fri, Jul 03, 2015 at 02:35:48PM +0300, Mika Kahola wrote:
> From EDID we can read and request higher pixel clock than
> our HW can support. This set of patches add 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. For example for Cherryview
> 
> 'cvt 2560 1600 60' gives
> 
> # 2560x1600 59.99 Hz (CVT 4.10MA) hsync: 99.46 kHz; pclk: 348.50 MHz
> Modeline "2560x1600_60.00"  348.50  2560 2760 3032 3504  1600 1603 1609 1658 -hsync +vsync
> 
> where pixel clock 348.50 MHz is higher than the supported 304 MHz.
> 
> The checks are implemented for DisplayPort, HDMI, LVDS, DVO, SDVO, DSI,
> CRT, TV, and DP-MST.

Why do I get the feeling that there was a lot of duplicated code?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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

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

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6717
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                                  302/302              302/302
SNB                                  312/316              312/316
IVB                                  343/343              343/343
BYT                 -3              287/287              284/287
HSW                                  380/380              380/380
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*BYT  igt@gem_partial_pwrite_pread@reads      PASS(1)      FAIL(1)
*BYT  igt@gem_partial_pwrite_pread@reads-uncached      PASS(1)      FAIL(1)
*BYT  igt@gem_tiled_partial_pwrite_pread@reads      PASS(1)      FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Sivakumar Thulasimani]

On 7/3/2015 6:27 PM, Ville Syrjälä wrote:
> 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
shouldn't we expect that only modes that were returned as valid through 
appropriate calls will be received as part of set_mode ?
i.e in case of dp through the intel_dp_mode_valid through the 
.mode_valid callback ?

-- 
regards,
Sivakumar

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Mon, Jul 06, 2015 at 12:15:25PM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 7/3/2015 6:27 PM, Ville Syrjälä wrote:
> > 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
> shouldn't we expect that only modes that were returned as valid through 
> appropriate calls will be received as part of set_mode ?
> i.e in case of dp through the intel_dp_mode_valid through the 
> .mode_valid callback ?

No, the user is free to supply a totally custom mode.

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

Re: [PATCH 0/9] drm/i915: Check pixel clock when setting mode

[Daniel Vetter]

On Fri, Jul 03, 2015 at 01:49:17PM +0100, Chris Wilson wrote:
> On Fri, Jul 03, 2015 at 02:35:48PM +0300, Mika Kahola wrote:
> > From EDID we can read and request higher pixel clock than
> > our HW can support. This set of patches add 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. For example for Cherryview
> > 
> > 'cvt 2560 1600 60' gives
> > 
> > # 2560x1600 59.99 Hz (CVT 4.10MA) hsync: 99.46 kHz; pclk: 348.50 MHz
> > Modeline "2560x1600_60.00"  348.50  2560 2760 3032 3504  1600 1603 1609 1658 -hsync +vsync
> > 
> > where pixel clock 348.50 MHz is higher than the supported 304 MHz.
> > 
> > The checks are implemented for DisplayPort, HDMI, LVDS, DVO, SDVO, DSI,
> > CRT, TV, and DP-MST.
> 
> Why do I get the feeling that there was a lot of duplicated code?

The problem on top is that this only changes the mode_valid callback as
used by the probe helpers. Which means userspace can still do an addmode
of something not supported and try to trick over the code into accepting
something it can't. That code is the stuff around compute_config.

Which means we have some unpretty duplication going on, both between the
probe and compute_config paths and across all the different encoder types.
For the later an easy solution would be to add a device-global mode_valid
function and integrate that into the probe helpers. Should be a helper
library vfunc, i.e. separate from the main display vtable.

For the duplication between probe code and modeset code we should at least
try to cross-check the results (i.e. make sure that anything the modeset
code taks is also considered valid by the probe code, the other way round
only works for single-pipe and is a bit tricky due to other constraints
like plane limits). One idea I had for at least the encoder specific
checks (e.g. hdmi dotclock limits) would be to call the compute_config
function from mode_valid with a minimal pipe_config and hope for the best.
But I think that's way too tricky code, so probably the only thing we can
do without creating really hard to read&maintain code is to cross-check
the inevitable duplication :(
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/9] drm/i915: Check pixel clock when setting mode

[Ville Syrjälä]

On Mon, Jul 06, 2015 at 05:21:48PM +0200, Daniel Vetter wrote:
> On Fri, Jul 03, 2015 at 01:49:17PM +0100, Chris Wilson wrote:
> > On Fri, Jul 03, 2015 at 02:35:48PM +0300, Mika Kahola wrote:
> > > From EDID we can read and request higher pixel clock than
> > > our HW can support. This set of patches add 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. For example for Cherryview
> > > 
> > > 'cvt 2560 1600 60' gives
> > > 
> > > # 2560x1600 59.99 Hz (CVT 4.10MA) hsync: 99.46 kHz; pclk: 348.50 MHz
> > > Modeline "2560x1600_60.00"  348.50  2560 2760 3032 3504  1600 1603 1609 1658 -hsync +vsync
> > > 
> > > where pixel clock 348.50 MHz is higher than the supported 304 MHz.
> > > 
> > > The checks are implemented for DisplayPort, HDMI, LVDS, DVO, SDVO, DSI,
> > > CRT, TV, and DP-MST.
> > 
> > Why do I get the feeling that there was a lot of duplicated code?
> 
> The problem on top is that this only changes the mode_valid callback as
> used by the probe helpers. Which means userspace can still do an addmode
> of something not supported and try to trick over the code into accepting
> something it can't. That code is the stuff around compute_config.
> 
> Which means we have some unpretty duplication going on, both between the
> probe and compute_config paths and across all the different encoder types.
> For the later an easy solution would be to add a device-global mode_valid
> function and integrate that into the probe helpers. Should be a helper
> library vfunc, i.e. separate from the main display vtable.
> 
> For the duplication between probe code and modeset code we should at least
> try to cross-check the results (i.e. make sure that anything the modeset
> code taks is also considered valid by the probe code, the other way round
> only works for single-pipe and is a bit tricky due to other constraints
> like plane limits). One idea I had for at least the encoder specific
> checks (e.g. hdmi dotclock limits) would be to call the compute_config
> function from mode_valid with a minimal pipe_config and hope for the best.
> But I think that's way too tricky code, so probably the only thing we can
> do without creating really hard to read&maintain code is to cross-check
> the inevitable duplication :(

I tried to look at sharing the checking code between .mode_valid() and
mdoeset a while back but it turned into a bit of a nightmare when I
started to think about stereo 3D. To do the checks properly for stereo
3D we'd basically need to feed the mode through
drm_mode_set_crtcinfo(CRTC_STEREO_DOUBLE) and then check the crtc_
timings instead of the normal ones. So that would mean changing all the
.mode_valid() callbacks, and when I started down that path I landed
somewhere in DVO land and couldn't even figure out what limitations
.mode_valid() functions were trying to check. At that point I gave up,
and I also suggested to Mika that he first just look at adding the
checks to the .mode_valid() callbacks and not worry too much about
stereo 3D. I think that's a good enough first step.

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

Re: [PATCH 0/9] drm/i915: Check pixel clock when setting mode

[Chris Wilson]

On Mon, Jul 06, 2015 at 05:21:48PM +0200, Daniel Vetter wrote:
> On Fri, Jul 03, 2015 at 01:49:17PM +0100, Chris Wilson wrote:
> > On Fri, Jul 03, 2015 at 02:35:48PM +0300, Mika Kahola wrote:
> > > From EDID we can read and request higher pixel clock than
> > > our HW can support. This set of patches add 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. For example for Cherryview
> > > 
> > > 'cvt 2560 1600 60' gives
> > > 
> > > # 2560x1600 59.99 Hz (CVT 4.10MA) hsync: 99.46 kHz; pclk: 348.50 MHz
> > > Modeline "2560x1600_60.00"  348.50  2560 2760 3032 3504  1600 1603 1609 1658 -hsync +vsync
> > > 
> > > where pixel clock 348.50 MHz is higher than the supported 304 MHz.
> > > 
> > > The checks are implemented for DisplayPort, HDMI, LVDS, DVO, SDVO, DSI,
> > > CRT, TV, and DP-MST.
> > 
> > Why do I get the feeling that there was a lot of duplicated code?
> 
> The problem on top is that this only changes the mode_valid callback as
> used by the probe helpers. Which means userspace can still do an addmode
> of something not supported and try to trick over the code into accepting
> something it can't. That code is the stuff around compute_config.

Heh, all I meant was that we seemed to be setting the same max clock for
each GPU several times, hardcoding those values in multiple locations looks
wrong.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/9] drm/i915: Check pixel clock when setting mode

[Daniel Vetter]

On Mon, Jul 06, 2015 at 06:28:56PM +0300, Ville Syrjälä wrote:
> On Mon, Jul 06, 2015 at 05:21:48PM +0200, Daniel Vetter wrote:
> > On Fri, Jul 03, 2015 at 01:49:17PM +0100, Chris Wilson wrote:
> > > On Fri, Jul 03, 2015 at 02:35:48PM +0300, Mika Kahola wrote:
> > > > From EDID we can read and request higher pixel clock than
> > > > our HW can support. This set of patches add 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. For example for Cherryview
> > > > 
> > > > 'cvt 2560 1600 60' gives
> > > > 
> > > > # 2560x1600 59.99 Hz (CVT 4.10MA) hsync: 99.46 kHz; pclk: 348.50 MHz
> > > > Modeline "2560x1600_60.00"  348.50  2560 2760 3032 3504  1600 1603 1609 1658 -hsync +vsync
> > > > 
> > > > where pixel clock 348.50 MHz is higher than the supported 304 MHz.
> > > > 
> > > > The checks are implemented for DisplayPort, HDMI, LVDS, DVO, SDVO, DSI,
> > > > CRT, TV, and DP-MST.
> > > 
> > > Why do I get the feeling that there was a lot of duplicated code?
> > 
> > The problem on top is that this only changes the mode_valid callback as
> > used by the probe helpers. Which means userspace can still do an addmode
> > of something not supported and try to trick over the code into accepting
> > something it can't. That code is the stuff around compute_config.
> > 
> > Which means we have some unpretty duplication going on, both between the
> > probe and compute_config paths and across all the different encoder types.
> > For the later an easy solution would be to add a device-global mode_valid
> > function and integrate that into the probe helpers. Should be a helper
> > library vfunc, i.e. separate from the main display vtable.
> > 
> > For the duplication between probe code and modeset code we should at least
> > try to cross-check the results (i.e. make sure that anything the modeset
> > code taks is also considered valid by the probe code, the other way round
> > only works for single-pipe and is a bit tricky due to other constraints
> > like plane limits). One idea I had for at least the encoder specific
> > checks (e.g. hdmi dotclock limits) would be to call the compute_config
> > function from mode_valid with a minimal pipe_config and hope for the best.
> > But I think that's way too tricky code, so probably the only thing we can
> > do without creating really hard to read&maintain code is to cross-check
> > the inevitable duplication :(
> 
> I tried to look at sharing the checking code between .mode_valid() and
> mdoeset a while back but it turned into a bit of a nightmare when I
> started to think about stereo 3D. To do the checks properly for stereo
> 3D we'd basically need to feed the mode through
> drm_mode_set_crtcinfo(CRTC_STEREO_DOUBLE) and then check the crtc_
> timings instead of the normal ones. So that would mean changing all the
> .mode_valid() callbacks, and when I started down that path I landed
> somewhere in DVO land and couldn't even figure out what limitations
> .mode_valid() functions were trying to check. At that point I gave up,
> and I also suggested to Mika that he first just look at adding the
> checks to the .mode_valid() callbacks and not worry too much about
> stereo 3D. I think that's a good enough first step.

Yeah makes sense as a first step. We should still look into how we could
at least share most of this across encoders a bit.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Mika Kahola]

On Fri, 2015-07-03 at 13:38 +0100, Chris Wilson wrote:
> On Fri, Jul 03, 2015 at 02:35:54PM +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 DSI.
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 98998e9..1cf35b8 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -635,15 +635,39 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> >  	pipe_config->port_clock = pclk;
> >  }
> >  
> > +static int
> > +intel_dsi_max_pixclk(struct intel_dsi *intel_dsi)
> > +{
> > +	struct drm_device *dev = intel_dsi->base.base.dev;
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > +	struct intel_crtc *intel_crtc = to_intel_crtc(intel_dsi->base.base.crtc);
> > +
> > +	if (IS_CHERRYVIEW(dev))
> > +		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
> > +	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)
> > +		return  DIV_ROUND_UP(dev_priv->max_cdclk_freq * 100, 95);
> > +	else
> > +		return dev_priv->max_cdclk_freq;
> > +}
> > +
> >  static enum drm_mode_status
> >  intel_dsi_mode_valid(struct drm_connector *connector,
> >  		     struct drm_display_mode *mode)
> >  {
> >  	struct intel_connector *intel_connector = to_intel_connector(connector);
> >  	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
> > +	struct intel_encoder *intel_encoder = intel_connector->encoder;
> > +	struct drm_encoder *encoder = &intel_encoder->base;
> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> > +	int max_pixclk = intel_dsi_max_pixclk(intel_dsi);
> >  
> >  	DRM_DEBUG_KMS("\n");
> >  
> > +	if (mode->clock > max_pixclk)
> > +		return MODE_PANEL;
> 
> return MODE_CLOCK_HIGH;
> -Chris
> 
Thanks Chris! I'll change this one.

Cheers,
Mika


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Mika Kahola]

On Fri, 2015-07-03 at 15:57 +0300, Ville Syrjälä wrote:
> 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.
> 

Thanks for the comments. I rephrase this patch series and store the
dotclock in dev_priv. This way, I hope, I can get rid of the repetitive
code what Chris pointed out.

Cheers,
Mika

> > +}
> > +
> >  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
> 


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx