Message ID | 570E6BB7.4060401@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
> -----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:
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:
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:
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 --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,