[PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[Maxime Ripard]

When running the trigger hook, ALSA by default will take a spinlock, and
thus will run the trigger hook in atomic context.

However, our HDMI driver will send the infoframes as part of the trigger
hook, and part of that process is to wait for a bit to be cleared for up to
100ms. To be nicer to the system, that wait has some usleep_range that
interact poorly with the atomic context.

There's several ways we can fix this, but the more obvious one is to make
ALSA take a mutex instead by setting the nonatomic flag on the DAI link.
That doesn't work though, since now the cyclic callback installed by the
dmaengine helpers in ALSA will take a mutex, while that callback is run by
dmaengine's virt-chan code in a tasklet where sleeping is not allowed
either.

Given the delay we need to poll the bit for, changing the usleep_range for
a udelay and keep running it from a context where interrupts are disabled
is not really a good option either.

However, we can move the infoframe setup code in the hw_params hook, like
is usually done in other HDMI controllers, that isn't protected by a
spinlock and thus where we can sleep. Infoframes will be sent on a regular
basis anyway, and since hw_params is where the audio parameters that end up
in the infoframes are setup, this also makes a bit more sense.

Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support")
Suggested-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 74da7c00ecd0..ec3ba3ecd32a 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1077,6 +1077,7 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
 				    struct snd_soc_dai *dai)
 {
 	struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
+	struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
 	struct device *dev = &vc4_hdmi->pdev->dev;
 	u32 audio_packet_config, channel_mask;
 	u32 channel_map;
@@ -1136,6 +1137,8 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
 	HDMI_WRITE(HDMI_AUDIO_PACKET_CONFIG, audio_packet_config);
 	vc4_hdmi_set_n_cts(vc4_hdmi);
 
+	vc4_hdmi_set_audio_infoframe(encoder);
+
 	return 0;
 }
 
