From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754145AbdDNJFT (ORCPT ); Fri, 14 Apr 2017 05:05:19 -0400 Received: from mail-wr0-f179.google.com ([209.85.128.179]:33486 "EHLO mail-wr0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753692AbdDNJFQ (ORCPT ); Fri, 14 Apr 2017 05:05:16 -0400 Subject: Re: [PATCH v2 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling To: Romain Perier , Archit Taneja , David Airlie References: <20170414083113.4255-1-romain.perier@collabora.com> <20170414083113.4255-2-romain.perier@collabora.com> Cc: dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Jose Abreu , Russell King From: Neil Armstrong Organization: Baylibre Message-ID: <18f8eac1-64c2-949e-c4e1-6c87a5e757c3@baylibre.com> Date: Fri, 14 Apr 2017 11:05:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <20170414083113.4255-2-romain.perier@collabora.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/14/2017 10:31 AM, Romain Perier wrote: > Currently, CTS+N is forced to zero as a workaround of the IP block for > i.MX platforms. This is requested in the datasheet of the corresponding > IP for AHB mode only. However, we have seen that it introduces glitches > or delays when playing a sound on HDMI for I2S mode. This proves that we > cannot keep the current functions for handling audio stream as-is if > these contain workaround that are specific to a mode. > > This commit introduces two callbacks, one for each variant. > dw_hdmi_setup defines the right function depending on the detected > variant. Then, the exported functions dw_hdmi_audio_enable and > dw_hdmi_audio_disable calls the corresponding callbacks > > Signed-off-by: Romain Perier > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 4b6f216..5b328c0 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -173,6 +173,8 @@ struct dw_hdmi { > > unsigned int reg_shift; > struct regmap *regm; > + void (*enable_audio)(struct dw_hdmi *hdmi); > + void (*disable_audio)(struct dw_hdmi *hdmi); > }; > > #define HDMI_IH_PHY_STAT0_RX_SENSE \ > @@ -542,13 +544,29 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate) > } > EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate); > > +void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi) > +{ > + hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n); > +} > + > +void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi) > +{ > + hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0); > +} > + > +void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi) > +{ > + hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n); > +} > + > void dw_hdmi_audio_enable(struct dw_hdmi *hdmi) > { > unsigned long flags; > > spin_lock_irqsave(&hdmi->audio_lock, flags); > hdmi->audio_enable = true; > - hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n); > + if (hdmi->enable_audio) > + hdmi->enable_audio(hdmi); > spin_unlock_irqrestore(&hdmi->audio_lock, flags); > } > EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable); > @@ -559,7 +577,8 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi) > > spin_lock_irqsave(&hdmi->audio_lock, flags); > hdmi->audio_enable = false; > - hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0); > + if (hdmi->disable_audio) > + hdmi->disable_audio(hdmi); > spin_unlock_irqrestore(&hdmi->audio_lock, flags); > } > EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable); > @@ -2404,6 +2423,8 @@ __dw_hdmi_probe(struct platform_device *pdev, > audio.irq = irq; > audio.hdmi = hdmi; > audio.eld = hdmi->connector.eld; > + hdmi->enable_audio = dw_hdmi_ahb_audio_enable; > + hdmi->disable_audio = dw_hdmi_ahb_audio_disable; > > pdevinfo.name = "dw-hdmi-ahb-audio"; > pdevinfo.data = &audio; > @@ -2416,6 +2437,7 @@ __dw_hdmi_probe(struct platform_device *pdev, > audio.hdmi = hdmi; > audio.write = hdmi_writeb; > audio.read = hdmi_readb; > + hdmi->enable_audio = dw_hdmi_i2s_audio_enable; > > pdevinfo.name = "dw-hdmi-i2s-audio"; > pdevinfo.data = &audio; > Hi Romain, Reviewed-by: Neil Armstrong From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Armstrong Subject: Re: [PATCH v2 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling Date: Fri, 14 Apr 2017 11:05:11 +0200 Message-ID: <18f8eac1-64c2-949e-c4e1-6c87a5e757c3@baylibre.com> References: <20170414083113.4255-1-romain.perier@collabora.com> <20170414083113.4255-2-romain.perier@collabora.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20170414083113.4255-2-romain.perier@collabora.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Romain Perier , Archit Taneja , David Airlie Cc: Jose Abreu , Russell King , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org List-Id: linux-rockchip.vger.kernel.org T24gMDQvMTQvMjAxNyAxMDozMSBBTSwgUm9tYWluIFBlcmllciB3cm90ZToKPiBDdXJyZW50bHks IENUUytOIGlzIGZvcmNlZCB0byB6ZXJvIGFzIGEgd29ya2Fyb3VuZCBvZiB0aGUgSVAgYmxvY2sg Zm9yCj4gaS5NWCBwbGF0Zm9ybXMuIFRoaXMgaXMgcmVxdWVzdGVkIGluIHRoZSBkYXRhc2hlZXQg b2YgdGhlIGNvcnJlc3BvbmRpbmcKPiBJUCBmb3IgQUhCIG1vZGUgb25seS4gSG93ZXZlciwgd2Ug aGF2ZSBzZWVuIHRoYXQgaXQgaW50cm9kdWNlcyBnbGl0Y2hlcwo+IG9yIGRlbGF5cyB3aGVuIHBs YXlpbmcgYSBzb3VuZCBvbiBIRE1JIGZvciBJMlMgbW9kZS4gVGhpcyBwcm92ZXMgdGhhdCB3ZQo+ IGNhbm5vdCBrZWVwIHRoZSBjdXJyZW50IGZ1bmN0aW9ucyBmb3IgaGFuZGxpbmcgYXVkaW8gc3Ry ZWFtIGFzLWlzIGlmCj4gdGhlc2UgY29udGFpbiB3b3JrYXJvdW5kIHRoYXQgYXJlIHNwZWNpZmlj IHRvIGEgbW9kZS4KPiAKPiBUaGlzIGNvbW1pdCBpbnRyb2R1Y2VzIHR3byBjYWxsYmFja3MsIG9u ZSBmb3IgZWFjaCB2YXJpYW50Lgo+IGR3X2hkbWlfc2V0dXAgZGVmaW5lcyB0aGUgcmlnaHQgZnVu Y3Rpb24gZGVwZW5kaW5nIG9uIHRoZSBkZXRlY3RlZAo+IHZhcmlhbnQuIFRoZW4sIHRoZSBleHBv cnRlZCBmdW5jdGlvbnMgZHdfaGRtaV9hdWRpb19lbmFibGUgYW5kCj4gZHdfaGRtaV9hdWRpb19k aXNhYmxlIGNhbGxzIHRoZSBjb3JyZXNwb25kaW5nIGNhbGxiYWNrcwo+IAo+IFNpZ25lZC1vZmYt Ynk6IFJvbWFpbiBQZXJpZXIgPHJvbWFpbi5wZXJpZXJAY29sbGFib3JhLmNvbT4KPiAtLS0KPiAg ZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9zeW5vcHN5cy9kdy1oZG1pLmMgfCAyNiArKysrKysrKysr KysrKysrKysrKysrKystLQo+ICAxIGZpbGUgY2hhbmdlZCwgMjQgaW5zZXJ0aW9ucygrKSwgMiBk ZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9zeW5v cHN5cy9kdy1oZG1pLmMgYi9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL3N5bm9wc3lzL2R3LWhkbWku Ywo+IGluZGV4IDRiNmYyMTYuLjViMzI4YzAgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJt L2JyaWRnZS9zeW5vcHN5cy9kdy1oZG1pLmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vYnJpZGdl L3N5bm9wc3lzL2R3LWhkbWkuYwo+IEBAIC0xNzMsNiArMTczLDggQEAgc3RydWN0IGR3X2hkbWkg ewo+ICAKPiAgCXVuc2lnbmVkIGludCByZWdfc2hpZnQ7Cj4gIAlzdHJ1Y3QgcmVnbWFwICpyZWdt Owo+ICsJdm9pZCAoKmVuYWJsZV9hdWRpbykoc3RydWN0IGR3X2hkbWkgKmhkbWkpOwo+ICsJdm9p ZCAoKmRpc2FibGVfYXVkaW8pKHN0cnVjdCBkd19oZG1pICpoZG1pKTsKPiAgfTsKPiAgCj4gICNk ZWZpbmUgSERNSV9JSF9QSFlfU1RBVDBfUlhfU0VOU0UgXAo+IEBAIC01NDIsMTMgKzU0NCwyOSBA QCB2b2lkIGR3X2hkbWlfc2V0X3NhbXBsZV9yYXRlKHN0cnVjdCBkd19oZG1pICpoZG1pLCB1bnNp Z25lZCBpbnQgcmF0ZSkKPiAgfQo+ICBFWFBPUlRfU1lNQk9MX0dQTChkd19oZG1pX3NldF9zYW1w bGVfcmF0ZSk7Cj4gIAo+ICt2b2lkIGR3X2hkbWlfYWhiX2F1ZGlvX2VuYWJsZShzdHJ1Y3QgZHdf aGRtaSAqaGRtaSkKPiArewo+ICsJaGRtaV9zZXRfY3RzX24oaGRtaSwgaGRtaS0+YXVkaW9fY3Rz LCBoZG1pLT5hdWRpb19uKTsKPiArfQo+ICsKPiArdm9pZCBkd19oZG1pX2FoYl9hdWRpb19kaXNh YmxlKHN0cnVjdCBkd19oZG1pICpoZG1pKQo+ICt7Cj4gKwloZG1pX3NldF9jdHNfbihoZG1pLCBo ZG1pLT5hdWRpb19jdHMsIDApOwo+ICt9Cj4gKwo+ICt2b2lkIGR3X2hkbWlfaTJzX2F1ZGlvX2Vu YWJsZShzdHJ1Y3QgZHdfaGRtaSAqaGRtaSkKPiArewo+ICsJaGRtaV9zZXRfY3RzX24oaGRtaSwg aGRtaS0+YXVkaW9fY3RzLCBoZG1pLT5hdWRpb19uKTsKPiArfQo+ICsKPiAgdm9pZCBkd19oZG1p X2F1ZGlvX2VuYWJsZShzdHJ1Y3QgZHdfaGRtaSAqaGRtaSkKPiAgewo+ICAJdW5zaWduZWQgbG9u ZyBmbGFnczsKPiAgCj4gIAlzcGluX2xvY2tfaXJxc2F2ZSgmaGRtaS0+YXVkaW9fbG9jaywgZmxh Z3MpOwo+ICAJaGRtaS0+YXVkaW9fZW5hYmxlID0gdHJ1ZTsKPiAtCWhkbWlfc2V0X2N0c19uKGhk bWksIGhkbWktPmF1ZGlvX2N0cywgaGRtaS0+YXVkaW9fbik7Cj4gKwlpZiAoaGRtaS0+ZW5hYmxl X2F1ZGlvKQo+ICsJCWhkbWktPmVuYWJsZV9hdWRpbyhoZG1pKTsKPiAgCXNwaW5fdW5sb2NrX2ly cXJlc3RvcmUoJmhkbWktPmF1ZGlvX2xvY2ssIGZsYWdzKTsKPiAgfQo+ICBFWFBPUlRfU1lNQk9M X0dQTChkd19oZG1pX2F1ZGlvX2VuYWJsZSk7Cj4gQEAgLTU1OSw3ICs1NzcsOCBAQCB2b2lkIGR3 X2hkbWlfYXVkaW9fZGlzYWJsZShzdHJ1Y3QgZHdfaGRtaSAqaGRtaSkKPiAgCj4gIAlzcGluX2xv Y2tfaXJxc2F2ZSgmaGRtaS0+YXVkaW9fbG9jaywgZmxhZ3MpOwo+ICAJaGRtaS0+YXVkaW9fZW5h YmxlID0gZmFsc2U7Cj4gLQloZG1pX3NldF9jdHNfbihoZG1pLCBoZG1pLT5hdWRpb19jdHMsIDAp Owo+ICsJaWYgKGhkbWktPmRpc2FibGVfYXVkaW8pCj4gKwkJaGRtaS0+ZGlzYWJsZV9hdWRpbyho ZG1pKTsKPiAgCXNwaW5fdW5sb2NrX2lycXJlc3RvcmUoJmhkbWktPmF1ZGlvX2xvY2ssIGZsYWdz KTsKPiAgfQo+ICBFWFBPUlRfU1lNQk9MX0dQTChkd19oZG1pX2F1ZGlvX2Rpc2FibGUpOwo+IEBA IC0yNDA0LDYgKzI0MjMsOCBAQCBfX2R3X2hkbWlfcHJvYmUoc3RydWN0IHBsYXRmb3JtX2Rldmlj ZSAqcGRldiwKPiAgCQlhdWRpby5pcnEgPSBpcnE7Cj4gIAkJYXVkaW8uaGRtaSA9IGhkbWk7Cj4g IAkJYXVkaW8uZWxkID0gaGRtaS0+Y29ubmVjdG9yLmVsZDsKPiArCQloZG1pLT5lbmFibGVfYXVk aW8gPSBkd19oZG1pX2FoYl9hdWRpb19lbmFibGU7Cj4gKwkJaGRtaS0+ZGlzYWJsZV9hdWRpbyA9 IGR3X2hkbWlfYWhiX2F1ZGlvX2Rpc2FibGU7Cj4gIAo+ICAJCXBkZXZpbmZvLm5hbWUgPSAiZHct aGRtaS1haGItYXVkaW8iOwo+ICAJCXBkZXZpbmZvLmRhdGEgPSAmYXVkaW87Cj4gQEAgLTI0MTYs NiArMjQzNyw3IEBAIF9fZHdfaGRtaV9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpwZGV2 LAo+ICAJCWF1ZGlvLmhkbWkJPSBoZG1pOwo+ICAJCWF1ZGlvLndyaXRlCT0gaGRtaV93cml0ZWI7 Cj4gIAkJYXVkaW8ucmVhZAk9IGhkbWlfcmVhZGI7Cj4gKwkJaGRtaS0+ZW5hYmxlX2F1ZGlvID0g ZHdfaGRtaV9pMnNfYXVkaW9fZW5hYmxlOwo+ICAKPiAgCQlwZGV2aW5mby5uYW1lID0gImR3LWhk bWktaTJzLWF1ZGlvIjsKPiAgCQlwZGV2aW5mby5kYXRhID0gJmF1ZGlvOwo+IAoKSGkgUm9tYWlu LAoKUmV2aWV3ZWQtYnk6IE5laWwgQXJtc3Ryb25nIDxuYXJtc3Ryb25nQGJheWxpYnJlLmNvbT4K X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: narmstrong@baylibre.com (Neil Armstrong) Date: Fri, 14 Apr 2017 11:05:11 +0200 Subject: [PATCH v2 1/2] drm: dw-hdmi: add specific I2S and AHB functions for stream handling In-Reply-To: <20170414083113.4255-2-romain.perier@collabora.com> References: <20170414083113.4255-1-romain.perier@collabora.com> <20170414083113.4255-2-romain.perier@collabora.com> Message-ID: <18f8eac1-64c2-949e-c4e1-6c87a5e757c3@baylibre.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 04/14/2017 10:31 AM, Romain Perier wrote: > Currently, CTS+N is forced to zero as a workaround of the IP block for > i.MX platforms. This is requested in the datasheet of the corresponding > IP for AHB mode only. However, we have seen that it introduces glitches > or delays when playing a sound on HDMI for I2S mode. This proves that we > cannot keep the current functions for handling audio stream as-is if > these contain workaround that are specific to a mode. > > This commit introduces two callbacks, one for each variant. > dw_hdmi_setup defines the right function depending on the detected > variant. Then, the exported functions dw_hdmi_audio_enable and > dw_hdmi_audio_disable calls the corresponding callbacks > > Signed-off-by: Romain Perier > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 4b6f216..5b328c0 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -173,6 +173,8 @@ struct dw_hdmi { > > unsigned int reg_shift; > struct regmap *regm; > + void (*enable_audio)(struct dw_hdmi *hdmi); > + void (*disable_audio)(struct dw_hdmi *hdmi); > }; > > #define HDMI_IH_PHY_STAT0_RX_SENSE \ > @@ -542,13 +544,29 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate) > } > EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate); > > +void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi) > +{ > + hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n); > +} > + > +void dw_hdmi_ahb_audio_disable(struct dw_hdmi *hdmi) > +{ > + hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0); > +} > + > +void dw_hdmi_i2s_audio_enable(struct dw_hdmi *hdmi) > +{ > + hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n); > +} > + > void dw_hdmi_audio_enable(struct dw_hdmi *hdmi) > { > unsigned long flags; > > spin_lock_irqsave(&hdmi->audio_lock, flags); > hdmi->audio_enable = true; > - hdmi_set_cts_n(hdmi, hdmi->audio_cts, hdmi->audio_n); > + if (hdmi->enable_audio) > + hdmi->enable_audio(hdmi); > spin_unlock_irqrestore(&hdmi->audio_lock, flags); > } > EXPORT_SYMBOL_GPL(dw_hdmi_audio_enable); > @@ -559,7 +577,8 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi) > > spin_lock_irqsave(&hdmi->audio_lock, flags); > hdmi->audio_enable = false; > - hdmi_set_cts_n(hdmi, hdmi->audio_cts, 0); > + if (hdmi->disable_audio) > + hdmi->disable_audio(hdmi); > spin_unlock_irqrestore(&hdmi->audio_lock, flags); > } > EXPORT_SYMBOL_GPL(dw_hdmi_audio_disable); > @@ -2404,6 +2423,8 @@ __dw_hdmi_probe(struct platform_device *pdev, > audio.irq = irq; > audio.hdmi = hdmi; > audio.eld = hdmi->connector.eld; > + hdmi->enable_audio = dw_hdmi_ahb_audio_enable; > + hdmi->disable_audio = dw_hdmi_ahb_audio_disable; > > pdevinfo.name = "dw-hdmi-ahb-audio"; > pdevinfo.data = &audio; > @@ -2416,6 +2437,7 @@ __dw_hdmi_probe(struct platform_device *pdev, > audio.hdmi = hdmi; > audio.write = hdmi_writeb; > audio.read = hdmi_readb; > + hdmi->enable_audio = dw_hdmi_i2s_audio_enable; > > pdevinfo.name = "dw-hdmi-i2s-audio"; > pdevinfo.data = &audio; > Hi Romain, Reviewed-by: Neil Armstrong