All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Add module param to test the load detect code
@ 2015-03-03 17:03 Daniel Vetter
  2015-03-03 19:23 ` Jesse Barnes
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Daniel Vetter @ 2015-03-03 17:03 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter

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@virtuousgeek.org>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@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(-)

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;
 	}
@@ -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;
 		intel_release_load_detect_pipe(connector, &tmp);
 	} else
 		status = connector_status_unknown;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Add module param to test the load detect code
  2015-03-03 17:03 [PATCH] drm/i915: Add module param to test the load detect code Daniel Vetter
@ 2015-03-03 19:23 ` Jesse Barnes
  2015-03-03 21:15   ` Chris Wilson
  2015-03-04 14:29 ` shuang.he
  2015-03-27  9:57 ` Ander Conselvan De Oliveira
  2 siblings, 1 reply; 10+ messages in thread
From: Jesse Barnes @ 2015-03-03 19:23 UTC (permalink / raw)
  To: Daniel Vetter, Intel Graphics Development; +Cc: Daniel Vetter

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@virtuousgeek.org>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@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@virtuousgeek.org>

Thanks,
Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Add module param to test the load detect code
  2015-03-03 19:23 ` Jesse Barnes
@ 2015-03-03 21:15   ` Chris Wilson
  2015-03-04  0:06     ` Daniel Vetter
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2015-03-03 21:15 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter

On Tue, Mar 03, 2015 at 11:23:53AM -0800, Jesse Barnes wrote:
> 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@virtuousgeek.org>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@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...

I am not wholly convinced. The primary purpose of the test suite is
prevent bugs of tomorrow, not to chase bugs of yesterday. If we focus too
much on bugs we have fixed, I worry we won't serendipitously detect bugs
early. Yes, bugs cluster and a mistake once made is likely to be made
again (so regression testing is vital) but I think we cross a line if
igt only exercises code written for conformance testing.

Yes to fault injection and the like. No to special conformance paths.

My 2p,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Add module param to test the load detect code
  2015-03-03 21:15   ` Chris Wilson
@ 2015-03-04  0:06     ` Daniel Vetter
  2015-03-04  0:27       ` Jesse Barnes
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2015-03-04  0:06 UTC (permalink / raw)
  To: Chris Wilson, Jesse Barnes, Daniel Vetter,
	Intel Graphics Development, Daniel Vetter

On Tue, Mar 3, 2015 at 10:15 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue, Mar 03, 2015 at 11:23:53AM -0800, Jesse Barnes wrote:
>> 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@virtuousgeek.org>
>> > Cc: Matt Roper <matthew.d.roper@intel.com>
>> > Signed-off-by: Daniel Vetter <daniel.vetter@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...
>
> I am not wholly convinced. The primary purpose of the test suite is
> prevent bugs of tomorrow, not to chase bugs of yesterday. If we focus too
> much on bugs we have fixed, I worry we won't serendipitously detect bugs
> early. Yes, bugs cluster and a mistake once made is likely to be made
> again (so regression testing is vital) but I think we cross a line if
> igt only exercises code written for conformance testing.

Also swapping tests are a solved problem really, we've had an "mlock
most of mem" todo since years and Thomas Wood is implementing it. Well
it's committed for gem_tiled_swapping already

commit 42b02c284ed24871528df8f1b3eaad7fe1554fd9
Author: Thomas Wood <thomas.wood@intel.com>
Date:   Mon Dec 8 11:12:51 2014 +0000

    lib: add a function to lock memory into RAM

what's missing is rolling this out for other tests.

Now I love igt bashing as much as the next person, but maybe check
occasionally whether your rant-du-jour is still relevant ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Add module param to test the load detect code
  2015-03-04  0:06     ` Daniel Vetter
