Re: [PATCH] drm/i915: rename preliminary_hw_support to alpha_support

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 31 Oct 2016, "Vivi, Rodrigo" <rodrigo.vivi@intel.com> 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@gmail.com>
>> Cc: Dave Airlie <airlied@gmail.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx