Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID

Tomi Valkeinen
2012-06-28T11:04:21Z

[-- Attachment #1: Type: text/plain, Size: 1732 bytes --]

On Thu, 2012-06-28 at 16:17 +0530, Jassi Brar wrote:
> On 28 June 2012 15:44, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> > When the display device is disabled, we're not listening to the hpd
> > interrupt anymore. So when it's disabled, the cable can be replugged and
> > the monitor changed, and the driver won't know about it. And so it'll
> > return the old EDID for the new monitor.
> >
> If that could be a problem, then we already have some problem with
> current omapdss.
>
> I think among the first things, while enabling HDMI, should be to see
> if there is really some display connected on the port i.e, HPD
> asserted. Only if ti_hdmi_4xxx_detect() returned true, should we
> proceed otherwise wait for HPQ irq.

I'm not sure I understand. The HDMI driver does just that. It calls
hdmi_check_hpd_state when it's being enabled to see if there's a display
connected.

> Unconditionally invalidating edid really seems like a regression - we
> impose atleast 50ms (edid read) as extra cost on
> hdmi_check_hpd_state(), which kills half the purpose of this patch.

If the HDMI hardware is turned off, we should unconditionally invalidate
the EDID. We have no way to keep track if the same monitor is kept
plugged in or not.

So what exactly is the purpose of this patch, what kind of scenario? I
thought it's to cache the EDID, so that if it will be read multiple
times while the same monitor is connected, we'll just return the cached
value.

Of course, I don't know why the upper layers would read the EDID
multiple times, because I think they should read it only once. So
perhaps I'm either not understanding something, or it's the omapdrm
layer that should be fixed?

Tomi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]