@ 2015-03-04  0:27       ` Jesse Barnes
  2015-03-04  8:56         ` Chris Wilson
  2015-03-04 16:09         ` Daniel Vetter
  0 siblings, 2 replies; 10+ messages in thread
From: Jesse Barnes @ 2015-03-04  0:27 UTC (permalink / raw)
  To: Daniel Vetter, Chris Wilson, Intel Graphics Development, Daniel Vetter

On 03/03/2015 04:06 PM, Daniel Vetter wrote:
> On Tue, Mar 3, 2015 at 10:15 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> On Tue, Mar 03, 2015 at 11:23:53AM -0800, Jesse Barnes wrote:
>>> 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@virtuousgeek.org>
>>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>>> Signed-off-by: Daniel Vetter <daniel.vetter@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...
>>
>> I am not wholly convinced. The primary purpose of the test suite is
>> prevent bugs of tomorrow, not to chase bugs of yesterday. If we focus too
>> much on bugs we have fixed, I worry we won't serendipitously detect bugs
>> early. Yes, bugs cluster and a mistake once made is likely to be made
>> again (so regression testing is vital) but I think we cross a line if
>> igt only exercises code written for conformance testing.
> 
> Also swapping tests are a solved problem really, we've had an "mlock
> most of mem" todo since years and Thomas Wood is implementing it. Well
> it's committed for gem_tiled_swapping already
> 
> commit 42b02c284ed24871528df8f1b3eaad7fe1554fd9
> Author: Thomas Wood <thomas.wood@intel.com>
> Date:   Mon Dec 8 11:12:51 2014 +0000
> 
>     lib: add a function to lock memory into RAM
> 
> what's missing is rolling this out for other tests.
> 
> Now I love igt bashing as much as the next person, but maybe check
> occasionally whether your rant-du-jour is still relevant ...

My "rant du jour" still is.  mlock() is a good solution for some things,
but for the simple task of testing kernel swap out code, just running
that code is the most straightforward thing to do, rather than trying to
contort into it from userspace.

Jesse

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Add module param to test the load detect code
  2015-03-04  0:27       ` Jesse Barnes
@ 2015-03-04  8:56         ` Chris Wilson
  2015-03-04 16:09           ` Jesse Barnes
  2015-03-04 16:09         ` Daniel Vetter
  1 sibling, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2015-03-04  8:56 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter

On Tue, Mar 03, 2015 at 04:27:47PM -0800, Jesse Barnes wrote:
> My "rant du jour" still is.  mlock() is a good solution for some things,
> but for the simple task of testing kernel swap out code, just running
> that code is the most straightforward thing to do, rather than trying to
> contort into it from userspace.

Once upon a time, there was a minor request for MADV_POPULATE and
MADV_INVALIDATE (basically wrappers around get_pages() and put_pages())
which would give you the fuzzy feeling of testing those paths without
actually testing the shrinker. But no use cases.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Add module param to test the load detect code
  2015-03-03 17:03 [PATCH] drm/i915: Add module param to test the load detect code Daniel Vetter
  2015-03-03 19:23 ` Jesse Barnes
@ 2015-03-04 14:29 ` shuang.he
  2015-03-27  9:57 ` Ander Conselvan De Oliveira
  2 siblings, 0 replies; 10+ messages in thread
From: shuang.he @ 2015-03-04 14:29 UTC (permalink / raw)
  To: shuang.he, ethan.gao, intel-gfx, daniel.vetter

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5880
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              278/278              276/278
ILK                                  308/308              308/308
SNB                                  284/284              284/284
IVB                                  380/380              380/380
BYT                                  294/294              294/294
HSW                                  387/387              387/387
BDW                 -1              316/316              315/316
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt_gem_userptr_blits_coherency-sync      NO_RESULT(1)CRASH(8)NRUN(1)PASS(8)      CRASH(1)PASS(1)
 PNV  igt_gem_userptr_blits_coherency-unsync      FAIL(1)NO_RESULT(1)CRASH(6)PASS(7)      CRASH(1)PASS(1)
*BDW  igt_gem_gtt_hog      PASS(19)      DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Add module param to test the load detect code
  2015-03-04  0:27       ` Jesse Barnes
  2015-03-04  8:56         ` Chris Wilson
