<html>
    <head>
      <base href="https://bugs.freedesktop.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW --- - audio broken in 24Hz/24p since 3.11 (regression)"
   href="https://bugs.freedesktop.org/show_bug.cgi?id=69675#c19">Comment # 19</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW --- - audio broken in 24Hz/24p since 3.11 (regression)"
   href="https://bugs.freedesktop.org/show_bug.cgi?id=69675">bug 69675</a>
              from <span class="vcard"><a class="email" href="mailto:anssi@mageia.org" title="Anssi Hannula <anssi@mageia.org>"> <span class="fn">Anssi Hannula</span></a>
</span></b>
        <pre>(In reply to <a href="show_bug.cgi?id=69675#c16">comment #16</a>)
<span class="quote">> +      u64 n, d;
> +
> +  if (*CTS == 0) {
> +          n = (u64)clock * N;
> +          d = 128ULL * (u64)freq;
> +          *CTS = div64_u64(n, d);
> +          *CTS *= 1000ULL;
> +  }</span >

Hmm, I believe we should multiply before division, otherwise the result will
not be right... like this:
+    if (*CTS == 0) {
+        n = (u64)clock * N * 1000ULL;
+        d = 128ULL * (u64)freq;
+        *CTS = div64_u64(n, d);
+    }

Also, maximum value for d is actually 128*48000, so no need for u64 for it, and
then one can use div_64u(u64,u32) for division. Similarly, CTS/N values are
just 20-bit so no need to u64 them (it might even be confusing), u64 for just
the "n" variable should be enough.

<span class="quote">> Subject: [PATCH 3/3] drm/radeon: use hw generated CTS/N values for audio</span >

Just checking: What N value does the Hw use in that mode? The ones written in
by the driver, some hardcoded N or does it select one on its own? Though it
doesn't really matter much (since any reasonable N should work as long as CTS
is correct), except that if it uses the driver-set value we better not remove
the write to the N register :)

(In reply to <a href="show_bug.cgi?id=69675#c15">comment #15</a>)
<span class="quote">> Somewhat related, the calculation in r600_hdmi_calc_cts() is not very good
> as it loses a tonne of precision (roughly ten bits' worth). Given the range
> of the inputs, you might need to do the calculations in a 64-bit space.</span >

I'm a bit interested if the calculated values "seem" to work if you only fix
the precisions there (Alex' patch1 plus my first tweak above), without adding
the correct values in the table?

If the actual freq is closer to 74176 instead of 74175.8, the calculated values
should actually be better than the table value (as you noticed, the table
values expect exact /1.001 rate), and it could be the calculation precision
issue (*1000 in the wrong place) then caused the audio issues as the CTS was
more wildly wrong...</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>