All of lore.kernel.org
 help / color / mirror / Atom feed
From: Abhinav Kumar <quic_abhinavk@quicinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>
Cc: Stephen Boyd <swboyd@chromium.org>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	<linux-arm-msm@vger.kernel.org>,
	<dri-devel@lists.freedesktop.org>,
	<freedreno@lists.freedesktop.org>
Subject: Re: [PATCH v3 3/5] drm/msm: split the main platform driver
Date: Fri, 4 Mar 2022 18:10:07 -0800	[thread overview]
Message-ID: <f9f5df47-0eae-abd0-67f5-2fe73594b7af@quicinc.com> (raw)
In-Reply-To: <20220304032106.2866043-4-dmitry.baryshkov@linaro.org>



On 3/3/2022 7:21 PM, Dmitry Baryshkov wrote:
> Currently the msm platform driver is a multiplex handling several cases:
> - headless GPU-only driver,
> - MDP4 with flat device nodes,
> - MDP5/DPU MDSS with all the nodes being children of MDSS node.
> 
> This results in not-so-perfect code, checking the hardware version
> (MDP4/MDP5/DPU) in several places, checking for mdss even when it can
> not exist, etc. Split the code into three handling subdrivers (mdp4,
> mdss and headless msm).
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

If this has been verified across MDP4, MDP5 and DPU,

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

