[PATCH v2] drm/atomic: Allow for holes in connector state, v2.

[PATCH v2] drm/atomic: Allow for holes in connector state, v2.

[Maarten Lankhorst]

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.

Changes since v1:
- Whitespace. (Ville)
- Call ida_remove when destroying the connector. (Ville)
- u32 alloc -> int. (Ville)

Fixes: 14de6c44d149 ("drm/atomic: Remove drm_atomic_connectors_for_crtc.")
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c        | 44 +++++++++++++++------------------
 drivers/gpu/drm/drm_atomic_helper.c |  2 +-
 drivers/gpu/drm/drm_crtc.c          | 49 +++++++++++++++----------------------
 include/drm/drm_crtc.h              |  8 +++++-
 4 files changed, 48 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 8fb469c4e4b8..092620c6ff32 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,27 @@ 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;
+		int 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..84514001dcef 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -918,12 +918,19 @@ 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 +938,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 +966,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);
@@ -996,6 +1008,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
 	ida_remove(&drm_connector_enum_list[connector->connector_type].ida,
 		   connector->connector_type_id);
 
+	ida_remove(&dev->mode_config.connector_ida,
+		   connector->connector_id);
+
 	kfree(connector->display_info.bus_formats);
 	drm_mode_object_put(dev, &connector->base);
 	kfree(connector->name);
@@ -1013,32 +1028,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 +5827,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 +5908,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

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

✓ Fi.CI.BAT: success for drm/atomic: Allow for holes in connector state, v2.

[Patchwork]

== Summary ==

Series 3514v1 drm/atomic: Allow for holes in connector state, v2.
http://patchwork.freedesktop.org/api/1.0/series/3514/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-warn -> PASS       (byt-nuc)

byt-nuc          total:165  pass:141  dwarn:0   dfail:0   fail:0   skip:24 
ilk-hp8440p      total:165  pass:115  dwarn:1   dfail:0   fail:1   skip:48 
snb-x220t        total:165  pass:142  dwarn:0   dfail:0   fail:2   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1421/

bd0b1a9aa8b7fdb2e06a5cbf1756ef93de2fa3fd drm-intel-nightly: 2016y-02m-16d-17h-53m-05s UTC integration manifest
d8fd198f83edf12cb2611f483585350a77a2ccb3 drm/atomic: Allow for holes in connector state, v2.

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

Re: [v2] drm/atomic: Allow for holes in connector state, v2.

[Lyude]

Works perfectly, fixes the issues with MST over here.

Reviewed-by: Lyude <cpaul@redhat.com>

On Wed, 2016-02-17 at 08:32 +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.
> 
> Changes since v1:
> - Whitespace. (Ville)
> - Call ida_remove when destroying the connector. (Ville)
> - u32 alloc -> int. (Ville)
> 
> Fixes: 14de6c44d149 ("drm/atomic: Remove drm_atomic_connectors_for_crtc.")
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c        | 44 +++++++++++++++------------------
>  drivers/gpu/drm/drm_atomic_helper.c |  2 +-
>  drivers/gpu/drm/drm_crtc.c          | 49 +++++++++++++++---------------------
> -
>  include/drm/drm_crtc.h              |  8 +++++-
>  4 files changed, 48 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 8fb469c4e4b8..092620c6ff32 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,27 @@ 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;
> +		int 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..84514001dcef 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -918,12 +918,19 @@ 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 +938,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 +966,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);
> @@ -996,6 +1008,9 @@ void drm_connector_cleanup(struct drm_connector
> *connector)
>  	ida_remove(&drm_connector_enum_list[connector->connector_type].ida,
>  		   connector->connector_type_id);
>  
> +	ida_remove(&dev->mode_config.connector_ida,
> +		   connector->connector_id);
> +
>  	kfree(connector->display_info.bus_formats);
>  	drm_mode_object_put(dev, &connector->base);
>  	kfree(connector->name);
> @@ -1013,32 +1028,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 +5827,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 +5908,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);
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/atomic: Allow for holes in connector state, v2.

[Ville Syrjälä]

On Wed, Feb 17, 2016 at 08:32:05AM +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.
> 
> Changes since v1:
> - Whitespace. (Ville)
> - Call ida_remove when destroying the connector. (Ville)
> - u32 alloc -> int. (Ville)
> 
> Fixes: 14de6c44d149 ("drm/atomic: Remove drm_atomic_connectors_for_crtc.")
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c        | 44 +++++++++++++++------------------
>  drivers/gpu/drm/drm_atomic_helper.c |  2 +-
>  drivers/gpu/drm/drm_crtc.c          | 49 +++++++++++++++----------------------
>  include/drm/drm_crtc.h              |  8 +++++-
>  4 files changed, 48 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 8fb469c4e4b8..092620c6ff32 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,27 @@ 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;
> +		int 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..84514001dcef 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -918,12 +918,19 @@ 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 +938,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 +966,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);

That seems to fix a pre-existing connector_type_id leak as well. Could
mention it in the commit message.

Anyways, this seems OK to me. There's some potential for confusion with
connector_id, connector_type_id and connector->base.id perhaps, but I
don't really care so

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> @@ -996,6 +1008,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
>  	ida_remove(&drm_connector_enum_list[connector->connector_type].ida,
>  		   connector->connector_type_id);
>  
> +	ida_remove(&dev->mode_config.connector_ida,
> +		   connector->connector_id);
> +
>  	kfree(connector->display_info.bus_formats);
>  	drm_mode_object_put(dev, &connector->base);
>  	kfree(connector->name);
> @@ -1013,32 +1028,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 +5827,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 +5908,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

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