All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/hsw: Fix workaround for server AUX channel clock divisor
@ 2015-05-19 16:13 jim.bride
  2015-05-20 23:07 ` Todd Previte
  0 siblings, 1 reply; 6+ messages in thread
From: jim.bride @ 2015-05-19 16:13 UTC (permalink / raw)
  To: intel-gfx

From: Jim Bride <jim.bride@linux.intel.com>

According to the HSW b-spec we need to try clock divisors of 63
and 72, each 3 or more times, when attempting DP AUX channel
communication on a server chipset.  This actually wasn't happening
due to a short-circuit that only checked the DP_AUX_CH_CTL_DONE bit
in status rather than checking that the operation was done and
that DP_AUX_CH_CTL_TIME_OUT_ERROR was not set.

Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0edc305..c01a3f9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -895,7 +895,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 			if (status & DP_AUX_CH_CTL_DONE)
 				break;
 		}
-		if (status & DP_AUX_CH_CTL_DONE)
+		if ((status & DP_AUX_CH_CTL_DONE) &&
+		    !(status & DP_AUX_CH_CTL_TIME_OUT_ERROR))
 			break;
 	}
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/hsw: Fix workaround for server AUX channel clock divisor
  2015-05-19 16:13 [PATCH] drm/i915/hsw: Fix workaround for server AUX channel clock divisor jim.bride
@ 2015-05-20 23:07 ` Todd Previte
  2015-05-21  8:20   ` Ville Syrjälä
  0 siblings, 1 reply; 6+ messages in thread
From: Todd Previte @ 2015-05-20 23:07 UTC (permalink / raw)
  To: jim.bride, intel-gfx

Hi Jim,

I checked the BSpec as well and there's nothing indicating that these 
two bits are mutually exclusive. They are both sticky though, and if the 
loop times out 5 times, both _DONE and _TIMEOUT may very well be set. In 
that case the current code would just exit and never bother to change 
clock dividers. So I think your code here is valid.

The only thing you may want to do is capture some of the IRC discussion 
though. In particular, whether or not this is really HSW-specific, how 
this affects other platforms and and the additional delays incurred by 
running through the loop again after changing AUX channel clock dividers 
would be good information to put in there. That's probably more of a 
bikeshed though.

Reviewed-by: Todd Previte <tprevite@gmail.com>

-T

On 5/19/2015 9:13 AM, jim.bride@linux.intel.com wrote:
> From: Jim Bride <jim.bride@linux.intel.com>
>
> According to the HSW b-spec we need to try clock divisors of 63
> and 72, each 3 or more times, when attempting DP AUX channel
> communication on a server chipset.  This actually wasn't happening
> due to a short-circuit that only checked the DP_AUX_CH_CTL_DONE bit
> in status rather than checking that the operation was done and
> that DP_AUX_CH_CTL_TIME_OUT_ERROR was not set.
>
> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0edc305..c01a3f9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -895,7 +895,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>   			if (status & DP_AUX_CH_CTL_DONE)
>   				break;
>   		}
> -		if (status & DP_AUX_CH_CTL_DONE)
> +		if ((status & DP_AUX_CH_CTL_DONE) &&
> +		    !(status & DP_AUX_CH_CTL_TIME_OUT_ERROR))
>   			break;
>   	}
>   

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/hsw: Fix workaround for server AUX channel clock divisor
  2015-05-20 23:07 ` Todd Previte
@ 2015-05-21  8:20   ` Ville Syrjälä
  2015-05-21 12:04     ` Jani Nikula
  0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2015-05-21  8:20 UTC (permalink / raw)
  To: Todd Previte; +Cc: intel-gfx

On Wed, May 20, 2015 at 04:07:37PM -0700, Todd Previte wrote:
> Hi Jim,
> 
> I checked the BSpec as well and there's nothing indicating that these 
> two bits are mutually exclusive. They are both sticky though, and if the 
> loop times out 5 times, both _DONE and _TIMEOUT may very well be set. In 
> that case the current code would just exit and never bother to change 
> clock dividers. So I think your code here is valid.

Shouldn't we we checking receive error as well then?

In fact looking at our code after the loop, it's expecting DONE to be
set before it'll even report receive/timeout errors to the user as
EIO/ETIMEDOUT, otherwise it'll just return EBUSY.

