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@xxxxxxxxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
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@xxxxxxxxx>
> ---
> 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?
With this fixed,
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> -
> #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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
