| Message ID | 20191114204413.457504-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amdgpu/dm: Do not throw an error for a display with no audio | expand |
On 2019-11-14 3:44 p.m., Chris Wilson wrote: > An old display with no audio may not have an EDID with a CEA block, or > it may simply be too old to support audio. This is not a driver error, > so don't flag it as such. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112140 > References: ae2a3495973e ("drm/amd: be quiet when no SAD block is found") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Harry > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Jean Delvare <jdelvare@suse.de> > Cc: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > index 11e5784aa62a..04808dbecab3 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > @@ -97,8 +97,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps( > (struct edid *) edid->raw_edid); > > sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads); > - if (sad_count < 0) > - DRM_ERROR("Couldn't read SADs: %d\n", sad_count); > if (sad_count <= 0) > return result; > >
On Thu, Nov 14, 2019 at 4:23 PM Harry Wentland <hwentlan@amd.com> wrote: > > On 2019-11-14 3:44 p.m., Chris Wilson wrote: > > An old display with no audio may not have an EDID with a CEA block, or > > it may simply be too old to support audio. This is not a driver error, > > so don't flag it as such. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112140 > > References: ae2a3495973e ("drm/amd: be quiet when no SAD block is found") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Reviewed-by: Harry Wentland <harry.wentland@amd.com> > Applied. thanks! Alex > Harry > > > Cc: Harry Wentland <harry.wentland@amd.com> > > Cc: Jean Delvare <jdelvare@suse.de> > > Cc: Alex Deucher <alexander.deucher@amd.com> > > --- > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > index 11e5784aa62a..04808dbecab3 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > @@ -97,8 +97,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps( > > (struct edid *) edid->raw_edid); > > > > sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads); > > - if (sad_count < 0) > > - DRM_ERROR("Couldn't read SADs: %d\n", sad_count); > > if (sad_count <= 0) > > return result; > > > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Chris, On Thu, 14 Nov 2019 20:44:13 +0000, Chris Wilson wrote: > An old display with no audio may not have an EDID with a CEA block, or > it may simply be too old to support audio. This is not a driver error, > so don't flag it as such. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112140 > References: ae2a3495973e ("drm/amd: be quiet when no SAD block is found") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Jean Delvare <jdelvare@suse.de> > Cc: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > index 11e5784aa62a..04808dbecab3 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > @@ -97,8 +97,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps( > (struct edid *) edid->raw_edid); > > sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads); > - if (sad_count < 0) > - DRM_ERROR("Couldn't read SADs: %d\n", sad_count); > if (sad_count <= 0) > return result; > I still believe that my approach was a better way to solve this problem: https://patchwork.freedesktop.org/patch/329066/ Your approach would have to be applied to all individual drivers (all callers of drm_edid_to_sad). It also prevents reporting actual errors. Should I resend my patch?
On Fri, Nov 15, 2019 at 10:04 AM Jean Delvare <jdelvare@suse.de> wrote: > > Hi Chris, > > On Thu, 14 Nov 2019 20:44:13 +0000, Chris Wilson wrote: > > An old display with no audio may not have an EDID with a CEA block, or > > it may simply be too old to support audio. This is not a driver error, > > so don't flag it as such. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112140 > > References: ae2a3495973e ("drm/amd: be quiet when no SAD block is found") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Harry Wentland <harry.wentland@amd.com> > > Cc: Jean Delvare <jdelvare@suse.de> > > Cc: Alex Deucher <alexander.deucher@amd.com> > > --- > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > index 11e5784aa62a..04808dbecab3 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > @@ -97,8 +97,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps( > > (struct edid *) edid->raw_edid); > > > > sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads); > > - if (sad_count < 0) > > - DRM_ERROR("Couldn't read SADs: %d\n", sad_count); > > if (sad_count <= 0) > > return result; > > > > I still believe that my approach was a better way to solve this problem: > > https://patchwork.freedesktop.org/patch/329066/ > > Your approach would have to be applied to all individual drivers (all > callers of drm_edid_to_sad). It also prevents reporting actual errors. > > Should I resend my patch? I think we can do both, just ask Ville to apply the patch for you (he has commit rights to drm-misc). Or resend and then ping Ville on irc. -Daniel > > -- > Jean Delvare > SUSE L3 Support > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Nov 15, 2019 at 10:18:26AM +0100, Daniel Vetter wrote: > On Fri, Nov 15, 2019 at 10:04 AM Jean Delvare <jdelvare@suse.de> wrote: > > > > Hi Chris, > > > > On Thu, 14 Nov 2019 20:44:13 +0000, Chris Wilson wrote: > > > An old display with no audio may not have an EDID with a CEA block, or > > > it may simply be too old to support audio. This is not a driver error, > > > so don't flag it as such. > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112140 > > > References: ae2a3495973e ("drm/amd: be quiet when no SAD block is found") > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Harry Wentland <harry.wentland@amd.com> > > > Cc: Jean Delvare <jdelvare@suse.de> > > > Cc: Alex Deucher <alexander.deucher@amd.com> > > > --- > > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 -- > > > 1 file changed, 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > > index 11e5784aa62a..04808dbecab3 100644 > > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > > @@ -97,8 +97,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps( > > > (struct edid *) edid->raw_edid); > > > > > > sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads); > > > - if (sad_count < 0) > > > - DRM_ERROR("Couldn't read SADs: %d\n", sad_count); > > > if (sad_count <= 0) > > > return result; > > > > > > > I still believe that my approach was a better way to solve this problem: > > > > https://patchwork.freedesktop.org/patch/329066/ > > > > Your approach would have to be applied to all individual drivers (all > > callers of drm_edid_to_sad). It also prevents reporting actual errors. > > > > Should I resend my patch? > > I think we can do both, just ask Ville to apply the patch for you (he > has commit rights to drm-misc). Or resend and then ping Ville on irc. Ideally would have been something for AMD folks since they're the only user of those functions AFAICS. But I just pushed it anyway. Thanks for the patch.
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 11e5784aa62a..04808dbecab3 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -97,8 +97,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps( (struct edid *) edid->raw_edid); sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads); - if (sad_count < 0) - DRM_ERROR("Couldn't read SADs: %d\n", sad_count); if (sad_count <= 0) return result;
An old display with no audio may not have an EDID with a CEA block, or it may simply be too old to support audio. This is not a driver error, so don't flag it as such. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112140 References: ae2a3495973e ("drm/amd: be quiet when no SAD block is found") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Harry Wentland <harry.wentland@amd.com> Cc: Jean Delvare <jdelvare@suse.de> Cc: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 -- 1 file changed, 2 deletions(-)