Re: [Intel-gfx] [PATCH v2 06/20] drm/i915: Allow fuzzy matching in pipe_config_compare.
Op 08-07-15 om 11:09 schreef Daniel Vetter:
On Wed, Jul 08, 2015 at 10:38:33AM +0200, Maarten Lankhorst wrote:
Op 07-07-15 om 12:11 schreef Daniel Vetter:
On Tue, Jul 07, 2015 at 09:08:17AM +0200, Maarten Lankhorst wrote:
@@ -12392,27 +12379,124 @@ static bool intel_fuzzy_clock_check(int clock1,
int clock2)
base.head) \
if (mask (1 (intel_crtc)-pipe))
+
+static bool
+intel_compare_m_n(unsigned int m, unsigned int n,
+unsigned int m2, unsigned int n2)
+{
+ if (m == m2 n == n2)
+ return true;
+
+ if (!m || !n || !m2 || !n2)
+ return false;
+
+ if (m m2) {
+ while (m m2) {
I think we need to make sure we don't reduce precision by eating low bits,
i.e.
while (m m2 !(m 1) !(n 1)) {
Could be useful, lets hope everyone rounds in the same way. :)
+ m = 1;
+ n = 1;
+ }
+ } else if (m m2) {
+ while (m m2) {
+ m2 = 1;
+ n2 = 1;
+ }
+ }
+
+ return m == m2 n == n2;
+}
+
+static bool
+intel_compare_link_m_n(const struct intel_link_m_n *m_n,
+ const struct intel_link_m_n *m2_n2)
I think these need to take adjust as an agurment and check for exact match
(not just matching ratio), like before.
I just implemented it, then remembered why I didn't.
It will cause a failure in intel_modeset_check_state.
That shouldn't happen. I guess the bug is that adjust doesn't live up to
it's name and doesn't copy the current value over to the new config if
they're compatible?
It's a compare function, I didn't want to do adjustments in it and leaving it
as a multiple is harmless.
In case of gen = 8 it might even fix itself through intel_dp_set_drrs_state.
~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 06/20] drm/i915: Allow fuzzy matching in pipe_config_compare.
On Wed, Jul 08, 2015 at 11:18:27AM +0200, Maarten Lankhorst wrote:
Op 08-07-15 om 11:09 schreef Daniel Vetter:
On Wed, Jul 08, 2015 at 10:38:33AM +0200, Maarten Lankhorst wrote:
Op 07-07-15 om 12:11 schreef Daniel Vetter:
On Tue, Jul 07, 2015 at 09:08:17AM +0200, Maarten Lankhorst wrote:
@@ -12392,27 +12379,124 @@ static bool intel_fuzzy_clock_check(int
clock1, int clock2)
base.head) \
if (mask (1 (intel_crtc)-pipe))
+
+static bool
+intel_compare_m_n(unsigned int m, unsigned int n,
+ unsigned int m2, unsigned int n2)
+{
+if (m == m2 n == n2)
+return true;
+
+if (!m || !n || !m2 || !n2)
+return false;
+
+if (m m2) {
+while (m m2) {
I think we need to make sure we don't reduce precision by eating low bits,
i.e.
while (m m2 !(m 1) !(n 1)) {
Could be useful, lets hope everyone rounds in the same way. :)
+m = 1;
+n = 1;
+}
+} else if (m m2) {
+while (m m2) {
+m2 = 1;
+n2 = 1;
+}
+}
+
+return m == m2 n == n2;
+}
+
+static bool
+intel_compare_link_m_n(const struct intel_link_m_n *m_n,
+ const struct intel_link_m_n *m2_n2)
I think these need to take adjust as an agurment and check for exact match
(not just matching ratio), like before.
I just implemented it, then remembered why I didn't.
It will cause a failure in intel_modeset_check_state.
That shouldn't happen. I guess the bug is that adjust doesn't live up to
it's name and doesn't copy the current value over to the new config if
they're compatible?
It's a compare function, I didn't want to do adjustments in it and leaving
it as a multiple is harmless.
In case of gen = 8 it might even fix itself through intel_dp_set_drrs_state.
Well pass it the bool adjust like for config_compare. I think in general
(i.e. when it's not too much work, which isn't the case here) doing exact
matches for hw/sw state checks is the right approach.
-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: [Intel-gfx] [PATCH v2 06/20] drm/i915: Allow fuzzy matching in pipe_config_compare.
Op 07-07-15 om 12:11 schreef Daniel Vetter:
On Tue, Jul 07, 2015 at 09:08:17AM +0200, Maarten Lankhorst wrote:
Instead of doing ad-hoc checks we already have a way of checking
if the state is compatible or not. Use this to force a modeset.
Only during modesets, or with PIPE_CONFIG_QUIRK_INHERITED_MODE
we should check if a full modeset is really needed.
Fastboot will allow the adjust parameter to ignore some stuff
too, and it will fix up differences in state that are ignored
by the compare function.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 198
---
1 file changed, 139 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8cd3a7eb1e30..eb7c2e2819b7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12277,19 +12277,6 @@ encoder_retry:
DRM_DEBUG_KMS(plane bpp: %i, pipe bpp: %i, dithering: %i\n,
base_bpp, pipe_config-pipe_bpp, pipe_config-dither);
-/* Check if we need to force a modeset */
-if (pipe_config-has_audio !=
-to_intel_crtc_state(crtc-state)-has_audio) {
-pipe_config-base.mode_changed = true;
-ret = drm_atomic_add_affected_planes(state, crtc);
-}
-
-/*
- * Note we have an issue here with infoframes: current code
- * only updates them on the full mode set path per hw
- * requirements. So here we should be checking for any
- * required changes and forcing a mode set.
- */
fail:
return ret;
}
@@ -12392,27 +12379,124 @@ static bool intel_fuzzy_clock_check(int clock1,
int clock2)
base.head) \
if (mask (1 (intel_crtc)-pipe))
+
+static bool
+intel_compare_m_n(unsigned int m, unsigned int n,
+ unsigned int m2, unsigned int n2)
+{
+if (m == m2 n == n2)
+return true;
+
+if (!m || !n || !m2 || !n2)
+return false;
+
+if (m m2) {
+while (m m2) {
I think we need to make sure we don't reduce precision by eating low bits,
i.e.
while (m m2 !(m 1) !(n 1)) {
Could be useful, lets hope everyone rounds in the same way. :)
+m = 1;
+n = 1;
+}
+} else if (m m2) {
+while (m m2) {
+m2 = 1;
+n2 = 1;
+}
+}
+
+return m == m2 n == n2;
+}
+
+static bool
+intel_compare_link_m_n(const struct intel_link_m_n *m_n,
+ const struct intel_link_m_n *m2_n2)
I think these need to take adjust as an agurment and check for exact match
(not just matching ratio), like before.
I just implemented it, then remembered why I didn't.
It will cause a failure in intel_modeset_check_state.
+{
+if (m_n-tu == m2_n2-tu
+intel_compare_m_n(m_n-gmch_m, m_n-gmch_n,
+ m2_n2-gmch_m, m2_n2-gmch_n)
+intel_compare_m_n(m_n-link_m, m_n-link_n,
+ m2_n2-link_m, m2_n2-link_n))
+return true;
+
+return false;
+}
+
static bool
intel_pipe_config_compare(struct drm_device *dev,
struct intel_crtc_state *current_config,
- struct intel_crtc_state *pipe_config)
+ struct intel_crtc_state *pipe_config,
+ bool adjust)
{
+bool ret = true;
+
+#define INTEL_ERR_OR_DBG_KMS(fmt, ...) \
+do { \
+if (!adjust) \
+DRM_ERROR(fmt, ##__VA_ARGS__); \
+else \
+DRM_DEBUG_KMS(fmt, ##__VA_ARGS__); \
+} while (0)
+
#define PIPE_CONF_CHECK_X(name) \
if (current_config-name != pipe_config-name) { \
-DRM_ERROR(mismatch in #name \
+INTEL_ERR_OR_DBG_KMS(mismatch in #name \
(expected 0x%08x, found 0x%08x)\n, \
current_config-name, \
pipe_config-name); \
-return false; \
+ret = false; \
}
#define PIPE_CONF_CHECK_I(name) \
if (current_config-name != pipe_config-name) { \
-DRM_ERROR(mismatch in #name \
+INTEL_ERR_OR_DBG_KMS(mismatch in #name \
(expected %i, found %i)\n, \
current_config-name, \
pipe_config-name); \
-return false; \
+ret = false; \
+}
+
+#define PIPE_CONF_CHECK_M_N(name) \
+if (!intel_compare_link_m_n(current_config-name, \
+pipe_config-name)) { \
+INTEL_ERR_OR_DBG_KMS(mismatch in #name \
+ (expected tu %i gmch %i/%i link %i/%i, \
+ found tu %i, gmch %i/%i
Re: [Intel-gfx] [PATCH v2 06/20] drm/i915: Allow fuzzy matching in pipe_config_compare.
On Wed, Jul 08, 2015 at 10:38:33AM +0200, Maarten Lankhorst wrote:
Op 07-07-15 om 12:11 schreef Daniel Vetter:
On Tue, Jul 07, 2015 at 09:08:17AM +0200, Maarten Lankhorst wrote:
@@ -12392,27 +12379,124 @@ static bool intel_fuzzy_clock_check(int clock1,
int clock2)
base.head) \
if (mask (1 (intel_crtc)-pipe))
+
+static bool
+intel_compare_m_n(unsigned int m, unsigned int n,
+unsigned int m2, unsigned int n2)
+{
+ if (m == m2 n == n2)
+ return true;
+
+ if (!m || !n || !m2 || !n2)
+ return false;
+
+ if (m m2) {
+ while (m m2) {
I think we need to make sure we don't reduce precision by eating low bits,
i.e.
while (m m2 !(m 1) !(n 1)) {
Could be useful, lets hope everyone rounds in the same way. :)
+ m = 1;
+ n = 1;
+ }
+ } else if (m m2) {
+ while (m m2) {
+ m2 = 1;
+ n2 = 1;
+ }
+ }
+
+ return m == m2 n == n2;
+}
+
+static bool
+intel_compare_link_m_n(const struct intel_link_m_n *m_n,
+ const struct intel_link_m_n *m2_n2)
I think these need to take adjust as an agurment and check for exact match
(not just matching ratio), like before.
I just implemented it, then remembered why I didn't.
It will cause a failure in intel_modeset_check_state.
That shouldn't happen. I guess the bug is that adjust doesn't live up to
it's name and doesn't copy the current value over to the new config if
they're compatible?
-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: [Intel-gfx] [PATCH v2 06/20] drm/i915: Allow fuzzy matching in pipe_config_compare.
On Tue, Jul 07, 2015 at 09:08:17AM +0200, Maarten Lankhorst wrote:
Instead of doing ad-hoc checks we already have a way of checking
if the state is compatible or not. Use this to force a modeset.
Only during modesets, or with PIPE_CONFIG_QUIRK_INHERITED_MODE
we should check if a full modeset is really needed.
Fastboot will allow the adjust parameter to ignore some stuff
too, and it will fix up differences in state that are ignored
by the compare function.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 198
---
1 file changed, 139 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8cd3a7eb1e30..eb7c2e2819b7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12277,19 +12277,6 @@ encoder_retry:
DRM_DEBUG_KMS(plane bpp: %i, pipe bpp: %i, dithering: %i\n,
base_bpp, pipe_config-pipe_bpp, pipe_config-dither);
- /* Check if we need to force a modeset */
- if (pipe_config-has_audio !=
- to_intel_crtc_state(crtc-state)-has_audio) {
- pipe_config-base.mode_changed = true;
- ret = drm_atomic_add_affected_planes(state, crtc);
- }
-
- /*
- * Note we have an issue here with infoframes: current code
- * only updates them on the full mode set path per hw
- * requirements. So here we should be checking for any
- * required changes and forcing a mode set.
- */
fail:
return ret;
}
@@ -12392,27 +12379,124 @@ static bool intel_fuzzy_clock_check(int clock1,
int clock2)
base.head) \
if (mask (1 (intel_crtc)-pipe))
+
+static bool
+intel_compare_m_n(unsigned int m, unsigned int n,
+ unsigned int m2, unsigned int n2)
+{
+ if (m == m2 n == n2)
+ return true;
+
+ if (!m || !n || !m2 || !n2)
+ return false;
+
+ if (m m2) {
+ while (m m2) {
I think we need to make sure we don't reduce precision by eating low bits,
i.e.
while (m m2 !(m 1) !(n 1)) {
+ m = 1;
+ n = 1;
+ }
+ } else if (m m2) {
+ while (m m2) {
+ m2 = 1;
+ n2 = 1;
+ }
+ }
+
+ return m == m2 n == n2;
+}
+
+static bool
+intel_compare_link_m_n(const struct intel_link_m_n *m_n,
+const struct intel_link_m_n *m2_n2)
I think these need to take adjust as an agurment and check for exact match
(not just matching ratio), like before.
+{
+ if (m_n-tu == m2_n2-tu
+ intel_compare_m_n(m_n-gmch_m, m_n-gmch_n,
+ m2_n2-gmch_m, m2_n2-gmch_n)
+ intel_compare_m_n(m_n-link_m, m_n-link_n,
+ m2_n2-link_m, m2_n2-link_n))
+ return true;
+
+ return false;
+}
+
static bool
intel_pipe_config_compare(struct drm_device *dev,
struct intel_crtc_state *current_config,
- struct intel_crtc_state *pipe_config)
+ struct intel_crtc_state *pipe_config,
+ bool adjust)
{
+ bool ret = true;
+
+#define INTEL_ERR_OR_DBG_KMS(fmt, ...) \
+ do { \
+ if (!adjust) \
+ DRM_ERROR(fmt, ##__VA_ARGS__); \
+ else \
+ DRM_DEBUG_KMS(fmt, ##__VA_ARGS__); \
+ } while (0)
+
#define PIPE_CONF_CHECK_X(name) \
if (current_config-name != pipe_config-name) { \
- DRM_ERROR(mismatch in #name \
+ INTEL_ERR_OR_DBG_KMS(mismatch in #name \
(expected 0x%08x, found 0x%08x)\n, \
current_config-name, \
pipe_config-name); \
- return false; \
+ ret = false; \
}
#define PIPE_CONF_CHECK_I(name) \
if (current_config-name != pipe_config-name) { \
- DRM_ERROR(mismatch in #name \
+ INTEL_ERR_OR_DBG_KMS(mismatch in #name \
(expected %i, found %i)\n, \
current_config-name, \
pipe_config-name); \
- return false; \
+ ret = false; \
+ }
+
+#define PIPE_CONF_CHECK_M_N(name) \
+ if (!intel_compare_link_m_n(current_config-name, \
+ pipe_config-name)) { \
+ INTEL_ERR_OR_DBG_KMS(mismatch in #name \
+ (expected tu %i gmch %i/%i link %i/%i, \
+ found tu %i, gmch %i/%i link %i/%i)\n, \
+ current_config-name.tu, \
+ current_config-name.gmch_m, \
+
