drm: Tighten locking in drm_mode_getconnector
- Modeset state needs mode_config->connection mutex, that covers figuring out the encoder, and reading properties (since in the atomic case those need to look at connector->state). - Don't hold any locks for stuff that's invariant (i.e. possible connectors). - Same for connector lookup and unref, those don't need any locks. - And finally the probe stuff is only protected by mode_config->mutex. While at it updated the kerneldoc for these fields in drm_connector and add docs explaining what's protected by which locks. Reviewed-by: Sean Paul <seanpaul@chromium.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161213230814.19598-10-daniel.vetter@ffwll.ch
This commit is contained in:
parent
e73ab00e9a
commit
91eefc05f0
|
@ -1163,36 +1163,9 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
|
||||||
|
|
||||||
memset(&u_mode, 0, sizeof(struct drm_mode_modeinfo));
|
memset(&u_mode, 0, sizeof(struct drm_mode_modeinfo));
|
||||||
|
|
||||||
mutex_lock(&dev->mode_config.mutex);
|
|
||||||
|
|
||||||
connector = drm_connector_lookup(dev, out_resp->connector_id);
|
connector = drm_connector_lookup(dev, out_resp->connector_id);
|
||||||
if (!connector) {
|
if (!connector)
|
||||||
ret = -ENOENT;
|
return -ENOENT;
|
||||||
goto out_unlock;
|
|
||||||
}
|
|
||||||
|
|
||||||
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
|
|
||||||
if (connector->encoder_ids[i] != 0)
|
|
||||||
encoders_count++;
|
|
||||||
|
|
||||||
if (out_resp->count_modes == 0) {
|
|
||||||
connector->funcs->fill_modes(connector,
|
|
||||||
dev->mode_config.max_width,
|
|
||||||
dev->mode_config.max_height);
|
|
||||||
}
|
|
||||||
|
|
||||||
/* delayed so we get modes regardless of pre-fill_modes state */
|
|
||||||
list_for_each_entry(mode, &connector->modes, head)
|
|
||||||
if (drm_mode_expose_to_userspace(mode, file_priv))
|
|
||||||
mode_count++;
|
|
||||||
|
|
||||||
out_resp->connector_id = connector->base.id;
|
|
||||||
out_resp->connector_type = connector->connector_type;
|
|
||||||
out_resp->connector_type_id = connector->connector_type_id;
|
|
||||||
out_resp->mm_width = connector->display_info.width_mm;
|
|
||||||
out_resp->mm_height = connector->display_info.height_mm;
|
|
||||||
out_resp->subpixel = connector->display_info.subpixel_order;
|
|
||||||
out_resp->connection = connector->status;
|
|
||||||
|
|
||||||
drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
|
drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
|
||||||
encoder = drm_connector_get_encoder(connector);
|
encoder = drm_connector_get_encoder(connector);
|
||||||
|
@ -1201,6 +1174,55 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
|
||||||
else
|
else
|
||||||
out_resp->encoder_id = 0;
|
out_resp->encoder_id = 0;
|
||||||
|
|
||||||
|
ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic,
|
||||||
|
(uint32_t __user *)(unsigned long)(out_resp->props_ptr),
|
||||||
|
(uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr),
|
||||||
|
&out_resp->count_props);
|
||||||
|
drm_modeset_unlock(&dev->mode_config.connection_mutex);
|
||||||
|
if (ret)
|
||||||
|
goto out_unref;
|
||||||
|
|
||||||
|
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
|
||||||
|
if (connector->encoder_ids[i] != 0)
|
||||||
|
encoders_count++;
|
||||||
|
|
||||||
|
if ((out_resp->count_encoders >= encoders_count) && encoders_count) {
|
||||||
|
copied = 0;
|
||||||
|
encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr);
|
||||||
|
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
|
||||||
|
if (connector->encoder_ids[i] != 0) {
|
||||||
|
if (put_user(connector->encoder_ids[i],
|
||||||
|
encoder_ptr + copied)) {
|
||||||
|
ret = -EFAULT;
|
||||||
|
goto out_unref;
|
||||||
|
}
|
||||||
|
copied++;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
out_resp->count_encoders = encoders_count;
|
||||||
|
|
||||||
|
out_resp->connector_id = connector->base.id;
|
||||||
|
out_resp->connector_type = connector->connector_type;
|
||||||
|
out_resp->connector_type_id = connector->connector_type_id;
|
||||||
|
|
||||||
|
mutex_lock(&dev->mode_config.mutex);
|
||||||
|
if (out_resp->count_modes == 0) {
|
||||||
|
connector->funcs->fill_modes(connector,
|
||||||
|
dev->mode_config.max_width,
|
||||||
|
dev->mode_config.max_height);
|
||||||
|
}
|
||||||
|
|
||||||
|
out_resp->mm_width = connector->display_info.width_mm;
|
||||||
|
out_resp->mm_height = connector->display_info.height_mm;
|
||||||
|
out_resp->subpixel = connector->display_info.subpixel_order;
|
||||||
|
out_resp->connection = connector->status;
|
||||||
|
|
||||||
|
/* delayed so we get modes regardless of pre-fill_modes state */
|
||||||
|
list_for_each_entry(mode, &connector->modes, head)
|
||||||
|
if (drm_mode_expose_to_userspace(mode, file_priv))
|
||||||
|
mode_count++;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* This ioctl is called twice, once to determine how much space is
|
* This ioctl is called twice, once to determine how much space is
|
||||||
* needed, and the 2nd time to fill it.
|
* needed, and the 2nd time to fill it.
|
||||||
|
@ -1222,36 +1244,10 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
out_resp->count_modes = mode_count;
|
out_resp->count_modes = mode_count;
|
||||||
|
|
||||||
ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic,
|
|
||||||
(uint32_t __user *)(unsigned long)(out_resp->props_ptr),
|
|
||||||
(uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr),
|
|
||||||
&out_resp->count_props);
|
|
||||||
if (ret)
|
|
||||||
goto out;
|
|
||||||
|
|
||||||
if ((out_resp->count_encoders >= encoders_count) && encoders_count) {
|
|
||||||
copied = 0;
|
|
||||||
encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr);
|
|
||||||
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
|
|
||||||
if (connector->encoder_ids[i] != 0) {
|
|
||||||
if (put_user(connector->encoder_ids[i],
|
|
||||||
encoder_ptr + copied)) {
|
|
||||||
ret = -EFAULT;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
copied++;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
out_resp->count_encoders = encoders_count;
|
|
||||||
|
|
||||||
out:
|
out:
|
||||||
drm_modeset_unlock(&dev->mode_config.connection_mutex);
|
|
||||||
|
|
||||||
drm_connector_unreference(connector);
|
|
||||||
out_unlock:
|
|
||||||
mutex_unlock(&dev->mode_config.mutex);
|
mutex_unlock(&dev->mode_config.mutex);
|
||||||
|
out_unref:
|
||||||
|
drm_connector_unreference(connector);
|
||||||
|
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
|
|
@ -563,9 +563,6 @@ struct drm_cmdline_mode {
|
||||||
* @interlace_allowed: can this connector handle interlaced modes?
|
* @interlace_allowed: can this connector handle interlaced modes?
|
||||||
* @doublescan_allowed: can this connector handle doublescan?
|
* @doublescan_allowed: can this connector handle doublescan?
|
||||||
* @stereo_allowed: can this connector handle stereo modes?
|
* @stereo_allowed: can this connector handle stereo modes?
|
||||||
* @modes: modes available on this connector (from fill_modes() + user)
|
|
||||||
* @status: one of the drm_connector_status enums (connected, not, or unknown)
|
|
||||||
* @probed_modes: list of modes derived directly from the display
|
|
||||||
* @funcs: connector control functions
|
* @funcs: connector control functions
|
||||||
* @edid_blob_ptr: DRM property containing EDID if present
|
* @edid_blob_ptr: DRM property containing EDID if present
|
||||||
* @properties: property tracking for this connector
|
* @properties: property tracking for this connector
|
||||||
|
@ -635,11 +632,27 @@ struct drm_connector {
|
||||||
* Protected by @mutex.
|
* Protected by @mutex.
|
||||||
*/
|
*/
|
||||||
bool registered;
|
bool registered;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @modes:
|
||||||
|
* Modes available on this connector (from fill_modes() + user).
|
||||||
|
* Protected by dev->mode_config.mutex.
|
||||||
|
*/
|
||||||
struct list_head modes; /* list of modes on this connector */
|
struct list_head modes; /* list of modes on this connector */
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @status:
|
||||||
|
* One of the drm_connector_status enums (connected, not, or unknown).
|
||||||
|
* Protected by dev->mode_config.mutex.
|
||||||
|
*/
|
||||||
enum drm_connector_status status;
|
enum drm_connector_status status;
|
||||||
|
|
||||||
/* these are modes added by probing with DDC or the BIOS */
|
/**
|
||||||
|
* @probed_modes:
|
||||||
|
* These are modes added by probing with DDC or the BIOS, before
|
||||||
|
* filtering is applied. Used by the probe helpers.Protected by
|
||||||
|
* dev->mode_config.mutex.
|
||||||
|
*/
|
||||||
struct list_head probed_modes;
|
struct list_head probed_modes;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -648,6 +661,8 @@ struct drm_connector {
|
||||||
* flat panels in embedded systems, the driver should initialize the
|
* flat panels in embedded systems, the driver should initialize the
|
||||||
* display_info.width_mm and display_info.height_mm fields with the
|
* display_info.width_mm and display_info.height_mm fields with the
|
||||||
* physical size of the display.
|
* physical size of the display.
|
||||||
|
*
|
||||||
|
* Protected by dev->mode_config.mutex.
|
||||||
*/
|
*/
|
||||||
struct drm_display_info display_info;
|
struct drm_display_info display_info;
|
||||||
const struct drm_connector_funcs *funcs;
|
const struct drm_connector_funcs *funcs;
|
||||||
|
|
Loading…
Reference in a new issue