[PATCH] drm/i915: Reject modeset if the dotclock is too high

public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed

* [PATCH] drm/i915: Reject modeset if the dotclock is too high
@ 2016-05-24 18:34 ville.syrjala
  2016-05-24 18:34 ` [PATCH i-g-t] tests: Add kms_invalid_dotclock ville.syrjala
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: ville.syrjala @ 2016-05-24 18:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reject the modeset if the requested dotclock exceeds the maximum allowed
by the hardware. So far we've only checked this on gen2/3 while also
handling the double wide vs. single wide pipe selection. Extend the
check to all platforms since we have the max dotclock correctly
populated now across the board.

Testcase: igt/kms_invalid_dotclock
Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1e5138497e6a..adb489508f25 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6510,10 +6510,10 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
+	int clock_limit = dev_priv->max_dotclk_freq;
 
-	/* FIXME should check pixel clock limits on all platforms */
 	if (INTEL_INFO(dev)->gen < 4) {
-		int clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
+		clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
 
 		/*
 		 * Enable double wide mode when the dot clock
@@ -6521,16 +6521,16 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		 */
 		if (intel_crtc_supports_double_wide(crtc) &&
 		    adjusted_mode->crtc_clock > clock_limit) {
-			clock_limit *= 2;
+			clock_limit = dev_priv->max_dotclk_freq;
 			pipe_config->double_wide = true;
 		}
+	}
 
-		if (adjusted_mode->crtc_clock > clock_limit) {
-			DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
-				      adjusted_mode->crtc_clock, clock_limit,
-				      yesno(pipe_config->double_wide));
-			return -EINVAL;
-		}
+	if (adjusted_mode->crtc_clock > clock_limit) {
+		DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
+			      adjusted_mode->crtc_clock, clock_limit,
+			      yesno(pipe_config->double_wide));
+		return -EINVAL;
 	}
 
 	/*
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH i-g-t] tests: Add kms_invalid_dotclock
  2016-05-24 18:34 [PATCH] drm/i915: Reject modeset if the dotclock is too high ville.syrjala
@ 2016-05-24 18:34 ` ville.syrjala
  2016-05-25  5:22 ` ✗ Ro.CI.BAT: warning for drm/i915: Reject modeset if the dotclock is too high Patchwork
  2016-05-25  8:15 ` [PATCH] " Jani Nikula
  2 siblings, 0 replies; 7+ messages in thread
From: ville.syrjala @ 2016-05-24 18:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a test that makes sure every modeset gets rejected by the kernel if
the requested dotclock is beyond the hardware capabilities.

For now we just test the preferred mode for each connector, should
perhaps test them all to be more sure everything is getting rejected.

We also skip the test on connectros that have a fixed mode as the kernel
will ignore most of the user timings. We should make the kernel more
strict I think, to at least check that the user gets roughly the refresh
rate they requeted.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/Makefile.sources       |   1 +
 tests/kms_invalid_dotclock.c | 148 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 149 insertions(+)
 create mode 100644 tests/kms_invalid_dotclock.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c81eeeb5dbad..122b528f8dc8 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -93,6 +93,7 @@ TESTS_progs_M = \
 	kms_flip_event_leak \
 	kms_flip_tiling \
 	kms_frontbuffer_tracking \
+	kms_invalid_dotclock \
 	kms_legacy_colorkey \
 	kms_mmio_vs_cs_flip \
 	kms_pipe_b_c_ivb \
