Question regarding clocks in the DW-HDMI DT bindings
Hi Vladimir, On Saturday 03 Dec 2016 23:10:35 Vladimir Zapolskiy wrote: > On 12/03/2016 07:16 PM, Laurent Pinchart wrote: > > On Friday 25 Nov 2016 13:29:37 Fabio Estevam wrote: > >> On Fri, Nov 25, 2016 at 1:22 PM, Laurent Pinchart wrote: > I got the clock name from I.MX6Q TRM, I also checked the name again > with Rockchip IC design team now, hope to get some new information > soon. > >>> > >>> Thank you. While at it, could you ask them which version of the DW HDMI > >>> IP used in the SoC ? > >> > >> DW HDMI IP used in Rockchip is: > >> dwhdmi-rockchip ff98.hdmi: Detected HDMI controller > >> 0x20:0xa:0xa0:0xc1 > >> > >> as shown at > >> https://storage.kernelci.org/mainline/v4.9-rc6-157-g16ae16c6e561/ > >> arm-multi_v7_defconfig/lab-collabora/boot-rk3288-rock2-square_rootfs: > >> nfs.html > >> > >> DW HDMI IP used on mx6q is: > >> dwhdmi-imx 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1 > > > > Would you be able to print the value of the CONFIG[0-3]_ID registers as > > well ? I'm also interested in the same information for RK3288, as well as > > for IMX6DL. > i.MX6Qi.MX6DL > DESIGN_ID 0x13 0x13 > REVISION_ID 0x0a 0x1a<--- the only difference > PRODUCT_ID0 0xa0 0xa0 > PRODUCT_ID1 0xc1 0xc1 > CONFIG0_ID0x8f 0x8f > CONFIG1_ID0x01 0x01 > CONFIG2_ID0xf2 0xf2<--- HDMI 3D TX PHY > CONFIG3_ID0x02 0x02 > > I'm not sure, if i.MX6D and MX6S have the same DW HDMI IP as on i.MX6Q > and i.MXDL respectively, and I don't have i.MX6DP or i.MX6QP powered board > on hand to dump the registers. Thank you for the information. Here are the HDMI TX versions I've found so far. Allwinner H3/A64/A801.32a Freescale i.MX6Q1.30a Freescale i.MX6DL 1.31a Renesas R-Car H32.01a Rockchip RK3288 2.00a It would be useful to know what the other Freescale i.MX6 SoCs contain and whether they're subject to the HDMI errata worked around by the dw_hdmi_clear_overflow() function (ERR004308 "HDMI: 8000504668 â The arithmetic unit may get wrong video timing values although the FC_* registers hold correct values"). However, unless I'm mistaken only i.MX6DL and i.MX6Q have upstream support for HDMI output, so it might be difficult to find this out at the moment. If we could establish that the problem isn't specific to Freescale but affects all 1.30a and 1.31a revisions, we could enable the workaround dynamically based on runtime identification. I've tested R-Car H3 without the workaround and haven't noticed the problem explained by Russell (magenta line on the left side of the image) or any other issue. Could someone test this on Rockchip RK3288 ? -- Regards, Laurent Pinchart
Question regarding clocks in the DW-HDMI DT bindings
Hi Laurent, On 12/03/2016 07:16 PM, Laurent Pinchart wrote: > Hi Fabio, > > On Friday 25 Nov 2016 13:29:37 Fabio Estevam wrote: >> On Fri, Nov 25, 2016 at 1:22 PM, Laurent Pinchart wrote: I got the clock name from I.MX6Q TRM, I also checked the name again with Rockchip IC design team now, hope to get some new information soon. >>> >>> Thank you. While at it, could you ask them which version of the DW HDMI IP >>> used in the SoC ? >> >> DW HDMI IP used in Rockchip is: >> dwhdmi-rockchip ff98.hdmi: Detected HDMI controller 0x20:0xa:0xa0:0xc1 >> >> as shown at >> https://storage.kernelci.org/mainline/v4.9-rc6-157-g16ae16c6e561/arm-multi_ >> v7_defconfig/lab-collabora/boot-rk3288-rock2-square_rootfs:nfs.html >> >> DW HDMI IP used on mx6q is: >> dwhdmi-imx 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1 > > Would you be able to print the value of the CONFIG[0-3]_ID registers as well ? > I'm also interested in the same information for RK3288, as well as for IMX6DL. > i.MX6Qi.MX6DL DESIGN_ID 0x13 0x13 REVISION_ID 0x0a 0x1a<--- the only difference PRODUCT_ID0 0xa0 0xa0 PRODUCT_ID1 0xc1 0xc1 CONFIG0_ID0x8f 0x8f CONFIG1_ID0x01 0x01 CONFIG2_ID0xf2 0xf2<--- HDMI 3D TX PHY CONFIG3_ID0x02 0x02 I'm not sure, if i.MX6D and MX6S have the same DW HDMI IP as on i.MX6Q and i.MXDL respectively, and I don't have i.MX6DP or i.MX6QP powered board on hand to dump the registers. -- With best wishes, Vladimir
Question regarding clocks in the DW-HDMI DT bindings
Hi Fabio, On Friday 25 Nov 2016 13:29:37 Fabio Estevam wrote: > On Fri, Nov 25, 2016 at 1:22 PM, Laurent Pinchart wrote: > >> I got the clock name from I.MX6Q TRM, I also checked the name again > >> with Rockchip IC design team now, hope to get some new information soon. > > > > Thank you. While at it, could you ask them which version of the DW HDMI IP > > used in the SoC ? > > DW HDMI IP used in Rockchip is: > dwhdmi-rockchip ff98.hdmi: Detected HDMI controller 0x20:0xa:0xa0:0xc1 > > as shown at > https://storage.kernelci.org/mainline/v4.9-rc6-157-g16ae16c6e561/arm-multi_ > v7_defconfig/lab-collabora/boot-rk3288-rock2-square_rootfs:nfs.html > > DW HDMI IP used on mx6q is: > dwhdmi-imx 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1 Would you be able to print the value of the CONFIG[0-3]_ID registers as well ? I'm also interested in the same information for RK3288, as well as for IMX6DL. -- Regards, Laurent Pinchart
Question regarding clocks in the DW-HDMI DT bindings
Hi Mike, On Monday 28 Nov 2016 22:27:01 Michael Turquette wrote: > On Mon, Nov 28, 2016 at 10:04 PM, Laurent Pinchart wrote: > > On Monday 28 Nov 2016 13:56:11 Michael Turquette wrote: > >> On Fri, Nov 25, 2016 at 7:22 AM, Laurent Pinchart wrote: > >>> On Friday 25 Nov 2016 10:56:53 Andy Yan wrote: > On 2016å¹´11æ25æ¥ 07:26, Laurent Pinchart wrote: > > On Friday 25 Nov 2016 00:16:00 Vladimir Zapolskiy wrote: > >> On 11/25/2016 12:07 AM, Fabio Estevam wrote: > >>> On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart wrote: > Hi Andy, > > As the author of the DW-HDMI DT bindings this question is > addressed to you, but information from anyone is more than welcome. > > The DT bindings specify two clocks named "iahb" and "isfr" but > don't describe them. While I assume that the "isfr" clock > corresponds to the "isfrclk" input signal of the DW HDMI, there is > no "iahb" clock described in the IP core datasheet. > >>> > >>> i.MX6Q has a DW-HDMI IP block. > >>> > >>> The names in the devicetree binding matches the ones listed at the > >>> i.MX6Q Reference Manual - Table 33-1. HDMI Clocks > >> > >> correct, for your convenience the table is copied below: > >> > >> Clock name | Clock Root | Description > >> ---++--- > >> iahbclk | ahb_clk_root | Bus clock > >> icecclk | ckil_sync_clk_root | CEC low-frequency clock (32kHZ) > >> ihclk| ahb_clk_root | Module clock > >> isfrclk | video_27m_clk_root | Internal SFR clock (video clock > >> 27MHz) > >> > >> Here AHB stands for ARM Advanced High-performance Bus. > > > > That's what I suspected. I believe the "iahb" name is wrong, as the > > DW HDMI TX IP core clearly documents the bus clock as being called > > "iapbclk". We could rename that in the DT bindings (with > > compatibility code in the driver to keep supporting the old name) but > > it might not be worth it. The bindings should however document that > > the "iahb" clock is the IP core's "iapbclk" bus clock. > > I got the clock name from I.MX6Q TRM, I also checked the name again > with Rockchip IC design team now, hope to get some new information > soon. > >>> > >>> Thank you. While at it, could you ask them which version of the DW HDMI > >>> IP used in the SoC ? > >>> > > Another question I have about the bus clock (CC'ing the devicetree > > mailing list as well as the clock maintainers) is whether it should > > be made optional. The clock is obviously mandatory from a hardware > > point of view (given that APB is a synchronous bus and thus requires a > > clock), but in some SoCs (specifically for the Renesas SoCs) that > > clock is always on and can't be controlled. We already omit bus clocks > > in DT for most IP cores when the clock can never be controlled (and we > > also omit a bunch of other clocks that we don't even know exist), so > > it could make sense to make the clock optional. Otherwise there would > > be runtime overhead trying to handle a clock that can't be controlled. > > If this is the case on Renesas SOCs, we can consider make the clock as > optional. Or move all the clock operations to platform specific > code(dw_hdmi-rockchip.c/dw_hdmi-imx.c)? > >>> > >>> I'd prefer keeping the code generic, otherwise we'd end up with > >>> platform-specific code that would perform the same operations on most > >>> platforms. I'll submit a patch soon to make the clock optional, we can > >>> discuss it then. > >> > >> Yes, let's keep the code generic. Absence of a "standard' clock is OK > >> and we should accept the small overhead incurred in providing a > >> solution that works for everyone. This prevents hardware-specific > >> hacks in the driver. > >> > >> Related: we really should model bus clocks whenever possible. I've > >> seen other attempts to merge functional/logic and bus clocks into a > >> single entity (e.g. a single struct clk_hw/clk_core that turns both > >> clocks on and off) and this defeats some fine-grained power management > >> scenarios that the hardware designers had in mind when creating > >> separate controls for the clocks. > > > > Sure, but that wasn't really the question :-) When the bus clock is > > separately controllable then I agree it should be modelled separately in > > DT. In my case the bus clock is always on, and I'm thus wondering whether > > it would be better to make it optional in DT to reduce the runtime > > overhead incurred by trying to control something that can't be > > controlled. > > I thought I answered this, but maybe not directly enough :-) > > We should make the clock mandatory in DT if the physical line must be > there. This is regardless of whether a given chip/IP variant
Question regarding clocks in the DW-HDMI DT bindings
Hi Laurent, On 28-11-2016 19:19, Laurent Pinchart wrote: > Hi Jose, > > On Monday 28 Nov 2016 15:25:18 Jose Abreu wrote: >> On 28-11-2016 14:24, Laurent Pinchart wrote: >>> On Monday 28 Nov 2016 12:09:59 Jose Abreu wrote: On 28-11-2016 11:45, Laurent Pinchart wrote: > On Monday 28 Nov 2016 19:34:59 Andy Yan wrote: >> On 2016å¹´11æ26æ¥ 03:26, Laurent Pinchart wrote: >>> On Friday 25 Nov 2016 17:08:13 Philipp Zabel wrote: Am Freitag, den 25.11.2016, 17:45 +0200 schrieb Laurent Pinchart: > [snip] > > I'm also a bit puzzled by the differences in the HDMI PHY between > Freescale and Renesas. The Renesas datasheet documents three > registers only for the PHY (other might exist and be undocumented), > and while they contain fields similar to the ones documented in the > Freescale datasheet, the exact register layouts are different. Do you mean the HDMI_PHY_* registers in the HDMI TX register space or the separate HDMI PHY i2c registers? The PHY may very well be completely different. I think OMAP5 HDMI driver uses the same DesignWare HDMI TX as i.MX6, but not the same PHY. >>> I meant the HDMI PHY I2C registers, yes. If the PHYs are really >>> SoC-specific maybe we should move the supporting code to the platform >>> glue driver. >> Yes, it's really have this case, Some Soc uses DW HDMI controller, >> but uses another different phy. So it's worth moving the phy related >> code to soc platform glue driver. As a fellow worker with DW-HDMI driver and as a Synopsys SW developer there is something I would like to say: The configuration of the phy in this scenario is made through controller registers. There is a I2C interface but the regbank of this interface is mapped in the controller, so in order to configure phy you need to have access to the controller regbank and status. >>> Sure, of course. The idea would be to delegate PHY configuration to the >>> platform glue code, using an API exported by the dw-hdmi core driver to >>> read/write the PHY registers through I2C. >> Sounds fine but might be beneficial if instead of doing this in >> the glue code, do it in a new driver that then glues with the >> dw-hdmi driver. This way we can avoid code duplication. > Yes, the code should certainly be shared between compatible implementations. > I'm thinking about moving it to dw-hdmi-phy.c but still compiling that in in > dw-hdmi.ko (we're talking about a very small amount of code), and letting > glue > code select the PHY handler they want to use through a field in the dw-hdmi > platform data. Sounds fine by me. Let me know if you need someone to review. >> I'm working in HDMI RX now (that uses a JTAG interface to communicate >> with the phy, again mapped in the controller [not my fault :)]) >> and this is what I am doing: >> - Create a phy interface in controller driver (which would be >> dw-hdmi) >> - Create a phy driver >> - Create a controller driver (dw-hdmi) >> - Phy to be used is passed by pdata to controller driver >> - Controller driver requests and registers phy driver >> >> I submitted a RFC for the phy driver to media ML, you can find it >> here: http://www.spinics.net/lists/linux-media/msg107610.html > I'll try to review that when time permits, but I'm afraid I'm very busy at > the > moment. > > [snip] > Best regards, Jose Miguel Abreu
Question regarding clocks in the DW-HDMI DT bindings
Hi Mike, On Monday 28 Nov 2016 13:56:11 Michael Turquette wrote: > On Fri, Nov 25, 2016 at 7:22 AM, Laurent Pinchart wrote: > > On Friday 25 Nov 2016 10:56:53 Andy Yan wrote: > >> On 2016å¹´11æ25æ¥ 07:26, Laurent Pinchart wrote: > >>> On Friday 25 Nov 2016 00:16:00 Vladimir Zapolskiy wrote: > On 11/25/2016 12:07 AM, Fabio Estevam wrote: > > On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart wrote: > >> Hi Andy, > >> > >> As the author of the DW-HDMI DT bindings this question is addressed > >> to you, but information from anyone is more than welcome. > >> > >> The DT bindings specify two clocks named "iahb" and "isfr" but don't > >> describe them. While I assume that the "isfr" clock corresponds to > >> the "isfrclk" input signal of the DW HDMI, there is no "iahb" clock > >> described in the IP core datasheet. > > > > i.MX6Q has a DW-HDMI IP block. > > > > The names in the devicetree binding matches the ones listed at the > > i.MX6Q Reference Manual - Table 33-1. HDMI Clocks > > correct, for your convenience the table is copied below: > > Clock name | Clock Root | Description > ---++- > iahbclk | ahb_clk_root | Bus clock > icecclk | ckil_sync_clk_root | CEC low-frequency clock (32kHZ) > ihclk| ahb_clk_root | Module clock > isfrclk | video_27m_clk_root | Internal SFR clock (video clock > 27MHz) > > Here AHB stands for ARM Advanced High-performance Bus. > >>> > >>> That's what I suspected. I believe the "iahb" name is wrong, as the DW > >>> HDMI TX IP core clearly documents the bus clock as being called > >>> "iapbclk". We could rename that in the DT bindings (with compatibility > >>> code in the driver to keep supporting the old name) but it might not be > >>> worth it. The bindings should however document that the "iahb" clock is > >>> the IP core's "iapbclk" bus clock. > >> > >> I got the clock name from I.MX6Q TRM, I also checked the name again > >> with Rockchip IC design team now, hope to get some new information soon. > > > > Thank you. While at it, could you ask them which version of the DW HDMI IP > > used in the SoC ? > > > >>> Another question I have about the bus clock (CC'ing the devicetree > >>> mailing list as well as the clock maintainers) is whether it should be > >>> made optional. The clock is obviously mandatory from a hardware point > >>> of view (given that APB is a synchronous bus and thus requires a > >>> clock), but in some SoCs (specifically for the Renesas SoCs) that clock > >>> is always on and can't be controlled. We already omit bus clocks in DT > >>> for most IP cores when the clock can never be controlled (and we also > >>> omit a bunch of other clocks that we don't even know exist), so it > >>> could make sense to make the clock optional. Otherwise there would be > >>> runtime overhead trying to handle a clock that can't be controlled. > >> > >> If this is the case on Renesas SOCs, we can consider make the clock as > >> optional. Or move all the clock operations to platform specific > >> code(dw_hdmi-rockchip.c/dw_hdmi-imx.c)? > > > > I'd prefer keeping the code generic, otherwise we'd end up with platform- > > specific code that would perform the same operations on most platforms. > > I'll submit a patch soon to make the clock optional, we can discuss it > > then. > > Yes, let's keep the code generic. Absence of a "standard' clock is OK > and we should accept the small overhead incurred in providing a > solution that works for everyone. This prevents hardware-specific > hacks in the driver. > > Related: we really should model bus clocks whenever possible. I've > seen other attempts to merge functional/logic and bus clocks into a > single entity (e.g. a single struct clk_hw/clk_core that turns both > clocks on and off) and this defeats some fine-grained power management > scenarios that the hardware designers had in mind when creating > separate controls for the clocks. Sure, but that wasn't really the question :-) When the bus clock is separately controllable then I agree it should be modelled separately in DT. In my case the bus clock is always on, and I'm thus wondering whether it would be better to make it optional in DT to reduce the runtime overhead incurred by trying to control something that can't be controlled. > By the way while we're discussing DW HDMI bindings specific to iMX, > I would recommend to remove utterly hackish and iMX only "gpr" > property from the example in bindings/display/bridge/dw_hdmi.txt -- Regards, Laurent Pinchart
Question regarding clocks in the DW-HDMI DT bindings
Hi Laurent, [fixing Stephen's email address] On Mon, Nov 28, 2016 at 10:04 PM, Laurent Pinchart wrote: > Hi Mike, > > On Monday 28 Nov 2016 13:56:11 Michael Turquette wrote: >> On Fri, Nov 25, 2016 at 7:22 AM, Laurent Pinchart wrote: >> > On Friday 25 Nov 2016 10:56:53 Andy Yan wrote: >> >> On 2016å¹´11æ25æ¥ 07:26, Laurent Pinchart wrote: >> >>> On Friday 25 Nov 2016 00:16:00 Vladimir Zapolskiy wrote: >> On 11/25/2016 12:07 AM, Fabio Estevam wrote: >> > On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart wrote: >> >> Hi Andy, >> >> >> >> As the author of the DW-HDMI DT bindings this question is addressed >> >> to you, but information from anyone is more than welcome. >> >> >> >> The DT bindings specify two clocks named "iahb" and "isfr" but don't >> >> describe them. While I assume that the "isfr" clock corresponds to >> >> the "isfrclk" input signal of the DW HDMI, there is no "iahb" clock >> >> described in the IP core datasheet. >> > >> > i.MX6Q has a DW-HDMI IP block. >> > >> > The names in the devicetree binding matches the ones listed at the >> > i.MX6Q Reference Manual - Table 33-1. HDMI Clocks >> >> correct, for your convenience the table is copied below: >> >> Clock name | Clock Root | Description >> ---++- >> iahbclk | ahb_clk_root | Bus clock >> icecclk | ckil_sync_clk_root | CEC low-frequency clock (32kHZ) >> ihclk| ahb_clk_root | Module clock >> isfrclk | video_27m_clk_root | Internal SFR clock (video clock >> 27MHz) >> >> Here AHB stands for ARM Advanced High-performance Bus. >> >>> >> >>> That's what I suspected. I believe the "iahb" name is wrong, as the DW >> >>> HDMI TX IP core clearly documents the bus clock as being called >> >>> "iapbclk". We could rename that in the DT bindings (with compatibility >> >>> code in the driver to keep supporting the old name) but it might not be >> >>> worth it. The bindings should however document that the "iahb" clock is >> >>> the IP core's "iapbclk" bus clock. >> >> >> >> I got the clock name from I.MX6Q TRM, I also checked the name again >> >> with Rockchip IC design team now, hope to get some new information soon. >> > >> > Thank you. While at it, could you ask them which version of the DW HDMI IP >> > used in the SoC ? >> > >> >>> Another question I have about the bus clock (CC'ing the devicetree >> >>> mailing list as well as the clock maintainers) is whether it should be >> >>> made optional. The clock is obviously mandatory from a hardware point >> >>> of view (given that APB is a synchronous bus and thus requires a >> >>> clock), but in some SoCs (specifically for the Renesas SoCs) that clock >> >>> is always on and can't be controlled. We already omit bus clocks in DT >> >>> for most IP cores when the clock can never be controlled (and we also >> >>> omit a bunch of other clocks that we don't even know exist), so it >> >>> could make sense to make the clock optional. Otherwise there would be >> >>> runtime overhead trying to handle a clock that can't be controlled. >> >> >> >> If this is the case on Renesas SOCs, we can consider make the clock as >> >> optional. Or move all the clock operations to platform specific >> >> code(dw_hdmi-rockchip.c/dw_hdmi-imx.c)? >> > >> > I'd prefer keeping the code generic, otherwise we'd end up with platform- >> > specific code that would perform the same operations on most platforms. >> > I'll submit a patch soon to make the clock optional, we can discuss it >> > then. >> >> Yes, let's keep the code generic. Absence of a "standard' clock is OK >> and we should accept the small overhead incurred in providing a >> solution that works for everyone. This prevents hardware-specific >> hacks in the driver. >> >> Related: we really should model bus clocks whenever possible. I've >> seen other attempts to merge functional/logic and bus clocks into a >> single entity (e.g. a single struct clk_hw/clk_core that turns both >> clocks on and off) and this defeats some fine-grained power management >> scenarios that the hardware designers had in mind when creating >> separate controls for the clocks. > > Sure, but that wasn't really the question :-) When the bus clock is separately > controllable then I agree it should be modelled separately in DT. In my case > the bus clock is always on, and I'm thus wondering whether it would be better > to make it optional in DT to reduce the runtime overhead incurred by trying to > control something that can't be controlled. I thought I answered this, but maybe not directly enough :-) We should make the clock mandatory in DT if the physical line must be there. This is regardless of whether a given chip/IP variant has control over that clock; so long as the physical clock line always exists then it is not really "optional". In the case where there is an
Question regarding clocks in the DW-HDMI DT bindings
Hi Jose, On Monday 28 Nov 2016 15:25:18 Jose Abreu wrote: > On 28-11-2016 14:24, Laurent Pinchart wrote: > > On Monday 28 Nov 2016 12:09:59 Jose Abreu wrote: > >> On 28-11-2016 11:45, Laurent Pinchart wrote: > >>> On Monday 28 Nov 2016 19:34:59 Andy Yan wrote: > On 2016å¹´11æ26æ¥ 03:26, Laurent Pinchart wrote: > > On Friday 25 Nov 2016 17:08:13 Philipp Zabel wrote: > >> Am Freitag, den 25.11.2016, 17:45 +0200 schrieb Laurent Pinchart: [snip] > >>> I'm also a bit puzzled by the differences in the HDMI PHY between > >>> Freescale and Renesas. The Renesas datasheet documents three > >>> registers only for the PHY (other might exist and be undocumented), > >>> and while they contain fields similar to the ones documented in the > >>> Freescale datasheet, the exact register layouts are different. > >> > >> Do you mean the HDMI_PHY_* registers in the HDMI TX register space or > >> the separate HDMI PHY i2c registers? The PHY may very well be > >> completely different. I think OMAP5 HDMI driver uses the same > >> DesignWare HDMI TX as i.MX6, but not the same PHY. > > > > I meant the HDMI PHY I2C registers, yes. If the PHYs are really > > SoC-specific maybe we should move the supporting code to the platform > > glue driver. > > Yes, it's really have this case, Some Soc uses DW HDMI controller, > but uses another different phy. So it's worth moving the phy related > code to soc platform glue driver. > >> > >> As a fellow worker with DW-HDMI driver and as a Synopsys SW > >> developer there is something I would like to say: The > >> configuration of the phy in this scenario is made through > >> controller registers. There is a I2C interface but the regbank of > >> this interface is mapped in the controller, so in order to > >> configure phy you need to have access to the controller regbank > >> and status. > > > > Sure, of course. The idea would be to delegate PHY configuration to the > > platform glue code, using an API exported by the dw-hdmi core driver to > > read/write the PHY registers through I2C. > > Sounds fine but might be beneficial if instead of doing this in > the glue code, do it in a new driver that then glues with the > dw-hdmi driver. This way we can avoid code duplication. Yes, the code should certainly be shared between compatible implementations. I'm thinking about moving it to dw-hdmi-phy.c but still compiling that in in dw-hdmi.ko (we're talking about a very small amount of code), and letting glue code select the PHY handler they want to use through a field in the dw-hdmi platform data. > I'm working in HDMI RX now (that uses a JTAG interface to communicate > with the phy, again mapped in the controller [not my fault :)]) > and this is what I am doing: > - Create a phy interface in controller driver (which would be > dw-hdmi) > - Create a phy driver > - Create a controller driver (dw-hdmi) > - Phy to be used is passed by pdata to controller driver > - Controller driver requests and registers phy driver > > I submitted a RFC for the phy driver to media ML, you can find it > here: http://www.spinics.net/lists/linux-media/msg107610.html I'll try to review that when time permits, but I'm afraid I'm very busy at the moment. [snip] -- Regards, Laurent Pinchart
Question regarding clocks in the DW-HDMI DT bindings
Hi: On 2016å¹´11æ26æ¥ 03:26, Laurent Pinchart wrote: > Hi Philipp, > > On Friday 25 Nov 2016 17:08:13 Philipp Zabel wrote: >> Am Freitag, den 25.11.2016, 17:45 +0200 schrieb Laurent Pinchart: >>> On Friday 25 Nov 2016 10:56:55 Philipp Zabel wrote: Am Donnerstag, den 24.11.2016, 23:16 +0200 schrieb Laurent Pinchart: > Hi Andy, > > As the author of the DW-HDMI DT bindings this question is addressed to > you, but information from anyone is more than welcome. > > The DT bindings specify two clocks named "iahb" and "isfr" but don't > describe them. While I assume that the "isfr" clock corresponds to the > "isfrclk" input signal of the DW HDMI, there is no "iahb" clock > described in the IP core datasheet. The Table 33-1 "HDMI clocks" in the i.MX6DQ reference manual [1] lists iahbclk (AHB bus clock), icecclk (32 kHz CEC clock), ihclk (module clock) and isfrclk (27 MHz internal SFR clock). [1] http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.p df > The DW HDMI IP exposes control registers through an APB, not an AHB. > There is a bus clock named "iapbclk", is "iahb" just an incorrect name > for that clock ? According to figure 33-3 "HDMI TX Top Level Block Diagram" the control interface is AMBA AHB on i.MX6. Both iahbclk and ihclk are clocked by ahb_clk_root on i.MX6. Register 5 (HDMI_CONFIG1_ID) contains a few bits (confsfrdir, confi2c, confocp, confapb, confahb) that indicate which control interface is used. >>> That's interesting. The DW HDMI TX documentation I found (1.40a-ea00, >>> Early Adopter Edition) only has the confahb bit in that register. Do you >>> know which version of the IP is used in iMX.6 ? I wonder whether the above >>> is a Freescale-specific modification to the IP core. >> The driver reports:r >> >> dwhdmi-imx 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1 >> >> That is DESIGN_ID 0x13, REVISION_ID 0x0a. >> >>> I'm also a bit puzzled by the differences in the HDMI PHY between >>> Freescale and Renesas. The Renesas datasheet documents three registers >>> only for the PHY (other might exist and be undocumented), and while they >>> contain fields similar to the ones documented in the Freescale datasheet, >>> the exact register layouts are different. >> Do you mean the HDMI_PHY_* registers in the HDMI TX register space or >> the separate HDMI PHY i2c registers? The PHY may very well be completely >> different. I think OMAP5 HDMI driver uses the same DesignWare HDMI TX as >> i.MX6, but not the same PHY. > I meant the HDMI PHY I2C registers, yes. If the PHYs are really SoC-specific > maybe we should move the supporting code to the platform glue driver. Yes, it's really have this case, Some Soc uses DW HDMI controller, but uses another different phy. So it's worth moving the phy related code to soc platform glue driver. > > Would anyone happen to have access to the Synopsys HDMI TX PHY documentation ? >
Question regarding clocks in the DW-HDMI DT bindings
Hi Jose, On Monday 28 Nov 2016 12:09:59 Jose Abreu wrote: > On 28-11-2016 11:45, Laurent Pinchart wrote: > > On Monday 28 Nov 2016 19:34:59 Andy Yan wrote: > >> On 2016å¹´11æ26æ¥ 03:26, Laurent Pinchart wrote: > >>> On Friday 25 Nov 2016 17:08:13 Philipp Zabel wrote: > Am Freitag, den 25.11.2016, 17:45 +0200 schrieb Laurent Pinchart: > > On Friday 25 Nov 2016 10:56:55 Philipp Zabel wrote: > >> Am Donnerstag, den 24.11.2016, 23:16 +0200 schrieb Laurent Pinchart: > >>> Hi Andy, > >>> > >>> As the author of the DW-HDMI DT bindings this question is addressed > >>> to > >>> you, but information from anyone is more than welcome. > >>> > >>> The DT bindings specify two clocks named "iahb" and "isfr" but don't > >>> describe them. While I assume that the "isfr" clock corresponds to > >>> the > >>> "isfrclk" input signal of the DW HDMI, there is no "iahb" clock > >>> described in the IP core datasheet. > >> > >> The Table 33-1 "HDMI clocks" in the i.MX6DQ reference manual [1] > >> lists > >> iahbclk (AHB bus clock), icecclk (32 kHz CEC clock), ihclk (module > >> clock) and isfrclk (27 MHz internal SFR clock). > >> > >> [1] > >> http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQR > >> M. > >> pdf > >> > >>> The DW HDMI IP exposes control registers through an APB, not an AHB. > >>> There is a bus clock named "iapbclk", is "iahb" just an incorrect > >>> name > >>> for that clock ? > >> > >> According to figure 33-3 "HDMI TX Top Level Block Diagram" the > >> control > >> interface is AMBA AHB on i.MX6. Both iahbclk and ihclk are clocked by > >> ahb_clk_root on i.MX6. > >> > >> Register 5 (HDMI_CONFIG1_ID) contains a few bits (confsfrdir, > >> confi2c, > >> confocp, confapb, confahb) that indicate which control interface is > >> used. > > > > That's interesting. The DW HDMI TX documentation I found (1.40a-ea00, > > Early Adopter Edition) only has the confahb bit in that register. Do > > you > > know which version of the IP is used in iMX.6 ? I wonder whether the > > above > > is a Freescale-specific modification to the IP core. > > The driver reports:r > > dwhdmi-imx 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1 > > That is DESIGN_ID 0x13, REVISION_ID 0x0a. > > > I'm also a bit puzzled by the differences in the HDMI PHY between > > Freescale and Renesas. The Renesas datasheet documents three registers > > only for the PHY (other might exist and be undocumented), and while > > they > > contain fields similar to the ones documented in the Freescale > > datasheet, > > the exact register layouts are different. > > Do you mean the HDMI_PHY_* registers in the HDMI TX register space or > the separate HDMI PHY i2c registers? The PHY may very well be > completely > different. I think OMAP5 HDMI driver uses the same DesignWare HDMI TX > as > i.MX6, but not the same PHY. > >>> > >>> I meant the HDMI PHY I2C registers, yes. If the PHYs are really > >>> SoC-specific maybe we should move the supporting code to the platform > >>> glue driver. > >> > >> Yes, it's really have this case, Some Soc uses DW HDMI controller, > >> but uses another different phy. So it's worth moving the phy related > >> code to soc platform glue driver. > > As a fellow worker with DW-HDMI driver and as a Synopsys SW > developer there is something I would like to say: The > configuration of the phy in this scenario is made through > controller registers. There is a I2C interface but the regbank of > this interface is mapped in the controller, so in order to > configure phy you need to have access to the controller regbank > and status. Sure, of course. The idea would be to delegate PHY configuration to the platform glue code, using an API exported by the dw-hdmi core driver to read/write the PHY registers through I2C. Speaking of this, I assume that the rationale is a reduction in the number signals, but using I2C internally between HDMI TX and the PHY ? Seriously ? :-) > There are many phys available for our customers and some of them > are "custom" made. That was my conclusion, yes. So far there are two vendors using the dw-hdmi driver (Freescale i.MX6 and Rockchip RK3288), and a third one I'm working on (Renesas Gen3). The Freescale and Rockchip platforms seem to use very similar PHYs, which I assume is the Synopsys IP (perhaps customized by the vendors). Renesas platforms seem to use a different PHY: although there are some similarities in register names, only 3 registers are documented, with a bit layout different than the Freescale and Rockchip PHYs. Synopsys offers two different kind of PHYs (3D TX and HEAC), I'm thus also wondering whether Freescale and Rockchip could be using one of them and Renesas the
Question regarding clocks in the DW-HDMI DT bindings
Hi Laurent, On 28-11-2016 14:24, Laurent Pinchart wrote: > Hi Jose, > > On Monday 28 Nov 2016 12:09:59 Jose Abreu wrote: >> On 28-11-2016 11:45, Laurent Pinchart wrote: >>> On Monday 28 Nov 2016 19:34:59 Andy Yan wrote: On 2016å¹´11æ26æ¥ 03:26, Laurent Pinchart wrote: > On Friday 25 Nov 2016 17:08:13 Philipp Zabel wrote: >> Am Freitag, den 25.11.2016, 17:45 +0200 schrieb Laurent Pinchart: >>> On Friday 25 Nov 2016 10:56:55 Philipp Zabel wrote: Am Donnerstag, den 24.11.2016, 23:16 +0200 schrieb Laurent Pinchart: > Hi Andy, > > As the author of the DW-HDMI DT bindings this question is addressed > to > you, but information from anyone is more than welcome. > > The DT bindings specify two clocks named "iahb" and "isfr" but don't > describe them. While I assume that the "isfr" clock corresponds to > the > "isfrclk" input signal of the DW HDMI, there is no "iahb" clock > described in the IP core datasheet. The Table 33-1 "HDMI clocks" in the i.MX6DQ reference manual [1] lists iahbclk (AHB bus clock), icecclk (32 kHz CEC clock), ihclk (module clock) and isfrclk (27 MHz internal SFR clock). [1] http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQR M. pdf > The DW HDMI IP exposes control registers through an APB, not an AHB. > There is a bus clock named "iapbclk", is "iahb" just an incorrect > name > for that clock ? According to figure 33-3 "HDMI TX Top Level Block Diagram" the control interface is AMBA AHB on i.MX6. Both iahbclk and ihclk are clocked by ahb_clk_root on i.MX6. Register 5 (HDMI_CONFIG1_ID) contains a few bits (confsfrdir, confi2c, confocp, confapb, confahb) that indicate which control interface is used. >>> That's interesting. The DW HDMI TX documentation I found (1.40a-ea00, >>> Early Adopter Edition) only has the confahb bit in that register. Do >>> you >>> know which version of the IP is used in iMX.6 ? I wonder whether the >>> above >>> is a Freescale-specific modification to the IP core. >> The driver reports:r >> >> dwhdmi-imx 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1 >> >> That is DESIGN_ID 0x13, REVISION_ID 0x0a. >> >>> I'm also a bit puzzled by the differences in the HDMI PHY between >>> Freescale and Renesas. The Renesas datasheet documents three registers >>> only for the PHY (other might exist and be undocumented), and while >>> they >>> contain fields similar to the ones documented in the Freescale >>> datasheet, >>> the exact register layouts are different. >> Do you mean the HDMI_PHY_* registers in the HDMI TX register space or >> the separate HDMI PHY i2c registers? The PHY may very well be >> completely >> different. I think OMAP5 HDMI driver uses the same DesignWare HDMI TX >> as >> i.MX6, but not the same PHY. > I meant the HDMI PHY I2C registers, yes. If the PHYs are really > SoC-specific maybe we should move the supporting code to the platform > glue driver. Yes, it's really have this case, Some Soc uses DW HDMI controller, but uses another different phy. So it's worth moving the phy related code to soc platform glue driver. >> As a fellow worker with DW-HDMI driver and as a Synopsys SW >> developer there is something I would like to say: The >> configuration of the phy in this scenario is made through >> controller registers. There is a I2C interface but the regbank of >> this interface is mapped in the controller, so in order to >> configure phy you need to have access to the controller regbank >> and status. > Sure, of course. The idea would be to delegate PHY configuration to the > platform glue code, using an API exported by the dw-hdmi core driver to > read/write the PHY registers through I2C. Sounds fine but might be beneficial if instead of doing this in the glue code, do it in a new driver that then glues with the dw-hdmi driver. This way we can avoid code duplication. I'm working in HDMI RX now (that uses a JTAG interface to communicate with the phy, again mapped in the controller [not my fault :)]) and this is what I am doing: - Create a phy interface in controller driver (which would be dw-hdmi) - Create a phy driver - Create a controller driver (dw-hdmi) - Phy to be used is passed by pdata to controller driver - Controller driver requests and registers phy driver I submitted a RFC for the phy driver to media ML, you can find it here: http://www.spinics.net/lists/linux-media/msg107610.html > > Speaking of this, I assume that the rationale is a reduction in the number > signals, but using I2C internally between HDMI TX and the PHY ? Seriously ? > :-) Yeah, not very
Question regarding clocks in the DW-HDMI DT bindings
Hi Laurent, all, On Fri, Nov 25, 2016 at 7:22 AM, Laurent Pinchart wrote: > Hi Andy, > > On Friday 25 Nov 2016 10:56:53 Andy Yan wrote: >> On 2016å¹´11æ25æ¥ 07:26, Laurent Pinchart wrote: >> > On Friday 25 Nov 2016 00:16:00 Vladimir Zapolskiy wrote: >> >> On 11/25/2016 12:07 AM, Fabio Estevam wrote: >> >>> On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart wrote: >> Hi Andy, >> >> As the author of the DW-HDMI DT bindings this question is addressed to >> you, but information from anyone is more than welcome. >> >> The DT bindings specify two clocks named "iahb" and "isfr" but don't >> describe them. While I assume that the "isfr" clock corresponds to the >> "isfrclk" input signal of the DW HDMI, there is no "iahb" clock >> described in the IP core datasheet. >> >>> >> >>> i.MX6Q has a DW-HDMI IP block. >> >>> >> >>> The names in the devicetree binding matches the ones listed at the >> >>> i.MX6Q Reference Manual - Table 33-1. HDMI Clocks >> >> >> >> correct, for your convenience the table is copied below: >> >> >> >> Clock name | Clock Root | Description >> >> ---++--- >> >> iahbclk | ahb_clk_root | Bus clock >> >> icecclk | ckil_sync_clk_root | CEC low-frequency clock (32kHZ) >> >> ihclk| ahb_clk_root | Module clock >> >> isfrclk | video_27m_clk_root | Internal SFR clock (video clock >> >> 27MHz) >> >> >> >> Here AHB stands for ARM Advanced High-performance Bus. >> > >> > That's what I suspected. I believe the "iahb" name is wrong, as the DW >> > HDMI TX IP core clearly documents the bus clock as being called >> > "iapbclk". We could rename that in the DT bindings (with compatibility >> > code in the driver to keep supporting the old name) but it might not be >> > worth it. The bindings should however document that the "iahb" clock is >> > the IP core's "iapbclk" bus clock. >> >> I got the clock name from I.MX6Q TRM, I also checked the name again >> with Rockchip IC design team now, hope to get some new information soon. > > Thank you. While at it, could you ask them which version of the DW HDMI IP > used in the SoC ? > >> > Another question I have about the bus clock (CC'ing the devicetree mailing >> > list as well as the clock maintainers) is whether it should be made >> > optional. The clock is obviously mandatory from a hardware point of view >> > (given that APB is a synchronous bus and thus requires a clock), but in >> > some SoCs (specifically for the Renesas SoCs) that clock is always on and >> > can't be controlled. We already omit bus clocks in DT for most IP cores >> > when the clock can never be controlled (and we also omit a bunch of other >> > clocks that we don't even know exist), so it could make sense to make the >> > clock optional. Otherwise there would be runtime overhead trying to handle >> > a clock that can't be controlled. >> >> If this is the case on Renesas SOCs, we can consider make the clock as >> optional. Or move all the clock operations to platform specific >> code(dw_hdmi-rockchip.c/dw_hdmi-imx.c)? > > I'd prefer keeping the code generic, otherwise we'd end up with platform- > specific code that would perform the same operations on most platforms. I'll > submit a patch soon to make the clock optional, we can discuss it then. Yes, let's keep the code generic. Absence of a "standard' clock is OK and we should accept the small overhead incurred in providing a solution that works for everyone. This prevents hardware-specific hacks in the driver. Related: we really should model bus clocks whenever possible. I've seen other attempts to merge functional/logic and bus clocks into a single entity (e.g. a single struct clk_hw/clk_core that turns both clocks on and off) and this defeats some fine-grained power management scenarios that the hardware designers had in mind when creating separate controls for the clocks. Regards, Mike > >> >> By the way while we're discussing DW HDMI bindings specific to iMX, >> >> I would recommend to remove utterly hackish and iMX only "gpr" >> >> property from the example in bindings/display/bridge/dw_hdmi.txt > > -- > Regards, > > Laurent Pinchart > -- Michael Turquette CEO BayLibre - At the Heart of Embedded Linux http://baylibre.com/
Question regarding clocks in the DW-HDMI DT bindings
Hi Andy, (CC'ing Kieran Bingham) On Monday 28 Nov 2016 19:34:59 Andy Yan wrote: > On 2016å¹´11æ26æ¥ 03:26, Laurent Pinchart wrote: > > On Friday 25 Nov 2016 17:08:13 Philipp Zabel wrote: > >> Am Freitag, den 25.11.2016, 17:45 +0200 schrieb Laurent Pinchart: > >>> On Friday 25 Nov 2016 10:56:55 Philipp Zabel wrote: > Am Donnerstag, den 24.11.2016, 23:16 +0200 schrieb Laurent Pinchart: > > Hi Andy, > > > > As the author of the DW-HDMI DT bindings this question is addressed to > > you, but information from anyone is more than welcome. > > > > The DT bindings specify two clocks named "iahb" and "isfr" but don't > > describe them. While I assume that the "isfr" clock corresponds to the > > "isfrclk" input signal of the DW HDMI, there is no "iahb" clock > > described in the IP core datasheet. > > The Table 33-1 "HDMI clocks" in the i.MX6DQ reference manual [1] lists > iahbclk (AHB bus clock), icecclk (32 kHz CEC clock), ihclk (module > clock) and isfrclk (27 MHz internal SFR clock). > > [1] > http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM. > pdf > > > The DW HDMI IP exposes control registers through an APB, not an AHB. > > There is a bus clock named "iapbclk", is "iahb" just an incorrect name > > for that clock ? > > According to figure 33-3 "HDMI TX Top Level Block Diagram" the control > interface is AMBA AHB on i.MX6. Both iahbclk and ihclk are clocked by > ahb_clk_root on i.MX6. > > Register 5 (HDMI_CONFIG1_ID) contains a few bits (confsfrdir, confi2c, > confocp, confapb, confahb) that indicate which control interface is > used. > >>> > >>> That's interesting. The DW HDMI TX documentation I found (1.40a-ea00, > >>> Early Adopter Edition) only has the confahb bit in that register. Do you > >>> know which version of the IP is used in iMX.6 ? I wonder whether the > >>> above > >>> is a Freescale-specific modification to the IP core. > >> > >> The driver reports:r > >> > >> dwhdmi-imx 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1 > >> > >> That is DESIGN_ID 0x13, REVISION_ID 0x0a. > >> > >>> I'm also a bit puzzled by the differences in the HDMI PHY between > >>> Freescale and Renesas. The Renesas datasheet documents three registers > >>> only for the PHY (other might exist and be undocumented), and while they > >>> contain fields similar to the ones documented in the Freescale > >>> datasheet, > >>> the exact register layouts are different. > >> > >> Do you mean the HDMI_PHY_* registers in the HDMI TX register space or > >> the separate HDMI PHY i2c registers? The PHY may very well be completely > >> different. I think OMAP5 HDMI driver uses the same DesignWare HDMI TX as > >> i.MX6, but not the same PHY. > > > > I meant the HDMI PHY I2C registers, yes. If the PHYs are really > > SoC-specific maybe we should move the supporting code to the platform > > glue driver. > > Yes, it's really have this case, Some Soc uses DW HDMI controller, > but uses another different phy. So it's worth moving the phy related > code to soc platform glue driver. OK, sounds like we have a plan. Kieran, here's work for you :-) > > Would anyone happen to have access to the Synopsys HDMI TX PHY > > documentation ? -- Regards, Laurent Pinchart
Question regarding clocks in the DW-HDMI DT bindings
Hi All, On 28-11-2016 11:45, Laurent Pinchart wrote: > Hi Andy, > > (CC'ing Kieran Bingham) > > On Monday 28 Nov 2016 19:34:59 Andy Yan wrote: >> On 2016å¹´11æ26æ¥ 03:26, Laurent Pinchart wrote: >>> On Friday 25 Nov 2016 17:08:13 Philipp Zabel wrote: Am Freitag, den 25.11.2016, 17:45 +0200 schrieb Laurent Pinchart: > On Friday 25 Nov 2016 10:56:55 Philipp Zabel wrote: >> Am Donnerstag, den 24.11.2016, 23:16 +0200 schrieb Laurent Pinchart: >>> Hi Andy, >>> >>> As the author of the DW-HDMI DT bindings this question is addressed to >>> you, but information from anyone is more than welcome. >>> >>> The DT bindings specify two clocks named "iahb" and "isfr" but don't >>> describe them. While I assume that the "isfr" clock corresponds to the >>> "isfrclk" input signal of the DW HDMI, there is no "iahb" clock >>> described in the IP core datasheet. >> The Table 33-1 "HDMI clocks" in the i.MX6DQ reference manual [1] lists >> iahbclk (AHB bus clock), icecclk (32 kHz CEC clock), ihclk (module >> clock) and isfrclk (27 MHz internal SFR clock). >> >> [1] >> http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM. >> pdf >> >>> The DW HDMI IP exposes control registers through an APB, not an AHB. >>> There is a bus clock named "iapbclk", is "iahb" just an incorrect name >>> for that clock ? >> According to figure 33-3 "HDMI TX Top Level Block Diagram" the control >> interface is AMBA AHB on i.MX6. Both iahbclk and ihclk are clocked by >> ahb_clk_root on i.MX6. >> >> Register 5 (HDMI_CONFIG1_ID) contains a few bits (confsfrdir, confi2c, >> confocp, confapb, confahb) that indicate which control interface is >> used. > That's interesting. The DW HDMI TX documentation I found (1.40a-ea00, > Early Adopter Edition) only has the confahb bit in that register. Do you > know which version of the IP is used in iMX.6 ? I wonder whether the > above > is a Freescale-specific modification to the IP core. The driver reports:r dwhdmi-imx 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1 That is DESIGN_ID 0x13, REVISION_ID 0x0a. > I'm also a bit puzzled by the differences in the HDMI PHY between > Freescale and Renesas. The Renesas datasheet documents three registers > only for the PHY (other might exist and be undocumented), and while they > contain fields similar to the ones documented in the Freescale > datasheet, > the exact register layouts are different. Do you mean the HDMI_PHY_* registers in the HDMI TX register space or the separate HDMI PHY i2c registers? The PHY may very well be completely different. I think OMAP5 HDMI driver uses the same DesignWare HDMI TX as i.MX6, but not the same PHY. >>> I meant the HDMI PHY I2C registers, yes. If the PHYs are really >>> SoC-specific maybe we should move the supporting code to the platform >>> glue driver. >> Yes, it's really have this case, Some Soc uses DW HDMI controller, >> but uses another different phy. So it's worth moving the phy related >> code to soc platform glue driver. As a fellow worker with DW-HDMI driver and as a Synopsys SW developer there is something I would like to say: The configuration of the phy in this scenario is made through controller registers. There is a I2C interface but the regbank of this interface is mapped in the controller, so in order to configure phy you need to have access to the controller regbank and status. There are many phys available for our customers and some of them are "custom" made. In my tests I only used one phy, but at the time I checked the source of DW-HDMI and the phy initialization procedure matched the one that we used internally for most of the phys. > OK, sounds like we have a plan. Kieran, here's work for you :-) > >>> Would anyone happen to have access to the Synopsys HDMI TX PHY >>> documentation ? I have but I can't share :( Though, we are moving into mainline now and we are planning to submit patches to DW-HDMI introducing HDMI 2.0 features, like scrambling and 4:2:0 support. Best regards, Jose Miguel Abreu
Question regarding clocks in the DW-HDMI DT bindings
Hi Philipp, On Friday 25 Nov 2016 17:08:13 Philipp Zabel wrote: > Am Freitag, den 25.11.2016, 17:45 +0200 schrieb Laurent Pinchart: > > On Friday 25 Nov 2016 10:56:55 Philipp Zabel wrote: > >> Am Donnerstag, den 24.11.2016, 23:16 +0200 schrieb Laurent Pinchart: > >>> Hi Andy, > >>> > >>> As the author of the DW-HDMI DT bindings this question is addressed to > >>> you, but information from anyone is more than welcome. > >>> > >>> The DT bindings specify two clocks named "iahb" and "isfr" but don't > >>> describe them. While I assume that the "isfr" clock corresponds to the > >>> "isfrclk" input signal of the DW HDMI, there is no "iahb" clock > >>> described in the IP core datasheet. > >> > >> The Table 33-1 "HDMI clocks" in the i.MX6DQ reference manual [1] lists > >> iahbclk (AHB bus clock), icecclk (32 kHz CEC clock), ihclk (module > >> clock) and isfrclk (27 MHz internal SFR clock). > >> > >> [1] > >> http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.p > >> df > >> > >>> The DW HDMI IP exposes control registers through an APB, not an AHB. > >>> There is a bus clock named "iapbclk", is "iahb" just an incorrect name > >>> for that clock ? > >> > >> According to figure 33-3 "HDMI TX Top Level Block Diagram" the control > >> interface is AMBA AHB on i.MX6. Both iahbclk and ihclk are clocked by > >> ahb_clk_root on i.MX6. > >> > >> Register 5 (HDMI_CONFIG1_ID) contains a few bits (confsfrdir, confi2c, > >> confocp, confapb, confahb) that indicate which control interface is > >> used. > > > > That's interesting. The DW HDMI TX documentation I found (1.40a-ea00, > > Early Adopter Edition) only has the confahb bit in that register. Do you > > know which version of the IP is used in iMX.6 ? I wonder whether the above > > is a Freescale-specific modification to the IP core. > > The driver reports: > > dwhdmi-imx 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1 > > That is DESIGN_ID 0x13, REVISION_ID 0x0a. > > > I'm also a bit puzzled by the differences in the HDMI PHY between > > Freescale and Renesas. The Renesas datasheet documents three registers > > only for the PHY (other might exist and be undocumented), and while they > > contain fields similar to the ones documented in the Freescale datasheet, > > the exact register layouts are different. > > Do you mean the HDMI_PHY_* registers in the HDMI TX register space or > the separate HDMI PHY i2c registers? The PHY may very well be completely > different. I think OMAP5 HDMI driver uses the same DesignWare HDMI TX as > i.MX6, but not the same PHY. I meant the HDMI PHY I2C registers, yes. If the PHYs are really SoC-specific maybe we should move the supporting code to the platform glue driver. Would anyone happen to have access to the Synopsys HDMI TX PHY documentation ? -- Regards, Laurent Pinchart
Question regarding clocks in the DW-HDMI DT bindings
Hi Fabio, On Friday 25 Nov 2016 13:29:37 Fabio Estevam wrote: > On Fri, Nov 25, 2016 at 1:22 PM, Laurent Pinchart wrote: > >> I got the clock name from I.MX6Q TRM, I also checked the name again > >> with Rockchip IC design team now, hope to get some new information soon. > > > > Thank you. While at it, could you ask them which version of the DW HDMI IP > > used in the SoC ? > > DW HDMI IP used in Rockchip is: > dwhdmi-rockchip ff98.hdmi: Detected HDMI controller 0x20:0xa:0xa0:0xc1 > > as shown at > https://storage.kernelci.org/mainline/v4.9-rc6-157-g16ae16c6e561/arm-multi_ > v7_defconfig/lab-collabora/boot-rk3288-rock2-square_rootfs:nfs.html > > DW HDMI IP used on mx6q is: > dwhdmi-imx 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1 Thank you for the information. This is what I get on the Renesas R-Car H3. rcar-dw-hdmi fead.hdmi0: Detected HDMI controller 0x20:0x1a:0xa0:0xc1 rcar-dw-hdmi feae.hdmi1: Detected HDMI controller 0x20:0x1a:0xa0:0xc1 -- Regards, Laurent Pinchart
Question regarding clocks in the DW-HDMI DT bindings
Hi Philipp, On Friday 25 Nov 2016 10:56:55 Philipp Zabel wrote: > Am Donnerstag, den 24.11.2016, 23:16 +0200 schrieb Laurent Pinchart: > > Hi Andy, > > > > As the author of the DW-HDMI DT bindings this question is addressed to > > you, but information from anyone is more than welcome. > > > > The DT bindings specify two clocks named "iahb" and "isfr" but don't > > describe them. While I assume that the "isfr" clock corresponds to the > > "isfrclk" input signal of the DW HDMI, there is no "iahb" clock described > > in the IP core datasheet. > > The Table 33-1 "HDMI clocks" in the i.MX6DQ reference manual [1] lists > iahbclk (AHB bus clock), icecclk (32 kHz CEC clock), ihclk (module > clock) and isfrclk (27 MHz internal SFR clock). > > [1] > http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.pdf > > > The DW HDMI IP exposes control registers through an APB, not an AHB. There > > is a bus clock named "iapbclk", is "iahb" just an incorrect name for that > > clock ? > > According to figure 33-3 "HDMI TX Top Level Block Diagram" the control > interface is AMBA AHB on i.MX6. Both iahbclk and ihclk are clocked by > ahb_clk_root on i.MX6. > > Register 5 (HDMI_CONFIG1_ID) contains a few bits (confsfrdir, confi2c, > confocp, confapb, confahb) that indicate which control interface is > used. That's interesting. The DW HDMI TX documentation I found (1.40a-ea00, Early Adopter Edition) only has the confahb bit in that register. Do you know which version of the IP is used in iMX.6 ? I wonder whether the above is a Freescale-specific modification to the IP core. I'm also a bit puzzled by the differences in the HDMI PHY between Freescale and Renesas. The Renesas datasheet documents three registers only for the PHY (other might exist and be undocumented), and while they contain fields similar to the ones documented in the Freescale datasheet, the exact register layouts are different. -- Regards, Laurent Pinchart
Question regarding clocks in the DW-HDMI DT bindings
Hi Andy, On Friday 25 Nov 2016 10:56:53 Andy Yan wrote: > On 2016å¹´11æ25æ¥ 07:26, Laurent Pinchart wrote: > > On Friday 25 Nov 2016 00:16:00 Vladimir Zapolskiy wrote: > >> On 11/25/2016 12:07 AM, Fabio Estevam wrote: > >>> On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart wrote: > Hi Andy, > > As the author of the DW-HDMI DT bindings this question is addressed to > you, but information from anyone is more than welcome. > > The DT bindings specify two clocks named "iahb" and "isfr" but don't > describe them. While I assume that the "isfr" clock corresponds to the > "isfrclk" input signal of the DW HDMI, there is no "iahb" clock > described in the IP core datasheet. > >>> > >>> i.MX6Q has a DW-HDMI IP block. > >>> > >>> The names in the devicetree binding matches the ones listed at the > >>> i.MX6Q Reference Manual - Table 33-1. HDMI Clocks > >> > >> correct, for your convenience the table is copied below: > >> > >> Clock name | Clock Root | Description > >> ---++--- > >> iahbclk | ahb_clk_root | Bus clock > >> icecclk | ckil_sync_clk_root | CEC low-frequency clock (32kHZ) > >> ihclk| ahb_clk_root | Module clock > >> isfrclk | video_27m_clk_root | Internal SFR clock (video clock > >> 27MHz) > >> > >> Here AHB stands for ARM Advanced High-performance Bus. > > > > That's what I suspected. I believe the "iahb" name is wrong, as the DW > > HDMI TX IP core clearly documents the bus clock as being called > > "iapbclk". We could rename that in the DT bindings (with compatibility > > code in the driver to keep supporting the old name) but it might not be > > worth it. The bindings should however document that the "iahb" clock is > > the IP core's "iapbclk" bus clock. > > I got the clock name from I.MX6Q TRM, I also checked the name again > with Rockchip IC design team now, hope to get some new information soon. Thank you. While at it, could you ask them which version of the DW HDMI IP used in the SoC ? > > Another question I have about the bus clock (CC'ing the devicetree mailing > > list as well as the clock maintainers) is whether it should be made > > optional. The clock is obviously mandatory from a hardware point of view > > (given that APB is a synchronous bus and thus requires a clock), but in > > some SoCs (specifically for the Renesas SoCs) that clock is always on and > > can't be controlled. We already omit bus clocks in DT for most IP cores > > when the clock can never be controlled (and we also omit a bunch of other > > clocks that we don't even know exist), so it could make sense to make the > > clock optional. Otherwise there would be runtime overhead trying to handle > > a clock that can't be controlled. > > If this is the case on Renesas SOCs, we can consider make the clock as > optional. Or move all the clock operations to platform specific > code(dw_hdmi-rockchip.c/dw_hdmi-imx.c)? I'd prefer keeping the code generic, otherwise we'd end up with platform- specific code that would perform the same operations on most platforms. I'll submit a patch soon to make the clock optional, we can discuss it then. > >> By the way while we're discussing DW HDMI bindings specific to iMX, > >> I would recommend to remove utterly hackish and iMX only "gpr" > >> property from the example in bindings/display/bridge/dw_hdmi.txt -- Regards, Laurent Pinchart
Question regarding clocks in the DW-HDMI DT bindings
Am Freitag, den 25.11.2016, 17:45 +0200 schrieb Laurent Pinchart: > Hi Philipp, > > On Friday 25 Nov 2016 10:56:55 Philipp Zabel wrote: > > Am Donnerstag, den 24.11.2016, 23:16 +0200 schrieb Laurent Pinchart: > > > Hi Andy, > > > > > > As the author of the DW-HDMI DT bindings this question is addressed to > > > you, but information from anyone is more than welcome. > > > > > > The DT bindings specify two clocks named "iahb" and "isfr" but don't > > > describe them. While I assume that the "isfr" clock corresponds to the > > > "isfrclk" input signal of the DW HDMI, there is no "iahb" clock described > > > in the IP core datasheet. > > > > The Table 33-1 "HDMI clocks" in the i.MX6DQ reference manual [1] lists > > iahbclk (AHB bus clock), icecclk (32 kHz CEC clock), ihclk (module > > clock) and isfrclk (27 MHz internal SFR clock). > > > > [1] > > http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.pdf > > > > > The DW HDMI IP exposes control registers through an APB, not an AHB. There > > > is a bus clock named "iapbclk", is "iahb" just an incorrect name for that > > > clock ? > > > > According to figure 33-3 "HDMI TX Top Level Block Diagram" the control > > interface is AMBA AHB on i.MX6. Both iahbclk and ihclk are clocked by > > ahb_clk_root on i.MX6. > > > > Register 5 (HDMI_CONFIG1_ID) contains a few bits (confsfrdir, confi2c, > > confocp, confapb, confahb) that indicate which control interface is > > used. > > That's interesting. The DW HDMI TX documentation I found (1.40a-ea00, Early > Adopter Edition) only has the confahb bit in that register. Do you know which > version of the IP is used in iMX.6 ? I wonder whether the above is a > Freescale-specific modification to the IP core. The driver reports: dwhdmi-imx 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1 That is DESIGN_ID 0x13, REVISION_ID 0x0a. > I'm also a bit puzzled by the differences in the HDMI PHY between Freescale > and Renesas. The Renesas datasheet documents three registers only for the PHY > (other might exist and be undocumented), and while they contain fields > similar > to the ones documented in the Freescale datasheet, the exact register layouts > are different. Do you mean the HDMI_PHY_* registers in the HDMI TX register space or the separate HDMI PHY i2c registers? The PHY may very well be completely different. I think OMAP5 HDMI driver uses the same DesignWare HDMI TX as i.MX6, but not the same PHY. regards Philipp
Question regarding clocks in the DW-HDMI DT bindings
On 11/25/2016 03:06 PM, Fabio Estevam wrote: > Hi Vladimir, > > On Fri, Nov 25, 2016 at 11:00 AM, Vladimir Zapolskiy > wrote: > >> according to the DTSI files in the vanilla kernel DW HDMI IP is found >> only on iMX6Q/D and iMX6DL/iMX6S SoCs (but please double check it), >> so this approach should work ideally. > > After thinking more about this I think we can not get rid of "gpr". If > we have a new i.MX SoC that is not compatible with > "fsl,imx6q-iomuxc-gpr" then the lookup will fail. > Practically and when it becomes needed it should be possible to add SoC specific hooks related to IOMUX_GPRx controls on basis of device compatibles bound to the SoC variant. Hypothetically if in future there is one more iMX SoC with a similar PCIe or SATA controller but different GPR controls, to preserve backward compatibility with old iMX6* DTB firmware the same handling of device compatibles must be done in the drivers. >> I see that the same has already been done in PCIe and SATA drivers, >> but please consider to send a similar change against iMX LDB driver > > The i.MX LDB driver is also used on imx53, so we cannot search for > "fsl,imx6q-iomuxc-gpr" compatible, as it will fail on imx53. > > So it seems we need to keep the "gpr" property in this case. > Right, I missed it. By chance GPR controls of LDB/LVDS are the same on iMX53 and iMX6*, otherwise the driver shall care about GPR controls differently, for example get a SoC specific GPR device compatible. Nevertheless it is just a suggestion and it may remain just a mental exercise of how to beautify/standardize iMX device binding descriptions. -- With best wishes, Vladimir
Question regarding clocks in the DW-HDMI DT bindings
Hi Fabio, On Friday 25 Nov 2016 10:43:04 Fabio Estevam wrote: > On Thu, Nov 24, 2016 at 9:26 PM, Laurent Pinchart wrote: > > Another question I have about the bus clock (CC'ing the devicetree mailing > > list as well as the clock maintainers) is whether it should be made > > optional. The clock is obviously mandatory from a hardware point of view > > (given that APB is a synchronous bus and thus requires a clock), but in > > some SoCs (specifically for the Renesas SoCs) that clock is always on and > > can't be controlled. We already omit bus clocks in DT for most IP cores > > when the clock can never be controlled (and we also omit a bunch of other > > clocks that we don't even know exist), so it could make sense to make the > > clock optional. Otherwise there would be runtime overhead trying to handle > > a clock that can't be controlled. > > What if you register the clock as a "dummy" clock instead? In that case I can as well specify the correct clock. My point was that, for clocks that we know are always on, specifying them in DT will lead to runtime overhead (registration of the clock, lookup, runtime handling, ...) that we could as well avoid. I think the question has a broader scope than just the dw-hdmi driver, hence the widened audience in CC. -- Regards, Laurent Pinchart
Question regarding clocks in the DW-HDMI DT bindings
Hi Fabio,
On 11/25/2016 02:29 PM, Fabio Estevam wrote:
> Hi Vladimir,
>
> On Thu, Nov 24, 2016 at 8:16 PM, Vladimir Zapolskiy
> wrote:
>
>> By the way while we're discussing DW HDMI bindings specific to iMX,
>> I would recommend to remove utterly hackish and iMX only "gpr"
>> property from the example in bindings/display/bridge/dw_hdmi.txt
>
> What if we get rid of the "gpr" property completely?
>
> --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> @@ -99,9 +99,8 @@ static const struct dw_hdmi_phy_config imx_phy_config[] = {
>
> static int dw_hdmi_imx_parse_dt(struct imx_hdmi *hdmi)
> {
> - struct device_node *np = hdmi->dev->of_node;
> + hdmi->regmap =
> syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
>
> - hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "gpr");
> if (IS_ERR(hdmi->regmap)) {
> dev_err(hdmi->dev, "Unable to get gpr\n");
> return PTR_ERR(hdmi->regmap);
>
> Then we can remove the gpr from the device tree files.
>
according to the DTSI files in the vanilla kernel DW HDMI IP is found
only on iMX6Q/D and iMX6DL/iMX6S SoCs (but please double check it),
so this approach should work ideally.
I see that the same has already been done in PCIe and SATA drivers,
but please consider to send a similar change against iMX LDB driver
including removal of the property from imx6qdl.dtsi and
Documentation/devicetree/bindings/display/imx/ldb.txt.
And back to DW HDMI IP it seems that after removing "gpr" property
Documentation/devicetree/bindings/display/imx/hdmi.txt can be removed,
because bindings/display/bridge/dw_hdmi.txt replaces it.
--
With best wishes,
Vladimir
Question regarding clocks in the DW-HDMI DT bindings
On Fri, Nov 25, 2016 at 1:22 PM, Laurent Pinchart wrote: >> I got the clock name from I.MX6Q TRM, I also checked the name again >> with Rockchip IC design team now, hope to get some new information soon. > > Thank you. While at it, could you ask them which version of the DW HDMI IP > used in the SoC ? DW HDMI IP used in Rockchip is: dwhdmi-rockchip ff98.hdmi: Detected HDMI controller 0x20:0xa:0xa0:0xc1 as shown at https://storage.kernelci.org/mainline/v4.9-rc6-157-g16ae16c6e561/arm-multi_v7_defconfig/lab-collabora/boot-rk3288-rock2-square_rootfs:nfs.html DW HDMI IP used on mx6q is: dwhdmi-imx 12.hdmi: Detected HDMI controller 0x13:0xa:0xa0:0xc1
Question regarding clocks in the DW-HDMI DT bindings
Hi Vladimir, On Fri, Nov 25, 2016 at 11:00 AM, Vladimir Zapolskiy wrote: > according to the DTSI files in the vanilla kernel DW HDMI IP is found > only on iMX6Q/D and iMX6DL/iMX6S SoCs (but please double check it), > so this approach should work ideally. After thinking more about this I think we can not get rid of "gpr". If we have a new i.MX SoC that is not compatible with "fsl,imx6q-iomuxc-gpr" then the lookup will fail. > I see that the same has already been done in PCIe and SATA drivers, > but please consider to send a similar change against iMX LDB driver The i.MX LDB driver is also used on imx53, so we cannot search for "fsl,imx6q-iomuxc-gpr" compatible, as it will fail on imx53. So it seems we need to keep the "gpr" property in this case.
Question regarding clocks in the DW-HDMI DT bindings
Hi Laurent, Am Donnerstag, den 24.11.2016, 23:16 +0200 schrieb Laurent Pinchart: > Hi Andy, > > As the author of the DW-HDMI DT bindings this question is addressed to you, > but information from anyone is more than welcome. > > The DT bindings specify two clocks named "iahb" and "isfr" but don't describe > them. While I assume that the "isfr" clock corresponds to the "isfrclk" input > signal of the DW HDMI, there is no "iahb" clock described in the IP core > datasheet. The Table 33-1 "HDMI clocks" in the i.MX6DQ reference manual [1] lists iahbclk (AHB bus clock), icecclk (32 kHz CEC clock), ihclk (module clock) and isfrclk (27 MHz internal SFR clock). [1] http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.pdf > The DW HDMI IP exposes control registers through an APB, not an AHB. There is > a bus clock named "iapbclk", is "iahb" just an incorrect name for that clock ? According to figure 33-3 "HDMI TX Top Level Block Diagram" the control interface is AMBA AHB on i.MX6. Both iahbclk and ihclk are clocked by ahb_clk_root on i.MX6. Register 5 (HDMI_CONFIG1_ID) contains a few bits (confsfrdir, confi2c, confocp, confapb, confahb) that indicate which control interface is used. regards Philipp
Question regarding clocks in the DW-HDMI DT bindings
Hi: On 2016å¹´11æ25æ¥ 07:26, Laurent Pinchart wrote: > Hello Fabio and Vladimir, > > Thank you for your quick responses. > > On Friday 25 Nov 2016 00:16:00 Vladimir Zapolskiy wrote: >> On 11/25/2016 12:07 AM, Fabio Estevam wrote: >>> On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart wrote: Hi Andy, As the author of the DW-HDMI DT bindings this question is addressed to you, but information from anyone is more than welcome. The DT bindings specify two clocks named "iahb" and "isfr" but don't describe them. While I assume that the "isfr" clock corresponds to the "isfrclk" input signal of the DW HDMI, there is no "iahb" clock described in the IP core datasheet. >>> i.MX6Q has a DW-HDMI IP block. >>> >>> The names in the devicetree binding matches the ones listed at the >>> i.MX6Q Reference Manual - Table 33-1. HDMI Clocks >> correct, for your convenience the table is copied below: >> >> Clock name | Clock Root | Description >> ---++--- >> iahbclk | ahb_clk_root | Bus clock >> icecclk | ckil_sync_clk_root | CEC low-frequency clock (32kHZ) >> ihclk| ahb_clk_root | Module clock >> isfrclk | video_27m_clk_root | Internal SFR clock (video clock 27MHz) >> >> Here AHB stands for ARM Advanced High-performance Bus. > That's what I suspected. I believe the "iahb" name is wrong, as the DW HDMI TX > IP core clearly documents the bus clock as being called "iapbclk". We could > rename that in the DT bindings (with compatibility code in the driver to keep > supporting the old name) but it might not be worth it. The bindings should > however document that the "iahb" clock is the IP core's "iapbclk" bus clock. I got the clock name from I.MX6Q TRM, I also checked the name again with Rockchip IC design team now, hope to get some new information soon. > > Another question I have about the bus clock (CC'ing the devicetree mailing > list as well as the clock maintainers) is whether it should be made optional. > The clock is obviously mandatory from a hardware point of view (given that APB > is a synchronous bus and thus requires a clock), but in some SoCs > (specifically for the Renesas SoCs) that clock is always on and can't be > controlled. We already omit bus clocks in DT for most IP cores when the clock > can never be controlled (and we also omit a bunch of other clocks that we > don't even know exist), so it could make sense to make the clock optional. > Otherwise there would be runtime overhead trying to handle a clock that can't > be controlled. If this is the case on Renesas SOCs, we can consider make the clock as optional. Or move all the clock operations to platform specific code(dw_hdmi-rockchip.c/dw_hdmi-imx.c)? >> By the way while we're discussing DW HDMI bindings specific to iMX, >> I would recommend to remove utterly hackish and iMX only "gpr" >> property from the example in bindings/display/bridge/dw_hdmi.txt
Question regarding clocks in the DW-HDMI DT bindings
Hi Laurent, On Thu, Nov 24, 2016 at 9:26 PM, Laurent Pinchart wrote: > Another question I have about the bus clock (CC'ing the devicetree mailing > list as well as the clock maintainers) is whether it should be made optional. > The clock is obviously mandatory from a hardware point of view (given that APB > is a synchronous bus and thus requires a clock), but in some SoCs > (specifically for the Renesas SoCs) that clock is always on and can't be > controlled. We already omit bus clocks in DT for most IP cores when the clock > can never be controlled (and we also omit a bunch of other clocks that we > don't even know exist), so it could make sense to make the clock optional. > Otherwise there would be runtime overhead trying to handle a clock that can't > be controlled. What if you register the clock as a "dummy" clock instead?
Question regarding clocks in the DW-HDMI DT bindings
Hi Vladimir,
On Thu, Nov 24, 2016 at 8:16 PM, Vladimir Zapolskiy
wrote:
> By the way while we're discussing DW HDMI bindings specific to iMX,
> I would recommend to remove utterly hackish and iMX only "gpr"
> property from the example in bindings/display/bridge/dw_hdmi.txt
What if we get rid of the "gpr" property completely?
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -99,9 +99,8 @@ static const struct dw_hdmi_phy_config imx_phy_config[] = {
static int dw_hdmi_imx_parse_dt(struct imx_hdmi *hdmi)
{
- struct device_node *np = hdmi->dev->of_node;
+ hdmi->regmap =
syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
- hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "gpr");
if (IS_ERR(hdmi->regmap)) {
dev_err(hdmi->dev, "Unable to get gpr\n");
return PTR_ERR(hdmi->regmap);
Then we can remove the gpr from the device tree files.
Question regarding clocks in the DW-HDMI DT bindings
Hello Fabio and Vladimir, Thank you for your quick responses. On Friday 25 Nov 2016 00:16:00 Vladimir Zapolskiy wrote: > On 11/25/2016 12:07 AM, Fabio Estevam wrote: > > On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart wrote: > >> Hi Andy, > >> > >> As the author of the DW-HDMI DT bindings this question is addressed to > >> you, but information from anyone is more than welcome. > >> > >> The DT bindings specify two clocks named "iahb" and "isfr" but don't > >> describe them. While I assume that the "isfr" clock corresponds to the > >> "isfrclk" input signal of the DW HDMI, there is no "iahb" clock > >> described in the IP core datasheet. > > > > i.MX6Q has a DW-HDMI IP block. > > > > The names in the devicetree binding matches the ones listed at the > > i.MX6Q Reference Manual - Table 33-1. HDMI Clocks > > correct, for your convenience the table is copied below: > > Clock name | Clock Root | Description > ---++--- >iahbclk | ahb_clk_root | Bus clock >icecclk | ckil_sync_clk_root | CEC low-frequency clock (32kHZ) >ihclk| ahb_clk_root | Module clock >isfrclk | video_27m_clk_root | Internal SFR clock (video clock 27MHz) > > Here AHB stands for ARM Advanced High-performance Bus. That's what I suspected. I believe the "iahb" name is wrong, as the DW HDMI TX IP core clearly documents the bus clock as being called "iapbclk". We could rename that in the DT bindings (with compatibility code in the driver to keep supporting the old name) but it might not be worth it. The bindings should however document that the "iahb" clock is the IP core's "iapbclk" bus clock. Another question I have about the bus clock (CC'ing the devicetree mailing list as well as the clock maintainers) is whether it should be made optional. The clock is obviously mandatory from a hardware point of view (given that APB is a synchronous bus and thus requires a clock), but in some SoCs (specifically for the Renesas SoCs) that clock is always on and can't be controlled. We already omit bus clocks in DT for most IP cores when the clock can never be controlled (and we also omit a bunch of other clocks that we don't even know exist), so it could make sense to make the clock optional. Otherwise there would be runtime overhead trying to handle a clock that can't be controlled. > By the way while we're discussing DW HDMI bindings specific to iMX, > I would recommend to remove utterly hackish and iMX only "gpr" > property from the example in bindings/display/bridge/dw_hdmi.txt -- Regards, Laurent Pinchart
Question regarding clocks in the DW-HDMI DT bindings
Hi, On 11/25/2016 12:07 AM, Fabio Estevam wrote: > Hi Laurent, > > On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart > wrote: >> Hi Andy, >> >> As the author of the DW-HDMI DT bindings this question is addressed to you, >> but information from anyone is more than welcome. >> >> The DT bindings specify two clocks named "iahb" and "isfr" but don't describe >> them. While I assume that the "isfr" clock corresponds to the "isfrclk" input >> signal of the DW HDMI, there is no "iahb" clock described in the IP core >> datasheet. > > i.MX6Q has a DW-HDMI IP block. > > The names in the devicetree binding matches the ones listed at the > i.MX6Q Reference Manual - Table 33-1. HDMI Clocks correct, for your convenience the table is copied below: Clock name | Clock Root | Description ---++--- iahbclk | ahb_clk_root | Bus clock icecclk | ckil_sync_clk_root | CEC low-frequency clock (32kHZ) ihclk| ahb_clk_root | Module clock isfrclk | video_27m_clk_root | Internal SFR clock (video clock 27MHz) Here AHB stands for ARM Advanced High-performance Bus. By the way while we're discussing DW HDMI bindings specific to iMX, I would recommend to remove utterly hackish and iMX only "gpr" property from the example in bindings/display/bridge/dw_hdmi.txt -- With best wishes, Vladimir
Question regarding clocks in the DW-HDMI DT bindings
Hi Andy, As the author of the DW-HDMI DT bindings this question is addressed to you, but information from anyone is more than welcome. The DT bindings specify two clocks named "iahb" and "isfr" but don't describe them. While I assume that the "isfr" clock corresponds to the "isfrclk" input signal of the DW HDMI, there is no "iahb" clock described in the IP core datasheet. The DW HDMI IP exposes control registers through an APB, not an AHB. There is a bus clock named "iapbclk", is "iahb" just an incorrect name for that clock ? -- Regards, Laurent Pinchart
Question regarding clocks in the DW-HDMI DT bindings
On Thu, Nov 24, 2016 at 8:16 PM, Vladimir Zapolskiy wrote: > correct, for your convenience the table is copied below: > > Clock name | Clock Root | Description > ---++--- > iahbclk | ahb_clk_root | Bus clock > icecclk | ckil_sync_clk_root | CEC low-frequency clock (32kHZ) > ihclk| ahb_clk_root | Module clock > isfrclk | video_27m_clk_root | Internal SFR clock (video clock 27MHz) > > Here AHB stands for ARM Advanced High-performance Bus. > > By the way while we're discussing DW HDMI bindings specific to iMX, > I would recommend to remove utterly hackish and iMX only "gpr" > property from the example in bindings/display/bridge/dw_hdmi.txt Yes, the "gpr" property is i.MX specific and it is described in Documentation/devicetree/bindings/display/imx/hdmi.txt, so we can remove it from dw_hdmi.txt. Will send a patch removing it. Thanks
Question regarding clocks in the DW-HDMI DT bindings
Hi Laurent, On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart wrote: > Hi Andy, > > As the author of the DW-HDMI DT bindings this question is addressed to you, > but information from anyone is more than welcome. > > The DT bindings specify two clocks named "iahb" and "isfr" but don't describe > them. While I assume that the "isfr" clock corresponds to the "isfrclk" input > signal of the DW HDMI, there is no "iahb" clock described in the IP core > datasheet. i.MX6Q has a DW-HDMI IP block. The names in the devicetree binding matches the ones listed at the i.MX6Q Reference Manual - Table 33-1. HDMI Clocks Regards, Fabio Estevam
