Additionally, we should also just fix this ratelimit macro anyway
since it's intended purpose is not to print anything when debugging isn't
enabled. What do you think Alex?
Well, at least I think that the described behavior of the macro is a bug
which should be fixed.
But I think that is independent of the problem that radeon is a bit
flaky about DP aux transfers.
Regards,
Christian.
Am 06.11.2017 um 17:17 schrieb Lyude Paul:
The main reason I added this was because the radeon driver's hotplugging paths
for DP do a ton of unnessecary probing, and because the driver usually also
checks all connectors every time there's a hotplug (there isn't much of a good
reason for this, it's just an old driver) it's not at all difficult to get
well more then 70 callbacks from this that end up filling the kernel log with
useless information (all the messages mean is that for some reason or another,
a DP aux transaction fails which usually just means the port is disconnected).
Ideally: We should be printing the first error code that the i2c handlers for
radeon return in addition to the suppressed messages (and this itself should
not be suppressed iirc), since that's almost always the only piece of
information that matters.
I wouldn't drop the message entirely though unless we're printing something
from DRM. Additionally, we should also just fix this ratelimit macro anyway
since it's intended purpose is not to print anything when debugging isn't
enabled. What do you think Alex?
On Mon, 2017-11-06 at 10:21 +0100, Jean Delvare wrote:
Hi all,
commit 92c177b7947d9c889ea7b024871445015ea74221
Author: Lyude
Date: Wed Feb 22 16:34:53 2017 -0500
drm/radeon/dp_auxch: Ratelimit aux transfer debug messages
does more harm than good in my opinion. Since this commit, I see
several occurrences of the following message in my kernel log daily:
radeon_dp_aux_transfer_native: 74 callbacks suppressed
I never got to see the "callback" in question though, not even once, as
this is a debug message which is off by default. Before the change, I
would not get any such message in the kernel log (as I would expect
when everything works as intended.)
Does this debug message really have any practical value? If not, the
easiest solution would be to simply drop it:
--- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
@@ -168,8 +168,10 @@ radeon_dp_aux_transfer_native(struct drm
goto done;
}
if (tmp & AUX_RX_ERROR_FLAGS) {
- DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero:
%08x\n",
- tmp);
+ /*
+ * aux transfers always fail with non-zero status flags
when
+ * there's nothing connected on the port
+ */
ret = -EIO;
goto done;
}
I can resend this as a formal patch if you agree with this solution.
The actual cause of the problem is that ___ratelimit() prints its
message at KERN_WARNING level regardless of the level of the message
being suppressed. This makes ratelimiting debug, info or notice
messages awkward. Looks like a design overlook to me, maybe it should
be fixed, but that's a much bigger and intrusive change than the
proposal above.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
