[PATCH] drm/msm/mdp5: Use drm_device for creating gem address space

[PATCH] drm/msm/mdp5: Use drm_device for creating gem address space

[Jeffrey Hugo]

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");
 		if (IS_ERR(aspace)) {
 			ret = PTR_ERR(aspace);
-- 
2.17.1

Re: [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space

[Rob Clark]

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
>

Re: [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space

[Jeffrey Hugo]

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.

Re: [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space

[Bjorn Andersson]

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
> 

Re: [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space

[Rob Clark]

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

Re: [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space

[Rob Clark]

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
> >
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space

[Jeffrey Hugo]

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.