* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 11:10 ` Tomi Valkeinen
0 siblings, 0 replies; 44+ messages in thread
From: Tomi Valkeinen @ 2012-06-28 11:10 UTC (permalink / raw)
To: Jassi Brar
Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne, patches
[-- Attachment #1: Type: text/plain, Size: 3118 bytes --]
On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:
> On 28 June 2012 16:17, Jassi Brar <jaswinder.singh@linaro.org> wrote:
> > On 28 June 2012 15:44, Tomi Valkeinen <tomi.valkeinen@ti.com> 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.
Ok, I see. But that's not acceptable. It would require us to keep the
TPD12S015 always powered and enabled. Even if you're not interested in
using HDMI at all.
For this to work like you want we need a bigger restructuring of HDMI
and partly omapdss also. Currently we have just one big "enabled" or
"disabled" state. We need multiple states. Probably something like:
- disabled, everything totally off
- low power, hotplug detection enabled
- powered on, but no video
- video enabled
Been long in my mind, but I'm not very familiar with HDMI so I could get
my simple prototypes to work when I tried something like this once.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 11:10 ` Tomi Valkeinen
@ 2012-06-28 11:38 ` Tomi Valkeinen
-1 siblings, 0 replies; 44+ messages in thread
From: Tomi Valkeinen @ 2012-06-28 11:38 UTC (permalink / raw)
To: Jassi Brar
Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne, patches
[-- Attachment #1: Type: text/plain, Size: 921 bytes --]
On Thu, 2012-06-28 at 14:10 +0300, Tomi Valkeinen wrote:
> On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:
> > Sorry a correction. Reading detect() won't work. I suggest we keep HPD
> > IRQ enabled for the lifetime of the driver.
>
> Ok, I see. But that's not acceptable. It would require us to keep the
> TPD12S015 always powered and enabled. Even if you're not interested in
> using HDMI at all.
Btw, a bigger problem that I see is how we have to do read_edid() (and
detect(), if I recall correctly): we enable the whole video pipeline and
output. We should only enable enough of the HW to be able to read the
EDID or read the HPD GPIO.
I've noticed that this leads to sync losts quite easily, as we switch
the hdmi output on and off quickly multiple times. I couldn't figure out
why the sync losts happen though, and I did try quite many different
combinations how to handle it.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 11:38 ` Tomi Valkeinen
0 siblings, 0 replies; 44+ messages in thread
From: Tomi Valkeinen @ 2012-06-28 11:38 UTC (permalink / raw)
To: Jassi Brar
Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne, patches
[-- Attachment #1: Type: text/plain, Size: 921 bytes --]
On Thu, 2012-06-28 at 14:10 +0300, Tomi Valkeinen wrote:
> On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:
> > Sorry a correction. Reading detect() won't work. I suggest we keep HPD
> > IRQ enabled for the lifetime of the driver.
>
> Ok, I see. But that's not acceptable. It would require us to keep the
> TPD12S015 always powered and enabled. Even if you're not interested in
> using HDMI at all.
Btw, a bigger problem that I see is how we have to do read_edid() (and
detect(), if I recall correctly): we enable the whole video pipeline and
output. We should only enable enough of the HW to be able to read the
EDID or read the HPD GPIO.
I've noticed that this leads to sync losts quite easily, as we switch
the hdmi output on and off quickly multiple times. I couldn't figure out
why the sync losts happen though, and I did try quite many different
combinations how to handle it.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 11:38 ` Tomi Valkeinen
@ 2012-06-28 12:15 ` Andy Green
-1 siblings, 0 replies; 44+ messages in thread
From: Andy Green @ 2012-06-28 12:15 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jassi Brar, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 06/28/12 19:38, the mail apparently from Tomi Valkeinen included:
> On Thu, 2012-06-28 at 14:10 +0300, Tomi Valkeinen wrote:
>> On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:
>
>>> Sorry a correction. Reading detect() won't work. I suggest we keep HPD
>>> IRQ enabled for the lifetime of the driver.
>>
>> Ok, I see. But that's not acceptable. It would require us to keep the
>> TPD12S015 always powered and enabled. Even if you're not interested in
>> using HDMI at all.
>
> Btw, a bigger problem that I see is how we have to do read_edid() (and
> detect(), if I recall correctly): we enable the whole video pipeline and
> output. We should only enable enough of the HW to be able to read the
> EDID or read the HPD GPIO.
>
> I've noticed that this leads to sync losts quite easily, as we switch
> the hdmi output on and off quickly multiple times. I couldn't figure out
> why the sync losts happen though, and I did try quite many different
> combinations how to handle it.
SYNC LOST is one evil lurking in there, the other evil is EDID fetch
"operation stopped" error. We were unable to figure out what was
trampling on what there without the SoC HDMI PHY IP data which we don't
have.
Also at the moment I think we depower / repower the internal SoC and
external PHY chip more than we need. Each time we remove the HDMI link
power, after a short time where the big capacitor at the charge pump
output decays enough (a time dependent on exact details of load
presented by the TV or monitor...), hpd from the monitor goes low and
remains there until we power the charge pump again. In turn the new hpd
recognition provokes a new edid fetch. Something about that sequence
provokes the "operation stopped" on EDID fetch, with Jassi's patches we
manage to avoid it.
Removing that syndrome, and just not enabling and disabling stuff like
SoC HDMI PHY willy nilly can maybe be something else targeted by this
proposed 4-state power scheme.
-Andy
--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106 -
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 12:15 ` Andy Green
0 siblings, 0 replies; 44+ messages in thread
From: Andy Green @ 2012-06-28 12:15 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jassi Brar, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 06/28/12 19:38, the mail apparently from Tomi Valkeinen included:
> On Thu, 2012-06-28 at 14:10 +0300, Tomi Valkeinen wrote:
>> On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:
>
>>> Sorry a correction. Reading detect() won't work. I suggest we keep HPD
>>> IRQ enabled for the lifetime of the driver.
>>
>> Ok, I see. But that's not acceptable. It would require us to keep the
>> TPD12S015 always powered and enabled. Even if you're not interested in
>> using HDMI at all.
>
> Btw, a bigger problem that I see is how we have to do read_edid() (and
> detect(), if I recall correctly): we enable the whole video pipeline and
> output. We should only enable enough of the HW to be able to read the
> EDID or read the HPD GPIO.
>
> I've noticed that this leads to sync losts quite easily, as we switch
> the hdmi output on and off quickly multiple times. I couldn't figure out
> why the sync losts happen though, and I did try quite many different
> combinations how to handle it.
SYNC LOST is one evil lurking in there, the other evil is EDID fetch
"operation stopped" error. We were unable to figure out what was
trampling on what there without the SoC HDMI PHY IP data which we don't
have.
Also at the moment I think we depower / repower the internal SoC and
external PHY chip more than we need. Each time we remove the HDMI link
power, after a short time where the big capacitor at the charge pump
output decays enough (a time dependent on exact details of load
presented by the TV or monitor...), hpd from the monitor goes low and
remains there until we power the charge pump again. In turn the new hpd
recognition provokes a new edid fetch. Something about that sequence
provokes the "operation stopped" on EDID fetch, with Jassi's patches we
manage to avoid it.
Removing that syndrome, and just not enabling and disabling stuff like
SoC HDMI PHY willy nilly can maybe be something else targeted by this
proposed 4-state power scheme.
-Andy
--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106 -
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 11:10 ` Tomi Valkeinen
@ 2012-06-28 12:03 ` Andy Green
-1 siblings, 0 replies; 44+ messages in thread
From: Andy Green @ 2012-06-28 12:03 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jassi Brar, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 06/28/12 19:10, the mail apparently from Tomi Valkeinen included:
> On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:
>> On 28 June 2012 16:17, Jassi Brar <jaswinder.singh@linaro.org> wrote:
>>> On 28 June 2012 15:44, Tomi Valkeinen <tomi.valkeinen@ti.com> 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.
>
> Ok, I see. But that's not acceptable. It would require us to keep the
> TPD12S015 always powered and enabled. Even if you're not interested in
> using HDMI at all.
>
> For this to work like you want we need a bigger restructuring of HDMI
> and partly omapdss also. Currently we have just one big "enabled" or
> "disabled" state. We need multiple states. Probably something like:
>
> - disabled, everything totally off
> - low power, hotplug detection enabled
> - powered on, but no video
> - video enabled
>
> Been long in my mind, but I'm not very familiar with HDMI so I could get
> my simple prototypes to work when I tried something like this once.
That doesn't sound too hard since the difference between the first three
states at the HDMI chip is just whether the two gpio controlling it are
00, 10 or 11.
If Jassi's alright with it we might have a go at implementing this, but
can you define a bit more about how we logically tell DSS that we want
to, eg, disable HDMI totally?
-Andy
--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106 -
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 12:03 ` Andy Green
0 siblings, 0 replies; 44+ messages in thread
From: Andy Green @ 2012-06-28 12:03 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jassi Brar, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 06/28/12 19:10, the mail apparently from Tomi Valkeinen included:
> On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:
>> On 28 June 2012 16:17, Jassi Brar <jaswinder.singh@linaro.org> wrote:
>>> On 28 June 2012 15:44, Tomi Valkeinen <tomi.valkeinen@ti.com> 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.
>
> Ok, I see. But that's not acceptable. It would require us to keep the
> TPD12S015 always powered and enabled. Even if you're not interested in
> using HDMI at all.
>
> For this to work like you want we need a bigger restructuring of HDMI
> and partly omapdss also. Currently we have just one big "enabled" or
> "disabled" state. We need multiple states. Probably something like:
>
> - disabled, everything totally off
> - low power, hotplug detection enabled
> - powered on, but no video
> - video enabled
>
> Been long in my mind, but I'm not very familiar with HDMI so I could get
> my simple prototypes to work when I tried something like this once.
That doesn't sound too hard since the difference between the first three
states at the HDMI chip is just whether the two gpio controlling it are
00, 10 or 11.
If Jassi's alright with it we might have a go at implementing this, but
can you define a bit more about how we logically tell DSS that we want
to, eg, disable HDMI totally?
-Andy
--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106 -
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 12:03 ` Andy Green
@ 2012-06-28 13:08 ` Tomi Valkeinen
-1 siblings, 0 replies; 44+ messages in thread
From: Tomi Valkeinen @ 2012-06-28 13:08 UTC (permalink / raw)
To: Andy Green
Cc: Jassi Brar, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
[-- Attachment #1: Type: text/plain, Size: 2000 bytes --]
On Thu, 2012-06-28 at 20:03 +0800, Andy Green wrote:
> On 06/28/12 19:10, the mail apparently from Tomi Valkeinen included:
> > For this to work like you want we need a bigger restructuring of HDMI
> > and partly omapdss also. Currently we have just one big "enabled" or
> > "disabled" state. We need multiple states. Probably something like:
> >
> > - disabled, everything totally off
> > - low power, hotplug detection enabled
> > - powered on, but no video
> > - video enabled
> >
> > Been long in my mind, but I'm not very familiar with HDMI so I could get
> > my simple prototypes to work when I tried something like this once.
>
> That doesn't sound too hard since the difference between the first three
> states at the HDMI chip is just whether the two gpio controlling it are
> 00, 10 or 11.
I don't think it's that simple. We should be able to do EDID read on one
of the states, perhaps second or third. For that we need parts of the
HDMI IP to be enabled.
Also, the third state should be something where the IP is fully
functional. For DSI this would mean that you can communicate over DSI
bus etc. So I guess EDID read should be possible at this level.
> If Jassi's alright with it we might have a go at implementing this, but
> can you define a bit more about how we logically tell DSS that we want
> to, eg, disable HDMI totally?
Disabling HDMI is easy, it's already done by the disable call. And the
same for the video enabled mode. The middle ones are the ones that need
implementation.
And for HDMI, there's currently no way to enable it partially. This is
what I tried with my quick hack, but I couldn't figure out what parts
need to be enabled (but I didn't spend much time on it).
This is something that should be implemented for all outputs, obviously,
but we could approach it bit by bit. For example, implement it first for
HDMI, and all other outputs just consider anything else than "disabled"
as full enable.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 13:08 ` Tomi Valkeinen
0 siblings, 0 replies; 44+ messages in thread
From: Tomi Valkeinen @ 2012-06-28 13:08 UTC (permalink / raw)
To: Andy Green
Cc: Jassi Brar, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
[-- Attachment #1: Type: text/plain, Size: 2000 bytes --]
On Thu, 2012-06-28 at 20:03 +0800, Andy Green wrote:
> On 06/28/12 19:10, the mail apparently from Tomi Valkeinen included:
> > For this to work like you want we need a bigger restructuring of HDMI
> > and partly omapdss also. Currently we have just one big "enabled" or
> > "disabled" state. We need multiple states. Probably something like:
> >
> > - disabled, everything totally off
> > - low power, hotplug detection enabled
> > - powered on, but no video
> > - video enabled
> >
> > Been long in my mind, but I'm not very familiar with HDMI so I could get
> > my simple prototypes to work when I tried something like this once.
>
> That doesn't sound too hard since the difference between the first three
> states at the HDMI chip is just whether the two gpio controlling it are
> 00, 10 or 11.
I don't think it's that simple. We should be able to do EDID read on one
of the states, perhaps second or third. For that we need parts of the
HDMI IP to be enabled.
Also, the third state should be something where the IP is fully
functional. For DSI this would mean that you can communicate over DSI
bus etc. So I guess EDID read should be possible at this level.
> If Jassi's alright with it we might have a go at implementing this, but
> can you define a bit more about how we logically tell DSS that we want
> to, eg, disable HDMI totally?
Disabling HDMI is easy, it's already done by the disable call. And the
same for the video enabled mode. The middle ones are the ones that need
implementation.
And for HDMI, there's currently no way to enable it partially. This is
what I tried with my quick hack, but I couldn't figure out what parts
need to be enabled (but I didn't spend much time on it).
This is something that should be implemented for all outputs, obviously,
but we could approach it bit by bit. For example, implement it first for
HDMI, and all other outputs just consider anything else than "disabled"
as full enable.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 12:03 ` Andy Green
@ 2012-06-28 13:25 ` Jassi Brar
-1 siblings, 0 replies; 44+ messages in thread
From: Jassi Brar @ 2012-06-28 13:13 UTC (permalink / raw)
To: Andy Green
Cc: Tomi Valkeinen, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 28 June 2012 17:33, Andy Green <andy.green@linaro.org> wrote:
> On 06/28/12 19:10, the mail apparently from Tomi Valkeinen included:
>
>> On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:
>>>
>>> On 28 June 2012 16:17, Jassi Brar <jaswinder.singh@linaro.org> wrote:
>>>>
>>>> On 28 June 2012 15:44, Tomi Valkeinen <tomi.valkeinen@ti.com> 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.
>>
>>
>> Ok, I see. But that's not acceptable. It would require us to keep the
>> TPD12S015 always powered and enabled. Even if you're not interested in
>> using HDMI at all.
>>
>> For this to work like you want we need a bigger restructuring of HDMI
>> and partly omapdss also. Currently we have just one big "enabled" or
>> "disabled" state. We need multiple states. Probably something like:
>>
>> - disabled, everything totally off
>> - low power, hotplug detection enabled
>> - powered on, but no video
>> - video enabled
>>
>> Been long in my mind, but I'm not very familiar with HDMI so I could get
>> my simple prototypes to work when I tried something like this once.
>
>
> That doesn't sound too hard since the difference between the first three
> states at the HDMI chip is just whether the two gpio controlling it are 00,
> 10 or 11.
>
> If Jassi's alright with it we might have a go at implementing this, but can
> you define a bit more about how we logically tell DSS that we want to, eg,
> disable HDMI totally?
>
A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
probe and disable during remove.
HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
HDMI_PHY on/off.
The user selecting "Autodetect and Configure" option would then equate
to "(un)loading" of the HDMI driver.
Not to mean a trivial job.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 13:25 ` Jassi Brar
0 siblings, 0 replies; 44+ messages in thread
From: Jassi Brar @ 2012-06-28 13:25 UTC (permalink / raw)
To: Andy Green
Cc: Tomi Valkeinen, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 28 June 2012 17:33, Andy Green <andy.green@linaro.org> wrote:
> On 06/28/12 19:10, the mail apparently from Tomi Valkeinen included:
>
>> On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:
>>>
>>> On 28 June 2012 16:17, Jassi Brar <jaswinder.singh@linaro.org> wrote:
>>>>
>>>> On 28 June 2012 15:44, Tomi Valkeinen <tomi.valkeinen@ti.com> 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.
>>
>>
>> Ok, I see. But that's not acceptable. It would require us to keep the
>> TPD12S015 always powered and enabled. Even if you're not interested in
>> using HDMI at all.
>>
>> For this to work like you want we need a bigger restructuring of HDMI
>> and partly omapdss also. Currently we have just one big "enabled" or
>> "disabled" state. We need multiple states. Probably something like:
>>
>> - disabled, everything totally off
>> - low power, hotplug detection enabled
>> - powered on, but no video
>> - video enabled
>>
>> Been long in my mind, but I'm not very familiar with HDMI so I could get
>> my simple prototypes to work when I tried something like this once.
>
>
> That doesn't sound too hard since the difference between the first three
> states at the HDMI chip is just whether the two gpio controlling it are 00,
> 10 or 11.
>
> If Jassi's alright with it we might have a go at implementing this, but can
> you define a bit more about how we logically tell DSS that we want to, eg,
> disable HDMI totally?
>
A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
probe and disable during remove.
HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
HDMI_PHY on/off.
The user selecting "Autodetect and Configure" option would then equate
to "(un)loading" of the HDMI driver.
Not to mean a trivial job.
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 13:25 ` Jassi Brar
@ 2012-06-28 13:31 ` Tomi Valkeinen
-1 siblings, 0 replies; 44+ messages in thread
From: Tomi Valkeinen @ 2012-06-28 13:31 UTC (permalink / raw)
To: Jassi Brar
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]
On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:
> On 28 June 2012 17:33, Andy Green <andy.green@linaro.org> wrote:
> > If Jassi's alright with it we might have a go at implementing this, but can
> > you define a bit more about how we logically tell DSS that we want to, eg,
> > disable HDMI totally?
> >
> A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
> probe and disable during remove.
The problem with this is a feature of omapdss: we can have multiple
displays for the same output, of which only one can be enabled at the
same time. What this means is that you shouldn't (and in some cases
can't) allocate or enable resources in probe that may be shared, because
then the driver for both displays would try to allocate the same
resource.
Sure, this is not a problem for the HDMI configuration we are using now,
but it's still against the panel model we have. Thus we should allocate
resources only when the panel device is turned on, and release them when
it's disabled.
I do think the model is slightly broken, but that's what we have now.
And I'm also not even sure how it should be fixed...
And also, as I said earlier, if you keep it enabled all the time, it'll
eat power even if the user is never going to use HDMI.
On a desktop I guess the power consumption wouldn't be an issue, but I
do feel a bit uneasy about it on an embedded device.
> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
> HDMI_PHY on/off.
> The user selecting "Autodetect and Configure" option would then equate
> to "(un)loading" of the HDMI driver.
HDMI cannot be currently compiled as a separate module. Although I think
you can detach a device and a driver, achieving the same. Is that what
you meant with unloading?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 13:31 ` Tomi Valkeinen
0 siblings, 0 replies; 44+ messages in thread
From: Tomi Valkeinen @ 2012-06-28 13:31 UTC (permalink / raw)
To: Jassi Brar
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]
On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:
> On 28 June 2012 17:33, Andy Green <andy.green@linaro.org> wrote:
> > If Jassi's alright with it we might have a go at implementing this, but can
> > you define a bit more about how we logically tell DSS that we want to, eg,
> > disable HDMI totally?
> >
> A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
> probe and disable during remove.
The problem with this is a feature of omapdss: we can have multiple
displays for the same output, of which only one can be enabled at the
same time. What this means is that you shouldn't (and in some cases
can't) allocate or enable resources in probe that may be shared, because
then the driver for both displays would try to allocate the same
resource.
Sure, this is not a problem for the HDMI configuration we are using now,
but it's still against the panel model we have. Thus we should allocate
resources only when the panel device is turned on, and release them when
it's disabled.
I do think the model is slightly broken, but that's what we have now.
And I'm also not even sure how it should be fixed...
And also, as I said earlier, if you keep it enabled all the time, it'll
eat power even if the user is never going to use HDMI.
On a desktop I guess the power consumption wouldn't be an issue, but I
do feel a bit uneasy about it on an embedded device.
> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
> HDMI_PHY on/off.
> The user selecting "Autodetect and Configure" option would then equate
> to "(un)loading" of the HDMI driver.
HDMI cannot be currently compiled as a separate module. Although I think
you can detach a device and a driver, achieving the same. Is that what
you meant with unloading?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 13:31 ` Tomi Valkeinen
@ 2012-06-28 15:26 ` Jassi Brar
-1 siblings, 0 replies; 44+ messages in thread
From: Jassi Brar @ 2012-06-28 15:14 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 28 June 2012 19:01, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:
>> On 28 June 2012 17:33, Andy Green <andy.green@linaro.org> wrote:
>
>> > If Jassi's alright with it we might have a go at implementing this, but can
>> > you define a bit more about how we logically tell DSS that we want to, eg,
>> > disable HDMI totally?
>> >
>> A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
>> probe and disable during remove.
>
> The problem with this is a feature of omapdss: we can have multiple
> displays for the same output, of which only one can be enabled at the
> same time. What this means is that you shouldn't (and in some cases
> can't) allocate or enable resources in probe that may be shared, because
> then the driver for both displays would try to allocate the same
> resource.
>
> Sure, this is not a problem for the HDMI configuration we are using now,
> but it's still against the panel model we have. Thus we should allocate
> resources only when the panel device is turned on, and release them when
> it's disabled.
>
> I do think the model is slightly broken, but that's what we have now.
> And I'm also not even sure how it should be fixed...
>
I won't press further with my Utopian ideas, but I think we need to
segregate 5V/HPD enabling from PHY enabling somehow. Because that is
already failing slow but otherwise perfectly legit displays (like
Andy's "HPD taking 700ms" TV)
> And also, as I said earlier, if you keep it enabled all the time, it'll
> eat power even if the user is never going to use HDMI.
>
> On a desktop I guess the power consumption wouldn't be an issue, but I
> do feel a bit uneasy about it on an embedded device.
>
As I said, it should be platform dependent. If a device doesn't have
HDMI port, the board file would not even have platform_enable. And if
it has, some user action should enable it while 'making the device
ready for new display'.
IOW, how do you envision an OMAP4 based tablet with HDMI port react to
display connections ?
>> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
>> HDMI_PHY on/off.
>> The user selecting "Autodetect and Configure" option would then equate
>> to "(un)loading" of the HDMI driver.
>
> HDMI cannot be currently compiled as a separate module. Although I think
> you can detach a device and a driver, achieving the same. Is that what
> you meant with unloading?
>
Yeah, I meant something to the effect of bringing HDMI driver to life.
> By the way, when the device is in system suspend, we surely won't detect
> the HPD even if we kept the HPD always enabled. So there we'll miss the
> HPD interrupt anyway, and the EDID cache would be invalid.
>
If omapdss already handles the possibility of display changed during
suspend, I think we should be good :)
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 15:26 ` Jassi Brar
0 siblings, 0 replies; 44+ messages in thread
From: Jassi Brar @ 2012-06-28 15:26 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 28 June 2012 19:01, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:
>> On 28 June 2012 17:33, Andy Green <andy.green@linaro.org> wrote:
>
>> > If Jassi's alright with it we might have a go at implementing this, but can
>> > you define a bit more about how we logically tell DSS that we want to, eg,
>> > disable HDMI totally?
>> >
>> A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
>> probe and disable during remove.
>
> The problem with this is a feature of omapdss: we can have multiple
> displays for the same output, of which only one can be enabled at the
> same time. What this means is that you shouldn't (and in some cases
> can't) allocate or enable resources in probe that may be shared, because
> then the driver for both displays would try to allocate the same
> resource.
>
> Sure, this is not a problem for the HDMI configuration we are using now,
> but it's still against the panel model we have. Thus we should allocate
> resources only when the panel device is turned on, and release them when
> it's disabled.
>
> I do think the model is slightly broken, but that's what we have now.
> And I'm also not even sure how it should be fixed...
>
I won't press further with my Utopian ideas, but I think we need to
segregate 5V/HPD enabling from PHY enabling somehow. Because that is
already failing slow but otherwise perfectly legit displays (like
Andy's "HPD taking 700ms" TV)
> And also, as I said earlier, if you keep it enabled all the time, it'll
> eat power even if the user is never going to use HDMI.
>
> On a desktop I guess the power consumption wouldn't be an issue, but I
> do feel a bit uneasy about it on an embedded device.
>
As I said, it should be platform dependent. If a device doesn't have
HDMI port, the board file would not even have platform_enable. And if
it has, some user action should enable it while 'making the device
ready for new display'.
IOW, how do you envision an OMAP4 based tablet with HDMI port react to
display connections ?
>> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
>> HDMI_PHY on/off.
>> The user selecting "Autodetect and Configure" option would then equate
>> to "(un)loading" of the HDMI driver.
>
> HDMI cannot be currently compiled as a separate module. Although I think
> you can detach a device and a driver, achieving the same. Is that what
> you meant with unloading?
>
Yeah, I meant something to the effect of bringing HDMI driver to life.
> By the way, when the device is in system suspend, we surely won't detect
> the HPD even if we kept the HPD always enabled. So there we'll miss the
> HPD interrupt anyway, and the EDID cache would be invalid.
>
If omapdss already handles the possibility of display changed during
suspend, I think we should be good :)
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 15:26 ` Jassi Brar
@ 2012-06-28 15:27 ` Tomi Valkeinen
-1 siblings, 0 replies; 44+ messages in thread
From: Tomi Valkeinen @ 2012-06-28 15:27 UTC (permalink / raw)
To: Jassi Brar
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
[-- Attachment #1: Type: text/plain, Size: 2853 bytes --]
On Thu, 2012-06-28 at 20:44 +0530, Jassi Brar wrote:
> I won't press further with my Utopian ideas, but I think we need to
> segregate 5V/HPD enabling from PHY enabling somehow. Because that is
> already failing slow but otherwise perfectly legit displays (like
> Andy's "HPD taking 700ms" TV)
Yes, I agree. That's what the four power states I suggested in the other
mail were about. The second power state would have HPD enabled, and the
third would enable the PHY. Or at least enough to do i2c transfers, I'm
not sure if the PHY is needed for that.
> > And also, as I said earlier, if you keep it enabled all the time, it'll
> > eat power even if the user is never going to use HDMI.
> >
> > On a desktop I guess the power consumption wouldn't be an issue, but I
> > do feel a bit uneasy about it on an embedded device.
> >
> As I said, it should be platform dependent. If a device doesn't have
> HDMI port, the board file would not even have platform_enable. And if
Not that it's relevant here, but I have a patch series where I remove
the platform_enable stuff for HDMI, and move the work to the hdmi
driver. That needs to be done for device tree stuff, when we don't have
board files.
> it has, some user action should enable it while 'making the device
> ready for new display'.
> IOW, how do you envision an OMAP4 based tablet with HDMI port react to
> display connections ?
I guess this was covered in my mail about the phone's HDMI. If the
tablet is unlocked, and I plug in a HDMI cable, I expect the device to
do something. Either clone the display, or perhaps ask me what I want to
do.
So yes, HPD would be always enabled, when the tablet is active
(unlocked).
> >> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
> >> HDMI_PHY on/off.
> >> The user selecting "Autodetect and Configure" option would then equate
> >> to "(un)loading" of the HDMI driver.
> >
> > HDMI cannot be currently compiled as a separate module. Although I think
> > you can detach a device and a driver, achieving the same. Is that what
> > you meant with unloading?
> >
> Yeah, I meant something to the effect of bringing HDMI driver to life.
>
>
> > By the way, when the device is in system suspend, we surely won't detect
> > the HPD even if we kept the HPD always enabled. So there we'll miss the
> > HPD interrupt anyway, and the EDID cache would be invalid.
> >
> If omapdss already handles the possibility of display changed during
> suspend, I think we should be good :)
Hmm I'm not sure I understand what you mean. I was referring to your
patch, which invalidated the EDID cache only on HPD interrupt when the
cable is unplugged. And we'd miss that interrupt when the board is in
system suspend, even if we otherwise kept the HPD interrupt always
enabled.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 15:27 ` Tomi Valkeinen
0 siblings, 0 replies; 44+ messages in thread
From: Tomi Valkeinen @ 2012-06-28 15:27 UTC (permalink / raw)
To: Jassi Brar
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
[-- Attachment #1: Type: text/plain, Size: 2853 bytes --]
On Thu, 2012-06-28 at 20:44 +0530, Jassi Brar wrote:
> I won't press further with my Utopian ideas, but I think we need to
> segregate 5V/HPD enabling from PHY enabling somehow. Because that is
> already failing slow but otherwise perfectly legit displays (like
> Andy's "HPD taking 700ms" TV)
Yes, I agree. That's what the four power states I suggested in the other
mail were about. The second power state would have HPD enabled, and the
third would enable the PHY. Or at least enough to do i2c transfers, I'm
not sure if the PHY is needed for that.
> > And also, as I said earlier, if you keep it enabled all the time, it'll
> > eat power even if the user is never going to use HDMI.
> >
> > On a desktop I guess the power consumption wouldn't be an issue, but I
> > do feel a bit uneasy about it on an embedded device.
> >
> As I said, it should be platform dependent. If a device doesn't have
> HDMI port, the board file would not even have platform_enable. And if
Not that it's relevant here, but I have a patch series where I remove
the platform_enable stuff for HDMI, and move the work to the hdmi
driver. That needs to be done for device tree stuff, when we don't have
board files.
> it has, some user action should enable it while 'making the device
> ready for new display'.
> IOW, how do you envision an OMAP4 based tablet with HDMI port react to
> display connections ?
I guess this was covered in my mail about the phone's HDMI. If the
tablet is unlocked, and I plug in a HDMI cable, I expect the device to
do something. Either clone the display, or perhaps ask me what I want to
do.
So yes, HPD would be always enabled, when the tablet is active
(unlocked).
> >> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
> >> HDMI_PHY on/off.
> >> The user selecting "Autodetect and Configure" option would then equate
> >> to "(un)loading" of the HDMI driver.
> >
> > HDMI cannot be currently compiled as a separate module. Although I think
> > you can detach a device and a driver, achieving the same. Is that what
> > you meant with unloading?
> >
> Yeah, I meant something to the effect of bringing HDMI driver to life.
>
>
> > By the way, when the device is in system suspend, we surely won't detect
> > the HPD even if we kept the HPD always enabled. So there we'll miss the
> > HPD interrupt anyway, and the EDID cache would be invalid.
> >
> If omapdss already handles the possibility of display changed during
> suspend, I think we should be good :)
Hmm I'm not sure I understand what you mean. I was referring to your
patch, which invalidated the EDID cache only on HPD interrupt when the
cable is unplugged. And we'd miss that interrupt when the board is in
system suspend, even if we otherwise kept the HPD interrupt always
enabled.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 15:27 ` Tomi Valkeinen
@ 2012-06-28 15:51 ` Jassi Brar
-1 siblings, 0 replies; 44+ messages in thread
From: Jassi Brar @ 2012-06-28 15:48 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 28 June 2012 20:57, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-06-28 at 20:44 +0530, Jassi Brar wrote:
>> it has, some user action should enable it while 'making the device
>> ready for new display'.
>> IOW, how do you envision an OMAP4 based tablet with HDMI port react to
>> display connections ?
>
> I guess this was covered in my mail about the phone's HDMI. If the
> tablet is unlocked, and I plug in a HDMI cable, I expect the device to
> do something. Either clone the display, or perhaps ask me what I want to
> do.
>
> So yes, HPD would be always enabled, when the tablet is active
> (unlocked).
>
OK, somehow I was under impression you didn't wanna spare even the 5V+ floating.
Though there could also be some option in settings to enable 5/HPD
only when the user is about the connect a display... so that the
activate window is even narrowed down. Anyway... I am glad we are in
sync.
>> > By the way, when the device is in system suspend, we surely won't detect
>> > the HPD even if we kept the HPD always enabled. So there we'll miss the
>> > HPD interrupt anyway, and the EDID cache would be invalid.
>> >
>> If omapdss already handles the possibility of display changed during
>> suspend, I think we should be good :)
>
> Hmm I'm not sure I understand what you mean. I was referring to your
> patch, which invalidated the EDID cache only on HPD interrupt when the
> cable is unplugged. And we'd miss that interrupt when the board is in
> system suspend, even if we otherwise kept the HPD interrupt always
> enabled.
>
I meant before stale-edid, we face potential problem of omapdss
behaving badly to the displays switched during suspend ?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 15:51 ` Jassi Brar
0 siblings, 0 replies; 44+ messages in thread
From: Jassi Brar @ 2012-06-28 15:51 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 28 June 2012 20:57, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-06-28 at 20:44 +0530, Jassi Brar wrote:
>> it has, some user action should enable it while 'making the device
>> ready for new display'.
>> IOW, how do you envision an OMAP4 based tablet with HDMI port react to
>> display connections ?
>
> I guess this was covered in my mail about the phone's HDMI. If the
> tablet is unlocked, and I plug in a HDMI cable, I expect the device to
> do something. Either clone the display, or perhaps ask me what I want to
> do.
>
> So yes, HPD would be always enabled, when the tablet is active
> (unlocked).
>
OK, somehow I was under impression you didn't wanna spare even the 5V+ floating.
Though there could also be some option in settings to enable 5/HPD
only when the user is about the connect a display... so that the
activate window is even narrowed down. Anyway... I am glad we are in
sync.
>> > By the way, when the device is in system suspend, we surely won't detect
>> > the HPD even if we kept the HPD always enabled. So there we'll miss the
>> > HPD interrupt anyway, and the EDID cache would be invalid.
>> >
>> If omapdss already handles the possibility of display changed during
>> suspend, I think we should be good :)
>
> Hmm I'm not sure I understand what you mean. I was referring to your
> patch, which invalidated the EDID cache only on HPD interrupt when the
> cable is unplugged. And we'd miss that interrupt when the board is in
> system suspend, even if we otherwise kept the HPD interrupt always
> enabled.
>
I meant before stale-edid, we face potential problem of omapdss
behaving badly to the displays switched during suspend ?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 15:51 ` Jassi Brar
@ 2012-06-28 16:32 ` Jassi Brar
-1 siblings, 0 replies; 44+ messages in thread
From: Jassi Brar @ 2012-06-28 16:20 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 28 June 2012 21:18, Jassi Brar <jaswinder.singh@linaro.org> wrote:
> On 28 June 2012 20:57, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>> > By the way, when the device is in system suspend, we surely won't detect
>>> > the HPD even if we kept the HPD always enabled. So there we'll miss the
>>> > HPD interrupt anyway, and the EDID cache would be invalid.
>>> >
>>> If omapdss already handles the possibility of display changed during
>>> suspend, I think we should be good :)
>>
>> Hmm I'm not sure I understand what you mean. I was referring to your
>> patch, which invalidated the EDID cache only on HPD interrupt when the
>> cable is unplugged. And we'd miss that interrupt when the board is in
>> system suspend, even if we otherwise kept the HPD interrupt always
>> enabled.
>>
> I meant before stale-edid, we face potential problem of omapdss
> behaving badly to the displays switched during suspend ?
>
OK, I think I get now what you mean. We do need to invalidate
edid-cache in the suspend path, irrespective of how omapdss behaves.
Thanks.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 16:32 ` Jassi Brar
0 siblings, 0 replies; 44+ messages in thread
From: Jassi Brar @ 2012-06-28 16:32 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 28 June 2012 21:18, Jassi Brar <jaswinder.singh@linaro.org> wrote:
> On 28 June 2012 20:57, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>> > By the way, when the device is in system suspend, we surely won't detect
>>> > the HPD even if we kept the HPD always enabled. So there we'll miss the
>>> > HPD interrupt anyway, and the EDID cache would be invalid.
>>> >
>>> If omapdss already handles the possibility of display changed during
>>> suspend, I think we should be good :)
>>
>> Hmm I'm not sure I understand what you mean. I was referring to your
>> patch, which invalidated the EDID cache only on HPD interrupt when the
>> cable is unplugged. And we'd miss that interrupt when the board is in
>> system suspend, even if we otherwise kept the HPD interrupt always
>> enabled.
>>
> I meant before stale-edid, we face potential problem of omapdss
> behaving badly to the displays switched during suspend ?
>
OK, I think I get now what you mean. We do need to invalidate
edid-cache in the suspend path, irrespective of how omapdss behaves.
Thanks.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 13:25 ` Jassi Brar
@ 2012-06-28 15:14 ` Tomi Valkeinen
-1 siblings, 0 replies; 44+ messages in thread
From: Tomi Valkeinen @ 2012-06-28 15:14 UTC (permalink / raw)
To: Jassi Brar
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]
On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:
> A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
> probe and disable during remove.
> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
> HDMI_PHY on/off.
> The user selecting "Autodetect and Configure" option would then equate
> to "(un)loading" of the HDMI driver.
> Not to mean a trivial job.
One more thing I realized while thinking about this:
While it could be argued that the power draw from having the tpd12s015
always enabled is very small, I think it could matter. If you consider a
phone with HDMI output, it's likely that the phone is locked 99% of the
time. When the phone is locked, there's no need to keep the HDMI HPD
enabled. So this could add to a considerable amount of power wasted, if
the HPD was always enabled.
At least I can't figure out a reason why one would want the HPD to work
when the phone is locked. Also, I have never used the HDMI output on my
phone, so I'd be glad if it was totally powered off if it gave me more
standby hours =).
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 15:14 ` Tomi Valkeinen
0 siblings, 0 replies; 44+ messages in thread
From: Tomi Valkeinen @ 2012-06-28 15:14 UTC (permalink / raw)
To: Jassi Brar
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]
On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:
> A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
> probe and disable during remove.
> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
> HDMI_PHY on/off.
> The user selecting "Autodetect and Configure" option would then equate
> to "(un)loading" of the HDMI driver.
> Not to mean a trivial job.
One more thing I realized while thinking about this:
While it could be argued that the power draw from having the tpd12s015
always enabled is very small, I think it could matter. If you consider a
phone with HDMI output, it's likely that the phone is locked 99% of the
time. When the phone is locked, there's no need to keep the HDMI HPD
enabled. So this could add to a considerable amount of power wasted, if
the HPD was always enabled.
At least I can't figure out a reason why one would want the HPD to work
when the phone is locked. Also, I have never used the HDMI output on my
phone, so I'd be glad if it was totally powered off if it gave me more
standby hours =).
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 15:14 ` Tomi Valkeinen
@ 2012-06-28 15:30 ` Jassi Brar
-1 siblings, 0 replies; 44+ messages in thread
From: Jassi Brar @ 2012-06-28 15:18 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 28 June 2012 20:44, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:
>
>> A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
>> probe and disable during remove.
>> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
>> HDMI_PHY on/off.
>> The user selecting "Autodetect and Configure" option would then equate
>> to "(un)loading" of the HDMI driver.
>> Not to mean a trivial job.
>
> One more thing I realized while thinking about this:
>
> While it could be argued that the power draw from having the tpd12s015
> always enabled is very small, I think it could matter. If you consider a
> phone with HDMI output, it's likely that the phone is locked 99% of the
> time. When the phone is locked, there's no need to keep the HDMI HPD
> enabled. So this could add to a considerable amount of power wasted, if
> the HPD was always enabled.
>
> At least I can't figure out a reason why one would want the HPD to work
> when the phone is locked. Also, I have never used the HDMI output on my
> phone, so I'd be glad if it was totally powered off if it gave me more
> standby hours =).
>
Of course, I don't suggest imposing any hard rule here.
All I suggest is make it platform dependent and provide a way from
user-space too to enable/disable HPD.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 15:30 ` Jassi Brar
0 siblings, 0 replies; 44+ messages in thread
From: Jassi Brar @ 2012-06-28 15:30 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Andy Green, mythripk, linux-omap, linux-fbdev, n-dechesne, patches
On 28 June 2012 20:44, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:
>
>> A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
>> probe and disable during remove.
>> HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
>> HDMI_PHY on/off.
>> The user selecting "Autodetect and Configure" option would then equate
>> to "(un)loading" of the HDMI driver.
>> Not to mean a trivial job.
>
> One more thing I realized while thinking about this:
>
> While it could be argued that the power draw from having the tpd12s015
> always enabled is very small, I think it could matter. If you consider a
> phone with HDMI output, it's likely that the phone is locked 99% of the
> time. When the phone is locked, there's no need to keep the HDMI HPD
> enabled. So this could add to a considerable amount of power wasted, if
> the HPD was always enabled.
>
> At least I can't figure out a reason why one would want the HPD to work
> when the phone is locked. Also, I have never used the HDMI output on my
> phone, so I'd be glad if it was totally powered off if it gave me more
> standby hours =).
>
Of course, I don't suggest imposing any hard rule here.
All I suggest is make it platform dependent and provide a way from
user-space too to enable/disable HPD.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
2012-06-28 11:10 ` Tomi Valkeinen
@ 2012-06-28 12:43 ` Jassi Brar
-1 siblings, 0 replies; 44+ messages in thread
From: Jassi Brar @ 2012-06-28 12:31 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne, patches
On 28 June 2012 16:40, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:
> >
>> Sorry a correction. Reading detect() won't work. I suggest we keep HPD
>> IRQ enabled for the lifetime of the driver.
>
> Ok, I see. But that's not acceptable. It would require us to keep the
> TPD12S015 always powered and enabled. Even if you're not interested in
> using HDMI at all.
>
I think we need to differentiate between HDMI PHY enable and HDMI
5V+,HPD enable [1]... currently they are clubbed together in
omap_dss_device.platform_enable. AFAIK, at least with TPD12S015, they
can be controlled independently and PHY enabling is actually the main
source of power consumption if no display is connected.
By 'lifetime' I mean when the end-user selects some option to the
effect of "Automatically detect and configure display over HDMI" ....
and then we simply enable the HDMI 5V+/HPD, HDMI-PHY would be
enabled only when we actually detect HPD asserted. If a device doesn't
have a port or the user doesn't have a display, neither would be ever
enabled. I mean we should provide a way to make it platform dependent.
[1] Thanks to Andy and his crappy TV, he found clubbing enabling PHY
with 5V+ application comes in the way of detecting cheapo displays
that take ~700ms before asserting HPD i.e, making EDID available. See
how we don't leave it to a HDMI display to take it's own time before
asserting HPD - omapdss_hdmi_display_enable/disable pairs don't care
for that.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID
@ 2012-06-28 12:43 ` Jassi Brar
0 siblings, 0 replies; 44+ messages in thread
From: Jassi Brar @ 2012-06-28 12:43 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: mythripk, linux-omap, linux-fbdev, andy.green, n-dechesne, patches
On 28 June 2012 16:40, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:
> >
>> Sorry a correction. Reading detect() won't work. I suggest we keep HPD
>> IRQ enabled for the lifetime of the driver.
>
> Ok, I see. But that's not acceptable. It would require us to keep the
> TPD12S015 always powered and enabled. Even if you're not interested in
> using HDMI at all.
>
I think we need to differentiate between HDMI PHY enable and HDMI
5V+,HPD enable [1]... currently they are clubbed together in
omap_dss_device.platform_enable. AFAIK, at least with TPD12S015, they
can be controlled independently and PHY enabling is actually the main
source of power consumption if no display is connected.
By 'lifetime' I mean when the end-user selects some option to the
effect of "Automatically detect and configure display over HDMI" ....
and then we simply enable the HDMI 5V+/HPD, HDMI-PHY would be
enabled only when we actually detect HPD asserted. If a device doesn't
have a port or the user doesn't have a display, neither would be ever
enabled. I mean we should provide a way to make it platform dependent.
[1] Thanks to Andy and his crappy TV, he found clubbing enabling PHY
with 5V+ application comes in the way of detecting cheapo displays
that take ~700ms before asserting HPD i.e, making EDID available. See
how we don't leave it to a HDMI display to take it's own time before
asserting HPD - omapdss_hdmi_display_enable/disable pairs don't care
for that.
^ permalink raw reply [flat|nested] 44+ messages in thread