Re: [PATCH v3 06/13] drm: bridge: Add LVDS encoder driver
On Thu, Mar 02, 2017 at 02:30:09AM +0200, Laurent Pinchart wrote: > Hi Daniel, > > On Wednesday 04 Jan 2017 17:13:23 Laurent Pinchart wrote: > > On Wednesday 04 Jan 2017 15:58:25 Daniel Vetter wrote: > > > On Wed, Jan 04, 2017 at 04:33:57PM +0200, Laurent Pinchart wrote: > > >> On Wednesday 04 Jan 2017 14:51:48 Daniel Vetter wrote: > > >>> Hm, something like drm_bridge_panel_bridge_init(dev, panel) should be > > >>> enough, or not? My idea is to use this for the case where the only > > >>> thing in dt is the panel, with no real bridge chip. And I think we > > >>> don't need anything beyond that one _init function, plus maybe some > > >>> additional paramaters ... > > >> > > >> There should be no bridge then. If you want the DRM core to manage > > >> panels automatically, then we should create specific helpers for that, > > >> not abuse the bridge infrastructure. Bridges should be instantiated from > > >> a hardware device and bound to drivers as usual. > > > > > > I guess that's the part where I disagree: Just because there's physically > > > no bridge doesn't mean we shouldn't just treat it as one in the software > > > abstraction. If it looks and acts like a bridge (even an empty one), then > > > imo it can be a bridge. > > > > > > If you insist on panels being panels, then I guess we need some other kind > > > of glue to bind them into arbitrary bridge chains. But given that the > > > callbacks match very closely, I don't see the point. > > > > > > In an idea world a panel would probably derive from a drm_bridge, but > > > we're not in that universe unfortunately ;-) > > > > Or both would derive from another object, but I agree that's how it should > > work. That's what I want to achieve, one step at a time. Creating dummy > > bridges isn't a step in that direction in my opinion, so I'd rather not do > > that, but work towards the right abstraction. > > Do you object getting this patch merged as-is as a first step in the right > direction ? :-) Well, I'm definitely no fan at all of typing code that's only there to satisfy some fairly arbitrary normative choices we attach to the words we pick for our abstraction. But I'm also not going to stop you, just don't complain if I ask the next dummy panel to reuse your LVDS bridge :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v2 17/29] drm: bridge: dw-hdmi: Refactor PHY power handling
Hi Jose,
On Wednesday 01 Mar 2017 16:25:46 Jose Abreu wrote:
> On 01-03-2017 11:09, Laurent Pinchart wrote:
> > On Thursday 05 Jan 2017 15:06:49 Jose Abreu wrote:
> >> On 05-01-2017 12:29, Laurent Pinchart wrote:
> >>> On Tuesday 20 Dec 2016 12:17:23 Jose Abreu wrote:
> On 20-12-2016 11:45, Russell King - ARM Linux wrote:
> > On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote:
> >> Instead of spreading version-dependent PHY power handling code
> >> around, group it in two functions to power the PHY on and off and use
> >> them through the driver.
> >>
> >> Powering off the PHY at the beginning of the setup phase is currently
> >> split in two locations for first and second generation PHYs, group
> >> all the operations in the dw_hdmi_phy_init() function.
> >
> > This changes the behaviour of the driver.
> >
> >> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
> >> +{
> >> + if (hdmi->phy->gen == 1) {
> >> + dw_hdmi_phy_enable_tmds(hdmi, 0);
> >> + dw_hdmi_phy_enable_powerdown(hdmi, true);
> >> + } else {
> >> + dw_hdmi_phy_gen2_txpwron(hdmi, 0);
> >> + dw_hdmi_phy_gen2_pddq(hdmi, 1);
> >> + }
> >> +}
> >> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi
> >> *hdmi)
> >>if (!hdmi->phy_enabled)
> >>return;
> >>
> >> - dw_hdmi_phy_enable_tmds(hdmi, 0);
> >> - dw_hdmi_phy_enable_powerdown(hdmi, true);
> >> -
> >> + dw_hdmi_phy_power_off(hdmi);
> >
> > This makes dw_hdmi_phy_disable() power down a gen2 phy.
> >
> > The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a
> > gen2 phy. I've been carrying this change for a while, which I've had
> > to revert (and finally expunge), as it causes problems on iMX6:
> >
> > @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi
> > *hdmi)
> > if (!hdmi->phy_enabled)
> > return;
> >
> > + /* Actually set the phy into low power mode */
> > + dw_hdmi_phy_gen2_txpwron(hdmi, 0);
> > +
> > + /* FIXME: We should wait for TX_READY to be deasserted */
> > +
> > + dw_hdmi_phy_gen2_pddq(hdmi, 1);
> > +
> > + /* This appears to have no effect on iMX6 */
> > dw_hdmi_phy_enable_tmds(hdmi, 0);
> > dw_hdmi_phy_enable_powerdown(hdmi, true);
> >
> > So, I think your change here will cause problems for iMX6.
> >
> > From what I remember, it seems that iMX6 has issues with RXSENSE/HPD
> > bouncing when the PHY is powered down. I can't promise when I'll be
> > able to check for that again.
>
> Indeed TX_READY must be low before asserting pddq.
> >>>
> >>> The TX_READY signal is documented in the i.MX6 datasheet as being a PHY
> >>> output signal, but there seems to be no HDMI TX register from which its
> >>> state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register
> >>> through I2C ? How long is the PHY expected to take to set TX_READY to 0
> >>> ?
> >>
> >> TX_READY can be read from register 0x1A of phy, BIT(2) (through
> >> I2C). Not sure if same offset for all phys though.
> >
> > I have been told that another option is to poll the TX_PHY_LOCK bit in the
> > phy_stat0 register. That would be a simpler solution that doesn't require
> > I2C access. Could you confirm (or disconfirm) this ?
>
> Yes (In our implementation we have TX_PHY_LOCK wired up to
> TX_READY that comes from phy. But if using a custom phy or an
> unusual implementation then this may not hold).
Thank you for the information. I've submitted a v4 that poll the TX_PHY_LOCK
bit. With the patch series applied the vendor PHY handling code already
delegates PHY power control to the glue code, there's thus no issue there. For
platforms using a Synopsys PHY, all the ones we have to support today have
TX_READY wired to the PHY lock signal so we should be good as well there. If
that doesn't remain true in the future we'll fix it.
> HPD and RXSENSE should work in power down mode by enabling enhpdrxsense
> bit in phy_conf0 BUT to enter power down you must disable TX_PWRON,
> enhpdrxsense and enable PDDQ and PHY_RESET. Only after doing this (and
> consequently entering power down mode) you can enable again
> enhpdrxsense.
> >>>
> >>> Thanks, I'll give it a try.
--
Regards,
Laurent Pinchart
Re: [PATCH v3 06/13] drm: bridge: Add LVDS encoder driver
Hi Daniel, On Wednesday 04 Jan 2017 17:13:23 Laurent Pinchart wrote: > On Wednesday 04 Jan 2017 15:58:25 Daniel Vetter wrote: > > On Wed, Jan 04, 2017 at 04:33:57PM +0200, Laurent Pinchart wrote: > >> On Wednesday 04 Jan 2017 14:51:48 Daniel Vetter wrote: > >>> Hm, something like drm_bridge_panel_bridge_init(dev, panel) should be > >>> enough, or not? My idea is to use this for the case where the only > >>> thing in dt is the panel, with no real bridge chip. And I think we > >>> don't need anything beyond that one _init function, plus maybe some > >>> additional paramaters ... > >> > >> There should be no bridge then. If you want the DRM core to manage > >> panels automatically, then we should create specific helpers for that, > >> not abuse the bridge infrastructure. Bridges should be instantiated from > >> a hardware device and bound to drivers as usual. > > > > I guess that's the part where I disagree: Just because there's physically > > no bridge doesn't mean we shouldn't just treat it as one in the software > > abstraction. If it looks and acts like a bridge (even an empty one), then > > imo it can be a bridge. > > > > If you insist on panels being panels, then I guess we need some other kind > > of glue to bind them into arbitrary bridge chains. But given that the > > callbacks match very closely, I don't see the point. > > > > In an idea world a panel would probably derive from a drm_bridge, but > > we're not in that universe unfortunately ;-) > > Or both would derive from another object, but I agree that's how it should > work. That's what I want to achieve, one step at a time. Creating dummy > bridges isn't a step in that direction in my opinion, so I'd rather not do > that, but work towards the right abstraction. Do you object getting this patch merged as-is as a first step in the right direction ? :-) -- Regards, Laurent Pinchart
[PATCH v3 1/3] watchdog: add rza_wdt driver
Adds a watchdog timer driver for the Renesas RZ/A Series SoCs. A reset
handler is also included since a WDT overflow is the only method for
restarting an RZ/A SoC.
Signed-off-by: Chris Brandt
---
drivers/watchdog/Kconfig | 8 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/rza_wdt.c | 208 +
3 files changed, 217 insertions(+)
create mode 100644 drivers/watchdog/rza_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index acb00b5..123c516 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -701,6 +701,14 @@ config RENESAS_WDT
This driver adds watchdog support for the integrated watchdogs in the
Renesas R-Car and other SH-Mobile SoCs (usually named RWDT or SWDT).
+config RENESAS_RZAWDT
+ tristate "Renesas RZ/A WDT Watchdog"
+ depends on ARCH_RENESAS || COMPILE_TEST
+ select WATCHDOG_CORE
+ help
+ This driver adds watchdog support for the integrated watchdogs in the
+ Renesas RZ/A SoCs. These watchdogs can be used to reset a system.
+
config ASPEED_WATCHDOG
tristate "Aspeed 2400 watchdog support"
depends on ARCH_ASPEED || COMPILE_TEST
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 0c3d35e..84b897c 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_LPC18XX_WATCHDOG) += lpc18xx_wdt.o
obj-$(CONFIG_BCM7038_WDT) += bcm7038_wdt.o
obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
+obj-$(CONFIG_RENESAS_RZAWDT) += rza_wdt.o
obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
# AVR32 Architecture
diff --git a/drivers/watchdog/rza_wdt.c b/drivers/watchdog/rza_wdt.c
new file mode 100644
index 000..17442c5
--- /dev/null
+++ b/drivers/watchdog/rza_wdt.c
@@ -0,0 +1,208 @@
+/*
+ * Renesas RZ/A Series WDT Driver
+ *
+ * Copyright (C) 2017 Renesas Electronics America, Inc.
+ * Copyright (C) 2017 Chris Brandt
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DEFAULT_TIMEOUT 30
+
+/* Watchdog Timer Registers */
+#define WTCSR 0
+#define WTCSR_MAGIC 0xA500
+#define WTSCR_WT (1<<6)
+#define WTSCR_TME (1<<5)
+#define WTSCR_CKS(i) i
+
+#define WTCNT 2
+#define WTCNT_MAGIC 0x5A00
+
+#define WRCSR 4
+#define WRCSR_MAGIC 0x5A00
+#define WRCSR_RSTE (1<<6)
+#define WRCSR_CLEAR_WOVF 0xA500 /* special value */
+
+struct rza_wdt {
+ struct watchdog_device wdev;
+ void __iomem *base;
+ struct clk *clk;
+};
+
+static int rza_wdt_start(struct watchdog_device *wdev)
+{
+ struct rza_wdt *priv = watchdog_get_drvdata(wdev);
+
+ /* Stop timer */
+ writew(WTCSR_MAGIC | 0, priv->base + WTCSR);
+
+ /* Must dummy read WRCSR:WOVF at least once before clearing */
+ readb(priv->base + WRCSR);
+ writew(WRCSR_CLEAR_WOVF, priv->base + WRCSR);
+
+ /*
+* Start timer with slowest clock source and reset option enabled.
+*/
+ writew(WRCSR_MAGIC | WRCSR_RSTE, priv->base + WRCSR);
+ writew(WTCNT_MAGIC | 0, priv->base + WTCNT);
+ writew(WTCSR_MAGIC | WTSCR_WT | WTSCR_TME | WTSCR_CKS(7),
+ priv->base + WTCSR);
+
+ return 0;
+}
+
+static int rza_wdt_stop(struct watchdog_device *wdev)
+{
+ struct rza_wdt *priv = watchdog_get_drvdata(wdev);
+
+ writew(WTCSR_MAGIC | 0, priv->base + WTCSR);
+
+ return 0;
+}
+
+static int rza_wdt_ping(struct watchdog_device *wdev)
+{
+ struct rza_wdt *priv = watchdog_get_drvdata(wdev);
+
+ writew(WTCNT_MAGIC | 0, priv->base + WTCNT);
+
+ return 0;
+}
+
+static int rza_wdt_restart(struct watchdog_device *wdev, unsigned long action,
+ void *data)
+{
+ struct rza_wdt *priv = watchdog_get_drvdata(wdev);
+
+ /* Stop timer */
+ writew(WTCSR_MAGIC | 0, priv->base + WTCSR);
+
+ /* Must dummy read WRCSR:WOVF at least once before clearing */
+ readb(priv->base + WRCSR);
+ writew(WRCSR_CLEAR_WOVF, priv->base + WRCSR);
+
+ /*
+* Start timer with fastest clock source and only 1 clock left before
+* overflow with reset option enabled.
+*/
+ writew(WRCSR_MAGIC | WRCSR_RSTE, priv->base + WRCSR);
+ writew(WTCNT_MAGIC | 255, priv->base + WTCNT);
+ writew(WTCSR_MAGIC | WTSCR_WT | WTSCR_TME, priv->base + WTCSR);
+
+ /*
+* Actually make sure the above sequence hits hardware before sleeping.
+*/
+ wmb();
+
+ /* Wait for WDT overflow (reset) */
+ udelay(20);
+
+ return 0;
+}
+
+static const struct watchdog_info rza_wdt_ident = {
+ .options = WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+ .identity = "Renesas RZ/A WDT Watchdo
[PATCH v3 2/3] watchdog: renesas-wdt: add support for rza
Describe the WDT hardware in the RZ/A series. Signed-off-by: Chris Brandt Reviewed-by: Geert Uytterhoeven Acked-by: Rob Herring --- v3: * Add Acked-by, Reviewed-by. v2: * added to renesas-wdt.txt instead of creating a new file * changed commit title * added "renesas,rza-wdt" as a fallback * added interrupts property --- Documentation/devicetree/bindings/watchdog/renesas-wdt.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt b/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt index da24e31..9e306af 100644 --- a/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt +++ b/Documentation/devicetree/bindings/watchdog/renesas-wdt.txt @@ -2,10 +2,11 @@ Renesas Watchdog Timer (WDT) Controller Required properties: - compatible : Should be "renesas,-wdt", and - "renesas,rcar-gen3-wdt" as fallback. + "renesas,rcar-gen3-wdt" or "renesas,rza-wdt" as fallback. Examples with soctypes are: - "renesas,r8a7795-wdt" (R-Car H3) - "renesas,r8a7796-wdt" (R-Car M3-W) +- "renesas,r7s72100-wdt" (RZ/A1) When compatible with the generic version, nodes must list the SoC-specific version corresponding to the platform first, followed by the generic @@ -17,6 +18,7 @@ Required properties: Optional properties: - timeout-sec : Contains the watchdog timeout in seconds - power-domains : the power domain the WDT belongs to +- interrupts: Some WDTs have an interrupt when used in interval timer mode Examples: -- 2.10.1
[PATCH v3 3/3] ARM: dts: r7s72100: Add watchdog timer
Add watchdog timer support for RZ/A1.
For the RZ/A1, the only way to do a reset is to overflow the WDT, so this
is useful even if you don't need the watchdog functionality.
Signed-off-by: Chris Brandt
Reviewed-by: Geert Uytterhoeven
---
v3:
* added Reviewed-by
v2:
* changed "renesas,r7s72100-reset" to "renesas,r7s72100-wdt"
* changed "renesas,wdt-reset" to "renesas,rza-wdt"
* added interupt property (even though it is not used)
* added clocks property
---
arch/arm/boot/dts/r7s72100.dtsi | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index ed62e19..6ecee72 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -382,6 +382,13 @@
cache-level = <2>;
};
+ wdt: timer@fcfe {
+ compatible = "renesas,r7s72100-wdt", "renesas,rza-wdt";
+ reg = <0xfcfe 0x6>;
+ interrupts = ;
+ clocks = <&p0_clk>;
+ };
+
i2c0: i2c@fcfee000 {
#address-cells = <1>;
#size-cells = <0>;
--
2.10.1
[PATCH v3 0/3] watchdog: add wdt and reset for renesas r7s72100
Some Renesas SoCs do not have a reset register and the only way to do a SW controlled reset is to use the watchdog timer. So while this series started out by only adding a reset feature, now it's a full watchdog timer driver that includes a reset handler. The longest WDT overflow you can get with a RZ/A1 (R7S72100) with its 8-bit wide counter is 125ms. Not very long. However, by setting max_hw_heartbeat_ms, the watchdog core will now handle pinging the WDT automatically and allow the user to set times as big as they want. Therefore, the default timeout for this driver is 30 seconds. Of course if system interrupts are disabled too long and wdt can't be pinged by the watchdog core, then the system will reset before the user specified timeout. But hey, it's better nothing. This driver was tested on an RZ/A1 RSK board using watchdog-simple.c from samples/watchdog/. v3: * changed from a reset driver to a watchdog timer driver * use udelay(20) instead of msleep for reset handler * added Reviewed-by for r7s72100.dtsi and renesas-wdt.txt * added Acked-by for renesas-wdt.txt v2: * added to renesas-wdt.txt instead of creating a new file * changed "renesas,r7s72100-reset" to "renesas,r7s72100-wdt" * changed "renesas,wdt-reset" to "renesas,rza-wdt" * added "renesas,rza-wdt" as a fallback * added interupt property (even though it is not used) * added clocks property * changed hard coded register values to defines * added msleep to while(1) loop * removed unnecessary #include files * added Reviewed-by: Geert Uytterhoeven for renesas-reset.c Chris Brandt (3): watchdog: add rza_wdt driver watchdog: renesas-wdt: add support for rza ARM: dts: r7s72100: Add watchdog timer .../devicetree/bindings/watchdog/renesas-wdt.txt | 4 +- arch/arm/boot/dts/r7s72100.dtsi| 7 + drivers/watchdog/Kconfig | 8 + drivers/watchdog/Makefile | 1 + drivers/watchdog/rza_wdt.c | 208 + 5 files changed, 227 insertions(+), 1 deletion(-) create mode 100644 drivers/watchdog/rza_wdt.c -- 2.10.1
[PATCH v4 3/9] drm: bridge: dw-hdmi: Enable CSC even for DVI
From: Neil Armstrong If the input pixel format is not RGB, the CSC must be enabled in order to provide valid pixel to DVI sinks. This patch removes the hdmi only dependency on the CSC enabling. Reviewed-by: Jose Abreu Reviewed-by: Laurent Pinchart Signed-off-by: Neil Armstrong --- drivers/gpu/drm/bridge/dw-hdmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c index 906583beb08b..d863b3393aee 100644 --- a/drivers/gpu/drm/bridge/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/dw-hdmi.c @@ -1291,8 +1291,8 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi) hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS); } - /* Enable color space conversion if needed (for HDMI sinks only). */ - if (hdmi->sink_is_hdmi && is_color_space_conversion(hdmi)) + /* Enable color space conversion if needed */ + if (is_color_space_conversion(hdmi)) hdmi_writeb(hdmi, HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_IN_PATH, HDMI_MC_FLOWCTRL); else -- Regards, Laurent Pinchart
[PATCH v4 0/9] drm: bridge: dw-hdmi: Refactor PHY support
Hello, This patch series refactors all the PHY handling code in order to allow support of vendor PHYs and Synopsys DWC HDMI 2.0 TX PHYs. The series starts with a few cleanups and small fixes. Patch 1/9 just removes unused code, patch 2/9 moves the color converter code out of the PHY configure function as it isn't PHY-dependent, and patch 3/9 enables color conversion even for DVI as it is needed to output RGB when the input format is YUV. The next two patches fix the power down (4/9) and up (5/9) sequences to comply with the HDMI TX PHY specifications. They are the biggest functional changes in the whole set, and have been tested successfully (with the rest of the series) on i.MX6Q and R-Car H3. I'll try to perform tests on RK3288 tomorrow if nobody beats me to it (Neil, that's for you :-)). The PLL PHY lock delay has been measured to be between 300µs and 350µs on R-Car H3 and between 400µs and 600µs on i.MX6Q. The PHY power down delay has been measured to be less than 50µs on both platforms, and was often close to instant with power down reported in the first poll iteration. We should thus be more than safe with a 5ms timeout. Patch 6/9 breaks the PHY operations out. Glue code is then allowed to pass a PHY operations structure to support vendor PHYs. The existing PHY support code is turned into a default Synopsys PHYs implementation for those PHY operations. Patch 7/9 further refactors the Synopsys PHY configuration function to make it modular, in order to support DWC HDMI 2.0 TX PHYs that have a very different register layout compared to the currently supported PHYs. Glue code is again allowed to provide a custom PHY configuration implementation, with the existing PHY support code turned into the default implementation for all currently supported Synopsys PHYs. Patch 8/9 is a small cleanup that removes the now unneeded device type for glue code platform data, and patch 9/9 follows by switching the driver to regmap in order to support vendor-specific register access more easily. Kieran Bingham (2): drm: bridge: dw-hdmi: Add support for custom PHY configuration drm: bridge: dw-hdmi: Remove device type from platform data Laurent Pinchart (5): drm: bridge: dw-hdmi: Remove unused functions drm: bridge: dw-hdmi: Move CSC configuration out of PHY code drm: bridge: dw-hdmi: Fix the PHY power down sequence drm: bridge: dw-hdmi: Fix the PHY power up sequence drm: bridge: dw-hdmi: Create PHY operations Neil Armstrong (2): drm: bridge: dw-hdmi: Enable CSC even for DVI drm: bridge: dw-hdmi: Switch to regmap for register access drivers/gpu/drm/bridge/dw-hdmi.c| 467 +--- drivers/gpu/drm/imx/dw_hdmi-imx.c | 2 - drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 1 - include/drm/bridge/dw_hdmi.h| 33 +- 4 files changed, 304 insertions(+), 199 deletions(-) -- Regards, Laurent Pinchart
[PATCH v4 5/9] drm: bridge: dw-hdmi: Fix the PHY power up sequence
When powering the PHY up we need to wait for the PLL to lock. This is
done by polling the TX_PHY_LOCK bit in the HDMI_PHY_STAT0 register
(interrupt-based wait could be implemented as well but is likely
overkill). The bit is asserted when the PLL locks, but the current code
incorrectly waits for the bit to be deasserted. Fix it, and while at it,
replace the udelay() with a sleep as the code never runs in
non-sleepable context.
To be consistent with the power down implementation move the poll loop
to the power off function.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/bridge/dw-hdmi.c | 65 +++-
1 file changed, 37 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 85348ba6bb1c..0aa3ad404f77 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -949,9 +949,44 @@ static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
dw_hdmi_phy_gen2_pddq(hdmi, 1);
}
+static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
+{
+ const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
+ unsigned int i;
+ u8 val;
+
+ if (phy->gen == 1) {
+ dw_hdmi_phy_enable_powerdown(hdmi, false);
+
+ /* Toggle TMDS enable. */
+ dw_hdmi_phy_enable_tmds(hdmi, 0);
+ dw_hdmi_phy_enable_tmds(hdmi, 1);
+ return 0;
+ }
+
+ dw_hdmi_phy_gen2_txpwron(hdmi, 1);
+ dw_hdmi_phy_gen2_pddq(hdmi, 0);
+
+ /* Wait for PHY PLL lock */
+ for (i = 0; i < 5; ++i) {
+ val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
+ if (val)
+ break;
+
+ usleep_range(1000, 2000);
+ }
+
+ if (!val) {
+ dev_err(hdmi->dev, "PHY PLL failed to lock\n");
+ return -ETIMEDOUT;
+ }
+
+ dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
+ return 0;
+}
+
static int hdmi_phy_configure(struct dw_hdmi *hdmi)
{
- u8 val, msec;
const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
const struct dw_hdmi_mpll_config *mpll_config = pdata->mpll_cfg;
const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
@@ -1019,33 +1054,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
hdmi_phy_i2c_write(hdmi, HDMI_3D_TX_PHY_CKCALCTRL_OVERRIDE,
HDMI_3D_TX_PHY_CKCALCTRL);
- dw_hdmi_phy_enable_powerdown(hdmi, false);
-
- /* toggle TMDS enable */
- dw_hdmi_phy_enable_tmds(hdmi, 0);
- dw_hdmi_phy_enable_tmds(hdmi, 1);
-
- /* gen2 tx power on */
- dw_hdmi_phy_gen2_txpwron(hdmi, 1);
- dw_hdmi_phy_gen2_pddq(hdmi, 0);
-
- /* Wait for PHY PLL lock */
- msec = 5;
- do {
- val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
- if (!val)
- break;
-
- if (msec == 0) {
- dev_err(hdmi->dev, "PHY PLL not locked\n");
- return -ETIMEDOUT;
- }
-
- udelay(1000);
- msec--;
- } while (1);
-
- return 0;
+ return dw_hdmi_phy_power_on(hdmi);
}
static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
--
Regards,
Laurent Pinchart
[PATCH v4 6/9] drm: bridge: dw-hdmi: Create PHY operations
The HDMI TX controller support different PHYs whose programming
interface can vary significantly, especially with vendor PHYs that are
not provided by Synopsys. To support them, create a PHY operation
structure that can be provided by the platform glue layer. The existing
PHY handling code (limited to Synopsys PHY support) is refactored into a
set of default PHY operations that are used automatically when the
platform glue doesn't provide its own operations.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/bridge/dw-hdmi.c | 91
include/drm/bridge/dw_hdmi.h | 18 +++-
2 files changed, 80 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 0aa3ad404f77..cb2703862be2 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -141,8 +141,12 @@ struct dw_hdmi {
u8 edid[HDMI_EDID_LEN];
bool cable_plugin;
- const struct dw_hdmi_phy_data *phy;
- bool phy_enabled;
+ struct {
+ const struct dw_hdmi_phy_ops *ops;
+ const char *name;
+ void *data;
+ bool enabled;
+ } phy;
struct drm_display_mode previous_mode;
@@ -831,6 +835,10 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi)
HDMI_VP_CONF);
}
+/*
-
+ * Synopsys PHY Handling
+ */
+
static inline void hdmi_phy_test_clear(struct dw_hdmi *hdmi,
unsigned char bit)
{
@@ -987,6 +995,7 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
static int hdmi_phy_configure(struct dw_hdmi *hdmi)
{
+ const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
const struct dw_hdmi_mpll_config *mpll_config = pdata->mpll_cfg;
const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
@@ -1019,7 +1028,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
dw_hdmi_phy_power_off(hdmi);
/* Leave low power consumption mode by asserting SVSRET. */
- if (hdmi->phy->has_svsret)
+ if (phy->has_svsret)
dw_hdmi_phy_enable_svsret(hdmi, 1);
/* PHY reset. The reset signal is active high on Gen2 PHYs. */
@@ -1057,7 +1066,8 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
return dw_hdmi_phy_power_on(hdmi);
}
-static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
+static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
+ struct drm_display_mode *mode)
{
int i, ret;
@@ -1071,10 +1081,31 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
return ret;
}
- hdmi->phy_enabled = true;
return 0;
}
+static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi, void *data)
+{
+ dw_hdmi_phy_power_off(hdmi);
+}
+
+static enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
+ void *data)
+{
+ return hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD ?
+ connector_status_connected : connector_status_disconnected;
+}
+
+static const struct dw_hdmi_phy_ops dw_hdmi_synopsys_phy_ops = {
+ .init = dw_hdmi_phy_init,
+ .disable = dw_hdmi_phy_disable,
+ .read_hpd = dw_hdmi_phy_read_hpd,
+};
+
+/*
-
+ * HDMI TX Setup
+ */
+
static void hdmi_tx_hdcp_config(struct dw_hdmi *hdmi)
{
u8 de;
@@ -1289,16 +1320,6 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
hdmi_writeb(hdmi, vsync_len, HDMI_FC_VSYNCINWIDTH);
}
-static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi)
-{
- if (!hdmi->phy_enabled)
- return;
-
- dw_hdmi_phy_power_off(hdmi);
-
- hdmi->phy_enabled = false;
-}
-
/* HDMI Initialization Step B.4 */
static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi)
{
@@ -1431,9 +1452,10 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct
drm_display_mode *mode)
hdmi_av_composer(hdmi, mode);
/* HDMI Initializateion Step B.2 */
- ret = dw_hdmi_phy_init(hdmi);
+ ret = hdmi->phy.ops->init(hdmi, hdmi->phy.data, &hdmi->previous_mode);
if (ret)
return ret;
+ hdmi->phy.enabled = true;
/* HDMI Initialization Step B.3 */
dw_hdmi_enable_video_path(hdmi);
@@ -1548,7 +1570,11 @@ static void dw_hdmi_poweron(struct dw_hdmi *hdmi)
static void dw_hdmi_poweroff(struct dw_hdmi *hdmi)
{
- dw_hdmi_phy_disable(hdmi);
+ if (hdmi->phy.enabled) {
+ hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
+ hdmi->phy.enabled = false;
+ }
+
hdmi->bridge_is_on = false;
}
@@ -1611,8 +1637,7 @@ dw_hdmi_connector_detect(struct drm_connector *co
[PATCH v4 4/9] drm: bridge: dw-hdmi: Fix the PHY power down sequence
The PHY requires us to wait for the PHY to switch to low power mode
after deasserting TXPWRON and before asserting PDDQ in the power down
sequence, otherwise power down will fail.
The PHY power down can be monitored though the TX_READY bit, available
through I2C in the PHY registers, or the TX_PHY_LOCK bit, available
through the HDMI TX registers. As the two are equivalent, let's pick the
easier solution of polling the TX_PHY_LOCK bit.
The power down code is currently duplicated in multiple places. To avoid
spreading multiple calls to a TX_PHY_LOCK poll function, we have to
refactor the power down code and group it all in a single function.
Tests showed that one poll iteration was enough for TX_PHY_LOCK to
become low, without requiring any additional delay. Retrying the read
five times with a 1ms to 2ms delay between each attempt should thus be
more than enough.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/bridge/dw-hdmi.c | 52 +---
1 file changed, 43 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index d863b3393aee..85348ba6bb1c 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -116,6 +116,7 @@ struct dw_hdmi_i2c {
struct dw_hdmi_phy_data {
enum dw_hdmi_phy_type type;
const char *name;
+ unsigned int gen;
bool has_svsret;
};
@@ -914,6 +915,40 @@ static void dw_hdmi_phy_sel_interface_control(struct
dw_hdmi *hdmi, u8 enable)
HDMI_PHY_CONF0_SELDIPIF_MASK);
}
+static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
+{
+ const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
+ unsigned int i;
+ u16 val;
+
+ if (phy->gen == 1) {
+ dw_hdmi_phy_enable_tmds(hdmi, 0);
+ dw_hdmi_phy_enable_powerdown(hdmi, true);
+ return;
+ }
+
+ dw_hdmi_phy_gen2_txpwron(hdmi, 0);
+
+ /*
+* Wait for TX_PHY_LOCK to be deasserted to indicate that the PHY went
+* to low power mode.
+*/
+ for (i = 0; i < 5; ++i) {
+ val = hdmi_readb(hdmi, HDMI_PHY_STAT0);
+ if (!(val & HDMI_PHY_TX_PHY_LOCK))
+ break;
+
+ usleep_range(1000, 2000);
+ }
+
+ if (val & HDMI_PHY_TX_PHY_LOCK)
+ dev_warn(hdmi->dev, "PHY failed to power down\n");
+ else
+ dev_dbg(hdmi->dev, "PHY powered down in %u iterations\n", i);
+
+ dw_hdmi_phy_gen2_pddq(hdmi, 1);
+}
+
static int hdmi_phy_configure(struct dw_hdmi *hdmi)
{
u8 val, msec;
@@ -946,11 +981,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
return -EINVAL;
}
- /* gen2 tx power off */
- dw_hdmi_phy_gen2_txpwron(hdmi, 0);
-
- /* gen2 pddq */
- dw_hdmi_phy_gen2_pddq(hdmi, 1);
+ dw_hdmi_phy_power_off(hdmi);
/* Leave low power consumption mode by asserting SVSRET. */
if (hdmi->phy->has_svsret)
@@ -1025,8 +1056,6 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
for (i = 0; i < 2; i++) {
dw_hdmi_phy_sel_data_en_pol(hdmi, 1);
dw_hdmi_phy_sel_interface_control(hdmi, 0);
- dw_hdmi_phy_enable_tmds(hdmi, 0);
- dw_hdmi_phy_enable_powerdown(hdmi, true);
ret = hdmi_phy_configure(hdmi);
if (ret)
@@ -1256,8 +1285,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi)
if (!hdmi->phy_enabled)
return;
- dw_hdmi_phy_enable_tmds(hdmi, 0);
- dw_hdmi_phy_enable_powerdown(hdmi, true);
+ dw_hdmi_phy_power_off(hdmi);
hdmi->phy_enabled = false;
}
@@ -1827,23 +1855,29 @@ static const struct dw_hdmi_phy_data dw_hdmi_phys[] = {
{
.type = DW_HDMI_PHY_DWC_HDMI_TX_PHY,
.name = "DWC HDMI TX PHY",
+ .gen = 1,
}, {
.type = DW_HDMI_PHY_DWC_MHL_PHY_HEAC,
.name = "DWC MHL PHY + HEAC PHY",
+ .gen = 2,
.has_svsret = true,
}, {
.type = DW_HDMI_PHY_DWC_MHL_PHY,
.name = "DWC MHL PHY",
+ .gen = 2,
.has_svsret = true,
}, {
.type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY_HEAC,
.name = "DWC HDMI 3D TX PHY + HEAC PHY",
+ .gen = 2,
}, {
.type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY,
.name = "DWC HDMI 3D TX PHY",
+ .gen = 2,
}, {
.type = DW_HDMI_PHY_DWC_HDMI20_TX_PHY,
.name = "DWC HDMI 2.0 TX PHY",
+ .gen = 2,
.has_svsret = true,
}
};
--
Regards,
Laurent Pinchart
[PATCH v4 2/9] drm: bridge: dw-hdmi: Move CSC configuration out of PHY code
The color space converter isn't part of the PHY, move its configuration
out of PHY code.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/bridge/dw-hdmi.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index ce7496399ccf..906583beb08b 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -914,7 +914,7 @@ static void dw_hdmi_phy_sel_interface_control(struct
dw_hdmi *hdmi, u8 enable)
HDMI_PHY_CONF0_SELDIPIF_MASK);
}
-static int hdmi_phy_configure(struct dw_hdmi *hdmi, int cscon)
+static int hdmi_phy_configure(struct dw_hdmi *hdmi)
{
u8 val, msec;
const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
@@ -946,14 +946,6 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, int
cscon)
return -EINVAL;
}
- /* Enable csc path */
- if (cscon)
- val = HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_IN_PATH;
- else
- val = HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_BYPASS;
-
- hdmi_writeb(hdmi, val, HDMI_MC_FLOWCTRL);
-
/* gen2 tx power off */
dw_hdmi_phy_gen2_txpwron(hdmi, 0);
@@ -1028,10 +1020,6 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, int
cscon)
static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
{
int i, ret;
- bool cscon;
-
- /*check csc whether needed activated in HDMI mode */
- cscon = hdmi->sink_is_hdmi && is_color_space_conversion(hdmi);
/* HDMI Phy spec says to do the phy initialization sequence twice */
for (i = 0; i < 2; i++) {
@@ -1040,8 +1028,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
dw_hdmi_phy_enable_tmds(hdmi, 0);
dw_hdmi_phy_enable_powerdown(hdmi, true);
- /* Enable CSC */
- ret = hdmi_phy_configure(hdmi, cscon);
+ ret = hdmi_phy_configure(hdmi);
if (ret)
return ret;
}
@@ -1303,6 +1290,14 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi
*hdmi)
clkdis &= ~HDMI_MC_CLKDIS_CSCCLK_DISABLE;
hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS);
}
+
+ /* Enable color space conversion if needed (for HDMI sinks only). */
+ if (hdmi->sink_is_hdmi && is_color_space_conversion(hdmi))
+ hdmi_writeb(hdmi, HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_IN_PATH,
+ HDMI_MC_FLOWCTRL);
+ else
+ hdmi_writeb(hdmi, HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_BYPASS,
+ HDMI_MC_FLOWCTRL);
}
static void hdmi_enable_audio_clk(struct dw_hdmi *hdmi)
--
Regards,
Laurent Pinchart
[PATCH v4 1/9] drm: bridge: dw-hdmi: Remove unused functions
Most of the hdmi_phy_test_*() functions are unused. Remove them.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/bridge/dw-hdmi.c | 26 --
1 file changed, 26 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 9a9ec27d9e28..ce7496399ccf 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -837,32 +837,6 @@ static inline void hdmi_phy_test_clear(struct dw_hdmi
*hdmi,
HDMI_PHY_TST0_TSTCLR_MASK, HDMI_PHY_TST0);
}
-static inline void hdmi_phy_test_enable(struct dw_hdmi *hdmi,
- unsigned char bit)
-{
- hdmi_modb(hdmi, bit << HDMI_PHY_TST0_TSTEN_OFFSET,
- HDMI_PHY_TST0_TSTEN_MASK, HDMI_PHY_TST0);
-}
-
-static inline void hdmi_phy_test_clock(struct dw_hdmi *hdmi,
- unsigned char bit)
-{
- hdmi_modb(hdmi, bit << HDMI_PHY_TST0_TSTCLK_OFFSET,
- HDMI_PHY_TST0_TSTCLK_MASK, HDMI_PHY_TST0);
-}
-
-static inline void hdmi_phy_test_din(struct dw_hdmi *hdmi,
-unsigned char bit)
-{
- hdmi_writeb(hdmi, bit, HDMI_PHY_TST1);
-}
-
-static inline void hdmi_phy_test_dout(struct dw_hdmi *hdmi,
- unsigned char bit)
-{
- hdmi_writeb(hdmi, bit, HDMI_PHY_TST2);
-}
-
static bool hdmi_phy_wait_i2c_done(struct dw_hdmi *hdmi, int msec)
{
u32 val;
--
Regards,
Laurent Pinchart
[PATCH v4 9/9] drm: bridge: dw-hdmi: Switch to regmap for register access
From: Neil Armstrong
The Synopsys Designware HDMI TX Controller does not enforce register
access on platforms instanciating it. The current driver supports two
different types of memory-mapped flat register access, but in order to
support the Amlogic Meson SoCs integration, and provide a more generic
way to handle all sorts of register mapping, switch the register access
to use the regmap infrastructure.
In the case of registers that are not flat memory-mapped or do not
conform to the current driver implementation, a regmap struct can be
given in the plat_data and be used at probe or bind.
Since the AHB audio driver is only available with direct memory access,
only allow the I2S audio driver to be registered is directly
memory-mapped.
Signed-off-by: Neil Armstrong
Reviewed-by: Laurent Pinchart
Tested-by: Laurent Pinchart
---
drivers/gpu/drm/bridge/dw-hdmi.c | 109 +--
include/drm/bridge/dw_hdmi.h | 1 +
2 files changed, 59 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 132c00685796..026a0dce7661 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
@@ -171,8 +172,8 @@ struct dw_hdmi {
unsigned int audio_n;
bool audio_enable;
- void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
- u8 (*read)(struct dw_hdmi *hdmi, int offset);
+ unsigned int reg_shift;
+ struct regmap *regm;
};
#define HDMI_IH_PHY_STAT0_RX_SENSE \
@@ -183,42 +184,23 @@ struct dw_hdmi {
(HDMI_PHY_RX_SENSE0 | HDMI_PHY_RX_SENSE1 | \
HDMI_PHY_RX_SENSE2 | HDMI_PHY_RX_SENSE3)
-static void dw_hdmi_writel(struct dw_hdmi *hdmi, u8 val, int offset)
-{
- writel(val, hdmi->regs + (offset << 2));
-}
-
-static u8 dw_hdmi_readl(struct dw_hdmi *hdmi, int offset)
-{
- return readl(hdmi->regs + (offset << 2));
-}
-
-static void dw_hdmi_writeb(struct dw_hdmi *hdmi, u8 val, int offset)
-{
- writeb(val, hdmi->regs + offset);
-}
-
-static u8 dw_hdmi_readb(struct dw_hdmi *hdmi, int offset)
-{
- return readb(hdmi->regs + offset);
-}
-
static inline void hdmi_writeb(struct dw_hdmi *hdmi, u8 val, int offset)
{
- hdmi->write(hdmi, val, offset);
+ regmap_write(hdmi->regm, offset << hdmi->reg_shift, val);
}
static inline u8 hdmi_readb(struct dw_hdmi *hdmi, int offset)
{
- return hdmi->read(hdmi, offset);
+ unsigned int val = 0;
+
+ regmap_read(hdmi->regm, offset << hdmi->reg_shift, &val);
+
+ return val;
}
static void hdmi_modb(struct dw_hdmi *hdmi, u8 data, u8 mask, unsigned reg)
{
- u8 val = hdmi_readb(hdmi, reg) & ~mask;
-
- val |= data & mask;
- hdmi_writeb(hdmi, val, reg);
+ regmap_update_bits(hdmi->regm, reg << hdmi->reg_shift, mask, data);
}
static void hdmi_mask_writeb(struct dw_hdmi *hdmi, u8 data, unsigned int reg,
@@ -1989,6 +1971,20 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi)
return -ENODEV;
}
+static const struct regmap_config hdmi_regmap_8bit_config = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .reg_stride = 1,
+ .max_register = HDMI_I2CM_FS_SCL_LCNT_0_ADDR,
+};
+
+static const struct regmap_config hdmi_regmap_32bit_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = HDMI_I2CM_FS_SCL_LCNT_0_ADDR << 2,
+};
+
static struct dw_hdmi *
__dw_hdmi_probe(struct platform_device *pdev,
const struct dw_hdmi_plat_data *plat_data)
@@ -1998,7 +1994,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
struct platform_device_info pdevinfo;
struct device_node *ddc_node;
struct dw_hdmi *hdmi;
- struct resource *iores;
+ struct resource *iores = NULL;
int irq;
int ret;
u32 val = 1;
@@ -2022,22 +2018,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
mutex_init(&hdmi->audio_mutex);
spin_lock_init(&hdmi->audio_lock);
- of_property_read_u32(np, "reg-io-width", &val);
-
- switch (val) {
- case 4:
- hdmi->write = dw_hdmi_writel;
- hdmi->read = dw_hdmi_readl;
- break;
- case 1:
- hdmi->write = dw_hdmi_writeb;
- hdmi->read = dw_hdmi_readb;
- break;
- default:
- dev_err(dev, "reg-io-width must be 1 or 4\n");
- return ERR_PTR(-EINVAL);
- }
-
ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
if (ddc_node) {
hdmi->ddc = of_get_i2c_adapter_by_node(ddc_node);
@@ -2051,11 +2031,38 @@ __dw_hdmi_probe(struct platform_device *pdev,
dev_dbg(hdmi->dev, "no ddc property found\n");
}
- iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- hdmi->regs =
[PATCH v4 7/9] drm: bridge: dw-hdmi: Add support for custom PHY configuration
From: Kieran Bingham
The DWC HDMI TX controller interfaces with a companion PHY. While
Synopsys provides multiple standard PHYs, SoC vendors can also integrate
a custom PHY.
Modularize PHY configuration to support vendor PHYs through platform
data. The existing PHY configuration code was originally written to
support the DWC HDMI 3D TX PHY, and seems to be compatible with the DWC
MLP PHY. The HDMI 2.0 PHY will require a separate configuration
function.
Signed-off-by: Kieran Bingham
Signed-off-by: Laurent Pinchart
---
Changes since v1:
- Check pdata->phy_configure in hdmi_phy_configure() to avoid
dereferencing NULL pointer.
---
drivers/gpu/drm/bridge/dw-hdmi.c | 109 ++-
include/drm/bridge/dw_hdmi.h | 7 +++
2 files changed, 81 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index cb2703862be2..b835d81bb471 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -118,6 +118,9 @@ struct dw_hdmi_phy_data {
const char *name;
unsigned int gen;
bool has_svsret;
+ int (*configure)(struct dw_hdmi *hdmi,
+const struct dw_hdmi_plat_data *pdata,
+unsigned long mpixelclock);
};
struct dw_hdmi {
@@ -860,8 +863,8 @@ static bool hdmi_phy_wait_i2c_done(struct dw_hdmi *hdmi,
int msec)
return true;
}
-static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
-unsigned char addr)
+void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
+ unsigned char addr)
{
hdmi_writeb(hdmi, 0xFF, HDMI_IH_I2CMPHY_STAT0);
hdmi_writeb(hdmi, addr, HDMI_PHY_I2CM_ADDRESS_ADDR);
@@ -873,6 +876,7 @@ static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi,
unsigned short data,
HDMI_PHY_I2CM_OPERATION_ADDR);
hdmi_phy_wait_i2c_done(hdmi, 1000);
}
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_i2c_write);
static void dw_hdmi_phy_enable_powerdown(struct dw_hdmi *hdmi, bool enable)
{
@@ -993,37 +997,67 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
return 0;
}
-static int hdmi_phy_configure(struct dw_hdmi *hdmi)
+/*
+ * PHY configuration function for the DWC HDMI 3D TX PHY. Based on the
available
+ * information the DWC MHL PHY has the same register layout and is thus also
+ * supported by this function.
+ */
+static int hdmi_phy_configure_dwc_hdmi_3d_tx(struct dw_hdmi *hdmi,
+ const struct dw_hdmi_plat_data *pdata,
+ unsigned long mpixelclock)
{
- const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
- const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
const struct dw_hdmi_mpll_config *mpll_config = pdata->mpll_cfg;
const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
const struct dw_hdmi_phy_config *phy_config = pdata->phy_config;
/* PLL/MPLL Cfg - always match on final entry */
for (; mpll_config->mpixelclock != ~0UL; mpll_config++)
- if (hdmi->hdmi_data.video_mode.mpixelclock <=
- mpll_config->mpixelclock)
+ if (mpixelclock <= mpll_config->mpixelclock)
break;
for (; curr_ctrl->mpixelclock != ~0UL; curr_ctrl++)
- if (hdmi->hdmi_data.video_mode.mpixelclock <=
- curr_ctrl->mpixelclock)
+ if (mpixelclock <= curr_ctrl->mpixelclock)
break;
for (; phy_config->mpixelclock != ~0UL; phy_config++)
- if (hdmi->hdmi_data.video_mode.mpixelclock <=
- phy_config->mpixelclock)
+ if (mpixelclock <= phy_config->mpixelclock)
break;
if (mpll_config->mpixelclock == ~0UL ||
curr_ctrl->mpixelclock == ~0UL ||
- phy_config->mpixelclock == ~0UL) {
- dev_err(hdmi->dev, "Pixel clock %d - unsupported by HDMI\n",
- hdmi->hdmi_data.video_mode.mpixelclock);
+ phy_config->mpixelclock == ~0UL)
return -EINVAL;
- }
+
+ dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].cpce,
+ HDMI_3D_TX_PHY_CPCE_CTRL);
+ dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].gmp,
+ HDMI_3D_TX_PHY_GMPCTRL);
+ dw_hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[0],
+ HDMI_3D_TX_PHY_CURRCTRL);
+
+ dw_hdmi_phy_i2c_write(hdmi, 0, HDMI_3D_TX_PHY_PLLPHBYCTRL);
+ dw_hdmi_phy_i2c_write(hdmi, HDMI_3D_TX_PHY_MSM_CTRL_CKO_SEL_FB_CLK,
+ HDMI_3D_TX_PHY_MSM_CTRL);
+
+ dw_hdmi_phy_i2c_write(hdmi, phy_config->term, HDMI_3D_TX_PHY_TXTERM);
+ dw_hdmi_phy_i2c_write(hdmi, phy_config->sym_ctr,
+ HDMI_3D_TX_PHY_CKSYMTXCTRL);
+ dw_hdmi_phy_i2c_write(hdmi, phy_confi
[PATCH v4 8/9] drm: bridge: dw-hdmi: Remove device type from platform data
From: Kieran Bingham
The device type isn't used anymore now that workarounds and PHY-specific
operations are performed based on version information read at runtime.
Remove it.
Signed-off-by: Kieran Bingham
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/bridge/dw-hdmi.c| 2 --
drivers/gpu/drm/imx/dw_hdmi-imx.c | 2 --
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 1 -
include/drm/bridge/dw_hdmi.h| 7 ---
4 files changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index b835d81bb471..132c00685796 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -127,7 +127,6 @@ struct dw_hdmi {
struct drm_connector connector;
struct drm_bridge bridge;
- enum dw_hdmi_devtype dev_type;
unsigned int version;
struct platform_device *audio;
@@ -2014,7 +2013,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
hdmi->plat_data = plat_data;
hdmi->dev = dev;
- hdmi->dev_type = plat_data->dev_type;
hdmi->sample_rate = 48000;
hdmi->disabled = true;
hdmi->rxsense = true;
diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c
b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index f645275e6e63..f039641070ac 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -175,7 +175,6 @@ static struct dw_hdmi_plat_data imx6q_hdmi_drv_data = {
.mpll_cfg = imx_mpll_cfg,
.cur_ctr= imx_cur_ctr,
.phy_config = imx_phy_config,
- .dev_type = IMX6Q_HDMI,
.mode_valid = imx6q_hdmi_mode_valid,
};
@@ -183,7 +182,6 @@ static struct dw_hdmi_plat_data imx6dl_hdmi_drv_data = {
.mpll_cfg = imx_mpll_cfg,
.cur_ctr = imx_cur_ctr,
.phy_config = imx_phy_config,
- .dev_type = IMX6DL_HDMI,
.mode_valid = imx6dl_hdmi_mode_valid,
};
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index a6d4a0236e8f..d53827413996 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -237,7 +237,6 @@ static const struct dw_hdmi_plat_data
rockchip_hdmi_drv_data = {
.mpll_cfg = rockchip_mpll_cfg,
.cur_ctr= rockchip_cur_ctr,
.phy_config = rockchip_phy_config,
- .dev_type = RK3288_HDMI,
};
static const struct of_device_id dw_hdmi_rockchip_dt_ids[] = {
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index dd330259a3dc..545f04fae3b6 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -21,12 +21,6 @@ enum {
DW_HDMI_RES_MAX,
};
-enum dw_hdmi_devtype {
- IMX6Q_HDMI,
- IMX6DL_HDMI,
- RK3288_HDMI,
-};
-
enum dw_hdmi_phy_type {
DW_HDMI_PHY_DWC_HDMI_TX_PHY = 0x00,
DW_HDMI_PHY_DWC_MHL_PHY_HEAC = 0xb2,
@@ -65,7 +59,6 @@ struct dw_hdmi_phy_ops {
};
struct dw_hdmi_plat_data {
- enum dw_hdmi_devtype dev_type;
enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
struct drm_display_mode *mode);
--
Regards,
Laurent Pinchart
[RFC PATCH 3/3] drm: rcar-du: Register a completion callback with VSP1
Updating the state in a running VSP1 requires two interrupts from the
VSP. Initially, the updated state will be committed - but only after the
VSP1 has completed processing it's current frame will the new state be
taken into account. As such, the committed state will only be 'completed'
after an extra frame completion interrupt.
Track this delay, by passing the frame flip event through the VSP
module; It will be returned only when the frame has completed and can be
returned to the caller.
Signed-off-by: Kieran Bingham
---
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 8 +-
drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 1 +-
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 34 ++-
3 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 7391dd95c733..0a824633a012 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -328,7 +328,7 @@ static bool rcar_du_crtc_page_flip_pending(struct
rcar_du_crtc *rcrtc)
bool pending;
spin_lock_irqsave(&dev->event_lock, flags);
- pending = rcrtc->event != NULL;
+ pending = (rcrtc->event != NULL) || (rcrtc->pending != NULL);
spin_unlock_irqrestore(&dev->event_lock, flags);
return pending;
@@ -578,6 +578,12 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
rcar_du_crtc_write(rcrtc, DSRCR, status & DSRCR_MASK);
if (status & DSSR_FRM) {
+
+ if (rcrtc->pending) {
+ trace_printk("VBlank loss due to VSP Overrun\n");
+ return IRQ_HANDLED;
+ }
+
drm_crtc_handle_vblank(&rcrtc->crtc);
rcar_du_crtc_finish_page_flip(rcrtc);
ret = IRQ_HANDLED;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index a7194812997e..8374a858446a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -46,6 +46,7 @@ struct rcar_du_crtc {
bool started;
struct drm_pending_vblank_event *event;
+ struct drm_pending_vblank_event *pending;
wait_queue_head_t flip_wait;
unsigned int outputs;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 71e70e1e0881..408375aff1a0 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -28,6 +28,26 @@
#include "rcar_du_kms.h"
#include "rcar_du_vsp.h"
+static void rcar_du_vsp_complete(void *private, void *data)
+{
+ struct rcar_du_crtc *crtc = (struct rcar_du_crtc *)private;
+ struct drm_device *dev = crtc->crtc.dev;
+ struct drm_pending_vblank_event *event;
+ bool match;
+ unsigned long flags;
+
+ spin_lock_irqsave(&dev->event_lock, flags);
+ event = crtc->event;
+ crtc->event = data;
+ match = (crtc->event == crtc->pending);
+ crtc->pending = NULL;
+ spin_unlock_irqrestore(&dev->event_lock, flags);
+
+ /* Safety checks */
+ WARN(event, "Event lost by VSP completion callback\n");
+ WARN(!match, "Stored pending event, does not match completion\n");
+}
+
void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
{
const struct drm_display_mode *mode = &crtc->crtc.state->adjusted_mode;
@@ -66,6 +86,8 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
*/
crtc->group->need_restart = true;
+ vsp1_du_register_callback(crtc->vsp->vsp, rcar_du_vsp_complete, crtc);
+
vsp1_du_setup_lif(crtc->vsp->vsp, mode->hdisplay, mode->vdisplay);
}
@@ -81,7 +103,17 @@ void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc)
{
- vsp1_du_atomic_flush(crtc->vsp->vsp, NULL);
+ struct drm_device *dev = crtc->crtc.dev;
+ struct drm_pending_vblank_event *event;
+ unsigned long flags;
+
+ /* Move the event to the VSP, track it locally as 'pending' */
+ spin_lock_irqsave(&dev->event_lock, flags);
+ event = crtc->pending = crtc->event;
+ crtc->event = NULL;
+ spin_unlock_irqrestore(&dev->event_lock, flags);
+
+ vsp1_du_atomic_flush(crtc->vsp->vsp, event);
}
/* Keep the two tables in sync. */
--
git-series 0.9.1
Re: [PATCH v2 17/29] drm: bridge: dw-hdmi: Refactor PHY power handling
Hi Laurent,
On 01-03-2017 11:09, Laurent Pinchart wrote:
> Hi Jose,
>
> On Thursday 05 Jan 2017 15:06:49 Jose Abreu wrote:
>> On 05-01-2017 12:29, Laurent Pinchart wrote:
>>> On Tuesday 20 Dec 2016 12:17:23 Jose Abreu wrote:
On 20-12-2016 11:45, Russell King - ARM Linux wrote:
> On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote:
>> Instead of spreading version-dependent PHY power handling code around,
>> group it in two functions to power the PHY on and off and use them
>> through the driver.
>>
>> Powering off the PHY at the beginning of the setup phase is currently
>> split in two locations for first and second generation PHYs, group all
>> the operations in the dw_hdmi_phy_init() function.
> This changes the behaviour of the driver.
>
>> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
>> +{
>> +if (hdmi->phy->gen == 1) {
>> +dw_hdmi_phy_enable_tmds(hdmi, 0);
>> +dw_hdmi_phy_enable_powerdown(hdmi, true);
>> +} else {
>> +dw_hdmi_phy_gen2_txpwron(hdmi, 0);
>> +dw_hdmi_phy_gen2_pddq(hdmi, 1);
>> +}
>> +}
>> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi
>> *hdmi)
>> if (!hdmi->phy_enabled)
>> return;
>>
>> -dw_hdmi_phy_enable_tmds(hdmi, 0);
>> -dw_hdmi_phy_enable_powerdown(hdmi, true);
>> -
>> +dw_hdmi_phy_power_off(hdmi);
> This makes dw_hdmi_phy_disable() power down a gen2 phy.
>
> The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a
> gen2 phy. I've been carrying this change for a while, which I've had
> to revert (and finally expunge), as it causes problems on iMX6:
>
> @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi
> *hdmi)
> if (!hdmi->phy_enabled)
> return;
>
> + /* Actually set the phy into low power mode */
> + dw_hdmi_phy_gen2_txpwron(hdmi, 0);
> +
> + /* FIXME: We should wait for TX_READY to be deasserted */
> +
> + dw_hdmi_phy_gen2_pddq(hdmi, 1);
> +
> + /* This appears to have no effect on iMX6 */
> dw_hdmi_phy_enable_tmds(hdmi, 0);
> dw_hdmi_phy_enable_powerdown(hdmi, true);
>
> So, I think your change here will cause problems for iMX6.
>
> From what I remember, it seems that iMX6 has issues with RXSENSE/HPD
> bouncing when the PHY is powered down. I can't promise when I'll be
> able to check for that again.
Indeed TX_READY must be low before asserting pddq.
>>> The TX_READY signal is documented in the i.MX6 datasheet as being a PHY
>>> output signal, but there seems to be no HDMI TX register from which its
>>> state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register
>>> through I2C ? How long is the PHY expected to take to set TX_READY to 0 ?
>> TX_READY can be read from register 0x1A of phy, BIT(2) (through
>> I2C). Not sure if same offset for all phys though.
> I have been told that another option is to poll the TX_PHY_LOCK bit in the
> phy_stat0 register. That would be a simpler solution that doesn't require I2C
> access. Could you confirm (or disconfirm) this ?
Yes (In our implementation we have TX_PHY_LOCK wired up to
TX_READY that comes from phy. But if using a custom phy or an
unusual implementation then this may not hold).
Best regards,
Jose Miguel Abreu
HPD and RXSENSE should work in power down mode by enabling enhpdrxsense
bit in phy_conf0 BUT to enter power down you must disable TX_PWRON,
enhpdrxsense and enable PDDQ and PHY_RESET. Only after doing this (and
consequently entering power down mode) you can enable again enhpdrxsense.
>>> Thanks, I'll give it a try.
[RFC PATCH 1/3] v4l: vsp1: Register pipe with output WPF
The DRM object does not register the pipe with the WPF object. This is used internally throughout the driver as a means of accessing the pipe. As such this breaks operations which require access to the pipe from WPF interrupts. Register the pipe inside the WPF object after it has been declared as the output. Signed-off-by: Kieran Bingham --- drivers/media/platform/vsp1/vsp1_drm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index cd209dccff1b..8e2aa3f8e52f 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -596,6 +596,7 @@ int vsp1_drm_init(struct vsp1_device *vsp1) pipe->bru = &vsp1->bru->entity; pipe->lif = &vsp1->lif->entity; pipe->output = vsp1->wpf[0]; + pipe->output->pipe = pipe; return 0; } -- git-series 0.9.1
[RFC PATCH 0/3] RCAR-DU, VSP1: Prevent pre-emptive frame flips on VSP1-DRM pipelines
The RCAR-DU utilises a running VSPD pipeline to perform processing for the display pipeline. Changes to this pipeline are performed with an atomic flush operation which updates the state in the VSPD. Due to the way the running pipeline is operated, any flush operation has an implicit latency of one frame interval. This comes about as the display list is committed, but not updated until the next VSP1 interrupt. At this point the frame is being processed, but is not complete until the following VSP1 frame end interrupt. To prevent reporting page flips early, we must track this timing through the VSP1, and only allow the rcar-du object to report the page-flip completion event after the VSP1 has processed. [PATCH 1/3] fixes the VSP DRM object to register it's pipeline correctly. [PATCH 2/3] extends the VSP1 to allow a callback to be registered allowing the VSP1 to notify completion events, and extend the existing atomic flush API to allow private event data to be passed through. [PATCH 3/3] Utilises this API extension to postpone page flips as required. In current testing, with kmstest, and kmscube, it can be seen that our refresh rate has halved. I believe this is due to the one frame latency imposed by the VSPD and will need further investigation. Kieran Bingham (3): v4l: vsp1: Register pipe with output WPF v4l: vsp1: extend VSP1 module API to allow DRM callback registration drm: rcar-du: Register a completion callback with VSP1 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 8 - drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 1 +- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 34 - drivers/media/platform/vsp1/vsp1_drm.c | 43 +-- drivers/media/platform/vsp1/vsp1_drm.h | 12 - include/media/vsp1.h | 6 +++- 6 files changed, 99 insertions(+), 5 deletions(-) base-commit: a194138cd82dff52d4c39895fd89dc6f26eafc97 -- git-series 0.9.1
[RFC PATCH 2/3] v4l: vsp1: extend VSP1 module API to allow DRM callback registration
To be able to perform page flips in DRM without flicker we need to be
able to notify the rcar-du module when the VSP has completed its
processing.
To synchronise the page flip events for userspace, we move the required
event through the VSP to track the data flow. When the frame is
completed, the event can be returned back to the originator through the
registered callback.
We must not have bidirectional dependencies on the two components to
maintain support for loadable modules, thus we extend the API to allow
a callback to be registered within the VSP DRM interface.
Signed-off-by: Kieran Bingham
---
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 +-
drivers/media/platform/vsp1/vsp1_drm.c | 42 +--
drivers/media/platform/vsp1/vsp1_drm.h | 12 -
include/media/vsp1.h | 6 +++-
4 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index b5bfbe50bd87..71e70e1e0881 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -81,7 +81,7 @@ void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc)
{
- vsp1_du_atomic_flush(crtc->vsp->vsp);
+ vsp1_du_atomic_flush(crtc->vsp->vsp, NULL);
}
/* Keep the two tables in sync. */
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
b/drivers/media/platform/vsp1/vsp1_drm.c
index 8e2aa3f8e52f..743cbce48d0c 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -52,6 +52,40 @@ int vsp1_du_init(struct device *dev)
EXPORT_SYMBOL_GPL(vsp1_du_init);
/**
+ * vsp1_du_register_callback - Register VSP completion notifier callback
+ *
+ * Allow the DRM framework to register a callback with us to notify the end of
+ * processing each frame. This allows synchronisation for page flipping.
+ *
+ * @dev: the VSP device
+ * @callback: the callback function to notify the DU module
+ * @private: private structure data to pass with the callback
+ *
+ */
+void vsp1_du_register_callback(struct device *dev,
+ void (*callback)(void *, void *),
+ void *private)
+{
+ struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+
+ vsp1->drm->du_complete = callback;
+ vsp1->drm->du_private = private;
+}
+EXPORT_SYMBOL_GPL(vsp1_du_register_callback);
+
+static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe)
+{
+ struct vsp1_drm *drm = to_vsp1_drm(pipe);
+
+ if (drm->du_complete && drm->active_data)
+ drm->du_complete(drm->du_private, drm->active_data);
+
+ /* The pending frame is now active */
+ drm->active_data = drm->pending_data;
+ drm->pending_data = NULL;
+}
+
+/**
* vsp1_du_setup_lif - Setup the output part of the VSP pipeline
* @dev: the VSP device
* @width: output frame width in pixels
@@ -99,7 +133,8 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int width,
}
pipe->num_inputs = 0;
-
+ pipe->frame_end = NULL;
+ vsp1->drm->du_complete = NULL;
vsp1_dlm_reset(pipe->output->dlm);
vsp1_device_put(vsp1);
@@ -196,6 +231,8 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int
width,
if (ret < 0)
return ret;
+ pipe->frame_end = vsp1_du_pipeline_frame_end;
+
ret = media_entity_pipeline_start(&pipe->output->entity.subdev.entity,
&pipe->pipe);
if (ret < 0) {
@@ -420,7 +457,7 @@ static unsigned int rpf_zpos(struct vsp1_device *vsp1,
struct vsp1_rwpf *rpf)
* vsp1_du_atomic_flush - Commit an atomic update
* @dev: the VSP device
*/
-void vsp1_du_atomic_flush(struct device *dev)
+void vsp1_du_atomic_flush(struct device *dev, void *data)
{
struct vsp1_device *vsp1 = dev_get_drvdata(dev);
struct vsp1_pipeline *pipe = &vsp1->drm->pipe;
@@ -504,6 +541,7 @@ void vsp1_du_atomic_flush(struct device *dev)
vsp1_dl_list_commit(pipe->dl);
pipe->dl = NULL;
+ vsp1->drm->pending_data = data;
/* Start or stop the pipeline if needed. */
if (!vsp1->drm->num_inputs && pipe->num_inputs) {
diff --git a/drivers/media/platform/vsp1/vsp1_drm.h
b/drivers/media/platform/vsp1/vsp1_drm.h
index 9e28ab9254ba..fde19e5948a0 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.h
+++ b/drivers/media/platform/vsp1/vsp1_drm.h
@@ -33,8 +33,20 @@ struct vsp1_drm {
struct v4l2_rect compose;
unsigned int zpos;
} inputs[VSP1_MAX_RPF];
+
+ /* Frame syncronisation */
+ void (*du_complete)(void *, void *);
+ void *du_private;
+ void *pending_data;
+ void *active_data;
};
+static inline struct vsp1_drm *
+to_vsp1_drm(struct vsp1_pipeline *pipe)
+{
+ return container_of(pipe, struct vsp1_drm, pipe
Re: [PATCH v2 17/29] drm: bridge: dw-hdmi: Refactor PHY power handling
Hi Jose,
On Thursday 05 Jan 2017 15:06:49 Jose Abreu wrote:
> On 05-01-2017 12:29, Laurent Pinchart wrote:
> > On Tuesday 20 Dec 2016 12:17:23 Jose Abreu wrote:
> >> On 20-12-2016 11:45, Russell King - ARM Linux wrote:
> >>> On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote:
> Instead of spreading version-dependent PHY power handling code around,
> group it in two functions to power the PHY on and off and use them
> through the driver.
>
> Powering off the PHY at the beginning of the setup phase is currently
> split in two locations for first and second generation PHYs, group all
> the operations in the dw_hdmi_phy_init() function.
> >>>
> >>> This changes the behaviour of the driver.
> >>>
> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
> +{
> +if (hdmi->phy->gen == 1) {
> +dw_hdmi_phy_enable_tmds(hdmi, 0);
> +dw_hdmi_phy_enable_powerdown(hdmi, true);
> +} else {
> +dw_hdmi_phy_gen2_txpwron(hdmi, 0);
> +dw_hdmi_phy_gen2_pddq(hdmi, 1);
> +}
> +}
> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi
> *hdmi)
> if (!hdmi->phy_enabled)
> return;
>
> -dw_hdmi_phy_enable_tmds(hdmi, 0);
> -dw_hdmi_phy_enable_powerdown(hdmi, true);
> -
> +dw_hdmi_phy_power_off(hdmi);
> >>>
> >>> This makes dw_hdmi_phy_disable() power down a gen2 phy.
> >>>
> >>> The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a
> >>> gen2 phy. I've been carrying this change for a while, which I've had
> >>> to revert (and finally expunge), as it causes problems on iMX6:
> >>>
> >>> @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi
> >>> *hdmi)
> >>> if (!hdmi->phy_enabled)
> >>> return;
> >>>
> >>> + /* Actually set the phy into low power mode */
> >>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0);
> >>> +
> >>> + /* FIXME: We should wait for TX_READY to be deasserted */
> >>> +
> >>> + dw_hdmi_phy_gen2_pddq(hdmi, 1);
> >>> +
> >>> + /* This appears to have no effect on iMX6 */
> >>> dw_hdmi_phy_enable_tmds(hdmi, 0);
> >>> dw_hdmi_phy_enable_powerdown(hdmi, true);
> >>>
> >>> So, I think your change here will cause problems for iMX6.
> >>>
> >>> From what I remember, it seems that iMX6 has issues with RXSENSE/HPD
> >>> bouncing when the PHY is powered down. I can't promise when I'll be
> >>> able to check for that again.
> >>
> >> Indeed TX_READY must be low before asserting pddq.
> >
> > The TX_READY signal is documented in the i.MX6 datasheet as being a PHY
> > output signal, but there seems to be no HDMI TX register from which its
> > state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register
> > through I2C ? How long is the PHY expected to take to set TX_READY to 0 ?
>
> TX_READY can be read from register 0x1A of phy, BIT(2) (through
> I2C). Not sure if same offset for all phys though.
I have been told that another option is to poll the TX_PHY_LOCK bit in the
phy_stat0 register. That would be a simpler solution that doesn't require I2C
access. Could you confirm (or disconfirm) this ?
> >> HPD and RXSENSE should work in power down mode by enabling enhpdrxsense
> >> bit in phy_conf0 BUT to enter power down you must disable TX_PWRON,
> >> enhpdrxsense and enable PDDQ and PHY_RESET. Only after doing this (and
> >> consequently entering power down mode) you can enable again enhpdrxsense.
> >
> > Thanks, I'll give it a try.
--
Regards,
Laurent Pinchart
