drm/amdgpu/dm: Do not throw an error for a display with no audio
diff mbox series

Message ID 20191114204413.457504-1-chris@chris-wilson.co.uk
State New
Headers show
Series
  • drm/amdgpu/dm: Do not throw an error for a display with no audio
Related show

Commit Message

Chris Wilson Nov. 14, 2019, 8:44 p.m. UTC
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(-)

Comments

Harry Wentland Nov. 14, 2019, 9:23 p.m. UTC | #1
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;
>  
>
Alex Deucher Nov. 14, 2019, 9:47 p.m. UTC | #2
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
Jean Delvare Nov. 15, 2019, 9:04 a.m. UTC | #3
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?
Daniel Vetter Nov. 15, 2019, 9:18 a.m. UTC | #4
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
Ville Syrjälä Nov. 15, 2019, 7:27 p.m. UTC | #5
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.

Patch
diff mbox series

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;