Hi Andy, This driver adds HDMI to rockchip/drm. The fact that rockchip's hdmi uses dw_hdmi is an implementation detail. I do not think that the names used for rk3288-hdmi should include "dw" in them. See inline for what I mean... On Thu, Dec 4, 2014 at 10:34 PM, Andy Yan <andy.yan@xxxxxxxxxxxxxx> wrote: > Rockchip RK3288 hdmi is compatible with dw_hdmi > > this patch is depend on patch by Mark Yao > drm: rockchip: Add basic drm driver > see https://lkml.org/lkml/2014/12/2/161 > > Signed-off-by: Andy Yan <andy.yan@xxxxxxxxxxxxxx> > > --- > > Changes in v18: None > Changes in v17: > - parse resource and irq in platform driver > > Changes in v16: None > Changes in v15: > - remove THIS_MODULE in platform driver > > Changes in v14: None > Changes in v13: None > Changes in v12: > - add comment for the depend on patch > > Changes in v11: None > Changes in v10: > - add more display mode support mpll configuration for rk3288 > > Changes in v9: > - move some phy configuration to platform driver > > Changes in v8: None > Changes in v7: None > Changes in v6: None > Changes in v5: None > Changes in v4: None > Changes in v3: None > > drivers/gpu/drm/bridge/dw_hdmi.c | 3 + > drivers/gpu/drm/rockchip/Kconfig | 10 + > drivers/gpu/drm/rockchip/Makefile | 2 + > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 341 ++++++++++++++++++++++++++++ > include/drm/bridge/dw_hdmi.h | 1 + > 5 files changed, 357 insertions(+) > create mode 100644 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > > diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c > index cecc46a..01c95a8 100644 > --- a/drivers/gpu/drm/bridge/dw_hdmi.c > +++ b/drivers/gpu/drm/bridge/dw_hdmi.c > @@ -852,6 +852,9 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep, > dw_hdmi_phy_gen2_txpwron(hdmi, 1); > dw_hdmi_phy_gen2_pddq(hdmi, 0); > > + if (hdmi->dev_type == RK3288_HDMI) > + dw_hdmi_phy_enable_spare(hdmi, 1); > + > /*Wait for PHY PLL lock */ > msec = 5; > do { > diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig > index ca9f085..6ebebe8 100644 > --- a/drivers/gpu/drm/rockchip/Kconfig > +++ b/drivers/gpu/drm/rockchip/Kconfig > @@ -15,3 +15,13 @@ config DRM_ROCKCHIP > management to userspace. This driver does not provide > 2D or 3D acceleration; acceleration is performed by other > IP found on the SoC. > + > +config ROCKCHIP_DW_HDMI I would rather call this ROCKCHIP_HDMI, since this driver implements the HDMI for Rockchip. The fact that it uses dw_hdmi is an implementation detail. > + bool "Rockchip specific extensions for Synopsys DW HDMI" > + depends on DRM_ROCKCHIP > + select DRM_DW_HDMI > + help > + This selects support for Rockchip SoC specific extensions > + for the Synopsys DesignWare HDMI driver. If you want to > + enable HDMI on RK3288 based SoC, you should selet this > + option. This could become simply: Select this option to enable HDMI support for Rockchip SoCs. > diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile > index 2cb0672..beed7df 100644 > --- a/drivers/gpu/drm/rockchip/Makefile > +++ b/drivers/gpu/drm/rockchip/Makefile > @@ -5,4 +5,6 @@ > rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o rockchip_drm_fbdev.o \ > rockchip_drm_gem.o > > +rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o > + > obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o rockchip_drm_vop.o > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > new file mode 100644 > index 0000000..11d54b0 > --- /dev/null > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c Similarly, I'd rather this file be called drm_rockchip_hdmi.c to be consistent with the rest of the files in drm/rockchip. > @@ -0,0 +1,341 @@ > +/* > + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/mfd/syscon.h> > +#include <linux/regmap.h> > +#include <drm/drm_of.h> > +#include <drm/drmP.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_edid.h> > +#include <drm/drm_encoder_slave.h> > +#include <drm/bridge/dw_hdmi.h> > + > +#include "rockchip_drm_drv.h" > +#include "rockchip_drm_vop.h" > + > +#define GRF_SOC_CON6 0x025c > +#define HDMI_SEL_VOP_LIT (1 << 4) > + > +struct rockchip_hdmi { > + struct device *dev; > + struct regmap *regmap; > + struct drm_encoder encoder; > +}; > + > +#define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x) > + > +static const struct dw_hdmi_mpll_config rockchip_mpll_cfg[] = { Let's stick to mpll_config. Not much value to abbreviate an abbreviation. > + { > + 27000000, { > + { 0x00b3, 0x0000}, space before all of these '}'. > + { 0x2153, 0x0000}, > + { 0x40f3, 0x0000} > + }, > + }, { > + 36000000, { > + { 0x00b3, 0x0000}, > + { 0x2153, 0x0000}, > + { 0x40f3, 0x0000} > + }, > + }, { > + 40000000, { > + { 0x00b3, 0x0000}, > + { 0x2153, 0x0000}, > + { 0x40f3, 0x0000} > + }, > + }, { > + 54000000, { > + { 0x0072, 0x0001}, > + { 0x2142, 0x0001}, > + { 0x40a2, 0x0001}, > + }, > + }, { > + 65000000, { > + { 0x0072, 0x0001}, > + { 0x2142, 0x0001}, > + { 0x40a2, 0x0001}, > + }, > + }, { > + 66000000, { > + { 0x013e, 0x0003}, > + { 0x217e, 0x0002}, > + { 0x4061, 0x0002} > + }, > + }, { > + 74250000, { > + { 0x0072, 0x0001}, > + { 0x2145, 0x0002}, > + { 0x4061, 0x0002} > + }, > + }, { > + 83500000, { > + { 0x0072, 0x0001}, > + }, > + }, { > + 108000000, { > + { 0x0051, 0x0002}, > + { 0x2145, 0x0002}, > + { 0x4061, 0x0002} > + }, > + }, { > + 106500000, { > + { 0x0051, 0x0002}, > + { 0x2145, 0x0002}, > + { 0x4061, 0x0002} > + }, > + }, { > + 146250000, { > + { 0x0051, 0x0002}, > + { 0x2145, 0x0002}, > + { 0x4061, 0x0002} > + }, > + }, { > + 148500000, { > + { 0x0051, 0x0003}, > + { 0x214c, 0x0003}, > + { 0x4064, 0x0003} > + }, > + }, { > + ~0UL, { > + { 0x00a0, 0x000a }, > + { 0x2001, 0x000f }, > + { 0x4002, 0x000f }, > + }, > + } > +}; > + > +static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = { > + /* pixelclk bpp8 bpp10 bpp12 */ > + { > + 40000000, { 0x0018, 0x0018, 0x0018 }, > + }, { > + 65000000, { 0x0028, 0x0028, 0x0028 }, > + }, { > + 66000000, { 0x0038, 0x0038, 0x0038 }, > + }, { > + 74250000, { 0x0028, 0x0038, 0x0038 }, > + }, { > + 83500000, { 0x0028, 0x0038, 0x0038 }, > + }, { > + 146250000, { 0x0038, 0x0038, 0x0038 }, > + }, { > + 148500000, { 0x0000, 0x0038, 0x0038 }, > + }, { > + ~0UL, { 0x0000, 0x0000, 0x0000}, > + } > +}; > + > +static const struct dw_hdmi_sym_term rockchip_sym_term[] = { > + /*pixelclk symbol term*/ > + { 74250000, 0x8009, 0x0004 }, > + { 148500000, 0x8029, 0x0004 }, > + { 297000000, 0x8039, 0x0005 }, > + { ~0UL, 0x0000, 0x0000 } > +}; > + > +static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi) > +{ > + struct device_node *np = hdmi->dev->of_node; > + > + hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf"); > + if (IS_ERR(hdmi->regmap)) { > + dev_err(hdmi->dev, "Unable to get rockchip,grf\n"); > + return PTR_ERR(hdmi->regmap); > + } > + > + return 0; > +} > + > +static enum drm_mode_status > +dw_hdmi_rockchip_mode_valid(struct drm_connector *connector, Similarly, I would rename these function names to start with rockchip_hdmi (or maybe rk_hdmi for brevity). Especially the ones for the module & driver: (bind/unbind/probe/remove). > + struct drm_display_mode *mode) > +{ > + const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg; > + int pclk = mode->clock * 1000; > + bool valid = false; > + int i; > + > + for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) { > + if (pclk == mpll_cfg[i].mpixelclock) { > + valid = true; Perhaps you can simplify this a bit: for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) if (pclk == mpll_cfg[i].mpixelclock) return MODE_OK; return MODE_BAD; > +} > + > +static struct drm_encoder_funcs dw_hdmi_rockchip_encoder_funcs = { > + .destroy = drm_encoder_cleanup, > +}; > + > +static void dw_hdmi_rockchip_encoder_disable(struct drm_encoder *encoder) > +{ > +} > + > +static bool > +dw_hdmi_rockchip_encoder_mode_fixup(struct drm_encoder *encoder, > + const struct drm_display_mode *mode, > + struct drm_display_mode *adj_mode) > +{ > + return true; > +} > + > +static void dw_hdmi_rockchip_encoder_mode_set(struct drm_encoder *encoder, > + struct drm_display_mode *mode, > + struct drm_display_mode *adj_mode) > +{ > +} > + > +static void dw_hdmi_rockchip_encoder_commit(struct drm_encoder *encoder) > +{ > + struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder); > + u32 val; > + int mux; > + > + mux = rockchip_drm_encoder_get_mux_id(hdmi->dev->of_node, encoder); > + if (mux) > + val = HDMI_SEL_VOP_LIT | (HDMI_SEL_VOP_LIT << 16); > + else > + val = HDMI_SEL_VOP_LIT << 16; > + > + regmap_write(hdmi->regmap, GRF_SOC_CON6, val); > + dev_dbg(hdmi->dev, "vop %s output to hdmi\n", > + (mux) ? "LIT" : "BIG"); > +} > + > +static void dw_hdmi_rockchip_encoder_prepare(struct drm_encoder *encoder) > +{ > + rockchip_drm_crtc_mode_config(encoder->crtc, DRM_MODE_CONNECTOR_HDMIA, > + ROCKCHIP_OUT_MODE_AAAA); > +} > + > +static struct drm_encoder_helper_funcs dw_hdmi_rockchip_encoder_helper_funcs = { "static const" here and for the other function tables. > + .mode_fixup = dw_hdmi_rockchip_encoder_mode_fixup, > + .mode_set = dw_hdmi_rockchip_encoder_mode_set, > + .prepare = dw_hdmi_rockchip_encoder_prepare, > + .commit = dw_hdmi_rockchip_encoder_commit, > + .disable = dw_hdmi_rockchip_encoder_disable, > +}; > + > +static const struct dw_hdmi_plat_data rockchip_hdmi_drv_data = { > + .mode_valid = dw_hdmi_rockchip_mode_valid, > + .mpll_cfg = rockchip_mpll_cfg, > + .cur_ctr = rockchip_cur_ctr, > + .sym_term = rockchip_sym_term, > + .dev_type = RK3288_HDMI, > +}; > + > +static const struct of_device_id dw_hdmi_rockchip_ids[] = { > + { .compatible = "rockchip,rk3288-dw-hdmi", .compatible = "rockchip,rk3288-hdmi", > + .data = &rockchip_hdmi_drv_data > + }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, dw_hdmi_rockchip_dt_ids); > + > +static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master, > + void *data) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + const struct dw_hdmi_plat_data *plat_data; > + const struct of_device_id *match; > + struct drm_device *drm = data; > + struct drm_encoder *encoder; > + struct rockchip_hdmi *hdmi; > + struct resource *iores; > + int irq; > + int ret; > + > + if (!pdev->dev.of_node) > + return -ENODEV; > + > + hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL); > + if (!hdmi) > + return -ENOMEM; > + > + match = of_match_node(dw_hdmi_rockchip_ids, pdev->dev.of_node); > + plat_data = match->data; > + hdmi->dev = &pdev->dev; > + encoder = &hdmi->encoder; > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > + > + iores = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!iores) > + return -ENXIO; > + > + platform_set_drvdata(pdev, hdmi); > + > + encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node); > + /* > + * If we failed to find the CRTC(s) which this encoder is > + * supposed to be connected to, it's because the CRTC has > + * not been registered yet. Defer probing, and hope that > + * the required CRTC is added later. Nit: it looks like the text lines for this comment could be longer > + */ > + if (encoder->possible_crtcs == 0) > + return -EPROBE_DEFER; > + > + ret = rockchip_hdmi_parse_dt(hdmi); > + if (ret) { > + dev_err(hdmi->dev, "Unable to parse OF data\n"); > + return ret; > + } > + > + drm_encoder_helper_add(encoder, &dw_hdmi_rockchip_encoder_helper_funcs); > + drm_encoder_init(drm, encoder, &dw_hdmi_rockchip_encoder_funcs, > + DRM_MODE_ENCODER_TMDS); > + > + return dw_hdmi_bind(dev, master, data, encoder, iores, irq, plat_data); > +} > + > +static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master, > + void *data) > +{ > + return dw_hdmi_unbind(dev, master, data); > +} > + > +static const struct component_ops dw_hdmi_rockchip_ops = { > + .bind = dw_hdmi_rockchip_bind, > + .unbind = dw_hdmi_rockchip_unbind, > +}; > + > +static int dw_hdmi_rockchip_probe(struct platform_device *pdev) > +{ > + return component_add(&pdev->dev, &dw_hdmi_rockchip_ops); > +} > + > +static int dw_hdmi_rockchip_remove(struct platform_device *pdev) > +{ > + component_del(&pdev->dev, &dw_hdmi_rockchip_ops); > + > + return 0; > +} > + > +static struct platform_driver dw_hdmi_rockchip_pltfm_driver = { > + .probe = dw_hdmi_rockchip_probe, > + .remove = dw_hdmi_rockchip_remove, > + .driver = { > + .name = "dwhdmi-rockchip", "rockchip-hdmi" > + .of_match_table = dw_hdmi_rockchip_ids, > + }, > +}; > + > +module_platform_driver(dw_hdmi_rockchip_pltfm_driver); > + > +MODULE_AUTHOR("Andy Yan <andy.yan@xxxxxxxxxxxxxx>"); > +MODULE_AUTHOR("Yakir Yang <ykk@xxxxxxxxxxxxxx>"); > +MODULE_DESCRIPTION("Rockchip Specific DW-HDMI Driver Extension"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:dwhdmi-rockchip"); Why do we need this alias? > diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h > index b64e58a..5a4f490 100644 > --- a/include/drm/bridge/dw_hdmi.h > +++ b/include/drm/bridge/dw_hdmi.h > @@ -22,6 +22,7 @@ enum { > enum dw_hdmi_devtype { > IMX6Q_HDMI, > IMX6DL_HDMI, > + RK3288_HDMI, > }; > > struct dw_hdmi_mpll_config { > -- > 1.9.1 > > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
