@@ -497,8 +497,23 @@ int dp_audio_hw_params(struct device *dev,
int rc = 0;
struct dp_audio_private *audio;
struct platform_device *pdev;
+ struct msm_dp *dp_display;
pdev = to_platform_device(dev);
+ dp_display = platform_get_drvdata(pdev);
+
+ /*
+ * there could be cases where sound card can be opened even
+ * before OR even when DP is not connected . This can cause
+ * unclocked access as the audio subsystem relies on the DP
+ * driver to maintain the correct state of clocks. To protect
+ * such cases check for connection status and bail out if not
+ * connected.
+ */
+ if (!dp_display->power_on) {
+ rc = -EINVAL;
+ goto end;
+ }
audio = dp_audio_get_data(pdev);
if (IS_ERR(audio)) {
@@ -512,6 +527,8 @@ int dp_audio_hw_params(struct device *dev,
dp_audio_setup_acr(audio);
dp_audio_safe_to_exit_level(audio);
dp_audio_enable(audio, true);
+ dp_display->audio_enabled = true;
+
end:
return rc;
}
@@ -520,15 +537,30 @@ static void dp_audio_shutdown(struct device *dev, void *data)
{
struct dp_audio_private *audio;
struct platform_device *pdev;
+ struct msm_dp *dp_display;
pdev = to_platform_device(dev);
+ dp_display = platform_get_drvdata(pdev);
audio = dp_audio_get_data(pdev);
if (IS_ERR(audio)) {
DRM_ERROR("failed to get audio data\n");
return;
}
+ /*
+ * if audio was not enabled there is no need
+ * to execute the shutdown and we can bail out early.
+ * This also makes sure that we dont cause an unclocked
+ * access when audio subsystem calls this without DP being
+ * connected. is_connected cannot be used here as its set
+ * to false earlier than this call
+ */
+ if (!dp_display->audio_enabled)
+ return;
+
dp_audio_enable(audio, false);
+ /* signal the dp display to safely shutdown clocks */
+ dp_display_signal_audio_complete(dp_display);
}
static const struct hdmi_codec_ops dp_audio_codec_ops = {
@@ -82,7 +82,6 @@ struct dp_display_private {
/* state variables */
bool core_initialized;
- bool power_on;
bool hpd_irq_on;
bool audio_supported;
@@ -104,6 +103,9 @@ struct dp_display_private {
struct dp_display_mode dp_mode;
struct msm_dp dp_display;
+ /* wait for audio signaling */
+ struct completion audio_comp;
+
/* event related only access by event thread */
struct mutex event_mutex;
wait_queue_head_t event_q;
@@ -177,6 +179,15 @@ static int dp_del_event(struct dp_display_private *dp_priv, u32 event)
return 0;
}
+void dp_display_signal_audio_complete(struct msm_dp *dp_display)
+{
+ struct dp_display_private *dp;
+
+ dp = container_of(dp_display, struct dp_display_private, dp_display);
+
+ complete_all(&dp->audio_comp);
+}
+
static int dp_display_bind(struct device *dev, struct device *master,
void *data)
{
@@ -599,6 +610,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data)
/* signal the disconnect event early to ensure proper teardown */
dp_display_handle_plugged_change(g_dp_display, false);
+ reinit_completion(&dp->audio_comp);
dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK |
DP_DP_IRQ_HPD_INT_MASK, true);
@@ -793,15 +805,18 @@ static int dp_display_prepare(struct msm_dp *dp)
static int dp_display_enable(struct dp_display_private *dp, u32 data)
{
int rc = 0;
+ struct msm_dp *dp_display;
- if (dp->power_on) {
+ dp_display = g_dp_display;
+
+ if (dp_display->power_on) {
DRM_DEBUG_DP("Link already setup, return\n");
return 0;
}
rc = dp_ctrl_on_stream(dp->ctrl);
if (!rc)
- dp->power_on = true;
+ dp_display->power_on = true;
/* complete resuem_comp regardless it is armed or not */
complete(&dp->resume_comp);
@@ -829,14 +844,27 @@ static int dp_display_post_enable(struct msm_dp *dp_display)
static int dp_display_disable(struct dp_display_private *dp, u32 data)
{
- if (!dp->power_on)
+ struct msm_dp *dp_display;
+
+ dp_display = g_dp_display;
+
+ if (!dp_display->power_on)
return -EINVAL;
+ /* wait only if audio was enabled */
+ if (dp_display->audio_enabled) {
+ if (!wait_for_completion_timeout(&dp->audio_comp,
+ HZ * 5))
+ DRM_ERROR("audio comp timeout\n");
+ }
+
+ dp_display->audio_enabled = false;
+
dp_ctrl_off(dp->ctrl);
dp->core_initialized = false;
- dp->power_on = false;
+ dp_display->power_on = false;
return 0;
}
@@ -1152,6 +1180,8 @@ static int dp_display_probe(struct platform_device *pdev)
/* Store DP audio handle inside DP display */
g_dp_display->dp_audio = dp->audio;
+ init_completion(&dp->audio_comp);
+
platform_set_drvdata(pdev, g_dp_display);
rc = component_add(&pdev->dev, &dp_display_comp_ops);
@@ -15,6 +15,8 @@ struct msm_dp {
struct drm_connector *connector;
struct drm_encoder *encoder;
bool is_connected;
+ bool audio_enabled;
+ bool power_on;
hdmi_codec_plugged_cb plugged_cb;
@@ -32,6 +34,7 @@ int dp_display_get_modes(struct msm_dp *dp_display,
int dp_display_request_irq(struct msm_dp *dp_display);
bool dp_display_check_video_test(struct msm_dp *dp_display);
int dp_display_get_test_bpp(struct msm_dp *dp_display);
+void dp_display_signal_audio_complete(struct msm_dp *dp_display);
void __init msm_dp_pll_driver_register(void);
void __exit msm_dp_pll_driver_unregister(void);
In the current implementation, there is a very small window for the audio side to safely signal the hdmi_code_shutdown() before the clocks are disabled. Add some synchronization between the DP display and DP audio module to safely disable the clocks to avoid unclocked access from audio side. In addition, audio side can open the sound card even if DP monitor is not connected. Avoid programming hardware registers in this case and bail out early. Changes in v4: - removed some leftover prints Changes in v5: - fix crash when user tries to play audio in suspended state Changes in v6: - rebased on top of latest patchset of dependency Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org> --- drivers/gpu/drm/msm/dp/dp_audio.c | 32 +++++++++++++++++++++++ drivers/gpu/drm/msm/dp/dp_display.c | 40 +++++++++++++++++++++++++---- drivers/gpu/drm/msm/dp/dp_display.h | 3 +++ 3 files changed, 70 insertions(+), 5 deletions(-)