Skip to content

Commit f8df31d

Browse files
Lyudeintel-lab-lkp
authored andcommitted
drm/dp_mst: Introduce drm_dp_mst_connector_atomic_check()
Currently the way that we prevent userspace from performing new modesets on MST connectors that have just been destroyed is rather broken. There's nothing in the actual DRM DP MST topology helpers that checks whether or not a connector still exists, instead each DRM driver does this on it's own, usually by returning NULL from the best_encoder callback which in turn, causes the atomic commit to fail. However, this is wrong in a rather subtle way. If ->best_encoder() returns NULL, this makes ALL modesets involving the connector fail. This includes modesets from userspace that would shut off the CRTCs being used by the connector. Since this results in blocking any changes to a connector's DPMS prop, it has the sideaffect of preventing legacy modesetting users from ever disabling a CRTC that was previously enabled for use in an MST topology. An example of this, where X tries to change the DPMS property of an MST connector that was just detached from the system: [ 2908.320131] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] [CONNECTOR:82:DP-6] [ 2908.320148] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] [CONNECTOR:82:DP-6] status updated from connected to disconnected [ 2908.320166] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] [CONNECTOR:82:DP-6] disconnected [ 2908.320193] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 111 (1) [ 2908.320230] [drm:drm_sysfs_hotplug_event [drm]] generating hotplug event ... [ 2908.638539] [drm:drm_ioctl [drm]] pid=12928, dev=0xe201, auth=1, DRM_IOCTL_MODE_SETPROPERTY [ 2908.638546] [drm:drm_atomic_state_init [drm]] Allocated atomic state 000000007155ba49 [ 2908.638553] [drm:drm_mode_object_get [drm]] OBJ ID: 114 (1) [ 2908.638560] [drm:drm_mode_object_get [drm]] OBJ ID: 108 (1) [ 2908.638568] [drm:drm_atomic_get_crtc_state [drm]] Added [CRTC:41:head-0] 0000000097a6396e state to 000000007155ba49 [ 2908.638575] [drm:drm_atomic_add_affected_connectors [drm]] Adding all current connectors for [CRTC:41:head-0] to 000000007155ba49 [ 2908.638582] [drm:drm_mode_object_get [drm]] OBJ ID: 82 (3) [ 2908.638589] [drm:drm_mode_object_get [drm]] OBJ ID: 82 (4) [ 2908.638596] [drm:drm_atomic_get_connector_state [drm]] Added [CONNECTOR:82:DP-6] 0000000087427144 state to 000000007155ba49 [ 2908.638603] [drm:drm_atomic_check_only [drm]] checking 000000007155ba49 [ 2908.638609] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] [CRTC:41:head-0] active changed [ 2908.638613] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] Updating routing for [CONNECTOR:82:DP-6] [ 2908.638616] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] No suitable encoder found for [CONNECTOR:82:DP-6] [ 2908.638623] [drm:drm_atomic_check_only [drm]] atomic driver check for 000000007155ba49 failed: -22 [ 2908.638630] [drm:drm_atomic_state_default_clear [drm]] Clearing atomic state 000000007155ba49 [ 2908.638637] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 82 (4) [ 2908.638643] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 82 (3) [ 2908.638650] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 114 (2) [ 2908.638656] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 108 (2) [ 2908.638663] [drm:__drm_atomic_state_free [drm]] Freeing atomic state 000000007155ba49 [ 2908.638669] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 82 (2) [ 2908.638676] [drm:drm_ioctl [drm]] pid=12928, ret = -22 While this doesn't usually result in any errors that would be obvious to the user, it does result in us leaving display resources on. This in turn leads to unwanted sideaffects like inactive GPUs being left on (usually from the resulting leaked runtime PM ref). So, provide an easier way of doing this that doesn't require breaking ->best_encoder(): add a common drm_dp_mst_connector_atomic_check() function that DRM drivers can call in order to have CRTC enabling commits fail automatically if the MST port driving the connector no longer exists. We'll also be able to expand upon this later as well once we add MST fallback retraining support. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: stable@vger.kernel.org
1 parent 2693efd commit f8df31d

File tree

2 files changed

+79
-0
lines changed

2 files changed

+79
-0
lines changed

drivers/gpu/drm/drm_dp_mst_topology.c

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3129,6 +3129,82 @@ static const struct drm_private_state_funcs mst_state_funcs = {
31293129
.atomic_destroy_state = drm_dp_mst_destroy_state,
31303130
};
31313131

3132+
static bool
3133+
drm_dp_mst_connector_still_exists(struct drm_connector *connector,
3134+
struct drm_dp_mst_topology_mgr *mgr,
3135+
struct drm_dp_mst_branch *mstb)
3136+
{
3137+
struct drm_dp_mst_port *port;
3138+
bool exists = false;
3139+
3140+
mstb = drm_dp_get_validated_mstb_ref(mgr, mstb);
3141+
if (!mstb)
3142+
return false;
3143+
3144+
list_for_each_entry(port, &mstb->ports, next) {
3145+
port = drm_dp_get_validated_port_ref(mgr, port);
3146+
if (!port)
3147+
continue;
3148+
3149+
exists = (port->connector == connector ||
3150+
(port->mstb &&
3151+
drm_dp_mst_connector_still_exists(connector, mgr,
3152+
port->mstb)));
3153+
3154+
drm_dp_put_port(port);
3155+
if (exists)
3156+
break;
3157+
}
3158+
3159+
drm_dp_put_mst_branch_device(mstb);
3160+
return exists;
3161+
}
3162+
3163+
/**
3164+
* drm_dp_mst_connector_atomic_check - Helper for validating a new atomic
3165+
* state on an MST connector
3166+
* @connector: drm connector
3167+
* @connector_state: the new atomic state of @connector
3168+
* @mgr: the MST topology mgr for @connector
3169+
*
3170+
* This function performs various atomic checks that apply to all drivers
3171+
* using the DRM DP MST helpers. This should be called by all drivers at the
3172+
* start of the atomic_check function for their MST connectors.
3173+
*
3174+
* Return 0 for success, or negative error code on failure.
3175+
*/
3176+
int
3177+
drm_dp_mst_connector_atomic_check(struct drm_connector *connector,
3178+
struct drm_connector_state *connector_state,
3179+
struct drm_dp_mst_topology_mgr *mgr)
3180+
{
3181+
struct drm_atomic_state *state = connector_state->state;
3182+
struct drm_crtc *crtc = connector_state->crtc;
3183+
struct drm_crtc_state *new_crtc_state;
3184+
3185+
if (!crtc)
3186+
return 0;
3187+
3188+
new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
3189+
if (!new_crtc_state)
3190+
return 0;
3191+
3192+
if (!drm_atomic_crtc_needs_modeset(new_crtc_state) ||
3193+
!new_crtc_state->active)
3194+
return 0;
3195+
3196+
/* Make sure that the port for this MST connector still exists */
3197+
if (!drm_dp_mst_connector_still_exists(connector, mgr,
3198+
mgr->mst_primary)) {
3199+
DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] has disappeared from the MST topology\n",
3200+
connector->base.id, connector->name);
3201+
return -EINVAL;
3202+
}
3203+
3204+
return 0;
3205+
}
3206+
EXPORT_SYMBOL(drm_dp_mst_connector_atomic_check);
3207+
31323208
/**
31333209
* drm_atomic_get_mst_topology_state: get MST topology state
31343210
*

include/drm/drm_dp_mst_helper.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,9 @@ void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr);
625625
int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr);
626626
struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state,
627627
struct drm_dp_mst_topology_mgr *mgr);
628+
int drm_dp_mst_connector_atomic_check(struct drm_connector *connector,
629+
struct drm_connector_state *connector_state,
630+
struct drm_dp_mst_topology_mgr *mgr);
628631
int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
629632
struct drm_dp_mst_topology_mgr *mgr,
630633
struct drm_dp_mst_port *port, int pbn);

0 commit comments

Comments
 (0)