[Intel-gfx] [PATCH] drm/i915: Add module param to test the load detect code

[Jesse Barnes]

On 03/03/2015 09:03 AM, Daniel Vetter wrote:
> This is useful for writing igts to make sure we don't break this,
> without being forced to own a one of these dinosaurs.
> 
> Suggested-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h    | 1 +
>  drivers/gpu/drm/i915/i915_params.c | 8 +++++++-
>  drivers/gpu/drm/i915/intel_crt.c   | 6 ++++--
>  3 files changed, 12 insertions(+), 3 deletions(-)

See below for comments.

I think there's probably even more room for testing like this.  E.g. the
tiled swapping test could be done this way rather than trying to force
swapping.  Some of the races we try to induce could probably also be
done this way with code in the kernel to trigger the case we're worried
about...

> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e07a1cb5db67..878b16ed61b3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2429,6 +2429,7 @@ struct i915_params {
>  	bool enable_hangcheck;
>  	bool fastboot;
>  	bool prefault_disable;
> +	bool load_detect_test;
>  	bool reset;
>  	bool disable_display;
>  	bool disable_vtd_wa;
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 44f2262a5553..9f7f9a644c45 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -44,6 +44,7 @@ struct i915_params i915 __read_mostly = {
>  	.enable_ips = 1,
>  	.fastboot = 0,
>  	.prefault_disable = 0,
> +	.load_detect_test = 0,
>  	.reset = true,
>  	.invert_brightness = 0,
>  	.disable_display = 0,
> @@ -144,11 +145,16 @@ module_param_named(fastboot, i915.fastboot, bool, 0600);
>  MODULE_PARM_DESC(fastboot,
>  	"Try to skip unnecessary mode sets at boot time (default: false)");
>  
> -module_param_named(prefault_disable, i915.prefault_disable, bool, 0600);
> +module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, 0600);
>  MODULE_PARM_DESC(prefault_disable,
>  	"Disable page prefaulting for pread/pwrite/reloc (default:false). "
>  	"For developers only.");
>  
> +module_param_named_unsafe(load_detect_test, i915.load_detect_test, bool, 0600);
> +MODULE_PARM_DESC(load_detect_test,
> +	"Force-enable the VGA load detect code for testing (default:false). "
> +	"For developers only.");
> +
>  module_param_named(invert_brightness, i915.invert_brightness, int, 0600);
>  MODULE_PARM_DESC(invert_brightness,
>  	"Invert backlight brightness "
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index e66e17af0a56..b3421ac0be57 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -690,7 +690,7 @@ intel_crt_detect(struct drm_connector *connector, bool force)
>  	 * broken monitor (without edid) to work behind a broken kvm (that fails
>  	 * to have the right resistors for HP detection) needs to fix this up.
>  	 * For now just bail out. */
> -	if (I915_HAS_HOTPLUG(dev)) {
> +	if (I915_HAS_HOTPLUG(dev) && !i915.load_detect_test) {
>  		status = connector_status_disconnected;
>  		goto out;
>  	}

Looks fine up to here.

> @@ -706,8 +706,10 @@ intel_crt_detect(struct drm_connector *connector, bool force)
>  	if (intel_get_load_detect_pipe(connector, NULL, &tmp, &ctx)) {
>  		if (intel_crt_detect_ddc(connector))
>  			status = connector_status_connected;
> -		else
> +		else if (INTEL_INFO(dev)->gen < 4)
>  			status = intel_crt_load_detect(crt);
> +		else
> +			status = connector_status_unknown;

This is a subtle change though...  Previously we'd try a ddc read
followed by a load detect if that failed.  With this, we'll only fallb
ack to load detect on pre-gen4.  Are you just trying to avoid using the
load detect code with the load_detect_test flag set, preferring only DDC
in that case?

Otherwise,
Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>

Thanks,
Jesse