From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com ([134.134.136.31]:16833 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751230AbdA2FQB (ORCPT ); Sun, 29 Jan 2017 00:16:01 -0500 Subject: Re: [PATCH 2/4] drm/i915/lspcon: Fix resume time initialization due to unasserted HPD To: imre.deak@intel.com References: <1485509961-9010-1-git-send-email-imre.deak@intel.com> <1485509961-9010-3-git-send-email-imre.deak@intel.com> <20170128081742.GB32065@ideak-desk.fi.intel.com> Cc: intel-gfx@lists.freedesktop.org, Jani Nikula , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Daniel Vetter , stable@vger.kernel.org From: "Sharma, Shashank" Message-ID: <46bb02bb-a555-e2f7-dc80-0d0cdfd8cd45@intel.com> Date: Sun, 29 Jan 2017 10:33:07 +0530 MIME-Version: 1.0 In-Reply-To: <20170128081742.GB32065@ideak-desk.fi.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: Regards Shashank On 1/28/2017 1:47 PM, Imre Deak wrote: > On Sat, Jan 28, 2017 at 10:32:03AM +0530, Sharma, Shashank wrote: >> Regards >> >> Shashank >> >> >> On 1/27/2017 3:09 PM, Imre Deak wrote: >>> During system resume time initialization the HPD level on LSPCON ports >>> can stay low for an extended amount of time, leading to failed AUX >>> transfers and LSPCON initialization. Fix this by waiting for HPD to get >>> asserted. >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99178 >>> Cc: Shashank Sharma >>> Cc: Jani Nikula >>> Cc: Ville Syrj�l� >>> Cc: Daniel Vetter >>> Cc: # v4.9+ >>> Signed-off-by: Imre Deak >>> --- >>> drivers/gpu/drm/i915/intel_dp.c | 4 ++-- >>> drivers/gpu/drm/i915/intel_drv.h | 2 ++ >>> drivers/gpu/drm/i915/intel_lspcon.c | 5 ++++- >>> 3 files changed, 8 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >>> index e0f9b9e..a7785ce 100644 >>> --- a/drivers/gpu/drm/i915/intel_dp.c >>> +++ b/drivers/gpu/drm/i915/intel_dp.c >>> @@ -4400,8 +4400,8 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, >>> * >>> * Return %true if @port is connected, %false otherwise. >>> */ >>> -static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, >>> - struct intel_digital_port *port) >>> +bool intel_digital_port_connected(struct drm_i915_private *dev_priv, >>> + struct intel_digital_port *port) >>> { >>> if (HAS_PCH_IBX(dev_priv)) >>> return ibx_digital_port_connected(dev_priv, port); >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >>> index b71fece..b9cde11 100644 >>> --- a/drivers/gpu/drm/i915/intel_drv.h >>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>> @@ -1489,6 +1489,8 @@ bool __intel_dp_read_desc(struct intel_dp *intel_dp, >>> bool intel_dp_read_desc(struct intel_dp *intel_dp); >>> int intel_dp_link_required(int pixel_clock, int bpp); >>> int intel_dp_max_data_rate(int max_link_clock, int max_lanes); >>> +bool intel_digital_port_connected(struct drm_i915_private *dev_priv, >>> + struct intel_digital_port *port); >>> /* intel_dp_aux_backlight.c */ >>> int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector); >>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c >>> index f6d4e69..c300647 100644 >>> --- a/drivers/gpu/drm/i915/intel_lspcon.c >>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c >>> @@ -158,6 +158,8 @@ static bool lspcon_probe(struct intel_lspcon *lspcon) >>> static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon) >>> { >>> struct intel_dp *intel_dp = lspcon_to_intel_dp(lspcon); >>> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); >>> + struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev); >>> unsigned long start = jiffies; >>> if (!lspcon->desc_valid) >>> @@ -173,7 +175,8 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon) >>> if (!__intel_dp_read_desc(intel_dp, &desc)) >>> return; >>> - if (!memcmp(&intel_dp->desc, &desc, sizeof(desc))) { >>> + if (intel_digital_port_connected(dev_priv, dig_port) && >> when in PCON mode, live_status is always up for LSPCON port, so this check >> will be always true, isn't it ? > Not at least in case of the MegaChips LSPCON I've seen, there it takes a > while for HPD to get asserted. And while it's not asserted AUX > transactions will either: > - return garbage in case of native AUX transactions > - hang the chip/FW until next cold reboot in case of I2C-over-AUX > transactions > - simply not get a reply if the chip/FW initialization has reached a > certain phase > > Based on the DP specification the sink/branch device is not required to > respond in case the HPD is not asserted, so the 3rd scenario is > compliant, but the first two are not. > > In PCON mode after initialization and HPD getting asserted, HPD will > stay asserted except for the short pulses signaling an output > connect/disconnect. The reason might be, that LSPCON resumes in LS type2 adapter state, where the live_status behavior is normal and reflects the real HPD line, but the moment we move to PCON mode, HPD gets asserted low. I know you would have tested this well, but I also want to test this code and logic first, before we go ahead with the patch. Shashank > --Imre > >> - Shashank >>> + !memcmp(&intel_dp->desc, &desc, sizeof(desc))) { >>> DRM_DEBUG_KMS("LSPCON recovering in PCON mode after %u ms\n", >>> jiffies_to_msecs(jiffies - start)); >>> return;