* [PATCH] drm: use ida to allocate connector ids
@ 2013-07-30 7:16 Ilia Mirkin
2013-07-30 7:51 ` [PATCH v2] " Ilia Mirkin
0 siblings, 1 reply; 11+ messages in thread
From: Ilia Mirkin @ 2013-07-30 7:16 UTC (permalink / raw)
To: David Airlie; +Cc: dri-devel
This makes it so that reloading a module does not cause all the
connector ids to change, which are user-visible and sometimes used
for configuration.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
---
Only mild testing... reloaded nouveau a few times, all the connectors
kept their original ids.
drivers/gpu/drm/drm_crtc.c | 61 +++++++++++++++++++++++++++++++---------------
drivers/gpu/drm/drm_drv.c | 2 ++
include/drm/drm_crtc.h | 2 ++
3 files changed, 46 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index fc83bb9..fff238f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -186,29 +186,29 @@ static const struct drm_prop_enum_list drm_dirty_info_enum_list[] = {
struct drm_conn_prop_enum_list {
int type;
const char *name;
- int count;
+ struct ida count;
};
/*
* Connector and encoder types.
*/
static struct drm_conn_prop_enum_list drm_connector_enum_list[] =
-{ { DRM_MODE_CONNECTOR_Unknown, "Unknown", 0 },
- { DRM_MODE_CONNECTOR_VGA, "VGA", 0 },
- { DRM_MODE_CONNECTOR_DVII, "DVI-I", 0 },
- { DRM_MODE_CONNECTOR_DVID, "DVI-D", 0 },
- { DRM_MODE_CONNECTOR_DVIA, "DVI-A", 0 },
- { DRM_MODE_CONNECTOR_Composite, "Composite", 0 },
- { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO", 0 },
- { DRM_MODE_CONNECTOR_LVDS, "LVDS", 0 },
- { DRM_MODE_CONNECTOR_Component, "Component", 0 },
- { DRM_MODE_CONNECTOR_9PinDIN, "DIN", 0 },
- { DRM_MODE_CONNECTOR_DisplayPort, "DP", 0 },
- { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A", 0 },
- { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B", 0 },
- { DRM_MODE_CONNECTOR_TV, "TV", 0 },
- { DRM_MODE_CONNECTOR_eDP, "eDP", 0 },
- { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual", 0},
+{ { DRM_MODE_CONNECTOR_Unknown, "Unknown" },
+ { DRM_MODE_CONNECTOR_VGA, "VGA" },
+ { DRM_MODE_CONNECTOR_DVII, "DVI-I" },
+ { DRM_MODE_CONNECTOR_DVID, "DVI-D" },
+ { DRM_MODE_CONNECTOR_DVIA, "DVI-A" },
+ { DRM_MODE_CONNECTOR_Composite, "Composite" },
+ { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO" },
+ { DRM_MODE_CONNECTOR_LVDS, "LVDS" },
+ { DRM_MODE_CONNECTOR_Component, "Component" },
+ { DRM_MODE_CONNECTOR_9PinDIN, "DIN" },
+ { DRM_MODE_CONNECTOR_DisplayPort, "DP" },
+ { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A" },
+ { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B" },
+ { DRM_MODE_CONNECTOR_TV, "TV" },
+ { DRM_MODE_CONNECTOR_eDP, "eDP" },
+ { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
};
static const struct drm_prop_enum_list drm_encoder_enum_list[] =
@@ -220,6 +220,22 @@ static const struct drm_prop_enum_list drm_encoder_enum_list[] =
{ DRM_MODE_ENCODER_VIRTUAL, "Virtual" },
};
+void drm_connector_ida_init(void)
+{
+ int i;
+
+ for (i = 0; i <= ARRAY_SIZE(drm_connector_enum_list); i++)
+ ida_init(&drm_connector_enum_list[i].count);
+}
+
+void drm_connector_ida_destroy(void)
+{
+ int i;
+
+ for (i = 0; i <= ARRAY_SIZE(drm_connector_enum_list); i++)
+ ida_destroy(&drm_connector_enum_list[i].count);
+}
+
const char *drm_get_encoder_name(const struct drm_encoder *encoder)
{
static char buf[32];
@@ -711,6 +727,9 @@ int drm_connector_init(struct drm_device *dev,
int connector_type)
{
int ret;
+ struct ida *count = &drm_connector_enum_list[connector_type].count;
+
+ ida_pre_get(count, GFP_KERNEL);
drm_modeset_lock_all(dev);
@@ -722,8 +741,9 @@ int drm_connector_init(struct drm_device *dev,
connector->dev = dev;
connector->funcs = funcs;
connector->connector_type = connector_type;
- connector->connector_type_id =
- ++drm_connector_enum_list[connector_type].count; /* TODO */
+ ret = ida_get_new_above(count, 1, &connector->connector_type_id);
+ if (ret)
+ goto out;
INIT_LIST_HEAD(&connector->probed_modes);
INIT_LIST_HEAD(&connector->modes);
connector->edid_blob_ptr = NULL;
@@ -764,6 +784,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
list_for_each_entry_safe(mode, t, &connector->modes, head)
drm_mode_remove(connector, mode);
+ ida_remove(&drm_connector_enum_list[connector->connector_type].count,
+ connector->connector_type_id);
+
drm_mode_object_put(dev, &connector->base);
list_del(&connector->head);
dev->mode_config.num_connector--;
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 99fcd7c..00597a1 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -251,6 +251,7 @@ static int __init drm_core_init(void)
int ret = -ENOMEM;
drm_global_init();
+ drm_connector_ida_init();
idr_init(&drm_minors_idr);
if (register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops))
@@ -298,6 +299,7 @@ static void __exit drm_core_exit(void)
unregister_chrdev(DRM_MAJOR, "drm");
+ drm_connector_ida_destroy();
idr_destroy(&drm_minors_idr);
}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index fa12a2f..effee9d 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -869,6 +869,8 @@ extern int drm_crtc_init(struct drm_device *dev,
const struct drm_crtc_funcs *funcs);
extern void drm_crtc_cleanup(struct drm_crtc *crtc);
+extern void drm_connector_ida_init(void);
+extern void drm_connector_ida_destroy(void);
extern int drm_connector_init(struct drm_device *dev,
struct drm_connector *connector,
const struct drm_connector_funcs *funcs,
--
1.8.1.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2] drm: use ida to allocate connector ids
2013-07-30 7:16 [PATCH] drm: use ida to allocate connector ids Ilia Mirkin
@ 2013-07-30 7:51 ` Ilia Mirkin
2013-08-07 0:12 ` Dave Airlie
2013-08-07 8:00 ` Ville Syrjälä
0 siblings, 2 replies; 11+ messages in thread
From: Ilia Mirkin @ 2013-07-30 7:51 UTC (permalink / raw)
To: David Airlie; +Cc: dri-devel
This makes it so that reloading a module does not cause all the
connector ids to change, which are user-visible and sometimes used
for configuration.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
---
v1 -> v2: correct loop condition... not sure how that slipped past
me... the code started out by being <= DRM...VIRTUAL, I guess
drivers/gpu/drm/drm_crtc.c | 61 +++++++++++++++++++++++++++++++---------------
drivers/gpu/drm/drm_drv.c | 2 ++
include/drm/drm_crtc.h | 2 ++
3 files changed, 46 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index fc83bb9..ed7599a 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -186,29 +186,29 @@ static const struct drm_prop_enum_list drm_dirty_info_enum_list[] = {
struct drm_conn_prop_enum_list {
int type;
const char *name;
- int count;
+ struct ida count;
};
/*
* Connector and encoder types.
*/
static struct drm_conn_prop_enum_list drm_connector_enum_list[] =
-{ { DRM_MODE_CONNECTOR_Unknown, "Unknown", 0 },
- { DRM_MODE_CONNECTOR_VGA, "VGA", 0 },
- { DRM_MODE_CONNECTOR_DVII, "DVI-I", 0 },
- { DRM_MODE_CONNECTOR_DVID, "DVI-D", 0 },
- { DRM_MODE_CONNECTOR_DVIA, "DVI-A", 0 },
- { DRM_MODE_CONNECTOR_Composite, "Composite", 0 },
- { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO", 0 },
- { DRM_MODE_CONNECTOR_LVDS, "LVDS", 0 },
- { DRM_MODE_CONNECTOR_Component, "Component", 0 },
- { DRM_MODE_CONNECTOR_9PinDIN, "DIN", 0 },
- { DRM_MODE_CONNECTOR_DisplayPort, "DP", 0 },
- { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A", 0 },
- { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B", 0 },
- { DRM_MODE_CONNECTOR_TV, "TV", 0 },
- { DRM_MODE_CONNECTOR_eDP, "eDP", 0 },
- { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual", 0},
+{ { DRM_MODE_CONNECTOR_Unknown, "Unknown" },
+ { DRM_MODE_CONNECTOR_VGA, "VGA" },
+ { DRM_MODE_CONNECTOR_DVII, "DVI-I" },
+ { DRM_MODE_CONNECTOR_DVID, "DVI-D" },
+ { DRM_MODE_CONNECTOR_DVIA, "DVI-A" },
+ { DRM_MODE_CONNECTOR_Composite, "Composite" },
+ { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO" },
+ { DRM_MODE_CONNECTOR_LVDS, "LVDS" },
+ { DRM_MODE_CONNECTOR_Component, "Component" },
+ { DRM_MODE_CONNECTOR_9PinDIN, "DIN" },
+ { DRM_MODE_CONNECTOR_DisplayPort, "DP" },
+ { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A" },
+ { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B" },
+ { DRM_MODE_CONNECTOR_TV, "TV" },
+ { DRM_MODE_CONNECTOR_eDP, "eDP" },
+ { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
};
static const struct drm_prop_enum_list drm_encoder_enum_list[] =
@@ -220,6 +220,22 @@ static const struct drm_prop_enum_list drm_encoder_enum_list[] =
{ DRM_MODE_ENCODER_VIRTUAL, "Virtual" },
};
+void drm_connector_ida_init(void)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
+ ida_init(&drm_connector_enum_list[i].count);
+}
+
+void drm_connector_ida_destroy(void)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
+ ida_destroy(&drm_connector_enum_list[i].count);
+}
+
const char *drm_get_encoder_name(const struct drm_encoder *encoder)
{
static char buf[32];
@@ -711,6 +727,9 @@ int drm_connector_init(struct drm_device *dev,
int connector_type)
{
int ret;
+ struct ida *count = &drm_connector_enum_list[connector_type].count;
+
+ ida_pre_get(count, GFP_KERNEL);
drm_modeset_lock_all(dev);
@@ -722,8 +741,9 @@ int drm_connector_init(struct drm_device *dev,
connector->dev = dev;
connector->funcs = funcs;
connector->connector_type = connector_type;
- connector->connector_type_id =
- ++drm_connector_enum_list[connector_type].count; /* TODO */
+ ret = ida_get_new_above(count, 1, &connector->connector_type_id);
+ if (ret)
+ goto out;
INIT_LIST_HEAD(&connector->probed_modes);
INIT_LIST_HEAD(&connector->modes);
connector->edid_blob_ptr = NULL;
@@ -764,6 +784,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
list_for_each_entry_safe(mode, t, &connector->modes, head)
drm_mode_remove(connector, mode);
+ ida_remove(&drm_connector_enum_list[connector->connector_type].count,
+ connector->connector_type_id);
+
drm_mode_object_put(dev, &connector->base);
list_del(&connector->head);
dev->mode_config.num_connector--;
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 99fcd7c..00597a1 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -251,6 +251,7 @@ static int __init drm_core_init(void)
int ret = -ENOMEM;
drm_global_init();
+ drm_connector_ida_init();
idr_init(&drm_minors_idr);
if (register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops))
@@ -298,6 +299,7 @@ static void __exit drm_core_exit(void)
unregister_chrdev(DRM_MAJOR, "drm");
+ drm_connector_ida_destroy();
idr_destroy(&drm_minors_idr);
}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index fa12a2f..effee9d 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -869,6 +869,8 @@ extern int drm_crtc_init(struct drm_device *dev,
const struct drm_crtc_funcs *funcs);
extern void drm_crtc_cleanup(struct drm_crtc *crtc);
+extern void drm_connector_ida_init(void);
+extern void drm_connector_ida_destroy(void);
extern int drm_connector_init(struct drm_device *dev,
struct drm_connector *connector,
const struct drm_connector_funcs *funcs,
--
1.8.1.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: use ida to allocate connector ids
2013-07-30 7:51 ` [PATCH v2] " Ilia Mirkin
@ 2013-08-07 0:12 ` Dave Airlie
2013-08-07 0:40 ` Rob Clark
2013-08-07 8:00 ` Ville Syrjälä
1 sibling, 1 reply; 11+ messages in thread
From: Dave Airlie @ 2013-08-07 0:12 UTC (permalink / raw)
To: Ilia Mirkin; +Cc: dri-devel
On Tue, Jul 30, 2013 at 5:51 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> This makes it so that reloading a module does not cause all the
> connector ids to change, which are user-visible and sometimes used
> for configuration.
I like this, anyone else want to comment/review?
Dave.
>
> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> ---
>
> v1 -> v2: correct loop condition... not sure how that slipped past
> me... the code started out by being <= DRM...VIRTUAL, I guess
>
> drivers/gpu/drm/drm_crtc.c | 61 +++++++++++++++++++++++++++++++---------------
> drivers/gpu/drm/drm_drv.c | 2 ++
> include/drm/drm_crtc.h | 2 ++
> 3 files changed, 46 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index fc83bb9..ed7599a 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -186,29 +186,29 @@ static const struct drm_prop_enum_list drm_dirty_info_enum_list[] = {
> struct drm_conn_prop_enum_list {
> int type;
> const char *name;
> - int count;
> + struct ida count;
> };
>
> /*
> * Connector and encoder types.
> */
> static struct drm_conn_prop_enum_list drm_connector_enum_list[] =
> -{ { DRM_MODE_CONNECTOR_Unknown, "Unknown", 0 },
> - { DRM_MODE_CONNECTOR_VGA, "VGA", 0 },
> - { DRM_MODE_CONNECTOR_DVII, "DVI-I", 0 },
> - { DRM_MODE_CONNECTOR_DVID, "DVI-D", 0 },
> - { DRM_MODE_CONNECTOR_DVIA, "DVI-A", 0 },
> - { DRM_MODE_CONNECTOR_Composite, "Composite", 0 },
> - { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO", 0 },
> - { DRM_MODE_CONNECTOR_LVDS, "LVDS", 0 },
> - { DRM_MODE_CONNECTOR_Component, "Component", 0 },
> - { DRM_MODE_CONNECTOR_9PinDIN, "DIN", 0 },
> - { DRM_MODE_CONNECTOR_DisplayPort, "DP", 0 },
> - { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A", 0 },
> - { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B", 0 },
> - { DRM_MODE_CONNECTOR_TV, "TV", 0 },
> - { DRM_MODE_CONNECTOR_eDP, "eDP", 0 },
> - { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual", 0},
> +{ { DRM_MODE_CONNECTOR_Unknown, "Unknown" },
> + { DRM_MODE_CONNECTOR_VGA, "VGA" },
> + { DRM_MODE_CONNECTOR_DVII, "DVI-I" },
> + { DRM_MODE_CONNECTOR_DVID, "DVI-D" },
> + { DRM_MODE_CONNECTOR_DVIA, "DVI-A" },
> + { DRM_MODE_CONNECTOR_Composite, "Composite" },
> + { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO" },
> + { DRM_MODE_CONNECTOR_LVDS, "LVDS" },
> + { DRM_MODE_CONNECTOR_Component, "Component" },
> + { DRM_MODE_CONNECTOR_9PinDIN, "DIN" },
> + { DRM_MODE_CONNECTOR_DisplayPort, "DP" },
> + { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A" },
> + { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B" },
> + { DRM_MODE_CONNECTOR_TV, "TV" },
> + { DRM_MODE_CONNECTOR_eDP, "eDP" },
> + { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
> };
>
> static const struct drm_prop_enum_list drm_encoder_enum_list[] =
> @@ -220,6 +220,22 @@ static const struct drm_prop_enum_list drm_encoder_enum_list[] =
> { DRM_MODE_ENCODER_VIRTUAL, "Virtual" },
> };
>
> +void drm_connector_ida_init(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
> + ida_init(&drm_connector_enum_list[i].count);
> +}
> +
> +void drm_connector_ida_destroy(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
> + ida_destroy(&drm_connector_enum_list[i].count);
> +}
> +
> const char *drm_get_encoder_name(const struct drm_encoder *encoder)
> {
> static char buf[32];
> @@ -711,6 +727,9 @@ int drm_connector_init(struct drm_device *dev,
> int connector_type)
> {
> int ret;
> + struct ida *count = &drm_connector_enum_list[connector_type].count;
> +
> + ida_pre_get(count, GFP_KERNEL);
>
> drm_modeset_lock_all(dev);
>
> @@ -722,8 +741,9 @@ int drm_connector_init(struct drm_device *dev,
> connector->dev = dev;
> connector->funcs = funcs;
> connector->connector_type = connector_type;
> - connector->connector_type_id =
> - ++drm_connector_enum_list[connector_type].count; /* TODO */
> + ret = ida_get_new_above(count, 1, &connector->connector_type_id);
> + if (ret)
> + goto out;
> INIT_LIST_HEAD(&connector->probed_modes);
> INIT_LIST_HEAD(&connector->modes);
> connector->edid_blob_ptr = NULL;
> @@ -764,6 +784,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
> list_for_each_entry_safe(mode, t, &connector->modes, head)
> drm_mode_remove(connector, mode);
>
> + ida_remove(&drm_connector_enum_list[connector->connector_type].count,
> + connector->connector_type_id);
> +
> drm_mode_object_put(dev, &connector->base);
> list_del(&connector->head);
> dev->mode_config.num_connector--;
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 99fcd7c..00597a1 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -251,6 +251,7 @@ static int __init drm_core_init(void)
> int ret = -ENOMEM;
>
> drm_global_init();
> + drm_connector_ida_init();
> idr_init(&drm_minors_idr);
>
> if (register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops))
> @@ -298,6 +299,7 @@ static void __exit drm_core_exit(void)
>
> unregister_chrdev(DRM_MAJOR, "drm");
>
> + drm_connector_ida_destroy();
> idr_destroy(&drm_minors_idr);
> }
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index fa12a2f..effee9d 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -869,6 +869,8 @@ extern int drm_crtc_init(struct drm_device *dev,
> const struct drm_crtc_funcs *funcs);
> extern void drm_crtc_cleanup(struct drm_crtc *crtc);
>
> +extern void drm_connector_ida_init(void);
> +extern void drm_connector_ida_destroy(void);
> extern int drm_connector_init(struct drm_device *dev,
> struct drm_connector *connector,
> const struct drm_connector_funcs *funcs,
> --
> 1.8.1.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: use ida to allocate connector ids
2013-08-07 0:12 ` Dave Airlie
@ 2013-08-07 0:40 ` Rob Clark
2013-08-07 1:04 ` Ilia Mirkin
0 siblings, 1 reply; 11+ messages in thread
From: Rob Clark @ 2013-08-07 0:40 UTC (permalink / raw)
To: Dave Airlie; +Cc: dri-devel
On Tue, Aug 6, 2013 at 8:12 PM, Dave Airlie <airlied@gmail.com> wrote:
> On Tue, Jul 30, 2013 at 5:51 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>> This makes it so that reloading a module does not cause all the
>> connector ids to change, which are user-visible and sometimes used
>> for configuration.
>
> I like this, anyone else want to comment/review?
looks nice.. not sure I actually want to *encourage* someone to rely
on stable connector id's (since it can change from kernel version to
kernel version, etc), but otoh it would make life nicer for modetest
users (since it isn't clever enough to use connector-names)
BR,
-R
> Dave.
>
>>
>> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
>> ---
>>
>> v1 -> v2: correct loop condition... not sure how that slipped past
>> me... the code started out by being <= DRM...VIRTUAL, I guess
>>
>> drivers/gpu/drm/drm_crtc.c | 61 +++++++++++++++++++++++++++++++---------------
>> drivers/gpu/drm/drm_drv.c | 2 ++
>> include/drm/drm_crtc.h | 2 ++
>> 3 files changed, 46 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index fc83bb9..ed7599a 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -186,29 +186,29 @@ static const struct drm_prop_enum_list drm_dirty_info_enum_list[] = {
>> struct drm_conn_prop_enum_list {
>> int type;
>> const char *name;
>> - int count;
>> + struct ida count;
>> };
>>
>> /*
>> * Connector and encoder types.
>> */
>> static struct drm_conn_prop_enum_list drm_connector_enum_list[] =
>> -{ { DRM_MODE_CONNECTOR_Unknown, "Unknown", 0 },
>> - { DRM_MODE_CONNECTOR_VGA, "VGA", 0 },
>> - { DRM_MODE_CONNECTOR_DVII, "DVI-I", 0 },
>> - { DRM_MODE_CONNECTOR_DVID, "DVI-D", 0 },
>> - { DRM_MODE_CONNECTOR_DVIA, "DVI-A", 0 },
>> - { DRM_MODE_CONNECTOR_Composite, "Composite", 0 },
>> - { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO", 0 },
>> - { DRM_MODE_CONNECTOR_LVDS, "LVDS", 0 },
>> - { DRM_MODE_CONNECTOR_Component, "Component", 0 },
>> - { DRM_MODE_CONNECTOR_9PinDIN, "DIN", 0 },
>> - { DRM_MODE_CONNECTOR_DisplayPort, "DP", 0 },
>> - { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A", 0 },
>> - { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B", 0 },
>> - { DRM_MODE_CONNECTOR_TV, "TV", 0 },
>> - { DRM_MODE_CONNECTOR_eDP, "eDP", 0 },
>> - { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual", 0},
>> +{ { DRM_MODE_CONNECTOR_Unknown, "Unknown" },
>> + { DRM_MODE_CONNECTOR_VGA, "VGA" },
>> + { DRM_MODE_CONNECTOR_DVII, "DVI-I" },
>> + { DRM_MODE_CONNECTOR_DVID, "DVI-D" },
>> + { DRM_MODE_CONNECTOR_DVIA, "DVI-A" },
>> + { DRM_MODE_CONNECTOR_Composite, "Composite" },
>> + { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO" },
>> + { DRM_MODE_CONNECTOR_LVDS, "LVDS" },
>> + { DRM_MODE_CONNECTOR_Component, "Component" },
>> + { DRM_MODE_CONNECTOR_9PinDIN, "DIN" },
>> + { DRM_MODE_CONNECTOR_DisplayPort, "DP" },
>> + { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A" },
>> + { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B" },
>> + { DRM_MODE_CONNECTOR_TV, "TV" },
>> + { DRM_MODE_CONNECTOR_eDP, "eDP" },
>> + { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
>> };
>>
>> static const struct drm_prop_enum_list drm_encoder_enum_list[] =
>> @@ -220,6 +220,22 @@ static const struct drm_prop_enum_list drm_encoder_enum_list[] =
>> { DRM_MODE_ENCODER_VIRTUAL, "Virtual" },
>> };
>>
>> +void drm_connector_ida_init(void)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
>> + ida_init(&drm_connector_enum_list[i].count);
>> +}
>> +
>> +void drm_connector_ida_destroy(void)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
>> + ida_destroy(&drm_connector_enum_list[i].count);
>> +}
>> +
>> const char *drm_get_encoder_name(const struct drm_encoder *encoder)
>> {
>> static char buf[32];
>> @@ -711,6 +727,9 @@ int drm_connector_init(struct drm_device *dev,
>> int connector_type)
>> {
>> int ret;
>> + struct ida *count = &drm_connector_enum_list[connector_type].count;
>> +
>> + ida_pre_get(count, GFP_KERNEL);
>>
>> drm_modeset_lock_all(dev);
>>
>> @@ -722,8 +741,9 @@ int drm_connector_init(struct drm_device *dev,
>> connector->dev = dev;
>> connector->funcs = funcs;
>> connector->connector_type = connector_type;
>> - connector->connector_type_id =
>> - ++drm_connector_enum_list[connector_type].count; /* TODO */
>> + ret = ida_get_new_above(count, 1, &connector->connector_type_id);
>> + if (ret)
>> + goto out;
>> INIT_LIST_HEAD(&connector->probed_modes);
>> INIT_LIST_HEAD(&connector->modes);
>> connector->edid_blob_ptr = NULL;
>> @@ -764,6 +784,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
>> list_for_each_entry_safe(mode, t, &connector->modes, head)
>> drm_mode_remove(connector, mode);
>>
>> + ida_remove(&drm_connector_enum_list[connector->connector_type].count,
>> + connector->connector_type_id);
>> +
>> drm_mode_object_put(dev, &connector->base);
>> list_del(&connector->head);
>> dev->mode_config.num_connector--;
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 99fcd7c..00597a1 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -251,6 +251,7 @@ static int __init drm_core_init(void)
>> int ret = -ENOMEM;
>>
>> drm_global_init();
>> + drm_connector_ida_init();
>> idr_init(&drm_minors_idr);
>>
>> if (register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops))
>> @@ -298,6 +299,7 @@ static void __exit drm_core_exit(void)
>>
>> unregister_chrdev(DRM_MAJOR, "drm");
>>
>> + drm_connector_ida_destroy();
>> idr_destroy(&drm_minors_idr);
>> }
>>
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index fa12a2f..effee9d 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -869,6 +869,8 @@ extern int drm_crtc_init(struct drm_device *dev,
>> const struct drm_crtc_funcs *funcs);
>> extern void drm_crtc_cleanup(struct drm_crtc *crtc);
>>
>> +extern void drm_connector_ida_init(void);
>> +extern void drm_connector_ida_destroy(void);
>> extern int drm_connector_init(struct drm_device *dev,
>> struct drm_connector *connector,
>> const struct drm_connector_funcs *funcs,
>> --
>> 1.8.1.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: use ida to allocate connector ids
2013-08-07 0:40 ` Rob Clark
@ 2013-08-07 1:04 ` Ilia Mirkin
0 siblings, 0 replies; 11+ messages in thread
From: Ilia Mirkin @ 2013-08-07 1:04 UTC (permalink / raw)
To: Rob Clark; +Cc: dri-devel
On Tue, Aug 6, 2013 at 8:40 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Tue, Aug 6, 2013 at 8:12 PM, Dave Airlie <airlied@gmail.com> wrote:
>> On Tue, Jul 30, 2013 at 5:51 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>> This makes it so that reloading a module does not cause all the
>>> connector ids to change, which are user-visible and sometimes used
>>> for configuration.
>>
>> I like this, anyone else want to comment/review?
>
> looks nice.. not sure I actually want to *encourage* someone to rely
> on stable connector id's (since it can change from kernel version to
> kernel version, etc), but otoh it would make life nicer for modetest
> users (since it isn't clever enough to use connector-names)
My personal use-case is running xrandr --output DVI-I-1 --brightness
.8 every time. Really nice if I can just ^Rxr and be done with it.
[Would be even nicer if I could stick it in the X config, but I
digress.] Also e.g. for Zaphod heads, you specify the connector names
in the X config (although I don't personally use that).
-ilia
>
> BR,
> -R
>
>> Dave.
>>
>>>
>>> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
>>> ---
>>>
>>> v1 -> v2: correct loop condition... not sure how that slipped past
>>> me... the code started out by being <= DRM...VIRTUAL, I guess
>>>
>>> drivers/gpu/drm/drm_crtc.c | 61 +++++++++++++++++++++++++++++++---------------
>>> drivers/gpu/drm/drm_drv.c | 2 ++
>>> include/drm/drm_crtc.h | 2 ++
>>> 3 files changed, 46 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>>> index fc83bb9..ed7599a 100644
>>> --- a/drivers/gpu/drm/drm_crtc.c
>>> +++ b/drivers/gpu/drm/drm_crtc.c
>>> @@ -186,29 +186,29 @@ static const struct drm_prop_enum_list drm_dirty_info_enum_list[] = {
>>> struct drm_conn_prop_enum_list {
>>> int type;
>>> const char *name;
>>> - int count;
>>> + struct ida count;
>>> };
>>>
>>> /*
>>> * Connector and encoder types.
>>> */
>>> static struct drm_conn_prop_enum_list drm_connector_enum_list[] =
>>> -{ { DRM_MODE_CONNECTOR_Unknown, "Unknown", 0 },
>>> - { DRM_MODE_CONNECTOR_VGA, "VGA", 0 },
>>> - { DRM_MODE_CONNECTOR_DVII, "DVI-I", 0 },
>>> - { DRM_MODE_CONNECTOR_DVID, "DVI-D", 0 },
>>> - { DRM_MODE_CONNECTOR_DVIA, "DVI-A", 0 },
>>> - { DRM_MODE_CONNECTOR_Composite, "Composite", 0 },
>>> - { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO", 0 },
>>> - { DRM_MODE_CONNECTOR_LVDS, "LVDS", 0 },
>>> - { DRM_MODE_CONNECTOR_Component, "Component", 0 },
>>> - { DRM_MODE_CONNECTOR_9PinDIN, "DIN", 0 },
>>> - { DRM_MODE_CONNECTOR_DisplayPort, "DP", 0 },
>>> - { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A", 0 },
>>> - { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B", 0 },
>>> - { DRM_MODE_CONNECTOR_TV, "TV", 0 },
>>> - { DRM_MODE_CONNECTOR_eDP, "eDP", 0 },
>>> - { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual", 0},
>>> +{ { DRM_MODE_CONNECTOR_Unknown, "Unknown" },
>>> + { DRM_MODE_CONNECTOR_VGA, "VGA" },
>>> + { DRM_MODE_CONNECTOR_DVII, "DVI-I" },
>>> + { DRM_MODE_CONNECTOR_DVID, "DVI-D" },
>>> + { DRM_MODE_CONNECTOR_DVIA, "DVI-A" },
>>> + { DRM_MODE_CONNECTOR_Composite, "Composite" },
>>> + { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO" },
>>> + { DRM_MODE_CONNECTOR_LVDS, "LVDS" },
>>> + { DRM_MODE_CONNECTOR_Component, "Component" },
>>> + { DRM_MODE_CONNECTOR_9PinDIN, "DIN" },
>>> + { DRM_MODE_CONNECTOR_DisplayPort, "DP" },
>>> + { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A" },
>>> + { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B" },
>>> + { DRM_MODE_CONNECTOR_TV, "TV" },
>>> + { DRM_MODE_CONNECTOR_eDP, "eDP" },
>>> + { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
>>> };
>>>
>>> static const struct drm_prop_enum_list drm_encoder_enum_list[] =
>>> @@ -220,6 +220,22 @@ static const struct drm_prop_enum_list drm_encoder_enum_list[] =
>>> { DRM_MODE_ENCODER_VIRTUAL, "Virtual" },
>>> };
>>>
>>> +void drm_connector_ida_init(void)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
>>> + ida_init(&drm_connector_enum_list[i].count);
>>> +}
>>> +
>>> +void drm_connector_ida_destroy(void)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
>>> + ida_destroy(&drm_connector_enum_list[i].count);
>>> +}
>>> +
>>> const char *drm_get_encoder_name(const struct drm_encoder *encoder)
>>> {
>>> static char buf[32];
>>> @@ -711,6 +727,9 @@ int drm_connector_init(struct drm_device *dev,
>>> int connector_type)
>>> {
>>> int ret;
>>> + struct ida *count = &drm_connector_enum_list[connector_type].count;
>>> +
>>> + ida_pre_get(count, GFP_KERNEL);
>>>
>>> drm_modeset_lock_all(dev);
>>>
>>> @@ -722,8 +741,9 @@ int drm_connector_init(struct drm_device *dev,
>>> connector->dev = dev;
>>> connector->funcs = funcs;
>>> connector->connector_type = connector_type;
>>> - connector->connector_type_id =
>>> - ++drm_connector_enum_list[connector_type].count; /* TODO */
>>> + ret = ida_get_new_above(count, 1, &connector->connector_type_id);
>>> + if (ret)
>>> + goto out;
>>> INIT_LIST_HEAD(&connector->probed_modes);
>>> INIT_LIST_HEAD(&connector->modes);
>>> connector->edid_blob_ptr = NULL;
>>> @@ -764,6 +784,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
>>> list_for_each_entry_safe(mode, t, &connector->modes, head)
>>> drm_mode_remove(connector, mode);
>>>
>>> + ida_remove(&drm_connector_enum_list[connector->connector_type].count,
>>> + connector->connector_type_id);
>>> +
>>> drm_mode_object_put(dev, &connector->base);
>>> list_del(&connector->head);
>>> dev->mode_config.num_connector--;
>>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>>> index 99fcd7c..00597a1 100644
>>> --- a/drivers/gpu/drm/drm_drv.c
>>> +++ b/drivers/gpu/drm/drm_drv.c
>>> @@ -251,6 +251,7 @@ static int __init drm_core_init(void)
>>> int ret = -ENOMEM;
>>>
>>> drm_global_init();
>>> + drm_connector_ida_init();
>>> idr_init(&drm_minors_idr);
>>>
>>> if (register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops))
>>> @@ -298,6 +299,7 @@ static void __exit drm_core_exit(void)
>>>
>>> unregister_chrdev(DRM_MAJOR, "drm");
>>>
>>> + drm_connector_ida_destroy();
>>> idr_destroy(&drm_minors_idr);
>>> }
>>>
>>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>>> index fa12a2f..effee9d 100644
>>> --- a/include/drm/drm_crtc.h
>>> +++ b/include/drm/drm_crtc.h
>>> @@ -869,6 +869,8 @@ extern int drm_crtc_init(struct drm_device *dev,
>>> const struct drm_crtc_funcs *funcs);
>>> extern void drm_crtc_cleanup(struct drm_crtc *crtc);
>>>
>>> +extern void drm_connector_ida_init(void);
>>> +extern void drm_connector_ida_destroy(void);
>>> extern int drm_connector_init(struct drm_device *dev,
>>> struct drm_connector *connector,
>>> const struct drm_connector_funcs *funcs,
>>> --
>>> 1.8.1.5
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: use ida to allocate connector ids
2013-07-30 7:51 ` [PATCH v2] " Ilia Mirkin
2013-08-07 0:12 ` Dave Airlie
@ 2013-08-07 8:00 ` Ville Syrjälä
2013-08-07 8:28 ` Ilia Mirkin
2013-08-08 2:34 ` [PATCH v3] " Ilia Mirkin
1 sibling, 2 replies; 11+ messages in thread
From: Ville Syrjälä @ 2013-08-07 8:00 UTC (permalink / raw)
To: Ilia Mirkin; +Cc: dri-devel
On Tue, Jul 30, 2013 at 03:51:49AM -0400, Ilia Mirkin wrote:
> This makes it so that reloading a module does not cause all the
> connector ids to change, which are user-visible and sometimes used
> for configuration.
>
> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> ---
>
> v1 -> v2: correct loop condition... not sure how that slipped past
> me... the code started out by being <= DRM...VIRTUAL, I guess
>
> drivers/gpu/drm/drm_crtc.c | 61 +++++++++++++++++++++++++++++++---------------
> drivers/gpu/drm/drm_drv.c | 2 ++
> include/drm/drm_crtc.h | 2 ++
> 3 files changed, 46 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index fc83bb9..ed7599a 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -186,29 +186,29 @@ static const struct drm_prop_enum_list drm_dirty_info_enum_list[] = {
> struct drm_conn_prop_enum_list {
> int type;
> const char *name;
> - int count;
> + struct ida count;
I'd rename this to something else. 'count' is just confusing.
> };
>
> /*
> * Connector and encoder types.
> */
> static struct drm_conn_prop_enum_list drm_connector_enum_list[] =
> -{ { DRM_MODE_CONNECTOR_Unknown, "Unknown", 0 },
> - { DRM_MODE_CONNECTOR_VGA, "VGA", 0 },
> - { DRM_MODE_CONNECTOR_DVII, "DVI-I", 0 },
> - { DRM_MODE_CONNECTOR_DVID, "DVI-D", 0 },
> - { DRM_MODE_CONNECTOR_DVIA, "DVI-A", 0 },
> - { DRM_MODE_CONNECTOR_Composite, "Composite", 0 },
> - { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO", 0 },
> - { DRM_MODE_CONNECTOR_LVDS, "LVDS", 0 },
> - { DRM_MODE_CONNECTOR_Component, "Component", 0 },
> - { DRM_MODE_CONNECTOR_9PinDIN, "DIN", 0 },
> - { DRM_MODE_CONNECTOR_DisplayPort, "DP", 0 },
> - { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A", 0 },
> - { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B", 0 },
> - { DRM_MODE_CONNECTOR_TV, "TV", 0 },
> - { DRM_MODE_CONNECTOR_eDP, "eDP", 0 },
> - { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual", 0},
> +{ { DRM_MODE_CONNECTOR_Unknown, "Unknown" },
> + { DRM_MODE_CONNECTOR_VGA, "VGA" },
> + { DRM_MODE_CONNECTOR_DVII, "DVI-I" },
> + { DRM_MODE_CONNECTOR_DVID, "DVI-D" },
> + { DRM_MODE_CONNECTOR_DVIA, "DVI-A" },
> + { DRM_MODE_CONNECTOR_Composite, "Composite" },
> + { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO" },
> + { DRM_MODE_CONNECTOR_LVDS, "LVDS" },
> + { DRM_MODE_CONNECTOR_Component, "Component" },
> + { DRM_MODE_CONNECTOR_9PinDIN, "DIN" },
> + { DRM_MODE_CONNECTOR_DisplayPort, "DP" },
> + { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A" },
> + { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B" },
> + { DRM_MODE_CONNECTOR_TV, "TV" },
> + { DRM_MODE_CONNECTOR_eDP, "eDP" },
> + { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
> };
>
> static const struct drm_prop_enum_list drm_encoder_enum_list[] =
> @@ -220,6 +220,22 @@ static const struct drm_prop_enum_list drm_encoder_enum_list[] =
> { DRM_MODE_ENCODER_VIRTUAL, "Virtual" },
> };
>
> +void drm_connector_ida_init(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
> + ida_init(&drm_connector_enum_list[i].count);
> +}
> +
> +void drm_connector_ida_destroy(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
> + ida_destroy(&drm_connector_enum_list[i].count);
> +}
> +
> const char *drm_get_encoder_name(const struct drm_encoder *encoder)
> {
> static char buf[32];
> @@ -711,6 +727,9 @@ int drm_connector_init(struct drm_device *dev,
> int connector_type)
> {
> int ret;
> + struct ida *count = &drm_connector_enum_list[connector_type].count;
> +
> + ida_pre_get(count, GFP_KERNEL);
>
> drm_modeset_lock_all(dev);
>
> @@ -722,8 +741,9 @@ int drm_connector_init(struct drm_device *dev,
> connector->dev = dev;
> connector->funcs = funcs;
> connector->connector_type = connector_type;
> - connector->connector_type_id =
> - ++drm_connector_enum_list[connector_type].count; /* TODO */
> + ret = ida_get_new_above(count, 1, &connector->connector_type_id);
> + if (ret)
> + goto out;
Leak.
Also there's no retry loop w/ ida_pre_get() and since that's outside the
big kms lock, there could be a (small) chance that someone else already
consumed the allocation done in ida_pre_get(). And then we'll return
-EAGAIN which is a rather confusing error from an init function. I
guess you could just move the ida_pre_get() inside the kms lock and
avoid that race.
> INIT_LIST_HEAD(&connector->probed_modes);
> INIT_LIST_HEAD(&connector->modes);
> connector->edid_blob_ptr = NULL;
> @@ -764,6 +784,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
> list_for_each_entry_safe(mode, t, &connector->modes, head)
> drm_mode_remove(connector, mode);
>
> + ida_remove(&drm_connector_enum_list[connector->connector_type].count,
> + connector->connector_type_id);
> +
> drm_mode_object_put(dev, &connector->base);
> list_del(&connector->head);
> dev->mode_config.num_connector--;
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 99fcd7c..00597a1 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -251,6 +251,7 @@ static int __init drm_core_init(void)
> int ret = -ENOMEM;
>
> drm_global_init();
> + drm_connector_ida_init();
> idr_init(&drm_minors_idr);
>
> if (register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops))
> @@ -298,6 +299,7 @@ static void __exit drm_core_exit(void)
>
> unregister_chrdev(DRM_MAJOR, "drm");
>
> + drm_connector_ida_destroy();
> idr_destroy(&drm_minors_idr);
> }
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index fa12a2f..effee9d 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -869,6 +869,8 @@ extern int drm_crtc_init(struct drm_device *dev,
> const struct drm_crtc_funcs *funcs);
> extern void drm_crtc_cleanup(struct drm_crtc *crtc);
>
> +extern void drm_connector_ida_init(void);
> +extern void drm_connector_ida_destroy(void);
> extern int drm_connector_init(struct drm_device *dev,
> struct drm_connector *connector,
> const struct drm_connector_funcs *funcs,
> --
> 1.8.1.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: use ida to allocate connector ids
2013-08-07 8:00 ` Ville Syrjälä
@ 2013-08-07 8:28 ` Ilia Mirkin
2013-08-07 8:40 ` Daniel Vetter
2013-08-07 10:47 ` Ville Syrjälä
2013-08-08 2:34 ` [PATCH v3] " Ilia Mirkin
1 sibling, 2 replies; 11+ messages in thread
From: Ilia Mirkin @ 2013-08-07 8:28 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: dri-devel
On Wed, Aug 7, 2013 at 4:00 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Tue, Jul 30, 2013 at 03:51:49AM -0400, Ilia Mirkin wrote:
>> This makes it so that reloading a module does not cause all the
>> connector ids to change, which are user-visible and sometimes used
>> for configuration.
>>
>> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
>> ---
>>
>> v1 -> v2: correct loop condition... not sure how that slipped past
>> me... the code started out by being <= DRM...VIRTUAL, I guess
>>
>> drivers/gpu/drm/drm_crtc.c | 61 +++++++++++++++++++++++++++++++---------------
>> drivers/gpu/drm/drm_drv.c | 2 ++
>> include/drm/drm_crtc.h | 2 ++
>> 3 files changed, 46 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index fc83bb9..ed7599a 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -186,29 +186,29 @@ static const struct drm_prop_enum_list drm_dirty_info_enum_list[] = {
>> struct drm_conn_prop_enum_list {
>> int type;
>> const char *name;
>> - int count;
>> + struct ida count;
>
> I'd rename this to something else. 'count' is just confusing.
>
>> };
>>
>> /*
>> * Connector and encoder types.
>> */
>> static struct drm_conn_prop_enum_list drm_connector_enum_list[] =
>> -{ { DRM_MODE_CONNECTOR_Unknown, "Unknown", 0 },
>> - { DRM_MODE_CONNECTOR_VGA, "VGA", 0 },
>> - { DRM_MODE_CONNECTOR_DVII, "DVI-I", 0 },
>> - { DRM_MODE_CONNECTOR_DVID, "DVI-D", 0 },
>> - { DRM_MODE_CONNECTOR_DVIA, "DVI-A", 0 },
>> - { DRM_MODE_CONNECTOR_Composite, "Composite", 0 },
>> - { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO", 0 },
>> - { DRM_MODE_CONNECTOR_LVDS, "LVDS", 0 },
>> - { DRM_MODE_CONNECTOR_Component, "Component", 0 },
>> - { DRM_MODE_CONNECTOR_9PinDIN, "DIN", 0 },
>> - { DRM_MODE_CONNECTOR_DisplayPort, "DP", 0 },
>> - { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A", 0 },
>> - { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B", 0 },
>> - { DRM_MODE_CONNECTOR_TV, "TV", 0 },
>> - { DRM_MODE_CONNECTOR_eDP, "eDP", 0 },
>> - { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual", 0},
>> +{ { DRM_MODE_CONNECTOR_Unknown, "Unknown" },
>> + { DRM_MODE_CONNECTOR_VGA, "VGA" },
>> + { DRM_MODE_CONNECTOR_DVII, "DVI-I" },
>> + { DRM_MODE_CONNECTOR_DVID, "DVI-D" },
>> + { DRM_MODE_CONNECTOR_DVIA, "DVI-A" },
>> + { DRM_MODE_CONNECTOR_Composite, "Composite" },
>> + { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO" },
>> + { DRM_MODE_CONNECTOR_LVDS, "LVDS" },
>> + { DRM_MODE_CONNECTOR_Component, "Component" },
>> + { DRM_MODE_CONNECTOR_9PinDIN, "DIN" },
>> + { DRM_MODE_CONNECTOR_DisplayPort, "DP" },
>> + { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A" },
>> + { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B" },
>> + { DRM_MODE_CONNECTOR_TV, "TV" },
>> + { DRM_MODE_CONNECTOR_eDP, "eDP" },
>> + { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
>> };
>>
>> static const struct drm_prop_enum_list drm_encoder_enum_list[] =
>> @@ -220,6 +220,22 @@ static const struct drm_prop_enum_list drm_encoder_enum_list[] =
>> { DRM_MODE_ENCODER_VIRTUAL, "Virtual" },
>> };
>>
>> +void drm_connector_ida_init(void)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
>> + ida_init(&drm_connector_enum_list[i].count);
>> +}
>> +
>> +void drm_connector_ida_destroy(void)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
>> + ida_destroy(&drm_connector_enum_list[i].count);
>> +}
>> +
>> const char *drm_get_encoder_name(const struct drm_encoder *encoder)
>> {
>> static char buf[32];
>> @@ -711,6 +727,9 @@ int drm_connector_init(struct drm_device *dev,
>> int connector_type)
>> {
>> int ret;
>> + struct ida *count = &drm_connector_enum_list[connector_type].count;
>> +
>> + ida_pre_get(count, GFP_KERNEL);
>>
>> drm_modeset_lock_all(dev);
>>
>> @@ -722,8 +741,9 @@ int drm_connector_init(struct drm_device *dev,
>> connector->dev = dev;
>> connector->funcs = funcs;
>> connector->connector_type = connector_type;
>> - connector->connector_type_id =
>> - ++drm_connector_enum_list[connector_type].count; /* TODO */
>> + ret = ida_get_new_above(count, 1, &connector->connector_type_id);
>> + if (ret)
>> + goto out;
>
> Leak.
I assume this is in reference to the drm_mode_object_get above? I just
noticed that... or was there something else too?
>
> Also there's no retry loop w/ ida_pre_get() and since that's outside the
> big kms lock, there could be a (small) chance that someone else already
> consumed the allocation done in ida_pre_get(). And then we'll return
> -EAGAIN which is a rather confusing error from an init function. I
> guess you could just move the ida_pre_get() inside the kms lock and
> avoid that race.
OK, I'm not really sure what the whole drm locking situation is. If
it's OK to do a GFP_KERNEL alloc inside the "big kms lock" (BKL v2?),
I might as well use the ida_simple_get interface.
>
>> INIT_LIST_HEAD(&connector->probed_modes);
>> INIT_LIST_HEAD(&connector->modes);
>> connector->edid_blob_ptr = NULL;
>> @@ -764,6 +784,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
>> list_for_each_entry_safe(mode, t, &connector->modes, head)
>> drm_mode_remove(connector, mode);
>>
>> + ida_remove(&drm_connector_enum_list[connector->connector_type].count,
>> + connector->connector_type_id);
>> +
>> drm_mode_object_put(dev, &connector->base);
>> list_del(&connector->head);
>> dev->mode_config.num_connector--;
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 99fcd7c..00597a1 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -251,6 +251,7 @@ static int __init drm_core_init(void)
>> int ret = -ENOMEM;
>>
>> drm_global_init();
>> + drm_connector_ida_init();
>> idr_init(&drm_minors_idr);
>>
>> if (register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops))
>> @@ -298,6 +299,7 @@ static void __exit drm_core_exit(void)
>>
>> unregister_chrdev(DRM_MAJOR, "drm");
>>
>> + drm_connector_ida_destroy();
>> idr_destroy(&drm_minors_idr);
>> }
>>
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index fa12a2f..effee9d 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -869,6 +869,8 @@ extern int drm_crtc_init(struct drm_device *dev,
>> const struct drm_crtc_funcs *funcs);
>> extern void drm_crtc_cleanup(struct drm_crtc *crtc);
>>
>> +extern void drm_connector_ida_init(void);
>> +extern void drm_connector_ida_destroy(void);
>> extern int drm_connector_init(struct drm_device *dev,
>> struct drm_connector *connector,
>> const struct drm_connector_funcs *funcs,
>> --
>> 1.8.1.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: use ida to allocate connector ids
2013-08-07 8:28 ` Ilia Mirkin
@ 2013-08-07 8:40 ` Daniel Vetter
2013-08-07 10:47 ` Ville Syrjälä
1 sibling, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2013-08-07 8:40 UTC (permalink / raw)
To: Ilia Mirkin; +Cc: dri-devel
On Wed, Aug 7, 2013 at 10:28 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>> Also there's no retry loop w/ ida_pre_get() and since that's outside the
>> big kms lock, there could be a (small) chance that someone else already
>> consumed the allocation done in ida_pre_get(). And then we'll return
>> -EAGAIN which is a rather confusing error from an init function. I
>> guess you could just move the ida_pre_get() inside the kms lock and
>> avoid that race.
>
> OK, I'm not really sure what the whole drm locking situation is. If
> it's OK to do a GFP_KERNEL alloc inside the "big kms lock" (BKL v2?),
> I might as well use the ida_simple_get interface.
The big kms lock is just a pile of nested mutexes which all may not be
held from shrinker callbacks and similar places. So you can allocate
GFP_KERNEL memory with impunity.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: use ida to allocate connector ids
2013-08-07 8:28 ` Ilia Mirkin
2013-08-07 8:40 ` Daniel Vetter
@ 2013-08-07 10:47 ` Ville Syrjälä
1 sibling, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2013-08-07 10:47 UTC (permalink / raw)
To: Ilia Mirkin; +Cc: dri-devel
On Wed, Aug 07, 2013 at 04:28:35AM -0400, Ilia Mirkin wrote:
> On Wed, Aug 7, 2013 at 4:00 AM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Tue, Jul 30, 2013 at 03:51:49AM -0400, Ilia Mirkin wrote:
> >> @@ -722,8 +741,9 @@ int drm_connector_init(struct drm_device *dev,
> >> connector->dev = dev;
> >> connector->funcs = funcs;
> >> connector->connector_type = connector_type;
> >> - connector->connector_type_id =
> >> - ++drm_connector_enum_list[connector_type].count; /* TODO */
> >> + ret = ida_get_new_above(count, 1, &connector->connector_type_id);
> >> + if (ret)
> >> + goto out;
> >
> > Leak.
>
> I assume this is in reference to the drm_mode_object_get above? I just
> noticed that... or was there something else too?
Just the obj id.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] drm: use ida to allocate connector ids
2013-08-07 8:00 ` Ville Syrjälä
2013-08-07 8:28 ` Ilia Mirkin
@ 2013-08-08 2:34 ` Ilia Mirkin
2013-08-08 8:18 ` Ville Syrjälä
1 sibling, 1 reply; 11+ messages in thread
From: Ilia Mirkin @ 2013-08-08 2:34 UTC (permalink / raw)
To: David Airlie; +Cc: dri-devel
This makes it so that reloading a module does not cause all the
connector ids to change, which are user-visible and sometimes used
for configuration.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
---
v2 -> v3:
- rename ida struct name to... ida. (i'm ready to accept my "creative name
of the year" award.)
- fix object id leak when allocation fails.
- use ida_simple_get, since it's OK to allocate memory inside kms lock
drivers/gpu/drm/drm_crtc.c | 62 ++++++++++++++++++++++++++++++++--------------
drivers/gpu/drm/drm_drv.c | 2 ++
include/drm/drm_crtc.h | 2 ++
3 files changed, 48 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index fc83bb9..a691764 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -186,29 +186,29 @@ static const struct drm_prop_enum_list drm_dirty_info_enum_list[] = {
struct drm_conn_prop_enum_list {
int type;
const char *name;
- int count;
+ struct ida ida;
};
/*
* Connector and encoder types.
*/
static struct drm_conn_prop_enum_list drm_connector_enum_list[] =
-{ { DRM_MODE_CONNECTOR_Unknown, "Unknown", 0 },
- { DRM_MODE_CONNECTOR_VGA, "VGA", 0 },
- { DRM_MODE_CONNECTOR_DVII, "DVI-I", 0 },
- { DRM_MODE_CONNECTOR_DVID, "DVI-D", 0 },
- { DRM_MODE_CONNECTOR_DVIA, "DVI-A", 0 },
- { DRM_MODE_CONNECTOR_Composite, "Composite", 0 },
- { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO", 0 },
- { DRM_MODE_CONNECTOR_LVDS, "LVDS", 0 },
- { DRM_MODE_CONNECTOR_Component, "Component", 0 },
- { DRM_MODE_CONNECTOR_9PinDIN, "DIN", 0 },
- { DRM_MODE_CONNECTOR_DisplayPort, "DP", 0 },
- { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A", 0 },
- { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B", 0 },
- { DRM_MODE_CONNECTOR_TV, "TV", 0 },
- { DRM_MODE_CONNECTOR_eDP, "eDP", 0 },
- { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual", 0},
+{ { DRM_MODE_CONNECTOR_Unknown, "Unknown" },
+ { DRM_MODE_CONNECTOR_VGA, "VGA" },
+ { DRM_MODE_CONNECTOR_DVII, "DVI-I" },
+ { DRM_MODE_CONNECTOR_DVID, "DVI-D" },
+ { DRM_MODE_CONNECTOR_DVIA, "DVI-A" },
+ { DRM_MODE_CONNECTOR_Composite, "Composite" },
+ { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO" },
+ { DRM_MODE_CONNECTOR_LVDS, "LVDS" },
+ { DRM_MODE_CONNECTOR_Component, "Component" },
+ { DRM_MODE_CONNECTOR_9PinDIN, "DIN" },
+ { DRM_MODE_CONNECTOR_DisplayPort, "DP" },
+ { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A" },
+ { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B" },
+ { DRM_MODE_CONNECTOR_TV, "TV" },
+ { DRM_MODE_CONNECTOR_eDP, "eDP" },
+ { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
};
static const struct drm_prop_enum_list drm_encoder_enum_list[] =
@@ -220,6 +220,22 @@ static const struct drm_prop_enum_list drm_encoder_enum_list[] =
{ DRM_MODE_ENCODER_VIRTUAL, "Virtual" },
};
+void drm_connector_ida_init(void)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
+ ida_init(&drm_connector_enum_list[i].ida);
+}
+
+void drm_connector_ida_destroy(void)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
+ ida_destroy(&drm_connector_enum_list[i].ida);
+}
+
const char *drm_get_encoder_name(const struct drm_encoder *encoder)
{
static char buf[32];
@@ -711,6 +727,8 @@ int drm_connector_init(struct drm_device *dev,
int connector_type)
{
int ret;
+ struct ida *connector_ida =
+ &drm_connector_enum_list[connector_type].ida;
drm_modeset_lock_all(dev);
@@ -723,7 +741,12 @@ int drm_connector_init(struct drm_device *dev,
connector->funcs = funcs;
connector->connector_type = connector_type;
connector->connector_type_id =
- ++drm_connector_enum_list[connector_type].count; /* TODO */
+ ida_simple_get(connector_ida, 1, 0, GFP_KERNEL);
+ if (connector->connector_type_id < 0) {
+ ret = connector->connector_type_id;
+ drm_mode_object_put(dev, &connector->base);
+ goto out;
+ }
INIT_LIST_HEAD(&connector->probed_modes);
INIT_LIST_HEAD(&connector->modes);
connector->edid_blob_ptr = NULL;
@@ -764,6 +787,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
list_for_each_entry_safe(mode, t, &connector->modes, head)
drm_mode_remove(connector, mode);
+ ida_remove(&drm_connector_enum_list[connector->connector_type].ida,
+ connector->connector_type_id);
+
drm_mode_object_put(dev, &connector->base);
list_del(&connector->head);
dev->mode_config.num_connector--;
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 99fcd7c..00597a1 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -251,6 +251,7 @@ static int __init drm_core_init(void)
int ret = -ENOMEM;
drm_global_init();
+ drm_connector_ida_init();
idr_init(&drm_minors_idr);
if (register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops))
@@ -298,6 +299,7 @@ static void __exit drm_core_exit(void)
unregister_chrdev(DRM_MAJOR, "drm");
+ drm_connector_ida_destroy();
idr_destroy(&drm_minors_idr);
}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index fa12a2f..effee9d 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -869,6 +869,8 @@ extern int drm_crtc_init(struct drm_device *dev,
const struct drm_crtc_funcs *funcs);
extern void drm_crtc_cleanup(struct drm_crtc *crtc);
+extern void drm_connector_ida_init(void);
+extern void drm_connector_ida_destroy(void);
extern int drm_connector_init(struct drm_device *dev,
struct drm_connector *connector,
const struct drm_connector_funcs *funcs,
--
1.8.1.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3] drm: use ida to allocate connector ids
2013-08-08 2:34 ` [PATCH v3] " Ilia Mirkin
@ 2013-08-08 8:18 ` Ville Syrjälä
0 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2013-08-08 8:18 UTC (permalink / raw)
To: Ilia Mirkin; +Cc: dri-devel
On Wed, Aug 07, 2013 at 10:34:48PM -0400, Ilia Mirkin wrote:
> This makes it so that reloading a module does not cause all the
> connector ids to change, which are user-visible and sometimes used
> for configuration.
>
> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> ---
>
> v2 -> v3:
> - rename ida struct name to... ida. (i'm ready to accept my "creative name
> of the year" award.)
Seems fine to me. We already have the foo_idr and whatever.
> - fix object id leak when allocation fails.
> - use ida_simple_get, since it's OK to allocate memory inside kms lock
Looks fine to me.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> drivers/gpu/drm/drm_crtc.c | 62 ++++++++++++++++++++++++++++++++--------------
> drivers/gpu/drm/drm_drv.c | 2 ++
> include/drm/drm_crtc.h | 2 ++
> 3 files changed, 48 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index fc83bb9..a691764 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -186,29 +186,29 @@ static const struct drm_prop_enum_list drm_dirty_info_enum_list[] = {
> struct drm_conn_prop_enum_list {
> int type;
> const char *name;
> - int count;
> + struct ida ida;
> };
>
> /*
> * Connector and encoder types.
> */
> static struct drm_conn_prop_enum_list drm_connector_enum_list[] =
> -{ { DRM_MODE_CONNECTOR_Unknown, "Unknown", 0 },
> - { DRM_MODE_CONNECTOR_VGA, "VGA", 0 },
> - { DRM_MODE_CONNECTOR_DVII, "DVI-I", 0 },
> - { DRM_MODE_CONNECTOR_DVID, "DVI-D", 0 },
> - { DRM_MODE_CONNECTOR_DVIA, "DVI-A", 0 },
> - { DRM_MODE_CONNECTOR_Composite, "Composite", 0 },
> - { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO", 0 },
> - { DRM_MODE_CONNECTOR_LVDS, "LVDS", 0 },
> - { DRM_MODE_CONNECTOR_Component, "Component", 0 },
> - { DRM_MODE_CONNECTOR_9PinDIN, "DIN", 0 },
> - { DRM_MODE_CONNECTOR_DisplayPort, "DP", 0 },
> - { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A", 0 },
> - { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B", 0 },
> - { DRM_MODE_CONNECTOR_TV, "TV", 0 },
> - { DRM_MODE_CONNECTOR_eDP, "eDP", 0 },
> - { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual", 0},
> +{ { DRM_MODE_CONNECTOR_Unknown, "Unknown" },
> + { DRM_MODE_CONNECTOR_VGA, "VGA" },
> + { DRM_MODE_CONNECTOR_DVII, "DVI-I" },
> + { DRM_MODE_CONNECTOR_DVID, "DVI-D" },
> + { DRM_MODE_CONNECTOR_DVIA, "DVI-A" },
> + { DRM_MODE_CONNECTOR_Composite, "Composite" },
> + { DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO" },
> + { DRM_MODE_CONNECTOR_LVDS, "LVDS" },
> + { DRM_MODE_CONNECTOR_Component, "Component" },
> + { DRM_MODE_CONNECTOR_9PinDIN, "DIN" },
> + { DRM_MODE_CONNECTOR_DisplayPort, "DP" },
> + { DRM_MODE_CONNECTOR_HDMIA, "HDMI-A" },
> + { DRM_MODE_CONNECTOR_HDMIB, "HDMI-B" },
> + { DRM_MODE_CONNECTOR_TV, "TV" },
> + { DRM_MODE_CONNECTOR_eDP, "eDP" },
> + { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
> };
>
> static const struct drm_prop_enum_list drm_encoder_enum_list[] =
> @@ -220,6 +220,22 @@ static const struct drm_prop_enum_list drm_encoder_enum_list[] =
> { DRM_MODE_ENCODER_VIRTUAL, "Virtual" },
> };
>
> +void drm_connector_ida_init(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
> + ida_init(&drm_connector_enum_list[i].ida);
> +}
> +
> +void drm_connector_ida_destroy(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(drm_connector_enum_list); i++)
> + ida_destroy(&drm_connector_enum_list[i].ida);
> +}
> +
> const char *drm_get_encoder_name(const struct drm_encoder *encoder)
> {
> static char buf[32];
> @@ -711,6 +727,8 @@ int drm_connector_init(struct drm_device *dev,
> int connector_type)
> {
> int ret;
> + struct ida *connector_ida =
> + &drm_connector_enum_list[connector_type].ida;
>
> drm_modeset_lock_all(dev);
>
> @@ -723,7 +741,12 @@ int drm_connector_init(struct drm_device *dev,
> connector->funcs = funcs;
> connector->connector_type = connector_type;
> connector->connector_type_id =
> - ++drm_connector_enum_list[connector_type].count; /* TODO */
> + ida_simple_get(connector_ida, 1, 0, GFP_KERNEL);
> + if (connector->connector_type_id < 0) {
> + ret = connector->connector_type_id;
> + drm_mode_object_put(dev, &connector->base);
> + goto out;
> + }
> INIT_LIST_HEAD(&connector->probed_modes);
> INIT_LIST_HEAD(&connector->modes);
> connector->edid_blob_ptr = NULL;
> @@ -764,6 +787,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
> list_for_each_entry_safe(mode, t, &connector->modes, head)
> drm_mode_remove(connector, mode);
>
> + ida_remove(&drm_connector_enum_list[connector->connector_type].ida,
> + connector->connector_type_id);
> +
> drm_mode_object_put(dev, &connector->base);
> list_del(&connector->head);
> dev->mode_config.num_connector--;
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 99fcd7c..00597a1 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -251,6 +251,7 @@ static int __init drm_core_init(void)
> int ret = -ENOMEM;
>
> drm_global_init();
> + drm_connector_ida_init();
> idr_init(&drm_minors_idr);
>
> if (register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops))
> @@ -298,6 +299,7 @@ static void __exit drm_core_exit(void)
>
> unregister_chrdev(DRM_MAJOR, "drm");
>
> + drm_connector_ida_destroy();
> idr_destroy(&drm_minors_idr);
> }
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index fa12a2f..effee9d 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -869,6 +869,8 @@ extern int drm_crtc_init(struct drm_device *dev,
> const struct drm_crtc_funcs *funcs);
> extern void drm_crtc_cleanup(struct drm_crtc *crtc);
>
> +extern void drm_connector_ida_init(void);
> +extern void drm_connector_ida_destroy(void);
> extern int drm_connector_init(struct drm_device *dev,
> struct drm_connector *connector,
> const struct drm_connector_funcs *funcs,
> --
> 1.8.1.5
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-08-08 8:18 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-30 7:16 [PATCH] drm: use ida to allocate connector ids Ilia Mirkin
2013-07-30 7:51 ` [PATCH v2] " Ilia Mirkin
2013-08-07 0:12 ` Dave Airlie
2013-08-07 0:40 ` Rob Clark
2013-08-07 1:04 ` Ilia Mirkin
2013-08-07 8:00 ` Ville Syrjälä
2013-08-07 8:28 ` Ilia Mirkin
2013-08-07 8:40 ` Daniel Vetter
2013-08-07 10:47 ` Ville Syrjälä
2013-08-08 2:34 ` [PATCH v3] " Ilia Mirkin
2013-08-08 8:18 ` Ville Syrjälä
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.