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 at nvidia.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 -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130828/d09d9c2f/attachment-0001.pgp>