* [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.