Re: [Intel-xe] [PATCH 08/10] fixup! drm/xe/display: Implement display support

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Oct 04, 2023 at 05:37:10PM +0300, Jani Nikula wrote:
> On Wed, 04 Oct 2023, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > On Tue, Oct 03, 2023 at 05:34:55PM +0300, Jani Nikula wrote:
> >> Turn the enable_display module parameter to the same thing as it is for
> >> i915: assuming you have display hardware, take over it, put it to sleep,
> >> and keep connectors disconnected.
> >> 
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h |  5 ++++-
> >>  drivers/gpu/drm/xe/xe_display.c                   | 11 +++++------
> >>  drivers/gpu/drm/xe/xe_module.c                    |  4 ----
> >>  drivers/gpu/drm/xe/xe_module.h                    |  1 -
> >>  drivers/gpu/drm/xe/xe_pci.c                       |  1 -
> >>  5 files changed, 9 insertions(+), 13 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
> >> index e7c9b4ea2153..feaa1e244838 100644
> >> --- a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
> >> +++ b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
> >> @@ -24,6 +24,9 @@
> >>  #include "intel_runtime_pm.h"
> >>  #include <linux/pm_runtime.h>
> >>  
> >> +/* For INTEL_DISPLAY_ENABLED() */
> >> +extern bool enable_display;
> >> +
> >>  static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
> >>  {
> >>  	return container_of(dev, struct drm_i915_private, drm);
> >> @@ -89,7 +92,7 @@ static inline struct drm_i915_private *kdev_to_i915(struct device *kdev)
> >>  
> >>  #define IP_VER(ver, rel)                ((ver) << 8 | (rel))
> >>  
> >> -#define INTEL_DISPLAY_ENABLED(xe) (HAS_DISPLAY((xe)) && !intel_opregion_headless_sku((xe)))
> >> +#define INTEL_DISPLAY_ENABLED(xe) (enable_display && !intel_opregion_headless_sku((xe)))
> >>  #define DISPLAY_VER(i915)	(DISPLAY_RUNTIME_INFO(i915)->ip.ver)
> >>  #define DISPLAY_VER_FULL(i915)	IP_VER(DISPLAY_RUNTIME_INFO(i915)->ip.ver, \
> >>  				       DISPLAY_RUNTIME_INFO(i915)->ip.rel)
> >> diff --git a/drivers/gpu/drm/xe/xe_display.c b/drivers/gpu/drm/xe/xe_display.c
> >> index 391f08c1caca..75054f78d7ae 100644
> >> --- a/drivers/gpu/drm/xe/xe_display.c
> >> +++ b/drivers/gpu/drm/xe/xe_display.c
> >> @@ -29,6 +29,11 @@
> >>  #include "intel_opregion.h"
> >>  #include "xe_module.h"
> >>  
> >> +/* Only to be used for INTEL_DISPLAY_ENABLED() */
> >> +bool enable_display = true;
> >> +module_param_named(enable_display, enable_display, bool, 0444);
> >> +MODULE_PARM_DESC(enable_display, "Enable display");
> >
> > there's a work in progress to create the unified extern mod_param struct.
> > But this goes on the other way to split the mod_params...
> 
> There's *also* work in progress to split out i915 display related module
> parameters. That would give us unified disable_display on both i915 and
> xe, and we could just nuke xe.enable_display.

Great then!

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> BR,
> Jani.
> 
> >
> >> +
> >>  /* Xe device functions */
> >>  
> >>  static bool has_display(struct xe_device *xe)
> >> @@ -45,9 +50,6 @@ static bool has_display(struct xe_device *xe)
> >>   */
> >>  bool xe_display_driver_probe_defer(struct pci_dev *pdev)
> >>  {
> >> -	if (!enable_display)
> >> -		return 0;
> >> -
> >>  	return intel_display_driver_probe_defer(pdev);
> >>  }
> >>  
> >> @@ -69,9 +71,6 @@ static void xe_display_last_close(struct drm_device *dev)
> >>   */
> >>  void xe_display_driver_set_hooks(struct drm_driver *driver)
> >>  {
> >> -	if (!enable_display)
> >> -		return;
> >> -
> >>  	driver->driver_features |= DRIVER_MODESET | DRIVER_ATOMIC;
> >>  	driver->lastclose = xe_display_last_close;
> >>  }
> >> diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c
> >> index 7194595e7f31..82817a46f887 100644
> >> --- a/drivers/gpu/drm/xe/xe_module.c
> >> +++ b/drivers/gpu/drm/xe/xe_module.c
> >> @@ -19,10 +19,6 @@ bool force_execlist = false;
> >>  module_param_named_unsafe(force_execlist, force_execlist, bool, 0444);
> >>  MODULE_PARM_DESC(force_execlist, "Force Execlist submission");
> >>  
> >> -bool enable_display = true;
> >> -module_param_named(enable_display, enable_display, bool, 0444);
> >> -MODULE_PARM_DESC(enable_display, "Enable display");
> >> -
> >>  u32 xe_force_vram_bar_size;
> >>  module_param_named(vram_bar_size, xe_force_vram_bar_size, uint, 0600);
> >>  MODULE_PARM_DESC(vram_bar_size, "Set the vram bar size(in MiB)");
> >> diff --git a/drivers/gpu/drm/xe/xe_module.h b/drivers/gpu/drm/xe/xe_module.h
> >> index e1da1e9ca5cb..58604ed161cc 100644
> >> --- a/drivers/gpu/drm/xe/xe_module.h
> >> +++ b/drivers/gpu/drm/xe/xe_module.h
> >> @@ -7,7 +7,6 @@
> >>  
> >>  /* Module modprobe variables */
> >>  extern bool force_execlist;
> >> -extern bool enable_display;
> >>  extern u32 xe_force_vram_bar_size;
> >>  extern int xe_guc_log_level;
> >>  extern char *xe_guc_firmware_path;
> >> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> >> index b19506fe3e18..8ee430c6f8b1 100644
> >> --- a/drivers/gpu/drm/xe/xe_pci.c
> >> +++ b/drivers/gpu/drm/xe/xe_pci.c
> >> @@ -575,7 +575,6 @@ static int xe_info_init(struct xe_device *xe,
> >>  	xe->info.has_range_tlb_invalidation = graphics_desc->has_range_tlb_invalidation;
> >>  
> >>  	xe->info.enable_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) &&
> >> -				  enable_display &&
> >>  				  desc->has_display;
> >>  	/*
> >>  	 * All platforms have at least one primary GT.  Any platform with media
> >> -- 
> >> 2.39.2
> >> 
> 
> -- 
> Jani Nikula, Intel