Re: [PATCH AUTOSEL 4.18 043/131] ASoC: soc-pcm: Use delay set in component pointer function
On Fri, Sep 07, 2018 at 12:46:21PM +0530, Agrawal, Akshu wrote: > But more likely I think the delay was just getting left out and there > wouldn't have been a compensation in userspace. Yes, I don't think most users are going to notice this either way - the overwhelming majority of them won't have noticed a problem and won't notice the fix going in. It's then a question of if the people who saw the issue and cared did the userspace compenstation or not, my concern is that more will have done than won't given that it's a fairly common thing to have control for given that you also often get delays added in things like surround sound systems which aren't visible to the playback device. signature.asc Description: PGP signature
Re: [PATCH AUTOSEL 4.18 043/131] ASoC: soc-pcm: Use delay set in component pointer function
On Fri, Sep 07, 2018 at 12:46:21PM +0530, Agrawal, Akshu wrote: > But more likely I think the delay was just getting left out and there > wouldn't have been a compensation in userspace. Yes, I don't think most users are going to notice this either way - the overwhelming majority of them won't have noticed a problem and won't notice the fix going in. It's then a question of if the people who saw the issue and cared did the userspace compenstation or not, my concern is that more will have done than won't given that it's a fairly common thing to have control for given that you also often get delays added in things like surround sound systems which aren't visible to the playback device. signature.asc Description: PGP signature
Re: [PATCH AUTOSEL 4.18 043/131] ASoC: soc-pcm: Use delay set in component pointer function
On Fri, Sep 07, 2018 at 12:23:14AM +, Sasha Levin wrote: > On Mon, Sep 03, 2018 at 12:16:26PM +0100, Mark Brown wrote: > >I'm worried that if anyone notices this at all they will have already > >compensated for the delays in userspace and therefore this will cause > >them to see problems as they get double compenstation for delays. > But what happens when they update to a newer Stable? They're going to > hit that issue anyways. Yes, but taking an entire new kernel release is something that'd normally warrant a lot more integration testing than just merging in a new stable release. signature.asc Description: PGP signature
Re: [PATCH AUTOSEL 4.18 043/131] ASoC: soc-pcm: Use delay set in component pointer function
On Fri, Sep 07, 2018 at 12:23:14AM +, Sasha Levin wrote: > On Mon, Sep 03, 2018 at 12:16:26PM +0100, Mark Brown wrote: > >I'm worried that if anyone notices this at all they will have already > >compensated for the delays in userspace and therefore this will cause > >them to see problems as they get double compenstation for delays. > But what happens when they update to a newer Stable? They're going to > hit that issue anyways. Yes, but taking an entire new kernel release is something that'd normally warrant a lot more integration testing than just merging in a new stable release. signature.asc Description: PGP signature
Re: [PATCH AUTOSEL 4.18 043/131] ASoC: soc-pcm: Use delay set in component pointer function
On 9/7/2018 5:53 AM, Sasha Levin wrote: > On Mon, Sep 03, 2018 at 12:16:26PM +0100, Mark Brown wrote: >> On Sun, Sep 02, 2018 at 01:03:55PM +, Sasha Levin wrote: >>> From: Akshu Agrawal >>> >>> [ Upstream commit 9fb4c2bf130b922c77c16a8368732699799c40de ] >>> >>> Take into account the base delay set in pointer callback. >>> >>> There are cases where a pointer function populates >>> runtime->delay, such as: >>> ./sound/pci/hda/hda_controller.c >>> ./sound/soc/intel/atom/sst-mfld-platform-pcm.c >> >> I'm worried that if anyone notices this at all they will have already >> compensated for the delays in userspace and therefore this will cause >> them to see problems as they get double compenstation for delays. > > But what happens when they update to a newer Stable? They're going to > hit that issue anyways. > Drivers which had exposed this delay in pointer function but have compensated for the issue in userspace are likely see the problem of double delay when the update happens. I Don't know what is the best way to communicate that issue is fixed in kernel and usersapce compensation isn't required. But more likely I think the delay was just getting left out and there wouldn't have been a compensation in userspace. Thanks, Akshu
Re: [PATCH AUTOSEL 4.18 043/131] ASoC: soc-pcm: Use delay set in component pointer function
On 9/7/2018 5:53 AM, Sasha Levin wrote: > On Mon, Sep 03, 2018 at 12:16:26PM +0100, Mark Brown wrote: >> On Sun, Sep 02, 2018 at 01:03:55PM +, Sasha Levin wrote: >>> From: Akshu Agrawal >>> >>> [ Upstream commit 9fb4c2bf130b922c77c16a8368732699799c40de ] >>> >>> Take into account the base delay set in pointer callback. >>> >>> There are cases where a pointer function populates >>> runtime->delay, such as: >>> ./sound/pci/hda/hda_controller.c >>> ./sound/soc/intel/atom/sst-mfld-platform-pcm.c >> >> I'm worried that if anyone notices this at all they will have already >> compensated for the delays in userspace and therefore this will cause >> them to see problems as they get double compenstation for delays. > > But what happens when they update to a newer Stable? They're going to > hit that issue anyways. > Drivers which had exposed this delay in pointer function but have compensated for the issue in userspace are likely see the problem of double delay when the update happens. I Don't know what is the best way to communicate that issue is fixed in kernel and usersapce compensation isn't required. But more likely I think the delay was just getting left out and there wouldn't have been a compensation in userspace. Thanks, Akshu
Re: [PATCH AUTOSEL 4.18 043/131] ASoC: soc-pcm: Use delay set in component pointer function
On Mon, Sep 03, 2018 at 12:16:26PM +0100, Mark Brown wrote: >On Sun, Sep 02, 2018 at 01:03:55PM +, Sasha Levin wrote: >> From: Akshu Agrawal >> >> [ Upstream commit 9fb4c2bf130b922c77c16a8368732699799c40de ] >> >> Take into account the base delay set in pointer callback. >> >> There are cases where a pointer function populates >> runtime->delay, such as: >> ./sound/pci/hda/hda_controller.c >> ./sound/soc/intel/atom/sst-mfld-platform-pcm.c > >I'm worried that if anyone notices this at all they will have already >compensated for the delays in userspace and therefore this will cause >them to see problems as they get double compenstation for delays. But what happens when they update to a newer Stable? They're going to hit that issue anyways.
Re: [PATCH AUTOSEL 4.18 043/131] ASoC: soc-pcm: Use delay set in component pointer function
On Mon, Sep 03, 2018 at 12:16:26PM +0100, Mark Brown wrote: >On Sun, Sep 02, 2018 at 01:03:55PM +, Sasha Levin wrote: >> From: Akshu Agrawal >> >> [ Upstream commit 9fb4c2bf130b922c77c16a8368732699799c40de ] >> >> Take into account the base delay set in pointer callback. >> >> There are cases where a pointer function populates >> runtime->delay, such as: >> ./sound/pci/hda/hda_controller.c >> ./sound/soc/intel/atom/sst-mfld-platform-pcm.c > >I'm worried that if anyone notices this at all they will have already >compensated for the delays in userspace and therefore this will cause >them to see problems as they get double compenstation for delays. But what happens when they update to a newer Stable? They're going to hit that issue anyways.
Re: [PATCH AUTOSEL 4.18 043/131] ASoC: soc-pcm: Use delay set in component pointer function
On Sun, Sep 02, 2018 at 01:03:55PM +, Sasha Levin wrote: > From: Akshu Agrawal > > [ Upstream commit 9fb4c2bf130b922c77c16a8368732699799c40de ] > > Take into account the base delay set in pointer callback. > > There are cases where a pointer function populates > runtime->delay, such as: > ./sound/pci/hda/hda_controller.c > ./sound/soc/intel/atom/sst-mfld-platform-pcm.c I'm worried that if anyone notices this at all they will have already compensated for the delays in userspace and therefore this will cause them to see problems as they get double compenstation for delays. signature.asc Description: PGP signature
Re: [PATCH AUTOSEL 4.18 043/131] ASoC: soc-pcm: Use delay set in component pointer function
On Sun, Sep 02, 2018 at 01:03:55PM +, Sasha Levin wrote: > From: Akshu Agrawal > > [ Upstream commit 9fb4c2bf130b922c77c16a8368732699799c40de ] > > Take into account the base delay set in pointer callback. > > There are cases where a pointer function populates > runtime->delay, such as: > ./sound/pci/hda/hda_controller.c > ./sound/soc/intel/atom/sst-mfld-platform-pcm.c I'm worried that if anyone notices this at all they will have already compensated for the delays in userspace and therefore this will cause them to see problems as they get double compenstation for delays. signature.asc Description: PGP signature
[PATCH AUTOSEL 4.18 043/131] ASoC: soc-pcm: Use delay set in component pointer function
From: Akshu Agrawal
[ Upstream commit 9fb4c2bf130b922c77c16a8368732699799c40de ]
Take into account the base delay set in pointer callback.
There are cases where a pointer function populates
runtime->delay, such as:
./sound/pci/hda/hda_controller.c
./sound/soc/intel/atom/sst-mfld-platform-pcm.c
This delay was getting lost and was overwritten by delays
from codec or cpu dai delay function if exposed.
Now,
Total delay = base delay + cpu_dai delay + codec_dai delay
Signed-off-by: Akshu Agrawal
Reviewed-by: Takashi Iwai
Signed-off-by: Mark Brown
Signed-off-by: Sasha Levin
---
sound/soc/soc-pcm.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 5e7ae47a9658..1cdd21f6827e 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1165,6 +1165,9 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct
snd_pcm_substream *substream)
snd_pcm_sframes_t codec_delay = 0;
int i;
+ /* clearing the previous total delay */
+ runtime->delay = 0;
+
for_each_rtdcom(rtd, rtdcom) {
component = rtdcom->component;
@@ -1176,6 +1179,8 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct
snd_pcm_substream *substream)
offset = component->driver->ops->pointer(substream);
break;
}
+ /* base delay if assigned in pointer callback */
+ delay = runtime->delay;
if (cpu_dai->driver->ops->delay)
delay += cpu_dai->driver->ops->delay(substream, cpu_dai);
--
2.17.1
[PATCH AUTOSEL 4.18 043/131] ASoC: soc-pcm: Use delay set in component pointer function
From: Akshu Agrawal
[ Upstream commit 9fb4c2bf130b922c77c16a8368732699799c40de ]
Take into account the base delay set in pointer callback.
There are cases where a pointer function populates
runtime->delay, such as:
./sound/pci/hda/hda_controller.c
./sound/soc/intel/atom/sst-mfld-platform-pcm.c
This delay was getting lost and was overwritten by delays
from codec or cpu dai delay function if exposed.
Now,
Total delay = base delay + cpu_dai delay + codec_dai delay
Signed-off-by: Akshu Agrawal
Reviewed-by: Takashi Iwai
Signed-off-by: Mark Brown
Signed-off-by: Sasha Levin
---
sound/soc/soc-pcm.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 5e7ae47a9658..1cdd21f6827e 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1165,6 +1165,9 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct
snd_pcm_substream *substream)
snd_pcm_sframes_t codec_delay = 0;
int i;
+ /* clearing the previous total delay */
+ runtime->delay = 0;
+
for_each_rtdcom(rtd, rtdcom) {
component = rtdcom->component;
@@ -1176,6 +1179,8 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct
snd_pcm_substream *substream)
offset = component->driver->ops->pointer(substream);
break;
}
+ /* base delay if assigned in pointer callback */
+ delay = runtime->delay;
if (cpu_dai->driver->ops->delay)
delay += cpu_dai->driver->ops->delay(substream, cpu_dai);
--
2.17.1