@@ -1143,11 +1146,9 @@ static int vc4_hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
 				  struct snd_soc_dai *dai)
 {
 	struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
-	struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		vc4_hdmi_set_audio_infoframe(encoder);
 		vc4_hdmi->audio.streaming = true;
 
 		if (vc4_hdmi->variant->phy_rng_enable)
-- 
2.26.2

[PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[Maxime Ripard]

When running the trigger hook, ALSA by default will take a spinlock, and
thus will run the trigger hook in atomic context.

However, our HDMI driver will send the infoframes as part of the trigger
hook, and part of that process is to wait for a bit to be cleared for up to
100ms. To be nicer to the system, that wait has some usleep_range that
interact poorly with the atomic context.

There's several ways we can fix this, but the more obvious one is to make
ALSA take a mutex instead by setting the nonatomic flag on the DAI link.
That doesn't work though, since now the cyclic callback installed by the
dmaengine helpers in ALSA will take a mutex, while that callback is run by
dmaengine's virt-chan code in a tasklet where sleeping is not allowed
either.

Given the delay we need to poll the bit for, changing the usleep_range for
a udelay and keep running it from a context where interrupts are disabled
is not really a good option either.

However, we can move the infoframe setup code in the hw_params hook, like
is usually done in other HDMI controllers, that isn't protected by a
spinlock and thus where we can sleep. Infoframes will be sent on a regular
basis anyway, and since hw_params is where the audio parameters that end up
in the infoframes are setup, this also makes a bit more sense.

Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support")
Suggested-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 74da7c00ecd0..ec3ba3ecd32a 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1077,6 +1077,7 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
 				    struct snd_soc_dai *dai)
 {
 	struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
+	struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
 	struct device *dev = &vc4_hdmi->pdev->dev;
 	u32 audio_packet_config, channel_mask;
 	u32 channel_map;
@@ -1136,6 +1137,8 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
 	HDMI_WRITE(HDMI_AUDIO_PACKET_CONFIG, audio_packet_config);
 	vc4_hdmi_set_n_cts(vc4_hdmi);
 
+	vc4_hdmi_set_audio_infoframe(encoder);
+
 	return 0;
 }
 
@@ -1143,11 +1146,9 @@ static int vc4_hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
 				  struct snd_soc_dai *dai)
 {
 	struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
-	struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		vc4_hdmi_set_audio_infoframe(encoder);
 		vc4_hdmi->audio.streaming = true;
 
 		if (vc4_hdmi->variant->phy_rng_enable)
-- 
2.26.2

[PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[Maxime Ripard]

When running the trigger hook, ALSA by default will take a spinlock, and
thus will run the trigger hook in atomic context.

However, our HDMI driver will send the infoframes as part of the trigger
hook, and part of that process is to wait for a bit to be cleared for up to
100ms. To be nicer to the system, that wait has some usleep_range that
interact poorly with the atomic context.

There's several ways we can fix this, but the more obvious one is to make
ALSA take a mutex instead by setting the nonatomic flag on the DAI link.
That doesn't work though, since now the cyclic callback installed by the
dmaengine helpers in ALSA will take a mutex, while that callback is run by
dmaengine's virt-chan code in a tasklet where sleeping is not allowed
either.

Given the delay we need to poll the bit for, changing the usleep_range for
a udelay and keep running it from a context where interrupts are disabled
is not really a good option either.

However, we can move the infoframe setup code in the hw_params hook, like
is usually done in other HDMI controllers, that isn't protected by a
spinlock and thus where we can sleep. Infoframes will be sent on a regular
basis anyway, and since hw_params is where the audio parameters that end up
in the infoframes are setup, this also makes a bit more sense.

Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support")
Suggested-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 74da7c00ecd0..ec3ba3ecd32a 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1077,6 +1077,7 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
 				    struct snd_soc_dai *dai)
 {
 	struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
+	struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
 	struct device *dev = &vc4_hdmi->pdev->dev;
 	u32 audio_packet_config, channel_mask;
 	u32 channel_map;
@@ -1136,6 +1137,8 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
 	HDMI_WRITE(HDMI_AUDIO_PACKET_CONFIG, audio_packet_config);
 	vc4_hdmi_set_n_cts(vc4_hdmi);
 
+	vc4_hdmi_set_audio_infoframe(encoder);
+
 	return 0;
 }
 
@@ -1143,11 +1146,9 @@ static int vc4_hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
 				  struct snd_soc_dai *dai)
 {
 	struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
-	struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		vc4_hdmi_set_audio_infoframe(encoder);
 		vc4_hdmi->audio.streaming = true;
 
 		if (vc4_hdmi->variant->phy_rng_enable)
-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[Takashi Iwai]

On Tue, 27 Oct 2020 11:15:58 +0100,
Maxime Ripard wrote:
> 
> When running the trigger hook, ALSA by default will take a spinlock, and
> thus will run the trigger hook in atomic context.
> 
> However, our HDMI driver will send the infoframes as part of the trigger
> hook, and part of that process is to wait for a bit to be cleared for up to
> 100ms. To be nicer to the system, that wait has some usleep_range that
> interact poorly with the atomic context.
> 
> There's several ways we can fix this, but the more obvious one is to make
> ALSA take a mutex instead by setting the nonatomic flag on the DAI link.
> That doesn't work though, since now the cyclic callback installed by the
> dmaengine helpers in ALSA will take a mutex, while that callback is run by
> dmaengine's virt-chan code in a tasklet where sleeping is not allowed
> either.
> 
> Given the delay we need to poll the bit for, changing the usleep_range for
> a udelay and keep running it from a context where interrupts are disabled
> is not really a good option either.
> 
> However, we can move the infoframe setup code in the hw_params hook, like
> is usually done in other HDMI controllers, that isn't protected by a
> spinlock and thus where we can sleep. Infoframes will be sent on a regular
> basis anyway, and since hw_params is where the audio parameters that end up
> in the infoframes are setup, this also makes a bit more sense.
> 
> Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support")
> Suggested-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 74da7c00ecd0..ec3ba3ecd32a 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -1077,6 +1077,7 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
>  				    struct snd_soc_dai *dai)
>  {
>  	struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
> +	struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
>  	struct device *dev = &vc4_hdmi->pdev->dev;
>  	u32 audio_packet_config, channel_mask;
>  	u32 channel_map;
> @@ -1136,6 +1137,8 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
>  	HDMI_WRITE(HDMI_AUDIO_PACKET_CONFIG, audio_packet_config);
>  	vc4_hdmi_set_n_cts(vc4_hdmi);
>  
> +	vc4_hdmi_set_audio_infoframe(encoder);
> +
>  	return 0;
>  }
>  
> @@ -1143,11 +1146,9 @@ static int vc4_hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
>  				  struct snd_soc_dai *dai)
>  {
>  	struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
> -	struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
>  
>  	switch (cmd) {
>  	case SNDRV_PCM_TRIGGER_START:
> -		vc4_hdmi_set_audio_infoframe(encoder);
>  		vc4_hdmi->audio.streaming = true;
>  
>  		if (vc4_hdmi->variant->phy_rng_enable)
> -- 
> 2.26.2
> 

Re: [PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[Takashi Iwai]

On Tue, 27 Oct 2020 11:15:58 +0100,
Maxime Ripard wrote:
> 
> When running the trigger hook, ALSA by default will take a spinlock, and
> thus will run the trigger hook in atomic context.
> 
> However, our HDMI driver will send the infoframes as part of the trigger
> hook, and part of that process is to wait for a bit to be cleared for up to
> 100ms. To be nicer to the system, that wait has some usleep_range that
> interact poorly with the atomic context.
> 
> There's several ways we can fix this, but the more obvious one is to make
> ALSA take a mutex instead by setting the nonatomic flag on the DAI link.
> That doesn't work though, since now the cyclic callback installed by the
> dmaengine helpers in ALSA will take a mutex, while that callback is run by
> dmaengine's virt-chan code in a tasklet where sleeping is not allowed
> either.
> 
> Given the delay we need to poll the bit for, changing the usleep_range for
> a udelay and keep running it from a context where interrupts are disabled
> is not really a good option either.
> 
> However, we can move the infoframe setup code in the hw_params hook, like
> is usually done in other HDMI controllers, that isn't protected by a
> spinlock and thus where we can sleep. Infoframes will be sent on a regular
> basis anyway, and since hw_params is where the audio parameters that end up
> in the infoframes are setup, this also makes a bit more sense.
> 
> Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support")
> Suggested-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 74da7c00ecd0..ec3ba3ecd32a 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -1077,6 +1077,7 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
>  				    struct snd_soc_dai *dai)
>  {
>  	struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
> +	struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
>  	struct device *dev = &vc4_hdmi->pdev->dev;
>  	u32 audio_packet_config, channel_mask;
>  	u32 channel_map;
> @@ -1136,6 +1137,8 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
>  	HDMI_WRITE(HDMI_AUDIO_PACKET_CONFIG, audio_packet_config);
>  	vc4_hdmi_set_n_cts(vc4_hdmi);
>  
> +	vc4_hdmi_set_audio_infoframe(encoder);
> +
>  	return 0;
>  }
>  
> @@ -1143,11 +1146,9 @@ static int vc4_hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
>  				  struct snd_soc_dai *dai)
>  {
>  	struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
> -	struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
>  
>  	switch (cmd) {
>  	case SNDRV_PCM_TRIGGER_START:
> -		vc4_hdmi_set_audio_infoframe(encoder);
>  		vc4_hdmi->audio.streaming = true;
>  
>  		if (vc4_hdmi->variant->phy_rng_enable)
> -- 
> 2.26.2
> 

Re: [PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[Takashi Iwai]

On Tue, 27 Oct 2020 11:15:58 +0100,
Maxime Ripard wrote:
> 
> When running the trigger hook, ALSA by default will take a spinlock, and
> thus will run the trigger hook in atomic context.
> 
> However, our HDMI driver will send the infoframes as part of the trigger
> hook, and part of that process is to wait for a bit to be cleared for up to
> 100ms. To be nicer to the system, that wait has some usleep_range that
> interact poorly with the atomic context.
> 
> There's several ways we can fix this, but the more obvious one is to make
> ALSA take a mutex instead by setting the nonatomic flag on the DAI link.
> That doesn't work though, since now the cyclic callback installed by the
> dmaengine helpers in ALSA will take a mutex, while that callback is run by
> dmaengine's virt-chan code in a tasklet where sleeping is not allowed
> either.
> 
> Given the delay we need to poll the bit for, changing the usleep_range for
> a udelay and keep running it from a context where interrupts are disabled
> is not really a good option either.
> 
> However, we can move the infoframe setup code in the hw_params hook, like
> is usually done in other HDMI controllers, that isn't protected by a
> spinlock and thus where we can sleep. Infoframes will be sent on a regular
> basis anyway, and since hw_params is where the audio parameters that end up
> in the infoframes are setup, this also makes a bit more sense.
> 
> Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support")
> Suggested-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 74da7c00ecd0..ec3ba3ecd32a 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -1077,6 +1077,7 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
>  				    struct snd_soc_dai *dai)
>  {
>  	struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
> +	struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
>  	struct device *dev = &vc4_hdmi->pdev->dev;
>  	u32 audio_packet_config, channel_mask;
>  	u32 channel_map;
> @@ -1136,6 +1137,8 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
>  	HDMI_WRITE(HDMI_AUDIO_PACKET_CONFIG, audio_packet_config);
>  	vc4_hdmi_set_n_cts(vc4_hdmi);
>  
> +	vc4_hdmi_set_audio_infoframe(encoder);
> +
>  	return 0;
>  }
>  
> @@ -1143,11 +1146,9 @@ static int vc4_hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
>  				  struct snd_soc_dai *dai)
>  {
>  	struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
> -	struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
>  
>  	switch (cmd) {
>  	case SNDRV_PCM_TRIGGER_START:
> -		vc4_hdmi_set_audio_infoframe(encoder);
>  		vc4_hdmi->audio.streaming = true;
>  
>  		if (vc4_hdmi->variant->phy_rng_enable)
> -- 
> 2.26.2
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 236 bytes --]

On Tue, Oct 27, 2020 at 11:15:58AM +0100, Maxime Ripard wrote:
> When running the trigger hook, ALSA by default will take a spinlock, and
> thus will run the trigger hook in atomic context.

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 236 bytes --]

On Tue, Oct 27, 2020 at 11:15:58AM +0100, Maxime Ripard wrote:
> When running the trigger hook, ALSA by default will take a spinlock, and
> thus will run the trigger hook in atomic context.

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 236 bytes --]

On Tue, Oct 27, 2020 at 11:15:58AM +0100, Maxime Ripard wrote:
> When running the trigger hook, ALSA by default will take a spinlock, and
> thus will run the trigger hook in atomic context.

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]

