[Intel-gfx] [PATCH] drm/i915: add force_probe module parameter to replace alpha_support

Jani Nikula jani.nikula at intel.com
Fri May 31 13:47:46 UTC 2019


On Tue, 21 May 2019, Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
> On Mon, May 06, 2019 at 04:48:01PM +0300, Jani Nikula wrote:
>> The i915.alpha_support module parameter has caused some confusion along
>> the way. Add new i915.force_probe parameter to specify PCI IDs of
>> devices to probe, when the devices are recognized but not automatically
>> probed by the driver. The name is intended to reflect what the parameter
>> effectively does, avoiding any overloaded semantics of "alpha" and
>> "support".
>> 
>> The parameter supports "" to disable, "<pci-id>,[<pci-id>,...]" to
>> enable force probe for one or more devices, and "*" to enable force
>> probe for all known devices.
>> 
>> Also add new CONFIG_DRM_I915_FORCE_PROBE config option to replace the
>> DRM_I915_ALPHA_SUPPORT option. This defaults to "*" if
>> DRM_I915_ALPHA_SUPPORT=y.
>> 
>> Instead of replacing i915.alpha_support immediately, let the two coexist
>> for a while, with a deprecation message, for a transition period.
>> 
>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
> First of all I'm sorry for the delay. I could swear that I had
> reviewed it already.
>
>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>> ---
>>  drivers/gpu/drm/i915/Kconfig             | 29 +++++++++-----
>>  drivers/gpu/drm/i915/i915_drv.h          |  2 -
>>  drivers/gpu/drm/i915/i915_params.c       |  7 +++-
>>  drivers/gpu/drm/i915/i915_params.h       |  1 +
>>  drivers/gpu/drm/i915/i915_pci.c          | 51 +++++++++++++++++++++---
>>  drivers/gpu/drm/i915/intel_device_info.h |  2 +-
>>  6 files changed, 72 insertions(+), 20 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
>> index f05563..e7b617 100644
>> --- a/drivers/gpu/drm/i915/Kconfig
>> +++ b/drivers/gpu/drm/i915/Kconfig
>> @@ -45,19 +45,28 @@ config DRM_I915
>>  config DRM_I915_ALPHA_SUPPORT
>>  	bool "Enable alpha quality support for new Intel hardware by default"
>>  	depends on DRM_I915
>> -	default n
>>  	help
>> -	  Choose this option if you have new Intel hardware and want to enable
>> -	  the alpha quality i915 driver support for the hardware in this kernel
>> -	  version. You can also enable the support at runtime using the module
>> -	  parameter i915.alpha_support=1; this option changes the default for
>> -	  that module parameter.
>> +	  This option is deprecated. Use DRM_I915_FORCE_PROBE option instead.
>>  
>> -	  It is recommended to upgrade to a kernel version with proper support
>> -	  as soon as it is available. Generally fixes for platforms with alpha
>> -	  support are not backported to older kernels.
>> +config DRM_I915_FORCE_PROBE
>> +	string "Force probe driver for selected new Intel hardware"
>> +	depends on DRM_I915
>> +	default "*" if DRM_I915_ALPHA_SUPPORT
>> +	help
>> +	  This is the default value for the i915.force_probe module
>> +	  parameter. Using the module parameter overrides this option.
>>  
>> -	  If in doubt, say "N".
>> +	  Force probe the driver for new Intel graphics devices that are
>> +	  recognized but not properly supported by this kernel version. It is
>> +	  recommended to upgrade to a kernel version with proper support as soon
>> +	  as it is available.
>> +
>> +	  Use "" to disable force probe. If in doubt, use this.
>> +
>> +	  Use "<pci-id>[,<pci-id>,...]" to force probe the driver for listed
>
> This is kind of confusing.... "[]" suggest optional, but based on the line above
> "" is also allowed.
>
>> +	  devices. For example, "4500" or "4500,4571".
>
> But it is good that we have an example here so I won't worry much and it
> is up to you on how to proceed.
>
>> +
>> +	  Use "*" to force probe the driver for all known devices.
>>  
>>  config DRM_I915_CAPTURE_ERROR
>>  	bool "Enable capturing GPU state following a hang"
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 64fa35..04415d 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2435,8 +2435,6 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>>  #define IS_ICL_WITH_PORT_F(dev_priv) \
>>  	IS_SUBPLATFORM(dev_priv, INTEL_ICELAKE, INTEL_SUBPLATFORM_PORTF)
>>  
>> -#define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
>
> We need to remove the usage of this on i915_pci.c...
>
> But I'm wondering how just kbuild bot got that and not CI?

Because I've already removed it in dinq with 117aca43f717
("drm/i915/csr: alpha_support doesn't depend on csr or vice versa").

>
> With this fixed,
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

Thanks, pushed to dinq.

BR,
Jani.

>
>> -
>>  #define SKL_REVID_A0		0x0
>>  #define SKL_REVID_B0		0x1
>>  #define SKL_REVID_C0		0x2
>> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
>> index b5be0a..5b0776 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -87,9 +87,12 @@ i915_param_named_unsafe(enable_psr, int, 0600,
>>  	"(0=disabled, 1=enabled) "
>>  	"Default: -1 (use per-chip default)");
>>  
>> +i915_param_named_unsafe(force_probe, charp, 0400,
>> +	"Force probe the driver for specified devices. "
>> +	"See CONFIG_DRM_I915_FORCE_PROBE for details.");
>> +
>>  i915_param_named_unsafe(alpha_support, bool, 0400,
>> -	"Enable alpha quality driver support for latest hardware. "
>> -	"See also CONFIG_DRM_I915_ALPHA_SUPPORT.");
>> +	"Deprecated. See i915.force_probe.");
>>  
>>  i915_param_named_unsafe(disable_power_well, int, 0400,
>>  	"Disable display power wells when possible "
>> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
>> index 3f14e9..a2bacd0 100644
>> --- a/drivers/gpu/drm/i915/i915_params.h
>> +++ b/drivers/gpu/drm/i915/i915_params.h
>> @@ -64,6 +64,7 @@ struct drm_printer;
>>  	param(int, reset, 2) \
>>  	param(unsigned int, inject_load_failure, 0) \
>>  	param(int, fastboot, -1) \
>> +	param(char *, force_probe, CONFIG_DRM_I915_FORCE_PROBE) \
>>  	/* leave bools at the end to not create holes */ \
>>  	param(bool, alpha_support, IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT)) \
>>  	param(bool, enable_hangcheck, true) \
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> index ffa2ee..892f2ac 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -761,7 +761,7 @@ static const struct intel_device_info intel_icelake_11_info = {
>>  static const struct intel_device_info intel_elkhartlake_info = {
>>  	GEN11_FEATURES,
>>  	PLATFORM(INTEL_ELKHARTLAKE),
>> -	.is_alpha_support = 1,
>> +	.require_force_probe = 1,
>>  	.engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VCS0),
>>  	.ppgtt_size = 36,
>>  };
>> @@ -855,16 +855,57 @@ static void i915_pci_remove(struct pci_dev *pdev)
>>  	pci_set_drvdata(pdev, NULL);
>>  }
>>  
>> +/* is device_id present in comma separated list of ids */
>> +static bool force_probe(u16 device_id, const char *devices)
>> +{
>> +	char *s, *p, *tok;
>> +	bool ret;
>> +
>> +	/* FIXME: transitional */
>> +	if (i915_modparams.alpha_support) {
>> +		DRM_INFO("i915.alpha_support is deprecated, use i915.force_probe=%04x instead\n",
>> +			 device_id);
>> +		return true;
>> +	}
>> +
>> +	if (!devices || !*devices)
>> +		return false;
>> +
>> +	/* match everything */
>> +	if (strcmp(devices, "*") == 0)
>> +		return true;
>> +
>> +	s = kstrdup(devices, GFP_KERNEL);
>> +	if (!s)
>> +		return false;
>> +
>> +	for (p = s, ret = false; (tok = strsep(&p, ",")) != NULL; ) {
>> +		u16 val;
>> +
>> +		if (kstrtou16(tok, 16, &val) == 0 && val == device_id) {
>> +			ret = true;
>> +			break;
>> +		}
>> +	}
>> +
>> +	kfree(s);
>> +
>> +	return ret;
>> +}
>> +
>>  static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  {
>>  	struct intel_device_info *intel_info =
>>  		(struct intel_device_info *) ent->driver_data;
>>  	int err;
>>  
>> -	if (IS_ALPHA_SUPPORT(intel_info) && !i915_modparams.alpha_support) {
>> -		DRM_INFO("The driver support for your hardware in this kernel version is alpha quality\n"
>> -			 "See CONFIG_DRM_I915_ALPHA_SUPPORT or i915.alpha_support module parameter\n"
>> -			 "to enable support in this kernel version, or check for kernel updates.\n");
>> +	if (intel_info->require_force_probe &&
>> +	    !force_probe(pdev->device, i915_modparams.force_probe)) {
>> +		DRM_INFO("Your graphics device %04x is not properly supported by the driver in this\n"
>> +			 "kernel version. To force driver probe anyway, use i915.force_probe=%04x\n"
>> +			 "module parameter or CONFIG_DRM_I915_FORCE_PROBE=%04x configuration option,\n"
>> +			 "or (recommended) check for kernel updates.\n",
>> +			 pdev->device, pdev->device, pdev->device);
>>  		return -ENODEV;
>>  	}
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
>> index 5a2e17..0187d6 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.h
>> +++ b/drivers/gpu/drm/i915/intel_device_info.h
>> @@ -105,7 +105,7 @@ enum intel_ppgtt_type {
>>  #define DEV_INFO_FOR_EACH_FLAG(func) \
>>  	func(is_mobile); \
>>  	func(is_lp); \
>> -	func(is_alpha_support); \
>> +	func(require_force_probe); \
>>  	/* Keep has_* in alphabetical order */ \
>>  	func(has_64bit_reloc); \
>>  	func(gpu_reset_clobbers_display); \
>> -- 
>> 2.20.1
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center


More information about the Intel-gfx mailing list