Re: Fw: [Bug 1155202] [Intel DZ77SL-50K, Intel PantherPoint HDMI, Digital Out, HDMI] No sound at all

Re: Fw: [Bug 1155202] [Intel DZ77SL-50K, Intel PantherPoint HDMI, Digital Out, HDMI] No sound at all

[niraj kulkarni]

Removing these 3 usleep ranges is working now. Although I have a doubt for
applying this patch on all chips. There have been a number of boards
released after this particular board, but nobody else is facing this
problem. Since spec does not specify a max on deasserting RST, some boards
might actually take more than 100uS to reset. Will this patch work for
those boards?

@@ -1149,7 +1149,7 @@
        timeout = jiffies + msecs_to_jiffies(100);
        while ((azx_readb(chip, GCTL) & ICH6_GCTL_RESET) &&
                        time_before(jiffies, timeout))
-               usleep_range(500, 1000);
+               cpu_relax();
 }

 /* exit link reset */
@@ -1162,7 +1162,7 @@
        timeout = jiffies + msecs_to_jiffies(100);
        while (!azx_readb(chip, GCTL) &&
                        time_before(jiffies, timeout))
-               usleep_range(500, 1000);
+               cpu_relax();
 }

 /* reset codec link */
@@ -1180,7 +1180,7 @@
        /* delay for >= 100us for codec PLL to settle per spec
         * Rev 0.9 section 5.5.1
         */
-       usleep_range(500, 1000);
+       //usleep_range(500, 1000);

        /* Bring controller out of reset */
        azx_exit_link_reset(chip);

Re: Fw: [Bug 1155202] [Intel DZ77SL-50K, Intel PantherPoint HDMI, Digital Out, HDMI] No sound at all

[Takashi Iwai]

At Mon, 10 Feb 2014 11:35:41 +0530,
niraj kulkarni wrote:
> 
> Removing these 3 usleep ranges is working now. Although I have a doubt for
> applying this patch on all chips. There have been a number of boards
> released after this particular board, but nobody else is facing this
> problem. Since spec does not specify a max on deasserting RST, some boards
> might actually take more than 100uS to reset. Will this patch work for
> those boards?
> 
> @@ -1149,7 +1149,7 @@
>         timeout = jiffies + msecs_to_jiffies(100);
>         while ((azx_readb(chip, GCTL) & ICH6_GCTL_RESET) &&
>                         time_before(jiffies, timeout))
> -               usleep_range(500, 1000);
> +               cpu_relax();
>  }
> 
>  /* exit link reset */
> @@ -1162,7 +1162,7 @@
>         timeout = jiffies + msecs_to_jiffies(100);
>         while (!azx_readb(chip, GCTL) &&
>                         time_before(jiffies, timeout))
> -               usleep_range(500, 1000);
> +               cpu_relax();
>  }
> 
>  /* reset codec link */
> @@ -1180,7 +1180,7 @@
>         /* delay for >= 100us for codec PLL to settle per spec
>          * Rev 0.9 section 5.5.1
>          */
> -       usleep_range(500, 1000);
> +       //usleep_range(500, 1000);

Why didn't you change as I suggested?  In my patch, I placed udelay(2)
since the spec tells so.  Didn't it interfere?

Replacing usleep_range() with cpu_relax() should make still working on
other boards because it doesn't change the length of the timeout
itself.  The timeout is set to very long (100ms).  The only drawback
would be a higher CPU usage at probing, but the impact should be
negligible.


Takashi

Re: Fw: [Bug 1155202] [Intel DZ77SL-50K, Intel PantherPoint HDMI, Digital Out, HDMI] No sound at all

[niraj kulkarni]

Sorry, I did not see that part earlier. Putting udelay(2) is also working
fine.

Niraj


On Mon, Feb 10, 2014 at 2:31 PM, Takashi Iwai <tiwai@suse.de> wrote:

