From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753970AbbAaLH1 (ORCPT ); Sat, 31 Jan 2015 06:07:27 -0500 Received: from pandora.arm.linux.org.uk ([78.32.30.218]:38541 "EHLO pandora.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750967AbbAaLHW (ORCPT ); Sat, 31 Jan 2015 06:07:22 -0500 Date: Sat, 31 Jan 2015 11:07:04 +0000 From: Russell King - ARM Linux To: Yakir Yang Cc: David Airlie , Philipp Zabel , Fabio Estevam , Shawn Guo , Rob Clark , Mark Yao , Daniel Vetter , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, djkurtz@chromium.org, dbehr@chromoum.org, mmind00@googlemail.com, dianders@chromium.org, marcheu@chromium.org, rockchip-discuss@chromium.org Subject: Re: [PATCH v2 01/12] drm: bridge/dw_hdmi: adjust n/cts setting order Message-ID: <20150131110704.GU26493@n2100.arm.linux.org.uk> References: <1422617031-25098-1-git-send-email-ykk@rock-chips.com> <1422617130-25187-1-git-send-email-ykk@rock-chips.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1422617130-25187-1-git-send-email-ykk@rock-chips.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 30, 2015 at 06:25:30AM -0500, Yakir Yang wrote: > For Designerware HDMI, the following write sequence is recommended: > 1. aud_n3 (set bit ncts_atomic_write if desired) > 2. aud_cts3 (set CTS_manual and CTS value if desired/enabled) > 3. aud_cts2 (required in CTS_manual) > 4. aud_cts1 (required in CTS_manual) > 5. aud_n3 (bit ncts_atomic_write with same value as in step 1.) > 6. aud_n2 > 7. aud_n1 > > Signed-off-by: Yakir Yang > --- > Changes in v2: > - adjust n/cts setting order > > drivers/gpu/drm/bridge/dw_hdmi.c | 37 +++++++++++++++++++++---------------- > drivers/gpu/drm/bridge/dw_hdmi.h | 6 ++++++ > 2 files changed, 27 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c > index cd6a706..423addc 100644 > --- a/drivers/gpu/drm/bridge/dw_hdmi.c > +++ b/drivers/gpu/drm/bridge/dw_hdmi.c > @@ -177,26 +177,31 @@ static void hdmi_mask_writeb(struct dw_hdmi *hdmi, u8 data, unsigned int reg, > hdmi_modb(hdmi, data << shift, mask, reg); > } > > -static void hdmi_set_clock_regenerator_n(struct dw_hdmi *hdmi, > - unsigned int value) > +static void hdmi_regenerate_n_cts(struct dw_hdmi *hdmi, unsigned int n, > + unsigned int cts) > { > - hdmi_writeb(hdmi, value & 0xff, HDMI_AUD_N1); > - hdmi_writeb(hdmi, (value >> 8) & 0xff, HDMI_AUD_N2); > - hdmi_writeb(hdmi, (value >> 16) & 0x0f, HDMI_AUD_N3); > + /* set ncts_atomic_write */ > + hdmi_modb(hdmi, HDMI_AUD_N3_NCTS_ATOMIC_WRITE_SET, > + HDMI_AUD_N3_NCTS_ATOMIC_WRITE_MASK, HDMI_AUD_N3); Bits 7:4 of N3 are marked up in the iMX6 manuals as "reserved". We need some clarification from FSL whether this matters, but at the moment my opinion on that is this should be conditionalised against the IP version. Setting bit 7 as you do above may not cause any harm on iMX6, but on the other hand, we shouldn't be setting bits which are marked as reserved. > + > + /* set cts manual */ > + hdmi_modb(hdmi, HDMI_AUD_CTS3_CTS_MANUAL, > + HDMI_AUD_CTS3_CTS_MANUAL, HDMI_AUD_CTS3); > > /* nshift factor = 0 */ > hdmi_modb(hdmi, 0, HDMI_AUD_CTS3_N_SHIFT_MASK, HDMI_AUD_CTS3); > -} > - > -static void hdmi_regenerate_cts(struct dw_hdmi *hdmi, unsigned int cts) > -{ > - /* Must be set/cleared first */ > - hdmi_modb(hdmi, 0, HDMI_AUD_CTS3_CTS_MANUAL, HDMI_AUD_CTS3); > > - hdmi_writeb(hdmi, cts & 0xff, HDMI_AUD_CTS1); > + /* set cts values */ > + hdmi_modb(hdmi, ((cts >> 16) & HDMI_AUD_CTS3_AUDCTS19_16_MASK), > + HDMI_AUD_CTS3_AUDCTS19_16_MASK, HDMI_AUD_CTS3); > hdmi_writeb(hdmi, (cts >> 8) & 0xff, HDMI_AUD_CTS2); > - hdmi_writeb(hdmi, ((cts >> 16) & HDMI_AUD_CTS3_AUDCTS19_16_MASK) | > - HDMI_AUD_CTS3_CTS_MANUAL, HDMI_AUD_CTS3); > + hdmi_writeb(hdmi, cts & 0xff, HDMI_AUD_CTS1); > + > + /* set n values */ > + hdmi_modb(hdmi, (n >> 16) & HDMI_AUD_N3_AUDN19_16_MASK, > + HDMI_AUD_N3_AUDN19_16_MASK, HDMI_AUD_N3); > + hdmi_writeb(hdmi, (n >> 8) & 0xff, HDMI_AUD_N2); > + hdmi_writeb(hdmi, n & 0xff, HDMI_AUD_N1); This is definitely moving things in the right direction. However, I would ask that the read-modify-writes to HDMI_AUD_N3 are open-coded rather than using hdmi_modb(), and consolidated. We really don't need to keep re-reading this register. I'd also like to check that this does not cause a regression on iMX6 SoCs with my audio driver; I'm aware that there are some issues to do with how these registers are written, and the published errata workaround in the iMX6 errata documentation doesn't seem to always work. > } > > static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk, > @@ -355,8 +360,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi, > __func__, hdmi->sample_rate, hdmi->ratio, > pixel_clk, clk_n, clk_cts); > > - hdmi_set_clock_regenerator_n(hdmi, clk_n); > - hdmi_regenerate_cts(hdmi, clk_cts); > + hdmi_regenerate_n_cts(hdmi, clk_n, clk_cts); > + hdmi_set_schnl(hdmi); I'd prefer the addition of hdmi_set_schnl() to be in the patch which adds that new function. However, as I mentioned in my reply to that patch, other versions of this IP do not have these registers, and it may not be safe to write to them. We need to find a way to know whether this IP has the AHB DMA support or not and act accordingly. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Russell King - ARM Linux Subject: Re: [PATCH v2 01/12] drm: bridge/dw_hdmi: adjust n/cts setting order Date: Sat, 31 Jan 2015 11:07:04 +0000 Message-ID: <20150131110704.GU26493@n2100.arm.linux.org.uk> References: <1422617031-25098-1-git-send-email-ykk@rock-chips.com> <1422617130-25187-1-git-send-email-ykk@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from pandora.arm.linux.org.uk (pandora.arm.linux.org.uk [78.32.30.218]) by gabe.freedesktop.org (Postfix) with ESMTP id 964086E1F5 for ; Sat, 31 Jan 2015 03:07:21 -0800 (PST) Content-Disposition: inline In-Reply-To: <1422617130-25187-1-git-send-email-ykk@rock-chips.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Yakir Yang Cc: Fabio Estevam , mmind00@googlemail.com, dbehr@chromoum.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, dianders@chromium.org, rockchip-discuss@chromium.org, marcheu@chromium.org, Mark Yao List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCBKYW4gMzAsIDIwMTUgYXQgMDY6MjU6MzBBTSAtMDUwMCwgWWFraXIgWWFuZyB3cm90 ZToKPiBGb3IgRGVzaWduZXJ3YXJlIEhETUksIHRoZSBmb2xsb3dpbmcgd3JpdGUgc2VxdWVuY2Ug aXMgcmVjb21tZW5kZWQ6Cj4gMS4gYXVkX24zIChzZXQgYml0IG5jdHNfYXRvbWljX3dyaXRlIGlm IGRlc2lyZWQpCj4gMi4gYXVkX2N0czMgKHNldCBDVFNfbWFudWFsIGFuZCBDVFMgdmFsdWUgaWYg ZGVzaXJlZC9lbmFibGVkKQo+IDMuIGF1ZF9jdHMyIChyZXF1aXJlZCBpbiBDVFNfbWFudWFsKQo+ IDQuIGF1ZF9jdHMxIChyZXF1aXJlZCBpbiBDVFNfbWFudWFsKQo+IDUuIGF1ZF9uMyAoYml0IG5j dHNfYXRvbWljX3dyaXRlIHdpdGggc2FtZSB2YWx1ZSBhcyBpbiBzdGVwIDEuKQo+IDYuIGF1ZF9u Mgo+IDcuIGF1ZF9uMQo+IAo+IFNpZ25lZC1vZmYtYnk6IFlha2lyIFlhbmcgPHlra0Byb2NrLWNo aXBzLmNvbT4KPiAtLS0KPiBDaGFuZ2VzIGluIHYyOgo+IC0gYWRqdXN0IG4vY3RzIHNldHRpbmcg b3JkZXIKPiAKPiAgZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9kd19oZG1pLmMgfCAzNyArKysrKysr KysrKysrKysrKysrKystLS0tLS0tLS0tLS0tLS0tCj4gIGRyaXZlcnMvZ3B1L2RybS9icmlkZ2Uv ZHdfaGRtaS5oIHwgIDYgKysrKysrCj4gIDIgZmlsZXMgY2hhbmdlZCwgMjcgaW5zZXJ0aW9ucygr KSwgMTYgZGVsZXRpb25zKC0pCj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9icmlk Z2UvZHdfaGRtaS5jIGIvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9kd19oZG1pLmMKPiBpbmRleCBj ZDZhNzA2Li40MjNhZGRjIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvZHdf aGRtaS5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9kd19oZG1pLmMKPiBAQCAtMTc3 LDI2ICsxNzcsMzEgQEAgc3RhdGljIHZvaWQgaGRtaV9tYXNrX3dyaXRlYihzdHJ1Y3QgZHdfaGRt aSAqaGRtaSwgdTggZGF0YSwgdW5zaWduZWQgaW50IHJlZywKPiAgCWhkbWlfbW9kYihoZG1pLCBk YXRhIDw8IHNoaWZ0LCBtYXNrLCByZWcpOwo+ICB9Cj4gIAo+IC1zdGF0aWMgdm9pZCBoZG1pX3Nl dF9jbG9ja19yZWdlbmVyYXRvcl9uKHN0cnVjdCBkd19oZG1pICpoZG1pLAo+IC0JCQkJCSB1bnNp Z25lZCBpbnQgdmFsdWUpCj4gK3N0YXRpYyB2b2lkIGhkbWlfcmVnZW5lcmF0ZV9uX2N0cyhzdHJ1 Y3QgZHdfaGRtaSAqaGRtaSwgdW5zaWduZWQgaW50IG4sCj4gKwkJCQkgIHVuc2lnbmVkIGludCBj dHMpCj4gIHsKPiAtCWhkbWlfd3JpdGViKGhkbWksIHZhbHVlICYgMHhmZiwgSERNSV9BVURfTjEp Owo+IC0JaGRtaV93cml0ZWIoaGRtaSwgKHZhbHVlID4+IDgpICYgMHhmZiwgSERNSV9BVURfTjIp Owo+IC0JaGRtaV93cml0ZWIoaGRtaSwgKHZhbHVlID4+IDE2KSAmIDB4MGYsIEhETUlfQVVEX04z KTsKPiArCS8qIHNldCBuY3RzX2F0b21pY193cml0ZSAqLwo+ICsJaGRtaV9tb2RiKGhkbWksIEhE TUlfQVVEX04zX05DVFNfQVRPTUlDX1dSSVRFX1NFVCwKPiArCQkgIEhETUlfQVVEX04zX05DVFNf QVRPTUlDX1dSSVRFX01BU0ssIEhETUlfQVVEX04zKTsKCkJpdHMgNzo0IG9mIE4zIGFyZSBtYXJr ZWQgdXAgaW4gdGhlIGlNWDYgbWFudWFscyBhcyAicmVzZXJ2ZWQiLiAgV2UgbmVlZApzb21lIGNs YXJpZmljYXRpb24gZnJvbSBGU0wgd2hldGhlciB0aGlzIG1hdHRlcnMsIGJ1dCBhdCB0aGUgbW9t ZW50IG15Cm9waW5pb24gb24gdGhhdCBpcyB0aGlzIHNob3VsZCBiZSBjb25kaXRpb25hbGlzZWQg YWdhaW5zdCB0aGUgSVAgdmVyc2lvbi4KClNldHRpbmcgYml0IDcgYXMgeW91IGRvIGFib3ZlIG1h eSBub3QgY2F1c2UgYW55IGhhcm0gb24gaU1YNiwgYnV0IG9uIHRoZQpvdGhlciBoYW5kLCB3ZSBz aG91bGRuJ3QgYmUgc2V0dGluZyBiaXRzIHdoaWNoIGFyZSBtYXJrZWQgYXMgcmVzZXJ2ZWQuCgo+ ICsKPiArCS8qIHNldCBjdHMgbWFudWFsICovCj4gKwloZG1pX21vZGIoaGRtaSwgSERNSV9BVURf Q1RTM19DVFNfTUFOVUFMLAo+ICsJCSAgSERNSV9BVURfQ1RTM19DVFNfTUFOVUFMLCBIRE1JX0FV RF9DVFMzKTsKPiAgCj4gIAkvKiBuc2hpZnQgZmFjdG9yID0gMCAqLwo+ICAJaGRtaV9tb2RiKGhk bWksIDAsIEhETUlfQVVEX0NUUzNfTl9TSElGVF9NQVNLLCBIRE1JX0FVRF9DVFMzKTsKPiAtfQo+ IC0KPiAtc3RhdGljIHZvaWQgaGRtaV9yZWdlbmVyYXRlX2N0cyhzdHJ1Y3QgZHdfaGRtaSAqaGRt aSwgdW5zaWduZWQgaW50IGN0cykKPiAtewo+IC0JLyogTXVzdCBiZSBzZXQvY2xlYXJlZCBmaXJz dCAqLwo+IC0JaGRtaV9tb2RiKGhkbWksIDAsIEhETUlfQVVEX0NUUzNfQ1RTX01BTlVBTCwgSERN SV9BVURfQ1RTMyk7Cj4gIAo+IC0JaGRtaV93cml0ZWIoaGRtaSwgY3RzICYgMHhmZiwgSERNSV9B VURfQ1RTMSk7Cj4gKwkvKiBzZXQgY3RzIHZhbHVlcyAqLwo+ICsJaGRtaV9tb2RiKGhkbWksICgo Y3RzID4+IDE2KSAmIEhETUlfQVVEX0NUUzNfQVVEQ1RTMTlfMTZfTUFTSyksCj4gKwkJICBIRE1J X0FVRF9DVFMzX0FVRENUUzE5XzE2X01BU0ssIEhETUlfQVVEX0NUUzMpOwo+ICAJaGRtaV93cml0 ZWIoaGRtaSwgKGN0cyA+PiA4KSAmIDB4ZmYsIEhETUlfQVVEX0NUUzIpOwo+IC0JaGRtaV93cml0 ZWIoaGRtaSwgKChjdHMgPj4gMTYpICYgSERNSV9BVURfQ1RTM19BVURDVFMxOV8xNl9NQVNLKSB8 Cj4gLQkJICAgIEhETUlfQVVEX0NUUzNfQ1RTX01BTlVBTCwgSERNSV9BVURfQ1RTMyk7Cj4gKwlo ZG1pX3dyaXRlYihoZG1pLCBjdHMgJiAweGZmLCBIRE1JX0FVRF9DVFMxKTsKPiArCj4gKwkvKiBz ZXQgbiB2YWx1ZXMgKi8KPiArCWhkbWlfbW9kYihoZG1pLCAobiA+PiAxNikgJiBIRE1JX0FVRF9O M19BVUROMTlfMTZfTUFTSywKPiArCQkgIEhETUlfQVVEX04zX0FVRE4xOV8xNl9NQVNLLCBIRE1J X0FVRF9OMyk7Cj4gKwloZG1pX3dyaXRlYihoZG1pLCAobiA+PiA4KSAmIDB4ZmYsIEhETUlfQVVE X04yKTsKPiArCWhkbWlfd3JpdGViKGhkbWksIG4gJiAweGZmLCBIRE1JX0FVRF9OMSk7CgpUaGlz IGlzIGRlZmluaXRlbHkgbW92aW5nIHRoaW5ncyBpbiB0aGUgcmlnaHQgZGlyZWN0aW9uLiAgSG93 ZXZlciwgSSB3b3VsZAphc2sgdGhhdCB0aGUgcmVhZC1tb2RpZnktd3JpdGVzIHRvIEhETUlfQVVE X04zIGFyZSBvcGVuLWNvZGVkIHJhdGhlciB0aGFuCnVzaW5nIGhkbWlfbW9kYigpLCBhbmQgY29u c29saWRhdGVkLiAgV2UgcmVhbGx5IGRvbid0IG5lZWQgdG8ga2VlcApyZS1yZWFkaW5nIHRoaXMg cmVnaXN0ZXIuCgpJJ2QgYWxzbyBsaWtlIHRvIGNoZWNrIHRoYXQgdGhpcyBkb2VzIG5vdCBjYXVz ZSBhIHJlZ3Jlc3Npb24gb24gaU1YNiBTb0NzCndpdGggbXkgYXVkaW8gZHJpdmVyOyBJJ20gYXdh cmUgdGhhdCB0aGVyZSBhcmUgc29tZSBpc3N1ZXMgdG8gZG8gd2l0aCBob3cKdGhlc2UgcmVnaXN0 ZXJzIGFyZSB3cml0dGVuLCBhbmQgdGhlIHB1Ymxpc2hlZCBlcnJhdGEgd29ya2Fyb3VuZCBpbiB0 aGUKaU1YNiBlcnJhdGEgZG9jdW1lbnRhdGlvbiBkb2Vzbid0IHNlZW0gdG8gYWx3YXlzIHdvcmsu Cgo+ICB9Cj4gIAo+ICBzdGF0aWMgdW5zaWduZWQgaW50IGhkbWlfY29tcHV0ZV9uKHVuc2lnbmVk IGludCBmcmVxLCB1bnNpZ25lZCBsb25nIHBpeGVsX2NsaywKPiBAQCAtMzU1LDggKzM2MCw4IEBA IHN0YXRpYyB2b2lkIGhkbWlfc2V0X2Nsa19yZWdlbmVyYXRvcihzdHJ1Y3QgZHdfaGRtaSAqaGRt aSwKPiAgCQlfX2Z1bmNfXywgaGRtaS0+c2FtcGxlX3JhdGUsIGhkbWktPnJhdGlvLAo+ICAJCXBp eGVsX2NsaywgY2xrX24sIGNsa19jdHMpOwo+ICAKPiAtCWhkbWlfc2V0X2Nsb2NrX3JlZ2VuZXJh dG9yX24oaGRtaSwgY2xrX24pOwo+IC0JaGRtaV9yZWdlbmVyYXRlX2N0cyhoZG1pLCBjbGtfY3Rz KTsKPiArCWhkbWlfcmVnZW5lcmF0ZV9uX2N0cyhoZG1pLCBjbGtfbiwgY2xrX2N0cyk7Cj4gKwlo ZG1pX3NldF9zY2hubChoZG1pKTsKCkknZCBwcmVmZXIgdGhlIGFkZGl0aW9uIG9mIGhkbWlfc2V0 X3NjaG5sKCkgdG8gYmUgaW4gdGhlIHBhdGNoIHdoaWNoIGFkZHMKdGhhdCBuZXcgZnVuY3Rpb24u ICBIb3dldmVyLCBhcyBJIG1lbnRpb25lZCBpbiBteSByZXBseSB0byB0aGF0IHBhdGNoLApvdGhl ciB2ZXJzaW9ucyBvZiB0aGlzIElQIGRvIG5vdCBoYXZlIHRoZXNlIHJlZ2lzdGVycywgYW5kIGl0 IG1heSBub3QgYmUKc2FmZSB0byB3cml0ZSB0byB0aGVtLiAgV2UgbmVlZCB0byBmaW5kIGEgd2F5 IHRvIGtub3cgd2hldGhlciB0aGlzIElQCmhhcyB0aGUgQUhCIERNQSBzdXBwb3J0IG9yIG5vdCBh bmQgYWN0IGFjY29yZGluZ2x5LgoKLS0gCkZUVEMgYnJvYWRiYW5kIGZvciAwLjhtaWxlIGxpbmU6 IGN1cnJlbnRseSBhdCAxMC41TWJwcyBkb3duIDQwMGticHMgdXAKYWNjb3JkaW5nIHRvIHNwZWVk dGVzdC5uZXQuCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f CmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpo dHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg==