[PATCH 42/43] drm/fbdev-generic: Convert to fbdev-ttm

[Thomas Zimmermann]

Hi

Am 22.10.24 um 15:31 schrieb Jon Hunter:
> Hi Thomas,
>
> On 12/03/2024 15:45, Thomas Zimmermann wrote:
>> Only TTM-based drivers use fbdev-generic. Rename it to fbdev-ttm and
>> change the symbol infix from _generic_ to _ttm_. Link the source file
>> into TTM helpers, so that it is only build if TTM-based drivers have
>> been selected. Select DRM_TTM_HELPER for loongson.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>>   Documentation/gpu/drm-kms-helpers.rst         |  2 +-
>>   drivers/gpu/drm/Makefile                      |  5 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  6 +-
>>   .../{drm_fbdev_generic.c => drm_fbdev_ttm.c}  | 80 +++++++++----------
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  4 +-
>>   drivers/gpu/drm/loongson/Kconfig              |  1 +
>>   drivers/gpu/drm/loongson/lsdc_drv.c           |  4 +-
>>   drivers/gpu/drm/nouveau/nouveau_drm.c         |  6 +-
>>   drivers/gpu/drm/qxl/qxl_drv.c                 |  4 +-
>>   drivers/gpu/drm/tiny/bochs.c                  |  4 +-
>>   drivers/gpu/drm/vboxvideo/vbox_drv.c          |  4 +-
>>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c           |  4 +-
>>   include/drm/drm_fbdev_generic.h               | 15 ----
>>   include/drm/drm_fbdev_ttm.h                   | 15 ++++
>>   14 files changed, 77 insertions(+), 77 deletions(-)
>>   rename drivers/gpu/drm/{drm_fbdev_generic.c => drm_fbdev_ttm.c} (76%)
>>   delete mode 100644 include/drm/drm_fbdev_generic.h
>>   create mode 100644 include/drm/drm_fbdev_ttm.h
>
> ...
>
>> diff --git a/include/drm/drm_fbdev_ttm.h b/include/drm/drm_fbdev_ttm.h
>> new file mode 100644
>> index 0000000000000..9e6c3bdf35376
>> --- /dev/null
>> +++ b/include/drm/drm_fbdev_ttm.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: MIT */
>> +
>> +#ifndef DRM_FBDEV_TTM_H
>> +#define DRM_FBDEV_TTM_H
>> +
>> +struct drm_device;
>> +
>> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>> +void drm_fbdev_ttm_setup(struct drm_device *dev, unsigned int 
>> preferred_bpp);
>> +#else
>> +static inline void drm_fbdev_ttm_setup(struct drm_device *dev, 
>> unsigned int preferred_bpp)
>> +{ }
>> +#endif
>> +
>> +#endif
>
>
> I recently noticed that with Linux v6.11 it is possible to enable
> CONFIG_DRM_FBDEV_EMULATION without enabling CONFIG_DRM_TTM_HELPER. Now
> while this does not currently cause any build issues, I believe that if
> the function drm_fbdev_ttm_setup() is ever used somewhere that does not
> explicitly select CONFIG_DRM_TTM_HELPER, then you could get a 'ERROR:
> modpost: "drm_fbdev_ttm_setup"'.
>
> So I was thinking that may be this should be ...
>
> diff --git a/include/drm/drm_fbdev_ttm.h b/include/drm/drm_fbdev_ttm.h
> index 9e6c3bdf3537..5b6723a37caa 100644
> --- a/include/drm/drm_fbdev_ttm.h
> +++ b/include/drm/drm_fbdev_ttm.h
> @@ -5,7 +5,7 @@
>
>  struct drm_device;
>
> -#ifdef CONFIG_DRM_FBDEV_EMULATION
> +#if defined(CONFIG_DRM_FBDEV_EMULATION) && 
> defined(CONFIG_DRM_TTM_HELPER)
>  void drm_fbdev_ttm_setup(struct drm_device *dev, unsigned int 
> preferred_bpp);
>  #else
>  static inline void drm_fbdev_ttm_setup(struct drm_device *dev, 
> unsigned int preferred_bpp)
>
>
> The above function has been removed now in -next, but I believe we could
> have the same problem with drm_fbdev_ttm_helper_fb_probe()?

We'd turn a linker/modpost error into a compiler error. Likely makes no 
difference. And AFAICT every driver that selects TTM also selects 
TTM_HELPER. Drivers without TTM should not use this header.

(DRM_TTM and DRM_TTM_HELPER are separate config options for historical 
reasons. Maybe one of them could go away.)

Best regards
Thomas

>
> Cheers
> Jon
>

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