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

[Jon Hunter <jonathanh@nvidia.com>]

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@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()?

Cheers
Jon

-- 
nvpublic