From: Dave Airlie airlied@redhat.com
Signed-off-by: Dave Airlie airlied@redhat.com --- include/drm/drm_crtc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 297e527..6279989 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -2606,7 +2606,7 @@ static inline uint32_t drm_color_lut_extract(uint32_t user_input, return clamp_val(val, 0, max); }
-/* +/** * drm_framebuffer_reference - incr the fb refcnt * @fb: framebuffer *
From: Dave Airlie airlied@redhat.com
This uses the previous changes to add reference counts to drm connector objects.
v2: move fbdev changes to their own patch. add some kerneldoc
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/drm_atomic.c | 19 +++++++++++++++++-- drivers/gpu/drm/drm_crtc.c | 28 ++++++++++++++++++++++++---- include/drm/drm_crtc.h | 32 +++++++++++++++++++++++++++++++- 3 files changed, 72 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 8ee1db8..9d5e3c8 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -154,6 +154,7 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) state->connector_states[i]); state->connectors[i] = NULL; state->connector_states[i] = NULL; + drm_connector_unreference(connector); }
for (i = 0; i < config->num_crtc; i++) { @@ -924,6 +925,7 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state, if (!connector_state) return ERR_PTR(-ENOMEM);
+ drm_connector_reference(connector); state->connector_states[index] = connector_state; state->connectors[index] = connector; connector_state->state = state; @@ -1614,12 +1616,19 @@ retry: }
obj = drm_mode_object_find(dev, obj_id, DRM_MODE_OBJECT_ANY); - if (!obj || !obj->properties) { + if (!obj) { + ret = -ENOENT; + goto out; + } + + if (!obj->properties) { + drm_mode_object_unreference(obj); ret = -ENOENT; goto out; }
if (get_user(count_props, count_props_ptr + copied_objs)) { + drm_mode_object_unreference(obj); ret = -EFAULT; goto out; } @@ -1632,12 +1641,14 @@ retry: struct drm_property *prop;
if (get_user(prop_id, props_ptr + copied_props)) { + drm_mode_object_unreference(obj); ret = -EFAULT; goto out; }
prop = drm_property_find(dev, prop_id); if (!prop) { + drm_mode_object_unreference(obj); ret = -ENOENT; goto out; } @@ -1645,13 +1656,16 @@ retry: if (copy_from_user(&prop_value, prop_values_ptr + copied_props, sizeof(prop_value))) { + drm_mode_object_unreference(obj); ret = -EFAULT; goto out; }
ret = atomic_set_prop(state, obj, prop, prop_value); - if (ret) + if (ret) { + drm_mode_object_unreference(obj); goto out; + }
copied_props++; } @@ -1662,6 +1676,7 @@ retry: plane_mask |= (1 << drm_plane_index(plane)); plane->old_fb = plane->fb; } + drm_mode_object_unreference(obj); }
if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) { diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 4e5b015..a78e202 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -862,6 +862,16 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector) mode->interlace ? " interlaced" : ""); }
+static void drm_connector_free(struct kref *kref) +{ + struct drm_connector *connector = + container_of(kref, struct drm_connector, base.refcount); + struct drm_device *dev = connector->dev; + + drm_mode_object_unregister(dev, &connector->base); + connector->funcs->destroy(connector); +} + /** * drm_connector_init - Init a preallocated connector * @dev: DRM device @@ -887,7 +897,9 @@ int drm_connector_init(struct drm_device *dev,
drm_modeset_lock_all(dev);
- ret = drm_mode_object_get_reg(dev, &connector->base, DRM_MODE_OBJECT_CONNECTOR, false, NULL); + ret = drm_mode_object_get_reg(dev, &connector->base, + DRM_MODE_OBJECT_CONNECTOR, + false, drm_connector_free); if (ret) goto out_unlock;
@@ -2147,7 +2159,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
mutex_lock(&dev->mode_config.mutex);
- connector = drm_connector_find(dev, out_resp->connector_id); + connector = drm_connector_lookup(dev, out_resp->connector_id); if (!connector) { ret = -ENOENT; goto out_unlock; @@ -2231,6 +2243,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, out: drm_modeset_unlock(&dev->mode_config.connection_mutex);
+ drm_connector_unreference(connector); out_unlock: mutex_unlock(&dev->mode_config.mutex);
@@ -2875,13 +2888,14 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, }
for (i = 0; i < crtc_req->count_connectors; i++) { + connector_set[i] = NULL; set_connectors_ptr = (uint32_t __user *)(unsigned long)crtc_req->set_connectors_ptr; if (get_user(out_id, &set_connectors_ptr[i])) { ret = -EFAULT; goto out; }
- connector = drm_connector_find(dev, out_id); + connector = drm_connector_lookup(dev, out_id); if (!connector) { DRM_DEBUG_KMS("Connector id %d unknown\n", out_id); @@ -2909,6 +2923,12 @@ out: if (fb) drm_framebuffer_unreference(fb);
+ if (connector_set) { + for (i = 0; i < crtc_req->count_connectors; i++) { + if (connector_set[i]) + drm_connector_unreference(connector_set[i]); + } + } kfree(connector_set); drm_mode_destroy(dev, mode); drm_modeset_unlock_all(dev); @@ -4999,7 +5019,7 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, property = obj_to_property(prop_obj);
if (!drm_property_change_valid_get(property, arg->value, &ref)) - goto out; + goto out_unref;
switch (arg_obj->type) { case DRM_MODE_OBJECT_CONNECTOR: diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 6279989..68100b8 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -2571,7 +2571,15 @@ static inline struct drm_encoder *drm_encoder_find(struct drm_device *dev, return mo ? obj_to_encoder(mo) : NULL; }
-static inline struct drm_connector *drm_connector_find(struct drm_device *dev, +/** + * drm_connector_lookup - lookup connector object + * @dev: DRM device + * @id: connector object id + * + * This function looks up the connector object specified by id + * add takes a reference to it. + */ +static inline struct drm_connector *drm_connector_lookup(struct drm_device *dev, uint32_t id) { struct drm_mode_object *mo; @@ -2639,6 +2647,28 @@ static inline uint32_t drm_framebuffer_read_refcount(struct drm_framebuffer *fb) return atomic_read(&fb->base.refcount.refcount); }
+/** + * drm_connector_reference - incr the connector refcnt + * @connector: connector + * + * This function increments the connector's refcount. + */ +static inline void drm_connector_reference(struct drm_connector *connector) +{ + drm_mode_object_reference(&connector->base); +} + +/** + * drm_connector_unreference - unref a connector + * @connector: connector to unref + * + * This function decrements the connector's refcount and frees it if it drops to zero. + */ +static inline void drm_connector_unreference(struct drm_connector *connector) +{ + drm_mode_object_unreference(&connector->base); +} + /* Plane list iterator for legacy (overlay only) planes. */ #define drm_for_each_legacy_plane(plane, dev) \ list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
From: Dave Airlie airlied@redhat.com
This takes a reference count when fbdev adds the connector, and drops it when it removes the connector.
It also drops the now unneeded code to find connectors and remove the from the modeset as they are reference counted.
v2: drop references when removing all connectors at end.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/drm_fb_helper.c | 38 +++++--------------------------------- 1 file changed, 5 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 855108e..19a7a71 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -153,40 +153,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_ if (!fb_helper_connector) return -ENOMEM;
+ drm_connector_reference(connector); fb_helper_connector->connector = connector; fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector; return 0; } EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
-static void remove_from_modeset(struct drm_mode_set *set, - struct drm_connector *connector) -{ - int i, j; - - for (i = 0; i < set->num_connectors; i++) { - if (set->connectors[i] == connector) - break; - } - - if (i == set->num_connectors) - return; - - for (j = i + 1; j < set->num_connectors; j++) { - set->connectors[j - 1] = set->connectors[j]; - } - set->num_connectors--; - - /* - * TODO maybe need to makes sure we set it back to !=NULL somewhere? - */ - if (set->num_connectors == 0) { - set->fb = NULL; - drm_mode_destroy(connector->dev, set->mode); - set->mode = NULL; - } -} - int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector) { @@ -206,6 +179,7 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, if (i == fb_helper->connector_count) return -EINVAL; fb_helper_connector = fb_helper->connector_info[i]; + drm_connector_unreference(fb_helper_connector->connector);
for (j = i + 1; j < fb_helper->connector_count; j++) { fb_helper->connector_info[j - 1] = fb_helper->connector_info[j]; @@ -213,10 +187,6 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, fb_helper->connector_count--; kfree(fb_helper_connector);
- /* also cleanup dangling references to the connector: */ - for (i = 0; i < fb_helper->crtc_count; i++) - remove_from_modeset(&fb_helper->crtc_info[i].mode_set, connector); - return 0; } EXPORT_SYMBOL(drm_fb_helper_remove_one_connector); @@ -626,8 +596,10 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) { int i;
- for (i = 0; i < helper->connector_count; i++) + for (i = 0; i < helper->connector_count; i++) { + drm_connector_unreference(helper->connector_info[i]->connector); kfree(helper->connector_info[i]); + } kfree(helper->connector_info); for (i = 0; i < helper->crtc_count; i++) { kfree(helper->crtc_info[i].mode_set.connectors);
On Tue, May 03, 2016 at 02:28:23PM +1000, Dave Airlie wrote:
From: Dave Airlie airlied@redhat.com
This takes a reference count when fbdev adds the connector, and drops it when it removes the connector.
It also drops the now unneeded code to find connectors and remove the from the modeset as they are reference counted.
v2: drop references when removing all connectors at end.
Signed-off-by: Dave Airlie airlied@redhat.com
Yeah I think this looks good now.
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_fb_helper.c | 38 +++++--------------------------------- 1 file changed, 5 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 855108e..19a7a71 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -153,40 +153,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_ if (!fb_helper_connector) return -ENOMEM;
- drm_connector_reference(connector); fb_helper_connector->connector = connector; fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector; return 0;
} EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
-static void remove_from_modeset(struct drm_mode_set *set,
struct drm_connector *connector)-{
- int i, j;
- for (i = 0; i < set->num_connectors; i++) {
if (set->connectors[i] == connector)break;- }
- if (i == set->num_connectors)
return;- for (j = i + 1; j < set->num_connectors; j++) {
set->connectors[j - 1] = set->connectors[j];- }
- set->num_connectors--;
- /*
* TODO maybe need to makes sure we set it back to !=NULL somewhere?*/- if (set->num_connectors == 0) {
set->fb = NULL;drm_mode_destroy(connector->dev, set->mode);set->mode = NULL;- }
-}
int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector) { @@ -206,6 +179,7 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, if (i == fb_helper->connector_count) return -EINVAL; fb_helper_connector = fb_helper->connector_info[i];
drm_connector_unreference(fb_helper_connector->connector);
for (j = i + 1; j < fb_helper->connector_count; j++) { fb_helper->connector_info[j - 1] = fb_helper->connector_info[j];
@@ -213,10 +187,6 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, fb_helper->connector_count--; kfree(fb_helper_connector);
- /* also cleanup dangling references to the connector: */
- for (i = 0; i < fb_helper->crtc_count; i++)
remove_from_modeset(&fb_helper->crtc_info[i].mode_set, connector);- return 0;
} EXPORT_SYMBOL(drm_fb_helper_remove_one_connector); @@ -626,8 +596,10 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) { int i;
- for (i = 0; i < helper->connector_count; i++)
- for (i = 0; i < helper->connector_count; i++) {
drm_connector_unreference(helper->connector_info[i]->connector);- } kfree(helper->connector_info); for (i = 0; i < helper->crtc_count; i++) { kfree(helper->crtc_info[i].mode_set.connectors);
-- 2.5.5
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
From: Dave Airlie airlied@redhat.com
This just takes a reference on the connector when we set a mode in the non-atomic paths.
v2: Follow Daniel Stone's suggestions on when to take/drop references.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/drm_crtc_helper.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 66ca313..f47a252 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -456,6 +456,9 @@ drm_crtc_helper_disable(struct drm_crtc *crtc) * between them is henceforth no longer available. */ connector->dpms = DRM_MODE_DPMS_OFF; + + /* we keep a reference while the encoder is bound */ + drm_connector_unreference(connector); } }
@@ -606,6 +609,11 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) mode_changed = true; }
+ /* take a reference on all connectors in set */ + for (ro = 0; ro < set->num_connectors; ro++) { + drm_connector_reference(set->connectors[ro]); + } + /* a) traverse passed in connector list and get encoders for them */ count = 0; drm_for_each_connector(connector, dev) { @@ -724,6 +732,12 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) } }
+ /* after fail drop reference on all connectors in save set */ + count = 0; + drm_for_each_connector(connector, dev) { + drm_connector_unreference(&save_connectors[count++]); + } + kfree(save_connectors); kfree(save_encoders); return 0; @@ -740,6 +754,11 @@ fail: *connector = save_connectors[count++]; }
+ /* after fail drop reference on all connectors in set */ + for (ro = 0; ro < set->num_connectors; ro++) { + drm_connector_unreference(set->connectors[ro]); + } + /* Try to restore the config */ if (mode_changed && !drm_crtc_helper_set_mode(save_set.crtc, save_set.mode, save_set.x,
On Tue, May 03, 2016 at 02:28:24PM +1000, Dave Airlie wrote:
From: Dave Airlie airlied@redhat.com
This just takes a reference on the connector when we set a mode in the non-atomic paths.
v2: Follow Daniel Stone's suggestions on when to take/drop references.
Signed-off-by: Dave Airlie airlied@redhat.com
I've already embarrassed myself once with a bonghits r-b, let's try this again:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_crtc_helper.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 66ca313..f47a252 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -456,6 +456,9 @@ drm_crtc_helper_disable(struct drm_crtc *crtc) * between them is henceforth no longer available. */ connector->dpms = DRM_MODE_DPMS_OFF;
/* we keep a reference while the encoder is bound */drm_connector_unreference(connector);@@ -606,6 +609,11 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) mode_changed = true; }
- /* take a reference on all connectors in set */
- for (ro = 0; ro < set->num_connectors; ro++) {
drm_connector_reference(set->connectors[ro]);- }
- /* a) traverse passed in connector list and get encoders for them */ count = 0; drm_for_each_connector(connector, dev) {
@@ -724,6 +732,12 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) } }
- /* after fail drop reference on all connectors in save set */
- count = 0;
- drm_for_each_connector(connector, dev) {
drm_connector_unreference(&save_connectors[count++]);- }
- kfree(save_connectors); kfree(save_encoders); return 0;
@@ -740,6 +754,11 @@ fail: *connector = save_connectors[count++]; }
- /* after fail drop reference on all connectors in set */
- for (ro = 0; ro < set->num_connectors; ro++) {
drm_connector_unreference(set->connectors[ro]);- }
- /* Try to restore the config */ if (mode_changed && !drm_crtc_helper_set_mode(save_set.crtc, save_set.mode, save_set.x,
-- 2.5.5
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
From: Dave Airlie airlied@redhat.com
Take a reference when setting a crtc on a connecter, also take one when duplicating if a crtc is set, and drop one on destroy if a crtc is set.
v2: take Daniel Stone's advice and simplify the ref/unref dances, also take care of NULL as connector to state reset.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/drm_atomic.c | 4 ++++ drivers/gpu/drm/drm_atomic_helper.c | 5 +++++ 2 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 9d5e3c8..91cae88 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1160,6 +1160,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, { struct drm_crtc_state *crtc_state;
+ if (crtc) + drm_connector_reference(conn_state->connector); if (conn_state->crtc && conn_state->crtc != crtc) { crtc_state = drm_atomic_get_existing_crtc_state(conn_state->state, conn_state->crtc); @@ -1177,6 +1179,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, 1 << drm_connector_index(conn_state->connector); }
+ if (conn_state->crtc) + drm_connector_unreference(conn_state->connector); conn_state->crtc = crtc;
if (crtc) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index d25abce..f3e2642 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2762,6 +2762,8 @@ __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector, struct drm_connector_state *state) { memcpy(state, connector->state, sizeof(*state)); + if (state->crtc) + drm_connector_reference(connector); } EXPORT_SYMBOL(__drm_atomic_helper_connector_duplicate_state);
@@ -2889,6 +2891,9 @@ __drm_atomic_helper_connector_destroy_state(struct drm_connector *connector, * state will automatically do the right thing if code is ever added * to this function. */ + if (connector && state->crtc) { + drm_connector_unreference(state->connector); + } } EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state);
On Tue, May 03, 2016 at 02:28:25PM +1000, Dave Airlie wrote:
From: Dave Airlie airlied@redhat.com
Take a reference when setting a crtc on a connecter, also take one when duplicating if a crtc is set, and drop one on destroy if a crtc is set.
v2: take Daniel Stone's advice and simplify the ref/unref dances, also take care of NULL as connector to state reset.
Signed-off-by: Dave Airlie airlied@redhat.com
drivers/gpu/drm/drm_atomic.c | 4 ++++ drivers/gpu/drm/drm_atomic_helper.c | 5 +++++ 2 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 9d5e3c8..91cae88 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1160,6 +1160,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, { struct drm_crtc_state *crtc_state;
- if (crtc)
drm_connector_reference(conn_state->connector);@@ -1177,6 +1179,8 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, 1 << drm_connector_index(conn_state->connector); }
if (conn_state->crtc)
drm_connector_unreference(conn_state->connector);conn_state->crtc = crtc;
if (crtc)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index d25abce..f3e2642 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2762,6 +2762,8 @@ __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector, struct drm_connector_state *state) { memcpy(state, connector->state, sizeof(*state));
- if (state->crtc)
drm_connector_reference(connector);} EXPORT_SYMBOL(__drm_atomic_helper_connector_duplicate_state);
@@ -2889,6 +2891,9 @@ __drm_atomic_helper_connector_destroy_state(struct drm_connector *connector, * state will automatically do the right thing if code is ever added * to this function. */
- if (connector && state->crtc) {
drm_connector_unreference(state->connector);- }
Bikeshed: no need for {} and you don't need to check that connector is NULL either. Tbh all the destroy_state helpers don't really need their object argument, only the state. Cleaning that up is somewhere on my todo.
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
} EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state);
-- 2.5.5
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
*/
if (connector && state->crtc) {drm_connector_unreference(state->connector);}Bikeshed: no need for {} and you don't need to check that connector is NULL either. Tbh all the destroy_state helpers don't really need their object argument, only the state. Cleaning that up is somewhere on my todo.
Yes you really do need the connector, lost a boot to an oops,
drm_atomic_state_default_clear does some funky stuff that can possibly go away after this, but I'm not sure.
Dave.
On Tue, May 3, 2016 at 12:09 PM, Dave Airlie airlied@gmail.com wrote:
*/
if (connector && state->crtc) {drm_connector_unreference(state->connector);}Bikeshed: no need for {} and you don't need to check that connector is NULL either. Tbh all the destroy_state helpers don't really need their object argument, only the state. Cleaning that up is somewhere on my todo.
Yes you really do need the connector, lost a boot to an oops,
drm_atomic_state_default_clear does some funky stuff that can possibly go away after this, but I'm not sure.
Surprising, didn't expect that. Do you still have the backtrace somewhere? I really wonder what goes boom with that one ...
What we might need is check for state->connector (instead of connector), but that being NULL for a valid state is also a bit a bug. -Daniel
Hi,
On 3 May 2016 at 05:28, Dave Airlie airlied@gmail.com wrote:
From: Dave Airlie airlied@redhat.com
Take a reference when setting a crtc on a connecter, also take one when duplicating if a crtc is set, and drop one on destroy if a crtc is set.
v2: take Daniel Stone's advice and simplify the ref/unref dances, also take care of NULL as connector to state reset.
Signed-off-by: Dave Airlie airlied@redhat.com
I expect we'll still end up with all kinds of comedy races here, but on the grounds that it's unambiguously an improvement on what came before, and I can't see anything obviously wrong, for patches 1-5: Reviewed-by: Daniel Stone daniels@collabora.com
Cheers, Daniel
From: Dave Airlie airlied@redhat.com
Don't just free the connector when we get the destroy callback.
Drop a reference to it, and set it's mst_port to NULL so no more mst work is done on it.
v2: core mst accepts NULLs fine. Cleanup EDID code properly. v3: drop the extra reference we were taking.
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/i915/intel_dp_mst.c | 43 +++++++++++++++++-------------------- drivers/gpu/drm/i915/intel_drv.h | 2 +- 2 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 94b4e83..b6bf7fd 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -109,7 +109,7 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder)
DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
- drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, intel_mst->port); + drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, intel_mst->connector->port);
ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr); if (ret) { @@ -134,10 +134,11 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder) /* and this can also fail */ drm_dp_update_payload_part2(&intel_dp->mst_mgr);
- drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, intel_mst->port); + drm_dp_mst_deallocate_vcpi(&intel_dp->mst_mgr, intel_mst->connector->port);
intel_dp->active_mst_links--; - intel_mst->port = NULL; + + intel_mst->connector = NULL; if (intel_dp->active_mst_links == 0) { intel_dig_port->base.post_disable(&intel_dig_port->base); intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); @@ -177,7 +178,8 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder) found->encoder = encoder;
DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links); - intel_mst->port = found->port; + + intel_mst->connector = found;
if (intel_dp->active_mst_links == 0) { intel_prepare_ddi_buffer(&intel_dig_port->base); @@ -195,7 +197,7 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder) }
ret = drm_dp_mst_allocate_vcpi(&intel_dp->mst_mgr, - intel_mst->port, + intel_mst->connector->port, intel_crtc->config->pbn, &slots); if (ret == false) { DRM_ERROR("failed to allocate vcpi\n"); @@ -244,7 +246,7 @@ static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder, { struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base); *pipe = intel_mst->pipe; - if (intel_mst->port) + if (intel_mst->connector) return true; return false; } @@ -308,10 +310,11 @@ static int intel_dp_mst_get_ddc_modes(struct drm_connector *connector) struct edid *edid; int ret;
- edid = drm_dp_mst_get_edid(connector, &intel_dp->mst_mgr, intel_connector->port); - if (!edid) - return 0; + if (!intel_dp) { + return intel_connector_update_modes(connector, NULL); + }
+ edid = drm_dp_mst_get_edid(connector, &intel_dp->mst_mgr, intel_connector->port); ret = intel_connector_update_modes(connector, edid); kfree(edid);
@@ -324,6 +327,8 @@ intel_dp_mst_detect(struct drm_connector *connector, bool force) struct intel_connector *intel_connector = to_intel_connector(connector); struct intel_dp *intel_dp = intel_connector->mst_port;
+ if (!intel_dp) + return connector_status_disconnected; return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr, intel_connector->port); }
@@ -389,6 +394,8 @@ static struct drm_encoder *intel_mst_atomic_best_encoder(struct drm_connector *c struct intel_dp *intel_dp = intel_connector->mst_port; struct intel_crtc *crtc = to_intel_crtc(state->crtc);
+ if (!intel_dp) + return NULL; return &intel_dp->mst_encoders[crtc->pipe]->base.base; }
@@ -396,6 +403,8 @@ static struct drm_encoder *intel_mst_best_encoder(struct drm_connector *connecto { struct intel_connector *intel_connector = to_intel_connector(connector); struct intel_dp *intel_dp = intel_connector->mst_port; + if (!intel_dp) + return NULL; return &intel_dp->mst_encoders[0]->base.base; }
@@ -506,23 +515,11 @@ static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
/* need to nuke the connector */ drm_modeset_lock_all(dev); - if (connector->state->crtc) { - struct drm_mode_set set; - int ret; - - memset(&set, 0, sizeof(set)); - set.crtc = connector->state->crtc, - - ret = drm_atomic_helper_set_config(&set); - - WARN(ret, "Disabling mst crtc failed with %i\n", ret); - } - intel_connector_remove_from_fbdev(intel_connector); - drm_connector_cleanup(connector); + intel_connector->mst_port = NULL; drm_modeset_unlock_all(dev);
- kfree(intel_connector); + drm_connector_unreference(&intel_connector->base); DRM_DEBUG_KMS("\n"); }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index e0fcfa1..288135de 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -878,7 +878,7 @@ struct intel_dp_mst_encoder { struct intel_encoder base; enum pipe pipe; struct intel_digital_port *primary; - void *port; /* store this opaque as its illegal to dereference it */ + struct intel_connector *connector; };
static inline enum dpio_channel
On Tue, May 03, 2016 at 02:28:21PM +1000, Dave Airlie wrote:
From: Dave Airlie airlied@redhat.com
Signed-off-by: Dave Airlie airlied@redhat.com
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
include/drm/drm_crtc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 297e527..6279989 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -2606,7 +2606,7 @@ static inline uint32_t drm_color_lut_extract(uint32_t user_input, return clamp_val(val, 0, max); }
-/* +/**
- drm_framebuffer_reference - incr the fb refcnt
- @fb: framebuffer
-- 2.5.5
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Daniel Stone -
Daniel Vetter -
Dave Airlie