drm/msm/mdp5: Use drm_device for creating gem address space
diff mbox series

Message ID 20190701173907.15494-1-jeffrey.l.hugo@gmail.com
State New
Headers show
Series
  • drm/msm/mdp5: Use drm_device for creating gem address space
Related show

Commit Message

Jeffrey Hugo July 1, 2019, 5:39 p.m. UTC
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(-)

Comments

Jeffrey Hugo July 1, 2019, 8:22 p.m. UTC | #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.
Bjorn Andersson July 3, 2019, 4:08 a.m. UTC | #2
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
>
Rob Clark July 3, 2019, 12:25 p.m. UTC | #3
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
> >
Jeffrey Hugo July 3, 2019, 3:09 p.m. UTC | #4
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.

Patch
diff mbox series

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);