@ 2015-03-04 16:09         ` Daniel Vetter
  1 sibling, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2015-03-04 16:09 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: Daniel Vetter, Daniel Vetter, Intel Graphics Development

On Tue, Mar 03, 2015 at 04:27:47PM -0800, Jesse Barnes wrote:
> On 03/03/2015 04:06 PM, Daniel Vetter wrote:
> > On Tue, Mar 3, 2015 at 10:15 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >> On Tue, Mar 03, 2015 at 11:23:53AM -0800, Jesse Barnes wrote:
> >>> 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@virtuousgeek.org>
> >>>> Cc: Matt Roper <matthew.d.roper@intel.com>
> >>>> Signed-off-by: Daniel Vetter <daniel.vetter@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...
> >>
> >> I am not wholly convinced. The primary purpose of the test suite is
> >> prevent bugs of tomorrow, not to chase bugs of yesterday. If we focus too
> >> much on bugs we have fixed, I worry we won't serendipitously detect bugs
> >> early. Yes, bugs cluster and a mistake once made is likely to be made
> >> again (so regression testing is vital) but I think we cross a line if
> >> igt only exercises code written for conformance testing.
> > 
> > Also swapping tests are a solved problem really, we've had an "mlock
> > most of mem" todo since years and Thomas Wood is implementing it. Well
> > it's committed for gem_tiled_swapping already
> > 
> > commit 42b02c284ed24871528df8f1b3eaad7fe1554fd9
> > Author: Thomas Wood <thomas.wood@intel.com>
> > Date:   Mon Dec 8 11:12:51 2014 +0000
> > 
> >     lib: add a function to lock memory into RAM
> > 
> > what's missing is rolling this out for other tests.
> > 
> > Now I love igt bashing as much as the next person, but maybe check
> > occasionally whether your rant-du-jour is still relevant ...
> 
> My "rant du jour" still is.  mlock() is a good solution for some things,
> but for the simple task of testing kernel swap out code, just running
> that code is the most straightforward thing to do, rather than trying to
> contort into it from userspace.

swap-out/in isn't really simple. We could extract the bit 17 swizzling
logic into a separate bit with everything else mock-ups, but that's just
testing our sw implementation against our sw model. Not that useful imo,
and like Chris I'm not convinced of the value of lots of such special-case
code.

On top of that bit17 swizzling is but a facet of swap-in/out support.
Active tracking, page dirtying and all that also need to work correctly. I
don't see how you can test that in a simple fashion.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Add module param to test the load detect code
  2015-03-04  8:56         ` Chris Wilson
@ 2015-03-04 16:09           ` Jesse Barnes
  0 siblings, 0 replies; 10+ messages in thread
From: Jesse Barnes @ 2015-03-04 16:09 UTC (permalink / raw)
  To: Chris Wilson, Daniel Vetter, Intel Graphics Development, Daniel Vetter

On 03/04/2015 12:56 AM, Chris Wilson wrote:
> On Tue, Mar 03, 2015 at 04:27:47PM -0800, Jesse Barnes wrote:
>> My "rant du jour" still is.  mlock() is a good solution for some things,
>> but for the simple task of testing kernel swap out code, just running
>> that code is the most straightforward thing to do, rather than trying to
>> contort into it from userspace.
> 
> Once upon a time, there was a minor request for MADV_POPULATE and
> MADV_INVALIDATE (basically wrappers around get_pages() and put_pages())
> which would give you the fuzzy feeling of testing those paths without
> actually testing the shrinker. But no use cases.

Yeah, that sounds cool, but otoh adding stable ABI just for debug is a
little scary...

Jesse

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Add module param to test the load detect code
  2015-03-03 17:03 [PATCH] drm/i915: Add module param to test the load detect code Daniel Vetter
  2015-03-03 19:23 ` Jesse Barnes
  2015-03-04 14:29 ` shuang.he
@ 2015-03-27  9:57 ` Ander Conselvan De Oliveira
  2 siblings, 0 replies; 10+ messages in thread
From: Ander Conselvan De Oliveira @ 2015-03-27  9:57 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>

On Tue, 2015-03-03 at 18:03 +0100, 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@virtuousgeek.org>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@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(-)
> 
> 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;
>  	}
> @@ -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;
>  		intel_release_load_detect_pipe(connector, &tmp);
>  	} else
>  		status = connector_status_unknown;


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-03-27  9:57 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-03 17:03 [PATCH] drm/i915: Add module param to test the load detect code Daniel Vetter
2015-03-03 19:23 ` Jesse Barnes
2015-03-03 21:15   ` Chris Wilson
2015-03-04  0:06     ` Daniel Vetter
2015-03-04  0:27       ` Jesse Barnes
2015-03-04  8:56         ` Chris Wilson
2015-03-04 16:09           ` Jesse Barnes
2015-03-04 16:09         ` Daniel Vetter
2015-03-04 14:29 ` shuang.he
2015-03-27  9:57 ` Ander Conselvan De Oliveira

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.