* Radeon regression fix
@ 2011-09-29 14:21 Brad Campbell
2011-09-29 14:36 ` Alex Deucher
0 siblings, 1 reply; 12+ messages in thread
From: Brad Campbell @ 2011-09-29 14:21 UTC (permalink / raw)
To: alexdeucher; +Cc: airlied, dri-devel, linux-kernel
This patch fixes a regression introduced between 2.6.39 & 3.1-rc1
whereby the displayport AUX channel stopped re-trying commands that
elicited a DEFER response.
Signed-off-by: Brad Campbell <brad@fnarfbargle.com>
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
b/drivers/gpu/drm/radeon/atombios_dp.c
index 7ad43c6..b8450f4 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -60,11 +60,13 @@ static int radeon_process_aux_ch(struct
radeon_i2c_chan *chan,
int index = GetIndexIntoMasterTable(COMMAND,
ProcessAuxChannelTransaction);
unsigned char *base;
int recv_bytes;
+ int retry_count = 0;
memset(&args, 0, sizeof(args));
base = (unsigned char *)rdev->mode_info.atom_context->scratch;
+retry:
memcpy(base, send, send_bytes);
args.v1.lpAuxRequest = 0;
@@ -79,6 +81,16 @@ static int radeon_process_aux_ch(struct
radeon_i2c_chan *chan,
*ack = args.v1.ucReplyStatus;
+ /* defer */
+ if (args.v1.ucReplyStatus == 0x20){
+ DRM_DEBUG_KMS("dp_aux_ch defer\n");
+ /* 10 is an arbitrary value from the pre-regression patch
+ in practice I've never seen more than one */
+ if (retry_count++ < 10)
+ goto retry;
+ return -EBUSY;
+ }
+
/* timeout */
if (args.v1.ucReplyStatus == 1) {
DRM_DEBUG_KMS("dp_aux_ch timeout\n");
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Radeon regression fix
2011-09-29 14:21 Radeon regression fix Brad Campbell
@ 2011-09-29 14:36 ` Alex Deucher
2011-09-29 15:10 ` Brad Campbell
2011-09-29 15:21 ` Brad Campbell
0 siblings, 2 replies; 12+ messages in thread
From: Alex Deucher @ 2011-09-29 14:36 UTC (permalink / raw)
To: Brad Campbell; +Cc: airlied, dri-devel, linux-kernel
On Thu, Sep 29, 2011 at 10:21 AM, Brad Campbell <brad@fnarfbargle.com> wrote:
> This patch fixes a regression introduced between 2.6.39 & 3.1-rc1 whereby
> the displayport AUX channel stopped re-trying commands that elicited a DEFER
> response.
>
It should still be retrying, just restructured slightly. The retry
logic just moved into radeon_dp_i2c_aux_ch(),
radeon_dp_aux_native_write(), and radeon_dp_aux_native_read(), e.g.,
else if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
udelay(400);
Perhaps the delay is causing a problem. Does removing the udelay(400); help?
Alex
> Signed-off-by: Brad Campbell <brad@fnarfbargle.com>
>
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
> b/drivers/gpu/drm/radeon/atombios_dp.c
> index 7ad43c6..b8450f4 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -60,11 +60,13 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan
> *chan,
> int index = GetIndexIntoMasterTable(COMMAND,
> ProcessAuxChannelTransaction);
> unsigned char *base;
> int recv_bytes;
> + int retry_count = 0;
>
> memset(&args, 0, sizeof(args));
>
> base = (unsigned char *)rdev->mode_info.atom_context->scratch;
>
> +retry:
> memcpy(base, send, send_bytes);
>
> args.v1.lpAuxRequest = 0;
> @@ -79,6 +81,16 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan
> *chan,
>
> *ack = args.v1.ucReplyStatus;
>
> + /* defer */
> + if (args.v1.ucReplyStatus == 0x20){
> + DRM_DEBUG_KMS("dp_aux_ch defer\n");
> + /* 10 is an arbitrary value from the pre-regression
> patch
> + in practice I've never seen more than one */
> + if (retry_count++ < 10)
> + goto retry;
> + return -EBUSY;
> + }
> +
> /* timeout */
> if (args.v1.ucReplyStatus == 1) {
> DRM_DEBUG_KMS("dp_aux_ch timeout\n");
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Radeon regression fix
2011-09-29 14:36 ` Alex Deucher
@ 2011-09-29 15:10 ` Brad Campbell
2011-09-29 15:21 ` Brad Campbell
1 sibling, 0 replies; 12+ messages in thread
From: Brad Campbell @ 2011-09-29 15:10 UTC (permalink / raw)
To: Alex Deucher; +Cc: airlied, dri-devel, linux-kernel
On 29/09/11 22:36, Alex Deucher wrote:
> On Thu, Sep 29, 2011 at 10:21 AM, Brad Campbell<brad@fnarfbargle.com> wrote:
>> This patch fixes a regression introduced between 2.6.39& 3.1-rc1 whereby
>> the displayport AUX channel stopped re-trying commands that elicited a DEFER
>> response.
>>
> It should still be retrying, just restructured slightly. The retry
> logic just moved into radeon_dp_i2c_aux_ch(),
> radeon_dp_aux_native_write(), and radeon_dp_aux_native_read(), e.g.,
>
> else if ((ack& AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
> udelay(400);
One problem with that logic I'm afraid.
if (ret == 0)
return -EPROTO;
if (ret < 0)
return ret;
if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
return ret;
else if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
udelay(400);
else
return -EIO;
}
ret == 0 with a defer as there is no data in the packet. It never even gets past the first hurdle.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Radeon regression fix
2011-09-29 14:36 ` Alex Deucher
2011-09-29 15:10 ` Brad Campbell
@ 2011-09-29 15:21 ` Brad Campbell
2011-09-30 0:23 ` Brad Campbell
1 sibling, 1 reply; 12+ messages in thread
From: Brad Campbell @ 2011-09-29 15:21 UTC (permalink / raw)
To: Alex Deucher; +Cc: airlied, dri-devel, linux-kernel
On 29/09/11 22:36, Alex Deucher wrote:
> On Thu, Sep 29, 2011 at 10:21 AM, Brad Campbell<brad@fnarfbargle.com> wrote:
>> This patch fixes a regression introduced between 2.6.39& 3.1-rc1 whereby
>> the displayport AUX channel stopped re-trying commands that elicited a DEFER
>> response.
>>
>
> It should still be retrying, just restructured slightly. The retry
> logic just moved into radeon_dp_i2c_aux_ch(),
> radeon_dp_aux_native_write(), and radeon_dp_aux_native_read(), e.g.,
>
> else if ((ack& AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
> udelay(400);
>
> Perhaps the delay is causing a problem. Does removing the udelay(400); help?
How's this look ?
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
b/drivers/gpu/drm/radeon/atombios_dp.c
index 7ad43c6..aa5624a 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -128,12 +128,14 @@ static int radeon_dp_aux_native_write(struct
radeon_connector *radeon_connector,
while (1) {
ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
msg, msg_bytes, NULL, 0,
delay, &ack);
+ if (ret == 0 && ((ack & AUX_NATIVE_REPLY_MASK) ==
AUX_NATIVE_REPLY_DEFER)){
+ udelay(400);
+ continue;
+ }
if (ret < 0)
return ret;
if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
break;
- else if ((ack & AUX_NATIVE_REPLY_MASK) ==
AUX_NATIVE_REPLY_DEFER)
- udelay(400);
else
return -EIO;
}
@@ -158,14 +160,16 @@ static int radeon_dp_aux_native_read(struct
radeon_connector *radeon_connector,
while (1) {
ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
msg, msg_bytes, recv,
recv_bytes, delay, &ack);
+ if (ret == 0 && ((ack & AUX_NATIVE_REPLY_MASK) ==
AUX_NATIVE_REPLY_DEFER)){
+ udelay(400);
+ continue;
+ }
if (ret == 0)
return -EPROTO;
if (ret < 0)
return ret;
if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
return ret;
- else if ((ack & AUX_NATIVE_REPLY_MASK) ==
AUX_NATIVE_REPLY_DEFER)
- udelay(400);
else
return -EIO;
}
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Radeon regression fix
2011-09-29 15:21 ` Brad Campbell
@ 2011-09-30 0:23 ` Brad Campbell
2011-09-30 4:59 ` Alex Deucher
0 siblings, 1 reply; 12+ messages in thread
From: Brad Campbell @ 2011-09-30 0:23 UTC (permalink / raw)
To: Alex Deucher; +Cc: airlied, dri-devel, linux-kernel
On 29/09/11 23:21, Brad Campbell wrote:
> On 29/09/11 22:36, Alex Deucher wrote:
>> On Thu, Sep 29, 2011 at 10:21 AM, Brad Campbell<brad@fnarfbargle.com>
>> wrote:
>>> This patch fixes a regression introduced between 2.6.39& 3.1-rc1 whereby
>>> the displayport AUX channel stopped re-trying commands that elicited
>>> a DEFER
>>> response.
>>>
>>
>> It should still be retrying, just restructured slightly. The retry
>> logic just moved into radeon_dp_i2c_aux_ch(),
>> radeon_dp_aux_native_write(), and radeon_dp_aux_native_read(), e.g.,
>>
>> else if ((ack& AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
>> udelay(400);
>>
>> Perhaps the delay is causing a problem. Does removing the udelay(400);
>> help?
Looking at it with a nights sleep, it's obvious the code path in
aux_native_write is ok. Is this a bit cleaner than the last patch?
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
b/drivers/gpu/drm/radeon/atombios_dp.c
index 7ad43c6..aacc97d 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -158,14 +158,17 @@ static int radeon_dp_aux_native_read(struct
radeon_connector *radeon_connector,
while (1) {
ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
msg, msg_bytes, recv,
recv_bytes, delay, &ack);
- if (ret == 0)
+ if (ret == 0){
+ if ((ack & AUX_NATIVE_REPLY_MASK) ==
AUX_NATIVE_REPLY_DEFER){
+ udelay(400);
+ continue;
+ }
return -EPROTO;
+ }
if (ret < 0)
return ret;
if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
return ret;
- else if ((ack & AUX_NATIVE_REPLY_MASK) ==
AUX_NATIVE_REPLY_DEFER)
- udelay(400);
else
return -EIO;
}
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Radeon regression fix
2011-09-30 0:23 ` Brad Campbell
@ 2011-09-30 4:59 ` Alex Deucher
2011-09-30 5:14 ` Brad Campbell
0 siblings, 1 reply; 12+ messages in thread
From: Alex Deucher @ 2011-09-30 4:59 UTC (permalink / raw)
To: Brad Campbell; +Cc: airlied, dri-devel, linux-kernel
On Thu, Sep 29, 2011 at 8:23 PM, Brad Campbell
<lists2009@fnarfbargle.com> wrote:
> On 29/09/11 23:21, Brad Campbell wrote:
>>
>> On 29/09/11 22:36, Alex Deucher wrote:
>>>
>>> On Thu, Sep 29, 2011 at 10:21 AM, Brad Campbell<brad@fnarfbargle.com>
>>> wrote:
>>>>
>>>> This patch fixes a regression introduced between 2.6.39& 3.1-rc1 whereby
>>>> the displayport AUX channel stopped re-trying commands that elicited
>>>> a DEFER
>>>> response.
>>>>
>>>
>>> It should still be retrying, just restructured slightly. The retry
>>> logic just moved into radeon_dp_i2c_aux_ch(),
>>> radeon_dp_aux_native_write(), and radeon_dp_aux_native_read(), e.g.,
>>>
>>> else if ((ack& AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
>>> udelay(400);
>>>
>>> Perhaps the delay is causing a problem. Does removing the udelay(400);
>>> help?
>
> Looking at it with a nights sleep, it's obvious the code path in
> aux_native_write is ok. Is this a bit cleaner than the last patch?
Looks pretty good. I was thinking of something more like this (sorry
for the lack of a patch, I'm away from my source trees at the moment):
while (1) {
ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
msg, msg_bytes, recv, recv_bytes, delay, &ack);
if (ret < 0)
return ret;
if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
return ret;
else if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
udelay(400);
else if (ret == 0)
return -EPROTO;
else
return -EIO;
}
Thanks for tracking this down.
Alex
>
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
> b/drivers/gpu/drm/radeon/atombios_dp.c
> index 7ad43c6..aacc97d 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -158,14 +158,17 @@ static int radeon_dp_aux_native_read(struct
> radeon_connector *radeon_connector,
> while (1) {
> ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
> msg, msg_bytes, recv, recv_bytes,
> delay, &ack);
> - if (ret == 0)
> + if (ret == 0){
> + if ((ack & AUX_NATIVE_REPLY_MASK) ==
> AUX_NATIVE_REPLY_DEFER){
> + udelay(400);
> + continue;
> + }
> return -EPROTO;
> + }
> if (ret < 0)
> return ret;
> if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
> return ret;
> - else if ((ack & AUX_NATIVE_REPLY_MASK) ==
> AUX_NATIVE_REPLY_DEFER)
> - udelay(400);
> else
> return -EIO;
> }
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Radeon regression fix
2011-09-30 4:59 ` Alex Deucher
@ 2011-09-30 5:14 ` Brad Campbell
0 siblings, 0 replies; 12+ messages in thread
From: Brad Campbell @ 2011-09-30 5:14 UTC (permalink / raw)
To: Alex Deucher; +Cc: airlied, dri-devel, linux-kernel
On 30/09/11 12:59, Alex Deucher wrote:
> On Thu, Sep 29, 2011 at 8:23 PM, Brad Campbell
>> Looking at it with a nights sleep, it's obvious the code path in
>> aux_native_write is ok. Is this a bit cleaner than the last patch?
>
> Looks pretty good. I was thinking of something more like this (sorry
> for the lack of a patch, I'm away from my source trees at the moment):
>
> while (1) {
> ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
> msg, msg_bytes, recv, recv_bytes, delay,&ack);
>
> if (ret< 0)
> return ret;
> if ((ack& AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
> return ret;
> else if ((ack& AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
> udelay(400);
> else if (ret == 0)
> return -EPROTO;
> else
> return -EIO;
> }
Yep, that looks cleaner.
My only thought was the pre-3.0 code had a limit to the number of
retries. Was that for a specific reason or is it ok to attempt to retry
indefinitely if we receive a DEFER ?
>
> Thanks for tracking this down.
No worries. I learned quite a bit about some kernel internals and more
than a few quirks about apple hardware while muddling around.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Radeon regression fix
@ 2011-09-30 5:14 ` Brad Campbell
0 siblings, 0 replies; 12+ messages in thread
From: Brad Campbell @ 2011-09-30 5:14 UTC (permalink / raw)
To: Alex Deucher; +Cc: airlied, linux-kernel, dri-devel
On 30/09/11 12:59, Alex Deucher wrote:
> On Thu, Sep 29, 2011 at 8:23 PM, Brad Campbell
>> Looking at it with a nights sleep, it's obvious the code path in
>> aux_native_write is ok. Is this a bit cleaner than the last patch?
>
> Looks pretty good. I was thinking of something more like this (sorry
> for the lack of a patch, I'm away from my source trees at the moment):
>
> while (1) {
> ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
> msg, msg_bytes, recv, recv_bytes, delay,&ack);
>
> if (ret< 0)
> return ret;
> if ((ack& AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
> return ret;
> else if ((ack& AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
> udelay(400);
> else if (ret == 0)
> return -EPROTO;
> else
> return -EIO;
> }
Yep, that looks cleaner.
My only thought was the pre-3.0 code had a limit to the number of
retries. Was that for a specific reason or is it ok to attempt to retry
indefinitely if we receive a DEFER ?
>
> Thanks for tracking this down.
No worries. I learned quite a bit about some kernel internals and more
than a few quirks about apple hardware while muddling around.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Radeon regression fix
2011-09-30 5:14 ` Brad Campbell
@ 2011-09-30 5:39 ` Alex Deucher
-1 siblings, 0 replies; 12+ messages in thread
From: Alex Deucher @ 2011-09-30 5:39 UTC (permalink / raw)
To: Brad Campbell; +Cc: airlied, dri-devel, linux-kernel
On Fri, Sep 30, 2011 at 1:14 AM, Brad Campbell
<lists2009@fnarfbargle.com> wrote:
> On 30/09/11 12:59, Alex Deucher wrote:
>>
>> On Thu, Sep 29, 2011 at 8:23 PM, Brad Campbell
>
>>> Looking at it with a nights sleep, it's obvious the code path in
>>> aux_native_write is ok. Is this a bit cleaner than the last patch?
>>
>> Looks pretty good. I was thinking of something more like this (sorry
>> for the lack of a patch, I'm away from my source trees at the moment):
>>
>> while (1) {
>> ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
>> msg, msg_bytes, recv,
>> recv_bytes, delay,&ack);
>>
>> if (ret< 0)
>> return ret;
>> if ((ack& AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
>> return ret;
>> else if ((ack& AUX_NATIVE_REPLY_MASK) ==
>> AUX_NATIVE_REPLY_DEFER)
>> udelay(400);
>> else if (ret == 0)
>> return -EPROTO;
>> else
>> return -EIO;
>> }
>
> Yep, that looks cleaner.
>
> My only thought was the pre-3.0 code had a limit to the number of retries.
> Was that for a specific reason or is it ok to attempt to retry indefinitely
> if we receive a DEFER ?
I need to double check the DP spec, but I think it's 4. The while (1)
loops in radeon_dp_aux_native_write() and radeon_dp_aux_native_read()
should probably be changed to:
for (retry = 0; retry < 4; retry++)
to match what we do in radeon_dp_i2c_aux_ch().
Alex
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Radeon regression fix
@ 2011-09-30 5:39 ` Alex Deucher
0 siblings, 0 replies; 12+ messages in thread
From: Alex Deucher @ 2011-09-30 5:39 UTC (permalink / raw)
To: Brad Campbell; +Cc: airlied, linux-kernel, dri-devel
On Fri, Sep 30, 2011 at 1:14 AM, Brad Campbell
<lists2009@fnarfbargle.com> wrote:
> On 30/09/11 12:59, Alex Deucher wrote:
>>
>> On Thu, Sep 29, 2011 at 8:23 PM, Brad Campbell
>
>>> Looking at it with a nights sleep, it's obvious the code path in
>>> aux_native_write is ok. Is this a bit cleaner than the last patch?
>>
>> Looks pretty good. I was thinking of something more like this (sorry
>> for the lack of a patch, I'm away from my source trees at the moment):
>>
>> while (1) {
>> ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
>> msg, msg_bytes, recv,
>> recv_bytes, delay,&ack);
>>
>> if (ret< 0)
>> return ret;
>> if ((ack& AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
>> return ret;
>> else if ((ack& AUX_NATIVE_REPLY_MASK) ==
>> AUX_NATIVE_REPLY_DEFER)
>> udelay(400);
>> else if (ret == 0)
>> return -EPROTO;
>> else
>> return -EIO;
>> }
>
> Yep, that looks cleaner.
>
> My only thought was the pre-3.0 code had a limit to the number of retries.
> Was that for a specific reason or is it ok to attempt to retry indefinitely
> if we receive a DEFER ?
I need to double check the DP spec, but I think it's 4. The while (1)
loops in radeon_dp_aux_native_write() and radeon_dp_aux_native_read()
should probably be changed to:
for (retry = 0; retry < 4; retry++)
to match what we do in radeon_dp_i2c_aux_ch().
Alex
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] drm/radeon/kms: fix regression in DP aux defer handling
2011-09-30 5:39 ` Alex Deucher
(?)
@ 2011-10-03 13:13 ` alexdeucher
-1 siblings, 0 replies; 12+ messages in thread
From: alexdeucher @ 2011-10-03 13:13 UTC (permalink / raw)
To: airlied, dri-devel; +Cc: Alex Deucher, Brad Campbell, stable
From: Alex Deucher <alexander.deucher@amd.com>
An incorrect ordering in the error checking code lead
to DP aux defer being skipped in the aux native write
path. Move the bytes transferred check (ret == 0)
below the defer check.
Tracked down by: Brad Campbell <brad@fnarfbargle.com>
Fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=41121
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Brad Campbell <brad@fnarfbargle.com>
Cc: stable@kernel.org
---
drivers/gpu/drm/radeon/atombios_dp.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
index 7ad43c6..f526fa7 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -158,14 +158,14 @@ static int radeon_dp_aux_native_read(struct radeon_connector *radeon_connector,
while (1) {
ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
msg, msg_bytes, recv, recv_bytes, delay, &ack);
- if (ret == 0)
- return -EPROTO;
if (ret < 0)
return ret;
if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
return ret;
else if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
udelay(400);
+ else if (ret == 0)
+ return -EPROTO;
else
return -EIO;
}
--
1.7.1.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] drm/radeon/kms: add retry limits for native DP aux defer
2011-09-30 5:39 ` Alex Deucher
(?)
(?)
@ 2011-10-03 13:13 ` alexdeucher
-1 siblings, 0 replies; 12+ messages in thread
From: alexdeucher @ 2011-10-03 13:13 UTC (permalink / raw)
To: airlied, dri-devel; +Cc: Alex Deucher, stable
From: Alex Deucher <alexander.deucher@amd.com>
The previous code could potentially loop forever. Limit
the number of DP aux defer retries to 4 for native aux
transactions, same as i2c over aux transactions.
Noticed by: Brad Campbell <lists2009@fnarfbargle.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Brad Campbell <lists2009@fnarfbargle.com>
Cc: stable@kernel.org
---
drivers/gpu/drm/radeon/atombios_dp.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
index f526fa7..4da2388 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -115,6 +115,7 @@ static int radeon_dp_aux_native_write(struct radeon_connector *radeon_connector,
u8 msg[20];
int msg_bytes = send_bytes + 4;
u8 ack;
+ unsigned retry;
if (send_bytes > 16)
return -1;
@@ -125,20 +126,20 @@ static int radeon_dp_aux_native_write(struct radeon_connector *radeon_connector,
msg[3] = (msg_bytes << 4) | (send_bytes - 1);
memcpy(&msg[4], send, send_bytes);
- while (1) {
+ for (retry = 0; retry < 4; retry++) {
ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
msg, msg_bytes, NULL, 0, delay, &ack);
if (ret < 0)
return ret;
if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
- break;
+ return send_bytes;
else if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
udelay(400);
else
return -EIO;
}
- return send_bytes;
+ return -EIO;
}
static int radeon_dp_aux_native_read(struct radeon_connector *radeon_connector,
@@ -149,13 +150,14 @@ static int radeon_dp_aux_native_read(struct radeon_connector *radeon_connector,
int msg_bytes = 4;
u8 ack;
int ret;
+ unsigned retry;
msg[0] = address;
msg[1] = address >> 8;
msg[2] = AUX_NATIVE_READ << 4;
msg[3] = (msg_bytes << 4) | (recv_bytes - 1);
- while (1) {
+ for (retry = 0; retry < 4; retry++) {
ret = radeon_process_aux_ch(dig_connector->dp_i2c_bus,
msg, msg_bytes, recv, recv_bytes, delay, &ack);
if (ret < 0)
@@ -169,6 +171,8 @@ static int radeon_dp_aux_native_read(struct radeon_connector *radeon_connector,
else
return -EIO;
}
+
+ return -EIO;
}
static void radeon_write_dpcd_reg(struct radeon_connector *radeon_connector,
--
1.7.1.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-10-03 13:14 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-29 14:21 Radeon regression fix Brad Campbell
2011-09-29 14:36 ` Alex Deucher
2011-09-29 15:10 ` Brad Campbell
2011-09-29 15:21 ` Brad Campbell
2011-09-30 0:23 ` Brad Campbell
2011-09-30 4:59 ` Alex Deucher
2011-09-30 5:14 ` Brad Campbell
2011-09-30 5:14 ` Brad Campbell
2011-09-30 5:39 ` Alex Deucher
2011-09-30 5:39 ` Alex Deucher
2011-10-03 13:13 ` [PATCH 1/2] drm/radeon/kms: fix regression in DP aux defer handling alexdeucher
2011-10-03 13:13 ` [PATCH 2/2] drm/radeon/kms: add retry limits for native DP aux defer alexdeucher
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.