From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 29B9FC433EF for ; Thu, 10 Mar 2022 21:52:23 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A89C310E231; Thu, 10 Mar 2022 21:52:22 +0000 (UTC) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3CB3610E231 for ; Thu, 10 Mar 2022 21:52:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1646949141; x=1678485141; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=tCRGBc6LJ8SlhUZkciA10/6hPJd7oqtLeqbF8yO6Mh8=; b=bkmyaHPlsDxkbqDRuS02TnuSE50soXUDMsI2Otvjssq/b4Vx9ZflApzU QvP471NTpaU9lTnS2d3WjZoj+6vJDWp0mcv4cXW4TL5+gpCUuXja0QROA 45Uit5gtX93y9SVBf/v1g+zSveLkP0ZYTMpoEKb0ysBkU1ithXoitRRTV sV3gX6AIf+QuxCBL9ATh1U8Oq+ZUFW7mpPF2IcU6FvUUXTfxxIsPJU+nN R6P4hjbSVyDW155HxtRb36hPZjo6pDOCHOnSkxYmL8vKKQxrYisv1x8lT D49V+FDQzjJT8oVBmzWPKIrAH8vVrfGrtJLg9dlJuyduAKEHVTmkbyr7E A==; X-IronPort-AV: E=McAfee;i="6200,9189,10282"; a="255338511" X-IronPort-AV: E=Sophos;i="5.90,171,1643702400"; d="scan'208";a="255338511" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Mar 2022 13:52:20 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.90,171,1643702400"; d="scan'208";a="554865670" Received: from stinkpipe.fi.intel.com (HELO stinkbox) ([10.237.72.151]) by orsmga008.jf.intel.com with SMTP; 10 Mar 2022 13:52:17 -0800 Received: by stinkbox (sSMTP sendmail emulation); Thu, 10 Mar 2022 23:52:17 +0200 Date: Thu, 10 Mar 2022 23:52:17 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: =?iso-8859-1?Q?Jos=E9?= Roberto de Souza Message-ID: References: <20220310200518.247909-1-jose.souza@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20220310200518.247909-1-jose.souza@intel.com> X-Patchwork-Hint: comment Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Fix HPD short pulse handling for eDP X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jani Nikula , intel-gfx@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Thu, Mar 10, 2022 at 12:05:17PM -0800, José Roberto de Souza wrote: > Commit 13ea6db2cf24 ("drm/i915/edp: Ignore short pulse when panel > powered off") completely broke short pulse handling for eDP as it is > usually generated by sink when it is displaying image and there is > some error or status that source needs to handle. > > When power panel is enabled, this state is enough to power aux > transactions and VDD override is disabled, so intel_pps_have_power() > is always returning false causing short pulses to be ignored. I think the times that we use the vdd override should be limited to: - aux transfers while the display off - potentially short periods of time during the modeset sequence So I guess what you're saying here is that during those times some panel is triggering an IRQ_HPD which, if ignored, causes some problem for us? > > So here better naming this function that intends to check if aux > lines are powered to avoid the endless cycle mentioned in the commit > being fixed and fixing the check for what it is intended. > > Fixes: 13ea6db2cf24 ("drm/i915/edp: Ignore short pulse when panel powered off") > Cc: Anshuman Gupta > Cc: Jani Nikula > Cc: Uma Shankar > Cc: Ville Syrjälä > Signed-off-by: José Roberto de Souza > --- > drivers/gpu/drm/i915/display/intel_dp.c | 2 +- > drivers/gpu/drm/i915/display/intel_pps.c | 4 ++-- > drivers/gpu/drm/i915/display/intel_pps.h | 2 +- > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 619546441eae5..b029b064000d6 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -4867,7 +4867,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *dig_port, bool long_hpd) > struct intel_dp *intel_dp = &dig_port->dp; > > if (dig_port->base.type == INTEL_OUTPUT_EDP && > - (long_hpd || !intel_pps_have_power(intel_dp))) { > + (long_hpd || !intel_pps_have_vdd_power(intel_dp))) { > /* > * vdd off can generate a long/short pulse on eDP which > * would require vdd on to handle it, and thus we > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c > index 9c986e8932f87..d3e6083ad5b79 100644 > --- a/drivers/gpu/drm/i915/display/intel_pps.c > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > @@ -1075,13 +1075,13 @@ static void intel_pps_vdd_sanitize(struct intel_dp *intel_dp) > edp_panel_vdd_schedule_off(intel_dp); > } > > -bool intel_pps_have_power(struct intel_dp *intel_dp) > +bool intel_pps_have_vdd_power(struct intel_dp *intel_dp) > { > intel_wakeref_t wakeref; > bool have_power = false; > > with_intel_pps_lock(intel_dp, wakeref) { > - have_power = edp_have_panel_power(intel_dp) && > + have_power = edp_have_panel_power(intel_dp) || > edp_have_panel_vdd(intel_dp); > } > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.h b/drivers/gpu/drm/i915/display/intel_pps.h > index fbb47f6f453e4..948523ce32417 100644 > --- a/drivers/gpu/drm/i915/display/intel_pps.h > +++ b/drivers/gpu/drm/i915/display/intel_pps.h > @@ -37,7 +37,7 @@ void intel_pps_vdd_on(struct intel_dp *intel_dp); > void intel_pps_on(struct intel_dp *intel_dp); > void intel_pps_off(struct intel_dp *intel_dp); > void intel_pps_vdd_off_sync(struct intel_dp *intel_dp); > -bool intel_pps_have_power(struct intel_dp *intel_dp); > +bool intel_pps_have_vdd_power(struct intel_dp *intel_dp); > void intel_pps_wait_power_cycle(struct intel_dp *intel_dp); > > void intel_pps_init(struct intel_dp *intel_dp); > -- > 2.35.1 -- Ville Syrjälä Intel