Re: linux-next: manual merge of the drm-msm tree with the mm, drm trees

[Rob Clark]

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 <sfr@xxxxxxxxxxxxxxxx> 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,
>         },
>   };
>