Hi,
This series adds dw-hdmi CEC support. This is done in four stages:
1. Add cec-notifier support 2. Fix up the clkdis register support, as this register contains a clock disable bit for the CEC module. 3. Add the driver. 4. Remove definitions that are not required from dw-hdmi.h
The CEC driver has been updated to use the register accessors in the main driver - it would be nice if it was possible to use the regmap support directly, but there's some knowledge private to the main driver that's required to correctly access the registers. (I don't understand why the register stride isn't part of regmap.)
drivers/gpu/drm/bridge/synopsys/Kconfig | 10 + drivers/gpu/drm/bridge/synopsys/Makefile | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 326 ++++++++++++++++++++++++++ drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h | 19 ++ drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 93 +++++++- drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 46 +--- 6 files changed, 437 insertions(+), 58 deletions(-)
v2 - fix selection of CEC_NOTIFIER in Kconfig
On 08/02/17 15:27, Russell King - ARM Linux wrote:
On Wed, Aug 02, 2017 at 04:14:34PM +0300, Laurent Pinchart wrote:
Hi Hans,
On Wednesday 02 Aug 2017 08:47:23 Hans Verkuil wrote:
On 08/02/2017 12:32 AM, Laurent Pinchart wrote:
- cec_register_cec_notifier(cec->adap, cec->notify);
- return 0;
+}
+static int dw_hdmi_cec_remove(struct platform_device *pdev) +{
- struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
- cec_unregister_adapter(cec->adap);
- cec_notifier_put(cec->notify);
- return 0;
+}
+static struct platform_driver dw_hdmi_cec_driver = {
- .probe = dw_hdmi_cec_probe,
- .remove = dw_hdmi_cec_remove,
- .driver = {
.name = "dw-hdmi-cec",- },
+}; +module_platform_driver(dw_hdmi_cec_driver);
Is there a particular reason why this has to be a separate module instead of simply calling the CEC init/cleanup functions directly from the main dw-hdmi driver ?
Not all SoCs that use dw-hdmi also use the dw-hdmi CEC implementation. Some use their own implementation (amlogic).
Lovely. Of course we need to reinvent the wheel every time, where would the fun be otherwise ?
So by implementing the cec-notifier in the dw-hdmi driver and keeping dw- hdmi CEC separate you can easily choose whether or not you want to use this CEC driver or another SoC CEC driver.
I'm certainly fine with such a split, but I don't think it requires a separate platform_driver. We could use a similar approach as with the HDMI PHY that can also differ between SoCs. The PHY is identified at runtime when possible, and the SoC-specific glue code can override that with a few data fields and function pointers.
Excuse me if I completely lose interest in reworking the driver at this point, as it's enough of an effort to follow the churn in CEC from one kernel version to another. I'm not about to rewrite the driver and restart the review cycle from scratch and then have several iterations of having to update it as CEC continues to evolve.
Let's get this driver in mainline, and then if we want further changes we can do that later.
+1
I also don't think the phy idea applies here. CEC implementations range from tightly coupled to the HDMI implementation to a completely independent i2c device that is unrelated to any HDMI receiver/transmitter. And various shades in between those extremes.
The only thing that is needed is that the CEC device needs to be informed when an EDID is read and when the hotplug disappears. The CEC notifier does just that and it is already in use, so it is nothing new. No need to rework this.
Regards,
Hans
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/Kconfig | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig index 53e78d092d18..351db000599a 100644 --- a/drivers/gpu/drm/bridge/synopsys/Kconfig +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig @@ -2,6 +2,7 @@ config DRM_DW_HDMI tristate select DRM_KMS_HELPER select REGMAP_MMIO + select CEC_CORE if CEC_NOTIFIER
config DRM_DW_HDMI_AHB_AUDIO tristate "Synopsys Designware AHB Audio interface" diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead11242c4b9..82e55ee8e4fa 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 { @@ -175,6 +178,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 +1901,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); @@ -2119,11 +2125,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"); @@ -2376,6 +2387,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 @@ -2452,6 +2469,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 Hans,
On Wednesday 02 Aug 2017 15:34:33 Hans Verkuil wrote:
On 08/02/17 15:27, Russell King - ARM Linux wrote:
On Wed, Aug 02, 2017 at 04:14:34PM +0300, Laurent Pinchart wrote:
On Wednesday 02 Aug 2017 08:47:23 Hans Verkuil wrote:
On 08/02/2017 12:32 AM, Laurent Pinchart wrote:
- cec_register_cec_notifier(cec->adap, cec->notify);
- return 0;
+}
+static int dw_hdmi_cec_remove(struct platform_device *pdev) +{
- struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
- cec_unregister_adapter(cec->adap);
- cec_notifier_put(cec->notify);
- return 0;
+}
+static struct platform_driver dw_hdmi_cec_driver = {
- .probe = dw_hdmi_cec_probe,
- .remove = dw_hdmi_cec_remove,
- .driver = {
.name = "dw-hdmi-cec",- },
+}; +module_platform_driver(dw_hdmi_cec_driver);
Is there a particular reason why this has to be a separate module instead of simply calling the CEC init/cleanup functions directly from the main dw-hdmi driver ?
Not all SoCs that use dw-hdmi also use the dw-hdmi CEC implementation. Some use their own implementation (amlogic).
Lovely. Of course we need to reinvent the wheel every time, where would the fun be otherwise ?
So by implementing the cec-notifier in the dw-hdmi driver and keeping dw-hdmi CEC separate you can easily choose whether or not you want to use this CEC driver or another SoC CEC driver.
I'm certainly fine with such a split, but I don't think it requires a separate platform_driver. We could use a similar approach as with the HDMI PHY that can also differ between SoCs. The PHY is identified at runtime when possible, and the SoC-specific glue code can override that with a few data fields and function pointers.
Excuse me if I completely lose interest in reworking the driver at this point, as it's enough of an effort to follow the churn in CEC from one kernel version to another. I'm not about to rewrite the driver and restart the review cycle from scratch and then have several iterations of having to update it as CEC continues to evolve.
Let's get this driver in mainline, and then if we want further changes we can do that later.
+1
I also don't think the phy idea applies here. CEC implementations range from tightly coupled to the HDMI implementation to a completely independent i2c device that is unrelated to any HDMI receiver/transmitter. And various shades in between those extremes.
Then it should be even simpler. Instead of creating a separate platform device for the DW HDMI CEC, we can just call the DW HDMI CEC driver init/cleanup functions directly when the IP core implements CEC. When it doesn't, a separate device will be described in DT (at least to my understanding, otherwise where would it come from ?) and a separate driver would handle it.
The only thing that is needed is that the CEC device needs to be informed when an EDID is read and when the hotplug disappears. The CEC notifier does just that and it is already in use, so it is nothing new. No need to rework this.
The notifier implementation in patch 1/4 looks good to me (except for the missing cec_notifier_put() call that I mentioned in my review).
On 07/31/2017 04:29 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/Kconfig | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig index 53e78d092d18..351db000599a 100644 --- a/drivers/gpu/drm/bridge/synopsys/Kconfig +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig @@ -2,6 +2,7 @@ config DRM_DW_HDMI tristate select DRM_KMS_HELPER select REGMAP_MMIO
- select CEC_CORE if CEC_NOTIFIER
config DRM_DW_HDMI_AHB_AUDIO tristate "Synopsys Designware AHB Audio interface" diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead11242c4b9..82e55ee8e4fa 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 { @@ -175,6 +178,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 +1901,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);@@ -2119,11 +2125,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");
@@ -2376,6 +2387,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
@@ -2452,6 +2469,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);
Tested-by: Neil Armstrong narmstrong@baylibre.com Reviewed-by: Neil Armstrong narmstrong@baylibre.com
On Wed, Aug 02, 2017 at 05:22:54PM +0300, Laurent Pinchart wrote:
Then it should be even simpler. Instead of creating a separate platform device for the DW HDMI CEC, we can just call the DW HDMI CEC driver init/cleanup functions directly when the IP core implements CEC. When it doesn't, a separate device will be described in DT (at least to my understanding, otherwise where would it come from ?) and a separate driver would handle it.
Let me put this plainly, because my previous message seems not to have the desired effect.
I am not changing this driver again until it is merged.
On 07/31/2017 04:29 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
Acked-by: Hans Verkuil hans.verkuil@cisco.com
Regards,
Hans
drivers/gpu/drm/bridge/synopsys/Kconfig | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig index 53e78d092d18..351db000599a 100644 --- a/drivers/gpu/drm/bridge/synopsys/Kconfig +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig @@ -2,6 +2,7 @@ config DRM_DW_HDMI tristate select DRM_KMS_HELPER select REGMAP_MMIO
- select CEC_CORE if CEC_NOTIFIER
config DRM_DW_HDMI_AHB_AUDIO tristate "Synopsys Designware AHB Audio interface" diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead11242c4b9..82e55ee8e4fa 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 { @@ -175,6 +178,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 +1901,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);@@ -2119,11 +2125,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");
@@ -2376,6 +2387,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
@@ -2452,6 +2469,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);
On 07/31/2017 08:55 PM, Hans Verkuil wrote:
On 07/31/2017 04:29 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
Acked-by: Hans Verkuil hans.verkuil@cisco.com
Queued to drm-misc-next.
Thanks, Archit
Regards,
Hans
drivers/gpu/drm/bridge/synopsys/Kconfig | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig index 53e78d092d18..351db000599a 100644 --- a/drivers/gpu/drm/bridge/synopsys/Kconfig +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig @@ -2,6 +2,7 @@ config DRM_DW_HDMI tristate select DRM_KMS_HELPER select REGMAP_MMIO
select CEC_CORE if CEC_NOTIFIER
config DRM_DW_HDMI_AHB_AUDIO tristate "Synopsys Designware AHB Audio interface"
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead11242c4b9..82e55ee8e4fa 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 {
@@ -175,6 +178,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 +1901,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);@@ -2119,11 +2125,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");
@@ -2376,6 +2387,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
@@ -2452,6 +2469,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);
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Archit,
I posted a v3 that added a missing cec_notifier_put to the remove() function. Apparently you missed that. The chunk in question from the v3 patch is this:
@@ -2469,6 +2489,9 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi) /* Disable all interrupts */ hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0);
+ if (hdmi->cec_notifier) + cec_notifier_put(hdmi->cec_notifier); + clk_disable_unprepare(hdmi->iahb_clk); clk_disable_unprepare(hdmi->isfr_clk);
Can you add that or should I make a new patch?
Regards,
Hans
On 04/08/17 15:36, Archit Taneja wrote:
On 07/31/2017 08:55 PM, Hans Verkuil wrote:
On 07/31/2017 04:29 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
Acked-by: Hans Verkuil hans.verkuil@cisco.com
Queued to drm-misc-next.
Thanks, Archit
Regards,
Hans
drivers/gpu/drm/bridge/synopsys/Kconfig | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig index 53e78d092d18..351db000599a 100644 --- a/drivers/gpu/drm/bridge/synopsys/Kconfig +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig @@ -2,6 +2,7 @@ config DRM_DW_HDMI tristate select DRM_KMS_HELPER select REGMAP_MMIO
- select CEC_CORE if CEC_NOTIFIER config DRM_DW_HDMI_AHB_AUDIO tristate "Synopsys Designware AHB Audio interface"
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead11242c4b9..82e55ee8e4fa 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 {
@@ -175,6 +178,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 +1901,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);@@ -2119,11 +2125,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");@@ -2376,6 +2387,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@@ -2452,6 +2469,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);
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 08/05/2017 02:53 PM, Hans Verkuil wrote:
Archit,
I posted a v3 that added a missing cec_notifier_put to the remove() function. Apparently you missed that. The chunk in question from the v3 patch is this:
I did. Sorry, about that. There was an issue with my mailer.
@@ -2469,6 +2489,9 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi) /* Disable all interrupts */ hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0);
- if (hdmi->cec_notifier)
cec_notifier_put(hdmi->cec_notifier);- clk_disable_unprepare(hdmi->iahb_clk); clk_disable_unprepare(hdmi->isfr_clk);
Can you add that or should I make a new patch?
We aren't allowed to do rebases on drm-misc-next unless absolutely needed. A separate patch that adds the missing notifier_put() call in remove would be great to have.
I'll queue the patches #3/4 and #4/4 from the v3 revision.
Thanks, Archit
Regards,
Hans
On 04/08/17 15:36, Archit Taneja wrote:
On 07/31/2017 08:55 PM, Hans Verkuil wrote:
On 07/31/2017 04:29 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
Acked-by: Hans Verkuil hans.verkuil@cisco.com
Queued to drm-misc-next.
Thanks, Archit
Regards,
Hans
drivers/gpu/drm/bridge/synopsys/Kconfig | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig index 53e78d092d18..351db000599a 100644 --- a/drivers/gpu/drm/bridge/synopsys/Kconfig +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig @@ -2,6 +2,7 @@ config DRM_DW_HDMI tristate select DRM_KMS_HELPER select REGMAP_MMIO
- select CEC_CORE if CEC_NOTIFIER config DRM_DW_HDMI_AHB_AUDIO tristate "Synopsys Designware AHB Audio interface"
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead11242c4b9..82e55ee8e4fa 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 {
@@ -175,6 +178,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 +1901,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);@@ -2119,11 +2125,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");@@ -2376,6 +2387,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@@ -2452,6 +2469,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);
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Russell,
Thank you for the patch.
On Monday 31 Jul 2017 15:29:41 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/Kconfig | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-)
[snip]
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead11242c4b9..82e55ee8e4fa 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
[snip]
@@ -2376,6 +2387,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
@@ -2452,6 +2469,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);
I'm not very familiar yet with the CEC API so I made have missed something, but shouldn't you call cec_notifier_put() in the remove() handler ?
On 08/02/17 16:11, Laurent Pinchart wrote:
Hi Russell,
Thank you for the patch.
On Monday 31 Jul 2017 15:29:41 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/Kconfig | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-)
[snip]
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead11242c4b9..82e55ee8e4fa 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
[snip]
@@ -2376,6 +2387,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
@@ -2452,6 +2469,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);
I'm not very familiar yet with the CEC API so I made have missed something, but shouldn't you call cec_notifier_put() in the remove() handler ?
Yes, you should. Well spotted, I missed that one.
Regards,
Hans
On Wed, Aug 02, 2017 at 04:17:15PM +0200, Hans Verkuil wrote:
Yes, you should. Well spotted, I missed that one.
Great, another respin. I give up.
The video setup path aways sets the clock disable register to a specific value, which has the effect of disabling the CEC engine. When we add the CEC driver, this becomes a problem.
Fix this by only setting/clearing the bits that the video path needs to.
Reviewed-by: Jose Abreu joabreu@synopsys.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 82e55ee8e4fa..b08cc0c95590 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -166,6 +166,7 @@ struct dw_hdmi { bool bridge_is_on; /* indicates the bridge is on */ bool rxsense; /* rxsense state */ u8 phy_mask; /* desired phy int mask settings */ + u8 mc_clkdis; /* clock disable register */
spinlock_t audio_lock; struct mutex audio_mutex; @@ -551,8 +552,11 @@ EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable) { - hdmi_modb(hdmi, enable ? 0 : HDMI_MC_CLKDIS_AUDCLK_DISABLE, - HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS); + if (enable) + hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_AUDCLK_DISABLE; + else + hdmi->mc_clkdis |= HDMI_MC_CLKDIS_AUDCLK_DISABLE; + hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS); }
static void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi) @@ -1574,8 +1578,6 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi, /* HDMI Initialization Step B.4 */ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi) { - u8 clkdis; - /* control period minimum duration */ hdmi_writeb(hdmi, 12, HDMI_FC_CTRLDUR); hdmi_writeb(hdmi, 32, HDMI_FC_EXCTRLDUR); @@ -1587,17 +1589,21 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi) hdmi_writeb(hdmi, 0x21, HDMI_FC_CH2PREAM);
/* Enable pixel clock and tmds data path */ - clkdis = 0x7F; - clkdis &= ~HDMI_MC_CLKDIS_PIXELCLK_DISABLE; - hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS); + hdmi->mc_clkdis |= HDMI_MC_CLKDIS_HDCPCLK_DISABLE | + HDMI_MC_CLKDIS_CSCCLK_DISABLE | + HDMI_MC_CLKDIS_AUDCLK_DISABLE | + HDMI_MC_CLKDIS_PREPCLK_DISABLE | + HDMI_MC_CLKDIS_TMDSCLK_DISABLE; + hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_PIXELCLK_DISABLE; + hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
- clkdis &= ~HDMI_MC_CLKDIS_TMDSCLK_DISABLE; - hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS); + hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_TMDSCLK_DISABLE; + hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
/* Enable csc path */ if (is_color_space_conversion(hdmi)) { - clkdis &= ~HDMI_MC_CLKDIS_CSCCLK_DISABLE; - hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS); + hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_CSCCLK_DISABLE; + hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS); }
/* Enable color space conversion if needed */ @@ -2272,6 +2278,7 @@ __dw_hdmi_probe(struct platform_device *pdev, hdmi->disabled = true; hdmi->rxsense = true; hdmi->phy_mask = (u8)~(HDMI_PHY_HPD | HDMI_PHY_RX_SENSE); + hdmi->mc_clkdis = 0x7f;
mutex_init(&hdmi->mutex); mutex_init(&hdmi->audio_mutex);
On 07/31/2017 04:29 PM, Russell King wrote:
The video setup path aways sets the clock disable register to a specific value, which has the effect of disabling the CEC engine. When we add the CEC driver, this becomes a problem.
Fix this by only setting/clearing the bits that the video path needs to.
Reviewed-by: Jose Abreu joabreu@synopsys.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk
Acked-by: Hans Verkuil hans.verkuil@cisco.com
Regards,
Hans
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 82e55ee8e4fa..b08cc0c95590 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -166,6 +166,7 @@ struct dw_hdmi { bool bridge_is_on; /* indicates the bridge is on */ bool rxsense; /* rxsense state */ u8 phy_mask; /* desired phy int mask settings */
u8 mc_clkdis; /* clock disable register */
spinlock_t audio_lock; struct mutex audio_mutex;
@@ -551,8 +552,11 @@ EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable) {
- hdmi_modb(hdmi, enable ? 0 : HDMI_MC_CLKDIS_AUDCLK_DISABLE,
HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
- if (enable)
hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_AUDCLK_DISABLE;- else
hdmi->mc_clkdis |= HDMI_MC_CLKDIS_AUDCLK_DISABLE;- hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
}
static void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi) @@ -1574,8 +1578,6 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi, /* HDMI Initialization Step B.4 */ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi) {
- u8 clkdis;
- /* control period minimum duration */ hdmi_writeb(hdmi, 12, HDMI_FC_CTRLDUR); hdmi_writeb(hdmi, 32, HDMI_FC_EXCTRLDUR);
@@ -1587,17 +1589,21 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi) hdmi_writeb(hdmi, 0x21, HDMI_FC_CH2PREAM);
/* Enable pixel clock and tmds data path */
- clkdis = 0x7F;
- clkdis &= ~HDMI_MC_CLKDIS_PIXELCLK_DISABLE;
- hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS);
- hdmi->mc_clkdis |= HDMI_MC_CLKDIS_HDCPCLK_DISABLE |
HDMI_MC_CLKDIS_CSCCLK_DISABLE |HDMI_MC_CLKDIS_AUDCLK_DISABLE |HDMI_MC_CLKDIS_PREPCLK_DISABLE |HDMI_MC_CLKDIS_TMDSCLK_DISABLE;- hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_PIXELCLK_DISABLE;
- hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
- clkdis &= ~HDMI_MC_CLKDIS_TMDSCLK_DISABLE;
- hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS);
hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_TMDSCLK_DISABLE;
hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
/* Enable csc path */ if (is_color_space_conversion(hdmi)) {
clkdis &= ~HDMI_MC_CLKDIS_CSCCLK_DISABLE;hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS);
hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_CSCCLK_DISABLE;hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);}
/* Enable color space conversion if needed */
@@ -2272,6 +2278,7 @@ __dw_hdmi_probe(struct platform_device *pdev, hdmi->disabled = true; hdmi->rxsense = true; hdmi->phy_mask = (u8)~(HDMI_PHY_HPD | HDMI_PHY_RX_SENSE);
hdmi->mc_clkdis = 0x7f;
mutex_init(&hdmi->mutex); mutex_init(&hdmi->audio_mutex);
On 07/31/2017 08:56 PM, Hans Verkuil wrote:
On 07/31/2017 04:29 PM, Russell King wrote:
The video setup path aways sets the clock disable register to a specific value, which has the effect of disabling the CEC engine. When we add the CEC driver, this becomes a problem.
Fix this by only setting/clearing the bits that the video path needs to.
Reviewed-by: Jose Abreu joabreu@synopsys.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk
Acked-by: Hans Verkuil hans.verkuil@cisco.com
Queued to drm-misc-next.
Thanks, Archit
Regards,
Hans
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 82e55ee8e4fa..b08cc0c95590 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -166,6 +166,7 @@ struct dw_hdmi { bool bridge_is_on; /* indicates the bridge is on */ bool rxsense; /* rxsense state */ u8 phy_mask; /* desired phy int mask settings */
u8 mc_clkdis; /* clock disable register */
spinlock_t audio_lock; struct mutex audio_mutex;
@@ -551,8 +552,11 @@ EXPORT_SYMBOL_GPL(dw_hdmi_set_sample_rate);
static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi, bool enable) {
- hdmi_modb(hdmi, enable ? 0 : HDMI_MC_CLKDIS_AUDCLK_DISABLE,
HDMI_MC_CLKDIS_AUDCLK_DISABLE, HDMI_MC_CLKDIS);
if (enable)
hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_AUDCLK_DISABLE;else
hdmi->mc_clkdis |= HDMI_MC_CLKDIS_AUDCLK_DISABLE;hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS); }
static void dw_hdmi_ahb_audio_enable(struct dw_hdmi *hdmi)
@@ -1574,8 +1578,6 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi, /* HDMI Initialization Step B.4 */ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi) {
- u8 clkdis;
- /* control period minimum duration */ hdmi_writeb(hdmi, 12, HDMI_FC_CTRLDUR); hdmi_writeb(hdmi, 32, HDMI_FC_EXCTRLDUR);
@@ -1587,17 +1589,21 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi) hdmi_writeb(hdmi, 0x21, HDMI_FC_CH2PREAM);
/* Enable pixel clock and tmds data path */
- clkdis = 0x7F;
- clkdis &= ~HDMI_MC_CLKDIS_PIXELCLK_DISABLE;
- hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS);
- hdmi->mc_clkdis |= HDMI_MC_CLKDIS_HDCPCLK_DISABLE |
HDMI_MC_CLKDIS_CSCCLK_DISABLE |HDMI_MC_CLKDIS_AUDCLK_DISABLE |HDMI_MC_CLKDIS_PREPCLK_DISABLE |HDMI_MC_CLKDIS_TMDSCLK_DISABLE;- hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_PIXELCLK_DISABLE;
- hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
- clkdis &= ~HDMI_MC_CLKDIS_TMDSCLK_DISABLE;
- hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS);
hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_TMDSCLK_DISABLE;
hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
/* Enable csc path */ if (is_color_space_conversion(hdmi)) {
clkdis &= ~HDMI_MC_CLKDIS_CSCCLK_DISABLE;hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS);
hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_CSCCLK_DISABLE;hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);}
/* Enable color space conversion if needed */
@@ -2272,6 +2278,7 @@ __dw_hdmi_probe(struct platform_device *pdev, hdmi->disabled = true; hdmi->rxsense = true; hdmi->phy_mask = (u8)~(HDMI_PHY_HPD | HDMI_PHY_RX_SENSE);
hdmi->mc_clkdis = 0x7f;
mutex_init(&hdmi->mutex); mutex_init(&hdmi->audio_mutex);
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Add a CEC driver for the dw-hdmi hardware.
Reviewed-by: Neil Armstrong narmstrong@baylibre.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/bridge/synopsys/Kconfig | 9 + drivers/gpu/drm/bridge/synopsys/Makefile | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 326 ++++++++++++++++++++++++++ drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h | 19 ++ drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 42 +++- drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 1 + 6 files changed, 397 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h
diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig index 351db000599a..d34c3dc04ba9 100644 --- a/drivers/gpu/drm/bridge/synopsys/Kconfig +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig @@ -23,3 +23,12 @@ config DRM_DW_HDMI_I2S_AUDIO help Support the I2S Audio interface which is part of the Synopsys Designware HDMI block. + +config DRM_DW_HDMI_CEC + tristate "Synopsis Designware CEC interface" + depends on DRM_DW_HDMI + select CEC_CORE + select CEC_NOTIFIER + help + Support the CE interface which is part of the Synopsys + Designware HDMI block. diff --git a/drivers/gpu/drm/bridge/synopsys/Makefile b/drivers/gpu/drm/bridge/synopsys/Makefile index 17aa7a65b57e..6fe415903668 100644 --- a/drivers/gpu/drm/bridge/synopsys/Makefile +++ b/drivers/gpu/drm/bridge/synopsys/Makefile @@ -3,3 +3,4 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o obj-$(CONFIG_DRM_DW_HDMI_I2S_AUDIO) += dw-hdmi-i2s-audio.o +obj-$(CONFIG_DRM_DW_HDMI_CEC) += dw-hdmi-cec.o diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c new file mode 100644 index 000000000000..52c9d93b9602 --- /dev/null +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c @@ -0,0 +1,326 @@ +/* + * Designware HDMI CEC driver + * + * Copyright (C) 2015-2017 Russell King. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/sched.h> +#include <linux/slab.h> + +#include <drm/drm_edid.h> + +#include <media/cec.h> +#include <media/cec-notifier.h> + +#include "dw-hdmi-cec.h" + +enum { + HDMI_IH_CEC_STAT0 = 0x0106, + HDMI_IH_MUTE_CEC_STAT0 = 0x0186, + + HDMI_CEC_CTRL = 0x7d00, + CEC_CTRL_START = BIT(0), + CEC_CTRL_FRAME_TYP = 3 << 1, + CEC_CTRL_RETRY = 0 << 1, + CEC_CTRL_NORMAL = 1 << 1, + CEC_CTRL_IMMED = 2 << 1, + + HDMI_CEC_STAT = 0x7d01, + CEC_STAT_DONE = BIT(0), + CEC_STAT_EOM = BIT(1), + CEC_STAT_NACK = BIT(2), + CEC_STAT_ARBLOST = BIT(3), + CEC_STAT_ERROR_INIT = BIT(4), + CEC_STAT_ERROR_FOLL = BIT(5), + CEC_STAT_WAKEUP = BIT(6), + + HDMI_CEC_MASK = 0x7d02, + HDMI_CEC_POLARITY = 0x7d03, + HDMI_CEC_INT = 0x7d04, + HDMI_CEC_ADDR_L = 0x7d05, + HDMI_CEC_ADDR_H = 0x7d06, + HDMI_CEC_TX_CNT = 0x7d07, + HDMI_CEC_RX_CNT = 0x7d08, + HDMI_CEC_TX_DATA0 = 0x7d10, + HDMI_CEC_RX_DATA0 = 0x7d20, + HDMI_CEC_LOCK = 0x7d30, + HDMI_CEC_WKUPCTRL = 0x7d31, +}; + +struct dw_hdmi_cec { + struct dw_hdmi *hdmi; + const struct dw_hdmi_cec_ops *ops; + u32 addresses; + struct cec_adapter *adap; + struct cec_msg rx_msg; + unsigned int tx_status; + bool tx_done; + bool rx_done; + struct cec_notifier *notify; + int irq; +}; + +static void dw_hdmi_write(struct dw_hdmi_cec *cec, u8 val, int offset) +{ + cec->ops->write(cec->hdmi, val, offset); +} + +static u8 dw_hdmi_read(struct dw_hdmi_cec *cec, int offset) +{ + return cec->ops->read(cec->hdmi, offset); +} + +static int dw_hdmi_cec_log_addr(struct cec_adapter *adap, u8 logical_addr) +{ + struct dw_hdmi_cec *cec = cec_get_drvdata(adap); + + if (logical_addr == CEC_LOG_ADDR_INVALID) + cec->addresses = 0; + else + cec->addresses |= BIT(logical_addr) | BIT(15); + + dw_hdmi_write(cec, cec->addresses & 255, HDMI_CEC_ADDR_L); + dw_hdmi_write(cec, cec->addresses >> 8, HDMI_CEC_ADDR_H); + + return 0; +} + +static int dw_hdmi_cec_transmit(struct cec_adapter *adap, u8 attempts, + u32 signal_free_time, struct cec_msg *msg) +{ + struct dw_hdmi_cec *cec = cec_get_drvdata(adap); + unsigned i, ctrl; + + switch (signal_free_time) { + case CEC_SIGNAL_FREE_TIME_RETRY: + ctrl = CEC_CTRL_RETRY; + break; + case CEC_SIGNAL_FREE_TIME_NEW_INITIATOR: + default: + ctrl = CEC_CTRL_NORMAL; + break; + case CEC_SIGNAL_FREE_TIME_NEXT_XFER: + ctrl = CEC_CTRL_IMMED; + break; + } + + for (i = 0; i < msg->len; i++) + dw_hdmi_write(cec, msg->msg[i], HDMI_CEC_TX_DATA0 + i); + + dw_hdmi_write(cec, msg->len, HDMI_CEC_TX_CNT); + dw_hdmi_write(cec, ctrl | CEC_CTRL_START, HDMI_CEC_CTRL); + + return 0; +} + +static irqreturn_t dw_hdmi_cec_hardirq(int irq, void *data) +{ + struct cec_adapter *adap = data; + struct dw_hdmi_cec *cec = cec_get_drvdata(adap); + unsigned stat = dw_hdmi_read(cec, HDMI_IH_CEC_STAT0); + irqreturn_t ret = IRQ_HANDLED; + + if (stat == 0) + return IRQ_NONE; + + dw_hdmi_write(cec, stat, HDMI_IH_CEC_STAT0); + + if (stat & CEC_STAT_ERROR_INIT) { + cec->tx_status = CEC_TX_STATUS_ERROR; + cec->tx_done = true; + ret = IRQ_WAKE_THREAD; + } else if (stat & CEC_STAT_DONE) { + cec->tx_status = CEC_TX_STATUS_OK; + cec->tx_done = true; + ret = IRQ_WAKE_THREAD; + } else if (stat & CEC_STAT_NACK) { + cec->tx_status = CEC_TX_STATUS_NACK; + cec->tx_done = true; + ret = IRQ_WAKE_THREAD; + } + + if (stat & CEC_STAT_EOM) { + unsigned len, i; + + len = dw_hdmi_read(cec, HDMI_CEC_RX_CNT); + if (len > sizeof(cec->rx_msg.msg)) + len = sizeof(cec->rx_msg.msg); + + for (i = 0; i < len; i++) + cec->rx_msg.msg[i] = + dw_hdmi_read(cec, HDMI_CEC_RX_DATA0 + i); + + dw_hdmi_write(cec, 0, HDMI_CEC_LOCK); + + cec->rx_msg.len = len; + smp_wmb(); + cec->rx_done = true; + + ret = IRQ_WAKE_THREAD; + } + + return ret; +} + +static irqreturn_t dw_hdmi_cec_thread(int irq, void *data) +{ + struct cec_adapter *adap = data; + struct dw_hdmi_cec *cec = cec_get_drvdata(adap); + + if (cec->tx_done) { + cec->tx_done = false; + cec_transmit_done(adap, cec->tx_status, 0, 0, 0, 0); + } + if (cec->rx_done) { + cec->rx_done = false; + smp_rmb(); + cec_received_msg(adap, &cec->rx_msg); + } + return IRQ_HANDLED; +} + +static int dw_hdmi_cec_enable(struct cec_adapter *adap, bool enable) +{ + struct dw_hdmi_cec *cec = cec_get_drvdata(adap); + + if (!enable) { + dw_hdmi_write(cec, ~0, HDMI_CEC_MASK); + dw_hdmi_write(cec, ~0, HDMI_IH_MUTE_CEC_STAT0); + dw_hdmi_write(cec, 0, HDMI_CEC_POLARITY); + + cec->ops->disable(cec->hdmi); + } else { + unsigned irqs; + + dw_hdmi_write(cec, 0, HDMI_CEC_CTRL); + dw_hdmi_write(cec, ~0, HDMI_IH_CEC_STAT0); + dw_hdmi_write(cec, 0, HDMI_CEC_LOCK); + + dw_hdmi_cec_log_addr(cec->adap, CEC_LOG_ADDR_INVALID); + + cec->ops->enable(cec->hdmi); + + irqs = CEC_STAT_ERROR_INIT | CEC_STAT_NACK | CEC_STAT_EOM | + CEC_STAT_DONE; + dw_hdmi_write(cec, irqs, HDMI_CEC_POLARITY); + dw_hdmi_write(cec, ~irqs, HDMI_CEC_MASK); + dw_hdmi_write(cec, ~irqs, HDMI_IH_MUTE_CEC_STAT0); + } + return 0; +} + +static const struct cec_adap_ops dw_hdmi_cec_ops = { + .adap_enable = dw_hdmi_cec_enable, + .adap_log_addr = dw_hdmi_cec_log_addr, + .adap_transmit = dw_hdmi_cec_transmit, +}; + +static void dw_hdmi_cec_del(void *data) +{ + struct dw_hdmi_cec *cec = data; + + cec_delete_adapter(cec->adap); +} + +static int dw_hdmi_cec_probe(struct platform_device *pdev) +{ + struct dw_hdmi_cec_data *data = dev_get_platdata(&pdev->dev); + struct dw_hdmi_cec *cec; + int ret; + + if (!data) + return -ENXIO; + + /* + * Our device is just a convenience - we want to link to the real + * hardware device here, so that userspace can see the association + * between the HDMI hardware and its associated CEC chardev. + */ + cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL); + if (!cec) + return -ENOMEM; + + cec->irq = data->irq; + cec->ops = data->ops; + cec->hdmi = data->hdmi; + + platform_set_drvdata(pdev, cec); + + dw_hdmi_write(cec, 0, HDMI_CEC_TX_CNT); + dw_hdmi_write(cec, ~0, HDMI_CEC_MASK); + dw_hdmi_write(cec, ~0, HDMI_IH_MUTE_CEC_STAT0); + dw_hdmi_write(cec, 0, HDMI_CEC_POLARITY); + + cec->adap = cec_allocate_adapter(&dw_hdmi_cec_ops, cec, "dw_hdmi", + CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT | + CEC_CAP_RC, CEC_MAX_LOG_ADDRS); + if (IS_ERR(cec->adap)) + return PTR_ERR(cec->adap); + + /* override the module pointer */ + cec->adap->owner = THIS_MODULE; + + ret = devm_add_action(&pdev->dev, dw_hdmi_cec_del, cec); + if (ret) { + cec_delete_adapter(cec->adap); + return ret; + } + + ret = devm_request_threaded_irq(&pdev->dev, cec->irq, + dw_hdmi_cec_hardirq, + dw_hdmi_cec_thread, IRQF_SHARED, + "dw-hdmi-cec", cec->adap); + if (ret < 0) + return ret; + + cec->notify = cec_notifier_get(pdev->dev.parent); + if (!cec->notify) + return -ENOMEM; + + ret = cec_register_adapter(cec->adap, pdev->dev.parent); + if (ret < 0) { + cec_notifier_put(cec->notify); + return ret; + } + + /* + * CEC documentation says we must not call cec_delete_adapter + * after a successful call to cec_register_adapter(). + */ + devm_remove_action(&pdev->dev, dw_hdmi_cec_del, cec); + + cec_register_cec_notifier(cec->adap, cec->notify); + + return 0; +} + +static int dw_hdmi_cec_remove(struct platform_device *pdev) +{ + struct dw_hdmi_cec *cec = platform_get_drvdata(pdev); + + cec_unregister_adapter(cec->adap); + cec_notifier_put(cec->notify); + + return 0; +} + +static struct platform_driver dw_hdmi_cec_driver = { + .probe = dw_hdmi_cec_probe, + .remove = dw_hdmi_cec_remove, + .driver = { + .name = "dw-hdmi-cec", + }, +}; +module_platform_driver(dw_hdmi_cec_driver); + +MODULE_AUTHOR("Russell King rmk+kernel@armlinux.org.uk"); +MODULE_DESCRIPTION("Synopsys Designware HDMI CEC driver for i.MX"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS(PLATFORM_MODULE_PREFIX "dw-hdmi-cec"); diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h new file mode 100644 index 000000000000..cf4dc121a2c4 --- /dev/null +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h @@ -0,0 +1,19 @@ +#ifndef DW_HDMI_CEC_H +#define DW_HDMI_CEC_H + +struct dw_hdmi; + +struct dw_hdmi_cec_ops { + void (*write)(struct dw_hdmi *hdmi, u8 val, int offset); + u8 (*read)(struct dw_hdmi *hdmi, int offset); + void (*enable)(struct dw_hdmi *hdmi); + void (*disable)(struct dw_hdmi *hdmi); +}; + +struct dw_hdmi_cec_data { + struct dw_hdmi *hdmi; + const struct dw_hdmi_cec_ops *ops; + int irq; +}; + +#endif diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index b08cc0c95590..63386976501d 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -35,6 +35,7 @@
#include "dw-hdmi.h" #include "dw-hdmi-audio.h" +#include "dw-hdmi-cec.h"
#include <media/cec-notifier.h>
@@ -133,6 +134,7 @@ struct dw_hdmi { unsigned int version;
struct platform_device *audio; + struct platform_device *cec; struct device *dev; struct clk *isfr_clk; struct clk *iahb_clk; @@ -1794,7 +1796,6 @@ static void initialize_hdmi_ih_mutes(struct dw_hdmi *hdmi) hdmi_writeb(hdmi, 0xff, HDMI_AUD_HBR_MASK); hdmi_writeb(hdmi, 0xff, HDMI_GP_MASK); hdmi_writeb(hdmi, 0xff, HDMI_A_APIINTMSK); - hdmi_writeb(hdmi, 0xff, HDMI_CEC_MASK); hdmi_writeb(hdmi, 0xff, HDMI_I2CM_INT); hdmi_writeb(hdmi, 0xff, HDMI_I2CM_CTLINT);
@@ -2236,6 +2237,29 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) return -ENODEV; }
+static void dw_hdmi_cec_enable(struct dw_hdmi *hdmi) +{ + mutex_lock(&hdmi->mutex); + hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_CECCLK_DISABLE; + hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS); + mutex_unlock(&hdmi->mutex); +} + +static void dw_hdmi_cec_disable(struct dw_hdmi *hdmi) +{ + mutex_lock(&hdmi->mutex); + hdmi->mc_clkdis |= HDMI_MC_CLKDIS_CECCLK_DISABLE; + hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS); + mutex_unlock(&hdmi->mutex); +} + +static const struct dw_hdmi_cec_ops dw_hdmi_cec_ops = { + .write = hdmi_writeb, + .read = hdmi_readb, + .enable = dw_hdmi_cec_enable, + .disable = dw_hdmi_cec_disable, +}; + static const struct regmap_config hdmi_regmap_8bit_config = { .reg_bits = 32, .val_bits = 8, @@ -2258,6 +2282,7 @@ __dw_hdmi_probe(struct platform_device *pdev, struct device_node *np = dev->of_node; struct platform_device_info pdevinfo; struct device_node *ddc_node; + struct dw_hdmi_cec_data cec; struct dw_hdmi *hdmi; struct resource *iores = NULL; int irq; @@ -2462,6 +2487,19 @@ __dw_hdmi_probe(struct platform_device *pdev, hdmi->audio = platform_device_register_full(&pdevinfo); }
+ if (config0 & HDMI_CONFIG0_CEC) { + cec.hdmi = hdmi; + cec.ops = &dw_hdmi_cec_ops; + cec.irq = irq; + + pdevinfo.name = "dw-hdmi-cec"; + pdevinfo.data = &cec; + pdevinfo.size_data = sizeof(cec); + pdevinfo.dma_mask = 0; + + hdmi->cec = platform_device_register_full(&pdevinfo); + } + /* Reset HDMI DDC I2C master controller and mute I2CM interrupts */ if (hdmi->i2c) dw_hdmi_i2c_init(hdmi); @@ -2492,6 +2530,8 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi) { if (hdmi->audio && !IS_ERR(hdmi->audio)) platform_device_unregister(hdmi->audio); + if (!IS_ERR(hdmi->cec)) + platform_device_unregister(hdmi->cec);
/* Disable all interrupts */ hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0); diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index c59f87e1483e..69644c83a0f8 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h @@ -555,6 +555,7 @@ enum {
/* CONFIG0_ID field values */ HDMI_CONFIG0_I2S = 0x10, + HDMI_CONFIG0_CEC = 0x02,
/* CONFIG1_ID field values */ HDMI_CONFIG1_AHB = 0x01,
Hi Russell,
Just two small comments below.
After that's fixed you can add my:
Acked-by: Hans Verkuil hans.verkuil@cisco.com
and:
Tested-by: Hans Verkuil hans.verkuil@cisco.com
(on my SolidRun Cubox-i).
On 07/31/2017 04:29 PM, Russell King wrote:
Add a CEC driver for the dw-hdmi hardware.
Reviewed-by: Neil Armstrong narmstrong@baylibre.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk
drivers/gpu/drm/bridge/synopsys/Kconfig | 9 + drivers/gpu/drm/bridge/synopsys/Makefile | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 326 ++++++++++++++++++++++++++ drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h | 19 ++ drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 42 +++- drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 1 + 6 files changed, 397 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h
<snip>
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c new file mode 100644 index 000000000000..52c9d93b9602 --- /dev/null +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c @@ -0,0 +1,326 @@
<snip>
+static irqreturn_t dw_hdmi_cec_thread(int irq, void *data) +{
- struct cec_adapter *adap = data;
- struct dw_hdmi_cec *cec = cec_get_drvdata(adap);
- if (cec->tx_done) {
cec->tx_done = false;cec_transmit_done(adap, cec->tx_status, 0, 0, 0, 0);
Replace by:
cec_transmit_attempt_done(adap, cec->tx_status);
This gets the counters right.
- }
- if (cec->rx_done) {
cec->rx_done = false;smp_rmb();cec_received_msg(adap, &cec->rx_msg);- }
- return IRQ_HANDLED;
+}
+static int dw_hdmi_cec_probe(struct platform_device *pdev) +{
- struct dw_hdmi_cec_data *data = dev_get_platdata(&pdev->dev);
- struct dw_hdmi_cec *cec;
- int ret;
- if (!data)
return -ENXIO;- /*
* Our device is just a convenience - we want to link to the real* hardware device here, so that userspace can see the association* between the HDMI hardware and its associated CEC chardev.*/- cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL);
- if (!cec)
return -ENOMEM;- cec->irq = data->irq;
- cec->ops = data->ops;
- cec->hdmi = data->hdmi;
- platform_set_drvdata(pdev, cec);
- dw_hdmi_write(cec, 0, HDMI_CEC_TX_CNT);
- dw_hdmi_write(cec, ~0, HDMI_CEC_MASK);
- dw_hdmi_write(cec, ~0, HDMI_IH_MUTE_CEC_STAT0);
- dw_hdmi_write(cec, 0, HDMI_CEC_POLARITY);
- cec->adap = cec_allocate_adapter(&dw_hdmi_cec_ops, cec, "dw_hdmi",
CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |CEC_CAP_RC, CEC_MAX_LOG_ADDRS);
Add the missing CEC_CAP_PASSTHROUGH capability.
I think I am going to add a new define to media/cec.h:
#define CEC_CAP_DEFAULTS (CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT | \ CEC_CAP_PASSTHROUGH | CEC_CAP_RC)
to simplify this since these are standard capabilities.
But that's something I'll do later. For now just add CEC_CAP_PASSTHROUGH.
- if (IS_ERR(cec->adap))
return PTR_ERR(cec->adap);- /* override the module pointer */
- cec->adap->owner = THIS_MODULE;
- ret = devm_add_action(&pdev->dev, dw_hdmi_cec_del, cec);
- if (ret) {
cec_delete_adapter(cec->adap);return ret;- }
- ret = devm_request_threaded_irq(&pdev->dev, cec->irq,
dw_hdmi_cec_hardirq,dw_hdmi_cec_thread, IRQF_SHARED,"dw-hdmi-cec", cec->adap);- if (ret < 0)
return ret;- cec->notify = cec_notifier_get(pdev->dev.parent);
- if (!cec->notify)
return -ENOMEM;- ret = cec_register_adapter(cec->adap, pdev->dev.parent);
- if (ret < 0) {
cec_notifier_put(cec->notify);return ret;- }
- /*
* CEC documentation says we must not call cec_delete_adapter* after a successful call to cec_register_adapter().*/- devm_remove_action(&pdev->dev, dw_hdmi_cec_del, cec);
- cec_register_cec_notifier(cec->adap, cec->notify);
- return 0;
+}
<snip>
Thank you for writing this driver!
Hans
Hi Russell,
Thank you for the patch.
On Monday 31 Jul 2017 15:29:51 Russell King wrote:
Add a CEC driver for the dw-hdmi hardware.
Reviewed-by: Neil Armstrong narmstrong@baylibre.com Signed-off-by: Russell King rmk+kernel@armlinux.org.uk
drivers/gpu/drm/bridge/synopsys/Kconfig | 9 + drivers/gpu/drm/bridge/synopsys/Makefile | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 326 +++++++++++++++++++++++ drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h | 19 ++ drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 42 +++- drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 1 + 6 files changed, 397 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h
[snip]
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c new file mode 100644 index 000000000000..52c9d93b9602 --- /dev/null +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
[snip]
+static int dw_hdmi_cec_probe(struct platform_device *pdev) +{
- struct dw_hdmi_cec_data *data = dev_get_platdata(&pdev->dev);
- struct dw_hdmi_cec *cec;
- int ret;
- if (!data)
return -ENXIO;- /*
* Our device is just a convenience - we want to link to the real* hardware device here, so that userspace can see the association* between the HDMI hardware and its associated CEC chardev.*/- cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL);
- if (!cec)
return -ENOMEM;- cec->irq = data->irq;
- cec->ops = data->ops;
- cec->hdmi = data->hdmi;
- platform_set_drvdata(pdev, cec);
- dw_hdmi_write(cec, 0, HDMI_CEC_TX_CNT);
- dw_hdmi_write(cec, ~0, HDMI_CEC_MASK);
- dw_hdmi_write(cec, ~0, HDMI_IH_MUTE_CEC_STAT0);
- dw_hdmi_write(cec, 0, HDMI_CEC_POLARITY);
- cec->adap = cec_allocate_adapter(&dw_hdmi_cec_ops, cec, "dw_hdmi",
CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT
|
CEC_CAP_RC, CEC_MAX_LOG_ADDRS);- if (IS_ERR(cec->adap))
return PTR_ERR(cec->adap);- /* override the module pointer */
- cec->adap->owner = THIS_MODULE;
- ret = devm_add_action(&pdev->dev, dw_hdmi_cec_del, cec);
- if (ret) {
cec_delete_adapter(cec->adap);return ret;- }
- ret = devm_request_threaded_irq(&pdev->dev, cec->irq,
dw_hdmi_cec_hardirq,dw_hdmi_cec_thread, IRQF_SHARED,"dw-hdmi-cec", cec->adap);- if (ret < 0)
return ret;- cec->notify = cec_notifier_get(pdev->dev.parent);
- if (!cec->notify)
return -ENOMEM;- ret = cec_register_adapter(cec->adap, pdev->dev.parent);
- if (ret < 0) {
cec_notifier_put(cec->notify);return ret;- }
- /*
* CEC documentation says we must not call cec_delete_adapter* after a successful call to cec_register_adapter().*/- devm_remove_action(&pdev->dev, dw_hdmi_cec_del, cec);
dw_hdmi_cec_del() is only used to clean up in the error path of the probe function. It would be simpler and less resource-consuming to add an error label to this function instead of using devm.
- cec_register_cec_notifier(cec->adap, cec->notify);
- return 0;
+}
+static int dw_hdmi_cec_remove(struct platform_device *pdev) +{
- struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
- cec_unregister_adapter(cec->adap);
- cec_notifier_put(cec->notify);
- return 0;
+}
+static struct platform_driver dw_hdmi_cec_driver = {
- .probe = dw_hdmi_cec_probe,
- .remove = dw_hdmi_cec_remove,
- .driver = {
.name = "dw-hdmi-cec",- },
+}; +module_platform_driver(dw_hdmi_cec_driver);
Is there a particular reason why this has to be a separate module instead of simply calling the CEC init/cleanup functions directly from the main dw-hdmi driver ?
+MODULE_AUTHOR("Russell King rmk+kernel@armlinux.org.uk"); +MODULE_DESCRIPTION("Synopsys Designware HDMI CEC driver for i.MX"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS(PLATFORM_MODULE_PREFIX "dw-hdmi-cec");
On 08/02/2017 12:32 AM, Laurent Pinchart wrote:
- cec_register_cec_notifier(cec->adap, cec->notify);
- return 0;
+}
+static int dw_hdmi_cec_remove(struct platform_device *pdev) +{
- struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
- cec_unregister_adapter(cec->adap);
- cec_notifier_put(cec->notify);
- return 0;
+}
+static struct platform_driver dw_hdmi_cec_driver = {
- .probe = dw_hdmi_cec_probe,
- .remove = dw_hdmi_cec_remove,
- .driver = {
.name = "dw-hdmi-cec",- },
+}; +module_platform_driver(dw_hdmi_cec_driver);
Is there a particular reason why this has to be a separate module instead of simply calling the CEC init/cleanup functions directly from the main dw-hdmi driver ?
Not all SoCs that use dw-hdmi also use the dw-hdmi CEC implementation. Some use their own implementation (amlogic). So by implementing the cec-notifier in the dw-hdmi driver and keeping dw-hdmi CEC separate you can easily choose whether or not you want to use this CEC driver or another SoC CEC driver.
Regards,
Hans
+MODULE_AUTHOR("Russell King rmk+kernel@armlinux.org.uk"); +MODULE_DESCRIPTION("Synopsys Designware HDMI CEC driver for i.MX"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS(PLATFORM_MODULE_PREFIX "dw-hdmi-cec");
Hi Hans,
On Wednesday 02 Aug 2017 08:47:23 Hans Verkuil wrote:
On 08/02/2017 12:32 AM, Laurent Pinchart wrote:
- cec_register_cec_notifier(cec->adap, cec->notify);
- return 0;
+}
+static int dw_hdmi_cec_remove(struct platform_device *pdev) +{
- struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
- cec_unregister_adapter(cec->adap);
- cec_notifier_put(cec->notify);
- return 0;
+}
+static struct platform_driver dw_hdmi_cec_driver = {
- .probe = dw_hdmi_cec_probe,
- .remove = dw_hdmi_cec_remove,
- .driver = {
.name = "dw-hdmi-cec",- },
+}; +module_platform_driver(dw_hdmi_cec_driver);
Is there a particular reason why this has to be a separate module instead of simply calling the CEC init/cleanup functions directly from the main dw-hdmi driver ?
Not all SoCs that use dw-hdmi also use the dw-hdmi CEC implementation. Some use their own implementation (amlogic).
Lovely. Of course we need to reinvent the wheel every time, where would the fun be otherwise ?
So by implementing the cec-notifier in the dw-hdmi driver and keeping dw- hdmi CEC separate you can easily choose whether or not you want to use this CEC driver or another SoC CEC driver.
I'm certainly fine with such a split, but I don't think it requires a separate platform_driver. We could use a similar approach as with the HDMI PHY that can also differ between SoCs. The PHY is identified at runtime when possible, and the SoC-specific glue code can override that with a few data fields and function pointers.
+MODULE_AUTHOR("Russell King rmk+kernel@armlinux.org.uk"); +MODULE_DESCRIPTION("Synopsys Designware HDMI CEC driver for i.MX"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS(PLATFORM_MODULE_PREFIX "dw-hdmi-cec");
On Wed, Aug 02, 2017 at 04:14:34PM +0300, Laurent Pinchart wrote:
Hi Hans,
On Wednesday 02 Aug 2017 08:47:23 Hans Verkuil wrote:
On 08/02/2017 12:32 AM, Laurent Pinchart wrote:
- cec_register_cec_notifier(cec->adap, cec->notify);
- return 0;
+}
+static int dw_hdmi_cec_remove(struct platform_device *pdev) +{
- struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
- cec_unregister_adapter(cec->adap);
- cec_notifier_put(cec->notify);
- return 0;
+}
+static struct platform_driver dw_hdmi_cec_driver = {
- .probe = dw_hdmi_cec_probe,
- .remove = dw_hdmi_cec_remove,
- .driver = {
.name = "dw-hdmi-cec",- },
+}; +module_platform_driver(dw_hdmi_cec_driver);
Is there a particular reason why this has to be a separate module instead of simply calling the CEC init/cleanup functions directly from the main dw-hdmi driver ?
Not all SoCs that use dw-hdmi also use the dw-hdmi CEC implementation. Some use their own implementation (amlogic).
Lovely. Of course we need to reinvent the wheel every time, where would the fun be otherwise ?
So by implementing the cec-notifier in the dw-hdmi driver and keeping dw- hdmi CEC separate you can easily choose whether or not you want to use this CEC driver or another SoC CEC driver.
I'm certainly fine with such a split, but I don't think it requires a separate platform_driver. We could use a similar approach as with the HDMI PHY that can also differ between SoCs. The PHY is identified at runtime when possible, and the SoC-specific glue code can override that with a few data fields and function pointers.
Excuse me if I completely lose interest in reworking the driver at this point, as it's enough of an effort to follow the churn in CEC from one kernel version to another. I'm not about to rewrite the driver and restart the review cycle from scratch and then have several iterations of having to update it as CEC continues to evolve.
Let's get this driver in mainline, and then if we want further changes we can do that later.
On 08/02/2017 03:14 PM, Laurent Pinchart wrote:
Hi Hans,
On Wednesday 02 Aug 2017 08:47:23 Hans Verkuil wrote:
On 08/02/2017 12:32 AM, Laurent Pinchart wrote:
- cec_register_cec_notifier(cec->adap, cec->notify);
- return 0;
+}
+static int dw_hdmi_cec_remove(struct platform_device *pdev) +{
- struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
- cec_unregister_adapter(cec->adap);
- cec_notifier_put(cec->notify);
- return 0;
+}
+static struct platform_driver dw_hdmi_cec_driver = {
- .probe = dw_hdmi_cec_probe,
- .remove = dw_hdmi_cec_remove,
- .driver = {
.name = "dw-hdmi-cec",- },
+}; +module_platform_driver(dw_hdmi_cec_driver);
Is there a particular reason why this has to be a separate module instead of simply calling the CEC init/cleanup functions directly from the main dw-hdmi driver ?
Not all SoCs that use dw-hdmi also use the dw-hdmi CEC implementation. Some use their own implementation (amlogic).
Lovely. Of course we need to reinvent the wheel every time, where would the fun be otherwise ?
So by implementing the cec-notifier in the dw-hdmi driver and keeping dw- hdmi CEC separate you can easily choose whether or not you want to use this CEC driver or another SoC CEC driver.
I'm certainly fine with such a split, but I don't think it requires a separate platform_driver. We could use a similar approach as with the HDMI PHY that can also differ between SoCs. The PHY is identified at runtime when possible, and the SoC-specific glue code can override that with a few data fields and function pointers.
The HDMI CEC is in the Synopsys IP like the I2S or AHB Audio, and is optional. So it's legitimate to handle it in a similar fashion. Amlogic has a separate custom CEC implementation, having an optional driver makes it possible to avoid loading unnecessary code like the AHB audio driver for non-freescale platforms.
+MODULE_AUTHOR("Russell King rmk+kernel@armlinux.org.uk"); +MODULE_DESCRIPTION("Synopsys Designware HDMI CEC driver for i.MX"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS(PLATFORM_MODULE_PREFIX "dw-hdmi-cec");
Yes let's get this merged !
Neil
We don't need the CEC engine register definitions, so let's remove them.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 45 ------------------------------- 1 file changed, 45 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index 69644c83a0f8..9d90eb9c46e5 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h @@ -478,51 +478,6 @@ #define HDMI_A_PRESETUP 0x501A #define HDMI_A_SRM_BASE 0x5020
-/* CEC Engine Registers */ -#define HDMI_CEC_CTRL 0x7D00 -#define HDMI_CEC_STAT 0x7D01 -#define HDMI_CEC_MASK 0x7D02 -#define HDMI_CEC_POLARITY 0x7D03 -#define HDMI_CEC_INT 0x7D04 -#define HDMI_CEC_ADDR_L 0x7D05 -#define HDMI_CEC_ADDR_H 0x7D06 -#define HDMI_CEC_TX_CNT 0x7D07 -#define HDMI_CEC_RX_CNT 0x7D08 -#define HDMI_CEC_TX_DATA0 0x7D10 -#define HDMI_CEC_TX_DATA1 0x7D11 -#define HDMI_CEC_TX_DATA2 0x7D12 -#define HDMI_CEC_TX_DATA3 0x7D13 -#define HDMI_CEC_TX_DATA4 0x7D14 -#define HDMI_CEC_TX_DATA5 0x7D15 -#define HDMI_CEC_TX_DATA6 0x7D16 -#define HDMI_CEC_TX_DATA7 0x7D17 -#define HDMI_CEC_TX_DATA8 0x7D18 -#define HDMI_CEC_TX_DATA9 0x7D19 -#define HDMI_CEC_TX_DATA10 0x7D1a -#define HDMI_CEC_TX_DATA11 0x7D1b -#define HDMI_CEC_TX_DATA12 0x7D1c -#define HDMI_CEC_TX_DATA13 0x7D1d -#define HDMI_CEC_TX_DATA14 0x7D1e -#define HDMI_CEC_TX_DATA15 0x7D1f -#define HDMI_CEC_RX_DATA0 0x7D20 -#define HDMI_CEC_RX_DATA1 0x7D21 -#define HDMI_CEC_RX_DATA2 0x7D22 -#define HDMI_CEC_RX_DATA3 0x7D23 -#define HDMI_CEC_RX_DATA4 0x7D24 -#define HDMI_CEC_RX_DATA5 0x7D25 -#define HDMI_CEC_RX_DATA6 0x7D26 -#define HDMI_CEC_RX_DATA7 0x7D27 -#define HDMI_CEC_RX_DATA8 0x7D28 -#define HDMI_CEC_RX_DATA9 0x7D29 -#define HDMI_CEC_RX_DATA10 0x7D2a -#define HDMI_CEC_RX_DATA11 0x7D2b -#define HDMI_CEC_RX_DATA12 0x7D2c -#define HDMI_CEC_RX_DATA13 0x7D2d -#define HDMI_CEC_RX_DATA14 0x7D2e -#define HDMI_CEC_RX_DATA15 0x7D2f -#define HDMI_CEC_LOCK 0x7D30 -#define HDMI_CEC_WKUPCTRL 0x7D31 - /* I2C Master Registers (E-DDC) */ #define HDMI_I2CM_SLAVE 0x7E00 #define HDMI_I2CM_ADDRESS 0x7E01
On 07/31/2017 04:29 PM, Russell King wrote:
We don't need the CEC engine register definitions, so let's remove them.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk
Acked-by: Hans Verkuil hans.verkuil@cisco.com
Regards,
Hans
drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 45 ------------------------------- 1 file changed, 45 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index 69644c83a0f8..9d90eb9c46e5 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h @@ -478,51 +478,6 @@ #define HDMI_A_PRESETUP 0x501A #define HDMI_A_SRM_BASE 0x5020
-/* CEC Engine Registers */ -#define HDMI_CEC_CTRL 0x7D00 -#define HDMI_CEC_STAT 0x7D01 -#define HDMI_CEC_MASK 0x7D02 -#define HDMI_CEC_POLARITY 0x7D03 -#define HDMI_CEC_INT 0x7D04 -#define HDMI_CEC_ADDR_L 0x7D05 -#define HDMI_CEC_ADDR_H 0x7D06 -#define HDMI_CEC_TX_CNT 0x7D07 -#define HDMI_CEC_RX_CNT 0x7D08 -#define HDMI_CEC_TX_DATA0 0x7D10 -#define HDMI_CEC_TX_DATA1 0x7D11 -#define HDMI_CEC_TX_DATA2 0x7D12 -#define HDMI_CEC_TX_DATA3 0x7D13 -#define HDMI_CEC_TX_DATA4 0x7D14 -#define HDMI_CEC_TX_DATA5 0x7D15 -#define HDMI_CEC_TX_DATA6 0x7D16 -#define HDMI_CEC_TX_DATA7 0x7D17 -#define HDMI_CEC_TX_DATA8 0x7D18 -#define HDMI_CEC_TX_DATA9 0x7D19 -#define HDMI_CEC_TX_DATA10 0x7D1a -#define HDMI_CEC_TX_DATA11 0x7D1b -#define HDMI_CEC_TX_DATA12 0x7D1c -#define HDMI_CEC_TX_DATA13 0x7D1d -#define HDMI_CEC_TX_DATA14 0x7D1e -#define HDMI_CEC_TX_DATA15 0x7D1f -#define HDMI_CEC_RX_DATA0 0x7D20 -#define HDMI_CEC_RX_DATA1 0x7D21 -#define HDMI_CEC_RX_DATA2 0x7D22 -#define HDMI_CEC_RX_DATA3 0x7D23 -#define HDMI_CEC_RX_DATA4 0x7D24 -#define HDMI_CEC_RX_DATA5 0x7D25 -#define HDMI_CEC_RX_DATA6 0x7D26 -#define HDMI_CEC_RX_DATA7 0x7D27 -#define HDMI_CEC_RX_DATA8 0x7D28 -#define HDMI_CEC_RX_DATA9 0x7D29 -#define HDMI_CEC_RX_DATA10 0x7D2a -#define HDMI_CEC_RX_DATA11 0x7D2b -#define HDMI_CEC_RX_DATA12 0x7D2c -#define HDMI_CEC_RX_DATA13 0x7D2d -#define HDMI_CEC_RX_DATA14 0x7D2e -#define HDMI_CEC_RX_DATA15 0x7D2f -#define HDMI_CEC_LOCK 0x7D30 -#define HDMI_CEC_WKUPCTRL 0x7D31
/* I2C Master Registers (E-DDC) */ #define HDMI_I2CM_SLAVE 0x7E00 #define HDMI_I2CM_ADDRESS 0x7E01
Hi Russell,
Thank you for the patches.
On Monday 31 Jul 2017 15:29:06 Russell King - ARM Linux wrote:
Hi,
This series adds dw-hdmi CEC support. This is done in four stages:
- Add cec-notifier support
- Fix up the clkdis register support, as this register contains a clock disable bit for the CEC module.
- Add the driver.
- Remove definitions that are not required from dw-hdmi.h
The CEC driver has been updated to use the register accessors in the main driver - it would be nice if it was possible to use the regmap support directly, but there's some knowledge private to the main driver that's required to correctly access the registers. (I don't understand why the register stride isn't part of regmap.)
drivers/gpu/drm/bridge/synopsys/Kconfig | 10 + drivers/gpu/drm/bridge/synopsys/Makefile | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 326 +++++++++++++++++++++++ drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h | 19 ++ drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 93 +++++++- drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 46 +--- 6 files changed, 437 insertions(+), 58 deletions(-)
v2 - fix selection of CEC_NOTIFIER in Kconfig
For the whole series,
Tested-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
on R-Car H3.
The DWC HDMI controller in this SoC supports CEC, but I have no CEC-enabled sink, so tests were limited to regression checks.
dri-devel@lists.freedesktop.org
-
Archit Taneja -
Hans Verkuil -
Laurent Pinchart -
Neil Armstrong -
Russell King -
Russell King - ARM Linux