mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 02:30:34 +02:00 
			
		
		
		
	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
					
				
					 2 changed files with 72 additions and 61 deletions
				
			
		| 
						 | 
				
			
			@ -1163,36 +1163,9 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 | 
			
		|||
 | 
			
		||||
	memset(&u_mode, 0, sizeof(struct drm_mode_modeinfo));
 | 
			
		||||
 | 
			
		||||
	mutex_lock(&dev->mode_config.mutex);
 | 
			
		||||
 | 
			
		||||
	connector = drm_connector_lookup(dev, out_resp->connector_id);
 | 
			
		||||
	if (!connector) {
 | 
			
		||||
		ret = -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;
 | 
			
		||||
	if (!connector)
 | 
			
		||||
		return -ENOENT;
 | 
			
		||||
 | 
			
		||||
	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 | 
			
		||||
	encoder = drm_connector_get_encoder(connector);
 | 
			
		||||
| 
						 | 
				
			
			@ -1201,6 +1174,55 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 | 
			
		|||
	else
 | 
			
		||||
		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
 | 
			
		||||
	 * 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;
 | 
			
		||||
 | 
			
		||||
	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:
 | 
			
		||||
	drm_modeset_unlock(&dev->mode_config.connection_mutex);
 | 
			
		||||
 | 
			
		||||
	drm_connector_unreference(connector);
 | 
			
		||||
out_unlock:
 | 
			
		||||
	mutex_unlock(&dev->mode_config.mutex);
 | 
			
		||||
out_unref:
 | 
			
		||||
	drm_connector_unreference(connector);
 | 
			
		||||
 | 
			
		||||
	return ret;
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -563,9 +563,6 @@ struct drm_cmdline_mode {
 | 
			
		|||
 * @interlace_allowed: can this connector handle interlaced modes?
 | 
			
		||||
 * @doublescan_allowed: can this connector handle doublescan?
 | 
			
		||||
 * @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
 | 
			
		||||
 * @edid_blob_ptr: DRM property containing EDID if present
 | 
			
		||||
 * @properties: property tracking for this connector
 | 
			
		||||
| 
						 | 
				
			
			@ -635,11 +632,27 @@ struct drm_connector {
 | 
			
		|||
	 * Protected by @mutex.
 | 
			
		||||
	 */
 | 
			
		||||
	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 */
 | 
			
		||||
 | 
			
		||||
	/**
 | 
			
		||||
	 * @status:
 | 
			
		||||
	 * One of the drm_connector_status enums (connected, not, or unknown).
 | 
			
		||||
	 * Protected by dev->mode_config.mutex.
 | 
			
		||||
	 */
 | 
			
		||||
	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;
 | 
			
		||||
 | 
			
		||||
	/**
 | 
			
		||||
| 
						 | 
				
			
			@ -648,6 +661,8 @@ struct drm_connector {
 | 
			
		|||
	 * flat panels in embedded systems, the driver should initialize the
 | 
			
		||||
	 * display_info.width_mm and display_info.height_mm fields with the
 | 
			
		||||
	 * physical size of the display.
 | 
			
		||||
	 *
 | 
			
		||||
	 * Protected by dev->mode_config.mutex.
 | 
			
		||||
	 */
 | 
			
		||||
	struct drm_display_info display_info;
 | 
			
		||||
	const struct drm_connector_funcs *funcs;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue