From: Russell King rmk+kernel@armlinux.org.uk
Add CEC notifier support to the HDMI bridge driver, so that the CEC part of the IP can receive its physical address.
Tested-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Hans Verkuil hans.verkuil@cisco.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk Signed-off-by: Neil Armstrong narmstrong@baylibre.com --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
Hi Archit, Hans,
This is repost of Russell's patch from his dw-hdmi CEC patchset.
Since his CEC implementation will be integrated in the bridge driver, this notifier patch won't be re-posted.
But the Amlogic Platform needs a notifier since it uses a standalone CEC controller.
Thanks, Neil
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead1124..9c03846 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -33,6 +33,8 @@ #include <uapi/linux/media-bus-format.h> #include <uapi/linux/videodev2.h>
+#include <media/cec-notifier.h> + #include "dw-hdmi.h" #include "dw-hdmi-audio.h"
@@ -175,6 +177,8 @@ struct dw_hdmi { struct regmap *regm; void (*enable_audio)(struct dw_hdmi *hdmi); void (*disable_audio)(struct dw_hdmi *hdmi); + + struct cec_notifier *cec_notifier; };
#define HDMI_IH_PHY_STAT0_RX_SENSE \ @@ -1896,6 +1900,7 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector) hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid); hdmi->sink_has_audio = drm_detect_monitor_audio(edid); drm_mode_connector_update_edid_property(connector, edid); + cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid); ret = drm_add_edid_modes(connector, edid); /* Store the ELD */ drm_edid_to_eld(connector, edid); @@ -2077,6 +2082,10 @@ void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense) dw_hdmi_update_phy_mask(hdmi); } mutex_unlock(&hdmi->mutex); + + if (!rx_sense && !hpd) + cec_notifier_set_phys_addr(hdmi->cec_notifier, + CEC_PHYS_ADDR_INVALID); }
void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense) @@ -2376,6 +2385,12 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) if (ret) goto err_iahb;
+ hdmi->cec_notifier = cec_notifier_get(dev); + if (!hdmi->cec_notifier) { + ret = -ENOMEM; + goto err_iahb; + } + /* * To prevent overflows in HDMI_IH_FC_STAT2, set the clk regenerator * N and cts values before enabling phy @@ -2452,6 +2467,9 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) hdmi->ddc = NULL; }
+ if (hdmi->cec_notifier) + cec_notifier_put(hdmi->cec_notifier); + clk_disable_unprepare(hdmi->iahb_clk); err_isfr: clk_disable_unprepare(hdmi->isfr_clk);
Hi Neil,
On 06-07-2017 11:33, Neil Armstrong wrote:
From: Russell King rmk+kernel@armlinux.org.uk
Add CEC notifier support to the HDMI bridge driver, so that the CEC part of the IP can receive its physical address.
Tested-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Hans Verkuil hans.verkuil@cisco.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
Hi Archit, Hans,
This is repost of Russell's patch from his dw-hdmi CEC patchset.
Since his CEC implementation will be integrated in the bridge driver, this notifier patch won't be re-posted.
But the Amlogic Platform needs a notifier since it uses a standalone CEC controller.
Thanks, Neil
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead1124..9c03846 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -33,6 +33,8 @@ #include <uapi/linux/media-bus-format.h> #include <uapi/linux/videodev2.h>
+#include <media/cec-notifier.h>
Don't you also have to "select CEC_NOTIFIER" in Kconfig? Or is it not needed anymore with the recent Kconfig changes of CEC?
Best regards, Jose Miguel Abreu
#include "dw-hdmi.h" #include "dw-hdmi-audio.h"
@@ -175,6 +177,8 @@ struct dw_hdmi { struct regmap *regm; void (*enable_audio)(struct dw_hdmi *hdmi); void (*disable_audio)(struct dw_hdmi *hdmi);
- struct cec_notifier *cec_notifier;
};
#define HDMI_IH_PHY_STAT0_RX_SENSE \ @@ -1896,6 +1900,7 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector) hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid); hdmi->sink_has_audio = drm_detect_monitor_audio(edid); drm_mode_connector_update_edid_property(connector, edid);
cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid);@@ -2077,6 +2082,10 @@ void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense) dw_hdmi_update_phy_mask(hdmi); } mutex_unlock(&hdmi->mutex);
- if (!rx_sense && !hpd)
cec_notifier_set_phys_addr(hdmi->cec_notifier,CEC_PHYS_ADDR_INVALID);}
void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense) @@ -2376,6 +2385,12 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) if (ret) goto err_iahb;
- hdmi->cec_notifier = cec_notifier_get(dev);
- if (!hdmi->cec_notifier) {
ret = -ENOMEM;goto err_iahb;- }
- /*
- To prevent overflows in HDMI_IH_FC_STAT2, set the clk regenerator
- N and cts values before enabling phy
@@ -2452,6 +2467,9 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) hdmi->ddc = NULL; }
- if (hdmi->cec_notifier)
cec_notifier_put(hdmi->cec_notifier);- clk_disable_unprepare(hdmi->iahb_clk);
err_isfr: clk_disable_unprepare(hdmi->isfr_clk);
On 07/06/2017 12:43 PM, Jose Abreu wrote:
Hi Neil,
On 06-07-2017 11:33, Neil Armstrong wrote:
From: Russell King rmk+kernel@armlinux.org.uk
Add CEC notifier support to the HDMI bridge driver, so that the CEC part of the IP can receive its physical address.
Tested-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Hans Verkuil hans.verkuil@cisco.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
Hi Archit, Hans,
This is repost of Russell's patch from his dw-hdmi CEC patchset.
Since his CEC implementation will be integrated in the bridge driver, this notifier patch won't be re-posted.
But the Amlogic Platform needs a notifier since it uses a standalone CEC controller.
Thanks, Neil
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead1124..9c03846 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -33,6 +33,8 @@ #include <uapi/linux/media-bus-format.h> #include <uapi/linux/videodev2.h>
+#include <media/cec-notifier.h>
Don't you also have to "select CEC_NOTIFIER" in Kconfig? Or is it not needed anymore with the recent Kconfig changes of CEC?
Hi Jose,
Seems no, since the cec functions are protected with : #if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER)
and replaced with dummy inline functions.
Neil
Best regards, Jose Miguel Abreu
#include "dw-hdmi.h" #include "dw-hdmi-audio.h"
@@ -175,6 +177,8 @@ struct dw_hdmi { struct regmap *regm; void (*enable_audio)(struct dw_hdmi *hdmi); void (*disable_audio)(struct dw_hdmi *hdmi);
- struct cec_notifier *cec_notifier;
};
#define HDMI_IH_PHY_STAT0_RX_SENSE \ @@ -1896,6 +1900,7 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector) hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid); hdmi->sink_has_audio = drm_detect_monitor_audio(edid); drm_mode_connector_update_edid_property(connector, edid);
cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid);@@ -2077,6 +2082,10 @@ void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense) dw_hdmi_update_phy_mask(hdmi); } mutex_unlock(&hdmi->mutex);
- if (!rx_sense && !hpd)
cec_notifier_set_phys_addr(hdmi->cec_notifier,CEC_PHYS_ADDR_INVALID);}
void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense) @@ -2376,6 +2385,12 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) if (ret) goto err_iahb;
- hdmi->cec_notifier = cec_notifier_get(dev);
- if (!hdmi->cec_notifier) {
ret = -ENOMEM;goto err_iahb;- }
- /*
- To prevent overflows in HDMI_IH_FC_STAT2, set the clk regenerator
- N and cts values before enabling phy
@@ -2452,6 +2467,9 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) hdmi->ddc = NULL; }
- if (hdmi->cec_notifier)
cec_notifier_put(hdmi->cec_notifier);- clk_disable_unprepare(hdmi->iahb_clk);
err_isfr: clk_disable_unprepare(hdmi->isfr_clk);
On Thu, Jul 06, 2017 at 12:33:06PM +0200, Neil Armstrong wrote:
From: Russell King rmk+kernel@armlinux.org.uk
Add CEC notifier support to the HDMI bridge driver, so that the CEC part of the IP can receive its physical address.
Tested-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Hans Verkuil hans.verkuil@cisco.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
Hi Archit, Hans,
This is repost of Russell's patch from his dw-hdmi CEC patchset.
Since his CEC implementation will be integrated in the bridge driver, this notifier patch won't be re-posted.
But the Amlogic Platform needs a notifier since it uses a standalone CEC controller.
Without this, the dw-hdmi CEC support will be totally useless anyway, and it's pointless to merge it without this patch.
On 07/06/2017 01:05 PM, Russell King - ARM Linux wrote:
On Thu, Jul 06, 2017 at 12:33:06PM +0200, Neil Armstrong wrote:
From: Russell King rmk+kernel@armlinux.org.uk
Add CEC notifier support to the HDMI bridge driver, so that the CEC part of the IP can receive its physical address.
Tested-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Hans Verkuil hans.verkuil@cisco.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
Hi Archit, Hans,
This is repost of Russell's patch from his dw-hdmi CEC patchset.
Since his CEC implementation will be integrated in the bridge driver, this notifier patch won't be re-posted.
But the Amlogic Platform needs a notifier since it uses a standalone CEC controller.
Without this, the dw-hdmi CEC support will be totally useless anyway, and it's pointless to merge it without this patch.
Hi Russell,
While following discussion on your last patchset, it seems you agreed with Hans to no more rely on the cec-notifier but directly call the dw-hdmi-cec functions.
Anyway, if this patch is merged separately and you still depend on it, you could still rebase on it when it appears on drm-misc-next...
Neil
On Thu, Jul 06, 2017 at 01:29:54PM +0200, Neil Armstrong wrote:
On 07/06/2017 01:05 PM, Russell King - ARM Linux wrote:
On Thu, Jul 06, 2017 at 12:33:06PM +0200, Neil Armstrong wrote:
From: Russell King rmk+kernel@armlinux.org.uk
Add CEC notifier support to the HDMI bridge driver, so that the CEC part of the IP can receive its physical address.
Tested-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Hans Verkuil hans.verkuil@cisco.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
Hi Archit, Hans,
This is repost of Russell's patch from his dw-hdmi CEC patchset.
Since his CEC implementation will be integrated in the bridge driver, this notifier patch won't be re-posted.
But the Amlogic Platform needs a notifier since it uses a standalone CEC controller.
Without this, the dw-hdmi CEC support will be totally useless anyway, and it's pointless to merge it without this patch.
Hi Russell,
While following discussion on your last patchset, it seems you agreed with Hans to no more rely on the cec-notifier but directly call the dw-hdmi-cec functions.
Incorrect. I think you're looking at the discussion in "thread mode" and assuming that messages are stored in date order...
There was a discussion about removing the cec-notifier which happened on June 1st/2nd, based off the covering email.
However, there was a later discussion on June 9th (sparked by your requirement) which changed the resolution from "lets remove the notifier" to "we need to keep the notifier". This was based on patch 2/4.
I never posted an updated patch set, because of the dependencies, but Hans decided on June 12th to do I-don't-know-what with the patches I sent, resulting in what we have queued up today. This brought the sub-thread containing the cec-notifier removal discussion to be _after_ the June 9th discussion, which is probably what's causing some confusion here.
However, I can assure you that the resolution of the discussion with the cec-notifier was that it should remain in place - this is from the last few messages in the discussion:
Hans wrote on 9th June: | You wrote on 9th June: | > It won't since the Meson platform needs it... | | Ah, I wasn't aware of that when I wrote my original comments. In that case | we do need the notifier. Which is fine, as long as the reason for that is | documented.
From what you're saying, it sounds like the CEC dw-hdmi-cec.c still
relies on the notifier, but the patch which adds the CEC notifier to dw-hdmi.c was omited from what Hans did, which will result in the whole thing being a total waste of time.
Anyway, if this patch is merged separately and you still depend on it, you could still rebase on it when it appears on drm-misc-next...
Well, from what I can see in 4.12, the cec-notifier stuff is rather broken (tda998x has stopped working as its stuck with a physical address of f.f.f.f) so I think the whole thing is rather moot right now. I don't yet know what's going on with that, other than the notifier stuff seems to not be working, despite being enabled in the .config.
On 07/06/2017 01:45 PM, Russell King - ARM Linux wrote:
On Thu, Jul 06, 2017 at 01:29:54PM +0200, Neil Armstrong wrote:
On 07/06/2017 01:05 PM, Russell King - ARM Linux wrote:
On Thu, Jul 06, 2017 at 12:33:06PM +0200, Neil Armstrong wrote:
From: Russell King rmk+kernel@armlinux.org.uk
Add CEC notifier support to the HDMI bridge driver, so that the CEC part of the IP can receive its physical address.
Tested-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Neil Armstrong narmstrong@baylibre.com Acked-by: Hans Verkuil hans.verkuil@cisco.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
Hi Archit, Hans,
This is repost of Russell's patch from his dw-hdmi CEC patchset.
Since his CEC implementation will be integrated in the bridge driver, this notifier patch won't be re-posted.
But the Amlogic Platform needs a notifier since it uses a standalone CEC controller.
Without this, the dw-hdmi CEC support will be totally useless anyway, and it's pointless to merge it without this patch.
Hi Russell,
While following discussion on your last patchset, it seems you agreed with Hans to no more rely on the cec-notifier but directly call the dw-hdmi-cec functions.
Incorrect. I think you're looking at the discussion in "thread mode" and assuming that messages are stored in date order...
There was a discussion about removing the cec-notifier which happened on June 1st/2nd, based off the covering email.
However, there was a later discussion on June 9th (sparked by your requirement) which changed the resolution from "lets remove the notifier" to "we need to keep the notifier". This was based on patch 2/4.
I never posted an updated patch set, because of the dependencies, but Hans decided on June 12th to do I-don't-know-what with the patches I sent, resulting in what we have queued up today. This brought the sub-thread containing the cec-notifier removal discussion to be _after_ the June 9th discussion, which is probably what's causing some confusion here.
However, I can assure you that the resolution of the discussion with the cec-notifier was that it should remain in place - this is from the last few messages in the discussion:
Hans wrote on 9th June: | You wrote on 9th June: | > It won't since the Meson platform needs it... | | Ah, I wasn't aware of that when I wrote my original comments. In that case | we do need the notifier. Which is fine, as long as the reason for that is | documented.
From what you're saying, it sounds like the CEC dw-hdmi-cec.c still relies on the notifier, but the patch which adds the CEC notifier to dw-hdmi.c was omited from what Hans did, which will result in the whole thing being a total waste of time.
Anyway, if this patch is merged separately and you still depend on it, you could still rebase on it when it appears on drm-misc-next...
Well, from what I can see in 4.12, the cec-notifier stuff is rather broken (tda998x has stopped working as its stuck with a physical address of f.f.f.f) so I think the whole thing is rather moot right now. I don't yet know what's going on with that, other than the notifier stuff seems to not be working, despite being enabled in the .config.
Indeed, I missed some parts of the thread, sorry for the confusion.
Anyway, is it a showstopper to have this patch merged separately ?
If you prefer re-posting a serie with this patch inside, no problem, but the Amlogic platform still needs this patch to have CEC working.
Maybe it's because of fixes introduced in 4.13, but using next-20170706 and this patch, make things works perfectly.
Neil
On Thu, Jul 06, 2017 at 01:55:58PM +0200, Neil Armstrong wrote:
On 07/06/2017 01:45 PM, Russell King - ARM Linux wrote:
Well, from what I can see in 4.12, the cec-notifier stuff is rather broken (tda998x has stopped working as its stuck with a physical address of f.f.f.f) so I think the whole thing is rather moot right now. I don't yet know what's going on with that, other than the notifier stuff seems to not be working, despite being enabled in the .config.
Indeed, I missed some parts of the thread, sorry for the confusion.
Anyway, is it a showstopper to have this patch merged separately ?
It shouldn't be provided it makes _this_ merge window or the -rc following, but it doesn't really comprise a "fix" in that the feature never worked in the previous kernel (and we are supposed to have the rule that features are merged prior to the merge window.)
It's up to the maintainers to decide what to do at this point.
If you prefer re-posting a serie with this patch inside, no problem, but the Amlogic platform still needs this patch to have CEC working.
I suspect (because I've not seen what has been queued to be merged yet) that even the dw-hdmi "native" side will be similarly broken - unless Hans merged some other changes to make it work.
So, I suggest we all sit tight and wait until what was merged appears in Linus' tree.
On Thu, Jul 06, 2017 at 12:45:55PM +0100, Russell King - ARM Linux wrote:
Well, from what I can see in 4.12, the cec-notifier stuff is rather broken (tda998x has stopped working as its stuck with a physical address of f.f.f.f) so I think the whole thing is rather moot right now. I don't yet know what's going on with that, other than the notifier stuff seems to not be working, despite being enabled in the .config.
The problem there appears to be the changes that were made with the way the config works - which IMHO are totally broken.
Let's take this scenario:
- You have a HDMI bridge, and you build that into the kernel, because you want the display to come up early. - You have a CEC driver, which you build as a module.
If the HDMI bridge driver selects CEC_NOTIFIER and the CEC driver selects both CEC_NOTIFIER and CEC_CORE, you end up with CEC_NOTIFIER=y and CEC_CORE=m.
We now come to this:
#if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER)
The definition of IS_REACHABLE() is that it is false if the config symbol is selected as a module. So, in this case, we end up compiling out all the CEC notifier functions from the HDMI bridge, and building them into the CEC driver.
The CEC notifier also gets built as a module, meaning that there's no way for the built-in HDMI bridge could ever call the notifier.
The overall result of this is that such a configuration completely breaks such a setup - a setup that worked fine before the CEC Kconfig changes.
This isn't limited to tda998x - I'd expect the same to be true of dw-hdmi.
On Thu, Jul 06, 2017 at 12:56:43PM +0100, Russell King - ARM Linux wrote:
On Thu, Jul 06, 2017 at 12:45:55PM +0100, Russell King - ARM Linux wrote:
Well, from what I can see in 4.12, the cec-notifier stuff is rather broken (tda998x has stopped working as its stuck with a physical address of f.f.f.f) so I think the whole thing is rather moot right now. I don't yet know what's going on with that, other than the notifier stuff seems to not be working, despite being enabled in the .config.
The problem there appears to be the changes that were made with the way the config works - which IMHO are totally broken.
Let's take this scenario:
- You have a HDMI bridge, and you build that into the kernel, because you want the display to come up early.
- You have a CEC driver, which you build as a module.
If the HDMI bridge driver selects CEC_NOTIFIER and the CEC driver selects both CEC_NOTIFIER and CEC_CORE, you end up with CEC_NOTIFIER=y and CEC_CORE=m.
We now come to this:
#if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER)
The definition of IS_REACHABLE() is that it is false if the config symbol is selected as a module. So, in this case, we end up compiling out all the CEC notifier functions from the HDMI bridge, and building them into the CEC driver.
The CEC notifier also gets built as a module, meaning that there's no way for the built-in HDMI bridge could ever call the notifier.
The overall result of this is that such a configuration completely breaks such a setup - a setup that worked fine before the CEC Kconfig changes.
This isn't limited to tda998x - I'd expect the same to be true of dw-hdmi.
Fixing this so cec-notifier is built-in isn't sufficient, because we also need the cec-edid parsing code as well, which is currently part of cec-core, and that function gets stubbed out if cec-core is not built-in...
The patch below works for me.
drivers/media/Makefile | 2 +- drivers/media/cec/Makefile | 6 +++--- drivers/media/cec/cec-core.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/Makefile b/drivers/media/Makefile index 044503aa8801..0c02fbe4b9c7 100644 --- a/drivers/media/Makefile +++ b/drivers/media/Makefile @@ -24,7 +24,7 @@ obj-$(CONFIG_DVB_CORE) += dvb-core/ # There are both core and drivers at RC subtree - merge before drivers obj-y += rc/
-obj-$(CONFIG_CEC_CORE) += cec/ +obj-y += cec/
# # Finally, merge the drivers that require the core diff --git a/drivers/media/cec/Makefile b/drivers/media/cec/Makefile index eaf408e64669..58394b77a328 100644 --- a/drivers/media/cec/Makefile +++ b/drivers/media/cec/Makefile @@ -1,7 +1,7 @@ cec-objs := cec-core.o cec-adap.o cec-api.o cec-edid.o
-ifeq ($(CONFIG_CEC_NOTIFIER),y) - cec-objs += cec-notifier.o -endif +obj-$(CONFIG_CEC_NOTIFIER) += cec-notifier.o cec-edid.o +# Remove cec-edid from cec-core if the notifier is enabled +cec-objs := $(filter-out $(obj-y) $(obj-m), $(cec-objs))
obj-$(CONFIG_CEC_CORE) += cec.o diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c index 2f87748ba4fc..bce26b94c348 100644 --- a/drivers/media/cec/cec-core.c +++ b/drivers/media/cec/cec-core.c @@ -187,7 +187,7 @@ static void cec_devnode_unregister(struct cec_devnode *devnode) put_device(&devnode->dev); }
-#ifdef CONFIG_CEC_NOTIFIER +#if IS_ENABLED(CONFIG_CEC_NOTIFIER) static void cec_cec_notify(struct cec_adapter *adap, u16 pa) { cec_s_phys_addr(adap, pa, false);
On Thu, Jul 06, 2017 at 02:38:41PM +0100, Russell King - ARM Linux wrote:
On Thu, Jul 06, 2017 at 12:56:43PM +0100, Russell King - ARM Linux wrote:
On Thu, Jul 06, 2017 at 12:45:55PM +0100, Russell King - ARM Linux wrote:
Well, from what I can see in 4.12, the cec-notifier stuff is rather broken (tda998x has stopped working as its stuck with a physical address of f.f.f.f) so I think the whole thing is rather moot right now. I don't yet know what's going on with that, other than the notifier stuff seems to not be working, despite being enabled in the .config.
The problem there appears to be the changes that were made with the way the config works - which IMHO are totally broken.
Let's take this scenario:
- You have a HDMI bridge, and you build that into the kernel, because you want the display to come up early.
- You have a CEC driver, which you build as a module.
If the HDMI bridge driver selects CEC_NOTIFIER and the CEC driver selects both CEC_NOTIFIER and CEC_CORE, you end up with CEC_NOTIFIER=y and CEC_CORE=m.
We now come to this:
#if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER)
The definition of IS_REACHABLE() is that it is false if the config symbol is selected as a module. So, in this case, we end up compiling out all the CEC notifier functions from the HDMI bridge, and building them into the CEC driver.
The CEC notifier also gets built as a module, meaning that there's no way for the built-in HDMI bridge could ever call the notifier.
The overall result of this is that such a configuration completely breaks such a setup - a setup that worked fine before the CEC Kconfig changes.
This isn't limited to tda998x - I'd expect the same to be true of dw-hdmi.
Fixing this so cec-notifier is built-in isn't sufficient, because we also need the cec-edid parsing code as well, which is currently part of cec-core, and that function gets stubbed out if cec-core is not built-in...
The patch below works for me.
I missed the include/media changes...
drivers/media/Makefile | 2 +- drivers/media/cec/Makefile | 6 ++--- drivers/media/cec/cec-core.c | 2 +- include/media/cec-notifier.h | 2 +- include/media/cec.h | 54 ++++++++++++++++++++++++-------------------- 5 files changed, 36 insertions(+), 30 deletions(-)
diff --git a/drivers/media/Makefile b/drivers/media/Makefile index 044503aa8801..0c02fbe4b9c7 100644 --- a/drivers/media/Makefile +++ b/drivers/media/Makefile @@ -24,7 +24,7 @@ obj-$(CONFIG_DVB_CORE) += dvb-core/ # There are both core and drivers at RC subtree - merge before drivers obj-y += rc/
-obj-$(CONFIG_CEC_CORE) += cec/ +obj-y += cec/
# # Finally, merge the drivers that require the core diff --git a/drivers/media/cec/Makefile b/drivers/media/cec/Makefile index eaf408e64669..58394b77a328 100644 --- a/drivers/media/cec/Makefile +++ b/drivers/media/cec/Makefile @@ -1,7 +1,7 @@ cec-objs := cec-core.o cec-adap.o cec-api.o cec-edid.o
-ifeq ($(CONFIG_CEC_NOTIFIER),y) - cec-objs += cec-notifier.o -endif +obj-$(CONFIG_CEC_NOTIFIER) += cec-notifier.o cec-edid.o + +cec-objs := $(filter-out $(obj-y) $(obj-m), $(cec-objs))
obj-$(CONFIG_CEC_CORE) += cec.o diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c index 2f87748ba4fc..bce26b94c348 100644 --- a/drivers/media/cec/cec-core.c +++ b/drivers/media/cec/cec-core.c @@ -187,7 +187,7 @@ static void cec_devnode_unregister(struct cec_devnode *devnode) put_device(&devnode->dev); }
-#ifdef CONFIG_CEC_NOTIFIER +#if IS_ENABLED(CONFIG_CEC_NOTIFIER) static void cec_cec_notify(struct cec_adapter *adap, u16 pa) { cec_s_phys_addr(adap, pa, false); diff --git a/include/media/cec-notifier.h b/include/media/cec-notifier.h index 298f996969df..83bdc221d0a0 100644 --- a/include/media/cec-notifier.h +++ b/include/media/cec-notifier.h @@ -29,7 +29,7 @@ struct edid; struct cec_adapter; struct cec_notifier;
-#if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER) +#if IS_ENABLED(CONFIG_CEC_NOTIFIER)
/** * cec_notifier_get - find or create a new cec_notifier for the given device. diff --git a/include/media/cec.h b/include/media/cec.h index 201f060978da..039aad98462d 100644 --- a/include/media/cec.h +++ b/include/media/cec.h @@ -225,6 +225,36 @@ void cec_transmit_done(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt, u8 nack_cnt, u8 low_drive_cnt, u8 error_cnt); void cec_received_msg(struct cec_adapter *adap, struct cec_msg *msg);
+#if IS_ENABLED(CONFIG_CEC_NOTIFIER) +void cec_register_cec_notifier(struct cec_adapter *adap, + struct cec_notifier *notifier); +#endif + +#else + +static inline int cec_register_adapter(struct cec_adapter *adap, + struct device *parent) +{ + return 0; +} + +static inline void cec_unregister_adapter(struct cec_adapter *adap) +{ +} + +static inline void cec_delete_adapter(struct cec_adapter *adap) +{ +} + +static inline void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, + bool block) +{ +} + +#endif + +#if IS_REACHABLE(CONFIG_CEC_CORE) || IS_REACHABLE(CONFIG_CEC_NOTIFIER) + /** * cec_get_edid_phys_addr() - find and return the physical address * @@ -300,32 +330,8 @@ u16 cec_phys_addr_for_input(u16 phys_addr, u8 input); */ int cec_phys_addr_validate(u16 phys_addr, u16 *parent, u16 *port);
-#ifdef CONFIG_CEC_NOTIFIER -void cec_register_cec_notifier(struct cec_adapter *adap, - struct cec_notifier *notifier); -#endif - #else
-static inline int cec_register_adapter(struct cec_adapter *adap, - struct device *parent) -{ - return 0; -} - -static inline void cec_unregister_adapter(struct cec_adapter *adap) -{ -} - -static inline void cec_delete_adapter(struct cec_adapter *adap) -{ -} - -static inline void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, - bool block) -{ -} - static inline u16 cec_get_edid_phys_addr(const u8 *edid, unsigned int size, unsigned int *offset) {
dri-devel@lists.freedesktop.org
-
Jose Abreu -
Neil Armstrong -
Russell King - ARM Linux