* [PATCH 0/3] drm/msm: drm_gpuvm leftovers @ 2025-07-06 10:50 Dmitry Baryshkov 2025-07-06 10:50 ` [PATCH 1/3] drm/msm/mdp4: stop supporting no-IOMMU configuration Dmitry Baryshkov ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Dmitry Baryshkov @ 2025-07-06 10:50 UTC (permalink / raw) To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Antonino Maniscalco Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel After switching to drm_gpuvm, the IOMMU becomes a requirement even for KMS-only devices (e.g. allocating a buffer for DSI commands now also requires the IOMMU / GPUVM). Disallow non-IOMMU configurations. Note: MDP4 patches were compile-tested only. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> --- Dmitry Baryshkov (3): drm/msm/mdp4: stop supporting no-IOMMU configuration drm/msm: stop supporting no-IOMMU configuration drm/msm/mdp4: use msm_kms_init_vm() instead of duplicating it drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 27 +++++---------------------- drivers/gpu/drm/msm/msm_kms.c | 4 ++-- 2 files changed, 7 insertions(+), 24 deletions(-) --- base-commit: 8290d37ad2b087bbcfe65fa5bcaf260e184b250a change-id: 20250706-msm-no-iommu-c15212a0e7ac Best regards, -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] drm/msm/mdp4: stop supporting no-IOMMU configuration 2025-07-06 10:50 [PATCH 0/3] drm/msm: drm_gpuvm leftovers Dmitry Baryshkov @ 2025-07-06 10:50 ` Dmitry Baryshkov 2025-07-06 10:50 ` [PATCH 2/3] drm/msm: " Dmitry Baryshkov 2025-07-06 10:50 ` [PATCH 3/3] drm/msm/mdp4: use msm_kms_init_vm() instead of duplicating it Dmitry Baryshkov 2 siblings, 0 replies; 9+ messages in thread From: Dmitry Baryshkov @ 2025-07-06 10:50 UTC (permalink / raw) To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Antonino Maniscalco Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel With the switch to GPUVM the msm driver no longer supports the no-IOMMU configurations (even without the actual GPU). Return an error in case we face the lack of the IOMMU for an MDP4 device. Fixes: 111fdd2198e6 ("drm/msm: drm_gpuvm conversion") Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> --- drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c index 0952c7f18abdca4a7e24e5af8a7132456bfec129..88296c41d1a5eb0e16cb6ec4d0475000b6318c4e 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c @@ -463,9 +463,9 @@ static int mdp4_kms_init(struct drm_device *dev) ret = PTR_ERR(mmu); goto fail; } else if (!mmu) { - DRM_DEV_INFO(dev->dev, "no iommu, fallback to phys " - "contig buffers for scanout\n"); - vm = NULL; + DRM_DEV_INFO(dev->dev, "no IOMMU configuration is no longer supported\n"); + ret = -ENODEV; + goto fail; } else { vm = msm_gem_vm_create(dev, mmu, "mdp4", 0x1000, 0x100000000 - 0x1000, -- 2.39.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] drm/msm: stop supporting no-IOMMU configuration 2025-07-06 10:50 [PATCH 0/3] drm/msm: drm_gpuvm leftovers Dmitry Baryshkov 2025-07-06 10:50 ` [PATCH 1/3] drm/msm/mdp4: stop supporting no-IOMMU configuration Dmitry Baryshkov @ 2025-07-06 10:50 ` Dmitry Baryshkov 2025-07-07 11:39 ` Konrad Dybcio 2025-07-06 10:50 ` [PATCH 3/3] drm/msm/mdp4: use msm_kms_init_vm() instead of duplicating it Dmitry Baryshkov 2 siblings, 1 reply; 9+ messages in thread From: Dmitry Baryshkov @ 2025-07-06 10:50 UTC (permalink / raw) To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Antonino Maniscalco Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel With the switch to GPUVM the msm driver no longer supports the no-IOMMU configurations (even without the actual GPU). Return an error in case we face the lack of the IOMMU for an MDP4 device. Fixes: 111fdd2198e6 ("drm/msm: drm_gpuvm conversion") Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> --- drivers/gpu/drm/msm/msm_kms.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c index 6889f1c1e72121dcc735fa460ea04cdab11c6705..2e2ab93b0f6f6a462e99d54b33da6dc21b1e8435 100644 --- a/drivers/gpu/drm/msm/msm_kms.c +++ b/drivers/gpu/drm/msm/msm_kms.c @@ -201,8 +201,8 @@ struct drm_gpuvm *msm_kms_init_vm(struct drm_device *dev) return ERR_CAST(mmu); if (!mmu) { - drm_info(dev, "no IOMMU, fallback to phys contig buffers for scanout\n"); - return NULL; + drm_info(dev, "no IOMMU configuration is no longer supported\n"); + return ERR_PTR(-ENODEV); } vm = msm_gem_vm_create(dev, mmu, "mdp_kms", -- 2.39.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] drm/msm: stop supporting no-IOMMU configuration 2025-07-06 10:50 ` [PATCH 2/3] drm/msm: " Dmitry Baryshkov @ 2025-07-07 11:39 ` Konrad Dybcio 0 siblings, 0 replies; 9+ messages in thread From: Konrad Dybcio @ 2025-07-07 11:39 UTC (permalink / raw) To: Dmitry Baryshkov, Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Antonino Maniscalco Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel On 7/6/25 12:50 PM, Dmitry Baryshkov wrote: > With the switch to GPUVM the msm driver no longer supports the no-IOMMU > configurations (even without the actual GPU). Return an error in case we > face the lack of the IOMMU for an MDP4 device. > > Fixes: 111fdd2198e6 ("drm/msm: drm_gpuvm conversion") > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > --- > drivers/gpu/drm/msm/msm_kms.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c > index 6889f1c1e72121dcc735fa460ea04cdab11c6705..2e2ab93b0f6f6a462e99d54b33da6dc21b1e8435 100644 > --- a/drivers/gpu/drm/msm/msm_kms.c > +++ b/drivers/gpu/drm/msm/msm_kms.c > @@ -201,8 +201,8 @@ struct drm_gpuvm *msm_kms_init_vm(struct drm_device *dev) > return ERR_CAST(mmu); > > if (!mmu) { > - drm_info(dev, "no IOMMU, fallback to phys contig buffers for scanout\n"); > - return NULL; > + drm_info(dev, "no IOMMU configuration is no longer supported\n"); "Couldn't IOMMU-map buffers, bailing out"? I don't think we need to assume the user has knowledge of the driver history Konrad ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] drm/msm/mdp4: use msm_kms_init_vm() instead of duplicating it 2025-07-06 10:50 [PATCH 0/3] drm/msm: drm_gpuvm leftovers Dmitry Baryshkov 2025-07-06 10:50 ` [PATCH 1/3] drm/msm/mdp4: stop supporting no-IOMMU configuration Dmitry Baryshkov 2025-07-06 10:50 ` [PATCH 2/3] drm/msm: " Dmitry Baryshkov @ 2025-07-06 10:50 ` Dmitry Baryshkov 2025-07-06 13:11 ` Rob Clark 2 siblings, 1 reply; 9+ messages in thread From: Dmitry Baryshkov @ 2025-07-06 10:50 UTC (permalink / raw) To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Antonino Maniscalco Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel Use the msm_kms_init_vm() function to allocate memory manager instead of hand-coding a copy of it. Although MDP4 platforms don't have MDSS device, it's still safe to use the function as all MDP4 devices have IOMMU and the parent of the MDP4 is the root SoC device. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> --- drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c index 88296c41d1a5eb0e16cb6ec4d0475000b6318c4e..41d236d30e71ebb6ac8a59052529f36fadf15cd7 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c @@ -391,11 +391,9 @@ static void read_mdp_hw_revision(struct mdp4_kms *mdp4_kms, static int mdp4_kms_init(struct drm_device *dev) { - struct platform_device *pdev = to_platform_device(dev->dev); struct msm_drm_private *priv = dev->dev_private; struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(priv->kms)); struct msm_kms *kms = NULL; - struct msm_mmu *mmu; struct drm_gpuvm *vm; int ret; u32 major, minor; @@ -458,29 +456,14 @@ static int mdp4_kms_init(struct drm_device *dev) mdp4_disable(mdp4_kms); mdelay(16); - mmu = msm_iommu_new(&pdev->dev, 0); - if (IS_ERR(mmu)) { - ret = PTR_ERR(mmu); + vm = msm_kms_init_vm(mdp4_kms->dev); + if (IS_ERR(vm)) { + ret = PTR_ERR(vm); goto fail; - } else if (!mmu) { - DRM_DEV_INFO(dev->dev, "no IOMMU configuration is no longer supported\n"); - ret = -ENODEV; - goto fail; - } else { - vm = msm_gem_vm_create(dev, mmu, "mdp4", - 0x1000, 0x100000000 - 0x1000, - true); - - if (IS_ERR(vm)) { - if (!IS_ERR(mmu)) - mmu->funcs->destroy(mmu); - ret = PTR_ERR(vm); - goto fail; - } - - kms->vm = vm; } + kms->vm = vm; + ret = modeset_init(mdp4_kms); if (ret) { DRM_DEV_ERROR(dev->dev, "modeset_init failed: %d\n", ret); -- 2.39.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] drm/msm/mdp4: use msm_kms_init_vm() instead of duplicating it 2025-07-06 10:50 ` [PATCH 3/3] drm/msm/mdp4: use msm_kms_init_vm() instead of duplicating it Dmitry Baryshkov @ 2025-07-06 13:11 ` Rob Clark 2025-07-06 14:02 ` Dmitry Baryshkov 0 siblings, 1 reply; 9+ messages in thread From: Rob Clark @ 2025-07-06 13:11 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Antonino Maniscalco, linux-arm-msm, dri-devel, freedreno, linux-kernel On Sun, Jul 6, 2025 at 3:50 AM Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote: > > Use the msm_kms_init_vm() function to allocate memory manager instead of > hand-coding a copy of it. Although MDP4 platforms don't have MDSS > device, it's still safe to use the function as all MDP4 devices have > IOMMU and the parent of the MDP4 is the root SoC device. So, originally the distinction was that mdp4 didn't have the mdss wrapper. Maybe it works out because device_iommu_mapped(mdp_dev) returns true? BR, -R > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > --- > drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 27 +++++---------------------- > 1 file changed, 5 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c > index 88296c41d1a5eb0e16cb6ec4d0475000b6318c4e..41d236d30e71ebb6ac8a59052529f36fadf15cd7 100644 > --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c > +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c > @@ -391,11 +391,9 @@ static void read_mdp_hw_revision(struct mdp4_kms *mdp4_kms, > > static int mdp4_kms_init(struct drm_device *dev) > { > - struct platform_device *pdev = to_platform_device(dev->dev); > struct msm_drm_private *priv = dev->dev_private; > struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(priv->kms)); > struct msm_kms *kms = NULL; > - struct msm_mmu *mmu; > struct drm_gpuvm *vm; > int ret; > u32 major, minor; > @@ -458,29 +456,14 @@ static int mdp4_kms_init(struct drm_device *dev) > mdp4_disable(mdp4_kms); > mdelay(16); > > - mmu = msm_iommu_new(&pdev->dev, 0); > - if (IS_ERR(mmu)) { > - ret = PTR_ERR(mmu); > + vm = msm_kms_init_vm(mdp4_kms->dev); > + if (IS_ERR(vm)) { > + ret = PTR_ERR(vm); > goto fail; > - } else if (!mmu) { > - DRM_DEV_INFO(dev->dev, "no IOMMU configuration is no longer supported\n"); > - ret = -ENODEV; > - goto fail; > - } else { > - vm = msm_gem_vm_create(dev, mmu, "mdp4", > - 0x1000, 0x100000000 - 0x1000, > - true); > - > - if (IS_ERR(vm)) { > - if (!IS_ERR(mmu)) > - mmu->funcs->destroy(mmu); > - ret = PTR_ERR(vm); > - goto fail; > - } > - > - kms->vm = vm; > } > > + kms->vm = vm; > + > ret = modeset_init(mdp4_kms); > if (ret) { > DRM_DEV_ERROR(dev->dev, "modeset_init failed: %d\n", ret); > > -- > 2.39.5 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] drm/msm/mdp4: use msm_kms_init_vm() instead of duplicating it 2025-07-06 13:11 ` Rob Clark @ 2025-07-06 14:02 ` Dmitry Baryshkov 2025-07-06 15:11 ` Rob Clark 0 siblings, 1 reply; 9+ messages in thread From: Dmitry Baryshkov @ 2025-07-06 14:02 UTC (permalink / raw) To: rob.clark Cc: Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Antonino Maniscalco, linux-arm-msm, dri-devel, freedreno, linux-kernel On Sun, 6 Jul 2025 at 16:11, Rob Clark <rob.clark@oss.qualcomm.com> wrote: > > On Sun, Jul 6, 2025 at 3:50 AM Dmitry Baryshkov > <dmitry.baryshkov@oss.qualcomm.com> wrote: > > > > Use the msm_kms_init_vm() function to allocate memory manager instead of > > hand-coding a copy of it. Although MDP4 platforms don't have MDSS > > device, it's still safe to use the function as all MDP4 devices have > > IOMMU and the parent of the MDP4 is the root SoC device. > > So, originally the distinction was that mdp4 didn't have the mdss > wrapper. Maybe it works out because device_iommu_mapped(mdp_dev) > returns true? Yes, as expressed in the cover letter. > > BR, > -R > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > > --- > > drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 27 +++++---------------------- > > 1 file changed, 5 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c > > index 88296c41d1a5eb0e16cb6ec4d0475000b6318c4e..41d236d30e71ebb6ac8a59052529f36fadf15cd7 100644 > > --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c > > +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c > > @@ -391,11 +391,9 @@ static void read_mdp_hw_revision(struct mdp4_kms *mdp4_kms, > > > > static int mdp4_kms_init(struct drm_device *dev) > > { > > - struct platform_device *pdev = to_platform_device(dev->dev); > > struct msm_drm_private *priv = dev->dev_private; > > struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(priv->kms)); > > struct msm_kms *kms = NULL; > > - struct msm_mmu *mmu; > > struct drm_gpuvm *vm; > > int ret; > > u32 major, minor; > > @@ -458,29 +456,14 @@ static int mdp4_kms_init(struct drm_device *dev) > > mdp4_disable(mdp4_kms); > > mdelay(16); > > > > - mmu = msm_iommu_new(&pdev->dev, 0); > > - if (IS_ERR(mmu)) { > > - ret = PTR_ERR(mmu); > > + vm = msm_kms_init_vm(mdp4_kms->dev); > > + if (IS_ERR(vm)) { > > + ret = PTR_ERR(vm); > > goto fail; > > - } else if (!mmu) { > > - DRM_DEV_INFO(dev->dev, "no IOMMU configuration is no longer supported\n"); > > - ret = -ENODEV; > > - goto fail; > > - } else { > > - vm = msm_gem_vm_create(dev, mmu, "mdp4", > > - 0x1000, 0x100000000 - 0x1000, > > - true); > > - > > - if (IS_ERR(vm)) { > > - if (!IS_ERR(mmu)) > > - mmu->funcs->destroy(mmu); > > - ret = PTR_ERR(vm); > > - goto fail; > > - } > > - > > - kms->vm = vm; > > } > > > > + kms->vm = vm; > > + > > ret = modeset_init(mdp4_kms); > > if (ret) { > > DRM_DEV_ERROR(dev->dev, "modeset_init failed: %d\n", ret); > > > > -- > > 2.39.5 > > -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] drm/msm/mdp4: use msm_kms_init_vm() instead of duplicating it 2025-07-06 14:02 ` Dmitry Baryshkov @ 2025-07-06 15:11 ` Rob Clark 2025-07-06 15:49 ` Dmitry Baryshkov 0 siblings, 1 reply; 9+ messages in thread From: Rob Clark @ 2025-07-06 15:11 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Antonino Maniscalco, linux-arm-msm, dri-devel, freedreno, linux-kernel On Sun, Jul 6, 2025 at 7:02 AM Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote: > > On Sun, 6 Jul 2025 at 16:11, Rob Clark <rob.clark@oss.qualcomm.com> wrote: > > > > On Sun, Jul 6, 2025 at 3:50 AM Dmitry Baryshkov > > <dmitry.baryshkov@oss.qualcomm.com> wrote: > > > > > > Use the msm_kms_init_vm() function to allocate memory manager instead of > > > hand-coding a copy of it. Although MDP4 platforms don't have MDSS > > > device, it's still safe to use the function as all MDP4 devices have > > > IOMMU and the parent of the MDP4 is the root SoC device. > > > > So, originally the distinction was that mdp4 didn't have the mdss > > wrapper. Maybe it works out because device_iommu_mapped(mdp_dev) > > returns true? > > Yes, as expressed in the cover letter. Right, but with this patch, I think nothing is enforcing that, so we could end up dereferencing mdp_dev->parent if the device did not have an iommu. I guess you could solve that with an extra device_iommu_mapped() in mdp4_kms_init() BR, -R > > > > BR, > > -R > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > > > --- > > > drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 27 +++++---------------------- > > > 1 file changed, 5 insertions(+), 22 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c > > > index 88296c41d1a5eb0e16cb6ec4d0475000b6318c4e..41d236d30e71ebb6ac8a59052529f36fadf15cd7 100644 > > > --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c > > > +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c > > > @@ -391,11 +391,9 @@ static void read_mdp_hw_revision(struct mdp4_kms *mdp4_kms, > > > > > > static int mdp4_kms_init(struct drm_device *dev) > > > { > > > - struct platform_device *pdev = to_platform_device(dev->dev); > > > struct msm_drm_private *priv = dev->dev_private; > > > struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(priv->kms)); > > > struct msm_kms *kms = NULL; > > > - struct msm_mmu *mmu; > > > struct drm_gpuvm *vm; > > > int ret; > > > u32 major, minor; > > > @@ -458,29 +456,14 @@ static int mdp4_kms_init(struct drm_device *dev) > > > mdp4_disable(mdp4_kms); > > > mdelay(16); > > > > > > - mmu = msm_iommu_new(&pdev->dev, 0); > > > - if (IS_ERR(mmu)) { > > > - ret = PTR_ERR(mmu); > > > + vm = msm_kms_init_vm(mdp4_kms->dev); > > > + if (IS_ERR(vm)) { > > > + ret = PTR_ERR(vm); > > > goto fail; > > > - } else if (!mmu) { > > > - DRM_DEV_INFO(dev->dev, "no IOMMU configuration is no longer supported\n"); > > > - ret = -ENODEV; > > > - goto fail; > > > - } else { > > > - vm = msm_gem_vm_create(dev, mmu, "mdp4", > > > - 0x1000, 0x100000000 - 0x1000, > > > - true); > > > - > > > - if (IS_ERR(vm)) { > > > - if (!IS_ERR(mmu)) > > > - mmu->funcs->destroy(mmu); > > > - ret = PTR_ERR(vm); > > > - goto fail; > > > - } > > > - > > > - kms->vm = vm; > > > } > > > > > > + kms->vm = vm; > > > + > > > ret = modeset_init(mdp4_kms); > > > if (ret) { > > > DRM_DEV_ERROR(dev->dev, "modeset_init failed: %d\n", ret); > > > > > > -- > > > 2.39.5 > > > > > > > -- > With best wishes > Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] drm/msm/mdp4: use msm_kms_init_vm() instead of duplicating it 2025-07-06 15:11 ` Rob Clark @ 2025-07-06 15:49 ` Dmitry Baryshkov 0 siblings, 0 replies; 9+ messages in thread From: Dmitry Baryshkov @ 2025-07-06 15:49 UTC (permalink / raw) To: rob.clark Cc: Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Antonino Maniscalco, linux-arm-msm, dri-devel, freedreno, linux-kernel On 06/07/2025 18:11, Rob Clark wrote: > On Sun, Jul 6, 2025 at 7:02 AM Dmitry Baryshkov > <dmitry.baryshkov@oss.qualcomm.com> wrote: >> >> On Sun, 6 Jul 2025 at 16:11, Rob Clark <rob.clark@oss.qualcomm.com> wrote: >>> >>> On Sun, Jul 6, 2025 at 3:50 AM Dmitry Baryshkov >>> <dmitry.baryshkov@oss.qualcomm.com> wrote: >>>> >>>> Use the msm_kms_init_vm() function to allocate memory manager instead of >>>> hand-coding a copy of it. Although MDP4 platforms don't have MDSS >>>> device, it's still safe to use the function as all MDP4 devices have >>>> IOMMU and the parent of the MDP4 is the root SoC device. >>> >>> So, originally the distinction was that mdp4 didn't have the mdss >>> wrapper. Maybe it works out because device_iommu_mapped(mdp_dev) >>> returns true? >> >> Yes, as expressed in the cover letter. > > Right, but with this patch, I think nothing is enforcing that, so we > could end up dereferencing mdp_dev->parent if the device did not have > an iommu. > > I guess you could solve that with an extra device_iommu_mapped() in > mdp4_kms_init() ... or adding have_mdss arg to msm_kms_init_vm(). Anyway, let's probably get first two patches in, I'll repost the third patch later on. > > BR, > -R > >>> >>> BR, >>> -R >>> >>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> >>>> --- >>>> drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 27 +++++---------------------- >>>> 1 file changed, 5 insertions(+), 22 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c >>>> index 88296c41d1a5eb0e16cb6ec4d0475000b6318c4e..41d236d30e71ebb6ac8a59052529f36fadf15cd7 100644 >>>> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c >>>> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c >>>> @@ -391,11 +391,9 @@ static void read_mdp_hw_revision(struct mdp4_kms *mdp4_kms, >>>> >>>> static int mdp4_kms_init(struct drm_device *dev) >>>> { >>>> - struct platform_device *pdev = to_platform_device(dev->dev); >>>> struct msm_drm_private *priv = dev->dev_private; >>>> struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(priv->kms)); >>>> struct msm_kms *kms = NULL; >>>> - struct msm_mmu *mmu; >>>> struct drm_gpuvm *vm; >>>> int ret; >>>> u32 major, minor; >>>> @@ -458,29 +456,14 @@ static int mdp4_kms_init(struct drm_device *dev) >>>> mdp4_disable(mdp4_kms); >>>> mdelay(16); >>>> >>>> - mmu = msm_iommu_new(&pdev->dev, 0); >>>> - if (IS_ERR(mmu)) { >>>> - ret = PTR_ERR(mmu); >>>> + vm = msm_kms_init_vm(mdp4_kms->dev); >>>> + if (IS_ERR(vm)) { >>>> + ret = PTR_ERR(vm); >>>> goto fail; >>>> - } else if (!mmu) { >>>> - DRM_DEV_INFO(dev->dev, "no IOMMU configuration is no longer supported\n"); >>>> - ret = -ENODEV; >>>> - goto fail; >>>> - } else { >>>> - vm = msm_gem_vm_create(dev, mmu, "mdp4", >>>> - 0x1000, 0x100000000 - 0x1000, >>>> - true); >>>> - >>>> - if (IS_ERR(vm)) { >>>> - if (!IS_ERR(mmu)) >>>> - mmu->funcs->destroy(mmu); >>>> - ret = PTR_ERR(vm); >>>> - goto fail; >>>> - } >>>> - >>>> - kms->vm = vm; >>>> } >>>> >>>> + kms->vm = vm; >>>> + >>>> ret = modeset_init(mdp4_kms); >>>> if (ret) { >>>> DRM_DEV_ERROR(dev->dev, "modeset_init failed: %d\n", ret); >>>> >>>> -- >>>> 2.39.5 >>>> >> >> >> >> -- >> With best wishes >> Dmitry -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-07-07 11:39 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-06 10:50 [PATCH 0/3] drm/msm: drm_gpuvm leftovers Dmitry Baryshkov 2025-07-06 10:50 ` [PATCH 1/3] drm/msm/mdp4: stop supporting no-IOMMU configuration Dmitry Baryshkov 2025-07-06 10:50 ` [PATCH 2/3] drm/msm: " Dmitry Baryshkov 2025-07-07 11:39 ` Konrad Dybcio 2025-07-06 10:50 ` [PATCH 3/3] drm/msm/mdp4: use msm_kms_init_vm() instead of duplicating it Dmitry Baryshkov 2025-07-06 13:11 ` Rob Clark 2025-07-06 14:02 ` Dmitry Baryshkov 2025-07-06 15:11 ` Rob Clark 2025-07-06 15:49 ` Dmitry Baryshkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).