| Message ID | 20240223150333.1401582-1-thierry.reding@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] drm/tegra: Remove existing framebuffer only if we support display | expand |
Thierry Reding <thierry.reding@gmail.com> writes: Hello Thierry, > From: Thierry Reding <treding@nvidia.com> > > 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 <treding@nvidia.com> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Hi Am 23.02.24 um 16:03 schrieb Thierry Reding: > From: Thierry Reding <treding@nvidia.com> > > 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 <treding@nvidia.com> Makes sense as far as the aperture helpers are concerned. Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> 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(&tegra_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(&tegra_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 <tzimmermann@suse.de> writes: > Hi > > Am 23.02.24 um 16:03 schrieb Thierry Reding: >> From: Thierry Reding <treding@nvidia.com> >> >> 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 <treding@nvidia.com> > > Makes sense as far as the aperture helpers are concerned. > > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > 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 >
On Mon, Feb 26, 2024 at 12:36 PM Javier Martinez Canillas <javierm@redhat.com> wrote: > > Thomas Zimmermann <tzimmermann@suse.de> writes: > > > Hi > > > > Am 23.02.24 um 16:03 schrieb Thierry Reding: > >> From: Thierry Reding <treding@nvidia.com> > >> > >> 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 <treding@nvidia.com> > > > > Makes sense as far as the aperture helpers are concerned. > > > > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > > > > 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 >
On Fri, 23 Feb 2024 16:03:33 +0100, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > 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
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 > <javierm@redhat.com> wrote: > > > > Thomas Zimmermann <tzimmermann@suse.de> writes: > > > > > Hi > > > > > > Am 23.02.24 um 16:03 schrieb Thierry Reding: > > >> From: Thierry Reding <treding@nvidia.com> > > >> > > >> 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
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(&tegra_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(&tegra_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)