[PATCH v2] drm/tegra: Remove existing framebuffer only if we support display

[PATCH v2] drm/tegra: Remove existing framebuffer only if we support display

[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>
---
 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)
-- 
2.43.2

Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display

[Javier Martinez Canillas]

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>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display

[Thomas Zimmermann]

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

[Javier Martinez Canillas]

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
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display

[Robert Foss]

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
>

Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display

[Robert Foss]

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

Re: [PATCH v2] drm/tegra: Remove existing framebuffer only if we support display

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]