Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display
On Mon Feb 26, 2024 at 1:08 PM CET, Robert Foss wrote:
> On Mon, Feb 26, 2024 at 12:36 PM Javier Martinez Canillas
> wrote:
> >
> > Thomas Zimmermann writes:
> >
> > > Hi
> > >
> > > Am 23.02.24 um 16:03 schrieb Thierry Reding:
> > >> From: Thierry Reding
> > >>
> > >> Tegra DRM doesn't support display on Tegra234 and later, so make sure
> > >> not to remove any existing framebuffers in that case.
> > >>
> > >> v2: - add comments explaining how this situation can come about
> > >> - clear DRIVER_MODESET and DRIVER_ATOMIC feature bits
> > >>
>
> Fixes: 6848c291a54f ("drm/aperture: Convert drivers to aperture interfaces")
>
> Maybe this is more of a philosophical question, but either the
> introduction of this hardware generation is where this regression was
> introduced or this possibly this commit.
>
> Either way, I'd like to get this into the drm-misc-fixes branch.
That commit looks about right. Technically Tegra234 support was
introduced in Linux 5.10 but the first platform where you we would've
seen this wasn't added until 5.17. The above commit is from 5.14, which
puts it about right in between there, so I think that's fine.
Backporting to anything before 5.14 would need to be manual and isn't
worth it.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display
On Fri, 23 Feb 2024 16:03:33 +0100, Thierry Reding wrote: > From: Thierry Reding > > Tegra DRM doesn't support display on Tegra234 and later, so make sure > not to remove any existing framebuffers in that case. > > v2: - add comments explaining how this situation can come about > - clear DRIVER_MODESET and DRIVER_ATOMIC feature bits > > [...] Applied, thanks! [1/1] drm/tegra: Remove existing framebuffer only if we support display https://cgit.freedesktop.org/drm/drm-misc/commit/?id=86bf8cfda6d2 Rob
Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display
On Mon, Feb 26, 2024 at 12:36 PM Javier Martinez Canillas
wrote:
>
> Thomas Zimmermann writes:
>
> > Hi
> >
> > Am 23.02.24 um 16:03 schrieb Thierry Reding:
> >> From: Thierry Reding
> >>
> >> Tegra DRM doesn't support display on Tegra234 and later, so make sure
> >> not to remove any existing framebuffers in that case.
> >>
> >> v2: - add comments explaining how this situation can come about
> >> - clear DRIVER_MODESET and DRIVER_ATOMIC feature bits
> >>
Fixes: 6848c291a54f ("drm/aperture: Convert drivers to aperture interfaces")
Maybe this is more of a philosophical question, but either the
introduction of this hardware generation is where this regression was
introduced or this possibly this commit.
Either way, I'd like to get this into the drm-misc-fixes branch.
> >> Signed-off-by: Thierry Reding
> >
> > Makes sense as far as the aperture helpers are concerned.
> >
> > Reviewed-by: Thomas Zimmermann
> >
>
> I believe this is drm-misc-fixes material. Since the tegra DRM will remove
> simple{fb,drm} for Tegra234, even when the driver does not support display
> on that platform, leaving the system with no display output at all.
>
> Are you going to push this patch or is going to be done by Thierry?
I'm on it.
>
> > Best regards
> > Thomas
> >
>
> --
> Best regards,
>
> Javier Martinez Canillas
> Core Platforms
> Red Hat
>
Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display
Thomas Zimmermann writes:
> Hi
>
> Am 23.02.24 um 16:03 schrieb Thierry Reding:
>> From: Thierry Reding
>>
>> Tegra DRM doesn't support display on Tegra234 and later, so make sure
>> not to remove any existing framebuffers in that case.
>>
>> v2: - add comments explaining how this situation can come about
>> - clear DRIVER_MODESET and DRIVER_ATOMIC feature bits
>>
>> Signed-off-by: Thierry Reding
>
> Makes sense as far as the aperture helpers are concerned.
>
> Reviewed-by: Thomas Zimmermann
>
I believe this is drm-misc-fixes material. Since the tegra DRM will remove
simple{fb,drm} for Tegra234, even when the driver does not support display
on that platform, leaving the system with no display output at all.
Are you going to push this patch or is going to be done by Thierry?
> Best regards
> Thomas
>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display
Hi
Am 23.02.24 um 16:03 schrieb Thierry Reding:
From: Thierry Reding
Tegra DRM doesn't support display on Tegra234 and later, so make sure
not to remove any existing framebuffers in that case.
v2: - add comments explaining how this situation can come about
- clear DRIVER_MODESET and DRIVER_ATOMIC feature bits
Signed-off-by: Thierry Reding
Makes sense as far as the aperture helpers are concerned.
Reviewed-by: Thomas Zimmermann
Best regards
Thomas
---
drivers/gpu/drm/tegra/drm.c | 23 ---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index b1e1a78e30c6..2e1cfe6f6d74 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1220,9 +1220,26 @@ static int host1x_drm_probe(struct host1x_device *dev)
drm_mode_config_reset(drm);
- err = drm_aperture_remove_framebuffers(_drm_driver);
- if (err < 0)
- goto hub;
+ /*
+* Only take over from a potential firmware framebuffer if any CRTCs
+* have been registered. This must not be a fatal error because there
+* are other accelerators that are exposed via this driver.
+*
+* Another case where this happens is on Tegra234 where the display
+* hardware is no longer part of the host1x complex, so this driver
+* will not expose any modesetting features.
+*/
+ if (drm->mode_config.num_crtc > 0) {
+ err = drm_aperture_remove_framebuffers(_drm_driver);
+ if (err < 0)
+ goto hub;
+ } else {
+ /*
+* Indicate to userspace that this doesn't expose any display
+* capabilities.
+*/
+ drm->driver_features &= ~(DRIVER_MODESET | DRIVER_ATOMIC);
+ }
err = drm_dev_register(drm, 0);
if (err < 0)
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display
Thierry Reding writes: Hello Thierry, > From: Thierry Reding > > Tegra DRM doesn't support display on Tegra234 and later, so make sure > not to remove any existing framebuffers in that case. > > v2: - add comments explaining how this situation can come about > - clear DRIVER_MODESET and DRIVER_ATOMIC feature bits > > Signed-off-by: Thierry Reding > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
[PATCH v2] drm/tegra: Remove existing framebuffer only if we support display
From: Thierry Reding
Tegra DRM doesn't support display on Tegra234 and later, so make sure
not to remove any existing framebuffers in that case.
v2: - add comments explaining how this situation can come about
- clear DRIVER_MODESET and DRIVER_ATOMIC feature bits
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/tegra/drm.c | 23 ---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index b1e1a78e30c6..2e1cfe6f6d74 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1220,9 +1220,26 @@ static int host1x_drm_probe(struct host1x_device *dev)
drm_mode_config_reset(drm);
- err = drm_aperture_remove_framebuffers(_drm_driver);
- if (err < 0)
- goto hub;
+ /*
+* Only take over from a potential firmware framebuffer if any CRTCs
+* have been registered. This must not be a fatal error because there
+* are other accelerators that are exposed via this driver.
+*
+* Another case where this happens is on Tegra234 where the display
+* hardware is no longer part of the host1x complex, so this driver
+* will not expose any modesetting features.
+*/
+ if (drm->mode_config.num_crtc > 0) {
+ err = drm_aperture_remove_framebuffers(_drm_driver);
+ if (err < 0)
+ goto hub;
+ } else {
+ /*
+* Indicate to userspace that this doesn't expose any display
+* capabilities.
+*/
+ drm->driver_features &= ~(DRIVER_MODESET | DRIVER_ATOMIC);
+ }
err = drm_dev_register(drm, 0);
if (err < 0)
--
2.43.2
