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

On 28 June 2012 13:18, Tomi Valkeinen [off-list ref] wrote:

On Wed, 2012-06-27 at 19:35 +0530, jaswinder.singh@linaro.org wrote:

quoted

From: Jassi Brar <redacted>

We can easily keep track of latest EDID from the display and hence avoid
expensive EDID re-reads over I2C.
This could also help some cheapo displays that provide EDID reliably only
immediately after asserting HPD and not later.
Even with good displays, there is something in OMAPDSS that apparantly
messes up DDC occasionally while EDID is being read, giving the
  "operation stopped when reading edid" error.

Btw, this is in nitpicking area, but what editor do you use? I find it
difficult to read text that is not formatted properly =). At least vim
formats text nicely with its formating commands.

Indeed a nitpick :)
I use vim and, iirc, checkpatch.pl gave 0 warning. Perhaps my poor
cmoposition. Please do tell how I could I make it more appealing to
you ?

quoted

--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c

@@ -243,10 +243,13 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data *ip_data)

      hpd = gpio_get_value(ip_data->hpd_gpio);

-     if (hpd)
+     if (hpd) {
              r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
-     else
+     } else {
+             /* Invalidate EDID Cache */
+             ip_data->edid_len = 0;
              r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON);
+     }

There's a problem with this patch, which leaves a wrong EDID in the
cache: if you first have the cable connected and hdmi is enabled, you
then turn off the HDMI display device via sysfs, we do not go to
hdmi_check_hpd_state at all. The next time hdmi is enabled, we only get
the plug-in event, and thus EDID cache is never invalidated.

If the hdmi cable is not replugged during that period, I don't see why
would you want the EDID invalidated ?