| Message ID | 1461922746-17521-4-git-send-email-hverkuil@xs4all.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Hans, On 29/04/16 12:39, Hans Verkuil wrote: > From: Hans Verkuil <hans.verkuil@cisco.com> > > As long as there is a valid physical address in the EDID and the omap > CEC support is enabled, then we keep ls_oe_gpio on to ensure the CEC > signal is passed through the tpd12s015. > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > Suggested-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c > index 916a899..efbba23 100644 > --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c > +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c > @@ -16,6 +16,7 @@ > #include <linux/platform_device.h> > #include <linux/gpio/consumer.h> > > +#include <media/cec-edid.h> > #include <video/omapdss.h> > #include <video/omap-panel-data.h> > > @@ -65,6 +66,7 @@ static void tpd_disconnect(struct omap_dss_device *dssdev, > return; > > gpiod_set_value_cansleep(ddata->ct_cp_hpd_gpio, 0); > + gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0); > > dst->src = NULL; > dssdev->dst = NULL; > @@ -142,6 +144,7 @@ static int tpd_read_edid(struct omap_dss_device *dssdev, > { > struct panel_drv_data *ddata = to_panel_data(dssdev); > struct omap_dss_device *in = ddata->in; > + bool valid_phys_addr = 0; > int r; > > if (!gpiod_get_value_cansleep(ddata->hpd_gpio)) > @@ -151,7 +154,15 @@ static int tpd_read_edid(struct omap_dss_device *dssdev, > > r = in->ops.hdmi->read_edid(in, edid, len); > > - gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0); > +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC > + /* > + * In order to support CEC this pin should remain high > + * as long as the EDID has a valid physical address. > + */ > + valid_phys_addr = > + cec_get_edid_phys_addr(edid, r, NULL) != CEC_PHYS_ADDR_INVALID; > +#endif > + gpiod_set_value_cansleep(ddata->ls_oe_gpio, valid_phys_addr); > > return r; > } I think this works, but... Maybe it would be cleaner to have the LS_OE enabled if a cable is connected. That's actually what we had earlier, but I removed that due to a race issue: a87a6d6b09de3118e5679c2057b99b7791b7673b ("OMAPDSS: encoder-tpd12s015: Fix race issue with LS_OE"). Now, with CEC, there's need to have LS_OE enabled even after reading the EDID, so I think it's better to go back to the old model (after fixing the race issue, of course =). Tomi
Hi Tomi, On 05/10/2016 01:36 PM, Tomi Valkeinen wrote: > Hi Hans, > > On 29/04/16 12:39, Hans Verkuil wrote: >> From: Hans Verkuil <hans.verkuil@cisco.com> >> >> As long as there is a valid physical address in the EDID and the omap >> CEC support is enabled, then we keep ls_oe_gpio on to ensure the CEC >> signal is passed through the tpd12s015. >> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> >> Suggested-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c >> index 916a899..efbba23 100644 >> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c >> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c >> @@ -16,6 +16,7 @@ >> #include <linux/platform_device.h> >> #include <linux/gpio/consumer.h> >> >> +#include <media/cec-edid.h> >> #include <video/omapdss.h> >> #include <video/omap-panel-data.h> >> >> @@ -65,6 +66,7 @@ static void tpd_disconnect(struct omap_dss_device *dssdev, >> return; >> >> gpiod_set_value_cansleep(ddata->ct_cp_hpd_gpio, 0); >> + gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0); >> >> dst->src = NULL; >> dssdev->dst = NULL; >> @@ -142,6 +144,7 @@ static int tpd_read_edid(struct omap_dss_device *dssdev, >> { >> struct panel_drv_data *ddata = to_panel_data(dssdev); >> struct omap_dss_device *in = ddata->in; >> + bool valid_phys_addr = 0; >> int r; >> >> if (!gpiod_get_value_cansleep(ddata->hpd_gpio)) >> @@ -151,7 +154,15 @@ static int tpd_read_edid(struct omap_dss_device *dssdev, >> >> r = in->ops.hdmi->read_edid(in, edid, len); >> >> - gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0); >> +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC >> + /* >> + * In order to support CEC this pin should remain high >> + * as long as the EDID has a valid physical address. >> + */ >> + valid_phys_addr = >> + cec_get_edid_phys_addr(edid, r, NULL) != CEC_PHYS_ADDR_INVALID; >> +#endif >> + gpiod_set_value_cansleep(ddata->ls_oe_gpio, valid_phys_addr); >> >> return r; >> } > > I think this works, but... Maybe it would be cleaner to have the LS_OE > enabled if a cable is connected. That's actually what we had earlier, > but I removed that due to a race issue: > > a87a6d6b09de3118e5679c2057b99b7791b7673b ("OMAPDSS: encoder-tpd12s015: > Fix race issue with LS_OE"). Now, with CEC, there's need to have LS_OE > enabled even after reading the EDID, so I think it's better to go back > to the old model (after fixing the race issue, of course =). So, this is a bit of a blast from the past since the omap4 CEC development has been on hold for almost a year. But I am about to resume my work on this now that the CEC framework was merged. The latest code is here, if you are interested: https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=panda-cec It's pretty much unchanged from the version I posted a year ago, just rebased. But before I continue with this I have one question for you. First some background: There is a special corner case (and I wasn't aware of that a year ago!) where it is allowed to send a CEC message when there is *no HPD*. The reason is that some displays turn off the hotplug detect pin when they go into standby or when another input is active. The only way to communicate with such displays is via CEC. The problem is that without a HPD there is no EDID and basically no way for an HDMI transmitter to detect that something is connected at all, unless you are using CEC. What this means is that if we want to implement this on the omap4 the CEC support has to be on all the time. We have seen modern displays that behave like this, so this is a real issue. And this corner case is specifically allowed by the CEC specification: the Poll, Image/Text View On and the Active Source messages can be sent to a TV even when there is no HPD in order to turn on the display if it was in standby and to make us the active input. The CEC framework in the kernel supports this starting with 4.12 (this code is in the panda-cec branch above). If this *can't* be supported by the omap4, then I will likely have to add a CEC capability to signal to the application that this specific corner case is not supported. I just did some tests with omap4 and I my impression is that this can't be supported: when the HPD goes away it seems like most/all of the HDMI blocks are all powered off and any attempt to even access the CEC registers will fail. Changing this looks to be non-trivial if not impossible. Can you confirm that that isn't possible? If you think this can be done, then I'd appreciate if you can give me a few pointers. Regards, Hans
On 04/08/2017 12:11 PM, Hans Verkuil wrote: > Hi Tomi, > > On 05/10/2016 01:36 PM, Tomi Valkeinen wrote: >> Hi Hans, >> >> On 29/04/16 12:39, Hans Verkuil wrote: >>> From: Hans Verkuil <hans.verkuil@cisco.com> >>> >>> As long as there is a valid physical address in the EDID and the omap >>> CEC support is enabled, then we keep ls_oe_gpio on to ensure the CEC >>> signal is passed through the tpd12s015. >>> >>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> >>> Suggested-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >>> --- >>> drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 13 ++++++++++++- >>> 1 file changed, 12 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c >>> index 916a899..efbba23 100644 >>> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c >>> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c >>> @@ -16,6 +16,7 @@ >>> #include <linux/platform_device.h> >>> #include <linux/gpio/consumer.h> >>> >>> +#include <media/cec-edid.h> >>> #include <video/omapdss.h> >>> #include <video/omap-panel-data.h> >>> >>> @@ -65,6 +66,7 @@ static void tpd_disconnect(struct omap_dss_device *dssdev, >>> return; >>> >>> gpiod_set_value_cansleep(ddata->ct_cp_hpd_gpio, 0); >>> + gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0); >>> >>> dst->src = NULL; >>> dssdev->dst = NULL; >>> @@ -142,6 +144,7 @@ static int tpd_read_edid(struct omap_dss_device *dssdev, >>> { >>> struct panel_drv_data *ddata = to_panel_data(dssdev); >>> struct omap_dss_device *in = ddata->in; >>> + bool valid_phys_addr = 0; >>> int r; >>> >>> if (!gpiod_get_value_cansleep(ddata->hpd_gpio)) >>> @@ -151,7 +154,15 @@ static int tpd_read_edid(struct omap_dss_device *dssdev, >>> >>> r = in->ops.hdmi->read_edid(in, edid, len); >>> >>> - gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0); >>> +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC >>> + /* >>> + * In order to support CEC this pin should remain high >>> + * as long as the EDID has a valid physical address. >>> + */ >>> + valid_phys_addr = >>> + cec_get_edid_phys_addr(edid, r, NULL) != CEC_PHYS_ADDR_INVALID; >>> +#endif >>> + gpiod_set_value_cansleep(ddata->ls_oe_gpio, valid_phys_addr); >>> >>> return r; >>> } >> >> I think this works, but... Maybe it would be cleaner to have the LS_OE >> enabled if a cable is connected. That's actually what we had earlier, >> but I removed that due to a race issue: >> >> a87a6d6b09de3118e5679c2057b99b7791b7673b ("OMAPDSS: encoder-tpd12s015: >> Fix race issue with LS_OE"). Now, with CEC, there's need to have LS_OE >> enabled even after reading the EDID, so I think it's better to go back >> to the old model (after fixing the race issue, of course =). > > So, this is a bit of a blast from the past since the omap4 CEC development > has been on hold for almost a year. But I am about to resume my work on this > now that the CEC framework was merged. > > The latest code is here, if you are interested: > > https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=panda-cec > > It's pretty much unchanged from the version I posted a year ago, just rebased. > > But before I continue with this I have one question for you. First some > background: > > There is a special corner case (and I wasn't aware of that a year ago!) where > it is allowed to send a CEC message when there is *no HPD*. > > The reason is that some displays turn off the hotplug detect pin when they go > into standby or when another input is active. The only way to communicate with > such displays is via CEC. > > The problem is that without a HPD there is no EDID and basically no way for an > HDMI transmitter to detect that something is connected at all, unless you are > using CEC. > > What this means is that if we want to implement this on the omap4 the CEC support > has to be on all the time. > > We have seen modern displays that behave like this, so this is a real issue. And > this corner case is specifically allowed by the CEC specification: the Poll, > Image/Text View On and the Active Source messages can be sent to a TV even when > there is no HPD in order to turn on the display if it was in standby and to make > us the active input. > > The CEC framework in the kernel supports this starting with 4.12 (this code is > in the panda-cec branch above). > > If this *can't* be supported by the omap4, then I will likely have to add a CEC > capability to signal to the application that this specific corner case is not > supported. FYI: I've just added support for this to the panda-cec branch. CEC on the omap4 now works again, but you can't send CEC messages as long as there is no valid physical address. Regards, Hans > > I just did some tests with omap4 and I my impression is that this can't be > supported: when the HPD goes away it seems like most/all of the HDMI blocks are > all powered off and any attempt to even access the CEC registers will fail. > > Changing this looks to be non-trivial if not impossible. > > Can you confirm that that isn't possible? If you think this can be done, then > I'd appreciate if you can give me a few pointers. > > Regards, > > Hans >
On 08/04/17 13:11, Hans Verkuil wrote: > So, this is a bit of a blast from the past since the omap4 CEC development > has been on hold for almost a year. But I am about to resume my work on this > now that the CEC framework was merged. > > The latest code is here, if you are interested: > > https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=panda-cec > > It's pretty much unchanged from the version I posted a year ago, just rebased. > > But before I continue with this I have one question for you. First some > background: > > There is a special corner case (and I wasn't aware of that a year ago!) where > it is allowed to send a CEC message when there is *no HPD*. > > The reason is that some displays turn off the hotplug detect pin when they go > into standby or when another input is active. The only way to communicate with > such displays is via CEC. > > The problem is that without a HPD there is no EDID and basically no way for an > HDMI transmitter to detect that something is connected at all, unless you are > using CEC. > > What this means is that if we want to implement this on the omap4 the CEC support > has to be on all the time. > > We have seen modern displays that behave like this, so this is a real issue. And > this corner case is specifically allowed by the CEC specification: the Poll, > Image/Text View On and the Active Source messages can be sent to a TV even when > there is no HPD in order to turn on the display if it was in standby and to make > us the active input. > > The CEC framework in the kernel supports this starting with 4.12 (this code is > in the panda-cec branch above). > > If this *can't* be supported by the omap4, then I will likely have to add a CEC > capability to signal to the application that this specific corner case is not > supported. > > I just did some tests with omap4 and I my impression is that this can't be > supported: when the HPD goes away it seems like most/all of the HDMI blocks are > all powered off and any attempt to even access the CEC registers will fail. > > Changing this looks to be non-trivial if not impossible. > > Can you confirm that that isn't possible? If you think this can be done, then > I'd appreciate if you can give me a few pointers. HPD doesn't control the HW, it's all in the SW. So while I don't know much at all about CEC and haven't looked at this particular use case, I believe it's doable. HPD going off will make the DRM connector to be in "disconnected" state, which on omapdrm will cause everything about HDMI to be turned off. Does it work on some other DRM driver? I'm wondering if there's something in the DRM framework side that also has to be changed, in addition to omapdrm changes. It could require larger SW redesigns, though... Which makes me think that the work shouldn't be done until we have changed the omapdrm's driver model to DRM's common bridge driver model, and then all this could possibly be done in a more generic manner. Well, then again, I think the hdmi driver's internal power state handling could be improved even before that. Currently it's not very versatile. We should have ways to partially enable the IP for just the required parts. Tomi
Hi Tomi, On 04/10/2017 01:59 PM, Tomi Valkeinen wrote: > On 08/04/17 13:11, Hans Verkuil wrote: > >> So, this is a bit of a blast from the past since the omap4 CEC development >> has been on hold for almost a year. But I am about to resume my work on this >> now that the CEC framework was merged. >> >> The latest code is here, if you are interested: >> >> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=panda-cec >> >> It's pretty much unchanged from the version I posted a year ago, just rebased. >> >> But before I continue with this I have one question for you. First some >> background: >> >> There is a special corner case (and I wasn't aware of that a year ago!) where >> it is allowed to send a CEC message when there is *no HPD*. >> >> The reason is that some displays turn off the hotplug detect pin when they go >> into standby or when another input is active. The only way to communicate with >> such displays is via CEC. >> >> The problem is that without a HPD there is no EDID and basically no way for an >> HDMI transmitter to detect that something is connected at all, unless you are >> using CEC. >> >> What this means is that if we want to implement this on the omap4 the CEC support >> has to be on all the time. >> >> We have seen modern displays that behave like this, so this is a real issue. And >> this corner case is specifically allowed by the CEC specification: the Poll, >> Image/Text View On and the Active Source messages can be sent to a TV even when >> there is no HPD in order to turn on the display if it was in standby and to make >> us the active input. >> >> The CEC framework in the kernel supports this starting with 4.12 (this code is >> in the panda-cec branch above). >> >> If this *can't* be supported by the omap4, then I will likely have to add a CEC >> capability to signal to the application that this specific corner case is not >> supported. >> >> I just did some tests with omap4 and I my impression is that this can't be >> supported: when the HPD goes away it seems like most/all of the HDMI blocks are >> all powered off and any attempt to even access the CEC registers will fail. >> >> Changing this looks to be non-trivial if not impossible. >> >> Can you confirm that that isn't possible? If you think this can be done, then >> I'd appreciate if you can give me a few pointers. > > HPD doesn't control the HW, it's all in the SW. So while I don't know > much at all about CEC and haven't looked at this particular use case, I > believe it's doable. HPD going off will make the DRM connector to be in > "disconnected" state, which on omapdrm will cause everything about HDMI > to be turned off. > > Does it work on some other DRM driver? I'm wondering if there's > something in the DRM framework side that also has to be changed, in > addition to omapdrm changes. > > It could require larger SW redesigns, though... Which makes me think > that the work shouldn't be done until we have changed the omapdrm's > driver model to DRM's common bridge driver model, and then all this > could possibly be done in a more generic manner. > > Well, then again, I think the hdmi driver's internal power state > handling could be improved even before that. Currently it's not very > versatile. We should have ways to partially enable the IP for just the > required parts. I noticed while experimenting with this that tpd_disconnect() in drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c isn't called when I disconnect the HDMI cable. Is that a bug somewhere? I would expect that tpd_connect and tpd_disconnect are balanced. The tpd_enable and tpd_disable calls are properly balanced and I see the tpd_disable when I disconnect the HDMI cable. The key to keeping CEC up and running, even when there is no HPD is to keep the hdmi.vdda_reg regulator enabled. Also the HDMI_IRQ_CORE should always be on, otherwise I won't get any CEC interrupts. So if the omap4 CEC support is enabled in the kernel config, then always enable this regulator and irq, and otherwise keep the current code. Does that look sensible to you? Regards, Hans
On 04/12/2017 03:03 PM, Hans Verkuil wrote: > Hi Tomi, > > On 04/10/2017 01:59 PM, Tomi Valkeinen wrote: >> On 08/04/17 13:11, Hans Verkuil wrote: >> >>> So, this is a bit of a blast from the past since the omap4 CEC development >>> has been on hold for almost a year. But I am about to resume my work on this >>> now that the CEC framework was merged. >>> >>> The latest code is here, if you are interested: >>> >>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=panda-cec >>> >>> It's pretty much unchanged from the version I posted a year ago, just rebased. >>> >>> But before I continue with this I have one question for you. First some >>> background: >>> >>> There is a special corner case (and I wasn't aware of that a year ago!) where >>> it is allowed to send a CEC message when there is *no HPD*. >>> >>> The reason is that some displays turn off the hotplug detect pin when they go >>> into standby or when another input is active. The only way to communicate with >>> such displays is via CEC. >>> >>> The problem is that without a HPD there is no EDID and basically no way for an >>> HDMI transmitter to detect that something is connected at all, unless you are >>> using CEC. >>> >>> What this means is that if we want to implement this on the omap4 the CEC support >>> has to be on all the time. >>> >>> We have seen modern displays that behave like this, so this is a real issue. And >>> this corner case is specifically allowed by the CEC specification: the Poll, >>> Image/Text View On and the Active Source messages can be sent to a TV even when >>> there is no HPD in order to turn on the display if it was in standby and to make >>> us the active input. >>> >>> The CEC framework in the kernel supports this starting with 4.12 (this code is >>> in the panda-cec branch above). >>> >>> If this *can't* be supported by the omap4, then I will likely have to add a CEC >>> capability to signal to the application that this specific corner case is not >>> supported. >>> >>> I just did some tests with omap4 and I my impression is that this can't be >>> supported: when the HPD goes away it seems like most/all of the HDMI blocks are >>> all powered off and any attempt to even access the CEC registers will fail. >>> >>> Changing this looks to be non-trivial if not impossible. >>> >>> Can you confirm that that isn't possible? If you think this can be done, then >>> I'd appreciate if you can give me a few pointers. >> >> HPD doesn't control the HW, it's all in the SW. So while I don't know >> much at all about CEC and haven't looked at this particular use case, I >> believe it's doable. HPD going off will make the DRM connector to be in >> "disconnected" state, which on omapdrm will cause everything about HDMI >> to be turned off. >> >> Does it work on some other DRM driver? I'm wondering if there's >> something in the DRM framework side that also has to be changed, in >> addition to omapdrm changes. >> >> It could require larger SW redesigns, though... Which makes me think >> that the work shouldn't be done until we have changed the omapdrm's >> driver model to DRM's common bridge driver model, and then all this >> could possibly be done in a more generic manner. >> >> Well, then again, I think the hdmi driver's internal power state >> handling could be improved even before that. Currently it's not very >> versatile. We should have ways to partially enable the IP for just the >> required parts. > > I noticed while experimenting with this that tpd_disconnect() in > drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c isn't called when > I disconnect the HDMI cable. Is that a bug somewhere? > > I would expect that tpd_connect and tpd_disconnect are balanced. The tpd_enable > and tpd_disable calls are properly balanced and I see the tpd_disable when I > disconnect the HDMI cable. > > The key to keeping CEC up and running, even when there is no HPD is to keep > the hdmi.vdda_reg regulator enabled. Also the HDMI_IRQ_CORE should always be > on, otherwise I won't get any CEC interrupts. > > So if the omap4 CEC support is enabled in the kernel config, then always enable > this regulator and irq, and otherwise keep the current code. And of course the ls_oe_gpio in tpd12s015.c should always be high as well, otherwise the CEC signal would never get through. This too can depend on the kernel config. Hans > > Does that look sensible to you? > > Regards, > > Hans >
On 12/04/17 16:03, Hans Verkuil wrote: > I noticed while experimenting with this that tpd_disconnect() in > drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c isn't called when > I disconnect the HDMI cable. Is that a bug somewhere? > > I would expect that tpd_connect and tpd_disconnect are balanced. The tpd_enable > and tpd_disable calls are properly balanced and I see the tpd_disable when I > disconnect the HDMI cable. The connect/disconnect naming there is legacy... It's not about cable connect, it's about the initial "connect" of the drivers in the video pipeline. It's done just once when starting omapdrm. > The key to keeping CEC up and running, even when there is no HPD is to keep > the hdmi.vdda_reg regulator enabled. Also the HDMI_IRQ_CORE should always be > on, otherwise I won't get any CEC interrupts. At the moment there's no way to enable the pipeline without enabling the video. > So if the omap4 CEC support is enabled in the kernel config, then always enable > this regulator and irq, and otherwise keep the current code. Well, I have no clue about how CEC is used, but don't we have some userspace components using it? I presume there's an open() or something similar that signals that the userspace is interested in CEC. That should be the trigger to enable the HW required for CEC. So is some other driver supporting this already? Or is the omap4 the first platform you're trying this on? Tomi
On 04/12/2017 03:21 PM, Tomi Valkeinen wrote: > On 12/04/17 16:03, Hans Verkuil wrote: > >> I noticed while experimenting with this that tpd_disconnect() in >> drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c isn't called when >> I disconnect the HDMI cable. Is that a bug somewhere? >> >> I would expect that tpd_connect and tpd_disconnect are balanced. The tpd_enable >> and tpd_disable calls are properly balanced and I see the tpd_disable when I >> disconnect the HDMI cable. > > The connect/disconnect naming there is legacy... It's not about cable > connect, it's about the initial "connect" of the drivers in the video > pipeline. It's done just once when starting omapdrm. Ah, good to know. >> The key to keeping CEC up and running, even when there is no HPD is to keep >> the hdmi.vdda_reg regulator enabled. Also the HDMI_IRQ_CORE should always be >> on, otherwise I won't get any CEC interrupts. > > At the moment there's no way to enable the pipeline without enabling the > video. > >> So if the omap4 CEC support is enabled in the kernel config, then always enable >> this regulator and irq, and otherwise keep the current code. > > Well, I have no clue about how CEC is used, but don't we have some > userspace components using it? I presume there's an open() or something > similar that signals that the userspace is interested in CEC. That > should be the trigger to enable the HW required for CEC. Why didn't I think of that. I did a quick implementation to test this and it works. > So is some other driver supporting this already? Or is the omap4 the > first platform you're trying this on? No, there are quite a few CEC drivers by now, but typically the CEC block is a totally independent IP block with its own power, irq, etc. The omap4 is by far the most complex one to set up with various GPIO pins, interrupts, regulators, etc. to deal with. Normally it takes about 2 days to make a new CEC driver, but the omap4 is much more work :-( Regards, Hans
On 12/04/17 17:04, Hans Verkuil wrote: >> So is some other driver supporting this already? Or is the omap4 the >> first platform you're trying this on? > > No, there are quite a few CEC drivers by now, but typically the CEC block is > a totally independent IP block with its own power, irq, etc. The omap4 is by far > the most complex one to set up with various GPIO pins, interrupts, regulators, > etc. to deal with. > > Normally it takes about 2 days to make a new CEC driver, but the omap4 is much > more work :-( Ok. I mentioned the omapdrm restructuring that we've planned to do, I think after that this will be easier to implement in a nice way. For now, I think more or less what you have now is an acceptable solution. We can hack the tpd12s015 to keep the level shifter always enabled, and, afaics, everything else can be handled inside the hdmi4 driver, right? Generally speaking, what are the "dependencies" for CEC? It needs to access EDID? Does CEC care about HPD? Does it care if the cable is connected or not? For Panda, the level shifter of tpd12s015 is obviously one hard dendency. Is there anything else CEC needs to access or control (besides the CEC IP itself)? Tomi
On 04/13/2017 10:43 AM, Tomi Valkeinen wrote: > On 12/04/17 17:04, Hans Verkuil wrote: > >>> So is some other driver supporting this already? Or is the omap4 the >>> first platform you're trying this on? >> >> No, there are quite a few CEC drivers by now, but typically the CEC block is >> a totally independent IP block with its own power, irq, etc. The omap4 is by far >> the most complex one to set up with various GPIO pins, interrupts, regulators, >> etc. to deal with. >> >> Normally it takes about 2 days to make a new CEC driver, but the omap4 is much >> more work :-( > > Ok. > > I mentioned the omapdrm restructuring that we've planned to do, I think > after that this will be easier to implement in a nice way. > > For now, I think more or less what you have now is an acceptable > solution. We can hack the tpd12s015 to keep the level shifter always > enabled, and, afaics, everything else can be handled inside the hdmi4 > driver, right? Right. > Generally speaking, what are the "dependencies" for CEC? It needs to > access EDID? Does CEC care about HPD? Does it care if the cable is > connected or not? For Panda, the level shifter of tpd12s015 is obviously > one hard dendency. > > Is there anything else CEC needs to access or control (besides the CEC > IP itself)? The CEC framework needs to be informed about the physical address contained in the EDID (part of the CEA-861 block). And when the HPD goes down it needs to be informed as well (same call, but with CEC_PHYS_ADDR_INVALID as argument). And it needs to stay powered up even if the HPD is down. That's all. Regards, Hans
On 13/04/17 12:12, Hans Verkuil wrote: >> Is there anything else CEC needs to access or control (besides the CEC >> IP itself)? > > The CEC framework needs to be informed about the physical address contained > in the EDID (part of the CEA-861 block). And when the HPD goes down it needs > to be informed as well (same call, but with CEC_PHYS_ADDR_INVALID as argument). Ah, hmm... And currently that's (kind of) handled in hdmi_power_off_full() by setting CEC_PHYS_ADDR_INVALID? That's not the same thing as HPD off, though, but maybe it's enough (for now). Tomi
diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c index 916a899..efbba23 100644 --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c @@ -16,6 +16,7 @@ #include <linux/platform_device.h> #include <linux/gpio/consumer.h> +#include <media/cec-edid.h> #include <video/omapdss.h> #include <video/omap-panel-data.h> @@ -65,6 +66,7 @@ static void tpd_disconnect(struct omap_dss_device *dssdev, return; gpiod_set_value_cansleep(ddata->ct_cp_hpd_gpio, 0); + gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0); dst->src = NULL; dssdev->dst = NULL; @@ -142,6 +144,7 @@ static int tpd_read_edid(struct omap_dss_device *dssdev, { struct panel_drv_data *ddata = to_panel_data(dssdev); struct omap_dss_device *in = ddata->in; + bool valid_phys_addr = 0; int r; if (!gpiod_get_value_cansleep(ddata->hpd_gpio)) @@ -151,7 +154,15 @@ static int tpd_read_edid(struct omap_dss_device *dssdev, r = in->ops.hdmi->read_edid(in, edid, len); - gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0); +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC + /* + * In order to support CEC this pin should remain high + * as long as the EDID has a valid physical address. + */ + valid_phys_addr = + cec_get_edid_phys_addr(edid, r, NULL) != CEC_PHYS_ADDR_INVALID; +#endif + gpiod_set_value_cansleep(ddata->ls_oe_gpio, valid_phys_addr); return r; }