All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
@ 2016-07-31  8:08 Keith Packard
  2016-07-31  9:20   ` kbuild test robot
  2016-08-02 13:54   ` Daniel Vetter
  0 siblings, 2 replies; 11+ messages in thread
From: Keith Packard @ 2016-07-31  8:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: Keith Packard, dri-devel, David Airlie

When reconfiguring a plane position (as in moving the cursor), the
frame buffer for the cursor isn't changing, so don't call the prepare
or cleanup driver functions.

This avoids making cursor position updates block on all pending rendering.

v2: Track which planes have been prepared to know which to
    cleanup. Otherwise, failure paths and success paths would need
    different tests in the cleanup code as the plane state points to
    different places in the two cases.

cc: dri-devel@lists.freedesktop.org
cc: David Airlie <airlied@linux.ie>
Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 23 +++++++++++++----------
 include/drm/drm_crtc.h              |  1 +
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ddfa0d1..f7f3a51 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1246,18 +1246,20 @@ EXPORT_SYMBOL(drm_atomic_helper_commit);
  * Returns:
  * 0 on success, negative error code on failure.
  */
+
 int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 				     struct drm_atomic_state *state)
 {
-	int nplanes = dev->mode_config.num_total_plane;
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
 	int ret, i;
 
-	for (i = 0; i < nplanes; i++) {
+	for_each_plane_in_state(state, plane, plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
-		struct drm_plane *plane = state->planes[i];
-		struct drm_plane_state *plane_state = state->plane_states[i];
 
-		if (!plane)
+		plane->prepared = false;
+
+		if (plane->state->fb == plane_state->fb)
 			continue;
 
 		funcs = plane->helper_private;
@@ -1267,24 +1269,22 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 			if (ret)
 				goto fail;
 		}
+		plane->prepared = true;
 	}
 
 	return 0;
 
 fail:
-	for (i--; i >= 0; i--) {
+	for_each_plane_in_state(state, plane, plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
-		struct drm_plane *plane = state->planes[i];
-		struct drm_plane_state *plane_state = state->plane_states[i];
 
-		if (!plane)
+		if (!plane->prepared)
 			continue;
 
 		funcs = plane->helper_private;
 
 		if (funcs->cleanup_fb)
 			funcs->cleanup_fb(plane, plane_state);
-
 	}
 
 	return ret;
@@ -1527,6 +1527,9 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
 	for_each_plane_in_state(old_state, plane, plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
 
+		if (!plane->prepared)
+			continue;
+
 		funcs = plane->helper_private;
 
 		if (funcs->cleanup_fb)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d1559cd..08b2033 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1531,6 +1531,7 @@ struct drm_plane {
 	uint32_t *format_types;
 	unsigned int format_count;
 	bool format_default;
+	bool prepared;
 
 	struct drm_crtc *crtc;
 	struct drm_framebuffer *fb;
-- 
2.8.1

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

* Re: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
  2016-07-31  8:08 [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2] Keith Packard
@ 2016-07-31  9:20   ` kbuild test robot
  2016-08-02 13:54   ` Daniel Vetter
  1 sibling, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2016-07-31  9:20 UTC (permalink / raw)
  To: Keith Packard
  Cc: kbuild-all, linux-kernel, Keith Packard, dri-devel, David Airlie

[-- Attachment #1: Type: text/plain, Size: 12948 bytes --]

Hi,

[auto build test WARNING on v4.7-rc7]
[cannot apply to drm/drm-next next-20160729]
[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/Keith-Packard/drm-Don-t-prepare-or-cleanup-unchanging-frame-buffers-v2/20160731-161116
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/i915_irq.c:2722: warning: No description found for parameter 'fmt'
   include/drm/drm_crtc.h:374: warning: No description found for parameter 'mode_blob'
   include/drm/drm_crtc.h:789: warning: No description found for parameter 'name'
   include/drm/drm_crtc.h:1248: warning: No description found for parameter 'connector_id'
   include/drm/drm_crtc.h:1248: warning: No description found for parameter 'tile_blob_ptr'
   include/drm/drm_crtc.h:1287: warning: No description found for parameter 'rotation'
   include/drm/drm_crtc.h:1550: warning: No description found for parameter 'name'
   include/drm/drm_crtc.h:1550: warning: No description found for parameter 'mutex'
>> include/drm/drm_crtc.h:1550: warning: No description found for parameter 'prepared'
   include/drm/drm_crtc.h:1550: warning: No description found for parameter 'helper_private'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tile_idr'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'connector_ida'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'delayed_event'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'edid_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'dpms_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'path_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tile_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'plane_type_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'rotation_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_src_x'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_src_y'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_src_w'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_src_h'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_crtc_x'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_crtc_y'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_crtc_w'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_crtc_h'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_fb_id'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_crtc_id'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_active'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_mode_id'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'dvi_i_subconnector_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'dvi_i_select_subconnector_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_subconnector_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_select_subconnector_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_mode_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_left_margin_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_right_margin_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_top_margin_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_bottom_margin_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_brightness_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_contrast_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_flicker_reduction_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_overscan_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_saturation_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_hue_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'scaling_mode_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'aspect_ratio_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'dirty_info_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'suggested_x_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'suggested_y_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'allow_fb_modifiers'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: No description found for parameter 'nonblock'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: Excess function parameter 'nonblocking' description in 'drm_atomic_helper_commit'
   drivers/gpu/drm/drm_atomic_helper.c:2946: warning: No description found for parameter 'start'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: No description found for parameter 'nonblock'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: Excess function parameter 'nonblocking' description in 'drm_atomic_helper_commit'
   drivers/gpu/drm/drm_atomic_helper.c:2946: warning: No description found for parameter 'start'
   drivers/gpu/drm/drm_atomic_helper.c:1: warning: no structured comments found
       Was looking for 'implementing async commit'.
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: No description found for parameter 'nonblock'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: Excess function parameter 'nonblocking' description in 'drm_atomic_helper_commit'
   drivers/gpu/drm/drm_atomic_helper.c:2946: warning: No description found for parameter 'start'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: No description found for parameter 'nonblock'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: Excess function parameter 'nonblocking' description in 'drm_atomic_helper_commit'
   drivers/gpu/drm/drm_atomic_helper.c:2946: warning: No description found for parameter 'start'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'dev'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'file_priv'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'mode_cmd'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'funcs'
   drivers/gpu/drm/drm_fb_cma_helper.c:233: warning: No description found for parameter 'dev'
   drivers/gpu/drm/drm_fb_cma_helper.c:233: warning: No description found for parameter 'file_priv'
   drivers/gpu/drm/drm_fb_cma_helper.c:233: warning: No description found for parameter 'mode_cmd'
   drivers/gpu/drm/drm_fb_cma_helper.c:285: warning: No description found for parameter 'm'
   drivers/gpu/drm/drm_fb_cma_helper.c:285: warning: No description found for parameter 'arg'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'dev'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'file_priv'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'mode_cmd'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'funcs'
   drivers/gpu/drm/drm_fb_cma_helper.c:233: warning: No description found for parameter 'dev'
   drivers/gpu/drm/drm_fb_cma_helper.c:233: warning: No description found for parameter 'file_priv'
   drivers/gpu/drm/drm_fb_cma_helper.c:233: warning: No description found for parameter 'mode_cmd'
   drivers/gpu/drm/drm_fb_cma_helper.c:285: warning: No description found for parameter 'm'
   drivers/gpu/drm/drm_fb_cma_helper.c:285: warning: No description found for parameter 'arg'
   include/drm/drm_dp_helper.h:750: warning: No description found for parameter 'i2c_nack_count'
   include/drm/drm_dp_helper.h:750: warning: No description found for parameter 'i2c_defer_count'
   drivers/gpu/drm/drm_dp_mst_topology.c:2383: warning: No description found for parameter 'connector'
   include/drm/drm_dp_mst_helper.h:92: warning: No description found for parameter 'cached_edid'
   include/drm/drm_dp_mst_helper.h:92: warning: No description found for parameter 'has_audio'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'max_dpcd_transaction_bytes'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'sink_count'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'total_slots'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'avail_slots'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'total_pbn'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'qlock'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'tx_msg_downq'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'tx_down_in_progress'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'payload_lock'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'proposed_vcpis'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'payloads'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'payload_mask'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'vcpi_mask'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'tx_waitq'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'work'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'tx_work'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'destroy_connector_list'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'destroy_connector_lock'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'destroy_connector_work'
   drivers/gpu/drm/drm_dp_mst_topology.c:2383: warning: No description found for parameter 'connector'

vim +/prepared +1550 include/drm/drm_crtc.h

c0db650a Keith Packard 2016-07-31  1534  	bool prepared;
8cf5c917 Jesse Barnes  2011-11-14  1535  
8cf5c917 Jesse Barnes  2011-11-14  1536  	struct drm_crtc *crtc;
8cf5c917 Jesse Barnes  2011-11-14  1537  	struct drm_framebuffer *fb;
8cf5c917 Jesse Barnes  2011-11-14  1538  
3d30a59b Daniel Vetter 2014-07-27  1539  	struct drm_framebuffer *old_fb;
3d30a59b Daniel Vetter 2014-07-27  1540  
8cf5c917 Jesse Barnes  2011-11-14  1541  	const struct drm_plane_funcs *funcs;
4d93914a Rob Clark     2012-05-17  1542  
4d93914a Rob Clark     2012-05-17  1543  	struct drm_object_properties properties;
e27dde3e Matt Roper    2014-04-01  1544  
e27dde3e Matt Roper    2014-04-01  1545  	enum drm_plane_type type;
144ecb97 Daniel Vetter 2014-10-27  1546  
4490d4c7 Daniel Vetter 2015-12-04  1547  	const struct drm_plane_helper_funcs *helper_private;
c2fcd274 Daniel Vetter 2014-11-05  1548  
144ecb97 Daniel Vetter 2014-10-27  1549  	struct drm_plane_state *state;
8cf5c917 Jesse Barnes  2011-11-14 @1550  };
8cf5c917 Jesse Barnes  2011-11-14  1551  
8cf5c917 Jesse Barnes  2011-11-14  1552  /**
3bf0401c Daniel Vetter 2014-10-27  1553   * struct drm_bridge_funcs - drm_bridge control functions
3d3f8b1f Ajay Kumar    2015-01-20  1554   * @attach: Called during drm_bridge_attach
3b336ec4 Sean Paul     2013-08-14  1555   */
3b336ec4 Sean Paul     2013-08-14  1556  struct drm_bridge_funcs {
3d3f8b1f Ajay Kumar    2015-01-20  1557  	int (*attach)(struct drm_bridge *bridge);
da024fe5 Daniel Vetter 2015-12-04  1558  

:::::: The code at line 1550 was first introduced by commit
:::::: 8cf5c9177151537e73ff1816540e4ba24b174391 drm: add plane support v3

:::::: TO: Jesse Barnes <jbarnes@virtuousgeek.org>
:::::: CC: Dave Airlie <airlied@redhat.com>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6370 bytes --]

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

* Re: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
@ 2016-07-31  9:20   ` kbuild test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2016-07-31  9:20 UTC (permalink / raw)
  Cc: Keith Packard, kbuild-all, dri-devel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 12948 bytes --]

Hi,

[auto build test WARNING on v4.7-rc7]
[cannot apply to drm/drm-next next-20160729]
[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/Keith-Packard/drm-Don-t-prepare-or-cleanup-unchanging-frame-buffers-v2/20160731-161116
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/i915_irq.c:2722: warning: No description found for parameter 'fmt'
   include/drm/drm_crtc.h:374: warning: No description found for parameter 'mode_blob'
   include/drm/drm_crtc.h:789: warning: No description found for parameter 'name'
   include/drm/drm_crtc.h:1248: warning: No description found for parameter 'connector_id'
   include/drm/drm_crtc.h:1248: warning: No description found for parameter 'tile_blob_ptr'
   include/drm/drm_crtc.h:1287: warning: No description found for parameter 'rotation'
   include/drm/drm_crtc.h:1550: warning: No description found for parameter 'name'
   include/drm/drm_crtc.h:1550: warning: No description found for parameter 'mutex'
>> include/drm/drm_crtc.h:1550: warning: No description found for parameter 'prepared'
   include/drm/drm_crtc.h:1550: warning: No description found for parameter 'helper_private'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tile_idr'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'connector_ida'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'delayed_event'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'edid_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'dpms_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'path_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tile_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'plane_type_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'rotation_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_src_x'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_src_y'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_src_w'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_src_h'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_crtc_x'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_crtc_y'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_crtc_w'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_crtc_h'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_fb_id'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_crtc_id'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_active'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'prop_mode_id'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'dvi_i_subconnector_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'dvi_i_select_subconnector_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_subconnector_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_select_subconnector_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_mode_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_left_margin_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_right_margin_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_top_margin_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_bottom_margin_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_brightness_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_contrast_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_flicker_reduction_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_overscan_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_saturation_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'tv_hue_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'scaling_mode_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'aspect_ratio_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'dirty_info_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'suggested_x_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'suggested_y_property'
   include/drm/drm_crtc.h:2186: warning: No description found for parameter 'allow_fb_modifiers'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: No description found for parameter 'nonblock'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: Excess function parameter 'nonblocking' description in 'drm_atomic_helper_commit'
   drivers/gpu/drm/drm_atomic_helper.c:2946: warning: No description found for parameter 'start'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: No description found for parameter 'nonblock'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: Excess function parameter 'nonblocking' description in 'drm_atomic_helper_commit'
   drivers/gpu/drm/drm_atomic_helper.c:2946: warning: No description found for parameter 'start'
   drivers/gpu/drm/drm_atomic_helper.c:1: warning: no structured comments found
       Was looking for 'implementing async commit'.
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: No description found for parameter 'nonblock'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: Excess function parameter 'nonblocking' description in 'drm_atomic_helper_commit'
   drivers/gpu/drm/drm_atomic_helper.c:2946: warning: No description found for parameter 'start'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: No description found for parameter 'nonblock'
   drivers/gpu/drm/drm_atomic_helper.c:1147: warning: Excess function parameter 'nonblocking' description in 'drm_atomic_helper_commit'
   drivers/gpu/drm/drm_atomic_helper.c:2946: warning: No description found for parameter 'start'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'dev'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'file_priv'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'mode_cmd'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'funcs'
   drivers/gpu/drm/drm_fb_cma_helper.c:233: warning: No description found for parameter 'dev'
   drivers/gpu/drm/drm_fb_cma_helper.c:233: warning: No description found for parameter 'file_priv'
   drivers/gpu/drm/drm_fb_cma_helper.c:233: warning: No description found for parameter 'mode_cmd'
   drivers/gpu/drm/drm_fb_cma_helper.c:285: warning: No description found for parameter 'm'
   drivers/gpu/drm/drm_fb_cma_helper.c:285: warning: No description found for parameter 'arg'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'dev'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'file_priv'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'mode_cmd'
   drivers/gpu/drm/drm_fb_cma_helper.c:173: warning: No description found for parameter 'funcs'
   drivers/gpu/drm/drm_fb_cma_helper.c:233: warning: No description found for parameter 'dev'
   drivers/gpu/drm/drm_fb_cma_helper.c:233: warning: No description found for parameter 'file_priv'
   drivers/gpu/drm/drm_fb_cma_helper.c:233: warning: No description found for parameter 'mode_cmd'
   drivers/gpu/drm/drm_fb_cma_helper.c:285: warning: No description found for parameter 'm'
   drivers/gpu/drm/drm_fb_cma_helper.c:285: warning: No description found for parameter 'arg'
   include/drm/drm_dp_helper.h:750: warning: No description found for parameter 'i2c_nack_count'
   include/drm/drm_dp_helper.h:750: warning: No description found for parameter 'i2c_defer_count'
   drivers/gpu/drm/drm_dp_mst_topology.c:2383: warning: No description found for parameter 'connector'
   include/drm/drm_dp_mst_helper.h:92: warning: No description found for parameter 'cached_edid'
   include/drm/drm_dp_mst_helper.h:92: warning: No description found for parameter 'has_audio'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'max_dpcd_transaction_bytes'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'sink_count'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'total_slots'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'avail_slots'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'total_pbn'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'qlock'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'tx_msg_downq'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'tx_down_in_progress'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'payload_lock'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'proposed_vcpis'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'payloads'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'payload_mask'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'vcpi_mask'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'tx_waitq'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'work'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'tx_work'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'destroy_connector_list'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'destroy_connector_lock'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'destroy_connector_work'
   drivers/gpu/drm/drm_dp_mst_topology.c:2383: warning: No description found for parameter 'connector'

vim +/prepared +1550 include/drm/drm_crtc.h

c0db650a Keith Packard 2016-07-31  1534  	bool prepared;
8cf5c917 Jesse Barnes  2011-11-14  1535  
8cf5c917 Jesse Barnes  2011-11-14  1536  	struct drm_crtc *crtc;
8cf5c917 Jesse Barnes  2011-11-14  1537  	struct drm_framebuffer *fb;
8cf5c917 Jesse Barnes  2011-11-14  1538  
3d30a59b Daniel Vetter 2014-07-27  1539  	struct drm_framebuffer *old_fb;
3d30a59b Daniel Vetter 2014-07-27  1540  
8cf5c917 Jesse Barnes  2011-11-14  1541  	const struct drm_plane_funcs *funcs;
4d93914a Rob Clark     2012-05-17  1542  
4d93914a Rob Clark     2012-05-17  1543  	struct drm_object_properties properties;
e27dde3e Matt Roper    2014-04-01  1544  
e27dde3e Matt Roper    2014-04-01  1545  	enum drm_plane_type type;
144ecb97 Daniel Vetter 2014-10-27  1546  
4490d4c7 Daniel Vetter 2015-12-04  1547  	const struct drm_plane_helper_funcs *helper_private;
c2fcd274 Daniel Vetter 2014-11-05  1548  
144ecb97 Daniel Vetter 2014-10-27  1549  	struct drm_plane_state *state;
8cf5c917 Jesse Barnes  2011-11-14 @1550  };
8cf5c917 Jesse Barnes  2011-11-14  1551  
8cf5c917 Jesse Barnes  2011-11-14  1552  /**
3bf0401c Daniel Vetter 2014-10-27  1553   * struct drm_bridge_funcs - drm_bridge control functions
3d3f8b1f Ajay Kumar    2015-01-20  1554   * @attach: Called during drm_bridge_attach
3b336ec4 Sean Paul     2013-08-14  1555   */
3b336ec4 Sean Paul     2013-08-14  1556  struct drm_bridge_funcs {
3d3f8b1f Ajay Kumar    2015-01-20  1557  	int (*attach)(struct drm_bridge *bridge);
da024fe5 Daniel Vetter 2015-12-04  1558  

:::::: The code at line 1550 was first introduced by commit
:::::: 8cf5c9177151537e73ff1816540e4ba24b174391 drm: add plane support v3

:::::: TO: Jesse Barnes <jbarnes@virtuousgeek.org>
:::::: CC: Dave Airlie <airlied@redhat.com>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6370 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
  2016-07-31  8:08 [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2] Keith Packard
@ 2016-08-02 13:54   ` Daniel Vetter
  2016-08-02 13:54   ` Daniel Vetter
  1 sibling, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2016-08-02 13:54 UTC (permalink / raw)
  To: Keith Packard; +Cc: linux-kernel, dri-devel

On Sun, Jul 31, 2016 at 01:08:54AM -0700, Keith Packard wrote:
> When reconfiguring a plane position (as in moving the cursor), the
> frame buffer for the cursor isn't changing, so don't call the prepare
> or cleanup driver functions.
> 
> This avoids making cursor position updates block on all pending rendering.

Oops, I thought we've had this, but nope :(

> v2: Track which planes have been prepared to know which to
>     cleanup. Otherwise, failure paths and success paths would need
>     different tests in the cleanup code as the plane state points to
>     different places in the two cases.
> 
> cc: dri-devel@lists.freedesktop.org
> cc: David Airlie <airlied@linux.ie>
> Signed-off-by: Keith Packard <keithp@keithp.com>

Hm, I can't see v1 anywhere, but I think it'd be better. You can't store
any transient state related to the current update in struct drm_plane. In
this case the cleanup_buffers from a previous update might overlap (for
nonblocking atomic commits) with the prepare_planes for the next one.
Either we need special cleanup vs. error-path code, or some flag somewhere
in the drm_plane_state.

btw if you go with the flag, vc4 could be cleaned up to use that too. At
least I thought Eric hand-rolled this logic in there somewhere, can't find
it quickly right now.

Another bit worth considering: We could use this "has the plane switched"
instead ->legacy_cursor_update in drm_atomic_helper_wait_for_vblanks -
some drivers have iommus which get really angry when we unpin too early,
and your patch alone might be good enough.
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 23 +++++++++++++----------
>  include/drm/drm_crtc.h              |  1 +
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ddfa0d1..f7f3a51 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1246,18 +1246,20 @@ EXPORT_SYMBOL(drm_atomic_helper_commit);
>   * Returns:
>   * 0 on success, negative error code on failure.
>   */
> +
>  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>  				     struct drm_atomic_state *state)
>  {
> -	int nplanes = dev->mode_config.num_total_plane;
> +	struct drm_plane *plane;
> +	struct drm_plane_state *plane_state;
>  	int ret, i;
>  
> -	for (i = 0; i < nplanes; i++) {
> +	for_each_plane_in_state(state, plane, plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
> -		struct drm_plane *plane = state->planes[i];
> -		struct drm_plane_state *plane_state = state->plane_states[i];
>  
> -		if (!plane)
> +		plane->prepared = false;
> +
> +		if (plane->state->fb == plane_state->fb)
>  			continue;
>  
>  		funcs = plane->helper_private;
> @@ -1267,24 +1269,22 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>  			if (ret)
>  				goto fail;
>  		}
> +		plane->prepared = true;
>  	}
>  
>  	return 0;
>  
>  fail:
> -	for (i--; i >= 0; i--) {
> +	for_each_plane_in_state(state, plane, plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
> -		struct drm_plane *plane = state->planes[i];
> -		struct drm_plane_state *plane_state = state->plane_states[i];
>  
> -		if (!plane)
> +		if (!plane->prepared)
>  			continue;
>  
>  		funcs = plane->helper_private;
>  
>  		if (funcs->cleanup_fb)
>  			funcs->cleanup_fb(plane, plane_state);
> -
>  	}
>  
>  	return ret;
> @@ -1527,6 +1527,9 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
>  	for_each_plane_in_state(old_state, plane, plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
>  
> +		if (!plane->prepared)
> +			continue;
> +
>  		funcs = plane->helper_private;
>  
>  		if (funcs->cleanup_fb)
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d1559cd..08b2033 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1531,6 +1531,7 @@ struct drm_plane {
>  	uint32_t *format_types;
>  	unsigned int format_count;
>  	bool format_default;
> +	bool prepared;
>  
>  	struct drm_crtc *crtc;
>  	struct drm_framebuffer *fb;
> -- 
> 2.8.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
@ 2016-08-02 13:54   ` Daniel Vetter
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2016-08-02 13:54 UTC (permalink / raw)
  To: Keith Packard; +Cc: linux-kernel, dri-devel

On Sun, Jul 31, 2016 at 01:08:54AM -0700, Keith Packard wrote:
> When reconfiguring a plane position (as in moving the cursor), the
> frame buffer for the cursor isn't changing, so don't call the prepare
> or cleanup driver functions.
> 
> This avoids making cursor position updates block on all pending rendering.

Oops, I thought we've had this, but nope :(

> v2: Track which planes have been prepared to know which to
>     cleanup. Otherwise, failure paths and success paths would need
>     different tests in the cleanup code as the plane state points to
>     different places in the two cases.
> 
> cc: dri-devel@lists.freedesktop.org
> cc: David Airlie <airlied@linux.ie>
> Signed-off-by: Keith Packard <keithp@keithp.com>

Hm, I can't see v1 anywhere, but I think it'd be better. You can't store
any transient state related to the current update in struct drm_plane. In
this case the cleanup_buffers from a previous update might overlap (for
nonblocking atomic commits) with the prepare_planes for the next one.
Either we need special cleanup vs. error-path code, or some flag somewhere
in the drm_plane_state.

btw if you go with the flag, vc4 could be cleaned up to use that too. At
least I thought Eric hand-rolled this logic in there somewhere, can't find
it quickly right now.

Another bit worth considering: We could use this "has the plane switched"
instead ->legacy_cursor_update in drm_atomic_helper_wait_for_vblanks -
some drivers have iommus which get really angry when we unpin too early,
and your patch alone might be good enough.
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 23 +++++++++++++----------
>  include/drm/drm_crtc.h              |  1 +
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ddfa0d1..f7f3a51 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1246,18 +1246,20 @@ EXPORT_SYMBOL(drm_atomic_helper_commit);
>   * Returns:
>   * 0 on success, negative error code on failure.
>   */
> +
>  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>  				     struct drm_atomic_state *state)
>  {
> -	int nplanes = dev->mode_config.num_total_plane;
> +	struct drm_plane *plane;
> +	struct drm_plane_state *plane_state;
>  	int ret, i;
>  
> -	for (i = 0; i < nplanes; i++) {
> +	for_each_plane_in_state(state, plane, plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
> -		struct drm_plane *plane = state->planes[i];
> -		struct drm_plane_state *plane_state = state->plane_states[i];
>  
> -		if (!plane)
> +		plane->prepared = false;
> +
> +		if (plane->state->fb == plane_state->fb)
>  			continue;
>  
>  		funcs = plane->helper_private;
> @@ -1267,24 +1269,22 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>  			if (ret)
>  				goto fail;
>  		}
> +		plane->prepared = true;
>  	}
>  
>  	return 0;
>  
>  fail:
> -	for (i--; i >= 0; i--) {
> +	for_each_plane_in_state(state, plane, plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
> -		struct drm_plane *plane = state->planes[i];
> -		struct drm_plane_state *plane_state = state->plane_states[i];
>  
> -		if (!plane)
> +		if (!plane->prepared)
>  			continue;
>  
>  		funcs = plane->helper_private;
>  
>  		if (funcs->cleanup_fb)
>  			funcs->cleanup_fb(plane, plane_state);
> -
>  	}
>  
>  	return ret;
> @@ -1527,6 +1527,9 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
>  	for_each_plane_in_state(old_state, plane, plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
>  
> +		if (!plane->prepared)
> +			continue;
> +
>  		funcs = plane->helper_private;
>  
>  		if (funcs->cleanup_fb)
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d1559cd..08b2033 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1531,6 +1531,7 @@ struct drm_plane {
>  	uint32_t *format_types;
>  	unsigned int format_count;
>  	bool format_default;
> +	bool prepared;
>  
>  	struct drm_crtc *crtc;
>  	struct drm_framebuffer *fb;
> -- 
> 2.8.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

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

* Re: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
  2016-08-02 13:54   ` Daniel Vetter
@ 2016-08-07  5:26     ` Keith Packard
  -1 siblings, 0 replies; 11+ messages in thread
From: Keith Packard @ 2016-08-07  5:26 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: linux-kernel, dri-devel, airlied, keithp


[-- Attachment #1.1: Type: text/plain, Size: 680 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> Hm, I can't see v1 anywhere, but I think it'd be better. You can't store
> any transient state related to the current update in struct drm_plane. In
> this case the cleanup_buffers from a previous update might overlap (for
> nonblocking atomic commits) with the prepare_planes for the next one.
> Either we need special cleanup vs. error-path code, or some flag somewhere
> in the drm_plane_state.

Ok, here's pretty much the previous version, which works now that I've
fixed the intel driver. Instead of just comparing the fb's, I'm using
the framebuffer_changed function, which seems like a nice bit of
documentation if nothing else.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-drm-Don-t-prepare-or-cleanup-unchanging-frame-buffer.patch --]
[-- Type: text/x-diff, Size: 3024 bytes --]

From a75251d5762b1c200ed2f3dca2a5b00cc85bea95 Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Sat, 4 Jun 2016 01:16:22 -0700
Subject: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v3]

When reconfiguring a plane position (as in moving the cursor), the
frame buffer for the cursor isn't changing, so don't call the prepare
or cleanup driver functions.

This avoids making cursor position updates block on all pending rendering.

v3: use drm_atomic_helper_framebuffer_changed in both prepare and
    cleanup phases instead of keeping state in the plane.

cc: dri-devel@lists.freedesktop.org
cc: David Airlie <airlied@linux.ie>
cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ddfa0d1..72e50bc 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1246,18 +1246,19 @@ EXPORT_SYMBOL(drm_atomic_helper_commit);
  * Returns:
  * 0 on success, negative error code on failure.
  */
+
 int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 				     struct drm_atomic_state *state)
 {
-	int nplanes = dev->mode_config.num_total_plane;
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
 	int ret, i;
+	int max_prepared_i = 0;
 
-	for (i = 0; i < nplanes; i++) {
+	for_each_plane_in_state(state, plane, plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
-		struct drm_plane *plane = state->planes[i];
-		struct drm_plane_state *plane_state = state->plane_states[i];
 
-		if (!plane)
+		if (!drm_atomic_helper_framebuffer_changed(dev, state, plane_state->crtc))
 			continue;
 
 		funcs = plane->helper_private;
@@ -1267,24 +1268,25 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 			if (ret)
 				goto fail;
 		}
+		max_prepared_i = i;
 	}
 
 	return 0;
 
 fail:
-	for (i--; i >= 0; i--) {
+	for_each_plane_in_state(state, plane, plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
-		struct drm_plane *plane = state->planes[i];
-		struct drm_plane_state *plane_state = state->plane_states[i];
 
-		if (!plane)
+		if (i > max_prepared_i)
+			break;
+
+		if (!drm_atomic_helper_framebuffer_changed(dev, state, plane_state->crtc))
 			continue;
 
 		funcs = plane->helper_private;
 
 		if (funcs->cleanup_fb)
 			funcs->cleanup_fb(plane, plane_state);
-
 	}
 
 	return ret;
@@ -1527,6 +1529,9 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
 	for_each_plane_in_state(old_state, plane, plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
 
+		if (!drm_atomic_helper_framebuffer_changed(dev, old_state, plane_state->crtc))
+			continue;
+
 		funcs = plane->helper_private;
 
 		if (funcs->cleanup_fb)
-- 
2.8.1


[-- Attachment #1.3: Type: text/plain, Size: 15 bytes --]


-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 810 bytes --]

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

* Re: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
@ 2016-08-07  5:26     ` Keith Packard
  0 siblings, 0 replies; 11+ messages in thread
From: Keith Packard @ 2016-08-07  5:26 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: linux-kernel, dri-devel, airlied, keithp


[-- Attachment #1.1: Type: text/plain, Size: 680 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> Hm, I can't see v1 anywhere, but I think it'd be better. You can't store
> any transient state related to the current update in struct drm_plane. In
> this case the cleanup_buffers from a previous update might overlap (for
> nonblocking atomic commits) with the prepare_planes for the next one.
> Either we need special cleanup vs. error-path code, or some flag somewhere
> in the drm_plane_state.

Ok, here's pretty much the previous version, which works now that I've
fixed the intel driver. Instead of just comparing the fb's, I'm using
the framebuffer_changed function, which seems like a nice bit of
documentation if nothing else.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-drm-Don-t-prepare-or-cleanup-unchanging-frame-buffer.patch --]
[-- Type: text/x-diff, Size: 3024 bytes --]

From a75251d5762b1c200ed2f3dca2a5b00cc85bea95 Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Sat, 4 Jun 2016 01:16:22 -0700
Subject: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v3]

When reconfiguring a plane position (as in moving the cursor), the
frame buffer for the cursor isn't changing, so don't call the prepare
or cleanup driver functions.

This avoids making cursor position updates block on all pending rendering.

v3: use drm_atomic_helper_framebuffer_changed in both prepare and
    cleanup phases instead of keeping state in the plane.

cc: dri-devel@lists.freedesktop.org
cc: David Airlie <airlied@linux.ie>
cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ddfa0d1..72e50bc 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1246,18 +1246,19 @@ EXPORT_SYMBOL(drm_atomic_helper_commit);
  * Returns:
  * 0 on success, negative error code on failure.
  */
+
 int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 				     struct drm_atomic_state *state)
 {
-	int nplanes = dev->mode_config.num_total_plane;
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
 	int ret, i;
+	int max_prepared_i = 0;
 
-	for (i = 0; i < nplanes; i++) {
+	for_each_plane_in_state(state, plane, plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
-		struct drm_plane *plane = state->planes[i];
-		struct drm_plane_state *plane_state = state->plane_states[i];
 
-		if (!plane)
+		if (!drm_atomic_helper_framebuffer_changed(dev, state, plane_state->crtc))
 			continue;
 
 		funcs = plane->helper_private;
@@ -1267,24 +1268,25 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 			if (ret)
 				goto fail;
 		}
+		max_prepared_i = i;
 	}
 
 	return 0;
 
 fail:
-	for (i--; i >= 0; i--) {
+	for_each_plane_in_state(state, plane, plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
-		struct drm_plane *plane = state->planes[i];
-		struct drm_plane_state *plane_state = state->plane_states[i];
 
-		if (!plane)
+		if (i > max_prepared_i)
+			break;
+
+		if (!drm_atomic_helper_framebuffer_changed(dev, state, plane_state->crtc))
 			continue;
 
 		funcs = plane->helper_private;
 
 		if (funcs->cleanup_fb)
 			funcs->cleanup_fb(plane, plane_state);
-
 	}
 
 	return ret;
@@ -1527,6 +1529,9 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
 	for_each_plane_in_state(old_state, plane, plane_state, i) {
 		const struct drm_plane_helper_funcs *funcs;
 
+		if (!drm_atomic_helper_framebuffer_changed(dev, old_state, plane_state->crtc))
+			continue;
+
 		funcs = plane->helper_private;
 
 		if (funcs->cleanup_fb)
-- 
2.8.1


[-- Attachment #1.3: Type: text/plain, Size: 15 bytes --]


-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 810 bytes --]

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

* Re: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
  2016-08-07  5:26     ` Keith Packard
@ 2016-08-08  9:00       ` Daniel Vetter
  -1 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2016-08-08  9:00 UTC (permalink / raw)
  To: Keith Packard; +Cc: Daniel Vetter, linux-kernel, dri-devel, airlied

On Sat, Aug 06, 2016 at 10:26:09PM -0700, Keith Packard wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
> 
> > Hm, I can't see v1 anywhere, but I think it'd be better. You can't store
> > any transient state related to the current update in struct drm_plane. In
> > this case the cleanup_buffers from a previous update might overlap (for
> > nonblocking atomic commits) with the prepare_planes for the next one.
> > Either we need special cleanup vs. error-path code, or some flag somewhere
> > in the drm_plane_state.
> 
> Ok, here's pretty much the previous version, which works now that I've
> fixed the intel driver. Instead of just comparing the fb's, I'm using
> the framebuffer_changed function, which seems like a nice bit of
> documentation if nothing else.
> 

> From a75251d5762b1c200ed2f3dca2a5b00cc85bea95 Mon Sep 17 00:00:00 2001
> From: Keith Packard <keithp@keithp.com>
> Date: Sat, 4 Jun 2016 01:16:22 -0700
> Subject: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v3]
> 
> When reconfiguring a plane position (as in moving the cursor), the
> frame buffer for the cursor isn't changing, so don't call the prepare
> or cleanup driver functions.
> 
> This avoids making cursor position updates block on all pending rendering.
> 
> v3: use drm_atomic_helper_framebuffer_changed in both prepare and
>     cleanup phases instead of keeping state in the plane.
> 
> cc: dri-devel@lists.freedesktop.org
> cc: David Airlie <airlied@linux.ie>
> cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Keith Packard <keithp@keithp.com>

Rebased onto 4.8 while applying, which did simplify the patch a lot (4.8
is already using for_each_plane_in_state, but slightly differently).

Thanks, Daniel
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ddfa0d1..72e50bc 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1246,18 +1246,19 @@ EXPORT_SYMBOL(drm_atomic_helper_commit);
>   * Returns:
>   * 0 on success, negative error code on failure.
>   */
> +
>  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>  				     struct drm_atomic_state *state)
>  {
> -	int nplanes = dev->mode_config.num_total_plane;
> +	struct drm_plane *plane;
> +	struct drm_plane_state *plane_state;
>  	int ret, i;
> +	int max_prepared_i = 0;
>  
> -	for (i = 0; i < nplanes; i++) {
> +	for_each_plane_in_state(state, plane, plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
> -		struct drm_plane *plane = state->planes[i];
> -		struct drm_plane_state *plane_state = state->plane_states[i];
>  
> -		if (!plane)
> +		if (!drm_atomic_helper_framebuffer_changed(dev, state, plane_state->crtc))
>  			continue;
>  
>  		funcs = plane->helper_private;
> @@ -1267,24 +1268,25 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>  			if (ret)
>  				goto fail;
>  		}
> +		max_prepared_i = i;
>  	}
>  
>  	return 0;
>  
>  fail:
> -	for (i--; i >= 0; i--) {
> +	for_each_plane_in_state(state, plane, plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
> -		struct drm_plane *plane = state->planes[i];
> -		struct drm_plane_state *plane_state = state->plane_states[i];
>  
> -		if (!plane)
> +		if (i > max_prepared_i)
> +			break;
> +
> +		if (!drm_atomic_helper_framebuffer_changed(dev, state, plane_state->crtc))
>  			continue;
>  
>  		funcs = plane->helper_private;
>  
>  		if (funcs->cleanup_fb)
>  			funcs->cleanup_fb(plane, plane_state);
> -
>  	}
>  
>  	return ret;
> @@ -1527,6 +1529,9 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
>  	for_each_plane_in_state(old_state, plane, plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
>  
> +		if (!drm_atomic_helper_framebuffer_changed(dev, old_state, plane_state->crtc))
> +			continue;
> +
>  		funcs = plane->helper_private;
>  
>  		if (funcs->cleanup_fb)
> -- 
> 2.8.1
> 

> 
> -- 
> -keith




-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
@ 2016-08-08  9:00       ` Daniel Vetter
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2016-08-08  9:00 UTC (permalink / raw)
  To: Keith Packard; +Cc: dri-devel, linux-kernel

On Sat, Aug 06, 2016 at 10:26:09PM -0700, Keith Packard wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
> 
> > Hm, I can't see v1 anywhere, but I think it'd be better. You can't store
> > any transient state related to the current update in struct drm_plane. In
> > this case the cleanup_buffers from a previous update might overlap (for
> > nonblocking atomic commits) with the prepare_planes for the next one.
> > Either we need special cleanup vs. error-path code, or some flag somewhere
> > in the drm_plane_state.
> 
> Ok, here's pretty much the previous version, which works now that I've
> fixed the intel driver. Instead of just comparing the fb's, I'm using
> the framebuffer_changed function, which seems like a nice bit of
> documentation if nothing else.
> 

> From a75251d5762b1c200ed2f3dca2a5b00cc85bea95 Mon Sep 17 00:00:00 2001
> From: Keith Packard <keithp@keithp.com>
> Date: Sat, 4 Jun 2016 01:16:22 -0700
> Subject: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v3]
> 
> When reconfiguring a plane position (as in moving the cursor), the
> frame buffer for the cursor isn't changing, so don't call the prepare
> or cleanup driver functions.
> 
> This avoids making cursor position updates block on all pending rendering.
> 
> v3: use drm_atomic_helper_framebuffer_changed in both prepare and
>     cleanup phases instead of keeping state in the plane.
> 
> cc: dri-devel@lists.freedesktop.org
> cc: David Airlie <airlied@linux.ie>
> cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Keith Packard <keithp@keithp.com>

Rebased onto 4.8 while applying, which did simplify the patch a lot (4.8
is already using for_each_plane_in_state, but slightly differently).

Thanks, Daniel
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ddfa0d1..72e50bc 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1246,18 +1246,19 @@ EXPORT_SYMBOL(drm_atomic_helper_commit);
>   * Returns:
>   * 0 on success, negative error code on failure.
>   */
> +
>  int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>  				     struct drm_atomic_state *state)
>  {
> -	int nplanes = dev->mode_config.num_total_plane;
> +	struct drm_plane *plane;
> +	struct drm_plane_state *plane_state;
>  	int ret, i;
> +	int max_prepared_i = 0;
>  
> -	for (i = 0; i < nplanes; i++) {
> +	for_each_plane_in_state(state, plane, plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
> -		struct drm_plane *plane = state->planes[i];
> -		struct drm_plane_state *plane_state = state->plane_states[i];
>  
> -		if (!plane)
> +		if (!drm_atomic_helper_framebuffer_changed(dev, state, plane_state->crtc))
>  			continue;
>  
>  		funcs = plane->helper_private;
> @@ -1267,24 +1268,25 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>  			if (ret)
>  				goto fail;
>  		}
> +		max_prepared_i = i;
>  	}
>  
>  	return 0;
>  
>  fail:
> -	for (i--; i >= 0; i--) {
> +	for_each_plane_in_state(state, plane, plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
> -		struct drm_plane *plane = state->planes[i];
> -		struct drm_plane_state *plane_state = state->plane_states[i];
>  
> -		if (!plane)
> +		if (i > max_prepared_i)
> +			break;
> +
> +		if (!drm_atomic_helper_framebuffer_changed(dev, state, plane_state->crtc))
>  			continue;
>  
>  		funcs = plane->helper_private;
>  
>  		if (funcs->cleanup_fb)
>  			funcs->cleanup_fb(plane, plane_state);
> -
>  	}
>  
>  	return ret;
> @@ -1527,6 +1529,9 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
>  	for_each_plane_in_state(old_state, plane, plane_state, i) {
>  		const struct drm_plane_helper_funcs *funcs;
>  
> +		if (!drm_atomic_helper_framebuffer_changed(dev, old_state, plane_state->crtc))
> +			continue;
> +
>  		funcs = plane->helper_private;
>  
>  		if (funcs->cleanup_fb)
> -- 
> 2.8.1
> 

> 
> -- 
> -keith




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

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

* Re: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
  2016-08-08  9:00       ` Daniel Vetter
@ 2016-08-08 15:17         ` Keith Packard
  -1 siblings, 0 replies; 11+ messages in thread
From: Keith Packard @ 2016-08-08 15:17 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, linux-kernel, dri-devel, airlied

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> Rebased onto 4.8 while applying, which did simplify the patch a lot (4.8
> is already using for_each_plane_in_state, but slightly differently).

Your rebase looks great, thanks!

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 810 bytes --]

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

* Re: [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2]
@ 2016-08-08 15:17         ` Keith Packard
  0 siblings, 0 replies; 11+ messages in thread
From: Keith Packard @ 2016-08-08 15:17 UTC (permalink / raw)
  Cc: Daniel Vetter, linux-kernel, dri-devel, airlied

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> Rebased onto 4.8 while applying, which did simplify the patch a lot (4.8
> is already using for_each_plane_in_state, but slightly differently).

Your rebase looks great, thanks!

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 810 bytes --]

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

end of thread, other threads:[~2016-08-08 15:17 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-31  8:08 [PATCH] drm: Don't prepare or cleanup unchanging frame buffers [v2] Keith Packard
2016-07-31  9:20 ` kbuild test robot
2016-07-31  9:20   ` kbuild test robot
2016-08-02 13:54 ` Daniel Vetter
2016-08-02 13:54   ` Daniel Vetter
2016-08-07  5:26   ` Keith Packard
2016-08-07  5:26     ` Keith Packard
2016-08-08  9:00     ` Daniel Vetter
2016-08-08  9:00       ` Daniel Vetter
2016-08-08 15:17       ` Keith Packard
2016-08-08 15:17         ` Keith Packard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.