Message ID | 20211124220057.15763-8-digetx@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Support HDMI audio on NVIDIA Tegra20 | expand |
On Thu, Nov 25, 2021 at 01:00:44AM +0300, Dmitry Osipenko wrote:
> Program FIFO trigger level properly to fix x4 accelerated playback.
Fixes like this should really go before any new stuff so they can be
sent as fixes and backported.
25.11.2021 15:02, Mark Brown пишет: > On Thu, Nov 25, 2021 at 01:00:44AM +0300, Dmitry Osipenko wrote: >> Program FIFO trigger level properly to fix x4 accelerated playback. > > Fixes like this should really go before any new stuff so they can be > sent as fixes and backported. > This driver never worked before this patchset, hence there is nothing to backport, this is explained in the cover letter. But in general you're correct.
On Thu, Nov 25, 2021 at 03:04:35PM +0300, Dmitry Osipenko wrote: > 25.11.2021 15:02, Mark Brown пишет: > > On Thu, Nov 25, 2021 at 01:00:44AM +0300, Dmitry Osipenko wrote: > >> Program FIFO trigger level properly to fix x4 accelerated playback. > > Fixes like this should really go before any new stuff so they can be > > sent as fixes and backported. > This driver never worked before this patchset, hence there is nothing to > backport, this is explained in the cover letter. But in general you're > correct. That's not going to stop the stable people backporting things, and I'd guess it might've worked at some point on some systems - I'm not seeing anything that jumps out as making the driver completely unworkable in your patches.
25.11.2021 15:28, Mark Brown пишет: > On Thu, Nov 25, 2021 at 03:04:35PM +0300, Dmitry Osipenko wrote: >> 25.11.2021 15:02, Mark Brown пишет: >>> On Thu, Nov 25, 2021 at 01:00:44AM +0300, Dmitry Osipenko wrote: >>>> Program FIFO trigger level properly to fix x4 accelerated playback. > >>> Fixes like this should really go before any new stuff so they can be >>> sent as fixes and backported. > >> This driver never worked before this patchset, hence there is nothing to >> backport, this is explained in the cover letter. But in general you're >> correct. > > That's not going to stop the stable people backporting things, and I'd > guess it might've worked at some point on some systems - I'm not seeing > anything that jumps out as making the driver completely unworkable in > your patches. > I can change commit message with the "fix" word removed, this should prevent patch from backporting. This driver never worked in mainline because S/PDIF device was never created, thus driver was never bound. Driver doesn't work properly without this patch. Nobody used this driver as-is before this patchset.
On Thu, Nov 25, 2021 at 03:53:52PM +0300, Dmitry Osipenko wrote: > 25.11.2021 15:28, Mark Brown пишет: > > On Thu, Nov 25, 2021 at 03:04:35PM +0300, Dmitry Osipenko wrote: > >> This driver never worked before this patchset, hence there is nothing to > >> backport, this is explained in the cover letter. But in general you're > >> correct. > > That's not going to stop the stable people backporting things, and I'd > > guess it might've worked at some point on some systems - I'm not seeing > > anything that jumps out as making the driver completely unworkable in > > your patches. > I can change commit message with the "fix" word removed, this should > prevent patch from backporting. I wouldn't count on it TBH. In any case, definitely no need to resend for this alone. > This driver never worked in mainline because S/PDIF device was never > created, thus driver was never bound. Driver doesn't work properly > without this patch. Nobody used this driver as-is before this patchset. Someone might've been using it with an out of tree board file, I guess on an older stable at this point.
25.11.2021 16:18, Mark Brown пишет: > On Thu, Nov 25, 2021 at 03:53:52PM +0300, Dmitry Osipenko wrote: >> 25.11.2021 15:28, Mark Brown пишет: >>> On Thu, Nov 25, 2021 at 03:04:35PM +0300, Dmitry Osipenko wrote: > >>>> This driver never worked before this patchset, hence there is nothing to >>>> backport, this is explained in the cover letter. But in general you're >>>> correct. > >>> That's not going to stop the stable people backporting things, and I'd >>> guess it might've worked at some point on some systems - I'm not seeing >>> anything that jumps out as making the driver completely unworkable in >>> your patches. > >> I can change commit message with the "fix" word removed, this should >> prevent patch from backporting. > > I wouldn't count on it TBH. In any case, definitely no need to resend > for this alone. Alright, I'll keep this in mind for a potential v2. I guess Rob may ask to remove the assigned-clocks from S/PDIF DT binding because I just found that there is no needed to specify that property explicitly anymore. >> This driver never worked in mainline because S/PDIF device was never >> created, thus driver was never bound. Driver doesn't work properly >> without this patch. Nobody used this driver as-is before this patchset. > > Someone might've been using it with an out of tree board file, I guess > on an older stable at this point. > I'm very doubtful. Still, this patch could be easily backported because all code refactoring changes that potentially may cause merge conflicts are made after this patch. Ideally, if we really needed to backport this patch, then it should've been one of the first patches as you suggested.
diff --git a/sound/soc/tegra/tegra20_spdif.c b/sound/soc/tegra/tegra20_spdif.c index bd81df5378d1..6650875d2555 100644 --- a/sound/soc/tegra/tegra20_spdif.c +++ b/sound/soc/tegra/tegra20_spdif.c @@ -70,6 +70,14 @@ static int tegra20_spdif_hw_params(struct snd_pcm_substream *substream, regmap_update_bits(spdif->regmap, TEGRA20_SPDIF_CTRL, mask, val); + /* + * FIFO trigger level must be bigger than DMA burst or equal to it, + * otherwise data is discarded on overflow. + */ + regmap_update_bits(spdif->regmap, TEGRA20_SPDIF_DATA_FIFO_CSR, + TEGRA20_SPDIF_DATA_FIFO_CSR_TX_ATN_LVL_MASK, + TEGRA20_SPDIF_DATA_FIFO_CSR_TX_ATN_LVL_TU4_WORD_FULL); + switch (params_rate(params)) { case 32000: spdifclock = 4096000;
Program FIFO trigger level properly to fix x4 accelerated playback. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- sound/soc/tegra/tegra20_spdif.c | 8 ++++++++ 1 file changed, 8 insertions(+)