| Message ID | e6e1f3f44e6979a998ec9c372e329b6facaded15.1644681054.git.hns@goldelico.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: JZ4780 and CI20 HDMI | expand |
Hi, Le sam., févr. 12 2022 at 16:50:50 +0100, H. Nikolaus Schaller <hns@goldelico.com> a écrit : > From: Paul Boddie <paul@boddie.org.uk> > > A specialisation of the generic Synopsys HDMI driver is employed for > JZ4780 HDMI support. This requires a new driver, plus device tree and > configuration modifications. > > Here we add Kconfig DRM_INGENIC_DW_HDMI, Makefile and driver code. > > Signed-off-by: Paul Boddie <paul@boddie.org.uk> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> > --- > drivers/gpu/drm/ingenic/Kconfig | 9 ++ > drivers/gpu/drm/ingenic/Makefile | 1 + > drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 104 > ++++++++++++++++++++++ > 3 files changed, 114 insertions(+) > create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c > > diff --git a/drivers/gpu/drm/ingenic/Kconfig > b/drivers/gpu/drm/ingenic/Kconfig > index 001f59fb06d56..090830bcbde7f 100644 > --- a/drivers/gpu/drm/ingenic/Kconfig > +++ b/drivers/gpu/drm/ingenic/Kconfig > @@ -24,4 +24,13 @@ config DRM_INGENIC_IPU > > The Image Processing Unit (IPU) will appear as a second primary > plane. > > +config DRM_INGENIC_DW_HDMI > + tristate "Ingenic specific support for Synopsys DW HDMI" > + depends on MACH_JZ4780 > + select DRM_DW_HDMI > + help > + Choose this option to enable Synopsys DesignWare HDMI based > driver. > + If you want to enable HDMI on Ingenic JZ4780 based SoC, you should > + select this option. > + > endif > diff --git a/drivers/gpu/drm/ingenic/Makefile > b/drivers/gpu/drm/ingenic/Makefile > index d313326bdddbb..f10cc1c5a5f22 100644 > --- a/drivers/gpu/drm/ingenic/Makefile > +++ b/drivers/gpu/drm/ingenic/Makefile > @@ -1,3 +1,4 @@ > obj-$(CONFIG_DRM_INGENIC) += ingenic-drm.o > ingenic-drm-y = ingenic-drm-drv.o > ingenic-drm-$(CONFIG_DRM_INGENIC_IPU) += ingenic-ipu.o > +obj-$(CONFIG_DRM_INGENIC_DW_HDMI) += ingenic-dw-hdmi.o > diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c > b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c > new file mode 100644 > index 0000000000000..34e986dd606cf > --- /dev/null > +++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c > @@ -0,0 +1,104 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (C) 2011-2013 Freescale Semiconductor, Inc. > + * Copyright (C) 2019, 2020 Paul Boddie <paul@boddie.org.uk> > + * > + * Derived from dw_hdmi-imx.c with i.MX portions removed. > + * Probe and remove operations derived from rcar_dw_hdmi.c. > + */ > + > +#include <linux/module.h> > +#include <linux/of_platform.h> > +#include <linux/platform_device.h> > + > +#include <drm/bridge/dw_hdmi.h> > +#include <drm/drm_of.h> > +#include <drm/drm_print.h> > + > +static const struct dw_hdmi_mpll_config ingenic_mpll_cfg[] = { > + { 45250000, { { 0x01e0, 0x0000 }, { 0x21e1, 0x0000 }, { 0x41e2, > 0x0000 } } }, > + { 92500000, { { 0x0140, 0x0005 }, { 0x2141, 0x0005 }, { 0x4142, > 0x0005 } } }, > + { 148500000, { { 0x00a0, 0x000a }, { 0x20a1, 0x000a }, { 0x40a2, > 0x000a } } }, > + { 216000000, { { 0x00a0, 0x000a }, { 0x2001, 0x000f }, { 0x4002, > 0x000f } } }, > + { ~0UL, { { 0x0000, 0x0000 }, { 0x0000, 0x0000 }, { 0x0000, > 0x0000 } } } > +}; > + > +static const struct dw_hdmi_curr_ctrl ingenic_cur_ctr[] = { > + /*pixelclk bpp8 bpp10 bpp12 */ > + { 54000000, { 0x091c, 0x091c, 0x06dc } }, > + { 58400000, { 0x091c, 0x06dc, 0x06dc } }, > + { 72000000, { 0x06dc, 0x06dc, 0x091c } }, > + { 74250000, { 0x06dc, 0x0b5c, 0x091c } }, > + { 118800000, { 0x091c, 0x091c, 0x06dc } }, > + { 216000000, { 0x06dc, 0x0b5c, 0x091c } }, > + { ~0UL, { 0x0000, 0x0000, 0x0000 } }, > +}; > + > +/* > + * Resistance term 133Ohm Cfg > + * PREEMP config 0.00 > + * TX/CK level 10 > + */ > +static const struct dw_hdmi_phy_config ingenic_phy_config[] = { > + /*pixelclk symbol term vlev */ > + { 216000000, 0x800d, 0x0005, 0x01ad}, > + { ~0UL, 0x0000, 0x0000, 0x0000} > +}; > + > +static enum drm_mode_status > +ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data, > + const struct drm_display_info *info, > + const struct drm_display_mode *mode) > +{ > + if (mode->clock < 13500) > + return MODE_CLOCK_LOW; > + /* FIXME: Hardware is capable of 270MHz, but setup data is missing. > */ > + if (mode->clock > 216000) > + return MODE_CLOCK_HIGH; > + > + return MODE_OK; > +} > + > +static struct dw_hdmi_plat_data ingenic_dw_hdmi_plat_data = { > + .mpll_cfg = ingenic_mpll_cfg, > + .cur_ctr = ingenic_cur_ctr, > + .phy_config = ingenic_phy_config, > + .mode_valid = ingenic_dw_hdmi_mode_valid, > + .output_port = 1, > +}; > + > +static const struct of_device_id ingenic_dw_hdmi_dt_ids[] = { > + { .compatible = "ingenic,jz4780-dw-hdmi" }, > + { /* Sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, ingenic_dw_hdmi_dt_ids); > + > +static void ingenic_dw_hdmi_cleanup(void *data) > +{ > + struct dw_hdmi *hdmi = (struct dw_hdmi *)data; > + > + dw_hdmi_remove(hdmi); > +} > + > +static int ingenic_dw_hdmi_probe(struct platform_device *pdev) > +{ > + struct dw_hdmi *hdmi; > + > + hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data); > + if (IS_ERR(hdmi)) > + return PTR_ERR(hdmi); > + > + return devm_add_action_or_reset(&pdev->dev, > ingenic_dw_hdmi_cleanup, hdmi); I think I said it already, but in this driver you could use a .remove callback, there's not much point in using devm cleanups in such a simple setup. In your probe you could just: return PTR_ERR_OR_ZERO(hdmi); > +} > + > +static struct platform_driver ingenic_dw_hdmi_driver = { > + .probe = ingenic_dw_hdmi_probe, > + .driver = { > + .name = "dw-hdmi-ingenic", > + .of_match_table = ingenic_dw_hdmi_dt_ids, > + }, > +}; > +module_platform_driver(ingenic_dw_hdmi_driver); > + > +MODULE_DESCRIPTION("JZ4780 Specific DW-HDMI Driver Extension"); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform:dwhdmi-ingenic"); Should probably be "platform:dw-hdmi-ingenic"? Cheers, -Paul >
Hi Paul, > Am 14.02.2022 um 11:24 schrieb Paul Cercueil <paul@crapouillou.net>: > > Hi, > > Le sam., févr. 12 2022 at 16:50:50 +0100, H. Nikolaus Schaller <hns@goldelico.com> a écrit : >> +static void ingenic_dw_hdmi_cleanup(void *data) >> +{ >> + struct dw_hdmi *hdmi = (struct dw_hdmi *)data; >> + >> + dw_hdmi_remove(hdmi); >> +} >> + >> +static int ingenic_dw_hdmi_probe(struct platform_device *pdev) >> +{ >> + struct dw_hdmi *hdmi; >> + >> + hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data); >> + if (IS_ERR(hdmi)) >> + return PTR_ERR(hdmi); >> + >> + return devm_add_action_or_reset(&pdev->dev, ingenic_dw_hdmi_cleanup, hdmi); > > I think I said it already, but in this driver you could use a .remove callback, there's not much point in using devm cleanups in such a simple setup. Well it was your suggestion after v8: https://lore.kernel.org/all/DIA33R.QE29K7RKLI2C1@crapouillou.net/ So we now almost go back to RFC v1 almost 2 years ago: https://patchwork.kernel.org/project/linux-mips/patch/2c131e1fb19e19f958a612f7186bc83f4afb0b0a.1582744379.git.hns@goldelico.com/ Of course there was a good reason to better handle the regulator AND the dw_hdmi_remove() by a single mechanism. Now the regulator has gone and been replaced by the hdmi connector and we can go back. > > In your probe you could just: > return PTR_ERR_OR_ZERO(hdmi); No, this does not work since we need to platform_set_drvdata(). to be able to access the private struct in the remove callback. And checking errors after platform_set_drvdata() can be done but looks strange to me. It is up to you what you prefer. > >> +} >> + >> +static struct platform_driver ingenic_dw_hdmi_driver = { >> + .probe = ingenic_dw_hdmi_probe, >> + .driver = { >> + .name = "dw-hdmi-ingenic", >> + .of_match_table = ingenic_dw_hdmi_dt_ids, >> + }, >> +}; >> +module_platform_driver(ingenic_dw_hdmi_driver); >> + >> +MODULE_DESCRIPTION("JZ4780 Specific DW-HDMI Driver Extension"); >> +MODULE_LICENSE("GPL v2"); >> +MODULE_ALIAS("platform:dwhdmi-ingenic"); > > Should probably be "platform:dw-hdmi-ingenic"? Yes, indeed. Thanks for spotting! Was also good in v1. Probably someone deleted the hyphen unnoticed during editing of "jz4780" to "ingenic"... BR and thanks, Nikolaus
Le lun., févr. 14 2022 at 12:02:53 +0100, H. Nikolaus Schaller <hns@goldelico.com> a écrit : > Hi Paul, > >> Am 14.02.2022 um 11:24 schrieb Paul Cercueil <paul@crapouillou.net>: >> >> Hi, >> >> Le sam., févr. 12 2022 at 16:50:50 +0100, H. Nikolaus Schaller >> <hns@goldelico.com> a écrit : > >>> +static void ingenic_dw_hdmi_cleanup(void *data) >>> +{ >>> + struct dw_hdmi *hdmi = (struct dw_hdmi *)data; >>> + >>> + dw_hdmi_remove(hdmi); >>> +} >>> + >>> +static int ingenic_dw_hdmi_probe(struct platform_device *pdev) >>> +{ >>> + struct dw_hdmi *hdmi; >>> + >>> + hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data); >>> + if (IS_ERR(hdmi)) >>> + return PTR_ERR(hdmi); >>> + >>> + return devm_add_action_or_reset(&pdev->dev, >>> ingenic_dw_hdmi_cleanup, hdmi); >> >> I think I said it already, but in this driver you could use a >> .remove callback, there's not much point in using devm cleanups in >> such a simple setup. > > Well it was your suggestion after v8: > > https://lore.kernel.org/all/DIA33R.QE29K7RKLI2C1@crapouillou.net/ It made sense for your v8, not so much for your v15... > So we now almost go back to RFC v1 almost 2 years ago: > > https://patchwork.kernel.org/project/linux-mips/patch/2c131e1fb19e19f958a612f7186bc83f4afb0b0a.1582744379.git.hns@goldelico.com/ > > Of course there was a good reason to better handle the regulator > AND the dw_hdmi_remove() by a single mechanism. > > Now the regulator has gone and been replaced by the hdmi connector > and we can go back. > >> >> In your probe you could just: >> return PTR_ERR_OR_ZERO(hdmi); > > No, this does not work since we need to platform_set_drvdata(). > to be able to access the private struct in the remove callback. > And checking errors after platform_set_drvdata() can be done but > looks strange to me. Yeah, I guess it would look strange. Fine then. Then I guess just merge your current [6/7] with this one (and make sure it comes after your current [5/7]) and it looks mergeable to me. Cheers, -Paul > It is up to you what you prefer. > >> >>> +} >>> + >>> +static struct platform_driver ingenic_dw_hdmi_driver = { >>> + .probe = ingenic_dw_hdmi_probe, >>> + .driver = { >>> + .name = "dw-hdmi-ingenic", >>> + .of_match_table = ingenic_dw_hdmi_dt_ids, >>> + }, >>> +}; >>> +module_platform_driver(ingenic_dw_hdmi_driver); >>> + >>> +MODULE_DESCRIPTION("JZ4780 Specific DW-HDMI Driver Extension"); >>> +MODULE_LICENSE("GPL v2"); >>> +MODULE_ALIAS("platform:dwhdmi-ingenic"); >> >> Should probably be "platform:dw-hdmi-ingenic"? > > Yes, indeed. Thanks for spotting! > > Was also good in v1. Probably someone deleted the hyphen unnoticed > during editing of "jz4780" to "ingenic"...
diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig index 001f59fb06d56..090830bcbde7f 100644 --- a/drivers/gpu/drm/ingenic/Kconfig +++ b/drivers/gpu/drm/ingenic/Kconfig @@ -24,4 +24,13 @@ config DRM_INGENIC_IPU The Image Processing Unit (IPU) will appear as a second primary plane. +config DRM_INGENIC_DW_HDMI + tristate "Ingenic specific support for Synopsys DW HDMI" + depends on MACH_JZ4780 + select DRM_DW_HDMI + help + Choose this option to enable Synopsys DesignWare HDMI based driver. + If you want to enable HDMI on Ingenic JZ4780 based SoC, you should + select this option. + endif diff --git a/drivers/gpu/drm/ingenic/Makefile b/drivers/gpu/drm/ingenic/Makefile index d313326bdddbb..f10cc1c5a5f22 100644 --- a/drivers/gpu/drm/ingenic/Makefile +++ b/drivers/gpu/drm/ingenic/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_DRM_INGENIC) += ingenic-drm.o ingenic-drm-y = ingenic-drm-drv.o ingenic-drm-$(CONFIG_DRM_INGENIC_IPU) += ingenic-ipu.o +obj-$(CONFIG_DRM_INGENIC_DW_HDMI) += ingenic-dw-hdmi.o diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c new file mode 100644 index 0000000000000..34e986dd606cf --- /dev/null +++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2011-2013 Freescale Semiconductor, Inc. + * Copyright (C) 2019, 2020 Paul Boddie <paul@boddie.org.uk> + * + * Derived from dw_hdmi-imx.c with i.MX portions removed. + * Probe and remove operations derived from rcar_dw_hdmi.c. + */ + +#include <linux/module.h> +#include <linux/of_platform.h> +#include <linux/platform_device.h> + +#include <drm/bridge/dw_hdmi.h> +#include <drm/drm_of.h> +#include <drm/drm_print.h> + +static const struct dw_hdmi_mpll_config ingenic_mpll_cfg[] = { + { 45250000, { { 0x01e0, 0x0000 }, { 0x21e1, 0x0000 }, { 0x41e2, 0x0000 } } }, + { 92500000, { { 0x0140, 0x0005 }, { 0x2141, 0x0005 }, { 0x4142, 0x0005 } } }, + { 148500000, { { 0x00a0, 0x000a }, { 0x20a1, 0x000a }, { 0x40a2, 0x000a } } }, + { 216000000, { { 0x00a0, 0x000a }, { 0x2001, 0x000f }, { 0x4002, 0x000f } } }, + { ~0UL, { { 0x0000, 0x0000 }, { 0x0000, 0x0000 }, { 0x0000, 0x0000 } } } +}; + +static const struct dw_hdmi_curr_ctrl ingenic_cur_ctr[] = { + /*pixelclk bpp8 bpp10 bpp12 */ + { 54000000, { 0x091c, 0x091c, 0x06dc } }, + { 58400000, { 0x091c, 0x06dc, 0x06dc } }, + { 72000000, { 0x06dc, 0x06dc, 0x091c } }, + { 74250000, { 0x06dc, 0x0b5c, 0x091c } }, + { 118800000, { 0x091c, 0x091c, 0x06dc } }, + { 216000000, { 0x06dc, 0x0b5c, 0x091c } }, + { ~0UL, { 0x0000, 0x0000, 0x0000 } }, +}; + +/* + * Resistance term 133Ohm Cfg + * PREEMP config 0.00 + * TX/CK level 10 + */ +static const struct dw_hdmi_phy_config ingenic_phy_config[] = { + /*pixelclk symbol term vlev */ + { 216000000, 0x800d, 0x0005, 0x01ad}, + { ~0UL, 0x0000, 0x0000, 0x0000} +}; + +static enum drm_mode_status +ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data, + const struct drm_display_info *info, + const struct drm_display_mode *mode) +{ + if (mode->clock < 13500) + return MODE_CLOCK_LOW; + /* FIXME: Hardware is capable of 270MHz, but setup data is missing. */ + if (mode->clock > 216000) + return MODE_CLOCK_HIGH; + + return MODE_OK; +} + +static struct dw_hdmi_plat_data ingenic_dw_hdmi_plat_data = { + .mpll_cfg = ingenic_mpll_cfg, + .cur_ctr = ingenic_cur_ctr, + .phy_config = ingenic_phy_config, + .mode_valid = ingenic_dw_hdmi_mode_valid, + .output_port = 1, +}; + +static const struct of_device_id ingenic_dw_hdmi_dt_ids[] = { + { .compatible = "ingenic,jz4780-dw-hdmi" }, + { /* Sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, ingenic_dw_hdmi_dt_ids); + +static void ingenic_dw_hdmi_cleanup(void *data) +{ + struct dw_hdmi *hdmi = (struct dw_hdmi *)data; + + dw_hdmi_remove(hdmi); +} + +static int ingenic_dw_hdmi_probe(struct platform_device *pdev) +{ + struct dw_hdmi *hdmi; + + hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data); + if (IS_ERR(hdmi)) + return PTR_ERR(hdmi); + + return devm_add_action_or_reset(&pdev->dev, ingenic_dw_hdmi_cleanup, hdmi); +} + +static struct platform_driver ingenic_dw_hdmi_driver = { + .probe = ingenic_dw_hdmi_probe, + .driver = { + .name = "dw-hdmi-ingenic", + .of_match_table = ingenic_dw_hdmi_dt_ids, + }, +}; +module_platform_driver(ingenic_dw_hdmi_driver); + +MODULE_DESCRIPTION("JZ4780 Specific DW-HDMI Driver Extension"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:dwhdmi-ingenic");