From: hverkuil@xs4all.nl (Hans Verkuil) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 2/4] drm/bridge: dw-hdmi: add cec notifier support Date: Fri, 9 Jun 2017 16:04:20 +0200 [thread overview] Message-ID: <34525119-5700-76fc-fcf4-5d90bd674b77@xs4all.nl> (raw) In-Reply-To: <5b2fdadd-f1fa-c454-b3fe-facf221beb78@baylibre.com> On 09/06/17 15:56, Neil Armstrong wrote: > On 06/09/2017 03:38 PM, Russell King - ARM Linux wrote: >> On Fri, Jun 09, 2017 at 02:59:20PM +0200, Neil Armstrong wrote: >>> On 05/30/2017 04:23 PM, Russell King wrote: >>>> Add CEC notifier support to the HDMI bridge driver, so that the CEC >>>> part of the IP can receive its physical address. >>>> >>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> >>>> --- >>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 22 +++++++++++++++++++++- >>>> 1 file changed, 21 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >>>> index 4e1f54a675d8..966422576c44 100644 >>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >>>> @@ -36,7 +36,10 @@ >>>> #include "dw-hdmi.h" >>>> #include "dw-hdmi-audio.h" >>>> >>>> +#include <media/cec-notifier.h> >>>> + >>>> #define DDC_SEGMENT_ADDR 0x30 >>>> + >>>> #define HDMI_EDID_LEN 512 >>>> >>>> enum hdmi_datamap { >>>> @@ -173,6 +176,8 @@ struct dw_hdmi { >>>> >>>> unsigned int reg_shift; >>>> struct regmap *regm; >>>> + >>>> + struct cec_notifier *cec_notifier; >>>> }; >>>> >>>> #define HDMI_IH_PHY_STAT0_RX_SENSE \ >>>> @@ -1870,6 +1875,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); >>>> @@ -2108,11 +2114,16 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id) >>>> * ask the source to re-read the EDID. >>>> */ >>>> if (intr_stat & >>>> - (HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) >>>> + (HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) { >>>> __dw_hdmi_setup_rx_sense(hdmi, >>>> phy_stat & HDMI_PHY_HPD, >>>> phy_stat & HDMI_PHY_RX_SENSE); >>>> >>>> + if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) >>>> + cec_notifier_set_phys_addr(hdmi->cec_notifier, >>>> + CEC_PHYS_ADDR_INVALID); >>>> + } >>>> + >>>> if (intr_stat & HDMI_IH_PHY_STAT0_HPD) { >>>> dev_dbg(hdmi->dev, "EVENT=%s\n", >>>> phy_int_pol & HDMI_PHY_HPD ? "plugin" : "plugout"); >>>> @@ -2365,6 +2376,12 @@ __dw_hdmi_probe(struct platform_device *pdev, >>>> 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 >>>> @@ -2437,6 +2454,9 @@ __dw_hdmi_probe(struct platform_device *pdev, >>>> 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 Archit, >>> >>> I think this one could go through drm-next since it's quite >>> standalone and will reduce the DW-HDMI CEC patchset and dependencies. >> >> Not a good idea. If you read all the comments, Hans is suggesting that >> CEC should be part of dw-hdmi itself, not stand-alone. That would mean >> this patch probably changes - basically, with CEC support built-in to >> dw-hdmi, we don't need the notifier stuff. > > Hi Russell, > > Yes, but on the Amlogic Meson plarform, the DW-HDMI CEC controller is not used, > but a custom one, so this notifier is actually useful for this platform and > maybe others. > >> >> So, I'd suggest _not_ merging it at the moment, because the patch could >> well become obsolete. > > 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. > >> >> Wait until the CEC changes that Hans has talked about have hit mainline >> and I've had a chance to rework this for those. We're waiting on Mauro >> for that at the moment. >> >> (I do find it rather frustrating that CEC seems to evolve very rapidly, >> it makes it quite difficult to publish a patch set, and get it merged.) >> > > Should we really wait until I push the Amlogic AO CEC driver ? Having a > notifier in the DW-HDMI driver won't harm anybody since it *will be used*. I'm OK with the notifier in this case. Regards, Hans
WARNING: multiple messages have this Message-ID (diff)
From: Hans Verkuil <hverkuil@xs4all.nl> To: Neil Armstrong <narmstrong@baylibre.com>, Russell King - ARM Linux <linux@armlinux.org.uk> Cc: linux-arm-kernel@lists.infradead.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH 2/4] drm/bridge: dw-hdmi: add cec notifier support Date: Fri, 9 Jun 2017 16:04:20 +0200 [thread overview] Message-ID: <34525119-5700-76fc-fcf4-5d90bd674b77@xs4all.nl> (raw) In-Reply-To: <5b2fdadd-f1fa-c454-b3fe-facf221beb78@baylibre.com> On 09/06/17 15:56, Neil Armstrong wrote: > On 06/09/2017 03:38 PM, Russell King - ARM Linux wrote: >> On Fri, Jun 09, 2017 at 02:59:20PM +0200, Neil Armstrong wrote: >>> On 05/30/2017 04:23 PM, Russell King wrote: >>>> Add CEC notifier support to the HDMI bridge driver, so that the CEC >>>> part of the IP can receive its physical address. >>>> >>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> >>>> --- >>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 22 +++++++++++++++++++++- >>>> 1 file changed, 21 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >>>> index 4e1f54a675d8..966422576c44 100644 >>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >>>> @@ -36,7 +36,10 @@ >>>> #include "dw-hdmi.h" >>>> #include "dw-hdmi-audio.h" >>>> >>>> +#include <media/cec-notifier.h> >>>> + >>>> #define DDC_SEGMENT_ADDR 0x30 >>>> + >>>> #define HDMI_EDID_LEN 512 >>>> >>>> enum hdmi_datamap { >>>> @@ -173,6 +176,8 @@ struct dw_hdmi { >>>> >>>> unsigned int reg_shift; >>>> struct regmap *regm; >>>> + >>>> + struct cec_notifier *cec_notifier; >>>> }; >>>> >>>> #define HDMI_IH_PHY_STAT0_RX_SENSE \ >>>> @@ -1870,6 +1875,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); >>>> @@ -2108,11 +2114,16 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id) >>>> * ask the source to re-read the EDID. >>>> */ >>>> if (intr_stat & >>>> - (HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) >>>> + (HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) { >>>> __dw_hdmi_setup_rx_sense(hdmi, >>>> phy_stat & HDMI_PHY_HPD, >>>> phy_stat & HDMI_PHY_RX_SENSE); >>>> >>>> + if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) >>>> + cec_notifier_set_phys_addr(hdmi->cec_notifier, >>>> + CEC_PHYS_ADDR_INVALID); >>>> + } >>>> + >>>> if (intr_stat & HDMI_IH_PHY_STAT0_HPD) { >>>> dev_dbg(hdmi->dev, "EVENT=%s\n", >>>> phy_int_pol & HDMI_PHY_HPD ? "plugin" : "plugout"); >>>> @@ -2365,6 +2376,12 @@ __dw_hdmi_probe(struct platform_device *pdev, >>>> 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 >>>> @@ -2437,6 +2454,9 @@ __dw_hdmi_probe(struct platform_device *pdev, >>>> 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 Archit, >>> >>> I think this one could go through drm-next since it's quite >>> standalone and will reduce the DW-HDMI CEC patchset and dependencies. >> >> Not a good idea. If you read all the comments, Hans is suggesting that >> CEC should be part of dw-hdmi itself, not stand-alone. That would mean >> this patch probably changes - basically, with CEC support built-in to >> dw-hdmi, we don't need the notifier stuff. > > Hi Russell, > > Yes, but on the Amlogic Meson plarform, the DW-HDMI CEC controller is not used, > but a custom one, so this notifier is actually useful for this platform and > maybe others. > >> >> So, I'd suggest _not_ merging it at the moment, because the patch could >> well become obsolete. > > 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. > >> >> Wait until the CEC changes that Hans has talked about have hit mainline >> and I've had a chance to rework this for those. We're waiting on Mauro >> for that at the moment. >> >> (I do find it rather frustrating that CEC seems to evolve very rapidly, >> it makes it quite difficult to publish a patch set, and get it merged.) >> > > Should we really wait until I push the Amlogic AO CEC driver ? Having a > notifier in the DW-HDMI driver won't harm anybody since it *will be used*. I'm OK with the notifier in this case. Regards, Hans _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2017-06-09 14:04 UTC|newest] Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-05-30 14:22 [PATCH 0/4] dw-hdmi CEC support Russell King - ARM Linux 2017-05-30 14:22 ` Russell King - ARM Linux 2017-05-30 14:23 ` [PATCH 1/4] drm/bridge: dw-hdmi: remove CEC engine register definitions Russell King 2017-05-30 14:23 ` Russell King 2017-06-09 14:24 ` Hans Verkuil 2017-06-09 14:24 ` Hans Verkuil 2017-05-30 14:23 ` [PATCH 2/4] drm/bridge: dw-hdmi: add cec notifier support Russell King 2017-05-30 14:23 ` Russell King 2017-06-09 12:59 ` Neil Armstrong 2017-06-09 12:59 ` Neil Armstrong 2017-06-09 13:38 ` Russell King - ARM Linux 2017-06-09 13:38 ` Russell King - ARM Linux 2017-06-09 13:51 ` Hans Verkuil 2017-06-09 13:51 ` Hans Verkuil 2017-06-09 13:56 ` Neil Armstrong 2017-06-09 13:56 ` Neil Armstrong 2017-06-09 14:04 ` Hans Verkuil [this message] 2017-06-09 14:04 ` Hans Verkuil 2017-06-09 14:10 ` Russell King - ARM Linux 2017-06-09 14:10 ` Russell King - ARM Linux 2017-06-09 14:38 ` Hans Verkuil 2017-06-09 14:38 ` Hans Verkuil 2017-07-17 8:56 ` Hans Verkuil 2017-07-17 8:56 ` Hans Verkuil 2017-07-17 9:05 ` Russell King - ARM Linux 2017-07-17 9:05 ` Russell King - ARM Linux 2017-07-17 11:19 ` Hans Verkuil 2017-07-17 11:19 ` Hans Verkuil 2017-07-17 11:39 ` Hans Verkuil 2017-07-17 11:39 ` Hans Verkuil 2017-07-17 12:05 ` Russell King - ARM Linux 2017-07-17 12:05 ` Russell King - ARM Linux 2017-07-17 12:23 ` Hans Verkuil 2017-07-17 12:23 ` Hans Verkuil 2017-07-24 12:16 ` Hans Verkuil 2017-07-24 12:16 ` Hans Verkuil 2017-07-24 13:07 ` Russell King - ARM Linux 2017-07-24 13:07 ` Russell King - ARM Linux 2017-07-24 16:34 ` Russell King - ARM Linux 2017-07-24 16:34 ` Russell King - ARM Linux 2017-06-09 14:27 ` Hans Verkuil 2017-06-09 14:27 ` Hans Verkuil 2017-05-30 14:23 ` [PATCH 3/4] drm/bridge: dw-hdmi: add better clock disable control Russell King 2017-05-30 14:23 ` Russell King 2017-06-01 0:55 ` Jose Abreu 2017-06-01 0:55 ` Jose Abreu 2017-06-09 14:28 ` Hans Verkuil 2017-06-09 14:28 ` Hans Verkuil 2017-05-30 14:23 ` [PATCH 4/4] drm/bridge: dw-hdmi: add cec driver Russell King 2017-05-30 14:23 ` Russell King 2017-06-01 0:53 ` Jose Abreu 2017-06-01 0:53 ` Jose Abreu 2017-06-01 22:30 ` Russell King - ARM Linux 2017-06-01 22:30 ` Russell King - ARM Linux 2017-06-02 5:02 ` Jose Abreu 2017-06-02 5:02 ` Jose Abreu 2017-06-02 9:15 ` Russell King - ARM Linux 2017-06-02 9:15 ` Russell King - ARM Linux 2017-06-02 9:28 ` Hans Verkuil 2017-06-02 9:28 ` Hans Verkuil 2017-06-02 9:36 ` Russell King - ARM Linux 2017-06-02 9:36 ` Russell King - ARM Linux 2017-06-01 8:31 ` Hans Verkuil 2017-06-01 8:31 ` Hans Verkuil 2017-06-01 9:46 ` Russell King - ARM Linux 2017-06-01 9:46 ` Russell King - ARM Linux 2017-06-01 10:44 ` Hans Verkuil 2017-06-01 10:44 ` Hans Verkuil 2017-06-02 12:07 ` Russell King - ARM Linux 2017-06-02 12:07 ` Russell King - ARM Linux 2017-06-02 12:29 ` Hans Verkuil 2017-06-02 12:29 ` Hans Verkuil 2017-06-12 8:42 ` Hans Verkuil 2017-06-12 8:42 ` Hans Verkuil 2017-06-01 13:47 ` Neil Armstrong 2017-06-01 13:47 ` Neil Armstrong 2017-06-02 6:31 ` Hans Verkuil 2017-06-02 6:31 ` Hans Verkuil 2017-06-02 6:43 ` Jose Abreu 2017-06-02 6:43 ` Jose Abreu 2017-06-02 9:06 ` Hans Verkuil 2017-06-02 9:06 ` Hans Verkuil 2017-06-02 9:18 ` Russell King - ARM Linux 2017-06-02 9:18 ` Russell King - ARM Linux
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=34525119-5700-76fc-fcf4-5d90bd674b77@xs4all.nl \ --to=hverkuil@xs4all.nl \ --cc=linux-arm-kernel@lists.infradead.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.