* 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.