diff mbox

radeon_connector->audio is set by RADEON_AUDIO_DISABLE as default.

Message ID 570E6BB7.4060401@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hyungwon Hwang April 13, 2016, 3:54 p.m. UTC
Dear all,

I switched my desktop environment to GNOME wayland recently, and I found
that no sound in this environment. In X desktop environment, the ioctl
DRM_IOCTL_MODE_SETPROPERTY(I confused it with
DRM_IOCTL_MODE_OBJ_SETPROPERTY - I deleted the log already :( ) is
called by userspace and it makes the sound works. But in Gnome wayland
desktop environment, the ioctl is not called. I tried to fixed it, and
found that it is because radeon_connector->audio is set by
RADEON_AUDIO_DISABLE.

In atombios_encoders.c, atombios_get_encoder_mode()
if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
        return ATOM_ENCODER_MODE_HDMI;
else if (drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
         (radeon_connector->audio == RADEON_AUDIO_AUTO))
        return ATOM_ENCODER_MODE_HDMI;
else
        return ATOM_ENCODER_MODE_DVI;

This code returns ATOM_ENCODER_MODE_DVI.

In atombios_encoders.c, radeon_atom_encoder_mode_set():
encoder_mode = atombios_get_encoder_mode(encoder);

if (connector && (radeon_audio != 0) &&

    ((encoder_mode == ATOM_ENCODER_MODE_HDMI) ||

     ENCODER_MODE_IS_DP(encoder_mode)))

        radeon_audio_mode_set(encoder, adjusted_mode);b

So this code bypasses the calling of radeon_audio_mode_set().

I think that radeon_connector->audio should be set  by
RADEON_AUDIO_AUTO, at least for connectors which can output audio. I
fixed the code like below, and it works for me. But I am not familiar
with radeon DRM driver, and can't see the big picture. Can you review
this code?

Thanks,
Hyungwon Hwang

                                           &radeon_dp_connector_funcs,
connector_type);
                        drm_connector_helper_add(&radeon_connector->base,
@@ -2024,8 +2025,9 @@ radeon_add_atom_connector(struct drm_device *dev,
                                                              1);
                        }
                        break;
-               case DRM_MODE_CONNECTOR_LVDS:
                case DRM_MODE_CONNECTOR_eDP:
