On Tue, 21 May 2019, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> 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@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?
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@xxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