> 
> The only thing you may want to do is capture some of the IRC discussion 
> though. In particular, whether or not this is really HSW-specific, how 
> this affects other platforms and and the additional delays incurred by 
> running through the loop again after changing AUX channel clock dividers 
> would be good information to put in there. That's probably more of a 
> bikeshed though.
> 
> Reviewed-by: Todd Previte <tprevite@gmail.com>
> 
> -T
> 
> On 5/19/2015 9:13 AM, jim.bride@linux.intel.com wrote:
> > From: Jim Bride <jim.bride@linux.intel.com>
> >
> > According to the HSW b-spec we need to try clock divisors of 63
> > and 72, each 3 or more times, when attempting DP AUX channel
> > communication on a server chipset.  This actually wasn't happening
> > due to a short-circuit that only checked the DP_AUX_CH_CTL_DONE bit
> > in status rather than checking that the operation was done and
> > that DP_AUX_CH_CTL_TIME_OUT_ERROR was not set.
> >
> > Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_dp.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 0edc305..c01a3f9 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -895,7 +895,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> >   			if (status & DP_AUX_CH_CTL_DONE)
> >   				break;
> >   		}
> > -		if (status & DP_AUX_CH_CTL_DONE)
> > +		if ((status & DP_AUX_CH_CTL_DONE) &&
> > +		    !(status & DP_AUX_CH_CTL_TIME_OUT_ERROR))
> >   			break;
> >   	}
> >   
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/hsw: Fix workaround for server AUX channel clock divisor
  2015-05-21  8:20   ` Ville Syrjälä
@ 2015-05-21 12:04     ` Jani Nikula
  2015-05-21 18:34       ` Jim Bride
  0 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2015-05-21 12:04 UTC (permalink / raw)
  To: Ville Syrjälä, Todd Previte; +Cc: intel-gfx

On Thu, 21 May 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, May 20, 2015 at 04:07:37PM -0700, Todd Previte wrote:
>> Hi Jim,
>> 
>> I checked the BSpec as well and there's nothing indicating that these 
>> two bits are mutually exclusive. They are both sticky though, and if the 
>> loop times out 5 times, both _DONE and _TIMEOUT may very well be set. In 
>> that case the current code would just exit and never bother to change 
>> clock dividers. So I think your code here is valid.
>
> Shouldn't we we checking receive error as well then?

In other words,


diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7cf3fd43071a..179af62d803d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -893,10 +893,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 				continue;
 			}
 			if (status & DP_AUX_CH_CTL_DONE)
-				break;
+				goto done;
 		}
-		if (status & DP_AUX_CH_CTL_DONE)
-			break;
 	}
 
 	if ((status & DP_AUX_CH_CTL_DONE) == 0) {
@@ -921,7 +919,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 		ret = -ETIMEDOUT;
 		goto out;
 	}
-
+done:
 	/* Unload any bytes sent back from the other side */
 	recv_bytes = ((status & DP_AUX_CH_CTL_MESSAGE_SIZE_MASK) >>
 		      DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);



>
> In fact looking at our code after the loop, it's expecting DONE to be
> set before it'll even report receive/timeout errors to the user as
> EIO/ETIMEDOUT, otherwise it'll just return EBUSY.
>
>> 
>> The only thing you may want to do is capture some of the IRC discussion 
>> though. In particular, whether or not this is really HSW-specific, how 
>> this affects other platforms and and the additional delays incurred by 
>> running through the loop again after changing AUX channel clock dividers 
>> would be good information to put in there. That's probably more of a 
>> bikeshed though.
>> 
>> Reviewed-by: Todd Previte <tprevite@gmail.com>
>> 
>> -T
>> 
>> On 5/19/2015 9:13 AM, jim.bride@linux.intel.com wrote:
>> > From: Jim Bride <jim.bride@linux.intel.com>
>> >
>> > According to the HSW b-spec we need to try clock divisors of 63
>> > and 72, each 3 or more times, when attempting DP AUX channel
>> > communication on a server chipset.  This actually wasn't happening
>> > due to a short-circuit that only checked the DP_AUX_CH_CTL_DONE bit
>> > in status rather than checking that the operation was done and
>> > that DP_AUX_CH_CTL_TIME_OUT_ERROR was not set.
>> >
>> > Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
>> > ---
>> >   drivers/gpu/drm/i915/intel_dp.c | 3 ++-
>> >   1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > index 0edc305..c01a3f9 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -895,7 +895,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>> >   			if (status & DP_AUX_CH_CTL_DONE)
>> >   				break;
>> >   		}
>> > -		if (status & DP_AUX_CH_CTL_DONE)
>> > +		if ((status & DP_AUX_CH_CTL_DONE) &&
>> > +		    !(status & DP_AUX_CH_CTL_TIME_OUT_ERROR))
>> >   			break;
>> >   	}
>> >   
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/hsw: Fix workaround for server AUX channel clock divisor
  2015-05-21 12:04     ` Jani Nikula
@ 2015-05-21 18:34       ` Jim Bride
  2015-05-22  4:43         ` Jani Nikula
  0 siblings, 1 reply; 6+ messages in thread
