Re: [Freedreno] [PATCH v3 3/6] drm/msm: split the main platform driver
Quoting Dmitry Baryshkov (2022-03-23 02:25:35)
> 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).
>
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Dmitry Baryshkov
> ---
With the match table fixed and the nit below
Reviewed-by: Stephen Boyd
> 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.
Just delete master. It provides no value in this sentence.
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index 62007a4f29a2..512708101931 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -255,3 +258,167 @@ struct msm_mdss *msm_mdss_init(struct platform_device
> *pdev, bool is_mdp5)
[...]
> + 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
Like here.
Re: [Freedreno] [PATCH v3 3/6] drm/msm: split the main platform driver
Hi Dmitry,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm/drm-next]
[cannot apply to v5.17 next-20220323]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Dmitry-Baryshkov/drm-msm-rework-MDSS-drivers/20220323-172654
base: git://anongit.freedesktop.org/drm/drm drm-next
config: nios2-randconfig-p002-20220323
(https://download.01.org/0day-ci/archive/20220323/202203232120.4ejf1vfq-...@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/5484d7bfa709bbe2cd2cbb3b9959190d7a025c16
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Dmitry-Baryshkov/drm-msm-rework-MDSS-drivers/20220323-172654
git checkout 5484d7bfa709bbe2cd2cbb3b9959190d7a025c16
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross
O=build_dir ARCH=nios2 SHELL=/bin/bash drivers/gpu/drm/msm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
In file included from include/linux/device/driver.h:21,
from include/linux/device.h:32,
from include/linux/acpi.h:15,
from include/linux/irqchip.h:14,
from drivers/gpu/drm/msm/msm_mdss.c:8:
>> drivers/gpu/drm/msm/msm_mdss.c:403:25: error: 'dt_match' undeclared here
>> (not in a function); did you mean 'dr_match_t'?
403 | MODULE_DEVICE_TABLE(of, dt_match);
| ^~~~
include/linux/module.h:244:15: note: in definition of macro
'MODULE_DEVICE_TABLE'
244 | extern typeof(name) __mod_##type##__##name##_device_table
\
| ^~~~
>> include/linux/module.h:244:21: error: '__mod_of__dt_match_device_table'
>> aliased to undefined symbol 'dt_match'
244 | extern typeof(name) __mod_##type##__##name##_device_table
\
| ^~
drivers/gpu/drm/msm/msm_mdss.c:403:1: note: in expansion of macro
'MODULE_DEVICE_TABLE'
403 | MODULE_DEVICE_TABLE(of, dt_match);
| ^~~
vim +403 drivers/gpu/drm/msm/msm_mdss.c
390
391 static const struct of_device_id mdss_dt_match[] = {
392 { .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 },
393 { .compatible = "qcom,msm8998-mdss", .data = (void *)KMS_DPU },
394 { .compatible = "qcom,qcm2290-mdss", .data = (void *)KMS_DPU },
395 { .compatible = "qcom,sdm845-mdss", .data = (void *)KMS_DPU },
396 { .compatible = "qcom,sc7180-mdss", .data = (void *)KMS_DPU },
397 { .compatible = "qcom,sc7280-mdss", .data = (void *)KMS_DPU },
398 { .compatible = "qcom,sc8180x-mdss", .data = (void *)KMS_DPU },
399 { .compatible = "qcom,sm8150-mdss", .data = (void *)KMS_DPU },
400 { .compatible = "qcom,sm8250-mdss", .data = (void *)KMS_DPU },
401 {}
402 };
> 403 MODULE_DEVICE_TABLE(of, dt_match);
404
--
0-DAY CI Kernel Test Service
https://01.org/lkp
[Freedreno] [PATCH v3 3/6] drm/msm: split the main platform driver
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).
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
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 | 175 -
5 files changed, 288 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 d3cce39c13bf..16d8aa225405 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -256,10 +256,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;
@@ -970,50 +966,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;
@@ -1024,7 +977,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;
@@ -1036,8 +989,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
