Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 4 Apr 2018 11:44:36 -0700
From:   Manasi Navare <manasi.d.navare@intel.com>
To:     Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
Cc:     Lyude Paul <lyude@redhat.com>, dri-devel@lists.freedesktop.org,
        David Airlie <airlied@linux.ie>,
        intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>, stable@vger.kernel.org,
        Laura Abbott <labbott@redhat.com>
Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Keep AUX block running when
 disabling DPMS for MST
Message-ID: <20180404184435.GA2804@intel.com>
References: <20180402212142.19841-1-lyude@redhat.com>
 <20180402212617.21247-1-lyude@redhat.com>
 <20180404153429.GE5453@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20180404153429.GE5453@intel.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Apr 04, 2018 at 06:34:29PM +0300, Ville Syrj?l? wrote:
> On Mon, Apr 02, 2018 at 05:26:16PM -0400, Lyude Paul wrote:
> > While enabling/disabling DPMS before link training with MST hubs is
> > perfectly valid; unfortunately disabling DPMS results in some devices
> > disabling their AUX CH block as well. For SST this isn't as much of a
> > problem, but for MST we need to be able to continue handling aux
> > transactions even when none of the sinks are turned on since it's
> > possible for us to have a single atomic commit which results in
> > disabling each downstream sink, followed by subsequently re-enabling
> > each sink.
> > 
> > If we don't do this, we'll end up stalling any pending ESI interrupts
> > from the sink for up to 1ms. Unfortunately, dropping ESIs during this
> > timespan makes it so that link fallback retraining for MST (which I will
> > be submitting to the ML shortly) fails due to the channel EQ failure
> > interrupts potentially getting dropped. Additionally, when performing a
> > modeset that brings the hub status's link status from bad -> good having
> > ESIs disabled for that long causes us to miss the hub's response to us
> > trying to start link training as well.
> > 
> > Since any sink with MST is going to support DisplayPort 1.2 anyway, save
> > us the hassle of trying to wait until the sink comes back up and just
> > never shut the aux block down.
> > 
> > Changes since v2:
> >  - Fix patch name, no functional changes
> > 
> > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > Cc: Laura Abbott <labbott@redhat.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Ville Syrj?l? <ville.syrjala@linux.intel.com>
> > Cc: stable@vger.kernel.org
> > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.")
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 62f82c4298ac..0479c377981b 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> >  		return;
> >  
> >  	if (mode != DRM_MODE_DPMS_ON) {
> > +		unsigned char data = intel_dp->is_mst ?
> > +			DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
> 
> This smells like a workaround for an actual bug somewhere. Why exactly
> is the slower wakeup or the AUX block a problem for MST but not for SST
> when the link training is exactly the same for SST and MST?
>

The problem occurs only in case of MST because the Channel EQ failure is notified
through ESI sideband AUX messages which potentially  can get dropped because of disabling
DPMS. But in case of SST, we detect the channel EQ failure write during the EQ TPS sequence
which happens on the main link.

Manasi
 
> > +
> >  		if (downstream_hpd_needs_d0(intel_dp))
> >  			return;
> >  
> > -		ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > -					 DP_SET_POWER_D3);
> > +		ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, data);
> >  	} else {
> >  		struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
> >  
> > -- 
> > 2.14.3
> 
> -- 
> Ville Syrj?l?
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx