linux-kernel - Re: [PATCH 2/5] host1x: hdmi: Detect whether display is connected with HDMI or DVI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130828131608.GA604@ulmo>
Date:	Wed, 28 Aug 2013 15:16:09 +0200
From:	Thierry Reding <thierry.reding@...il.com>
To:	Mikko Perttunen <mperttunen@...dia.com>
Cc:	Terje Bergstrom <tbergstrom@...dia.com>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/5] host1x: hdmi: Detect whether display is connected
 with HDMI or DVI

On Wed, Aug 28, 2013 at 03:34:53PM +0300, Mikko Perttunen wrote:
> On 08/28/2013 03:07 PM, Thierry Reding wrote:
> >* PGP Signed by an unknown key
> >
> >On Wed, Aug 28, 2013 at 01:40:56PM +0300, Mikko Perttunen wrote:
> >>Use EDID data to determine whether the display supports HDMI or just DVI.
> >>This used to be hardcoded to be HDMI, which broke support for DVI displays
> >>that couldn't understand the interspersed audio/other data.
> >>
> >>If the EDID data isn't available, default to DVI, which should be a safer
> >>choice.
> >>
> >>Signed-off-by: Mikko Perttunen <mperttunen@...dia.com>
> >>---
> >>  drivers/gpu/host1x/drm/hdmi.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >>diff --git a/drivers/gpu/host1x/drm/hdmi.c b/drivers/gpu/host1x/drm/hdmi.c
> >>index d81fac8..140339b 100644
> >>--- a/drivers/gpu/host1x/drm/hdmi.c
> >>+++ b/drivers/gpu/host1x/drm/hdmi.c
> >>@@ -702,6 +702,14 @@ static int tegra_output_hdmi_enable(struct tegra_output *output)
> >>  	unsigned long value;
> >>  	int retries = 1000;
> >>  	int err;
> >>+	struct drm_property_blob *edid_blob = output->connector.edid_blob_ptr;
> >>+
> >>+	if (edid_blob && edid_blob->data &&
> >>+		drm_detect_hdmi_monitor((struct edid *)edid_blob->data)) {
> >>+		hdmi->dvi = false;
> >>+	} else {
> >>+		hdmi->dvi = true;
> >>+	}
> >>
> >>  	pclk = mode->clock * 1000;
> >>  	h_sync_width = mode->hsync_end - mode->hsync_start;
> >
> >Odd, now that I see that code I remember that there was a similar patch
> >a few months back, but it was never applied for some reason:
> >
> >	http://lists.freedesktop.org/archives/dri-devel/2013-January/033509.html
> >
> >That was already reviewed by me and Jon Mayo, so I'll go ahead and apply
> >that one instead.
> >
> >Thierry
> >
> >* Unknown Key
> >* 0x7F3EB3A1
> >
> 
> That patch seems to cause a warning for me:
> drivers/gpu/host1x/drm/hdmi.c: In function ‘tegra_output_hdmi_enable’:
> drivers/gpu/host1x/drm/hdmi.c:706:2: warning: passing argument 1 of
> ‘drm_detect_hdmi_monitor’ discards ‘const’ qualifier from pointer
> target type [enabled by default]
> include/drm/drm_crtc.h:1037:13: note: expected ‘struct edid *’ but
> argument is of type ‘const struct edid *’

Given the discussion about this back in January, I think the best way,
at least for now, is to keep a copy of the EDID data so that it can
actually be modified.

But looking at the problem more closely, I don't think the patch from
January is quite correct. The problem is that it uses the EDID stored
within the struct tegra_output. That's problematic because that field
is not updated when EDID is probed via DDC. That would make the patch
that you proposed more correct.

Thierry

Content of type "application/pgp-signature" skipped