| Message ID | 20190701173907.15494-1-jeffrey.l.hugo@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | drm/msm/mdp5: Use drm_device for creating gem address space | expand |
On Mon, Jul 1, 2019 at 10:39 AM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote: > > Creating the msm gem address space requires a reference to the dev where > the iommu is located. The driver currently assumes this is the same as > the platform device, which breaks when the iommu is outside of the > platform device. Use the drm_device instead, which happens to always have > a reference to the proper device. > > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> > --- > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > index 4a60f5fca6b0..1347a5223918 100644 > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > @@ -702,7 +702,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev) > mdelay(16); > > if (config->platform.iommu) { > - aspace = msm_gem_address_space_create(&pdev->dev, > + aspace = msm_gem_address_space_create(dev->dev, > config->platform.iommu, "mdp5"); hmm, do you have a tree somewhere with your dt files? This makes me think the display iommu is hooked up somewhere differently compared to, say, msm8916.dtsi BR, -R > if (IS_ERR(aspace)) { > ret = PTR_ERR(aspace); > -- > 2.17.1 >
On Mon, Jul 1, 2019 at 1:45 PM Rob Clark <robdclark@gmail.com> wrote: > > On Mon, Jul 1, 2019 at 10:39 AM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote: > > > > Creating the msm gem address space requires a reference to the dev where > > the iommu is located. The driver currently assumes this is the same as > > the platform device, which breaks when the iommu is outside of the > > platform device. Use the drm_device instead, which happens to always have > > a reference to the proper device. > > > > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> > > --- > > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > index 4a60f5fca6b0..1347a5223918 100644 > > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > @@ -702,7 +702,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev) > > mdelay(16); > > > > if (config->platform.iommu) { > > - aspace = msm_gem_address_space_create(&pdev->dev, > > + aspace = msm_gem_address_space_create(dev->dev, > > config->platform.iommu, "mdp5"); > > hmm, do you have a tree somewhere with your dt files? This makes me > think the display iommu is hooked up somewhere differently compared > to, say, msm8916.dtsi I'll post something somewhere and forward it to you.
On Mon 01 Jul 10:39 PDT 2019, Jeffrey Hugo wrote: > Creating the msm gem address space requires a reference to the dev where > the iommu is located. The driver currently assumes this is the same as > the platform device, which breaks when the iommu is outside of the > platform device. Use the drm_device instead, which happens to always have > a reference to the proper device. > > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> Sorry, but on db820c this patch results in: [ 64.803263] msm_mdp 901000.mdp: [drm:mdp5_kms_init [msm]] *ERROR* failed to attach iommu: -19 Followed by 3 oopses as we're trying to fail the initialization. Regards, Bjorn > --- > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > index 4a60f5fca6b0..1347a5223918 100644 > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > @@ -702,7 +702,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev) > mdelay(16); > > if (config->platform.iommu) { > - aspace = msm_gem_address_space_create(&pdev->dev, > + aspace = msm_gem_address_space_create(dev->dev, > config->platform.iommu, "mdp5"); > if (IS_ERR(aspace)) { > ret = PTR_ERR(aspace); > -- > 2.17.1 >
On Tue, Jul 2, 2019 at 9:08 PM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Mon 01 Jul 10:39 PDT 2019, Jeffrey Hugo wrote: > > > Creating the msm gem address space requires a reference to the dev where > > the iommu is located. The driver currently assumes this is the same as > > the platform device, which breaks when the iommu is outside of the > > platform device. Use the drm_device instead, which happens to always have > > a reference to the proper device. > > > > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> > > Sorry, but on db820c this patch results in: > > [ 64.803263] msm_mdp 901000.mdp: [drm:mdp5_kms_init [msm]] *ERROR* failed to attach iommu: -19 > > Followed by 3 oopses as we're trying to fail the initialization. yeah, that is kinda what I suspected would happen. I guess to deal with how things are hooked up on 8998, perhaps the best thing is to first try &pdev->dev, and then if that fails try dev->dev BR, -R > Regards, > Bjorn > > > --- > > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > index 4a60f5fca6b0..1347a5223918 100644 > > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > @@ -702,7 +702,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev) > > mdelay(16); > > > > if (config->platform.iommu) { > > - aspace = msm_gem_address_space_create(&pdev->dev, > > + aspace = msm_gem_address_space_create(dev->dev, > > config->platform.iommu, "mdp5"); > > if (IS_ERR(aspace)) { > > ret = PTR_ERR(aspace); > > -- > > 2.17.1 > >
On Wed, Jul 3, 2019 at 6:25 AM Rob Clark <robdclark@gmail.com> wrote: > > On Tue, Jul 2, 2019 at 9:08 PM Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > On Mon 01 Jul 10:39 PDT 2019, Jeffrey Hugo wrote: > > > > > Creating the msm gem address space requires a reference to the dev where > > > the iommu is located. The driver currently assumes this is the same as > > > the platform device, which breaks when the iommu is outside of the > > > platform device. Use the drm_device instead, which happens to always have > > > a reference to the proper device. > > > > > > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> > > > > Sorry, but on db820c this patch results in: > > > > [ 64.803263] msm_mdp 901000.mdp: [drm:mdp5_kms_init [msm]] *ERROR* failed to attach iommu: -19 > > > > Followed by 3 oopses as we're trying to fail the initialization. > > yeah, that is kinda what I suspected would happen. I guess to deal > with how things are hooked up on 8998, perhaps the best thing is to > first try &pdev->dev, and then if that fails try dev->dev Thanks for the test feedback Bjorn. Its unfortunate this solution didn't work as I expected. I'll give Rob's suggestion a shot and spin another version.
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c index 4a60f5fca6b0..1347a5223918 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c @@ -702,7 +702,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev) mdelay(16); if (config->platform.iommu) { - aspace = msm_gem_address_space_create(&pdev->dev, + aspace = msm_gem_address_space_create(dev->dev, config->platform.iommu, "mdp5"); if (IS_ERR(aspace)) { ret = PTR_ERR(aspace);
Creating the msm gem address space requires a reference to the dev where the iommu is located. The driver currently assumes this is the same as the platform device, which breaks when the iommu is outside of the platform device. Use the drm_device instead, which happens to always have a reference to the proper device. Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> --- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)