+                       radeon_connector->audio = RADEON_AUDIO_AUTO;
+               case DRM_MODE_CONNECTOR_LVDS:
                        drm_connector_init(dev, &radeon_connector->base,

&radeon_lvds_bridge_connector_funcs, connector_type);
                        drm_connector_helper_add(&radeon_connector->base,
@@ -2196,6 +2198,7 @@ radeon_add_atom_connector(struct drm_device *dev,
                                connector->doublescan_allowed = true;
                        else
                                connector->doublescan_allowed = false;
+                       radeon_connector->audio = RADEON_AUDIO_AUTO;
                        break;
                case DRM_MODE_CONNECTOR_DisplayPort:
                        radeon_dig_connector = kzalloc(sizeof(struct
radeon_connector_atom_dig), GFP_KERNEL);
@@ -2245,6 +2248,7 @@ radeon_add_atom_connector(struct drm_device *dev,
                        connector->interlace_allowed = true;
                        /* in theory with a DP to VGA converter... */
                        connector->doublescan_allowed = false;
+                       radeon_connector->audio = RADEON_AUDIO_AUTO;
                        break;
                case DRM_MODE_CONNECTOR_eDP:
                        radeon_dig_connector = kzalloc(sizeof(struct
radeon_connector_atom_dig), GFP_KERNEL);
@@ -2267,6 +2271,7 @@ radeon_add_atom_connector(struct drm_device *dev,
                        subpixel_order = SubPixelHorizontalRGB;
                        connector->interlace_allowed = false;
                        connector->doublescan_allowed = false;
+                       radeon_connector->audio = RADEON_AUDIO_AUTO;
                        break;
                case DRM_MODE_CONNECTOR_SVIDEO:
                case DRM_MODE_CONNECTOR_Composite:

Comments

Alex Deucher April 13, 2016, 4:12 p.m. UTC | #1
> -----Original Message-----

> From: Hyungwon Hwang [mailto:hyungwon.hwang7@gmail.com]

> Sent: Wednesday, April 13, 2016 11:55 AM

> To: Deucher, Alexander; Koenig, Christian; dri-devel@lists.freedesktop.org

> Subject: radeon_connector->audio is set by RADEON_AUDIO_DISABLE as

> default.

> 

> Dear all,

> 

> I switched my desktop environment to GNOME wayland recently, and I

> found

> that no sound in this environment. In X desktop environment, the ioctl

> DRM_IOCTL_MODE_SETPROPERTY(I confused it with

> DRM_IOCTL_MODE_OBJ_SETPROPERTY - I deleted the log already :( ) is

> called by userspace and it makes the sound works. But in Gnome wayland

> desktop environment, the ioctl is not called. I tried to fixed it, and

> found that it is because radeon_connector->audio is set by

> RADEON_AUDIO_DISABLE.


Thanks for spotting this.  Does the attached patch fix it?

Alex

> 

> In atombios_encoders.c, atombios_get_encoder_mode()

> if (radeon_connector->audio == RADEON_AUDIO_ENABLE)

>         return ATOM_ENCODER_MODE_HDMI;

> else if (drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&

>          (radeon_connector->audio == RADEON_AUDIO_AUTO))

>         return ATOM_ENCODER_MODE_HDMI;

> else

>         return ATOM_ENCODER_MODE_DVI;

> 

> This code returns ATOM_ENCODER_MODE_DVI.

> 

> In atombios_encoders.c, radeon_atom_encoder_mode_set():

> encoder_mode = atombios_get_encoder_mode(encoder);

> 

> if (connector && (radeon_audio != 0) &&

> 

>     ((encoder_mode == ATOM_ENCODER_MODE_HDMI) ||

> 

>      ENCODER_MODE_IS_DP(encoder_mode)))

> 

>         radeon_audio_mode_set(encoder, adjusted_mode);b

> 

> So this code bypasses the calling of radeon_audio_mode_set().

> 

> I think that radeon_connector->audio should be set  by

> RADEON_AUDIO_AUTO, at least for connectors which can output audio. I

> fixed the code like below, and it works for me. But I am not familiar

> with radeon DRM driver, and can't see the big picture. Can you review

> this code?

> 

> Thanks,

> Hyungwon Hwang

> 

> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c

> b/drivers/gpu/drm/radeon/radeon_connectors.c

> index cfcc099..cf52ea5 100644

> --- a/drivers/gpu/drm/radeon/radeon_connectors.c

> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c

> @@ -1975,11 +1975,12 @@ radeon_add_atom_connector(struct drm_device

> *dev,

> 

> rdev->mode_info.output_csc_property,

> 

> RADEON_OUTPUT_CSC_BYPASS);

>                         break;

> -               case DRM_MODE_CONNECTOR_DVII:

> -               case DRM_MODE_CONNECTOR_DVID:

>                 case DRM_MODE_CONNECTOR_HDMIA:

>                 case DRM_MODE_CONNECTOR_HDMIB:

>                 case DRM_MODE_CONNECTOR_DisplayPort:

> +                       radeon_connector->audio = RADEON_AUDIO_AUTO;

> +               case DRM_MODE_CONNECTOR_DVII:

> +               case DRM_MODE_CONNECTOR_DVID:

>                         drm_connector_init(dev, &radeon_connector->base,

>                                            &radeon_dp_connector_funcs,

> connector_type);

>                         drm_connector_helper_add(&radeon_connector->base,

> @@ -2024,8 +2025,9 @@ radeon_add_atom_connector(struct drm_device

> *dev,

>                                                               1);

>                         }

>                         break;

> -               case DRM_MODE_CONNECTOR_LVDS:

>                 case DRM_MODE_CONNECTOR_eDP:

> +                       radeon_connector->audio = RADEON_AUDIO_AUTO;

> +               case DRM_MODE_CONNECTOR_LVDS:

>                         drm_connector_init(dev, &radeon_connector->base,

> 

> &radeon_lvds_bridge_connector_funcs, connector_type);

>                         drm_connector_helper_add(&radeon_connector->base,

> @@ -2196,6 +2198,7 @@ radeon_add_atom_connector(struct drm_device

> *dev,

>                                 connector->doublescan_allowed = true;

>                         else

>                                 connector->doublescan_allowed = false;

> +                       radeon_connector->audio = RADEON_AUDIO_AUTO;

>                         break;

>                 case DRM_MODE_CONNECTOR_DisplayPort:

>                         radeon_dig_connector = kzalloc(sizeof(struct

> radeon_connector_atom_dig), GFP_KERNEL);

> @@ -2245,6 +2248,7 @@ radeon_add_atom_connector(struct drm_device

> *dev,

>                         connector->interlace_allowed = true;

>                         /* in theory with a DP to VGA converter... */

>                         connector->doublescan_allowed = false;

> +                       radeon_connector->audio = RADEON_AUDIO_AUTO;

>                         break;

>                 case DRM_MODE_CONNECTOR_eDP:

>                         radeon_dig_connector = kzalloc(sizeof(struct

> radeon_connector_atom_dig), GFP_KERNEL);

> @@ -2267,6 +2271,7 @@ radeon_add_atom_connector(struct drm_device

> *dev,

>                         subpixel_order = SubPixelHorizontalRGB;

>                         connector->interlace_allowed = false;

>                         connector->doublescan_allowed = false;

> +                       radeon_connector->audio = RADEON_AUDIO_AUTO;

>                         break;

>                 case DRM_MODE_CONNECTOR_SVIDEO:

>                 case DRM_MODE_CONNECTOR_Composite:
Hyungwon Hwang April 14, 2016, 2:59 a.m. UTC | #2
2016. 4. 14. ?? 1:12? "Deucher, Alexander" <Alexander.Deucher@amd.com>?? ??:
>
> > -----Original Message-----
> > From: Hyungwon Hwang [mailto:hyungwon.hwang7@gmail.com]
> > Sent: Wednesday, April 13, 2016 11:55 AM
> > To: Deucher, Alexander; Koenig, Christian;
dri-devel@lists.freedesktop.org
> > Subject: radeon_connector->audio is set by RADEON_AUDIO_DISABLE as
> > default.
> >
> > Dear all,
> >
> > I switched my desktop environment to GNOME wayland recently, and I
> > found
> > that no sound in this environment. In X desktop environment, the ioctl
> > DRM_IOCTL_MODE_SETPROPERTY(I confused it with
> > DRM_IOCTL_MODE_OBJ_SETPROPERTY - I deleted the log already :( ) is
> > called by userspace and it makes the sound works. But in Gnome wayland
> > desktop environment, the ioctl is not called. I tried to fixed it, and
> > found that it is because radeon_connector->audio is set by
> > RADEON_AUDIO_DISABLE.
>
> Thanks for spotting this.  Does the attached patch fix it?
>
> Alex

Yes. But I could test it only in my environment (radeon hd 5700 + hdmi
monitor).

Thanks,
Hyungwon Hwang

>
> >
> > In atombios_encoders.c, atombios_get_encoder_mode()
> > if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
> >         return ATOM_ENCODER_MODE_HDMI;
> > else if (drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
> >          (radeon_connector->audio == RADEON_AUDIO_AUTO))
> >         return ATOM_ENCODER_MODE_HDMI;
> > else
> >         return ATOM_ENCODER_MODE_DVI;
> >
> > This code returns ATOM_ENCODER_MODE_DVI.
> >
> > In atombios_encoders.c, radeon_atom_encoder_mode_set():
> > encoder_mode = atombios_get_encoder_mode(encoder);
> >
> > if (connector && (radeon_audio != 0) &&
> >
> >     ((encoder_mode == ATOM_ENCODER_MODE_HDMI) ||
> >
> >      ENCODER_MODE_IS_DP(encoder_mode)))
> >
> >         radeon_audio_mode_set(encoder, adjusted_mode);b
> >
> > So this code bypasses the calling of radeon_audio_mode_set().
> >
> > I think that radeon_connector->audio should be set  by
> > RADEON_AUDIO_AUTO, at least for connectors which can output audio. I
> > fixed the code like below, and it works for me. But I am not familiar
> > with radeon DRM driver, and can't see the big picture. Can you review
> > this code?
> >
> > Thanks,
> > Hyungwon Hwang
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
> > b/drivers/gpu/drm/radeon/radeon_connectors.c
> > index cfcc099..cf52ea5 100644
> > --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> > +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> > @@ -1975,11 +1975,12 @@ radeon_add_atom_connector(struct drm_device
> > *dev,
> >
> > rdev->mode_info.output_csc_property,
> >
> > RADEON_OUTPUT_CSC_BYPASS);
> >                         break;
> > -               case DRM_MODE_CONNECTOR_DVII:
> > -               case DRM_MODE_CONNECTOR_DVID:
> >                 case DRM_MODE_CONNECTOR_HDMIA:
> >                 case DRM_MODE_CONNECTOR_HDMIB:
> >                 case DRM_MODE_CONNECTOR_DisplayPort:
> > +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
> > +               case DRM_MODE_CONNECTOR_DVII:
> > +               case DRM_MODE_CONNECTOR_DVID:
> >                         drm_connector_init(dev, &radeon_connector->base,
> >                                            &radeon_dp_connector_funcs,
> > connector_type);
> >
 drm_connector_helper_add(&radeon_connector->base,
> > @@ -2024,8 +2025,9 @@ radeon_add_atom_connector(struct drm_device
> > *dev,
> >                                                               1);
> >                         }
> >                         break;
> > -               case DRM_MODE_CONNECTOR_LVDS:
> >                 case DRM_MODE_CONNECTOR_eDP:
> > +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
> > +               case DRM_MODE_CONNECTOR_LVDS:
> >                         drm_connector_init(dev, &radeon_connector->base,
> >
> > &radeon_lvds_bridge_connector_funcs, connector_type);
> >
 drm_connector_helper_add(&radeon_connector->base,
> > @@ -2196,6 +2198,7 @@ radeon_add_atom_connector(struct drm_device
> > *dev,
> >                                 connector->doublescan_allowed = true;
> >                         else
> >                                 connector->doublescan_allowed = false;
> > +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
> >                         break;
> >                 case DRM_MODE_CONNECTOR_DisplayPort:
> >                         radeon_dig_connector = kzalloc(sizeof(struct
> > radeon_connector_atom_dig), GFP_KERNEL);
> > @@ -2245,6 +2248,7 @@ radeon_add_atom_connector(struct drm_device
> > *dev,
> >                         connector->interlace_allowed = true;
> >                         /* in theory with a DP to VGA converter... */
> >                         connector->doublescan_allowed = false;
> > +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
> >                         break;
> >                 case DRM_MODE_CONNECTOR_eDP:
> >                         radeon_dig_connector = kzalloc(sizeof(struct
> > radeon_connector_atom_dig), GFP_KERNEL);
> > @@ -2267,6 +2271,7 @@ radeon_add_atom_connector(struct drm_device
> > *dev,
> >                         subpixel_order = SubPixelHorizontalRGB;
> >                         connector->interlace_allowed = false;
> >                         connector->doublescan_allowed = false;
> > +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
> >                         break;
> >                 case DRM_MODE_CONNECTOR_SVIDEO:
> >                 case DRM_MODE_CONNECTOR_Composite:
Hyungwon Hwang April 14, 2016, 3:03 a.m. UTC | #3
2016. 4. 14. ?? 11:59? "Hyungwon Hwang" <hyungwon.hwang7@gmail.com>?? ??:
>
>
> 2016. 4. 14. ?? 1:12? "Deucher, Alexander" <Alexander.Deucher@amd.com>??
??:
>
> >
> > > -----Original Message-----
> > > From: Hyungwon Hwang [mailto:hyungwon.hwang7@gmail.com]
> > > Sent: Wednesday, April 13, 2016 11:55 AM
> > > To: Deucher, Alexander; Koenig, Christian;
dri-devel@lists.freedesktop.org
> > > Subject: radeon_connector->audio is set by RADEON_AUDIO_DISABLE as
> > > default.
> > >
> > > Dear all,
> > >
> > > I switched my desktop environment to GNOME wayland recently, and I
> > > found
> > > that no sound in this environment. In X desktop environment, the ioctl
> > > DRM_IOCTL_MODE_SETPROPERTY(I confused it with
> > > DRM_IOCTL_MODE_OBJ_SETPROPERTY - I deleted the log already :( ) is
> > > called by userspace and it makes the sound works. But in Gnome wayland
> > > desktop environment, the ioctl is not called. I tried to fixed it, and
> > > found that it is because radeon_connector->audio is set by
> > > RADEON_AUDIO_DISABLE.
> >
> > Thanks for spotting this.  Does the attached patch fix it?
> >
> > Alex
>
> Yes. But I could test it only in my environment (radeon hd 5700 + hdmi
monitor).
>
> Thanks,
> Hyungwon Hwang

Oh. Sorry. I couldn't see that there was an attached patch, becaue I
checked it in mailing archive. I can test it at night. After that, I will
send the reply again.

Thanks,
Hyungwon Hwang

>
> >
> > >
> > > In atombios_encoders.c, atombios_get_encoder_mode()
> > > if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
> > >         return ATOM_ENCODER_MODE_HDMI;
> > > else if (drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
> > >          (radeon_connector->audio == RADEON_AUDIO_AUTO))
> > >         return ATOM_ENCODER_MODE_HDMI;
> > > else
> > >         return ATOM_ENCODER_MODE_DVI;
> > >
> > > This code returns ATOM_ENCODER_MODE_DVI.
> > >
> > > In atombios_encoders.c, radeon_atom_encoder_mode_set():
> > > encoder_mode = atombios_get_encoder_mode(encoder);
> > >
> > > if (connector && (radeon_audio != 0) &&
> > >
> > >     ((encoder_mode == ATOM_ENCODER_MODE_HDMI) ||
> > >
> > >      ENCODER_MODE_IS_DP(encoder_mode)))
> > >
> > >         radeon_audio_mode_set(encoder, adjusted_mode);b
> > >
> > > So this code bypasses the calling of radeon_audio_mode_set().
> > >
> > > I think that radeon_connector->audio should be set  by
> > > RADEON_AUDIO_AUTO, at least for connectors which can output audio. I
> > > fixed the code like below, and it works for me. But I am not familiar
> > > with radeon DRM driver, and can't see the big picture. Can you review
> > > this code?
> > >
> > > Thanks,
> > > Hyungwon Hwang
> > >
> > > diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
> > > b/drivers/gpu/drm/radeon/radeon_connectors.c
> > > index cfcc099..cf52ea5 100644
> > > --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> > > +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> > > @@ -1975,11 +1975,12 @@ radeon_add_atom_connector(struct drm_device
> > > *dev,
> > >
> > > rdev->mode_info.output_csc_property,
> > >
> > > RADEON_OUTPUT_CSC_BYPASS);
> > >                         break;
> > > -               case DRM_MODE_CONNECTOR_DVII:
> > > -               case DRM_MODE_CONNECTOR_DVID:
> > >                 case DRM_MODE_CONNECTOR_HDMIA:
> > >                 case DRM_MODE_CONNECTOR_HDMIB:
> > >                 case DRM_MODE_CONNECTOR_DisplayPort:
> > > +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
> > > +               case DRM_MODE_CONNECTOR_DVII:
> > > +               case DRM_MODE_CONNECTOR_DVID:
> > >                         drm_connector_init(dev,
&radeon_connector->base,
> > >                                            &radeon_dp_connector_funcs,
> > > connector_type);
> > >
 drm_connector_helper_add(&radeon_connector->base,
> > > @@ -2024,8 +2025,9 @@ radeon_add_atom_connector(struct drm_device
> > > *dev,
> > >                                                               1);
> > >                         }
> > >                         break;
> > > -               case DRM_MODE_CONNECTOR_LVDS:
> > >                 case DRM_MODE_CONNECTOR_eDP:
> > > +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
> > > +               case DRM_MODE_CONNECTOR_LVDS:
> > >                         drm_connector_init(dev,
&radeon_connector->base,
> > >
> > > &radeon_lvds_bridge_connector_funcs, connector_type);
> > >
 drm_connector_helper_add(&radeon_connector->base,
> > > @@ -2196,6 +2198,7 @@ radeon_add_atom_connector(struct drm_device
> > > *dev,
> > >                                 connector->doublescan_allowed = true;
> > >                         else
> > >                                 connector->doublescan_allowed = false;
> > > +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
> > >                         break;
> > >                 case DRM_MODE_CONNECTOR_DisplayPort:
> > >                         radeon_dig_connector = kzalloc(sizeof(struct
> > > radeon_connector_atom_dig), GFP_KERNEL);
> > > @@ -2245,6 +2248,7 @@ radeon_add_atom_connector(struct drm_device
> > > *dev,
> > >                         connector->interlace_allowed = true;
> > >                         /* in theory with a DP to VGA converter... */
> > >                         connector->doublescan_allowed = false;
> > > +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
> > >                         break;
> > >                 case DRM_MODE_CONNECTOR_eDP:
> > >                         radeon_dig_connector = kzalloc(sizeof(struct
> > > radeon_connector_atom_dig), GFP_KERNEL);
> > > @@ -2267,6 +2271,7 @@ radeon_add_atom_connector(struct drm_device
> > > *dev,
> > >                         subpixel_order = SubPixelHorizontalRGB;
> > >                         connector->interlace_allowed = false;
> > >                         connector->doublescan_allowed = false;
> > > +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
> > >                         break;
> > >                 case DRM_MODE_CONNECTOR_SVIDEO:
> > >                 case DRM_MODE_CONNECTOR_Composite:
Hyungwon Hwang April 14, 2016, 2:02 p.m. UTC | #4
2016? 04? 14? 01:12? Deucher, Alexander ?(?) ? ?:
>> -----Original Message-----
>> From: Hyungwon Hwang [mailto:hyungwon.hwang7@gmail.com]
>> Sent: Wednesday, April 13, 2016 11:55 AM
>> To: Deucher, Alexander; Koenig, Christian; dri-devel@lists.freedesktop.org
>> Subject: radeon_connector->audio is set by RADEON_AUDIO_DISABLE as
>> default.
>>
>> Dear all,
>>
>> I switched my desktop environment to GNOME wayland recently, and I
>> found
>> that no sound in this environment. In X desktop environment, the ioctl
>> DRM_IOCTL_MODE_SETPROPERTY(I confused it with
>> DRM_IOCTL_MODE_OBJ_SETPROPERTY - I deleted the log already :( ) is
>> called by userspace and it makes the sound works. But in Gnome wayland
>> desktop environment, the ioctl is not called. I tried to fixed it, and
>> found that it is because radeon_connector->audio is set by
>> RADEON_AUDIO_DISABLE.
> 
> Thanks for spotting this.  Does the attached patch fix it?
> 
> Alex

Great. Now it works with your patch. Thanks for your work.

Best regards,
Hyungwon Hwang

> 
>>
>> In atombios_encoders.c, atombios_get_encoder_mode()
>> if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
>>         return ATOM_ENCODER_MODE_HDMI;
>> else if (drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
>>          (radeon_connector->audio == RADEON_AUDIO_AUTO))
>>         return ATOM_ENCODER_MODE_HDMI;
>> else
>>         return ATOM_ENCODER_MODE_DVI;
>>
>> This code returns ATOM_ENCODER_MODE_DVI.
>>
>> In atombios_encoders.c, radeon_atom_encoder_mode_set():
>> encoder_mode = atombios_get_encoder_mode(encoder);
>>
>> if (connector && (radeon_audio != 0) &&
>>
>>     ((encoder_mode == ATOM_ENCODER_MODE_HDMI) ||
>>
>>      ENCODER_MODE_IS_DP(encoder_mode)))
>>
>>         radeon_audio_mode_set(encoder, adjusted_mode);b
>>
>> So this code bypasses the calling of radeon_audio_mode_set().
>>
>> I think that radeon_connector->audio should be set  by
>> RADEON_AUDIO_AUTO, at least for connectors which can output audio. I
>> fixed the code like below, and it works for me. But I am not familiar
>> with radeon DRM driver, and can't see the big picture. Can you review
>> this code?
>>
>> Thanks,
>> Hyungwon Hwang
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
>> b/drivers/gpu/drm/radeon/radeon_connectors.c
>> index cfcc099..cf52ea5 100644
>> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
>> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
>> @@ -1975,11 +1975,12 @@ radeon_add_atom_connector(struct drm_device
>> *dev,
>>
>> rdev->mode_info.output_csc_property,
>>
>> RADEON_OUTPUT_CSC_BYPASS);
>>                         break;
>> -               case DRM_MODE_CONNECTOR_DVII:
>> -               case DRM_MODE_CONNECTOR_DVID:
>>                 case DRM_MODE_CONNECTOR_HDMIA:
>>                 case DRM_MODE_CONNECTOR_HDMIB:
>>                 case DRM_MODE_CONNECTOR_DisplayPort:
>> +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
>> +               case DRM_MODE_CONNECTOR_DVII:
>> +               case DRM_MODE_CONNECTOR_DVID:
>>                         drm_connector_init(dev, &radeon_connector->base,
>>                                            &radeon_dp_connector_funcs,
>> connector_type);
>>                         drm_connector_helper_add(&radeon_connector->base,
>> @@ -2024,8 +2025,9 @@ radeon_add_atom_connector(struct drm_device
>> *dev,
>>                                                               1);
>>                         }
>>                         break;
>> -               case DRM_MODE_CONNECTOR_LVDS:
>>                 case DRM_MODE_CONNECTOR_eDP:
>> +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
>> +               case DRM_MODE_CONNECTOR_LVDS:
>>                         drm_connector_init(dev, &radeon_connector->base,
>>
>> &radeon_lvds_bridge_connector_funcs, connector_type);
>>                         drm_connector_helper_add(&radeon_connector->base,
>> @@ -2196,6 +2198,7 @@ radeon_add_atom_connector(struct drm_device
>> *dev,
>>                                 connector->doublescan_allowed = true;
>>                         else
>>                                 connector->doublescan_allowed = false;
>> +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
>>                         break;
>>                 case DRM_MODE_CONNECTOR_DisplayPort:
>>                         radeon_dig_connector = kzalloc(sizeof(struct
>> radeon_connector_atom_dig), GFP_KERNEL);
>> @@ -2245,6 +2248,7 @@ radeon_add_atom_connector(struct drm_device
>> *dev,
>>                         connector->interlace_allowed = true;
>>                         /* in theory with a DP to VGA converter... */
>>                         connector->doublescan_allowed = false;
>> +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
>>                         break;
>>                 case DRM_MODE_CONNECTOR_eDP:
>>                         radeon_dig_connector = kzalloc(sizeof(struct
>> radeon_connector_atom_dig), GFP_KERNEL);
>> @@ -2267,6 +2271,7 @@ radeon_add_atom_connector(struct drm_device
>> *dev,
>>                         subpixel_order = SubPixelHorizontalRGB;
>>                         connector->interlace_allowed = false;
>>                         connector->doublescan_allowed = false;
>> +                       radeon_connector->audio = RADEON_AUDIO_AUTO;
>>                         break;
>>                 case DRM_MODE_CONNECTOR_SVIDEO:
>>                 case DRM_MODE_CONNECTOR_Composite:
diff mbox

Patch

diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index cfcc099..cf52ea5 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1975,11 +1975,12 @@  radeon_add_atom_connector(struct drm_device *dev,

rdev->mode_info.output_csc_property,

RADEON_OUTPUT_CSC_BYPASS);
                        break;
-               case DRM_MODE_CONNECTOR_DVII:
-               case DRM_MODE_CONNECTOR_DVID:
                case DRM_MODE_CONNECTOR_HDMIA:
                case DRM_MODE_CONNECTOR_HDMIB:
                case DRM_MODE_CONNECTOR_DisplayPort:
+                       radeon_connector->audio = RADEON_AUDIO_AUTO;
+               case DRM_MODE_CONNECTOR_DVII:
+               case DRM_MODE_CONNECTOR_DVID:
                        drm_connector_init(dev, &radeon_connector->base,