[PATCH] drm/edid: limit printk when facing bad edid

[Jani Nikula]

On Thu, 16 Aug 2012, Jerome Glisse <j.glisse at gmail.com> wrote:
> On Thu, Aug 16, 2012 at 11:13 AM, Jani Nikula
> <jani.nikula at linux.intel.com> wrote:
>>
>> There's a bug [1] where the faster GMBUS transmissions fail with some
>> CRTs, and the fix [2] is to fallback to GPIO bit-banging upon errors. As
>> noted in the bug, the fix still leaves plenty of EDID dumps in dmesg, so
>> some measures to reduce the EDID error messages would be most welcome.
>>
>> [1] https://bugzilla.kernel.org/show_bug.cgi?id=45881
>> [2] http://thread.gmane.org/gmane.linux.kernel/1332810/focus=1341912
>>
>> On Tue, 14 Aug 2012, Jerome Glisse <j.glisse at gmail.com> wrote:
>>> On Tue, Aug 14, 2012 at 10:54 AM, Adam Jackson <ajax at redhat.com> wrote:
>>>> On 8/9/12 11:25 AM, j.glisse at gmail.com wrote:
>>>>>
>>>>> From: Jerome Glisse <jglisse at redhat.com>
>>>>>
>>>>> Limit printing bad edid information at one time per connector.
>>>>> Connector that are connected to a bad monitor/kvm will likely
>>>>> stay connected to the same bad monitor/kvm and it makes no
>>>>> sense to keep printing the bad edid message.
>>
>> Do I understand correctly that bad_edid_counter is only reset when you
>> reboot or reload the module? So if you have a laptop that you connect to
>> the monitor at home, the monitor at the office, the projector in the
>> meeting room, and to a TV somewhere else, etc, the message about bad
>> EDID will only printed once? I don't think that's good. But please do
>> correct me if I'm wrong.
>
> I wanted to reset the counter any time the connector is connected to
> something with good edid but i did not do that in the end. I can do a
> patch on top if you think it would be nicer. That way only thing with
> bad edid will be printed once and assuming you don't repeatly
> alternate btw good and bad edid device you would not get spam.

IMHO this is, with or without the additional patch, overengineering the
logic to print out error messages.

For me, as a developer, this would be annoying because, to debug an EDID
issue, I'd have to reload the module, connect to something with "good"
EDID in between, or patch this out, to repeat a problem. I might have to
ask a tester or a bug reporter to do the same.

Also, in the referenced bug, the first problem with GMBUS would disable
error messages, and the fallback retry with GPIO bit-banging would no
longer produce messages even if that failed too.

>
>>>>>
>>>>> Signed-off-by: Jerome Glisse <jglisse at redhat.com>
>>>>
>>>>
>>>> I guess.  I don't see why we don't just move it into DRM_DEBUG_KMS if we're
>>>> going to suppress it, but this does what it says on the box.
>>>>
>>>> Reviewed-by: Adam Jackson <ajax at redhat.com>
>>>>
>>>> - ajax
>>>>
>>>
>>> I think there is still value in getting at least once the bad edid.
>>
>> I think the raw edid dumps should be DEBUG level no matter what. Perhaps
>> some of the other messages could use WARNING/DEBUG too. And with that,
>> and my comment above, I not sure there really needs to be all that logic
>> to count errors and act differently further on.
>>
>
> No, i do think we want bad edid as normal log at least once per
> connector and we definitely don't want to spam bomb the log messages.

Well, at least we agree on silencing the dmesg here. ;)

I'd be happy with very simply adjusting the loglevel of the messages, so
that I can also very simply adjust the amount of messages I get. Or ask
a bug reported to do drm.debug=0xe instead of trying to ensure they
follow a bunch of steps in between. But I'm not going to bikeshed
further if others think the patch here is the way to go.


BR,
Jani.