Regards
Shashank
On 8/16/2017 9:42 PM, Imre Deak wrote:
On Wed, Aug 16, 2017 at 09:18:58PM +0530, Sharma, Shashank wrote:
Thanks for the review, Imre.
My comments, inline.
Regards
Shashank
On 8/16/2017 7:35 PM, Imre Deak wrote:
On Fri, Aug 11, 2017 at 06:58:26PM +0530, Shashank Sharma wrote:
It's an observation during some CI tests that few LSPCON chips
respond slow while system is under load, and need some delay
while reading current mode status using i2c-over-aux channel.
This patch:
- Adds few retries and delays before declaring a read
failure from LSPCON hardware.
- Changes the debug level of the print from ERROR->DEBUG
whereas another patch in I915 will add an ERROR message
from the caller when we have timed out all our limits.
Hm yea, I was wondering if this is the same issue we saw earlier due to
HPD not being asserted. But looking at the logs it's not the case. After
HPD is asserted it really takes this much time (~36ms) for the adaptor
to respond. This is against the DP 1.4 spec which specifies a 20ms
maximum wake up time, but what can you do.
Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
Cc: Imre Deak <imre.deak@xxxxxxxxx>
Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@xxxxxxxxx>
---
drivers/gpu/drm/drm_dp_dual_mode_helper.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
index 80e62f6..c63eac8 100644
--- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
+++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
@@ -409,6 +409,7 @@ int drm_lspcon_get_mode(struct i2c_adapter *adapter,
enum drm_lspcon_mode *mode)
{
u8 data;
+ u8 retry = 5;
Nitpick: no reason for a specific type so just int?
Actually, was trying to save 3 bytes, as I knew max value for retry would be
5 < 255, but might be too much optimization ?
Yes, imo it's odd to use u8 for simple loop counter. The compiler may
even generate some extra code to zero extend or truncate the result.
cool, so int it is.
int ret = 0;
if (!mode) {
@@ -417,10 +418,17 @@ int drm_lspcon_get_mode(struct i2c_adapter *adapter,
}
/* Read Status: i2c over aux */
- ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_LSPCON_CURRENT_MODE,
- &data, sizeof(data));
+ do {
Nitpick: a for loop whenever possible is generally clearer.
Sorry, I dint understand this comment, little more help required :) ?
for (retry = 0; retry < 5; retry++) ...
would be clearer imo.
Sure, will do it.
--Imre
- Shashank
With the above optionally changed:
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
+ ret = drm_dp_dual_mode_read(adapter,
+ DP_DUAL_MODE_LSPCON_CURRENT_MODE,
+ &data, sizeof(data));
+ if (!ret || !retry--)
+ break;
+ usleep_range(500, 1000);
+ } while (1);
+
if (ret < 0) {
- DRM_ERROR("LSPCON read(0x80, 0x41) failed\n");
+ DRM_DEBUG_KMS("LSPCON read(0x80, 0x41) failed\n");
return -EFAULT;
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
