From patchwork Wed Mar 2 13:38:58 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 8480661 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id E8CE4C0553 for ; Wed, 2 Mar 2016 13:39:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B21FE2038F for ; Wed, 2 Mar 2016 13:39:11 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 1DCB220383 for ; Wed, 2 Mar 2016 13:39:06 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DF9C06E867; Wed, 2 Mar 2016 13:39:02 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTP id BC0E66E866; Wed, 2 Mar 2016 13:39:00 +0000 (UTC) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga103.fm.intel.com with ESMTP; 02 Mar 2016 05:39:00 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,528,1449561600"; d="scan'208";a="58359757" Received: from jtconroy-mobl3.ger.corp.intel.com (HELO patser.lan) ([10.252.1.226]) by fmsmga004.fm.intel.com with ESMTP; 02 Mar 2016 05:38:59 -0800 To: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= References: <1456303053-28806-1-git-send-email-maarten.lankhorst@linux.intel.com> <1456303053-28806-6-git-send-email-maarten.lankhorst@linux.intel.com> <20160301172119.GL15993@intel.com> <56D5D551.2000004@linux.intel.com> <20160301175847.GN15993@intel.com> From: Maarten Lankhorst Message-ID: <56D6ECF2.5080009@linux.intel.com> Date: Wed, 2 Mar 2016 14:38:58 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <20160301175847.GN15993@intel.com> Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Subject: [Intel-gfx] [PATCH v2.1 5/6] drm/atomic: Handle encoder assignment conflicts in a separate check, v3. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The current check doesn't handle the case where we don't steal an encoder, but keep it on the current connector. If we repurpose disable_conflicting_encoders to do the checking, we just have to reject the ones that conflict. Changes since v1: - Return early with empty encoder_mask, drm_for_each_connector requires connection_mutex held. Changes since v2: - Add comments to the loops. Signed-off-by: Maarten Lankhorst Testcase: kms_setmode.invalid-clone-single-crtc-stealing diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index db11c2f9b098..bb60148c5c8d 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -86,7 +86,8 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state, } } -static int disable_conflicting_connectors(struct drm_atomic_state *state) +static int handle_conflicting_encoders(struct drm_atomic_state *state, + bool disable_conflicting_encoders) { struct drm_connector_state *conn_state; struct drm_connector *connector; @@ -94,6 +95,11 @@ static int disable_conflicting_connectors(struct drm_atomic_state *state) unsigned encoder_mask = 0; int i, ret; + /* + * First loop, find all newly assigned encoders from the connectors + * part of the state. If the same encoder is assigned to multiple + * connectors bail out. + */ for_each_connector_in_state(state, connector, conn_state, i) { const struct drm_connector_helper_funcs *funcs = connector->helper_private; struct drm_encoder *new_encoder; @@ -106,10 +112,33 @@ static int disable_conflicting_connectors(struct drm_atomic_state *state) else new_encoder = funcs->best_encoder(connector); - if (new_encoder) + if (new_encoder) { + if (encoder_mask & (1 << drm_encoder_index(new_encoder))) { + DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n", + new_encoder->base.id, new_encoder->name, + connector->base.id, connector->name); + + return -EINVAL; + } + encoder_mask |= 1 << drm_encoder_index(new_encoder); + } } + if (!encoder_mask) + return 0; + + /* + * Second loop, iterate over all connectors not part of the state. + * + * If a conflicting encoder is found and disable_conflicting_encoders + * is not set, an error is returned. Userspace can provide a solution + * through the atomic ioctl. + * + * If the flag is set conflicting connectors are removed from the crtc + * and the crtc is disabled if no encoder is left. This preserves + * compatibility with the legacy set_config behavior. + */ drm_for_each_connector(connector, state->dev) { struct drm_crtc_state *crtc_state; @@ -120,6 +149,15 @@ static int disable_conflicting_connectors(struct drm_atomic_state *state) if (!encoder || !(encoder_mask & (1 << drm_encoder_index(encoder)))) continue; + if (!disable_conflicting_encoders) { + DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n", + encoder->base.id, encoder->name, + connector->state->crtc->base.id, + connector->state->crtc->name, + connector->base.id, connector->name); + return -EINVAL; + } + conn_state = drm_atomic_get_connector_state(state, connector); if (IS_ERR(conn_state)) return PTR_ERR(conn_state); @@ -148,26 +186,6 @@ static int disable_conflicting_connectors(struct drm_atomic_state *state) return 0; } -static bool -check_pending_encoder_assignment(struct drm_atomic_state *state, - struct drm_encoder *new_encoder) -{ - struct drm_connector *connector; - struct drm_connector_state *conn_state; - int i; - - for_each_connector_in_state(state, connector, conn_state, i) { - if (conn_state->best_encoder != new_encoder) - continue; - - /* encoder already assigned and we're trying to re-steal it! */ - if (connector->state->best_encoder != conn_state->best_encoder) - return false; - } - - return true; -} - static void set_best_encoder(struct drm_atomic_state *state, struct drm_connector_state *conn_state, @@ -325,13 +343,6 @@ update_connector_routing(struct drm_atomic_state *state, return 0; } - if (!check_pending_encoder_assignment(state, new_encoder)) { - DRM_DEBUG_ATOMIC("Encoder for [CONNECTOR:%d:%s] already assigned\n", - connector->base.id, - connector->name); - return -EINVAL; - } - ret = steal_encoder(state, new_encoder); if (ret) { DRM_DEBUG_ATOMIC("Encoder stealing failed for [CONNECTOR:%d:%s]\n", @@ -510,11 +521,9 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, } } - if (state->legacy_set_config) { - ret = disable_conflicting_connectors(state); - if (ret) - return ret; - } + ret = handle_conflicting_encoders(state, state->legacy_set_config); + if (ret) + return ret; for_each_connector_in_state(state, connector, connector_state, i) { /*