From abd69c55dd8f1f71b33b8c6165217f4329db8f25 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 25 Nov 2014 23:50:05 +0100 Subject: [PATCH] drm: Handle atomic state properly in kms getfoo ioctl So the problem with async commit (especially async modeset commit) is that the legacy pointers only get updated after the point of no return, in the async part of the modeset sequence. At least as implemented by the current helper functions. This is done in the set_routing_links function in drm_atomic_helper.c. Which also means that access isn't protected by locks but only coordinated by synchronizing with async workers. No problem thus far, until we lock at the getconnector/encoder ioctls. So fix this up by adding special cases for atomic drivers: For those we need to look at state objects. Unfortunately digging out the correct encoder->crtc link is a bit of work, so wrap this up in a helper function. Moving the assignments of connector->encoder and encoder->crtc earlier isn't a good idea because the point of the atomic helpers is that we stage the state updates. That way the disable functions can still inspect the links and rely upon them. v2: Extract full encoder->crtc lookup into helper (Rob). v3: Extract drm_connector_get_encoder too since - we need to always return state->best_encoder when there is a state otherwise we might return stale data if there's a pending async disable (and chase unlocked pointers, too). Same issue with encoder_get_crtc but there it's a bit more tricky to handle. Cc: Rob Clark Cc: Sean Paul Reviewed-by: Sean Paul Lightly-Tested-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 49 +++++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 3fd85772afb8..de79283eaea7 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1968,6 +1968,15 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode, return true; } +static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *connector) +{ + /* For atomic drivers only state objects are synchronously updated and + * protected by modeset locks, so check those first. */ + if (connector->state) + return connector->state->best_encoder; + return connector->encoder; +} + /** * drm_mode_getconnector - get connector configuration * @dev: drm device for the ioctl @@ -1986,6 +1995,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, { struct drm_mode_get_connector *out_resp = data; struct drm_connector *connector; + struct drm_encoder *encoder; struct drm_display_mode *mode; int mode_count = 0; int props_count = 0; @@ -2041,8 +2051,10 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, out_resp->subpixel = connector->display_info.subpixel_order; out_resp->connection = connector->status; drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); - if (connector->encoder) - out_resp->encoder_id = connector->encoder->base.id; + + encoder = drm_connector_get_encoder(connector); + if (encoder) + out_resp->encoder_id = encoder->base.id; else out_resp->encoder_id = 0; drm_modeset_unlock(&dev->mode_config.connection_mutex); @@ -2112,6 +2124,33 @@ out: return ret; } +static struct drm_crtc *drm_encoder_get_crtc(struct drm_encoder *encoder) +{ + struct drm_connector *connector; + struct drm_device *dev = encoder->dev; + bool uses_atomic = false; + + /* For atomic drivers only state objects are synchronously updated and + * protected by modeset locks, so check those first. */ + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { + if (!connector->state) + continue; + + uses_atomic = true; + + if (connector->state->best_encoder != encoder) + continue; + + return connector->state->crtc; + } + + /* Don't return stale data (e.g. pending async disable). */ + if (uses_atomic) + return NULL; + + return encoder->crtc; +} + /** * drm_mode_getencoder - get encoder configuration * @dev: drm device for the ioctl @@ -2130,6 +2169,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, { struct drm_mode_get_encoder *enc_resp = data; struct drm_encoder *encoder; + struct drm_crtc *crtc; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; @@ -2139,7 +2179,10 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, return -ENOENT; drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); - if (encoder->crtc) + crtc = drm_encoder_get_crtc(encoder); + if (crtc) + enc_resp->crtc_id = crtc->base.id; + else if (encoder->crtc) enc_resp->crtc_id = encoder->crtc->base.id; else enc_resp->crtc_id = 0; -- 2.20.1