diff --git a/tests/kms_invalid_dotclock.c b/tests/kms_invalid_dotclock.c
new file mode 100644
index 000000000000..43f768fd26ea
--- /dev/null
+++ b/tests/kms_invalid_dotclock.c
@@ -0,0 +1,148 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "igt.h"
+#include <stdbool.h>
+
+IGT_TEST_DESCRIPTION("Make sure all modesets are rejected when the requested dotclock is too high");
+
+typedef struct {
+	int drm_fd;
+	igt_display_t display;
+	igt_output_t *output;
+	drmModeResPtr res;
+	int max_dotclock;
+} data_t;
+
+static bool has_scaling_mode_prop(data_t *data)
+{
+	return kmstest_get_property(data->drm_fd,
+				    data->output->id,
+				    DRM_MODE_OBJECT_CONNECTOR,
+				    "scaling mode",
+				    NULL, NULL, NULL);
+}
+
+static int
+test_output(data_t *data)
+{
+	igt_output_t *output = data->output;
+	drmModeModeInfo mode;
+	struct igt_fb fb;
+	int i;
+
+	/*
+	 * FIXME When we have a fixed mode, the kernel will ignore
+	 * the user timings apart from hdisplay/vdisplay. Should
+	 * fix the kernel to at least make sure the requested
+	 * refresh rate as specified by the user timings will
+	 * roughly match the user will get. For now skip the
+	 * test on  any connector with a fixed mode.
+	 */
+	if (has_scaling_mode_prop(data))
+		return 0;
+
+	/*
+	 * FIXME test every mode we have to be more
+	 * sure everything is really getting rejected?
+	 */
+	mode = *igt_output_get_mode(output);
+	mode.clock = data->max_dotclock + 1;
+
+	igt_create_fb(data->drm_fd,
+		      mode.hdisplay, mode.vdisplay,
+		      DRM_FORMAT_XRGB8888,
+		      LOCAL_DRM_FORMAT_MOD_NONE,
+		      &fb);
+
+	for (i = 0; i < data->res->count_crtcs; i++) {
+		int ret;
+
+		igt_info("Checking pipe %c connector %s with mode %s\n",
+			 'A'+i, output->name, mode.name);
+
+		ret = drmModeSetCrtc(data->drm_fd, data->res->crtcs[i],
+				     fb.fb_id, 0, 0,
+				     &output->id, 1, &mode);
+		igt_assert_lt(ret, 0);
+	}
+
+	igt_remove_fb(data->drm_fd, &fb);
+
+	return 1;
+}
+
+static void test(data_t *data)
+{
+	int valid_connectors = 0;
+
+	for_each_connected_output(&data->display, data->output) {
+		valid_connectors += test_output(data);
+	}
+
+	igt_require_f(valid_connectors, "No suitable connectors found\n");
+}
+
+static int i915_max_dotclock(void)
+{
+	char buf[4096];
+	char *s;
+	int max_dotclock = 0;
+
+	igt_debugfs_read("i915_frequency_info", buf);
+	s = strstr(buf, "Max pixel clock frequency:");
+	igt_assert(s);
+	igt_assert_eq(sscanf(s, "Max pixel clock frequency: %d kHz", &max_dotclock), 1);
+
+	/* 100 Mhz to 5 GHz seem like reasonable values to expect */
+	igt_assert_lt(max_dotclock, 5000000);
+	igt_assert_lt(100000, max_dotclock);
+
+	return max_dotclock;
+}
+
+static data_t data;
+
+igt_simple_main
+{
+	igt_skip_on_simulation();
+
+	data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+	igt_require_intel(data.drm_fd);
+
+	igt_enable_connectors();
+	kmstest_set_vt_graphics_mode();
+	igt_display_init(&data.display, data.drm_fd);
+	data.res = drmModeGetResources(data.drm_fd);
+	kmstest_unset_all_crtcs(data.drm_fd, data.res);
+
+	data.max_dotclock = i915_max_dotclock();
+	igt_info("Max dotclock: %d kHz\n", data.max_dotclock);
+
+	test(&data);
+
+	igt_display_fini(&data.display);
+	igt_reset_connectors();
+	drmModeFreeResources(data.res);
+}
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* ✗ Ro.CI.BAT: warning for drm/i915: Reject modeset if the dotclock is too high
  2016-05-24 18:34 [PATCH] drm/i915: Reject modeset if the dotclock is too high ville.syrjala
  2016-05-24 18:34 ` [PATCH i-g-t] tests: Add kms_invalid_dotclock ville.syrjala
@ 2016-05-25  5:22 ` Patchwork
  2016-05-27 11:53   ` Ville Syrjälä
  2016-05-25  8:15 ` [PATCH] " Jani Nikula
  2 siblings, 1 reply; 7+ messages in thread
From: Patchwork @ 2016-05-25  5:22 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Reject modeset if the dotclock is too high
URL   : https://patchwork.freedesktop.org/series/7653/
State : warning

== Summary ==

Series 7653v1 drm/i915: Reject modeset if the dotclock is too high
http://patchwork.freedesktop.org/api/1.0/series/7653/revisions/1/mbox

Test drv_module_reload_basic:
                skip       -> PASS       (ro-ivb-i7-3770)
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                fail       -> PASS       (fi-byt-n2820)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> SKIP       (fi-skl-i5-6260u)
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)

fi-bdw-i7-5557u  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:2   skip:38 
fi-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-i5-6260u  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:209  pass:172  dwarn:0   dfail:0   fail:0   skip:37 
ro-bdw-i7-5557U  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
ro-bdw-i7-5600u  total:209  pass:180  dwarn:0   dfail:0   fail:1   skip:28 
ro-bsw-n3050     total:209  pass:167  dwarn:0   dfail:0   fail:2   skip:40 
ro-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:209  pass:146  dwarn:0   dfail:0   fail:1   skip:62 
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:209  pass:177  dwarn:0   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:209  pass:181  dwarn:0   dfail:0   fail:0   skip:28 
ro-skl-i7-6700hq total:204  pass:181  dwarn:2   dfail:0   fail:0   skip:21 
ro-snb-i7-2620M  total:209  pass:170  dwarn:0   dfail:0   fail:1   skip:38 
fi-bsw-n3050 failed to connect after reboot
fi-hsw-i7-4770k failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_998/

3a7ef25 drm-intel-nightly: 2016y-05m-24d-12h-43m-51s UTC integration manifest
c2f3c3d drm/i915: Reject modeset if the dotclock is too high

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: ✗ Ro.CI.BAT: warning for drm/i915: Reject modeset if the dotclock is too high
  2016-05-25  5:22 ` ✗ Ro.CI.BAT: warning for drm/i915: Reject modeset if the dotclock is too high Patchwork
@ 2016-05-27 11:53   ` Ville Syrjälä
  0 siblings, 0 replies; 7+ messages in thread
From: Ville Syrjälä @ 2016-05-27 11:53 UTC (permalink / raw)
  To: intel-gfx

On Wed, May 25, 2016 at 05:22:21AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Reject modeset if the dotclock is too high
> URL   : https://patchwork.freedesktop.org/series/7653/
> State : warning
> 
> == Summary ==
> 
> Series 7653v1 drm/i915: Reject modeset if the dotclock is too high
> http://patchwork.freedesktop.org/api/1.0/series/7653/revisions/1/mbox
> 
> Test drv_module_reload_basic:
>                 skip       -> PASS       (ro-ivb-i7-3770)
>                 pass       -> DMESG-WARN (ro-skl-i7-6700hq)

[  143.693314] [drm:intel_pipe_update_start [i915]] *ERROR* Potential atomic update failure on pipe A

https://bugs.freedesktop.org/show_bug.cgi?id=95632

> Test gem_exec_flush:
>         Subgroup basic-batch-kernel-default-cmd:
>                 fail       -> PASS       (fi-byt-n2820)
> Test kms_flip:
>         Subgroup basic-flip-vs-wf_vblank:
>                 pass       -> SKIP       (fi-skl-i5-6260u)

(kms_flip:9621) igt-kms-WARNING: connector 37 has no modes
(kms_flip:9621) igt-kms-WARNING: connector 37 has no modes
(kms_flip:9621) igt-kms-WARNING: connector 37 has no modes

[  426.641160] [drm:intel_get_hpd_pins] hotplug event received, stat 0x00200000, dig 0x10101012, pins 0x00000020
[  426.641169] [drm:intel_hpd_irq_storm_detect] Received HPD interrupt on PIN 5 - cnt: 0
[  426.641243] [drm:i915_hotplug_work_func] running encoder hotplug functions
[  426.641259] [drm:i915_hotplug_work_func] Connector HDMI-A-1 (pin 5) received hotplug event.
[  426.641268] [drm:intel_hdmi_detect] [CONNECTOR:37:HDMI-A-1]
[  426.730833] [drm:intel_hdmi_detect] HDMI live status down
[  426.730845] [drm:intel_hpd_irq_event] [CONNECTOR:37:HDMI-A-1] status updated from connected to disconnected
[  426.730935] [drm:drm_fb_helper_hotplug_event] 
[  426.730938] [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:37:HDMI-A-1]
[  426.730941] [drm:intel_hdmi_detect] [CONNECTOR:37:HDMI-A-1]
[  426.746265] kms_flip: executing
[  426.746392] [drm:i915_gem_open] 
[  426.746583] [drm:i915_gem_open] 
[  426.746747] [drm:i915_gem_open] 
[  426.818966] [drm:intel_hdmi_detect] HDMI live status down
[  426.818976] [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:37:HDMI-A-1] disconnected
...
[  426.942081] [drm:i915_hotplug_work_func] Connector HDMI-A-1 (pin 5) received hotplug event.
[  426.942088] [drm:intel_hdmi_detect] [CONNECTOR:37:HDMI-A-1]
[  426.942099] [drm:intel_power_well_enable] enabling always-on
[  426.971071] [drm:do_gmbus_xfer] GMBUS [i915 gmbus dpb] NAK for addr: 0040 w(1)
[  426.971076] [drm:do_gmbus_xfer] GMBUS [i915 gmbus dpb] NAK on first message, retry
[  426.971702] [drm:do_gmbus_xfer] GMBUS [i915 gmbus dpb] NAK for addr: 0040 w(1)
[  426.971712] [drm:drm_detect_monitor_audio] Monitor has basic audio support
[  426.971716] [drm:intel_power_well_disable] disabling always-on
[  426.971720] [drm:intel_hpd_irq_event] [CONNECTOR:37:HDMI-A-1] status updated from disconnected to connected
[  426.972567] kms_flip: starting subtest basic-flip-vs-wf_vblank
[  426.980243] kms_flip: exiting, ret=77
[  426.991305] [drm:drm_fb_helper_hotplug_event] 
[  426.991309] [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:37:HDMI-A-1]
[  426.991312] [drm:intel_hdmi_detect] [CONNECTOR:37:HDMI-A-1]
[  426.991315] [drm:intel_power_well_enable] enabling always-on
[  427.020106] [drm:do_gmbus_xfer] GMBUS [i915 gmbus dpb] NAK for addr: 0040 w(1)
[  427.020108] [drm:do_gmbus_xfer] GMBUS [i915 gmbus dpb] NAK on first message, retry
[  427.020731] [drm:do_gmbus_xfer] GMBUS [i915 gmbus dpb] NAK for addr: 0040 w(1)
[  427.020736] [drm:drm_detect_monitor_audio] Monitor has basic audio support
[  427.020737] [drm:intel_power_well_disable] disabling always-on
[  427.021039] [drm:drm_edid_to_eld] ELD monitor G246HYL
[  427.021042] [drm:parse_hdmi_vsdb] HDMI: DVI dual 0, max TMDS clock 0, latency present 0 0, video latency 0 0, audio latency 0 0
[  427.021043] [drm:drm_edid_to_eld] ELD size 32, SAD count 1
[  427.021100] [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:37:HDMI-A-1] probed modes :
[  427.021103] [drm:drm_mode_debug_printmodeline] Modeline 48:"1920x1080" 60 148500 1920 2008 2052 2200 1080 1084 1089 1125 0x48 0x5
[  427.021105] [drm:drm_mode_debug_printmodeline] Modeline 73:"1920x1080" 60 148352 1920 2008 2052 2200 1080 1084 1089 1125 0x40 0x5

So looks like some kind of spurious hpd issue once again.

