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

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3149 bytes --]

Hi

Am 07.09.23 um 10:03 schrieb Thierry Reding:
> On Thu, Aug 31, 2023 at 10:04:29AM +0200, Daniel Vetter wrote:
>> On Thu, 31 Aug 2023 at 08:33, Mikko Perttunen <cyndis@kapsi.fi> wrote:
>>>
>>> On 8/30/23 13:19, Thomas Zimmermann wrote:
>>>> Hi
>>>>
>>>> Am 25.08.23 um 15:22 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.
>>>>>
>>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>>> ---
>>>>>    drivers/gpu/drm/tegra/drm.c | 8 +++++---
>>>>>    1 file changed, 5 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
>>>>> index b1e1a78e30c6..7a38dadbc264 100644
>>>>> --- a/drivers/gpu/drm/tegra/drm.c
>>>>> +++ b/drivers/gpu/drm/tegra/drm.c
>>>>> @@ -1220,9 +1220,11 @@ 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;
>>>>> +    if (drm->mode_config.num_crtc > 0) {
>>>>
>>>> If you don't support the hardware, wouldn't it be better to return
>>>> -ENODEV if !num_crtc?
>>>
>>> While display is not supported through TegraDRM on Tegra234+, certain
>>> multimedia accelerators are supported, so we need to finish probe for those.
>>
>> Ideally you also register the tegra driver without DRIVER_MODESET |
>> DRIVER_ATOMIC in that case, to avoid unecessary userspace confusion.
>> Most userspace can cope with a display driver without any crtc, but I
>> think xorg-modesettting actually falls over. Or at least I've seen
>> some hacks that the agx people added to make sure X doesn't
>> accidentally open the wrong driver.
> 
> That's a good point. However I recall from earlier attempts at doing
> something like this in Nouveau (although this is now very long ago) that
> it's not very easy. The problem, as I recall, is that the driver is a
> singleton, so we would essentially be supporting either modesetting or
> not, for any device in the system.

Take a look at struct drm_device.driver_features. It let's you clear the 
flags that your device doesn't support.

https://elixir.bootlin.com/linux/v6.5/source/include/drm/drm_device.h#L128

Best regards
Thomas

> 
> Now, it's unlikely that we'd have a mix of one Tegra DRM driver with
> display support and another without, but it's something that I recall
> back at the time with Nouveau was problematic because you could have the
> Tegra integrated graphics (without display support) and a PCI-connected
> discrete GPU (with display support) within the same system.
> 
> I need to look into it a bit more to see if I can come up with something
> good to account for this.
> 
> Thierry

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]