Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/mst: Do not retrain new links

2018-07-18 Thread Rodrigo Vivi
On Wed, Jul 18, 2018 at 02:30:18PM -0700, Dhinakaran Pandiyan wrote:
> On Wed, 2018-07-18 at 13:31 -0700, Manasi Navare wrote:
> > On Wed, Jul 18, 2018 at 01:34:12PM -0700, Dhinakaran Pandiyan wrote:
> > > 
> > > On Wed, 2018-07-18 at 10:45 -0700, Manasi Navare wrote:
> > > > 
> > > > On Wed, Jul 18, 2018 at 10:19:42AM -0700, Dhinakaran Pandiyan
> > > > wrote:
> > > > > 
> > > > > 
> > > > > The short pulse handler checks if channel equalization is okay
> > > > > and
> > > > > goes onto retrain a link if there are active MST links. This
> > > > > retraining
> > > > > path is not meant for new MST connections, but due to a bug
> > > > > elsewhere, if
> > > > > active_mst_links is < 0 the boolean check for active_mst_links
> > > > > passes and
> > > > This bug is probably around the way we track the active_mst_links
> > > > and
> > > > we are
> > > > probably decrementing it more times than the available links
> > > Yeah, that indeed is one aspect of the bug.
> > > 
> > > > 
> > > >  and since its an int
> > > > variable it goes to negative which is not the expected behaviour.
> > > > Why not change this active_mst_links variable to be an unsigned
> > > > int
> > > > since
> > > > we do not expect any negative values for this anyways.
> > > > That way we can still check against just intel_dp-
> > > > >active_mst_links
> > > > as opposed checking
> > > > for it being greater than 0 and would also not need a WARN_ON
> > > > here.
> > > I did not get this, mind sharing code diff?
> > My question was if negative values for active_mst_links are never
> > allowed
> > then why cant we define it as an unsigned int and avoid thrwoing a
> > Warning later.
> Hmm. I still do not get how defining it an unsigned int will prevent
> the decrement op from making intel_dp->active_mst_links == true.
> 
> > 
> > Also I think the following check can be added in
> > intel_mst_post_disable_dp():
> > 
> > if (intel_dp->active_mst_links)
> > intel_dp->active_mst_link--;
> 
> That's just band-aid, we will still go through the post_disable
> sequence while not decrementing ->active_mst_links.

yeap, we don't want to hide the other bug, but prevent the current
worst case while we warn the existence of the other bug.

So,

Reviewed-by: Rodrigo Vivi 

> 
> 
> > 
> > to avoid getting negative values in the first place.
> > 
> > Manasi
> > 
> > > 
> > > 
> > > -DK
> > > 
> > > > 
> > > > 
> > > > Manasi
> > > > 
> > > > > 
> > > > > 
> > > > > we proceed to retrain a new link. This results in a sequence of
> > > > > failed link
> > > > > training attempts, most likely due to the hardware not setup
> > > > > for
> > > > > link
> > > > > training at that point i.e., missing the DDI pre_enable
> > > > > sequence.
> > > > > 
> > > > > [   80.301272] [drm:intel_dp_check_mst_status] channel EQ not
> > > > > ok,
> > > > > retraining
> > > > > [   80.301312] [drm:intel_ddi_prepare_link_retrain] *ERROR*
> > > > > Timeout
> > > > > waiting for DDI BUF C idle bit
> > > > > 
> > > > > The above error gives us a hint something went wrong before
> > > > > link
> > > > > training started.
> > > > > 
> > > > > Check for a positive value of active_mst_links and throw in a
> > > > > warning for
> > > > > invalid active_mst_links as debug aid.
> > > > > 
> > > > > Cc: Nathan Ciobanu 
> > > > > Cc: Rodrigo Vivi 
> > > > > Signed-off-by: Dhinakaran Pandiyan  > > > > om>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index b45b08420c1f..2d61ff01cf51 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -4213,12 +4213,14 @@ intel_dp_check_mst_status(struct
> > > > > intel_dp
> > > > > *intel_dp)
> > > > >   int ret = 0;
> > > > >   int retry;
> > > > >   bool handled;
> > > > > +
> > > > > + WARN_ON_ONCE(intel_dp->active_mst_links < 0);
> > > > >   bret = intel_dp_get_sink_irq_esi(intel_dp,
> > > > > esi);
> > > > >  go_again:
> > > > >   if (bret == true) {
> > > > >  
> > > > >   /* check link status - esi[10] =
> > > > > 0x200c */
> > > > > - if (intel_dp->active_mst_links &&
> > > > > + if (intel_dp->active_mst_links > 0 &&
> > > > >   !drm_dp_channel_eq_ok([10],
> > > > > intel_dp->lane_count)) {
> > > > >   DRM_DEBUG_KMS("channel EQ not
> > > > > ok,
> > > > > retraining\n");
> > > > >   intel_dp_start_link_train(inte
> > > > > l_dp
> > > > > );
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> ___
> Intel-gfx mailing 

Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/mst: Do not retrain new links

2018-07-18 Thread Dhinakaran Pandiyan
On Wed, 2018-07-18 at 13:31 -0700, Manasi Navare wrote:
> On Wed, Jul 18, 2018 at 01:34:12PM -0700, Dhinakaran Pandiyan wrote:
> > 
> > On Wed, 2018-07-18 at 10:45 -0700, Manasi Navare wrote:
> > > 
> > > On Wed, Jul 18, 2018 at 10:19:42AM -0700, Dhinakaran Pandiyan
> > > wrote:
> > > > 
> > > > 
> > > > The short pulse handler checks if channel equalization is okay
> > > > and
> > > > goes onto retrain a link if there are active MST links. This
> > > > retraining
> > > > path is not meant for new MST connections, but due to a bug
> > > > elsewhere, if
> > > > active_mst_links is < 0 the boolean check for active_mst_links
> > > > passes and
> > > This bug is probably around the way we track the active_mst_links
> > > and
> > > we are
> > > probably decrementing it more times than the available links
> > Yeah, that indeed is one aspect of the bug.
> > 
> > > 
> > >  and since its an int
> > > variable it goes to negative which is not the expected behaviour.
> > > Why not change this active_mst_links variable to be an unsigned
> > > int
> > > since
> > > we do not expect any negative values for this anyways.
> > > That way we can still check against just intel_dp-
> > > >active_mst_links
> > > as opposed checking
> > > for it being greater than 0 and would also not need a WARN_ON
> > > here.
> > I did not get this, mind sharing code diff?
> My question was if negative values for active_mst_links are never
> allowed
> then why cant we define it as an unsigned int and avoid thrwoing a
> Warning later.
Hmm. I still do not get how defining it an unsigned int will prevent
the decrement op from making intel_dp->active_mst_links == true.

> 
> Also I think the following check can be added in
> intel_mst_post_disable_dp():
> 
> if (intel_dp->active_mst_links)
>   intel_dp->active_mst_link--;

That's just band-aid, we will still go through the post_disable
sequence while not decrementing ->active_mst_links.


> 
> to avoid getting negative values in the first place.
> 
> Manasi
> 
> > 
> > 
> > -DK
> > 
> > > 
> > > 
> > > Manasi
> > > 
> > > > 
> > > > 
> > > > we proceed to retrain a new link. This results in a sequence of
> > > > failed link
> > > > training attempts, most likely due to the hardware not setup
> > > > for
> > > > link
> > > > training at that point i.e., missing the DDI pre_enable
> > > > sequence.
> > > > 
> > > > [   80.301272] [drm:intel_dp_check_mst_status] channel EQ not
> > > > ok,
> > > > retraining
> > > > [   80.301312] [drm:intel_ddi_prepare_link_retrain] *ERROR*
> > > > Timeout
> > > > waiting for DDI BUF C idle bit
> > > > 
> > > > The above error gives us a hint something went wrong before
> > > > link
> > > > training started.
> > > > 
> > > > Check for a positive value of active_mst_links and throw in a
> > > > warning for
> > > > invalid active_mst_links as debug aid.
> > > > 
> > > > Cc: Nathan Ciobanu 
> > > > Cc: Rodrigo Vivi 
> > > > Signed-off-by: Dhinakaran Pandiyan  > > > om>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index b45b08420c1f..2d61ff01cf51 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -4213,12 +4213,14 @@ intel_dp_check_mst_status(struct
> > > > intel_dp
> > > > *intel_dp)
> > > >     int ret = 0;
> > > >     int retry;
> > > >     bool handled;
> > > > +
> > > > +   WARN_ON_ONCE(intel_dp->active_mst_links < 0);
> > > >     bret = intel_dp_get_sink_irq_esi(intel_dp,
> > > > esi);
> > > >  go_again:
> > > >     if (bret == true) {
> > > >  
> > > >     /* check link status - esi[10] =
> > > > 0x200c */
> > > > -   if (intel_dp->active_mst_links &&
> > > > +   if (intel_dp->active_mst_links > 0 &&
> > > >     !drm_dp_channel_eq_ok([10],
> > > > intel_dp->lane_count)) {
> > > >     DRM_DEBUG_KMS("channel EQ not
> > > > ok,
> > > > retraining\n");
> > > >     intel_dp_start_link_train(inte
> > > > l_dp
> > > > );
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/mst: Do not retrain new links

2018-07-18 Thread Nathan Ciobanu
On Wed, Jul 18, 2018 at 10:19:42AM -0700, Dhinakaran Pandiyan wrote:
> The short pulse handler checks if channel equalization is okay and
> goes onto retrain a link if there are active MST links. This retraining
> path is not meant for new MST connections, but due to a bug elsewhere, if
> active_mst_links is < 0 the boolean check for active_mst_links passes and
> we proceed to retrain a new link. This results in a sequence of failed link
> training attempts, most likely due to the hardware not setup for link
> training at that point i.e., missing the DDI pre_enable sequence.
> 
> [   80.301272] [drm:intel_dp_check_mst_status] channel EQ not ok, retraining
> [   80.301312] [drm:intel_ddi_prepare_link_retrain] *ERROR* Timeout waiting 
> for DDI BUF C idle bit
> 
> The above error gives us a hint something went wrong before link
> training started.
> 
> Check for a positive value of active_mst_links and throw in a warning for
> invalid active_mst_links as debug aid.
> 
> Cc: Nathan Ciobanu 
> Cc: Rodrigo Vivi 
> Signed-off-by: Dhinakaran Pandiyan 
Tested-by: Nathan Ciobanu 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b45b08420c1f..2d61ff01cf51 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4213,12 +4213,14 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>   int ret = 0;
>   int retry;
>   bool handled;
> +
> + WARN_ON_ONCE(intel_dp->active_mst_links < 0);
>   bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
>  go_again:
>   if (bret == true) {
>  
>   /* check link status - esi[10] = 0x200c */
> - if (intel_dp->active_mst_links &&
> + if (intel_dp->active_mst_links > 0 &&
>   !drm_dp_channel_eq_ok([10], 
> intel_dp->lane_count)) {
>   DRM_DEBUG_KMS("channel EQ not ok, 
> retraining\n");
>   intel_dp_start_link_train(intel_dp);
> -- 
> 2.17.1
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/mst: Do not retrain new links

2018-07-18 Thread Manasi Navare
On Wed, Jul 18, 2018 at 01:34:12PM -0700, Dhinakaran Pandiyan wrote:
> On Wed, 2018-07-18 at 10:45 -0700, Manasi Navare wrote:
> > On Wed, Jul 18, 2018 at 10:19:42AM -0700, Dhinakaran Pandiyan wrote:
> > > 
> > > The short pulse handler checks if channel equalization is okay and
> > > goes onto retrain a link if there are active MST links. This
> > > retraining
> > > path is not meant for new MST connections, but due to a bug
> > > elsewhere, if
> > > active_mst_links is < 0 the boolean check for active_mst_links
> > > passes and
> > This bug is probably around the way we track the active_mst_links and
> > we are
> > probably decrementing it more times than the available links
> 
> Yeah, that indeed is one aspect of the bug.
> 
> >  and since its an int
> > variable it goes to negative which is not the expected behaviour.
> > Why not change this active_mst_links variable to be an unsigned int
> > since
> > we do not expect any negative values for this anyways.
> > That way we can still check against just intel_dp->active_mst_links
> > as opposed checking
> > for it being greater than 0 and would also not need a WARN_ON here.
> 
> I did not get this, mind sharing code diff?

My question was if negative values for active_mst_links are never allowed
then why cant we define it as an unsigned int and avoid thrwoing a Warning 
later.

Also I think the following check can be added in intel_mst_post_disable_dp():

if (intel_dp->active_mst_links)
intel_dp->active_mst_link--;

to avoid getting negative values in the first place.

Manasi

> 
> -DK
> 
> > 
> > Manasi
> > 
> > > 
> > > we proceed to retrain a new link. This results in a sequence of
> > > failed link
> > > training attempts, most likely due to the hardware not setup for
> > > link
> > > training at that point i.e., missing the DDI pre_enable sequence.
> > > 
> > > [   80.301272] [drm:intel_dp_check_mst_status] channel EQ not ok,
> > > retraining
> > > [   80.301312] [drm:intel_ddi_prepare_link_retrain] *ERROR* Timeout
> > > waiting for DDI BUF C idle bit
> > > 
> > > The above error gives us a hint something went wrong before link
> > > training started.
> > > 
> > > Check for a positive value of active_mst_links and throw in a
> > > warning for
> > > invalid active_mst_links as debug aid.
> > > 
> > > Cc: Nathan Ciobanu 
> > > Cc: Rodrigo Vivi 
> > > Signed-off-by: Dhinakaran Pandiyan 
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index b45b08420c1f..2d61ff01cf51 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4213,12 +4213,14 @@ intel_dp_check_mst_status(struct intel_dp
> > > *intel_dp)
> > >   int ret = 0;
> > >   int retry;
> > >   bool handled;
> > > +
> > > + WARN_ON_ONCE(intel_dp->active_mst_links < 0);
> > >   bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> > >  go_again:
> > >   if (bret == true) {
> > >  
> > >   /* check link status - esi[10] = 0x200c */
> > > - if (intel_dp->active_mst_links &&
> > > + if (intel_dp->active_mst_links > 0 &&
> > >   !drm_dp_channel_eq_ok([10],
> > > intel_dp->lane_count)) {
> > >   DRM_DEBUG_KMS("channel EQ not ok,
> > > retraining\n");
> > >   intel_dp_start_link_train(intel_dp
> > > );
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/mst: Do not retrain new links

2018-07-18 Thread Dhinakaran Pandiyan
On Wed, 2018-07-18 at 10:45 -0700, Manasi Navare wrote:
> On Wed, Jul 18, 2018 at 10:19:42AM -0700, Dhinakaran Pandiyan wrote:
> > 
> > The short pulse handler checks if channel equalization is okay and
> > goes onto retrain a link if there are active MST links. This
> > retraining
> > path is not meant for new MST connections, but due to a bug
> > elsewhere, if
> > active_mst_links is < 0 the boolean check for active_mst_links
> > passes and
> This bug is probably around the way we track the active_mst_links and
> we are
> probably decrementing it more times than the available links

Yeah, that indeed is one aspect of the bug.

>  and since its an int
> variable it goes to negative which is not the expected behaviour.
> Why not change this active_mst_links variable to be an unsigned int
> since
> we do not expect any negative values for this anyways.
> That way we can still check against just intel_dp->active_mst_links
> as opposed checking
> for it being greater than 0 and would also not need a WARN_ON here.

I did not get this, mind sharing code diff?

-DK

> 
> Manasi
> 
> > 
> > we proceed to retrain a new link. This results in a sequence of
> > failed link
> > training attempts, most likely due to the hardware not setup for
> > link
> > training at that point i.e., missing the DDI pre_enable sequence.
> > 
> > [   80.301272] [drm:intel_dp_check_mst_status] channel EQ not ok,
> > retraining
> > [   80.301312] [drm:intel_ddi_prepare_link_retrain] *ERROR* Timeout
> > waiting for DDI BUF C idle bit
> > 
> > The above error gives us a hint something went wrong before link
> > training started.
> > 
> > Check for a positive value of active_mst_links and throw in a
> > warning for
> > invalid active_mst_links as debug aid.
> > 
> > Cc: Nathan Ciobanu 
> > Cc: Rodrigo Vivi 
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index b45b08420c1f..2d61ff01cf51 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4213,12 +4213,14 @@ intel_dp_check_mst_status(struct intel_dp
> > *intel_dp)
> >     int ret = 0;
> >     int retry;
> >     bool handled;
> > +
> > +   WARN_ON_ONCE(intel_dp->active_mst_links < 0);
> >     bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> >  go_again:
> >     if (bret == true) {
> >  
> >     /* check link status - esi[10] = 0x200c */
> > -   if (intel_dp->active_mst_links &&
> > +   if (intel_dp->active_mst_links > 0 &&
> >     !drm_dp_channel_eq_ok([10],
> > intel_dp->lane_count)) {
> >     DRM_DEBUG_KMS("channel EQ not ok,
> > retraining\n");
> >     intel_dp_start_link_train(intel_dp
> > );
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/mst: Do not retrain new links

2018-07-18 Thread Manasi Navare
On Wed, Jul 18, 2018 at 10:19:42AM -0700, Dhinakaran Pandiyan wrote:
> The short pulse handler checks if channel equalization is okay and
> goes onto retrain a link if there are active MST links. This retraining
> path is not meant for new MST connections, but due to a bug elsewhere, if
> active_mst_links is < 0 the boolean check for active_mst_links passes and

This bug is probably around the way we track the active_mst_links and we are
probably decrementing it more times than the available links and since its an 
int
variable it goes to negative which is not the expected behaviour.
Why not change this active_mst_links variable to be an unsigned int since
we do not expect any negative values for this anyways.
That way we can still check against just intel_dp->active_mst_links as opposed 
checking
for it being greater than 0 and would also not need a WARN_ON here.

Manasi

> we proceed to retrain a new link. This results in a sequence of failed link
> training attempts, most likely due to the hardware not setup for link
> training at that point i.e., missing the DDI pre_enable sequence.
> 
> [   80.301272] [drm:intel_dp_check_mst_status] channel EQ not ok, retraining
> [   80.301312] [drm:intel_ddi_prepare_link_retrain] *ERROR* Timeout waiting 
> for DDI BUF C idle bit
> 
> The above error gives us a hint something went wrong before link
> training started.
> 
> Check for a positive value of active_mst_links and throw in a warning for
> invalid active_mst_links as debug aid.
> 
> Cc: Nathan Ciobanu 
> Cc: Rodrigo Vivi 
> Signed-off-by: Dhinakaran Pandiyan 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b45b08420c1f..2d61ff01cf51 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4213,12 +4213,14 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>   int ret = 0;
>   int retry;
>   bool handled;
> +
> + WARN_ON_ONCE(intel_dp->active_mst_links < 0);
>   bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
>  go_again:
>   if (bret == true) {
>  
>   /* check link status - esi[10] = 0x200c */
> - if (intel_dp->active_mst_links &&
> + if (intel_dp->active_mst_links > 0 &&
>   !drm_dp_channel_eq_ok([10], 
> intel_dp->lane_count)) {
>   DRM_DEBUG_KMS("channel EQ not ok, 
> retraining\n");
>   intel_dp_start_link_train(intel_dp);
> -- 
> 2.17.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v2 1/2] drm/i915/mst: Do not retrain new links

2018-07-18 Thread Dhinakaran Pandiyan
The short pulse handler checks if channel equalization is okay and
goes onto retrain a link if there are active MST links. This retraining
path is not meant for new MST connections, but due to a bug elsewhere, if
active_mst_links is < 0 the boolean check for active_mst_links passes and
we proceed to retrain a new link. This results in a sequence of failed link
training attempts, most likely due to the hardware not setup for link
training at that point i.e., missing the DDI pre_enable sequence.

[   80.301272] [drm:intel_dp_check_mst_status] channel EQ not ok, retraining
[   80.301312] [drm:intel_ddi_prepare_link_retrain] *ERROR* Timeout waiting for 
DDI BUF C idle bit

The above error gives us a hint something went wrong before link
training started.

Check for a positive value of active_mst_links and throw in a warning for
invalid active_mst_links as debug aid.

Cc: Nathan Ciobanu 
Cc: Rodrigo Vivi 
Signed-off-by: Dhinakaran Pandiyan 
---
 drivers/gpu/drm/i915/intel_dp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b45b08420c1f..2d61ff01cf51 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4213,12 +4213,14 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
int ret = 0;
int retry;
bool handled;
+
+   WARN_ON_ONCE(intel_dp->active_mst_links < 0);
bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
 go_again:
if (bret == true) {
 
/* check link status - esi[10] = 0x200c */
-   if (intel_dp->active_mst_links &&
+   if (intel_dp->active_mst_links > 0 &&
!drm_dp_channel_eq_ok([10], 
intel_dp->lane_count)) {
DRM_DEBUG_KMS("channel EQ not ok, 
retraining\n");
intel_dp_start_link_train(intel_dp);
-- 
2.17.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx