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 <robdclark@gmail.com>
Cc: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Lightly-Tested-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 3fd8577..de79283 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1968,6 +1968,15 @@
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 @@
{
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 @@
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 @@
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 @@
{
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 @@
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;