On 28 June 2012 15:44, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
> On Thu, 2012-06-28 at 15:18 +0530, Jassi Brar wrote:
>> >> --- 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 ?
>
> I wasn't very clear with my comment.
>
> 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.
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
