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@xxxxxxxxxx>
> >>---
> >> 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
Attachment:
pgpOa6L3g3clH.pgp
Description: PGP signature
