Re: [PATCH V5 1/7] imx-drm: imx-hdmi: split imx soc specific code from imx-hdmi
Hi Andy, A few comments inline. On 08/11/14 05:28, Andy Yan wrote: imx6 and rockchip rk3288 and JZ4780 (Ingenic Xburst/MIPS) use the interface compatible Designware HDMI IP, but they also have some lightly difference, such as phy pll configuration, register width, 4K support, clk useage, and the crtc mux configuration is also platform specific. To reuse the imx hdmi driver, split the platform specific code out to dw_hdmi-imx.c. Signed-off-by: Andy Yan andy@rock-chips.com --- drivers/staging/imx-drm/Makefile | 2 +- drivers/staging/imx-drm/dw_hdmi-imx.c | 214 drivers/staging/imx-drm/imx-hdmi.c| 257 -- drivers/staging/imx-drm/imx-hdmi.h| 43 ++ 4 files changed, 320 insertions(+), 196 deletions(-) create mode 100644 drivers/staging/imx-drm/dw_hdmi-imx.c diff --git a/drivers/staging/imx-drm/Makefile b/drivers/staging/imx-drm/Makefile index 582c438..809027d 100644 --- a/drivers/staging/imx-drm/Makefile +++ b/drivers/staging/imx-drm/Makefile @@ -9,4 +9,4 @@ obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o imx-ipuv3-crtc-objs := ipuv3-crtc.o ipuv3-plane.o obj-$(CONFIG_DRM_IMX_IPUV3) += imx-ipuv3-crtc.o -obj-$(CONFIG_DRM_IMX_HDMI) += imx-hdmi.o +obj-$(CONFIG_DRM_IMX_HDMI) += imx-hdmi.o dw_hdmi-imx.o diff --git a/drivers/staging/imx-drm/dw_hdmi-imx.c b/drivers/staging/imx-drm/dw_hdmi-imx.c new file mode 100644 index 000..5422679 --- /dev/null +++ b/drivers/staging/imx-drm/dw_hdmi-imx.c @@ -0,0 +1,214 @@ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ Please add the old freescale copyrights and a comment on how this file is derived from. And a note on platform specific file for imx hdmi using dwc hdmi drm bridge. +#include linux/module.h +#include linux/platform_device.h +#include linux/mfd/syscon.h +#include linux/mfd/syscon/imx6q-iomuxc-gpr.h +#include video/imx-ipu-v3.h +#include linux/regmap.h +#include linux/clk.h + +#include imx-drm.h +#include imx-hdmi.h + +struct imx_hdmi_priv { + struct device *dev; + struct clk *isfr_clk; + struct clk *iahb_clk; + struct regmap *regmap; +}; + +static const struct mpll_config imx_mpll_cfg[] = { + { + 4525, { + { 0x01e0, 0x }, + { 0x21e1, 0x }, + { 0x41e2, 0x } + }, + }, { + 9250, { + { 0x0140, 0x0005 }, + { 0x2141, 0x0005 }, + { 0x4142, 0x0005 }, + }, + }, { + 14850, { + { 0x00a0, 0x000a }, + { 0x20a1, 0x000a }, + { 0x40a2, 0x000a }, + }, + }, { + ~0UL, { + { 0x00a0, 0x000a }, + { 0x2001, 0x000f }, + { 0x4002, 0x000f }, + }, + } +}; + +static const struct curr_ctrl imx_cur_ctr[] = { + /* pixelclk bpp8bpp10 bpp12 */ + { + 5400, { 0x091c, 0x091c, 0x06dc }, + }, { + 5840, { 0x091c, 0x06dc, 0x06dc }, + }, { + 7200, { 0x06dc, 0x06dc, 0x091c }, + }, { + 7425, { 0x06dc, 0x0b5c, 0x091c }, + }, { + 11880, { 0x091c, 0x091c, 0x06dc }, + }, { + 21600, { 0x06dc, 0x0b5c, 0x091c }, + } +}; These pll clocks. Are they completely different? I assume they are dependent on display resolutions. The old ones should remain the same. With new additions for 4K clock rates e.g. + +static int imx_hdmi_parse_dt(struct imx_hdmi_priv *hdmi) +{ + struct device_node *np = hdmi-dev-of_node; + + 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); + } + + hdmi-isfr_clk = devm_clk_get(hdmi-dev, isfr); + if (IS_ERR(hdmi-isfr_clk)) { + dev_err(hdmi-dev, Unable to get HDMI isfr clk\n); + return PTR_ERR(hdmi-isfr_clk); + } + + hdmi-iahb_clk = devm_clk_get(hdmi-dev, iahb); + if (IS_ERR(hdmi-iahb_clk)) { + dev_err(hdmi-dev, Unable to get HDMI iahb clk\n); + return PTR_ERR(hdmi-iahb_clk); + } + These two clocks. This is how they are structured on the jz4780 as well. From an earlier patch series yes, this IP core need to be clocked. But different Soc has different usage of the clk, on freescale imx platform one clk is used for isfr, one is used for iahb, but on rockchip rk3288, one clk is used for control logic , the another is used for hdcp. I am not sure how are the clocks on jz4780 They might be
Re: [PATCH V5 1/7] imx-drm: imx-hdmi: split imx soc specific code from imx-hdmi
Hi Zubair: thanks very much for your comments. On 2014年11月10日 18:51, Zubair Lutfullah Kakakhel wrote: Hi Andy, A few comments inline. On 08/11/14 05:28, Andy Yan wrote: imx6 and rockchip rk3288 and JZ4780 (Ingenic Xburst/MIPS) use the interface compatible Designware HDMI IP, but they also have some lightly difference, such as phy pll configuration, register width, 4K support, clk useage, and the crtc mux configuration is also platform specific. To reuse the imx hdmi driver, split the platform specific code out to dw_hdmi-imx.c. Signed-off-by: Andy Yan andy@rock-chips.com --- drivers/staging/imx-drm/Makefile | 2 +- drivers/staging/imx-drm/dw_hdmi-imx.c | 214 drivers/staging/imx-drm/imx-hdmi.c| 257 -- drivers/staging/imx-drm/imx-hdmi.h| 43 ++ 4 files changed, 320 insertions(+), 196 deletions(-) create mode 100644 drivers/staging/imx-drm/dw_hdmi-imx.c diff --git a/drivers/staging/imx-drm/Makefile b/drivers/staging/imx-drm/Makefile index 582c438..809027d 100644 --- a/drivers/staging/imx-drm/Makefile +++ b/drivers/staging/imx-drm/Makefile @@ -9,4 +9,4 @@ obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o imx-ipuv3-crtc-objs := ipuv3-crtc.o ipuv3-plane.o obj-$(CONFIG_DRM_IMX_IPUV3) += imx-ipuv3-crtc.o -obj-$(CONFIG_DRM_IMX_HDMI) += imx-hdmi.o +obj-$(CONFIG_DRM_IMX_HDMI) += imx-hdmi.o dw_hdmi-imx.o diff --git a/drivers/staging/imx-drm/dw_hdmi-imx.c b/drivers/staging/imx-drm/dw_hdmi-imx.c new file mode 100644 index 000..5422679 --- /dev/null +++ b/drivers/staging/imx-drm/dw_hdmi-imx.c @@ -0,0 +1,214 @@ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ Please add the old freescale copyrights and a comment on how this file is derived from. ok, I will add it in V7 And a note on platform specific file for imx hdmi using dwc hdmi drm bridge. sorry, I am not clear which specific file you means. Would you please tell me more about the note? +#include linux/module.h +#include linux/platform_device.h +#include linux/mfd/syscon.h +#include linux/mfd/syscon/imx6q-iomuxc-gpr.h +#include video/imx-ipu-v3.h +#include linux/regmap.h +#include linux/clk.h + +#include imx-drm.h +#include imx-hdmi.h + +struct imx_hdmi_priv { + struct device *dev; + struct clk *isfr_clk; + struct clk *iahb_clk; + struct regmap *regmap; +}; + +static const struct mpll_config imx_mpll_cfg[] = { + { + 4525, { + { 0x01e0, 0x }, + { 0x21e1, 0x }, + { 0x41e2, 0x } + }, + }, { + 9250, { + { 0x0140, 0x0005 }, + { 0x2141, 0x0005 }, + { 0x4142, 0x0005 }, + }, + }, { + 14850, { + { 0x00a0, 0x000a }, + { 0x20a1, 0x000a }, + { 0x40a2, 0x000a }, + }, + }, { + ~0UL, { + { 0x00a0, 0x000a }, + { 0x2001, 0x000f }, + { 0x4002, 0x000f }, + }, + } +}; + +static const struct curr_ctrl imx_cur_ctr[] = { + /* pixelclk bpp8bpp10 bpp12 */ + { + 5400, { 0x091c, 0x091c, 0x06dc }, + }, { + 5840, { 0x091c, 0x06dc, 0x06dc }, + }, { + 7200, { 0x06dc, 0x06dc, 0x091c }, + }, { + 7425, { 0x06dc, 0x0b5c, 0x091c }, + }, { + 11880, { 0x091c, 0x091c, 0x06dc }, + }, { + 21600, { 0x06dc, 0x0b5c, 0x091c }, + } +}; These pll clocks. Are they completely different? I assume they are dependent on display resolutions. The old ones should remain the same. With new additions for 4K clock rates e.g. even the 1080P on rockchip platform, we need a different configuration, otherwise we can't get the best SI. + +static int imx_hdmi_parse_dt(struct imx_hdmi_priv *hdmi) +{ + struct device_node *np = hdmi-dev-of_node; + + 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); + } + + hdmi-isfr_clk = devm_clk_get(hdmi-dev, isfr); + if (IS_ERR(hdmi-isfr_clk)) { + dev_err(hdmi-dev, Unable to get HDMI isfr clk\n); + return PTR_ERR(hdmi-isfr_clk); + } + + hdmi-iahb_clk = devm_clk_get(hdmi-dev, iahb); + if (IS_ERR(hdmi-iahb_clk)) { + dev_err(hdmi-dev, Unable to get HDMI iahb clk\n); + return PTR_ERR(hdmi-iahb_clk); + } + These two clocks. This is how they are structured on the