| Message ID | 20240926164202.2668922-1-ak@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm: Print bad EDID notices less often | expand |
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);
> > -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. In this case it doesn't matter because the counter only prints once per life time of a connector structure, or if it wraps. I suppose this can be moved into a wrapper. > > > + !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. But that would prevent it always unless debug is set? I was trying to print it according to counter first. This matches the existing usage. -Andi
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++ && + !drm_debug_enabled(DRM_UT_KMS)) + 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);
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(-)