| Message ID | 20220505001605.1268483-2-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm: fixes for KMS iommu handling | expand |
On 5/4/2022 5:16 PM, Dmitry Baryshkov wrote: > Follow the lead of MDP5 driver and check both DPU and MDSS devices for > the IOMMU specifiers. > > Historically DPU devices had IOMMU specified in the MDSS device tree > node, but as some of MDP5 devices are being converted to the supported > by the DPU driver, the driver should adapt and check both devices. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index 143d6643be53..5ccda0766f6c 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -1004,14 +1004,22 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms) > struct msm_mmu *mmu; > struct device *dpu_dev = dpu_kms->dev->dev; > struct device *mdss_dev = dpu_dev->parent; > + struct device *iommu_dev; > > domain = iommu_domain_alloc(&platform_bus_type); > if (!domain) > return 0; > > - /* IOMMUs are a part of MDSS device tree binding, not the > - * MDP/DPU device. */ > - mmu = msm_iommu_new(mdss_dev, domain); > + /* > + * IOMMUs can be a part of MDSS device tree binding, or the > + * MDP/DPU device. > + */ Can you please explain this a little more? So even if some of the mdp5 devices are getting converted to use DPU driver, their device trees are also updated right? In other words, if DPU driver was using mdss_dev to initialize the iommu, why should the new devices which are going to use DPU have the binding in the dpu_dev? > + if (dev_iommu_fwspec_get(dpu_dev)) > + iommu_dev = dpu_dev; > + else > + iommu_dev = mdss_dev; > + > + mmu = msm_iommu_new(iommu_dev, domain); > if (IS_ERR(mmu)) { > iommu_domain_free(domain); > return PTR_ERR(mmu);
On 15/06/2022 20:55, Abhinav Kumar wrote: > > > On 5/4/2022 5:16 PM, Dmitry Baryshkov wrote: >> Follow the lead of MDP5 driver and check both DPU and MDSS devices for >> the IOMMU specifiers. >> >> Historically DPU devices had IOMMU specified in the MDSS device tree >> node, but as some of MDP5 devices are being converted to the supported >> by the DPU driver, the driver should adapt and check both devices. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +++++++++++--- >> 1 file changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> index 143d6643be53..5ccda0766f6c 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> @@ -1004,14 +1004,22 @@ static int _dpu_kms_mmu_init(struct dpu_kms >> *dpu_kms) >> struct msm_mmu *mmu; >> struct device *dpu_dev = dpu_kms->dev->dev; >> struct device *mdss_dev = dpu_dev->parent; >> + struct device *iommu_dev; >> domain = iommu_domain_alloc(&platform_bus_type); >> if (!domain) >> return 0; >> - /* IOMMUs are a part of MDSS device tree binding, not the >> - * MDP/DPU device. */ >> - mmu = msm_iommu_new(mdss_dev, domain); >> + /* >> + * IOMMUs can be a part of MDSS device tree binding, or the >> + * MDP/DPU device. >> + */ > > Can you please explain this a little more? > > So even if some of the mdp5 devices are getting converted to use DPU > driver, their device trees are also updated right? No. The DT describes the device, not the Linux drivers. So while updating the drivers we should not change the DT. > > In other words, if DPU driver was using mdss_dev to initialize the > iommu, why should the new devices which are going to use DPU have the > binding in the dpu_dev? > > >> + if (dev_iommu_fwspec_get(dpu_dev)) >> + iommu_dev = dpu_dev; >> + else >> + iommu_dev = mdss_dev; >> + >> + mmu = msm_iommu_new(iommu_dev, domain); >> if (IS_ERR(mmu)) { >> iommu_domain_free(domain); >> return PTR_ERR(mmu);
On 6/15/2022 11:42 AM, Dmitry Baryshkov wrote: > On 15/06/2022 20:55, Abhinav Kumar wrote: >> >> >> On 5/4/2022 5:16 PM, Dmitry Baryshkov wrote: >>> Follow the lead of MDP5 driver and check both DPU and MDSS devices for >>> the IOMMU specifiers. >>> >>> Historically DPU devices had IOMMU specified in the MDSS device tree >>> node, but as some of MDP5 devices are being converted to the supported >>> by the DPU driver, the driver should adapt and check both devices. >>> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +++++++++++--- >>> 1 file changed, 11 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >>> index 143d6643be53..5ccda0766f6c 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >>> @@ -1004,14 +1004,22 @@ static int _dpu_kms_mmu_init(struct dpu_kms >>> *dpu_kms) >>> struct msm_mmu *mmu; >>> struct device *dpu_dev = dpu_kms->dev->dev; >>> struct device *mdss_dev = dpu_dev->parent; >>> + struct device *iommu_dev; >>> domain = iommu_domain_alloc(&platform_bus_type); >>> if (!domain) >>> return 0; >>> - /* IOMMUs are a part of MDSS device tree binding, not the >>> - * MDP/DPU device. */ >>> - mmu = msm_iommu_new(mdss_dev, domain); >>> + /* >>> + * IOMMUs can be a part of MDSS device tree binding, or the >>> + * MDP/DPU device. >>> + */ >> >> Can you please explain this a little more? >> >> So even if some of the mdp5 devices are getting converted to use DPU >> driver, their device trees are also updated right? > > No. The DT describes the device, not the Linux drivers. So while > updating the drivers we should not change the DT. > Alright, agreed Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >> >> In other words, if DPU driver was using mdss_dev to initialize the >> iommu, why should the new devices which are going to use DPU have the >> binding in the dpu_dev? >> >> >>> + if (dev_iommu_fwspec_get(dpu_dev)) >>> + iommu_dev = dpu_dev; >>> + else >>> + iommu_dev = mdss_dev; >>> + >>> + mmu = msm_iommu_new(iommu_dev, domain); >>> if (IS_ERR(mmu)) { >>> iommu_domain_free(domain); >>> return PTR_ERR(mmu); > >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 143d6643be53..5ccda0766f6c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1004,14 +1004,22 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms) struct msm_mmu *mmu; struct device *dpu_dev = dpu_kms->dev->dev; struct device *mdss_dev = dpu_dev->parent; + struct device *iommu_dev; domain = iommu_domain_alloc(&platform_bus_type); if (!domain) return 0; - /* IOMMUs are a part of MDSS device tree binding, not the - * MDP/DPU device. */ - mmu = msm_iommu_new(mdss_dev, domain); + /* + * IOMMUs can be a part of MDSS device tree binding, or the + * MDP/DPU device. + */ + if (dev_iommu_fwspec_get(dpu_dev)) + iommu_dev = dpu_dev; + else + iommu_dev = mdss_dev; + + mmu = msm_iommu_new(iommu_dev, domain); if (IS_ERR(mmu)) { iommu_domain_free(domain); return PTR_ERR(mmu);
Follow the lead of MDP5 driver and check both DPU and MDSS devices for the IOMMU specifiers. Historically DPU devices had IOMMU specified in the MDSS device tree node, but as some of MDP5 devices are being converted to the supported by the DPU driver, the driver should adapt and check both devices. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)