Hi all,
This is my attempt to fix bug fdo#107825: https://bugs.freedesktop.org/show_bug.cgi?id=107825
[PATCH 1/3] drm/amd: be quiet when no SAD block is found [PATCH 2/3] drm/radeon: be quiet when no SAD block is found [PATCH 3/3] drm/edid: no CEA extension is not an error
It is fine for displays without audio functionality to not provide any SAD block in their EDID. Do not log an error in that case, just return quietly.
This fixes half of bug fdo#107825: https://bugs.freedesktop.org/show_bug.cgi?id=107825
Signed-off-by: Jean Delvare jdelvare@suse.de Cc: Alex Deucher alexander.deucher@amd.com Cc: "Christian König" christian.koenig@amd.com Cc: "David (ChunMing) Zhou" David1.Zhou@amd.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: Harry Wentland harry.wentland@amd.com Cc: Leo Li sunpeng.li@amd.com --- drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 4 ++-- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 7 +++---- 5 files changed, 11 insertions(+), 12 deletions(-)
--- linux-5.2.orig/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 2019-07-08 00:41:56.000000000 +0200 +++ linux-5.2/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 2019-08-30 14:28:46.081682223 +0200 @@ -1345,10 +1345,10 @@ static void dce_v10_0_audio_write_sad_re }
sad_count = drm_edid_to_sad(amdgpu_connector_edid(connector), &sads); - if (sad_count <= 0) { + if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); + if (sad_count <= 0) return; - } BUG_ON(!sads);
for (i = 0; i < ARRAY_SIZE(eld_reg_to_type); i++) { --- linux-5.2.orig/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 2019-07-08 00:41:56.000000000 +0200 +++ linux-5.2/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 2019-08-30 14:29:27.276205310 +0200 @@ -1371,10 +1371,10 @@ static void dce_v11_0_audio_write_sad_re }
sad_count = drm_edid_to_sad(amdgpu_connector_edid(connector), &sads); - if (sad_count <= 0) { + if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); + if (sad_count <= 0) return; - } BUG_ON(!sads);
for (i = 0; i < ARRAY_SIZE(eld_reg_to_type); i++) { --- linux-5.2.orig/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 2019-07-08 00:41:56.000000000 +0200 +++ linux-5.2/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 2019-08-30 17:58:53.613953458 +0200 @@ -1248,10 +1248,10 @@ static void dce_v6_0_audio_write_sad_reg }
sad_count = drm_edid_to_sad(amdgpu_connector_edid(connector), &sads); - if (sad_count <= 0) { + if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); + if (sad_count <= 0) return; - }
for (i = 0; i < ARRAY_SIZE(eld_reg_to_type); i++) { u32 tmp = 0; --- linux-5.2.orig/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 2019-07-08 00:41:56.000000000 +0200 +++ linux-5.2/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 2019-08-30 14:29:01.948883708 +0200 @@ -1298,10 +1298,10 @@ static void dce_v8_0_audio_write_sad_reg }
sad_count = drm_edid_to_sad(amdgpu_connector_edid(connector), &sads); - if (sad_count <= 0) { + if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); + if (sad_count <= 0) return; - } BUG_ON(!sads);
for (i = 0; i < ARRAY_SIZE(eld_reg_to_type); i++) { --- linux-5.2.orig/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 2019-07-08 00:41:56.000000000 +0200 +++ linux-5.2/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 2019-08-30 14:31:03.086421910 +0200 @@ -98,11 +98,10 @@ enum dc_edid_status dm_helpers_parse_edi (struct edid *) edid->raw_edid);
sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads); - if (sad_count <= 0) { - DRM_INFO("SADs count is: %d, don't need to read it\n", - sad_count); + if (sad_count < 0) + DRM_ERROR("Couldn't read SADs: %d\n", sad_count); + if (sad_count <= 0) return result; - }
edid_caps->audio_mode_count = sad_count < DC_MAX_AUDIO_DESC_COUNT ? sad_count : DC_MAX_AUDIO_DESC_COUNT; for (i = 0; i < edid_caps->audio_mode_count; ++i) {
It is fine for displays without audio functionality to not provide any SAD block in their EDID. Do not log an error in that case, just return quietly.
Inspired by a similar fix to the amdgpu driver in the context of bug fdo#107825: https://bugs.freedesktop.org/show_bug.cgi?id=107825
Signed-off-by: Jean Delvare jdelvare@suse.de Cc: Alex Deucher alexander.deucher@amd.com Cc: "Christian König" christian.koenig@amd.com Cc: "David (ChunMing) Zhou" David1.Zhou@amd.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch --- drivers/gpu/drm/radeon/radeon_audio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
--- linux-5.2.orig/drivers/gpu/drm/radeon/radeon_audio.c 2019-08-30 18:04:15.125056697 +0200 +++ linux-5.2/drivers/gpu/drm/radeon/radeon_audio.c 2019-08-30 18:04:35.078311347 +0200 @@ -367,10 +367,10 @@ static void radeon_audio_write_sad_regs( return;
sad_count = drm_edid_to_sad(radeon_connector_edid(connector), &sads); - if (sad_count <= 0) { + if (sad_count < 0) DRM_ERROR("Couldn't read SADs: %d\n", sad_count); + if (sad_count <= 0) return; - } BUG_ON(!sads);
if (radeon_encoder->audio && radeon_encoder->audio->write_sad_regs)
Oops, sorry, I messed up the subject line of that one, which should really read "drm/radeon: be quiet when no SAD block is found".
It is fine for displays without audio functionality to not implement CEA extension in their EDID. Do not return an error in that case, instead return 0 as if there was a CEA extension with no audio or speaker block.
This fixes half of bug fdo#107825: https://bugs.freedesktop.org/show_bug.cgi?id=107825
Signed-off-by: Jean Delvare jdelvare@suse.de Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard maxime.ripard@bootlin.com Cc: Sean Paul sean@poorly.run Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch --- drivers/gpu/drm/drm_edid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
--- linux-5.2.orig/drivers/gpu/drm/drm_edid.c 2019-08-30 17:57:38.199990995 +0200 +++ linux-5.2/drivers/gpu/drm/drm_edid.c 2019-08-30 18:04:36.840333834 +0200 @@ -4130,7 +4130,7 @@ int drm_edid_to_sad(struct edid *edid, s cea = drm_find_cea_extension(edid); if (!cea) { DRM_DEBUG_KMS("SAD: no CEA Extension found\n"); - return -ENOENT; + return 0; }
if (cea_revision(cea) < 3) { @@ -4191,7 +4191,7 @@ int drm_edid_to_speaker_allocation(struc cea = drm_find_cea_extension(edid); if (!cea) { DRM_DEBUG_KMS("SAD: no CEA Extension found\n"); - return -ENOENT; + return 0; }
if (cea_revision(cea) < 3) {
On Fri, Aug 30, 2019 at 06:16:52PM +0200, Jean Delvare wrote:
It is fine for displays without audio functionality to not implement CEA extension in their EDID. Do not return an error in that case, instead return 0 as if there was a CEA extension with no audio or speaker block.
This fixes half of bug fdo#107825: https://bugs.freedesktop.org/show_bug.cgi?id=107825
Signed-off-by: Jean Delvare jdelvare@suse.de Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard maxime.ripard@bootlin.com Cc: Sean Paul sean@poorly.run Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch
drivers/gpu/drm/drm_edid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
--- linux-5.2.orig/drivers/gpu/drm/drm_edid.c 2019-08-30 17:57:38.199990995 +0200 +++ linux-5.2/drivers/gpu/drm/drm_edid.c 2019-08-30 18:04:36.840333834 +0200 @@ -4130,7 +4130,7 @@ int drm_edid_to_sad(struct edid *edid, s cea = drm_find_cea_extension(edid); if (!cea) { DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
return -ENOENT;
return 0;
Seems reasonable. Maybe the cea_revision<3 branches should alse return 0?
Either way Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
if (cea_revision(cea) < 3) { @@ -4191,7 +4191,7 @@ int drm_edid_to_speaker_allocation(struc cea = drm_find_cea_extension(edid); if (!cea) { DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
return -ENOENT;
return 0;}
if (cea_revision(cea) < 3) {
-- Jean Delvare SUSE L3 Support _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, 2 Sep 2019 14:46:51 +0300, Ville Syrjälä wrote:
On Fri, Aug 30, 2019 at 06:16:52PM +0200, Jean Delvare wrote:
It is fine for displays without audio functionality to not implement CEA extension in their EDID. Do not return an error in that case, instead return 0 as if there was a CEA extension with no audio or speaker block.
This fixes half of bug fdo#107825: https://bugs.freedesktop.org/show_bug.cgi?id=107825
Signed-off-by: Jean Delvare jdelvare@suse.de Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard maxime.ripard@bootlin.com Cc: Sean Paul sean@poorly.run Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch
drivers/gpu/drm/drm_edid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
--- linux-5.2.orig/drivers/gpu/drm/drm_edid.c 2019-08-30 17:57:38.199990995 +0200 +++ linux-5.2/drivers/gpu/drm/drm_edid.c 2019-08-30 18:04:36.840333834 +0200 @@ -4130,7 +4130,7 @@ int drm_edid_to_sad(struct edid *edid, s cea = drm_find_cea_extension(edid); if (!cea) { DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
return -ENOENT;
return 0;Seems reasonable. Maybe the cea_revision<3 branches should alse return 0?
I wasn't sure about that one, as I'm not familiar with this CEA extension thing.
If revision < 3 means the data is invalid then returning an error is fine. If on the other hand revision < 3 simply means that the block types we are looking for were not defined back then yes returning 0 instead would be better.
I'll do whatever developers more familiar with this topic think is better.
Either way Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
Thanks,
On Mon, Sep 02, 2019 at 01:55:21PM +0200, Jean Delvare wrote:
On Mon, 2 Sep 2019 14:46:51 +0300, Ville Syrjälä wrote:
On Fri, Aug 30, 2019 at 06:16:52PM +0200, Jean Delvare wrote:
It is fine for displays without audio functionality to not implement CEA extension in their EDID. Do not return an error in that case, instead return 0 as if there was a CEA extension with no audio or speaker block.
This fixes half of bug fdo#107825: https://bugs.freedesktop.org/show_bug.cgi?id=107825
Signed-off-by: Jean Delvare jdelvare@suse.de Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard maxime.ripard@bootlin.com Cc: Sean Paul sean@poorly.run Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch
drivers/gpu/drm/drm_edid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
--- linux-5.2.orig/drivers/gpu/drm/drm_edid.c 2019-08-30 17:57:38.199990995 +0200 +++ linux-5.2/drivers/gpu/drm/drm_edid.c 2019-08-30 18:04:36.840333834 +0200 @@ -4130,7 +4130,7 @@ int drm_edid_to_sad(struct edid *edid, s cea = drm_find_cea_extension(edid); if (!cea) { DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
return -ENOENT;
return 0;Seems reasonable. Maybe the cea_revision<3 branches should alse return 0?
I wasn't sure about that one, as I'm not familiar with this CEA extension thing.
If revision < 3 means the data is invalid then returning an error is fine. If on the other hand revision < 3 simply means that the block types we are looking for were not defined back then yes returning 0 instead would be better.
That is indeed the case. A quick read through the code showed that we're not 100% consistent in checing for that though. I just send a few patches to fix that up.
dri-devel@lists.freedesktop.org
-
Jean Delvare -
Ville Syrjälä