> 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
