On Fri 10 Jan 2025 at 13:39, Ao Xu via B4 Relay
<devnull+ao.xu.amlogic....@kernel.org> wrote:
> From: Ao Xu <ao...@amlogic.com>
>
> Add S4 compatible for DRM driver. This update driver logic to support
> S4-specific configurations. This also add vpu clock operation in
> bind, suspend, resume, shutdown stage.
>
> Signed-off-by: Ao Xu <ao...@amlogic.com>
> ---
> drivers/gpu/drm/meson/meson_drv.c | 127
> +++++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/meson/meson_drv.h | 6 ++
> 2 files changed, 132 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_drv.c
> b/drivers/gpu/drm/meson/meson_drv.c
> index
> 81d2ee37e7732dca89d02347b9c972300b38771a..d28094efeb137ae0b9990ab3608825d563358dba
> 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -11,6 +11,7 @@
> #include <linux/aperture.h>
> #include <linux/component.h>
> #include <linux/module.h>
> +#include <linux/clk.h>
> #include <linux/of_graph.h>
> #include <linux/sys_soc.h>
> #include <linux/platform_device.h>
> @@ -160,6 +161,34 @@ static void meson_vpu_init(struct meson_drm *priv)
> writel_relaxed(value, priv->io_base + _REG(VPU_WRARB_MODE_L2C1));
> }
>
> +static void meson_setup_clk(struct meson_drm *priv, bool enable)
> +{
> + int ret;
> +
> + if (!priv || !priv->vpu_clk || !priv->vapb_clk)
> + return;
> +
> + if (!meson_vpu_is_compatible(priv, VPU_COMPATIBLE_S4))
> + return;
> +
> + if (enable) {
> + ret = clk_prepare_enable(priv->vpu_clk);
> + if (ret) {
> + dev_err(priv->dev, "Failed to set vpu clk\n");
> + return;
> + }
> + ret = clk_prepare_enable(priv->vapb_clk);
> + if (ret) {
> + dev_err(priv->dev, "Failed to Set vapb clk\n");
> + clk_disable_unprepare(priv->vpu_clk);
> + return;
> + }
> + } else {
> + clk_disable_unprepare(priv->vpu_clk);
> + clk_disable_unprepare(priv->vapb_clk);
> + }
> +}
> +
> struct meson_drm_soc_attr {
> struct meson_drm_soc_limits limits;
> const struct soc_device_attribute *attrs;
> @@ -241,6 +270,83 @@ static int meson_drv_bind_master(struct device *dev,
> bool has_components)
> goto free_drm;
> }
>
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_S4)) {
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "sysctrl");
> + if (!res) {
> + ret = -EINVAL;
> + goto free_drm;
> + }
> + /* Simply ioremap since it may be a shared register zone */
This comment, the name of the zone and even the usage you are making of
it clearly show this is repeating the error of past, directly accessing
improperly shared registers which should otherwise have been implemented
as proper controller using the kernel available framework, such PD, phys,
clock, reset, etc ...
Worse, it gets encoded into the dt binding, making extremely difficult
to fix later on.
> + regs = devm_ioremap(dev, res->start, resource_size(res));
> + if (!regs) {
> + ret = -EADDRNOTAVAIL;
> + goto free_drm;
> + }
> +
> + priv->sysctrl = devm_regmap_init_mmio(dev, regs,
> + &meson_regmap_config);
> + if (IS_ERR(priv->sysctrl)) {
> + dev_err(&pdev->dev, "Couldn't create the SYSCTRL
> regmap\n");
> + ret = PTR_ERR(priv->sysctrl);
> + goto free_drm;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "clkctrl");
> + if (!res) {
> + ret = -EINVAL;
> + goto free_drm;
> + }
> + /* Simply ioremap since it may be a shared register zone */
> + regs = devm_ioremap(dev, res->start, resource_size(res));
> + if (!regs) {
> + ret = -EADDRNOTAVAIL;
> + goto free_drm;
> + }
> +
> + priv->clkctrl = devm_regmap_init_mmio(dev, regs,
> + &meson_regmap_config);
> + if (IS_ERR(priv->clkctrl)) {
> + dev_err(&pdev->dev, "Couldn't create the clkctrl
> regmap\n");
> + ret = PTR_ERR(priv->clkctrl);
> + goto free_drm;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "pwrctrl");
> + if (!res) {
> + ret = -EINVAL;
> + goto free_drm;
> + }
> + /* Simply ioremap since it may be a shared register zone */
> + regs = devm_ioremap(dev, res->start, resource_size(res));
> + if (!regs) {
> + ret = -EADDRNOTAVAIL;
> + goto free_drm;
> + }
> +
> + priv->pwrctrl = devm_regmap_init_mmio(dev, regs,
> + &meson_regmap_config);
> + if (IS_ERR(priv->pwrctrl)) {
> + dev_err(&pdev->dev, "Couldn't create the pwrctrl
> regmap\n");
> + ret = PTR_ERR(priv->pwrctrl);
> + goto free_drm;
> + }
> +
> + priv->vpu_clk = devm_clk_get(&pdev->dev, "vpu");
> + if (IS_ERR(priv->vpu_clk)) {
> + dev_err(&pdev->dev, "vpu clock request failed\n");
> + ret = PTR_ERR(priv->vpu_clk);
> + goto free_drm;
> + }
> +
> + priv->vapb_clk = devm_clk_get(&pdev->dev, "vapb");
> + if (IS_ERR(priv->vapb_clk)) {
> + dev_err(&pdev->dev, "vapb clock request failed\n");
> + ret = PTR_ERR(priv->vapb_clk);
> + goto free_drm;
> + }
> + meson_setup_clk(priv, true);
> + }
> +
> priv->canvas = meson_canvas_get(dev);
> if (IS_ERR(priv->canvas)) {
> ret = PTR_ERR(priv->canvas);
> @@ -424,12 +530,21 @@ static const struct component_master_ops
> meson_drv_master_ops = {
>
> static int __maybe_unused meson_drv_pm_suspend(struct device *dev)
> {
> + int ret;
> struct meson_drm *priv = dev_get_drvdata(dev);
>
> if (!priv)
> return 0;
>
> - return drm_mode_config_helper_suspend(priv->drm);
> + ret = drm_mode_config_helper_suspend(priv->drm);
> + if (unlikely(ret)) {
> + drm_err(dev, "suspend error: %d", ret);
> + return ret;
> + }
> +
> + meson_setup_clk(priv, false);
> +
> + return ret;
> }
>
> static int __maybe_unused meson_drv_pm_resume(struct device *dev)
> @@ -439,6 +554,7 @@ static int __maybe_unused meson_drv_pm_resume(struct
> device *dev)
> if (!priv)
> return 0;
>
> + meson_setup_clk(priv, true);
> meson_vpu_init(priv);
> meson_venc_init(priv);
> meson_vpp_init(priv);
> @@ -458,6 +574,7 @@ static void meson_drv_shutdown(struct platform_device
> *pdev)
>
> drm_kms_helper_poll_fini(priv->drm);
> drm_atomic_helper_shutdown(priv->drm);
> + meson_setup_clk(priv, false);
> }
>
> /*
> @@ -471,6 +588,7 @@ static const struct of_device_id components_dev_match[] =
> {
> { .compatible = "amlogic,meson-gxl-dw-hdmi" },
> { .compatible = "amlogic,meson-gxm-dw-hdmi" },
> { .compatible = "amlogic,meson-g12a-dw-hdmi" },
> + { .compatible = "amlogic,meson-s4-dw-hdmi" },
> {}
> };
>
> @@ -539,6 +657,11 @@ static struct meson_drm_match_data meson_drm_g12a_data =
> {
> .afbcd_ops = &meson_afbcd_g12a_ops,
> };
>
> +static struct meson_drm_match_data meson_drm_s4_data = {
> + .compat = VPU_COMPATIBLE_S4,
> + .afbcd_ops = &meson_afbcd_g12a_ops,
> +};
> +
> static const struct of_device_id dt_match[] = {
> { .compatible = "amlogic,meson-gxbb-vpu",
> .data = (void *)&meson_drm_gxbb_data },
> @@ -548,6 +671,8 @@ static const struct of_device_id dt_match[] = {
> .data = (void *)&meson_drm_gxm_data },
> { .compatible = "amlogic,meson-g12a-vpu",
> .data = (void *)&meson_drm_g12a_data },
> + { .compatible = "amlogic,meson-s4-vpu",
> + .data = (void *)&meson_drm_s4_data },
> {}
> };
> MODULE_DEVICE_TABLE(of, dt_match);
> diff --git a/drivers/gpu/drm/meson/meson_drv.h
> b/drivers/gpu/drm/meson/meson_drv.h
> index
> 3f9345c14f31c13b071f420533fe8a450d3e0f36..c801a2e3e55a054247710aebae5602e44c9e1624
> 100644
> --- a/drivers/gpu/drm/meson/meson_drv.h
> +++ b/drivers/gpu/drm/meson/meson_drv.h
> @@ -22,6 +22,7 @@ enum vpu_compatible {
> VPU_COMPATIBLE_GXL = 1,
> VPU_COMPATIBLE_GXM = 2,
> VPU_COMPATIBLE_G12A = 3,
> + VPU_COMPATIBLE_S4 = 4,
> };
>
> enum {
> @@ -45,6 +46,11 @@ struct meson_drm {
> enum vpu_compatible compat;
> void __iomem *io_base;
> struct regmap *hhi;
> + struct regmap *sysctrl;
> + struct regmap *clkctrl;
> + struct regmap *pwrctrl;
> + struct clk *vpu_clk;
> + struct clk *vapb_clk;
> int vsync_irq;
>
> struct meson_canvas *canvas;
--
Jerome
