linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space
@ 2019-07-01 17:39 Jeffrey Hugo
  2019-07-01 19:45 ` Rob Clark
  2019-07-03  4:08 ` Bjorn Andersson
  0 siblings, 2 replies; 6+ messages in thread
From: Jeffrey Hugo @ 2019-07-01 17:39 UTC (permalink / raw)
  To: robdclark, sean, airlied, daniel
  Cc: bjorn.andersson, linux-arm-msm, dri-devel, freedreno,
	linux-kernel, 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space
  2019-07-01 17:39 [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space Jeffrey Hugo
@ 2019-07-01 19:45 ` Rob Clark
  2019-07-01 20:22   ` Jeffrey Hugo
  2019-07-03  4:08 ` Bjorn Andersson
  1 sibling, 1 reply; 6+ messages in thread
From: Rob Clark @ 2019-07-01 19:45 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: Sean Paul, David Airlie, Daniel Vetter, Bjorn Andersson,
	linux-arm-msm, dri-devel, freedreno, Linux Kernel Mailing List

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space
  2019-07-01 19:45 ` Rob Clark
@ 2019-07-01 20:22   ` Jeffrey Hugo
  0 siblings, 0 replies; 6+ messages in thread
From: Jeffrey Hugo @ 2019-07-01 20:22 UTC (permalink / raw)
  To: Rob Clark
  Cc: Sean Paul, David Airlie, Daniel Vetter, Bjorn Andersson,
	linux-arm-msm, dri-devel, freedreno, Linux Kernel Mailing List

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space
  2019-07-01 17:39 [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space Jeffrey Hugo
  2019-07-01 19:45 ` Rob Clark
@ 2019-07-03  4:08 ` Bjorn Andersson
  2019-07-03 12:25   ` Rob Clark
  1 sibling, 1 reply; 6+ messages in thread
From: Bjorn Andersson @ 2019-07-03  4:08 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: robdclark, sean, airlied, daniel, linux-arm-msm, dri-devel,
	freedreno, linux-kernel

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
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space
  2019-07-03  4:08 ` Bjorn Andersson
@ 2019-07-03 12:25   ` Rob Clark
  2019-07-03 15:09     ` Jeffrey Hugo
  0 siblings, 1 reply; 6+ messages in thread
From: Rob Clark @ 2019-07-03 12:25 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Jeffrey Hugo, Sean Paul, David Airlie, Daniel Vetter,
	linux-arm-msm, dri-devel, freedreno, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space
  2019-07-03 12:25   ` Rob Clark
@ 2019-07-03 15:09     ` Jeffrey Hugo
  0 siblings, 0 replies; 6+ messages in thread
From: Jeffrey Hugo @ 2019-07-03 15:09 UTC (permalink / raw)
  To: Rob Clark
  Cc: Bjorn Andersson, Sean Paul, David Airlie, Daniel Vetter,
	linux-arm-msm, dri-devel, freedreno, Linux Kernel Mailing List

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-07-03 15:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-01 17:39 [PATCH] drm/msm/mdp5: Use drm_device for creating gem address space Jeffrey Hugo
2019-07-01 19:45 ` Rob Clark
2019-07-01 20:22   ` Jeffrey Hugo
2019-07-03  4:08 ` Bjorn Andersson
2019-07-03 12:25   ` Rob Clark
2019-07-03 15:09     ` Jeffrey Hugo

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