[PATCH v3 05/12] drm/fbdev: Store fbdev module parameters in separate file

[Cavitt, Jonathan]

-----Original Message-----
From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Thomas Zimmermann
Sent: Tuesday, October 8, 2024 4:59 AM
To: simona at ffwll.ch; airlied at gmail.com; javierm at redhat.com; jfalempe at redhat.com
Cc: dri-devel at lists.freedesktop.org; amd-gfx at lists.freedesktop.org; intel-gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org; Thomas Zimmermann <tzimmermann at suse.de>
Subject: [PATCH v3 05/12] drm/fbdev: Store fbdev module parameters in separate file
> 
> The fbdev code does not really belong into drm_kms_helper.ko. But
> there are module parameters that control the behavior of the fbdev
> emulation. It is not possible to remove them from the module without
> breaking someone's installation.
> 
> Therefore move the fbdev module parameters to drm_kms_helper_common.c,
> so that the actual fbdev implementaton can later go into a separate
> module.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>

LGTM.

Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
-Jonathan Cavitt

> ---
>  drivers/gpu/drm/drm_fb_helper.c         | 31 ----------------------
>  drivers/gpu/drm/drm_internal.h          |  7 +++++
>  drivers/gpu/drm/drm_kms_helper_common.c | 35 +++++++++++++++++++++++++
>  3 files changed, 42 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index d5e8994345bb..004f7c437897 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -46,37 +46,6 @@
>  #include "drm_internal.h"
>  #include "drm_crtc_internal.h"
>  
> -static bool drm_fbdev_emulation = true;
> -module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600);
> -MODULE_PARM_DESC(fbdev_emulation,
> -		 "Enable legacy fbdev emulation [default=true]");
> -
> -static int drm_fbdev_overalloc = CONFIG_DRM_FBDEV_OVERALLOC;
> -module_param(drm_fbdev_overalloc, int, 0444);
> -MODULE_PARM_DESC(drm_fbdev_overalloc,
> -		 "Overallocation of the fbdev buffer (%) [default="
> -		 __MODULE_STRING(CONFIG_DRM_FBDEV_OVERALLOC) "]");
> -
> -/*
> - * In order to keep user-space compatibility, we want in certain use-cases
> - * to keep leaking the fbdev physical address to the user-space program
> - * handling the fbdev buffer.
> - *
> - * This is a bad habit, essentially kept to support closed-source OpenGL
> - * drivers that should really be moved into open-source upstream projects
> - * instead of using legacy physical addresses in user space to communicate
> - * with other out-of-tree kernel modules.
> - *
> - * This module_param *should* be removed as soon as possible and be
> - * considered as a broken and legacy behaviour from a modern fbdev device.
> - */
> -static bool drm_leak_fbdev_smem;
> -#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)
> -module_param_unsafe(drm_leak_fbdev_smem, bool, 0600);
> -MODULE_PARM_DESC(drm_leak_fbdev_smem,
> -		 "Allow unsafe leaking fbdev physical smem address [default=false]");
> -#endif
> -
>  static LIST_HEAD(kernel_fb_helper_list);
>  static DEFINE_MUTEX(kernel_fb_helper_lock);
>  
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 1705bfc90b1e..9af72bab86d1 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -54,6 +54,13 @@ bool drm_dev_needs_global_mutex(struct drm_device *dev);
>  struct drm_file *drm_file_alloc(struct drm_minor *minor);
>  void drm_file_free(struct drm_file *file);
>  
> +/* drm_kms_helper_common.c */
> +#if defined(CONFIG_DRM_FBDEV_EMULATION)
> +extern bool drm_fbdev_emulation;
> +extern int drm_fbdev_overalloc;
> +extern bool drm_leak_fbdev_smem;
> +#endif
> +
>  #ifdef CONFIG_PCI
>  
>  /* drm_pci.c */
> diff --git a/drivers/gpu/drm/drm_kms_helper_common.c b/drivers/gpu/drm/drm_kms_helper_common.c
> index 0c7550c0462b..cfdbc1ac88dc 100644
> --- a/drivers/gpu/drm/drm_kms_helper_common.c
> +++ b/drivers/gpu/drm/drm_kms_helper_common.c
> @@ -27,6 +27,41 @@
>  
>  #include <linux/module.h>
>  
> +#include "drm_internal.h"
> +
> +#if defined(CONFIG_DRM_FBDEV_EMULATION)
> +bool drm_fbdev_emulation = true;
> +module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600);
> +MODULE_PARM_DESC(fbdev_emulation,
> +		 "Enable legacy fbdev emulation [default=true]");
> +
> +int drm_fbdev_overalloc = CONFIG_DRM_FBDEV_OVERALLOC;
> +module_param(drm_fbdev_overalloc, int, 0444);
> +MODULE_PARM_DESC(drm_fbdev_overalloc,
> +		 "Overallocation of the fbdev buffer (%) [default="
> +		 __MODULE_STRING(CONFIG_DRM_FBDEV_OVERALLOC) "]");
> +
> +/*
> + * In order to keep user-space compatibility, we want in certain use-cases
> + * to keep leaking the fbdev physical address to the user-space program
> + * handling the fbdev buffer.
> + *
> + * This is a bad habit, essentially kept to support closed-source OpenGL
> + * drivers that should really be moved into open-source upstream projects
> + * instead of using legacy physical addresses in user space to communicate
> + * with other out-of-tree kernel modules.
> + *
> + * This module_param *should* be removed as soon as possible and be
> + * considered as a broken and legacy behaviour from a modern fbdev device.
> + */
> +bool drm_leak_fbdev_smem;
> +#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)
> +module_param_unsafe(drm_leak_fbdev_smem, bool, 0600);
> +MODULE_PARM_DESC(drm_leak_fbdev_smem,
> +		 "Allow unsafe leaking fbdev physical smem address [default=false]");
> +#endif
> +#endif
> +
>  MODULE_AUTHOR("David Airlie, Jesse Barnes");
>  MODULE_DESCRIPTION("DRM KMS helper");
>  MODULE_LICENSE("GPL and additional rights");
> -- 
> 2.46.0
> 
>