On Tue, Oct 27, 2020 at 11:15:58AM +0100, Maxime Ripard wrote:
> When running the trigger hook, ALSA by default will take a spinlock, and
> thus will run the trigger hook in atomic context.
> 
> However, our HDMI driver will send the infoframes as part of the trigger
> hook, and part of that process is to wait for a bit to be cleared for up to
> 100ms. To be nicer to the system, that wait has some usleep_range that
> interact poorly with the atomic context.
> 
> There's several ways we can fix this, but the more obvious one is to make
> ALSA take a mutex instead by setting the nonatomic flag on the DAI link.
> That doesn't work though, since now the cyclic callback installed by the
> dmaengine helpers in ALSA will take a mutex, while that callback is run by
> dmaengine's virt-chan code in a tasklet where sleeping is not allowed
> either.
> 
> Given the delay we need to poll the bit for, changing the usleep_range for
> a udelay and keep running it from a context where interrupts are disabled
> is not really a good option either.
> 
> However, we can move the infoframe setup code in the hw_params hook, like
> is usually done in other HDMI controllers, that isn't protected by a
> spinlock and thus where we can sleep. Infoframes will be sent on a regular
> basis anyway, and since hw_params is where the audio parameters that end up
> in the infoframes are setup, this also makes a bit more sense.
> 
> Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support")
> Suggested-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Applied to drm-misc-fixes

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]

