[PATCH v2 3/4] drm/msm: split the main platform driver
Stephen Boyd
swboyd at chromium.org
Thu Mar 3 23:00:29 UTC 2022
Quoting Dmitry Baryshkov (2022-01-19 14:40:04)
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 06d26c5fb274..6895c056be19 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -451,10 +451,18 @@ static inline void msm_dp_debugfs_init(struct msm_dp *dp_display,
>
> #endif
>
> +#define KMS_MDP4 4
> +#define KMS_MDP5 5
> +#define KMS_DPU 3
> +
> +void __init msm_mdp4_register(void);
> +void __exit msm_mdp4_unregister(void);
> void __init msm_mdp_register(void);
> void __exit msm_mdp_unregister(void);
> void __init msm_dpu_register(void);
> void __exit msm_dpu_unregister(void);
> +void __init msm_mdss_register(void);
> +void __exit msm_mdss_unregister(void);
Don't need __init or __exit on prototypes.
>
> #ifdef CONFIG_DEBUG_FS
> void msm_framebuffer_describe(struct drm_framebuffer *fb, struct seq_file *m);
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index 92562221b517..759076357e0e 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -8,6 +8,8 @@
> #include <linux/irqdesc.h>
> #include <linux/irqchip/chained_irq.h>
>
> +#include <drm/drm_of.h>
What's this include for?
> +
> #include "msm_drv.h"
> #include "msm_kms.h"
>
> @@ -127,7 +129,7 @@ static int _msm_mdss_irq_domain_add(struct msm_mdss *msm_mdss)
> return 0;
> }
>
> -int msm_mdss_enable(struct msm_mdss *msm_mdss)
> +static int msm_mdss_enable(struct msm_mdss *msm_mdss)
> {
> int ret;
>
> @@ -163,14 +165,14 @@ int msm_mdss_enable(struct msm_mdss *msm_mdss)
> return ret;
> }
>
> -int msm_mdss_disable(struct msm_mdss *msm_mdss)
> +static int msm_mdss_disable(struct msm_mdss *msm_mdss)
> {
> clk_bulk_disable_unprepare(msm_mdss->num_clocks, msm_mdss->clocks);
>
> return 0;
> }
>
> -void msm_mdss_destroy(struct msm_mdss *msm_mdss)
> +static void msm_mdss_destroy(struct msm_mdss *msm_mdss)
> {
> struct platform_device *pdev = to_platform_device(msm_mdss->dev);
> int irq;
> @@ -228,7 +230,7 @@ int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_data **c
> return num_clocks;
> }
>
> -struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool mdp5)
> +static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool mdp5)
> {
> struct msm_mdss *msm_mdss;
> int ret;
> @@ -269,3 +271,171 @@ struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool mdp5)
>
> return msm_mdss;
> }
> +
> +static int __maybe_unused mdss_runtime_suspend(struct device *dev)
> +{
> + struct msm_drm_private *priv = dev_get_drvdata(dev);
> +
> + DBG("");
> +
> + return msm_mdss_disable(priv->mdss);
> +}
> +
> +static int __maybe_unused mdss_runtime_resume(struct device *dev)
> +{
> + struct msm_drm_private *priv = dev_get_drvdata(dev);
> +
> + DBG("");
> +
> + return msm_mdss_enable(priv->mdss);
> +}
> +
> +static int __maybe_unused mdss_pm_suspend(struct device *dev)
> +{
> +
> + if (pm_runtime_suspended(dev))
> + return 0;
> +
> + return mdss_runtime_suspend(dev);
> +}
> +
> +static int __maybe_unused mdss_pm_resume(struct device *dev)
> +{
> + if (pm_runtime_suspended(dev))
> + return 0;
> +
> + return mdss_runtime_resume(dev);
> +}
> +
> +static const struct dev_pm_ops mdss_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(mdss_pm_suspend, mdss_pm_resume)
> + SET_RUNTIME_PM_OPS(mdss_runtime_suspend, mdss_runtime_resume, NULL)
> + .prepare = msm_pm_prepare,
> + .complete = msm_pm_complete,
> +};
> +
> +static int get_mdp_ver(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> +
> + return (int) (unsigned long) of_device_get_match_data(dev);
> +}
> +
> +static int find_mdp_node(struct device *dev, void *data)
> +{
> + return of_match_node(dpu_dt_match, dev->of_node) ||
> + of_match_node(mdp5_dt_match, dev->of_node);
> +}
> +
> +static int mdss_probe(struct platform_device *pdev)
> +{
> + struct msm_mdss *mdss;
> + struct msm_drm_private *priv;
> + int mdp_ver = get_mdp_ver(pdev);
> + struct device *mdp_dev;
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + if (mdp_ver != KMS_MDP5 && mdp_ver != KMS_DPU)
> + return -EINVAL;
Is it possible anymore? Now that the driver is split it seems like no.
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, priv);
> +
> + mdss = msm_mdss_init(pdev, mdp_ver == KMS_MDP5);
> + if (IS_ERR(mdss)) {
> + ret = PTR_ERR(mdss);
> + platform_set_drvdata(pdev, NULL);
> +
> + return ret;
> + }
> +
> + priv->mdss = mdss;
> + pm_runtime_enable(&pdev->dev);
> +
> + /*
> + * MDP5/DPU based devices don't have a flat hierarchy. There is a top
> + * level parent: MDSS, and children: MDP5/DPU, DSI, HDMI, eDP etc.
> + * Populate the children devices, find the MDP5/DPU node, and then add
> + * the interfaces to our components list.
> + */
> + ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "failed to populate children devices\n");
> + goto fail;
> + }
> +
> + mdp_dev = device_find_child(dev, NULL, find_mdp_node);
> + if (!mdp_dev) {
> + DRM_DEV_ERROR(dev, "failed to find MDSS MDP node\n");
> + of_platform_depopulate(dev);
> + ret = -ENODEV;
> + goto fail;
> + }
> +
> + /*
> + * on MDP5 based platforms, the MDSS platform device is the component
> + * master that adds MDP5 and other display interface components to
s/master//
> + * itself.
> + */
> + ret = msm_drv_probe(dev, mdp_dev);
> + put_device(mdp_dev);
> + if (ret)
> + goto fail;
> +
> + return 0;
> +
> +fail:
> + of_platform_depopulate(dev);
> + msm_mdss_destroy(priv->mdss);
> +
> + return ret;
> +}
> +
> +static int mdss_remove(struct platform_device *pdev)
> +{
> + struct msm_drm_private *priv = platform_get_drvdata(pdev);
> + struct msm_mdss *mdss = priv->mdss;
> +
> + component_master_del(&pdev->dev, &msm_drm_ops);
> + of_platform_depopulate(&pdev->dev);
> +
> + msm_mdss_destroy(mdss);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mdss_dt_match[] = {
> + { .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 },
> + { .compatible = "qcom,sdm845-mdss", .data = (void *)KMS_DPU },
> + { .compatible = "qcom,sc7180-mdss", .data = (void *)KMS_DPU },
> + { .compatible = "qcom,sc7280-mdss", .data = (void *)KMS_DPU },
> + { .compatible = "qcom,sm8150-mdss", .data = (void *)KMS_DPU },
> + { .compatible = "qcom,sm8250-mdss", .data = (void *)KMS_DPU },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, dt_match);
> +
> +static struct platform_driver mdss_platform_driver = {
> + .probe = mdss_probe,
> + .remove = mdss_remove,
> + .shutdown = msm_drv_shutdown,
> + .driver = {
> + .name = "msm-mdss",
> + .of_match_table = mdss_dt_match,
> + .pm = &mdss_pm_ops,
> + },
> +};
> +
> +void __init msm_mdss_register(void)
> +{
> + platform_driver_register(&mdss_platform_driver);
I'd like to go a step further and not even compile drivers in that we
don't use. Can we get some Kconfigs for these new drivers so that the
number of drivers registered with the system is reduced and memory is
conserved?
More information about the dri-devel
mailing list