2023-10-10 01:34:17

by Stephen Rothwell

[permalink] [raw]
Subject: linux-next: manual merge of the drm-msm tree with the mm, drm trees

Hi all,

FIXME: Add owner of second tree to To:
Add author(s)/SOB of conflicting commits.

Today's linux-next merge of the drm-msm tree got conflicts in:

drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
drivers/gpu/drm/msm/msm_drv.c

between commits:

01790d5e372f ("drm/msm: Convert to platform remove callback returning void")
cd61a76c210a ("drm/msm: dynamically allocate the drm-msm_gem shrinker")

from the mm, drm trees and commits:

283add3e6405 ("drm/msm: remove shutdown callback from msm_platform_driver")
506efcba3129 ("drm/msm: carve out KMS code from msm_drv.c")

from the drm-msm tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

--
Cheers,
Stephen Rothwell

diff --cc drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 82381d12414d,d14ae316796c..000000000000
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@@ -1299,12 -1230,72 +1230,70 @@@ static int dpu_kms_init(struct drm_devi

static int dpu_dev_probe(struct platform_device *pdev)
{
- return msm_drv_probe(&pdev->dev, dpu_kms_init);
+ struct device *dev = &pdev->dev;
+ struct dpu_kms *dpu_kms;
+ int irq;
+ int ret = 0;
+
+ dpu_kms = devm_kzalloc(dev, sizeof(*dpu_kms), GFP_KERNEL);
+ if (!dpu_kms)
+ return -ENOMEM;
+
+ dpu_kms->pdev = pdev;
+
+ ret = devm_pm_opp_set_clkname(dev, "core");
+ if (ret)
+ return ret;
+ /* OPP table is optional */
+ ret = devm_pm_opp_of_add_table(dev);
+ if (ret && ret != -ENODEV)
+ return dev_err_probe(dev, ret, "invalid OPP table in device tree\n");
+
+ ret = devm_clk_bulk_get_all(&pdev->dev, &dpu_kms->clocks);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "failed to parse clocks\n");
+
+ dpu_kms->num_clocks = ret;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return dev_err_probe(dev, irq, "failed to get irq\n");
+
+ dpu_kms->base.irq = irq;
+
+ dpu_kms->mmio = msm_ioremap(pdev, "mdp");
+ if (IS_ERR(dpu_kms->mmio)) {
+ ret = PTR_ERR(dpu_kms->mmio);
+ DPU_ERROR("mdp register memory map failed: %d\n", ret);
+ dpu_kms->mmio = NULL;
+ return ret;
+ }
+ DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio);
+
+ dpu_kms->vbif[VBIF_RT] = msm_ioremap(pdev, "vbif");
+ if (IS_ERR(dpu_kms->vbif[VBIF_RT])) {
+ ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]);
+ DPU_ERROR("vbif register memory map failed: %d\n", ret);
+ dpu_kms->vbif[VBIF_RT] = NULL;
+ return ret;
+ }
+
+ dpu_kms->vbif[VBIF_NRT] = msm_ioremap_quiet(pdev, "vbif_nrt");
+ if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) {
+ dpu_kms->vbif[VBIF_NRT] = NULL;
+ DPU_DEBUG("VBIF NRT is not defined");
+ }
+
+ ret = dpu_kms_parse_data_bus_icc_path(dpu_kms);
+ if (ret)
+ return ret;
+
+ return msm_drv_probe(&pdev->dev, dpu_kms_init, &dpu_kms->base);
}

-static int dpu_dev_remove(struct platform_device *pdev)
+static void dpu_dev_remove(struct platform_device *pdev)
{
component_master_del(&pdev->dev, &msm_drm_ops);
-
- return 0;
}

static int __maybe_unused dpu_runtime_suspend(struct device *dev)
@@@ -1380,8 -1371,8 +1369,8 @@@ MODULE_DEVICE_TABLE(of, dpu_dt_match)

static struct platform_driver dpu_driver = {
.probe = dpu_dev_probe,
- .remove = dpu_dev_remove,
+ .remove_new = dpu_dev_remove,
- .shutdown = msm_drv_shutdown,
+ .shutdown = msm_kms_shutdown,
.driver = {
.name = "msm_dpu",
.of_match_table = dpu_dt_match,
diff --cc drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index e5012fa6771f,982b7689e5b6..000000000000
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@@ -557,12 -507,60 +507,58 @@@ static const struct dev_pm_ops mdp4_pm_

static int mdp4_probe(struct platform_device *pdev)
{
- return msm_drv_probe(&pdev->dev, mdp4_kms_init);
+ struct device *dev = &pdev->dev;
+ struct mdp4_kms *mdp4_kms;
+ int irq;
+
+ mdp4_kms = devm_kzalloc(dev, sizeof(*mdp4_kms), GFP_KERNEL);
+ if (!mdp4_kms)
+ return dev_err_probe(dev, -ENOMEM, "failed to allocate kms\n");
+
+ mdp4_kms->mmio = msm_ioremap(pdev, NULL);
+ if (IS_ERR(mdp4_kms->mmio))
+ return PTR_ERR(mdp4_kms->mmio);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return dev_err_probe(dev, irq, "failed to get irq\n");
+
+ mdp4_kms->base.base.irq = irq;
+
+ /* NOTE: driver for this regulator still missing upstream.. use
+ * _get_exclusive() and ignore the error if it does not exist
+ * (and hope that the bootloader left it on for us)
+ */
+ mdp4_kms->vdd = devm_regulator_get_exclusive(&pdev->dev, "vdd");
+ if (IS_ERR(mdp4_kms->vdd))
+ mdp4_kms->vdd = NULL;
+
+ mdp4_kms->clk = devm_clk_get(&pdev->dev, "core_clk");
+ if (IS_ERR(mdp4_kms->clk))
+ return dev_err_probe(dev, PTR_ERR(mdp4_kms->clk), "failed to get core_clk\n");
+
+ mdp4_kms->pclk = devm_clk_get(&pdev->dev, "iface_clk");
+ if (IS_ERR(mdp4_kms->pclk))
+ mdp4_kms->pclk = NULL;
+
+ mdp4_kms->axi_clk = devm_clk_get(&pdev->dev, "bus_clk");
+ if (IS_ERR(mdp4_kms->axi_clk))
+ return dev_err_probe(dev, PTR_ERR(mdp4_kms->axi_clk), "failed to get axi_clk\n");
+
+ /*
+ * This is required for revn >= 2. Handle errors here and let the kms
+ * init bail out if the clock is not provided.
+ */
+ mdp4_kms->lut_clk = devm_clk_get_optional(&pdev->dev, "lut_clk");
+ if (IS_ERR(mdp4_kms->lut_clk))
+ return dev_err_probe(dev, PTR_ERR(mdp4_kms->lut_clk), "failed to get lut_clk\n");
+
+ return msm_drv_probe(&pdev->dev, mdp4_kms_init, &mdp4_kms->base.base);
}

-static int mdp4_remove(struct platform_device *pdev)
+static void 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[] = {
@@@ -573,8 -571,8 +569,8 @@@ MODULE_DEVICE_TABLE(of, mdp4_dt_match)

static struct platform_driver mdp4_platform_driver = {
.probe = mdp4_probe,
- .remove = mdp4_remove,
+ .remove_new = mdp4_remove,
- .shutdown = msm_drv_shutdown,
+ .shutdown = msm_kms_shutdown,
.driver = {
.name = "mdp4",
.of_match_table = mdp4_dt_match,
diff --cc drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 8a7b44376bc6,a28fbcd09684..000000000000
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@@ -939,10 -894,43 +894,43 @@@ static int mdp5_dev_probe(struct platfo
if (ret)
return ret;

- return msm_drv_probe(&pdev->dev, mdp5_kms_init);
+ mdp5_kms->pdev = pdev;
+
+ spin_lock_init(&mdp5_kms->resource_lock);
+
+ mdp5_kms->mmio = msm_ioremap(pdev, "mdp_phys");
+ if (IS_ERR(mdp5_kms->mmio))
+ return PTR_ERR(mdp5_kms->mmio);
+
+ /* mandatory clocks: */
+ ret = get_clk(pdev, &mdp5_kms->axi_clk, "bus", true);
+ if (ret)
+ return ret;
+ ret = get_clk(pdev, &mdp5_kms->ahb_clk, "iface", true);
+ if (ret)
+ return ret;
+ ret = get_clk(pdev, &mdp5_kms->core_clk, "core", true);
+ if (ret)
+ return ret;
+ ret = get_clk(pdev, &mdp5_kms->vsync_clk, "vsync", true);
+ if (ret)
+ return ret;
+
+ /* optional clocks: */
+ get_clk(pdev, &mdp5_kms->lut_clk, "lut", false);
+ get_clk(pdev, &mdp5_kms->tbu_clk, "tbu", false);
+ get_clk(pdev, &mdp5_kms->tbu_rt_clk, "tbu_rt", false);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return dev_err_probe(&pdev->dev, irq, "failed to get irq\n");
+
+ mdp5_kms->base.base.irq = irq;
+
+ return msm_drv_probe(&pdev->dev, mdp5_kms_init, &mdp5_kms->base.base);
}

-static int mdp5_dev_remove(struct platform_device *pdev)
+static void mdp5_dev_remove(struct platform_device *pdev)
{
DBG("");
component_master_del(&pdev->dev, &msm_drm_ops);
@@@ -986,8 -975,8 +974,8 @@@ MODULE_DEVICE_TABLE(of, mdp5_dt_match)

static struct platform_driver mdp5_driver = {
.probe = mdp5_dev_probe,
- .remove = mdp5_dev_remove,
+ .remove_new = mdp5_dev_remove,
- .shutdown = msm_drv_shutdown,
+ .shutdown = msm_kms_shutdown,
.driver = {
.name = "msm_mdp",
.of_match_table = mdp5_dt_match,
diff --cc drivers/gpu/drm/msm/msm_drv.c
index 05fe32c3a4b4,401e9ef86074..000000000000
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@@ -457,23 -264,12 +264,14 @@@ static int msm_drm_init(struct device *
if (ret)
goto err_deinit_vram;

- /* the fw fb could be anywhere in memory */
- ret = drm_aperture_remove_framebuffers(drv);
- if (ret)
- goto err_msm_uninit;
-
- msm_gem_shrinker_init(ddev);
+ ret = msm_gem_shrinker_init(ddev);
+ if (ret)
+ goto err_msm_uninit;

if (priv->kms_init) {
- ret = priv->kms_init(ddev);
- if (ret) {
- DRM_DEV_ERROR(dev, "failed to load kms\n");
- priv->kms = NULL;
+ ret = msm_drm_kms_init(dev, drv);
+ if (ret)
goto err_msm_uninit;
- }
- kms = priv->kms;
} else {
/* valid only for the dummy headless case, where of_node=NULL */
WARN_ON(dev->of_node);
@@@ -1277,37 -971,21 +973,19 @@@ int msm_drv_probe(struct device *master

static int msm_pdev_probe(struct platform_device *pdev)
{
- return msm_drv_probe(&pdev->dev, NULL);
+ return msm_drv_probe(&pdev->dev, NULL, NULL);
}

-static int msm_pdev_remove(struct platform_device *pdev)
+static void msm_pdev_remove(struct platform_device *pdev)
{
component_master_del(&pdev->dev, &msm_drm_ops);
-
- return 0;
}

- 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;
-
- /*
- * Shutdown the hw if we're far enough along where things might be on.
- * If we run this too early, we'll end up panicking in any variety of
- * places. Since we don't register the drm device until late in
- * msm_drm_init, drm_dev->registered is used as an indicator that the
- * shutdown will be successful.
- */
- if (drm && drm->registered && priv->kms)
- drm_atomic_helper_shutdown(drm);
- }
-
static struct platform_driver msm_platform_driver = {
.probe = msm_pdev_probe,
- .remove = msm_pdev_remove,
+ .remove_new = msm_pdev_remove,
- .shutdown = msm_drv_shutdown,
.driver = {
.name = "msm",
- .pm = &msm_pm_ops,
},
};


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature

2023-10-10 07:23:09

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: linux-next: manual merge of the drm-msm tree with the mm, drm trees

Hello Stephen,

On Tue, Oct 10, 2023 at 12:33:45PM +1100, Stephen Rothwell wrote:
> Today's linux-next merge of the drm-msm tree got conflicts in:
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> drivers/gpu/drm/msm/msm_drv.c
>
> between commits:
>
> 01790d5e372f ("drm/msm: Convert to platform remove callback returning void")
> cd61a76c210a ("drm/msm: dynamically allocate the drm-msm_gem shrinker")
>
> from the mm, drm trees and commits:
>
> 283add3e6405 ("drm/msm: remove shutdown callback from msm_platform_driver")
> 506efcba3129 ("drm/msm: carve out KMS code from msm_drv.c")
>
> from the drm-msm tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.

FTR: The conflict resolution looks right to me. Thanks!

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |


Attachments:
(No filename) (1.33 kB)
signature.asc (499.00 B)
Download all attachments

2023-10-11 03:38:42

by Rob Clark

[permalink] [raw]
Subject: Re: linux-next: manual merge of the drm-msm tree with the mm, drm trees

Hey Dave,

lmk how you want me to handle this to make it easier for you when I
send my pull request for 6.7.. I can merge drm-next to take care of
*that* conflict (either before I send my PR or push it somewhere where
you can see the resolution) but not sure about the mm conflict since
pulling that might get me ahead of drm-next. Either way, Stephen's
resolution looks correct.

BR,
-R

On Mon, Oct 9, 2023 at 6:33 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi all,
>
> FIXME: Add owner of second tree to To:
> Add author(s)/SOB of conflicting commits.
>
> Today's linux-next merge of the drm-msm tree got conflicts in:
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> drivers/gpu/drm/msm/msm_drv.c
>
> between commits:
>
> 01790d5e372f ("drm/msm: Convert to platform remove callback returning void")
> cd61a76c210a ("drm/msm: dynamically allocate the drm-msm_gem shrinker")
>
> from the mm, drm trees and commits:
>
> 283add3e6405 ("drm/msm: remove shutdown callback from msm_platform_driver")
> 506efcba3129 ("drm/msm: carve out KMS code from msm_drv.c")
>
> from the drm-msm tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 82381d12414d,d14ae316796c..000000000000
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@@ -1299,12 -1230,72 +1230,70 @@@ static int dpu_kms_init(struct drm_devi
>
> static int dpu_dev_probe(struct platform_device *pdev)
> {
> - return msm_drv_probe(&pdev->dev, dpu_kms_init);
> + struct device *dev = &pdev->dev;
> + struct dpu_kms *dpu_kms;
> + int irq;
> + int ret = 0;
> +
> + dpu_kms = devm_kzalloc(dev, sizeof(*dpu_kms), GFP_KERNEL);
> + if (!dpu_kms)
> + return -ENOMEM;
> +
> + dpu_kms->pdev = pdev;
> +
> + ret = devm_pm_opp_set_clkname(dev, "core");
> + if (ret)
> + return ret;
> + /* OPP table is optional */
> + ret = devm_pm_opp_of_add_table(dev);
> + if (ret && ret != -ENODEV)
> + return dev_err_probe(dev, ret, "invalid OPP table in device tree\n");
> +
> + ret = devm_clk_bulk_get_all(&pdev->dev, &dpu_kms->clocks);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to parse clocks\n");
> +
> + dpu_kms->num_clocks = ret;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return dev_err_probe(dev, irq, "failed to get irq\n");
> +
> + dpu_kms->base.irq = irq;
> +
> + dpu_kms->mmio = msm_ioremap(pdev, "mdp");
> + if (IS_ERR(dpu_kms->mmio)) {
> + ret = PTR_ERR(dpu_kms->mmio);
> + DPU_ERROR("mdp register memory map failed: %d\n", ret);
> + dpu_kms->mmio = NULL;
> + return ret;
> + }
> + DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio);
> +
> + dpu_kms->vbif[VBIF_RT] = msm_ioremap(pdev, "vbif");
> + if (IS_ERR(dpu_kms->vbif[VBIF_RT])) {
> + ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]);
> + DPU_ERROR("vbif register memory map failed: %d\n", ret);
> + dpu_kms->vbif[VBIF_RT] = NULL;
> + return ret;
> + }
> +
> + dpu_kms->vbif[VBIF_NRT] = msm_ioremap_quiet(pdev, "vbif_nrt");
> + if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) {
> + dpu_kms->vbif[VBIF_NRT] = NULL;
> + DPU_DEBUG("VBIF NRT is not defined");
> + }
> +
> + ret = dpu_kms_parse_data_bus_icc_path(dpu_kms);
> + if (ret)
> + return ret;
> +
> + return msm_drv_probe(&pdev->dev, dpu_kms_init, &dpu_kms->base);
> }
>
> -static int dpu_dev_remove(struct platform_device *pdev)
> +static void dpu_dev_remove(struct platform_device *pdev)
> {
> component_master_del(&pdev->dev, &msm_drm_ops);
> -
> - return 0;
> }
>
> static int __maybe_unused dpu_runtime_suspend(struct device *dev)
> @@@ -1380,8 -1371,8 +1369,8 @@@ MODULE_DEVICE_TABLE(of, dpu_dt_match)
>
> static struct platform_driver dpu_driver = {
> .probe = dpu_dev_probe,
> - .remove = dpu_dev_remove,
> + .remove_new = dpu_dev_remove,
> - .shutdown = msm_drv_shutdown,
> + .shutdown = msm_kms_shutdown,
> .driver = {
> .name = "msm_dpu",
> .of_match_table = dpu_dt_match,
> diff --cc drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> index e5012fa6771f,982b7689e5b6..000000000000
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> @@@ -557,12 -507,60 +507,58 @@@ static const struct dev_pm_ops mdp4_pm_
>
> static int mdp4_probe(struct platform_device *pdev)
> {
> - return msm_drv_probe(&pdev->dev, mdp4_kms_init);
> + struct device *dev = &pdev->dev;
> + struct mdp4_kms *mdp4_kms;
> + int irq;
> +
> + mdp4_kms = devm_kzalloc(dev, sizeof(*mdp4_kms), GFP_KERNEL);
> + if (!mdp4_kms)
> + return dev_err_probe(dev, -ENOMEM, "failed to allocate kms\n");
> +
> + mdp4_kms->mmio = msm_ioremap(pdev, NULL);
> + if (IS_ERR(mdp4_kms->mmio))
> + return PTR_ERR(mdp4_kms->mmio);
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return dev_err_probe(dev, irq, "failed to get irq\n");
> +
> + mdp4_kms->base.base.irq = irq;
> +
> + /* NOTE: driver for this regulator still missing upstream.. use
> + * _get_exclusive() and ignore the error if it does not exist
> + * (and hope that the bootloader left it on for us)
> + */
> + mdp4_kms->vdd = devm_regulator_get_exclusive(&pdev->dev, "vdd");
> + if (IS_ERR(mdp4_kms->vdd))
> + mdp4_kms->vdd = NULL;
> +
> + mdp4_kms->clk = devm_clk_get(&pdev->dev, "core_clk");
> + if (IS_ERR(mdp4_kms->clk))
> + return dev_err_probe(dev, PTR_ERR(mdp4_kms->clk), "failed to get core_clk\n");
> +
> + mdp4_kms->pclk = devm_clk_get(&pdev->dev, "iface_clk");
> + if (IS_ERR(mdp4_kms->pclk))
> + mdp4_kms->pclk = NULL;
> +
> + mdp4_kms->axi_clk = devm_clk_get(&pdev->dev, "bus_clk");
> + if (IS_ERR(mdp4_kms->axi_clk))
> + return dev_err_probe(dev, PTR_ERR(mdp4_kms->axi_clk), "failed to get axi_clk\n");
> +
> + /*
> + * This is required for revn >= 2. Handle errors here and let the kms
> + * init bail out if the clock is not provided.
> + */
> + mdp4_kms->lut_clk = devm_clk_get_optional(&pdev->dev, "lut_clk");
> + if (IS_ERR(mdp4_kms->lut_clk))
> + return dev_err_probe(dev, PTR_ERR(mdp4_kms->lut_clk), "failed to get lut_clk\n");
> +
> + return msm_drv_probe(&pdev->dev, mdp4_kms_init, &mdp4_kms->base.base);
> }
>
> -static int mdp4_remove(struct platform_device *pdev)
> +static void 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[] = {
> @@@ -573,8 -571,8 +569,8 @@@ MODULE_DEVICE_TABLE(of, mdp4_dt_match)
>
> static struct platform_driver mdp4_platform_driver = {
> .probe = mdp4_probe,
> - .remove = mdp4_remove,
> + .remove_new = mdp4_remove,
> - .shutdown = msm_drv_shutdown,
> + .shutdown = msm_kms_shutdown,
> .driver = {
> .name = "mdp4",
> .of_match_table = mdp4_dt_match,
> diff --cc drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 8a7b44376bc6,a28fbcd09684..000000000000
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@@ -939,10 -894,43 +894,43 @@@ static int mdp5_dev_probe(struct platfo
> if (ret)
> return ret;
>
> - return msm_drv_probe(&pdev->dev, mdp5_kms_init);
> + mdp5_kms->pdev = pdev;
> +
> + spin_lock_init(&mdp5_kms->resource_lock);
> +
> + mdp5_kms->mmio = msm_ioremap(pdev, "mdp_phys");
> + if (IS_ERR(mdp5_kms->mmio))
> + return PTR_ERR(mdp5_kms->mmio);
> +
> + /* mandatory clocks: */
> + ret = get_clk(pdev, &mdp5_kms->axi_clk, "bus", true);
> + if (ret)
> + return ret;
> + ret = get_clk(pdev, &mdp5_kms->ahb_clk, "iface", true);
> + if (ret)
> + return ret;
> + ret = get_clk(pdev, &mdp5_kms->core_clk, "core", true);
> + if (ret)
> + return ret;
> + ret = get_clk(pdev, &mdp5_kms->vsync_clk, "vsync", true);
> + if (ret)
> + return ret;
> +
> + /* optional clocks: */
> + get_clk(pdev, &mdp5_kms->lut_clk, "lut", false);
> + get_clk(pdev, &mdp5_kms->tbu_clk, "tbu", false);
> + get_clk(pdev, &mdp5_kms->tbu_rt_clk, "tbu_rt", false);
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return dev_err_probe(&pdev->dev, irq, "failed to get irq\n");
> +
> + mdp5_kms->base.base.irq = irq;
> +
> + return msm_drv_probe(&pdev->dev, mdp5_kms_init, &mdp5_kms->base.base);
> }
>
> -static int mdp5_dev_remove(struct platform_device *pdev)
> +static void mdp5_dev_remove(struct platform_device *pdev)
> {
> DBG("");
> component_master_del(&pdev->dev, &msm_drm_ops);
> @@@ -986,8 -975,8 +974,8 @@@ MODULE_DEVICE_TABLE(of, mdp5_dt_match)
>
> static struct platform_driver mdp5_driver = {
> .probe = mdp5_dev_probe,
> - .remove = mdp5_dev_remove,
> + .remove_new = mdp5_dev_remove,
> - .shutdown = msm_drv_shutdown,
> + .shutdown = msm_kms_shutdown,
> .driver = {
> .name = "msm_mdp",
> .of_match_table = mdp5_dt_match,
> diff --cc drivers/gpu/drm/msm/msm_drv.c
> index 05fe32c3a4b4,401e9ef86074..000000000000
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@@ -457,23 -264,12 +264,14 @@@ static int msm_drm_init(struct device *
> if (ret)
> goto err_deinit_vram;
>
> - /* the fw fb could be anywhere in memory */
> - ret = drm_aperture_remove_framebuffers(drv);
> - if (ret)
> - goto err_msm_uninit;
> -
> - msm_gem_shrinker_init(ddev);
> + ret = msm_gem_shrinker_init(ddev);
> + if (ret)
> + goto err_msm_uninit;
>
> if (priv->kms_init) {
> - ret = priv->kms_init(ddev);
> - if (ret) {
> - DRM_DEV_ERROR(dev, "failed to load kms\n");
> - priv->kms = NULL;
> + ret = msm_drm_kms_init(dev, drv);
> + if (ret)
> goto err_msm_uninit;
> - }
> - kms = priv->kms;
> } else {
> /* valid only for the dummy headless case, where of_node=NULL */
> WARN_ON(dev->of_node);
> @@@ -1277,37 -971,21 +973,19 @@@ int msm_drv_probe(struct device *master
>
> static int msm_pdev_probe(struct platform_device *pdev)
> {
> - return msm_drv_probe(&pdev->dev, NULL);
> + return msm_drv_probe(&pdev->dev, NULL, NULL);
> }
>
> -static int msm_pdev_remove(struct platform_device *pdev)
> +static void msm_pdev_remove(struct platform_device *pdev)
> {
> component_master_del(&pdev->dev, &msm_drm_ops);
> -
> - return 0;
> }
>
> - 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;
> -
> - /*
> - * Shutdown the hw if we're far enough along where things might be on.
> - * If we run this too early, we'll end up panicking in any variety of
> - * places. Since we don't register the drm device until late in
> - * msm_drm_init, drm_dev->registered is used as an indicator that the
> - * shutdown will be successful.
> - */
> - if (drm && drm->registered && priv->kms)
> - drm_atomic_helper_shutdown(drm);
> - }
> -
> static struct platform_driver msm_platform_driver = {
> .probe = msm_pdev_probe,
> - .remove = msm_pdev_remove,
> + .remove_new = msm_pdev_remove,
> - .shutdown = msm_drv_shutdown,
> .driver = {
> .name = "msm",
> - .pm = &msm_pm_ops,
> },
> };
>

2023-10-11 08:26:22

by Daniel Vetter

[permalink] [raw]
Subject: Re: linux-next: manual merge of the drm-msm tree with the mm, drm trees

On Wed, 11 Oct 2023 at 05:38, Rob Clark <[email protected]> wrote:
>
> Hey Dave,
>
> lmk how you want me to handle this to make it easier for you when I
> send my pull request for 6.7.. I can merge drm-next to take care of
> *that* conflict (either before I send my PR or push it somewhere where
> you can see the resolution) but not sure about the mm conflict since
> pulling that might get me ahead of drm-next. Either way, Stephen's
> resolution looks correct.

The -mm one will get resolved in the merge window by Linus (unless
Andrew wants to do a topic branch, but I don't think that's needed).
The drm one I think we can sort out on the next drm-msm-next pull or
so.
-Sima

>
> BR,
> -R
>
> On Mon, Oct 9, 2023 at 6:33 PM Stephen Rothwell <[email protected]> wrote:
> >
> > Hi all,
> >
> > FIXME: Add owner of second tree to To:
> > Add author(s)/SOB of conflicting commits.
> >
> > Today's linux-next merge of the drm-msm tree got conflicts in:
> >
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > drivers/gpu/drm/msm/msm_drv.c
> >
> > between commits:
> >
> > 01790d5e372f ("drm/msm: Convert to platform remove callback returning void")
> > cd61a76c210a ("drm/msm: dynamically allocate the drm-msm_gem shrinker")
> >
> > from the mm, drm trees and commits:
> >
> > 283add3e6405 ("drm/msm: remove shutdown callback from msm_platform_driver")
> > 506efcba3129 ("drm/msm: carve out KMS code from msm_drv.c")
> >
> > from the drm-msm tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary. This
> > is now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your tree
> > is submitted for merging. You may also want to consider cooperating
> > with the maintainer of the conflicting tree to minimise any particularly
> > complex conflicts.
> >
> > --
> > Cheers,
> > Stephen Rothwell
> >
> > diff --cc drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index 82381d12414d,d14ae316796c..000000000000
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@@ -1299,12 -1230,72 +1230,70 @@@ static int dpu_kms_init(struct drm_devi
> >
> > static int dpu_dev_probe(struct platform_device *pdev)
> > {
> > - return msm_drv_probe(&pdev->dev, dpu_kms_init);
> > + struct device *dev = &pdev->dev;
> > + struct dpu_kms *dpu_kms;
> > + int irq;
> > + int ret = 0;
> > +
> > + dpu_kms = devm_kzalloc(dev, sizeof(*dpu_kms), GFP_KERNEL);
> > + if (!dpu_kms)
> > + return -ENOMEM;
> > +
> > + dpu_kms->pdev = pdev;
> > +
> > + ret = devm_pm_opp_set_clkname(dev, "core");
> > + if (ret)
> > + return ret;
> > + /* OPP table is optional */
> > + ret = devm_pm_opp_of_add_table(dev);
> > + if (ret && ret != -ENODEV)
> > + return dev_err_probe(dev, ret, "invalid OPP table in device tree\n");
> > +
> > + ret = devm_clk_bulk_get_all(&pdev->dev, &dpu_kms->clocks);
> > + if (ret < 0)
> > + return dev_err_probe(dev, ret, "failed to parse clocks\n");
> > +
> > + dpu_kms->num_clocks = ret;
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0)
> > + return dev_err_probe(dev, irq, "failed to get irq\n");
> > +
> > + dpu_kms->base.irq = irq;
> > +
> > + dpu_kms->mmio = msm_ioremap(pdev, "mdp");
> > + if (IS_ERR(dpu_kms->mmio)) {
> > + ret = PTR_ERR(dpu_kms->mmio);
> > + DPU_ERROR("mdp register memory map failed: %d\n", ret);
> > + dpu_kms->mmio = NULL;
> > + return ret;
> > + }
> > + DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio);
> > +
> > + dpu_kms->vbif[VBIF_RT] = msm_ioremap(pdev, "vbif");
> > + if (IS_ERR(dpu_kms->vbif[VBIF_RT])) {
> > + ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]);
> > + DPU_ERROR("vbif register memory map failed: %d\n", ret);
> > + dpu_kms->vbif[VBIF_RT] = NULL;
> > + return ret;
> > + }
> > +
> > + dpu_kms->vbif[VBIF_NRT] = msm_ioremap_quiet(pdev, "vbif_nrt");
> > + if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) {
> > + dpu_kms->vbif[VBIF_NRT] = NULL;
> > + DPU_DEBUG("VBIF NRT is not defined");
> > + }
> > +
> > + ret = dpu_kms_parse_data_bus_icc_path(dpu_kms);
> > + if (ret)
> > + return ret;
> > +
> > + return msm_drv_probe(&pdev->dev, dpu_kms_init, &dpu_kms->base);
> > }
> >
> > -static int dpu_dev_remove(struct platform_device *pdev)
> > +static void dpu_dev_remove(struct platform_device *pdev)
> > {
> > component_master_del(&pdev->dev, &msm_drm_ops);
> > -
> > - return 0;
> > }
> >
> > static int __maybe_unused dpu_runtime_suspend(struct device *dev)
> > @@@ -1380,8 -1371,8 +1369,8 @@@ MODULE_DEVICE_TABLE(of, dpu_dt_match)
> >
> > static struct platform_driver dpu_driver = {
> > .probe = dpu_dev_probe,
> > - .remove = dpu_dev_remove,
> > + .remove_new = dpu_dev_remove,
> > - .shutdown = msm_drv_shutdown,
> > + .shutdown = msm_kms_shutdown,
> > .driver = {
> > .name = "msm_dpu",
> > .of_match_table = dpu_dt_match,
> > diff --cc drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> > index e5012fa6771f,982b7689e5b6..000000000000
> > --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> > @@@ -557,12 -507,60 +507,58 @@@ static const struct dev_pm_ops mdp4_pm_
> >
> > static int mdp4_probe(struct platform_device *pdev)
> > {
> > - return msm_drv_probe(&pdev->dev, mdp4_kms_init);
> > + struct device *dev = &pdev->dev;
> > + struct mdp4_kms *mdp4_kms;
> > + int irq;
> > +
> > + mdp4_kms = devm_kzalloc(dev, sizeof(*mdp4_kms), GFP_KERNEL);
> > + if (!mdp4_kms)
> > + return dev_err_probe(dev, -ENOMEM, "failed to allocate kms\n");
> > +
> > + mdp4_kms->mmio = msm_ioremap(pdev, NULL);
> > + if (IS_ERR(mdp4_kms->mmio))
> > + return PTR_ERR(mdp4_kms->mmio);
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0)
> > + return dev_err_probe(dev, irq, "failed to get irq\n");
> > +
> > + mdp4_kms->base.base.irq = irq;
> > +
> > + /* NOTE: driver for this regulator still missing upstream.. use
> > + * _get_exclusive() and ignore the error if it does not exist
> > + * (and hope that the bootloader left it on for us)
> > + */
> > + mdp4_kms->vdd = devm_regulator_get_exclusive(&pdev->dev, "vdd");
> > + if (IS_ERR(mdp4_kms->vdd))
> > + mdp4_kms->vdd = NULL;
> > +
> > + mdp4_kms->clk = devm_clk_get(&pdev->dev, "core_clk");
> > + if (IS_ERR(mdp4_kms->clk))
> > + return dev_err_probe(dev, PTR_ERR(mdp4_kms->clk), "failed to get core_clk\n");
> > +
> > + mdp4_kms->pclk = devm_clk_get(&pdev->dev, "iface_clk");
> > + if (IS_ERR(mdp4_kms->pclk))
> > + mdp4_kms->pclk = NULL;
> > +
> > + mdp4_kms->axi_clk = devm_clk_get(&pdev->dev, "bus_clk");
> > + if (IS_ERR(mdp4_kms->axi_clk))
> > + return dev_err_probe(dev, PTR_ERR(mdp4_kms->axi_clk), "failed to get axi_clk\n");
> > +
> > + /*
> > + * This is required for revn >= 2. Handle errors here and let the kms
> > + * init bail out if the clock is not provided.
> > + */
> > + mdp4_kms->lut_clk = devm_clk_get_optional(&pdev->dev, "lut_clk");
> > + if (IS_ERR(mdp4_kms->lut_clk))
> > + return dev_err_probe(dev, PTR_ERR(mdp4_kms->lut_clk), "failed to get lut_clk\n");
> > +
> > + return msm_drv_probe(&pdev->dev, mdp4_kms_init, &mdp4_kms->base.base);
> > }
> >
> > -static int mdp4_remove(struct platform_device *pdev)
> > +static void 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[] = {
> > @@@ -573,8 -571,8 +569,8 @@@ MODULE_DEVICE_TABLE(of, mdp4_dt_match)
> >
> > static struct platform_driver mdp4_platform_driver = {
> > .probe = mdp4_probe,
> > - .remove = mdp4_remove,
> > + .remove_new = mdp4_remove,
> > - .shutdown = msm_drv_shutdown,
> > + .shutdown = msm_kms_shutdown,
> > .driver = {
> > .name = "mdp4",
> > .of_match_table = mdp4_dt_match,
> > diff --cc drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > index 8a7b44376bc6,a28fbcd09684..000000000000
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > @@@ -939,10 -894,43 +894,43 @@@ static int mdp5_dev_probe(struct platfo
> > if (ret)
> > return ret;
> >
> > - return msm_drv_probe(&pdev->dev, mdp5_kms_init);
> > + mdp5_kms->pdev = pdev;
> > +
> > + spin_lock_init(&mdp5_kms->resource_lock);
> > +
> > + mdp5_kms->mmio = msm_ioremap(pdev, "mdp_phys");
> > + if (IS_ERR(mdp5_kms->mmio))
> > + return PTR_ERR(mdp5_kms->mmio);
> > +
> > + /* mandatory clocks: */
> > + ret = get_clk(pdev, &mdp5_kms->axi_clk, "bus", true);
> > + if (ret)
> > + return ret;
> > + ret = get_clk(pdev, &mdp5_kms->ahb_clk, "iface", true);
> > + if (ret)
> > + return ret;
> > + ret = get_clk(pdev, &mdp5_kms->core_clk, "core", true);
> > + if (ret)
> > + return ret;
> > + ret = get_clk(pdev, &mdp5_kms->vsync_clk, "vsync", true);
> > + if (ret)
> > + return ret;
> > +
> > + /* optional clocks: */
> > + get_clk(pdev, &mdp5_kms->lut_clk, "lut", false);
> > + get_clk(pdev, &mdp5_kms->tbu_clk, "tbu", false);
> > + get_clk(pdev, &mdp5_kms->tbu_rt_clk, "tbu_rt", false);
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0)
> > + return dev_err_probe(&pdev->dev, irq, "failed to get irq\n");
> > +
> > + mdp5_kms->base.base.irq = irq;
> > +
> > + return msm_drv_probe(&pdev->dev, mdp5_kms_init, &mdp5_kms->base.base);
> > }
> >
> > -static int mdp5_dev_remove(struct platform_device *pdev)
> > +static void mdp5_dev_remove(struct platform_device *pdev)
> > {
> > DBG("");
> > component_master_del(&pdev->dev, &msm_drm_ops);
> > @@@ -986,8 -975,8 +974,8 @@@ MODULE_DEVICE_TABLE(of, mdp5_dt_match)
> >
> > static struct platform_driver mdp5_driver = {
> > .probe = mdp5_dev_probe,
> > - .remove = mdp5_dev_remove,
> > + .remove_new = mdp5_dev_remove,
> > - .shutdown = msm_drv_shutdown,
> > + .shutdown = msm_kms_shutdown,
> > .driver = {
> > .name = "msm_mdp",
> > .of_match_table = mdp5_dt_match,
> > diff --cc drivers/gpu/drm/msm/msm_drv.c
> > index 05fe32c3a4b4,401e9ef86074..000000000000
> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > @@@ -457,23 -264,12 +264,14 @@@ static int msm_drm_init(struct device *
> > if (ret)
> > goto err_deinit_vram;
> >
> > - /* the fw fb could be anywhere in memory */
> > - ret = drm_aperture_remove_framebuffers(drv);
> > - if (ret)
> > - goto err_msm_uninit;
> > -
> > - msm_gem_shrinker_init(ddev);
> > + ret = msm_gem_shrinker_init(ddev);
> > + if (ret)
> > + goto err_msm_uninit;
> >
> > if (priv->kms_init) {
> > - ret = priv->kms_init(ddev);
> > - if (ret) {
> > - DRM_DEV_ERROR(dev, "failed to load kms\n");
> > - priv->kms = NULL;
> > + ret = msm_drm_kms_init(dev, drv);
> > + if (ret)
> > goto err_msm_uninit;
> > - }
> > - kms = priv->kms;
> > } else {
> > /* valid only for the dummy headless case, where of_node=NULL */
> > WARN_ON(dev->of_node);
> > @@@ -1277,37 -971,21 +973,19 @@@ int msm_drv_probe(struct device *master
> >
> > static int msm_pdev_probe(struct platform_device *pdev)
> > {
> > - return msm_drv_probe(&pdev->dev, NULL);
> > + return msm_drv_probe(&pdev->dev, NULL, NULL);
> > }
> >
> > -static int msm_pdev_remove(struct platform_device *pdev)
> > +static void msm_pdev_remove(struct platform_device *pdev)
> > {
> > component_master_del(&pdev->dev, &msm_drm_ops);
> > -
> > - return 0;
> > }
> >
> > - 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;
> > -
> > - /*
> > - * Shutdown the hw if we're far enough along where things might be on.
> > - * If we run this too early, we'll end up panicking in any variety of
> > - * places. Since we don't register the drm device until late in
> > - * msm_drm_init, drm_dev->registered is used as an indicator that the
> > - * shutdown will be successful.
> > - */
> > - if (drm && drm->registered && priv->kms)
> > - drm_atomic_helper_shutdown(drm);
> > - }
> > -
> > static struct platform_driver msm_platform_driver = {
> > .probe = msm_pdev_probe,
> > - .remove = msm_pdev_remove,
> > + .remove_new = msm_pdev_remove,
> > - .shutdown = msm_drv_shutdown,
> > .driver = {
> > .name = "msm",
> > - .pm = &msm_pm_ops,
> > },
> > };
> >



--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch