2019-07-23 23:11:12

by Viswanath L

[permalink] [raw]
Subject: [PATCH v3] drm/tegra: sor: Enable HDA interrupts at plug-in

HDMI plugout calls runtime suspend, which clears interrupt registers
and causes audio functionality to break on subsequent plug-in; setting
interrupt registers in sor_audio_prepare() solves the issue.

Signed-off-by: Viswanath L <[email protected]>
Fixes: 8e2988a76c26 ("drm/tegra: sor: Support for audio over HDMI")
Cc: <[email protected]>
---
drivers/gpu/drm/tegra/sor.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 5be5a08..0470cfe 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -2164,6 +2164,15 @@ static void tegra_sor_audio_prepare(struct tegra_sor *sor)

value = SOR_AUDIO_HDA_PRESENSE_ELDV | SOR_AUDIO_HDA_PRESENSE_PD;
tegra_sor_writel(sor, value, SOR_AUDIO_HDA_PRESENSE);
+
+ /*
+ * Enable and unmask the HDA codec SCRATCH0 register interrupt. This
+ * is used for interoperability between the HDA codec driver and the
+ * HDMI/DP driver.
+ */
+ value = SOR_INT_CODEC_SCRATCH1 | SOR_INT_CODEC_SCRATCH0;
+ tegra_sor_writel(sor, value, SOR_INT_ENABLE);
+ tegra_sor_writel(sor, value, SOR_INT_MASK);
}

static void tegra_sor_audio_unprepare(struct tegra_sor *sor)
@@ -2913,15 +2922,6 @@ static int tegra_sor_init(struct host1x_client *client)
if (err < 0)
return err;

- /*
- * Enable and unmask the HDA codec SCRATCH0 register interrupt. This
- * is used for interoperability between the HDA codec driver and the
- * HDMI/DP driver.
- */
- value = SOR_INT_CODEC_SCRATCH1 | SOR_INT_CODEC_SCRATCH0;
- tegra_sor_writel(sor, value, SOR_INT_ENABLE);
- tegra_sor_writel(sor, value, SOR_INT_MASK);
-
return 0;
}

--
2.7.4


2019-07-24 11:13:02

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH v3] drm/tegra: sor: Enable HDA interrupts at plug-in


On 24/07/2019 10:27, Dmitry Osipenko wrote:
> 23.07.2019 15:40, Viswanath L пишет:
>> HDMI plugout calls runtime suspend, which clears interrupt registers
>> and causes audio functionality to break on subsequent plug-in; setting
>> interrupt registers in sor_audio_prepare() solves the issue.
>>
>> Signed-off-by: Viswanath L <[email protected]>
>
> Yours signed-off-by always should be the last line of the commit's
> message because the text below it belongs to a person who applies this
> patch, Thierry in this case. This is not a big deal at all and Thierry
> could make a fixup while applying the patch if will deem that as necessary.
>
> Secondly, there is no need to add "[email protected]" to the
> email's recipients because the patch will flow into stable kernel
> versions from the mainline once it will get applied. That happens based
> on the stable tag presence, hence it's enough to add the 'Cc' tag to the
> commit's message in order to get patch backported.

I believe 'git send-email' automatically does this.

Jon

--
nvpublic

2019-07-24 11:31:32

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v3] drm/tegra: sor: Enable HDA interrupts at plug-in

24.07.2019 14:11, Jon Hunter пишет:
>
> On 24/07/2019 10:27, Dmitry Osipenko wrote:
>> 23.07.2019 15:40, Viswanath L пишет:
>>> HDMI plugout calls runtime suspend, which clears interrupt registers
>>> and causes audio functionality to break on subsequent plug-in; setting
>>> interrupt registers in sor_audio_prepare() solves the issue.
>>>
>>> Signed-off-by: Viswanath L <[email protected]>
>>
>> Yours signed-off-by always should be the last line of the commit's
>> message because the text below it belongs to a person who applies this
>> patch, Thierry in this case. This is not a big deal at all and Thierry
>> could make a fixup while applying the patch if will deem that as necessary.
>>
>> Secondly, there is no need to add "[email protected]" to the
>> email's recipients because the patch will flow into stable kernel
>> versions from the mainline once it will get applied. That happens based
>> on the stable tag presence, hence it's enough to add the 'Cc' tag to the
>> commit's message in order to get patch backported.
>
> I believe 'git send-email' automatically does this.

Indeed, completely forgot that I have '--suppress-cc=all' in my git's setup.

2019-07-24 12:08:27

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v3] drm/tegra: sor: Enable HDA interrupts at plug-in

23.07.2019 15:40, Viswanath L пишет:
> HDMI plugout calls runtime suspend, which clears interrupt registers
> and causes audio functionality to break on subsequent plug-in; setting
> interrupt registers in sor_audio_prepare() solves the issue.
>
> Signed-off-by: Viswanath L <[email protected]>

Yours signed-off-by always should be the last line of the commit's
message because the text below it belongs to a person who applies this
patch, Thierry in this case. This is not a big deal at all and Thierry
could make a fixup while applying the patch if will deem that as necessary.

Secondly, there is no need to add "[email protected]" to the
email's recipients because the patch will flow into stable kernel
versions from the mainline once it will get applied. That happens based
on the stable tag presence, hence it's enough to add the 'Cc' tag to the
commit's message in order to get patch backported.

