Re: [PATCH] OMAPDSS: HDMI: Cache EDID

[Jassi Brar <jaswinder.singh@linaro.org>]

On 25 June 2012 13:41, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
>> I am perfectly OK to resend as a patch series, if you want.
>
> Yes please.
>
OK, will do.

>> >>  bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
>> >>  {
>> >> -     return gpio_get_value(ip_data->hpd_gpio);
>> >> +     if (gpio_get_value(ip_data->hpd_gpio))
>> >> +             return true;
>> >> +
>> >> +     /* Invalidate EDID Cache */
>> >> +     ip_data->edid_len = 0;
>> >> +
>> >> +     return false;
>> >
>> > Why is this needed? The HPD interrupt should handle this already. And if
>> > the HPD interrupt doesn't work for some reason, this won't work either,
>> > as the user can plug and unplug his HDMI monitors a thousand times
>> > between two detect calls.
>> >
>> I thought it is almost impossible to unplug->plug cycle the HDMI cable
>> even twice a second, let alone 1000 times against the 10Hz .detect()
>> poll  :)    Or you mean some relay controlled HDMI switching
>> mechanism?
>
> omapdss doesn't call the detect function, so it can't presume it's used
> in some certain frequency. Also, last time I tested omapdrm, I think
> detect was called once in 5 secs or so.
>
It's not omapdss.  It's the DRM stack, via the omapdrm, that polls
every 10 secs (not 5).
Sorry I said 10Hz instead of 1/10Hz.

>>  Anyways, that is not the point of this patch. This patch only aims to
>> avoid un-ncessary EDID reads while doing its best to make sure we
>> invalidate EDID 'as soon as possible'.
>
> I'm not sure I understand your point. If the HPD interrupt works
> properly, EDID is invalidated there, and the code in detect is not
> needed. And if the HPD interrupt doesn't work (although I don't see why
> it wouldn't), the code in detect doesn't work. In either case it's not
> correct.
>
Well, the idea was to tie edid-cache invalidating with de-asserted
HPD, wherever we read HPD. I will drop it.

thnx