[PATCH v3] drm: Print bad EDID notices less often

[PATCH v3] drm: Print bad EDID notices less often

[Andi Kleen]

I have an old monitor that reports a zero EDID block, which results in a
warning message. This happens on every screen save cycle, and maybe in
some other situations, and over time the whole kernel log gets filled
with these redundant messages.

Make most of these prints conditional on bad_edid_count like other verbose EDID
messages.

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---

v2: Use bad_edid_count instead of _once.
v3: Move rate limit logic into dedicated wrapper.
---
 drivers/gpu/drm/drm_edid.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 855beafb76ff..64d41dd605f1 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1949,6 +1949,18 @@ static void edid_block_status_print(enum edid_block_status status,
 	}
 }
 
+static void edid_block_status_print_rl(struct drm_connector *connector,
+				    enum edid_block_status status,
+				    const struct edid *block,
+				    int block_num)
+{
+	if (status != EDID_BLOCK_OK &&
+		!connector->bad_edid_counter++ &&
+		!drm_debug_enabled(DRM_UT_KMS))
+		return;
+	edid_block_status_print(status, block, block_num);
+}
+
 static void edid_block_dump(const char *level, const void *block, int block_num)
 {
 	enum edid_block_status status;
@@ -2375,7 +2387,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
 
 	status = edid_block_read(edid, 0, read_block, context);
 
-	edid_block_status_print(status, edid, 0);
+	edid_block_status_print_rl(connector, status, edid, 0);
 
 	if (status == EDID_BLOCK_READ_FAIL)
 		goto fail;
@@ -2409,7 +2421,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
 
 		status = edid_block_read(block, i, read_block, context);
 
-		edid_block_status_print(status, block, i);
+		edid_block_status_print_rl(connector, status, block, i);
 
 		if (!edid_block_status_valid(status, edid_block_tag(block))) {
 			if (status == EDID_BLOCK_READ_FAIL)
-- 
2.46.1

Re: [PATCH v3] drm: Print bad EDID notices less often

[Jani Nikula]

On Thu, 26 Sep 2024, Andi Kleen <ak@linux.intel.com> wrote:
> I have an old monitor that reports a zero EDID block, which results in a
> warning message. This happens on every screen save cycle, and maybe in
> some other situations, and over time the whole kernel log gets filled
> with these redundant messages.
>
> Make most of these prints conditional on bad_edid_count like other verbose EDID
> messages.

Honestly I think merging this would be counter-productive to addressing
the issue properly.

Please just give me the chance to look into it.

Is your zero EDID block the base block or an extension block?

BR,
Jani.


>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>
> ---
>
> v2: Use bad_edid_count instead of _once.
> v3: Move rate limit logic into dedicated wrapper.
> ---
>  drivers/gpu/drm/drm_edid.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 855beafb76ff..64d41dd605f1 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1949,6 +1949,18 @@ static void edid_block_status_print(enum edid_block_status status,
>  	}
>  }
>  
> +static void edid_block_status_print_rl(struct drm_connector *connector,
> +				    enum edid_block_status status,
> +				    const struct edid *block,
> +				    int block_num)
> +{
> +	if (status != EDID_BLOCK_OK &&
> +		!connector->bad_edid_counter++ &&
> +		!drm_debug_enabled(DRM_UT_KMS))
> +		return;
> +	edid_block_status_print(status, block, block_num);
> +}
> +
>  static void edid_block_dump(const char *level, const void *block, int block_num)
>  {
>  	enum edid_block_status status;
> @@ -2375,7 +2387,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
>  
>  	status = edid_block_read(edid, 0, read_block, context);
>  
> -	edid_block_status_print(status, edid, 0);
> +	edid_block_status_print_rl(connector, status, edid, 0);
>  
>  	if (status == EDID_BLOCK_READ_FAIL)
>  		goto fail;
> @@ -2409,7 +2421,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
>  
>  		status = edid_block_read(block, i, read_block, context);
>  
> -		edid_block_status_print(status, block, i);
> +		edid_block_status_print_rl(connector, status, block, i);
>  
>  		if (!edid_block_status_valid(status, edid_block_tag(block))) {
>  			if (status == EDID_BLOCK_READ_FAIL)

-- 
Jani Nikula, Intel

Re: [PATCH v3] drm: Print bad EDID notices less often

[Andi Kleen]

On Fri, Sep 27, 2024 at 12:30:04PM +0300, Jani Nikula wrote:
> On Thu, 26 Sep 2024, Andi Kleen <ak@linux.intel.com> wrote:
> > I have an old monitor that reports a zero EDID block, which results in a
> > warning message. This happens on every screen save cycle, and maybe in
> > some other situations, and over time the whole kernel log gets filled
> > with these redundant messages.
> >
> > Make most of these prints conditional on bad_edid_count like other verbose EDID
> > messages.
> 
> Honestly I think merging this would be counter-productive to addressing
> the issue properly.

You think it's a problem on the host side, not the monitor side?

> 
> Please just give me the chance to look into it.
> 
> Is your zero EDID block the base block or an extension block?

It's an extension block I believe.

-Andi

Re: [PATCH v3] drm: Print bad EDID notices less often

[Jani Nikula]

On Fri, 27 Sep 2024, Andi Kleen <ak@linux.intel.com> wrote:
> On Fri, Sep 27, 2024 at 12:30:04PM +0300, Jani Nikula wrote:
>> On Thu, 26 Sep 2024, Andi Kleen <ak@linux.intel.com> wrote:
>> > I have an old monitor that reports a zero EDID block, which results in a
>> > warning message. This happens on every screen save cycle, and maybe in
>> > some other situations, and over time the whole kernel log gets filled
>> > with these redundant messages.
>> >
>> > Make most of these prints conditional on bad_edid_count like other verbose EDID
>> > messages.
>> 
>> Honestly I think merging this would be counter-productive to addressing
>> the issue properly.
>
> You think it's a problem on the host side, not the monitor side?

Both. :)

Having zero EDID block is an issue with the display, logging excessively
about it is an issue on the host side.

>> Please just give me the chance to look into it.
>> 
>> Is your zero EDID block the base block or an extension block?
>
> It's an extension block I believe.

Now that should be helpful in detecting display changes, and only
logging once per display. Unless you unplug and plug, but that's not so
common.

BR,
Jani.


-- 
Jani Nikula, Intel