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

[Jani Nikula <jani.nikula@intel.com>]

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.

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