| Message ID | 1455542221-4848-1-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Maarten, [auto build test WARNING on drm/drm-next] [also build test WARNING on v4.5-rc4 next-20160215] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Maarten-Lankhorst/drm-atomic-Allow-for-holes-in-connector-state/20160215-212056 base: git://people.freedesktop.org/~airlied/linux.git drm-next reproduce: make htmldocs All warnings (new ones prefixed by >>): drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'wedged' drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'fmt' include/drm/drm_crtc.h:359: warning: No description found for parameter 'mode_blob' include/drm/drm_crtc.h:774: warning: No description found for parameter 'name' >> include/drm/drm_crtc.h:1233: warning: No description found for parameter 'connector_id' include/drm/drm_crtc.h:1233: warning: No description found for parameter 'tile_blob_ptr' include/drm/drm_crtc.h:1272: warning: No description found for parameter 'rotation' include/drm/drm_crtc.h:1534: warning: No description found for parameter 'name' include/drm/drm_crtc.h:1534: warning: No description found for parameter 'mutex' include/drm/drm_crtc.h:1534: warning: No description found for parameter 'helper_private' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tile_idr' >> include/drm/drm_crtc.h:2144: warning: No description found for parameter 'connector_ida' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'delayed_event' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'edid_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'dpms_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'path_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tile_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'plane_type_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'rotation_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'prop_src_x' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'prop_src_y' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'prop_src_w' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'prop_src_h' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'prop_crtc_x' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'prop_crtc_y' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'prop_crtc_w' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'prop_crtc_h' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'prop_fb_id' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'prop_crtc_id' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'prop_active' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'prop_mode_id' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'dvi_i_subconnector_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'dvi_i_select_subconnector_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_subconnector_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_select_subconnector_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_mode_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_left_margin_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_right_margin_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_top_margin_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_bottom_margin_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_brightness_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_contrast_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_flicker_reduction_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_overscan_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_saturation_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'tv_hue_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'scaling_mode_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'aspect_ratio_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'dirty_info_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'suggested_x_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'suggested_y_property' include/drm/drm_crtc.h:2144: warning: No description found for parameter 'allow_fb_modifiers' include/drm/drm_dp_helper.h:749: warning: No description found for parameter 'i2c_nack_count' include/drm/drm_dp_helper.h:749: warning: No description found for parameter 'i2c_defer_count' drivers/gpu/drm/drm_dp_mst_topology.c:2364: 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:2364: warning: No description found for parameter 'connector' drivers/gpu/drm/drm_irq.c:176: warning: No description found for parameter 'flags' include/drm/drmP.h:168: warning: No description found for parameter 'fmt' include/drm/drmP.h:184: warning: No description found for parameter 'fmt' include/drm/drmP.h:202: warning: No description found for parameter 'fmt' include/drm/drmP.h:247: warning: No description found for parameter 'dev' include/drm/drmP.h:247: warning: No description found for parameter 'data' include/drm/drmP.h:247: warning: No description found for parameter 'file_priv' include/drm/drmP.h:280: warning: No description found for parameter 'ioctl' include/drm/drmP.h:280: warning: No description found for parameter '_func' include/drm/drmP.h:280: warning: No description found for parameter '_flags' include/drm/drmP.h:362: warning: cannot understand function prototype: 'struct drm_lock_data ' include/drm/drmP.h:415: warning: cannot understand function prototype: 'struct drm_driver ' include/drm/drmP.h:672: warning: cannot understand function prototype: 'struct drm_info_list ' include/drm/drmP.h:682: warning: cannot understand function prototype: 'struct drm_info_node ' include/drm/drmP.h:692: warning: cannot understand function prototype: 'struct drm_minor ' include/drm/drmP.h:740: warning: cannot understand function prototype: 'struct drm_device ' drivers/gpu/drm/i915/intel_runtime_pm.c:2173: warning: No description found for parameter 'resume' drivers/gpu/drm/i915/intel_runtime_pm.c:2173: warning: No description found for parameter 'resume' drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'wedged' drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'fmt' drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'wedged' drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'fmt' drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'wedged' drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'fmt' drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'wedged' drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'fmt' drivers/gpu/drm/i915/i915_gem.c:421: warning: No description found for parameter 'dev' drivers/gpu/drm/i915/i915_gem.c:421: warning: No description found for parameter 'data' drivers/gpu/drm/i915/i915_gem.c:421: warning: No description found for parameter 'file' drivers/gpu/drm/i915/i915_gem.c:686: warning: No description found for parameter 'dev' drivers/gpu/drm/i915/i915_gem.c:686: warning: No description found for parameter 'data' drivers/gpu/drm/i915/i915_gem.c:686: warning: No description found for parameter 'file' drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'dev' drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'obj' drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'args' vim +/connector_id +1233 include/drm/drm_crtc.h 6ba2bd3d Todd Previte 2015-04-21 1217 * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6 6ba2bd3d Todd Previte 2015-04-21 1218 */ 6ba2bd3d Todd Previte 2015-04-21 1219 bool edid_corrupt; 6ba2bd3d Todd Previte 2015-04-21 1220 30f65707 Thomas Wood 2014-06-18 1221 struct dentry *debugfs_entry; 144ecb97 Daniel Vetter 2014-10-27 1222 144ecb97 Daniel Vetter 2014-10-27 1223 struct drm_connector_state *state; 40d9b043 Dave Airlie 2014-10-20 1224 40d9b043 Dave Airlie 2014-10-20 1225 /* DisplayID bits */ 40d9b043 Dave Airlie 2014-10-20 1226 bool has_tile; 40d9b043 Dave Airlie 2014-10-20 1227 struct drm_tile_group *tile_group; 40d9b043 Dave Airlie 2014-10-20 1228 bool tile_is_single_monitor; 40d9b043 Dave Airlie 2014-10-20 1229 40d9b043 Dave Airlie 2014-10-20 1230 uint8_t num_h_tile, num_v_tile; 40d9b043 Dave Airlie 2014-10-20 1231 uint8_t tile_h_loc, tile_v_loc; 40d9b043 Dave Airlie 2014-10-20 1232 uint16_t tile_h_size, tile_v_size; f453ba04 Dave Airlie 2008-11-07 @1233 }; f453ba04 Dave Airlie 2008-11-07 1234 f453ba04 Dave Airlie 2008-11-07 1235 /** 144ecb97 Daniel Vetter 2014-10-27 1236 * struct drm_plane_state - mutable plane state 07cc0ef6 Daniel Vetter 2014-11-27 1237 * @plane: backpointer to the plane 144ecb97 Daniel Vetter 2014-10-27 1238 * @crtc: currently bound CRTC, NULL if disabled cc4ceb48 Daniel Vetter 2014-07-25 1239 * @fb: currently bound framebuffer e2330f07 Daniel Vetter 2014-10-29 1240 * @fence: optional fence to wait for before scanning out @fb 144ecb97 Daniel Vetter 2014-10-27 1241 * @crtc_x: left position of visible portion of plane on crtc :::::: The code at line 1233 was first introduced by commit :::::: f453ba0460742ad027ae0c4c7d61e62817b3e7ef DRM: add mode setting support :::::: TO: Dave Airlie <airlied@redhat.com> :::::: CC: Dave Airlie <airlied@linux.ie> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Mon, Feb 15, 2016 at 02:17:01PM +0100, Maarten Lankhorst wrote: > Because we record connector_mask using 1 << drm_connector_index now > the connector_mask should stay the same even when other connectors > are removed. This was not the case with MST, in that case when removing > a connector all other connectors may change their index. > > This is fixed by waiting until the first get_connector_state to allocate > connector_state, and force reallocation when state is too small. > > As a side effect connector arrays no longer have to be preallocated, > and can be allocated on first use which means a less allocations in > the page flip only path. > > Fixes: 14de6c44d149 ("drm/atomic: Remove drm_atomic_connectors_for_crtc.") > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/drm_atomic.c | 45 +++++++++++++++++-------------------- > drivers/gpu/drm/drm_atomic_helper.c | 2 +- > drivers/gpu/drm/drm_crtc.c | 45 +++++++++++++------------------------ > include/drm/drm_crtc.h | 8 ++++++- > 4 files changed, 45 insertions(+), 55 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 8fb469c4e4b8..5d4774450540 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -65,8 +65,6 @@ drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state) > */ > state->allow_modeset = true; > > - state->num_connector = ACCESS_ONCE(dev->mode_config.num_connector); > - > state->crtcs = kcalloc(dev->mode_config.num_crtc, > sizeof(*state->crtcs), GFP_KERNEL); > if (!state->crtcs) > @@ -83,16 +81,6 @@ drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state) > sizeof(*state->plane_states), GFP_KERNEL); > if (!state->plane_states) > goto fail; > - state->connectors = kcalloc(state->num_connector, > - sizeof(*state->connectors), > - GFP_KERNEL); > - if (!state->connectors) > - goto fail; > - state->connector_states = kcalloc(state->num_connector, > - sizeof(*state->connector_states), > - GFP_KERNEL); > - if (!state->connector_states) > - goto fail; > > state->dev = dev; > > @@ -823,19 +811,28 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state, > > index = drm_connector_index(connector); > > - /* > - * Construction of atomic state updates can race with a connector > - * hot-add which might overflow. In this case flip the table and just > - * restart the entire ioctl - no one is fast enough to livelock a cpu > - * with physical hotplug events anyway. > - * > - * Note that we only grab the indexes once we have the right lock to > - * prevent hotplug/unplugging of connectors. So removal is no problem, > - * at most the array is a bit too large. > - */ > if (index >= state->num_connector) { > - DRM_DEBUG_ATOMIC("Hot-added connector would overflow state array, restarting\n"); > - return ERR_PTR(-EAGAIN); > + struct drm_connector **c; > + struct drm_connector_state **cs; > + > + u32 alloc = max(index + 1, config->num_connector); Why u32? > + > + c = krealloc(state->connectors, alloc * sizeof(*state->connectors), GFP_KERNEL); > + if (!c) > + return ERR_PTR(-ENOMEM); > + > + state->connectors = c; > + memset(&state->connectors[state->num_connector], 0, > + sizeof(*state->connectors) * (alloc - state->num_connector)); > + > + cs = krealloc(state->connector_states, alloc * sizeof(*state->connector_states), GFP_KERNEL); > + if (!cs) > + return ERR_PTR(-ENOMEM); > + > + state->connector_states = cs; > + memset(&state->connector_states[state->num_connector], 0, > + sizeof(*state->connector_states) * (alloc - state->num_connector)); > + state->num_connector = alloc; > } > > if (state->connector_states[index]) > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 2b430b05f35d..4da4f2a49078 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1535,7 +1535,7 @@ void drm_atomic_helper_swap_state(struct drm_device *dev, > { > int i; > > - for (i = 0; i < dev->mode_config.num_connector; i++) { > + for (i = 0; i < state->num_connector; i++) { > struct drm_connector *connector = state->connectors[i]; > > if (!connector) > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 65258acddb90..ea00aea88de7 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -918,12 +918,18 @@ int drm_connector_init(struct drm_device *dev, > connector->base.properties = &connector->properties; > connector->dev = dev; > connector->funcs = funcs; Could use an empty line here. > + connector->connector_id = ida_simple_get(&config->connector_ida, 0, 0, GFP_KERNEL); > + if (connector->connector_id < 0) { > + ret = connector->connector_id; > + goto out_put; > + } > + > connector->connector_type = connector_type; > connector->connector_type_id = > ida_simple_get(connector_ida, 1, 0, GFP_KERNEL); > if (connector->connector_type_id < 0) { > ret = connector->connector_type_id; > - goto out_put; > + goto out_put_id; > } > connector->name = > kasprintf(GFP_KERNEL, "%s-%d", > @@ -931,7 +937,7 @@ int drm_connector_init(struct drm_device *dev, > connector->connector_type_id); > if (!connector->name) { > ret = -ENOMEM; > - goto out_put; > + goto out_put_type_id; > } > > INIT_LIST_HEAD(&connector->probed_modes); > @@ -959,7 +965,12 @@ int drm_connector_init(struct drm_device *dev, > } > > connector->debugfs_entry = NULL; > - > +out_put_type_id: > + if (ret) > + ida_remove(connector_ida, connector->connector_type_id); > +out_put_id: > + if (ret) > + ida_remove(&config->connector_ida, connector->connector_id); Should there be an ida_remove() call somewhere when unregistering or destroying the connector? BTW looking at intel_dp_destroy_mst_connector(), shouldn't we unregister/do something before we do the modeset? What's there to prevent userspac/someone from racing with this and re-enabling the connector before we unregister it? Or maybe the connector is already defunct by this time and something else will prevent any modeset using the connector from succeeding? > out_put: > if (ret) > drm_mode_object_put(dev, &connector->base); > @@ -1013,32 +1024,6 @@ void drm_connector_cleanup(struct drm_connector *connector) > EXPORT_SYMBOL(drm_connector_cleanup); > > /** > - * drm_connector_index - find the index of a registered connector > - * @connector: connector to find index for > - * > - * Given a registered connector, return the index of that connector within a DRM > - * device's list of connectors. > - */ > -unsigned int drm_connector_index(struct drm_connector *connector) > -{ > - unsigned int index = 0; > - struct drm_connector *tmp; > - struct drm_mode_config *config = &connector->dev->mode_config; > - > - WARN_ON(!drm_modeset_is_locked(&config->connection_mutex)); > - > - drm_for_each_connector(tmp, connector->dev) { > - if (tmp == connector) > - return index; > - > - index++; > - } > - > - BUG(); > -} > -EXPORT_SYMBOL(drm_connector_index); > - > -/** > * drm_connector_register - register a connector > * @connector: the connector to register > * > @@ -5838,6 +5823,7 @@ void drm_mode_config_init(struct drm_device *dev) > INIT_LIST_HEAD(&dev->mode_config.plane_list); > idr_init(&dev->mode_config.crtc_idr); > idr_init(&dev->mode_config.tile_idr); > + ida_init(&dev->mode_config.connector_ida); > > drm_modeset_lock_all(dev); > drm_mode_create_standard_properties(dev); > @@ -5918,6 +5904,7 @@ void drm_mode_config_cleanup(struct drm_device *dev) > crtc->funcs->destroy(crtc); > } > > + ida_destroy(&dev->mode_config.connector_ida); > idr_destroy(&dev->mode_config.tile_idr); > idr_destroy(&dev->mode_config.crtc_idr); > drm_modeset_lock_fini(&dev->mode_config.connection_mutex); > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 8c7fb3d0f9d0..7fad193dc645 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -1168,6 +1168,7 @@ struct drm_connector { > struct drm_mode_object base; > > char *name; > + int connector_id; > int connector_type; > int connector_type_id; > bool interlace_allowed; > @@ -2049,6 +2050,7 @@ struct drm_mode_config { > struct list_head fb_list; > > int num_connector; > + struct ida connector_ida; > struct list_head connector_list; > int num_encoder; > struct list_head encoder_list; > @@ -2213,7 +2215,11 @@ int drm_connector_register(struct drm_connector *connector); > void drm_connector_unregister(struct drm_connector *connector); > > extern void drm_connector_cleanup(struct drm_connector *connector); > -extern unsigned int drm_connector_index(struct drm_connector *connector); > +static inline unsigned drm_connector_index(struct drm_connector *connector) > +{ > + return connector->connector_id; > +} > + > /* helper to unplug all connectors from sysfs for device */ > extern void drm_connector_unplug_all(struct drm_device *dev); > > -- > 2.1.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 16 February 2016 at 21:37, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Mon, Feb 15, 2016 at 02:17:01PM +0100, Maarten Lankhorst wrote: >> Because we record connector_mask using 1 << drm_connector_index now >> the connector_mask should stay the same even when other connectors >> are removed. This was not the case with MST, in that case when removing >> a connector all other connectors may change their index. >> >> This is fixed by waiting until the first get_connector_state to allocate >> connector_state, and force reallocation when state is too small. >> >> As a side effect connector arrays no longer have to be preallocated, >> and can be allocated on first use which means a less allocations in >> the page flip only path. Daniel you said something on irc about v2 of this for -fixes? Did I miss v2? Dave.
Op 19-02-16 om 04:21 schreef Dave Airlie: > On 16 February 2016 at 21:37, Ville Syrjälä > <ville.syrjala@linux.intel.com> wrote: >> On Mon, Feb 15, 2016 at 02:17:01PM +0100, Maarten Lankhorst wrote: >>> Because we record connector_mask using 1 << drm_connector_index now >>> the connector_mask should stay the same even when other connectors >>> are removed. This was not the case with MST, in that case when removing >>> a connector all other connectors may change their index. >>> >>> This is fixed by waiting until the first get_connector_state to allocate >>> connector_state, and force reallocation when state is too small. >>> >>> As a side effect connector arrays no longer have to be preallocated, >>> and can be allocated on first use which means a less allocations in >>> the page flip only path. > Daniel you said something on irc about v2 of this for -fixes? Did I miss v2? > > Dave. "[PATCH v2] drm/atomic: Allow for holes in connector state, v2." It wasn't sent in this thread to give CI a chance to run.
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 8fb469c4e4b8..5d4774450540 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -65,8 +65,6 @@ drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state) */ state->allow_modeset = true; - state->num_connector = ACCESS_ONCE(dev->mode_config.num_connector); - state->crtcs = kcalloc(dev->mode_config.num_crtc, sizeof(*state->crtcs), GFP_KERNEL); if (!state->crtcs) @@ -83,16 +81,6 @@ drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state) sizeof(*state->plane_states), GFP_KERNEL); if (!state->plane_states) goto fail; - state->connectors = kcalloc(state->num_connector, - sizeof(*state->connectors), - GFP_KERNEL); - if (!state->connectors) - goto fail; - state->connector_states = kcalloc(state->num_connector, - sizeof(*state->connector_states), - GFP_KERNEL); - if (!state->connector_states) - goto fail; state->dev = dev; @@ -823,19 +811,28 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state, index = drm_connector_index(connector); - /* - * Construction of atomic state updates can race with a connector - * hot-add which might overflow. In this case flip the table and just - * restart the entire ioctl - no one is fast enough to livelock a cpu - * with physical hotplug events anyway. - * - * Note that we only grab the indexes once we have the right lock to - * prevent hotplug/unplugging of connectors. So removal is no problem, - * at most the array is a bit too large. - */ if (index >= state->num_connector) { - DRM_DEBUG_ATOMIC("Hot-added connector would overflow state array, restarting\n"); - return ERR_PTR(-EAGAIN); + struct drm_connector **c; + struct drm_connector_state **cs; + + u32 alloc = max(index + 1, config->num_connector); + + c = krealloc(state->connectors, alloc * sizeof(*state->connectors), GFP_KERNEL); + if (!c) + return ERR_PTR(-ENOMEM); + + state->connectors = c; + memset(&state->connectors[state->num_connector], 0, + sizeof(*state->connectors) * (alloc - state->num_connector)); + + cs = krealloc(state->connector_states, alloc * sizeof(*state->connector_states), GFP_KERNEL); + if (!cs) + return ERR_PTR(-ENOMEM); + + state->connector_states = cs; + memset(&state->connector_states[state->num_connector], 0, + sizeof(*state->connector_states) * (alloc - state->num_connector)); + state->num_connector = alloc; } if (state->connector_states[index]) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 2b430b05f35d..4da4f2a49078 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1535,7 +1535,7 @@ void drm_atomic_helper_swap_state(struct drm_device *dev, { int i; - for (i = 0; i < dev->mode_config.num_connector; i++) { + for (i = 0; i < state->num_connector; i++) { struct drm_connector *connector = state->connectors[i]; if (!connector) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 65258acddb90..ea00aea88de7 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -918,12 +918,18 @@ int drm_connector_init(struct drm_device *dev, connector->base.properties = &connector->properties; connector->dev = dev; connector->funcs = funcs; + connector->connector_id = ida_simple_get(&config->connector_ida, 0, 0, GFP_KERNEL); + if (connector->connector_id < 0) { + ret = connector->connector_id; + goto out_put; + } + connector->connector_type = connector_type; connector->connector_type_id = ida_simple_get(connector_ida, 1, 0, GFP_KERNEL); if (connector->connector_type_id < 0) { ret = connector->connector_type_id; - goto out_put; + goto out_put_id; } connector->name = kasprintf(GFP_KERNEL, "%s-%d", @@ -931,7 +937,7 @@ int drm_connector_init(struct drm_device *dev, connector->connector_type_id); if (!connector->name) { ret = -ENOMEM; - goto out_put; + goto out_put_type_id; } INIT_LIST_HEAD(&connector->probed_modes); @@ -959,7 +965,12 @@ int drm_connector_init(struct drm_device *dev, } connector->debugfs_entry = NULL; - +out_put_type_id: + if (ret) + ida_remove(connector_ida, connector->connector_type_id); +out_put_id: + if (ret) + ida_remove(&config->connector_ida, connector->connector_id); out_put: if (ret) drm_mode_object_put(dev, &connector->base); @@ -1013,32 +1024,6 @@ void drm_connector_cleanup(struct drm_connector *connector) EXPORT_SYMBOL(drm_connector_cleanup); /** - * drm_connector_index - find the index of a registered connector - * @connector: connector to find index for - * - * Given a registered connector, return the index of that connector within a DRM - * device's list of connectors. - */ -unsigned int drm_connector_index(struct drm_connector *connector) -{ - unsigned int index = 0; - struct drm_connector *tmp; - struct drm_mode_config *config = &connector->dev->mode_config; - - WARN_ON(!drm_modeset_is_locked(&config->connection_mutex)); - - drm_for_each_connector(tmp, connector->dev) { - if (tmp == connector) - return index; - - index++; - } - - BUG(); -} -EXPORT_SYMBOL(drm_connector_index); - -/** * drm_connector_register - register a connector * @connector: the connector to register * @@ -5838,6 +5823,7 @@ void drm_mode_config_init(struct drm_device *dev) INIT_LIST_HEAD(&dev->mode_config.plane_list); idr_init(&dev->mode_config.crtc_idr); idr_init(&dev->mode_config.tile_idr); + ida_init(&dev->mode_config.connector_ida); drm_modeset_lock_all(dev); drm_mode_create_standard_properties(dev); @@ -5918,6 +5904,7 @@ void drm_mode_config_cleanup(struct drm_device *dev) crtc->funcs->destroy(crtc); } + ida_destroy(&dev->mode_config.connector_ida); idr_destroy(&dev->mode_config.tile_idr); idr_destroy(&dev->mode_config.crtc_idr); drm_modeset_lock_fini(&dev->mode_config.connection_mutex); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 8c7fb3d0f9d0..7fad193dc645 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1168,6 +1168,7 @@ struct drm_connector { struct drm_mode_object base; char *name; + int connector_id; int connector_type; int connector_type_id; bool interlace_allowed; @@ -2049,6 +2050,7 @@ struct drm_mode_config { struct list_head fb_list; int num_connector; + struct ida connector_ida; struct list_head connector_list; int num_encoder; struct list_head encoder_list; @@ -2213,7 +2215,11 @@ int drm_connector_register(struct drm_connector *connector); void drm_connector_unregister(struct drm_connector *connector); extern void drm_connector_cleanup(struct drm_connector *connector); -extern unsigned int drm_connector_index(struct drm_connector *connector); +static inline unsigned drm_connector_index(struct drm_connector *connector) +{ + return connector->connector_id; +} + /* helper to unplug all connectors from sysfs for device */ extern void drm_connector_unplug_all(struct drm_device *dev);
Because we record connector_mask using 1 << drm_connector_index now the connector_mask should stay the same even when other connectors are removed. This was not the case with MST, in that case when removing a connector all other connectors may change their index. This is fixed by waiting until the first get_connector_state to allocate connector_state, and force reallocation when state is too small. As a side effect connector arrays no longer have to be preallocated, and can be allocated on first use which means a less allocations in the page flip only path. Fixes: 14de6c44d149 ("drm/atomic: Remove drm_atomic_connectors_for_crtc.") Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/drm_atomic.c | 45 +++++++++++++++++-------------------- drivers/gpu/drm/drm_atomic_helper.c | 2 +- drivers/gpu/drm/drm_crtc.c | 45 +++++++++++++------------------------ include/drm/drm_crtc.h | 8 ++++++- 4 files changed, 45 insertions(+), 55 deletions(-)