> At Mon, 10 Feb 2014 11:35:41 +0530,
> niraj kulkarni wrote:
> >
> > Removing these 3 usleep ranges is working now. Although I have a doubt
> for
> > applying this patch on all chips. There have been a number of boards
> > released after this particular board, but nobody else is facing this
> > problem. Since spec does not specify a max on deasserting RST, some
> boards
> > might actually take more than 100uS to reset. Will this patch work for
> > those boards?
> >
> > @@ -1149,7 +1149,7 @@
> >         timeout = jiffies + msecs_to_jiffies(100);
> >         while ((azx_readb(chip, GCTL) & ICH6_GCTL_RESET) &&
> >                         time_before(jiffies, timeout))
> > -               usleep_range(500, 1000);
> > +               cpu_relax();
> >  }
> >
> >  /* exit link reset */
> > @@ -1162,7 +1162,7 @@
> >         timeout = jiffies + msecs_to_jiffies(100);
> >         while (!azx_readb(chip, GCTL) &&
> >                         time_before(jiffies, timeout))
> > -               usleep_range(500, 1000);
> > +               cpu_relax();
> >  }
> >
> >  /* reset codec link */
> > @@ -1180,7 +1180,7 @@
> >         /* delay for >= 100us for codec PLL to settle per spec
> >          * Rev 0.9 section 5.5.1
> >          */
> > -       usleep_range(500, 1000);
> > +       //usleep_range(500, 1000);
>
> Why didn't you change as I suggested?  In my patch, I placed udelay(2)
> since the spec tells so.  Didn't it interfere?
>
> Replacing usleep_range() with cpu_relax() should make still working on
> other boards because it doesn't change the length of the timeout
> itself.  The timeout is set to very long (100ms).  The only drawback
> would be a higher CPU usage at probing, but the impact should be
> negligible.
>
>
> Takashi
>

Re: Fw: [Bug 1155202] [Intel DZ77SL-50K, Intel PantherPoint HDMI, Digital Out, HDMI] No sound at all

[Takashi Iwai]

At Mon, 10 Feb 2014 17:10:16 +0530,
niraj kulkarni wrote:
> 
> Sorry, I did not see that part earlier. Putting udelay(2) is also working
> fine.

OK, thanks for testing.  I'm going to test the patch with other
machines and then merge to the upstream.


Takashi

> 
> Niraj
> 
> 
> On Mon, Feb 10, 2014 at 2:31 PM, Takashi Iwai <tiwai@suse.de> wrote:
> 
> > At Mon, 10 Feb 2014 11:35:41 +0530,
> > niraj kulkarni wrote:
> > >
> > > Removing these 3 usleep ranges is working now. Although I have a doubt
> > for
> > > applying this patch on all chips. There have been a number of boards
> > > released after this particular board, but nobody else is facing this
> > > problem. Since spec does not specify a max on deasserting RST, some
> > boards
> > > might actually take more than 100uS to reset. Will this patch work for
> > > those boards?
> > >
> > > @@ -1149,7 +1149,7 @@
> > >         timeout = jiffies + msecs_to_jiffies(100);
> > >         while ((azx_readb(chip, GCTL) & ICH6_GCTL_RESET) &&
> > >                         time_before(jiffies, timeout))
> > > -               usleep_range(500, 1000);
> > > +               cpu_relax();
> > >  }
> > >
> > >  /* exit link reset */
> > > @@ -1162,7 +1162,7 @@
> > >         timeout = jiffies + msecs_to_jiffies(100);
> > >         while (!azx_readb(chip, GCTL) &&
> > >                         time_before(jiffies, timeout))
> > > -               usleep_range(500, 1000);
> > > +               cpu_relax();
> > >  }
> > >
> > >  /* reset codec link */
> > > @@ -1180,7 +1180,7 @@
> > >         /* delay for >= 100us for codec PLL to settle per spec
> > >          * Rev 0.9 section 5.5.1
> > >          */
> > > -       usleep_range(500, 1000);
> > > +       //usleep_range(500, 1000);
> >
> > Why didn't you change as I suggested?  In my patch, I placed udelay(2)
> > since the spec tells so.  Didn't it interfere?
> >
> > Replacing usleep_range() with cpu_relax() should make still working on
> > other boards because it doesn't change the length of the timeout
> > itself.  The timeout is set to very long (100ms).  The only drawback
> > would be a higher CPU usage at probing, but the impact should be
> > negligible.
> >
> >
> > Takashi
> >
> [2  <text/html; ISO-8859-1 (quoted-printable)>]
> 

Re: Fw: [Bug 1155202] [Intel DZ77SL-50K, Intel PantherPoint HDMI, Digital Out, HDMI] No sound at all

[Takashi Iwai]

At 22 Jan 2014 14:20:23 -0000,
Niraj Kulkarni wrote:
> 
> Let me try to explain my findings:
> 
> 
> 
> > How does this magic sequence come?
> 
> By dumping Win7 driver's init sequence and reproducing it through ALSA driver.
> 
> 
> 
> >why does it need to set UNSOL flag at first?
> 
> No such need, I've put it after regular init so that only affected chipsets are reset again. Putting this anywhere around init code works (tried it).

OK.

> >Is the while loop guaranteed to quit for a reasonable time?
> 
> Being household user, I can only confirm on my chipset. Only a thorough testing may ensure sanity.

Then you must put a timeout not to lock up the machine.

> >Can't it be simply calling azx_reset() twice?
> 
> It is interesting hardware bug which kicks in only during first init after power-on. So if Win7 driver in qemu inits the board, and I simply plugin "unchanged" hda_intel afterwards, it still detects codecs. But multiple normal resets do not have any effect. Being no hardware expert, I can only speculate that tight spin loop after reset has the crux. A normal delay call there does not detect codec.

This is doubtful.  Here we see the behavior of the PCI controller, and
there shouldn't be difference by CPU instructions.

The difference in the code between azx_reset() and your code are:

- Use of azx_writeb() and azx_writel()
- Limited loop time for GCTL_RESET bit check

For the first one, you can try the code change like below:
-- 8< --
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index fa2879a21a50..03525cff565a 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1143,10 +1143,10 @@ static void azx_enter_link_reset(struct azx *chip)
 	unsigned long timeout;
 
 	/* reset controller */
-	azx_writel(chip, GCTL, azx_readl(chip, GCTL) & ~ICH6_GCTL_RESET);
+	azx_writel(chip, GCTL, 0);
 
 	timeout = jiffies + msecs_to_jiffies(100);
-	while ((azx_readb(chip, GCTL) & ICH6_GCTL_RESET) &&
+	while (azx_readl(chip, GCTL) &&
 			time_before(jiffies, timeout))
 		usleep_range(500, 1000);
 }
@@ -1156,10 +1156,10 @@ static void azx_exit_link_reset(struct azx *chip)
 {
 	unsigned long timeout;
 
-	azx_writeb(chip, GCTL, azx_readb(chip, GCTL) | ICH6_GCTL_RESET);
+	azx_writel(chip, GCTL, ICH6_GCTL_RESET);
 
 	timeout = jiffies + msecs_to_jiffies(100);
-	while (!azx_readb(chip, GCTL) &&
+	while (!azx_readl(chip, GCTL) &&
 			time_before(jiffies, timeout))
 		usleep_range(500, 1000);
 }
-- 8< --

For the second point, you can increase the timeout value e.g.
	timeout = jiffies + msecs_to_jiffies(500);

> &gt;And, yet another question comes up: doesn't S4 need the similar workaround? If S4 works, what's the difference?
> 
> Haven't tried with sleep-wake or hibernate-restore sequence yet.

Please check it.  I guess S4 is broken with your patch.


Takashi