Re: [Intel-gfx] [PATCH] drm/i915: add force_probe module parameter to replace alpha_support
On Tue, 21 May 2019, Rodrigo Vivi 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, ",[,...]" 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
>> Cc: Rodrigo Vivi
>
> First of all I'm sorry for the delay. I could swear that I had
> reviewed it already.
>
>> Signed-off-by: Jani Nikula
>> ---
>> 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 "[,,...]" 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
Thanks, pushed to dinq.
BR,
Jani.
>
>> -
>> #define SKL_REVID_A00x0
>> #define SKL_REVID_B00x1
>> #define SKL_REVID_C00x2
>> 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_unsaf
Re: [Intel-gfx] [PATCH] drm/i915: add force_probe module parameter to replace alpha_support
Quoting Jani Nikula (2019-05-06 16:48:01) > 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, ",[,...]" 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 > Cc: Rodrigo Vivi > Signed-off-by: Jani Nikula Definitely in favor of this: Acked-by: Joonas Lahtinen Regards, Joonas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add force_probe module parameter to replace alpha_support
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, ",[,...]" 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 > Cc: Rodrigo Vivi First of all I'm sorry for the delay. I could swear that I had reviewed it already. > Signed-off-by: Jani Nikula > --- > 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 "[,,...]" 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 > - > #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, 0
Re: [Intel-gfx] [PATCH] drm/i915: add force_probe module parameter to replace alpha_support
Hi Jani,
I love your patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20190506]
[cannot apply to v5.1]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Jani-Nikula/drm-i915-add-force_probe-module-parameter-to-replace-alpha_support/20190507-045927
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-x009-201918 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
All error/warnings (new ones prefixed by >>):
In file included from arch/x86/include/asm/bug.h:83:0,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/firmware.h:7,
from drivers/gpu/drm/i915/intel_csr.c:25:
drivers/gpu/drm/i915/intel_csr.c: In function 'intel_csr_ucode_init':
>> drivers/gpu/drm/i915/intel_csr.c:532:12: error: implicit declaration of
>> function 'IS_ALPHA_SUPPORT'; did you mean 'DP_PSR_SUPPORT'?
>> [-Werror=implicit-function-declaration]
WARN_ON(!IS_ALPHA_SUPPORT(INTEL_INFO(dev_priv)));
^
include/asm-generic/bug.h:131:25: note: in definition of macro 'WARN'
int __ret_warn_on = !!(condition);\
^
>> drivers/gpu/drm/i915/intel_csr.c:532:3: note: in expansion of macro 'WARN_ON'
WARN_ON(!IS_ALPHA_SUPPORT(INTEL_INFO(dev_priv)));
^~~
cc1: some warnings being treated as errors
vim +532 drivers/gpu/drm/i915/intel_csr.c
eb805623 Daniel Vetter 2015-05-04 462
aa9145c4 Animesh Manna 2015-05-13 463 /**
aa9145c4 Animesh Manna 2015-05-13 464 * intel_csr_ucode_init() -
initialize the firmware loading.
f4448375 Daniel Vetter 2015-10-28 465 * @dev_priv: i915 drm device.
aa9145c4 Animesh Manna 2015-05-13 466 *
aa9145c4 Animesh Manna 2015-05-13 467 * This function is called at the
time of loading the display driver to read
aa9145c4 Animesh Manna 2015-05-13 468 * firmware from a .bin file and
copied into a internal memory.
aa9145c4 Animesh Manna 2015-05-13 469 */
f4448375 Daniel Vetter 2015-10-28 470 void intel_csr_ucode_init(struct
drm_i915_private *dev_priv)
eb805623 Daniel Vetter 2015-05-04 471 {
eb805623 Daniel Vetter 2015-05-04 472struct intel_csr *csr =
&dev_priv->csr;
8144ac59 Daniel Vetter 2015-10-28 473
8144ac59 Daniel Vetter 2015-10-28 474INIT_WORK(&dev_priv->csr.work,
csr_load_work_fn);
eb805623 Daniel Vetter 2015-05-04 475
f4448375 Daniel Vetter 2015-10-28 476if (!HAS_CSR(dev_priv))
eb805623 Daniel Vetter 2015-05-04 477return;
eb805623 Daniel Vetter 2015-05-04 478
d8a5b7d7 Jani Nikula 2018-09-26 479/*
d8a5b7d7 Jani Nikula 2018-09-26 480 * Obtain a runtime pm
reference, until CSR is loaded, to avoid entering
d8a5b7d7 Jani Nikula 2018-09-26 481 * runtime-suspend.
d8a5b7d7 Jani Nikula 2018-09-26 482 *
d8a5b7d7 Jani Nikula 2018-09-26 483 * On error, we return with the
rpm wakeref held to prevent runtime
d8a5b7d7 Jani Nikula 2018-09-26 484 * suspend as runtime suspend
*requires* a working CSR for whatever
d8a5b7d7 Jani Nikula 2018-09-26 485 * reason.
d8a5b7d7 Jani Nikula 2018-09-26 486 */
0e6e0be4 Chris Wilson2019-01-14 487
intel_csr_runtime_pm_get(dev_priv);
d8a5b7d7 Jani Nikula 2018-09-26 488
02c07b76 Lucas De Marchi 2018-11-16 489if (INTEL_GEN(dev_priv) >= 12) {
02c07b76 Lucas De Marchi 2018-11-16 490/* Allow to load fw via
parameter using the last known size */
02c07b76 Lucas De Marchi 2018-11-16 491csr->max_fw_size =
GEN12_CSR_MAX_FW_SIZE;
4b225248 Anusha Srivatsa 2019-03-22 492} else if (IS_GEN(dev_priv,
11)) {
7fe78985 Jani Nikula 2018-09-27 493csr->fw_path =
ICL_CSR_PATH;
180e9d23 Jani Nikula 2018-09-26 494csr->required_version =
ICL_CSR_VERSION_REQUIRED;
d8a5b7d7 Jani Nikula 2018-09-26 495csr->max_fw_size =
ICL_CSR_MAX_FW_SIZE;
180e9d23 Jani Nikula 2018-09-26 496} else if
(IS_CANNONLAKE(dev_priv)) {
7fe78985 Jani Nikula 2018-09-27 497csr->fw_path =
CNL_CSR_PATH;
180e9d23 Jani Nikula 2018-09-26 498csr->required_version =
CNL_CSR_VERSION_REQUIRED;
7fe78985 Jani Nikula 2018-09-27 499csr->max_fw_size =
CNL_CSR_MAX_FW_SIZE;
180e9d23 Jani Nikula 2018-09-26 500} else if
(IS_GEMINILAKE(dev_priv)) {
7fe78985 Jani Nikula 2018-09-27 501csr->fw