> Test kms_setmode:
>         Subgroup basic-clone-single-crtc:
>                 dmesg-warn -> PASS       (ro-skl-i7-6700hq)
> 
> fi-bdw-i7-5557u  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
> fi-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:2   skip:38 
> fi-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
> fi-skl-i5-6260u  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
> fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0   skip:25 
> fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
> ro-bdw-i5-5250u  total:209  pass:172  dwarn:0   dfail:0   fail:0   skip:37 
> ro-bdw-i7-5557U  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
> ro-bdw-i7-5600u  total:209  pass:180  dwarn:0   dfail:0   fail:1   skip:28 
> ro-bsw-n3050     total:209  pass:167  dwarn:0   dfail:0   fail:2   skip:40 
> ro-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:3   skip:37 
> ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
> ro-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
> ro-ilk-i7-620lm  total:209  pass:146  dwarn:0   dfail:0   fail:1   skip:62 
> ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
> ro-ivb-i7-3770   total:209  pass:177  dwarn:0   dfail:0   fail:0   skip:32 
> ro-ivb2-i7-3770  total:209  pass:181  dwarn:0   dfail:0   fail:0   skip:28 
> ro-skl-i7-6700hq total:204  pass:181  dwarn:2   dfail:0   fail:0   skip:21 
> ro-snb-i7-2620M  total:209  pass:170  dwarn:0   dfail:0   fail:1   skip:38 
> fi-bsw-n3050 failed to connect after reboot
> fi-hsw-i7-4770k failed to connect after reboot
> 
> Results at /archive/results/CI_IGT_test/RO_Patchwork_998/
> 
> 3a7ef25 drm-intel-nightly: 2016y-05m-24d-12h-43m-51s UTC integration manifest
> c2f3c3d drm/i915: Reject modeset if the dotclock is too high

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Reject modeset if the dotclock is too high
  2016-05-24 18:34 [PATCH] drm/i915: Reject modeset if the dotclock is too high ville.syrjala
  2016-05-24 18:34 ` [PATCH i-g-t] tests: Add kms_invalid_dotclock ville.syrjala
  2016-05-25  5:22 ` ✗ Ro.CI.BAT: warning for drm/i915: Reject modeset if the dotclock is too high Patchwork
@ 2016-05-25  8:15 ` Jani Nikula
  2016-05-27 11:54   ` Ville Syrjälä
  2016-05-27 12:35   ` Ville Syrjälä
  2 siblings, 2 replies; 7+ messages in thread
From: Jani Nikula @ 2016-05-25  8:15 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Tue, 24 May 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Reject the modeset if the requested dotclock exceeds the maximum allowed
> by the hardware. So far we've only checked this on gen2/3 while also
> handling the double wide vs. single wide pipe selection. Extend the
> check to all platforms since we have the max dotclock correctly
> populated now across the board.
>
> Testcase: igt/kms_invalid_dotclock
> Cc: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Seems good. Maybe we have bugs open about this?

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1e5138497e6a..adb489508f25 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6510,10 +6510,10 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> +	int clock_limit = dev_priv->max_dotclk_freq;
>  
> -	/* FIXME should check pixel clock limits on all platforms */
>  	if (INTEL_INFO(dev)->gen < 4) {
> -		int clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
> +		clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
>  
>  		/*
>  		 * Enable double wide mode when the dot clock
> @@ -6521,16 +6521,16 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  		 */
>  		if (intel_crtc_supports_double_wide(crtc) &&
>  		    adjusted_mode->crtc_clock > clock_limit) {
> -			clock_limit *= 2;
> +			clock_limit = dev_priv->max_dotclk_freq;
>  			pipe_config->double_wide = true;
>  		}
> +	}
>  
> -		if (adjusted_mode->crtc_clock > clock_limit) {
> -			DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
> -				      adjusted_mode->crtc_clock, clock_limit,
> -				      yesno(pipe_config->double_wide));
> -			return -EINVAL;
> -		}
> +	if (adjusted_mode->crtc_clock > clock_limit) {
> +		DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
> +			      adjusted_mode->crtc_clock, clock_limit,
> +			      yesno(pipe_config->double_wide));
> +		return -EINVAL;
>  	}
>  
>  	/*

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Reject modeset if the dotclock is too high
  2016-05-25  8:15 ` [PATCH] " Jani Nikula
@ 2016-05-27 11:54   ` Ville Syrjälä
  2016-05-27 12:35   ` Ville Syrjälä
  1 sibling, 0 replies; 7+ messages in thread
From: Ville Syrjälä @ 2016-05-27 11:54 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, May 25, 2016 at 11:15:35AM +0300, Jani Nikula wrote:
> On Tue, 24 May 2016, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Reject the modeset if the requested dotclock exceeds the maximum allowed
> > by the hardware. So far we've only checked this on gen2/3 while also
> > handling the double wide vs. single wide pipe selection. Extend the
> > check to all platforms since we have the max dotclock correctly
> > populated now across the board.
> >
> > Testcase: igt/kms_invalid_dotclock
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Seems good. Maybe we have bugs open about this?

I hope all of those got sorted by the SKL cdclk programming patches. But
I should go through the list anyways...

> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 1e5138497e6a..adb489508f25 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6510,10 +6510,10 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> >  	struct drm_device *dev = crtc->base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> > +	int clock_limit = dev_priv->max_dotclk_freq;
> >  
> > -	/* FIXME should check pixel clock limits on all platforms */
> >  	if (INTEL_INFO(dev)->gen < 4) {
> > -		int clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
> > +		clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
> >  
> >  		/*
> >  		 * Enable double wide mode when the dot clock
> > @@ -6521,16 +6521,16 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> >  		 */
> >  		if (intel_crtc_supports_double_wide(crtc) &&
> >  		    adjusted_mode->crtc_clock > clock_limit) {
> > -			clock_limit *= 2;
> > +			clock_limit = dev_priv->max_dotclk_freq;
> >  			pipe_config->double_wide = true;
> >  		}
> > +	}
> >  
> > -		if (adjusted_mode->crtc_clock > clock_limit) {
> > -			DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
> > -				      adjusted_mode->crtc_clock, clock_limit,
> > -				      yesno(pipe_config->double_wide));
> > -			return -EINVAL;
> > -		}
> > +	if (adjusted_mode->crtc_clock > clock_limit) {
> > +		DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
> > +			      adjusted_mode->crtc_clock, clock_limit,
> > +			      yesno(pipe_config->double_wide));
> > +		return -EINVAL;
> >  	}
> >  
> >  	/*
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Reject modeset if the dotclock is too high
  2016-05-25  8:15 ` [PATCH] " Jani Nikula
  2016-05-27 11:54   ` Ville Syrjälä
@ 2016-05-27 12:35   ` Ville Syrjälä
  1 sibling, 0 replies; 7+ messages in thread
From: Ville Syrjälä @ 2016-05-27 12:35 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, May 25, 2016 at 11:15:35AM +0300, Jani Nikula wrote:
> On Tue, 24 May 2016, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Reject the modeset if the requested dotclock exceeds the maximum allowed
> > by the hardware. So far we've only checked this on gen2/3 while also
> > handling the double wide vs. single wide pipe selection. Extend the
> > check to all platforms since we have the max dotclock correctly
> > populated now across the board.
> >
> > Testcase: igt/kms_invalid_dotclock
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Seems good. Maybe we have bugs open about this?
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Pushed to dinq. Thanks for the review.

> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 1e5138497e6a..adb489508f25 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6510,10 +6510,10 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> >  	struct drm_device *dev = crtc->base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> > +	int clock_limit = dev_priv->max_dotclk_freq;
> >  
> > -	/* FIXME should check pixel clock limits on all platforms */
> >  	if (INTEL_INFO(dev)->gen < 4) {
> > -		int clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
> > +		clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
> >  
> >  		/*
> >  		 * Enable double wide mode when the dot clock
> > @@ -6521,16 +6521,16 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> >  		 */
> >  		if (intel_crtc_supports_double_wide(crtc) &&
> >  		    adjusted_mode->crtc_clock > clock_limit) {
> > -			clock_limit *= 2;
> > +			clock_limit = dev_priv->max_dotclk_freq;
> >  			pipe_config->double_wide = true;
> >  		}
> > +	}
> >  
> > -		if (adjusted_mode->crtc_clock > clock_limit) {
> > -			DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
> > -				      adjusted_mode->crtc_clock, clock_limit,
> > -				      yesno(pipe_config->double_wide));
> > -			return -EINVAL;
> > -		}
> > +	if (adjusted_mode->crtc_clock > clock_limit) {
> > +		DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
> > +			      adjusted_mode->crtc_clock, clock_limit,
> > +			      yesno(pipe_config->double_wide));
> > +		return -EINVAL;
> >  	}
> >  
> >  	/*
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-05-27 12:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-24 18:34 [PATCH] drm/i915: Reject modeset if the dotclock is too high ville.syrjala
2016-05-24 18:34 ` [PATCH i-g-t] tests: Add kms_invalid_dotclock ville.syrjala
2016-05-25  5:22 ` ✗ Ro.CI.BAT: warning for drm/i915: Reject modeset if the dotclock is too high Patchwork
2016-05-27 11:53   ` Ville Syrjälä
2016-05-25  8:15 ` [PATCH] " Jani Nikula
2016-05-27 11:54   ` Ville Syrjälä
2016-05-27 12:35   ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox