Re: [Intel-xe] [PATCH 1/3] drm/xe: Introduce a module parameter to control XeLink

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

On Tue, 08 Aug 2023, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Tue, Aug 08, 2023 at 12:45:32PM -0400, David Kershner wrote:
>> Add a new module parameter to control enable/disable of the XeLink
>> feature.
>> 
>> Signed-off-by: David Kershner <david.kershner@intel.com>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Basically the direction from upstream is crystal clear, and has been for
years: never ask users to modify module parameters for normal operation.

Also, it's not possible to adjust this per-device.

Why are we still doing this?


BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/xe/Makefile    |  3 ++-
>>  drivers/gpu/drm/xe/xe_xelink.c | 17 +++++++++++++++++
>>  2 files changed, 19 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/gpu/drm/xe/xe_xelink.c
>> 
>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>> index 1b59702cd9f9..d46558cf12a9 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -115,7 +115,8 @@ xe-y += xe_bb.o \
>>  	xe_vm_madvise.o \
>>  	xe_wait_user_fence.o \
>>  	xe_wa.o \
>> -	xe_wopcm.o
>> +	xe_wopcm.o \
>> +	xe_xelink.o
>>  
>>  # i915 Display compat #defines and #includes
>>  subdir-ccflags-$(CONFIG_DRM_XE_DISPLAY) += \
>> diff --git a/drivers/gpu/drm/xe/xe_xelink.c b/drivers/gpu/drm/xe/xe_xelink.c
>> new file mode 100644
>> index 000000000000..51a31f6a4740
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_xelink.c
>> @@ -0,0 +1,17 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +
>> +#include <linux/moduleparam.h>
>> +
>> +/*
>> + * This module parameter is needed because SRIOV PF and XeLink are mutually
>> + * exclusive. The driver needs to be able to enable/disable the XeLink
>> + * infrastructure (specifically Device Physical Addressing).  Since there
>> + * will be no enable/disable for the SRIOV PF path, this parameter is
>> + * needed to explicitly disable XeLink when SRIOV PF is required.
>> + */
>> +static bool xe_enable_xelink = true;
>> +module_param_named(enable_xelink, xe_enable_xelink, bool, 0400);
>> +MODULE_PARM_DESC(enable_xelink, "Enable XeLink feature (default: true)");
>> -- 
>> 2.35.1
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center