On Thu, 26 Sep 2024, Andi Kleen <ak@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
>
> ---
>
> 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
