[PATCH] drm/msm: log iommu init failure

[PATCH] drm/msm: log iommu init failure

[Marc Gonzalez]

When create_address_space() fails (e.g. when smmu node is disabled)
msm_gpu_init() silently fails:

msm_dpu c901000.display-controller: failed to load adreno gpu
msm_dpu c901000.display-controller: failed to bind 5000000.gpu (ops a3xx_ops): -19

Log create_address_space() failure.

Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
---
 drivers/gpu/drm/msm/msm_gpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 655002b21b0d5..f1e692866cc38 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -941,6 +941,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 		DRM_DEV_INFO(drm->dev, "%s: no IOMMU, fallback to VRAM carveout!\n", name);
 	else if (IS_ERR(gpu->aspace)) {
 		ret = PTR_ERR(gpu->aspace);
+		DRM_DEV_ERROR(drm->dev, "could not create address space: %d\n", ret);
 		goto fail;
 	}
 
-- 
2.34.1

Re: [PATCH] drm/msm: log iommu init failure

[Marijn Suijten]

On 2024-05-15 17:09:02, Marc Gonzalez wrote:
> When create_address_space() fails (e.g. when smmu node is disabled)
> msm_gpu_init() silently fails:
> 
> msm_dpu c901000.display-controller: failed to load adreno gpu
> msm_dpu c901000.display-controller: failed to bind 5000000.gpu (ops a3xx_ops): -19
> 
> Log create_address_space() failure.
> 
> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>

Thanks!

Suggested-by: Marijn Suijten <marijn.suijten@somainline.org>

And, after checking the below:

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>

> ---
>  drivers/gpu/drm/msm/msm_gpu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 655002b21b0d5..f1e692866cc38 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -941,6 +941,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>  		DRM_DEV_INFO(drm->dev, "%s: no IOMMU, fallback to VRAM carveout!\n", name);
>  	else if (IS_ERR(gpu->aspace)) {
>  		ret = PTR_ERR(gpu->aspace);
> +		DRM_DEV_ERROR(drm->dev, "could not create address space: %d\n", ret);

Maybe this wasn't done before because this also includes `-EPROBE_DEFER`, so you
might want to wrap this in

	if (ret != -EPROBE_DEFER)
		DRM_DEV_ERROR...

But then dev_err_probe() was built specifically to be less verbose about this
(and track defer reasons).  While this is an init and not probe function, it's
called from struct component_ops->bind where it should be okay to call that,
as long as you have access to the component `struct device*` and not its master
(IIRC).

- Marijn

>  		goto fail;
>  	}
>  
> -- 
> 2.34.1
> 

Re: [PATCH] drm/msm: log iommu init failure

[Marc Gonzalez]

On 16/05/2024 10:43, Marijn Suijten wrote:

> On 2024-05-15 17:09:02, Marc Gonzalez wrote:
>
>> When create_address_space() fails (e.g. when smmu node is disabled)
>> msm_gpu_init() silently fails:
>>
>> msm_dpu c901000.display-controller: failed to load adreno gpu
>> msm_dpu c901000.display-controller: failed to bind 5000000.gpu (ops a3xx_ops): -19
>>
>> Log create_address_space() failure.
>>
>> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
> 
> Thanks!
> 
> Suggested-by: Marijn Suijten <marijn.suijten@somainline.org>
> 
> And, after checking the below:
> 
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> 
>> ---
>>  drivers/gpu/drm/msm/msm_gpu.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
>> index 655002b21b0d5..f1e692866cc38 100644
>> --- a/drivers/gpu/drm/msm/msm_gpu.c
>> +++ b/drivers/gpu/drm/msm/msm_gpu.c
>> @@ -941,6 +941,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>>  		DRM_DEV_INFO(drm->dev, "%s: no IOMMU, fallback to VRAM carveout!\n", name);
>>  	else if (IS_ERR(gpu->aspace)) {
>>  		ret = PTR_ERR(gpu->aspace);
>> +		DRM_DEV_ERROR(drm->dev, "could not create address space: %d\n", ret);
> 
> Maybe this wasn't done before because this also includes `-EPROBE_DEFER`, so you
> might want to wrap this in
> 
> 	if (ret != -EPROBE_DEFER)
> 		DRM_DEV_ERROR...
> 
> But then dev_err_probe() was built specifically to be less verbose about this
> (and track defer reasons).  While this is an init and not probe function, it's
> called from struct component_ops->bind where it should be okay to call that,
> as long as you have access to the component `struct device*` and not its master
> (IIRC).


Hello Marijn,

I have moved on to HDMI.

Feel free to take full ownership of this submission,
as I won't have the energy to get it accepted.

Regards,

Marc

Re: [PATCH] drm/msm: log iommu init failure

[Rob Clark]

On Thu, May 30, 2024 at 2:48 AM Marc Gonzalez <mgonzalez@freebox.fr> wrote:
>
> On 16/05/2024 10:43, Marijn Suijten wrote:
>
> > On 2024-05-15 17:09:02, Marc Gonzalez wrote:
> >
> >> When create_address_space() fails (e.g. when smmu node is disabled)

Note that smmu support is going to become a hard dependency with the
drm_gpuvm/VM_BIND conversion.. which I think means we should never get
far enough to hit this error path..

BR,
-R

> >> msm_gpu_init() silently fails:
> >>
> >> msm_dpu c901000.display-controller: failed to load adreno gpu
> >> msm_dpu c901000.display-controller: failed to bind 5000000.gpu (ops a3xx_ops): -19
> >>
> >> Log create_address_space() failure.
> >>
> >> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
> >
> > Thanks!
> >
> > Suggested-by: Marijn Suijten <marijn.suijten@somainline.org>
> >
> > And, after checking the below:
> >
> > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> >
> >> ---
> >>  drivers/gpu/drm/msm/msm_gpu.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> >> index 655002b21b0d5..f1e692866cc38 100644
> >> --- a/drivers/gpu/drm/msm/msm_gpu.c
> >> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> >> @@ -941,6 +941,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> >>              DRM_DEV_INFO(drm->dev, "%s: no IOMMU, fallback to VRAM carveout!\n", name);
> >>      else if (IS_ERR(gpu->aspace)) {
> >>              ret = PTR_ERR(gpu->aspace);
> >> +            DRM_DEV_ERROR(drm->dev, "could not create address space: %d\n", ret);
> >
> > Maybe this wasn't done before because this also includes `-EPROBE_DEFER`, so you
> > might want to wrap this in
> >
> >       if (ret != -EPROBE_DEFER)
> >               DRM_DEV_ERROR...
> >
> > But then dev_err_probe() was built specifically to be less verbose about this
> > (and track defer reasons).  While this is an init and not probe function, it's
> > called from struct component_ops->bind where it should be okay to call that,
> > as long as you have access to the component `struct device*` and not its master
> > (IIRC).
>
>
> Hello Marijn,
>
> I have moved on to HDMI.
>
> Feel free to take full ownership of this submission,
> as I won't have the energy to get it accepted.
>
> Regards,
>
> Marc
>

Re: [PATCH] drm/msm: log iommu init failure

[Dmitry Baryshkov]

On Thu, 20 Jun 2024 at 20:32, Rob Clark <robdclark@gmail.com> wrote:
>
> On Thu, May 30, 2024 at 2:48 AM Marc Gonzalez <mgonzalez@freebox.fr> wrote:
> >
> > On 16/05/2024 10:43, Marijn Suijten wrote:
> >
> > > On 2024-05-15 17:09:02, Marc Gonzalez wrote:
> > >
> > >> When create_address_space() fails (e.g. when smmu node is disabled)
>
> Note that smmu support is going to become a hard dependency with the
> drm_gpuvm/VM_BIND conversion.. which I think means we should never get
> far enough to hit this error path..

Does that mean that we will lose GPU support on  MSM8974?

>
> BR,
> -R
>
> > >> msm_gpu_init() silently fails:
> > >>
> > >> msm_dpu c901000.display-controller: failed to load adreno gpu
> > >> msm_dpu c901000.display-controller: failed to bind 5000000.gpu (ops a3xx_ops): -19
> > >>
> > >> Log create_address_space() failure.
> > >>
> > >> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
> > >
> > > Thanks!
> > >
> > > Suggested-by: Marijn Suijten <marijn.suijten@somainline.org>
> > >
> > > And, after checking the below:
> > >
> > > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> > >



-- 
With best wishes
Dmitry

Re: [PATCH] drm/msm: log iommu init failure

[Konrad Dybcio]

On 6/20/24 20:24, Dmitry Baryshkov wrote:
> On Thu, 20 Jun 2024 at 20:32, Rob Clark <robdclark@gmail.com> wrote:
>>
>> On Thu, May 30, 2024 at 2:48 AM Marc Gonzalez <mgonzalez@freebox.fr> wrote:
>>>
>>> On 16/05/2024 10:43, Marijn Suijten wrote:
>>>
>>>> On 2024-05-15 17:09:02, Marc Gonzalez wrote:
>>>>
>>>>> When create_address_space() fails (e.g. when smmu node is disabled)
>>
>> Note that smmu support is going to become a hard dependency with the
>> drm_gpuvm/VM_BIND conversion.. which I think means we should never get
>> far enough to hit this error path..
> 
> Does that mean that we will lose GPU support on  MSM8974?

Yeah, that was brought up on #freedreno some time ago

Konrad

Re: [PATCH] drm/msm: log iommu init failure

[Luca Weiss]

On Fri Jun 21, 2024 at 12:47 AM CEST, Konrad Dybcio wrote:
>
>
> On 6/20/24 20:24, Dmitry Baryshkov wrote:
> > On Thu, 20 Jun 2024 at 20:32, Rob Clark <robdclark@gmail.com> wrote:
> >>
> >> On Thu, May 30, 2024 at 2:48 AM Marc Gonzalez <mgonzalez@freebox.fr> wrote:
> >>>
> >>> On 16/05/2024 10:43, Marijn Suijten wrote:
> >>>
> >>>> On 2024-05-15 17:09:02, Marc Gonzalez wrote:
> >>>>
> >>>>> When create_address_space() fails (e.g. when smmu node is disabled)
> >>
> >> Note that smmu support is going to become a hard dependency with the
> >> drm_gpuvm/VM_BIND conversion.. which I think means we should never get
> >> far enough to hit this error path..
> > 
> > Does that mean that we will lose GPU support on  MSM8974?
>
> Yeah, that was brought up on #freedreno some time ago

Also on MSM8226 which I also care about...

Anyone at all knowledgable on IOMMU would be very welcome to help out
with IOMMU support on these two platforms (and anything else that
old?) in any case, since me and some other people have looked at this
(on and off) for years but haven't gotten to any stable or usable point
unfortunately.

Regards
Luca

>
> Konrad

Re: [PATCH] drm/msm: log iommu init failure

[Rob Clark]

On Thu, Jun 20, 2024 at 11:48 PM Luca Weiss <luca.weiss@fairphone.com> wrote:
>
> On Fri Jun 21, 2024 at 12:47 AM CEST, Konrad Dybcio wrote:
> >
> >
> > On 6/20/24 20:24, Dmitry Baryshkov wrote:
> > > On Thu, 20 Jun 2024 at 20:32, Rob Clark <robdclark@gmail.com> wrote:
> > >>
> > >> On Thu, May 30, 2024 at 2:48 AM Marc Gonzalez <mgonzalez@freebox.fr> wrote:
> > >>>
> > >>> On 16/05/2024 10:43, Marijn Suijten wrote:
> > >>>
> > >>>> On 2024-05-15 17:09:02, Marc Gonzalez wrote:
> > >>>>
> > >>>>> When create_address_space() fails (e.g. when smmu node is disabled)
> > >>
> > >> Note that smmu support is going to become a hard dependency with the
> > >> drm_gpuvm/VM_BIND conversion.. which I think means we should never get
> > >> far enough to hit this error path..
> > >
> > > Does that mean that we will lose GPU support on  MSM8974?

And display support as well :-/

Note that GPU should be disabled by default without smmu.. you can
override with modparam, but please don't.  It is incredibly insecure,
you might as well make /dev/mem world readable/writeable.

Is simplefb an option on 8974 or 8226 to keep display support?

BR,
-R

> >
> > Yeah, that was brought up on #freedreno some time ago
>
> Also on MSM8226 which I also care about...
>
> Anyone at all knowledgable on IOMMU would be very welcome to help out
> with IOMMU support on these two platforms (and anything else that
> old?) in any case, since me and some other people have looked at this
> (on and off) for years but haven't gotten to any stable or usable point
> unfortunately.
>
> Regards
> Luca
>
> >
> > Konrad
>

Re: [PATCH] drm/msm: log iommu init failure

[Dmitry Baryshkov]

On Mon, 24 Jun 2024 at 20:59, Rob Clark <robdclark@gmail.com> wrote:
>
> On Thu, Jun 20, 2024 at 11:48 PM Luca Weiss <luca.weiss@fairphone.com> wrote:
> >
> > On Fri Jun 21, 2024 at 12:47 AM CEST, Konrad Dybcio wrote:
> > >
> > >
> > > On 6/20/24 20:24, Dmitry Baryshkov wrote:
> > > > On Thu, 20 Jun 2024 at 20:32, Rob Clark <robdclark@gmail.com> wrote:
> > > >>
> > > >> On Thu, May 30, 2024 at 2:48 AM Marc Gonzalez <mgonzalez@freebox.fr> wrote:
> > > >>>
> > > >>> On 16/05/2024 10:43, Marijn Suijten wrote:
> > > >>>
> > > >>>> On 2024-05-15 17:09:02, Marc Gonzalez wrote:
> > > >>>>
> > > >>>>> When create_address_space() fails (e.g. when smmu node is disabled)
> > > >>
> > > >> Note that smmu support is going to become a hard dependency with the
> > > >> drm_gpuvm/VM_BIND conversion.. which I think means we should never get
> > > >> far enough to hit this error path..
> > > >
> > > > Does that mean that we will lose GPU support on  MSM8974?
>
> And display support as well :-/
>
> Note that GPU should be disabled by default without smmu.. you can
> override with modparam, but please don't.  It is incredibly insecure,
> you might as well make /dev/mem world readable/writeable.
>
> Is simplefb an option on 8974 or 8226 to keep display support?

Not in a longer term, I still hope to push HDMI PHY/PLL support for
MSM8974, which means dynamic resolution support.

>
> BR,
> -R
>
> > >
> > > Yeah, that was brought up on #freedreno some time ago
> >
> > Also on MSM8226 which I also care about...
> >
> > Anyone at all knowledgable on IOMMU would be very welcome to help out
> > with IOMMU support on these two platforms (and anything else that
> > old?) in any case, since me and some other people have looked at this
> > (on and off) for years but haven't gotten to any stable or usable point
> > unfortunately.
> >
> > Regards
> > Luca
> >
> > >
> > > Konrad
> >



-- 
With best wishes
Dmitry

Re: [PATCH] drm/msm: log iommu init failure

[Rob Clark]

On Mon, Jun 24, 2024 at 11:29 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Mon, 24 Jun 2024 at 20:59, Rob Clark <robdclark@gmail.com> wrote:
> >
> > On Thu, Jun 20, 2024 at 11:48 PM Luca Weiss <luca.weiss@fairphone.com> wrote:
> > >
> > > On Fri Jun 21, 2024 at 12:47 AM CEST, Konrad Dybcio wrote:
> > > >
> > > >
> > > > On 6/20/24 20:24, Dmitry Baryshkov wrote:
> > > > > On Thu, 20 Jun 2024 at 20:32, Rob Clark <robdclark@gmail.com> wrote:
> > > > >>
> > > > >> On Thu, May 30, 2024 at 2:48 AM Marc Gonzalez <mgonzalez@freebox.fr> wrote:
> > > > >>>
> > > > >>> On 16/05/2024 10:43, Marijn Suijten wrote:
> > > > >>>
> > > > >>>> On 2024-05-15 17:09:02, Marc Gonzalez wrote:
> > > > >>>>
> > > > >>>>> When create_address_space() fails (e.g. when smmu node is disabled)
> > > > >>
> > > > >> Note that smmu support is going to become a hard dependency with the
> > > > >> drm_gpuvm/VM_BIND conversion.. which I think means we should never get
> > > > >> far enough to hit this error path..
> > > > >
> > > > > Does that mean that we will lose GPU support on  MSM8974?
> >
> > And display support as well :-/
> >
> > Note that GPU should be disabled by default without smmu.. you can
> > override with modparam, but please don't.  It is incredibly insecure,
> > you might as well make /dev/mem world readable/writeable.
> >
> > Is simplefb an option on 8974 or 8226 to keep display support?
>
> Not in a longer term, I still hope to push HDMI PHY/PLL support for
> MSM8974, which means dynamic resolution support.

Hmm, maybe it would be possible to re-add carveout support.. but my
hopes aren't too high.  It would be better if we could get smmu going.
(Not to mention, I don't really like the idea of people using the gpu
without an smmu... it is a really insecure thing to do.)

BR,
-R

> >
> > BR,
> > -R
> >
> > > >
> > > > Yeah, that was brought up on #freedreno some time ago
> > >
> > > Also on MSM8226 which I also care about...
> > >
> > > Anyone at all knowledgable on IOMMU would be very welcome to help out
> > > with IOMMU support on these two platforms (and anything else that
> > > old?) in any case, since me and some other people have looked at this
> > > (on and off) for years but haven't gotten to any stable or usable point
> > > unfortunately.
> > >
> > > Regards
> > > Luca
> > >
> > > >
> > > > Konrad
> > >
>
>
>
> --
> With best wishes
> Dmitry

Re: [PATCH] drm/msm: log iommu init failure

[Brian Masney]

On Fri, Jun 21, 2024 at 2:48 AM Luca Weiss <luca.weiss@fairphone.com> wrote:
> On Fri Jun 21, 2024 at 12:47 AM CEST, Konrad Dybcio wrote:
> > On 6/20/24 20:24, Dmitry Baryshkov wrote:
> > > Does that mean that we will lose GPU support on  MSM8974?
> >
> > Yeah, that was brought up on #freedreno some time ago
>
> Also on MSM8226 which I also care about...
>
> Anyone at all knowledgable on IOMMU would be very welcome to help out
> with IOMMU support on these two platforms (and anything else that
> old?) in any case, since me and some other people have looked at this
> (on and off) for years but haven't gotten to any stable or usable point
> unfortunately.

I posted in 2020 some details about what I encountered when I tried to
enable the IOMMU on msm8974:

https://lore.kernel.org/lkml/20200109002606.35653-1-masneyb@onstation.org/

Brian