diff mbox series

drm/msm: log iommu init failure

Message ID 64ec16b9-c680-408c-b547-5debae2f7f87@freebox.fr
State New
Headers show
Series drm/msm: log iommu init failure | expand

Commit Message

Marc Gonzalez May 15, 2024, 3:09 p.m. UTC
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(+)

Comments

Marijn Suijten May 16, 2024, 8:43 a.m. UTC | #1
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
>
Luca Weiss June 21, 2024, 6:48 a.m. UTC | #2
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
Dmitry Baryshkov June 24, 2024, 6:29 p.m. UTC | #3
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
> >
Rob Clark June 24, 2024, 9:54 p.m. UTC | #4
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
diff mbox series

Patch

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