On Tue, Oct 27, 2020 at 11:15:58AM +0100, Maxime Ripard wrote:
> When running the trigger hook, ALSA by default will take a spinlock, and
> thus will run the trigger hook in atomic context.
> 
> However, our HDMI driver will send the infoframes as part of the trigger
> hook, and part of that process is to wait for a bit to be cleared for up to
> 100ms. To be nicer to the system, that wait has some usleep_range that
> interact poorly with the atomic context.
> 
> There's several ways we can fix this, but the more obvious one is to make
> ALSA take a mutex instead by setting the nonatomic flag on the DAI link.
> That doesn't work though, since now the cyclic callback installed by the
> dmaengine helpers in ALSA will take a mutex, while that callback is run by
> dmaengine's virt-chan code in a tasklet where sleeping is not allowed
> either.
> 
> Given the delay we need to poll the bit for, changing the usleep_range for
> a udelay and keep running it from a context where interrupts are disabled
> is not really a good option either.
> 
> However, we can move the infoframe setup code in the hw_params hook, like
> is usually done in other HDMI controllers, that isn't protected by a
> spinlock and thus where we can sleep. Infoframes will be sent on a regular
> basis anyway, and since hw_params is where the audio parameters that end up
> in the infoframes are setup, this also makes a bit more sense.
> 
> Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support")
> Suggested-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Applied to drm-misc-fixes

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1642 bytes --]

On Tue, Oct 27, 2020 at 11:15:58AM +0100, Maxime Ripard wrote:
> When running the trigger hook, ALSA by default will take a spinlock, and
> thus will run the trigger hook in atomic context.
> 
> However, our HDMI driver will send the infoframes as part of the trigger
> hook, and part of that process is to wait for a bit to be cleared for up to
> 100ms. To be nicer to the system, that wait has some usleep_range that
> interact poorly with the atomic context.
> 
> There's several ways we can fix this, but the more obvious one is to make
> ALSA take a mutex instead by setting the nonatomic flag on the DAI link.
> That doesn't work though, since now the cyclic callback installed by the
> dmaengine helpers in ALSA will take a mutex, while that callback is run by
> dmaengine's virt-chan code in a tasklet where sleeping is not allowed
> either.
> 
> Given the delay we need to poll the bit for, changing the usleep_range for
> a udelay and keep running it from a context where interrupts are disabled
> is not really a good option either.
> 
> However, we can move the infoframe setup code in the hw_params hook, like
> is usually done in other HDMI controllers, that isn't protected by a
> spinlock and thus where we can sleep. Infoframes will be sent on a regular
> basis anyway, and since hw_params is where the audio parameters that end up
> in the infoframes are setup, this also makes a bit more sense.
> 
> Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support")
> Suggested-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Applied to drm-misc-fixes

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel