Re: [PATCH] ALSA: hda: Enable sync-write operation as default for all controllers

[Takashi Iwai <tiwai-l3A5Bk7waGM@public.gmane.org>]

On Tue, 14 Jul 2020 11:08:18 +0200,
Jon Hunter wrote:
> 
> 
> On 14/07/2020 09:30, Takashi Iwai wrote:
> > On Tue, 14 Jul 2020 10:08:02 +0200,
> > Jon Hunter wrote:
> >>
> >> Hi Takashi,
> >>
> >> On 18/06/2020 15:40, Takashi Iwai wrote:
> >>> In the end we already enabled the sync-write mode for most of HD-audio
> >>> controllers including Intel, and it's no big merit to keep the async
> >>> write mode for the rest.  Let's make it as default and drop the
> >>> superfluous AZX_DCAPS_SYNC_WRITE bit flag.
> >>>
> >>> Also, avoid to set the allow_bus_reset flag, which is a quite unstable
> >>> and hackish behavior that was needed only for some early platforms
> >>> (decades ago).  The straight fallback to the single cmd mode is more
> >>> robust.
> >>>
> >>> Signed-off-by: Takashi Iwai <tiwai-l3A5Bk7waGM@public.gmane.org>
> >>
> >>
> >> I have noticed a regression in HDA playback on our Tegra186 Jetson TX2
> >> platform. Bisect is pointing to this patch and reverting this does
> >> appear to fix it. Interestingly, I am not seeing any problems on other
> >> Tegra platforms, however, Tegra186 does have the IOMMU enabled for HDA
> >> which is one different between the other platforms.
> >>
> >> We can take a closer look at this for Tegra, but I am wondering if we
> >> revert this for Tegra for now.
> > 
> > It's a deja vu, we (or someone else in Nvidia?) discussed it in the
> > past?
> > 
> > The patch below should cure the problem temporarily, as it sets the
> > polling mode as default for Tegra.  But it'd be appreciated if you can
> > find the root cause.
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > --- a/sound/pci/hda/hda_tegra.c
> > +++ b/sound/pci/hda/hda_tegra.c
> > @@ -394,6 +394,7 @@ static int hda_tegra_create(struct snd_card *card,
> >  	if (err < 0)
> >  		return err;
> >  
> > +	chip->bus.core.polling = 1;
> >  	chip->bus.core.needs_damn_long_delay = 1;
> >  
> >  	err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
> > 
> 
> Did you mean ...
> 
> diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
> index 0cc5fad1af8a..5637f0129932 100644
> --- a/sound/pci/hda/hda_tegra.c
> +++ b/sound/pci/hda/hda_tegra.c
> @@ -443,6 +443,7 @@ static int hda_tegra_create(struct snd_card *card,
>         if (err < 0)
>                 return err;
> 
> +       chip->bus.core.sync_write = 0;
>         chip->bus.core.needs_damn_long_delay = 1;
>         chip->bus.core.aligned_mmio = 1;
> 
> The above works for me.

Yeah, sorry, that was a wrong patch :)
I'm fine for applying it with some more comments.

Care to submit a proper patch?


thanks,

Takashi