Re: [PATCH] drm/bridge: dw_hdmi: add cec notifier support
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;
+}
Re: [PATCH] drm/bridge: dw_hdmi: add cec notifier support
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);
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
Re: [PATCH] drm/bridge: dw_hdmi: add cec notifier support
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. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
Re: [PATCH] drm/bridge: dw_hdmi: add cec notifier support
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. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
Re: [PATCH] drm/bridge: dw_hdmi: add cec notifier support
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 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 Acked-by: Neil Armstrong Acked-by: Hans Verkuil Signed-off-by: Russell King Signed-off-by: Neil Armstrong --- 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
Re: [PATCH] drm/bridge: dw_hdmi: add cec notifier support
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 > >> > >> 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 > >> Acked-by: Neil Armstrong > >> Acked-by: Hans Verkuil > >> Signed-off-by: Russell King > >> Signed-off-by: Neil Armstrong > >> --- > >> 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. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
Re: [PATCH] drm/bridge: dw_hdmi: add cec notifier support
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 >> >> 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 >> Acked-by: Neil Armstrong >> Acked-by: Hans Verkuil >> Signed-off-by: Russell King >> Signed-off-by: Neil Armstrong >> --- >> 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
Re: [PATCH] drm/bridge: dw_hdmi: add cec notifier support
On 07/06/2017 12:43 PM, Jose Abreu wrote:
> Hi Neil,
>
>
> On 06-07-2017 11:33, Neil Armstrong wrote:
>> From: Russell King
>>
>> 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
>> Acked-by: Neil Armstrong
>> Acked-by: Hans Verkuil
>> Signed-off-by: Russell King
>> Signed-off-by: Neil Armstrong
>> ---
>> 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
>> #include
>>
>> +#include
>
> 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);
>> 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);
>
Re: [PATCH] drm/bridge: dw_hdmi: add cec notifier support
On Thu, Jul 06, 2017 at 12:33:06PM +0200, Neil Armstrong wrote: > From: Russell King > > 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 > Acked-by: Neil Armstrong > Acked-by: Hans Verkuil > Signed-off-by: Russell King > Signed-off-by: Neil Armstrong > --- > 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. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
Re: [PATCH] drm/bridge: dw_hdmi: add cec notifier support
Hi Neil,
On 06-07-2017 11:33, Neil Armstrong wrote:
> From: Russell King
>
> 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
> Acked-by: Neil Armstrong
> Acked-by: Hans Verkuil
> Signed-off-by: Russell King
> Signed-off-by: Neil Armstrong
> ---
> 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
> #include
>
> +#include
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);
> 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);
