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

[Jani Nikula <jani.nikula@linux.intel.com>]

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.
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>
> ---
>
> v2: Use bad_edid_count instead of _once.
> ---
>  drivers/gpu/drm/drm_edid.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 855beafb76ff..52c8f08152fd 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1902,10 +1902,17 @@ static bool edid_block_valid(const void *block, bool base)
>  				       edid_block_tag(block));
>  }
>  
> -static void edid_block_status_print(enum edid_block_status status,
> +static void edid_block_status_print(struct drm_connector *connector,
> +				    enum edid_block_status status,
>  				    const struct edid *block,
>  				    int block_num)
>  {
> +	if (status != EDID_BLOCK_OK &&
> +		connector &&
> +		!connector->bad_edid_counter++ &&

The status print has no business changing anything. Besides, this
function gets called per EDID block, not per EDID.

> +		!drm_debug_enabled(DRM_UT_KMS))

If we did that, we might just as well change the pr_* prints to
drm_dbg_kms() and be done with it.

BR,
Jani.

> +		return;
> +
>  	switch (status) {
>  	case EDID_BLOCK_OK:
>  		break;
> @@ -2004,7 +2011,7 @@ static bool drm_edid_block_valid(void *_block, int block_num, bool print_bad_edi
>  			*edid_corrupt = true;
>  	}
>  
> -	edid_block_status_print(status, block, block_num);
> +	edid_block_status_print(NULL, status, block, block_num);
>  
>  	/* Determine whether we can use this block with this status. */
>  	valid = edid_block_status_valid(status, edid_block_tag(block));
> @@ -2375,7 +2382,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(connector, status, edid, 0);
>  
>  	if (status == EDID_BLOCK_READ_FAIL)
>  		goto fail;
> @@ -2409,7 +2416,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(connector, status, block, i);
>  
>  		if (!edid_block_status_valid(status, edid_block_tag(block))) {
>  			if (status == EDID_BLOCK_READ_FAIL)
> @@ -2842,7 +2849,7 @@ const struct drm_edid *drm_edid_read_base_block(struct i2c_adapter *adapter)
>  
>  	status = edid_block_read(base_block, 0, drm_do_probe_ddc_edid, adapter);
>  
> -	edid_block_status_print(status, base_block, 0);
> +	edid_block_status_print(NULL, status, base_block, 0);
>  
>  	if (!edid_block_status_valid(status, edid_block_tag(base_block))) {
>  		edid_block_dump(KERN_NOTICE, base_block, 0);

-- 
Jani Nikula, Intel