From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760345AbbIDUAW (ORCPT ); Fri, 4 Sep 2015 16:00:22 -0400 Received: from mail-yk0-f177.google.com ([209.85.160.177]:36030 "EHLO mail-yk0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760328AbbIDUAM (ORCPT ); Fri, 4 Sep 2015 16:00:12 -0400 MIME-Version: 1.0 In-Reply-To: References: <20150808160936.GN7557@n2100.arm.linux.org.uk> Date: Fri, 4 Sep 2015 13:00:11 -0700 X-Google-Sender-Auth: uZTJuBv4Vgi09TZcgjhPo8EsOkI Message-ID: Subject: Re: [PATCH 8/9] drm: bridge/dw_hdmi: replace CTS calculation for the ACR From: Doug Anderson To: Russell King Cc: "open list:ARM/Rockchip SoC..." , alsa-devel@alsa-project.org, "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Philipp Zabel , Andy Yan , Yakir Yang , Fabio Estevam , Mark Brown , Takashi Iwai , Jaroslav Kysela , Sascha Hauer , Jon Nettleton , David Airlie Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Russell, On Sat, Aug 8, 2015 at 9:10 AM, Russell King wrote: > Given the TDMS clock, audio sample rate, and the N parameter, we can > calculate the CTS value for the audio clock regenerator (ACR) using the > following calculation given in the HDMI specification: > > CTS = ftdms * N / (128 * fs) > > The specification says that the CTS value is an average value, which is > true if the source hardware measures it. Where source hardware needs it > to be programmed, it is particularly difficult to alternate it between > two values correctly to ensure that we achieve a correct "average" > fractional value at the sink. > > Also, there's the problem that our "ftdms" is not a fully accurate > value; it is rounded to a kHz value. This introduces an unnecessary > (and harmless) fractional value into the above equation for combinations > like 148.5MHz/1.001 for 44100Hz - we still calculate the correct CTS > value. > > Signed-off-by: Russell King > --- > drivers/gpu/drm/bridge/dw_hdmi.c | 92 +++++++--------------------------------- > 1 file changed, 16 insertions(+), 76 deletions(-) If you take my feedback about your "drm: bridge/dw_hdmi: adjust pixel clock values in N calculation" patch [1], all this math just works out to "cts = pixel_clk / 1000". ...but doing the math does future proof us a bit, so it seems like a good idea. Reviewed-by: Douglas Anderson Tested-by: Douglas Anderson [1] https://patchwork.kernel.org/patch/6975221/ From mboxrd@z Thu Jan 1 00:00:00 1970 From: Doug Anderson Subject: Re: [PATCH 8/9] drm: bridge/dw_hdmi: replace CTS calculation for the ACR Date: Fri, 4 Sep 2015 13:00:11 -0700 Message-ID: References: <20150808160936.GN7557@n2100.arm.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Russell King Cc: Fabio Estevam , alsa-devel@alsa-project.org, "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , Jaroslav Kysela , "open list:ARM/Rockchip SoC..." , Mark Brown , Yakir Yang , Andy Yan , "linux-arm-kernel@lists.infradead.org" List-Id: alsa-devel@alsa-project.org UnVzc2VsbCwKCk9uIFNhdCwgQXVnIDgsIDIwMTUgYXQgOToxMCBBTSwgUnVzc2VsbCBLaW5nCjxy bWsra2VybmVsQGFybS5saW51eC5vcmcudWs+IHdyb3RlOgo+IEdpdmVuIHRoZSBURE1TIGNsb2Nr LCBhdWRpbyBzYW1wbGUgcmF0ZSwgYW5kIHRoZSBOIHBhcmFtZXRlciwgd2UgY2FuCj4gY2FsY3Vs YXRlIHRoZSBDVFMgdmFsdWUgZm9yIHRoZSBhdWRpbyBjbG9jayByZWdlbmVyYXRvciAoQUNSKSB1 c2luZyB0aGUKPiBmb2xsb3dpbmcgY2FsY3VsYXRpb24gZ2l2ZW4gaW4gdGhlIEhETUkgc3BlY2lm aWNhdGlvbjoKPgo+ICAgICAgICAgQ1RTID0gZnRkbXMgKiBOIC8gKDEyOCAqIGZzKQo+Cj4gVGhl IHNwZWNpZmljYXRpb24gc2F5cyB0aGF0IHRoZSBDVFMgdmFsdWUgaXMgYW4gYXZlcmFnZSB2YWx1 ZSwgd2hpY2ggaXMKPiB0cnVlIGlmIHRoZSBzb3VyY2UgaGFyZHdhcmUgbWVhc3VyZXMgaXQuICBX aGVyZSBzb3VyY2UgaGFyZHdhcmUgbmVlZHMgaXQKPiB0byBiZSBwcm9ncmFtbWVkLCBpdCBpcyBw YXJ0aWN1bGFybHkgZGlmZmljdWx0IHRvIGFsdGVybmF0ZSBpdCBiZXR3ZWVuCj4gdHdvIHZhbHVl cyBjb3JyZWN0bHkgdG8gZW5zdXJlIHRoYXQgd2UgYWNoaWV2ZSBhIGNvcnJlY3QgImF2ZXJhZ2Ui Cj4gZnJhY3Rpb25hbCB2YWx1ZSBhdCB0aGUgc2luay4KPgo+IEFsc28sIHRoZXJlJ3MgdGhlIHBy b2JsZW0gdGhhdCBvdXIgImZ0ZG1zIiBpcyBub3QgYSBmdWxseSBhY2N1cmF0ZQo+IHZhbHVlOyBp dCBpcyByb3VuZGVkIHRvIGEga0h6IHZhbHVlLiAgVGhpcyBpbnRyb2R1Y2VzIGFuIHVubmVjZXNz YXJ5Cj4gKGFuZCBoYXJtbGVzcykgZnJhY3Rpb25hbCB2YWx1ZSBpbnRvIHRoZSBhYm92ZSBlcXVh dGlvbiBmb3IgY29tYmluYXRpb25zCj4gbGlrZSAxNDguNU1Iei8xLjAwMSBmb3IgNDQxMDBIeiAt IHdlIHN0aWxsIGNhbGN1bGF0ZSB0aGUgY29ycmVjdCBDVFMKPiB2YWx1ZS4KPgo+IFNpZ25lZC1v ZmYtYnk6IFJ1c3NlbGwgS2luZyA8cm1rK2tlcm5lbEBhcm0ubGludXgub3JnLnVrPgo+IC0tLQo+ ICBkcml2ZXJzL2dwdS9kcm0vYnJpZGdlL2R3X2hkbWkuYyB8IDkyICsrKysrKystLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KPiAgMSBmaWxlIGNoYW5nZWQsIDE2IGluc2VydGlvbnMo KyksIDc2IGRlbGV0aW9ucygtKQoKSWYgeW91IHRha2UgbXkgZmVlZGJhY2sgYWJvdXQgeW91ciAi ZHJtOiBicmlkZ2UvZHdfaGRtaTogYWRqdXN0IHBpeGVsCmNsb2NrIHZhbHVlcyBpbiBOIGNhbGN1 bGF0aW9uIiBwYXRjaCBbMV0sIGFsbCB0aGlzIG1hdGgganVzdCB3b3JrcyBvdXQKdG8gImN0cyA9 IHBpeGVsX2NsayAvIDEwMDAiLiAgLi4uYnV0IGRvaW5nIHRoZSBtYXRoIGRvZXMgZnV0dXJlIHBy b29mCnVzIGEgYml0LCBzbyBpdCBzZWVtcyBsaWtlIGEgZ29vZCBpZGVhLgoKUmV2aWV3ZWQtYnk6 IERvdWdsYXMgQW5kZXJzb24gPGRpYW5kZXJzQGNocm9taXVtLm9yZz4KVGVzdGVkLWJ5OiBEb3Vn bGFzIEFuZGVyc29uIDxkaWFuZGVyc0BjaHJvbWl1bS5vcmc+CgoKWzFdIGh0dHBzOi8vcGF0Y2h3 b3JrLmtlcm5lbC5vcmcvcGF0Y2gvNjk3NTIyMS8KX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlz dHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9s aXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: dianders@chromium.org (Doug Anderson) Date: Fri, 4 Sep 2015 13:00:11 -0700 Subject: [PATCH 8/9] drm: bridge/dw_hdmi: replace CTS calculation for the ACR In-Reply-To: References: <20150808160936.GN7557@n2100.arm.linux.org.uk> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Russell, On Sat, Aug 8, 2015 at 9:10 AM, Russell King wrote: > Given the TDMS clock, audio sample rate, and the N parameter, we can > calculate the CTS value for the audio clock regenerator (ACR) using the > following calculation given in the HDMI specification: > > CTS = ftdms * N / (128 * fs) > > The specification says that the CTS value is an average value, which is > true if the source hardware measures it. Where source hardware needs it > to be programmed, it is particularly difficult to alternate it between > two values correctly to ensure that we achieve a correct "average" > fractional value at the sink. > > Also, there's the problem that our "ftdms" is not a fully accurate > value; it is rounded to a kHz value. This introduces an unnecessary > (and harmless) fractional value into the above equation for combinations > like 148.5MHz/1.001 for 44100Hz - we still calculate the correct CTS > value. > > Signed-off-by: Russell King > --- > drivers/gpu/drm/bridge/dw_hdmi.c | 92 +++++++--------------------------------- > 1 file changed, 16 insertions(+), 76 deletions(-) If you take my feedback about your "drm: bridge/dw_hdmi: adjust pixel clock values in N calculation" patch [1], all this math just works out to "cts = pixel_clk / 1000". ...but doing the math does future proof us a bit, so it seems like a good idea. Reviewed-by: Douglas Anderson Tested-by: Douglas Anderson [1] https://patchwork.kernel.org/patch/6975221/