On Mon, 31 Oct 2016, "Vivi, Rodrigo" <rodrigo.vivi@xxxxxxxxx> wrote:
> I was about to put my rv-b here. I do believe we need to find a better
> name and the patch was clear and correct. And indeed in a good timing.
>
> However right before clicking the send button I had a vision that this
> will bring another kind of confusion and miss leading.
>
> Traditionally this tag has been removed from new platforms around (but
> not tight to) "PV millestone", few months away from "Alpha milestone".
>
> I understand that it is alpha quality from Alpha to PV, but people might
> think that passing Alpha this is Beta or pre-PV and that the flag should
> be removed right after alpha.
> Also from PO to Alpha we cannot say it is alpha quality. It is just too
> preliminary yet.
I was mostly referring to what it looks on the outside, but it would be
great if it also made sense internally.
BR,
Jani.
>
> Maybe we just remove the "_hw_" and improve the texts?
>
> Thanks,
> Rodrigo.
>
>
> On Mon, 2016-10-31 at 12:18 +0200, Jani Nikula wrote:
>> The term "preliminary hardware support" has always caused confusion both
>> among users and developers. It has always been about preliminary driver
>> support for new hardware, and not so much about preliminary hardware. Of
>> course, initially both the software and hardware are in early stages,
>> but the distinction becomes more clear when the user picks up production
>> hardware and an older kernel to go with it, with just the early support
>> we had for the hardware at the time the kernel was released. The user
>> has to specifically enable the alpha quality *driver* support for the
>> hardware in that specific kernel version.
>
>
>>
>> Rename preliminary_hw_support to alpha_support to emphasize that the
>> module parameter, config option, and flag are about software, not about
>> hardware. Improve the language in help texts and debug logging as well.
>>
>> This appears to be a good time to do the change, as there are currently
>> no platforms with preliminary^W alpha support.
>>
>> Cc: Rob Clark <robdclark@xxxxxxxxx>
>> Cc: Dave Airlie <airlied@xxxxxxxxx>
>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
>> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/Kconfig | 17 +++++++++++------
>> drivers/gpu/drm/i915/i915_drv.h | 4 ++--
>> drivers/gpu/drm/i915/i915_params.c | 9 +++++----
>> drivers/gpu/drm/i915/i915_params.h | 2 +-
>> drivers/gpu/drm/i915/i915_pci.c | 7 ++++---
>> 5 files changed, 23 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
>> index df96aed6975a..36941afba43f 100644
>> --- a/drivers/gpu/drm/i915/Kconfig
>> +++ b/drivers/gpu/drm/i915/Kconfig
>> @@ -36,15 +36,20 @@ config DRM_I915
>>
>> If "M" is selected, the module will be called i915.
>>
>> -config DRM_I915_PRELIMINARY_HW_SUPPORT
>> - bool "Enable preliminary support for prerelease Intel hardware by default"
>> +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 prerelease Intel hardware and want the
>> - i915 driver to support it by default. You can enable such support at
>> - runtime with the module option i915.preliminary_hw_support=1; this
>> - option changes the default for that module option.
>> + 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.
>> +
>> + 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.
>>
>> If in doubt, say "N".
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 42a499681966..abddafba6220 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -668,7 +668,7 @@ struct intel_csr {
>> func(is_skylake); \
>> func(is_broxton); \
>> func(is_kabylake); \
>> - func(is_preliminary); \
>> + func(is_alpha_support); \
>> /* Keep has_* in alphabetical order */ \
>> func(has_csr); \
>> func(has_ddi); \
>> @@ -2782,7 +2782,7 @@ struct drm_i915_cmd_table {
>> #define IS_SKL_GT4(dev_priv) (IS_SKYLAKE(dev_priv) && \
>> (INTEL_DEVID(dev_priv) & 0x00F0) == 0x0030)
>>
>> -#define IS_PRELIMINARY_HW(intel_info) ((intel_info)->is_preliminary)
>> +#define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
>>
>> #define SKL_REVID_A0 0x0
>> #define SKL_REVID_B0 0x1
>> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
>> index 629e4334719c..d46ffe7086bc 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -39,7 +39,7 @@ struct i915_params i915 __read_mostly = {
>> .enable_hangcheck = true,
>> .enable_ppgtt = -1,
>> .enable_psr = -1,
>> - .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
>> + .alpha_support = IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT),
>> .disable_power_well = -1,
>> .enable_ips = 1,
>> .fastboot = 0,
>> @@ -145,9 +145,10 @@ MODULE_PARM_DESC(enable_psr, "Enable PSR "
>> "(0=disabled, 1=enabled - link mode chosen per-platform, 2=force link-standby mode, 3=force link-off mode) "
>> "Default: -1 (use per-chip default)");
>>
>> -module_param_named_unsafe(preliminary_hw_support, i915.preliminary_hw_support, int, 0400);
>> -MODULE_PARM_DESC(preliminary_hw_support,
>> - "Enable preliminary hardware support.");
>> +module_param_named_unsafe(alpha_support, i915.alpha_support, int, 0400);
>> +MODULE_PARM_DESC(alpha_support,
>> + "Enable alpha quality driver support for latest hardware. "
>> + "See also CONFIG_DRM_I915_ALPHA_SUPPORT.");
>>
>> module_param_named_unsafe(disable_power_well, i915.disable_power_well, int, 0400);
>> MODULE_PARM_DESC(disable_power_well,
>> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
>> index 94efc899c1ef..817ad959941e 100644
>> --- a/drivers/gpu/drm/i915/i915_params.h
>> +++ b/drivers/gpu/drm/i915/i915_params.h
>> @@ -40,7 +40,7 @@ struct i915_params {
>> int enable_ppgtt;
>> int enable_execlists;
>> int enable_psr;
>> - unsigned int preliminary_hw_support;
>> + unsigned int alpha_support;
>> int disable_power_well;
>> int enable_ips;
>> int invert_brightness;
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> index 31e6edd08dd0..cdf436fe4e24 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -436,9 +436,10 @@ 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;
>>
>> - if (IS_PRELIMINARY_HW(intel_info) && !i915.preliminary_hw_support) {
>> - DRM_INFO("This hardware requires preliminary hardware support.\n"
>> - "See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support\n");
>> + if (IS_ALPHA_SUPPORT(intel_info) && !i915.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");
>> return -ENODEV;
>> }
>>
>
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
