On 25 June 2012 12:05, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
> On Sat, 2012-06-23 at 13:36 +0530, jaswinder.singh@xxxxxxxxxx wrote:
>> From: Jassi Brar <jaswinder.singh@xxxxxxxxxx>
>>
>> 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.
>>
>> Signed-off-by: Jassi Brar <jaswinder.singh@xxxxxxxxxx>
>> ---
>> drivers/video/omap2/dss/hdmi.c | 1 +
>> drivers/video/omap2/dss/ti_hdmi.h | 2 ++
>> drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 23 ++++++++++++++++++++---
>> 3 files changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
>> index 900e621..0a8c825 100644
>> --- a/drivers/video/omap2/dss/hdmi.c
>> +++ b/drivers/video/omap2/dss/hdmi.c
>> @@ -764,6 +764,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
>> hdmi.ip_data.core_av_offset = HDMI_CORE_AV;
>> hdmi.ip_data.pll_offset = HDMI_PLLCTRL;
>> hdmi.ip_data.phy_offset = HDMI_PHY;
>> + hdmi.ip_data.edid_len = 0; /* Invalidate EDID Cache */
>> mutex_init(&hdmi.ip_data.lock);
>
> Your HDMI patches seem to depend on each other. Please post them as a
> proper patch series, instead of each one separately.
>
They don't depend functionality wise. Any fix can be accepted
regardless of others.
I deliberately avoided a series, because revision of just one could
require resending 3, otherwise
perfectly OK, patches. I just wanted to limit the noise.
I understand, 'git am' might complain but I think that should be trivial to fix.
I am perfectly OK to resend as a patch series, if you want.
>> hdmi_panel_init();
>> diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
>> index cc292b8..4735860 100644
>> --- a/drivers/video/omap2/dss/ti_hdmi.h
>> +++ b/drivers/video/omap2/dss/ti_hdmi.h
>> @@ -178,6 +178,8 @@ struct hdmi_ip_data {
>> /* ti_hdmi_4xxx_ip private data. These should be in a separate struct */
>> int hpd_gpio;
>> struct mutex lock;
>> + u8 edid_cached[256];
>> + unsigned edid_len;
>> };
>> int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
>> void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
>> diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
>> index 04acca9..2633ade 100644
>> --- 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);
>> + }
>>
>> if (r) {
>> DSSERR("Failed to %s PHY TX power\n",
>> @@ -454,6 +457,11 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
>> {
>> int r, l;
>>
>> + if (ip_data->edid_len) {
>> + memcpy(edid, ip_data->edid_cached, ip_data->edid_len);
>> + return ip_data->edid_len;
>> + }
>> +
>> if (len < 128)
>> return -EINVAL;
>>
>> @@ -474,12 +482,21 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
>> l += 128;
>> }
>>
>> + ip_data->edid_len = l;
>> + memcpy(ip_data->edid_cached, edid, l);
>> +
>> return l;
>> }
>>
>> 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?
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'.
Thanks.
--
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