> ---
>   drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c |  56 ++++++
>   drivers/gpu/drm/msm/msm_drv.c            | 228 ++++-------------------
>   drivers/gpu/drm/msm/msm_drv.h            |  27 ++-
>   drivers/gpu/drm/msm/msm_kms.h            |   7 -
>   drivers/gpu/drm/msm/msm_mdss.c           | 178 +++++++++++++++++-
>   5 files changed, 291 insertions(+), 205 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> index 3cf476c55158..c5c0650414c5 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> @@ -569,3 +569,59 @@ static struct mdp4_platform_config *mdp4_get_config(struct platform_device *dev)
>   
>   	return &config;
>   }
> +
> +static const struct dev_pm_ops mdp4_pm_ops = {
> +	.prepare = msm_pm_prepare,
> +	.complete = msm_pm_complete,
> +};
> +
> +static int mdp4_probe(struct platform_device *pdev)
> +{
> +	struct msm_drm_private *priv;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	/*
> +	 * on MDP4 based platforms, the MDP platform device is the component
> +	 * master that adds other display interface components to itself.
> +	 */
> +	return msm_drv_probe(&pdev->dev, &pdev->dev);
> +}
> +
> +static int mdp4_remove(struct platform_device *pdev)
> +{
> +	component_master_del(&pdev->dev, &msm_drm_ops);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id mdp4_dt_match[] = {
> +	{ .compatible = "qcom,mdp4", .data = (void *)KMS_MDP4 },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mdp4_dt_match);
> +
> +static struct platform_driver mdp4_platform_driver = {
> +	.probe      = mdp4_probe,
> +	.remove     = mdp4_remove,
> +	.shutdown   = msm_drv_shutdown,
> +	.driver     = {
> +		.name   = "mdp4",
> +		.of_match_table = mdp4_dt_match,
> +		.pm     = &mdp4_pm_ops,
> +	},
> +};
> +
> +void __init msm_mdp4_register(void)
> +{
> +	platform_driver_register(&mdp4_platform_driver);
> +}
> +
> +void __exit msm_mdp4_unregister(void)
> +{
> +	platform_driver_unregister(&mdp4_platform_driver);
> +}
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index f3f33b8c6eba..2f44df8c5585 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -255,10 +255,6 @@ static int msm_drm_uninit(struct device *dev)
>   	return 0;
>   }
>   
> -#define KMS_MDP4 4
> -#define KMS_MDP5 5
> -#define KMS_DPU  3
> -
>   static int get_mdp_ver(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
> @@ -941,50 +937,7 @@ static const struct drm_driver msm_driver = {
>   	.patchlevel         = MSM_VERSION_PATCHLEVEL,
>   };
>   
> -static int __maybe_unused msm_runtime_suspend(struct device *dev)
> -{
> -	struct msm_drm_private *priv = dev_get_drvdata(dev);
> -	struct msm_mdss *mdss = priv->mdss;
> -
> -	DBG("");
> -
> -	if (mdss)
> -		return msm_mdss_disable(mdss);
> -
> -	return 0;
> -}
> -
> -static int __maybe_unused msm_runtime_resume(struct device *dev)
> -{
> -	struct msm_drm_private *priv = dev_get_drvdata(dev);
> -	struct msm_mdss *mdss = priv->mdss;
> -
> -	DBG("");
> -
> -	if (mdss)
> -		return msm_mdss_enable(mdss);
> -
> -	return 0;
> -}
> -
> -static int __maybe_unused msm_pm_suspend(struct device *dev)
> -{
> -
> -	if (pm_runtime_suspended(dev))
> -		return 0;
> -
> -	return msm_runtime_suspend(dev);
> -}
> -
> -static int __maybe_unused msm_pm_resume(struct device *dev)
> -{
> -	if (pm_runtime_suspended(dev))
> -		return 0;
> -
> -	return msm_runtime_resume(dev);
> -}
> -
> -static int __maybe_unused msm_pm_prepare(struct device *dev)
> +int msm_pm_prepare(struct device *dev)
>   {
>   	struct msm_drm_private *priv = dev_get_drvdata(dev);
>   	struct drm_device *ddev = priv ? priv->dev : NULL;
> @@ -995,7 +948,7 @@ static int __maybe_unused msm_pm_prepare(struct device *dev)
>   	return drm_mode_config_helper_suspend(ddev);
>   }
>   
> -static void __maybe_unused msm_pm_complete(struct device *dev)
> +void msm_pm_complete(struct device *dev)
>   {
>   	struct msm_drm_private *priv = dev_get_drvdata(dev);
>   	struct drm_device *ddev = priv ? priv->dev : NULL;
> @@ -1007,8 +960,6 @@ static void __maybe_unused msm_pm_complete(struct device *dev)
>   }
>   
>   static const struct dev_pm_ops msm_pm_ops = {
> -	SET_SYSTEM_SLEEP_PM_OPS(msm_pm_suspend, msm_pm_resume)
> -	SET_RUNTIME_PM_OPS(msm_runtime_suspend, msm_runtime_resume, NULL)
>   	.prepare = msm_pm_prepare,
>   	.complete = msm_pm_complete,
>   };
> @@ -1032,25 +983,11 @@ static int compare_of(struct device *dev, void *data)
>    * is no external component that we need to add since LVDS is within MDP4
>    * itself.
>    */
> -static int add_components_mdp(struct device *mdp_dev,
> +static int add_components_mdp(struct device *master_dev, struct device *mdp_dev,
>   			      struct component_match **matchptr)
>   {
>   	struct device_node *np = mdp_dev->of_node;
>   	struct device_node *ep_node;
> -	struct device *master_dev;
> -
> -	/*
> -	 * on MDP4 based platforms, the MDP platform device is the component
> -	 * master that adds other display interface components to itself.
> -	 *
> -	 * on MDP5 based platforms, the MDSS platform device is the component
> -	 * master that adds MDP5 and other display interface components to
> -	 * itself.
> -	 */
> -	if (of_device_is_compatible(np, "qcom,mdp4"))
> -		master_dev = mdp_dev;
> -	else
> -		master_dev = mdp_dev->parent;
>   
>   	for_each_endpoint_of_node(np, ep_node) {
>   		struct device_node *intf;
> @@ -1091,60 +1028,6 @@ static int add_components_mdp(struct device *mdp_dev,
>   	return 0;
>   }
>   
> -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 add_display_components(struct platform_device *pdev,
> -				  struct component_match **matchptr)
> -{
> -	struct device *mdp_dev;
> -	struct device *dev = &pdev->dev;
> -	int ret;
> -
> -	/*
> -	 * 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.
> -	 */
> -	switch (get_mdp_ver(pdev)) {
> -	case KMS_MDP5:
> -	case KMS_DPU:
> -		ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
> -		if (ret) {
> -			DRM_DEV_ERROR(dev, "failed to populate children devices\n");
> -			return ret;
> -		}
> -
> -		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);
> -			return -ENODEV;
> -		}
> -
> -		put_device(mdp_dev);
> -
> -		/* add the MDP component itself */
> -		drm_of_component_match_add(dev, matchptr, compare_of,
> -					   mdp_dev->of_node);
> -		break;
> -	case KMS_MDP4:
> -		/* MDP4 */
> -		mdp_dev = dev;
> -		break;
> -	}
> -
> -	ret = add_components_mdp(mdp_dev, matchptr);
> -	if (ret)
> -		of_platform_depopulate(dev);
> -
> -	return ret;
> -}
> -
>   /*
>    * We don't know what's the best binding to link the gpu with the drm device.
>    * Fow now, we just hunt for all the possible gpus that we support, and add them
> @@ -1185,93 +1068,70 @@ static void msm_drm_unbind(struct device *dev)
>   	msm_drm_uninit(dev);
>   }
>   
> -static const struct component_master_ops msm_drm_ops = {
> +const struct component_master_ops msm_drm_ops = {
>   	.bind = msm_drm_bind,
>   	.unbind = msm_drm_unbind,
>   };
>   
> -/*
> - * Platform driver:
> - */
> -
> -static int msm_pdev_probe(struct platform_device *pdev)
> +int msm_drv_probe(struct device *master_dev, struct device *mdp_dev)
>   {
>   	struct component_match *match = NULL;
> -	struct msm_mdss *mdss;
> -	struct msm_drm_private *priv;
>   	int ret;
>   
> -	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> -	if (!priv)
> -		return -ENOMEM;
> -
> -	platform_set_drvdata(pdev, priv);
> -
> -	switch (get_mdp_ver(pdev)) {
> -	case KMS_MDP5:
> -		mdss = msm_mdss_init(pdev, true);
> -		break;
> -	case KMS_DPU:
> -		mdss = msm_mdss_init(pdev, false);
> -		break;
> -	default:
> -		mdss = NULL;
> -		break;
> -	}
> -	if (IS_ERR(mdss)) {
> -		ret = PTR_ERR(mdss);
> -		return ret;
> -	}
> -
> -	priv->mdss = mdss;
> +	if (mdp_dev) {
> +		/* add the MDP component itself */
> +		drm_of_component_match_add(master_dev, &match, compare_of,
> +				mdp_dev->of_node);
>   
> -	if (get_mdp_ver(pdev)) {
> -		ret = add_display_components(pdev, &match);
> +		ret = add_components_mdp(master_dev, mdp_dev, &match);
>   		if (ret)
> -			goto fail;
> +			return ret;
>   	}
>   
> -	ret = add_gpu_components(&pdev->dev, &match);
> +	ret = add_gpu_components(master_dev, &match);
>   	if (ret)
> -		goto fail;
> +		return ret;
>   
>   	/* on all devices that I am aware of, iommu's which can map
>   	 * any address the cpu can see are used:
>   	 */
> -	ret = dma_set_mask_and_coherent(&pdev->dev, ~0);
> +	ret = dma_set_mask_and_coherent(master_dev, ~0);
>   	if (ret)
> -		goto fail;
> +		return ret;
>   
> -	ret = component_master_add_with_match(&pdev->dev, &msm_drm_ops, match);
> +	ret = component_master_add_with_match(master_dev, &msm_drm_ops, match);
>   	if (ret)
> -		goto fail;
> +		return ret;
>   
>   	return 0;
> +}
>   
> -fail:
> -	of_platform_depopulate(&pdev->dev);
> +/*
> + * Platform driver:
> + * Used only for headlesss GPU instances
> + */
>   
> -	if (priv->mdss)
> -		msm_mdss_destroy(priv->mdss);
> +static int msm_pdev_probe(struct platform_device *pdev)
> +{
> +	struct msm_drm_private *priv;
>   
> -	return ret;
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	return msm_drv_probe(&pdev->dev, NULL);
>   }
>   
>   static int msm_pdev_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);
> -
> -	if (mdss)
> -		msm_mdss_destroy(mdss);
>   
>   	return 0;
>   }
>   
> -static void msm_pdev_shutdown(struct platform_device *pdev)
> +void msm_drv_shutdown(struct platform_device *pdev)
>   {
>   	struct msm_drm_private *priv = platform_get_drvdata(pdev);
>   	struct drm_device *drm = priv ? priv->dev : NULL;
> @@ -1282,28 +1142,12 @@ static void msm_pdev_shutdown(struct platform_device *pdev)
>   	drm_atomic_helper_shutdown(drm);
>   }
>   
> -static const struct of_device_id dt_match[] = {
> -	{ .compatible = "qcom,mdp4", .data = (void *)KMS_MDP4 },
> -	{ .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 },
> -	{ .compatible = "qcom,msm8998-mdss", .data = (void *)KMS_DPU },
> -	{ .compatible = "qcom,qcm2290-mdss", .data = (void *)KMS_DPU },
> -	{ .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,sc8180x-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 msm_platform_driver = {
>   	.probe      = msm_pdev_probe,
>   	.remove     = msm_pdev_remove,
> -	.shutdown   = msm_pdev_shutdown,
> +	.shutdown   = msm_drv_shutdown,
>   	.driver     = {
>   		.name   = "msm",
> -		.of_match_table = dt_match,
>   		.pm     = &msm_pm_ops,
>   	},
>   };
> @@ -1320,6 +1164,8 @@ static int __init msm_drm_register(void)
>   	msm_hdmi_register();
>   	msm_dp_register();
>   	adreno_register();
> +	msm_mdp4_register();
> +	msm_mdss_register();
>   	return platform_driver_register(&msm_platform_driver);
>   }
>   
> @@ -1327,6 +1173,8 @@ static void __exit msm_drm_unregister(void)
>   {
>   	DBG("fini");
>   	platform_driver_unregister(&msm_platform_driver);
> +	msm_mdss_unregister();
> +	msm_mdp4_unregister();
>   	msm_dp_unregister();
>   	msm_hdmi_unregister();
>   	adreno_unregister();
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 9f68aa685ed7..2fdaf4a614cc 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -431,10 +431,18 @@ static inline void msm_dp_debugfs_init(struct msm_dp *dp_display,
>   
>   #endif
>   
> -void __init msm_mdp_register(void);
> -void __exit msm_mdp_unregister(void);
> -void __init msm_dpu_register(void);
> -void __exit msm_dpu_unregister(void);
> +#define KMS_MDP4 4
> +#define KMS_MDP5 5
> +#define KMS_DPU  3
> +
> +void msm_mdp4_register(void);
> +void msm_mdp4_unregister(void);
> +void msm_mdp_register(void);
> +void msm_mdp_unregister(void);
> +void msm_dpu_register(void);
> +void msm_dpu_unregister(void);
> +void msm_mdss_register(void);
> +void msm_mdss_unregister(void);
>   
>   #ifdef CONFIG_DEBUG_FS
>   void msm_framebuffer_describe(struct drm_framebuffer *fb, struct seq_file *m);
> @@ -534,4 +542,15 @@ static inline unsigned long timeout_to_jiffies(const ktime_t *timeout)
>   	return clamp(remaining_jiffies, 0LL, (s64)INT_MAX);
>   }
>   
> +/* Driver helpers */
> +
> +extern const struct component_master_ops msm_drm_ops;
> +
> +int msm_pm_prepare(struct device *dev);
> +void msm_pm_complete(struct device *dev);
> +
> +int msm_drv_probe(struct device *master_dev, struct device *mdp_dev);
> +void msm_drv_shutdown(struct platform_device *pdev);
> +
> +
>   #endif /* __MSM_DRV_H__ */
> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> index 09c219988884..13c2eb0b2bcf 100644
> --- a/drivers/gpu/drm/msm/msm_kms.h
> +++ b/drivers/gpu/drm/msm/msm_kms.h
> @@ -201,13 +201,6 @@ struct msm_kms *dpu_kms_init(struct drm_device *dev);
>   extern const struct of_device_id dpu_dt_match[];
>   extern const struct of_device_id mdp5_dt_match[];
>   
> -struct msm_mdss;
> -
> -struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5);
> -int msm_mdss_enable(struct msm_mdss *mdss);
> -int msm_mdss_disable(struct msm_mdss *mdss);
> -void msm_mdss_destroy(struct msm_mdss *mdss);
> -
>   #define for_each_crtc_mask(dev, crtc, crtc_mask) \
>   	drm_for_each_crtc(crtc, dev) \
>   		for_each_if (drm_crtc_mask(crtc) & (crtc_mask))
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index 857eefbb8649..c89de88ed2d1 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -10,6 +10,9 @@
>   #include <linux/irqchip/chained_irq.h>
>   #include <linux/pm_runtime.h>
>   
> +#include "msm_drv.h"
> +#include "msm_kms.h"
> +
>   /* for DPU_HW_* defines */
>   #include "disp/dpu1/dpu_hw_catalog.h"
>   
> @@ -126,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;
>   
> @@ -162,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;
> @@ -212,7 +215,7 @@ static int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_d
>   	return num_clocks;
>   }
>   
> -struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5)
> +static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5)
>   {
>   	struct msm_mdss *msm_mdss;
>   	int ret;
> @@ -255,3 +258,170 @@ struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_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;
> +
> +	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;
> +
> +	/*
> +	 * 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
> +	 * that adds MDP5 and other display interface components to
> +	 * 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,msm8998-mdss", .data = (void *)KMS_DPU },
> +	{ .compatible = "qcom,qcm2290-mdss", .data = (void *)KMS_DPU },
> +	{ .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,sc8180x-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);
> +}
> +
> +void __exit msm_mdss_unregister(void)
> +{
> +	platform_driver_unregister(&mdss_platform_driver);
> +}

WARNING: multiple messages have this Message-ID (diff)
From: Abhinav Kumar <quic_abhinavk@quicinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>,
	linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Stephen Boyd <swboyd@chromium.org>,
	freedreno@lists.freedesktop.org
Subject: Re: [PATCH v3 3/5] drm/msm: split the main platform driver
Date: Fri, 4 Mar 2022 18:10:07 -0800	[thread overview]
Message-ID: <f9f5df47-0eae-abd0-67f5-2fe73594b7af@quicinc.com> (raw)
In-Reply-To: <20220304032106.2866043-4-dmitry.baryshkov@linaro.org>



On 3/3/2022 7:21 PM, Dmitry Baryshkov wrote:
> Currently the msm platform driver is a multiplex handling several cases:
> - headless GPU-only driver,
> - MDP4 with flat device nodes,
> - MDP5/DPU MDSS with all the nodes being children of MDSS node.
> 
> This results in not-so-perfect code, checking the hardware version
> (MDP4/MDP5/DPU) in several places, checking for mdss even when it can
> not exist, etc. Split the code into three handling subdrivers (mdp4,
> mdss and headless msm).
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

If this has been verified across MDP4, MDP5 and DPU,

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

> ---
>   drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c |  56 ++++++
>   drivers/gpu/drm/msm/msm_drv.c            | 228 ++++-------------------
>   drivers/gpu/drm/msm/msm_drv.h            |  27 ++-
>   drivers/gpu/drm/msm/msm_kms.h            |   7 -
>   drivers/gpu/drm/msm/msm_mdss.c           | 178 +++++++++++++++++-
>   5 files changed, 291 insertions(+), 205 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> index 3cf476c55158..c5c0650414c5 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> @@ -569,3 +569,59 @@ static struct mdp4_platform_config *mdp4_get_config(struct platform_device *dev)
>   
>   	return &config;
>   }
> +
> +static const struct dev_pm_ops mdp4_pm_ops = {
> +	.prepare = msm_pm_prepare,
> +	.complete = msm_pm_complete,
> +};
> +
> +static int mdp4_probe(struct platform_device *pdev)
> +{
> +	struct msm_drm_private *priv;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	/*
> +	 * on MDP4 based platforms, the MDP platform device is the component
> +	 * master that adds other display interface components to itself.
> +	 */
> +	return msm_drv_probe(&pdev->dev, &pdev->dev);
> +}
> +
> +static int mdp4_remove(struct platform_device *pdev)
> +{
> +	component_master_del(&pdev->dev, &msm_drm_ops);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id mdp4_dt_match[] = {
> +	{ .compatible = "qcom,mdp4", .data = (void *)KMS_MDP4 },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mdp4_dt_match);
> +
> +static struct platform_driver mdp4_platform_driver = {
> +	.probe      = mdp4_probe,
> +	.remove     = mdp4_remove,
> +	.shutdown   = msm_drv_shutdown,
> +	.driver     = {
> +		.name   = "mdp4",
> +		.of_match_table = mdp4_dt_match,
> +		.pm     = &mdp4_pm_ops,
> +	},
> +};
> +
> +void __init msm_mdp4_register(void)
> +{
> +	platform_driver_register(&mdp4_platform_driver);
> +}
> +
> +void __exit msm_mdp4_unregister(void)
> +{
> +	platform_driver_unregister(&mdp4_platform_driver);
> +}
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index f3f33b8c6eba..2f44df8c5585 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -255,10 +255,6 @@ static int msm_drm_uninit(struct device *dev)
>   	return 0;
>   }
>   
> -#define KMS_MDP4 4
> -#define KMS_MDP5 5
> -#define KMS_DPU  3
> -
>   static int get_mdp_ver(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
> @@ -941,50 +937,7 @@ static const struct drm_driver msm_driver = {
>   	.patchlevel         = MSM_VERSION_PATCHLEVEL,
>   };
>   
> -static int __maybe_unused msm_runtime_suspend(struct device *dev)
> -{
> -	struct msm_drm_private *priv = dev_get_drvdata(dev);
> -	struct msm_mdss *mdss = priv->mdss;
> -
> -	DBG("");
> -
> -	if (mdss)
> -		return msm_mdss_disable(mdss);
> -
> -	return 0;
> -}
> -
> -static int __maybe_unused msm_runtime_resume(struct device *dev)
> -{
> -	struct msm_drm_private *priv = dev_get_drvdata(dev);
> -	struct msm_mdss *mdss = priv->mdss;
> -
> -	DBG("");
> -
> -	if (mdss)
> -		return msm_mdss_enable(mdss);
> -
> -	return 0;
> -}
> -
> -static int __maybe_unused msm_pm_suspend(struct device *dev)
> -{
> -
> -	if (pm_runtime_suspended(dev))
> -		return 0;
> -
> -	return msm_runtime_suspend(dev);
> -}
> -
> -static int __maybe_unused msm_pm_resume(struct device *dev)
> -{
> -	if (pm_runtime_suspended(dev))
> -		return 0;
> -
> -	return msm_runtime_resume(dev);
> -}
> -
> -static int __maybe_unused msm_pm_prepare(struct device *dev)
> +int msm_pm_prepare(struct device *dev)
>   {
>   	struct msm_drm_private *priv = dev_get_drvdata(dev);
>   	struct drm_device *ddev = priv ? priv->dev : NULL;
> @@ -995,7 +948,7 @@ static int __maybe_unused msm_pm_prepare(struct device *dev)
>   	return drm_mode_config_helper_suspend(ddev);
>   }
>   
> -static void __maybe_unused msm_pm_complete(struct device *dev)
> +void msm_pm_complete(struct device *dev)
>   {
>   	struct msm_drm_private *priv = dev_get_drvdata(dev);
>   	struct drm_device *ddev = priv ? priv->dev : NULL;
> @@ -1007,8 +960,6 @@ static void __maybe_unused msm_pm_complete(struct device *dev)
>   }
>   
>   static const struct dev_pm_ops msm_pm_ops = {
> -	SET_SYSTEM_SLEEP_PM_OPS(msm_pm_suspend, msm_pm_resume)
> -	SET_RUNTIME_PM_OPS(msm_runtime_suspend, msm_runtime_resume, NULL)
>   	.prepare = msm_pm_prepare,
>   	.complete = msm_pm_complete,
>   };
> @@ -1032,25 +983,11 @@ static int compare_of(struct device *dev, void *data)
>    * is no external component that we need to add since LVDS is within MDP4
>    * itself.
>    */
> -static int add_components_mdp(struct device *mdp_dev,
> +static int add_components_mdp(struct device *master_dev, struct device *mdp_dev,
>   			      struct component_match **matchptr)
>   {
>   	struct device_node *np = mdp_dev->of_node;
>   	struct device_node *ep_node;
> -	struct device *master_dev;
> -
> -	/*
> -	 * on MDP4 based platforms, the MDP platform device is the component
> -	 * master that adds other display interface components to itself.
> -	 *
> -	 * on MDP5 based platforms, the MDSS platform device is the component
> -	 * master that adds MDP5 and other display interface components to
> -	 * itself.
> -	 */
> -	if (of_device_is_compatible(np, "qcom,mdp4"))
> -		master_dev = mdp_dev;
> -	else
> -		master_dev = mdp_dev->parent;
>   
>   	for_each_endpoint_of_node(np, ep_node) {
>   		struct device_node *intf;
> @@ -1091,60 +1028,6 @@ static int add_components_mdp(struct device *mdp_dev,
>   	return 0;
>   }
>   
> -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 add_display_components(struct platform_device *pdev,
> -				  struct component_match **matchptr)
> -{
> -	struct device *mdp_dev;
> -	struct device *dev = &pdev->dev;
> -	int ret;
> -
> -	/*
> -	 * 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.
> -	 */
> -	switch (get_mdp_ver(pdev)) {
> -	case KMS_MDP5:
> -	case KMS_DPU:
> -		ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
> -		if (ret) {
> -			DRM_DEV_ERROR(dev, "failed to populate children devices\n");
> -			return ret;
> -		}
> -
> -		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);
> -			return -ENODEV;
> -		}
> -
> -		put_device(mdp_dev);
> -
> -		/* add the MDP component itself */
> -		drm_of_component_match_add(dev, matchptr, compare_of,
> -					   mdp_dev->of_node);
> -		break;
> -	case KMS_MDP4:
> -		/* MDP4 */
> -		mdp_dev = dev;
> -		break;
> -	}
> -
> -	ret = add_components_mdp(mdp_dev, matchptr);
> -	if (ret)
> -		of_platform_depopulate(dev);
> -
> -	return ret;
> -}
> -
>   /*
>    * We don't know what's the best binding to link the gpu with the drm device.
>    * Fow now, we just hunt for all the possible gpus that we support, and add them
> @@ -1185,93 +1068,70 @@ static void msm_drm_unbind(struct device *dev)
>   	msm_drm_uninit(dev);
>   }
>   
> -static const struct component_master_ops msm_drm_ops = {
> +const struct component_master_ops msm_drm_ops = {
>   	.bind = msm_drm_bind,
>   	.unbind = msm_drm_unbind,
>   };
>   
> -/*
> - * Platform driver:
> - */
> -
> -static int msm_pdev_probe(struct platform_device *pdev)
> +int msm_drv_probe(struct device *master_dev, struct device *mdp_dev)
>   {
>   	struct component_match *match = NULL;
> -	struct msm_mdss *mdss;
> -	struct msm_drm_private *priv;
>   	int ret;
>   
> -	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> -	if (!priv)
> -		return -ENOMEM;
> -
> -	platform_set_drvdata(pdev, priv);
> -
> -	switch (get_mdp_ver(pdev)) {
> -	case KMS_MDP5:
> -		mdss = msm_mdss_init(pdev, true);
> -		break;
> -	case KMS_DPU:
> -		mdss = msm_mdss_init(pdev, false);
> -		break;
> -	default:
> -		mdss = NULL;
> -		break;
> -	}
> -	if (IS_ERR(mdss)) {
> -		ret = PTR_ERR(mdss);
> -		return ret;
> -	}
> -
> -	priv->mdss = mdss;
> +	if (mdp_dev) {
> +		/* add the MDP component itself */
> +		drm_of_component_match_add(master_dev, &match, compare_of,
> +				mdp_dev->of_node);
>   
> -	if (get_mdp_ver(pdev)) {
> -		ret = add_display_components(pdev, &match);
> +		ret = add_components_mdp(master_dev, mdp_dev, &match);
>   		if (ret)
> -			goto fail;
> +			return ret;
>   	}
>   
> -	ret = add_gpu_components(&pdev->dev, &match);
> +	ret = add_gpu_components(master_dev, &match);
>   	if (ret)
> -		goto fail;
> +		return ret;
>   
>   	/* on all devices that I am aware of, iommu's which can map
>   	 * any address the cpu can see are used:
>   	 */
> -	ret = dma_set_mask_and_coherent(&pdev->dev, ~0);
> +	ret = dma_set_mask_and_coherent(master_dev, ~0);
>   	if (ret)
> -		goto fail;
> +		return ret;
>   
> -	ret = component_master_add_with_match(&pdev->dev, &msm_drm_ops, match);
> +	ret = component_master_add_with_match(master_dev, &msm_drm_ops, match);
>   	if (ret)
> -		goto fail;
> +		return ret;
>   
>   	return 0;
> +}
>   
> -fail:
> -	of_platform_depopulate(&pdev->dev);
> +/*
> + * Platform driver:
> + * Used only for headlesss GPU instances
> + */
>   
> -	if (priv->mdss)
> -		msm_mdss_destroy(priv->mdss);
> +static int msm_pdev_probe(struct platform_device *pdev)
> +{
> +	struct msm_drm_private *priv;
>   
> -	return ret;
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	return msm_drv_probe(&pdev->dev, NULL);
>   }
>   
>   static int msm_pdev_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);
> -
> -	if (mdss)
> -		msm_mdss_destroy(mdss);
>   
>   	return 0;
>   }
>   
> -static void msm_pdev_shutdown(struct platform_device *pdev)
> +void msm_drv_shutdown(struct platform_device *pdev)
>   {
>   	struct msm_drm_private *priv = platform_get_drvdata(pdev);
>   	struct drm_device *drm = priv ? priv->dev : NULL;
> @@ -1282,28 +1142,12 @@ static void msm_pdev_shutdown(struct platform_device *pdev)
>   	drm_atomic_helper_shutdown(drm);
>   }
>   
> -static const struct of_device_id dt_match[] = {
> -	{ .compatible = "qcom,mdp4", .data = (void *)KMS_MDP4 },
> -	{ .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 },
> -	{ .compatible = "qcom,msm8998-mdss", .data = (void *)KMS_DPU },
> -	{ .compatible = "qcom,qcm2290-mdss", .data = (void *)KMS_DPU },
> -	{ .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,sc8180x-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 msm_platform_driver = {
>   	.probe      = msm_pdev_probe,
>   	.remove     = msm_pdev_remove,
> -	.shutdown   = msm_pdev_shutdown,
> +	.shutdown   = msm_drv_shutdown,
>   	.driver     = {
>   		.name   = "msm",
> -		.of_match_table = dt_match,
>   		.pm     = &msm_pm_ops,
>   	},
>   };
> @@ -1320,6 +1164,8 @@ static int __init msm_drm_register(void)
>   	msm_hdmi_register();
>   	msm_dp_register();
>   	adreno_register();
> +	msm_mdp4_register();
> +	msm_mdss_register();
>   	return platform_driver_register(&msm_platform_driver);
>   }
>   
> @@ -1327,6 +1173,8 @@ static void __exit msm_drm_unregister(void)
>   {
>   	DBG("fini");
>   	platform_driver_unregister(&msm_platform_driver);
> +	msm_mdss_unregister();
> +	msm_mdp4_unregister();
>   	msm_dp_unregister();
>   	msm_hdmi_unregister();
>   	adreno_unregister();
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 9f68aa685ed7..2fdaf4a614cc 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -431,10 +431,18 @@ static inline void msm_dp_debugfs_init(struct msm_dp *dp_display,
>   
>   #endif
>   
> -void __init msm_mdp_register(void);
> -void __exit msm_mdp_unregister(void);
> -void __init msm_dpu_register(void);
> -void __exit msm_dpu_unregister(void);
> +#define KMS_MDP4 4
> +#define KMS_MDP5 5
> +#define KMS_DPU  3
> +
> +void msm_mdp4_register(void);
> +void msm_mdp4_unregister(void);
> +void msm_mdp_register(void);
> +void msm_mdp_unregister(void);
> +void msm_dpu_register(void);
> +void msm_dpu_unregister(void);
> +void msm_mdss_register(void);
> +void msm_mdss_unregister(void);
>   
>   #ifdef CONFIG_DEBUG_FS
>   void msm_framebuffer_describe(struct drm_framebuffer *fb, struct seq_file *m);
> @@ -534,4 +542,15 @@ static inline unsigned long timeout_to_jiffies(const ktime_t *timeout)
>   	return clamp(remaining_jiffies, 0LL, (s64)INT_MAX);
>   }
>   
> +/* Driver helpers */
> +
> +extern const struct component_master_ops msm_drm_ops;
> +
> +int msm_pm_prepare(struct device *dev);
> +void msm_pm_complete(struct device *dev);
> +
> +int msm_drv_probe(struct device *master_dev, struct device *mdp_dev);
> +void msm_drv_shutdown(struct platform_device *pdev);
> +
> +
>   #endif /* __MSM_DRV_H__ */
> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> index 09c219988884..13c2eb0b2bcf 100644
> --- a/drivers/gpu/drm/msm/msm_kms.h
> +++ b/drivers/gpu/drm/msm/msm_kms.h
> @@ -201,13 +201,6 @@ struct msm_kms *dpu_kms_init(struct drm_device *dev);
>   extern const struct of_device_id dpu_dt_match[];
>   extern const struct of_device_id mdp5_dt_match[];
>   
> -struct msm_mdss;
> -
> -struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5);
> -int msm_mdss_enable(struct msm_mdss *mdss);
> -int msm_mdss_disable(struct msm_mdss *mdss);
> -void msm_mdss_destroy(struct msm_mdss *mdss);
> -
>   #define for_each_crtc_mask(dev, crtc, crtc_mask) \
>   	drm_for_each_crtc(crtc, dev) \
>   		for_each_if (drm_crtc_mask(crtc) & (crtc_mask))
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index 857eefbb8649..c89de88ed2d1 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -10,6 +10,9 @@
>   #include <linux/irqchip/chained_irq.h>
>   #include <linux/pm_runtime.h>
>   
> +#include "msm_drv.h"
> +#include "msm_kms.h"
> +
>   /* for DPU_HW_* defines */
>   #include "disp/dpu1/dpu_hw_catalog.h"
>   
> @@ -126,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;
>   
> @@ -162,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;
> @@ -212,7 +215,7 @@ static int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_d
>   	return num_clocks;
>   }
>   
> -struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5)
> +static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5)
>   {
>   	struct msm_mdss *msm_mdss;
>   	int ret;
> @@ -255,3 +258,170 @@ struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_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;
> +
> +	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;
> +
> +	/*
> +	 * 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
> +	 * that adds MDP5 and other display interface components to
> +	 * 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,msm8998-mdss", .data = (void *)KMS_DPU },
> +	{ .compatible = "qcom,qcm2290-mdss", .data = (void *)KMS_DPU },
> +	{ .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,sc8180x-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);
> +}
> +
> +void __exit msm_mdss_unregister(void)
> +{
> +	platform_driver_unregister(&mdss_platform_driver);
> +}

  reply	other threads:[~2022-03-05  2:10 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-04  3:21 [PATCH v3 0/5] drm/msm: rework MDSS drivers Dmitry Baryshkov
2022-03-04  3:21 ` Dmitry Baryshkov
2022-03-04  3:21 ` [PATCH v3 1/5] drm/msm: unify " Dmitry Baryshkov
2022-03-04  3:21   ` Dmitry Baryshkov
2022-03-05  0:39   ` Abhinav Kumar
2022-03-05  0:39     ` Abhinav Kumar
2022-03-08 20:25   ` Stephen Boyd
2022-03-08 20:25     ` Stephen Boyd
2022-03-04  3:21 ` [PATCH v3 2/5] drm/msm: remove extra indirection for msm_mdss Dmitry Baryshkov
2022-03-04  3:21   ` Dmitry Baryshkov
2022-03-05  0:42   ` Abhinav Kumar
2022-03-05  0:42     ` Abhinav Kumar
2022-03-08 20:26   ` Stephen Boyd
2022-03-08 20:26     ` Stephen Boyd
2022-03-04  3:21 ` [PATCH v3 3/5] drm/msm: split the main platform driver Dmitry Baryshkov
2022-03-04  3:21   ` Dmitry Baryshkov
2022-03-05  2:10   ` Abhinav Kumar [this message]
2022-03-05  2:10     ` Abhinav Kumar
2022-03-08 20:31   ` Stephen Boyd
2022-03-08 20:31     ` Stephen Boyd
2022-03-04  3:21 ` [PATCH v3 4/5] drm/msm: stop using device's match data pointer Dmitry Baryshkov
2022-03-04  3:21   ` Dmitry Baryshkov
2022-03-08 20:32   ` Stephen Boyd
2022-03-08 20:32     ` Stephen Boyd
2022-03-16  0:20   ` Abhinav Kumar
2022-03-16  0:20     ` Abhinav Kumar
2022-03-04  3:21 ` [PATCH v3 5/5] drm/msm: allow compile time selection of driver components Dmitry Baryshkov
2022-03-04  3:21   ` Dmitry Baryshkov
2022-03-04 22:00   ` Rob Clark
2022-03-04 22:00     ` Rob Clark
2022-03-08 20:36   ` Stephen Boyd
2022-03-08 20:36     ` Stephen Boyd
2022-03-16  0:28   ` Abhinav Kumar
2022-03-16  0:28     ` Abhinav Kumar
2022-03-16  7:31     ` Dmitry Baryshkov
2022-03-16  7:31       ` Dmitry Baryshkov
2022-03-16 17:26       ` [Freedreno] " Abhinav Kumar
2022-03-16 17:26         ` Abhinav Kumar
2022-03-17 12:44         ` Dmitry Baryshkov
2022-03-17 12:44           ` Dmitry Baryshkov
2022-03-17 15:43           ` Dmitry Baryshkov
2022-03-17 15:43             ` Dmitry Baryshkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f9f5df47-0eae-abd0-67f5-2fe73594b7af@quicinc.com \
    --to=quic_abhinavk@quicinc.com \
    --cc=airlied@linux.ie \
    --cc=bjorn.andersson@linaro.org \
    --cc=daniel@ffwll.ch \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=robdclark@gmail.com \
    --cc=sean@poorly.run \
    --cc=swboyd@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.