Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1456322 > unrolled thread

[PATCH 2/3 v3] drm: bridge/dw-hdmi: Enable ISCR1, ISCR2 and ACP packets

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/3 v3] drm: bridge/dw-hdmi: Enable ISCR1, ISCR2 and ACP packets Jose Abreu <Jose.Abreu@synopsys.com> - 2016-08-04 12:50 +0200
    Re: [PATCH 2/3 v3] drm: bridge/dw-hdmi: Enable ISCR1, ISCR2 and ACP  packets Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-08-04 17:40 +0200
      Re: [PATCH 2/3 v3] drm: bridge/dw-hdmi: Enable ISCR1, ISCR2 and ACP  packets Jose Abreu <Jose.Abreu@synopsys.com> - 2016-08-04 18:20 +0200

#1456322 — [PATCH 2/3 v3] drm: bridge/dw-hdmi: Enable ISCR1, ISCR2 and ACP packets

Currently ISCR and ACP packets are not being sent causing
HDMI compliance tests like CTS 7-19 HDMI 1.4b to fail.

With this pacth the mentioned packets are activated when
needed.

Verified using HDMI compliance equipment.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Carlos Palminha <palminha@synopsys.com>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: David Airlie <airlied@linux.ie>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
---

This patch was only introduced in v3.

 drivers/gpu/drm/bridge/dw-hdmi.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index b5fac27..9122a20 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -127,6 +127,7 @@ struct dw_hdmi {
 	void __iomem *regs;
 	bool sink_is_hdmi;
 	bool sink_has_audio;
+	bool sink_supports_ai;
 
 	struct mutex mutex;		/* for state below and previous_mode */
 	enum drm_connector_force force;	/* mutex-protected force state */
@@ -216,6 +217,21 @@ static void hdmi_set_cts_n(struct dw_hdmi *hdmi, unsigned int cts,
 	hdmi_writeb(hdmi, (n >> 8) & 0xff, HDMI_AUD_N2);
 	hdmi_writeb(hdmi, n & 0xff, HDMI_AUD_N1);
 
+	/* Set ISCR1, ISCR2, and ACP packets to automatic scheduling */
+	if (hdmi->sink_supports_ai) {
+		dev_dbg(hdmi->dev, "sink supports AI packets\n");
+		hdmi_writeb(hdmi, 0x06, HDMI_FC_ISCR1_0);
+		hdmi_writeb(hdmi, 0x03, HDMI_FC_DATAUTO0);
+		hdmi_writeb(hdmi, 0x01, HDMI_FC_DATAUTO1);
+		hdmi_writeb(hdmi, 0x11, HDMI_FC_DATAUTO2);
+	} else {
+		dev_dbg(hdmi->dev, "sink does not support AI packets\n");
+		hdmi_writeb(hdmi, 0x00, HDMI_FC_ISCR1_0);
+		hdmi_writeb(hdmi, 0x00, HDMI_FC_DATAUTO0);
+		hdmi_writeb(hdmi, 0x00, HDMI_FC_DATAUTO1);
+		hdmi_writeb(hdmi, 0x00, HDMI_FC_DATAUTO2);
+	}
+
 	/* Set Frame Composer Audio Sample sampling frequency */
 	if (hdmi->dev_type == DWC_HDMI) {
 		u8 val = 0x0;
@@ -1474,8 +1490,13 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
 		/* Store the ELD */
 		drm_edid_to_eld(connector, edid);
 		kfree(edid);
+
+		hdmi->sink_supports_ai = connector->eld[5] & (0x1 << 1);
 	} else {
 		dev_dbg(hdmi->dev, "failed to get edid\n");
+		hdmi->sink_is_hdmi = false;
+		hdmi->sink_has_audio = false;
+		hdmi->sink_supports_ai = false;
 	}
 
 	return ret;
-- 
2.1.4

[toc] | [next] | [standalone]

#1456495 — Re: [PATCH 2/3 v3] drm: bridge/dw-hdmi: Enable ISCR1, ISCR2 and ACP packets

On Thu, Aug 04, 2016 at 11:44:50AM +0100, Jose Abreu wrote:
> Currently ISCR and ACP packets are not being sent causing
> HDMI compliance tests like CTS 7-19 HDMI 1.4b to fail.

Hmm.  Reading the HDMI specifications (v1.3, being the publically
available one), the specification does _not_ say that a source _must_
transmit ISRC and ACP packets if Supports_AI is set.

What it does say is that:
* a sink shall set Supports_AI if it supports at least one function
  which uses ISRC or ACP packets, and it shall accept such packets.
* a source shall not transmit ISRC or ACP packets to a sink that
  has Supports_AI=0.

I don't see anything that mandates the transmission of ISRC and ACP
packets when Supports_AI is set.

Moreover, transmission of wrong ISRC information probably goes against
the HDMI spec section 8.8, which requires specific ISRC status values
in HB1 - I don't see how that _could_ be handled, as the kernel has no
knowledge about the EAN of the audio content being played, nor when a
track starts or finishes.

Please clarify.

> +	/* Set ISCR1, ISCR2, and ACP packets to automatic scheduling */
> +	if (hdmi->sink_supports_ai) {
> +		dev_dbg(hdmi->dev, "sink supports AI packets\n");
> +		hdmi_writeb(hdmi, 0x06, HDMI_FC_ISCR1_0);
> +		hdmi_writeb(hdmi, 0x03, HDMI_FC_DATAUTO0);
> +		hdmi_writeb(hdmi, 0x01, HDMI_FC_DATAUTO1);
> +		hdmi_writeb(hdmi, 0x11, HDMI_FC_DATAUTO2);

So what data gets sent in these packets - I guess whatever random data
happens to be in the HDMI_FC_ACP* and HDMI_FC_ISCR1_* registers at the
time the sending is enabled.  I don't think that's particularly a good
idea.

> @@ -1474,8 +1490,13 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
>  		/* Store the ELD */
>  		drm_edid_to_eld(connector, edid);
>  		kfree(edid);
> +
> +		hdmi->sink_supports_ai = connector->eld[5] & (0x1 << 1);

We have a definition for this bit - DRM_ELD_SUPPORTS_AI.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1456525 — Re: [PATCH 2/3 v3] drm: bridge/dw-hdmi: Enable ISCR1, ISCR2 and ACP packets

Hi Russell,


On 04-08-2016 16:32, Russell King - ARM Linux wrote:
> On Thu, Aug 04, 2016 at 11:44:50AM +0100, Jose Abreu wrote:
>> Currently ISCR and ACP packets are not being sent causing
>> HDMI compliance tests like CTS 7-19 HDMI 1.4b to fail.
> Hmm.  Reading the HDMI specifications (v1.3, being the publically
> available one), the specification does _not_ say that a source _must_
> transmit ISRC and ACP packets if Supports_AI is set.
>
> What it does say is that:
> * a sink shall set Supports_AI if it supports at least one function
>   which uses ISRC or ACP packets, and it shall accept such packets.
> * a source shall not transmit ISRC or ACP packets to a sink that
>   has Supports_AI=0.
>
> I don't see anything that mandates the transmission of ISRC and ACP
> packets when Supports_AI is set.
>
> Moreover, transmission of wrong ISRC information probably goes against
> the HDMI spec section 8.8, which requires specific ISRC status values
> in HB1 - I don't see how that _could_ be handled, as the kernel has no
> knowledge about the EAN of the audio content being played, nor when a
> track starts or finishes.
>
> Please clarify.

You are correct. I ran the test again and indeed it is not
failing but it is skipped. Sorry for the confusion. Anyway, with
this patch the test that verifies the ISRC packets succeeds so I
think it would be a nice addition.

>
>> +	/* Set ISCR1, ISCR2, and ACP packets to automatic scheduling */
>> +	if (hdmi->sink_supports_ai) {
>> +		dev_dbg(hdmi->dev, "sink supports AI packets\n");
>> +		hdmi_writeb(hdmi, 0x06, HDMI_FC_ISCR1_0);
>> +		hdmi_writeb(hdmi, 0x03, HDMI_FC_DATAUTO0);
>> +		hdmi_writeb(hdmi, 0x01, HDMI_FC_DATAUTO1);
>> +		hdmi_writeb(hdmi, 0x11, HDMI_FC_DATAUTO2);
> So what data gets sent in these packets - I guess whatever random data
> happens to be in the HDMI_FC_ACP* and HDMI_FC_ISCR1_* registers at the
> time the sending is enabled.  I don't think that's particularly a good
> idea.

It is not random as the registers defaults to zero per databook.
It is sending an empty packet.

>> @@ -1474,8 +1490,13 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
>>  		/* Store the ELD */
>>  		drm_edid_to_eld(connector, edid);
>>  		kfree(edid);
>> +
>> +		hdmi->sink_supports_ai = connector->eld[5] & (0x1 << 1);
> We have a definition for this bit - DRM_ELD_SUPPORTS_AI.
>

Thanks. Will change in next version if you agree with this
addition. If not I will drop the patch.


Best regards,
Jose Miguel Abreu

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web