From: Jim Bride @ 2015-05-21 18:34 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Thu, May 21, 2015 at 03:04:40PM +0300, Jani Nikula wrote:
> On Thu, 21 May 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Wed, May 20, 2015 at 04:07:37PM -0700, Todd Previte wrote:
> >> Hi Jim,
> >> 
> >> I checked the BSpec as well and there's nothing indicating that these 
> >> two bits are mutually exclusive. They are both sticky though, and if the 
> >> loop times out 5 times, both _DONE and _TIMEOUT may very well be set. In 
> >> that case the current code would just exit and never bother to change 
> >> clock dividers. So I think your code here is valid.
> >
> > Shouldn't we we checking receive error as well then?
> 
> In other words,
> 
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7cf3fd43071a..179af62d803d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -893,10 +893,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  				continue;
>  			}
>  			if (status & DP_AUX_CH_CTL_DONE)
> -				break;
> +				goto done;
>  		}
> -		if (status & DP_AUX_CH_CTL_DONE)
> -			break;
>  	}
>  
>  	if ((status & DP_AUX_CH_CTL_DONE) == 0) {
> @@ -921,7 +919,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  		ret = -ETIMEDOUT;
>  		goto out;
>  	}
> -
> +done:
>  	/* Unload any bytes sent back from the other side */
>  	recv_bytes = ((status & DP_AUX_CH_CTL_MESSAGE_SIZE_MASK) >>
>  		      DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);

This looks good, except I think we might want something more like:

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0edc305..89ab714 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -893,10 +893,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
                                continue;
                        }
                        if (status & DP_AUX_CH_CTL_DONE)
-                               break;
+                               goto done;;
                }
-               if (status & DP_AUX_CH_CTL_DONE)
-                       break;
        }
 
        if ((status & DP_AUX_CH_CTL_DONE) == 0) {
@@ -905,6 +903,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
                goto out;
        }
 
+done:
        /* Check for timeout or receive error.
         * Timeouts occur when the sink is not connected
         */

This way we actually set ret appropriately in the event of a receive error
or a timeout.

Thoughts?

Jim


> 
> 
> 
> >
> > In fact looking at our code after the loop, it's expecting DONE to be
> > set before it'll even report receive/timeout errors to the user as
> > EIO/ETIMEDOUT, otherwise it'll just return EBUSY.
> >
> >> 
> >> The only thing you may want to do is capture some of the IRC discussion 
> >> though. In particular, whether or not this is really HSW-specific, how 
> >> this affects other platforms and and the additional delays incurred by 
> >> running through the loop again after changing AUX channel clock dividers 
> >> would be good information to put in there. That's probably more of a 
> >> bikeshed though.
> >> 
> >> Reviewed-by: Todd Previte <tprevite@gmail.com>
> >> 
> >> -T
> >> 
> >> On 5/19/2015 9:13 AM, jim.bride@linux.intel.com wrote:
> >> > From: Jim Bride <jim.bride@linux.intel.com>
> >> >
> >> > According to the HSW b-spec we need to try clock divisors of 63
> >> > and 72, each 3 or more times, when attempting DP AUX channel
> >> > communication on a server chipset.  This actually wasn't happening
> >> > due to a short-circuit that only checked the DP_AUX_CH_CTL_DONE bit
> >> > in status rather than checking that the operation was done and
> >> > that DP_AUX_CH_CTL_TIME_OUT_ERROR was not set.
> >> >
> >> > Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> >> > ---
> >> >   drivers/gpu/drm/i915/intel_dp.c | 3 ++-
> >> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> > index 0edc305..c01a3f9 100644
> >> > --- a/drivers/gpu/drm/i915/intel_dp.c
> >> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> > @@ -895,7 +895,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> >> >   			if (status & DP_AUX_CH_CTL_DONE)
> >> >   				break;
> >> >   		}
> >> > -		if (status & DP_AUX_CH_CTL_DONE)
> >> > +		if ((status & DP_AUX_CH_CTL_DONE) &&
> >> > +		    !(status & DP_AUX_CH_CTL_TIME_OUT_ERROR))
> >> >   			break;
> >> >   	}
> >> >   
> >> 
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > -- 
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/hsw: Fix workaround for server AUX channel clock divisor
  2015-05-21 18:34       ` Jim Bride
@ 2015-05-22  4:43         ` Jani Nikula
  0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2015-05-22  4:43 UTC (permalink / raw)
  To: Jim Bride; +Cc: intel-gfx

On Thu, 21 May 2015, Jim Bride <jim.bride@linux.intel.com> wrote:
> On Thu, May 21, 2015 at 03:04:40PM +0300, Jani Nikula wrote:
>> On Thu, 21 May 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > On Wed, May 20, 2015 at 04:07:37PM -0700, Todd Previte wrote:
>> >> Hi Jim,
>> >> 
>> >> I checked the BSpec as well and there's nothing indicating that these 
>> >> two bits are mutually exclusive. They are both sticky though, and if the 
>> >> loop times out 5 times, both _DONE and _TIMEOUT may very well be set. In 
>> >> that case the current code would just exit and never bother to change 
>> >> clock dividers. So I think your code here is valid.
>> >
>> > Shouldn't we we checking receive error as well then?
>> 
>> In other words,
>> 
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 7cf3fd43071a..179af62d803d 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -893,10 +893,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>>  				continue;
>>  			}
>>  			if (status & DP_AUX_CH_CTL_DONE)
>> -				break;
>> +				goto done;
>>  		}
>> -		if (status & DP_AUX_CH_CTL_DONE)
>> -			break;
>>  	}
>>  
>>  	if ((status & DP_AUX_CH_CTL_DONE) == 0) {
>> @@ -921,7 +919,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>>  		ret = -ETIMEDOUT;
>>  		goto out;
>>  	}
>> -
>> +done:
>>  	/* Unload any bytes sent back from the other side */
>>  	recv_bytes = ((status & DP_AUX_CH_CTL_MESSAGE_SIZE_MASK) >>
>>  		      DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);
>
> This looks good, except I think we might want something more like:
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0edc305..89ab714 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -893,10 +893,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>                                 continue;
>                         }
>                         if (status & DP_AUX_CH_CTL_DONE)
> -                               break;
> +                               goto done;;
>                 }
> -               if (status & DP_AUX_CH_CTL_DONE)
> -                       break;
>         }
>  
>         if ((status & DP_AUX_CH_CTL_DONE) == 0) {
> @@ -905,6 +903,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>                 goto out;
>         }
>  
> +done:
>         /* Check for timeout or receive error.
>          * Timeouts occur when the sink is not connected
>          */
>
> This way we actually set ret appropriately in the event of a receive error
> or a timeout.
>
> Thoughts?

When we hit the "goto done" part, I think we have all the error
conditions already covered within the innermost loop, and everything is
okay. We have to drop out of the loops to hit the errors.

But I'm fine either way.

BR,
Jani


>
> Jim
>
>
>> 
>> 
>> 
>> >
>> > In fact looking at our code after the loop, it's expecting DONE to be
>> > set before it'll even report receive/timeout errors to the user as
>> > EIO/ETIMEDOUT, otherwise it'll just return EBUSY.
>> >
>> >> 
>> >> The only thing you may want to do is capture some of the IRC discussion 
>> >> though. In particular, whether or not this is really HSW-specific, how 
>> >> this affects other platforms and and the additional delays incurred by 
>> >> running through the loop again after changing AUX channel clock dividers 
>> >> would be good information to put in there. That's probably more of a 
>> >> bikeshed though.
>> >> 
>> >> Reviewed-by: Todd Previte <tprevite@gmail.com>
>> >> 
>> >> -T
>> >> 
>> >> On 5/19/2015 9:13 AM, jim.bride@linux.intel.com wrote:
>> >> > From: Jim Bride <jim.bride@linux.intel.com>
>> >> >
>> >> > According to the HSW b-spec we need to try clock divisors of 63
>> >> > and 72, each 3 or more times, when attempting DP AUX channel
>> >> > communication on a server chipset.  This actually wasn't happening
>> >> > due to a short-circuit that only checked the DP_AUX_CH_CTL_DONE bit
>> >> > in status rather than checking that the operation was done and
>> >> > that DP_AUX_CH_CTL_TIME_OUT_ERROR was not set.
>> >> >
>> >> > Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
>> >> > ---
>> >> >   drivers/gpu/drm/i915/intel_dp.c | 3 ++-
>> >> >   1 file changed, 2 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> >> > index 0edc305..c01a3f9 100644
>> >> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> >> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> >> > @@ -895,7 +895,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>> >> >   			if (status & DP_AUX_CH_CTL_DONE)
>> >> >   				break;
>> >> >   		}
>> >> > -		if (status & DP_AUX_CH_CTL_DONE)
>> >> > +		if ((status & DP_AUX_CH_CTL_DONE) &&
>> >> > +		    !(status & DP_AUX_CH_CTL_TIME_OUT_ERROR))
>> >> >   			break;
>> >> >   	}
>> >> >   
>> >> 
>> >> _______________________________________________
>> >> Intel-gfx mailing list
>> >> Intel-gfx@lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> > -- 
>> > Ville Syrjälä
>> > Intel OTC
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-05-22  4:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-19 16:13 [PATCH] drm/i915/hsw: Fix workaround for server AUX channel clock divisor jim.bride
2015-05-20 23:07 ` Todd Previte
2015-05-21  8:20   ` Ville Syrjälä
2015-05-21 12:04     ` Jani Nikula
2015-05-21 18:34       ` Jim Bride
2015-05-22  4:43         ` Jani Nikula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.