Move workarounds from intel_dp_dpcd_read_wake() into drm's DP helpers
Subsystems (3)
v4 cover Move workarounds from intel_dp_dpcd_read_wake() into drm's DP helpers
Resending this because it looks like replying to my previous series of patches causes patchwork to pick up patches from the original version of this and try to apply them along with this one.
This series of patches takes all of the workarounds we used in intel_dp_dpcd_read_wake() for working around misbehaving sinks into drm's DP aux transaction helpers, so that they can be applied to all aux transactions across each driver. While this patch series was intended to fix issues with the ThinkPad T560 not bringing displays connected to it's dock back up properly after suspend, this should fix a lot of other various DP issues by ensuring that we retry transactions appropriately in every possible failure scenario.
Changes since v3
- Split the patch that moves the logic out of intel_dp_dpcd_read_wake() into multiple patches for each workaround that we apply, so that bisecting this change isn't difficult in the event that this breaks something
- Made sure that drm_dp_dpcd_read() only returns the error it encountered during the first attempted aux transaction, since the error that following retries encounter might be different from the original
Changes since v2
- Reworked the patch again to incorporate all of the behavior of intel_dp_dpcd_read_wake() into drm_dp_dpcd_read() and drm_dp_dpcd_access()
Changes since v1
- Patch has been reworked to take the retry logic out of intel_dp_mst_resume() and into drm_dp_dpcd_access(), based off a suggestion from Daniel Vetter
- Commit message is much longer and gives a better description of the issue this was originally intended to workaround.
Lyude (5):
drm/dp_helper: Increase retry interval to 1000us drm/dp_helper: Always wait before retrying native aux transactions drm/dp_helper: Retry aux transactions on all errors drm/dp_helper: Perform throw-away read before actual read in
drm_dp_dpcd_read()
drm/i915: Get rid of intel_dp_dpcd_read_wake()
drivers/gpu/drm/drm_dp_helper.c | 55 ++++++++++++++++------------ drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----------------------------- 2 files changed, 54 insertions(+), 80 deletions(-)
Signature · 6 lines
2.5.5 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ Fi.CI.BAT: failure for Move workarounds from intel_dp_dpcd_read_wake() into drm's DP helpers
== Series Details ==
== Series Details ==
Series: Move workarounds from intel_dp_dpcd_read_wake() into drm's DP helpers
URL : https://patchwork.freedesktop.org/series/4957/
State : failure
== Summary ==
Series 4957v1 Move workarounds from intel_dp_dpcd_read_wake() into drm's DP helpers
http://patchwork.freedesktop.org/api/1.0/series/4957/revisions/1/mbox/
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> DMESG-WARN (bsw-nuc-2)
Test gem_exec_whisper:
Subgroup basic:
fail -> PASS (bdw-ultra)
pass -> FAIL (ivb-t430s)bdw-nuci7 total:192 pass:179 dwarn:0 dfail:0 fail:1 skip:12 bdw-ultra total:192 pass:171 dwarn:0 dfail:0 fail:0 skip:21 bsw-nuc-2 total:192 pass:154 dwarn:1 dfail:0 fail:0 skip:37 byt-nuc total:192 pass:156 dwarn:1 dfail:0 fail:0 skip:35 hsw-brixbox total:192 pass:170 dwarn:0 dfail:0 fail:0 skip:22 hsw-gt2 total:192 pass:175 dwarn:0 dfail:0 fail:0 skip:17 ivb-t430s total:192 pass:166 dwarn:0 dfail:0 fail:1 skip:25 skl-i7k-2 total:192 pass:169 dwarn:0 dfail:0 fail:0 skip:23 snb-dellxps total:192 pass:158 dwarn:0 dfail:0 fail:0 skip:34 snb-x220t total:192 pass:158 dwarn:0 dfail:0 fail:1 skip:33
Results at /archive/results/CI_IGT_test/Patchwork_1724/
f5d413cccefa1f93d64c34f357151d42add63a84 drm-intel-nightly: 2016y-03m-24d-14h-34m-29s UTC integration manifest 4b1da02a3b7720e57859b2dccbbe1d6cdfcc7ae0 drm/i915: Get rid of intel_dp_dpcd_read_wake() 51a1b63a40a1d2355732dd590d8ad46fced76350 drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read() fa755d7cc99a1caa783fea8673894f81b99531f6 drm/dp_helper: Retry aux transactions on all errors 45da5f7ff25ba40aebe5c3f7cea37d05e65dc047 drm/dp_helper: Always wait before retrying native aux transactions 505080d0478b8131803b64f62dde4440bf7ec833 drm/dp_helper: Increase retry interval to 1000us
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH RESEND v4 1/5 drm/dp_helper: Increase retry interval to 1000us
In reply to LyudeThis is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's DP helper.
--- drivers/gpu/drm/drm_dp_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/drm_dp_helper.c +1 -1
@@ -160,7 +160,7 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw) }EXPORT_SYMBOL (drm_dp_bw_code_to_link_rate); -#define AUX_RETRY_INTERVAL 500/* us */ +#define AUX_RETRY_INTERVAL 1000/* us */ /** * DOC: dp helpers
Signature · 6 lines
2.5.5 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 RESEND 1/5] drm/dp_helper: Increase retry interval to 1000us
Quoted 14 lines · Lyude
This is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in
intel_dp_dpcd_read_wake()to drm's DP helper.Signed-off-by Lyude <[email protected]>--- drivers/gpu/drm/drm_dp_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/drm_dp_helper.c +0
-0
drivers/gpu/drm/drm_dp_helper.c int drm_dp_bw_code_to_link_rate(u8 link_bw) · line 163
Quoted 5 lines
drivers/gpu/drm/drm_dp_helper.c +1
-1@@ -160,7 +160,7 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw) }EXPORT_SYMBOL (drm_dp_bw_code_to_link_rate); -#define AUX_RETRY_INTERVAL 500/* us */ +#define AUX_RETRY_INTERVAL 1000/* us */
Was this to adapt to the msleep(1) in the i915 function? If so it's kinda wrong anyway, since an msleep(1) actually sleeps 1 jiffy, and on most systems that's a lot more than 1 ms. If it all still works, I'd just drop this patch here. I suspect that the magic is all in the more aggressive retrying and the throwaway read, not in how long we actually wait.
On patches 2-5: Reviewed-by: Daniel Vetter <[email protected]>
Quoted 10 lines
drivers/gpu/drm/drm_dp_helper.c +0
-0/** * DOC: dp helpers Signature · 6 lines
2.5.5 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v4 RESEND 1/5] drm/dp_helper: Increase retry interval to 1000us
Yep, the rest of the patchset works fine without this patch
Quoted 41 lines · Daniel Vetter
Quoted 19 lines · Lyude
This is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in
intel_dp_dpcd_read_wake()to drm's DP helper.Signed-off-by Lyude <[email protected]>---
drivers/gpu/drm/drm_dp_helper.c | 2 +-1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/drm_dp_helper.c +0
-0@@ -160,7 +160,7 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw) } EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);drivers/gpu/drm/drm_dp_helper.c int drm_dp_bw_code_to_link_rate(u8 link_bw) · line 163
Quoted 2 lines
drivers/gpu/drm/drm_dp_helper.c +1
-1-#define AUX_RETRY_INTERVAL 500/* us */ +#define AUX_RETRY_INTERVAL 1000/* us */ Was this to adapt to the msleep(1) in the i915 function? If so it's kinda wrong anyway, since an msleep(1) actually sleeps 1 jiffy, and on most systems that's a lot more than 1 ms. If it all still works, I'd just drop this patch here. I suspect that the magic is all in the more aggressive retrying and the throwaway read, not in how long we actually wait.
On patches 2-5: Reviewed-by: Daniel Vetter <[email protected]>
Quoted 11 lines
/**
* DOC: dp helpers
--
2.5.5_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 2 lines
Cheers, Lyude
PATCH RESEND v4 2/5 drm/dp_helper: Always wait before retrying native aux transactions
In reply to LyudeThis is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's DP helper.
Some sinks need some time during the process of resuming the system from sleep before they're ready to handle transactions. While it would be nice if they responded with NACKs in these scenarios, this isn't always the case as a few sinks will just timeout on all of the transactions they receive until they're ready.
--- drivers/gpu/drm/drm_dp_helper.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
drivers/gpu/drm/drm_dp_helper.c +5 -6
@@ -179,7 +179,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, {struct drm_dp_aux_msg msg;unsigned int retry; -int err; +int err =0 ;memset (&msg,0 ,sizeof (msg)); msg.address = offset;
@@ -194,6 +194,10 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,* sufficient, bump to 32 which makes Dell 4k monitors happier. */ for (retry =0 ; retry <32 ; retry++) { +if (err !=0 && err != -ETIMEDOUT ) { +usleep_range (AUX_RETRY_INTERVAL , +AUX_RETRY_INTERVAL +100 ); + }mutex_lock (&aux->hw_mutex); err = aux->transfer (aux, &msg);
@@ -205,7 +209,6 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,return err; } -switch (msg.reply &DP_AUX_NATIVE_REPLY_MASK ) {case DP_AUX_NATIVE_REPLY_ACK :if (err < size)
@@ -214,10 +217,6 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,case DP_AUX_NATIVE_REPLY_NACK :return -EIO ; - -case DP_AUX_NATIVE_REPLY_DEFER : -usleep_range (AUX_RETRY_INTERVAL ,AUX_RETRY_INTERVAL +100 ); -break ; } }
Signature · 6 lines
2.5.5 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH RESEND v4 3/5 drm/dp_helper: Retry aux transactions on all errors
In reply to LyudeThis is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's DP helper.
We cannot rely on sinks NACKing or deferring when they can't receive transactions, nor can we rely on any other sort of consistent error to know when we should stop retrying. As such, we need to just retry unconditionally on errors. We also make sure here to return the error we encountered during the first transaction, since it's possible that retrying the transaction might return a different error then we had originally.
This, along with the previous patch, work around a weird bug with the ThinkPad T560's and it's dock. When resuming the laptop, it appears that there's a short period of time where we're unable to complete any aux transactions, as they all immediately timeout. The only machine I'm able to reproduce this on is the T560 as other production Skylake models seem to be fine. The period during which AUX transactions fail appears to be around 22ms long. AFAIK, the dock for the T560 never actually turns off, the only difference is that it's in SST mode at the start of the resume process, so it's unclear as to why it would need so much time to come back up.
There's been a discussion on this issue going on for a while on the intel-gfx mailing list about this that has, in addition to including developers from Intel, also had the correspondence of one of the hardware engineers for Intel:
http://www.spinics.net/lists/intel-gfx/msg88831.html
http://www.spinics.net/lists/intel-gfx/msg88410.html
We've already looked into a couple of possible explanations for the problem:
- Calling intel_dp_mst_resume() before right fix. intel_runtime_pm_enable_interrupts(). This was the first fix I tried, and while it worked it definitely wasn't the right fix. This worked because DP aux transactions don't actually require interrupts to work:
static uint32_t
intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
{
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg;
uint32_t status;
bool done;
#define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
if (has_aux_irq)
done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
msecs_to_jiffies_timeout(10));
else
done = wait_for_atomic(C, 10) == 0;
if (!done)
DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
has_aux_irq);
#undef C
return status;
} When there's no interrupts enabled, we end up timing out on the wait_event_timeout() call, which causes us to check the DP status register once to see if the transaction was successful or not. Since this adds a 10ms delay to each aux transaction, it ends up adding a long enough delay to the resume process for aux transactions to become functional again. This gave us the illusion that enabling interrupts had something to do with making things work again, and put me on the wrong track for a while.
- Interrupts occurring when we try to perform the aux transactions required to put the dock back into MST mode. This isn't the problem, as the only interrupts I've observed that come during this timeout period are from the snd_hda_intel driver, and disabling that driver doesn't appear to change the behavior at all.
- Skylake's PSR block causing issues by performing aux transactions while we try to bring the dock out of MST mode. Disabling PSR through i915's command line options doesn't seem to change the behavior either, nor does preventing the DMC firmware from being loaded.
Since this investigation went on for about 2 weeks, we decided it would be better for the time being to just workaround this issue by making sure AUX transactions wait a short period of time before retrying.
--- drivers/gpu/drm/drm_dp_helper.c | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-)
drivers/gpu/drm/drm_dp_helper.c +21 -18
@@ -178,8 +178,8 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, unsigned int offset, void *buffer, size_t size) { struct drm_dp_aux_msg msg; -unsigned int retry; -int err =0 ; +unsigned int retry, native_reply; +int err =0 , ret =0 ;memset (&msg,0 ,sizeof (msg)); msg.address = offset;
@@ -194,34 +194,37 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,* sufficient, bump to 32 which makes Dell 4k monitors happier. */ for (retry =0 ; retry <32 ; retry++) { -if (err !=0 && err != -ETIMEDOUT ) { +if (ret !=0 && ret != -ETIMEDOUT ) {usleep_range (AUX_RETRY_INTERVAL ,AUX_RETRY_INTERVAL +100 ); }mutex_lock (&aux->hw_mutex); - err = aux->transfer (aux, &msg); + ret = aux->transfer (aux, &msg);mutex_unlock (&aux->hw_mutex); -if (err <0 ) { -if (err == -EBUSY ) -continue ; -return err; - } - -switch (msg.reply &DP_AUX_NATIVE_REPLY_MASK ) { -case DP_AUX_NATIVE_REPLY_ACK : -if (err < size) -return -EPROTO ; -return err; +if (ret >0 ) { + native_reply = msg.reply &DP_AUX_NATIVE_REPLY_MASK ; +if (native_reply ==DP_AUX_NATIVE_REPLY_ACK ) { +if (ret == size) +return ret; -case DP_AUX_NATIVE_REPLY_NACK : -return -EIO ; + ret = -EPROTO ; + }else + ret = -EIO ; } + +/* +* We want the error we return to be the error we received on +* the first transaction, since we may get a different error the +* next time we retry +*/ +if (!err) + err = ret; }DRM_DEBUG_KMS ("too many retries, giving up\n" ); -return -EIO ; +return err; }/**
Signature · 6 lines
2.5.5 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH RESEND v4 4/5 drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read()
In reply to LyudeThis is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's DP helper.
Some sinks will just return garbage for the first aux tranaction they receive when coming out of sleep mode, so we need to perform an additional read before the actual read to workaround this.
--- drivers/gpu/drm/drm_dp_helper.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/gpu/drm/drm_dp_helper.c +7 -0
@@ -244,6 +244,13 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,ssize_t drm_dp_dpcd_read (struct drm_dp_aux *aux,unsigned int offset,void *buffer,size_t size) { +/* +* Sometimes we just get the same incorrect byte repeated over the +* entire buffer. Doing one throw away read initially seems to "solve" +* it. +*/ +drm_dp_dpcd_access (aux,DP_AUX_NATIVE_READ ,DP_DPCD_REV , buffer,1 ); +return drm_dp_dpcd_access (aux,DP_AUX_NATIVE_READ , offset, buffer, size); }
Signature · 6 lines
2.5.5 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v5 4/5 drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read()
This is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's DP helper.
Some sinks will just return garbage for the first aux tranaction they receive when coming out of sleep mode, so we need to perform an additional read before the actual read to workaround this.
Changes since v5
- If the throwaway read in drm_dp_dpcd_read() fails, return the error from that instead of continuing. This follows the same logic we do in drm_dp_dpcd_access() (e.g. the error from the first transaction may differ from the errors that proceeding attempts might return).
--- drivers/gpu/drm/drm_dp_helper.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/gpu/drm/drm_dp_helper.c +12 -0
@@ -244,6 +244,18 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,ssize_t drm_dp_dpcd_read (struct drm_dp_aux *aux,unsigned int offset,void *buffer,size_t size) { +int ret; + +/* +* Sometimes we just get the same incorrect byte repeated over the +* entire buffer. Doing one throw away read initially seems to "solve" +* it. +*/ + ret =drm_dp_dpcd_access (aux,DP_AUX_NATIVE_READ ,DP_DPCD_REV , buffer, +1 ); +if (ret !=1 ) +return ret; +return drm_dp_dpcd_access (aux,DP_AUX_NATIVE_READ , offset, buffer, size); }
Signature · 6 lines
2.5.5 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH RESEND v4 5/5 drm/i915: Get rid of intel_dp_dpcd_read_wake()
In reply to LyudeSince we've fixed up drm_dp_dpcd_read() to allow for retries when things timeout, there's no use for having this function anymore. Good riddens.
--- drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----------------------------- 1 file changed, 22 insertions(+), 57 deletions(-)
drivers/gpu/drm/i915/intel_dp.c +22 -57
@@ -3184,47 +3184,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) }/* -* Native read with retry for link status and receiver capability reads for -* cases where the sink may still be asleep. -* -* Sinks are *supposed* to come up within 1ms from an off state, but we're also -* supposed to retry 3 times per the spec. -*/ -static ssize_t -intel_dp_dpcd_read_wake (struct drm_dp_aux *aux,unsigned int offset, -void *buffer,size_t size) -{ -ssize_t ret; -int i; - -/* -* Sometime we just get the same incorrect byte repeated -* over the entire buffer. Doing just one throw away read -* initially seems to "solve" it. -*/ -drm_dp_dpcd_read (aux,DP_DPCD_REV , buffer,1 ); - -for (i =0 ; i <3 ; i++) { - ret =drm_dp_dpcd_read (aux, offset, buffer, size); -if (ret == size) -return ret; -msleep (1 ); - } - -return ret; -} - -/* * Fetch AUX CH registers 0x202 - 0x207 which contain * link status information */ bool intel_dp_get_link_status (struct intel_dp *intel_dp,uint8_t link_status[DP_LINK_STATUS_SIZE ]) { -return intel_dp_dpcd_read_wake (&intel_dp->aux, -DP_LANE0_1_STATUS , - link_status, -DP_LINK_STATUS_SIZE ) ==DP_LINK_STATUS_SIZE ; +return drm_dp_dpcd_read (&intel_dp->aux,DP_LANE0_1_STATUS , link_status, +DP_LINK_STATUS_SIZE ) ==DP_LINK_STATUS_SIZE ; }/* These are source-specific values. */
@@ -3859,8 +3826,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)struct drm_i915_private *dev_priv = dev->dev_private;uint8_t rev; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,0x000 , intel_dp->dpcd, -sizeof (intel_dp->dpcd)) <0 ) +if (drm_dp_dpcd_read (&intel_dp->aux,0x000 , intel_dp->dpcd, +sizeof (intel_dp->dpcd)) <0 )return false;/* aux transfer failed */ DRM_DEBUG_KMS ("DPCD: %*ph\n" , (int )sizeof (intel_dp->dpcd), intel_dp->dpcd);
@@ -3871,9 +3838,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)/* Check if the panel supports PSR */ memset (intel_dp->psr_dpcd,0 ,sizeof (intel_dp->psr_dpcd));if (is_edp (intel_dp)) { -intel_dp_dpcd_read_wake (&intel_dp->aux,DP_PSR_SUPPORT , - intel_dp->psr_dpcd, -sizeof (intel_dp->psr_dpcd)); +drm_dp_dpcd_read (&intel_dp->aux,DP_PSR_SUPPORT , + intel_dp->psr_dpcd, +sizeof (intel_dp->psr_dpcd));if (intel_dp->psr_dpcd[0 ] &DP_PSR_IS_SUPPORTED ) { dev_priv->psr.sink_support = true;DRM_DEBUG_KMS ("Detected EDP PSR Panel.\n" );
@@ -3884,9 +3851,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)uint8_t frame_sync_cap; dev_priv->psr.sink_support = true; -intel_dp_dpcd_read_wake (&intel_dp->aux, -DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP , - &frame_sync_cap,1 ); +drm_dp_dpcd_read (&intel_dp->aux, +DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP , + &frame_sync_cap,1 ); dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;/* PSR2 needs frame sync as well */ dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
@@ -3902,15 +3869,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)/* Intermediate frequency support */ if (is_edp (intel_dp) && (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP ] &DP_DPCD_DISPLAY_CONTROL_CAPABLE ) && - (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_EDP_DPCD_REV , &rev,1 ) ==1 ) && + (drm_dp_dpcd_read (&intel_dp->aux,DP_EDP_DPCD_REV , &rev,1 ) ==1 ) && (rev >=0x03 )) {/* eDp v1.4 or higher */ __le16 sink_rates[DP_MAX_SUPPORTED_RATES ];int i; -intel_dp_dpcd_read_wake (&intel_dp->aux, -DP_SUPPORTED_LINK_RATES , - sink_rates, -sizeof (sink_rates)); +drm_dp_dpcd_read (&intel_dp->aux,DP_SUPPORTED_LINK_RATES , + sink_rates,sizeof (sink_rates));for (i =0 ; i <ARRAY_SIZE (sink_rates); i++) {int val =le16_to_cpu (sink_rates[i]);
@@ -3933,9 +3898,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)if (intel_dp->dpcd[DP_DPCD_REV ] ==0x10 )return true;/* no per-port downstream info */ -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_DOWNSTREAM_PORT_0 , - intel_dp->downstream_ports, -DP_MAX_DOWNSTREAM_PORTS ) <0 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_DOWNSTREAM_PORT_0 , + intel_dp->downstream_ports, +DP_MAX_DOWNSTREAM_PORTS ) <0 )return false;/* downstream port status fetch failed */ return true;
@@ -3949,11 +3914,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT ] &DP_OUI_SUPPORT ))return ; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_SINK_OUI , buf,3 ) ==3 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_SINK_OUI , buf,3 ) ==3 )DRM_DEBUG_KMS ("Sink OUI: %02hx%02hx%02hx\n" , buf[0 ], buf[1 ], buf[2 ]); -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_BRANCH_OUI , buf,3 ) ==3 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_BRANCH_OUI , buf,3 ) ==3 )DRM_DEBUG_KMS ("Branch OUI: %02hx%02hx%02hx\n" , buf[0 ], buf[1 ], buf[2 ]); }
@@ -3969,7 +3934,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp)if (intel_dp->dpcd[DP_DPCD_REV ] <0x12 )return false; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_MSTM_CAP , buf,1 )) { +if (drm_dp_dpcd_read (&intel_dp->aux,DP_MSTM_CAP , buf,1 )) {if (buf[0 ] &DP_MST_CAP ) {DRM_DEBUG_KMS ("Sink is MST capable\n" ); intel_dp->is_mst = true;
@@ -4106,7 +4071,7 @@ stop:static bool intel_dp_get_sink_irq (struct intel_dp *intel_dp,u8 *sink_irq_vector) { -return intel_dp_dpcd_read_wake (&intel_dp->aux, +return drm_dp_dpcd_read (&intel_dp->aux,DP_DEVICE_SERVICE_IRQ_VECTOR , sink_irq_vector,1 ) ==1 ; }
@@ -4116,7 +4081,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) {int ret; - ret =intel_dp_dpcd_read_wake (&intel_dp->aux, + ret =drm_dp_dpcd_read (&intel_dp->aux,DP_SINK_COUNT_ESI , sink_irq_vector,14 );if (ret !=14 )
@@ -4377,7 +4342,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {uint8_t reg; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_SINK_COUNT , +if (drm_dp_dpcd_read (&intel_dp->aux,DP_SINK_COUNT , ®,1 ) <0 )return connector_status_unknown;
Signature · 6 lines
2.5.5 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Jani Nikula
maintainerRe: [PATCH v4 RESEND 5/5] drm/i915: Get rid of intel_dp_dpcd_read_wake()
Quoted 25 lines · Lyude
Since we've fixed up
drm_dp_dpcd_read()to allow for retries when things timeout, there's no use for having this function anymore. Good riddens.Signed-off-by Lyude <[email protected]>--- drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----------------------------- 1 file changed, 22 insertions(+), 57 deletions(-)drivers/gpu/drm/i915/intel_dp.c +0
-9@@ -3184,47 +3184,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) }/* -* Native read with retry for link status and receiver capability reads for -* cases where the sink may still be asleep. -* -* Sinks are *supposed* to come up within 1ms from an off state, but we're also -* supposed to retry 3 times per the spec. -*/ -static ssize_t -intel_dp_dpcd_read_wake (struct drm_dp_aux *aux,unsigned int offset, -void *buffer,size_t size)
drivers/gpu/drm/i915/intel_dp.c static void chv_dp_post_pll_disable(struct intel_encoder *encoder) · line 3205
Quoted 10 lines
drivers/gpu/drm/i915/intel_dp.c +0
-10-{ -ssize_t ret; -int i; - -/* -* Sometime we just get the same incorrect byte repeated -* over the entire buffer. Doing just one throw away read -* initially seems to "solve" it. -*/ -drm_dp_dpcd_read (aux,DP_DPCD_REV , buffer,1 );
Ville, care to check this series (except patch 1) with your display to see if it still works?
BR,
Jani.
Quoted 145 lines
drivers/gpu/drm/i915/intel_dp.c +22
-38- -for (i =0 ; i <3 ; i++) { - ret =drm_dp_dpcd_read (aux, offset, buffer, size); -if (ret == size) -return ret; -msleep (1 ); - } - -return ret; -} - -/* * Fetch AUX CH registers 0x202 - 0x207 which contain * link status information */ bool intel_dp_get_link_status (struct intel_dp *intel_dp,uint8_t link_status[DP_LINK_STATUS_SIZE ]) { -return intel_dp_dpcd_read_wake (&intel_dp->aux, -DP_LANE0_1_STATUS , - link_status, -DP_LINK_STATUS_SIZE ) ==DP_LINK_STATUS_SIZE ; +return drm_dp_dpcd_read (&intel_dp->aux,DP_LANE0_1_STATUS , link_status, +DP_LINK_STATUS_SIZE ) ==DP_LINK_STATUS_SIZE ; }/* These are source-specific values. */ @@ -3859,8 +3826,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)struct drm_i915_private *dev_priv = dev->dev_private;uint8_t rev; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,0x000 , intel_dp->dpcd, -sizeof (intel_dp->dpcd)) <0 ) +if (drm_dp_dpcd_read (&intel_dp->aux,0x000 , intel_dp->dpcd, +sizeof (intel_dp->dpcd)) <0 )return false;/* aux transfer failed */ DRM_DEBUG_KMS ("DPCD: %*ph\n" , (int )sizeof (intel_dp->dpcd), intel_dp->dpcd);@@ -3871,9 +3838,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)/* Check if the panel supports PSR */ memset (intel_dp->psr_dpcd,0 ,sizeof (intel_dp->psr_dpcd));if (is_edp (intel_dp)) { -intel_dp_dpcd_read_wake (&intel_dp->aux,DP_PSR_SUPPORT , - intel_dp->psr_dpcd, -sizeof (intel_dp->psr_dpcd)); +drm_dp_dpcd_read (&intel_dp->aux,DP_PSR_SUPPORT , + intel_dp->psr_dpcd, +sizeof (intel_dp->psr_dpcd));if (intel_dp->psr_dpcd[0 ] &DP_PSR_IS_SUPPORTED ) { dev_priv->psr.sink_support = true;DRM_DEBUG_KMS ("Detected EDP PSR Panel.\n" );@@ -3884,9 +3851,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)uint8_t frame_sync_cap; dev_priv->psr.sink_support = true; -intel_dp_dpcd_read_wake (&intel_dp->aux, -DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP , - &frame_sync_cap,1 ); +drm_dp_dpcd_read (&intel_dp->aux, +DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP , + &frame_sync_cap,1 ); dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;/* PSR2 needs frame sync as well */ dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;@@ -3902,15 +3869,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)/* Intermediate frequency support */ if (is_edp (intel_dp) && (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP ] &DP_DPCD_DISPLAY_CONTROL_CAPABLE ) && - (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_EDP_DPCD_REV , &rev,1 ) ==1 ) && + (drm_dp_dpcd_read (&intel_dp->aux,DP_EDP_DPCD_REV , &rev,1 ) ==1 ) && (rev >=0x03 )) {/* eDp v1.4 or higher */ __le16 sink_rates[DP_MAX_SUPPORTED_RATES ];int i; -intel_dp_dpcd_read_wake (&intel_dp->aux, -DP_SUPPORTED_LINK_RATES , - sink_rates, -sizeof (sink_rates)); +drm_dp_dpcd_read (&intel_dp->aux,DP_SUPPORTED_LINK_RATES , + sink_rates,sizeof (sink_rates));for (i =0 ; i <ARRAY_SIZE (sink_rates); i++) {int val =le16_to_cpu (sink_rates[i]);@@ -3933,9 +3898,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)if (intel_dp->dpcd[DP_DPCD_REV ] ==0x10 )return true;/* no per-port downstream info */ -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_DOWNSTREAM_PORT_0 , - intel_dp->downstream_ports, -DP_MAX_DOWNSTREAM_PORTS ) <0 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_DOWNSTREAM_PORT_0 , + intel_dp->downstream_ports, +DP_MAX_DOWNSTREAM_PORTS ) <0 )return false;/* downstream port status fetch failed */ return true;@@ -3949,11 +3914,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT ] &DP_OUI_SUPPORT ))return ; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_SINK_OUI , buf,3 ) ==3 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_SINK_OUI , buf,3 ) ==3 )DRM_DEBUG_KMS ("Sink OUI: %02hx%02hx%02hx\n" , buf[0 ], buf[1 ], buf[2 ]); -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_BRANCH_OUI , buf,3 ) ==3 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_BRANCH_OUI , buf,3 ) ==3 )DRM_DEBUG_KMS ("Branch OUI: %02hx%02hx%02hx\n" , buf[0 ], buf[1 ], buf[2 ]); }@@ -3969,7 +3934,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp)if (intel_dp->dpcd[DP_DPCD_REV ] <0x12 )return false; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_MSTM_CAP , buf,1 )) { +if (drm_dp_dpcd_read (&intel_dp->aux,DP_MSTM_CAP , buf,1 )) {if (buf[0 ] &DP_MST_CAP ) {DRM_DEBUG_KMS ("Sink is MST capable\n" ); intel_dp->is_mst = true;@@ -4106,7 +4071,7 @@ stop:static bool intel_dp_get_sink_irq (struct intel_dp *intel_dp,u8 *sink_irq_vector) { -return intel_dp_dpcd_read_wake (&intel_dp->aux, +return drm_dp_dpcd_read (&intel_dp->aux,DP_DEVICE_SERVICE_IRQ_VECTOR , sink_irq_vector,1 ) ==1 ; }@@ -4116,7 +4081,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) {int ret; - ret =intel_dp_dpcd_read_wake (&intel_dp->aux, + ret =drm_dp_dpcd_read (&intel_dp->aux,DP_SINK_COUNT_ESI , sink_irq_vector,14 );if (ret !=14 )@@ -4377,7 +4342,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {uint8_t reg; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_SINK_COUNT , +if (drm_dp_dpcd_read (&intel_dp->aux,DP_SINK_COUNT , ®,1 ) <0 )return connector_status_unknown;
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Jani Nikula
maintainerQuoted 3 lines · Lyude
Resending this because it looks like replying to my previous series of patches causes patchwork to pick up patches from the original version of this and try to apply them along with this one.
Lyude, these don't seem to apply cleanly, please rebase and repost, and we can pick them up once Ville confirms they don't break his display.
BR,
Jani.
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Sounds good, I'll have the rebased versions posted in a bit
Quoted 15 lines · Jani Nikula
Quoted 6 lines · Lyude
Resending this because it looks like replying to my previous series of patches
causes patchwork to pick up patches from the original version of this and
try to apply them along with this one.Lyude, these don't seem to apply cleanly, please rebase and repost, and
we can pick them up once Ville confirms they don't break his display.BR,
Jani.
Signature · 7 lines
Cheers, Lyude _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
v5 cover Move workarounds from intel_dp_dpcd_read_wake() into drm's DP helpers
In reply to Jani NikulaRebased the patches to apply against drm-intel-nightly
Lyude (4):
drm/dp_helper: Always wait before retrying native aux transactions drm/dp_helper: Retry aux transactions on all errors drm/dp_helper: Perform throw-away read before actual read in
drm_dp_dpcd_read()
drm/i915: Get rid of intel_dp_dpcd_read_wake()
drivers/gpu/drm/drm_dp_helper.c | 59 ++++++++++++++++++------------ drivers/gpu/drm/i915/intel_dp.c | 81 ++++++++++++----------------------------- 2 files changed, 59 insertions(+), 81 deletions(-)
Signature · 6 lines
2.5.5 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v5 1/4 drm/dp_helper: Always wait before retrying native aux transactions
This is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's DP helper.
Some sinks need some time during the process of resuming the system from sleep before they're ready to handle transactions. While it would be nice if they responded with NACKs in these scenarios, this isn't always the case as a few sinks will just timeout on all of the transactions they receive until they're ready.
--- drivers/gpu/drm/drm_dp_helper.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/gpu/drm/drm_dp_helper.c +4 -5
@@ -196,6 +196,10 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,* sufficient, bump to 32 which makes Dell 4k monitors happier. */ for (retry =0 ; retry <32 ; retry++) { +if (err !=0 && err != -ETIMEDOUT ) { +usleep_range (AUX_RETRY_INTERVAL , +AUX_RETRY_INTERVAL +100 ); + } err = aux->transfer (aux, &msg);if (err <0 ) {
@@ -205,7 +209,6 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, goto unlock; } -switch (msg.reply &DP_AUX_NATIVE_REPLY_MASK ) {case DP_AUX_NATIVE_REPLY_ACK :if (err < size)
@@ -215,10 +218,6 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,case DP_AUX_NATIVE_REPLY_NACK : err = -EIO ; goto unlock; - -case DP_AUX_NATIVE_REPLY_DEFER : -usleep_range (AUX_RETRY_INTERVAL ,AUX_RETRY_INTERVAL +100 ); -break ; } }
Signature · 6 lines
2.5.5 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v5 2/4 drm/dp_helper: Retry aux transactions on all errors
In reply to LyudeThis is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's DP helper.
We cannot rely on sinks NACKing or deferring when they can't receive transactions, nor can we rely on any other sort of consistent error to know when we should stop retrying. As such, we need to just retry unconditionally on errors. We also make sure here to return the error we encountered during the first transaction, since it's possible that retrying the transaction might return a different error then we had originally.
This, along with the previous patch, work around a weird bug with the ThinkPad T560's and it's dock. When resuming the laptop, it appears that there's a short period of time where we're unable to complete any aux transactions, as they all immediately timeout. The only machine I'm able to reproduce this on is the T560 as other production Skylake models seem to be fine. The period during which AUX transactions fail appears to be around 22ms long. AFAIK, the dock for the T560 never actually turns off, the only difference is that it's in SST mode at the start of the resume process, so it's unclear as to why it would need so much time to come back up.
There's been a discussion on this issue going on for a while on the intel-gfx mailing list about this that has, in addition to including developers from Intel, also had the correspondence of one of the hardware engineers for Intel:
http://www.spinics.net/lists/intel-gfx/msg88831.html
http://www.spinics.net/lists/intel-gfx/msg88410.html
We've already looked into a couple of possible explanations for the problem:
- Calling intel_dp_mst_resume() before right fix. intel_runtime_pm_enable_interrupts(). This was the first fix I tried, and while it worked it definitely wasn't the right fix. This worked because DP aux transactions don't actually require interrupts to work:
static uint32_t
intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
{
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg;
uint32_t status;
bool done;
#define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
if (has_aux_irq)
done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
msecs_to_jiffies_timeout(10));
else
done = wait_for_atomic(C, 10) == 0;
if (!done)
DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
has_aux_irq);
#undef C
return status;
} When there's no interrupts enabled, we end up timing out on the wait_event_timeout() call, which causes us to check the DP status register once to see if the transaction was successful or not. Since this adds a 10ms delay to each aux transaction, it ends up adding a long enough delay to the resume process for aux transactions to become functional again. This gave us the illusion that enabling interrupts had something to do with making things work again, and put me on the wrong track for a while.
- Interrupts occurring when we try to perform the aux transactions required to put the dock back into MST mode. This isn't the problem, as the only interrupts I've observed that come during this timeout period are from the snd_hda_intel driver, and disabling that driver doesn't appear to change the behavior at all.
- Skylake's PSR block causing issues by performing aux transactions while we try to bring the dock out of MST mode. Disabling PSR through i915's command line options doesn't seem to change the behavior either, nor does preventing the DMC firmware from being loaded.
Since this investigation went on for about 2 weeks, we decided it would be better for the time being to just workaround this issue by making sure AUX transactions wait a short period of time before retrying.
--- drivers/gpu/drm/drm_dp_helper.c | 42 +++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-)
drivers/gpu/drm/drm_dp_helper.c +22 -20
@@ -178,8 +178,8 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, unsigned int offset, void *buffer, size_t size) { struct drm_dp_aux_msg msg; -unsigned int retry; -int err =0 ; +unsigned int retry, native_reply; +int err =0 , ret =0 ;memset (&msg,0 ,sizeof (msg)); msg.address = offset;
@@ -196,37 +196,39 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,* sufficient, bump to 32 which makes Dell 4k monitors happier. */ for (retry =0 ; retry <32 ; retry++) { -if (err !=0 && err != -ETIMEDOUT ) { +if (ret !=0 && ret != -ETIMEDOUT ) {usleep_range (AUX_RETRY_INTERVAL ,AUX_RETRY_INTERVAL +100 ); } - err = aux->transfer (aux, &msg); -if (err <0 ) { -if (err == -EBUSY ) -continue ; - - goto unlock; - } + ret = aux->transfer (aux, &msg); -switch (msg.reply &DP_AUX_NATIVE_REPLY_MASK ) { -case DP_AUX_NATIVE_REPLY_ACK : -if (err < size) - err = -EPROTO ; - goto unlock; +if (ret >0 ) { + native_reply = msg.reply &DP_AUX_NATIVE_REPLY_MASK ; +if (native_reply ==DP_AUX_NATIVE_REPLY_ACK ) { +if (ret == size) + goto unlock; -case DP_AUX_NATIVE_REPLY_NACK : - err = -EIO ; - goto unlock; + ret = -EPROTO ; + }else + ret = -EIO ; } + +/* +* We want the error we return to be the error we received on +* the first transaction, since we may get a different error the +* next time we retry +*/ +if (!err) + err = ret; }DRM_DEBUG_KMS ("too many retries, giving up\n" ); - err = -EIO ; + ret = err; unlock:mutex_unlock (&aux->hw_mutex); -return err; +return ret; }/**
Signature · 6 lines
2.5.5 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v5 3/4 drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read()
In reply to LyudeThis is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's DP helper.
Some sinks will just return garbage for the first aux tranaction they receive when coming out of sleep mode, so we need to perform an additional read before the actual read to workaround this.
Changes since v5
- If the throwaway read in drm_dp_dpcd_read() fails, return the error from that instead of continuing. This follows the same logic we do in drm_dp_dpcd_access() (e.g. the error from the first transaction may differ from the errors that proceeding attempts might return).
--- drivers/gpu/drm/drm_dp_helper.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/gpu/drm/drm_dp_helper.c +12 -0
@@ -248,6 +248,18 @@ unlock:ssize_t drm_dp_dpcd_read (struct drm_dp_aux *aux,unsigned int offset,void *buffer,size_t size) { +int ret; + +/* +* Sometimes we just get the same incorrect byte repeated over the +* entire buffer. Doing one throw away read initially seems to "solve" +* it. +*/ + ret =drm_dp_dpcd_access (aux,DP_AUX_NATIVE_READ ,DP_DPCD_REV , buffer, +1 ); +if (ret !=1 ) +return ret; +return drm_dp_dpcd_access (aux,DP_AUX_NATIVE_READ , offset, buffer, size); }
Signature · 6 lines
2.5.5 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v5 3/4] drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read()
Quoted 24 lines · Lyude
This is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in
intel_dp_dpcd_read_wake()to drm's DP helper.Some sinks will just return garbage for the first aux tranaction they receive when coming out of sleep mode, so we need to perform an additional read before the actual read to workaround this.
Changes since v5
- If the throwaway read indrm_dp_dpcd_read()fails, return the error from that instead of continuing. This follows the same logic we do indrm_dp_dpcd_access()(e.g. the error from the first transaction may differ from the errors that proceeding attempts might return).Signed-off-by Lyude <[email protected]>--- drivers/gpu/drm/drm_dp_helper.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/gpu/drm/drm_dp_helper.c +0
-0
drivers/gpu/drm/drm_dp_helper.c unlock: · line 257
Quoted 10 lines
drivers/gpu/drm/drm_dp_helper.c +7
-0@@ -248,6 +248,18 @@ unlock:ssize_t drm_dp_dpcd_read (struct drm_dp_aux *aux,unsigned int offset,void *buffer,size_t size) { +int ret; + +/* +* Sometimes we just get the same incorrect byte repeated over the +* entire buffer. Doing one throw away read initially seems to "solve" +* it. +*/
We might want to improve the comment a bit, now that I analyzed the problem in more detail. So perhaps something like:
/*
* HP ZR24w corrupts the first DPCD access after entering power save
* mode. Eg. on a read, the entire buffer will be filled with the same
* byte. Do a throw away read to avoid corrupting anything we care
* about. Afterwards things will work correctly until the monitor
* gets woken up and subsequently re-enters power save mode. *
* The user pressing any button on the monitor is enough to wake it
* up, so there is no particularly good place to do the workaround.
* We just have to do it before any DPCD access and hope that the
* monitor doesn't power down exactly after the throw away read. */
I'm thinking we should probably also do it for writes, since those seem to be affected as well.
Quoted 10 lines
drivers/gpu/drm/drm_dp_helper.c +5
-0+ ret =drm_dp_dpcd_access (aux,DP_AUX_NATIVE_READ ,DP_DPCD_REV , buffer, +1 ); +if (ret !=1 ) +return ret; +return drm_dp_dpcd_access (aux,DP_AUX_NATIVE_READ , offset, buffer, size); }Signature · 1 line
2.5.5
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v6 3/4 drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read()
This is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's DP helper.
Some sinks will just return garbage for the first aux tranaction they receive when coming out of sleep mode, so we need to perform an additional read before the actual read to workaround this.
Changes since v5
- If the throwaway read in drm_dp_dpcd_read() fails, return the error from that instead of continuing. This follows the same logic we do in drm_dp_dpcd_access() (e.g. the error from the first transaction may differ from the errors that proceeding attempts might return).
--- drivers/gpu/drm/drm_dp_helper.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
drivers/gpu/drm/drm_dp_helper.c +19 -0
@@ -248,6 +248,25 @@ unlock:ssize_t drm_dp_dpcd_read (struct drm_dp_aux *aux,unsigned int offset,void *buffer,size_t size) { +int ret; + +/* +* HP ZR24w corrupts the first DPCD access after entering power save +* mode. Eg. on a read, the entire buffer will be filled with the same +* byte. Do a throw away read to avoid corrupting anything we care +* about. Afterwards things will work correctly until the monitor +* gets woken up and subsequently re-enters power save mode. +* +* The user pressing any button on the monitor is enough to wake it +* up, so there is no particularly good place to do the workaround. +* We just have to do it before any DPCD access and hope that the +* monitor doesn't power down exactly after the throw away read. +*/ + ret =drm_dp_dpcd_access (aux,DP_AUX_NATIVE_READ ,DP_DPCD_REV , buffer, +1 ); +if (ret !=1 ) +return ret; +return drm_dp_dpcd_access (aux,DP_AUX_NATIVE_READ , offset, buffer, size); }
Signature · 6 lines
2.5.5 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v6 3/4] drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read()
Quoted 8 lines · Lyude
This is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in
intel_dp_dpcd_read_wake()to drm's DP helper.Some sinks will just return garbage for the first aux tranaction they receive when coming out of sleep mode, so we need to perform an additional read before the actual read to workaround this.
Quoted 7 lines
Changes since v5
- If the throwaway read indrm_dp_dpcd_read()fails, return the error from that instead of continuing. This follows the same logic we do indrm_dp_dpcd_access()(e.g. the error from the first transaction may differ from the errors that proceeding attempts might return).Signed-off-by Lyude <[email protected]>
Quoted 36 lines
--- drivers/gpu/drm/drm_dp_helper.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)drivers/gpu/drm/drm_dp_helper.c +19
-0@@ -248,6 +248,25 @@ unlock:ssize_t drm_dp_dpcd_read (struct drm_dp_aux *aux,unsigned int offset,void *buffer,size_t size) { +int ret; + +/* +* HP ZR24w corrupts the first DPCD access after entering power save +* mode. Eg. on a read, the entire buffer will be filled with the same +* byte. Do a throw away read to avoid corrupting anything we care +* about. Afterwards things will work correctly until the monitor +* gets woken up and subsequently re-enters power save mode. +* +* The user pressing any button on the monitor is enough to wake it +* up, so there is no particularly good place to do the workaround. +* We just have to do it before any DPCD access and hope that the +* monitor doesn't power down exactly after the throw away read. +*/ + ret =drm_dp_dpcd_access (aux,DP_AUX_NATIVE_READ ,DP_DPCD_REV , buffer, +1 ); +if (ret !=1 ) +return ret; +return drm_dp_dpcd_access (aux,DP_AUX_NATIVE_READ , offset, buffer, size); }Signature · 1 line
2.5.5
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v5 4/4 drm/i915: Get rid of intel_dp_dpcd_read_wake()
In reply to LyudeSince we've fixed up drm_dp_dpcd_read() to allow for retries when things timeout, there's no use for having this function anymore. Good riddens.
--- drivers/gpu/drm/i915/intel_dp.c | 81 ++++++++++++----------------------------- 1 file changed, 23 insertions(+), 58 deletions(-)
drivers/gpu/drm/i915/intel_dp.c +23 -58
@@ -3074,47 +3074,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) }/* -* Native read with retry for link status and receiver capability reads for -* cases where the sink may still be asleep. -* -* Sinks are *supposed* to come up within 1ms from an off state, but we're also -* supposed to retry 3 times per the spec. -*/ -static ssize_t -intel_dp_dpcd_read_wake (struct drm_dp_aux *aux,unsigned int offset, -void *buffer,size_t size) -{ -ssize_t ret; -int i; - -/* -* Sometime we just get the same incorrect byte repeated -* over the entire buffer. Doing just one throw away read -* initially seems to "solve" it. -*/ -drm_dp_dpcd_read (aux,DP_DPCD_REV , buffer,1 ); - -for (i =0 ; i <3 ; i++) { - ret =drm_dp_dpcd_read (aux, offset, buffer, size); -if (ret == size) -return ret; -msleep (1 ); - } - -return ret; -} - -/* * Fetch AUX CH registers 0x202 - 0x207 which contain * link status information */ bool intel_dp_get_link_status (struct intel_dp *intel_dp,uint8_t link_status[DP_LINK_STATUS_SIZE ]) { -return intel_dp_dpcd_read_wake (&intel_dp->aux, -DP_LANE0_1_STATUS , - link_status, -DP_LINK_STATUS_SIZE ) ==DP_LINK_STATUS_SIZE ; +return drm_dp_dpcd_read (&intel_dp->aux,DP_LANE0_1_STATUS , link_status, +DP_LINK_STATUS_SIZE ) ==DP_LINK_STATUS_SIZE ; }/* These are source-specific values. */
@@ -3749,8 +3716,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)struct drm_i915_private *dev_priv = dev->dev_private;uint8_t rev; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,0x000 , intel_dp->dpcd, -sizeof (intel_dp->dpcd)) <0 ) +if (drm_dp_dpcd_read (&intel_dp->aux,0x000 , intel_dp->dpcd, +sizeof (intel_dp->dpcd)) <0 )return false;/* aux transfer failed */ DRM_DEBUG_KMS ("DPCD: %*ph\n" , (int )sizeof (intel_dp->dpcd), intel_dp->dpcd);
@@ -3758,8 +3725,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)if (intel_dp->dpcd[DP_DPCD_REV ] ==0 )return false;/* DPCD not present */ -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_SINK_COUNT , - &intel_dp->sink_count,1 ) <0 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_SINK_COUNT , + &intel_dp->sink_count,1 ) <0 )return false;/*
@@ -3782,9 +3749,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)/* Check if the panel supports PSR */ memset (intel_dp->psr_dpcd,0 ,sizeof (intel_dp->psr_dpcd));if (is_edp (intel_dp)) { -intel_dp_dpcd_read_wake (&intel_dp->aux,DP_PSR_SUPPORT , - intel_dp->psr_dpcd, -sizeof (intel_dp->psr_dpcd)); +drm_dp_dpcd_read (&intel_dp->aux,DP_PSR_SUPPORT , + intel_dp->psr_dpcd, +sizeof (intel_dp->psr_dpcd));if (intel_dp->psr_dpcd[0 ] &DP_PSR_IS_SUPPORTED ) { dev_priv->psr.sink_support = true;DRM_DEBUG_KMS ("Detected EDP PSR Panel.\n" );
@@ -3795,9 +3762,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)uint8_t frame_sync_cap; dev_priv->psr.sink_support = true; -intel_dp_dpcd_read_wake (&intel_dp->aux, -DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP , - &frame_sync_cap,1 ); +drm_dp_dpcd_read (&intel_dp->aux, +DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP , + &frame_sync_cap,1 ); dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;/* PSR2 needs frame sync as well */ dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
@@ -3813,15 +3780,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)/* Intermediate frequency support */ if (is_edp (intel_dp) && (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP ] &DP_DPCD_DISPLAY_CONTROL_CAPABLE ) && - (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_EDP_DPCD_REV , &rev,1 ) ==1 ) && + (drm_dp_dpcd_read (&intel_dp->aux,DP_EDP_DPCD_REV , &rev,1 ) ==1 ) && (rev >=0x03 )) {/* eDp v1.4 or higher */ __le16 sink_rates[DP_MAX_SUPPORTED_RATES ];int i; -intel_dp_dpcd_read_wake (&intel_dp->aux, -DP_SUPPORTED_LINK_RATES , - sink_rates, -sizeof (sink_rates)); +drm_dp_dpcd_read (&intel_dp->aux,DP_SUPPORTED_LINK_RATES , + sink_rates,sizeof (sink_rates));for (i =0 ; i <ARRAY_SIZE (sink_rates); i++) {int val =le16_to_cpu (sink_rates[i]);
@@ -3844,9 +3809,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)if (intel_dp->dpcd[DP_DPCD_REV ] ==0x10 )return true;/* no per-port downstream info */ -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_DOWNSTREAM_PORT_0 , - intel_dp->downstream_ports, -DP_MAX_DOWNSTREAM_PORTS ) <0 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_DOWNSTREAM_PORT_0 , + intel_dp->downstream_ports, +DP_MAX_DOWNSTREAM_PORTS ) <0 )return false;/* downstream port status fetch failed */ return true;
@@ -3860,11 +3825,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT ] &DP_OUI_SUPPORT ))return ; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_SINK_OUI , buf,3 ) ==3 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_SINK_OUI , buf,3 ) ==3 )DRM_DEBUG_KMS ("Sink OUI: %02hx%02hx%02hx\n" , buf[0 ], buf[1 ], buf[2 ]); -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_BRANCH_OUI , buf,3 ) ==3 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_BRANCH_OUI , buf,3 ) ==3 )DRM_DEBUG_KMS ("Branch OUI: %02hx%02hx%02hx\n" , buf[0 ], buf[1 ], buf[2 ]); }
@@ -3883,7 +3848,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp)if (intel_dp->dpcd[DP_DPCD_REV ] <0x12 )return false; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_MSTM_CAP , buf,1 )) { +if (drm_dp_dpcd_read (&intel_dp->aux,DP_MSTM_CAP , buf,1 )) {if (buf[0 ] &DP_MST_CAP ) {DRM_DEBUG_KMS ("Sink is MST capable\n" ); intel_dp->is_mst = true;
@@ -4020,7 +3985,7 @@ stop:static bool intel_dp_get_sink_irq (struct intel_dp *intel_dp,u8 *sink_irq_vector) { -return intel_dp_dpcd_read_wake (&intel_dp->aux, +return drm_dp_dpcd_read (&intel_dp->aux,DP_DEVICE_SERVICE_IRQ_VECTOR , sink_irq_vector,1 ) ==1 ; }
@@ -4030,7 +3995,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) {int ret; - ret =intel_dp_dpcd_read_wake (&intel_dp->aux, + ret =drm_dp_dpcd_read (&intel_dp->aux,DP_SINK_COUNT_ESI , sink_irq_vector,14 );if (ret !=14 )
Signature · 1 line
2.5.5
Re: [PATCH v5 4/4] drm/i915: Get rid of intel_dp_dpcd_read_wake()
Quoted 4 lines · Lyude
Since we've fixed up
drm_dp_dpcd_read()to allow for retries when things timeout, there's no use for having this function anymore. Good riddens.Signed-off-by Lyude <[email protected]>
All applied to drm-misc, thanks.
-Daniel
Quoted 186 lines
--- drivers/gpu/drm/i915/intel_dp.c | 81 ++++++++++++----------------------------- 1 file changed, 23 insertions(+), 58 deletions(-)drivers/gpu/drm/i915/intel_dp.c +23
-58@@ -3074,47 +3074,14 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) }/* -* Native read with retry for link status and receiver capability reads for -* cases where the sink may still be asleep. -* -* Sinks are *supposed* to come up within 1ms from an off state, but we're also -* supposed to retry 3 times per the spec. -*/ -static ssize_t -intel_dp_dpcd_read_wake (struct drm_dp_aux *aux,unsigned int offset, -void *buffer,size_t size) -{ -ssize_t ret; -int i; - -/* -* Sometime we just get the same incorrect byte repeated -* over the entire buffer. Doing just one throw away read -* initially seems to "solve" it. -*/ -drm_dp_dpcd_read (aux,DP_DPCD_REV , buffer,1 ); - -for (i =0 ; i <3 ; i++) { - ret =drm_dp_dpcd_read (aux, offset, buffer, size); -if (ret == size) -return ret; -msleep (1 ); - } - -return ret; -} - -/* * Fetch AUX CH registers 0x202 - 0x207 which contain * link status information */ bool intel_dp_get_link_status (struct intel_dp *intel_dp,uint8_t link_status[DP_LINK_STATUS_SIZE ]) { -return intel_dp_dpcd_read_wake (&intel_dp->aux, -DP_LANE0_1_STATUS , - link_status, -DP_LINK_STATUS_SIZE ) ==DP_LINK_STATUS_SIZE ; +return drm_dp_dpcd_read (&intel_dp->aux,DP_LANE0_1_STATUS , link_status, +DP_LINK_STATUS_SIZE ) ==DP_LINK_STATUS_SIZE ; }/* These are source-specific values. */ @@ -3749,8 +3716,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)struct drm_i915_private *dev_priv = dev->dev_private;uint8_t rev; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,0x000 , intel_dp->dpcd, -sizeof (intel_dp->dpcd)) <0 ) +if (drm_dp_dpcd_read (&intel_dp->aux,0x000 , intel_dp->dpcd, +sizeof (intel_dp->dpcd)) <0 )return false;/* aux transfer failed */ DRM_DEBUG_KMS ("DPCD: %*ph\n" , (int )sizeof (intel_dp->dpcd), intel_dp->dpcd);@@ -3758,8 +3725,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)if (intel_dp->dpcd[DP_DPCD_REV ] ==0 )return false;/* DPCD not present */ -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_SINK_COUNT , - &intel_dp->sink_count,1 ) <0 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_SINK_COUNT , + &intel_dp->sink_count,1 ) <0 )return false;/* @@ -3782,9 +3749,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)/* Check if the panel supports PSR */ memset (intel_dp->psr_dpcd,0 ,sizeof (intel_dp->psr_dpcd));if (is_edp (intel_dp)) { -intel_dp_dpcd_read_wake (&intel_dp->aux,DP_PSR_SUPPORT , - intel_dp->psr_dpcd, -sizeof (intel_dp->psr_dpcd)); +drm_dp_dpcd_read (&intel_dp->aux,DP_PSR_SUPPORT , + intel_dp->psr_dpcd, +sizeof (intel_dp->psr_dpcd));if (intel_dp->psr_dpcd[0 ] &DP_PSR_IS_SUPPORTED ) { dev_priv->psr.sink_support = true;DRM_DEBUG_KMS ("Detected EDP PSR Panel.\n" );@@ -3795,9 +3762,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)uint8_t frame_sync_cap; dev_priv->psr.sink_support = true; -intel_dp_dpcd_read_wake (&intel_dp->aux, -DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP , - &frame_sync_cap,1 ); +drm_dp_dpcd_read (&intel_dp->aux, +DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP , + &frame_sync_cap,1 ); dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;/* PSR2 needs frame sync as well */ dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;@@ -3813,15 +3780,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)/* Intermediate frequency support */ if (is_edp (intel_dp) && (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP ] &DP_DPCD_DISPLAY_CONTROL_CAPABLE ) && - (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_EDP_DPCD_REV , &rev,1 ) ==1 ) && + (drm_dp_dpcd_read (&intel_dp->aux,DP_EDP_DPCD_REV , &rev,1 ) ==1 ) && (rev >=0x03 )) {/* eDp v1.4 or higher */ __le16 sink_rates[DP_MAX_SUPPORTED_RATES ];int i; -intel_dp_dpcd_read_wake (&intel_dp->aux, -DP_SUPPORTED_LINK_RATES , - sink_rates, -sizeof (sink_rates)); +drm_dp_dpcd_read (&intel_dp->aux,DP_SUPPORTED_LINK_RATES , + sink_rates,sizeof (sink_rates));for (i =0 ; i <ARRAY_SIZE (sink_rates); i++) {int val =le16_to_cpu (sink_rates[i]);@@ -3844,9 +3809,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)if (intel_dp->dpcd[DP_DPCD_REV ] ==0x10 )return true;/* no per-port downstream info */ -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_DOWNSTREAM_PORT_0 , - intel_dp->downstream_ports, -DP_MAX_DOWNSTREAM_PORTS ) <0 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_DOWNSTREAM_PORT_0 , + intel_dp->downstream_ports, +DP_MAX_DOWNSTREAM_PORTS ) <0 )return false;/* downstream port status fetch failed */ return true;@@ -3860,11 +3825,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT ] &DP_OUI_SUPPORT ))return ; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_SINK_OUI , buf,3 ) ==3 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_SINK_OUI , buf,3 ) ==3 )DRM_DEBUG_KMS ("Sink OUI: %02hx%02hx%02hx\n" , buf[0 ], buf[1 ], buf[2 ]); -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_BRANCH_OUI , buf,3 ) ==3 ) +if (drm_dp_dpcd_read (&intel_dp->aux,DP_BRANCH_OUI , buf,3 ) ==3 )DRM_DEBUG_KMS ("Branch OUI: %02hx%02hx%02hx\n" , buf[0 ], buf[1 ], buf[2 ]); }@@ -3883,7 +3848,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp)if (intel_dp->dpcd[DP_DPCD_REV ] <0x12 )return false; -if (intel_dp_dpcd_read_wake (&intel_dp->aux,DP_MSTM_CAP , buf,1 )) { +if (drm_dp_dpcd_read (&intel_dp->aux,DP_MSTM_CAP , buf,1 )) {if (buf[0 ] &DP_MST_CAP ) {DRM_DEBUG_KMS ("Sink is MST capable\n" ); intel_dp->is_mst = true;@@ -4020,7 +3985,7 @@ stop:static bool intel_dp_get_sink_irq (struct intel_dp *intel_dp,u8 *sink_irq_vector) { -return intel_dp_dpcd_read_wake (&intel_dp->aux, +return drm_dp_dpcd_read (&intel_dp->aux,DP_DEVICE_SERVICE_IRQ_VECTOR , sink_irq_vector,1 ) ==1 ; }@@ -4030,7 +3995,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) {int ret; - ret =intel_dp_dpcd_read_wake (&intel_dp->aux, + ret =drm_dp_dpcd_read (&intel_dp->aux,DP_SINK_COUNT_ESI , sink_irq_vector,14 );if (ret !=14 )Signature · 6 lines
2.5.5 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ Fi.CI.BAT: failure for series starting with [v5,1/4] drm/dp_helper: Always wait before retrying native aux transactions
In reply to Jani Nikula== Series Details ==
== Series Details ==
Series: series starting with [v5,1/4] drm/dp_helper: Always wait before retrying native aux transactions
URL : https://patchwork.freedesktop.org/series/5667/
State : failure
== Summary ==
Series 5667v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/5667/revisions/1/mbox/
Test drv_getparams_basic:
Subgroup basic-eu-total:
pass -> DMESG-WARN (bsw-nuc-2)
Test gem_basic:
Subgroup create-close:
pass -> DMESG-WARN (bsw-nuc-2)
Test gem_busy:
Subgroup basic-bsd:
pass -> SKIP (bsw-nuc-2)
Subgroup basic-render:
pass -> SKIP (bsw-nuc-2)
Test gem_ctx_basic:
pass -> DMESG-WARN (bsw-nuc-2)
Test gem_ctx_param_basic:
Subgroup basic:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup invalid-ctx-set:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup invalid-size-get:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup non-root-set-no-zeromap:
pass -> DMESG-WARN (bsw-nuc-2)
Test gem_exec_basic:
Subgroup basic-render:
pass -> SKIP (bsw-nuc-2)
Subgroup gtt-vebox:
pass -> SKIP (bsw-nuc-2)
Subgroup readonly-blt:
pass -> SKIP (bsw-nuc-2)
Subgroup readonly-default:
pass -> SKIP (bsw-nuc-2)
Test gem_exec_store:
Subgroup basic-blt:
pass -> SKIP (bsw-nuc-2)
Subgroup basic-vebox:
pass -> SKIP (bsw-nuc-2)
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> SKIP (bsw-nuc-2)
Test gem_flink_basic:
Subgroup bad-open:
pass -> DMESG-WARN (bsw-nuc-2)
Test gem_linear_blits:
Subgroup basic:
pass -> DMESG-FAIL (bsw-nuc-2)
Test gem_mmap_gtt:
Subgroup basic-small-bo-tiledx:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup basic-write-read-distinct:
pass -> DMESG-WARN (bsw-nuc-2)
Test gem_render_tiled_blits:
Subgroup basic:
pass -> DMESG-FAIL (bsw-nuc-2)
Test gem_ringfill:
Subgroup basic-default-forked:
pass -> SKIP (bsw-nuc-2)
Test gem_sync:
Subgroup basic-default:
pass -> SKIP (bsw-nuc-2)
Subgroup basic-render:
pass -> SKIP (bsw-nuc-2)
Test gem_tiled_fence_blits:
Subgroup basic:
pass -> DMESG-FAIL (bsw-nuc-2)
Test kms_addfb_basic:
Subgroup addfb25-modifier-no-flag:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup addfb25-y-tiled:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup bad-pitch-128:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup bad-pitch-32:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup bo-too-small-due-to-tiling:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup unused-handle:
pass -> DMESG-WARN (bsw-nuc-2)
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
fail -> PASS (bsw-nuc-2)
Test kms_force_connector_basic:
Subgroup force-connector-state:
skip -> INCOMPLETE (bsw-nuc-2)
Test kms_pipe_crc_basic:
Subgroup bad-pipe:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup bad-source:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup nonblocking-crc-pipe-c:
pass -> DMESG-FAIL (bsw-nuc-2)
Subgroup read-crc-pipe-c:
pass -> DMESG-FAIL (bsw-nuc-2)
Test prime_self_import:
Subgroup basic-with_one_bo_two_files:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup basic-with_two_bos:
pass -> DMESG-WARN (bsw-nuc-2)bdw-ultra total:203 pass:180 dwarn:0 dfail:0 fail:0 skip:23 bsw-nuc-2 total:121 pass:56 dwarn:20 dfail:5 fail:0 skip:39 byt-nuc total:202 pass:164 dwarn:0 dfail:0 fail:0 skip:38 hsw-brixbox total:203 pass:179 dwarn:0 dfail:0 fail:0 skip:24 ilk-hp8440p total:203 pass:135 dwarn:0 dfail:0 fail:0 skip:68 ivb-t430s total:203 pass:175 dwarn:0 dfail:0 fail:0 skip:28 skl-i7k-2 total:203 pass:178 dwarn:0 dfail:0 fail:0 skip:25 skl-nuci5 total:203 pass:192 dwarn:0 dfail:0 fail:0 skip:11 snb-x220t total:203 pass:165 dwarn:0 dfail:0 fail:1 skip:37
BOOT FAILED for bdw-nuci7
BOOT FAILED for hsw-gt2
BOOT FAILED for snb-dellxps
Results at /archive/results/CI_IGT_test/Patchwork_1892/
88f5cc0db4ed45a3b5eec48e0ef909e5bc0ddead drm-intel-nightly: 2016y-04m-14d-09h-45m-59s UTC integration manifest 6b36c7b drm/i915: Get rid of intel_dp_dpcd_read_wake()
cf805f9 drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read() f2990b6 drm/dp_helper: Retry aux transactions on all errors
269d6d5 drm/dp_helper: Always wait before retrying native aux transactions
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx