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, > }, > }; >