diff mbox

[TRYBOT] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.

Message ID 20170720130802.29372-1-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maarten Lankhorst July 20, 2017, 1:08 p.m. UTC
The watermarks it should calculate against are the old optimal watermarks.
The currently active crtc watermarks are pure fiction, and are invalid in
case of a nonblocking modeset, page flip enabling/disabling planes or any
other reason.

When the crtc is disabled or during a modeset the intermediate watermarks
don't need to be programmed separately, and could be directly assigned
to the optimal watermarks.

CXSR must always be disabled in the intermediate case for modesets, else
we get a WARN for vblank wait timeout.

Also rename crtc_state to new_crtc_state, to distinguish it from the old state.

Changes since v1:
- Use intel_atomic_get_old_crtc_state. (ville)
Changes since v2:
- Always unset cxsr during modeset.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101597
---
 drivers/gpu/drm/i915/intel_pm.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

Comments

kernel test robot July 22, 2017, 9:09 p.m. UTC | #1
Hi Maarten,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.13-rc1 next-20170721]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Maarten-Lankhorst/drm-i915-Calculate-vlv-chv-intermediate-watermarks-correctly-v3/20170723-044645
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-x010-201730 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_pm.c: In function 'vlv_compute_intermediate_wm':
   drivers/gpu/drm/i915/intel_pm.c:2012:3: error: implicit declaration of function 'intel_atomic_get_old_crtc_state' [-Werror=implicit-function-declaration]
      intel_atomic_get_old_crtc_state(new_crtc_state->base.state, crtc);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/intel_pm.c:2012:3: error: initialization makes pointer from integer without a cast [-Werror=int-conversion]
   cc1: all warnings being treated as errors

vim +2012 drivers/gpu/drm/i915/intel_pm.c

  2004	
  2005	static int vlv_compute_intermediate_wm(struct drm_device *dev,
  2006					       struct intel_crtc *crtc,
  2007					       struct intel_crtc_state *new_crtc_state)
  2008	{
  2009		struct vlv_wm_state *intermediate = &new_crtc_state->wm.vlv.intermediate;
  2010		const struct vlv_wm_state *optimal = &new_crtc_state->wm.vlv.optimal;
  2011		const struct intel_crtc_state *old_crtc_state =
> 2012			intel_atomic_get_old_crtc_state(new_crtc_state->base.state, crtc);
  2013		const struct vlv_wm_state *active = &old_crtc_state->wm.vlv.optimal;
  2014		int level;
  2015	
  2016		if (!new_crtc_state->base.active || drm_atomic_crtc_needs_modeset(&new_crtc_state->base)) {
  2017			*intermediate = *optimal;
  2018	
  2019			intermediate->cxsr = false;
  2020			goto out;
  2021		}
  2022	
  2023		intermediate->num_levels = min(optimal->num_levels, active->num_levels);
  2024		intermediate->cxsr = optimal->cxsr && active->cxsr &&
  2025			!new_crtc_state->disable_cxsr;
  2026	
  2027		for (level = 0; level < intermediate->num_levels; level++) {
  2028			enum plane_id plane_id;
  2029	
  2030			for_each_plane_id_on_crtc(crtc, plane_id) {
  2031				intermediate->wm[level].plane[plane_id] =
  2032					min(optimal->wm[level].plane[plane_id],
  2033					    active->wm[level].plane[plane_id]);
  2034			}
  2035	
  2036			intermediate->sr[level].plane = min(optimal->sr[level].plane,
  2037							    active->sr[level].plane);
  2038			intermediate->sr[level].cursor = min(optimal->sr[level].cursor,
  2039							     active->sr[level].cursor);
  2040		}
  2041	
  2042		vlv_invalidate_wms(crtc, intermediate, level);
  2043	
  2044	out:
  2045		/*
  2046		 * If our intermediate WM are identical to the final WM, then we can
  2047		 * omit the post-vblank programming; only update if it's different.
  2048		 */
  2049		if (memcmp(intermediate, optimal, sizeof(*intermediate)) != 0)
  2050			new_crtc_state->wm.need_postvbl_update = true;
  2051	
  2052		return 0;
  2053	}
  2054	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot July 22, 2017, 9:47 p.m. UTC | #2
Hi Maarten,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.13-rc1 next-20170721]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Maarten-Lankhorst/drm-i915-Calculate-vlv-chv-intermediate-watermarks-correctly-v3/20170723-044645
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-x013-201730 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_pm.c: In function 'vlv_compute_intermediate_wm':
>> drivers/gpu/drm/i915/intel_pm.c:2012:3: error: implicit declaration of function 'intel_atomic_get_old_crtc_state' [-Werror=implicit-function-declaration]
      intel_atomic_get_old_crtc_state(new_crtc_state->base.state, crtc);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/intel_pm.c:2012:3: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
   In file included from include/linux/err.h:4:0,
                    from include/linux/clk.h:15,
                    from include/linux/cpufreq.h:14,
                    from drivers/gpu/drm/i915/intel_pm.c:28:
   drivers/gpu/drm/i915/intel_pm.c: At top level:
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strcpy' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:390:2: note: in expansion of macro 'if'
     if (p_size == (size_t)-1 && q_size == (size_t)-1)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'kmemdup' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:380:2: note: in expansion of macro 'if'
     if (p_size < size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'kmemdup' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:378:2: note: in expansion of macro 'if'
     if (__builtin_constant_p(size) && p_size < size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr_inv' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:369:2: note: in expansion of macro 'if'
     if (p_size < size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr_inv' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:367:2: note: in expansion of macro 'if'
     if (__builtin_constant_p(size) && p_size < size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:358:2: note: in expansion of macro 'if'
     if (p_size < size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:356:2: note: in expansion of macro 'if'
     if (__builtin_constant_p(size) && p_size < size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:348:2: note: in expansion of macro 'if'
     if (p_size < size || q_size < size)
     ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:345:3: note: in expansion of macro 'if'
      if (q_size < size)
      ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
                          ^~~~~~~~~~
   include/linux/string.h:343:3: note: in expansion of macro 'if'
      if (p_size < size)
      ^~
   include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
       ______f = {     \
       ^
   include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
    #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )

vim +/intel_atomic_get_old_crtc_state +2012 drivers/gpu/drm/i915/intel_pm.c

  2004	
  2005	static int vlv_compute_intermediate_wm(struct drm_device *dev,
  2006					       struct intel_crtc *crtc,
  2007					       struct intel_crtc_state *new_crtc_state)
  2008	{
  2009		struct vlv_wm_state *intermediate = &new_crtc_state->wm.vlv.intermediate;
  2010		const struct vlv_wm_state *optimal = &new_crtc_state->wm.vlv.optimal;
  2011		const struct intel_crtc_state *old_crtc_state =
> 2012			intel_atomic_get_old_crtc_state(new_crtc_state->base.state, crtc);
  2013		const struct vlv_wm_state *active = &old_crtc_state->wm.vlv.optimal;
  2014		int level;
  2015	
  2016		if (!new_crtc_state->base.active || drm_atomic_crtc_needs_modeset(&new_crtc_state->base)) {
  2017			*intermediate = *optimal;
  2018	
  2019			intermediate->cxsr = false;
  2020			goto out;
  2021		}
  2022	
  2023		intermediate->num_levels = min(optimal->num_levels, active->num_levels);
  2024		intermediate->cxsr = optimal->cxsr && active->cxsr &&
  2025			!new_crtc_state->disable_cxsr;
  2026	
  2027		for (level = 0; level < intermediate->num_levels; level++) {
  2028			enum plane_id plane_id;
  2029	
  2030			for_each_plane_id_on_crtc(crtc, plane_id) {
  2031				intermediate->wm[level].plane[plane_id] =
  2032					min(optimal->wm[level].plane[plane_id],
  2033					    active->wm[level].plane[plane_id]);
  2034			}
  2035	
  2036			intermediate->sr[level].plane = min(optimal->sr[level].plane,
  2037							    active->sr[level].plane);
  2038			intermediate->sr[level].cursor = min(optimal->sr[level].cursor,
  2039							     active->sr[level].cursor);
  2040		}
  2041	
  2042		vlv_invalidate_wms(crtc, intermediate, level);
  2043	
  2044	out:
  2045		/*
  2046		 * If our intermediate WM are identical to the final WM, then we can
  2047		 * omit the post-vblank programming; only update if it's different.
  2048		 */
  2049		if (memcmp(intermediate, optimal, sizeof(*intermediate)) != 0)
  2050			new_crtc_state->wm.need_postvbl_update = true;
  2051	
  2052		return 0;
  2053	}
  2054	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 48785ef75d33..7b900ad90745 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2004,16 +2004,25 @@  static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 static int vlv_compute_intermediate_wm(struct drm_device *dev,
 				       struct intel_crtc *crtc,
-				       struct intel_crtc_state *crtc_state)
+				       struct intel_crtc_state *new_crtc_state)
 {
-	struct vlv_wm_state *intermediate = &crtc_state->wm.vlv.intermediate;
-	const struct vlv_wm_state *optimal = &crtc_state->wm.vlv.optimal;
-	const struct vlv_wm_state *active = &crtc->wm.active.vlv;
+	struct vlv_wm_state *intermediate = &new_crtc_state->wm.vlv.intermediate;
+	const struct vlv_wm_state *optimal = &new_crtc_state->wm.vlv.optimal;
+	const struct intel_crtc_state *old_crtc_state =
+		intel_atomic_get_old_crtc_state(new_crtc_state->base.state, crtc);
+	const struct vlv_wm_state *active = &old_crtc_state->wm.vlv.optimal;
 	int level;
 
+	if (!new_crtc_state->base.active || drm_atomic_crtc_needs_modeset(&new_crtc_state->base)) {
+		*intermediate = *optimal;
+
+		intermediate->cxsr = false;
+		goto out;
+	}
+
 	intermediate->num_levels = min(optimal->num_levels, active->num_levels);
 	intermediate->cxsr = optimal->cxsr && active->cxsr &&
-		!crtc_state->disable_cxsr;
+		!new_crtc_state->disable_cxsr;
 
 	for (level = 0; level < intermediate->num_levels; level++) {
 		enum plane_id plane_id;
@@ -2032,12 +2041,13 @@  static int vlv_compute_intermediate_wm(struct drm_device *dev,
 
 	vlv_invalidate_wms(crtc, intermediate, level);
 
+out:
 	/*
 	 * If our intermediate WM are identical to the final WM, then we can
 	 * omit the post-vblank programming; only update if it's different.
 	 */
 	if (memcmp(intermediate, optimal, sizeof(*intermediate)) != 0)
-		crtc_state->wm.need_postvbl_update = true;
+		new_crtc_state->wm.need_postvbl_update = true;
 
 	return 0;
 }