From patchwork Thu Nov 25 10:10:54 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chris Chiu X-Patchwork-Id: 1559626 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=DMllCni1; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4J0DFC3SgPz9s5P for ; Thu, 25 Nov 2021 21:11:59 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1mqBjF-0007RR-FE; Thu, 25 Nov 2021 10:11:45 +0000 Received: from smtp-relay-internal-0.internal ([10.131.114.225] helo=smtp-relay-internal-0.canonical.com) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1mqBij-00065q-Tk for kernel-team@lists.ubuntu.com; Thu, 25 Nov 2021 10:11:14 +0000 Received: from mail-pf1-f197.google.com (mail-pf1-f197.google.com [209.85.210.197]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id 4630C4001F for ; Thu, 25 Nov 2021 10:11:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1637835072; bh=3eTULlF90l7HAw7/ZtPqcskwMiH5QyFLeXJ+0N8QMxQ=; h=From:To:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version:Content-Type; b=DMllCni1riomgNWuaTbpR1TK63FZmiKG6sCZ2jq3jCevmOVDkOowy/0cnQLD13r4F 1AoRUX3G92Cl94lDvstJKhz9/cW1okDa2rFgEH7zAQoi2q1JI5GywX7JhzxEvidtoz UGgg7/RSzDeKYQb922hXWZlumXyLrHvnxgPxxD8w5hHcJ9cNnSjjVVObT5J+jWL05R s18hd5Pad37P1sVsTVWsY0Pnt2fkWsdIsPC7NVrRPxwByEwT82LxMWAOjHVsGL0m+A BJdS2v3sOcOIUKVvYkckf0WdvMVNBHHTW1ngX7p6b/HtACu3N1tVhr6ljI+neZVtGL ls5Lt2diMxoRA== Received: by mail-pf1-f197.google.com with SMTP id q19-20020a056a00151300b004a4ca777976so3130783pfu.19 for ; Thu, 25 Nov 2021 02:11:12 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=3eTULlF90l7HAw7/ZtPqcskwMiH5QyFLeXJ+0N8QMxQ=; b=KeZNJxC6pqedTC5xEaVGu9XrM6ilYdXGqUzwIsxl0jqkmGfXxNzqdhG4UwrYXbkkGu j2acyL894kd89pkafViASy0tOVzg/RE8m5UjDBCukYcONFl58kPOj3n3rLDbOxhhe83B s42LTZE4LNCQfDKkdolgJ84H9LsCahCa466HoBmgMnnIQFAbbImer2kIClR57tFZxk9O JZMuFjbJ4llWIyB4oPHsLKp5fzVt4MTqNYvP7nNFFXazo+QY7hD0PByFOLdMEP3dKVpv /DJL+9eIdLw/EHFHKSEk1FZWnBos51IflCMuauDAD6DRMDNUOEK3bPP46m3lt7nlmNcF ixwQ== X-Gm-Message-State: AOAM530Xn51hhbj4TyT9elXNj1BlXddWuurc6ytNe3uVKR4lf1KqVx7N k4/il301RE1Ud0qTb4DH5JKg1GDCGeYXdmf/SfW1jEIr+AaVc8HYka0HoTu+vJJsl1y4eQB6lSL ZFHHwSjThovatsFFuIcKrkO57v+lLfH6HILluYd3YjA== X-Received: by 2002:a05:6a00:1816:b0:49f:cd0d:b51b with SMTP id y22-20020a056a00181600b0049fcd0db51bmr13168213pfa.6.1637835070677; Thu, 25 Nov 2021 02:11:10 -0800 (PST) X-Google-Smtp-Source: ABdhPJwrDJN2Y/xBstezvymKqO0dekxlJn6MprbCp0xhgA+4GKWJgILUm2dhySx+8VZ7AoUIS1x1nQ== X-Received: by 2002:a05:6a00:1816:b0:49f:cd0d:b51b with SMTP id y22-20020a056a00181600b0049fcd0db51bmr13168163pfa.6.1637835070151; Thu, 25 Nov 2021 02:11:10 -0800 (PST) Received: from localhost.localdomain (111-240-133-170.dynamic-ip.hinet.net. [111.240.133.170]) by smtp.gmail.com with ESMTPSA id il7sm2554831pjb.54.2021.11.25.02.11.09 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Nov 2021 02:11:09 -0800 (PST) From: Chris Chiu To: kernel-team@lists.ubuntu.com Subject: [PATCH 10/12][SRU][Jammy/Unstable] drm/i915/tc: Fix TypeC PHY connect/disconnect logic on ADL-P Date: Thu, 25 Nov 2021 18:10:54 +0800 Message-Id: <20211125101056.20316-11-chris.chiu@canonical.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20211125101056.20316-1-chris.chiu@canonical.com> References: <20211125101056.20316-1-chris.chiu@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Imre Deak BugLink: https://bugs.launchpad.net/bugs/1952041 So far TC-cold was blocked only for the duration of TypeC mode resets. The DP-alt and legacy modes require TC-cold to be blocked also whenever the port is in use (AUX transfers, enable modeset), and this was ensured by the held PHY ownership flag. On ADL-P this doesn't work, since the PHY ownership flag is in a register backed by the PW#2 power well. Whenever this power well is disabled the ownership flag is cleared by the HW under the driver. The only way to cleanly release and re-acquire the PHY ownership flag and also allow for power saving (by disabling the display power wells and reaching DC5/6 states) is to hold the TC-cold blocking power domains while the PHY is connected and disconnect/reconnect the PHY on-demand around AUX transfers and modeset enable/disables. Let's do that, disconnecting a PHY with a 1 sec delay after it becomes idle. For consistency do this on all platforms and TypeC modes. v2: Add tc_mode!=disconnected and phy_is_owned asserts to __intel_tc_port_lock(). Cc: José Roberto de Souza Signed-off-by: Imre Deak Reviewed-by: José Roberto de Souza Link: https://patchwork.freedesktop.org/patch/msgid/20210929132833.2253961-6-imre.deak@intel.com (cherry picked from commit 3e0abc7661c82266d3a3f27fbbadcc74cc4997e1) Signed-off-by: Chris Chiu --- drivers/gpu/drm/i915/display/intel_ddi.c | 7 +- .../drm/i915/display/intel_display_types.h | 1 + drivers/gpu/drm/i915/display/intel_tc.c | 81 ++++++++++++------- drivers/gpu/drm/i915/display/intel_tc.h | 2 +- 4 files changed, 59 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 485a74ae0545..0862963be82c 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -4036,8 +4036,11 @@ static void intel_ddi_encoder_destroy(struct drm_encoder *encoder) { struct drm_i915_private *i915 = to_i915(encoder->dev); struct intel_digital_port *dig_port = enc_to_dig_port(to_intel_encoder(encoder)); + enum phy phy = intel_port_to_phy(i915, dig_port->base.port); intel_dp_encoder_flush_work(encoder); + if (intel_phy_is_tc(i915, phy)) + intel_tc_port_flush_work(dig_port); intel_display_power_flush_work(i915); drm_encoder_cleanup(encoder); @@ -4480,7 +4483,7 @@ static void intel_ddi_encoder_suspend(struct intel_encoder *encoder) if (!intel_phy_is_tc(i915, phy)) return; - intel_tc_port_disconnect_phy(dig_port); + intel_tc_port_flush_work(dig_port); } static void intel_ddi_encoder_shutdown(struct intel_encoder *encoder) @@ -4496,7 +4499,7 @@ static void intel_ddi_encoder_shutdown(struct intel_encoder *encoder) if (!intel_phy_is_tc(i915, phy)) return; - intel_tc_port_disconnect_phy(dig_port); + intel_tc_port_flush_work(dig_port); } #define port_tc_name(port) ((port) - PORT_TC1 + '1') diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 025c58c2f47c..68fcb48f195a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1655,6 +1655,7 @@ struct intel_digital_port { struct mutex tc_lock; /* protects the TypeC port mode */ intel_wakeref_t tc_lock_wakeref; enum intel_display_power_domain tc_lock_power_domain; + struct delayed_work tc_disconnect_phy_work; int tc_link_refcount; bool tc_legacy_port:1; char tc_port_name[8]; diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 9367770de5d7..66cb321a4488 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -666,6 +666,13 @@ static void intel_tc_port_update_mode(struct intel_digital_port *dig_port, intel_tc_port_reset_mode(dig_port, required_lanes, force_disconnect); + /* Get power domain matching the new mode after reset. */ + tc_cold_unblock(dig_port, dig_port->tc_lock_power_domain, + fetch_and_zero(&dig_port->tc_lock_wakeref)); + if (dig_port->tc_mode != TC_PORT_DISCONNECTED) + dig_port->tc_lock_wakeref = tc_cold_block(dig_port, + &dig_port->tc_lock_power_domain); + tc_cold_unblock(dig_port, domain, wref); } @@ -693,6 +700,7 @@ void intel_tc_port_sanitize(struct intel_digital_port *dig_port) active_links = to_intel_crtc(encoder->base.crtc)->active; drm_WARN_ON(&i915->drm, dig_port->tc_mode != TC_PORT_DISCONNECTED); + drm_WARN_ON(&i915->drm, dig_port->tc_lock_wakeref); if (active_links) { enum intel_display_power_domain domain; intel_wakeref_t tc_cold_wref = tc_cold_block(dig_port, &domain); @@ -705,6 +713,9 @@ void intel_tc_port_sanitize(struct intel_digital_port *dig_port) dig_port->tc_port_name, active_links); intel_tc_port_link_init_refcount(dig_port, active_links); + dig_port->tc_lock_wakeref = tc_cold_block(dig_port, + &dig_port->tc_lock_power_domain); + tc_cold_unblock(dig_port, domain, tc_cold_wref); } @@ -745,56 +756,67 @@ bool intel_tc_port_connected(struct intel_encoder *encoder) } static void __intel_tc_port_lock(struct intel_digital_port *dig_port, - int required_lanes, bool force_disconnect) + int required_lanes) { struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); - intel_wakeref_t wakeref; - - wakeref = intel_display_power_get(i915, POWER_DOMAIN_DISPLAY_CORE); mutex_lock(&dig_port->tc_lock); + cancel_delayed_work(&dig_port->tc_disconnect_phy_work); if (!dig_port->tc_link_refcount) intel_tc_port_update_mode(dig_port, required_lanes, - force_disconnect); + false); - drm_WARN_ON(&i915->drm, dig_port->tc_lock_wakeref); - dig_port->tc_lock_wakeref = wakeref; + drm_WARN_ON(&i915->drm, dig_port->tc_mode == TC_PORT_DISCONNECTED); + drm_WARN_ON(&i915->drm, dig_port->tc_mode != TC_PORT_TBT_ALT && + !tc_phy_is_owned(dig_port)); } void intel_tc_port_lock(struct intel_digital_port *dig_port) { - __intel_tc_port_lock(dig_port, 1, false); + __intel_tc_port_lock(dig_port, 1); } -void intel_tc_port_unlock(struct intel_digital_port *dig_port) +/** + * intel_tc_port_disconnect_phy_work: disconnect TypeC PHY from display port + * @dig_port: digital port + * + * Disconnect the given digital port from its TypeC PHY (handing back the + * control of the PHY to the TypeC subsystem). This will happen in a delayed + * manner after each aux transactions and modeset disables. + */ +static void intel_tc_port_disconnect_phy_work(struct work_struct *work) { - struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); - intel_wakeref_t wakeref = fetch_and_zero(&dig_port->tc_lock_wakeref); + struct intel_digital_port *dig_port = + container_of(work, struct intel_digital_port, tc_disconnect_phy_work.work); - mutex_unlock(&dig_port->tc_lock); + mutex_lock(&dig_port->tc_lock); - intel_display_power_put_async(i915, POWER_DOMAIN_DISPLAY_CORE, - wakeref); + if (!dig_port->tc_link_refcount) + intel_tc_port_update_mode(dig_port, 1, true); + + mutex_unlock(&dig_port->tc_lock); } /** - * intel_tc_port_disconnect_phy: disconnect TypeC PHY from display port + * intel_tc_port_flush_work: flush the work disconnecting the PHY * @dig_port: digital port * - * Disconnect the given digital port from its TypeC PHY (handing back the - * control of the PHY to the TypeC subsystem). The only purpose of this - * function is to force the disconnect even with a TypeC display output still - * plugged to the TypeC connector, which is required by the TypeC firmwares - * during system suspend and shutdown. Otherwise - during the unplug event - * handling - the PHY ownership is released automatically by - * intel_tc_port_reset_mode(), when calling this function is not required. + * Flush the delayed work disconnecting an idle PHY. */ -void intel_tc_port_disconnect_phy(struct intel_digital_port *dig_port) +void intel_tc_port_flush_work(struct intel_digital_port *dig_port) { - __intel_tc_port_lock(dig_port, 1, true); - intel_tc_port_unlock(dig_port); + flush_delayed_work(&dig_port->tc_disconnect_phy_work); +} + +void intel_tc_port_unlock(struct intel_digital_port *dig_port) +{ + if (!dig_port->tc_link_refcount && dig_port->tc_mode != TC_PORT_DISCONNECTED) + queue_delayed_work(system_unbound_wq, &dig_port->tc_disconnect_phy_work, + msecs_to_jiffies(1000)); + + mutex_unlock(&dig_port->tc_lock); } bool intel_tc_port_ref_held(struct intel_digital_port *dig_port) @@ -806,16 +828,16 @@ bool intel_tc_port_ref_held(struct intel_digital_port *dig_port) void intel_tc_port_get_link(struct intel_digital_port *dig_port, int required_lanes) { - __intel_tc_port_lock(dig_port, required_lanes, false); + __intel_tc_port_lock(dig_port, required_lanes); dig_port->tc_link_refcount++; intel_tc_port_unlock(dig_port); } void intel_tc_port_put_link(struct intel_digital_port *dig_port) { - mutex_lock(&dig_port->tc_lock); - dig_port->tc_link_refcount--; - mutex_unlock(&dig_port->tc_lock); + intel_tc_port_lock(dig_port); + --dig_port->tc_link_refcount; + intel_tc_port_unlock(dig_port); } static bool @@ -871,6 +893,7 @@ void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy) "%c/TC#%d", port_name(port), tc_port + 1); mutex_init(&dig_port->tc_lock); + INIT_DELAYED_WORK(&dig_port->tc_disconnect_phy_work, intel_tc_port_disconnect_phy_work); dig_port->tc_legacy_port = is_legacy; dig_port->tc_mode = TC_PORT_DISCONNECTED; dig_port->tc_link_refcount = 0; diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h index 0fdcddb4fc87..6b47b29f551c 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.h +++ b/drivers/gpu/drm/i915/display/intel_tc.h @@ -17,7 +17,6 @@ bool intel_tc_port_in_dp_alt_mode(struct intel_digital_port *dig_port); bool intel_tc_port_in_legacy_mode(struct intel_digital_port *dig_port); bool intel_tc_port_connected(struct intel_encoder *encoder); -void intel_tc_port_disconnect_phy(struct intel_digital_port *dig_port); u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port); u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port); @@ -28,6 +27,7 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port, void intel_tc_port_sanitize(struct intel_digital_port *dig_port); void intel_tc_port_lock(struct intel_digital_port *dig_port); void intel_tc_port_unlock(struct intel_digital_port *dig_port); +void intel_tc_port_flush_work(struct intel_digital_port *dig_port); void intel_tc_port_get_link(struct intel_digital_port *dig_port, int required_lanes); void intel_tc_port_put_link(struct intel_digital_port *dig_port);