[Intel-gfx] [PATCH] drm/i915: rename preliminary_hw_support to alpha_support

Daniel Vetter daniel at ffwll.ch
Wed Nov 9 11:20:26 UTC 2016


On Mon, Oct 31, 2016 at 07:37:29PM +0200, Jani Nikula wrote:
> On Mon, 31 Oct 2016, "Vivi, Rodrigo" <rodrigo.vivi at 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.

Removing that tag should imo happen around alpha, not around PV. We simply
suck at delivering something working soon enough. Somehow it's really hard
to not regress in modesetting compared to the firmware driver :(

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> 
> 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 at gmail.com>
> >> Cc: Dave Airlie <airlied at gmail.com>
> >> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> >> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula at 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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the Intel-gfx mailing list