On 28 June 2012 16:17, Jassi Brar <jaswinder.singh@xxxxxxxxxx> wrote:
> 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_4_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.
>
Sorry a correction. Reading detect() won't work. I suggest we keep HPD
IRQ enabled for the lifetime of the driver.
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