Lastly, next time please add everyone to the email's recipients whom
you're expecting to get a reply. Otherwise there is a chance that patch
will be left unnoticed.

Everything else looks good to me, thanks!

Reviewed-by: Dmitry Osipenko <[email protected]>

> Fixes: 8e2988a76c26 ("drm/tegra: sor: Support for audio over HDMI")
> Cc: <[email protected]>
> ---
> drivers/gpu/drm/tegra/sor.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index 5be5a08..0470cfe 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -2164,6 +2164,15 @@ static void tegra_sor_audio_prepare(struct tegra_sor *sor)
>
> value = SOR_AUDIO_HDA_PRESENSE_ELDV | SOR_AUDIO_HDA_PRESENSE_PD;
> tegra_sor_writel(sor, value, SOR_AUDIO_HDA_PRESENSE);
> +
> + /*
> + * Enable and unmask the HDA codec SCRATCH0 register interrupt. This
> + * is used for interoperability between the HDA codec driver and the
> + * HDMI/DP driver.
> + */
> + value = SOR_INT_CODEC_SCRATCH1 | SOR_INT_CODEC_SCRATCH0;
> + tegra_sor_writel(sor, value, SOR_INT_ENABLE);
> + tegra_sor_writel(sor, value, SOR_INT_MASK);
> }
>
> static void tegra_sor_audio_unprepare(struct tegra_sor *sor)
> @@ -2913,15 +2922,6 @@ static int tegra_sor_init(struct host1x_client *client)
> if (err < 0)
> return err;
>
> - /*
> - * Enable and unmask the HDA codec SCRATCH0 register interrupt. This
> - * is used for interoperability between the HDA codec driver and the
> - * HDMI/DP driver.
> - */
> - value = SOR_INT_CODEC_SCRATCH1 | SOR_INT_CODEC_SCRATCH0;
> - tegra_sor_writel(sor, value, SOR_INT_ENABLE);
> - tegra_sor_writel(sor, value, SOR_INT_MASK);
> -
> return 0;
> }
>
>

2019-07-24 12:43:48

by Viswanath L

[permalink] [raw]
Subject: RE: [PATCH v3] drm/tegra: sor: Enable HDA interrupts at plug-in

Thanks very much for the review, Dmitry!

-----Original Message-----
From: Dmitry Osipenko <[email protected]>
Sent: Wednesday, July 24, 2019 2:58 PM
To: Viswanath L <[email protected]>; [email protected]; Jonathan Hunter <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v3] drm/tegra: sor: Enable HDA interrupts at plug-in

23.07.2019 15:40, Viswanath L пишет:
> HDMI plugout calls runtime suspend, which clears interrupt registers
> and causes audio functionality to break on subsequent plug-in; setting
> interrupt registers in sor_audio_prepare() solves the issue.
>
> Signed-off-by: Viswanath L <[email protected]>

Yours signed-off-by always should be the last line of the commit's message because the text below it belongs to a person who applies this patch, Thierry in this case. This is not a big deal at all and Thierry could make a fixup while applying the patch if will deem that as necessary.

Secondly, there is no need to add "[email protected]" to the email's recipients because the patch will flow into stable kernel versions from the mainline once it will get applied. That happens based on the stable tag presence, hence it's enough to add the 'Cc' tag to the commit's message in order to get patch backported.

Lastly, next time please add everyone to the email's recipients whom you're expecting to get a reply. Otherwise there is a chance that patch will be left unnoticed.

Everything else looks good to me, thanks!

Reviewed-by: Dmitry Osipenko <[email protected]>

> Fixes: 8e2988a76c26 ("drm/tegra: sor: Support for audio over HDMI")
> Cc: <[email protected]>
> ---
> drivers/gpu/drm/tegra/sor.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index 5be5a08..0470cfe 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -2164,6 +2164,15 @@ static void tegra_sor_audio_prepare(struct
> tegra_sor *sor)
>
> value = SOR_AUDIO_HDA_PRESENSE_ELDV | SOR_AUDIO_HDA_PRESENSE_PD;
> tegra_sor_writel(sor, value, SOR_AUDIO_HDA_PRESENSE);
> +
> + /*
> + * Enable and unmask the HDA codec SCRATCH0 register interrupt. This
> + * is used for interoperability between the HDA codec driver and the
> + * HDMI/DP driver.
> + */
> + value = SOR_INT_CODEC_SCRATCH1 | SOR_INT_CODEC_SCRATCH0;
> + tegra_sor_writel(sor, value, SOR_INT_ENABLE);
> + tegra_sor_writel(sor, value, SOR_INT_MASK);
> }
>
> static void tegra_sor_audio_unprepare(struct tegra_sor *sor) @@
> -2913,15 +2922,6 @@ static int tegra_sor_init(struct host1x_client *client)
> if (err < 0)
> return err;
>
> - /*
> - * Enable and unmask the HDA codec SCRATCH0 register interrupt. This
> - * is used for interoperability between the HDA codec driver and the
> - * HDMI/DP driver.
> - */
> - value = SOR_INT_CODEC_SCRATCH1 | SOR_INT_CODEC_SCRATCH0;
> - tegra_sor_writel(sor, value, SOR_INT_ENABLE);
> - tegra_sor_writel(sor, value, SOR_INT_MASK);
> -
> return 0;
> }
>
>