Re: [PATCH v2 2/3] drm/msm/dp: initialize audio_comp when audio starts

2021-04-15 Thread khsieh 

On 2021-04-14 14:22, Stephen Boyd wrote:

Quoting Kuogee Hsieh (2021-04-14 14:02:50)

Initialize audio_comp when audio starts and wait for audio_comp at
dp_display_disable(). This will take care of both dongle unplugged
and display off (suspend) cases.

Changes in v2:
-- add dp_display_start_audio()

Signed-off-by: Kuogee Hsieh 


Looking better. Thanks!

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c

index 0ba71c7..8a69bcd 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -177,6 +177,14 @@ static int dp_del_event(struct dp_display_private 
*dp_priv, u32 event)


return 0;
 }
+void dp_display_start_audio(struct msm_dp *dp_display)


Please unstick this from previous function by adding a newline above.


+{
+   struct dp_display_private *dp;
+
+   dp = container_of(dp_display, struct dp_display_private, 
dp_display);

+
+   reinit_completion(&dp->audio_comp);
+}

 void dp_display_signal_audio_complete(struct msm_dp *dp_display)
 {
@@ -648,10 +656,6 @@ static int dp_hpd_unplug_handle(struct 
dp_display_private *dp, u32 data)

/* start sentinel checking in case of missing uevent */
dp_add_event(dp, EV_DISCONNECT_PENDING_TIMEOUT, 0, 
DP_TIMEOUT_5_SECOND);


-   /* signal the disconnect event early to ensure proper teardown 
*/


This doesn't need to be done early anymore? Please mention why in the
commit text.


This is my mistake, it still need signal audio here, will fix it


-   reinit_completion(&dp->audio_comp);
-   dp_display_handle_plugged_change(g_dp_display, false);
-
dp_catalog_hpd_config_intr(dp->catalog, 
DP_DP_HPD_PLUG_INT_MASK |

DP_DP_IRQ_HPD_INT_MASK, true);

@@ -894,7 +898,6 @@ static int dp_display_disable(struct 
dp_display_private *dp, u32 data)

/* wait only if audio was enabled */
if (dp_display->audio_enabled) {
/* signal the disconnect event */
-   reinit_completion(&dp->audio_comp);
dp_display_handle_plugged_change(dp_display, false);
if (!wait_for_completion_timeout(&dp->audio_comp,
HZ * 5))

Re: [PATCH v2 2/3] drm/msm/dp: initialize audio_comp when audio starts

2021-04-14 Thread Stephen Boyd 
Quoting Kuogee Hsieh (2021-04-14 14:02:50)
> Initialize audio_comp when audio starts and wait for audio_comp at
> dp_display_disable(). This will take care of both dongle unplugged
> and display off (suspend) cases.
> 
> Changes in v2:
> -- add dp_display_start_audio()
> 
> Signed-off-by: Kuogee Hsieh 

Looking better. Thanks!

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 0ba71c7..8a69bcd 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -177,6 +177,14 @@ static int dp_del_event(struct dp_display_private 
> *dp_priv, u32 event)
>  
> return 0;
>  }
> +void dp_display_start_audio(struct msm_dp *dp_display)

Please unstick this from previous function by adding a newline above.

> +{
> +   struct dp_display_private *dp;
> +
> +   dp = container_of(dp_display, struct dp_display_private, dp_display);
> +
> +   reinit_completion(&dp->audio_comp);
> +}
>  
>  void dp_display_signal_audio_complete(struct msm_dp *dp_display)
>  {
> @@ -648,10 +656,6 @@ static int dp_hpd_unplug_handle(struct 
> dp_display_private *dp, u32 data)
> /* start sentinel checking in case of missing uevent */
> dp_add_event(dp, EV_DISCONNECT_PENDING_TIMEOUT, 0, 
> DP_TIMEOUT_5_SECOND);
>  
> -   /* signal the disconnect event early to ensure proper teardown */

This doesn't need to be done early anymore? Please mention why in the
commit text.

> -   reinit_completion(&dp->audio_comp);
> -   dp_display_handle_plugged_change(g_dp_display, false);
> -
> dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK |
> DP_DP_IRQ_HPD_INT_MASK, true);
>  
> @@ -894,7 +898,6 @@ static int dp_display_disable(struct dp_display_private 
> *dp, u32 data)
> /* wait only if audio was enabled */
> if (dp_display->audio_enabled) {
> /* signal the disconnect event */
> -   reinit_completion(&dp->audio_comp);
> dp_display_handle_plugged_change(dp_display, false);
> if (!wait_for_completion_timeout(&dp->audio_comp,
> HZ * 5))

[PATCH v2 2/3] drm/msm/dp: initialize audio_comp when audio starts

2021-04-14 Thread Kuogee Hsieh 
Initialize audio_comp when audio starts and wait for audio_comp at
dp_display_disable(). This will take care of both dongle unplugged
and display off (suspend) cases.

Changes in v2:
-- add dp_display_start_audio()

Signed-off-by: Kuogee Hsieh 
---
 drivers/gpu/drm/msm/dp/dp_audio.c   |  1 +
 drivers/gpu/drm/msm/dp/dp_display.c | 13 -
 drivers/gpu/drm/msm/dp/dp_display.h |  1 +
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c 
b/drivers/gpu/drm/msm/dp/dp_audio.c
index 82a8673..52994f2 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.c
+++ b/drivers/gpu/drm/msm/dp/dp_audio.c
@@ -527,6 +527,7 @@ 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_start_audio(dp_display);
dp_display->audio_enabled = true;
 
 end:
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index 0ba71c7..8a69bcd 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -177,6 +177,14 @@ static int dp_del_event(struct dp_display_private 
*dp_priv, u32 event)
 
return 0;
 }
+void dp_display_start_audio(struct msm_dp *dp_display)
+{
+   struct dp_display_private *dp;
+
+   dp = container_of(dp_display, struct dp_display_private, dp_display);
+
+   reinit_completion(&dp->audio_comp);
+}
 
 void dp_display_signal_audio_complete(struct msm_dp *dp_display)
 {
@@ -648,10 +656,6 @@ static int dp_hpd_unplug_handle(struct dp_display_private 
*dp, u32 data)
/* start sentinel checking in case of missing uevent */
dp_add_event(dp, EV_DISCONNECT_PENDING_TIMEOUT, 0, DP_TIMEOUT_5_SECOND);
 
-   /* signal the disconnect event early to ensure proper teardown */
-   reinit_completion(&dp->audio_comp);
-   dp_display_handle_plugged_change(g_dp_display, false);
-
dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK |
DP_DP_IRQ_HPD_INT_MASK, true);
 
@@ -894,7 +898,6 @@ static int dp_display_disable(struct dp_display_private 
*dp, u32 data)
/* wait only if audio was enabled */
if (dp_display->audio_enabled) {
/* signal the disconnect event */
-   reinit_completion(&dp->audio_comp);
dp_display_handle_plugged_change(dp_display, false);
if (!wait_for_completion_timeout(&dp->audio_comp,
HZ * 5))
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h 
b/drivers/gpu/drm/msm/dp/dp_display.h
index 6092ba1..55fe0d4 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -34,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_start_audio(struct msm_dp *dp_display);
 void dp_display_signal_audio_complete(struct msm_dp *dp_display);
 
 #endif /* _DP_DISPLAY_H_ */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project