On Tue, Nov 11, 2014 at 10:12:01AM +0100, Daniel Vetter wrote:
Motivated by the per-plane locking I've gone through all the get* ioctls and reduced the locking to the bare minimum required.
v2: Rebase and make it compile ...
Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Just a couple nits.
drivers/gpu/drm/drm_crtc.c | 46 ++++++++++++++++++---------------------------- 1 file changed, 18 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 3652ed8dda80..8850f32994e3 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1743,7 +1743,9 @@ int drm_mode_getresources(struct drm_device *dev, void *data, card_res->count_fbs = fb_count; mutex_unlock(&file_priv->fbs_lock);
- drm_modeset_lock_all(dev);
/* mode_config.mutex protects the connector list against e.g. DP MST
* connector hot-adding. CRTC/Plane lists are invariant. */mutex_lock(&dev->mode_config.mutex); if (!drm_is_primary_client(file_priv)) {
mode_group = NULL;
@@ -1863,7 +1865,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data, card_res->count_connectors, card_res->count_encoders);
out:
- drm_modeset_unlock_all(dev);
- mutex_unlock(&dev->mode_config.mutex); return ret;
}
@@ -1890,14 +1892,11 @@ int drm_mode_getcrtc(struct drm_device *dev, if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL;
- drm_modeset_lock_all(dev);
- crtc = drm_crtc_find(dev, crtc_resp->crtc_id);
- if (!crtc) {
ret = -ENOENT;goto out;- }
if (!crtc)
return -ENOENT;drm_modeset_lock_crtc(crtc, crtc->primary); crtc_resp->x = crtc->x; crtc_resp->y = crtc->y; crtc_resp->gamma_size = crtc->gamma_size;
@@ -1914,9 +1913,8 @@ int drm_mode_getcrtc(struct drm_device *dev, } else { crtc_resp->mode_valid = 0; }
- drm_modeset_unlock_crtc(crtc);
-out:
- drm_modeset_unlock_all(dev); return ret;
}
@@ -2100,24 +2098,22 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL;
- drm_modeset_lock_all(dev); encoder = drm_encoder_find(dev, enc_resp->encoder_id);
- if (!encoder) {
ret = -ENOENT;goto out;- }
if (!encoder)
return -ENOENT;drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); if (encoder->crtc) enc_resp->crtc_id = encoder->crtc->base.id; else enc_resp->crtc_id = 0;
drm_modeset_unlock(&dev->mode_config.connection_mutex);
enc_resp->encoder_type = encoder->encoder_type; enc_resp->encoder_id = encoder->base.id; enc_resp->possible_crtcs = encoder->possible_crtcs; enc_resp->possible_clones = encoder->possible_clones;
-out:
- drm_modeset_unlock_all(dev); return ret;
}
@@ -2147,7 +2143,6 @@ int drm_mode_getplane_res(struct drm_device *dev, void *data, if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL;
- drm_modeset_lock_all(dev); config = &dev->mode_config;
I'd feel better if you added a comment here similar to the one above explaining that the plane_list cannot change and thus accessing it does not require locking config->mutex.
if (file_priv->universal_planes) @@ -2182,7 +2177,6 @@ int drm_mode_getplane_res(struct drm_device *dev, void *data, plane_resp->count_planes = num_planes;
out:
I think you can just remove this label entirely and return straight from the failure cases.
- drm_modeset_unlock_all(dev); return ret;
}
@@ -2210,13 +2204,11 @@ int drm_mode_getplane(struct drm_device *dev, void *data, if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL;
- drm_modeset_lock_all(dev); plane = drm_plane_find(dev, plane_resp->plane_id);
- if (!plane) {
ret = -ENOENT;goto out;- }
if (!plane)
return -ENOENT;drm_modeset_lock(&plane->mutex, NULL); if (plane->crtc) plane_resp->crtc_id = plane->crtc->base.id; else
@@ -2226,6 +2218,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data, plane_resp->fb_id = plane->fb->base.id; else plane_resp->fb_id = 0;
drm_modeset_unlock(&plane->mutex);
plane_resp->plane_id = plane->base.id; plane_resp->possible_crtcs = plane->possible_crtcs;
@@ -2241,14 +2234,11 @@ int drm_mode_getplane(struct drm_device *dev, void *data, if (copy_to_user(format_ptr, plane->format_types, sizeof(uint32_t) * plane->format_count)) {
ret = -EFAULT;goto out;
return -EFAULT;-out:
- drm_modeset_unlock_all(dev); return ret;
}
-- 2.1.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx